fix(3264): document cross-wave-deviation cleanup tail in execute-phase step 5.5 (#3273)

* fix(3264): document cross-wave-deviation cleanup tail in execute-phase step 5.5

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(changeset): add fragment for #3273

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-09 00:14:54 -04:00
committed by GitHub
parent ac51864621
commit d8a93ad12d
3 changed files with 184 additions and 0 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3273
---
Orchestrators now have a documented cleanup-tail snippet to run when wave merges deviate from the templated path (e.g., cross-wave dependency merges with custom messages) — residual worktree-agent-* directories can be removed without manual forensics.

View File

@@ -694,6 +694,10 @@ increases monotonically across waves. `{status}` is `complete` (success),
5.5. **Worktree cleanup (when `isolation="worktree"` was used):**
**Standard wave contract:** Each wave's worktrees merge to main via the templated path below before the next wave's worktrees fork. The cleanup loop runs once per wave at the end of the wave lifecycle. Worktrees created in wave N must be fully removed before wave N+1 forks new ones.
**Cross-wave dependency deviation (supported execution mode):** When the orchestrator legitimately deviates from the standard wave model — for example, a phase with cross-wave plan dependencies that requires custom inter-worktree base-update merges (e.g., `merge: bring 09-01 + 09-02 into 09-03 base`) — the cleanup loop below is NOT automatically re-entered for those custom merges. The deviation path produces correct final history but bypasses this loop, leaving `worktree-agent-*` directories in place. Use the **cleanup-tail snippet** below to remove any residual worktrees after such a deviation.
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
@@ -828,8 +832,43 @@ increases monotonically across waves. `{status}` is `complete` (success),
done < <(git worktree list --porcelain | grep "^worktree " | grep "\.claude/worktrees/agent-" | sed 's/^worktree //')
```
**Cleanup-tail snippet (use after any wave whose merges did not flow through the templated path above):**
If the orchestrator deviated from the standard wave merge path (e.g., custom inter-worktree base-update merges with `merge: bring …` style messages), run this snippet after the custom merges are complete. It discovers and removes any residual `worktree-agent-*` worktrees. Safe to run when no residuals exist — it is a no-op in that case.
```bash
# Cleanup-tail: remove residual agent worktrees after a cross-wave-dependency deviation.
# Inclusion-based filter (#2774): match ONLY agent-spawned worktrees under
# `.claude/worktrees/agent-`. Do NOT use exclusion filters (grep -v "$(pwd)$") —
# they destroy the parent workspace's .git in multi-workspace or cross-drive setups.
# Read line-by-line so worktree paths containing whitespace are preserved (#2774).
while IFS= read -r WT; do
[ -z "$WT" ] && continue
WT_BRANCH=$(git -C "$WT" rev-parse --abbrev-ref HEAD 2>/dev/null)
[ -z "$WT_BRANCH" ] || [ "$WT_BRANCH" = "HEAD" ] && continue
echo "Cleaning up residual worktree: $WT (branch: $WT_BRANCH)"
git worktree unlock "$WT" 2>/dev/null || true
if ! git worktree remove "$WT" --force; then
WT_NAME=$(basename "$WT")
if [ -f ".git/worktrees/${WT_NAME}/locked" ]; then
echo "⚠ Worktree $WT is locked — unlock failed; manual cleanup required:"
echo " git worktree unlock \"$WT\" && git worktree remove \"$WT\" --force && git branch -D \"$WT_BRANCH\""
else
echo "⚠ Residual worktree at $WT — remove failed; manual cleanup required"
fi
else
git branch -D "$WT_BRANCH" 2>/dev/null || true
fi
done < <(git worktree list --porcelain | grep "^worktree " | grep "\.claude/worktrees/agent-" | sed 's/^worktree //')
git worktree prune
```
**When to skip step 5.5:**
**If no plan in this wave used worktree isolation** (project-level `USE_WORKTREES=false` OR every plan in the wave had `USE_WORKTREES_FOR_PLAN=false` — i.e. `WAVE_WORKTREE_PLANS` from step 2.5 is empty): all agents ran on the main working tree — skip this step entirely.
**If the orchestrator merged via custom messages (cross-wave-dependency deviation):** the templated cleanup loop above was not triggered for those merges. Run the cleanup-tail snippet above instead. After the snippet completes, proceed to step 5.6.
**If at least one plan used worktrees but others did not:** still run this cleanup — it iterates over actual `git worktree list` output and only merges back the worktrees that were created, leaving sequential plans' commits on the main tree untouched.
**If no worktrees found at runtime:** Skip silently — agents may have been spawned without worktree isolation, or the orchestrator already cleaned them up.

View File

@@ -0,0 +1,140 @@
// allow-test-rule: source-text-is-the-product
// The workflow .md file is the installed AI contract — its text IS what the orchestrator
// executes at runtime. Testing structural content of step 5.5 guards against accidental
// deletion of the cross-wave-deviation cleanup documentation (#3264).
/**
* Regression tests for #3264: cross-wave-dependency deviation cleanup documentation
*
* Guards that step 5.5 of execute-phase.md documents both skip conditions and
* contains a self-contained cleanup-tail snippet for the deviation path.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const WORKFLOW_PATH = path.join(
__dirname,
'..',
'get-shit-done',
'workflows',
'execute-phase.md',
);
/**
* Locate the step 5.5 block in the workflow file.
* Returns the substring from "5.5." up to (but not including) "5.6.".
* Throws if the block cannot be found.
*/
function extractStep55Block(content) {
const start = content.indexOf('\n5.5.');
assert.ok(start !== -1, 'execute-phase.md must contain a step 5.5 block');
const end = content.indexOf('\n5.6.', start + 1);
assert.ok(end !== -1, 'execute-phase.md must contain a step 5.6 block after 5.5');
return content.slice(start, end);
}
describe('execute-phase step 5.5: cross-wave-deviation cleanup documentation (#3264)', () => {
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
test('workflow file exists', () => {
assert.ok(fs.existsSync(WORKFLOW_PATH), 'workflows/execute-phase.md should exist');
});
test('step 5.5 block exists and is bounded', () => {
// extractStep55Block throws on failure — this test validates the helper itself
const block = extractStep55Block(content);
assert.ok(block.length > 0, 'step 5.5 block must be non-empty');
});
test('step 5.5 documents the standard wave contract', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('Standard wave contract'),
'step 5.5 must name the standard wave contract explicitly',
);
});
test('step 5.5 names cross-wave dependency deviation as a supported execution mode', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('Cross-wave dependency deviation'),
'step 5.5 must name the cross-wave dependency deviation as a supported mode',
);
});
test('cleanup-tail snippet contains git worktree prune', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('git worktree prune'),
'step 5.5 cleanup-tail snippet must include git worktree prune',
);
});
test('cleanup-tail snippet contains git worktree remove --force', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('git worktree remove') && block.includes('--force'),
'step 5.5 cleanup-tail snippet must include git worktree remove --force',
);
});
test('cleanup-tail snippet contains git worktree unlock', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('git worktree unlock'),
'step 5.5 cleanup-tail snippet must include git worktree unlock',
);
});
test('cleanup-tail snippet contains git branch -D', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('git branch -D'),
'step 5.5 cleanup-tail snippet must include git branch -D',
);
});
test('skip conditions enumerate empty-WAVE_WORKTREE_PLANS case', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('WAVE_WORKTREE_PLANS'),
'step 5.5 must document the empty-WAVE_WORKTREE_PLANS skip condition',
);
});
test('skip conditions enumerate custom-merge-deviation case', () => {
const block = extractStep55Block(content);
// The deviation skip condition must reference the cleanup-tail as the alternative
assert.ok(
block.includes('cleanup-tail'),
'step 5.5 must document the custom-merge-deviation skip condition with a pointer to the cleanup-tail',
);
});
test('cleanup-tail uses inclusion-based filter for agent namespace', () => {
const block = extractStep55Block(content);
// Must use .claude/worktrees/agent- inclusion filter, not exclusion (per #2774 precedent)
assert.ok(
block.includes('.claude/worktrees/agent-'),
'cleanup-tail must use inclusion-based filter matching .claude/worktrees/agent- namespace',
);
});
test('cleanup-tail reads git worktree list --porcelain line-by-line', () => {
const block = extractStep55Block(content);
assert.ok(
block.includes('git worktree list --porcelain'),
'cleanup-tail must parse git worktree list --porcelain output',
);
// Line-by-line reading requires IFS= read -r pattern
assert.ok(
block.includes('IFS= read -r'),
'cleanup-tail must read line-by-line to preserve paths with whitespace',
);
});
});