fix(cli): use static tool name in confirmation prompt to avoid parsing errors (#26866)

This commit is contained in:
Coco Sheng
2026-05-11 13:45:58 -04:00
committed by GitHub
parent 4739495e39
commit 36a7fa089c
11 changed files with 128 additions and 31 deletions
@@ -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();
@@ -84,8 +84,8 @@
<text x="711" y="291" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="308" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="9" y="308" fill="#ffffff" textLength="180" lengthAdjust="spacingAndGlyphs"> Allow execution of </text>
<text x="189" y="308" fill="#ffffaf" textLength="54" lengthAdjust="spacingAndGlyphs" font-weight="bold">[echo]</text>
<text x="243" y="308" fill="#ffffff" textLength="468" lengthAdjust="spacingAndGlyphs">? </text>
<text x="189" y="308" fill="#ffffaf" textLength="63" lengthAdjust="spacingAndGlyphs" font-weight="bold">[Shell]</text>
<text x="252" y="308" fill="#ffffff" textLength="459" lengthAdjust="spacingAndGlyphs">? </text>
<text x="711" y="308" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="325" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="711" y="325" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>

Before

Width:  |  Height:  |  Size: 11 KiB

After

Width:  |  Height:  |  Size: 11 KiB

@@ -191,8 +191,8 @@
<text x="711" y="546" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="563" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="9" y="563" fill="#ffffff" textLength="180" lengthAdjust="spacingAndGlyphs"> Allow execution of </text>
<text x="189" y="563" fill="#ffffaf" textLength="54" lengthAdjust="spacingAndGlyphs" font-weight="bold">[echo]</text>
<text x="243" y="563" fill="#ffffff" textLength="468" lengthAdjust="spacingAndGlyphs">? </text>
<text x="189" y="563" fill="#ffffaf" textLength="63" lengthAdjust="spacingAndGlyphs" font-weight="bold">[Shell]</text>
<text x="252" y="563" fill="#ffffff" textLength="459" lengthAdjust="spacingAndGlyphs">? </text>
<text x="711" y="563" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="580" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="711" y="580" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>

Before

Width:  |  Height:  |  Size: 22 KiB

After

Width:  |  Height:  |  Size: 22 KiB

@@ -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 │
@@ -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(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={confirmationDetails}
config={mockConfig}
getPreferredEditor={vi.fn()}
availableTerminalHeight={30}
terminalWidth={80}
toolName="shell"
/>,
);
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(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={confirmationDetails}
config={mockConfig}
getPreferredEditor={vi.fn()}
availableTerminalHeight={30}
terminalWidth={80}
toolName={toolName}
/>,
);
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(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={confirmationDetails}
config={mockConfig}
getPreferredEditor={vi.fn()}
availableTerminalHeight={30}
terminalWidth={80}
toolName="run_shell_command"
/>,
);
expect(lastFrame()).toContain(
'To run [Shell], allow access to the following?',
);
unmount();
});
});
describe('with folder trust', () => {
const editConfirmationDetails: SerializableConfirmationDetails = {
type: 'edit',
@@ -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 = (
<Text>
@@ -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'
);
}
@@ -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
@@ -133,7 +133,9 @@
<text x="63" y="546" fill="#cdcd00" textLength="81" lengthAdjust="spacingAndGlyphs">&quot;Line 50&quot;</text>
<text x="711" y="546" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="563" fill="#333333" textLength="720" lengthAdjust="spacingAndGlyphs">╰──────────────────────────────────────────────────────────────────────────────╯</text>
<text x="0" y="580" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Allow execution of [echo]? </text>
<text x="0" y="580" fill="#ffffff" textLength="171" lengthAdjust="spacingAndGlyphs">Allow execution of </text>
<text x="171" y="580" fill="#ffffaf" textLength="63" lengthAdjust="spacingAndGlyphs" font-weight="bold">[Shell]</text>
<text x="234" y="580" fill="#ffffff" textLength="666" lengthAdjust="spacingAndGlyphs">? </text>
<rect x="0" y="612" width="9" height="17" fill="#001a00" />
<text x="0" y="614" fill="#00cd00" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<rect x="9" y="612" width="9" height="17" fill="#001a00" />

Before

Width:  |  Height:  |  Size: 15 KiB

After

Width:  |  Height:  |  Size: 15 KiB

@@ -24,7 +24,9 @@
<text x="18" y="70" fill="#0000ee" textLength="36" lengthAdjust="spacingAndGlyphs">done</text>
<text x="711" y="70" fill="#333333" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<text x="0" y="87" fill="#333333" textLength="720" lengthAdjust="spacingAndGlyphs">╰──────────────────────────────────────────────────────────────────────────────╯</text>
<text x="0" y="104" fill="#ffffff" textLength="900" lengthAdjust="spacingAndGlyphs">Allow execution of [echo]? </text>
<text x="0" y="104" fill="#ffffff" textLength="171" lengthAdjust="spacingAndGlyphs">Allow execution of </text>
<text x="171" y="104" fill="#ffffaf" textLength="63" lengthAdjust="spacingAndGlyphs" font-weight="bold">[Shell]</text>
<text x="234" y="104" fill="#ffffff" textLength="666" lengthAdjust="spacingAndGlyphs">? </text>
<rect x="0" y="136" width="9" height="17" fill="#001a00" />
<text x="0" y="138" fill="#00cd00" textLength="9" lengthAdjust="spacingAndGlyphs"></text>
<rect x="9" y="136" width="9" height="17" fill="#001a00" />

Before

Width:  |  Height:  |  Size: 4.0 KiB

After

Width:  |  Height:  |  Size: 4.2 KiB

@@ -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