mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-10 22:21:22 -07:00
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.
This commit is contained in:
@@ -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(
|
||||
<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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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],
|
||||
|
||||
Reference in New Issue
Block a user