Skip to content

feat(modelerrors): surface structured provider error details on non-2xx responses#2549

Merged
dgageot merged 3 commits intodocker:mainfrom
dgageot:board/improving-error-messages-for-http-400-fa-1f08df4a
Apr 28, 2026
Merged

feat(modelerrors): surface structured provider error details on non-2xx responses#2549
dgageot merged 3 commits intodocker:mainfrom
dgageot:board/improving-error-messages-for-http-400-fa-1f08df4a

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Apr 27, 2026

What

Adds the four lifecycle hook events that Claude Code, OpenCode, and pi all expose but docker-agent did not, plus a small documentation fix.

Why

While auditing how docker-agent's hook system compares to peer AI coding agents, four events stood out as universally supported by competitors but missing from us. Each one unlocks a real use case that today either has to be hacked in via pre_tool_use/on_user_input or simply isn't possible.

New hook events

Event When it fires Can shape behaviour?
user_prompt_submit Once per real user message in RunStream, after session_start and before the first model call. Skipped for sub-sessions (whose kick-off message is synthesised by the runtime). Block submission, or contribute transient additional_context for that turn.
pre_compact Inside summarizeWithSource before context compaction. Trigger source (manual / auto / overflow / tool_overflow) is reported in Input.Source. Cancel compaction, or append guidance to the compaction prompt.
subagent_stop After runSubSessionForwarding and runSubSessionCollecting complete — success or failure (deferred dispatch). Observational.
permission_request Inside askUserForConfirmation just before the runtime would prompt the user. Auto-allow (skip the prompt) or auto-deny (permission_decision: allow / deny), mirroring pre_tool_use. Returning nothing falls through to the interactive confirmation.

Also fixed

  • post_tool_use doc / schema / example wording: it fires on both success and failure, with tool_response.is_error distinguishing them. The previous "after a tool completes successfully" claim was wrong.
  • EventPermissionRequest's doc comment now spells out the asymmetry with pre_tool_use (where allow is the implicit default vs. permission_request where it's an explicit auto-approve verdict). That's why Result.PermissionAllowed exists separately from Result.Allowed.

Files touched

  • pkg/config/latest/types.go — 4 new fields on HooksConfig + validation
  • pkg/hooks/{types,executor,config}.go — 4 new EventType constants, Result.PermissionAllowed, Input.{Prompt, AgentName, ParentSessionID}, executor wiring
  • pkg/runtime/{hooks,loop,runtime,agent_delegation,skill_runner,tool_dispatch}.go — dispatch helpers and call-site integration
  • agent-schema.json, examples/hooks.yaml, docs/configuration/hooks/index.md — schema, example yaml demonstrating all events, and user-facing docs

Testing

  • 7 contract tests in pkg/hooks/contract_widening_test.go pin the wire format for every new event (block-produces-deny, allow-produces-permission-allowed, fields-reach-the-hook, …).
  • 2 runtime tests in pkg/runtime/user_prompt_submit_test.go pin the gating contract: fires-once for top-level submissions, never for sub-sessions (SendUserMessage=false).
  • The example examples/hooks.yaml parses through config.Load and validates against agent-schema.json.
  • mise run lint → 0 issues. go test -count=1 ./... → 0 failures across ~150 packages.

Commits

  1. feat(hooks): add 4 new hook events to match Claude Code / OpenCode / pi — the feature
  2. refactor(hooks): simplify the call sites added for the new events — small in-place readability cleanup (merge duplicated guards, switch on PermissionDecision, stop double-decoding tool args, take *agent.Agent directly instead of name + lookup)
  3. fix(hooks): address review findings on the new hook eventssubagent_stop now fires on the error path of both sub-session helpers (defer); EventPermissionRequest doc clarification; examples/hooks.yaml jq-dependency note; user_prompt_submit gating regression test

Backward compatibility

  • The four new fields on HooksConfig are all omitempty; existing configs continue to parse unchanged.
  • Summarize's public signature is unchanged; internal call-sites use a private summarizeWithSource to attribute the trigger to pre_compact hooks.
  • runSubSessionForwarding's parameter list changed (string → *agent.Agent) but the function is package-private; both call-sites updated.

dgageot added 3 commits April 27, 2026 18:02
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
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
- 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
@dgageot dgageot requested a review from a team as a code owner April 27, 2026 16:27
@dgageot dgageot merged commit 9e333a6 into docker:main Apr 28, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants