fix: enforce 12 terminal limit per project (#1264)

* fix 12 max limit terminals per project

* fix(tests): address PR review findings and CI failures

- Extract duplicated terminal counting logic to helper function
  (getActiveProjectTerminalCount) to improve maintainability
- Add comprehensive rationale comment for 12-terminal limit
  explaining memory/resource constraints
- Add debug logging when terminal limit is reached for better
  observability
- Document that addRestoredTerminal intentionally bypasses limit
  to preserve user state from previous sessions
- Fix macOS test failure: use globalThis instead of window in
  requestAnimationFrame mock (terminal-copy-paste.test.ts)
- Fix Windows test timeouts: add 15000ms timeouts to all
  subprocess-spawn tests for slower CI environments
- Increase PRDetail.integration.test.tsx timeout to 15000ms

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

apps/frontend/src/__tests__/integration/subprocess-spawn.test.ts

@@ -253,7 +253,7 @@ describe('Subprocess Spawn Integration', () => {
         ]),
         expect.any(Object)
       );
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should emit log events from stdout', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -269,7 +269,7 @@ describe('Subprocess Spawn Integration', () => {
       mockStdout.emit('data', Buffer.from('Test log output\n'));
 
       expect(logHandler).toHaveBeenCalledWith('task-1', 'Test log output\n');
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should emit log events from stderr', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -285,7 +285,7 @@ describe('Subprocess Spawn Integration', () => {
       mockStderr.emit('data', Buffer.from('Progress: 50%\n'));
 
       expect(logHandler).toHaveBeenCalledWith('task-1', 'Progress: 50%\n');
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should emit exit event when process exits', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -302,7 +302,7 @@ describe('Subprocess Spawn Integration', () => {
 
       // Exit event includes taskId, exit code, and process type
       expect(exitHandler).toHaveBeenCalledWith('task-1', 0, expect.any(String));
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should emit error event when process errors', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -318,7 +318,7 @@ describe('Subprocess Spawn Integration', () => {
       mockProcess.emit('error', new Error('Spawn failed'));
 
       expect(errorHandler).toHaveBeenCalledWith('task-1', 'Spawn failed');
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should kill task and remove from tracking', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -339,7 +339,7 @@ describe('Subprocess Spawn Integration', () => {
         expect(mockProcess.kill).toHaveBeenCalledWith('SIGTERM');
       }
       expect(manager.isRunning('task-1')).toBe(false);
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should return false when killing non-existent task', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -348,7 +348,7 @@ describe('Subprocess Spawn Integration', () => {
       const result = manager.killTask('nonexistent');
 
       expect(result).toBe(false);
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should track running tasks', async () => {
       const { AgentManager } = await import('../../main/agent');
@@ -391,7 +391,7 @@ describe('Subprocess Spawn Integration', () => {
         expect.any(Array),
         expect.any(Object)
       );
-    });
+    }, 15000);  // Increase timeout for Windows CI
 
     it('should kill all running tasks', async () => {
       const { AgentManager } = await import('../../main/agent');

apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts

@@ -80,7 +80,8 @@ describe('Terminal copy/paste integration', () => {
     // Mock requestAnimationFrame for xterm.js integration tests
     global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
       // Synchronously execute the callback to avoid timing issues in tests
-      callback.call(window, 0);
+      // Use globalThis instead of window for cross-platform compatibility
+      callback.call(globalThis, 0);
       return 0;
     }) as unknown as Mock;
 

apps/frontend/src/renderer/components/github-prs/components/__tests__/PRDetail.integration.test.tsx

@@ -185,7 +185,7 @@ describe('PRDetail - Clean Review State Reset Integration', () => {
     const postCleanReviewButtonAfterChange = screen.queryByRole('button', { name: /post clean review/i });
     expect(postCleanReviewButtonAfterChange).toBeInTheDocument();
     unmount();
-  });
+  }, 15000); // Increased timeout for slower CI environments (Windows)
 
   it('should show clean review success message after posting clean review', async () => {
     const { unmount } = renderPRDetail();

apps/frontend/src/renderer/stores/terminal-store.ts

@@ -138,16 +138,33 @@ interface TerminalState {
   getWorktreeCount: () => number;
 }
 
+/**
+ * Helper function to count active (non-exited) terminals for a specific project.
+ * Extracted to avoid duplicating the counting logic across multiple methods.
+ *
+ * @param terminals - The array of all terminals
+ * @param projectPath - The project path to filter by
+ * @returns The count of active terminals for the given project
+ */
+function getActiveProjectTerminalCount(terminals: Terminal[], projectPath?: string): number {
+  return terminals.filter(t => t.status !== 'exited' && t.projectPath === projectPath).length;
+}
+
 export const useTerminalStore = create<TerminalState>((set, get) => ({
   terminals: [],
   layouts: [],
   activeTerminalId: null,
-  maxTerminals: Infinity, // No limit on terminals
+  // Maximum terminals per project - limited to 12 to prevent excessive memory usage
+  // from terminal buffers (~1MB each) and PTY process resource exhaustion.
+  // Each terminal maintains a scrollback buffer and associated xterm.js state.
+  maxTerminals: 12,
   hasRestoredSessions: false,
 
   addTerminal: (cwd?: string, projectPath?: string) => {
     const state = get();
-    if (state.terminals.length >= state.maxTerminals) {
+    const activeCount = getActiveProjectTerminalCount(state.terminals, projectPath);
+    if (activeCount >= state.maxTerminals) {
+      debugLog(`[TerminalStore] Cannot add terminal: limit of ${state.maxTerminals} reached for project ${projectPath}`);
       return null;
     }
 
@@ -180,6 +197,11 @@ export const useTerminalStore = create<TerminalState>((set, get) => ({
       return existingTerminal;
     }
 
+    // NOTE: Restored terminals are intentionally exempt from the per-project limit.
+    // This preserves user state from previous sessions - if a user had 12 terminals
+    // before closing the app, they should get all 12 back on restore.
+    // The limit only applies to newly created terminals.
+
     const restoredTerminal: Terminal = {
       id: session.id,
       title: session.title,
@@ -223,10 +245,9 @@ export const useTerminalStore = create<TerminalState>((set, get) => ({
       return existingTerminal;
     }
 
-    // Use the same logic as canAddTerminal - count only non-exited terminals
-    // This ensures consistency and doesn't block new terminals when only exited ones exist
-    const activeTerminalCount = state.terminals.filter(t => t.status !== 'exited').length;
-    if (activeTerminalCount >= state.maxTerminals) {
+    const activeCount = getActiveProjectTerminalCount(state.terminals, projectPath);
+    if (activeCount >= state.maxTerminals) {
+      debugLog(`[TerminalStore] Cannot add external terminal: limit of ${state.maxTerminals} reached for project ${projectPath}`);
       return null;
     }
 
@@ -384,17 +405,7 @@ export const useTerminalStore = create<TerminalState>((set, get) => ({
 
   canAddTerminal: (projectPath?: string) => {
     const state = get();
-    // Count only non-exited terminals, optionally filtered by project
-    const activeTerminals = state.terminals.filter(t => {
-      // Exclude exited terminals from the count
-      if (t.status === 'exited') return false;
-      // If projectPath specified, only count terminals for that project (or legacy without projectPath)
-      if (projectPath) {
-        return t.projectPath === projectPath || !t.projectPath;
-      }
-      return true;
-    });
-    return activeTerminals.length < state.maxTerminals;
+    return getActiveProjectTerminalCount(state.terminals, projectPath) < state.maxTerminals;
   },
 
   getTerminalsForProject: (projectPath: string) => {