fix(agent): close orphaned assistant(tool_calls) in repair_message_sequence (#56980) - #820
Closed
hashbender wants to merge 1 commit into
Closed
fix(agent): close orphaned assistant(tool_calls) in repair_message_sequence (#56980)#820hashbender wants to merge 1 commit into
hashbender wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 NousResearch#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 NousResearch#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 (NousResearch#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 NousResearch#56980 (design gap this closes), fixes NousResearch#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 NousResearch/hermes-agent#57036)Mirror-of: NousResearch#57013
NousResearch#57013