From 1a57d6705848e9f4ae191a3d637081b37df343d7 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:04:03 +0800 Subject: [PATCH 1/5] refactor(tools): tagged render-intent union for tool-call presentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the "bag of optional fields" tool-presentation types (ToolCallPresentation / ToolResultPresentation / ToolTerminal) with a card-tagged discriminated union — the standing FIXME(tool-presentation). A tool declares one render intent per call/result and the ACP bridge switches on `card`: ToolCallView = generic | terminal | diff ToolResultView = generic | terminal The `diff` card is new: fs write/edit now emit an ACP {type:'diff'} content block (an editor's inline diff), which the old shapes could not express. The bridge also relativizes a file card's title against the session cwd (mirroring claude-agent-acp's toDisplayPath) while keeping locations/diff paths raw, and derives the no-capability fenced console fallback from a terminal result's output. read gains the window-in-title (`Read foo.txt (5 - 8)`) and an always-set location line, matching the reference adapter field-for-field. Migrates all three producer families (tool-fs, tool-bash, tool-todo) and the sole consumer (the ACP bridge) together — the source does not compile piecewise. Adds snapshot coverage for the terminal _meta path (a new capability-advertising scenario) and re-records the fs goldens to show the diff cards. Applied-hunk (result-time, context-line) diffs need a new result/event shape and are a follow-up. RFC: docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md --- AGENTS.md | 2 +- docs/cookbook/adding-a-tool.md | 22 +- docs/cordis-catalog/events-and-services.md | 2 +- docs/core-data-structures/core.md | 2 +- docs/core-data-structures/tools.md | 31 +- docs/rfc/README.md | 1 + .../2026-07-02-tool-render-intent-union.md | 70 ++++ ...2026-06-20-core-data-structures-catalog.md | 2 +- examples/acp-agent/tests/acp.snapshot.ts | 1 + .../snapshots/fs-edit/stdout.golden.jsonl | 4 +- .../fs-policy-reject/stdout.golden.jsonl | 2 +- .../fs-read-window/stdout.golden.jsonl | 2 +- .../snapshots/fs-read/stdout.golden.jsonl | 2 +- .../snapshots/fs-terminal-card/input.json | 7 + .../snapshots/fs-terminal-card/session.jsonl | 97 +++++ .../fs-terminal-card/stdout.golden.jsonl | 51 +++ .../fs-write-overwrite/stdout.golden.jsonl | 4 +- .../snapshots/fs-write/stdout.golden.jsonl | 2 +- packages/bash/tool-bash/README.md | 2 +- packages/bash/tool-bash/src/index.ts | 58 +-- packages/bash/tool-bash/tests/tools.spec.ts | 62 ++- packages/core/tools/README.md | 23 +- packages/core/tools/src/index.ts | 232 +++++++----- packages/core/tools/src/schema.ts | 14 +- packages/core/tools/tests/tools.spec.ts | 16 +- packages/fs/tool-fs/src/edit.ts | 16 +- packages/fs/tool-fs/src/read.ts | 25 +- packages/fs/tool-fs/src/write.ts | 18 +- packages/fs/tool-fs/tests/tools.spec.ts | 40 +- packages/todo/tool-todo/src/index.ts | 2 +- .../todo/tool-todo/tests/tool-todo.spec.ts | 2 +- packages/ui/acp/README.md | 18 +- packages/ui/acp/acp-feature-support.md | 4 +- packages/ui/acp/src/index.ts | 358 ++++++++++-------- packages/ui/acp/tests/stream-update.spec.ts | 271 +++++++++++-- 35 files changed, 1015 insertions(+), 450 deletions(-) create mode 100644 docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md create mode 100644 examples/acp-agent/tests/snapshots/fs-terminal-card/input.json create mode 100644 examples/acp-agent/tests/snapshots/fs-terminal-card/session.jsonl create mode 100644 examples/acp-agent/tests/snapshots/fs-terminal-card/stdout.golden.jsonl diff --git a/AGENTS.md b/AGENTS.md index 4bdeb727fe..ec7a41f865 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -250,7 +250,7 @@ Dev/test/demo run **unbuilt** via tsx + the source `paths` map in the root `tsco - **Tests**: vitest, colocated under `packages///tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/core/agent-loop/tests/review-fixes.spec.ts`). The same generosity applies to **real-API (with-key) e2e tests — inference is cheap here (we are DeepSeek), so do not ration them**: cover the agent's real flows (a real prompt that writes a file, multi-turn, tool use, cancellation) and run them frequently while developing, especially cheap **smoke tests** that boot the real example and check the world. A green mock/no-key suite proves the plumbing, not the product — the with-key smoke test is what catches "green units, broken product". See § Secrets / .env for the with-key policy and why self-skip is a CI accommodation, not a verdict that real-API tests are expensive. - **Prefer the REAL implementation over a mock/stand-in in tests.** When the genuine collaborator is available in the repo, wire it up instead of hand-rolling a fake — a test that registers an inline `defineTool({ name: 'bash', … })` to stand in for `dsh-tool-bash` proves the *bridge* moves bytes but not that the *shipping tool* renders the way the test asserts; the two drift and the test passes while the product is wrong. Mock only the genuinely expensive/non-deterministic boundary (the LLM adapter, the network, the clock) and keep everything downstream real: a bridge tool-call test runs the scripted mock MODEL but the REAL tool + REAL executor (e.g. `makeBridgeHarness({ withBash: true })` plugs `dsh-bash-local` + `dsh-tool-bash` and runs an actual `echo`), so it verifies the actual `presentCall`/`presentResult` an editor sees. This is the unit-test echo of "verify the world, not a synthetic stand-in" (see § Defensive patterns) — a fake you wrote will agree with whatever you assumed; the real thing won't. - **A change that affects the editor-facing transcript or end-to-end agent UX needs a snapshot test (or an explicit note in the PR why none applies).** The snapshot tier (`examples/*/tests/**/*.snapshot.ts`, `pnpm run test:snapshot`) boots the real example subprocess, replays a recorded session JSONL deterministically (keyless), and diffs the normalized stdout transcript + re-persisted session log against committed goldens — the full-transcript regression net that mock-level unit tests structurally cannot be (it is what catches a bridge-translation or loop-structure regression that leaves every unit green). When you change the ACP bridge, the agent loop's observable output, tool presentation, or anything an editor renders, add or update a scenario under `examples/acp-agent/tests/snapshots/` and re-record with `pnpm run test:snapshot:record`. Reviewing the golden diff is part of the review. The rule is scoped to transcript/UX-affecting changes — a pure internal refactor with no observable-output change does not need one, but say so. See [docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md](docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md). -- **Designing a new subsystem includes designing its test infrastructure — END TO END, up front, as part of the same plan.** When you introduce a new capability seam, a new agent-lifecycle shape, or anything that produces an observable transcript (a new tool family, a subagent transport, a new UI surface), the plan must name how it will be covered at EVERY tier it touches — unit, real-API e2e, AND the full-transcript snapshot tier — and, critically, must check that the existing test infrastructure can actually express that coverage. Do not assume a snapshot/e2e harness built for one shape (e.g. a single top-level ACP session) transparently supports a new shape (e.g. a parent agent driving nested child agents): verify it, and if it cannot, the harness extension is in-scope work to plan and schedule, not a detail to discover mid-implementation. This rule exists because a real plan under-scoped exactly this: the subagent backends were planned with unit + e2e coverage but the snapshot tier turned out to assume one session per process (`dsh-llm-replay`'s single positional cursor, single-file harvest), so nested-agent snapshot coverage became unplanned net-new infrastructure (`TODO(subagent-snapshots)`). The cost of finding that during design is a paragraph; the cost of finding it mid-build is a re-plan. When the harness gap is large enough to be its own reviewable unit, schedule it as a dedicated stacked follow-up with its own RFC — but SAY SO in the originating plan, with the gap named, rather than letting it surface as a surprise. +- **A tool's editor/ACP representation is part of its design — decide it up front, not after.** When you add or change a model-facing tool, its ACP tool-call card is as much a deliverable as its `execute`: decide which render intent it declares via `presentCall`/`presentResult` (`generic` — a titled card with `kind`/`rawInput`/`content`/`locations`; `terminal` — a shell command; `diff` — a file create/modify rendered as an inline diff), and cover it with a snapshot test (the transcript tier is the only place card rendering is actually verified end-to-end — a unit test on the pure presenter proves the shape, not that an editor renders it). A tool that reads/writes files should almost always emit `locations` (for editor follow-along) and, for a mutation, a `diff` card; a tool that runs a command is a `terminal`. The presentation methods are pure functions of `args` (they run on live streaming AND session-log replay), so they must not do I/O or read session state — the bridge, not the tool, relativizes display paths and fills the session cwd. The reference implementations are `dsh-tool-fs` (generic/diff) and `dsh-tool-bash` (terminal); the vocabulary and the why are pinned in [docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md](docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md), and the step-by-step is in [docs/cookbook/adding-a-tool.md](docs/cookbook/adding-a-tool.md). When you introduce a new capability seam, a new agent-lifecycle shape, or anything that produces an observable transcript (a new tool family, a subagent transport, a new UI surface), the plan must name how it will be covered at EVERY tier it touches — unit, real-API e2e, AND the full-transcript snapshot tier — and, critically, must check that the existing test infrastructure can actually express that coverage. Do not assume a snapshot/e2e harness built for one shape (e.g. a single top-level ACP session) transparently supports a new shape (e.g. a parent agent driving nested child agents): verify it, and if it cannot, the harness extension is in-scope work to plan and schedule, not a detail to discover mid-implementation. This rule exists because a real plan under-scoped exactly this: the subagent backends were planned with unit + e2e coverage but the snapshot tier turned out to assume one session per process (`dsh-llm-replay`'s single positional cursor, single-file harvest), so nested-agent snapshot coverage became unplanned net-new infrastructure (`TODO(subagent-snapshots)`). The cost of finding that during design is a paragraph; the cost of finding it mid-build is a re-plan. When the harness gap is large enough to be its own reviewable unit, schedule it as a dedicated stacked follow-up with its own RFC — but SAY SO in the originating plan, with the gap named, rather than letting it surface as a surprise. ## Defensive patterns (hard-won) diff --git a/docs/cookbook/adding-a-tool.md b/docs/cookbook/adding-a-tool.md index 73706c84f1..c5e7931871 100644 --- a/docs/cookbook/adding-a-tool.md +++ b/docs/cookbook/adding-a-tool.md @@ -48,6 +48,26 @@ Follow tool-bash's background pattern: a `run_in_background` flag returns a task Prefer not to build policy into the tool. The seam is the `tools/execute` waterfall (veto or wrap — see the permission-gate example in [extension-cookbook.md](./extension-cookbook.md)), or a sandboxing implementation behind the tool's executor seam. +## How your tool renders in an editor (ACP presentation) + +Your tool's `execute` returns model-facing content; its **editor card** is a separate, optional concern you declare with two pure display methods on the `defineTool` options. Design this alongside `execute`, not after — an editor (Zed, over the ACP bridge) shows the card, and a tool with no presentation falls back to a bland generic card (title = tool name, raw args as input). + +Both methods return a **`card`-tagged render intent** — pick the card kind that matches what your tool does: + +- `presentCall(args)` → a `ToolCallView` (the PENDING card): + - `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` — the default. Set `kind` for an icon (`read`/`search`/…); set `locations: [{ path, line? }]` for any file your tool touches so a capable editor follows along / jumps to it. + - `{ card: 'terminal', title, description?, cwd? }` — your call IS a shell command. `title` is the command, `description` renders above the terminal card. (tool-bash.) + - `{ card: 'diff', title, diffs, locations? }` — your call creates or modifies a file. `diffs: [{ path, oldText, newText }]` (`oldText: null` for a new file) renders as an inline diff card. (tool-fs `write`/`edit`.) +- `presentResult(args, { content, isError })` → a `ToolResultView` (the COMPLETED card): `{ card: 'generic', title?, content? }` or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the run's captured output + exit — the bridge shows an exit pill and derives a fenced ` ```console ` fallback for editors without the terminal capability). + +Hard rules (they bite if broken): + +- **Purity.** These run on live streaming AND on session-log REPLAY, so they must be pure functions of `args` (+ the result) — NO I/O, NO reading session state, NO clock/random. A diff is derived from the args (`write` uses `oldText: null` because a call-time presenter has no prior file content); the BRIDGE, not the tool, fills the session cwd and relativizes a display-path title. If you find yourself wanting the file's old content or the working directory inside `presentCall`, stop — that belongs on the bridge or a future result-event shape, not the presenter. +- **UI-only formatting stays out of the model result.** A fenced ` ```console ` block, a diff, a relativized path — none of these may appear in what `execute` returns to the model; they live only in the presentation. (A `terminal` result view carries RAW `output`; the bridge adds the fences.) +- **`defineTool` soft-validates the display path.** A malformed/older logged arg shape makes the wrapper return `undefined` (a generic fallback) rather than throw — display must never crash a replay. + +The neutral vocabulary lives in `dsh-tools` (never import an ACP type into a tool); the ACP bridge maps each `card` to the wire. The design and the why are in [the render-intent-union RFC](../rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md); `dsh-tool-fs` (generic/diff) and `dsh-tool-bash` (terminal) are the reference implementations. + ## Tests every tool needs -Arg-validation rejections, result shaping for every outcome, the HMR disposal test, and — for tools with side effects — an integration spec that drives the tool through the agent loop with a scripted `MockAdapter` (`packages/core/agent-loop/tests/mock-adapter.ts`), asserting the `tool/call` / `tool/result` session events. +Arg-validation rejections, result shaping for every outcome, the HMR disposal test, and — for tools with side effects — an integration spec that drives the tool through the agent loop with a scripted `MockAdapter` (`packages/core/agent-loop/tests/mock-adapter.ts`), asserting the `tool/call` / `tool/result` session events. **If your tool has an editor card, also add:** a unit test on `presentCall`/`presentResult` asserting the exact view shape, AND — because a unit test proves the shape but not that an editor renders it — a **snapshot scenario** under `examples/acp-agent/tests/snapshots/` that drives the real tool through the ACP bridge and pins the rendered `tool_call` transcript (the card kind is only verified end-to-end there; see the [ACP snapshot-tests RFC](../rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md)). A tool whose card is a `terminal` needs a scenario whose `input.json` sets `terminalOutput: true` to exercise the capable-client `_meta` path. diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index 22b6dc791b..bb7dcb58a4 100644 --- a/docs/cordis-catalog/events-and-services.md +++ b/docs/cordis-catalog/events-and-services.md @@ -547,7 +547,7 @@ async execute(exec: ToolExecution): Promise Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecution](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) -Source: [`packages/core/tools/src/index.ts:287`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:319`](../../packages/core/tools/src/index.ts) ## Inherited tier (cordis core + loader/hmr/timer) diff --git a/docs/core-data-structures/core.md b/docs/core-data-structures/core.md index 187a4b19a8..2395f180f7 100644 --- a/docs/core-data-structures/core.md +++ b/docs/core-data-structures/core.md @@ -11,7 +11,7 @@ Precisely, a data structure is **core** if either: 1. it flows through the agent-loop spine — the loop holds it, derives it, streams it, or logs it on every turn (a `Message`, a `StreamChunk`, a `SessionEvent`, the `Agent` handle itself), independent of which plugins are present; **or** 2. it is the single headline type a plugin author writes against a pipeline — `ToolDefinition` (what every tool *is*). -Everything else is documented on a **sub-page**, not here. The rule that draws the line: *the type you write, hold, or receive is core; the machinery that types it, renders it, or persists it is a sub-page detail.* So `ToolDefinition` is core, but the `SchemaSpec`/`InferArgs` DSL that types it, the `ToolCallPresentation` vocabulary that renders it, and the `SessionPersistence` seam that stores the event log are not — they live on the sub-pages below. +Everything else is documented on a **sub-page**, not here. The rule that draws the line: *the type you write, hold, or receive is core; the machinery that types it, renders it, or persists it is a sub-page detail.* So `ToolDefinition` is core, but the `SchemaSpec`/`InferArgs` DSL that types it, the `ToolCallView`/`ToolResultView` render-intent vocabulary that renders it, and the `SessionPersistence` seam that stores the event log are not — they live on the sub-pages below. | Sub-page | Owns | |---|---| diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index 1c1744c45c..42aa4e70fd 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -12,21 +12,23 @@ A `ToolSchema` (the model-facing fields) plus the `execute` function and optiona interface ToolDefinition extends ToolSchema { execute(args: unknown, exec: ToolExecution): Promise /** - * Optional: how to present the PENDING state of one call in a UI, derived - * from the call's `args` (parsed arguments, `unknown` — the tool validates/ - * narrows its own input). Returning `undefined` (or omitting the method) tells - * a UI to fall back to a generic presentation (title = tool name, raw args as - * input). Pure and side-effect-free: a UI may call it during live streaming - * AND a session-log replay, so it must depend only on `args`. + * Optional: how to present the PENDING state of one call in a UI, derived from + * the call's `args` (parsed arguments, `unknown` — the tool validates/narrows + * its own input). Returns a {@link ToolCallView} (a `card`-tagged render intent), + * or `undefined` (or omit the method) to fall back to a generic presentation + * (title = tool name, raw args as input). Pure and side-effect-free: a UI may + * call it during live streaming AND a session-log replay, so it must depend + * only on `args`. */ - presentCall?(args: unknown): ToolCallPresentation | undefined + presentCall?(args: unknown): ToolCallView | undefined /** * Optional: how to present the COMPLETED state, given the same `args` and the - * `result` (`execute`'s content + whether it errored). Returning `undefined` - * (or omitting the method) tells a UI to keep the pending title and render the - * raw result content. Pure and side-effect-free for the same replay reason. + * `result` (`execute`'s content + whether it errored). Returns a + * {@link ToolResultView}, or `undefined` (or omit the method) to keep the + * pending title and render the raw result content. Pure and side-effect-free + * for the same replay reason. */ - presentResult?(args: unknown, result: ToolResult): ToolResultPresentation | undefined + presentResult?(args: unknown, result: ToolResult): ToolResultView | undefined } ``` @@ -105,8 +107,11 @@ A waterfall listener receives `(exec, next)`: call `next()` to proceed (possibly ## Tool-presentation UI vocabulary -How a tool wants its call shown in a UI (an editor tool-call card, a CLI log line), provider-neutral so a tool describes itself without depending on any client protocol. `presentCall` returns a `ToolCallPresentation` (pending state: `title`, `kind`, `rawInput`, `content`, `locations` — `{ path, line? }[]` files the call reads/modifies, for editor follow-along — and optional `terminal`); `presentResult` returns a `ToolResultPresentation` (completed state: replacement `title`, reformatted `content`, terminal `output`/exit). `ToolCallKind` (`'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other'`) picks an icon. A `ToolTerminal` asks a capable UI to render the call as a terminal card (cwd header, output, exit-status pill). +How a tool wants its call shown in a UI (an editor tool-call card, a CLI log line), provider-neutral so a tool describes itself without depending on any client protocol. `presentCall`/`presentResult` return a **`card`-tagged render intent** — a discriminated union a UI bridge switches on: -> These shapes carry a `FIXME(tool-presentation)` in source: they grew incrementally and the call-vs-result terminal split is muddy. Before more tools/UIs depend on them, they will be redesigned (a tagged union over card kinds) and pinned in an RFC, migrating `dsh-tool-bash` and the ACP bridge together. Treat the field-level shapes here as provisional; the source is authoritative. +- `ToolCallView` (pending): `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` (the default card; `locations` is `{ path, line? }[]` files the call reads/modifies, for editor follow-along), `{ card: 'terminal', title, description?, cwd? }` (a shell command → a terminal card), or `{ card: 'diff', title, diffs, locations? }` (a file create/modify → an inline diff card; `diffs` is `{ path, oldText, newText }[]`, `oldText: null` for a new file). +- `ToolResultView` (completed): `{ card: 'generic', title?, content? }` or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit; a capable UI shows an exit-status pill, an incapable one gets a fenced ` ```console ` fallback the bridge derives from `output`). + +`ToolCallKind` (`'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other'`) picks an icon on a generic card. `FileLocation` (`{ path, line? }`) and `FileDiff` (`{ path, oldText, newText }`) are the shared file-card vocabulary. The design is pinned in [the render-intent-union RFC](../rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md); the ACP bridge maps a `diff` card to a `{ type: 'diff' }` content block, a `terminal` card to the `_meta` terminal convention, and relativizes a file card's title against the session cwd. The full presentation field docs live in [`packages/core/tools/src/index.ts`](../../packages/core/tools/src/index.ts). The bash tool's own schemas (`bash`/`bash_output`/`bash_kill`) and the executor they drive are on [bash.md](bash.md). diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 59516c24c5..fbefdd3727 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -125,6 +125,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Extract example apps into packages](implemented/architecture/2026-06-20-extract-example-app-packages.md) | 2026-06-20 | | [Make `dsh-fs-policy` an event-gate plugin, not a method interface](implemented/architecture/2026-06-26-file-context-as-event-gate.md) | 2026-06-26 | | [Resolve filesystem paths against the caller's session cwd](implemented/architecture/2026-07-02-fs-per-session-cwd.md) | 2026-07-02 | +| [Tagged render-intent union for tool-call presentation](implemented/architecture/2026-07-02-tool-render-intent-union.md) | 2026-07-02 | ### Process diff --git a/docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md b/docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md new file mode 100644 index 0000000000..1ad5e84e33 --- /dev/null +++ b/docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md @@ -0,0 +1,70 @@ +# RFC: Tagged render-intent union for tool-call presentation + +Status: implemented + +## Problem + +A tool declares how its calls render in a UI (an editor's tool-call card) through two callbacks, `presentCall`/`presentResult` on `ToolDefinition`, returning `ToolCallPresentation` / `ToolResultPresentation` with an optional `ToolTerminal` sub-shape. These grew incrementally into a **bag of optional fields**: `title`, `kind`, `rawInput`, `content`, `locations`, `terminal` on the call; `title`, `content`, `terminal` on the result; `cwd`/`output`/`exitCode`/`signal` on `ToolTerminal`. The split of responsibility is muddy: + +- The call-side and result-side `terminal` fields overlap, and the bridge reconciles a `content` block AND a `terminal` block AND `rawInput` per call, stitching them together with ad-hoc conditionals. +- Which combinations are *valid* is unwritten: a `terminal` call that also sets `content` means "description above the card"; a generic call that sets `terminal` is meaningless but representable. The type permits nonsense. +- There is no way to express the one file-tool affordance an editor most wants — a **diff card** (`{path, oldText, newText}`, which Zed renders as an inline diff / new-file preview). `ToolCallPresentation.content` is the *LLM* `ContentBlock[]` vocabulary (text/image), so a tool literally cannot ask for a diff. + +The existing `FIXME(tool-presentation)` in `packages/core/tools/src/index.ts` named the fix: "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." The rejected RFC [Collapse tool-owned UI presentation](../../rejected/simplification/2026-06-20-generic-tool-rendering.md) deferred it explicitly: rich rendering "should return later as a tagged render-intent union after there are at least two real tools and two real consumers to validate the vocabulary." That bar is now met — two producer families (`dsh-tool-bash`, `dsh-tool-fs`) and two consumers (the ACP bridge live path + the snapshot-golden replay path). + +## Decision + +Replace the optional-field bag with a **`card`-tagged discriminated union**. A tool declares one render intent per call/result; the bridge switches on the tag. + +```ts ignore-check +type FileLocation = { path: string; line?: number } +type FileDiff = { path: string; oldText: string | null; newText: string } // oldText null ⇒ new file + +// presentCall → ToolCallView +type ToolCallView = GenericCallView | TerminalCallView | DiffCallView +interface GenericCallView { card: 'generic'; title: string; kind?: ToolCallKind; rawInput?: unknown; content?: ContentBlock[]; locations?: FileLocation[] } +interface TerminalCallView { card: 'terminal'; title: string; description?: string; cwd?: string } +interface DiffCallView { card: 'diff'; title: string; diffs: FileDiff[]; locations?: FileLocation[] } + +// presentResult → ToolResultView +type ToolResultView = GenericResultView | TerminalResultView +interface GenericResultView { card: 'generic'; title?: string; content?: ContentBlock[] } +interface TerminalResultView { card: 'terminal'; title?: string; output?: string; exitCode?: number; signal?: string } +``` + +`card` is **required** on every variant — a real discriminant, not an optional default. The bridge does `switch (view.card) { case 'generic': … case 'terminal': … case 'diff': … default: assertNever(view) }`. The union is **closed** (per the [switch-exhaustiveness convention](../../../../AGENTS.md)): a fourth render intent (a table, a chart) needs new bridge code to render it anyway, so a plugin-added variant that the bridge silently drops would be worse than a compile error. Adding a variant breaks compilation at the bridge switch — exactly the signal we want. + +### Why a tagged union beats the field-bag + +- **Invalid states become unrepresentable.** A generic card cannot carry terminal output; a terminal card cannot carry a diff. The old bag permitted all of these. +- **The bridge switches instead of stitching.** One arm per card kind, each producing exactly the wire shape that card needs, rather than reconciling five optional fields whose interactions are undocumented. +- **`diff` is a first-class intent.** `dsh-tool-fs` write/edit declare `card:'diff'`; the bridge emits an ACP `{type:'diff', path, oldText, newText}` `ToolCallContent` (already in the SDK's `ToolCallContent` union, previously unused by the bridge). This is the affordance the redesign unlocks. + +### Producer mapping + +- `dsh-tool-fs` read → `generic` (`kind:'read'`, a follow-along `location`); write → `diff` (`oldText:null`); edit → `diff` (`oldText:old_string || null`, `newText:new_string ?? ''`). This mirrors `claude-agent-acp`'s `toolInfoFromToolUse` Read/Write/Edit arms field-for-field. +- `dsh-tool-bash` foreground → `terminal` call + `terminal` result; `run_in_background` and `bash_output`/`bash_kill` → `generic`. +- `dsh-tool-todo` → `generic`. + +### Terminal fallback ownership + +`TerminalResultView` carries only `output`/`exitCode`/`signal`. A UI without the terminal capability needs a fenced ` ```console ` text fallback; that derivation moves to the **bridge** (it wraps `output` in a fenced block on the no-capability path), rather than the tool double-encoding it. This keeps the bash tool's result a single structured shape and preserves the existing capability-gated behavior byte-for-byte. + +### Purity preserved + +`presentCall`/`presentResult` remain pure functions of `args` (+ the result for `presentResult`) — they run on live streaming AND session-log replay, so they must be replay-deterministic. Every view is derived from args alone: write's diff is new-file style (`oldText:null`) because the tool has no old content at call time; edit's diff is `old_string`→`new_string`. + +## Relative-path display titles + +`claude-agent-acp` relativizes a file card's title path against the session cwd (`toDisplayPath`) — `Read src/foo.ts`, not `/abs/proj/src/foo.ts` — while keeping `locations[]`/`diff.path` **raw** (the editor opens the real path). Our `presentCall` is pure/args-only and cannot see the session cwd, so this relativization happens at the **bridge**, which already threads the session cwd into tool-call rendering (the same cwd it uses to resolve a terminal card's header). The bridge relativizes the title only, by an exact structured replace of the known `locations[0].path`/`diffs[0].path` substring — generic over the file-card kinds, never special-casing tool names. + +## Non-goals + +- **Applied-hunk diffs.** `claude-agent-acp` additionally rewrites Write/Edit diffs at *result* time with real structured-patch hunks (via a PostToolUse hook: `toolUpdateFromDiffToolResponse`). Our diffs are call-time and args-derived (the whole `old_string`→`new_string`, no surrounding context lines), because `presentResult` sees only `{content, isError}` and `FsEditOutcome` carries a replacement count/version, not hunk text. Real hunks would need a new result/event shape carrying the patch — a follow-up, not this change. This is the one remaining representation difference from `claude-agent-acp`, and it is architectural (needs a new event), not cosmetic. +- **Live incremental `terminal_output_delta` streaming** and **command classification** — the terminal-rendering RFC's own deferred follow-ups, untouched here. + +## Related + +- Supersedes the deferral in [Collapse tool-owned UI presentation](../../rejected/simplification/2026-06-20-generic-tool-rendering.md) (rejected — "wait for two real tools and two real consumers, then a tagged render-intent union"). That bar is now met; this is that union. +- Folds `ToolTerminal` into the `terminal` views described by [ACP terminal and tool-call rendering](../feature/2026-06-18-acp-terminal-and-tool-rendering.md) (the `_meta` terminal-card convention and capability gate are unchanged; only the harness-side presentation type changes). +- The ACP SDK's `Diff` / `ToolCallContent` types back the new `diff` card. diff --git a/docs/rfc/implemented/process/2026-06-20-core-data-structures-catalog.md b/docs/rfc/implemented/process/2026-06-20-core-data-structures-catalog.md index d589f6136b..d578d8091c 100644 --- a/docs/rfc/implemented/process/2026-06-20-core-data-structures-catalog.md +++ b/docs/rfc/implemented/process/2026-06-20-core-data-structures-catalog.md @@ -23,7 +23,7 @@ The rule that settled the remaining cases: ***the type you write, hold, or recei - A data structure is **core** if it flows through the agent-loop spine — the loop holds, derives, streams, or logs it on every turn regardless of which plugins load (`Message`, `StreamChunk`, `SessionEvent`, the `Agent` handle) — **or** it is the single headline type a plugin author writes against a pipeline (`ToolDefinition`). - `ToolDefinition` is core (it is what every tool author writes) **even though the loop never holds one** — authoring-importance overrides the strict flows-through-spine rule for this one headline type. But its typing machinery — the `SchemaSpec`/`InferArgs` DSL — is a sub-page detail (you write a `ToolDefinition`; the type-level machinery that types it you do not). That is the spine-vs-seam line made sharp. - `ToolSchema` is core (it is a field of `GenerateOptions`, the model request that flows through every step) even though it is conceptually part of the tool pipeline — *flows through the spine* wins over *conceptual home* when they conflict. -- The tool-presentation vocabulary (`ToolCallPresentation`, …, carrying a `FIXME(tool-presentation)` redesign marker), the `SessionPersistence` durability seam, and bash vocabulary are sub-pages. +- The tool-presentation vocabulary (`ToolCallView`/`ToolResultView`, …), the `SessionPersistence` durability seam, and bash vocabulary are sub-pages. `core.md` is a **self-contained spine doc**: it states the exact type definition of each spine structure with minimal prose and links to sub-pages for the per-seam detail. The sub-pages are `llm-streaming.md`, `session.md`, `persistence.md` (split from session along the in-memory-model vs. durability-seam line), `tools.md`, and `bash.md`. diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index 3a27ad25d0..cd14e8dc2e 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -52,6 +52,7 @@ const SCENARIOS: Scenario[] = [ { name: 'reject-extra-dirs', hasModelTurn: false, recorded: false }, { name: 'text-turn', hasModelTurn: true, recorded: true }, { name: 'tool-call-turn', hasModelTurn: true, recorded: true }, + { name: 'fs-terminal-card', hasModelTurn: true, recorded: true }, { name: 'todo-plan', hasModelTurn: true, recorded: true }, { name: 'workspace-edit', hasModelTurn: true, recorded: true }, { name: 'fs-read', hasModelTurn: true, recorded: true }, diff --git a/examples/acp-agent/tests/snapshots/fs-edit/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-edit/stdout.golden.jsonl index 8f5d02632d..f7601d52e3 100644 --- a/examples/acp-agent/tests/snapshots/fs-edit/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-edit/stdout.golden.jsonl @@ -14,7 +14,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" its"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" contents"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_Qm1fHx9Xd5wOv1WZvgmj2865","title":"Read config.txt","kind":"read","status":"in_progress","locations":[{"path":"config.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_Qm1fHx9Xd5wOv1WZvgmj2865","title":"Read config.txt","kind":"read","status":"in_progress","locations":[{"path":"config.txt","line":1}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_Qm1fHx9Xd5wOv1WZvgmj2865","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/config.txt\nfile\n\n1: mode=DEBUG\n2: level=info\n\n(End of file - total 2 lines)\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" file"}}}} @@ -50,7 +50,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" edit"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" tool"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_OwPhDMqS06VEbY7rO4Rv1204","title":"Edit config.txt","kind":"edit","status":"in_progress","rawInput":"\"DEBUG\" → \"RELEASE\"","locations":[{"path":"config.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_OwPhDMqS06VEbY7rO4Rv1204","title":"Edit config.txt","kind":"edit","status":"in_progress","locations":[{"path":"config.txt"}],"content":[{"type":"diff","path":"config.txt","oldText":"DEBUG","newText":"RELEASE"}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_OwPhDMqS06VEbY7rO4Rv1204","status":"completed","content":[{"type":"content","content":{"type":"text","text":"The file {{cwd}}/config.txt has been updated successfully."}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" edit"}}}} diff --git a/examples/acp-agent/tests/snapshots/fs-policy-reject/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-policy-reject/stdout.golden.jsonl index 69c1bbd1c0..0de98c4a44 100644 --- a/examples/acp-agent/tests/snapshots/fs-policy-reject/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-policy-reject/stdout.golden.jsonl @@ -37,7 +37,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" D"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"ONE"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_3fuirRMnjFj7LWlJL1eU3690","title":"Edit settings.txt","kind":"edit","status":"in_progress","rawInput":"\"blue\" → \"green\"","locations":[{"path":"settings.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_3fuirRMnjFj7LWlJL1eU3690","title":"Edit settings.txt","kind":"edit","status":"in_progress","locations":[{"path":"settings.txt"}],"content":[{"type":"diff","path":"settings.txt","oldText":"blue","newText":"green"}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_3fuirRMnjFj7LWlJL1eU3690","status":"failed","content":[{"type":"content","content":{"type":"text","text":"Error: edit requires reading \"{{cwd}}/settings.txt\" first"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" edit"}}}} diff --git a/examples/acp-agent/tests/snapshots/fs-read-window/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-read-window/stdout.golden.jsonl index 577f8adaa6..8b40d5fba5 100644 --- a/examples/acp-agent/tests/snapshots/fs-read-window/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-read-window/stdout.golden.jsonl @@ -24,7 +24,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" "}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"4"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_0htYNlUzC9b8aH2gHN8h2706","title":"Read big.txt","kind":"read","status":"in_progress","rawInput":"offset 5, limit 4","locations":[{"path":"big.txt","line":5}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_0htYNlUzC9b8aH2gHN8h2706","title":"Read big.txt (5 - 8)","kind":"read","status":"in_progress","locations":[{"path":"big.txt","line":5}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_0htYNlUzC9b8aH2gHN8h2706","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/big.txt\nfile\n\n5: line five\n6: line six\n7: line seven\n8: line eight\n\n(Showing lines 5-8 of 10. Use offset=9 to continue.)\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" user"}}}} diff --git a/examples/acp-agent/tests/snapshots/fs-read/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-read/stdout.golden.jsonl index 1d19e735e4..e84a07b931 100644 --- a/examples/acp-agent/tests/snapshots/fs-read/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-read/stdout.golden.jsonl @@ -27,7 +27,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"D"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"ONE"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"\"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_6cBhaXfexPCkwewPFfJd4624","title":"Read greeting.txt","kind":"read","status":"in_progress","locations":[{"path":"greeting.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_6cBhaXfexPCkwewPFfJd4624","title":"Read greeting.txt","kind":"read","status":"in_progress","locations":[{"path":"greeting.txt","line":1}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_6cBhaXfexPCkwewPFfJd4624","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/greeting.txt\nfile\n\n1: hello\n\n(End of file - total 1 lines)\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" file"}}}} diff --git a/examples/acp-agent/tests/snapshots/fs-terminal-card/input.json b/examples/acp-agent/tests/snapshots/fs-terminal-card/input.json new file mode 100644 index 0000000000..de9237ea82 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/fs-terminal-card/input.json @@ -0,0 +1,7 @@ +{ + "steps": [ + { "op": "initialize", "terminalOutput": true }, + { "op": "newSession" }, + { "op": "prompt", "text": "Use the bash tool to run exactly: echo TERMINAL_OK. Then reply with the single word DONE and stop." } + ] +} diff --git a/examples/acp-agent/tests/snapshots/fs-terminal-card/session.jsonl b/examples/acp-agent/tests/snapshots/fs-terminal-card/session.jsonl new file mode 100644 index 0000000000..7c0652d478 --- /dev/null +++ b/examples/acp-agent/tests/snapshots/fs-terminal-card/session.jsonl @@ -0,0 +1,97 @@ +{"type":"session","version":0,"id":"2a35d875-5d43-4d39-a995-a378d341643d","createdAt":1783012637644,"cwd":"/tmp/acp-snap-cwd-o9lBfw"} +{"type":"turn/start","seq":0,"time":1783012637647,"data":{"turn":1,"trigger":{"kind":"message","source":{"kind":"user"}}}} +{"type":"user/message","seq":1,"time":1783012637647,"data":{"content":[{"type":"text","text":"Use the bash tool to run exactly: echo TERMINAL_OK. Then reply with the single word DONE and stop."}],"source":{"kind":"user"}},"surfaceOp":"append"} +{"type":"step/start","seq":2,"time":1783012637648,"data":{"turn":1,"step":1}} +{"type":"assistant/chunk","seq":3,"time":1783012638390,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} +{"type":"assistant/chunk","seq":4,"time":1783012638390,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":"The"}}} +{"type":"assistant/chunk","seq":5,"time":1783012638548,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" user"}}} +{"type":"assistant/chunk","seq":6,"time":1783012638578,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" wants"}}} +{"type":"assistant/chunk","seq":7,"time":1783012638578,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" me"}}} +{"type":"assistant/chunk","seq":8,"time":1783012638578,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" to"}}} +{"type":"assistant/chunk","seq":9,"time":1783012638579,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" run"}}} +{"type":"assistant/chunk","seq":10,"time":1783012638579,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" a"}}} +{"type":"assistant/chunk","seq":11,"time":1783012638604,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" bash"}}} +{"type":"assistant/chunk","seq":12,"time":1783012638634,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" command"}}} +{"type":"assistant/chunk","seq":13,"time":1783012638635,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" and"}}} +{"type":"assistant/chunk","seq":14,"time":1783012638635,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" then"}}} +{"type":"assistant/chunk","seq":15,"time":1783012638663,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" reply"}}} +{"type":"assistant/chunk","seq":16,"time":1783012638663,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" with"}}} +{"type":"assistant/chunk","seq":17,"time":1783012638663,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" a"}}} +{"type":"assistant/chunk","seq":18,"time":1783012638696,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" single"}}} +{"type":"assistant/chunk","seq":19,"time":1783012638697,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":" word"}}} +{"type":"assistant/chunk","seq":20,"time":1783012638697,"data":{"turn":1,"step":1,"chunk":{"type":"reasoning-delta","index":0,"text":"."}}} +{"type":"assistant/chunk","seq":21,"time":1783012638778,"data":{"turn":1,"step":1,"chunk":{"type":"block-start","index":1,"blockType":"tool-call"}}} +{"type":"assistant/chunk","seq":22,"time":1783012638778,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":""}}} +{"type":"assistant/chunk","seq":23,"time":1783012638778,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"{"}}} +{"type":"assistant/chunk","seq":24,"time":1783012638779,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":25,"time":1783012638807,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"command"}}} +{"type":"assistant/chunk","seq":26,"time":1783012638807,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":27,"time":1783012638807,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":": "}}} +{"type":"assistant/chunk","seq":28,"time":1783012638807,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":29,"time":1783012638837,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"echo"}}} +{"type":"assistant/chunk","seq":30,"time":1783012638837,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":" TER"}}} +{"type":"assistant/chunk","seq":31,"time":1783012638838,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"MIN"}}} +{"type":"assistant/chunk","seq":32,"time":1783012638838,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"AL"}}} +{"type":"assistant/chunk","seq":33,"time":1783012638838,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"_OK"}}} +{"type":"assistant/chunk","seq":34,"time":1783012638865,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":35,"time":1783012638894,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":", "}}} +{"type":"assistant/chunk","seq":36,"time":1783012638894,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":37,"time":1783012638894,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"description"}}} +{"type":"assistant/chunk","seq":38,"time":1783012638894,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":39,"time":1783012638894,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":": "}}} +{"type":"assistant/chunk","seq":40,"time":1783012638921,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":41,"time":1783012638921,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"Run"}}} +{"type":"assistant/chunk","seq":42,"time":1783012638921,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":" echo"}}} +{"type":"assistant/chunk","seq":43,"time":1783012638951,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":" TER"}}} +{"type":"assistant/chunk","seq":44,"time":1783012638978,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"MIN"}}} +{"type":"assistant/chunk","seq":45,"time":1783012638978,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"AL"}}} +{"type":"assistant/chunk","seq":46,"time":1783012638978,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"_OK"}}} +{"type":"assistant/chunk","seq":47,"time":1783012638978,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"\""}}} +{"type":"assistant/chunk","seq":48,"time":1783012639008,"data":{"turn":1,"step":1,"chunk":{"type":"tool-call-delta","index":1,"id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","argumentsDelta":"}"}}} +{"type":"assistant/chunk","seq":49,"time":1783012639069,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The user wants me to run a bash command and then reply with a single word."}}}} +{"type":"assistant/chunk","seq":50,"time":1783012639069,"data":{"turn":1,"step":1,"chunk":{"type":"block-end","index":1,"block":{"type":"tool-call","id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","arguments":"{\"command\": \"echo TERMINAL_OK\", \"description\": \"Run echo TERMINAL_OK\"}"}}}} +{"type":"assistant/chunk","seq":51,"time":1783012639069,"data":{"turn":1,"step":1,"chunk":{"type":"usage","usage":{"inputTokens":102,"outputTokens":85,"cacheReadTokens":2176,"reasoningTokens":17}}}} +{"type":"assistant/chunk","seq":52,"time":1783012639069,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"tool-calls"}}}} +{"type":"assistant/message","seq":53,"time":1783012639071,"data":{"turn":1,"step":1,"content":[{"type":"reasoning","text":"The user wants me to run a bash command and then reply with a single word."},{"type":"tool-call","id":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","arguments":"{\"command\": \"echo TERMINAL_OK\", \"description\": \"Run echo TERMINAL_OK\"}"}],"usage":{"inputTokens":102,"outputTokens":85,"cacheReadTokens":2176,"reasoningTokens":17}},"sourceEventSeqs":[3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52],"surfaceOp":"append"} +{"type":"tool/call","seq":54,"time":1783012639071,"data":{"turn":1,"step":1,"callId":"call_00_olli3mOeSioBRKRuiYlA1408","name":"bash","arguments":"{\"command\": \"echo TERMINAL_OK\", \"description\": \"Run echo TERMINAL_OK\"}"}} +{"type":"tool/result","seq":55,"time":1783012639084,"data":{"turn":1,"step":1,"callId":"call_00_olli3mOeSioBRKRuiYlA1408","content":[{"type":"text","text":"TERMINAL_OK\n"}],"isError":false},"sourceEventSeqs":[54],"surfaceOp":"append"} +{"type":"step/end","seq":56,"time":1783012639084,"data":{"turn":1,"step":1}} +{"type":"step/start","seq":57,"time":1783012639085,"data":{"turn":1,"step":2}} +{"type":"assistant/chunk","seq":58,"time":1783012639687,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":0,"blockType":"reasoning"}}} +{"type":"assistant/chunk","seq":59,"time":1783012639687,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"The"}}} +{"type":"assistant/chunk","seq":60,"time":1783012639763,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" command"}}} +{"type":"assistant/chunk","seq":61,"time":1783012639791,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" ran"}}} +{"type":"assistant/chunk","seq":62,"time":1783012639821,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" successfully"}}} +{"type":"assistant/chunk","seq":63,"time":1783012639821,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" and"}}} +{"type":"assistant/chunk","seq":64,"time":1783012639821,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" output"}}} +{"type":"assistant/chunk","seq":65,"time":1783012639848,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" \""}}} +{"type":"assistant/chunk","seq":66,"time":1783012639849,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"TER"}}} +{"type":"assistant/chunk","seq":67,"time":1783012639849,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"MIN"}}} +{"type":"assistant/chunk","seq":68,"time":1783012639849,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"AL"}}} +{"type":"assistant/chunk","seq":69,"time":1783012639849,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"_OK"}}} +{"type":"assistant/chunk","seq":70,"time":1783012639849,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"\"."}}} +{"type":"assistant/chunk","seq":71,"time":1783012639877,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" The"}}} +{"type":"assistant/chunk","seq":72,"time":1783012639877,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" user"}}} +{"type":"assistant/chunk","seq":73,"time":1783012639877,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" asked"}}} +{"type":"assistant/chunk","seq":74,"time":1783012639905,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" me"}}} +{"type":"assistant/chunk","seq":75,"time":1783012639905,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" to"}}} +{"type":"assistant/chunk","seq":76,"time":1783012639905,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" reply"}}} +{"type":"assistant/chunk","seq":77,"time":1783012639906,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" with"}}} +{"type":"assistant/chunk","seq":78,"time":1783012639906,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" the"}}} +{"type":"assistant/chunk","seq":79,"time":1783012639933,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" single"}}} +{"type":"assistant/chunk","seq":80,"time":1783012639934,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" word"}}} +{"type":"assistant/chunk","seq":81,"time":1783012639934,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" D"}}} +{"type":"assistant/chunk","seq":82,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"ONE"}}} +{"type":"assistant/chunk","seq":83,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" and"}}} +{"type":"assistant/chunk","seq":84,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":" stop"}}} +{"type":"assistant/chunk","seq":85,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"reasoning-delta","index":0,"text":"."}}} +{"type":"assistant/chunk","seq":86,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"block-start","index":1,"blockType":"text"}}} +{"type":"assistant/chunk","seq":87,"time":1783012639963,"data":{"turn":1,"step":2,"chunk":{"type":"text-delta","index":1,"text":"D"}}} +{"type":"assistant/chunk","seq":88,"time":1783012639991,"data":{"turn":1,"step":2,"chunk":{"type":"text-delta","index":1,"text":"ONE"}}} +{"type":"assistant/chunk","seq":89,"time":1783012639992,"data":{"turn":1,"step":2,"chunk":{"type":"block-end","index":0,"block":{"type":"reasoning","text":"The command ran successfully and output \"TERMINAL_OK\". The user asked me to reply with the single word DONE and stop."}}}} +{"type":"assistant/chunk","seq":90,"time":1783012639992,"data":{"turn":1,"step":2,"chunk":{"type":"block-end","index":1,"block":{"type":"text","text":"DONE"}}}} +{"type":"assistant/chunk","seq":91,"time":1783012639992,"data":{"turn":1,"step":2,"chunk":{"type":"usage","usage":{"inputTokens":204,"outputTokens":30,"cacheReadTokens":2176,"reasoningTokens":27}}}} +{"type":"assistant/chunk","seq":92,"time":1783012639992,"data":{"turn":1,"step":2,"chunk":{"type":"finish","reason":{"kind":"stop"}}}} +{"type":"assistant/message","seq":93,"time":1783012639992,"data":{"turn":1,"step":2,"content":[{"type":"reasoning","text":"The command ran successfully and output \"TERMINAL_OK\". The user asked me to reply with the single word DONE and stop."},{"type":"text","text":"DONE"}],"usage":{"inputTokens":204,"outputTokens":30,"cacheReadTokens":2176,"reasoningTokens":27}},"sourceEventSeqs":[58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92],"surfaceOp":"append"} +{"type":"step/end","seq":94,"time":1783012639992,"data":{"turn":1,"step":2}} +{"type":"turn/end","seq":95,"time":1783012639993,"data":{"turn":1,"reason":{"kind":"completed"}}} diff --git a/examples/acp-agent/tests/snapshots/fs-terminal-card/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-terminal-card/stdout.golden.jsonl new file mode 100644 index 0000000000..6bbff6b55c --- /dev/null +++ b/examples/acp-agent/tests/snapshots/fs-terminal-card/stdout.golden.jsonl @@ -0,0 +1,51 @@ +{"jsonrpc":"2.0","id":1,"result":{"protocolVersion":1,"agentInfo":{"name":"deepseek-harness-acp","version":"0.0.1"},"agentCapabilities":{"loadSession":true,"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"authMethods":[]}} +{"jsonrpc":"2.0","id":2,"result":{"sessionId":"{{sessionId}}"}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" user"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" wants"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" me"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" to"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" run"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" a"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" bash"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" command"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" and"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" then"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" reply"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" with"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" a"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" single"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" word"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_olli3mOeSioBRKRuiYlA1408","title":"echo TERMINAL_OK","kind":"execute","status":"in_progress","rawInput":"echo TERMINAL_OK","content":[{"type":"content","content":{"type":"text","text":"Run echo TERMINAL_OK"}},{"type":"terminal","terminalId":"call_00_olli3mOeSioBRKRuiYlA1408"}],"_meta":{"terminal_info":{"terminal_id":"call_00_olli3mOeSioBRKRuiYlA1408","cwd":"{{cwd}}"}}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_olli3mOeSioBRKRuiYlA1408","status":"completed","_meta":{"terminal_output":{"terminal_id":"call_00_olli3mOeSioBRKRuiYlA1408","data":"TERMINAL_OK\n"},"terminal_exit":{"terminal_id":"call_00_olli3mOeSioBRKRuiYlA1408","exit_code":0}}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" command"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" ran"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" successfully"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" and"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" output"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" \""}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"TER"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"MIN"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"AL"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"_OK"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"\"."}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" The"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" user"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" asked"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" me"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" to"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" reply"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" with"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" the"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" single"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" word"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" D"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"ONE"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" and"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" stop"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"D"}}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"ONE"}}}} +{"jsonrpc":"2.0","id":3,"result":{"stopReason":"end_turn"}} diff --git a/examples/acp-agent/tests/snapshots/fs-write-overwrite/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-write-overwrite/stdout.golden.jsonl index 1d234c2dd4..b595a90083 100644 --- a/examples/acp-agent/tests/snapshots/fs-write-overwrite/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-write-overwrite/stdout.golden.jsonl @@ -28,7 +28,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"D"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"ONE"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"\"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_GtqlR9riew6wgdQzLbbu6019","title":"Read data.txt","kind":"read","status":"in_progress","locations":[{"path":"data.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_GtqlR9riew6wgdQzLbbu6019","title":"Read data.txt","kind":"read","status":"in_progress","locations":[{"path":"data.txt","line":1}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_GtqlR9riew6wgdQzLbbu6019","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/data.txt\nfile\n\n1: original contents\n\n(End of file - total 1 lines)\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"Now"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" I"}}}} @@ -50,7 +50,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"re"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"placed"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"\"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_CkV4RzKjuERcr4NdJbtY3226","title":"Write data.txt","kind":"edit","status":"in_progress","locations":[{"path":"data.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_CkV4RzKjuERcr4NdJbtY3226","title":"Write data.txt","kind":"edit","status":"in_progress","locations":[{"path":"data.txt"}],"content":[{"type":"diff","path":"data.txt","oldText":null,"newText":"replaced"}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_CkV4RzKjuERcr4NdJbtY3226","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/data.txt\nfile\n\nUpdated file\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"Done"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"."}}}} diff --git a/examples/acp-agent/tests/snapshots/fs-write/stdout.golden.jsonl b/examples/acp-agent/tests/snapshots/fs-write/stdout.golden.jsonl index daa26245bb..eac3a6ea99 100644 --- a/examples/acp-agent/tests/snapshots/fs-write/stdout.golden.jsonl +++ b/examples/acp-agent/tests/snapshots/fs-write/stdout.golden.jsonl @@ -31,7 +31,7 @@ {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"D"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"ONE"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"\"."}}}} -{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_OxAUP9dIc6I1B6Coo5vs8586","title":"Write notes.txt","kind":"edit","status":"in_progress","locations":[{"path":"notes.txt"}]}}} +{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call","toolCallId":"call_00_OxAUP9dIc6I1B6Coo5vs8586","title":"Write notes.txt","kind":"edit","status":"in_progress","locations":[{"path":"notes.txt"}],"content":[{"type":"diff","path":"notes.txt","oldText":null,"newText":"hello world"}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"tool_call_update","toolCallId":"call_00_OxAUP9dIc6I1B6Coo5vs8586","status":"completed","content":[{"type":"content","content":{"type":"text","text":"{{cwd}}/notes.txt\nfile\n\nCreated file\n"}}]}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":"The"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"{{sessionId}}","update":{"sessionUpdate":"agent_thought_chunk","content":{"type":"text","text":" file"}}}} diff --git a/packages/bash/tool-bash/README.md b/packages/bash/tool-bash/README.md index 656a22cdb1..29cec70970 100644 --- a/packages/bash/tool-bash/README.md +++ b/packages/bash/tool-bash/README.md @@ -34,7 +34,7 @@ The owning agent's session token (`session.header.id`) is stamped onto the task ## 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. (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/ui/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/core/tools` ("Tool-owned UI presentation") and `packages/ui/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, each returning a `card`-tagged render intent — a UI never special-cases tool names. A FOREGROUND `bash` run declares a **terminal card**: `presentCall` returns `{ card: 'terminal', title, description?, cwd? }` — the **title** is the exact `command` ("ls -la src"), the model-written `description` rides along (rendered ABOVE the card), and `cwd` comes from the model `workdir` when given (absolute as-is, relative for the UI bridge to resolve against the session cwd; else left for the bridge to fill from the session cwd) — and `presentResult` returns `{ card: 'terminal', title?, output?, exitCode?, signal? }` carrying the raw output plus the parsed `exitCode`/`signal`, so a capable client (Zed) renders a terminal card with an exit-status pill. The result carries the raw `output`; the bridge DERIVES the ` ```console ` fenced fallback for a no-terminal-capability UI (the tool no longer encodes the fences itself), so the model-facing result text stays unfenced. 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 instead returns a **generic card** (`{ card: 'generic', title, kind: 'execute', rawInput: command, content: [description] }`); an `isError` result (spawn failure / abort) likewise returns a `generic` result view with no exit pill (there is no real process exit). `bash_output`/`bash_kill` return a `generic` card with a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") and 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/core/tools` ("Tool-owned UI presentation") and `packages/ui/acp` ("Terminal card" / "Tool-call presentation"). ## Background completion notices diff --git a/packages/bash/tool-bash/src/index.ts b/packages/bash/tool-bash/src/index.ts index 9ad1f9a17c..29a6475c2c 100644 --- a/packages/bash/tool-bash/src/index.ts +++ b/packages/bash/tool-bash/src/index.ts @@ -41,7 +41,7 @@ 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 { GenericCallView, TerminalCallView, ToolResult, ToolResultView } from '@deepseek-ai/dsh-tools' import type { Agent } from '@deepseek-ai/dsh-agent' import { BashTaskId, OwnerToken } from '@deepseek-ai/dsh-bash' import type { BashRunResult, BashTask, CollectedOutput } from '@deepseek-ai/dsh-bash' @@ -158,16 +158,26 @@ export function renderResult(result: BashRunResult): string { */ type BashCallArgs = { command: string; description: string; workdir?: string; run_in_background?: boolean } -function presentBashCall(args: BashCallArgs): ToolCallPresentation { - const base = { - title: args.command, - kind: 'execute' as const, - rawInput: args.command, - content: [{ type: 'text' as const, text: args.description }], +function presentBashCall(args: BashCallArgs): GenericCallView | TerminalCallView { + // A background start is not an interactive terminal — a generic execute card + // with the command as rawInput and the description as a content block. + if (args.run_in_background === true) { + return { + card: 'generic', + title: args.command, + kind: 'execute', + rawInput: args.command, + content: [{ type: 'text', text: args.description }], + } + } + // A foreground run IS a terminal: the command titles the card, the description + // renders above it, and the cwd (when the model gave a workdir) heads it. + return { + card: 'terminal', + title: args.command, + description: args.description, + ...args.workdir !== undefined ? { cwd: args.workdir } : {}, } - // 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 } : {} } } /** @@ -186,21 +196,25 @@ function presentBashCall(args: BashCallArgs): ToolCallPresentation { * 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`. + * return a `generic` result whose content is the fenced ```console block. A + * finished foreground run returns a `terminal` result carrying the RAW output + * and the parsed exit status; the BRIDGE derives the fenced fallback from + * `output` for a UI without terminal support, so the tool does not double-encode + * it. 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): ToolResultView | 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+$/, '') - 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) } } + // A background ack or an errored run is not a real terminal exit: render the + // fenced ```console fallback as generic content (no exit pill). + if (isBackground || result.isError) { + return { card: 'generic', content: [{ type: 'text', text: `\`\`\`console\n${raw.replace(/\n+$/, '')}\n\`\`\`` }] } + } + // A finished foreground run: RAW output + parsed exit for the terminal card. + // The bridge derives the no-capability fenced fallback from `output`. + return { card: 'terminal', output: raw, ...parseExitStatus(raw) } } /** @@ -237,8 +251,8 @@ function parseExitStatus(text: string): { exitCode: number } | { signal: string } /** 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 } +function presentTaskCall(verb: string, args: { task_id: string }): GenericCallView { + return { card: 'generic', title: `${verb} background task ${args.task_id}`, kind: 'execute', rawInput: args.task_id } } /** diff --git a/packages/bash/tool-bash/tests/tools.spec.ts b/packages/bash/tool-bash/tests/tools.spec.ts index 9de079fd33..368ed4fdd7 100644 --- a/packages/bash/tool-bash/tests/tools.spec.ts +++ b/packages/bash/tool-bash/tests/tools.spec.ts @@ -716,45 +716,40 @@ describe('status lines', () => { }) describe('tool-owned UI presentation (presentCall / presentResult)', () => { - it('bash presentCall: title is the command, description as a content block, marks a terminal; workdir → cwd (absolute or relative, bridge resolves)', async () => { + it('bash presentCall: a foreground run is a terminal card (command title, description, workdir → cwd absolute or relative)', 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). + // No explicit workdir → a terminal card 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: 'ls -la src', kind: 'execute', rawInput: 'ls -la src', content: [{ type: 'text', text: 'List files in src' }], terminal: {} }) + .toEqual({ card: 'terminal', title: 'ls -la src', description: 'List files in src' }) // 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: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: '/tmp/x' } }) + .toEqual({ card: 'terminal', title: 'pwd', description: 'Print dir', 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: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: 'sub' } }) + .toEqual({ card: 'terminal', title: 'pwd', description: 'Print dir', cwd: 'sub' }) }) - it('bash presentResult: console-block content AND terminal.output (RAW newlines) + parsed exit code', async () => { + it('bash presentResult: a terminal result carries RAW output (newlines intact) + 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 }, ) - // 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]\n\n', exitCode: 0 }, - }) + // A terminal result keeps the RAW bytes (newlines intact) a terminal renderer + // needs; the bridge derives the fenced fallback. exitCode is parsed back from + // the [exit code: N] marker. + expect(present).toEqual({ card: '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 }) + expect(nonzero).toEqual({ card: 'terminal', 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' }) + expect(killed).toEqual({ card: 'terminal', output: 'gone\n[killed by signal: SIGKILL]', signal: 'SIGKILL' }) }) it('bash presentResult exit parse is the inverse of renderResult markers (round-trip)', async () => { @@ -779,7 +774,8 @@ describe('tool-owned UI presentation (presentCall / presentResult)', () => { 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 ?? {} + // Drop card + output; the remaining fields are the parsed exit. + const { card: _c, output: _o, ...exit } = out as { card: string; output?: string; exitCode?: number; signal?: string } expect(exit).toEqual(c.expect) } }) @@ -793,37 +789,35 @@ describe('tool-owned UI presentation (presentCall / presentResult)', () => { // 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 }) + expect(out).toEqual({ card: 'terminal', 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 }) + expect(sig).toEqual({ card: 'terminal', 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 () => { + it('bash presentCall/presentResult: a run_in_background call is a generic card 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. + // The background start returns a task-id ack, not a streamed run — a generic + // execute card with the command as rawInput and the description as content. 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. + expect(call).toEqual({ card: 'generic', title: 'sleep 100', kind: 'execute', rawInput: 'sleep 100', content: [{ type: 'text', text: 'wait' }] }) + // The ack result is a generic fenced-text card — 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```' }]) + expect(result).toEqual({ card: 'generic', content: [{ 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 () => { + it('bash presentResult: an isError result is a generic card (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. + // not renderResult output, so a generic fenced card, no terminal output/exit. 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```' }]) + expect(out).toEqual({ card: 'generic', content: [{ type: 'text', text: '```console\ncommand aborted\n```' }] }) }) it('bash presentResult: leaves a non-text (unexpected) result untouched → undefined (UI keeps raw content)', async () => { @@ -849,9 +843,9 @@ describe('tool-owned UI presentation (presentCall / presentResult)', () => { it('bash_output / bash_kill presentCall: a readable task-scoped title, task id as rawInput', async () => { const ctx = await setup() expect(ctx.tools.get('bash_output')!.presentCall!({ task_id: 'bash-3' })) - .toEqual({ title: 'Read output from background task bash-3', kind: 'execute', rawInput: 'bash-3' }) + .toEqual({ card: 'generic', title: 'Read output from background task bash-3', kind: 'execute', rawInput: 'bash-3' }) expect(ctx.tools.get('bash_kill')!.presentCall!({ task_id: 'bash-3' })) - .toEqual({ title: 'Kill background task bash-3', kind: 'execute', rawInput: 'bash-3' }) + .toEqual({ card: 'generic', title: 'Kill background task bash-3', kind: 'execute', rawInput: 'bash-3' }) }) it('presentCall validates softly: malformed args (missing required description) return undefined, never throw', async () => { diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 01a2e09d3b..a01881de5b 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -27,7 +27,7 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e - `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise`, plus optional `presentCall(args)` / `presentResult(args, result)` for tool-owned UI presentation (see below). - `ToolExecution` — one pending tool call: `{ callId, name, arguments, agent?, signal? }`. - `ToolExecutionResult` — outcome: `{ callId, content, isError, error? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). -- `ToolCallPresentation` / `ToolResultPresentation` — provider-neutral shapes a tool returns from `presentCall` / `presentResult` to own how a UI renders ITS calls (see "Tool-owned UI presentation"). +- `ToolCallView` / `ToolResultView` — provider-neutral `card`-tagged render intents a tool returns from `presentCall` / `presentResult` to own how a UI renders ITS calls (see "Tool-owned UI presentation"). ### Extension points @@ -70,12 +70,17 @@ See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, an ### Tool-owned UI presentation -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: +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 that return a **`card`-tagged render intent** (a discriminated union — a tool declares its card kind once and a UI bridge switches on `card`): -- `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), an optional `locations` (`{ path, line? }[]` — the files this call reads/modifies, so a capable UI can follow along / jump to them; the ACP bridge forwards them as `tool_call.locations`), 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`. +- `presentCall(args): ToolCallView | undefined` — the PENDING state, one of: + - `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` — the default card: a human-readable `title`, 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 background task id, NOT the whole args object), optional `content` (extra UI content blocks), and optional `locations` (`{ path, line? }[]` — files this call reads/modifies, so a capable UI can follow along; the ACP bridge forwards them as `tool_call.locations`). + - `{ card: 'terminal', title, description?, cwd? }` — a shell command: a capable UI renders a terminal card (the `title` is the command, `description` renders above it, `cwd` heads it); an incapable UI falls back to a generic execute card. + - `{ card: 'diff', title, diffs, locations? }` — a file create/modify: a capable UI renders an inline diff card from `diffs` (`{ path, oldText, newText }[]`; `oldText: null` for a new file). Used by `write`/`edit`. +- `presentResult(args, result): ToolResultView | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result, one of: + - `{ card: 'generic', title?, content? }` — an optional replacement `title` and reformatted `content`. + - `{ card: 'terminal', title?, output?, exitCode?, signal? }` — a terminal run's captured `output` and exit status. A capable UI shows an exit-status pill; an incapable UI gets a fenced ` ```console ` fallback the BRIDGE derives from `output` (the tool does not encode the fences). -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. +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 views are provider-neutral — the ACP bridge (`dsh-acp`) maps each `card` to ACP `tool_call`/`tool_call_update` wire fields (a `diff` card to a `{ type: 'diff' }` content block, a `terminal` card to the `_meta` terminal convention), and relativizes a file card's title against the session cwd. See the render-intent-union RFC (`docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md`); `dsh-tool-bash` (terminal) and `dsh-tool-fs` (diff/generic) are the reference implementations. ```ts import { defineTool } from '@deepseek-ai/dsh-tools' @@ -90,13 +95,13 @@ const bash = defineTool({ async execute(args) { return [{ type: 'text', text: `ran: ${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). + // A terminal card: the command is the title, the description renders above it. + presentCall: args => ({ card: 'terminal', title: args.command, description: args.description }), + // A terminal result: the raw output + exit; the bridge derives the fenced fallback. presentResult: (_args, result) => { const block = result.content.length === 1 ? result.content[0] : undefined if (block === undefined || block.type !== 'text') return undefined - return { content: [{ type: 'text', text: '```console\n' + block.text + '\n```' }] } + return { card: 'terminal', output: block.text } }, }) ``` diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index c2f324e87b..594e5761cb 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -62,150 +62,182 @@ 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. +/** + * A file location a tool reads or modifies, so a capable UI can "follow along" — + * highlight or jump to the file (and line) as the tool runs. Provider-neutral; + * a UI bridge maps it to its own affordance (the ACP bridge forwards it as + * `tool_call.locations`). `path` is what the tool operated on (the model-facing + * path); `line` is an optional 1-based line to focus (e.g. a read's offset). + */ +export interface FileLocation { + path: string + line?: number +} /** - * 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- - * neutral: a tool returns this from {@link ToolDefinition.presentCall} and a UI - * plugin (e.g. the ACP bridge) maps it to its own wire shape. The tool owns its - * own presentation — the UI must not special-case tool names. + * A single-file change a tool is about to make, for a UI that renders inline + * diffs (an editor's diff card). Provider-neutral; the ACP bridge forwards it as + * a `{ type: 'diff' }` tool-call content block. `oldText` is `null` for a + * new-file create (nothing to diff against); an overwrite also uses `null`, + * because a call-time presenter has no access to the file's prior content. */ -export interface ToolCallPresentation { +export interface FileDiff { + path: string + /** Prior content, or `null` for a new file / an overwrite (no prior content available at call time). */ + oldText: string | null + /** Content after the change. */ + newText: string +} + +/** + * 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. A `card`-tagged + * discriminated union: a tool declares its render INTENT once and a UI bridge + * switches on `card` to map it to the bridge's own wire shape. Provider-neutral — + * the tool owns its presentation, so a UI never special-cases tool names. + * + * Returned by {@link ToolDefinition.presentCall}. See the render-intent-union + * RFC (docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md). + */ +export type ToolCallView = GenericCallView | TerminalCallView | DiffCallView + +/** + * The default card: a titled tool-call row with an optional category icon, a + * salient raw input, extra content blocks, and follow-along file locations. Any + * tool whose call is not a terminal or a diff uses this. + */ +export interface GenericCallView { + card: 'generic' /** - * Human-readable, always-visible label describing what THIS call does (e.g. - * the model-written one-line summary of a bash command). Keep it short — a UI - * shows it as a card header / log line. Required: a presentation must have a - * title (a UI falls back to the tool name only when `presentCall` is absent). + * Human-readable, always-visible label describing what THIS call does. Keep it + * short — a UI shows it as a card header / log line. */ title: string /** Category for icon/treatment; defaults to `other` when omitted. */ kind?: ToolCallKind /** - * The salient input to surface in a detail/expanded view — e.g. the bash - * COMMAND itself (as a string), so the title can stay a readable summary - * while the exact command is still visible. Omit to show nothing; a string is - * rendered as-is, an object as pretty JSON. NOT the full raw args object - * unless that is genuinely what a reader wants. + * The salient input to surface in a detail/expanded view (e.g. a background + * task id). Omit to show nothing; a string renders as-is, an object as pretty + * JSON. NOT the full raw args object 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. + * UI-facing content blocks to show on the pending call alongside the title. + * Omit to show none. A UI maps these to its own content blocks. */ content?: ContentBlock[] - /** - * Files this call reads or modifies, so a capable UI can "follow along" — - * highlight or jump to the file (and line) as the tool runs. Provider-neutral - * `{ path, line? }` pairs; a UI bridge maps them to its own affordance (the ACP - * bridge forwards them as `tool_call.locations`). `path` is what the tool - * operated on (the model-facing path); `line` is an optional 1-based line to - * focus (e.g. a read's offset). Omit for a call that touches no file (e.g. - * `bash`). - */ - locations?: { path: string; line?: number }[] - /** - * 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 + /** Files this call reads/modifies, for editor follow-along. Omit for a call that touches no file. */ + locations?: FileLocation[] } /** - * 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 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. + * A call that IS a shell command running in a working directory: a capable UI + * renders it as a terminal card (cwd-headed, with the command as the title and + * live/afterward output from the {@link TerminalResultView}); an incapable UI + * falls back to a generic card whose body is the fenced command output. Set by a + * tool whose call is a foreground command (e.g. `bash`). */ -export interface ToolTerminal { +export interface TerminalCallView { + card: 'terminal' + /** The command, shown as the terminal card's title / header line. */ + title: string /** - * Working directory the command ran in, shown as the terminal header. An + * A human-readable one-line summary of what the command does, rendered ABOVE + * the terminal card (the card itself has no description slot). Omit for none. + */ + description?: string + /** + * Working directory the command runs 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. + * against the session workspace (the pure 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 } /** - * How a tool wants the COMPLETED call shown — the *result* state, after - * `execute` returns. Lets the tool reformat its result for a UI distinctly from - * the model-facing text it returned from `execute` (e.g. wrap command output in - * a fenced ```console block for monospace rendering, which the model-facing - * result must NOT carry). All fields optional: a UI keeps the pending-state - * title and renders the raw result content for anything left unset. + * A call that creates or modifies files, rendered as an inline diff card by a + * capable UI. Set by a tool whose call writes/edits a file (e.g. `write`, + * `edit`). The diffs are derived from the call ARGUMENTS (a create's `oldText` is + * `null`); result-time applied-hunk diffs are a separate follow-up. */ -export interface ToolResultPresentation { - /** Replacement title for the completed call (e.g. append an exit status). Omit to keep the pending-state title. */ +export interface DiffCallView { + card: 'diff' + /** Card header (e.g. `Write foo.txt`). */ + title: string + /** One entry per file the call changes. */ + diffs: FileDiff[] + /** Files this call modifies, for editor follow-along (usually the diffs' paths). */ + locations?: FileLocation[] +} + +/** + * How a tool wants the COMPLETED call shown — the *result* state, after `execute` + * returns. A `card`-tagged union mirroring {@link ToolCallView}: a UI switches on + * `card`. Lets the tool reformat its result for a UI distinctly from the + * model-facing text it returned from `execute`. Returned by + * {@link ToolDefinition.presentResult}; omitting the method keeps the pending + * title and renders the raw result content. + */ +export type ToolResultView = GenericResultView | TerminalResultView + +/** + * The default completed card: an optional replacement title and reformatted + * content. Omit a field to keep the pending title / render the raw result content. + */ +export interface GenericResultView { + card: 'generic' + /** Replacement title for the completed call. Omit to keep the pending-state title. */ title?: string /** * UI-facing result content (harness {@link ContentBlock}s), reformatted from * the model-facing result. Omit to let the UI render the raw result content. - * Stays in harness vocabulary; the UI maps these to its own content blocks. */ content?: ContentBlock[] +} + +/** + * The completed state of a {@link TerminalCallView}: the captured output and exit + * status. A capable UI renders `output` in the terminal card and shows an + * exit-status pill; an incapable UI gets a fenced ```console fallback the BRIDGE + * derives from `output` (the tool does not double-encode it). + */ +export interface TerminalResultView { + card: 'terminal' + /** Replacement title for the completed call. Omit to keep the pending-state title. */ + title?: string + /** Captured command output (stdout+stderr as the tool chooses to combine them). */ + output?: string /** - * 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). + * Process exit code, when the run ended by exiting (not a signal). Lets a + * capable UI show an exit-status pill. Omit when killed by a signal or unknown. */ - terminal?: ToolTerminal + exitCode?: number + /** Signal name that killed the process (e.g. `SIGTERM`). Mutually exclusive with `exitCode`. */ + signal?: string } /** A registered tool: its schema plus the execution function. */ export interface ToolDefinition extends ToolSchema { execute(args: unknown, exec: ToolExecution): Promise /** - * Optional: how to present the PENDING state of one call in a UI, derived - * from the call's `args` (parsed arguments, `unknown` — the tool validates/ - * narrows its own input). Returning `undefined` (or omitting the method) tells - * a UI to fall back to a generic presentation (title = tool name, raw args as - * input). Pure and side-effect-free: a UI may call it during live streaming - * AND a session-log replay, so it must depend only on `args`. + * Optional: how to present the PENDING state of one call in a UI, derived from + * the call's `args` (parsed arguments, `unknown` — the tool validates/narrows + * its own input). Returns a {@link ToolCallView} (a `card`-tagged render intent), + * or `undefined` (or omit the method) to fall back to a generic presentation + * (title = tool name, raw args as input). Pure and side-effect-free: a UI may + * call it during live streaming AND a session-log replay, so it must depend + * only on `args`. */ - presentCall?(args: unknown): ToolCallPresentation | undefined + presentCall?(args: unknown): ToolCallView | undefined /** * Optional: how to present the COMPLETED state, given the same `args` and the - * `result` (`execute`'s content + whether it errored). Returning `undefined` - * (or omitting the method) tells a UI to keep the pending title and render the - * raw result content. Pure and side-effect-free for the same replay reason. + * `result` (`execute`'s content + whether it errored). Returns a + * {@link ToolResultView}, or `undefined` (or omit the method) to keep the + * pending title and render the raw result content. Pure and side-effect-free + * for the same replay reason. */ - presentResult?(args: unknown, result: ToolResult): ToolResultPresentation | undefined + presentResult?(args: unknown, result: ToolResult): ToolResultView | undefined } /** The completed outcome handed to {@link ToolDefinition.presentResult}. */ diff --git a/packages/core/tools/src/schema.ts b/packages/core/tools/src/schema.ts index b717eabf9a..0b3fc749f4 100644 --- a/packages/core/tools/src/schema.ts +++ b/packages/core/tools/src/schema.ts @@ -21,7 +21,7 @@ import type { ContentBlock } from '@deepseek-ai/dsh-llm' import { assertNever, HarnessError } from '@deepseek-ai/dsh-llm' -import type { ToolCallPresentation, ToolDefinition, ToolExecution, ToolResult, ToolResultPresentation } from './index.ts' +import type { ToolCallView, ToolDefinition, ToolExecution, ToolResult, ToolResultView } from './index.ts' // --------------------------------------------------------------------------- // SchemaSpec — the author-facing per-property type @@ -300,16 +300,16 @@ export interface DefineToolOptions { * argument shape — zero casts. Pure and side-effect-free: a UI may call it * during live streaming AND a session-log replay, so depend only on `args`. * The tool owns its presentation so a UI never special-cases tool names. See - * {@link ToolCallPresentation}. + * {@link ToolCallView}. */ - presentCall?(args: InferArgs): ToolCallPresentation | undefined + presentCall?(args: InferArgs): ToolCallView | undefined /** * Optional: how to present the COMPLETED state, given the typed `args` and the * `result`. Use it to reformat result content for a UI distinctly from the * model-facing text (e.g. a fenced ```console block). Pure and side-effect- - * free for the same replay reason. See {@link ToolResultPresentation}. + * free for the same replay reason. See {@link ToolResultView}. */ - presentResult?(args: InferArgs, result: ToolResult): ToolResultPresentation | undefined + presentResult?(args: InferArgs, result: ToolResult): ToolResultView | undefined /** Whether the tool requires structured output (default false). */ strict?: boolean } @@ -369,13 +369,13 @@ export function defineTool(options: DefineToolOptions): // fall back to `undefined` (a generic UI presentation) on any mismatch, rather // than the hard `ToolArgsError` the execute path raises. if (userPresentCall) { - tool.presentCall = (args: unknown): ToolCallPresentation | undefined => { + tool.presentCall = (args: unknown): ToolCallView | undefined => { if (validateArgs(options.parameters, args).length > 0) return undefined return userPresentCall(args as InferArgs) } } if (userPresentResult) { - tool.presentResult = (args: unknown, result: ToolResult): ToolResultPresentation | undefined => { + tool.presentResult = (args: unknown, result: ToolResult): ToolResultView | undefined => { if (validateArgs(options.parameters, args).length > 0) return undefined return userPresentResult(args as InferArgs, result) } diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index c88963ecc1..55aa5866ce 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -52,8 +52,8 @@ describe('ToolRegistry', () => { description: 'has presenters', parameters: { x: { type: 'string', required: true } }, async execute() { return [] }, - presentCall: args => ({ title: args.x }), - presentResult: (args, result) => ({ title: args.x, content: result.content }), + presentCall: args => ({ card: 'generic', title: args.x }), + presentResult: (args, result) => ({ card: 'generic', title: args.x, content: result.content }), })) const schema = ctx.tools.schemas()[0] as unknown as Record expect(Object.keys(schema).sort()).toEqual(['description', 'name', 'parameters']) @@ -906,15 +906,15 @@ describe('defineTool presentation (presentCall / presentResult)', () => { presentCall(args) { // args is typed { path: string; n?: number } — zero casts. expectTypeOf(args).toEqualTypeOf<{ path: string; n?: number }>() - return { title: `Open ${args.path}`, kind: 'read', rawInput: args.path } + return { card: 'generic', title: `Open ${args.path}`, kind: 'read', rawInput: args.path } }, presentResult(args, result) { - return { title: `Opened ${args.path}`, content: result.content } + return { card: 'generic', title: `Opened ${args.path}`, content: result.content } }, }) - expect(tool.presentCall!({ path: '/a', n: 2 })).toEqual({ title: 'Open /a', kind: 'read', rawInput: '/a' }) + expect(tool.presentCall!({ path: '/a', n: 2 })).toEqual({ card: 'generic', title: 'Open /a', kind: 'read', rawInput: '/a' }) expect(tool.presentResult!({ path: '/a' }, { content: [{ type: 'text', text: 'x' }], isError: false })) - .toEqual({ title: 'Opened /a', content: [{ type: 'text', text: 'x' }] }) + .toEqual({ card: 'generic', title: 'Opened /a', content: [{ type: 'text', text: 'x' }] }) }) it('a tool without presentCall/presentResult leaves them undefined (UI falls back generically)', () => { @@ -934,8 +934,8 @@ describe('defineTool presentation (presentCall / presentResult)', () => { description: 'demo', parameters: { path: { type: 'string', required: true } }, async execute() { return [] }, - presentCall: args => ({ title: args.path }), - presentResult: (args, result) => ({ title: args.path, content: result.content }), + presentCall: args => ({ card: 'generic', title: args.path }), + presentResult: (args, result) => ({ card: 'generic', title: args.path, content: result.content }), }) // Unlike execute (which throws ToolArgsError on a mismatch), the display // methods soft-validate and fall back to undefined so a UI never crashes diff --git a/packages/fs/tool-fs/src/edit.ts b/packages/fs/tool-fs/src/edit.ts index 1fb76ffd3c..7450bd895a 100644 --- a/packages/fs/tool-fs/src/edit.ts +++ b/packages/fs/tool-fs/src/edit.ts @@ -14,6 +14,7 @@ import type { Context } from 'cordis' import { defineTool } from '@deepseek-ai/dsh-tools' +import type { DiffCallView } from '@deepseek-ai/dsh-tools' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { FsEditOutcome } from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-fs' @@ -83,16 +84,15 @@ export function applyEditTool(ctx: Context): void { ctx.emit('fs/observed', target, outcome.version, exec) return [{ type: 'text', text: formatEditOutput(target.displayPath, outcome) }] }, - // Pure display: `edit` kind, a location for editor follow-along, and a short - // old→new summary as rawInput (truncated so a large replacement stays a - // readable card). The replacement COUNT is not available here — presentResult - // only sees `{ content, isError }`, not the outcome — so the title is static. - presentCall(args) { - const clip = (s: string): string => (s.length > 40 ? `${s.slice(0, 40)}…` : s) + // Pure display: a diff card of the literal replacement (old_string → + // new_string), derived from the call args. `oldText: old_string || null` + // matches claude-agent-acp's Edit arm; new_string is a required arg here, so + // it maps straight to newText. A follow-along location points at the file. + presentCall(args): DiffCallView { return { + card: 'diff', title: `Edit ${args.file_path}`, - kind: 'edit', - rawInput: `${JSON.stringify(clip(args.old_string))} → ${JSON.stringify(clip(args.new_string))}`, + diffs: [{ path: args.file_path, oldText: args.old_string || null, newText: args.new_string }], locations: [{ path: args.file_path }], } }, diff --git a/packages/fs/tool-fs/src/read.ts b/packages/fs/tool-fs/src/read.ts index 17fa7aa7ab..c984b53c9f 100644 --- a/packages/fs/tool-fs/src/read.ts +++ b/packages/fs/tool-fs/src/read.ts @@ -14,6 +14,7 @@ import type { Context } from 'cordis' import { defineTool } from '@deepseek-ai/dsh-tools' +import type { GenericCallView } from '@deepseek-ai/dsh-tools' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import { FsError } from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-fs' @@ -101,19 +102,21 @@ export function applyReadTool(ctx: Context): void { ctx.emit('fs/observed', target, info.version, exec) return [{ type: 'text', text: formatReadOutput(target.displayPath, outcome) }] }, - // Pure display: a UI card titled by the file, `read` kind (icon), and a - // location so an editor can follow along to the file (and the read's offset - // line). `rawInput` surfaces offset/limit when the model narrowed the read. - presentCall(args) { - const detail = [ - ...args.offset !== undefined ? [`offset ${args.offset}`] : [], - ...args.limit !== undefined ? [`limit ${args.limit}`] : [], - ].join(', ') + // Pure display: a generic card titled by the file with the read window + // appended (`Read foo.txt (5 - 8)`), `read` kind (icon), and a follow-along + // location whose line is the read's offset (defaulting to 1). The window is + // derived from the RAW args (offset/limit as the model passed them), NOT the + // tool's defaulted 1/READ_LIMIT, so an unbounded read shows a bare title. + presentCall(args): GenericCallView { + const { offset, limit } = args + const window = limit !== undefined && limit > 0 + ? ` (${offset ?? 1} - ${(offset ?? 1) + limit - 1})` + : offset !== undefined ? ` (from line ${offset})` : '' return { - title: `Read ${args.file_path}`, + card: 'generic', + title: `Read ${args.file_path}${window}`, kind: 'read', - locations: [{ path: args.file_path, ...args.offset !== undefined ? { line: args.offset } : {} }], - ...detail.length > 0 ? { rawInput: detail } : {}, + locations: [{ path: args.file_path, line: offset ?? 1 }], } }, })) diff --git a/packages/fs/tool-fs/src/write.ts b/packages/fs/tool-fs/src/write.ts index 97098bd78d..69844c55f6 100644 --- a/packages/fs/tool-fs/src/write.ts +++ b/packages/fs/tool-fs/src/write.ts @@ -13,6 +13,7 @@ import type { Context } from 'cordis' import { defineTool } from '@deepseek-ai/dsh-tools' +import type { DiffCallView } from '@deepseek-ai/dsh-tools' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { FsWriteOutcome } from '@deepseek-ai/dsh-fs' import type {} from '@deepseek-ai/dsh-fs' @@ -62,12 +63,17 @@ export function applyWriteTool(ctx: Context): void { ctx.emit('fs/observed', target, outcome.version, exec) return [{ type: 'text', text: formatWriteOutput(target.displayPath, outcome) }] }, - // Pure display: `edit` kind (an editor treats create/replace as an edit) and - // a location so the UI can follow along to the written file. The create-vs- - // overwrite fact lives in the model-facing result text; `presentResult` only - // sees `{ content, isError }` (not the outcome), so the title stays static. - presentCall(args) { - return { title: `Write ${args.file_path}`, kind: 'edit', locations: [{ path: args.file_path }] } + // Pure display: a diff card (an editor renders write as a new-file / full- + // replace diff). `oldText: null` — a call-time presenter has no access to the + // file's prior content, so even an overwrite renders new-file style, matching + // claude-agent-acp. A follow-along location points at the written file. + presentCall(args): DiffCallView { + return { + card: 'diff', + title: `Write ${args.file_path}`, + diffs: [{ path: args.file_path, oldText: null, newText: args.content }], + locations: [{ path: args.file_path }], + } }, })) } diff --git a/packages/fs/tool-fs/tests/tools.spec.ts b/packages/fs/tool-fs/tests/tools.spec.ts index 12f373753e..7bdedf894a 100644 --- a/packages/fs/tool-fs/tests/tools.spec.ts +++ b/packages/fs/tool-fs/tests/tools.spec.ts @@ -352,34 +352,46 @@ describe('tool-owned presentation (pure presentCall)', () => { return ctx.tools.get(name)?.presentCall?.(args) } - it('read: titles by file, read kind, location with the offset line', async () => { + it('read: generic card titled by file with the read window, read kind, location with the offset line', async () => { expect(await presentCall('read', { file_path: 'src/a.ts', offset: 12, limit: 40 })).toEqual({ - title: 'Read src/a.ts', kind: 'read', rawInput: 'offset 12, limit 40', + card: 'generic', title: 'Read src/a.ts (12 - 51)', kind: 'read', locations: [{ path: 'src/a.ts', line: 12 }], }) }) - it('read: omits rawInput and the location line when offset/limit are unset', async () => { + it('read: bare title and line-1 location when offset/limit are unset', async () => { expect(await presentCall('read', { file_path: 'a.txt' })).toEqual({ - title: 'Read a.txt', kind: 'read', locations: [{ path: 'a.txt' }], + card: 'generic', title: 'Read a.txt', kind: 'read', locations: [{ path: 'a.txt', line: 1 }], }) }) - it('write: titles by file, edit kind, location', async () => { - expect(await presentCall('write', { file_path: 'out.txt', content: 'x' })).toEqual({ - title: 'Write out.txt', kind: 'edit', locations: [{ path: 'out.txt' }], + it('read: "from line N" window when only offset is set', async () => { + expect(await presentCall('read', { file_path: 'a.txt', offset: 5 })).toEqual({ + card: 'generic', title: 'Read a.txt (from line 5)', kind: 'read', locations: [{ path: 'a.txt', line: 5 }], }) }) - it('edit: titles by file, edit kind, an old→new rawInput summary, location', async () => { - expect(await presentCall('edit', { file_path: 'a.txt', old_string: 'foo', new_string: 'bar' })).toEqual({ - title: 'Edit a.txt', kind: 'edit', rawInput: '"foo" → "bar"', locations: [{ path: 'a.txt' }], + it('write: diff card (new-file style, oldText null), location', async () => { + expect(await presentCall('write', { file_path: 'out.txt', content: 'hello' })).toEqual({ + card: 'diff', title: 'Write out.txt', + diffs: [{ path: 'out.txt', oldText: null, newText: 'hello' }], + locations: [{ path: 'out.txt' }], }) }) - it('edit: clips a long old/new string in the rawInput summary', async () => { - const long = 'a'.repeat(60) - const p = await presentCall('edit', { file_path: 'a.txt', old_string: long, new_string: 'b' }) - expect((p as { rawInput: string }).rawInput).toBe(`${JSON.stringify(`${'a'.repeat(40)}…`)} → ${JSON.stringify('b')}`) + it('read: a limit with no offset windows from line 1', async () => { + expect(await presentCall('read', { file_path: 'a.txt', limit: 10 })).toEqual({ + card: 'generic', title: 'Read a.txt (1 - 10)', kind: 'read', locations: [{ path: 'a.txt', line: 1 }], + }) + }) + + it('edit: an empty old_string maps to oldText null (a whole-file replace diff)', async () => { + // presentCall runs on replay of raw logged args, which parseEditArgs does not + // gate — an empty old_string must still produce a valid diff (oldText null). + expect(await presentCall('edit', { file_path: 'a.txt', old_string: '', new_string: 'seed' })).toEqual({ + card: 'diff', title: 'Edit a.txt', + diffs: [{ path: 'a.txt', oldText: null, newText: 'seed' }], + locations: [{ path: 'a.txt' }], + }) }) }) diff --git a/packages/todo/tool-todo/src/index.ts b/packages/todo/tool-todo/src/index.ts index 912e08f269..01d862e4d5 100644 --- a/packages/todo/tool-todo/src/index.ts +++ b/packages/todo/tool-todo/src/index.ts @@ -118,6 +118,6 @@ export function apply(ctx: Context): void { text: `Updated todo list: ${count('pending')} pending, ${count('in_progress')} in progress, ${count('completed')} completed.`, }]) }, - presentCall: args => ({ title: 'Update todo list', kind: 'other', rawInput: args.todos }), + presentCall: args => ({ card: 'generic', title: 'Update todo list', kind: 'other', rawInput: args.todos }), })) } diff --git a/packages/todo/tool-todo/tests/tool-todo.spec.ts b/packages/todo/tool-todo/tests/tool-todo.spec.ts index 8b1c504840..86e8814633 100644 --- a/packages/todo/tool-todo/tests/tool-todo.spec.ts +++ b/packages/todo/tool-todo/tests/tool-todo.spec.ts @@ -135,7 +135,7 @@ describe('dsh-tool-todo', () => { const ctx = await setup() const def = ctx.tools.get('todo_write')! const todos = [{ content: 'a', status: 'pending' }] - expect(def.presentCall?.({ todos })).toEqual({ title: 'Update todo list', kind: 'other', rawInput: todos }) + expect(def.presentCall?.({ todos })).toEqual({ card: 'generic', title: 'Update todo list', kind: 'other', rawInput: todos }) }) it('unregisters the tool when its contributing fiber is disposed (HMR-safety)', async () => { diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index 8d0444de70..32e4326b99 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -28,7 +28,7 @@ It is a **client-driver / UI plugin**, the structured analogue of the readline ` | `session/load` | `ctx.agents.resume(...)` | replays the persisted event log to the client as `session/update` — the USER side (`user/message` → `user_message_chunk`), assistant text/reasoning (`assistant/chunk`), and tool calls/results (`tool/call` + `tool/result`). Re-loading an already-live id is rejected; the id's load slot is reserved (`loadingIds`) BEFORE the async resume so a pipelined load of the SAME id can't leak a second agent (distinct ids load concurrently). The resumed session keeps its PERSISTED header `cwd`, so its bash tools run in the original workspace; the requested `cwd` must be absolute and match the persisted `cwd`. After the async resume a `closed` re-check refuses to install a record if the bridge tore down mid-load | | `session/prompt` | `agent.send()` | supports ACP `text` and `resource_link` blocks; rejects image/audio/embedded resource and empty prompts; one in-flight prompt PER session (independent); settles on the OWNING turn's end (a turn that ends in `error` rejects the RPC) | | `session/cancel` | `agent.cancel()` | the queue-aware cancel: aborts a running step, clears queued + steering work, and drops a turn about to start, then settles the prompt `cancelled` — for ONLY that session (a cancel never touches another session's stream or prompt) | -| `session/update` | `session/event` | `agent_message_chunk` (text-delta), `agent_thought_chunk` (reasoning-delta), `user_message_chunk` (load replay), `tool_call`/`tool_call_update` (title/kind/rawInput/content/locations owned by the TOOL via `presentCall`/`presentResult` — see Tool-call presentation) | +| `session/update` | `session/event` | `agent_message_chunk` (text-delta), `agent_thought_chunk` (reasoning-delta), `user_message_chunk` (load replay), `tool_call`/`tool_call_update` (the render intent — a `card`-tagged `ToolCallView`/`ToolResultView` — owned by the TOOL via `presentCall`/`presentResult`, which the bridge switches on to build the wire shape — see Tool-call presentation) | ## Multi-session @@ -42,18 +42,24 @@ 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, the salient `rawInput` to show in a detail view, optional `content` blocks shown alongside, and optional `locations` — `{ path, line? }[]` files the call reads/modifies, forwarded as `tool_call.locations` so an editor can follow along) 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 `dsh-tool-fs` `read`/`write`/`edit` tools set a `Read/Write/Edit ` title, a `read`/`edit` kind, and a `locations` entry for the file. (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.) +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) and `presentResult(args, result)` (completed state) on its `dsh-tools` definition, each returning a **`card`-tagged render intent** — a discriminated union the bridge switches on. `presentCall` returns a `ToolCallView`, one of three cards: + +- `{ card: 'generic', title, kind?, rawInput?, content?, locations? }` — the default card: a human-readable `title`, a `kind` for the icon, the salient `rawInput` for a detail view, optional `content` blocks shown alongside, and optional `locations` (`FileLocation[]` = `{ path, line? }[]` files the call reads/modifies, forwarded as `tool_call.locations` so an editor can follow along). +- `{ card: 'terminal', title, description?, cwd? }` — a shell command → a terminal card (see Terminal card). +- `{ card: 'diff', title, diffs, locations? }` — a file create/modify → an inline diff card; `diffs` is `FileDiff[]` (`{ path, oldText, newText }`, `oldText: null` ⇒ new file). The bridge emits each diff as an ACP `{ type: 'diff', path, oldText, newText }` `tool_call.content` block, which Zed renders as an inline diff / new-file preview. + +`presentResult` returns a `ToolResultView`, one of two cards: `{ card: 'generic', title?, content? }` (an optional replacement `title` and reformatted `content`) or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit — see Terminal card). The bridge looks the definition up by name in `ctx.tools` and `switch (view.card)`es to build the ACP `tool_call`/`tool_call_update` wire shape per card; 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` returns a `terminal` card for a foreground `bash` (title = the exact `command` "ls -la src", `description` = the model description); the `dsh-tool-fs` `write`/`edit` tools return a `diff` card and `read` returns a `generic` card (`kind: 'read'`, the read window in its title — `Read foo.txt (5 - 8)` — and a `locations` entry for the file). For a file card the bridge **relativizes the title** against the session cwd (mirroring `claude-agent-acp`'s `toDisplayPath` — `Read src/foo.ts`, not the absolute path) while keeping `locations[]`/`diffs[].path` **raw** so the editor opens the real path. 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 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`: +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 `terminal` card variant of its render intent (`dsh-tools`: `{ card: 'terminal', title, description?, cwd? }` from `presentCall`, `{ card: 'terminal', title?, output?, exitCode?, signal? }` from `presentResult`); 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 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. +- `tool_call`: `content:[…, {type:'terminal', terminalId}]` + `_meta.terminal_info.{terminal_id, cwd}` — the terminal id is the harness `callId`; the cwd is the card's explicit absolute `cwd`, else a relative `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). The card's `description` renders as a content block BEFORE the terminal block, so the description sits above the card. +- `tool_call_update`: `_meta.terminal_output.{terminal_id, data}` (the terminal card's `output`) plus `_meta.terminal_exit.{terminal_id, exit_code | signal}` when the card reported a structured `exitCode`/`signal`. 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: 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/feature/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 a ` ```console ` text block the bridge DERIVES by fencing the terminal result's `output` (the tool no longer double-encodes the fences) — 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/feature/2026-06-18-acp-terminal-and-tool-rendering.md) and [the render-intent-union RFC](../../../docs/rfc/implemented/architecture/2026-07-02-tool-render-intent-union.md). ## Settle-exactly-once diff --git a/packages/ui/acp/acp-feature-support.md b/packages/ui/acp/acp-feature-support.md index c39b518aba..4d14c79f5e 100644 --- a/packages/ui/acp/acp-feature-support.md +++ b/packages/ui/acp/acp-feature-support.md @@ -99,7 +99,7 @@ Tool-call presentation is **owned by each tool** (`presentCall` / `presentResult | `ToolCallKind` mapping | S | ✅ | ✅ | ✅ | `execute`/`read`/`edit`/`other` inferred from the tool; richer mapping possible. | | `ToolCallStatus` | S | ✅ | ✅ | ✅ | `in_progress` → `completed`/`failed`. | | `content` blocks | S | ✅ | ✅ | ✅ | Text content; the description renders above the card. | -| `diff` content | S | ❌ | ✅ | ✅ | No structured diff rendering for edits (would need a diffing edit tool + presenter). | +| `diff` content | S | ✅ | ✅ | ✅ | The `write`/`edit` tools declare a `diff` render intent (`presentCall` → `{ card: 'diff' }`); the bridge emits `{ type: 'diff', path, oldText, newText }` content blocks (call-time, args-derived — applied-hunk diffs are a follow-up). | | `terminal` content | S | ✅ | ✅ | ✅ | Via the Zed `_meta` terminal convention (see below), not the spec `terminal/*` sub-protocol. | | `locations` (follow-along) | S | ✅ | ✅ | ✅ | The `read`/`write`/`edit` tools emit `{ path, line? }` file-location hints via `presentCall`. | | `rawInput` | S | ✅ | ⚠️ | ✅ | Parsed tool args surfaced as `rawInput`. | @@ -147,7 +147,7 @@ Ranked by how commonly the reference adapters ship them and how much UX they unl 5. **Slash commands** (`available_commands_update`). 6. **MCP passthrough** (`mcpServers` on `session/new` + `mcpCapabilities`). 7. **Richer prompt content** — image / embedded `resource` blocks (needs a multimodal model path). -8. **Diff tool rendering** — structured `diff` content for edit tools (the `locations` follow-along hint already ships on `read`/`write`/`edit`). +8. **Applied-hunk diff rendering** — the `write`/`edit` diff cards ship (call-time, args-derived: whole `old_string`→`new_string`). Result-time structured-patch hunks with surrounding context (what `claude-agent-acp` derives from a PostToolUse hook) need a new result/event shape carrying the patch — a follow-up. 9. **Usage reporting** (`usage_update`) — the harness already records token usage internally (on `assistant/message`). 10. **Editor filesystem delegation** (`fs/read_text_file` / `fs/write_text_file`) — lets the agent see unsaved buffers; lower priority since the harness has direct disk access. diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 255ca9c990..6d388e9f89 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -36,7 +36,7 @@ import type { Context } from 'cordis' import { Readable, Writable } from 'node:stream' import { randomUUID } from 'node:crypto' -import { isAbsolute, resolve as resolvePath } from 'node:path' +import { isAbsolute, relative as relativePath, resolve as resolvePath } from 'node:path' import Schema from 'schemastery' import { AgentSideConnection, @@ -62,12 +62,12 @@ import { type StopReason, } from '@agentclientprotocol/sdk' import type { ContentBlock } from '@deepseek-ai/dsh-llm' -import { CallId } from '@deepseek-ai/dsh-llm' +import { assertNever, CallId } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' import { AgentId } from '@deepseek-ai/dsh-agent' import { SessionId } from '@deepseek-ai/dsh-session' import type { SessionEvent, TodoItem, TurnEndReason } from '@deepseek-ai/dsh-session' -import type { ToolCallKind, ToolCallPresentation, ToolRegistry, ToolResultPresentation, ToolTerminal } from '@deepseek-ai/dsh-tools' +import type { ToolCallKind, ToolCallView, ToolRegistry, ToolResultView, TerminalResultView } 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' @@ -813,67 +813,13 @@ export function streamSessionEventUpdate( return } 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 - // 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: { - sessionUpdate: 'tool_call', - toolCallId: event.data.callId, - title: present.title, - kind: present.kind, - status: 'in_progress', - ...present.rawInput !== undefined ? { rawInput: present.rawInput } : {}, - ...present.locations !== undefined ? { locations: present.locations } : {}, - ...callContent.length > 0 ? { content: callContent } : {}, - ...asTerminal - ? { _meta: { terminal_info: { terminal_id: event.data.callId, cwd: terminalCwd(present.terminal, terminal.cwd) } } } - : {}, - }, - }) + const view = presenter.call(event.data.callId, event.data.name, event.data.arguments) + notify({ sessionId, update: toolCallUpdate(event.data.callId, view, terminal) }) 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, 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', - ...asTerminal ? {} : { content: toolResultContent(present.content) }, - ...present.title !== undefined ? { title: present.title } : {}, - ...terminalResultMeta, - }, - }) + const view = presenter.result(event.data.callId, event.data.content, event.data.isError) + notify({ sessionId, update: toolResultUpdate(event.data.callId, view, event.data.isError, terminal) }) return } case 'todo/write': { @@ -916,46 +862,20 @@ export interface TerminalRendering { /** 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` - * and `rawInput` are optional. - */ -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[] - /** Files this call reads/modifies (mapped to ACP `tool_call.locations`), for editor follow-along. */ - locations?: { path: string; line?: number }[] - /** 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`. */ -interface ResolvedResultPresentation { - /** UI content for the result (harness blocks; the tool may reformat, else the raw result). */ - 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 -} - /** * Resolves tool-owned presentation for a session's tool-call events. A tool - * declares `presentCall`/`presentResult` (see `dsh-tools`); this looks them up - * by name in the registry and applies the generic fallback when a tool defines - * neither. + * declares `presentCall`/`presentResult` (see `dsh-tools`) returning a + * `card`-tagged {@link ToolCallView}/{@link ToolResultView}; this looks them up + * by name in the registry and applies a generic fallback when a tool defines + * neither. The returned view is what {@link streamSessionEventUpdate} switches on. * * The `tool/result` session event carries only `{ callId, content, isError }` — * NOT the tool name or args — so to call a tool's `presentResult` (which needs - * both), the presenter remembers each `tool/call`'s `{ name, args }` keyed by - * callId and looks it up on the matching result. The map is bridge-LOCAL (not a - * change to the event schema or a core service): one presenter per live session - * (and a throwaway per `session/load` replay), and each entry is removed when - * its result arrives. In the normal loop a `tool/call` is always followed by a + * both), the presenter remembers each `tool/call`'s `{ name, args, card }` keyed + * by callId and looks it up on the matching result. The map is bridge-LOCAL (not + * a change to the event schema or a core service): one presenter per live session + * (and a throwaway per `session/load` replay), and each entry is removed when its + * result arrives. In the normal loop a `tool/call` is always followed by a * `tool/result` (the registry turns even a thrown tool into an isError result), * so the map holds only currently-in-flight calls. The one exception is a step * torn down mid-tool (an abort between `tool/call` and `tool/result`), which can @@ -965,7 +885,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. @@ -980,10 +900,10 @@ export class ToolPresenter { private readonly onError: (message: string) => void = () => {}, ) {} - /** Pending-state presentation for a `tool/call`; remembers `(name, args)` for the matching result. */ - call(callId: CallId, name: string, argsJson: string): ResolvedCallPresentation { + /** Pending-state render intent for a `tool/call`; remembers `(name, args, card)` for the matching result. */ + call(callId: CallId, name: string, argsJson: string): ToolCallView { const args = parseToolArguments(argsJson) - let present: ToolCallPresentation | undefined + let present: ToolCallView | undefined try { present = this.tools.get(name)?.presentCall?.(args) } catch (error: unknown) { @@ -991,35 +911,20 @@ export class ToolPresenter { this.onError(`acp: tool "${name}" presentCall threw, using generic presentation: ${String(error)}`) present = undefined } - 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). 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.locations !== undefined ? { locations: present.locations } : {}, - ...present.terminal !== undefined ? { terminal: present.terminal } : {}, - } + // No tool-owned presentation: fall back to the tool name as the title and the + // full parsed args as the raw input (the generic card). + const view: ToolCallView = present ?? { card: 'generic', title: name, kind: toolKindFor(name), rawInput: args } + this.pending.set(callId, { name, args, card: view.card }) + return view } - /** Completed-state presentation for a `tool/result`; consumes the remembered `(name, args)`. */ - result(callId: CallId, content: ContentBlock[], isError: boolean): ResolvedResultPresentation { + /** Completed-state render intent for a `tool/result`; consumes the remembered `(name, args, card)`. */ + result(callId: CallId, content: ContentBlock[], isError: boolean): ToolResultView { const call = this.pending.get(callId) this.pending.delete(callId) // No remembered call (unknown/late callId) → nothing to present from; raw content. - if (call === undefined) return { content } - let present: ToolResultPresentation | undefined + if (call === undefined) return { card: 'generic', content } + let present: ToolResultView | undefined try { present = this.tools.get(call.name)?.presentResult?.(call.args, { content, isError }) } catch (error: unknown) { @@ -1027,15 +932,16 @@ export class ToolPresenter { this.onError(`acp: tool "${call.name}" presentResult threw, using raw result: ${String(error)}`) present = undefined } - if (present === undefined) return { content } - return { - content: present.content ?? content, - ...present.title !== undefined ? { title: present.title } : {}, - // 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 } : {}, - } + if (present === undefined) return { card: 'generic', content } + // Orphan guard: only honor a `terminal` result when the PENDING call was a + // terminal. A result-only terminal with no matching call-side terminal would + // orphan `_meta.terminal_output` to a terminal Zed never made — drop it back + // to the raw content. + if (present.card === 'terminal' && call.card !== 'terminal') return { card: 'generic', content } + // A generic result that reformats no content keeps the RAW result content + // (the tool replaced only the title); fill it so the card is never blanked. + if (present.card === 'generic' && present.content === undefined) return { ...present, content } + return present } } @@ -1045,8 +951,8 @@ export class ToolPresenter { * results pass their raw content through unchanged. */ export const nullToolPresenter: Pick = { - call: (_callId, name, argsJson) => ({ title: name, kind: toolKindFor(name), rawInput: parseToolArguments(argsJson) }), - result: (_callId, content) => ({ content }), + call: (_callId, name, argsJson) => ({ card: 'generic', title: name, kind: toolKindFor(name), rawInput: parseToolArguments(argsJson) }), + result: (_callId, content) => ({ card: 'generic', content }), } /** Map a harness tool name to an ACP ToolKind (best-effort; default `other`). */ @@ -1079,20 +985,118 @@ function toolResultContent(blocks: ContentBlock[]): { type: 'content'; content: return out } +/** The `session/update` payload for a `tool_call` / `tool_call_update`. */ +type ToolCallSessionUpdate = SessionNotification['update'] + +/** An ACP tool-call content block (a text/image `content`, a `diff`, or a `terminal`). */ +type AcpToolCallContent = + | { type: 'content'; content: AcpContentBlock } + | { type: 'diff'; path: string; oldText: string | null; newText: string } + | { type: 'terminal'; terminalId: string } + /** - * 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"). + * Relativize a file card's TITLE path against the session workspace cwd, so a + * card reads `Read src/foo.ts` rather than `/abs/proj/src/foo.ts` — matching the + * reference ACP adapter's `toDisplayPath`. Only the TITLE is relativized; the + * card's `locations`/`diff` paths stay RAW (the editor opens the real path). The + * pure tool presenter can't see the session cwd, so this happens here where the + * bridge knows it. The rewrite is an exact substring replace of the known raw + * path (a card carries the same path in `locations[0]`/`diffs[0]`), never a + * heuristic. A path outside the workspace, or an absent/relative session cwd, is + * left unchanged. */ -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 +function displayTitle(title: string, rawPath: string | undefined, sessionCwd: string | undefined): string { + if (rawPath === undefined || sessionCwd === undefined || !isAbsolute(rawPath) || !isAbsolute(sessionCwd)) return title + const rel = relativePath(sessionCwd, rawPath) + // `relative` returns a `..`-prefixed path for a target outside the workspace; + // only relativize paths that stay inside it (and never to the empty string). + if (rel.length === 0 || rel.startsWith('..')) return title + return title.split(rawPath).join(rel) +} + +/** + * Resolve the terminal card's header cwd. A `TerminalCallView.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 view gives no cwd, the + * session workspace cwd is the default. Returns `undefined` only when neither the + * view nor the session supplies one (Zed then shows "current directory"). + */ +function terminalCwd(viewCwd: string | undefined, sessionCwd: string | undefined): string | undefined { + if (viewCwd === undefined) return sessionCwd + if (isAbsolute(viewCwd)) return viewCwd + return sessionCwd !== undefined ? resolvePath(sessionCwd, viewCwd) : viewCwd +} + +/** + * Build the `tool_call` (pending) `session/update` from a tool's render intent. + * Switches on `view.card`: a `generic` card maps title/kind/rawInput/content/ + * locations; a `diff` card emits `{ type: 'diff' }` content blocks (the editor's + * inline diff) plus follow-along locations; a `terminal` card renders as a + * terminal when the client is capable (a `terminal` content block + the + * `_meta.terminal_info` cwd header) and otherwise falls back to a generic execute + * card whose body is the description. File-card titles are relativized against the + * session cwd (see {@link displayTitle}). + */ +function toolCallUpdate(callId: CallId, view: ToolCallView, terminal: TerminalRendering): ToolCallSessionUpdate { + switch (view.card) { + case 'generic': + return { + sessionUpdate: 'tool_call', + toolCallId: callId, + // Relativize the title against the session cwd when the card carries a + // file location (a read/file card); a location-less card (bash, todo) + // has no path to relativize, so the title is used as-is. + title: displayTitle(view.title, view.locations?.[0]?.path, terminal.cwd), + kind: view.kind ?? 'other', + status: 'in_progress', + ...view.rawInput !== undefined ? { rawInput: view.rawInput } : {}, + ...view.locations !== undefined ? { locations: view.locations } : {}, + ...view.content !== undefined && view.content.length > 0 ? { content: toolResultContent(view.content) } : {}, + } + case 'diff': { + const rawPath = view.locations?.[0]?.path ?? view.diffs[0]?.path + const content: AcpToolCallContent[] = view.diffs.map(d => ({ type: 'diff', path: d.path, oldText: d.oldText, newText: d.newText })) + return { + sessionUpdate: 'tool_call', + toolCallId: callId, + title: displayTitle(view.title, rawPath, terminal.cwd), + kind: 'edit', + status: 'in_progress', + ...view.locations !== undefined ? { locations: view.locations } : {}, + ...content.length > 0 ? { content } : {}, + } + } + case 'terminal': { + // A terminal-rendered call gets a terminal CARD when the client supports it: + // the description renders ABOVE the card, then the terminal block, plus + // `_meta.terminal_info` (the cwd header). Without the capability it is an + // ordinary execute card whose body is the description and whose rawInput is + // the command; the output arrives as text on the result. + const asTerminal = terminal.enabled + const description: AcpToolCallContent[] = view.description !== undefined + ? [{ type: 'content', content: { type: 'text', text: view.description } }] + : [] + const content: AcpToolCallContent[] = [ + ...description, + ...asTerminal ? [{ type: 'terminal' as const, terminalId: callId }] : [], + ] + return { + sessionUpdate: 'tool_call', + toolCallId: callId, + title: view.title, + kind: 'execute', + status: 'in_progress', + rawInput: view.title, + ...content.length > 0 ? { content } : {}, + ...asTerminal + ? { _meta: { terminal_info: { terminal_id: callId, cwd: terminalCwd(view.cwd, terminal.cwd) } } } + : {}, + } + } + default: + return assertNever(view, 'ToolCallView.card') + } } /** The `terminal_exit` `_meta` entry for a completed terminal call. */ @@ -1102,12 +1106,60 @@ interface TerminalExitMeta { /** * 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`. + * from a 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 } } +function terminalExitMeta(callId: string, view: TerminalResultView): TerminalExitMeta { + if (view.signal !== undefined) return { terminal_exit: { terminal_id: callId, signal: view.signal } } + if (view.exitCode !== undefined) return { terminal_exit: { terminal_id: callId, exit_code: view.exitCode } } return {} } + +/** + * Build the `tool_call_update` (completed) `session/update` from a result render + * intent. A `generic` result sends its reformatted content (or the raw result); + * a `terminal` result rides its output/exit on `_meta` when the client is capable + * (the terminal card consumes them and `content` is OMITTED — a + * `tool_call_update.content` REPLACES the call's content collection in Zed, so + * re-sending would clobber the terminal block the call installed) and otherwise + * derives the fenced ```console fallback from `output`. + */ +function toolResultUpdate(callId: CallId, view: ToolResultView, isError: boolean, terminal: TerminalRendering): ToolCallSessionUpdate { + const status = isError ? 'failed' as const : 'completed' as const + if (view.card === 'terminal') { + const output = view.output ?? '' + if (terminal.enabled) { + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + ...view.title !== undefined ? { title: view.title } : {}, + _meta: { + terminal_output: { terminal_id: callId, data: output }, + ...terminalExitMeta(callId, view), + }, + } + } + // No terminal capability: the bridge derives the fenced ```console fallback. + const fenced = `\`\`\`console\n${output.replace(/\n+$/, '')}\n\`\`\`` + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + content: [{ type: 'content', content: { type: 'text', text: fenced } }], + ...view.title !== undefined ? { title: view.title } : {}, + } + } + // The presenter fills a generic result's content from the raw result, so + // `content` is always defined here; the guard keeps this total for a + // directly-constructed view. + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + /* v8 ignore next -- content always defined via the presenter (see above) */ + ...view.content !== undefined ? { content: toolResultContent(view.content) } : {}, + ...view.title !== undefined ? { title: view.title } : {}, + } +} diff --git a/packages/ui/acp/tests/stream-update.spec.ts b/packages/ui/acp/tests/stream-update.spec.ts index 9e1578c8c1..85e21d19a2 100644 --- a/packages/ui/acp/tests/stream-update.spec.ts +++ b/packages/ui/acp/tests/stream-update.spec.ts @@ -163,7 +163,7 @@ describe('todosToPlan', () => { }) describe('ToolPresenter (tool-owned presentation via the tool registry)', () => { - /** A tool whose presentCall/presentResult mirror what tool-bash declares. */ + /** A tool whose presentCall/presentResult return generic-card views. */ const bashLike: ToolDefinition = { name: 'bash', description: 'run a command', @@ -171,9 +171,10 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => execute: async () => [], presentCall: (args: unknown) => { const a = args as { command: string; description: string } - return { title: a.description, kind: 'execute', rawInput: a.command } + return { card: 'generic', title: a.description, kind: 'execute', rawInput: a.command } }, presentResult: (_args: unknown, result: { content: { type: string }[] }) => ({ + card: 'generic', content: [{ type: 'text', text: `wrapped:${result.content.length}` }], }), } @@ -247,8 +248,8 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => description: 'm', parameters: {}, execute: async () => [], - presentCall: () => ({ title: 'Doing a thing' }), - presentResult: () => ({ title: 'Did the thing' }), + presentCall: () => ({ card: 'generic', title: 'Doing a thing' }), + presentResult: () => ({ card: 'generic', title: 'Did the thing' }), } const presenter = new ToolPresenter(registryOf(minimal)) const updates = updatesWith( @@ -336,11 +337,30 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => expect(updates[1]).toMatchObject({ sessionUpdate: 'tool_call_update', content: [{ type: 'content', content: { type: 'text', text: 'raw' } }] }) }) - it('forwards a tool-owned `locations` onto the wire tool_call (REAL fs read/edit tools)', async () => { + it('an unknown render-intent card throws via the exhaustiveness guard (closed union)', () => { + // The bridge switches on `view.card` and ends with assertNever: a rogue card + // (only reachable by a cast — the union is closed) must throw, so adding a + // real variant later fails to compile at the switch instead of silently + // dropping the card. + const rogue: ToolDefinition = { + name: 'rogue', + description: 'r', + parameters: {}, + execute: async () => [], + // A card value outside the union — forced with a cast (no valid input reaches this). + presentCall: () => ({ card: 'chart', title: 'nope' }) as unknown as ReturnType>, + } + const presenter = new ToolPresenter(registryOf(rogue)) + expect(() => updatesWith(presenter, evt('tool/call', { + turn: 1, step: 1, callId: CallId('c1'), name: 'rogue', arguments: '{}', + }))).toThrow('unreachable variant') + }) + + it('forwards fs-tool render intents onto the wire (REAL read → generic locations, edit → diff content)', async () => { // Use the SHIPPING fs tools (not a stand-in), booted through their real // plugins, so the wire tool_call carries the actual presentCall output — - // including `locations` for editor follow-along. (AGENTS.md "prefer the real - // implementation over a mock".) + // read's follow-along `locations` and edit's `diff` content block. (AGENTS.md + // "prefer the real implementation over a mock".) const ctx = new Context() await ctx.plugin(SystemPrompt) await ctx.plugin(ToolRegistry) @@ -352,44 +372,50 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => turn: 1, step: 1, callId: CallId('r1'), name: 'read', arguments: JSON.stringify({ file_path: 'src/a.ts', offset: 12 }), })) + // A generic card: the read window is in the title, the offset drives the + // follow-along location line. No rawInput (the window lives in the title). expect(readCall).toMatchObject({ - sessionUpdate: 'tool_call', toolCallId: 'r1', title: 'Read src/a.ts', kind: 'read', - rawInput: 'offset 12', locations: [{ path: 'src/a.ts', line: 12 }], + sessionUpdate: 'tool_call', toolCallId: 'r1', title: 'Read src/a.ts (from line 12)', kind: 'read', + locations: [{ path: 'src/a.ts', line: 12 }], }) + expect((readCall as { rawInput?: unknown }).rawInput).toBeUndefined() const [editCall] = updatesWith(presenter, evt('tool/call', { turn: 1, step: 1, callId: CallId('e1'), name: 'edit', arguments: JSON.stringify({ file_path: 'src/b.ts', old_string: 'x', new_string: 'y' }), })) + // A diff card: `edit` kind, a `{ type: 'diff' }` content block carrying the + // literal old→new replacement, plus the follow-along location. expect(editCall).toMatchObject({ sessionUpdate: 'tool_call', toolCallId: 'e1', title: 'Edit src/b.ts', kind: 'edit', locations: [{ path: 'src/b.ts' }], + content: [{ type: 'diff', path: 'src/b.ts', oldText: 'x', newText: 'y' }], }) await ctx.fiber.dispose() }) }) 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 => ({ + // A tool that renders as a terminal — a stand-in for tool-bash's shape, letting + // us drive the bridge's terminal mapping without the real executor. `callCard` + // selects a terminal call view (optionally with a cwd) or a generic one (for the + // orphan-guard test); `resultTerminal` is the terminal result view's output/exit. + type CallCard = { card: 'terminal'; cwd?: string } | { card: 'generic' } + type ResultTerm = { title?: string; output?: string; exitCode?: number; signal?: string } + const termTool = (callCard: CallCard, 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 } : {}, - }), + presentCall: (args: unknown) => { + const command = (args as { command: string }).command + const description = (args as { description: string }).description + if (callCard.card === 'terminal') { + return { card: 'terminal', title: command, description, ...callCard.cwd !== undefined ? { cwd: callCard.cwd } : {} } + } + return { card: 'generic', title: command, kind: 'execute', rawInput: command, content: [{ type: 'text', text: description }] } + }, + presentResult: () => ({ card: 'terminal', ...resultTerminal }), }) const callEvent = evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: JSON.stringify({ command: 'echo hi', description: 'Greet' }) }) @@ -403,7 +429,7 @@ describe('terminal-card mapping (capability-gated)', () => { } 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) + const [call, update] = termUpdates(termTool({ card: 'terminal' }, { output: 'hi\n', exitCode: 0 }), true, '/work/proj', callEvent, resultEvent) expect(call).toMatchObject({ sessionUpdate: 'tool_call', content: [ @@ -422,33 +448,33 @@ describe('terminal-card mapping (capability-gated)', () => { }) 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) + const [absCall] = termUpdates(termTool({ card: 'terminal', 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) + const [relCall] = termUpdates(termTool({ card: 'terminal', 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) + const [noSessionCwd] = termUpdates(termTool({ card: 'terminal', 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) + const [, update] = termUpdates(termTool({ card: 'terminal' }, { 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 [, update] = termUpdates(termTool({ card: 'terminal' }, { 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) + it('capability OFF: no terminal block or _meta; the description content and the bridge-derived fenced result render', () => { + const [call, update] = termUpdates(termTool({ card: 'terminal' }, { output: 'hi\n' }), false, '/work/proj', callEvent, resultEvent) expect(call).toEqual({ sessionUpdate: 'tool_call', toolCallId: 'c1', @@ -458,24 +484,187 @@ describe('terminal-card mapping (capability-gated)', () => { rawInput: 'echo hi', content: [{ type: 'content', content: { type: 'text', text: 'Greet' } }], }) + // The bridge derives the fenced ```console fallback from the terminal output. expect(update).toEqual({ sessionUpdate: 'tool_call_update', toolCallId: 'c1', status: 'completed', - content: [{ type: 'content', content: { type: 'text', text: 'fallback' } }], + content: [{ type: 'content', content: { type: 'text', text: '```console\nhi\n```' } }], }) }) - 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). + it('orphan guard: a result-side terminal with a GENERIC call is dropped (no orphan terminal_output)', () => { + // presentCall is a generic card, but presentResult returns a terminal view — + // the bridge must not emit _meta.terminal_output for a terminal Zed never made. + const [call, update] = termUpdates(termTool({ card: 'generic' }, { output: 'hi\n', exitCode: 0 }), true, '/w', callEvent, resultEvent) + // The call was generic → 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. + // The result falls back to the RAW result content (the tool/result event's text); NO terminal _meta. expect((update as { _meta?: unknown })._meta).toBeUndefined() - expect((update as { content: unknown }).content).toEqual([{ type: 'content', content: { type: 'text', text: 'fallback' } }]) + expect((update as { content: unknown }).content).toEqual([{ type: 'content', content: { type: 'text', text: 'hi\n' } }]) + }) + + it('capability ON: a terminal result title replaces the completed-card title; missing output emits empty data', () => { + // A terminal result MAY carry a replacement title and MAY omit output (a run + // that produced nothing) — the _meta carries empty data, not a dropped key. + const [, update] = termUpdates(termTool({ card: 'terminal' }, { title: 'Ran echo', exitCode: 0 }), true, '/w', callEvent, resultEvent) + expect(update).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + title: 'Ran echo', + _meta: { terminal_output: { terminal_id: 'c1', data: '' }, terminal_exit: { terminal_id: 'c1', exit_code: 0 } }, + }) + }) + + it('capability OFF: a terminal result title rides on the fenced fallback update', () => { + const [, update] = termUpdates(termTool({ card: 'terminal' }, { title: 'Ran echo', output: 'hi\n' }), false, '/w', callEvent, resultEvent) + expect(update).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: '```console\nhi\n```' } }], + title: 'Ran echo', + }) + }) + + it('a terminal call with NO description and NO capability is a bare execute card (no content key)', () => { + // A terminal view whose presentCall omits `description`, with the capability + // OFF: no description block and no terminal block → the card carries no content. + const noDesc: ToolDefinition = { + name: 'bash', + description: 'run a command', + parameters: {}, + execute: async () => [], + presentCall: (args: unknown) => ({ card: 'terminal', title: (args as { command: string }).command }), + } + const [call] = termUpdates(noDesc, false, undefined, callEvent) + expect(call).toEqual({ + sessionUpdate: 'tool_call', + toolCallId: 'c1', + title: 'echo hi', + kind: 'execute', + status: 'in_progress', + rawInput: 'echo hi', + }) + }) +}) + +describe('diff-card mapping', () => { + // A stand-in diff tool, letting us drive the bridge's diff arm across shapes + // the shipping fs tools don't emit (no locations, empty diffs). + const diffTool = (view: unknown): ToolDefinition => ({ + name: 'writer', + description: 'writes a file', + parameters: {}, + execute: async () => [], + presentCall: () => view as ReturnType>, + }) + function callUpdate(tool: ToolDefinition, cwd: string | undefined): SessionNotification['update'] { + const presenter = new ToolPresenter(registryOf(tool)) + const out: SessionNotification['update'][] = [] + streamSessionEventUpdate( + SessionId('s1'), + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'writer', arguments: '{}' }), + n => out.push(n.update), + presenter, + { enabled: false, cwd }, + ) + return out[0]! + } + + it('a diff with NO locations relativizes the title off the first diff path; omits the locations key', () => { + const update = callUpdate(diffTool({ card: 'diff', title: 'Write /work/proj/a.txt', diffs: [{ path: '/work/proj/a.txt', oldText: null, newText: 'x' }] }), '/work/proj') + expect(update).toEqual({ + sessionUpdate: 'tool_call', + toolCallId: 'c1', + title: 'Write a.txt', + kind: 'edit', + status: 'in_progress', + content: [{ type: 'diff', path: '/work/proj/a.txt', oldText: null, newText: 'x' }], + }) + }) + + it('a diff with an EMPTY diffs array omits the content key (no diff blocks to send)', () => { + const update = callUpdate(diffTool({ card: 'diff', title: 'Write nothing', diffs: [] }), undefined) + expect(update).toEqual({ + sessionUpdate: 'tool_call', + toolCallId: 'c1', + title: 'Write nothing', + kind: 'edit', + status: 'in_progress', + }) + }) +}) + +describe('relative-path display titles (bridge relativizes the title against the session cwd)', () => { + // The bridge relativizes a file card's TITLE against the session workspace cwd + // (mirroring the reference adapter's toDisplayPath), while leaving locations/ + // diff paths RAW. Drive it with the REAL fs tools so the title/locations come + // from the shipping presentCall, and pass an ABSOLUTE file path (which a real + // editor forwards). The presenter is pure/args-only; the cwd is known only here. + async function fsCtx(): Promise { + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(FsLocal) + await ctx.plugin(ToolFs) + return ctx + } + function callUpdate(ctx: Context, sessionCwd: string | undefined, name: string, args: unknown): SessionNotification['update'] { + const presenter = new ToolPresenter(ctx.tools) + const out: SessionNotification['update'][] = [] + streamSessionEventUpdate( + SessionId('s1'), + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name, arguments: JSON.stringify(args) }), + n => out.push(n.update), + presenter, + { enabled: false, cwd: sessionCwd }, + ) + return out[0]! + } + + it('read: an absolute path inside the workspace relativizes the TITLE; the location path stays absolute', async () => { + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'read', { file_path: '/work/proj/src/a.ts', offset: 5 }) + expect(update).toMatchObject({ + title: 'Read src/a.ts (from line 5)', + locations: [{ path: '/work/proj/src/a.ts', line: 5 }], + }) + await ctx.fiber.dispose() + }) + + it('edit: the diff TITLE relativizes; the diff/location paths stay absolute (the editor opens the real path)', async () => { + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'edit', { file_path: '/work/proj/src/b.ts', old_string: 'x', new_string: 'y' }) + expect(update).toMatchObject({ + title: 'Edit src/b.ts', + locations: [{ path: '/work/proj/src/b.ts' }], + content: [{ type: 'diff', path: '/work/proj/src/b.ts', oldText: 'x', newText: 'y' }], + }) + await ctx.fiber.dispose() + }) + + it('a path OUTSIDE the workspace is left as-is (no `..` title)', async () => { + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'read', { file_path: '/etc/passwd' }) + expect((update as { title: string }).title).toBe('Read /etc/passwd') + await ctx.fiber.dispose() + }) + + it('no session cwd → the absolute title is left unchanged', async () => { + const ctx = await fsCtx() + const update = callUpdate(ctx, undefined, 'read', { file_path: '/work/proj/src/a.ts' }) + expect((update as { title: string }).title).toBe('Read /work/proj/src/a.ts') + await ctx.fiber.dispose() + }) + + it('a relative path is passed through unchanged (already display-friendly)', async () => { + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'read', { file_path: 'src/a.ts' }) + expect((update as { title: string }).title).toBe('Read src/a.ts') + await ctx.fiber.dispose() }) }) From af79ceea1c03e4ded615fb23cd73894cbf2f7a64 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:27:20 +0800 Subject: [PATCH 2/5] fix(acp): exhaustive result-card switch + tighten display-path guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the render-intent-union review: - toolResultUpdate branched on `if (card === 'terminal')` with a generic fallthrough; ToolResultView is a closed union, so make it an exhaustive `switch (view.card)` ending in assertNever (matching the call-side renderer and the § Conventions closed-union rule). Adding a result card later now fails to compile at the switch. Regression test: a rogue result card throws. - displayTitle's `rel.startsWith('..')` guard mis-rejected an in-workspace target whose relative form merely begins with the chars `..` (e.g. `..cache/x`, a real sibling name), leaving its title absolute. Test for a `..` SEGMENT (`..` alone or `..…`) so such paths relativize, matching claude-agent-acp's `cwd + sep` prefix check. Regression test added. --- packages/ui/acp/src/index.ts | 70 ++++++++++++--------- packages/ui/acp/tests/stream-update.spec.ts | 31 +++++++++ 2 files changed, 70 insertions(+), 31 deletions(-) diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 6d388e9f89..c01c004d91 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -36,7 +36,7 @@ import type { Context } from 'cordis' import { Readable, Writable } from 'node:stream' import { randomUUID } from 'node:crypto' -import { isAbsolute, relative as relativePath, resolve as resolvePath } from 'node:path' +import { isAbsolute, relative as relativePath, resolve as resolvePath, sep as pathSep } from 'node:path' import Schema from 'schemastery' import { AgentSideConnection, @@ -1008,9 +1008,12 @@ type AcpToolCallContent = function displayTitle(title: string, rawPath: string | undefined, sessionCwd: string | undefined): string { if (rawPath === undefined || sessionCwd === undefined || !isAbsolute(rawPath) || !isAbsolute(sessionCwd)) return title const rel = relativePath(sessionCwd, rawPath) - // `relative` returns a `..`-prefixed path for a target outside the workspace; - // only relativize paths that stay inside it (and never to the empty string). - if (rel.length === 0 || rel.startsWith('..')) return title + // Only relativize a target that stays INSIDE the workspace. `relative` prefixes + // a `..` SEGMENT for a target above the cwd — test for the segment (`..` alone + // or `..…`), NOT a bare `..` char prefix, so a sibling like `..cache/x` + // (a real in-workspace name) still relativizes. Never relativize to the empty + // string (rawPath === cwd — a non-file target). + if (rel.length === 0 || rel === '..' || rel.startsWith(`..${pathSep}`)) return title return title.split(rawPath).join(rel) } @@ -1127,39 +1130,44 @@ function terminalExitMeta(callId: string, view: TerminalResultView): TerminalExi */ function toolResultUpdate(callId: CallId, view: ToolResultView, isError: boolean, terminal: TerminalRendering): ToolCallSessionUpdate { const status = isError ? 'failed' as const : 'completed' as const - if (view.card === 'terminal') { - const output = view.output ?? '' - if (terminal.enabled) { + switch (view.card) { + case 'terminal': { + const output = view.output ?? '' + if (terminal.enabled) { + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + ...view.title !== undefined ? { title: view.title } : {}, + _meta: { + terminal_output: { terminal_id: callId, data: output }, + ...terminalExitMeta(callId, view), + }, + } + } + // No terminal capability: the bridge derives the fenced ```console fallback. + const fenced = `\`\`\`console\n${output.replace(/\n+$/, '')}\n\`\`\`` return { sessionUpdate: 'tool_call_update', toolCallId: callId, status, + content: [{ type: 'content', content: { type: 'text', text: fenced } }], ...view.title !== undefined ? { title: view.title } : {}, - _meta: { - terminal_output: { terminal_id: callId, data: output }, - ...terminalExitMeta(callId, view), - }, } } - // No terminal capability: the bridge derives the fenced ```console fallback. - const fenced = `\`\`\`console\n${output.replace(/\n+$/, '')}\n\`\`\`` - return { - sessionUpdate: 'tool_call_update', - toolCallId: callId, - status, - content: [{ type: 'content', content: { type: 'text', text: fenced } }], - ...view.title !== undefined ? { title: view.title } : {}, - } - } - // The presenter fills a generic result's content from the raw result, so - // `content` is always defined here; the guard keeps this total for a - // directly-constructed view. - return { - sessionUpdate: 'tool_call_update', - toolCallId: callId, - status, - /* v8 ignore next -- content always defined via the presenter (see above) */ - ...view.content !== undefined ? { content: toolResultContent(view.content) } : {}, - ...view.title !== undefined ? { title: view.title } : {}, + case 'generic': + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + // The presenter fills a generic result's content from the raw result, so + // `content` is always defined here; the guard keeps this total for a + // directly-constructed view. + /* v8 ignore next -- content always defined via the presenter (see above) */ + ...view.content !== undefined ? { content: toolResultContent(view.content) } : {}, + ...view.title !== undefined ? { title: view.title } : {}, + } + default: + return assertNever(view, 'ToolResultView.card') } } diff --git a/packages/ui/acp/tests/stream-update.spec.ts b/packages/ui/acp/tests/stream-update.spec.ts index 85e21d19a2..9c1898b3c1 100644 --- a/packages/ui/acp/tests/stream-update.spec.ts +++ b/packages/ui/acp/tests/stream-update.spec.ts @@ -356,6 +356,26 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => }))).toThrow('unreachable variant') }) + it('an unknown render-intent RESULT card throws via the exhaustiveness guard (closed union)', () => { + // The result-side renderer is also an exhaustive switch + assertNever: a rogue + // result card (only reachable by a cast) must throw, so adding a real result + // variant later fails to compile at the switch. + const rogue: ToolDefinition = { + name: 'rogue', + description: 'r', + parameters: {}, + execute: async () => [], + presentCall: () => ({ card: 'generic', title: 'r' }), + presentResult: () => ({ card: 'chart' }) as unknown as ReturnType>, + } + const presenter = new ToolPresenter(registryOf(rogue)) + expect(() => updatesWith( + presenter, + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'rogue', arguments: '{}' }), + evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'x' }], isError: false }), + )).toThrow('unreachable variant') + }) + it('forwards fs-tool render intents onto the wire (REAL read → generic locations, edit → diff content)', async () => { // Use the SHIPPING fs tools (not a stand-in), booted through their real // plugins, so the wire tool_call carries the actual presentCall output — @@ -653,6 +673,17 @@ describe('relative-path display titles (bridge relativizes the title against the await ctx.fiber.dispose() }) + it('an in-workspace file whose relative form starts with `..` chars (a sibling name) still relativizes', async () => { + // `/work/proj/..cache/x` is INSIDE the workspace — its relative form + // `..cache/x` begins with the chars `..` but is NOT a parent segment. The + // guard tests for a `..` SEGMENT, so this relativizes (matching the reference + // adapter, which accepts any target under `cwd + sep`). + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'read', { file_path: '/work/proj/..cache/x.ts' }) + expect((update as { title: string }).title).toBe('Read ..cache/x.ts') + await ctx.fiber.dispose() + }) + it('no session cwd → the absolute title is left unchanged', async () => { const ctx = await fsCtx() const update = callUpdate(ctx, undefined, 'read', { file_path: '/work/proj/src/a.ts' }) From a928a5a47a92d3082c7636da41e4bfa2bfdac300 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:04:11 +0800 Subject: [PATCH 3/5] docs(acp): don't enumerate tool/result fields in the presenter note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The callId→args map note said the tool/result event "carries only { callId, content, isError }" — an exhaustive field list that drifts as the event grows (it also carries error, and now meta). State the load- bearing fact instead: the event omits the tool name/args, which is why the bridge remembers them per callId. --- packages/ui/acp/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index 32e4326b99..7cb425cf74 100644 --- a/packages/ui/acp/README.md +++ b/packages/ui/acp/README.md @@ -50,7 +50,7 @@ How a tool call renders in the editor is owned by the TOOL, not the bridge — t `presentResult` returns a `ToolResultView`, one of two cards: `{ card: 'generic', title?, content? }` (an optional replacement `title` and reformatted `content`) or `{ card: 'terminal', title?, output?, exitCode?, signal? }` (the captured run output + exit — see Terminal card). The bridge looks the definition up by name in `ctx.tools` and `switch (view.card)`es to build the ACP `tool_call`/`tool_call_update` wire shape per card; 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` returns a `terminal` card for a foreground `bash` (title = the exact `command` "ls -la src", `description` = the model description); the `dsh-tool-fs` `write`/`edit` tools return a `diff` card and `read` returns a `generic` card (`kind: 'read'`, the read window in its title — `Read foo.txt (5 - 8)` — and a `locations` entry for the file). For a file card the bridge **relativizes the title** against the session cwd (mirroring `claude-agent-acp`'s `toDisplayPath` — `Read src/foo.ts`, not the absolute path) while keeping `locations[]`/`diffs[].path` **raw** so the editor opens the real path. -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. +The `tool/result` session event does not carry 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) From 4d36c0466bd41d9d8693d5c29c195a84d67562f7 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:20:14 +0800 Subject: [PATCH 4/5] docs(acp): drop tool/result field enumeration in ToolPresenter comment Same stale enumeration as the presenter-note fix, in the ToolPresenter JSDoc: it said the tool/result event "carries only { callId, content, isError }". The event also carries error and meta; the load-bearing fact is that it omits the tool name/args (why the presenter remembers them per callId). State that instead of an exhaustive list that drifts. --- packages/ui/acp/src/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index c01c004d91..231848de01 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -869,11 +869,11 @@ const noTerminalRendering: TerminalRendering = { enabled: false, cwd: undefined * by name in the registry and applies a generic fallback when a tool defines * neither. The returned view is what {@link streamSessionEventUpdate} switches on. * - * The `tool/result` session event carries only `{ callId, content, isError }` — - * NOT the tool name or args — so to call a tool's `presentResult` (which needs - * both), the presenter remembers each `tool/call`'s `{ name, args, card }` keyed - * by callId and looks it up on the matching result. The map is bridge-LOCAL (not - * a change to the event schema or a core service): one presenter per live session + * The `tool/result` session event does NOT carry the tool name or args — so to + * call a tool's `presentResult` (which needs both), the presenter remembers each + * `tool/call`'s `{ name, args, card }` keyed by callId and looks it up on the + * matching result. The map is bridge-LOCAL (not a change to the event schema or a + * core service): one presenter per live session * (and a throwaway per `session/load` replay), and each entry is removed when its * result arrives. In the normal loop a `tool/call` is always followed by a * `tool/result` (the registry turns even a thrown tool into an isError result), From 8ca82d03ab3edf72096ce8aa8f7316cd2e1dcb6f Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Fri, 3 Jul 2026 22:52:18 +0800 Subject: [PATCH 5/5] refactor(tool-web): port web tools to the render-intent union master's web_search/web_fetch tools were authored against the old ToolCallPresentation bag; the render-intent union replaces it with a card-tagged discriminated union. Both are simple generic cards, so they declare card:'generic' explicitly. --- packages/web/tool-web/src/fetch.ts | 6 +++--- packages/web/tool-web/src/search.ts | 6 +++--- packages/web/tool-web/tests/tool-web.spec.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/web/tool-web/src/fetch.ts b/packages/web/tool-web/src/fetch.ts index 85977fbea4..5f7334d952 100644 --- a/packages/web/tool-web/src/fetch.ts +++ b/packages/web/tool-web/src/fetch.ts @@ -7,7 +7,7 @@ import type { Context } from 'cordis' import { defineTool } from '@deepseek-ai/dsh-tools' -import type { ToolCallPresentation } from '@deepseek-ai/dsh-tools' +import type { GenericCallView } from '@deepseek-ai/dsh-tools' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { WebFetchBody, WebFetchResult } from '@deepseek-ai/dsh-web' import { assertNever } from '@deepseek-ai/dsh-llm' @@ -44,8 +44,8 @@ export function formatFetchOutput(result: WebFetchResult): string { } /** Pending-call presentation: a fetch card titled by the URL. */ -export function presentFetchCall(args: { url: string; timeout_ms?: number }): ToolCallPresentation { - return { title: args.url, kind: 'fetch', rawInput: args.url } +export function presentFetchCall(args: { url: string; timeout_ms?: number }): GenericCallView { + return { card: 'generic', title: args.url, kind: 'fetch', rawInput: args.url } } /** Register the `web_fetch` tool and its system-prompt guidance. */ diff --git a/packages/web/tool-web/src/search.ts b/packages/web/tool-web/src/search.ts index 2aa93ef10e..6394d3f7e0 100644 --- a/packages/web/tool-web/src/search.ts +++ b/packages/web/tool-web/src/search.ts @@ -7,7 +7,7 @@ import type { Context } from 'cordis' import { defineTool } from '@deepseek-ai/dsh-tools' -import type { ToolCallPresentation } from '@deepseek-ai/dsh-tools' +import type { GenericCallView } from '@deepseek-ai/dsh-tools' import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { WebSearchResult } from '@deepseek-ai/dsh-web' import type {} from '@deepseek-ai/dsh-system-prompt' @@ -63,8 +63,8 @@ export function formatSearchOutput(result: WebSearchResult): string { } /** Pending-call presentation: a search card titled by the query. */ -export function presentSearchCall(args: { query: string }): ToolCallPresentation { - return { title: args.query, kind: 'search', rawInput: args.query } +export function presentSearchCall(args: { query: string }): GenericCallView { + return { card: 'generic', title: args.query, kind: 'search', rawInput: args.query } } /** Register the `web_search` tool and its system-prompt guidance. */ diff --git a/packages/web/tool-web/tests/tool-web.spec.ts b/packages/web/tool-web/tests/tool-web.spec.ts index 2422c32ce3..7af1ce7c36 100644 --- a/packages/web/tool-web/tests/tool-web.spec.ts +++ b/packages/web/tool-web/tests/tool-web.spec.ts @@ -80,7 +80,7 @@ describe('search formatting', () => { }) it('presents a search call as a search-kind card titled by the query', () => { - expect(presentSearchCall({ query: 'find me' })).toEqual({ title: 'find me', kind: 'search', rawInput: 'find me' }) + expect(presentSearchCall({ query: 'find me' })).toEqual({ card: 'generic', title: 'find me', kind: 'search', rawInput: 'find me' }) }) }) @@ -116,7 +116,7 @@ describe('fetch formatting', () => { }) it('presents a fetch call as a fetch-kind card titled by the url', () => { - expect(presentFetchCall({ url: 'https://a.test' })).toEqual({ title: 'https://a.test', kind: 'fetch', rawInput: 'https://a.test' }) + expect(presentFetchCall({ url: 'https://a.test' })).toEqual({ card: 'generic', title: 'https://a.test', kind: 'fetch', rawInput: 'https://a.test' }) }) })