From 02ed91c91c51add3993ac673d2c9981340713e88 Mon Sep 17 00:00:00 2001 From: kaigler Date: Fri, 30 Jan 2026 05:46:13 -0600 Subject: [PATCH] feat(kanban): add bulk task delete and worktree cleanup improvements (#1588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(windows): add Windows file-based credential fallback (PR #1561) This includes all changes from PR #1561: - Add shared file credential helpers (getCredentialsFromFile, getFullCredentialsFromFile) - Add Windows file-based credential storage functions: - getWindowsCredentialsPath() - path to .credentials.json - getCredentialsFromWindowsFile() - read basic credentials from file - getCredentialsFromWindows() - tries Credential Manager first, falls back to file - getFullCredentialsFromWindowsFile() - read full credentials from file - getFullCredentialsFromWindows() - tries Credential Manager first, falls back to file - updateWindowsFileCredentials() - write credentials to file - updateWindowsCredentials() - updates both Credential Manager and file - Fix PtrToStructure failure in Windows Credential Manager PowerShell script by defining proper CREDENTIAL struct with IntPtr fields - Update index.ts to check migrated profiles for valid credentials via file fallback before showing re-auth modal - Update claude-code-handlers.ts to check Windows .credentials.json file - Refactor Linux credential code to use shared helpers - Add/update tests for Windows file fallback scenarios Co-Authored-By: Claude Opus 4.5 * fix(windows): add path normalization and smart credential source comparison Additional fixes on top of PR #1561: 1. Path normalization in claude-profile-manager.ts: - Normalize forward slashes to backslashes on Windows in getActiveProfileEnv() and getProfileEnv() - This ensures CLAUDE_CONFIG_DIR hash matches what Claude CLI computes - Fixes credential lookup failures when paths have mixed separators 2. Smart credential source comparison in credential-utils.ts: - getCredentialsFromWindows() now checks both file and Credential Manager - When both have tokens, prefers file (Claude CLI primary storage) - getFullCredentialsFromWindows() compares expiry times when both have tokens - Returns the token with later expiry (more recently refreshed) - Fixes stale token issue when Credential Manager has old tokens 3. Updated tests: - Fixed test to properly mock file not existing when testing Credential Manager - Added test for preferring file when both sources have tokens Co-Authored-By: Claude Opus 4.5 * fix: add orphaned worktree detection and cleanup support (#1531) - Add isOrphaned flag to WorktreeListItem for detecting worktrees without tasks - Add discardOrphanedWorktree API for deleting worktrees by spec name - Add TASK_WORKTREE_DISCARD_ORPHAN IPC channel - Add validation for projectId and project.path in listWorktrees - Handle git errors by including worktree with isOrphaned flag instead of skipping - Add worktree branch validation tests - Add worktree-cleanup utility Co-Authored-By: Claude Opus 4.5 * fix(windows): add async worktree deletion with retry logic for file locks On Windows, file locking by IDEs, antivirus, etc. can cause worktree deletion to fail with EPERM errors. Added forceDeleteWorktreeAsync() with exponential backoff retry logic (5 retries, 500ms base delay). - Added deleteDirectoryWithRetry() helper with fs/promises rm - Added forceDeleteWorktreeAsync() that uses retry on Windows - Updated orphan worktree discard handler to use async version - Preserved sync version for backwards compatibility Co-Authored-By: Claude Opus 4.5 * feat(worktrees): add success toast notification after delete Shows a toast with "Worktree 'branch-name' deleted successfully" after a worktree is deleted, so the user gets feedback. Co-Authored-By: Claude Opus 4.5 * feat(kanban): add bulk task delete and worktree cleanup improvements - Add bulk task delete with confirmation dialog for all Kanban columns - Enable task selection across all columns (not just Human Review) - Add deleteTasks() function in task-store for batch deletion - Add worktree delete success toast notifications - Add bulk worktree delete success toast - Add i18n keys for delete operations (en/fr) Closes #767 Co-Authored-By: Claude Opus 4.5 * fix: correct TypeScript types for selectAllTasks column status Co-Authored-By: Claude Opus 4.5 * fix: auto-commit before orphaned worktree deletion to prevent data loss Previously, deleting orphaned worktrees would destroy any uncommitted changes. Now uses cleanupWorktree() which auto-commits changes before deletion, preserving work in git history (recoverable via reflog for ~90 days). Co-Authored-By: Claude Opus 4.5 * fix: remove #1585 credential code from this PR Reverts the Windows credential fallback changes that were accidentally included. Those changes belong in PR #1585 separately. Co-Authored-By: Claude Opus 4.5 * fix: address PR #1588 review findings - Remove unused imports: forceDeleteWorktree from crud-handlers.ts, rmSync/forceDeleteWorktree/forceDeleteWorktreeAsync from worktree-handlers.ts - Fix misleading comments: "exponential backoff" → "linear backoff" in shared.ts and worktree-cleanup.ts (retryDelay * attempt is linear) - Export GIT_BRANCH_REGEX from worktree-handlers.ts and import in test to keep regex in sync with production code Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 Co-authored-by: AndyMik90 --- apps/backend/prompts/coder.md | 63 +--- apps/backend/prompts_pkg/prompt_generator.py | 12 +- .../worktree-branch-validation.test.ts | 161 +++++++++ .../main/ipc-handlers/task/crud-handlers.ts | 50 ++- .../ipc-handlers/task/worktree-handlers.ts | 338 ++++++++++++++---- .../src/main/utils/worktree-cleanup.ts | 307 ++++++++++++++++ apps/frontend/src/main/worktree-paths.ts | 55 +++ apps/frontend/src/preload/api/task-api.ts | 4 + .../src/renderer/components/KanbanBoard.tsx | 178 +++++++-- .../src/renderer/components/Worktrees.tsx | 65 +++- .../src/renderer/lib/mocks/workspace-mock.ts | 8 + .../src/renderer/stores/task-store.ts | 46 +++ apps/frontend/src/shared/constants/ipc.ts | 1 + .../src/shared/i18n/locales/en/common.json | 11 +- .../src/shared/i18n/locales/en/tasks.json | 8 + .../src/shared/i18n/locales/fr/common.json | 11 +- .../src/shared/i18n/locales/fr/tasks.json | 8 + apps/frontend/src/shared/types/ipc.ts | 1 + apps/frontend/src/shared/types/task.ts | 10 +- tests/test_prompt_generator.py | 168 ++++++--- 20 files changed, 1248 insertions(+), 257 deletions(-) create mode 100644 apps/frontend/src/main/ipc-handlers/task/__tests__/worktree-branch-validation.test.ts create mode 100644 apps/frontend/src/main/utils/worktree-cleanup.ts diff --git a/apps/backend/prompts/coder.md b/apps/backend/prompts/coder.md index 3b4d28d9..d7a7bc52 100644 --- a/apps/backend/prompts/coder.md +++ b/apps/backend/prompts/coder.md @@ -57,7 +57,9 @@ You may see absolute paths like `/e/projects/myapp/prod/src/file.ts` in: ```bash # Verify you're still in the worktree pwd -# Should show: .../.auto-claude/worktrees/tasks/{spec-name}/ (or legacy .../.worktrees/{spec-name}/) +# Should show: .../.auto-claude/worktrees/tasks/{spec-name}/ +# Or (legacy): .../.worktrees/{spec-name}/ +# Or (PR review): .../.auto-claude/github/pr/worktrees/{pr-number}/ # NOT: /path/to/main/project ``` @@ -125,65 +127,6 @@ git add [verified-path] --- -## 🚨 CRITICAL: WORKTREE ISOLATION 🚨 - -**You may be in an ISOLATED GIT WORKTREE environment.** - -Check the "YOUR ENVIRONMENT" section at the top of this prompt. If you see an -**"ISOLATED WORKTREE - CRITICAL"** section, you are in a worktree. - -### What is a Worktree? - -A worktree is a **complete copy of the project** isolated from the main project. -This allows safe development without affecting the main branch. - -### Worktree Rules (CRITICAL) - -**If you are in a worktree, the environment section will show:** - -* **YOUR LOCATION:** The path to your isolated worktree -* **FORBIDDEN PATH:** The parent project path you must NEVER `cd` to - -**CRITICAL RULES:** -* **NEVER** `cd` to the forbidden parent path -* **NEVER** use `cd ../..` to escape the worktree -* **STAY** within your working directory at all times -* **ALL** file operations use paths relative to your current location - -### Why This Matters - -Escaping the worktree causes: -* ❌ Git commits going to the wrong branch -* ❌ Files created/modified in the wrong location -* ❌ Breaking worktree isolation guarantees -* ❌ Losing the safety of isolated development - -### How to Stay Safe - -**Before ANY `cd` command:** - -```bash -# 1. Check where you are -pwd - -# 2. Verify the target is within your worktree -# If pwd shows: /path/to/.auto-claude/worktrees/tasks/spec-name/ -# Then: cd ./apps/backend ✅ SAFE -# But: cd /path/to/parent/project ❌ FORBIDDEN - ESCAPES ISOLATION - -# 3. When in doubt, don't use cd at all -# Use relative paths from your current directory instead -git add ./apps/backend/file.py # Works from anywhere in worktree -``` - -### The Golden Rule in Worktrees - -**If you're in a worktree, pretend the parent project doesn't exist.** - -Everything you need is in your worktree, accessible via relative paths. - ---- - ## STEP 1: GET YOUR BEARINGS (MANDATORY) First, check your environment. The prompt should tell you your working directory and spec location. diff --git a/apps/backend/prompts_pkg/prompt_generator.py b/apps/backend/prompts_pkg/prompt_generator.py index d5b9e55e..62ce7b96 100644 --- a/apps/backend/prompts_pkg/prompt_generator.py +++ b/apps/backend/prompts_pkg/prompt_generator.py @@ -49,13 +49,11 @@ def detect_worktree_isolation(project_dir: Path) -> tuple[bool, Path | None]: match = re.search(pattern, project_str) if match: # Extract the parent project path (everything before the worktree marker) - parent_prefix = project_str[: match.start()] - # Handle root-level worktrees where prefix would be empty - if not parent_prefix: - parent_path = Path(resolved_dir.anchor) - else: - parent_path = Path(parent_prefix) - return True, parent_path + parent_path = project_str[: match.start()] + # Handle edge case where worktree is at filesystem root + if not parent_path: + parent_path = resolved_dir.anchor + return True, Path(parent_path) return False, None diff --git a/apps/frontend/src/main/ipc-handlers/task/__tests__/worktree-branch-validation.test.ts b/apps/frontend/src/main/ipc-handlers/task/__tests__/worktree-branch-validation.test.ts new file mode 100644 index 00000000..a7d318bd --- /dev/null +++ b/apps/frontend/src/main/ipc-handlers/task/__tests__/worktree-branch-validation.test.ts @@ -0,0 +1,161 @@ +/** + * Tests for worktree branch validation logic. + * + * Issue #1479: When cleaning up a corrupted worktree, git rev-parse walks up + * to the main project and returns its current branch instead of the worktree's branch. + * This could cause deletion of the wrong branch. + * + * These tests verify the validation logic that prevents this. + */ + +import { describe, expect, it } from 'vitest'; +import { GIT_BRANCH_REGEX, validateWorktreeBranch } from '../worktree-handlers'; + +describe('GIT_BRANCH_REGEX', () => { + it('should accept valid auto-claude branch names', () => { + expect(GIT_BRANCH_REGEX.test('auto-claude/my-feature')).toBe(true); + expect(GIT_BRANCH_REGEX.test('auto-claude/123-fix-bug')).toBe(true); + expect(GIT_BRANCH_REGEX.test('auto-claude/feature_with_underscore')).toBe(true); + }); + + it('should accept valid feature branch names', () => { + expect(GIT_BRANCH_REGEX.test('feature/my-feature')).toBe(true); + expect(GIT_BRANCH_REGEX.test('fix/bug-123')).toBe(true); + expect(GIT_BRANCH_REGEX.test('main')).toBe(true); + expect(GIT_BRANCH_REGEX.test('develop')).toBe(true); + }); + + it('should accept single character branch names', () => { + expect(GIT_BRANCH_REGEX.test('a')).toBe(true); + expect(GIT_BRANCH_REGEX.test('1')).toBe(true); + }); + + it('should reject invalid branch names', () => { + expect(GIT_BRANCH_REGEX.test('')).toBe(false); + expect(GIT_BRANCH_REGEX.test('-invalid')).toBe(false); + expect(GIT_BRANCH_REGEX.test('invalid-')).toBe(false); + expect(GIT_BRANCH_REGEX.test('.invalid')).toBe(false); + }); + + it('should accept HEAD as syntactically valid (handled specially in validation logic)', () => { + // HEAD is technically valid as a git branch name syntactically, + // but when detected from rev-parse it indicates detached state. + // The validateWorktreeBranch function handles this case specially. + expect(GIT_BRANCH_REGEX.test('HEAD')).toBe(true); + }); +}); + +describe('validateWorktreeBranch', () => { + const expectedBranch = 'auto-claude/my-feature-123'; + + describe('exact match scenarios', () => { + it('should use detected branch when it matches expected exactly', () => { + const result = validateWorktreeBranch('auto-claude/my-feature-123', expectedBranch); + expect(result.branchToDelete).toBe('auto-claude/my-feature-123'); + expect(result.usedFallback).toBe(false); + expect(result.reason).toBe('exact_match'); + }); + }); + + describe('pattern match scenarios', () => { + it('should allow other auto-claude branches (specId renamed)', () => { + const result = validateWorktreeBranch('auto-claude/renamed-feature', expectedBranch); + expect(result.branchToDelete).toBe('auto-claude/renamed-feature'); + expect(result.usedFallback).toBe(false); + expect(result.reason).toBe('pattern_match'); + }); + + it('should allow auto-claude branches with different formats', () => { + const result = validateWorktreeBranch('auto-claude/001-task', expectedBranch); + expect(result.branchToDelete).toBe('auto-claude/001-task'); + expect(result.usedFallback).toBe(false); + expect(result.reason).toBe('pattern_match'); + }); + }); + + describe('security: corrupted worktree scenarios (issue #1479)', () => { + it('should reject main project branch and use expected pattern', () => { + // This is the critical case: corrupted worktree returns main project's branch + const result = validateWorktreeBranch('feature/xstate-task-machine', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject develop branch', () => { + const result = validateWorktreeBranch('develop', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject main branch', () => { + const result = validateWorktreeBranch('main', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject master branch', () => { + const result = validateWorktreeBranch('master', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject fix/ branches from main project', () => { + const result = validateWorktreeBranch('fix/some-bug', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject feature/ branches from main project', () => { + const result = validateWorktreeBranch('feature/new-feature', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + }); + + describe('detection failure scenarios', () => { + it('should use expected pattern when detection returns null', () => { + const result = validateWorktreeBranch(null, expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('detection_failed'); + }); + }); + + describe('edge cases', () => { + it('should handle empty detected branch', () => { + const result = validateWorktreeBranch('', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should handle HEAD (detached state)', () => { + const result = validateWorktreeBranch('HEAD', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should handle branch that starts with auto-claude but is malformed', () => { + // "auto-claude" without a slash should still be rejected + const result = validateWorktreeBranch('auto-claude', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + + it('should reject auto-claude/ with no suffix (invalid branch name)', () => { + // "auto-claude/" alone is not a valid branch name - needs actual specId + const result = validateWorktreeBranch('auto-claude/', expectedBranch); + expect(result.branchToDelete).toBe(expectedBranch); + expect(result.usedFallback).toBe(true); + expect(result.reason).toBe('invalid_pattern'); + }); + }); +}); diff --git a/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts b/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts index bff3a2e6..5c2ccc7c 100644 --- a/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/crud-handlers.ts @@ -8,7 +8,8 @@ import { titleGenerator } from '../../title-generator'; import { AgentManager } from '../../agent'; import { findTaskAndProject } from './shared'; import { findAllSpecPaths, isValidTaskId } from '../../utils/spec-path-helpers'; -import { isPathWithinBase } from '../../worktree-paths'; +import { isPathWithinBase, findTaskWorktree } from '../../worktree-paths'; +import { cleanupWorktree } from '../../utils/worktree-cleanup'; /** * Register task CRUD (Create, Read, Update, Delete) handlers @@ -216,6 +217,15 @@ export function registerTaskCRUDHandlers(agentManager: AgentManager): void { /** * Delete a task + * + * This handler: + * 1. Checks if task exists and is not running + * 2. Cleans up the worktree (auto-commits, deletes directory, prunes refs, deletes branch) + * 3. Deletes all spec directories (main project + any remaining worktree locations) + * + * Note: Worktree cleanup uses manual deletion instead of `git worktree remove --force` + * because the latter fails on Windows when the directory contains untracked files + * (node_modules, build artifacts, etc.). See: https://github.com/AndyMik90/Auto-Claude/issues/1539 */ ipcMain.handle( IPC_CHANNELS.TASK_DELETE, @@ -235,21 +245,49 @@ export function registerTaskCRUDHandlers(agentManager: AgentManager): void { return { success: false, error: 'Cannot delete a running task. Stop the task first.' }; } - // Find ALL locations where this task exists (main + worktrees) + let hasErrors = false; + const errors: string[] = []; + + // Clean up the worktree first if it exists + // This uses the robust cleanup that handles Windows file locking issues + const worktreePath = findTaskWorktree(project.path, task.specId); + if (worktreePath) { + console.warn(`[TASK_DELETE] Found worktree at: ${worktreePath}`); + const cleanupResult = await cleanupWorktree({ + worktreePath, + projectPath: project.path, + specId: task.specId, + commitMessage: 'Auto-save before task deletion', + logPrefix: '[TASK_DELETE]', + deleteBranch: true + }); + + if (!cleanupResult.success) { + console.error(`[TASK_DELETE] Worktree cleanup failed:`, cleanupResult.warnings); + hasErrors = true; + errors.push(`Worktree cleanup: ${cleanupResult.warnings.join('; ')}`); + } else { + if (cleanupResult.autoCommitted) { + console.warn(`[TASK_DELETE] Auto-committed uncommitted work before deletion`); + } + if (cleanupResult.warnings.length > 0) { + console.warn(`[TASK_DELETE] Cleanup warnings:`, cleanupResult.warnings); + } + } + } + + // Find ALL locations where this task exists (main + any remaining worktree dirs) // Following the archiveTasks() pattern from project-store.ts const specsBaseDir = getSpecsDir(project.autoBuildPath); const specPaths = findAllSpecPaths(project.path, specsBaseDir, task.specId); // If spec directory doesn't exist anywhere, return success (already removed) - if (specPaths.length === 0) { + if (specPaths.length === 0 && !hasErrors) { console.warn(`[TASK_DELETE] No spec directories found for task ${taskId} - already removed`); projectStore.invalidateTasksCache(project.id); return { success: true }; } - let hasErrors = false; - const errors: string[] = []; - // Delete from ALL locations for (const specDir of specPaths) { try { 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 211d33f8..f5798d77 100644 --- a/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts @@ -20,11 +20,67 @@ import { } from '../../worktree-paths'; import { persistPlanStatus, updateTaskMetadataPrUrl } from './plan-file-utils'; import { getIsolatedGitEnv, detectWorktreeBranch, refreshGitIndex } from '../../utils/git-isolation'; +import { cleanupWorktree } from '../../utils/worktree-cleanup'; import { killProcessGracefully } from '../../platform'; import { stripAnsiCodes } from '../../../shared/utils/ansi-sanitizer'; // Regex pattern for validating git branch names -const GIT_BRANCH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/; +export const GIT_BRANCH_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9._/-]*[a-zA-Z0-9]$|^[a-zA-Z0-9]$/; + +/** + * Validates a detected branch name and returns the safe branch to delete. + * + * Why `auto-claude/` prefix is considered safe: + * - All task worktrees use branches named `auto-claude/{specId}` + * - This pattern is controlled by Auto-Claude, not user input + * - If detected branch matches this pattern, it's a valid task branch + * - If it doesn't match (e.g., `main`, `develop`, `feature/xxx`), it's likely + * the main project's branch being incorrectly detected from a corrupted worktree + * + * Issue #1479: When cleaning up a corrupted worktree, git rev-parse walks up + * to the main project and returns its current branch instead of the worktree's branch. + * This could cause deletion of the wrong branch. + */ +export function validateWorktreeBranch( + detectedBranch: string | null, + expectedBranch: string +): { branchToDelete: string; usedFallback: boolean; reason: string } { + // If detection failed, use expected pattern + if (detectedBranch === null) { + return { + branchToDelete: expectedBranch, + usedFallback: true, + reason: 'detection_failed', + }; + } + + // Exact match - ideal case + if (detectedBranch === expectedBranch) { + return { + branchToDelete: detectedBranch, + usedFallback: false, + reason: 'exact_match', + }; + } + + // Matches auto-claude pattern with valid specId (not just "auto-claude/") + // The specId must be non-empty for this to be a valid task branch + if (detectedBranch.startsWith('auto-claude/') && detectedBranch.length > 'auto-claude/'.length) { + return { + branchToDelete: detectedBranch, + usedFallback: false, + reason: 'pattern_match', + }; + } + + // Detected branch doesn't match expected pattern - use fallback + // This is the critical security fix for issue #1479 + return { + branchToDelete: expectedBranch, + usedFallback: true, + reason: 'invalid_pattern', + }; +} // Maximum PR title length (GitHub's limit is 256 characters) const MAX_PR_TITLE_LENGTH = 256; @@ -1297,6 +1353,12 @@ async function openInTerminal(dirPath: string, terminal: SupportedTerminal, cust * This is the branch the task was created from (set by user during task creation) */ function getTaskBaseBranch(specDir: string): string | undefined { + // Defensive check for undefined input + if (!specDir || typeof specDir !== 'string') { + console.error('[getTaskBaseBranch] specDir is undefined or not a string'); + return undefined; + } + try { const metadataPath = path.join(specDir, 'task_metadata.json'); if (existsSync(metadataPath)) { @@ -1327,6 +1389,16 @@ function getTaskBaseBranch(specDir: string): string | undefined { * as the user may be on a feature branch when viewing worktree status. */ function getEffectiveBaseBranch(projectPath: string, specId: string, projectMainBranch?: string): string { + // Defensive check for undefined inputs + if (!projectPath || typeof projectPath !== 'string') { + console.error('[getEffectiveBaseBranch] projectPath is undefined or not a string'); + return 'main'; + } + if (!specId || typeof specId !== 'string') { + console.error('[getEffectiveBaseBranch] specId is undefined or not a string'); + return 'main'; + } + // 1. Try task metadata baseBranch const specDir = path.join(projectPath, '.auto-claude', 'specs', specId); const taskBaseBranch = getTaskBaseBranch(specDir); @@ -2224,29 +2296,31 @@ export function registerWorktreeHandlers( // Clean up worktree after successful full merge (fixes #243) // This allows drag-to-Done workflow since TASK_UPDATE_STATUS blocks 'done' when worktree exists - try { - if (worktreePath && existsSync(worktreePath)) { - execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { - cwd: project.path, - encoding: 'utf-8' - }); - debug('Worktree cleaned up after full merge:', worktreePath); + // Uses shared cleanup utility for robust Windows support (fixes #1539) + if (worktreePath && existsSync(worktreePath)) { + const cleanupResult = await cleanupWorktree({ + worktreePath, + projectPath: project.path, + specId: task.specId, + commitMessage: 'Auto-save before merge cleanup', + logPrefix: '[TASK_WORKTREE_MERGE]', + deleteBranch: true + }); - // Also delete the task branch since we merged successfully - const taskBranch = `auto-claude/${task.specId}`; - try { - execFileSync(getToolPath('git'), ['branch', '-D', taskBranch], { - cwd: project.path, - encoding: 'utf-8' - }); - debug('Task branch deleted:', taskBranch); - } catch { - // Branch might not exist or already deleted + if (cleanupResult.success) { + debug('Worktree cleaned up after full merge:', worktreePath); + if (cleanupResult.branch) { + debug('Task branch deleted:', cleanupResult.branch); } + } else { + debug('Worktree cleanup failed (non-fatal):', cleanupResult.warnings); + // Non-fatal - merge succeeded, cleanup can be done manually + } + + // Log any warnings for debugging + if (cleanupResult.warnings.length > 0) { + debug('Cleanup warnings:', cleanupResult.warnings); } - } catch (cleanupErr) { - debug('Worktree cleanup failed (non-fatal):', cleanupErr); - // Non-fatal - merge succeeded, cleanup can be done manually } } @@ -2639,6 +2713,10 @@ export function registerWorktreeHandlers( /** * Discard the worktree changes * Per-spec architecture: Each spec has its own worktree at .auto-claude/worktrees/tasks/{spec-name}/ + * + * Note: Uses the shared cleanupWorktree utility which handles Windows-specific issues + * where `git worktree remove --force` fails when the directory contains untracked files. + * See: https://github.com/AndyMik90/Auto-Claude/issues/1539 */ ipcMain.handle( IPC_CHANNELS.TASK_WORKTREE_DISCARD, @@ -2662,64 +2740,49 @@ export function registerWorktreeHandlers( }; } - try { - // Get the branch name before removing - // 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]' } - ); + // Use the shared cleanup utility for robust, cross-platform worktree deletion + const cleanupResult = await cleanupWorktree({ + worktreePath, + projectPath: project.path, + specId: task.specId, + commitMessage: 'Auto-save before discard', + logPrefix: '[TASK_WORKTREE_DISCARD]', + deleteBranch: true + }); - // Remove the worktree - execFileSync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], { - cwd: project.path, - encoding: 'utf-8', - env: getIsolatedGitEnv(), - timeout: 30000 - }); - - // Delete the branch - try { - execFileSync(getToolPath('git'), ['branch', '-D', branch], { - cwd: project.path, - encoding: 'utf-8', - env: getIsolatedGitEnv(), - timeout: 30000 - }); - } 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 - // (skip when caller will set a different status, e.g., 'done') - if (!skipStatusChange) { - const mainWindow = getMainWindow(); - if (mainWindow) { - mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, 'backlog'); - } - } - - return { - success: true, - data: { - success: true, - message: 'Worktree discarded successfully' - } - }; - } catch (gitError) { - console.error('Git error discarding worktree:', gitError); + if (!cleanupResult.success) { + console.error('[TASK_WORKTREE_DISCARD] Cleanup failed:', cleanupResult.warnings); return { success: false, - error: `Failed to discard worktree: ${gitError instanceof Error ? gitError.message : 'Unknown error'}` + error: `Failed to discard worktree: ${cleanupResult.warnings.join('; ')}` }; } + + // Log any non-fatal warnings + if (cleanupResult.warnings.length > 0) { + console.warn('[TASK_WORKTREE_DISCARD] Cleanup warnings:', cleanupResult.warnings); + } + if (cleanupResult.autoCommitted) { + console.warn('[TASK_WORKTREE_DISCARD] Auto-committed uncommitted work before discard'); + } + + + // Only send status change to backlog if not skipped + // (skip when caller will set a different status, e.g., 'done') + if (!skipStatusChange) { + const mainWindow = getMainWindow(); + if (mainWindow) { + mainWindow.webContents.send(IPC_CHANNELS.TASK_STATUS_CHANGE, taskId, 'backlog'); + } + } + + return { + success: true, + data: { + success: true, + message: 'Worktree discarded successfully' + } + }; } catch (error) { console.error('Failed to discard worktree:', error); return { @@ -2733,6 +2796,85 @@ export function registerWorktreeHandlers( // Promisified execFile for async git operations const execFileAsync = promisify(execFile); + /** + * Discard an orphaned worktree by spec name (no task association required) + * Used when the worktree exists but the task is missing or git state is corrupted + */ + ipcMain.handle( + IPC_CHANNELS.TASK_WORKTREE_DISCARD_ORPHAN, + async (_, projectId: string, specName: string): Promise> => { + try { + // Validate inputs + if (!projectId || typeof projectId !== 'string') { + console.error('discardOrphanedWorktree: Invalid projectId:', projectId); + return { success: false, error: 'Invalid projectId' }; + } + if (!specName || typeof specName !== 'string') { + console.error('discardOrphanedWorktree: Invalid specName:', specName); + return { success: false, error: 'Invalid specName' }; + } + + const project = projectStore.getProject(projectId); + if (!project) { + return { success: false, error: 'Project not found' }; + } + + // Validate project.path + if (!project.path || typeof project.path !== 'string') { + console.error('discardOrphanedWorktree: Project path is invalid:', project.path); + return { success: false, error: 'Project path is invalid' }; + } + + // Find worktree at .auto-claude/worktrees/tasks/{spec-name}/ + const worktreePath = findTaskWorktree(project.path, specName); + + if (!worktreePath) { + return { + success: true, + data: { + success: true, + message: 'No worktree to discard' + } + }; + } + + // Use cleanupWorktree which auto-commits any uncommitted changes before deletion + // This preserves work in git history (recoverable via reflog for ~90 days) + const cleanupResult = await cleanupWorktree({ + worktreePath, + projectPath: project.path, + specId: specName, + commitMessage: 'Auto-save before orphaned worktree deletion', + logPrefix: '[ORPHAN_CLEANUP]', + deleteBranch: true + }); + + if (!cleanupResult.success) { + return { + success: false, + error: cleanupResult.warnings.join(', ') || 'Failed to cleanup orphaned worktree' + }; + } + + return { + success: true, + data: { + success: true, + message: cleanupResult.autoCommitted + ? 'Orphaned worktree deleted (uncommitted changes were auto-saved)' + : 'Orphaned worktree deleted successfully' + } + }; + } catch (error) { + console.error('Failed to discard orphaned worktree:', error); + return { + success: false, + error: error instanceof Error ? error.message : 'Failed to discard orphaned worktree' + }; + } + } + ); + /** * List all spec worktrees for a project * Per-spec architecture: Each spec has its own worktree at .auto-claude/worktrees/tasks/{spec-name}/ @@ -2741,13 +2883,32 @@ export function registerWorktreeHandlers( IPC_CHANNELS.TASK_LIST_WORKTREES, async (_, projectId: string): Promise> => { try { + // Validate projectId + if (!projectId || typeof projectId !== 'string') { + console.error('listWorktrees: Invalid projectId:', projectId); + return { success: false, error: 'Invalid projectId' }; + } + const project = projectStore.getProject(projectId); if (!project) { return { success: false, error: 'Project not found' }; } +// Validate project.path + if (!project.path || typeof project.path !== 'string') { + console.error('listWorktrees: Project path is invalid:', project.path); + return { success: false, error: 'Project path is invalid' }; + } + const worktreesDir = getTaskWorktreeDir(project.path); + // Fetch tasks once before iterating (avoids repeated lookups per entry) + // Used for orphan detection - worktrees without a matching task are orphaned + const tasks = projectStore.getTasks(projectId); + // Track if task lookup was successful (empty array with existing specs dir = lookup failed) + const mainSpecsDir = path.join(project.path, '.auto-claude', 'specs'); + const taskLookupSuccessful = tasks.length > 0 || !existsSync(mainSpecsDir); + // Helper to process a single worktree entry (async) const processWorktreeEntry = async (entry: string, entryPath: string): Promise => { try { @@ -2763,7 +2924,7 @@ export function registerWorktreeHandlers( // Note: We do NOT use current HEAD as that may be a feature branch const baseBranch = getEffectiveBaseBranch(project.path, entry, project.settings?.mainBranch); - // Get commit count (async, cross-platform - no shell syntax) +// Get commit count (async, cross-platform - no shell syntax) let commitCount = 0; try { const countResult = await execFileAsync(getToolPath('git'), ['rev-list', '--count', `${baseBranch}..HEAD`], { @@ -2798,6 +2959,11 @@ export function registerWorktreeHandlers( // Ignore diff errors } + // Check if there's a task associated with this worktree + // A worktree without a task is considered orphaned (can happen if task was deleted) + // Only mark as orphaned if task lookup was successful (avoid false positives) + const hasTask = tasks.some(t => t.specId === entry); + return { specName: entry, path: entryPath, @@ -2806,12 +2972,26 @@ export function registerWorktreeHandlers( commitCount, filesChanged, additions, - deletions + deletions, + isOrphaned: taskLookupSuccessful ? !hasTask : false }; } catch (gitError) { - console.error(`Error getting info for worktree ${entry}:`, gitError); - // Skip this worktree if we can't get git info - return null; + // FIX: Don't skip worktree if git fails - it may be orphaned/corrupted + // Include it so it can be managed (deleted if orphaned) + const hasTask = tasks.some(t => t.specId === entry); + console.warn(`[Worktree] Git commands failed for ${entry}, hasTask=${hasTask}:`, gitError); + // Note: branch is empty - renderer should handle based on isOrphaned flag + return { + specName: entry, + path: entryPath, + branch: '', + baseBranch: '', + commitCount: 0, + filesChanged: 0, + additions: 0, + deletions: 0, + isOrphaned: taskLookupSuccessful ? !hasTask : false + }; } }; diff --git a/apps/frontend/src/main/utils/worktree-cleanup.ts b/apps/frontend/src/main/utils/worktree-cleanup.ts new file mode 100644 index 00000000..6e2a4917 --- /dev/null +++ b/apps/frontend/src/main/utils/worktree-cleanup.ts @@ -0,0 +1,307 @@ +/** + * Worktree Cleanup Utility + * + * Provides a robust, cross-platform worktree cleanup implementation that handles + * Windows-specific issues with git worktree deletion when untracked files exist. + * + * The standard `git worktree remove --force` fails on Windows when the worktree + * contains untracked files (node_modules, build artifacts, etc.). This utility: + * + * 1. Auto-commits any uncommitted work (preserves in git history for ~90 days via reflog) + * 2. Manually deletes the worktree directory with retry logic for Windows file locks + * 3. Prunes git's internal worktree references + * 4. Optionally deletes the associated branch + * + * Related issue: https://github.com/AndyMik90/Auto-Claude/issues/1539 + */ + +import { execFileSync } from 'child_process'; +import { rm } from 'fs/promises'; +import { existsSync } from 'fs'; +import { getToolPath } from '../cli-tool-manager'; +import { getIsolatedGitEnv } from './git-isolation'; +import { getTaskWorktreeDir, isPathWithinBase } from '../worktree-paths'; + +/** + * Options for worktree cleanup operation + */ +export interface WorktreeCleanupOptions { + /** Absolute path to the worktree directory to delete */ + worktreePath: string; + /** Absolute path to the main project directory (for git operations) */ + projectPath: string; + /** Spec ID for generating branch name (e.g., "001-my-feature") */ + specId: string; + /** Custom commit message for auto-commit (default: "Auto-save before deletion") */ + commitMessage?: string; + /** Log prefix for console messages (e.g., "[TASK_DELETE]") */ + logPrefix?: string; + /** Whether to delete the associated branch (default: true) */ + deleteBranch?: boolean; + /** Timeout in milliseconds for git operations (default: 30000) */ + timeout?: number; + /** Maximum retries for directory deletion on Windows (default: 3) */ + maxRetries?: number; + /** Delay between retries in milliseconds (default: 500) */ + retryDelay?: number; +} + +/** + * Result of the cleanup operation + */ +export interface WorktreeCleanupResult { + /** Whether the cleanup was successful */ + success: boolean; + /** The branch that was deleted (if deleteBranch was true) */ + branch?: string; + /** Whether uncommitted changes were auto-committed */ + autoCommitted?: boolean; + /** Warnings that occurred during cleanup (non-fatal issues) */ + warnings: string[]; +} + +/** + * Gets the worktree branch name based on spec ID + */ +function getWorktreeBranch(worktreePath: string, specId: string, timeout: number): string | null { + // First try to get branch from the worktree's HEAD + if (existsSync(worktreePath)) { + try { + const branch = execFileSync(getToolPath('git'), ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd: worktreePath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }).trim(); + + if (branch && branch !== 'HEAD') { + return branch; + } + } catch { + // Worktree might be corrupted, fall back to naming convention + } + } + + // Fall back to the naming convention: auto-claude/{spec-id} + return `auto-claude/${specId}`; +} + +/** + * Delays execution for specified milliseconds + */ +function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +/** + * Deletes a directory with retry logic for Windows file locking issues + * + * On Windows, files can be locked by other processes (IDE, build tools, etc.) + * which causes immediate deletion to fail. This function retries with linear + * backoff to handle transient file locks. + */ +async function deleteDirectoryWithRetry( + dirPath: string, + maxRetries: number, + retryDelay: number, + logPrefix: string +): Promise { + let lastError: Error | null = null; + + for (let attempt = 1; attempt <= maxRetries; attempt++) { + try { + await rm(dirPath, { recursive: true, force: true }); + return; // Success + } catch (error) { + lastError = error instanceof Error ? error : new Error(String(error)); + + if (attempt < maxRetries) { + const waitTime = retryDelay * attempt; // Linear backoff + console.warn( + `${logPrefix} Directory deletion attempt ${attempt}/${maxRetries} failed, ` + + `retrying in ${waitTime}ms: ${lastError.message}` + ); + await delay(waitTime); + } + } + } + + // All retries exhausted + throw lastError || new Error('Failed to delete directory after retries'); +} + +/** + * Cleans up a worktree directory in a robust, cross-platform manner + * + * This function handles the Windows-specific issue where `git worktree remove --force` + * fails when the worktree contains untracked files. The approach is: + * + * 1. Auto-commit any uncommitted changes (preserves work in git history) + * 2. Manually delete the directory with retry logic for file locks + * 3. Run `git worktree prune` to clean up git's internal references + * 4. Optionally delete the associated branch + * + * All errors except directory deletion are logged but don't fail the operation. + * + * @param options - Cleanup configuration options + * @returns Result object with success status and any warnings + * + * @example + * ```typescript + * const result = await cleanupWorktree({ + * worktreePath: 'C:/projects/my-app/.auto-claude/worktrees/tasks/001-feature', + * projectPath: 'C:/projects/my-app', + * specId: '001-feature', + * logPrefix: '[TASK_DELETE]' + * }); + * + * if (result.success) { + * console.log('Cleanup successful'); + * if (result.autoCommitted) { + * console.log('Note: Uncommitted work was auto-saved'); + * } + * } + * ``` + */ +export async function cleanupWorktree(options: WorktreeCleanupOptions): Promise { + const { + worktreePath, + projectPath, + specId, + commitMessage = 'Auto-save before deletion', + logPrefix = '[WORKTREE_CLEANUP]', + deleteBranch = true, + timeout = 30000, + maxRetries = 3, + retryDelay = 500 + } = options; + + const warnings: string[] = []; + let autoCommitted = false; + + // Security: Validate that worktreePath is within the expected worktree directory + // This prevents path traversal attacks and accidental deletion of wrong directories + const expectedBase = getTaskWorktreeDir(projectPath); + if (!isPathWithinBase(worktreePath, expectedBase)) { + console.error(`${logPrefix} Security: Path validation failed - worktree path is outside expected directory`); + return { + success: false, + warnings: ['Invalid worktree path'] + }; + } + + // 1. Get the branch name before we delete the directory + const branch = getWorktreeBranch(worktreePath, specId, timeout); + console.warn(`${logPrefix} Starting cleanup for worktree: ${worktreePath}`); + if (branch) { + console.warn(`${logPrefix} Associated branch: ${branch}`); + } + + // 2. Auto-commit any uncommitted changes to preserve work + // This ensures the user can recover their work via `git reflog` for ~90 days + if (existsSync(worktreePath)) { + try { + // Check if there are any changes to commit + const status = execFileSync(getToolPath('git'), ['status', '--porcelain'], { + cwd: worktreePath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }); + + if (status.trim()) { + // There are uncommitted changes - commit them before deletion + console.warn(`${logPrefix} Found uncommitted changes, auto-committing...`); + + execFileSync(getToolPath('git'), ['add', '-A'], { + cwd: worktreePath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }); + + execFileSync(getToolPath('git'), ['commit', '-m', commitMessage], { + cwd: worktreePath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }); + + console.warn(`${logPrefix} Auto-committed changes before deletion`); + autoCommitted = true; + } + } catch (commitError) { + // Non-critical - log and continue with deletion + const msg = commitError instanceof Error ? commitError.message : String(commitError); + console.warn(`${logPrefix} Failed to auto-commit changes (non-critical): ${msg}`); + warnings.push(`Auto-commit failed: ${msg}`); + } + } + + // 3. Delete the worktree directory manually + // This is required because `git worktree remove --force` fails on Windows + // when the directory contains untracked files (node_modules, build artifacts, etc.) + if (existsSync(worktreePath)) { + console.warn(`${logPrefix} Deleting worktree directory...`); + try { + await deleteDirectoryWithRetry(worktreePath, maxRetries, retryDelay, logPrefix); + console.warn(`${logPrefix} Worktree directory deleted successfully`); + } catch (deleteError) { + // This IS critical - if we can't delete the directory, the cleanup failed + const msg = deleteError instanceof Error ? deleteError.message : String(deleteError); + console.error(`${logPrefix} Failed to delete worktree directory: ${msg}`); + return { + success: false, + branch: branch || undefined, + autoCommitted, + warnings: [...warnings, `Directory deletion failed: ${msg}`] + }; + } + } else { + console.warn(`${logPrefix} Worktree directory already deleted`); + } + + // 4. Prune git's internal worktree references + // After manual deletion, git still thinks the worktree exists in .git/worktrees/ + // Running prune cleans up these stale references + try { + execFileSync(getToolPath('git'), ['worktree', 'prune'], { + cwd: projectPath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }); + console.warn(`${logPrefix} Git worktree references pruned`); + } catch (pruneError) { + // Non-critical - the worktree is already gone, prune is just cleanup + const msg = pruneError instanceof Error ? pruneError.message : String(pruneError); + console.warn(`${logPrefix} Failed to prune worktree references (non-critical): ${msg}`); + warnings.push(`Worktree prune failed: ${msg}`); + } + + // 5. Delete the branch if requested + if (deleteBranch && branch) { + try { + execFileSync(getToolPath('git'), ['branch', '-D', branch], { + cwd: projectPath, + encoding: 'utf-8', + env: getIsolatedGitEnv(), + timeout + }); + console.warn(`${logPrefix} Branch deleted: ${branch}`); + } catch (branchError) { + // Non-critical - branch might not exist or already deleted + const msg = branchError instanceof Error ? branchError.message : String(branchError); + console.warn(`${logPrefix} Failed to delete branch (non-critical): ${msg}`); + warnings.push(`Branch deletion failed: ${msg}`); + } + } + + console.warn(`${logPrefix} Cleanup completed successfully`); + return { + success: true, + branch: branch || undefined, + autoCommitted, + warnings + }; +} diff --git a/apps/frontend/src/main/worktree-paths.ts b/apps/frontend/src/main/worktree-paths.ts index c78e6ea5..5d4d8746 100644 --- a/apps/frontend/src/main/worktree-paths.ts +++ b/apps/frontend/src/main/worktree-paths.ts @@ -22,6 +22,10 @@ export const LEGACY_WORKTREE_DIR = '.worktrees'; * Get the task worktrees directory path */ export function getTaskWorktreeDir(projectPath: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTaskWorktreeDir: projectPath is undefined or not a string'); + return ''; + } return path.join(projectPath, TASK_WORKTREE_DIR); } @@ -29,6 +33,14 @@ export function getTaskWorktreeDir(projectPath: string): string { * Get the full path for a specific task worktree */ export function getTaskWorktreePath(projectPath: string, specId: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTaskWorktreePath: projectPath is undefined or not a string'); + return ''; + } + if (!specId || typeof specId !== 'string') { + console.error('[worktree-paths] getTaskWorktreePath: specId is undefined or not a string'); + return ''; + } return path.join(projectPath, TASK_WORKTREE_DIR, specId); } @@ -48,6 +60,16 @@ export function isPathWithinBase(resolvedPath: string, basePath: string): boolea * Includes path traversal protection to ensure paths stay within project */ export function findTaskWorktree(projectPath: string, specId: string): string | null { + // Defensive check for undefined inputs + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] findTaskWorktree: projectPath is undefined or not a string'); + return null; + } + if (!specId || typeof specId !== 'string') { + console.error('[worktree-paths] findTaskWorktree: specId is undefined or not a string'); + return null; + } + const normalizedProject = path.resolve(projectPath); // Check new path first @@ -81,6 +103,10 @@ export function findTaskWorktree(projectPath: string, specId: string): string | * Get the terminal worktrees directory path */ export function getTerminalWorktreeDir(projectPath: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTerminalWorktreeDir: projectPath is undefined or not a string'); + return ''; + } return path.join(projectPath, TERMINAL_WORKTREE_DIR); } @@ -88,6 +114,14 @@ export function getTerminalWorktreeDir(projectPath: string): string { * Get the full path for a specific terminal worktree */ export function getTerminalWorktreePath(projectPath: string, name: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTerminalWorktreePath: projectPath is undefined or not a string'); + return ''; + } + if (!name || typeof name !== 'string') { + console.error('[worktree-paths] getTerminalWorktreePath: name is undefined or not a string'); + return ''; + } return path.join(projectPath, TERMINAL_WORKTREE_DIR, name); } @@ -97,6 +131,15 @@ export function getTerminalWorktreePath(projectPath: string, name: string): stri * Includes path traversal protection to ensure paths stay within project */ export function findTerminalWorktree(projectPath: string, name: string): string | null { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] findTerminalWorktree: projectPath is undefined or not a string'); + return null; + } + if (!name || typeof name !== 'string') { + console.error('[worktree-paths] findTerminalWorktree: name is undefined or not a string'); + return null; + } + const normalizedProject = path.resolve(projectPath); // Check new path first @@ -131,6 +174,10 @@ export function findTerminalWorktree(projectPath: string, name: string): string * This is separate from the git worktree to avoid uncommitted files */ export function getTerminalWorktreeMetadataDir(projectPath: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTerminalWorktreeMetadataDir: projectPath is undefined or not a string'); + return ''; + } return path.join(projectPath, TERMINAL_WORKTREE_METADATA_DIR); } @@ -138,5 +185,13 @@ export function getTerminalWorktreeMetadataDir(projectPath: string): string { * Get the metadata file path for a specific terminal worktree */ export function getTerminalWorktreeMetadataPath(projectPath: string, name: string): string { + if (!projectPath || typeof projectPath !== 'string') { + console.error('[worktree-paths] getTerminalWorktreeMetadataPath: projectPath is undefined or not a string'); + return ''; + } + if (!name || typeof name !== 'string') { + console.error('[worktree-paths] getTerminalWorktreeMetadataPath: name is undefined or not a string'); + return ''; + } return path.join(projectPath, TERMINAL_WORKTREE_METADATA_DIR, `${name}.json`); } diff --git a/apps/frontend/src/preload/api/task-api.ts b/apps/frontend/src/preload/api/task-api.ts index 7be2ded7..16922510 100644 --- a/apps/frontend/src/preload/api/task-api.ts +++ b/apps/frontend/src/preload/api/task-api.ts @@ -60,6 +60,7 @@ export interface TaskAPI { mergeWorktree: (taskId: string, options?: { noCommit?: boolean }) => Promise>; mergeWorktreePreview: (taskId: string) => Promise>; discardWorktree: (taskId: string, skipStatusChange?: boolean) => Promise>; + discardOrphanedWorktree: (projectId: string, specName: string) => Promise>; clearStagedState: (taskId: string) => Promise>; listWorktrees: (projectId: string, options?: { includeStats?: boolean }) => Promise>; worktreeOpenInIDE: (worktreePath: string, ide: SupportedIDE, customPath?: string) => Promise>; @@ -162,6 +163,9 @@ export const createTaskAPI = (): TaskAPI => ({ discardWorktree: (taskId: string, skipStatusChange?: boolean): Promise> => ipcRenderer.invoke(IPC_CHANNELS.TASK_WORKTREE_DISCARD, taskId, skipStatusChange), + discardOrphanedWorktree: (projectId: string, specName: string): Promise> => + ipcRenderer.invoke(IPC_CHANNELS.TASK_WORKTREE_DISCARD_ORPHAN, projectId, specName), + clearStagedState: (taskId: string): Promise> => ipcRenderer.invoke(IPC_CHANNELS.TASK_CLEAR_STAGED_STATE, taskId), diff --git a/apps/frontend/src/renderer/components/KanbanBoard.tsx b/apps/frontend/src/renderer/components/KanbanBoard.tsx index 1385f0a4..a682a8ba 100644 --- a/apps/frontend/src/renderer/components/KanbanBoard.tsx +++ b/apps/frontend/src/renderer/components/KanbanBoard.tsx @@ -19,7 +19,7 @@ import { sortableKeyboardCoordinates, verticalListSortingStrategy } from '@dnd-kit/sortable'; -import { Plus, Inbox, Loader2, Eye, CheckCircle2, Archive, RefreshCw, GitPullRequest, X, Settings, ListPlus, ChevronLeft, ChevronRight, ChevronsRight, Lock, Unlock } from 'lucide-react'; +import { Plus, Inbox, Loader2, Eye, CheckCircle2, Archive, RefreshCw, GitPullRequest, X, Settings, ListPlus, ChevronLeft, ChevronRight, ChevronsRight, Lock, Unlock, Trash2 } from 'lucide-react'; import { Checkbox } from './ui/checkbox'; import { ScrollArea } from './ui/scroll-area'; import { Button } from './ui/button'; @@ -30,12 +30,22 @@ import { QueueSettingsModal } from './QueueSettingsModal'; import { TASK_STATUS_COLUMNS, TASK_STATUS_LABELS } from '../../shared/constants'; import { debugLog } from '../../shared/utils/debug-logger'; import { cn } from '../lib/utils'; -import { persistTaskStatus, forceCompleteTask, archiveTasks, useTaskStore } from '../stores/task-store'; +import { persistTaskStatus, forceCompleteTask, archiveTasks, deleteTasks, useTaskStore } from '../stores/task-store'; import { updateProjectSettings, useProjectStore } from '../stores/project-store'; import { useKanbanSettingsStore, COLLAPSED_COLUMN_WIDTH, DEFAULT_COLUMN_WIDTH, MIN_COLUMN_WIDTH, MAX_COLUMN_WIDTH } from '../stores/kanban-settings-store'; import { useToast } from '../hooks/use-toast'; import { WorktreeCleanupDialog } from './WorktreeCleanupDialog'; import { BulkPRDialog } from './BulkPRDialog'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from './ui/alert-dialog'; import type { Task, TaskStatus, TaskOrderState } from '../../shared/types'; // Type guard for valid drop column targets - preserves literal type from TASK_STATUS_COLUMNS @@ -227,12 +237,11 @@ const DroppableColumn = memo(function DroppableColumn({ status, tasks, onTaskCli id: status }); - // Calculate selection state for human_review column - const isHumanReview = status === 'human_review'; - const selectedCount = selectedTaskIds?.size ?? 0; + // Calculate selection state for this column const taskCount = tasks.length; - const isAllSelected = isHumanReview && taskCount > 0 && selectedCount === taskCount; - const isSomeSelected = isHumanReview && selectedCount > 0 && selectedCount < taskCount; + const columnSelectedCount = tasks.filter(t => selectedTaskIds?.has(t.id)).length; + const isAllSelected = taskCount > 0 && columnSelectedCount === taskCount; + const isSomeSelected = columnSelectedCount > 0 && columnSelectedCount < taskCount; // Determine checkbox checked state: true (all), 'indeterminate' (some), false (none) const selectAllCheckedState: boolean | 'indeterminate' = isAllSelected @@ -271,7 +280,7 @@ const DroppableColumn = memo(function DroppableColumn({ status, tasks, onTaskCli return handlers; }, [tasks, onStatusChange]); - // Create stable onToggleSelect handlers for each task (only for human_review column) + // Create stable onToggleSelect handlers for each task (for bulk selection) const onToggleSelectHandlers = useMemo(() => { if (!onToggleSelect) return null; const handlers = new Map void>(); @@ -407,8 +416,8 @@ const DroppableColumn = memo(function DroppableColumn({ status, tasks, onTaskCli )} - {/* Select All checkbox for human_review column */} - {isHumanReview && onSelectAll && onDeselectAll && ( + {/* Select All checkbox for column */} + {onSelectAll && onDeselectAll && (
@@ -671,6 +680,10 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR // Bulk PR dialog state const [bulkPRDialogOpen, setBulkPRDialogOpen] = useState(false); + // Delete confirmation dialog state + const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false); + const [isDeleting, setIsDeleting] = useState(false); + // Worktree cleanup dialog state const [worktreeCleanupDialog, setWorktreeCleanupDialog] = useState<{ open: boolean; @@ -790,16 +803,16 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR return grouped; }, [filteredTasks, taskOrder]); - // Prune stale IDs when tasks move out of human_review column + // Prune stale IDs when tasks are deleted or filtered out useEffect(() => { - const validIds = new Set(tasksByStatus.human_review.map(t => t.id)); + const allTaskIds = new Set(filteredTasks.map(t => t.id)); setSelectedTaskIds(prev => { - const filtered = new Set([...prev].filter(id => validIds.has(id))); + const filtered = new Set([...prev].filter(id => allTaskIds.has(id))); return filtered.size === prev.size ? prev : filtered; }); - }, [tasksByStatus.human_review]); + }, [filteredTasks]); - // Selection callbacks for bulk actions (Human Review column) + // Selection callbacks for bulk actions (all columns) const toggleTaskSelection = useCallback((taskId: string) => { setSelectedTaskIds(prev => { const next = new Set(prev); @@ -812,20 +825,27 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR }); }, []); - const selectAllTasks = useCallback(() => { - const humanReviewTasks = tasksByStatus.human_review; - const allIds = new Set(humanReviewTasks.map(t => t.id)); - setSelectedTaskIds(allIds); - }, [tasksByStatus.human_review]); + const selectAllTasks = useCallback((columnStatus?: typeof TASK_STATUS_COLUMNS[number]) => { + if (columnStatus) { + // Select all in specific column + const columnTasks = tasksByStatus[columnStatus] || []; + const columnIds = new Set(columnTasks.map((t: Task) => t.id)); + setSelectedTaskIds(prev => new Set([...prev, ...columnIds])); + } else { + // Select all across all columns + const allIds = new Set(filteredTasks.map(t => t.id)); + setSelectedTaskIds(allIds); + } + }, [tasksByStatus, filteredTasks]); const deselectAllTasks = useCallback(() => { setSelectedTaskIds(new Set()); }, []); - // Get selected task objects for the BulkPRDialog + // Get selected task objects for bulk actions const selectedTasks = useMemo(() => { - return tasksByStatus.human_review.filter(task => selectedTaskIds.has(task.id)); - }, [tasksByStatus.human_review, selectedTaskIds]); + return filteredTasks.filter(task => selectedTaskIds.has(task.id)); + }, [filteredTasks, selectedTaskIds]); // Handle opening the bulk PR dialog const handleOpenBulkPRDialog = useCallback(() => { @@ -839,6 +859,43 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR deselectAllTasks(); }, [deselectAllTasks]); + // Handle opening delete confirmation dialog + const handleOpenDeleteConfirm = useCallback(() => { + if (selectedTaskIds.size > 0) { + setDeleteConfirmOpen(true); + } + }, [selectedTaskIds.size]); + + // Handle confirmed bulk delete + const handleConfirmDelete = useCallback(async () => { + if (selectedTaskIds.size === 0) return; + + setIsDeleting(true); + const taskIdsToDelete = Array.from(selectedTaskIds); + const result = await deleteTasks(taskIdsToDelete); + + setIsDeleting(false); + setDeleteConfirmOpen(false); + + if (result.success) { + toast({ + title: t('kanban.deleteSuccess', { count: taskIdsToDelete.length }), + }); + deselectAllTasks(); + } else { + toast({ + title: t('kanban.deleteError'), + description: result.error, + variant: 'destructive', + }); + // Still clear selection for successfully deleted tasks + if (result.failedIds) { + const remainingIds = new Set(result.failedIds); + setSelectedTaskIds(remainingIds); + } + } + }, [selectedTaskIds, deselectAllTasks, toast, t]); + const handleArchiveAll = async () => { // Get projectId from the first task (all tasks should have the same projectId) const projectId = tasks[0]?.projectId; @@ -1525,10 +1582,10 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR archivedCount={status === 'done' ? archivedCount : undefined} showArchived={status === 'done' ? showArchived : undefined} onToggleArchived={status === 'done' ? toggleShowArchived : undefined} - selectedTaskIds={status === 'human_review' ? selectedTaskIds : undefined} - onSelectAll={status === 'human_review' ? selectAllTasks : undefined} - onDeselectAll={status === 'human_review' ? deselectAllTasks : undefined} - onToggleSelect={status === 'human_review' ? toggleTaskSelection : undefined} + selectedTaskIds={selectedTaskIds} + onSelectAll={() => selectAllTasks(status)} + onDeselectAll={deselectAllTasks} + onToggleSelect={toggleTaskSelection} isCollapsed={columnPreferences?.[status]?.isCollapsed} onToggleCollapsed={() => handleToggleColumnCollapsed(status)} columnWidth={columnPreferences?.[status]?.width} @@ -1567,6 +1624,15 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR {t('kanban.createPRs')} +