From 481ac7c71bf71cb1418f32e4b2cea1f882f619af Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 5 Aug 2026 10:42:42 -0400 Subject: [PATCH] fix(#2946): run milestone complete unstarted-phase guard independent of STATE (#3081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2946): milestone complete unstarted-phase guard fails open on STATE desync Row 1 of the test matrix: the regression test that fails first. Adds seven cases to tests/milestone.test.cjs covering the desync, absent, no-file, --force-override, mismatch-WARNING, fresh-project-noop, and sentinel-skip behaviors. RED on next: the guard's entire scan is nested inside `if (stateVersion && stateVersion === version)`, so any STATE.md milestone: value that does not exactly equal the version argument skips the scan with no warning — functionally an implicit --force on a one-way-door operation. * fix(#2946): run milestone complete unstarted-phase guard independent of STATE The entire ROADMAP phase-directory scan was nested inside `if (stateVersion && stateVersion === version)`, so any STATE.md milestone: value that did not exactly string-equal the version argument — a desynced value, or no milestone: field at all — skipped the scan with no warning, functionally an implicit --force. The operation the guard fronts is a one-way door: ROADMAP.md and REQUIREMENTS.md are archived and phase directories are MOVED into .planning/milestones/-phases/. The scan was already driven by the version argument through getMilestonePhaseFilter / extractCurrentMilestone; the STATE match was a redundant second gate that shadowed and broke it. Decouple: the scan now runs whenever --force is absent, and a present-but-mismatched STATE milestone: field emits a WARNING naming both values so the suspicious condition is visible rather than silent. A fresh project with no Phase headings in the scoped slice still yields an empty scan (no false positives) — the intent the STATE-match short-circuit was reaching for, now achieved by the scan itself. * docs(#2946): document milestone complete --force, --dry-run, and the unstarted-phase guard The CLI-TOOLS reference signature omitted --force and --dry-run entirely, and neither the unstarted-phase guard nor its override was documented anywhere user-facing. Add a flags table and a factual guard description to the Reference page (CLI-TOOLS.md), and a practical guard note to the /gsd-complete-milestone How-to (COMMANDS.md) covering what to do when the guard fires and the new STATE-mismatch WARNING (#2946). American English per CONTRIBUTING.md language policy. * fix(#2946): emit STATE-mismatch WARNING as JSON in --json-errors mode Follow-up to the guard decoupling: a structured caller using --json-errors parses stderr line-by-line as JSON, so the plain-text WARNING would break such a parser. Honor getJsonErrorMode() and emit a structured JSON object ({ ok, level, message }) in that mode, plain text otherwise — mirroring io.cts error()'s JSON shape. Addresses the isolated-review observation (~45% but credible, since --json-errors is a documented CLI flag). * fix(#2946): address review — drop JSON-mode WARNING scope creep, tighten test assertions Standards + spec review findings (code-review two-axis + isolated adversarial): 1. The --json-errors JSON WARNING branch (commit dee719404) was scope creep the issue never asked for, AND emitted ok:true for a suspicious-condition warning (a category error — a stderr JSON parser keying on ok would treat the suspicious state as success), AND its comment falsely claimed to mirror io.cts error()'s {ok:false,reason,message} shape. Dropped: the WARNING is now plain-text stderr only, matching the existing [gsd-tools] WARNING convention (state.cts). The issue asked for 'at minimum warn', not a structured JSON surface. 2. The WARNING test asserted on /WARNING/ regex (raw-text matching on stderr prose). Tightened to assert on the stable operator-facing tokens — the WARNING: marker and both version literals the operator must see — not the surrounding formatter prose. Added a paired negative test confirming no WARNING is emitted for an absent milestone: field (a missing declaration is a normal fresh-project state, not suspicious drift). * fix(#2946): sanitize STATE milestone value before stderr WARNING interpolation Security review (minor): stateVersion is read from a user-controlled file (STATE.md) and is not validated like the CLI version arg. Sanitize before interpolating into the WARNING — strip ANSI/control chars (/[\x00-\x1f\x7f]/g -> '?') and truncate to 80 chars — so a corrupted or hostile STATE.md cannot echo terminal escapes or secret-looking strings verbatim into a CI log or terminal aggregator (CONTRIBUTING.md security: secret-looking values in stderr). version is already constrained to [A-Za-z0-9._-] by ARCHIVE_VERSION_LABEL_RE upstream, so it needs no sanitization. * test(#2946): correct stale fixtures that relied on the guard being silently disabled Four pre-existing tests broke under the #2946 fix because their fixtures only passed thanks to the bug — the unstarted-phase guard was skipping on STATE mismatch, so fixtures with missing or non-matching phase directories slipped through. The tests exercise version-forwarding / version-scoping, not the guard, so give them legitimate directories: - milestone.test.cjs #3043: dirs were '103.old'/'104.old'/'108.new' (dot), which phaseTokenMatches rejects — renamed to hyphen form. The v3.6 stats scoping still yields 1 phase (getMilestonePhaseFilter scopes correctly); the guard now sees all three phases as having directories. - milestone-archive.test.cjs 'returns version in response data': ROADMAP listed Phase 1 but no directory was created. Added 01-foundation so the scan is satisfied. Per CONTRIBUTING.md, test-fixture corrections land as their own test: commit, not bundled into fix: (release hotfix cherry-pick routes by prefix). * chore(#2946): backfill changeset PR number 3081 --------- Co-authored-by: sim --- .changeset/humble-sloths-forage.md | 5 + docs/CLI-TOOLS.md | 14 +- docs/COMMANDS.md | 2 + src/milestone.cts | 122 ++++++++++------- tests/milestone-archive.test.cjs | 4 + tests/milestone.test.cjs | 201 ++++++++++++++++++++++++++++- 6 files changed, 300 insertions(+), 48 deletions(-) create mode 100644 .changeset/humble-sloths-forage.md diff --git a/.changeset/humble-sloths-forage.md b/.changeset/humble-sloths-forage.md new file mode 100644 index 000000000..6a7470f6f --- /dev/null +++ b/.changeset/humble-sloths-forage.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3081 +--- +**`milestone complete` no longer silently disarms its unstarted-phase guard when STATE.md's `milestone:` field drifts** — the guard now runs whenever the ROADMAP can be scoped for the requested version (independent of STATE), and a STATE mismatch emits a WARNING naming both values instead of skipping the scan. (#2946) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 4004b109a..7d430e493 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -533,13 +533,25 @@ if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi ```bash # Archive milestone -node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] +node gsd-tools.cjs milestone complete [--name ] [--no-archive-phases] [--force] [--dry-run] # Mark requirements as complete node gsd-tools.cjs requirements mark-complete # Accepts: REQ-01,REQ-02 or REQ-01 REQ-02 or [REQ-01, REQ-02] ``` +**`milestone complete` flags** + +| Flag | Description | +|------|-------------| +| `` | Milestone version label to archive (e.g. `v1.0`). | +| `--name ` | Display name for the MILESTONES.md entry. Defaults to ``. | +| `--no-archive-phases` | Leave phase directories in place instead of moving them into `.planning/milestones/-phases/`. | +| `--force` | Override the unstarted-phase guard (see below). | +| `--dry-run` | Print the archive plan (roadmap, requirements, phases to move) without mutating anything. | + +**Unstarted-phase guard.** Before archiving, the command scans the ROADMAP scoped for `` and refuses if any `### Phase N:` heading in that slice has no matching phase directory on disk (`disk_status: no_directory`). Phase 0 (pre-milestone) and Phase 999 (backlog) sentinels are excluded. The guard runs whenever `--force` is absent, independent of `STATE.md`'s `milestone:` field — if that field is present but does not match ``, a WARNING naming both values is emitted to stderr and the scan still runs (#2946). Pass `--force` to override. + --- ## Agent Skills diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index acf384afc..774a7bc14 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -472,6 +472,8 @@ If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowle > **Note:** the `deferred-items.md` category is the per-phase SCOPE BOUNDARY log a phase agent writes when it finds a defect it should not fix. It is a different artifact from the `## Deferred Items` section `[A]` writes into `STATE.md`, which records what you acknowledged at close. +> **Unstarted-phase guard.** Archiving refuses if the milestone's ROADMAP still lists a phase with no phase directory on disk — `Cannot mark milestone complete: ROADMAP lists N unstarted phase(s)`. If a phase was intentionally deferred or merged without a directory, run `gsd-tools milestone complete --force` (the `/gsd-complete-milestone` workflow runs the underlying command without `--force`, so use the CLI directly to override). A `STATE.md` `milestone:` value that does not match `` prints a WARNING and still runs the guard (#2946). + --- ### `/gsd-milestone-summary` diff --git a/src/milestone.cts b/src/milestone.cts index 6ce333999..e9babe30e 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -527,13 +527,25 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo // Guard: prevent marking complete when ROADMAP still lists phases that have // no directory on disk (disk_status: no_directory). This catches the case // where the active milestone was erroneously marked complete before phases - // were even started. Only fires when STATE.md confirms the current milestone - // version matches what is being completed — no false positives on fresh - // projects where phases haven't been scaffolded yet. + // were even started. The scan scopes the ROADMAP via the `version` argument + // (getMilestonePhaseFilter / extractCurrentMilestone above) and runs whenever + // --force is absent — a fresh project with no `### Phase N:` headings in the + // scoped slice yields an empty `noDirectoryPhases` and the guard is a no-op, + // so no STATE match is required to avoid false positives. // Pass --force to override this guard. + // + // #2946: the scan used to be nested inside `if (stateVersion && stateVersion + // === version)`, which silently disarmed the guard whenever STATE.md's + // `milestone:` field was desynced or absent — functionally an implicit + // --force on a one-way-door operation (ROADMAP/REQUIREMENTS archived, phase + // directories MOVED). The STATE field is not the source of truth for which + // phases belong to this milestone; the ROADMAP scoping is. The scan now runs + // unconditionally, and a present-but-mismatched STATE field emits a WARNING + // so the suspicious condition is visible rather than silent. if (!options.force) { try { - // Only guard when STATE.md's milestone field matches the version being completed. + // Read STATE.md's milestone field only to detect a suspicious mismatch; + // it no longer gates the scan. (#2946) let stateVersion: string | null = null; try { const stateRaw = fs.existsSync(statePath) ? fs.readFileSync(statePath, 'utf-8') : null; @@ -542,57 +554,75 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo if (milestoneMatch) stateVersion = milestoneMatch[1].trim(); } } catch { - /* skip */ + /* skip — stateVersion stays null, scan still runs */ + } + if (stateVersion !== null && stateVersion !== version) { + // #2946: emit a WARNING so the suspicious STATE mismatch is visible + // rather than silently disarming the guard. Plain-text diagnostic on + // stderr, matching the existing [gsd-tools] WARNING convention + // (state.cts). A missing STATE.md `milestone:` field is not warned + // here — the scan still runs, and "no milestone declared" is a normal + // state for a fresh project, not a suspicious drift. + // + // `stateVersion` comes from a user-controlled file (STATE.md) and is + // not validated like the CLI `version` arg (ARCHIVE_VERSION_LABEL_RE). + // Sanitize before interpolating into stderr so ANSI escapes / control + // chars / secret-looking strings cannot be echoed verbatim into a CI + // log or terminal (CONTRIBUTING.md security: secret-looking values in + // stderr). `version` is already constrained to [A-Za-z0-9._-]. + const safeStateVersion = stateVersion.replace(/[\x00-\x1f\x7f]/g, '?').slice(0, 80); + process.stderr.write( + `[gsd-tools] WARNING: STATE.md milestone: "${safeStateVersion}" ≠ requested "${version}" — ` + + `running the unstarted-phase guard against the ROADMAP scoped for "${version}" anyway.\n`, + ); } - if (stateVersion && stateVersion === version) { - const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); - const scopedContent = extractCurrentMilestone(roadmapContent, cwd); - // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); - const noDirectoryPhases: string[] = []; - let pm: RegExpExecArray | null; - const phaseDirEntries = ((): string[] => { - try { - return fs - .readdirSync(phasesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory()) - .map((e) => e.name); - } catch { - return []; - } - })(); - while ((pm = phasePattern.exec(scopedContent)) !== null) { - const phaseNum = pm[1]; - // Phase 0 (pre-milestone) and Phase 999 (backlog) are sentinels, not - // real phases — they legitimately have no directory and must not block - // milestone completion. Mirrors the engine-wide sentinel convention - // (phase-id getMilestoneFromPhaseId, roadmap-command-router SENTINELS, - // the #1445 /^999/ progress filters). (#1580) - const major = parseInt(phaseNum, 10); - if (major === 0 || major === 999) continue; - const normalized = normalizePhaseName(phaseNum); - // A phase has disk_status: 'no_directory' when no phase directory - // with a matching token exists on disk. Use the same phaseTokenMatches - // helper that roadmap.analyze uses to avoid false positives on decimal - // (2.1) and letter-suffix (12A) phase IDs. - const hasDirectory = phaseDirEntries.some((d) => phaseTokenMatches(d, normalized)); - if (!hasDirectory) { - noDirectoryPhases.push(phaseNum); - } + const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); + const scopedContent = extractCurrentMilestone(roadmapContent, cwd); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). + const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + const noDirectoryPhases: string[] = []; + let pm: RegExpExecArray | null; + const phaseDirEntries = ((): string[] => { + try { + return fs + .readdirSync(phasesDir, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name); + } catch { + return []; } - if (noDirectoryPhases.length > 0) { - error( - `Cannot mark milestone complete: ROADMAP lists ${noDirectoryPhases.length} unstarted phase(s) ` + - `(e.g. Phase ${noDirectoryPhases[0]}). Re-run with --force to override.`, - ); + })(); + while ((pm = phasePattern.exec(scopedContent)) !== null) { + const phaseNum = pm[1]; + // Phase 0 (pre-milestone) and Phase 999 (backlog) are sentinels, not + // real phases — they legitimately have no directory and must not block + // milestone completion. Mirrors the engine-wide sentinel convention + // (phase-id getMilestoneFromPhaseId, roadmap-command-router SENTINELS, + // the #1445 /^999/ progress filters). (#1580) + const major = parseInt(phaseNum, 10); + if (major === 0 || major === 999) continue; + const normalized = normalizePhaseName(phaseNum); + // A phase has disk_status: 'no_directory' when no phase directory + // with a matching token exists on disk. Use the same phaseTokenMatches + // helper that roadmap.analyze uses to avoid false positives on decimal + // (2.1) and letter-suffix (12A) phase IDs. + const hasDirectory = phaseDirEntries.some((d) => phaseTokenMatches(d, normalized)); + if (!hasDirectory) { + noDirectoryPhases.push(phaseNum); } } + if (noDirectoryPhases.length > 0) { + error( + `Cannot mark milestone complete: ROADMAP lists ${noDirectoryPhases.length} unstarted phase(s) ` + + `(e.g. Phase ${noDirectoryPhases[0]}). Re-run with --force to override.`, + ); + } } catch (e) { // If the error came from our guard, re-throw it; otherwise skip silently. const message = e instanceof Error ? e.message : String(e); if (message && message.startsWith('Cannot mark milestone complete:')) throw e; - // Phase scan failed or STATE version mismatch — allow completion to proceed. + // Phase scan failed (e.g. ROADMAP unreadable) — allow completion to proceed. } } diff --git a/tests/milestone-archive.test.cjs b/tests/milestone-archive.test.cjs index 706477a60..ca7c4f187 100644 --- a/tests/milestone-archive.test.cjs +++ b/tests/milestone-archive.test.cjs @@ -58,6 +58,10 @@ describe('bug #2684: milestone.complete forwards version to phases.archive', () path.join(tmpDir, '.planning', 'ROADMAP.md'), `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n`, ); + // #2946: the unstarted-phase guard now runs whenever --force is absent + // (independent of STATE.md). This test exercises version-forwarding, not + // the guard, so give Phase 1 a real directory so the scan is satisfied. + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); const result = runSdkQuery(['milestone.complete', 'v2.5'], tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); diff --git a/tests/milestone.test.cjs b/tests/milestone.test.cjs index 342714ae7..32dc11ddb 100644 --- a/tests/milestone.test.cjs +++ b/tests/milestone.test.cjs @@ -1514,7 +1514,7 @@ describe('milestone complete explicit version scope (#3043)', () => { ); fs.writeFileSync(path.join(tmpDir, '.planning', 'REQUIREMENTS.md'), '# Requirements\n'); - for (const [dir, liner] of [['103.old', 'old milestone A'], ['104.old', 'old milestone B'], ['108.new', 'new milestone']]) { + for (const [dir, liner] of [['103-old', 'old milestone A'], ['104-old', 'old milestone B'], ['108-new', 'new milestone']]) { const p = path.join(tmpDir, '.planning', 'phases', dir); fs.mkdirSync(p, { recursive: true }); fs.writeFileSync(path.join(p, 'SUMMARY.md'), `one-liner: ${liner}\n\n## Summary\n${liner.split(' ')[0]}\n`); @@ -1762,6 +1762,205 @@ describe('bug-978: milestone complete --force overrides unstarted-phase guard', }); } +// ──────────────────────────────────────────────────────────────────────── +// bug #2946: milestone complete unstarted-phase guard fails open on STATE desync +// ──────────────────────────────────────────────────────────────────────── +// +// The guard that refuses to archive a milestone while the ROADMAP still lists +// unstarted phases used to nest its entire ROADMAP scan inside +// `if (stateVersion && stateVersion === version)`. Any STATE.md `milestone:` +// value that did not exactly string-equal the version argument — a desynced +// value, or no `milestone:` field at all — skipped the scan with no warning, +// functionally equivalent to an implicit `--force`. The operation the guard +// fronts is a one-way door: ROADMAP.md and REQUIREMENTS.md are archived and +// phase directories are MOVED into `.planning/milestones/-phases/`. +// +// The scan was already driven by the `version` argument through +// getMilestonePhaseFilter / extractCurrentMilestone; the STATE match was a +// redundant second gate that shadowed and broke it. After the fix the scan +// runs whenever `--force` is absent and the ROADMAP can be scoped for the +// version; a present-but-mismatched STATE.md `milestone:` field additionally +// emits a WARNING naming both values. + +describe('bug #2946: unstarted-phase guard runs independent of STATE.md milestone field', () => { + const { test, beforeEach, afterEach } = require('node:test'); + const assert = require('node:assert/strict'); + const fs = require('fs'); + const path = require('path'); + const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('gsd-bug-2946-'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + /** + * Build a fixture where the guard MUST fire if it runs: + * - ROADMAP.md scopes for `version` (heading includes the version literal) + * and lists a `### Phase 2:` heading with no on-disk phase directory. + * - STATE.md `milestone:` field is shaped by `stateMode`: + * 'sync' → milestone: + * 'desync' → milestone: -closing + * 'absent' → no milestone: line in frontmatter + * 'no-file' → STATE.md not written at all + * The unstarted phase is a REAL phase number (Phase 0 / 999 are sentinels + * excluded by the scan, #1580, so they would not fire it). + */ + function makeFixture(tmpDir, version, stateMode) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap ${version}\n\n### Phase 2: Real Work\n**Goal:** Not started\n`, + ); + if (stateMode === 'no-file') return; + let frontmatter; + if (stateMode === 'sync') frontmatter = `milestone: ${version}\n`; + else if (stateMode === 'desync') frontmatter = `milestone: ${version}-closing\n`; + else frontmatter = ''; // 'absent' + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\n${frontmatter}---\n# State\n\n**Status:** In progress\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, + ); + } + + test('without --force the guard fires even when STATE.md milestone: desyncs from the requested version', () => { + makeFixture(tmpDir, 'v1.0', 'desync'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run'], + tmpDir, + ); + assert.strictEqual(result.success, false, 'guard must fire on STATE desync, not fail open'); + assert.ok( + result.error.includes('Re-run with --force to override'), + `expected guard error message; got: ${result.error}`, + ); + }); + + test('without --force the guard fires even when STATE.md has no milestone: field', () => { + makeFixture(tmpDir, 'v1.0', 'absent'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run'], + tmpDir, + ); + assert.strictEqual(result.success, false, 'guard must fire when milestone: field is absent'); + assert.ok( + result.error.includes('Re-run with --force to override'), + `expected guard error message; got: ${result.error}`, + ); + }); + + test('without --force the guard fires even when STATE.md does not exist', () => { + makeFixture(tmpDir, 'v1.0', 'no-file'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run'], + tmpDir, + ); + assert.strictEqual(result.success, false, 'guard must fire when STATE.md is missing entirely'); + assert.ok( + result.error.includes('Re-run with --force to override'), + `expected guard error message; got: ${result.error}`, + ); + }); + + test('with --force the guard is bypassed even when STATE.md milestone: desyncs', () => { + makeFixture(tmpDir, 'v1.0', 'desync'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run', '--force'], + tmpDir, + ); + assert.ok( + result.success, + `--force must override the guard on the desync path too; got: ${result.error}`, + ); + const out = JSON.parse(result.output); + assert.strictEqual(out.dry_run, true, 'preview should run past the guard with --force'); + }); + + test('a STATE milestone mismatch emits a warning naming both versions', () => { + makeFixture(tmpDir, 'v1.0', 'desync'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run'], + tmpDir, + ); + // Guard fires (failure path) — the WARNING is written to stderr before + // the `Error:` line, so it appears in result.error. Assert on the stable + // operator-facing tokens (the WARNING marker and both version literals + // the operator must see), not the surrounding prose — the prose is a + // human formatter and may be reworded. + assert.strictEqual(result.success, false); + assert.ok( + result.error.includes('WARNING:'), + `expected a WARNING marker on stderr; got: ${result.error}`, + ); + assert.ok( + result.error.includes('v1.0-closing') && result.error.includes('v1.0'), + `warning should name both the STATE value and the requested version; got: ${result.error}`, + ); + }); + + test('no WARNING is emitted when STATE.md milestone: field is absent (fresh project is not suspicious drift)', () => { + // The scan still runs and fires (covered by the absent-field test above), + // but a missing milestone: declaration is a normal fresh-project state, + // not a mismatch — so no WARNING should accompany it. + makeFixture(tmpDir, 'v1.0', 'absent'); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression Test', '--dry-run'], + tmpDir, + ); + assert.strictEqual(result.success, false, 'guard must still fire on absent field'); + assert.ok( + !result.error.includes('WARNING:'), + `no WARNING expected for an absent milestone: field; got: ${result.error}`, + ); + }); + + test('guard is a no-op when the scoped ROADMAP has no Phase headings (fresh project)', () => { + // STATE in sync, ROADMAP scopes for v1.0 but lists NO phase headings → + // scan yields zero unstarted phases, guard must not fire. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap v1.0\n\nThis milestone has no phases yet.\n`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\nmilestone: v1.0\n---\n# State\n\n**Status:** In progress\n`, + ); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Fresh', '--dry-run'], + tmpDir, + ); + assert.ok( + result.success, + `guard must be a no-op when the scoped slice has no phase headings; got: ${result.error}`, + ); + }); + + test('Phase 0 and Phase 999 sentinels do not fire the unstarted-phase guard', () => { + // STATE absent (the strictest case for the new guard). ROADMAP has only + // sentinel phases with no directories — they must be skipped (#1580). + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap v1.0\n\n### Phase 0: Pre-milestone\n**Goal:** Setup\n\n### Phase 999: Backlog\n**Goal:** Later\n`, + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `---\n---\n# State\n\n**Status:** In progress\n`, + ); + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Sentinel', '--dry-run'], + tmpDir, + ); + assert.ok( + result.success, + `Phase 0 / 999 sentinels must not fire the guard; got: ${result.error}`, + ); + }); +}); + // ──────────────────────────────────────────────────────────────────────── // Folded from tests/bug-2660-one-liner-extraction.test.cjs — consolidation epic #1969 (B3 #1972) // ────────────────────────────────────────────────────────────────────────