Commit Graph
20 Commits
Author SHA1 Message Date
Arnaud Héritier 445fd38023 fix: add bounded jittered backoff to StartableToolSet retry path
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.
2026-09-01 13:20:38 +02:00
Arnaud Héritier 1ec5fecd5d fix: resolve golangci-lint v2.13.1 / gofumpt v0.11.0 findings after linter bump
- 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.
2026-08-23 09:35:55 +00:00
Trung Nguyen fa9d191ad1 fix(modelerrors): make the context overflow message frontend-agnostic 2026-08-06 09:46:42 +02:00
Sayt-0 2062e03853 fix(modelerrors): cover mid-stream cut in truncation message
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.
2026-06-29 15:57:22 +02:00
Sayt-0 8643591c9a fix(modelerrors): retry stream truncation and clarify the error
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
2026-06-29 15:48:00 +02:00
Trung Nguyen a6c7044988 feat(modelerrors): classify overflow errors by kind
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.
2026-05-19 11:04:22 +02:00
David Gageot 1c6326d6fe fix(modelerrors): defensively lowercase transient patterns
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
2026-05-07 12:58:40 +02:00
David Gageot 9a1f3c61a2 fix: retry vertex 'function response parts' 400 transparently 2026-05-07 12:29:31 +02:00
David Gageot c0e17a9335 fix(modelerrors): add JSON bomb protection and handle braces in URLs
- 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
2026-04-27 18:13:12 +02:00
David Gageot a3d0cd9728 feat(modelerrors): surface structured provider error details
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
2026-04-27 18:02:33 +02:00
David Gageot dcfb3c76d5 modelerrors: unexport IsRetryableStatusCode and ExtractHTTPStatusCode
Both are only used within the package. Rename to isRetryableStatusCode
and extractHTTPStatusCode to reduce the public API surface.

Assisted-By: docker-agent
2026-03-19 19:47:36 +01:00
David Gageot f699806729 modelerrors: unexport ParseRetryAfterHeader
Only called within the package (by WrapHTTPError and tests).
Rename to parseRetryAfterHeader to reduce the public API surface.

Assisted-By: docker-agent
2026-03-19 19:46:59 +01:00
David Gageot 4a94178f9d modelerrors: add missing test coverage
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
2026-03-19 19:35:24 +01:00
David Gageot 744db2501d modelerrors: include HTTP status code in StatusError.Error()
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
2026-03-19 19:33:01 +01:00
David Gageot 4fd44ed62d modelerrors: move backoff and sleep utilities to pkg/backoff
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
2026-03-19 19:31:49 +01:00
David Gageot f18f5b5546 modelerrors: unexport IsRetryableModelError
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
2026-03-19 19:25:00 +01:00
Go Expert Coding agent 3d44b7cd40 refactor: use typed StatusError for retry metadata, providers wrap errors at Recv()
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
2026-03-13 11:07:50 +00:00
Go Expert Coding agent 7c22ff16f9 fix: make HTTP 429 retryable when no fallback model, respect Retry-After header
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
2026-03-13 10:17:35 +00:00
David Gageot 943d3b0606 This can be retried
Signed-off-by: David Gageot <david.gageot@docker.com>
2026-03-12 17:17:54 +01:00
David Gageot 6068592017 refactor: split runtime.go and extract pkg/modelerrors
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
2026-03-09 17:32:50 +01:00