diff --git a/acp_adapter/tools.py b/acp_adapter/tools.py index fce98cf54e6c..1f272ab252dc 100644 --- a/acp_adapter/tools.py +++ b/acp_adapter/tools.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import logging import uuid from typing import Any, Dict, List, Optional @@ -14,6 +15,8 @@ from acp.schema import ( ToolKind, ) +logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # Map hermes tool names -> ACP ToolKind # --------------------------------------------------------------------------- @@ -1026,7 +1029,37 @@ def build_tool_start( *, edit_diff: Any = None, ) -> ToolCallStart: - """Create a ToolCallStart event for the given hermes tool invocation.""" + """Create a ToolCallStart event for the given hermes tool invocation. + + A malformed tool argument (e.g. a non-string ``command``/``path`` from a + model that ignores the schema) must never abort the ACP tool-call render — + ``build_tool_start`` runs on the live tool-progress callback and during + session history replay. On any failure in the title/content/location + builders, fall back to a minimal, valid start event. Mirrors + ``get_cute_tool_message`` in ``agent/display.py``, wrapped for the same + reason on the CLI side. + """ + try: + return _build_tool_start( + tool_call_id, tool_name, arguments, edit_diff=edit_diff + ) + except Exception as exc: # noqa: BLE001 — a tool-call render must never abort the turn + logger.debug("ACP tool-start render failed for %r: %s", tool_name, exc) + safe_name = tool_name if isinstance(tool_name, str) and tool_name else "tool" + return acp.start_tool_call( + tool_call_id, safe_name, kind=get_tool_kind(safe_name), + content=None, locations=[], raw_input=None, + ) + + +def _build_tool_start( + tool_call_id: str, + tool_name: str, + arguments: Dict[str, Any], + *, + edit_diff: Any = None, +) -> ToolCallStart: + """Build the ToolCallStart event (unguarded; see ``build_tool_start``).""" kind = get_tool_kind(tool_name) title = build_tool_title(tool_name, arguments) locations = extract_locations(arguments) diff --git a/tests/acp/test_tools.py b/tests/acp/test_tools.py index b291b36531a1..8ca3aafd2037 100644 --- a/tests/acp/test_tools.py +++ b/tests/acp/test_tools.py @@ -227,6 +227,26 @@ class TestBuildToolStart: assert result.content is None assert result.raw_input is None + def test_build_tool_start_survives_non_string_command(self): + """A malformed (non-string) terminal command previously raised + TypeError in build_tool_title (len(None)) and aborted the render.""" + result = build_tool_start("tc-bad-cmd", "terminal", {"command": None}) + assert isinstance(result, ToolCallStart) + assert result.kind == "execute" # tool identity preserved in the fallback + + def test_build_tool_start_survives_non_string_path(self): + """A non-string read_file path previously raised a ToolCallLocation + pydantic ValidationError in extract_locations and aborted the render.""" + result = build_tool_start("tc-bad-path", "read_file", {"path": {"p": "x"}}) + assert isinstance(result, ToolCallStart) + assert result.kind == "read" + + def test_build_tool_start_survives_non_string_goal(self): + """A non-string delegate_task goal previously raised TypeError + (len(123)) in build_tool_title and aborted the render.""" + result = build_tool_start("tc-bad-goal", "delegate_task", {"goal": 123}) + assert isinstance(result, ToolCallStart) + def test_build_tool_start_for_web_extract_is_compact(self): """web_extract start should stay compact; title identifies URLs.""" args = {"urls": ["https://example.com/docs"]}