diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 648b73df1be9..85cd7edf9237 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -390,6 +390,9 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: any preceding assistant tool_call — dropped. 2. Consecutive ``user`` messages — merged with newline separator so no user input is lost. + 3. Duplicate ``tool`` messages for the same live tool_call_id are + dropped; distinct consecutive tool results from parallel calls are + preserved. Deliberately does NOT rewind orphan ``assistant(tool_calls)+tool`` pairs that precede a user message — that pattern IS valid when the @@ -486,6 +489,7 @@ def _is_codex_interim(m: Dict) -> bool: # ``_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() + seen_tool_ids: set = set() filtered: List[Dict] = [] for msg in collapsed: if not isinstance(msg, dict): @@ -494,6 +498,7 @@ def _is_codex_interim(m: Dict) -> bool: role = msg.get("role") if role == "assistant": known_tool_ids = set() + seen_tool_ids = set() for tc in (msg.get("tool_calls") or []): if not isinstance(tc, dict): continue @@ -504,8 +509,9 @@ def _is_codex_interim(m: Dict) -> bool: filtered.append(msg) elif role == "tool": tc_id = msg.get("tool_call_id") - if tc_id and tc_id in known_tool_ids: + if tc_id and tc_id in known_tool_ids and tc_id not in seen_tool_ids: filtered.append(msg) + seen_tool_ids.add(tc_id) else: repairs += 1 else: @@ -514,6 +520,7 @@ def _is_codex_interim(m: Dict) -> bool: # tool messages without a fresh assistant tool_call # are orphans. known_tool_ids = set() + seen_tool_ids = set() filtered.append(msg) # Pass 2: merge consecutive user messages. Preserves all user input @@ -2478,6 +2485,61 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] "Pre-call sanitizer: added %d stub tool result(s)", len(missing_results), ) + + # 3. Enforce unique tool_call_id. Strict providers (DeepSeek) reject any + # tool_call_id that appears more than once — either as two tool results or + # as two assistant tool_calls — with HTTP 400 "Duplicate value for + # 'tool_call_id' of ... in message[N]". Context compaction / session-merge + # paths can leave the SAME assistant(tool_calls) group duplicated across two + # messages; the stub injection above then doubles a dropped result into two + # identical stubs, so both sides can end up with repeated ids. The orphan + # repair passes only fix missing/dangling links — never a genuine duplicate — + # so this dedup is the piece that keeps the pairing invariant atomic. First + # occurrence wins (assistant call and its first matching result are kept), + # so the surviving pair stays well-formed. + seen_assistant_ids: set = set() + seen_result_ids: set = set() + deduped: List[Dict[str, Any]] = [] + dropped_dup_calls = 0 + dropped_dup_results = 0 + for msg in messages: + role = msg.get("role") + if role == "assistant" and msg.get("tool_calls"): + kept_tcs = [] + for tc in msg["tool_calls"]: + cid = _ra().AIAgent._get_tool_call_id_static(tc) + if cid and cid in seen_assistant_ids: + dropped_dup_calls += 1 + continue + if cid: + seen_assistant_ids.add(cid) + kept_tcs.append(tc) + if len(kept_tcs) != len(msg["tool_calls"]): + msg = {**msg, "tool_calls": kept_tcs} + if not kept_tcs: + msg.pop("tool_calls", None) + content = msg.get("content") + if not content or (isinstance(content, str) and not content.strip()): + msg["content"] = "(tool call removed)" + deduped.append(msg) + elif role == "tool": + cid = (msg.get("tool_call_id") or "").strip() + if cid and cid in seen_result_ids: + dropped_dup_results += 1 + continue + if cid: + seen_result_ids.add(cid) + deduped.append(msg) + else: + deduped.append(msg) + if dropped_dup_calls or dropped_dup_results: + messages = deduped + _ra().logger.debug( + "Pre-call sanitizer: dropped %d duplicate tool_call(s) and %d " + "duplicate tool result(s)", + dropped_dup_calls, + dropped_dup_results, + ) return messages diff --git a/scripts/release.py b/scripts/release.py index 36d47c519244..69dfa9c74fd9 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -45,6 +45,7 @@ # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { + "enzo.eliott.adami@gmail.com": "enzo-adami", # PR #54594 fold (agent: drop duplicate tool results for the same live tool_call_id during repair_message_sequence; folded under the sanitize_api_messages send-path dedup for #58327) "yingwaizhiying@gmail.com": "msh01", # PR #58250 salvage (telegram: wall-clock init timeout via daemon-thread deadline + abandon the shielded initialize task on timeout so the retry ladder advances instead of hanging on attempt 1/8 under s6 supervision; #58236). Also covers PR #58276 salvage (compression: preserve a real user turn after compaction; #55677). "huanshan5195@users.noreply.github.com": "huanshan5195", # PR #57601 salvage (custom-provider: emit reasoning_effort at the live CustomProfile path so GLM-5.2/ARK/vLLM/Ollama endpoints receive it; + "max" reasoning level) "infinitycrew39@gmail.com": "infinitycrew39", # PR #56431 salvage (honor live vLLM context limits on local endpoints) diff --git a/tests/run_agent/test_agent_guardrails.py b/tests/run_agent/test_agent_guardrails.py index eb89cdda9c00..ee892948f693 100644 --- a/tests/run_agent/test_agent_guardrails.py +++ b/tests/run_agent/test_agent_guardrails.py @@ -132,6 +132,68 @@ def test_truly_orphaned_with_whitespace_still_removed(self): assert len(tool_msgs) == 1 assert tool_msgs[0]["tool_call_id"] == "c_valid" + def test_duplicate_tool_result_deduped(self): + """Two tool results sharing a tool_call_id → keep first, drop the rest. + + Strict providers (DeepSeek) reject the second with HTTP 400 + "Duplicate value for 'tool_call_id' ... in message[N]" (#58327). + """ + msgs = [ + {"role": "assistant", "tool_calls": [assistant_dict_call("dup")]}, + tool_result("dup", "first"), + tool_result("dup", "second"), + ] + out = AIAgent._sanitize_api_messages(msgs) + tool_msgs = [m for m in out if m["role"] == "tool"] + assert len(tool_msgs) == 1 + assert tool_msgs[0]["content"] == "first" + + def test_duplicate_assistant_tool_call_deduped(self): + """Same tool_call_id declared by two assistant messages → keep first.""" + msgs = [ + {"role": "assistant", "tool_calls": [assistant_dict_call("dup")]}, + tool_result("dup"), + {"role": "assistant", "tool_calls": [assistant_dict_call("dup")]}, + tool_result("dup"), + ] + out = AIAgent._sanitize_api_messages(msgs) + call_ids = [ + AIAgent._get_tool_call_id_static(tc) + for m in out + if m.get("role") == "assistant" + for tc in (m.get("tool_calls") or []) + ] + assert call_ids == ["dup"] + assert len([m for m in out if m.get("role") == "tool"]) == 1 + + def test_duplicate_call_leaving_empty_assistant_gets_placeholder(self): + """Dropping the only (duplicate) tool_call must not leave an empty turn.""" + msgs = [ + {"role": "assistant", "tool_calls": [assistant_dict_call("dup")]}, + tool_result("dup"), + {"role": "assistant", "tool_calls": [assistant_dict_call("dup")]}, + ] + out = AIAgent._sanitize_api_messages(msgs) + # Second assistant loses its duplicate call → keeps visible content, + # no dangling tool_calls, and no duplicate tool result. + assert all(m.get("tool_calls") != [] for m in out) + empty_asst = [ + m for m in out + if m.get("role") == "assistant" and not m.get("tool_calls") + ] + assert empty_asst and empty_asst[0]["content"] + + def test_distinct_ids_not_treated_as_duplicates(self): + """A valid assistant-call + matching result pair is NOT a duplicate.""" + msgs = [ + {"role": "assistant", "tool_calls": [assistant_dict_call("a")]}, + tool_result("a"), + {"role": "assistant", "tool_calls": [assistant_dict_call("b")]}, + tool_result("b"), + ] + out = AIAgent._sanitize_api_messages(msgs) + assert out == msgs + # --------------------------------------------------------------------------- # Phase 2a — _cap_delegate_task_calls diff --git a/tests/run_agent/test_message_sequence_repair.py b/tests/run_agent/test_message_sequence_repair.py index ddfe1ba4d9b2..0b0091fe55b1 100644 --- a/tests/run_agent/test_message_sequence_repair.py +++ b/tests/run_agent/test_message_sequence_repair.py @@ -259,6 +259,55 @@ def test_repair_leaves_valid_conversation_unchanged(): assert messages == original +def test_repair_preserves_parallel_tool_results(): + """One assistant turn may legitimately produce multiple consecutive tool results.""" + agent = _bare_agent() + messages = [ + {"role": "user", "content": "inspect both"}, + {"role": "assistant", "content": "", + "tool_calls": [ + {"id": "t1", "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"a"}'}}, + {"id": "t2", "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"b"}'}}, + ]}, + {"role": "tool", "tool_call_id": "t1", "content": "A"}, + {"role": "tool", "tool_call_id": "t2", "content": "B"}, + {"role": "assistant", "content": "A and B"}, + ] + original = [dict(m) for m in messages] + + repairs = AIAgent._repair_message_sequence(agent, messages) + + assert repairs == 0 + assert messages == original + + +def test_repair_drops_duplicate_tool_result_for_same_call_id(): + agent = _bare_agent() + messages = [ + {"role": "user", "content": "inspect"}, + {"role": "assistant", "content": "", + "tool_calls": [{"id": "t1", "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"a"}'}}]}, + {"role": "tool", "tool_call_id": "t1", "content": "A"}, + {"role": "tool", "tool_call_id": "t1", "content": "A retry duplicate"}, + {"role": "assistant", "content": "A"}, + ] + + repairs = AIAgent._repair_message_sequence(agent, messages) + + assert repairs == 1 + assert messages == [ + {"role": "user", "content": "inspect"}, + {"role": "assistant", "content": "", + "tool_calls": [{"id": "t1", "type": "function", + "function": {"name": "read_file", "arguments": '{"path":"a"}'}}]}, + {"role": "tool", "tool_call_id": "t1", "content": "A"}, + {"role": "assistant", "content": "A"}, + ] + + def test_repair_preserves_multimodal_user_content(): """Multimodal (list) content must NOT be merged — risks mangling attachments.""" agent = _bare_agent()