fix: synthesize tool results for orphaned tool calls at ingest and request build#56
Conversation
…ries Incident ses_01kx48z4rqfkpbwmzfdv1jzeg6: a goal worker turn died with the Anthropic API 400 "tool_use ids were found without tool_result blocks immediately after", and every subsequent goal-loop retry then failed identically, killing the goal. An assistant message carrying a ToolCall part had entered session history with no tool-role result ever following it -- the provider stream died (or the turn otherwise errored) between emitting the tool_call and the engine executing it -- so every later request replayed the same orphaned tool_use and was rejected the same way by the API. This is the wire-protocol sibling of the marshal-level poisoning fixed in "fix(message,engine): truncated ToolCall.Arguments must never poison history": that fix keeps a poisoned ToolCall marshalable; this one must keep a poisoned history transcodable. ResolveOrphanToolCalls(messages) is the shared primitive both transcoders (anthropic, openaicompat -- wired in a following commit) will call at request-build time: for every ToolCall without a matching ToolResult in the immediately-following message, it merges a synthetic, is_error ToolResult into that message (partial orphan) or inserts a new RoleTool message carrying only the synthetic results (no following tool message at all, including the orphan being the final message in history). Synthetic results are never silent: Content always says "synthesized" via the new message.SyntheticOrphanResultText constant, so they are visibly distinguishable from a result a tool actually produced. Input is never mutated; a history with no orphans returns the identical input slice. TestResolveOrphanToolCallsNoOrphans/MidHistory/FinalMessage/PartialMerge cover: no-op on well-formed history, an orphan buried mid-transcript, an orphan as the very last message (both single- and multi-call turns), and a turn where only some of several tool calls got a result before the turn died. Verified red first: reverting only the message.go change (the tests already existed) fails the package build with "undefined: ResolveOrphanToolCalls"; re-applying it is green.
…uild Defense-in-depth for incident ses_01kx48z4rqfkpbwmzfdv1jzeg6: transcodeRequest now calls message.ResolveOrphanToolCalls(req.Messages) before transcoding, so any ToolCall lacking a matching ToolResult in the immediately-following message — however it got into history: a plugin's chat.message hook, a hand-rolled provider adapter, a replayed log from an older, unpatched binary, or simply a producer other than engine.Session's own turn loop (which gets its own primary fix in the next commit) — gets a synthetic, is_error tool_result injected rather than shipping (or silently dropping) a dangling tool_use the API rejects wholesale with HTTP 400 "tool_use ids were found without tool_result blocks immediately after". TestTranscodeOrphanToolUseMidHistory covers an orphan buried mid-transcript (no tool message follows it at all); TestTranscodeOrphanToolUseFinalMessage covers the orphan as the last message in history (nothing to look at, let alone merge into). Both assert every tool_use is immediately followed by a tool_result carrying its id, IsError true, with text containing "synthesized". Verified red first: reverting only transcode.go's ResolveOrphanToolCalls call (tests already in place) fails both new tests — "no matching tool_result in the immediately-following wire message" / "is in the final wire message, no tool_result can follow" — and passes once re-applied.
…est build Defense-in-depth for incident ses_01kx48z4rqfkpbwmzfdv1jzeg6, mirroring the identical anthropic fix: transcodeRequest now calls message.ResolveOrphanToolCalls(req.Messages) before transcoding, so any tool_call with no matching result in the immediately-following message gets a synthetic, is_error "tool"-role message injected rather than shipping a dangling tool_calls entry this wire protocol also requires a paired result for. TestTranscodeOrphanToolCallMidHistory covers an orphan buried mid-transcript; TestTranscodeOrphanToolCallFinalMessage covers the orphan as the last message in history (a turn with two tool calls, both orphaned). Both assert the synthesized "tool" message's content contains "synthesized" and the existing is_error string marker (toolResultOutput's "[tool error]" prefix — this wire has no boolean error field). Verified red first: reverting only transcode.go's ResolveOrphanToolCalls call fails both new tests with "no matching \"tool\" wire message after it"; re-applying it is green. TestTranscodeAssistantTextAndToolCalls and TestTranscodeAssistantToolCallOnlyNoContent previously assumed the assistant message transcodes to the *last* wire message, which no longer holds now that an orphaned final tool_call gets a synthetic message appended after it (both tests' own scripted histories are, unintentionally, exactly that shape) — updated to locate the assistant message by role via two new small helpers (findWireMessage/findRawWireMessage) rather than by position, preserving each test's original intent unchanged.
…a tool_call Incident ses_01kx48z4rqfkpbwmzfdv1jzeg6: a goal worker turn died with the Anthropic API 400 "tool_use ids were found without tool_result blocks immediately after", naming one tool_use id, and every subsequent goal-loop retry then failed identically, killing the goal. Mechanism: a provider stream can emit one or more complete tool_call blocks (provider.EventToolCall — see provider/anthropic/anthropic.go's content_block_stop handler and provider/openaicompat/openaicompat.go's emitToolCalls, both of which queue it once a tool_use/tool_call's arguments are fully buffered) and then die — a transport drop, or any other stream.Next error — before ever reaching EventDone. Before this fix, streamTurn's error path discarded that accumulated content entirely: nothing entered history, so no *this-session* history looked poisoned, but the model's tool_call vanished with no record and no result, and Prompt's caller (the goal loop) simply saw an ordinary turn error with nothing to indicate a tool call had ever been in flight. This is the wire-protocol sibling of the marshal-level poisoning fixed in "fix (message,engine): truncated ToolCall.Arguments must never poison history": that fix keeps a poisoned ToolCall marshalable at ingest; this one keeps a turn that died mid-flight from ever being replayed unpaired by any later code path. streamTurn now accumulates text and tool calls as EventTextDelta/ EventToolCall events arrive. When stream.Next ultimately errors, a turn that recorded zero tool calls behaves exactly as before (the ordinary provider-failure case, unaffected); one that recorded at least one wraps the error in a new interruptedTurnError carrying the assembled partial assistant message. Prompt unwraps it via errors.As, appends the partial message (preserving the model's visible intent — which tool, with what arguments) followed immediately by a synthetic tool-role message: one is_error ToolResult per recorded call, Content "interrupted: tool call was never executed because the turn ended abnormally" (interruptedTurnErrorText). The turn is still reported as a failure — Unwrap() makes the original error visible to errors.Is/As, so goal.go's context.Canceled check and promptTurnWithRetry's retry bookkeeping are unaffected — but history is self-consistent at ingest, so every later request build replays a paired tool_use/tool_result instead of the orphan. toolExecCount is deliberately NOT incremented for these synthetic results (the calls never ran), so a goal-loop retry of the same attempt remains exactly as safe as it always was for a turn that failed before executing anything (see promptTurnWithRetry's non-idempotency doc comment in goal.go). provider/anthropic/transcode.go and provider/openaicompat/transcode.go carry the composing defense-in-depth layer (message.ResolveOrphanToolCalls, landed in prior commits on this branch) for histories poisoned by any producer OTHER than this loop. TestOrphanedToolCallAppendsSyntheticResult reproduces the incident end-to-end with a scripted provider (diesAfterToolCallProvider/ dyingStream) standing in for a stream that emits one tool_call then a transport-style error: asserts the interrupted assistant message and its synthetic tool result land in history in order, that history marshals cleanly (the GET /message shape), that toolExecutions() stays at 0, and — the actual production failure mode — that a SUBSEQUENT worker turn's request build succeeds and recovers instead of dying identically again. TestOrphanedToolCallMultipleCalls covers a turn that recorded more than one tool call before dying (each gets its own synthetic result, none dropped). TestStreamErrorWithoutToolCallIsUnaffected proves the fix is scoped: a turn erroring with no tool call ever recorded is untouched — same bare, unwrapped error, nothing appended to history — exactly the ordinary-failure case every other engine test already covers. Verified red first: every one of these tests references interruptedTurnError/ interruptedTurnErrorText, so reverting engine.go's change alone (these tests already in place) fails the package build outright; re-applying it is green under go test -race.
📦 Previous Review (superseded by newer review)Claude finished @andybons's task in 3m 56s —— View job Review: PR #56 — synthesize tool results for orphaned tool callsI read
Overall assessmentThe two-layer approach is sound and the mechanics are correct:
Main point — description vs. implementation (see inline on
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The transcoder-level ResolveOrphanToolCalls is a wire-time backstop: it operates on the request copy and never persists, so a history loaded with an orphan (older-binary log, external writer) re-ran the synthesis on every request while GET /message and goal replay kept showing a protocol-invalid history. Run the repair in LoadSession after the log scan — the load-path counterpart of Session.append's Normalize — so resident history is protocol-valid everywhere. The log stays append-only; the repair is re-derived deterministically on each load.
📦 Previous Review (superseded by newer review)Claude finished @andybons's task in 3m 3s —— View job Review: PR #56 — synthesize tool results for orphaned tool calls
I re-read The three layers now hold together
Correctness checks I ran
Minor, non-blocking (carried over — all still just notes)
Nice work — the incident-linked doc comments in |
…rce index Provider call IDs are not guaranteed unique across turns; two orphaned turns reusing a CallID produced identical synthetic message IDs. Mix in the source message index — still deterministic for retranscode/reload. Also document that the interrupted-turn synthetic results are appended without an EventMessage emit (review note on PR #56).
|
Claude finished @andybons's task in 2m 39s —— View job Review: PR #56 — synthesize tool results for orphaned tool callsI re-read
Assessment: LGTM — no new findings; prior notes resolvedThe three-layer story is coherent and each layer is correct in isolation and in composition. Layer 1 — in-process turn death (
Layer 2 — load ingest ( Layer 3 — transcode backstop (both adapters). Correctness checks I ran:
Minor, non-blocking (both already documented in-code)
The incident-linked doc comments across |
1 participant
Fixes the second member of the truncated-turn poison family (#52 fixed the first): when a turn dies between emitting a tool_call and executing it — max_tokens cutoff mid tool call arrives as a NORMAL stop, mid-stream disconnect, process death — the history contains a tool_use with no tool_result. Anthropic rejects any subsequent request over that history with a 400 (
tool_use ids without tool_result blocks), permanently poisoning the session. This killed the github-port box session three times.Three layers, defense in depth:
engine.go): a turn that ends abnormally after recording tool_calls synthesizes theiris_errorresults ("interrupted: tool call was never executed…") immediately, before the history could ever be replayed. This is the durable primary fix for turns that die inside this process — the synthetic results are appended and persisted.store.go):LoadSessionrunsmessage.ResolveOrphanToolCallsafter the log scan — the load-path counterpart ofSession.append'sNormalize— so a history written by an older binary or an external writer is repaired the moment it enters memory, andGET /message, goal replay, and every downstream consumer see a protocol-valid history. The log stays append-only; the repair is re-derived deterministically on each load.anthropicandopenaicompatadapters): any orphan that still reaches the wire (a history mutated by a pluginchat.messagehook, a hand-rolled adapter, a scripted test provider) is paired with a synthetic error result at transcode time, so the request is valid regardless of how the history was produced. Wire-time backstop only — operates on the request copy, never persists.Synthetic results are visibly marked ("synthesized: no tool_result was found…") so they can never be mistaken for real tool output in a transcript.
Tests cover: orphan at end of history, orphan mid-history followed by user text, multiple orphans in one assistant message, mixed resolved/orphaned calls, both adapters' wire shapes, LoadSession repair of an orphaned log, and an engine-level replay test that kills a turn between tool_call emission and execution then proves the next prompt succeeds.
go test -race ./...green.