fix(codemode): preserve tools after partial startup

[Sayt-0]

Summary

• Keep run_tools_with_javascript available when one wrapped toolset fails to start.
• Preserve successfully started toolsets, omit failed toolsets, and retry failures on later turns.
• Recover previously healthy MCP toolsets through Restart, preserve OAuth warning semantics, and list partially started composites during startup progress.
• Avoid holding Code Mode state locks across inner lifecycle I/O.

Issue expectations

Expectation	Implementation
Initialize Code Mode when one MCP toolset fails	Partial startup keeps the Code Mode wrapper available
Include successfully loaded toolsets in the JavaScript environment	Healthy toolsets remain declared and callable
Keep failed MCP tools unavailable	Failed or disconnected inner toolsets are omitted until recovery
Preserve direct todo tools	Todo-category tools remain top-level
Surface failures without warning spam	Existing once-per-streak warning semantics are preserved
Retry unavailable toolsets	Initial failures retry through Start; disconnected MCP toolsets recover through Restart
Avoid blocking healthy tools	Inner lifecycle I/O runs without holding the Code Mode state mutex

Validation

Check	Result
task build	passed
task lint	passed
go test -race -count=1 ./pkg/tools ./pkg/tools/codemode ./pkg/agent ./pkg/runtime ./pkg/teamloader	passed
Hermetic integration with a real failing MCP subprocess, real fetch tool, todo toolset, and loopback HTTP server	9/9 assertions passed
MCP retry across two turns	subprocess invocation count increased from 1 to 2
Warning deduplication	one warning on the first turn, none on the second

The full local task test run was also attempted. Environment-specific SSRF assertions in pkg/config, pkg/tools/builtin/api, fetch, and openapi failed because private-address requests were intercepted or returned HTTP 405. The changed packages pass their focused tests.

Scope notes

Item	Decision
Listing failure from an inner toolset that still reports itself started	Unchanged because this issue concerns startup failures
Aggregate /tools degraded status	Warning reporting is preserved; exposing a dedicated degraded status remains separate work

Fixes #3978

[aheritier]

Reviewed at head d4266f91. CI is green (build-and-test, windows-tests, lint, license-check, CodeQL, build-image linux/amd64+arm64); locally go build ./..., task lint (0 issues) and go test -race -count=1 ./pkg/tools/... ./pkg/agent/... ./pkg/runtime/... ./pkg/teamloader/... all pass on this branch.

The design is the right shape: PartialStartError + StartReporter + the per-inner innerIdle/innerStarted/innerFailed/innerLost state machine, availableToolsets() used by both Tools() and runJavascript() so declarations and injected functions cannot drift, recovery dispatching Restart instead of a blind Start, and a documented lock order (lifecycleMu → mu, never an inner call under mu). Test coverage of the new paths is genuinely thorough.

Two things need a change before merge.

[blocking] A first-time OAuth deferral inside code mode now emits a spurious "needs re-authentication" notice

pkg/tools/startable.go:239-241 marks the recovery streak whenever a recovering plain Start fails. Combined with the new partial latch (startable.go:246-248), a composite is started=true after its first partial failure, so every later Start is classified as a recovery — including retries of an inner that has never been up. An inner MCP toolset that simply is not authorized yet returns AuthorizationRequiredError on every attempt, so turn 2 fires the notice that is meant for a background token loss.

Repro (drop into pkg/agent, go test -run Probe -v ./pkg/agent):

type probeAuthToolSet struct{ desc string }

func (p *probeAuthToolSet) Describe() string { return p.desc }
func (p *probeAuthToolSet) Start(context.Context) error {
	return &tools.AuthorizationRequiredError{URL: "https://example.test/mcp"}
}
func (p *probeAuthToolSet) Stop(context.Context) error                  { return nil }
func (p *probeAuthToolSet) Tools(context.Context) ([]tools.Tool, error) { return nil, nil }

func TestProbeAuthDeferral(t *testing.T) {
	a := New("root", "test", WithToolSets(codemode.Wrap(&probeAuthToolSet{desc: "mcp(ref=needs-oauth)"})))
	for turn := 1; turn <= 3; turn++ {
		_, err := a.Tools(t.Context())
		require.NoError(t, err)
		t.Logf("turn %d warnings: %#v", turn, a.DrainWarnings())
	}
}

This branch:

turn 1 warnings: []string(nil)
turn 2 warnings: []string{"*codemode.codeModeTool needs re-authentication — it will prompt on your next message, or use /toolset-restart"}
turn 3 warnings: []string(nil)

main — and the same toolset without the codemode wrapper on this branch — stays silent on all three turns.

It hurts most in a non-interactive session (sess.NonInteractive → tools.WithoutInteractivePrompts, pkg/runtime/loop.go:283-289), where every turn re-defers: the user is told a dialog will appear on their next message in a session that can never prompt — precisely the case the deferral comment in pkg/runtime/runtime.go:1816-1830 says to keep silent.

