Merge pull request #1502 from Tibsfox/fix/worktree-cleanup-1496
fix(worktree): add post-execution cleanup for orphan worktrees
This commit is contained in:
@@ -336,6 +336,39 @@ Execute each selected wave in sequence. Within a wave: parallel if `PARALLELIZAT
|
||||
```
|
||||
If hooks fail: report the failure and ask "Fix hook issues now?" or "Continue to next wave?"
|
||||
|
||||
4.5. **Worktree cleanup (when `isolation="worktree"` was used):**
|
||||
|
||||
When executor agents ran in worktree isolation, their commits land on temporary branches in separate working trees. After the wave completes, merge these changes back and clean up:
|
||||
|
||||
```bash
|
||||
# List worktrees created by this wave's agents
|
||||
WORKTREES=$(git worktree list --porcelain | grep "^worktree " | grep -v "$(pwd)$" | sed 's/^worktree //')
|
||||
|
||||
for WT in $WORKTREES; do
|
||||
# Get the branch name for this worktree
|
||||
WT_BRANCH=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null)
|
||||
if [ -n "$WT_BRANCH" ] && [ "$WT_BRANCH" != "HEAD" ]; then
|
||||
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD)
|
||||
|
||||
# Merge the worktree branch into the current branch
|
||||
git merge "$WT_BRANCH" --no-edit -m "chore: merge executor worktree ($WT_BRANCH)" 2>&1 || {
|
||||
echo "⚠ Merge conflict from worktree $WT_BRANCH — resolve manually"
|
||||
continue
|
||||
}
|
||||
|
||||
# Remove the worktree
|
||||
git worktree remove "$WT" --force 2>/dev/null || true
|
||||
|
||||
# Delete the temporary branch
|
||||
git branch -D "$WT_BRANCH" 2>/dev/null || true
|
||||
fi
|
||||
done
|
||||
```
|
||||
|
||||
**If `workflow.use_worktrees` is `false`:** Agents ran on the main working tree — skip this step entirely.
|
||||
|
||||
**If no worktrees found:** Skip silently — agents may have been spawned without worktree isolation.
|
||||
|
||||
5. **Report completion — spot-check claims first:**
|
||||
|
||||
For each SUMMARY.md:
|
||||
|
||||
@@ -581,9 +581,23 @@ ${AGENT_SKILLS_EXECUTOR}
|
||||
```
|
||||
|
||||
After executor returns:
|
||||
1. Verify summary exists at `${QUICK_DIR}/${quick_id}-SUMMARY.md`
|
||||
2. Extract commit hash from executor output
|
||||
3. Report completion status
|
||||
1. **Worktree cleanup:** If the executor ran with `isolation="worktree"`, merge the worktree branch back and clean up:
|
||||
```bash
|
||||
# Find worktrees created by the executor
|
||||
WORKTREES=$(git worktree list --porcelain | grep "^worktree " | grep -v "$(pwd)$" | sed 's/^worktree //')
|
||||
for WT in $WORKTREES; do
|
||||
WT_BRANCH=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null)
|
||||
if [ -n "$WT_BRANCH" ] && [ "$WT_BRANCH" != "HEAD" ]; then
|
||||
git merge "$WT_BRANCH" --no-edit -m "chore: merge quick task worktree ($WT_BRANCH)" 2>&1 || echo "⚠ Merge conflict — resolve manually"
|
||||
git worktree remove "$WT" --force 2>/dev/null || true
|
||||
git branch -D "$WT_BRANCH" 2>/dev/null || true
|
||||
fi
|
||||
done
|
||||
```
|
||||
If `workflow.use_worktrees` is `false`, skip this step.
|
||||
2. Verify summary exists at `${QUICK_DIR}/${quick_id}-SUMMARY.md`
|
||||
3. Extract commit hash from executor output
|
||||
4. Report completion status
|
||||
|
||||
**Known Claude Code bug (classifyHandoffIfNeeded):** If executor reports "failed" with error `classifyHandoffIfNeeded is not defined`, this is a Claude Code runtime bug — not a real failure. Check if summary file exists and git log shows commits. If so, treat as successful.
|
||||
|
||||
|
||||
70
tests/worktree-cleanup.test.cjs
Normal file
70
tests/worktree-cleanup.test.cjs
Normal file
@@ -0,0 +1,70 @@
|
||||
/**
|
||||
* GSD Tools Tests - worktree cleanup after executor completes
|
||||
*
|
||||
* Validates that execute-phase.md and quick.md include post-execution
|
||||
* worktree cleanup logic (merge branch, remove worktree, delete branch).
|
||||
*
|
||||
* Closes: #1496
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
describe('worktree cleanup after executor completes (#1496)', () => {
|
||||
const executePhasePath = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'execute-phase.md');
|
||||
const quickPath = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md');
|
||||
|
||||
test('execute-phase.md includes worktree cleanup step', () => {
|
||||
const content = fs.readFileSync(executePhasePath, 'utf8');
|
||||
assert.ok(content.includes('Worktree cleanup'),
|
||||
'execute-phase should have a worktree cleanup step');
|
||||
assert.ok(content.includes('git worktree remove'),
|
||||
'cleanup should remove worktrees');
|
||||
assert.ok(content.includes('git branch -D'),
|
||||
'cleanup should delete temporary branches');
|
||||
});
|
||||
|
||||
test('execute-phase.md merges worktree branch before removing', () => {
|
||||
const content = fs.readFileSync(executePhasePath, 'utf8');
|
||||
assert.ok(content.includes('git merge'),
|
||||
'cleanup should merge worktree branch into current branch');
|
||||
});
|
||||
|
||||
test('execute-phase.md handles merge conflicts gracefully', () => {
|
||||
const content = fs.readFileSync(executePhasePath, 'utf8');
|
||||
assert.ok(
|
||||
content.includes('Merge conflict') || content.includes('merge conflict'),
|
||||
'cleanup should handle merge conflicts gracefully'
|
||||
);
|
||||
});
|
||||
|
||||
test('execute-phase.md skips cleanup when use_worktrees is false', () => {
|
||||
const content = fs.readFileSync(executePhasePath, 'utf8');
|
||||
assert.ok(content.includes('use_worktrees'),
|
||||
'cleanup should respect workflow.use_worktrees config');
|
||||
});
|
||||
|
||||
test('quick.md includes worktree cleanup after executor returns', () => {
|
||||
const content = fs.readFileSync(quickPath, 'utf8');
|
||||
assert.ok(content.includes('Worktree cleanup') || content.includes('worktree cleanup'),
|
||||
'quick should have worktree cleanup');
|
||||
assert.ok(content.includes('git worktree remove'),
|
||||
'quick cleanup should remove worktrees');
|
||||
assert.ok(content.includes('git branch -D'),
|
||||
'quick cleanup should delete temporary branches');
|
||||
});
|
||||
|
||||
test('quick.md merges worktree branch before removing', () => {
|
||||
const content = fs.readFileSync(quickPath, 'utf8');
|
||||
assert.ok(content.includes('git merge'),
|
||||
'quick cleanup should merge worktree branch');
|
||||
});
|
||||
|
||||
test('cleanup uses git worktree list to discover orphans', () => {
|
||||
const content = fs.readFileSync(executePhasePath, 'utf8');
|
||||
assert.ok(content.includes('git worktree list'),
|
||||
'cleanup should discover worktrees via git worktree list');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user