[v5] [architect] pkg/agent/manager.go: Manager is a god object — 10,167 lines, 179 methods, 20 late-bound callback fields behind one mutex

[kubestellar-hive]

Architecture Finding

Type: anti-pattern / tech-debt (god object, setter-injection temporal coupling)
Affected area: src/pkg/agent/manager.go (type Manager, NewManager, the 24 Set* mutators)

Manager has grown into the process-wide god object for agent lifecycle:

• 10,167 lines in one file, 287 top-level funcs, 179 methods on *Manager.
• The struct carries 20 func-typed callback/resolver fields (bobAPIKeyResolver, linearCredentialResolver, explainModeDefaultResolver, kickObserver, inferenceRouteCallback, persistPauseCallback, recordPromptCallback, sandboxAuditCallback, paneCapture, visiblePaneCapture, sessionAttached, sendLiteralForAgent, sendKeysForAgent, promptDismissSleep, captureFullLogFn, clearHistoryFn, …) wired post-construction through 24 Set* methods instead of constructor parameters or interfaces.
• Only ~5 of those callback reads are nil-guarded, so correctness depends on undocumented call-order contracts between NewManager and the Set* calls in cmd/hive wiring (temporal coupling; a missed setter is a runtime nil-deref, not a compile error).
• Concurrency is funneled through a single m.mu used at 230 sites across unrelated concerns (credential resolution, tmux pane IO, kick delivery, pause bookkeeping), making lock-scope reasoning and future decomposition progressively harder — several fields already escaped to atomic.Pointer precisely to dodge deadlocks, as the in-file comments admit.

Impact

• Every new agent-runtime feature lands in this one file/type; merge conflicts and review blast radius grow superlinearly.
• The unguarded callback fields make startup wiring fragile: reordering cmd/hive initialization can panic only at kick time.
• The single mutex couples slow tmux/pane IO with hot state reads; the ad-hoc atomic.Pointer escape hatches show the lock has already stopped scaling.
• Testing requires poking unexported func fields directly (30+ test-file assignments), locking tests to implementation details.

Recommendation

Phased, behavior-preserving decomposition (each phase independently reviewable):

1. Extract a TerminalSession interface for the 8 tmux/pane fields (paneCapture, visiblePaneCapture, sessionAttached, sendLiteralForAgent, sendKeysForAgent, promptDismissSleep, captureFullLogFn, clearHistoryFn) with the current funcs as the default impl and a fake for tests (~16 prod call sites, mostly manager.go + kick_logs.go).
2. Group credential/backend resolvers (bobAPIKeyResolver, linearCredentialResolver, bobKeySourceResolver, explainModeDefaultResolver, copilot/claude token state) into a CredentialSource value passed to NewManager, eliminating their Set* mutators and the temporal coupling.
3. Split observer callbacks (kickObserver, pauseObserver, sandboxAuditCallback, recordPromptCallback, inference-route pair) into a nil-safe Hooks struct with no-op defaults.
4. Only after 1–3: split manager.go by concern into sibling files (kick delivery, pause state, sandbox, backends).

No behavior change in any phase; each is compile-and-test verifiable.

---

Filed by architect agent (ACMM L5 — hold-gated mode)

🐝 Hive Agent: architect | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=architect backend=copilot model=claude-opus-4-6

[clubanderson]

Decomposition RFC — Manager god object, v5 line

