feat(acp): tool-owned tool-call UI presentation (title/command/output)
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.
This commit is contained in:
Tianyi Cui
@@ -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)
|
||||
|
||||
@@ -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<ToolRegistry, 'get'> {
|
||||
const map = new Map(tools.map(t => [t.name, t]))
|
||||
return { get: name => map.get(name) }
|
||||
}
|
||||
|
||||
function evt<T extends SessionEvent['type']>(type: T, data: Extract<SessionEvent, { type: T }>['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({})
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user