From 7b9620c2c3ab38308a12d5e491aa610d6fe3ae02 Mon Sep 17 00:00:00 2001 From: konsisumer Date: Wed, 1 Jul 2026 15:00:40 +0200 Subject: [PATCH] fix(agent): coordinate truncated tool call argument repair --- agent/agent_runtime_helpers.py | 5 +- agent/chat_completion_helpers.py | 13 +++-- agent/conversation_loop.py | 57 ++++++++++++------- agent/message_sanitization.py | 47 +++++++++++---- run_agent.py | 2 +- .../test_repair_tool_call_arguments.py | 20 ++++++- tests/run_agent/test_run_agent.py | 47 ++++++++++++--- .../test_tool_call_args_sanitizer.py | 2 +- 8 files changed, 145 insertions(+), 48 deletions(-) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 5560e4cd5c134..37f7300f7f6d0 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -249,6 +249,8 @@ def sanitize_tool_call_arguments( session_id: str = None, ) -> int: """Repair corrupted assistant tool-call argument JSON in-place.""" + from agent.message_sanitization import repair_tool_call_arguments_with_status + log = logger or logging.getLogger(__name__) if not isinstance(messages, list): return 0 @@ -308,6 +310,7 @@ def _prepend_marker(tool_msg: dict) -> None: except json.JSONDecodeError: tool_call_id = tool_call.get("id") function_name = function.get("name", "?") + repair = repair_tool_call_arguments_with_status(arguments, function_name) preview = arguments[:80] log.warning( "Corrupted tool_call arguments repaired before request " @@ -318,7 +321,7 @@ def _prepend_marker(tool_msg: dict) -> None: function_name, preview, ) - function["arguments"] = "{}" + function["arguments"] = repair.arguments existing_tool_msg = None scan_index = message_index + 1 diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 5eca94a3222dc..c4f04f7e5321c 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -32,7 +32,7 @@ from agent.model_metadata import is_local_endpoint from agent.message_sanitization import ( _sanitize_surrogates, - _repair_tool_call_arguments, + repair_tool_call_arguments_with_status, ) from tools.terminal_tool import is_persistent_env from utils import base_url_host_matches, base_url_hostname, env_float, env_int @@ -2234,12 +2234,13 @@ def _call_chat_completions(): # commas, unclosed brackets, Python None, etc. # Without repair, these hit the truncation handler # and kill the session. _repair_tool_call_arguments - # returns "{}" for unrepairable args, which is far - # better than a crashed session. - repaired = _repair_tool_call_arguments(arguments, tool_name) - if repaired != "{}": + # reports unrepairable args separately from legitimate + # empty-object normalization, so only genuine failures + # hit the truncation handler. + repair = repair_tool_call_arguments_with_status(arguments, tool_name) + if repair.success: # Successfully repaired — use the fixed args - arguments = repaired + arguments = repair.arguments else: # Unrepairable — flag for truncation handling has_truncated_tool_args = True diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 5d3dfb572efb9..f4ebcf5d60509 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -37,7 +37,7 @@ from agent.memory_manager import build_memory_context_block from agent.message_sanitization import ( close_interrupted_tool_sequence, - _repair_tool_call_arguments, + repair_tool_call_arguments_with_status, _sanitize_messages_non_ascii, _sanitize_messages_surrogates, _sanitize_structure_non_ascii, @@ -932,10 +932,10 @@ def run_conversation( ), }} except Exception: - tc["function"]["arguments"] = _repair_tool_call_arguments( + tc["function"]["arguments"] = repair_tool_call_arguments_with_status( tc["function"]["arguments"], tc["function"].get("name", "?"), - ) + ).arguments new_tcs.append(tc) am["tool_calls"] = new_tcs @@ -4346,7 +4346,14 @@ def _perform_api_call(next_api_kwargs): try: json.loads(args) except json.JSONDecodeError as e: - invalid_json_args.append((tc.function.name, str(e))) + repair = repair_tool_call_arguments_with_status( + args, + tc.function.name, + ) + if repair.success: + tc.function.arguments = repair.arguments + continue + invalid_json_args.append((tc, str(e))) if invalid_json_args: # Check if the invalid JSON is due to truncation rather @@ -4357,31 +4364,39 @@ def _perform_api_call(next_api_kwargs): # (after stripping whitespace) are cut off mid-stream. _truncated = any( not (tc.function.arguments or "").rstrip().endswith(("}", "]")) - for tc in assistant_message.tool_calls - if tc.function.name in {n for n, _ in invalid_json_args} + for tc, _ in invalid_json_args ) if _truncated: agent._vprint( f"{agent.log_prefix}⚠️ Truncated tool call arguments detected " - f"(finish_reason={finish_reason!r}) — refusing to execute.", + f"(finish_reason={finish_reason!r}) — returning tool error.", force=True, ) agent._invalid_json_retries = 0 - agent._cleanup_task_resources(effective_task_id) - agent._persist_session(messages, conversation_history) - return { - "final_response": "Response truncated due to output length limit", - "messages": messages, - "api_calls": api_call_count, - "completed": False, - "partial": True, - "error": "Response truncated due to output length limit", - } + recovery_assistant = agent._build_assistant_message(assistant_message, finish_reason) + messages.append(recovery_assistant) + invalid_call_ids = {tc.id for tc, _ in invalid_json_args} + for tc in assistant_message.tool_calls: + if tc.id in invalid_call_ids: + tool_result = ( + "Error: Tool call arguments were truncated due to the " + "output length limit and could not be repaired. Please " + "shorten the content or split it into multiple tool calls." + ) + else: + tool_result = "Skipped: other tool call in this response had truncated arguments." + messages.append({ + "role": "tool", + "name": tc.function.name, + "tool_call_id": tc.id, + "content": tool_result, + }) + continue # Track retries for invalid JSON arguments agent._invalid_json_retries += 1 - tool_name, error_msg = invalid_json_args[0] + tool_name, error_msg = invalid_json_args[0][0].function.name, invalid_json_args[0][1] agent._buffer_vprint(f"⚠️ Invalid JSON in tool call arguments for '{tool_name}': {error_msg}") if agent._invalid_json_retries < 3: @@ -4399,10 +4414,10 @@ def _perform_api_call(next_api_kwargs): messages.append(recovery_assistant) # Respond with tool error results for each tool call - invalid_names = {name for name, _ in invalid_json_args} + invalid_call_ids = {tc.id for tc, _ in invalid_json_args} for tc in assistant_message.tool_calls: - if tc.function.name in invalid_names: - err = next(e for n, e in invalid_json_args if n == tc.function.name) + if tc.id in invalid_call_ids: + err = next(e for invalid_tc, e in invalid_json_args if invalid_tc.id == tc.id) tool_result = ( f"Error: Invalid JSON arguments. {err}. " f"For tools with no required parameters, use an empty object: {{}}. " diff --git a/agent/message_sanitization.py b/agent/message_sanitization.py index 29a4b8691ae83..05be67ed144e3 100644 --- a/agent/message_sanitization.py +++ b/agent/message_sanitization.py @@ -17,10 +17,20 @@ import json import logging import re +from dataclasses import dataclass from typing import Any logger = logging.getLogger(__name__) + +@dataclass(frozen=True) +class ToolCallArgumentsRepair: + """Result of a tool-call argument repair attempt.""" + + arguments: str + repaired: bool + success: bool + # Lone surrogate code points are invalid in UTF-8 and crash json.dumps # inside the OpenAI SDK. Used by every surrogate-sanitization helper # below as well as by run_agent and the CLI for paste-from-clipboard @@ -182,26 +192,31 @@ def _escape_invalid_chars_in_json_strings(raw: str) -> str: return "".join(out) -def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: +def repair_tool_call_arguments_with_status( + raw_args: str, + tool_name: str = "?", +) -> ToolCallArgumentsRepair: """Attempt to repair malformed tool_call argument JSON. Models like GLM-5.1 via Ollama can produce truncated JSON, trailing commas, Python ``None``, etc. The API proxy rejects these with HTTP 400 - "invalid tool call arguments". This function applies common repairs; - if all fail it returns ``"{}"`` so the request succeeds (better than - crashing the session). All repairs are logged at WARNING level. + "invalid tool call arguments". This function applies common repairs and + reports whether repair actually succeeded. Empty argument strings and + Python ``None`` are successful normalisations to ``{}``; unrepairable + non-empty JSON returns ``{}`` with ``success=False`` so callers can surface + a model-visible tool error instead of executing an empty call. """ raw_stripped = raw_args.strip() if isinstance(raw_args, str) else "" # Fast-path: empty / whitespace-only -> empty object if not raw_stripped: logger.warning("Sanitized empty tool_call arguments for %s", tool_name) - return "{}" + return ToolCallArgumentsRepair("{}", True, True) # Python-literal None -> normalise to {} if raw_stripped == "None": logger.warning("Sanitized Python-None tool_call arguments for %s", tool_name) - return "{}" + return ToolCallArgumentsRepair("{}", True, True) # Repair pass 0: llama.cpp backends sometimes emit literal control # characters (tabs, newlines) inside JSON string values. json.loads @@ -216,7 +231,11 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: "Repaired unescaped control chars in tool_call arguments for %s", tool_name, ) - return reserialised + return ToolCallArgumentsRepair( + reserialised, + reserialised != raw_stripped, + True, + ) except (json.JSONDecodeError, TypeError, ValueError): pass @@ -224,6 +243,7 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: fixed = raw_stripped # 1. Strip trailing commas before } or ] fixed = re.sub(r',\s*([}\]])', r'\1', fixed) + fixed = re.sub(r',\s*$', '', fixed) # 2. Close unclosed structures open_curly = fixed.count('{') - fixed.count('}') open_bracket = fixed.count('[') - fixed.count(']') @@ -250,7 +270,7 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: "Repaired malformed tool_call arguments for %s: %s → %s", tool_name, raw_stripped[:80], fixed[:80], ) - return fixed + return ToolCallArgumentsRepair(fixed, fixed != raw_stripped, True) except json.JSONDecodeError: pass @@ -265,7 +285,7 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: "Repaired control-char-laced tool_call arguments for %s: %s → %s", tool_name, raw_stripped[:80], escaped[:80], ) - return escaped + return ToolCallArgumentsRepair(escaped, True, True) except (json.JSONDecodeError, TypeError, ValueError): pass @@ -276,7 +296,12 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: "replaced with empty object (was: %s)", tool_name, raw_stripped[:80], ) - return "{}" + return ToolCallArgumentsRepair("{}", True, False) + + +def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str: + """Backward-compatible wrapper returning only repaired argument JSON.""" + return repair_tool_call_arguments_with_status(raw_args, tool_name).arguments def close_interrupted_tool_sequence(messages: list, final_response: Any = None) -> bool: @@ -468,6 +493,8 @@ def _walk(node): "_sanitize_structure_surrogates", "_sanitize_messages_surrogates", "_escape_invalid_chars_in_json_strings", + "ToolCallArgumentsRepair", + "repair_tool_call_arguments_with_status", "_repair_tool_call_arguments", "_strip_non_ascii", "_sanitize_messages_non_ascii", diff --git a/run_agent.py b/run_agent.py index 497197f76e74d..841a36ae0d034 100644 --- a/run_agent.py +++ b/run_agent.py @@ -410,7 +410,7 @@ class AIAgent: _TOOL_CALL_ARGUMENTS_CORRUPTION_MARKER = ( "[hermes-agent: tool call arguments were corrupted in this session and " - "have been dropped to keep the conversation alive. See issue #15236.]" + "were repaired or dropped to keep the conversation alive. See issue #15236.]" ) @property diff --git a/tests/run_agent/test_repair_tool_call_arguments.py b/tests/run_agent/test_repair_tool_call_arguments.py index dcd98b5acff68..49806d32e1763 100644 --- a/tests/run_agent/test_repair_tool_call_arguments.py +++ b/tests/run_agent/test_repair_tool_call_arguments.py @@ -2,6 +2,7 @@ import json +from agent.message_sanitization import repair_tool_call_arguments_with_status from run_agent import _repair_tool_call_arguments @@ -80,6 +81,18 @@ def test_unrepairable_partial_returns_empty_object(self): # Truncated in the middle of a string key — bracket closing won't help assert _repair_tool_call_arguments('{"truncated": "val', "t") == "{}" + def test_unrepairable_partial_reports_failure(self): + result = repair_tool_call_arguments_with_status('{"truncated": "val', "t") + assert result.arguments == "{}" + assert result.repaired is True + assert result.success is False + + def test_legitimate_empty_object_reports_success(self): + result = repair_tool_call_arguments_with_status(" \n\t ", "t") + assert result.arguments == "{}" + assert result.repaired is True + assert result.success is True + # -- Valid JSON passthrough (this path is via except, but still works) -- def test_already_valid_json_passes_through(self): @@ -98,6 +111,12 @@ def test_trailing_comma_plus_unclosed_brace(self): # May or may not fully recover — verify valid JSON at minimum. json.loads(result) + def test_repairable_truncation_reports_success(self): + result = repair_tool_call_arguments_with_status('{"a": 1, "b": 2,', "t") + assert json.loads(result.arguments) == {"a": 1, "b": 2} + assert result.repaired is True + assert result.success is True + def test_real_world_glm_truncation(self): """Simulates GLM-5.1 truncating mid-argument.""" raw = '{"command": "ls -la /tmp", "timeout": 30, "background":' @@ -139,4 +158,3 @@ def test_control_chars_with_trailing_comma(self): result = _repair_tool_call_arguments(raw, "t") parsed = json.loads(result) assert "line" in parsed["msg"] - diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 98f42c68e165e..09fcc48173962 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -5186,10 +5186,10 @@ def test_stub_stall_mid_tool_call_recovers_within_3_retries(self, agent): mock_hfc.assert_called_once() assert result["final_response"] == "Done!" - def test_truncated_tool_args_detected_when_finish_reason_not_length(self, agent): + def test_unrepairable_truncated_tool_args_return_tool_error_when_finish_reason_not_length(self, agent): """When a router rewrites finish_reason from 'length' to 'tool_calls', - truncated JSON arguments should still be detected and refused rather - than wasting 3 retry attempts.""" + unrepairable truncated JSON arguments should return a tool error to + the model instead of executing an empty-argument tool call.""" self._setup_agent(agent) agent.valid_tool_names.add("write_file") bad_tc = _mock_tool_call( @@ -5200,7 +5200,7 @@ def test_truncated_tool_args_detected_when_finish_reason_not_length(self, agent) resp = _mock_response( content="", finish_reason="tool_calls", tool_calls=[bad_tc], ) - agent.client.chat.completions.create.return_value = resp + final_resp = _mock_response(content="I will split the write.", finish_reason="stop") with ( patch("run_agent.handle_function_call") as mock_handle_function_call, @@ -5208,13 +5208,46 @@ def test_truncated_tool_args_detected_when_finish_reason_not_length(self, agent) patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), ): + agent.client.chat.completions.create.side_effect = [resp, final_resp] result = agent.run_conversation("write the report") - assert result["completed"] is False - assert result["partial"] is True - assert "truncated due to output length limit" in result["error"] + assert result["final_response"] == "I will split the write." + assert any( + msg.get("role") == "tool" + and "Tool call arguments were truncated" in msg.get("content", "") + for msg in result["messages"] + ) mock_handle_function_call.assert_not_called() + def test_repairable_truncated_tool_args_continue_when_finish_reason_not_length(self, agent): + """Router-mislabeled tool-call truncation gets one repair attempt before + falling back to a tool error.""" + self._setup_agent(agent) + agent.valid_tool_names.add("write_file") + repaired_tc = _mock_tool_call( + name="write_file", + arguments='{"path":"report.md","content":"partial"', + call_id="c1", + ) + resp = _mock_response( + content="", finish_reason="tool_calls", tool_calls=[repaired_tc], + ) + final_resp = _mock_response(content="Done!", finish_reason="stop") + + with ( + patch("run_agent.handle_function_call", return_value='{"success":true}') as mock_hfc, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + agent.client.chat.completions.create.side_effect = [resp, final_resp] + result = agent.run_conversation("write the report") + + mock_hfc.assert_called_once() + args, _kwargs = mock_hfc.call_args + assert args[1] == {"path": "report.md", "content": "partial"} + assert result["final_response"] == "Done!" + def test_kanban_block_called_on_iteration_exhaustion(self, agent, monkeypatch): """Regression: kanban worker must signal the dispatcher when its iteration budget is exhausted, otherwise the task silently re-runs diff --git a/tests/run_agent/test_tool_call_args_sanitizer.py b/tests/run_agent/test_tool_call_args_sanitizer.py index 16178b9954a99..f632dbc1356fd 100644 --- a/tests/run_agent/test_tool_call_args_sanitizer.py +++ b/tests/run_agent/test_tool_call_args_sanitizer.py @@ -125,7 +125,7 @@ def test_multiple_corrupted_tool_calls_in_one_message(): assert repaired == 2 assert messages[0]["tool_calls"][0]["function"]["arguments"] == "{}" assert messages[0]["tool_calls"][1]["function"]["arguments"] == '{"path":"/tmp/bar"}' - assert messages[0]["tool_calls"][2]["function"]["arguments"] == "{}" + assert messages[0]["tool_calls"][2]["function"]["arguments"] == '{"mode":"tail"}' assert messages[1]["tool_call_id"] == "call_1" assert messages[1]["content"] == marker assert messages[2]["tool_call_id"] == "call_3"