diff --git a/GEMINI.md b/GEMINI.md index 42366ace2b..ed9ba8ac25 100644 --- a/GEMINI.md +++ b/GEMINI.md @@ -62,6 +62,14 @@ powerful tool for developers. - **Imports:** Use specific imports and avoid restricted relative imports between packages (enforced by ESLint). +## Testing Conventions + +- **Environment Variables:** When testing code that depends on environment + variables, use `vi.stubEnv('NAME', 'value')` in `beforeEach` and + `vi.unstubAllEnvs()` in `afterEach`. Avoid modifying `process.env` directly as + it can lead to test leakage and is less reliable. To "unset" a variable, use + an empty string `vi.stubEnv('NAME', '')`. + ## Documentation - Suggest documentation updates when code changes render existing documentation diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 54e528deaf..a7f90aecfe 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -9,6 +9,7 @@ import { Box } from 'ink'; import type React from 'react'; import { vi } from 'vitest'; import { act, useState } from 'react'; +import os from 'node:os'; import { LoadedSettings, type Settings } from '../config/settings.js'; import { KeypressProvider } from '../ui/contexts/KeypressContext.js'; import { SettingsContext } from '../ui/contexts/SettingsContext.js'; @@ -27,8 +28,9 @@ import { import { type HistoryItemToolGroup, StreamingState } from '../ui/types.js'; import { ToolActionsProvider } from '../ui/contexts/ToolActionsContext.js'; -import { type Config } from '@google/gemini-cli-core'; +import { makeFakeConfig, type Config } from '@google/gemini-cli-core'; import { FakePersistentState } from './persistentStateFake.js'; +import { AppContext, type AppState } from '../ui/contexts/AppContext.js'; export const persistentStateMock = new FakePersistentState(); @@ -91,21 +93,27 @@ export const simulateClick = async ( }); }; -const mockConfig = { - getModel: () => 'gemini-pro', - getTargetDir: () => - '/Users/test/project/foo/bar/and/some/more/directories/to/make/it/long', - getDebugMode: () => false, - isTrustedFolder: () => true, - getIdeMode: () => false, - getEnableInteractiveShell: () => true, - getPreviewFeatures: () => false, +let mockConfigInternal: Config | undefined; + +const getMockConfigInternal = (): Config => { + if (!mockConfigInternal) { + mockConfigInternal = makeFakeConfig({ + targetDir: os.tmpdir(), + enableEventDrivenScheduler: true, + }); + } + return mockConfigInternal; }; -const configProxy = new Proxy(mockConfig, { - get(target, prop) { - if (prop in target) { - return target[prop as keyof typeof target]; +const configProxy = new Proxy({} as Config, { + get(_target, prop) { + if (prop === 'getTargetDir') { + return () => + '/Users/test/project/foo/bar/and/some/more/directories/to/make/it/long'; + } + const internal = getMockConfigInternal(); + if (prop in internal) { + return internal[prop as keyof typeof internal]; } throw new Error(`mockConfig does not have property ${String(prop)}`); }, @@ -146,6 +154,11 @@ const baseMockUiState = { terminalBackgroundColor: undefined, }; +export const mockAppState: AppState = { + version: '1.2.3', + startupWarnings: [], +}; + const mockUIActions: UIActions = { handleThemeSelect: vi.fn(), closeThemeDialog: vi.fn(), @@ -199,6 +212,7 @@ export const renderWithProviders = ( useAlternateBuffer = true, uiActions, persistentState, + appState = mockAppState, }: { shellFocus?: boolean; settings?: LoadedSettings; @@ -212,6 +226,7 @@ export const renderWithProviders = ( get?: typeof persistentStateMock.get; set?: typeof persistentStateMock.set; }; + appState?: AppState; } = {}, ): ReturnType & { simulateClick: typeof simulateClick } => { const baseState: UIState = new Proxy( @@ -268,36 +283,41 @@ export const renderWithProviders = ( .flatMap((item) => item.tools); const renderResult = render( - - - - - - - - - - - - - {component} - - - - - - - - - - - - , + + + + + + + + + + + + + + {component} + + + + + + + + + + + + + , terminalWidth, ); diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index 0f806702ea..8bbdc0db60 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -4,17 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, type Mock } from 'vitest'; -import { render } from '../test-utils/render.js'; -import { Text, useIsScreenReaderEnabled } from 'ink'; -import { makeFakeConfig } from '@google/gemini-cli-core'; +import { describe, it, expect, vi, type Mock, beforeEach } from 'vitest'; +import type React from 'react'; +import { renderWithProviders } from '../test-utils/render.js'; +import { Text, useIsScreenReaderEnabled, type DOMElement } from 'ink'; +import { type Config } from '@google/gemini-cli-core'; import { App } from './App.js'; -import { UIStateContext, type UIState } from './contexts/UIStateContext.js'; -import { StreamingState } from './types.js'; -import { ConfigContext } from './contexts/ConfigContext.js'; -import { AppContext, type AppState } from './contexts/AppContext.js'; -import { SettingsContext } from './contexts/SettingsContext.js'; -import { LoadedSettings, type SettingsFile } from '../config/settings.js'; +import { type UIState } from './contexts/UIStateContext.js'; +import { StreamingState, ToolCallStatus } from './types.js'; +import { makeFakeConfig } from '@google/gemini-cli-core'; vi.mock('ink', async (importOriginal) => { const original = await importOriginal(); @@ -53,12 +51,20 @@ vi.mock('./components/Footer.js', () => ({ })); describe('App', () => { + beforeEach(() => { + (useIsScreenReaderEnabled as Mock).mockReturnValue(false); + }); + const mockUIState: Partial = { streamingState: StreamingState.Idle, quittingMessages: null, dialogsVisible: false, - mainControlsRef: { current: null }, - rootUiRef: { current: null }, + mainControlsRef: { + current: null, + } as unknown as React.MutableRefObject, + rootUiRef: { + current: null, + } as unknown as React.MutableRefObject, historyManager: { addItem: vi.fn(), history: [], @@ -68,49 +74,18 @@ describe('App', () => { }, history: [], pendingHistoryItems: [], + pendingGeminiHistoryItems: [], bannerData: { defaultText: 'Mock Banner Text', warningText: '', }, }; - const mockConfig = makeFakeConfig(); - - const mockSettingsFile: SettingsFile = { - settings: {}, - originalSettings: {}, - path: '/mock/path', - }; - - const mockLoadedSettings = new LoadedSettings( - mockSettingsFile, - mockSettingsFile, - mockSettingsFile, - mockSettingsFile, - true, - [], - ); - - const mockAppState: AppState = { - version: '1.0.0', - startupWarnings: [], - }; - - const renderWithProviders = (ui: React.ReactElement, state: UIState) => - render( - - - - - {ui} - - - - , - ); - it('should render main content and composer when not quitting', () => { - const { lastFrame } = renderWithProviders(, mockUIState as UIState); + const { lastFrame } = renderWithProviders(, { + uiState: mockUIState, + useAlternateBuffer: false, + }); expect(lastFrame()).toContain('MainContent'); expect(lastFrame()).toContain('Notifications'); @@ -123,7 +98,10 @@ describe('App', () => { quittingMessages: [{ id: 1, type: 'user', text: 'test' }], } as UIState; - const { lastFrame } = renderWithProviders(, quittingUIState); + const { lastFrame } = renderWithProviders(, { + uiState: quittingUIState, + useAlternateBuffer: false, + }); expect(lastFrame()).toContain('Quitting...'); }); @@ -136,15 +114,13 @@ describe('App', () => { pendingHistoryItems: [{ type: 'user', text: 'pending item' }], } as UIState; - mockLoadedSettings.merged.ui.useAlternateBuffer = true; - - const { lastFrame } = renderWithProviders(, quittingUIState); + const { lastFrame } = renderWithProviders(, { + uiState: quittingUIState, + useAlternateBuffer: true, + }); expect(lastFrame()).toContain('HistoryItemDisplay'); expect(lastFrame()).toContain('Quitting...'); - - // Reset settings - mockLoadedSettings.merged.ui.useAlternateBuffer = false; }); it('should render dialog manager when dialogs are visible', () => { @@ -153,7 +129,9 @@ describe('App', () => { dialogsVisible: true, } as UIState; - const { lastFrame } = renderWithProviders(, dialogUIState); + const { lastFrame } = renderWithProviders(, { + uiState: dialogUIState, + }); expect(lastFrame()).toContain('MainContent'); expect(lastFrame()).toContain('Notifications'); @@ -172,7 +150,9 @@ describe('App', () => { [stateKey]: true, } as UIState; - const { lastFrame } = renderWithProviders(, uiState); + const { lastFrame } = renderWithProviders(, { + uiState, + }); expect(lastFrame()).toContain(`Press Ctrl+${key} again to exit.`); }, @@ -181,37 +161,88 @@ describe('App', () => { it('should render ScreenReaderAppLayout when screen reader is enabled', () => { (useIsScreenReaderEnabled as Mock).mockReturnValue(true); - const { lastFrame } = renderWithProviders(, mockUIState as UIState); + const { lastFrame } = renderWithProviders(, { + uiState: mockUIState, + }); - expect(lastFrame()).toContain( - 'Notifications\nFooter\nMainContent\nComposer', - ); + expect(lastFrame()).toContain(`Notifications +Footer +MainContent +Composer`); }); it('should render DefaultAppLayout when screen reader is not enabled', () => { (useIsScreenReaderEnabled as Mock).mockReturnValue(false); - const { lastFrame } = renderWithProviders(, mockUIState as UIState); + const { lastFrame } = renderWithProviders(, { + uiState: mockUIState, + }); - expect(lastFrame()).toContain('MainContent\nNotifications\nComposer'); + expect(lastFrame()).toContain(`MainContent +Notifications +Composer`); + }); + + it('should render ToolConfirmationQueue along with Composer when tool is confirming and experiment is on', () => { + (useIsScreenReaderEnabled as Mock).mockReturnValue(false); + + const toolCalls = [ + { + callId: 'call-1', + name: 'ls', + description: 'list directory', + status: ToolCallStatus.Confirming, + resultDisplay: '', + confirmationDetails: { + type: 'exec' as const, + title: 'Confirm execution', + command: 'ls', + rootCommand: 'ls', + rootCommands: ['ls'], + }, + }, + ]; + + const stateWithConfirmingTool = { + ...mockUIState, + pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], + pendingGeminiHistoryItems: [{ type: 'tool_group', tools: toolCalls }], + } as UIState; + + const configWithExperiment = { + ...makeFakeConfig(), + isEventDrivenSchedulerEnabled: () => true, + isTrustedFolder: () => true, + getIdeMode: () => false, + } as unknown as Config; + + const { lastFrame } = renderWithProviders(, { + uiState: stateWithConfirmingTool, + config: configWithExperiment, + }); + + expect(lastFrame()).toContain('MainContent'); + expect(lastFrame()).toContain('Notifications'); + expect(lastFrame()).toContain('Action Required'); // From ToolConfirmationQueue + expect(lastFrame()).toContain('1 of 1'); + expect(lastFrame()).toContain('Composer'); + expect(lastFrame()).toMatchSnapshot(); }); describe('Snapshots', () => { it('renders default layout correctly', () => { (useIsScreenReaderEnabled as Mock).mockReturnValue(false); - const { lastFrame } = renderWithProviders( - , - mockUIState as UIState, - ); + const { lastFrame } = renderWithProviders(, { + uiState: mockUIState, + }); expect(lastFrame()).toMatchSnapshot(); }); it('renders screen reader layout correctly', () => { (useIsScreenReaderEnabled as Mock).mockReturnValue(true); - const { lastFrame } = renderWithProviders( - , - mockUIState as UIState, - ); + const { lastFrame } = renderWithProviders(, { + uiState: mockUIState, + }); expect(lastFrame()).toMatchSnapshot(); }); @@ -220,7 +251,9 @@ describe('App', () => { ...mockUIState, dialogsVisible: true, } as UIState; - const { lastFrame } = renderWithProviders(, dialogUIState); + const { lastFrame } = renderWithProviders(, { + uiState: dialogUIState, + }); expect(lastFrame()).toMatchSnapshot(); }); }); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 70cf7b6cb1..43553efe14 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1010,6 +1010,7 @@ Logging in with Google... Restarting Gemini CLI to continue. * - Any future streaming states not explicitly allowed */ const isInputActive = + isConfigInitialized && !initError && !isProcessing && !!slashCommands && diff --git a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap index c91912df21..07103d2e0c 100644 --- a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap +++ b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap @@ -3,7 +3,44 @@ exports[`App > Snapshots > renders default layout correctly 1`] = ` "MainContent Notifications -Composer" +Composer + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +" `; exports[`App > Snapshots > renders screen reader layout correctly 1`] = ` @@ -14,8 +51,87 @@ Composer" `; exports[`App > Snapshots > renders with dialogs visible 1`] = ` -"Notifications -Footer -MainContent -DialogManager" +"MainContent +Notifications +DialogManager + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +" +`; + +exports[`App > should render ToolConfirmationQueue along with Composer when tool is confirming and experiment is on 1`] = ` +"MainContent +Notifications +╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ Action Required 1 of 1 │ +│ │ +│ ? ls list directory │ +│ │ +│ ls │ +│ │ +│ Allow execution of: 'ls'? │ +│ │ +│ ● 1. Allow once │ +│ 2. Allow for this session │ +│ 3. No, suggest changes (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ +Composer + + + + + + + + + + + + + + + + + + + + + + +" `; diff --git a/packages/cli/src/ui/auth/AuthDialog.test.tsx b/packages/cli/src/ui/auth/AuthDialog.test.tsx index 6757979c42..b71d2cd2d2 100644 --- a/packages/cli/src/ui/auth/AuthDialog.test.tsx +++ b/packages/cli/src/ui/auth/AuthDialog.test.tsx @@ -74,11 +74,12 @@ describe('AuthDialog', () => { onAuthError: (error: string | null) => void; setAuthContext: (context: { requiresRestart?: boolean }) => void; }; - const originalEnv = { ...process.env }; - beforeEach(() => { vi.resetAllMocks(); - process.env = {}; + vi.stubEnv('CLOUD_SHELL', undefined as unknown as string); + vi.stubEnv('GEMINI_CLI_USE_COMPUTE_ADC', undefined as unknown as string); + vi.stubEnv('GEMINI_DEFAULT_AUTH_TYPE', undefined as unknown as string); + vi.stubEnv('GEMINI_API_KEY', undefined as unknown as string); props = { config: { @@ -100,7 +101,7 @@ describe('AuthDialog', () => { }); afterEach(() => { - process.env = originalEnv; + vi.unstubAllEnvs(); }); describe('Environment Variable Effects on Auth Options', () => { @@ -138,7 +139,9 @@ describe('AuthDialog', () => { ])( 'correctly shows/hides COMPUTE_ADC options $desc', ({ env, shouldContain, shouldNotContain }) => { - process.env = { ...env }; + for (const [key, value] of Object.entries(env)) { + vi.stubEnv(key, value as string); + } renderWithProviders(); const items = mockedRadioButtonSelect.mock.calls[0][0].items; for (const item of shouldContain) { @@ -178,14 +181,14 @@ describe('AuthDialog', () => { }, { setup: () => { - process.env['GEMINI_DEFAULT_AUTH_TYPE'] = AuthType.USE_GEMINI; + vi.stubEnv('GEMINI_DEFAULT_AUTH_TYPE', AuthType.USE_GEMINI); }, expected: AuthType.USE_GEMINI, desc: 'from GEMINI_DEFAULT_AUTH_TYPE env var', }, { setup: () => { - process.env['GEMINI_API_KEY'] = 'test-key'; + vi.stubEnv('GEMINI_API_KEY', 'test-key'); }, expected: AuthType.USE_GEMINI, desc: 'from GEMINI_API_KEY env var', @@ -243,7 +246,7 @@ describe('AuthDialog', () => { it('skips API key dialog on initial setup if env var is present', async () => { mockedValidateAuthMethod.mockReturnValue(null); - process.env['GEMINI_API_KEY'] = 'test-key-from-env'; + vi.stubEnv('GEMINI_API_KEY', 'test-key-from-env'); // props.settings.merged.security.auth.selectedType is undefined here, simulating initial setup renderWithProviders(); @@ -258,7 +261,7 @@ describe('AuthDialog', () => { it('skips API key dialog if env var is present but empty', async () => { mockedValidateAuthMethod.mockReturnValue(null); - process.env['GEMINI_API_KEY'] = ''; // Empty string + vi.stubEnv('GEMINI_API_KEY', ''); // Empty string // props.settings.merged.security.auth.selectedType is undefined here renderWithProviders(); @@ -288,7 +291,7 @@ describe('AuthDialog', () => { it('skips API key dialog on re-auth if env var is present (cannot edit)', async () => { mockedValidateAuthMethod.mockReturnValue(null); - process.env['GEMINI_API_KEY'] = 'test-key-from-env'; + vi.stubEnv('GEMINI_API_KEY', 'test-key-from-env'); // Simulate that the user has already authenticated once props.settings.merged.security.auth.selectedType = AuthType.LOGIN_WITH_GOOGLE; diff --git a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx index 521c088ea2..68b662df7b 100644 --- a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx +++ b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx @@ -13,17 +13,21 @@ import { AlternateBufferQuittingDisplay } from './AlternateBufferQuittingDisplay import { ToolCallStatus } from '../types.js'; import type { HistoryItem, HistoryItemWithoutId } from '../types.js'; import { Text } from 'ink'; -import type { Config } from '@google/gemini-cli-core'; vi.mock('../utils/terminalSetup.js', () => ({ getTerminalProgram: () => null, })); -vi.mock('../contexts/AppContext.js', () => ({ - useAppContext: () => ({ - version: '0.10.0', - }), -})); +vi.mock('../contexts/AppContext.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + useAppContext: () => ({ + version: '0.10.0', + }), + }; +}); vi.mock('@google/gemini-cli-core', async (importOriginal) => { const actual = @@ -85,21 +89,6 @@ const mockPendingHistoryItems: HistoryItemWithoutId[] = [ }, ]; -const mockConfig = { - getScreenReader: () => false, - getEnableInteractiveShell: () => false, - getModel: () => 'gemini-pro', - getTargetDir: () => '/tmp', - getDebugMode: () => false, - getIdeMode: () => false, - getGeminiMdFileCount: () => 0, - getExperiments: () => ({ - flags: {}, - experimentIds: [], - }), - getPreviewFeatures: () => false, -} as unknown as Config; - describe('AlternateBufferQuittingDisplay', () => { beforeEach(() => { vi.clearAllMocks(); @@ -127,7 +116,6 @@ describe('AlternateBufferQuittingDisplay', () => { history: mockHistory, pendingHistoryItems: mockPendingHistoryItems, }, - config: mockConfig, }, ); expect(lastFrame()).toMatchSnapshot('with_history_and_pending'); @@ -143,7 +131,6 @@ describe('AlternateBufferQuittingDisplay', () => { history: [], pendingHistoryItems: [], }, - config: mockConfig, }, ); expect(lastFrame()).toMatchSnapshot('empty'); @@ -159,7 +146,6 @@ describe('AlternateBufferQuittingDisplay', () => { history: mockHistory, pendingHistoryItems: [], }, - config: mockConfig, }, ); expect(lastFrame()).toMatchSnapshot('with_history_no_pending'); @@ -175,12 +161,50 @@ describe('AlternateBufferQuittingDisplay', () => { history: [], pendingHistoryItems: mockPendingHistoryItems, }, - config: mockConfig, }, ); expect(lastFrame()).toMatchSnapshot('with_pending_no_history'); }); + it('renders with a tool awaiting confirmation', () => { + persistentStateMock.setData({ tipsShown: 0 }); + const pendingHistoryItems: HistoryItemWithoutId[] = [ + { + type: 'tool_group', + tools: [ + { + callId: 'call4', + name: 'confirming_tool', + description: 'Confirming tool description', + status: ToolCallStatus.Confirming, + resultDisplay: undefined, + confirmationDetails: { + type: 'info', + title: 'Confirm Tool', + prompt: 'Confirm this action?', + onConfirm: async () => {}, + }, + }, + ], + }, + ]; + const { lastFrame } = renderWithProviders( + , + { + uiState: { + ...baseUIState, + history: [], + pendingHistoryItems, + }, + }, + ); + const output = lastFrame(); + expect(output).toContain('Action Required (was prompted):'); + expect(output).toContain('confirming_tool'); + expect(output).toContain('Confirming tool description'); + expect(output).toMatchSnapshot('with_confirming_tool'); + }); + it('renders with user and gemini messages', () => { persistentStateMock.setData({ tipsShown: 0 }); const history: HistoryItem[] = [ @@ -195,7 +219,6 @@ describe('AlternateBufferQuittingDisplay', () => { history, pendingHistoryItems: [], }, - config: mockConfig, }, ); expect(lastFrame()).toMatchSnapshot('with_user_gemini_messages'); diff --git a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx index 0defa735e4..fec35d46c3 100644 --- a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx +++ b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.tsx @@ -4,17 +4,26 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Box } from 'ink'; +import { Box, Text } from 'ink'; import { useUIState } from '../contexts/UIStateContext.js'; import { AppHeader } from './AppHeader.js'; import { HistoryItemDisplay } from './HistoryItemDisplay.js'; 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; // 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 @@ -52,6 +61,25 @@ export const AlternateBufferQuittingDisplay = () => { embeddedShellFocused={uiState.embeddedShellFocused} /> ))} + {showPromptedTool && ( + + + Action Required (was prompted): + + + + + + + )} ); diff --git a/packages/cli/src/ui/components/Composer.tsx b/packages/cli/src/ui/components/Composer.tsx index 4fca6e8b0b..9a550a323e 100644 --- a/packages/cli/src/ui/components/Composer.tsx +++ b/packages/cli/src/ui/components/Composer.tsx @@ -29,7 +29,7 @@ import { StreamingState } from '../types.js'; import { ConfigInitDisplay } from '../components/ConfigInitDisplay.js'; import { TodoTray } from './messages/Todo.js'; -export const Composer = () => { +export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => { const config = useConfig(); const settings = useSettings(); const isScreenReaderEnabled = useIsScreenReaderEnabled(); @@ -133,7 +133,7 @@ export const Composer = () => { setShellModeActive={uiActions.setShellModeActive} approvalMode={showApprovalModeIndicator} onEscapePromptChange={uiActions.onEscapePromptChange} - focus={true} + focus={isFocused} vimHandleInput={uiActions.vimHandleInput} isEmbeddedShellFocused={uiState.embeddedShellFocused} popAllMessages={uiActions.popAllMessages} diff --git a/packages/cli/src/ui/components/ContextUsageDisplay.test.tsx b/packages/cli/src/ui/components/ContextUsageDisplay.test.tsx index caa81ee968..4183090559 100644 --- a/packages/cli/src/ui/components/ContextUsageDisplay.test.tsx +++ b/packages/cli/src/ui/components/ContextUsageDisplay.test.tsx @@ -8,9 +8,14 @@ import { render } from '../../test-utils/render.js'; import { ContextUsageDisplay } from './ContextUsageDisplay.js'; import { describe, it, expect, vi } from 'vitest'; -vi.mock('@google/gemini-cli-core', () => ({ - tokenLimit: () => 10000, -})); +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + tokenLimit: () => 10000, + }; +}); vi.mock('../../config/settings.js', () => ({ DEFAULT_MODEL_CONFIGS: {}, diff --git a/packages/cli/src/ui/components/Notifications.test.tsx b/packages/cli/src/ui/components/Notifications.test.tsx index df35ac02ab..6870eb0373 100644 --- a/packages/cli/src/ui/components/Notifications.test.tsx +++ b/packages/cli/src/ui/components/Notifications.test.tsx @@ -33,11 +33,17 @@ vi.mock('node:fs/promises', async () => { unlink: vi.fn().mockResolvedValue(undefined), }; }); -vi.mock('node:os', () => ({ - default: { +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + default: { + ...actual, + homedir: () => '/mock/home', + }, homedir: () => '/mock/home', - }, -})); + }; +}); vi.mock('node:path', async () => { const actual = await vi.importActual('node:path'); @@ -47,13 +53,19 @@ vi.mock('node:path', async () => { }; }); -vi.mock('@google/gemini-cli-core', () => ({ - GEMINI_DIR: '.gemini', - homedir: () => '/mock/home', - Storage: { - getGlobalTempDir: () => '/mock/temp', - }, -})); +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + GEMINI_DIR: '.gemini', + homedir: () => '/mock/home', + Storage: { + ...actual.Storage, + getGlobalTempDir: () => '/mock/temp', + }, + }; +}); vi.mock('../../config/settings.js', () => ({ DEFAULT_MODEL_CONFIGS: {}, diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx new file mode 100644 index 0000000000..9b1858217f --- /dev/null +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx @@ -0,0 +1,89 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ToolConfirmationQueue } from './ToolConfirmationQueue.js'; +import { ToolCallStatus } from '../types.js'; +import { renderWithProviders } from '../../test-utils/render.js'; +import type { Config } from '@google/gemini-cli-core'; +import type { ConfirmingToolState } from '../hooks/useConfirmingTool.js'; + +describe('ToolConfirmationQueue', () => { + const mockConfig = { + isTrustedFolder: () => true, + getIdeMode: () => false, + getModel: () => 'gemini-pro', + getDebugMode: () => false, + } as unknown as Config; + + it('renders the confirming tool with progress indicator', () => { + const confirmingTool = { + tool: { + callId: 'call-1', + name: 'ls', + description: 'list files', + status: ToolCallStatus.Confirming, + confirmationDetails: { + type: 'exec' as const, + title: 'Confirm execution', + command: 'ls', + rootCommand: 'ls', + rootCommands: ['ls'], + onConfirm: vi.fn(), + }, + }, + index: 1, + total: 3, + }; + + const { lastFrame } = renderWithProviders( + , + { + config: mockConfig, + uiState: { + terminalWidth: 80, + }, + }, + ); + + const output = lastFrame(); + expect(output).toContain('Action Required'); + 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).toMatchSnapshot(); + }); + + it('returns null if tool has no confirmation details', () => { + const confirmingTool = { + tool: { + callId: 'call-1', + name: 'ls', + status: ToolCallStatus.Confirming, + confirmationDetails: undefined, + }, + index: 1, + total: 1, + }; + + const { lastFrame } = renderWithProviders( + , + { + config: mockConfig, + uiState: { + terminalWidth: 80, + }, + }, + ); + + expect(lastFrame()).toBe(''); + }); +}); diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx new file mode 100644 index 0000000000..9e378fb5f6 --- /dev/null +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx @@ -0,0 +1,89 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { Box, Text } from 'ink'; +import { theme } from '../semantic-colors.js'; +import { useConfig } from '../contexts/ConfigContext.js'; +import { ToolConfirmationMessage } from './messages/ToolConfirmationMessage.js'; +import { ToolStatusIndicator, ToolInfo } from './messages/ToolShared.js'; +import { useUIState } from '../contexts/UIStateContext.js'; +import type { ConfirmingToolState } from '../hooks/useConfirmingTool.js'; + +interface ToolConfirmationQueueProps { + confirmingTool: ConfirmingToolState; +} + +export const ToolConfirmationQueue: React.FC = ({ + confirmingTool, +}) => { + const config = useConfig(); + const { terminalWidth, terminalHeight } = useUIState(); + const { tool, index, total } = confirmingTool; + + // Safety check: ToolConfirmationMessage requires confirmationDetails + if (!tool.confirmationDetails) return null; + + // V1: Constrain the queue to at most 50% of the terminal height to ensure + // some history is always visible and to prevent flickering. + // We pass this to ToolConfirmationMessage so it can calculate internal + // truncation while keeping buttons visible. + const maxHeight = Math.floor(terminalHeight * 0.5); + + // ToolConfirmationMessage needs to know the height available for its OWN content. + // We subtract the lines used by the Queue wrapper: + // - 2 lines for the rounded border + // - 2 lines for the Header (text + margin) + // - 2 lines for Tool Identity (text + margin) + const availableContentHeight = Math.max(maxHeight - 6, 4); + + return ( + + {/* Header */} + + + Action Required + + + {index} of {total} + + + + {/* Tool Identity (Context) */} + + + + + + {/* Interactive Area */} + {/* + Note: We force isFocused={true} because if this component is rendered, + it effectively acts as a modal over the shell/composer. + */} + + + ); +}; diff --git a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap index 75debcab74..fa02687659 100644 --- a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap @@ -1,5 +1,28 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`AlternateBufferQuittingDisplay > renders with a tool awaiting confirmation > with_confirming_tool 1`] = ` +" + ███ █████████ +░░░███ ███░░░░░███ + ░░░███ ███ ░░░ + ░░░███░███ + ███░ ░███ █████ + ███░ ░░███ ░░███ + ███░ ░░█████████ +░░░ ░░░░░░░░░ + +Tips for getting started: +1. Ask questions, edit files, or run commands. +2. Be specific for the best results. +3. Create GEMINI.md files to customize your interactions with Gemini. +4. /help for more information. + +Action Required (was prompted): + +? confirming_tool Confirming tool description +" +`; + exports[`AlternateBufferQuittingDisplay > renders with active and pending tool messages > with_history_and_pending 1`] = ` " ███ █████████ @@ -23,10 +46,6 @@ Tips for getting started: ╭─────────────────────────────────────────────────────────────────────────────╮ │ ✓ tool2 Description for tool 2 │ │ │ -╰─────────────────────────────────────────────────────────────────────────────╯ -╭─────────────────────────────────────────────────────────────────────────────╮ -│ o tool3 Description for tool 3 │ -│ │ ╰─────────────────────────────────────────────────────────────────────────────╯" `; @@ -89,11 +108,7 @@ Tips for getting started: 1. Ask questions, edit files, or run commands. 2. Be specific for the best results. 3. Create GEMINI.md files to customize your interactions with Gemini. -4. /help for more information. -╭─────────────────────────────────────────────────────────────────────────────╮ -│ o tool3 Description for tool 3 │ -│ │ -╰─────────────────────────────────────────────────────────────────────────────╯" +4. /help for more information." `; exports[`AlternateBufferQuittingDisplay > renders with user and gemini messages > with_user_gemini_messages 1`] = ` diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap new file mode 100644 index 0000000000..57ccfb8df3 --- /dev/null +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -0,0 +1,18 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ToolConfirmationQueue > renders the confirming tool with progress indicator 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ Action Required 1 of 3 │ +│ │ +│ ? ls list files │ +│ │ +│ ls │ +│ │ +│ Allow execution of: 'ls'? │ +│ │ +│ ● 1. Allow once │ +│ 2. Allow for this session │ +│ 3. No, suggest changes (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯" +`; diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 12521b472a..3717c2a5b0 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -5,7 +5,7 @@ */ import type React from 'react'; -import { useMemo } from 'react'; +import { useMemo, useCallback } from 'react'; import { Box, Text } from 'ink'; import { DiffRenderer } from './DiffRenderer.js'; import { RenderInline } from '../../utils/InlineMarkdownRenderer.js'; @@ -58,14 +58,17 @@ export const ToolConfirmationMessage: React.FC< const allowPermanentApproval = settings.merged.security.enablePermanentToolApproval; - const handleConfirm = (outcome: ToolConfirmationOutcome) => { - void confirm(callId, outcome).catch((error) => { - debugLogger.error( - `Failed to handle tool confirmation for ${callId}:`, - error, - ); - }); - }; + const handleConfirm = useCallback( + (outcome: ToolConfirmationOutcome) => { + void confirm(callId, outcome).catch((error) => { + debugLogger.error( + `Failed to handle tool confirmation for ${callId}:`, + error, + ); + }); + }, + [confirm, callId], + ); const isTrustedFolder = config.isTrustedFolder(); @@ -79,16 +82,16 @@ export const ToolConfirmationMessage: React.FC< { isActive: isFocused }, ); - const handleSelect = (item: ToolConfirmationOutcome) => handleConfirm(item); + const handleSelect = useCallback( + (item: ToolConfirmationOutcome) => handleConfirm(item), + [handleConfirm], + ); - const { question, bodyContent, options } = useMemo(() => { - let bodyContent: React.ReactNode | null = null; - let question = ''; + const getOptions = useCallback(() => { const options: Array> = []; if (confirmationDetails.type === 'edit') { if (!confirmationDetails.isModifying) { - question = `Apply this change?`; options.push({ label: 'Allow once', value: ToolConfirmationOutcome.ProceedOnce, @@ -125,13 +128,6 @@ export const ToolConfirmationMessage: React.FC< }); } } else if (confirmationDetails.type === 'exec') { - const executionProps = confirmationDetails; - - if (executionProps.commands && executionProps.commands.length > 1) { - question = `Allow execution of ${executionProps.commands.length} commands?`; - } else { - question = `Allow execution of: '${executionProps.rootCommand}'?`; - } options.push({ label: 'Allow once', value: ToolConfirmationOutcome.ProceedOnce, @@ -157,7 +153,6 @@ export const ToolConfirmationMessage: React.FC< key: 'No, suggest changes (esc)', }); } else if (confirmationDetails.type === 'info') { - question = `Do you want to proceed?`; options.push({ label: 'Allow once', value: ToolConfirmationOutcome.ProceedOnce, @@ -184,8 +179,6 @@ export const ToolConfirmationMessage: React.FC< }); } else { // mcp tool confirmation - const mcpProps = confirmationDetails; - question = `Allow execution of MCP tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"?`; options.push({ label: 'Allow once', value: ToolConfirmationOutcome.ProceedOnce, @@ -216,33 +209,56 @@ export const ToolConfirmationMessage: React.FC< key: 'No, suggest changes (esc)', }); } + return options; + }, [confirmationDetails, isTrustedFolder, allowPermanentApproval, config]); - function availableBodyContentHeight() { - if (options.length === 0) { - // Should not happen if we populated options correctly above for all types - // except when isModifying is true, but in that case we don't call this because we don't enter the if block for it. - return undefined; + const availableBodyContentHeight = useCallback(() => { + if (availableTerminalHeight === undefined) { + return undefined; + } + + // Calculate the vertical space (in lines) consumed by UI elements + // surrounding the main body content. + const PADDING_OUTER_Y = 2; // Main container has `padding={1}` (top & bottom). + const MARGIN_BODY_BOTTOM = 1; // margin on the body container. + const HEIGHT_QUESTION = 1; // The question text is one line. + const MARGIN_QUESTION_BOTTOM = 1; // Margin on the question container. + + const optionsCount = getOptions().length; + + const surroundingElementsHeight = + PADDING_OUTER_Y + + MARGIN_BODY_BOTTOM + + HEIGHT_QUESTION + + MARGIN_QUESTION_BOTTOM + + optionsCount; + + return Math.max(availableTerminalHeight - surroundingElementsHeight, 1); + }, [availableTerminalHeight, getOptions]); + + const { question, bodyContent, options } = useMemo(() => { + let bodyContent: React.ReactNode | null = null; + let question = ''; + const options = getOptions(); + + if (confirmationDetails.type === 'edit') { + if (!confirmationDetails.isModifying) { + question = `Apply this change?`; } + } else if (confirmationDetails.type === 'exec') { + const executionProps = confirmationDetails; - if (availableTerminalHeight === undefined) { - return undefined; + if (executionProps.commands && executionProps.commands.length > 1) { + question = `Allow execution of ${executionProps.commands.length} commands?`; + } else { + question = `Allow execution of: '${executionProps.rootCommand}'?`; } - - // Calculate the vertical space (in lines) consumed by UI elements - // surrounding the main body content. - const PADDING_OUTER_Y = 2; // Main container has `padding={1}` (top & bottom). - const MARGIN_BODY_BOTTOM = 1; // margin on the body container. - const HEIGHT_QUESTION = 1; // The question text is one line. - const MARGIN_QUESTION_BOTTOM = 1; // Margin on the question container. - const HEIGHT_OPTIONS = options.length; // Each option in the radio select takes one line. - - const surroundingElementsHeight = - PADDING_OUTER_Y + - MARGIN_BODY_BOTTOM + - HEIGHT_QUESTION + - MARGIN_QUESTION_BOTTOM + - HEIGHT_OPTIONS; - return Math.max(availableTerminalHeight - surroundingElementsHeight, 1); + } else if (confirmationDetails.type === 'info') { + question = `Do you want to proceed?`; + } else { + // mcp tool confirmation + const mcpProps = confirmationDetails; + question = `Allow execution of MCP tool "${mcpProps.toolName}" from server "${mcpProps.serverName}"?`; } if (confirmationDetails.type === 'edit') { @@ -376,11 +392,9 @@ export const ToolConfirmationMessage: React.FC< return { question, bodyContent, options }; }, [ confirmationDetails, - isTrustedFolder, - config, - availableTerminalHeight, + getOptions, + availableBodyContentHeight, terminalWidth, - allowPermanentApproval, ]); if (confirmationDetails.type === 'edit') { @@ -409,7 +423,13 @@ export const ToolConfirmationMessage: React.FC< {/* Body Content (Diff Renderer or Command Info) */} {/* No separate context display here anymore for edits */} - {bodyContent} + + {bodyContent} + {/* Confirmation Question */} diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 3f61959440..61ee78f498 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -13,6 +13,7 @@ import { ToolGroupMessage } from './ToolGroupMessage.js'; import type { IndividualToolCallDisplay } from '../../types.js'; import { ToolCallStatus } from '../../types.js'; import { Scrollable } from '../shared/Scrollable.js'; +import type { Config } from '@google/gemini-cli-core'; describe('', () => { const createToolCall = ( @@ -34,12 +35,24 @@ describe('', () => { isFocused: true, }; + const baseMockConfig = { + getModel: () => 'gemini-pro', + getTargetDir: () => '/test', + getDebugMode: () => false, + isTrustedFolder: () => true, + getIdeMode: () => false, + getEnableInteractiveShell: () => true, + getPreviewFeatures: () => false, + isEventDrivenSchedulerEnabled: () => true, + } as unknown as Config; + describe('Golden Snapshots', () => { it('renders single successful tool call', () => { const toolCalls = [createToolCall()]; const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -70,9 +83,15 @@ describe('', () => { status: ToolCallStatus.Error, }), ]; + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -97,9 +116,15 @@ describe('', () => { }, }), ]; + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -121,6 +146,7 @@ describe('', () => { const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -151,9 +177,15 @@ describe('', () => { status: ToolCallStatus.Pending, }), ]; + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -186,6 +218,7 @@ describe('', () => { availableTerminalHeight={10} />, { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -204,6 +237,7 @@ describe('', () => { isFocused={false} />, { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -228,6 +262,7 @@ describe('', () => { terminalWidth={40} />, { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -241,6 +276,7 @@ describe('', () => { const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: [] }], }, @@ -271,6 +307,7 @@ describe('', () => { , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -293,6 +330,7 @@ describe('', () => { const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -326,6 +364,7 @@ describe('', () => { , { + config: baseMockConfig, uiState: { pendingHistoryItems: [ { type: 'tool_group', tools: toolCalls1 }, @@ -342,9 +381,15 @@ describe('', () => { describe('Border Color Logic', () => { it('uses yellow border when tools are pending', () => { const toolCalls = [createToolCall({ status: ToolCallStatus.Pending })]; + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -365,6 +410,7 @@ describe('', () => { const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -386,6 +432,7 @@ describe('', () => { const { lastFrame, unmount } = renderWithProviders( , { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -419,6 +466,7 @@ describe('', () => { availableTerminalHeight={20} />, { + config: baseMockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -455,9 +503,15 @@ describe('', () => { }, }), ]; + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -485,10 +539,16 @@ describe('', () => { const settings = createMockSettings({ security: { enablePermanentToolApproval: true }, }); + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , { settings, + config: mockConfig, uiState: { pendingHistoryItems: [{ type: 'tool_group', tools: toolCalls }], }, @@ -502,32 +562,100 @@ describe('', () => { it('renders confirmation with permanent approval disabled', () => { const toolCalls = [ createToolCall({ - callId: 'tool-1', + callId: 'confirm-tool', name: 'confirm-tool', status: ToolCallStatus.Confirming, confirmationDetails: { type: 'info', - title: 'Confirm Tool', + title: 'Confirm tool', prompt: 'Do you want to proceed?', onConfirm: vi.fn(), }, }), ]; - const settings = createMockSettings({ - security: { enablePermanentToolApproval: false }, - }); + + const mockConfig = { + ...baseMockConfig, + isEventDrivenSchedulerEnabled: () => false, + } as unknown as Config; + const { lastFrame, unmount } = renderWithProviders( , - { - settings, - uiState: { - pendingHistoryItems: [{ type: 'tool_group', tools: 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, + isEventDrivenSchedulerEnabled: () => true, + } as unknown as Config; + + 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, + isEventDrivenSchedulerEnabled: () => true, + } as unknown as Config; + + 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(); + }); + }); }); diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index ac6f36ad60..6c865640c3 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -36,7 +36,28 @@ export const ToolGroupMessage: React.FC = ({ activeShellPtyId, embeddedShellFocused, }) => { - const isEmbeddedShellFocused = toolCalls.some((t) => + const config = useConfig(); + + 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]); + + const isEmbeddedShellFocused = visibleToolCalls.some((t) => isThisShellFocused( t.name, t.status, @@ -46,11 +67,10 @@ export const ToolGroupMessage: React.FC = ({ ), ); - const hasPending = !toolCalls.every( + const hasPending = !visibleToolCalls.every( (t) => t.status === ToolCallStatus.Success, ); - const config = useConfig(); const isShellCommand = toolCalls.some((t) => isShellTool(t.name)); const borderColor = (isShellCommand && hasPending) || isEmbeddedShellFocused @@ -64,20 +84,29 @@ export const ToolGroupMessage: React.FC = ({ const staticHeight = /* border */ 2 + /* marginBottom */ 1; - // only prompt for tool approval on the first 'confirming' tool in the list - // note, after the CTA, this automatically moves over to the next 'confirming' tool + // Inline confirmations are ONLY used when the Global Queue is disabled. const toolAwaitingApproval = useMemo( - () => toolCalls.find((tc) => tc.status === ToolCallStatus.Confirming), - [toolCalls], + () => + isEventDriven + ? undefined + : toolCalls.find((tc) => tc.status === ToolCallStatus.Confirming), + [toolCalls, isEventDriven], ); + // If all tools are hidden (e.g. group only contains confirming or pending tools), + // render nothing in the history log. + if (visibleToolCalls.length === 0) { + return null; + } + let countToolCallsWithResults = 0; - for (const tool of toolCalls) { + for (const tool of visibleToolCalls) { if (tool.resultDisplay !== undefined && tool.resultDisplay !== '') { countToolCallsWithResults++; } } - const countOneLineToolCalls = toolCalls.length - countToolCallsWithResults; + const countOneLineToolCalls = + visibleToolCalls.length - countToolCallsWithResults; const availableTerminalHeightPerToolMessage = availableTerminalHeight ? Math.max( Math.floor( @@ -102,7 +131,7 @@ export const ToolGroupMessage: React.FC = ({ */ width={terminalWidth} > - {toolCalls.map((tool, index) => { + {visibleToolCalls.map((tool, index) => { const isConfirming = toolAwaitingApproval?.callId === tool.callId; const isFirst = index === 0; const isShellToolCall = isShellTool(tool.name); @@ -180,7 +209,7 @@ export const ToolGroupMessage: React.FC = ({ We have to keep the bottom border separate so it doesn't get drawn over by the sticky header directly inside it. */ - toolCalls.length > 0 && ( + visibleToolCalls.length > 0 && ( with folder trust > 'for edit confirmations' > should NOT show "allow always" when folder is untrusted 1`] = ` -"╭──────────────────────╮ -│ │ -│ No changes detected. │ -│ │ -╰──────────────────────╯ +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ No changes detected. │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯ Apply this change? @@ -54,11 +54,11 @@ Apply this change? `; exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations' > should show "allow always" when folder is trusted 1`] = ` -"╭──────────────────────╮ -│ │ -│ No changes detected. │ -│ │ -╰──────────────────────╯ +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ No changes detected. │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯ Apply this change? 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 6bea5eecd5..a8644b7536 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 @@ -81,6 +81,16 @@ exports[` > Confirmation Handling > shows confirmation dialo ╰──────────────────────────────────────────────────────────────────────────────╯" `; +exports[` > Event-Driven Scheduler > hides confirming tools when event-driven scheduler is enabled 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 > renders empty tool calls array 1`] = `""`; exports[` > Golden Snapshots > renders header when scrolled 1`] = ` diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.tsx index 46c2026c26..417625d998 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.tsx @@ -52,6 +52,7 @@ export const ToolActionsProvider: React.FC = ( props: ToolActionsProviderProps, ) => { const { children, config, toolCalls } = props; + // Hoist IdeClient logic here to keep UI pure const [ideClient, setIdeClient] = useState(null); useEffect(() => { @@ -124,7 +125,7 @@ export const ToolActionsProvider: React.FC = ( debugLogger.warn(`ToolActions: No confirmation mechanism for ${callId}`); }, - [config, toolCalls, ideClient], + [config, ideClient, toolCalls], ); const cancel = useCallback( diff --git a/packages/cli/src/ui/hooks/toolMapping.test.ts b/packages/cli/src/ui/hooks/toolMapping.test.ts index 16d518135f..41dc974adb 100644 --- a/packages/cli/src/ui/hooks/toolMapping.test.ts +++ b/packages/cli/src/ui/hooks/toolMapping.test.ts @@ -7,7 +7,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { mapCoreStatusToDisplayStatus, mapToDisplay } from './toolMapping.js'; import { - debugLogger, type AnyDeclarativeTool, type AnyToolInvocation, type ToolCallRequestInfo, @@ -40,7 +39,7 @@ describe('toolMapping', () => { describe('mapCoreStatusToDisplayStatus', () => { it.each([ - ['validating', ToolCallStatus.Executing], + ['validating', ToolCallStatus.Pending], ['awaiting_approval', ToolCallStatus.Confirming], ['executing', ToolCallStatus.Executing], ['success', ToolCallStatus.Success], @@ -53,12 +52,10 @@ describe('toolMapping', () => { ); }); - it('logs warning and defaults to Error for unknown status', () => { - const result = mapCoreStatusToDisplayStatus('unknown_status' as Status); - expect(result).toBe(ToolCallStatus.Error); - expect(debugLogger.warn).toHaveBeenCalledWith( - 'Unknown core status encountered: unknown_status', - ); + it('throws error for unknown status due to checkExhaustive', () => { + expect(() => + mapCoreStatusToDisplayStatus('unknown_status' as Status), + ).toThrow('unexpected value unknown_status!'); }); }); diff --git a/packages/cli/src/ui/hooks/toolMapping.ts b/packages/cli/src/ui/hooks/toolMapping.ts index 237044135f..f9c0a3eb19 100644 --- a/packages/cli/src/ui/hooks/toolMapping.ts +++ b/packages/cli/src/ui/hooks/toolMapping.ts @@ -18,12 +18,14 @@ import { type IndividualToolCallDisplay, } from '../types.js'; +import { checkExhaustive } from '../../utils/checks.js'; + export function mapCoreStatusToDisplayStatus( coreStatus: CoreStatus, ): ToolCallStatus { switch (coreStatus) { case 'validating': - return ToolCallStatus.Executing; + return ToolCallStatus.Pending; case 'awaiting_approval': return ToolCallStatus.Confirming; case 'executing': @@ -37,8 +39,7 @@ export function mapCoreStatusToDisplayStatus( case 'scheduled': return ToolCallStatus.Pending; default: - debugLogger.warn(`Unknown core status encountered: ${coreStatus}`); - return ToolCallStatus.Error; + return checkExhaustive(coreStatus); } } diff --git a/packages/cli/src/ui/hooks/useConfirmingTool.ts b/packages/cli/src/ui/hooks/useConfirmingTool.ts new file mode 100644 index 0000000000..115473ae9f --- /dev/null +++ b/packages/cli/src/ui/hooks/useConfirmingTool.ts @@ -0,0 +1,62 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useMemo } from 'react'; +import { useUIState } from '../contexts/UIStateContext.js'; +import { + ToolCallStatus, + type IndividualToolCallDisplay, + type HistoryItemToolGroup, +} from '../types.js'; + +export interface ConfirmingToolState { + tool: IndividualToolCallDisplay; + index: number; + total: number; +} + +/** + * Selects the "Head" of the confirmation queue. + * Returns the first tool in the pending state that requires confirmation. + */ +export function useConfirmingTool(): ConfirmingToolState | null { + // We use pendingHistoryItems to ensure we capture tools from both + // Gemini responses and Slash commands. + const { pendingHistoryItems } = useUIState(); + + return useMemo(() => { + // 1. Flatten all pending tools from all pending history groups + const allPendingTools = pendingHistoryItems + .filter( + (item): item is HistoryItemToolGroup => item.type === 'tool_group', + ) + .flatMap((group) => group.tools); + + // 2. Filter for those requiring confirmation + const confirmingTools = allPendingTools.filter( + (t) => t.status === ToolCallStatus.Confirming, + ); + + if (confirmingTools.length === 0) { + return null; + } + + // 3. Select Head (FIFO) + const head = confirmingTools[0]; + + // 4. Calculate progress based on the full tool list + // This gives the user context of where they are in the current batch. + const headIndexInFullList = allPendingTools.findIndex( + (t) => t.callId === head.callId, + ); + + return { + tool: head, + index: headIndexInFullList + 1, + total: allPendingTools.length, + }; + }, [pendingHistoryItems]); +} diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index b1f25bdea3..1978198b37 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -938,7 +938,7 @@ describe('mapToDisplay', () => { name: 'validating', status: 'validating', extraProps: { tool: baseTool, invocation: baseInvocation }, - expectedStatus: ToolCallStatus.Executing, + expectedStatus: ToolCallStatus.Pending, expectedName: baseTool.displayName, expectedDescription: baseInvocation.getDescription(), }, diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx index 2e83efdcb6..4189cf25ca 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx @@ -15,11 +15,21 @@ import { useUIState } from '../contexts/UIStateContext.js'; import { useFlickerDetector } from '../hooks/useFlickerDetector.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; import { CopyModeWarning } from '../components/CopyModeWarning.js'; +import { ToolConfirmationQueue } from '../components/ToolConfirmationQueue.js'; +import { useConfirmingTool } from '../hooks/useConfirmingTool.js'; +import { useConfig } from '../contexts/ConfigContext.js'; export const DefaultAppLayout: React.FC = () => { const uiState = useUIState(); + const config = useConfig(); const isAlternateBuffer = useAlternateBuffer(); + // If the event-driven scheduler is enabled AND we have a tool waiting, + // we switch the footer mode to "Queue". + const confirmingTool = useConfirmingTool(); + const showConfirmationQueue = + config.isEventDrivenSchedulerEnabled() && confirmingTool !== null; + const { rootUiRef, terminalHeight } = uiState; useFlickerDetector(rootUiRef, terminalHeight); // If in alternate buffer mode, need to leave room to draw the scrollbar on @@ -57,7 +67,12 @@ export const DefaultAppLayout: React.FC = () => { addItem={uiState.historyManager.addItem} /> ) : ( - + <> + {showConfirmationQueue && confirmingTool && ( + + )} + + )} diff --git a/packages/cli/src/ui/utils/terminalSetup.test.ts b/packages/cli/src/ui/utils/terminalSetup.test.ts index a6f7b290a7..1c565f1d7d 100644 --- a/packages/cli/src/ui/utils/terminalSetup.test.ts +++ b/packages/cli/src/ui/utils/terminalSetup.test.ts @@ -58,11 +58,12 @@ vi.mock('./terminalCapabilityManager.js', () => ({ })); describe('terminalSetup', () => { - const originalEnv = process.env; - beforeEach(() => { vi.resetAllMocks(); - process.env = { ...originalEnv }; + vi.stubEnv('TERM_PROGRAM', ''); + vi.stubEnv('CURSOR_TRACE_ID', ''); + vi.stubEnv('VSCODE_GIT_ASKPASS_MAIN', ''); + vi.stubEnv('VSCODE_GIT_IPC_HANDLE', ''); // Default mocks mocks.homedir.mockReturnValue('/home/user'); @@ -73,7 +74,7 @@ describe('terminalSetup', () => { }); afterEach(() => { - process.env = originalEnv; + vi.unstubAllEnvs(); }); describe('detectTerminal', () => {