refactor(cli): simplify UI and remove legacy inline tool confirmation logic (#18566)

This commit is contained in:
Abhi
2026-02-11 19:46:58 -05:00
committed by GitHub
parent 08e8eeab84
commit c370d2397b
7 changed files with 83 additions and 625 deletions
@@ -5,7 +5,6 @@
*/
import { renderWithProviders } from '../../../test-utils/render.js';
import { createMockSettings } from '../../../test-utils/settings.js';
import { describe, it, expect, vi, afterEach } from 'vitest';
import { ToolGroupMessage } from './ToolGroupMessage.js';
import type { IndividualToolCallDisplay } from '../../types.js';
@@ -35,7 +34,6 @@ describe('<ToolGroupMessage />', () => {
const baseProps = {
groupId: 1,
terminalWidth: 80,
isFocused: true,
};
const baseMockConfig = makeFakeConfig({
@@ -45,7 +43,6 @@ describe('<ToolGroupMessage />', () => {
folderTrust: false,
ideMode: false,
enableInteractiveShell: true,
enableEventDrivenScheduler: true,
});
describe('Golden Snapshots', () => {
@@ -64,7 +61,31 @@ describe('<ToolGroupMessage />', () => {
unmount();
});
it('renders multiple tool calls with different statuses', () => {
it('hides confirming tools (standard behavior)', () => {
const toolCalls = [
createToolCall({
callId: 'confirm-tool',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm tool',
prompt: 'Do you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{ config: baseMockConfig },
);
// Should render nothing because all tools in the group are confirming
expect(lastFrame()).toBe('');
unmount();
});
it('renders multiple tool calls with different statuses (only visible ones)', () => {
const toolCalls = [
createToolCall({
callId: 'tool-1',
@@ -85,68 +106,7 @@ describe('<ToolGroupMessage />', () => {
status: ToolCallStatus.Error,
}),
];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
config: mockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders tool call awaiting confirmation', () => {
const toolCalls = [
createToolCall({
callId: 'tool-confirm',
name: 'confirmation-tool',
description: 'This tool needs confirmation',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm Tool Execution',
prompt: 'Are you sure you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
config: mockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders shell command with yellow border', () => {
const toolCalls = [
createToolCall({
callId: 'shell-1',
name: 'run_shell_command',
description: 'Execute shell command',
status: ToolCallStatus.Success,
}),
];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
@@ -156,7 +116,12 @@ describe('<ToolGroupMessage />', () => {
},
},
);
expect(lastFrame()).toMatchSnapshot();
// pending-tool should be hidden
const output = lastFrame();
expect(output).toContain('successful-tool');
expect(output).not.toContain('pending-tool');
expect(output).toContain('error-tool');
expect(output).toMatchSnapshot();
unmount();
});
@@ -181,22 +146,22 @@ describe('<ToolGroupMessage />', () => {
status: ToolCallStatus.Pending,
}),
];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
config: mockConfig,
config: baseMockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
expect(lastFrame()).toMatchSnapshot();
// write_file (Pending) should be hidden
const output = lastFrame();
expect(output).toContain('read_file');
expect(output).toContain('run_shell_command');
expect(output).not.toContain('write_file');
expect(output).toMatchSnapshot();
unmount();
});
@@ -233,25 +198,6 @@ describe('<ToolGroupMessage />', () => {
unmount();
});
it('renders when not focused', () => {
const toolCalls = [createToolCall()];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage
{...baseProps}
toolCalls={toolCalls}
isFocused={false}
/>,
{
config: baseMockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders with narrow terminal width', () => {
const toolCalls = [
createToolCall({
@@ -384,28 +330,6 @@ describe('<ToolGroupMessage />', () => {
});
describe('Border Color Logic', () => {
it('uses yellow border when tools are pending', () => {
const toolCalls = [createToolCall({ status: ToolCallStatus.Pending })];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
config: mockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
// The snapshot will capture the visual appearance including border color
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('uses yellow border for shell commands even when successful', () => {
const toolCalls = [
createToolCall({
@@ -483,210 +407,6 @@ describe('<ToolGroupMessage />', () => {
});
});
describe('Confirmation Handling', () => {
it('shows confirmation dialog for first confirming tool only', () => {
const toolCalls = [
createToolCall({
callId: 'tool-1',
name: 'first-confirm',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm First Tool',
prompt: 'Confirm first tool',
onConfirm: vi.fn(),
},
}),
createToolCall({
callId: 'tool-2',
name: 'second-confirm',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm Second Tool',
prompt: 'Confirm second tool',
onConfirm: vi.fn(),
},
}),
];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
config: mockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
// Should only show confirmation for the first tool
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders confirmation with permanent approval enabled', () => {
const toolCalls = [
createToolCall({
callId: 'tool-1',
name: 'confirm-tool',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm Tool',
prompt: 'Do you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const settings = createMockSettings({
security: { enablePermanentToolApproval: true },
});
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{
settings,
config: mockConfig,
uiState: {
pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }],
},
},
);
expect(lastFrame()).toContain('Allow for all future sessions');
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders confirmation with permanent approval disabled', () => {
const toolCalls = [
createToolCall({
callId: 'confirm-tool',
name: 'confirm-tool',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm tool',
prompt: 'Do you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const mockConfig = makeFakeConfig({
model: 'gemini-pro',
targetDir: os.tmpdir(),
enableEventDrivenScheduler: false,
});
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{ config: mockConfig },
);
expect(lastFrame()).not.toContain('Allow for all future sessions');
expect(lastFrame()).toMatchSnapshot();
unmount();
});
});
describe('Event-Driven Scheduler', () => {
it('hides confirming tools when event-driven scheduler is enabled', () => {
const toolCalls = [
createToolCall({
callId: 'confirm-tool',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm tool',
prompt: 'Do you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const mockConfig = baseMockConfig;
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{ config: mockConfig },
);
// Should render nothing because all tools in the group are confirming
expect(lastFrame()).toBe('');
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('shows only successful tools when mixed with confirming tools', () => {
const toolCalls = [
createToolCall({
callId: 'success-tool',
name: 'success-tool',
status: ToolCallStatus.Success,
}),
createToolCall({
callId: 'confirm-tool',
name: 'confirm-tool',
status: ToolCallStatus.Confirming,
confirmationDetails: {
type: 'info',
title: 'Confirm tool',
prompt: 'Do you want to proceed?',
onConfirm: vi.fn(),
},
}),
];
const mockConfig = baseMockConfig;
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{ config: mockConfig },
);
const output = lastFrame();
expect(output).toContain('success-tool');
expect(output).not.toContain('confirm-tool');
expect(output).not.toContain('Do you want to proceed?');
expect(output).toMatchSnapshot();
unmount();
});
it('renders nothing when only tool is in-progress AskUser with borderBottom=false', () => {
// AskUser tools in progress are rendered by AskUserDialog, not ToolGroupMessage.
// When AskUser is the only tool and borderBottom=false (no border to close),
// the component should render nothing.
const toolCalls = [
createToolCall({
callId: 'ask-user-tool',
name: 'Ask User',
status: ToolCallStatus.Executing,
}),
];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage
{...baseProps}
toolCalls={toolCalls}
borderBottom={false}
/>,
{ config: baseMockConfig },
);
// AskUser tools in progress are rendered by AskUserDialog, so we expect nothing.
expect(lastFrame()).toMatchSnapshot();
unmount();
});
});
describe('Ask User Filtering', () => {
it.each([
ToolCallStatus.Pending,
@@ -753,5 +473,30 @@ describe('<ToolGroupMessage />', () => {
expect(lastFrame()).toMatchSnapshot();
unmount();
});
it('renders nothing when only tool is in-progress AskUser with borderBottom=false', () => {
// AskUser tools in progress are rendered by AskUserDialog, not ToolGroupMessage.
// When AskUser is the only tool and borderBottom=false (no border to close),
// the component should render nothing.
const toolCalls = [
createToolCall({
callId: 'ask-user-tool',
name: ASK_USER_DISPLAY_NAME,
status: ToolCallStatus.Executing,
}),
];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage
{...baseProps}
toolCalls={toolCalls}
borderBottom={false}
/>,
{ config: baseMockConfig },
);
// AskUser tools in progress are rendered by AskUserDialog, so we expect nothing.
expect(lastFrame()).toBe('');
unmount();
});
});
});