From 386ee14af356bcfe51cedd0b2f9d9f50eded9339 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:36:02 +0800 Subject: [PATCH 1/7] docs(acp): RFC for the terminal-card rendering (implemented design) Records the verified design before implementing: keep dsh-bash agent-side execution and render Zed's terminal tool-call card via the `_meta` convention (terminal_info/terminal_output/terminal_exit), capability-gated on clientCapabilities._meta.terminal_output, with the ```console text block as the no-capability fallback. Rejects the spec's client-side terminal/create path (it would bypass dsh-bash's sandbox/env-scrub/ownership/cwd). Studied claude-agent-acp, codex-acp, and Zed's renderer to ground the wire contract. Live streaming and command classification are noted as separate follow-ups. --- docs/rfc/README.md | 2 +- ...6-06-18-acp-terminal-and-tool-rendering.md | 40 +++++++++++++++ ...6-06-18-acp-terminal-and-tool-rendering.md | 49 ------------------- packages/acp/README.md | 2 +- 4 files changed, 42 insertions(+), 51 deletions(-) create mode 100644 docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md delete mode 100644 docs/rfc/proposed/2026-06-18-acp-terminal-and-tool-rendering.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 90fd5fe6d4..bc56275477 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -31,7 +31,6 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Multiplex concurrent ACP sessions over one connection](proposed/2026-06-14-acp-multi-session.md) | 2026-06-14 | | [Optional Code Mode — model writes TypeScript against an SDK of all tools](proposed/2026-06-15-optional-code-mode.md) | 2026-06-15 | | [Runtime schemas for the event vocabulary (Zod vs the merge-extensible-map pattern)](proposed/2026-06-16-typed-event-schemas.md) | 2026-06-16 | -| [Rich ACP bash rendering — the terminal sub-protocol and command classification](proposed/2026-06-18-acp-terminal-and-tool-rendering.md) | 2026-06-18 | ## Implemented @@ -56,6 +55,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Session persistence as an abstract service over `SessionEvent`](implemented/2026-06-14-session-persistence.md) | 2026-06-14 | | [Every session event is enclosed in a turn](implemented/2026-06-15-turn-enclosure-invariant.md) | 2026-06-15 | | [pnpm as the package manager instead of Yarn 4](implemented/2026-06-16-pnpm-over-yarn.md) | 2026-06-16 | +| [Rich ACP bash rendering — the terminal card (`_meta`) and command classification](implemented/2026-06-18-acp-terminal-and-tool-rendering.md) | 2026-06-18 | ## Rejected diff --git a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md new file mode 100644 index 0000000000..b1588461f0 --- /dev/null +++ b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md @@ -0,0 +1,40 @@ +# RFC: Rich ACP bash rendering — the terminal card via the `_meta` convention + +Status: implemented + +## Problem + +The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../proposed/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the model's `description` plus the command as the `tool_call` title, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block. + +That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, and the command output rendered as a terminal — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why our command rides inside the title.) + +## Key finding: agent-executed terminals use a `_meta` convention, NOT `terminal/create` + +The ACP spec has a *client-side* terminal sub-protocol — the agent calls the client's `terminal/create` with `{ command, args, cwd, env }` and the **editor** executes the process, then the agent reads `terminal/output` / `wait_for_exit`. That model is wrong for us: our harness executes bash itself through `dsh-bash` (sandboxed env-scrub, background-task ownership, per-session cwd). Routing execution to the editor would bypass all of that and fork execution into two backends. + +Studying the two reference agents (2026-06-18) shows neither uses `terminal/create` for their own shell tool — **both keep agent-side execution and emit a `_meta` convention** that Zed special-cases: + +- **`claude-agent-acp`** (`tools.ts`, `acp-agent.ts`): gated on `clientCapabilities._meta.terminal_output`. The `tool_call` carries `content: [{ type: 'terminal', terminalId }]` and `_meta.terminal_info.{ terminal_id, cwd }`; output/exit arrive on the `tool_call_update`'s `_meta.terminal_output.{ terminal_id, data }` and `_meta.terminal_exit.{ terminal_id, exit_code, signal }`. +- **`codex-acp`** (`CodexToolCallMapper.ts`, `TerminalOutputMode.ts`): same `terminal_info` on the call; output via `_meta.terminal_output` (full) or `_meta.terminal_output_delta` (incremental), selected from the same `_meta.terminal_output` capability. + +Zed's side (`crates/agent_servers/src/acp.rs`, verified): on a `ToolCall` whose `_meta.terminal_info.terminal_id` is set, it registers a **display-only** terminal (header = `terminal_info.cwd`, label = `tool_call.title`); on a `ToolCallUpdate`, `_meta.terminal_output.data` writes to that terminal and `_meta.terminal_exit.{exit_code,signal}` sets the status. It advertises the capability as `clientCapabilities._meta.terminal_output = true`. This is an off-spec `_meta` extension, but it is the de-facto contract for the Zed integration and the only way to get the terminal card while keeping execution agent-side. + +## Decision + +Keep `dsh-bash` agent-side execution; render the terminal card via the `_meta` convention, capability-gated, with the ` ```console ` text block as the guaranteed fallback. + +1. **Capability.** `initialize` reads `clientCapabilities._meta.terminal_output` and the bridge remembers it per connection. +2. **Neutral presentation vocabulary.** `dsh-tools` gains a terminal-shaped presentation a tool can return — provider-neutral (`cwd`, the output `data`, an `exitCode`/`signal`), NO ACP types. `dsh-tool-bash` returns it for `bash` (cwd from the resolved workdir; output + exit parsed from the run result). +3. **Bridge mapping.** When the client advertised the capability, the bridge maps that presentation to: on `tool_call`, `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id,cwd}`; on `tool_call_update`, `_meta.terminal_output.{terminal_id,data}` + `_meta.terminal_exit.{terminal_id,exit_code,signal}`. `terminalId` is derived from the harness `callId` (stable, unique per call). When the capability is absent, the bridge uses the existing ` ```console ` text content — unchanged. +4. **No new execution path, no live streaming.** Output is attached at completion (from the agent's own `tool/result`), not streamed token-by-token. Disposal is unaffected: nothing new to tear down, since the bridge never creates a client-side terminal. + +## Risks / trade-offs + +- **Off-spec `_meta`.** The terminal card rides on a Zed-specific `_meta` extension, not the ACP terminal sub-protocol. A client that doesn't recognize it still gets the text fallback (the capability gate ensures we only emit it when the client opted in via `_meta.terminal_output`), so a non-Zed client is never worse off. If ACP later standardizes agent-executed terminals, migrate to that and drop the `_meta`. +- **Capability honesty.** Emit terminal metadata ONLY when the client advertised `_meta.terminal_output`; the text fallback is the contract for everyone else and must never regress. Covered by a no-capability test asserting the ` ```console ` path. +- **terminalId collisions.** Deriving it from the per-call `callId` keeps it unique within a session and stable across the call/result pair; never reuse one across calls. +- **Provider-neutral vocabulary creep.** The terminal presentation widens the `dsh-tools` surface; keep it neutral (no ACP types leak into `dsh-tools`) and only as rich as a second UI consumer would also want. + +## Out of scope / non-goals + +The text-block baseline stays the no-capability default. Client-side `terminal/create` execution is explicitly rejected (it bypasses `dsh-bash`). Two follow-ups are deliberately NOT built here and would each warrant their own RFC when someone takes them on: **live incremental streaming** (`_meta.terminal_output_delta` as chunks arrive, which needs an incremental-output seam on `dsh-bash`), and **command classification** (parsing a `cat`/`sed` as a `read` card with a file location, a `grep` as a `search`, etc., falling back to the terminal card — display-only, must never change what executes). diff --git a/docs/rfc/proposed/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/proposed/2026-06-18-acp-terminal-and-tool-rendering.md deleted file mode 100644 index e138609db0..0000000000 --- a/docs/rfc/proposed/2026-06-18-acp-terminal-and-tool-rendering.md +++ /dev/null @@ -1,49 +0,0 @@ -# RFC: Rich ACP bash rendering — the terminal sub-protocol and command classification - -Status: proposed - -## Problem - -The ACP bridge now lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the model's `description` plus the command as the `tool_call` title, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block. - -That is a correct, capability-free MVP, but it is not how the reference editors render a *terminal* tool at its best. Two gaps: - -1. **No live terminal card.** An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command, a copy button, and **streaming** output with an exit-status pill — but it only uses that card when the `tool_call`'s `content` is an ACP `terminal` block (`{ type: 'terminal', terminalId }`), not a text block. With a text block the command output appears only as static markdown *after the turn completes*; there is no live stream and no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why our command currently has to ride inside the title.) - -2. **No command classification.** A bash invocation is opaque — `bash -lc "sed -n 1,40p foo.ts"` is really a file read, `rg foo` is a search. The reference adapters classify common commands and present them with a *semantic* kind/title/locations (a `read` card titled "Read file 'foo.ts'" with a follow-along file location, a `search` card), falling back to a terminal card only for an unrecognized command. This is what makes one bash call render with a search icon and "List …" while the next renders as a raw terminal. - -## What the reference adapters do (studied 2026-06-18) - -- **`codex-acp`** (`CodexToolCallMapper.ts`): classifies each command into `commandActions`. A recognized action maps to a semantic update — `read` → `{kind:'read', title:"Read file '…'", locations:[{path}]}`, `search` → `{kind:'search', title:"Search for '…' in …"}`, `listFiles` → `{kind:'read', title:"List files in '…'"}`. An `unknown` action becomes a terminal card: `{kind:'execute', title: stripShellPrefix(command), content:[{type:'terminal', terminalId}], _meta:{terminal_info:{cwd, terminal_id}}}`. The `_meta.terminal_info.cwd` is what renders the working directory as the card header. -- **`claude-agent-acp`** (`tools.ts`): gates on `clientCapabilities._meta.terminal_output`. WITH it: a terminal content block plus `_meta.terminal_{info,output,exit}` (output + exit code). WITHOUT it: the same fenced ` ```console ` text-block fallback this bridge ships today. Title is the command; the model's `description` (when present) is shown as content. -- **Zed** (`crates/agent_ui/.../thread_view.rs`, `crates/acp_thread/.../acp_thread.rs`): `render_terminal_tool_call` reads the terminal's `working_dir` as the header and `tool_call.label` (the title) as the command; a non-terminal text `content` block renders via `render_markdown_output`. `should_show_raw_input = !is_terminal_tool` confirms `rawInput` is suppressed for execute-kind cards. - -The full terminal experience is an ACP **sub-protocol**, not just a content shape: the client advertises a terminal capability, and the agent drives `terminal/create` → streams via `terminal/output` → `terminal/release`, attaching the `terminalId` to the `tool_call` content. That is a cross-seam feature (bridge ⇄ `dsh-bash` executor ⇄ client), which is why it is deferred to this RFC rather than folded into the presentation-seam PR. - -## Proposal - -Two independent, separately shippable pieces. Both build on the existing tool-owned presentation seam — neither reintroduces tool-name special-casing in the bridge. - -### A. Terminal content type + cwd metadata (capability-gated) - -1. In `initialize`, read the client's terminal capability (`clientCapabilities.terminal` / the `_meta.terminal_output` convention the references use) and remember it per connection. -2. Extend the `dsh-tools` presentation vocabulary so a tool can ask for a terminal rendering — e.g. a `ToolResultPresentation`/`ToolCallPresentation` variant carrying `{ kind: 'terminal', cwd, terminalId? }` (provider-neutral; the bridge maps it to the ACP `terminal` content block + `_meta.terminal_info`). `dsh-tool-bash` returns it for `bash` when a cwd is known. -3. When the client supports it, the bridge maps that to `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{cwd,terminal_id}`; otherwise it keeps the current ` ```console ` text fallback. The fenced-text path stays the guaranteed baseline. -4. *(Stretch)* drive live streaming through the real `terminal/*` methods so output appears as it is produced, with an exit-status pill — this needs a streaming seam on `dsh-bash` (the executor already has the process; it would push incremental output to the bridge). Scope this as a follow-up sub-step; steps 1–3 already give the cwd-header card with output attached at completion. - -### B. Command classification (capability-free) - -A small, pure classifier (in `dsh-tool-bash`, since it owns the bash schema) maps a command string to an optional semantic presentation: detect common read/search/list shapes (`cat`/`sed -n`/`head`/`tail` → `read` + a `path` location; `grep`/`rg` → `search`; `ls` → list) and return the richer `ToolCallPresentation` (`kind`, a human title, `locations`). Anything unrecognized falls through to the current execute/terminal presentation. This needs a `locations?: ToolCallLocation[]`-style field on `ToolCallPresentation` (neutral `{ path, line? }`), which the bridge maps to ACP `tool_call.locations` to drive editor "follow-along". - -Classification is best-effort and explicitly fallible: a misparse must degrade to the plain terminal card, never mislabel destructively (e.g. never title a `rm` as a "read"). Keep the matcher conservative and unit-test each recognized shape plus the fallthrough. - -## Risks / trade-offs - -- **Terminal sub-protocol is cross-seam and stateful.** Live streaming couples the bridge, the `dsh-bash` executor, and the client's terminal lifecycle; getting disposal/cancel right (release the terminal on turn end, abort, and disconnect) is the hard part — it must honor the same quiescence rules as the rest of the bridge. Steps A1–A3 (static cwd header + output at completion) are low-risk; A4 (live streaming) is where the lifecycle complexity lives. -- **Capability detection must stay honest.** Advertise/emit terminal content only when the client opted in; the text fallback is the contract for everyone else, so it must never regress. -- **Classification can mislead.** A wrong guess is worse than no guess. Bias to the terminal fallback; treat the classifier as additive polish, not a correctness path. (Security note: classification is display-only — it must never change what actually executes.) -- **Provider-neutral vocabulary creep.** Adding `terminal`/`locations` to `ToolCallPresentation` widens the `dsh-tools` surface. Keep the additions neutral (no ACP types leak into `dsh-tools`) and only as rich as a second consumer would also want. - -## Out of scope / non-goals - -The MVP shipped in the tool-call-UI PR (description+command title, `kind:'execute'`, ` ```console ` output fallback) stays the baseline and the no-capability default. This RFC is purely additive polish on top of it. diff --git a/packages/acp/README.md b/packages/acp/README.md index 3cc54dff3b..c86be26994 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -46,7 +46,7 @@ How a tool call renders in the editor is owned by the TOOL, not the bridge — t The `tool/result` session event carries only `{ callId, content, isError }` — not the tool name or args — so to call a tool's `presentResult` the bridge keeps a small per-session map from `callId` to the in-flight call's `(name, args)`, populated on `tool/call` and removed as each result is presented (it holds only currently-in-flight calls, never finished ones). This is bridge-local state — NOT a change to the event schema or a core service. The map lives on the `SessionRecord`, so two concurrent sessions never cross their in-flight tool state; a `session/load` replay uses a throwaway presenter that pairs each `tool/call` with its `tool/result` as the log replays in order, so replayed tool cards render identically to live ones. -A richer rendering — the ACP **terminal** content type (a live cwd-header terminal card with streaming output) and command classification (a `cat` shown as a `read`, a `grep` as a `search`) — is a capability-gated follow-up; the ` ```console ` text block here is the guaranteed baseline for clients without the terminal capability. See [the terminal-rendering RFC](../../docs/rfc/proposed/2026-06-18-acp-terminal-and-tool-rendering.md). +A richer rendering — the ACP **terminal** content type (a live cwd-header terminal card with streaming output) and command classification (a `cat` shown as a `read`, a `grep` as a `search`) — is a capability-gated follow-up; the ` ```console ` text block here is the guaranteed baseline for clients without the terminal capability. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). ## Settle-exactly-once From 149ab1bba4c3eeee8d9970531c697c3ce9cae5b9 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 17:25:09 +0800 Subject: [PATCH 2/7] feat(acp): render bash as a terminal card via the _meta convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the client advertises clientCapabilities._meta.terminal_output (Zed), a bash tool call now renders as a real TERMINAL card — a cwd header + the command + its output — instead of the plain ```console text block. Keeps agent-side dsh-bash execution; rejects the spec's client-side terminal/create (which would bypass sandbox/env-scrub/ownership/cwd). Matches what claude-agent-acp and codex-acp do; wire contract verified against Zed's source. - dsh-tools: a provider-neutral ToolTerminal shape ({ cwd?, output? }) on ToolCallPresentation/ToolResultPresentation — a tool asks "render me as a terminal"; no ACP types leak in. - dsh-tool-bash: bash presentCall marks terminal (cwd from an explicit absolute workdir, else left for the bridge to fill from the session cwd); presentResult carries the output alongside the ```console fallback. - dsh-acp: initialize reads/remembers the _meta.terminal_output capability; streamSessionEventUpdate maps a terminal presentation to content:[{type:'terminal',terminalId}] + _meta.terminal_info on the call and _meta.terminal_output on the update WHEN capable — else the unchanged text path. terminalId is the callId; cwd defaults to the session header. The pure translator gained a TerminalRendering {enabled,cwd} param (off by default). Tests via the REAL tool-bash + bash-local: capability ON -> terminal content + _meta; OFF -> no _meta (text path). The with-key e2e adds a real-model terminal card case (echo over ACP with the capability on). 773 tests, 100% coverage. The exit-status pill (_meta.terminal_exit), live streaming (_meta.terminal_output_delta), and command classification are RFC follow-ups. --- ...6-06-18-acp-terminal-and-tool-rendering.md | 6 +- examples/acp-agent/tests/acp.e2e.ts | 31 ++++++++ packages/acp/README.md | 9 ++- packages/acp/src/index.ts | 74 ++++++++++++++++++- packages/acp/tests/turns.spec.ts | 37 +++++++++- packages/tool-bash/README.md | 2 +- packages/tool-bash/src/index.ts | 37 +++++++--- packages/tool-bash/tests/tools.spec.ts | 24 ++++-- packages/tools/README.md | 4 +- packages/tools/src/index.ts | 30 ++++++++ 10 files changed, 224 insertions(+), 30 deletions(-) diff --git a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md index b1588461f0..7817b1e441 100644 --- a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md +++ b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md @@ -25,8 +25,8 @@ Keep `dsh-bash` agent-side execution; render the terminal card via the `_meta` c 1. **Capability.** `initialize` reads `clientCapabilities._meta.terminal_output` and the bridge remembers it per connection. 2. **Neutral presentation vocabulary.** `dsh-tools` gains a terminal-shaped presentation a tool can return — provider-neutral (`cwd`, the output `data`, an `exitCode`/`signal`), NO ACP types. `dsh-tool-bash` returns it for `bash` (cwd from the resolved workdir; output + exit parsed from the run result). -3. **Bridge mapping.** When the client advertised the capability, the bridge maps that presentation to: on `tool_call`, `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id,cwd}`; on `tool_call_update`, `_meta.terminal_output.{terminal_id,data}` + `_meta.terminal_exit.{terminal_id,exit_code,signal}`. `terminalId` is derived from the harness `callId` (stable, unique per call). When the capability is absent, the bridge uses the existing ` ```console ` text content — unchanged. -4. **No new execution path, no live streaming.** Output is attached at completion (from the agent's own `tool/result`), not streamed token-by-token. Disposal is unaffected: nothing new to tear down, since the bridge never creates a client-side terminal. +3. **Bridge mapping.** When the client advertised the capability, the bridge maps that presentation to: on `tool_call`, `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id,cwd}`; on `tool_call_update`, `_meta.terminal_output.{terminal_id,data}` (the captured output). `terminalId` is derived from the harness `callId` (stable, unique per call). When the capability is absent, the bridge uses the existing ` ```console ` text content — unchanged. +4. **No new execution path, no live streaming, no exit pill yet.** Output is attached at completion (from the agent's own `tool/result`), not streamed token-by-token. The exit-status pill (`_meta.terminal_exit.{exit_code,signal}`) is NOT emitted: it needs a structured exit code the pure `presentResult(args, result)` seam doesn't get (the result is content blocks), and the exit is already visible in the output text's `[exit code: N]` / `[killed by signal: …]` marker. Disposal is unaffected: nothing new to tear down, since the bridge never creates a client-side terminal. ## Risks / trade-offs @@ -37,4 +37,4 @@ Keep `dsh-bash` agent-side execution; render the terminal card via the `_meta` c ## Out of scope / non-goals -The text-block baseline stays the no-capability default. Client-side `terminal/create` execution is explicitly rejected (it bypasses `dsh-bash`). Two follow-ups are deliberately NOT built here and would each warrant their own RFC when someone takes them on: **live incremental streaming** (`_meta.terminal_output_delta` as chunks arrive, which needs an incremental-output seam on `dsh-bash`), and **command classification** (parsing a `cat`/`sed` as a `read` card with a file location, a `grep` as a `search`, etc., falling back to the terminal card — display-only, must never change what executes). +The text-block baseline stays the no-capability default. Client-side `terminal/create` execution is explicitly rejected (it bypasses `dsh-bash`). Three follow-ups are deliberately NOT built here and would each warrant their own RFC when someone takes them on: the **exit-status pill** (`_meta.terminal_exit.{exit_code,signal}`, which needs the structured exit surfaced from the run rather than parsed out of the rendered output text), **live incremental streaming** (`_meta.terminal_output_delta` as chunks arrive, which needs an incremental-output seam on `dsh-bash`), and **command classification** (parsing a `cat`/`sed` as a `read` card with a file location, a `grep` as a `search`, etc., falling back to the terminal card — display-only, must never change what executes). diff --git a/examples/acp-agent/tests/acp.e2e.ts b/examples/acp-agent/tests/acp.e2e.ts index caab2b0f4a..4a43429eb3 100644 --- a/examples/acp-agent/tests/acp.e2e.ts +++ b/examples/acp-agent/tests/acp.e2e.ts @@ -190,5 +190,36 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over expect(bashCall.title.length).toBeGreaterThan(0) expect(bashCall.title).not.toBe('bash') // the old, unhelpful title expect(typeof bashCall.rawInput).toBe('string') // the exact command + // Capability OFF: no terminal _meta — the ```console text path renders. + expect((bashCall as { _meta?: unknown })._meta).toBeUndefined() + }, 180_000) + + it('with the terminal_output capability, a real bash call renders as a terminal card (content + _meta)', async () => { + workdir = await mkdtemp(join(tmpdir(), 'acp-e2e-')) + spawned = spawnAcpAgent(workdir) + const { client, updates } = spawned + + // Advertise the Zed `_meta.terminal_output` capability so the bridge emits + // the terminal card for the real bash tool. + await client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: { _meta: { terminal_output: true } } }) + const { sessionId } = await client.newSession({ cwd: workdir, mcpServers: [] }) + const res = await client.prompt({ + sessionId, + prompt: [{ type: 'text', text: 'Use the bash tool to run: echo ACP_TERMINAL_OK. Then stop.' }], + }) + expect(['end_turn', 'max_tokens']).toContain(res.stopReason) + + // A bash tool_call now carries a terminal content block + _meta.terminal_info + // with the session cwd as the header; the matching update streams the output + // on _meta.terminal_output. + const bashCall = updates.find(u => u.sessionUpdate === 'tool_call' && u.kind === 'execute') + if (bashCall?.sessionUpdate !== 'tool_call') throw new Error('expected an execute tool_call') + const block = bashCall.content?.[0] as { type: string; terminalId?: string } | undefined + expect(block?.type).toBe('terminal') + expect(typeof block?.terminalId).toBe('string') + const info = (bashCall._meta as { terminal_info?: { terminal_id: string; cwd?: string } }).terminal_info + expect(info?.cwd).toBe(workdir) + const updatesForTerminal = updates.filter(u => u.sessionUpdate === 'tool_call_update' && (u._meta as { terminal_output?: unknown } | undefined)?.terminal_output !== undefined) + expect(updatesForTerminal.length).toBeGreaterThan(0) }, 180_000) }) diff --git a/packages/acp/README.md b/packages/acp/README.md index c86be26994..3cb311a43e 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -46,7 +46,14 @@ How a tool call renders in the editor is owned by the TOOL, not the bridge — t The `tool/result` session event carries only `{ callId, content, isError }` — not the tool name or args — so to call a tool's `presentResult` the bridge keeps a small per-session map from `callId` to the in-flight call's `(name, args)`, populated on `tool/call` and removed as each result is presented (it holds only currently-in-flight calls, never finished ones). This is bridge-local state — NOT a change to the event schema or a core service. The map lives on the `SessionRecord`, so two concurrent sessions never cross their in-flight tool state; a `session/load` replay uses a throwaway presenter that pairs each `tool/call` with its `tool/result` as the log replays in order, so replayed tool cards render identically to live ones. -A richer rendering — the ACP **terminal** content type (a live cwd-header terminal card with streaming output) and command classification (a `cat` shown as a `read`, a `grep` as a `search`) — is a capability-gated follow-up; the ` ```console ` text block here is the guaranteed baseline for clients without the terminal capability. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). +## Terminal card (capability-gated) + +A tool whose call IS a shell command (`bash`) can render as a real **terminal card** — a working-directory header with the command's output — rather than a plain text block. The tool asks for this with the neutral `terminal` field on its presentation (`dsh-tools`: a `{ cwd?, output? }` shape on `ToolCallPresentation`/`ToolResultPresentation`); the bridge maps it to the Zed `_meta` convention, gated on the client advertising `clientCapabilities._meta.terminal_output` in `initialize`: + +- `tool_call`: `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the tool's explicit `terminal.cwd` if it has one, else the session's workspace cwd (the bridge fills that, since the pure tool presenter can't see it). +- `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` — the captured output, attached at completion. + +When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted and the ` ```console ` text block (above) is the rendering — so a non-Zed client is never worse off. This is an off-spec Zed `_meta` extension, not the ACP `terminal/create` sub-protocol: that would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd. The exit-status pill, live streaming, and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). ## Settle-exactly-once diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index b418fad9f5..7d0624bc92 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -60,7 +60,7 @@ import { import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' import type { SessionEvent } from '@deepseek-ai/dsh-session' -import type { ToolCallKind, ToolCallPresentation, ToolRegistry, ToolResultPresentation } from '@deepseek-ai/dsh-tools' +import type { ToolCallKind, ToolCallPresentation, ToolRegistry, ToolResultPresentation, ToolTerminal } from '@deepseek-ai/dsh-tools' // Side-effect type import: declaration-merges `ctx.sessionPersistence` onto // Context (the bridge injects it and reads `list()` for load cwd validation). import type {} from '@deepseek-ai/dsh-session-persistence' @@ -212,6 +212,11 @@ export function apply(ctx: Context, config: AcpConfig): void { // await and NOT install a record (which would resurrect a live agent/listeners // after the bridge closed). Checked after every load await. let closed = false + // Whether the client advertised the Zed `_meta.terminal_output` capability in + // `initialize`. When true, a tool's terminal presentation is rendered as a + // terminal card (content + `_meta.terminal_*`); when false, the bridge uses + // the tool's text fallback. Set once in `initialize`, read on every tool event. + let terminalOutputCap = false // Assigned at the bottom, before any agent event can fire (a session only // exists after `newSession`, which the client calls after construction), so @@ -284,7 +289,10 @@ export function apply(ctx: Context, config: AcpConfig): void { ctx.on('session/event', (session, event: SessionEvent) => { const rec = sessions.get(session.header.id) if (rec === undefined) return - streamSessionEventUpdate(rec.sessionId, event, notify, rec.presenter) + streamSessionEventUpdate(rec.sessionId, event, notify, rec.presenter, { + enabled: terminalOutputCap, + cwd: session.header.cwd, + }) const inflight = rec.inflight if (inflight === undefined) return if (event.type === 'turn/start') { @@ -380,6 +388,11 @@ export function apply(ctx: Context, config: AcpConfig): void { // exactly PROTOCOL_VERSION; any other requested version negotiates // down to ours (the client disconnects if it can't speak it). const protocolVersion = params.protocolVersion === PROTOCOL_VERSION ? params.protocolVersion : PROTOCOL_VERSION + // Remember the Zed terminal-output `_meta` capability: when set, bash and + // other shell tools render as a terminal card (see streamSessionEventUpdate + // + the terminal-rendering RFC). `_meta` is `{[k]: unknown} | null`, so + // narrow defensively to a strict boolean true. + terminalOutputCap = params.clientCapabilities?._meta?.['terminal_output'] === true return Promise.resolve({ protocolVersion, agentInfo: { name: agentName, version: agentVersion }, @@ -478,8 +491,12 @@ export function apply(ctx: Context, config: AcpConfig): void { // as the log replays in order (same as live) and is discarded after, // so the record's presenter starts clean for the post-load live stream. const replayPresenter = makePresenter() + const replayTerminal: TerminalRendering = { + enabled: terminalOutputCap, + cwd: agent.session.header.cwd, + } for (const event of agent.session.events) { - streamSessionEventUpdate(params.sessionId, event, notify, replayPresenter) + streamSessionEventUpdate(params.sessionId, event, notify, replayPresenter, replayTerminal) } return {} } finally { @@ -699,6 +716,7 @@ export function streamSessionEventUpdate( event: SessionEvent, notify: (notification: SessionNotification) => void, presenter: Pick = nullToolPresenter, + terminal: TerminalRendering = noTerminalRendering, ): void { switch (event.type) { case 'assistant/chunk': { @@ -724,6 +742,11 @@ export function streamSessionEventUpdate( } case 'tool/call': { const present = presenter.call(event.data.callId, event.data.name, event.data.arguments) + // A terminal-rendered call (a shell command) gets a terminal CARD when the + // client supports it: a `terminal` content block plus `_meta.terminal_info` + // (the cwd header). Otherwise it is an ordinary tool_call and the output + // arrives as text on the result. See the terminal-rendering RFC. + const asTerminal = present.terminal !== undefined && terminal.enabled notify({ sessionId, update: { @@ -733,12 +756,27 @@ export function streamSessionEventUpdate( kind: present.kind, status: 'in_progress', ...present.rawInput !== undefined ? { rawInput: present.rawInput } : {}, + ...asTerminal + ? { + content: [{ type: 'terminal', terminalId: event.data.callId }], + _meta: { terminal_info: { terminal_id: event.data.callId, cwd: present.terminal?.cwd ?? terminal.cwd } }, + } + : {}, }, }) return } case 'tool/result': { const present = presenter.result(event.data.callId, event.data.content, event.data.isError) + const term = present.terminal + // When the call rendered as a terminal AND the client is capable, stream + // the output on the update's `_meta.terminal_output` (the terminal card + // consumes it). The text `content` is still sent as the record/fallback; + // a capable UI shows the terminal card, an incapable one shows the text. + // (The exit-status pill via `_meta.terminal_exit` needs a structured exit + // code the tool doesn't surface yet — see the RFC follow-up; the exit is + // already visible in the output text's `[exit code: N]` marker.) + const asTerminal = term?.output !== undefined && terminal.enabled notify({ sessionId, update: { @@ -747,6 +785,7 @@ export function streamSessionEventUpdate( status: event.data.isError ? 'failed' : 'completed', content: toolResultContent(present.content), ...present.title !== undefined ? { title: present.title } : {}, + ...asTerminal ? { _meta: { terminal_output: { terminal_id: event.data.callId, data: term.output } } } : {}, }, }) return @@ -758,6 +797,23 @@ export function streamSessionEventUpdate( } } +/** + * Per-connection terminal-rendering context threaded into + * {@link streamSessionEventUpdate}: whether the client advertised the + * `_meta.terminal_output` capability, and the session's workspace cwd (the + * default terminal-card header when a tool doesn't supply its own). Kept out of + * the pure translator's required params so the no-capability / no-presenter + * tests stay terse. + */ +export interface TerminalRendering { + enabled: boolean + /** The session workspace cwd (terminal-card header default); `undefined` when the session has none. */ + cwd: string | undefined +} + +/** Default: terminal rendering off (the ` ```console ` text fallback path). */ +const noTerminalRendering: TerminalRendering = { enabled: false, cwd: undefined } + /** * Resolved pending-state presentation the bridge feeds into a `tool_call` * update: a title is always present (tool name when the tool gives none), `kind` @@ -767,6 +823,8 @@ interface ResolvedCallPresentation { title: string kind: ToolCallKind rawInput?: unknown + /** Tool's request to render as a terminal (the pending side carries the cwd). */ + terminal?: ToolTerminal } /** Resolved completed-state presentation fed into a `tool_call_update`. */ @@ -775,6 +833,8 @@ interface ResolvedResultPresentation { content: ContentBlock[] /** Optional replacement title for the completed call. */ title?: string + /** Tool's terminal output/exit for a terminal-rendered call (the result side). */ + terminal?: ToolTerminal } /** @@ -831,7 +891,12 @@ export class ToolPresenter { // the full parsed args as the raw input (the pre-seam behavior). return { title: name, kind: toolKindFor(name), rawInput: args } } - return { title: present.title, kind: present.kind ?? 'other', rawInput: present.rawInput } + return { + title: present.title, + kind: present.kind ?? 'other', + rawInput: present.rawInput, + ...present.terminal !== undefined ? { terminal: present.terminal } : {}, + } } /** Completed-state presentation for a `tool/result`; consumes the remembered `(name, args)`. */ @@ -852,6 +917,7 @@ export class ToolPresenter { return { content: present.content ?? content, ...present.title !== undefined ? { title: present.title } : {}, + ...present.terminal !== undefined ? { terminal: present.terminal } : {}, } } } diff --git a/packages/acp/tests/turns.spec.ts b/packages/acp/tests/turns.spec.ts index 7b37233585..23f4c2e706 100644 --- a/packages/acp/tests/turns.spec.ts +++ b/packages/acp/tests/turns.spec.ts @@ -14,8 +14,8 @@ import { } from './harness.ts' /** Boilerplate: initialize + create one session, returning its id. */ -async function newSession(h: BridgeHarness): Promise { - await h.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) +async function newSession(h: BridgeHarness, clientCapabilities: Record = {}): Promise { + await h.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities }) const { sessionId } = await h.client.newSession({ cwd: process.cwd(), mcpServers: [] }) return sessionId } @@ -110,6 +110,39 @@ describe('acp bridge — turn outcomes', () => { expect(update).toMatchObject({ toolCallId: 'c1', status: 'completed' }) const content = update.content as { content: { type: string; text: string } }[] expect(content[0]?.content.text).toBe('```console\nhello\n```') + // Capability OFF (the default newSession): NO terminal _meta on either update. + expect((call as { _meta?: unknown })._meta).toBeUndefined() + expect((update as { _meta?: unknown })._meta).toBeUndefined() + }) + + it('with the terminal_output capability ON, a real bash call renders as a TERMINAL card (content + _meta)', async () => { + // Drive the REAL bash tool, and advertise the Zed `_meta.terminal_output` + // capability in initialize. The bridge must then emit the terminal CARD: a + // terminal content block + `_meta.terminal_info` (cwd header) on the call, + // and `_meta.terminal_output`/`terminal_exit` on the result. + harness = await makeBridgeHarness({ + storageDir, + withBash: true, + script: [toolCallResponse('c1', 'bash', { command: 'echo hi', description: 'Greet' }), textResponse('done')], + }) + // Capability lives under clientCapabilities._meta.terminal_output. + const sessionId = await newSession(harness, { _meta: { terminal_output: true } }) + await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] }) + + const call = harness.updates.find(u => u.sessionUpdate === 'tool_call') + if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') + // A terminal content block keyed by the callId, and terminal_info with the + // session cwd (the bridge fills it from the session header). + expect(call.content).toEqual([{ type: 'terminal', terminalId: 'c1' }]) + expect((call._meta as { terminal_info?: unknown }).terminal_info).toEqual({ terminal_id: 'c1', cwd: process.cwd() }) + + const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update') + if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update') + // Output rides on _meta.terminal_output; the text content is still present + // as the fallback for a UI that ignores the _meta. + const meta = update._meta as { terminal_output?: { terminal_id: string; data: string } } + expect(meta.terminal_output?.terminal_id).toBe('c1') + expect(meta.terminal_output?.data).toBe('hi') }) it('a throwing tool presenter does not break the turn: the bridge falls back generically', async () => { diff --git a/packages/tool-bash/README.md b/packages/tool-bash/README.md index 194c94c1ca..3068bc34dd 100644 --- a/packages/tool-bash/README.md +++ b/packages/tool-bash/README.md @@ -34,7 +34,7 @@ The owning agent is recorded per task id at spawn and kept for the lifetime of t ## UI presentation -These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the model-written `description` followed by the exact `command` ("List files in src — ls -la src"), `kind` is `execute` (terminal/run treatment), and the `command` is ALSO the **rawInput**. Why both in the title: an execute-kind card hides `rawInput` (Zed renders it only for non-terminal tools), so the command must ride in the always-visible title to be seen — the reference ACP adapters (claude-agent-acp, codex-acp) likewise put the command in an execute tool's title. The completed output is wrapped in a fenced ` ```console ` block — a UI-only affordance, so the model-facing result text stays unfenced. `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Tool-call presentation"). +These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the model-written `description` followed by the exact `command` ("List files in src — ls -la src"), `kind` is `execute` (terminal/run treatment), and the `command` is ALSO the **rawInput**. Why both in the title: an execute-kind card hides `rawInput` (Zed renders it only for non-terminal tools), so the command must ride in the always-visible title to be seen — the reference ACP adapters (claude-agent-acp, codex-acp) likewise put the command in an execute tool's title. The completed output is wrapped in a fenced ` ```console ` block — a UI-only affordance, so the model-facing result text stays unfenced. `bash` also flags itself as a **terminal** (the neutral `terminal` field on its presentation: `presentCall` sets a `cwd` from an explicit absolute `workdir`, else leaves it for the UI bridge to fill from the session cwd; `presentResult` carries the output) so a capable client (Zed) renders a terminal card instead of the text block — see `packages/acp` ("Terminal card"). `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Terminal card" / "Tool-call presentation"). ## Background completion notices diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index 660c4ecfea..908fdf274d 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -40,7 +40,6 @@ import type { Context } from 'cordis' import { isAbsolute, resolve as resolvePath } from 'node:path' import { defineTool } from '@deepseek-ai/dsh-tools' import type { ToolCallPresentation, ToolResult, ToolResultPresentation } from '@deepseek-ai/dsh-tools' -import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { Agent } from '@deepseek-ai/dsh-agent' import type { BashRunResult, BashTask, CollectedOutput } from '@deepseek-ai/dsh-bash' @@ -142,24 +141,40 @@ export function renderResult(result: BashRunResult): string { * title for execute tools. The description leads (a readable summary the schema * requires); the command follows so the verbatim text is still there. `rawInput` * still carries the bare command for non-execute UIs that DO render it. + * + * `terminal` marks the call so a capable UI renders a TERMINAL card. The cwd + * header comes from an explicit absolute model `workdir` when given; otherwise + * the call ran in the session workspace, which this PURE presenter (args only, + * no `exec`) can't see — the UI bridge fills that default from the session's own + * cwd. An empty `terminal: {}` still flags "this is a terminal". */ -function presentBashCall(args: { command: string; description: string }): ToolCallPresentation { - return { title: `${args.description} — ${args.command}`, kind: 'execute', rawInput: args.command } +function presentBashCall(args: { command: string; description: string; workdir?: string }): ToolCallPresentation { + const cwd = args.workdir !== undefined && isAbsolute(args.workdir) ? args.workdir : undefined + return { + title: `${args.description} — ${args.command}`, + kind: 'execute', + rawInput: args.command, + terminal: cwd !== undefined ? { cwd } : {}, + } } /** - * Completed-state presentation for a `bash` call: wrap the model-facing result - * text in a fenced ```console block so a UI renders the output monospaced as a - * terminal transcript. The model-facing `content` (what `execute` returned) is - * intentionally NOT fenced — the fences are a UI-only affordance, so they live - * here, not in `renderResult`. A non-text result (unexpected for bash) is left - * untouched by falling back to `undefined`. + * Completed-state presentation for a `bash` call. Two parallel renderings of the + * same output: `terminal.output` for a UI that shows a terminal card (the run's + * stdout/stderr + status markers, exactly as the model sees them — it already + * carries the `[exit code: N]` marker), and a fenced ```console `content` block + * as the fallback for a UI without terminal support (the fences are a UI-only + * affordance, so they live here, not in `renderResult`). A non-text result + * (unexpected for bash) falls through to `undefined` (UI keeps the raw result). */ function presentBashResult(_args: unknown, result: ToolResult): ToolResultPresentation | undefined { const block = result.content.length === 1 ? result.content[0] : undefined if (block === undefined || block.type !== 'text') return undefined - const fenced: ContentBlock = { type: 'text', text: `\`\`\`console\n${block.text.replace(/\n+$/, '')}\n\`\`\`` } - return { content: [fenced] } + const text = block.text.replace(/\n+$/, '') + return { + content: [{ type: 'text', text: `\`\`\`console\n${text}\n\`\`\`` }], + terminal: { output: text }, + } } /** Pending-state presentation for `bash_output`/`bash_kill` (background-task tools). */ diff --git a/packages/tool-bash/tests/tools.spec.ts b/packages/tool-bash/tests/tools.spec.ts index c1b1e18808..a5a5cdb58f 100644 --- a/packages/tool-bash/tests/tools.spec.ts +++ b/packages/tool-bash/tests/tools.spec.ts @@ -564,20 +564,32 @@ describe('status lines', () => { }) describe('tool-owned UI presentation (presentCall / presentResult)', () => { - it('bash presentCall: title is "description — command" (execute cards hide rawInput), command also in rawInput', async () => { + it('bash presentCall: title is "description — command", marks a terminal; explicit absolute workdir → cwd header', async () => { const ctx = await setup() - const present = ctx.tools.get('bash')?.presentCall?.({ command: 'ls -la src', description: 'List files in src' }) - expect(present).toEqual({ title: 'List files in src — ls -la src', kind: 'execute', rawInput: 'ls -la src' }) + // No explicit workdir → the call still flags a terminal, but with no cwd (the + // UI bridge fills the session cwd it owns; the pure presenter can't see it). + expect(ctx.tools.get('bash')?.presentCall?.({ command: 'ls -la src', description: 'List files in src' })) + .toEqual({ title: 'List files in src — ls -la src', kind: 'execute', rawInput: 'ls -la src', terminal: {} }) + // An explicit ABSOLUTE workdir is surfaced as the terminal cwd header. + expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: '/tmp/x' })) + .toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: { cwd: '/tmp/x' } }) + // A RELATIVE workdir is not an absolute cwd → omitted (terminal still flagged). + expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: 'sub' })) + .toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: {} }) }) - it('bash presentResult: wraps the model-facing text in a fenced console block', async () => { + it('bash presentResult: console-block content AND terminal.output (both renderings of the run)', async () => { const ctx = await setup() const present = ctx.tools.get('bash')!.presentResult!( { command: 'echo hi', description: 'echo' }, { content: [{ type: 'text', text: 'hi\n[exit code: 0]\n\n' }], isError: false }, ) - // Trailing blank lines are trimmed; the body is fenced as ```console. - expect(present).toEqual({ content: [{ type: 'text', text: '```console\nhi\n[exit code: 0]\n```' }] }) + // Trailing blank lines trimmed; content is the fenced ```console fallback, + // terminal.output is the same text for a capable terminal card. + expect(present).toEqual({ + content: [{ type: 'text', text: '```console\nhi\n[exit code: 0]\n```' }], + terminal: { output: 'hi\n[exit code: 0]' }, + }) }) it('bash presentResult: leaves a non-text (unexpected) result untouched → undefined (UI keeps raw content)', async () => { diff --git a/packages/tools/README.md b/packages/tools/README.md index d33e28f1e9..d15b1d1687 100644 --- a/packages/tools/README.md +++ b/packages/tools/README.md @@ -72,8 +72,8 @@ See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, an A tool owns how ITS calls render in a UI (an editor's tool-call card, a CLI log line) — a UI plugin must NOT special-case tool names. A `ToolDefinition` may declare two optional, pure, display-only methods: -- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), and an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object). -- `presentResult(args, result): ToolResultPresentation | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result: an optional replacement `title` and reformatted `content` (e.g. wrap command output in a fenced ` ```console ` block — a UI-only affordance that must NOT appear in the model-facing `execute` result). +- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object), and an optional `terminal` (a neutral `{ cwd? }` asking a capable UI to render this call as a TERMINAL, e.g. for `bash`). +- `presentResult(args, result): ToolResultPresentation | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result: an optional replacement `title`, reformatted `content` (e.g. wrap command output in a fenced ` ```console ` block — a UI-only affordance that must NOT appear in the model-facing `execute` result), and an optional `terminal` (the `{ output? }` for a terminal-rendered call). The `ToolTerminal` shape is provider-neutral; a UI bridge (the ACP bridge) maps it to a terminal card and a UI that can't ignores it and uses `content`. Returning `undefined` (or omitting a method) tells a UI to fall back to a generic presentation (title = tool name, raw args as input, raw result content). Both methods must be **pure and side-effect-free**: a UI may call them during live streaming AND during a session-log replay, so they depend only on their arguments. With `defineTool`, `args` is the typed `InferArgs` shape; the helper soft-validates before calling (a malformed/older logged arg shape yields `undefined` rather than throwing, since display must never crash a replay). The shapes are provider-neutral — the ACP bridge (`dsh-acp`) maps them to ACP `tool_call`/`tool_call_update` wire fields, and `dsh-tool-bash` is the reference implementation. diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index c967f0fba2..4de993b5c9 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -83,6 +83,29 @@ export interface ToolCallPresentation { * unless that is genuinely what a reader wants. */ rawInput?: unknown + /** + * Ask a capable UI to render this call as a TERMINAL (a command running in a + * working directory), not a generic tool card — set by a tool whose call IS a + * shell command (e.g. `bash`). Provider-neutral; a UI bridge maps it to its + * own terminal affordance and a UI that can't falls back to the normal card. + * Pair with {@link ToolResultPresentation.terminal} for the output/exit. + */ + terminal?: ToolTerminal +} + +/** + * A request to render a tool call as a terminal. The pending presentation + * supplies the working directory; the result presentation (see + * {@link ToolResultPresentation.terminal}) supplies the captured output. + * Provider-neutral — no client-protocol types. A UI that supports terminals + * shows a cwd-headed terminal card with the command and its output; a UI that + * does not ignores this and renders the ordinary card/content. + */ +export interface ToolTerminal { + /** Absolute working directory the command ran in, shown as the terminal header. Omit if unknown. */ + cwd?: string + /** Captured command output (stdout+stderr as the tool chooses to combine them). Result-state only. */ + output?: string } /** @@ -102,6 +125,13 @@ export interface ToolResultPresentation { * Stays in harness vocabulary; the UI maps these to its own content blocks. */ content?: ContentBlock[] + /** + * Terminal output/exit for a call the pending presentation marked as a + * terminal (see {@link ToolCallPresentation.terminal}). A capable UI renders + * `output` in the terminal card and shows the exit status; an incapable UI + * uses `content` (the tool should supply a text fallback there too). + */ + terminal?: ToolTerminal } /** A registered tool: its schema plus the execution function. */ From c8dbe6567afecc85524e9a71253783e09c99ca27 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 17:34:50 +0800 Subject: [PATCH 3/7] docs(acp): clarify _meta is a spec extensibility point; the terminal keys are the Zed convention --- .../implemented/2026-06-18-acp-terminal-and-tool-rendering.md | 4 ++-- packages/acp/README.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md index 7817b1e441..db0f744c1f 100644 --- a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md +++ b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md @@ -17,7 +17,7 @@ Studying the two reference agents (2026-06-18) shows neither uses `terminal/crea - **`claude-agent-acp`** (`tools.ts`, `acp-agent.ts`): gated on `clientCapabilities._meta.terminal_output`. The `tool_call` carries `content: [{ type: 'terminal', terminalId }]` and `_meta.terminal_info.{ terminal_id, cwd }`; output/exit arrive on the `tool_call_update`'s `_meta.terminal_output.{ terminal_id, data }` and `_meta.terminal_exit.{ terminal_id, exit_code, signal }`. - **`codex-acp`** (`CodexToolCallMapper.ts`, `TerminalOutputMode.ts`): same `terminal_info` on the call; output via `_meta.terminal_output` (full) or `_meta.terminal_output_delta` (incremental), selected from the same `_meta.terminal_output` capability. -Zed's side (`crates/agent_servers/src/acp.rs`, verified): on a `ToolCall` whose `_meta.terminal_info.terminal_id` is set, it registers a **display-only** terminal (header = `terminal_info.cwd`, label = `tool_call.title`); on a `ToolCallUpdate`, `_meta.terminal_output.data` writes to that terminal and `_meta.terminal_exit.{exit_code,signal}` sets the status. It advertises the capability as `clientCapabilities._meta.terminal_output = true`. This is an off-spec `_meta` extension, but it is the de-facto contract for the Zed integration and the only way to get the terminal card while keeping execution agent-side. +Zed's side (`crates/agent_servers/src/acp.rs`, verified): on a `ToolCall` whose `_meta.terminal_info.terminal_id` is set, it registers a **display-only** terminal (header = `terminal_info.cwd`, label = `tool_call.title`); on a `ToolCallUpdate`, `_meta.terminal_output.data` writes to that terminal and `_meta.terminal_exit.{exit_code,signal}` sets the status. It advertises the capability as `clientCapabilities._meta.terminal_output = true`. `_meta` itself is a spec-blessed ACP extensibility point (typed `{[k]: unknown} | null` on `ToolCall`/`ToolCallUpdate`); the *specific keys* here (`terminal_info`/`terminal_output`/`terminal_exit`) are a Zed convention, not part of the ACP spec — but they are the de-facto contract for the Zed integration and the only way to get the terminal card while keeping execution agent-side. ## Decision @@ -30,7 +30,7 @@ Keep `dsh-bash` agent-side execution; render the terminal card via the `_meta` c ## Risks / trade-offs -- **Off-spec `_meta`.** The terminal card rides on a Zed-specific `_meta` extension, not the ACP terminal sub-protocol. A client that doesn't recognize it still gets the text fallback (the capability gate ensures we only emit it when the client opted in via `_meta.terminal_output`), so a non-Zed client is never worse off. If ACP later standardizes agent-executed terminals, migrate to that and drop the `_meta`. +- **Zed-convention `_meta` keys.** The terminal card rides on Zed-specific keys (`terminal_info`/`terminal_output`) inside ACP's spec-blessed `_meta` extensibility point, NOT on the ACP terminal sub-protocol. A client that doesn't recognize the keys still gets the text fallback (the capability gate ensures we only emit them when the client opted in via `_meta.terminal_output`), so a non-Zed client is never worse off. If ACP later standardizes agent-executed terminals, migrate to that and drop the convention keys. - **Capability honesty.** Emit terminal metadata ONLY when the client advertised `_meta.terminal_output`; the text fallback is the contract for everyone else and must never regress. Covered by a no-capability test asserting the ` ```console ` path. - **terminalId collisions.** Deriving it from the per-call `callId` keeps it unique within a session and stable across the call/result pair; never reuse one across calls. - **Provider-neutral vocabulary creep.** The terminal presentation widens the `dsh-tools` surface; keep it neutral (no ACP types leak into `dsh-tools`) and only as rich as a second UI consumer would also want. diff --git a/packages/acp/README.md b/packages/acp/README.md index 3cb311a43e..4b6d81dae5 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -53,7 +53,7 @@ A tool whose call IS a shell command (`bash`) can render as a real **terminal ca - `tool_call`: `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the tool's explicit `terminal.cwd` if it has one, else the session's workspace cwd (the bridge fills that, since the pure tool presenter can't see it). - `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` — the captured output, attached at completion. -When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted and the ` ```console ` text block (above) is the rendering — so a non-Zed client is never worse off. This is an off-spec Zed `_meta` extension, not the ACP `terminal/create` sub-protocol: that would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd. The exit-status pill, live streaming, and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). +When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted and the ` ```console ` text block (above) is the rendering — so a non-Zed client is never worse off. The `_meta` object is ACP's spec-blessed extensibility point; the specific `terminal_info`/`terminal_output` keys are a Zed convention, not the ACP `terminal/create` sub-protocol (which would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd). The exit-status pill, live streaming, and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). ## Settle-exactly-once From e51dabbb8b002001fe1d12e1bfa94b88480a1106 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 18:54:32 +0800 Subject: [PATCH 4/7] feat(acp): align bash terminal card with reference adapters (command title, description block, exit pill) Match claude-agent-acp / codex-acp: the bash tool_call title IS the command (an execute card hides rawInput), the model description rides as a content text block above the card, and the completed card carries an exit-status pill via _meta.terminal_exit. Bridge fixes found in review of the prior terminal-card commit: - tool_call_update.content is OMITTED in terminal mode (an ACP update.content REPLACES the call's content collection in Zed, so the fenced ```console block would clobber the terminal content block). - terminal.output preserves RAW newlines (terminal renderers rely on exact bytes); only the fenced fallback trims trailing blank lines. - a relative workdir is resolved against the session cwd for the card header, matching where the command actually ran. - result-side terminal output is gated on the pending call having registered a terminal (no orphan _meta.terminal_output for a terminal Zed never made). The exit pill is recovered by parsing renderResult's status markers (the pure presentResult seam sees only content blocks); a round-trip test pins the parse to the marker emission. Neutral ToolTerminal gains exitCode/signal; widened ToolCallPresentation with a content block. Docs (RFC + 3 READMEs) updated; with-key e2e verifies the card + exit pill against the real model. --- ...6-06-18-acp-terminal-and-tool-rendering.md | 13 ++- examples/acp-agent/tests/acp.e2e.ts | 19 ++- packages/acp/README.md | 10 +- packages/acp/src/index.ts | 94 ++++++++++++--- packages/acp/tests/load.spec.ts | 41 ++++++- packages/acp/tests/stream-update.spec.ts | 110 ++++++++++++++++++ packages/acp/tests/turns.spec.ts | 44 ++++--- packages/tool-bash/README.md | 2 +- packages/tool-bash/src/index.ts | 75 ++++++++---- packages/tool-bash/tests/tools.spec.ts | 60 ++++++++-- packages/tools/README.md | 8 +- packages/tools/src/index.ts | 37 +++++- 12 files changed, 417 insertions(+), 96 deletions(-) diff --git a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md index db0f744c1f..d7b0ee12f1 100644 --- a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md +++ b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md @@ -4,9 +4,9 @@ Status: implemented ## Problem -The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../proposed/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the model's `description` plus the command as the `tool_call` title, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block. +The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../proposed/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block. -That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, and the command output rendered as a terminal — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why our command rides inside the title.) +That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, the command output rendered as a terminal, and an exit-status pill — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why the command IS the title — both reference adapters do the same — and the human-readable description rides as a separate content block, since a terminal card has no description slot.) ## Key finding: agent-executed terminals use a `_meta` convention, NOT `terminal/create` @@ -25,16 +25,17 @@ Keep `dsh-bash` agent-side execution; render the terminal card via the `_meta` c 1. **Capability.** `initialize` reads `clientCapabilities._meta.terminal_output` and the bridge remembers it per connection. 2. **Neutral presentation vocabulary.** `dsh-tools` gains a terminal-shaped presentation a tool can return — provider-neutral (`cwd`, the output `data`, an `exitCode`/`signal`), NO ACP types. `dsh-tool-bash` returns it for `bash` (cwd from the resolved workdir; output + exit parsed from the run result). -3. **Bridge mapping.** When the client advertised the capability, the bridge maps that presentation to: on `tool_call`, `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id,cwd}`; on `tool_call_update`, `_meta.terminal_output.{terminal_id,data}` (the captured output). `terminalId` is derived from the harness `callId` (stable, unique per call). When the capability is absent, the bridge uses the existing ` ```console ` text content — unchanged. -4. **No new execution path, no live streaming, no exit pill yet.** Output is attached at completion (from the agent's own `tool/result`), not streamed token-by-token. The exit-status pill (`_meta.terminal_exit.{exit_code,signal}`) is NOT emitted: it needs a structured exit code the pure `presentResult(args, result)` seam doesn't get (the result is content blocks), and the exit is already visible in the output text's `[exit code: N]` / `[killed by signal: …]` marker. Disposal is unaffected: nothing new to tear down, since the bridge never creates a client-side terminal. +3. **Bridge mapping.** When the client advertised the capability, the bridge maps that presentation to: on `tool_call`, `content:[…, {type:'terminal', terminalId}]` (any tool `content`, e.g. the description, rendered BEFORE the terminal block) + `_meta.terminal_info.{terminal_id,cwd}`; on `tool_call_update`, `_meta.terminal_output.{terminal_id,data}` (the captured output) + `_meta.terminal_exit.{terminal_id, exit_code|signal}` (the parsed exit), with the update's text `content` OMITTED (an ACP `tool_call_update.content` REPLACES the call's content collection, so re-sending the fenced block would clobber the terminal content block). `terminalId` is derived from the harness `callId` (stable, unique per call). When the capability is absent, the bridge sends the description content block on the call and the existing ` ```console ` text content on the update — unchanged. +4. **The exit pill is parsed from the rendered output; no new execution path, no live streaming.** Output is attached at completion (from the agent's own `tool/result`), not streamed token-by-token. The exit-status pill (`_meta.terminal_exit.{exit_code,signal}`) IS emitted: the pure `presentResult(args, result)` seam sees only content blocks, so `dsh-tool-bash` recovers the structured exit by parsing the status markers (`[exit code: N]` / `[killed by signal: …]`) that `renderResult` appended — the parse is the exact inverse of the marker emission, the two co-evolve in one file, and a round-trip test guards the pair. Disposal is unaffected: nothing new to tear down, since the bridge never creates a client-side terminal. ## Risks / trade-offs -- **Zed-convention `_meta` keys.** The terminal card rides on Zed-specific keys (`terminal_info`/`terminal_output`) inside ACP's spec-blessed `_meta` extensibility point, NOT on the ACP terminal sub-protocol. A client that doesn't recognize the keys still gets the text fallback (the capability gate ensures we only emit them when the client opted in via `_meta.terminal_output`), so a non-Zed client is never worse off. If ACP later standardizes agent-executed terminals, migrate to that and drop the convention keys. +- **Zed-convention `_meta` keys.** The terminal card rides on Zed-specific keys (`terminal_info`/`terminal_output`/`terminal_exit`) inside ACP's spec-blessed `_meta` extensibility point, NOT on the ACP terminal sub-protocol. A client that doesn't recognize the keys still gets the text fallback (the capability gate ensures we only emit them when the client opted in via `_meta.terminal_output`), so a non-Zed client is never worse off. If ACP later standardizes agent-executed terminals, migrate to that and drop the convention keys. - **Capability honesty.** Emit terminal metadata ONLY when the client advertised `_meta.terminal_output`; the text fallback is the contract for everyone else and must never regress. Covered by a no-capability test asserting the ` ```console ` path. - **terminalId collisions.** Deriving it from the per-call `callId` keeps it unique within a session and stable across the call/result pair; never reuse one across calls. +- **Exit parsed from rendered text.** The exit pill recovers `exit_code`/`signal` by parsing `renderResult`'s status markers rather than threading a structured exit through the event schema (which the pure `presentResult` seam never sees). The parse is the exact inverse of the marker emission and lives in the same file; a round-trip test pins the pair so a marker-format change that breaks the parse fails the suite. If the markers ever need to diverge from what the pill wants, surface a structured exit on the result event instead. - **Provider-neutral vocabulary creep.** The terminal presentation widens the `dsh-tools` surface; keep it neutral (no ACP types leak into `dsh-tools`) and only as rich as a second UI consumer would also want. ## Out of scope / non-goals -The text-block baseline stays the no-capability default. Client-side `terminal/create` execution is explicitly rejected (it bypasses `dsh-bash`). Three follow-ups are deliberately NOT built here and would each warrant their own RFC when someone takes them on: the **exit-status pill** (`_meta.terminal_exit.{exit_code,signal}`, which needs the structured exit surfaced from the run rather than parsed out of the rendered output text), **live incremental streaming** (`_meta.terminal_output_delta` as chunks arrive, which needs an incremental-output seam on `dsh-bash`), and **command classification** (parsing a `cat`/`sed` as a `read` card with a file location, a `grep` as a `search`, etc., falling back to the terminal card — display-only, must never change what executes). +The text-block baseline stays the no-capability default. Client-side `terminal/create` execution is explicitly rejected (it bypasses `dsh-bash`). Two follow-ups are deliberately NOT built here and would each warrant their own RFC when someone takes them on: **live incremental streaming** (`_meta.terminal_output_delta` as chunks arrive, which needs an incremental-output seam on `dsh-bash`), and **command classification** (parsing a `cat`/`sed` as a `read` card with a file location, a `grep` as a `search`, etc., falling back to the terminal card — display-only, must never change what executes). diff --git a/examples/acp-agent/tests/acp.e2e.ts b/examples/acp-agent/tests/acp.e2e.ts index 4a43429eb3..8dd8af6d01 100644 --- a/examples/acp-agent/tests/acp.e2e.ts +++ b/examples/acp-agent/tests/acp.e2e.ts @@ -178,8 +178,8 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over expect(toolCalls.length).toBeGreaterThan(0) // Tool-call UI quality (the tool owns its presentation): the bash tool's - // `presentCall` sets the title to the model's human-readable `description` - // and the `rawInput` to the exact command — NOT the bare tool name "bash". + // `presentCall` sets the title to the exact command (an execute card hides + // rawInput, so the command IS the title) — NOT the bare tool name "bash". // A `bash` call must therefore carry an execute kind, a non-"bash" title, // and a string rawInput (the command). `toolCalls` is already narrowed to // the `tool_call` shape by the filter above, so these fields are reachable. @@ -194,7 +194,7 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over expect((bashCall as { _meta?: unknown })._meta).toBeUndefined() }, 180_000) - it('with the terminal_output capability, a real bash call renders as a terminal card (content + _meta)', async () => { + it('with the terminal_output capability, a real bash call renders as a terminal card (content + _meta + exit)', async () => { workdir = await mkdtemp(join(tmpdir(), 'acp-e2e-')) spawned = spawnAcpAgent(workdir) const { client, updates } = spawned @@ -214,12 +214,19 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over // on _meta.terminal_output. const bashCall = updates.find(u => u.sessionUpdate === 'tool_call' && u.kind === 'execute') if (bashCall?.sessionUpdate !== 'tool_call') throw new Error('expected an execute tool_call') - const block = bashCall.content?.[0] as { type: string; terminalId?: string } | undefined - expect(block?.type).toBe('terminal') - expect(typeof block?.terminalId).toBe('string') + // The content carries the description text block AND a terminal block (the + // description renders above the card) — find the terminal block by type, not + // by position. + const blocks = (bashCall.content ?? []) as { type: string; terminalId?: string }[] + const terminalBlock = blocks.find(b => b.type === 'terminal') + expect(terminalBlock).toBeDefined() + expect(typeof terminalBlock?.terminalId).toBe('string') const info = (bashCall._meta as { terminal_info?: { terminal_id: string; cwd?: string } }).terminal_info expect(info?.cwd).toBe(workdir) const updatesForTerminal = updates.filter(u => u.sessionUpdate === 'tool_call_update' && (u._meta as { terminal_output?: unknown } | undefined)?.terminal_output !== undefined) expect(updatesForTerminal.length).toBeGreaterThan(0) + // The completed update also carries the parsed exit on _meta.terminal_exit. + const exitUpdate = updates.find(u => u.sessionUpdate === 'tool_call_update' && (u._meta as { terminal_exit?: unknown } | undefined)?.terminal_exit !== undefined) + expect(exitUpdate).toBeDefined() }, 180_000) }) diff --git a/packages/acp/README.md b/packages/acp/README.md index 4b6d81dae5..a47a7327c2 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -42,18 +42,18 @@ Each session runs in its own workspace, recorded as the session's `SessionHeader ## Tool-call presentation -How a tool call renders in the editor is owned by the TOOL, not the bridge — the bridge never special-cases tool names. Each tool may declare `presentCall(args)` (pending state: a human-readable `title`, a `kind` for the icon, and the salient `rawInput` to show in a detail view) and `presentResult(args, result)` (completed state: an optional replacement `title` and reformatted `content`) on its `dsh-tools` definition. The bridge looks the definition up by name in `ctx.tools` and maps the neutral `ToolCallPresentation`/`ToolResultPresentation` to the ACP `tool_call`/`tool_call_update` wire shapes. A tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` sets the title to the model `description` + the exact `command` ("List files in src — ls -la src"), `kind: 'execute'`, the `command` as `rawInput`, and wraps the completed output in a fenced ` ```console ` block. (The command goes in the title because an editor hides `rawInput` for execute-kind cards — Zed renders it only for non-terminal tools.) +How a tool call renders in the editor is owned by the TOOL, not the bridge — the bridge never special-cases tool names. Each tool may declare `presentCall(args)` (pending state: a human-readable `title`, a `kind` for the icon, the salient `rawInput` to show in a detail view, and optional `content` blocks shown alongside) and `presentResult(args, result)` (completed state: an optional replacement `title` and reformatted `content`) on its `dsh-tools` definition. The bridge looks the definition up by name in `ctx.tools` and maps the neutral `ToolCallPresentation`/`ToolResultPresentation` to the ACP `tool_call`/`tool_call_update` wire shapes. A tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` sets the title to the exact `command` ("ls -la src"), `kind: 'execute'`, the `command` as `rawInput`, the model `description` as a `content` text block, and wraps the completed output in a fenced ` ```console ` block. (The command is the title because an editor hides `rawInput` for execute-kind cards — Zed renders it only for non-terminal tools — and the reference adapters likewise use the command as an execute tool's title.) The `tool/result` session event carries only `{ callId, content, isError }` — not the tool name or args — so to call a tool's `presentResult` the bridge keeps a small per-session map from `callId` to the in-flight call's `(name, args)`, populated on `tool/call` and removed as each result is presented (it holds only currently-in-flight calls, never finished ones). This is bridge-local state — NOT a change to the event schema or a core service. The map lives on the `SessionRecord`, so two concurrent sessions never cross their in-flight tool state; a `session/load` replay uses a throwaway presenter that pairs each `tool/call` with its `tool/result` as the log replays in order, so replayed tool cards render identically to live ones. ## Terminal card (capability-gated) -A tool whose call IS a shell command (`bash`) can render as a real **terminal card** — a working-directory header with the command's output — rather than a plain text block. The tool asks for this with the neutral `terminal` field on its presentation (`dsh-tools`: a `{ cwd?, output? }` shape on `ToolCallPresentation`/`ToolResultPresentation`); the bridge maps it to the Zed `_meta` convention, gated on the client advertising `clientCapabilities._meta.terminal_output` in `initialize`: +A tool whose call IS a shell command (`bash`) can render as a real **terminal card** — a working-directory header with the command's output and an exit-status pill — rather than a plain text block. The tool asks for this with the neutral `terminal` field on its presentation (`dsh-tools`: a `{ cwd?, output?, exitCode?, signal? }` shape on `ToolCallPresentation`/`ToolResultPresentation`); the bridge maps it to the Zed `_meta` convention, gated on the client advertising `clientCapabilities._meta.terminal_output` in `initialize`: -- `tool_call`: `content:[{type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the tool's explicit `terminal.cwd` if it has one, else the session's workspace cwd (the bridge fills that, since the pure tool presenter can't see it). -- `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` — the captured output, attached at completion. +- `tool_call`: `content:[…, {type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the tool's explicit absolute `terminal.cwd`, else a relative `terminal.cwd` resolved against the session cwd, else the session's workspace cwd (the bridge fills the default, since the pure tool presenter can't see it). Any pending `content` the tool supplied (e.g. bash's `description`) renders BEFORE the terminal block, so the description sits above the card. +- `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` (the captured output) plus `_meta.terminal_exit.{terminal_id, exit_code | signal}` when the tool reported a structured exit. In terminal mode the update's `content` is OMITTED — an ACP `tool_call_update.content` REPLACES the call's content, so sending the fenced text block would clobber the terminal content block from the call. -When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted and the ` ```console ` text block (above) is the rendering — so a non-Zed client is never worse off. The `_meta` object is ACP's spec-blessed extensibility point; the specific `terminal_info`/`terminal_output` keys are a Zed convention, not the ACP `terminal/create` sub-protocol (which would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd). The exit-status pill, live streaming, and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). +When the client does NOT advertise the capability, none of the `_meta`/terminal content is emitted: the `tool_call` shows the `description` content block and the `tool_call_update` carries the ` ```console ` text block (above) as the rendering — so a non-Zed client is never worse off. The `_meta` object is ACP's spec-blessed extensibility point; the specific `terminal_info`/`terminal_output`/`terminal_exit` keys are a Zed convention, not the ACP `terminal/create` sub-protocol (which would make the editor execute the command, bypassing `dsh-bash`'s sandbox/env-scrub/ownership/cwd). Live incremental streaming and command classification are follow-ups. See [the terminal-rendering RFC](../../docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md). ## Settle-exactly-once diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index 7d0624bc92..20d77f9a20 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -34,7 +34,7 @@ import type { Context } from 'cordis' import { Readable, Writable } from 'node:stream' import { randomUUID } from 'node:crypto' -import { isAbsolute } from 'node:path' +import { isAbsolute, resolve as resolvePath } from 'node:path' import Schema from 'schemastery' import { AgentSideConnection, @@ -747,6 +747,14 @@ export function streamSessionEventUpdate( // (the cwd header). Otherwise it is an ordinary tool_call and the output // arrives as text on the result. See the terminal-rendering RFC. const asTerminal = present.terminal !== undefined && terminal.enabled + // The tool's pending content (e.g. bash's `description`) renders ABOVE the + // card; when the card is shown, append the terminal block AFTER it so the + // description sits over the command (Zed renders content blocks in order). + // Without the capability the description still renders as the card's body. + const callContent: ({ type: 'content'; content: AcpContentBlock } | { type: 'terminal'; terminalId: string })[] = [ + ...present.content !== undefined ? toolResultContent(present.content) : [], + ...asTerminal ? [{ type: 'terminal' as const, terminalId: event.data.callId }] : [], + ] notify({ sessionId, update: { @@ -756,11 +764,9 @@ export function streamSessionEventUpdate( kind: present.kind, status: 'in_progress', ...present.rawInput !== undefined ? { rawInput: present.rawInput } : {}, + ...callContent.length > 0 ? { content: callContent } : {}, ...asTerminal - ? { - content: [{ type: 'terminal', terminalId: event.data.callId }], - _meta: { terminal_info: { terminal_id: event.data.callId, cwd: present.terminal?.cwd ?? terminal.cwd } }, - } + ? { _meta: { terminal_info: { terminal_id: event.data.callId, cwd: terminalCwd(present.terminal, terminal.cwd) } } } : {}, }, }) @@ -769,23 +775,30 @@ export function streamSessionEventUpdate( case 'tool/result': { const present = presenter.result(event.data.callId, event.data.content, event.data.isError) const term = present.terminal - // When the call rendered as a terminal AND the client is capable, stream - // the output on the update's `_meta.terminal_output` (the terminal card - // consumes it). The text `content` is still sent as the record/fallback; - // a capable UI shows the terminal card, an incapable one shows the text. - // (The exit-status pill via `_meta.terminal_exit` needs a structured exit - // code the tool doesn't surface yet — see the RFC follow-up; the exit is - // already visible in the output text's `[exit code: N]` marker.) + // When the call rendered as a terminal AND the client is capable, the output + // and exit status ride on `_meta` (the terminal card consumes them) and the + // text `content` is OMITTED: a `tool_call_update.content` REPLACES the call's + // content collection in Zed, so sending the fenced ```console block here + // would clobber the terminal content block the call installed. The incapable + // path keeps sending `content` (the fenced fallback is the only rendering). const asTerminal = term?.output !== undefined && terminal.enabled + const terminalResultMeta = asTerminal + ? { + _meta: { + terminal_output: { terminal_id: event.data.callId, data: term.output }, + ...terminalExitMeta(event.data.callId, term), + }, + } + : {} notify({ sessionId, update: { sessionUpdate: 'tool_call_update', toolCallId: event.data.callId, status: event.data.isError ? 'failed' : 'completed', - content: toolResultContent(present.content), + ...asTerminal ? {} : { content: toolResultContent(present.content) }, ...present.title !== undefined ? { title: present.title } : {}, - ...asTerminal ? { _meta: { terminal_output: { terminal_id: event.data.callId, data: term.output } } } : {}, + ...terminalResultMeta, }, }) return @@ -823,6 +836,8 @@ interface ResolvedCallPresentation { title: string kind: ToolCallKind rawInput?: unknown + /** UI content shown on the pending call (e.g. a bash description text block above the card). */ + content?: ContentBlock[] /** Tool's request to render as a terminal (the pending side carries the cwd). */ terminal?: ToolTerminal } @@ -859,7 +874,7 @@ interface ResolvedResultPresentation { * stale entry's only cost is one map slot until the session ends. */ export class ToolPresenter { - private readonly pending = new Map() + private readonly pending = new Map() /** * @param tools the registry to resolve tool definitions by name. @@ -877,7 +892,6 @@ export class ToolPresenter { /** Pending-state presentation for a `tool/call`; remembers `(name, args)` for the matching result. */ call(callId: string, name: string, argsJson: string): ResolvedCallPresentation { const args = parseToolArguments(argsJson) - this.pending.set(callId, { name, args }) let present: ToolCallPresentation | undefined try { present = this.tools.get(name)?.presentCall?.(args) @@ -888,13 +902,21 @@ export class ToolPresenter { } if (present === undefined) { // No tool-owned presentation: fall back to the tool name as the title and - // the full parsed args as the raw input (the pre-seam behavior). + // the full parsed args as the raw input (the pre-seam behavior). A generic + // call is never a terminal, so a later result can't emit terminal output. + this.pending.set(callId, { name, args, isTerminal: false }) return { title: name, kind: toolKindFor(name), rawInput: args } } + // Remember whether THIS call rendered as a terminal, so `result()` only emits + // terminal output/exit for a call that actually registered a terminal — a + // `presentResult().terminal` without a matching `presentCall().terminal` + // would otherwise orphan `_meta.terminal_output` to a terminal Zed never made. + this.pending.set(callId, { name, args, isTerminal: present.terminal !== undefined }) return { title: present.title, kind: present.kind ?? 'other', rawInput: present.rawInput, + ...present.content !== undefined ? { content: present.content } : {}, ...present.terminal !== undefined ? { terminal: present.terminal } : {}, } } @@ -917,7 +939,10 @@ export class ToolPresenter { return { content: present.content ?? content, ...present.title !== undefined ? { title: present.title } : {}, - ...present.terminal !== undefined ? { terminal: present.terminal } : {}, + // Only propagate terminal output/exit when the PENDING call registered a + // terminal (finding: orphan terminal output otherwise). A result-only + // terminal with no matching call-side terminal is dropped. + ...present.terminal !== undefined && call.isTerminal ? { terminal: present.terminal } : {}, } } } @@ -961,3 +986,36 @@ function toolResultContent(blocks: ContentBlock[]): { type: 'content'; content: } return out } + +/** + * Resolve the terminal card's header cwd. The tool's `terminal.cwd` (a model + * `workdir`) wins when ABSOLUTE; a RELATIVE one resolves against the session + * cwd (matching how `dsh-tool-bash` resolves a relative workdir for execution, + * so the header matches where the command actually ran); when the tool gives no + * cwd, the session workspace cwd is the default. Returns `undefined` only when + * neither the tool nor the session supplies one (Zed then shows "current + * directory"). + */ +function terminalCwd(term: ToolTerminal | undefined, sessionCwd: string | undefined): string | undefined { + const toolCwd = term?.cwd + if (toolCwd === undefined) return sessionCwd + if (isAbsolute(toolCwd)) return toolCwd + return sessionCwd !== undefined ? resolvePath(sessionCwd, toolCwd) : toolCwd +} + +/** The `terminal_exit` `_meta` entry for a completed terminal call. */ +interface TerminalExitMeta { + terminal_exit?: { terminal_id: string; exit_code?: number; signal?: string } +} + +/** + * Build the optional `terminal_exit` portion of a `tool_call_update`'s `_meta` + * from the tool's terminal result: a `signal` death yields `{signal}`, an + * `exitCode` yields `{exit_code}`, and neither yields nothing (the card simply + * shows no exit pill). Spread into the `_meta` object alongside `terminal_output`. + */ +function terminalExitMeta(callId: string, term: ToolTerminal): TerminalExitMeta { + if (term.signal !== undefined) return { terminal_exit: { terminal_id: callId, signal: term.signal } } + if (term.exitCode !== undefined) return { terminal_exit: { terminal_id: callId, exit_code: term.exitCode } } + return {} +} diff --git a/packages/acp/tests/load.spec.ts b/packages/acp/tests/load.spec.ts index 89bd8253f7..a10aca4bd4 100644 --- a/packages/acp/tests/load.spec.ts +++ b/packages/acp/tests/load.spec.ts @@ -81,7 +81,10 @@ describe('acp bridge — session/load replay', () => { await loader.client.loadSession({ sessionId, cwd: process.cwd(), mcpServers: [] }) const call = loader.updates.find(u => u.sessionUpdate === 'tool_call') - expect(call).toMatchObject({ toolCallId: 'c1', title: 'Print a greeting — echo hello', kind: 'execute', rawInput: 'echo hello' }) + expect(call).toMatchObject({ toolCallId: 'c1', title: 'echo hello', kind: 'execute', rawInput: 'echo hello' }) + if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') + // Capability OFF on this loader: the description renders as a content block, no terminal block. + expect(call.content).toEqual([{ type: 'content', content: { type: 'text', text: 'Print a greeting' } }]) const update = loader.updates.find(u => u.sessionUpdate === 'tool_call_update') expect(update?.sessionUpdate).toBe('tool_call_update') if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update') @@ -90,6 +93,42 @@ describe('acp bridge — session/load replay', () => { expect(content[0]?.content.text).toBe('```console\nhello\n```') }) + it('replays a persisted bash call as a TERMINAL card when the loader advertises the capability', async () => { + // The presentation is resolved at replay time, so a loader that advertised + // _meta.terminal_output must reconstruct the terminal card (content + _meta) + // from the persisted log — identical to how it would have streamed live. + live = await makeBridgeHarness({ + storageDir, + withBash: true, + script: [toolCallResponse('c1', 'bash', { command: 'echo hi', description: 'Greet' }), textResponse('done')], + }) + await live.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + const { sessionId } = await live.client.newSession({ cwd: process.cwd(), mcpServers: [] }) + await live.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] }) + await live.dispose() + live = undefined + + loader = await makeBridgeHarness({ storageDir, withBash: true, script: [] }) + await loader.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: { _meta: { terminal_output: true } } }) + await loader.client.loadSession({ sessionId, cwd: process.cwd(), mcpServers: [] }) + + const call = loader.updates.find(u => u.sessionUpdate === 'tool_call') + if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') + // Replay reconstructs the terminal card: description block, then terminal block. + expect(call.content).toEqual([ + { type: 'content', content: { type: 'text', text: 'Greet' } }, + { type: 'terminal', terminalId: 'c1' }, + ]) + expect((call._meta as { terminal_info?: unknown }).terminal_info).toEqual({ terminal_id: 'c1', cwd: process.cwd() }) + const update = loader.updates.find(u => u.sessionUpdate === 'tool_call_update') + if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update') + // Terminal mode: content omitted, output + exit on _meta — matching live. + expect(update.content).toBeUndefined() + const meta = update._meta as { terminal_output?: { data: string }; terminal_exit?: { exit_code?: number } } + expect(meta.terminal_output?.data).toBe('hi\n') + expect(meta.terminal_exit?.exit_code).toBe(0) + }) + it('a load whose resume finishes after a client disconnect leaks no live session', async () => { // A session/load is mid-resume() when the client transport closes. The load // must NOT end up with a live registered agent for the connection that is diff --git a/packages/acp/tests/stream-update.spec.ts b/packages/acp/tests/stream-update.spec.ts index 8a22bc71f1..bf73fe0629 100644 --- a/packages/acp/tests/stream-update.spec.ts +++ b/packages/acp/tests/stream-update.spec.ts @@ -281,6 +281,116 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => }) }) +describe('terminal-card mapping (capability-gated)', () => { + // A tool that asks to render as a terminal — a stand-in for tool-bash's shape, + // letting us drive the bridge's terminal mapping without the real executor. + type CallTerm = { cwd?: string } | undefined + type ResultTerm = { output?: string; exitCode?: number; signal?: string } | undefined + const termTool = (callTerminal: CallTerm, resultTerminal: ResultTerm): ToolDefinition => ({ + name: 'bash', + description: 'run a command', + parameters: {}, + execute: async () => [], + presentCall: (args: unknown) => ({ + title: (args as { command: string }).command, + kind: 'execute', + rawInput: (args as { command: string }).command, + content: [{ type: 'text', text: (args as { description: string }).description }], + ...callTerminal !== undefined ? { terminal: callTerminal } : {}, + }), + presentResult: () => ({ + content: [{ type: 'text', text: 'fallback' }], + ...resultTerminal !== undefined ? { terminal: resultTerminal } : {}, + }), + }) + + const callEvent = evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: JSON.stringify({ command: 'echo hi', description: 'Greet' }) }) + const resultEvent = evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'hi\n' }], isError: false }) + + function termUpdates(tool: ToolDefinition, enabled: boolean, cwd: string | undefined, ...events: SessionEvent[]): SessionNotification['update'][] { + const presenter = new ToolPresenter(registryOf(tool)) + const out: SessionNotification['update'][] = [] + for (const event of events) streamSessionEventUpdate('s1', event, n => out.push(n.update), presenter, { enabled, cwd }) + return out + } + + it('capability ON: description content THEN terminal block; cwd from the session header when the tool gives none', () => { + const [call, update] = termUpdates(termTool({}, { output: 'hi\n', exitCode: 0 }), true, '/work/proj', callEvent, resultEvent) + expect(call).toMatchObject({ + sessionUpdate: 'tool_call', + content: [ + { type: 'content', content: { type: 'text', text: 'Greet' } }, + { type: 'terminal', terminalId: 'c1' }, + ], + _meta: { terminal_info: { terminal_id: 'c1', cwd: '/work/proj' } }, + }) + // The update OMITS content (it would clobber the terminal block) and carries output + exit. + expect(update).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + _meta: { terminal_output: { terminal_id: 'c1', data: 'hi\n' }, terminal_exit: { terminal_id: 'c1', exit_code: 0 } }, + }) + }) + + it('capability ON: an ABSOLUTE tool cwd wins; a RELATIVE one resolves against the session cwd', () => { + const [absCall] = termUpdates(termTool({ cwd: '/explicit/abs' }, { output: 'x' }), true, '/work/proj', callEvent) + expect((absCall as unknown as { _meta: { terminal_info: { cwd: string } } })._meta.terminal_info.cwd).toBe('/explicit/abs') + const [relCall] = termUpdates(termTool({ cwd: 'sub/dir' }, { output: 'x' }), true, '/work/proj', callEvent) + // Relative workdir resolved against the session cwd — the card header matches + // where execution actually ran (tool-bash resolves the same way). + expect((relCall as unknown as { _meta: { terminal_info: { cwd: string } } })._meta.terminal_info.cwd).toBe('/work/proj/sub/dir') + // No session cwd to resolve against → the relative tool cwd is passed through as-is. + const [noSessionCwd] = termUpdates(termTool({ cwd: 'rel/only' }, { output: 'x' }), true, undefined, callEvent) + expect((noSessionCwd as unknown as { _meta: { terminal_info: { cwd: string } } })._meta.terminal_info.cwd).toBe('rel/only') + }) + + it('capability ON: a signal kill maps to terminal_exit.signal', () => { + const [, update] = termUpdates(termTool({}, { output: 'gone', signal: 'SIGKILL' }), true, '/w', callEvent, resultEvent) + expect((update as unknown as { _meta: { terminal_exit: unknown } })._meta.terminal_exit).toEqual({ terminal_id: 'c1', signal: 'SIGKILL' }) + }) + + it('capability ON: a terminal result with output but NO exit/signal emits terminal_output and NO exit pill', () => { + // A terminal-rendering tool that reports no structured exit (neither exitCode + // nor signal) — the card shows output but no exit pill. + const [, update] = termUpdates(termTool({}, { output: 'partial' }), true, '/w', callEvent, resultEvent) + const meta = (update as unknown as { _meta: { terminal_output?: unknown; terminal_exit?: unknown } })._meta + expect(meta.terminal_output).toEqual({ terminal_id: 'c1', data: 'partial' }) + expect(meta.terminal_exit).toBeUndefined() + }) + + it('capability OFF: no terminal block or _meta; the description content and fenced result still render', () => { + const [call, update] = termUpdates(termTool({}, { output: 'hi\n' }), false, '/work/proj', callEvent, resultEvent) + expect(call).toEqual({ + sessionUpdate: 'tool_call', + toolCallId: 'c1', + title: 'echo hi', + kind: 'execute', + status: 'in_progress', + rawInput: 'echo hi', + content: [{ type: 'content', content: { type: 'text', text: 'Greet' } }], + }) + expect(update).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: 'fallback' } }], + }) + }) + + it('orphan guard: a result-side terminal with NO call-side terminal is dropped (no orphan terminal_output)', () => { + // presentCall declares NO terminal, but presentResult returns one — the + // bridge must not emit _meta.terminal_output for a terminal Zed never made. + const [call, update] = termUpdates(termTool(undefined, { output: 'hi\n', exitCode: 0 }), true, '/w', callEvent, resultEvent) + // The call had no terminal → ordinary tool_call (description content, no _meta). + expect((call as { _meta?: unknown })._meta).toBeUndefined() + expect((call as { content: unknown }).content).toEqual([{ type: 'content', content: { type: 'text', text: 'Greet' } }]) + // The result falls back to text content; NO terminal _meta. + expect((update as { _meta?: unknown })._meta).toBeUndefined() + expect((update as { content: unknown }).content).toEqual([{ type: 'content', content: { type: 'text', text: 'fallback' } }]) + }) +}) + describe('agentOptions', () => { it('includes only the fields present in config', () => { expect(agentOptions({})).toEqual({}) diff --git a/packages/acp/tests/turns.spec.ts b/packages/acp/tests/turns.spec.ts index 23f4c2e706..3cf12a67e8 100644 --- a/packages/acp/tests/turns.spec.ts +++ b/packages/acp/tests/turns.spec.ts @@ -75,7 +75,7 @@ describe('acp bridge — turn outcomes', () => { expect(callIdx).toBeLessThan(updIdx) }) - it('the REAL bash tool drives the tool-call UI end-to-end: description—command title + console output', async () => { + it('the REAL bash tool drives the tool-call UI end-to-end: command title + description block + console output', async () => { // Use the SHIPPING tool (dsh-tool-bash + dsh-bash-local), not an inline // stand-in, so this verifies the actual presentCall/presentResult the editor // sees (AGENTS.md "prefer the real implementation over a mock in tests"). @@ -93,16 +93,20 @@ describe('acp bridge — turn outcomes', () => { const sessionId = await newSession(harness) await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] }) - // presentCall: execute kind, title is "description — command" (an execute - // card hides rawInput, so the command rides in the title), command in rawInput. + // presentCall: execute kind, title IS the command (an execute card hides + // rawInput, so the command is the title), the description rides as a content + // text block, the command is also rawInput for non-terminal UIs. const call = harness.updates.find(u => u.sessionUpdate === 'tool_call') expect(call).toMatchObject({ toolCallId: 'c1', - title: 'Print a greeting — echo hello', + title: 'echo hello', kind: 'execute', rawInput: 'echo hello', status: 'in_progress', }) + if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') + // Capability OFF: the description renders as the only content block (no terminal block). + expect(call.content).toEqual([{ type: 'content', content: { type: 'text', text: 'Print a greeting' } }]) // presentResult: the REAL command output, wrapped in a fenced console block. const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update') expect(update?.sessionUpdate).toBe('tool_call_update') @@ -115,11 +119,12 @@ describe('acp bridge — turn outcomes', () => { expect((update as { _meta?: unknown })._meta).toBeUndefined() }) - it('with the terminal_output capability ON, a real bash call renders as a TERMINAL card (content + _meta)', async () => { + it('with the terminal_output capability ON, a real bash call renders as a TERMINAL card (content + _meta + exit)', async () => { // Drive the REAL bash tool, and advertise the Zed `_meta.terminal_output` - // capability in initialize. The bridge must then emit the terminal CARD: a - // terminal content block + `_meta.terminal_info` (cwd header) on the call, - // and `_meta.terminal_output`/`terminal_exit` on the result. + // capability in initialize. The bridge must then emit the terminal CARD: the + // description content block THEN a terminal content block + `_meta.terminal_info` + // (cwd header) on the call, and `_meta.terminal_output`/`terminal_exit` on the + // result — and OMIT the update's text content (it would clobber the card). harness = await makeBridgeHarness({ storageDir, withBash: true, @@ -131,18 +136,27 @@ describe('acp bridge — turn outcomes', () => { const call = harness.updates.find(u => u.sessionUpdate === 'tool_call') if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') - // A terminal content block keyed by the callId, and terminal_info with the + // The description content block FIRST (renders above the card), then a + // terminal content block keyed by the callId; terminal_info carries the // session cwd (the bridge fills it from the session header). - expect(call.content).toEqual([{ type: 'terminal', terminalId: 'c1' }]) + expect(call.content).toEqual([ + { type: 'content', content: { type: 'text', text: 'Greet' } }, + { type: 'terminal', terminalId: 'c1' }, + ]) expect((call._meta as { terminal_info?: unknown }).terminal_info).toEqual({ terminal_id: 'c1', cwd: process.cwd() }) const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update') if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update') - // Output rides on _meta.terminal_output; the text content is still present - // as the fallback for a UI that ignores the _meta. - const meta = update._meta as { terminal_output?: { terminal_id: string; data: string } } - expect(meta.terminal_output?.terminal_id).toBe('c1') - expect(meta.terminal_output?.data).toBe('hi') + // In terminal mode the text content is OMITTED (a tool_call_update.content + // REPLACES the call's content — it would clobber the terminal block). + expect(update.content).toBeUndefined() + // Output rides on _meta.terminal_output; the parsed exit on _meta.terminal_exit. + const meta = update._meta as { + terminal_output?: { terminal_id: string; data: string } + terminal_exit?: { terminal_id: string; exit_code?: number; signal?: string } + } + expect(meta.terminal_output).toEqual({ terminal_id: 'c1', data: 'hi\n' }) + expect(meta.terminal_exit).toEqual({ terminal_id: 'c1', exit_code: 0 }) }) it('a throwing tool presenter does not break the turn: the bridge falls back generically', async () => { diff --git a/packages/tool-bash/README.md b/packages/tool-bash/README.md index 3068bc34dd..39e56a6126 100644 --- a/packages/tool-bash/README.md +++ b/packages/tool-bash/README.md @@ -34,7 +34,7 @@ The owning agent is recorded per task id at spawn and kept for the lifetime of t ## UI presentation -These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the model-written `description` followed by the exact `command` ("List files in src — ls -la src"), `kind` is `execute` (terminal/run treatment), and the `command` is ALSO the **rawInput**. Why both in the title: an execute-kind card hides `rawInput` (Zed renders it only for non-terminal tools), so the command must ride in the always-visible title to be seen — the reference ACP adapters (claude-agent-acp, codex-acp) likewise put the command in an execute tool's title. The completed output is wrapped in a fenced ` ```console ` block — a UI-only affordance, so the model-facing result text stays unfenced. `bash` also flags itself as a **terminal** (the neutral `terminal` field on its presentation: `presentCall` sets a `cwd` from an explicit absolute `workdir`, else leaves it for the UI bridge to fill from the session cwd; `presentResult` carries the output) so a capable client (Zed) renders a terminal card instead of the text block — see `packages/acp` ("Terminal card"). `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Terminal card" / "Tool-call presentation"). +These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the exact `command` ("ls -la src") and `kind` is `execute` (terminal/run treatment), matching the reference ACP adapters (claude-agent-acp, codex-acp), which both use the bare command as an execute tool's title. The command is ALSO the **rawInput** for non-terminal UIs that render it (an execute-kind card hides rawInput — Zed shows it only for non-terminal tools — so the command must BE the title to be seen). The model-written `description` rides as a **content** text block shown ABOVE the card (a terminal card has no description slot, so it sits over the command; claude-agent-acp likewise surfaces the description as a separate content block). The completed output is wrapped in a fenced ` ```console ` block as the no-terminal-capability fallback — a UI-only affordance, so the model-facing result text stays unfenced. `bash` also flags itself as a **terminal** (the neutral `terminal` field: `presentCall` sets a `cwd` from the model `workdir` when given — absolute as-is, relative for the UI bridge to resolve against the session cwd — else leaves it for the bridge to fill from the session cwd; `presentResult` carries the raw output plus the parsed `exitCode`/`signal`) so a capable client (Zed) renders a terminal card with an exit-status pill instead of the text block — see `packages/acp` ("Terminal card"). `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Terminal card" / "Tool-call presentation"). ## Background completion notices diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index 908fdf274d..f48b1f7a08 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -132,51 +132,76 @@ export function renderResult(result: BashRunResult): string { // --------------------------------------------------------------------------- /** - * Pending-state presentation for a `bash` call. The title is the model-written - * `description` followed by the exact `command` ("List files — ls -la src"): - * `kind: 'execute'` gets a terminal/run treatment in a UI, but an execute-kind - * card HIDES `rawInput` (Zed: `should_show_raw_input = !is_terminal_tool`), so - * the command MUST ride in the always-visible title to be seen — the reference - * ACP adapters (claude-agent-acp, codex-acp) likewise put the command in the - * title for execute tools. The description leads (a readable summary the schema - * requires); the command follows so the verbatim text is still there. `rawInput` - * still carries the bare command for non-execute UIs that DO render it. + * Pending-state presentation for a `bash` call. The TITLE is the exact `command` + * — a `kind: 'execute'` card is rendered as a terminal whose header label IS the + * title, and an execute-kind card HIDES `rawInput` (Zed: `should_show_raw_input + * = !is_terminal_tool`), so the command must BE the title to be seen. This + * mirrors the reference ACP adapters (claude-agent-acp, codex-acp), which both + * use the bare command as an execute tool's title. The model-written + * `description` (a readable summary) rides as a `content` text block shown ABOVE + * the card, since a terminal card has no description slot — claude-agent-acp + * likewise surfaces its description as a separate content block. `rawInput` still + * carries the bare command for non-execute UIs that DO render it. * - * `terminal` marks the call so a capable UI renders a TERMINAL card. The cwd - * header comes from an explicit absolute model `workdir` when given; otherwise - * the call ran in the session workspace, which this PURE presenter (args only, - * no `exec`) can't see — the UI bridge fills that default from the session's own - * cwd. An empty `terminal: {}` still flags "this is a terminal". + * `terminal` marks the call so a capable UI renders a TERMINAL card. Its `cwd` + * (header) is the model `workdir` when given — ABSOLUTE as-is, RELATIVE for the + * UI bridge to resolve against the session cwd; when omitted entirely the bridge + * fills the session workspace cwd (this PURE presenter, args only, can't see it). */ function presentBashCall(args: { command: string; description: string; workdir?: string }): ToolCallPresentation { - const cwd = args.workdir !== undefined && isAbsolute(args.workdir) ? args.workdir : undefined return { - title: `${args.description} — ${args.command}`, + title: args.command, kind: 'execute', rawInput: args.command, - terminal: cwd !== undefined ? { cwd } : {}, + content: [{ type: 'text', text: args.description }], + terminal: args.workdir !== undefined ? { cwd: args.workdir } : {}, } } /** * Completed-state presentation for a `bash` call. Two parallel renderings of the * same output: `terminal.output` for a UI that shows a terminal card (the run's - * stdout/stderr + status markers, exactly as the model sees them — it already - * carries the `[exit code: N]` marker), and a fenced ```console `content` block - * as the fallback for a UI without terminal support (the fences are a UI-only - * affordance, so they live here, not in `renderResult`). A non-text result - * (unexpected for bash) falls through to `undefined` (UI keeps the raw result). + * stdout/stderr + status markers, exactly as the model sees them — the RAW text, + * newlines preserved, since a terminal renderer relies on exact bytes), and a + * fenced ```console `content` block as the fallback for a UI without terminal + * support (the fences are a UI-only affordance, so they live here, not in the + * model-facing result; the fenced body is trimmed of trailing blank lines for a + * tidy block). A capable UI also gets an exit-status pill from `terminal.exitCode` + * / `terminal.signal`, parsed from the status markers `renderResult` appended + * (this parse is the exact inverse of those markers — they co-evolve in this + * file and a round-trip test guards the pair). A non-text result (unexpected for + * bash) falls through to `undefined` (UI keeps the raw result). */ function presentBashResult(_args: unknown, result: ToolResult): ToolResultPresentation | undefined { const block = result.content.length === 1 ? result.content[0] : undefined if (block === undefined || block.type !== 'text') return undefined - const text = block.text.replace(/\n+$/, '') + const raw = block.text + const fenced = raw.replace(/\n+$/, '') return { - content: [{ type: 'text', text: `\`\`\`console\n${text}\n\`\`\`` }], - terminal: { output: text }, + content: [{ type: 'text', text: `\`\`\`console\n${fenced}\n\`\`\`` }], + terminal: { output: raw, ...parseExitStatus(raw) }, } } +/** + * Recover the structured exit status from a rendered `renderResult` string — the + * inverse of the status markers it appends. A `[killed by signal: SIG]` marker + * yields `{signal}`; otherwise an `[exit code: N]` marker yields `{exitCode:N}`; + * a clean run appends neither, so absent both we report `{exitCode:0}`. (A + * trapped-timeout run that exits 0 has no signal/exit marker either and reads as + * exitCode 0, which is accurate — it did exit 0.) `renderResult` always appends + * the exit/signal marker LAST (after any timeout marker) onto a non-empty body, + * so the marker is anchored at end-of-string here — output that merely CONTAINS + * such text earlier is not mistaken for it. + */ +function parseExitStatus(text: string): { exitCode: number } | { signal: string } { + const signal = /\[killed by signal: ([^\]\n]+)\]$/.exec(text) + if (signal?.[1] !== undefined) return { signal: signal[1] } + const exit = /\[exit code: (\d+)\]$/.exec(text) + if (exit?.[1] !== undefined) return { exitCode: Number(exit[1]) } + return { exitCode: 0 } +} + /** Pending-state presentation for `bash_output`/`bash_kill` (background-task tools). */ function presentTaskCall(verb: string, args: { task_id: string }): ToolCallPresentation { return { title: `${verb} background task ${args.task_id}`, kind: 'execute', rawInput: args.task_id } diff --git a/packages/tool-bash/tests/tools.spec.ts b/packages/tool-bash/tests/tools.spec.ts index a5a5cdb58f..06eb16ac07 100644 --- a/packages/tool-bash/tests/tools.spec.ts +++ b/packages/tool-bash/tests/tools.spec.ts @@ -564,34 +564,74 @@ describe('status lines', () => { }) describe('tool-owned UI presentation (presentCall / presentResult)', () => { - it('bash presentCall: title is "description — command", marks a terminal; explicit absolute workdir → cwd header', async () => { + it('bash presentCall: title is the command, description as a content block, marks a terminal; workdir → cwd (absolute or relative, bridge resolves)', async () => { const ctx = await setup() // No explicit workdir → the call still flags a terminal, but with no cwd (the // UI bridge fills the session cwd it owns; the pure presenter can't see it). + // The command is the title (an execute card hides rawInput); the description + // rides as a content text block (shown above the terminal card). expect(ctx.tools.get('bash')?.presentCall?.({ command: 'ls -la src', description: 'List files in src' })) - .toEqual({ title: 'List files in src — ls -la src', kind: 'execute', rawInput: 'ls -la src', terminal: {} }) - // An explicit ABSOLUTE workdir is surfaced as the terminal cwd header. + .toEqual({ title: 'ls -la src', kind: 'execute', rawInput: 'ls -la src', content: [{ type: 'text', text: 'List files in src' }], terminal: {} }) + // An ABSOLUTE workdir is surfaced verbatim as the terminal cwd header. expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: '/tmp/x' })) - .toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: { cwd: '/tmp/x' } }) - // A RELATIVE workdir is not an absolute cwd → omitted (terminal still flagged). + .toEqual({ title: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: '/tmp/x' } }) + // A RELATIVE workdir is passed through AS-IS (the bridge resolves it against + // the session cwd, matching where execution runs) — not dropped. expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: 'sub' })) - .toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: {} }) + .toEqual({ title: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: 'sub' } }) }) - it('bash presentResult: console-block content AND terminal.output (both renderings of the run)', async () => { + it('bash presentResult: console-block content AND terminal.output (RAW newlines) + parsed exit code', async () => { const ctx = await setup() const present = ctx.tools.get('bash')!.presentResult!( { command: 'echo hi', description: 'echo' }, { content: [{ type: 'text', text: 'hi\n[exit code: 0]\n\n' }], isError: false }, ) - // Trailing blank lines trimmed; content is the fenced ```console fallback, - // terminal.output is the same text for a capable terminal card. + // The fenced ```console content trims trailing blank lines for a tidy block; + // terminal.output keeps the RAW bytes (newlines intact) a terminal renderer + // needs; exitCode is parsed back from the [exit code: N] marker. expect(present).toEqual({ content: [{ type: 'text', text: '```console\nhi\n[exit code: 0]\n```' }], - terminal: { output: 'hi\n[exit code: 0]' }, + terminal: { output: 'hi\n[exit code: 0]\n\n', exitCode: 0 }, }) }) + it('bash presentResult: a non-zero exit and a signal kill parse into exitCode / signal', async () => { + const ctx = await setup() + const args = { command: 'x', description: 'x' } + const nonzero = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: 'oops\n[exit code: 3]' }], isError: false }) + expect(nonzero?.terminal).toEqual({ output: 'oops\n[exit code: 3]', exitCode: 3 }) + const killed = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: 'gone\n[killed by signal: SIGKILL]' }], isError: false }) + expect(killed?.terminal).toEqual({ output: 'gone\n[killed by signal: SIGKILL]', signal: 'SIGKILL' }) + }) + + it('bash presentResult exit parse is the inverse of renderResult markers (round-trip)', async () => { + const ctx = await setup() + const present = ctx.tools.get('bash')! + // For each renderResult outcome, the rendered text fed back through + // presentResult recovers the matching structured exit — the parse and the + // marker emission co-evolve in one file, so this pins the pair. + const base = { + aborted: false, + timeoutMs: 1000, + stdout: { text: 'out', truncated: false }, + stderr: { text: '', truncated: false }, + } + const cases = [ + { result: { ...base, exitCode: 0, signal: null, timedOut: false }, expect: { exitCode: 0 } }, + { result: { ...base, exitCode: 7, signal: null, timedOut: false }, expect: { exitCode: 7 } }, + { result: { ...base, exitCode: null, signal: 'SIGTERM' as const, timedOut: false }, expect: { signal: 'SIGTERM' } }, + // A trapped-timeout run that exits 0 has no signal/exit marker → reads as exit 0 (it did exit 0). + { result: { ...base, exitCode: 0, signal: null, timedOut: true }, expect: { exitCode: 0 } }, + ] + for (const c of cases) { + const rendered = renderResult(c.result) + const out = present.presentResult!({ command: 'x', description: 'x' }, { content: [{ type: 'text', text: rendered }], isError: false }) + const { output: _o, ...exit } = out?.terminal ?? {} + expect(exit).toEqual(c.expect) + } + }) + it('bash presentResult: leaves a non-text (unexpected) result untouched → undefined (UI keeps raw content)', async () => { const ctx = await setup() const present = ctx.tools.get('bash')!.presentResult!( diff --git a/packages/tools/README.md b/packages/tools/README.md index d15b1d1687..6b1634cd70 100644 --- a/packages/tools/README.md +++ b/packages/tools/README.md @@ -72,8 +72,8 @@ See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, an A tool owns how ITS calls render in a UI (an editor's tool-call card, a CLI log line) — a UI plugin must NOT special-case tool names. A `ToolDefinition` may declare two optional, pure, display-only methods: -- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object), and an optional `terminal` (a neutral `{ cwd? }` asking a capable UI to render this call as a TERMINAL, e.g. for `bash`). -- `presentResult(args, result): ToolResultPresentation | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result: an optional replacement `title`, reformatted `content` (e.g. wrap command output in a fenced ` ```console ` block — a UI-only affordance that must NOT appear in the model-facing `execute` result), and an optional `terminal` (the `{ output? }` for a terminal-rendered call). The `ToolTerminal` shape is provider-neutral; a UI bridge (the ACP bridge) maps it to a terminal card and a UI that can't ignores it and uses `content`. +- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object), an optional `content` (UI content shown alongside the title/card — e.g. a bash `description` as a text block above the terminal card), and an optional `terminal` (a neutral `{ cwd? }` asking a capable UI to render this call as a TERMINAL, e.g. for `bash`). +- `presentResult(args, result): ToolResultPresentation | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result: an optional replacement `title`, reformatted `content` (e.g. wrap command output in a fenced ` ```console ` block — a UI-only affordance that must NOT appear in the model-facing `execute` result), and an optional `terminal` (the `{ output?, exitCode?, signal? }` for a terminal-rendered call). The `ToolTerminal` shape is provider-neutral; a UI bridge (the ACP bridge) maps it to a terminal card (with an exit-status pill) and a UI that can't ignores it and uses `content`. Returning `undefined` (or omitting a method) tells a UI to fall back to a generic presentation (title = tool name, raw args as input, raw result content). Both methods must be **pure and side-effect-free**: a UI may call them during live streaming AND during a session-log replay, so they depend only on their arguments. With `defineTool`, `args` is the typed `InferArgs` shape; the helper soft-validates before calling (a malformed/older logged arg shape yields `undefined` rather than throwing, since display must never crash a replay). The shapes are provider-neutral — the ACP bridge (`dsh-acp`) maps them to ACP `tool_call`/`tool_call_update` wire fields, and `dsh-tool-bash` is the reference implementation. @@ -90,8 +90,8 @@ const bash = defineTool({ async execute(args) { return [{ type: 'text', text: `ran: ${args.command}` }] }, - // The model-written description is the readable title; the command is the detail. - presentCall: args => ({ title: args.description, kind: 'execute', rawInput: args.command }), + // The command is the readable title; the description rides as a content block. + presentCall: args => ({ title: args.command, kind: 'execute', rawInput: args.command, content: [{ type: 'text', text: args.description }] }), // Wrap the output as a console block for the UI (not in the model-facing result). presentResult: (_args, result) => { const block = result.content.length === 1 ? result.content[0] : undefined diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index 4de993b5c9..fad513fa52 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -83,6 +83,16 @@ export interface ToolCallPresentation { * unless that is genuinely what a reader wants. */ rawInput?: unknown + /** + * UI-facing content to show on the PENDING call alongside the title/card — + * harness {@link ContentBlock}s, in render order. A terminal tool uses this to + * surface its human-readable `description` as a text block ABOVE the terminal + * card (the card itself is requested via {@link terminal} and labelled by the + * command in `title`), since the card has no description slot. Omit to show no + * extra content. A UI maps these to its own content blocks and renders a + * {@link terminal} block (if any) as a terminal card. + */ + content?: ContentBlock[] /** * Ask a capable UI to render this call as a TERMINAL (a command running in a * working directory), not a generic tool card — set by a tool whose call IS a @@ -96,16 +106,33 @@ export interface ToolCallPresentation { /** * A request to render a tool call as a terminal. The pending presentation * supplies the working directory; the result presentation (see - * {@link ToolResultPresentation.terminal}) supplies the captured output. - * Provider-neutral — no client-protocol types. A UI that supports terminals - * shows a cwd-headed terminal card with the command and its output; a UI that - * does not ignores this and renders the ordinary card/content. + * {@link ToolResultPresentation.terminal}) supplies the captured output and exit + * status. Provider-neutral — no client-protocol types. A UI that supports + * terminals shows a cwd-headed terminal card with the command, its output, and + * an exit-status pill; a UI that does not ignores this and renders the ordinary + * card/content. */ export interface ToolTerminal { - /** Absolute working directory the command ran in, shown as the terminal header. Omit if unknown. */ + /** + * Working directory the command ran in, shown as the terminal header. An + * ABSOLUTE path is used as-is; a RELATIVE path is resolved by the UI bridge + * against the session workspace (the pure tool presenter can't see the + * session cwd). Omit entirely to let the bridge use the session workspace. + */ cwd?: string /** Captured command output (stdout+stderr as the tool chooses to combine them). Result-state only. */ output?: string + /** + * Process exit code, when the run ended by exiting (not a signal). Result-state + * only; lets a capable UI show an exit-status pill on the terminal card. Omit + * when the command was killed by a signal or the exit code is unknown. + */ + exitCode?: number + /** + * Signal name that killed the process (e.g. `SIGTERM`), when it died by signal + * rather than exiting. Result-state only; mutually exclusive with `exitCode`. + */ + signal?: string } /** From 9f6b96c55506648bf4e9d7233fefe72db2010c6c Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 19:35:15 +0800 Subject: [PATCH 5/7] fix(acp): address review of the terminal-card alignment (exit parse, background/error, capability snapshot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex + an independent review pass found three real defects in the prior commit: 1. parseExitStatus could misreport a SUCCESSFUL command as a failure: a clean exit 0 appends no marker, so output ending in "[exit code: 5]" (no trailing newline) was read as the marker. Anchor the parse to a LEADING newline — renderResult always inserts one before a real marker, so a body that merely ends in marker-like text no longer matches. A narrow residual (a clean exit 0 whose final line is exactly the marker) is inherent to the replay-only-sees- text design and documented; the complete fix (a structured exit on the event) is the RFC's named escape hatch. 2. A run_in_background start and an isError result were rendered as exited terminal cards with a false exit-0 pill. A background start returns a task-id ack (not a streamed terminal) and is no longer marked terminal; an isError result (spawn failure / abort) carries no exit pill. 3. The terminal capability was re-read live on the result path, so a second initialize between a call and its result could desync them (orphan terminal_output or clobbered card). Snapshot the capability per session at creation (SessionRecord.terminalEnabled) so call and result always agree. Also reword the reference-parity claim: keeping the description as a content block in terminal mode is a DELIBERATE divergence (claude-agent-acp drops it). Tests added for each; with-key e2e still green. --- ...6-06-18-acp-terminal-and-tool-rendering.md | 2 +- packages/acp/src/index.ts | 25 +++++- packages/acp/tests/turns.spec.ts | 28 ++++++ packages/tool-bash/README.md | 2 +- packages/tool-bash/src/index.ts | 88 +++++++++++++------ packages/tool-bash/tests/tools.spec.ts | 42 +++++++++ 6 files changed, 152 insertions(+), 35 deletions(-) diff --git a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md index d7b0ee12f1..3ff014f55f 100644 --- a/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md +++ b/docs/rfc/implemented/2026-06-18-acp-terminal-and-tool-rendering.md @@ -6,7 +6,7 @@ Status: implemented The ACP bridge lets each tool own its call rendering via `presentCall`/`presentResult` (see [tool-call UI presentation](../proposed/2026-06-14-acp-agent-client-protocol.md) and `packages/tools`). For `bash` we surface the exact command as the `tool_call` title, the model's `description` as a content text block, `kind: 'execute'`, and the completed output wrapped in a fenced ` ```console ` text block. -That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, the command output rendered as a terminal, and an exit-status pill — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why the command IS the title — both reference adapters do the same — and the human-readable description rides as a separate content block, since a terminal card has no description slot.) +That is a correct, capability-free baseline, but not how the reference editors render a *terminal* tool at its best. An editor like Zed has a dedicated terminal tool-call card — a header showing the working directory, the command as the label, the command output rendered as a terminal, and an exit-status pill — but it only builds that card when the `tool_call` carries terminal metadata (below). With a plain text block the output appears as static markdown and there is no cwd header. (Zed also HIDES `rawInput` for `kind: 'execute'`, which is why the command IS the title — both reference adapters do the same. The human-readable description rides as a separate content block above the card; note this is a DELIBERATE divergence — claude-agent-acp DROPS the description in terminal mode and renders only the card — we keep the summary visible alongside.) ## Key finding: agent-executed terminals use a `_meta` convention, NOT `terminal/create` diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index 20d77f9a20..84c1b585ed 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -141,6 +141,17 @@ interface SessionRecord { * tool state. */ presenter: ToolPresenter + /** + * Whether THIS session renders shell tools as terminal cards — snapshotted + * from the client's `_meta.terminal_output` capability at session creation + * (`session/new`/`session/load`), NOT re-read live. A capability snapshot per + * session means the `tool_call` (which registers the terminal) and the matching + * `tool_call_update` (which streams its output) ALWAYS agree, even if a later + * `initialize` mutates the connection-level capability between them — otherwise + * a re-`initialize` mid-call could orphan a `terminal_output` (call non-terminal, + * result terminal) or clobber the card (call terminal, result non-terminal). + */ + terminalEnabled: boolean /** * The in-flight `session/prompt`, or `undefined` when none is pending. A * prompt resolves with a {@link StopReason} or rejects with an Error (a @@ -290,7 +301,7 @@ export function apply(ctx: Context, config: AcpConfig): void { const rec = sessions.get(session.header.id) if (rec === undefined) return streamSessionEventUpdate(rec.sessionId, event, notify, rec.presenter, { - enabled: terminalOutputCap, + enabled: rec.terminalEnabled, cwd: session.header.cwd, }) const inflight = rec.inflight @@ -422,7 +433,7 @@ export function apply(ctx: Context, config: AcpConfig): void { agentOptions: agentOptions(config), }) bySession.set(agent, sessionId) - sessions.set(sessionId, { sessionId, agent, presenter: makePresenter(), inflight: undefined }) + sessions.set(sessionId, { sessionId, agent, presenter: makePresenter(), terminalEnabled: terminalOutputCap, inflight: undefined }) return Promise.resolve({ sessionId }) }, @@ -475,7 +486,13 @@ export function apply(ctx: Context, config: AcpConfig): void { throw invalidParams('connection closed during session/load') } bySession.set(agent, params.sessionId) - const record: SessionRecord = { sessionId: params.sessionId, agent, presenter: makePresenter(), inflight: undefined } + // Snapshot the terminal capability ONCE for this session (used by both + // the replay below and the post-load live stream) so a later + // `initialize` can't desync the call/result of a tool card. + const terminalEnabled = terminalOutputCap + const record: SessionRecord = { + sessionId: params.sessionId, agent, presenter: makePresenter(), terminalEnabled, inflight: undefined, + } sessions.set(params.sessionId, record) // Replay the persisted event log to the client as session/update. Use // the raw event log (NOT deriveMessages, which drops assistant/chunk @@ -492,7 +509,7 @@ export function apply(ctx: Context, config: AcpConfig): void { // so the record's presenter starts clean for the post-load live stream. const replayPresenter = makePresenter() const replayTerminal: TerminalRendering = { - enabled: terminalOutputCap, + enabled: terminalEnabled, cwd: agent.session.header.cwd, } for (const event of agent.session.events) { diff --git a/packages/acp/tests/turns.spec.ts b/packages/acp/tests/turns.spec.ts index 3cf12a67e8..19c3be2556 100644 --- a/packages/acp/tests/turns.spec.ts +++ b/packages/acp/tests/turns.spec.ts @@ -159,6 +159,34 @@ describe('acp bridge — turn outcomes', () => { expect(meta.terminal_exit).toEqual({ terminal_id: 'c1', exit_code: 0 }) }) + it('the terminal capability is snapshotted per-session: a later initialize cannot desync a call/result', async () => { + // The session is created with the capability ON. A SECOND initialize then + // turns it OFF at the connection level — but this session keeps its snapshot, + // so its bash call STILL renders as a terminal card (call + result agree). + // Without the snapshot, the result path would re-read the now-OFF capability + // and either clobber the card (content sent) or be inconsistent with the call. + harness = await makeBridgeHarness({ + storageDir, + withBash: true, + script: [toolCallResponse('c1', 'bash', { command: 'echo hi', description: 'Greet' }), textResponse('done')], + }) + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: { _meta: { terminal_output: true } } }) + const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) + // A re-initialize that DROPS the capability after the session exists. + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] }) + + const call = harness.updates.find(u => u.sessionUpdate === 'tool_call') + if (call?.sessionUpdate !== 'tool_call') throw new Error('expected a tool_call') + // Still a terminal card (the session's snapshot, not the mutated connection cap). + expect((call._meta as { terminal_info?: unknown }).terminal_info).toBeDefined() + const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update') + if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update') + // The result AGREES with the call: terminal output present, content omitted. + expect(update.content).toBeUndefined() + expect((update._meta as { terminal_output?: unknown }).terminal_output).toBeDefined() + }) + it('a throwing tool presenter does not break the turn: the bridge falls back generically', async () => { // A buggy tool whose presentCall throws must not fail the live turn — the // bridge's presenter contains the throw (logging via its onError sink) and diff --git a/packages/tool-bash/README.md b/packages/tool-bash/README.md index 39e56a6126..f5e0e1dfda 100644 --- a/packages/tool-bash/README.md +++ b/packages/tool-bash/README.md @@ -34,7 +34,7 @@ The owning agent is recorded per task id at spawn and kept for the lifetime of t ## UI presentation -These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the exact `command` ("ls -la src") and `kind` is `execute` (terminal/run treatment), matching the reference ACP adapters (claude-agent-acp, codex-acp), which both use the bare command as an execute tool's title. The command is ALSO the **rawInput** for non-terminal UIs that render it (an execute-kind card hides rawInput — Zed shows it only for non-terminal tools — so the command must BE the title to be seen). The model-written `description` rides as a **content** text block shown ABOVE the card (a terminal card has no description slot, so it sits over the command; claude-agent-acp likewise surfaces the description as a separate content block). The completed output is wrapped in a fenced ` ```console ` block as the no-terminal-capability fallback — a UI-only affordance, so the model-facing result text stays unfenced. `bash` also flags itself as a **terminal** (the neutral `terminal` field: `presentCall` sets a `cwd` from the model `workdir` when given — absolute as-is, relative for the UI bridge to resolve against the session cwd — else leaves it for the bridge to fill from the session cwd; `presentResult` carries the raw output plus the parsed `exitCode`/`signal`) so a capable client (Zed) renders a terminal card with an exit-status pill instead of the text block — see `packages/acp` ("Terminal card"). `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Terminal card" / "Tool-call presentation"). +These tools own how their calls render in a UI (an editor's tool-call card) via the `dsh-tools` `presentCall`/`presentResult` seam — a UI never special-cases tool names. For `bash`: the **title** is the exact `command` ("ls -la src") and `kind` is `execute` (terminal/run treatment), matching the reference ACP adapters (claude-agent-acp, codex-acp), which both use the bare command as an execute tool's title. The command is ALSO the **rawInput** for non-terminal UIs that render it (an execute-kind card hides rawInput — Zed shows it only for non-terminal tools — so the command must BE the title to be seen). The model-written `description` rides as a **content** text block shown ABOVE the card. (claude-agent-acp DROPS the description in terminal mode and shows only the card; surfacing it as a content block is a deliberate divergence — we keep the human summary visible alongside the card.) The completed output is wrapped in a fenced ` ```console ` block as the no-terminal-capability fallback — a UI-only affordance, so the model-facing result text stays unfenced. A FOREGROUND `bash` run also flags itself as a **terminal** (the neutral `terminal` field: `presentCall` sets a `cwd` from the model `workdir` when given — absolute as-is, relative for the UI bridge to resolve against the session cwd — else leaves it for the bridge to fill from the session cwd; `presentResult` carries the raw output plus the parsed `exitCode`/`signal`) so a capable client (Zed) renders a terminal card with an exit-status pill instead of the text block — see `packages/acp` ("Terminal card"). A `run_in_background` call is NOT a terminal (it returns a task id immediately and never streams a terminal — poll with `bash_output`), and an `isError` result (spawn failure / abort) carries no exit pill (there is no real process exit); both render as the ordinary execute card / fenced text. `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Terminal card" / "Tool-call presentation"). ## Background completion notices diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index f48b1f7a08..5ea6a93a8f 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -139,23 +139,32 @@ export function renderResult(result: BashRunResult): string { * mirrors the reference ACP adapters (claude-agent-acp, codex-acp), which both * use the bare command as an execute tool's title. The model-written * `description` (a readable summary) rides as a `content` text block shown ABOVE - * the card, since a terminal card has no description slot — claude-agent-acp - * likewise surfaces its description as a separate content block. `rawInput` still - * carries the bare command for non-execute UIs that DO render it. + * the card. (Note: claude-agent-acp DROPS the description in terminal mode and + * shows only the card; surfacing it as a content block is a deliberate + * divergence here — we keep the human summary visible alongside the card.) + * `rawInput` still carries the bare command for non-execute UIs that DO render it. * - * `terminal` marks the call so a capable UI renders a TERMINAL card. Its `cwd` - * (header) is the model `workdir` when given — ABSOLUTE as-is, RELATIVE for the - * UI bridge to resolve against the session cwd; when omitted entirely the bridge - * fills the session workspace cwd (this PURE presenter, args only, can't see it). + * `terminal` marks the call so a capable UI renders a TERMINAL card — but ONLY a + * FOREGROUND run is a terminal: a `run_in_background` call returns a task id + * immediately (it never streams a terminal; its output is polled via + * `bash_output`), so it is NOT marked terminal and renders as an ordinary + * execute card. For a foreground run the `terminal.cwd` (header) is the model + * `workdir` when given — ABSOLUTE as-is, RELATIVE for the UI bridge to resolve + * against the session cwd; when omitted the bridge fills the session workspace + * cwd (this PURE presenter, args only, can't see it). */ -function presentBashCall(args: { command: string; description: string; workdir?: string }): ToolCallPresentation { - return { +type BashCallArgs = { command: string; description: string; workdir?: string; run_in_background?: boolean } + +function presentBashCall(args: BashCallArgs): ToolCallPresentation { + const base = { title: args.command, - kind: 'execute', + kind: 'execute' as const, rawInput: args.command, - content: [{ type: 'text', text: args.description }], - terminal: args.workdir !== undefined ? { cwd: args.workdir } : {}, + content: [{ type: 'text' as const, text: args.description }], } + // A background start is not an interactive terminal — no terminal card. + if (args.run_in_background === true) return base + return { ...base, terminal: args.workdir !== undefined ? { cwd: args.workdir } : {} } } /** @@ -167,37 +176,58 @@ function presentBashCall(args: { command: string; description: string; workdir?: * support (the fences are a UI-only affordance, so they live here, not in the * model-facing result; the fenced body is trimmed of trailing blank lines for a * tidy block). A capable UI also gets an exit-status pill from `terminal.exitCode` - * / `terminal.signal`, parsed from the status markers `renderResult` appended - * (this parse is the exact inverse of those markers — they co-evolve in this - * file and a round-trip test guards the pair). A non-text result (unexpected for - * bash) falls through to `undefined` (UI keeps the raw result). + * / `terminal.signal`, parsed from the status markers `renderResult` appended. + * + * Terminal output/exit is suppressed for results that are NOT a finished + * foreground run: a `run_in_background` start (`isBackground` — the text is a + * task-id ack, not a streamed run) and an `isError` result (a spawn failure or + * abort — there is no real process exit to pill, and the body is an error + * message, not `renderResult` output, so parsing it would be meaningless). Those + * fall back to the fenced `content` block with no terminal metadata. The bridge's + * orphan guard also drops a result terminal when the call wasn't terminal, so a + * background call (not marked terminal in `presentBashCall`) is doubly safe. + * A non-text result (unexpected for bash) falls through to `undefined`. */ -function presentBashResult(_args: unknown, result: ToolResult): ToolResultPresentation | undefined { +function presentBashResult(args: unknown, result: ToolResult): ToolResultPresentation | undefined { const block = result.content.length === 1 ? result.content[0] : undefined if (block === undefined || block.type !== 'text') return undefined const raw = block.text const fenced = raw.replace(/\n+$/, '') - return { - content: [{ type: 'text', text: `\`\`\`console\n${fenced}\n\`\`\`` }], - terminal: { output: raw, ...parseExitStatus(raw) }, - } + const content = [{ type: 'text' as const, text: `\`\`\`console\n${fenced}\n\`\`\`` }] + const isBackground = typeof args === 'object' && args !== null && (args as { run_in_background?: unknown }).run_in_background === true + // No exit pill / terminal output for a background ack or an errored run. + if (isBackground || result.isError) return { content } + return { content, terminal: { output: raw, ...parseExitStatus(raw) } } } /** * Recover the structured exit status from a rendered `renderResult` string — the * inverse of the status markers it appends. A `[killed by signal: SIG]` marker * yields `{signal}`; otherwise an `[exit code: N]` marker yields `{exitCode:N}`; - * a clean run appends neither, so absent both we report `{exitCode:0}`. (A - * trapped-timeout run that exits 0 has no signal/exit marker either and reads as - * exitCode 0, which is accurate — it did exit 0.) `renderResult` always appends - * the exit/signal marker LAST (after any timeout marker) onto a non-empty body, - * so the marker is anchored at end-of-string here — output that merely CONTAINS - * such text earlier is not mistaken for it. + * absent both we report `{exitCode:0}` (a clean run appends no marker — and a + * trapped-timeout run that exits 0 also has none and is accurately exit 0). + * + * Why parse rendered text at all: `presentResult` is replay-safe and on a + * `session/load` the ONLY thing persisted is this content text — the structured + * `BashRunResult` is long gone — so unless the exit were added to the persisted + * event schema (deliberately NOT done; see the terminal-rendering RFC), parsing + * is the only channel. The match is anchored to a LEADING newline + end-of-string + * because `renderResult` always inserts a `\n` before the marker (line ~124) onto + * a non-empty body: a real marker is therefore always its own final line. That + * defeats the common spoof (program output that simply ENDS in `[exit code: 5]` + * with no trailing newline — a clean exit 0 — no longer reads as a failure). + * + * KNOWN RESIDUAL (inherent to the replay-only-sees-text design): a clean exit 0 + * whose body's FINAL line is itself exactly `\n[exit code: N]` (the program + * printed that line and nothing after) is still indistinguishable from a real + * marker and would show a wrong pill. This is display-only (execution and the + * model-facing text are unaffected) and narrow; the complete fix is to persist a + * structured exit on the result event, which the RFC names as the escape hatch. */ function parseExitStatus(text: string): { exitCode: number } | { signal: string } { - const signal = /\[killed by signal: ([^\]\n]+)\]$/.exec(text) + const signal = /\n\[killed by signal: ([^\]\n]+)\]$/.exec(text) if (signal?.[1] !== undefined) return { signal: signal[1] } - const exit = /\[exit code: (\d+)\]$/.exec(text) + const exit = /\n\[exit code: (\d+)\]$/.exec(text) if (exit?.[1] !== undefined) return { exitCode: Number(exit[1]) } return { exitCode: 0 } } diff --git a/packages/tool-bash/tests/tools.spec.ts b/packages/tool-bash/tests/tools.spec.ts index 06eb16ac07..cb54a8a60f 100644 --- a/packages/tool-bash/tests/tools.spec.ts +++ b/packages/tool-bash/tests/tools.spec.ts @@ -632,6 +632,48 @@ describe('tool-owned UI presentation (presentCall / presentResult)', () => { } }) + it('bash presentResult: a clean exit-0 whose output ENDS in marker-like text is NOT read as a failure', async () => { + const ctx = await setup() + const args = { command: 'printf "[exit code: 5]"', description: 'print' } + // A successful command can print text that looks like a marker. renderResult + // for a clean exit 0 appends NOTHING (and no trailing newline), so the body's + // own tail is `[exit code: 5]`. The parse requires a LEADING newline before + // the marker (renderResult always inserts one before a REAL marker), so this + // no-trailing-newline body is NOT mistaken for a failure → exitCode 0. + const out = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: '[exit code: 5]' }], isError: false }) + expect(out?.terminal).toEqual({ output: '[exit code: 5]', exitCode: 0 }) + // Same for a fake signal marker with no leading newline. + const sig = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: '[killed by signal: SIGKILL]' }], isError: false }) + expect(sig?.terminal).toEqual({ output: '[killed by signal: SIGKILL]', exitCode: 0 }) + }) + + it('bash presentCall/presentResult: a run_in_background call is NOT a terminal and its ack carries no exit pill', async () => { + const ctx = await setup() + // The background start returns a task-id ack, not a streamed run — no terminal. + const call = ctx.tools.get('bash')!.presentCall!({ command: 'sleep 100', description: 'wait', run_in_background: true }) + expect(call).toEqual({ title: 'sleep 100', kind: 'execute', rawInput: 'sleep 100', content: [{ type: 'text', text: 'wait' }] }) + expect((call as { terminal?: unknown }).terminal).toBeUndefined() + // The ack result is fenced text only — no terminal output / exit pill. + const result = ctx.tools.get('bash')!.presentResult!( + { command: 'sleep 100', description: 'wait', run_in_background: true }, + { content: [{ type: 'text', text: 'started background task bash-1' }], isError: false }, + ) + expect(result?.terminal).toBeUndefined() + expect(result?.content).toEqual([{ type: 'text', text: '```console\nstarted background task bash-1\n```' }]) + }) + + it('bash presentResult: an isError result carries no exit pill (no real process exit to report)', async () => { + const ctx = await setup() + // A spawn failure / abort has no process exit — the body is an error message, + // not renderResult output, so no terminal output/exit is emitted. + const out = ctx.tools.get('bash')!.presentResult!( + { command: 'x', description: 'x' }, + { content: [{ type: 'text', text: 'command aborted' }], isError: true }, + ) + expect(out?.terminal).toBeUndefined() + expect(out?.content).toEqual([{ type: 'text', text: '```console\ncommand aborted\n```' }]) + }) + it('bash presentResult: leaves a non-text (unexpected) result untouched → undefined (UI keeps raw content)', async () => { const ctx = await setup() const present = ctx.tools.get('bash')!.presentResult!( From 21855d42b74fb1765c400c614a97f8eacd9d4bd8 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 19:45:05 +0800 Subject: [PATCH 6/7] docs(tool-bash): note the parseExitStatus residual applies to the signal marker too Review nit: the KNOWN RESIDUAL comment named only the [exit code: N] case, but the same end-of-string spoof applies to [killed by signal: SIG]. Reword to cover both markers. --- packages/tool-bash/src/index.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index 5ea6a93a8f..7c52705a44 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -218,11 +218,12 @@ function presentBashResult(args: unknown, result: ToolResult): ToolResultPresent * with no trailing newline — a clean exit 0 — no longer reads as a failure). * * KNOWN RESIDUAL (inherent to the replay-only-sees-text design): a clean exit 0 - * whose body's FINAL line is itself exactly `\n[exit code: N]` (the program - * printed that line and nothing after) is still indistinguishable from a real - * marker and would show a wrong pill. This is display-only (execution and the - * model-facing text are unaffected) and narrow; the complete fix is to persist a - * structured exit on the result event, which the RFC names as the escape hatch. + * whose body's FINAL line is itself exactly the marker text — `[exit code: N]` + * or `[killed by signal: SIG]`, printed by the program with nothing after — is + * still indistinguishable from a real marker and would show a wrong pill. This is + * display-only (execution and the model-facing text are unaffected) and narrow; + * the complete fix is to persist a structured exit on the result event, which the + * RFC names as the escape hatch. */ function parseExitStatus(text: string): { exitCode: number } | { signal: string } { const signal = /\n\[killed by signal: ([^\]\n]+)\]$/.exec(text) From a009b1e995d38a2149d84067bbf461f8be0e3456 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 23:01:16 +0800 Subject: [PATCH 7/7] docs(tools): FIXME to rethink the ToolPresentation type shapes The ToolCallPresentation / ToolResultPresentation / ToolTerminal shapes grew incrementally and the responsibility split is now muddy (overlapping call/result terminal fields, the bridge stitching content + terminal + rawInput per call). Flag it as a release-blocking FIXME to redesign around a tool's render INTENT (a tagged union over card kinds) and pin it in an RFC before more tools/UIs depend on the current bag-of-optionals. --- packages/tools/src/index.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index fad513fa52..5730b778c7 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -58,6 +58,18 @@ declare module 'cordis' { */ export type ToolCallKind = 'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other' +// FIXME(tool-presentation): the ToolCallPresentation / ToolResultPresentation / +// ToolTerminal shapes need a rethink. They grew incrementally (title/kind/ +// rawInput, then a `content` block, then a `terminal` sub-shape carrying cwd/ +// output/exit) and the split of responsibility is now muddy: the call vs result +// terminal fields overlap, the bridge has to reconcile a `content` block AND a +// `terminal` block AND `rawInput` per call, and the "pending vs completed" +// boundary doesn't cleanly map to how editors actually render (terminal card, +// diff, generic card). Before more tools/UIs depend on this, redesign the type +// so a tool declares its render INTENT once (e.g. a tagged union over card +// kinds) rather than a bag of optional fields the bridge stitches together. +// Pin the design in an RFC and migrate dsh-tool-bash + the ACP bridge together. + /** * How a tool wants ONE of its calls shown in a UI (an editor's tool-call card, * a CLI log line) BEFORE the result is known — the *pending* state. Provider-