From 40fc7e4d4edfb2555efe90b133bd8dc5c7e6018b Mon Sep 17 00:00:00 2001 From: Andy <119136210+AndyMik90@users.noreply.github.com> Date: Wed, 7 Jan 2026 19:15:43 +0100 Subject: [PATCH] fix(terminal): prevent crash after worktree creation (#771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * auto-claude: Fix terminal recreation coordination for worktree switching Add isRecreatingRef to coordinate between Terminal.tsx, usePtyProcess.ts, and useTerminalEvents.ts to prevent race conditions during deliberate terminal destruction and recreation (e.g., worktree switching). Changes: - Terminal.tsx: Add isRecreatingRef to track deliberate recreation and pass it to both hooks. Set flag before prepareForRecreate() in handleWorktreeCreated and handleSelectWorktree. - usePtyProcess.ts: Accept isRecreatingRef option. When recreating, reset terminal status from 'exited' to 'idle' to allow proper recreation. Clear the recreation flag after successful PTY creation. - useTerminalEvents.ts: Accept isRecreatingRef option. During deliberate recreation, skip setting status to 'exited' and skip the 2-second auto-removal timeout to allow proper recreation. This fixes the terminal crash issue after worktree creation where the exit handler would mark the terminal as 'exited' and schedule removal before the new PTY could be created. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * auto-claude: subtask-2-2 - Fix flaky tmpdir test and verify no regressions Add os.tmpdir() mock to claude-integration-handler tests to ensure consistent behavior across different operating systems. On macOS, os.tmpdir() returns /var/folders/.../T/ instead of /tmp/, which caused test failures. All 1247 frontend tests now pass. * perf(merge): optimize merge-preview to sub-second and fix branch detection - Remove expensive refresh_from_git() that processed 534 files (~21s) - Remove redundant preview_merge() call for single-task preview - Add lightweight _detect_parallel_task_conflicts() using existing evolution data - Add _detect_worktree_base_branch() to auto-detect source branch from git history - Fix 'name summary is not defined' error from stale references - Update _check_git_merge_conflicts() to accept base_branch parameter - Fix frontend to use proper priority: task metadata > project settings > detect The merge-preview now correctly detects which branch a task was created from (e.g., develop vs main) and compares against that branch instead of always using main. Performance improved from ~21s to sub-second for typical cases. * fix(merge): actually run git merge when no AI conflict resolution needed Bug: merge_existing_build checked 'files_merged > 0' to skip git merge, assuming smart merge had already staged the files. But 'files_merged' was just the preview count (files TO merge), not files that WERE merged. In the common no-conflict case, _try_smart_merge_inner returns success with a files_merged count but doesn't actually perform any merge. The git merge was being skipped, leaving nothing staged. Fix: Only skip git merge when AI actually did work (conflicts_resolved > 0 or ai_assisted > 0). Otherwise, always call manager.merge_worktree() to perform the actual git merge and stage the files. * fix terminal resuming * fix: address PR feedback for terminal deferred resume - Fix isClaudeMode not being set, causing Claude mode to be lost across restarts - Clear isRecreatingRef on PTY creation failure paths to prevent stuck terminals - Remove duplicate vi.mock('os') declaration in test file 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 Co-Authored-By: CodeRabbit * fix(terminal): persist worktree labels across app restarts Worktree labels were disappearing after app restart because: 1. Renderer didn't pass worktreeConfig to restoreTerminalSession() 2. Backend's createTerminal() persisted before worktreeConfig was set, overwriting the saved session data with worktreeConfig: undefined Fix: Pass worktreeConfig from renderer during restore, and re-persist in backend after setting worktreeConfig on the terminal. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 Co-authored-by: CodeRabbit --- apps/backend/cli/workspace_commands.py | 297 ++++++++++++++---- apps/backend/core/workspace.py | 13 +- .../file_evolution/modification_tracker.py | 55 +++- apps/backend/merge/file_evolution/tracker.py | 6 + .../ipc-handlers/task/worktree-handlers.ts | 30 +- .../main/ipc-handlers/terminal-handlers.ts | 11 + .../terminal/worktree-handlers.ts | 202 +++++++++++- .../src/main/terminal/terminal-lifecycle.ts | 40 +-- .../src/main/terminal/terminal-manager.ts | 22 ++ apps/frontend/src/main/terminal/types.ts | 2 + apps/frontend/src/main/worktree-paths.ts | 18 ++ apps/frontend/src/preload/api/terminal-api.ts | 21 ++ .../src/renderer/components/Terminal.tsx | 30 +- .../components/terminal/usePtyProcess.ts | 43 ++- .../components/terminal/useTerminalEvents.ts | 25 +- .../src/renderer/lib/mocks/terminal-mock.ts | 7 +- .../src/renderer/stores/terminal-store.ts | 10 + apps/frontend/src/shared/constants/ipc.ts | 2 + apps/frontend/src/shared/types/ipc.ts | 3 + 19 files changed, 714 insertions(+), 123 deletions(-) diff --git a/apps/backend/cli/workspace_commands.py b/apps/backend/cli/workspace_commands.py index 88c772fe..e6f25091 100644 --- a/apps/backend/cli/workspace_commands.py +++ b/apps/backend/cli/workspace_commands.py @@ -154,6 +154,172 @@ def _get_changed_files_from_git( return [] +def _detect_worktree_base_branch( + project_dir: Path, + worktree_path: Path, + spec_name: str, +) -> str | None: + """ + Detect which branch a worktree was created from. + + Tries multiple strategies: + 1. Check worktree config file (.auto-claude/worktree-config.json) + 2. Find merge-base with known branches (develop, main, master) + 3. Return None if unable to detect + + Args: + project_dir: Project root directory + worktree_path: Path to the worktree + spec_name: Name of the spec + + Returns: + The detected base branch name, or None if unable to detect + """ + import json + + # Strategy 1: Check for worktree config file + config_path = worktree_path / ".auto-claude" / "worktree-config.json" + if config_path.exists(): + try: + config = json.loads(config_path.read_text()) + if config.get("base_branch"): + debug( + MODULE, + f"Found base branch in worktree config: {config['base_branch']}", + ) + return config["base_branch"] + except Exception as e: + debug_warning(MODULE, f"Failed to read worktree config: {e}") + + # Strategy 2: Find which branch has the closest merge-base + # Check common branches: develop, main, master + spec_branch = f"auto-claude/{spec_name}" + candidate_branches = ["develop", "main", "master"] + + best_branch = None + best_commits_behind = float("inf") + + for branch in candidate_branches: + try: + # Check if branch exists + check = subprocess.run( + ["git", "rev-parse", "--verify", branch], + cwd=project_dir, + capture_output=True, + text=True, + ) + if check.returncode != 0: + continue + + # Get merge base + merge_base_result = subprocess.run( + ["git", "merge-base", branch, spec_branch], + cwd=project_dir, + capture_output=True, + text=True, + ) + if merge_base_result.returncode != 0: + continue + + merge_base = merge_base_result.stdout.strip() + + # Count commits between merge-base and branch tip + # The branch with fewer commits ahead is likely the one we branched from + ahead_result = subprocess.run( + ["git", "rev-list", "--count", f"{merge_base}..{branch}"], + cwd=project_dir, + capture_output=True, + text=True, + ) + if ahead_result.returncode == 0: + commits_ahead = int(ahead_result.stdout.strip()) + debug( + MODULE, + f"Branch {branch} is {commits_ahead} commits ahead of merge-base", + ) + if commits_ahead < best_commits_behind: + best_commits_behind = commits_ahead + best_branch = branch + except Exception as e: + debug_warning(MODULE, f"Error checking branch {branch}: {e}") + continue + + if best_branch: + debug( + MODULE, + f"Detected base branch from git history: {best_branch} (commits ahead: {best_commits_behind})", + ) + return best_branch + + return None + + +def _detect_parallel_task_conflicts( + project_dir: Path, + current_task_id: str, + current_task_files: list[str], +) -> list[dict]: + """ + Detect potential conflicts between this task and other active tasks. + + Uses existing evolution data to check if any of this task's files + have been modified by other active tasks. This is a lightweight check + that doesn't require re-processing all files. + + Args: + project_dir: Project root directory + current_task_id: ID of the current task + current_task_files: Files modified by this task (from git diff) + + Returns: + List of conflict dictionaries with 'file' and 'tasks' keys + """ + try: + from merge import MergeOrchestrator + + # Initialize orchestrator just to access evolution data + orchestrator = MergeOrchestrator( + project_dir, + enable_ai=False, + dry_run=True, + ) + + # Get all active tasks from evolution data + active_tasks = orchestrator.evolution_tracker.get_active_tasks() + + # Remove current task from active tasks + other_active_tasks = active_tasks - {current_task_id} + + if not other_active_tasks: + return [] + + # Convert current task files to a set for fast lookup + current_files_set = set(current_task_files) + + # Get files modified by other active tasks + conflicts = [] + other_task_files = orchestrator.evolution_tracker.get_files_modified_by_tasks( + list(other_active_tasks) + ) + + # Find intersection - files modified by both this task and other tasks + for file_path, tasks in other_task_files.items(): + if file_path in current_files_set: + # This file was modified by both current task and other task(s) + all_tasks = [current_task_id] + tasks + conflicts.append({"file": file_path, "tasks": all_tasks}) + + return conflicts + + except Exception as e: + # If anything fails, just return empty - parallel task detection is optional + debug_warning( + "workspace_commands", + f"Parallel task conflict detection failed: {e}", + ) + return [] + + # Import debug utilities try: from debug import ( @@ -369,7 +535,9 @@ def handle_cleanup_worktrees_command(project_dir: Path) -> None: cleanup_all_worktrees(project_dir, confirm=True) -def _check_git_merge_conflicts(project_dir: Path, spec_name: str) -> dict: +def _check_git_merge_conflicts( + project_dir: Path, spec_name: str, base_branch: str | None = None +) -> dict: """ Check for git-level merge conflicts WITHOUT modifying the working directory. @@ -379,6 +547,7 @@ def _check_git_merge_conflicts(project_dir: Path, spec_name: str) -> dict: Args: project_dir: Project root directory spec_name: Name of the spec + base_branch: Branch the task was created from (default: auto-detect) Returns: Dictionary with git conflict information: @@ -397,21 +566,25 @@ def _check_git_merge_conflicts(project_dir: Path, spec_name: str) -> dict: "has_conflicts": False, "conflicting_files": [], "needs_rebase": False, - "base_branch": "main", + "base_branch": base_branch or "main", "spec_branch": spec_branch, "commits_behind": 0, } try: - # Get the current branch (base branch) - base_result = subprocess.run( - ["git", "rev-parse", "--abbrev-ref", "HEAD"], - cwd=project_dir, - capture_output=True, - text=True, - ) - if base_result.returncode == 0: - result["base_branch"] = base_result.stdout.strip() + # Use provided base_branch, or detect from current HEAD + if not base_branch: + base_result = subprocess.run( + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + cwd=project_dir, + capture_output=True, + text=True, + ) + if base_result.returncode == 0: + result["base_branch"] = base_result.stdout.strip() + else: + result["base_branch"] = base_branch + debug(MODULE, f"Using provided base branch: {base_branch}") # Get the merge base commit merge_base_result = subprocess.run( @@ -570,7 +743,6 @@ def handle_merge_preview_command( spec_name=spec_name, ) - from merge import MergeOrchestrator from workspace import get_existing_build_worktree worktree_path = get_existing_build_worktree(project_dir, spec_name) @@ -597,16 +769,32 @@ def handle_merge_preview_command( } try: - # First, check for git-level conflicts (diverged branches) - git_conflicts = _check_git_merge_conflicts(project_dir, spec_name) - # Determine the task's source branch (where the task was created from) - # Use provided base_branch (from task metadata), or fall back to detected default + # Priority: + # 1. Provided base_branch (from task metadata) + # 2. Detect from worktree's git history (find which branch it diverged from) + # 3. Fall back to default branch detection (main/master) task_source_branch = base_branch if not task_source_branch: - # Auto-detect the default branch (main/master) that worktrees are typically created from + # Try to detect from worktree's git history + task_source_branch = _detect_worktree_base_branch( + project_dir, worktree_path, spec_name + ) + if not task_source_branch: + # Fall back to auto-detecting main/master task_source_branch = _detect_default_branch(project_dir) + debug( + MODULE, + f"Using task source branch: {task_source_branch}", + provided=base_branch is not None, + ) + + # Check for git-level conflicts (diverged branches) using the task's source branch + git_conflicts = _check_git_merge_conflicts( + project_dir, spec_name, base_branch=task_source_branch + ) + # Get actual changed files from git diff (this is the authoritative count) all_changed_files = _get_changed_files_from_git( worktree_path, task_source_branch @@ -617,56 +805,39 @@ def handle_merge_preview_command( changed_files=all_changed_files[:10], # Log first 10 ) - # NOTE: We intentionally do NOT have a fast path here. - # Even if commits_behind == 0 (main hasn't moved), we still need to: - # 1. Call refresh_from_git() to update evolution data for this task - # 2. Call preview_merge() to detect potential conflicts with OTHER parallel tasks - # that may be tracked in the evolution data but haven't been merged yet. - # Skipping semantic analysis when commits_behind == 0 would miss these conflicts. + # OPTIMIZATION: Skip expensive refresh_from_git() and preview_merge() calls + # For merge-preview, we only need to detect: + # 1. Git conflicts (task vs base branch) - already calculated in _check_git_merge_conflicts() + # 2. Parallel task conflicts (this task vs other active tasks) + # + # For parallel task detection, we just check if this task's files overlap + # with files OTHER tasks have already recorded - no need to re-process all files. - debug(MODULE, "Initializing MergeOrchestrator for preview...") + debug(MODULE, "Checking for parallel task conflicts (lightweight)...") - # Initialize the orchestrator - orchestrator = MergeOrchestrator( - project_dir, - enable_ai=False, # Don't use AI for preview - dry_run=True, # Don't write anything + # Check for parallel task conflicts by looking at existing evolution data + parallel_conflicts = _detect_parallel_task_conflicts( + project_dir, spec_name, all_changed_files ) - - # Refresh evolution data from the worktree - # Compare against the task's source branch (where the task was created from) debug( MODULE, - f"Refreshing evolution data from worktree: {worktree_path}", - task_source_branch=task_source_branch, - ) - orchestrator.evolution_tracker.refresh_from_git( - spec_name, worktree_path, target_branch=task_source_branch + f"Parallel task conflicts detected: {len(parallel_conflicts)}", + conflicts=parallel_conflicts[:5] if parallel_conflicts else [], ) - # Get merge preview (semantic conflicts between parallel tasks) - debug(MODULE, "Generating merge preview...") - preview = orchestrator.preview_merge([spec_name]) - - # Transform semantic conflicts to UI-friendly format + # Build conflict list - start with parallel task conflicts conflicts = [] - for c in preview.get("conflicts", []): - debug_verbose( - MODULE, - "Processing semantic conflict", - file=c.get("file", ""), - severity=c.get("severity", "unknown"), - ) + for pc in parallel_conflicts: conflicts.append( { - "file": c.get("file", ""), - "location": c.get("location", ""), - "tasks": c.get("tasks", []), - "severity": c.get("severity", "unknown"), - "canAutoMerge": c.get("can_auto_merge", False), - "strategy": c.get("strategy"), - "reason": c.get("reason", ""), - "type": "semantic", + "file": pc["file"], + "location": "file-level", + "tasks": pc["tasks"], + "severity": "medium", + "canAutoMerge": False, + "strategy": None, + "reason": f"File modified by multiple active tasks: {', '.join(pc['tasks'])}", + "type": "parallel", } ) @@ -693,13 +864,14 @@ def handle_merge_preview_command( } ) - summary = preview.get("summary", {}) # Count only non-lock-file conflicts git_conflict_count = len(git_conflicts.get("conflicting_files", [])) - len( lock_files_excluded ) - total_conflicts = summary.get("total_conflicts", 0) + git_conflict_count - conflict_files = summary.get("conflict_files", 0) + git_conflict_count + # Calculate totals from our conflict lists (git conflicts + parallel conflicts) + parallel_conflict_count = len(parallel_conflicts) + total_conflicts = git_conflict_count + parallel_conflict_count + conflict_files = git_conflict_count + parallel_conflict_count # Filter lock files from the git conflicts list for the response non_lock_conflicting_files = [ @@ -785,7 +957,7 @@ def handle_merge_preview_command( "totalFiles": total_files_from_git, "conflictFiles": conflict_files, "totalConflicts": total_conflicts, - "autoMergeable": summary.get("auto_mergeable", 0), + "autoMergeable": 0, # Not tracking auto-merge in lightweight mode "hasGitConflicts": git_conflicts["has_conflicts"] and len(non_lock_conflicting_files) > 0, # Include path-mapped AI merge count for UI display @@ -800,10 +972,9 @@ def handle_merge_preview_command( "Merge preview complete", total_files=result["summary"]["totalFiles"], total_files_source="git_diff", - semantic_tracked_files=summary.get("total_files", 0), total_conflicts=result["summary"]["totalConflicts"], has_git_conflicts=git_conflicts["has_conflicts"], - auto_mergeable=result["summary"]["autoMergeable"], + parallel_conflicts=parallel_conflict_count, path_mapped_ai_merges=len(path_mapped_ai_merges), total_renames=len(path_mappings), ) diff --git a/apps/backend/core/workspace.py b/apps/backend/core/workspace.py index 1f85d083..6ae292ab 100644 --- a/apps/backend/core/workspace.py +++ b/apps/backend/core/workspace.py @@ -245,14 +245,16 @@ def merge_existing_build( if smart_result is not None: # Smart merge handled it (success or identified conflicts) if smart_result.get("success"): - # Check if smart merge resolved git conflicts or path-mapped files + # Check if smart merge actually DID work (resolved conflicts via AI) + # NOTE: "files_merged" in stats is misleading - it's "files TO merge" not "files WERE merged" + # The smart merge preview returns this count but doesn't actually perform the merge + # in the no-conflict path. We only skip git merge if AI actually did work. stats = smart_result.get("stats", {}) had_conflicts = stats.get("conflicts_resolved", 0) > 0 - files_merged = stats.get("files_merged", 0) > 0 ai_assisted = stats.get("ai_assisted", 0) > 0 - if had_conflicts or files_merged or ai_assisted: - # Git conflicts were resolved OR path-mapped files were AI merged + if had_conflicts or ai_assisted: + # AI actually resolved conflicts or assisted with merges # Changes are already written and staged - no need for git merge _print_merge_success( no_commit, stats, spec_name=spec_name, keep_worktree=True @@ -264,7 +266,8 @@ def merge_existing_build( return True else: - # No conflicts and no files merged - do standard git merge + # No conflicts needed AI resolution - do standard git merge + # This is the common case: no divergence, just need to merge changes success_result = manager.merge_worktree( spec_name, delete_after=False, no_commit=no_commit ) diff --git a/apps/backend/merge/file_evolution/modification_tracker.py b/apps/backend/merge/file_evolution/modification_tracker.py index 115a175a..6d75237e 100644 --- a/apps/backend/merge/file_evolution/modification_tracker.py +++ b/apps/backend/merge/file_evolution/modification_tracker.py @@ -68,6 +68,7 @@ class ModificationTracker: new_content: str, evolutions: dict[str, FileEvolution], raw_diff: str | None = None, + skip_semantic_analysis: bool = False, ) -> TaskSnapshot | None: """ Record a file modification by a task. @@ -79,6 +80,9 @@ class ModificationTracker: new_content: File content after modification evolutions: Current evolution data (will be updated) raw_diff: Optional unified diff for reference + skip_semantic_analysis: If True, skip expensive semantic analysis. + Use this for lightweight file tracking when only conflict + detection is needed (not conflict resolution). Returns: Updated TaskSnapshot, or None if file not being tracked @@ -105,9 +109,19 @@ class ModificationTracker: content_hash_before=compute_content_hash(old_content), ) - # Analyze semantic changes - analysis = self.analyzer.analyze_diff(rel_path, old_content, new_content) - semantic_changes = analysis.changes + # Analyze semantic changes (or skip for lightweight tracking) + if skip_semantic_analysis: + # Fast path: just track the file change without analysis + # This is used for files that don't have conflicts + semantic_changes = [] + debug( + MODULE, + f"Skipping semantic analysis for {rel_path} (lightweight tracking)", + ) + else: + # Full analysis (only for conflict files) + analysis = self.analyzer.analyze_diff(rel_path, old_content, new_content) + semantic_changes = analysis.changes # Update snapshot snapshot.completed_at = datetime.now() @@ -121,6 +135,7 @@ class ModificationTracker: logger.info( f"Recorded modification to {rel_path} by {task_id}: " f"{len(semantic_changes)} semantic changes" + + (" (lightweight)" if skip_semantic_analysis else "") ) return snapshot @@ -130,6 +145,7 @@ class ModificationTracker: worktree_path: Path, evolutions: dict[str, FileEvolution], target_branch: str | None = None, + analyze_only_files: set[str] | None = None, ) -> None: """ Refresh task snapshots by analyzing git diff from worktree. @@ -142,6 +158,10 @@ class ModificationTracker: worktree_path: Path to the task's worktree evolutions: Current evolution data (will be updated) target_branch: Branch to compare against (default: detect from worktree) + analyze_only_files: If provided, only run full semantic analysis on + these files. Other files will be tracked with lightweight mode + (no semantic analysis). This optimizes performance by only + analyzing files that have actual conflicts. """ # Determine the target branch to compare against if not target_branch: @@ -154,6 +174,9 @@ class ModificationTracker: task_id=task_id, worktree_path=str(worktree_path), target_branch=target_branch, + analyze_only_files=list(analyze_only_files)[:10] + if analyze_only_files + else "all", ) try: @@ -243,6 +266,13 @@ class ModificationTracker: baseline_commit=merge_base[:8], ) + # Determine if this file needs full semantic analysis + # If analyze_only_files is provided, only analyze files in that set + # Otherwise, analyze all files (backward compatible) + skip_analysis = False + if analyze_only_files is not None: + skip_analysis = rel_path not in analyze_only_files + # Record the modification self.record_modification( task_id=task_id, @@ -251,6 +281,7 @@ class ModificationTracker: new_content=new_content, evolutions=evolutions, raw_diff=diff_result.stdout, + skip_semantic_analysis=skip_analysis, ) processed_count += 1 @@ -261,9 +292,21 @@ class ModificationTracker: ) continue - logger.info( - f"Refreshed {processed_count}/{len(changed_files)} files from worktree for task {task_id}" - ) + # Calculate how many files were fully analyzed vs just tracked + if analyze_only_files is not None: + analyzed_count = len( + [f for f in changed_files if f in analyze_only_files] + ) + tracked_only_count = processed_count - analyzed_count + logger.info( + f"Refreshed {processed_count}/{len(changed_files)} files from worktree for task {task_id} " + f"(analyzed: {analyzed_count}, tracked only: {tracked_only_count})" + ) + else: + logger.info( + f"Refreshed {processed_count}/{len(changed_files)} files from worktree for task {task_id} " + "(full analysis on all files)" + ) except subprocess.CalledProcessError as e: logger.error(f"Failed to refresh from git: {e}") diff --git a/apps/backend/merge/file_evolution/tracker.py b/apps/backend/merge/file_evolution/tracker.py index c9df3b1a..2a8d248e 100644 --- a/apps/backend/merge/file_evolution/tracker.py +++ b/apps/backend/merge/file_evolution/tracker.py @@ -327,6 +327,7 @@ class FileEvolutionTracker: task_id: str, worktree_path: Path, target_branch: str | None = None, + analyze_only_files: set[str] | None = None, ) -> None: """ Refresh task snapshots by analyzing git diff from worktree. @@ -338,11 +339,16 @@ class FileEvolutionTracker: task_id: The task identifier worktree_path: Path to the task's worktree target_branch: Branch to compare against (default: auto-detect) + analyze_only_files: If provided, only run full semantic analysis on + these files. Other files will be tracked with lightweight mode + (no semantic analysis). This optimizes performance by only + analyzing files that have actual conflicts. """ self.modification_tracker.refresh_from_git( task_id=task_id, worktree_path=worktree_path, evolutions=self._evolutions, target_branch=target_branch, + analyze_only_files=analyze_only_files, ) self._save_evolutions() diff --git a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts index c00ee1f9..8c97ae76 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -1621,11 +1621,18 @@ export function registerWorktreeHandlers( args.push('--no-commit'); } - // Add --base-branch if task was created with a specific base branch + // Add --base-branch with proper priority: + // 1. Task metadata baseBranch (explicit task-level override) + // 2. Project settings mainBranch (project-level default) + // This matches the logic in execution-handlers.ts const taskBaseBranch = getTaskBaseBranch(specDir); - if (taskBaseBranch) { - args.push('--base-branch', taskBaseBranch); - debug('Using stored base branch:', taskBaseBranch); + const projectMainBranch = project.settings?.mainBranch; + const effectiveBaseBranch = taskBaseBranch || projectMainBranch; + + if (effectiveBaseBranch) { + args.push('--base-branch', effectiveBaseBranch); + debug('Using base branch:', effectiveBaseBranch, + `(source: ${taskBaseBranch ? 'task metadata' : 'project settings'})`); } // Use configured Python path (venv if ready, otherwise bundled/system) @@ -2146,11 +2153,18 @@ export function registerWorktreeHandlers( '--merge-preview' ]; - // Add --base-branch if task was created with a specific base branch + // Add --base-branch with proper priority: + // 1. Task metadata baseBranch (explicit task-level override) + // 2. Project settings mainBranch (project-level default) + // This matches the logic in execution-handlers.ts const taskBaseBranch = getTaskBaseBranch(specDir); - if (taskBaseBranch) { - args.push('--base-branch', taskBaseBranch); - console.warn('[IPC] Using stored base branch for preview:', taskBaseBranch); + const projectMainBranch = project.settings?.mainBranch; + const effectiveBaseBranch = taskBaseBranch || projectMainBranch; + + if (effectiveBaseBranch) { + args.push('--base-branch', effectiveBaseBranch); + console.warn('[IPC] Using base branch for preview:', effectiveBaseBranch, + `(source: ${taskBaseBranch ? 'task metadata' : 'project settings'})`); } // Use configured Python path (venv if ready, otherwise bundled/system) diff --git a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts index d68e8ab9..96edd3c4 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal-handlers.ts @@ -645,6 +645,17 @@ export function registerTerminalHandlers( } ); + // Activate deferred Claude resume when terminal becomes active + // This is triggered by the renderer when a terminal with pendingClaudeResume becomes the active tab + ipcMain.on( + IPC_CHANNELS.TERMINAL_ACTIVATE_DEFERRED_RESUME, + (_, id: string) => { + terminalManager.activateDeferredResume(id).catch((error) => { + console.error('[terminal-handlers] Failed to activate deferred Claude resume:', error); + }); + } + ); + // Get available session dates for a project ipcMain.handle( IPC_CHANNELS.TERMINAL_GET_SESSION_DATES, 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 6ebd86f3..ca91fd70 100644 --- a/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts @@ -9,12 +9,15 @@ import type { import path from 'path'; import { existsSync, mkdirSync, writeFileSync, readFileSync, readdirSync, rmSync } from 'fs'; import { execFileSync } from 'child_process'; +import { minimatch } from 'minimatch'; import { debugLog, debugError } from '../../../shared/utils/debug-logger'; import { projectStore } from '../../project-store'; import { parseEnvFile } from '../utils'; import { getTerminalWorktreeDir, getTerminalWorktreePath, + getTerminalWorktreeMetadataDir, + getTerminalWorktreeMetadataPath, } from '../../worktree-paths'; // Shared validation regex for worktree names - lowercase alphanumeric with dashes/underscores @@ -24,6 +27,109 @@ const WORKTREE_NAME_REGEX = /^[a-z0-9][a-z0-9_-]*[a-z0-9]$|^[a-z0-9]$/; // Validation regex for git branch names - allows alphanumeric, dots, slashes, dashes, underscores const GIT_BRANCH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/; +/** + * Fix repositories that are incorrectly marked with core.bare=true. + * This can happen when git worktree operations incorrectly set bare=true + * on a working repository that has source files. + * + * Returns true if a fix was applied, false otherwise. + */ +function fixMisconfiguredBareRepo(projectPath: string): boolean { + try { + // Check if bare=true is set + const bareConfig = execFileSync( + 'git', + ['config', '--get', 'core.bare'], + { cwd: projectPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] } + ).trim().toLowerCase(); + + if (bareConfig !== 'true') { + return false; // Not marked as bare, nothing to fix + } + + // Check if there are source files (indicating misconfiguration) + // A truly bare repo would only have git internals, not source code + // This covers multiple ecosystems: JS/TS, Python, Rust, Go, Java, C#, etc. + const EXACT_MARKERS = [ + // JavaScript/TypeScript ecosystem + 'package.json', 'apps', 'src', + // Python ecosystem + 'pyproject.toml', 'setup.py', 'requirements.txt', 'Pipfile', + // Rust ecosystem + 'Cargo.toml', + // Go ecosystem + 'go.mod', 'go.sum', 'cmd', 'main.go', + // Java/JVM ecosystem + 'pom.xml', 'build.gradle', 'build.gradle.kts', + // Ruby ecosystem + 'Gemfile', 'Rakefile', + // PHP ecosystem + 'composer.json', + // General project markers + 'Makefile', 'CMakeLists.txt', 'README.md', 'LICENSE' + ]; + + const GLOB_MARKERS = [ + // .NET/C# ecosystem - patterns that need glob matching + '*.csproj', '*.sln', '*.fsproj' + ]; + + // Check exact matches first (fast path) + const hasExactMatch = EXACT_MARKERS.some(marker => + existsSync(path.join(projectPath, marker)) + ); + + if (hasExactMatch) { + // Found a project marker, proceed to fix + } else { + // Check glob patterns - read directory once and cache for all patterns + let directoryFiles: string[] | null = null; + const MAX_FILES_TO_CHECK = 500; + + const hasGlobMatch = GLOB_MARKERS.some(pattern => { + // Validate pattern - only support simple glob patterns for security + if (pattern.includes('..') || pattern.includes('/')) { + debugLog('[TerminalWorktree] Unsupported glob pattern ignored:', pattern); + return false; + } + + // Lazy-load directory listing, cached across patterns + if (directoryFiles === null) { + try { + const allFiles = readdirSync(projectPath); + directoryFiles = allFiles.slice(0, MAX_FILES_TO_CHECK); + if (allFiles.length > MAX_FILES_TO_CHECK) { + debugLog(`[TerminalWorktree] Directory has ${allFiles.length} entries, checking only first ${MAX_FILES_TO_CHECK}`); + } + } catch (error) { + debugError('[TerminalWorktree] Failed to read directory:', error); + directoryFiles = []; + } + } + + // Use minimatch for proper glob pattern matching + return directoryFiles.some(file => minimatch(file, pattern, { nocase: true })); + }); + + if (!hasGlobMatch) { + return false; // Legitimately bare repo + } + } + + // Fix the misconfiguration + debugLog('[TerminalWorktree] Detected misconfigured bare repository with source files. Auto-fixing by unsetting core.bare...'); + execFileSync( + 'git', + ['config', '--unset', 'core.bare'], + { cwd: projectPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] } + ); + debugLog('[TerminalWorktree] Fixed: core.bare has been unset. Git operations should now work correctly.'); + return true; + } catch { + return false; + } +} + /** * Validate that projectPath is a registered project */ @@ -87,20 +193,46 @@ function getDefaultBranch(projectPath: string): string { } } -function saveWorktreeConfig(worktreePath: string, config: TerminalWorktreeConfig): void { - writeFileSync(path.join(worktreePath, 'config.json'), JSON.stringify(config, null, 2)); +function saveWorktreeConfig(projectPath: string, name: string, config: TerminalWorktreeConfig): void { + const metadataDir = getTerminalWorktreeMetadataDir(projectPath); + mkdirSync(metadataDir, { recursive: true }); + const metadataPath = getTerminalWorktreeMetadataPath(projectPath, name); + writeFileSync(metadataPath, JSON.stringify(config, null, 2)); } -function loadWorktreeConfig(worktreePath: string): TerminalWorktreeConfig | null { - const configPath = path.join(worktreePath, 'config.json'); - if (existsSync(configPath)) { +function loadWorktreeConfig(projectPath: string, name: string): TerminalWorktreeConfig | null { + // Check new metadata location first + const metadataPath = getTerminalWorktreeMetadataPath(projectPath, name); + if (existsSync(metadataPath)) { try { - return JSON.parse(readFileSync(configPath, 'utf-8')); + return JSON.parse(readFileSync(metadataPath, 'utf-8')); } catch (error) { - debugError('[TerminalWorktree] Corrupted config.json in:', configPath, error); + debugError('[TerminalWorktree] Corrupted config at:', metadataPath, error); return null; } } + + // Backwards compatibility: check legacy location inside worktree + const legacyConfigPath = path.join(getTerminalWorktreePath(projectPath, name), 'config.json'); + if (existsSync(legacyConfigPath)) { + try { + const config = JSON.parse(readFileSync(legacyConfigPath, 'utf-8')); + // Migrate to new location + saveWorktreeConfig(projectPath, name, config); + // Clean up legacy file + try { + rmSync(legacyConfigPath); + debugLog('[TerminalWorktree] Migrated config from legacy location:', name); + } catch { + debugLog('[TerminalWorktree] Could not remove legacy config:', legacyConfigPath); + } + return config; + } catch (error) { + debugError('[TerminalWorktree] Corrupted legacy config at:', legacyConfigPath, error); + return null; + } + } + return null; } @@ -143,6 +275,12 @@ async function createTerminalWorktree( }; } + // Auto-fix any misconfigured bare repo before worktree operations + // This prevents crashes when git worktree operations have incorrectly set bare=true + if (fixMisconfiguredBareRepo(projectPath)) { + debugLog('[TerminalWorktree] Fixed misconfigured bare repository at:', projectPath); + } + const worktreePath = getTerminalWorktreePath(projectPath, name); const branchName = `terminal/${name}`; let directoryCreated = false; @@ -223,7 +361,7 @@ async function createTerminalWorktree( terminalId, }; - saveWorktreeConfig(worktreePath, config); + saveWorktreeConfig(projectPath, name, config); debugLog('[TerminalWorktree] Saved config for worktree:', name); return { success: true, config }; @@ -266,21 +404,41 @@ async function listTerminalWorktrees(projectPath: string): Promise(); + // Scan new metadata directory + const metadataDir = getTerminalWorktreeMetadataDir(projectPath); + if (existsSync(metadataDir)) { + try { + for (const file of readdirSync(metadataDir, { withFileTypes: true })) { + if (file.isFile() && file.name.endsWith('.json')) { + const name = file.name.replace('.json', ''); + const config = loadWorktreeConfig(projectPath, name); + if (config) { + configs.push(config); + seenNames.add(name); + } + } + } + } catch (error) { + debugError('[TerminalWorktree] Error scanning metadata dir:', error); + } + } + + // Also scan worktree directory for legacy configs (will be migrated on load) + const worktreeDir = getTerminalWorktreeDir(projectPath); if (existsSync(worktreeDir)) { try { for (const dir of readdirSync(worktreeDir, { withFileTypes: true })) { - if (dir.isDirectory()) { - const worktreePath = path.join(worktreeDir, dir.name); - const config = loadWorktreeConfig(worktreePath); + if (dir.isDirectory() && !seenNames.has(dir.name)) { + const config = loadWorktreeConfig(projectPath, dir.name); if (config) { configs.push(config); } } } } catch (error) { - debugError('[TerminalWorktree] Error listing worktrees:', error); + debugError('[TerminalWorktree] Error scanning worktree dir:', error); } } @@ -304,8 +462,13 @@ async function removeTerminalWorktree( return { success: false, error: 'Invalid worktree name' }; } + // Auto-fix any misconfigured bare repo before worktree operations + if (fixMisconfiguredBareRepo(projectPath)) { + debugLog('[TerminalWorktree] Fixed misconfigured bare repository at:', projectPath); + } + const worktreePath = getTerminalWorktreePath(projectPath, name); - const config = loadWorktreeConfig(worktreePath); + const config = loadWorktreeConfig(projectPath, name); if (!config) { return { success: false, error: 'Worktree not found' }; @@ -339,6 +502,17 @@ async function removeTerminalWorktree( } } + // Remove metadata file + const metadataPath = getTerminalWorktreeMetadataPath(projectPath, name); + if (existsSync(metadataPath)) { + try { + rmSync(metadataPath); + debugLog('[TerminalWorktree] Removed metadata file:', metadataPath); + } catch { + debugLog('[TerminalWorktree] Could not remove metadata file:', metadataPath); + } + } + return { success: true }; } catch (error) { debugError('[TerminalWorktree] Error removing worktree:', error); diff --git a/apps/frontend/src/main/terminal/terminal-lifecycle.ts b/apps/frontend/src/main/terminal/terminal-lifecycle.ts index c366ed7e..22d7eaec 100644 --- a/apps/frontend/src/main/terminal/terminal-lifecycle.ts +++ b/apps/frontend/src/main/terminal/terminal-lifecycle.ts @@ -179,43 +179,43 @@ export async function restoreTerminal( debugLog('[TerminalLifecycle] Cleared worktree config for terminal with deleted worktree:', session.id); } + // Re-persist after restoring title and worktreeConfig + // (createTerminal persists before these are set, so we need to persist again) + if (terminal.projectPath) { + SessionHandler.persistSession(terminal); + } + // 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); } - // Auto-resume Claude if session was in Claude mode + // Defer Claude resume until terminal becomes active (is viewed by user) + // This prevents all terminals from resuming Claude simultaneously on app startup, + // which can cause crashes and resource contention. + // // Use storedIsClaudeMode which comes from the persisted store, // not the renderer-passed values (renderer always passes isClaudeMode: false) - // - // Note: We no longer require storedClaudeSessionId because resumeClaude uses - // `claude --continue` which resumes the most recent session in the directory - // automatically. Session IDs are deprecated and ignored. if (options.resumeClaudeSession && storedIsClaudeMode) { + // Set Claude mode so it persists correctly across app restarts + // Without this, storedIsClaudeMode would be false on next restore terminal.isClaudeMode = true; - // Don't set claudeSessionId - it's deprecated and --continue doesn't use it - debugLog('[TerminalLifecycle] Auto-resuming Claude session using --continue'); + // Mark terminal as having a pending Claude resume + // The actual resume will be triggered when the terminal becomes active + terminal.pendingClaudeResume = true; + debugLog('[TerminalLifecycle] Marking terminal for deferred Claude resume:', terminal.id); - // Notify renderer that we're in Claude mode (session ID is deprecated) - // This prevents the renderer from also trying to resume (duplicate command) + // 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_CLAUDE_SESSION, terminal.id, storedClaudeSessionId); + win.webContents.send(IPC_CHANNELS.TERMINAL_PENDING_RESUME, terminal.id, storedClaudeSessionId); } - // Persist the restored Claude mode state immediately to avoid data loss - // if app closes before the 30-second periodic save + // Persist the Claude mode and pending resume state if (terminal.projectPath) { SessionHandler.persistSession(terminal); } - - // Small delay to ensure PTY is ready before sending resume command - // Note: sessionId parameter is deprecated and ignored by resumeClaude - if (options.onResumeNeeded) { - setTimeout(() => { - options.onResumeNeeded!(terminal.id, storedClaudeSessionId); - }, 500); - } } return { diff --git a/apps/frontend/src/main/terminal/terminal-manager.ts b/apps/frontend/src/main/terminal/terminal-manager.ts index 2f86c218..52b83a01 100644 --- a/apps/frontend/src/main/terminal/terminal-manager.ts +++ b/apps/frontend/src/main/terminal/terminal-manager.ts @@ -219,6 +219,28 @@ export class TerminalManager { await ClaudeIntegration.resumeClaudeAsync(terminal, sessionId, this.getWindow); } + /** + * Activate deferred Claude resume for a terminal + * Called when a terminal with pendingClaudeResume becomes active (user views it) + */ + async activateDeferredResume(id: string): Promise { + const terminal = this.terminals.get(id); + if (!terminal) { + return; + } + + // Check if terminal has a pending resume + if (!terminal.pendingClaudeResume) { + return; + } + + // Clear the pending flag + terminal.pendingClaudeResume = false; + + // Now actually resume Claude + await ClaudeIntegration.resumeClaudeAsync(terminal, undefined, this.getWindow); + } + /** * Resume Claude in a terminal with a specific session ID * @deprecated Use resumeClaudeAsync for non-blocking behavior diff --git a/apps/frontend/src/main/terminal/types.ts b/apps/frontend/src/main/terminal/types.ts index f203973f..b8ef1012 100644 --- a/apps/frontend/src/main/terminal/types.ts +++ b/apps/frontend/src/main/terminal/types.ts @@ -17,6 +17,8 @@ export interface TerminalProcess { title: string; /** Associated worktree configuration (persisted across restarts) */ worktreeConfig?: TerminalWorktreeConfig; + /** Whether this terminal has a pending Claude resume that should be triggered on activation */ + pendingClaudeResume?: boolean; } /** diff --git a/apps/frontend/src/main/worktree-paths.ts b/apps/frontend/src/main/worktree-paths.ts index b446562a..b7e3d02e 100644 --- a/apps/frontend/src/main/worktree-paths.ts +++ b/apps/frontend/src/main/worktree-paths.ts @@ -12,6 +12,9 @@ import { existsSync } from 'fs'; export const TASK_WORKTREE_DIR = '.auto-claude/worktrees/tasks'; export const TERMINAL_WORKTREE_DIR = '.auto-claude/worktrees/terminal'; +// Metadata directories (separate from git worktrees to avoid uncommitted files) +export const TERMINAL_WORKTREE_METADATA_DIR = '.auto-claude/terminal/metadata'; + // Legacy path for backwards compatibility export const LEGACY_WORKTREE_DIR = '.worktrees'; @@ -74,3 +77,18 @@ export function findTerminalWorktree(projectPath: string, name: string): string return null; } + +/** + * Get the terminal worktree metadata directory path + * This is separate from the git worktree to avoid uncommitted files + */ +export function getTerminalWorktreeMetadataDir(projectPath: string): string { + return path.join(projectPath, TERMINAL_WORKTREE_METADATA_DIR); +} + +/** + * Get the metadata file path for a specific terminal worktree + */ +export function getTerminalWorktreeMetadataPath(projectPath: string, name: string): string { + return path.join(projectPath, TERMINAL_WORKTREE_METADATA_DIR, `${name}.json`); +} diff --git a/apps/frontend/src/preload/api/terminal-api.ts b/apps/frontend/src/preload/api/terminal-api.ts index e7b509f8..7ea08f71 100644 --- a/apps/frontend/src/preload/api/terminal-api.ts +++ b/apps/frontend/src/preload/api/terminal-api.ts @@ -46,6 +46,7 @@ export interface TerminalAPI { ) => Promise>; clearTerminalSessions: (projectPath: string) => Promise; resumeClaudeInTerminal: (id: string, sessionId?: string) => void; + activateDeferredClaudeResume: (id: string) => void; getTerminalSessionDates: (projectPath?: string) => Promise>; getTerminalSessionsForDate: ( date: string, @@ -77,6 +78,7 @@ export interface TerminalAPI { callback: (info: { terminalId: string; profileId: string; profileName: string }) => void ) => () => void; onTerminalClaudeBusy: (callback: (id: string, isBusy: boolean) => void) => () => void; + onTerminalPendingResume: (callback: (id: string, sessionId?: string) => void) => () => void; // Claude Profile Management getClaudeProfiles: () => Promise>; @@ -143,6 +145,9 @@ export const createTerminalAPI = (): TerminalAPI => ({ resumeClaudeInTerminal: (id: string, sessionId?: string): void => ipcRenderer.send(IPC_CHANNELS.TERMINAL_RESUME_CLAUDE, id, sessionId), + activateDeferredClaudeResume: (id: string): void => + ipcRenderer.send(IPC_CHANNELS.TERMINAL_ACTIVATE_DEFERRED_RESUME, id), + getTerminalSessionDates: (projectPath?: string): Promise> => ipcRenderer.invoke(IPC_CHANNELS.TERMINAL_GET_SESSION_DATES, projectPath), @@ -299,6 +304,22 @@ export const createTerminalAPI = (): TerminalAPI => ({ }; }, + onTerminalPendingResume: ( + callback: (id: string, sessionId?: string) => void + ): (() => void) => { + const handler = ( + _event: Electron.IpcRendererEvent, + id: string, + sessionId?: string + ): void => { + callback(id, sessionId); + }; + ipcRenderer.on(IPC_CHANNELS.TERMINAL_PENDING_RESUME, handler); + return () => { + ipcRenderer.removeListener(IPC_CHANNELS.TERMINAL_PENDING_RESUME, handler); + }; + }, + // Claude Profile Management getClaudeProfiles: (): Promise> => ipcRenderer.invoke(IPC_CHANNELS.CLAUDE_PROFILES_GET), diff --git a/apps/frontend/src/renderer/components/Terminal.tsx b/apps/frontend/src/renderer/components/Terminal.tsx index 70e52440..50d0d105 100644 --- a/apps/frontend/src/renderer/components/Terminal.tsx +++ b/apps/frontend/src/renderer/components/Terminal.tsx @@ -36,6 +36,9 @@ export function Terminal({ }: TerminalProps) { const isMountedRef = useRef(true); const isCreatedRef = useRef(false); + // Track deliberate terminal recreation (e.g., worktree switching) + // This prevents exit handlers from triggering auto-removal during controlled recreation + const isRecreatingRef = useRef(false); // Worktree dialog state const [showWorktreeDialog, setShowWorktreeDialog] = useState(false); @@ -133,6 +136,8 @@ export function Terminal({ rows: ptyDimensions?.rows ?? 24, // Only allow PTY creation when dimensions are ready skipCreation: !ptyDimensions, + // Pass recreation ref to coordinate with deliberate terminal destruction/recreation + isRecreatingRef, onCreated: () => { isCreatedRef.current = true; }, @@ -144,6 +149,8 @@ export function Terminal({ // Handle terminal events useTerminalEvents({ terminalId: id, + // Pass recreation ref to skip auto-removal during deliberate terminal recreation + isRecreatingRef, onOutput: (data) => { write(data); }, @@ -160,6 +167,17 @@ export function Terminal({ } }, [isActive, focus]); + // Trigger deferred Claude resume when terminal becomes active + // This ensures Claude sessions are only resumed when the user actually views the terminal, + // preventing all terminals from resuming simultaneously on app startup (which can crash the app) + useEffect(() => { + if (isActive && terminal?.pendingClaudeResume) { + // Clear the pending flag and trigger the actual resume + useTerminalStore.getState().setPendingClaudeResume(id, false); + window.electronAPI.activateDeferredClaudeResume(id); + } + }, [isActive, id, terminal?.pendingClaudeResume]); + // Handle keyboard shortcuts for this terminal useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { @@ -244,7 +262,11 @@ Please confirm you're ready by saying: I'm ready to work on ${selectedTask.title }, []); const handleWorktreeCreated = useCallback(async (config: TerminalWorktreeConfig) => { - // IMPORTANT: Set isCreatingRef BEFORE updating the store to prevent race condition + // IMPORTANT: Set isRecreatingRef BEFORE destruction to signal deliberate recreation + // This prevents exit handlers from triggering auto-removal during controlled recreation + isRecreatingRef.current = true; + + // Set isCreatingRef BEFORE updating the store to prevent race condition // This prevents the PTY effect from running before destroyTerminal completes prepareForRecreate(); @@ -269,7 +291,11 @@ Please confirm you're ready by saying: I'm ready to work on ${selectedTask.title }, [id, setWorktreeConfig, updateTerminal, prepareForRecreate, resetForRecreate]); const handleSelectWorktree = useCallback(async (config: TerminalWorktreeConfig) => { - // IMPORTANT: Set isCreatingRef BEFORE updating the store to prevent race condition + // IMPORTANT: Set isRecreatingRef BEFORE destruction to signal deliberate recreation + // This prevents exit handlers from triggering auto-removal during controlled recreation + isRecreatingRef.current = true; + + // Set isCreatingRef BEFORE updating the store to prevent race condition prepareForRecreate(); // Same logic as handleWorktreeCreated - attach terminal to existing worktree diff --git a/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts b/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts index 9bd97a7d..2b40fe4a 100644 --- a/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts +++ b/apps/frontend/src/renderer/components/terminal/usePtyProcess.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useCallback, useState } from 'react'; +import { useEffect, useRef, useCallback, useState, type RefObject } from 'react'; import { useTerminalStore } from '../../stores/terminal-store'; interface UsePtyProcessOptions { @@ -8,6 +8,9 @@ interface UsePtyProcessOptions { cols: number; rows: number; skipCreation?: boolean; // Skip PTY creation until dimensions are ready + // Track deliberate recreation scenarios (e.g., worktree switching) + // When true, resets terminal status to 'idle' to allow proper recreation + isRecreatingRef?: RefObject; onCreated?: () => void; onError?: (error: string) => void; } @@ -19,6 +22,7 @@ export function usePtyProcess({ cols, rows, skipCreation = false, + isRecreatingRef, onCreated, onError, }: UsePtyProcessOptions) { @@ -59,10 +63,17 @@ export function usePtyProcess({ if (skipCreation) return; if (isCreatingRef.current || isCreatedRef.current) return; - const terminalState = useTerminalStore.getState().terminals.find((t) => t.id === terminalId); + const store = getStore(); + const terminalState = store.terminals.find((t) => t.id === terminalId); const alreadyRunning = terminalState?.status === 'running' || terminalState?.status === 'claude-active'; const isRestored = terminalState?.isRestored; + // When recreating (e.g., worktree switching), reset status from 'exited' to 'idle' + // This allows proper recreation after deliberate terminal destruction + if (isRecreatingRef?.current && terminalState?.status === 'exited') { + store.setTerminalStatus(terminalId, 'idle'); + } + isCreatingRef.current = true; if (isRestored && terminalState) { @@ -77,23 +88,37 @@ export function usePtyProcess({ claudeSessionId: terminalState.claudeSessionId, outputBuffer: '', createdAt: terminalState.createdAt.toISOString(), - lastActiveAt: new Date().toISOString() + lastActiveAt: new Date().toISOString(), + // Pass worktreeConfig so backend can restore it and persist correctly + worktreeConfig: terminalState.worktreeConfig, }, cols, rows ).then((result) => { if (result.success && result.data?.success) { isCreatedRef.current = true; + // Clear recreation flag after successful PTY creation + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } const store = getStore(); store.setTerminalStatus(terminalId, terminalState.isClaudeMode ? 'claude-active' : 'running'); store.updateTerminal(terminalId, { isRestored: false }); onCreated?.(); } else { const error = `Error restoring session: ${result.data?.error || result.error}`; + // Clear recreation flag on failure to prevent terminal from being stuck + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } onError?.(error); } isCreatingRef.current = false; }).catch((err) => { + // Clear recreation flag on failure to prevent terminal from being stuck + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } onError?.(err.message); isCreatingRef.current = false; }); @@ -108,15 +133,27 @@ export function usePtyProcess({ }).then((result) => { if (result.success) { isCreatedRef.current = true; + // Clear recreation flag after successful PTY creation + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } if (!alreadyRunning) { getStore().setTerminalStatus(terminalId, 'running'); } onCreated?.(); } else { + // Clear recreation flag on failure to prevent terminal from being stuck + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } onError?.(result.error || 'Unknown error'); } isCreatingRef.current = false; }).catch((err) => { + // Clear recreation flag on failure to prevent terminal from being stuck + if (isRecreatingRef?.current) { + isRecreatingRef.current = false; + } onError?.(err.message); isCreatingRef.current = false; }); diff --git a/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts b/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts index 5e13d8fe..60e60d9f 100644 --- a/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts +++ b/apps/frontend/src/renderer/components/terminal/useTerminalEvents.ts @@ -1,9 +1,12 @@ -import { useEffect, useRef } from 'react'; +import { useEffect, useRef, type RefObject } from 'react'; import { useTerminalStore } from '../../stores/terminal-store'; import { terminalBufferManager } from '../../lib/terminal-buffer-manager'; interface UseTerminalEventsOptions { terminalId: string; + // Track deliberate recreation scenarios (e.g., worktree switching) + // When true, skips auto-removal to allow proper recreation + isRecreatingRef?: RefObject; onOutput?: (data: string) => void; onExit?: (exitCode: number) => void; onTitleChange?: (title: string) => void; @@ -12,6 +15,7 @@ interface UseTerminalEventsOptions { export function useTerminalEvents({ terminalId, + isRecreatingRef, onOutput, onExit, onTitleChange, @@ -58,6 +62,14 @@ export function useTerminalEvents({ useEffect(() => { const cleanup = window.electronAPI.onTerminalExit((id, exitCode) => { if (id === terminalId) { + // During deliberate recreation (e.g., worktree switching), skip the normal + // exit handling to prevent setting status to 'exited' and scheduling removal. + // The recreation flow will handle status transitions. + if (isRecreatingRef?.current) { + onExitRef.current?.(exitCode); + return; + } + const store = useTerminalStore.getState(); store.setTerminalStatus(terminalId, 'exited'); // Reset Claude mode when terminal exits - the Claude process has ended @@ -129,4 +141,15 @@ export function useTerminalEvents({ return cleanup; }, [terminalId]); + + // Handle pending Claude resume notification (for deferred resume on tab activation) + useEffect(() => { + const cleanup = window.electronAPI.onTerminalPendingResume((id, _sessionId) => { + if (id === terminalId) { + useTerminalStore.getState().setPendingClaudeResume(terminalId, true); + } + }); + + return cleanup; + }, [terminalId]); } diff --git a/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts b/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts index a8a52b19..4a806165 100644 --- a/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts +++ b/apps/frontend/src/renderer/lib/mocks/terminal-mock.ts @@ -58,6 +58,10 @@ export const terminalMock = { console.warn('[Browser Mock] resumeClaudeInTerminal called'); }, + activateDeferredClaudeResume: () => { + console.warn('[Browser Mock] activateDeferredClaudeResume called'); + }, + getTerminalSessionDates: async () => ({ success: true, data: [] @@ -92,5 +96,6 @@ export const terminalMock = { onTerminalRateLimit: () => () => {}, onTerminalOAuthToken: () => () => {}, onTerminalAuthCreated: () => () => {}, - onTerminalClaudeBusy: () => () => {} + onTerminalClaudeBusy: () => () => {}, + onTerminalPendingResume: () => () => {} }; diff --git a/apps/frontend/src/renderer/stores/terminal-store.ts b/apps/frontend/src/renderer/stores/terminal-store.ts index 57296387..4975603c 100644 --- a/apps/frontend/src/renderer/stores/terminal-store.ts +++ b/apps/frontend/src/renderer/stores/terminal-store.ts @@ -21,6 +21,7 @@ export interface Terminal { projectPath?: string; // Project this terminal belongs to (for multi-project support) worktreeConfig?: TerminalWorktreeConfig; // Associated worktree for isolated development isClaudeBusy?: boolean; // Whether Claude Code is actively processing (for visual indicator) + pendingClaudeResume?: boolean; // Whether this terminal has a pending Claude resume (deferred until tab activated) } interface TerminalLayout { @@ -52,6 +53,7 @@ interface TerminalState { setAssociatedTask: (id: string, taskId: string | undefined) => void; setWorktreeConfig: (id: string, config: TerminalWorktreeConfig | undefined) => void; setClaudeBusy: (id: string, isBusy: boolean) => void; + setPendingClaudeResume: (id: string, pending: boolean) => void; clearAllTerminals: () => void; setHasRestoredSessions: (value: boolean) => void; reorderTerminals: (activeId: string, overId: string) => void; @@ -256,6 +258,14 @@ export const useTerminalStore = create((set, get) => ({ })); }, + setPendingClaudeResume: (id: string, pending: boolean) => { + set((state) => ({ + terminals: state.terminals.map((t) => + t.id === id ? { ...t, pendingClaudeResume: pending } : t + ), + })); + }, + clearAllTerminals: () => { set({ terminals: [], activeTerminalId: null, hasRestoredSessions: false }); }, diff --git a/apps/frontend/src/shared/constants/ipc.ts b/apps/frontend/src/shared/constants/ipc.ts index 46a23f68..078f63a1 100644 --- a/apps/frontend/src/shared/constants/ipc.ts +++ b/apps/frontend/src/shared/constants/ipc.ts @@ -71,6 +71,7 @@ export const IPC_CHANNELS = { TERMINAL_RESTORE_SESSION: 'terminal:restoreSession', TERMINAL_CLEAR_SESSIONS: 'terminal:clearSessions', TERMINAL_RESUME_CLAUDE: 'terminal:resumeClaude', + TERMINAL_ACTIVATE_DEFERRED_RESUME: 'terminal:activateDeferredResume', // Trigger deferred Claude resume when terminal becomes active TERMINAL_GET_SESSION_DATES: 'terminal:getSessionDates', TERMINAL_GET_SESSIONS_FOR_DATE: 'terminal:getSessionsForDate', TERMINAL_RESTORE_FROM_DATE: 'terminal:restoreFromDate', @@ -86,6 +87,7 @@ export const IPC_CHANNELS = { TERMINAL_EXIT: 'terminal:exit', TERMINAL_TITLE_CHANGE: 'terminal:titleChange', TERMINAL_CLAUDE_SESSION: 'terminal:claudeSession', // Claude session ID captured + TERMINAL_PENDING_RESUME: 'terminal:pendingResume', // Terminal has pending Claude resume (for deferred activation) TERMINAL_RATE_LIMIT: 'terminal:rateLimit', // Claude Code rate limit detected TERMINAL_OAUTH_TOKEN: 'terminal:oauthToken', // OAuth token captured from setup-token output TERMINAL_AUTH_CREATED: 'terminal:authCreated', // Auth terminal created for OAuth flow diff --git a/apps/frontend/src/shared/types/ipc.ts b/apps/frontend/src/shared/types/ipc.ts index d8da007e..25fb4d30 100644 --- a/apps/frontend/src/shared/types/ipc.ts +++ b/apps/frontend/src/shared/types/ipc.ts @@ -199,6 +199,7 @@ export interface ElectronAPI { restoreTerminalSession: (session: TerminalSession, cols?: number, rows?: number) => Promise>; clearTerminalSessions: (projectPath: string) => Promise; resumeClaudeInTerminal: (id: string, sessionId?: string) => void; + activateDeferredClaudeResume: (id: string) => void; getTerminalSessionDates: (projectPath?: string) => Promise>; getTerminalSessionsForDate: (date: string, projectPath: string) => Promise>; restoreTerminalSessionsFromDate: (date: string, projectPath: string, cols?: number, rows?: number) => Promise>; @@ -233,6 +234,8 @@ export interface ElectronAPI { }) => void) => () => void; /** Listen for Claude busy state changes (for visual indicator: red=busy, green=idle) */ onTerminalClaudeBusy: (callback: (id: string, isBusy: boolean) => void) => () => void; + /** Listen for pending Claude resume notifications (for deferred resume on tab activation) */ + onTerminalPendingResume: (callback: (id: string, sessionId?: string) => void) => () => void; // Claude profile management (multi-account support) getClaudeProfiles: () => Promise>;