diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index 5e3f3666b1..7cbfdc0d61 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -97,6 +97,8 @@ export async function runAcpClient( await connection.closed.finally(runExitCleanup); } +let callIdCounter = 0; + export class GeminiAgent { private sessions: Map = new Map(); private clientCapabilities: acp.ClientCapabilities | undefined; @@ -897,7 +899,7 @@ export class Session { promptId: string, fc: FunctionCall, ): Promise { - const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const callId = fc.id ?? `${fc.name}-${Date.now()}-${++callIdCounter}`; const args = fc.args ?? {}; const startTime = Date.now(); @@ -1370,7 +1372,7 @@ export class Session { include: pathSpecsToRead, }; - const callId = `${readManyFilesTool.name}-${Date.now()}`; + const callId = `${readManyFilesTool.name}-${Date.now()}-${++callIdCounter}`; try { const invocation = readManyFilesTool.build(toolArgs); diff --git a/packages/cli/src/test-utils/render.tsx b/packages/cli/src/test-utils/render.tsx index 7d298b120d..a20a60ed5e 100644 --- a/packages/cli/src/test-utils/render.tsx +++ b/packages/cli/src/test-utils/render.tsx @@ -16,7 +16,7 @@ import { vi } from 'vitest'; import stripAnsi from 'strip-ansi'; import type React from 'react'; import { act, useState } from 'react'; -import type { LoadedSettings } from '../config/settings.js'; +import { LoadedSettings } from '../config/settings.js'; import { KeypressProvider } from '../ui/contexts/KeypressContext.js'; import { SettingsContext } from '../ui/contexts/SettingsContext.js'; import { ShellFocusContext } from '../ui/contexts/ShellFocusContext.js'; @@ -604,16 +604,18 @@ export const renderWithProviders = async ( uiState: providedUiState, width, mouseEventsEnabled = false, + useAlternateBuffer: explicitUseAlternateBuffer, config, uiActions, persistentState, appState = mockAppState, }: { shellFocus?: boolean; - settings?: LoadedSettings; + settings?: LoadedSettings | Partial; uiState?: Partial; width?: number; mouseEventsEnabled?: boolean; + useAlternateBuffer?: boolean; config?: Config; uiActions?: Partial; persistentState?: { @@ -660,15 +662,34 @@ export const renderWithProviders = async ( const terminalWidth = width ?? baseState.terminalWidth; + const finalSettings = + settings instanceof LoadedSettings + ? settings + : createMockSettings(settings || {}); + if (!config) { config = await loadCliConfig( - settings.merged, + finalSettings.merged, 'random-session-id', - {} as unknown as CliArgs, + {} as CliArgs, { cwd: '/' }, ); } + const useAlternateBuffer = + explicitUseAlternateBuffer ?? + finalSettings.merged.ui?.useAlternateBuffer ?? + false; + + const finalConfig = new Proxy(config, { + get(target, prop) { + if (prop === 'getUseAlternateBuffer') { + return () => useAlternateBuffer; + } + return Reflect.get(target, prop); + }, + }); + const mainAreaWidth = terminalWidth; const finalUiState = { @@ -697,8 +718,8 @@ export const renderWithProviders = async ( const wrapWithProviders = (comp: React.ReactElement) => ( - - + + @@ -709,7 +730,7 @@ export const renderWithProviders = async ( ( wrapper?: React.ComponentType<{ children: React.ReactNode }>; // Options for renderWithProviders shellFocus?: boolean; - settings?: LoadedSettings; + settings?: LoadedSettings | Partial; uiState?: Partial; width?: number; mouseEventsEnabled?: boolean; @@ -864,7 +885,7 @@ export async function renderHookWithProviders( const Wrapper = options.wrapper || (({ children }) => <>{children}); - let renderResult: ReturnType; + let renderResult: Awaited>; await act(async () => { renderResult = await renderWithProviders( diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 9d05f54347..a6f2d1d99a 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -30,8 +30,6 @@ import { import { ConfigContext } from './contexts/ConfigContext.js'; import { type HistoryItem, - type HistoryItemWithoutId, - type HistoryItemToolGroup, AuthState, type ConfirmationRequest, type PermissionConfirmationRequest, @@ -81,7 +79,6 @@ import { type AgentsDiscoveredPayload, ChangeAuthRequestedError, ProjectIdRequiredError, - CoreToolCallStatus, buildUserSteeringHintPrompt, logBillingEvent, ApiKeyUpdatedEvent, @@ -170,29 +167,11 @@ import { useIsHelpDismissKey } from './utils/shortcutsHelp.js'; import { useSuspend } from './hooks/useSuspend.js'; import { useRunEventNotifications } from './hooks/useRunEventNotifications.js'; import { isNotificationsEnabled } from '../utils/terminalNotifications.js'; - -function isToolExecuting(pendingHistoryItems: HistoryItemWithoutId[]) { - return pendingHistoryItems.some((item) => { - if (item && item.type === 'tool_group') { - return item.tools.some( - (tool) => CoreToolCallStatus.Executing === tool.status, - ); - } - return false; - }); -} - -function isToolAwaitingConfirmation( - pendingHistoryItems: HistoryItemWithoutId[], -) { - return pendingHistoryItems - .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') - .some((item) => - item.tools.some( - (tool) => CoreToolCallStatus.AwaitingApproval === tool.status, - ), - ); -} +import { + isToolExecuting, + isToolAwaitingConfirmation, + getAllToolCalls, +} from './utils/historyUtils.js'; interface AppContainerProps { config: Config; @@ -1151,6 +1130,16 @@ Logging in with Google... Restarting Gemini CLI to continue. consumePendingHints, ); + const pendingHistoryItems = useMemo( + () => [...pendingSlashCommandHistoryItems, ...pendingGeminiHistoryItems], + [pendingSlashCommandHistoryItems, pendingGeminiHistoryItems], + ); + + const hasPendingToolConfirmation = useMemo( + () => isToolAwaitingConfirmation(pendingHistoryItems), + [pendingHistoryItems], + ); + toggleBackgroundShellRef.current = toggleBackgroundShell; isBackgroundShellVisibleRef.current = isBackgroundShellVisible; backgroundShellsRef.current = backgroundShells; @@ -1222,10 +1211,6 @@ Logging in with Google... Restarting Gemini CLI to continue. cancelHandlerRef.current = useCallback( (shouldRestorePrompt: boolean = true) => { - const pendingHistoryItems = [ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]; if (isToolAwaitingConfirmation(pendingHistoryItems)) { return; // Don't clear - user may be composing a follow-up message } @@ -1259,8 +1244,7 @@ Logging in with Google... Restarting Gemini CLI to continue. inputHistory, getQueuedMessagesText, clearQueue, - pendingSlashCommandHistoryItems, - pendingGeminiHistoryItems, + pendingHistoryItems, ], ); @@ -1296,10 +1280,7 @@ Logging in with Google... Restarting Gemini CLI to continue. const isIdle = streamingState === StreamingState.Idle; const isAgentRunning = streamingState === StreamingState.Responding || - isToolExecuting([ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]); + isToolExecuting(pendingHistoryItems); if (isSlash && isAgentRunning) { const { commandToExecute } = parseSlashCommand( @@ -1361,8 +1342,7 @@ Logging in with Google... Restarting Gemini CLI to continue. isMcpReady, streamingState, messageQueue.length, - pendingSlashCommandHistoryItems, - pendingGeminiHistoryItems, + pendingHistoryItems, config, constrainHeight, setConstrainHeight, @@ -1684,6 +1664,11 @@ Logging in with Google... Restarting Gemini CLI to continue. const handleGlobalKeypress = useCallback( (key: Key): boolean => { + // Debug log keystrokes if enabled + if (settings.merged.general.debugKeystrokeLogging) { + debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key)); + } + if (shortcutsHelpVisible && isHelpDismissKey(key)) { setShortcutsHelpVisible(false); } @@ -1862,6 +1847,7 @@ Logging in with Google... Restarting Gemini CLI to continue. activePtyId, handleSuspend, embeddedShellFocused, + settings.merged.general.debugKeystrokeLogging, refreshStatic, setCopyModeEnabled, tabFocusTimeoutRef, @@ -2022,16 +2008,6 @@ Logging in with Google... Restarting Gemini CLI to continue. authState === AuthState.AwaitingApiKeyInput || !!newAgents; - const pendingHistoryItems = useMemo( - () => [...pendingSlashCommandHistoryItems, ...pendingGeminiHistoryItems], - [pendingSlashCommandHistoryItems, pendingGeminiHistoryItems], - ); - - const hasPendingToolConfirmation = useMemo( - () => isToolAwaitingConfirmation(pendingHistoryItems), - [pendingHistoryItems], - ); - const hasConfirmUpdateExtensionRequests = confirmUpdateExtensionRequests.length > 0; const hasLoopDetectionConfirmationRequest = @@ -2121,12 +2097,7 @@ Logging in with Google... Restarting Gemini CLI to continue. ]); const allToolCalls = useMemo( - () => - pendingHistoryItems - .filter( - (item): item is HistoryItemToolGroup => item.type === 'tool_group', - ) - .flatMap((item) => item.tools), + () => getAllToolCalls(pendingHistoryItems), [pendingHistoryItems], ); @@ -2292,10 +2263,7 @@ Logging in with Google... Restarting Gemini CLI to continue. showIsExpandableHint, hintMode: config.isModelSteeringEnabled() && - isToolExecuting([ - ...pendingSlashCommandHistoryItems, - ...pendingGeminiHistoryItems, - ]), + isToolExecuting(pendingHistoryItems), hintBuffer: '', }), [ diff --git a/packages/cli/src/ui/components/AskUserDialog.test.tsx b/packages/cli/src/ui/components/AskUserDialog.test.tsx index 8ed240389c..8369f4ec49 100644 --- a/packages/cli/src/ui/components/AskUserDialog.test.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.test.tsx @@ -288,7 +288,7 @@ describe('AskUserDialog', () => { }); describe.each([ - { useAlternateBuffer: true, expectedArrows: false }, + { useAlternateBuffer: true, expectedArrows: true }, { useAlternateBuffer: false, expectedArrows: true }, ])( 'Scroll Arrows (useAlternateBuffer: $useAlternateBuffer)', diff --git a/packages/cli/src/ui/components/AskUserDialog.tsx b/packages/cli/src/ui/components/AskUserDialog.tsx index b1d23885e6..f845c3dcfd 100644 --- a/packages/cli/src/ui/components/AskUserDialog.tsx +++ b/packages/cli/src/ui/components/AskUserDialog.tsx @@ -857,8 +857,14 @@ const ChoiceQuestionView: React.FC = ({ : undefined; const maxItemsToShow = - listHeight && questionHeightLimit - ? Math.max(1, Math.floor((listHeight - questionHeightLimit) / 2)) + listHeight && (!isAlternateBuffer || availableHeight !== undefined) + ? Math.min( + selectionItems.length, + Math.max( + 1, + Math.floor((listHeight - (questionHeightLimit ?? 0)) / 2), + ), + ) : selectionItems.length; return ( diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index b2c18aa7d8..66ebc772e9 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -97,7 +97,7 @@ describe('getToolGroupBorderAppearance', () => { }); it('inspects only the last pending tool_group item if current has no tools', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; const pendingItems = [ { type: 'tool_group' as const, @@ -158,7 +158,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -187,7 +187,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -276,7 +276,7 @@ describe('getToolGroupBorderAppearance', () => { confirmationDetails: undefined, } as IndividualToolCallDisplay, ], - id: 1, + id: -1, }; const result = getToolGroupBorderAppearance( item, @@ -292,7 +292,7 @@ describe('getToolGroupBorderAppearance', () => { }); it('handles empty tools with active shell turn (isCurrentlyInShellTurn)', () => { - const item = { type: 'tool_group' as const, tools: [], id: 1 }; + const item = { type: 'tool_group' as const, tools: [], id: -1 }; // active shell turn const result = getToolGroupBorderAppearance( @@ -693,7 +693,7 @@ describe('MainContent', () => { pendingHistoryItems: [ { type: 'tool_group', - id: 1, + id: -1, tools: [ { callId: 'call_1', diff --git a/packages/cli/src/ui/components/MainContent.tsx b/packages/cli/src/ui/components/MainContent.tsx index 0530e171b8..98b99d384f 100644 --- a/packages/cli/src/ui/components/MainContent.tsx +++ b/packages/cli/src/ui/components/MainContent.tsx @@ -88,6 +88,7 @@ export const MainContent = () => { () => augmentedHistory.map( ({ item, isExpandable, isFirstThinking, isFirstAfterThinking }) => ( + // ({ item, isExpandable, isFirstThinking, /* isFirstAfterThinking */ }) => ( { const pendingItems = useMemo( () => ( - + {pendingHistoryItems.map((item, i) => { const prevType = i === 0 @@ -140,12 +141,12 @@ export const MainContent = () => { return ( { ); })} {showConfirmationQueue && confirmingTool && ( - + )} ), diff --git a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap index 30caf0fb40..c0fae8bf19 100644 --- a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap @@ -39,37 +39,14 @@ Enter to select · ↑/↓ to navigate · Esc to cancel exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 1`] = ` "Choose an option +▲ ● 1. Option 1 Description 1 2. Option 2 Description 2 3. Option 3 Description 3 - 4. Option 4 - Description 4 - 5. Option 5 - Description 5 - 6. Option 6 - Description 6 - 7. Option 7 - Description 7 - 8. Option 8 - Description 8 - 9. Option 9 - Description 9 - 10. Option 10 - Description 10 - 11. Option 11 - Description 11 - 12. Option 12 - Description 12 - 13. Option 13 - Description 13 - 14. Option 14 - Description 14 - 15. Option 15 - Description 15 - 16. Enter a custom value +▼ Enter to select · ↑/↓ to navigate · Esc to cancel " 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 785dc6b6f0..4a5813f814 100644 --- a/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/MainContent.test.tsx.snap @@ -6,12 +6,11 @@ AppHeader(full) ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ Line 9 │ │ Line 10 │ │ Line 11 │ │ Line 12 │ │ Line 13 │ -│ Line 14 █ │ +│ Line 14 │ │ Line 15 █ │ │ Line 16 █ │ │ Line 17 █ │ @@ -28,12 +27,11 @@ AppHeader(full) ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ Line 9 │ │ Line 10 │ │ Line 11 │ │ Line 12 │ │ Line 13 │ -│ Line 14 █ │ +│ Line 14 │ │ Line 15 █ │ │ Line 16 █ │ │ Line 17 █ │ @@ -49,8 +47,7 @@ exports[`MainContent > MainContent Tool Output Height Logic > 'Normal mode - Con ╭──────────────────────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command Running a long command... │ │ │ -│ ... first 9 lines hidden (Ctrl+O to show) ... │ -│ Line 10 │ +│ ... first 10 lines hidden (Ctrl+O to show) ... │ │ Line 11 │ │ Line 12 │ │ Line 13 │ diff --git a/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx index 6135d3574e..e5f95465ca 100644 --- a/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ShellToolMessage.test.tsx @@ -185,28 +185,28 @@ describe('', () => { [ 'respects availableTerminalHeight when it is smaller than ACTIVE_SHELL_MAX_LINES', 10, - 8, + 7, false, true, ], [ 'uses ACTIVE_SHELL_MAX_LINES when availableTerminalHeight is large', 100, - ACTIVE_SHELL_MAX_LINES - 3, + ACTIVE_SHELL_MAX_LINES - 4, false, true, ], [ 'uses full availableTerminalHeight when focused in alternate buffer mode', 100, - 98, + 97, true, false, ], [ 'defaults to ACTIVE_SHELL_MAX_LINES in alternate buffer when availableTerminalHeight is undefined', undefined, - ACTIVE_SHELL_MAX_LINES - 3, + ACTIVE_SHELL_MAX_LINES - 4, false, false, ], @@ -328,8 +328,8 @@ describe('', () => { await waitUntilReady(); await waitFor(() => { const frame = lastFrame(); - // Should still be constrained to 12 (15 - 3) because isExpandable is false - expect(frame.match(/Line \d+/g)?.length).toBe(12); + // Should still be constrained to 11 (15 - 4) because isExpandable is false + expect(frame.match(/Line \d+/g)?.length).toBe(11); }); expect(lastFrame()).toMatchSnapshot(); unmount(); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 5398f2c23f..4f62e3cce7 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -469,8 +469,7 @@ describe('ToolConfirmationMessage', () => { confirm: vi.fn(), cancel: vi.fn(), isDiffingEnabled: false, - }); - + }); const { lastFrame, waitUntilReady, unmount } = await renderWithProviders( { confirm: vi.fn(), cancel: vi.fn(), isDiffingEnabled: false, - }); - + }); const { lastFrame, waitUntilReady, unmount } = await renderWithProviders( { confirm: mockConfirm, cancel: vi.fn(), isDiffingEnabled: false, - }); - + }); const confirmationDetails: SerializableConfirmationDetails = { type: 'info', title: 'Confirm Web Fetch', diff --git a/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap b/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap index 1847b8ce67..967ea81e14 100644 --- a/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap +++ b/packages/cli/src/ui/components/messages/__snapshots__/ShellToolMessage.test.tsx.snap @@ -4,7 +4,6 @@ exports[` > Height Constraints > defaults to ACTIVE_SHELL_MA "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -14,7 +13,7 @@ exports[` > Height Constraints > defaults to ACTIVE_SHELL_MA │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -130,7 +129,6 @@ exports[` > Height Constraints > respects availableTerminalH "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 93 │ │ Line 94 │ │ Line 95 │ │ Line 96 │ @@ -145,7 +143,6 @@ exports[` > Height Constraints > stays constrained in altern "╭──────────────────────────────────────────────────────────────────────────────╮ │ ✓ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -155,7 +152,7 @@ exports[` > Height Constraints > stays constrained in altern │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -164,7 +161,6 @@ exports[` > Height Constraints > uses ACTIVE_SHELL_MAX_LINES "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command │ │ │ -│ Line 89 │ │ Line 90 │ │ Line 91 │ │ Line 92 │ @@ -174,7 +170,7 @@ exports[` > Height Constraints > uses ACTIVE_SHELL_MAX_LINES │ Line 96 │ │ Line 97 │ │ Line 98 │ -│ Line 99 ▄ │ +│ Line 99 │ │ Line 100 █ │ " `; @@ -183,10 +179,9 @@ exports[` > Height Constraints > uses full availableTerminal "╭──────────────────────────────────────────────────────────────────────────────╮ │ ⊶ Shell Command A shell command (Shift+Tab to unfocus) │ │ │ -│ Line 3 │ │ Line 4 │ -│ Line 5 █ │ -│ Line 6 █ │ +│ Line 5 │ +│ Line 6 │ │ Line 7 █ │ │ Line 8 █ │ │ Line 9 █ │ 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 5e5c7ea2b0..e34e66cc48 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 @@ -37,8 +37,7 @@ exports[`ToolResultDisplay > renders string result as plain text when renderOutp `; exports[`ToolResultDisplay > truncates very long string results 1`] = ` -"... 248 hidden (Ctrl+O) ... -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +"... 249 hidden (Ctrl+O) ... aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx index 8a75bf7d57..21f2d6b1db 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx @@ -72,16 +72,21 @@ describe('ToolActionsContext', () => { beforeEach(() => { vi.clearAllMocks(); + // Default to a pending promise to avoid unwanted async state updates in tests + // that don't specifically test the IdeClient initialization. + vi.mocked(IdeClient.getInstance).mockReturnValue(new Promise(() => {})); }); - const wrapper = ({ children }: { children: React.ReactNode }) => ( - - {children} - - ); + const Wrapper = ({ children }: { children: React.ReactNode }) => { + return ( + + {children} + + ); + }; it('publishes to MessageBus for tools with correlationId', async () => { - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); await result.current.confirm( 'modern-call', @@ -99,7 +104,7 @@ describe('ToolActionsContext', () => { }); it('handles cancel by calling confirm with Cancel outcome', async () => { - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); await result.current.cancel('modern-call'); @@ -119,7 +124,7 @@ describe('ToolActionsContext', () => { vi.mocked(IdeClient.getInstance).mockResolvedValue(mockIdeClient); vi.mocked(mockConfig.getIdeMode).mockReturnValue(true); - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); // Wait for IdeClient initialization in useEffect await act(async () => { @@ -157,7 +162,7 @@ describe('ToolActionsContext', () => { vi.mocked(IdeClient.getInstance).mockResolvedValue(mockIdeClient); vi.mocked(mockConfig.getIdeMode).mockReturnValue(true); - const { result } = renderHook(() => useToolActions(), { wrapper }); + const { result } = renderHook(() => useToolActions(), { wrapper: Wrapper }); // Wait for initialization await act(async () => { diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.tsx index 10e063e098..4168853cbb 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.tsx @@ -5,13 +5,7 @@ */ import type React from 'react'; -import { - createContext, - useContext, - useCallback, - useState, - useEffect, -} from 'react'; +import { createContext, useContext, useCallback, useState, useEffect } from 'react'; import { IdeClient, ToolConfirmationOutcome, @@ -52,7 +46,7 @@ interface ToolActionsContextValue { const ToolActionsContext = createContext(null); -export const useToolActions = () => { +export const useToolActions = (): ToolActionsContextValue => { const context = useContext(ToolActionsContext); if (!context) { throw new Error('useToolActions must be used within a ToolActionsProvider'); @@ -77,24 +71,23 @@ export const ToolActionsProvider: React.FC = ( useEffect(() => { let isMounted = true; + let activeClient: IdeClient | null = null; + + const handleStatusChange = () => { + if (isMounted && activeClient) { + setIsDiffingEnabled(activeClient.isDiffingEnabled()); + } + }; + if (config.getIdeMode()) { IdeClient.getInstance() .then((client) => { if (!isMounted) return; + activeClient = client; setIdeClient(client); setIsDiffingEnabled(client.isDiffingEnabled()); - const handleStatusChange = () => { - if (isMounted) { - setIsDiffingEnabled(client.isDiffingEnabled()); - } - }; - client.addStatusChangeListener(handleStatusChange); - // Return a cleanup function for the listener - return () => { - client.removeStatusChangeListener(handleStatusChange); - }; }) .catch((error) => { debugLogger.error('Failed to get IdeClient instance:', error); @@ -102,6 +95,9 @@ export const ToolActionsProvider: React.FC = ( } return () => { isMounted = false; + if (activeClient) { + activeClient.removeStatusChangeListener(handleStatusChange); + } }; }, [config]); @@ -164,7 +160,13 @@ export const ToolActionsProvider: React.FC = ( ); return ( - + {children} ); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.test.ts b/packages/cli/src/ui/hooks/useHistoryManager.test.ts index 696f9d60c0..b657e5d4ba 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.test.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.test.ts @@ -39,6 +39,56 @@ describe('useHistoryManager', () => { expect(result.current.history[0].id).toBeGreaterThanOrEqual(timestamp); }); + it('should generate strictly increasing IDs even if baseTimestamp goes backwards', () => { + const { result } = renderHook(() => useHistory()); + const timestamp = 1000000; + const itemData: Omit = { type: 'info', text: 'First' }; + + let id1!: number; + let id2!: number; + + act(() => { + id1 = result.current.addItem(itemData, timestamp); + // Try to add with a smaller timestamp + id2 = result.current.addItem(itemData, timestamp - 500); + }); + + expect(id1).toBe(timestamp); + expect(id2).toBe(id1 + 1); + expect(result.current.history[1].id).toBe(id2); + }); + + it('should ensure new IDs start after existing IDs when resuming a session', () => { + const initialItems: HistoryItem[] = [ + { id: 5000, type: 'info', text: 'Existing' }, + ]; + const { result } = renderHook(() => useHistory({ initialItems })); + + let newId!: number; + act(() => { + // Try to add with a timestamp smaller than the highest existing ID + newId = result.current.addItem({ type: 'info', text: 'New' }, 2000); + }); + + expect(newId).toBe(5001); + expect(result.current.history[1].id).toBe(5001); + }); + + it('should update lastIdRef when loading new history', () => { + const { result } = renderHook(() => useHistory()); + + act(() => { + result.current.loadHistory([{ id: 8000, type: 'info', text: 'Loaded' }]); + }); + + let newId!: number; + act(() => { + newId = result.current.addItem({ type: 'info', text: 'New' }, 1000); + }); + + expect(newId).toBe(8001); + }); + it('should generate unique IDs for items added with the same base timestamp', () => { const { result } = renderHook(() => useHistory()); const timestamp = Date.now(); @@ -215,8 +265,8 @@ describe('useHistoryManager', () => { const after = Date.now(); expect(result.current.history).toHaveLength(1); - // ID should be >= before + 1 (since counter starts at 0 and increments to 1) - expect(result.current.history[0].id).toBeGreaterThanOrEqual(before + 1); + // ID should be >= before (since baseTimestamp defaults to Date.now()) + expect(result.current.history[0].id).toBeGreaterThanOrEqual(before); expect(result.current.history[0].id).toBeLessThanOrEqual(after + 1); }); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.ts b/packages/cli/src/ui/hooks/useHistoryManager.ts index 93f7f01f28..c6ceabb920 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.ts @@ -42,16 +42,22 @@ export function useHistory({ initialItems?: HistoryItem[]; } = {}): UseHistoryManagerReturn { const [history, setHistory] = useState(initialItems); - const messageIdCounterRef = useRef(0); + const lastIdRef = useRef( + initialItems.reduce((max, item) => Math.max(max, item.id), 0), + ); - // Generates a unique message ID based on a timestamp and a counter. + // Generates a unique message ID based on a timestamp, ensuring it is always + // greater than any previously assigned ID. const getNextMessageId = useCallback((baseTimestamp: number): number => { - messageIdCounterRef.current += 1; - return baseTimestamp + messageIdCounterRef.current; + const nextId = Math.max(baseTimestamp, lastIdRef.current + 1); + lastIdRef.current = nextId; + return nextId; }, []); const loadHistory = useCallback((newHistory: HistoryItem[]) => { setHistory(newHistory); + const maxId = newHistory.reduce((max, item) => Math.max(max, item.id), 0); + lastIdRef.current = Math.max(lastIdRef.current, maxId); }, []); // Adds a new item to the history state with a unique ID. @@ -153,7 +159,7 @@ export function useHistory({ // Clears the entire history state and resets the ID counter. const clearItems = useCallback(() => { setHistory([]); - messageIdCounterRef.current = 0; + lastIdRef.current = 0; }, []); return useMemo( diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 2f8e414a83..f32d6fbc5c 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -16,13 +16,20 @@ import { type AgentDefinition, type ApprovalMode, type Kind, + type AnsiOutput, CoreToolCallStatus, checkExhaustive, } from '@google/gemini-cli-core'; import type { PartListUnion } from '@google/genai'; import { type ReactNode } from 'react'; -export type { ThoughtSummary, SkillDefinition }; +export { CoreToolCallStatus }; +export type { + ThoughtSummary, + SkillDefinition, + SerializableConfirmationDetails, + ToolResultDisplay, +}; export enum AuthState { // Attempting to authenticate or re-authenticate @@ -86,6 +93,88 @@ export function mapCoreStatusToDisplayStatus( } } +export interface DiffStat { + model_added_lines: number; + model_removed_lines: number; + model_added_chars: number; + model_removed_chars: number; + user_added_lines: number; + user_removed_lines: number; + user_added_chars: number; + user_removed_chars: number; +} + +export interface FileDiff { + fileDiff: string; + fileName: string; + filePath: string; + originalContent: string | null; + newContent: string; + diffStat?: DiffStat; + isNewFile?: boolean; +} + +export interface GrepMatch { + filePath: string; + lineNumber: number; + line: string; +} + +export interface GrepResult { + summary: string; + matches?: GrepMatch[]; + payload?: string; +} + +export interface ListDirectoryResult { + summary: string; + files?: string[]; + payload?: string; +} + +export interface ReadManyFilesResult { + summary: string; + files?: string[]; + skipped?: Array<{ path: string; reason: string }>; + include?: string[]; + excludes?: string[]; + targetDir?: string; + payload?: string; +} + +/** + * --- TYPE GUARDS --- + */ + +export const isFileDiff = (res: unknown): res is FileDiff => + typeof res === 'object' && res !== null && 'fileDiff' in res; + +export const isGrepResult = (res: unknown): res is GrepResult => + typeof res === 'object' && + res !== null && + 'summary' in res && + ('matches' in res || 'payload' in res); + +export const isListResult = ( + res: unknown, +): res is ListDirectoryResult | ReadManyFilesResult => + typeof res === 'object' && + res !== null && + 'summary' in res && + ('files' in res || 'include' in res); + +export const isTodoList = (res: unknown): res is { todos: unknown[] } => + typeof res === 'object' && res !== null && 'todos' in res; + +export const isAnsiOutput = (res: unknown): res is AnsiOutput => + Array.isArray(res) && (res.length === 0 || Array.isArray(res[0])); + +export const hasSummary = (res: unknown): res is { summary: string } => + typeof res === 'object' && + res !== null && + 'summary' in res && + typeof res.summary === 'string'; + export interface ToolCallEvent { type: 'tool_call'; status: CoreToolCallStatus; @@ -352,10 +441,6 @@ export type HistoryItemMcpStatus = HistoryItemBase & { showSchema: boolean; }; -// Using Omit seems to have some issues with typescript's -// type inference e.g. historyItem.type === 'tool_group' isn't auto-inferring that -// 'tools' in historyItem. -// Individually exported types extending HistoryItemBase export type HistoryItemWithoutId = | HistoryItemUser | HistoryItemUserShell diff --git a/packages/cli/src/ui/utils/CodeColorizer.test.tsx b/packages/cli/src/ui/utils/CodeColorizer.test.tsx index 2628a36d0a..b9d5c65877 100644 --- a/packages/cli/src/ui/utils/CodeColorizer.test.tsx +++ b/packages/cli/src/ui/utils/CodeColorizer.test.tsx @@ -82,4 +82,28 @@ describe('colorizeCode', () => { await expect(renderResult).toMatchSvgSnapshot(); renderResult.unmount(); }); + + it('returns an array of lines when returnLines is true', () => { + const code = 'line 1\nline 2\nline 3'; + const settings = new LoadedSettings( + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + { path: '', settings: {}, originalSettings: {} }, + true, + [], + ); + + const result = colorizeCode({ + code, + language: 'javascript', + maxWidth: 80, + settings, + hideLineNumbers: true, + returnLines: true, + }); + + expect(Array.isArray(result)).toBe(true); + expect(result).toHaveLength(3); + }); }); diff --git a/packages/cli/src/ui/utils/CodeColorizer.tsx b/packages/cli/src/ui/utils/CodeColorizer.tsx index 948a5f8988..94dda9501e 100644 --- a/packages/cli/src/ui/utils/CodeColorizer.tsx +++ b/packages/cli/src/ui/utils/CodeColorizer.tsx @@ -21,8 +21,8 @@ import { MaxSizedBox, MINIMUM_MAX_HEIGHT, } from '../components/shared/MaxSizedBox.js'; -import type { LoadedSettings } from '../../config/settings.js'; import { debugLogger } from '@google/gemini-cli-core'; +import type { LoadedSettings } from '../../config/settings.js'; // Configure theming and parsing utilities. const lowlight = createLowlight(common); @@ -117,7 +117,11 @@ export function colorizeLine( line: string, language: string | null, theme?: Theme, + disableColor = false, ): React.ReactNode { + if (disableColor) { + return {line}; + } const activeTheme = theme || themeManager.getActiveTheme(); return highlightAndRenderLine(line, language, activeTheme); } @@ -130,6 +134,8 @@ export interface ColorizeCodeOptions { theme?: Theme | null; settings: LoadedSettings; hideLineNumbers?: boolean; + disableColor?: boolean; + returnLines?: boolean; } /** @@ -138,6 +144,12 @@ export interface ColorizeCodeOptions { * @param options The options for colorizing the code. * @returns A React.ReactNode containing Ink elements for the highlighted code. */ +export function colorizeCode( + options: ColorizeCodeOptions & { returnLines: true }, +): React.ReactNode[]; +export function colorizeCode( + options: ColorizeCodeOptions & { returnLines?: false }, +): React.ReactNode; export function colorizeCode({ code, language = null, @@ -146,13 +158,16 @@ export function colorizeCode({ theme = null, settings, hideLineNumbers = false, -}: ColorizeCodeOptions): React.ReactNode { + disableColor = false, + returnLines = false, +}: ColorizeCodeOptions): React.ReactNode | React.ReactNode[] { const codeToHighlight = code.replace(/\n$/, ''); const activeTheme = theme || themeManager.getActiveTheme(); const showLineNumbers = hideLineNumbers ? false : settings.merged.ui.showLineNumbers; + const useMaxSizedBox = !settings.merged.ui.useAlternateBuffer && !returnLines; try { // Render the HAST tree using the adapted theme // Apply the theme's default foreground color to the top-level Text element @@ -162,7 +177,7 @@ export function colorizeCode({ let hiddenLinesCount = 0; // Optimization to avoid highlighting lines that cannot possibly be displayed. - if (availableHeight !== undefined) { + if (availableHeight !== undefined && useMaxSizedBox) { availableHeight = Math.max(availableHeight, MINIMUM_MAX_HEIGHT); if (lines.length > availableHeight) { const sliceIndex = lines.length - availableHeight; @@ -172,11 +187,9 @@ export function colorizeCode({ } const renderedLines = lines.map((line, index) => { - const contentToRender = highlightAndRenderLine( - line, - language, - activeTheme, - ); + const contentToRender = disableColor + ? line + : highlightAndRenderLine(line, language, activeTheme); return ( @@ -188,19 +201,26 @@ export function colorizeCode({ alignItems="flex-start" justifyContent="flex-end" > - + {`${index + 1 + hiddenLinesCount}`} )} - + {contentToRender} ); }); - if (availableHeight !== undefined) { + if (returnLines) { + return renderedLines; + } + + if (useMaxSizedBox) { return ( - {`${index + 1}`} + + {`${index + 1}`} + )} - {stripAnsi(line)} + + {stripAnsi(line)} + )); - if (availableHeight !== undefined) { + if (returnLines) { + return fallbackLines; + } + + if (useMaxSizedBox) { return ( item.type === 'tool_group') - .flatMap((group) => group.tools); + const allPendingTools = getAllToolCalls(pendingHistoryItems); const confirmingTools = allPendingTools.filter( (tool) => tool.status === CoreToolCallStatus.AwaitingApproval, diff --git a/packages/cli/src/ui/utils/historyUtils.ts b/packages/cli/src/ui/utils/historyUtils.ts new file mode 100644 index 0000000000..ee607dca96 --- /dev/null +++ b/packages/cli/src/ui/utils/historyUtils.ts @@ -0,0 +1,83 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { CoreToolCallStatus } from '../types.js'; +import type { + HistoryItem, + HistoryItemWithoutId, + HistoryItemToolGroup, + IndividualToolCallDisplay, +} from '../types.js'; + +export function getLastTurnToolCallIds( + history: HistoryItem[], + pendingHistoryItems: HistoryItemWithoutId[], +): string[] { + const targetToolCallIds: string[] = []; + + // Find the boundary of the last user prompt + let lastUserPromptIndex = -1; + for (let i = history.length - 1; i >= 0; i--) { + const type = history[i].type; + if (type === 'user' || type === 'user_shell') { + lastUserPromptIndex = i; + break; + } + } + + // Collect IDs from history after last user prompt + history.forEach((item, index) => { + if (index > lastUserPromptIndex && item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + + // Collect IDs from pending items + pendingHistoryItems.forEach((item) => { + if (item.type === 'tool_group') { + item.tools.forEach((t) => { + if (t.callId) targetToolCallIds.push(t.callId); + }); + } + }); + + return targetToolCallIds; +} + +export function isToolExecuting( + pendingHistoryItems: HistoryItemWithoutId[], +): boolean { + return pendingHistoryItems.some((item) => { + if (item && item.type === 'tool_group') { + return item.tools.some( + (tool) => CoreToolCallStatus.Executing === tool.status, + ); + } + return false; + }); +} + +export function isToolAwaitingConfirmation( + pendingHistoryItems: HistoryItemWithoutId[], +): boolean { + return pendingHistoryItems + .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') + .some((item) => + item.tools.some( + (tool) => CoreToolCallStatus.AwaitingApproval === tool.status, + ), + ); +} + +export function getAllToolCalls( + historyItems: HistoryItemWithoutId[], +): IndividualToolCallDisplay[] { + return historyItems + .filter((item): item is HistoryItemToolGroup => item.type === 'tool_group') + .flatMap((group) => group.tools); +} diff --git a/packages/cli/src/ui/utils/toolLayoutUtils.test.ts b/packages/cli/src/ui/utils/toolLayoutUtils.test.ts index 57e1e3f190..768fccc111 100644 --- a/packages/cli/src/ui/utils/toolLayoutUtils.test.ts +++ b/packages/cli/src/ui/utils/toolLayoutUtils.test.ts @@ -9,6 +9,10 @@ import { calculateToolContentMaxLines, calculateShellMaxLines, SHELL_CONTENT_OVERHEAD, + TOOL_RESULT_STATIC_HEIGHT, + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, + TOOL_RESULT_ASB_RESERVED_LINE_COUNT, + TOOL_RESULT_MIN_LINES_SHOWN, } from './toolLayoutUtils.js'; import { CoreToolCallStatus } from '@google/gemini-cli-core'; import { @@ -48,7 +52,7 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 2, isAlternateBuffer: false, }, - expected: 3, + expected: TOOL_RESULT_MIN_LINES_SHOWN + 1, }, { desc: 'returns available space directly in constrained terminal (ASB mode)', @@ -56,7 +60,7 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 4, isAlternateBuffer: true, }, - expected: 3, + expected: TOOL_RESULT_MIN_LINES_SHOWN + 1, }, { desc: 'returns remaining space if sufficient space exists (Standard mode)', @@ -64,7 +68,10 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 20, isAlternateBuffer: false, }, - expected: 17, + expected: + 20 - + TOOL_RESULT_STATIC_HEIGHT - + TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'returns remaining space if sufficient space exists (ASB mode)', @@ -72,7 +79,8 @@ describe('toolLayoutUtils', () => { availableTerminalHeight: 20, isAlternateBuffer: true, }, - expected: 13, + expected: + 20 - TOOL_RESULT_STATIC_HEIGHT - TOOL_RESULT_ASB_RESERVED_LINE_COUNT, }, ]; @@ -148,7 +156,7 @@ describe('toolLayoutUtils', () => { constrainHeight: true, isExpandable: false, }, - expected: 4, + expected: 6 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'handles negative availableTerminalHeight gracefully', @@ -172,7 +180,7 @@ describe('toolLayoutUtils', () => { constrainHeight: false, isExpandable: false, }, - expected: 28, + expected: 30 - TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT, }, { desc: 'falls back to COMPLETED_SHELL_MAX_LINES - SHELL_CONTENT_OVERHEAD for completed shells if space allows', diff --git a/packages/cli/src/ui/utils/toolLayoutUtils.ts b/packages/cli/src/ui/utils/toolLayoutUtils.ts index 9f391dca4e..1f140b9bc9 100644 --- a/packages/cli/src/ui/utils/toolLayoutUtils.ts +++ b/packages/cli/src/ui/utils/toolLayoutUtils.ts @@ -17,7 +17,7 @@ import { CoreToolCallStatus } from '@google/gemini-cli-core'; */ export const TOOL_RESULT_STATIC_HEIGHT = 1; export const TOOL_RESULT_ASB_RESERVED_LINE_COUNT = 6; -export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 2; +export const TOOL_RESULT_STANDARD_RESERVED_LINE_COUNT = 3; export const TOOL_RESULT_MIN_LINES_SHOWN = 2; /** diff --git a/packages/core/src/confirmation-bus/types.ts b/packages/core/src/confirmation-bus/types.ts index 91aeab8308..3c96fdaa19 100644 --- a/packages/core/src/confirmation-bus/types.ts +++ b/packages/core/src/confirmation-bus/types.ts @@ -8,6 +8,7 @@ import { type FunctionCall } from '@google/genai'; import type { ToolConfirmationOutcome, ToolConfirmationPayload, + DiffStat, } from '../tools/tools.js'; import type { ToolCall } from '../scheduler/types.js'; @@ -88,6 +89,7 @@ export type SerializableConfirmationDetails = originalContent: string | null; newContent: string; isModifying?: boolean; + diffStat?: DiffStat; } | { type: 'exec'; diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 1ecae4ef33..4b4e6e0c54 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -23,6 +23,7 @@ import { getToolSuggestion, isToolCallResponseInfo, } from '../utils/tool-utils.js'; +import { getDiffStatFromPatch } from '../tools/diffOptions.js'; import type { ToolConfirmationRequest } from '../confirmation-bus/types.js'; import { MessageBusType } from '../confirmation-bus/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -312,6 +313,12 @@ export class CoreToolScheduler { waitingCall.confirmationDetails.originalContent, newContent: waitingCall.confirmationDetails.newContent, filePath: waitingCall.confirmationDetails.filePath, + // Derive stats from the patch if they aren't already present + diffStat: + waitingCall.confirmationDetails.diffStat ?? + getDiffStatFromPatch( + waitingCall.confirmationDetails.fileDiff, + ), }; } } diff --git a/packages/core/src/scheduler/state-manager.test.ts b/packages/core/src/scheduler/state-manager.test.ts index dd5071c5bf..ff69e0d207 100644 --- a/packages/core/src/scheduler/state-manager.test.ts +++ b/packages/core/src/scheduler/state-manager.test.ts @@ -22,6 +22,7 @@ import { ToolConfirmationOutcome, type AnyDeclarativeTool, type AnyToolInvocation, + type FileDiff, } from '../tools/tools.js'; import { MessageBusType } from '../confirmation-bus/types.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -359,7 +360,7 @@ describe('SchedulerStateManager', () => { expect(active.confirmationDetails).toEqual(details); }); - it('should preserve diff when cancelling an edit tool call', () => { + it('should preserve diff and derive stats when cancelling an edit tool call', () => { const call = createValidatingCall(); stateManager.enqueue([call]); stateManager.dequeue(); @@ -369,9 +370,9 @@ describe('SchedulerStateManager', () => { title: 'Edit', fileName: 'test.txt', filePath: '/path/to/test.txt', - fileDiff: 'diff', - originalContent: 'old', - newContent: 'new', + fileDiff: '@@ -1,1 +1,1 @@\n-old line\n+new line', + originalContent: 'old line', + newContent: 'new line', onConfirm: vi.fn(), }; @@ -389,13 +390,14 @@ describe('SchedulerStateManager', () => { const completed = stateManager.completedBatch[0] as CancelledToolCall; expect(completed.status).toBe(CoreToolCallStatus.Cancelled); - expect(completed.response.resultDisplay).toEqual({ - fileDiff: 'diff', - fileName: 'test.txt', - filePath: '/path/to/test.txt', - originalContent: 'old', - newContent: 'new', - }); + const result = completed.response.resultDisplay as FileDiff; + expect(result.fileDiff).toBe(details.fileDiff); + expect(result.diffStat).toEqual( + expect.objectContaining({ + model_added_lines: 1, + model_removed_lines: 1, + }), + ); }); it('should ignore status updates for non-existent callIds', () => { diff --git a/packages/core/src/scheduler/state-manager.ts b/packages/core/src/scheduler/state-manager.ts index 428b7f87a8..093aaa7308 100644 --- a/packages/core/src/scheduler/state-manager.ts +++ b/packages/core/src/scheduler/state-manager.ts @@ -32,6 +32,7 @@ import { type SerializableConfirmationDetails, } from '../confirmation-bus/types.js'; import { isToolCallResponseInfo } from '../utils/tool-utils.js'; +import { getDiffStatFromPatch } from '../tools/diffOptions.js'; /** * Handler for terminal tool calls. @@ -473,6 +474,8 @@ export class SchedulerStateManager { filePath: details.filePath, originalContent: details.originalContent, newContent: details.newContent, + // Derive stats from the patch if they aren't already present + diffStat: details.diffStat ?? getDiffStatFromPatch(details.fileDiff), }; } } diff --git a/packages/core/src/tools/diffOptions.ts b/packages/core/src/tools/diffOptions.ts index b026b14f7c..0a0e0fa49e 100644 --- a/packages/core/src/tools/diffOptions.ts +++ b/packages/core/src/tools/diffOptions.ts @@ -76,3 +76,39 @@ export function getDiffStat( user_removed_chars: userStats.removedChars, }; } + +/** + * Extracts line and character stats from a unified diff patch string. + * This is useful for reconstructing stats for rejected or errored operations + * where the full strings may no longer be easily accessible. + */ +export function getDiffStatFromPatch(patch: string): DiffStat { + let addedLines = 0; + let removedLines = 0; + let addedChars = 0; + let removedChars = 0; + + const lines = patch.split('\n'); + for (const line of lines) { + // Only count lines that are additions or removals, + // excluding the diff headers (--- and +++) and metadata (\) + if (line.startsWith('+') && !line.startsWith('+++')) { + addedLines++; + addedChars += line.length - 1; + } else if (line.startsWith('-') && !line.startsWith('---')) { + removedLines++; + removedChars += line.length - 1; + } + } + + return { + model_added_lines: addedLines, + model_removed_lines: removedLines, + model_added_chars: addedChars, + model_removed_chars: removedChars, + user_added_lines: 0, + user_removed_lines: 0, + user_added_chars: 0, + user_removed_chars: 0, + }; +} diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index bfa70565be..22d2bb3578 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -896,11 +896,33 @@ class EditToolInvocation DEFAULT_DIFF_OPTIONS, ); + // Determine the full content as originally proposed by the AI to ensure accurate diff stats. + let fullAiProposedContent = editData.newContent; + if ( + this.params.modified_by_user && + this.params.ai_proposed_content !== undefined + ) { + try { + const aiReplacement = await calculateReplacement(this.config, { + params: { + ...this.params, + new_string: this.params.ai_proposed_content, + }, + currentContent: editData.currentContent ?? '', + abortSignal: signal, + }); + fullAiProposedContent = aiReplacement.newContent; + } catch (_e) { + // Fallback to newContent if speculative calculation fails + fullAiProposedContent = editData.newContent; + } + } + const diffStat = getDiffStat( fileName, editData.currentContent ?? '', + fullAiProposedContent, editData.newContent, - this.params.new_string, ); displayResult = { fileDiff, diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index e818881662..3b55cef0a1 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -150,8 +150,6 @@ export { SKILL_PARAM_NAME, }; -export const LS_TOOL_NAME_LEGACY = 'list_directory'; // Just to be safe if anything used the old exported name directly - export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); /** @@ -183,6 +181,11 @@ export const EDIT_DISPLAY_NAME = 'Edit'; export const ASK_USER_DISPLAY_NAME = 'Ask User'; export const READ_FILE_DISPLAY_NAME = 'ReadFile'; export const GLOB_DISPLAY_NAME = 'FindFiles'; +export const LS_DISPLAY_NAME = 'ReadFolder'; +export const GREP_DISPLAY_NAME = 'SearchText'; +export const WEB_SEARCH_DISPLAY_NAME = 'GoogleSearch'; +export const WEB_FETCH_DISPLAY_NAME = 'WebFetch'; +export const READ_MANY_FILES_DISPLAY_NAME = 'ReadManyFiles'; /** * Mapping of legacy tool names to their current names. diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 3865aaf357..5e2d12ace2 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -816,6 +816,21 @@ export interface TodoList { export type ToolLiveOutput = string | AnsiOutput | SubagentProgress; +export interface StructuredToolResult { + summary: string; +} + +export function isStructuredToolResult( + obj: unknown, +): obj is StructuredToolResult { + return ( + typeof obj === 'object' && + obj !== null && + 'summary' in obj && + typeof obj.summary === 'string' + ); +} + export type ToolResultDisplay = | string | FileDiff @@ -869,6 +884,7 @@ export interface ToolEditConfirmationDetails { originalContent: string | null; newContent: string; isModifying?: boolean; + diffStat?: DiffStat; ideConfirmation?: Promise; } diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 27a60c4259..4a7bf6554e 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -30,7 +30,7 @@ import { NetworkRetryAttemptEvent, } from '../telemetry/index.js'; import { LlmRole } from '../telemetry/llmRole.js'; -import { WEB_FETCH_TOOL_NAME } from './tool-names.js'; +import { WEB_FETCH_TOOL_NAME, WEB_FETCH_DISPLAY_NAME } from './tool-names.js'; import { debugLogger } from '../utils/debugLogger.js'; import { coreEvents } from '../utils/events.js'; import { retryWithBackoff, getRetryErrorType } from '../utils/retry.js'; @@ -891,7 +891,7 @@ export class WebFetchTool extends BaseDeclarativeTool< ) { super( WebFetchTool.Name, - 'WebFetch', + WEB_FETCH_DISPLAY_NAME, WEB_FETCH_DEFINITION.base.description!, Kind.Fetch, WEB_FETCH_DEFINITION.base.parametersJsonSchema, diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index 18132d2c35..2a29291437 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -5,7 +5,7 @@ */ import type { MessageBus } from '../confirmation-bus/message-bus.js'; -import { WEB_SEARCH_TOOL_NAME } from './tool-names.js'; +import { WEB_SEARCH_TOOL_NAME, WEB_SEARCH_DISPLAY_NAME } from './tool-names.js'; import type { GroundingMetadata } from '@google/genai'; import { BaseDeclarativeTool, @@ -212,7 +212,7 @@ export class WebSearchTool extends BaseDeclarativeTool< ) { super( WebSearchTool.Name, - 'GoogleSearch', + WEB_SEARCH_DISPLAY_NAME, WEB_SEARCH_DEFINITION.base.description!, Kind.Search, WEB_SEARCH_DEFINITION.base.parametersJsonSchema,