diff --git a/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts index 6989e014..57d6bf58 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts @@ -44,6 +44,18 @@ const GIT_PORCELAIN = { COMMIT_SHA_LENGTH: 8, } as const; +/** + * Check if an error was caused by a timeout (execFileAsync with timeout sets killed=true). + * This helper centralizes the timeout detection logic to avoid duplication. + */ +function isTimeoutError(error: unknown): boolean { + return ( + error instanceof Error && + 'killed' in error && + (error as NodeJS.ErrnoException & { killed?: boolean }).killed === true + ); +} + /** * Fix repositories that are incorrectly marked with core.bare=true. * This can happen when git worktree operations incorrectly set bare=true @@ -399,12 +411,12 @@ async function createTerminalWorktree( const isRemoteRef = baseBranch.startsWith('origin/'); const remoteBranchName = isRemoteRef ? baseBranch.replace('origin/', '') : baseBranch; - // Fetch the branch from remote + // Fetch the branch from remote (async to avoid blocking main process) try { - execFileSync(getToolPath('git'), ['fetch', 'origin', remoteBranchName], { + await execFileAsync(getToolPath('git'), ['fetch', 'origin', remoteBranchName], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 30000, env: getIsolatedGitEnv(), }); debugLog('[TerminalWorktree] Fetched latest from origin/' + remoteBranchName); @@ -426,10 +438,10 @@ async function createTerminalWorktree( } else { // Default behavior: check if remote version exists and use it for latest code try { - execFileSync(getToolPath('git'), ['rev-parse', '--verify', `origin/${baseBranch}`], { + await execFileAsync(getToolPath('git'), ['rev-parse', '--verify', `origin/${baseBranch}`], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 10000, env: getIsolatedGitEnv(), }); baseRef = `origin/${baseBranch}`; @@ -443,18 +455,20 @@ async function createTerminalWorktree( // Use --no-track to prevent the new branch from inheriting upstream tracking // from the base ref (e.g., origin/main). This ensures users can push with -u // to correctly set up tracking to their own remote branch. - execFileSync(getToolPath('git'), ['worktree', 'add', '-b', branchName, '--no-track', worktreePath, baseRef], { + // Use async to avoid blocking the main process on large repos. + await execFileAsync(getToolPath('git'), ['worktree', 'add', '-b', branchName, '--no-track', worktreePath, baseRef], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 60000, env: getIsolatedGitEnv(), }); debugLog('[TerminalWorktree] Created worktree with branch:', branchName, 'from', baseRef); } else { - execFileSync(getToolPath('git'), ['worktree', 'add', '--detach', worktreePath, baseRef], { + // Use async to avoid blocking the main process on large repos. + await execFileAsync(getToolPath('git'), ['worktree', 'add', '--detach', worktreePath, baseRef], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 60000, env: getIsolatedGitEnv(), }); debugLog('[TerminalWorktree] Created worktree in detached HEAD mode from', baseRef); @@ -507,9 +521,16 @@ async function createTerminalWorktree( } } + // Check if error was due to timeout + const isTimeout = isTimeoutError(error); + return { success: false, - error: error instanceof Error ? error.message : 'Failed to create worktree', + error: isTimeout + ? 'Git operation timed out. The repository may be too large or the network connection is slow. Please try again.' + : error instanceof Error + ? error.message + : 'Failed to create worktree', }; } } @@ -718,10 +739,11 @@ async function removeTerminalWorktree( try { if (existsSync(worktreePath)) { - execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { + // Use async to avoid blocking the main process on large repos + await execFileAsync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 60000, env: getIsolatedGitEnv(), }); debugLog('[TerminalWorktree] Removed git worktree'); @@ -733,10 +755,11 @@ async function removeTerminalWorktree( debugError('[TerminalWorktree] Invalid branch name in config:', config.branchName); } else { try { - execFileSync(getToolPath('git'), ['branch', '-D', config.branchName], { + // Use async to avoid blocking the main process + await execFileAsync(getToolPath('git'), ['branch', '-D', config.branchName], { cwd: projectPath, encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], + timeout: 30000, env: getIsolatedGitEnv(), }); debugLog('[TerminalWorktree] Deleted branch:', config.branchName); @@ -760,9 +783,17 @@ async function removeTerminalWorktree( return { success: true }; } catch (error) { debugError('[TerminalWorktree] Error removing worktree:', error); + + // Check if error was due to timeout + const isTimeout = isTimeoutError(error); + return { success: false, - error: error instanceof Error ? error.message : 'Failed to remove worktree', + error: isTimeout + ? 'Git operation timed out. The repository may be too large. Please try again.' + : error instanceof Error + ? error.message + : 'Failed to remove worktree', }; } } diff --git a/apps/frontend/src/main/terminal/pty-manager.ts b/apps/frontend/src/main/terminal/pty-manager.ts index 18fc7ab1..b85d2ba4 100644 --- a/apps/frontend/src/main/terminal/pty-manager.ts +++ b/apps/frontend/src/main/terminal/pty-manager.ts @@ -9,6 +9,7 @@ import { existsSync } from 'fs'; import type { TerminalProcess, WindowGetter, WindowsShellType } from './types'; import { isWindows, getWindowsShellPaths } from '../platform'; import { IPC_CHANNELS } from '../../shared/constants'; +import { safeSendToRenderer } from '../ipc-handlers/utils'; import { getClaudeProfileManager } from '../claude-profile-manager'; import { readSettingsFile } from '../settings-utils'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; @@ -221,11 +222,9 @@ export function setupPtyHandlers( // Call custom data handler onDataCallback(terminal, data); - // Send to renderer - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_OUTPUT, id, data); - } + // Send to renderer with isDestroyed() check to prevent crashes + // when the window is closed during terminal activity + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_OUTPUT, id, data); }); // Handle terminal exit @@ -245,10 +244,9 @@ export function setupPtyHandlers( // to avoid pty.node SIGABRT from destroyed BrowserWindow resources if (isShuttingDown) return; - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_EXIT, id, exitCode); - } + // Send to renderer with isDestroyed() check to prevent crashes + // when the window is closed during terminal exit + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_EXIT, id, exitCode); // Call custom exit handler onExitCallback(terminal); diff --git a/apps/frontend/src/main/terminal/session-handler.ts b/apps/frontend/src/main/terminal/session-handler.ts index cd22f480..f04e4a85 100644 --- a/apps/frontend/src/main/terminal/session-handler.ts +++ b/apps/frontend/src/main/terminal/session-handler.ts @@ -10,6 +10,7 @@ import type { TerminalProcess, WindowGetter } from './types'; import { getTerminalSessionStore, type TerminalSession } from '../terminal-session-store'; import { IPC_CHANNELS } from '../../shared/constants'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +import { safeSendToRenderer } from '../ipc-handlers/utils'; /** * Track session IDs that have been claimed by terminals to prevent race conditions. @@ -336,10 +337,8 @@ export function captureClaudeSessionId( updateClaudeSessionId(terminal.projectPath, terminalId, sessionId); } - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminalId, sessionId); - } + // Use safeSendToRenderer with isDestroyed() check to prevent crashes + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_CLAUDE_SESSION, terminalId, sessionId); } else { // Session was claimed by another terminal, keep polling for a different one debugLog('[SessionHandler] Session ID was claimed by another terminal, continuing to poll:', sessionId); diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index 77565b6e..39b7728d 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -17,6 +17,7 @@ import type { } from './types'; import { isWindows } from '../platform'; import { debugLog, debugError } from '../../shared/utils/debug-logger'; +import { safeSendToRenderer } from '../ipc-handlers/utils'; /** * Options for terminal restoration @@ -205,13 +206,11 @@ export async function restoreTerminal( } // Send title change event for all restored terminals so renderer updates - const win = getWindow(); - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_TITLE_CHANGE, session.id, session.title); - // Always sync worktreeConfig to renderer (even if undefined) to ensure correct state - // This handles both: showing labels after recovery AND clearing stale labels when worktrees are deleted - win.webContents.send(IPC_CHANNELS.TERMINAL_WORKTREE_CONFIG_CHANGE, session.id, terminal.worktreeConfig); - } + // Use safeSendToRenderer with isDestroyed() check to prevent crashes + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_TITLE_CHANGE, session.id, session.title); + // Always sync worktreeConfig to renderer (even if undefined) to ensure correct state + // This handles both: showing labels after recovery AND clearing stale labels when worktrees are deleted + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_WORKTREE_CONFIG_CHANGE, session.id, terminal.worktreeConfig); // Defer Claude resume until terminal becomes active (is viewed by user) // This prevents all terminals from resuming Claude simultaneously on app startup, @@ -230,9 +229,8 @@ export async function restoreTerminal( // Notify renderer that this terminal has a pending Claude resume // The renderer will trigger the resume when the terminal tab becomes active - if (win) { - win.webContents.send(IPC_CHANNELS.TERMINAL_PENDING_RESUME, terminal.id, storedClaudeSessionId); - } + // Use safeSendToRenderer with isDestroyed() check to prevent crashes + safeSendToRenderer(getWindow, IPC_CHANNELS.TERMINAL_PENDING_RESUME, terminal.id, storedClaudeSessionId); // Persist the Claude mode and pending resume state if (terminal.projectPath) { diff --git a/apps/frontend/src/renderer/components/Terminal.tsx b/apps/frontend/src/renderer/components/Terminal.tsx index 76a9c733..fdb80c2f 100644 --- a/apps/frontend/src/renderer/components/Terminal.tsx +++ b/apps/frontend/src/renderer/components/Terminal.tsx @@ -604,12 +604,11 @@ export const Terminal = forwardRef(function Termi postCreationTimeoutRef.current = null; } - setTimeout(() => { - if (!isMountedRef.current) { - dispose(); - isCreatedRef.current = false; - } - }, 100); + // Dispose synchronously on unmount to prevent race conditions + // where a new terminal mounts before the old one is cleaned up. + // The previous 100ms delay created a window where both terminals existed. + dispose(); + isCreatedRef.current = false; }; }, [id, dispose, cleanupAutoNaming]); diff --git a/apps/frontend/src/renderer/components/terminal/useXterm.ts b/apps/frontend/src/renderer/components/terminal/useXterm.ts index 137fdadb..a5d4a709 100644 --- a/apps/frontend/src/renderer/components/terminal/useXterm.ts +++ b/apps/frontend/src/renderer/components/terminal/useXterm.ts @@ -72,6 +72,13 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea return; } + // Reset refs when (re)initializing xterm + // This is critical for React StrictMode which unmounts/remounts components, + // causing dispose() to set isDisposedRef.current = true on the first unmount. + // Without this reset, the remounted component would still have isDisposed = true. + isDisposedRef.current = false; + dimensionsReadyCalledRef.current = false; + debugLog(`[useXterm] Initializing xterm for terminal: ${terminalId}`); const xterm = new XTerm({ @@ -490,15 +497,22 @@ export function useXterm({ terminalId, onCommandEnter, onResize, onDimensionsRea // Serialize buffer before disposing to preserve ANSI formatting serializeBuffer(); - if (xtermRef.current) { - xtermRef.current.dispose(); - xtermRef.current = null; + // Dispose addons explicitly before disposing xterm + // While xterm.dispose() handles loaded addons, explicit disposal ensures + // resources are freed in a predictable order and prevents potential leaks + if (fitAddonRef.current) { + fitAddonRef.current.dispose(); + fitAddonRef.current = null; } if (serializeAddonRef.current) { serializeAddonRef.current.dispose(); serializeAddonRef.current = null; } - fitAddonRef.current = null; + // Note: webLinksAddon is local and will be disposed when xterm.dispose() is called + if (xtermRef.current) { + xtermRef.current.dispose(); + xtermRef.current = null; + } }, [serializeBuffer, terminalId]); return {