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
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
Trung Nguyen
David Gageot
Go Expert Coding agent