fix(agent): make sanitize_api_messages a fixpoint so dedup cannot re-wedge sessions (#83312)

[JoaoMarcos44]

Fixes #83312.

The bug is real, and it is not where the issue says it is

The issue reports that tool_calls: [] reaches DeepSeek "after the sanitizer already stripped it". That is accurate as an observation, but the payload is not surviving the sanitizer — the sanitizer creates it.

sanitize_api_messages is an ordered pipeline. Two of its passes enforce invariants near the top:

• repair_empty_non_final_messages — no non-final turn may have empty content
• the empty-tool_calls drop — no assistant turn may carry tool_calls: []

The tool_call_id dedup pass runs last. When every call on an assistant turn is a duplicate, kept_tcs is empty and the turn is rewritten to tool_calls: [] — re-creating the exact payload that was deleted a few steps earlier, at a point where nothing downstream can see it.

That "every call is a duplicate" shape is not exotic. It is what repair_message_sequence produces when it merges two consecutive assistant turns and unions their call lists onto the surviving text turn, which is why the issue's reporters correlate the failure with the Repaired N message-alternation violations log line.

Because the poisoned turn is persisted, every later send fails identically. The session is wedged permanently — restart included.

Reproduction

user      "hi"
assistant content=None, tool_calls=[call_dup]
tool      call_dup -> "file body"
assistant content="here is the file", tool_calls=[call_dup]   <- duplicate id
user      "and now?"

On main, sanitize_api_messages returns the 4th message as {"role": "assistant", "content": "here is the file", "tool_calls": []} → HTTP 400 Invalid 'messages[3].tool_calls': empty array.

Why patching the dedup site is only half a fix

Several open PRs (#83315, #80827, #82252, #77377, #74906, #64843) drop the key at the dedup site, and #82252 additionally re-runs an empty-array sweep afterwards. Credit where due: that closes the empty-array half.

It leaves the other half open. If the collapsed turn carried no text, it comes back with empty content and no tool calls. That is a second, independent 400 — all messages must have non-empty content except for the final one (INVALID_REQUEST_BODY) — and the content healer that exists to fix exactly this already ran, several passes earlier, back when the turn still had tool calls and therefore looked like it had a payload. The session stays wedged; only the error string changes.

This PR: make the sanitizer a fixpoint

A sanitizer is only useful if every invariant it claims still holds on the value it returns. This restores that property instead of patching one symptom site:

1. Extract the empty-array normalization into drop_empty_tool_calls_arrays — the same code, now callable, so nothing is duplicated.
2. After dedup, re-run both invariant passes on the deduped list, gated on removed_dupes so the common path is untouched.

Order matters: arrays are dropped first, so the content healer sees a genuinely payload-less turn and substitutes its placeholder.

This covers both 400 classes, and covers any future pass inserted before the return — not just this one.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%%
graph TD
    A[Transcript] --> B[Role Allowlist]
    B --> C[Heal Empty Content]
    C --> D[Drop Empty tool_calls]
    D --> E[Pair Orphan Calls / Results]
    E --> F[Dedup tool_call_ids]
    F -->|All calls duplicate| G[Turn Rewritten To tool_calls Empty Array]
    G -->|main| H[HTTP 400 Empty Array]
    G -->|main, turn had no text| I[HTTP 400 Empty Content]
    H --> J[Session Wedged Permanently]
    I --> J
    G -->|this PR| K[Re-run Drop Empty tool_calls]
    K --> L[Re-run Heal Empty Content]
    L --> M[Invariants Hold On Return]
    M --> N[Request Accepted]

Loading

Tests

tests/run_agent/test_message_sequence_repair.py (extended, no new file):

• test_dedup_does_not_reintroduce_empty_tool_calls_array — the empty-array 400
• test_dedup_collapse_heals_contentless_turn — the empty-content 400 the site patches miss
• test_sanitize_is_a_fixpoint_over_the_wedge_transcript — sanitize(sanitize(x)) == sanitize(x)

All three fail on main and pass here. Full file: 17 passed. test_agent_guardrails.py + test_restore_alternation_repair.py: 43 passed. test_tool_call_incremental_persistence.py shows 9 passed / 4 failed both with and without this change (pre-existing local Windows failures, unrelated).

Infographic :

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(agent): make sanitize_api_messages a fixpoint so dedup cannot re-wedge sessions (#83312)

1. Correct and well-tested fix — the idempotency test (sanitize twice == sanitize once over the wedge transcript) is exactly the right regression shape for a fixpoint claim, and extracting drop_empty_tool_calls_arrays into a reusable helper is clean.
2. The re-run after dedup covers the empty-array and empty-content invariants only. The orphan-repair pass (tool messages whose tool_call_id has no surviving assistant tool_call) is not re-run after dedup. In the traced wedge shape no orphan is created because the first assistant turn keeps the call, but asserting "no orphaned tool messages in the output" in the fixpoint test would make the fix robust to future changes in the dedup pass.
3. The comment "it covers any future pass inserted before this return" slightly overstates the guarantee — only the two re-run passes are covered; any future pass inserted before the return would need the same treatment.
4. Minor, pre-existing: repair_empty_non_final_messages does not heal a final assistant turn with empty content (the test deliberately checks assistants[:-1]). A final empty-content turn would still be sent as-is; noting it here so the fixpoint claim doesn't imply coverage of the final turn.