feat(infra) - Add logging for when user tries to exit multiple times (#11218)

Co-authored-by: gemini-cli-robot <gemini-cli-robot@google.com>
This commit is contained in:
shishu314
2025-10-22 17:39:27 -04:00
committed by GitHub
parent 30dd2f1dfe
commit 4f220e945a
5 changed files with 219 additions and 176 deletions
+128 -127
View File
@@ -105,6 +105,7 @@ import { useSessionStats } from './contexts/SessionContext.js';
import { useTextBuffer } from './components/shared/text-buffer.js'; import { useTextBuffer } from './components/shared/text-buffer.js';
import { useLogger } from './hooks/useLogger.js'; import { useLogger } from './hooks/useLogger.js';
import { useLoadingIndicator } from './hooks/useLoadingIndicator.js'; import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
import { useKeypress, type Key } from './hooks/useKeypress.js';
import { measureElement } from 'ink'; import { measureElement } from 'ink';
import { useTerminalSize } from './hooks/useTerminalSize.js'; import { useTerminalSize } from './hooks/useTerminalSize.js';
import { ShellExecutionService } from '@google/gemini-cli-core'; import { ShellExecutionService } from '@google/gemini-cli-core';
@@ -136,6 +137,7 @@ describe('AppContainer State Management', () => {
const mockedUseTextBuffer = useTextBuffer as Mock; const mockedUseTextBuffer = useTextBuffer as Mock;
const mockedUseLogger = useLogger as Mock; const mockedUseLogger = useLogger as Mock;
const mockedUseLoadingIndicator = useLoadingIndicator as Mock; const mockedUseLoadingIndicator = useLoadingIndicator as Mock;
const mockedUseKeypress = useKeypress as Mock;
beforeEach(() => { beforeEach(() => {
vi.clearAllMocks(); vi.clearAllMocks();
@@ -1089,16 +1091,55 @@ describe('AppContainer State Management', () => {
}); });
}); });
describe('Keyboard Input Handling', () => { describe('Keyboard Input Handling (CTRL+C / CTRL+D)', () => {
it('should block quit command during authentication', () => { let handleGlobalKeypress: (key: Key) => void;
mockedUseAuthCommand.mockReturnValue({ let mockHandleSlashCommand: Mock;
authState: 'unauthenticated', let mockCancelOngoingRequest: Mock;
setAuthState: vi.fn(), let rerender: () => void;
authError: null,
onAuthError: vi.fn(), // Helper function to reduce boilerplate in tests
const setupKeypressTest = () => {
const { rerender: inkRerender } = render(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
rerender = () =>
inkRerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
};
const pressKey = (key: Partial<Key>, times = 1) => {
for (let i = 0; i < times; i++) {
handleGlobalKeypress({
name: 'c',
ctrl: false,
meta: false,
shift: false,
...key,
} as Key);
rerender();
}
};
beforeEach(() => {
// Capture the keypress handler from the AppContainer
mockedUseKeypress.mockImplementation((callback: (key: Key) => void) => {
handleGlobalKeypress = callback;
}); });
const mockHandleSlashCommand = vi.fn(); // Mock slash command handler
mockHandleSlashCommand = vi.fn();
mockedUseSlashCommandProcessor.mockReturnValue({ mockedUseSlashCommandProcessor.mockReturnValue({
handleSlashCommand: mockHandleSlashCommand, handleSlashCommand: mockHandleSlashCommand,
slashCommands: [], slashCommands: [],
@@ -1108,86 +1149,10 @@ describe('AppContainer State Management', () => {
confirmationRequest: null, confirmationRequest: null,
}); });
render( // Mock request cancellation
<AppContainer mockCancelOngoingRequest = vi.fn();
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit');
});
it('should prevent exit command when text buffer has content', () => {
mockedUseTextBuffer.mockReturnValue({
text: 'some user input',
setText: vi.fn(),
});
const mockHandleSlashCommand = vi.fn();
mockedUseSlashCommandProcessor.mockReturnValue({
handleSlashCommand: mockHandleSlashCommand,
slashCommands: [],
pendingHistoryItems: [],
commandContext: {},
shellConfirmationRequest: null,
confirmationRequest: null,
});
render(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit');
});
it('should require double Ctrl+C to exit when dialogs are open', () => {
vi.useFakeTimers();
mockedUseThemeCommand.mockReturnValue({
isThemeDialogOpen: true,
openThemeDialog: vi.fn(),
handleThemeSelect: vi.fn(),
handleThemeHighlight: vi.fn(),
});
const mockHandleSlashCommand = vi.fn();
mockedUseSlashCommandProcessor.mockReturnValue({
handleSlashCommand: mockHandleSlashCommand,
slashCommands: [],
pendingHistoryItems: [],
commandContext: {},
shellConfirmationRequest: null,
confirmationRequest: null,
});
render(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit');
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit');
vi.useRealTimers();
});
it('should cancel ongoing request on first Ctrl+C', () => {
const mockCancelOngoingRequest = vi.fn();
mockedUseGeminiStream.mockReturnValue({ mockedUseGeminiStream.mockReturnValue({
streamingState: 'responding', streamingState: 'idle',
submitQuery: vi.fn(), submitQuery: vi.fn(),
initError: null, initError: null,
pendingHistoryItems: [], pendingHistoryItems: [],
@@ -1195,57 +1160,93 @@ describe('AppContainer State Management', () => {
cancelOngoingRequest: mockCancelOngoingRequest, cancelOngoingRequest: mockCancelOngoingRequest,
}); });
const mockHandleSlashCommand = vi.fn(); // Default empty text buffer
mockedUseSlashCommandProcessor.mockReturnValue({ mockedUseTextBuffer.mockReturnValue({
handleSlashCommand: mockHandleSlashCommand, text: '',
slashCommands: [], setText: vi.fn(),
pendingHistoryItems: [],
commandContext: {},
shellConfirmationRequest: null,
confirmationRequest: null,
}); });
render( vi.useFakeTimers();
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit');
}); });
it('should reset Ctrl+C state after timeout', () => { afterEach(() => {
vi.useFakeTimers(); vi.useRealTimers();
});
const mockHandleSlashCommand = vi.fn(); describe('CTRL+C', () => {
mockedUseSlashCommandProcessor.mockReturnValue({ it('should cancel ongoing request on first press', () => {
handleSlashCommand: mockHandleSlashCommand, mockedUseGeminiStream.mockReturnValue({
slashCommands: [], streamingState: 'responding',
pendingHistoryItems: [], submitQuery: vi.fn(),
commandContext: {}, initError: null,
shellConfirmationRequest: null, pendingHistoryItems: [],
confirmationRequest: null, thought: null,
cancelOngoingRequest: mockCancelOngoingRequest,
});
setupKeypressTest();
pressKey({ name: 'c', ctrl: true });
expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(1);
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
}); });
render( it('should quit on second press', () => {
<AppContainer setupKeypressTest();
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit'); pressKey({ name: 'c', ctrl: true }, 2);
vi.advanceTimersByTime(1001); expect(mockCancelOngoingRequest).toHaveBeenCalledTimes(2);
expect(mockHandleSlashCommand).toHaveBeenCalledWith('/quit');
});
expect(mockHandleSlashCommand).not.toHaveBeenCalledWith('/quit'); it('should reset press count after a timeout', () => {
setupKeypressTest();
vi.useRealTimers(); pressKey({ name: 'c', ctrl: true });
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
// Advance timer past the reset threshold
vi.advanceTimersByTime(1001);
pressKey({ name: 'c', ctrl: true });
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
});
});
describe('CTRL+D', () => {
it('should do nothing if text buffer is not empty', () => {
mockedUseTextBuffer.mockReturnValue({
text: 'some text',
setText: vi.fn(),
});
setupKeypressTest();
pressKey({ name: 'd', ctrl: true }, 2);
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
});
it('should quit on second press if buffer is empty', () => {
setupKeypressTest();
pressKey({ name: 'd', ctrl: true }, 2);
expect(mockHandleSlashCommand).toHaveBeenCalledWith('/quit');
});
it('should reset press count after a timeout', () => {
setupKeypressTest();
pressKey({ name: 'd', ctrl: true });
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
// Advance timer past the reset threshold
vi.advanceTimersByTime(1001);
pressKey({ name: 'd', ctrl: true });
expect(mockHandleSlashCommand).not.toHaveBeenCalled();
});
}); });
}); });
+50 -49
View File
@@ -41,6 +41,7 @@ import {
getAllGeminiMdFilenames, getAllGeminiMdFilenames,
AuthType, AuthType,
clearCachedCredentialFile, clearCachedCredentialFile,
recordExitFail,
ShellExecutionService, ShellExecutionService,
debugLogger, debugLogger,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
@@ -798,9 +799,9 @@ Logging in with Google... Please restart Gemini CLI to continue.
const [showFullTodos, setShowFullTodos] = useState<boolean>(false); const [showFullTodos, setShowFullTodos] = useState<boolean>(false);
const [renderMarkdown, setRenderMarkdown] = useState<boolean>(true); const [renderMarkdown, setRenderMarkdown] = useState<boolean>(true);
const [ctrlCPressedOnce, setCtrlCPressedOnce] = useState(false); const [ctrlCPressCount, setCtrlCPressCount] = useState(0);
const ctrlCTimerRef = useRef<NodeJS.Timeout | null>(null); const ctrlCTimerRef = useRef<NodeJS.Timeout | null>(null);
const [ctrlDPressedOnce, setCtrlDPressedOnce] = useState(false); const [ctrlDPressCount, setCtrlDPressCount] = useState(0);
const ctrlDTimerRef = useRef<NodeJS.Timeout | null>(null); const ctrlDTimerRef = useRef<NodeJS.Timeout | null>(null);
const [constrainHeight, setConstrainHeight] = useState<boolean>(true); const [constrainHeight, setConstrainHeight] = useState<boolean>(true);
const [ideContextState, setIdeContextState] = useState< const [ideContextState, setIdeContextState] = useState<
@@ -878,6 +879,42 @@ Logging in with Google... Please restart Gemini CLI to continue.
}; };
}, [handleNewMessage]); }, [handleNewMessage]);
useEffect(() => {
if (ctrlCTimerRef.current) {
clearTimeout(ctrlCTimerRef.current);
ctrlCTimerRef.current = null;
}
if (ctrlCPressCount > 2) {
recordExitFail(config);
}
if (ctrlCPressCount > 1) {
handleSlashCommand('/quit');
} else {
ctrlCTimerRef.current = setTimeout(() => {
setCtrlCPressCount(0);
ctrlCTimerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
}
}, [ctrlCPressCount, config, setCtrlCPressCount, handleSlashCommand]);
useEffect(() => {
if (ctrlDTimerRef.current) {
clearTimeout(ctrlDTimerRef.current);
ctrlCTimerRef.current = null;
}
if (ctrlDPressCount > 2) {
recordExitFail(config);
}
if (ctrlDPressCount > 1) {
handleSlashCommand('/quit');
} else {
ctrlDTimerRef.current = setTimeout(() => {
setCtrlDPressCount(0);
ctrlDTimerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
}
}, [ctrlDPressCount, config, setCtrlDPressCount, handleSlashCommand]);
const handleEscapePromptChange = useCallback((showPrompt: boolean) => { const handleEscapePromptChange = useCallback((showPrompt: boolean) => {
setShowEscapePrompt(showPrompt); setShowEscapePrompt(showPrompt);
}, []); }, []);
@@ -908,28 +945,6 @@ Logging in with Google... Please restart Gemini CLI to continue.
settings.merged.ui?.customWittyPhrases, settings.merged.ui?.customWittyPhrases,
); );
const handleExit = useCallback(
(
pressedOnce: boolean,
setPressedOnce: (value: boolean) => void,
timerRef: React.MutableRefObject<NodeJS.Timeout | null>,
) => {
if (pressedOnce) {
if (timerRef.current) {
clearTimeout(timerRef.current);
}
handleSlashCommand('/quit');
} else {
setPressedOnce(true);
timerRef.current = setTimeout(() => {
setPressedOnce(false);
timerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
}
},
[handleSlashCommand],
);
const handleGlobalKeypress = useCallback( const handleGlobalKeypress = useCallback(
(key: Key) => { (key: Key) => {
// Debug log keystrokes if enabled // Debug log keystrokes if enabled
@@ -938,26 +953,17 @@ Logging in with Google... Please restart Gemini CLI to continue.
} }
if (keyMatchers[Command.QUIT](key)) { if (keyMatchers[Command.QUIT](key)) {
if (!ctrlCPressedOnce) { // If the user presses Ctrl+C, we want to cancel any ongoing requests.
cancelOngoingRequest?.(); // This should happen regardless of the count.
} cancelOngoingRequest?.();
if (!ctrlCPressedOnce) { setCtrlCPressCount((prev) => prev + 1);
setCtrlCPressedOnce(true);
ctrlCTimerRef.current = setTimeout(() => {
setCtrlCPressedOnce(false);
ctrlCTimerRef.current = null;
}, CTRL_EXIT_PROMPT_DURATION_MS);
return;
}
handleExit(ctrlCPressedOnce, setCtrlCPressedOnce, ctrlCTimerRef);
return; return;
} else if (keyMatchers[Command.EXIT](key)) { } else if (keyMatchers[Command.EXIT](key)) {
if (buffer.text.length > 0) { if (buffer.text.length > 0) {
return; return;
} }
handleExit(ctrlDPressedOnce, setCtrlDPressedOnce, ctrlDTimerRef); setCtrlDPressCount((prev) => prev + 1);
return; return;
} }
@@ -1001,14 +1007,9 @@ Logging in with Google... Please restart Gemini CLI to continue.
setShowErrorDetails, setShowErrorDetails,
config, config,
ideContextState, ideContextState,
handleExit, setCtrlCPressCount,
ctrlCPressedOnce,
setCtrlCPressedOnce,
ctrlCTimerRef,
buffer.text.length, buffer.text.length,
ctrlDPressedOnce, setCtrlDPressCount,
setCtrlDPressedOnce,
ctrlDTimerRef,
handleSlashCommand, handleSlashCommand,
cancelOngoingRequest, cancelOngoingRequest,
activePtyId, activePtyId,
@@ -1143,8 +1144,8 @@ Logging in with Google... Please restart Gemini CLI to continue.
filteredConsoleMessages, filteredConsoleMessages,
ideContextState, ideContextState,
renderMarkdown, renderMarkdown,
ctrlCPressedOnce, ctrlCPressedOnce: ctrlCPressCount >= 1,
ctrlDPressedOnce, ctrlDPressedOnce: ctrlDPressCount >= 1,
showEscapePrompt, showEscapePrompt,
isFocused, isFocused,
elapsedTime, elapsedTime,
@@ -1224,8 +1225,8 @@ Logging in with Google... Please restart Gemini CLI to continue.
filteredConsoleMessages, filteredConsoleMessages,
ideContextState, ideContextState,
renderMarkdown, renderMarkdown,
ctrlCPressedOnce, ctrlCPressCount,
ctrlDPressedOnce, ctrlDPressCount,
showEscapePrompt, showEscapePrompt,
isFocused, isFocused,
elapsedTime, elapsedTime,
+1
View File
@@ -108,6 +108,7 @@ export {
// Custom metrics for token usage and API responses // Custom metrics for token usage and API responses
recordCustomTokenUsageMetrics, recordCustomTokenUsageMetrics,
recordCustomApiResponseMetrics, recordCustomApiResponseMetrics,
recordExitFail,
// OpenTelemetry GenAI semantic convention for token usage and operation duration // OpenTelemetry GenAI semantic convention for token usage and operation duration
recordGenAiClientTokenUsage, recordGenAiClientTokenUsage,
recordGenAiClientOperationDuration, recordGenAiClientOperationDuration,
@@ -92,6 +92,7 @@ describe('Telemetry Metrics', () => {
let recordGenAiClientTokenUsageModule: typeof import('./metrics.js').recordGenAiClientTokenUsage; let recordGenAiClientTokenUsageModule: typeof import('./metrics.js').recordGenAiClientTokenUsage;
let recordGenAiClientOperationDurationModule: typeof import('./metrics.js').recordGenAiClientOperationDuration; let recordGenAiClientOperationDurationModule: typeof import('./metrics.js').recordGenAiClientOperationDuration;
let recordFlickerFrameModule: typeof import('./metrics.js').recordFlickerFrame; let recordFlickerFrameModule: typeof import('./metrics.js').recordFlickerFrame;
let recordExitFailModule: typeof import('./metrics.js').recordExitFail;
let recordAgentRunMetricsModule: typeof import('./metrics.js').recordAgentRunMetrics; let recordAgentRunMetricsModule: typeof import('./metrics.js').recordAgentRunMetrics;
beforeEach(async () => { beforeEach(async () => {
@@ -133,6 +134,7 @@ describe('Telemetry Metrics', () => {
recordGenAiClientOperationDurationModule = recordGenAiClientOperationDurationModule =
metricsJsModule.recordGenAiClientOperationDuration; metricsJsModule.recordGenAiClientOperationDuration;
recordFlickerFrameModule = metricsJsModule.recordFlickerFrame; recordFlickerFrameModule = metricsJsModule.recordFlickerFrame;
recordExitFailModule = metricsJsModule.recordExitFail;
recordAgentRunMetricsModule = metricsJsModule.recordAgentRunMetrics; recordAgentRunMetricsModule = metricsJsModule.recordAgentRunMetrics;
const otelApiModule = await import('@opentelemetry/api'); const otelApiModule = await import('@opentelemetry/api');
@@ -170,6 +172,28 @@ describe('Telemetry Metrics', () => {
}); });
}); });
describe('recordExitFail', () => {
it('does not record metrics if not initialized', () => {
const config = makeFakeConfig({});
recordExitFailModule(config);
expect(mockCounterAddFn).not.toHaveBeenCalled();
});
it('records a exit fail event when initialized', () => {
const config = makeFakeConfig({});
initializeMetricsModule(config);
recordExitFailModule(config);
// Called for session, then for exit fail
expect(mockCounterAddFn).toHaveBeenCalledTimes(2);
expect(mockCounterAddFn).toHaveBeenNthCalledWith(2, 1, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
});
});
});
describe('initializeMetrics', () => { describe('initializeMetrics', () => {
const mockConfig = { const mockConfig = {
getSessionId: () => 'test-session-id', getSessionId: () => 'test-session-id',
+16
View File
@@ -56,6 +56,7 @@ const REGRESSION_PERCENTAGE_CHANGE =
'gemini_cli.performance.regression.percentage_change'; 'gemini_cli.performance.regression.percentage_change';
const BASELINE_COMPARISON = 'gemini_cli.performance.baseline.comparison'; const BASELINE_COMPARISON = 'gemini_cli.performance.baseline.comparison';
const FLICKER_FRAME_COUNT = 'gemini_cli.ui.flicker.count'; const FLICKER_FRAME_COUNT = 'gemini_cli.ui.flicker.count';
const EXIT_FAIL_COUNT = 'gemini_cli.exit.fail.count';
const baseMetricDefinition = { const baseMetricDefinition = {
getCommonAttributes, getCommonAttributes,
@@ -175,6 +176,12 @@ const COUNTER_DEFINITIONS = {
assign: (c: Counter) => (flickerFrameCounter = c), assign: (c: Counter) => (flickerFrameCounter = c),
attributes: {} as Record<string, never>, attributes: {} as Record<string, never>,
}, },
[EXIT_FAIL_COUNT]: {
description: 'Counts CLI exit failures.',
valueType: ValueType.INT,
assign: (c: Counter) => (exitFailCounter = c),
attributes: {} as Record<string, never>,
},
} as const; } as const;
const HISTOGRAM_DEFINITIONS = { const HISTOGRAM_DEFINITIONS = {
@@ -458,6 +465,7 @@ let agentRunCounter: Counter | undefined;
let agentDurationHistogram: Histogram | undefined; let agentDurationHistogram: Histogram | undefined;
let agentTurnsHistogram: Histogram | undefined; let agentTurnsHistogram: Histogram | undefined;
let flickerFrameCounter: Counter | undefined; let flickerFrameCounter: Counter | undefined;
let exitFailCounter: Counter | undefined;
// OpenTelemetry GenAI Semantic Convention Metrics // OpenTelemetry GenAI Semantic Convention Metrics
let genAiClientTokenUsageHistogram: Histogram | undefined; let genAiClientTokenUsageHistogram: Histogram | undefined;
@@ -623,6 +631,14 @@ export function recordFlickerFrame(config: Config): void {
flickerFrameCounter.add(1, baseMetricDefinition.getCommonAttributes(config)); flickerFrameCounter.add(1, baseMetricDefinition.getCommonAttributes(config));
} }
/**
* Records a metric for when user failed to exit
*/
export function recordExitFail(config: Config): void {
if (!exitFailCounter || !isMetricsInitialized) return;
exitFailCounter.add(1, baseMetricDefinition.getCommonAttributes(config));
}
/** /**
* Records a metric for when an invalid chunk is received from a stream. * Records a metric for when an invalid chunk is received from a stream.
*/ */