feat(acp): show the command in execute titles; test via the real bash tool; RFC for terminal rendering
- bash presentCall title is now "description — command" (e.g. "List files in
src — ls -la src"). An execute-kind ACP card HIDES rawInput (Zed renders it
only for non-terminal tools), so the command must ride in the always-visible
title to be seen — matching how claude-agent-acp/codex-acp title execute
tools. The command stays in rawInput too for non-execute UIs that show it.
- Rework the acp tool-call presentation tests (turns + load replay) to drive the
REAL dsh-tool-bash + dsh-bash-local via a new makeBridgeHarness({ withBash })
option, running an actual `echo` — instead of an inline fake bash tool. The
mock MODEL still scripts the call (deterministic, no key), but the tool and
executor are real, so the test verifies the shipping presentCall/presentResult.
- AGENTS.md: add the principle "prefer the REAL implementation over a mock/
stand-in in tests" (mock only the expensive/non-deterministic boundary).
- RFC (proposed): the ACP terminal sub-protocol + command classification — the
capability-gated rich rendering (live cwd-header terminal card, classify a
`cat` as a read / `grep` as a search) that the reference adapters do; the
fenced ```console text block stays the no-capability baseline. Studied
codex-acp, claude-agent-acp, and Zed's renderer to ground it.
This commit is contained in:
@@ -18,6 +18,8 @@ import ToolRegistry from '@deepseek-ai/dsh-tools'
|
||||
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 * as ToolBash from '@deepseek-ai/dsh-tool-bash'
|
||||
import {
|
||||
ClientSideConnection,
|
||||
ndJsonStream,
|
||||
@@ -148,6 +150,14 @@ export async function makeBridgeHarness(options: {
|
||||
script?: (StreamChunk[] | 'hang')[]
|
||||
config?: Partial<AcpConfig>
|
||||
storageDir: string
|
||||
/**
|
||||
* Plug the REAL `dsh-bash-local` executor + `dsh-tool-bash` tools (instead of
|
||||
* a test's own inline tool). Lets a test drive the actual `bash` tool — its
|
||||
* real `presentCall`/`presentResult` — through the bridge, so tool-call UI
|
||||
* tests verify the SHIPPING tool, not a stand-in (AGENTS.md "prefer the real
|
||||
* implementation over a mock in tests").
|
||||
*/
|
||||
withBash?: boolean
|
||||
} = { storageDir: '' }): Promise<BridgeHarness> {
|
||||
const adapter = new MockAdapter(options.script ?? [])
|
||||
|
||||
@@ -159,6 +169,10 @@ export async function makeBridgeHarness(options: {
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(AgentLoop, { agents: [] })
|
||||
await ctx.plugin(SessionPersistenceJsonl, { root: options.storageDir })
|
||||
if (options.withBash) {
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
await ctx.plugin(ToolBash)
|
||||
}
|
||||
ctx.llm.registerAdapter(['mock'], adapter)
|
||||
|
||||
// Two identity byte pipes cross-wired into the two ndJsonStreams: bytes the
|
||||
|
||||
@@ -4,7 +4,6 @@ import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { PROTOCOL_VERSION } from '@agentclientprotocol/sdk'
|
||||
import { SessionId } from '@deepseek-ai/dsh-session'
|
||||
import { defineTool } from '@deepseek-ai/dsh-tools'
|
||||
import { makeBridgeHarness, textResponse, toolCallResponse, type BridgeHarness, type CapturedUpdate } from './harness.ts'
|
||||
|
||||
/** Concatenate the text of all agent_message_chunk updates. */
|
||||
@@ -58,64 +57,37 @@ describe('acp bridge — session/load replay', () => {
|
||||
})
|
||||
|
||||
it('replays a persisted tool call with the TOOL-OWNED presentation (title/rawInput/console output)', async () => {
|
||||
// A turn with a tool call is persisted, then loaded by a fresh bridge. The
|
||||
// replayed tool_call/tool_call_update must carry the tool's OWN presentation
|
||||
// (presentCall/presentResult) — identical to how they streamed live — using
|
||||
// a throwaway presenter that pairs call→result as the log replays in order.
|
||||
// A turn with a REAL bash tool call is persisted, then loaded by a fresh
|
||||
// bridge. The replayed tool_call/tool_call_update must carry the tool's OWN
|
||||
// presentation — identical to how it streamed live — via a throwaway
|
||||
// presenter that pairs call→result as the log replays in order. Uses the
|
||||
// shipping tool (withBash), not a stand-in (AGENTS.md "prefer the real
|
||||
// implementation over a mock in tests").
|
||||
live = await makeBridgeHarness({
|
||||
storageDir,
|
||||
script: [toolCallResponse('c1', 'bash', { command: 'ls -la', description: 'List files' }), textResponse('done')],
|
||||
withBash: true,
|
||||
script: [toolCallResponse('c1', 'bash', { command: 'echo hello', description: 'Print a greeting' }), textResponse('done')],
|
||||
})
|
||||
live.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\`\`\`` }] }
|
||||
},
|
||||
}))
|
||||
await live.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} })
|
||||
const { sessionId } = await live.client.newSession({ cwd: process.cwd(), mcpServers: [] })
|
||||
await live.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'list' }] })
|
||||
await live.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] })
|
||||
await live.dispose()
|
||||
live = undefined
|
||||
|
||||
// A fresh bridge — which must ALSO have the tool registered, since the
|
||||
// presentation is resolved from the live registry at replay time — loads it.
|
||||
loader = await makeBridgeHarness({ storageDir, script: [] })
|
||||
loader.ctx.tools.register(defineTool({
|
||||
name: 'bash',
|
||||
description: 'run a command',
|
||||
parameters: {
|
||||
command: { type: 'string', required: true },
|
||||
description: { type: 'string', required: true },
|
||||
},
|
||||
async execute() { return [] },
|
||||
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\`\`\`` }] }
|
||||
},
|
||||
}))
|
||||
// A fresh bridge — also with the real bash tool, since the presentation is
|
||||
// resolved from the live registry at replay time — loads the session.
|
||||
loader = await makeBridgeHarness({ storageDir, withBash: true, script: [] })
|
||||
await loader.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} })
|
||||
await loader.client.loadSession({ sessionId, cwd: process.cwd(), mcpServers: [] })
|
||||
|
||||
const call = loader.updates.find(u => u.sessionUpdate === 'tool_call')
|
||||
expect(call).toMatchObject({ toolCallId: 'c1', title: 'List files', kind: 'execute', rawInput: 'ls -la' })
|
||||
expect(call).toMatchObject({ toolCallId: 'c1', title: 'Print a greeting — echo hello', kind: 'execute', rawInput: 'echo hello' })
|
||||
const update = loader.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```' } }],
|
||||
})
|
||||
expect(update?.sessionUpdate).toBe('tool_call_update')
|
||||
if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update')
|
||||
expect(update).toMatchObject({ toolCallId: 'c1', status: 'completed' })
|
||||
const content = update.content as { content: { text: string } }[]
|
||||
expect(content[0]?.content.text).toBe('```console\nhello\n```')
|
||||
})
|
||||
|
||||
it('a load whose resume finishes after a client disconnect leaks no live session', async () => {
|
||||
|
||||
@@ -75,40 +75,41 @@ 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 () => {
|
||||
it('the REAL bash tool drives the tool-call UI end-to-end: description—command title + console output', async () => {
|
||||
// Use the SHIPPING tool (dsh-tool-bash + dsh-bash-local), not an inline
|
||||
// stand-in, so this verifies the actual presentCall/presentResult the editor
|
||||
// sees (AGENTS.md "prefer the real implementation over a mock in tests").
|
||||
// The mock MODEL still scripts the tool call (no real LLM needed), but the
|
||||
// tool and executor are real: a real `echo` runs and its real output flows
|
||||
// back through the bridge.
|
||||
harness = await makeBridgeHarness({
|
||||
storageDir,
|
||||
script: [toolCallResponse('c1', 'bash', { command: 'ls -la', description: 'List files' }), textResponse('done')],
|
||||
withBash: true,
|
||||
script: [
|
||||
toolCallResponse('c1', 'bash', { command: 'echo hello', description: 'Print a greeting' }),
|
||||
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' }] })
|
||||
await harness.client.prompt({ sessionId, prompt: [{ type: 'text', text: 'greet' }] })
|
||||
|
||||
// presentCall: execute kind, title is "description — command" (an execute
|
||||
// card hides rawInput, so the command rides in the title), command in rawInput.
|
||||
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({
|
||||
expect(call).toMatchObject({
|
||||
toolCallId: 'c1',
|
||||
status: 'completed',
|
||||
content: [{ type: 'content', content: { type: 'text', text: '```console\na.txt\nb.txt\n```' } }],
|
||||
title: 'Print a greeting — echo hello',
|
||||
kind: 'execute',
|
||||
rawInput: 'echo hello',
|
||||
status: 'in_progress',
|
||||
})
|
||||
// presentResult: the REAL command output, wrapped in a fenced console block.
|
||||
const update = harness.updates.find(u => u.sessionUpdate === 'tool_call_update')
|
||||
expect(update?.sessionUpdate).toBe('tool_call_update')
|
||||
if (update?.sessionUpdate !== 'tool_call_update') throw new Error('expected a tool_call_update')
|
||||
expect(update).toMatchObject({ toolCallId: 'c1', status: 'completed' })
|
||||
const content = update.content as { content: { type: string; text: string } }[]
|
||||
expect(content[0]?.content.text).toBe('```console\nhello\n```')
|
||||
})
|
||||
|
||||
it('a throwing tool presenter does not break the turn: the bridge falls back generically', async () => {
|
||||
|
||||
Reference in New Issue
Block a user