From 316337816eae75927e8ed4f2000201a0869e104f Mon Sep 17 00:00:00 2001 From: Tibsfox Date: Mon, 30 Mar 2026 13:57:56 -0700 Subject: [PATCH] fix(worktree): add post-execution cleanup for orphan worktrees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Executor agents spawned with isolation="worktree" committed to temporary branches in separate working trees, but no step existed to merge those changes back or clean up. This left orphan worktrees and unmerged branches after every execution. Changes: - execute-phase.md: add step 4.5 "Worktree cleanup" after wave completion — merges worktree branch, removes worktree, deletes temp branch. Handles merge conflicts gracefully. - quick.md: add worktree cleanup step after executor returns, before summary verification - Both workflows skip cleanup when workflow.use_worktrees is false - Both workflows skip silently when no worktrees are found Closes #1496 Co-Authored-By: Claude Opus 4.6 --- get-shit-done/workflows/execute-phase.md | 33 +++++++++++ get-shit-done/workflows/quick.md | 20 ++++++- tests/worktree-cleanup.test.cjs | 70 ++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 tests/worktree-cleanup.test.cjs diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index 0bdb11174..20c7f6141 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -313,6 +313,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 89441f965..26ccb9d29 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -565,9 +565,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'); + }); +});