From d165b6d4e7685ec4044b77a4ee767c3ec4f52324 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Tue, 27 Jan 2026 16:06:24 -0800 Subject: [PATCH] feat(ux) Expandable (ctrl-O) and scrollable approvals in alternate buffer mode. (#17640) --- packages/cli/src/test-utils/render.tsx | 9 +- packages/cli/src/ui/App.test.tsx | 64 ++-- .../src/ui/__snapshots__/App.test.tsx.snap | 214 ++++++----- .../src/ui/components/HistoryItemDisplay.tsx | 2 + .../src/ui/components/MainContent.test.tsx | 62 +-- .../cli/src/ui/components/MainContent.tsx | 70 ++-- .../components/ToolConfirmationQueue.test.tsx | 95 ++++- .../ui/components/ToolConfirmationQueue.tsx | 124 +++--- ...ternateBufferQuittingDisplay.test.tsx.snap | 3 +- .../HistoryItemDisplay.test.tsx.snap | 30 +- .../__snapshots__/MainContent.test.tsx.snap | 3 +- .../ToolConfirmationQueue.test.tsx.snap | 59 ++- .../ui/components/messages/GeminiMessage.tsx | 10 +- .../messages/GeminiMessageContent.tsx | 10 +- .../messages/ToolConfirmationMessage.tsx | 7 +- .../ToolConfirmationMessageOverflow.test.tsx | 128 +++++++ .../components/messages/ToolGroupMessage.tsx | 29 +- .../messages/ToolResultDisplay.test.tsx | 3 + .../components/messages/ToolResultDisplay.tsx | 2 +- .../ToolResultDisplayOverflow.test.tsx | 76 ++++ .../__snapshots__/GeminiMessage.test.tsx.snap | 12 +- .../RedirectionConfirmation.test.tsx.snap | 1 - .../ToolConfirmationMessage.test.tsx.snap | 11 - ...lConfirmationMessageOverflow.test.tsx.snap | 18 + .../ToolGroupMessage.test.tsx.snap | 4 - .../ToolResultDisplay.test.tsx.snap | 3 +- .../ToolResultDisplayOverflow.test.tsx.snap | 14 + packages/cli/src/ui/hooks/toolMapping.ts | 4 + .../cli/src/ui/hooks/useGeminiStream.test.tsx | 356 +++++++++--------- packages/cli/src/ui/hooks/useGeminiStream.ts | 227 +++++++++-- packages/cli/src/ui/hooks/useStateAndRef.ts | 2 +- packages/cli/src/ui/hooks/useToolScheduler.ts | 2 + .../cli/src/ui/layouts/DefaultAppLayout.tsx | 17 +- packages/cli/src/ui/types.ts | 2 + 34 files changed, 1177 insertions(+), 496 deletions(-) create mode 100644 packages/cli/src/ui/components/messages/ToolConfirmationMessageOverflow.test.tsx create mode 100644 packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessageOverflow.test.tsx.snap create mode 100644 packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplayOverflow.test.tsx.snap diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 97891067fe..18744ee2b4 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -416,10 +416,13 @@ export function renderHookWithProviders( const result = { current: undefined as unknown as Result }; let setPropsFn: ((props: Props) => void) | undefined; + let forceUpdateFn: (() => void) | undefined; function TestComponent({ initialProps }: { initialProps: Props }) { const [props, setProps] = useState(initialProps); + const [, forceUpdate] = useState(0); setPropsFn = setProps; + forceUpdateFn = () => forceUpdate((n) => n + 1); result.current = renderCallback(props); return null; } @@ -439,8 +442,10 @@ export function renderHookWithProviders( function rerender(newProps?: Props) { act(() => { - if (setPropsFn && newProps) { - setPropsFn(newProps); + if (arguments.length > 0 && setPropsFn) { + setPropsFn(newProps as Props); + } else if (forceUpdateFn) { + forceUpdateFn(); } }); } diff --git a/packages/cli/src/ui/App.test.tsx b/packages/cli/src/ui/App.test.tsx index 8bbdc0db60..81df8b9574 100644 --- a/packages/cli/src/ui/App.test.tsx +++ b/packages/cli/src/ui/App.test.tsx @@ -8,7 +8,6 @@ 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 { type UIState } from './contexts/UIStateContext.js'; import { StreamingState, ToolCallStatus } from './types.js'; @@ -22,10 +21,6 @@ vi.mock('ink', async (importOriginal) => { }; }); -vi.mock('./components/MainContent.js', () => ({ - MainContent: () => MainContent, -})); - vi.mock('./components/DialogManager.js', () => ({ DialogManager: () => DialogManager, })); @@ -34,9 +29,16 @@ vi.mock('./components/Composer.js', () => ({ Composer: () => Composer, })); -vi.mock('./components/Notifications.js', () => ({ - Notifications: () => Notifications, -})); +vi.mock('./components/Notifications.js', async () => { + const { Text, Box } = await import('ink'); + return { + Notifications: () => ( + + Notifications + + ), + }; +}); vi.mock('./components/QuittingDisplay.js', () => ({ QuittingDisplay: () => Quitting..., @@ -46,9 +48,16 @@ vi.mock('./components/HistoryItemDisplay.js', () => ({ HistoryItemDisplay: () => HistoryItemDisplay, })); -vi.mock('./components/Footer.js', () => ({ - Footer: () => Footer, -})); +vi.mock('./components/Footer.js', async () => { + const { Text, Box } = await import('ink'); + return { + Footer: () => ( + + Footer + + ), + }; +}); describe('App', () => { beforeEach(() => { @@ -87,7 +96,7 @@ describe('App', () => { useAlternateBuffer: false, }); - expect(lastFrame()).toContain('MainContent'); + expect(lastFrame()).toContain('Tips for getting started'); expect(lastFrame()).toContain('Notifications'); expect(lastFrame()).toContain('Composer'); }); @@ -133,7 +142,7 @@ describe('App', () => { uiState: dialogUIState, }); - expect(lastFrame()).toContain('MainContent'); + expect(lastFrame()).toContain('Tips for getting started'); expect(lastFrame()).toContain('Notifications'); expect(lastFrame()).toContain('DialogManager'); }); @@ -165,10 +174,10 @@ describe('App', () => { uiState: mockUIState, }); - expect(lastFrame()).toContain(`Notifications -Footer -MainContent -Composer`); + expect(lastFrame()).toContain('Notifications'); + expect(lastFrame()).toContain('Footer'); + expect(lastFrame()).toContain('Tips for getting started'); + expect(lastFrame()).toContain('Composer'); }); it('should render DefaultAppLayout when screen reader is not enabled', () => { @@ -178,9 +187,9 @@ Composer`); uiState: mockUIState, }); - expect(lastFrame()).toContain(`MainContent -Notifications -Composer`); + expect(lastFrame()).toContain('Tips for getting started'); + expect(lastFrame()).toContain('Notifications'); + expect(lastFrame()).toContain('Composer'); }); it('should render ToolConfirmationQueue along with Composer when tool is confirming and experiment is on', () => { @@ -209,19 +218,20 @@ Composer`); pendingGeminiHistoryItems: [{ type: 'tool_group', tools: toolCalls }], } as UIState; - const configWithExperiment = { - ...makeFakeConfig(), - isEventDrivenSchedulerEnabled: () => true, - isTrustedFolder: () => true, - getIdeMode: () => false, - } as unknown as Config; + const configWithExperiment = makeFakeConfig(); + vi.spyOn( + configWithExperiment, + 'isEventDrivenSchedulerEnabled', + ).mockReturnValue(true); + vi.spyOn(configWithExperiment, 'isTrustedFolder').mockReturnValue(true); + vi.spyOn(configWithExperiment, 'getIdeMode').mockReturnValue(false); const { lastFrame } = renderWithProviders(, { uiState: stateWithConfirmingTool, config: configWithExperiment, }); - expect(lastFrame()).toContain('MainContent'); + expect(lastFrame()).toContain('Tips for getting started'); expect(lastFrame()).toContain('Notifications'); expect(lastFrame()).toContain('Action Required'); // From ToolConfirmationQueue expect(lastFrame()).toContain('1 of 1'); diff --git a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap index 07103d2e0c..2587751008 100644 --- a/packages/cli/src/ui/__snapshots__/App.test.tsx.snap +++ b/packages/cli/src/ui/__snapshots__/App.test.tsx.snap @@ -1,108 +1,135 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html exports[`App > Snapshots > renders default layout correctly 1`] = ` -"MainContent +" + ███ █████████ +░░░███ ███░░░░░███ + ░░░███ ███ ░░░ + ░░░███░███ + ███░ ░███ █████ + ███░ ░░███ ░░███ + ███░ ░░█████████ +░░░ ░░░░░░░░░ + +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. + + + + + + + + + + + + + + + + + + + + + + Notifications Composer - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - " `; exports[`App > Snapshots > renders screen reader layout correctly 1`] = ` "Notifications Footer -MainContent + + ███ █████████ +░░░███ ███░░░░░███ + ░░░███ ███ ░░░ + ░░░███░███ + ███░ ░███ █████ + ███░ ░░███ ░░███ + ███░ ░░█████████ +░░░ ░░░░░░░░░ + +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. Composer" `; exports[`App > Snapshots > renders with dialogs visible 1`] = ` -"MainContent +" + ███ █████████ +░░░███ ███░░░░░███ + ░░░███ ███ ░░░ + ░░░███░███ + ███░ ░███ █████ + ███░ ░░███ ░░███ + ███░ ░░█████████ +░░░ ░░░░░░░░░ + + + + + + + + + + + + + + + + + + + + + + + + + + + + Notifications DialogManager - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - " `; exports[`App > should render ToolConfirmationQueue along with Composer when tool is confirming and experiment is on 1`] = ` -"MainContent -Notifications +" + ███ █████████ +░░░███ ███░░░░░███ + ░░░███ ███ ░░░ + ░░░███░███ + ███░ ░███ █████ + ███░ ░░███ ░░███ + ███░ ░░█████████ +░░░ ░░░░░░░░░ + +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. +HistoryItemDisplay ╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │ Action Required 1 of 1 │ │ │ │ ? ls list directory │ │ │ │ ls │ -│ │ │ Allow execution of: 'ls'? │ │ │ │ ● 1. Allow once │ @@ -110,28 +137,15 @@ Notifications │ 3. No, suggest changes (esc) │ │ │ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ + + + + + + + + +Notifications Composer - - - - - - - - - - - - - - - - - - - - - - " `; diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.tsx index 3814e603d8..f04ba39ec6 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.tsx @@ -141,6 +141,8 @@ export const HistoryItemDisplay: React.FC = ({ isFocused={isFocused} activeShellPtyId={activeShellPtyId} embeddedShellFocused={embeddedShellFocused} + borderTop={itemForDisplay.borderTop} + borderBottom={itemForDisplay.borderBottom} /> )} {itemForDisplay.type === 'compression' && ( diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index 63cbdce790..f38a6350fa 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { render } from '../../test-utils/render.js'; +import { renderWithProviders } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; import { MainContent } from './MainContent.js'; import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -12,30 +12,38 @@ import { Box, Text } from 'ink'; import type React from 'react'; // Mock dependencies -vi.mock('../contexts/AppContext.js', () => ({ - useAppContext: () => ({ - version: '1.0.0', - }), -})); +vi.mock('../contexts/AppContext.js', async () => { + const actual = await vi.importActual('../contexts/AppContext.js'); + return { + ...actual, + useAppContext: () => ({ + version: '1.0.0', + }), + }; +}); -vi.mock('../contexts/UIStateContext.js', () => ({ - useUIState: () => ({ - history: [ - { id: 1, role: 'user', content: 'Hello' }, - { id: 2, role: 'model', content: 'Hi there' }, - ], - pendingHistoryItems: [], - mainAreaWidth: 80, - staticAreaMaxItemHeight: 20, - availableTerminalHeight: 24, - slashCommands: [], - constrainHeight: false, - isEditorDialogOpen: false, - activePtyId: undefined, - embeddedShellFocused: false, - historyRemountKey: 0, - }), -})); +vi.mock('../contexts/UIStateContext.js', async () => { + const actual = await vi.importActual('../contexts/UIStateContext.js'); + return { + ...actual, + useUIState: () => ({ + history: [ + { id: 1, role: 'user', content: 'Hello' }, + { id: 2, role: 'model', content: 'Hi there' }, + ], + pendingHistoryItems: [], + mainAreaWidth: 80, + staticAreaMaxItemHeight: 20, + availableTerminalHeight: 24, + slashCommands: [], + constrainHeight: false, + isEditorDialogOpen: false, + activePtyId: undefined, + embeddedShellFocused: false, + historyRemountKey: 0, + }), + }; +}); vi.mock('../hooks/useAlternateBuffer.js', () => ({ useAlternateBuffer: vi.fn(), @@ -95,7 +103,7 @@ describe('MainContent', () => { }); it('renders in normal buffer mode', async () => { - const { lastFrame } = render(); + const { lastFrame } = renderWithProviders(); await waitFor(() => expect(lastFrame()).toContain('AppHeader')); const output = lastFrame(); @@ -105,7 +113,7 @@ describe('MainContent', () => { it('renders in alternate buffer mode', async () => { vi.mocked(useAlternateBuffer).mockReturnValue(true); - const { lastFrame } = render(); + const { lastFrame } = renderWithProviders(); await waitFor(() => expect(lastFrame()).toContain('ScrollableList')); const output = lastFrame(); @@ -116,7 +124,7 @@ describe('MainContent', () => { it('does not constrain height in alternate buffer mode', async () => { vi.mocked(useAlternateBuffer).mockReturnValue(true); - const { lastFrame } = render(); + const { lastFrame } = renderWithProviders(); await waitFor(() => expect(lastFrame()).toContain('HistoryItem: Hello')); const output = lastFrame(); diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index 5239ec040a..e97b7a6211 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -6,16 +6,20 @@ import { Box, Static } from 'ink'; import { HistoryItemDisplay } from './HistoryItemDisplay.js'; -import { ShowMoreLines } from './ShowMoreLines.js'; -import { OverflowProvider } from '../contexts/OverflowContext.js'; import { useUIState } from '../contexts/UIStateContext.js'; import { useAppContext } from '../contexts/AppContext.js'; import { AppHeader } from './AppHeader.js'; import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; -import { SCROLL_TO_ITEM_END } from './shared/VirtualizedList.js'; +import { + SCROLL_TO_ITEM_END, + type VirtualizedListRef, +} from './shared/VirtualizedList.js'; import { ScrollableList } from './shared/ScrollableList.js'; -import { useMemo, memo, useCallback } from 'react'; +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); @@ -27,8 +31,21 @@ 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 scrollableListRef = useRef>(null); + + useEffect(() => { + if (showConfirmationQueue) { + scrollableListRef.current?.scrollToEnd(); + } + }, [showConfirmationQueue, confirmingTool]); + const { pendingHistoryItems, mainAreaWidth, @@ -59,27 +76,27 @@ export const MainContent = () => { const pendingItems = useMemo( () => ( - - - {pendingHistoryItems.map((item, i) => ( - - ))} - - - + + {pendingHistoryItems.map((item, i) => ( + + ))} + {showConfirmationQueue && confirmingTool && ( + + )} + ), [ pendingHistoryItems, @@ -90,6 +107,8 @@ export const MainContent = () => { uiState.isEditorDialogOpen, uiState.activePtyId, uiState.embeddedShellFocused, + showConfirmationQueue, + confirmingTool, ], ); @@ -128,6 +147,7 @@ export const MainContent = () => { if (isAlternateBuffer) { return ( { expect(lastFrame()).toBe(''); }); + + it('renders expansion hint when content is long and constrained', async () => { + const longDiff = '@@ -1,1 +1,50 @@\n' + '+line\n'.repeat(50); + const confirmingTool = { + tool: { + callId: 'call-1', + name: 'replace', + description: 'edit file', + status: ToolCallStatus.Confirming, + confirmationDetails: { + type: 'edit' as const, + title: 'Confirm edit', + fileName: 'test.ts', + filePath: '/test.ts', + fileDiff: longDiff, + originalContent: 'old', + newContent: 'new', + onConfirm: vi.fn(), + }, + }, + index: 1, + total: 1, + }; + + const { lastFrame } = renderWithProviders( + + + , + { + config: mockConfig, + useAlternateBuffer: false, + uiState: { + terminalWidth: 80, + terminalHeight: 20, + constrainHeight: true, + streamingState: StreamingState.WaitingForConfirmation, + }, + }, + ); + + await waitFor(() => + expect(lastFrame()).toContain('Press ctrl-o to show more lines'), + ); + expect(lastFrame()).toMatchSnapshot(); + expect(lastFrame()).toContain('Press ctrl-o to show more lines'); + }); + + it('does not render expansion hint when constrainHeight is false', () => { + const longDiff = 'line\n'.repeat(50); + const confirmingTool = { + tool: { + callId: 'call-1', + name: 'replace', + description: 'edit file', + status: ToolCallStatus.Confirming, + confirmationDetails: { + type: 'edit' as const, + title: 'Confirm edit', + fileName: 'test.ts', + filePath: '/test.ts', + fileDiff: longDiff, + originalContent: 'old', + newContent: 'new', + onConfirm: vi.fn(), + }, + }, + index: 1, + total: 1, + }; + + const { lastFrame } = renderWithProviders( + , + { + config: mockConfig, + uiState: { + terminalWidth: 80, + terminalHeight: 40, + constrainHeight: false, + streamingState: StreamingState.WaitingForConfirmation, + }, + }, + ); + + const output = lastFrame(); + expect(output).not.toContain('Press ctrl-o to show more lines'); + expect(output).toMatchSnapshot(); + }); }); diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx index 9e378fb5f6..35a3a66da2 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.tsx @@ -12,6 +12,10 @@ 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'; +import { OverflowProvider } from '../contexts/OverflowContext.js'; +import { ShowMoreLines } from './ShowMoreLines.js'; +import { StickyHeader } from './StickyHeader.js'; +import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; interface ToolConfirmationQueueProps { confirmingTool: ConfirmingToolState; @@ -21,7 +25,8 @@ export const ToolConfirmationQueue: React.FC = ({ confirmingTool, }) => { const config = useConfig(); - const { terminalWidth, terminalHeight } = useUIState(); + const isAlternateBuffer = useAlternateBuffer(); + const { mainAreaWidth, terminalHeight, constrainHeight } = useUIState(); const { tool, index, total } = confirmingTool; // Safety check: ToolConfirmationMessage requires confirmationDetails @@ -38,52 +43,85 @@ export const ToolConfirmationQueue: React.FC = ({ // - 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); + const availableContentHeight = + constrainHeight && !isAlternateBuffer + ? Math.max(maxHeight - 6, 4) + : undefined; + + const borderColor = theme.status.warning; return ( - - {/* Header */} - - - Action Required - - - {index} of {total} - - + + + + + {/* 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. + */} + + + - - {/* 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 c112a83117..24e92f85ce 100644 --- a/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AlternateBufferQuittingDisplay.test.tsx.snap @@ -130,5 +130,6 @@ Tips for getting started: ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀ > Hello Gemini ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄ -✦ Hello User!" +✦ Hello User! +" `; diff --git a/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap index 7c92db1ab6..1f6288c292 100644 --- a/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/HistoryItemDisplay.test.tsx.snap @@ -51,7 +51,8 @@ exports[` > gemini items (alternateBuffer=false) > should 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=false) > should render a full gemini_content item when using availableTerminalHeightGemini 1`] = ` @@ -105,13 +106,13 @@ exports[` > gemini items (alternateBuffer=false) > should 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=false) > should render a truncated gemini item 1`] = ` "✦ Example code block: - ... first 41 lines hidden ... - 42 Line 42 + ... first 42 lines hidden ... 43 Line 43 44 Line 44 45 Line 45 @@ -119,13 +120,13 @@ exports[` > gemini items (alternateBuffer=false) > should 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=false) > should render a truncated gemini_content item 1`] = ` " Example code block: - ... first 41 lines hidden ... - 42 Line 42 + ... first 42 lines hidden ... 43 Line 43 44 Line 44 45 Line 45 @@ -133,7 +134,8 @@ exports[` > gemini items (alternateBuffer=false) > should 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=true) > should render a full gemini item when using availableTerminalHeightGemini 1`] = ` @@ -187,7 +189,8 @@ exports[` > gemini items (alternateBuffer=true) > should r 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=true) > should render a full gemini_content item when using availableTerminalHeightGemini 1`] = ` @@ -241,7 +244,8 @@ exports[` > gemini items (alternateBuffer=true) > should r 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=true) > should render a truncated gemini item 1`] = ` @@ -295,7 +299,8 @@ exports[` > gemini items (alternateBuffer=true) > should r 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > gemini items (alternateBuffer=true) > should render a truncated gemini_content item 1`] = ` @@ -349,7 +354,8 @@ exports[` > gemini items (alternateBuffer=true) > should r 47 Line 47 48 Line 48 49 Line 49 - 50 Line 50" + 50 Line 50 +" `; exports[` > renders AgentsStatus for "agents_list" type 1`] = ` diff --git a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap index ffbb0ab2d2..73621e041f 100644 --- a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap @@ -4,6 +4,5 @@ exports[`MainContent > does not constrain height in alternate buffer mode 1`] = "ScrollableList AppHeader HistoryItem: Hello (height: undefined) -HistoryItem: Hi there (height: undefined) -ShowMoreLines" +HistoryItem: Hi there (height: undefined)" `; diff --git a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap index 57ccfb8df3..4cac209e18 100644 --- a/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ToolConfirmationQueue.test.tsx.snap @@ -1,5 +1,60 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`ToolConfirmationQueue > does not render expansion hint when constrainHeight is false 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ Action Required 1 of 1 │ +│ │ +│ ? replace edit file │ +│ │ +│ ╭──────────────────────────────────────────────────────────────────────────╮ │ +│ │ │ │ +│ │ No changes detected. │ │ +│ │ │ │ +│ ╰──────────────────────────────────────────────────────────────────────────╯ │ +│ Apply this change? │ +│ │ +│ ● 1. Allow once │ +│ 2. Allow for this session │ +│ 3. Modify with external editor │ +│ 4. No, suggest changes (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`ToolConfirmationQueue > renders expansion hint when content is long and constrained 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ Action Required 1 of 1 │ +│ │ +│ ? replace edit file │ +│ │ +│ ... first 49 lines hidden ... │ +│ 50 line │ +│ Apply this change? │ +│ │ +│ ● 1. Allow once │ +│ 2. Allow for this session │ +│ 3. Modify with external editor │ +│ 4. No, suggest changes (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯ + Press ctrl-o to show more lines + + + + + + + + + + + + + +" +`; + exports[`ToolConfirmationQueue > renders the confirming tool with progress indicator 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────╮ │ Action Required 1 of 3 │ @@ -7,12 +62,12 @@ exports[`ToolConfirmationQueue > renders the confirming tool with progress indic │ ? 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/GeminiMessage.tsx b/packages/cli/src/ui/components/messages/GeminiMessage.tsx index 87a2b1b622..caa191069e 100644 --- a/packages/cli/src/ui/components/messages/GeminiMessage.tsx +++ b/packages/cli/src/ui/components/messages/GeminiMessage.tsx @@ -7,6 +7,7 @@ import type React from 'react'; import { Text, Box } from 'ink'; import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; +import { ShowMoreLines } from '../ShowMoreLines.js'; import { theme } from '../../semantic-colors.js'; import { SCREEN_READER_MODEL_PREFIX } from '../../textConstants.js'; import { useUIState } from '../../contexts/UIStateContext.js'; @@ -42,11 +43,18 @@ export const GeminiMessage: React.FC = ({ text={text} isPending={isPending} availableTerminalHeight={ - isAlternateBuffer ? undefined : availableTerminalHeight + isAlternateBuffer || availableTerminalHeight === undefined + ? undefined + : Math.max(availableTerminalHeight - 1, 1) } terminalWidth={terminalWidth} renderMarkdown={renderMarkdown} /> + + + ); diff --git a/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx b/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx index 965a0bcb0f..e0a2131318 100644 --- a/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx +++ b/packages/cli/src/ui/components/messages/GeminiMessageContent.tsx @@ -7,6 +7,7 @@ import type React from 'react'; import { Box } from 'ink'; import { MarkdownDisplay } from '../../utils/MarkdownDisplay.js'; +import { ShowMoreLines } from '../ShowMoreLines.js'; import { useUIState } from '../../contexts/UIStateContext.js'; import { useAlternateBuffer } from '../../hooks/useAlternateBuffer.js'; @@ -40,11 +41,18 @@ export const GeminiMessageContent: React.FC = ({ text={text} isPending={isPending} availableTerminalHeight={ - isAlternateBuffer ? undefined : availableTerminalHeight + isAlternateBuffer || availableTerminalHeight === undefined + ? undefined + : Math.max(availableTerminalHeight - 1, 1) } terminalWidth={terminalWidth} renderMarkdown={renderMarkdown} /> + + + ); }; diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index e77178ff11..e200f0bb44 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -61,7 +61,7 @@ export const ToolConfirmationMessage: React.FC< const handleConfirm = useCallback( (outcome: ToolConfirmationOutcome) => { - void confirm(callId, outcome).catch((error) => { + void confirm(callId, outcome).catch((error: unknown) => { debugLogger.error( `Failed to handle tool confirmation for ${callId}:`, error, @@ -240,7 +240,8 @@ export const ToolConfirmationMessage: React.FC< MARGIN_BODY_BOTTOM + HEIGHT_QUESTION + MARGIN_QUESTION_BOTTOM + - optionsCount; + optionsCount + + 1; // Reserve one line for 'ShowMoreLines' hint return Math.max(availableTerminalHeight - surroundingElementsHeight, 1); }, [availableTerminalHeight, getOptions]); @@ -431,7 +432,7 @@ export const ToolConfirmationMessage: React.FC< {/* Body Content (Diff Renderer or Command Info) */} {/* No separate context display here anymore for edits */} - + { + 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.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index a43c67dadd..80c74baa58 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -16,6 +16,8 @@ import { theme } from '../../semantic-colors.js'; import { useConfig } from '../../contexts/ConfigContext.js'; import { isShellTool, isThisShellFocused } from './ToolShared.js'; import { ASK_USER_DISPLAY_NAME } from '@google/gemini-cli-core'; +import { ShowMoreLines } from '../ShowMoreLines.js'; +import { useUIState } from '../../contexts/UIStateContext.js'; interface ToolGroupMessageProps { groupId: number; @@ -26,6 +28,8 @@ interface ToolGroupMessageProps { activeShellPtyId?: number | null; embeddedShellFocused?: boolean; onShellInputSubmit?: (input: string) => void; + borderTop?: boolean; + borderBottom?: boolean; } // Helper to identify Ask User tools that are in progress (have their own dialog UI) @@ -45,6 +49,8 @@ export const ToolGroupMessage: React.FC = ({ isFocused = true, activeShellPtyId, embeddedShellFocused, + borderTop: borderTopOverride, + borderBottom: borderBottomOverride, }) => { // Filter out in-progress Ask User tools (they have their own AskUserDialog UI) const toolCalls = useMemo( @@ -53,6 +59,7 @@ export const ToolGroupMessage: React.FC = ({ ); const config = useConfig(); + const { constrainHeight } = useUIState(); const isEventDriven = config.isEventDrivenSchedulerEnabled(); @@ -110,8 +117,12 @@ export const ToolGroupMessage: React.FC = ({ ); // If all tools are hidden (e.g. group only contains confirming or pending tools), - // render nothing in the history log. - if (visibleToolCalls.length === 0) { + // render nothing in the history log unless we have a border override. + if ( + visibleToolCalls.length === 0 && + borderTopOverride === undefined && + borderBottomOverride === undefined + ) { return null; } @@ -161,7 +172,10 @@ export const ToolGroupMessage: React.FC = ({ : toolAwaitingApproval ? ('low' as const) : ('medium' as const), - isFirst, + isFirst: + borderTopOverride !== undefined + ? borderTopOverride && isFirst + : isFirst, borderColor, borderDimColor, }; @@ -225,20 +239,25 @@ 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. */ - visibleToolCalls.length > 0 && ( + (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) && ( ) } + {(borderBottomOverride ?? true) && visibleToolCalls.length > 0 && ( + + + + )} ); }; diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx index 41f79aab08..b0e6236496 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.test.tsx @@ -56,6 +56,9 @@ vi.mock('../../contexts/OverflowContext.js', () => ({ addOverflowingId: vi.fn(), removeOverflowingId: vi.fn(), }), + useOverflowState: () => ({ + overflowingIds: new Set(), + }), })); describe('ToolResultDisplay', () => { diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx index aa005c3e43..a729366044 100644 --- a/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx +++ b/packages/cli/src/ui/components/messages/ToolResultDisplay.tsx @@ -16,7 +16,7 @@ import { useUIState } from '../../contexts/UIStateContext.js'; import { tryParseJSON } from '../../../utils/jsonoutput.js'; const STATIC_HEIGHT = 1; -const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc. +const RESERVED_LINE_COUNT = 6; // for tool name, status, padding, and 'ShowMoreLines' hint const MIN_LINES_SHOWN = 2; // show at least this many lines // Large threshold to ensure we don't cause performance issues for very large diff --git a/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx b/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx new file mode 100644 index 0000000000..6e15d7902d --- /dev/null +++ b/packages/cli/src/ui/components/messages/ToolResultDisplayOverflow.test.tsx @@ -0,0 +1,76 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { ToolGroupMessage } from './ToolGroupMessage.js'; +import { renderWithProviders } from '../../../test-utils/render.js'; +import { + StreamingState, + ToolCallStatus, + type IndividualToolCallDisplay, +} from '../../types.js'; +import { OverflowProvider } from '../../contexts/OverflowContext.js'; +import { waitFor } from '../../../test-utils/async.js'; + +describe('ToolResultDisplay Overflow', () => { + it('should display "press ctrl-o" hint when content overflows in ToolGroupMessage', async () => { + // Large output that will definitely overflow + const lines = []; + for (let i = 0; i < 50; i++) { + lines.push(`line ${i + 1}`); + } + const resultDisplay = lines.join('\n'); + + const toolCalls: IndividualToolCallDisplay[] = [ + { + callId: 'call-1', + name: 'test-tool', + description: 'a test tool', + status: ToolCallStatus.Success, + resultDisplay, + confirmationDetails: undefined, + }, + ]; + + const { lastFrame } = renderWithProviders( + + + , + { + uiState: { + streamingState: StreamingState.Idle, + 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/__snapshots__/GeminiMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/GeminiMessage.test.tsx.snap index d2c032a953..166ac605be 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/GeminiMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/GeminiMessage.test.tsx.snap @@ -5,13 +5,15 @@ exports[` - Raw Markdown Display Snapshots > renders pending st \`\`\`javascript const x = 1; - \`\`\`" + \`\`\` +" `; exports[` - Raw Markdown Display Snapshots > renders pending state with renderMarkdown=true 1`] = ` "✦ Test bold and code markdown - 1 const x = 1;" + 1 const x = 1; +" `; exports[` - Raw Markdown Display Snapshots > renders with renderMarkdown=false '(raw markdown with syntax highlightin…' 1`] = ` @@ -19,11 +21,13 @@ exports[` - Raw Markdown Display Snapshots > renders with rende \`\`\`javascript const x = 1; - \`\`\`" + \`\`\` +" `; exports[` - Raw Markdown Display Snapshots > renders with renderMarkdown=true '(default)' 1`] = ` "✦ Test bold and code markdown - 1 const x = 1;" + 1 const x = 1; +" `; diff --git a/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap index 08648abdde..4f89811121 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/RedirectionConfirmation.test.tsx.snap @@ -5,7 +5,6 @@ exports[`ToolConfirmationMessage Redirection > should display redirection warnin Note: Command contains redirection which can be undesirable. Tip: Toggle auto-edit (Shift+Tab) to allow redirection in the future. - Allow execution of: 'echo, redirection (>)'? ● 1. Allow once diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap index 2f8dba6a6e..69574a60c6 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessage.test.tsx.snap @@ -4,7 +4,6 @@ exports[`ToolConfirmationMessage > should display multiple commands for exec typ "echo "hello" ls -la whoami - Allow execution of 3 commands? ● 1. Allow once @@ -18,7 +17,6 @@ exports[`ToolConfirmationMessage > should display urls if prompt and url are dif URLs to fetch: - https://raw.githubusercontent.com/google/gemini-react/main/README.md - Do you want to proceed? ● 1. Allow once @@ -29,7 +27,6 @@ Do you want to proceed? exports[`ToolConfirmationMessage > should not display urls if prompt and url are the same 1`] = ` "https://example.com - Do you want to proceed? ● 1. Allow once @@ -44,7 +41,6 @@ exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations' │ No changes detected. │ │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ - Apply this change? ● 1. Allow once @@ -59,7 +55,6 @@ exports[`ToolConfirmationMessage > with folder trust > 'for edit confirmations' │ No changes detected. │ │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ - Apply this change? ● 1. Allow once @@ -71,7 +66,6 @@ Apply this change? exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations' > should NOT show "allow always" when folder is untrusted 1`] = ` "echo "hello" - Allow execution of: 'echo'? ● 1. Allow once @@ -81,7 +75,6 @@ Allow execution of: 'echo'? exports[`ToolConfirmationMessage > with folder trust > 'for exec confirmations' > should show "allow always" when folder is trusted 1`] = ` "echo "hello" - Allow execution of: 'echo'? ● 1. Allow once @@ -92,7 +85,6 @@ Allow execution of: 'echo'? exports[`ToolConfirmationMessage > with folder trust > 'for info confirmations' > should NOT show "allow always" when folder is untrusted 1`] = ` "https://example.com - Do you want to proceed? ● 1. Allow once @@ -102,7 +94,6 @@ Do you want to proceed? exports[`ToolConfirmationMessage > with folder trust > 'for info confirmations' > should show "allow always" when folder is trusted 1`] = ` "https://example.com - Do you want to proceed? ● 1. Allow once @@ -114,7 +105,6 @@ Do you want to proceed? exports[`ToolConfirmationMessage > with folder trust > 'for mcp confirmations' > should NOT show "allow always" when folder is untrusted 1`] = ` "MCP Server: test-server Tool: test-tool - Allow execution of MCP tool "test-tool" from server "test-server"? ● 1. Allow once @@ -125,7 +115,6 @@ Allow execution of MCP tool "test-tool" from server "test-server"? exports[`ToolConfirmationMessage > with folder trust > 'for mcp confirmations' > should show "allow always" when folder is trusted 1`] = ` "MCP Server: test-server Tool: test-tool - Allow execution of MCP tool "test-tool" from server "test-server"? ● 1. Allow once diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessageOverflow.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessageOverflow.test.tsx.snap new file mode 100644 index 0000000000..0511704c9f --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolConfirmationMessageOverflow.test.tsx.snap @@ -0,0 +1,18 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ToolConfirmationMessage Overflow > should display "press ctrl-o" hint when content overflows in ToolGroupMessage 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ ? test-tool a test tool ← │ +│ │ +│ ... first 49 lines hidden ... │ +│ 50 line 50 │ +│ Apply this change? │ +│ │ +│ ● 1. Allow once │ +│ 2. Allow for this session │ +│ 3. Modify with external editor │ +│ 4. No, suggest changes (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────╯ + Press ctrl-o to show more lines" +`; 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 a8644b7536..e664288a06 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 @@ -34,7 +34,6 @@ exports[` > Confirmation Handling > renders confirmation wit │ │ │ Test result │ │ Do you want to proceed? │ -│ │ │ Do you want to proceed? │ │ │ │ ● 1. Allow once │ @@ -50,7 +49,6 @@ exports[` > Confirmation Handling > renders confirmation wit │ │ │ Test result │ │ Do you want to proceed? │ -│ │ │ Do you want to proceed? │ │ │ │ ● 1. Allow once │ @@ -67,7 +65,6 @@ exports[` > Confirmation Handling > shows confirmation dialo │ │ │ Test result │ │ Confirm first tool │ -│ │ │ Do you want to proceed? │ │ │ │ ● 1. Allow once │ @@ -160,7 +157,6 @@ exports[` > Golden Snapshots > renders tool call awaiting co │ │ │ Test result │ │ Are you sure you want to proceed? │ -│ │ │ Do you want to proceed? │ │ │ │ ● 1. Allow once │ diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap index 666a2f7fed..e90c365951 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplay.test.tsx.snap @@ -15,8 +15,7 @@ exports[`ToolResultDisplay > renders string result as markdown by default 1`] = exports[`ToolResultDisplay > renders string result as plain text when renderOutputAsMarkdown is false 1`] = `"**Some result**"`; exports[`ToolResultDisplay > truncates very long string results 1`] = ` -"... first 251 lines hidden ... -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +"... first 252 lines hidden ... aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplayOverflow.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplayOverflow.test.tsx.snap new file mode 100644 index 0000000000..09a1cef39f --- /dev/null +++ b/packages/cli/src/ui/components/messages/__snapshots__/ToolResultDisplayOverflow.test.tsx.snap @@ -0,0 +1,14 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`ToolResultDisplay Overflow > should display "press ctrl-o" hint when content overflows in ToolGroupMessage 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────╮ +│ ✓ test-tool a test tool │ +│ │ +│ ... first 46 lines hidden ... │ +│ line 47 │ +│ line 48 │ +│ line 49 │ +│ line 50 │ +╰──────────────────────────────────────────────────────────────────────────────╯ + Press ctrl-o to show more lines" +`; diff --git a/packages/cli/src/ui/hooks/toolMapping.ts b/packages/cli/src/ui/hooks/toolMapping.ts index f9c0a3eb19..902db1333e 100644 --- a/packages/cli/src/ui/hooks/toolMapping.ts +++ b/packages/cli/src/ui/hooks/toolMapping.ts @@ -50,8 +50,10 @@ export function mapCoreStatusToDisplayStatus( */ export function mapToDisplay( toolOrTools: ToolCall[] | ToolCall, + options: { borderTop?: boolean; borderBottom?: boolean } = {}, ): HistoryItemToolGroup { const toolCalls = Array.isArray(toolOrTools) ? toolOrTools : [toolOrTools]; + const { borderTop, borderBottom } = options; const toolDisplays = toolCalls.map((call): IndividualToolCallDisplay => { let description: string; @@ -128,5 +130,7 @@ export function mapToDisplay( return { type: 'tool_group', tools: toolDisplays, + borderTop, + borderBottom, }; } diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index ff7664a9c0..d9763b96f5 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -8,7 +8,7 @@ import type { Mock, MockInstance } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { act } from 'react'; -import { renderHook } from '../../test-utils/render.js'; +import { renderHookWithProviders } from '../../test-utils/render.js'; import { waitFor } from '../../test-utils/async.js'; import { useGeminiStream } from './useGeminiStream.js'; import { useKeypress } from './useKeypress.js'; @@ -56,7 +56,7 @@ const MockedGeminiClientClass = vi.hoisted(() => this.startChat = mockStartChat; this.sendMessageStream = mockSendMessageStream; this.addHistory = vi.fn(); - this.getCurrentSequenceModel = vi.fn(); + this.getCurrentSequenceModel = vi.fn().mockReturnValue('test-model'); this.getChat = vi.fn().mockReturnValue({ recordCompletedToolCalls: vi.fn(), }); @@ -93,6 +93,8 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { ValidationRequiredError: MockValidationRequiredError, parseAndFormatApiError: mockParseAndFormatApiError, tokenLimit: vi.fn().mockReturnValue(100), // Mock tokenLimit + recordToolCallInteractions: vi.fn().mockResolvedValue(undefined), + getCodeAssistServer: vi.fn().mockReturnValue(undefined), }; }); @@ -167,84 +169,92 @@ vi.mock('./useAlternateBuffer.js', () => ({ // --- Tests for useGeminiStream Hook --- describe('useGeminiStream', () => { - let mockAddItem: Mock; - let mockConfig: Config; - let mockOnDebugMessage: Mock; - let mockHandleSlashCommand: Mock; + let mockAddItem = vi.fn(); + let mockOnDebugMessage = vi.fn(); + let mockHandleSlashCommand = vi.fn().mockResolvedValue(false); let mockScheduleToolCalls: Mock; let mockCancelAllToolCalls: Mock; let mockMarkToolsAsSubmitted: Mock; let handleAtCommandSpy: MockInstance; - beforeEach(() => { - vi.clearAllMocks(); // Clear mocks before each test + const emptyHistory: any[] = []; + let capturedOnComplete: any = null; + const mockGetPreferredEditor = vi.fn(() => 'vscode' as EditorType); + const mockOnAuthError = vi.fn(); + const mockPerformMemoryRefresh = vi.fn(() => Promise.resolve()); + const mockSetModelSwitchedFromQuotaError = vi.fn(); + const mockOnCancelSubmit = vi.fn(); + const mockSetShellInputFocused = vi.fn(); - mockAddItem = vi.fn(); - // Define the mock for getGeminiClient - const mockGetGeminiClient = vi.fn().mockImplementation(() => { - // MockedGeminiClientClass is defined in the module scope by the previous change. - // It will use the mockStartChat and mockSendMessageStream that are managed within beforeEach. - const clientInstance = new MockedGeminiClientClass(mockConfig); - return clientInstance; - }); + const mockGetGeminiClient = vi.fn().mockImplementation(() => { + const clientInstance = new MockedGeminiClientClass(mockConfig); + return clientInstance; + }); - const mockMcpClientManager = { - getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED), - getMcpServerCount: vi.fn().mockReturnValue(0), - }; + const mockMcpClientManager = { + getDiscoveryState: vi.fn().mockReturnValue(MCPDiscoveryState.COMPLETED), + getMcpServerCount: vi.fn().mockReturnValue(0), + }; - const contentGeneratorConfig = { + const mockConfig: Config = { + apiKey: 'test-api-key', + model: 'gemini-pro', + sandbox: false, + targetDir: '/test/dir', + debugMode: false, + question: undefined, + coreTools: [], + toolDiscoveryCommand: undefined, + toolCallCommand: undefined, + mcpServerCommand: undefined, + mcpServers: undefined, + userAgent: 'test-agent', + userMemory: '', + geminiMdFileCount: 0, + alwaysSkipModificationConfirmation: false, + vertexai: false, + showMemoryUsage: false, + contextFileName: undefined, + storage: { + getProjectTempDir: vi.fn(() => '/test/temp'), + getProjectTempCheckpointsDir: vi.fn(() => '/test/temp/checkpoints'), + } as any, + getToolRegistry: vi.fn( + () => ({ getToolSchemaList: vi.fn(() => []) }) as any, + ), + getProjectRoot: vi.fn(() => '/test/dir'), + getCheckpointingEnabled: vi.fn(() => false), + getGeminiClient: mockGetGeminiClient, + getMcpClientManager: () => mockMcpClientManager as any, + getApprovalMode: vi.fn(() => ApprovalMode.DEFAULT), + getUsageStatisticsEnabled: () => true, + getDebugMode: () => false, + addHistory: vi.fn(), + getSessionId: vi.fn(() => 'test-session-id'), + setQuotaErrorOccurred: vi.fn(), + getQuotaErrorOccurred: vi.fn(() => false), + getModel: vi.fn(() => 'gemini-2.5-pro'), + getContentGeneratorConfig: vi.fn(() => ({ model: 'test-model', apiKey: 'test-key', vertexai: false, authType: AuthType.USE_GEMINI, - }; + })), + getContentGenerator: vi.fn(), + isInteractive: () => false, + getExperiments: () => {}, + isEventDrivenSchedulerEnabled: vi.fn(() => false), + getMaxSessionTurns: vi.fn(() => 100), + isJitContextEnabled: vi.fn(() => false), + getGlobalMemory: vi.fn(() => ''), + getUserMemory: vi.fn(() => ''), + getIdeMode: vi.fn(() => false), + getEnableHooks: vi.fn(() => false), + } as unknown as Config; - mockConfig = { - apiKey: 'test-api-key', - model: 'gemini-pro', - sandbox: false, - targetDir: '/test/dir', - debugMode: false, - question: undefined, - - coreTools: [], - toolDiscoveryCommand: undefined, - toolCallCommand: undefined, - mcpServerCommand: undefined, - mcpServers: undefined, - userAgent: 'test-agent', - userMemory: '', - geminiMdFileCount: 0, - alwaysSkipModificationConfirmation: false, - vertexai: false, - showMemoryUsage: false, - contextFileName: undefined, - getToolRegistry: vi.fn( - () => ({ getToolSchemaList: vi.fn(() => []) }) as any, - ), - getProjectRoot: vi.fn(() => '/test/dir'), - getCheckpointingEnabled: vi.fn(() => false), - getGeminiClient: mockGetGeminiClient, - getMcpClientManager: () => mockMcpClientManager as any, - getApprovalMode: () => ApprovalMode.DEFAULT, - getUsageStatisticsEnabled: () => true, - getDebugMode: () => false, - getWorkingDir: () => '/working/dir', - addHistory: vi.fn(), - getSessionId() { - return 'test-session-id'; - }, - setQuotaErrorOccurred: vi.fn(), - getQuotaErrorOccurred: vi.fn(() => false), - getModel: vi.fn(() => 'gemini-2.5-pro'), - getContentGenerator: vi.fn(), - getContentGeneratorConfig: vi - .fn() - .mockReturnValue(contentGeneratorConfig), - isInteractive: () => false, - getExperiments: () => {}, - } as unknown as Config; + beforeEach(() => { + vi.clearAllMocks(); // Clear mocks before each test + mockAddItem = vi.fn(); mockOnDebugMessage = vi.fn(); mockHandleSlashCommand = vi.fn().mockResolvedValue(false); @@ -253,6 +263,10 @@ describe('useGeminiStream', () => { mockCancelAllToolCalls = vi.fn(); mockMarkToolsAsSubmitted = vi.fn(); + // Reset properties of mockConfig if needed + (mockConfig.getCheckpointingEnabled as Mock).mockReturnValue(false); + (mockConfig.getApprovalMode as Mock).mockReturnValue(ApprovalMode.DEFAULT); + // Default mock for useReactToolScheduler to prevent toolCalls being undefined initially mockUseToolScheduler.mockReturnValue([ [], // Default to empty array for toolCalls @@ -289,10 +303,11 @@ describe('useGeminiStream', () => { geminiClient?: any, ) => { const client = geminiClient || mockConfig.getGeminiClient(); + let lastToolCalls = initialToolCalls; const initialProps = { client, - history: [], + history: emptyHistory, addItem: mockAddItem as unknown as UseHistoryManagerReturn['addItem'], config: mockConfig, onDebugMessage: mockOnDebugMessage, @@ -304,31 +319,26 @@ describe('useGeminiStream', () => { toolCalls: initialToolCalls, }; - const { result, rerender } = renderHook( - (props: typeof initialProps) => { - // This mock needs to be stateful. When setToolCallsForDisplay is called, - // it should trigger a rerender with the new state. - const mockSetToolCallsForDisplay = vi.fn((updater) => { - const newToolCalls = - typeof updater === 'function' ? updater(props.toolCalls) : updater; - rerender({ ...props, toolCalls: newToolCalls }); - }); - - // Create a stateful mock for cancellation that updates the toolCalls state. - const statefulCancelAllToolCalls = vi.fn((...args) => { - // Call the original spy so `toHaveBeenCalled` checks still work. + mockUseToolScheduler.mockImplementation((onComplete) => { + capturedOnComplete = onComplete; + return [ + lastToolCalls, + mockScheduleToolCalls, + mockMarkToolsAsSubmitted, + (updater: any) => { + lastToolCalls = + typeof updater === 'function' ? updater(lastToolCalls) : updater; + rerender({ ...initialProps, toolCalls: lastToolCalls }); + }, + (...args: any[]) => { mockCancelAllToolCalls(...args); - - const newToolCalls = props.toolCalls.map((tc) => { - // Only cancel tools that are in a cancellable state. + lastToolCalls = lastToolCalls.map((tc) => { if ( tc.status === 'awaiting_approval' || tc.status === 'executing' || tc.status === 'scheduled' || tc.status === 'validating' ) { - // A real cancelled tool call has a response object. - // We need to simulate this to avoid type errors downstream. return { ...tc, status: 'cancelled', @@ -337,23 +347,20 @@ describe('useGeminiStream', () => { responseParts: [], resultDisplay: 'Request cancelled.', }, - responseSubmittedToGemini: true, // Mark as "processed" + responseSubmittedToGemini: true, } as any as TrackedCancelledToolCall; } return tc; }); - rerender({ ...props, toolCalls: newToolCalls }); - }); + rerender({ ...initialProps, toolCalls: lastToolCalls }); + }, + 0, + ]; + }); - mockUseToolScheduler.mockImplementation(() => [ - props.toolCalls, - mockScheduleToolCalls, - mockMarkToolsAsSubmitted, - mockSetToolCallsForDisplay, - statefulCancelAllToolCalls, // Use the stateful mock - ]); - - return useGeminiStream( + const { result, rerender } = renderHookWithProviders( + (props: typeof initialProps) => + useGeminiStream( props.client, props.history, props.addItem, @@ -362,17 +369,16 @@ describe('useGeminiStream', () => { props.onDebugMessage, props.handleSlashCommand, props.shellModeActive, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), + mockGetPreferredEditor, + mockOnAuthError, + mockPerformMemoryRefresh, false, - () => {}, - () => {}, - () => {}, + mockSetModelSwitchedFromQuotaError, + mockOnCancelSubmit, + mockSetShellInputFocused, 80, 24, - ); - }, + ), { initialProps, }, @@ -454,7 +460,7 @@ describe('useGeminiStream', () => { modelSwitched = false, } = options; - return renderHook(() => + return renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -592,10 +598,17 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockImplementation((onComplete) => { capturedOnComplete = onComplete; - return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn()]; + return [ + [], + mockScheduleToolCalls, + mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, + ]; }); - renderHook(() => + renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -620,6 +633,8 @@ describe('useGeminiStream', () => { // Trigger the onComplete callback with completed tools await act(async () => { if (capturedOnComplete) { + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete(completedToolCalls); } }); @@ -674,10 +689,17 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockImplementation((onComplete) => { capturedOnComplete = onComplete; - return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn()]; + return [ + [], + mockScheduleToolCalls, + mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, + ]; }); - renderHook(() => + renderHookWithProviders(() => useGeminiStream( client, [], @@ -702,6 +724,8 @@ describe('useGeminiStream', () => { // Trigger the onComplete callback with cancelled tools await act(async () => { if (capturedOnComplete) { + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete(cancelledToolCalls); } }); @@ -746,48 +770,12 @@ describe('useGeminiStream', () => { ]; const client = new MockedGeminiClientClass(mockConfig); - // Capture the onComplete callback - let capturedOnComplete: - | ((completedTools: TrackedToolCall[]) => Promise) - | null = null; - - mockUseToolScheduler.mockImplementation((onComplete) => { - capturedOnComplete = onComplete; - return [ - [], - mockScheduleToolCalls, - mockMarkToolsAsSubmitted, - vi.fn(), - mockCancelAllToolCalls, - ]; - }); - - const { result } = renderHook(() => - useGeminiStream( - client, - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - 80, - 24, - ), - ); + const { result } = renderTestHook([], client); // Trigger the onComplete callback with STOP_EXECUTION tool await act(async () => { if (capturedOnComplete) { - await (capturedOnComplete as any)(stopExecutionToolCalls); + await capturedOnComplete(stopExecutionToolCalls); } }); @@ -877,10 +865,17 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockImplementation((onComplete) => { capturedOnComplete = onComplete; - return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn()]; + return [ + [], + mockScheduleToolCalls, + mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, + ]; }); - renderHook(() => + renderHookWithProviders(() => useGeminiStream( client, [], @@ -905,6 +900,8 @@ describe('useGeminiStream', () => { // Trigger the onComplete callback with multiple cancelled tools await act(async () => { if (capturedOnComplete) { + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete(allCancelledTools); } }); @@ -990,10 +987,12 @@ describe('useGeminiStream', () => { mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn(), // setToolCallsForDisplay + mockCancelAllToolCalls, + 0, ]; }); - const { result, rerender } = renderHook(() => + const { result, rerender } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -1027,6 +1026,8 @@ describe('useGeminiStream', () => { mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn(), // setToolCallsForDisplay + mockCancelAllToolCalls, + 0, ]; }); @@ -1041,6 +1042,8 @@ describe('useGeminiStream', () => { // 4. Trigger the onComplete callback to simulate tool completion await act(async () => { if (capturedOnComplete) { + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete(completedToolCalls); } }); @@ -1124,7 +1127,7 @@ describe('useGeminiStream', () => { })(); mockSendMessageStream.mockReturnValue(mockStream); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( mockConfig.getGeminiClient(), [], @@ -1165,7 +1168,7 @@ describe('useGeminiStream', () => { })(); mockSendMessageStream.mockReturnValue(mockStream); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( mockConfig.getGeminiClient(), [], @@ -1559,7 +1562,7 @@ describe('useGeminiStream', () => { }); it('should not call handleSlashCommand is shell mode is active', async () => { - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -1629,10 +1632,17 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockImplementation((onComplete) => { capturedOnComplete = onComplete; - return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted, vi.fn()]; + return [ + [], + mockScheduleToolCalls, + mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, + ]; }); - renderHook(() => + renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -1657,6 +1667,8 @@ describe('useGeminiStream', () => { // Trigger the onComplete callback with the completed save_memory tool await act(async () => { if (capturedOnComplete) { + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete([completedToolCall]); } }); @@ -1689,7 +1701,7 @@ describe('useGeminiStream', () => { getModel: vi.fn(() => 'gemini-2.5-pro'), } as unknown as Config; - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(testConfig), [], @@ -1990,7 +2002,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2095,7 +2107,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2243,6 +2255,8 @@ describe('useGeminiStream', () => { startTime: Date.now(), endTime: Date.now(), })); + // Wait a tick for refs to be set up + await new Promise((resolve) => setTimeout(resolve, 0)); await capturedOnComplete(tools); addItemOrder.push('scheduleToolCalls_END'); }); @@ -2264,7 +2278,7 @@ describe('useGeminiStream', () => { ]; }); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2330,7 +2344,7 @@ describe('useGeminiStream', () => { shouldProceed: true, }); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( mockConfig.getGeminiClient(), [], @@ -2489,7 +2503,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2565,11 +2579,13 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockReturnValue([ [], mockScheduleToolCalls, - mockCancelAllToolCalls, mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, ]); - const { result, rerender } = renderHook(() => + const { result, rerender } = renderHookWithProviders(() => useGeminiStream( mockConfig.getGeminiClient(), [], @@ -2616,8 +2632,10 @@ describe('useGeminiStream', () => { mockUseToolScheduler.mockReturnValue([ newToolCalls, mockScheduleToolCalls, - mockCancelAllToolCalls, mockMarkToolsAsSubmitted, + vi.fn(), + mockCancelAllToolCalls, + 0, ]); rerender(); @@ -2638,7 +2656,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2695,7 +2713,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], @@ -2763,7 +2781,7 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => + const { result } = renderHookWithProviders(() => useGeminiStream( new MockedGeminiClientClass(mockConfig), [], diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index b6da0a84d5..7ae0fdab6a 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -51,6 +51,7 @@ import type { HistoryItem, HistoryItemWithoutId, HistoryItemToolGroup, + IndividualToolCallDisplay, SlashCommandProcessorResult, HistoryItemModel, } from '../types.js'; @@ -91,6 +92,48 @@ function showCitations(settings: LoadedSettings): boolean { return true; } +/** + * Calculates the current streaming state based on tool call status and responding flag. + */ +function calculateStreamingState( + isResponding: boolean, + toolCalls: TrackedToolCall[], +): StreamingState { + if (toolCalls.some((tc) => tc.status === 'awaiting_approval')) { + return StreamingState.WaitingForConfirmation; + } + + const isAnyToolActive = toolCalls.some((tc) => { + // These statuses indicate active processing + if ( + tc.status === 'executing' || + tc.status === 'scheduled' || + tc.status === 'validating' + ) { + return true; + } + + // Terminal statuses (success, error, cancelled) still count as "Responding" + // if the result hasn't been submitted back to Gemini yet. + if ( + tc.status === 'success' || + tc.status === 'error' || + tc.status === 'cancelled' + ) { + return !(tc as TrackedCompletedToolCall | TrackedCancelledToolCall) + .responseSubmittedToGemini; + } + + return false; + }); + + if (isResponding || isAnyToolActive) { + return StreamingState.Responding; + } + + return StreamingState.Idle; +} + /** * Manages the Gemini stream, including user input, command processing, * API interaction, and tool call lifecycle. @@ -130,6 +173,10 @@ export const useGeminiStream = ( useStateAndRef(null); const [lastGeminiActivityTime, setLastGeminiActivityTime] = useState(0); + const [pushedToolCallIds, pushedToolCallIdsRef, setPushedToolCallIds] = + useStateAndRef>(new Set()); + const [_isFirstToolInGroup, isFirstToolInGroupRef, setIsFirstToolInGroup] = + useStateAndRef(true); const processedMemoryToolsRef = useRef>(new Set()); const { startNewPrompt, getPromptCount } = useSessionStats(); const storage = config.storage; @@ -162,12 +209,18 @@ export const useGeminiStream = ( async (completedToolCallsFromScheduler) => { // This onComplete is called when ALL scheduled tools for a given batch are done. if (completedToolCallsFromScheduler.length > 0) { - // Add the final state of these tools to the history for display. - addItem( - mapTrackedToolCallsToDisplay( - completedToolCallsFromScheduler as TrackedToolCall[], - ), + // Add only the tools that haven't been pushed to history yet. + const toolsToPush = completedToolCallsFromScheduler.filter( + (tc) => !pushedToolCallIdsRef.current.has(tc.request.callId), ); + if (toolsToPush.length > 0) { + addItem( + mapTrackedToolCallsToDisplay(toolsToPush as TrackedToolCall[], { + borderTop: isFirstToolInGroupRef.current, + borderBottom: true, + }), + ); + } // Clear the live-updating display now that the final state is in history. setToolCallsForDisplay([]); @@ -205,12 +258,139 @@ export const useGeminiStream = ( getPreferredEditor, ); - const pendingToolCallGroupDisplay = useMemo( - () => - toolCalls.length ? mapTrackedToolCallsToDisplay(toolCalls) : undefined, - [toolCalls], + const streamingState = useMemo( + () => calculateStreamingState(isResponding, toolCalls), + [isResponding, toolCalls], ); + // Reset tracking when a new batch of tools starts + useEffect(() => { + if (toolCalls.length > 0) { + const isNewBatch = !toolCalls.some((tc) => + pushedToolCallIdsRef.current.has(tc.request.callId), + ); + if (isNewBatch) { + setPushedToolCallIds(new Set()); + setIsFirstToolInGroup(true); + } + } else if (streamingState === StreamingState.Idle) { + // Clear when idle to be ready for next turn + setPushedToolCallIds(new Set()); + setIsFirstToolInGroup(true); + } + }, [ + toolCalls, + pushedToolCallIdsRef, + setPushedToolCallIds, + setIsFirstToolInGroup, + streamingState, + ]); + + // Push completed tools to history as they finish + useEffect(() => { + const toolsToPush: TrackedToolCall[] = []; + for (const tc of toolCalls) { + if (pushedToolCallIdsRef.current.has(tc.request.callId)) continue; + + if ( + tc.status === 'success' || + tc.status === 'error' || + tc.status === 'cancelled' + ) { + toolsToPush.push(tc); + } else { + // Stop at first non-terminal tool to preserve order + break; + } + } + + if (toolsToPush.length > 0) { + const newPushed = new Set(pushedToolCallIdsRef.current); + let isFirst = isFirstToolInGroupRef.current; + + for (const tc of toolsToPush) { + newPushed.add(tc.request.callId); + const isLastInBatch = tc === toolCalls[toolCalls.length - 1]; + + const historyItem = mapTrackedToolCallsToDisplay(tc, { + borderTop: isFirst, + borderBottom: isLastInBatch, + }); + addItem(historyItem); + isFirst = false; + } + + setPushedToolCallIds(newPushed); + setIsFirstToolInGroup(false); + } + }, [ + toolCalls, + pushedToolCallIdsRef, + isFirstToolInGroupRef, + setPushedToolCallIds, + setIsFirstToolInGroup, + addItem, + ]); + + const pendingToolGroupItems = useMemo((): HistoryItemWithoutId[] => { + const remainingTools = toolCalls.filter( + (tc) => !pushedToolCallIds.has(tc.request.callId), + ); + + const items: HistoryItemWithoutId[] = []; + + if (remainingTools.length > 0) { + items.push( + mapTrackedToolCallsToDisplay(remainingTools, { + borderTop: pushedToolCallIds.size === 0, + borderBottom: false, // Stay open to connect with the slice below + }), + ); + } + + // Always show a bottom border slice if we have ANY tools in the batch + // and we haven't finished pushing the whole batch to history yet. + // Once all tools are terminal and pushed, the last history item handles the closing border. + const allTerminal = + toolCalls.length > 0 && + toolCalls.every( + (tc) => + tc.status === 'success' || + tc.status === 'error' || + tc.status === 'cancelled', + ); + + const allPushed = + toolCalls.length > 0 && + toolCalls.every((tc) => pushedToolCallIds.has(tc.request.callId)); + + const isEventDriven = config.isEventDrivenSchedulerEnabled(); + const anyVisibleInHistory = pushedToolCallIds.size > 0; + const anyVisibleInPending = remainingTools.some((tc) => { + if (!isEventDriven) return true; + return ( + tc.status !== 'scheduled' && + tc.status !== 'validating' && + tc.status !== 'awaiting_approval' + ); + }); + + if ( + toolCalls.length > 0 && + !(allTerminal && allPushed) && + (anyVisibleInHistory || anyVisibleInPending) + ) { + items.push({ + type: 'tool_group' as const, + tools: [] as IndividualToolCallDisplay[], + borderTop: false, + borderBottom: true, + }); + } + + return items; + }, [toolCalls, pushedToolCallIds, config]); + const activeToolPtyId = useMemo(() => { const executingShellTool = toolCalls?.find( (tc) => @@ -271,29 +451,6 @@ export const useGeminiStream = ( prevActiveShellPtyIdRef.current = activeShellPtyId; }, [activeShellPtyId, addItem]); - const streamingState = useMemo(() => { - if (toolCalls.some((tc) => tc.status === 'awaiting_approval')) { - return StreamingState.WaitingForConfirmation; - } - if ( - isResponding || - toolCalls.some( - (tc) => - tc.status === 'executing' || - tc.status === 'scheduled' || - tc.status === 'validating' || - ((tc.status === 'success' || - tc.status === 'error' || - tc.status === 'cancelled') && - !(tc as TrackedCompletedToolCall | TrackedCancelledToolCall) - .responseSubmittedToGemini), - ) - ) { - return StreamingState.Responding; - } - return StreamingState.Idle; - }, [isResponding, toolCalls]); - useEffect(() => { if ( config.getApprovalMode() === ApprovalMode.YOLO && @@ -1349,10 +1506,10 @@ export const useGeminiStream = ( const pendingHistoryItems = useMemo( () => - [pendingHistoryItem, pendingToolCallGroupDisplay].filter( - (i) => i !== undefined && i !== null, + [pendingHistoryItem, ...pendingToolGroupItems].filter( + (i): i is HistoryItemWithoutId => i !== undefined && i !== null, ), - [pendingHistoryItem, pendingToolCallGroupDisplay], + [pendingHistoryItem, pendingToolGroupItems], ); useEffect(() => { diff --git a/packages/cli/src/ui/hooks/useStateAndRef.ts b/packages/cli/src/ui/hooks/useStateAndRef.ts index 8a10bab4cc..d8dce68ec8 100644 --- a/packages/cli/src/ui/hooks/useStateAndRef.ts +++ b/packages/cli/src/ui/hooks/useStateAndRef.ts @@ -11,7 +11,7 @@ import React from 'react'; // times in the same function. export const useStateAndRef = < // Everything but function. - T extends object | null | undefined | number | string, + T extends object | null | undefined | number | string | boolean, >( initialValue: T, ) => { diff --git a/packages/cli/src/ui/hooks/useToolScheduler.ts b/packages/cli/src/ui/hooks/useToolScheduler.ts index 079e3f1327..3a6d38aff4 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.ts @@ -36,6 +36,8 @@ export type { TrackedExecutingToolCall, TrackedCompletedToolCall, TrackedCancelledToolCall, + MarkToolsAsSubmittedFn, + CancelAllFn, }; // Unified type that covers both implementations diff --git a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx index 004503696e..7c22e46ac3 100644 --- a/packages/cli/src/ui/layouts/DefaultAppLayout.tsx +++ b/packages/cli/src/ui/layouts/DefaultAppLayout.tsx @@ -15,21 +15,11 @@ 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 @@ -65,12 +55,7 @@ export const DefaultAppLayout: React.FC = () => { addItem={uiState.historyManager.addItem} /> ) : ( - <> - {showConfirmationQueue && confirmingTool && ( - - )} - - + )} diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index ae865d2488..5cf8b42619 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -180,6 +180,8 @@ export type HistoryItemQuit = HistoryItemBase & { export type HistoryItemToolGroup = HistoryItemBase & { type: 'tool_group'; tools: IndividualToolCallDisplay[]; + borderTop?: boolean; + borderBottom?: boolean; }; export type HistoryItemUserShell = HistoryItemBase & {