From 86bebcefa2c5130c15e11ef89670a19642d32021 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 8 Aug 2026 19:06:13 -0400 Subject: [PATCH] refactor(#3216): bind milestone identity to the canonical locator (#3226) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(#3216): widen milestone-window guard to literal-## matchers The guard keyed only on the `#{N,M}` quantifier plus a literal version or phase-lookahead token. getMilestoneInfo hand-rolls its milestone-heading match with a literal `^##`/`## ` and an interpolated ${escapedVer}, so it satisfied neither token and the guard reported a clean zero on a file carrying live re-derivations (#3171, #3197) — a zero it did not earn. Widen token (a) to a literal 2-6 `#` run, admitted ONLY inside a heading-MATCHER literal (a regex literal, or a string/template handed to new RegExp) so a heading-BUILDING template is not mistaken for a re-derivation. Widen token (b) with the grouped `v(\d+(?:\.\d+)+)` shape and an interpolated version placeholder. Ships BEFORE the consolidation per ADR-3180 s7.2: a guard widened afterwards measures an already-cleaned surface. It is expected to be RED until the consolidation lands. * test(#3216): failing-first milestone-identity single-owner suite 63 tests across two files, from the matrix in .gsd/phase/. Section H of milestone-window-single-owner.test.cjs covers the 21 input classes of the design's behavior table plus its negative space; milestone-window-drift-guard covers the widened tokens and proves the exemption is function-scoped, not file-scoped. Copy count is 3 found by the guard, not 1 per the epic (ADR-3180 Amendment 3's standing rule, holding for the fourth consecutive phase): both getMilestoneInfo sites plus cmdRoadmapAnalyze's milestone enumeration at roadmap.cts:454, which carries the same #3171 truncation and #3197 phase-heading confusion. Expected RED until the consolidation lands. * refactor(#3216): bind milestone identity to the canonical locator getMilestoneInfo hand-rolled two milestone-heading regexes inside the owner's own file. Both were wrong, differently: the STATE-version site's ^## anchor is level-blind so [^\n]* absorbs a third #, and the fallback site had no anchor at all, so '## ' matched from the second # of '###'. Against '### Phase 7: Close v3.3 gaps' the fallback returned {v3.3, gaps} (#3197). Both captured names with [^\n(], truncating at a parenthetical (#3171). Bind both to the canonical grammar. locateMilestoneHeadings becomes a version-filtered view over one shared source, and a new version-agnostic listMilestoneHeadings enumerates milestone headings for callers that need all of them. getMilestoneInfo returns ScopedResult; the {v1.0,'milestone'} default, which was output-identical to a real v1.0 project, is deleted. The #2245 never-throws invariant is preserved. Copy count: 3 found by the guard, not 1 per the epic. The third was cmdRoadmapAnalyze's own milestone enumeration (roadmap.cts:454), carrying both defects in the implementation the epic blessed. buildStateFrontmatter and archivePhaseDirectories branch on scope: the first writes null rather than a fabricated identity, the second falls through to its dated-label fallback. A fabricated v3.3 passes ARCHIVE_VERSION_LABEL_RE, so it would otherwise misfile phase history. Also fixes an unsafe cast in init.cts that masked these type errors across five call sites, which would have shipped undefined milestone fields under green tsc. * fix(#3216): restore the #1761 unbounded guard and bullet precedence Review and the first full-matrix run surfaced five real defects in the consolidation, all fixed here rather than by relaxing the tests that caught them: - buildStateFrontmatter gated its isMilestoneBoundedInRoadmap check on the scope-gated milestone value, which is null on any non-COMPLETE scope, so the #1761 unbounded guard was silently skipped and state json reported a percent it must omit. It now gates on the STATE-asserted version, independent of identity scope. - The rewrite lost #2135's precedence: the name-bearing progress-marker bullet is consulted before the heading again. - A single-segment version (v3, no dot) did not resolve; the name-extraction fallback now accepts it. - A version carrying regex metacharacters, or a $& / $1 replacement pattern, is matched literally. - listMilestoneHeadings' heading field trimmed, so a CRLF roadmap no longer leaks a trailing carriage return into roadmap analyze's output. Also emits milestone_version / milestone_name / current_milestone as explicit null rather than omitting the key, so the prompt layer cannot render a bare placeholder, and corrects an init.cts comment plus a cast left inconsistent. * test(#3216): update milestone-identity expectations to the scoped contract getMilestoneInfo returns ScopedResult and the {v1.0,'milestone'} default is deleted, so the suites asserting the old shape assert removed behavior. Updated rather than weakened: every touched call site now asserts the scope explicitly against the frozen SCOPE enum. roadmap-parser.test.cjs: 20 expectations moved to {value,scope}. The #1881 unreadable-vs-absent diagnostic assertions are untouched and still prove their original point — only the return shape moved. One pre-existing assert.ok(info) is now a specific UNSCOPED assertion, so that case is stronger than before. new-milestone-clear-phases.test.cjs: the test asserting phases clear archives under the v1.0 default now asserts the dated archived- fallback, which is the deliberate consequence of deleting that default. Two of this branch's own tests were also corrected after they drove the implementation the wrong way: the parity test compared raw heading text and so pushed a stray ## prefix into roadmap analyze's public output, and the hostile metacharacter row demanded a pathological version resolve, which pushed a widening of the ADR-locked \b boundary. Both now assert what the contract actually requires. * docs(#3216): document milestone identity and correct the CONTEXT.md entry ADR-3180 s7.2 moves to Enforced and gains two rules that were unstated: the name derives from the heading's own version token and drops a trailing status marker, and a free-form legacy ROADMAP with no version anywhere is UNSCOPED with no identity rather than a defaulted v1.0 (decided by the maintainer before implementation, per s7's own rule that an unstated behavior is not decided). Amendment 4 records Phase 6's validation, including that the copy count was a lower bound for the fourth consecutive phase. CONTEXT.md's Roadmap Parser entry described locateMilestoneHeadings as boundary-matched with (?![\w.-]) — the alternative Amendment 2 tried and REVERTED. The code uses \b and says so, and the ADR agrees; the revert updated code and ADR and missed CONTEXT.md, which is the epic's own fixed-on-one-copy failure class in the docs layer, on a file that is itself a PR gate. * fix(#3216): persist the real version on a truncated identity buildStateFrontmatter wrote null for BOTH milestone and milestone_name on any non-COMPLETE scope, discarding a real version. ADR-3180 s7.2 rule 6: a version known with no resolvable name is TRUNCATED carrying {version, name: null} — 'the version is a real answer, the name is a non-answer, and collapsing the two is the failure this contract exists to prevent.' The two fields are now gated by what is actually known: the version whenever one exists (COMPLETE or TRUNCATED), the name only on COMPLETE. Never fabricated. Caught by this phase's own Decision 4(c) consumer-output test, which is the argument for asserting at the consumer rather than the owner — the owner was correct throughout; only the consumer collapsed its answer. * refactor(#3216): extract helpers and make cmdCommit's scope gate explicit From the two-axis code review: - init.cts repeated the identical getMilestoneInfo cast at five sites with copy-pasted comments — duplication inside a PR whose thesis is that duplicates get deleted. Extracted milestoneRecord(cwd); the one site-specific comment is kept, the four generic copies removed. - getMilestoneInfo hand-built its { value, scope } literal at ten return points; a local scoped() constructor now does it once. Every per-branch rationale comment is preserved and no returned value or scope changed. - cmdCommit gated the milestone branch name on plain truthiness, which is also true for TRUNCATED, so an unresolved identity drove branch creation incidentally rather than deliberately. It now gates on the SCOPE enum, accepting COMPLETE or TRUNCATED because both carry a real version, and the comment records why that differs from archivePhaseDirectories — which demands COMPLETE because it uses the value as a filesystem path component. * test(#3216): cover the bare-version-in-prose truncated path The spec review found the bareVersionMatch path — no STATE version, no milestone heading, a version token only in prose — returning TRUNCATED with no test exercising that exact shape, violating Decision 4's boundary-coverage requirement. * docs(#3216): record the missed Tier-2 surfaces and rule 5's corollary Decision 3 requires an explicit call-out for EVERY Tier-2 change, and Amendment 4's first draft named eight surfaces while the change touched thirteen. Adds cmdCommit's branch-name construction and the four init JSON bundles, an incomplete list being the same defect in miniature that this epic removes. s7.2 rule 5 gains a corollary separating two cases the original wording ran together: no version token ANYWHERE is UNSCOPED, while a bare version token in prose or a non-milestone heading is weak but real evidence and yields TRUNCATED under rule 6. * chore(#3216): set changeset fragment pr to 3226 --------- Co-authored-by: sim --- .changeset/tidy-lynx-sing.md | 5 + CONTEXT.md | 2 +- docs/CLI-TOOLS.md | 34 +- docs/COMMANDS.md | 8 + ...80-planning-semantic-model-single-owner.md | 83 +- scripts/lint-milestone-window-drift.cjs | 96 +- scripts/lint-test-file-count.allowlist.json | 3 +- src/commands.cts | 33 +- src/init.cts | 74 +- src/milestone.cts | 8 +- src/roadmap-parser.cts | 283 ++++- src/roadmap.cts | 20 +- src/state.cts | 51 +- src/verify.cts | 4 +- src/workstream.cts | 4 +- tests/milestone-window-drift-guard.test.cjs | 161 +++ tests/milestone-window-single-owner.test.cjs | 1049 +++++++++++++++++ tests/new-milestone-clear-phases.test.cjs | 31 +- tests/roadmap-parser.test.cjs | 81 +- 19 files changed, 1887 insertions(+), 143 deletions(-) create mode 100644 .changeset/tidy-lynx-sing.md create mode 100644 tests/milestone-window-drift-guard.test.cjs diff --git a/.changeset/tidy-lynx-sing.md b/.changeset/tidy-lynx-sing.md new file mode 100644 index 000000000..e2f55aec7 --- /dev/null +++ b/.changeset/tidy-lynx-sing.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3226 +--- +**Milestone names are no longer truncated at a parenthesis, and a phase heading is never mistaken for the milestone** — a ROADMAP whose `### Phase N` heading mentioned a version could cause a wrong `milestone:` to be written to `STATE.md`, and a milestone named `v3.3 — Portability (Windows)` was recorded and rendered as `Portability`. Milestone identity now has one implementation; when it cannot be determined it is reported as absent instead of defaulting to a plausible-looking `v1.0`/`milestone`. (#3216) diff --git a/CONTEXT.md b/CONTEXT.md index e11bf76f9..9a84e2aed 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -181,7 +181,7 @@ Canonical GFM table parsing + schema registry seam (`gsd-core/bin/lib/markdown-t Shared fail-loud `Result` and per-surface write-set contracts (`gsd-core/bin/lib/write-set.cjs`, generated from `src/write-set.cts`; ADR-2143 §5/§6, epic #2143). Pure, Node built-ins only, no I/O. Exports: `Result` (`{ok:true,value}\|{ok:false,reason}` — ADR-2143 §5 fail-loud parse shape, never a bare `null` a caller can mistake for "empty but fine"; the single source of truth `markdown-table.cjs` re-exports so its existing importers are unaffected; deliberately distinct from command-routing-hub's dispatch `Result` `{ok,data\|kind}`); `WriteOutcome` (`{surface: string, applied: boolean}` — one surface's outcome within a multi-surface write); `WriteSet` (`WriteOutcome[]`); `writeSetComplete(ws) → boolean` (true only when the set is non-empty AND every surface applied — ADR-2143 §6's "no OR-into-one-flag" rule: a command that mutates more than one surface must not collapse independent surface outcomes into a single boolean, the anti-pattern that let a checkbox-only partial write (#2140) report full success). `milestone.cts`'s `requirements mark-complete` handler is the first consumer: it reports a `write_set` (`checkbox`/`traceability` surfaces) and `write_set_complete` alongside its existing `updated`/`marked_complete`/`already_complete`/`not_found`/`table_unmatched` fields, which remain computed exactly as before — the write-set is additive, structured ADR-2143 documentation of the same per-surface facts #2140's tactical fix already exposed via `table_unmatched`. ### Roadmap Parser Module -Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`, `isMilestoneShippedInRoadmap`, `withPhaseSection`). Milestone shipped/active heading classification is owned here (#2562): `isMilestoneShippedInRoadmap(content, version)` answers "does the ROADMAP mark THIS milestone shipped" from heading and `` lines only — never a bullet that merely names the version — with the version token boundary-matched so `v2.0` does not match inside `v2.0.1`. `extractCurrentMilestone` and `getMilestonePhaseFilter` take an optional trailing workstream name so their `planningDir` resolution targets `.planning/workstreams//`; omitted, it resolves exactly as before (including the `GSD_WORKSTREAM` fallback). `getMilestonePhaseFilter` exposes `versionScoped`, true only when the returned phase set really is one milestone's — consumers must not read `phaseCount` as a current-milestone denominator otherwise — and `versionSectionFound`, true whenever the requested version's section was located at all. The two differ precisely for a located-but-EMPTY section: it falls through to the zero-count pass-all degrade, which resets `versionScoped` to false, leaving `versionSectionFound` the only surviving evidence that the milestone exists rather than being absent. `missingExplicitVersion` covers the complementary shape (versioned roadmap, no section for this version). `withPhaseSection(content, phaseId, edit)` resolves a phase's `### Phase N` detail-section heading via the #2121 phase-id source (`phaseMarkdownRegexSource`) and delegates to the markdown-sectionizer seam's `withSection`, so a per-phase ROADMAP edit is bounded to that phase's own section (ADR-2143 §4). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`, `markdown-sectionizer`, and — since #1881 — `unusable-input` for the out-of-band diagnostic) — no `loadConfig`, no other core dependency. An unreadable ROADMAP.md is reported rather than collapsed into the same sentinel as a genuinely absent one; absence itself stays silent, and neither lookup gains a throw (the #2245 audit records that `src/state.cts` removed its defensive try/catch on the strength of `getMilestoneInfo` never throwing). Milestone WINDOWING — which headings bound a milestone — is owned here as of #3184 (epic #3180 Phase 2, ADR-3180 Decision 1): `computeMilestoneSectionEnd` (the section-end walk, formerly duplicated as two distinct nested `computeSectionEnd` functions plus an inline third copy in `getMilestonePhaseFilter`'s `versionOverride` branch), `locateMilestoneHeadings` (heading location, version token boundary-matched with `(?![\w.-])` so `v2.0` does not match inside `v2.0.1` — `\b` does, because `.` is a non-word character), `sliceMilestoneWindow` (the one composition of locate → prefer-non-closed → section-end, so a consumer cannot re-assemble its own window from the primitives), and `isMilestoneBoundedInRoadmap` (the named predicate replacing two byte-identical re-derivations in `state.cts`). `extractCurrentMilestoneScoped` is the real extractor and returns the Planning Scope Module's `ScopedResult`; `extractCurrentMilestone` remains a one-line wrapper over `.value` because its blast radius is CRITICAL (200+ affected symbols, 20 direct callers) and its signature must not move. `getMilestonePhaseFilter` gains a `scope` field: its pass-all degrade is PRESERVED where its premise holds (a genuinely-empty, freshly-declared milestone reports `SCOPE.COMPLETE`) and is now labelled where it does not (`SCOPE.TRUNCATED` when the window reached no phase entries while the document has them), so the destructive consumer — `milestone.complete`, which MOVES phase directories — can refuse instead of archiving every phase directory on disk (#3166). The filter's function behavior is deliberately unchanged: making it deny-all on a non-COMPLETE scope would trade a silent over-inclusive answer for a silent under-inclusive one on the read paths that count with it. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`). +Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`, `isMilestoneShippedInRoadmap`, `withPhaseSection`). Milestone shipped/active heading classification is owned here (#2562): `isMilestoneShippedInRoadmap(content, version)` answers "does the ROADMAP mark THIS milestone shipped" from heading and `` lines only — never a bullet that merely names the version — with the version token boundary-matched so `v2.0` does not match inside `v2.0.1`. `extractCurrentMilestone` and `getMilestonePhaseFilter` take an optional trailing workstream name so their `planningDir` resolution targets `.planning/workstreams//`; omitted, it resolves exactly as before (including the `GSD_WORKSTREAM` fallback). `getMilestonePhaseFilter` exposes `versionScoped`, true only when the returned phase set really is one milestone's — consumers must not read `phaseCount` as a current-milestone denominator otherwise — and `versionSectionFound`, true whenever the requested version's section was located at all. The two differ precisely for a located-but-EMPTY section: it falls through to the zero-count pass-all degrade, which resets `versionScoped` to false, leaving `versionSectionFound` the only surviving evidence that the milestone exists rather than being absent. `missingExplicitVersion` covers the complementary shape (versioned roadmap, no section for this version). `withPhaseSection(content, phaseId, edit)` resolves a phase's `### Phase N` detail-section heading via the #2121 phase-id source (`phaseMarkdownRegexSource`) and delegates to the markdown-sectionizer seam's `withSection`, so a per-phase ROADMAP edit is bounded to that phase's own section (ADR-2143 §4). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`, `markdown-sectionizer`, and — since #1881 — `unusable-input` for the out-of-band diagnostic) — no `loadConfig`, no other core dependency. An unreadable ROADMAP.md is reported rather than collapsed into the same sentinel as a genuinely absent one; absence itself stays silent, and neither lookup gains a throw (the #2245 audit records that `src/state.cts` removed its defensive try/catch on the strength of `getMilestoneInfo` never throwing). Milestone WINDOWING — which headings bound a milestone — is owned here as of #3184 (epic #3180 Phase 2, ADR-3180 Decision 1): `computeMilestoneSectionEnd` (the section-end walk, formerly duplicated as two distinct nested `computeSectionEnd` functions plus an inline third copy in `getMilestonePhaseFilter`'s `versionOverride` branch), `locateMilestoneHeadings` (heading location, version token boundary-matched with `\b`, **NOT** the stricter `(?![\w.-])`: `v2.0` therefore DOES match inside `v2.0.1`, and a milestone STATE of `v8.0` legitimately selects a live `## v8.0-B …` over a closed `v8.0-A` sibling — deliberate, load-bearing #730 behavior that ADR-3180 Amendment 2 tried to tighten and then reverted; the earlier text here described that reverted alternative as if it had shipped, corrected by #3216), `listMilestoneHeadings` (#3216 — the version-AGNOSTIC enumeration of every milestone heading in document order, sharing ONE grammar source with `locateMilestoneHeadings` so the two cannot drift; `locateMilestoneHeadings` is now a version-filtered view over it rather than a second expression of the pattern). Milestone IDENTITY — which milestone is current and what it is CALLED — is owned by `getMilestoneInfo`, which since #3216 binds to that same locator instead of its own heading regexes and returns a `ScopedResult`: a name retains parentheses and drops a trailing `✅`/`📋`/`🚧` marker, a `### Phase N …` heading is never the milestone heading (#3197), and an identity that cannot be determined returns a non-`COMPLETE` scope rather than the former `{version:'v1.0', name:'milestone'}` default, which was output-identical to a successful read of a genuine v1.0 project. `buildStateFrontmatter` and `archivePhaseDirectories` branch on that scope, so a fabricated identity is never persisted to `STATE.md` nor used as a `milestones/-phases/` path component, `sliceMilestoneWindow` (the one composition of locate → prefer-non-closed → section-end, so a consumer cannot re-assemble its own window from the primitives), and `isMilestoneBoundedInRoadmap` (the named predicate replacing two byte-identical re-derivations in `state.cts`). `extractCurrentMilestoneScoped` is the real extractor and returns the Planning Scope Module's `ScopedResult`; `extractCurrentMilestone` remains a one-line wrapper over `.value` because its blast radius is CRITICAL (200+ affected symbols, 20 direct callers) and its signature must not move. `getMilestonePhaseFilter` gains a `scope` field: its pass-all degrade is PRESERVED where its premise holds (a genuinely-empty, freshly-declared milestone reports `SCOPE.COMPLETE`) and is now labelled where it does not (`SCOPE.TRUNCATED` when the window reached no phase entries while the document has them), so the destructive consumer — `milestone.complete`, which MOVES phase directories — can refuse instead of archiving every phase directory on disk (#3166). The filter's function behavior is deliberately unchanged: making it deny-all on a non-COMPLETE scope would trade a silent over-inclusive answer for a silent under-inclusive one on the read paths that count with it. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`). ### Core Utilities Module Module owning the shared low-level utility primitives extracted from Core: POSIX path normalization (`toPosixPath`), filesystem scanning (`detectSubRepos`, `readSubdirectories`, `getPhaseFileStats`, `pathExistsInternal`), plan/summary pairing helpers (`countMatchedSummaries`, `findUnsummarizedPlans`, `findOrphanSummaries`), and small pure helpers (`generateSlugInternal`, `extractOneLinerFromBody`, `extractCanonicalPlanId`, `timeAgo`). `filterPlanFiles`/`filterSummaryFiles` were retired by #3183 (ADR-3180 Decision 2) — `getPhaseFileStats` no longer re-derives plan/summary filename matching locally; it now sources `plans`/`summaries` (plus a `scope` field, `COMPLETE`/`TRUNCATED`/`UNREADABLE`) directly from `scanPhasePlans` (`src/plan-scan.cts`), the single owner of live-plan counting. Depends only on Node built-ins and already-leafed modules (`phase-id` for `comparePhaseNum`, `planning-workspace` for `findContextMdIn`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2c (#877) as the shared leaf that unblocks the phase-locator fs-search extraction (2d); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/core-utils.cjs` (generated from `src/core-utils.cts`). diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 9e584f39c..f71eabc1f 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -226,7 +226,39 @@ Before this field existed, all four cases produced the same well-formed milestone from a scoping failure. Branch on `scope`, not on `phase_count` alone. A ROADMAP with no versioned milestone headings at all (the free-form legacy -shape) reports `complete`: the whole document *is* the milestone there. +shape) reports `complete`: the whole document *is* the milestone there. Note +this answer is specific to *windowing* — see the next section for why milestone +*identity* answers the same document differently. + +### Milestone identity (which milestone, and what it is called) + +Milestone identity — the version and name behind `STATE.md`'s `milestone:` +field, `roadmap analyze`'s `milestones[]` array, and the milestone shown by +`query progress`, `stats`, `init manager`, `validate health` and +`workstream create` — is resolved by one implementation: + +- `STATE.md`'s `milestone:` field selects the version when present. The ROADMAP + heuristics are the fallback, not the primary. +- The heading is located by the same canonical locator that computes the + milestone window, so a `### Phase N: …` heading is **never** read as the + milestone heading — even when it mentions a version. Previously a ROADMAP + whose phase heading preceded its milestone heading could write a wrong + `milestone:` to disk. +- The **name** is the heading text after that heading's own version token, with + one leading delimiter (`—`, `–`, `:`, `-`) and any trailing `✅`/`📋`/`🚧` + marker removed. Parentheses are ordinary characters: a milestone named + `v3.3 — Portability (Windows)` keeps its full name rather than being cut at + the `(`. +- When identity **cannot** be determined it is reported as absent rather than + defaulted. A free-form legacy ROADMAP with no version anywhere is `unscoped` + with no identity — unlike windowing above, there is no version token to + report, and inventing one would be indistinguishable from a real answer. + +Two consumers act on that distinction rather than just displaying it: +`state sync` / `state record-session` write `null` instead of a fabricated +`milestone:`/name, and `phases clear` falls back to its dated archive label +(`archived-`) instead of filing phase history under a fabricated +`milestones/-phases/` directory. ### `milestone complete` refuses an untrustworthy window diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 5e6a40f36..95a4fe9bb 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -656,6 +656,14 @@ Show status, next steps, and automatically advance to the next logical workflow | `--do "task description"` | Analyze freeform intent and dispatch to the most appropriate GSD command | | `--forensic` | Append a 6-check integrity audit after the standard report (STATE consistency, orphaned handoffs, deferred scope drift, memory-flagged pending work, blocking todos, uncommitted code) | +> **Milestone name and version.** The milestone this report shows comes from one +> implementation shared with `/gsd-stats`, `/gsd-manager` and `roadmap analyze`. +> A name is no longer cut short at a parenthesis (`v3.3 — Portability (Windows)` +> keeps its full name), a `### Phase N:` heading that mentions a version is never +> mistaken for the milestone heading, and a milestone that cannot be identified is +> shown as absent rather than as a plausible-looking `v1.0`/`milestone`. See +> [CLI-TOOLS.md → Milestone identity](CLI-TOOLS.md#milestone-identity-which-milestone-and-what-it-is-called). + **Auto-routing behavior (`--next`):** - No project → suggests `/gsd-new-project` - Phase needs discussion → runs `/gsd-discuss-phase` diff --git a/docs/adr/3180-planning-semantic-model-single-owner.md b/docs/adr/3180-planning-semantic-model-single-owner.md index da703462b..37fc7af10 100644 --- a/docs/adr/3180-planning-semantic-model-single-owner.md +++ b/docs/adr/3180-planning-semantic-model-single-owner.md @@ -244,19 +244,21 @@ This section is that written rule. It is **normative**, and it is what the guard **Guard.** `scripts/lint-milestone-window-drift.cjs`. -#### 7.2 Milestone identity — *Required — Phase 6* +#### 7.2 Milestone identity — *Enforced (Phase 6, #3216)* **Question.** Which milestone is current, and what is it called? -**Owner (to be).** `getMilestoneInfo` binds to `locateMilestoneHeadings` and deletes its own heading regexes. It is a **sixth derivation family** — the coverage audit's gap 2 — that no phase of the original decomposition touches. +**Owner.** `src/roadmap-parser.cts` — `getMilestoneInfo`, returning `ScopedResult`, plus the version-agnostic `listMilestoneHeadings` added by Phase 6 for consumers that need *every* milestone heading rather than one. Both consume a single shared grammar source; `locateMilestoneHeadings` is a **version-filtered view** over that source, not a second expression of it. It is a **sixth derivation family** — the coverage audit's gap 2 — that no phase of the original decomposition touched. **Rule.** 1. `STATE.md`'s `milestone:` field selects the version when present; the ROADMAP heuristics are the fallback, not the primary. 2. The heading is located by the canonical locator of §7.1, which already excludes phase headings. A `### Phase N: Close v3.3 gaps` heading is **never** the milestone heading (#3197 — reproduced live, writing a wrong `milestone:` to disk). -3. The **name** is the heading text following the version token with a leading delimiter (`—`, `–`, `:`, `-`) stripped. `(` is an ordinary name character: the name is **not** truncated at a parenthetical (#3171). +3. The **name** derives from the heading's **own** version token, not the requested one: remove everything up to and including that token, strip one leading delimiter (`—`, `–`, `:`, `-`), then strip any trailing run of the status markers `✅`/`📋`/`🚧`. `(` is an ordinary name character: the name is **not** truncated at a parenthetical (#3171). *(Requested `v2.0` against heading `## v2.0.1 — Portability` yields exactly `Portability`, not `.1 — Portability`; `## v2.0 — Old ✅` yields `Old`, because the shipped state is already carried structurally and must not be duplicated into the name.)* 4. A failure returns a `scope` other than `COMPLETE`. It does **not** return `{version: 'v1.0', name: 'milestone'}` presented as an answer — that default is output-identical to a successful read of a genuine `v1.0` project, which is this epic's defining failure mode. +5. **A free-form legacy ROADMAP carrying no version token anywhere is `UNSCOPED` with no identity** — *not* `COMPLETE`, and not a defaulted `v1.0`. §7.1's "free-form is `COMPLETE`" governs *windowing*, where whole-document genuinely is the window; identity has no version to report and must not invent one. **Decided 2026-08-08** (maintainer), closing the gap this section previously left unstated. **Corollary, stated because it is a distinct case and was initially left implicit:** a bare version token appearing only in prose or in a non-milestone heading — no `milestone:` field, no milestone heading — is weak but *real* evidence, and yields `TRUNCATED` with that version and a `null` name under rule 6, not `UNSCOPED`. `UNSCOPED` is reserved for a document carrying no version token at all. +6. A version known but no name resolvable is `TRUNCATED` carrying `{version, name: null}` — the version is a real answer, the name is a non-answer, and collapsing the two is the failure this contract exists to prevent. -**Guard.** `lint-milestone-window-drift.cjs` today keys on the `#{N,M}` heading-level quantifier; `getMilestoneInfo`'s regexes anchor on a literal `##` and therefore slip past it. **Phase 6 ships the token widening together with the consolidation**, never after — a guard added later measures a surface already cleaned and reports a zero it did not earn. +**Guard.** `lint-milestone-window-drift.cjs`, token set widened by Phase 6 in the same change as the consolidation — never after, since a guard added later measures a surface already cleaned and reports a zero it did not earn. Token (a) now admits a **literal** `#`-run heading anchor in addition to the `#{N,M}` quantifier, but only inside a heading-**matcher** literal (a regex literal, or a string handed to `new RegExp(`), so a heading-**builder** template is not mistaken for a re-derivation. Token (b) additionally admits the grouped `v(\d+(?:\.\d+)+)` shape and an interpolated `${…Ver…}` placeholder. #### 7.3 Phase enumeration — *Enforced for the four named consumers (Phase 3, #3222); the fifth copy is unowned* @@ -369,7 +371,7 @@ One row per derivation. A blank owner is a derivation whose contract is locked ( | Derivation | Owner | Guard | Scan surface | Status | |---|---|---|---|---| | Milestone windowing (§7.1) | `roadmap-parser.cts` | `lint-milestone-window-drift.cjs` | `src/` | enforced | -| Milestone identity (§7.2) | — (Phase 6) | same guard, token set widened by Phase 6 | `src/` | contract only | +| Milestone identity (§7.2) | `roadmap-parser.cts` (Phase 6) | same guard, token set widened by Phase 6 | `src/` | enforced | | Phase enumeration (§7.3) | `phase-locator.cts` | `lint-phase-enumeration-drift.cjs` | `src/` | enforced for the four named consumers; `buildStateFrontmatter`/`syncStateFrontmatter` **unowned** | | Phase completion (§7.4) | `verification.cts` (Phase 4) | Phase 4 | `src/` | blocked on #2957 | | Live-plan counting (§7.5) | `plan-scan.cts` | `lint-plan-count-drift.cjs` | `src/` | enforced | @@ -662,3 +664,74 @@ underlying document-layout ambiguity is untouched by design. #3163 belongs to #2 layer and is not enumerated there yet; #3169 and #3170 are standalone parser/extraction defects with no epic home. Recording them here as *not covered* rather than leaving them to be re-tested by the next audit. + +### Amendment 4 — Phase 6 (#3216) validation: the contract held; the blessed implementation carried the defect again + +**The copy count was a lower bound for the FOURTH consecutive time.** The epic and §7.2 both scoped +this phase at one copy — `getMilestoneInfo`. The guard, built and run *before* the scope was fixed +per Amendment 3's standing rule, found **three**: + +| # | Site | Carries | +|---|---|---| +| 1 | `roadmap-parser.cts:785` — `^##[^\n]*${escapedVer}…` | #3171 + #3197 | +| 2 | `roadmap-parser.cts:806` — `/## (?!.*✅).*v(\d+(?:\.\d+)+)…/` | #3171 + #3197 | +| 3 | **`roadmap.cts:454`** — `cmdRoadmapAnalyze`'s own milestone enumeration | #3171 + #3197 | + +Site 3 is the notable one, and it repeats §7.6's finding exactly: **the implementation this epic +blessed carried the defect it was blessed over.** `cmdRoadmapAnalyze` is named in Decision 1 as the +correct enumeration implementation, and its `milestones[]` array was simultaneously truncating names +at a parenthetical and emitting `### Phase N` headings as milestones. Twice now the blessing has +been granted per-question rather than per-file, and twice the blessed file has held an unrelated +copy. **Blessing an implementation for one derivation says nothing about its others.** + +**Two mechanisms, not one.** Site 1 is line-anchored but level-blind (`^##` then `[^\n]*` absorbs a +third `#`); site 2 and site 3 have no anchor at all, so `## ` matches from the *second* `#` of +`###`. A reviewer checking "is it anchored?" would have passed site 1. This is why §7.2 rule 2 names +the canonical locator rather than describing the anchoring to be re-implemented. + +**Two under-specifications surfaced during implementation, both now closed in §7.2 rule 3.** Neither +was reachable by reading the contract alone; both appeared only when tests demanded an exact value. +(a) *Which* version token the name is measured from, when the requested version is a prefix of the +heading's own — `v2.0` against `## v2.0.1 — Portability` left `.1 — Portability` under the original +wording. (b) A trailing `✅` was being retained *in the name*, duplicating structural state into a +string that `buildStateFrontmatter` writes to disk. Both are now stated normatively rather than +settled inside the implementation. + +**§7.2's unstated case is now decided (rule 5).** Free-form legacy ROADMAP with no version anywhere +→ `UNSCOPED`, no identity. §7's own rule — *"a behavior not stated here is not decided"* — held: the +gap was raised and decided by the maintainer before implementation rather than resolved silently. + +**A latent defect was found in a caller, not by the guard.** `init.cts` carried +`getMilestoneInfo(cwd) as unknown as Record`. The cast masked the return-type change +across five call sites, so `tsc` stayed green while every one of them would have read `undefined`, +and one template would have rendered the literal string `"undefined"`. A structural drift guard +cannot see this class — an unsafe cast is not a re-derivation — which is the concrete argument for +Decision 4(b)'s pairing: the type checker was the output metric and it was green; migrating and +running the consumers was the outcome metric. + +**Tier-2 output changes (Decision 3).** `state sync` / `state record-session` persist a corrected +`milestone:` and name, or `null` rather than a fabricated identity; `roadmap analyze`'s `milestones[]` +no longer truncates names and no longer lists phase headings; `phases clear` falls back to its dated +archive label rather than misfiling under a fabricated version; `query progress`, `stats`, +`init manager`, `validate health` and `workstream create` render the full name. + +Five further Tier-2 surfaces were missed in this amendment's first draft and are recorded here after +the Phase 6 spec review caught the omission — Decision 3 requires *every* Tier-2 change be called +out, and an incomplete list is the same defect in miniature that this epic exists to remove: + +- **`commit`** (`cmdCommit`) — when `branching_strategy` is `milestone`, the constructed branch name + now derives from a scope-checked identity. A `COMPLETE` identity produces the same branch name as + before; a `TRUNCATED` one (real version, unresolved name) still produces a branch, deliberately, + because the version is real — the acceptance is now explicit in code rather than incidental to a + truthiness check. Contrast `archivePhaseDirectories`, which demands `COMPLETE` because it uses the + value as a filesystem path component. +- **The four `init` JSON bundles** — `init execute-phase`, `init new-milestone`, `init milestone-op` + and `init progress` — emit `milestone_version` / `milestone_name` / `current_milestone` as an + explicit `null` when identity is unavailable, where they previously carried the fabricated + `v1.0` / `milestone`. The keys are always PRESENT so the prompt layer cannot render a bare + placeholder; `JSON.stringify` had been dropping them when the value went `undefined`. + +**Guard.** The owner file stays scanned; `listMilestoneHeadings` joins `FUNCTION_SCOPED_EXEMPTIONS` +as a *named canonical function* — the only sanctioned exemption form. `locateMilestoneHeadings` was +refactored into a version-filtered view over one shared grammar source so the two primitives cannot +drift, with a parity test asserting the filtered enumeration equals the locator's selection. diff --git a/scripts/lint-milestone-window-drift.cjs b/scripts/lint-milestone-window-drift.cjs index d5474838f..7d54c0c0d 100644 --- a/scripts/lint-milestone-window-drift.cjs +++ b/scripts/lint-milestone-window-drift.cjs @@ -129,6 +129,41 @@ const VERSION_TOKEN_RE = /v\(?\\{1,2}d\+\)?\\{1,2}\.\\{1,2}d\+/; // prose-matching code); together, on one line, they are the boundary shape. const MARKER_EMOJI_RE = /[✅📋🚧]/u; +// (a-bis) #3216: a LITERAL markdown heading anchor — `##`, `###`, … — as +// opposed to the `#{N,M}` quantifier token (a) above. `getMilestoneInfo` +// hand-rolled its milestone-heading match with a literal `^##` / `## ` rather +// than a quantifier, so token (a) alone reported a clean zero on a file that +// carried two live re-derivations (#3171, #3197). The negative lookahead for +// `{` keeps this from double-matching the quantifier form. +// +// A `#` run is far more common in source than `#{N,M}` (private-field sigils, +// colour literals, fragment URLs, prose), so this token is admitted ONLY +// inside a heading-MATCHER literal — a regex literal, or a string/template +// literal handed to `new RegExp(` — never a bare line match. A template that +// BUILDS a heading for output (`## ${version}`) is not a re-derivation of +// where a milestone's section begins, and conflating the two would flag every +// heading writer in the tree. +const LITERAL_HEADING_RUN_RE = /#{2,6}(?!\{)/; + +// A line that constructs a regex from a string/template literal, which is what +// admits the `new RegExp(`^##…${escapedVer}…`)` shape while leaving ordinary +// heading-building templates alone. +const NEW_REGEXP_RE = /new\s+RegExp\s*\(/; + +// (b2-bis) #3216: the `v(\d+(?:\.\d+)+)` version shape — a capturing group +// around the major, then a NON-capturing `(?:\.\d+)+` repeat. VERSION_TOKEN_RE +// cannot see it: after `v(` + `\d+` it requires a backslash next, and this +// shape has `(` there instead. +const VERSION_TOKEN_GROUPED_RE = /v\(\\{1,2}d\+\(\?:\\{1,2}\.\\{1,2}d\+\)\+\)/; + +// (b3) #3216: an INTERPOLATED version placeholder — `${escapedVer}`, +// `${escapedVersion}`, `${version}`. A regex that interpolates its version +// spells no literal `v\d+\.\d+` anywhere, so (b) could never fire. Inside a +// heading-matcher literal, "a heading anchor plus an interpolated version" IS +// the milestone-heading shape the canonical `locateMilestoneHeadings` +// composes — and so is a copy of it. +const INTERPOLATED_VERSION_RE = /\$\{[A-Za-z0-9_.]*[Vv]er[A-Za-z0-9_.]*\}/; + // Authored TypeScript source only (the generated bin/lib/*.cjs mirror it). const SCAN_DIRS = ['src']; const SCAN_EXT = new Set(['.cts', '.ts', '.mts']); @@ -192,12 +227,26 @@ const OWNER_FILE = path.join('src', 'roadmap-parser.cts'); // `(?!Phase...)`/marker alternations to find "the next milestone // boundary" while assembling the current-milestone window) — not // re-derivations of a question answered elsewhere. +// - roadmap-parser.cts listMilestoneHeadings: #3216 (epic #3180 §7.2's +// Scope amendment) — the version-AGNOSTIC sibling of +// `locateMilestoneHeadings`, and the function that textually DEFINES +// `MILESTONE_HEADING_LINE_SOURCE` (the one shared grammar constant both +// it and `locateMilestoneHeadings` build their pattern from) in its own +// source span. It is a named canonical function defining the grammar, +// not a copy of it — replacing the third independent re-derivation the +// widened guard found at `roadmap.cts:454`. const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'roadmap-command-router.cts'), new Set(['checkW021'])], [path.join('src', 'verify.cts'), new Set(['checkMilestonePrefixMismatches'])], [ OWNER_FILE, - new Set(['isMilestoneShippedInRoadmap', 'locateMilestoneHeadings', 'hasMilestoneSectioning', 'extractCurrentMilestoneScoped']), + new Set([ + 'isMilestoneShippedInRoadmap', + 'locateMilestoneHeadings', + 'listMilestoneHeadings', + 'hasMilestoneSectioning', + 'extractCurrentMilestoneScoped', + ]), ], ]); @@ -273,6 +322,33 @@ function stripComments(line) { * text — comment-stripping is applied only to the detection decision, never * to the reported fragment. */ +/** + * All heading-MATCHER literals on `line` — a regex literal always counts; a + * quoted/backtick string literal counts only when `code` (the comment-stripped + * line passed in from the caller) constructs a regex via `new RegExp(`. A + * plain string/template literal that is not fed to `new RegExp(` is not a + * matcher — most commonly a heading BUILT for output, not one matched against. + */ +function headingMatcherLiterals(line, code) { + const out = []; + const allowStrings = NEW_REGEXP_RE.test(code); + for (let i = 0; i < line.length; i++) { + const ch = line[i]; + let literal = null; + let isRegex = false; + if (ch === '/') { + literal = readRegexLiteralAt(line, i); + isRegex = true; + } else if (ch === "'" || ch === '"' || ch === '`') { + literal = readStringLiteralAt(line, i); + } + if (!literal) continue; + if (isRegex || allowStrings) out.push(literal.text); + i = literal.end - 1; + } + return out; +} + function extractFragment(line) { for (let i = 0; i < line.length; i++) { const ch = line[i]; @@ -280,7 +356,7 @@ function extractFragment(line) { if (ch === '/') literal = readRegexLiteralAt(line, i); else if (ch === "'" || ch === '"' || ch === '`') literal = readStringLiteralAt(line, i); if (!literal) continue; - if (HEADING_QUANTIFIER_RE.test(literal.text)) return literal.text; + if (HEADING_QUANTIFIER_RE.test(literal.text) || LITERAL_HEADING_RUN_RE.test(literal.text)) return literal.text; i = literal.end - 1; // resume scanning just past this literal } return line.trim().slice(0, MAX_REGEX_LITERAL_LEN); @@ -305,8 +381,16 @@ function findMilestoneWindowDrift(text, relPath) { const code = stripComments(line); if (!code.trim()) continue; - if (!HEADING_QUANTIFIER_RE.test(code)) continue; - const isMilestoneWindowToken = PHASE_LOOKAHEAD_RE.test(code) || (VERSION_TOKEN_RE.test(code) && MARKER_EMOJI_RE.test(code)); + const matcherLiterals = headingMatcherLiterals(line, code); + const hasQuantifier = HEADING_QUANTIFIER_RE.test(code); + const hasLiteralHeading = matcherLiterals.some((t) => LITERAL_HEADING_RUN_RE.test(t)); + if (!hasQuantifier && !hasLiteralHeading) continue; + + const anyVersionToken = VERSION_TOKEN_RE.test(code) || VERSION_TOKEN_GROUPED_RE.test(code); + const isMilestoneWindowToken = + PHASE_LOOKAHEAD_RE.test(code) + || (anyVersionToken && MARKER_EMOJI_RE.test(code)) + || (hasLiteralHeading && (anyVersionToken || INTERPOLATED_VERSION_RE.test(code))); if (!isMilestoneWindowToken) continue; if (exemptFunctions && exemptFunctions.has(currentFunction)) continue; @@ -372,4 +456,8 @@ module.exports = { readStringLiteralAt, extractFragment, stripComments, + LITERAL_HEADING_RUN_RE, + VERSION_TOKEN_GROUPED_RE, + INTERPOLATED_VERSION_RE, + headingMatcherLiterals, }; diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index dcef479a6..9ba37d9ee 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -37,10 +37,11 @@ "milestone-helper.test.cjs", "milestone-prefixed-convention.test.cjs", "milestone-summary.test.cjs", + "milestone-window-drift-guard.test.cjs", "milestone-window-single-owner.test.cjs", "milestone.test.cjs" ], - "issue": "TBD" + "issue": "3216" }, "roadmap": { "files": [ diff --git a/src/commands.cts b/src/commands.cts index 5054e99bb..defed4981 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -29,6 +29,9 @@ const { getArchivedPhaseDirs, findPhaseInternal, listMilestonePhaseDirs } = phas import roadmapParserMod = require('./roadmap-parser.cjs'); const { extractCurrentMilestone, stripShippedMilestones: _stripShippedMilestones, getMilestoneInfo, getRoadmapPhaseInternal } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports +import planningScopeMod = require('./planning-scope.cjs'); +const { SCOPE } = planningScopeMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports import modelResolverMod = require('./model-resolver.cjs'); const { resolveModelInternal, resolveModelForTier, resolveProviderEscalation, resolveEffortInternal, resolveFastModeInternal, resolveEffortForTier, resolveGranularityInternal, assertValidGranularityOverride } = modelResolverMod; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -864,7 +867,19 @@ function cmdCommit(cwd: string, message: string | undefined, files: string[] | u } } } else if (branchingStrategy === 'milestone') { - const milestone = getMilestoneInfo(cwd); + const milestoneInfo = getMilestoneInfo(cwd); + // #3216 review Finding 3: explicit scope gate instead of plain truthiness. + // COMPLETE and TRUNCATED both carry a real `version` (ADR-3180 §7.2 rule + // 6 — TRUNCATED means the version resolved but the milestone's NAME did + // not), so a TRUNCATED identity is acceptable here: `milestone.version` + // only feeds a BRANCH NAME, and `generateSlugInternal(null) || 'milestone'` + // already degrades the missing name to the literal "milestone" slug on + // purpose. This differs from `archivePhaseDirectories` (milestone.cts), + // which uses the same value as a DIRECTORY NAME and therefore demands + // COMPLETE only — a real-but-unnamed version is not safe enough there. + const milestone = milestoneInfo.scope === SCOPE.COMPLETE || milestoneInfo.scope === SCOPE.TRUNCATED + ? milestoneInfo.value + : null; if (milestone && milestone.version) { branchName = (config['milestone_branch_template'] as string) .replace('{milestone}', milestone.version) @@ -1561,7 +1576,7 @@ async function cmdWebsearch(query: string | undefined, options: WebsearchOptions function cmdProgressRender(cwd: string, format: string | undefined, raw: boolean): void { const phasesDir = planningPaths(cwd).phases; - const milestone = getMilestoneInfo(cwd); + const milestone = getMilestoneInfo(cwd).value; const phases: PhaseProgress[] = []; let totalPlans = 0; @@ -1603,7 +1618,7 @@ function cmdProgressRender(cwd: string, format: string | undefined, raw: boolean const barWidth = 10; const filled = Math.round((percent / 100) * barWidth); const bar = '█'.repeat(filled) + '░'.repeat(barWidth - filled); - let out = `# ${milestone.version} ${milestone.name}\n\n`; + let out = `# ${milestone?.version ?? ''} ${milestone?.name ?? ''}\n\n`; out += `**Progress:** [${bar}] ${totalSummaries}/${totalPlans} plans (${percent}%)\n\n`; out += `| Phase | Name | Plans | Status |\n`; out += `|-------|------|-------|--------|\n`; @@ -1620,8 +1635,8 @@ function cmdProgressRender(cwd: string, format: string | undefined, raw: boolean } else { // JSON format output({ - milestone_version: milestone.version, - milestone_name: milestone.name, + milestone_version: milestone?.version ?? null, + milestone_name: milestone?.name ?? null, phases, total_plans: totalPlans, total_summaries: totalSummaries, @@ -1865,7 +1880,7 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { const roadmapPath = planningPaths(cwd).roadmap; const reqPath = planningPaths(cwd).requirements; const statePath = planningPaths(cwd).state; - const milestone = getMilestoneInfo(cwd); + const milestone = getMilestoneInfo(cwd).value; // Phase & plan stats (reuse progress pattern) const phasesByNumber = new Map 0) { out += `**Plans:** ${totalSummaries}/${totalPlans} complete (${planPercent}%)\n`; diff --git a/src/init.cts b/src/init.cts index e449d05c7..3fa3625a6 100644 --- a/src/init.cts +++ b/src/init.cts @@ -813,6 +813,17 @@ function buildSectionManifestField( } } +/** + * #3216 review Finding 1: `getMilestoneInfo(cwd).value` unwrap-and-cast was + * repeated identically (comment included) at five init call sites — factored + * out once so the cast and its `?? {}` "no milestone resolved" fallback live + * in exactly one place. Behavior-preserving: same call, same fallback, same + * cast, for every caller. + */ +function milestoneRecord(cwd: string): Record { + return (getMilestoneInfo(cwd).value ?? {}) as unknown as Record; +} + function cmdInitExecutePhase( cwd: string, phase: string, @@ -825,7 +836,16 @@ function cmdInitExecutePhase( const config = loadConfig(cwd); let phaseInfo = guardedFindPhase(cwd, phase, config.project_code); - const milestone = getMilestoneInfo(cwd) as unknown as Record; + // #3216: getMilestoneInfo now returns a ScopedResult — `.value` carries the + // MilestoneInfo (or null on any non-COMPLETE scope). NOT display-only: when + // `branching_strategy === 'milestone'`, `milestone['version']`/`['name']` + // below feed `branch_name` construction (see the milestone_branch_template + // branch below), so an unresolved milestone changes the constructed branch + // name, not merely what gets printed. bracket-access below naturally reads + // `undefined` when unresolved; the `milestone_version`/`milestone_name` + // output fields below coerce that to an explicit `null` (#3216 review + // Finding 2) so the key is never silently omitted from the JSON bundle. + const milestone = milestoneRecord(cwd); const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); phaseInfo = applyRoadmapFallback(phaseInfo, roadmapPhase, (rp) => { @@ -905,16 +925,16 @@ function cmdInitExecutePhase( .replace('{slug}', (phaseInfo['phase_slug'] as string) || 'phase') : config.branching_strategy === 'milestone' ? (config.milestone_branch_template as string) - .replace('{milestone}', milestone['version'] as string) + .replace('{milestone}', (milestone['version'] as string | undefined) ?? '') .replace( '{slug}', - generateSlugInternal(milestone['name'] as string) || 'milestone', + generateSlugInternal(milestone['name'] as string | undefined) || 'milestone', ) : null, - milestone_version: milestone['version'], - milestone_name: milestone['name'], - milestone_slug: generateSlugInternal(milestone['name'] as string), + milestone_version: milestone['version'] ?? null, + milestone_name: milestone['name'] ?? null, + milestone_slug: generateSlugInternal(milestone['name'] as string | undefined), state_exists: fs.existsSync(path.join(planningDir(cwd), 'STATE.md')), roadmap_exists: fs.existsSync(path.join(planningDir(cwd), 'ROADMAP.md')), @@ -1208,7 +1228,7 @@ function cmdInitNewProject(cwd: string, raw: boolean, options: Record = {}): void { const config = loadConfig(cwd); - const milestone = getMilestoneInfo(cwd) as unknown as Record; + const milestone = milestoneRecord(cwd); const latestCompleted = getLatestCompletedMilestone(cwd); const phasesDir = path.join(planningDir(cwd), 'phases'); // #3185 (ADR-3180 Decision 1): "how many phase directories belong to the @@ -1227,8 +1247,12 @@ function cmdInitNewMilestone(cwd: string, raw: boolean, options: Record; + const milestone = milestoneRecord(cwd); let phaseCount = 0; let completedPhases = 0; @@ -2076,9 +2100,13 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const result: Record = { commit_docs: config.commit_docs, - milestone_version: milestone['version'], - milestone_name: milestone['name'], - milestone_slug: generateSlugInternal(milestone['name'] as string), + // #3216 review Finding 2: `?? null` so an unresolved milestone still emits + // the key with an explicit `null` rather than letting JSON.stringify drop + // it — an omitted key reaches the prompt layer's `{milestone_version}` + // placeholder as literal, un-substituted text. + milestone_version: milestone['version'] ?? null, + milestone_name: milestone['name'] ?? null, + milestone_slug: generateSlugInternal(milestone['name'] as string | undefined), phase_count: phaseCount, completed_phases: completedPhases, @@ -2134,7 +2162,7 @@ function cmdInitMapCodebase(cwd: string, raw: boolean): void { function cmdInitManager(cwd: string, raw: boolean): void { const config = loadConfig(cwd); - const milestone = getMilestoneInfo(cwd) as unknown as Record; + const milestone = milestoneRecord(cwd); const _slashRuntime = resolveRuntime(cwd); const paths = planningPaths(cwd); @@ -2476,8 +2504,12 @@ function cmdInitManager(cwd: string, raw: boolean): void { }; const result: Record = { - milestone_version: milestone['version'], - milestone_name: milestone['name'], + // #3216 review Finding 2: `?? null` so an unresolved milestone still emits + // the key with an explicit `null` rather than letting JSON.stringify drop + // it — an omitted key reaches the prompt layer's `{milestone_version}` + // placeholder as literal, un-substituted text. + milestone_version: milestone['version'] ?? null, + milestone_name: milestone['name'] ?? null, phases, phase_count: phases.length, completed_count: completedCount, @@ -2751,7 +2783,7 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record; + const milestone = milestoneRecord(cwd); const _slashRuntime = resolveRuntime(cwd); // #1912: fail safe in workstream mode with no active workstream. With no active @@ -2937,8 +2969,12 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record { + const pattern = new RegExp(MILESTONE_HEADING_LINE_SOURCE, 'gmi'); + const out: Array<{ heading: string; version: string; name: string | null; closed: boolean }> = []; + let m: RegExpExecArray | null; + while ((m = pattern.exec(content)) !== null) { + // #3216 review (Finding 4): the shared grammar's `[^\n]*` captures a + // trailing `\r` on a CRLF-encoded ROADMAP (the inline `cmdRoadmapAnalyze` + // regex this replaced called `.trim()`; this did not). `.trim()` here + // matches that prior behavior. `version` (digits/dots/letters only, via + // `extractMilestoneHeadingName`'s regex) and `name` (already run through + // `stripLeadingDelimiter`, which ends in `.trim()`) cannot carry a + // trailing `\r`, so only `heading` needs the fix. + // + // `heading` carries the heading text WITHOUT the leading `#{1,3}` run and + // its following whitespace — matching the inline `cmdRoadmapAnalyze` + // regex this function replaced (`/##\s*(.*v(\d+(?:\.\d+)+)[^(\n]*)/gi`, + // whose capture group 1 begins AFTER `##\s*`). `locateMilestoneHeadings` + // legitimately returns a DIFFERENT representation (`m[1]`, `#`s included) + // — the two owners agree on WHICH milestone headings are selected, not on + // raw heading text. + const heading = m[0].replace(/^#{1,3}\s+/, '').trim(); + const extracted = extractMilestoneHeadingName(heading); + if (extracted === null) continue; // no version token on this heading — not a milestone heading + out.push({ + heading, + version: extracted.version, + name: extracted.name, + closed: isClosedMilestoneHeading(heading), + }); + } + return out; +} + +// #3216: the ONE textual expression of "level-bounded (h1-h3), phase-excluded +// heading line" in this file. `listMilestoneHeadings` and +// `locateMilestoneHeadings` both build their pattern from this constant +// rather than typing `^#{1,3}\s+(?!Phase\s+\S)` a second time — the exact +// duplication class ADR-3180 §7.2's widened guard exists to catch. +const MILESTONE_HEADING_LINE_SOURCE = '^#{1,3}\\s+(?!Phase\\s+\\S)[^\\n]*'; + /** * #3184: the sole milestone-heading locator. Boundary-matched on the version * token with `\b`, NOT the stricter `(?![\w.-])`: this function keeps `\b` @@ -142,17 +206,26 @@ function computeMilestoneSectionEnd(content: string, headingText: string, headin * selection. `extractCurrentMilestoneScoped`, `currentMilestoneRawRanges`, * and `getMilestonePhaseFilter`'s versionOverride branch all consume this * instead of re-deriving their own heading-location regex. + * + * #3216: rewritten as a version-FILTERED VIEW over `MILESTONE_HEADING_LINE_SOURCE` + * — the SAME grammar `listMilestoneHeadings` enumerates — rather than a + * second expression of it. The returned `RegExpExecArray[]` contract + * (`m[0] === m[1]`, `m.index` at the heading's start) is byte-for-byte + * unchanged, so its 4 existing callers are unaffected. */ function locateMilestoneHeadings(content: string, version: string): RegExpExecArray[] { const escapedVersion = escapeRegex(version); - const pattern = new RegExp( - `(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`, - 'gmi', - ); + // ADR-3180 §7.1 locks this boundary as `\b`, not the stricter + // `(?![\w.-])` — Amendment 2 tried the stricter boundary and reverted it. + // `\b` alone is what preserves the #730 sub-milestone selection this + // function owns: `v2.0` still matches inside `v2.0.1`, `v8.0` still + // matches `## v8.0-B …`. + const boundary = new RegExp(`${escapedVersion}\\b`, 'i'); + const pattern = new RegExp(`(${MILESTONE_HEADING_LINE_SOURCE})`, 'gmi'); const matches: RegExpExecArray[] = []; let m: RegExpExecArray | null; while ((m = pattern.exec(content)) !== null) { - matches.push(m); + if (boundary.test(m[1])) matches.push(m); } return matches; } @@ -715,7 +788,7 @@ function reportUnreadableRoadmap(err: unknown, roadmapPath: string): void { interface MilestoneInfo { version: string; - name: string; + name: string | null; } /** @@ -731,7 +804,94 @@ function stripLeadingDelimiter(s: string): string { return s.replace(/^[\s—–:-]+/, '').trim(); } -function getMilestoneInfo(cwd: string): MilestoneInfo { +/** + * #3216 (ADR-3180 §7.2's "Name extraction — pinned rule"): the sole "milestone + * heading text → version + curated name" rule. Strips everything through the + * heading's OWN version token — NOT necessarily a version a caller is + * separately asking about (a `v2.0` STATE selecting a `## v2.0.1 — Portability` + * heading yields the name `Portability`, never `.1 — Portability`) — then ONE + * leading delimiter and surrounding whitespace via `stripLeadingDelimiter`. + * `(` is an ordinary name character and is never a terminator (#3171). Shared + * by `listMilestoneHeadings` and `getMilestoneInfo` so this rule has exactly + * one implementation. Returns `null` when `headingText` carries no version + * token at all (e.g. a non-milestone heading reached this by mistake). + * + * @param expectedVersion - When the caller already knows the exact version it + * is looking for (the STATE-anchored `getMilestoneInfo` path, which located + * this heading via `selectMilestoneHeading(roadmap, stateVersion)`), pass it + * here so the "own version token" is found by anchoring to that KNOWN + * literal (escaped, then extended by the same dash/dot continuation grammar + * for the row-16/17 sub-milestone cases) instead of independently + * re-deriving a version-shaped pattern from scratch. `listMilestoneHeadings` + * (version-agnostic enumeration — no target version exists) omits this and + * keeps the generic re-derivation. Anchoring on the known literal is what + * makes a hostile STATE `milestone:` value (regex metacharacters, single- + * segment `vN`, a literal `$&`/`$1`) resolve correctly: the generic pattern + * only recognizes the real GSD version grammar and stops early on anything + * outside it, leaving hostile characters in the extracted "name". + */ +function extractMilestoneHeadingName( + headingText: string, + expectedVersion?: string, +): { version: string; name: string | null } | null { + const versionMatch = expectedVersion + // Anchor to the KNOWN literal version, then extend across any immediate + // dash/dot continuation the heading's OWN token carries beyond it (e.g. + // requested v8.0 -> heading's own v8.0-B; requested v2.0 -> v2.0.1). + // `.match()` here — never `.replace()` — so a `$&`/`$1`-bearing version + // is located as a literal substring and never interpreted as a + // String.replace() substitution pattern. + ? headingText.match(new RegExp(`${escapeRegex(expectedVersion)}(?:[-.][A-Za-z0-9]+)*`, 'i')) + // No known target: re-derive a version-shaped token generically. `v3` / + // `v3.3` / `v3.3.3` must all resolve to themselves (§7.2), so the dotted + // continuation is zero-or-more, not one-or-more. + : headingText.match(/v\d+(?:\.\d+)*(?:[-.][A-Za-z0-9]+)*/i); + if (!versionMatch) return null; + const version = versionMatch[0]; + const afterVersion = headingText.slice((versionMatch.index ?? 0) + version.length); + // Amendment (§7.2 pinned rule): after stripping the leading delimiter, also + // strip a trailing run of status markers (✅ 📋 🚧) plus surrounding + // whitespace — the marker is already carried structurally by `closed`, so + // duplicating it inside `name` (e.g. "Old ✅") is redundant and wrong. Only + // these three markers, only at the end; a marker inside a name is untouched. + const name = stripLeadingDelimiter(afterVersion).replace(/\s*(?:[✅📋🚧]\s*)+$/, '') || null; + return { version, name }; +} + +/** + * #3216 (epic #3180 §7.2, "Milestone identity"): which milestone is current, + * and what is it called. Binds to the canonical `locateMilestoneHeadings` / + * `listMilestoneHeadings` / `extractMilestoneHeadingName` owners and deletes + * both hand-rolled heading regexes this function used to carry — the + * level-blind STATE-version regex (#3197) and the unanchored fallback regex + * (#3171) — so the class of defect they produced ("### Phase N: … v3.3 …" + * read as milestone `v3.3`; a name truncated at `(`) is structurally + * unrepresentable rather than merely fixed on this one copy. + * + * Never throws (#2245) — the outer try/catch returns `{value: null, scope: + * UNREADABLE}` on any failure, preserving `state.cts:1663`'s "this wrapper + * could never be triggered" invariant. Absence (ENOENT) is silent (#1881, + * ADR-1411); a genuine read fault (e.g. EACCES) still reports via + * `reportUnreadableRoadmap`, which discriminates on the errno exactly as + * before. + * + * The `{version:'v1.0', name:'milestone'}` default this function used to + * return on every unresolved path is DELETED per §7.2 rule 4 — it was + * output-identical to a successful read of a genuine v1.0 project. Every + * unresolved path now returns a `scope` other than `COMPLETE` instead. + */ +/** + * #3216 review Finding 2: `getMilestoneInfo`'s `{ value, scope }` return shape + * was hand-built as an inline object literal at every return point — factored + * out once so the shape itself cannot drift between call sites. Purely a + * literal-shape constructor: does not decide, validate, or alter any value or + * scope — every per-branch rationale comment stays exactly where it was. + */ +function scoped(value: MilestoneInfo | null, scope: Scope): { value: MilestoneInfo | null; scope: Scope } { + return { value, scope }; +} + +function getMilestoneInfo(cwd?: string): { value: MilestoneInfo | null; scope: Scope } { // Declared here but RESOLVED INSIDE the try, so the catch can name the file without // moving planningDir() out of the protected region. planningDir throws a plain Error // for an invalid GSD_WORKSTREAM/GSD_PROJECT segment, and hoisting the call let that @@ -740,6 +900,8 @@ function getMilestoneInfo(cwd: string): MilestoneInfo { // skipped and the default is returned exactly as before. let roadmapPath: string | undefined; try { + if (!cwd) return scoped(null, SCOPE.UNREADABLE); + roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); const roadmap = platformReadSync(roadmapPath); if (roadmap === null) throw new Error('missing'); @@ -770,58 +932,98 @@ function getMilestoneInfo(cwd: string): MilestoneInfo { // (the active-milestone bullet). A `##` heading is often nameless // ("## vX.Y — Active Milestone") and, when unanchored, was matched // spuriously on a copy quoted inside backticks in this very bullet. + // #3216 fix (progressMarkerBulletIsConsultedBeforeHeading): the version + // is commonly wrapped in its OWN bold pair — `🚧 **v3.3** Name` — so a + // trailing `\*?\*?` after the version (mirroring the leading one) is + // required before the `\s+` that anchors the name capture; without it + // the closing `**` sits between the version and the required whitespace + // and the whole match fails, silently falling through to the heading. const listMatch = roadmap.match( - new RegExp(`🚧\\s*\\*?\\*?${escapedVer}\\s+([^*\\n]+)`, 'i') + new RegExp(`🚧\\s*\\*?\\*?${escapedVer}\\*?\\*?\\s+([^*\\n]+)`, 'i') ); if (listMatch) { const name = stripLeadingDelimiter(listMatch[1]); - if (name) return { version: stateVersion, name }; + if (name) return scoped({ version: stateVersion, name }, SCOPE.COMPLETE); } - // Fall back to the `##` heading — ANCHORED to line start (`^` + `m` flag) - // so a heading quoted inside backticks or prose mid-line can no longer - // match. Skip shipped (✅) headings. - const headingMatch = roadmap.match( - new RegExp(`^##[^\\n]*${escapedVer}[:\\s]+([^\\n(]+)`, 'im') - ); - if (headingMatch && !headingMatch[0].includes('✅')) { - // Strip a leading delimiter — `.trim()` removes whitespace, not the - // em-dash/colon that conventionally separates version from name. - const name = stripLeadingDelimiter(headingMatch[1]); - if (name) return { version: stateVersion, name }; + // #3216: heading selection routes through the shared owner + // (`selectMilestoneHeading` — locate → prefer-non-closed, mirroring + // `sliceMilestoneWindow`), deleting the level-blind `^##…` regex (#3197) + // and the unanchored `[:\s]+([^\n(]+)` name capture that truncated at a + // parenthetical (#3171). A CLOSED/shipped heading is not "current" (row + // 5) — it falls through to the TRUNCATED return below exactly as a + // missing heading would. + const selected = selectMilestoneHeading(roadmap, stateVersion); + if (selected) { + const headingText = selected[1].replace(/^#{1,3}\s+/, ''); + if (!isClosedMilestoneHeading(headingText)) { + // #3216 fix: pass the KNOWN stateVersion so name extraction anchors + // to it (see extractMilestoneHeadingName's `expectedVersion` doc) — + // fixes single-segment versions (`v3`, no dot) and hostile STATE + // values (regex metacharacters, literal `$&`/`$1`) that the generic + // re-derivation used by listMilestoneHeadings cannot recognize. + const extracted = extractMilestoneHeadingName(headingText, stateVersion); + if (extracted && extracted.name) { + return scoped({ version: stateVersion, name: extracted.name }, SCOPE.COMPLETE); + } + } } - return { version: stateVersion, name: 'milestone' }; + // Version is known (STATE.md), but no name-bearing evidence resolved: + // no 🚧 bullet, no usable heading (absent, phase-only-excluded, shipped, + // or heading-but-nameless). §7.2 rule 4 — never fabricate a name. + return scoped({ version: stateVersion, name: null }, SCOPE.TRUNCATED); } + // No STATE.md version. The 🚧 in-progress bullet is still consulted first + // (unchanged from the pre-#3216 fallback). const inProgressMatch = roadmap.match(/🚧\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); if (inProgressMatch) { - return { - version: 'v' + inProgressMatch[1], - name: inProgressMatch[2].trim(), - }; + return scoped( + { version: 'v' + inProgressMatch[1], name: inProgressMatch[2].trim() }, + SCOPE.COMPLETE, + ); } + // #3216: enumerate every OPEN (non-shipped) milestone heading via the + // shared owner and take the first in document order — deletes the + // unanchored `/## (?!.*✅).*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/` fallback + // regex (#3171/#3197), whose `## ` prefix matched starting at the SECOND + // `#` of a `### Phase N: …` heading. const cleaned = stripShippedMilestones(roadmap); - const headingMatch = cleaned.match(/## (?!.*✅).*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); - if (headingMatch) { - return { - version: 'v' + headingMatch[1], - name: headingMatch[2].trim(), - }; + const openHeadings = listMilestoneHeadings(cleaned).filter((h) => !h.closed); + if (openHeadings.length > 0) { + const first = openHeadings[0]; + if (first.name) { + return scoped({ version: first.version, name: first.name }, SCOPE.COMPLETE); + } + return scoped({ version: first.version, name: null }, SCOPE.TRUNCATED); } - const versionMatch = cleaned.match(/v(\d+(?:\.\d+)+)/); - return { - version: versionMatch ? versionMatch[0] : 'v1.0', - name: 'milestone', - }; + + // No usable milestone heading anywhere. A version token mentioned ONLY + // inside an excluded `### Phase N: … vX.Y …` heading is not evidence + // (#3197) and must not be reported as if it were a real version — value + // stays null, scope UNSCOPED. A version token mentioned OUTSIDE any Phase + // heading (prose, a bullet, a non-milestone heading) is weak-but-real + // evidence — version retained, name null, scope TRUNCATED. + const withoutPhaseHeadingLines = cleaned.replace(/^#{1,4}\s*Phase\s+\S[^\n]*$/gim, ''); + const bareVersionMatch = withoutPhaseHeadingLines.match(/v\d+(?:\.\d+)+/i); + if (bareVersionMatch) { + return scoped({ version: bareVersionMatch[0], name: null }, SCOPE.TRUNCATED); + } + + // Free-form legacy ROADMAP with no version anywhere reachable, OR the + // only version-bearing heading was a `### Phase N` heading. §7.1's + // "free-form is COMPLETE" governs WINDOWING (whole document is the + // window); identity has no version to report and must not invent one. + return scoped(null, SCOPE.UNSCOPED); } catch (err) { // This function has no existsSync guard, so an absent ROADMAP arrives here too, as a - // synthetic Error with no errno. Only a real read fault is reported; the populated - // default is returned unchanged either way, and a plausible-looking default needs the - // diagnostic more than an empty sentinel does, not less (ADR-1411). + // synthetic Error with no errno. Only a real read fault is reported; `value: null` is + // returned unchanged either way, and a plausible-looking default needs the diagnostic + // more than an empty sentinel does, not less (ADR-1411). if (roadmapPath !== undefined) reportUnreadableRoadmap(err, roadmapPath); - return { version: 'v1.0', name: 'milestone' }; + return scoped(null, SCOPE.UNREADABLE); } } @@ -1156,6 +1358,7 @@ export = { withPhaseSection, computeMilestoneSectionEnd, locateMilestoneHeadings, + listMilestoneHeadings, selectMilestoneHeading, classifyMilestoneWindow, // #3184: the sole "give me this version's window" composition — see its diff --git a/src/roadmap.cts b/src/roadmap.cts index 912f267cd..0f1266e91 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -20,7 +20,7 @@ import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserModule = require('./roadmap-parser.cjs'); -const { stripShippedMilestones, extractCurrentMilestone, extractCurrentMilestoneScoped, replaceInCurrentMilestone } = roadmapParserModule; +const { stripShippedMilestones, extractCurrentMilestone, extractCurrentMilestoneScoped, replaceInCurrentMilestone, listMilestoneHeadings } = roadmapParserModule; import { tokenizeHeadings } from './markdown-sectionizer.cjs'; import { updateTableCell } from './markdown-table.cjs'; import { clampPercent } from './phase-lifecycle.cjs'; @@ -449,16 +449,14 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { }); } - // Extract milestone info - const milestones: Array<{ heading: string; version: string }> = []; - const milestonePattern = /##\s*(.*v(\d+(?:\.\d+)+)[^(\n]*)/gi; - let mMatch: RegExpExecArray | null; - while ((mMatch = milestonePattern.exec(content)) !== null) { - milestones.push({ - heading: mMatch[1].trim(), - version: 'v' + mMatch[2], - }); - } + // Extract milestone info. #3216: routed through the canonical + // `listMilestoneHeadings` owner (deleted the inline `##…` regex, which + // truncated names at a parenthetical and had no phase-heading exclusion) + // rather than re-deriving the enumeration here. + const milestones: Array<{ heading: string; version: string }> = listMilestoneHeadings(content).map((m) => ({ + heading: m.heading, + version: m.version, + })); // Find current and next phase const currentPhase = phases.find(p => p.disk_status === 'planned' || p.disk_status === 'partial') || null; diff --git a/src/state.cts b/src/state.cts index 92f568c26..5e605bde7 100644 --- a/src/state.cts +++ b/src/state.cts @@ -1659,14 +1659,46 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto let milestone: string | null = null; let milestoneName: string | null = null; + // #1761 regression fix (#3216): the milestone STATE.md actually ASSERTS, + // independent of whether getMilestoneInfo's identity scope is COMPLETE. + // Needed below by the disk-scan block's `isMilestoneBoundedInRoadmap` guard + // — that check answers "is the ASSERTED version bounded to a versioned + // ROADMAP heading", a different question from "is the identity trustworthy + // enough to persist" (`milestone` above). Conflating the two regressed + // #1761: when a real STATE `milestone:` value has no matching ROADMAP + // heading, `info.scope` is never COMPLETE (rightly — there's no curated + // name to persist), but the version was still genuinely asserted and the + // bounded check must still run on it, or the guard silently no-ops and + // `state json` reports a conflated whole-document total_phases/percent. + let assertedMilestoneVersion: string | null = null; if (cwd) { // DEAD catch removed (#2245 audit): getMilestoneInfo has its own outer // try/catch (roadmap-parser.cts) that already swallows every internal - // failure and always returns a MilestoneInfo — it never throws, so this + // failure and always returns a ScopedResult — it never throws, so this // wrapper could never be triggered. + // #3216 (ADR-3180 §7.2 rule 6): this is the #3197 disk-write path. Rule 6 + // draws the line at the FIELD, not the scope as a whole — "a version known + // but no name resolvable is TRUNCATED carrying {version, name: null} — the + // version is a real answer, the name is a non-answer, and collapsing the + // two is the failure this contract exists to prevent." So `milestone` + // (the version) is written whenever COMPLETE or TRUNCATED — both carry a + // genuine version per rule 6 — while `milestoneName` is written only on + // COMPLETE, since TRUNCATED's name is by definition unresolved and must + // never be fabricated. UNSCOPED/UNREADABLE have no real version either + // way, so both stay null there. This mirrors cmdCommit (src/commands.cts), + // which accepts COMPLETE or TRUNCATED for the same reason (the version is + // real), and deliberately diverges from archivePhaseDirectories + // (src/milestone.cts), which demands COMPLETE only because it uses the + // value as a filesystem path component and a TRUNCATED version is not + // safe to use there. const info = getMilestoneInfo(cwd); - milestone = info.version; - milestoneName = info.name; + assertedMilestoneVersion = info.value ? info.value.version : null; + if ((info.scope === SCOPE.COMPLETE || info.scope === SCOPE.TRUNCATED) && info.value) { + milestone = info.value.version; + } + if (info.scope === SCOPE.COMPLETE && info.value) { + milestoneName = info.value.name; + } } let totalPhases: number | null = totalPhasesRaw ? parseInt(totalPhasesRaw, 10) : null; @@ -1783,13 +1815,22 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto // phase-dir count only, and mark unbounded so percent is skipped // downstream (mirrors the sync write-path guard). let milestoneBounded = true; - if (milestone && roadmapRaw !== null) { + // #3216 fix (#1761 regression): use `assertedMilestoneVersion` — + // the version STATE.md actually asserts — not the scope-gated + // `milestone`. `milestone` is null on any non-COMPLETE identity + // scope (deliberately, so a non-trustworthy identity never + // persists), but a real asserted version with no matching + // ROADMAP heading is EXACTLY the unbounded case this guard exists + // to catch; gating on `milestone` skipped the guard entirely and + // let the whole-document roadmapPhaseCount conflate sibling + // milestones again. + if (assertedMilestoneVersion && roadmapRaw !== null) { // #3184: routed through the single owner (roadmap-parser.cjs) // instead of a hand-rolled, unbounded-substring re-derivation — // the prior inline regex had no boundary assertion after the // version token, so `v2.0` matched inside `v2.0.1` (#2562-class // defect, design row 17). - milestoneBounded = isMilestoneBoundedInRoadmap(roadmapRaw, String(milestone).trim()); + milestoneBounded = isMilestoneBoundedInRoadmap(roadmapRaw, String(assertedMilestoneVersion).trim()); } // #2828: distinguish a FLAT unmilestoned roadmap (no milestone sectioning // at all — only Phase headings) from a MILESTONED-but-unbounded one diff --git a/src/verify.cts b/src/verify.cts index fd3b27eb0..5d0abc988 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -2320,7 +2320,7 @@ function cmdValidateHealth( fs.copyFileSync(statePath, backupPath); repairActions.push({ action: 'backupState', success: true, path: backupPath }); } - const milestone = getMilestoneInfo(cwd); + const milestone = getMilestoneInfo(cwd).value; const projectRef = path .relative(cwd, path.join(rootBase, 'PROJECT.md')) .split(path.sep) @@ -2329,7 +2329,7 @@ function cmdValidateHealth( stateContent += `## Project Reference\n\n`; stateContent += `See: ${projectRef}\n\n`; stateContent += `## Position\n\n`; - stateContent += `**Milestone:** ${milestone.version} ${milestone.name}\n`; + stateContent += `**Milestone:** ${milestone?.version ?? ''} ${milestone?.name ?? ''}\n`; stateContent += `**Current phase:** (determining...)\n`; stateContent += `**Status:** Resuming\n\n`; stateContent += `## Session Log\n\n`; diff --git a/src/workstream.cts b/src/workstream.cts index dba182022..63b662693 100644 --- a/src/workstream.cts +++ b/src/workstream.cts @@ -157,8 +157,8 @@ function cmdWorkstreamCreate(cwd: string, name: string | null | undefined, optio existingWsName = slugged; } else { try { - const milestone = getMilestoneInfo(cwd); - existingWsName = generateSlugInternal(milestone.name) || 'default'; + const milestone = getMilestoneInfo(cwd).value; + existingWsName = generateSlugInternal(milestone?.name ?? null) || 'default'; } catch { existingWsName = 'default'; } diff --git a/tests/milestone-window-drift-guard.test.cjs b/tests/milestone-window-drift-guard.test.cjs new file mode 100644 index 000000000..92429496e --- /dev/null +++ b/tests/milestone-window-drift-guard.test.cjs @@ -0,0 +1,161 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Unit coverage for the #3216 WIDENING of the milestone-window drift guard + * (scripts/lint-milestone-window-drift.cjs, epic #3180, issue #3184/#3216, + * ADR-3180 Decision 4(a)). + * + * Modelled on tests/enumeration-drift-guard.test.cjs: this file covers ONLY + * the tokens/behavior #3216 added — the literal `##` heading-anchor path, + * the grouped and interpolated version shapes, and the function-scoped + * (not file-scoped) exemption boundary. Behavioral throughout: assertions + * drive `findMilestoneWindowDrift` / `headingMatcherLiterals` / `scanRepo` + * directly — no `readFileSync().includes()` in a test body. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const { + findMilestoneWindowDrift, + scanRepo, + headingMatcherLiterals, +} = require(path.join(ROOT, 'scripts', 'lint-milestone-window-drift.cjs')); + +describe('#3216 widened tokens: findMilestoneWindowDrift (pure)', () => { + test('an interpolated-version heading matcher (the real :785 shape) IS flagged', () => { + const line = + " const headingMatch = roadmap.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));"; + const v = findMilestoneWindowDrift(line, 'src/fake.cts'); + assert.equal(v.length, 1); + assert.equal(v[0].line, 1); + }); + + test('a grouped-version regex-literal heading matcher (the real :806 shape) IS flagged', () => { + const line = ' const headingMatch = cleaned.match(/## (?!.*✅).*v(\\d+(?:\\.\\d+)+)[:\\s]+([^\\n(]+)/);'; + const v = findMilestoneWindowDrift(line, 'src/fake.cts'); + assert.equal(v.length, 1); + assert.equal(v[0].line, 1); + }); + + test('an unanchored enumeration shape (the real :454 shape) IS flagged', () => { + const line = ' const milestonePattern = /##\\s*(.*v(\\d+(?:\\.\\d+)+)[^(\\n]*)/gi;'; + const v = findMilestoneWindowDrift(line, 'src/fake.cts'); + assert.equal(v.length, 1); + assert.equal(v[0].line, 1); + }); + + test('phaseHeadingRegexCarryingOnlyTheQuantifierIsNotFlagged', () => { + // Token (a) alone — no literal heading run, no version/marker token — is + // the "where is phase N's heading" question, already single-owned + // elsewhere (#2121), and must stay clean. + const line = ' const phaseRe = /^#{2,4}\\s*Phase\\s+(\\d+)/gm;'; + assert.deepEqual(findMilestoneWindowDrift(line, 'src/fake.cts'), []); + }); + + test('a heading-BUILDING template is NOT flagged (the false-positive #3216 narrowing exists to prevent)', () => { + // A template that BUILDS a heading for output (`## ${version} — ${name}`) + // is not a re-derivation of WHERE a milestone section begins — it never + // matches against document text at all — so it must stay outside the + // literal-heading-run detector, or every heading writer in the tree + // would be flagged as if it were `locateMilestoneHeadings`. + const line = ' const heading = `## ${version} — ${name}`;'; + assert.deepEqual(findMilestoneWindowDrift(line, 'src/fake.cts'), []); + }); + + test('literalHashShapeInACommentIsNotFlagged', () => { + const line = + " // const headingMatch = roadmap.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));"; + assert.deepEqual(findMilestoneWindowDrift(line, 'src/fake.cts'), []); + }); + + test('literalHashShapeInAJsDocContinuationIsNotFlagged', () => { + const line = + " * const headingMatch = roadmap.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));"; + assert.deepEqual(findMilestoneWindowDrift(line, 'src/fake.cts'), []); + }); + + test('a # run inside a string literal that is never fed to new RegExp( is NOT flagged', () => { + const line = " const colour = fgHash + '#ffffff';"; + assert.deepEqual(findMilestoneWindowDrift(line, 'src/fake.cts'), []); + }); +}); + +describe('#3216: headingMatcherLiterals (pure)', () => { + test('a regex literal on the line is returned', () => { + const line = 'const x = /abc/;'; + assert.deepEqual(headingMatcherLiterals(line, line), ['/abc/']); + }); + + test('a string/template literal is returned ONLY when the line also contains new RegExp(', () => { + const withNewRegExp = 'const re = new RegExp(`##foo`);'; + assert.deepEqual(headingMatcherLiterals(withNewRegExp, withNewRegExp), ['`##foo`']); + + const plainAssignment = 'const heading = `##foo`;'; + assert.deepEqual(headingMatcherLiterals(plainAssignment, plainAssignment), []); + }); + + test('a line with no literals returns an empty array', () => { + const line = 'const x = 1 + 2;'; + assert.deepEqual(headingMatcherLiterals(line, line), []); + }); +}); + +describe('#3216: function-scoped exemptions', () => { + const OWNER_REL = path.join('src', 'roadmap-parser.cts'); + + test('namedCanonicalFunctionsRemainExemptAfterWidening', () => { + const text = [ + 'function locateMilestoneHeadings(content, version) {', + " const headingMatch = content.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));", + ' return headingMatch;', + '}', + ].join('\n'); + assert.deepEqual(findMilestoneWindowDrift(text, OWNER_REL), []); + }); + + test('the same flagged shape inside getMilestoneInfo IS flagged, proving the exemption is function-scoped not file-scoped (row 40/53)', () => { + const text = [ + 'function getMilestoneInfo(cwd) {', + " const headingMatch = content.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));", + ' return headingMatch;', + '}', + ].join('\n'); + const v = findMilestoneWindowDrift(text, OWNER_REL); + assert.equal(v.length, 1); + assert.equal(v[0].line, 2); + }); + + test('the same shape in an unrelated file path IS flagged (no exemption entry at all)', () => { + const text = [ + 'function getMilestoneInfo(cwd) {', + " const headingMatch = content.match(new RegExp(`^##[^\\\\n]*${escapedVer}[:\\\\s]+([^\\\\n(]+)`, 'im'));", + ' return headingMatch;', + '}', + ].join('\n'); + const v = findMilestoneWindowDrift(text, path.join('src', 'somewhere-else.cts')); + assert.equal(v.length, 1); + }); +}); + +describe('#3216: the live repo', () => { + test('liveRepoHasZeroMilestoneWindowRederivations', () => { + // Expected to FAIL (RED) until the #3216 consolidation lands — at write + // time the live repo still carries 3 un-consolidated re-derivations + // (roadmap-parser.cts getMilestoneInfo x2, roadmap.cts:454 + // cmdRoadmapAnalyze — see 50-test-matrix.md's "Scope amendment" section). + // This is the failing-first proof the widened guard binds to real code, + // not just synthetic fixtures. + const violations = scanRepo(ROOT); + assert.deepEqual( + violations, + [], + 'unsanctioned milestone-window re-derivation(s) — route through roadmap-parser.cts ' + + 'computeMilestoneSectionEnd / locateMilestoneHeadings / isMilestoneBoundedInRoadmap:\n' + + violations.map((d) => ` ${d.file}:${d.line} ${d.found}`).join('\n'), + ); + }); +}); diff --git a/tests/milestone-window-single-owner.test.cjs b/tests/milestone-window-single-owner.test.cjs index fafa592c3..a1c340cf1 100644 --- a/tests/milestone-window-single-owner.test.cjs +++ b/tests/milestone-window-single-owner.test.cjs @@ -1319,3 +1319,1052 @@ test('classification is newline-invariant', (t) => { } assert.strictEqual(report.failed, false, 'classification must be newline-invariant'); }); + +// ═════════════════════════════════════════════════════════════════════════ +// Section H — Milestone identity has one owner (#3216, epic #3180 Phase 6) +// Matrix: .gsd/phase/refactor-3216-milestone-identity-single-owner/50-test-matrix.md +// ═════════════════════════════════════════════════════════════════════════ +// +// `getMilestoneInfo(cwd)` moves from a bare `{version, name}` return (with a +// `{v1.0,'milestone'}` failure default output-identical to a genuine v1.0 +// project) to `ScopedResult` -- `{value, scope}`, scope +// drawn from the same frozen SCOPE enum Section A uses. `listMilestoneHeadings` +// is a new version-agnostic sibling of `locateMilestoneHeadings`, consolidating +// the THIRD copy the widened drift guard found at `roadmap.cts:454` +// (`cmdRoadmapAnalyze`'s inline milestone-heading regex). +// +// Neither symbol exists in this shape yet -- every test below is failing-first +// by construction, not only the rows the matrix marks (RED). Both are +// destructured locally (not added to the top-of-file import block) so this +// section is a pure append. + +const { + getMilestoneInfo, + listMilestoneHeadings, +} = roadmapParser; +const unusableInputMod = require('../gsd-core/bin/lib/unusable-input.cjs'); +const { + _resetUnusableInputWarningsForTests, + _unusableInputEmissionCountForTests, +} = unusableInputMod; +// `createTempGitProject`/`gitOrThrow` (not added to the top-of-file import +// block, same "pure append" rationale as above): the four destructive +// `phases clear --confirm` tests below need a real, clean git repo so the +// command's own uncommitted-changes guard behaves deterministically instead +// of being bypassed with `--force`. +const { createTempGitProject } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); + +// ─── Shared CRLF-twinned fixture table (rows 1, 3, 4, 5, 18, 19, 24) ────── + +const H_CRLF_TABLE = [ + { + label: 'H1 named-heading', + version: 'v3.3', + lines: ['# Roadmap', '', '## v3.3 — Portability', '', 'body text'], + }, + { + label: 'H3 parenthetical-name', + version: 'v3.3', + lines: ['# Roadmap', '', '## v3.3 — Portability (Windows)', '', 'body text'], + }, + { + label: 'H4 phase-heading-only', + version: 'v3.3', + lines: ['# Roadmap', '', '### Phase 7: Close v3.3 gaps', '', 'body text'], + }, + { + label: 'H5 phase-heading-only-unscoped', + version: null, + lines: ['# Roadmap', '', '### Phase 7: Close v3.3 gaps', '', 'body text'], + }, + { + label: 'H18 phase-and-real-heading', + version: 'v3.3', + lines: ['# Roadmap', '', '### Phase 7: Close v3.3 gaps', '', '## v3.3 — Real Name', '', 'body text'], + }, + { + label: 'H19 closed-then-live', + version: 'v3.3', + lines: ['# Roadmap', '', '## v3.3 — Old ✅ SHIPPED', '', '## v3.3 — New', '', 'body text'], + }, +]; + +function buildHFixture(t, entry, crlf = false) { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + if (entry.version) writeState(cwd, { milestone: entry.version }); + const content = entry.lines.join('\n'); + writeRoadmap(cwd, crlf ? content.replace(/\n/g, '\r\n') : content); + return cwd; +} + +test('stateVersionWithNamedHeadingReportsBothAndCompleteScope', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[0]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: 'Portability' }); +}); + +test('progressMarkerBulletIsConsultedBeforeHeading', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + const content = [ + '# Roadmap', + '', + '🚧 **v3.3** Bullet Name (Windows)', + '', + '## v3.3 — Decoy Heading Name', + ].join('\n'); + writeRoadmap(cwd, content); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + // The 🚧 bullet is consulted BEFORE the heading (#2135) -- its name wins + // even though a differently-named heading for the same version exists too. + assert.strictEqual(result.value.name, 'Bullet Name (Windows)'); +}); + +// (RED) #3171: the heading-name capture must not truncate at '('. +test('headingNameRetainsParentheticalInsteadOfTruncating', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[1]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'Portability (Windows)'); +}); + +// (RED) #3197: a Phase heading mentioning the milestone's version is never a +// milestone heading -- even on the STATE-anchored path. +test('phaseHeadingIsNeverAcceptedAsTheMilestoneHeadingOnStatePath', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[2]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.TRUNCATED); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: null }); +}); + +// (RED) #3197: same defect, the UNANCHORED `:806` fallback path (no STATE +// version to anchor on). +test('phaseHeadingIsNeverAcceptedAsTheMilestoneHeadingOnFallbackPath', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[3]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNSCOPED); + assert.strictEqual(result.value, null); +}); + +test('shippedHeadingIsNotReportedAsCurrentMilestone', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['# Roadmap', '', '## v3.3 — Old ✅ SHIPPED', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + // A shipped heading for the STATE-stored version is not "current" -- this + // reduces to the same disposition as "no live heading found". + assert.equal(result.scope, SCOPE.TRUNCATED); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: null }); +}); + +test('knownVersionWithNoHeadingReportsVersionAndNullName', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['# Roadmap', '', '## Overview', '', 'No milestone headings here.'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.TRUNCATED); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: null }); +}); + +// #3216 review Finding 4 / design row 10: no STATE `milestone:` field, no +// milestone heading anywhere, but a bare version token appears in plain +// prose (outside any Phase heading). That is weak-but-real evidence -- the +// version is retained, the name stays `null` (never fabricated), scope +// TRUNCATED. Sibling negative case (no version token anywhere) is +// `freeFormRoadmapWithNoVersionYieldsUnscopedNotV1Default` immediately below. +test('bareVersionInProseYieldsTruncatedWithNoName', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['# Roadmap', '', '## Overview', '', 'Targeting v3.3 for the next release.'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.TRUNCATED); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: null }); +}); + +// (RED) #3197: free-form legacy ROADMAP with zero version tokens anywhere -- +// the version is genuinely unresolvable and must not be invented. +test('freeFormRoadmapWithNoVersionYieldsUnscopedNotV1Default', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['# Roadmap', '', '## Overview', '', '### Phase 1: Foo'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNSCOPED); + assert.strictEqual(result.value, null); +}); + +// (RED) THE load-bearing row. Every other row in this section can pass while +// the scope wiring is inverted (e.g. COMPLETE and UNREADABLE swapped) -- only +// a genuine v1.0 project reporting {v1.0, }/COMPLETE proves the +// old `{version:'v1.0', name:'milestone'}` failure default is actually GONE, +// rather than renamed to a scope label nobody checks. +test('genuineV1ProjectIsDistinguishableFromTheOldFailureDefault', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, ['# Roadmap', '', '## v1.0 — Foundation Release', '', '### Phase 1: Bootstrap'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.deepStrictEqual(result.value, { version: 'v1.0', name: 'Foundation Release' }); + // The name is deliberately NOT the literal string 'milestone' -- that is the + // old failure default's signature, and a project that legitimately named + // its v1.0 milestone "milestone" is not this test's concern. + assert.notStrictEqual(result.value.name, 'milestone'); +}); + +test('absentRoadmapIsUnreadableScopeAndStaysSilent', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + // No ROADMAP.md written at all -- ENOENT. + _resetUnusableInputWarningsForTests(); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNREADABLE); + assert.strictEqual(result.value, null); + // #1881: absence alone must never be reported -- every brand-new project + // has no ROADMAP.md yet, and flagging that as corrupt would be noise. + assert.strictEqual(_unusableInputEmissionCountForTests(), 0); +}); + +test('unreadableRoadmapReportsDiagnosticAndUnreadableScope', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + writeRoadmap(cwd, ['# Roadmap', '', '## v3.3 — Name'].join('\n')); + writeState(cwd, { milestone: 'v3.3' }); + _resetUnusableInputWarningsForTests(); + + mock.method(shellProj, 'platformReadSync', () => { + const err = new Error('EACCES: simulated permission denied'); + err.code = 'EACCES'; + throw err; + }); + t.after(() => { + mock.restoreAll(); + cleanup(cwd); + }); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNREADABLE); + assert.strictEqual(result.value, null); + // ADR-1411: unlike absence, a genuine read fault IS reported. + assert.strictEqual(_unusableInputEmissionCountForTests(), 1); +}); + +test('unreadableStateFallsBackWithoutFabricatingAVersion', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + // No versioned milestones anywhere in the ROADMAP, so the fallback path + // (once STATE.md is unreachable) has no version evidence to find either. + writeRoadmap(cwd, ['# Roadmap', '', '## Overview', '', '### Phase 1: Foo'].join('\n')); + writeState(cwd, { milestone: 'v3.3' }); + + const originalRead = shellProj.platformReadSync; + mock.method(shellProj, 'platformReadSync', (targetPath) => { + if (typeof targetPath === 'string' && targetPath.endsWith('STATE.md')) { + const err = new Error('EACCES: simulated permission denied'); + err.code = 'EACCES'; + throw err; + } + return originalRead(targetPath); + }); + t.after(() => { + mock.restoreAll(); + cleanup(cwd); + }); + + const result = getMilestoneInfo(cwd); + // Falls back to ROADMAP-only heuristics -- which find no version -- rather + // than fabricating v1.0 or trusting a version it could not actually read. + assert.equal(result.scope, SCOPE.UNSCOPED); + assert.strictEqual(result.value, null); +}); + +test('emptyRoadmapYieldsUnscoped', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ''); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNSCOPED); + assert.strictEqual(result.value, null); +}); + +test('whitespaceOnlyRoadmapYieldsUnscoped', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ' \n\n\t \n'); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.UNSCOPED); + assert.strictEqual(result.value, null); +}); + +test('headingLevelsOneThroughThreeAreAllAccepted', (t) => { + const tmpDirs = []; + t.after(() => { for (const d of tmpDirs) cleanup(d); }); + + for (const level of [1, 2, 3]) { + const cwd = createTempDir('gsd-milestone-identity-'); + tmpDirs.push(cwd); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, [`${'#'.repeat(level)} v3.3 — Name`, '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE, `level ${level}`); + assert.strictEqual(result.value.name, 'Name', `level ${level}`); + } +}); + +test('headingLevelFourIsRejected', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['#### v3.3 — Name', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + // #{1,3} is the owner's level ceiling -- a level-4 heading is invisible to + // it, same as no heading at all. + assert.equal(result.scope, SCOPE.TRUNCATED); + assert.deepStrictEqual(result.value, { version: 'v3.3', name: null }); +}); + +test('versionSegmentCountsAtAndAroundTwoAreHandled', (t) => { + const tmpDirs = []; + t.after(() => { for (const d of tmpDirs) cleanup(d); }); + + for (const version of ['v3', 'v3.3', 'v3.3.3']) { + const cwd = createTempDir('gsd-milestone-identity-'); + tmpDirs.push(cwd); + writeState(cwd, { milestone: version }); + writeRoadmap(cwd, [`## ${version} — Name`, '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE, version); + assert.deepStrictEqual(result.value, { version, name: 'Name' }, version); + } +}); + +test('realMilestoneHeadingWinsOverAPhaseHeadingMentioningTheSameVersion', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[4]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'Real Name'); +}); + +test('prefersTheNonClosedHeadingWhenAVersionAppearsTwice', (t) => { + const cwd = buildHFixture(t, H_CRLF_TABLE[5]); + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'New'); +}); + +// #730 regression guard: `v8.0` (STATE) must select the LIVE `v8.0-B` +// sub-milestone heading over the CLOSED `v8.0-A` one -- the `\b` boundary +// that makes this possible is deliberate, load-bearing behavior (§7.1). +test('subMilestoneSelectionIsPreservedForBoundaryWordChars', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v8.0' }); + writeRoadmap(cwd, [ + '# Roadmap', + '', + '## v8.0-A — Old ✅ SHIPPED', + '', + '## v8.0-B — LiveMarker', + ].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.version, 'v8.0'); + // Exact equality, not .includes(): the name derives from the heading's OWN + // version token (v8.0-B), not the requested v8.0 -- an .includes() check + // would also pass on a leaked prefix from the closed v8.0-A heading, which + // is the specific misselection this row guards against (pinned rule, + // .gsd/phase/refactor-3216-milestone-identity-single-owner/40-design.md). + assert.equal(result.value.name, 'LiveMarker', `expected exact name LiveMarker, got ${JSON.stringify(result.value.name)}`); +}); + +// ADR-3180 §7.1 locks the `\b` boundary deliberately: `v2.0` (STATE) matching +// `## v2.0.1 — Name` is CORRECT, not a bug. Amendment 2 tried the stricter +// `(?![\w.-])` boundary specifically to stop this and REVERTED it because it +// breaks #730 sub-milestone selection (the row above). Do not "fix" this. +test('versionPrefixMatchInsideLongerVersionIsRetainedDeliberately', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v2.0' }); + writeRoadmap(cwd, ['## v2.0.1 — Name', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.version, 'v2.0'); + // Exact equality, not .includes(): the name derives from the heading's OWN + // version token (v2.0.1), not the requested v2.0 -- an .includes() check + // would also pass on the '.1 — Name' leftover that stripping the + // *requested* token (instead of the heading's own) would produce, which is + // precisely the defect the pinned name-extraction rule excludes + // (.gsd/phase/refactor-3216-milestone-identity-single-owner/40-design.md). + assert.equal(result.value.name, 'Name', `expected exact name Name, got ${JSON.stringify(result.value.name)}`); +}); + +test('leadingDelimiterIsStrippedFromName', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['## v3.3 — Delimited Name', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'Delimited Name'); +}); + +// #2135: a name beginning with '#' is a heading-parse failure -- it must stay +// loud (unstripped) rather than being silently cleaned, unlike a whitespace/ +// dash/colon/em-dash leading delimiter. +test('hashLedNameIsLeftLoudRatherThanCleaned', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['## v3.3 #HashName', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, '#HashName'); +}); + +test('crlfRoadmapsProduceIdenticalIdentityToLf', (t) => { + const tmpDirs = []; + t.after(() => { for (const d of tmpDirs) cleanup(d); }); + + for (const entry of H_CRLF_TABLE) { + const lfCwd = createTempDir('gsd-milestone-identity-crlf-lf-'); + const crlfCwd = createTempDir('gsd-milestone-identity-crlf-crlf-'); + tmpDirs.push(lfCwd, crlfCwd); + if (entry.version) { + writeState(lfCwd, { milestone: entry.version }); + writeState(crlfCwd, { milestone: entry.version }); + } + const lfContent = entry.lines.join('\n'); + writeRoadmap(lfCwd, lfContent); + writeRoadmap(crlfCwd, lfContent.replace(/\n/g, '\r\n')); + + const lfResult = getMilestoneInfo(lfCwd); + const crlfResult = getMilestoneInfo(crlfCwd); + assert.equal(crlfResult.scope, lfResult.scope, `scope mismatch for ${entry.label}`); + assert.strictEqual(crlfResult.value?.name ?? null, lfResult.value?.name ?? null, `name mismatch for ${entry.label}`); + if (crlfResult.value?.name) { + assert.ok(!crlfResult.value.name.includes('\r'), `CRLF leaked into name for ${entry.label}: ${JSON.stringify(crlfResult.value.name)}`); + } + } +}); + +test('unicodeNameIsPreserved', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['## v3.3 — Ünïcode ▲', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'Ünïcode ▲'); +}); + +test('veryLongNameIsNotTruncated', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + const longName = 'X'.repeat(4096); + writeRoadmap(cwd, [`## v3.3 — ${longName}`, '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name.length, 4096); + assert.strictEqual(result.value.name, longName); +}); + +// #2143: fence-awareness is explicitly out of scope for this phase (Decision +// 6) -- a milestone-shaped heading at line-start INSIDE a fenced code block +// still matches. Documented known limit, not a regression to fix here. +test('fencedHeadingStillMatchesAsADocumentedKnownLimit', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['# Roadmap', '', '```markdown', '## v3.3 — Fenced Name', '```', '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.name, 'Fenced Name'); +}); + +// #40-design.md row 28's actual requirement: `escapeRegex` holds -- no +// crash, no catastrophic match. It does NOT require a hostile version ending +// in regex metacharacters to successfully RESOLVE to a milestone (with `\b` +// restored per ADR-3180 §7.1, `v1.0+(x)` sits between two non-word +// characters on both sides of its own boundary and structurally cannot +// match there -- that is the LOCKED `\b` semantics, not a defect). +test('versionWithRegexMetacharactersIsEscaped', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + const hostileVersion = 'v1.0+(x)'; + writeState(cwd, { milestone: hostileVersion }); + writeRoadmap(cwd, [`## ${hostileVersion} — Name`, '', 'body'].join('\n')); + + // 1. No crash. + assert.doesNotThrow(() => getMilestoneInfo(cwd)); + const result = getMilestoneInfo(cwd); + assert.strictEqual(result.value.version, hostileVersion, 'the raw hostile version is preserved verbatim'); + + // 2. Metacharacters are treated as literals, never as regex operators. If + // `+` and `(x)` were left unescaped, `v1.0+(x)` would compile to "v1", + // then literal ".", then "one-or-more '0'", then a capturing group + // matching literal "x" -- which would falsely match a document like + // "v1.00000(x)" that the ESCAPED literal string must never match. + const unescapedWouldFalselyMatch = ['## v1.00000(x) — Wrong Match', '', 'body'].join('\n'); + assert.strictEqual( + locateMilestoneHeadings(unescapedWouldFalselyMatch, hostileVersion).length, + 0, + 'escaped metacharacters must not act as regex operators against an unrelated document', + ); + + // 3. Completes promptly -- no catastrophic backtracking. No elapsed-time + // bound is asserted (see normalize-test-command.test.cjs's identical + // rationale): a real ReDoS regression manifests as the run hanging / + // being killed, which is a louder, more reliable signal than a threshold. + const hostileRepeated = 'v1.0+(x)'.repeat(5000); + writeState(cwd, { milestone: hostileRepeated }); + writeRoadmap(cwd, [`## ${hostileRepeated} — Name`, '', 'body'].join('\n')); + assert.doesNotThrow(() => getMilestoneInfo(cwd)); +}); + +test('versionWithReplacementPatternIsTreatedLiterally', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + const hostileVersion = 'v1.0$&$1'; + writeState(cwd, { milestone: hostileVersion }); + writeRoadmap(cwd, [`## ${hostileVersion} — Name`, '', 'body'].join('\n')); + + const result = getMilestoneInfo(cwd); + assert.equal(result.scope, SCOPE.COMPLETE); + assert.strictEqual(result.value.version, hostileVersion); + // '$&'/'$1' must never be interpreted as a String.replace() substitution + // pattern -- the extracted name must be exactly 'Name', not a corrupted + // expansion of the hostile version token. + assert.strictEqual(result.value.name, 'Name'); +}); + +// Hostile / sink (#2288 defense in depth): a traversal-shaped STATE +// `milestone:` value must never let `archivePhaseDirectories` move phase +// history outside `.planning/milestones/`. +test('traversalVersionNeverEscapesTheMilestonesDirectory', (t) => { + // `createTempGitProject` (git init + initial commit) gives the command's + // own uncommitted-changes guard a deterministic clean repo, so the test + // proves the real destructive-command guard rather than bypassing it with + // `--force` -- `--force` is exactly the safety this row exercises. + const cwd = createTempGitProject('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['# Roadmap', '', 'No milestone headings here.'].join('\n')); + const hostileVersion = '../../etc/passwd'; + writeState(cwd, { milestone: hostileVersion }); + writeFile(cwd, path.join('.planning', 'phases', '01-example', 'PLAN.md'), '# Plan'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', 'seed fixture'], { cwd }); + + const escapeTarget = path.resolve(cwd, '.planning', 'milestones', '..', '..', 'etc', 'passwd-phases'); + assert.strictEqual(fs.existsSync(escapeTarget), false, 'precondition: escape target must not pre-exist'); + + const result = runGsdTools(['phases', 'clear', '--confirm', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(result.success, true, result.error); + + // Negative proof: the traversal string never left `.planning/milestones/`. + assert.strictEqual(fs.existsSync(escapeTarget), false); + const milestonesDir = path.join(cwd, '.planning', 'milestones'); + const entries = fs.existsSync(milestonesDir) ? fs.readdirSync(milestonesDir) : []; + assert.ok(entries.length > 0, 'expected the dated-fallback archive dir to exist'); + for (const entry of entries) { + assert.ok(!entry.includes('etc') && !entry.includes('passwd'), `traversal string leaked into archive dir name: ${entry}`); + // ARCHIVE_VERSION_LABEL_RE rejects the traversal string, so the archive + // falls back to the dated label -- never the hostile STATE value. + assert.match(entry, /^archived-\d{8}-phases$/); + } +}); + +test('controlCharacterVersionIsRefusedAsAPathComponent', (t) => { + // `createTempGitProject`, not a bare temp dir + `--force`: see rationale on + // `traversalVersionNeverEscapesTheMilestonesDirectory` above. + const cwd = createTempGitProject('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['# Roadmap', '', 'No milestone headings here.'].join('\n')); + const ESC = String.fromCharCode(0x1b); + const hostileVersion = `v1.0${ESC}[31mHACK`; + writeState(cwd, { milestone: hostileVersion }); + writeFile(cwd, path.join('.planning', 'phases', '01-example', 'PLAN.md'), '# Plan'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', 'seed fixture'], { cwd }); + + const result = runGsdTools(['phases', 'clear', '--confirm', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(result.success, true, result.error); + // Nothing replayed to the terminal: the hostile version never reaches the + // command's own output. + assert.strictEqual((result.output || '').includes(ESC), false); + + const milestonesDir = path.join(cwd, '.planning', 'milestones'); + const entries = fs.existsSync(milestonesDir) ? fs.readdirSync(milestonesDir) : []; + assert.ok(entries.length > 0, 'expected the dated-fallback archive dir to exist'); + for (const entry of entries) { + assert.ok(!entry.includes(ESC), `control character leaked into archive dir name: ${JSON.stringify(entry)}`); + assert.match(entry, /^archived-\d{8}-phases$/); + } +}); + +test('undefinedCwdDoesNotThrow', () => { + // The shipping caller shape: buildStateFrontmatter guards `if (cwd)` before + // calling getMilestoneInfo, but getMilestoneInfo itself must survive being + // called with cwd === undefined directly (#2245). + let result; + assert.doesNotThrow(() => { result = getMilestoneInfo(undefined); }); + assert.ok(result && typeof result === 'object' && 'scope' in result && 'value' in result); + assert.ok(Object.values(SCOPE).includes(result.scope)); +}); + +test('getMilestoneInfoNeverThrowsAcrossEveryInputClass', (t) => { + const tmpDirs = []; + t.after(() => { for (const d of tmpDirs) cleanup(d); }); + + function freshDir() { + const cwd = createTempDir('gsd-milestone-identity-'); + tmpDirs.push(cwd); + return cwd; + } + + const inputClasses = [ + // 1. No ROADMAP.md at all (ENOENT). + () => freshDir(), + // 2. Empty ROADMAP. + () => { const c = freshDir(); writeRoadmap(c, ''); return c; }, + // 3. Whitespace-only ROADMAP. + () => { const c = freshDir(); writeRoadmap(c, ' \n\t\n'); return c; }, + // 4. Phase-heading-only, V set. + () => { + const c = freshDir(); + writeState(c, { milestone: 'v3.3' }); + writeRoadmap(c, '### Phase 7: Close v3.3 gaps'); + return c; + }, + // 5. Free-form legacy, no version anywhere. + () => { const c = freshDir(); writeRoadmap(c, '## Overview\n### Phase 1: Foo'); return c; }, + // 6. Hostile regex-metacharacter version. + () => { + const c = freshDir(); + writeState(c, { milestone: 'v1.0+(x)[y]' }); + writeRoadmap(c, '## v1.0+(x)[y] — Name'); + return c; + }, + // 7. Control-character version. + () => { + const c = freshDir(); + writeState(c, { milestone: `v1.0${String.fromCharCode(0x1b)}HACK` }); + writeRoadmap(c, '## Overview'); + return c; + }, + // 8. Traversal-shaped version. + () => { + const c = freshDir(); + writeState(c, { milestone: '../../etc/passwd' }); + writeRoadmap(c, '## Overview'); + return c; + }, + // 9. CRLF roadmap. + () => { + const c = freshDir(); + writeState(c, { milestone: 'v3.3' }); + writeRoadmap(c, '## v3.3 — Name\r\n\r\nbody\r\n'); + return c; + }, + // 10. Sub-milestone selection shape. + () => { + const c = freshDir(); + writeState(c, { milestone: 'v8.0' }); + writeRoadmap(c, '## v8.0-A — Old ✅ SHIPPED\n\n## v8.0-B — Live'); + return c; + }, + // 11. cwd === undefined. + () => undefined, + // 12. cwd pointing at a nonexistent directory. + () => path.join(freshDir(), 'does-not-exist-subdir'), + ]; + + for (const build of inputClasses) { + const cwd = build(); + assert.doesNotThrow(() => getMilestoneInfo(cwd), `getMilestoneInfo threw for input class producing cwd=${cwd}`); + } +}); + +// Identity (4c): the consumers' OBSERVABLE output must agree with the +// owner's ScopedResult for the same fixture -- not a locally-reassembled +// copy of it (ADR-3180 Decision 4c: "post-filtering the canonical result" is +// itself a bypass this row is designed to catch). +test('consumerOutputsMatchTheOwnersScopedResultForTheSameFixture', (t) => { + // `createTempGitProject`, not a bare temp dir + `--force`: see rationale on + // `traversalVersionNeverEscapesTheMilestonesDirectory` above. + const cwd = createTempGitProject('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + writeRoadmap(cwd, ['# Roadmap', '', '## v3.3 — Consolidated Name', '', 'body'].join('\n')); + writeFile(cwd, path.join('.planning', 'phases', '01-example', 'PLAN.md'), '# Plan'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', 'seed fixture'], { cwd }); + + const owner = getMilestoneInfo(cwd); + assert.equal(owner.scope, SCOPE.COMPLETE); + + // Consumer 1: buildStateFrontmatter, via `state sync` -> `state json`. + const syncResult = runGsdTools(['state', 'sync', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(syncResult.success, true, syncResult.error); + const jsonResult = runGsdTools(['state', 'json', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(jsonResult.success, true, jsonResult.error); + const parsedJson = JSON.parse(jsonResult.output); + assert.strictEqual(parsedJson.milestone, owner.value.version); + assert.strictEqual(parsedJson.milestone_name, owner.value.name); + + // Consumer 2: archivePhaseDirectories, via `phases clear --confirm`. + const clearResult = runGsdTools(['phases', 'clear', '--confirm', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(clearResult.success, true, clearResult.error); + const milestonesDir = path.join(cwd, '.planning', 'milestones'); + const entries = fs.readdirSync(milestonesDir); + assert.deepStrictEqual(entries, [`${owner.value.version}-phases`]); +}); + +// (RED) Identity (4c), CLI: the full untruncated name must PERSIST through +// `state sync`, and a non-COMPLETE identity must never persist a fabricated +// name. +test('stateSyncPersistsTheOwnersIdentityNotAFabricatedOne', (t) => { + const tmpDirs = []; + t.after(() => { for (const d of tmpDirs) cleanup(d); }); + + // Row-3 fixture: parenthetical name must persist UNTRUNCATED. + const cwd3 = createTempDir('gsd-milestone-identity-'); + tmpDirs.push(cwd3); + writeState(cwd3, { milestone: 'v3.3' }); + writeRoadmap(cwd3, ['# Roadmap', '', '## v3.3 — Portability (Windows)', '', 'body'].join('\n')); + const sync3 = runGsdTools(['state', 'sync', '--cwd', cwd3, '--raw'], cwd3); + assert.strictEqual(sync3.success, true, sync3.error); + const json3result = runGsdTools(['state', 'json', '--cwd', cwd3, '--raw'], cwd3); + assert.strictEqual(json3result.success, true, json3result.error); + const json3 = JSON.parse(json3result.output); + assert.strictEqual(json3.milestone_name, 'Portability (Windows)'); + + // Row-4 fixture: only a Phase heading -- must persist NO fabricated name. + const cwd4 = createTempDir('gsd-milestone-identity-'); + tmpDirs.push(cwd4); + writeState(cwd4, { milestone: 'v3.3' }); + writeRoadmap(cwd4, ['# Roadmap', '', '### Phase 7: Close v3.3 gaps', '', 'body'].join('\n')); + const sync4 = runGsdTools(['state', 'sync', '--cwd', cwd4, '--raw'], cwd4); + assert.strictEqual(sync4.success, true, sync4.error); + const json4result = runGsdTools(['state', 'json', '--cwd', cwd4, '--raw'], cwd4); + assert.strictEqual(json4result.success, true, json4result.error); + const json4 = JSON.parse(json4result.output); + assert.strictEqual(json4.milestone, 'v3.3'); + assert.strictEqual('milestone_name' in json4, false, `milestone_name must be absent, not fabricated: ${JSON.stringify(json4.milestone_name)}`); +}); + +// (RED) Identity (4c), CLI: `archivePhaseDirectories` must refuse a +// syntactically-valid-but-not-COMPLETE version and fall back to the dated +// label instead -- #3197's exact bug (a fabricated-but-well-formed 'v3.3' +// passing ARCHIVE_VERSION_LABEL_RE and misfiling phase history). +test('phasesClearFallsBackToDatedLabelWhenIdentityIsNotComplete', (t) => { + // `createTempGitProject`, not a bare temp dir + `--force`: see rationale on + // `traversalVersionNeverEscapesTheMilestonesDirectory` above. + const cwd = createTempGitProject('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + // Row-4 fixture: only a Phase heading -- scope is TRUNCATED, not COMPLETE, + // even though 'v3.3' alone is a syntactically VALID archive label. + writeRoadmap(cwd, ['# Roadmap', '', '### Phase 7: Close v3.3 gaps', '', 'body'].join('\n')); + writeFile(cwd, path.join('.planning', 'phases', '01-example', 'PLAN.md'), '# Plan'); + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', 'seed fixture'], { cwd }); + + const owner = getMilestoneInfo(cwd); + assert.notEqual(owner.scope, SCOPE.COMPLETE); + + const result = runGsdTools(['phases', 'clear', '--confirm', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(result.success, true, result.error); + const entries = fs.readdirSync(path.join(cwd, '.planning', 'milestones')); + assert.strictEqual(entries.length, 1); + // Must NOT be 'v3.3-phases' -- that is #3197's fabricated-but-well-formed + // label misfiling phase history under the wrong milestone. + assert.notStrictEqual(entries[0], 'v3.3-phases'); + assert.match(entries[0], /^archived-\d{8}-phases$/); +}); + +// (RED) Property: any name without a newline, rendered into a +// `## — ` heading, round-trips through getMilestoneInfo with no +// truncation -- document-shaped (#2371): built from a local grammar here, +// never via any renderer in roadmap-parser.cjs. +test('propertyHeadingNameRoundTripsWithoutTruncation', (t) => { + const cwd = createTempDir('gsd-milestone-identity-property-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + + // Starts with a letter so it can never collide with a leading-delimiter + // strip; letters/digits/spaces/parens exercise the #3171 parenthetical + // retention without ambiguity about what "round-trips" means. + const nameGen = fc.stringMatching(/^[A-Za-z][A-Za-z0-9 ()]{0,60}$/).filter((s) => s.trim() === s && s.length > 0); + + const report = fc.check( + fc.property(nameGen, (name) => { + writeRoadmap(cwd, [`## v3.3 — ${name}`, '', 'body'].join('\n')); + const result = getMilestoneInfo(cwd); + return result.scope === SCOPE.COMPLETE && result.value.name === name; + }), + { seed: 3216, numRuns: 100 }, + ); + if (report.failed) { + t.diagnostic(`H43 counterexample (replay seed=3216): ${JSON.stringify(report.counterexample)}`); + } + assert.strictEqual(report.failed, false, 'heading name must round-trip without truncation'); +}); + +// Property: no generated `### Phase N …` heading -- with or without an +// embedded version token -- ever yields COMPLETE from getMilestoneInfo when +// it is the only heading in the document. +test('propertyPhaseHeadingsNeverYieldCompleteScope', (t) => { + const cwd = createTempDir('gsd-milestone-identity-property-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + + const phaseWordGen = fc.stringMatching(/^[A-Za-z][A-Za-z0-9]{0,12}$/); + const phaseNumGen = fc.integer({ min: 1, max: 999 }); + + const report = fc.check( + fc.property(phaseNumGen, phaseWordGen, (num, word) => { + writeRoadmap(cwd, [`### Phase ${num}: Close v3.3 ${word}`, '', 'body'].join('\n')); + const result = getMilestoneInfo(cwd); + return result.scope !== SCOPE.COMPLETE; + }), + { seed: 3216, numRuns: 100 }, + ); + if (report.failed) { + t.diagnostic(`H44 counterexample (replay seed=3216): ${JSON.stringify(report.counterexample)}`); + } + assert.strictEqual(report.failed, false, 'a Phase heading alone must never yield COMPLETE'); +}); + +test('listMilestoneHeadingsEnumeratesEveryMilestoneInDocumentOrder', () => { + const content = [ + '# Roadmap', + '', + '## v1.0 — First ✅ SHIPPED', + '', + '## v2.0 — Second', + '', + '### Phase 1: Foo', + '', + '## v3.0 — Third 🚧', + ].join('\n'); + + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 3); + assert.strictEqual(result[0].version, 'v1.0'); + assert.strictEqual(result[0].closed, true); + assert.strictEqual(result[1].version, 'v2.0'); + assert.strictEqual(result[1].closed, false); + assert.strictEqual(result[2].version, 'v3.0'); + assert.strictEqual(result[2].closed, false); +}); + +// (RED) #3197: a phase heading is never a milestone -- this is the SAME +// consolidation defect as site 1/2, in the THIRD copy (`roadmap.cts:454`). +test('listMilestoneHeadingsExcludesPhaseHeadings', () => { + const content = ['# Roadmap', '', '### Phase 7: Close v3.3 gaps'].join('\n'); + const result = listMilestoneHeadings(content); + assert.deepStrictEqual(result, []); +}); + +// (RED) #3171: the enumerated heading text must not be cut at a parenthetical. +test('listMilestoneHeadingsRetainsParentheticalNames', () => { + const content = ['# Roadmap', '', '## v3.3 — Portability (Windows)'].join('\n'); + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 1); + assert.ok(result[0].heading.includes('Portability (Windows)'), result[0].heading); +}); + +// (RED) #3216 review Finding 4: the shared grammar's `[^\n]*` captures a +// trailing `\r` on a CRLF-encoded ROADMAP, and unlike the inline +// `cmdRoadmapAnalyze` regex this replaced (which called `.trim()`), +// `listMilestoneHeadings` did not trim its `heading` extraction -- so a CRLF +// ROADMAP's `heading` field carried a stray trailing `\r` a LF ROADMAP never +// would. Every entry's `heading` must be `\r`-free and carry no leading/ +// trailing whitespace, for every milestone in the document, not just one. +test('listMilestoneHeadingsHeadingFieldIsCrlfFreeAndTrimmed', () => { + const content = ['## v3.3 — Portability (Windows)', '', '## v2.0 — Old ✅'].join('\r\n'); + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 2); + for (const entry of result) { + assert.ok(!entry.heading.includes('\r'), `CRLF leaked into heading: ${JSON.stringify(entry.heading)}`); + assert.strictEqual(entry.heading, entry.heading.trim(), `heading not trimmed: ${JSON.stringify(entry.heading)}`); + } + assert.strictEqual(result[0].name, 'Portability (Windows)'); + assert.strictEqual(result[1].name, 'Old'); +}); + +test('listMilestoneHeadingsRespectsTheOneToThreeLevelBound', () => { + const content = [ + '# v1.0 — Level One', + '', + '## v2.0 — Level Two', + '', + '### v3.0 — Level Three', + '', + '#### v4.0 — Level Four', + ].join('\n'); + + const result = listMilestoneHeadings(content); + const versions = result.map((h) => h.version); + assert.deepStrictEqual(versions, ['v1.0', 'v2.0', 'v3.0']); +}); + +test('listMilestoneHeadingsFlagsClosedMilestones', () => { + const content = ['## v1.0 — Closed ✅ SHIPPED', '', '## v2.0 — Live 🚧'].join('\n'); + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 2); + assert.strictEqual(result[0].closed, true); + assert.strictEqual(result[1].closed, false); +}); + +// (RED) Identity (4c), CLI: `roadmap analyze`'s `milestones[]` must agree +// with `listMilestoneHeadings` over the SAME scoped content the consumer +// actually used (extractCurrentMilestoneScoped's `.value`), not a +// re-derivation of it. +test('roadmapAnalyzeMilestonesMatchTheOwnersEnumeration', (t) => { + const cwd = createTempDir('gsd-milestone-identity-'); + t.after(() => cleanup(cwd)); + writeState(cwd, { milestone: 'v3.3' }); + const rawContent = [ + '# Roadmap', + '', + '## v3.3 — Portability (Windows)', + '', + '### Phase 7: Close v3.3 gaps', + '', + '### Phase 1: Foo', + ].join('\n'); + writeRoadmap(cwd, rawContent); + + const result = runGsdTools(['roadmap', 'analyze', '--cwd', cwd, '--raw'], cwd); + assert.strictEqual(result.success, true, result.error); + const parsed = JSON.parse(result.output); + + assert.strictEqual(parsed.milestones.length, 1); + assert.ok(parsed.milestones[0].heading.includes('Portability (Windows)'), parsed.milestones[0].heading); + assert.ok(!parsed.milestones.some((m) => /Phase\s+7/.test(m.heading)), 'a Phase heading must never be reported as a milestone'); + + // Agreement with the owner's enumeration over the SAME scoped content the + // consumer actually used. + const ownerContent = extractCurrentMilestoneScoped(rawContent, cwd).value; + const ownerHeadings = listMilestoneHeadings(ownerContent); + assert.strictEqual(parsed.milestones.length, ownerHeadings.length); + for (let i = 0; i < ownerHeadings.length; i += 1) { + assert.strictEqual(parsed.milestones[i].version, ownerHeadings[i].version); + } +}); + +// Parity (anti-drift seam): `listMilestoneHeadings` (version-agnostic +// enumeration) and `locateMilestoneHeadings` (version-filtered locator) must +// agree on WHICH milestone headings are selected for a given version — NOT +// on raw heading text. The two are legitimately different representations: +// `locateMilestoneHeadings` returns raw `RegExpExecArray`s (`m[1]` is the +// full matched line, `#`s included), while `listMilestoneHeadings` returns +// structured entries whose `heading` field has the `#{1,3}` prefix and +// following whitespace stripped (matching the inline `cmdRoadmapAnalyze` +// regex this replaced). Comparing raw strings between the two would make +// this test assert an accidental implementation detail rather than the +// actual parity contract, and would silently regress if either owner's +// textual representation ever changed for an unrelated reason. +test('versionFilteredEnumerationMatchesTheLocator', () => { + const content = [ + '# Roadmap', + '', + '## v1.0 — First ✅ SHIPPED', + '', + '## v2.0 — Second', + '', + '## v2.0 — Second Redux 🚧', + '', + '### Phase 1: Foo', + '', + '## v3.0 — Third', + ].join('\n'); + + const enumerated = listMilestoneHeadings(content); + const versions = [...new Set(enumerated.map((h) => h.version))]; + assert.ok(versions.length > 1, 'fixture must exercise more than one version'); + + for (const version of versions) { + const enumeratedCount = enumerated.filter((h) => h.version === version).length; + const located = locateMilestoneHeadings(content, version); + assert.strictEqual( + located.length > 0, + enumeratedCount > 0, + `selection disagreement for ${version}: listMilestoneHeadings found ${enumeratedCount}, locateMilestoneHeadings found ${located.length}`, + ); + assert.strictEqual( + located.length, + enumeratedCount, + `selection count mismatch for ${version}`, + ); + } +}); + +// H-P: document-shaped generator (#2371) local to this section -- built from +// a heading/prose grammar, never by calling any renderer in +// roadmap-parser.cjs. Deliberately independent of Section G's blockGen so a +// regression in one generator cannot mask a regression in the other. +const H_SAFE_WORD = fc.stringMatching(/^[A-Za-z][A-Za-z0-9]{0,8}$/); + +const hHeadingBlockGen = fc.record({ + level: fc.integer({ min: 1, max: 4 }), + isPhase: fc.boolean(), + hasVersion: fc.boolean(), + word: H_SAFE_WORD, +}).map((b) => ({ + render() { + const prefix = '#'.repeat(b.level); + const phasePart = b.isPhase ? 'Phase 3: ' : ''; + const versionPart = b.hasVersion ? ' v2.0' : ''; + return `${prefix} ${phasePart}${b.word}${versionPart}`; + }, +})); + +const hProseBlockGen = H_SAFE_WORD.map((word) => ({ render() { return `prose ${word} line`; } })); + +const hBlockGen = fc.oneof(hHeadingBlockGen, hProseBlockGen); + +// Property: for any generated document, no entry `listMilestoneHeadings` +// returns ever has a heading beginning with 'Phase '. +test('propertyEnumerationNeverReturnsAPhaseHeading', (t) => { + const documentGen = fc.array(hBlockGen, { minLength: 1, maxLength: 12 }); + + const report = fc.check( + fc.property(documentGen, (blocks) => { + const content = blocks.map((b) => b.render()).join('\n'); + const result = listMilestoneHeadings(content); + return result.every((h) => !/^Phase\s/i.test(h.heading.replace(/^#{1,3}\s+/, '').trim())); + }), + { seed: 3216, numRuns: 100 }, + ); + if (report.failed) { + t.diagnostic(`H54 counterexample (replay seed=3216): ${JSON.stringify(report.counterexample)}`); + } + assert.strictEqual(report.failed, false, 'listMilestoneHeadings must never enumerate a Phase heading'); +}); diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index fcc3bcf06..f39ecf0d8 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -328,12 +328,16 @@ describe('phases clear: archive-version override (#2288)', () => { ); }); - test('override omitted with no ROADMAP uses getMilestoneInfo default (no-override path unchanged)', () => { - // createTempProject writes no ROADMAP.md, so getMilestoneInfo does NOT throw — - // it returns its documented default version ('v1.0'). The dated `archived-*` - // label is only reached if no safe version label is resolvable at all, which - // this common case is not. This pins the no-override path to its pre-#2288 - // behavior (getMilestoneInfo-derived label), not a dated fallback. + test('override omitted with no ROADMAP archives under the dated fallback label (#3216: getMilestoneInfo default deleted)', () => { + // #3216 (ADR-3180 §7.2 Decision, roadmap-parser.cjs:788-791): getMilestoneInfo's + // plausible-looking {version:'v1.0', name:'milestone'} default — output-identical + // to a genuine v1.0 project — was deleted. With no ROADMAP.md, getMilestoneInfo + // now returns {value:null, scope:SCOPE.UNREADABLE}; archivePhaseDirectories only + // trusts a SCOPE.COMPLETE identity as a directory-name-safe version + // (milestone.cjs:959-965), so a non-COMPLETE scope falls through to the dated + // `archived-` label (milestone.cjs:977-978) instead of 'v1.0'. This + // was previously misfiled under 'v1.0-phases', which read as a genuine v1.0 + // milestone's archive rather than "no resolvable milestone identity". // eslint-disable-next-line local/no-raw-rmsync-in-tests -- ensure no ROADMAP.md (SUT fallback path, not teardown) fs.rmSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), { recursive: true, force: true }); @@ -346,8 +350,19 @@ describe('phases clear: archive-version override (#2288)', () => { assert.ok(result.success, `Command failed: ${result.error}`); assert.ok( - fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases', '01-foundation')), - 'no override + no ROADMAP archives under getMilestoneInfo default (v1.0), unchanged from pre-#2288' + !fs.existsSync(path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases')), + 'no version identity is resolvable — must NOT be misfiled under a plausible-looking v1.0-phases' + ); + const archive = findPhasesArchive(tmpDir); + assert.ok(archive, 'a milestones/*-phases/ archive should still be created'); + assert.match( + path.basename(archive), + /^archived-\d{8}-phases$/, + 'no resolvable milestone identity — must use the dated archived- fallback label' + ); + assert.ok( + fs.existsSync(path.join(archive, '01-foundation')), + 'the phase directory must still be archived (moved, not deleted) under the dated label' ); }); diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index df91773fb..ba04ad082 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -23,6 +23,7 @@ const fs = require('node:fs'); const path = require('node:path'); const roadmapParser = require('../gsd-core/bin/lib/roadmap-parser.cjs'); +const { SCOPE } = require('../gsd-core/bin/lib/planning-scope.cjs'); const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); const { @@ -512,25 +513,26 @@ describe('roadmap-parser: getMilestoneInfo', () => { beforeEach(() => { tmpDir = createTempProject(); }); afterEach(() => { cleanup(tmpDir); }); - test('returns default when ROADMAP.md missing', () => { + test('returns UNREADABLE scope (value null) when ROADMAP.md missing (#3216: the v1.0/"milestone" default was deleted per ADR-3180 §7.2 rule 4)', () => { const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v1.0'); - assert.strictEqual(info.name, 'milestone'); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNREADABLE }); }); test('reads version from STATE.md and heading name', () => { writeState(tmpDir, { milestone: 'v2.0' }); writeRoadmap(tmpDir, '## v2.0: The Big Launch\n### Phase 1: Setup\n'); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v2.0'); - assert.match(info.name, /Big Launch/); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v2.0'); + assert.match(info.value.name, /Big Launch/); }); test('falls back to 🚧 WIP marker when STATE.md has no milestone', () => { writeRoadmap(tmpDir, '## 🚧 **v1.5 Work In Progress**\n### Phase 1: Do stuff\n'); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v1.5'); - assert.match(info.name, /Work In Progress/i); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v1.5'); + assert.match(info.value.name, /Work In Progress/i); }); test('extracts from heading when no STATE.md and no WIP marker', () => { @@ -539,8 +541,9 @@ describe('roadmap-parser: getMilestoneInfo', () => { '### Phase 1: Not started', ].join('\n')); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v3.0'); - assert.match(info.name, /Future Milestone/); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v3.0'); + assert.match(info.value.name, /Future Milestone/); }); test('skips completed ✅ milestones', () => { @@ -550,7 +553,8 @@ describe('roadmap-parser: getMilestoneInfo', () => { ].join('\n')); const info = getMilestoneInfo(tmpDir); // Should not use the ✅-prefixed version as the current milestone - assert.strictEqual(info.version, 'v2.0'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v2.0'); }); }); @@ -580,8 +584,9 @@ describe('roadmap-parser: getMilestoneInfo #2135 — milestone_name clobber', () '### Phase 36: Something', ].join('\n')); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v1.8'); - assert.strictEqual(info.name, 'user session cleanup'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v1.8'); + assert.strictEqual(info.value.name, 'user session cleanup'); }); test('case B: nameless ## heading + 🚧 marker carries the real name', () => { @@ -594,32 +599,36 @@ describe('roadmap-parser: getMilestoneInfo #2135 — milestone_name clobber', () '### Phase 1: Hypothesis', ].join('\n')); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v1.9'); - assert.strictEqual(info.name, 'Falsifiability'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v1.9'); + assert.strictEqual(info.value.name, 'Falsifiability'); }); test('case C: canonical ## vX.Y: Name (no regression)', () => { writeState(tmpDir, { gsd_state_version: '1.0', milestone: 'v2.0' }); writeRoadmap(tmpDir, '## v2.0: The Big Launch\n### Phase 1: Setup\n'); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v2.0'); - assert.strictEqual(info.name, 'The Big Launch'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v2.0'); + assert.strictEqual(info.value.name, 'The Big Launch'); }); test('case D: canonical ## vX.Y — Name (em-dash delimiter stripped)', () => { writeState(tmpDir, { gsd_state_version: '1.0', milestone: 'v2.5' }); writeRoadmap(tmpDir, '## v2.5 — Galaxy Release\n### Phase 1: Start\n'); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v2.5'); - assert.strictEqual(info.name, 'Galaxy Release'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v2.5'); + assert.strictEqual(info.value.name, 'Galaxy Release'); }); test('case E: 🚧 bullet only, no ## heading (no regression)', () => { writeState(tmpDir, { gsd_state_version: '1.0', milestone: 'v1.5' }); writeRoadmap(tmpDir, 'Some intro text.\n\n- 🚧 **v1.5 Quick Fix** — minor\n'); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.version, 'v1.5'); - assert.strictEqual(info.name, 'Quick Fix'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.version, 'v1.5'); + assert.strictEqual(info.value.name, 'Quick Fix'); }); test('anchored regex never matches a ## heading quoted inside backticks mid-line', () => { @@ -632,7 +641,8 @@ describe('roadmap-parser: getMilestoneInfo #2135 — milestone_name clobber', () '## v3.0: Real Name', ].join('\n')); const info = getMilestoneInfo(tmpDir); - assert.strictEqual(info.name, 'Real Name'); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.name, 'Real Name'); }); }); @@ -2726,10 +2736,12 @@ describe('feat-3594: roadmap parser does not crash on ANY corpus fixture', () => // ─── #1881: an unreadable ROADMAP is not an absent one ─────────────────────── // // getRoadmapPhaseInternal returns null for a read failure exactly as it does for -// "phase not found", and getMilestoneInfo returns {v1.0, milestone} — which reads -// as a brand-new project — for a read failure exactly as it does for "no ROADMAP -// yet". Per ADR-1411's "corrupt is not absent" amendment both return values are -// preserved and the cause is surfaced out of band instead. +// "phase not found", and getMilestoneInfo (#3216: now {value, scope}) returns +// {value:null, scope:SCOPE.UNREADABLE} for a read failure exactly as it does for +// "no ROADMAP yet" — the shared return shape stays sentinel-identical between the +// two causes; only the out-of-band diagnostic distinguishes them. Per ADR-1411's +// "corrupt is not absent" amendment both return values are preserved and the +// cause is surfaced out of band instead. // // The discriminator is the errno, and it is load-bearing in the silent direction: // getMilestoneInfo has no existsSync guard, so platformReadSync's null-for-ENOENT @@ -2822,7 +2834,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { const dir = project(t, HEALTHY); let info; const emitted = emissionsDuring(() => { info = getMilestoneInfo(dir); }); - assert.deepStrictEqual(info, { version: 'v2.3', name: 'Alpha' }); + assert.deepStrictEqual(info, { value: { version: 'v2.3', name: 'Alpha' }, scope: SCOPE.COMPLETE }); assert.strictEqual(emitted, 0); }); @@ -2836,14 +2848,14 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { assert.strictEqual(emitted, 1); }); - test('an unreadable roadmap is reported on a milestone lookup, and still returns the default', (t) => { + test('an unreadable roadmap is reported on a milestone lookup, and returns {value:null, scope:UNREADABLE} (#3216: the plausible-looking v1.0 default was deleted)', (t) => { _resetUnusableInputWarningsForTests(); const dir = project(t, HEALTHY); failReads(t, (p) => p.endsWith('ROADMAP.md'), eacces()); let info; const emitted = emissionsDuring(() => { info = getMilestoneInfo(dir); }); - assert.deepStrictEqual(info, { version: 'v1.0', name: 'milestone' }, - 'the plausible-looking default must be preserved exactly'); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNREADABLE }, + 'a read fault must surface as UNREADABLE, never a fabricated version/name'); assert.strictEqual(emitted, 1); }); @@ -2857,7 +2869,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { info = getMilestoneInfo(dir); }); assert.strictEqual(phase, null); - assert.deepStrictEqual(info, { version: 'v1.0', name: 'milestone' }); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNREADABLE }); assert.strictEqual(emitted, 0, 'a missing ROADMAP.md must never be reported as unreadable'); }); @@ -2883,7 +2895,8 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { }); assert.strictEqual(phase, null); assert.strictEqual(emitted, 0); - assert.ok(info, 'still returns a milestone default'); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNSCOPED }, + 'no version token anywhere reachable — UNSCOPED, not a fabricated version'); }); test('an unreadable STATE.md alone stays silent — the inner catch is deliberate', (t) => { @@ -2897,7 +2910,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { failReads(t, (p) => p.endsWith('STATE.md'), eacces()); let info; const emitted = emissionsDuring(() => { info = getMilestoneInfo(dir); }); - assert.deepStrictEqual(info, { version: 'v2.3', name: 'Alpha' }); + assert.deepStrictEqual(info, { value: { version: 'v2.3', name: 'Alpha' }, scope: SCOPE.COMPLETE }); assert.strictEqual(emitted, 0); }); @@ -2980,7 +2993,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { emissionsDuring(() => { assert.doesNotThrow(() => { info = getMilestoneInfo(dir); }); }); - assert.deepStrictEqual(info, { version: 'v1.0', name: 'milestone' }); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNREADABLE }); }); test('getRoadmapPhaseInternal does not throw when the read fails', (t) => { @@ -3014,7 +3027,7 @@ describe('#1881 unreadable ROADMAP vs absent ROADMAP', () => { assert.doesNotThrow(() => { info = getMilestoneInfo(dir); }); assert.doesNotThrow(() => { phase = getRoadmapPhaseInternal(dir, '1'); }); }); - assert.deepStrictEqual(info, { version: 'v1.0', name: 'milestone' }); + assert.deepStrictEqual(info, { value: null, scope: SCOPE.UNREADABLE }); assert.strictEqual(phase, null); assert.strictEqual(emitted, 0, 'the path never resolved, so there is no file to name');