fix: pace retryable partial-start failures in code-mode composites - #4150
Conversation
PR #4062 exempted PartialStartError from the backoff gate so a composite toolset's healthy subset keeps listing while degraded, but this also cleared the gate on every retryable partial failure (e.g. a RAG toolset inside a code_mode composite hitting 429s), so the failed subset burst-retried on every turn just like #4060. startLocked now calls setStartBackoff instead of resetStartBackoff for a PartialStartError: it arms the gate when the aggregated cause is retryable (errors.As walks the whole errors.Join tree, so any single retryable inner cause is enough) and still resets it otherwise, preserving today's fail-fast behaviour for non-retryable partial failures. s.started stays latched either way, so the healthy subset is unaffected. Fixes #4067
aheritier
left a comment
There was a problem hiding this comment.
🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer
Review-of-record (self-review of my own PR; delegated an independent review to a reviewer sub-agent beforehand, which returned approve with no findings — see summary below). Self-approval is blocked for the PR author, so posting as COMMENTED with concrete evidence for maintainers to verify.
Core fix — pkg/tools/startable.go:585 (startLocked's PartialStartError branch): replaced s.resetStartBackoff() with s.setStartBackoff(err). setStartBackoff (pkg/tools/startable.go:463-481) internally calls startBackoffRetryable(err) (pkg/tools/startable_backoff.go:51-59), which does errors.Is/errors.As — both walk the whole errors.Join tree that PartialStartError.Unwrap() (pkg/tools/startable.go:142) exposes. So:
- A single retryable inner cause (429/503/etc via
*modelerrors.StatusError, orlifecycle.ErrCrashLooping) anywhere in the joined batch arms the gate — even mixed with a non-retryable/auth cause (ANY-cause semantics, deliberately different fromAuthOnly's ALL-cause semantics at pkg/tools/startable.go:113-122). - A batch with no retryable cause falls through to
resetStartBackoff()insidesetStartBackoff(pkg/tools/startable.go:467-470), preserving today's fail-fast behaviour for non-retryable/auth-deferral partial failures. s.started = true(pkg/tools/startable.go:582) is latched unconditionally before the gate call, so the healthy subset keeps listing regardless of gate state.
Tests:
pkg/tools/startable_backoff_test.go: renamed the old test toTestStartableToolSet_NonRetryablePartialStartClearsBackoffGate(narrowed doc to the non-retryable contract) and addedTestStartableToolSet_RetryablePartialStartArmsBackoffGate+TestStartableToolSet_MixedAuthPartialStartArmsBackoffGate(pins ANY-cause semantics: mixed auth+retryable arms the gate while stayingIsAuthorizationRequired() == false).pkg/tools/codemode/codemode_test.go:TestCodeModeTool_RateLimitedInnerPacesRetrydrives the realcodemode.Wrap()+tools.NewStartable()stack end-to-end — confirms the gated turn doesn't re-invoke either inner'sStart, the healthy tool declarations stay listed throughout, and the degraded inner recovers once the window elapses.
Validation: go build ./..., go test ./pkg/tools/... ./pkg/agent/... ./pkg/runtime/... (and full go test ./... — only the pre-existing, unrelated pkg/rag/treesitter CGO/gcc gap fails, reproduced identically on main), golangci-lint run --allow-parallel-runners (0 issues), go run ./lint . (no offenses), go mod tidy --diff (clean).
Docs: docs/tools/mcp/index.md and docs/features/code-mode/index.md updated to drop the "not yet applied in code mode" caveat, with verified cross-link anchors.
../mcp/index.md from docs/features/code-mode/ resolves to the nonexistent docs/features/mcp/index.md; the mcp docs actually live at docs/tools/mcp/index.md.
🤖 Automated implementer agent — this PR was posted by the implementer bot from Docker Agentic Platform, not by a human developer
Fixes #4067.
PR #4062 exempted
PartialStartErrorfrom the backoff gate so a composite toolset's healthy subset keeps listing while degraded, but this also cleared the gate on every retryable partial failure (e.g. a RAG toolset inside acode_modecomposite hitting 429s), so the failed subset burst-retried on every turn just like #4060.startLockednow callssetStartBackoffinstead ofresetStartBackofffor aPartialStartError: it arms the gate when the aggregated cause is retryable (errors.Aswalks the wholeerrors.Jointree, so any single retryable inner cause is enough) and still resets it otherwise, preserving today's fail-fast behaviour for non-retryable partial failures (e.g. auth deferrals).s.startedstays latched either way, so the healthy subset is unaffected.Testing:
pkg/tools/startable_backoff_test.go: retryable-cause arms the gate; mixed auth+retryable cause still arms the gate (ANY-cause semantics) while staying non-auth-classified.pkg/tools/codemode/codemode_test.gothrough the realcodemode.Wrap()+StartableToolSetstack: gated retry doesn't re-invoke either inner, healthy tools stay listed, degraded inner recovers once the window elapses.task build/go test ./.../golangci-lint run/go run ./lint .all pass (the only test failure,pkg/rag/treesitter, is a pre-existing sandbox limitation — missing gcc/CGO — reproduced identically onmain).Docs updated:
docs/tools/mcp/index.mdanddocs/features/code-mode/index.mdno longer claim this pacing is unapplied in code mode.