From bd7fb31ae36f87d9922749687a1d549889a6cca2 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 2 Jul 2026 19:36:17 +0800 Subject: [PATCH] feat(tool-fs): editor-facing presentation for read/write/edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fs tools rendered as generic cards (title = tool name, raw file content) in an ACP editor. Give them tool-owned presentation like bash/subagent have: - read → title "Read ", kind read, offset/limit as rawInput - write → title "Write ", kind edit - edit → title "Edit ", kind edit, a clipped old→new rawInput summary Add a provider-neutral `locations: { path, line? }[]` to ToolCallPresentation — the files a call reads/modifies — so a capable editor can follow along / jump to the file (read carries its offset as the line). The ACP bridge forwards it onto the wire `tool_call` (ResolvedCallPresentation + call() + the tool_call build in streamSessionEventUpdate). This flips the `locations` cell in the ACP feature matrix to supported. The SDK already carries `tool_call.locations` (ToolCallLocation `{ path, line? }`), so no ACP types leak into dsh-tools. presentResult is intentionally omitted: it only receives `{ content, isError }`, not the write/edit outcome, so titling by create-vs-overwrite or replacement count would mean parsing the model-facing text — the static title stays. Tests: pure presentCall assertions for all three tools incl. locations and the edit rawInput clip; a bridge test drives the REAL fs tools through ToolPresenter and asserts locations reaches the wire tool_call (proven to fail without the forwarding line). New withFs harness option + dsh-fs devDeps on dsh-acp. --- docs/cordis-catalog/events-and-services.md | 2 +- docs/core-data-structures/tools.md | 2 +- packages/core/tools/README.md | 2 +- packages/core/tools/src/index.ts | 10 +++++ packages/fs/tool-fs/src/edit.ts | 13 +++++++ packages/fs/tool-fs/src/read.ts | 15 ++++++++ packages/fs/tool-fs/src/write.ts | 7 ++++ packages/fs/tool-fs/tests/tools.spec.ts | 40 ++++++++++++++++++++ packages/ui/acp/README.md | 4 +- packages/ui/acp/acp-feature-support.md | 4 +- packages/ui/acp/package.json | 3 ++ packages/ui/acp/src/index.ts | 4 ++ packages/ui/acp/tests/harness.ts | 17 +++++++++ packages/ui/acp/tests/stream-update.spec.ts | 41 ++++++++++++++++++++- pnpm-lock.yaml | 9 +++++ 15 files changed, 164 insertions(+), 9 deletions(-) diff --git a/docs/cordis-catalog/events-and-services.md b/docs/cordis-catalog/events-and-services.md index c0fb5bd86b..22b6dc791b 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:277`](../../packages/core/tools/src/index.ts) +Source: [`packages/core/tools/src/index.ts:287`](../../packages/core/tools/src/index.ts) ## Inherited tier (cordis core + loader/hmr/timer) diff --git a/docs/core-data-structures/tools.md b/docs/core-data-structures/tools.md index f255fdbc32..1c1744c45c 100644 --- a/docs/core-data-structures/tools.md +++ b/docs/core-data-structures/tools.md @@ -105,7 +105,7 @@ 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`, 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` 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). > 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. diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 102645ad82..01a2e09d3b 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -72,7 +72,7 @@ See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, an A tool owns how ITS calls render in a UI (an editor's tool-call card, a CLI log line) — a UI plugin must NOT special-case tool names. A `ToolDefinition` may declare two optional, pure, display-only methods: -- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object), an optional `content` (UI content shown alongside the title/card — e.g. a bash `description` as a text block above the terminal card), and an optional `terminal` (a neutral `{ cwd? }` asking a capable UI to render this call as a TERMINAL, e.g. for `bash`). +- `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`. Returning `undefined` (or omitting a method) tells a UI to fall back to a generic presentation (title = tool name, raw args as input, raw result content). Both methods must be **pure and side-effect-free**: a UI may call them during live streaming AND during a session-log replay, so they depend only on their arguments. With `defineTool`, `args` is the typed `InferArgs` shape; the helper soft-validates before calling (a malformed/older logged arg shape yields `undefined` rather than throwing, since display must never crash a replay). The shapes are provider-neutral — the ACP bridge (`dsh-acp`) maps them to ACP `tool_call`/`tool_call_update` wire fields, and `dsh-tool-bash` is the reference implementation. diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 5a17aa2b0c..c2f324e87b 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -109,6 +109,16 @@ export interface ToolCallPresentation { * {@link terminal} block (if any) as a terminal card. */ 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 diff --git a/packages/fs/tool-fs/src/edit.ts b/packages/fs/tool-fs/src/edit.ts index c24692b012..1fb76ffd3c 100644 --- a/packages/fs/tool-fs/src/edit.ts +++ b/packages/fs/tool-fs/src/edit.ts @@ -83,5 +83,18 @@ 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) + return { + title: `Edit ${args.file_path}`, + kind: 'edit', + rawInput: `${JSON.stringify(clip(args.old_string))} → ${JSON.stringify(clip(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 5befd8be92..17fa7aa7ab 100644 --- a/packages/fs/tool-fs/src/read.ts +++ b/packages/fs/tool-fs/src/read.ts @@ -101,5 +101,20 @@ 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(', ') + return { + title: `Read ${args.file_path}`, + kind: 'read', + locations: [{ path: args.file_path, ...args.offset !== undefined ? { line: args.offset } : {} }], + ...detail.length > 0 ? { rawInput: detail } : {}, + } + }, })) } diff --git a/packages/fs/tool-fs/src/write.ts b/packages/fs/tool-fs/src/write.ts index 8cb91ececd..97098bd78d 100644 --- a/packages/fs/tool-fs/src/write.ts +++ b/packages/fs/tool-fs/src/write.ts @@ -62,5 +62,12 @@ 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 }] } + }, })) } diff --git a/packages/fs/tool-fs/tests/tools.spec.ts b/packages/fs/tool-fs/tests/tools.spec.ts index fb76f9215f..364d944b02 100644 --- a/packages/fs/tool-fs/tests/tools.spec.ts +++ b/packages/fs/tool-fs/tests/tools.spec.ts @@ -320,3 +320,43 @@ describe('edit tool', () => { expect(result.error).toMatchObject({ code: 'FS_NOT_OBSERVED' }) }) }) + +describe('tool-owned presentation (pure presentCall)', () => { + // presentCall is a pure display function of args (no I/O); it drives the ACP + // card's title/kind and the `locations` an editor follows along to. + const presentCall = async (name: string, args: unknown) => { + const { ctx } = await setup() + return ctx.tools.get(name)?.presentCall?.(args) + } + + it('read: titles by file, 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', + locations: [{ path: 'src/a.ts', line: 12 }], + }) + }) + + it('read: omits rawInput and the location line 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' }], + }) + }) + + 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('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('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')}`) + }) +}) diff --git a/packages/ui/acp/README.md b/packages/ui/acp/README.md index ad50383542..8d0444de70 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 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` (title/kind/rawInput/content/locations owned by the TOOL via `presentCall`/`presentResult` — see Tool-call presentation) | ## Multi-session @@ -42,7 +42,7 @@ 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, and optional `content` blocks shown alongside) and `presentResult(args, result)` (completed state: an optional replacement `title` and reformatted `content`) on its `dsh-tools` definition. The bridge looks the definition up by name in `ctx.tools` and maps the neutral `ToolCallPresentation`/`ToolResultPresentation` to the ACP `tool_call`/`tool_call_update` wire shapes. A tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` sets the title to the exact `command` ("ls -la src"), `kind: 'execute'`, the `command` as `rawInput`, the model `description` as a `content` text block, and wraps the completed output in a fenced ` ```console ` block. (The command is the title because an editor hides `rawInput` for execute-kind cards — Zed renders it only for non-terminal tools — and the reference adapters likewise use the command as an execute tool's title.) +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.) 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. diff --git a/packages/ui/acp/acp-feature-support.md b/packages/ui/acp/acp-feature-support.md index 429c862b27..c39b518aba 100644 --- a/packages/ui/acp/acp-feature-support.md +++ b/packages/ui/acp/acp-feature-support.md @@ -101,7 +101,7 @@ Tool-call presentation is **owned by each tool** (`presentCall` / `presentResult | `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). | | `terminal` content | S | ✅ | ✅ | ✅ | Via the Zed `_meta` terminal convention (see below), not the spec `terminal/*` sub-protocol. | -| `locations` (follow-along) | S | ❌ | ✅ | ✅ | No file-location hints emitted. | +| `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`. | | `rawOutput` | S | ❌ | ⚠️ | ✅ | Not emitted. | @@ -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 + location tool rendering** — `diff` content and `locations` for edit tools. +8. **Diff tool rendering** — structured `diff` content for edit tools (the `locations` follow-along hint already ships on `read`/`write`/`edit`). 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/package.json b/packages/ui/acp/package.json index 52080be092..b4c27f25e2 100644 --- a/packages/ui/acp/package.json +++ b/packages/ui/acp/package.json @@ -38,12 +38,15 @@ "@deepseek-ai/dsh-agent": "workspace:^", "@deepseek-ai/dsh-agent-loop": "workspace:^", "@deepseek-ai/dsh-bash-local": "workspace:^", + "@deepseek-ai/dsh-fs-local": "workspace:^", + "@deepseek-ai/dsh-fs-policy": "workspace:^", "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", "@deepseek-ai/dsh-session-persistence": "workspace:^", "@deepseek-ai/dsh-session-persistence-jsonl": "workspace:^", "@deepseek-ai/dsh-system-prompt": "workspace:^", "@deepseek-ai/dsh-tool-bash": "workspace:^", + "@deepseek-ai/dsh-tool-fs": "workspace:^", "@deepseek-ai/dsh-tool-todo": "workspace:^", "@deepseek-ai/dsh-tools": "workspace:^", "cordis": "^4.0.0-rc.6" diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 754b1750be..255ca9c990 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -836,6 +836,7 @@ export function streamSessionEventUpdate( 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) } } } @@ -926,6 +927,8 @@ interface ResolvedCallPresentation { 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 } @@ -1005,6 +1008,7 @@ export class ToolPresenter { 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 } : {}, } } diff --git a/packages/ui/acp/tests/harness.ts b/packages/ui/acp/tests/harness.ts index 9077d106d0..664ffbea5a 100644 --- a/packages/ui/acp/tests/harness.ts +++ b/packages/ui/acp/tests/harness.ts @@ -19,7 +19,10 @@ import AgentRegistry from '@deepseek-ai/dsh-agent' import AgentLoop from '@deepseek-ai/dsh-agent-loop' import SessionPersistenceJsonl from '@deepseek-ai/dsh-session-persistence-jsonl' import { LocalBashExecutor } from '@deepseek-ai/dsh-bash-local' +import LocalFileSystem from '@deepseek-ai/dsh-fs-local' +import * as FsPolicy from '@deepseek-ai/dsh-fs-policy' import * as ToolBash from '@deepseek-ai/dsh-tool-bash' +import * as ToolFs from '@deepseek-ai/dsh-tool-fs' import * as ToolTodo from '@deepseek-ai/dsh-tool-todo' import { ClientSideConnection, @@ -165,6 +168,15 @@ export async function makeBridgeHarness(options: { * tool + the bridge's own todo/write→plan mapping, not a stand-in. */ withTodo?: boolean + /** + * Plug the REAL filesystem stack (`dsh-fs-local` + `dsh-fs-policy` + + * `dsh-tool-fs`) so a test can drive `read`/`write`/`edit` through the bridge + * and assert their tool-owned presentation (title/kind/`locations`) on the + * wire — the shipping tools, not a stand-in. `fsCwd` sets the local backend's + * base directory (default: `storageDir`). + */ + withFs?: boolean + fsCwd?: string } = { storageDir: '' }): Promise { const adapter = new MockAdapter(options.script ?? []) @@ -183,6 +195,11 @@ export async function makeBridgeHarness(options: { if (options.withTodo) { await ctx.plugin(ToolTodo) } + if (options.withFs) { + await ctx.plugin(LocalFileSystem, { cwd: options.fsCwd ?? options.storageDir }) + await ctx.plugin(FsPolicy) + await ctx.plugin(ToolFs) + } ctx.llm.registerAdapter(['mock'], adapter) // Two identity byte pipes cross-wired into the two ndJsonStreams: bytes the diff --git a/packages/ui/acp/tests/stream-update.spec.ts b/packages/ui/acp/tests/stream-update.spec.ts index 89d68fd1c0..9e1578c8c1 100644 --- a/packages/ui/acp/tests/stream-update.spec.ts +++ b/packages/ui/acp/tests/stream-update.spec.ts @@ -1,8 +1,13 @@ import { describe, expect, it } from 'vitest' +import { Context } from 'cordis' import { CallId } from '@deepseek-ai/dsh-llm' import { SessionId, type SessionEvent } from '@deepseek-ai/dsh-session' import type { SessionNotification } from '@agentclientprotocol/sdk' -import type { ToolDefinition, ToolRegistry } from '@deepseek-ai/dsh-tools' +import type { ToolDefinition, ToolRegistry as ToolRegistryType } from '@deepseek-ai/dsh-tools' +import SystemPrompt from '@deepseek-ai/dsh-system-prompt' +import ToolRegistry from '@deepseek-ai/dsh-tools' +import FsLocal from '@deepseek-ai/dsh-fs-local' +import * as ToolFs from '@deepseek-ai/dsh-tool-fs' import { streamSessionEventUpdate, agentOptions, todosToPlan, ToolPresenter } from '../src/index.ts' /** Collect the updates a single event produces (no presenter → generic fallback). */ @@ -20,7 +25,7 @@ function liveUpdatesFor(event: SessionEvent): SessionNotification['update'][] { } /** A tiny tool registry stub exposing just `get` for {@link ToolPresenter}. */ -function registryOf(...tools: ToolDefinition[]): Pick { +function registryOf(...tools: ToolDefinition[]): Pick { const map = new Map(tools.map(t => [t.name, t])) return { get: name => map.get(name) } } @@ -330,6 +335,38 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => expect(updates[0]).toMatchObject({ sessionUpdate: 'tool_call', title: 'boom' }) 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 () => { + // 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".) + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(FsLocal) + await ctx.plugin(ToolFs) + const presenter = new ToolPresenter(ctx.tools) + + const [readCall] = updatesWith(presenter, evt('tool/call', { + turn: 1, step: 1, callId: CallId('r1'), name: 'read', + arguments: JSON.stringify({ file_path: 'src/a.ts', offset: 12 }), + })) + 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 }], + }) + + 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' }), + })) + expect(editCall).toMatchObject({ + sessionUpdate: 'tool_call', toolCallId: 'e1', title: 'Edit src/b.ts', kind: 'edit', + locations: [{ path: 'src/b.ts' }], + }) + await ctx.fiber.dispose() + }) }) describe('terminal-card mapping (capability-gated)', () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 58ea6d9bfe..380cfb3b4e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -739,6 +739,12 @@ importers: '@deepseek-ai/dsh-bash-local': specifier: workspace:^ version: link:../../bash/bash-local + '@deepseek-ai/dsh-fs-local': + specifier: workspace:^ + version: link:../../fs/fs-local + '@deepseek-ai/dsh-fs-policy': + specifier: workspace:^ + version: link:../../fs/fs-policy '@deepseek-ai/dsh-llm': specifier: workspace:^ version: link:../../llm/llm @@ -757,6 +763,9 @@ importers: '@deepseek-ai/dsh-tool-bash': specifier: workspace:^ version: link:../../bash/tool-bash + '@deepseek-ai/dsh-tool-fs': + specifier: workspace:^ + version: link:../../fs/tool-fs '@deepseek-ai/dsh-tool-todo': specifier: workspace:^ version: link:../../todo/tool-todo