From d812c25b26acd580eab7404f03bb13d7c8405995 Mon Sep 17 00:00:00 2001 From: ruomeng Date: Mon, 9 Mar 2026 18:06:48 -0400 Subject: [PATCH] fix(cli): resolve duplicate footer on tool cancel via ESC (#21743) The ToolConfirmationMessage component previously suffered from a race condition where cancelling a tool confirmation via the ESC key would synchronously call handleConfirm. This caused the component to be abruptly unmounted before Ink could flush its final render frame, resulting in UI glitches like duplicate footers. --- .../messages/ToolConfirmationMessage.test.tsx | 71 +++++++++++++++++-- .../messages/ToolConfirmationMessage.tsx | 11 ++- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index ec623f69a4..13a799c715 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -4,16 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; -import type { - SerializableConfirmationDetails, - ToolCallConfirmationDetails, - Config, +import { + type SerializableConfirmationDetails, + type ToolCallConfirmationDetails, + type Config, + ToolConfirmationOutcome, } from '@google/gemini-cli-core'; import { renderWithProviders } from '../../../test-utils/render.js'; import { createMockSettings } from '../../../test-utils/settings.js'; import { useToolActions } from '../../contexts/ToolActionsContext.js'; +import { act } from 'react'; vi.mock('../../contexts/ToolActionsContext.js', async (importOriginal) => { const actual = @@ -644,4 +646,63 @@ describe('ToolConfirmationMessage', () => { expect(output).not.toContain('Invocation Arguments:'); unmount(); }); + + describe('ESCAPE key behavior', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('should call confirm(Cancel) asynchronously via useEffect when ESC is pressed', async () => { + const mockConfirm = vi.fn().mockResolvedValue(undefined); + + vi.mocked(useToolActions).mockReturnValue({ + confirm: mockConfirm, + cancel: vi.fn(), + isDiffingEnabled: false, + }); + + const confirmationDetails: SerializableConfirmationDetails = { + type: 'info', + title: 'Confirm Web Fetch', + prompt: 'https://example.com', + urls: ['https://example.com'], + }; + + const { stdin, waitUntilReady, unmount } = renderWithProviders( + , + ); + await waitUntilReady(); + + stdin.write('\x1b'); + + // To assert that the confirmation happens asynchronously (via useEffect) rather than + // synchronously (directly inside the keystroke handler), we must run our assertion + // *inside* the act() block. + await act(async () => { + await vi.runAllTimersAsync(); + expect(mockConfirm).not.toHaveBeenCalled(); + }); + + // Now that the act() block has returned, React flushes the useEffect, calling handleConfirm. + expect(mockConfirm).toHaveBeenCalledWith( + 'test-call-id', + ToolConfirmationOutcome.Cancel, + undefined, + ); + + unmount(); + }); + }); }); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 113852cb8d..472d7a94f5 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -5,7 +5,7 @@ */ import type React from 'react'; -import { useMemo, useCallback, useState } from 'react'; +import { useEffect, useMemo, useCallback, useState } from 'react'; import { Box, Text } from 'ink'; import { DiffRenderer } from './DiffRenderer.js'; import { RenderInline } from '../../utils/InlineMarkdownRenderer.js'; @@ -77,6 +77,7 @@ export const ToolConfirmationMessage: React.FC< callId, expanded: false, }); + const [isCancelling, setIsCancelling] = useState(false); const isMcpToolDetailsExpanded = mcpDetailsExpansionState.callId === callId ? mcpDetailsExpansionState.expanded @@ -179,7 +180,7 @@ export const ToolConfirmationMessage: React.FC< return true; } if (keyMatchers[Command.ESCAPE](key)) { - handleConfirm(ToolConfirmationOutcome.Cancel); + setIsCancelling(true); return true; } if (keyMatchers[Command.QUIT](key)) { @@ -192,6 +193,12 @@ export const ToolConfirmationMessage: React.FC< { isActive: isFocused, priority: true }, ); + useEffect(() => { + if (isCancelling) { + handleConfirm(ToolConfirmationOutcome.Cancel); + } + }, [isCancelling, handleConfirm]); + const handleSelect = useCallback( (item: ToolConfirmationOutcome) => handleConfirm(item), [handleConfirm],