feat(capabilities): add ToolDisplayCapability for tool rename + diff event injection

[Leoyzen]

Summary

New global decorator capability ToolDisplayCapability that enables protocol clients (OpenCode TUI, ACP/Zed) to render file-change diff views for third-party capabilities (e.g. viking) — without modifying sub-capability sources and with zero protocol changes.

Motivation

viking write/edit tools write to a remote store and return plain text. OpenCode TUI renders diffs by a hardcoded tool-name whitelist; ACP renders via FileEditToolCallContent. Neither matched viking tools since they are custom-named and carry no diff info.

Approach

Aligned with the existing ToolInterceptCapability pattern: a standalone AbstractCapability overriding get_wrapper_toolset() + wrap_tool_execute(), acting as a global middleware over all assembled tools (no child-capability composition).

Three orthogonal switches:

• rename_mode + name_map: rename tools via pydantic-ai official RenamedToolset → hits the TUI whitelist (viking_write→write)
• emit_diff + emit_diff_for: after real execution, inject ToolCallProgressEvent with DiffContentItem (old/new from args) → flows through EventBus → ACP FileEditToolCallContent (proven path, same as fsspec toolset)
• Degenerates to a no-op decorator when both disabled

Protocol distinction via assembly-time config: opencode rename_mode:true+emit_diff:true, ACP rename_mode:false+emit_diff:true, self-emitting capabilities (fsspec) emit_diff:false.

Files

• src/agentpool/capabilities/tool_display_capability.py — the capability (+ _parse_diff_fields helper)
• entry-point registration in pyproject.toml
• 12 unit + 4 integration + 2 ACP end-to-end converter tests (all passing; full suite 6438 passed, only pre-existing test_vision failure unrelated — ALLOW_MODEL_REQUESTS=False)
• docs/explanation/tool-display-capability.md + 3-scenario example YAML + AGENTS.md entry

Verification

• ruff check / ruff format --check / mypy --strict all green
• Full pytest: 6438 passed, 1 failed (test_vision, pre-existing, needs real model — confirmed failing on main too)

[opencode-agent]

I have verified the critical findings directly against the pinned pydantic-ai API, the ACP converter, and the manifest schema. Here is my synthesized review.

---

Review: feat/capabilities: add ToolDisplayCapability

The design is sound (global decorator aligned with ToolInterceptCapability, orthogonal switches, clean no-op degradation, entry-point registration consistent with siblings), and the architecture wiring is correct — verified that config capabilities flow through build_config_capabilities → PydanticAgent(capabilities=...), and pydantic-ai invokes get_wrapper_toolset/wrap_tool_execute. The event pipeline claims in the docs are also accurate (DiffContentItem@events.py:203, DiffContentItem → FileEditToolCallContent@event_converter.py:780).

However, two high-severity functional bugs break the PR's two primary scenarios, and the example YAML won't load under the real schema. Request changes.

BLOCKING — functional

1. name_map orientation is inverted → rename layer silently does nothing.
tool_display_capability.py:126 passes name_map (documented as native→display, {viking_write: write}) straight into RenamedToolset. But pydantic-ai's RenamedToolset.name_map maps new→original (display→native): get_tools builds original_to_new = {v: k for k, v in self.name_map.items()} and call_tool resolves original_name = name_map.get(name) (verified in pydantic-ai-slim 2.22.0 toolsets/renamed.py:14,22,46, satisfies the repo's >=2.12.0 pin). So {viking_write: write} renames a tool literally named write to viking_write and leaves viking_write untouched. The OpenCode TUI whitelist scenario is broken. No test catches it — both rename tests only assert the map is stored verbatim ("viking_write" in wrapped.name_map), never the resulting toolset. Fix: invert before construction ({v: k for k, v in name_map.items()}) or redefine config semantics, and add a test asserting actual get_tools() output.

