diff --git a/AGENTS.md b/AGENTS.md index 68850c321..071c29056 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -80,6 +80,7 @@ Instrument critical-path code (RunLoop, Turn, delegation, protocol entry points) | Lifecycle dimensions (full) | `docs/explanation/lifecycle-dimensions.md` | | Hooks & events (full) | `docs/explanation/hooks-events.md` | | Capabilities (full) | `docs/explanation/capabilities.md` | +| ToolDisplayCapability (tool rename + diff events) | `docs/explanation/tool-display-capability.md` | | Telemetry rules | `docs/explanation/telemetry.md` | | Usage examples | `docs/explanation/usage-examples.md` | | Extending AgentPool | `docs/explanation/extending-agentpool.md` | diff --git a/docs/explanation/tool-display-capability.md b/docs/explanation/tool-display-capability.md new file mode 100644 index 000000000..c0f04df4c --- /dev/null +++ b/docs/explanation/tool-display-capability.md @@ -0,0 +1,76 @@ +# ToolDisplayCapability + +`ToolDisplayCapability` 是 agentpool 的**全局装饰器能力** (Global Decorator Capability):在不修改任何子能力源码的前提下,对 agent 已装配的全部工具统一起作用 —— **改名** (`rename_mode`) 与 **注入 diff 富信息事件** (`emit_diff`)。它解决的核心问题是:第三方 capability(如 viking)的工具名不在协议客户端(OpenCode TUI / Zed)的渲染白名单内、工具返回不含 diff 富信息,导致客户端无法渲染文件变更的 diff 视图。 + +模式对齐 `ToolInterceptCapability`(`src/agentpool/agents/native_agent/tool_intercept.py`):独立 `AbstractCapability` 直接覆写 `get_wrapper_toolset()` 与 `wrap_tool_execute()`,作为全局中间件横切 agent 的全部工具 —— 不组合子能力、无 `capabilities` 字段。 + +## 三个正交开关 + +| 开关 | 默认 | 作用 | +|---|---|---| +| `rename_mode: bool` | `true` | 启用工具改名。经 `get_wrapper_toolset()` 返回 pydantic-ai 官方 `RenamedToolset(wrapped=toolset, name_map=...)`,按 `name_map` 重写 `tool_def.name`,执行时自动还原 `ctx.tool_name`。`name_map` 为空或 `rename_mode: false` 时不包装 | +| `emit_diff: bool` | `true` | 启用 diff 事件注入。`wrap_tool_execute()` 在工具真实执行后,对命中的工具注入 `ToolCallProgressEvent.file_edit(...)`(携带 `DiffContentItem`) | +| `emit_diff_for: set[str]` | `set()`(空=不注入) | 选择注入白名单,**按工具名精确过滤**。仅当 `emit_diff: true` 且工具名在名单内时注入 | + +`name_map` 与 `emit_diff_for` 都为空时,退化为**无操作装饰器**:`get_wrapper_toolset` 返回 `None`,`wrap_tool_execute` 直接透传。 + +## Diff 数据来源(执行后注入) + +`wrap_tool_execute` 先调用 `handler(args)` **拿到真实执行结果**,再从 `args` 解析 diff 字段(模块级 `_parse_diff_fields` 辅助): + +- **write 风格** (工具入参含 `content`/`path`|`uri`):`new_text=content`、`old_text=None`(视为新增文件) +- **edit 风格** (工具入参含 `old_string` + `new_string`):`old_text=old_string`、`new_text=new_string` +- **退化兜底**:path 无法从入参解析、或 new_text 为空时,跳过注入且不报错 + +路径键取自 `path` / `file_path` / `uri` / `filepath` 的任意现值(按序取第一个),因此对 viking 的 `viking://...` URI 与本地文件路径同样有效。 + +## 事件注入通道 + +注入的 `ToolCallProgressEvent` + `DiffContentItem` 流经既有管道,零协议改动: + +``` +wrap_tool_execute → ctx.deps.events.tool_call_progress(title, items=[DiffContentItem(...)]) + → EventBus publish → EventMapper._is_rich_event 原样透传 + → ACP 转换器 DiffContentItem → FileEditToolCallContent + ToolCallLocation + → 客户端(Zed/OpenCode TUI)渲染 diff +``` + +- `ctx.deps` 在 agentpool 中**直接是 `AgentContext`**,携带 `.events` → `StreamEventEmitter`(POC 已验证,同 fsspec 工具集 `agentpool_toolsets/fsspec_toolset/toolset.py:575` 的先例通道) +- `DiffContentItem(path, old_text, new_text)` 定义于 `src/agentpool/agents/events/events.py:203` +- **不依赖 metadata 通道**:`ToolReturn.metadata` 在 `process_tool_event`/`event_mapper` 构造 `ToolCallCompleteEvent` 时会被丢弃(仅 `is_error`),本能力刻意绕开该断点,改用事件注入 + +## 协议区分配置 + +同一工具集在不同协议客户端下的展示诉求不同,通过**装配期**配置区分(零运行时协议标识改动): + +| 场景 | 配置 | 说明 | +|---|---|---| +| **OpenCode TUI** | `rename_mode: true` + `emit_diff: true` | TUI 按工具名白名单渲染 —— 改名命中白名单(`viking_write`→`write`)+ diff 注入 | +| **ACP (Zed)** | `rename_mode: false` + `emit_diff: true` | Zed 展示原名即可,`FileEditToolCallContent` 原生渲染差异 | +| **子能力已自发射** (fsspec 模式) | `rename_mode: true` + `emit_diff: false` | 子能力已自行 emit `DiffContentItem`,装饰器仅改名,避免重复注入 | + +**防重复原则**:子 capability 已自行发射 diff 事件的场景,必须用 `emit_diff: false`,否则同一变更被注入两次。 + +## 配置与注册 + +```yaml +# agent YAML capabilities 段 —— 与其它 capability 平级列出即可(全局中间件) +capabilities: + - type: tool_display + args: + rename_mode: true + name_map: + viking_write: write + viking_edit: edit + emit_diff: true + emit_diff_for: [viking_write, viking_edit] +``` + +- 注册:entry-point 组 `agentpool.capabilities`,key `tool_display` → `agentpool.capabilities.tool_display_capability:ToolDisplayCapability`(见 `pyproject.toml`),由 `registry.py` 发现 +- 构造:`EntryPointCapabilityConfig(type=..., args={...}).build()` 以 `cls(**args)` 实例化 —— dataclass 字段(`rename_mode`/`name_map`/`emit_diff`/`emit_diff_for`)+ `id` 天然兼容 YAML 装配 + +## 已知约束 + +- **`ctx.tool_name` 双名不一致**:改名后,事件映射层携带新名、工具自发射事件携带原名 —— 注入事件显式构造 `tool_call_id`,不依赖名称匹配 +- **只映射语义等价的标准名**:改名可能触发客户端内置行为(如 `write` 触发生成式 diff),只映射语义一致的工具,映射表见配置文档 +- **仅上游通道**:本能力不打通 OpenCode TUI 的 last-turn DiffViewer(远端写入无法被本地 git snapshot 捕获)—— 如需另立 change \ No newline at end of file diff --git a/docs/tool-display-capability.example.yaml b/docs/tool-display-capability.example.yaml new file mode 100644 index 000000000..49a3a1966 --- /dev/null +++ b/docs/tool-display-capability.example.yaml @@ -0,0 +1,66 @@ +# ToolDisplayCapability 配置示例 +# +# 全局装饰器:不改子能力源码,对 agent 已装配工具改名 + 注入 diff 富信息事件。 +# 三个正交开关:rename_mode(改名)/ emit_diff(注入)/ emit_diff_for(注入白名单)。 +# 详见 docs/explanation/tool-display-capability.md +--- +# 示例 1:OpenCode TUI —— 改名命中客户端渲染白名单 + diff 注入 +# viking_write/viking_edit 重命名为 write/edit,客户端按白名单渲染; +# 同时注入 DiffContentItem → TUI 显示文件变更 diff。 +agents: + wiki-librarian-opencode: + model: + identifier: anthropic:claude-sonnet-4-5 + capabilities: + - type: viking + mode: write + url: http://localhost:8765 + - type: tool_display + args: + rename_mode: true + name_map: + viking_write: write + viking_edit: edit + emit_diff: true + emit_diff_for: + - viking_write + - viking_edit +--- +# 示例 2:ACP (Zed) —— 展示原名 + diff 注入 +# Zed 通过 FileEditToolCallContent 原生渲染差异,无需改名; +# 仅注入 diff 富信息事件。 +agents: + wiki-librarian-acp: + model: + identifier: anthropic:claude-sonnet-4-5 + capabilities: + - type: viking + mode: write + url: http://localhost:8765 + - type: tool_display + args: + rename_mode: false + emit_diff: true + emit_diff_for: + - viking_write + - viking_edit +--- +# 示例 3:子能力已自发射 —— 仅改名,关闭注入避免重复 +# fsspec 等工具集已在内部 emit DiffContentItem(见 +# src/agentpool_toolsets/fsspec_toolset/toolset.py:575),装饰器只负责 +# 改名,emit_diff: false。 +agents: + fs-writer: + model: + identifier: anthropic:claude-sonnet-4-5 + tools: + - type: file_access + root: /data + capabilities: + - type: tool_display + args: + rename_mode: true + name_map: + fsspec_write: write + fsspec_edit: edit + emit_diff: false \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 1e1e90bc5..3f26983b3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,6 +103,7 @@ subagent = "agentpool.capabilities.subagent_capability:SubagentCapability" skill = "agentpool.capabilities.skill_manager_cap:SkillManagerCap" combined = "agentpool.capabilities.combined_toolset:CombinedToolsetCapability" code_mode = "agentpool.capabilities.code_mode_capability:CodeModeCapability" +tool_display = "agentpool.capabilities.tool_display_capability:ToolDisplayCapability" question = "agentpool_toolsets.builtin.question_tools:QuestionTools" [project.entry-points."fsspec.specs"] diff --git a/src/agentpool/capabilities/tool_display_capability.py b/src/agentpool/capabilities/tool_display_capability.py new file mode 100644 index 000000000..5c8e02651 --- /dev/null +++ b/src/agentpool/capabilities/tool_display_capability.py @@ -0,0 +1,233 @@ +"""ToolDisplayCapability — global decorator for tool display names and diff-rich events. + +A configurable ``AbstractCapability`` that decorates the agent's fully +assembled toolset without modifying any tool or capability: + +- **Rename layer** (``rename_mode``): maps selected tool names to + display names via :class:`~pydantic_ai.toolsets.RenamedToolset`, so + protocol clients (e.g. the OpenCode TUI) that dispatch on a whitelist + of standard tool names render the tools properly. +- **Rich-info layer** (``emit_diff``): injects a + :class:`~agentpool.agents.events.DiffContentItem` progress event after + a matching tool executes, so ACP clients (e.g. Zed) render a file + diff. + +The two layers are orthogonal: an OpenCode-facing deployment uses +``rename_mode=True + emit_diff=True``; an ACP-facing deployment uses +``rename_mode=False + emit_diff=True`` (original names displayed with +diffs); a child capability that already emits its own +``DiffContentItem`` uses ``rename_mode=True + emit_diff=False`` (rename +only, no duplicate diff). + +Modeled on :class:`~agentpool.agents.native_agent.tool_intercept.ToolInterceptCapability` +— a standalone ``AbstractCapability`` overriding ``get_wrapper_toolset`` +and ``wrap_tool_execute`` as a global middleware over all assembled +tools. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any + +import logfire +from pydantic_ai.capabilities import AbstractCapability +from pydantic_ai.toolsets import AbstractToolset, RenamedToolset + +from agentpool.agents.events import DiffContentItem + + +if TYPE_CHECKING: + from collections.abc import Awaitable, Callable, Mapping + + from pydantic_ai._run_context import RunContext + from pydantic_ai.messages import ToolCallPart + from pydantic_ai.tools import ToolDefinition + + +def _parse_diff_fields( + args: Mapping[str, Any], result: Any +) -> tuple[str | None, str | None, str | None]: + """Extract (path, old_text, new_text) from tool call arguments and result. + + Recognizes common parameter shapes across file-writing tools: + + - ``path``/``file_path``/``uri`` → target path + - ``content`` (write-style) → new text, old text ``None`` (new file) + - ``old_string``/``new_string`` (edit-style) → old/new text pair + + ``result`` is inspected as a fallback when ``new_text`` cannot be + derived from arguments (e.g. a tool that returns the written content + as a string). + + Args: + args: The validated tool call arguments. + result: The tool execution result. + + Returns: + A ``(path, old_text, new_text)`` tuple with ``None`` values for + fields that could not be derived. + """ + path = next( + ( + str(args[k]) + for k in ("path", "file_path", "uri", "filepath") + if isinstance(args.get(k), str) and args[k] + ), + None, + ) + if path is None: + return (None, None, None) + + old_text: str | None = None + new_text: str | None = None + if isinstance(args.get("old_string"), str) and isinstance(args.get("new_string"), str): + old_text = args["old_string"] + new_text = args["new_string"] + elif isinstance(args.get("content"), str): + new_text = args["content"] + elif isinstance(result, str) and result: + new_text = result + + return (path, old_text, new_text) + + +@dataclass(kw_only=True) +class ToolDisplayCapability(AbstractCapability[Any]): + """Global tool display decorator: rename tools + inject diff events. + + Attributes: + rename_mode: Enable tool name mapping via ``name_map``. When + ``False``, tools keep their native names (ACP-style display). + name_map: Mapping of **original** tool name to **display** name + (what the user writes in YAML). Internally inverted before + passing to ``RenamedToolset``, which expects ``{new: original}``. + emit_diff: Enable diff event injection after tool execution. + When ``False``, rely on tools' own diff emission. + emit_diff_for: Set of **original** tool names eligible for diff + event injection. Empty means no injection. When rename is + active, display names are resolved back to originals before + matching. + id: Optional capability id. + """ + + id: str | None = None + rename_mode: bool = True + name_map: Mapping[str, str] = field(default_factory=dict) + emit_diff: bool = True + emit_diff_for: set[str] = field(default_factory=set) + + def __post_init__(self) -> None: + """Coerce ``emit_diff_for`` to ``set`` if a list was provided via YAML.""" + if isinstance(self.emit_diff_for, list): + self.emit_diff_for = set(self.emit_diff_for) + + @property + def _reverse_name_map(self) -> dict[str, str]: + """Display → original lookup, derived from ``name_map`` (original → display).""" + return {v: k for k, v in self.name_map.items()} + + def get_wrapper_toolset(self, toolset: AbstractToolset[Any]) -> AbstractToolset[Any] | None: + """Wrap the assembled toolset with ``RenamedToolset`` when enabled. + + ``name_map`` is stored as ``{original: display}`` (user-facing + convention) but ``RenamedToolset`` expects ``{new: original}``, + so we invert before construction. + + Args: + toolset: The agent's fully assembled toolset. + + Returns: + A ``RenamedToolset`` applying ``name_map``, or ``None`` when + renaming is disabled or the map is empty (toolset unchanged). + """ + if not self.rename_mode or not self.name_map: + return None + # RenamedToolset.name_map is {new_name: original_name}. + # Our name_map is {original: display} → invert to {display: original}. + inverted = {v: k for k, v in self.name_map.items()} + return RenamedToolset(wrapped=toolset, name_map=inverted) + + async def wrap_tool_execute( + self, + ctx: RunContext[Any], + *, + call: ToolCallPart, + tool_def: ToolDefinition, + args: dict[str, Any], + handler: Callable[[dict[str, Any]], Awaitable[Any]], + ) -> Any: + """Execute the tool, then inject a diff progress event when enabled. + + After ``handler`` completes, derives ``(path, old_text, new_text)`` + from the call arguments and emits a + :class:`~agentpool.agents.events.ToolCallProgressEvent` carrying a + :class:`~agentpool.agents.events.DiffContentItem` via the run + context's ``events`` emitter — the same channel fsspec tools use, + which reaches ACP converters as ``FileEditToolCallContent``. + + Two critical steps before emitting: + + 1. **Populate ``ctx.deps.tool_call_id`` / ``tool_name``** — + capability tools (viking, fsspec, …) bypass + ``tool_wrapping.py``, so these fields are ``None``. Without + them, ``StreamEventEmitter`` reads ``""`` and the ACP + converter drops the event (``if tool_call_id:`` guard fails). + 2. **Resolve display → original name** — when rename is active, + ``call.tool_name`` is the *display* name. ``emit_diff_for`` + contains *original* names. We reverse-lookup through + ``name_map`` before matching. + + Args: + ctx: The pydantic-ai run context (carries ``deps`` → agentpool + ``AgentContext`` with the ``events`` emitter). + call: The tool call part. + tool_def: The tool definition. + args: The validated tool call arguments. + handler: The wrapped tool execution callable. + + Returns: + The tool execution result, unchanged. + """ + with logfire.span("capability.tool_display.wrap_tool_execute", tool_name=call.tool_name): + result = await handler(args) + + if not self.emit_diff or not self.emit_diff_for: + return result + + # Resolve display name → original for emit_diff_for matching. + # When rename is active, call.tool_name is the display name; + # emit_diff_for contains original names. + original_name = self._reverse_name_map.get(call.tool_name, call.tool_name) + if original_name not in self.emit_diff_for: + return result + + path, old_text, new_text = _parse_diff_fields(args, result) + if path is None or new_text is None: + return result + + # Populate ctx.deps.tool_call_id / tool_name for capability tools. + # tool_wrapping.py only does this for legacy direct tools (agent.py:1044-1052); + # capability tools (AbstractCapability) skip that path, leaving these as None. + # StreamEventEmitter reads self._context.tool_call_id → "" → ACP converter drops. + deps = ctx.deps + if hasattr(deps, "tool_call_id"): + deps.tool_call_id = call.tool_call_id + if hasattr(deps, "tool_name"): + deps.tool_name = original_name + + events = getattr(deps, "events", None) + if events is None: + return result + + await events.tool_call_progress( + title=f"Modified: {path}", + items=[ + DiffContentItem( + path=path, + old_text=old_text, + new_text=new_text, + ) + ], + ) + return result diff --git a/src/agentpool_server/opencode_server/converters.py b/src/agentpool_server/opencode_server/converters.py index 10ae7d09e..db9d979c1 100644 --- a/src/agentpool_server/opencode_server/converters.py +++ b/src/agentpool_server/opencode_server/converters.py @@ -77,6 +77,7 @@ _PARAM_NAME_MAP: dict[str, str] = { "path": "filePath", "file_path": "filePath", + "uri": "filePath", "old_string": "oldString", "new_string": "newString", "replace_all": "replaceAll", diff --git a/src/agentpool_server/opencode_server/event_processor.py b/src/agentpool_server/opencode_server/event_processor.py index 42170ce44..270f2684a 100644 --- a/src/agentpool_server/opencode_server/event_processor.py +++ b/src/agentpool_server/opencode_server/event_processor.py @@ -8,6 +8,7 @@ from __future__ import annotations from dataclasses import dataclass +import difflib from typing import TYPE_CHECKING, Any from pydantic_ai import FunctionToolCallEvent @@ -22,6 +23,7 @@ ) from agentpool.agents.events import ( + DiffContentItem, ElicitationDeferredEvent, FileContentItem, LocationContentItem, @@ -754,6 +756,29 @@ def _process_tool_progress( new_output += content case LocationContentItem(): pass + case DiffContentItem(path=path, old_text=old, new_text=new): + # Convert structured diff to unified diff text for TUI rendering. + # The opencode TUI Edit component reads metadata.diff as a string + # (createTwoFilesPatch format). Accumulate per tool_call_id; + # _process_tool_complete merges it into ToolStateCompleted.metadata. + # + # Use lineterm="" + splitlines(keepends=False) so no line carries + # an embedded \n. Join with \n and add trailing \n to ensure every + # line is properly terminated — the npm "diff" parser strictly + # validates hunk line counts and fails on missing terminators. + old_str = old or "" + new_str = new or "" + diff_iter = difflib.unified_diff( + old_str.splitlines(keepends=False), + new_str.splitlines(keepends=False), + fromfile=path, + tofile=path, + lineterm="", + ) + diff_text = "\n".join(diff_iter) + if diff_text: + diff_text += "\n" + ctx.tool_diffs[tool_call_id] = diff_text if new_output: ctx.append_tool_output(tool_call_id, new_output) @@ -845,11 +870,22 @@ def _process_tool_complete( error_string = str(result.get("error", "Unknown error")) new_state = ToolStateError(error=error_string, input=tool_input, time=t) else: + # Merge accumulated diff text (from DiffContentItem in progress events) + # into completion metadata so the TUI Edit component can render it. + # Also set diagnostics=[] when diff content exists: this triggers the + # Write component's code-block branch (props.metadata.diagnostics !== + # undefined) so the written content is visible instead of "Preparing + # write...". An empty diagnostics array renders no error messages. + diff_text = ctx.tool_diffs.pop(tool_call_id, "") + merged_metadata: dict[str, Any] = dict(event_metadata or {}) + if diff_text: + merged_metadata["diff"] = diff_text + merged_metadata.setdefault("diagnostics", []) new_state = ToolStateCompleted( title="Completed", input=tool_input, output=result_str, - metadata=event_metadata or {}, + metadata=merged_metadata, time=TimeStartEndCompacted(start=start, end=now_ms()), ) diff --git a/src/agentpool_server/opencode_server/event_processor_context.py b/src/agentpool_server/opencode_server/event_processor_context.py index e72944f6c..d1a561a90 100644 --- a/src/agentpool_server/opencode_server/event_processor_context.py +++ b/src/agentpool_server/opencode_server/event_processor_context.py @@ -86,6 +86,9 @@ class EventProcessorContext: tool_parts: dict[str, ToolPart] = field(default_factory=dict, init=False) tool_outputs: dict[str, str] = field(default_factory=dict, init=False) tool_inputs: dict[str, dict[str, Any]] = field(default_factory=dict, init=False) + # Unified diff text accumulated from DiffContentItem in ToolCallProgressEvent, + # carried to ToolStateCompleted.metadata.diff for TUI diff rendering. + tool_diffs: dict[str, str] = field(default_factory=dict, init=False) # Subagent tool parts tracking (key: "depth:source_name" -> ToolPart) subagent_tool_parts: dict[str, ToolPart] = field(default_factory=dict, init=False) diff --git a/tests/capabilities/test_tool_display_capability.py b/tests/capabilities/test_tool_display_capability.py new file mode 100644 index 000000000..c5e412d9c --- /dev/null +++ b/tests/capabilities/test_tool_display_capability.py @@ -0,0 +1,286 @@ +"""Tests for ToolDisplayCapability — global tool rename + diff-event injection.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, MagicMock + +from pydantic_ai.messages import ToolCallPart +from pydantic_ai.tools import ToolDefinition +from pydantic_ai.toolsets import RenamedToolset +import pytest + +from agentpool.capabilities.tool_display_capability import ToolDisplayCapability + + +if TYPE_CHECKING: + from agentpool.agents.events import DiffContentItem + + +pytestmark = pytest.mark.unit + + +def _make_call(tool_name: str, args: dict[str, Any]) -> ToolCallPart: + """Create a ToolCallPart with the given tool name and arguments.""" + return ToolCallPart(tool_name=tool_name, args={}, tool_call_id=f"call_{tool_name}") + + +def _make_tool_def(name: str) -> ToolDefinition: + """Create a minimal ToolDefinition.""" + return ToolDefinition(name=name, description="test tool") + + +def _make_handler(result: Any) -> AsyncMock: + """Create an async handler returning a fixed result.""" + return AsyncMock(return_value=result) + + +def _make_ctx(events: Any) -> MagicMock: + """Create a RunContext stand-in carrying deps with an events emitter.""" + deps = MagicMock() + deps.events = events + ctx = MagicMock() + ctx.deps = deps + return ctx + + +# ---- get_wrapper_toolset: RenamedToolset ---- + + +@pytest.mark.asyncio +async def test_get_wrapper_toolset_applies_rename() -> None: + """rename_mode=True with a non-empty name_map wraps in RenamedToolset.""" + cap = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=False, + ) + wrapped = cap.get_wrapper_toolset(MagicMock()) # type: ignore[arg-type] + assert isinstance(wrapped, RenamedToolset) + # name_map is {original: display} in config, inverted to {display: original} for RenamedToolset + assert wrapped.name_map == {"write": "viking_write"} + + +def test_get_wrapper_toolset_rename_disabled() -> None: + """rename_mode=False leaves the toolset unchanged (None wrapper).""" + cap = ToolDisplayCapability( + rename_mode=False, + name_map={"viking_write": "write"}, + emit_diff=True, + ) + assert cap.get_wrapper_toolset(MagicMock()) is None # type: ignore[arg-type] + + +def test_get_wrapper_toolset_empty_name_map() -> None: + """An empty name_map never wraps, regardless of rename_mode.""" + cap = ToolDisplayCapability(rename_mode=True, name_map={}, emit_diff=True) + assert cap.get_wrapper_toolset(MagicMock()) is None # type: ignore[arg-type] + + +# ---- wrap_tool_execute: diff event injection ---- + + +@pytest.mark.asyncio +async def test_wrap_execute_injects_diff_for_write_tool() -> None: + """A write-style tool in emit_diff_for gets a DiffContentItem event.""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability( + emit_diff=True, + emit_diff_for={"viking_write", "viking_edit"}, + ) + handler = _make_handler("Wrote 3 chars to viking://x.") + + result = await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=handler, # type: ignore[arg-type] + ) + + assert result == "Wrote 3 chars to viking://x." + events.tool_call_progress.assert_awaited_once() + call_kwargs = events.tool_call_progress.await_args.kwargs + assert call_kwargs["title"] == "Modified: viking://x.md" + items: list[DiffContentItem] = call_kwargs["items"] + assert len(items) == 1 + assert items[0].path == "viking://x.md" + assert items[0].old_text is None + assert items[0].new_text == "abc" + + +@pytest.mark.asyncio +async def test_wrap_execute_injects_diff_for_edit_tool() -> None: + """An edit-style tool carries old_string/new_string as old/new text.""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability(emit_diff=True, emit_diff_for={"viking_edit"}) + + await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_edit", {"uri": "viking://x.md"}), + tool_def=_make_tool_def("viking_edit"), # type: ignore[arg-type] + args={ + "uri": "viking://x.md", + "old_string": "old", + "new_string": "new", + }, + handler=_make_handler("Replaced 1 occurrence(s) in viking://x.md."), # type: ignore[arg-type] + ) + + events.tool_call_progress.assert_awaited_once() + items: list[DiffContentItem] = events.tool_call_progress.await_args.kwargs["items"] + assert items[0].old_text == "old" + assert items[0].new_text == "new" + + +@pytest.mark.asyncio +async def test_wrap_execute_no_event_outside_emit_diff_for() -> None: + """Tools not in emit_diff_for produce no diff event.""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability(emit_diff=True, emit_diff_for={"viking_edit"}) + + result = await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_read", {}), + tool_def=_make_tool_def("viking_read"), # type: ignore[arg-type] + args={}, + handler=_make_handler("content"), # type: ignore[arg-type] + ) + + assert result == "content" + events.tool_call_progress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_wrap_execute_no_event_when_emit_disabled() -> None: + """emit_diff=False (e.g. child capability self-emits) injects nothing.""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability(emit_diff=False, emit_diff_for={"viking_write"}) + + await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("ok"), # type: ignore[arg-type] + ) + + events.tool_call_progress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_wrap_execute_no_event_without_events_emitter() -> None: + """Missing deps/events emitter degrades gracefully (no crash).""" + cap = ToolDisplayCapability(emit_diff=True, emit_diff_for={"viking_write"}) + + for ctx in ( + MagicMock(deps=None), + MagicMock(deps=MagicMock(events=None)), + ): + result = await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("ok"), # type: ignore[arg-type] + ) + assert result == "ok" + + handler = _make_handler("ok") + await cap.wrap_tool_execute( + MagicMock(deps=None), + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=handler, # type: ignore[arg-type] + ) + handler.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_wrap_execute_no_event_when_path_missing() -> None: + """Tools lacking a derivable path produce no event.""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability(emit_diff=True, emit_diff_for={"viking_write"}) + + await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={}, + handler=_make_handler("ok"), # type: ignore[arg-type] + ) + + events.tool_call_progress.assert_not_awaited() + + +# ---- orthogonal switch matrix ---- + + +@pytest.mark.asyncio +async def test_rename_only_mode_no_diff() -> None: + """rename_mode=True + emit_diff=False: renames, no events (fsspec-style).""" + events = AsyncMock() + ctx = _make_ctx(events) + cap = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=False, + ) + wrapped = cap.get_wrapper_toolset(MagicMock()) # type: ignore[arg-type] + assert isinstance(wrapped, RenamedToolset) + + await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("ok"), # type: ignore[arg-type] + ) + events.tool_call_progress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_rich_only_mode_keeps_original_name() -> None: + """rename_mode=False + emit_diff=True: original names, diff events (ACP-style).""" + cap = ToolDisplayCapability( + rename_mode=False, + name_map={"viking_write": "write"}, + emit_diff=True, + emit_diff_for={"viking_write"}, + ) + assert cap.get_wrapper_toolset(MagicMock()) is None # type: ignore[arg-type] + + events = AsyncMock() + ctx = _make_ctx(events) + await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", {"uri": "viking://x.md", "content": "abc"}), + tool_def=_make_tool_def("viking_write"), # type: ignore[arg-type] + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("ok"), # type: ignore[arg-type] + ) + events.tool_call_progress.assert_awaited_once() + + +# ---- degenerate config: no-op decorator ---- + + +def test_empty_config_is_noop_decorator() -> None: + """All defaults off → toolset unchanged and no events.""" + cap = ToolDisplayCapability( + rename_mode=True, + name_map={}, + emit_diff=True, + emit_diff_for=set(), + ) + assert cap.get_wrapper_toolset(MagicMock()) is None # type: ignore[arg-type] + + # get_wrapper_toolset with no name_map is a no-op even with emit_diff on. + assert cap.emit_diff is True + assert cap.emit_diff_for == set() diff --git a/tests/capabilities/test_tool_display_integration.py b/tests/capabilities/test_tool_display_integration.py new file mode 100644 index 000000000..f41d4b208 --- /dev/null +++ b/tests/capabilities/test_tool_display_integration.py @@ -0,0 +1,163 @@ +"""Integration tests for ToolDisplayCapability — global decoration of assembled tools. + +Validates that ToolDisplayCapability, wired alongside a concrete +capability (viking), decorates exactly the tools in ``name_map`` / +``emit_diff_for`` without altering the underlying tool semantics. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, MagicMock + +from pydantic_ai.models import ModelRequestContext, ModelRequestParameters +from pydantic_ai.models.test import TestModel +from pydantic_ai.toolsets.renamed import RenamedToolset +import pytest + +from agentpool.capabilities.tool_display_capability import ToolDisplayCapability +from agentpool.capabilities.viking import VikingCapability +from agentpool.capabilities.viking.identity import VikingIdentity +from agentpool_config.capabilities import VikingCapabilityConfig, build_capability + + +if TYPE_CHECKING: + from agentpool.agents.events import DiffContentItem + + +pytestmark = pytest.mark.integration + + +def _make_mock_client() -> AsyncMock: + """Create a fully populated mock AsyncHTTPClient for the viking capability.""" + client = AsyncMock() + client.initialize = AsyncMock() + client.close = AsyncMock() + client.read = AsyncMock(return_value="file content") + client.write = AsyncMock(return_value={"status": "ok"}) + client.search = AsyncMock(return_value={"results": []}) + client.find = AsyncMock(return_value={"results": []}) + return client + + +def _build_viking_cap(client: AsyncMock) -> VikingCapability: + """Build a viking capability from config with mocked client.""" + config = VikingCapabilityConfig(type="viking", mode="write") + cap = build_capability(config) + assert isinstance(cap, VikingCapability) + cap._client = client + cap._identity = VikingIdentity(account_id="acc", user_id="user") # type: ignore[arg-type] + return cap + + +def _make_request_context(messages: list[Any]) -> ModelRequestContext: + """Build a minimal ModelRequestContext with a real TestModel.""" + return ModelRequestContext( + model=TestModel(), + messages=messages, + model_settings=None, + model_request_parameters=ModelRequestParameters( + function_tools=[], + native_tools=[], + ), + ) + + +def test_viking_tools_are_wrapped_after_assembly() -> None: + """Assembling viking + tool_display gives renamed tools via get_wrapper_toolset.""" + cap = _build_viking_cap(_make_mock_client()) + toolset = cap.get_toolset() + + display = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=True, + emit_diff_for={"viking_write", "viking_edit"}, + ) + + wrapped = display.get_wrapper_toolset(toolset) # type: ignore[arg-type] + assert wrapped is not None + assert isinstance(wrapped, RenamedToolset) + + +def test_assembled_rename_changes_tool_names() -> None: + """RenamedToolset applied over viking write-mode tools renames viking_write.""" + cap = _build_viking_cap(_make_mock_client()) + toolset = cap.get_toolset() + + # RenamedToolset renames ToolDefinitions during preparation; the wrapper + # carries the name_map regardless of runtime tool listing. + display = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=False, + ) + wrapped = display.get_wrapper_toolset(toolset) # type: ignore[arg-type] + assert wrapped is not None + + assert isinstance(wrapped, RenamedToolset) + # name_map is {original: display} in config, inverted to {display: original} for RenamedToolset + assert "write" in wrapped.name_map + + +def test_wrap_execute_passes_through_for_untouched_tool() -> None: + """A viking tool outside emit_diff_for executes unchanged with no event.""" + events = AsyncMock() + deps = MagicMock() + deps.events = events + ctx = MagicMock() + ctx.deps = deps + + display = ToolDisplayCapability(emit_diff=True, emit_diff_for={"viking_write"}) + handler = AsyncMock(return_value="viking_read result") + + import asyncio + + async def run() -> str: + return await display.wrap_tool_execute( # type: ignore[return-value] + ctx, + call=MagicMock(tool_name="viking_read", tool_call_id="c1"), + tool_def=MagicMock(name="viking_read"), + args={"uri": "viking://x.md"}, + handler=handler, # type: ignore[arg-type] + ) + + result = asyncio.run(run()) + assert result == "viking_read result" + handler.assert_awaited_once() + events.tool_call_progress.assert_not_awaited() + + +def test_wrap_execute_injects_diff_for_match() -> None: + """A viking tool in emit_diff_for gets a DiffContentItem event.""" + events = AsyncMock() + deps = MagicMock() + deps.events = events + ctx = MagicMock() + ctx.deps = deps + + display = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=True, + emit_diff_for={"viking_write", "viking_edit"}, + ) + handler = AsyncMock(return_value="Wrote 3 chars.") + + import asyncio + + async def run() -> str: + return await display.wrap_tool_execute( # type: ignore[return-value] + ctx, + call=MagicMock(tool_name="viking_write", tool_call_id="c1"), + tool_def=MagicMock(name="viking_write"), + args={"uri": "viking://x.md", "content": "abc"}, + handler=handler, # type: ignore[arg-type] + ) + + result = asyncio.run(run()) + assert result == "Wrote 3 chars." + events.tool_call_progress.assert_awaited_once() + items: list[DiffContentItem] = events.tool_call_progress.await_args.kwargs["items"] + assert items[0].path == "viking://x.md" + assert items[0].new_text == "abc" diff --git a/tests/capabilities/test_tool_display_semantics.py b/tests/capabilities/test_tool_display_semantics.py new file mode 100644 index 000000000..bbbf0b185 --- /dev/null +++ b/tests/capabilities/test_tool_display_semantics.py @@ -0,0 +1,208 @@ +"""Semantic tests for ToolDisplayCapability — rename output, event identity, cross-layer matching. + +These tests verify the three core behavioral contracts of the capability: + +1. **Rename produces display names**: ``get_tools()`` on the wrapped toolset + returns tools keyed by the *display* name, not the original. +2. **Injected events carry real tool_call_id**: ``wrap_tool_execute`` populates + ``ctx.deps.tool_call_id`` before emitting, so events aren't dropped by + downstream converters (capability tools skip ``tool_wrapping.py``). +3. **emit_diff_for matches across rename boundary**: when the model calls a + display name, the capability resolves it back to the original for + ``emit_diff_for`` matching. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock + +from pydantic_ai.messages import ToolCallPart +from pydantic_ai.tools import ToolDefinition +from pydantic_ai.toolsets import AbstractToolset, RenamedToolset +import pytest + +from agentpool.capabilities.tool_display_capability import ToolDisplayCapability + + +if TYPE_CHECKING: + from agentpool.agents.events import DiffContentItem + + +pytestmark = pytest.mark.unit + + +# --------------------------------------------------------------------------- +# Shared helpers +# --------------------------------------------------------------------------- + + +def _make_call(tool_name: str, tool_call_id: str = "call_123") -> ToolCallPart: + """Create a ToolCallPart with a real tool_call_id.""" + return ToolCallPart(tool_name=tool_name, args={}, tool_call_id=tool_call_id) + + +def _make_tool_def(name: str) -> ToolDefinition: + return ToolDefinition(name=name, description="test tool") + + +def _make_handler(result: Any) -> AsyncMock: + return AsyncMock(return_value=result) + + +class _FakeDeps: + """Lightweight stand-in for AgentContext. + + Simulates the capability-tool scenario where ``tool_wrapping.py`` + never runs, so ``tool_call_id`` and ``tool_name`` start as ``None``. + The ``events`` property returns whatever emitter is assigned. + """ + + def __init__(self, events: Any = None) -> None: + self.tool_call_id: str | None = None + self.tool_name: str | None = None + self._events = events or AsyncMock() + + @property + def events(self) -> Any: + return self._events + + +def _make_ctx(deps: Any = None) -> Any: + """Create a minimal ctx object with ``.deps``.""" + ctx = type("_Ctx", (), {})() + ctx.deps = deps or _FakeDeps() + return ctx + + +# --------------------------------------------------------------------------- +# 1. Rename produces display names in get_tools() +# --------------------------------------------------------------------------- + + +class TestRenameProducesDisplayNames: + """``RenamedToolset.get_tools()`` must return tools keyed by display names. + + ``name_map`` is documented as ``{original: display}`` (what the user + writes in YAML), but ``RenamedToolset`` expects ``{new: original}``. + The capability must invert before construction. + """ + + @pytest.mark.asyncio + async def test_get_tools_keys_by_display_name(self) -> None: + """A tool originally named 'viking_write' appears as 'write' after wrapping.""" + from pydantic_ai._run_context import RunContext + from pydantic_ai.models.test import TestModel + from pydantic_ai.tools import Tool + from pydantic_ai.toolsets import FunctionToolset + from pydantic_ai.usage import RunUsage + + async def viking_write(ctx: Any, uri: str, content: str) -> str: + return f"Wrote {len(content)} chars to {uri}." + + toolset: AbstractToolset[Any] = FunctionToolset(tools=[Tool(viking_write)]) # type: ignore[arg-type] + + cap = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=False, + ) + wrapped = cap.get_wrapper_toolset(toolset) + assert wrapped is not None + assert isinstance(wrapped, RenamedToolset) + + ctx = RunContext(deps=None, model=TestModel(), usage=RunUsage()) + tools = await wrapped.get_tools(ctx) + tool_names = set(tools.keys()) + + assert "write" in tool_names, ( + f"Expected 'write' in tool names after rename, got {tool_names}" + ) + assert "viking_write" not in tool_names, ( + f"'viking_write' should have been renamed away, got {tool_names}" + ) + + +# --------------------------------------------------------------------------- +# 2. Injected events carry real tool_call_id +# --------------------------------------------------------------------------- + + +class TestInjectedEventsCarryToolCallId: + """``wrap_tool_execute`` must set ``ctx.deps.tool_call_id`` before emitting. + + For capability tools (viking, fsspec, etc.) the legacy ``tool_wrapping.py`` + layer never runs, so ``ctx.deps.tool_call_id`` stays ``None``. Without + explicit population, ``StreamEventEmitter`` reads ``""`` and the ACP + converter's ``if tool_call_id:`` guard drops the event silently. + """ + + @pytest.mark.asyncio + async def test_populates_deps_tool_call_id_before_emit(self) -> None: + """After wrap_tool_execute, deps.tool_call_id matches call.tool_call_id.""" + deps = _FakeDeps() + ctx = _make_ctx(deps) + + cap = ToolDisplayCapability( + emit_diff=True, + emit_diff_for={"viking_write"}, + ) + + result = await cap.wrap_tool_execute( + ctx, + call=_make_call("viking_write", tool_call_id="call_abc"), + tool_def=_make_tool_def("viking_write"), + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("Wrote 3 chars."), + ) + + assert result == "Wrote 3 chars." + assert deps.tool_call_id == "call_abc", ( + f"Expected deps.tool_call_id='call_abc', got '{deps.tool_call_id}'" + ) + assert deps.tool_name == "viking_write", ( + f"Expected deps.tool_name='viking_write', got '{deps.tool_name}'" + ) + + +# --------------------------------------------------------------------------- +# 3. emit_diff_for matches across the rename boundary +# --------------------------------------------------------------------------- + + +class TestDiffMatchingAcrossRename: + """When rename is active, ``call.tool_name`` is the *display* name. + + ``emit_diff_for`` is configured with *original* names (the names the + user knows from the tool's source). The capability must resolve the + display name back to the original via ``name_map`` before checking + membership. + """ + + @pytest.mark.asyncio + async def test_display_name_resolves_to_original_for_matching(self) -> None: + """Model calls 'write' (display); emit_diff_for={'viking_write'} still matches.""" + events = AsyncMock() + deps = _FakeDeps(events=events) + ctx = _make_ctx(deps) + + cap = ToolDisplayCapability( + rename_mode=True, + name_map={"viking_write": "write"}, + emit_diff=True, + emit_diff_for={"viking_write"}, + ) + + result = await cap.wrap_tool_execute( + ctx, + call=_make_call("write", tool_call_id="call_xyz"), + tool_def=_make_tool_def("write"), + args={"uri": "viking://x.md", "content": "abc"}, + handler=_make_handler("Wrote 3 chars."), + ) + + assert result == "Wrote 3 chars." + events.tool_call_progress.assert_awaited_once() + items: list[DiffContentItem] = events.tool_call_progress.await_args.kwargs["items"] + assert items[0].path == "viking://x.md" + assert items[0].new_text == "abc" diff --git a/tests/messaging/test_tool_display_acp_diff.py b/tests/messaging/test_tool_display_acp_diff.py new file mode 100644 index 000000000..b04aa3d53 --- /dev/null +++ b/tests/messaging/test_tool_display_acp_diff.py @@ -0,0 +1,88 @@ +"""ACP Converter diff path — ToolDisplayCapability events as FileEditToolCallContent. + +Task 4.6: after an agent runs a decorated tool, the injected +``ToolCallProgressEvent(items=[DiffContentItem])`` must convert to an +ACP ``ToolCallProgress`` update carrying ``FileEditToolCallContent`` — +the ACP client (Zed) diff rendering path. +""" + +from __future__ import annotations + +import pytest + +from acp.schema import FileEditToolCallContent, ToolCallProgress +from agentpool.agents.events import DiffContentItem, ToolCallProgressEvent, ToolCallStartEvent +from agentpool_server.acp_server.event_converter import ACPEventConverter + + +pytestmark = pytest.mark.integration + + +async def collect_updates(converter: ACPEventConverter, event): + """Collect all ACP updates produced for a single event.""" + return [u async for u in converter.convert(event)] + + +@pytest.mark.anyio +async def test_diff_progress_converts_to_file_edit_content() -> None: + """ToolCallProgressEvent with a DiffContentItem yields FileEditToolCallContent. + + Exercises the same event shape ToolDisplayCapability.wrap_tool_execute + emits: a progress event carrying old/new text for a viking URI. + """ + converter = ACPEventConverter() + + # Seed the tool call state the converter tracks internally. + await collect_updates( + converter, + ToolCallStartEvent( + tool_call_id="call_1", + tool_name="viking_write", + title="Write viking://x.md", + ), + ) + + updates = await collect_updates( + converter, + ToolCallProgressEvent( + tool_call_id="call_1", + status="in_progress", + title="Modified: viking://x.md", + items=[ + DiffContentItem( + path="viking://x.md", + old_text=None, + new_text="new content", + ) + ], + ), + ) + + progress_updates = [u for u in updates if isinstance(u, ToolCallProgress)] + assert progress_updates, "expected at least one ToolCallProgress update" + progress = progress_updates[0] + assert progress.tool_call_id == "call_1" + assert progress.content is not None + file_edits = [c for c in progress.content if isinstance(c, FileEditToolCallContent)] + assert len(file_edits) == 1 + assert file_edits[0].path == "viking://x.md" + assert file_edits[0].old_text is None + assert file_edits[0].new_text == "new content" + + +@pytest.mark.anyio +async def test_converter_requires_no_start_for_rich_progress() -> None: + """A progress event without a prior start still converts safely.""" + converter = ACPEventConverter() + updates = await collect_updates( + converter, + ToolCallProgressEvent( + tool_call_id="orphan", + status="in_progress", + title="Modified: viking://x.md", + items=[DiffContentItem(path="viking://x.md", old_text="a", new_text="b")], + ), + ) + + # No crash; at minimum no updates or a progress update. + assert isinstance(updates, list) diff --git a/tests/servers/opencode_server/test_event_processor_diff.py b/tests/servers/opencode_server/test_event_processor_diff.py new file mode 100644 index 000000000..c324fd5de --- /dev/null +++ b/tests/servers/opencode_server/test_event_processor_diff.py @@ -0,0 +1,268 @@ +"""Tests for DiffContentItem → unified diff text in opencode event_processor. + +The opencode TUI Edit component reads ``metadata.diff`` as a unified diff +text string (produced by ``createTwoFilesPatch`` in opencode's edit.ts). +The agentpool event_processor must convert ``DiffContentItem`` from +``ToolCallProgressEvent`` into that text format and carry it through to +``ToolStateCompleted.metadata.diff`` so the TUI can render the diff view. +""" + +from __future__ import annotations + +import pytest + +from agentpool.agents.events import DiffContentItem, ToolCallProgressEvent +from agentpool.agents.events.events import ToolCallCompleteEvent, ToolCallStartEvent +from agentpool_server.opencode_server.event_processor import EventProcessor +from agentpool_server.opencode_server.event_processor_context import ( + EventProcessorContext, +) +from agentpool_server.opencode_server.models.message import ( + MessagePath, + MessageTime, + MessageWithParts, +) + + +def _make_ctx(server_state: object) -> EventProcessorContext: + """Create a minimal EventProcessorContext for tool diff tests.""" + assistant_msg = MessageWithParts.assistant( + message_id="msg_001", + session_id="test-diff-session", + time=MessageTime(created=0), + agent_name="test-agent", + model_id="test-model", + parent_id="parent-1", + provider_id="agentpool", + path=MessagePath(cwd="/tmp", root="/tmp"), + ) + return EventProcessorContext( + session_id="test-diff-session", + assistant_msg_id="msg_001", + assistant_msg=assistant_msg, + state=server_state, # type: ignore[arg-type] + working_dir="/tmp", + ) + + +class TestDiffContentItemToUnifiedDiff: + """DiffContentItem in ToolCallProgressEvent must produce metadata.diff.""" + + @pytest.mark.asyncio + async def test_write_diff_carries_to_complete( + self, + server_state: object, + ) -> None: + """DiffContentItem(old=None, new=content) → metadata.diff unified text. + + Simulates viking_write: tool start → progress with diff → complete. + The final ToolPart.state.metadata.diff must be a unified diff string. + """ + processor = EventProcessor() + ctx = _make_ctx(server_state) + + async def _feed(event: object) -> None: + async for _ in processor.process(event, ctx): # type: ignore[arg-type] + pass + + # Step 1: start + await _feed( + ToolCallStartEvent( + tool_call_id="call_diff_001", + tool_name="write", + raw_input={"file_path": "viking://test.md", "content": "new content"}, + title="Writing viking://test.md", + ) + ) + + # Step 2: progress with DiffContentItem + await _feed( + ToolCallProgressEvent( + tool_call_id="call_diff_001", + tool_name="write", + title="Modified: viking://test.md", + items=[ + DiffContentItem( + path="viking://test.md", + old_text=None, + new_text="new content", + ) + ], + ) + ) + + # Step 3: complete + await _feed( + ToolCallCompleteEvent( + tool_call_id="call_diff_001", + tool_name="write", + tool_input={"file_path": "viking://test.md", "content": "new content"}, + tool_result="Written successfully.", + agent_name="test-agent", + message_id="msg_001", + ) + ) + + # Assert: final ToolPart has metadata.diff as a unified diff text string + tool_part = ctx.get_tool_part("call_diff_001") + assert tool_part is not None, "ToolPart should exist after complete" + assert tool_part.state is not None + metadata = getattr(tool_part.state, "metadata", None) + assert metadata is not None, "metadata must be set on completed tool part" + diff = metadata.get("diff") + assert diff is not None, "metadata.diff must be present" + assert isinstance(diff, str), f"metadata.diff must be a string, got {type(diff)}" + assert "viking://test.md" in diff + assert "new content" in diff + # diagnostics=[] triggers Write component's code-block branch + assert metadata.get("diagnostics") == [] + + @pytest.mark.asyncio + async def test_edit_diff_carries_old_and_new( + self, + server_state: object, + ) -> None: + """DiffContentItem(old_text, new_text) → metadata.diff with both sides.""" + processor = EventProcessor() + ctx = _make_ctx(server_state) + + async def _feed(event: object) -> None: + async for _ in processor.process(event, ctx): # type: ignore[arg-type] + pass + + await _feed( + ToolCallStartEvent( + tool_call_id="call_diff_002", + tool_name="edit", + raw_input={ + "file_path": "viking://doc.md", + "old_string": "old", + "new_string": "new", + }, + title="Editing viking://doc.md", + ) + ) + + await _feed( + ToolCallProgressEvent( + tool_call_id="call_diff_002", + tool_name="edit", + title="Modified: viking://doc.md", + items=[ + DiffContentItem( + path="viking://doc.md", + old_text="old line", + new_text="new line", + ) + ], + ) + ) + + await _feed( + ToolCallCompleteEvent( + tool_call_id="call_diff_002", + tool_name="edit", + tool_input={"file_path": "viking://doc.md"}, + tool_result="Edited.", + agent_name="test-agent", + message_id="msg_001", + ) + ) + + tool_part = ctx.get_tool_part("call_diff_002") + assert tool_part is not None + metadata = getattr(tool_part.state, "metadata", None) + assert metadata is not None + diff = metadata.get("diff") + assert diff is not None + assert isinstance(diff, str) + assert "old line" in diff + assert "new line" in diff + # Unified diff should have removal and addition markers + assert "-old line" in diff or "-old" in diff + assert "+new line" in diff or "+new" in diff + + @pytest.mark.asyncio + async def test_diff_output_is_parseable_unified_diff( + self, + server_state: object, + ) -> None: + r"""Diff output must be parseable by strict unified diff parsers. + + The npm ``diff`` package's ``parsePatch`` (used by opencode TUI) + strictly validates hunk line counts against ``@@ -X,Y +A,B @@`` + headers. Missing ``\\n`` on the last content line causes + "Added line count did not match for hunk". + + Requirements: + - Every line properly ``\\n``-terminated (diff ends with ``\\n``) + - ``---`` and ``+++`` headers both use the file path (no ``(old)`` suffix) + """ + processor = EventProcessor() + ctx = _make_ctx(server_state) + + async def _feed(event: object) -> None: + async for _ in processor.process(event, ctx): # type: ignore[arg-type] + pass + + await _feed( + ToolCallStartEvent( + tool_call_id="call_diff_003", + tool_name="edit", + raw_input={"file_path": "viking://note.md"}, + title="Editing viking://note.md", + ) + ) + await _feed( + ToolCallProgressEvent( + tool_call_id="call_diff_003", + tool_name="edit", + title="Modified: viking://note.md", + items=[ + DiffContentItem( + path="viking://note.md", + old_text="old line", # No trailing \n — triggers the bug + new_text="new line", # No trailing \n + ) + ], + ) + ) + await _feed( + ToolCallCompleteEvent( + tool_call_id="call_diff_003", + tool_name="edit", + tool_input={"file_path": "viking://note.md"}, + tool_result="Edited.", + agent_name="test-agent", + message_id="msg_001", + ) + ) + + tool_part = ctx.get_tool_part("call_diff_003") + assert tool_part is not None + metadata = getattr(tool_part.state, "metadata", None) + assert metadata is not None + diff = metadata.get("diff") + assert diff is not None + assert isinstance(diff, str) + + # Bug 1: diff must end with \n (no dangling last line) + assert diff.endswith("\n"), f"Diff must end with \\n, got tail: {diff[-20:]!r}" + + # Bug 2: fromfile/tofile must both be the path (no "(old)" suffix) + assert "(old)" not in diff, f"Diff must not contain '(old)' suffix: {diff!r}" + assert "--- viking://note.md\n" in diff + assert "+++ viking://note.md\n" in diff + + # Bug 3: hunk line counts must match — verify by parsing + # Every non-header line in a hunk must start with ' ', '-', or '+' + hunk_lines = [ + line + for line in diff.split("\n") + if line + and not line.startswith("@@") + and not line.startswith("---") + and not line.startswith("+++") + ] + for line in hunk_lines: + assert line[0] in (" ", "-", "+"), f"Invalid hunk line prefix: {line!r}"