From 7803c38824f6617536a34b918e00fca0632c001e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 18 Jun 2026 09:01:36 +0800 Subject: [PATCH] feat(acp): tool-owned tool-call UI presentation (title/command/output) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Zed the tool-call card showed only "bash" — the bare tool name — instead of what the command does. Fix it by letting each TOOL own how its calls render, rather than the bridge special-casing names. dsh-tools: add an optional two-state presentation seam to ToolDefinition / defineTool — `presentCall(args)` (pending: title, kind, rawInput) and `presentResult(args, result)` (completed: title?, content?). Provider-neutral `ToolCallKind`/`ToolCallPresentation`/`ToolResultPresentation` vocabulary so tools never depend on ACP. defineTool soft-validates args (display runs on log replay, so a malformed/old shape returns undefined instead of throwing). dsh-tool-bash: bash declares presentCall (model `description` → title, exact `command` → rawInput, kind execute) and presentResult (wrap output in a fenced ```console block — a UI-only affordance kept out of the model-facing result); bash_output/bash_kill present task-scoped titles. dsh-acp: inject `tools`; a per-session `ToolPresenter` looks the tool up by name and maps its neutral presentation to the ACP tool_call/tool_call_update wire shape, with a generic fallback (title = name) for tools that declare nothing. Because the `tool/result` event carries only {callId, content, isError}, the presenter keeps a small bridge-local map of ONLY in-flight calls' (name, args), keyed by callId and removed as each result is presented — no event-schema or core change. Replay uses a throwaway presenter so loaded sessions render identically to live ones. Tests: dsh-tools defineTool presenters (typed args, soft-validate), tool-bash bash/bash_output/bash_kill presenters, acp ToolPresenter (tool-owned mapping, unknown-callId fallback, in-flight-only map), and an end-to-end turn through the bridge. The key-gated e2e now asserts a real bash call's title is the model description (not "bash") and rawInput is the command — verified against the real DeepSeek model. The test harness derives its inject from the bridge's exported `inject` so it can't drift again. --- docs/module-graph.md | 11 +- examples/acp-agent/tests/acp.e2e.ts | 17 ++- packages/acp/README.md | 8 +- packages/acp/package.json | 1 + packages/acp/src/index.ts | 127 +++++++++++++++++++-- packages/acp/tests/harness.ts | 6 +- packages/acp/tests/stream-update.spec.ts | 136 ++++++++++++++++++++++- packages/acp/tests/turns.spec.ts | 36 ++++++ packages/acp/tsconfig.json | 1 + packages/tool-bash/README.md | 4 + packages/tool-bash/src/index.ts | 44 ++++++++ packages/tool-bash/tests/tools.spec.ts | 55 +++++++++ packages/tools/README.md | 36 +++++- packages/tools/src/index.ts | 78 +++++++++++++ packages/tools/src/schema.ts | 41 ++++++- packages/tools/tests/tools.spec.ts | 51 +++++++++ 16 files changed, 626 insertions(+), 26 deletions(-) diff --git a/docs/module-graph.md b/docs/module-graph.md index a99e6cb071..5110178e0e 100644 --- a/docs/module-graph.md +++ b/docs/module-graph.md @@ -15,10 +15,6 @@ graph TD agent --> llm agent --> session session-persistence --> session - acp --> agent - acp --> llm - acp --> session - acp --> session-persistence invariants --> agent invariants --> llm invariants --> session @@ -29,6 +25,11 @@ graph TD tools --> agent tools --> llm tools --> system-prompt + acp --> agent + acp --> llm + acp --> session + acp --> session-persistence + acp --> tools agent-loop --> agent agent-loop --> llm agent-loop --> session @@ -52,10 +53,10 @@ graph TD | `system-prompt` | `llm` | | `agent` | `llm`, `session` | | `session-persistence` | `session` | -| `acp` | `agent`, `llm`, `session`, `session-persistence` | | `invariants` | `agent`, `llm`, `session` | | `session-persistence-jsonl` | `session`, `session-persistence` | | `session-persistence-sqlite` | `session`, `session-persistence` | | `tools` | `agent`, `llm`, `system-prompt` | +| `acp` | `agent`, `llm`, `session`, `session-persistence`, `tools` | | `agent-loop` | `agent`, `llm`, `session`, `session-persistence`, `system-prompt`, `tools` | | `tool-bash` | `agent`, `bash`, `llm`, `tools` | diff --git a/examples/acp-agent/tests/acp.e2e.ts b/examples/acp-agent/tests/acp.e2e.ts index 35bef79e14..caab2b0f4a 100644 --- a/examples/acp-agent/tests/acp.e2e.ts +++ b/examples/acp-agent/tests/acp.e2e.ts @@ -174,6 +174,21 @@ describe.skipIf(!process.env.DEEPSEEK_API_KEY)('acp-agent e2e: real prompt over expect(proof).toContain('ACP_OK') // And the client saw tool-call activity stream through. - expect(updates.some(u => u.sessionUpdate === 'tool_call')).toBe(true) + const toolCalls = updates.filter(u => u.sessionUpdate === 'tool_call') + expect(toolCalls.length).toBeGreaterThan(0) + + // Tool-call UI quality (the tool owns its presentation): the bash tool's + // `presentCall` sets the title to the model's human-readable `description` + // and the `rawInput` to the exact command — NOT the bare tool name "bash". + // A `bash` call must therefore carry an execute kind, a non-"bash" title, + // and a string rawInput (the command). `toolCalls` is already narrowed to + // the `tool_call` shape by the filter above, so these fields are reachable. + const bashCall = toolCalls.find(u => u.kind === 'execute') + expect(bashCall).toBeDefined() + if (bashCall === undefined) throw new Error('expected an execute tool_call') + expect(typeof bashCall.title).toBe('string') + expect(bashCall.title.length).toBeGreaterThan(0) + expect(bashCall.title).not.toBe('bash') // the old, unhelpful title + expect(typeof bashCall.rawInput).toBe('string') // the exact command }, 180_000) }) diff --git a/packages/acp/README.md b/packages/acp/README.md index e528acdb45..f515ae8293 100644 --- a/packages/acp/README.md +++ b/packages/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` only needs to be absolute. After the async resume a `closed` re-check refuses to install a record if the bridge tore down mid-load | | `session/prompt` | `agent.send()` | text-only; rejects image/audio 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.abort()` | aborts a running step + settles the prompt `cancelled` for ONLY that session — a cancel never touches another session's stream or prompt (see limitation below) | -| `session/update` | `session/event` | `agent_message_chunk` (text-delta), `agent_thought_chunk` (reasoning-delta), `user_message_chunk` (load replay), `tool_call`/`tool_call_update` | +| `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) | ## Multi-session (RFC 011) @@ -40,6 +40,12 @@ Background-task isolation rides on `dsh-tool-bash`: bash task ids are global and Each session runs in its own workspace, recorded as the session's `SessionHeader.cwd`. On `session/new` the (absolute) request `cwd` becomes that header cwd; on `session/load` the resumed session keeps its PERSISTED header cwd (the request `cwd` is only shape-checked — it does not override the stored one), and a load whose persisted session has no absolute cwd is REJECTED up front via a metadata-only `list()` check, BEFORE resume constructs an agent (else bash would silently fall back to the server's launch dir, and a post-resume reject would leak the registered agent). `dsh-tool-bash` then defaults the bash workdir to the calling agent's `session.header.cwd` (an explicit model `workdir` still wins; a relative one resolves against the session cwd; with no session cwd the executor falls back to its own config / `process.cwd()`). So the server no longer has to be launched in the workspace — an editor can open any project folder, and N sessions over one connection can each target a different directory. (`additionalDirectories` is still rejected: widening the tool/filesystem scope beyond the single cwd is a separate sandbox concern.) +## Tool-call presentation + +How a tool call renders in the editor is owned by the TOOL, not the bridge — the bridge never special-cases tool names. Each tool may declare `presentCall(args)` (pending state: a human-readable `title`, a `kind` for the icon, and the salient `rawInput` to show in a detail view) and `presentResult(args, result)` (completed state: an optional replacement `title` and reformatted `content`) on its `dsh-tools` definition. The bridge looks the definition up by name in `ctx.tools` and maps the neutral `ToolCallPresentation`/`ToolResultPresentation` to the ACP `tool_call`/`tool_call_update` wire shapes. A tool that declares neither gets a generic fallback (title = tool name, raw parsed args as `rawInput`, kind inferred from the name). For example `dsh-tool-bash` makes the model-written one-line `description` the title ("List files in the current directory"), the exact `command` the `rawInput`, `kind: 'execute'`, and wraps the completed output in a fenced ` ```console ` block. + +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. + ## Settle-exactly-once A `session/prompt` resolves (or rejects) exactly once, keyed off the canonical session log (the `session/event` stream), NOT the `agent/turn-start`/`agent/turn-end` events. One listener captures the prompt's owning turn from the log's `turn/start` and settles on the matching `turn/end` — the one signal that always fires (`closeTurn` appends it unconditionally, even when a boundary emit throws and the `agent/turn-end` EVENT is skipped). A prompt settles only on ITS OWN turn (`inflight.turn === turn/end.turn`), so a stale `turn/end` for a previously-cancelled turn whose end arrives late can never settle the wrong prompt. A turn that ends `error` REJECTS the RPC with an internal error carrying the failure message (ACP has no error stop reason); every other reason resolves via the codec. As a fallback, when the agent settles to `idle`/`disposed` with a prompt still pending — e.g. a `session/event` listener registered before the bridge threw and starved the bridge's listener — an `agent/status` handler reconciles the prompt from the log (the owning turn's `turn/end`, or `cancelled` if the turn was torn down without one). An empty/whitespace prompt is rejected up front — it would queue no work, so no turn would start and the RPC would hang. diff --git a/packages/acp/package.json b/packages/acp/package.json index 669c03043d..9cac888518 100644 --- a/packages/acp/package.json +++ b/packages/acp/package.json @@ -29,6 +29,7 @@ "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", "@deepseek-ai/dsh-session-persistence": "^0.0.1", + "@deepseek-ai/dsh-tools": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "devDependencies": { diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index 18f3adb670..793685072b 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -60,6 +60,7 @@ import { import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' import type { SessionEvent } from '@deepseek-ai/dsh-session' +import type { ToolCallKind, ToolRegistry } 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' @@ -73,8 +74,10 @@ import { export const name = 'acp' // The bridge programs against the interface packages only (architecture rule: // plugins never depend on dsh-agent-loop). `sessionPersistence` is required -// because `initialize` advertises `loadSession: true`. -export const inject = ['agents', 'sessions', 'sessionPersistence'] +// because `initialize` advertises `loadSession: true`. `tools` lets a tool own +// how its calls render (`presentCall`/`presentResult`); the bridge looks up the +// definition by name and falls back to a generic presentation when absent. +export const inject = ['agents', 'sessions', 'sessionPersistence', 'tools'] /** * Build an ACP "invalid params" error whose human detail rides in the message. @@ -131,6 +134,13 @@ export const Config: Schema = Schema.object({ interface SessionRecord { sessionId: string agent: Agent + /** + * Resolves tool-owned presentation for THIS session's tool calls and remembers + * each in-flight call's `(name, args)` so the matching `tool/result` can find + * its tool. Per-session so two concurrent sessions never cross their in-flight + * tool state. + */ + presenter: ToolPresenter /** * The in-flight `session/prompt`, or `undefined` when none is pending. A * prompt resolves with a {@link StopReason} or rejects with an Error (a @@ -182,6 +192,7 @@ export function apply(ctx: Context, config: AcpConfig): void { const agents = ctx.agents const sessionPersistence = ctx.sessionPersistence const logger = ctx.logger + const tools = ctx.tools // Live sessions keyed by id (RFC 011 multi-session), plus an agent→sessionId // reverse map so `agent/*` events (which carry only the Agent) demux in O(1). @@ -270,7 +281,7 @@ export function apply(ctx: Context, config: AcpConfig): void { ctx.on('session/event', (session, event: SessionEvent) => { const rec = sessions.get(session.header.id) if (rec === undefined) return - streamSessionEventUpdate(rec.sessionId, event, notify) + streamSessionEventUpdate(rec.sessionId, event, notify, rec.presenter) const inflight = rec.inflight if (inflight === undefined) return if (event.type === 'turn/start') { @@ -395,7 +406,7 @@ export function apply(ctx: Context, config: AcpConfig): void { agentOptions: agentOptions(config), }) bySession.set(agent, sessionId) - sessions.set(sessionId, { sessionId, agent, inflight: undefined }) + sessions.set(sessionId, { sessionId, agent, presenter: new ToolPresenter(tools), inflight: undefined }) return Promise.resolve({ sessionId }) }, @@ -448,14 +459,18 @@ export function apply(ctx: Context, config: AcpConfig): void { throw invalidParams('connection closed during session/load') } bySession.set(agent, params.sessionId) - sessions.set(params.sessionId, { sessionId: params.sessionId, agent, inflight: undefined }) + const record: SessionRecord = { sessionId: params.sessionId, agent, presenter: new ToolPresenter(tools), inflight: undefined } + sessions.set(params.sessionId, record) // Replay the persisted event log to the client as session/update. Use // the raw event log (NOT deriveMessages, which drops assistant/chunk // and trace events): RFC 010's load contract reconstructs the streamed // turns — user prompts (user/message → user_message_chunk), assistant - // text and reasoning (assistant/chunk), and tool calls/results. + // text and reasoning (assistant/chunk), and tool calls/results. The + // record's presenter pairs each tool/call with its tool/result as the + // log replays in order, so the replayed tool cards render identically + // to the live ones. for (const event of agent.session.events) { - streamSessionEventUpdate(params.sessionId, event, notify) + streamSessionEventUpdate(params.sessionId, event, notify, record.presenter) } return {} } finally { @@ -659,6 +674,14 @@ function validateWorkspaceParams(params: { cwd: string; additionalDirectories?: * - `tool/call` → `tool_call` (pending) * - `tool/result` → `tool_call_update` (completed/failed) * + * Tool-call presentation (title/kind/rawInput, and the completed-state content) + * is owned by each TOOL via `presentCall`/`presentResult` — the bridge never + * special-cases tool names. `presenter` resolves those from the tool registry + * and remembers each call's `(name, args)` so the completed `tool/result` (which + * carries neither) can find its tool. A {@link nullToolPresenter} gives the + * generic fallback (title = tool name, raw args as input) when no registry is + * available (e.g. pure translator tests). + * * Other event types (turn/step boundaries, context/message, usage, …) produce * no client update. */ @@ -666,6 +689,7 @@ export function streamSessionEventUpdate( sessionId: string, event: SessionEvent, notify: (notification: SessionNotification) => void, + presenter: Pick = nullToolPresenter, ): void { switch (event.type) { case 'assistant/chunk': { @@ -690,27 +714,30 @@ export function streamSessionEventUpdate( return } case 'tool/call': { + const present = presenter.call(event.data.callId, event.data.name, event.data.arguments) notify({ sessionId, update: { sessionUpdate: 'tool_call', toolCallId: event.data.callId, - title: event.data.name, - kind: toolKindFor(event.data.name), + title: present.title, + kind: present.kind, status: 'in_progress', - rawInput: parseToolArguments(event.data.arguments), + ...present.rawInput !== undefined ? { rawInput: present.rawInput } : {}, }, }) return } case 'tool/result': { + const present = presenter.result(event.data.callId, event.data.content, event.data.isError) notify({ sessionId, update: { sessionUpdate: 'tool_call_update', toolCallId: event.data.callId, status: event.data.isError ? 'failed' : 'completed', - content: toolResultContent(event.data.content), + content: toolResultContent(present.content), + ...present.title !== undefined ? { title: present.title } : {}, }, }) return @@ -722,8 +749,84 @@ export function streamSessionEventUpdate( } } +/** + * 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 +} + +/** 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 +} + +/** + * 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. + * + * 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), entries removed as each result + * arrives, so it holds only the currently-in-flight calls. + */ +export class ToolPresenter { + private readonly pending = new Map() + + constructor(private readonly tools: Pick) {} + + /** Pending-state presentation for a `tool/call`; remembers `(name, args)` for the matching result. */ + call(callId: string, name: string, argsJson: string): ResolvedCallPresentation { + const args = parseToolArguments(argsJson) + this.pending.set(callId, { name, args }) + const present = this.tools.get(name)?.presentCall?.(args) + 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). + return { title: name, kind: toolKindFor(name), rawInput: args } + } + return { title: present.title, kind: present.kind ?? 'other', rawInput: present.rawInput } + } + + /** Completed-state presentation for a `tool/result`; consumes the remembered `(name, args)`. */ + result(callId: string, content: ContentBlock[], isError: boolean): ResolvedResultPresentation { + const call = this.pending.get(callId) + this.pending.delete(callId) + const present = call !== undefined + ? this.tools.get(call.name)?.presentResult?.(call.args, { content, isError }) + : undefined + if (present === undefined) return { content } + return { + content: present.content ?? content, + ...present.title !== undefined ? { title: present.title } : {}, + } + } +} + +/** + * The no-op presenter used when no tool registry is available (e.g. the pure + * translator tests): every tool gets the generic fallback presentation, and + * 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 }), +} + /** Map a harness tool name to an ACP ToolKind (best-effort; default `other`). */ -function toolKindFor(name: string): 'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other' { +function toolKindFor(name: string): ToolCallKind { if (name === 'bash' || name === 'bash_output' || name === 'bash_kill') return 'execute' if (name === 'read' || name.startsWith('read')) return 'read' if (name === 'write' || name === 'edit' || name.startsWith('edit')) return 'edit' diff --git a/packages/acp/tests/harness.ts b/packages/acp/tests/harness.ts index bee16c46eb..27335cfcff 100644 --- a/packages/acp/tests/harness.ts +++ b/packages/acp/tests/harness.ts @@ -234,7 +234,11 @@ export async function makeBridgeHarness(options: { // tears down JUST the bridge (its listeners + effect) for the HMR test. harness.acpFiber = await ctx.plugin({ name: 'acp-test', - inject: ['agents', 'sessions', 'sessionPersistence'], + // Use the bridge's REAL exported `inject` so this never drifts from the + // plugin's actual dependency list (adding a service to the bridge must not + // require editing the harness — a hardcoded list silently broke when `tools` + // was added). The bridge programs against the interface packages only. + inject: [...AcpPlugin.inject], apply: (inner: Context) => { AcpPlugin.apply(inner, cfg) }, }) harness.client = new ClientSideConnection(makeClient, clientStream) diff --git a/packages/acp/tests/stream-update.spec.ts b/packages/acp/tests/stream-update.spec.ts index c37de8b344..4328b5fb6b 100644 --- a/packages/acp/tests/stream-update.spec.ts +++ b/packages/acp/tests/stream-update.spec.ts @@ -2,15 +2,22 @@ import { describe, expect, it } from 'vitest' import { CallId } from '@deepseek-ai/dsh-llm' import type { SessionEvent } from '@deepseek-ai/dsh-session' import type { SessionNotification } from '@agentclientprotocol/sdk' -import { streamSessionEventUpdate, agentOptions } from '../src/index.ts' +import type { ToolDefinition, ToolRegistry } from '@deepseek-ai/dsh-tools' +import { streamSessionEventUpdate, agentOptions, ToolPresenter } from '../src/index.ts' -/** Collect the updates a single event produces. */ +/** Collect the updates a single event produces (no presenter → generic fallback). */ function updatesFor(event: SessionEvent): SessionNotification['update'][] { const out: SessionNotification['update'][] = [] streamSessionEventUpdate('s1', event, n => out.push(n.update)) return out } +/** A tiny tool registry stub exposing just `get` for {@link ToolPresenter}. */ +function registryOf(...tools: ToolDefinition[]): Pick { + const map = new Map(tools.map(t => [t.name, t])) + return { get: name => map.get(name) } +} + function evt(type: T, data: Extract['data']): SessionEvent { return { type, seq: 0, time: 0, data } as SessionEvent } @@ -31,7 +38,7 @@ describe('streamSessionEventUpdate', () => { .toEqual([]) }) - it('maps tool/call to an in_progress tool_call with inferred kind and parsed rawInput', () => { + it('maps tool/call to an in_progress tool_call with inferred kind and parsed rawInput (generic fallback, no presenter)', () => { const updates = updatesFor(evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: '{"command":"ls"}' })) expect(updates).toEqual([{ sessionUpdate: 'tool_call', @@ -99,6 +106,129 @@ describe('streamSessionEventUpdate', () => { }) }) +describe('ToolPresenter (tool-owned presentation via the tool registry)', () => { + /** A tool whose presentCall/presentResult mirror what tool-bash declares. */ + const bashLike: ToolDefinition = { + name: 'bash', + description: 'run a command', + parameters: {}, + execute: async () => [], + presentCall: (args: unknown) => { + const a = args as { command: string; description: string } + return { title: a.description, kind: 'execute', rawInput: a.command } + }, + presentResult: (_args: unknown, result: { content: { type: string }[] }) => ({ + content: [{ type: 'text', text: `wrapped:${result.content.length}` }], + }), + } + + function updatesWith(presenter: ToolPresenter, ...events: SessionEvent[]): SessionNotification['update'][] { + const out: SessionNotification['update'][] = [] + for (const event of events) streamSessionEventUpdate('s1', event, n => out.push(n.update), presenter) + return out + } + + it('tool/call uses the tool: description→title, command→rawInput, tool kind', () => { + const presenter = new ToolPresenter(registryOf(bashLike)) + const [update] = updatesWith(presenter, evt('tool/call', { + turn: 1, step: 1, callId: CallId('c1'), name: 'bash', + arguments: JSON.stringify({ command: 'ls -la', description: 'List files' }), + })) + expect(update).toEqual({ + sessionUpdate: 'tool_call', + toolCallId: 'c1', + title: 'List files', + kind: 'execute', + status: 'in_progress', + rawInput: 'ls -la', + }) + }) + + it('tool/result uses the tool to reformat content (resolved by the remembered tool/call)', () => { + const presenter = new ToolPresenter(registryOf(bashLike)) + const updates = updatesWith( + presenter, + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: JSON.stringify({ command: 'x', description: 'd' }) }), + evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'out' }], isError: false }), + ) + expect(updates[1]).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: 'wrapped:1' } }], + }) + }) + + it('a result with NO preceding call (unknown callId) falls back to the raw content', () => { + const presenter = new ToolPresenter(registryOf(bashLike)) + // No tool/call for c9 → presenter has nothing remembered → generic fallback. + const [update] = updatesWith(presenter, evt('tool/result', { + turn: 1, step: 1, callId: CallId('c9'), content: [{ type: 'text', text: 'raw' }], isError: false, + })) + expect(update).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c9', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: 'raw' } }], + }) + }) + + it('a tool with no presentCall/presentResult gets the generic fallback (title = name)', () => { + const plain: ToolDefinition = { name: 'plain', description: 'p', parameters: {}, execute: async () => [] } + const presenter = new ToolPresenter(registryOf(plain)) + const [update] = updatesWith(presenter, evt('tool/call', { + turn: 1, step: 1, callId: CallId('c1'), name: 'plain', arguments: '{"a":1}', + })) + expect(update).toMatchObject({ title: 'plain', kind: 'other', rawInput: { a: 1 } }) + }) + + it('a presentation that omits kind/content/rawInput uses the defaults (kind other, raw result content kept)', () => { + // A minimal tool-owned presentation: presentCall returns only a title (no + // kind → defaults to `other`, no rawInput → omitted); presentResult returns + // only a title (no content → the raw result content is kept). + const minimal: ToolDefinition = { + name: 'mini', + description: 'm', + parameters: {}, + execute: async () => [], + presentCall: () => ({ title: 'Doing a thing' }), + presentResult: () => ({ title: 'Did the thing' }), + } + const presenter = new ToolPresenter(registryOf(minimal)) + const updates = updatesWith( + presenter, + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'mini', arguments: '{}' }), + evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'kept' }], isError: false }), + ) + // No kind → 'other'; no rawInput key at all. + expect(updates[0]).toEqual({ sessionUpdate: 'tool_call', toolCallId: 'c1', title: 'Doing a thing', kind: 'other', status: 'in_progress' }) + // Title replaced; content falls back to the raw result content. + expect(updates[1]).toEqual({ + sessionUpdate: 'tool_call_update', + toolCallId: 'c1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: 'kept' } }], + title: 'Did the thing', + }) + }) + + it('holds ONLY in-flight calls: the callId entry is removed once its result is presented', () => { + const presenter = new ToolPresenter(registryOf(bashLike)) + updatesWith( + presenter, + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'bash', arguments: JSON.stringify({ command: 'x', description: 'd' }) }), + evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'o' }], isError: false }), + ) + // A SECOND result for the same callId now finds nothing remembered, so it + // falls back to raw content (proving the first result consumed the entry — + // the map does not retain finished calls). + const [late] = updatesWith(presenter, evt('tool/result', { + turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'late' }], isError: false, + })) + expect(late).toMatchObject({ content: [{ type: 'content', content: { type: 'text', text: 'late' } }] }) + }) +}) + describe('agentOptions', () => { it('includes only the fields present in config', () => { expect(agentOptions({})).toEqual({}) diff --git a/packages/acp/tests/turns.spec.ts b/packages/acp/tests/turns.spec.ts index eef0f392aa..7258c590b4 100644 --- a/packages/acp/tests/turns.spec.ts +++ b/packages/acp/tests/turns.spec.ts @@ -75,6 +75,42 @@ describe('acp bridge — turn outcomes', () => { expect(callIdx).toBeLessThan(updIdx) }) + it('a tool-owned presentation flows end-to-end: presentCall sets title/rawInput, presentResult reformats output', async () => { + harness = await makeBridgeHarness({ + storageDir, + script: [toolCallResponse('c1', 'bash', { command: 'ls -la', description: 'List files' }), textResponse('done')], + }) + // A tool that declares its OWN presentation (like the real tool-bash). The + // bridge must use it — NOT the generic title=name fallback — proving the + // tool-owns-its-rendering seam works through the real session-event path. + harness.ctx.tools.register(defineTool({ + name: 'bash', + description: 'run a command', + parameters: { + command: { type: 'string', required: true }, + description: { type: 'string', required: true }, + }, + async execute() { return [{ type: 'text', text: 'a.txt\nb.txt\n' }] }, + presentCall: args => ({ title: args.description, kind: 'execute', rawInput: args.command }), + 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.trimEnd()}\n\`\`\`` }] } + }, + })) + const sessionId = await newSession(harness) + await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'list' }] }) + + const call = harness.updates.find(u => u.sessionUpdate === 'tool_call') + expect(call).toMatchObject({ toolCallId: 'c1', title: 'List files', kind: 'execute', rawInput: 'ls -la', status: 'in_progress' }) + const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update') + expect(update).toMatchObject({ + toolCallId: 'c1', + status: 'completed', + content: [{ type: 'content', content: { type: 'text', text: '```console\na.txt\nb.txt\n```' } }], + }) + }) + it('a failing tool yields a failed tool_call_update', async () => { harness = await makeBridgeHarness({ storageDir, diff --git a/packages/acp/tsconfig.json b/packages/acp/tsconfig.json index 2efad36448..83330256e3 100644 --- a/packages/acp/tsconfig.json +++ b/packages/acp/tsconfig.json @@ -12,6 +12,7 @@ { "path": "../llm" }, { "path": "../session" }, { "path": "../agent" }, + { "path": "../tools" }, { "path": "../session-persistence" } ] } diff --git a/packages/tool-bash/README.md b/packages/tool-bash/README.md index 83f84e967d..473f5a4099 100644 --- a/packages/tool-bash/README.md +++ b/packages/tool-bash/README.md @@ -32,6 +32,10 @@ Result text: stdout, then a `[stderr]` section, then status markers — `[timed The owning agent is recorded per task id at spawn and kept for the lifetime of the loaded plugin instance (it is **not** cleared on completion). `bash_output`/`bash_kill` reject a task owned by a *different* agent with `task belongs to another session` (a task started with no agent — a non-loop caller — has no owner and is open to anyone; a call with no `exec.agent` cannot access an owned task). Task ids are global and predictable, so under multi-session ACP this ownership check is the fence that stops one session's agent from reading or killing another session's background task. (`TODO(tool-bash-owner-hmr)`: an independent HMR reload of this plugin starts a fresh map, so a task spawned before the reload becomes un-owned — acceptable as HMR is dev-only and the session boundary is one user's cooperative editor; a durable fix attaches ownership to the executor/task lifetime.) +## 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 model-written `description` is the always-visible **title** (e.g. "List files in the current directory"), the exact `command` is the **rawInput** (the verbatim command stays visible in a detail view without crowding the title), `kind` is `execute` (terminal/run treatment), and the completed output is wrapped in a fenced ` ```console ` block — a UI-only affordance, so the model-facing result text stays unfenced. `bash_output`/`bash_kill` present a task-scoped title ("Read output from background task bash-3" / "Kill background task bash-3") with the task id as rawInput. These methods are pure/display-only (they also run on `session/load` replay), and a malformed/older logged arg shape falls back to a generic presentation rather than throwing. See `packages/tools` ("Tool-owned UI presentation") and `packages/acp` ("Tool-call presentation"). + ## Background completion notices When a background task finishes, a short notice is injected into the owning agent's session (`agent.inject()`, source `{kind: 'plugin', plugin: 'tool-bash'}`). Injection is **durable context for the next model request, not a wake-up** — an idle agent stays idle until something sends a message. That's why the tool descriptions tell the model to poll with `bash_output`. diff --git a/packages/tool-bash/src/index.ts b/packages/tool-bash/src/index.ts index 9a15c6ad7a..9b4aa0819b 100644 --- a/packages/tool-bash/src/index.ts +++ b/packages/tool-bash/src/index.ts @@ -39,6 +39,8 @@ import type { Context } from 'cordis' import { isAbsolute, resolve as resolvePath } from 'node:path' import { defineTool } from '@deepseek-ai/dsh-tools' +import type { ToolCallPresentation, ToolResult, ToolResultPresentation } from '@deepseek-ai/dsh-tools' +import type { ContentBlock } from '@deepseek-ai/dsh-llm' import type { Agent } from '@deepseek-ai/dsh-agent' import type { BashRunResult, BashTask, CollectedOutput } from '@deepseek-ai/dsh-bash' @@ -124,6 +126,44 @@ export function renderResult(result: BashRunResult): string { return body + markers.join('\n') } +// --------------------------------------------------------------------------- +// UI presentation (tool-owned). These shape how a UI (e.g. the ACP bridge) +// renders a bash call's pending and completed states. They are display-only and +// pure — a UI may call them during live streaming AND a session-log replay. +// --------------------------------------------------------------------------- + +/** + * Pending-state presentation for a `bash` call: the model-written `description` + * is the always-visible title (the schema requires it precisely so a UI has a + * readable summary — "List files in the current directory"), `kind: 'execute'` + * (a terminal/run treatment), and the exact `command` is the `rawInput` so the + * verbatim command stays visible in a UI's detail view without crowding the + * title. Mirrors how Zed / the reference ACP adapters render execute tools. + */ +function presentBashCall(args: { command: string; description: string }): ToolCallPresentation { + return { title: args.description, kind: 'execute', rawInput: args.command } +} + +/** + * Completed-state presentation for a `bash` call: wrap the model-facing result + * text in a fenced ```console block so a UI renders the output monospaced as a + * terminal transcript. The model-facing `content` (what `execute` returned) is + * intentionally NOT fenced — the fences are a UI-only affordance, so they live + * here, not in `renderResult`. A non-text result (unexpected for bash) is left + * untouched by falling back to `undefined`. + */ +function presentBashResult(_args: unknown, result: ToolResult): ToolResultPresentation | undefined { + const block = result.content.length === 1 ? result.content[0] : undefined + if (block === undefined || block.type !== 'text') return undefined + const fenced: ContentBlock = { type: 'text', text: `\`\`\`console\n${block.text.replace(/\n+$/, '')}\n\`\`\`` } + return { content: [fenced] } +} + +/** 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 } +} + /** * Resolve the working directory for a bash call. Precedence: an explicit model * `workdir` wins; otherwise default to the calling agent's session cwd @@ -242,6 +282,8 @@ export function apply(ctx: Context): void { if (result.aborted) throw new Error('command aborted') return [{ type: 'text', text: renderResult(result) }] }, + presentCall: presentBashCall, + presentResult: presentBashResult, })) ctx.tools.register(defineTool({ @@ -266,6 +308,7 @@ export function apply(ctx: Context): void { text += `\n${statusLine(read.task)}` return Promise.resolve([{ type: 'text', text }]) }, + presentCall: args => presentTaskCall('Read output from', args), })) ctx.tools.register(defineTool({ @@ -283,5 +326,6 @@ export function apply(ctx: Context): void { text: killed ? `killed background task ${id}` : `task ${id} had already finished`, }]) }, + presentCall: args => presentTaskCall('Kill', args), })) } diff --git a/packages/tool-bash/tests/tools.spec.ts b/packages/tool-bash/tests/tools.spec.ts index cc25d9d9d4..8b68de213b 100644 --- a/packages/tool-bash/tests/tools.spec.ts +++ b/packages/tool-bash/tests/tools.spec.ts @@ -562,3 +562,58 @@ describe('status lines', () => { expect(text(read)).toContain('[status: completed, exit code: 0]') }) }) + +describe('tool-owned UI presentation (presentCall / presentResult)', () => { + it('bash presentCall: the model description is the title, the command is the rawInput, kind execute', async () => { + const ctx = await setup() + const present = ctx.tools.get('bash')!.presentCall!({ command: 'ls -la src', description: 'List files in src' }) + expect(present).toEqual({ title: 'List files in src', kind: 'execute', rawInput: 'ls -la src' }) + }) + + it('bash presentResult: wraps the model-facing text in a fenced console block', async () => { + const ctx = await setup() + const present = ctx.tools.get('bash')!.presentResult!( + { command: 'echo hi', description: 'echo' }, + { content: [{ type: 'text', text: 'hi\n[exit code: 0]\n\n' }], isError: false }, + ) + // Trailing blank lines are trimmed; the body is fenced as ```console. + expect(present).toEqual({ content: [{ type: 'text', text: '```console\nhi\n[exit code: 0]\n```' }] }) + }) + + it('bash presentResult: leaves a non-text (unexpected) result untouched → undefined (UI keeps raw content)', async () => { + const ctx = await setup() + const present = ctx.tools.get('bash')!.presentResult!( + { command: 'x', description: 'x' }, + { content: [{ type: 'image', url: 'https://x/y.png' }], isError: false }, + ) + expect(present).toBeUndefined() + }) + + it('bash presentResult: a result that is not exactly one block → undefined (no single text to fence)', async () => { + const ctx = await setup() + const args = { command: 'x', description: 'x' } + // Empty content (no block) and multi-block content both fall through. + expect(ctx.tools.get('bash')!.presentResult!(args, { content: [], isError: false })).toBeUndefined() + expect(ctx.tools.get('bash')!.presentResult!(args, { + content: [{ type: 'text', text: 'a' }, { type: 'text', text: 'b' }], + isError: false, + })).toBeUndefined() + }) + + 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' }) + expect(ctx.tools.get('bash_kill')!.presentCall!({ task_id: 'bash-3' })) + .toEqual({ 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 () => { + const ctx = await setup() + // defineTool wraps presentCall to soft-validate against the schema and fall + // back to undefined (a generic UI presentation) rather than throwing on the + // display path — it may run on replay of arbitrary logged args. The + // ToolDefinition.presentCall takes `unknown`, so a malformed shape needs no cast. + expect(ctx.tools.get('bash')?.presentCall?.({ command: 'ls' })).toBeUndefined() + }) +}) diff --git a/packages/tools/README.md b/packages/tools/README.md index 560195387d..d33e28f1e9 100644 --- a/packages/tools/README.md +++ b/packages/tools/README.md @@ -24,9 +24,10 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e ### Key types -- `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise`. +- `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"). ### Extension points @@ -67,6 +68,39 @@ A `defineTool` tool also **validates the model-generated arguments against its ` See `defineTool`, `validateArgs`, `ToolArgsError`, `SchemaSpec`, `InferArgs`, and `schemaSpecToJsonSchema` in the public API for details. +### 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: + +- `presentCall(args): ToolCallPresentation | undefined` — the PENDING state: a human-readable `title` (always-visible label), an optional `kind` (`read`/`edit`/`execute`/… for icon/treatment, default `other`), and an optional `rawInput` (the salient input to show in a detail view — e.g. a shell command as a string, NOT the whole args object). +- `presentResult(args, result): ToolResultPresentation | undefined` — the COMPLETED state, given the same `args` and the `{ content, isError }` result: an optional replacement `title` and reformatted `content` (e.g. wrap command output in a fenced ` ```console ` block — a UI-only affordance that must NOT appear in the model-facing `execute` result). + +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. + +```ts +import { defineTool } from '@deepseek-ai/dsh-tools' + +const bash = defineTool({ + name: 'bash', + description: 'Run a shell command.', + parameters: { + command: { type: 'string', required: true, description: 'The command to run.' }, + description: { type: 'string', required: true, description: 'One-line summary shown in the UI.' }, + }, + async execute(args) { + return [{ type: 'text', text: `ran: ${args.command}` }] + }, + // The model-written description is the readable title; the command is the detail. + presentCall: args => ({ title: args.description, kind: 'execute', rawInput: args.command }), + // Wrap the output as a console block for the UI (not in the model-facing result). + presentResult: (_args, result) => { + const block = result.content.length === 1 ? result.content[0] : undefined + if (block === undefined || block.type !== 'text') return undefined + return { content: [{ type: 'text', text: '```console\n' + block.text + '\n```' }] } + }, +}) +``` + ### What is NOT here (TODO) - **Tool shapes review** — when real tools land (e.g. a concurrency-safety hint for parallel execution); phase 1 executes tool calls sequentially. diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index 8e8c106147..9e1e7b74a9 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -50,9 +50,87 @@ declare module 'cordis' { // parallel execution — Claude Code partitions read-only tools; phase 1 // executes sequentially). +/** + * Category of a tool call, used by a UI to pick an icon / treatment. A neutral + * vocabulary owned here (NOT an ACP type) so tools describe themselves without + * depending on any client protocol; a UI bridge maps it to its own enum. The + * member set mirrors the common ACP `ToolKind` values; `other` is the default. + */ +export type ToolCallKind = 'read' | 'edit' | 'delete' | 'move' | 'search' | 'execute' | 'fetch' | 'other' + +/** + * 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. + */ +export interface ToolCallPresentation { + /** + * 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). + */ + 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. + */ + rawInput?: unknown +} + +/** + * 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. + */ +export interface ToolResultPresentation { + /** Replacement title for the completed call (e.g. append an exit status). 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[] +} + /** 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`. + */ + presentCall?(args: unknown): ToolCallPresentation | 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. + */ + presentResult?(args: unknown, result: ToolResult): ToolResultPresentation | undefined +} + +/** The completed outcome handed to {@link ToolDefinition.presentResult}. */ +export interface ToolResult { + /** The model-facing content `execute` returned (or the error text on failure). */ + content: ContentBlock[] + /** Whether the call failed. */ + isError: boolean } /** One pending tool call, as it flows through the execution waterfall. */ diff --git a/packages/tools/src/schema.ts b/packages/tools/src/schema.ts index 5572e43ff3..5e8887f11b 100644 --- a/packages/tools/src/schema.ts +++ b/packages/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 { ToolDefinition, ToolExecution } from './index.ts' +import type { ToolCallPresentation, ToolDefinition, ToolExecution, ToolResult, ToolResultPresentation } from './index.ts' // --------------------------------------------------------------------------- // SchemaSpec — the author-facing per-property type @@ -287,6 +287,22 @@ export interface DefineToolOptions { * casts needed. */ execute(args: InferArgs, exec: ToolExecution): Promise + /** + * Optional: how to present the PENDING state of one call in a UI (an editor + * tool-call card, a CLI log line). `args` is the typed, schema-validated + * 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}. + */ + presentCall?(args: InferArgs): ToolCallPresentation | 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}. + */ + presentResult?(args: InferArgs, result: ToolResult): ToolResultPresentation | undefined /** Whether the tool requires structured output (default false). */ strict?: boolean } @@ -322,7 +338,11 @@ export function defineTool(options: DefineToolOptions): // Object-literal execute methods don't use `this`; the reference is safe. // eslint-disable-next-line @typescript-eslint/unbound-method const userExecute = options.execute - return { + // eslint-disable-next-line @typescript-eslint/unbound-method + const userPresentCall = options.presentCall + // eslint-disable-next-line @typescript-eslint/unbound-method + const userPresentResult = options.presentResult + const tool: ToolDefinition = { name: options.name, description: options.description, parameters: schemaSpecToJsonSchema(options.parameters) as unknown as Record, @@ -337,4 +357,21 @@ export function defineTool(options: DefineToolOptions): return userExecute(args as InferArgs, exec) }, } + // Presentation is display-only and may run on REPLAY of arbitrary logged args + // (possibly from an older schema), so it must never throw: validate softly and + // 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 => { + if (validateArgs(options.parameters, args).length > 0) return undefined + return userPresentCall(args as InferArgs) + } + } + if (userPresentResult) { + tool.presentResult = (args: unknown, result: ToolResult): ToolResultPresentation | undefined => { + if (validateArgs(options.parameters, args).length > 0) return undefined + return userPresentResult(args as InferArgs, result) + } + } + return tool } diff --git a/packages/tools/tests/tools.spec.ts b/packages/tools/tests/tools.spec.ts index fb42df4492..e42e7b4387 100644 --- a/packages/tools/tests/tools.spec.ts +++ b/packages/tools/tests/tools.spec.ts @@ -814,3 +814,54 @@ describe('defineTool validation (the runtime-validation RFC, part 1)', () => { expect(result.isError).toBe(false) }) }) + +describe('defineTool presentation (presentCall / presentResult)', () => { + it('threads presentCall/presentResult onto the ToolDefinition with typed args', () => { + const tool = defineTool({ + name: 'demo', + description: 'demo', + parameters: { path: { type: 'string', required: true }, n: { type: 'number' } }, + async execute() { return [{ type: 'text', text: 'ok' }] }, + 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 } + }, + presentResult(args, result) { + return { title: `Opened ${args.path}`, content: result.content } + }, + }) + expect(tool.presentCall!({ path: '/a', n: 2 })).toEqual({ 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' }] }) + }) + + it('a tool without presentCall/presentResult leaves them undefined (UI falls back generically)', () => { + const tool = defineTool({ + name: 'plain', + description: 'plain', + parameters: { x: { type: 'string', required: true } }, + async execute() { return [] }, + }) + expect(typeof tool.presentCall).toBe('undefined') + expect(typeof tool.presentResult).toBe('undefined') + }) + + it('presentCall/presentResult validate softly: malformed args return undefined, never throw (display runs on replay)', () => { + const tool = defineTool({ + name: 'demo', + 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 }), + }) + // Unlike execute (which throws ToolArgsError on a mismatch), the display + // methods soft-validate and fall back to undefined so a UI never crashes + // replaying an old/foreign log entry. The ToolDefinition methods take + // `unknown`, so malformed shapes pass without a cast. + expect(tool.presentCall?.({})).toBeUndefined() + expect(tool.presentResult?.({ wrong: 1 }, { content: [], isError: false })).toBeUndefined() + }) +}) +