fix(agent): close orphaned assistant(tool_calls) in repair_message_sequence (#56980) - #57013
fix(agent): close orphaned assistant(tool_calls) in repair_message_sequence (#56980)#57013JoaoMarcos44 wants to merge 1 commit into
Conversation
…quence Interruption (/stop, process kill, resume mid tool-loop) can leave an assistant(tool_calls) turn with some or all of its tool_call_ids unanswered. Strict OpenAI-compatible providers (DeepSeek v4, Moonshot/Kimi) reject that shape outright instead of completing, which bricks the session until /new. Add a repair pass that detects tool_calls not fully answered by the tool messages immediately following them and inserts a synthetic error result for each missing id, using the same stub shape already used by sanitize_api_messages. Stubbing (instead of stripping the tool_calls) keeps the model's stated intent visible in the transcript and heals the persisted message list at the source, rather than only the ephemeral per-call copy. No-op when every call already has a matching result, so the existing ongoing-dialog pattern (complete assistant(tool_calls)+tool pair followed by a user redirect) is left untouched. Fixes NousResearch#56980
Related: fixes #56980 (session-bricking HTTP 400 on strict providers). Distinct mechanism from the closed sibling #56990 — this PR stubs a synthetic tool result for each unanswered |
|
Following up after this was closed: investigation found #56980 isn't reproducible as filed (the cited build/commit doesn't exist in repo history, and the pipeline already patches this shape downstream via sanitize_api_messages before it reaches the provider). The design gap this PR closed is still real though — repair_message_sequence isn't self-contained without a follow-up sanitize_api_messages call. Reopened cleanly as #57039, with a fix for an id-extractor gap this PR also had. New PR incoming from there. |
What does this PR do?
repair_message_sequencehandles straytoolmessages, consecutive-assistant merges, and consecutive-user merges, but had no pass for the inverse case: anassistantmessage withtool_callswhose ids are not answered by thetoolmessages that follow. This happens after an interruption (/stop, process kill, resume mid tool-loop) when some or all of a turn's tool calls never got a result appended.Strict OpenAI-compatible providers (DeepSeek v4, Moonshot/Kimi) reject that message shape outright:
Scope, honestly
This does not fix an actively-reproducing HTTP 400 in the main request pipeline.
sanitize_api_messages(called right afterrepair_message_sequenceon every call, inconversation_loop.pyandchat_completion_helpers.py— the single path CLI, gateway, and webui all funnel through) already injects a stub result for orphanedtool_call_ids before the request reaches the provider. Verified this directly against the real pipeline for every orphan shape in #56980; none reproduce the 400 on currentmain.What this PR actually fixes:
repair_message_sequenceoperates on the canonical, persistedmessageslist, whilesanitize_api_messagesonly patches the ephemeral per-callapi_messagescopy. Any caller that runsrepair_message_sequencewithout a follow-upsanitize_api_messagescall — sub-agents, MoA reference-model calls, plugins, or the existingrun_agent.pyforwarder used only by tests — stays exposed to the exact symptom in #56980. This closes that gap at the source instead of depending on a second, separately-invoked function to catch it downstream every single call.Fix
Added a pass to
repair_message_sequence: for eachassistant(tool_calls), scan thetoolmessages immediately following it, and for anytool_call_idstill unanswered, insert a synthetic error result:{"role": "tool", "tool_call_id": "<id>", "name": "<tool name>", "content": "Tool execution was interrupted before a result was returned."}Stubs the missing result instead of stripping
tool_callsfrom the assistant message, to avoid leaving{"role": "assistant", "content": None}when a turn's calls are all orphaned, and to keep the model's stated intent visible in the transcript.No-op when every
tool_call_idalready has a matching result — the existing "ongoing dialog" pattern (a completeassistant(tool_calls)+toolpair followed by a user redirect) is left untouched, as documented in the function's docstring.Follow-up fix (#57036): the id lookup in this pass initially read
tool_calls[].iddirectly. Switched it toAIAgent._get_tool_call_id_static/_get_tool_call_name_static— thecall_id || idextractorsanitize_api_messagesand the context compressor already use — since Codex/Responses-API-style tool_calls key the id ascall_id, notid. Without this, the pass silently had no effect on that shape.Related
Fixes #56980 (design gap this closes), fixes #57036 (extractor follow-up).
Testing
tests/run_agent/test_message_sequence_repair.py: 6 new tests — fully unanswered trailingtool_calls, partially answered paralleltool_calls, orphanedtool_callsfollowed by an injected user/wakeup message, idempotency,call_id-keyed (Codex-shaped) orphans, and a non-regression check that a complete pair before a user redirect is left untouched.python -m pytest tests/run_agent/test_message_sequence_repair.py -v— 31/31 passing.tests/agent/test_context_compressor.py,tests/agent/test_replay_cleanup.py,tests/agent/test_close_interrupted_tool_sequence.py— 159/159 passing.test_run_agent.pysuite — 414/414 passing.Type of Change
Checklist
call_id||idextractor as the rest of the file (post repair_message_sequence orphan-close pass doesn't use canonical call_id||id extractor #57036)