diff --git a/apps/frontend/src/renderer/App.tsx b/apps/frontend/src/renderer/App.tsx index cbccbc5c..d5f4ed76 100644 --- a/apps/frontend/src/renderer/App.tsx +++ b/apps/frontend/src/renderer/App.tsx @@ -60,6 +60,7 @@ import { initializeGitHubListeners } from './stores/github'; import { initDownloadProgressListener } from './stores/download-store'; import { GlobalDownloadIndicator } from './components/GlobalDownloadIndicator'; import { useIpcListeners } from './hooks/useIpc'; +import { useGlobalTerminalListeners } from './hooks/useGlobalTerminalListeners'; import { COLOR_THEMES, UI_SCALE_MIN, UI_SCALE_MAX, UI_SCALE_DEFAULT } from '../shared/constants'; import type { Task, Project, ColorTheme } from '../shared/types'; import { ProjectTabBar } from './components/ProjectTabBar'; @@ -100,6 +101,10 @@ export function App() { // Load IPC listeners for real-time updates useIpcListeners(); + // Load global terminal output listeners to buffer output across project switches + // This ensures terminal output is captured even when the terminal component is not rendered + useGlobalTerminalListeners(); + // Stores const projects = useProjectStore((state) => state.projects); const selectedProjectId = useProjectStore((state) => state.selectedProjectId); diff --git a/apps/frontend/src/renderer/components/Terminal.tsx b/apps/frontend/src/renderer/components/Terminal.tsx index c1d64168..570266a7 100644 --- a/apps/frontend/src/renderer/components/Terminal.tsx +++ b/apps/frontend/src/renderer/components/Terminal.tsx @@ -103,7 +103,7 @@ export function Terminal({ const { terminalRef, xtermRef: _xtermRef, - write, + write: _write, // Output now handled by useGlobalTerminalListeners writeln, focus, dispose, @@ -153,14 +153,11 @@ export function Terminal({ }, }); - // Handle terminal events + // Handle terminal events (output is now handled globally via useGlobalTerminalListeners) useTerminalEvents({ terminalId: id, // Pass recreation ref to skip auto-removal during deliberate terminal recreation isRecreatingRef, - onOutput: (data) => { - write(data); - }, onExit: (exitCode) => { isCreatedRef.current = false; writeln(`\r\n\x1b[90mProcess exited with code ${exitCode}\x1b[0m`); diff --git a/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts b/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts index 6fed5db1..0fd26af7 100644 --- a/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts +++ b/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts @@ -1,13 +1,11 @@ import { useEffect, useRef, type RefObject } from 'react'; import { useTerminalStore } from '../../stores/terminal-store'; -import { terminalBufferManager } from '../../lib/terminal-buffer-manager'; interface UseTerminalEventsOptions { terminalId: string; // Track deliberate recreation scenarios (e.g., worktree switching) // When true, skips auto-removal to allow proper recreation isRecreatingRef?: RefObject; - onOutput?: (data: string) => void; onExit?: (exitCode: number) => void; onTitleChange?: (title: string) => void; onClaudeSession?: (sessionId: string) => void; @@ -16,23 +14,17 @@ interface UseTerminalEventsOptions { export function useTerminalEvents({ terminalId, isRecreatingRef, - onOutput, onExit, onTitleChange, onClaudeSession, }: UseTerminalEventsOptions) { // Use refs to always have the latest callbacks without re-registering listeners // This prevents duplicate listener registration when callbacks change identity - const onOutputRef = useRef(onOutput); const onExitRef = useRef(onExit); const onTitleChangeRef = useRef(onTitleChange); const onClaudeSessionRef = useRef(onClaudeSession); // Keep refs updated with latest callbacks - useEffect(() => { - onOutputRef.current = onOutput; - }, [onOutput]); - useEffect(() => { onExitRef.current = onExit; }, [onExit]); @@ -45,19 +37,6 @@ export function useTerminalEvents({ onClaudeSessionRef.current = onClaudeSession; }, [onClaudeSession]); - // Handle terminal output from main process - // Only depends on terminalId (stable) to prevent listener re-registration - useEffect(() => { - const cleanup = window.electronAPI.onTerminalOutput((id, data) => { - if (id === terminalId) { - terminalBufferManager.append(terminalId, data); - onOutputRef.current?.(data); - } - }); - - return cleanup; - }, [terminalId]); - // Handle terminal exit useEffect(() => { const cleanup = window.electronAPI.onTerminalExit((id, exitCode) => { @@ -100,7 +79,7 @@ export function useTerminalEvents({ }); return cleanup; - }, [terminalId]); + }, [terminalId, isRecreatingRef]); // Handle terminal title change useEffect(() => { diff --git a/apps/frontend/src/renderer/components/terminal/useXterm.ts b/apps/frontend/src/renderer/components/terminal/useXterm.ts index 6412d971..001b0fa4 100644 --- a/apps/frontend/src/renderer/components/terminal/useXterm.ts +++ b/apps/frontend/src/renderer/components/terminal/useXterm.ts @@ -4,6 +4,7 @@ import { FitAddon } from '@xterm/addon-fit'; import { WebLinksAddon } from '@xterm/addon-web-links'; import { SerializeAddon } from '@xterm/addon-serialize'; import { terminalBufferManager } from '../../lib/terminal-buffer-manager'; +import { registerOutputCallback, unregisterOutputCallback } from '../../stores/terminal-store'; // Type augmentation for navigator.userAgentData (modern User-Agent Client Hints API) interface NavigatorUAData { @@ -289,6 +290,28 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea }; }, [terminalId, onCommandEnter, onResize, onDimensionsReady]); + // Register xterm write callback with terminal-store for global output listener + // This allows the global listener to write directly to xterm when terminal is visible + useEffect(() => { + // Only register if xterm is ready + if (!xtermRef.current) return; + + // Create a write function that writes directly to this xterm instance + const writeCallback = (data: string) => { + if (xtermRef.current && !isDisposedRef.current) { + xtermRef.current.write(data); + } + }; + + // Register the callback so global listener can write to this terminal + registerOutputCallback(terminalId, writeCallback); + + // Cleanup: unregister callback when component unmounts + return () => { + unregisterOutputCallback(terminalId); + }; + }, [terminalId]); + // Handle resize on container resize with debouncing useEffect(() => { const handleResize = debounce(() => { diff --git a/apps/frontend/src/renderer/hooks/__tests__/useGlobalTerminalListeners.test.ts b/apps/frontend/src/renderer/hooks/__tests__/useGlobalTerminalListeners.test.ts new file mode 100644 index 00000000..9db5b671 --- /dev/null +++ b/apps/frontend/src/renderer/hooks/__tests__/useGlobalTerminalListeners.test.ts @@ -0,0 +1,387 @@ +/** + * @vitest-environment jsdom + */ + +/** + * Unit tests for useGlobalTerminalListeners hook + * Tests global terminal output listener registration and cleanup + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; + +// Mock terminal-store module +vi.mock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), +})); + +// Mock terminal-buffer-manager module +vi.mock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { + getSize: vi.fn(() => 100), + }, +})); + +// Mock debug-logger module +vi.mock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), +})); + +describe('useGlobalTerminalListeners', () => { + let mockOnTerminalOutput: ReturnType; + let mockCleanupFn: ReturnType; + let terminalOutputCallback: ((terminalId: string, data: string) => void) | null = null; + + beforeEach(() => { + vi.clearAllMocks(); + + // Reset the module-level globalCleanup by re-importing + // This ensures tests don't interfere with each other + terminalOutputCallback = null; + mockCleanupFn = vi.fn(); + + // Mock window.electronAPI.onTerminalOutput + mockOnTerminalOutput = vi.fn((callback: (terminalId: string, data: string) => void) => { + terminalOutputCallback = callback; + return mockCleanupFn; + }); + + // Ensure window and electronAPI exist + if (typeof window === 'undefined') { + (global as { window: unknown }).window = {}; + } + + (window as unknown as { electronAPI: { onTerminalOutput: typeof mockOnTerminalOutput } }).electronAPI = { + onTerminalOutput: mockOnTerminalOutput, + }; + }); + + afterEach(() => { + vi.restoreAllMocks(); + terminalOutputCallback = null; + }); + + describe('listener registration', () => { + it('should register global terminal output listener on mount', async () => { + // Need to reset the module to clear globalCleanup state + vi.resetModules(); + + // Re-mock after reset - use vi.fn() directly + const mockWriteToTerminal = vi.fn(); + const mockGetSize = vi.fn(() => 100); + const mockDebugLog = vi.fn(); + const mockDebugWarn = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: mockGetSize }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog, + debugWarn: mockDebugWarn, + })); + + // Re-import the hook after mocking + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + expect(mockOnTerminalOutput).toHaveBeenCalledTimes(1); + expect(mockOnTerminalOutput).toHaveBeenCalledWith(expect.any(Function)); + expect(mockDebugLog).toHaveBeenCalledWith( + '[GlobalTerminalListeners] Registering global terminal output listener' + ); + }); + + it('should skip registration if listener already registered', async () => { + vi.resetModules(); + + const mockDebugLog = vi.fn(); + const mockDebugWarn = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog, + debugWarn: mockDebugWarn, + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + // First mount + const { unmount: unmount1 } = renderHook(() => freshHook()); + + // Second mount without unmounting first - should skip registration + renderHook(() => freshHook()); + + // Should only register once + expect(mockOnTerminalOutput).toHaveBeenCalledTimes(1); + expect(mockDebugWarn).toHaveBeenCalledWith( + '[GlobalTerminalListeners] Listener already registered, skipping' + ); + + // Cleanup + unmount1(); + }); + }); + + describe('terminal output handling', () => { + it('should call writeToTerminal when output is received', async () => { + vi.resetModules(); + + const mockWriteToTerminal = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate terminal output + expect(terminalOutputCallback).not.toBeNull(); + terminalOutputCallback!('terminal-123', 'Hello, World!'); + + expect(mockWriteToTerminal).toHaveBeenCalledWith('terminal-123', 'Hello, World!'); + }); + + it('should log output processing with buffer size', async () => { + vi.resetModules(); + + const mockDebugLog = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog, + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate terminal output + terminalOutputCallback!('terminal-456', 'Test output'); + + expect(mockDebugLog).toHaveBeenCalledWith( + '[GlobalTerminalListeners] Processed output for terminal-456, buffer size: 100' + ); + }); + + it('should handle multiple terminals', async () => { + vi.resetModules(); + + const mockWriteToTerminal = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate output from multiple terminals + terminalOutputCallback!('terminal-1', 'Output 1'); + terminalOutputCallback!('terminal-2', 'Output 2'); + terminalOutputCallback!('terminal-3', 'Output 3'); + + expect(mockWriteToTerminal).toHaveBeenCalledTimes(3); + expect(mockWriteToTerminal).toHaveBeenNthCalledWith(1, 'terminal-1', 'Output 1'); + expect(mockWriteToTerminal).toHaveBeenNthCalledWith(2, 'terminal-2', 'Output 2'); + expect(mockWriteToTerminal).toHaveBeenNthCalledWith(3, 'terminal-3', 'Output 3'); + }); + }); + + describe('cleanup', () => { + it('should cleanup listener on unmount', async () => { + vi.resetModules(); + + const mockDebugLog = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog, + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + const { unmount } = renderHook(() => freshHook()); + + // Unmount + unmount(); + + expect(mockCleanupFn).toHaveBeenCalledTimes(1); + expect(mockDebugLog).toHaveBeenCalledWith( + '[GlobalTerminalListeners] Cleaning up global terminal output listener' + ); + }); + + it('should allow re-registration after cleanup', async () => { + vi.resetModules(); + + const mockDebugLog1 = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog1, + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + // First mount and unmount + const { unmount: unmount1 } = renderHook(() => freshHook()); + unmount1(); + + // Clear call counts + mockOnTerminalOutput.mockClear(); + + // Need to reset modules again to clear the globalCleanup state + vi.resetModules(); + + const mockDebugLog2 = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: vi.fn(), + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog2, + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook2 } = await import('../useGlobalTerminalListeners'); + + // Second mount should register successfully + renderHook(() => freshHook2()); + + expect(mockOnTerminalOutput).toHaveBeenCalledTimes(1); + expect(mockDebugLog2).toHaveBeenCalledWith( + '[GlobalTerminalListeners] Registering global terminal output listener' + ); + }); + }); + + describe('edge cases', () => { + it('should handle empty data string', async () => { + vi.resetModules(); + + const mockWriteToTerminal = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate empty output + terminalOutputCallback!('terminal-123', ''); + + expect(mockWriteToTerminal).toHaveBeenCalledWith('terminal-123', ''); + }); + + it('should handle special characters in terminal output', async () => { + vi.resetModules(); + + const mockWriteToTerminal = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate output with ANSI escape codes and special characters + const specialOutput = '\x1b[32mGreen text\x1b[0m\nNew line\t\ttabs'; + terminalOutputCallback!('terminal-123', specialOutput); + + expect(mockWriteToTerminal).toHaveBeenCalledWith('terminal-123', specialOutput); + }); + + it('should handle rapid successive outputs', async () => { + vi.resetModules(); + + const mockWriteToTerminal = vi.fn(); + + vi.doMock('../../stores/terminal-store', () => ({ + writeToTerminal: mockWriteToTerminal, + })); + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { getSize: vi.fn(() => 100) }, + })); + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugWarn: vi.fn(), + })); + + const { useGlobalTerminalListeners: freshHook } = await import('../useGlobalTerminalListeners'); + + renderHook(() => freshHook()); + + // Simulate rapid outputs + for (let i = 0; i < 100; i++) { + terminalOutputCallback!('terminal-123', `Line ${i}\n`); + } + + expect(mockWriteToTerminal).toHaveBeenCalledTimes(100); + }); + }); +}); diff --git a/apps/frontend/src/renderer/hooks/useGlobalTerminalListeners.ts b/apps/frontend/src/renderer/hooks/useGlobalTerminalListeners.ts new file mode 100644 index 00000000..706e4a89 --- /dev/null +++ b/apps/frontend/src/renderer/hooks/useGlobalTerminalListeners.ts @@ -0,0 +1,69 @@ +import { useEffect } from 'react'; +import { writeToTerminal } from '../stores/terminal-store'; +import { terminalBufferManager } from '../lib/terminal-buffer-manager'; +import { debugLog, debugWarn } from '../../shared/utils/debug-logger'; + +/** + * Module-level cleanup function storage. + * + * DESIGN NOTE: This module-level variable is intentionally shared across all hook instances. + * This is acceptable because: + * 1. There's only one app instance that uses this hook (in App.tsx) + * 2. The listener needs to persist across component re-renders + * 3. Having a single global listener ensures all terminal output is captured + * regardless of which project is currently active or which terminals are rendered + * + * This pattern mirrors useIpc.ts where module-level state is used for IPC batching. + */ +let globalCleanup: (() => void) | null = null; + +/** + * Hook to set up global terminal output listeners that persist across project switches. + * + * This hook solves the terminal output freezing issue when switching between projects. + * The problem was that terminal output listeners were registered in useTerminalEvents.ts + * per-terminal component - when a terminal component unmounted (user switches project), + * the listener was removed and output stopped being buffered. + * + * By registering the listener at the app level (like useIpcListeners), we ensure: + * 1. Terminal output is ALWAYS buffered to terminalBufferManager, regardless of which + * project is active or which terminal components are mounted + * 2. When a terminal has a registered callback (visible), output is written to xterm immediately + * 3. When a terminal becomes visible again, it can replay the buffered output + * 4. No output is lost during project navigation + * + * This hook should be called once in App.tsx alongside useIpcListeners(). + */ +export function useGlobalTerminalListeners(): void { + useEffect(() => { + // Only register once - prevent duplicate listeners + if (globalCleanup) { + debugWarn('[GlobalTerminalListeners] Listener already registered, skipping'); + return; + } + + debugLog('[GlobalTerminalListeners] Registering global terminal output listener'); + + // Register global terminal output listener + // This listener runs for ALL terminals, regardless of which project is active + globalCleanup = window.electronAPI.onTerminalOutput((terminalId: string, data: string) => { + // Use writeToTerminal which: + // 1. Always buffers to terminalBufferManager for persistence + // 2. Writes to xterm immediately if terminal has a registered callback (visible) + writeToTerminal(terminalId, data); + + debugLog( + `[GlobalTerminalListeners] Processed output for ${terminalId}, buffer size: ${terminalBufferManager.getSize(terminalId)}` + ); + }); + + // Cleanup on unmount (app shutdown) + return () => { + if (globalCleanup) { + debugLog('[GlobalTerminalListeners] Cleaning up global terminal output listener'); + globalCleanup(); + globalCleanup = null; + } + }; + }, []); // Empty deps - only run once on mount +} diff --git a/apps/frontend/src/renderer/stores/__tests__/terminal-store.callbacks.test.ts b/apps/frontend/src/renderer/stores/__tests__/terminal-store.callbacks.test.ts new file mode 100644 index 00000000..5fa26368 --- /dev/null +++ b/apps/frontend/src/renderer/stores/__tests__/terminal-store.callbacks.test.ts @@ -0,0 +1,383 @@ +/** + * @vitest-environment jsdom + */ + +/** + * Unit tests for terminal-store callback registration functions + * Tests registerOutputCallback, unregisterOutputCallback, and writeToTerminal + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +// Mock terminal-buffer-manager module +vi.mock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { + append: vi.fn(), + getSize: vi.fn(() => 100), + get: vi.fn(() => ''), + set: vi.fn(), + clear: vi.fn(), + dispose: vi.fn(), + }, +})); + +// Mock debug-logger module +vi.mock('../../../shared/utils/debug-logger', () => ({ + debugLog: vi.fn(), + debugError: vi.fn(), +})); + +// Mock uuid for zustand store +vi.mock('uuid', () => ({ + v4: vi.fn(() => 'mock-uuid-1234'), +})); + +// Mock @dnd-kit/sortable for zustand store +vi.mock('@dnd-kit/sortable', () => ({ + arrayMove: vi.fn((arr, from, to) => { + const result = [...arr]; + const [item] = result.splice(from, 1); + result.splice(to, 0, item); + return result; + }), +})); + +describe('terminal-store callback registration functions', () => { + let registerOutputCallback: typeof import('../terminal-store').registerOutputCallback; + let unregisterOutputCallback: typeof import('../terminal-store').unregisterOutputCallback; + let writeToTerminal: typeof import('../terminal-store').writeToTerminal; + let mockTerminalBufferManager: { + append: ReturnType; + getSize: ReturnType; + }; + let mockDebugLog: ReturnType; + let mockDebugError: ReturnType; + + beforeEach(async () => { + vi.clearAllMocks(); + vi.resetModules(); + + // Re-mock after reset to ensure fresh state + mockDebugLog = vi.fn(); + mockDebugError = vi.fn(); + + vi.doMock('../../../shared/utils/debug-logger', () => ({ + debugLog: mockDebugLog, + debugError: mockDebugError, + })); + + mockTerminalBufferManager = { + append: vi.fn(), + getSize: vi.fn(() => 100), + }; + + vi.doMock('../../lib/terminal-buffer-manager', () => ({ + terminalBufferManager: { + ...mockTerminalBufferManager, + get: vi.fn(() => ''), + set: vi.fn(), + clear: vi.fn(), + dispose: vi.fn(), + }, + })); + + vi.doMock('uuid', () => ({ + v4: vi.fn(() => 'mock-uuid-1234'), + })); + + vi.doMock('@dnd-kit/sortable', () => ({ + arrayMove: vi.fn((arr: unknown[], from: number, to: number) => { + const result = [...arr]; + const [item] = result.splice(from, 1); + result.splice(to, 0, item); + return result; + }), + })); + + // Import fresh module + const storeModule = await import('../terminal-store'); + registerOutputCallback = storeModule.registerOutputCallback; + unregisterOutputCallback = storeModule.unregisterOutputCallback; + writeToTerminal = storeModule.writeToTerminal; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('registerOutputCallback', () => { + it('should store callback for terminal ID', async () => { + const callback = vi.fn(); + + registerOutputCallback('terminal-123', callback); + + expect(mockDebugLog).toHaveBeenCalledWith( + '[TerminalStore] Registered output callback for terminal: terminal-123' + ); + }); + + it('should overwrite existing callback when registering same terminal ID', async () => { + const callback1 = vi.fn(); + const callback2 = vi.fn(); + + registerOutputCallback('terminal-123', callback1); + registerOutputCallback('terminal-123', callback2); + + // Both registrations should log + expect(mockDebugLog).toHaveBeenCalledTimes(2); + + // Write should only call the latest callback + writeToTerminal('terminal-123', 'test'); + expect(callback1).not.toHaveBeenCalled(); + expect(callback2).toHaveBeenCalledWith('test'); + }); + + it('should support multiple terminal callbacks simultaneously', async () => { + const callback1 = vi.fn(); + const callback2 = vi.fn(); + const callback3 = vi.fn(); + + registerOutputCallback('terminal-1', callback1); + registerOutputCallback('terminal-2', callback2); + registerOutputCallback('terminal-3', callback3); + + // Write to each terminal + writeToTerminal('terminal-1', 'data1'); + writeToTerminal('terminal-2', 'data2'); + writeToTerminal('terminal-3', 'data3'); + + expect(callback1).toHaveBeenCalledWith('data1'); + expect(callback2).toHaveBeenCalledWith('data2'); + expect(callback3).toHaveBeenCalledWith('data3'); + }); + }); + + describe('unregisterOutputCallback', () => { + it('should remove callback for terminal ID', async () => { + const callback = vi.fn(); + + registerOutputCallback('terminal-123', callback); + unregisterOutputCallback('terminal-123'); + + expect(mockDebugLog).toHaveBeenCalledWith( + '[TerminalStore] Unregistered output callback for terminal: terminal-123' + ); + + // Writing after unregistration should not call callback + writeToTerminal('terminal-123', 'test'); + expect(callback).not.toHaveBeenCalled(); + }); + + it('should handle unregistering non-existent terminal ID gracefully', async () => { + // Should not throw + expect(() => { + unregisterOutputCallback('non-existent-terminal'); + }).not.toThrow(); + + expect(mockDebugLog).toHaveBeenCalledWith( + '[TerminalStore] Unregistered output callback for terminal: non-existent-terminal' + ); + }); + + it('should only unregister specified terminal ID', async () => { + const callback1 = vi.fn(); + const callback2 = vi.fn(); + + registerOutputCallback('terminal-1', callback1); + registerOutputCallback('terminal-2', callback2); + + unregisterOutputCallback('terminal-1'); + + // terminal-1 callback should not be called + writeToTerminal('terminal-1', 'data1'); + expect(callback1).not.toHaveBeenCalled(); + + // terminal-2 callback should still work + writeToTerminal('terminal-2', 'data2'); + expect(callback2).toHaveBeenCalledWith('data2'); + }); + }); + + describe('writeToTerminal', () => { + it('should call callback when registered', async () => { + const callback = vi.fn(); + + registerOutputCallback('terminal-123', callback); + writeToTerminal('terminal-123', 'Hello, World!'); + + expect(callback).toHaveBeenCalledWith('Hello, World!'); + }); + + it('should always buffer data via terminalBufferManager', async () => { + const callback = vi.fn(); + + // Without callback registered + writeToTerminal('terminal-no-callback', 'data1'); + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-no-callback', 'data1'); + + // With callback registered + registerOutputCallback('terminal-with-callback', callback); + writeToTerminal('terminal-with-callback', 'data2'); + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-with-callback', 'data2'); + }); + + it('should buffer but not call callback when not registered', async () => { + // Write to terminal without registered callback + writeToTerminal('unregistered-terminal', 'buffered-data'); + + // Data should be buffered + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith( + 'unregistered-terminal', + 'buffered-data' + ); + }); + + it('should handle callback errors gracefully', async () => { + const errorCallback = vi.fn(() => { + throw new Error('Callback error'); + }); + + registerOutputCallback('terminal-error', errorCallback); + + // Should not throw + expect(() => { + writeToTerminal('terminal-error', 'test'); + }).not.toThrow(); + + // Error should be logged + expect(mockDebugError).toHaveBeenCalledWith( + '[TerminalStore] Error writing to terminal terminal-error:', + expect.any(Error) + ); + + // Data should still be buffered + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-error', 'test'); + }); + + it('should handle empty data string', async () => { + const callback = vi.fn(); + + registerOutputCallback('terminal-123', callback); + writeToTerminal('terminal-123', ''); + + expect(callback).toHaveBeenCalledWith(''); + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-123', ''); + }); + + it('should handle special characters and ANSI codes', async () => { + const callback = vi.fn(); + const specialData = '\x1b[32mGreen text\x1b[0m\nNew line\t\ttabs\r\nCRLF'; + + registerOutputCallback('terminal-123', callback); + writeToTerminal('terminal-123', specialData); + + expect(callback).toHaveBeenCalledWith(specialData); + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-123', specialData); + }); + + it('should handle large data chunks', async () => { + const callback = vi.fn(); + const largeData = 'x'.repeat(100000); // 100KB of data + + registerOutputCallback('terminal-123', callback); + writeToTerminal('terminal-123', largeData); + + expect(callback).toHaveBeenCalledWith(largeData); + expect(mockTerminalBufferManager.append).toHaveBeenCalledWith('terminal-123', largeData); + }); + + it('should handle rapid successive writes', async () => { + const callback = vi.fn(); + + registerOutputCallback('terminal-123', callback); + + // Simulate rapid writes + for (let i = 0; i < 100; i++) { + writeToTerminal('terminal-123', `Line ${i}\n`); + } + + expect(callback).toHaveBeenCalledTimes(100); + expect(mockTerminalBufferManager.append).toHaveBeenCalledTimes(100); + }); + }); + + describe('callback lifecycle', () => { + it('should support register -> write -> unregister -> write flow', async () => { + const callback = vi.fn(); + + // Register callback + registerOutputCallback('terminal-123', callback); + + // First write should call callback + writeToTerminal('terminal-123', 'first'); + expect(callback).toHaveBeenCalledWith('first'); + expect(callback).toHaveBeenCalledTimes(1); + + // Unregister callback + unregisterOutputCallback('terminal-123'); + + // Second write should NOT call callback + writeToTerminal('terminal-123', 'second'); + expect(callback).toHaveBeenCalledTimes(1); // Still 1 + + // But data should still be buffered + expect(mockTerminalBufferManager.append).toHaveBeenCalledTimes(2); + }); + + it('should support re-registration after unregister', async () => { + const callback1 = vi.fn(); + const callback2 = vi.fn(); + + // Register first callback + registerOutputCallback('terminal-123', callback1); + writeToTerminal('terminal-123', 'first'); + expect(callback1).toHaveBeenCalledWith('first'); + + // Unregister + unregisterOutputCallback('terminal-123'); + + // Register new callback + registerOutputCallback('terminal-123', callback2); + writeToTerminal('terminal-123', 'second'); + + // Only new callback should receive data + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith('second'); + }); + }); + + describe('concurrent terminal operations', () => { + it('should handle interleaved operations on multiple terminals', async () => { + const callbacks = { + t1: vi.fn(), + t2: vi.fn(), + t3: vi.fn(), + }; + + // Register all callbacks + registerOutputCallback('t1', callbacks.t1); + registerOutputCallback('t2', callbacks.t2); + registerOutputCallback('t3', callbacks.t3); + + // Interleaved writes + writeToTerminal('t1', 'a1'); + writeToTerminal('t2', 'b1'); + writeToTerminal('t1', 'a2'); + writeToTerminal('t3', 'c1'); + writeToTerminal('t2', 'b2'); + + // Unregister one in the middle + unregisterOutputCallback('t2'); + + writeToTerminal('t1', 'a3'); + writeToTerminal('t2', 'b3'); // Should not call callback + writeToTerminal('t3', 'c2'); + + expect(callbacks.t1).toHaveBeenCalledTimes(3); + expect(callbacks.t2).toHaveBeenCalledTimes(2); // b1, b2 only + expect(callbacks.t3).toHaveBeenCalledTimes(2); + + // All data should be buffered + expect(mockTerminalBufferManager.append).toHaveBeenCalledTimes(8); + }); + }); +}); diff --git a/apps/frontend/src/renderer/stores/terminal-store.ts b/apps/frontend/src/renderer/stores/terminal-store.ts index 263edb82..7c671913 100644 --- a/apps/frontend/src/renderer/stores/terminal-store.ts +++ b/apps/frontend/src/renderer/stores/terminal-store.ts @@ -5,6 +5,77 @@ import type { TerminalSession, TerminalWorktreeConfig } from '../../shared/types import { terminalBufferManager } from '../lib/terminal-buffer-manager'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +/** + * Module-level Map to store terminal ID -> xterm write callback mappings. + * + * DESIGN NOTE: This is stored outside of Zustand state because: + * 1. Callbacks are functions and shouldn't be serialized in state + * 2. The callbacks need to be accessible from the global terminal listener + * 3. Registration/unregistration happens on terminal mount/unmount, not state changes + * + * When a terminal component mounts, it registers its xterm.write function here. + * When the global terminal output listener receives data, it calls the callback + * if registered (terminal is visible), otherwise just buffers the data. + * This allows output to be written to xterm immediately when visible, while + * still buffering when the terminal is not rendered (project switched away). + */ +const xtermCallbacks = new Map void>(); + +/** + * Register an xterm write callback for a terminal. + * Called when a terminal component mounts and xterm is ready. + * + * @param terminalId - The terminal ID + * @param callback - Function to write data to xterm instance + */ +export function registerOutputCallback( + terminalId: string, + callback: (data: string) => void +): void { + xtermCallbacks.set(terminalId, callback); + debugLog(`[TerminalStore] Registered output callback for terminal: ${terminalId}`); +} + +/** + * Unregister an xterm write callback for a terminal. + * Called when a terminal component unmounts. + * + * @param terminalId - The terminal ID + */ +export function unregisterOutputCallback(terminalId: string): void { + xtermCallbacks.delete(terminalId); + debugLog(`[TerminalStore] Unregistered output callback for terminal: ${terminalId}`); +} + +/** + * Write terminal output to the appropriate destination. + * + * If the terminal has a registered callback (component is mounted and visible), + * writes directly to xterm AND buffers. If no callback is registered (terminal + * component is unmounted due to project switch), only buffers the data. + * + * This function is called by the global terminal output listener in + * useGlobalTerminalListeners, which ensures output is always captured + * regardless of which project is currently active. + * + * @param terminalId - The terminal ID + * @param data - The output data to write + */ +export function writeToTerminal(terminalId: string, data: string): void { + // Always buffer the data to ensure persistence + terminalBufferManager.append(terminalId, data); + + // If terminal has a registered callback, write to xterm immediately + const callback = xtermCallbacks.get(terminalId); + if (callback) { + try { + callback(data); + } catch (error) { + debugError(`[TerminalStore] Error writing to terminal ${terminalId}:`, error); + } + } +} + export type TerminalStatus = 'idle' | 'running' | 'claude-active' | 'exited'; export interface Terminal { @@ -174,8 +245,9 @@ export const useTerminalStore = create((set, get) => ({ }, removeTerminal: (id: string) => { - // Clean up buffer manager + // Clean up buffer manager and output callback terminalBufferManager.dispose(id); + xtermCallbacks.delete(id); set((state) => { const newTerminals = state.terminals.filter((t) => t.id !== id);