* 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<MilestoneInfo|null>; 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<MilestoneInfo|null> 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-<YYYYMMDD> 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 <sim@local>
This commit is contained in:
Tom Boucher
@@ -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-<YYYYMMDD>`) instead of filing phase history under a fabricated
|
||||
`milestones/<version>-phases/` directory.
|
||||
|
||||
### `milestone complete` refuses an untrustworthy window
|
||||
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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<MilestoneInfo | null>`, 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<string, unknown>`. 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.
|
||||
|
||||
Reference in New Issue
Block a user