fix: M1-M11 medium-severity issues from prior code review

cmd/chat.go

@@ -315,7 +315,7 @@ func newChatModel(ref *progRef, systemPrompt string, settings hawkconfig.Setting
 	startup.EndPhase("newChatModel:commandPalette")
 
 	// Pre-warm footer connection line so ctx (e.g. 0k/1.0m) shows on first paint.
-	if m.session != nil && m.session.ContextWindowCached > 0 {
+	if m.session != nil && m.session.ContextWindowCachedValue() > 0 {
 		m.connStatusVal = m.buildConnectionStatusPlain()
 		m.connStatusKey = m.connStatusFingerprint()
 	}
@@ -896,11 +896,11 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 				m.updateViewportContent()
 				return m, nil
 			}
-			next := nextAutonomyTier(m.session.Autonomy)
-			if m.session.Autonomy == 0 || autonomyTierIndex(m.session.Autonomy) < 0 {
+			next := nextAutonomyTier(m.session.PermSvc().Autonomy())
+			if m.session.PermSvc().Autonomy() == 0 || autonomyTierIndex(m.session.PermSvc().Autonomy()) < 0 {
 				next = DefaultContainerAutonomy
 			}
-			m.session.Autonomy = next
+			m.session.PermSvc().SetAutonomy(next)
 			m.invalidateConnStatus()
 			m.messages = append(m.messages, displayMsg{role: "system", content: formatAutonomyTierMessage(next)})
 			m.viewDirty = true
@@ -1275,17 +1275,17 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		if msg.sandbox != nil {
 			m.containerSandbox = msg.sandbox
 			if m.session != nil {
-				m.session.ContainerExecutor = msg.sandbox
+				m.session.SetContainerExecutor(msg.sandbox)
 			}
 		}
 		if msg.ready && m.session != nil {
-			if m.session.Autonomy == 0 {
-				m.session.Autonomy = DefaultContainerAutonomy
+			if m.session.PermSvc().Autonomy() == 0 {
+				m.session.PermSvc().SetAutonomy(DefaultContainerAutonomy)
 			}
 			if m.phase == phaseWelcomeGate {
 				m.sandboxReadyPending = true
 			} else {
-				m.messages = append(m.messages, displayMsg{role: "system", content: formatSandboxReadyAutonomyMessage(m.session.Autonomy)})
+				m.messages = append(m.messages, displayMsg{role: "system", content: formatSandboxReadyAutonomyMessage(m.session.PermSvc().Autonomy())})
 			}
 			m.invalidateConnStatus()
 		}
@@ -1294,8 +1294,8 @@ func (m chatModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 			m.containerEnabled = false
 			m.containerReady = false
 			if m.session != nil {
-				m.session.ContainerRequired = false
-				m.session.ContainerExecutor = nil
+				m.session.SetContainerRequired(false)
+				m.session.SetContainerExecutor(nil)
 			}
 			m.messages = append(m.messages, displayMsg{
 				role:    "system",

cmd/chat_commands_session.go

@@ -11,7 +11,7 @@
 
 	tea "github.com/charmbracelet/bubbletea"
 
 	"github.com/GrayCodeAI/eyrie/client"
 	"github.com/GrayCodeAI/hawk/internal/home"
 	"github.com/GrayCodeAI/hawk/internal/session"
 )
@@ -200,7 +200,7 @@
 
 	case "/fork":
 		// If convodag is active, fork from the current head node
-		if m.session.ConvoDAG != nil {
+		if m.session.Persistence().DAG() != nil {
 			headID := m.session.ConvoHead()
 			if headID == "" {
 				m.messages = append(m.messages, displayMsg{role: "error", content: "No conversation to fork from."})
@@ -415,7 +415,7 @@
 	case "/session":
 		info := fmt.Sprintf("Session: %s\nModel: %s/%s\nPermission mode: %s\nMessages: %d\nTools: %d\n%s",
 			m.sessionID, m.session.Provider(), m.session.Model(),
-			permissionModeLabel(m.session), m.session.MessageCount(), len(m.registry.EyrieTools()), m.session.Cost.Summary())
+			permissionModeLabel(m.session), m.session.MessageCount(), len(m.registry.EyrieTools()), m.session.CostValue().Summary())
 		m.messages = append(m.messages, displayMsg{role: "system", content: info})
 		return m, nil
 

cmd/chat_commands_util.go

@@ -123,7 +123,7 @@ func (m *chatModel) mcpSummary() string {
 
 func sessionStats(sess *engine.Session, id string) string {
 	return fmt.Sprintf("Session: %s\nMessages: %d\nModel: %s/%s\n%s",
-		id, sess.MessageCount(), sess.Provider(), sess.Model(), sess.Cost.Summary())
+		id, sess.MessageCount(), sess.Provider(), sess.Model(), sess.CostValue().Summary())
 }
 
 func hooksSummary() string {

cmd/chat_model_test.go

@@ -12,7 +12,7 @@ import (
 
 func newTestChatModel() *chatModel {
 	sess := engine.NewSession("", "test-model", "you are helpful", nil)
-	sess.MaxTurns = 1
+	sess.PermSvc().SetMaxTurns(1)
 	sess.SetTestClient(engine.NewMockClientForTest())
 
 	m := &chatModel{

cmd/chat_status.go

@@ -139,7 +139,7 @@ func (m chatModel) connectionStatusParts() (gateway, model, contextLabel string)
 	model, contextLabel = modelStatusMeta(gw, modelID)
 	if contextLabel == "" || contextLabel == "—" || contextLabel == "0k" {
 		if m.session != nil {
-			if w := m.session.ContextWindowCached; w > 0 {
+			if w := m.session.ContextWindowCachedValue(); w > 0 {
 				contextLabel = formatModelTableContext(w)
 			} else if w := m.session.ContextWindowSize(); w > 0 && w != engine.DefaultContextWindow {
 				contextLabel = formatModelTableContext(w)
@@ -149,7 +149,7 @@ func (m chatModel) connectionStatusParts() (gateway, model, contextLabel string)
 			if w := platformContextForNativeModel(modelID); w > 0 {
 				contextLabel = formatModelTableContext(w)
 				if m.session != nil {
-					m.session.ContextWindowCached = w
+					m.session.SetContextWindowCached(w)
 					m.session.EnsureAutoCompactor()
 				}
 			}

cmd/chat_subcommand.go

@@ -89,7 +89,10 @@ func NewSubcommandRegistry() *SubcommandRegistry {
 // all aliases are indexed. If a name is already registered, this
 // is a no-op (the existing entry is kept) — duplicate registration
 // is treated as a configuration error but doesn't panic, so test
-// ordering and re-init don't blow up the binary.
+// ordering and re-init don't blow up the binary. The same applies
+// to alias collisions: if any of the subcommand's aliases is already
+// registered (either as a primary or as another alias), registration
+// is rejected (see M5 in the code review).
 func (r *SubcommandRegistry) Register(cmd ChatSubcommand) {
 	if cmd == nil {
 		return
@@ -100,6 +103,11 @@ func (r *SubcommandRegistry) Register(cmd ChatSubcommand) {
 	if _, exists := r.primary[name]; exists {
 		return // duplicate
 	}
+	for _, alias := range cmd.Aliases() {
+		if _, exists := r.aliasOf[alias]; exists {
+			return // alias collision
+		}
+	}
 	r.primary[name] = cmd
 	for _, alias := range cmd.Aliases() {
 		r.aliasOf[alias] = name

cmd/chat_subcommand_branches.go

@@ -14,7 +14,7 @@ func (b *branchesSubcommand) Aliases() []string   { return nil }
 func (b *branchesSubcommand) Description() string { return "list conversation DAG branches" }
 func (b *branchesSubcommand) Usage() string       { return "" }
 func (b *branchesSubcommand) Handle(m *chatModel, args []string, text string) (tea.Model, tea.Cmd) {
-	if m.session.ConvoDAG == nil {
+	if m.session.Persistence().DAG() == nil {
 		m.messages = append(m.messages, displayMsg{role: "system", content: "No conversation branches (DAG not active)."})
 		return m, nil
 	}

cmd/chat_subcommand_cost.go

@@ -13,7 +13,7 @@ func (c *costSubcommand) Aliases() []string   { return nil }
 func (c *costSubcommand) Description() string { return "print session cost and token usage summary" }
 func (c *costSubcommand) Usage() string       { return "" }
 func (c *costSubcommand) Handle(m *chatModel, args []string, text string) (tea.Model, tea.Cmd) {
-	m.messages = append(m.messages, displayMsg{role: "system", content: m.session.Cost.Summary()})
+	m.messages = append(m.messages, displayMsg{role: "system", content: m.session.CostValue().Summary()})
 	return m, nil
 }
 

cmd/chat_subcommand_pin.go

@@ -25,7 +25,7 @@ func (p *pinSubcommand) Handle(m *chatModel, args []string, text string) (tea.Mo
 			n = parsed
 		}
 	}
-	m.session.PinnedMessages = n
+	m.session.SetPinnedMessages(n)
 	m.messages = append(m.messages, displayMsg{role: "system", content: fmt.Sprintf("Pinned last %d messages (protected from compaction).", n)})
 	return m, nil
 }

cmd/chat_subcommand_session.go

@@ -24,20 +24,45 @@ func (s *sessionSubcommand) Description() string {
 	return "session management: clear, compact, diff, recover, resume, history, quit, exit"
 }
 func (s *sessionSubcommand) Usage() string { return "" }
-func (s *sessionSubcommand) Handle(m *chatModel, args []string, text string) (tea.Model, tea.Cmd) {
-	name := ""
+
+// sessionSubcommandNames is the ordered list of slash names covered
+// by sessionSubcommand. Exposed for tests and help rendering.
+var sessionSubcommandNames = []string{"/clear", "/compact", "/diff", "/recover", "/resume", "/history", "/quit", "/exit"}
+
+// resolveSessionName inspects the raw slash text and returns the
+// matching command name (with the leading "/"). Falls back to
+// "/<primary>" when text doesn't start with a slash.
+func resolveSessionName(text string, primary string) string {
 	if len(text) > 0 && text[0] == '/' {
-		for _, c := range []string{"/clear", "/compact", "/diff", "/recover", "/resume", "/history", "/quit", "/exit"} {
+		for _, c := range sessionSubcommandNames {
 			if len(text) >= len(c) && text[:len(c)] == c {
-				name = c
-				break
+				return c
 			}
 		}
 	}
-	if name == "" {
-		name = "/" + s.Name()
-	}
-	return m.handleSessionCommand(name, args, text)
+	return "/" + primary
+}
+
+// buildSessionParts reconstructs the full parts slice that
+// handleSessionCommand expects: parts[0] is the command name,
+// parts[1:] is the post-name argument list. Exposed for testing
+// the dispatcher contract (see M6 in the code review).
+func buildSessionParts(name string, args []string) []string {
+	parts := make([]string, 0, 1+len(args))
+	parts = append(parts, name)
+	parts = append(parts, args...)
+	return parts
+}
+
+func (s *sessionSubcommand) Handle(m *chatModel, args []string, text string) (tea.Model, tea.Cmd) {
+	name := resolveSessionName(text, s.Name())
+	// handleSessionCommand expects parts[0] to be the command name (e.g.
+	// "/recover"), with the remaining entries being the post-name args.
+	// The dispatcher hands us the post-name slice; reconstruct the
+	// full parts slice so /recover <id>, /resume <id>, /tag <label>, etc.
+	// receive the trailing argument.
+	parts := buildSessionParts(name, args)
+	return m.handleSessionCommand(name, parts, text)
 }
 
 func init() {

cmd/chat_subcommand_simple.go

@@ -296,16 +296,16 @@ func init() {
 			case "on":
 				_ = hawkconfig.SetGlobalSetting("glmthinking", "true")
 				enabled := true
-				m.session.GLMThinkingEnabled = &enabled
+				m.session.SetGLMThinkingEnabled(&enabled)
 				m.messages = append(m.messages, displayMsg{role: "system", content: "GLM thinking → enabled"})
 			case "off":
 				_ = hawkconfig.SetGlobalSetting("glmthinking", "false")
 				disabled := false
-				m.session.GLMThinkingEnabled = &disabled
+				m.session.SetGLMThinkingEnabled(&disabled)
 				m.messages = append(m.messages, displayMsg{role: "system", content: "GLM thinking → disabled"})
 			case "default":
 				_ = hawkconfig.SetGlobalSetting("glmthinking", "default")
-				m.session.GLMThinkingEnabled = nil
+				m.session.SetGLMThinkingEnabled(nil)
 				m.messages = append(m.messages, displayMsg{role: "system", content: "GLM thinking → default (model decides)"})
 			default:
 				m.messages = append(m.messages, displayMsg{role: "error", content: "Valid options: on, off, default"})

cmd/chat_subcommand_status.go

@@ -32,7 +32,7 @@ func buildStatusInfo(m *chatModel) string {
 	info := fmt.Sprintf("Session: %s\nModel: %s/%s\nMode: %s\nPermission mode: %s\nMessages: %d\nTools: %d\n%s",
 		m.sessionID, m.session.Provider(), m.session.Model(),
 		m.modeManager.Current().String(),
-		permissionModeLabel(m.session), m.session.MessageCount(), toolCount, m.session.Cost.Summary())
+		permissionModeLabel(m.session), m.session.MessageCount(), toolCount, m.session.CostValue().Summary())
 	if len(addDirs) > 0 {
 		info += "\nAdditional dirs: " + strings.Join(addDirs, ", ")
 	}

cmd/chat_subcommand_test.go

@@ -572,3 +572,89 @@ func TestSnapshotSubcommand_Registered(t *testing.T) {
 		t.Error("Description is empty")
 	}
 }
+
+// --- M6: sessionSubcommand /recover <id> contract ---
+//
+// M6 fix: sessionSubcommand.Handle used to pass the post-name args
+// directly to handleSessionCommand, which expected parts[0] to be
+// the command name. This broke /recover <id>, /resume <id>, and
+// /tag <label>: the trailing arg landed at parts[0] instead of
+// parts[1], so `len(parts) >= 2` saw 1 and reported a usage error.
+//
+// Tests below assert the fix: buildSessionParts produces the
+// expected ["/recover", "id-123"] slice, and the registry-resolved
+// sessionSubcommand sees the id at the right index.
+
+func TestBuildSessionParts_PrependsCommandName(t *testing.T) {
+	parts := buildSessionParts("/recover", []string{"abc-123"})
+	if len(parts) != 2 {
+		t.Fatalf("len(parts) = %d, want 2", len(parts))
+	}
+	if parts[0] != "/recover" {
+		t.Errorf("parts[0] = %q, want /recover", parts[0])
+	}
+	if parts[1] != "abc-123" {
+		t.Errorf("parts[1] = %q, want abc-123 (the session id)", parts[1])
+	}
+}
+
+func TestBuildSessionParts_NoArgs(t *testing.T) {
+	parts := buildSessionParts("/clear", nil)
+	if len(parts) != 1 {
+		t.Fatalf("len(parts) = %d, want 1", len(parts))
+	}
+	if parts[0] != "/clear" {
+		t.Errorf("parts[0] = %q, want /clear", parts[0])
+	}
+}
+
+func TestBuildSessionParts_MultipleArgs(t *testing.T) {
+	parts := buildSessionParts("/tag", []string{"bugfix", "urgent"})
+	if len(parts) != 3 {
+		t.Fatalf("len(parts) = %d, want 3", len(parts))
+	}
+	if parts[0] != "/tag" || parts[1] != "bugfix" || parts[2] != "urgent" {
+		t.Errorf("parts = %v, want [/tag bugfix urgent]", parts)
+	}
+}
+
+func TestSessionSubcommand_RecoverIDReachesPartsIndex1(t *testing.T) {
+	// End-to-end: simulate the dispatcher calling Handle with the
+	// post-name args, then check that the second element of the
+	// parts slice (the one handleSessionCommand reads for the
+	// session id) is the id.
+	const sessionID = "01HXY12345ABCDEF"
+
+	// Use a stub chatModel so we can capture the parts slice
+	// handleSessionCommand would receive without running the full
+	// session-recovery path. We replace the model's handleSessionCommand
+	// via a small interface shim.
+	//
+	// Since chatModel is a concrete struct, we drive the test by
+	// calling buildSessionParts directly with the values the
+	// dispatcher would have produced for "/recover <id>". This
+	// proves the contract; the actual session-resume code path is
+	// covered by the existing recovery tests.
+	parts := buildSessionParts("/recover", []string{sessionID})
+	if len(parts) < 2 {
+		t.Fatalf("handleSessionCommand would see len(parts) = %d, want >= 2 (the usage error path fires otherwise)", len(parts))
+	}
+	if parts[1] != sessionID {
+		t.Fatalf("handleSessionCommand would read parts[1] = %q, want %q (the session id)", parts[1], sessionID)
+	}
+}
+
+func TestResolveSessionName_PicksLongestMatch(t *testing.T) {
+	// /recover must take precedence over /re (no such command, but
+	// the loop iterates in declaration order, so /recover wins when
+	// text starts with /recover).
+	if got := resolveSessionName("/recover abc", "clear"); got != "/recover" {
+		t.Errorf("resolveSessionName(/recover abc) = %q, want /recover", got)
+	}
+	if got := resolveSessionName("/compact", "clear"); got != "/compact" {
+		t.Errorf("resolveSessionName(/compact) = %q, want /compact", got)
+	}
+	if got := resolveSessionName("not-a-slash", "clear"); got != "/clear" {
+		t.Errorf("resolveSessionName(no-slash) = %q, want /clear (fallback to primary)", got)
+	}
+}

cmd/chat_subcommand_usage.go

@@ -13,7 +13,7 @@ func (u *usageSubcommand) Aliases() []string   { return nil }
 func (u *usageSubcommand) Description() string { return "show session token usage (alias for /cost)" }
 func (u *usageSubcommand) Usage() string       { return "" }
 func (u *usageSubcommand) Handle(m *chatModel, args []string, text string) (tea.Model, tea.Cmd) {
-	m.messages = append(m.messages, displayMsg{role: "system", content: m.session.Cost.Summary()})
+	m.messages = append(m.messages, displayMsg{role: "system", content: m.session.CostValue().Summary()})
 	return m, nil
 }
 

cmd/hud_panel.go

@@ -163,7 +163,7 @@ func (m *chatModel) collectHUDData() HUDData {
 	data := HUDData{
 		MissionStatus: "idle",
 	}
-	if m.session != nil && m.session.YaadBridge != nil && m.session.YaadBridge.Ready() {
+	if m.session != nil && m.session.MemorySvc().Yaad() != nil && m.session.MemorySvc().Yaad().Ready() {
 		data.MemoryReady = true
 	}
 	return data

cmd/permissions_center.go

@@ -157,7 +157,7 @@ func permissionCenterSummary(m *chatModel) string {
 	b.WriteString(fmt.Sprintf("  Mode: %s\n", permissionModeLabel(m.session)))
 	b.WriteString(fmt.Sprintf("  Rules: %d allow, %d deny\n", len(allowRules), len(denyRules)))
 	b.WriteString(fmt.Sprintf("  Behavior: %s\n", permissionBehaviorSummary(level)))
-	b.WriteString(fmt.Sprintf("  Mode behavior: %s\n", permissionModeSummary(m.session.Mode)))
+	b.WriteString(fmt.Sprintf("  Mode behavior: %s\n", permissionModeSummary(m.session.ModeValue())))
 	if len(allowRules) > 0 {
 		b.WriteString("  Allow: " + strings.Join(allowRules, ", ") + "\n")
 	}
@@ -298,7 +298,7 @@ func resetPermissionCenter(m *chatModel) {
 	if m == nil || m.session == nil {
 		return
 	}
-	m.session.Autonomy = DefaultContainerAutonomy
+	m.session.PermSvc().SetAutonomy(DefaultContainerAutonomy)
 	m.settings.Autonomy = permissionTierSettingValue(DefaultContainerAutonomy)
 	m.settings.Sandbox = defaultPermissionSandbox
 	sandboxFlag = defaultPermissionSandbox
@@ -324,7 +324,7 @@ func (m *chatModel) handlePermissionsCommand(parts []string) (chatModel, tea.Cmd
 		m.messages = append(m.messages, displayMsg{role: "system", content: permissionCenterSummary(m)})
 	case "mode":
 		if len(parts) < 3 {
-			m.messages = append(m.messages, displayMsg{role: "system", content: fmt.Sprintf("Current mode: %s\nBehavior: %s\nUsage: /permissions mode <default|edits|bypass|dontask|plan>", permissionModeLabel(m.session), permissionModeSummary(m.session.Mode))})
+			m.messages = append(m.messages, displayMsg{role: "system", content: fmt.Sprintf("Current mode: %s\nBehavior: %s\nUsage: /permissions mode <default|edits|bypass|dontask|plan>", permissionModeLabel(m.session), permissionModeSummary(m.session.ModeValue()))})
 			return *m, nil
 		}
 		mode, label, ok := normalizePermissionMode(parts[2])
@@ -347,7 +347,7 @@ func (m *chatModel) handlePermissionsCommand(parts []string) (chatModel, tea.Cmd
 			m.messages = append(m.messages, displayMsg{role: "error", content: "Valid tiers: scout, builder, operator, autonomous"})
 			return *m, nil
 		}
-		m.session.Autonomy = level
+		m.session.PermSvc().SetAutonomy(level)
 		m.settings.Autonomy = permissionTierSettingValue(level)
 		m.messages = append(m.messages, displayMsg{role: "system", content: fmt.Sprintf("Permission tier → %s\nBehavior: %s", label, permissionBehaviorSummary(level))})
 	case "sandbox":

cmd/session_sync.go

@@ -11,7 +11,7 @@ import (
 
 // syncSessionFromPersistedSelection copies eyrie provider.json selection into the
 // live session when the session fields are empty (status bar can show ActiveModel
-// while s.model is still unset, which breaks deployment routing).
+// while the model field is still unset, which breaks deployment routing).
 func syncSessionFromPersistedSelection(sess *engine.Session, settings hawkconfig.Settings) {
 	if sess == nil {
 		return