fix(3569): surface phase_status from init.plan-phase + gate /gsd:plan-phase on closed phases
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 <N>` 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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user