Recover truncated tool calls, and never pass a leaked one off as an answer

Small local models drift off the tool-call format, especially with a large
tool schema in play. Two failures followed from that, both of which ended
the turn looking like the model had simply stopped mid-sentence.

Salvage handled well-formed Qwen/Hermes XML but gave up entirely on a call
cut off partway through. It now takes the function name plus every
parameter that actually closed. A trailing unterminated `<parameter=…>` is
dropped rather than guessed, so a half-written path or file body can never
reach a tool; if that leaves a required argument missing, the call fails
validation and the model gets a corrective tool error, which is the agent
loop working.

Worse, a call that never parsed at all was reported as `status: completed`
— indistinguishable from the model deciding it was done, leaving the user
with narration trailing off into stray closing tags. It now ends on the
error path, so the GUI offers Retry. That is the right affordance here: the
drift is probabilistic, not deterministic, so the same model usually
succeeds on a second attempt.

Detection ignores fenced and inline code, so a model *explaining* tool-call
syntax is still a real answer, and requires that tools were offered at all.

Reported against qwen3.5-9b on LM Studio, 2026-07-26.

Co-Authored-By: Claude <noreply@anthropic.com>

tests/test_engine.py

@@ -439,3 +439,39 @@ def test_outbound_replaces_images_for_non_vision_models(tmp_path):
     assert all(p["type"] != "image_url" for p in parts)
     assert "not viewable" in parts[-1]["text"]
     assert engine.messages[-1]["content"][1]["type"] == "image_url"  # history untouched
+
+
+def test_leaked_tool_call_ends_the_turn_as_a_retriable_error(tmp_path):
+    """A tool call the endpoint couldn't parse must not pass as an answer. Ending "completed"
+    made a half-written call indistinguishable from the model deciding it was done — the user
+    saw narration trailing off into stray tags (owner report 2026-07-26, qwen3.5-9b on LM
+    Studio). It ends on the error path so the GUI offers Retry; the drift is probabilistic, so
+    retrying the same model usually works."""
+    leaked = "Let me read the key files.\n<tool_call>\n<function=nope_not_a_tool>\n<parameter="
+    engine, _ = _engine(tmp_path, [_text_turn(leaked)])
+    events = _collect(engine, "explore the codebase")
+
+    assert EventType.ERROR in _types(events)
+    assert EventType.TURN_END not in _types(events)
+    err = next(ev for ev in events if ev.type == EventType.ERROR)
+    assert err.data["error_type"] == "UnparsedToolCall"
+    assert "couldn't parse" in err.data["error"]
+    # Persisted as an error notice, which is what unlocks retry().
+    assert engine.messages[-1] == {
+        **engine.messages[-1],
+        "role": "notice",
+        "kind": "error",
+    }
+    assert engine._tail_is_retriable_error() is True
+
+
+def test_ordinary_text_answer_still_completes(tmp_path):
+    """Guard the other side: prose that merely mentions tool syntax inside code fences is a
+    real answer and must still complete normally."""
+    engine, _ = _engine(
+        tmp_path,
+        [_text_turn("Qwen writes calls like:\n```\n<tool_call><function=x>\n```\nThat's it.")],
+    )
+    events = _collect(engine, "how does qwen format tool calls?")
+    assert EventType.ERROR not in _types(events)
+    assert next(ev for ev in events if ev.type == EventType.TURN_END).data["status"] == "completed"

tests/test_provider_router.py

@@ -15,7 +15,10 @@ from coworker.providers import (
     capabilities_for,
 )
 from coworker.providers.registry import _normalize_ollama_url, build_provider_client
-from coworker.providers.openai_provider import _salvage_tool_calls_from_text
+from coworker.providers.openai_provider import (
+    _salvage_tool_calls_from_text,
+    looks_like_unparsed_tool_call,
+)
 
 
 # -- base_url passthrough -------------------------------------------------------
@@ -219,6 +222,21 @@ _TODO_TOOLS = [
 ]
 
 
+_GREP_TOOL = [
+    {
+        "type": "function",
+        "function": {
+            "name": "grep",
+            "parameters": {
+                "type": "object",
+                "properties": {"pattern": {"type": "string"}, "path": {"type": "string"}},
+                "required": ["pattern"],
+            },
+        },
+    }
+]
+
+
 def test_salvage_mixed_prose_and_object():
     # The model wrote prose THEN a bare-JSON tool call in one message.
     text = 'It seems the workspace is empty. {"name": "list_files", "arguments": {"recursive": true}}'
@@ -254,6 +272,39 @@ def test_salvage_filters_unknown_tool_name():
     assert _salvage_tool_calls_from_text(text, _TODO_TOOLS) == []
 
 
+def test_salvage_truncated_xml_call_keeps_only_complete_parameters():
+    """A local model that runs out of tokens mid-call leaves `<function=…>` unclosed. Take the
+    name and every parameter that DID close; NEVER the half-written trailing one — a truncated
+    path or file body reaching a tool is worse than no call at all."""
+    text = "<tool_call>\n<function=grep>\n<parameter=pattern>TODO</parameter>\n<parameter=path>sr"
+    calls = _salvage_tool_calls_from_text(text, _TODO_TOOLS + _GREP_TOOL)
+    assert len(calls) == 1 and calls[0].name == "grep"
+    assert calls[0].arguments == {"pattern": "TODO"}  # the partial `path` is gone
+
+
+def test_salvage_truncated_xml_prefers_a_complete_call_and_filters_unknown_names():
+    complete_then_cut = (
+        "<tool_call><function=list_files><parameter=recursive>true</parameter>"
+        "</function></tool_call>\n<tool_call>\n<function=grep>"
+    )
+    calls = _salvage_tool_calls_from_text(complete_then_cut, _TODO_TOOLS + _GREP_TOOL)
+    assert [c.name for c in calls] == ["list_files"]  # the finished one wins
+    # An unfinished call naming something we never offered stays text (no false positives).
+    assert _salvage_tool_calls_from_text("<function=rm_rf>\n<parameter=p>/", _TODO_TOOLS) == []
+
+
+def test_looks_like_unparsed_tool_call_ignores_code_and_needs_tools():
+    """Distinguishes a leaked call from a model *explaining* tool syntax — the latter is a real
+    answer and must not be turned into an error."""
+    leaked = "Let me read the files.\n</parameter>\n</function>\n</tool_call>"
+    assert looks_like_unparsed_tool_call(leaked, _TODO_TOOLS) is True
+    assert looks_like_unparsed_tool_call("A CLI that greets people.", _TODO_TOOLS) is False
+    fenced = "Qwen writes calls like:\n```\n<tool_call><function=x>\n```\nThat's the shape."
+    assert looks_like_unparsed_tool_call(fenced, _TODO_TOOLS) is False
+    assert looks_like_unparsed_tool_call("The `<tool_call>` wrapper.", _TODO_TOOLS) is False
+    assert looks_like_unparsed_tool_call(leaked, None) is False  # no tools offered → not a call
+
+
 def test_salvage_nested_braces_in_tag():
     text = '<tool_call>{"name": "todo_write", "arguments": {"items": [{"content": "a", "status": "pending"}]}}</tool_call>'
     calls = _salvage_tool_calls_from_text(text, _TODO_TOOLS)