feat(acp): align bash terminal card with reference adapters (command title, description block, exit pill)
Match claude-agent-acp / codex-acp: the bash tool_call title IS the command (an execute card hides rawInput), the model description rides as a content text block above the card, and the completed card carries an exit-status pill via _meta.terminal_exit. Bridge fixes found in review of the prior terminal-card commit: - tool_call_update.content is OMITTED in terminal mode (an ACP update.content REPLACES the call's content collection in Zed, so the fenced ```console block would clobber the terminal content block). - terminal.output preserves RAW newlines (terminal renderers rely on exact bytes); only the fenced fallback trims trailing blank lines. - a relative workdir is resolved against the session cwd for the card header, matching where the command actually ran. - result-side terminal output is gated on the pending call having registered a terminal (no orphan _meta.terminal_output for a terminal Zed never made). The exit pill is recovered by parsing renderResult's status markers (the pure presentResult seam sees only content blocks); a round-trip test pins the parse to the marker emission. Neutral ToolTerminal gains exitCode/signal; widened ToolCallPresentation with a content block. Docs (RFC + 3 READMEs) updated; with-key e2e verifies the card + exit pill against the real model.
This commit is contained in:
Tianyi Cui
@@ -564,34 +564,74 @@ describe('status lines', () => {
|
||||
})
|
||||
|
||||
describe('tool-owned UI presentation (presentCall / presentResult)', () => {
|
||||
it('bash presentCall: title is "description — command", marks a terminal; explicit absolute workdir → cwd header', async () => {
|
||||
it('bash presentCall: title is the command, description as a content block, marks a terminal; workdir → cwd (absolute or relative, bridge resolves)', async () => {
|
||||
const ctx = await setup()
|
||||
// No explicit workdir → the call still flags a terminal, but with no cwd (the
|
||||
// UI bridge fills the session cwd it owns; the pure presenter can't see it).
|
||||
// The command is the title (an execute card hides rawInput); the description
|
||||
// rides as a content text block (shown above the terminal card).
|
||||
expect(ctx.tools.get('bash')?.presentCall?.({ command: 'ls -la src', description: 'List files in src' }))
|
||||
.toEqual({ title: 'List files in src — ls -la src', kind: 'execute', rawInput: 'ls -la src', terminal: {} })
|
||||
// An explicit ABSOLUTE workdir is surfaced as the terminal cwd header.
|
||||
.toEqual({ title: 'ls -la src', kind: 'execute', rawInput: 'ls -la src', content: [{ type: 'text', text: 'List files in src' }], terminal: {} })
|
||||
// An ABSOLUTE workdir is surfaced verbatim as the terminal cwd header.
|
||||
expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: '/tmp/x' }))
|
||||
.toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: { cwd: '/tmp/x' } })
|
||||
// A RELATIVE workdir is not an absolute cwd → omitted (terminal still flagged).
|
||||
.toEqual({ title: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: '/tmp/x' } })
|
||||
// A RELATIVE workdir is passed through AS-IS (the bridge resolves it against
|
||||
// the session cwd, matching where execution runs) — not dropped.
|
||||
expect(ctx.tools.get('bash')?.presentCall?.({ command: 'pwd', description: 'Print dir', workdir: 'sub' }))
|
||||
.toEqual({ title: 'Print dir — pwd', kind: 'execute', rawInput: 'pwd', terminal: {} })
|
||||
.toEqual({ title: 'pwd', kind: 'execute', rawInput: 'pwd', content: [{ type: 'text', text: 'Print dir' }], terminal: { cwd: 'sub' } })
|
||||
})
|
||||
|
||||
it('bash presentResult: console-block content AND terminal.output (both renderings of the run)', async () => {
|
||||
it('bash presentResult: console-block content AND terminal.output (RAW newlines) + parsed exit code', async () => {
|
||||
const ctx = await setup()
|
||||
const present = ctx.tools.get('bash')!.presentResult!(
|
||||
{ command: 'echo hi', description: 'echo' },
|
||||
{ content: [{ type: 'text', text: 'hi\n[exit code: 0]\n\n' }], isError: false },
|
||||
)
|
||||
// Trailing blank lines trimmed; content is the fenced ```console fallback,
|
||||
// terminal.output is the same text for a capable terminal card.
|
||||
// The fenced ```console content trims trailing blank lines for a tidy block;
|
||||
// terminal.output keeps the RAW bytes (newlines intact) a terminal renderer
|
||||
// needs; exitCode is parsed back from the [exit code: N] marker.
|
||||
expect(present).toEqual({
|
||||
content: [{ type: 'text', text: '```console\nhi\n[exit code: 0]\n```' }],
|
||||
terminal: { output: 'hi\n[exit code: 0]' },
|
||||
terminal: { output: 'hi\n[exit code: 0]\n\n', exitCode: 0 },
|
||||
})
|
||||
})
|
||||
|
||||
it('bash presentResult: a non-zero exit and a signal kill parse into exitCode / signal', async () => {
|
||||
const ctx = await setup()
|
||||
const args = { command: 'x', description: 'x' }
|
||||
const nonzero = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: 'oops\n[exit code: 3]' }], isError: false })
|
||||
expect(nonzero?.terminal).toEqual({ output: 'oops\n[exit code: 3]', exitCode: 3 })
|
||||
const killed = ctx.tools.get('bash')!.presentResult!(args, { content: [{ type: 'text', text: 'gone\n[killed by signal: SIGKILL]' }], isError: false })
|
||||
expect(killed?.terminal).toEqual({ output: 'gone\n[killed by signal: SIGKILL]', signal: 'SIGKILL' })
|
||||
})
|
||||
|
||||
it('bash presentResult exit parse is the inverse of renderResult markers (round-trip)', async () => {
|
||||
const ctx = await setup()
|
||||
const present = ctx.tools.get('bash')!
|
||||
// For each renderResult outcome, the rendered text fed back through
|
||||
// presentResult recovers the matching structured exit — the parse and the
|
||||
// marker emission co-evolve in one file, so this pins the pair.
|
||||
const base = {
|
||||
aborted: false,
|
||||
timeoutMs: 1000,
|
||||
stdout: { text: 'out', truncated: false },
|
||||
stderr: { text: '', truncated: false },
|
||||
}
|
||||
const cases = [
|
||||
{ result: { ...base, exitCode: 0, signal: null, timedOut: false }, expect: { exitCode: 0 } },
|
||||
{ result: { ...base, exitCode: 7, signal: null, timedOut: false }, expect: { exitCode: 7 } },
|
||||
{ result: { ...base, exitCode: null, signal: 'SIGTERM' as const, timedOut: false }, expect: { signal: 'SIGTERM' } },
|
||||
// A trapped-timeout run that exits 0 has no signal/exit marker → reads as exit 0 (it did exit 0).
|
||||
{ result: { ...base, exitCode: 0, signal: null, timedOut: true }, expect: { exitCode: 0 } },
|
||||
]
|
||||
for (const c of cases) {
|
||||
const rendered = renderResult(c.result)
|
||||
const out = present.presentResult!({ command: 'x', description: 'x' }, { content: [{ type: 'text', text: rendered }], isError: false })
|
||||
const { output: _o, ...exit } = out?.terminal ?? {}
|
||||
expect(exit).toEqual(c.expect)
|
||||
}
|
||||
})
|
||||
|
||||
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!(
|
||||
|
||||
Reference in New Issue
Block a user