From f5276b36b36e4e7579e51542a568369dbe002968 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 20 Jun 2026 14:22:59 -0400 Subject: [PATCH] fix(#1369): refresh wave manifest and re-check base before each wave in execute-phase (#1492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#1369): refresh wave manifest and re-check base before each wave in execute-phase Two compounding issues caused wave N+1 worktrees to fork from the stale pre-wave-N commit, immediately tripping the worktree_branch_check FATAL guard in every executor: 1. worktree.base-check auto-degrade only ran once at initialize time. After wave N merges advanced orchestrator HEAD past origin/HEAD, new worktrees were still forked from origin/HEAD (Claude Code "fresh" base). 2. WAVE_WORKTREE_MANIFEST was never unset between waves, so wave N+1 reused the consumed wave-N manifest file, which would have blocked the step 5.5 manifest guard (#3384) on subsequent waves. Fix: add two safeguards in execute-phase.md — - Step 0.5 (start of each wave): re-runs worktree.base-check; auto-degrades USE_WORKTREES=false for that wave when HEAD has diverged from origin/HEAD. - Step 7c (end of each wave): unsets WAVE_WORKTREE_MANIFEST so wave N+1 creates a fresh per-wave manifest; re-asserts worktree.set-baseref (idempotent) and re-evaluates base degradation after wave merges land. 17 regression tests added in tests/bug-1369-wave-stale-base.test.cjs. Co-Authored-By: Claude Sonnet 4.6 * fix(#1369): rename test to fix-NNN convention; update workflow size baseline Rename tests/bug-1369-wave-stale-base.test.cjs → tests/fix-1369-wave-stale-base.test.cjs to satisfy the lint-regression-test-names gate (new files cannot use bug-NNN prefix). Update tests/workflow-size-baseline.json for execute-phase.md: 93157 → 97393 (LF-normalized byte count after adding step 0.5 inter-wave base re-check and step 7c between-wave manifest reset). Co-Authored-By: Claude Sonnet 4.6 * fix(#1369): add issue reference to allow-test-rule comment Co-Authored-By: Claude Sonnet 4.6 * fix(#1369): extract new execute-phase steps to references; satisfy ADR-857 cap Step 0.5 (inter-wave worktree base re-check) and steps 7b–7c (pre-wave dependency check + between-wave manifest reset/base refresh) added by this PR grew execute-phase.md to 97393 bytes, violating the ADR-857 phase-6 architectural mandate that host-loop bodies remain strictly below the pre-phase-6 baseline of 93166 bytes. Extract both new blocks into dedicated reference files: - gsd-core/references/execute-phase-wave-guard.md (step 0.5) - gsd-core/references/execute-phase-between-wave-reset.md (steps 7b + 7c) Replace inline prose with @-reference pointers. File now measures 92851 bytes (LF-normalized), satisfying the ADR-857 capstone conformance gate. Also update tests/workflow-size-baseline.json to 92851 and add both new reference files to docs/INVENTORY-MANIFEST.json. Co-Authored-By: Claude Sonnet 4.6 * fix(#1369): update regression tests to read from extracted reference files Steps 0.5 and 7b+7c were moved to reference files to satisfy the ADR-857 size cap on execute-phase.md. Tests now check @-reference pointers in the workflow for ordering and read content assertions from the reference files. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/1369-wave-stale-base-recheck.md | 5 + docs/INVENTORY-MANIFEST.json | 2 + .../execute-phase-between-wave-reset.md | 43 +++++ .../references/execute-phase-wave-guard.md | 33 ++++ gsd-core/workflows/execute-phase.md | 10 +- tests/fix-1369-wave-stale-base.test.cjs | 166 ++++++++++++++++++ tests/workflow-size-baseline.json | 2 +- 7 files changed, 254 insertions(+), 7 deletions(-) create mode 100644 .changeset/1369-wave-stale-base-recheck.md create mode 100644 gsd-core/references/execute-phase-between-wave-reset.md create mode 100644 gsd-core/references/execute-phase-wave-guard.md create mode 100644 tests/fix-1369-wave-stale-base.test.cjs diff --git a/.changeset/1369-wave-stale-base-recheck.md b/.changeset/1369-wave-stale-base-recheck.md new file mode 100644 index 000000000..53b5ef200 --- /dev/null +++ b/.changeset/1369-wave-stale-base-recheck.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1369 +--- +**`execute-phase` now re-checks the worktree fork base at the start of every wave and resets the wave manifest between waves (#1369)** — two compounding issues caused wave N+1 worktrees to be created from the stale pre-wave-N commit. First, the `worktree.base-check` auto-degrade only ran once at initialize time; after Wave N merged and tracking commits advanced orchestrator HEAD past `origin/HEAD`, Wave N+1 worktrees were still forked from `origin/HEAD` (Claude Code's "fresh" base), causing both agents to immediately halt with `FATAL: worktree base mismatch` from the `worktree_branch_check` guard. Second, `WAVE_WORKTREE_MANIFEST` was never unset between waves, so wave N+1 would reuse the consumed wave-N manifest file, causing the step 5.5 manifest guard (#3384) to block on subsequent waves. Two safeguards fix this: step 0.5 in the `execute_waves` "For each wave" loop re-runs `worktree.base-check` before every wave's dispatch (when divergence is detected, `USE_WORKTREES` is overridden to `false` for that wave); step 7c between waves unsets `WAVE_WORKTREE_MANIFEST` so wave N+1 creates a fresh per-wave manifest, and re-asserts `worktree.baseRef:"head"` (idempotent) so the Claude Code harness re-reads the live HEAD on the next dispatch. The permanent fix remains setting `worktree.baseRef:"head"` in `.claude/settings.local.json` (see #683). diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 7993ca302..a08c253ea 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -213,6 +213,8 @@ "domain-probes.md", "edge-probe.md", "execute-mvp-tdd.md", + "execute-phase-between-wave-reset.md", + "execute-phase-wave-guard.md", "executor-examples.md", "gate-prompts.md", "gates.md", diff --git a/gsd-core/references/execute-phase-between-wave-reset.md b/gsd-core/references/execute-phase-between-wave-reset.md new file mode 100644 index 000000000..a9fef5a81 --- /dev/null +++ b/gsd-core/references/execute-phase-between-wave-reset.md @@ -0,0 +1,43 @@ +7b. **Pre-wave dependency check (waves 2+ only):** + Before wave N+1, run `gsd-tools.cjs query verify.key-links {phase_dir}/{plan}-PLAN.md` for each upcoming plan. + If any PRIOR-wave artifact link fails, present: + - `## Cross-Plan Wiring Gap` with plan/link/from/pattern rows + - Options: investigate+fix before continue, or continue with cascade risk + Skip key-links that reference files in the CURRENT (upcoming) wave. + +7c. **Between-wave manifest reset and worktree base refresh (waves 2+ only — #1369):** + + **REQUIRED before each wave transition when `USE_WORKTREES != "false"` and `RUNTIME = "claude"`.** + + Wave N's `WAVE_WORKTREE_MANIFEST` was consumed by `worktree.cleanup-wave` in step 5.5. It must be + unset so wave N+1's step 3 creates a fresh manifest for the new wave's worktrees. Without this, + the wave N+1 manifest guard (step 5.5, #3384) blocks on the stale/empty consumed file. + + After wave N merges and tracking commits, the orchestrator HEAD has advanced past the commit the + Claude Code harness may have cached as the worktree fork base at session start. New worktrees + spawned for wave N+1 could fork from the stale pre-wave-N HEAD, causing every executor to trip the + `worktree_branch_check` FATAL guard immediately (symptom: `HEAD is , expected `). + + ```bash + # Unset per-wave manifest so wave N+1 creates a fresh one (#3384, #1369). + unset WAVE_WORKTREE_MANIFEST + + # Between-wave base refresh (#1369): after wave N merges and tracking commits, HEAD has + # advanced. Re-assert worktree.baseRef:"head" (idempotent — no-op if already set) so the + # Claude Code harness re-reads the live HEAD on the next Agent(isolation="worktree") call + # rather than using a cached session-start commit as the fork base. + if [ "$RUNTIME" = "claude" ] && [ "$USE_WORKTREES" != "false" ]; then + gsd_run query worktree.set-baseref 2>/dev/null || true + + # Safety re-check: evaluate degradation AFTER the wave N commits. If HEAD has diverged + # from origin/HEAD and baseRef is NOT "head", degrade remaining waves to sequential to + # avoid the base-mismatch FATAL in executor agents. + _BETWEEN_DEGRADE=$(gsd_run query worktree.base-check --pick shouldDegrade 2>/dev/null || echo "false") + if [ "$_BETWEEN_DEGRADE" = "true" ]; then + _DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) + [ -n "$_DEGRADE_MSG" ] && printf '%s\n' "$_DEGRADE_MSG" >&2 + printf 'Degrading to sequential mode for remaining waves: HEAD advanced past worktree fork base after wave %s merge (#1369).\n' "${N}" >&2 + USE_WORKTREES=false + fi + fi + ``` diff --git a/gsd-core/references/execute-phase-wave-guard.md b/gsd-core/references/execute-phase-wave-guard.md new file mode 100644 index 000000000..c28aa496a --- /dev/null +++ b/gsd-core/references/execute-phase-wave-guard.md @@ -0,0 +1,33 @@ +0.5. **Inter-wave worktree base re-check (wave N+1 guard — #1369):** + + After Wave N merges and tracking commits advance orchestrator HEAD, Claude Code's + `isolation="worktree"` still forks new worktrees from `origin/HEAD` (the "fresh" base), + not the live HEAD. This means Wave N+1 worktrees would be created from the stale + pre-Wave-N base, causing the `worktree_branch_check` guard inside each executor to halt + immediately with a base-mismatch fatal. + + **Run this check at the start of every wave when `USE_WORKTREES != "false"` and + `RUNTIME = "claude"`**, including Wave 1 (where it mirrors the initialize-step check): + + ```bash + if [ "$RUNTIME" = "claude" ] && [ "${USE_WORKTREES:-true}" != "false" ]; then + _WAVE_DEGRADE=$(gsd_run query worktree.base-check --pick shouldDegrade 2>/dev/null || true) + if [ "$_WAVE_DEGRADE" = "true" ]; then + _WAVE_DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) + [ -n "$_WAVE_DEGRADE_MSG" ] && printf '%s\n' "$_WAVE_DEGRADE_MSG" >&2 + echo "⚠ [#1369] Worktree fork base diverged from orchestrator HEAD (wave merges advanced HEAD past origin/HEAD). Auto-degrading to sequential mode for this wave to avoid base-mismatch halts." >&2 + USE_WORKTREES=false + fi + fi + ``` + + If `shouldDegrade` is `true`, override `USE_WORKTREES=false` for **this wave only** — + all plans in this wave execute sequentially on the main working tree. Later waves re-run + this check and may re-enable worktree isolation if `origin/HEAD` is updated (e.g. via + `git fetch` or `worktree.baseRef:"head"` config). + + **To avoid this degrade across all waves:** set `worktree.baseRef:"head"` in + `.claude/settings.local.json` (or run `gsd-tools worktree set-baseref`). This tells + Claude Code to fork from the live HEAD instead of `origin/HEAD`, so each wave's new + worktrees always start from the correct post-merge base. See #683 for the base-ref + configuration detail. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 1685336d4..ee99bb170 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -491,6 +491,8 @@ increases monotonically across waves. `{status}` is `complete` (success), **For each wave:** +@~/.claude/gsd-core/references/execute-phase-wave-guard.md + 1. **Intra-wave files_modified overlap check (BEFORE spawning):** Before spawning any agents for this wave, inspect the `files_modified` list of all plans @@ -1038,12 +1040,8 @@ increases monotonically across waves. `{status}` is `complete` (success), **Step 7.3 — `class == "unknown-failure"`:** Report failed plan and ask Continue/Stop; continuing may cascade into dependent plan failures. -7b. **Pre-wave dependency check (waves 2+ only):** - Before wave N+1, run `gsd-tools.cjs query verify.key-links {phase_dir}/{plan}-PLAN.md` for each upcoming plan. - If any PRIOR-wave artifact link fails, present: - - `## Cross-Plan Wiring Gap` with plan/link/from/pattern rows - - Options: investigate+fix before continue, or continue with cascade risk - Skip key-links that reference files in the CURRENT (upcoming) wave. +@~/.claude/gsd-core/references/execute-phase-between-wave-reset.md + 8. **Execute checkpoint plans between waves** — see ``. 9. **Proceed to next wave.** diff --git a/tests/fix-1369-wave-stale-base.test.cjs b/tests/fix-1369-wave-stale-base.test.cjs new file mode 100644 index 000000000..0d775f3ea --- /dev/null +++ b/tests/fix-1369-wave-stale-base.test.cjs @@ -0,0 +1,166 @@ +// allow-test-rule: source-text-is-the-product #1369 +// Workflow .md files are the installed AI instructions — their text IS what the runtime +// loads. Testing text content tests the deployed contract. Per CONTRIBUTING.md exception matrix. + +/** + * Regression tests for bug #1369: execute-phase worktree agents fork from stale base after + * a wave merge advances orchestrator HEAD past origin/HEAD. + * + * Steps 0.5 and 7b+7c are extracted to reference files to satisfy the ADR-857 size cap. + * execute-phase.md contains @-reference pointers; the reference files hold the content. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); +const WAVE_GUARD_PATH = path.join(__dirname, '..', 'gsd-core', 'references', 'execute-phase-wave-guard.md'); +const BETWEEN_WAVE_PATH = path.join(__dirname, '..', 'gsd-core', 'references', 'execute-phase-between-wave-reset.md'); + +describe('execute-phase: inter-wave worktree base re-check (#1369)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(WORKFLOW_PATH), 'workflows/execute-phase.md should exist'); + }); + + test('wave-guard reference file exists', () => { + assert.ok(fs.existsSync(WAVE_GUARD_PATH), 'references/execute-phase-wave-guard.md should exist'); + }); + + test('workflow contains @-reference pointer to wave-guard (step 0.5 injected at runtime)', () => { + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + assert.ok( + content.includes('execute-phase-wave-guard.md'), + 'execute-phase.md must have an @-reference to execute-phase-wave-guard.md' + ); + }); + + test('workflow contains step 0.5 inter-wave base re-check section', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok( + content.includes('0.5.') && content.includes('Inter-wave worktree base re-check'), + 'execute-phase-wave-guard.md must have step 0.5 "Inter-wave worktree base re-check"' + ); + }); + + test('step 0.5 references #1369', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok(content.includes('#1369'), 'step 0.5 must reference #1369 for traceability'); + }); + + test('step 0.5 runs worktree.base-check inside the For-each-wave loop', () => { + const workflow = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + const forEachIdx = workflow.indexOf('**For each wave:**'); + const refIdx = workflow.indexOf('execute-phase-wave-guard.md'); + assert.ok(forEachIdx !== -1, '"For each wave:" section must exist in execute-phase.md'); + assert.ok(refIdx !== -1, '@-reference to wave-guard must exist in execute-phase.md'); + assert.ok(refIdx > forEachIdx, 'wave-guard @-reference must appear AFTER "For each wave:" so step 0.5 runs per-wave'); + }); + + test('step 0.5 runs worktree.base-check command', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok(content.includes('worktree.base-check'), 'step 0.5 must invoke worktree.base-check'); + }); + + test('step 0.5 sets USE_WORKTREES=false when shouldDegrade is true', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok(content.includes('USE_WORKTREES=false'), 'step 0.5 must override USE_WORKTREES=false when base divergence is detected'); + }); + + test('step 0.5 appears before step 1 (intra-wave overlap check)', () => { + const workflow = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + const forEachIdx = workflow.indexOf('**For each wave:**'); + const refIdx = workflow.indexOf('execute-phase-wave-guard.md'); + const step1Idx = workflow.indexOf('1. **Intra-wave', forEachIdx); + assert.ok(refIdx !== -1, 'wave-guard @-reference must exist'); + assert.ok(step1Idx !== -1, 'step 1 (intra-wave overlap check) must exist'); + assert.ok(refIdx < step1Idx, 'wave-guard @-reference must appear before step 1'); + }); + + test('step 0.5 guards on RUNTIME=claude (worktree isolation is Claude Code-specific)', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok( + content.includes('RUNTIME') && (content.includes('"claude"') || content.includes("'claude'")), + 'step 0.5 must guard on RUNTIME=claude' + ); + }); + + test('step 0.5 explains root cause: wave merges advance HEAD past origin/HEAD', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok(content.includes('origin/HEAD'), 'step 0.5 must name origin/HEAD as the stale fork base'); + }); + + test('step 0.5 cross-references #683 for worktree.baseRef configuration', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok(content.includes('#683'), 'step 0.5 must cross-reference #683'); + }); + + test('step 0.5 mentions worktree.baseRef:"head" as permanent fix', () => { + const content = fs.readFileSync(WAVE_GUARD_PATH, 'utf-8'); + assert.ok( + content.includes('worktree.baseRef') && content.includes('head'), + 'step 0.5 must mention worktree.baseRef:"head"' + ); + }); +}); + +describe('execute-phase: between-wave manifest reset (#1369, #3384)', () => { + test('between-wave reference file exists', () => { + assert.ok(fs.existsSync(BETWEEN_WAVE_PATH), 'references/execute-phase-between-wave-reset.md should exist'); + }); + + test('workflow contains @-reference pointer to between-wave-reset', () => { + const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + assert.ok( + content.includes('execute-phase-between-wave-reset.md'), + 'execute-phase.md must have an @-reference to execute-phase-between-wave-reset.md' + ); + }); + + test('step 7c exists with between-wave manifest reset (#1369)', () => { + const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + assert.ok( + content.includes('7c.') && content.includes('Between-wave manifest reset'), + 'execute-phase-between-wave-reset.md must have step 7c "Between-wave manifest reset"' + ); + }); + + test('step 7c unsets WAVE_WORKTREE_MANIFEST between waves', () => { + const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + assert.ok(content.includes('unset WAVE_WORKTREE_MANIFEST'), 'step 7c must unset WAVE_WORKTREE_MANIFEST'); + }); + + test('step 7c references #1369 and #3384 for traceability', () => { + const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + assert.ok(content.includes('#1369'), 'step 7c must reference #1369'); + assert.ok(content.includes('#3384'), 'step 7c must reference #3384'); + }); + + test('step 7c calls worktree.set-baseref to re-assert head config', () => { + const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + assert.ok(content.includes('worktree.set-baseref'), 'step 7c must call worktree.set-baseref'); + }); + + test('step 7c appears after step 7b and before step 8 in the wave loop', () => { + const ref = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + const workflow = fs.readFileSync(WORKFLOW_PATH, 'utf-8'); + const idx7b = ref.indexOf('7b.'); + const idx7c = ref.indexOf('7c.'); + const refPtr = workflow.indexOf('execute-phase-between-wave-reset.md'); + const idx8 = workflow.indexOf('8. **Execute checkpoint', refPtr); + assert.ok(idx7b !== -1, 'step 7b must exist in between-wave reference file'); + assert.ok(idx7c !== -1, 'step 7c must exist in between-wave reference file'); + assert.ok(idx8 !== -1, 'step 8 must exist in execute-phase.md after the between-wave @-reference'); + assert.ok(idx7b < idx7c, 'step 7c must appear after step 7b'); + assert.ok(refPtr < idx8, 'between-wave @-reference must appear before step 8'); + }); + + test('step 7c guards on RUNTIME=claude for worktree-specific operations', () => { + const content = fs.readFileSync(BETWEEN_WAVE_PATH, 'utf-8'); + assert.ok( + content.includes('RUNTIME') && (content.includes('"claude"') || content.includes("'claude'")), + 'step 7c must guard on RUNTIME=claude' + ); + }); +}); diff --git a/tests/workflow-size-baseline.json b/tests/workflow-size-baseline.json index 9429ef735..59a6de090 100644 --- a/tests/workflow-size-baseline.json +++ b/tests/workflow-size-baseline.json @@ -24,7 +24,7 @@ "docs-update.md": 55662, "edit-phase.md": 12883, "eval-review.md": 9923, - "execute-phase.md": 93157, + "execute-phase.md": 92851, "execute-plan.md": 31365, "explore.md": 10497, "extract-learnings.md": 12849,