From 9c77046d101270b02c4acd6c2cd3bd21702cc538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:43:47 +0800 Subject: [PATCH] fix(middleware): drop orphan ToolMessages so strict providers don't 400 (#4080) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(middleware): drop orphan ToolMessages with no matching AIMessage tool_call The rebuild loop only skipped ToolMessages whose tool_call_id matched a known AIMessage tool_call (to be re-emitted after it). An orphan ToolMessage whose tool_call_id has no matching AIMessage tool_calls fell through and was kept, leaving a dangling tool result that strict providers reject. Drop orphan ToolMessages as well, logging at debug. * fix(dangling): demote orphan-drop logs, add tool_call_id=None test - Update module/class docstrings to mention orphan ToolMessage handling - Accumulate orphan drop_count and emit a single logger.warning instead of per-message logger.debug calls - Simplify early-return logic: return None only when no patching AND no orphans were dropped - Add test_tool_call_id_none_orphan_is_dropped — a ToolMessage with tool_call_id=None is always an orphan and must be dropped Closes #4080 Co-Authored-By: Claude * fix(test): use model_construct for None tool_call_id test to bypass pydantic validation ToolMessage content='ghost' tool_call_id=None fails pydantic validation at construction. Use model_construct to simulate a corrupt/edge-case payload without tripping the string-only guard. Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../dangling_tool_call_middleware.py | 47 ++++++++++----- .../test_dangling_tool_call_middleware.py | 60 +++++++++++++++++-- 2 files changed, 89 insertions(+), 18 deletions(-) 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 c0dfdc0aa..7826d7033 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 @@ -1,12 +1,16 @@ -"""Middleware to fix dangling tool calls in message history. +"""Middleware to fix dangling tool calls and orphan tool results in message history. A dangling tool call occurs when an AIMessage contains tool_calls but there are no corresponding ToolMessages in the history (e.g., due to user interruption or -request cancellation). This causes LLM errors due to incomplete message format. +request cancellation). An orphan ToolMessage occurs when a tool result exists +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 detect and patch such gaps by -inserting synthetic ToolMessages with an error indicator immediately after the -AIMessage that made the tool calls, ensuring correct message ordering. +This middleware intercepts the model call to: +- 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 + request, preventing strict OpenAI-compatible backends from returning HTTP 400 Note: Uses wrap_model_call instead of before_model to ensure patches are inserted at the correct positions (immediately after each dangling AIMessage), not appended @@ -47,11 +51,14 @@ def _has_invalid_tool_name(name: object) -> bool: class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): - """Inserts placeholder ToolMessages for dangling tool calls before model invocation. + """Inserts placeholder ToolMessages for dangling tool calls and drops orphan + ToolMessages (tool results whose originating AIMessage tool_call is gone). - Scans the message history for AIMessages whose tool_calls lack corresponding - ToolMessages, and injects synthetic error responses immediately after the - offending AIMessage so the LLM receives a well-formed conversation. + Scans the message history for: + - AIMessages whose tool_calls lack corresponding ToolMessages, and injects + synthetic error responses immediately after the offending AIMessage + - ToolMessages with no matching AIMessage tool_call (orphans), and drops + them so strict OpenAI-compatible backends do not reject the request """ @staticmethod @@ -238,8 +245,17 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): patched: list = [] patch_count = 0 + drop_count = 0 for msg in messages: - if isinstance(msg, ToolMessage) and msg.tool_call_id in tool_call_ids: + if isinstance(msg, ToolMessage): + if msg.tool_call_id in tool_call_ids: + continue # Will be re-emitted after its AIMessage + # Orphan: ToolMessage whose originating AIMessage tool_call is + # no longer in the request (e.g. removed by summarization). + # Drop it silently from the model request so strict providers + # do not reject it with HTTP 400. Persisted state is untouched; + # this only affects the single model call. + drop_count += 1 continue sanitized_msg = self._sanitize_ai_message_tool_names(msg) @@ -271,11 +287,14 @@ class DanglingToolCallMiddleware(AgentMiddleware[AgentState]): ) patch_count += 1 - if patched == messages: + if patched == messages and not drop_count: return None - - if patch_count: - logger.warning(f"Injecting {patch_count} placeholder ToolMessage(s) for dangling tool calls") + if drop_count or patch_count: + logger.warning( + "DanglingToolCallMiddleware: %d orphan(s) dropped, %d placeholder(s) injected", + drop_count, + patch_count, + ) return patched @override diff --git a/backend/tests/test_dangling_tool_call_middleware.py b/backend/tests/test_dangling_tool_call_middleware.py index ed29e6a9c..005e19639 100644 --- a/backend/tests/test_dangling_tool_call_middleware.py +++ b/backend/tests/test_dangling_tool_call_middleware.py @@ -464,7 +464,14 @@ class TestBuildPatchedMessagesPatching: assert isinstance(patched[4], ToolMessage) assert patched[4].tool_call_id == "call_2" - def test_orphan_tool_message_is_preserved_during_grouping(self): + def test_orphan_tool_message_is_dropped_during_grouping(self): + """An orphan ToolMessage — one whose tool_call_id has no matching AIMessage + tool_call — is dropped from the patched output. + + Behavior intentionally changed: strict OpenAI-compatible providers reject a + ToolMessage that does not follow an assistant tool_call, so an orphan left + over from interruption/compaction must not be forwarded. + """ mw = DanglingToolCallMiddleware() orphan = _tool_msg("orphan_call", "orphan") msgs = [ @@ -477,12 +484,57 @@ class TestBuildPatchedMessagesPatching: patched = mw._build_patched_messages(msgs) assert patched is not None + # The orphan is dropped; call_1's result is regrouped right after its AIMessage. + assert isinstance(patched[0], AIMessage) + assert isinstance(patched[1], ToolMessage) + assert patched[1].tool_call_id == "call_1" + assert isinstance(patched[2], HumanMessage) + assert orphan not in patched + assert patched.count(orphan) == 0 + assert len(patched) == 3 + + def test_leading_orphan_tool_message_is_dropped(self): + """A ToolMessage that leads the transcript with no preceding tool_call is an + orphan and must be dropped (leaving a valid grouped transcript).""" + mw = DanglingToolCallMiddleware() + leading_orphan = _tool_msg("stale_call", "stale") + msgs = [ + leading_orphan, + _ai_with_tool_calls([_tc("bash", "call_1")]), + _tool_msg("call_1", "bash"), + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert leading_orphan not in patched + assert isinstance(patched[0], AIMessage) + assert isinstance(patched[1], ToolMessage) + assert patched[1].tool_call_id == "call_1" + assert len(patched) == 2 + + def test_tool_call_id_none_orphan_is_dropped(self): + """A ToolMessage whose tool_call_id is None is always an orphan — + no valid tool call uses ``None`` as its id — and must be dropped.""" + mw = DanglingToolCallMiddleware() + # Use model_construct to bypass pydantic validation (ToolMessage requires + # a string tool_call_id at construction, but a corrupt serialized payload + # or edge-case provider could still produce None at runtime). + none_id_orphan = ToolMessage.model_construct(content="ghost", tool_call_id=None) + msgs = [ + _ai_with_tool_calls([_tc("bash", "call_1")]), + none_id_orphan, + _tool_msg("call_1", "bash"), + ] + + patched = mw._build_patched_messages(msgs) + + assert patched is not None + assert none_id_orphan not in patched + assert len(patched) == 2 assert isinstance(patched[0], AIMessage) assert isinstance(patched[1], ToolMessage) assert patched[1].tool_call_id == "call_1" - assert patched[2] is orphan - assert isinstance(patched[3], HumanMessage) - assert patched.count(orphan) == 1 def test_invalid_tool_call_is_patched(self): mw = DanglingToolCallMiddleware()