diff --git a/packages/cli/src/ui/components/shared/text-buffer.test.ts b/packages/cli/src/ui/components/shared/text-buffer.test.ts index ac828959f7..5ff088a3f0 100644 --- a/packages/cli/src/ui/components/shared/text-buffer.test.ts +++ b/packages/cli/src/ui/components/shared/text-buffer.test.ts @@ -223,44 +223,49 @@ describe('textBufferReducer', () => { }); describe('delete_word_left action', () => { - it('should delete a simple word', () => { - const stateWithText: TextBufferState = { - ...initialState, - lines: ['hello world'], - cursorRow: 0, + const createSingleLineState = ( + text: string, + col: number, + ): TextBufferState => ({ + ...initialState, + lines: [text], + cursorRow: 0, + cursorCol: col, + }); + + it.each([ + { + input: 'hello world', cursorCol: 11, - }; - const action: TextBufferAction = { type: 'delete_word_left' }; - const state = textBufferReducer(stateWithText, action); - expect(state.lines).toEqual(['hello ']); - expect(state.cursorCol).toBe(6); - }); - - it('should delete a path segment', () => { - const stateWithText: TextBufferState = { - ...initialState, - lines: ['path/to/file'], - cursorRow: 0, + expectedLines: ['hello '], + expectedCol: 6, + desc: 'simple word', + }, + { + input: 'path/to/file', cursorCol: 12, - }; - const action: TextBufferAction = { type: 'delete_word_left' }; - const state = textBufferReducer(stateWithText, action); - expect(state.lines).toEqual(['path/to/']); - expect(state.cursorCol).toBe(8); - }); - - it('should delete variable_name parts', () => { - const stateWithText: TextBufferState = { - ...initialState, - lines: ['variable_name'], - cursorRow: 0, + expectedLines: ['path/to/'], + expectedCol: 8, + desc: 'path segment', + }, + { + input: 'variable_name', cursorCol: 13, - }; - const action: TextBufferAction = { type: 'delete_word_left' }; - const state = textBufferReducer(stateWithText, action); - expect(state.lines).toEqual(['variable_']); - expect(state.cursorCol).toBe(9); - }); + expectedLines: ['variable_'], + expectedCol: 9, + desc: 'variable_name parts', + }, + ])( + 'should delete $desc', + ({ input, cursorCol, expectedLines, expectedCol }) => { + const state = textBufferReducer( + createSingleLineState(input, cursorCol), + { type: 'delete_word_left' }, + ); + expect(state.lines).toEqual(expectedLines); + expect(state.cursorCol).toBe(expectedCol); + }, + ); it('should act like backspace at the beginning of a line', () => { const stateWithText: TextBufferState = { @@ -269,8 +274,9 @@ describe('textBufferReducer', () => { cursorRow: 1, cursorCol: 0, }; - const action: TextBufferAction = { type: 'delete_word_left' }; - const state = textBufferReducer(stateWithText, action); + const state = textBufferReducer(stateWithText, { + type: 'delete_word_left', + }); expect(state.lines).toEqual(['helloworld']); expect(state.cursorRow).toBe(0); expect(state.cursorCol).toBe(5); @@ -278,46 +284,58 @@ describe('textBufferReducer', () => { }); describe('delete_word_right action', () => { - it('should delete a simple word', () => { - const stateWithText: TextBufferState = { - ...initialState, - lines: ['hello world'], - cursorRow: 0, - cursorCol: 0, - }; - const action: TextBufferAction = { type: 'delete_word_right' }; - const state = textBufferReducer(stateWithText, action); - expect(state.lines).toEqual(['world']); - expect(state.cursorCol).toBe(0); + const createSingleLineState = ( + text: string, + col: number, + ): TextBufferState => ({ + ...initialState, + lines: [text], + cursorRow: 0, + cursorCol: col, }); - it('should delete a path segment', () => { + it.each([ + { + input: 'hello world', + cursorCol: 0, + expectedLines: ['world'], + expectedCol: 0, + desc: 'simple word', + }, + { + input: 'variable_name', + cursorCol: 0, + expectedLines: ['_name'], + expectedCol: 0, + desc: 'variable_name parts', + }, + ])( + 'should delete $desc', + ({ input, cursorCol, expectedLines, expectedCol }) => { + const state = textBufferReducer( + createSingleLineState(input, cursorCol), + { type: 'delete_word_right' }, + ); + expect(state.lines).toEqual(expectedLines); + expect(state.cursorCol).toBe(expectedCol); + }, + ); + + it('should delete path segments progressively', () => { const stateWithText: TextBufferState = { ...initialState, lines: ['path/to/file'], cursorRow: 0, cursorCol: 0, }; - const action: TextBufferAction = { type: 'delete_word_right' }; - let state = textBufferReducer(stateWithText, action); + let state = textBufferReducer(stateWithText, { + type: 'delete_word_right', + }); expect(state.lines).toEqual(['/to/file']); - state = textBufferReducer(state, action); + state = textBufferReducer(state, { type: 'delete_word_right' }); expect(state.lines).toEqual(['to/file']); }); - it('should delete variable_name parts', () => { - const stateWithText: TextBufferState = { - ...initialState, - lines: ['variable_name'], - cursorRow: 0, - cursorCol: 0, - }; - const action: TextBufferAction = { type: 'delete_word_right' }; - const state = textBufferReducer(stateWithText, action); - expect(state.lines).toEqual(['_name']); - expect(state.cursorCol).toBe(0); - }); - it('should act like delete at the end of a line', () => { const stateWithText: TextBufferState = { ...initialState, @@ -325,8 +343,9 @@ describe('textBufferReducer', () => { cursorRow: 0, cursorCol: 5, }; - const action: TextBufferAction = { type: 'delete_word_right' }; - const state = textBufferReducer(stateWithText, action); + const state = textBufferReducer(stateWithText, { + type: 'delete_word_right', + }); expect(state.lines).toEqual(['helloworld']); expect(state.cursorRow).toBe(0); expect(state.cursorCol).toBe(5); @@ -334,7 +353,6 @@ describe('textBufferReducer', () => { }); }); -// Helper to get the state from the hook const getBufferState = (result: { current: TextBuffer }) => { expect(result.current).toHaveOnlyValidCharacters(); return { @@ -1386,58 +1404,42 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); describe('Input Sanitization', () => { - it('should strip ANSI escape codes from input', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); - const textWithAnsi = '\x1B[31mHello\x1B[0m \x1B[32mWorld\x1B[0m'; - act(() => - result.current.handleInput({ - name: '', - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: textWithAnsi, - }), - ); - expect(getBufferState(result).text).toBe('Hello World'); + const createInput = (sequence: string) => ({ + name: '', + ctrl: false, + meta: false, + shift: false, + paste: false, + sequence, }); - it('should strip control characters from input', () => { + it.each([ + { + input: '\x1B[31mHello\x1B[0m \x1B[32mWorld\x1B[0m', + expected: 'Hello World', + desc: 'ANSI escape codes', + }, + { + input: 'H\x07e\x08l\x0Bl\x0Co', + expected: 'Hello', + desc: 'control characters', + }, + { + input: '\u001B[4mH\u001B[0mello', + expected: 'Hello', + desc: 'mixed ANSI and control characters', + }, + { + input: '\u001B[4mPasted\u001B[4m Text', + expected: 'Pasted Text', + desc: 'pasted text with ANSI', + }, + ])('should strip $desc from input', ({ input, expected }) => { const { result } = renderHook(() => useTextBuffer({ viewport, isValidPath: () => false }), ); - const textWithControlChars = 'H\x07e\x08l\x0Bl\x0Co'; // BELL, BACKSPACE, VT, FF - act(() => - result.current.handleInput({ - name: '', - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: textWithControlChars, - }), - ); - expect(getBufferState(result).text).toBe('Hello'); - }); - - it('should strip mixed ANSI and control characters from input', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); - const textWithMixed = '\u001B[4mH\u001B[0mello'; - act(() => - result.current.handleInput({ - name: '', - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: textWithMixed, - }), - ); - expect(getBufferState(result).text).toBe('Hello'); + act(() => result.current.handleInput(createInput(input))); + expect(getBufferState(result).text).toBe(expected); }); it('should not strip standard characters or newlines', () => { @@ -1445,37 +1447,10 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots useTextBuffer({ viewport, isValidPath: () => false }), ); const validText = 'Hello World\nThis is a test.'; - act(() => - result.current.handleInput({ - name: '', - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: validText, - }), - ); + act(() => result.current.handleInput(createInput(validText))); expect(getBufferState(result).text).toBe(validText); }); - it('should sanitize pasted text via handleInput', () => { - const { result } = renderHook(() => - useTextBuffer({ viewport, isValidPath: () => false }), - ); - const pastedText = '\u001B[4mPasted\u001B[4m Text'; - act(() => - result.current.handleInput({ - name: '', - ctrl: false, - meta: false, - shift: false, - paste: false, - sequence: pastedText, - }), - ); - expect(getBufferState(result).text).toBe('Pasted Text'); - }); - it('should sanitize large text (>5000 chars) and strip unsafe characters', () => { const { result } = renderHook(() => useTextBuffer({ viewport, isValidPath: () => false }), @@ -1765,98 +1740,161 @@ Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots }); describe('offsetToLogicalPos', () => { - it('should return [0,0] for offset 0', () => { - expect(offsetToLogicalPos('any text', 0)).toEqual([0, 0]); + it.each([ + { text: 'any text', offset: 0, expected: [0, 0], desc: 'offset 0' }, + { text: 'hello', offset: 0, expected: [0, 0], desc: 'single line start' }, + { text: 'hello', offset: 2, expected: [0, 2], desc: 'single line middle' }, + { text: 'hello', offset: 5, expected: [0, 5], desc: 'single line end' }, + { text: 'hello', offset: 10, expected: [0, 5], desc: 'beyond end clamps' }, + { + text: 'a\n\nc', + offset: 0, + expected: [0, 0], + desc: 'empty lines - first char', + }, + { + text: 'a\n\nc', + offset: 1, + expected: [0, 1], + desc: 'empty lines - end of first', + }, + { + text: 'a\n\nc', + offset: 2, + expected: [1, 0], + desc: 'empty lines - empty line', + }, + { + text: 'a\n\nc', + offset: 3, + expected: [2, 0], + desc: 'empty lines - last line start', + }, + { + text: 'a\n\nc', + offset: 4, + expected: [2, 1], + desc: 'empty lines - last line end', + }, + { + text: 'hello\n', + offset: 5, + expected: [0, 5], + desc: 'newline end - before newline', + }, + { + text: 'hello\n', + offset: 6, + expected: [1, 0], + desc: 'newline end - after newline', + }, + { + text: 'hello\n', + offset: 7, + expected: [1, 0], + desc: 'newline end - beyond', + }, + { + text: '\nhello', + offset: 0, + expected: [0, 0], + desc: 'newline start - first line', + }, + { + text: '\nhello', + offset: 1, + expected: [1, 0], + desc: 'newline start - second line', + }, + { + text: '\nhello', + offset: 3, + expected: [1, 2], + desc: 'newline start - middle of second', + }, + { text: '', offset: 0, expected: [0, 0], desc: 'empty string at 0' }, + { text: '', offset: 5, expected: [0, 0], desc: 'empty string beyond' }, + { + text: '你好\n世界', + offset: 0, + expected: [0, 0], + desc: 'unicode - start', + }, + { + text: '你好\n世界', + offset: 1, + expected: [0, 1], + desc: 'unicode - after first char', + }, + { + text: '你好\n世界', + offset: 2, + expected: [0, 2], + desc: 'unicode - end first line', + }, + { + text: '你好\n世界', + offset: 3, + expected: [1, 0], + desc: 'unicode - second line start', + }, + { + text: '你好\n世界', + offset: 4, + expected: [1, 1], + desc: 'unicode - second line middle', + }, + { + text: '你好\n世界', + offset: 5, + expected: [1, 2], + desc: 'unicode - second line end', + }, + { + text: '你好\n世界', + offset: 6, + expected: [1, 2], + desc: 'unicode - beyond', + }, + { + text: 'abc\ndef', + offset: 3, + expected: [0, 3], + desc: 'at newline - end of line', + }, + { + text: 'abc\ndef', + offset: 4, + expected: [1, 0], + desc: 'at newline - after newline', + }, + { text: '🐶🐱', offset: 0, expected: [0, 0], desc: 'emoji - start' }, + { text: '🐶🐱', offset: 1, expected: [0, 1], desc: 'emoji - middle' }, + { text: '🐶🐱', offset: 2, expected: [0, 2], desc: 'emoji - end' }, + ])('should handle $desc', ({ text, offset, expected }) => { + expect(offsetToLogicalPos(text, offset)).toEqual(expected); }); - it('should handle single line text', () => { - const text = 'hello'; - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); // Start - expect(offsetToLogicalPos(text, 2)).toEqual([0, 2]); // Middle 'l' - expect(offsetToLogicalPos(text, 5)).toEqual([0, 5]); // End - expect(offsetToLogicalPos(text, 10)).toEqual([0, 5]); // Beyond end - }); - - it('should handle multi-line text', () => { + describe('multi-line text', () => { const text = 'hello\nworld\n123'; - // "hello" (5) + \n (1) + "world" (5) + \n (1) + "123" (3) - // h e l l o \n w o r l d \n 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 - // Line 0: "hello" (length 5) - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); // Start of 'hello' - expect(offsetToLogicalPos(text, 3)).toEqual([0, 3]); // 'l' in 'hello' - expect(offsetToLogicalPos(text, 5)).toEqual([0, 5]); // End of 'hello' (before \n) - // Line 1: "world" (length 5) - expect(offsetToLogicalPos(text, 6)).toEqual([1, 0]); // Start of 'world' (after \n) - expect(offsetToLogicalPos(text, 8)).toEqual([1, 2]); // 'r' in 'world' - expect(offsetToLogicalPos(text, 11)).toEqual([1, 5]); // End of 'world' (before \n) - - // Line 2: "123" (length 3) - expect(offsetToLogicalPos(text, 12)).toEqual([2, 0]); // Start of '123' (after \n) - expect(offsetToLogicalPos(text, 13)).toEqual([2, 1]); // '2' in '123' - expect(offsetToLogicalPos(text, 15)).toEqual([2, 3]); // End of '123' - expect(offsetToLogicalPos(text, 20)).toEqual([2, 3]); // Beyond end of text - }); - - it('should handle empty lines', () => { - const text = 'a\n\nc'; // "a" (1) + \n (1) + "" (0) + \n (1) + "c" (1) - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); // 'a' - expect(offsetToLogicalPos(text, 1)).toEqual([0, 1]); // End of 'a' - expect(offsetToLogicalPos(text, 2)).toEqual([1, 0]); // Start of empty line - expect(offsetToLogicalPos(text, 3)).toEqual([2, 0]); // Start of 'c' - expect(offsetToLogicalPos(text, 4)).toEqual([2, 1]); // End of 'c' - }); - - it('should handle text ending with a newline', () => { - const text = 'hello\n'; // "hello" (5) + \n (1) - expect(offsetToLogicalPos(text, 5)).toEqual([0, 5]); // End of 'hello' - expect(offsetToLogicalPos(text, 6)).toEqual([1, 0]); // Position on the new empty line after - - expect(offsetToLogicalPos(text, 7)).toEqual([1, 0]); // Still on the new empty line - }); - - it('should handle text starting with a newline', () => { - const text = '\nhello'; // "" (0) + \n (1) + "hello" (5) - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); // Start of first empty line - expect(offsetToLogicalPos(text, 1)).toEqual([1, 0]); // Start of 'hello' - expect(offsetToLogicalPos(text, 3)).toEqual([1, 2]); // 'l' in 'hello' - }); - - it('should handle empty string input', () => { - expect(offsetToLogicalPos('', 0)).toEqual([0, 0]); - expect(offsetToLogicalPos('', 5)).toEqual([0, 0]); - }); - - it('should handle multi-byte unicode characters correctly', () => { - const text = '你好\n世界'; // "你好" (2 chars) + \n (1) + "世界" (2 chars) - // Total "code points" for offset calculation: 2 + 1 + 2 = 5 - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); // Start of '你好' - expect(offsetToLogicalPos(text, 1)).toEqual([0, 1]); // After '你', before '好' - expect(offsetToLogicalPos(text, 2)).toEqual([0, 2]); // End of '你好' - expect(offsetToLogicalPos(text, 3)).toEqual([1, 0]); // Start of '世界' - expect(offsetToLogicalPos(text, 4)).toEqual([1, 1]); // After '世', before '界' - expect(offsetToLogicalPos(text, 5)).toEqual([1, 2]); // End of '世界' - expect(offsetToLogicalPos(text, 6)).toEqual([1, 2]); // Beyond end - }); - - it('should handle offset exactly at newline character', () => { - const text = 'abc\ndef'; - // a b c \n d e f - // 0 1 2 3 4 5 6 - expect(offsetToLogicalPos(text, 3)).toEqual([0, 3]); // End of 'abc' - // The next character is the newline, so an offset of 4 means the start of the next line. - expect(offsetToLogicalPos(text, 4)).toEqual([1, 0]); // Start of 'def' - }); - - it('should handle offset in the middle of a multi-byte character (should place at start of that char)', () => { - // This scenario is tricky as "offset" is usually character-based. - // Assuming cpLen and related logic handles this by treating multi-byte as one unit. - // The current implementation of offsetToLogicalPos uses cpLen, so it should be code-point aware. - const text = '🐶🐱'; // 2 code points - expect(offsetToLogicalPos(text, 0)).toEqual([0, 0]); - expect(offsetToLogicalPos(text, 1)).toEqual([0, 1]); // After 🐶 - expect(offsetToLogicalPos(text, 2)).toEqual([0, 2]); // After 🐱 + it.each([ + { offset: 0, expected: [0, 0], desc: 'start of first line' }, + { offset: 3, expected: [0, 3], desc: 'middle of first line' }, + { offset: 5, expected: [0, 5], desc: 'end of first line' }, + { offset: 6, expected: [1, 0], desc: 'start of second line' }, + { offset: 8, expected: [1, 2], desc: 'middle of second line' }, + { offset: 11, expected: [1, 5], desc: 'end of second line' }, + { offset: 12, expected: [2, 0], desc: 'start of third line' }, + { offset: 13, expected: [2, 1], desc: 'middle of third line' }, + { offset: 15, expected: [2, 3], desc: 'end of third line' }, + { offset: 20, expected: [2, 3], desc: 'beyond end' }, + ])( + 'should return $expected for $desc (offset $offset)', + ({ offset, expected }) => { + expect(offsetToLogicalPos(text, offset)).toEqual(expected); + }, + ); }); }); @@ -1920,7 +1958,6 @@ describe('logicalPosToOffset', () => { }); }); -// Helper to create state for reducer tests const createTestState = ( lines: string[], cursorRow: number, diff --git a/packages/cli/src/ui/hooks/__snapshots__/useToolScheduler.test.ts.snap b/packages/cli/src/ui/hooks/__snapshots__/useToolScheduler.test.ts.snap new file mode 100644 index 0000000000..871be0e764 --- /dev/null +++ b/packages/cli/src/ui/hooks/__snapshots__/useToolScheduler.test.ts.snap @@ -0,0 +1,93 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`useReactToolScheduler > should handle live output updates 1`] = ` +{ + "callId": "liveCall", + "contentLength": 12, + "error": undefined, + "errorType": undefined, + "outputFile": undefined, + "responseParts": [ + { + "functionResponse": { + "id": "liveCall", + "name": "mockToolWithLiveOutput", + "response": { + "output": "Final output", + }, + }, + }, + ], + "resultDisplay": "Final display", +} +`; + +exports[`useReactToolScheduler > should handle tool requiring confirmation - approved 1`] = ` +{ + "callId": "callConfirm", + "contentLength": 16, + "error": undefined, + "errorType": undefined, + "outputFile": undefined, + "responseParts": [ + { + "functionResponse": { + "id": "callConfirm", + "name": "mockToolRequiresConfirmation", + "response": { + "output": "Confirmed output", + }, + }, + }, + ], + "resultDisplay": "Confirmed display", +} +`; + +exports[`useReactToolScheduler > should handle tool requiring confirmation - cancelled by user 1`] = ` +{ + "callId": "callConfirmCancel", + "contentLength": 59, + "error": undefined, + "errorType": undefined, + "responseParts": [ + { + "functionResponse": { + "id": "callConfirmCancel", + "name": "mockToolRequiresConfirmation", + "response": { + "error": "[Operation Cancelled] Reason: User cancelled the operation.", + }, + }, + }, + ], + "resultDisplay": { + "fileDiff": "Mock tool requires confirmation", + "fileName": "mockToolRequiresConfirmation.ts", + "newContent": undefined, + "originalContent": undefined, + }, +} +`; + +exports[`useReactToolScheduler > should schedule and execute a tool call successfully 1`] = ` +{ + "callId": "call1", + "contentLength": 11, + "error": undefined, + "errorType": undefined, + "outputFile": undefined, + "responseParts": [ + { + "functionResponse": { + "id": "call1", + "name": "mockTool", + "response": { + "output": "Tool output", + }, + }, + }, + ], + "resultDisplay": "Formatted tool output", +} +`; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx index 251998d96d..b8b0081872 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.tsx @@ -431,34 +431,39 @@ describe('useSlashCommandProcessor', () => { }); describe('Action Result Handling', () => { - it('should handle "dialog: theme" action', async () => { - const command = createTestCommand({ - name: 'themecmd', - action: vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'theme' }), - }); - const result = await setupProcessorHook([command]); - await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); + describe('Dialog actions', () => { + it.each([ + { + dialogType: 'theme', + commandName: 'themecmd', + mockFn: mockOpenThemeDialog, + }, + { + dialogType: 'model', + commandName: 'modelcmd', + mockFn: mockOpenModelDialog, + }, + ])( + 'should handle "dialog: $dialogType" action', + async ({ dialogType, commandName, mockFn }) => { + const command = createTestCommand({ + name: commandName, + action: vi + .fn() + .mockResolvedValue({ type: 'dialog', dialog: dialogType }), + }); + const result = await setupProcessorHook([command]); + await waitFor(() => + expect(result.current.slashCommands).toHaveLength(1), + ); - await act(async () => { - await result.current.handleSlashCommand('/themecmd'); - }); + await act(async () => { + await result.current.handleSlashCommand(`/${commandName}`); + }); - expect(mockOpenThemeDialog).toHaveBeenCalled(); - }); - - it('should handle "dialog: model" action', async () => { - const command = createTestCommand({ - name: 'modelcmd', - action: vi.fn().mockResolvedValue({ type: 'dialog', dialog: 'model' }), - }); - const result = await setupProcessorHook([command]); - await waitFor(() => expect(result.current.slashCommands).toHaveLength(1)); - - await act(async () => { - await result.current.handleSlashCommand('/modelcmd'); - }); - - expect(mockOpenModelDialog).toHaveBeenCalled(); + expect(mockFn).toHaveBeenCalled(); + }, + ); }); it('should handle "load_history" action', async () => { @@ -1007,98 +1012,67 @@ describe('useSlashCommandProcessor', () => { vi.mocked(logSlashCommand).mockClear(); }); - it('should log a simple slash command', async () => { - const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/logtest'); - }); - - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ + it.each([ + { + command: '/logtest', + expectedLog: { command: 'logtest', subcommand: undefined, status: SlashCommandStatus.SUCCESS, - }), - ); - }); - - it('logs nothing for a bogus command', async () => { - const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/bogusbogusbogus'); - }); - - expect(logSlashCommand).not.toHaveBeenCalled(); - }); - - it('logs a failure event for a failed command', async () => { - const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/fail'); - }); - - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ + }, + desc: 'simple slash command', + }, + { + command: '/fail', + expectedLog: { command: 'fail', status: 'error', subcommand: undefined, - }), - ); - }); - - it('should log a slash command with a subcommand', async () => { - const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/logwithsub sub'); - }); - - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ + }, + desc: 'failure event for failed command', + }, + { + command: '/logwithsub sub', + expectedLog: { command: 'logwithsub', subcommand: 'sub', - }), - ); - }); - - it('should log the command path when an alias is used', async () => { - const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); - await act(async () => { - await result.current.handleSlashCommand('/la'); - }); - expect(logSlashCommand).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ + }, + desc: 'slash command with subcommand', + }, + { + command: '/la', + expectedLog: { command: 'logalias', - }), - ); + }, + desc: 'command path when alias is used', + }, + ])('should log $desc', async ({ command, expectedLog }) => { + const result = await setupProcessorHook(loggingTestCommands); + await waitFor(() => expect(result.current.slashCommands).toBeDefined()); + + await act(async () => { + await result.current.handleSlashCommand(command); + }); + + await waitFor(() => { + expect(logSlashCommand).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining(expectedLog), + ); + }); }); - it('should not log for unknown commands', async () => { + it.each([ + { command: '/bogusbogusbogus', desc: 'bogus command' }, + { command: '/unknown', desc: 'unknown command' }, + ])('should not log for $desc', async ({ command }) => { const result = await setupProcessorHook(loggingTestCommands); - await waitFor(() => - expect(result.current.slashCommands?.length).toBeGreaterThan(0), - ); + await waitFor(() => expect(result.current.slashCommands).toBeDefined()); + await act(async () => { - await result.current.handleSlashCommand('/unknown'); + await result.current.handleSlashCommand(command); }); + expect(logSlashCommand).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index f9b05314da..dd4d4393da 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -360,6 +360,100 @@ describe('useGeminiStream', () => { }; }; + // Helper to create mock tool calls - reduces boilerplate + const createMockToolCall = ( + toolName: string, + callId: string, + confirmationType: 'edit' | 'info', + mockOnConfirm: Mock, + status: TrackedToolCall['status'] = 'awaiting_approval', + ): TrackedWaitingToolCall => ({ + request: { + callId, + name: toolName, + args: {}, + isClientInitiated: false, + prompt_id: 'prompt-id-1', + }, + status: status as 'awaiting_approval', + responseSubmittedToGemini: false, + confirmationDetails: + confirmationType === 'edit' + ? { + type: 'edit', + title: 'Confirm Edit', + onConfirm: mockOnConfirm, + fileName: 'file.txt', + filePath: '/test/file.txt', + fileDiff: 'fake diff', + originalContent: 'old', + newContent: 'new', + } + : { + type: 'info', + title: `${toolName} confirmation`, + onConfirm: mockOnConfirm, + prompt: `Execute ${toolName}?`, + }, + tool: { + name: toolName, + displayName: toolName, + description: `${toolName} description`, + build: vi.fn(), + } as any, + invocation: { + getDescription: () => 'Mock description', + } as unknown as AnyToolInvocation, + }); + + // Helper to render hook with default parameters - reduces boilerplate + const renderHookWithDefaults = ( + options: { + shellModeActive?: boolean; + onCancelSubmit?: () => void; + setShellInputFocused?: (focused: boolean) => void; + performMemoryRefresh?: () => Promise; + onAuthError?: () => void; + onEditorClose?: () => void; + setModelSwitched?: Mock; + modelSwitched?: boolean; + } = {}, + ) => { + const { + shellModeActive = false, + onCancelSubmit = () => {}, + setShellInputFocused = () => {}, + performMemoryRefresh = () => Promise.resolve(), + onAuthError = () => {}, + onEditorClose = () => {}, + setModelSwitched = vi.fn(), + modelSwitched = false, + } = options; + + return renderHook(() => + useGeminiStream( + new MockedGeminiClientClass(mockConfig), + [], + mockAddItem, + mockConfig, + mockLoadedSettings, + mockOnDebugMessage, + mockHandleSlashCommand, + shellModeActive, + () => 'vscode' as EditorType, + onAuthError, + performMemoryRefresh, + modelSwitched, + setModelSwitched, + onEditorClose, + onCancelSubmit, + setShellInputFocused, + 80, + 24, + ), + ); + }; + it('should not submit tool responses if not all tool calls are completed', () => { const toolCalls: TrackedToolCall[] = [ { @@ -1473,62 +1567,8 @@ describe('useGeminiStream', () => { it('should auto-approve all pending tool calls when switching to YOLO mode', async () => { const mockOnConfirm = vi.fn().mockResolvedValue(undefined); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirm, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - }, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, - { - request: { - callId: 'call2', - name: 'read_file', - args: { path: '/test/file.txt' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'info', - title: 'Read File', - onConfirm: mockOnConfirm, - prompt: 'Read /test/file.txt?', - }, - tool: { - name: 'read_file', - displayName: 'read_file', - description: 'Read file', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, + createMockToolCall('replace', 'call1', 'edit', mockOnConfirm), + createMockToolCall('read_file', 'call2', 'info', mockOnConfirm), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1539,12 +1579,7 @@ describe('useGeminiStream', () => { // Both tool calls should be auto-approved expect(mockOnConfirm).toHaveBeenCalledTimes(2); - expect(mockOnConfirm).toHaveBeenNthCalledWith( - 1, - ToolConfirmationOutcome.ProceedOnce, - ); - expect(mockOnConfirm).toHaveBeenNthCalledWith( - 2, + expect(mockOnConfirm).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); }); @@ -1555,92 +1590,9 @@ describe('useGeminiStream', () => { const mockOnConfirmRead = vi.fn().mockResolvedValue(undefined); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirmReplace, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - }, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, - { - request: { - callId: 'call2', - name: 'write_file', - args: { path: '/test/new.txt', content: 'content' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirmWrite, - fileName: 'new.txt', - filePath: '/test/new.txt', - fileDiff: 'fake diff', - originalContent: null, - newContent: 'content', - }, - tool: { - name: 'write_file', - displayName: 'write_file', - description: 'Write file', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, - { - request: { - callId: 'call3', - name: 'read_file', - args: { path: '/test/file.txt' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'info', - title: 'Read File', - onConfirm: mockOnConfirmRead, - prompt: 'Read /test/file.txt?', - }, - tool: { - name: 'read_file', - displayName: 'read_file', - description: 'Read file', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, + createMockToolCall('replace', 'call1', 'edit', mockOnConfirmReplace), + createMockToolCall('write_file', 'call2', 'edit', mockOnConfirmWrite), + createMockToolCall('read_file', 'call3', 'info', mockOnConfirmRead), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1650,11 +1602,9 @@ describe('useGeminiStream', () => { }); // Only replace and write_file should be auto-approved - expect(mockOnConfirmReplace).toHaveBeenCalledTimes(1); expect(mockOnConfirmReplace).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); - expect(mockOnConfirmWrite).toHaveBeenCalledTimes(1); expect(mockOnConfirmWrite).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); @@ -1666,36 +1616,7 @@ describe('useGeminiStream', () => { it('should not auto-approve any tools when switching to REQUIRE_CONFIRMATION mode', async () => { const mockOnConfirm = vi.fn().mockResolvedValue(undefined); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirm, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - }, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, + createMockToolCall('replace', 'call1', 'edit', mockOnConfirm), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1718,66 +1639,8 @@ describe('useGeminiStream', () => { .mockRejectedValue(new Error('Approval failed')); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirmSuccess, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - }, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, - { - request: { - callId: 'call2', - name: 'write_file', - args: { path: '/test/file.txt', content: 'content' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirmError, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: null, - newContent: 'content', - }, - tool: { - name: 'write_file', - displayName: 'write_file', - description: 'Write file', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, + createMockToolCall('replace', 'call1', 'edit', mockOnConfirmSuccess), + createMockToolCall('write_file', 'call2', 'edit', mockOnConfirmError), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1787,8 +1650,8 @@ describe('useGeminiStream', () => { }); // Both confirmation methods should be called - expect(mockOnConfirmSuccess).toHaveBeenCalledTimes(1); - expect(mockOnConfirmError).toHaveBeenCalledTimes(1); + expect(mockOnConfirmSuccess).toHaveBeenCalled(); + expect(mockOnConfirmError).toHaveBeenCalled(); // Error should be logged expect(debuggerSpy).toHaveBeenCalledWith( @@ -2006,115 +1869,53 @@ describe('useGeminiStream', () => { vi.mocked(tokenLimit).mockReturnValue(100); }); - it('should add message without suggestion when remaining tokens are > 75% of limit', async () => { - // Setup mock to return a stream with ContextWindowWillOverflow event - // Limit is 100, remaining is 80 (> 75) - mockSendMessageStream.mockReturnValue( - (async function* () { - yield { - type: ServerGeminiEventType.ContextWindowWillOverflow, - value: { - estimatedRequestTokenCount: 20, - remainingTokenCount: 80, - }, - }; - })(), - ); - - const { result } = renderHook(() => - useGeminiStream( - new MockedGeminiClientClass(mockConfig), - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - () => {}, - 80, - 24, - ), - ); - - // Submit a query - await act(async () => { - await result.current.submitQuery('Test overflow'); - }); - - // Check that the message was added without suggestion - await waitFor(() => { - expect(mockAddItem).toHaveBeenCalledWith( - { - type: 'info', - text: `Sending this message (20 tokens) might exceed the remaining context window limit (80 tokens).`, - }, - expect.any(Number), + it.each([ + { + name: 'without suggestion when remaining tokens are > 75% of limit', + requestTokens: 20, + remainingTokens: 80, + expectedMessage: + 'Sending this message (20 tokens) might exceed the remaining context window limit (80 tokens).', + }, + { + name: 'with suggestion when remaining tokens are < 75% of limit', + requestTokens: 30, + remainingTokens: 70, + expectedMessage: + 'Sending this message (30 tokens) might exceed the remaining context window limit (70 tokens). Please try reducing the size of your message or use the `/compress` command to compress the chat history.', + }, + ])( + 'should add message $name', + async ({ requestTokens, remainingTokens, expectedMessage }) => { + mockSendMessageStream.mockReturnValue( + (async function* () { + yield { + type: ServerGeminiEventType.ContextWindowWillOverflow, + value: { + estimatedRequestTokenCount: requestTokens, + remainingTokenCount: remainingTokens, + }, + }; + })(), ); - }); - }); - it('should add message with suggestion when remaining tokens are < 75% of limit', async () => { - // Setup mock to return a stream with ContextWindowWillOverflow event - // Limit is 100, remaining is 70 (< 75) - mockSendMessageStream.mockReturnValue( - (async function* () { - yield { - type: ServerGeminiEventType.ContextWindowWillOverflow, - value: { - estimatedRequestTokenCount: 30, - remainingTokenCount: 70, + const { result } = renderHookWithDefaults(); + + await act(async () => { + await result.current.submitQuery('Test overflow'); + }); + + await waitFor(() => { + expect(mockAddItem).toHaveBeenCalledWith( + { + type: 'info', + text: expectedMessage, }, - }; - })(), - ); - - const { result } = renderHook(() => - useGeminiStream( - new MockedGeminiClientClass(mockConfig), - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - () => {}, - 80, - 24, - ), - ); - - // Submit a query - await act(async () => { - await result.current.submitQuery('Test overflow'); - }); - - // Check that the message was added with suggestion - await waitFor(() => { - expect(mockAddItem).toHaveBeenCalledWith( - { - type: 'info', - text: `Sending this message (30 tokens) might exceed the remaining context window limit (70 tokens). Please try reducing the size of your message or use the \`/compress\` command to compress the chat history.`, - }, - expect.any(Number), - ); - }); - }); + expect.any(Number), + ); + }); + }, + ); }); it('should call onCancelSubmit when ContextWindowWillOverflow event is received', async () => { @@ -2166,160 +1967,59 @@ describe('useGeminiStream', () => { }); }); - it('should not add message for STOP finish reason', async () => { - // Setup mock to return a stream with STOP finish reason - mockSendMessageStream.mockReturnValue( - (async function* () { - yield { - type: ServerGeminiEventType.Content, - value: 'Complete response', - }; - yield { - type: ServerGeminiEventType.Finished, - value: { reason: 'STOP', usageMetadata: undefined }, - }; - })(), - ); - - const { result } = renderHook(() => - useGeminiStream( - new MockedGeminiClientClass(mockConfig), - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - () => {}, - 80, - 24, - ), - ); - - // Submit a query - await act(async () => { - await result.current.submitQuery('Test normal completion'); - }); - - // Wait a bit to ensure no message is added - await new Promise((resolve) => setTimeout(resolve, 100)); - - // Check that no info message was added for STOP - const infoMessages = mockAddItem.mock.calls.filter( - (call) => call[0].type === 'info', - ); - expect(infoMessages).toHaveLength(0); - }); - - it('should not add message for FINISH_REASON_UNSPECIFIED', async () => { - // Setup mock to return a stream with FINISH_REASON_UNSPECIFIED - mockSendMessageStream.mockReturnValue( - (async function* () { - yield { - type: ServerGeminiEventType.Content, - value: 'Response with unspecified finish', - }; - yield { - type: ServerGeminiEventType.Finished, - value: { - reason: 'FINISH_REASON_UNSPECIFIED', - usageMetadata: undefined, - }, - }; - })(), - ); - - const { result } = renderHook(() => - useGeminiStream( - new MockedGeminiClientClass(mockConfig), - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - () => {}, - 80, - 24, - ), - ); - - // Submit a query - await act(async () => { - await result.current.submitQuery('Test unspecified finish'); - }); - - // Wait a bit to ensure no message is added - await new Promise((resolve) => setTimeout(resolve, 100)); - - // Check that no info message was added - const infoMessages = mockAddItem.mock.calls.filter( - (call) => call[0].type === 'info', - ); - expect(infoMessages).toHaveLength(0); - }); - - it('should add appropriate messages for other finish reasons', async () => { - const testCases = [ - { - reason: 'SAFETY', - message: '⚠️ Response stopped due to safety reasons.', - }, - { - reason: 'RECITATION', - message: '⚠️ Response stopped due to recitation policy.', - }, - { - reason: 'LANGUAGE', - message: '⚠️ Response stopped due to unsupported language.', - }, - { - reason: 'BLOCKLIST', - message: '⚠️ Response stopped due to forbidden terms.', - }, - { - reason: 'PROHIBITED_CONTENT', - message: '⚠️ Response stopped due to prohibited content.', - }, - { - reason: 'SPII', - message: - '⚠️ Response stopped due to sensitive personally identifiable information.', - }, - { reason: 'OTHER', message: '⚠️ Response stopped for other reasons.' }, - { - reason: 'MALFORMED_FUNCTION_CALL', - message: '⚠️ Response stopped due to malformed function call.', - }, - { - reason: 'IMAGE_SAFETY', - message: '⚠️ Response stopped due to image safety violations.', - }, - { - reason: 'UNEXPECTED_TOOL_CALL', - message: '⚠️ Response stopped due to unexpected tool call.', - }, - ]; - - for (const { reason, message } of testCases) { - // Reset mocks for each test case - mockAddItem.mockClear(); + it.each([ + { + reason: 'STOP', + shouldAddMessage: false, + }, + { + reason: 'FINISH_REASON_UNSPECIFIED', + shouldAddMessage: false, + }, + { + reason: 'SAFETY', + message: '⚠️ Response stopped due to safety reasons.', + }, + { + reason: 'RECITATION', + message: '⚠️ Response stopped due to recitation policy.', + }, + { + reason: 'LANGUAGE', + message: '⚠️ Response stopped due to unsupported language.', + }, + { + reason: 'BLOCKLIST', + message: '⚠️ Response stopped due to forbidden terms.', + }, + { + reason: 'PROHIBITED_CONTENT', + message: '⚠️ Response stopped due to prohibited content.', + }, + { + reason: 'SPII', + message: + '⚠️ Response stopped due to sensitive personally identifiable information.', + }, + { + reason: 'OTHER', + message: '⚠️ Response stopped for other reasons.', + }, + { + reason: 'MALFORMED_FUNCTION_CALL', + message: '⚠️ Response stopped due to malformed function call.', + }, + { + reason: 'IMAGE_SAFETY', + message: '⚠️ Response stopped due to image safety violations.', + }, + { + reason: 'UNEXPECTED_TOOL_CALL', + message: '⚠️ Response stopped due to unexpected tool call.', + }, + ])( + 'should handle $reason finish reason correctly', + async ({ reason, shouldAddMessage = true, message }) => { mockSendMessageStream.mockReturnValue( (async function* () { yield { @@ -2333,44 +2033,35 @@ describe('useGeminiStream', () => { })(), ); - const { result } = renderHook(() => - useGeminiStream( - new MockedGeminiClientClass(mockConfig), - [], - mockAddItem, - mockConfig, - mockLoadedSettings, - mockOnDebugMessage, - mockHandleSlashCommand, - false, - () => 'vscode' as EditorType, - () => {}, - () => Promise.resolve(), - false, - () => {}, - () => {}, - () => {}, - vi.fn(), - 80, - 24, - ), - ); + const { result } = renderHookWithDefaults(); await act(async () => { await result.current.submitQuery(`Test ${reason}`); }); - await waitFor(() => { - expect(mockAddItem).toHaveBeenCalledWith( - { - type: 'info', - text: message, - }, - expect.any(Number), + if (shouldAddMessage) { + await waitFor(() => { + expect(mockAddItem).toHaveBeenCalledWith( + { + type: 'info', + text: message, + }, + expect.any(Number), + ); + }); + } else { + // Verify state returns to idle without any info messages + await waitFor(() => { + expect(result.current.streamingState).toBe(StreamingState.Idle); + }); + + const infoMessages = mockAddItem.mock.calls.filter( + (call) => call[0].type === 'info', ); - }); - } - }); + expect(infoMessages).toHaveLength(0); + } + }, + ); }); it('should process @include commands, adding user turn after processing to prevent race conditions', async () => { diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index 4eb4948d99..30eeab06e9 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -201,6 +201,28 @@ describe('useReactToolScheduler', () => { | ((outcome: ToolConfirmationOutcome) => void | Promise) | undefined; + const advanceAndSettle = async () => { + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + }; + + const scheduleAndWaitForExecution = async ( + schedule: ( + req: ToolCallRequestInfo | ToolCallRequestInfo[], + signal: AbortSignal, + ) => void, + request: ToolCallRequestInfo | ToolCallRequestInfo[], + ) => { + act(() => { + schedule(request, new AbortController().signal); + }); + + await advanceAndSettle(); + await advanceAndSettle(); + await advanceAndSettle(); + }; + beforeEach(() => { onComplete = vi.fn(); capturedOnConfirmForTest = undefined; @@ -259,7 +281,6 @@ describe('useReactToolScheduler', () => { (mockTool.shouldConfirmExecute as Mock).mockResolvedValue(null); const { result } = renderScheduler(); - const schedule = result.current[1]; const request: ToolCallRequestInfo = { callId: 'call1', name: 'mockTool', @@ -271,42 +292,19 @@ describe('useReactToolScheduler', () => { completedToolCalls = calls; }); - act(() => { - schedule(request, new AbortController().signal); - }); - - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await scheduleAndWaitForExecution(result.current[1], request); expect(mockTool.execute).toHaveBeenCalledWith(request.args); - expect(onComplete).toHaveBeenCalledWith([ - expect.objectContaining({ - status: 'success', - request, - response: expect.objectContaining({ - resultDisplay: 'Formatted tool output', - responseParts: [ - { - functionResponse: { - id: 'call1', - name: 'mockTool', - response: { output: 'Tool output' }, - }, - }, - ], - }), - }), - ]); expect(completedToolCalls).toHaveLength(1); expect(completedToolCalls[0].status).toBe('success'); expect(completedToolCalls[0].request).toBe(request); + + if ( + completedToolCalls[0].status === 'success' || + completedToolCalls[0].status === 'error' + ) { + expect(completedToolCalls[0].response).toMatchSnapshot(); + } }); it('should clear previous tool calls when scheduling new ones', async () => { @@ -412,113 +410,83 @@ describe('useReactToolScheduler', () => { }); }); - it('should handle tool not found', async () => { - mockToolRegistry.getTool.mockReturnValue(undefined); - const { result } = renderScheduler(); - const schedule = result.current[1]; - const request: ToolCallRequestInfo = { - callId: 'call1', - name: 'nonexistentTool', - args: {}, - } as any; + it.each([ + { + desc: 'tool not found', + setup: () => { + mockToolRegistry.getTool.mockReturnValue(undefined); + }, + request: { + callId: 'call1', + name: 'nonexistentTool', + args: {}, + } as any, + expectedErrorContains: [ + 'Tool "nonexistentTool" not found in registry', + 'Did you mean one of:', + ], + }, + { + desc: 'error during shouldConfirmExecute', + setup: () => { + mockToolRegistry.getTool.mockReturnValue(mockTool); + const confirmError = new Error('Confirmation check failed'); + (mockTool.shouldConfirmExecute as Mock).mockRejectedValue(confirmError); + }, + request: { + callId: 'call1', + name: 'mockTool', + args: {}, + } as any, + expectedError: new Error('Confirmation check failed'), + }, + { + desc: 'error during execute', + setup: () => { + mockToolRegistry.getTool.mockReturnValue(mockTool); + (mockTool.shouldConfirmExecute as Mock).mockResolvedValue(null); + const execError = new Error('Execution failed'); + (mockTool.execute as Mock).mockRejectedValue(execError); + }, + request: { + callId: 'call1', + name: 'mockTool', + args: {}, + } as any, + expectedError: new Error('Execution failed'), + }, + ])( + 'should handle $desc', + async ({ setup, request, expectedErrorContains, expectedError }) => { + setup(); + const { result } = renderScheduler(); - let completedToolCalls: ToolCall[] = []; - onComplete.mockImplementation((calls) => { - completedToolCalls = calls; - }); + let completedToolCalls: ToolCall[] = []; + onComplete.mockImplementation((calls) => { + completedToolCalls = calls; + }); - act(() => { - schedule(request, new AbortController().signal); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await scheduleAndWaitForExecution(result.current[1], request); - expect(completedToolCalls).toHaveLength(1); - expect(completedToolCalls[0].status).toBe('error'); - expect(completedToolCalls[0].request).toBe(request); - expect((completedToolCalls[0] as any).response.error.message).toContain( - 'Tool "nonexistentTool" not found in registry', - ); - expect((completedToolCalls[0] as any).response.error.message).toContain( - 'Did you mean one of:', - ); - }); + expect(completedToolCalls).toHaveLength(1); + expect(completedToolCalls[0].status).toBe('error'); + expect(completedToolCalls[0].request).toBe(request); - it('should handle error during shouldConfirmExecute', async () => { - mockToolRegistry.getTool.mockReturnValue(mockTool); - const confirmError = new Error('Confirmation check failed'); - (mockTool.shouldConfirmExecute as Mock).mockRejectedValue(confirmError); + if (expectedErrorContains) { + expectedErrorContains.forEach((errorText) => { + expect( + (completedToolCalls[0] as any).response.error.message, + ).toContain(errorText); + }); + } - const { result } = renderScheduler(); - const schedule = result.current[1]; - const request: ToolCallRequestInfo = { - callId: 'call1', - name: 'mockTool', - args: {}, - } as any; - - let completedToolCalls: ToolCall[] = []; - onComplete.mockImplementation((calls) => { - completedToolCalls = calls; - }); - - act(() => { - schedule(request, new AbortController().signal); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - - expect(completedToolCalls).toHaveLength(1); - expect(completedToolCalls[0].status).toBe('error'); - expect(completedToolCalls[0].request).toBe(request); - expect((completedToolCalls[0] as any).response.error).toBe(confirmError); - }); - - it('should handle error during execute', async () => { - mockToolRegistry.getTool.mockReturnValue(mockTool); - (mockTool.shouldConfirmExecute as Mock).mockResolvedValue(null); - const execError = new Error('Execution failed'); - (mockTool.execute as Mock).mockRejectedValue(execError); - - const { result } = renderScheduler(); - const schedule = result.current[1]; - const request: ToolCallRequestInfo = { - callId: 'call1', - name: 'mockTool', - args: {}, - } as any; - - let completedToolCalls: ToolCall[] = []; - onComplete.mockImplementation((calls) => { - completedToolCalls = calls; - }); - - act(() => { - schedule(request, new AbortController().signal); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - - expect(completedToolCalls).toHaveLength(1); - expect(completedToolCalls[0].status).toBe('error'); - expect(completedToolCalls[0].request).toBe(request); - expect((completedToolCalls[0] as any).response.error).toBe(execError); - }); + if (expectedError) { + expect((completedToolCalls[0] as any).response.error.message).toBe( + expectedError.message, + ); + } + }, + ); it('should handle tool requiring confirmation - approved', async () => { mockToolRegistry.getTool.mockReturnValue(mockToolRequiresConfirmation); @@ -539,9 +507,7 @@ describe('useReactToolScheduler', () => { act(() => { schedule(request, new AbortController().signal); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); const waitingCall = result.current[0][0] as any; expect(waitingCall.status).toBe('awaiting_approval'); @@ -552,36 +518,24 @@ describe('useReactToolScheduler', () => { await capturedOnConfirmForTest?.(ToolConfirmationOutcome.ProceedOnce); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); + await advanceAndSettle(); + await advanceAndSettle(); expect(mockOnUserConfirmForToolConfirmation).toHaveBeenCalledWith( ToolConfirmationOutcome.ProceedOnce, ); expect(mockToolRequiresConfirmation.execute).toHaveBeenCalled(); - expect(onComplete).toHaveBeenCalledWith([ - expect.objectContaining({ - status: 'success', - request, - response: expect.objectContaining({ - resultDisplay: 'Confirmed display', - responseParts: expect.arrayContaining([ - expect.objectContaining({ - functionResponse: expect.objectContaining({ - response: { output: expectedOutput }, - }), - }), - ]), - }), - }), - ]); + + const completedCalls = onComplete.mock.calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('success'); + expect(completedCalls[0].request).toBe(request); + if ( + completedCalls[0].status === 'success' || + completedCalls[0].status === 'error' + ) { + expect(completedCalls[0].response).toMatchSnapshot(); + } }); it('should handle tool requiring confirmation - cancelled by user', async () => { @@ -597,9 +551,7 @@ describe('useReactToolScheduler', () => { act(() => { schedule(request, new AbortController().signal); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); const waitingCall = result.current[0][0] as any; expect(waitingCall.status).toBe('awaiting_approval'); @@ -609,34 +561,23 @@ describe('useReactToolScheduler', () => { await act(async () => { await capturedOnConfirmForTest?.(ToolConfirmationOutcome.Cancel); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); + await advanceAndSettle(); expect(mockOnUserConfirmForToolConfirmation).toHaveBeenCalledWith( ToolConfirmationOutcome.Cancel, ); - expect(onComplete).toHaveBeenCalledWith([ - expect.objectContaining({ - status: 'cancelled', - request, - response: expect.objectContaining({ - responseParts: expect.arrayContaining([ - expect.objectContaining({ - functionResponse: expect.objectContaining({ - response: expect.objectContaining({ - error: - '[Operation Cancelled] Reason: User cancelled the operation.', - }), - }), - }), - ]), - }), - }), - ]); + + const completedCalls = onComplete.mock.calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('cancelled'); + expect(completedCalls[0].request).toBe(request); + if ( + completedCalls[0].status === 'success' || + completedCalls[0].status === 'error' || + completedCalls[0].status === 'cancelled' + ) { + expect(completedCalls[0].response).toMatchSnapshot(); + } }); it('should handle live output updates', async () => { @@ -662,7 +603,6 @@ describe('useReactToolScheduler', () => { ); const { result } = renderScheduler(); - const schedule = result.current[1]; const request: ToolCallRequestInfo = { callId: 'liveCall', name: 'mockToolWithLiveOutput', @@ -670,11 +610,9 @@ describe('useReactToolScheduler', () => { } as any; act(() => { - schedule(request, new AbortController().signal); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); + result.current[1](request, new AbortController().signal); }); + await advanceAndSettle(); expect(liveUpdateFn).toBeDefined(); expect(result.current[0][0].status).toBe('executing'); @@ -682,16 +620,12 @@ describe('useReactToolScheduler', () => { await act(async () => { liveUpdateFn?.('Live output 1'); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); await act(async () => { liveUpdateFn?.('Live output 2'); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); act(() => { resolveExecutePromise({ @@ -699,29 +633,18 @@ describe('useReactToolScheduler', () => { returnDisplay: 'Final display', } as ToolResult); }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); - await act(async () => { - await vi.advanceTimersByTimeAsync(0); - }); + await advanceAndSettle(); + await advanceAndSettle(); - expect(onComplete).toHaveBeenCalledWith([ - expect.objectContaining({ - status: 'success', - request, - response: expect.objectContaining({ - resultDisplay: 'Final display', - responseParts: expect.arrayContaining([ - expect.objectContaining({ - functionResponse: expect.objectContaining({ - response: { output: 'Final output' }, - }), - }), - ]), - }), - }), - ]); + const completedCalls = onComplete.mock.calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('success'); + expect(completedCalls[0].request).toBe(request); + if ( + completedCalls[0].status === 'success' || + completedCalls[0].status === 'error' + ) { + expect(completedCalls[0].response).toMatchSnapshot(); + } expect(result.current[0]).toEqual([]); }); @@ -874,7 +797,6 @@ describe('useReactToolScheduler', () => { response: expect.objectContaining({ resultDisplay: 'done display' }), }), ]); - // Wait for request2 to complete await act(async () => { await vi.advanceTimersByTimeAsync(50); await vi.advanceTimersByTimeAsync(0);