feat: simplify tool confirmation labels for better UX (#15296)

This commit is contained in:
N. Taylor Mullen
2025-12-18 16:38:53 -08:00
committed by GitHub
parent 402148dbc4
commit e0f159085e
10 changed files with 72 additions and 72 deletions

View File

@@ -24,23 +24,23 @@ describe('ShellConfirmationDialog', () => {
expect(lastFrame()).toMatchSnapshot();
});
it('calls onConfirm with ProceedOnce when "Yes, allow once" is selected', () => {
it('calls onConfirm with ProceedOnce when "Allow once" is selected', () => {
const { lastFrame } = renderWithProviders(
<ShellConfirmationDialog request={request} />,
);
const select = lastFrame()!.toString();
// Simulate selecting the first option
// This is a simplified way to test the selection
expect(select).toContain('Yes, allow once');
expect(select).toContain('Allow once');
});
it('calls onConfirm with ProceedAlways when "Yes, allow always for this session" is selected', () => {
it('calls onConfirm with ProceedAlways when "Allow for this session" is selected', () => {
const { lastFrame } = renderWithProviders(
<ShellConfirmationDialog request={request} />,
);
const select = lastFrame()!.toString();
// Simulate selecting the second option
expect(select).toContain('Yes, allow always for this session');
expect(select).toContain('Allow for this session');
});
it('calls onConfirm with Cancel when "No (esc)" is selected', () => {

View File

@@ -51,14 +51,14 @@ export const ShellConfirmationDialog: React.FC<
const options: Array<RadioSelectItem<ToolConfirmationOutcome>> = [
{
label: 'Yes, allow once',
label: 'Allow once',
value: ToolConfirmationOutcome.ProceedOnce,
key: 'Yes, allow once',
key: 'Allow once',
},
{
label: 'Yes, allow always for this session',
label: 'Allow for this session',
value: ToolConfirmationOutcome.ProceedAlways,
key: 'Yes, allow always for this session',
key: 'Allow for this session',
},
{
label: 'No (esc)',

View File

@@ -13,8 +13,8 @@ exports[`ShellConfirmationDialog > renders correctly 1`] = `
│ │
│ Do you want to proceed? │
│ │
│ ● 1. Yes, allow once │
│ 2. Yes, allow always for this session │
│ ● 1. Allow once
│ 2. Allow for this session
│ 3. No (esc) │
│ │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯"

View File

@@ -104,17 +104,17 @@ describe('ToolConfirmationMessage', () => {
{
description: 'for edit confirmations',
details: editConfirmationDetails,
alwaysAllowText: 'Yes, allow always',
alwaysAllowText: 'Allow for this session',
},
{
description: 'for exec confirmations',
details: execConfirmationDetails,
alwaysAllowText: 'Yes, allow always',
alwaysAllowText: 'Allow for this session',
},
{
description: 'for info confirmations',
details: infoConfirmationDetails,
alwaysAllowText: 'Yes, allow always',
alwaysAllowText: 'Allow for this session',
},
{
description: 'for mcp confirmations',

View File

@@ -102,20 +102,20 @@ export const ToolConfirmationMessage: React.FC<
if (!confirmationDetails.isModifying) {
question = `Apply this change?`;
options.push({
label: 'Yes, allow once',
label: 'Allow once',
value: ToolConfirmationOutcome.ProceedOnce,
key: 'Yes, allow once',
key: 'Allow once',
});
if (isTrustedFolder) {
options.push({
label: 'Yes, allow always',
label: 'Allow for this session',
value: ToolConfirmationOutcome.ProceedAlways,
key: 'Yes, allow always',
key: 'Allow for this session',
});
options.push({
label: 'Yes, allow always and save to policy',
label: 'Allow for all future sessions',
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: 'Yes, allow always and save to policy',
key: 'Allow for all future sessions',
});
}
if (!config.getIdeMode() || !isDiffingEnabled) {
@@ -137,20 +137,20 @@ export const ToolConfirmationMessage: React.FC<
question = `Allow execution of: '${executionProps.rootCommand}'?`;
options.push({
label: 'Yes, allow once',
label: 'Allow once',
value: ToolConfirmationOutcome.ProceedOnce,
key: 'Yes, allow once',
key: 'Allow once',
});
if (isTrustedFolder) {
options.push({
label: `Yes, allow always ...`,
label: `Allow for this session`,
value: ToolConfirmationOutcome.ProceedAlways,
key: `Yes, allow always ...`,
key: `Allow for this session`,
});
options.push({
label: `Yes, allow always and save to policy`,
label: `Allow for all future sessions`,
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: `Yes, allow always and save to policy`,
key: `Allow for all future sessions`,
});
}
options.push({
@@ -161,20 +161,20 @@ export const ToolConfirmationMessage: React.FC<
} else if (confirmationDetails.type === 'info') {
question = `Do you want to proceed?`;
options.push({
label: 'Yes, allow once',
label: 'Allow once',
value: ToolConfirmationOutcome.ProceedOnce,
key: 'Yes, allow once',
key: 'Allow once',
});
if (isTrustedFolder) {
options.push({
label: 'Yes, allow always',
label: 'Allow for this session',
value: ToolConfirmationOutcome.ProceedAlways,
key: 'Yes, allow always',
key: 'Allow for this session',
});
options.push({
label: 'Yes, allow always and save to policy',
label: 'Allow for all future sessions',
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: 'Yes, allow always and save to policy',
key: 'Allow for all future sessions',
});
}
options.push({
@@ -187,25 +187,25 @@ export const ToolConfirmationMessage: React.FC<
const mcpProps = confirmationDetails;
question = `Allow execution of MCP tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"?`;
options.push({
label: 'Yes, allow once',
label: 'Allow once',
value: ToolConfirmationOutcome.ProceedOnce,
key: 'Yes, allow once',
key: 'Allow once',
});
if (isTrustedFolder) {
options.push({
label: `Yes, always allow tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"`,
value: ToolConfirmationOutcome.ProceedAlwaysTool, // Cast until types are updated
key: `Yes, always allow tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"`,
label: 'Allow tool for this session',
value: ToolConfirmationOutcome.ProceedAlwaysTool,
key: 'Allow tool for this session',
});
options.push({
label: `Yes, always allow all tools from server "${mcpProps.serverName}"`,
label: 'Allow all server tools for this session',
value: ToolConfirmationOutcome.ProceedAlwaysServer,
key: `Yes, always allow all tools from server "${mcpProps.serverName}"`,
key: 'Allow all server tools for this session',
});
options.push({
label: `Yes, allow always tool "${mcpProps.toolName}" and save to policy`,
label: 'Allow tool for all future sessions',
value: ToolConfirmationOutcome.ProceedAlwaysAndSave,
key: `Yes, allow always tool "${mcpProps.toolName}" and save to policy`,
key: 'Allow tool for all future sessions',
});
}
options.push({

View File

@@ -8,9 +8,9 @@ URLs to fetch:
Do you want to proceed?
● 1. Yes, allow once
2. Yes, allow always
3. Yes, allow always and save to policy
● 1. Allow once
2. Allow for this session
3. Allow for all future sessions
4. No, suggest changes (esc)
"
`;
@@ -20,9 +20,9 @@ exports[`ToolConfirmationMessage > should not display urls if prompt and url are
Do you want to proceed?
● 1. Yes, allow once
2. Yes, allow always
3. Yes, allow always and save to policy
● 1. Allow once
2. Allow for this session
3. Allow for all future sessions
4. No, suggest changes (esc)
"
`;
@@ -36,7 +36,7 @@ exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations'
Apply this change?
● 1. Yes, allow once
● 1. Allow once
2. Modify with external editor
3. No, suggest changes (esc)
"
@@ -51,9 +51,9 @@ exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations'
Apply this change?
● 1. Yes, allow once
2. Yes, allow always
3. Yes, allow always and save to policy
● 1. Allow once
2. Allow for this session
3. Allow for all future sessions
4. Modify with external editor
5. No, suggest changes (esc)
"
@@ -64,7 +64,7 @@ exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations'
Allow execution of: 'echo'?
● 1. Yes, allow once
● 1. Allow once
2. No, suggest changes (esc)
"
`;
@@ -74,9 +74,9 @@ exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations'
Allow execution of: 'echo'?
● 1. Yes, allow once
2. Yes, allow always ...
3. Yes, allow always and save to policy
● 1. Allow once
2. Allow for this session
3. Allow for all future sessions
4. No, suggest changes (esc)
"
`;
@@ -86,7 +86,7 @@ exports[`ToolConfirmationMessage > with folder trust > 'for info confirmations'
Do you want to proceed?
● 1. Yes, allow once
● 1. Allow once
2. No, suggest changes (esc)
"
`;
@@ -96,9 +96,9 @@ exports[`ToolConfirmationMessage > with folder trust > 'for info confirmations'
Do you want to proceed?
● 1. Yes, allow once
2. Yes, allow always
3. Yes, allow always and save to policy
● 1. Allow once
2. Allow for this session
3. Allow for all future sessions
4. No, suggest changes (esc)
"
`;
@@ -109,7 +109,7 @@ Tool: test-tool
Allow execution of MCP tool "test-tool" from server "test-server"?
● 1. Yes, allow once
● 1. Allow once
2. No, suggest changes (esc)
"
`;
@@ -120,10 +120,10 @@ Tool: test-tool
Allow execution of MCP tool "test-tool" from server "test-server"?
● 1. Yes, allow once
2. Yes, always allow tool "test-tool" from server "test-server"
3. Yes, always allow all tools from server "test-server"
4. Yes, allow always tool "test-tool" and save to policy
● 1. Allow once
2. Allow tool for this session
3. Allow all server tools for this session
4. Allow tool for all future sessions
5. No, suggest changes (esc)
"
`;

View File

@@ -37,9 +37,9 @@ exports[`<ToolGroupMessage /> > Confirmation Handling > shows confirmation dialo
│ │
│ Do you want to proceed? │
│ │
│ ● 1. Yes, allow once │
│ 2. Yes, allow always
│ 3. Yes, allow always and save to policy
│ ● 1. Allow once
│ 2. Allow for this session
│ 3. Allow for all future sessions
│ 4. No, suggest changes (esc) │
│ │
│ │
@@ -121,9 +121,9 @@ exports[`<ToolGroupMessage /> > Golden Snapshots > renders tool call awaiting co
│ │
│ Do you want to proceed? │
│ │
│ ● 1. Yes, allow once │
│ 2. Yes, allow always
│ 3. Yes, allow always and save to policy
│ ● 1. Allow once
│ 2. Allow for this session
│ 3. Allow for all future sessions
│ 4. No, suggest changes (esc) │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯"