fix(issue): Improve worktree implementation safety

- Use absolute paths via git rev-parse --show-toplevel
- Add cleanup traps for graceful failure handling (EXIT/INT/TERM)
- Add git worktree prune at startup for stale worktrees
- Validate main repo state before merge (fallback to PR if dirty)
- Change default to "Create PR" for parallel execution safety
- Move worktree directory to .ccw/worktrees/ (gitignored)
This commit is contained in:
catlog22
2026-01-03 11:56:15 +08:00
parent 713894090d
commit 990cf8a05d
2 changed files with 112 additions and 21 deletions

View File

@@ -148,9 +148,14 @@ TodoWrite({
console.log(`\n### Executing Solutions (DAG batch 1): ${batch.join(', ')}`); console.log(`\n### Executing Solutions (DAG batch 1): ${batch.join(', ')}`);
// Setup worktree base directory if needed // Setup worktree base directory if needed (using absolute paths)
if (useWorktree) { if (useWorktree) {
Bash('mkdir -p ../.worktrees'); // Use absolute paths to avoid issues when running from subdirectories
const repoRoot = Bash('git rev-parse --show-toplevel').trim();
const worktreeBase = `${repoRoot}/.ccw/worktrees`;
Bash(`mkdir -p "${worktreeBase}"`);
// Prune stale worktrees from previous interrupted executions
Bash('git worktree prune');
} }
// Launch ALL solutions in batch in parallel (DAG guarantees no conflicts) // Launch ALL solutions in batch in parallel (DAG guarantees no conflicts)
@@ -167,13 +172,34 @@ batch.forEach(id => updateTodo(id, 'completed'));
```javascript ```javascript
function dispatchExecutor(solutionId, executorType, useWorktree = false) { function dispatchExecutor(solutionId, executorType, useWorktree = false) {
// Worktree setup commands (if enabled) // Worktree setup commands (if enabled) - using absolute paths
const worktreeSetup = useWorktree ? ` const worktreeSetup = useWorktree ? `
### Step 0: Setup Isolated Worktree ### Step 0: Setup Isolated Worktree
\`\`\`bash \`\`\`bash
# Use absolute paths to avoid issues when running from subdirectories
REPO_ROOT=$(git rev-parse --show-toplevel)
WORKTREE_BASE="\${REPO_ROOT}/.ccw/worktrees"
WORKTREE_NAME="exec-${solutionId}-$(date +%H%M%S)" WORKTREE_NAME="exec-${solutionId}-$(date +%H%M%S)"
WORKTREE_PATH="../.worktrees/\${WORKTREE_NAME}" WORKTREE_PATH="\${WORKTREE_BASE}/\${WORKTREE_NAME}"
# Ensure worktree base exists
mkdir -p "\${WORKTREE_BASE}"
# Prune stale worktrees
git worktree prune
# Create worktree
git worktree add "\${WORKTREE_PATH}" -b "\${WORKTREE_NAME}" git worktree add "\${WORKTREE_PATH}" -b "\${WORKTREE_NAME}"
# Setup cleanup trap for graceful failure handling
cleanup_worktree() {
echo "Cleaning up worktree due to interruption..."
cd "\${REPO_ROOT}" 2>/dev/null || true
git worktree remove "\${WORKTREE_PATH}" --force 2>/dev/null || true
echo "Worktree removed. Branch '\${WORKTREE_NAME}' kept for inspection."
}
trap cleanup_worktree EXIT INT TERM
cd "\${WORKTREE_PATH}" cd "\${WORKTREE_PATH}"
\`\`\` \`\`\`
` : ''; ` : '';
@@ -190,8 +216,8 @@ AskUserQuestion({
header: "Merge", header: "Merge",
multiSelect: false, multiSelect: false,
options: [ options: [
{ label: "Merge to main", description: "Merge branch and cleanup worktree" }, { label: "Create PR (Recommended)", description: "Push branch and create pull request - safest for parallel execution" },
{ label: "Create PR", description: "Push branch and create pull request" }, { label: "Merge to main", description: "Merge branch and cleanup worktree (requires clean main)" },
{ label: "Keep branch", description: "Cleanup worktree, keep branch for manual handling" } { label: "Keep branch", description: "Cleanup worktree, keep branch for manual handling" }
] ]
}] }]
@@ -200,22 +226,44 @@ AskUserQuestion({
**Based on selection:** **Based on selection:**
\`\`\`bash \`\`\`bash
MAIN_REPO=$(git worktree list | head -1 | awk '{print $1}') # Disable cleanup trap before intentional cleanup
cd "\${MAIN_REPO}" trap - EXIT INT TERM
# Merge to main: # Return to repo root (use REPO_ROOT from setup)
git merge --no-ff "\${WORKTREE_NAME}" -m "Merge solution ${solutionId}" cd "\${REPO_ROOT}"
git worktree remove "\${WORKTREE_PATH}" && git branch -d "\${WORKTREE_NAME}"
# Create PR: # Validate main repo state before merge
validate_main_clean() {
if [[ -n \$(git status --porcelain) ]]; then
echo "⚠️ Warning: Main repo has uncommitted changes."
echo "Cannot auto-merge. Falling back to 'Create PR' option."
return 1
fi
return 0
}
# Create PR (Recommended for parallel execution):
git push -u origin "\${WORKTREE_NAME}" git push -u origin "\${WORKTREE_NAME}"
gh pr create --title "Solution ${solutionId}" --body "Issue queue execution" gh pr create --title "Solution ${solutionId}" --body "Issue queue execution"
git worktree remove "\${WORKTREE_PATH}" git worktree remove "\${WORKTREE_PATH}"
# Merge to main (only if main is clean):
if validate_main_clean; then
git merge --no-ff "\${WORKTREE_NAME}" -m "Merge solution ${solutionId}"
git worktree remove "\${WORKTREE_PATH}" && git branch -d "\${WORKTREE_NAME}"
else
# Fallback to PR if main is dirty
git push -u origin "\${WORKTREE_NAME}"
gh pr create --title "Solution ${solutionId}" --body "Issue queue execution (main had uncommitted changes)"
git worktree remove "\${WORKTREE_PATH}"
fi
# Keep branch: # Keep branch:
git worktree remove "\${WORKTREE_PATH}" git worktree remove "\${WORKTREE_PATH}"
echo "Branch \${WORKTREE_NAME} kept for manual handling" echo "Branch \${WORKTREE_NAME} kept for manual handling"
\`\`\` \`\`\`
**Parallel Execution Safety**: "Create PR" is the default and safest option for parallel executors, avoiding merge race conditions.
` : ''; ` : '';
const prompt = ` const prompt = `

View File

@@ -15,18 +15,40 @@ When `--worktree` is specified, create a separate git worktree to isolate work:
```bash ```bash
# Step 0: Setup worktree before starting # Step 0: Setup worktree before starting
# Use absolute paths to avoid issues when running from subdirectories
REPO_ROOT=$(git rev-parse --show-toplevel)
WORKTREE_BASE="${REPO_ROOT}/.ccw/worktrees"
WORKTREE_NAME="issue-exec-$(date +%Y%m%d-%H%M%S)" WORKTREE_NAME="issue-exec-$(date +%Y%m%d-%H%M%S)"
WORKTREE_PATH="../.worktrees/${WORKTREE_NAME}" WORKTREE_PATH="${WORKTREE_BASE}/${WORKTREE_NAME}"
# Ensure worktree base directory exists (gitignored)
mkdir -p "${WORKTREE_BASE}"
# Prune stale worktrees from previous interrupted executions
git worktree prune
# Create worktree from current branch # Create worktree from current branch
git worktree add "${WORKTREE_PATH}" -b "${WORKTREE_NAME}" git worktree add "${WORKTREE_PATH}" -b "${WORKTREE_NAME}"
# Setup cleanup trap for graceful failure handling
cleanup_worktree() {
echo "Cleaning up worktree due to interruption..."
cd "${REPO_ROOT}" 2>/dev/null || true
git worktree remove "${WORKTREE_PATH}" --force 2>/dev/null || true
# Keep branch for debugging failed executions
echo "Worktree removed. Branch '${WORKTREE_NAME}' kept for inspection."
}
trap cleanup_worktree EXIT INT TERM
# Change to worktree directory # Change to worktree directory
cd "${WORKTREE_PATH}" cd "${WORKTREE_PATH}"
# Now execute in isolated worktree... # Now execute in isolated worktree...
``` ```
**Note**: Add `.ccw/worktrees/` to `.gitignore` to prevent tracking worktree contents.
**Benefits:** **Benefits:**
- Parallel executors don't conflict with each other - Parallel executors don't conflict with each other
- Main working directory stays clean - Main working directory stays clean
@@ -63,16 +85,35 @@ AskUserQuestion({
**Based on user selection:** **Based on user selection:**
```bash ```bash
# Return to main repo first # Disable cleanup trap before intentional cleanup
MAIN_REPO=$(git worktree list | head -1 | awk '{print $1}') trap - EXIT INT TERM
cd "${MAIN_REPO}"
# Option 1: Merge to main # Return to main repo first (use REPO_ROOT from setup)
git merge "${WORKTREE_NAME}" --no-ff -m "Merge issue queue execution: ${WORKTREE_NAME}" cd "${REPO_ROOT}"
git worktree remove "${WORKTREE_PATH}"
git branch -d "${WORKTREE_NAME}"
# Option 2: Create PR # Validate main repo state before merge (prevents conflicts)
validate_main_clean() {
if [[ -n $(git status --porcelain) ]]; then
echo "⚠️ Warning: Main repo has uncommitted changes."
echo "Cannot auto-merge. Falling back to 'Create PR' option."
return 1
fi
return 0
}
# Option 1: Merge to main (only if main is clean)
if validate_main_clean; then
git merge "${WORKTREE_NAME}" --no-ff -m "Merge issue queue execution: ${WORKTREE_NAME}"
git worktree remove "${WORKTREE_PATH}"
git branch -d "${WORKTREE_NAME}"
else
# Fallback to PR if main is dirty
git push -u origin "${WORKTREE_NAME}"
gh pr create --title "Issue Queue: ${WORKTREE_NAME}" --body "Automated issue queue execution (main had uncommitted changes)"
git worktree remove "${WORKTREE_PATH}"
fi
# Option 2: Create PR (Recommended for parallel execution)
git push -u origin "${WORKTREE_NAME}" git push -u origin "${WORKTREE_NAME}"
gh pr create --title "Issue Queue: ${WORKTREE_NAME}" --body "Automated issue queue execution" gh pr create --title "Issue Queue: ${WORKTREE_NAME}" --body "Automated issue queue execution"
git worktree remove "${WORKTREE_PATH}" git worktree remove "${WORKTREE_PATH}"
@@ -84,6 +125,8 @@ git worktree remove "${WORKTREE_PATH}"
echo "Branch '${WORKTREE_NAME}' kept. Merge manually when ready." echo "Branch '${WORKTREE_NAME}' kept. Merge manually when ready."
``` ```
**Parallel Execution Safety**: For parallel executors, "Create PR" is the safest option as it avoids race conditions during merge. Multiple PRs can be reviewed and merged sequentially.
## Execution Flow ## Execution Flow
``` ```