diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index d763325e9..2db0e2f29 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -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: diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index 0c0bbf0cf..475fd6194 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.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. diff --git a/tests/worktree-cleanup.test.cjs b/tests/worktree-cleanup.test.cjs new file mode 100644 index 000000000..0fe56153e --- /dev/null +++ b/tests/worktree-cleanup.test.cjs @@ -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'); + }); +});