From f092c6da8593153851f3281e8145e2711ae35272 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 31 Jul 2026 19:15:04 -0400 Subject: [PATCH] fix(#2649): diagnose-issues + execute-plan run worktree.base-check before worktree dispatch (#2955) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2649): failing-first — diagnose-issues + execute-plan must run base-check before worktree dispatch * fix(#2649): diagnose-issues + execute-plan run worktree.base-check before dispatch diagnose-issues.md spawn_agents and execute-plan.md Pattern A spawned worktree-isolated subagents (gsd-debugger / gsd-executor) without the pre-dispatch worktree.base-check gate that execute-phase (#683/#1369) and quick (#1941) already run. Claude Code's isolation="worktree" forks from origin/HEAD, not live local HEAD; without the gate, the documented GSD steady state (commit every step locally, push only on request) hits the verify-only worktree_branch_check guard's exit-42 halt mid-investigation with no auto-degrade. Mirror the quick.md #1941 pattern: before dispatch, run `gsd_run query worktree.base-check --pick shouldDegrade`; if true, print its message + a #2649 warning to stderr and set USE_WORKTREES=false (sequential main-tree dispatch). The verify-only guard stays as a backstop in both cases. Per the triage and #2649 acceptance criterion 5, execute-plan.md's Pattern A (identified as a second site with the identical gap) is fixed in the SAME change — same bug class, same one-line gate, two workflow files — rather than filed as a separate follow-up. * fix(#2649): ack the diagnose-issues + execute-plan growth (per-PR fragment) The two workflow files grew vs next (diagnose-issues.md +1381, execute-plan.md +905) adding the #2649 base-check gate. emitted-attribution requires an ack; this is a per-PR fragment under tests/emitted-drift-acks/ (#2914 mechanism, replacing the legacy shared emitted-drift-ack.json). * test(#2649): tighten base-check ordering assertion + guard backstop survival Address code-review minors: - the ordering assertion was a loose disjunction that passed even if the base-check moved AFTER the dispatch; tighten to assert base-check < Agent() (the real invariant). - add a test that the verify-only backstop remains embedded in the Agent() prompt (acceptance criterion 4 — the base-check is a pre-dispatch degrade, the guard is a post-fork fail-closed backstop; both layers must survive). * changeset(#2649): diagnose-issues + execute-plan auto-degrade on stale worktree base * changeset(#2649): backfill PR number 2955 --------- Co-authored-by: sim --- .changeset/mellow-ravens-fly.md | 5 + gsd-core/workflows/diagnose-issues.md | 22 ++++ gsd-core/workflows/execute-plan.md | 2 +- ...2649-diagnose-execute-plan-base-check.json | 7 + ...agnose-issues-worktree-stale-base.test.cjs | 120 ++++++++++++++++++ 5 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 .changeset/mellow-ravens-fly.md create mode 100644 tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json create mode 100644 tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs diff --git a/.changeset/mellow-ravens-fly.md b/.changeset/mellow-ravens-fly.md new file mode 100644 index 000000000..14ccf584d --- /dev/null +++ b/.changeset/mellow-ravens-fly.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2955 +--- +**/gsd-verify-work diagnosis and interactive plan execution no longer halt on a stale worktree fork base** — when worktrees are enabled and local HEAD has advanced past `origin/HEAD` (the GSD steady state of committing every step and pushing only on request), the spawned debug/executor agent used to fork from the stale ref and hit a base-mismatch fatal mid-investigation with no recovery. Both dispatch sites now run the same pre-dispatch `worktree.base-check` gate the executor and quick-task paths already run, auto-degrading to sequential main-tree dispatch with an explanatory message. (#2649) diff --git a/gsd-core/workflows/diagnose-issues.md b/gsd-core/workflows/diagnose-issues.md index ea95af440..da494e5b9 100644 --- a/gsd-core/workflows/diagnose-issues.md +++ b/gsd-core/workflows/diagnose-issues.md @@ -97,6 +97,28 @@ AGENT_SKILLS_DEBUGGER=$(gsd_run query agent-skills gsd-debugger) EXPECTED_BASE=$(git rev-parse HEAD) ``` +**Pre-dispatch worktree base-check (#2649, mirrors execute-phase #683/#1369 and quick #1941).** +Claude Code's `isolation="worktree"` forks new worktrees from `origin/HEAD`, not the live local +HEAD. If local HEAD has advanced without an intervening `git push` (the documented GSD steady +state — commit every step, push only on request), `origin/HEAD` is pinned to a stale ancestor +and the debug agent's `worktree_branch_check` guard halts with a base-mismatch fatal *after* the +worktree already exists, with no automatic degrade. Run the same pre-dispatch check the four +sibling dispatch sites run, and auto-degrade to sequential (main-tree) debug-agent dispatch when +the fork base cannot be reliably resolved. The verify-only `` guard below +stays active as a backstop in both cases. + +```bash +if [ "${USE_WORKTREES:-true}" != "false" ]; then + _DIAG_SHOULD_DEGRADE=$(gsd_run query worktree.base-check --pick shouldDegrade 2>/dev/null || true) + if [ "$_DIAG_SHOULD_DEGRADE" = "true" ]; then + _DIAG_DEGRADE_MSG=$(gsd_run query worktree.base-check --pick message 2>/dev/null || true) + [ -n "$_DIAG_DEGRADE_MSG" ] && printf '%s\n' "$_DIAG_DEGRADE_MSG" >&2 + echo "⚠ [#2649] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for diagnosis to avoid a base-mismatch halt." >&2 + USE_WORKTREES=false + fi +fi +``` + **Spawn debug agents in parallel:** For each gap, fill the debug-subagent-prompt template and spawn: diff --git a/gsd-core/workflows/execute-plan.md b/gsd-core/workflows/execute-plan.md index d20d86b3c..26d297ccd 100644 --- a/gsd-core/workflows/execute-plan.md +++ b/gsd-core/workflows/execute-plan.md @@ -113,7 +113,7 @@ Otherwise: Apply checkpoint-based routing below. > **Runtime-aware dispatch (#2508 Phase 4).** GSD workflows dispatch specialized subagents by role. Before dispatching on a built-in-only runtime (kimi-code — three built-ins only), resolve the role to a built-in via `gsd_run query resolve-dispatch-type --requested --raw`. On named-dispatch runtimes (Claude/OpenCode/…) the role is returned unchanged; on kimi-code it maps to `coder`/`explore`/`plan` by role-suffix. The persona rides `${AGENT_SKILLS_}` (Phase 3) regardless. See @gsd-core/references/runtime-aware-dispatch.md. -**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`). **When using `isolation="worktree"`, embed the `` block from `gsd-core/references/worktree-branch-check.md` into the prompt, substituting `{EXPECTED_BASE}` with the captured base SHA.** That guard is **verify-only and fail-closed** (#48): it asserts a per-agent `agent-*` / `worktree-agent-*` branch and the exact base, forbids `git update-ref` self-recovery (#2924), and on any mismatch prints `FATAL:` and `exit 42` so the orchestrator can recover — the sub-agent never rewrites a worktree it did not create. This supersedes the former self-recovery (#2015), whose destructive base rewrite could fail silently under a deny rule; the base-drift it addressed affects all platforms, and base correction is now the orchestrator's responsibility. +**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → **before spawning, run the #2649 pre-dispatch worktree base-check** (mirrors execute-phase #683/#1369 and quick #1941): if `workflow.use_worktrees` is not `false`, run `gsd_run query worktree.base-check --pick shouldDegrade`; if it returns `true`, print its `--pick message` to stderr, emit the `⚠ [#2649] Worktree fork base diverged from orchestrator HEAD — auto-degrading to sequential mode for this plan to avoid a base-mismatch halt.` warning, and treat worktrees as disabled for this dispatch (spawn WITHOUT `isolation="worktree"`). Claude Code's `isolation="worktree"` forks from `origin/HEAD`, not live local HEAD; without this gate a plan whose commit advanced local HEAD past a stale `origin/HEAD` hits the verify-only guard's `exit 42` mid-execution with no auto-degrade. → print `Spawning executor agent (runs in a subagent — no output until it returns, ~1–5 min; expected, not a freeze)` → spawn Agent(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`) **and the #2649 base-check did not degrade**. **When using `isolation="worktree"`, embed the `` block from `gsd-core/references/worktree-branch-check.md` into the prompt, substituting `{EXPECTED_BASE}` with the captured base SHA.** That guard is **verify-only and fail-closed** (#48) and stays active as a backstop whether or not the base-check degraded: it asserts a per-agent `agent-*` / `worktree-agent-*` branch and the exact base, forbids `git update-ref` self-recovery (#2924), and on any mismatch prints `FATAL:` and `exit 42` so the orchestrator can recover — the sub-agent never rewrites a worktree it did not create. This supersedes the former self-recovery (#2015), whose destructive base rewrite could fail silently under a deny rule; the base-drift it addressed affects all platforms, and base correction is now the orchestrator's responsibility. **Pattern B:** Execute segment-by-segment. Autonomous segments: spawn subagent for assigned tasks only (no SUMMARY/commit). Checkpoints: main context. After all segments: aggregate, create SUMMARY, commit. See segment_execution. diff --git a/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json b/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json new file mode 100644 index 000000000..8e0ec0af6 --- /dev/null +++ b/tests/emitted-drift-acks/2649-diagnose-execute-plan-base-check.json @@ -0,0 +1,7 @@ +{ + "version": 1, + "paths": { + "diagnose-issues.md": "#2649: spawn_agents step gained a pre-dispatch worktree.base-check gate (mirrors execute-phase #683/#1369 and quick #1941). Claude Code's isolation=\"worktree\" forks from origin/HEAD, not live local HEAD; without the gate the documented GSD steady state (commit every step locally, push only on request) hit the verify-only worktree_branch_check guard's exit-42 halt mid-investigation. Growth is the base-check bash block (gsd_run query worktree.base-check --pick shouldDegrade → USE_WORKTREES=false + stderr warning) + the #2649 rationale comment. The verify-only guard stays as a backstop.", + "execute-plan.md": "#2649: Pattern A (single-plan interactive dispatch) gained the same pre-dispatch worktree.base-check gate the wave path (executor-isolation-dispatch.md) and quick.md already run — the triage found Pattern A had the identical missing-gate gap. Growth is the base-check instruction in the Pattern A description (consult shouldDegrade, auto-degrade to sequential on true, keep the verify-only guard as backstop) + the #2649 rationale." + } +} diff --git a/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs b/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs new file mode 100644 index 000000000..b7ef01f3e --- /dev/null +++ b/tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs @@ -0,0 +1,120 @@ +// allow-test-rule: source-text-is-the-product #2649 +// 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. Mirrors tests/fix-1941-quick-worktree-stale-base.test.cjs. + +/** + * Regression tests for bug #2649: /gsd-verify-work's UAT-gap diagnosis step + * (workflows/diagnose-issues.md) and execute-phase's single-plan interactive + * dispatch (workflows/execute-plan.md Pattern A) spawn worktree-isolated + * subagents without first checking whether the harness's worktree fork base has + * diverged from live local HEAD — unlike every other worktree-dispatch site. + * + * Root cause: Claude Code's isolation="worktree" forks new worktrees from + * origin/HEAD, not the live local HEAD. When local commits advance HEAD without + * an intervening `git push` (the documented GSD steady state), origin/HEAD is + * pinned to a stale ancestor and the subagent's worktree_branch_check guard + * halts with a base-mismatch fatal mid-investigation, with no auto-degrade. The + * fix ports the worktree.base-check auto-degrade pattern (execute-phase #683/ + * #1369, quick #1941) into these two not-yet-covered dispatch sites. + * + * The triage for #2649 found execute-plan.md's Pattern A has the identical gap; + * per the bug's acceptance criterion 5 it is fixed in the same change (same bug + * class, same one-line gate) rather than filed as a separate follow-up. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const DIAGNOSE_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'diagnose-issues.md'); +const EXECUTE_PLAN_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-plan.md'); + +describe('diagnose-issues: pre-dispatch worktree base-check (#2649)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(DIAGNOSE_PATH), 'workflows/diagnose-issues.md should exist'); + }); + + test('spawn_agents step runs worktree.base-check before the Agent() dispatch', () => { + const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); + const spawnIdx = content.indexOf(''); + assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist in diagnose-issues.md'); + const baseCheckIdx = content.indexOf('worktree.base-check', spawnIdx); + assert.ok(baseCheckIdx !== -1, 'worktree.base-check must be invoked within the spawn_agents step'); + // The load-bearing invariant is "base-check BEFORE the Agent() dispatch" so the + // degrade decision can drop isolation from the spawn. (Where EXPECTED_BASE is + // captured relative to the check is cosmetic — the check only reads HEAD, never + // mutates it — so assert the real invariant, not a loose disjunction.) + const agentIdx = content.indexOf('Agent(', spawnIdx); + assert.ok(agentIdx !== -1, 'spawn_agents must contain an Agent() dispatch'); + assert.ok( + baseCheckIdx < agentIdx, + 'worktree.base-check must run before the Agent() dispatch so the degrade decision can drop isolation from the spawn', + ); + }); + + test('verify-only worktree_branch_check backstop remains embedded in the Agent() prompt', () => { + // Acceptance criterion #4: the base-check is a PRE-DISPATCH degrade; the + // guard is a POST-FORK fail-closed backstop. Both + // layers must survive — a future edit that dropped the backstop embedding + // would re-open the silent-stale-base class. Guard its continued presence. + const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); + const spawnIdx = content.indexOf(''); + assert.ok(spawnIdx !== -1, '"spawn_agents" step must exist'); + assert.ok( + content.indexOf('worktree-branch-check.md', spawnIdx) !== -1, + 'spawn_agents must still materialize the backstop after the base-check gate (#2649 acceptance criterion 4)', + ); + }); + + test('degrade check sets USE_WORKTREES=false when shouldDegrade is true', () => { + const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); + const baseCheckIdx = content.indexOf('worktree.base-check'); + const block = content.slice(baseCheckIdx, baseCheckIdx + 600); + assert.ok( + block.includes('shouldDegrade') && block.includes('USE_WORKTREES=false'), + 'degrade check must override USE_WORKTREES=false when shouldDegrade is true', + ); + }); + + test('degrade check references #2649 for traceability', () => { + const content = fs.readFileSync(DIAGNOSE_PATH, 'utf-8'); + assert.ok(content.includes('#2649'), 'diagnose-issues.md must reference #2649'); + }); +}); + +describe('execute-plan Pattern A: pre-dispatch worktree base-check (#2649)', () => { + test('workflow file exists', () => { + assert.ok(fs.existsSync(EXECUTE_PLAN_PATH), 'workflows/execute-plan.md should exist'); + }); + + test('Pattern A runs the worktree base-check before spawning the executor', () => { + const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); + const patternAIdx = content.indexOf('**Pattern A:**'); + assert.ok(patternAIdx !== -1, '"Pattern A:" must exist in execute-plan.md'); + // The base-check instruction must appear within the Pattern A description, + // before the isolation="worktree" embedding instruction. + const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); + const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); + assert.ok( + patternA.includes('#2649') && /worktree\.base-check|base-check/.test(patternA), + 'Pattern A must run the #2649 worktree base-check before dispatching the executor', + ); + assert.ok( + patternA.includes('shouldDegrade'), + 'Pattern A base-check must consult shouldDegrade', + ); + }); + + test('Pattern A documents the auto-degrade (drop isolation on shouldDegrade)', () => { + const content = fs.readFileSync(EXECUTE_PLAN_PATH, 'utf-8'); + const patternAIdx = content.indexOf('**Pattern A:**'); + const patternAEnd = content.indexOf('**Pattern B:**', patternAIdx); + const patternA = content.slice(patternAIdx, patternAEnd === -1 ? undefined : patternAEnd); + assert.ok( + /degrad|sequential/i.test(patternA), + 'Pattern A must document auto-degrading to sequential mode when shouldDegrade is true', + ); + }); +});