Fixes /clear command to preserve input history for up-arrow navigation while still clearing the context window and screen (#14182)

This commit is contained in:
korade-krushna
2025-12-01 06:21:53 +05:30
committed by GitHub
parent f98e84f03f
commit f2466e5224
2 changed files with 47 additions and 96 deletions

View File

@@ -133,6 +133,7 @@ vi.mock('./contexts/VimModeContext.js');
vi.mock('./contexts/SessionContext.js');
vi.mock('./components/shared/text-buffer.js');
vi.mock('./hooks/useLogger.js');
vi.mock('./hooks/useInputHistoryStore.js');
// Mock external utilities
vi.mock('../utils/events.js');
@@ -160,6 +161,7 @@ import { useSessionStats } from './contexts/SessionContext.js';
import { useTextBuffer } from './components/shared/text-buffer.js';
import { useLogger } from './hooks/useLogger.js';
import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
import { useInputHistoryStore } from './hooks/useInputHistoryStore.js';
import { useKeypress, type Key } from './hooks/useKeypress.js';
import { measureElement } from 'ink';
import { useTerminalSize } from './hooks/useTerminalSize.js';
@@ -231,6 +233,7 @@ describe('AppContainer State Management', () => {
const mockedUseLogger = useLogger as Mock;
const mockedUseLoadingIndicator = useLoadingIndicator as Mock;
const mockedUseKeypress = useKeypress as Mock;
const mockedUseInputHistoryStore = useInputHistoryStore as Mock;
beforeEach(() => {
vi.clearAllMocks();
@@ -342,6 +345,11 @@ describe('AppContainer State Management', () => {
mockedUseLogger.mockReturnValue({
getPreviousUserMessages: vi.fn().mockReturnValue(new Promise(() => {})),
});
mockedUseInputHistoryStore.mockReturnValue({
inputHistory: [],
addInput: vi.fn(),
initializeFromLogger: vi.fn(),
});
mockedUseLoadingIndicator.mockReturnValue({
elapsedTime: '0.0s',
currentLoadingPhrase: '',
@@ -1846,10 +1854,11 @@ describe('AppContainer State Management', () => {
});
it('restores the prompt when onCancelSubmit is called with shouldRestorePrompt=true (or undefined)', async () => {
mockedUseLogger.mockReturnValue({
getPreviousUserMessages: vi
.fn()
.mockResolvedValue(['previous message']),
// Mock useInputHistoryStore to provide input history
mockedUseInputHistoryStore.mockReturnValue({
inputHistory: ['previous message'],
addInput: vi.fn(),
initializeFromLogger: vi.fn(),
});
const { unmount } = renderAppContainer();
@@ -1870,74 +1879,38 @@ describe('AppContainer State Management', () => {
unmount();
});
it('correctly restores prompt even if userMessages is stale (race condition fix)', async () => {
// Setup initial history with one message
const initialHistory = [{ type: 'user', text: 'Previous Prompt' }];
mockedUseHistory.mockReturnValue({
history: initialHistory,
addItem: vi.fn(),
updateItem: vi.fn(),
clearItems: vi.fn(),
loadHistory: vi.fn(),
it('input history is independent from conversation history (survives /clear)', async () => {
// This test verifies that input history (used for up-arrow navigation) is maintained
// separately from conversation history and survives /clear operations.
const mockAddInput = vi.fn();
mockedUseInputHistoryStore.mockReturnValue({
inputHistory: ['first prompt', 'second prompt'],
addInput: mockAddInput,
initializeFromLogger: vi.fn(),
});
let resolveLoggerPromise: (val: string[]) => void;
const loggerPromise = new Promise<string[]>((resolve) => {
resolveLoggerPromise = resolve;
});
const { unmount } = renderAppContainer();
// Mock logger to control when userMessages updates
const getPreviousUserMessagesMock = vi
.fn()
.mockResolvedValueOnce([]) // Initial mount
.mockReturnValueOnce(loggerPromise); // Second render (simulated update)
mockedUseLogger.mockReturnValue({
getPreviousUserMessages: getPreviousUserMessagesMock,
});
const { unmount, rerender } = renderAppContainer();
// Wait for userMessages to be populated with 'Previous Prompt'
// Verify userMessages is populated from inputHistory
await waitFor(() =>
expect(capturedUIState.userMessages).toContain('Previous Prompt'),
expect(capturedUIState.userMessages).toContain('first prompt'),
);
expect(capturedUIState.userMessages).toContain('second prompt');
// Simulate a new prompt being added (e.g., user sent it, but it overflowed)
const newPrompt = 'Current Prompt that Overflowed';
const newHistory = [...initialHistory, { type: 'user', text: newPrompt }];
// Clear the conversation history (simulating /clear command)
const mockClearItems = vi.fn();
mockedUseHistory.mockReturnValue({
history: newHistory,
history: [],
addItem: vi.fn(),
updateItem: vi.fn(),
clearItems: vi.fn(),
clearItems: mockClearItems,
loadHistory: vi.fn(),
});
// Rerender to reflect the history change.
// This triggers the effect to update userMessages, but it hangs on loggerPromise.
rerender(getAppContainer());
const { onCancelSubmit } = extractUseGeminiStreamArgs(
mockedUseGeminiStream.mock.lastCall!,
);
// Call onCancelSubmit immediately. userMessages is still stale (has only 'Previous Prompt')
// because the effect is waiting on loggerPromise.
act(() => {
onCancelSubmit(true);
});
// Now resolve the promise to let the effect complete and update userMessages
await act(async () => {
resolveLoggerPromise!([]);
});
// With the fix, it should have waited for userMessages to update and then set the new prompt
await waitFor(() => {
expect(mockSetText).toHaveBeenCalledWith(newPrompt);
});
// Verify that userMessages still contains the input history
// (it should not be affected by clearing conversation history)
expect(capturedUIState.userMessages).toContain('first prompt');
expect(capturedUIState.userMessages).toContain('second prompt');
unmount();
});

View File

@@ -118,6 +118,7 @@ import { isWorkspaceTrusted } from '../config/trustedFolders.js';
import { useAlternateBuffer } from './hooks/useAlternateBuffer.js';
import { useSettings } from './contexts/SettingsContext.js';
import { enableSupportedProtocol } from './utils/kittyProtocolDetector.js';
import { useInputHistoryStore } from './hooks/useInputHistoryStore.js';
import { enableBracketedPaste } from './utils/bracketedPaste.js';
const WARNING_PROMPT_DURATION_MS = 1000;
@@ -252,7 +253,8 @@ export const AppContainer = (props: AppContainerProps) => {
const [isConfigInitialized, setConfigInitialized] = useState(false);
const logger = useLogger(config.storage);
const [userMessages, setUserMessages] = useState<string[]>([]);
const { inputHistory, addInput, initializeFromLogger } =
useInputHistoryStore();
// Terminal and layout hooks
const { columns: terminalWidth, rows: terminalHeight } = useTerminalSize();
@@ -343,35 +345,10 @@ export const AppContainer = (props: AppContainerProps) => {
shellModeActive,
});
// Initialize input history from logger (past sessions)
useEffect(() => {
const fetchUserMessages = async () => {
const pastMessagesRaw = (await logger?.getPreviousUserMessages()) || [];
const currentSessionUserMessages = historyManager.history
.filter(
(item): item is HistoryItem & { type: 'user'; text: string } =>
item.type === 'user' &&
typeof item.text === 'string' &&
item.text.trim() !== '',
)
.map((item) => item.text)
.reverse();
const combinedMessages = [
...currentSessionUserMessages,
...pastMessagesRaw,
];
const deduplicatedMessages: string[] = [];
if (combinedMessages.length > 0) {
deduplicatedMessages.push(combinedMessages[0]);
for (let i = 1; i < combinedMessages.length; i++) {
if (combinedMessages[i] !== combinedMessages[i - 1]) {
deduplicatedMessages.push(combinedMessages[i]);
}
}
}
setUserMessages(deduplicatedMessages.reverse());
};
fetchUserMessages();
}, [historyManager.history, logger]);
initializeFromLogger(logger);
}, [logger, initializeFromLogger]);
const refreshStatic = useCallback(() => {
if (!isAlternateBuffer) {
@@ -710,7 +687,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
const lastHistoryUserMsg = historyManager.history.findLast(
(h) => h.type === 'user',
);
const lastUserMsg = userMessages.at(-1);
const lastUserMsg = inputHistory.at(-1);
if (
!lastHistoryUserMsg ||
@@ -721,7 +698,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
setPendingRestorePrompt(false);
}
}
}, [pendingRestorePrompt, userMessages, historyManager.history]);
}, [pendingRestorePrompt, inputHistory, historyManager.history]);
const {
streamingState,
@@ -785,7 +762,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
return;
}
const lastUserMessage = userMessages.at(-1);
const lastUserMessage = inputHistory.at(-1);
let textToSet = shouldRestorePrompt ? lastUserMessage || '' : '';
const queuedText = getQueuedMessagesText();
@@ -800,7 +777,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
},
[
buffer,
userMessages,
inputHistory,
getQueuedMessagesText,
clearQueue,
pendingSlashCommandHistoryItems,
@@ -811,8 +788,9 @@ Logging in with Google... Restarting Gemini CLI to continue.
const handleFinalSubmit = useCallback(
(submittedValue: string) => {
addMessage(submittedValue);
addInput(submittedValue); // Track input for up-arrow history
},
[addMessage],
[addMessage, addInput],
);
const handleClearScreen = useCallback(() => {
@@ -1438,7 +1416,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
pendingGeminiHistoryItems,
thought,
shellModeActive,
userMessages,
userMessages: inputHistory,
buffer,
inputWidth,
suggestionsWidth,
@@ -1529,7 +1507,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
pendingGeminiHistoryItems,
thought,
shellModeActive,
userMessages,
inputHistory,
buffer,
inputWidth,
suggestionsWidth,