Refs #4060 (partial): RAG semantic-embeddings indexing triggered a
rate-limit retry storm — repeated toolset-start attempts had no pacing
after a 429 from the embedding provider (the 15s/5min gate is the partial
fix; DefaultStartTimeout and chunk-level checkpointing are deferred).
Implementation:
- modelerrors.RetryableHTTPStatus(err): HTTP-status classifier that
recognises 429, 408, and 5xx via *StatusError first, then falls back
to statusCodeRegex. The toolset gate pre-filters to *StatusError via
errors.As before calling it, so port numbers and chunk counts in
plain error strings cannot arm the gate.
- pkg/tools/startable_backoff.go: bounded exponential backoff with
additive 0-20% jitter (base=15s, cap=5min, delay∈[d,1.2d]).
- Gate in tryStartLocked (TryStart/TryStartWithTimeout only): blocking
Start() bypasses it so mcpcatalog enable and skill startup are
immediate.
- Gate adopts a live StartReporter after /toolset-restart without
waiting for the window to expire.
- Wrap embedding errors via oaistream.WrapOpenAIError at openai/client.go
and dmr/embed.go so a 429 from the embedding provider surfaces as
*StatusError and correctly arms the gate.
- WithStartRetryJitter / WithStartRetryClock options via variadic
NewStartable for deterministic test control.
- Stale 'retry on next turn' log messages updated in agent.go/mcp.go.
- Partial-start exemption documented (code-mode composites remain
unpaced; follow-up at issue #4067).
Tests (same commit, covering the above):
- startable_backoff_test.go: unit tests for the gate (gate fires on
429/408/5xx StatusError, not on plain text / context errors,
blocking Start() ungated, concurrency, jitter bounds)
- startable_backoff_regression_test.go: consumer-shaped regression
suite (RAG/MCP/LSP error shapes, no-goroutine/timer leak, latch)
- rag_backoff_test.go: real-toolset integration test via rag.New +
fake clock
Docs:
- docs/tools/rag/index.md: 'Indexing failures, retries and backoff'
section with trigger table, parameters, and troubleshooting.
- docs/tools/mcp/index.md, docs/tools/lsp/index.md: lifecycle notes
confirming local startup failures fail fast.
Scope: DefaultStartTimeout (30s) unchanged — deferred.
- Run golangci-lint fmt to apply gofumpt v0.11.0 formatting (v0.9.2→v0.11.0
bundled in golangci-lint) across 30 files: purely mechanical whitespace
and grouping changes, no logic changes.
- Remove 3 stale //nolint:recvcheck directives from pkg/config/latest/types.go
(recvcheck in golangci-lint v2.13.1 no longer flags MarshalYAML/JSON patterns,
making those suppression comments unused).
- Disable newly-activated modernize sub-checks in .golangci.yml:
errorsastype, embedlit, stringscut, reflecttypeassert — all require Go 1.27
APIs or x/tools ≥ v0.48; pre-existing violations in unrelated files.
A follow-up PR should re-enable these and clean up the violations.
- Add .golangci.yml exclusions for pkg/config/v*/ (recvcheck + nolintlint):
frozen versioned config types that cannot be modified; recvcheck changed
behaviour between v2.12.2 and v2.13.1 leaving stale nolint directives.
The stream-truncation FormatError message claimed the model produced no
output, which is inaccurate when the connection drops after partial tokens
have streamed. Reword to cover both the silent-prefill drop and a mid-stream
cut, and drop the prefill-only 'shorter prompt' framing as the sole cause.
Addresses review feedback on #3302.
A long local-model prefill produces no bytes until the first token. An idle or
connection timeout in the model server, or in a proxy in front of it (such as
Docker Model Runner's socket path), can drop the connection during that window.
The resulting "unexpected EOF" / "unexpected end of JSON input" was
classified non-retryable (the truncated-JSON variant even matched the
"invalid" non-retryable pattern), unlike the sibling "connection reset"
error which is already retryable.
Treat a mid-stream truncation as retryable so the retry can succeed once the
backend's prompt cache makes the prefill fast enough to beat the idle limit,
and surface an actionable message instead of a raw decoder error.
Refs #3298
Split the single ContextOverflowError bucket into three OverflowKinds so
the runtime and clients can react differently to each:
* tokens — conversation exceeds the model's context window.
Compaction can usually help.
* wire — request body exceeds the provider's wire-level limit
(e.g. HTTP 413). The latest turn alone is over the cap;
compaction-as-retry cannot help.
* media — image, PDF, or similar attachment too large.
Classification runs two tiers, in order:
1. Structured signals (high confidence): HTTP 413, body.error.type =
"request_too_large", body.error.code = "context_length_exceeded".
2. Provider-prose substring patterns as fallback, covering the
observed wording across Anthropic, OpenAI, Bedrock, Gemini,
Mistral, Groq, Vertex, OpenRouter, Ollama, Kimi, MiniMax, z.ai,
and others. Best-effort; provider wording is not contractual.
FormatError now returns three distinct, actionable, provider-agnostic
messages instead of one. The runtime emits new ErrorCodes
"request_too_large" and "media_too_large" alongside the existing
"context_exceeded" so external consumers can render the right hint.
Behaviour is preserved: IsContextOverflowError matches the same set of
errors as before, retry classification is unchanged, and auto-compaction
still fires on overflow. NewContextOverflowError now auto-fills Kind
from the underlying error so existing wrap sites get the correct shape
for free.
Address PR #2691 review: matchesTransientPattern lowercased the
message but used pattern strings as-is. The current pattern is
all-lowercase by accident, so a future maintainer adding a
mixed-case pattern would silently never match — re-introducing
the bug this PR is meant to fix.
- Document that transientStatusCodePatterns MUST be lowercase
- Defensively strings.ToLower(p) in the comparison so a
mixed-case entry would still work
- Add TestMatchesTransientPattern with exact, mixed-case, and
wrapped-error cases to lock in the contract
- Add 1MB size limit to JSON parsing to prevent memory exhaustion from
malicious or accidentally huge error responses
- Fix firstJSONObject to try each '{' position until valid JSON is found,
handling cases where '{' appears in URLs (e.g., param={value})
- Add comprehensive edge case tests for empty JSON, malformed JSON,
multiple JSON objects, unicode, large numbers, special characters in
Request-ID, braces in URLs, and nested JSON in message fields
Security: Prevents potential DoS via unbounded JSON parsing
Bug fix: Correctly extracts JSON body when '{' appears in URL parameters
Assisted-By: docker-agent
Same behaviour, less code, easier to read:
* StatusError.Error() 14 -> 9 lines (request-id concat moved into
parseProviderError, no second branch here)
* extractFirstJSONObject 35 -> 9 lines, renamed firstJSONObject:
replace the hand-rolled brace/string/escape
state machine with json.Decoder, which
handles escapes correctly by construction
* parseProviderErrorDetails split into two single-purpose helpers:
- parseProviderError finds the JSON body, parses it, appends
the request id to the rendered details
- formatProviderError renders the parsed struct as a one-liner
* unquoteJSONScalar renamed scalarString and switched to plain
any/float64 instead of json.RawMessage +
per-scalar json.Unmarshal calls
All existing tests still pass unchanged; lint clean.
Assisted-By: docker-agent
Parse the JSON body that providers attach to non-2xx responses and lift
the structured fields (error.type, error.message, error.code, error.param,
status, request id) into StatusError.Error(), stripping the URL noise.
Recognised shapes:
Anthropic {"type":"error","error":{"type":"...","message":"..."}}
OpenAI {"error":{"message":"...","type":"...","code":"...","param":"..."}}
Gemini {"error":{"code":N,"message":"...","status":"..."}}
Proxies {"message":"Bad Request"}
When the body is opaque (Docker AI gateway returning only
{"message":"Bad Request"}) we still strip the URL so the user gets
"HTTP 400: Bad Request" instead of a 200-character POST/URL/JSON wall.
Falls back to the original err.Error() text whenever no JSON object is
found or the body has no recognised fields, so non-SDK errors are
unchanged (e.g. "HTTP 429: rate limit exceeded").
Assisted-By: docker-agent
No callers reference modelerrors.MaxRetryAfterWait — all usage goes
through backoff.MaxRetryAfterWait directly. Remove the constant and
the now-unused backoff import.
Assisted-By: docker-agent
Both are only used within the package. Rename to isRetryableStatusCode
and extractHTTPStatusCode to reduce the public API surface.
Assisted-By: docker-agent
Only called within the package (by WrapHTTPError and tests).
Rename to parseRetryAfterHeader to reduce the public API surface.
Assisted-By: docker-agent
CalculateBackoff and SleepWithContext were deprecated in favor of
backoff.Calculate and backoff.SleepWithContext. No callers use the
modelerrors versions — all code imports pkg/backoff directly.
Remove the deprecated wrappers to reduce API surface.
Assisted-By: docker-agent
Add tests for:
- ClassifyModelError with ContextOverflowError wrapping a StatusError
- ExtractHTTPStatusCode with *StatusError in the error chain
- FormatError with a wrapped ContextOverflowError (via fmt.Errorf)
- NewContextOverflowError constructor usage
- ContextOverflowError with nil Underlying (Unwrap returns nil)
Assisted-By: docker-agent
Callers were constructing &ContextOverflowError{Underlying: err}
directly. Adding a constructor improves encapsulation and allows
future field additions without breaking callers. Update
runtime/fallback.go to use the constructor.
Assisted-By: docker-agent
StatusError.Error() previously delegated directly to the wrapped error,
discarding the status code. This made debugging harder since log lines
wouldn't reveal the HTTP status. Now Error() returns a message like
"HTTP 429: rate limit exceeded" for better observability.
Assisted-By: docker-agent
SleepWithContext and CalculateBackoff are general-purpose utilities not
specific to model errors. Move them to a new pkg/backoff package for
clarity and reusability. MaxRetryAfterWait moves there too.
Deprecated wrapper variables are left in modelerrors to maintain
backward compatibility. The runtime/fallback package now imports
pkg/backoff directly.
Assisted-By: docker-agent
The retryablePatterns and nonRetryablePatterns slices were local
variables inside isRetryableModelError, causing a new allocation on
every call. Promote them to package-level vars (like
contextOverflowPatterns) so they are allocated once.
Assisted-By: docker-agent
The regex match is a simple integer string. strconv.Atoi is simpler,
faster, and already imported. This also removes the fmt import.
Assisted-By: docker-agent
Now that all providers (Anthropic, OpenAI, Gemini) wrap their errors in
*StatusError via WrapHTTPError, the provider-specific type assertions
(anthropic.Error, genai.APIError) in ExtractHTTPStatusCode are redundant.
ClassifyModelError already checks *StatusError first.
Replace the SDK-specific checks with a *StatusError check, keeping the
regex fallback for unwrapped errors. This removes the coupling between
the error classification package and provider-specific SDKs.
Assisted-By: docker-agent
The only caller outside the package is ClassifyModelError itself
(as a fallback path). ClassifyModelError is the intended public API
and provides strictly more information (retryable + rateLimited +
retryAfter). Rename to isRetryableModelError to reduce API surface.
Assisted-By: docker-agent
ExtractHTTPStatusCode + IsRetryableStatusCode already handle numeric
status codes (500, 502, 429, 401, etc.) via regex with word boundaries.
Having the same codes as bare substrings in the retryable/non-retryable
pattern lists caused false positives (e.g., "processed 500 items"
would match). Remove the numeric patterns and keep only the descriptive
text patterns.
Assisted-By: docker-agent
Addresses review feedback from rumpl and simonferquel on PR #2096.
The previous approach extracted Retry-After metadata in a dispatcher function
that type-switched on provider SDK error types (`*anthropic.Error`,
`*openai.Error`). This kept all providers coupled to the modelerrors package
and duplicated parsing logic.
New design:
1. `modelerrors.StatusError` — a thin typed wrapper carrying:
- StatusCode int
- RetryAfter time.Duration (parsed from Retry-After header)
- Err error (original SDK error, accessible via Unwrap)
2. `modelerrors.WrapHTTPError(statusCode, resp, err)` — wraps any provider
error in StatusError; parses Retry-After from resp.Header if non-nil.
3. `modelerrors.ParseRetryAfterHeader(value)` — shared RFC 7231 §7.1.3
parser (integer seconds + HTTP-date). No SDK imports.
4. `modelerrors.ClassifyModelError(err)` — single-arg; uses errors.As to
find *StatusError in the chain; falls back to message/pattern matching for
providers not yet wrapped (e.g. Bedrock).
5. Three new wrap.go files (one per provider family), each a single function:
- pkg/model/provider/anthropic/wrap.go — wrapAnthropicError(*anthropic.Error)
- pkg/model/provider/oaistream/wrap.go — WrapOpenAIError(*openai.Error) (exported)
- pkg/model/provider/gemini/wrap.go — wrapGeminiError(*genai.APIError)
6. Wrap sites — one-line change at Recv() in each adapter:
- anthropic/adapter.go, beta_adapter.go
- oaistream/adapter.go
- openai/response_stream.go (reuses oaistream.WrapOpenAIError)
- gemini/adapter.go
No changes to fallback.go logic — ClassifyModelError signature stays
`(err error)` and retry/rate-limit decisions are unchanged.
Assisted-By: docker-agent
When a model returns HTTP 429 (Too Many Requests):
- If fallback models are configured: skip to next fallback immediately (existing behavior preserved)
- If no fallbacks are configured: retry the same model using the Retry-After header duration (capped at 60s), falling back to exponential backoff if the header is absent
HTTP 500 was already handled as retryable via IsRetryableStatusCode — no change needed.
New additions to pkg/modelerrors:
- ExtractRetryAfter(err): extracts Retry-After duration from *anthropic.Error and *openai.Error responses
- parseRetryAfterHeader(value): parses integer-seconds or HTTP-date Retry-After values
- ClassifyModelError(err): unified classifier returning (retryable, rateLimited bool, retryAfter time.Duration)
- MaxRetryAfterWait constant (60s cap)
pkg/runtime/fallback.go changes:
- Replace both IsRetryableModelError call sites with ClassifyModelError
- Add handleModelError helper to avoid duplicating classification logic
- 429 with fallbacks → break to next model (unchanged behavior)
- 429 without fallbacks → sleep(min(retryAfter, 60s) or backoff), then continue
Fixes#2095
Assisted-By: docker-agent
Add modernize and perfsprint to .golangci.yml and resolve all lint
issues across 97 files:
- Replace fmt.Errorf with errors.New where no format verbs are used
- Replace fmt.Sprintf(%d) with strconv.Itoa for int conversions
- Replace redundant fmt.Sprintf(%s) with direct string values
- Use slices.Contains instead of manual loops (SkillsConfig.HasLocal)
- Use omitzero instead of omitempty on struct fields (time.Time, SkillsConfig)
- Inline boolPtr helpers to new(bool) and remove unused functions
- Replace string concatenation in loops with strings.Builder
- Use fmt.Fprintf instead of WriteString(fmt.Sprintf(...))
Assisted-By: docker-agent
Break up the oversized runtime.go (2510 lines) by extracting cohesive
groups of functions into dedicated files, and move pure error
classification utilities into a new reusable package.
New files in pkg/runtime:
- model_picker.go: model change/revert tool handlers
- agent_delegation.go: sub-agent dispatch, task transfer, handoffs
New package pkg/modelerrors:
- ContextOverflowError type and detection
- HTTP status code extraction (Anthropic, Gemini, OpenAI)
- Retryable error classification
- Exponential backoff with jitter
- User-friendly error formatting
Assisted-By: docker-agent
David Gageot
Arnaud Héritier
Trung Nguyen
Sayt-0
Go Expert Coding agent