auto-claude: 196-fix-worktrees-dialog-auto-close-race-condition-and (#1727)
* auto-claude: subtask-1-1 - Fix task worktree delete dialog auto-close at line 914 Applied e.preventDefault() pattern to prevent dialog from closing before async delete operation completes. Dialog now stays open with spinner until delete finishes, matching pattern from DiscardDialog and bulk delete handler. * auto-claude: subtask-1-2 - Fix terminal worktree delete dialog auto-close at line 954 Add e.preventDefault() to delete action onClick handler to prevent dialog from auto-closing before async deletion completes. Now matches pattern from bulk delete and discard dialogs. * auto-claude: subtask-1-2 - Fix terminal worktree delete dialog auto-close - Prevent dialog from closing while isDeletingTerminal is true - Added success toast notification after delete - Follows pattern from DiscardDialog and task worktree delete * auto-claude: subtask-2-1 - Replace execFileSync git call with cleanupWorktree - Import cleanupWorktree utility in terminal worktree handlers - Replace direct git worktree remove call with cleanupWorktree() - Update cleanupWorktree to support both task and terminal worktrees - Add validation for terminal worktree directory in security check - Handle Windows file locks and orphaned worktrees automatically - Auto-commits uncommitted changes before deletion - Includes retry logic for Windows file lock issues * auto-claude: subtask-2-1 - Replace execFileSync git call with cleanupWorktree Verified that cleanupWorktree() utility is already properly implemented in removeTerminalWorktree() function. The implementation includes: - Import of cleanupWorktree from '../../utils/worktree-cleanup' - Async function signature - Proper await cleanupWorktree() call with correct parameters - Error handling via cleanupResult.success check - Warning logging via cleanupResult.warnings TypeScript compilation verified with no errors. * auto-claude: Update build progress for subtask-2-1 completion * auto-claude: subtask-3-1 - Fix terminalWorktrees state update to handle empty arrays correctly * auto-claude: subtask-3-1 - Fix terminalWorktrees state update to handle empty arrays Ensure React detects state changes when terminalWorktrees becomes empty by: - Creating a new array reference using spread operator - Explicitly checking if data is an array before spreading - This forces React to re-render and show the empty state correctly Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Fix branch name fallback for terminal worktrees and dialog race conditions - Add branchName parameter to WorktreeCleanupOptions so terminal worktrees pass config.branchName instead of falling back to auto-claude/{name} - Protect task worktree and bulk delete dialogs from closing during active delete operations (matching terminal worktree dialog pattern) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Test User <test@example.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
+49
@@ -0,0 +1,49 @@
|
||||
[2026-02-02 - Subtask 1-2 Completed]
|
||||
✅ Fixed terminal worktree delete dialog auto-close race condition
|
||||
|
||||
Changes made:
|
||||
1. Line 942: Updated AlertDialog onOpenChange handler to check isDeletingTerminal state
|
||||
- Prevents dialog from closing while deletion is in progress
|
||||
- Pattern: onOpenChange={(open) => !open && !isDeletingTerminal && setTerminalWorktreeToDelete(null)}
|
||||
|
||||
2. Lines 446-449: Added success toast notification after worktree refresh
|
||||
- Follows same pattern as task worktree delete
|
||||
- Shows user-friendly confirmation message
|
||||
|
||||
Result:
|
||||
- Dialog now stays open with loading spinner during async delete operation
|
||||
- List refreshes, toast shows, then dialog closes gracefully
|
||||
- No premature dialog dismissal race condition
|
||||
- Matches pattern from DiscardDialog.tsx and task worktree delete handler
|
||||
|
||||
Commit: a6846761e - "auto-claude: subtask-1-2 - Fix terminal worktree delete dialog auto-close"
|
||||
|
||||
Verification:
|
||||
- Manual testing required: Click delete on a terminal worktree
|
||||
- Expected: Dialog stays open with spinner → worktree deletes → dialog closes → list updates
|
||||
- No race condition where dialog closes before operation completes
|
||||
|
||||
=== Subtask-2-1 Completion ===
|
||||
Date: $(date -u +"%Y-%m-%d %H:%M:%S UTC")
|
||||
Status: COMPLETED
|
||||
|
||||
Description: Replace execFileSync git call with cleanupWorktree() utility in terminal worktree deletion handler
|
||||
|
||||
Findings:
|
||||
- The work was already completed in a previous session
|
||||
- The removeTerminalWorktree() function (apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts:686-757) already implements the required changes:
|
||||
* cleanupWorktree import present (line 27)
|
||||
* Function is async (line 686)
|
||||
* Uses await cleanupWorktree() with proper parameters (lines 716-724)
|
||||
* Handles cleanupResult.success correctly (lines 726-731)
|
||||
* Logs warnings when present (lines 734-736)
|
||||
* Cleans up metadata file after successful deletion (lines 739-747)
|
||||
|
||||
Verification:
|
||||
- TypeScript compilation: PASSED (npm run typecheck - no errors)
|
||||
- Code pattern: MATCHES reference implementation in worktree-cleanup.ts
|
||||
- Error handling: PROPERLY IMPLEMENTED
|
||||
- Security: Path validation included (lines 693-706)
|
||||
|
||||
Commit: 67c50ce95 "auto-claude: subtask-2-1 - Replace execFileSync git call with cleanupWorktree"
|
||||
|
||||
+352
@@ -0,0 +1,352 @@
|
||||
{
|
||||
"feature": "fix(worktrees): Dialog auto-close race condition and terminal worktree deletion failures",
|
||||
"workflow_type": "feature",
|
||||
"workflow_rationale": "While these are bug fixes, they require coordinating async operations, importing utilities, and following established patterns across multiple files—making this a standard feature-level task rather than a simple hotfix.",
|
||||
"phases": [
|
||||
{
|
||||
"id": "phase-1-fix-dialog-autoclose",
|
||||
"name": "Fix Dialog Auto-Close Race Condition",
|
||||
"type": "implementation",
|
||||
"description": "Add e.preventDefault() to AlertDialogAction onClick handlers to prevent Radix UI from auto-closing dialogs before async deletion completes (Bug #1)",
|
||||
"depends_on": [],
|
||||
"parallel_safe": true,
|
||||
"subtasks": [
|
||||
{
|
||||
"id": "subtask-1-1",
|
||||
"description": "Fix task worktree delete dialog auto-close at line 914",
|
||||
"service": "frontend",
|
||||
"files_to_modify": [
|
||||
"apps/frontend/src/renderer/components/Worktrees.tsx"
|
||||
],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [
|
||||
"apps/frontend/src/renderer/components/Worktrees.tsx:989-991",
|
||||
"apps/frontend/src/renderer/components/task-detail/task-review/DiscardDialog.tsx:70-72"
|
||||
],
|
||||
"verification": {
|
||||
"type": "manual",
|
||||
"instructions": "Click delete on a task worktree → dialog must stay open with spinner → worktree deletes → dialog closes → list updates"
|
||||
},
|
||||
"status": "completed",
|
||||
"notes": "Wrap handleDelete call with arrow function containing e.preventDefault(). Pattern already exists at line 989 for bulk delete. ✅ Completed - Applied e.preventDefault() pattern at line 914."
|
||||
},
|
||||
{
|
||||
"id": "subtask-1-2",
|
||||
"description": "Fix terminal worktree delete dialog auto-close at line 954",
|
||||
"service": "frontend",
|
||||
"files_to_modify": [
|
||||
"apps/frontend/src/renderer/components/Worktrees.tsx"
|
||||
],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [
|
||||
"apps/frontend/src/renderer/components/Worktrees.tsx:989-991",
|
||||
"apps/frontend/src/renderer/components/task-detail/task-review/DiscardDialog.tsx:70-72"
|
||||
],
|
||||
"verification": {
|
||||
"type": "manual",
|
||||
"instructions": "Click delete on a terminal worktree → dialog must stay open with spinner → worktree deletes → dialog closes → list updates"
|
||||
},
|
||||
"status": "completed",
|
||||
"notes": "✅ Completed - Fixed race condition by preventing dialog close while isDeletingTerminal is true (line 942). Added success toast notification (lines 446-449). Dialog now stays open with spinner until all async operations complete."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "phase-2-fix-windows-deletion",
|
||||
"name": "Fix Terminal Worktree Deletion on Windows",
|
||||
"type": "implementation",
|
||||
"description": "Replace direct git worktree remove call with cleanupWorktree() utility to handle Windows file locks and orphaned worktrees (Bug #2)",
|
||||
"depends_on": [],
|
||||
"parallel_safe": true,
|
||||
"subtasks": [
|
||||
{
|
||||
"id": "subtask-2-1",
|
||||
"description": "Replace execFileSync git call with cleanupWorktree() utility in terminal worktree deletion handler",
|
||||
"service": "frontend",
|
||||
"files_to_modify": [
|
||||
"apps/frontend/src/main/ipc-handlers/terminal/worktree-handlers.ts"
|
||||
],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [
|
||||
"apps/frontend/src/main/utils/worktree-cleanup.ts"
|
||||
],
|
||||
"verification": {
|
||||
"type": "command",
|
||||
"command": "npm run typecheck",
|
||||
"expected": "No TypeScript errors"
|
||||
},
|
||||
"status": "completed",
|
||||
"notes": "✅ Completed - cleanupWorktree() utility already imported and used in removeTerminalWorktree() function. Function is async, handles cleanupResult.success properly, and includes error handling with warnings. TypeScript compilation verified with no errors."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "phase-3-fix-state-update",
|
||||
"name": "Fix State Update for Empty Arrays",
|
||||
"type": "implementation",
|
||||
"description": "Fix conditional logic to properly update state when terminal worktree list is empty (Bug #3)",
|
||||
"depends_on": [],
|
||||
"parallel_safe": true,
|
||||
"subtasks": [
|
||||
{
|
||||
"id": "subtask-3-1",
|
||||
"description": "Fix terminalWorktrees state update to handle empty arrays correctly",
|
||||
"service": "frontend",
|
||||
"files_to_modify": [
|
||||
"apps/frontend/src/renderer/components/Worktrees.tsx"
|
||||
],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [],
|
||||
"verification": {
|
||||
"type": "manual",
|
||||
"instructions": "Delete all worktrees → UI should show empty state, not stale data"
|
||||
},
|
||||
"status": "pending",
|
||||
"notes": "Line 184-189: Change condition from 'if (terminalResult.success && terminalResult.data)' to 'if (terminalResult.success)'. Use 'terminalResult.data || []' fallback."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "phase-4-bonus-task-execution",
|
||||
"name": "Optional: Add Fallback to Task Execution Handler",
|
||||
"type": "implementation",
|
||||
"description": "Apply same cleanupWorktree() pattern to task execution handler for consistency (Bonus fix)",
|
||||
"depends_on": [
|
||||
"phase-2-fix-windows-deletion"
|
||||
],
|
||||
"parallel_safe": true,
|
||||
"subtasks": [
|
||||
{
|
||||
"id": "subtask-4-1",
|
||||
"description": "Add cleanupWorktree() fallback to task execution handler",
|
||||
"service": "frontend",
|
||||
"files_to_modify": [
|
||||
"apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts"
|
||||
],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [
|
||||
"apps/frontend/src/main/utils/worktree-cleanup.ts"
|
||||
],
|
||||
"verification": {
|
||||
"type": "command",
|
||||
"command": "npm run typecheck",
|
||||
"expected": "No TypeScript errors"
|
||||
},
|
||||
"status": "pending",
|
||||
"notes": "Lines 555-561: Same pattern as phase-2 subtask. Replace direct git call with cleanupWorktree() utility. This is optional but recommended for consistency."
|
||||
}
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "phase-5-integration",
|
||||
"name": "Integration Testing and Verification",
|
||||
"type": "integration",
|
||||
"description": "Verify all fixes work together: dialogs stay open, Windows deletion succeeds, state updates properly",
|
||||
"depends_on": [
|
||||
"phase-1-fix-dialog-autoclose",
|
||||
"phase-2-fix-windows-deletion",
|
||||
"phase-3-fix-state-update"
|
||||
],
|
||||
"parallel_safe": false,
|
||||
"subtasks": [
|
||||
{
|
||||
"id": "subtask-5-1",
|
||||
"description": "Run TypeScript compilation and linting",
|
||||
"service": "frontend",
|
||||
"all_services": false,
|
||||
"files_to_modify": [],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [],
|
||||
"verification": {
|
||||
"type": "command",
|
||||
"command": "cd apps/frontend && npm run typecheck && npm run lint",
|
||||
"expected": "No errors"
|
||||
},
|
||||
"status": "pending",
|
||||
"notes": "Ensure strict TypeScript mode and Biome linting pass"
|
||||
},
|
||||
{
|
||||
"id": "subtask-5-2",
|
||||
"description": "End-to-end manual verification of all deletion flows",
|
||||
"service": "frontend",
|
||||
"all_services": false,
|
||||
"files_to_modify": [],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [],
|
||||
"verification": {
|
||||
"type": "e2e",
|
||||
"steps": [
|
||||
"Launch Electron app with 'npm run dev'",
|
||||
"Navigate to Worktrees page",
|
||||
"Delete task worktree - verify dialog stays open with spinner",
|
||||
"Delete terminal worktree - verify dialog stays open with spinner",
|
||||
"Delete all worktrees - verify empty state displays",
|
||||
"Test bulk delete - verify no regression",
|
||||
"Check browser console for errors"
|
||||
]
|
||||
},
|
||||
"status": "pending",
|
||||
"notes": "Primary UX verification - dialogs must visibly stay open during deletion"
|
||||
},
|
||||
{
|
||||
"id": "subtask-5-3",
|
||||
"description": "Verify Windows-specific fixes (if platform available)",
|
||||
"service": "frontend",
|
||||
"all_services": false,
|
||||
"files_to_modify": [],
|
||||
"files_to_create": [],
|
||||
"patterns_from": [],
|
||||
"verification": {
|
||||
"type": "manual",
|
||||
"instructions": "On Windows: Delete terminal worktree with node_modules → should succeed without 'not a working tree' error. Delete orphaned worktree → should succeed."
|
||||
},
|
||||
"status": "pending",
|
||||
"notes": "Windows platform testing recommended but not blocking if unavailable. Bug #2 primarily affects Windows."
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"summary": {
|
||||
"total_phases": 5,
|
||||
"total_subtasks": 8,
|
||||
"services_involved": [
|
||||
"frontend"
|
||||
],
|
||||
"parallelism": {
|
||||
"max_parallel_phases": 3,
|
||||
"parallel_groups": [
|
||||
{
|
||||
"phases": [
|
||||
"phase-1-fix-dialog-autoclose",
|
||||
"phase-2-fix-windows-deletion",
|
||||
"phase-3-fix-state-update"
|
||||
],
|
||||
"reason": "All three phases have no dependencies and modify different files/lines - can be implemented in parallel"
|
||||
}
|
||||
],
|
||||
"recommended_workers": 1,
|
||||
"speedup_estimate": "Sequential execution recommended for careful testing of async behavior"
|
||||
},
|
||||
"startup_command": "cd apps/frontend && npm run dev"
|
||||
},
|
||||
"verification_strategy": {
|
||||
"risk_level": "medium",
|
||||
"skip_validation": false,
|
||||
"test_creation_phase": "post_implementation",
|
||||
"test_types_required": [
|
||||
"unit",
|
||||
"integration"
|
||||
],
|
||||
"security_scanning_required": false,
|
||||
"staging_deployment_required": false,
|
||||
"acceptance_criteria": [
|
||||
"Task worktree delete dialog stays open with loading spinner until completion",
|
||||
"Terminal worktree delete dialog stays open with loading spinner until completion",
|
||||
"Terminal worktrees with untracked files delete successfully on Windows without errors",
|
||||
"Orphaned worktrees delete successfully without 'not a working tree' fatal errors",
|
||||
"Deleting all worktrees results in empty state UI (not stale data)",
|
||||
"Bulk delete continues to work (regression test)",
|
||||
"No console errors during deletion workflows",
|
||||
"TypeScript strict mode compilation succeeds",
|
||||
"Biome linting passes"
|
||||
],
|
||||
"verification_steps": [
|
||||
{
|
||||
"name": "TypeScript Compilation",
|
||||
"command": "cd apps/frontend && npm run typecheck",
|
||||
"expected_outcome": "No TypeScript errors",
|
||||
"type": "test",
|
||||
"required": true,
|
||||
"blocking": true
|
||||
},
|
||||
{
|
||||
"name": "Biome Linting",
|
||||
"command": "cd apps/frontend && npm run lint",
|
||||
"expected_outcome": "No linting errors",
|
||||
"type": "test",
|
||||
"required": true,
|
||||
"blocking": true
|
||||
},
|
||||
{
|
||||
"name": "Frontend Unit Tests",
|
||||
"command": "cd apps/frontend && npm test",
|
||||
"expected_outcome": "All tests pass",
|
||||
"type": "test",
|
||||
"required": false,
|
||||
"blocking": false
|
||||
}
|
||||
],
|
||||
"reasoning": "Medium risk due to async dialog lifecycle and Windows-specific behavior. Unit tests for state logic and integration tests for complete deletion flows required. Manual Windows testing needed for Bug #2."
|
||||
},
|
||||
"qa_acceptance": {
|
||||
"unit_tests": {
|
||||
"required": false,
|
||||
"commands": [
|
||||
"cd apps/frontend && npm test"
|
||||
],
|
||||
"minimum_coverage": null
|
||||
},
|
||||
"integration_tests": {
|
||||
"required": true,
|
||||
"commands": [
|
||||
"cd apps/frontend && npm run typecheck",
|
||||
"cd apps/frontend && npm run lint"
|
||||
],
|
||||
"services_to_test": [
|
||||
"frontend"
|
||||
]
|
||||
},
|
||||
"e2e_tests": {
|
||||
"required": true,
|
||||
"commands": [
|
||||
"cd apps/frontend && npm run dev"
|
||||
],
|
||||
"flows": [
|
||||
"task-worktree-deletion",
|
||||
"terminal-worktree-deletion",
|
||||
"delete-all-worktrees",
|
||||
"bulk-delete-regression"
|
||||
]
|
||||
},
|
||||
"browser_verification": {
|
||||
"required": true,
|
||||
"pages": [
|
||||
{
|
||||
"url": "Electron app → Worktrees page",
|
||||
"checks": [
|
||||
"Delete dialogs stay open during async operations",
|
||||
"Loading spinners visible",
|
||||
"No premature dialog closes",
|
||||
"Empty state displays after deleting all worktrees",
|
||||
"No console errors"
|
||||
]
|
||||
}
|
||||
]
|
||||
},
|
||||
"database_verification": {
|
||||
"required": false,
|
||||
"checks": []
|
||||
},
|
||||
"platform_specific_testing": {
|
||||
"required": true,
|
||||
"platforms": [
|
||||
"Windows",
|
||||
"macOS",
|
||||
"Linux"
|
||||
],
|
||||
"notes": "Bug #2 primarily affects Windows - prioritize Windows testing if available"
|
||||
}
|
||||
},
|
||||
"qa_signoff": null,
|
||||
"executionPhase": "coding",
|
||||
"status": "in_progress",
|
||||
"planStatus": "in_progress",
|
||||
"updated_at": "2026-02-02T12:31:54.018Z",
|
||||
"xstateState": "coding",
|
||||
"lastEvent": {
|
||||
"eventId": "6629a4c1-4b4e-434c-b9a4-916845f09d41",
|
||||
"sequence": 0,
|
||||
"type": "PLANNING_COMPLETE",
|
||||
"timestamp": "2026-01-31T22:58:46.447840+00:00"
|
||||
}
|
||||
}
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
} from '../../worktree-paths';
|
||||
import { getIsolatedGitEnv } from '../../utils/git-isolation';
|
||||
import { getToolPath } from '../../cli-tool-manager';
|
||||
import { cleanupWorktree } from '../../utils/worktree-cleanup';
|
||||
|
||||
// Promisify execFile for async operations
|
||||
const execFileAsync = promisify(execFile);
|
||||
@@ -780,35 +781,27 @@ async function removeTerminalWorktree(
|
||||
}
|
||||
|
||||
try {
|
||||
if (existsSync(worktreePath)) {
|
||||
// Use async to avoid blocking the main process on large repos
|
||||
await execFileAsync(getToolPath('git'), ['worktree', 'remove', '--force', worktreePath], {
|
||||
cwd: projectPath,
|
||||
encoding: 'utf-8',
|
||||
timeout: 60000,
|
||||
env: getIsolatedGitEnv(),
|
||||
});
|
||||
debugLog('[TerminalWorktree] Removed git worktree');
|
||||
// Use the robust cleanupWorktree utility to handle Windows file locks and orphaned worktrees
|
||||
const cleanupResult = await cleanupWorktree({
|
||||
worktreePath,
|
||||
projectPath,
|
||||
specId: name,
|
||||
logPrefix: '[TerminalWorktree]',
|
||||
deleteBranch: deleteBranch && config.hasGitBranch,
|
||||
branchName: config.branchName || undefined,
|
||||
commitMessage: 'Auto-save before terminal worktree deletion',
|
||||
});
|
||||
|
||||
if (!cleanupResult.success) {
|
||||
return {
|
||||
success: false,
|
||||
error: cleanupResult.warnings.join('; ') || 'Failed to remove worktree',
|
||||
};
|
||||
}
|
||||
|
||||
if (deleteBranch && config.hasGitBranch && config.branchName) {
|
||||
// Re-validate branch name from config file (defense in depth - config could be modified)
|
||||
if (!GIT_BRANCH_REGEX.test(config.branchName)) {
|
||||
debugError('[TerminalWorktree] Invalid branch name in config:', config.branchName);
|
||||
} else {
|
||||
try {
|
||||
// Use async to avoid blocking the main process
|
||||
await execFileAsync(getToolPath('git'), ['branch', '-D', config.branchName], {
|
||||
cwd: projectPath,
|
||||
encoding: 'utf-8',
|
||||
timeout: 30000,
|
||||
env: getIsolatedGitEnv(),
|
||||
});
|
||||
debugLog('[TerminalWorktree] Deleted branch:', config.branchName);
|
||||
} catch {
|
||||
debugLog('[TerminalWorktree] Branch not found or already deleted:', config.branchName);
|
||||
}
|
||||
}
|
||||
// Log warnings if any occurred during cleanup
|
||||
if (cleanupResult.warnings.length > 0) {
|
||||
debugLog('[TerminalWorktree] Cleanup completed with warnings:', cleanupResult.warnings);
|
||||
}
|
||||
|
||||
// Remove metadata file
|
||||
|
||||
@@ -20,7 +20,7 @@ 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';
|
||||
import { getTaskWorktreeDir, getTerminalWorktreeDir, isPathWithinBase } from '../worktree-paths';
|
||||
|
||||
/**
|
||||
* Options for worktree cleanup operation
|
||||
@@ -38,6 +38,8 @@ export interface WorktreeCleanupOptions {
|
||||
logPrefix?: string;
|
||||
/** Whether to delete the associated branch (default: true) */
|
||||
deleteBranch?: boolean;
|
||||
/** Explicit branch name to use for deletion (overrides auto-detection fallback) */
|
||||
branchName?: string;
|
||||
/** Timeout in milliseconds for git operations (default: 30000) */
|
||||
timeout?: number;
|
||||
/** Maximum retries for directory deletion on Windows (default: 3) */
|
||||
@@ -63,7 +65,7 @@ export interface WorktreeCleanupResult {
|
||||
/**
|
||||
* Gets the worktree branch name based on spec ID
|
||||
*/
|
||||
function getWorktreeBranch(worktreePath: string, specId: string, timeout: number): string | null {
|
||||
function getWorktreeBranch(worktreePath: string, specId: string, timeout: number, explicitBranchName?: string): string | null {
|
||||
// First try to get branch from the worktree's HEAD
|
||||
if (existsSync(worktreePath)) {
|
||||
try {
|
||||
@@ -78,10 +80,15 @@ function getWorktreeBranch(worktreePath: string, specId: string, timeout: number
|
||||
return branch;
|
||||
}
|
||||
} catch {
|
||||
// Worktree might be corrupted, fall back to naming convention
|
||||
// Worktree might be corrupted, fall back to explicit name or naming convention
|
||||
}
|
||||
}
|
||||
|
||||
// Use explicit branch name if provided (e.g., terminal worktrees use terminal/{name})
|
||||
if (explicitBranchName) {
|
||||
return explicitBranchName;
|
||||
}
|
||||
|
||||
// Fall back to the naming convention: auto-claude/{spec-id}
|
||||
return `auto-claude/${specId}`;
|
||||
}
|
||||
@@ -171,6 +178,7 @@ export async function cleanupWorktree(options: WorktreeCleanupOptions): Promise<
|
||||
commitMessage = 'Auto-save before deletion',
|
||||
logPrefix = '[WORKTREE_CLEANUP]',
|
||||
deleteBranch = true,
|
||||
branchName,
|
||||
timeout = 30000,
|
||||
maxRetries = 3,
|
||||
retryDelay = 500
|
||||
@@ -179,11 +187,15 @@ export async function cleanupWorktree(options: WorktreeCleanupOptions): Promise<
|
||||
const warnings: string[] = [];
|
||||
let autoCommitted = false;
|
||||
|
||||
// Security: Validate that worktreePath is within the expected worktree directory
|
||||
// Security: Validate that worktreePath is within the expected worktree directories
|
||||
// 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`);
|
||||
// Supports both task worktrees (.auto-claude/worktrees/tasks) and terminal worktrees (.auto-claude/worktrees/terminal)
|
||||
const taskBase = getTaskWorktreeDir(projectPath);
|
||||
const terminalBase = getTerminalWorktreeDir(projectPath);
|
||||
const isValidPath = isPathWithinBase(worktreePath, taskBase) || isPathWithinBase(worktreePath, terminalBase);
|
||||
|
||||
if (!isValidPath) {
|
||||
console.error(`${logPrefix} Security: Path validation failed - worktree path is outside expected directories`);
|
||||
return {
|
||||
success: false,
|
||||
warnings: ['Invalid worktree path']
|
||||
@@ -191,7 +203,7 @@ export async function cleanupWorktree(options: WorktreeCleanupOptions): Promise<
|
||||
}
|
||||
|
||||
// 1. Get the branch name before we delete the directory
|
||||
const branch = getWorktreeBranch(worktreePath, specId, timeout);
|
||||
const branch = getWorktreeBranch(worktreePath, specId, timeout, branchName);
|
||||
console.warn(`${logPrefix} Starting cleanup for worktree: ${worktreePath}`);
|
||||
if (branch) {
|
||||
console.warn(`${logPrefix} Associated branch: ${branch}`);
|
||||
|
||||
@@ -175,17 +175,21 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
console.log('[Worktrees] Task worktrees result:', taskResult);
|
||||
console.log('[Worktrees] Terminal worktrees result:', terminalResult);
|
||||
|
||||
if (taskResult.success && taskResult.data) {
|
||||
setWorktrees(taskResult.data.worktrees);
|
||||
if (taskResult.success) {
|
||||
// Always update state when successful, even if data is null/undefined
|
||||
setWorktrees(taskResult.data?.worktrees || []);
|
||||
} else {
|
||||
setError(taskResult.error || 'Failed to load task worktrees');
|
||||
}
|
||||
|
||||
if (terminalResult.success && terminalResult.data) {
|
||||
console.log('[Worktrees] Setting terminal worktrees:', terminalResult.data);
|
||||
setTerminalWorktrees(terminalResult.data);
|
||||
if (terminalResult.success) {
|
||||
// Always update state when successful, ensuring a new array reference to force React re-render
|
||||
// This is critical when data is an empty array - we need a new reference to update the UI
|
||||
const newWorktrees = Array.isArray(terminalResult.data) ? [...terminalResult.data] : [];
|
||||
console.log('[Worktrees] Setting terminal worktrees:', newWorktrees);
|
||||
setTerminalWorktrees(newWorktrees);
|
||||
} else {
|
||||
console.warn('[Worktrees] Terminal worktrees fetch failed or empty:', terminalResult);
|
||||
console.warn('[Worktrees] Terminal worktrees fetch failed:', terminalResult);
|
||||
}
|
||||
} catch (err) {
|
||||
console.error('[Worktrees] Error loading worktrees:', err);
|
||||
@@ -443,6 +447,10 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
if (result.success) {
|
||||
// Refresh worktrees after successful delete
|
||||
await loadWorktrees();
|
||||
toast({
|
||||
title: t('common:actions.success'),
|
||||
description: t('common:worktrees.deleteSuccess', { branch: terminalWorktreeToDelete.name }),
|
||||
});
|
||||
setTerminalWorktreeToDelete(null);
|
||||
} else {
|
||||
setError(result.error || 'Failed to delete terminal worktree');
|
||||
@@ -894,7 +902,7 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
</Dialog>
|
||||
|
||||
{/* Delete Confirmation Dialog */}
|
||||
<AlertDialog open={showDeleteConfirm} onOpenChange={setShowDeleteConfirm}>
|
||||
<AlertDialog open={showDeleteConfirm} onOpenChange={(open) => !open && !isDeleting && setShowDeleteConfirm(false)}>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>Delete Worktree?</AlertDialogTitle>
|
||||
@@ -911,7 +919,10 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
<AlertDialogFooter>
|
||||
<AlertDialogCancel disabled={isDeleting}>Cancel</AlertDialogCancel>
|
||||
<AlertDialogAction
|
||||
onClick={handleDelete}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
handleDelete();
|
||||
}}
|
||||
disabled={isDeleting}
|
||||
className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
|
||||
>
|
||||
@@ -932,7 +943,7 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
</AlertDialog>
|
||||
|
||||
{/* Terminal Worktree Delete Confirmation Dialog */}
|
||||
<AlertDialog open={!!terminalWorktreeToDelete} onOpenChange={(open) => !open && setTerminalWorktreeToDelete(null)}>
|
||||
<AlertDialog open={!!terminalWorktreeToDelete} onOpenChange={(open) => !open && !isDeletingTerminal && setTerminalWorktreeToDelete(null)}>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>Delete Terminal Worktree?</AlertDialogTitle>
|
||||
@@ -951,7 +962,10 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
<AlertDialogFooter>
|
||||
<AlertDialogCancel disabled={isDeletingTerminal}>Cancel</AlertDialogCancel>
|
||||
<AlertDialogAction
|
||||
onClick={handleDeleteTerminalWorktree}
|
||||
onClick={(e) => {
|
||||
e.preventDefault();
|
||||
handleDeleteTerminalWorktree();
|
||||
}}
|
||||
disabled={isDeletingTerminal}
|
||||
className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
|
||||
>
|
||||
@@ -972,7 +986,7 @@ export function Worktrees({ projectId }: WorktreesProps) {
|
||||
</AlertDialog>
|
||||
|
||||
{/* Bulk Delete Confirmation Dialog */}
|
||||
<AlertDialog open={showBulkDeleteConfirm} onOpenChange={setShowBulkDeleteConfirm}>
|
||||
<AlertDialog open={showBulkDeleteConfirm} onOpenChange={(open) => !open && !isBulkDeleting && setShowBulkDeleteConfirm(false)}>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle className="flex items-center gap-2">
|
||||
|
||||
Reference in New Issue
Block a user