From 958cc4593787b625f26a83924c44d1906fe2245f Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Thu, 22 Jan 2026 12:54:23 -0800 Subject: [PATCH] Fix so rewind starts at the bottom and loadHistory refreshes static content. (#17335) --- .../src/ui/components/RewindViewer.test.tsx | 2 + .../cli/src/ui/components/RewindViewer.tsx | 2 + .../__snapshots__/RewindViewer.test.tsx.snap | 86 +++++----- .../shared/BaseSelectionList.test.tsx | 1 + .../components/shared/BaseSelectionList.tsx | 3 + .../ui/hooks/slashCommandProcessor.test.tsx | 153 +++++++++++++----- .../cli/src/ui/hooks/slashCommandProcessor.ts | 1 + .../src/ui/hooks/useSelectionList.test.tsx | 34 ++++ packages/cli/src/ui/hooks/useSelectionList.ts | 68 ++++++-- .../cli/src/ui/hooks/useSessionResume.test.ts | 5 +- 10 files changed, 259 insertions(+), 96 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.test.tsx b/packages/cli/src/ui/components/RewindViewer.test.tsx index 8272fc9c9f..cdb0650408 100644 --- a/packages/cli/src/ui/components/RewindViewer.test.tsx +++ b/packages/cli/src/ui/components/RewindViewer.test.tsx @@ -239,6 +239,7 @@ describe('RewindViewer', () => { // Select act(() => { + stdin.write('\x1b[A'); // Move up from 'Stay at current position' stdin.write('\r'); }); expect(lastFrame()).toMatchSnapshot('confirmation-dialog'); @@ -280,6 +281,7 @@ describe('RewindViewer', () => { // Select act(() => { + stdin.write('\x1b[A'); // Move up from 'Stay at current position' stdin.write('\r'); // Select }); diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 956d94ac91..38c026f3d1 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -188,8 +188,10 @@ export const RewindViewer: React.FC = ({ { const userPrompt = item; if (userPrompt && userPrompt.id) { diff --git a/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap index 64bb27dba3..d5acee4a7b 100644 --- a/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap @@ -5,10 +5,10 @@ exports[`RewindViewer > Content Filtering > 'removes reference markers' 1`] = ` │ │ │ > Rewind │ │ │ -│ ● some command @file │ +│ some command @file │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -22,10 +22,10 @@ exports[`RewindViewer > Content Filtering > 'strips expanded MCP resource conten │ │ │ > Rewind │ │ │ -│ ● read @server3:mcp://demo-resource hello │ +│ read @server3:mcp://demo-resource hello │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -72,13 +72,13 @@ exports[`RewindViewer > Navigation > handles 'down' navigation > after-down 1`] │ Q1 │ │ No files have been changed │ │ │ -│ ● Q2 │ +│ Q2 │ │ No files have been changed │ │ │ │ Q3 │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -98,30 +98,7 @@ exports[`RewindViewer > Navigation > handles 'up' navigation > after-up 1`] = ` │ Q2 │ │ No files have been changed │ │ │ -│ Q3 │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" -`; - -exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-down 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ ● Q1 │ -│ No files have been changed │ -│ │ -│ Q2 │ -│ No files have been changed │ -│ │ -│ Q3 │ +│ ● Q3 │ │ No files have been changed │ │ │ │ Stay at current position │ @@ -133,7 +110,7 @@ exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-down 1`] ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" `; -exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-up 1`] = ` +exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-down 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Rewind │ @@ -156,15 +133,38 @@ exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-up 1`] = ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" `; +exports[`RewindViewer > Navigation > handles cyclic navigation > cyclic-up 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Q1 │ +│ No files have been changed │ +│ │ +│ Q2 │ +│ No files have been changed │ +│ │ +│ ● Q3 │ +│ No files have been changed │ +│ │ +│ Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯" +`; + exports[`RewindViewer > Rendering > renders 'a single interaction' 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ │ > Rewind │ │ │ -│ ● Hello │ +│ Hello │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -178,11 +178,11 @@ exports[`RewindViewer > Rendering > renders 'full text for selected item' 1`] = │ │ │ > Rewind │ │ │ -│ ● 1 │ +│ 1 │ │ 2... │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -210,13 +210,13 @@ exports[`RewindViewer > updates content when conversation changes (background up │ │ │ > Rewind │ │ │ -│ ● Message 1 │ +│ Message 1 │ │ No files have been changed │ │ │ │ Message 2 │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -230,10 +230,10 @@ exports[`RewindViewer > updates content when conversation changes (background up │ │ │ > Rewind │ │ │ -│ ● Message 1 │ +│ Message 1 │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -251,11 +251,11 @@ exports[`RewindViewer > updates selection and expansion on navigation > after-do │ Line B... │ │ No files have been changed │ │ │ -│ ● Line 1 │ +│ Line 1 │ │ Line 2... │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ @@ -269,7 +269,7 @@ exports[`RewindViewer > updates selection and expansion on navigation > initial- │ │ │ > Rewind │ │ │ -│ ● Line A │ +│ Line A │ │ Line B... │ │ No files have been changed │ │ │ @@ -277,7 +277,7 @@ exports[`RewindViewer > updates selection and expansion on navigation > initial- │ Line 2... │ │ No files have been changed │ │ │ -│ Stay at current position │ +│ ● Stay at current position │ │ Cancel rewind and stay here │ │ │ │ │ diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx index 708de9f10a..0d1eb43f6c 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.test.tsx @@ -125,6 +125,7 @@ describe('BaseSelectionList', () => { onHighlight: mockOnHighlight, isFocused, showNumbers, + wrapAround: true, }); }); diff --git a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx index 8071582f75..2f2e36457a 100644 --- a/packages/cli/src/ui/components/shared/BaseSelectionList.tsx +++ b/packages/cli/src/ui/components/shared/BaseSelectionList.tsx @@ -30,6 +30,7 @@ export interface BaseSelectionListProps< showNumbers?: boolean; showScrollArrows?: boolean; maxItemsToShow?: number; + wrapAround?: boolean; renderItem: (item: TItem, context: RenderItemContext) => React.ReactNode; } @@ -59,6 +60,7 @@ export function BaseSelectionList< showNumbers = true, showScrollArrows = false, maxItemsToShow = 10, + wrapAround = true, renderItem, }: BaseSelectionListProps): React.JSX.Element { const { activeIndex } = useSelectionList({ @@ -68,6 +70,7 @@ export function BaseSelectionList< onHighlight, isFocused, showNumbers, + wrapAround, }); const [scrollOffset, setScrollOffset] = useState(0); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx index 717da57805..05b3366561 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx @@ -156,11 +156,22 @@ describe('useSlashCommandProcessor', () => { }); const setupProcessorHook = async ( - builtinCommands: SlashCommand[] = [], - fileCommands: SlashCommand[] = [], - mcpCommands: SlashCommand[] = [], - setIsProcessing = vi.fn(), + options: { + builtinCommands?: SlashCommand[]; + fileCommands?: SlashCommand[]; + mcpCommands?: SlashCommand[]; + setIsProcessing?: (isProcessing: boolean) => void; + refreshStatic?: () => void; + } = {}, ) => { + const { + builtinCommands = [], + fileCommands = [], + mcpCommands = [], + setIsProcessing = vi.fn(), + refreshStatic = vi.fn(), + } = options; + mockBuiltinLoadCommands.mockResolvedValue(Object.freeze(builtinCommands)); mockFileLoadCommands.mockResolvedValue(Object.freeze(fileCommands)); mockMcpLoadCommands.mockResolvedValue(Object.freeze(mcpCommands)); @@ -177,7 +188,7 @@ describe('useSlashCommandProcessor', () => { mockAddItem, mockClearItems, mockLoadHistory, - vi.fn(), // refreshStatic + refreshStatic, vi.fn(), // toggleVimEnabled setIsProcessing, { @@ -234,7 +245,9 @@ describe('useSlashCommandProcessor', () => { context.ui.clear(); }, }); - const result = await setupProcessorHook([clearCommand]); + const result = await setupProcessorHook({ + builtinCommands: [clearCommand], + }); await act(async () => { await result.current.handleSlashCommand('/clear'); @@ -251,7 +264,9 @@ describe('useSlashCommandProcessor', () => { context.ui.clear(); }, }); - const result = await setupProcessorHook([clearCommand]); + const result = await setupProcessorHook({ + builtinCommands: [clearCommand], + }); await act(async () => { await result.current.handleSlashCommand('/clear'); @@ -271,7 +286,9 @@ describe('useSlashCommandProcessor', () => { it('should call loadCommands and populate state after mounting', async () => { const testCommand = createTestCommand({ name: 'test' }); - const result = await setupProcessorHook([testCommand]); + const result = await setupProcessorHook({ + builtinCommands: [testCommand], + }); await waitFor(() => { expect(result.current.slashCommands).toHaveLength(1); @@ -285,7 +302,9 @@ describe('useSlashCommandProcessor', () => { it('should provide an immutable array of commands to consumers', async () => { const testCommand = createTestCommand({ name: 'test' }); - const result = await setupProcessorHook([testCommand]); + const result = await setupProcessorHook({ + builtinCommands: [testCommand], + }); await waitFor(() => { expect(result.current.slashCommands).toHaveLength(1); @@ -313,7 +332,10 @@ describe('useSlashCommandProcessor', () => { CommandKind.FILE, ); - const result = await setupProcessorHook([builtinCommand], [fileCommand]); + const result = await setupProcessorHook({ + builtinCommands: [builtinCommand], + fileCommands: [fileCommand], + }); await waitFor(() => { // The service should only return one command with the name 'override' @@ -363,7 +385,9 @@ describe('useSlashCommandProcessor', () => { }, ], }; - const result = await setupProcessorHook([parentCommand]); + const result = await setupProcessorHook({ + builtinCommands: [parentCommand], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -397,7 +421,9 @@ describe('useSlashCommandProcessor', () => { }, ], }; - const result = await setupProcessorHook([parentCommand]); + const result = await setupProcessorHook({ + builtinCommands: [parentCommand], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -421,7 +447,9 @@ describe('useSlashCommandProcessor', () => { it('sets isProcessing to false if the the input is not a command', async () => { const setMockIsProcessing = vi.fn(); - const result = await setupProcessorHook([], [], [], setMockIsProcessing); + const result = await setupProcessorHook({ + setIsProcessing: setMockIsProcessing, + }); await act(async () => { await result.current.handleSlashCommand('imnotacommand'); @@ -437,12 +465,10 @@ describe('useSlashCommandProcessor', () => { action: vi.fn().mockRejectedValue(new Error('oh no!')), }); - const result = await setupProcessorHook( - [failCommand], - [], - [], - setMockIsProcessing, - ); + const result = await setupProcessorHook({ + builtinCommands: [failCommand], + setIsProcessing: setMockIsProcessing, + }); await waitFor(() => expect(result.current.slashCommands).toBeDefined()); @@ -461,12 +487,10 @@ describe('useSlashCommandProcessor', () => { action: () => new Promise((resolve) => setTimeout(resolve, 50)), }); - const result = await setupProcessorHook( - [command], - [], - [], - mockSetIsProcessing, - ); + const result = await setupProcessorHook({ + builtinCommands: [command], + setIsProcessing: mockSetIsProcessing, + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); const executionPromise = act(async () => { @@ -508,7 +532,9 @@ describe('useSlashCommandProcessor', () => { .fn() .mockResolvedValue({ type: 'dialog', dialog: dialogType }), }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1), ); @@ -537,20 +563,42 @@ describe('useSlashCommandProcessor', () => { clientHistory: [{ role: 'user', parts: [{ text: 'old prompt' }] }], }), }); - const result = await setupProcessorHook([command]); + + const mockRefreshStatic = vi.fn(); + const result = await setupProcessorHook({ + builtinCommands: [command], + refreshStatic: mockRefreshStatic, + }); + await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { await result.current.handleSlashCommand('/load'); }); + // ui.clear() is called which calls refreshStatic() expect(mockClearItems).toHaveBeenCalledTimes(1); + expect(mockRefreshStatic).toHaveBeenCalledTimes(1); expect(mockAddItem).toHaveBeenCalledWith( { type: 'user', text: 'old prompt' }, expect.any(Number), ); }); + it('should call refreshStatic exactly once when ui.loadHistory is called', async () => { + const mockRefreshStatic = vi.fn(); + const result = await setupProcessorHook({ + refreshStatic: mockRefreshStatic, + }); + + await act(async () => { + result.current.commandContext.ui.loadHistory([]); + }); + + expect(mockLoadHistory).toHaveBeenCalled(); + expect(mockRefreshStatic).toHaveBeenCalledTimes(1); + }); + it('should handle a "quit" action', async () => { const quitAction = vi .fn() @@ -559,7 +607,9 @@ describe('useSlashCommandProcessor', () => { name: 'exit', action: quitAction, }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); @@ -582,7 +632,9 @@ describe('useSlashCommandProcessor', () => { CommandKind.FILE, ); - const result = await setupProcessorHook([], [fileCommand]); + const result = await setupProcessorHook({ + fileCommands: [fileCommand], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); let actionResult; @@ -614,7 +666,9 @@ describe('useSlashCommandProcessor', () => { CommandKind.MCP_PROMPT, ); - const result = await setupProcessorHook([], [], [mcpCommand]); + const result = await setupProcessorHook({ + mcpCommands: [mcpCommand], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); let actionResult; @@ -637,7 +691,9 @@ describe('useSlashCommandProcessor', () => { describe('Command Parsing and Matching', () => { it('should be case-sensitive', async () => { const command = createTestCommand({ name: 'test' }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -663,7 +719,9 @@ describe('useSlashCommandProcessor', () => { description: 'a command with an alias', action, }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -679,7 +737,9 @@ describe('useSlashCommandProcessor', () => { it('should handle extra whitespace around the command', async () => { const action = vi.fn(); const command = createTestCommand({ name: 'test', action }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -692,7 +752,9 @@ describe('useSlashCommandProcessor', () => { it('should handle `?` as a command prefix', async () => { const action = vi.fn(); const command = createTestCommand({ name: 'help', action }); - const result = await setupProcessorHook([command]); + const result = await setupProcessorHook({ + builtinCommands: [command], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); await act(async () => { @@ -721,7 +783,10 @@ describe('useSlashCommandProcessor', () => { CommandKind.FILE, ); - const result = await setupProcessorHook([], [fileCommand], [mcpCommand]); + const result = await setupProcessorHook({ + fileCommands: [fileCommand], + mcpCommands: [mcpCommand], + }); await waitFor(() => { // The service should only return one command with the name 'override' @@ -757,7 +822,10 @@ describe('useSlashCommandProcessor', () => { // The order of commands in the final loaded array is not guaranteed, // so the test must work regardless of which comes first. - const result = await setupProcessorHook([quitCommand], [exitCommand]); + const result = await setupProcessorHook({ + builtinCommands: [quitCommand], + fileCommands: [exitCommand], + }); await waitFor(() => { expect(result.current.slashCommands).toHaveLength(2); @@ -784,7 +852,10 @@ describe('useSlashCommandProcessor', () => { CommandKind.FILE, ); - const result = await setupProcessorHook([quitCommand], [exitCommand]); + const result = await setupProcessorHook({ + builtinCommands: [quitCommand], + fileCommands: [exitCommand], + }); await waitFor(() => expect(result.current.slashCommands).toHaveLength(2)); await act(async () => { @@ -880,7 +951,9 @@ describe('useSlashCommandProcessor', () => { desc: 'command path when alias is used', }, ])('should log $desc', async ({ command, expectedLog }) => { - const result = await setupProcessorHook(loggingTestCommands); + const result = await setupProcessorHook({ + builtinCommands: loggingTestCommands, + }); await waitFor(() => expect(result.current.slashCommands).toBeDefined()); await act(async () => { @@ -899,7 +972,9 @@ describe('useSlashCommandProcessor', () => { { command: '/bogusbogusbogus', desc: 'bogus command' }, { command: '/unknown', desc: 'unknown command' }, ])('should not log for $desc', async ({ command }) => { - const result = await setupProcessorHook(loggingTestCommands); + const result = await setupProcessorHook({ + builtinCommands: loggingTestCommands, + }); await waitFor(() => expect(result.current.slashCommands).toBeDefined()); await act(async () => { diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index c2bb7ebcee..f336579284 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -213,6 +213,7 @@ export const useSlashCommandProcessor = ( }, loadHistory: (history, postLoadInput) => { loadHistory(history); + refreshStatic(); if (postLoadInput !== undefined) { actions.setText(postLoadInput); } diff --git a/packages/cli/src/ui/hooks/useSelectionList.test.tsx b/packages/cli/src/ui/hooks/useSelectionList.test.tsx index 7006f0f5d3..231407dce9 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.test.tsx +++ b/packages/cli/src/ui/hooks/useSelectionList.test.tsx @@ -79,6 +79,7 @@ describe('useSelectionList', () => { initialIndex?: number; isFocused?: boolean; showNumbers?: boolean; + wrapAround?: boolean; }) => { let hookResult: ReturnType; function TestComponent(props: typeof initialProps) { @@ -285,6 +286,39 @@ describe('useSelectionList', () => { }); }); + describe('Wrapping (wrapAround)', () => { + it('should wrap by default (wrapAround=true)', async () => { + const { result } = await renderSelectionListHook({ + items, + initialIndex: items.length - 1, + onSelect: mockOnSelect, + }); + expect(result.current.activeIndex).toBe(3); + pressKey('down'); + expect(result.current.activeIndex).toBe(0); + + pressKey('up'); + expect(result.current.activeIndex).toBe(3); + }); + + it('should not wrap when wrapAround is false', async () => { + const { result } = await renderSelectionListHook({ + items, + initialIndex: items.length - 1, + onSelect: mockOnSelect, + wrapAround: false, + }); + expect(result.current.activeIndex).toBe(3); + pressKey('down'); + expect(result.current.activeIndex).toBe(3); // Should stay at bottom + + act(() => result.current.setActiveIndex(0)); + expect(result.current.activeIndex).toBe(0); + pressKey('up'); + expect(result.current.activeIndex).toBe(0); // Should stay at top + }); + }); + describe('Selection (Enter)', () => { it('should call onSelect when "return" is pressed on enabled item', async () => { await renderSelectionListHook({ diff --git a/packages/cli/src/ui/hooks/useSelectionList.ts b/packages/cli/src/ui/hooks/useSelectionList.ts index 11ce449f11..dea4015969 100644 --- a/packages/cli/src/ui/hooks/useSelectionList.ts +++ b/packages/cli/src/ui/hooks/useSelectionList.ts @@ -27,6 +27,7 @@ export interface UseSelectionListOptions { onHighlight?: (value: T) => void; isFocused?: boolean; showNumbers?: boolean; + wrapAround?: boolean; } export interface UseSelectionListResult { @@ -40,6 +41,7 @@ interface SelectionListState { pendingHighlight: boolean; pendingSelect: boolean; items: BaseSelectionItem[]; + wrapAround: boolean; } type SelectionListAction = @@ -60,7 +62,11 @@ type SelectionListAction = } | { type: 'INITIALIZE'; - payload: { initialIndex: number; items: BaseSelectionItem[] }; + payload: { + initialIndex: number; + items: BaseSelectionItem[]; + wrapAround: boolean; + }; } | { type: 'CLEAR_PENDING_FLAGS'; @@ -75,6 +81,7 @@ const findNextValidIndex = ( currentIndex: number, direction: 'up' | 'down', items: BaseSelectionItem[], + wrapAround = true, ): number => { const len = items.length; if (len === 0) return currentIndex; @@ -83,13 +90,34 @@ const findNextValidIndex = ( const step = direction === 'down' ? 1 : -1; for (let i = 0; i < len; i++) { - // Calculate the next index, wrapping around if necessary. - // We add `len` before the modulo to ensure a positive result in JS for negative steps. - nextIndex = (nextIndex + step + len) % len; + const candidateIndex = nextIndex + step; + + if (wrapAround) { + // Calculate the next index, wrapping around if necessary. + // We add `len` before the modulo to ensure a positive result in JS for negative steps. + nextIndex = (candidateIndex + len) % len; + } else { + if (candidateIndex < 0 || candidateIndex >= len) { + // Out of bounds and wrapping is disabled + return currentIndex; + } + nextIndex = candidateIndex; + } if (!items[nextIndex]?.disabled) { return nextIndex; } + + if (!wrapAround) { + // If the item is disabled and we're not wrapping, we continue searching + // in the same direction, but we must stop if we hit the bounds. + if ( + (direction === 'down' && nextIndex === len - 1) || + (direction === 'up' && nextIndex === 0) + ) { + return currentIndex; + } + } } // If all items are disabled, return the original index @@ -120,7 +148,7 @@ const computeInitialIndex = ( } if (items[targetIndex]?.disabled) { - const nextValid = findNextValidIndex(targetIndex, 'down', items); + const nextValid = findNextValidIndex(targetIndex, 'down', items, true); targetIndex = nextValid; } @@ -148,8 +176,13 @@ function selectionListReducer( } case 'MOVE_UP': { - const { items } = state; - const newIndex = findNextValidIndex(state.activeIndex, 'up', items); + const { items, wrapAround } = state; + const newIndex = findNextValidIndex( + state.activeIndex, + 'up', + items, + wrapAround, + ); if (newIndex !== state.activeIndex) { return { ...state, activeIndex: newIndex, pendingHighlight: true }; } @@ -157,8 +190,13 @@ function selectionListReducer( } case 'MOVE_DOWN': { - const { items } = state; - const newIndex = findNextValidIndex(state.activeIndex, 'down', items); + const { items, wrapAround } = state; + const newIndex = findNextValidIndex( + state.activeIndex, + 'down', + items, + wrapAround, + ); if (newIndex !== state.activeIndex) { return { ...state, activeIndex: newIndex, pendingHighlight: true }; } @@ -170,7 +208,7 @@ function selectionListReducer( } case 'INITIALIZE': { - const { initialIndex, items } = action.payload; + const { initialIndex, items, wrapAround } = action.payload; const activeKey = initialIndex === state.initialIndex && state.activeIndex !== state.initialIndex @@ -186,6 +224,7 @@ function selectionListReducer( initialIndex, activeIndex: targetIndex, pendingHighlight: false, + wrapAround, }; } @@ -245,6 +284,7 @@ export function useSelectionList({ onHighlight, isFocused = true, showNumbers = false, + wrapAround = true, }: UseSelectionListOptions): UseSelectionListResult { const baseItems = toBaseItems(items); @@ -254,12 +294,14 @@ export function useSelectionList({ pendingHighlight: false, pendingSelect: false, items: baseItems, + wrapAround, }); const numberInputRef = useRef(''); const numberInputTimer = useRef(null); const prevBaseItemsRef = useRef(baseItems); const prevInitialIndexRef = useRef(initialIndex); + const prevWrapAroundRef = useRef(wrapAround); // Initialize/synchronize state when initialIndex or items change useEffect(() => { @@ -268,14 +310,16 @@ export function useSelectionList({ baseItems, ); const initialIndexChanged = prevInitialIndexRef.current !== initialIndex; + const wrapAroundChanged = prevWrapAroundRef.current !== wrapAround; - if (baseItemsChanged || initialIndexChanged) { + if (baseItemsChanged || initialIndexChanged || wrapAroundChanged) { dispatch({ type: 'INITIALIZE', - payload: { initialIndex, items: baseItems }, + payload: { initialIndex, items: baseItems, wrapAround }, }); prevBaseItemsRef.current = baseItems; prevInitialIndexRef.current = initialIndex; + prevWrapAroundRef.current = wrapAround; } }); diff --git a/packages/cli/src/ui/hooks/useSessionResume.test.ts b/packages/cli/src/ui/hooks/useSessionResume.test.ts index e135006471..029d23d725 100644 --- a/packages/cli/src/ui/hooks/useSessionResume.test.ts +++ b/packages/cli/src/ui/hooks/useSessionResume.test.ts @@ -109,7 +109,7 @@ describe('useSessionResume', () => { 1, true, ); - expect(mockRefreshStatic).toHaveBeenCalled(); + expect(mockRefreshStatic).toHaveBeenCalledTimes(1); expect(mockGeminiClient.resumeChat).toHaveBeenCalledWith( clientHistory, resumedData, @@ -174,7 +174,7 @@ describe('useSessionResume', () => { expect(mockHistoryManager.clearItems).toHaveBeenCalled(); expect(mockHistoryManager.addItem).not.toHaveBeenCalled(); - expect(mockRefreshStatic).toHaveBeenCalled(); + expect(mockRefreshStatic).toHaveBeenCalledTimes(1); expect(mockGeminiClient.resumeChat).toHaveBeenCalledWith([], resumedData); }); }); @@ -338,6 +338,7 @@ describe('useSessionResume', () => { 1, true, ); + expect(mockRefreshStatic).toHaveBeenCalledTimes(1); expect(mockGeminiClient.resumeChat).toHaveBeenCalled(); });