From c51c30ede6c2fd58d3513bfee8c9f19619194831 Mon Sep 17 00:00:00 2001 From: Abhi Date: Sun, 8 Feb 2026 00:17:28 -0500 Subject: [PATCH 1/2] refactor(cli): simplify UI by removing legacy inline tool confirmation logic --- .../AlternateBufferQuittingDisplay.tsx | 6 +- .../src/ui/components/HistoryItemDisplay.tsx | 3 - .../cli/src/ui/components/MainContent.tsx | 7 +- .../ToolConfirmationMessageOverflow.test.tsx | 128 ------ .../messages/ToolGroupMessage.test.tsx | 368 ++++-------------- .../components/messages/ToolGroupMessage.tsx | 63 +-- .../ToolGroupMessage.test.tsx.snap | 105 +---- 7 files changed, 84 insertions(+), 596 deletions(-) delete mode 100644 packages/cli/src/ui/components/messages/ToolConfirmationMessageOverflow.test.tsx diff --git a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx index fec35d46c39..8e0ede2e098 100644 --- a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx +++ b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx @@ -12,18 +12,15 @@ import { QuittingDisplay } from './QuittingDisplay.js'; import { useAppContext } from '../contexts/AppContext.js'; import { MAX_GEMINI_MESSAGE_LINES } from '../constants.js'; import { useConfirmingTool } from '../hooks/useConfirmingTool.js'; -import { useConfig } from '../contexts/ConfigContext.js'; import { ToolStatusIndicator, ToolInfo } from './messages/ToolShared.js'; import { theme } from '../semantic-colors.js'; export const AlternateBufferQuittingDisplay = () => { const { version } = useAppContext(); const uiState = useUIState(); - const config = useConfig(); const confirmingTool = useConfirmingTool(); - const showPromptedTool = - config.isEventDrivenSchedulerEnabled() && confirmingTool !== null; + const showPromptedTool = confirmingTool !== null; // We render the entire chat history and header here to ensure that the // conversation history is visible to the user after the app quits and the @@ -56,7 +53,6 @@ export const AlternateBufferQuittingDisplay = () => { terminalWidth={uiState.mainAreaWidth} item={{ ...item, id: 0 }} isPending={true} - isFocused={false} activeShellPtyId={uiState.activePtyId} embeddedShellFocused={uiState.embeddedShellFocused} /> diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.tsx index 41340c1b089..f41ee20895d 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.tsx @@ -43,7 +43,6 @@ interface HistoryItemDisplayProps { availableTerminalHeight?: number; terminalWidth: number; isPending: boolean; - isFocused?: boolean; commands?: readonly SlashCommand[]; activeShellPtyId?: number | null; embeddedShellFocused?: boolean; @@ -56,7 +55,6 @@ export const HistoryItemDisplay: React.FC = ({ terminalWidth, isPending, commands, - isFocused = true, activeShellPtyId, embeddedShellFocused, availableTerminalHeightGemini, @@ -179,7 +177,6 @@ export const HistoryItemDisplay: React.FC = ({ groupId={itemForDisplay.id} availableTerminalHeight={availableTerminalHeight} terminalWidth={terminalWidth} - isFocused={isFocused} activeShellPtyId={activeShellPtyId} embeddedShellFocused={embeddedShellFocused} borderTop={itemForDisplay.borderTop} diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index 32c70e8cadc..586553a1f29 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -19,7 +19,6 @@ import { useMemo, memo, useCallback, useEffect, useRef } from 'react'; import { MAX_GEMINI_MESSAGE_LINES } from '../constants.js'; import { useConfirmingTool } from '../hooks/useConfirmingTool.js'; import { ToolConfirmationQueue } from './ToolConfirmationQueue.js'; -import { useConfig } from '../contexts/ConfigContext.js'; const MemoizedHistoryItemDisplay = memo(HistoryItemDisplay); const MemoizedAppHeader = memo(AppHeader); @@ -31,12 +30,10 @@ const MemoizedAppHeader = memo(AppHeader); export const MainContent = () => { const { version } = useAppContext(); const uiState = useUIState(); - const config = useConfig(); const isAlternateBuffer = useAlternateBuffer(); const confirmingTool = useConfirmingTool(); - const showConfirmationQueue = - config.isEventDrivenSchedulerEnabled() && confirmingTool !== null; + const showConfirmationQueue = confirmingTool !== null; const scrollableListRef = useRef>(null); @@ -89,7 +86,6 @@ export const MainContent = () => { terminalWidth={mainAreaWidth} item={{ ...item, id: 0 }} isPending={true} - isFocused={!uiState.isEditorDialogOpen} activeShellPtyId={uiState.activePtyId} embeddedShellFocused={uiState.embeddedShellFocused} /> @@ -105,7 +101,6 @@ export const MainContent = () => { isAlternateBuffer, availableTerminalHeight, mainAreaWidth, - uiState.isEditorDialogOpen, uiState.activePtyId, uiState.embeddedShellFocused, showConfirmationQueue, diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessageOverflow.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessageOverflow.test.tsx deleted file mode 100644 index b59b6c5adf8..00000000000 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessageOverflow.test.tsx +++ /dev/null @@ -1,128 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { describe, it, expect, vi } from 'vitest'; -import { ToolGroupMessage } from './ToolGroupMessage.js'; -import type { - ToolCallConfirmationDetails, - Config, -} from '@google/gemini-cli-core'; -import { renderWithProviders } from '../../../test-utils/render.js'; -import { useToolActions } from '../../contexts/ToolActionsContext.js'; -import { - StreamingState, - ToolCallStatus, - type IndividualToolCallDisplay, -} from '../../types.js'; -import { OverflowProvider } from '../../contexts/OverflowContext.js'; -import { waitFor } from '../../../test-utils/async.js'; - -vi.mock('../../contexts/ToolActionsContext.js', async (importOriginal) => { - const actual = - await importOriginal< - typeof import('../../contexts/ToolActionsContext.js') - >(); - return { - ...actual, - useToolActions: vi.fn(), - }; -}); - -describe('ToolConfirmationMessage Overflow', () => { - const mockConfirm = vi.fn(); - vi.mocked(useToolActions).mockReturnValue({ - confirm: mockConfirm, - cancel: vi.fn(), - isDiffingEnabled: false, - }); - - const mockConfig = { - isTrustedFolder: () => true, - getIdeMode: () => false, - getMessageBus: () => ({ - subscribe: vi.fn(), - unsubscribe: vi.fn(), - publish: vi.fn(), - }), - isEventDrivenSchedulerEnabled: () => false, - getTheme: () => ({ - status: { warning: 'yellow' }, - text: { primary: 'white', secondary: 'gray', link: 'blue' }, - border: { default: 'gray' }, - ui: { symbol: 'cyan' }, - }), - } as unknown as Config; - - it('should display "press ctrl-o" hint when content overflows in ToolGroupMessage', async () => { - // Large diff that will definitely overflow - const diffLines = ['--- a/test.txt', '+++ b/test.txt', '@@ -1,20 +1,20 @@']; - for (let i = 0; i < 50; i++) { - diffLines.push(`+ line ${i + 1}`); - } - const fileDiff = diffLines.join('\n'); - - const confirmationDetails: ToolCallConfirmationDetails = { - type: 'edit', - title: 'Confirm Edit', - fileName: 'test.txt', - filePath: '/test.txt', - fileDiff, - originalContent: '', - newContent: 'lots of lines', - onConfirm: vi.fn(), - }; - - const toolCalls: IndividualToolCallDisplay[] = [ - { - callId: 'test-call-id', - name: 'test-tool', - description: 'a test tool', - status: ToolCallStatus.Confirming, - confirmationDetails, - resultDisplay: undefined, - }, - ]; - - const { lastFrame } = renderWithProviders( - - - , - { - config: mockConfig, - uiState: { - streamingState: StreamingState.WaitingForConfirmation, - constrainHeight: true, - }, - }, - ); - - // ResizeObserver might take a tick - await waitFor(() => - expect(lastFrame()).toContain('Press ctrl-o to show more lines'), - ); - - const frame = lastFrame(); - expect(frame).toBeDefined(); - if (frame) { - expect(frame).toContain('Press ctrl-o to show more lines'); - // Ensure it's AFTER the bottom border - const linesOfOutput = frame.split('\n'); - const bottomBorderIndex = linesOfOutput.findLastIndex((l) => - l.includes('╰─'), - ); - const hintIndex = linesOfOutput.findIndex((l) => - l.includes('Press ctrl-o to show more lines'), - ); - expect(hintIndex).toBeGreaterThan(bottomBorderIndex); - expect(frame).toMatchSnapshot(); - } - }); -}); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 5368684ea28..bdb3b7a0ed1 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -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('', () => { const baseProps = { groupId: 1, terminalWidth: 80, - isFocused: true, }; const baseMockConfig = makeFakeConfig({ @@ -45,7 +43,6 @@ describe('', () => { folderTrust: false, ideMode: false, enableInteractiveShell: true, - enableEventDrivenScheduler: true, }); describe('Golden Snapshots', () => { @@ -64,89 +61,53 @@ describe('', () => { unmount(); }); - it('renders multiple tool calls with different statuses', () => { + it('hides confirming tools (standard behavior)', () => { const toolCalls = [ createToolCall({ - callId: 'tool-1', - name: 'successful-tool', - description: 'This tool succeeded', - status: ToolCallStatus.Success, - }), - createToolCall({ - callId: 'tool-2', - name: 'pending-tool', - description: 'This tool is pending', - status: ToolCallStatus.Pending, - }), - createToolCall({ - callId: 'tool-3', - name: 'error-tool', - description: 'This tool failed', - status: ToolCallStatus.Error, - }), - ]; - const mockConfig = makeFakeConfig({ - model: 'gemini-pro', - targetDir: os.tmpdir(), - enableEventDrivenScheduler: false, - }); - - const { lastFrame, unmount } = renderWithProviders( - , - { - 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', + callId: 'confirm-tool', status: ToolCallStatus.Confirming, confirmationDetails: { type: 'info', - title: 'Confirm Tool Execution', - prompt: 'Are you sure you want to proceed?', + 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( , - { - config: mockConfig, - uiState: { - pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], - }, - }, + { config: baseMockConfig }, ); + + // Should render nothing because all tools in the group are confirming + expect(lastFrame()).toBe(''); expect(lastFrame()).toMatchSnapshot(); unmount(); }); - it('renders shell command with yellow border', () => { + it('renders multiple tool calls with different statuses (only visible ones)', () => { const toolCalls = [ createToolCall({ - callId: 'shell-1', - name: 'run_shell_command', - description: 'Execute shell command', + callId: 'tool-1', + name: 'successful-tool', + description: 'This tool succeeded', status: ToolCallStatus.Success, }), + createToolCall({ + callId: 'tool-2', + name: 'pending-tool', + description: 'This tool is pending', + status: ToolCallStatus.Pending, + }), + createToolCall({ + callId: 'tool-3', + name: 'error-tool', + description: 'This tool failed', + status: ToolCallStatus.Error, + }), ]; + const { lastFrame, unmount } = renderWithProviders( , { @@ -156,7 +117,12 @@ describe('', () => { }, }, ); - 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 +147,22 @@ describe('', () => { status: ToolCallStatus.Pending, }), ]; - const mockConfig = makeFakeConfig({ - model: 'gemini-pro', - targetDir: os.tmpdir(), - enableEventDrivenScheduler: false, - }); const { lastFrame, unmount } = renderWithProviders( , { - 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(); }); @@ -236,11 +202,7 @@ describe('', () => { it('renders when not focused', () => { const toolCalls = [createToolCall()]; const { lastFrame, unmount } = renderWithProviders( - , + , { config: baseMockConfig, uiState: { @@ -384,28 +346,6 @@ describe('', () => { }); 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( - , - { - 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 +423,6 @@ describe('', () => { }); }); - 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( - , - { - 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( - , - { - 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( - , - { 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( - , - { 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( - , - { 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( - , - { 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 +489,31 @@ describe('', () => { 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( + , + { config: baseMockConfig }, + ); + // AskUser tools in progress are rendered by AskUserDialog, so we expect nothing. + expect(lastFrame()).toBe(''); + expect(lastFrame()).toMatchSnapshot(); + unmount(); + }); }); }); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index f9225b60e7a..01fcb4b8784 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -11,7 +11,6 @@ import type { IndividualToolCallDisplay } from '../../types.js'; import { ToolCallStatus } from '../../types.js'; import { ToolMessage } from './ToolMessage.js'; import { ShellToolMessage } from './ShellToolMessage.js'; -import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import { theme } from '../../semantic-colors.js'; import { useConfig } from '../../contexts/ConfigContext.js'; import { isShellTool, isThisShellFocused } from './ToolShared.js'; @@ -24,7 +23,6 @@ interface ToolGroupMessageProps { toolCalls: IndividualToolCallDisplay[]; availableTerminalHeight?: number; terminalWidth: number; - isFocused?: boolean; activeShellPtyId?: number | null; embeddedShellFocused?: boolean; onShellInputSubmit?: (input: string) => void; @@ -43,13 +41,11 @@ const isAskUserInProgress = (t: IndividualToolCallDisplay): boolean => // Main component renders the border and maps the tools using ToolMessage const TOOL_MESSAGE_HORIZONTAL_MARGIN = 4; -const TOOL_CONFIRMATION_INTERNAL_PADDING = 4; export const ToolGroupMessage: React.FC = ({ toolCalls: allToolCalls, availableTerminalHeight, terminalWidth, - isFocused = true, activeShellPtyId, embeddedShellFocused, borderTop: borderTopOverride, @@ -64,24 +60,21 @@ export const ToolGroupMessage: React.FC = ({ const config = useConfig(); const { constrainHeight } = useUIState(); - const isEventDriven = config.isEventDrivenSchedulerEnabled(); - // If Event-Driven Scheduler is enabled, we HIDE tools that are still in // pre-execution states (Confirming, Pending) from the History log. // They live in the Global Queue or wait for their turn. - const visibleToolCalls = useMemo(() => { - if (!isEventDriven) { - return toolCalls; - } - // Only show tools that are actually running or finished. - // We explicitly exclude Pending and Confirming to ensure they only - // appear in the Global Queue until they are approved and start executing. - return toolCalls.filter( - (t) => - t.status !== ToolCallStatus.Pending && - t.status !== ToolCallStatus.Confirming, - ); - }, [toolCalls, isEventDriven]); + // Only show tools that are actually running or finished. + // We explicitly exclude Pending and Confirming to ensure they only + // appear in the Global Queue until they are approved and start executing. + const visibleToolCalls = useMemo( + () => + toolCalls.filter( + (t) => + t.status !== ToolCallStatus.Pending && + t.status !== ToolCallStatus.Confirming, + ), + [toolCalls], + ); const isEmbeddedShellFocused = visibleToolCalls.some((t) => isThisShellFocused( @@ -110,15 +103,6 @@ export const ToolGroupMessage: React.FC = ({ const staticHeight = /* border */ 2 + /* marginBottom */ 1; - // Inline confirmations are ONLY used when the Global Queue is disabled. - const toolAwaitingApproval = useMemo( - () => - isEventDriven - ? undefined - : toolCalls.find((tc) => tc.status === ToolCallStatus.Confirming), - [toolCalls, isEventDriven], - ); - // If all tools are filtered out (e.g., in-progress AskUser tools, confirming tools // in event-driven mode), only render if we need to close a border from previous // tool groups. borderBottomOverride=true means we must render the closing border; @@ -163,7 +147,6 @@ export const ToolGroupMessage: React.FC = ({ paddingRight={TOOL_MESSAGE_HORIZONTAL_MARGIN} > {visibleToolCalls.map((tool, index) => { - const isConfirming = toolAwaitingApproval?.callId === tool.callId; const isFirst = index === 0; const isShellToolCall = isShellTool(tool.name); @@ -171,11 +154,7 @@ export const ToolGroupMessage: React.FC = ({ ...tool, availableTerminalHeight: availableTerminalHeightPerToolMessage, terminalWidth: contentWidth, - emphasis: isConfirming - ? ('high' as const) - : toolAwaitingApproval - ? ('low' as const) - : ('medium' as const), + emphasis: 'medium' as const, isFirst: borderTopOverride !== undefined ? borderTopOverride && isFirst @@ -213,22 +192,6 @@ export const ToolGroupMessage: React.FC = ({ paddingLeft={1} paddingRight={1} > - {tool.status === ToolCallStatus.Confirming && - isConfirming && - tool.confirmationDetails && ( - - )} {tool.outputFile && ( diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap index 369fa591741..394a604506d 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap @@ -22,6 +22,8 @@ exports[` > Ask User Filtering > filters out ask_user when s exports[` > Ask User Filtering > filters out ask_user when status is Pending 1`] = `""`; +exports[` > Ask User Filtering > renders nothing when only tool is in-progress AskUser with borderBottom=false 1`] = `""`; + exports[` > Ask User Filtering > shows other tools when ask_user is filtered out 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ other-tool A tool for testing │ @@ -50,75 +52,7 @@ exports[` > Border Color Logic > uses yellow border for shel ╰──────────────────────────────────────────────────────────────────────────╯" `; -exports[` > Border Color Logic > uses yellow border when tools are pending 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ o test-tool A tool for testing │ -│ │ -│ Test result │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - -exports[` > Confirmation Handling > renders confirmation with permanent approval disabled 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ? confirm-tool A tool for testing ← │ -│ │ -│ Test result │ -│ Do you want to proceed? │ -│ Do you want to proceed? │ -│ │ -│ ● 1. Allow once │ -│ 2. Allow for this session │ -│ 3. No, suggest changes (esc) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - -exports[` > Confirmation Handling > renders confirmation with permanent approval enabled 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ? confirm-tool A tool for testing ← │ -│ │ -│ Test result │ -│ Do you want to proceed? │ -│ Do you want to proceed? │ -│ │ -│ ● 1. Allow once │ -│ 2. Allow for this session │ -│ 3. Allow for all future sessions │ -│ 4. No, suggest changes (esc) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - -exports[` > Confirmation Handling > shows confirmation dialog for first confirming tool only 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ? first-confirm A tool for testing ← │ -│ │ -│ Test result │ -│ Confirm first tool │ -│ Do you want to proceed? │ -│ │ -│ ● 1. Allow once │ -│ 2. Allow for this session │ -│ 3. No, suggest changes (esc) │ -│ │ -│ │ -│ ? second-confirm A tool for testing │ -│ │ -│ Test result │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - -exports[` > Event-Driven Scheduler > hides confirming tools when event-driven scheduler is enabled 1`] = `""`; - -exports[` > Event-Driven Scheduler > renders nothing when only tool is in-progress AskUser with borderBottom=false 1`] = `""`; - -exports[` > Event-Driven Scheduler > shows only successful tools when mixed with confirming tools 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ✓ success-tool A tool for testing │ -│ │ -│ Test result │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; +exports[` > Golden Snapshots > hides confirming tools (standard behavior) 1`] = `""`; exports[` > Golden Snapshots > renders empty tool calls array 1`] = `""`; @@ -144,37 +78,21 @@ exports[` > Golden Snapshots > renders mixed tool calls incl │ ⊷ run_shell_command Run command │ │ │ │ Test result │ -│ │ -│ o write_file Write to file │ -│ │ -│ Test result │ ╰──────────────────────────────────────────────────────────────────────────╯" `; -exports[` > Golden Snapshots > renders multiple tool calls with different statuses 1`] = ` +exports[` > Golden Snapshots > renders multiple tool calls with different statuses (only visible ones) 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ successful-tool This tool succeeded │ │ │ │ Test result │ │ │ -│ o pending-tool This tool is pending │ -│ │ -│ Test result │ -│ │ │ x error-tool This tool failed │ │ │ │ Test result │ ╰──────────────────────────────────────────────────────────────────────────╯" `; -exports[` > Golden Snapshots > renders shell command with yellow border 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ✓ run_shell_command Execute shell command │ -│ │ -│ Test result │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - exports[` > Golden Snapshots > renders single successful tool call 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ test-tool A tool for testing │ @@ -183,21 +101,6 @@ exports[` > Golden Snapshots > renders single successful too ╰──────────────────────────────────────────────────────────────────────────╯" `; -exports[` > Golden Snapshots > renders tool call awaiting confirmation 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ? confirmation-tool This tool needs confirmation ← │ -│ │ -│ Test result │ -│ Are you sure you want to proceed? │ -│ Do you want to proceed? │ -│ │ -│ ● 1. Allow once │ -│ 2. Allow for this session │ -│ 3. No, suggest changes (esc) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - exports[` > Golden Snapshots > renders tool call with outputFile 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool-with-file Tool that saved output to file │ From b6141535e25a59a306eabfe52043c7ef96cd381a Mon Sep 17 00:00:00 2001 From: Abhi Date: Thu, 12 Feb 2026 09:28:58 +0900 Subject: [PATCH 2/2] fix(cli): address review comments for tool cleanup --- .../messages/ToolGroupMessage.test.tsx | 17 ----------------- .../ui/components/messages/ToolGroupMessage.tsx | 9 ++++----- .../ToolGroupMessage.test.tsx.snap | 12 ------------ 3 files changed, 4 insertions(+), 34 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index bdb3b7a0ed1..d2d3cd277a1 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -82,7 +82,6 @@ describe('', () => { // Should render nothing because all tools in the group are confirming expect(lastFrame()).toBe(''); - expect(lastFrame()).toMatchSnapshot(); unmount(); }); @@ -199,21 +198,6 @@ describe('', () => { unmount(); }); - it('renders when not focused', () => { - const toolCalls = [createToolCall()]; - const { lastFrame, unmount } = renderWithProviders( - , - { - config: baseMockConfig, - uiState: { - pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], - }, - }, - ); - expect(lastFrame()).toMatchSnapshot(); - unmount(); - }); - it('renders with narrow terminal width', () => { const toolCalls = [ createToolCall({ @@ -512,7 +496,6 @@ describe('', () => { ); // AskUser tools in progress are rendered by AskUserDialog, so we expect nothing. expect(lastFrame()).toBe(''); - expect(lastFrame()).toMatchSnapshot(); unmount(); }); }); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 01fcb4b8784..07ae280558c 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -60,9 +60,8 @@ export const ToolGroupMessage: React.FC = ({ const config = useConfig(); const { constrainHeight } = useUIState(); - // If Event-Driven Scheduler is enabled, we HIDE tools that are still in - // pre-execution states (Confirming, Pending) from the History log. - // They live in the Global Queue or wait for their turn. + // We HIDE tools that are still in pre-execution states (Confirming, Pending) + // from the History log. They live in the Global Queue or wait for their turn. // Only show tools that are actually running or finished. // We explicitly exclude Pending and Confirming to ensure they only // appear in the Global Queue until they are approved and start executing. @@ -103,8 +102,8 @@ export const ToolGroupMessage: React.FC = ({ const staticHeight = /* border */ 2 + /* marginBottom */ 1; - // If all tools are filtered out (e.g., in-progress AskUser tools, confirming tools - // in event-driven mode), only render if we need to close a border from previous + // If all tools are filtered out (e.g., in-progress AskUser tools, confirming tools), + // only render if we need to close a border from previous // tool groups. borderBottomOverride=true means we must render the closing border; // undefined or false means there's nothing to display. if (visibleToolCalls.length === 0 && borderBottomOverride !== true) { diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap index 394a604506d..3586b32c217 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolGroupMessage.test.tsx.snap @@ -22,8 +22,6 @@ exports[` > Ask User Filtering > filters out ask_user when s exports[` > Ask User Filtering > filters out ask_user when status is Pending 1`] = `""`; -exports[` > Ask User Filtering > renders nothing when only tool is in-progress AskUser with borderBottom=false 1`] = `""`; - exports[` > Ask User Filtering > shows other tools when ask_user is filtered out 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ other-tool A tool for testing │ @@ -52,8 +50,6 @@ exports[` > Border Color Logic > uses yellow border for shel ╰──────────────────────────────────────────────────────────────────────────╯" `; -exports[` > Golden Snapshots > hides confirming tools (standard behavior) 1`] = `""`; - exports[` > Golden Snapshots > renders empty tool calls array 1`] = `""`; exports[` > Golden Snapshots > renders header when scrolled 1`] = ` @@ -119,14 +115,6 @@ exports[` > Golden Snapshots > renders two tool groups where ╰──────────────────────────────────────────────────────────────────────────╯ █" `; -exports[` > Golden Snapshots > renders when not focused 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────╮ -│ ✓ test-tool A tool for testing │ -│ │ -│ Test result │ -╰──────────────────────────────────────────────────────────────────────────╯" -`; - exports[` > Golden Snapshots > renders with limited terminal height 1`] = ` "╭──────────────────────────────────────────────────────────────────────────╮ │ ✓ tool-with-result Tool with output │