diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index b5b512434e..b6fdd53325 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -84,7 +84,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { }; }); import ansiEscapes from 'ansi-escapes'; -import { type LoadedSettings, mergeSettings } from '../config/settings.js'; +import { mergeSettings, type LoadedSettings } from '../config/settings.js'; import type { InitializationResult } from '../core/initializer.js'; import { useQuotaAndFallback } from './hooks/useQuotaAndFallback.js'; import { UIStateContext, type UIState } from './contexts/UIStateContext.js'; @@ -92,6 +92,7 @@ import { UIActionsContext, type UIActions, } from './contexts/UIActionsContext.js'; +import { KeypressProvider } from './contexts/KeypressContext.js'; // Mock useStdout to capture terminal title writes vi.mock('ink', async (importOriginal) => { @@ -133,7 +134,6 @@ vi.mock('./hooks/useGeminiStream.js'); vi.mock('./hooks/vim.js'); vi.mock('./hooks/useFocus.js'); vi.mock('./hooks/useBracketedPaste.js'); -vi.mock('./hooks/useKeypress.js'); vi.mock('./hooks/useLoadingIndicator.js'); vi.mock('./hooks/useFolderTrust.js'); vi.mock('./hooks/useIdeTrustListener.js'); @@ -197,7 +197,7 @@ import { useTextBuffer } from './components/shared/text-buffer.js'; import { useLogger } from './hooks/useLogger.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useInputHistoryStore } from './hooks/useInputHistoryStore.js'; -import { useKeypress, type Key } from './hooks/useKeypress.js'; +import { useKeypress } from './hooks/useKeypress.js'; import { measureElement } from 'ink'; import { useTerminalSize } from './hooks/useTerminalSize.js'; import { @@ -232,13 +232,15 @@ describe('AppContainer State Management', () => { resumedSessionData?: ResumedSessionData; } = {}) => ( - + + + ); @@ -268,7 +270,6 @@ describe('AppContainer State Management', () => { const mockedUseTextBuffer = useTextBuffer as Mock; const mockedUseLogger = useLogger as Mock; const mockedUseLoadingIndicator = useLoadingIndicator as Mock; - const mockedUseKeypress = useKeypress as Mock; const mockedUseInputHistoryStore = useInputHistoryStore as Mock; const mockedUseHookDisplayState = useHookDisplayState as Mock; const mockedUseTerminalTheme = useTerminalTheme as Mock; @@ -1770,47 +1771,36 @@ describe('AppContainer State Management', () => { }); describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => { - let handleGlobalKeypress: (key: Key) => boolean; let mockHandleSlashCommand: Mock; let mockCancelOngoingRequest: Mock; let rerender: () => void; let unmount: () => void; + let stdin: ReturnType['stdin']; // Helper function to reduce boilerplate in tests const setupKeypressTest = async () => { const renderResult = renderAppContainer(); + stdin = renderResult.stdin; await act(async () => { vi.advanceTimersByTime(0); }); - rerender = () => renderResult.rerender(getAppContainer()); + rerender = () => { + renderResult.rerender(getAppContainer()); + }; unmount = renderResult.unmount; }; - const pressKey = (key: Partial, times = 1) => { + const pressKey = (sequence: string, times = 1) => { for (let i = 0; i < times; i++) { act(() => { - handleGlobalKeypress({ - name: 'c', - shift: false, - alt: false, - ctrl: false, - cmd: false, - ...key, - } as Key); + stdin.write(sequence); }); rerender(); } }; beforeEach(() => { - // Capture the keypress handler from the AppContainer - mockedUseKeypress.mockImplementation( - (callback: (key: Key) => boolean) => { - handleGlobalKeypress = callback; - }, - ); - // Mock slash command handler mockHandleSlashCommand = vi.fn(); mockedUseSlashCommandProcessor.mockReturnValue({ @@ -1855,7 +1845,7 @@ describe('AppContainer State Management', () => { }); await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(1); expect(mockHandleSlashCommand).not.toHaveBeenCalled(); @@ -1865,7 +1855,7 @@ describe('AppContainer State Management', () => { it('should quit on second press', async () => { await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }, 2); + pressKey('\x03', 2); // Ctrl+C expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(2); expect(mockHandleSlashCommand).toHaveBeenCalledWith( @@ -1880,7 +1870,7 @@ describe('AppContainer State Management', () => { it('should reset press count after a timeout', async () => { await setupKeypressTest(); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockHandleSlashCommand).not.toHaveBeenCalled(); // Advance timer past the reset threshold @@ -1888,7 +1878,7 @@ describe('AppContainer State Management', () => { vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1); }); - pressKey({ name: 'c', ctrl: true }); + pressKey('\x03'); // Ctrl+C expect(mockHandleSlashCommand).not.toHaveBeenCalled(); unmount(); }); @@ -1898,7 +1888,7 @@ describe('AppContainer State Management', () => { it('should quit on second press if buffer is empty', async () => { await setupKeypressTest(); - pressKey({ name: 'd', ctrl: true }, 2); + pressKey('\x04', 2); // Ctrl+D expect(mockHandleSlashCommand).toHaveBeenCalledWith( '/quit', @@ -1909,7 +1899,7 @@ describe('AppContainer State Management', () => { unmount(); }); - it('should NOT quit if buffer is not empty (bubbles from InputPrompt)', async () => { + it('should NOT quit if buffer is not empty', async () => { mockedUseTextBuffer.mockReturnValue({ text: 'some text', setText: vi.fn(), @@ -1919,30 +1909,12 @@ describe('AppContainer State Management', () => { }); await setupKeypressTest(); - // Capture return value - let result = true; - const originalPressKey = (key: Partial) => { - act(() => { - result = handleGlobalKeypress({ - name: 'd', - shift: false, - alt: false, - ctrl: true, - cmd: false, - ...key, - } as Key); - }); - rerender(); - }; + pressKey('\x04'); // Ctrl+D - originalPressKey({ name: 'd', ctrl: true }); - - // AppContainer's handler should return true if it reaches it - expect(result).toBe(true); - // But it should only be called once, so count is 1, not quitting yet. + // Should only be called once, so count is 1, not quitting yet. expect(mockHandleSlashCommand).not.toHaveBeenCalled(); - originalPressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D // Now count is 2, it should quit. expect(mockHandleSlashCommand).toHaveBeenCalledWith( '/quit', @@ -1956,7 +1928,7 @@ describe('AppContainer State Management', () => { it('should reset press count after a timeout', async () => { await setupKeypressTest(); - pressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D expect(mockHandleSlashCommand).not.toHaveBeenCalled(); // Advance timer past the reset threshold @@ -1964,7 +1936,7 @@ describe('AppContainer State Management', () => { vi.advanceTimersByTime(WARNING_PROMPT_DURATION_MS + 1); }); - pressKey({ name: 'd', ctrl: true }); + pressKey('\x04'); // Ctrl+D expect(mockHandleSlashCommand).not.toHaveBeenCalled(); unmount(); }); @@ -1982,7 +1954,7 @@ describe('AppContainer State Management', () => { it('should focus shell input on Tab', async () => { await setupKeypressTest(); - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); expect(capturedUIState.embeddedShellFocused).toBe(true); unmount(); @@ -1992,11 +1964,11 @@ describe('AppContainer State Management', () => { await setupKeypressTest(); // Focus first - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); expect(capturedUIState.embeddedShellFocused).toBe(true); // Unfocus via Shift+Tab - pressKey({ name: 'tab', shift: true }); + pressKey('\x1b[Z'); expect(capturedUIState.embeddedShellFocused).toBe(false); unmount(); }); @@ -2015,13 +1987,7 @@ describe('AppContainer State Management', () => { // Focus it act(() => { - handleGlobalKeypress({ - name: 'tab', - shift: false, - alt: false, - ctrl: false, - cmd: false, - } as Key); + renderResult.stdin.write('\t'); }); expect(capturedUIState.embeddedShellFocused).toBe(true); @@ -2056,7 +2022,7 @@ describe('AppContainer State Management', () => { expect(capturedUIState.embeddedShellFocused).toBe(false); // Press Tab - pressKey({ name: 'tab', shift: false }); + pressKey('\t'); // Should be focused expect(capturedUIState.embeddedShellFocused).toBe(true); @@ -2084,7 +2050,7 @@ describe('AppContainer State Management', () => { expect(capturedUIState.embeddedShellFocused).toBe(false); // Press Ctrl+B - pressKey({ name: 'b', ctrl: true }); + pressKey('\x02'); // Should have toggled (closed) the shell expect(mockToggleBackgroundShell).toHaveBeenCalled(); @@ -2113,7 +2079,7 @@ describe('AppContainer State Management', () => { }); // Press Ctrl+B - pressKey({ name: 'b', ctrl: true }); + pressKey('\x02'); // Should have toggled (shown) the shell expect(mockToggleBackgroundShell).toHaveBeenCalled(); @@ -2126,11 +2092,14 @@ describe('AppContainer State Management', () => { }); describe('Copy Mode (CTRL+S)', () => { - let handleGlobalKeypress: (key: Key) => boolean; let rerender: () => void; let unmount: () => void; + let stdin: ReturnType['stdin']; - const setupCopyModeTest = async (isAlternateMode = false) => { + const setupCopyModeTest = async ( + isAlternateMode = false, + childHandler?: Mock, + ) => { // Update settings for this test run const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); const testSettings = { @@ -2144,23 +2113,39 @@ describe('AppContainer State Management', () => { }, } as unknown as LoadedSettings; - const renderResult = renderAppContainer({ settings: testSettings }); + function TestChild() { + useKeypress(childHandler || (() => {}), { + isActive: !!childHandler, + priority: true, + }); + return null; + } + + const getTree = (settings: LoadedSettings) => ( + + + + + + + ); + + const renderResult = render(getTree(testSettings)); + stdin = renderResult.stdin; await act(async () => { vi.advanceTimersByTime(0); }); - rerender = () => - renderResult.rerender(getAppContainer({ settings: testSettings })); + rerender = () => renderResult.rerender(getTree(testSettings)); unmount = renderResult.unmount; }; beforeEach(() => { mocks.mockStdout.write.mockClear(); - mockedUseKeypress.mockImplementation( - (callback: (key: Key) => boolean) => { - handleGlobalKeypress = callback; - }, - ); vi.useFakeTimers(); }); @@ -2186,15 +2171,7 @@ describe('AppContainer State Management', () => { mocks.mockStdout.write.mockClear(); // Clear initial enable call act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); @@ -2213,30 +2190,14 @@ describe('AppContainer State Management', () => { // Turn it on (disable mouse) act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); expect(disableMouseEvents).toHaveBeenCalled(); // Turn it off (enable mouse) act(() => { - handleGlobalKeypress({ - name: 'any', // Any key should exit copy mode - shift: false, - alt: false, - ctrl: false, - cmd: false, - insertable: true, - sequence: 'a', - }); + stdin.write('a'); // Any key should exit copy mode }); rerender(); @@ -2249,15 +2210,7 @@ describe('AppContainer State Management', () => { // Enter copy mode act(() => { - handleGlobalKeypress({ - name: 's', - shift: false, - alt: false, - ctrl: true, - cmd: false, - insertable: false, - sequence: '\x13', - }); + stdin.write('\x13'); // Ctrl+S }); rerender(); @@ -2265,15 +2218,7 @@ describe('AppContainer State Management', () => { // Press any other key act(() => { - handleGlobalKeypress({ - name: 'a', - shift: false, - alt: false, - ctrl: false, - cmd: false, - insertable: true, - sequence: 'a', - }); + stdin.write('a'); }); rerender(); @@ -2281,6 +2226,37 @@ describe('AppContainer State Management', () => { expect(enableMouseEvents).toHaveBeenCalled(); unmount(); }); + + it('should have higher priority than other priority listeners when enabled', async () => { + // 1. Initial state with a child component's priority listener (already subscribed) + // It should NOT handle Ctrl+S so we can enter copy mode. + const childHandler = vi.fn().mockReturnValue(false); + await setupCopyModeTest(true, childHandler); + + // 2. Enter copy mode + act(() => { + stdin.write('\x13'); // Ctrl+S + }); + rerender(); + + // 3. Verify we are in copy mode + expect(disableMouseEvents).toHaveBeenCalled(); + + // 4. Press any key + childHandler.mockClear(); + // Now childHandler should return true for other keys, simulating a greedy listener + childHandler.mockReturnValue(true); + + act(() => { + stdin.write('a'); + }); + rerender(); + + // 5. Verify that the exit handler took priority and childHandler was NOT called + expect(childHandler).not.toHaveBeenCalled(); + expect(enableMouseEvents).toHaveBeenCalled(); + unmount(); + }); } }); }); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 97e1cec2b7..174510d066 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -101,6 +101,7 @@ import { type LoadableSettingScope, SettingScope } from '../config/settings.js'; import { type InitializationResult } from '../core/initializer.js'; import { useFocus } from './hooks/useFocus.js'; import { useKeypress, type Key } from './hooks/useKeypress.js'; +import { KeypressPriority } from './contexts/KeypressContext.js'; import { keyMatchers, Command } from './keyMatchers.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useShellInactivityStatus } from './hooks/useShellInactivityStatus.js'; @@ -1483,13 +1484,6 @@ Logging in with Google... Restarting Gemini CLI to continue. const handleGlobalKeypress = useCallback( (key: Key): boolean => { - if (copyModeEnabled) { - setCopyModeEnabled(false); - enableMouseEvents(); - // We don't want to process any other keys if we're in copy mode. - return true; - } - // Debug log keystrokes if enabled if (settings.merged.general.debugKeystrokeLogging) { debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key)); @@ -1656,7 +1650,6 @@ Logging in with Google... Restarting Gemini CLI to continue. settings.merged.general.debugKeystrokeLogging, refreshStatic, setCopyModeEnabled, - copyModeEnabled, isAlternateBuffer, backgroundCurrentShell, toggleBackgroundShell, @@ -1672,6 +1665,20 @@ Logging in with Google... Restarting Gemini CLI to continue. useKeypress(handleGlobalKeypress, { isActive: true, priority: true }); + useKeypress( + () => { + setCopyModeEnabled(false); + enableMouseEvents(); + return true; + }, + { + isActive: copyModeEnabled, + // We need to receive keypresses first so they do not bubble to other + // handlers. + priority: KeypressPriority.Critical, + }, + ); + useEffect(() => { // Respect hideWindowTitle settings if (settings.merged.ui.hideWindowTitle) return; diff --git a/packages/cli/src/ui/contexts/KeypressContext.tsx b/packages/cli/src/ui/contexts/KeypressContext.tsx index 6b3a7db6d9..d31b1e4fbd 100644 --- a/packages/cli/src/ui/contexts/KeypressContext.tsx +++ b/packages/cli/src/ui/contexts/KeypressContext.tsx @@ -6,6 +6,7 @@ import { debugLogger, type Config } from '@google/gemini-cli-core'; import { useStdin } from 'ink'; +import { MultiMap } from 'mnemonist'; import type React from 'react'; import { createContext, @@ -26,6 +27,13 @@ export const ESC_TIMEOUT = 50; export const PASTE_TIMEOUT = 30_000; export const FAST_RETURN_TIMEOUT = 30; +export enum KeypressPriority { + Low = -100, + Normal = 0, + High = 100, + Critical = 200, +} + // Parse the key itself const KEY_INFO_MAP: Record< string, @@ -645,7 +653,10 @@ export interface Key { export type KeypressHandler = (key: Key) => boolean | void; interface KeypressContextValue { - subscribe: (handler: KeypressHandler, priority?: boolean) => void; + subscribe: ( + handler: KeypressHandler, + priority?: KeypressPriority | boolean, + ) => void; unsubscribe: (handler: KeypressHandler) => void; } @@ -674,44 +685,75 @@ export function KeypressProvider({ }) { const { stdin, setRawMode } = useStdin(); - const prioritySubscribers = useRef>(new Set()).current; - const normalSubscribers = useRef>(new Set()).current; + const subscribersToPriority = useRef>( + new Map(), + ).current; + const subscribers = useRef( + new MultiMap(Set), + ).current; + const sortedPriorities = useRef([]); const subscribe = useCallback( - (handler: KeypressHandler, priority = false) => { - const set = priority ? prioritySubscribers : normalSubscribers; - set.add(handler); + ( + handler: KeypressHandler, + priority: KeypressPriority | boolean = KeypressPriority.Normal, + ) => { + const p = + typeof priority === 'boolean' + ? priority + ? KeypressPriority.High + : KeypressPriority.Normal + : priority; + + subscribersToPriority.set(handler, p); + const hadPriority = subscribers.has(p); + subscribers.set(p, handler); + + if (!hadPriority) { + // Cache sorted priorities only when a new priority level is added + sortedPriorities.current = Array.from(subscribers.keys()).sort( + (a, b) => b - a, + ); + } }, - [prioritySubscribers, normalSubscribers], + [subscribers, subscribersToPriority], ); const unsubscribe = useCallback( (handler: KeypressHandler) => { - prioritySubscribers.delete(handler); - normalSubscribers.delete(handler); + const p = subscribersToPriority.get(handler); + if (p !== undefined) { + subscribers.remove(p, handler); + subscribersToPriority.delete(handler); + + if (!subscribers.has(p)) { + // Cache sorted priorities only when a priority level is completely removed + sortedPriorities.current = Array.from(subscribers.keys()).sort( + (a, b) => b - a, + ); + } + } }, - [prioritySubscribers, normalSubscribers], + [subscribers, subscribersToPriority], ); const broadcast = useCallback( (key: Key) => { - // Process priority subscribers first, in reverse order (stack behavior: last subscribed is first to handle) - const priorityHandlers = Array.from(prioritySubscribers).reverse(); - for (const handler of priorityHandlers) { - if (handler(key) === true) { - return; - } - } + // Use cached sorted priorities to avoid sorting on every keypress + for (const p of sortedPriorities.current) { + const set = subscribers.get(p); + if (!set) continue; - // Then process normal subscribers, also in reverse order - const normalHandlers = Array.from(normalSubscribers).reverse(); - for (const handler of normalHandlers) { - if (handler(key) === true) { - return; + // Within a priority level, use stack behavior (last subscribed is first to handle) + const handlers = Array.from(set).reverse(); + for (const handler of handlers) { + if (handler(key) === true) { + return; + } } } }, - [prioritySubscribers, normalSubscribers], + [subscribers], ); useEffect(() => { diff --git a/packages/cli/src/ui/hooks/useKeypress.ts b/packages/cli/src/ui/hooks/useKeypress.ts index 7df1b195a6..a06599d2d2 100644 --- a/packages/cli/src/ui/hooks/useKeypress.ts +++ b/packages/cli/src/ui/hooks/useKeypress.ts @@ -5,8 +5,12 @@ */ import { useEffect } from 'react'; -import type { KeypressHandler, Key } from '../contexts/KeypressContext.js'; -import { useKeypressContext } from '../contexts/KeypressContext.js'; +import { + useKeypressContext, + type KeypressHandler, + type Key, + type KeypressPriority, +} from '../contexts/KeypressContext.js'; export type { Key }; @@ -16,11 +20,14 @@ export type { Key }; * @param onKeypress - The callback function to execute on each keypress. * @param options - Options to control the hook's behavior. * @param options.isActive - Whether the hook should be actively listening for input. - * @param options.priority - Whether the hook should have priority over normal subscribers. + * @param options.priority - Priority level (integer or KeypressPriority enum) or boolean for backward compatibility. */ export function useKeypress( onKeypress: KeypressHandler, - { isActive, priority }: { isActive: boolean; priority?: boolean }, + { + isActive, + priority, + }: { isActive: boolean; priority?: KeypressPriority | boolean }, ) { const { subscribe, unsubscribe } = useKeypressContext();