From 8c03f74a1a9f1a8c25486fa7ac35f02b4dbf330f Mon Sep 17 00:00:00 2001 From: PansaLegrand <119485913+PansaLegrand@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:54:44 +0800 Subject: [PATCH] fix(cli): avoid duplicate history and telemetry after confirmation --- docs/cli/session-management.md | 4 + .../ui/hooks/slashCommandProcessor.test.tsx | 102 ++++++++++++++++++ .../cli/src/ui/hooks/slashCommandProcessor.ts | 12 ++- 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/docs/cli/session-management.md b/docs/cli/session-management.md index c18dd1523df..f269dc6bec2 100644 --- a/docs/cli/session-management.md +++ b/docs/cli/session-management.md @@ -91,6 +91,10 @@ For named branch points inside a session, use chat checkpoints: /resume resume decision-point ``` +If a checkpoint with the same tag exists, Gemini CLI asks you to confirm before +overwriting it. Confirming the save keeps a single command entry in your chat +history. Canceling preserves the existing checkpoint. + Compatibility aliases: - `/chat ...` works for the same commands. diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx index e4f00661897..648807403f3 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx @@ -17,10 +17,12 @@ import { FileCommandLoader } from '../../services/FileCommandLoader.js'; import { McpPromptLoader } from '../../services/McpPromptLoader.js'; import { SlashCommandStatus, + ToolConfirmationOutcome, MCPDiscoveryState, makeFakeConfig, coreEvents, type GeminiClient, + type ToolExecuteConfirmationDetails, } from '@google/gemini-cli-core'; const { @@ -1042,6 +1044,106 @@ describe('useSlashCommandProcessor', () => { }); }); + describe.each(['confirm_action', 'confirm_shell_commands'] as const)( + '%s continuations', + (confirmationType) => { + it.each(['success', 'failure', 'cancel'] as const)( + 'records one history entry and one terminal event on %s', + async (outcome) => { + const action = vi + .fn>() + .mockResolvedValueOnce( + confirmationType === 'confirm_action' + ? { + type: 'confirm_action', + prompt: 'Overwrite the checkpoint?', + originalInvocation: { raw: '/test' }, + } + : { + type: 'confirm_shell_commands', + commandsToConfirm: ['echo test'], + originalInvocation: { raw: '/test' }, + }, + ); + if (outcome === 'failure') { + action.mockRejectedValue(new Error('Command failed')); + } else { + action.mockResolvedValue({ + type: 'message', + messageType: 'info', + content: 'Command completed', + }); + } + + const result = await setupProcessorHook({ + builtinCommands: [createTestCommand({ action })], + }); + let commandPromise!: ReturnType< + typeof result.current.handleSlashCommand + >; + await act(async () => { + commandPromise = result.current.handleSlashCommand('/test'); + }); + + expect(action).toHaveBeenCalledTimes(1); + expect(logSlashCommand).not.toHaveBeenCalled(); + + // An unrelated update must not expose a duplicate command after + // confirmation, even when consecutive-history deduplication cannot help. + result.current.commandContext.ui.addItem({ + type: MessageType.INFO, + text: 'Background update', + }); + + await act(async () => { + if (confirmationType === 'confirm_action') { + expect(result.current.confirmationRequest).not.toBeNull(); + result.current.confirmationRequest!.onConfirm( + outcome !== 'cancel', + ); + } else { + const pendingItem = result.current.pendingHistoryItems[0]; + if (pendingItem?.type !== 'tool_group') { + throw new Error('Expected a pending shell confirmation'); + } + // Client-initiated shell confirmations retain their callback. + const confirmation = pendingItem.tools[0] + .confirmationDetails as ToolExecuteConfirmationDetails; + expect(confirmation).toBeDefined(); + await confirmation.onConfirm( + outcome === 'cancel' + ? ToolConfirmationOutcome.Cancel + : ToolConfirmationOutcome.ProceedOnce, + ); + } + await commandPromise; + }); + + expect(action).toHaveBeenCalledTimes(outcome === 'cancel' ? 1 : 2); + expect( + mockAddItem.mock.calls.filter( + ([item]) => item.type === MessageType.USER, + ), + ).toEqual([ + [{ type: MessageType.USER, text: '/test' }, expect.any(Number)], + ]); + expect(logSlashCommand).toHaveBeenCalledExactlyOnceWith( + mockConfig, + expect.objectContaining({ + command: 'test', + status: + outcome === 'failure' + ? SlashCommandStatus.ERROR + : SlashCommandStatus.SUCCESS, + }), + ); + expect(result.current.confirmationRequest).toBeNull(); + expect(result.current.pendingHistoryItems).toEqual([]); + }, + ); + }, + ); + describe('Slash Command Logging', () => { const mockCommandAction = vi.fn().mockResolvedValue({ type: 'handled' }); let loggingTestCommands: SlashCommand[]; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 6e880ed4bba..1d9eff164b8 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -411,6 +411,7 @@ export const useSlashCommandProcessor = ( } let hasError = false; + let commandReinvoked = false; const subcommand = resolvedCommandPath.length > 1 @@ -641,6 +642,7 @@ export const useSlashCommandProcessor = ( ); } + commandReinvoked = true; return await handleSlashCommand( result.originalInvocation.raw, // Pass the approved commands as a one-time grant for this execution. @@ -673,10 +675,12 @@ export const useSlashCommandProcessor = ( return { type: 'handled' }; } + commandReinvoked = true; return await handleSlashCommand( result.originalInvocation.raw, undefined, true, + false, // Do not add to history again ); } case 'custom_dialog': { @@ -727,7 +731,13 @@ export const useSlashCommandProcessor = ( ); return { type: 'handled' }; } finally { - if (config && resolvedCommandPath[0] && !hasError) { + // A confirmed command's resumed invocation records its final outcome. + if ( + config && + resolvedCommandPath[0] && + !hasError && + !commandReinvoked + ) { const event = makeSlashCommandEvent({ command: resolvedCommandPath[0], subcommand,