fix: never send empty tool_calls arrays to strict providers (#83312)

[bsgdigital]

Summary

Fixes a session-wedging bug on strict OpenAI-compatible providers (DeepSeek v4): an assistant message carrying tool_calls: [] (an empty array) is rejected with HTTP 400 (Invalid 'messages[N].tool_calls': empty array. Expected an array with minimum length 1), and because the poisoned message stays in replayed history, every subsequent message to that session fails the same way until the agent cache is cleared.

Motivation

We observed this across multiple sessions on DeepSeek. The pre-API sanitizer added for #56980/#58755 (sanitize_api_messages) correctly strips tool_calls: [] when it sees it, but the empty array can be reintroduced after that sanitizer — e.g. the consecutive-assistant merge in repair_message_sequence explicitly preserves a pre-existing [] on the surviving turn (documented in the sanitizer's own comment as a known source). Once it reaches the wire, the failure is non-retryable and permanent for the session.

Changes

agent/transports/chat_completions.py

convert_messages() is the final chokepoint before the HTTP body is serialized. Two additions:

1. Detection scan — an assistant message with an empty tool_calls list now marks the history dirty, so the sanitize pass runs.
2. Sanitize pass — empty tool_calls on an assistant message is dropped (key removed entirely) via the existing copy-on-write path, so the caller's in-memory history is never mutated and the persisted transcript stays byte-stable.

This guarantees the wire invariant regardless of which path built the messages (main loop, retries, fallback re-builds, host-fed histories).

agent/agent_runtime_helpers.py

In repair_message_sequence, the consecutive-assistant merge unioned tool_calls but left a pre-existing [] in place when both sides were empty. The merge now pops the key when the union is empty — fixing the documented source of the poison.

Tests

• tests/agent/transports/test_chat_completions.py
  • test_convert_messages_drops_empty_tool_calls_array — empty array stripped on the sanitized copy, original untouched.
  • test_convert_messages_keeps_nonempty_tool_calls_identity — healthy non-empty tool_calls passes through without a copy (no regression to the copy-on-write optimization).
• tests/run_agent/test_message_sequence_repair.py
  • test_repair_merge_drops_empty_tool_calls_on_surviving_turn — merge pops the key when neither side carries calls.
  • test_repair_merge_keeps_nonempty_tool_calls_union — real tool calls are still unioned onto the surviving turn.

Design decisions

• Fix at the wire boundary, not only mid-pipeline. The earlier sanitizer is correct but insufficient — something after it was re-adding the empty array. convert_messages runs after every request-builder path, so it is the right place to enforce "no assistant message ever carries an empty tool_calls array".
• Copy-on-write preserved. The transport only copies messages that actually need stripping, so the common healthy path stays allocation-free.
• Repair fix addresses the known source so the poison is not created in the first place; the transport fix is the belt that catches any future path that creates it.

Backwards compatibility

Zero breaking changes. Messages with non-empty tool_calls are untouched (identity-preserving). Empty tool_calls is semantically "no tool calls" — omitting the key is the canonical OpenAI shape and is what permissive providers already receive from this codebase's other message builders.

Testing

• tests/agent/transports/test_chat_completions.py — pass
• tests/run_agent/test_message_sequence_repair.py — pass
• tests/run_agent/test_agent_guardrails.py, tests/providers/test_transport_parity.py, related repair/sanitize subsets — pass
• Real-world replay: the exact failing payload captured from the production incident (163 messages, request_dump_1f819f5dbd28_20260808_144115_112720.json — assistant message 161 with tool_calls: []) now passes through convert_messages() with zero empty arrays remaining.

[misterboo]

Review from the #6545 analysis side (we maintain the repro session 20260810_102548_bd851193 and opened nesquena/hermes-webui#6904 for the WebUI layer):

This is the right shape for the agent layer. Our root-cause trace identified the mutation as the dedup pass inside sanitize_api_messages (agent_runtime_helpers.py, dedup block (a) "collapse duplicate tool_calls WITHIN an assistant message" — a stripped-to-zero call set is re-assigned as msg = {**msg, "tool_calls": kept_tcs} with kept_tcs == []). That fresh empty array is created after the pre-API empty-drop runs, so it survives to the wire. Putting the fail-closed guard at convert_messages (the final chokepoint before serialization) catches that array regardless of which sanitizer re-introduced it — exactly the maintainer's "keep both layers fail-closed" ask.

What we particularly like:

• Copy-on-write (dirty-history copy, caller history untouched) — the persisted transcript stays byte-stable, which matters because the poisoned row is what wedges the session.
• The repair_message_sequence merge fix (prev.pop("tool_calls", None)) addresses the source-side producer.
• The empty-array check is provider-independent (no DeepSeek-specific branch).

One note: the maintainer's invariant also mentions dropping a content-empty assistant message; at the chokepoint removing the key is sufficient (an empty assistant with no calls is harmless to strict providers once the key is gone) — the message-drop belongs in the earlier sanitizer, which already handles it.

We could not run the transport tests locally (our installed agent version predates parts of current main; the tests' CI will cover them). Based on the diff, this PR looks mergeable.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #64345, the earlier open fix for the same empty tool_calls strict-provider failure. Please consolidate review effort there.

[teknium1]

The empty-tool_calls 400 class this PR targets is now fixed on main via #86654, which closes all three chokepoints: the sanitize_api_messages dedup pass drops the key instead of writing [] (salvaged from #64345 by @liuhao1024, earliest fix for that site), the repair_message_sequence merge pops the stale key at the source (salvaged from #77944 by @webtecnica), and ChatCompletionsTransport.convert_messages() strips empty/null tool_calls at the wire boundary (salvaged from #72591 by @TurgutKural, earliest fix for that site). Your analysis matched the merged approach — thank you for the contribution; closing in favor of the merged class fix with credit to the earliest filer per site.