diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index f77a14870f..5f29ba5667 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -58,6 +58,11 @@ export const ToolMessage: React.FC = ({ ptyId === activeShellPtyId && shellFocused; + const isThisShellFocusable = + (name === SHELL_COMMAND_NAME || name === 'Shell') && + status === ToolCallStatus.Executing && + config?.getShouldUseNodePtyShell(); + const availableHeight = availableTerminalHeight ? Math.max( availableTerminalHeight - STATIC_HEIGHT - RESERVED_LINE_COUNT, @@ -90,9 +95,11 @@ export const ToolMessage: React.FC = ({ description={description} emphasis={emphasis} /> - {isThisShellFocused && ( - - [Focused] + {isThisShellFocusable && ( + + + {isThisShellFocused ? '(Focused)' : '(ctrl+f to focus)'} + )} {emphasis === 'high' && } @@ -118,7 +125,7 @@ export const ToolMessage: React.FC = ({ ) : typeof resultDisplay === 'object' && - !Array.isArray(resultDisplay) ? ( + 'fileDiff' in resultDisplay ? ( vi.fn()); @@ -54,6 +55,10 @@ vi.mock('../utils/terminalSerializer.js', () => ({ serializeTerminalToObject: mockSerializeTerminalToObject, })); +const mockProcessKill = vi + .spyOn(process, 'kill') + .mockImplementation(() => true); + const shellExecutionConfig = { terminalWidth: 80, terminalHeight: 24, @@ -61,9 +66,25 @@ const shellExecutionConfig = { showColor: false, }; -const mockProcessKill = vi - .spyOn(process, 'kill') - .mockImplementation(() => true); +const createExpectedAnsiOutput = (text: string | string[]): AnsiOutput => { + const lines = Array.isArray(text) ? text : text.split('\n'); + const expected: AnsiOutput = Array.from( + { length: shellExecutionConfig.terminalHeight }, + (_, i) => [ + { + text: expect.stringMatching((lines[i] || '').trim()), + bold: false, + italic: false, + underline: false, + dim: false, + inverse: false, + fg: '', + bg: '', + }, + ], + ); + return expected; +}; describe('ShellExecutionService', () => { let mockPtyProcess: EventEmitter & { @@ -77,6 +98,11 @@ describe('ShellExecutionService', () => { let mockHeadlessTerminal: { resize: Mock; scrollLines: Mock; + buffer: { + active: { + viewportY: number; + }; + }; }; let onOutputEventMock: Mock<(event: ShellOutputEvent) => void>; @@ -110,6 +136,11 @@ describe('ShellExecutionService', () => { mockHeadlessTerminal = { resize: vi.fn(), scrollLines: vi.fn(), + buffer: { + active: { + viewportY: 0, + }, + }, }; mockPtySpawn.mockReturnValue(mockPtyProcess); @@ -161,7 +192,7 @@ describe('ShellExecutionService', () => { expect(onOutputEventMock).toHaveBeenCalledWith({ type: 'data', - chunk: 'file1.txt', + chunk: createExpectedAnsiOutput('file1.txt'), }); }); @@ -175,7 +206,7 @@ describe('ShellExecutionService', () => { expect(onOutputEventMock).toHaveBeenCalledWith( expect.objectContaining({ type: 'data', - chunk: expect.stringContaining('aredword'), + chunk: createExpectedAnsiOutput('aredword'), }), ); }); @@ -197,7 +228,7 @@ describe('ShellExecutionService', () => { expect(onOutputEventMock).toHaveBeenCalledWith( expect.objectContaining({ - chunk: expect.stringMatching(/^\s*$/), + chunk: createExpectedAnsiOutput(''), }), ); }); @@ -438,17 +469,44 @@ describe('ShellExecutionService', () => { ); }); - it('should call onOutputEvent with plain string when showColor is false', async () => { - await simulateExecution('ls --color=auto', (pty) => { - pty.onData.mock.calls[0][0]('a\u001b[31mred\u001b[0mword'); - pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); - }); + it('should call onOutputEvent with AnsiOutput when showColor is false', async () => { + await simulateExecution( + 'ls --color=auto', + (pty) => { + pty.onData.mock.calls[0][0]('a\u001b[31mred\u001b[0mword'); + pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); + }, + { ...shellExecutionConfig, showColor: false }, + ); + + const expected = createExpectedAnsiOutput('aredword'); - expect(mockSerializeTerminalToObject).not.toHaveBeenCalled(); expect(onOutputEventMock).toHaveBeenCalledWith( expect.objectContaining({ type: 'data', - chunk: 'aredword', + chunk: expected, + }), + ); + }); + + it('should handle multi-line output correctly when showColor is false', async () => { + await simulateExecution( + 'ls --color=auto', + (pty) => { + pty.onData.mock.calls[0][0]( + 'line 1\n\u001b[32mline 2\u001b[0m\nline 3', + ); + pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }); + }, + { ...shellExecutionConfig, showColor: false }, + ); + + const expected = createExpectedAnsiOutput(['line 1', 'line 2', 'line 3']); + + expect(onOutputEventMock).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'data', + chunk: expected, }), ); }); @@ -541,7 +599,7 @@ describe('ShellExecutionService child_process fallback', () => { expect(onOutputEventMock).toHaveBeenCalledWith( expect.objectContaining({ type: 'data', - chunk: expect.stringContaining('aredword'), + chunk: 'aredword', }), ); }); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 23cff439a2..9dbb539e7e 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -85,17 +85,6 @@ interface ActivePty { headlessTerminal: pkg.Terminal; } -const getVisibleText = (terminal: pkg.Terminal): string => { - const buffer = terminal.buffer.active; - const lines: string[] = []; - for (let i = 0; i < terminal.rows; i++) { - const line = buffer.getLine(buffer.viewportY + i); - const lineContent = line ? line.translateToString(true) : ''; - lines.push(lineContent); - } - return lines.join('\n').trimEnd(); -}; - const getFullBufferText = (terminal: pkg.Terminal): string => { const buffer = terminal.buffer.active; const lines: string[] = []; @@ -379,6 +368,7 @@ export class ShellExecutionService { cols, rows, }); + headlessTerminal.scrollToTop(); this.activePtys.set(ptyProcess.pid, { ptyProcess, headlessTerminal }); @@ -404,14 +394,33 @@ export class ShellExecutionService { if (!isStreamingRawContent) { return; } - const newOutput = shellExecutionConfig.showColor - ? serializeTerminalToObject(headlessTerminal, { - defaultFg: shellExecutionConfig.defaultFg, - defaultBg: shellExecutionConfig.defaultBg, - }) - : getVisibleText(headlessTerminal); - - // console.log(newOutput) + let newOutput: AnsiOutput; + if (shellExecutionConfig.showColor) { + newOutput = serializeTerminalToObject(headlessTerminal, { + defaultFg: shellExecutionConfig.defaultFg, + defaultBg: shellExecutionConfig.defaultBg, + }); + } else { + const buffer = headlessTerminal.buffer.active; + const lines: AnsiOutput = []; + for (let y = 0; y < headlessTerminal.rows; y++) { + const line = buffer.getLine(buffer.viewportY + y); + const lineContent = line ? line.translateToString(true) : ''; + lines.push([ + { + text: lineContent, + bold: false, + italic: false, + underline: false, + dim: false, + inverse: false, + fg: '', + bg: '', + }, + ]); + } + newOutput = lines; + } // Using stringify for a quick deep comparison. if (JSON.stringify(output) !== JSON.stringify(newOutput)) { @@ -602,6 +611,9 @@ export class ShellExecutionService { if (activePty) { try { activePty.headlessTerminal.scrollLines(lines); + if (activePty.headlessTerminal.buffer.active.viewportY < 0) { + activePty.headlessTerminal.scrollToTop(); + } } catch (e) { // Ignore errors if the pty has already exited, which can happen // due to a race condition between the exit event and this call.