From 3247f6175029cdd012ea96d361acf9fa7d8a2110 Mon Sep 17 00:00:00 2001 From: Huixin615 Date: Wed, 15 Jul 2026 22:06:29 +0800 Subject: [PATCH] fix(middleware): sanitize invalid tool call arguments (#4193) * fix(middleware): sanitize invalid tool call arguments * refactor(middleware): share tool argument parsing --- backend/AGENTS.md | 2 +- .../dangling_tool_call_middleware.py | 66 ++++++-- .../test_dangling_tool_call_middleware.py | 145 +++++++++++++++++- 3 files changed, 196 insertions(+), 17 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index f4a13158b..7667c4555 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -229,7 +229,7 @@ Lead-agent middlewares are assembled in strict order across three functions: the 4. **ThreadDataMiddleware** - Creates per-thread directories under the user's isolation scope (`backend/.deer-flow/users/{user_id}/threads/{thread_id}/user-data/{workspace,uploads,outputs}`); resolves `user_id` via `get_effective_user_id()` (falls back to `"default"` in no-auth mode) 5. **UploadsMiddleware** - Tracks and injects newly uploaded files into conversation (lead agent only) 6. **SandboxMiddleware** - Acquires sandbox, stores `sandbox_id` in state -7. **DanglingToolCallMiddleware** - Injects placeholder ToolMessages for AIMessage tool_calls that lack responses (e.g., user interruption), preserving raw provider tool-call payloads in `additional_kwargs["tool_calls"]`; malformed empty tool-call names are normalized to a recoverable error so strict OpenAI-compatible providers do not reject the next request +7. **DanglingToolCallMiddleware** - Injects placeholder ToolMessages for AIMessage tool_calls that lack responses (e.g., user interruption), preserving raw provider tool-call payloads in `additional_kwargs["tool_calls"]`; malformed tool-call names and arguments are sanitized in the model-bound request so strict OpenAI-compatible providers do not reject the next request 8. **LLMErrorHandlingMiddleware** - Normalizes provider/model invocation failures into recoverable assistant-facing errors before later stages run 9. **GuardrailMiddleware** - *(optional, if `guardrails.enabled`)* Pre-tool-call authorization via pluggable `GuardrailProvider`; returns an error ToolMessage on deny. Providers: built-in `AllowlistProvider` (zero deps), OAP policy providers (e.g. `aport-agent-guardrails`), or custom. See [docs/GUARDRAILS.md](docs/GUARDRAILS.md) 10. **SandboxAuditMiddleware** - Audits sandboxed shell/file operations for security logging before tool execution diff --git a/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py index 7826d7033..f83118c1e 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/dangling_tool_call_middleware.py @@ -7,6 +7,7 @@ without a matching AIMessage tool_call (e.g., after summarization/branching dropped the upstream AIMessage). Both cause strict-provider rejections. This middleware intercepts the model call to: +- Sanitize malformed tool-call names and arguments before provider serialization - Insert synthetic ToolMessages with an error indicator for each dangling AIMessage tool_call, ensuring correct message ordering - Drop orphan ToolMessages whose originating tool_call is no longer present in the @@ -50,6 +51,27 @@ def _has_invalid_tool_name(name: object) -> bool: return not _valid_tool_name(name) +def _parse_json_object(value: object) -> dict | None: + """Parse a JSON-object string, returning None for other inputs.""" + if not isinstance(value, str): + return None + try: + parsed = json.loads(value) + except ValueError: + return None + return parsed if isinstance(parsed, dict) else None + + +def _normalize_tool_arguments(arguments: object) -> str: + """Return a JSON-object string safe for OpenAI-compatible replay.""" + if isinstance(arguments, dict): + try: + return json.dumps(arguments, ensure_ascii=False, allow_nan=False) + except (TypeError, ValueError): + return "{}" + return arguments if _parse_json_object(arguments) is not None else "{}" + + class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): """Inserts placeholder ToolMessages for dangling tool calls and drops orphan ToolMessages (tool results whose originating AIMessage tool_call is gone). @@ -99,13 +121,8 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): args = raw_tc.get("args", {}) if not args and isinstance(function, dict): - raw_args = function.get("arguments") - if isinstance(raw_args, str): - try: - parsed_args = json.loads(raw_args) - except (TypeError, ValueError, json.JSONDecodeError): - parsed_args = {} - args = parsed_args if isinstance(parsed_args, dict) else {} + parsed_args = _parse_json_object(function.get("arguments")) + args = parsed_args if parsed_args is not None else {} normalized_call = { "id": raw_tc.get("id"), @@ -161,8 +178,8 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): return "[Tool call was interrupted and did not return a result.]" @staticmethod - def _sanitize_ai_message_tool_names(msg): - """Return an AIMessage with model-bound tool-call names made non-empty.""" + def _sanitize_ai_message_tool_calls(msg): + """Return an AIMessage with model-bound tool calls safe to serialize.""" if getattr(msg, "type", None) != "ai": return msg @@ -188,6 +205,28 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): update["tool_calls"] = sanitized_tool_calls changed = True + invalid_tool_calls = getattr(msg, "invalid_tool_calls", None) + if invalid_tool_calls: + invalid_changed = False + sanitized_invalid_tool_calls = [] + for invalid_tool_call in invalid_tool_calls: + if not isinstance(invalid_tool_call, dict): + sanitized_invalid_tool_calls.append(invalid_tool_call) + continue + sanitized = dict(invalid_tool_call) + normalized_name = _normalize_tool_name(sanitized.get("name")) + normalized_arguments = _normalize_tool_arguments(sanitized.get("args")) + if sanitized.get("name") != normalized_name: + sanitized["name"] = normalized_name + invalid_changed = True + if sanitized.get("args") != normalized_arguments: + sanitized["args"] = normalized_arguments + invalid_changed = True + sanitized_invalid_tool_calls.append(sanitized) + if invalid_changed: + update["invalid_tool_calls"] = sanitized_invalid_tool_calls + changed = True + additional_kwargs = dict(getattr(msg, "additional_kwargs", {}) or {}) raw_tool_calls = additional_kwargs.get("tool_calls") if isinstance(raw_tool_calls, list): @@ -203,10 +242,15 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): if isinstance(function, dict): sanitized_function = dict(function) normalized_name = _normalize_tool_name(sanitized_function.get("name")) + normalized_arguments = _normalize_tool_arguments(sanitized_function.get("arguments")) if sanitized_function.get("name") != normalized_name: sanitized_function["name"] = normalized_name - sanitized_raw["function"] = sanitized_function raw_changed = True + if sanitized_function.get("arguments") != normalized_arguments: + sanitized_function["arguments"] = normalized_arguments + raw_changed = True + if sanitized_function != function: + sanitized_raw["function"] = sanitized_function else: normalized_name = _normalize_tool_name(sanitized_raw.get("name")) if sanitized_raw.get("name") != normalized_name: @@ -258,7 +302,7 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): drop_count += 1 continue - sanitized_msg = self._sanitize_ai_message_tool_names(msg) + sanitized_msg = self._sanitize_ai_message_tool_calls(msg) patched.append(sanitized_msg) if getattr(msg, "type", None) != "ai": continue diff --git a/backend/tests/test_dangling_tool_call_middleware.py b/backend/tests/test_dangling_tool_call_middleware.py index 005e19639..91dcde72e 100644 --- a/backend/tests/test_dangling_tool_call_middleware.py +++ b/backend/tests/test_dangling_tool_call_middleware.py @@ -1,12 +1,13 @@ """Tests for DanglingToolCallMiddleware.""" +import json from unittest.mock import AsyncMock, MagicMock import pytest from langchain_core.messages import AIMessage, HumanMessage, ToolMessage # Intentional private import: these tests lock the OpenAI serialization boundary -# that strict providers reject when assistant tool-call names are empty. +# that strict providers reject when assistant tool-call names or arguments are malformed. from langchain_openai.chat_models.base import _convert_message_to_dict from deerflow.agents.middlewares.dangling_tool_call_middleware import ( @@ -319,6 +320,132 @@ class TestBuildPatchedMessagesPatching: assert "name was missing or empty" in patched[1].content assert "arguments were invalid" not in patched[1].content + def test_issue_4172_mixed_tool_calls_serialize_with_valid_names_and_arguments(self): + mw = DanglingToolCallMiddleware() + invalid_args = '{"description": "读取CSV数据文件前部内容", "path": "/mnt/user-data/uploads/test2.csv"}}' + ai_message = AIMessage( + content="", + tool_calls=[ + _tc("read_file", "valid_call"), + _tc("", "empty_name_call"), + ], + invalid_tool_calls=[ + { + "type": "invalid_tool_call", + "id": "invalid_args_call", + "name": "read_file", + "args": invalid_args, + "error": None, + } + ], + ) + msgs = [ + ai_message, + _tool_msg("valid_call", "read_file"), + ToolMessage( + content="Error: invalid tool", + tool_call_id="empty_name_call", + name="", + status="error", + ), + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + payload = _convert_message_to_dict(patched[0]) + assert all(call["function"]["name"] for call in payload["tool_calls"]) + assert [json.loads(call["function"]["arguments"]) for call in payload["tool_calls"]] == [ + {}, + {}, + {}, + ] + assert ai_message.invalid_tool_calls[0]["args"] == invalid_args + tool_messages = [message for message in patched if isinstance(message, ToolMessage)] + assert [message.tool_call_id for message in tool_messages] == [ + "valid_call", + "empty_name_call", + "invalid_args_call", + ] + assert tool_messages[1].name == "unknown_tool" + assert tool_messages[2].status == "error" + + def test_empty_name_and_malformed_arguments_in_invalid_tool_call_are_sanitized(self): + mw = DanglingToolCallMiddleware() + msgs = [ + _ai_with_invalid_tool_calls( + [ + _invalid_tc( + name="", + tc_id="empty_invalid_call", + error="Failed to parse tool arguments: malformed JSON", + ) + ] + ) + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + payload_call = _convert_message_to_dict(patched[0])["tool_calls"][0] + assert payload_call["function"]["name"] == "unknown_tool" + assert json.loads(payload_call["function"]["arguments"]) == {} + + @pytest.mark.parametrize( + "arguments", + [ + '{"path":"/tmp/data.csv"}}', + None, + '["not", "an", "object"]', + {"path": "/tmp/data.csv"}, + ], + ) + def test_raw_provider_tool_call_arguments_are_sanitized(self, arguments): + mw = DanglingToolCallMiddleware() + msgs = [ + AIMessage.model_construct( + content="", + type="ai", + tool_calls=[], + invalid_tool_calls=[], + additional_kwargs={ + "tool_calls": [ + { + "id": "raw_invalid_args_call", + "type": "function", + "function": {"name": "read_file", "arguments": arguments}, + } + ] + }, + response_metadata={}, + ) + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + raw_arguments = patched[0].additional_kwargs["tool_calls"][0]["function"]["arguments"] + assert json.loads(raw_arguments) == (arguments if isinstance(arguments, dict) else {}) + + def test_valid_invalid_tool_call_arguments_are_sanitization_noop(self): + mw = DanglingToolCallMiddleware() + msgs = [ + _ai_with_invalid_tool_calls( + [ + { + "type": "invalid_tool_call", + "id": "valid_args_call", + "name": "read_file", + "args": '{"path": "/tmp/data.csv"}', + "error": "schema validation failed", + } + ] + ), + _tool_msg("valid_args_call", "read_file"), + ] + + assert mw._build_patched_messages(msgs) is None + def test_non_adjacent_tool_result_is_moved_next_to_tool_call(self): middleware = DanglingToolCallMiddleware() msgs = [ @@ -583,13 +710,21 @@ class TestBuildPatchedMessagesPatching: assert len(tool_msgs) == 2 assert {tm.tool_call_id for tm in tool_msgs} == {"call_1", "write_file:36"} - def test_invalid_tool_call_already_responded_is_not_patched(self): + def test_invalid_tool_call_already_responded_is_sanitized_without_placeholder(self): mw = DanglingToolCallMiddleware() + ai_message = _ai_with_invalid_tool_calls([_invalid_tc()]) + tool_message = _tool_msg("write_file:36", "write_file") msgs = [ - _ai_with_invalid_tool_calls([_invalid_tc()]), - _tool_msg("write_file:36", "write_file"), + ai_message, + tool_message, ] - assert mw._build_patched_messages(msgs) is None + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[1] is tool_message + assert json.loads(_convert_message_to_dict(patched[0])["tool_calls"][0]["function"]["arguments"]) == {} + assert ai_message.invalid_tool_calls[0]["args"] == _invalid_tc()["args"] class TestWrapModelCall: