Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions agent/agent_runtime_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -824,12 +824,24 @@ def _is_verification_candidate(m: Dict) -> bool:
_remaining = dict(answered_counts)
answered_flags = [] # parallel to ``calls``: True = this entry answered
for tc in calls:
_cid = tc.get("id") if isinstance(tc, dict) else None
if _cid and _remaining.get(_cid, 0) > 0:
_remaining[_cid] -= 1
answered_flags.append(True)
else:
answered_flags.append(False)
# Match on the SUPERSET of ``id`` and ``call_id`` -- the same
# tolerance Pass 1 applies when registering the answer budget
# (#58168). Resolving against ``id`` ALONE made a tool_call
# carrying only ``call_id`` (or a Codex-Responses call whose
# ``id`` and ``call_id`` differ) read as unanswered, so the
# "none answered" branch below DELETED an assistant turn that
# was in fact answered by the following ``tool`` message --
# leaving a stray tool result and the very 400 this pass
# exists to prevent.
_matched = False
if isinstance(tc, dict):
for _key in ("id", "call_id"):
_cid = tc.get(_key)
if _cid and _remaining.get(_cid, 0) > 0:
_remaining[_cid] -= 1
_matched = True
break
answered_flags.append(_matched)
unanswered_calls = [
tc for tc, ok in zip(calls, answered_flags) if not ok
]
Expand Down
69 changes: 30 additions & 39 deletions tests/run_agent/test_message_sequence_repair.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
recovery every turn.
"""

import pytest
from run_agent import AIAgent


Expand Down Expand Up @@ -108,25 +107,6 @@ def test_repair_drops_stray_tool_with_unknown_tool_call_id():
assert all(m.get("role") != "tool" for m in messages)


@pytest.mark.xfail(
reason=(
"INHERITED fork/main defect, NOT parity-merge damage -- see card t_08cca32f. "
"Pass 1.5 of repair_message_sequence (agent/agent_runtime_helpers.py, "
"fork-only code from PR #196 / aaa34311e0) resolves a tool_call's answered "
"budget against tc.get('id') ONLY, while Pass 1 correctly matches the "
"id||call_id superset per PR #58168. A tool_call carrying only call_id (or a "
"Codex-Responses call whose id and call_id differ) therefore reads as "
"unanswered, and the 'none answered' branch deletes an assistant turn that "
"WAS answered. Evidence: reproduced identically on a clean detached worktree "
"at fork/main ee2fce2876 (repairs=1, expected 0); the merge's diff over the "
"Pass 1.5 region is empty; 'Pass 1.5' appears 6x at fork/main HEAD and 0x at "
"both merge-base a7a696ba and upstream target 1e5b5074. These tests exist at "
"the merge base and upstream but were dropped from fork/main -- the merge "
"restored them, which is how the bug surfaced. strict=False so this xpasses "
"and flags itself for deletion once t_08cca32f lands."
),
strict=False,
)
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
Expand Down Expand Up @@ -157,25 +137,6 @@ def test_repair_keeps_tool_matching_codex_call_id():
assert messages[2]["tool_call_id"] == "call_ABC"


@pytest.mark.xfail(
reason=(
"INHERITED fork/main defect, NOT parity-merge damage -- see card t_08cca32f. "
"Pass 1.5 of repair_message_sequence (agent/agent_runtime_helpers.py, "
"fork-only code from PR #196 / aaa34311e0) resolves a tool_call's answered "
"budget against tc.get('id') ONLY, while Pass 1 correctly matches the "
"id||call_id superset per PR #58168. A tool_call carrying only call_id (or a "
"Codex-Responses call whose id and call_id differ) therefore reads as "
"unanswered, and the 'none answered' branch deletes an assistant turn that "
"WAS answered. Evidence: reproduced identically on a clean detached worktree "
"at fork/main ee2fce2876 (repairs=1, expected 0); the merge's diff over the "
"Pass 1.5 region is empty; 'Pass 1.5' appears 6x at fork/main HEAD and 0x at "
"both merge-base a7a696ba and upstream target 1e5b5074. These tests exist at "
"the merge base and upstream but were dropped from fork/main -- the merge "
"restored them, which is how the bug surfaced. strict=False so this xpasses "
"and flags itself for deletion once t_08cca32f lands."
),
strict=False,
)
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).
Expand All @@ -196,6 +157,36 @@ def test_repair_keeps_tool_matching_only_call_id():
assert any(m.get("role") == "tool" for m in messages)


def test_repair_pass_1_5_keeps_call_id_only_assistant_turn():
"""Pass 1.5 must resolve a tool_call's answered budget on ``call_id`` OR
``id``, matching Pass 1's ``id``/``call_id`` superset (#58168).

Regression guard: Pass 1.5 previously read only ``tc.get("id")``, so an
assistant turn whose tool_call carried ONLY ``call_id`` read as entirely
unanswered even though the following ``tool`` message answered it. The
"none answered" branch then DELETED the valid assistant turn, producing
``repairs=1`` and a history of ``[user, tool, user]`` -- a stray tool
message with no preceding tool_calls, i.e. the exact HTTP 400 shape the
repair exists to prevent.
"""
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 [m["role"] for m in messages] == ["user", "assistant", "tool", "user"]
# the assistant turn survives INTACT -- its tool_calls are not stripped
assert messages[1]["tool_calls"][0]["call_id"] == "call_XYZ"





Expand Down
Loading