diff --git a/packages/cli/src/config/keyBindings.ts b/packages/cli/src/config/keyBindings.ts index b261d7aac..dc53448d4 100644 --- a/packages/cli/src/config/keyBindings.ts +++ b/packages/cli/src/config/keyBindings.ts @@ -122,9 +122,10 @@ export const defaultKeyBindings: KeyBindingConfig = { // Auto-completion [Command.ACCEPT_SUGGESTION]: [{ key: 'tab' }, { key: 'return', ctrl: false }], - // Completion navigation (arrow or Ctrl+P/N) - [Command.COMPLETION_UP]: [{ key: 'up' }, { key: 'p', ctrl: true }], - [Command.COMPLETION_DOWN]: [{ key: 'down' }, { key: 'n', ctrl: true }], + // Completion navigation uses only arrow keys + // Ctrl+P/N are reserved for history navigation (HISTORY_UP/DOWN) + [Command.COMPLETION_UP]: [{ key: 'up' }], + [Command.COMPLETION_DOWN]: [{ key: 'down' }], // Text input // Must also exclude shift to allow shift+enter for newline diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index cf8b9685c..aefc6b413 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -278,7 +278,7 @@ describe('InputPrompt', () => { unmount(); }); - it('should call completion.navigateUp for both up arrow and Ctrl+P when suggestions are showing', async () => { + it('should call completion.navigateUp for up arrow when suggestions are showing', async () => { mockedUseCommandCompletion.mockReturnValue({ ...mockCommandCompletion, showSuggestions: true, @@ -293,19 +293,22 @@ describe('InputPrompt', () => { const { stdin, unmount } = renderWithProviders(); await wait(); - // Test up arrow + // Test up arrow for completion navigation stdin.write('\u001B[A'); // Up arrow await wait(); + expect(mockCommandCompletion.navigateUp).toHaveBeenCalledTimes(1); + expect(mockCommandCompletion.navigateDown).not.toHaveBeenCalled(); + // Ctrl+P should navigate history, not completion stdin.write('\u0010'); // Ctrl+P await wait(); - expect(mockCommandCompletion.navigateUp).toHaveBeenCalledTimes(2); - expect(mockCommandCompletion.navigateDown).not.toHaveBeenCalled(); + expect(mockCommandCompletion.navigateUp).toHaveBeenCalledTimes(1); + expect(mockInputHistory.navigateUp).toHaveBeenCalled(); unmount(); }); - it('should call completion.navigateDown for both down arrow and Ctrl+N when suggestions are showing', async () => { + it('should call completion.navigateDown for down arrow when suggestions are showing', async () => { mockedUseCommandCompletion.mockReturnValue({ ...mockCommandCompletion, showSuggestions: true, @@ -319,14 +322,17 @@ describe('InputPrompt', () => { const { stdin, unmount } = renderWithProviders(); await wait(); - // Test down arrow + // Test down arrow for completion navigation stdin.write('\u001B[B'); // Down arrow await wait(); + expect(mockCommandCompletion.navigateDown).toHaveBeenCalledTimes(1); + expect(mockCommandCompletion.navigateUp).not.toHaveBeenCalled(); + // Ctrl+N should navigate history, not completion stdin.write('\u000E'); // Ctrl+N await wait(); - expect(mockCommandCompletion.navigateDown).toHaveBeenCalledTimes(2); - expect(mockCommandCompletion.navigateUp).not.toHaveBeenCalled(); + expect(mockCommandCompletion.navigateDown).toHaveBeenCalledTimes(1); + expect(mockInputHistory.navigateDown).toHaveBeenCalled(); unmount(); }); @@ -764,6 +770,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -791,6 +799,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -818,6 +828,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -845,6 +857,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -872,6 +886,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -900,6 +916,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -927,6 +945,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -955,6 +975,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -983,6 +1005,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -1011,6 +1035,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -1039,6 +1065,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -1069,6 +1097,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -1097,6 +1127,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); @@ -1127,6 +1159,8 @@ describe('InputPrompt', () => { mockCommandContext, false, expect.any(Object), + // active parameter: completion enabled when not just navigated history + true, ); unmount(); diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 7d1742505..1370ecdc7 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -135,6 +135,8 @@ export const InputPrompt: React.FC = ({ commandContext, reverseSearchActive, config, + // Suppress completion when history navigation just occurred + !justNavigatedHistory, ); const reverseSearchCompletion = useReverseSearchCompletion( @@ -219,9 +221,9 @@ export const InputPrompt: React.FC = ({ const inputHistory = useInputHistory({ userMessages, onSubmit: handleSubmitAndClear, - isActive: - (!completion.showSuggestions || completion.suggestions.length === 1) && - !shellModeActive, + // History navigation (Ctrl+P/N) now always works since completion navigation + // only uses arrow keys. Only disable in shell mode. + isActive: !shellModeActive, currentQuery: buffer.text, onChange: customSetTextAndResetCompletionSignal, }); diff --git a/packages/cli/src/ui/hooks/useCommandCompletion.tsx b/packages/cli/src/ui/hooks/useCommandCompletion.tsx index 3deaa8a53..cb5d9f276 100644 --- a/packages/cli/src/ui/hooks/useCommandCompletion.tsx +++ b/packages/cli/src/ui/hooks/useCommandCompletion.tsx @@ -45,6 +45,8 @@ export function useCommandCompletion( commandContext: CommandContext, reverseSearchActive: boolean = false, config?: Config, + // When false, suppresses showing suggestions (e.g., after history navigation) + active: boolean = true, ): UseCommandCompletionReturn { const { suggestions, @@ -152,7 +154,11 @@ export function useCommandCompletion( }, [suggestions, setActiveSuggestionIndex, setVisibleStartIndex]); useEffect(() => { - if (completionMode === CompletionMode.IDLE || reverseSearchActive) { + if ( + completionMode === CompletionMode.IDLE || + reverseSearchActive || + !active + ) { resetCompletionState(); return; } @@ -163,6 +169,7 @@ export function useCommandCompletion( suggestions.length, isLoadingSuggestions, reverseSearchActive, + active, resetCompletionState, setShowSuggestions, ]); diff --git a/packages/cli/src/ui/keyMatchers.test.ts b/packages/cli/src/ui/keyMatchers.test.ts index 690a5cee4..a0a9b8279 100644 --- a/packages/cli/src/ui/keyMatchers.test.ts +++ b/packages/cli/src/ui/keyMatchers.test.ts @@ -38,10 +38,10 @@ describe('keyMatchers', () => { [Command.NAVIGATION_DOWN]: (key: Key) => key.name === 'down', [Command.ACCEPT_SUGGESTION]: (key: Key) => key.name === 'tab' || (key.name === 'return' && !key.ctrl), - [Command.COMPLETION_UP]: (key: Key) => - key.name === 'up' || (key.ctrl && key.name === 'p'), - [Command.COMPLETION_DOWN]: (key: Key) => - key.name === 'down' || (key.ctrl && key.name === 'n'), + // Completion navigation only uses arrow keys (not Ctrl+P/N) + // to allow Ctrl+P/N to always navigate history + [Command.COMPLETION_UP]: (key: Key) => key.name === 'up', + [Command.COMPLETION_DOWN]: (key: Key) => key.name === 'down', [Command.ESCAPE]: (key: Key) => key.name === 'escape', [Command.SUBMIT]: (key: Key) => key.name === 'return' && !key.ctrl && !key.meta && !key.paste, @@ -164,14 +164,26 @@ describe('keyMatchers', () => { negative: [createKey('return', { ctrl: true }), createKey('space')], }, { + // Completion navigation only uses arrow keys (not Ctrl+P/N) + // to allow Ctrl+P/N to always navigate history command: Command.COMPLETION_UP, - positive: [createKey('up'), createKey('p', { ctrl: true })], - negative: [createKey('p'), createKey('down')], + positive: [createKey('up')], + negative: [ + createKey('p'), + createKey('down'), + createKey('p', { ctrl: true }), + ], }, { + // Completion navigation only uses arrow keys (not Ctrl+P/N) + // to allow Ctrl+P/N to always navigate history command: Command.COMPLETION_DOWN, - positive: [createKey('down'), createKey('n', { ctrl: true })], - negative: [createKey('n'), createKey('up')], + positive: [createKey('down')], + negative: [ + createKey('n'), + createKey('up'), + createKey('n', { ctrl: true }), + ], }, // Text input