From b77b7f8e56863321202eb728cd3360861ad2d521 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 13 Aug 2026 20:31:55 -0400 Subject: [PATCH] fix(#1526): delegate auto-chain post-completion to transition workflow (#3419) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#1526): delegate auto-chain post-completion to transition workflow execute-phase's auto-chain completion called phase.complete then a light inline set (partial PROJECT.md update + offer-next) and never invoked the transition workflow, silently skipping graduation scan, session-continuity, project-reference, accumulated-context, and current-position updates — so a phase completed via auto-chain left different project state than a normal transition. Fix (delegate, user decision 2026-08-13): replace execute-phase's update_project_md + offer_next with a delegation step that @-includes transition.md in post-completion mode. Add a post_completion_mode step to transition.md that skips verify_completion + update_roadmap_and_state (phase.complete already ran; avoids double-write) and begins at evolve_project. Standalone transition (mode 1) is unchanged. Regression: tests/auto-chain-transition-delegation.test.cjs (source-text-is-the- product) asserts the delegation, the skip-set, the removed inline step, and the mode. Ack fragment 1526 covers execute-phase.md + transition.md growth (spent 2930 fragment removed — same-path owner conflict, like #3025/#3024). * docs(#1526): backfill changeset PR number (#3419) --------- Co-authored-by: sim --- .changeset/lucky-ravens-leap.md | 5 ++ gsd-core/workflows/execute-phase.md | 31 ++----- gsd-core/workflows/transition.md | 25 ++++++ .../lint-allow-test-rule-refs.ceiling.json | 2 +- .../auto-chain-transition-delegation.test.cjs | 80 +++++++++++++++++++ ...1526-auto-chain-transition-delegation.json | 8 ++ ...930-fragmentize-execute-phase-markers.json | 6 -- tests/execute-phase-active-flags.test.cjs | 60 +++++++------- tests/new-milestone-clear-phases.test.cjs | 2 +- 9 files changed, 159 insertions(+), 60 deletions(-) create mode 100644 .changeset/lucky-ravens-leap.md create mode 100644 tests/auto-chain-transition-delegation.test.cjs create mode 100644 tests/emitted-drift-acks/1526-auto-chain-transition-delegation.json delete mode 100644 tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json diff --git a/.changeset/lucky-ravens-leap.md b/.changeset/lucky-ravens-leap.md new file mode 100644 index 000000000..b3075e8d5 --- /dev/null +++ b/.changeset/lucky-ravens-leap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3419 +--- +**Auto-chain phase completion now runs the same post-processing as a normal transition** (#1526) — completing a phase via `/gsd:execute-phase` (auto-chain) previously skipped the transition workflow's graduation scan, session-continuity, project-reference, accumulated-context, and current-position updates, leaving project state different from a normal transition. execute-phase now delegates post-completion processing to the transition workflow (post-completion mode: skips re-verify + re-running `phase.complete` to avoid a double-write). Identity/standalone transition behavior is unchanged. diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 1413b6247..81981b0b0 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -1519,30 +1519,15 @@ fi **No matches:** skip silently (always additive, non-blocking). - -**Evolve PROJECT.md to reflect phase completion (prevents planning document drift — #956):** + +**#1526 — Delegate post-completion to the transition workflow** (parity: the auto-chain +path must run the SAME post-processing as a normal transition). `phase.complete` +(`update_roadmap` above) and verification (`verify_phase_goal`) already ran, so invoke +transition in **post-completion mode**: SKIP its `verify_completion` and +`update_roadmap_and_state` (re-running `phase.complete` would double-write state) and +BEGIN at `evolve_project`, running the full set through `offer_next_phase`. -PROJECT.md tracks validated requirements, decisions, and current state. Without this step, -PROJECT.md falls behind silently over multiple phases. - -1. Read `.planning/PROJECT.md` -2. If the file exists and has a `## Validated Requirements` or `## Requirements` section: - - Move any requirements validated by this phase from Active → Validated - - Add a brief note: `Validated in Phase {X}: {Name}` -3. If the file has a `## Current State` or similar section: - - Update it to reflect this phase's completion (e.g., "Phase {X} complete — {one-liner}") -4. Update the `Last updated:` footer to today's date -5. Commit the change: - -```bash -gsd_run query commit "docs(phase-{X}): evolve PROJECT.md after phase completion" --files .planning/PROJECT.md -``` - -**Skip this step if** `.planning/PROJECT.md` does not exist. - - - -@~/.claude/gsd-core/references/offer-next.md +@~/.claude/gsd-core/workflows/transition.md diff --git a/gsd-core/workflows/transition.md b/gsd-core/workflows/transition.md index 57d008ca1..a520fb8e5 100644 --- a/gsd-core/workflows/transition.md +++ b/gsd-core/workflows/transition.md @@ -36,6 +36,31 @@ Mark current phase complete and advance to next. This is the natural point where + + +**Invocation mode — read this FIRST.** This workflow runs two ways: + +1. **Standalone transition** (normal path): the phase is being marked complete AND + transitioned by this workflow. Run EVERY step below in order — `verify_completion`, + `update_roadmap_and_state` (which calls `gsd_run query phase.complete`), then the + post-processing. + +2. **Post-completion delegation** (invoked by `execute-phase` after its auto-chain + completion — #1526): `phase.complete` was already called by execute-phase's + `update_roadmap` step and verification already passed in execute-phase's + `verify_phase_goal`. SKIP `verify_completion` and `update_roadmap_and_state` + (re-running `phase.complete` would double-write STATE.md/ROADMAP.md). Run + `cleanup_handoff` (stale `.continue-here` handoffs are still cleared post-completion), + then BEGIN at `evolve_project` and run every step from there through + `offer_next_phase` (this is the post-processing parity set: graduation scan, + session-continuity, project-reference, accumulated-context, current-position/progress). + `archive_prompts` is a documented no-op in either mode. + +Detect post-completion mode when the caller states that phase completion and +verification have already run. When in doubt, run standalone (mode 1) — it is +idempotent enough to be safe, just slower. + + Before transition, read project state: diff --git a/scripts/lint-allow-test-rule-refs.ceiling.json b/scripts/lint-allow-test-rule-refs.ceiling.json index 9e97dd204..28a04739f 100644 --- a/scripts/lint-allow-test-rule-refs.ceiling.json +++ b/scripts/lint-allow-test-rule-refs.ceiling.json @@ -1,4 +1,4 @@ { - "maxFiles": 298, + "maxFiles": 299, "grace": 3 } diff --git a/tests/auto-chain-transition-delegation.test.cjs b/tests/auto-chain-transition-delegation.test.cjs new file mode 100644 index 000000000..98c64c43d --- /dev/null +++ b/tests/auto-chain-transition-delegation.test.cjs @@ -0,0 +1,80 @@ +// allow-test-rule: source-text-is-the-product (see #1526) +// execute-phase.md and transition.md are shipped workflows whose deployed text IS what the +// runtime loads — asserting their cross-workflow delegation tests the deployed contract. + +/** + * #1526 — auto-chain completion must delegate post-processing to the transition workflow. + * + * Previously execute-phase's completion called `phase.complete` then a LIGHT inline set + * (a partial PROJECT.md update + offer-next) and never invoked the transition workflow, + * silently skipping graduation scan, session-continuity, project-reference, accumulated- + * context, and the current-position/progress update. The normal transition path ran all of + * those, so the two paths left different project state behind. + * + * Chosen fix (user decision, 2026-08-13): delegate — execute-phase invokes transition.md in + * a post-completion mode that skips `verify_completion` + `update_roadmap_and_state` + * (phase.complete already ran; avoids double-write) and begins at `evolve_project`. + */ + +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const EXEC = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); +const TRANS = path.join(__dirname, '..', 'gsd-core', 'workflows', 'transition.md'); + +describe('#1526: auto-chain completion delegates post-processing to transition', () => { + const exec = fs.readFileSync(EXEC, 'utf8'); + const trans = fs.readFileSync(TRANS, 'utf8'); + + test('execute-phase delegates to transition.md after phase.complete (post-completion)', () => { + // The delegation step must include the transition workflow and name post-completion mode. + const delegateIdx = exec.indexOf('delegate_post_completion_to_transition'); + const includeIdx = exec.indexOf('@~/.claude/gsd-core/workflows/transition.md'); + assert.notEqual(delegateIdx, -1, 'execute-phase must have a delegation step'); + assert.notEqual(includeIdx, -1, 'execute-phase must @-include transition.md'); + assert.ok(includeIdx > delegateIdx, 'the transition include must be inside the delegation step'); + }); + + test('the delegation skips re-running phase.complete (no double-write) and re-verification', () => { + // Slice the delegation step and assert it names the skip set. + const start = exec.indexOf(' { + const modeIdx = trans.indexOf('', modeIdx); + const mode = trans.slice(modeIdx, modeEnd); + assert.match(mode, /SKIP\s+`verify_completion`\s+and\s+`update_roadmap_and_state`/i, 'mode must name the exact skip set'); + assert.match(mode, /evolve_project/, 'mode must begin at evolve_project'); + }); + + test('transition.md still documents standalone mode (full verify → complete → post-process)', () => { + // Negative space: the normal transition path must remain a full run. + const modeIdx = trans.indexOf('', modeIdx); + const mode = trans.slice(modeIdx, modeEnd); + assert.match(mode, /Standalone transition/i, 'standalone (mode 1) must still be documented'); + }); +}); diff --git a/tests/emitted-drift-acks/1526-auto-chain-transition-delegation.json b/tests/emitted-drift-acks/1526-auto-chain-transition-delegation.json new file mode 100644 index 000000000..09c622bd5 --- /dev/null +++ b/tests/emitted-drift-acks/1526-auto-chain-transition-delegation.json @@ -0,0 +1,8 @@ +{ + "version": 1, + "paths": { + "transition.md": { + "reason": "#1526 (delegate, user decision 2026-08-13): added a `post_completion_mode` step at the top of so transition.md is reusable when invoked by execute-phase AFTER phase.complete + verification already ran. The mode documents two invocations — standalone (run all steps) and post-completion delegation (SKIP verify_completion + update_roadmap_and_state to avoid a double phase.complete; RUN cleanup_handoff; BEGIN at evolve_project). Growth (~1.4 KB) is this one new step; no other step changed. (execute-phase.md, also touched by this PR, shrank slightly — its inline update_project_md + offer_next were replaced by a shorter delegation step — so it needs no ack.)" + } + } +} diff --git a/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json b/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json deleted file mode 100644 index e24a1b290..000000000 --- a/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": 1, - "paths": { - "execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5. #2639: handle_branching now warns when local is ahead of origin (+376 B condensed one-line WARNING + rev-list --count check). #2993 (epic #1671 Phase 6.2): fixes the sibling gap #2932 shipped \u2014 `flag:--wave` section gating was added to the WHEN_VOCABULARY and to init.execute-phase's section manifest, but execute-phase.md never actually parsed `--wave` out of `$ARGUMENTS` or forwarded it on the `gsd_run query init.execute-phase` line, so the flag could never fire. This diff adds a WAVE_PARAM extraction (`--wave ` via BASH_REMATCH) and appends it to the init call, growing the file 163 bytes (89,507 -> 89,670). #2830: the discover_and_group_plans step gained the halt-aware skip rule \u2014 plans whose blocked_by is non-empty are skipped in addition to the existing has_summary skip, and each is reported BY NAME with its cause (\"Skipping {id}: blocked by halted {\u2026}\") rather than silently vanishing from the executable list. Growth is the added rule plus the blocked_by/runnable fields in the documented phase-plan-index parse contract, growing the file 518 bytes (89,670 -> 90,188); no other content changed. #2868: the discover_and_group_plans step no longer exits unconditionally when every plan is filtered out \u2014 it now checks whether the phase ever produced a VERIFICATION.md and, when one is missing on an unfiltered run, resumes at the tail gates (code_review_gate -> close_parent_artifacts -> regression_gate -> verify_phase_goal) instead of stranding the phase. Growth is that rule plus its filter-active and already-verified guards; no other content changed. #3021: orchestrator-cwd branch check regex widened to accept worktree-wf_*." - } -} diff --git a/tests/execute-phase-active-flags.test.cjs b/tests/execute-phase-active-flags.test.cjs index fd9fd9848..7109726a4 100644 --- a/tests/execute-phase-active-flags.test.cjs +++ b/tests/execute-phase-active-flags.test.cjs @@ -359,54 +359,56 @@ const workflowPath = path.resolve( __dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md' ); -describe('bug #2002: offer_next checks CONTEXT.md before suggesting next step', () => { - // offer_next body extracted to references/offer-next.md (#2537); content tests - // read the reference file. The tag + @-reference remain - // in execute-phase.md. - const offerNextRefPath = path.join(__dirname, '..', 'gsd-core', 'references', 'offer-next.md'); - let content; +describe('bug #2002: next-step suggestion checks CONTEXT.md (now via transition offer_next_phase, reached by execute-phase post-completion delegation — #1526)', () => { + // #1526: execute-phase no longer carries an inline offer_next step — it delegates + // post-completion to the transition workflow, whose offer_next_phase step performs + // the #2002 CONTEXT.md-gated next-step suggestion. These tests track that behavior + // in its new home (transition.md) and assert the delegation reaches it. + const transPath = path.resolve(__dirname, '..', 'gsd-core', 'workflows', 'transition.md'); + let offerNextPhase; - test('setup: offer-next reference file is readable', () => { - content = fs.readFileSync(offerNextRefPath, 'utf-8'); - assert.ok(content.length > 0, 'offer-next.md must not be empty'); + test('setup: transition.md offer_next_phase section is readable', () => { + const trans = fs.readFileSync(transPath, 'utf-8'); + const start = trans.indexOf(''); + const end = trans.indexOf('', start); + assert.notEqual(start, -1, 'transition.md must have an offer_next_phase step'); + offerNextPhase = trans.slice(start, end); + assert.ok(offerNextPhase.length > 0, 'offer_next_phase section must be non-empty'); }); - test('execute-phase.md still carries the offer_next step + @-reference', () => { + test('#1526: execute-phase delegates post-completion to transition (no inline offer_next step)', () => { const wf = fs.readFileSync(workflowPath, 'utf-8'); - assert.ok(wf.includes(''), 'offer_next step tag must remain in execute-phase.md'); - assert.ok(wf.includes('references/offer-next.md'), 'execute-phase.md must @-reference the extracted offer-next.md'); + assert.ok(wf.includes('delegate_post_completion_to_transition'), 'execute-phase must delegate post-completion to transition'); + assert.ok(wf.includes('@~/.claude/gsd-core/workflows/transition.md'), 'execute-phase must @-include transition.md'); + assert.equal(wf.includes(''), false, 'inline offer_next step is intentionally removed (delegated to transition.offer_next_phase)'); }); - test('offer_next section checks for CONTEXT.md existence', () => { - content = content || fs.readFileSync(offerNextRefPath, 'utf-8'); + test('offer_next_phase checks for CONTEXT.md existence (#2002 preserved)', () => { assert.ok( - content.includes('CONTEXT.md'), - 'offer_next must reference CONTEXT.md to determine primary next step' + offerNextPhase.includes('CONTEXT.md'), + 'offer_next_phase must reference CONTEXT.md to determine primary next step' ); }); - test('offer_next presents /gsd-discuss-phase when CONTEXT.md does not exist', () => { - content = content || fs.readFileSync(offerNextRefPath, 'utf-8'); + test('offer_next_phase presents /gsd-discuss-phase when CONTEXT.md does not exist', () => { assert.ok( - /CONTEXT\.md.*does not exist|CONTEXT\.md.*not.*exist|If CONTEXT\.md does/i.test(content) || - /gsd-discuss-phase.*recommended|recommended.*gsd-discuss-phase/i.test(content), - 'offer_next must present /gsd-discuss-phase as primary when CONTEXT.md does not exist' + /CONTEXT\.md.*does not exist|CONTEXT\.md.*not.*exist|If CONTEXT\.md does/i.test(offerNextPhase) || + /discuss-phase/i.test(offerNextPhase), + 'offer_next_phase must present /gsd-discuss-phase as primary when CONTEXT.md does not exist' ); }); - test('offer_next presents /gsd-plan-phase when CONTEXT.md exists', () => { - content = content || fs.readFileSync(offerNextRefPath, 'utf-8'); + test('offer_next_phase presents /gsd-plan-phase when CONTEXT.md exists', () => { assert.ok( - /CONTEXT\.md.*exists|exists.*CONTEXT\.md|If CONTEXT\.md/i.test(content), - 'offer_next must present /gsd-plan-phase as primary when CONTEXT.md exists' + /CONTEXT\.md.*exists|exists.*CONTEXT\.md|If CONTEXT\.md/i.test(offerNextPhase), + 'offer_next_phase must present /gsd-plan-phase as primary when CONTEXT.md exists' ); }); - test('offer_next section contains at least one conditional guard before listing commands', () => { - content = content || fs.readFileSync(offerNextRefPath, 'utf-8'); + test('offer_next_phase contains at least one conditional guard before listing commands', () => { assert.ok( - /If CONTEXT\.md/i.test(content), - 'offer_next must contain at least one "If CONTEXT.md" conditional guard' + /If CONTEXT\.md/i.test(offerNextPhase), + 'offer_next_phase must contain at least one "If CONTEXT.md" conditional guard' ); }); }); diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index f39ecf0d8..12b9d80ff 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -565,7 +565,7 @@ test('execute-phase.md: close_phase_todos runs after update_roadmap', () => { test('execute-phase.md: auto-close never blocks phase completion', () => { const closeTodosSection = EXECUTE_PHASE.slice( EXECUTE_PHASE.indexOf('name="close_phase_todos"'), - EXECUTE_PHASE.indexOf('name="update_project_md"') + EXECUTE_PHASE.indexOf('name="delegate_post_completion_to_transition"') ); assert.ok( closeTodosSection.includes('never blocks') || closeTodosSection.includes('additive'),