diff --git a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts index 3cd68bbd..686082c0 100644 --- a/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts @@ -17,7 +17,7 @@ import { } from './plan-file-utils'; import { findTaskWorktree } from '../../worktree-paths'; import { projectStore } from '../../project-store'; -import { getIsolatedGitEnv } from '../../utils/git-isolation'; +import { getIsolatedGitEnv, detectWorktreeBranch } from '../../utils/git-isolation'; /** * Atomic file write to prevent TOCTOU race conditions. @@ -597,21 +597,13 @@ export function registerTaskExecutionHandlers( console.warn(`[TASK_UPDATE_STATUS] Cleaning up worktree for task ${taskId} (user confirmed)`); try { // Get the branch name before removing the worktree - let branch = ''; - let usingFallbackBranch = false; - try { - branch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: worktreePath, - encoding: 'utf-8', - timeout: 30000, - env: getIsolatedGitEnv() - }).trim(); - } catch (branchError) { - // If we can't get branch name, use the default pattern - branch = `auto-claude/${task.specId}`; - usingFallbackBranch = true; - console.warn(`[TASK_UPDATE_STATUS] Could not get branch name, using fallback pattern: ${branch}`, branchError); - } + // Use shared utility to validate detected branch matches expected pattern + // This prevents deleting wrong branch when worktree is corrupted/orphaned + const { branch, usingFallback: usingFallbackBranch } = detectWorktreeBranch( + worktreePath, + task.specId, + { timeout: 30000, logPrefix: '[TASK_UPDATE_STATUS]' } + ); // Remove the worktree execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { @@ -637,7 +629,10 @@ export function registerTaskExecutionHandlers( // More concerning - fallback pattern didn't match actual branch console.warn(`[TASK_UPDATE_STATUS] Could not delete branch ${branch} using fallback pattern. Actual branch may still exist and need manual cleanup.`, branchDeleteError); } else { - console.warn(`[TASK_UPDATE_STATUS] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`); + console.warn( + `[TASK_UPDATE_STATUS] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`, + branchDeleteError + ); } } 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 af417caf..c891d0bf 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -19,7 +19,7 @@ import { findTaskWorktree, } from '../../worktree-paths'; import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils'; -import { getIsolatedGitEnv, refreshGitIndex } from '../../utils/git-isolation'; +import { getIsolatedGitEnv, detectWorktreeBranch, refreshGitIndex } from '../../utils/git-isolation'; import { killProcessGracefully } from '../../platform'; // Regex pattern for validating git branch names @@ -2588,25 +2588,37 @@ export function registerWorktreeHandlers( try { // Get the branch name before removing - const branch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { - cwd: worktreePath, - encoding: 'utf-8' - }).trim(); + // Use shared utility to validate detected branch matches expected pattern + // This prevents deleting wrong branch when worktree is corrupted/orphaned + const { branch, usingFallback } = detectWorktreeBranch( + worktreePath, + task.specId, + { timeout: 30000, logPrefix: '[TASK_WORKTREE_DISCARD]' } + ); // Remove the worktree execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { cwd: project.path, - encoding: 'utf-8' + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout: 30000 }); // Delete the branch try { execFileSync(getToolPath('git'), ['branch', '-D', branch], { cwd: project.path, - encoding: 'utf-8' + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout: 30000 }); - } catch { + } catch (branchDeleteError) { // Branch might already be deleted or not exist + if (usingFallback) { + console.warn(`[TASK_WORKTREE_DISCARD] Could not delete branch ${branch} using fallback pattern. Actual branch may still exist and need manual cleanup.`, branchDeleteError); + } else { + console.warn(`[TASK_WORKTREE_DISCARD] Could not delete branch ${branch} (may not exist or be checked out elsewhere)`, branchDeleteError); + } } // Only send status change to backlog if not skipped diff --git a/apps/frontend/src/main/utils/git-isolation.ts b/apps/frontend/src/main/utils/git-isolation.ts index 48a09d00..ba15b08f 100644 --- a/apps/frontend/src/main/utils/git-isolation.ts +++ b/apps/frontend/src/main/utils/git-isolation.ts @@ -109,6 +109,79 @@ export function getIsolatedGitSpawnOptions( }; } +/** + * Result type for detectWorktreeBranch function. + */ +export interface WorktreeBranchDetectionResult { + /** The branch name to use for deletion */ + branch: string; + /** Whether the fallback branch pattern was used */ + usingFallback: boolean; +} + +/** + * Detects the branch name in a worktree with safety validation. + * + * This function prevents a critical bug where git rev-parse in a corrupted/orphaned + * worktree can return the main project's current branch instead of the worktree's branch. + * It validates the detected branch matches the expected pattern before using it. + * + * @param worktreePath - Path to the worktree directory + * @param specId - The spec ID used to generate the expected branch name + * @param options - Optional configuration + * @param options.timeout - Timeout in milliseconds for git commands (default: 30000) + * @param options.logPrefix - Prefix for log messages (e.g., "[TASK_UPDATE_STATUS]") + * @returns Object containing the branch name and whether fallback was used + * + * @example + * ```typescript + * import { detectWorktreeBranch } from './utils/git-isolation'; + * import { getToolPath } from './cli-tool-manager'; + * + * const { branch, usingFallback } = detectWorktreeBranch( + * worktreePath, + * task.specId, + * { timeout: 30000, logPrefix: '[TASK_WORKTREE_DISCARD]' } + * ); + * ``` + */ +export function detectWorktreeBranch( + worktreePath: string, + specId: string, + options: { timeout?: number; logPrefix?: string } = {} +): WorktreeBranchDetectionResult { + const { timeout = 30000, logPrefix = '[WORKTREE_BRANCH_DETECTION]' } = options; + const expectedBranch = `auto-claude/${specId}`; + let branch = expectedBranch; + let usingFallback = false; + + try { + const detectedBranch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd: worktreePath, + encoding: 'utf-8', + timeout, + env: getIsolatedGitEnv() + }).trim(); + + // SECURITY: Use strict exact-match validation (not prefix matching) to prevent + // accidentally deleting a different task's auto-claude branch. When git rev-parse + // returns an unexpected branch, we MUST fall back to the expected pattern rather + // than risking deletion of the wrong branch. This is critical for data safety. + if (detectedBranch === expectedBranch) { + branch = detectedBranch; + } else { + console.warn(`${logPrefix} Detected branch '${detectedBranch}' doesn't match expected branch '${expectedBranch}', using fallback: ${expectedBranch}`); + usingFallback = true; + } + } catch (branchError) { + // If we can't get branch name, use the default pattern + usingFallback = true; + console.warn(`${logPrefix} Could not get branch name, using fallback pattern: ${branch}`, branchError); + } + + return { branch, usingFallback }; +} + /** * Refreshes the git index to ensure accurate status after external commits. *