* 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 <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). * changeset(#2649): diagnose-issues + execute-plan auto-degrade on stale worktree base * changeset(#2649): backfill PR number 2955 --------- Co-authored-by: sim <sim@users.noreply.github.com>
This commit is contained in:
Tom Boucher
5
.changeset/mellow-ravens-fly.md
Normal file
5
.changeset/mellow-ravens-fly.md
Normal file
@@ -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)
|
||||
@@ -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 `<worktree_branch_check>` 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:
|
||||
|
||||
@@ -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 <role> --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_<ROLE>}` (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 `<worktree_branch_check>` 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 `<worktree_branch_check>` 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.
|
||||
|
||||
|
||||
@@ -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."
|
||||
}
|
||||
}
|
||||
120
tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs
Normal file
120
tests/fix-2649-diagnose-issues-worktree-stale-base.test.cjs
Normal file
@@ -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('<step name="spawn_agents">');
|
||||
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
|
||||
// <worktree_branch_check> 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('<step name="spawn_agents">');
|
||||
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 <worktree_branch_check> 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',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user