mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-19 15:18:03 +00:00
fix(acp): don't let a malformed tool argument abort the tool-call render
build_tool_start renders every ACP (Zed) tool call — on the live tool-progress callback (acp_adapter/events.py) and during session history replay (acp_adapter/server.py). It called build_tool_title and extract_locations directly, so a model that emits a malformed argument crashed the render: - terminal `command` as null/number -> TypeError (len() in build_tool_title) - delegate_task `goal` as a number -> TypeError (len()) - read_file `path` as a non-string -> pydantic ValidationError building a ToolCallLocation A live crash breaks the tool-call event; a persisted one breaks history replay on every resume of that session. The sibling CLI label builder get_cute_tool_message was already wrapped for exactly this reason (agent/display.py: "display must never abort a turn"). Wrap build_tool_start the same way: on any builder failure, fall back to a minimal, valid start event (tool name as title, resolved kind). The happy path is unchanged. Adds tests for the non-string command, path, and goal cases.
This commit is contained in:
parent
be100d4dae
commit
05473428cb
2 changed files with 54 additions and 1 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"]}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue