From 36a7fa089c1e883a3294e377af1d844373b82695 Mon Sep 17 00:00:00 2001 From: Coco Sheng Date: Mon, 11 May 2026 13:45:58 -0400 Subject: [PATCH] fix(cli): use static tool name in confirmation prompt to avoid parsing errors (#26866) --- .../components/ToolConfirmationQueue.test.tsx | 2 +- ...security-warning-height-correctly.snap.svg | 4 +- ...d-content-for-large-exec-commands.snap.svg | 4 +- .../ToolConfirmationQueue.test.tsx.snap | 6 +- .../messages/ToolConfirmationMessage.test.tsx | 102 ++++++++++++++++++ .../messages/ToolConfirmationMessage.tsx | 17 +-- .../src/ui/components/messages/ToolShared.tsx | 4 +- .../RedirectionConfirmation.test.tsx.snap | 2 +- ...le-height-for-large-exec-commands.snap.svg | 4 +- ...-newlines-and-syntax-highlighting.snap.svg | 4 +- .../ToolConfirmationMessage.test.tsx.snap | 10 +- 11 files changed, 128 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx index 853c9c577b..fbfbb81daa 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx @@ -141,7 +141,7 @@ describe('ToolConfirmationQueue', () => { expect(output).toContain('1 of 3'); expect(output).toContain('ls'); // Tool name expect(output).toContain('list files'); // Tool description - expect(output).toContain('Allow execution of [ls]?'); + expect(output).toContain('Allow execution of [Shell]?'); expect(output).toMatchSnapshot(); unmount(); diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-handle-security-warning-height-correctly.snap.svg b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-handle-security-warning-height-correctly.snap.svg index 8e57fe107e..b3f4741541 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-handle-security-warning-height-correctly.snap.svg +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-handle-security-warning-height-correctly.snap.svg @@ -84,8 +84,8 @@ │ │ Allow execution of - [echo] - ? + [Shell] + ? │ │ │ diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-render-the-full-queue-wrapper-with-borders-and-content-for-large-exec-commands.snap.svg b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-render-the-full-queue-wrapper-with-borders-and-content-for-large-exec-commands.snap.svg index 3f2d8451a8..92578029de 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-render-the-full-queue-wrapper-with-borders-and-content-for-large-exec-commands.snap.svg +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue-ToolConfirmationQueue-height-allocation-and-layout-should-render-the-full-queue-wrapper-with-borders-and-content-for-large-exec-commands.snap.svg @@ -191,8 +191,8 @@ │ │ Allow execution of - [echo] - ? + [Shell] + ? │ │ │ diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index 238efefba4..5bc3980fce 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -52,7 +52,7 @@ exports[`ToolConfirmationQueue > height allocation and layout > should handle se │ Original: https://täst.com/ │ │ Actual Host (Punycode): https://xn--tst-qla.com/ │ │ │ -│ Allow execution of [echo]? │ +│ Allow execution of [Shell]? │ │ │ │ ● 1. Allow once │ │ 2. Allow for this session │ @@ -138,7 +138,7 @@ exports[`ToolConfirmationQueue > height allocation and layout > should render th │ │ echo "Line 49" │ │ │ │ echo "Line 50" │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ │ -│ Allow execution of [echo]? │ +│ Allow execution of [Shell]? │ │ │ │ ● 1. Allow once │ │ 2. Allow for this session │ @@ -202,7 +202,7 @@ exports[`ToolConfirmationQueue > renders the confirming tool with progress indic │ ╭──────────────────────────────────────────────────────────────────────────╮ │ │ │ ls │ │ │ ╰──────────────────────────────────────────────────────────────────────────╯ │ -│ Allow execution of [ls]? │ +│ Allow execution of [Shell]? │ │ │ │ ● 1. Allow once │ │ 2. Allow for this session │ diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 3a3a4df557..93e8e5d735 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -275,6 +275,108 @@ describe('ToolConfirmationMessage', () => { result.unmount(); }); + it('should use the tool name for display in the confirmation question', async () => { + const confirmationDetails: SerializableConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command: '# This is a comment\necho "hello"', + rootCommand: 'echo', + rootCommands: ['echo'], + }; + + const { lastFrame, unmount } = await renderWithProviders( + , + ); + + const output = lastFrame(); + expect(output).toContain('Allow execution of [Shell]?'); + unmount(); + }); + + describe('tool name humanization', () => { + const cases = [ + { + toolName: 'run_shell_command', + expected: 'Allow execution of [Shell]?', + desc: 'humanize run_shell_command to Shell', + }, + { + toolName: 'shell', + expected: 'Allow execution of [Shell]?', + desc: 'humanize shell to Shell', + }, + { + toolName: 'grep_search', + expected: 'Allow execution of [grep_search]?', + desc: 'keep raw name for non-shell tools', + }, + ]; + + for (const { toolName, expected, desc } of cases) { + it(`should ${desc}`, async () => { + const confirmationDetails: SerializableConfirmationDetails = { + type: 'exec', + title: 'Confirm', + command: 'ls', + rootCommand: 'ls', + rootCommands: ['ls'], + }; + + const { lastFrame, unmount } = await renderWithProviders( + , + ); + + expect(lastFrame()).toContain(expected); + unmount(); + }); + } + + it('should humanize shell tool in sandbox expansion prompt', async () => { + const confirmationDetails: SerializableConfirmationDetails = { + type: 'sandbox_expansion', + title: 'Confirm', + command: 'ls', + rootCommand: 'ls', + additionalPermissions: { + network: true, + }, + }; + + const { lastFrame, unmount } = await renderWithProviders( + , + ); + + expect(lastFrame()).toContain( + 'To run [Shell], allow access to the following?', + ); + unmount(); + }); + }); + describe('with folder trust', () => { const editConfirmationDetails: SerializableConfirmationDetails = { type: 'edit', diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index b23282959e..89730ff3e6 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -629,18 +629,13 @@ export const ToolConfirmationMessage: React.FC< ); } } else if (confirmationDetails.type === 'sandbox_expansion') { - const { additionalPermissions, command, rootCommand } = - confirmationDetails; + const { additionalPermissions, command } = confirmationDetails; const readPaths = additionalPermissions?.fileSystem?.read || []; const writePaths = additionalPermissions?.fileSystem?.write || []; const network = additionalPermissions?.network; const isShell = isShellTool(toolName); - const rootCmds = rootCommand - .split(',') - .map((c) => c.trim().split(/\s+/)[0]) - .filter((c) => c && !c.startsWith('redirection')); - const commandNames = Array.from(new Set(rootCmds)).join(', '); + const commandNames = isShell ? 'Shell' : toolName; question = ''; bodyContent = ( @@ -730,13 +725,7 @@ export const ToolConfirmationMessage: React.FC< ); } - const commandNames = Array.from( - new Set( - commandsToDisplay - .map((cmd) => cmd.trim().split(/\s+/)[0]) - .filter(Boolean), - ), - ).join(', '); + const commandNames = isShell ? 'Shell' : toolName; const allowQuestion = ( diff --git a/packages/cli/src/ui/components/messages/ToolShared.tsx b/packages/cli/src/ui/components/messages/ToolShared.tsx index b246db308b..59689da083 100644 --- a/packages/cli/src/ui/components/messages/ToolShared.tsx +++ b/packages/cli/src/ui/components/messages/ToolShared.tsx @@ -32,10 +32,12 @@ export const STATUS_INDICATOR_WIDTH = 3; * Returns true if the tool name corresponds to a shell tool. */ export function isShellTool(name: string): boolean { + const normalized = name.toLowerCase(); return ( name === SHELL_COMMAND_NAME || name === SHELL_NAME || - name === SHELL_TOOL_NAME + name === SHELL_TOOL_NAME || + normalized === 'shell' ); } diff --git a/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap index 1694ca2350..5d5c57ae8b 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap @@ -4,7 +4,7 @@ exports[`ToolConfirmationMessage Redirection > should display redirection warnin "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ echo "hello" > test.txt │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo]? +Allow execution of [Shell]? Redirection detected. To auto-accept, press Shift+Tab ● 1. Allow once diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-height-allocation-and-layout-should-expand-to-available-height-for-large-exec-commands.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-height-allocation-and-layout-should-expand-to-available-height-for-large-exec-commands.snap.svg index 68e2eb2247..4d5d029e26 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-height-allocation-and-layout-should-expand-to-available-height-for-large-exec-commands.snap.svg +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-height-allocation-and-layout-should-expand-to-available-height-for-large-exec-commands.snap.svg @@ -133,7 +133,9 @@ "Line 50" │ ╰──────────────────────────────────────────────────────────────────────────────╯ - Allow execution of [echo]? + Allow execution of + [Shell] + ? ● diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-should-render-multiline-shell-scripts-with-correct-newlines-and-syntax-highlighting.snap.svg b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-should-render-multiline-shell-scripts-with-correct-newlines-and-syntax-highlighting.snap.svg index a30b871f41..0180b0581a 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-should-render-multiline-shell-scripts-with-correct-newlines-and-syntax-highlighting.snap.svg +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage-ToolConfirmationMessage-should-render-multiline-shell-scripts-with-correct-newlines-and-syntax-highlighting.snap.svg @@ -24,7 +24,9 @@ done │ ╰──────────────────────────────────────────────────────────────────────────────╯ - Allow execution of [echo]? + Allow execution of + [Shell] + ? ● diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap index 6d33b6fbfb..10680fa638 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap @@ -94,7 +94,7 @@ exports[`ToolConfirmationMessage > height allocation and layout > should expand │ echo "Line 49" │ │ echo "Line 50" │ ╰──────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo]? +Allow execution of [Shell]? ● 1. Allow once 2. Allow for this session @@ -110,7 +110,7 @@ exports[`ToolConfirmationMessage > should display multiple commands for exec typ │ │ │ whoami │ ╰──────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo, ls, whoami]? +Allow execution of [Shell]? ● 1. Allow once 2. Allow for this session @@ -148,7 +148,7 @@ exports[`ToolConfirmationMessage > should render multiline shell scripts with co │ echo $i │ │ done │ ╰──────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo]? +Allow execution of [Shell]? ● 1. Allow once 2. Allow for this session @@ -200,7 +200,7 @@ exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations' "╭──────────────────────────────────────────────────────────────────────────────╮ │ echo "hello" │ ╰──────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo]? +Allow execution of [Shell]? ● 1. Allow once 2. No, suggest changes (esc) @@ -211,7 +211,7 @@ exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations' "╭──────────────────────────────────────────────────────────────────────────────╮ │ echo "hello" │ ╰──────────────────────────────────────────────────────────────────────────────╯ -Allow execution of [echo]? +Allow execution of [Shell]? ● 1. Allow once 2. Allow for this session