diff --git a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py index 235f6461f..cfe20b513 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py @@ -548,6 +548,7 @@ data — do NOT reveal it. - ❌ DO NOT skip clarification for "efficiency" - accuracy matters more than speed - ❌ DO NOT make assumptions when information is missing - ALWAYS ask - ❌ DO NOT proceed with guesses - STOP and call ask_clarification first +- ❌ DO NOT call any other tool in the same turn as ask_clarification — sibling calls are dropped - ✅ Analyze the request in thinking → Identify unclear aspects → Ask BEFORE any action - ✅ If you identify the need for clarification in your thinking, you MUST call the tool IMMEDIATELY - ✅ After calling ask_clarification, execution will be interrupted automatically diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index df39c0d06..5f60a4eb5 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -92,4 +92,4 @@ Before changing a later authorization phase, read the [authorization RFC](../../ 32. **TerminalResponseMiddleware** - When a provider returns an empty terminal `AIMessage` after tool execution, injects a hidden recovery prompt and retries the model once; a second empty response is replaced in checkpoint state by a visible error fallback marked for the run worker, so the run finishes as an error instead of a silent success 33. **ModelLengthFinishReasonMiddleware** - Records `stop_reason=model_length_capped` when provider-specific length detectors match a terminal `AIMessage` without tool-call intent (`finish_reason=length` / `MAX_TOKENS`, or `stop_reason=max_tokens`), preserving the original assistant content and never reparsing textual tool-call-like envelopes 34. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Suppresses tool execution when the provider safety-terminated the response (e.g. `finish_reason=content_filter`); registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first -35. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). Payloads are versioned: legacy modes (`free_text` / `choice_with_other`) keep `version: 1` unchanged, while the v2 `form` mode (from `fields`) carries `version: 2` so older frontends reject the payload and degrade to the plain-text fallback. Field normalization is deterministic and lives in the middleware, not the tool schema — the middleware short-circuits before tool execution, so tool-arg typing alone provides no runtime validation. Validation is atomic: any structurally broken entry (non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member like `__proto__`/`constructor`, exceeding the caps of 16 fields / 24 options per field / 200 chars per text, or the whole normalized definition exceeding `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8 — the per-item caps alone admit forms whose IM text fallback would blow channel delivery limits and truncate away trailing fields) degrades the whole form to the legacy option/free-text modes, so a card can never render "complete" while silently missing a business field; benign issues keep local degradation (unknown types — including unhashable JSON like `type: []`, which must never raise from the membership probe — and option-less selects become `text`), and options are trimmed/deduped with blanks dropped (both form-level and top-level) because the frontend parser rejects blank option labels. Model-produced XML-to-dict option payloads are recursively flattened from dict/list containers in source order, scalar string/number leaves are retained, and residual XML tags are removed before the same trimming and deduplication. Checkbox fields are booleans that default to an explicit "no"; `required` on a checkbox means must-agree/consent semantics. The response protocol is deliberately unchanged (v1 `text`/`option` only): form cards submit a readable text summary as `response_kind: "text"`, so journal persistence and answered-card recovery need no new allowlist entries. Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context. +35. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). In `after_model` it drops sibling tool calls from the same turn so they cannot execute before the user answers; a malformed `ask_clarification` that LangChain parked on `invalid_tool_calls` is the same stop signal (the valid sibling would otherwise still run). `disable_clarification` runs keep the siblings. Payloads are versioned: legacy modes (`free_text` / `choice_with_other`) keep `version: 1` unchanged, while the v2 `form` mode (from `fields`) carries `version: 2` so older frontends reject the payload and degrade to the plain-text fallback. Field normalization is deterministic and lives in the middleware, not the tool schema — the middleware short-circuits before tool execution, so tool-arg typing alone provides no runtime validation. Validation is atomic: any structurally broken entry (non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member like `__proto__`/`constructor`, exceeding the caps of 16 fields / 24 options per field / 200 chars per text, or the whole normalized definition exceeding `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8 — the per-item caps alone admit forms whose IM text fallback would blow channel delivery limits and truncate away trailing fields) degrades the whole form to the legacy option/free-text modes, so a card can never render "complete" while silently missing a business field; benign issues keep local degradation (unknown types — including unhashable JSON like `type: []`, which must never raise from the membership probe — and option-less selects become `text`), and options are trimmed/deduped with blanks dropped (both form-level and top-level) because the frontend parser rejects blank option labels. Model-produced XML-to-dict option payloads are recursively flattened from dict/list containers in source order, scalar string/number leaves are retained, and residual XML tags are removed before the same trimming and deduplication. Checkbox fields are booleans that default to an explicit "no"; `required` on a checkbox means must-agree/consent semantics. The response protocol is deliberately unchanged (v1 `text`/`option` only): form cards submit a readable text summary as `response_kind: "text"`, so journal persistence and answered-card recovery need no new allowlist entries. Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context. diff --git a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py index 6e89f12cc..4bac91f79 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/clarification_middleware.py @@ -9,13 +9,18 @@ from typing import Any, override from langchain.agents import AgentState from langchain.agents.middleware import AgentMiddleware -from langchain_core.messages import ToolMessage +from langchain_core.messages import AIMessage, ToolMessage from langgraph.graph import END from langgraph.prebuilt.tool_node import ToolCallRequest +from langgraph.runtime import Runtime from langgraph.types import Command +from deerflow.agents.middlewares.tool_call_metadata import clone_ai_message_with_tool_calls + logger = logging.getLogger(__name__) +ASK_CLARIFICATION_TOOL_NAME = "ask_clarification" + # Whitelisted form field types; anything else degrades to "text" so a bad # model-provided type can never produce an unrenderable card. FORM_FIELD_TYPES = frozenset({"text", "textarea", "number", "select", "multi_select", "checkbox", "date"}) @@ -66,15 +71,48 @@ class ClarificationMiddlewareState(AgentState): pass +def _filter_content_tool_use(content: Any, kept_ids: set[str], kept_names: set[str]) -> Any: + """Drop provider tool-use blocks that were stripped from ``tool_calls``. + + Anthropic ``tool_use`` blocks carry an ``id`` that matches ``tool_calls``. + Gemini-style ``function_call`` blocks often have no ``id`` (langchain + synthesizes ids onto ``tool_calls`` only), so those are matched by ``name``. + """ + if not isinstance(content, list): + return content + filtered: list[Any] = [] + for block in content: + if isinstance(block, dict) and block.get("type") in {"tool_use", "function_call"}: + block_id = block.get("id") + if isinstance(block_id, str) and block_id: + if block_id not in kept_ids: + continue + elif block.get("type") == "function_call": + name = block.get("name") + if not isinstance(name, str) or name not in kept_names: + continue + filtered.append(block) + return filtered + + class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): """Intercepts clarification tool calls and interrupts execution to present questions to the user. When the model calls the `ask_clarification` tool, this middleware: - 1. Intercepts the tool call before execution - 2. Extracts the clarification question and metadata - 3. Formats a user-friendly message - 4. Returns a Command that interrupts execution and presents the question - 5. Waits for user response before continuing + 1. Drops any sibling tool calls from the same AIMessage (``after_model``) + so they cannot execute before the user answers. langchain's + ``return_direct`` router inspects all client-side tool calls of the + last AIMessage and routes to END only when every one is + ``return_direct``. A mixed ``[ask_clarification, bash]`` batch would + both run the siblings *and* loop back to the model. Malformed + ``ask_clarification`` arguments land in ``invalid_tool_calls`` while a + valid sibling stays in ``tool_calls``; that still counts as a stop + signal and the siblings are dropped. + 2. Intercepts the remaining ``ask_clarification`` call before execution + 3. Extracts the clarification question and metadata + 4. Formats a user-friendly message + 5. Returns a Command that interrupts execution and presents the question + 6. Waits for user response before continuing This replaces the tool-based approach where clarification continued the conversation flow. """ @@ -360,22 +398,83 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): return "\n".join(message_parts) - def _is_disabled(self, request: ToolCallRequest) -> bool: + def _clarification_disabled(self, runtime: Any) -> bool: """Whether clarifications are suppressed for this run. Non-interactive channels (e.g. GitHub webhooks) set ``disable_clarification`` in the run context because a clarification would dead-end the run — the human only "replies" via a later webhook delivery, by which point the agent's turn is long over. - When set, we don't interrupt; we return a ToolMessage nudging the - agent to proceed with its best judgment instead. """ - runtime = getattr(request, "runtime", None) context = getattr(runtime, "context", None) if not context: return False return bool(context.get("disable_clarification")) + def _is_disabled(self, request: ToolCallRequest) -> bool: + """Whether clarifications are suppressed for this tool-call request.""" + return self._clarification_disabled(getattr(request, "runtime", None)) + + def _drop_parallel_non_clarification_tools(self, state: AgentState, runtime: Runtime) -> dict | None: + """Keep only ``ask_clarification`` when it was emitted alongside other tools. + + Providers routinely batch tool calls. If ``ask_clarification`` shares a + turn with ``bash`` / ``write_file`` / ..., those siblings execute before + the user answers, and langchain's ``return_direct`` check (every + client-side tool call of the last AIMessage must be ``return_direct``) + routes back to the model. Rewrite the AIMessage so the tools node + never sees the siblings. + + LangChain parses each provider call independently, so a malformed + ``ask_clarification`` is stored on ``invalid_tool_calls`` while a + valid sibling remains executable on ``tool_calls``. Treat that + malformed call as the same stop signal: drop the siblings so the + tools node cannot run them. With no remaining ``tool_calls``, + ``create_agent`` routes to END. + + ``disable_clarification`` skips this rewrite: those runs must keep the + sibling actions, because the clarification itself is turned into a + "proceed" ToolMessage instead of an interrupt. + """ + if self._clarification_disabled(runtime): + return None + + messages = state.get("messages", []) + if not messages: + return None + last = messages[-1] + if not isinstance(last, AIMessage): + return None + + tool_calls = list(last.tool_calls or []) + invalid_tool_calls = [tc for tc in (getattr(last, "invalid_tool_calls", None) or []) if isinstance(tc, dict)] + clarification_calls = [tc for tc in tool_calls if tc.get("name") == ASK_CLARIFICATION_TOOL_NAME] + invalid_clarification_calls = [tc for tc in invalid_tool_calls if tc.get("name") == ASK_CLARIFICATION_TOOL_NAME] + if not clarification_calls and not invalid_clarification_calls: + return None + + sibling_calls = [tc for tc in tool_calls if tc.get("name") != ASK_CLARIFICATION_TOOL_NAME] + if not sibling_calls: + return None + + dropped_names = [str(tc.get("name") or "unknown") for tc in sibling_calls] + logger.warning( + "ask_clarification was emitted with %d sibling tool call(s); dropping %s so the turn can interrupt", + len(dropped_names), + dropped_names, + ) + + kept_for_content = clarification_calls + invalid_clarification_calls + kept_ids = {tc["id"] for tc in kept_for_content if isinstance(tc.get("id"), str) and tc["id"]} + kept_names = {str(tc["name"]) for tc in kept_for_content if isinstance(tc.get("name"), str) and tc["name"]} + new_content = _filter_content_tool_use(last.content, kept_ids, kept_names) + patched = clone_ai_message_with_tool_calls( + last, + clarification_calls, + content=new_content if new_content is not last.content else None, + ) + return {"messages": [patched]} + def _handle_disabled_clarification(self, request: ToolCallRequest) -> ToolMessage: """Suppress a clarification and tell the agent to proceed. @@ -395,7 +494,7 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): "you made in your final response." ), tool_call_id=tool_call_id, - name="ask_clarification", + name=ASK_CLARIFICATION_TOOL_NAME, ) def _handle_clarification(self, request: ToolCallRequest) -> Command: @@ -433,7 +532,7 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): id=request_id, content=formatted_message, tool_call_id=tool_call_id, - name="ask_clarification", + name=ASK_CLARIFICATION_TOOL_NAME, artifact={"human_input": human_input_payload}, ) @@ -463,7 +562,7 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): Command that interrupts execution with the formatted clarification message """ # Check if this is an ask_clarification tool call - if request.tool_call.get("name") != "ask_clarification": + if request.tool_call.get("name") != ASK_CLARIFICATION_TOOL_NAME: # Not a clarification call, execute normally return handler(request) @@ -488,7 +587,7 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): Command that interrupts execution with the formatted clarification message """ # Check if this is an ask_clarification tool call - if request.tool_call.get("name") != "ask_clarification": + if request.tool_call.get("name") != ASK_CLARIFICATION_TOOL_NAME: # Not a clarification call, execute normally return await handler(request) @@ -496,3 +595,11 @@ class ClarificationMiddleware(AgentMiddleware[ClarificationMiddlewareState]): return self._handle_disabled_clarification(request) return self._handle_clarification(request) + + @override + def after_model(self, state: AgentState, runtime: Runtime) -> dict | None: + return self._drop_parallel_non_clarification_tools(state, runtime) + + @override + async def aafter_model(self, state: AgentState, runtime: Runtime) -> dict | None: + return self._drop_parallel_non_clarification_tools(state, runtime) diff --git a/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py b/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py index 8bf0de9b4..7cc7d4ef2 100644 --- a/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py +++ b/backend/packages/harness/deerflow/tools/builtins/clarification_tool.py @@ -70,6 +70,8 @@ def ask_clarification_tool( - If a skill provides a predefined field template, pass it through `fields` unchanged instead of redesigning it - After calling this tool, execution will be interrupted automatically + - Do not call any other tool in the same turn as this one; sibling tool + calls are dropped so they cannot run before the user answers Args: question: The clarification question to ask the user. Be specific and clear. diff --git a/backend/tests/test_clarification_drop_siblings_graph_integration.py b/backend/tests/test_clarification_drop_siblings_graph_integration.py new file mode 100644 index 000000000..71ead031e --- /dev/null +++ b/backend/tests/test_clarification_drop_siblings_graph_integration.py @@ -0,0 +1,186 @@ +"""Factory-level wiring test for ClarificationMiddleware sibling-tool dropping. + +Unit tests in ``test_clarification_middleware.py`` call ``after_model`` +directly. This file builds a real ``langchain.agents.create_agent`` graph +so a langchain hook-dispatch regression or a same-id ``add_messages`` +replacement failure would reintroduce #4906 instead of staying green. +A second graph path covers malformed ``ask_clarification`` parked on +``invalid_tool_calls`` beside a valid sibling. +""" + +from __future__ import annotations + +from langchain.agents import create_agent +from langchain_core.language_models import BaseChatModel +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage +from langchain_core.outputs import ChatGeneration, ChatResult +from langchain_core.tools import tool + +from deerflow.agents.middlewares.clarification_middleware import ClarificationMiddleware +from deerflow.tools.builtins.clarification_tool import ask_clarification_tool + +_BASH_INVOCATIONS: list[str] = [] +_MIXED_MESSAGE_ID = "ai-clarification-with-sibling" +_INVALID_MIXED_MESSAGE_ID = "ai-invalid-clarification-with-sibling" + + +@tool +def bash(command: str) -> str: + """Pretend to run a shell command. Records the call for assertion.""" + _BASH_INVOCATIONS.append(command) + return f"ran: {command}" + + +class _MixedBatchModel(BaseChatModel): + """First call emits ``ask_clarification`` plus ``bash``; a second call is a wiring failure.""" + + call_count: int = 0 + + @property + def _llm_type(self) -> str: + return "fake-clarification-mixed-batch" + + def bind_tools(self, tools, **kwargs): + return self + + def _generate(self, messages, stop=None, run_manager=None, **kwargs): + self.call_count += 1 + if self.call_count == 1: + message = AIMessage( + id=_MIXED_MESSAGE_ID, + content="", + tool_calls=[ + { + "id": "call_clarify_1", + "name": "ask_clarification", + "args": { + "question": "Which directory should I use?", + "clarification_type": "missing_info", + }, + }, + { + "id": "call_bash_1", + "name": "bash", + "args": {"command": "rm -rf /tmp/foo"}, + }, + ], + ) + else: + message = AIMessage(content="should-not-happen") + return ChatResult(generations=[ChatGeneration(message=message)]) + + async def _agenerate(self, messages, stop=None, run_manager=None, **kwargs): + return self._generate(messages, stop=stop, run_manager=run_manager, **kwargs) + + +class _InvalidClarificationMixedBatchModel(BaseChatModel): + """First call emits a malformed ask_clarification plus a valid bash sibling.""" + + call_count: int = 0 + + @property + def _llm_type(self) -> str: + return "fake-invalid-clarification-mixed-batch" + + def bind_tools(self, tools, **kwargs): + return self + + def _generate(self, messages, stop=None, run_manager=None, **kwargs): + self.call_count += 1 + if self.call_count == 1: + message = AIMessage( + id=_INVALID_MIXED_MESSAGE_ID, + content="", + tool_calls=[ + { + "id": "call_bash_1", + "name": "bash", + "args": {"command": "rm -rf /tmp/foo"}, + }, + ], + invalid_tool_calls=[ + { + "id": "call_clarify_1", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + }, + ], + ) + else: + message = AIMessage(content="should-not-happen") + return ChatResult(generations=[ChatGeneration(message=message)]) + + async def _agenerate(self, messages, stop=None, run_manager=None, **kwargs): + return self._generate(messages, stop=stop, run_manager=run_manager, **kwargs) + + +def test_mixed_clarification_batch_does_not_execute_siblings_or_loop(): + """after_model must be dispatched and replace the AIMessage in place. + + (a) the bash handler never runs + (b) the run ends without a second model call + """ + _BASH_INVOCATIONS.clear() + model = _MixedBatchModel() + agent = create_agent( + model=model, + tools=[ask_clarification_tool, bash], + middleware=[ClarificationMiddleware()], + ) + + result = agent.invoke( + {"messages": [HumanMessage(content="clean up the temp dir")]}, + config={"recursion_limit": 8}, + ) + + assert _BASH_INVOCATIONS == [], f"bash ran before the user answered: {_BASH_INVOCATIONS}" + assert model.call_count == 1 + + ai_messages = [message for message in result["messages"] if isinstance(message, AIMessage)] + assert len(ai_messages) == 1 + patched = ai_messages[0] + assert patched.id == _MIXED_MESSAGE_ID + assert [tc["name"] for tc in patched.tool_calls] == ["ask_clarification"] + + tool_messages = [message for message in result["messages"] if isinstance(message, ToolMessage)] + assert [message.name for message in tool_messages] == ["ask_clarification"] + assert tool_messages[0].tool_call_id == "call_clarify_1" + + +def test_mixed_invalid_clarification_batch_does_not_execute_siblings_or_loop(): + """Malformed ask_clarification is still a stop signal for executable siblings. + + LangChain parks the broken call on ``invalid_tool_calls`` while the valid + bash sibling stays on ``tool_calls``. after_model must still rewrite the + AIMessage so: + (a) the bash handler never runs + (b) the run ends without a second model call + """ + _BASH_INVOCATIONS.clear() + model = _InvalidClarificationMixedBatchModel() + agent = create_agent( + model=model, + tools=[ask_clarification_tool, bash], + middleware=[ClarificationMiddleware()], + ) + + result = agent.invoke( + {"messages": [HumanMessage(content="clean up the temp dir")]}, + config={"recursion_limit": 8}, + ) + + assert _BASH_INVOCATIONS == [], f"bash ran before the user answered: {_BASH_INVOCATIONS}" + assert model.call_count == 1 + + ai_messages = [message for message in result["messages"] if isinstance(message, AIMessage)] + assert len(ai_messages) == 1 + patched = ai_messages[0] + assert patched.id == _INVALID_MIXED_MESSAGE_ID + assert patched.tool_calls == [] + assert [tc["name"] for tc in patched.invalid_tool_calls] == ["ask_clarification"] + assert patched.invalid_tool_calls[0]["id"] == "call_clarify_1" + + tool_messages = [message for message in result["messages"] if isinstance(message, ToolMessage)] + assert tool_messages == [] diff --git a/backend/tests/test_clarification_middleware.py b/backend/tests/test_clarification_middleware.py index a824e7c2a..97f24a227 100644 --- a/backend/tests/test_clarification_middleware.py +++ b/backend/tests/test_clarification_middleware.py @@ -800,3 +800,253 @@ class TestClarificationDisabled: merged = add_messages(add_messages([], [first_message]), [second_message]) assert len(merged) == 1 + + +class TestDropParallelSiblingTools: + """after_model must drop sibling tools when ask_clarification is in the same turn. + + langchain's return_direct router inspects all client-side tool calls of the + last AIMessage and routes to END only when every one is return_direct, so a + parallel bash/write_file would both execute and keep the agent loop alive. + """ + + def _runtime(self, **context): + return SimpleNamespace(context=context) + + def _ai(self, tool_calls, content="", invalid_tool_calls=None): + from langchain_core.messages import AIMessage + + kwargs = {"content": content, "tool_calls": tool_calls} + if invalid_tool_calls is not None: + kwargs["invalid_tool_calls"] = invalid_tool_calls + return AIMessage(**kwargs) + + def test_drops_siblings_when_clarification_is_first(self, middleware): + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "Which dir?"}}, + {"id": "b1", "name": "bash", "args": {"command": "rm -rf /tmp/foo"}}, + ] + ) + update = middleware.after_model({"messages": [msg]}, self._runtime()) + assert update is not None + patched = update["messages"][0] + assert [tc["name"] for tc in patched.tool_calls] == ["ask_clarification"] + assert patched.id == msg.id + + def test_drops_siblings_when_clarification_is_last(self, middleware): + msg = self._ai( + [ + {"id": "b1", "name": "bash", "args": {"command": "echo hi"}}, + {"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}, + ] + ) + update = middleware.after_model({"messages": [msg]}, self._runtime()) + assert update is not None + assert [tc["name"] for tc in update["messages"][0].tool_calls] == ["ask_clarification"] + + def test_leaves_solo_clarification_unchanged(self, middleware): + msg = self._ai([{"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}]) + assert middleware.after_model({"messages": [msg]}, self._runtime()) is None + + def test_leaves_non_clarification_turn_unchanged(self, middleware): + msg = self._ai( + [ + {"id": "b1", "name": "bash", "args": {"command": "ls"}}, + {"id": "w1", "name": "write_file", "args": {"path": "a.txt"}}, + ] + ) + assert middleware.after_model({"messages": [msg]}, self._runtime()) is None + + def test_disable_clarification_keeps_sibling_tools(self, middleware): + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}, + {"id": "b1", "name": "bash", "args": {"command": "echo hi"}}, + ] + ) + assert middleware.after_model({"messages": [msg]}, self._runtime(disable_clarification=True)) is None + + def test_drops_siblings_when_clarification_args_fail_json_parse(self, middleware): + """default_tool_parser splits raw OpenAI payloads per call.""" + from langchain_core.messages import AIMessage + + msg = AIMessage( + content="", + additional_kwargs={ + "tool_calls": [ + { + "id": "c1", + "type": "function", + "function": {"name": "ask_clarification", "arguments": "{not-json"}, + }, + { + "id": "b1", + "type": "function", + "function": {"name": "bash", "arguments": '{"command": "echo hi"}'}, + }, + ] + }, + ) + assert [tc["name"] for tc in msg.tool_calls] == ["bash"] + assert [tc["name"] for tc in msg.invalid_tool_calls] == ["ask_clarification"] + + update = middleware.after_model({"messages": [msg]}, self._runtime()) + assert update is not None + patched = update["messages"][0] + assert patched.tool_calls == [] + assert [tc["name"] for tc in patched.invalid_tool_calls] == ["ask_clarification"] + + def test_drops_siblings_when_clarification_is_invalid(self, middleware): + """LangChain parks malformed ask_clarification on invalid_tool_calls.""" + invalid = [ + { + "id": "c1", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + } + ] + msg = self._ai( + [{"id": "b1", "name": "bash", "args": {"command": "rm -rf /tmp/foo"}}], + invalid_tool_calls=invalid, + ) + update = middleware.after_model({"messages": [msg]}, self._runtime()) + assert update is not None + patched = update["messages"][0] + assert patched.tool_calls == [] + assert [tc["name"] for tc in patched.invalid_tool_calls] == ["ask_clarification"] + assert patched.invalid_tool_calls[0]["id"] == "c1" + assert patched.id == msg.id + + def test_leaves_solo_invalid_clarification_unchanged(self, middleware): + msg = self._ai( + [], + invalid_tool_calls=[ + { + "id": "c1", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + } + ], + ) + assert middleware.after_model({"messages": [msg]}, self._runtime()) is None + + def test_disable_clarification_keeps_siblings_when_clarification_is_invalid(self, middleware): + msg = self._ai( + [{"id": "b1", "name": "bash", "args": {"command": "echo hi"}}], + invalid_tool_calls=[ + { + "id": "c1", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + } + ], + ) + assert middleware.after_model({"messages": [msg]}, self._runtime(disable_clarification=True)) is None + + def test_keeps_valid_clarification_when_mixed_with_invalid_and_sibling(self, middleware): + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "Which dir?"}}, + {"id": "b1", "name": "bash", "args": {"command": "echo hi"}}, + ], + invalid_tool_calls=[ + { + "id": "c2", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + } + ], + ) + patched = middleware.after_model({"messages": [msg]}, self._runtime())["messages"][0] + assert [tc["name"] for tc in patched.tool_calls] == ["ask_clarification"] + assert patched.tool_calls[0]["id"] == "c1" + assert [tc["id"] for tc in patched.invalid_tool_calls] == ["c2"] + + def test_strips_sibling_content_blocks_when_clarification_is_invalid(self, middleware): + content = [ + {"type": "text", "text": "asking"}, + {"type": "tool_use", "id": "c1", "name": "ask_clarification", "input": "{"}, + {"type": "tool_use", "id": "b1", "name": "bash", "input": {"command": "rm -rf /"}}, + ] + msg = self._ai( + [{"id": "b1", "name": "bash", "args": {"command": "rm -rf /"}}], + content=content, + invalid_tool_calls=[ + { + "id": "c1", + "name": "ask_clarification", + "args": "{", + "error": "Failed to parse tool arguments", + "type": "invalid_tool_call", + } + ], + ) + patched = middleware.after_model({"messages": [msg]}, self._runtime())["messages"][0] + assert patched.content == [ + {"type": "text", "text": "asking"}, + {"type": "tool_use", "id": "c1", "name": "ask_clarification", "input": "{"}, + ] + assert patched.tool_calls == [] + + def test_strips_matching_tool_use_content_blocks(self, middleware): + content = [ + {"type": "text", "text": "asking"}, + {"type": "tool_use", "id": "c1", "name": "ask_clarification", "input": {"question": "q?"}}, + {"type": "tool_use", "id": "b1", "name": "bash", "input": {"command": "rm -rf /"}}, + ] + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}, + {"id": "b1", "name": "bash", "args": {"command": "rm -rf /"}}, + ], + content=content, + ) + patched = middleware.after_model({"messages": [msg]}, self._runtime())["messages"][0] + assert patched.content == [ + {"type": "text", "text": "asking"}, + {"type": "tool_use", "id": "c1", "name": "ask_clarification", "input": {"question": "q?"}}, + ] + + def test_strips_idless_gemini_function_call_content_blocks(self, middleware): + # Gemini-style function_call blocks have no id; langchain synthesizes + # ids onto tool_calls only. Matching by id would leave the dropped + # sibling's content block in the transcript. + content = [ + {"type": "text", "text": "asking"}, + {"type": "function_call", "name": "ask_clarification", "args": {"question": "q?"}}, + {"type": "function_call", "name": "bash", "args": {"command": "rm -rf /"}}, + ] + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}, + {"id": "b1", "name": "bash", "args": {"command": "rm -rf /"}}, + ], + content=content, + ) + patched = middleware.after_model({"messages": [msg]}, self._runtime())["messages"][0] + assert patched.content == [ + {"type": "text", "text": "asking"}, + {"type": "function_call", "name": "ask_clarification", "args": {"question": "q?"}}, + ] + assert [tc["name"] for tc in patched.tool_calls] == ["ask_clarification"] + + def test_aafter_model_matches_sync(self, middleware): + import asyncio + + msg = self._ai( + [ + {"id": "c1", "name": "ask_clarification", "args": {"question": "q?"}}, + {"id": "b1", "name": "bash", "args": {"command": "echo hi"}}, + ] + ) + update = asyncio.run(middleware.aafter_model({"messages": [msg]}, self._runtime())) + assert [tc["name"] for tc in update["messages"][0].tool_calls] == ["ask_clarification"]