codeModeTool already knows the difference (innerFailed = never up, innerLost = died after a good start). Suggest propagating it — e.g. a Recovering/LostAfterStart field on PartialStartError, set when at least one cause came from an innerLost transition — and calling s.recoveryStreak.fail() only in that case. A test pinning "an initial auth deferral inside code mode stays silent across turns", next to TestAgentToolsCodeModeInnerDiesAfterStart, would lock it in.

[should-fix] A total start failure is reported as partial, advertising an empty run_tools_with_javascript

codeModeTool.Start (pkg/tools/codemode/codemode.go:232-234) returns NewPartialStartError whenever len(errs) > 0, including when no inner came up. The wrapper then latches started and the model is offered a tool with an empty function list:

// both inners fail with "connection refused"
a := New("root", "test", WithToolSets(codemode.Wrap(
	&probeDeadToolSet{desc: "mcp(ref=a)"}, &probeDeadToolSet{desc: "mcp(ref=b)"})))
got, _ := a.Tools(t.Context())

exposed tool: "run_tools_with_javascript"; description tail: "…Available tools/functions:\n\n"
tool count: 1

main drops the toolset entirely here. Returning the joined error un-wrapped when no inner toolset is available (instead of a PartialStartError) keeps that behaviour, keeps the cold-start retry path, and avoids spending prompt tokens on a code-mode tool that can call nothing.

[optional]

• pkg/tools/startable.go:95-106: the authOnly classification re-assigns authOnly = true on each non-nil auth cause; an explicit "at least one cause, and all of them are auth" formulation reads more directly.
• Degraded-composite warnings surface as *codemode.codeModeTool … (the DescribeToolSet type-name fallback, pkg/tools/startable.go:26-37). Pre-existing, but this PR makes composite warnings routine, so a Describe() string on codeModeTool would make these user-facing strings readable.

[aheritier]

Re-reviewed at head 442c4d7b. CI is green (build-and-test, windows-tests, lint, license-check, CodeQL, build-image linux/amd64+arm64); locally on this commit go build ./..., golangci-lint run (0 issues) and go test -race -count=1 ./pkg/tools/... ./pkg/agent/... ./pkg/runtime/... ./pkg/teamloader/... all pass.

Both findings from my review of d4266f91 are resolved, and I verified each with the original repro.

[blocking] initial OAuth deferral inside code mode — fixed. PartialStartError.LostAfterStart (pkg/tools/startable.go:93-99), set from the innerLost transition in codeModeTool.Start (pkg/tools/codemode/codemode.go:236-256) and gating s.recoveryStreak.fail() (pkg/tools/startable.go:296-311), separates a retried initial failure from a real post-start loss. My original repro is now silent on all three turns, sole-inner and with a healthy peer:

SINGLE turn 1/2/3 warnings: []string(nil)
MIXED  turn 1/2/3 tools=1 warnings: []string(nil)

I also checked the inverse, since that was the risk of the fix: a genuine background token loss (Restart returning AuthorizationRequiredError after a good start) still produces the targeted notice exactly once per streak, in both the partial and total variants:

turn 1: []           turn 2: ["… needs re-authentication — it will prompt on your next message, or use /toolset-restart"]           turn 3: []

TestAgentToolsCodeModeInitialAuthDeferralStaysSilent and TestStartableToolSet_PartialStartInitialFailureNeverMarksRecoveryStreak / …LostAfterStartMarksRecoveryStreak pin both directions.

[should-fix] total start failure — fixed. tools.TotalStartError (pkg/tools/startable.go:151-190) is deliberately not a PartialStartError, so the wrapper never latches and the next turn is a cold retry, while NewTotalStartError keeps the all-causes auth classification that a bare errors.Join would lose. Two dead inners now expose nothing and warn once with the real causes:

TOTAL tool count: 0
TOTAL warnings: ["*codemode.codeModeTool start failed: mcp(ref=a): connection refused\nmcp(ref=b): connection refused"]

The switch refactor in pkg/runtime/runtime.go:1815-1853 preserves the auth-before-ShouldReportFailure ordering that the comment above it requires, and only lets partial starts fall through to listing.

Test coverage of the new paths remains genuinely thorough — agent-level turn-by-turn arcs, runtime sidebar counting, error-classification tables, and the wedged-inner test that pins the "never hold mu across inner lifecycle I/O" contract.

Nothing blocking left. My two earlier [optional] notes still stand and are your call: the authOnly accumulation in allCausesAuthorizationRequired (pkg/tools/startable.go:109-124) reads indirectly, and codeModeTool still has no Describe(), so degraded-composite warnings surface as *codemode.codeModeTool needs re-authentication … — visible in the outputs above and now a routine message rather than a rare one.

LGTM.

[aheritier]

👋 This PR has merge conflicts with the base branch. Please rebase or merge the latest base branch and resolve them. I've moved it to draft and added status/needs-rebase; it'll be picked back up automatically once the conflicts are cleared.

[aheritier]

👋 This PR has merge conflicts with the base branch. Please rebase or merge the latest base branch and resolve them. I've moved it to draft and added status/needs-rebase; it'll be picked back up automatically once the conflicts are cleared.