diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index b970093f150..9f283060b49 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1219,8 +1219,11 @@ export const AppContainer = (props: AppContainerProps) => { hideTips: tipsDisabled, }); - // Track whether suggestions are visible for Tab key handling - const [hasSuggestionsVisible, setHasSuggestionsVisible] = useState(false); + // Track whether the input area has any Tab consumer (autocomplete dropdown, + // followup suggestion, mid-input ghost text, reverse/command search). When + // true, we suppress the Windows-only "bare Tab cycles approval mode" + // fallback so a single Tab keystroke triggers only one action. See #4171. + const [hasTabConsumer, setHasTabConsumer] = useState(false); const agentViewState = useAgentViewState(); const { dialogOpen: bgTasksDialogOpen } = useBackgroundTaskViewState(); @@ -1246,7 +1249,7 @@ export const AppContainer = (props: AppContainerProps) => { config, addItem: historyManager.addItem, onApprovalModeChange: handleApprovalModeChange, - shouldBlockTab: () => hasSuggestionsVisible, + shouldBlockTab: () => hasTabConsumer, disabled: agentViewState.activeView !== 'main', }); @@ -3250,7 +3253,7 @@ export const AppContainer = (props: AppContainerProps) => { handleFolderTrustSelect, setConstrainHeight, onEscapePromptChange: handleEscapePromptChange, - onSuggestionsVisibilityChange: setHasSuggestionsVisible, + onTabConsumerChange: setHasTabConsumer, refreshStatic, handleFinalSubmit, handleRetryLastPrompt: retryLastPrompt, diff --git a/packages/cli/src/ui/components/Composer.tsx b/packages/cli/src/ui/components/Composer.tsx index 846244b85d7..5e55f2efcbc 100644 --- a/packages/cli/src/ui/components/Composer.tsx +++ b/packages/cli/src/ui/components/Composer.tsx @@ -75,13 +75,18 @@ export const Composer = () => { setShowShortcuts((prev) => !prev); }, []); - // State for suggestions visibility + // State for autocomplete-dropdown visibility (narrow signal). Drives the + // Footer / KeyboardShortcuts hide-when-dropdown-visible logic below; kept + // local to Composer because nothing outside this component needs the + // narrow signal. const [showSuggestions, setShowSuggestions] = useState(false); - const handleSuggestionsVisibilityChange = useCallback( - (visible: boolean) => { - setShowSuggestions(visible); - // Also notify AppContainer for Tab key handling - uiActions.onSuggestionsVisibilityChange(visible); + + // Broad signal — any input-area Tab consumer. Forwarded to AppContainer + // via UIActionsContext so useAutoAcceptIndicator's `shouldBlockTab` can + // suppress the Windows-only bare-Tab approval-mode fallback. See #4171. + const handleTabConsumerChange = useCallback( + (active: boolean) => { + uiActions.onTabConsumerChange(active); }, [uiActions], ); @@ -145,7 +150,8 @@ export const Composer = () => { onEscapePromptChange={uiActions.onEscapePromptChange} onToggleShortcuts={handleToggleShortcuts} showShortcuts={showShortcuts} - onSuggestionsVisibilityChange={handleSuggestionsVisibilityChange} + onSuggestionsVisibilityChange={setShowSuggestions} + onTabConsumerChange={handleTabConsumerChange} focus={true} vimHandleInput={uiActions.vimHandleInput} isEmbeddedShellFocused={uiState.embeddedShellFocused} diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 515ac10cebf..d62ff88cbd3 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -431,6 +431,214 @@ describe('InputPrompt', () => { }); }); + // Regression for #4171: `onTabConsumerChange` (consumed by AppContainer + // as `shouldBlockTab`) must report `true` whenever ANY input-side handler + // would consume Tab — autocomplete dropdown, followup suggestion, or + // mid-input ghost text. Otherwise on Windows the bare-Tab approval-mode + // fallback double-fires alongside the input-side handler. + describe('onTabConsumerChange reporting (issue #4171)', () => { + // Match the SUGGESTION_DELAY_MS debounce inside createFollowupController. + const SUGGESTION_VISIBLE_WAIT_MS = 700; + + it('reports true while the followup prompt suggestion is visible', async () => { + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(SUGGESTION_VISIBLE_WAIT_MS); + + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + unmount(); + }); + + it('reports true while mid-input ghost text offers an accept', async () => { + mockCommandCompletion.midInputGhostText = { + text: 'ile.txt', + insertPosition: 1, + acceptText: 'ile.txt', + }; + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + unmount(); + }); + + it('reports true while the autocomplete dropdown is visible', async () => { + mockCommandCompletion.showSuggestions = true; + mockCommandCompletion.suggestions = [ + { + value: '/clear', + label: '/clear', + description: 'Clear screen', + }, + ] as UseCommandCompletionReturn['suggestions']; + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + unmount(); + }); + + it('reports false when the input area is idle (no dropdown, no followup, no ghost)', async () => { + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + // Pin the actual value (not just "never true") — the mount-time effect + // must report false when nothing in the input area wants Tab. + expect(onTabConsumerChange).toHaveBeenCalledWith(false); + expect(onTabConsumerChange).not.toHaveBeenCalledWith(true); + unmount(); + }); + + it('reports false again after the autocomplete dropdown is dismissed', async () => { + mockedUseCommandCompletion.mockReturnValue({ + ...mockCommandCompletion, + showSuggestions: true, + suggestions: [ + { + value: '/clear', + label: '/clear', + description: 'Clear screen', + }, + ] as UseCommandCompletionReturn['suggestions'], + }); + const onTabConsumerChange = vi.fn(); + const { rerender, unmount } = renderWithProviders( + , + ); + await wait(); + expect(onTabConsumerChange).toHaveBeenLastCalledWith(true); + + // Dismiss the dropdown and re-render — Windows Tab cycling must be + // re-enabled. Without this transition signal, the parent would keep + // suppressing the approval-mode fallback after the dropdown closed. + mockedUseCommandCompletion.mockReturnValue(mockCommandCompletion); + rerender( + , + ); + await wait(); + + expect(onTabConsumerChange).toHaveBeenLastCalledWith(false); + unmount(); + }); + + it('reports false on unmount even if a Tab consumer was active', async () => { + // Regression for the stale-signal bug: if InputPrompt unmounts while + // some Tab consumer is true (e.g. streaming starts while autocomplete + // is open), AppContainer would otherwise keep blocking Windows Tab + // approval-mode cycling for the entire streaming window. + mockedUseCommandCompletion.mockReturnValue({ + ...mockCommandCompletion, + showSuggestions: true, + suggestions: [ + { + value: '/clear', + label: '/clear', + description: 'Clear screen', + }, + ] as UseCommandCompletionReturn['suggestions'], + }); + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + expect(onTabConsumerChange).toHaveBeenLastCalledWith(true); + + unmount(); + // Last call after unmount must be false — the cleanup function fires. + expect(onTabConsumerChange).toHaveBeenLastCalledWith(false); + }); + }); + + // Regression for #4308 review: `onSuggestionsVisibilityChange` must stay + // narrow (autocomplete dropdown only). Composer uses this signal to hide + // the Footer / KeyboardShortcuts when the dropdown competes for vertical + // space. Followup suggestions and mid-input ghost text are inline within + // the input box and must NOT hide the Footer — broadening this signal + // would cause Footer churn on all platforms. + describe('onSuggestionsVisibilityChange stays narrow (autocomplete only)', () => { + const SUGGESTION_VISIBLE_WAIT_MS = 700; + + it('stays false when only a followup prompt suggestion is visible', async () => { + const onSuggestionsVisibilityChange = vi.fn(); + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(SUGGESTION_VISIBLE_WAIT_MS); + + // Tab consumer signal flips true (followup is a Tab consumer)… + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + // …but the narrow signal must NOT — Footer should stay visible. + expect(onSuggestionsVisibilityChange).not.toHaveBeenCalledWith(true); + unmount(); + }); + + it('stays false when only mid-input ghost text is present', async () => { + mockCommandCompletion.midInputGhostText = { + text: 'ile.txt', + insertPosition: 1, + acceptText: 'ile.txt', + }; + const onSuggestionsVisibilityChange = vi.fn(); + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + expect(onSuggestionsVisibilityChange).not.toHaveBeenCalledWith(true); + unmount(); + }); + + it('flips true only when the autocomplete dropdown is visible', async () => { + mockCommandCompletion.showSuggestions = true; + mockCommandCompletion.suggestions = [ + { + value: '/clear', + label: '/clear', + description: 'Clear screen', + }, + ] as UseCommandCompletionReturn['suggestions']; + const onSuggestionsVisibilityChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + expect(onSuggestionsVisibilityChange).toHaveBeenCalledWith(true); + unmount(); + }); + }); + it('should call shellHistory.getPreviousCommand on up arrow in shell mode', async () => { props.shellModeActive = true; const { stdin, unmount } = renderWithProviders(); diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 1400b16ec8a..259f0de5e28 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -87,7 +87,22 @@ export interface InputPromptProps { onEscapePromptChange?: (showPrompt: boolean) => void; onToggleShortcuts?: () => void; showShortcuts?: boolean; + /** + * Reports autocomplete-dropdown visibility specifically. Composer uses + * this to hide the Footer / KeyboardShortcuts when the dropdown would + * overlap their vertical space. Must stay narrow — followup suggestions + * and mid-input ghost text don't take Footer's space and shouldn't hide + * it. See #4171 / #4308 review. + */ onSuggestionsVisibilityChange?: (visible: boolean) => void; + /** + * Reports whether any input-area handler will consume a Tab keystroke + * (autocomplete dropdown, followup prompt suggestion, or mid-input ghost + * text). AppContainer feeds this into useAutoAcceptIndicator's + * `shouldBlockTab` to suppress the Windows-only "bare Tab cycles approval + * mode" fallback. See #4171. + */ + onTabConsumerChange?: (active: boolean) => void; vimHandleInput?: (key: Key) => boolean; isEmbeddedShellFocused?: boolean; /** Prompt suggestion text to display after response completes */ @@ -122,6 +137,7 @@ export const InputPrompt: React.FC = ({ onToggleShortcuts, showShortcuts, onSuggestionsVisibilityChange, + onTabConsumerChange, vimHandleInput, isEmbeddedShellFocused, promptSuggestion, @@ -1418,13 +1434,38 @@ export const InputPrompt: React.FC = ({ (shouldUseExportSuggestions && exportCompletion.shouldShowSuggestions) || activeCompletion.showSuggestions; - // Notify parent about suggestions visibility changes + // Whether any input-side handler would consume a Tab keystroke. AppContainer + // feeds this into useAutoAcceptIndicator's `shouldBlockTab` so the + // Windows-only "bare Tab cycles approval mode" fallback doesn't double-fire + // alongside an input-area Tab handler. See issue #4171. + // + // Note on reverse/command-search: when those overlays have matches, their + // `showSuggestions` flag flows into `shouldShowSuggestions` above and Tab IS + // consumed (ACCEPT_SUGGESTION_REVERSE_SEARCH). When they are active with no + // matches, Tab is not consumed — so the bare `reverseSearchActive` / + // `commandSearchActive` flags are intentionally NOT included here. + const hasTabConsumer = + shouldShowSuggestions || + (followup.state.isVisible && Boolean(followup.state.suggestion)) || + Boolean(completion.midInputGhostText?.acceptText); + + // Narrow signal — autocomplete dropdown only. Composer hides Footer / + // KeyboardShortcuts when this is true because the dropdown competes for + // the same vertical space. Followup / ghost text are inline within the + // input box and must NOT hide the Footer (#4308 review). useEffect(() => { - if (onSuggestionsVisibilityChange) { - onSuggestionsVisibilityChange(shouldShowSuggestions); - } + onSuggestionsVisibilityChange?.(shouldShowSuggestions); }, [shouldShowSuggestions, onSuggestionsVisibilityChange]); + // Broad signal — any Tab consumer. Reset to false on unmount (e.g. when + // InputPrompt unmounts during streaming) so AppContainer's stale + // `hasTabConsumer` doesn't keep blocking Windows Tab approval-mode cycling + // while there is no input area to consume the keystroke. + useEffect(() => { + onTabConsumerChange?.(hasTabConsumer); + return () => onTabConsumerChange?.(false); + }, [hasTabConsumer, onTabConsumerChange]); + // Trigger prompt suggestion when prop changes useEffect(() => { followup.setSuggestion(promptSuggestion ?? null); diff --git a/packages/cli/src/ui/contexts/UIActionsContext.tsx b/packages/cli/src/ui/contexts/UIActionsContext.tsx index 7ce19a9f7bb..212ab066ac9 100644 --- a/packages/cli/src/ui/contexts/UIActionsContext.tsx +++ b/packages/cli/src/ui/contexts/UIActionsContext.tsx @@ -59,7 +59,7 @@ export interface UIActions { handleFolderTrustSelect: (choice: FolderTrustChoice) => void; setConstrainHeight: (value: boolean) => void; onEscapePromptChange: (show: boolean) => void; - onSuggestionsVisibilityChange: (visible: boolean) => void; + onTabConsumerChange: (active: boolean) => void; refreshStatic: () => void; handleFinalSubmit: (value: string) => void; handleRetryLastPrompt: () => void;