From 2594b7b8e1bb744063497aeba18b9af4ca33a28d Mon Sep 17 00:00:00 2001 From: Jang Date: Fri, 21 Aug 2026 15:53:37 +0800 Subject: [PATCH] fix(acp-server): include full bash command in permission requests ACP session/request_permission for a Bash tool call used to show only the first 50 characters of args.command (followed by an ellipsis when longer), because displayBlockToAcpContent did not recognise the `command` ToolInputDisplay kind and silently dropped display.command. Add a `kind: 'command'` branch that projects block.command into a text content entry on the wire so clients see the full command in the approval card. The 50-character action preview is unchanged. Resolve #3106 --- .changeset/bash-full-command-in-permission.md | 5 + packages/acp-server/src/convert.ts | 9 +- packages/acp-server/test/approval.test.ts | 29 +++++ packages/acp-server/test/convert.test.ts | 106 ++++++++++++++++++ packages/acp-server/test/e2e-turn.test.ts | 47 ++++++++ 5 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 .changeset/bash-full-command-in-permission.md diff --git a/.changeset/bash-full-command-in-permission.md b/.changeset/bash-full-command-in-permission.md new file mode 100644 index 0000000000..3adf3aaf49 --- /dev/null +++ b/.changeset/bash-full-command-in-permission.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Show the full Bash command in tool permission requests instead of a 50-character preview. \ No newline at end of file diff --git a/packages/acp-server/src/convert.ts b/packages/acp-server/src/convert.ts index 308d400a0d..a6dccc242f 100644 --- a/packages/acp-server/src/convert.ts +++ b/packages/acp-server/src/convert.ts @@ -266,8 +266,10 @@ function parseLineRange(suffix: string): string | null { /** * Project a {@link ToolInputDisplay} block into an ACP {@link ToolCallContent} * entry for the tool-call card. Diff/file_io blocks become inline diffs; - * plan_review becomes a text content entry; everything else yields `null` - * (the caller drops it). + * plan_review becomes a text content entry; command blocks project the full + * shell command so approval cards surface more than the 50-char preview that + * the engine packs into `ApprovalRequest.action`; everything else yields + * `null` (the caller drops it). */ export function displayBlockToAcpContent(block: ToolInputDisplay): ToolCallContent | null { if (block.kind === 'diff') { @@ -291,6 +293,9 @@ export function displayBlockToAcpContent(block: ToolInputDisplay): ToolCallConte if (text === null) return null; return { type: 'content', content: { type: 'text', text } }; } + if (block.kind === 'command') { + return { type: 'content', content: { type: 'text', text: block.command } }; + } return null; } diff --git a/packages/acp-server/test/approval.test.ts b/packages/acp-server/test/approval.test.ts index c97ba23fe8..5e91b44d23 100644 --- a/packages/acp-server/test/approval.test.ts +++ b/packages/acp-server/test/approval.test.ts @@ -175,6 +175,35 @@ describe('buildPermissionToolCallUpdate', () => { content: { type: 'text', text: 'Requesting approval to run `echo hi`' }, }); }); + + it('prepends a text entry carrying the full command from a `command` display block', () => { + const longCommand = + 'echo "a longer command that crosses the 50-char threshold used for the action preview"'; + expect(longCommand.length).toBeGreaterThan(50); + const update = buildPermissionToolCallUpdate({ + toolName: 'Bash', + action: `Running: ${longCommand.slice(0, 50)}…`, + toolCallId: 'call_1', + turnId: 2, + display: { + kind: 'command', + command: longCommand, + cwd: '/tmp/example.test', + description: 'echo a long string', + language: 'bash', + } as unknown as ToolInputDisplay, + }); + const first = update.content?.[0]; + expect(first).toEqual({ + type: 'content', + content: { type: 'text', text: longCommand }, + }); + const last = update.content?.at(-1); + expect(last).toMatchObject({ + type: 'content', + content: { type: 'text', text: `Requesting approval to Running: ${longCommand.slice(0, 50)}…` }, + }); + }); }); describe('attachSelectedLabel', () => { diff --git a/packages/acp-server/test/convert.test.ts b/packages/acp-server/test/convert.test.ts index c5a1c490ca..9b87e742dc 100644 --- a/packages/acp-server/test/convert.test.ts +++ b/packages/acp-server/test/convert.test.ts @@ -4,12 +4,14 @@ import { join } from 'node:path'; import type { McpServer } from '@agentclientprotocol/sdk'; import type { ContentPart } from '@moonshot-ai/agent-core-v2'; +import type { ToolInputDisplay } from '@moonshot-ai/protocol'; import { afterEach, describe, expect, it } from 'vitest'; import { acpBlocksToContentParts, acpMcpServersToConfigRecord, compressPromptImageParts, + displayBlockToAcpContent, } from '../src/convert'; import { solidPng, solidPngBase64 } from './_helpers/png'; @@ -164,3 +166,107 @@ describe('compressPromptImageParts', () => { expect(await readFile(join(originalsDir, files[0]!))).toEqual(original); }); }); + +describe('displayBlockToAcpContent', () => { + it('renders a diff block as an inline diff entry', () => { + expect( + displayBlockToAcpContent({ + kind: 'diff', + path: 'example.ts', + before: 'old', + after: 'new', + }), + ).toEqual({ type: 'diff', path: 'example.ts', oldText: 'old', newText: 'new' }); + }); + + it('renders a file_io block with both sides as a diff entry', () => { + expect( + displayBlockToAcpContent({ + kind: 'file_io', + operation: 'edit', + path: 'example.ts', + before: 'old', + after: 'new', + }), + ).toEqual({ type: 'diff', path: 'example.ts', oldText: 'old', newText: 'new' }); + }); + + it('drops a file_io block when one side is missing', () => { + expect( + displayBlockToAcpContent({ + kind: 'file_io', + operation: 'write', + path: 'example.ts', + before: 'old', + }), + ).toBeNull(); + }); + + it('renders a plan_review block as a text content entry', () => { + expect( + displayBlockToAcpContent({ + kind: 'plan_review', + plan: 'do the thing', + }), + ).toEqual({ type: 'content', content: { type: 'text', text: 'do the thing' } }); + }); + + it('prefixes plan_review with its on-disk path when one is set', () => { + expect( + displayBlockToAcpContent({ + kind: 'plan_review', + plan: 'do the thing', + path: '/tmp/plan.md', + }), + ).toEqual({ + type: 'content', + content: { type: 'text', text: 'Plan saved to: /tmp/plan.md\n\ndo the thing' }, + }); + }); + + it('drops an empty plan_review', () => { + expect( + displayBlockToAcpContent({ + kind: 'plan_review', + plan: ' ', + }), + ).toBeNull(); + }); + + it('projects a command block as a text content entry carrying the full command', () => { + expect( + displayBlockToAcpContent({ + kind: 'command', + command: 'echo example.com && ls -la /tmp/example.test', + }), + ).toEqual({ + type: 'content', + content: { + type: 'text', + text: 'echo example.com && ls -la /tmp/example.test', + }, + }); + }); + + it('preserves the full command even when it exceeds the 50-char action preview', () => { + const longCommand = + 'echo "long command that is well past the fifty character preview cap used elsewhere"'; + expect(longCommand.length).toBeGreaterThan(50); + const entry = displayBlockToAcpContent({ + kind: 'command', + command: longCommand, + cwd: '/tmp/example.test', + description: 'echo a string', + language: 'bash', + }); + expect(entry).toEqual({ + type: 'content', + content: { type: 'text', text: longCommand }, + }); + }); + + it('returns null for display kinds that have no projection', () => { + const generic: ToolInputDisplay = { kind: 'generic', summary: 'noop' }; + expect(displayBlockToAcpContent(generic)).toBeNull(); + }); +}); diff --git a/packages/acp-server/test/e2e-turn.test.ts b/packages/acp-server/test/e2e-turn.test.ts index 5aea540424..a179dcaa40 100644 --- a/packages/acp-server/test/e2e-turn.test.ts +++ b/packages/acp-server/test/e2e-turn.test.ts @@ -165,6 +165,53 @@ describe('acp-server real prompt turn (scripted LLM)', () => { expect(JSON.stringify(scripted!.callHistory()[1])).toContain('hello_from_bash'); }, 30_000); + + it('ships the full Bash command to the client in the permission request', async () => { + // Runs against a client that does NOT advertise the terminal capability + // (see below) so the approval goes through the request_permission bridge + // instead of being routed to a client-side terminal. That is the path + // where the 50-char preview used to be the only command text on the wire. + const c = await boot({ terminal: false }); + const longCommand = + 'echo "a longer command that crosses the 50-char threshold used for the action preview"'; + expect(longCommand.length).toBeGreaterThan(50); + scripted!.mockNextResponse({ + type: 'function', + id: 'call_long', + name: 'Bash', + arguments: JSON.stringify({ command: longCommand }), + }); + scripted!.mockNextText('done'); + + const permissionRequests: Array<{ + toolCall?: { title?: string; content?: Array<{ content?: { text?: string } }> }; + }> = []; + c.onRequest('session/request_permission', (params) => { + permissionRequests.push(params as (typeof permissionRequests)[number]); + return { outcome: { outcome: 'selected', optionId: 'approve_once' } }; + }); + + const created = (await c.send('session/new', { cwd: homeDir, mcpServers: [] })) as { + sessionId: string; + }; + await c.waitForSessionUpdate('available_commands_update', 10_000); + await c.send('session/prompt', { + sessionId: created.sessionId, + prompt: [{ type: 'text', text: 'run a long command' }], + }); + + expect(permissionRequests).toHaveLength(1); + const toolCall = permissionRequests[0]!.toolCall!; + expect(toolCall.title).toBe('Bash'); + // The first content entry now carries the full command (a text content + // entry whose text is block.command); the trailing summary still uses the + // 50-char preview. Clients read content[0] to display the command. + const textContents = (toolCall.content ?? []) + .map((c) => c.content?.text) + .filter((t): t is string => typeof t === 'string'); + expect(textContents).toContain(longCommand); + expect(textContents.some((t) => t.includes('Requesting approval to'))).toBe(true); + }, 30_000); it('bridges AskUserQuestion through elicitation/create for form-capable clients', async () => { const c = await boot({ elicitation: { form: {} } }); // First model response: an AskUserQuestion tool call with a single-select