From 05d4ba814771d23d806c833a84be1673f89c8a62 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 15:16:55 -0400 Subject: [PATCH] Revert "Merge pull request #3578 from gsd-build/fix/3569-init-plan-phase-status" This reverts commit a244dd7fc393be3da09e5aec1215a482d0b08e10, reversing changes made to 8f95b2fe23755437bfe5708fe899f0a60f18c899. --- .changeset/daring-otters-fly.md | 5 --- 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 -------- 6 files changed, 2 insertions(+), 127 deletions(-) delete mode 100644 .changeset/daring-otters-fly.md diff --git a/.changeset/daring-otters-fly.md b/.changeset/daring-otters-fly.md deleted file mode 100644 index 0ae87f843..000000000 --- a/.changeset/daring-otters-fly.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -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). diff --git a/get-shit-done/bin/lib/commands.cjs b/get-shit-done/bin/lib/commands.cjs index dc3b83b28..cc45dc9b7 100644 --- a/get-shit-done/bin/lib/commands.cjs +++ b/get-shit-done/bin/lib/commands.cjs @@ -1011,7 +1011,6 @@ 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 f5e63912b..c5e2b709e 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -10,7 +10,6 @@ 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 @@ -296,20 +295,6 @@ 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 87f219516..dc21ea48c 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`, `phase_status` (#3569), `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`, `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,52 +53,9 @@ 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`, `--force` (override closed-phase gate, see §1.5)). +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`). **`--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 67113e603..16bb97a00 100644 --- a/sdk/src/query/init.test.ts +++ b/sdk/src/query/init.test.ts @@ -443,54 +443,6 @@ 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 db0a59a86..c415910f9 100644 --- a/sdk/src/query/init.ts +++ b/sdk/src/query/init.ts @@ -28,7 +28,6 @@ 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'; @@ -470,17 +469,6 @@ 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. @@ -515,7 +503,6 @@ 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,