diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index fb2408525..67629781c 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -1093,6 +1093,10 @@ def convert_messages_to_anthropic( Anthropic-proprietary — third-party endpoints cannot validate them and will reject them with HTTP 400 "Invalid signature in thinking block". """ + from agent.transports.base import sanitize_tool_calls_in_messages + + messages = sanitize_tool_calls_in_messages(messages) + system = None result = [] diff --git a/agent/transports/base.py b/agent/transports/base.py index b516967b6..a00e566fb 100644 --- a/agent/transports/base.py +++ b/agent/transports/base.py @@ -7,12 +7,121 @@ It does NOT own: client construction, streaming, credential refresh, prompt caching, interrupt handling, or retry logic. Those stay on AIAgent. """ +import copy +import json from abc import ABC, abstractmethod -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Set from agent.transports.types import NormalizedResponse +def _is_valid_json(s: str) -> bool: + """Return True if *s* is valid JSON.""" + try: + json.loads(s) + return True + except (json.JSONDecodeError, ValueError, TypeError): + return False + + +def sanitize_tool_calls_in_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + """Sanitize messages by dropping malformed tool_calls and orphan tool results. + + If a streamed assistant response is cut off mid ``input_json_delta``, the + resulting ``tool_call.arguments`` can be a truncated (non-JSON) string. + Re-sending that history to Anthropic-compatible proxies (LiteLLM, etc.) + causes a deterministic 400 that retries cannot recover from. + + This function: + + 1. Drops assistant messages whose ``tool_calls`` contain malformed + ``function.arguments`` (not valid JSON). + 2. Drops ``tool`` result messages whose ``tool_call_id`` no longer has a + matching assistant tool_call (orphaned results). + + Dropping (rather than repairing to ``{}``) is intentional — a ``{}``-arg + tool call would execute a garbage edit; dropping lets the model re-attempt + on the next turn. + + Returns a shallow copy of the list when mutations are needed; returns the + original list when no sanitization is required. + """ + # First pass: collect valid tool_call_ids and detect malformed tool_calls + valid_tool_call_ids: Set[str] = set() + needs_sanitize = False + + for msg in messages: + if not isinstance(msg, dict): + continue + if msg.get("role") != "assistant": + continue + tool_calls = msg.get("tool_calls") + if not isinstance(tool_calls, list): + continue + valid_tcs = [] + for tc in tool_calls: + if not isinstance(tc, dict): + needs_sanitize = True + continue + fn = tc.get("function", {}) if isinstance(tc.get("function"), dict) else {} + args = fn.get("arguments", "{}") + if isinstance(args, str) and not _is_valid_json(args): + needs_sanitize = True + continue + valid_tcs.append(tc) + tc_id = tc.get("id") + if isinstance(tc_id, str): + valid_tool_call_ids.add(tc_id) + if len(valid_tcs) != len(tool_calls): + needs_sanitize = True + # Also mark for sanitize if assistant message has no valid tool_calls + # but originally had some (all were malformed) + if tool_calls and not valid_tcs: + needs_sanitize = True + + if not needs_sanitize: + return messages + + # Second pass: rebuild messages, dropping malformed assistant messages + # and orphan tool results + sanitized = [] + for msg in messages: + if not isinstance(msg, dict): + sanitized.append(msg) + continue + + if msg.get("role") == "assistant": + tool_calls = msg.get("tool_calls") + if isinstance(tool_calls, list): + valid_tcs = [] + for tc in tool_calls: + if not isinstance(tc, dict): + continue + fn = tc.get("function", {}) if isinstance(tc.get("function"), dict) else {} + args = fn.get("arguments", "{}") + if isinstance(args, str) and not _is_valid_json(args): + continue + valid_tcs.append(tc) + if valid_tcs: + new_msg = copy.copy(msg) + new_msg["tool_calls"] = valid_tcs + sanitized.append(new_msg) + else: + # All tool_calls malformed — drop the whole assistant msg + pass + continue + + if msg.get("role") == "tool": + tc_id = msg.get("tool_call_id") + if isinstance(tc_id, str) and tc_id not in valid_tool_call_ids: + # Orphan tool result — drop it + continue + + sanitized.append(msg) + + return sanitized + + class ProviderTransport(ABC): """Base class for provider-specific format conversion and normalization.""" diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index 900f59dcf..b89974840 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -13,7 +13,7 @@ import copy from typing import Any, Dict, List, Optional from agent.prompt_builder import DEVELOPER_ROLE_MODELS -from agent.transports.base import ProviderTransport +from agent.transports.base import ProviderTransport, sanitize_tool_calls_in_messages from agent.transports.types import NormalizedResponse, ToolCall, Usage @@ -28,32 +28,41 @@ class ChatCompletionsTransport(ProviderTransport): return "chat_completions" def convert_messages(self, messages: List[Dict[str, Any]], **kwargs) -> List[Dict[str, Any]]: - """Messages are already in OpenAI format — sanitize Codex leaks only. + """Messages are already in OpenAI format — sanitize Codex leaks and malformed tool_calls. Strips Codex Responses API fields (``codex_reasoning_items`` on the message, ``call_id``/``response_item_id`` on tool_calls) that strict chat-completions providers reject with 400/422. + + Also drops assistant messages with malformed (non-JSON) + ``tool_calls[*].function.arguments`` and orphan ``tool`` result + messages, preventing deterministic 400 retries when a truncated + stream leaves bad JSON in history. See hermes-agent#14443. """ - needs_sanitize = False - for msg in messages: + # First: sanitize malformed tool_calls and orphan tool results + sanitized = sanitize_tool_calls_in_messages(messages) + + # Second: strip Codex-specific fields + needs_codex_sanitize = False + for msg in sanitized: if not isinstance(msg, dict): continue if "codex_reasoning_items" in msg: - needs_sanitize = True + needs_codex_sanitize = True break tool_calls = msg.get("tool_calls") if isinstance(tool_calls, list): for tc in tool_calls: if isinstance(tc, dict) and ("call_id" in tc or "response_item_id" in tc): - needs_sanitize = True + needs_codex_sanitize = True break - if needs_sanitize: + if needs_codex_sanitize: break - if not needs_sanitize: - return messages + if not needs_codex_sanitize: + return sanitized - sanitized = copy.deepcopy(messages) + sanitized = copy.deepcopy(sanitized) for msg in sanitized: if not isinstance(msg, dict): continue diff --git a/tests/agent/transports/test_chat_completions.py b/tests/agent/transports/test_chat_completions.py index b44eafd45..1b805a0c5 100644 --- a/tests/agent/transports/test_chat_completions.py +++ b/tests/agent/transports/test_chat_completions.py @@ -332,6 +332,97 @@ class TestChatCompletionsNormalize: assert nr.provider_data == {"reasoning_content": "detailed scratchpad"} +class TestChatCompletionsSanitizeMalformedToolCalls: + """Regression tests for hermes-agent#14443. + + Truncated tool_call.arguments (non-JSON) in history must be dropped + before the message leaves the process, or Anthropic-compatible proxies + (LiteLLM) reject every retry with HTTP 400. + """ + + def test_drops_assistant_with_truncated_tool_call(self, transport): + msgs = [ + {"role": "user", "content": "patch file.py"}, + {"role": "assistant", "content": None, "tool_calls": [ + {"id": "call_1", "type": "function", "function": { + "name": "patch", + # Truncated JSON — unterminated string + "arguments": '{"path": "file.py", "old_string": "', + }}, + ]}, + ] + result = transport.convert_messages(msgs) + # Assistant message with malformed tool_calls is dropped entirely + assert len(result) == 1 + assert result[0]["role"] == "user" + + def test_drops_orphan_tool_result(self, transport): + msgs = [ + {"role": "user", "content": "patch file.py"}, + {"role": "assistant", "content": None, "tool_calls": [ + {"id": "call_1", "type": "function", "function": { + "name": "patch", + # Truncated JSON + "arguments": '{"path": "file.py", "old_string": "', + }}, + ]}, + {"role": "tool", "tool_call_id": "call_1", "content": "ok"}, + ] + result = transport.convert_messages(msgs) + # Both assistant and orphan tool result are dropped + assert len(result) == 1 + assert result[0]["role"] == "user" + + def test_keeps_valid_tool_calls_and_results(self, transport): + msgs = [ + {"role": "user", "content": "patch file.py"}, + {"role": "assistant", "content": None, "tool_calls": [ + {"id": "call_1", "type": "function", "function": { + "name": "patch", + "arguments": '{"path": "file.py", "old_string": "x", "new_string": "y"}', + }}, + ]}, + {"role": "tool", "tool_call_id": "call_1", "content": "done"}, + ] + result = transport.convert_messages(msgs) + assert len(result) == 3 + assert result[1]["role"] == "assistant" + assert result[2]["role"] == "tool" + + def test_mixed_valid_and_invalid_tool_calls(self, transport): + msgs = [ + {"role": "assistant", "content": None, "tool_calls": [ + {"id": "call_good", "type": "function", "function": { + "name": "read", + "arguments": '{"path": "/etc/hosts"}', + }}, + {"id": "call_bad", "type": "function", "function": { + "name": "patch", + # Truncated + "arguments": '{"path": "file.py", "old_string": "', + }}, + ]}, + {"role": "tool", "tool_call_id": "call_good", "content": "127.0.0.1 localhost"}, + {"role": "tool", "tool_call_id": "call_bad", "content": "ok"}, + ] + result = transport.convert_messages(msgs) + # Only valid tool_call and its result remain + assert len(result) == 2 + assert result[0]["role"] == "assistant" + assert len(result[0]["tool_calls"]) == 1 + assert result[0]["tool_calls"][0]["id"] == "call_good" + assert result[1]["role"] == "tool" + assert result[1]["tool_call_id"] == "call_good" + + def test_no_sanitize_when_all_valid(self, transport): + msgs = [ + {"role": "user", "content": "hi"}, + ] + result = transport.convert_messages(msgs) + # Identity — same list returned when no sanitization needed + assert result is msgs + + class TestChatCompletionsCacheStats: def test_no_usage(self, transport):