fix(terminal): wait for PTY exit on Windows before recreating terminal (#1184)

* fix(terminal): wait for PTY exit on Windows before recreating terminal

On Windows, PTY process termination is asynchronous and takes longer than
macOS/Linux. When switching worktrees, the terminal would close but not
reopen because the new PTY creation conflicted with the still-shutting-down
old PTY.

This fix adds promise-based wait for PTY exit on Windows:
- Add pendingExitPromises map to track terminals being destroyed
- Add waitForPtyExit() function with platform-specific timeouts
- Modify killPty() to optionally wait for exit
- Update destroyTerminal() to wait for PTY exit on Windows only

The timeout fallback (2000ms Windows, 500ms Unix) ensures no hangs if the
exit event never fires.

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

* fix(terminal): address PR review feedback

- Fix race condition: compare terminal object reference (not just ID) in
  onExit handler to prevent deleting newly created terminals with same ID
- Add function overloads for killPty() for type-safe return types
- Add error cleanup: wrap kill() in try/catch to clean up pending promise
  if kill() throws
- Add comment explaining why destroyAllTerminals() doesn't wait for PTY exit

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

---------

Co-authored-by: Test User <test@example.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

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<string, {
+  resolve: () => 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<void> {
+  const timeout = timeoutMs ?? (isWindows() ? PTY_EXIT_TIMEOUT_WINDOWS : PTY_EXIT_TIMEOUT_UNIX);
+
+  return new Promise<void>((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<void>;
+export function killPty(terminal: TerminalProcess, waitForExit?: false): void;
+export function killPty(terminal: TerminalProcess, waitForExit?: boolean): Promise<void> | 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();
 }
 

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