mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-07-08 13:12:08 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(agent): match tool results on call_id||id in pre-request repair (#58168)

Browse source 
repair_message_sequence Pass 1 registered only tc.get("id") when building
the set of known assistant tool_call ids, then matched tool results against
it by tool_call_id. In the Codex Responses format an assistant tool_call
carries both id (fc_...) and a distinct call_id (call_...); a tool result's
tool_call_id may be keyed on either depending on which builder produced it.
Registering only id made a valid tool result whose tool_call_id matched
call_id look orphaned, so the pass dropped it and left the assistant
tool_call unanswered -- producing HTTP 400 on strict providers (DeepSeek,
Kimi): 'Messages with role tool must be a response to a preceding message
with tool_calls'. Long-running sessions that persisted such a sequence were
permanently broken, re-sending the orphan every turn.

Register both id and call_id for each assistant tool_call so a result
matching either key is recognized, consistent with
AIAgent._get_tool_call_id_static and the compressor's _sanitize_tool_pairs.
Apply the same call_id||id precedence to the corrupted-args sanitizer's
existing-result scan / stub insertion, which had the identical mismatch.

Adds 3 regression tests covering the codex id!=call_id case (match on
call_id, match on only call_id, match on id when both present).
This commit is contained in:
kshitijk4poor 2026-07-04 15:04:11 +05:30 • committed by kshitij
parent ca596e228c
commit 88f2c0caf6
2 changed files with 125 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

32
agent/agent_runtime_helpers.py
View file

@ -306,7 +306,13 @@ def sanitize_tool_call_arguments(
try:
json.loads(arguments)
except json.JSONDecodeError:
tool_call_id = tool_call.get("id")
# Use the canonical ``call_id || id`` precedence so both the
# scan for an existing tool result and any inserted stub key
# on the same id the rest of the pipeline uses. Keying on bare
# ``id`` here would fail to find a result built with ``call_id``
# (Codex Responses format) and insert a duplicate stub that
# itself becomes an orphan (#58168).
tool_call_id = _ra().AIAgent._get_tool_call_id_static(tool_call) or None
function_name = function.get("name", "?")
preview = arguments[:80]
log.warning(
@ -464,6 +470,21 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int:
# Pass 1: drop stray tool messages that don't follow a known
# assistant tool_call_id. Uses a rolling set of known ids refreshed
# on each assistant message.
#
# Both ``id`` AND ``call_id`` are registered for every assistant
# tool_call. In the Codex Responses API format the two differ
# (``id`` = ``fc_...`` response-item id, ``call_id`` = ``call_...``
# the function-call id), and a tool result's ``tool_call_id`` may be
# matched against *either* depending on which code path built it
# (the OpenAI-compatible path stores ``tc.id``; codex paths store
# ``call_id``). Registering only ``id`` — as this pass did before —
# made a valid tool result look orphaned whenever the assistant
# tool_call carried a distinct ``call_id`` (or only ``call_id``); the
# pass then dropped it, leaving the assistant tool_call unanswered and
# producing an HTTP 400 on strict providers (DeepSeek, Kimi). Matching
# on the *superset* of both keys achieves the same tolerance as
# ``_get_tool_call_id_static``'s ``call_id || id`` — a match set must
# accept every legitimate reference, not just the canonical one (#58168).
known_tool_ids: set = set()
filtered: List[Dict] = []
for msg in collapsed:
@ -474,9 +495,12 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int:
if role == "assistant":
known_tool_ids = set()
for tc in (msg.get("tool_calls") or []):
tc_id = tc.get("id") if isinstance(tc, dict) else None
if tc_id:
known_tool_ids.add(tc_id)
if not isinstance(tc, dict):
continue
for key in ("id", "call_id"):
tc_id = tc.get(key)
if tc_id:
known_tool_ids.add(tc_id)
filtered.append(msg)
elif role == "tool":
tc_id = msg.get("tool_call_id")

97
tests/run_agent/test_message_sequence_repair.py
View file

@ -143,6 +143,103 @@ def test_repair_drops_stray_tool_with_unknown_tool_call_id():
assert all(m.get("role") != "tool" for m in messages)
def test_repair_keeps_tool_matching_codex_call_id():
"""A valid tool result must survive when the assistant tool_call carries a
Codex-format ``call_id`` distinct from ``id`` and the result matches on
``call_id`` (#58168).
Before the fix, Pass 1 registered only ``tc.get("id")`` (``fc_...``) in the
known-id set, so a result keyed on ``call_id`` (``call_...``) looked
orphaned and was dropped -- leaving the assistant tool_call unanswered and
triggering an HTTP 400 on strict providers (DeepSeek, Kimi):
"Messages with role 'tool' must be a response to a preceding message with
'tool_calls'".
"""
agent = _bare_agent()
messages = [
{"role": "user", "content": "do it"},
{"role": "assistant", "content": "",
"tool_calls": [{"id": "fc_123", "call_id": "call_ABC",
"type": "function",
"function": {"name": "x", "arguments": "{}"}}]},
{"role": "tool", "tool_call_id": "call_ABC", "content": "result"},
{"role": "user", "content": "next"},
]
repairs = AIAgent._repair_message_sequence(agent, messages)
assert repairs == 0
assert [m["role"] for m in messages] == ["user", "assistant", "tool", "user"]
assert messages[2]["tool_call_id"] == "call_ABC"
def test_repair_keeps_tool_matching_only_call_id():
"""Same as above but the assistant tool_call carries ONLY ``call_id`` (no
``id``). The result keyed on ``call_id`` must still be recognized (#58168).
"""
agent = _bare_agent()
messages = [
{"role": "user", "content": "do it"},
{"role": "assistant", "content": "",
"tool_calls": [{"call_id": "call_XYZ", "type": "function",
"function": {"name": "x", "arguments": "{}"}}]},
{"role": "tool", "tool_call_id": "call_XYZ", "content": "result"},
{"role": "user", "content": "next"},
]
repairs = AIAgent._repair_message_sequence(agent, messages)
assert repairs == 0
assert any(m.get("role") == "tool" for m in messages)
def test_repair_keeps_tool_matching_id_when_call_id_also_present():
"""When the assistant tool_call carries both ``id`` and ``call_id`` and the
result matches on ``id`` (OpenAI-compatible builder path), it must be kept
(#58168 -- both keys are registered, so either matches).
"""
agent = _bare_agent()
messages = [
{"role": "user", "content": "do it"},
{"role": "assistant", "content": "",
"tool_calls": [{"id": "fc_9", "call_id": "call_9",
"type": "function",
"function": {"name": "x", "arguments": "{}"}}]},
{"role": "tool", "tool_call_id": "fc_9", "content": "result"},
{"role": "user", "content": "next"},
]
repairs = AIAgent._repair_message_sequence(agent, messages)
assert repairs == 0
assert any(m.get("role") == "tool" for m in messages)
def test_repair_still_drops_genuine_orphan_alongside_codex_pair():
"""Negative control for #58168: registering both id and call_id must NOT
over-relax orphan detection. A genuinely orphaned tool result (matching
neither the id nor the call_id of any assistant tool_call) is still
dropped, while the valid codex-format pair in the same window survives.
"""
agent = _bare_agent()
messages = [
{"role": "user", "content": "go"},
{"role": "assistant", "content": "",
"tool_calls": [{"id": "fc_1", "call_id": "call_1",
"type": "function",
"function": {"name": "x", "arguments": "{}"}}]},
{"role": "tool", "tool_call_id": "call_1", "content": "valid"},
{"role": "tool", "tool_call_id": "call_ORPHAN", "content": "stray"},
{"role": "user", "content": "next"},
]
repairs = AIAgent._repair_message_sequence(agent, messages)
assert repairs == 1
tool_ids = [m["tool_call_id"] for m in messages if m.get("role") == "tool"]
assert tool_ids == ["call_1"]
def test_repair_leaves_valid_conversation_unchanged():
agent = _bare_agent()
messages = [