From aca27709dfec3a56f775459e5b0b9d25bc593e28 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Sat, 17 May 2025 21:25:28 -0700 Subject: [PATCH] feat: Add auto-accept indicator and toggle - This commit introduces a visual indicator in the CLI to show when auto-accept for tool confirmations is enabled. Users can now also toggle this setting on/off using Shift + Tab. - This addresses user feedback for better visibility and control over the auto-accept feature, improving the overall user experience. - This behavior is similar to Claude Code, providing a familiar experience for users transitioning from that environment. - Added tests for the new auto indicator hook. Fixes https://b.corp.google.com/issues/413740468 --- packages/cli/src/ui/App.tsx | 30 ++- .../src/ui/components/AutoAcceptIndicator.tsx | 18 ++ .../src/ui/components/LoadingIndicator.tsx | 6 +- .../ui/hooks/useAutoAcceptIndicator.test.ts | 233 ++++++++++++++++++ .../src/ui/hooks/useAutoAcceptIndicator.ts | 39 +++ 5 files changed, 314 insertions(+), 12 deletions(-) create mode 100644 packages/cli/src/ui/components/AutoAcceptIndicator.tsx create mode 100644 packages/cli/src/ui/hooks/useAutoAcceptIndicator.test.ts create mode 100644 packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 707b8b9a..de1f0bf9 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -12,8 +12,10 @@ import { useGeminiStream } from './hooks/useGeminiStream.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useThemeCommand } from './hooks/useThemeCommand.js'; import { useSlashCommandProcessor } from './hooks/slashCommandProcessor.js'; +import { useAutoAcceptIndicator } from './hooks/useAutoAcceptIndicator.js'; import { Header } from './components/Header.js'; import { LoadingIndicator } from './components/LoadingIndicator.js'; +import { AutoAcceptIndicator } from './components/AutoAcceptIndicator.js'; import { EditorState, InputPrompt } from './components/InputPrompt.js'; import { Footer } from './components/Footer.js'; import { ThemeDialog } from './components/ThemeDialog.js'; @@ -144,6 +146,7 @@ export const App = ({ ); const { elapsedTime, currentLoadingPhrase } = useLoadingIndicator(streamingState); + const showAutoAcceptIndicator = useAutoAcceptIndicator({ config }); const handleFinalSubmit = useCallback( (submittedValue: string) => { @@ -327,23 +330,30 @@ export const App = ({ isLoading={streamingState === StreamingState.Responding} currentLoadingPhrase={currentLoadingPhrase} elapsedTime={elapsedTime} + rightContent={ + showAutoAcceptIndicator ? : undefined + } /> {isInputActive && ( - <> - - + + + <> cwd: {shortenPath(config.getTargetDir(), 70)} - + - + {showAutoAcceptIndicator && } + + )} + {isInputActive && ( + <> ( + + + accepting edits + (shift + tab to disable) + + +); diff --git a/packages/cli/src/ui/components/LoadingIndicator.tsx b/packages/cli/src/ui/components/LoadingIndicator.tsx index ca5fb5de..4f342c9d 100644 --- a/packages/cli/src/ui/components/LoadingIndicator.tsx +++ b/packages/cli/src/ui/components/LoadingIndicator.tsx @@ -13,12 +13,14 @@ interface LoadingIndicatorProps { isLoading: boolean; currentLoadingPhrase: string; elapsedTime: number; + rightContent?: React.ReactNode; } export const LoadingIndicator: React.FC = ({ isLoading, currentLoadingPhrase, elapsedTime, + rightContent, }) => { if (!isLoading) { return null; // Don't render anything if not loading @@ -30,10 +32,10 @@ export const LoadingIndicator: React.FC = ({ - {currentLoadingPhrase} ({elapsedTime}s) + {currentLoadingPhrase} (esc to cancel, {elapsedTime}s) {/* Spacer */} - (ESC to cancel) + {rightContent && {rightContent}} ); }; diff --git a/packages/cli/src/ui/hooks/useAutoAcceptIndicator.test.ts b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.test.ts new file mode 100644 index 00000000..9973b8eb --- /dev/null +++ b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.test.ts @@ -0,0 +1,233 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + describe, + it, + expect, + vi, + beforeEach, + type MockedFunction, + type Mock, +} from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { useAutoAcceptIndicator } from './useAutoAcceptIndicator.js'; + +import type { Config as ActualConfigType } from '@gemini-code/server'; +import { useInput, type Key as InkKey } from 'ink'; + +vi.mock('ink'); + +vi.mock('@gemini-code/server', async () => { + const actualServerModule = (await vi.importActual( + '@gemini-code/server', + )) as Record; + return { + ...actualServerModule, + Config: vi.fn(), + }; +}); + +import { Config } from '@gemini-code/server'; + +interface MockConfigInstanceShape { + getAlwaysSkipModificationConfirmation: Mock<() => boolean>; + setAlwaysSkipModificationConfirmation: Mock<(value: boolean) => void>; + getCoreTools: Mock<() => string[]>; + getToolDiscoveryCommand: Mock<() => string | undefined>; + getTargetDir: Mock<() => string>; + getApiKey: Mock<() => string>; + getModel: Mock<() => string>; + getSandbox: Mock<() => boolean | string>; + getDebugMode: Mock<() => boolean>; + getQuestion: Mock<() => string | undefined>; + getFullContext: Mock<() => boolean>; + getUserAgent: Mock<() => string>; + getUserMemory: Mock<() => string>; + getGeminiMdFileCount: Mock<() => number>; + getToolRegistry: Mock<() => { discoverTools: Mock<() => void> }>; +} + +type UseInputKey = InkKey; +type UseInputHandler = (input: string, key: UseInputKey) => void; + +describe('useAutoAcceptIndicator', () => { + let mockConfigInstance: MockConfigInstanceShape; + let capturedUseInputHandler: UseInputHandler; + let mockedInkUseInput: MockedFunction; + + beforeEach(() => { + vi.resetAllMocks(); + + ( + Config as unknown as MockedFunction<() => MockConfigInstanceShape> + ).mockImplementation(() => { + const instanceGetAlwaysSkipMock = vi.fn(); + const instanceSetAlwaysSkipMock = vi.fn(); + + const instance: MockConfigInstanceShape = { + getAlwaysSkipModificationConfirmation: + instanceGetAlwaysSkipMock as Mock<() => boolean>, + setAlwaysSkipModificationConfirmation: + instanceSetAlwaysSkipMock as Mock<(value: boolean) => void>, + getCoreTools: vi.fn().mockReturnValue([]) as Mock<() => string[]>, + getToolDiscoveryCommand: vi.fn().mockReturnValue(undefined) as Mock< + () => string | undefined + >, + getTargetDir: vi.fn().mockReturnValue('.') as Mock<() => string>, + getApiKey: vi.fn().mockReturnValue('test-api-key') as Mock< + () => string + >, + getModel: vi.fn().mockReturnValue('test-model') as Mock<() => string>, + getSandbox: vi.fn().mockReturnValue(false) as Mock< + () => boolean | string + >, + getDebugMode: vi.fn().mockReturnValue(false) as Mock<() => boolean>, + getQuestion: vi.fn().mockReturnValue(undefined) as Mock< + () => string | undefined + >, + getFullContext: vi.fn().mockReturnValue(false) as Mock<() => boolean>, + getUserAgent: vi.fn().mockReturnValue('test-user-agent') as Mock< + () => string + >, + getUserMemory: vi.fn().mockReturnValue('') as Mock<() => string>, + getGeminiMdFileCount: vi.fn().mockReturnValue(0) as Mock<() => number>, + getToolRegistry: vi + .fn() + .mockReturnValue({ discoverTools: vi.fn() }) as Mock< + () => { discoverTools: Mock<() => void> } + >, + }; + instanceSetAlwaysSkipMock.mockImplementation((value: boolean) => { + instanceGetAlwaysSkipMock.mockReturnValue(value); + }); + return instance; + }); + + mockedInkUseInput = useInput as MockedFunction; + mockedInkUseInput.mockImplementation((handler: UseInputHandler) => { + capturedUseInputHandler = handler; + }); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockConfigInstance = new (Config as any)() as MockConfigInstanceShape; + }); + + it('should initialize with true if config.getAlwaysSkipModificationConfirmation returns true', () => { + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + true, + ); + const { result } = renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + }), + ); + expect(result.current).toBe(true); + expect( + mockConfigInstance.getAlwaysSkipModificationConfirmation, + ).toHaveBeenCalledTimes(1); + }); + + it('should initialize with false if config.getAlwaysSkipModificationConfirmation returns false', () => { + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + false, + ); + const { result } = renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + }), + ); + expect(result.current).toBe(false); + expect( + mockConfigInstance.getAlwaysSkipModificationConfirmation, + ).toHaveBeenCalledTimes(1); + }); + + it('should toggle the indicator and update config when Shift+Tab is pressed', () => { + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + false, + ); + const { result } = renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + }), + ); + expect(result.current).toBe(false); + + act(() => { + capturedUseInputHandler('', { tab: true, shift: true } as InkKey); + }); + expect( + mockConfigInstance.setAlwaysSkipModificationConfirmation, + ).toHaveBeenCalledWith(true); + expect(result.current).toBe(true); + + act(() => { + capturedUseInputHandler('', { tab: true, shift: true } as InkKey); + }); + expect( + mockConfigInstance.setAlwaysSkipModificationConfirmation, + ).toHaveBeenCalledWith(false); + expect(result.current).toBe(false); + }); + + it('should not toggle if only Tab, only Shift, or other keys are pressed', () => { + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + false, + ); + renderHook(() => + useAutoAcceptIndicator({ + config: mockConfigInstance as unknown as ActualConfigType, + }), + ); + + act(() => { + capturedUseInputHandler('', { tab: true, shift: false } as InkKey); + }); + expect( + mockConfigInstance.setAlwaysSkipModificationConfirmation, + ).not.toHaveBeenCalled(); + + act(() => { + capturedUseInputHandler('', { tab: false, shift: true } as InkKey); + }); + expect( + mockConfigInstance.setAlwaysSkipModificationConfirmation, + ).not.toHaveBeenCalled(); + + act(() => { + capturedUseInputHandler('a', { tab: false, shift: false } as InkKey); + }); + expect( + mockConfigInstance.setAlwaysSkipModificationConfirmation, + ).not.toHaveBeenCalled(); + }); + + it('should update indicator when config value changes externally (useEffect dependency)', () => { + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + false, + ); + const { result, rerender } = renderHook( + (props: { config: ActualConfigType }) => useAutoAcceptIndicator(props), + { + initialProps: { + config: mockConfigInstance as unknown as ActualConfigType, + }, + }, + ); + expect(result.current).toBe(false); + + mockConfigInstance.getAlwaysSkipModificationConfirmation.mockReturnValue( + true, + ); + + rerender({ config: mockConfigInstance as unknown as ActualConfigType }); + expect(result.current).toBe(true); + expect( + mockConfigInstance.getAlwaysSkipModificationConfirmation, + ).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts new file mode 100644 index 00000000..17cffd40 --- /dev/null +++ b/packages/cli/src/ui/hooks/useAutoAcceptIndicator.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { useState, useEffect } from 'react'; +import { useInput } from 'ink'; +import type { Config } from '@gemini-code/server'; + +export interface UseAutoAcceptIndicatorArgs { + config: Config; +} + +export function useAutoAcceptIndicator({ + config, +}: UseAutoAcceptIndicatorArgs): boolean { + const currentConfigValue = config.getAlwaysSkipModificationConfirmation(); + const [showAutoAcceptIndicator, setShowAutoAcceptIndicator] = + useState(currentConfigValue); + + useEffect(() => { + setShowAutoAcceptIndicator(currentConfigValue); + }, [currentConfigValue]); + + useInput((_input, key) => { + if (key.tab && key.shift) { + const alwaysAcceptModificationConfirmations = + !config.getAlwaysSkipModificationConfirmation(); + config.setAlwaysSkipModificationConfirmation( + alwaysAcceptModificationConfirmations, + ); + // Update local state immediately for responsiveness + setShowAutoAcceptIndicator(alwaysAcceptModificationConfirmations); + } + }); + + return showAutoAcceptIndicator; +}