2. Diff events carry empty tool_call_id for capability tools → dropped on the ACP path.
wrap_tool_execute emits via ctx.deps.events.tool_call_progress(...) (:168-182). AgentContext.events builds a fresh emitter that reads self._context.tool_call_id/tool_name (event_emitter.py:117-124). Those fields are only populated by tool_wrapping.py:113-114, which is applied only to legacy direct tools (agent.py:1044-1052) — not to AbstractCapability tools like viking, the PR's primary use case. For viking, ctx.deps.tool_call_id is None → event carries "" → the ACP converter's guard case ToolCallProgressEvent(...) if tool_call_id: (event_converter.py:740) fails and the event is silently dropped (falls through to case _: at :1136). The fsspec precedent works only because it receives agent_ctx as an explicit per-call param (toolset.py:573). Tests miss this because events is always AsyncMock and tool_call_id is never asserted. Fix: set ctx.deps.tool_call_id/tool_name from call before emitting, or emit a ToolCallProgressEvent(tool_call_id=call.tool_call_id) via events.emit_event(...).

3. Rename + diff combo is namespace-incoherent (untested). Even after fixing #1, the model calls display names, so call.tool_name (write) never matches emit_diff_for={"viking_write", ...} and no diff fires — the docs' scenario 1. Resolve the original name through the (inverted) map before filtering, or require display names in emit_diff_for, and add a combined-mode test.

HIGH — example YAML is not loadable

docs/tool-display-capability.example.yaml uses a top-level agent: (singular) + name: — but AgentsManifest requires agents: dict keyed by id with type: native (manifest.py:131; cf. docs/tutorials/examples/round_robin/config.yml). Also: model: {provider, model} has no discriminator/field (AnyModelConfig expects a string like anthropic:claude-sonnet-4-5 or {type, identifier}); type: viking wrapped in args: with server: doesn't match VikingCapabilityConfig (fields are direct, URL key is url:); and type: fsspec is not a capability (fsspec mounts via tools: - type: file_access). As written, all three examples fail validation or silently drop fields.

MEDIUM — red lines / conventions

• getattr(ctx, "deps", None) / getattr(deps, "events", None) violate the "No getattr/hasattr" red line, and ctx: Any + AbstractCapability[Any] defeats static checking (:130,168-169). Type ctx: RunContext[Any] like tool_intercept.py:105.
• emit_diff_for: set[str] is a runtime type lie: EntryPointCapabilityConfig.build() does cls(**args) (capabilities.py:342) with no list→set coercion, so a YAML list stays a list. Annotate list[str] or coerce in __post_init__.
• 31 # type: ignore in the new tests — tests/AGENTS.md explicitly calls this an anti-pattern ("If you can't type a test properly, the interface under test is probably wrong").
• No logfire instrumentation on a tool-execution critical path, despite "span instrumentation is mandatory on … capabilities". Inconsistent with resource_capability.py/subagent_capability.py.
• "2 ACP end-to-end" is inaccurate: both test_tool_display_acp_diff.py tests construct ACPEventConverter() directly with synthetic events (no EventBus/agent). The "integration" file uses MagicMock ctx/deps and calls wrap_tool_execute directly — unit tests mislabeled; no real assembly or EntryPointCapabilityConfig(type="tool_display").build() test, no agent run. L1/L2 requirement not genuinely met.
• src/agentpool/capabilities/AGENTS.md "Where to Look" table not updated; new page missing from mkdocs.yml nav; doc is entirely Chinese while docs/explanation/ is English.
• id docstring claims default "tool_display" but field default is None (:105,108).

LOW

• test_get_wrapper_toolset_applies_rename is @pytest.mark.asyncio on a sync function; test_assembled_rename_changes_tool_names / test_empty_config_is_noop_decorator assert nothing behavioral; import asyncio inside function bodies; fsspec path omits the src/ prefix (agentpool_toolsets/…:575 → src/agentpool_toolsets/…:575; the line number itself is correct).

Bottom line

Strong concept and well-documented intent, but the rename layer is inverted against the pydantic-ai API, the diff-injection path emits events the ACP converter drops for capability tools (viking), the combined rename+diff mode can't fire as configured, and the shipped example YAML doesn't validate. Recommend addressing #1–#3 plus the example YAML before merge, and adding genuine assembly-level tests.

opencode session  |  github run