From 0aa4bd92fb39950f0e424c8f6888a027474ae59e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 15:04:11 -0400 Subject: [PATCH 1/2] fix(3569): surface phase_status from init.plan-phase + gate /gsd:plan-phase on closed phases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new `phase_status` field to the `init.plan-phase` SDK + CJS query output and a §1.5 "Closed-Phase Gate" in workflows/plan-phase.md that short-circuits on closed phases instead of silently replanning over shipped code. ## What was broken `gsd-sdk query init.plan-phase ` returned the same "ready to plan" payload for a closed phase (REQUIREMENTS Met, VERIFICATION.md status: passed, ROADMAP flipped) as for an open one. No field signaled closure, so `/gsd:plan-phase --reviews` happily replanned over closed phases — risking documentation drift on already-shipped code. ## Fix - Export `determinePhaseStatus` from `commands.cjs` (already present, was module-private). - Both `cmdInitPlanPhase` (CJS) and `initPlanPhase` (TS SDK) now compute `phase_status` from plan/summary counts + VERIFICATION.md status using the existing `determinePhaseStatus` helper — the project-wide phase lifecycle vocabulary (Pending | Planned | In Progress | Executed | Complete | Needs Review). No directory yet → Pending. - Workflow `plan-phase.md` adds §1.5 "Closed-Phase Gate": - `phase_status == "Complete"` with `--reviews` → hard-stop, no override (replanning a closed phase via review feedback is never legitimate; concerns belong in a follow-up phase or new issue). - `phase_status == "Complete"` without `--force` → exit with a clear notice pointing at VERIFICATION.md. - `phase_status == "Complete"` with `--force` → continue with a transcript banner so the deliberate replan is visible. `Executed` and `Needs Review` are intentionally not gated — those mean planning finished but verification did not pass, and replanning is the correct next step. ## Tests - SDK: 4 new `phase_status` cases in init.test.ts covering Pending / Planned / Executed / Complete transitions. - Existing init.plan-phase golden parity test continues to pass (the `researcher_model: '' vs sonnet` drift in that test predates this change and is unrelated). - Full Mac+Docker suite: 9323 / 9323 passed (Mac), 9318 / 9323 passed (Docker, 5 skipped). Fixes #3569 --- get-shit-done/bin/lib/commands.cjs | 1 + get-shit-done/bin/lib/init.cjs | 15 +++++++++ get-shit-done/workflows/plan-phase.md | 47 ++++++++++++++++++++++++-- sdk/src/query/init.test.ts | 48 +++++++++++++++++++++++++++ sdk/src/query/init.ts | 13 ++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) diff --git a/get-shit-done/bin/lib/commands.cjs b/get-shit-done/bin/lib/commands.cjs index cc45dc9b7..dc3b83b28 100644 --- a/get-shit-done/bin/lib/commands.cjs +++ b/get-shit-done/bin/lib/commands.cjs @@ -1011,6 +1011,7 @@ function cmdCheckCommit(cwd, raw) { } module.exports = { + determinePhaseStatus, cmdGenerateSlug, cmdCurrentTimestamp, cmdListTodos, diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index c5e2b709e..f5e63912b 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -10,6 +10,7 @@ const { planningPaths, planningDir, planningRoot } = require('./planning-workspa const { maskIfSecret } = require('./secrets.cjs'); const scanPhasePlans = require('./plan-scan.cjs'); const { stateExtractField } = require('./state-document.cjs'); +const { determinePhaseStatus } = require('./commands.cjs'); // Accept all bold/colon variants of the Requirements header (#2769): // **Requirements:** / **Requirements**: / **Requirements** : render the @@ -295,6 +296,20 @@ function cmdInitPlanPhase(cwd, phase, raw, options = {}) { padded_phase: phaseNumberPlan ? normalizePhaseName(phaseNumberPlan) : null, phase_req_ids, + // #3569: surface phase lifecycle status so /gsd:plan-phase can short-circuit + // on closed (Complete) phases instead of silently replanning over shipped + // code. Reuses determinePhaseStatus — the project-wide vocabulary + // (Pending | Planned | In Progress | Executed | Complete | Needs Review). + // No directory yet → Pending (phase has not been started). + phase_status: phaseDirPlan + ? determinePhaseStatus( + phaseInfo?.plans?.length || 0, + phaseInfo?.summaries?.length || 0, + path.join(cwd, phaseDirPlan), + 'Pending', + ) + : 'Pending', + // Existing artifacts has_research: phaseInfo?.has_research || false, has_context: phaseInfo?.has_context || false, diff --git a/get-shit-done/workflows/plan-phase.md b/get-shit-done/workflows/plan-phase.md index dc21ea48c..87f219516 100644 --- a/get-shit-done/workflows/plan-phase.md +++ b/get-shit-done/workflows/plan-phase.md @@ -45,7 +45,7 @@ When `TDD_MODE` is `true`, the planner agent is instructed to apply `type: tdd` When `CONTEXT_WINDOW >= 500000`, the planner prompt includes the 3 most recent prior phase CONTEXT.md and SUMMARY.md files PLUS any phases explicitly listed in the current phase's `Depends on:` field in ROADMAP.md. Explicit dependencies always load regardless of recency (e.g., Phase 7 declaring `Depends on: Phase 2` always sees Phase 2's context). Bounded recency keeps the planner's context budget focused on recent work. -Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`. +Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_enabled`, `plan_checker_enabled`, `nyquist_validation_enabled`, `commit_docs`, `text_mode`, `phase_found`, `phase_dir`, `phase_number`, `phase_name`, `phase_slug`, `padded_phase`, `has_research`, `has_context`, `has_reviews`, `has_plans`, `plan_count`, `phase_status` (#3569), `planning_exists`, `roadmap_exists`, `phase_req_ids`, `response_language`. **If `response_language` is set:** Include `response_language: {value}` in all spawned subagent prompts so any user-facing output stays in the configured language. @@ -53,9 +53,52 @@ Parse JSON for: `researcher_model`, `planner_model`, `checker_model`, `research_ **If `planning_exists` is false:** Error — run `/gsd:new-project` first. +## 1.5. Closed-Phase Gate (#3569) + +The init JSON includes `phase_status` — one of `Pending | Planned | In Progress | Executed | Complete | Needs Review`. `Complete` means the phase has all summaries AND a `VERIFICATION.md` with `status: passed`. Replanning a closed phase silently rewrites plan docs that no longer match the shipped code, so the workflow must hard-stop here unless the operator explicitly overrides. + +Parse `phase_status` from the init JSON, then: + +```bash +FORCE_REPLAN=false +if [[ "$ARGUMENTS" =~ (^|[[:space:]])--force([[:space:]]|$) ]]; then + FORCE_REPLAN=true +fi + +if [ "${phase_status}" = "Complete" ]; then + if [[ "$ARGUMENTS" =~ (^|[[:space:]])--reviews([[:space:]]|$) ]]; then + # --reviews on a closed phase is never legitimate — concerns belong in a + # new phase or issue against the closed phase's commits. + cat <&2 +Phase ${phase_number} (${phase_name}) is already CLOSED (VERIFICATION status: passed). +/gsd:plan-phase --reviews cannot replan a closed phase. If the review surfaced +real concerns, open a follow-up phase or file an issue against the closed +phase's commits. There is no --force override for --reviews on a closed phase. +EOF + exit 1 + fi + if [ "$FORCE_REPLAN" != "true" ]; then + cat <&2 +Phase ${phase_number} (${phase_name}) is already CLOSED (VERIFICATION status: passed). +Replanning a closed phase will overwrite plan docs that no longer match the +shipped code. If you intentionally want to replan over closed work, re-run +with: /gsd:plan-phase ${phase_number} --force + +Otherwise, to view what shipped, see: ${verification_path} +EOF + exit 1 + fi + # FORCE_REPLAN=true: continue, but emit a banner so the operator sees the + # decision in the transcript and in any committed plan docs. + echo "WARNING: Replanning CLOSED phase ${phase_number} under --force. Verify the closeout was wrong before committing new plan docs." >&2 +fi +``` + +The gate fires only on `Complete`. `Executed` and `Needs Review` are not gated — those states mean planning was finished but verification did not pass, and replanning is a legitimate next step. + ## 2. Parse and Normalize Arguments -Extract from $ARGUMENTS: phase number (integer or decimal like `2.1`), flags (`--research`, `--skip-research`, `--research-phase `, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd `, `--ingest `, `--ingest-format `, `--reviews`, `--text`, `--bounce`, `--skip-bounce`, `--chunked`, `--mvp`). +Extract from $ARGUMENTS: phase number (integer or decimal like `2.1`), flags (`--research`, `--skip-research`, `--research-phase `, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd `, `--ingest `, `--ingest-format `, `--reviews`, `--text`, `--bounce`, `--skip-bounce`, `--chunked`, `--mvp`, `--force` (override closed-phase gate, see §1.5)). **`--research-phase ` — research-only mode (#3042 + #3044).** When this flag is present, parse `` as the phase number (overrides any positional phase argument), set `RESEARCH_ONLY=true`, and treat the rest of this workflow as a research-dispatch only — the planner spawn (step 8), plan-checker, verification, gaps, bounce, and post-planning-gaps blocks all skip on `RESEARCH_ONLY`. Use this for cross-phase research, doc review before committing to a planning approach, and correction-without-replanning loops. Replaces the deleted `/gsd-research-phase` command. diff --git a/sdk/src/query/init.test.ts b/sdk/src/query/init.test.ts index 16bb97a00..67113e603 100644 --- a/sdk/src/query/init.test.ts +++ b/sdk/src/query/init.test.ts @@ -443,6 +443,54 @@ describe('initPlanPhase', () => { expect(data.error).toBeDefined(); }); + // #3569: init.plan-phase must surface a phase_status field so the + // /gsd-plan-phase workflow can short-circuit on closed phases instead of + // happily replanning over shipped code. Reuses the project-wide phase + // lifecycle vocabulary from determinePhaseStatus (Pending | Planned | + // In Progress | Executed | Complete | Needs Review). + describe('phase_status (#3569)', () => { + it('reports "Complete" when summaries match plans and VERIFICATION.md status: passed', async () => { + // Phase 9 fixture already has 1 plan + 1 summary; add a passing VERIFICATION. + await writeFile( + join(tmpDir, '.planning', 'phases', '09-foundation', '09-VERIFICATION.md'), + ['---', 'phase: 09', 'status: passed', 'score: 100', 'verified: true', '---', '# Verification'].join('\n'), + ); + + const result = await initPlanPhase(['9'], tmpDir); + const data = result.data as Record; + expect(data.phase_status).toBe('Complete'); + }); + + it('reports "Planned" when plans exist but no summaries written', async () => { + // Phase 10 has no plan files in the beforeEach fixture. Add a plan to flip + // it from "Pending" (no plans) to "Planned" (plans, no summaries). + await writeFile( + join(tmpDir, '.planning', 'phases', '10-read-only-queries', '10-01-PLAN.md'), + ['---', 'phase: 10-read-only-queries', 'plan: 01', '---', 'x'].join('\n'), + ); + + const result = await initPlanPhase(['10'], tmpDir); + const data = result.data as Record; + expect(data.phase_status).toBe('Planned'); + }); + + it('reports "Pending" when phase has no plans yet', async () => { + const result = await initPlanPhase(['10'], tmpDir); + const data = result.data as Record; + expect(data.phase_status).toBe('Pending'); + }); + + it('reports "Executed" when summaries match plans but VERIFICATION.md is absent', async () => { + // Phase 9 fixture: 1 plan, 1 summary, no VERIFICATION yet — executed but + // not closed. This is the regression hot zone: pre-fix, init.plan-phase + // gave no signal here, so the workflow couldn't distinguish this from + // an already-closed phase either. + const result = await initPlanPhase(['9'], tmpDir); + const data = result.data as Record; + expect(data.phase_status).toBe('Executed'); + }); + }); + // #2769: extractReqIds must accept all bold/colon variants of the // Requirements header. The forms render identically in markdown but differ // textually; the previous regex only matched **Requirements**: (colon diff --git a/sdk/src/query/init.ts b/sdk/src/query/init.ts index c415910f9..db0a59a86 100644 --- a/sdk/src/query/init.ts +++ b/sdk/src/query/init.ts @@ -28,6 +28,7 @@ import { resolveModel, MODEL_PROFILES } from './config-query.js'; import { maskIfSecret } from './secrets.js'; import { findPhase } from './phase.js'; import { roadmapGetPhase, getMilestoneInfo, extractCurrentMilestone, extractPhasesFromSection } from './roadmap.js'; +import { determinePhaseStatus } from './progress.js'; import { planningPaths, normalizePhaseName, toPosixPath, resolveAgentsDir, detectRuntime } from './helpers.js'; import { generatePhaseSlug, assertSafeProjectCode } from './phase-lifecycle-policy.js'; import type { QueryHandler } from './utils.js'; @@ -469,6 +470,17 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream) const phaseName = (phaseInfo?.phase_name as string) ?? null; const phaseDir = (phaseInfo?.directory as string) ?? null; const plans = (phaseInfo?.plans || []) as string[]; + const summaries = (phaseInfo?.summaries || []) as string[]; + + // #3569: surface phase lifecycle status so /gsd-plan-phase can short-circuit + // on closed (Complete) phases instead of silently replanning over shipped + // code. Reuses determinePhaseStatus — the project-wide vocabulary used by + // `progress` (Pending | Planned | In Progress | Executed | Complete | + // Needs Review). When the phase has no directory on disk yet, treat it as + // Pending (it has not been started). + const phaseStatus = phaseDir + ? await determinePhaseStatus(plans.length, summaries.length, join(projectDir, phaseDir)) + : 'Pending'; // #3287: compute the canonical directory name with project_code prefix so // the first-touch mkdir in /gsd-plan-phase stays consistent with phase.add. @@ -503,6 +515,7 @@ export const initPlanPhase: QueryHandler = async (args, projectDir, workstream) phase_slug: (phaseInfo?.phase_slug as string) ?? null, padded_phase: phaseNumber ? normalizePhaseName(phaseNumber) : null, phase_req_ids, + phase_status: phaseStatus, has_research: (phaseInfo?.has_research as boolean) || false, has_context: (phaseInfo?.has_context as boolean) || false, has_reviews: (phaseInfo?.has_reviews as boolean) || false, From 0080c791ed3d2e6fad29b773f67421e48a35e6d6 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 15:07:49 -0400 Subject: [PATCH 2/2] chore(3569): add changeset fragment for PR #3578 Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/daring-otters-fly.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/daring-otters-fly.md diff --git a/.changeset/daring-otters-fly.md b/.changeset/daring-otters-fly.md new file mode 100644 index 000000000..0ae87f843 --- /dev/null +++ b/.changeset/daring-otters-fly.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3578 +--- +**`/gsd:plan-phase` now refuses to replan closed phases** — `init.plan-phase` exposes a new `phase_status` field and the workflow short-circuits on `Complete` phases (use `--force` to override; `--reviews` has no override).