From 493f9cfe8652f91a70e80195ae6fcc1a02536774 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:55:52 -0700 Subject: [PATCH] fix: execute tool calls returned by Ollama models Ollama-served models sometimes emit a tool call as plain-text JSON in final_output instead of through the tool-calling interface, so the turn ended with no lifecycle call and the generic nudge did not tell the model what went wrong. Detect the tool-call shape (bare object, fenced JSON, tool_calls array, or function wrapper) and send a targeted recovery message naming the failure. Reworked onto upstream's _run_until_lifecycle, which replaced the separate interactive/noninteractive paths this fix originally targeted. --- strix/core/execution.py | 72 +++++++++++++++++++++++++++++++++++++++-- tests/test_execution.py | 51 +++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 3 deletions(-) diff --git a/strix/core/execution.py b/strix/core/execution.py index bd99e7c3..80931488 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio import contextlib +import json import logging import uuid from collections.abc import Callable @@ -54,6 +55,7 @@ StreamEventSink = Callable[[str, Any], None] _INPUT_REJECTION_CODES = frozenset({400, 404, 422}) _MAX_COMPACTIONS_PER_CYCLE = 2 +_TOOL_ARGUMENT_KEYS = frozenset({"action_input", "arguments", "input", "parameters", "params"}) class ProviderRefusalError(AgentsException): @@ -501,11 +503,17 @@ async def _run_until_lifecycle( await coordinator.reset_recovery(agent_id) return result + serialized_tool_call = _looks_like_unexecuted_tool_call(result) recoveries = await coordinator.record_recovery(agent_id) + recovery_reason = ( + "produced tool-call-shaped final output as plain text" + if serialized_tool_call + else "ended a turn without a lifecycle tool call" + ) logger.warning( - "agent %s ended a turn without a lifecycle tool call (interactive=%s); " - "forcing tool continuation (%d/%d): %s", + "agent %s %s (interactive=%s); forcing tool continuation (%d/%d): %s", agent_id, + recovery_reason, interactive, recoveries, recovery_limit, @@ -521,6 +529,7 @@ async def _run_until_lifecycle( attempt=recoveries, limit=recovery_limit, interactive=interactive, + serialized_tool_call=serialized_tool_call, ) @@ -815,6 +824,52 @@ def _final_output_preview(result: RunResultBase | None) -> str: return text[:300] +def _looks_like_unexecuted_tool_call(result: RunResultBase | None) -> bool: + final_output = getattr(result, "final_output", None) + if final_output is None: + return False + if isinstance(final_output, str): + parsed = _parse_json_final_output(final_output) + return parsed is not None and _is_tool_call_payload(parsed) + return _is_tool_call_payload(final_output) + + +def _parse_json_final_output(text: str) -> Any | None: + stripped = text.strip() + if not stripped: + return None + if stripped.startswith("```"): + lines = stripped.splitlines() + if len(lines) >= 2 and lines[-1].strip() == "```": + stripped = "\n".join(lines[1:-1]).strip() + try: + return json.loads(stripped) + except (TypeError, ValueError): + return None + + +def _is_tool_call_payload(payload: Any) -> bool: + if isinstance(payload, list): + return any(_is_tool_call_payload(item) for item in payload) + if not isinstance(payload, dict): + return False + + tool_calls = payload.get("tool_calls") + if isinstance(tool_calls, list) and any(_is_tool_call_payload(item) for item in tool_calls): + return True + + function = payload.get("function") + if isinstance(function, dict) and _is_tool_call_payload(function): + return True + + tool_name = payload.get("action") or payload.get("tool") or payload.get("name") + return ( + isinstance(tool_name, str) + and bool(tool_name.strip()) + and any(key in payload for key in _TOOL_ARGUMENT_KEYS) + ) + + async def _append_tool_required_message( *, session: Session | None, @@ -822,9 +877,20 @@ async def _append_tool_required_message( attempt: int, limit: int, interactive: bool, + serialized_tool_call: bool = False, ) -> list[dict[str, str]]: finish_tool = "finish_scan" if context.get("parent_id") is None else "agent_finish" - if interactive: + if serialized_tool_call: + message = ( + "Your previous response looked like a tool call, but it was returned as plain text " + "instead of being executed. Plain-text tool-call JSON is not executed by Strix. " + "Continue immediately and call exactly one tool using the tool-calling interface. " + f"If your work is complete, call {finish_tool}. " + "If you are blocked waiting for another agent, call wait_for_agents. " + "Otherwise call the intended execution or planning tool. " + f"This is recovery attempt {attempt}/{limit}." + ) + elif interactive: message = ( "Your previous message ended a turn without a tool call. Plain text never ends " "execution and never hands control to the user: it is shown to the user, and the " diff --git a/tests/test_execution.py b/tests/test_execution.py index d389bde3..d43f257f 100644 --- a/tests/test_execution.py +++ b/tests/test_execution.py @@ -991,6 +991,57 @@ async def test_interactive_text_only_turn_is_nudged_instead_of_parking( assert coordinator.statuses["root"] == "completed" +@pytest.mark.asyncio +async def test_interactive_serialized_tool_call_gets_targeted_nudge( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Tool-call JSON returned as text must be retried as an actual tool call.""" + coordinator = AgentCoordinator() + await coordinator.register("root", "strix", parent_id=None) + calls: list[Any] = [] + outputs = [ + '{"action": "exec_command", "params": {"cmd": "ls"}}', + "scan completed", + ] + + async def _cycle(*_args: Any, **kwargs: Any) -> Any: + calls.append(kwargs.get("input_data")) + status = "running" if len(calls) == 1 else "completed" + await coordinator.set_status("root", status) + return MagicMock(final_output=outputs[len(calls) - 1]) + + monkeypatch.setattr(execution, "_run_cycle_parked", _cycle) + + await _drive(coordinator, "root", interactive=True) + + assert len(calls) == 2 + nudge = calls[1][0]["content"] + assert "returned as plain text instead of being executed" in nudge + assert coordinator.statuses["root"] == "completed" + + +@pytest.mark.parametrize( + ("final_output", "expected"), + [ + ('{"action": "exec_command", "params": {"cmd": "ls"}}', True), + ( + "```json\n" + '{"tool_calls": [{"function": {"name": "exec_command", ' + '"arguments": "{\\"cmd\\": \\"ls\\"}"}}]}\n' + "```", + True, + ), + ({"name": "finish_scan", "arguments": {}}, True), + ('{"status": "complete"}', False), + ("ordinary final text", False), + ], +) +def test_detects_tool_calls_serialized_as_final_output(final_output: Any, expected: bool) -> None: + result = MagicMock(final_output=final_output) + + assert execution._looks_like_unexecuted_tool_call(result) is expected + + @pytest.mark.asyncio async def test_interactive_explicit_park_gets_no_nudge( monkeypatch: pytest.MonkeyPatch,