fix(#4060): extend backoff gate to remote MCP HTTP errors

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Stacked on PR #4062. Base retargets to main once that merges.

Refs #4060 (partial — A2A pacing and LSP crash-loop pacing deferred; see below)

What

Remote MCP servers responding with 503/429/5xx during the initialize handshake previously triggered a fresh connect attempt on every agent turn (the #4060 burst pattern). This PR fixes that by wrapping the HTTP status from the remote server in a *modelerrors.StatusError so the StartableToolSet backoff gate arming logic can pace retries — including a server-supplied Retry-After hint, and correctly for responses with no body.

Design

Wrap point — enrichConnectError in pkg/tools/mcp/remote.go. The oauthTransport already records the last HTTP error status via logErrorResponse (for any >= 400 response). enrichConnectError wraps that status (regardless of whether the response carried a body — many rate-limit/load-balancer responses don't) via modelerrors.WrapHTTPError, surfacing it as a *StatusError in the chain.

Retry-After is honored. pkg/tools/mcp/oauth.go's oauthTransport now also captures the raw Retry-After header value alongside the status/body it already tracked. lastServerErrorSnapshot() reads status, message, and Retry-After together under a single lock (not three separate accessor calls) so a caller can never pair a status from one response with a Retry-After header captured from a different concurrent response on the same transport — this transport's RoundTrip can run concurrently for one logical connect attempt (e.g. a standalone SSE probe racing the initialize call). enrichConnectError builds a minimal *http.Response carrying that header and passes it to WrapHTTPError, matching the handling already in place for model-provider adapters (PR #4062).

What arms the gate. startBackoffRetryable checks for a *modelerrors.StatusError with a retryable HTTP status (429/408/5xx) via errors.As — exactly as it already does for RAG embedding failures. No regex heuristics; no new classification logic in the gate itself.

What does NOT arm (unchanged policy):

• Local stdio MCP spawn failures (missing binary, connection refused) — never reach enrichConnectError.
• 4xx client errors (400/401/403) — wrapped in *StatusError for structured access but RetryableHTTPStatus returns false → fail promptly.
• Auth paths (oauthDeclined, authorizationRequired) — handled by their own early-return paths before the status branch; unaffected.
• lifecycle.ErrServerUnavailable, ErrTransport, ErrAuthRequired, ErrInitTimeout, ErrSessionMissing — the gate classifier explicitly excludes all of these.

Deferred:

• A2A pacing: the agent-card resolver does not cleanly expose HTTP status in its error chain; deferred to a follow-up.
• LSP crash-loop pacing: lifecycle.ErrServerCrashed is produced only inside lspSession.Wait() which flows to the supervisor's internal watcher, not to supervisor.Start(). The gate never sees it via the current error propagation path; deferred.

Changed files

File	What changed
pkg/tools/mcp/remote.go	enrichConnectError: wrap on status alone (not gated on a non-empty body); forwards Retry-After via a minimal synthetic *http.Response
pkg/tools/mcp/oauth.go	oauthTransport captures the raw Retry-After header; new lastServerErrorSnapshot() reads status/message/Retry-After together under one lock
pkg/tools/startable_backoff.go	Classifier doc comment: complete excluded-sentinel list (adds ErrInitTimeout, ErrSessionMissing), notes the deferred LSP crash-loop path
pkg/tools/mcp/remote_test.go	8 tests: 503/429/500 → retryable *StatusError; 403 → non-retryable; empty-body 503/429 still arm; Retry-After present/absent; end-to-end TestBackoffGate_* driving a real NewRemoteToolset through tools.StartableToolSet.TryStart
pkg/tools/startable_backoff_test.go	4 tests: lifecycle sentinels (ErrServerUnavailable, ErrTransport, ErrAuthRequired) do NOT arm the gate; 4xx StatusError does NOT arm the gate
docs/tools/mcp/index.md	Startup-failure note: remote MCP 5xx/429 now paced
docs/tools/lsp/index.md	Startup-failure note: crash-loop pacing not yet in place; supervisor policy is the throttle

[aheritier]

Reviewed at head 01f4bf5c (base main, MERGEABLE/CLEAN). CI for this SHA is fully green: build-and-test, lint, windows-tests, CodeQL/Analyze, build-image ×2, docs checks — 19 check-runs, 0 failed, 0 pending.

Re-validated locally from a git archive of the head SHA: go build clean; go test -race -count=1 ./pkg/tools/ ./pkg/tools/mcp/ ./pkg/modelerrors/ ./pkg/tools/codemode/ green; golangci-lint run 0 issues.

Tests are load-bearing (mutation-checked): re-gating the wrap on msg != "" fails TestEnrichConnectError_EmptyBodyStatusStillArms + TestBackoffGate_RemoteMCPRecoversAfterBackoffWindow; dropping the Retry-After forward fails TestEnrichConnectError_RetryAfterHonoured; dropping the *StatusError guard in startBackoffRetryable is caught by the pre-existing TestStartableToolSet_PlainTextStatusShapeDoesNotArmGate.

Classification is structural, not textual: startBackoffRetryable (pkg/tools/startable_backoff.go:40-46) hands the extracted *StatusError itself to RetryableHTTPStatus, so extractHTTPStatusCode always takes the errors.As branch and the \b[45]\d{2}\b regex fallback is unreachable from the gate.

Stdio path unchanged: enrichConnectError has a single caller (remote.go:181, remote client only).

Findings

[should-fix] Code-mode caveat missing from docs/tools/mcp/index.md:261. The paragraph says remote MCP retryable statuses "are paced by the same bounded exponential backoff gate" without qualification. I probed a codemode.Wrap composite (one Startable inner returning a 503 *StatusError + one non-Startable builtin) through tools.NewStartable(...).TryStart: the failing inner's Start ran 3/3 times — not paced — because pkg/tools/startable.go:576-586 calls resetStartBackoff() on PartialStartError. That's pre-existing behaviour from #4062 and already tracked in #4067, so it's not a regression here, but code-mode users reading this doc will expect pacing they won't get. Suggest a one-liner: "Not yet applied when toolsets are wrapped in code mode — see #4067."

For the record, the total-failure code-mode case (every inner Startable and failing, one with 503) does arm the gate for the whole composite, including a plain-error sibling — new since the 503 now carries a *StatusError, but reasonable given nothing is up.

[optional] pkg/tools/mcp/oauth.go:912-921 — the single-lock snapshot rationale (SSE probe racing initialize) isn't pinned by a test: moving the lastErrRetryAfter read outside t.mu still passes the suite under -race. A small concurrent logErrorResponse / lastServerErrorSnapshot test would guard the claim.

[optional] PR description's "Changed files" table omits docs/tools/rag/index.md (+11/-4, added in round 2).

#4060 checklist

Remote MCP 429/408/5xx-set → paced (remote.go:224-239, e2e TestBackoffGate_*); 4xx + lifecycle sentinels → prompt fail (startable_backoff_test.go:763-816); empty-body statuses → still arm; Retry-After → honoured; LSP crash-loop → deferred to #4099; A2A → deferred to #4098 / stacked #4104 (overlaps this PR only on docs/tools/rag/index.md). All referenced issues are open.

No blocking findings. Filing as a comment because GitHub blocks self-approval by the author.