diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts index 6d388e9f89..c01c004d91 100644 --- a/packages/ui/acp/src/index.ts +++ b/packages/ui/acp/src/index.ts @@ -36,7 +36,7 @@ import type { Context } from 'cordis' import { Readable, Writable } from 'node:stream' import { randomUUID } from 'node:crypto' -import { isAbsolute, relative as relativePath, resolve as resolvePath } from 'node:path' +import { isAbsolute, relative as relativePath, resolve as resolvePath, sep as pathSep } from 'node:path' import Schema from 'schemastery' import { AgentSideConnection, @@ -1008,9 +1008,12 @@ type AcpToolCallContent = function displayTitle(title: string, rawPath: string | undefined, sessionCwd: string | undefined): string { if (rawPath === undefined || sessionCwd === undefined || !isAbsolute(rawPath) || !isAbsolute(sessionCwd)) return title const rel = relativePath(sessionCwd, rawPath) - // `relative` returns a `..`-prefixed path for a target outside the workspace; - // only relativize paths that stay inside it (and never to the empty string). - if (rel.length === 0 || rel.startsWith('..')) return title + // Only relativize a target that stays INSIDE the workspace. `relative` prefixes + // a `..` SEGMENT for a target above the cwd — test for the segment (`..` alone + // or `..…`), NOT a bare `..` char prefix, so a sibling like `..cache/x` + // (a real in-workspace name) still relativizes. Never relativize to the empty + // string (rawPath === cwd — a non-file target). + if (rel.length === 0 || rel === '..' || rel.startsWith(`..${pathSep}`)) return title return title.split(rawPath).join(rel) } @@ -1127,39 +1130,44 @@ function terminalExitMeta(callId: string, view: TerminalResultView): TerminalExi */ function toolResultUpdate(callId: CallId, view: ToolResultView, isError: boolean, terminal: TerminalRendering): ToolCallSessionUpdate { const status = isError ? 'failed' as const : 'completed' as const - if (view.card === 'terminal') { - const output = view.output ?? '' - if (terminal.enabled) { + switch (view.card) { + case 'terminal': { + const output = view.output ?? '' + if (terminal.enabled) { + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + ...view.title !== undefined ? { title: view.title } : {}, + _meta: { + terminal_output: { terminal_id: callId, data: output }, + ...terminalExitMeta(callId, view), + }, + } + } + // No terminal capability: the bridge derives the fenced ```console fallback. + const fenced = `\`\`\`console\n${output.replace(/\n+$/, '')}\n\`\`\`` return { sessionUpdate: 'tool_call_update', toolCallId: callId, status, + content: [{ type: 'content', content: { type: 'text', text: fenced } }], ...view.title !== undefined ? { title: view.title } : {}, - _meta: { - terminal_output: { terminal_id: callId, data: output }, - ...terminalExitMeta(callId, view), - }, } } - // No terminal capability: the bridge derives the fenced ```console fallback. - const fenced = `\`\`\`console\n${output.replace(/\n+$/, '')}\n\`\`\`` - return { - sessionUpdate: 'tool_call_update', - toolCallId: callId, - status, - content: [{ type: 'content', content: { type: 'text', text: fenced } }], - ...view.title !== undefined ? { title: view.title } : {}, - } - } - // The presenter fills a generic result's content from the raw result, so - // `content` is always defined here; the guard keeps this total for a - // directly-constructed view. - return { - sessionUpdate: 'tool_call_update', - toolCallId: callId, - status, - /* v8 ignore next -- content always defined via the presenter (see above) */ - ...view.content !== undefined ? { content: toolResultContent(view.content) } : {}, - ...view.title !== undefined ? { title: view.title } : {}, + case 'generic': + return { + sessionUpdate: 'tool_call_update', + toolCallId: callId, + status, + // The presenter fills a generic result's content from the raw result, so + // `content` is always defined here; the guard keeps this total for a + // directly-constructed view. + /* v8 ignore next -- content always defined via the presenter (see above) */ + ...view.content !== undefined ? { content: toolResultContent(view.content) } : {}, + ...view.title !== undefined ? { title: view.title } : {}, + } + default: + return assertNever(view, 'ToolResultView.card') } } diff --git a/packages/ui/acp/tests/stream-update.spec.ts b/packages/ui/acp/tests/stream-update.spec.ts index 85e21d19a2..9c1898b3c1 100644 --- a/packages/ui/acp/tests/stream-update.spec.ts +++ b/packages/ui/acp/tests/stream-update.spec.ts @@ -356,6 +356,26 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () => }))).toThrow('unreachable variant') }) + it('an unknown render-intent RESULT card throws via the exhaustiveness guard (closed union)', () => { + // The result-side renderer is also an exhaustive switch + assertNever: a rogue + // result card (only reachable by a cast) must throw, so adding a real result + // variant later fails to compile at the switch. + const rogue: ToolDefinition = { + name: 'rogue', + description: 'r', + parameters: {}, + execute: async () => [], + presentCall: () => ({ card: 'generic', title: 'r' }), + presentResult: () => ({ card: 'chart' }) as unknown as ReturnType>, + } + const presenter = new ToolPresenter(registryOf(rogue)) + expect(() => updatesWith( + presenter, + evt('tool/call', { turn: 1, step: 1, callId: CallId('c1'), name: 'rogue', arguments: '{}' }), + evt('tool/result', { turn: 1, step: 1, callId: CallId('c1'), content: [{ type: 'text', text: 'x' }], isError: false }), + )).toThrow('unreachable variant') + }) + it('forwards fs-tool render intents onto the wire (REAL read → generic locations, edit → diff content)', 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 — @@ -653,6 +673,17 @@ describe('relative-path display titles (bridge relativizes the title against the await ctx.fiber.dispose() }) + it('an in-workspace file whose relative form starts with `..` chars (a sibling name) still relativizes', async () => { + // `/work/proj/..cache/x` is INSIDE the workspace — its relative form + // `..cache/x` begins with the chars `..` but is NOT a parent segment. The + // guard tests for a `..` SEGMENT, so this relativizes (matching the reference + // adapter, which accepts any target under `cwd + sep`). + const ctx = await fsCtx() + const update = callUpdate(ctx, '/work/proj', 'read', { file_path: '/work/proj/..cache/x.ts' }) + expect((update as { title: string }).title).toBe('Read ..cache/x.ts') + await ctx.fiber.dispose() + }) + it('no session cwd → the absolute title is left unchanged', async () => { const ctx = await fsCtx() const update = callUpdate(ctx, undefined, 'read', { file_path: '/work/proj/src/a.ts' })