diff --git a/apps/frontend/src/main/terminal/pty-manager.ts b/apps/frontend/src/main/terminal/pty-manager.ts index e90157b4..8a9e337a 100644 --- a/apps/frontend/src/main/terminal/pty-manager.ts +++ b/apps/frontend/src/main/terminal/pty-manager.ts @@ -16,6 +16,43 @@ import type { SupportedTerminal } from '../../shared/types/settings'; // Windows shell paths are now imported from the platform module via getWindowsShellPaths() +/** + * Track pending exit promises for terminals being destroyed. + * Used to wait for PTY process exit on Windows where termination is async. + */ +const pendingExitPromises = new Map void; + timeoutId: NodeJS.Timeout; +}>(); + +/** + * Default timeouts for waiting for PTY exit (in milliseconds). + * Windows needs longer timeout due to slower process termination. + */ +const PTY_EXIT_TIMEOUT_WINDOWS = 2000; +const PTY_EXIT_TIMEOUT_UNIX = 500; + +/** + * Wait for a PTY process to exit. + * Returns a promise that resolves when the PTY's onExit event fires. + * Has a timeout fallback in case the exit event never fires. + */ +export function waitForPtyExit(terminalId: string, timeoutMs?: number): Promise { + const timeout = timeoutMs ?? (isWindows() ? PTY_EXIT_TIMEOUT_WINDOWS : PTY_EXIT_TIMEOUT_UNIX); + + return new Promise((resolve) => { + // Set up timeout fallback + const timeoutId = setTimeout(() => { + debugLog('[PtyManager] PTY exit timeout for terminal:', terminalId); + pendingExitPromises.delete(terminalId); + resolve(); + }, timeout); + + // Store the promise resolver + pendingExitPromises.set(terminalId, { resolve, timeoutId }); + }); +} + /** * Get the Windows shell executable based on preferred terminal setting */ @@ -115,6 +152,14 @@ export function setupPtyHandlers( ptyProcess.onExit(({ exitCode }) => { debugLog('[PtyManager] Terminal exited:', id, 'code:', exitCode); + // Resolve any pending exit promise FIRST (before other cleanup) + const pendingExit = pendingExitPromises.get(id); + if (pendingExit) { + clearTimeout(pendingExit.timeoutId); + pendingExitPromises.delete(id); + pendingExit.resolve(); + } + const win = getWindow(); if (win) { win.webContents.send(IPC_CHANNELS.TERMINAL_EXIT, id, exitCode); @@ -123,7 +168,12 @@ export function setupPtyHandlers( // Call custom exit handler onExitCallback(terminal); - terminals.delete(id); + // Only delete if this is the SAME terminal object (not a newly created one with same ID). + // This prevents a race where destroyTerminal() awaits PTY exit, a new terminal is created + // with the same ID during the await, and then the old PTY's onExit deletes the new terminal. + if (terminals.get(id) === terminal) { + terminals.delete(id); + } }); } @@ -228,9 +278,30 @@ export function resizePty(terminal: TerminalProcess, cols: number, rows: number) } /** - * Kill a PTY process + * Kill a PTY process. + * @param terminal The terminal process to kill + * @param waitForExit If true, returns a promise that resolves when the PTY exits. + * Used on Windows where PTY termination is async. */ -export function killPty(terminal: TerminalProcess): void { +export function killPty(terminal: TerminalProcess, waitForExit: true): Promise; +export function killPty(terminal: TerminalProcess, waitForExit?: false): void; +export function killPty(terminal: TerminalProcess, waitForExit?: boolean): Promise | void { + if (waitForExit) { + const exitPromise = waitForPtyExit(terminal.id); + try { + terminal.pty.kill(); + } catch (error) { + // Clean up the pending promise if kill() throws + const pending = pendingExitPromises.get(terminal.id); + if (pending) { + clearTimeout(pending.timeoutId); + pendingExitPromises.delete(terminal.id); + pending.resolve(); + } + throw error; + } + return exitPromise; + } terminal.pty.kill(); } diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index bb1f9141..49d000bb 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -15,6 +15,7 @@ import type { WindowGetter, TerminalOperationResult } from './types'; +import { isWindows } from '../platform'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; /** @@ -228,7 +229,9 @@ export async function restoreTerminal( } /** - * Destroy a terminal process + * Destroy a terminal process. + * On Windows, waits for the PTY to actually exit before returning to prevent + * race conditions when recreating terminals (e.g., worktree switching). */ export async function destroyTerminal( id: string, @@ -245,8 +248,18 @@ export async function destroyTerminal( // Release any claimed session ID for this terminal SessionHandler.releaseSessionId(id); onCleanup(id); - PtyManager.killPty(terminal); + + // Delete from map BEFORE killing to prevent race with onExit handler terminals.delete(id); + + // On Windows, wait for PTY to actually exit before returning + // This prevents race conditions when recreating terminals + if (isWindows()) { + await PtyManager.killPty(terminal, true); + } else { + PtyManager.killPty(terminal); + } + return { success: true }; } catch (error) { return { @@ -276,6 +289,9 @@ export async function destroyAllTerminals( promises.push( new Promise((resolve) => { try { + // Note: We intentionally don't wait for PTY exit here (unlike destroyTerminal) + // because this function is only called during app shutdown when no terminals + // will be recreated. Waiting would only delay shutdown unnecessarily. PtyManager.killPty(terminal); } catch { // Ignore errors during cleanup