Grounded in the v5 tree as of 9068ecb (post-#5626). The numbers moved since filing: src/pkg/agent/manager.go is now 10,240 lines, 393 top-level declarations, 181 *Manager methods in the file (another 42 methods live in 11 sibling files — kick_logs.go 11, interactive_home.go 6, authprobe.go 5, claude_session_adopt.go 4, audit.go/bob_settings.go/kick_async.go/turn_loss.go 3 each, kick_observer.go 2, stuck_login_diagnosis.go/trajectory_view.go 1 each — so Manager carries ~223 methods package-wide).

This RFC does three things: inventories the 393 declarations by concern, maps the lock discipline that any extraction must preserve, and sequences the phases against the #5555 re-entrant-turn roadmap so nothing here collides with its Phase 4. Slice 1 (the mechanical same-file split, PR linked at the bottom) is the only change shipped now; everything else is sequenced, not started.

1. Method inventory by concern

Derived from the real declarations (receiver-qualified, doc comments included), and realized 1:1 as the slice-1 file map — each concern below is now one same-package file:

Concern	File (slice 1)	Lines	Decls	Funcs	*Manager methods	Representative members
Core: structs, lifecycle, status	manager.go	1,205	29	20	18	Manager, AgentProcess, ProjectContext, NewManager, Start, Stop, AddAgent, ReconcileAgents, RemoveAgent, UpdateConfig, GetStatus, AllStatuses, SessionMissing
Token/auth: mint, refresh, watchdog	manager_tokens.go	864	52	38	27	SetAppAuth, SetAgentMint, issueAgentMintToken, mintAgentTokenUnlocked, StartAgentTokenRefresh, StartCredentialWatchdog, StartCopilotSessionRefresh, SetBobAPIKeyResolver, SetLinearCredentialResolver, SetBobKeySourceResolver
Token/auth: credential files, diagnostics	manager_copilot_auth.go	607	24	15	2	restoreCopilotTokens, writeDurableCopilotToken, copilotIdentityKey, claudeCredentialReachable, runCopilotDiagnostic, fixSharedConfigPerms
Terminal/session IO (the #5638 seams)	manager_tmux.go	630	32	29	21	ensureTmuxSession, captureTmuxPaneForAgent, captureVisiblePaneForAgent, tmuxSessionHasAttachedClientForAgent, tmuxSendLiteralForAgent, tmuxSendKeysForAgent, CaptureFullLog, waitForInputPromptForAgent
UID isolation	manager_uid_isolation.go	124	3	3	2	uidIsolationMarkersReady, publishRuntimeUIDIsolationMarker, awaitUIDIsolation
Launch: command assembly, caveman, failure banners	manager_launch.go	916	18	13	3	launchInTmux, backendLaunchCmd, bobLaunchCmd, toolRulesToLaunchCmd, installCavemanForAgent, announceLaunchFailureInPane
Launch: env + prompt preamble	manager_env.go	526	16	10	9	agentEnvPairs, buildBootstrapPrompt, buildProjectPreamble, findACMMFragments, applySecretEnv, sanitizeGitRemotes, filteredEnv
Watchdog/liveness: output polling, thrash, trust watch	manager_poll.go	596	20	11	7	pollTmuxOutputForAgent, pollTmuxOutput, watchForTrustPrompt, checkBlockedThrash, checkKickRefusal, logOutputSignals
Pane-content classification	manager_pane_signals.go	818	38	22	0	paneShowsLoginPrompt, paneShowsQuotaExhausted, paneShowsTransientAPIError, paneShowsBlockingPrompt family, filterPaneOutput, DeduplicateBlocks
Kick delivery	manager_kick.go	873	29	21	12	SendKick, deliverKickLocked, deliverStartupKick, nudgeIfKickStalled, nudgeIfTransientAPIError, recordInferenceKick, SeedLastKick, SeedKickHistory
Sandboxed kicks	manager_sandbox.go	223	12	12	12	SetSandboxConfig/Launcher/PushMinter/PRClient/AuditCallback, startSandboxKickLocked, runSandboxKick, auditSandbox
Consent/settings prompt dismissal	manager_consent.go	301	9	7	5	dismissInferencePrompts, confirmMenuOption, dismissConsentIfStuck, clearConsentTracking, paneShowsConsentScreen
Pause/breaker state	manager_pause.go	389	13	12	12	PauseByCause, Resume, EngageBreaker, ReleaseBreaker, RestoreBreaker, SeedPauseState, SetPersistPauseCallback, SetPauseTransitionObserver
Crash recovery / restart / reap	manager_restart.go	769	21	17	13	CheckAndRestartCrashedAgents, Restart, RestartWithBootstrap, RestartThenSendKick, RelaunchBobAgentsAwaitingKey, KillSession, reapAgentCLI, killAgentProcesses, SetDeadSessionRecoveryOwner
Backend/model routing + overrides	manager_routing.go	463	25	22	14	routableBackend, validateBackendName, backendBinary, normalizeModelNameForBackend, PinCLI, PinModel, SetModelOverride, SetBackendOverride, RefreshInferenceRoutes, SetInferenceCallbacks
Backend homes + config seeding	manager_homes.go	823	38	24	12	setupCodexHome, healCodexHomeOwnership, chownTreeAsRoot, ensureClaudeSettings, tightenInferenceHome, writeAgentStateFile, seedClaudeUserConfig
Mode/capabilities/authz	manager_modes.go	320	14	14	12	SyncModeFiles, AuthorizePROpen, AuthorizeIssueOpen, AuthorizeMerge, AgentCapabilities, EffectiveBackend, InvocationMetadata, ClearAllModeOverrides

Totals: 393 declarations (290 funcs, 181 *Manager methods) across 17 files; core manager.go drops from 10,240 to 1,205 lines. Both structs — Manager and AgentProcess, including the stall/no-action watchdog and kickLogPending bookkeeping fields — stay in core manager.go on purpose (see §4).

2. What can extract behind interfaces later — and what cannot yet

Phase A — TerminalSession (safe, already designed). #5638 extracted exactly this on v4 (PR open, base v4): the 8 seam fields paneCapture, visiblePaneCapture, sessionAttached, sendLiteralForAgent, sendKeysForAgent, promptDismissSleep, captureFullLogFn, clearHistoryFn collapse to one terminal TerminalSession field with a tmux default (nil ⇒ tmux, preserving zero-value &Manager{} test semantics). It is not on v5 — no terminal.go in the v5 tree. Slice 1 deliberately colocates all eight seams' tmux fallback methods in manager_tmux.go, so the v5 forward-port of #5638 becomes a one-file interface introduction. Do it as its own PR, aligned to #5638's shape (same interface name, same funcTerminal test fake), after #5638 settles on v4 so the two lines don't diverge on the contract.

Phase B — CredentialSource (safe). Group bobAPIKeyResolver, linearCredentialResolver, bobKeySourceResolver, explainModeDefaultResolver, and copilot/claude token state into a constructor-injected value, eliminating those Set* mutators and their temporal coupling. Slice-1 boundary: manager_tokens.go + manager_copilot_auth.go. Constraint: the mint path must keep the mintAgentTokenUnlocked contract (§3).

Phase C — nil-safe Hooks (safe). kickObserver, pauseObserver, sandboxAuditCallback, recordPromptCallback, and the inferenceRouteCallback/clearInferenceRouteCallback pair become a no-op-default struct. Note most observers are already atomic.Pointer escapes from m.mu; the Hooks struct must keep them lock-free reads, not fold them back under the mutex.

Frozen — kick delivery and the watchdog/liveness loop. No interface extraction for manager_kick.go, manager_poll.go, or the AgentProcess kick/watchdog bookkeeping until #5555 Phase 4 lands or explicitly supersedes this (see §4). This region is the mutex-reentrancy incident epicenter: the kick_async.go header documents the startup deadlock from re-entering m.mu on a holding goroutine, SendKickAsync's in-flight guard is the at-most-once-delivery property #5555 names T7, and the mint-under-lock fleet flap is codified in mintAgentTokenUnlocked. Behavior-preserving file moves are the ceiling here for now.

3. Lock-discipline map (the invariant any later phase must preserve)

From the real bodies (v5, post-split — the split moved no lock code):

• Take m.mu (write) — 48 methods: lifecycle (Start, Stop, AddAgent, UpdateConfig, ReconcileAgents, RemoveAgent), kick (SendKick, deliverStartupKick, runSandboxKick, nudgeIfKickStalled, nudgeIfTransientAPIError, SeedLastKick, SeedKickHistory), pause/breaker (PauseByCause, SeedPauseState, Resume, EngageBreaker, ReleaseBreaker, RestoreBreaker), restart (Restart, RestartWithBootstrap, KillSession, killSessionForRelaunch, ResetRestartCount, SeedRestartCount, SetBootstrapOverride, SetDeadSessionRecoveryOwner), overrides (PinCLI, UnpinCLI, PinModel, UnpinModel, SetModelOverride, SetBackendOverride, RefreshInferenceRoutes, ClearAllModeOverrides, SetACMMLevel), consent (clearConsentTracking, dismissConsentIfStuck), and the wiring setters (SetAppAuth, SetAgentMint, SetCopilotToken, ReloadClaudeToken, SetInferenceCallbacks, SetPersistPauseCallback, the five sandbox setters).
• Take m.mu (read) — 29 methods: status surface (GetStatus, AllStatuses, CountAgentsWithModel, GetOutput, GetBufferOutput, IsPaused, SessionMissing, TmuxSession, ResolveAgent, GetACMMLevel), authz (AuthorizePROpen, AuthorizeIssueOpen, AuthorizeMerge, AgentCapabilities, EffectiveBackend, InvocationMetadata, SyncModeFiles), tokens (CopilotToken, BackendAuthAvailable, syncCopilotToken, backendInUse, RefreshAgentTokenFor, refreshAgentTokens, runCopilotDiagnostic), liveness (pollTmuxOutputForAgent, CheckAndRestartCrashedAgents, bobAgentsAwaitingKey, CaptureFullLog, RestartThenSendKick).
• Must NEVER take m.mu (called with it held) — the *Locked family: deliverKickLocked, agentSandboxEnabledLocked, startSandboxKickLocked, sandboxWorkspaceDirLocked, tieredSandboxMinterLocked, agentMintIssuerLocked, removeAgentsBelowACMMGateLocked.
• Must run with m.mu RELEASED: mintAgentTokenUnlocked — holding the lock across the outbound mint caused the fleet-wide liveness flap; mint_deadlock_test.go pins it. This is [strategist] Re-entrant agent turn model (#4002) needs a phased delivery roadmap before implementation begins #5555's T10 in miniature: no blocking I/O under m.mu.
• Re-entrancy: no path may re-enter m.mu on a goroutine that holds it — the kick_async.go header records the startup deadlock this caused (the isGatewayBackend atomic.Pointer escape exists precisely for a callback invoked under the lock).
• Other mutexes: AgentProcess.paneMu (pane buffer: pollTmuxOutputForAgent, snapshot, PaneLines, FilteredPaneLines), thrashMu (checkBlockedThrash), plus the atomic.Pointer observer fields, which are deliberately outside m.mu and must stay there.

Any Phase A–C PR must state where its reads/writes sit relative to m.mu, following the mintAgentTokenUnlocked pattern.

4. Sequencing against the #5555 re-entrant-turn roadmap

The #5555 roadmap (comment of 2026-09-02) plans, in Phase 4(b), to replace the in-memory kick bookkeeping — lastInferKickAt, lastInferKickPane, stallNudgeSent, lastInferKickMarks, kickLogPending on AgentProcess — with a persisted per-agent kick envelope, gated on a formal model of the dispatch lifecycle (T7 against the SendKickAsync in-flight guard) and the T10 lock rule. To avoid collision:

• Slice 1 keeps Manager and AgentProcess whole, in core manager.go. When [strategist] Re-entrant agent turn model (#4002) needs a phased delivery roadmap before implementation begins #5555 Phase 4(b) swaps those fields for an envelope, it edits one file and manager_kick.go/manager_poll.go bodies — no decomposition phase will have restructured them first.
• Phases A–C here are orthogonal to [strategist] Re-entrant agent turn model (#4002) needs a phased delivery roadmap before implementation begins #5555 Phases 0–3 (which build inert pkg/turn machinery and wire headless agents) and can proceed on their own review cadence.
• The kick-delivery/watchdog freeze (§2) holds until [strategist] Re-entrant agent turn model (#4002) needs a phased delivery roadmap before implementation begins #5555 Phase 4 ships or is re-scoped. If [strategist] Re-entrant agent turn model (#4002) needs a phased delivery roadmap before implementation begins #5555's Phase 0 exit ramp closes [v5] RFC: re-entrant conversation-as-state agent turn model (durable, handoff-able agents) #4002 instead, the freeze lifts and kick delivery becomes ordinary Phase-D extraction ground — behind the same lock-discipline gates.
• Nothing in this plan touches pkg/turn, pkg/convergence/mutation, the governor's kick arbitration, or SendKickAsync semantics.

5. Slice 1 — shipped now

The mechanical split PR (verbatim moves, per-declaration SHA-256 proof 393/393 identical, four source-reading tests re-pointed by filename only, full go test ./pkg/agent/ parity with clean v5 including identical coverage): #5667

This issue stays open as the epic; Phases A–C are follow-up PRs sequenced above.

[clubanderson]

Dispatch against current origin/v5 tip d18d03952d6b42345109e01ce6d0a57126711c82.

Measured inventory: the original 10k-line pkg/agent/manager.go finding is substantially improved on v5. Today manager.go is 1,205 lines, with 20 funcs, 19 Manager methods, and 7 types. Manager-related code is already split into manager_launch.go (916 lines), manager_kick.go (873), manager_tokens.go (864), manager_homes.go (823), manager_pane_signals.go (818), manager_restart.go (769), manager_tmux.go (630), manager_poll.go (596), manager_env.go (526), manager_routing.go (463), manager_pause.go (389), manager_modes.go (320), manager_consent.go (301), and manager_sandbox.go (223).

Remaining Manager seams still worth extracting:

• UID / identity isolation: project context, per-agent identity, interactive home, token/cache homes.
• Lifecycle: Start, Stop, AddAgent, ReconcileAgents, RemoveAgent, status enumeration.
• Terminal/tmux: pane capture, send literal/keys, session attached, prompt-dismiss sleep, full-log/history functions.
• Hooks/resolvers: bob/Linear/explain/key-source resolvers, kick/pause/prompt/sandbox audit callbacks, inference-route callbacks.
• Concurrency: core agent map mutex plus specialized atomic pointers/mutexes introduced to avoid deadlocks.

Mechanical split plan:

1. Extract a TerminalSession/PaneIO interface for tmux/pane methods, preserving defaults and fakes for tests.
2. Extract credential/backend resolver state into a CredentialSource/BackendRoutes value passed at construction or configured atomically in one place.
3. Extract observer callbacks into a nil-safe Hooks struct with no-op defaults.
4. Extract lifecycle orchestration into a Lifecycle helper around the agent map, keeping UID isolation boundaries explicit.
5. Add tests that UID-specific homes/tokens survive the seams and that lifecycle operations never call terminal I/O while holding the wrong lock.

Phase-1 PR scope: terminal/pane interface extraction only, with no behavior change and focused tests around kick/log capture paths.