golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #3578 from gsd-build/fix/3569-init-plan-phase-status

Browse Source 
fix(3569): surface phase_status from init.plan-phase; gate /gsd:plan-phase on closed phases
This commit is contained in:
Tom Boucher
2026-05-15 15:11:38 -04:00
committed by GitHub
parent 8f95b2fe23 0080c791ed
commit a244dd7fc3
6 changed files with 127 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/daring-otters-fly.md Normal file
View File

@@ -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).

1
get-shit-done/bin/lib/commands.cjs
View File

@@ -1011,6 +1011,7 @@ function cmdCheckCommit(cwd, raw) {
}
module.exports = {
determinePhaseStatus,
cmdGenerateSlug,
cmdCurrentTimestamp,
cmdListTodos,

15
get-shit-done/bin/lib/init.cjs
View File

@@ -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,

47
get-shit-done/workflows/plan-phase.md
View File

@@ -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 <<EOF >&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 <<EOF >&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 <N>`, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd <filepath>`, `--ingest <path-or-glob>`, `--ingest-format <auto|nygard|madr|narrative>`, `--reviews`, `--text`, `--bounce`, `--skip-bounce`, `--chunked`, `--mvp`).
Extract from $ARGUMENTS: phase number (integer or decimal like `2.1`), flags (`--research`, `--skip-research`, `--research-phase <N>`, `--gaps`, `--skip-verify`, `--skip-ui`, `--prd <filepath>`, `--ingest <path-or-glob>`, `--ingest-format <auto|nygard|madr|narrative>`, `--reviews`, `--text`, `--bounce`, `--skip-bounce`, `--chunked`, `--mvp`, `--force` (override closed-phase gate, see §1.5)).
**`--research-phase <N>` — research-only mode (#3042 + #3044).** When this flag is present, parse `<N>` 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.

48
sdk/src/query/init.test.ts
View File

@@ -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<string, unknown>;
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', '---', '<objective>x</objective>'].join('\n'),
);
const result = await initPlanPhase(['10'], tmpDir);
const data = result.data as Record<string, unknown>;
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<string, unknown>;
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<string, unknown>;
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

13
sdk/src/query/init.ts
View File

@@ -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,