diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index fa22f59267..4872e1b3d1 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -93,6 +93,7 @@ import { setupTerminalAndTheme } from './utils/terminalTheme.js'; import { runDeferredCommand } from './deferred.js'; import { cleanupBackgroundLogs } from './utils/logCleanup.js'; import { SlashCommandConflictHandler } from './services/SlashCommandConflictHandler.js'; +import { initializeConsoleStore } from './ui/hooks/useConsoleMessages.js'; export function validateDnsResolutionOrder( order: string | undefined, @@ -294,6 +295,7 @@ export async function main() { process.exit(ExitCodes.FATAL_INPUT_ERROR); } + initializeConsoleStore(); const isDebugMode = cliConfig.isDebugMode(argv); const consolePatcher = new ConsolePatcher({ stderr: true, diff --git a/packages/cli/src/ui/components/DetailedMessagesDisplay.test.tsx b/packages/cli/src/ui/components/DetailedMessagesDisplay.test.tsx index 30f98a6eda..6cb61ea95c 100644 --- a/packages/cli/src/ui/components/DetailedMessagesDisplay.test.tsx +++ b/packages/cli/src/ui/components/DetailedMessagesDisplay.test.tsx @@ -35,10 +35,7 @@ vi.mock('./shared/ScrollableList.js', () => ({ describe('DetailedMessagesDisplay', () => { beforeEach(() => { - vi.mocked(useConsoleMessages).mockReturnValue({ - consoleMessages: [], - clearConsoleMessages: vi.fn(), - }); + vi.mocked(useConsoleMessages).mockReturnValue([]); }); it('renders nothing when messages are empty', async () => { const { lastFrame, unmount } = await renderWithProviders( @@ -58,10 +55,7 @@ describe('DetailedMessagesDisplay', () => { { type: 'error', content: 'Error message', count: 1 }, { type: 'debug', content: 'Debug message', count: 1 }, ]; - vi.mocked(useConsoleMessages).mockReturnValue({ - consoleMessages: messages, - clearConsoleMessages: vi.fn(), - }); + vi.mocked(useConsoleMessages).mockReturnValue(messages); const { lastFrame, unmount } = await renderWithProviders( , @@ -79,10 +73,7 @@ describe('DetailedMessagesDisplay', () => { const messages: ConsoleMessageItem[] = [ { type: 'error', content: 'Error message', count: 1 }, ]; - vi.mocked(useConsoleMessages).mockReturnValue({ - consoleMessages: messages, - clearConsoleMessages: vi.fn(), - }); + vi.mocked(useConsoleMessages).mockReturnValue(messages); const { lastFrame, unmount } = await renderWithProviders( , @@ -98,10 +89,7 @@ describe('DetailedMessagesDisplay', () => { const messages: ConsoleMessageItem[] = [ { type: 'error', content: 'Error message', count: 1 }, ]; - vi.mocked(useConsoleMessages).mockReturnValue({ - consoleMessages: messages, - clearConsoleMessages: vi.fn(), - }); + vi.mocked(useConsoleMessages).mockReturnValue(messages); const { lastFrame, unmount } = await renderWithProviders( , @@ -117,10 +105,7 @@ describe('DetailedMessagesDisplay', () => { const messages: ConsoleMessageItem[] = [ { type: 'log', content: 'Repeated message', count: 5 }, ]; - vi.mocked(useConsoleMessages).mockReturnValue({ - consoleMessages: messages, - clearConsoleMessages: vi.fn(), - }); + vi.mocked(useConsoleMessages).mockReturnValue(messages); const { lastFrame, unmount } = await renderWithProviders( , diff --git a/packages/cli/src/ui/components/DetailedMessagesDisplay.tsx b/packages/cli/src/ui/components/DetailedMessagesDisplay.tsx index 2daa1c39e3..97e456eb99 100644 --- a/packages/cli/src/ui/components/DetailedMessagesDisplay.tsx +++ b/packages/cli/src/ui/components/DetailedMessagesDisplay.tsx @@ -29,7 +29,7 @@ export const DetailedMessagesDisplay: React.FC< > = ({ maxHeight, width, hasFocus }) => { const scrollableListRef = useRef>(null); - const { consoleMessages } = useConsoleMessages(); + const consoleMessages = useConsoleMessages(); const config = useConfig(); const messages = useMemo(() => { diff --git a/packages/cli/src/ui/hooks/useConsoleMessages.test.tsx b/packages/cli/src/ui/hooks/useConsoleMessages.test.tsx index af78f73447..c062c4bc50 100644 --- a/packages/cli/src/ui/hooks/useConsoleMessages.test.tsx +++ b/packages/cli/src/ui/hooks/useConsoleMessages.test.tsx @@ -7,76 +7,93 @@ import { act, useCallback } from 'react'; import { vi } from 'vitest'; import { render } from '../../test-utils/render.js'; -import { useConsoleMessages } from './useConsoleMessages.js'; -import { CoreEvent, type ConsoleLogPayload } from '@google/gemini-cli-core'; - -// Mock coreEvents -let consoleLogHandler: ((payload: ConsoleLogPayload) => void) | undefined; +import { + useConsoleMessages, + useErrorCount, + initializeConsoleStore, +} from './useConsoleMessages.js'; +import { coreEvents } from '@google/gemini-cli-core'; vi.mock('@google/gemini-cli-core', async (importOriginal) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const actual = (await importOriginal()) as any; + const actual = await importOriginal(); + const handlers = new Map void>(); + return { - ...actual, + ...(actual as Record), coreEvents: { - on: vi.fn((event, handler) => { - if (event === CoreEvent.ConsoleLog) { - consoleLogHandler = handler; - } + ...((actual as Record)['coreEvents'] as Record< + string, + unknown + >), + on: vi.fn((event: string, handler: (payload: unknown) => void) => { + handlers.set(event, handler); }), - off: vi.fn((event) => { - if (event === CoreEvent.ConsoleLog) { - consoleLogHandler = undefined; - } + off: vi.fn((event: string) => { + handlers.delete(event); }), - emitConsoleLog: vi.fn(), + // Helper for testing to trigger the handlers + _trigger: (event: string, payload: unknown) => { + handlers.get(event)?.(payload); + }, }, }; }); describe('useConsoleMessages', () => { + let unmounts: Array<() => void> = []; + beforeEach(() => { vi.useFakeTimers(); - consoleLogHandler = undefined; + initializeConsoleStore(); }); afterEach(() => { + for (const unmount of unmounts) { + try { + unmount(); + } catch (_e) { + // Ignore unmount errors + } + } + unmounts = []; vi.runOnlyPendingTimers(); vi.useRealTimers(); vi.restoreAllMocks(); }); const useTestableConsoleMessages = () => { - const { ...rest } = useConsoleMessages(); + const consoleMessages = useConsoleMessages(); const log = useCallback((content: string) => { - if (consoleLogHandler) { - consoleLogHandler({ type: 'log', content }); - } + // @ts-expect-error - internal testing helper + coreEvents._trigger('console-log', { type: 'log', content }); }, []); const error = useCallback((content: string) => { - if (consoleLogHandler) { - consoleLogHandler({ type: 'error', content }); - } + // @ts-expect-error - internal testing helper + coreEvents._trigger('console-log', { type: 'error', content }); + }, []); + const clearConsoleMessages = useCallback(() => { + initializeConsoleStore(); }, []); return { - ...rest, + consoleMessages, log, error, - clearConsoleMessages: rest.clearConsoleMessages, + clearConsoleMessages, }; }; const renderConsoleMessagesHook = async () => { - let hookResult: ReturnType; + let hookResult: ReturnType | undefined; function TestComponent() { hookResult = useTestableConsoleMessages(); return null; } const { unmount } = await render(); + unmounts.push(unmount); return { result: { get current() { - return hookResult; + return hookResult!; }, }, unmount, @@ -93,10 +110,7 @@ describe('useConsoleMessages', () => { act(() => { result.current.log('Test message'); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(60); + vi.runAllTimers(); }); expect(result.current.consoleMessages).toEqual([ @@ -111,10 +125,7 @@ describe('useConsoleMessages', () => { result.current.log('Test message'); result.current.log('Test message'); result.current.log('Test message'); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(60); + vi.runAllTimers(); }); expect(result.current.consoleMessages).toEqual([ @@ -128,10 +139,7 @@ describe('useConsoleMessages', () => { act(() => { result.current.log('First message'); result.current.error('Second message'); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(60); + vi.runAllTimers(); }); expect(result.current.consoleMessages).toEqual([ @@ -139,53 +147,85 @@ describe('useConsoleMessages', () => { { type: 'error', content: 'Second message', count: 1 }, ]); }); +}); - it('should clear all messages when clearConsoleMessages is called', async () => { - const { result } = await renderConsoleMessagesHook(); +describe('useErrorCount', () => { + let unmounts: Array<() => void> = []; - act(() => { - result.current.log('A message'); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(60); - }); - - expect(result.current.consoleMessages).toHaveLength(1); - - act(() => { - result.current.clearConsoleMessages(); - }); - - expect(result.current.consoleMessages).toHaveLength(0); + beforeEach(() => { + vi.useFakeTimers(); + initializeConsoleStore(); }); - it('should clear the pending timeout when clearConsoleMessages is called', async () => { - const { result } = await renderConsoleMessagesHook(); - const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout'); - - act(() => { - result.current.log('A message'); - }); - - act(() => { - result.current.clearConsoleMessages(); - }); - - expect(clearTimeoutSpy).toHaveBeenCalled(); - // clearTimeoutSpy.mockRestore() is handled by afterEach restoreAllMocks + afterEach(() => { + for (const unmount of unmounts) { + try { + unmount(); + } catch (_e) { + // Ignore unmount errors + } + } + unmounts = []; + vi.runOnlyPendingTimers(); + vi.useRealTimers(); + vi.restoreAllMocks(); }); - it('should clean up the timeout on unmount', async () => { - const { result, unmount } = await renderConsoleMessagesHook(); - const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout'); + const renderErrorCountHook = async () => { + let hookResult: ReturnType; + function TestComponent() { + hookResult = useErrorCount(); + return null; + } + const { unmount } = await render(); + unmounts.push(unmount); + return { + result: { + get current() { + return hookResult; + }, + }, + unmount, + }; + }; + + it('should initialize with an error count of 0', async () => { + const { result } = await renderErrorCountHook(); + expect(result.current.errorCount).toBe(0); + }); + + it('should increment error count when an error is logged', async () => { + const { result } = await renderErrorCountHook(); + act(() => { + // @ts-expect-error - internal testing helper + coreEvents._trigger('console-log', { type: 'error', content: 'error' }); + vi.runAllTimers(); + }); + expect(result.current.errorCount).toBe(1); + }); + + it('should not increment error count for non-error logs', async () => { + const { result } = await renderErrorCountHook(); + act(() => { + // @ts-expect-error - internal testing helper + coreEvents._trigger('console-log', { type: 'log', content: 'log' }); + vi.runAllTimers(); + }); + expect(result.current.errorCount).toBe(0); + }); + + it('should clear the error count', async () => { + const { result } = await renderErrorCountHook(); + act(() => { + // @ts-expect-error - internal testing helper + coreEvents._trigger('console-log', { type: 'error', content: 'error' }); + vi.runAllTimers(); + }); + expect(result.current.errorCount).toBe(1); act(() => { - result.current.log('A message'); + result.current.clearErrorCount(); }); - - unmount(); - - expect(clearTimeoutSpy).toHaveBeenCalled(); + expect(result.current.errorCount).toBe(0); }); }); diff --git a/packages/cli/src/ui/hooks/useConsoleMessages.ts b/packages/cli/src/ui/hooks/useConsoleMessages.ts index da000a9da1..7cfa0a6ce3 100644 --- a/packages/cli/src/ui/hooks/useConsoleMessages.ts +++ b/packages/cli/src/ui/hooks/useConsoleMessages.ts @@ -4,13 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { - useCallback, - useEffect, - useReducer, - useRef, - startTransition, -} from 'react'; +import { useCallback, useSyncExternalStore } from 'react'; import type { ConsoleMessageItem } from '../types.js'; import { coreEvents, @@ -18,207 +12,170 @@ import { type ConsoleLogPayload, } from '@google/gemini-cli-core'; -export interface UseConsoleMessagesReturn { - consoleMessages: ConsoleMessageItem[]; - clearConsoleMessages: () => void; -} - -type Action = - | { type: 'ADD_MESSAGES'; payload: ConsoleMessageItem[] } - | { type: 'CLEAR' }; - -function consoleMessagesReducer( - state: ConsoleMessageItem[], - action: Action, -): ConsoleMessageItem[] { - const MAX_CONSOLE_MESSAGES = 1000; - switch (action.type) { - case 'ADD_MESSAGES': { - const newMessages = [...state]; - for (const queuedMessage of action.payload) { - const lastMessage = newMessages[newMessages.length - 1]; - if ( - lastMessage && - lastMessage.type === queuedMessage.type && - lastMessage.content === queuedMessage.content - ) { - // Create a new object for the last message to ensure React detects - // the change, preventing mutation of the existing state object. - newMessages[newMessages.length - 1] = { - ...lastMessage, - count: lastMessage.count + 1, - }; - } else { - newMessages.push({ ...queuedMessage, count: 1 }); - } - } - - // Limit the number of messages to prevent memory issues - if (newMessages.length > MAX_CONSOLE_MESSAGES) { - return newMessages.slice(newMessages.length - MAX_CONSOLE_MESSAGES); - } - - return newMessages; - } - case 'CLEAR': - return []; - default: - return state; - } -} - -export function useConsoleMessages(): UseConsoleMessagesReturn { - const [consoleMessages, dispatch] = useReducer(consoleMessagesReducer, []); - const messageQueueRef = useRef([]); - const timeoutRef = useRef(null); - const isProcessingRef = useRef(false); - - const processQueue = useCallback(() => { - if (messageQueueRef.current.length > 0) { - isProcessingRef.current = true; - const messagesToProcess = messageQueueRef.current; - messageQueueRef.current = []; - startTransition(() => { - dispatch({ type: 'ADD_MESSAGES', payload: messagesToProcess }); - }); - } - timeoutRef.current = null; - }, []); - - const handleNewMessage = useCallback( - (message: ConsoleMessageItem) => { - messageQueueRef.current.push(message); - if (!isProcessingRef.current && !timeoutRef.current) { - // Batch updates using a timeout. 50ms is a reasonable delay to batch - // rapid-fire messages without noticeable lag while avoiding React update - // queue flooding. - timeoutRef.current = setTimeout(processQueue, 50); - } - }, - [processQueue], - ); - - // Once the updated consoleMessages have been committed to the screen, - // we can safely process the next batch of queued messages if any exist. - // This completely eliminates overlapping concurrent updates to this state. - useEffect(() => { - isProcessingRef.current = false; - if (messageQueueRef.current.length > 0 && !timeoutRef.current) { - timeoutRef.current = setTimeout(processQueue, 50); - } - }, [consoleMessages, processQueue]); - - useEffect(() => { - const handleConsoleLog = (payload: ConsoleLogPayload) => { - let content = payload.content; - const MAX_CONSOLE_MSG_LENGTH = 10000; - if (content.length > MAX_CONSOLE_MSG_LENGTH) { - content = - content.slice(0, MAX_CONSOLE_MSG_LENGTH) + - `... [Truncated ${content.length - MAX_CONSOLE_MSG_LENGTH} characters]`; - } - - handleNewMessage({ - type: payload.type, - content, - count: 1, - }); - }; - - const handleOutput = (payload: { - isStderr: boolean; - chunk: Uint8Array | string; - }) => { - let content = - typeof payload.chunk === 'string' - ? payload.chunk - : new TextDecoder().decode(payload.chunk); - - const MAX_OUTPUT_CHUNK_LENGTH = 10000; - if (content.length > MAX_OUTPUT_CHUNK_LENGTH) { - content = - content.slice(0, MAX_OUTPUT_CHUNK_LENGTH) + - `... [Truncated ${content.length - MAX_OUTPUT_CHUNK_LENGTH} characters]`; - } - - // It would be nice if we could show stderr as 'warn' but unfortunately - // we log non warning info to stderr before the app starts so that would - // be misleading. - handleNewMessage({ type: 'log', content, count: 1 }); - }; - - coreEvents.on(CoreEvent.ConsoleLog, handleConsoleLog); - coreEvents.on(CoreEvent.Output, handleOutput); - return () => { - coreEvents.off(CoreEvent.ConsoleLog, handleConsoleLog); - coreEvents.off(CoreEvent.Output, handleOutput); - }; - }, [handleNewMessage]); - - const clearConsoleMessages = useCallback(() => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - timeoutRef.current = null; - } - messageQueueRef.current = []; - isProcessingRef.current = true; - startTransition(() => { - dispatch({ type: 'CLEAR' }); - }); - }, []); - - // Cleanup on unmount - useEffect( - () => () => { - if (timeoutRef.current) { - clearTimeout(timeoutRef.current); - } - }, - [], - ); - - return { consoleMessages, clearConsoleMessages }; -} - export interface UseErrorCountReturn { errorCount: number; clearErrorCount: () => void; } +// --- Global Console Store --- + +const MAX_CONSOLE_MESSAGES = 1000; +let globalConsoleMessages: ConsoleMessageItem[] = []; +let globalErrorCount = 0; +const listeners = new Set<() => void>(); + +let messageQueue: ConsoleMessageItem[] = []; +let timeoutId: NodeJS.Timeout | null = null; + +/** + * Initializes the global console store and subscribes to coreEvents. + * Acts as a safe reset function, making it idempotent and useful for test isolation. + * Must be called during application startup. + */ +export function initializeConsoleStore() { + if (timeoutId) { + clearTimeout(timeoutId); + timeoutId = null; + } + messageQueue = []; + globalConsoleMessages = []; + globalErrorCount = 0; + notifyListeners(); + + // Safely detach first to ensure idempotency and prevent listener leaks + coreEvents.off(CoreEvent.ConsoleLog, handleConsoleLog); + coreEvents.off(CoreEvent.Output, handleOutput); + + coreEvents.on(CoreEvent.ConsoleLog, handleConsoleLog); + coreEvents.on(CoreEvent.Output, handleOutput); +} + +function notifyListeners() { + for (const listener of listeners) { + listener(); + } +} + +function processQueue() { + if (messageQueue.length === 0) return; + + // Create a new array to trigger React updates + const newMessages = [...globalConsoleMessages]; + + for (const queuedMessage of messageQueue) { + if (queuedMessage.type === 'error') { + globalErrorCount++; + } + + // Coalesce consecutive identical messages + const prev = newMessages[newMessages.length - 1]; + if ( + prev && + prev.type === queuedMessage.type && + prev.content === queuedMessage.content + ) { + newMessages[newMessages.length - 1] = { + ...prev, + count: prev.count + 1, + }; + } else { + newMessages.push({ ...queuedMessage, count: 1 }); + } + } + + globalConsoleMessages = + newMessages.length > MAX_CONSOLE_MESSAGES + ? newMessages.slice(-MAX_CONSOLE_MESSAGES) + : newMessages; + + messageQueue = []; + timeoutId = null; + notifyListeners(); +} + +function handleNewMessage(message: ConsoleMessageItem) { + messageQueue.push(message); + if (!timeoutId) { + // Batch updates using a timeout. 50ms is a reasonable delay to batch + // rapid-fire messages without noticeable lag while avoiding React update + // queue flooding. + timeoutId = setTimeout(processQueue, 50); + } +} + +// --- Subscription API for useSyncExternalStore --- + +function subscribe(listener: () => void) { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +function getConsoleMessagesSnapshot() { + return globalConsoleMessages; +} + +function getErrorCountSnapshot() { + return globalErrorCount; +} + +// --- Core Event Listeners (Always active at module level) --- + +const handleConsoleLog = (payload: ConsoleLogPayload) => { + let content = payload.content; + const MAX_CONSOLE_MSG_LENGTH = 10000; + if (content.length > MAX_CONSOLE_MSG_LENGTH) { + content = + content.slice(0, MAX_CONSOLE_MSG_LENGTH) + + `... [Truncated ${content.length - MAX_CONSOLE_MSG_LENGTH} characters]`; + } + + handleNewMessage({ + type: payload.type, + content, + count: 1, + }); +}; + +const handleOutput = (payload: { + isStderr: boolean; + chunk: Uint8Array | string; +}) => { + let content = + typeof payload.chunk === 'string' + ? payload.chunk + : new TextDecoder().decode(payload.chunk); + + const MAX_OUTPUT_CHUNK_LENGTH = 10000; + if (content.length > MAX_OUTPUT_CHUNK_LENGTH) { + content = + content.slice(0, MAX_OUTPUT_CHUNK_LENGTH) + + `... [Truncated ${content.length - MAX_OUTPUT_CHUNK_LENGTH} characters]`; + } + + handleNewMessage({ type: 'log', content, count: 1 }); +}; + +/** + * Hook to access the global console message history. + * Decoupled from any component lifecycle to ensure history is preserved even + * when the UI is unmounted. + */ +export function useConsoleMessages(): ConsoleMessageItem[] { + return useSyncExternalStore(subscribe, getConsoleMessagesSnapshot); +} + +/** + * Hook to access the global error count. + * Uses the same external store as useConsoleMessages for consistency. + */ export function useErrorCount(): UseErrorCountReturn { - const [errorCount, dispatch] = useReducer( - (state: number, action: 'INCREMENT' | 'CLEAR') => { - switch (action) { - case 'INCREMENT': - return state + 1; - case 'CLEAR': - return 0; - default: - return state; - } - }, - 0, - ); - - useEffect(() => { - const handleConsoleLog = (payload: ConsoleLogPayload) => { - if (payload.type === 'error') { - startTransition(() => { - dispatch('INCREMENT'); - }); - } - }; - - coreEvents.on(CoreEvent.ConsoleLog, handleConsoleLog); - return () => { - coreEvents.off(CoreEvent.ConsoleLog, handleConsoleLog); - }; - }, []); + const errorCount = useSyncExternalStore(subscribe, getErrorCountSnapshot); const clearErrorCount = useCallback(() => { - startTransition(() => { - dispatch('CLEAR'); - }); + globalErrorCount = 0; + notifyListeners(); }, []); return { errorCount, clearErrorCount };