diff --git a/.changeset/acp-approval-command-display.md b/.changeset/acp-approval-command-display.md new file mode 100644 index 000000000..7d6ec7f76 --- /dev/null +++ b/.changeset/acp-approval-command-display.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Include concrete shell commands in ACP permission requests so clients can display and policy-match approvals. diff --git a/packages/acp-adapter/src/approval.ts b/packages/acp-adapter/src/approval.ts index 9d6b9c28b..b29a1ebcf 100644 --- a/packages/acp-adapter/src/approval.ts +++ b/packages/acp-adapter/src/approval.ts @@ -224,6 +224,37 @@ function mapPlanReviewOptionId( return { decision: 'rejected' }; } +type CommandDisplay = Extract; + +interface CommandRawInput { + command: string; + cwd?: string; + description?: string; + language?: 'bash'; +} + +function commandDisplayToAcpContent(display: CommandDisplay): ToolCallContent { + return { + type: 'content', + content: { type: 'text', text: display.command }, + }; +} + +function commandDisplayToRawInput(display: CommandDisplay): CommandRawInput { + const rawInput: CommandRawInput = { command: display.command }; + if (display.cwd !== undefined) rawInput.cwd = display.cwd; + if (display.description !== undefined) rawInput.description = display.description; + if (display.language !== undefined) rawInput.language = display.language; + return rawInput; +} + +function permissionTitle(req: ApprovalRequest): string { + if (req.display.kind === 'command' && req.display.command.length > 0) { + return req.display.command; + } + return req.toolName; +} + /** * Build the ACP {@link ToolCallUpdate} that scopes a permission request * to a specific in-flight tool call. @@ -241,9 +272,10 @@ function mapPlanReviewOptionId( * - If `req.display` produces a diff-bearing entry via * {@link displayBlockToAcpContent} (diff kind, or file_io with * before+after), prepend it so the diff card is the headline of - * the approval prompt. Non-diff display kinds (command, search, …) - * contribute no structured content here — their information is - * already conveyed by the action text below. + * the approval prompt. + * - If `req.display.kind === 'command'`, prepend the concrete shell + * command so ACP clients can display or policy-match the command + * being authorized, not just the Bash tool name. * - Phase 13.2 adds a `plan_review` entry so the full plan markdown * (and the optional `Plan saved to:` path prefix) lands at the top * of the approval card — the previous Phase-5 fallback truncated @@ -260,24 +292,28 @@ export function buildPermissionToolCallUpdate( const toolCallId = turnId !== undefined ? acpToolCallId(turnId, req.toolCallId) : req.toolCallId; const content: ToolCallContent[] = []; - // Diff entry first — diffs and file-io previews carry the most - // context and should land at the top of the approval card. Phase 13.2 - // adds plan_review to the same path so plan markdown surfaces in the - // headline too. + const headlineEntry = displayBlockToAcpContent(req.display); if (headlineEntry !== null) { content.push(headlineEntry); + } else if (req.display.kind === 'command') { + content.push(commandDisplayToAcpContent(req.display)); } // Always include the action summary so the prompt is never empty. content.push({ type: 'content', content: { type: 'text', text: `Requesting approval to ${req.action}` }, }); - return { + const update: ToolCallUpdate = { toolCallId, - title: req.toolName, + title: permissionTitle(req), content, }; + if (req.display.kind === 'command') { + update.kind = 'execute'; + update.rawInput = commandDisplayToRawInput(req.display); + } + return update; } /** diff --git a/packages/acp-adapter/test/approval-display.test.ts b/packages/acp-adapter/test/approval-display.test.ts index bbedb07e5..20e8ec203 100644 --- a/packages/acp-adapter/test/approval-display.test.ts +++ b/packages/acp-adapter/test/approval-display.test.ts @@ -171,19 +171,26 @@ describe('buildPermissionToolCallUpdate (Phase 5.2 content shape)', () => { }); }); - it('emits only the action summary for non-diff display kinds (e.g. command)', () => { + it('includes the concrete command before the action summary', () => { const update = buildPermissionToolCallUpdate( 5, { toolCallId: 'tc-cmd', toolName: 'Bash', action: 'run shell command', - display: { kind: 'command', command: 'ls -la' }, + display: { kind: 'command', command: 'ls -la', cwd: '/tmp/project' }, }, ); - expect(update.content).toHaveLength(1); - const [only] = update.content as [ToolCallContent]; - expect(only).toEqual({ + expect(update.title).toBe('ls -la'); + expect(update.kind).toBe('execute'); + expect(update.rawInput).toEqual({ command: 'ls -la', cwd: '/tmp/project' }); + expect(update.content).toHaveLength(2); + const [command, action] = update.content as [ToolCallContent, ToolCallContent]; + expect(command).toEqual({ + type: 'content', + content: { type: 'text', text: 'ls -la' }, + }); + expect(action).toEqual({ type: 'content', content: { type: 'text', text: 'Requesting approval to run shell command' }, }); diff --git a/packages/acp-adapter/test/approval.test.ts b/packages/acp-adapter/test/approval.test.ts index 19c7bd6be..7ebcbeec9 100644 --- a/packages/acp-adapter/test/approval.test.ts +++ b/packages/acp-adapter/test/approval.test.ts @@ -9,6 +9,7 @@ import { type RequestPermissionRequest, type RequestPermissionResponse, type SessionNotification, + type ToolCallContent, type WriteTextFileRequest, type WriteTextFileResponse, } from '@agentclientprotocol/sdk'; @@ -217,13 +218,15 @@ describe('buildPermissionToolCallUpdate (Phase 5.1 minimal shape)', () => { it('prefixes the toolCallId with the turnId when one is known', () => { const update = buildPermissionToolCallUpdate(42, baseReq); expect(update.toolCallId).toBe('42:abc'); - expect(update.title).toBe('Bash'); + expect(update.title).toBe('ls -la'); + expect(update.kind).toBe('execute'); + expect(update.rawInput).toEqual({ command: 'ls -la' }); }); it('falls back to the raw SDK toolCallId when no turnId is tracked yet', () => { const update = buildPermissionToolCallUpdate(undefined, baseReq); expect(update.toolCallId).toBe('abc'); - expect(update.title).toBe('Bash'); + expect(update.title).toBe('ls -la'); }); }); @@ -296,7 +299,22 @@ describe('AcpSession ↔ requestPermission bridge (end-to-end via wire)', () => REJECT_OPTION_ID, ]); expect(req.toolCall.toolCallId).toBe(`${turnId}:tc-1`); - expect(req.toolCall.title).toBe('Bash'); + expect(req.toolCall.title).toBe('echo hi'); + expect(req.toolCall.kind).toBe('execute'); + expect(req.toolCall.rawInput).toEqual({ command: 'echo hi' }); + expect(req.toolCall.content).toHaveLength(2); + const [command, action] = req.toolCall.content as [ + ToolCallContent, + ToolCallContent, + ]; + expect(command).toEqual({ + type: 'content', + content: { type: 'text', text: 'echo hi' }, + }); + expect(action).toEqual({ + type: 'content', + content: { type: 'text', text: 'Requesting approval to run command' }, + }); // Settle the parked prompt with a turn.ended so the test exits // cleanly.