diff --git a/.changeset/daring-hawks-zip.md b/.changeset/daring-hawks-zip.md new file mode 100644 index 000000000..ba3a47243 --- /dev/null +++ b/.changeset/daring-hawks-zip.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4358 +--- +**`milestone_name` no longer corrupts to ")" for a first-milestone ROADMAP whose H1 puts the version after the name** — a punctuation-only heading remainder (e.g. the closing paren of `# Roadmap: Project — Name (v1.13)`) is refused as a name, so `init.*` output reports `null` instead of garbage, and the roadmapper agent now templates the canonical version-free H1. (#4134) diff --git a/CONTEXT.md b/CONTEXT.md index 941250fc2..dcb451fef 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -260,7 +260,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`; 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 `\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`). `hasMilestoneSectioning(content)` is the sibling predicate answering "could a whole-document phase count conflate two different milestones?" — and since #3185 it is decided by milestone VOCABULARY, not by heading position: a heading is a milestone heading iff it is a non-Phase heading (level 1-3) carrying a version token, a shipped/active marker, or the word `Milestone`, and sectioning means two or more of them, since one section cannot conflate siblings. Since #3642, `buildStateFrontmatter`'s flat-vs-milestoned gate consumes the >=1 sibling `hasAnyMilestoneSection` over the same `countMilestoneHeadings` walk instead: asserted-vs-section is a different question than sibling conflation, and needs only ONE section to go wrong — with exactly one section and an asserted milestone absent from the ROADMAP, the whole-document count IS that foreign section's phases, so the #3354 withhold (stored value preserved + warning) governs; a zero-signal (genuinely flat) roadmap keeps the whole-document count per #2828. Three position-based models were tried and each shipped a defect — "any non-Phase heading" over-detects, so a flat ROADMAP carrying an ordinary `## Progress` was called sectioned and its declared phase count discarded for the on-disk directory count (#3204, #2828 regressing at 1.9.1 via #3184's own consolidation); strict nesting misses same-level siblings (regressing #1761) and false-positives on the bundled `templates/roadmap.md` shape, where a `## Phases` wrapper holds a single nested milestone; adjacency false-positives whenever a structural heading merely precedes a phase heading. Known limit: two milestone sections carrying none of the three signals are not detected. `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. `findRoadmapProgressTable(content)` (#1956) locates the `## Progress` table — scoped to that heading via the markdown-sectionizer seam, falling back to the whole document for a headingless milestone slice — so a differently-headed table sharing the `Phase | Plans Complete | Status | Completed` columns cannot be read instead (the #2012 decoy class). `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` expresses the same scope independently for the completion RATIO; the two are held in agreement by a parity test rather than by a shared call, because that symbol's blast radius does not justify a refactor. 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 heading remainder with no letter or digit anywhere is heading structure rather than a curated name and is refused as `name: null`, so a name-then-version H1 (`# Roadmap: Project — Name (v1.13)` — the shape a first-ever ROADMAP.md drifts into) reports the rule-6 TRUNCATED identity instead of the literal `)` left after the version token (#4134), 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`). `hasMilestoneSectioning(content)` is the sibling predicate answering "could a whole-document phase count conflate two different milestones?" — and since #3185 it is decided by milestone VOCABULARY, not by heading position: a heading is a milestone heading iff it is a non-Phase heading (level 1-3) carrying a version token, a shipped/active marker, or the word `Milestone`, and sectioning means two or more of them, since one section cannot conflate siblings. Since #3642, `buildStateFrontmatter`'s flat-vs-milestoned gate consumes the >=1 sibling `hasAnyMilestoneSection` over the same `countMilestoneHeadings` walk instead: asserted-vs-section is a different question than sibling conflation, and needs only ONE section to go wrong — with exactly one section and an asserted milestone absent from the ROADMAP, the whole-document count IS that foreign section's phases, so the #3354 withhold (stored value preserved + warning) governs; a zero-signal (genuinely flat) roadmap keeps the whole-document count per #2828. Three position-based models were tried and each shipped a defect — "any non-Phase heading" over-detects, so a flat ROADMAP carrying an ordinary `## Progress` was called sectioned and its declared phase count discarded for the on-disk directory count (#3204, #2828 regressing at 1.9.1 via #3184's own consolidation); strict nesting misses same-level siblings (regressing #1761) and false-positives on the bundled `templates/roadmap.md` shape, where a `## Phases` wrapper holds a single nested milestone; adjacency false-positives whenever a structural heading merely precedes a phase heading. Known limit: two milestone sections carrying none of the three signals are not detected. `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. `findRoadmapProgressTable(content)` (#1956) locates the `## Progress` table — scoped to that heading via the markdown-sectionizer seam, falling back to the whole document for a headingless milestone slice — so a differently-headed table sharing the `Phase | Plans Complete | Status | Completed` columns cannot be read instead (the #2012 decoy class). `phase-lifecycle.cts`'s `deriveProgressFromRoadmap` expresses the same scope independently for the completion RATIO; the two are held in agreement by a parity test rather than by a shared call, because that symbol's blast radius does not justify a refactor. 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/agents/gsd-roadmapper.md b/agents/gsd-roadmapper.md index b2e28b0ac..4a219fa85 100644 --- a/agents/gsd-roadmapper.md +++ b/agents/gsd-roadmapper.md @@ -331,6 +331,19 @@ After roadmap creation, REQUIREMENTS.md gets updated with phase mappings: **CRITICAL: ROADMAP.md requires TWO phase representations. Both are mandatory.** +### 0. Top-Level Title (H1) + +The H1 carries the PROJECT name only — never a version and never a milestone name: + +```markdown +# Roadmap: [Project Name] +``` + +Milestone identity (version + name) lives in milestone headings (`## vX.Y — [Name]`) or +`## Milestones` bullets (`🚧 **vX.Y [Name]**`), never in the H1. A trailing version in the +H1 (`# Roadmap: [Project] — [Name] (vX.Y)`) corrupts milestone-name extraction (#4134). +`~/.claude/gsd-core/templates/roadmap.md` is the canonical shape. + ### 1. Summary Checklist (under `## Phases`) Use the form matching `phase_id_convention` from config. diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index c7daca942..d4ae05690 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -1589,6 +1589,13 @@ function stripLeadingDelimiter(s: string): string { * one implementation. Returns `null` when `headingText` carries no version * token at all (e.g. a non-milestone heading reached this by mistake). * + * #4134 (§7.2 rule 6 floor): the rule's direction assumes version-then-name. + * A name-then-version heading (`# Roadmap: Project — Name (v1.13)`) leaves a + * punctuation fragment (`)`) after the token; a remainder with no letter or + * digit anywhere is heading structure, not a curated name, and is refused as + * `name: null` so callers report the honest rule-6 answer instead of + * fabricating garbage. Names that merely CONTAIN punctuation are unaffected. + * * @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 @@ -1627,7 +1634,18 @@ function extractMilestoneHeadingName( // 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; + const candidate = stripLeadingDelimiter(afterVersion).replace(/\s*(?:[✅📋🚧]\s*)+$/, '') || null; + // #4134 (§7.2 rule 6 floor): a "name" with no letter or digit anywhere is + // heading structure, not a curated name. The pinned rule takes everything + // AFTER the version token, so a name-then-version heading (`# Roadmap: + // Project — Name (v1.13)` — the shape a first-ever ROADMAP.md drifts into) + // leaves exactly `)` there, which used to be returned as a COMPLETE-scope + // name and propagated into init.* output and STATE.md. Refuse it: callers + // already report the honest rule-6 answer (version kept, `name: null`, + // scope TRUNCATED) for an unresolvable name. A name that merely CONTAINS + // punctuation is untouched — `(` is an ordinary name character (#3171) — + // and digits alone qualify (`## v4.0 — 42` is the name `42`). + const name = candidate !== null && /[\p{L}\p{N}]/u.test(candidate) ? candidate : null; return { version, name }; } diff --git a/tests/init-manager.test.cjs b/tests/init-manager.test.cjs index cebfcedeb..ff0e0f14a 100644 --- a/tests/init-manager.test.cjs +++ b/tests/init-manager.test.cjs @@ -495,6 +495,55 @@ describe('init manager', () => { assert.strictEqual(output.phases[0].is_active, false); }); + // #4134 — a first-ever ROADMAP.md whose H1 puts the version after the name + // (`# Roadmap: — (v1.13)` — the shape nothing + // templates for a project's first milestone) used to make the §7.2 pinned + // name rule return the literal `)` left after the version token as the + // milestone's "name", and init manager displayed it verbatim. The refusal + // reports the honest ADR-3180 §7.2 rule-6 answer instead: the version is + // real, the name is not resolvable from that heading, milestone_name is + // null — never a punctuation fragment. + test('#4134 — milestone_name is null, never ")", for a name-then-version H1', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap: GSD Core — Native OMP Runtime Support (v1.13)', + '', + '## Phases', + '', + '- [ ] **Phase 1: Runtime Adapter Interface**', + '', + '## Phase Details', + '', + '### Phase 1: Runtime Adapter Interface', + '', + '**Goal:** Define the adapter contract.', + '', + ].join('\n') + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.13', + 'milestone_name: Native OMP Runtime Support', + 'status: planning', + '---', + '', + '# Project State', + '', + ].join('\n') + ); + + const result = runGsdTools('init manager', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.milestone_version, 'v1.13'); + assert.strictEqual(output.milestone_name, null); + assert.ok(!result.output.includes('")"'), 'a lone ")" value must not appear anywhere in the JSON'); + }); + // #3314 — ADR-456 subprocess clock pin: cmdInitManager's `is_active` gate // (nowMs - newestMtime < 300000) is CLI-subprocess tested, so only the // GSD_TEST_MODE+GSD_NOW_MS pin can control "now" here (t.mock.timers in the diff --git a/tests/milestone-window-single-owner.test.cjs b/tests/milestone-window-single-owner.test.cjs index 21638cf3f..0ec4837ba 100644 --- a/tests/milestone-window-single-owner.test.cjs +++ b/tests/milestone-window-single-owner.test.cjs @@ -2332,6 +2332,38 @@ test('listMilestoneHeadingsHeadingFieldIsCrlfFreeAndTrimmed', () => { assert.strictEqual(result[1].name, 'Old'); }); +// (RED) #4134: the §7.2 pinned rule takes everything after the heading's OWN +// version token as the name, so a name-then-version heading (`… (v1.13)`) -- +// the shape a first-ever ROADMAP.md drifts into when nothing templates its H1 +// -- leaves exactly `)` after the token, which used to be enumerated as the +// milestone's "name". ADR-3180 §7.2 rule 6: a remainder with no letter or +// digit anywhere is heading structure, not a curated name -- enumerate it as +// `name: null` and let consumers report the honest TRUNCATED identity. The +// enumeration itself (which headings, which version, closed status) is +// unchanged; only the garbage "name" is refused. +test('listMilestoneHeadingsRefusesPunctuationOnlyNames', () => { + const content = [ + '# Roadmap: GSD Core — Native OMP Runtime Support (v1.13)', + '', + '### Phase 1: Runtime Adapter Interface', + ].join('\n'); + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].version, 'v1.13'); + assert.strictEqual(result[0].closed, false); + assert.strictEqual(result[0].name, null, `name must be null, not ${JSON.stringify(result[0].name)}`); +}); + +// (RED) #4134 negative space: a name that merely CONTAINS punctuation is a +// name -- `(` is an ordinary name character and never a terminator (#3171). +// Only a remainder with zero word characters is refused. +test('listMilestoneHeadingsKeepsNamesThatContainPunctuation', () => { + const content = ['# Roadmap', '', '## v3.3 — Name (Part 2: Revenge)'].join('\n'); + const result = listMilestoneHeadings(content); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].name, 'Name (Part 2: Revenge)'); +}); + test('listMilestoneHeadingsRespectsTheOneToThreeLevelBound', () => { const content = [ '# v1.0 — Level One', diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index 355272893..7bfee1888 100644 --- a/tests/no-bare-gsd-tools-command-position.test.cjs +++ b/tests/no-bare-gsd-tools-command-position.test.cjs @@ -105,7 +105,7 @@ const BARE_COMMAND_RE = new RegExp( const PROSE_ALLOWLIST = [ { file: 'agents/gsd-executor.md', line: 823, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, { file: 'agents/gsd-phase-researcher.md', line: 33, reason: 'package-legitimacy provenance rule names the command as the source of an OK verdict; descriptive' }, - { file: 'agents/gsd-roadmapper.md', line: 647, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, + { file: 'agents/gsd-roadmapper.md', line: 660, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction (#4134 shifted it from 647: the H1 template section added above moved the line, the mention is unchanged)' }, { file: 'agents/gsd-intel-updater.md', line: 40, reason: 'cross-platform note names the `gsd-tools intel ` CLI surface descriptively ("CLI invocations go through..."); not an agent instruction' }, { file: 'gsd-core/workflows/execute-plan.md', line: 419, reason: 'describes the downstream SDK validation step (`validated downstream by ...`); names the mechanism, does not instruct the agent to type it' }, ]; diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index f3ab36fc0..6712ce9f5 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -949,6 +949,168 @@ describe('roadmap-parser: getMilestoneInfo #2135 — milestone_name clobber', () }); }); +// ─── getMilestoneInfo — #4134 punctuation-fragment name refusal ─────────────── +// The §7.2 pinned rule takes everything AFTER the heading's own version token +// as the name. For a name-then-version heading (`# Roadmap: Project — Name +// (v1.13)` — the shape a first-ever ROADMAP.md drifts into, since nothing +// templates its H1) that remainder is literally `)`, which the rule used to +// return as a COMPLETE-scope "name". ADR-3180 §7.2 rule 6 is the floor this +// violates: a version known but a name unresolvable is TRUNCATED carrying +// `name: null` — a punctuation-only remainder is heading structure, not a +// curated name (#4134). + +describe('roadmap-parser: getMilestoneInfo #4134 — name-then-version heading', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('#4134 — name-then-version H1 never yields a punctuation-fragment name (rule 6: TRUNCATED, name null)', () => { + writeState(tmpDir, { milestone: 'v1.13' }); + writeRoadmap(tmpDir, [ + '# Roadmap: GSD Core — Native OMP Runtime Support (v1.13)', + '', + '### Phase 1: Runtime Adapter Interface', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v1.13'); + assert.strictEqual(info.value.name, null); + }); + + test('#4134 — ROADMAP-only fallback path also refuses the ")" fragment', () => { + // No STATE.md: the first open milestone heading supplies the version. + writeRoadmap(tmpDir, [ + '# Roadmap: GSD Core — Native OMP Runtime Support (v1.13)', + '', + '### Phase 1: Runtime Adapter Interface', + ].join('\n')); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `scope: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v1.13'); + assert.strictEqual(info.value.name, null); + }); + + test('#4134 — the refusal is level-agnostic (H2/H3 carry the same fragment)', () => { + for (const [level, heading] of [ + [2, '## Native OMP Runtime Support (v1.13)'], + [3, '### Native OMP Runtime Support (v1.13)'], + ]) { + writeState(tmpDir, { milestone: 'v1.13' }); + writeRoadmap(tmpDir, `${heading}\n\n### Phase 1: Setup\n`); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `H${level}: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v1.13'); + assert.strictEqual(info.value.name, null); + } + }); + + test('#4134 — every punctuation-only remainder is refused (garbage family)', () => { + // Each fragment survives stripLeadingDelimiter (it does not START with a + // delimiter char) and carries no letter or digit anywhere — the exact + // shape that used to be returned as a "name". + const fragments = [')', '()', '**', '.,;:', ']}', '🎉']; + for (const fragment of fragments) { + writeState(tmpDir, { milestone: 'v1.2' }); + writeRoadmap(tmpDir, `## v1.2 — ${fragment}\n\n### Phase 1: Setup\n`); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.TRUNCATED, `fragment ${JSON.stringify(fragment)}: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.version, 'v1.2'); + assert.strictEqual(info.value.name, null, `fragment ${JSON.stringify(fragment)} must not become a name`); + } + }); + + test('#4134 control — version-last without parens was already name:null and stays so', () => { + writeState(tmpDir, { milestone: 'v1.2.3' }); + writeRoadmap(tmpDir, '# Milestone Name v1.2.3\n\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.value.version, 'v1.2.3'); + assert.strictEqual(info.value.name, null); + assert.strictEqual(info.scope, SCOPE.TRUNCATED); + }); + + test('#4134 negative space — canonical delimiter forms parse identically', () => { + const cases = [ + ['## v2.0: The Big Launch', 'The Big Launch'], + ['## v2.5 — Galaxy Release', 'Galaxy Release'], + ['## v2.6 – En Dash Form', 'En Dash Form'], + ['## v2.7 - Hyphen Form', 'Hyphen Form'], + ['## v2.8 Space Only Form', 'Space Only Form'], + ]; + for (const [heading, expected] of cases) { + writeState(tmpDir, { milestone: heading.match(/v\d+(?:\.\d+)*/)[0] }); + writeRoadmap(tmpDir, `${heading}\n\n### Phase 1: Setup\n`); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE, `${heading}: ${JSON.stringify(info)}`); + assert.strictEqual(info.value.name, expected, `${heading}: ${JSON.stringify(info.value)}`); + } + }); + + test('#4134 negative space — parenthetical names are retained (#3171)', () => { + writeState(tmpDir, { milestone: 'v1.2' }); + writeRoadmap(tmpDir, '## v1.2 — Name (Part 2)\n\n### Phase 1: Setup\n'); + const info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.name, 'Name (Part 2)'); + }); + + test('#4134 negative space — markers, digit-only names, CRLF headings unchanged', () => { + // Trailing status marker still stripped, not treated as a "name" (a ✅ + // TRAILING marker would make the heading closed and skipped — 📋 does not). + writeState(tmpDir, { milestone: 'v3.0' }); + writeRoadmap(tmpDir, '## v3.0 — Planned 📋\n\n### Phase 1: Setup\n'); + let info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.name, 'Planned'); + + // A digit-only name IS a name (\p{N} counts as a word character). + writeState(tmpDir, { milestone: 'v4.0' }); + writeRoadmap(tmpDir, '## v4.0 — 42\n\n### Phase 1: Setup\n'); + info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.name, '42'); + + // CRLF heading: the trailing \r must never become part of the verdict. + writeState(tmpDir, { milestone: 'v2.0' }); + writeRoadmap(tmpDir, '## v2.0 — CRLF Name\r\n\r\n### Phase 1: Setup\r\n'); + info = getMilestoneInfo(tmpDir); + assert.strictEqual(info.scope, SCOPE.COMPLETE); + assert.strictEqual(info.value.name, 'CRLF Name'); + }); + + test('#4134 — property: a word-char remainder is always a name, a punctuation-only remainder never is', () => { + // Document-shaped generator (#2371): fixed literal token alphabets, NOT + // derived from the parser's own regexes. Fragments are token lists joined + // with single spaces, so no token can glue onto the version token and + // trigger the sub-milestone continuation grammar (`v1.3-B`). + const WORD = fc.constantFrom('Alpha', 'Beta', 'R2D2', '42', '名称', 'küche'); + const PUNCT = fc.constantFrom(')', '(', '—', ':', '.', '**', ']'); + const minor = fc.integer({ min: 0, max: 9 }); + const tokens = fc.array(fc.oneof(WORD, PUNCT), { minLength: 1, maxLength: 6 }); + + const prop = fc.property(minor, tokens, (m, toks) => { + const version = `v1.${m}`; + const fragment = toks.join(' ').trim(); + const hasWordChar = /[\p{L}\p{N}]/u.test(fragment); + const out = roadmapParser.listMilestoneHeadings(`## ${version} ${fragment}\n`); + assert.strictEqual(out.length, 1, `heading not enumerated: ${version} ${fragment}`); + assert.strictEqual(out[0].version, version, `continuation grammar leaked into the version: ${JSON.stringify(out[0])}`); + // The biconditional IS the #4134 contract: a remainder with at least one + // letter/digit is a curated name; one with none is heading structure. + assert.strictEqual( + out[0].name !== null, + hasWordChar, + `fragment ${JSON.stringify(fragment)} (hasWordChar=${hasWordChar}) yielded name ${JSON.stringify(out[0].name)}`, + ); + }); + + const result = fc.check(prop, { seed: 20260905, numRuns: 300 }); + if (result.failed) { + assert.fail(`#4134 property violated (replay seed=20260905): ${JSON.stringify(result.counterexample)}`); + } + }); +}); + // ─── isMilestoneShippedInRoadmap ────────────────────────────────────────────── // #2562: this module owns milestone-heading classification, so its own shipped