refactor(cli): simplify keypress and mouse providers and update tests (#22853)

2026-04-06 19:31:15 -07:00 · 2026-03-18 16:38:56 +00:00
parent 81a97e78f1
commit d7dfcf7f99
40 changed files with 923 additions and 863 deletions
--- a/packages/cli/src/ui/contexts/KeypressContext.test.tsx
+++ b/packages/cli/src/ui/contexts/KeypressContext.test.tsx
@@ -5,13 +5,12 @@
 */

 import { debugLogger } from '@google/gemini-cli-core';
-import type React from 'react';
 import { act } from 'react';
-import { renderHook } from '../../test-utils/render.js';
+import { renderHookWithProviders } from '../../test-utils/render.js';
+import { createMockSettings } from '../../test-utils/settings.js';
 import { waitFor } from '../../test-utils/async.js';
 import { vi, afterAll, beforeAll, type Mock } from 'vitest';
 import {
-  KeypressProvider,
  useKeypressContext,
  ESC_TIMEOUT,
  FAST_RETURN_TIMEOUT,
@@ -52,11 +51,8 @@ class MockStdin extends EventEmitter {
 // Helper function to setup keypress test with standard configuration
 const setupKeypressTest = () => {
  const keyHandler = vi.fn();
-  const wrapper = ({ children }: { children: React.ReactNode }) => (
-    <KeypressProvider>{children}</KeypressProvider>
-  );

-  const { result } = renderHook(() => useKeypressContext(), { wrapper });
+  const { result } = renderHookWithProviders(() => useKeypressContext());
  act(() => result.current.subscribe(keyHandler));

  return { result, keyHandler };
@@ -66,10 +62,6 @@ describe('KeypressContext', () => {
  let stdin: MockStdin;
  const mockSetRawMode = vi.fn();

-  const wrapper = ({ children }: { children: React.ReactNode }) => (
-    <KeypressProvider>{children}</KeypressProvider>
-  );
-
  beforeAll(() => vi.useFakeTimers());
  afterAll(() => vi.useRealTimers());

@@ -269,10 +261,7 @@ describe('KeypressContext', () => {

    it('should handle double Escape', async () => {
      const keyHandler = vi.fn();
-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider>{children}</KeypressProvider>
-      );
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());
      act(() => result.current.subscribe(keyHandler));

      act(() => {
@@ -306,10 +295,7 @@ describe('KeypressContext', () => {
    it('should handle lone Escape key (keycode 27) with timeout when kitty protocol is enabled', async () => {
      // Use real timers for this test to avoid issues with stream/buffer timing
      const keyHandler = vi.fn();
-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider>{children}</KeypressProvider>
-      );
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());
      act(() => result.current.subscribe(keyHandler));

      // Send just ESC
@@ -432,7 +418,7 @@ describe('KeypressContext', () => {
    ])('should $name', async ({ pastedText, writeSequence }) => {
      const keyHandler = vi.fn();

-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -452,7 +438,7 @@ describe('KeypressContext', () => {

    it('should parse valid OSC 52 response', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -473,7 +459,7 @@ describe('KeypressContext', () => {

    it('should handle split OSC 52 response', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -499,7 +485,7 @@ describe('KeypressContext', () => {

    it('should handle OSC 52 response terminated by ESC \\', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -520,7 +506,7 @@ describe('KeypressContext', () => {

    it('should ignore unknown OSC sequences', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -537,7 +523,7 @@ describe('KeypressContext', () => {

    it('should ignore invalid OSC 52 format', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -569,13 +555,11 @@ describe('KeypressContext', () => {
    it('should not log keystrokes when debugKeystrokeLogging is false', async () => {
      const keyHandler = vi.fn();

-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider debugKeystrokeLogging={false}>
-          {children}
-        </KeypressProvider>
-      );
-
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext(), {
+        settings: createMockSettings({
+          general: { debugKeystrokeLogging: false },
+        }),
+      });

      act(() => result.current.subscribe(keyHandler));

@@ -593,13 +577,11 @@ describe('KeypressContext', () => {
    it('should log kitty buffer accumulation when debugKeystrokeLogging is true', async () => {
      const keyHandler = vi.fn();

-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider debugKeystrokeLogging={true}>
-          {children}
-        </KeypressProvider>
-      );
-
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext(), {
+        settings: createMockSettings({
+          general: { debugKeystrokeLogging: true },
+        }),
+      });

      act(() => result.current.subscribe(keyHandler));

@@ -614,13 +596,11 @@ describe('KeypressContext', () => {
    it('should show char codes when debugKeystrokeLogging is true even without debug mode', async () => {
      const keyHandler = vi.fn();

-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider debugKeystrokeLogging={true}>
-          {children}
-        </KeypressProvider>
-      );
-
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext(), {
+        settings: createMockSettings({
+          general: { debugKeystrokeLogging: true },
+        }),
+      });

      act(() => result.current.subscribe(keyHandler));

@@ -765,7 +745,7 @@ describe('KeypressContext', () => {
      'should recognize sequence "$sequence" as $expected.name',
      ({ sequence, expected }) => {
        const keyHandler = vi.fn();
-        const { result } = renderHook(() => useKeypressContext(), { wrapper });
+        const { result } = renderHookWithProviders(() => useKeypressContext());
        act(() => result.current.subscribe(keyHandler));

        act(() => stdin.write(sequence));
@@ -1000,12 +980,7 @@ describe('KeypressContext', () => {
      'should handle Alt+$key in $terminal',
      ({ chunk, expected }: { chunk: string; expected: Partial<Key> }) => {
        const keyHandler = vi.fn();
-        const testWrapper = ({ children }: { children: React.ReactNode }) => (
-          <KeypressProvider>{children}</KeypressProvider>
-        );
-        const { result } = renderHook(() => useKeypressContext(), {
-          wrapper: testWrapper,
-        });
+        const { result } = renderHookWithProviders(() => useKeypressContext());
        act(() => result.current.subscribe(keyHandler));

        act(() => stdin.write(chunk));
@@ -1042,7 +1017,7 @@ describe('KeypressContext', () => {

  it('should timeout and flush incomplete kitty sequences after 50ms', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1077,7 +1052,7 @@ describe('KeypressContext', () => {

  it('should immediately flush non-kitty CSI sequences', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1099,7 +1074,7 @@ describe('KeypressContext', () => {

  it('should parse valid kitty sequences immediately when complete', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1117,7 +1092,7 @@ describe('KeypressContext', () => {

  it('should handle batched kitty sequences correctly', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1144,7 +1119,7 @@ describe('KeypressContext', () => {

  it('should handle mixed valid and invalid sequences', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1172,7 +1147,7 @@ describe('KeypressContext', () => {
    'should handle sequences arriving character by character with %s ms delay',
    async (delay) => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1196,7 +1171,7 @@ describe('KeypressContext', () => {

  it('should reset timeout when new input arrives', async () => {
    const keyHandler = vi.fn();
-    const { result } = renderHook(() => useKeypressContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useKeypressContext());

    act(() => result.current.subscribe(keyHandler));

@@ -1231,7 +1206,7 @@ describe('KeypressContext', () => {
  describe('SGR Mouse Handling', () => {
    it('should ignore SGR mouse sequences', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1249,7 +1224,7 @@ describe('KeypressContext', () => {

    it('should handle mixed SGR mouse and key sequences', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1275,7 +1250,7 @@ describe('KeypressContext', () => {

    it('should ignore X11 mouse sequences', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1291,7 +1266,7 @@ describe('KeypressContext', () => {

    it('should not flush slow SGR mouse sequences as garbage', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1311,7 +1286,7 @@ describe('KeypressContext', () => {

    it('should ignore specific SGR mouse sequence sandwiched between keystrokes', async () => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());

      act(() => result.current.subscribe(keyHandler));

@@ -1342,12 +1317,7 @@ describe('KeypressContext', () => {
      { name: 'another mouse', sequence: '\u001b[<0;29;19m' },
    ])('should ignore $name sequence', async ({ sequence }) => {
      const keyHandler = vi.fn();
-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider>{children}</KeypressProvider>
-      );
-      const { result } = renderHook(() => useKeypressContext(), {
-        wrapper,
-      });
+      const { result } = renderHookWithProviders(() => useKeypressContext());
      act(() => result.current.subscribe(keyHandler));

      for (const char of sequence) {
@@ -1372,10 +1342,7 @@ describe('KeypressContext', () => {

    it('should handle F12', async () => {
      const keyHandler = vi.fn();
-      const wrapper = ({ children }: { children: React.ReactNode }) => (
-        <KeypressProvider>{children}</KeypressProvider>
-      );
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());
      act(() => result.current.subscribe(keyHandler));

      act(() => {
@@ -1404,7 +1371,7 @@ describe('KeypressContext', () => {
      'A你B好C', // Mixed characters
    ])('should correctly handle string "%s"', async (inputString) => {
      const keyHandler = vi.fn();
-      const { result } = renderHook(() => useKeypressContext(), { wrapper });
+      const { result } = renderHookWithProviders(() => useKeypressContext());
      act(() => result.current.subscribe(keyHandler));

      act(() => stdin.write(inputString));
--- a/packages/cli/src/ui/contexts/KeypressContext.tsx
+++ b/packages/cli/src/ui/contexts/KeypressContext.tsx
@@ -13,6 +13,7 @@ import {
  useCallback,
  useContext,
  useEffect,
+  useMemo,
  useRef,
 } from 'react';

@@ -21,6 +22,7 @@ import { parseMouseEvent } from '../utils/mouse.js';
 import { FOCUS_IN, FOCUS_OUT } from '../hooks/useFocus.js';
 import { appEvents, AppEvent } from '../../utils/events.js';
 import { terminalCapabilityManager } from '../utils/terminalCapabilityManager.js';
+import { useSettingsStore } from './SettingsContext.js';

 export const BACKSLASH_ENTER_TIMEOUT = 5;
 export const ESC_TIMEOUT = 50;
@@ -766,12 +768,13 @@ export function useKeypressContext() {
 export function KeypressProvider({
  children,
  config,
-  debugKeystrokeLogging,
 }: {
  children: React.ReactNode;
  config?: Config;
-  debugKeystrokeLogging?: boolean;
 }) {
+  const { settings } = useSettingsStore();
+  const debugKeystrokeLogging = settings.merged.general.debugKeystrokeLogging;
+
  const { stdin, setRawMode } = useStdin();

  const subscribersToPriority = useRef<Map<KeypressHandler, number>>(
@@ -828,6 +831,9 @@ export function KeypressProvider({

  const broadcast = useCallback(
    (key: Key) => {
+      if (debugKeystrokeLogging) {
+        debugLogger.log('[DEBUG] Keystroke:', JSON.stringify(key));
+      }
      // Use cached sorted priorities to avoid sorting on every keypress
      for (const p of sortedPriorities.current) {
        const set = subscribers.get(p);
@@ -842,7 +848,7 @@ export function KeypressProvider({
        }
      }
    },
-    [subscribers],
+    [subscribers, debugKeystrokeLogging],
  );

  useEffect(() => {
@@ -882,8 +888,13 @@ export function KeypressProvider({
    };
  }, [stdin, setRawMode, config, debugKeystrokeLogging, broadcast]);

+  const contextValue = useMemo(
+    () => ({ subscribe, unsubscribe }),
+    [subscribe, unsubscribe],
+  );
+
  return (
-    <KeypressContext.Provider value={{ subscribe, unsubscribe }}>
+    <KeypressContext.Provider value={contextValue}>
      {children}
    </KeypressContext.Provider>
  );
--- a/packages/cli/src/ui/contexts/MouseContext.test.tsx
+++ b/packages/cli/src/ui/contexts/MouseContext.test.tsx
@@ -4,10 +4,9 @@
 * SPDX-License-Identifier: Apache-2.0
 */

-import { renderHook } from '../../test-utils/render.js';
-import type React from 'react';
+import { renderHookWithProviders } from '../../test-utils/render.js';
 import { act } from 'react';
-import { MouseProvider, useMouseContext, useMouse } from './MouseContext.js';
+import { useMouseContext, useMouse } from './MouseContext.js';
 import { vi, type Mock } from 'vitest';
 import { useStdin } from 'ink';
 import { EventEmitter } from 'node:events';
@@ -49,7 +48,6 @@ class MockStdin extends EventEmitter {

 describe('MouseContext', () => {
  let stdin: MockStdin;
-  let wrapper: React.FC<{ children: React.ReactNode }>;

  beforeEach(() => {
    stdin = new MockStdin();
@@ -57,9 +55,6 @@ describe('MouseContext', () => {
      stdin,
      setRawMode: vi.fn(),
    });
-    wrapper = ({ children }: { children: React.ReactNode }) => (
-      <MouseProvider mouseEventsEnabled={true}>{children}</MouseProvider>
-    );
    vi.mocked(appEvents.emit).mockClear();
  });

@@ -69,7 +64,9 @@ describe('MouseContext', () => {

  it('should subscribe and unsubscribe a handler', () => {
    const handler = vi.fn();
-    const { result } = renderHook(() => useMouseContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      result.current.subscribe(handler);
@@ -94,8 +91,8 @@ describe('MouseContext', () => {

  it('should not call handler if not active', () => {
    const handler = vi.fn();
-    renderHook(() => useMouse(handler, { isActive: false }), {
-      wrapper,
+    renderHookWithProviders(() => useMouse(handler, { isActive: false }), {
+      mouseEventsEnabled: true,
    });

    act(() => {
@@ -106,7 +103,9 @@ describe('MouseContext', () => {
  });

  it('should emit SelectionWarning when move event is unhandled and has coordinates', () => {
-    renderHook(() => useMouseContext(), { wrapper });
+    renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      // Move event (32) at 10, 20
@@ -118,7 +117,9 @@ describe('MouseContext', () => {

  it('should not emit SelectionWarning when move event is handled', () => {
    const handler = vi.fn().mockReturnValue(true);
-    const { result } = renderHook(() => useMouseContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      result.current.subscribe(handler);
@@ -218,7 +219,9 @@ describe('MouseContext', () => {
      'should recognize sequence "$sequence" as $expected.name',
      ({ sequence, expected }) => {
        const mouseHandler = vi.fn();
-        const { result } = renderHook(() => useMouseContext(), { wrapper });
+        const { result } = renderHookWithProviders(() => useMouseContext(), {
+          mouseEventsEnabled: true,
+        });
        act(() => result.current.subscribe(mouseHandler));

        act(() => stdin.write(sequence));
@@ -232,7 +235,9 @@ describe('MouseContext', () => {

  it('should emit a double-click event when two left-presses occur quickly at the same position', () => {
    const handler = vi.fn();
-    const { result } = renderHook(() => useMouseContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      result.current.subscribe(handler);
@@ -262,7 +267,9 @@ describe('MouseContext', () => {

  it('should NOT emit a double-click event if clicks are too far apart', () => {
    const handler = vi.fn();
-    const { result } = renderHook(() => useMouseContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      result.current.subscribe(handler);
@@ -287,7 +294,9 @@ describe('MouseContext', () => {
  it('should NOT emit a double-click event if too much time passes', async () => {
    vi.useFakeTimers();
    const handler = vi.fn();
-    const { result } = renderHook(() => useMouseContext(), { wrapper });
+    const { result } = renderHookWithProviders(() => useMouseContext(), {
+      mouseEventsEnabled: true,
+    });

    act(() => {
      result.current.subscribe(handler);
--- a/packages/cli/src/ui/contexts/MouseContext.tsx
+++ b/packages/cli/src/ui/contexts/MouseContext.tsx
@@ -11,6 +11,7 @@ import {
  useCallback,
  useContext,
  useEffect,
+  useMemo,
  useRef,
 } from 'react';
 import { ESC } from '../utils/input.js';
@@ -25,6 +26,7 @@ import {
  DOUBLE_CLICK_THRESHOLD_MS,
  DOUBLE_CLICK_DISTANCE_TOLERANCE,
 } from '../utils/mouse.js';
+import { useSettingsStore } from './SettingsContext.js';

 export type { MouseEvent, MouseEventName, MouseHandler };

@@ -61,12 +63,13 @@ export function useMouse(handler: MouseHandler, { isActive = true } = {}) {
 export function MouseProvider({
  children,
  mouseEventsEnabled,
-  debugKeystrokeLogging,
 }: {
  children: React.ReactNode;
  mouseEventsEnabled?: boolean;
-  debugKeystrokeLogging?: boolean;
 }) {
+  const { settings } = useSettingsStore();
+  const debugKeystrokeLogging = settings.merged.general.debugKeystrokeLogging;
+
  const { stdin } = useStdin();
  const subscribers = useRef<Set<MouseHandler>>(new Set()).current;
  const lastClickRef = useRef<{
@@ -189,8 +192,13 @@ export function MouseProvider({
    };
  }, [stdin, mouseEventsEnabled, subscribers, debugKeystrokeLogging]);

+  const contextValue = useMemo(
+    () => ({ subscribe, unsubscribe }),
+    [subscribe, unsubscribe],
+  );
+
  return (
-    <MouseContext.Provider value={{ subscribe, unsubscribe }}>
+    <MouseContext.Provider value={contextValue}>
      {children}
    </MouseContext.Provider>
  );