fix(acp): name the video tool the way an ACP client offers it (#2513)

[jonathanKingston]

Closes #2513 (P2, area:acp).

The report, and what was actually wrong

An ACP agent asked to inspect a screen recording said "the advertised video_frames reader isn't exposed in this session's tool set" and read the file with shell tooling instead.

The tool was exposed. I checked rather than assumed, and the issue's premise ("an ACP session does not receive it") is wrong:

• video_frames is registered unconditionally (registry-bootstrap.ts) — the per-turn withholding in parentTools is native-loop only.
• It is in BRIDGE_TOOL_NAMES (acp-native-bridge.ts), and was already at 370cb70, the commit the issue was triaged against.
• It survives the draft-2020-12 conversion that bridgedTools() filters on — I ran the real conversion to confirm, because toMcpTools() drops unconvertible schemas with nothing but a console.warn, which would have been a plausible culprit.

What the agent could not find was the name. The bridge mounts Copse's tools as an MCP server, so the client renames them. acp-bridge-name.ts already documents the three observed shapes:

client	name the agent sees
Cursor	copse-video_frames
Claude	mcp__copse__video_frames
Codex	mcp.copse.video_frames

The steering block said "Use the video_frames tool" — only ever literally true under the native loop. This is the same class of miss as #1659, where ^copse failed to match Claude's mcp__-prefixed titles.

The fix

The video block now tells the model to match on the name's ending and shows a namespaced example, and says to speak up when no such tool is offered rather than reach for the shell.

That second half is not new thinking. The archive block sitting directly beside it already carries exactly those sentences, added after an ACP run "quietly went off grepping" — its test says so in as many words. The video block simply never got them. Had it, the reported turn would have ended in a question instead of a video read with shell commands, which cannot decode a video and left the user believing it had been watched.

The archive block gets the ending/example hint too, because it has the same defect one line up: its hedge "if you have a read_archive tool" reads as false to an agent whose only matching entry is mcp__copse__read_archive. So the hedge that was meant to prevent silent failure was instead sending agents to ask for a tool they already had. Same root cause, adjacent constant — fixing one and not the other would leave the bug in place under a different name.

The drift seam

packages/agent cannot import from src/main by design (headless-contract.ts is explicit about staying free of main-process code), and build-text-with-attachments.ts has no imports at all. So the example name has to be a literal there, and could drift from the real server name.

src/main/services/acp/attachment-steering-names.test.ts is the seam. It lives in src/, where both sides are importable, and asserts the example is a string the bridge's own matchesBridgedToolName accepts — not merely a plausible-looking one — and that it carries BRIDGE_MCP_SERVER_NAME. Rename the server and a test fails, rather than stale copy sitting in front of every agent.

Verification

Check	Result
node scripts/run-tests.mts (whole repo)	8738 tests, 8728 pass, 10 skipped, 0 fail
6 new tests against the unfixed preambles	6 of 6 fail, as designed
tsc --noEmit (node + web)	clean
pnpm run lint / oxfmt --check .	clean
check:dead-code / check:oracle / check:e2e-syntax / demo:site:check	clean

Six new tests: the video block tells a model without the tool to speak up rather than shell out; both blocks name the namespaced form; and three seam tests binding the examples to the live bridge name.

Two existing assertions changed. emits one steering preamble + a line per {video,archive} counted raw occurrences of the tool name and expected exactly one; the preamble now names each tool three times deliberately. They count the preamble's own opening line instead, which is what "one preamble however many attachments" actually means — the property the test is named for is preserved, not weakened.

Reaching a full local run needs an uncommitted shim over node-pty's native loader, reverted before committing.

One note for the reviewer: this fixes the instruction. If you would rather the block never name a client-specific example at all, the alternative is to drop the example and keep only "look for a tool whose name ends in video_frames" — less actionable for the model, but it would let the seam test go away.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6

---

Generated by Claude Code