fix(cli): resolve duplicate footer on tool cancel via ESC (#21743) (#21781)

This commit is contained in:
ruomeng
2026-03-18 16:27:38 -04:00
committed by GitHub
parent fd44718bfe
commit 94e6bf8591
2 changed files with 83 additions and 7 deletions

View File

@@ -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 =
@@ -646,4 +648,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(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={confirmationDetails}
config={mockConfig}
getPreferredEditor={vi.fn()}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);
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();
});
});
});

View File

@@ -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';
@@ -79,6 +79,7 @@ export const ToolConfirmationMessage: React.FC<
callId,
expanded: false,
});
const [isCancelling, setIsCancelling] = useState(false);
const isMcpToolDetailsExpanded =
mcpDetailsExpansionState.callId === callId
? mcpDetailsExpansionState.expanded
@@ -183,7 +184,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)) {
@@ -196,6 +197,20 @@ export const ToolConfirmationMessage: React.FC<
{ isActive: isFocused, priority: true },
);
// TODO(#23009): Remove this hack once we migrate to the new renderer.
// Why useEffect is used here instead of calling handleConfirm directly:
// There is a race condition where calling handleConfirm immediately upon
// keypress removes the tool UI component while the UI is in an expanded state.
// This simultaneously triggers setConstrainHeight, causing render two footers.
// By bridging the cancel action through state (isCancelling) and this useEffect,
// we delay handleConfirm until the next render cycle, ensuring setConstrainHeight
// resolves properly first.
useEffect(() => {
if (isCancelling) {
handleConfirm(ToolConfirmationOutcome.Cancel);
}
}, [isCancelling, handleConfirm]);
const handleSelect = useCallback(
(item: ToolConfirmationOutcome) => handleConfirm(item),
[handleConfirm],