feat(cli): Prevent queuing of slash and shell commands (#11094)

Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Jainam M
2025-10-15 22:32:50 +05:30
committed by GitHub
parent b8df8b2ab8
commit 4f17eae5cc
7 changed files with 215 additions and 2 deletions
+103
View File
@@ -956,6 +956,109 @@ describe('AppContainer State Management', () => {
}); });
}); });
describe('Queue Error Message', () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it('should set and clear the queue error message after a timeout', async () => {
const { rerender } = render(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBeNull();
capturedUIActions.setQueueErrorMessage('Test error');
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBe('Test error');
vi.advanceTimersByTime(3000);
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBeNull();
});
it('should reset the timer if a new error message is set', async () => {
const { rerender } = render(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
capturedUIActions.setQueueErrorMessage('First error');
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBe('First error');
vi.advanceTimersByTime(1500);
capturedUIActions.setQueueErrorMessage('Second error');
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBe('Second error');
vi.advanceTimersByTime(2000);
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBe('Second error');
// 5. Advance time past the 3 second timeout from the second message
vi.advanceTimersByTime(1000);
rerender(
<AppContainer
config={mockConfig}
settings={mockSettings}
version="1.0.0"
initializationResult={mockInitResult}
/>,
);
expect(capturedUIState.queueErrorMessage).toBeNull();
});
});
describe('Terminal Height Calculation', () => { describe('Terminal Height Calculation', () => {
const mockedMeasureElement = measureElement as Mock; const mockedMeasureElement = measureElement as Mock;
const mockedUseTerminalSize = useTerminalSize as Mock; const mockedUseTerminalSize = useTerminalSize as Mock;
+20
View File
@@ -93,6 +93,7 @@ import { useExtensionUpdates } from './hooks/useExtensionUpdates.js';
import { ShellFocusContext } from './contexts/ShellFocusContext.js'; import { ShellFocusContext } from './contexts/ShellFocusContext.js';
const CTRL_EXIT_PROMPT_DURATION_MS = 1000; const CTRL_EXIT_PROMPT_DURATION_MS = 1000;
const QUEUE_ERROR_DISPLAY_DURATION_MS = 3000;
function isToolExecuting(pendingHistoryItems: HistoryItemWithoutId[]) { function isToolExecuting(pendingHistoryItems: HistoryItemWithoutId[]) {
return pendingHistoryItems.some((item) => { return pendingHistoryItems.some((item) => {
@@ -154,6 +155,10 @@ export const AppContainer = (props: AppContainerProps) => {
config.isTrustedFolder(), config.isTrustedFolder(),
); );
const [queueErrorMessage, setQueueErrorMessage] = useState<string | null>(
null,
);
const extensions = config.getExtensions(); const extensions = config.getExtensions();
const { const {
extensionsUpdateState, extensionsUpdateState,
@@ -815,6 +820,17 @@ Logging in with Google... Please restart Gemini CLI to continue.
} }
}, [ideNeedsRestart]); }, [ideNeedsRestart]);
useEffect(() => {
if (queueErrorMessage) {
const timer = setTimeout(() => {
setQueueErrorMessage(null);
}, QUEUE_ERROR_DISPLAY_DURATION_MS);
return () => clearTimeout(timer);
}
return undefined;
}, [queueErrorMessage, setQueueErrorMessage]);
useEffect(() => { useEffect(() => {
if (isInitialMount.current) { if (isInitialMount.current) {
isInitialMount.current = false; isInitialMount.current = false;
@@ -1131,6 +1147,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
currentLoadingPhrase, currentLoadingPhrase,
historyRemountKey, historyRemountKey,
messageQueue, messageQueue,
queueErrorMessage,
showAutoAcceptIndicator, showAutoAcceptIndicator,
showWorkspaceMigrationDialog, showWorkspaceMigrationDialog,
workspaceExtensions, workspaceExtensions,
@@ -1212,6 +1229,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
currentLoadingPhrase, currentLoadingPhrase,
historyRemountKey, historyRemountKey,
messageQueue, messageQueue,
queueErrorMessage,
showAutoAcceptIndicator, showAutoAcceptIndicator,
showWorkspaceMigrationDialog, showWorkspaceMigrationDialog,
workspaceExtensions, workspaceExtensions,
@@ -1276,6 +1294,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
onWorkspaceMigrationDialogOpen, onWorkspaceMigrationDialogOpen,
onWorkspaceMigrationDialogClose, onWorkspaceMigrationDialogClose,
handleProQuotaChoice, handleProQuotaChoice,
setQueueErrorMessage,
}), }),
[ [
handleThemeSelect, handleThemeSelect,
@@ -1301,6 +1320,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
onWorkspaceMigrationDialogOpen, onWorkspaceMigrationDialogOpen,
onWorkspaceMigrationDialogClose, onWorkspaceMigrationDialogClose,
handleProQuotaChoice, handleProQuotaChoice,
setQueueErrorMessage,
], ],
); );
@@ -89,6 +89,8 @@ export const Composer = () => {
</Text> </Text>
) : uiState.showEscapePrompt ? ( ) : uiState.showEscapePrompt ? (
<Text color={theme.text.secondary}>Press Esc again to clear.</Text> <Text color={theme.text.secondary}>Press Esc again to clear.</Text>
) : uiState.queueErrorMessage ? (
<Text color={theme.status.error}>{uiState.queueErrorMessage}</Text>
) : ( ) : (
!settings.merged.ui?.hideContextSummary && ( !settings.merged.ui?.hideContextSummary && (
<ContextSummaryDisplay <ContextSummaryDisplay
@@ -149,6 +151,8 @@ export const Composer = () => {
? " Press 'i' for INSERT mode and 'Esc' for NORMAL mode." ? " Press 'i' for INSERT mode and 'Esc' for NORMAL mode."
: ' Type your message or @path/to/file' : ' Type your message or @path/to/file'
} }
setQueueErrorMessage={uiActions.setQueueErrorMessage}
streamingState={uiState.streamingState}
/> />
)} )}
@@ -28,6 +28,7 @@ import { useKittyKeyboardProtocol } from '../hooks/useKittyKeyboardProtocol.js';
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
import stripAnsi from 'strip-ansi'; import stripAnsi from 'strip-ansi';
import chalk from 'chalk'; import chalk from 'chalk';
import { StreamingState } from '../types.js';
vi.mock('../hooks/useShellHistory.js'); vi.mock('../hooks/useShellHistory.js');
vi.mock('../hooks/useCommandCompletion.js'); vi.mock('../hooks/useCommandCompletion.js');
@@ -2167,7 +2168,58 @@ describe('InputPrompt', () => {
expect(mockBuffer.handleInput).toHaveBeenCalled(); expect(mockBuffer.handleInput).toHaveBeenCalled();
unmount(); unmount();
}); });
it('should prevent slash commands from being queued while streaming', async () => {
props.onSubmit = vi.fn();
props.buffer.text = '/help';
props.setQueueErrorMessage = vi.fn();
props.streamingState = StreamingState.Responding;
const { stdin, unmount } = renderWithProviders(<InputPrompt {...props} />);
await wait();
stdin.write('/help');
stdin.write('\r');
await wait();
expect(props.onSubmit).not.toHaveBeenCalled();
expect(props.setQueueErrorMessage).toHaveBeenCalledWith(
'Slash commands cannot be queued',
);
unmount();
});
it('should prevent shell commands from being queued while streaming', async () => {
props.onSubmit = vi.fn();
props.buffer.text = 'ls';
props.setQueueErrorMessage = vi.fn();
props.streamingState = StreamingState.Responding;
props.shellModeActive = true;
const { stdin, unmount } = renderWithProviders(<InputPrompt {...props} />);
await wait();
stdin.write('ls');
stdin.write('\r');
await wait();
expect(props.onSubmit).not.toHaveBeenCalled();
expect(props.setQueueErrorMessage).toHaveBeenCalledWith(
'Shell commands cannot be queued',
);
unmount();
});
it('should allow regular messages to be queued while streaming', async () => {
props.onSubmit = vi.fn();
props.buffer.text = 'regular message';
props.setQueueErrorMessage = vi.fn();
props.streamingState = StreamingState.Responding;
const { stdin, unmount } = renderWithProviders(<InputPrompt {...props} />);
await wait();
stdin.write('regular message');
stdin.write('\r');
await wait();
expect(props.onSubmit).toHaveBeenCalledWith('regular message');
expect(props.setQueueErrorMessage).not.toHaveBeenCalled();
unmount();
});
}); });
function clean(str: string | undefined): string { function clean(str: string | undefined): string {
if (!str) return ''; if (!str) return '';
// Remove ANSI escape codes and trim whitespace // Remove ANSI escape codes and trim whitespace
+34 -2
View File
@@ -38,6 +38,8 @@ import * as path from 'node:path';
import { SCREEN_READER_USER_PREFIX } from '../textConstants.js'; import { SCREEN_READER_USER_PREFIX } from '../textConstants.js';
import { useShellFocusState } from '../contexts/ShellFocusContext.js'; import { useShellFocusState } from '../contexts/ShellFocusContext.js';
import { useUIState } from '../contexts/UIStateContext.js'; import { useUIState } from '../contexts/UIStateContext.js';
import { StreamingState } from '../types.js';
import { isSlashCommand } from '../utils/commandUtils.js';
/** /**
* Returns if the terminal can be trusted to handle paste events atomically * Returns if the terminal can be trusted to handle paste events atomically
@@ -71,6 +73,8 @@ export interface InputPromptProps {
onEscapePromptChange?: (showPrompt: boolean) => void; onEscapePromptChange?: (showPrompt: boolean) => void;
vimHandleInput?: (key: Key) => boolean; vimHandleInput?: (key: Key) => boolean;
isEmbeddedShellFocused?: boolean; isEmbeddedShellFocused?: boolean;
setQueueErrorMessage: (message: string | null) => void;
streamingState: StreamingState;
} }
// The input content, input container, and input suggestions list may have different widths // The input content, input container, and input suggestions list may have different widths
@@ -107,6 +111,8 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
onEscapePromptChange, onEscapePromptChange,
vimHandleInput, vimHandleInput,
isEmbeddedShellFocused, isEmbeddedShellFocused,
setQueueErrorMessage,
streamingState,
}) => { }) => {
const kittyProtocol = useKittyKeyboardProtocol(); const kittyProtocol = useKittyKeyboardProtocol();
const isShellFocused = useShellFocusState(); const isShellFocused = useShellFocusState();
@@ -221,6 +227,31 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
], ],
); );
const handleSubmit = useCallback(
(submittedValue: string) => {
const trimmedMessage = submittedValue.trim();
const isSlash = isSlashCommand(trimmedMessage);
const isShell = shellModeActive;
if (
(isSlash || isShell) &&
streamingState === StreamingState.Responding
) {
setQueueErrorMessage(
`${isShell ? 'Shell' : 'Slash'} commands cannot be queued`,
);
return;
}
handleSubmitAndClear(trimmedMessage);
},
[
handleSubmitAndClear,
shellModeActive,
streamingState,
setQueueErrorMessage,
],
);
const customSetTextAndResetCompletionSignal = useCallback( const customSetTextAndResetCompletionSignal = useCallback(
(newText: string) => { (newText: string) => {
buffer.setText(newText); buffer.setText(newText);
@@ -514,7 +545,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
// If the command is a perfect match, pressing enter should execute it. // If the command is a perfect match, pressing enter should execute it.
if (completion.isPerfectMatch && keyMatchers[Command.RETURN](key)) { if (completion.isPerfectMatch && keyMatchers[Command.RETURN](key)) {
handleSubmitAndClear(buffer.text); handleSubmit(buffer.text);
return; return;
} }
@@ -625,7 +656,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
buffer.backspace(); buffer.backspace();
buffer.newline(); buffer.newline();
} else { } else {
handleSubmitAndClear(buffer.text); handleSubmit(buffer.text);
} }
} }
return; return;
@@ -706,6 +737,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
onClearScreen, onClearScreen,
inputHistory, inputHistory,
handleSubmitAndClear, handleSubmitAndClear,
handleSubmit,
shellHistory, shellHistory,
reverseSearchCompletion, reverseSearchCompletion,
handleClipboardImage, handleClipboardImage,
@@ -45,6 +45,7 @@ export interface UIActions {
onWorkspaceMigrationDialogOpen: () => void; onWorkspaceMigrationDialogOpen: () => void;
onWorkspaceMigrationDialogClose: () => void; onWorkspaceMigrationDialogClose: () => void;
handleProQuotaChoice: (choice: 'auth' | 'continue') => void; handleProQuotaChoice: (choice: 'auth' | 'continue') => void;
setQueueErrorMessage: (message: string | null) => void;
} }
export const UIActionsContext = createContext<UIActions | null>(null); export const UIActionsContext = createContext<UIActions | null>(null);
@@ -89,6 +89,7 @@ export interface UIState {
currentLoadingPhrase: string; currentLoadingPhrase: string;
historyRemountKey: number; historyRemountKey: number;
messageQueue: string[]; messageQueue: string[];
queueErrorMessage: string | null;
showAutoAcceptIndicator: ApprovalMode; showAutoAcceptIndicator: ApprovalMode;
showWorkspaceMigrationDialog: boolean; showWorkspaceMigrationDialog: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any