From c30c8ef75941719b26bf8d4d23a1cd2a83d63e9b Mon Sep 17 00:00:00 2001 From: Huixin615 Date: Thu, 9 Jul 2026 16:40:10 +0800 Subject: [PATCH] fix: recover from empty tool call names (#4008) * fix: recover from empty tool call names * test: harden empty tool call recovery --------- Co-authored-by: Willem Jiang --- backend/AGENTS.md | 2 +- .../dangling_tool_call_middleware.py | 132 ++++++++++++-- .../test_dangling_tool_call_middleware.py | 161 ++++++++++++++++++ 3 files changed, 276 insertions(+), 19 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 8e6655abd..8c7d33d51 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -224,7 +224,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"]` +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 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 5fcc238fe..c0dfdc0aa 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 @@ -30,6 +30,20 @@ logger = logging.getLogger(__name__) # payloads in invalid tool-call args. Keep recovery error details short so the # synthetic ToolMessage does not echo large or malformed content back to the model. _MAX_RECOVERY_ERROR_DETAIL_LEN = 500 +_UNKNOWN_TOOL_NAME = "unknown_tool" +_EMPTY_TOOL_NAME_ERROR = "Tool call could not be executed because its name was missing or empty." + + +def _valid_tool_name(name: object) -> bool: + return isinstance(name, str) and bool(name.strip()) + + +def _normalize_tool_name(name: object) -> str: + return name.strip() if _valid_tool_name(name) else _UNKNOWN_TOOL_NAME + + +def _has_invalid_tool_name(name: object) -> bool: + return not _valid_tool_name(name) class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): @@ -54,7 +68,16 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): normalized: list[dict] = [] tool_calls = getattr(msg, "tool_calls", None) or [] - normalized.extend(list(tool_calls)) + for tool_call in tool_calls: + if not isinstance(tool_call, dict): + logger.debug("Skipping malformed non-dict tool_call in AIMessage: %r", tool_call) + continue + original_name = tool_call.get("name") + normalized_call = dict(tool_call) + normalized_call["name"] = _normalize_tool_name(original_name) + if _has_invalid_tool_name(original_name): + normalized_call["invalid_tool_name"] = True + normalized.append(normalized_call) raw_tool_calls = (getattr(msg, "additional_kwargs", None) or {}).get("tool_calls") or [] if not tool_calls: @@ -77,31 +100,36 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): parsed_args = {} args = parsed_args if isinstance(parsed_args, dict) else {} - normalized.append( - { - "id": raw_tc.get("id"), - "name": name or "unknown", - "args": args if isinstance(args, dict) else {}, - } - ) + normalized_call = { + "id": raw_tc.get("id"), + "name": _normalize_tool_name(name), + "args": args if isinstance(args, dict) else {}, + } + if _has_invalid_tool_name(name): + normalized_call["invalid_tool_name"] = True + normalized.append(normalized_call) for invalid_tc in getattr(msg, "invalid_tool_calls", None) or []: if not isinstance(invalid_tc, dict): continue - normalized.append( - { - "id": invalid_tc.get("id"), - "name": invalid_tc.get("name") or "unknown", - "args": {}, - "invalid": True, - "error": invalid_tc.get("error"), - } - ) + original_name = invalid_tc.get("name") + normalized_call = { + "id": invalid_tc.get("id"), + "name": _normalize_tool_name(original_name), + "args": {}, + "invalid": True, + "error": invalid_tc.get("error"), + } + if _has_invalid_tool_name(original_name): + normalized_call["invalid_tool_name"] = True + normalized.append(normalized_call) return normalized @staticmethod def _synthetic_tool_message_content(tool_call: dict) -> str: + if tool_call.get("invalid_tool_name"): + return f"[{_EMPTY_TOOL_NAME_ERROR} Use one of the available tool names when retrying.]" if tool_call.get("invalid"): name = tool_call.get("name") error = tool_call.get("error") @@ -125,6 +153,69 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): return "[Tool call could not be executed because its arguments were invalid.]" 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.""" + if getattr(msg, "type", None) != "ai": + return msg + + changed = False + update: dict = {} + + tool_calls = getattr(msg, "tool_calls", None) + if tool_calls: + structured_changed = False + sanitized_tool_calls = [] + for tool_call in tool_calls: + if not isinstance(tool_call, dict): + sanitized_tool_calls.append(tool_call) + continue + name = tool_call.get("name") + sanitized = dict(tool_call) + normalized_name = _normalize_tool_name(name) + if sanitized.get("name") != normalized_name: + sanitized["name"] = normalized_name + structured_changed = True + sanitized_tool_calls.append(sanitized) + if structured_changed: + update["tool_calls"] = sanitized_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): + raw_changed = False + sanitized_raw_tool_calls = [] + for raw_tool_call in raw_tool_calls: + if not isinstance(raw_tool_call, dict): + sanitized_raw_tool_calls.append(raw_tool_call) + continue + + sanitized_raw = dict(raw_tool_call) + function = sanitized_raw.get("function") + if isinstance(function, dict): + sanitized_function = dict(function) + normalized_name = _normalize_tool_name(sanitized_function.get("name")) + if sanitized_function.get("name") != normalized_name: + sanitized_function["name"] = normalized_name + sanitized_raw["function"] = sanitized_function + raw_changed = True + else: + normalized_name = _normalize_tool_name(sanitized_raw.get("name")) + if sanitized_raw.get("name") != normalized_name: + sanitized_raw["name"] = normalized_name + raw_changed = True + sanitized_raw_tool_calls.append(sanitized_raw) + + if raw_changed: + additional_kwargs["tool_calls"] = sanitized_raw_tool_calls + update["additional_kwargs"] = additional_kwargs + changed = True + + if not changed: + return msg + return msg.model_copy(update=update) + def _build_patched_messages(self, messages: list) -> list | None: """Return messages with tool results grouped after their tool-call AIMessage. @@ -151,10 +242,13 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): if isinstance(msg, ToolMessage) and msg.tool_call_id in tool_call_ids: continue - patched.append(msg) + sanitized_msg = self._sanitize_ai_message_tool_names(msg) + patched.append(sanitized_msg) if getattr(msg, "type", None) != "ai": continue + # Intentionally inspect the original message so empty names can be + # classified before the sanitized message replaces them. for tc in self._message_tool_calls(msg): tc_id = tc.get("id") if not tc_id: @@ -163,6 +257,8 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): tool_msg_queue = tool_messages_by_id.get(tc_id) existing_tool_msg = tool_msg_queue.popleft() if tool_msg_queue else None if existing_tool_msg is not None: + if tc.get("invalid_tool_name") and _has_invalid_tool_name(existing_tool_msg.name): + existing_tool_msg = existing_tool_msg.model_copy(update={"name": tc["name"]}) patched.append(existing_tool_msg) else: patched.append( diff --git a/backend/tests/test_dangling_tool_call_middleware.py b/backend/tests/test_dangling_tool_call_middleware.py index 2ccce9fa9..ed29e6a9c 100644 --- a/backend/tests/test_dangling_tool_call_middleware.py +++ b/backend/tests/test_dangling_tool_call_middleware.py @@ -5,6 +5,10 @@ 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. +from langchain_openai.chat_models.base import _convert_message_to_dict + from deerflow.agents.middlewares.dangling_tool_call_middleware import ( DanglingToolCallMiddleware, ) @@ -59,6 +63,27 @@ class TestBuildPatchedMessagesNoPatch: ] assert mw._build_patched_messages(msgs) is None + def test_valid_tool_call_names_are_sanitization_noop(self): + mw = DanglingToolCallMiddleware() + msgs = [ + AIMessage( + content="", + tool_calls=[_tc("bash", "call_1")], + additional_kwargs={ + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "bash", "arguments": "{}"}, + } + ] + }, + ), + _tool_msg("call_1", "bash"), + ] + + assert mw._build_patched_messages(msgs) is None + class TestBuildPatchedMessagesPatching: def test_single_dangling_call(self): @@ -158,6 +183,142 @@ class TestBuildPatchedMessagesPatching: assert patched[1].name == "bash" assert patched[1].status == "error" + def test_empty_structured_tool_call_name_is_sanitized(self): + mw = DanglingToolCallMiddleware() + msgs = [_ai_with_tool_calls([_tc("", "empty_name_call")])] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[0].tool_calls[0]["name"] == "unknown_tool" + payload = _convert_message_to_dict(patched[0]) + assert payload["tool_calls"][0]["function"]["name"] == "unknown_tool" + assert isinstance(patched[1], ToolMessage) + assert patched[1].tool_call_id == "empty_name_call" + assert patched[1].name == "unknown_tool" + assert patched[1].status == "error" + assert "name was missing or empty" in patched[1].content + + @pytest.mark.parametrize( + "raw_tool_call", + [ + {"id": "missing_name_call", "type": "function", "function": {"arguments": "{}"}}, + {"id": "non_string_name_call", "type": "function", "function": {"name": 42, "arguments": "{}"}}, + ], + ) + def test_malformed_raw_provider_tool_call_name_is_sanitized(self, raw_tool_call): + mw = DanglingToolCallMiddleware() + msgs = [ + AIMessage.model_construct( + content="", + type="ai", + tool_calls=[], + invalid_tool_calls=[], + additional_kwargs={"tool_calls": [raw_tool_call]}, + response_metadata={}, + ) + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[0].additional_kwargs["tool_calls"][0]["function"]["name"] == "unknown_tool" + payload = _convert_message_to_dict(patched[0]) + assert payload["tool_calls"][0]["function"]["name"] == "unknown_tool" + assert patched[1].name == "unknown_tool" + assert patched[1].status == "error" + + def test_existing_tool_result_still_sanitizes_empty_structured_tool_call_name(self): + mw = DanglingToolCallMiddleware() + msgs = [ + _ai_with_tool_calls([_tc(" ", "empty_name_call")]), + ToolMessage(content="Error: invalid tool", tool_call_id="empty_name_call", name=""), + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[0].tool_calls[0]["name"] == "unknown_tool" + payload = _convert_message_to_dict(patched[0]) + assert payload["tool_calls"][0]["function"]["name"] == "unknown_tool" + assert patched[1].tool_call_id == "empty_name_call" + assert patched[1].name == "unknown_tool" + + def test_raw_provider_tool_call_empty_function_name_is_sanitized(self): + mw = DanglingToolCallMiddleware() + msgs = [ + AIMessage( + content="", + tool_calls=[], + additional_kwargs={ + "tool_calls": [ + { + "id": "raw_empty_name_call", + "type": "function", + "function": {"name": "", "arguments": "{}"}, + } + ] + }, + ) + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + raw_tool_call = patched[0].additional_kwargs["tool_calls"][0] + assert raw_tool_call["function"]["name"] == "unknown_tool" + payload = _convert_message_to_dict(patched[0]) + assert payload["tool_calls"][0]["function"]["name"] == "unknown_tool" + assert patched[1].tool_call_id == "raw_empty_name_call" + assert patched[1].name == "unknown_tool" + assert patched[1].status == "error" + assert "name was missing or empty" in patched[1].content + + def test_valid_structured_call_with_empty_raw_provider_name_is_sanitized(self): + mw = DanglingToolCallMiddleware() + msgs = [ + AIMessage.model_construct( + content="", + type="ai", + tool_calls=[_tc("bash", "call_1")], + invalid_tool_calls=[], + additional_kwargs={ + "tool_calls": [ + { + "id": "call_1", + "type": "function", + "function": {"name": "", "arguments": "{}"}, + } + ] + }, + response_metadata={}, + ), + _tool_msg("call_1", "bash"), + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[0].tool_calls[0]["name"] == "bash" + raw_tool_call = patched[0].additional_kwargs["tool_calls"][0] + assert raw_tool_call["function"]["name"] == "unknown_tool" + payload = _convert_message_to_dict(patched[0]) + assert payload["tool_calls"][0]["function"]["name"] == "bash" + assert patched[1].tool_call_id == "call_1" + assert patched[1].name == "bash" + + def test_empty_name_invalid_tool_call_uses_name_recovery_message(self): + mw = DanglingToolCallMiddleware() + msgs = [_ai_with_invalid_tool_calls([_invalid_tc(name="", tc_id="empty_invalid_call")])] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert patched[1].tool_call_id == "empty_invalid_call" + assert patched[1].name == "unknown_tool" + assert "name was missing or empty" in patched[1].content + assert "arguments were invalid" not in patched[1].content + def test_non_adjacent_tool_result_is_moved_next_to_tool_call(self): middleware = DanglingToolCallMiddleware() msgs = [