From f2e43edcc309a1306ee95fade63a4d4132d21b95 Mon Sep 17 00:00:00 2001 From: Luiggi Date: Sat, 25 Apr 2026 18:40:46 +0200 Subject: [PATCH 1/8] fix: prevent infinite PTY recreation loop on terminal natural exit When the shell exited naturally (e.g. typing `exit`), the terminal would enter an infinite mount/unmount cycle. TerminalGrid's pendingCleanup grace period (150ms) kept remounting the Terminal component while its status was still 'exited', causing usePtyProcess to spin up a new PTY on each mount before the previous one could complete Guard: skip PTY creation when status === 'exited' and isRecreatingRef is not set, which is the deliberate-recreation path (worktree switch) Adds usePtyProcess.test.ts with 6 cases covering the guard, the recreation exception, and the normal creation path --- .../terminal/__tests__/usePtyProcess.test.ts | 141 ++++++++++++++++++ .../components/terminal/usePtyProcess.ts | 24 +++ 2 files changed, 165 insertions(+) create mode 100644 apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts new file mode 100644 index 0000000000..865c9806e1 --- /dev/null +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -0,0 +1,141 @@ +/** + * @vitest-environment jsdom + */ + +/** + * Unit tests for usePtyProcess + * + * Covers the guard that prevents PTY creation when a terminal has exited + * naturally (status === 'exited'), which was causing an infinite + * mount → create PTY → unmount → mount loop via the TerminalGrid + * pendingCleanup grace-period mechanism. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { useRef } from 'react'; +import { usePtyProcess } from '../usePtyProcess'; + +const mockCreateTerminal = vi.fn(); +const mockDestroyTerminal = vi.fn(); +const mockRestoreTerminalSession = vi.fn(); + +vi.stubGlobal('window', { + electronAPI: { + createTerminal: mockCreateTerminal, + destroyTerminal: mockDestroyTerminal, + restoreTerminalSession: mockRestoreTerminalSession, + }, +}); + +/** Mutable terminal state controlled per test */ +let mockTerminalStatus: string = 'idle'; +let mockIsRestored: boolean = false; + +const mockSetTerminalStatus = vi.fn(); +const mockGetTerminal = vi.fn(); + +vi.mock('../../../stores/terminal-store', () => ({ + useTerminalStore: Object.assign(vi.fn(), { + getState: () => ({ + terminals: [ + { + id: 'term-1', + status: mockTerminalStatus, + isRestored: mockIsRestored, + isCLIMode: false, + cwd: '/test', + }, + ], + setTerminalStatus: mockSetTerminalStatus, + getTerminal: mockGetTerminal, + }), + }), +})); + +const DEFAULT_OPTIONS = { + terminalId: 'term-1', + cwd: '/test', + projectPath: '/test', + cols: 80, + rows: 24, + skipCreation: false, +} as const; + +describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recreation-loop)', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockTerminalStatus = 'idle'; + mockIsRestored = false; + mockCreateTerminal.mockResolvedValue({ success: true }); + mockDestroyTerminal.mockResolvedValue({ success: true }); + }); + + it('does not call createTerminal when terminal status is exited (natural exit)', async () => { + mockTerminalStatus = 'exited'; + + await act(async () => { + renderHook(() => usePtyProcess(DEFAULT_OPTIONS)); + }); + + expect(mockCreateTerminal).not.toHaveBeenCalled(); + }); + + it('does not call createTerminal on repeated mounts when status remains exited', async () => { + mockTerminalStatus = 'exited'; + + for (let i = 0; i < 3; i++) { + await act(async () => { + const { unmount } = renderHook(() => usePtyProcess(DEFAULT_OPTIONS)); + unmount(); + }); + } + + expect(mockCreateTerminal).not.toHaveBeenCalled(); + }); + + it('allows PTY creation when status is exited but isRecreatingRef is true (worktree switch)', async () => { + mockTerminalStatus = 'exited'; + + await act(async () => { + renderHook(() => { + const isRecreatingRef = useRef(true); + return usePtyProcess({ ...DEFAULT_OPTIONS, isRecreatingRef }); + }); + }); + + expect(mockCreateTerminal).toHaveBeenCalledTimes(1); + }); + + it('calls createTerminal when terminal status is idle', async () => { + mockTerminalStatus = 'idle'; + + await act(async () => { + renderHook(() => usePtyProcess(DEFAULT_OPTIONS)); + }); + + expect(mockCreateTerminal).toHaveBeenCalledTimes(1); + expect(mockCreateTerminal).toHaveBeenCalledWith( + expect.objectContaining({ id: 'term-1', cwd: '/test', cols: 80, rows: 24 }), + ); + }); + + it('calls createTerminal when terminal status is running (e.g. reconnect)', async () => { + mockTerminalStatus = 'running'; + + await act(async () => { + renderHook(() => usePtyProcess(DEFAULT_OPTIONS)); + }); + + expect(mockCreateTerminal).toHaveBeenCalledTimes(1); + }); + + it('does not call createTerminal when skipCreation is true', async () => { + mockTerminalStatus = 'idle'; + + await act(async () => { + renderHook(() => usePtyProcess({ ...DEFAULT_OPTIONS, skipCreation: true })); + }); + + expect(mockCreateTerminal).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts b/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts index c75465edb3..730c8d7833 100644 --- a/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts +++ b/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts @@ -23,6 +23,18 @@ interface UsePtyProcessOptions { onError?: (error: string) => void; } +/** + * Manages the PTY process lifecycle for a terminal instance. + * + * Creates and tracks the underlying PTY process, handling: + * - Initial creation once xterm has measured valid dimensions (`skipCreation`) + * - Session restoration for terminals persisted across hot-reloads + * - Deliberate recreation after worktree switches (`isRecreatingRef`) + * - Retry logic when dimensions are not yet ready during recreation + * + * @returns `prepareForRecreate` / `resetForRecreate` callbacks used by + * `Terminal.tsx` to coordinate controlled PTY destruction and recreation. + */ export function usePtyProcess({ terminalId, cwd, @@ -140,6 +152,18 @@ export function usePtyProcess({ const alreadyRunning = terminalState?.status === 'running' || terminalState?.status === 'claude-active'; const isRestored = terminalState?.isRestored; + // Do not create a new PTY for a terminal that has exited naturally. + // When the shell exits (e.g. user types "exit"), the terminal is kept in the DOM + // for a short grace period (pendingCleanup). During that period the component can + // mount/unmount rapidly — without this guard each mount would spin up a new PTY, + // causing an infinite mount → create PTY → unmount → mount loop. + // Deliberate recreation (worktree switch) sets isRecreatingRef before exiting, so + // it is excluded from this guard. + if (terminalState?.status === 'exited' && !isRecreatingRef?.current) { + debugLog(`[usePtyProcess] Skipping PTY creation for terminal: ${terminalId} - terminal has exited naturally (status=exited)`); + return; + } + debugLog(`[usePtyProcess] Starting PTY creation for terminal: ${terminalId}`); debugLog(`[usePtyProcess] Terminal ${terminalId} state: isRestored=${isRestored}, status=${terminalState?.status}`); debugLog(`[usePtyProcess] Terminal ${terminalId} dimensions for PTY: cols=${cols}, rows=${rows}`); From 73dbba9c50645544ecfc5bf7d1c6d9cdf9a6d8c5 Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Sat, 25 Apr 2026 19:55:44 +0200 Subject: [PATCH 2/8] fix: also skip PTY creation when terminal is not in store --- .../src/renderer/components/terminal/usePtyProcess.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts b/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts index 730c8d7833..6f4fd64f6e 100644 --- a/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts +++ b/apps/desktop/src/renderer/components/terminal/usePtyProcess.ts @@ -152,15 +152,16 @@ export function usePtyProcess({ const alreadyRunning = terminalState?.status === 'running' || terminalState?.status === 'claude-active'; const isRestored = terminalState?.isRestored; - // Do not create a new PTY for a terminal that has exited naturally. + // Do not create a new PTY when the terminal has exited naturally or is no longer + // in the store (removed while the component was still mounted). // When the shell exits (e.g. user types "exit"), the terminal is kept in the DOM // for a short grace period (pendingCleanup). During that period the component can // mount/unmount rapidly — without this guard each mount would spin up a new PTY, // causing an infinite mount → create PTY → unmount → mount loop. // Deliberate recreation (worktree switch) sets isRecreatingRef before exiting, so // it is excluded from this guard. - if (terminalState?.status === 'exited' && !isRecreatingRef?.current) { - debugLog(`[usePtyProcess] Skipping PTY creation for terminal: ${terminalId} - terminal has exited naturally (status=exited)`); + if (!terminalState || (terminalState.status === 'exited' && !isRecreatingRef?.current)) { + debugLog(`[usePtyProcess] Skipping PTY creation for terminal: ${terminalId} - terminal has exited naturally or is not in store (status=${terminalState?.status})`); return; } From 7e378247ecdc586ceec967a151b90888b920305d Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Sat, 25 Apr 2026 19:55:47 +0200 Subject: [PATCH 3/8] test: tighten TerminalStatus type and assert status reset on recreation --- .../components/terminal/__tests__/usePtyProcess.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index 865c9806e1..08582a4a2c 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import { useRef } from 'react'; import { usePtyProcess } from '../usePtyProcess'; +import type { TerminalStatus } from '../../../stores/terminal-store'; const mockCreateTerminal = vi.fn(); const mockDestroyTerminal = vi.fn(); @@ -28,7 +29,7 @@ vi.stubGlobal('window', { }); /** Mutable terminal state controlled per test */ -let mockTerminalStatus: string = 'idle'; +let mockTerminalStatus: TerminalStatus = 'idle'; let mockIsRestored: boolean = false; const mockSetTerminalStatus = vi.fn(); @@ -104,6 +105,8 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea }); expect(mockCreateTerminal).toHaveBeenCalledTimes(1); + // The recreation path resets status from 'exited' → 'idle' before creating the PTY + expect(mockSetTerminalStatus).toHaveBeenCalledWith('term-1', 'idle'); }); it('calls createTerminal when terminal status is idle', async () => { From 13f9c4937e7c71811de782a88392190dc526f1c1 Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 25 Apr 2026 20:56:02 +0200 Subject: [PATCH 4/8] Asserting destroyTerminal is not invoked during the loop Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../renderer/components/terminal/__tests__/usePtyProcess.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index 08582a4a2c..40d5ff09ed 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -92,6 +92,7 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea } expect(mockCreateTerminal).not.toHaveBeenCalled(); + expect(mockDestroyTerminal).not.toHaveBeenCalled(); }); it('allows PTY creation when status is exited but isRecreatingRef is true (worktree switch)', async () => { From 1654b4e347b5eaac405e69032a0781c810b1c4a3 Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Sat, 25 Apr 2026 21:00:41 +0200 Subject: [PATCH 5/8] test: attach electronAPI to jsdom window instead of replacing it --- .../components/terminal/__tests__/usePtyProcess.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index 40d5ff09ed..f6bb0696e2 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -20,8 +20,10 @@ const mockCreateTerminal = vi.fn(); const mockDestroyTerminal = vi.fn(); const mockRestoreTerminalSession = vi.fn(); -vi.stubGlobal('window', { - electronAPI: { +Object.defineProperty(window, 'electronAPI', { + configurable: true, + writable: true, + value: { createTerminal: mockCreateTerminal, destroyTerminal: mockDestroyTerminal, restoreTerminalSession: mockRestoreTerminalSession, From fd228d5b24f468babd70c4cd0e525c80aeb9c18b Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Sat, 25 Apr 2026 21:03:08 +0200 Subject: [PATCH 6/8] test: remove unused getTerminal mock from usePtyProcess tests --- .../components/terminal/__tests__/usePtyProcess.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index f6bb0696e2..7cd6d1a2de 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -34,9 +34,6 @@ Object.defineProperty(window, 'electronAPI', { let mockTerminalStatus: TerminalStatus = 'idle'; let mockIsRestored: boolean = false; -const mockSetTerminalStatus = vi.fn(); -const mockGetTerminal = vi.fn(); - vi.mock('../../../stores/terminal-store', () => ({ useTerminalStore: Object.assign(vi.fn(), { getState: () => ({ @@ -50,11 +47,12 @@ vi.mock('../../../stores/terminal-store', () => ({ }, ], setTerminalStatus: mockSetTerminalStatus, - getTerminal: mockGetTerminal, }), }), })); +const mockSetTerminalStatus = vi.fn(); + const DEFAULT_OPTIONS = { terminalId: 'term-1', cwd: '/test', From f2c48fa213575eb1d88079378655176edd4579fa Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Mon, 27 Apr 2026 08:47:16 +0200 Subject: [PATCH 7/8] test: address CR comments on usePtyProcess tests - Move mockSetTerminalStatus declaration before vi.mock to eliminate TDZ footgun - Remove redundant `: boolean` annotation on mockIsRestored - Complete terminal mock shape (createdAt, title, etc.) required by restore path - Rename running-status test to clarify it exercises the new-terminal branch (isRestored=false) - Add negative assertion: mockRestoreTerminalSession not called on new-terminal branch - Add restore-branch test (isRestored=true): asserts restoreTerminalSession called, createTerminal skipped, and updateTerminal resets isRestored flag Co-Authored-By: Claude Sonnet 4.6 --- .../terminal/__tests__/usePtyProcess.test.ts | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index 7cd6d1a2de..fa679eea8f 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -32,7 +32,10 @@ Object.defineProperty(window, 'electronAPI', { /** Mutable terminal state controlled per test */ let mockTerminalStatus: TerminalStatus = 'idle'; -let mockIsRestored: boolean = false; +let mockIsRestored = false; + +const mockSetTerminalStatus = vi.fn(); +const mockUpdateTerminal = vi.fn(); vi.mock('../../../stores/terminal-store', () => ({ useTerminalStore: Object.assign(vi.fn(), { @@ -44,15 +47,18 @@ vi.mock('../../../stores/terminal-store', () => ({ isRestored: mockIsRestored, isCLIMode: false, cwd: '/test', + title: 'Terminal 1', + claudeSessionId: undefined, + worktreeConfig: undefined, + createdAt: new Date('2024-01-01T00:00:00Z'), }, ], setTerminalStatus: mockSetTerminalStatus, + updateTerminal: mockUpdateTerminal, }), }), })); -const mockSetTerminalStatus = vi.fn(); - const DEFAULT_OPTIONS = { terminalId: 'term-1', cwd: '/test', @@ -69,6 +75,7 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea mockIsRestored = false; mockCreateTerminal.mockResolvedValue({ success: true }); mockDestroyTerminal.mockResolvedValue({ success: true }); + mockRestoreTerminalSession.mockResolvedValue({ success: true, data: { success: true } }); }); it('does not call createTerminal when terminal status is exited (natural exit)', async () => { @@ -123,7 +130,7 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea ); }); - it('calls createTerminal when terminal status is running (e.g. reconnect)', async () => { + it('calls createTerminal (new-terminal branch) when status is running and isRestored is false', async () => { mockTerminalStatus = 'running'; await act(async () => { @@ -131,6 +138,20 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea }); expect(mockCreateTerminal).toHaveBeenCalledTimes(1); + expect(mockRestoreTerminalSession).not.toHaveBeenCalled(); + }); + + it('calls restoreTerminalSession (restore branch) when status is running and isRestored is true', async () => { + mockTerminalStatus = 'running'; + mockIsRestored = true; + + await act(async () => { + renderHook(() => usePtyProcess(DEFAULT_OPTIONS)); + }); + + expect(mockRestoreTerminalSession).toHaveBeenCalledTimes(1); + expect(mockCreateTerminal).not.toHaveBeenCalled(); + expect(mockUpdateTerminal).toHaveBeenCalledWith('term-1', { isRestored: false }); }); it('does not call createTerminal when skipCreation is true', async () => { From 362cc62df572e9e6ed3aaa2a906297d6a3325ca0 Mon Sep 17 00:00:00 2001 From: Luiggibcn Date: Mon, 27 Apr 2026 22:28:38 +0200 Subject: [PATCH 8/8] test(terminal): lock status reset call order with toHaveBeenNthCalledWith Co-Authored-By: Claude Sonnet 4.6 --- .../components/terminal/__tests__/usePtyProcess.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts index fa679eea8f..0a5036c031 100644 --- a/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts +++ b/apps/desktop/src/renderer/components/terminal/__tests__/usePtyProcess.test.ts @@ -113,8 +113,9 @@ describe('usePtyProcess — exited-terminal guard (#fix/terminal-exit-pty-recrea }); expect(mockCreateTerminal).toHaveBeenCalledTimes(1); - // The recreation path resets status from 'exited' → 'idle' before creating the PTY - expect(mockSetTerminalStatus).toHaveBeenCalledWith('term-1', 'idle'); + // The recreation path must reset status from 'exited' → 'idle' BEFORE creating the PTY, + // otherwise the exited-guard above would short-circuit creation. + expect(mockSetTerminalStatus).toHaveBeenNthCalledWith(1, 'term-1', 'idle'); }); it('calls createTerminal when terminal status is idle', async () => {