fix(message,engine): truncated ToolCall.Arguments must never poison history

[andybons]

Fixes the in-memory history poisoning that killed two production goal sessions today (ses_01kx453ewfedqrg7p3c64f8sca, ses_01kx453ev9ejattygpf7rbzptw): json: error calling MarshalJSON for type json.RawMessage: unexpected end of JSON input at the start of a worker turn, three identical retry failures, and GET /session/{id}/message 500ing — while the on-disk journal stayed clean (verified by chunk-bisecting both logs through engine.LoadSession: all lines load and re-marshal).

Mechanism: a provider stream that ends mid tool_use block — a connection drop during input_json_delta accumulation, or, notably, a max_tokens cutoff that the Anthropic API closes out with a normal stop sequence rather than an error — leaves ToolCall.Arguments holding non-empty but truncated JSON. Every existing guard (safeArguments, ProviderData.Get) special-cases len == 0 only; json.RawMessage.MarshalJSON doesn't validate its bytes, so the invalid value fails only when embedded in a larger document — persist, request build, or a later GET.

Fix, two layers (red-first, incident-named tests):

• ee45eb8 — Message.Normalize drops syntactically invalid Arguments at the single ingest choke point, so a poisoned tool call never enters history (tool name + call ID survive; only the unusable truncated bytes are cleared)
• c497e65 — defense-in-depth: safeArguments now also normalizes non-json.Valid Arguments to {}, so a producer that bypasses Normalize (plugin chat.message hook, hand-rolled adapter, scripted test provider) still cannot make a history marshal fail; red-verified by reverting just this change with the Normalize fix still applied

Note: an earlier duplicate of this fix (fcb5d7f) briefly landed on feat/mcp-engine via a shared box workdir; it has been dropped from that branch — this PR is the canonical fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_013B9pwUYhT4o8se9skmS5PP

[claude]

Claude finished @andybons's task in 3m 43s —— View job

---

Review: truncated ToolCall.Arguments poisoning fix

• Read AGENTS.md and repo conventions
• Review message/message.go (Normalize + safeArguments)
• Review tests for TDD / red-first / no-sleep rules
• Check edge cases and correctness gaps
• Verify build + red-first regression

Verdict: LGTM. This is a correct, well-scoped, and genuinely red-verified fix. I reverted both fix lines locally and confirmed all three new tests fail against pre-fix code with the exact production error text, then pass once restored:

--- FAIL: TestPersistTruncatedToolCallArguments
    PersistErr = json: error calling MarshalJSON for type message.Parts:
      ...json.RawMessage: unexpected end of JSON input, want nil
--- FAIL: TestNormalizeDropsInvalidToolCallArguments
    Normalize left invalid Arguments = {"command":"echo hel, want cleared
--- FAIL: TestToolCallInvalidTruncatedArgumentsMarshal (all 4 subcases)

Satisfies the AGENTS.md red-verification rule; no time.Sleep, uses the scripted-provider harness.

What's right

• Root cause is correctly identified. I traced the accumulation path: anthropic.go:258 appends raw input_json_delta bytes into b.inputJSON, and toolCall() (anthropic.go:337) clones them verbatim into Arguments — so a stream that ends mid-block (max_tokens cutoff closed out normally, per assemble()) leaves non-empty truncated JSON. json.RawMessage.MarshalJSON doesn't validate, so it only explodes once nested and compacted. Exactly as described.
• Two-layer fix is the right shape. Normalize sanitizes at the single ingest choke point; safeArguments' json.Valid check is real defense-in-depth for producers that bypass Normalize (plugin chat.message hook, hand-rolled adapters). Reverting only safeArguments (with Normalize kept) still leaves the bare-ToolCall-marshal path red, so the second layer earns its place.
• Clearing to nil and keeping CallID/Name is correct — those are provider-set strings with no marshal risk, and nil collapses to {} at every marshal/wire site, introducing no new downstream shape.
• The if→switch conversion in Normalize preserves the *Reasoning behavior unchanged.

One thing I checked that could look like a gap (it isn't)

The new doc comments (message.go:289–319) call Reasoning.ProviderData "the map-shaped twin of the ToolCall.Arguments footgun," yet the ProviderData guards (Get, MarshalJSON, and the Normalize *Reasoning branch) were not given the same json.Valid treatment — only len==0/oversize. That asymmetry is correct: ProviderData entries are always built via json.Marshal(anthropicReasoningData{...}) (anthropic.go:358,364), i.e. valid-by-construction — a truncated signature string still yields structurally valid JSON ({"signature":"partial…"}). Only Arguments, which accumulates raw wire bytes, can truncate into invalid JSON. So the "twin" comment is about the zero-length footgun, and extending json.Valid to ProviderData would be dead code. Worth keeping this reasoning in mind if anyone later "notices the twin was missed."

Minor (non-blocking, out of scope)

json.Valid accepts valid-but-non-object scalars (123, "x", [1]) as Arguments — these survive both guards and marshal fine, so they're not the incident and not a regression; the model effectively never emits them for a tool_use block. Flagging only for awareness; no change requested.

Nice work — clean, minimal, and the incident-named tests document the failure mode precisely.
· fix/partial-stream-poison