From bf9fe4630de55b8fc00ac4c82f4f5517ffa6f9d7 Mon Sep 17 00:00:00 2001 From: BeeHiggs Date: Fri, 24 Jul 2026 10:49:53 -0600 Subject: [PATCH] =?UTF-8?q?feat(#2249):=20bracket=20phase-id=20core=20gram?= =?UTF-8?q?mar=20=E2=80=94=20parse/render/toDir=20round-trip=20pair=20(epi?= =?UTF-8?q?c=20#612=20PR-1)=20(#2258)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#2249): bracket phase-id core grammar — parse/render/toDir + READING-B + guards PR-1 of epic #612 (ADR-612, in-tree at docs/adr/612-bracket-phase-id-convention.md). Adds the bracket-convention grammar INSIDE src/phase-id.cts — the ADR-2121 single canonical owner — as a pure, additive extension. The 17 locked exports and PHASE_NUMBER_TOKEN_SOURCE are untouched, and normalizePhaseName is byte-identical, so the PR-0 collision anchor (tests/adr-612-collision-characterization.test.cjs) stays green. New pure round-trippable model (ADR Decision 4): - PhaseId { project, milestone, phase, subphase?, plan? }. - parsePhaseId(input): accepts display `[GSD.02] 05.03-01`, dir/token `GSD.02-05.03-slug`, or bare `GSD.02-05`; rejects ambiguous non-bracket tokens (`02-04`, `05`) rather than guessing. The rejection lives ONLY in this new parser — normalizePhaseName and every legacy reader keep accepting those tokens unchanged (conservative default; no existing path gains a throw). - renderPhaseId(id) -> `[GSD.02] 05.03-01`; toDir(id, slug) -> `GSD.02-05.03-slug` with a slug guard that sanitizes path-traversal input. - getMilestoneFromPhaseId(phaseId, convention?): READING-B derives the milestone from the `[PROJECT.MM]` prefix, gated on convention === 'bracket' and returning the `vN.0` form (parity with READING-A). The optional parameter keeps the helper pure (no config read) and byte-compatible — every existing single-arg caller resolves to the unchanged READING-A body (ADR Decision 6). - extractPhaseToken(dirName, convention?): bracket dir branch GATED on convention === 'bracket'. A bracket dir `{CODE}.{MM}-{PP}` is string-indistinguishable from the legacy #2043/#1324 letter-prefixed-decimal family (`P0.3-2`, `P0.12-34`) whenever the code ends in a digit, so no string-only discriminator is complete — an ungated auto-detect silently reinterpreted legacy reads on this CRITICAL 6-caller helper. The explicit convention signal keeps every existing convention-less call site byte-identical (pinned by a #2043 numeric-tail characterization in tests/phase-id.test.cjs). - comparator: no new code — comparePhaseNum already orders the dot-decimal `PP[.SS]` tokens extractPhaseToken yields; milestone-qualified ordering is a PR-2 resolution concern (bracketQualifiedKey), not core grammar. - SENTINEL_RANGES / isSentinelPhaseId(phaseId, convention?): {0, 999} non-milestone guard; the bracket-prefix reading is gated the same way (an ungated read called `P0.0-foundation` a sentinel), legacy leading-int form unchanged. - BRACKET_PHASE_TOKEN_SOURCE (dot-or-dash `[.-]` sub-separator; deliberately more permissive than parsePhaseId — a read-tolerance source for PR-2, not the emit grammar) and PHASE_HEADING_PREFIX_SRC exported from the drift-guard-exempt owner so PR-2 builds every bracket read regex from the canonical source and check:phase-id-drift stays green stack-wide. The bracket project code follows the repo's config-validated `[A-Z][A-Z0-9_]*` grammar (not the ADR §1 illustration's `[A-Z]{1,6}`), so every project_code the config permits parses. parsePhaseId has no live callers in PR-1, so this grammar choice is forward-facing for PR-2 with zero PR-1 behavior impact. Tests: tests/adr-612-bracket-grammar.test.cjs (28) — ADR §3 example round-trips, full 5-tuple parse, READING-B (+ legacy-unchanged and sentinel cases), extractPhaseToken bracket ON/OFF, comparator ordering of extracted tokens, sentinel + slug guards, bare-token rejection, exported-source behavioral assertions, and two generative fast-check properties: render∘parse identity over well-formed displays, and the toDir/disk↔display bijection. Plus a #2043 numeric-tail characterization (single- AND multi-digit rows) in tests/phase-id.test.cjs pinning the convention-less reading byte-identical. The compiled gsd-core/bin/lib/phase-id.cjs is gitignored (ADR-457 build-at-publish) and rebuilt by CI, so it is intentionally not committed. Co-authored-by: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Fable 5 * chore(#2249): changeset fragment for PR #2258 (docs-exempt: internal grammar behind flag) Co-Authored-By: Claude Fable 5 * fix(#2249): reject non-canonical phase-id input + harden toDir (review B1/M1-M3) PR-1 CHANGES_REQUESTED follow-up (epic #612, ADR-612 Decision 4). B1 (blocker): parsePhaseId accepted non-canonical input (unpadded numbers, over-padded numbers, multi-space separators, stray whitespace), so render(parse(x)) === x did not hold for every well-formed x as ADR-612 Decision 4 requires. Both branches now enforce canonicality by construction: parse permissively, rebuild the canonical string via the same emit path (renderPhaseId for display, a hand-rebuilt token for dir/token), and throw "parsePhaseId: not canonical" on any mismatch. The .trim() at the parser's entry is removed — the match anchors now reject leading/trailing whitespace outright, folding into the existing "not a bracket phase id" rejection. M1 (major): toDir only ever guarded the slug; project/milestone/phase/ subphase were interpolated unsanitized, so a hand-built PhaseId (a structural, not nominal, type) could smuggle a path-traversal segment onto disk. Every field is now validated against the exact shape parsePhaseId itself would produce before use. M2 (major): a slug that sanitized to empty (e.g. '!!!') left a dangling trailing hyphen in the emitted dir name. toDir now throws in that case. M3 (major): an all-digit slug (e.g. '2026') was string-indistinguishable from the dir-branch's plan tail, so it silently broke the disk<->identity bijection on read-back. toDir now rejects all-digit slugs. Nits: toDir now rejects a non-string slug instead of coercing it to the literal token 'undefined'/'null'; sentinel boundary tests added for milestones 1/998/1000 (SENTINEL_RANGES is the two discrete values {0, 999}, not an inclusive range — these were already correct, now locked by test). Test-first: every new assertion (concrete examples + fast-check mutation property for B1; concrete cases for M1-M3 and the nits) was written and confirmed red before the implementation changes, per repo TDD convention. Co-Authored-By: Claude Fable 5 * chore(#2249): reformat changeset body to house convention (review Mi2) The fragment added in ab26190a was a plain paragraph — no bold headline, no trailing issue reference. Reformat to the repo's `**Bold headline** — symptom/explanation. (#issue)` body shape (see e.g. .changeset/agile-pandas-dance.md, .changeset/fierce-pumas-gather.md). Uses (#2249), the issue every commit on this branch references, not the PR number already carried in frontmatter (`pr: 2258`) — the changelog serializer appends `(#{pr})` unconditionally, so a body also ending in `(#2258)` would double-render as `(#2258) (#2258)`. Verified the rendered bullet directly via parseFragment + serializeChangelog: it now reads `... (#2249) (#2258)`, matching the dominant convention across the other fragments (frontmatter pr = merged PR, body reference = originating issue). Also moved the docs-exempt marker back before the paragraph -> after it (matching the file's original order): the marker sits on its own line and is stripped before the body is used, but placing it first left a leading blank line in front of the bold headline once reformatted. Co-Authored-By: Claude Fable 5 * test(#2249): widen property generators — 3+-digit numerics + subphase-pad mutation (re-review Minor 1/2) PR-1 re-review follow-up (epic #612, ADR-612 Decision 4). Test-only: closes two property-generator coverage gaps the reviewer flagged; no source change (src/phase-id.cts and gsd-core/bin/lib/phase-id.cjs are byte-unchanged). Minor 1 (3+-digit numerics never exercised): numArb capped at 99, so no property fed a 3+-digit milestone/phase/subphase/plan through parse/render/ toDir despite CANONICAL_NUMERIC_RE's dedicated `[1-9]\d{2,}` branch. Widen numArb to 1–999 so the round-trip and disk↔display bijection properties both span 3-digit widths (pad2 passes ≥3-digit values through un-truncated with no leading zero, so canonicality still holds). Add a concrete regression pinning the reviewer's hand-traced example: '[GSD.100] 05' round-trips, renders, and toDirs to 'GSD.100-05-feature' without truncation. Minor 2 (no subphase-pad mutation): the B1 mutation-rejection property covered milestone/phase pad + whitespace mutations but never a subphase pad. Add unpad-subphase / overpad-subphase to the mutation set and a generated `includeSub` boolean that decides whether the canonical carries a `.SS` (forced in for the subphase mutations so there is always a `.SS` to mutate); non-subphase mutations keep their original no-subphase coverage. Non-vacuity verified against the compiled lib by temporarily probing each widened/new property and confirming it fails: round-trip counterexample ["A",100,1,…] and bijection counterexample ["A",1,100,…,"a"] prove 3-digit tokens are genuinely generated and reach the body; a no-op unpad-subphase mutation trips the mutated===canonical guard (counterexample ["A",1,1,1,false,"unpad-subphase"]), proving the subphase branch is reached with a subphase present. Probes reverted; numRuns unchanged. Gates: tests/adr-612-bracket-grammar.test.cjs 44 pass / 0 fail; `npm run test:unit` 1079 pass / 0 fail; `npm run lint:ci` exit 0. Co-Authored-By: Claude Opus 4.8 * fix(#2249): consume the #2232 continuation seam at the bracket token's slug-adjacent position (review Major) BRACKET_PHASE_TOKEN_SOURCE was a sixth continuation-recognition site that re-derived the grammar as an unbounded `\d+` literal instead of consuming PHASE_CONTINUATION_SEGMENT_SOURCE, re-opening the #2232 bug class on the bracket path: a PR-2 reader interpolating it over dir `PROJ.01-14-2026-photos-…` (a slug whose first word is a year) over-collected the token as `01-14-2026` instead of `01-14`. Interpolating the cap verbatim at every position was rejected on evidence: the bracket run is `MM-PP[.SS][-LL]` and only the LAST position is slug-adjacent. The exactly-2 cap at the others would under-collect ids toDir itself emits — `PROJ.02-105-slug` (3-digit phase) reads as `02`, `[GSD.02] 05.100` (3-digit sub-phase) as `05` — because CANONICAL_NUMERIC_RE admits `[1-9]\d{2,}` and `[GSD.100] 05` is a pinned regression. Those positions are delimiter- disambiguated (a required field separator; a dot a slug can never contain), not heuristically recognized, so they have no year collision to defend against. Upstream draws the same line for the same reason: core-utils/phase cap the paired PLAN component while the leading phase component stays unbounded. So the run is now positional rather than a free `(?:[.-]\d+)*` repetition, and each position takes the width its delimiter affords: leading unbounded, dash-1 and dot canonical, and the slug-adjacent dash-2 interpolating the single-owner seam. The accepted trade-off is #2232's policy verbatim: a PLAN ≥100 is out of the token grammar. Also derives CANONICAL_NUMERIC_RE from the new BRACKET_CANONICAL_NUMERIC_SOURCE instead of re-spelling it as a literal, so the emit-side gate and the read-side token source are one rule — the same single-owner discipline this fix is about. Behaviour-identical (the anchors make the source's `(?!\d)` guard redundant). Refs #2249 * test(#2249): pin the bracket/#2232 reconciliation — parity surface 6 + divergence gate + property (review Major) The comment block alone cannot hold the divergence: src/phase-id.cts is exempt from the #2128 drift guard by construction, so lint-phase-id-drift.cjs would not catch the bracket token source drifting from the seam. Per the Generative Fix Divergence rule, the divergence is pinned behaviorally instead. Surface 6 joins the existing #2232 parity gate rather than starting a rival one: the review named the bracket token source "a sixth continuation-recognition site", and continuation-grammar-parity.test.cjs is already the invariant-named home where the five #2043 sites agree with the owner on a shared width corpus. Surface 6 asserts the same contract at the bracket run's slug-adjacent position (`01-14--photos-…`, mirroring surface 1 with the extra milestone level), so the bracket path now fails the same gate the other five do. A second block pins the DELIBERATE half — the wider canonical width at the delimiter-disambiguated positions, plus the accepted bound (a plan >=100 is out of the grammar). Without it, "unifying" bracket onto the exactly-2 cap would look like a cleanup rather than a regression. The generative property ties the READ side to the EMIT side metamorphically: for every id toDir can produce, BRACKET_PHASE_TOKEN_SOURCE must collect exactly that id's numeric run — no more, no less. It needed a new arbitrary: the existing slugArb generates one [a-z0-9] word and so can never produce the number-leading slug the collision requires. Probe-falsified, both directions (probes reverted): - reverting the source to the old unbounded `\d+` fails 8: the parity gate reports `"01-14-2026-photos-performance" collected "01-14-2026"` — the review's scenario verbatim — and the property shrinks to ["A",1,1,undefined,"100-a"]. - interpolating the seam at EVERY position (the rejected verbatim option) leaves the repro and parity green but fails the divergence gate `'02' !== '02-105'` and the property at ["A",1,1,100,"100-a"] (3-digit sub-phase), which is the evidence that a verbatim cap under-collects ids toDir emits. Width 2 stays green under both probes — the corpus agrees with the owner exactly where the old and new rules coincide, so the gate discriminates rather than merely mirroring the regex. Refs #2249 * docs(#2249): add the new phase-id exports to the CONTEXT.md glossary bullet (round-4 Major) * test(#2249): pin deterministic grammar boundary cases (re-review m1) PR-1 re-review follow-up (epic #612, ADR-612 Decision 4). Test-only: closes the m1 proof gap — the grammar's bounds were exercised only incidentally through the fast-check domain (1-999, [a-z0-9] slugs). No source change (src/phase-id.cts and gsd-core/bin/lib/phase-id.cjs byte-unchanged). Adds a deterministic boundary block (7 describe groups, +22 tests) pinning the compiled lib's CURRENT behavior — a proof gap, not a behavior gap: - m1.1 numeric-width 99/100/101 at milestone/phase/subphase/plan: parse (display + dir) -> render/toDir round-trip byte-equality. The plan position is identity-symmetric (parse/render accept 99/100/101) but toDir drops it (filename-surface dimension only). - m1.2 read-token width is POSITIONAL: BRACKET_PHASE_TOKEN_SOURCE absorbs 99/100/101 at milestone/phase/subphase (delimiter-disambiguated) but caps the slug-adjacent plan (dash-2) at exactly 2 digits — plan >=100 is out of the token grammar (#2232 seam). Pinned as asymmetry, NOT symmetry. - m1.3 leading-zero 007 -> not-canonical rejection at every position/form. - m1.4 slug abuse: parse DROPS a null-byte/control/unicode/emoji trailing slug (never stored, never mis-read as a plan) and rejects a line terminator; toDir's allow-list sanitizer collapses each to a safe [a-z0-9-] token or rejects sanitize-to-empty. - m1.5 absolute-path slug sanitizes (next to the ../../etc traversal test); an absolute-path project on a hand-built id is rejected by PROJECT_ID_RE; an abs-path string is not a bracket id; an abs-path dir slug is dropped to a clean tuple. - m1.6 whitespace-only -> not-a-bracket-phase-id. - m1.7 very-long input (10k) resolves promptly (ReDoS smoke, behavioral): garbage/partial-prefix throw; a 10k-char slug parses (dropped)/sanitizes. No accept-not-reject case is a src bug: parse never STORES an abusive slug (dropped from the identity tuple) and toDir independently re-sanitizes on emit, so the only slug reaching disk is allow-listed. Plan >=100 accepted by parse is the documented positional design (toDir drops the plan; the read-token caps it) — divergence pinned, not papered over. Probe-falsify: corrupted one assertion in each of the 7 groups (m1.4 both its parse-side and emit-side), ran -> 8 distinct named failures, reverted -> 66/66 green. Confirms every new group executes and can fail. Gates: tests/adr-612-bracket-grammar.test.cjs 66 pass / 0 fail; grammar + continuation-grammar-parity + collision-characterization + phase-id family 175 pass / 0 fail; `npm run lint:ci` exit 0. `npm run test:unit` is green except one pre-existing, unrelated env failure (npm-integrity-gate: a live npm-audit advisory in the production dep tree — reproduces with this change stashed; no package.json/lock change here). Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .changeset/kind-foxes-wave.md | 8 + CONTEXT.md | 2 +- src/phase-id.cts | 291 +++++++- tests/adr-612-bracket-grammar.test.cjs | 796 +++++++++++++++++++++ tests/continuation-grammar-parity.test.cjs | 66 +- tests/phase-id.test.cjs | 21 + 6 files changed, 1180 insertions(+), 4 deletions(-) create mode 100644 .changeset/kind-foxes-wave.md create mode 100644 tests/adr-612-bracket-grammar.test.cjs diff --git a/.changeset/kind-foxes-wave.md b/.changeset/kind-foxes-wave.md new file mode 100644 index 000000000..315a1437f --- /dev/null +++ b/.changeset/kind-foxes-wave.md @@ -0,0 +1,8 @@ +--- +type: Added +pr: 2258 +--- +**Bracket phase-ID core grammar lands behind an opt-in flag** — `parsePhaseId`/`renderPhaseId`/`toDir` add one pure round-trippable `PhaseId` model inside the ADR-2121 canonical owner (`src/phase-id.cts`), gated on `phase_id_convention: 'bracket'`, with generative round-trip properties; legacy `null`/`milestone-prefixed` paths stay byte-untouched (epic #612 PR-1). (#2249) + + + diff --git a/CONTEXT.md b/CONTEXT.md index 6e4e0c155..a2c0c4697 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -15,7 +15,7 @@ Module owning `milestone complete` (archive roadmap/requirements/phases, build M Module that composes Dispatch Policy Module, Query Execution Policy Module, and per-stage handlers (input-validation, plan, execution, result-builder, formatting, error-mapping, observability) into the end-to-end pipeline that produces a `QueryDispatchResult`. The SDK-era pipeline collapsed onto the Command Routing Hub per ADR-0174; current dispatch seam: `gsd-core/bin/lib/command-routing-hub.cjs` (see Command Routing Hub below). ### Phase Id Module -Module owning the pure phase-id parsing and matching helpers: phase-name normalization, phase-token extraction/matching, milestone- and phase-dir id parsing, and phase-markdown regex builders (`escapeRegex`, `normalizePhaseName`, `comparePhaseNum`, `extractPhaseToken`, `phaseTokenMatches`, `phaseMarkdownRegexSource`/`phaseMarkdownRegexSourceExact`, `getMilestoneFromPhaseId`, `getPhaseDirFromPhaseId`). Pure string/regex — no I/O, no config, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2a (#865) as the cycle-free leaf that unblocks the roadmap-parser and phase-locator extractions; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-id.cjs` (generated from `src/phase-id.cts`). +Module owning the pure phase-id parsing and matching helpers: phase-name normalization, phase-token extraction/matching, milestone- and phase-dir id parsing, phase-markdown regex builders, and the ADR-612 bracket phase-id round-trip grammar (`escapeRegex`, `normalizePhaseName`, `comparePhaseNum`, `extractPhaseToken`, `phaseTokenMatches`, `phaseMarkdownRegexSource`/`phaseMarkdownRegexSourceExact`, `getMilestoneFromPhaseId`, `getPhaseDirFromPhaseId`, `parsePhaseId`/`renderPhaseId`/`toDir` over the `PhaseId` type, `isSentinelPhaseId`/`SENTINEL_RANGES`, and the `BRACKET_PHASE_TOKEN_SOURCE`/`PHASE_HEADING_PREFIX_SRC` grammar sources). Pure string/regex — no I/O, no config, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2a (#865) as the cycle-free leaf that unblocks the roadmap-parser and phase-locator extractions; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-id.cjs` (generated from `src/phase-id.cts`). ### Phase Lifecycle Module Module owning phase create, rename, complete, remove, list, and plan-index operations, plus phase-dir prefix validation, STATE.md staleness detection, and auto-prune behaviour. Entry point: `gsd-core/bin/lib/phase.cjs` (CJS surface). Typed phase events: `GSDPhaseStartEvent`, `GSDPhaseStepStartEvent`, `GSDPhaseStepCompleteEvent`, `GSDPhaseCompleteEvent`. (The SDK native-query surface, the `types.ts` event definitions, `phase-runner.ts`, and `phase-prompt.ts` were retired with the SDK package per ADR-0174.) diff --git a/src/phase-id.cts b/src/phase-id.cts index d9f256f4c..b2e3abda1 100644 --- a/src/phase-id.cts +++ b/src/phase-id.cts @@ -72,6 +72,74 @@ function isPhaseContinuationSegment(seg: string): boolean { return PHASE_CONTINUATION_SEGMENT_PREFIX_RE.test(seg); } +// #612 (PR-1): bracket-convention token/heading sources, kept next to the M-NN +// PHASE_NUMBER_TOKEN_SOURCE so this owner file stays the single origin of every +// phase-token grammar. `src/phase-id.cts` is exempt from the #2128 drift guard +// (scripts/lint-phase-id-drift.cjs) by construction, and that guard fails any +// literal re-derivation of the token grammar elsewhere — so the downstream +// bracket readers (PR-2: roadmap/validate/verify) must build their regexes by +// interpolating these exports, never by copying the literal. +// +// The canonical numeric WIDTH of a bracket identity field, mirroring pad2()'s +// output: exactly 2 digits, or 3+ with no leading zero. Owned here as a SOURCE +// so the read side (BRACKET_PHASE_TOKEN_SOURCE, below) and the emit-side +// validator (CANONICAL_NUMERIC_RE, which toDir enforces) are one rule rather +// than two literals that agree today and drift tomorrow. +const BRACKET_CANONICAL_NUMERIC_SOURCE = '(?:[1-9]\\d{2,}|\\d{2})'; + +// BRACKET_PHASE_TOKEN_SOURCE differs from PHASE_NUMBER_TOKEN_SOURCE by a +// dot-OR-dash sub-separator: a bracket dir/heading numeric run is `MM-PP[.SS]` +// (a hyphen joins milestone↔phase, a dot joins phase↔sub-phase), whereas M-NN +// sub-phases are dot-only. +// +// The run is POSITIONAL, not a free repetition — `MM-PP[.SS][-LL]` — and each +// position gets the width its DELIMITER can actually afford: +// +// MM leading unbounded — delimited by the `{CODE}.` prefix +// -PP dash-1 canonical — the grammar REQUIRES this dash, so it is a field +// separator, not a continuation heuristic +// .SS dot canonical — a slug carries no dot (toDir sanitizes them +// away), so this position cannot collide +// -LL dash-2 #2232 cap — the ONLY slug-adjacent position, and therefore +// the only one a slug word can collide with +// +// #2232 reconciliation: the slug-adjacent position interpolates the single-owner +// PHASE_CONTINUATION_SEGMENT_SOURCE, so the #2232 bug class cannot reopen on the +// bracket path — dir `PROJ.01-14-2026-photos-…` (a slug leading with a year) +// yields `01-14`, never `01-14-2026`. +// +// DELIBERATE DIVERGENCE from the M-NN dir-token path (pinned by the parity gate +// in tests/continuation-grammar-parity.test.cjs, which fails if these two rules +// drift for a reason nobody intended): the non-slug-adjacent positions stay +// WIDER than #2232's cap. Bracket admits 3+-digit milestone/phase/sub-phase +// (CANONICAL_NUMERIC_RE — `[GSD.100] 05` is a pinned regression), and unlike the +// M-NN continuations those positions are delimiter-disambiguated rather than +// heuristically recognized, so there is no year collision to defend against. +// Interpolating the cap verbatim at every position would only under-collect ids +// that toDir itself emits: `PROJ.02-105-slug` (3-digit phase) would read as +// `02`, and `[GSD.02] 05.100` (3-digit sub-phase) as `05`. Upstream draws this +// same line for the same reason — core-utils/phase cap the paired PLAN component +// while the leading phase component stays unbounded (phase numbers ≥100 are +// legitimate). The trade-off this accepts is #2232's policy verbatim: a PLAN +// ≥100 is out of the token grammar. +// +// Still deliberately MORE PERMISSIVE than parsePhaseId's strict grammar (it +// admits a letter-suffixed and unpadded leading token that the parser rejects): +// this is a READ-TOLERANCE source for the PR-2 readers, which must recognize a +// bracket-shaped token before deciding what to do with it — it is not the +// emit/identity grammar. parsePhaseId stays the arbiter of well-formedness. +const BRACKET_PHASE_TOKEN_SOURCE = + `\\d+[A-Z]?` + + `(?:-${BRACKET_CANONICAL_NUMERIC_SOURCE}(?!\\d))?` + + `(?:\\.${BRACKET_CANONICAL_NUMERIC_SOURCE}(?!\\d))?` + + `(?:-${PHASE_CONTINUATION_SEGMENT_SOURCE})?`; + +// A phase HEADING intro under bracket is either a `[...]` bracket (optionally +// followed by a `Phase ` label) or a bare `Phase ` label; a bare number is NOT +// a phase-heading intro. The `[^\]]{1,200}` bound mirrors the existing +// roadmap-parser heading regexes (ReDoS-safe: a header is one short line). +const PHASE_HEADING_PREFIX_SRC = '(?:\\[[^\\]]{1,200}\\]\\s*(?:Phase\\s+)?|Phase\\s+)'; + function stripProjectCodePrefix(value: unknown, caseInsensitive = true): string { const input = String(value); const re = caseInsensitive ? PROJECT_CODE_PREFIX_STRIP_RE_I : PROJECT_CODE_PREFIX_STRIP_RE; @@ -107,7 +175,21 @@ function normalizePhaseName(phase: unknown): string { return str; } -function getMilestoneFromPhaseId(phaseId: unknown): string | null { +function getMilestoneFromPhaseId(phaseId: unknown, convention?: string): string | null { + // READING-B (#612): under the bracket convention the milestone comes from the + // `[PROJECT.MM]` / `{CODE}.{MM}-` prefix, never the phase-token leading + // integer (ADR-612 Decision 6). Gated on 'bracket' so the `null` and + // 'milestone-prefixed' (M-NN) paths keep the legacy leading-int rule + // (READING-A) below, byte-untouched. The optional parameter keeps this helper + // pure (no config read) and backward-compatible: every existing single-arg + // caller resolves to the unchanged READING-A body. + if (convention === 'bracket') { + const b = String(phaseId).match(/^([A-Z][A-Z0-9_]*)\.(\d+)/); + if (!b) return null; + const mm = parseInt(b[2], 10); + if (SENTINEL_RANGES.includes(mm)) return null; // sentinel milestones have no real milestone + return `v${mm}.0`; + } const stripped = stripProjectCodePrefix(phaseId); const m = stripped.match(/^0*(\d+)-\d/); if (!m) return null; @@ -131,6 +213,186 @@ function getPhaseDirFromPhaseId(phaseId: unknown, phaseName: string | null | und return projectCode ? `${projectCode}-${base}` : base; } +// ─── Bracket phase-ID grammar (#612, PR-1) ────────────────────────────────── +// One pure round-trippable model (ADR-612 §3 / Decision 4). parsePhaseId +// accepts the display form `[PROJECT.MM] PP[.SS][-LL]` or the on-disk/token form +// `{PROJECT}.{MM}-{PP}[.{SS}][-{LL|slug}]`; renderPhaseId / toDir are its two +// emitters. READING-B: the milestone lives in the `[PROJECT.MM]` prefix, so no +// token dimension is ever overloaded (the M-NN collapse pinned in +// tests/adr-612-collision-characterization.test.cjs cannot occur on this path). +// `plan` is a filename-surface dimension only — renderPhaseId emits it; toDir +// drops it (directories carry a slug, not a plan). The project code follows the +// repo's established `[A-Z][A-Z0-9_]*` grammar (the config-validated +// project_code shape shared with OPTIONAL_PROJECT_CODE_PREFIX_SOURCE), not the +// ADR §1 illustration's `[A-Z]{1,6}`, so that every project_code the config +// permits (digits / underscore / >6 chars) parses. +// +// Strict-reject posture (ADR-612 Decision 4's `render(parse(x)) === x` +// contract, held exactly): parsePhaseId accepts ONLY the canonical form of +// each branch — unpadded numbers, over-padded numbers, and multi-space or +// stray leading/trailing whitespace are all rejected rather than silently +// normalized, so two distinct input strings can never parse to the same +// tuple while one of them fails to round-trip. toDir mirrors this on the +// write side: every interpolated PhaseId field is validated (PhaseId is a +// structural type — nothing forces callers through parsePhaseId, so a hand- +// built id must not be able to smuggle a path-traversal segment onto disk), +// and the slug must sanitize to a non-empty, non-all-digit token (an empty +// slug would leave a dangling trailing hyphen; an all-digit slug is +// string-indistinguishable from the plan grammar's trailing tail and would +// silently break the disk↔identity bijection on read-back). +type PhaseId = { + project: string; // 'GSD' + milestone: string; // '02' (zero-padded, from the bracket/dir prefix) + phase: string; // '05' (zero-padded) + subphase?: string; // '03' (optional) + plan?: string; // '01' (filename surface only) +}; + +const pad2 = (n: string): string => String(parseInt(n, 10)).padStart(2, '0'); + +function parsePhaseId(input: string): PhaseId { + // No .trim(): the match anchors (`^`...`$`) then reject leading/trailing + // whitespace outright, folding that case into the same "not a bracket + // phase id" rejection below rather than needing its own check. + const str = String(input); + + // Display form: [PROJECT.MM] PP[.SS][-LL]. The match itself stays + // permissive on purpose (it will happily match an unpadded number or a + // multi-space run) — canonicality is enforced UNIFORMLY below via the + // render round-trip (ADR-612 Decision 4) rather than by hand-tuning every + // numeric / whitespace sub-pattern, so a field added later inherits the + // check for free instead of needing its own regex micro-surgery. + const disp = str.match(/^\[([A-Z][A-Z0-9_]*)\.(\d+)\]\s+(\d+)(?:\.(\d+))?(?:-(\d+))?$/); + if (disp) { + const id: PhaseId = { project: disp[1], milestone: pad2(disp[2]), phase: pad2(disp[3]) }; + if (disp[4] !== undefined) id.subphase = pad2(disp[4]); + if (disp[5] !== undefined) id.plan = pad2(disp[5]); + // Canonicality by construction: re-render the parsed id and require + // byte-equality with the input. This rejects unpadded ('[GSD.5] 5'), + // over-padded ('[GSD.005] 05'), and multi-space-separated ('[GSD.02] 05') + // variants uniformly, without special-casing any one of them — the emit + // path (renderPhaseId) is the single source of truth for "canonical". + if (renderPhaseId(id) !== str) { + throw new Error(`parsePhaseId: not canonical: ${JSON.stringify(input)}`); + } + return id; + } + + // Dir / token form: {PROJECT}.{MM}-{PP}[.{SS}][-{plan|slug}] + const dir = str.match(/^([A-Z][A-Z0-9_]*)\.(\d+)-(\d+)(?:\.(\d+))?(?:-(.+))?$/); + if (dir) { + const id: PhaseId = { project: dir[1], milestone: pad2(dir[2]), phase: pad2(dir[3]) }; + if (dir[4] !== undefined) id.subphase = pad2(dir[4]); + // Trailing segment: a pure-integer tail is the plan; anything else is a + // slug (dropped from the tuple — it is not an identity dimension). The + // plan tail participates in the canonicality check below; the slug tail + // is read-tolerant pass-through (a slug is not an identity dimension) and + // is exempt from it. + const tail = dir[5]; + const tailIsPlan = tail !== undefined && /^\d+$/.test(tail); + if (tailIsPlan) id.plan = pad2(tail); + + // Canonicality by construction, mirroring the display branch: rebuild the + // exact dir/token string this id would emit and require it match the + // input verbatim. Rejects unpadded milestone/phase ('GSD.2-5') and + // unpadded plan tails ('GSD.02-05-1') without special-casing either. + const sub = id.subphase ? `.${id.subphase}` : ''; + const tailOut = tail === undefined ? '' : tailIsPlan ? `-${pad2(tail)}` : `-${tail}`; + const canonical = `${id.project}.${id.milestone}-${id.phase}${sub}${tailOut}`; + if (canonical !== str) { + throw new Error(`parsePhaseId: not canonical: ${JSON.stringify(input)}`); + } + return id; + } + + // Ambiguous / bare tokens (e.g. `02-04`, `05`, `2-01`) match neither branch, + // as does a display/dir form carrying leading/trailing whitespace (the + // anchors never match it): reject rather than guess a tuple (ADR-612 + // conservative default). The rejection lives ONLY in this new parser — + // normalizePhaseName and every other legacy reader keep accepting those + // tokens unchanged. + throw new Error(`parsePhaseId: not a bracket phase id: ${JSON.stringify(input)}`); +} + +function renderPhaseId(id: PhaseId): string { + const sub = id.subphase ? `.${id.subphase}` : ''; + const plan = id.plan ? `-${id.plan}` : ''; + return `[${id.project}.${id.milestone}] ${id.phase}${sub}${plan}`; +} + +// PhaseId is a structural type: nothing forces a caller through parsePhaseId, +// so toDir cannot trust project/milestone/phase/subphase are already +// canonical — each is validated below against the exact shape parsePhaseId +// itself would ever produce, closing off a hand-built id as a path-traversal +// vector. PROJECT_ID_RE mirrors the parser's `[A-Z][A-Z0-9_]*` grammar; +// CANONICAL_NUMERIC_RE mirrors pad2()'s output shape — exactly 2 digits, or +// 3+ digits with no leading zero. It is BUILT from +// BRACKET_CANONICAL_NUMERIC_SOURCE rather than re-spelled as a literal, so this +// emit-side gate and the read-side token source cannot disagree about what +// "canonical width" means (the anchors here make the source's trailing `(?!\d)` +// guard, which the unanchored read side needs, redundant). +const PROJECT_ID_RE = /^[A-Z][A-Z0-9_]*$/; +const CANONICAL_NUMERIC_RE = new RegExp(`^${BRACKET_CANONICAL_NUMERIC_SOURCE}$`); + +function toDir(id: PhaseId, slug: string): string { + if (!PROJECT_ID_RE.test(id.project)) { + throw new Error(`toDir: invalid project: ${JSON.stringify(id.project)}`); + } + if (!CANONICAL_NUMERIC_RE.test(id.milestone)) { + throw new Error(`toDir: invalid milestone: ${JSON.stringify(id.milestone)}`); + } + if (!CANONICAL_NUMERIC_RE.test(id.phase)) { + throw new Error(`toDir: invalid phase: ${JSON.stringify(id.phase)}`); + } + if (id.subphase !== undefined && !CANONICAL_NUMERIC_RE.test(id.subphase)) { + throw new Error(`toDir: invalid subphase: ${JSON.stringify(id.subphase)}`); + } + // A non-string slug (e.g. an omitted second argument) must not be silently + // coerced by String(...) into the literal token 'undefined'/'null' on disk. + if (typeof slug !== 'string') { + throw new Error(`toDir: slug must be a string: ${JSON.stringify(slug)}`); + } + + const sub = id.subphase ? `.${id.subphase}` : ''; + // Slug guard: the slug becomes an on-disk path segment, so collapse it to a + // safe lowercase token — never a path separator or `..` traversal. + const safeSlug = slug.toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, ''); + // A slug that sanitizes to nothing (e.g. '!!!') would otherwise emit a + // dangling trailing hyphen. + if (!safeSlug) { + throw new Error(`toDir: slug sanitizes to empty: ${JSON.stringify(slug)}`); + } + // An all-digit slug (e.g. '2026') is string-indistinguishable from the + // parsePhaseId dir branch's plan tail, so it would re-parse as a plan, not + // a slug — silently breaking the disk↔identity bijection on read-back. + if (/^\d+$/.test(safeSlug)) { + throw new Error(`toDir: slug must not be all-digit: ${JSON.stringify(slug)}`); + } + return `${id.project}.${id.milestone}-${id.phase}${sub}-${safeSlug}`; +} + +// Milestone integers reserved as non-milestone sentinels (0.x backlog / 999.x +// icebox); a phase id in these ranges has no real milestone. +const SENTINEL_RANGES: readonly number[] = Object.freeze([0, 999]); + +function isSentinelPhaseId(phaseId: unknown, convention?: string): boolean { + const s = String(phaseId); + // Bracket milestone lives in the `{CODE}.{MM}` prefix. GATED on + // convention === 'bracket' for the same reason as extractPhaseToken below and + // getMilestoneFromPhaseId above: that prefix is string-indistinguishable from + // the legacy #1324 letter-prefixed-decimal family (`P0.0-foundation` is a real + // phase, NOT sentinel milestone 0) whenever the code ends in a digit. A + // convention-less caller uses the legacy/bare leading-int rule below, so no + // existing reader gains a false positive; the bracket reading is opt-in. + if (convention === 'bracket') { + const bracket = s.match(/^[A-Z][A-Z0-9_]*\.(\d+)/); // bracket: milestone in the prefix + if (bracket) return SENTINEL_RANGES.includes(parseInt(bracket[1], 10)); + } + const legacy = stripProjectCodePrefix(s).match(/^0*(\d+)/); // legacy/bare: leading int + if (!legacy) return false; + return SENTINEL_RANGES.includes(parseInt(legacy[1], 10)); +} + /** * Render a regex source fragment matching a phase number against ROADMAP/STATE * prose regardless of zero-padding on either side. @@ -225,7 +487,25 @@ function comparePhaseNum(a: unknown, b: unknown): number { /** * Extract the phase token from a directory name. */ -function extractPhaseToken(dirName: string): string { +function extractPhaseToken(dirName: string, convention?: string): string { + // #612 bracket dir form `{CODE}.{MM}-{PP}[.{SS}]-slug` → phase token `PP[.SS]`. + // GATED on convention === 'bracket' (mirrors getMilestoneFromPhaseId's READING-B + // decision above). A bracket dir `{CODE}.{MM}-{PP}` is string-INDISTINGUISHABLE + // from the legacy #2043/#1324 letter-prefixed-decimal family (`P0.3-2`, + // `P0.12-34`) whenever the project code ends in a digit, so NO string-only + // discriminator can separate the two conventions — auto-detecting here silently + // reinterpreted `P0.3-2` → `2` (was `P0.3-2`), a byte-identical-read regression + // on this CRITICAL 6-caller helper (ADR-2121). Requiring an explicit convention + // signal keeps every existing (convention-less) call site byte-identical to + // prior behaviour — see the #2043 numeric-tail characterization in + // tests/phase-id.test.cjs — while keeping the helper pure (optional param, no + // config read). The captured token is dot-only (`PP[.SS]`); the milestone↔phase + // hyphen and any trailing plan/slug are excluded. + if (convention === 'bracket') { + const bracketDir = dirName.match(/^[A-Z][A-Z0-9_]*\.\d+-(\d+(?:\.\d+)?)/); + if (bracketDir) return bracketDir[1]; + } + const codePrefixMatch = dirName.match(PROJECT_CODE_PREFIX_CAPTURE_RE_I); let prefix = ''; let rest = dirName; @@ -386,10 +666,17 @@ export = { PHASE_NUMBER_TOKEN_SOURCE, PHASE_CONTINUATION_SEGMENT_SOURCE, isPhaseContinuationSegment, + BRACKET_PHASE_TOKEN_SOURCE, + PHASE_HEADING_PREFIX_SRC, stripProjectCodePrefix, normalizePhaseName, getMilestoneFromPhaseId, getPhaseDirFromPhaseId, + parsePhaseId, + renderPhaseId, + toDir, + SENTINEL_RANGES, + isSentinelPhaseId, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, comparePhaseNum, diff --git a/tests/adr-612-bracket-grammar.test.cjs b/tests/adr-612-bracket-grammar.test.cjs new file mode 100644 index 000000000..04d1b3ff6 --- /dev/null +++ b/tests/adr-612-bracket-grammar.test.cjs @@ -0,0 +1,796 @@ +'use strict'; + +/** + * PR-1 (#2249 / epic #612) — bracket phase-ID core grammar. + * + * Ratified contract: docs/adr/612-bracket-phase-id-convention.md, Decisions 1/4/6. + * One pure round-trippable model added INSIDE src/phase-id.cts (the ADR-2121 + * single canonical owner): parsePhaseId / renderPhaseId / toDir sharing one + * PhaseId shape, alongside the existing M-NN helpers. READING-B: the milestone + * comes from the `[PROJECT.MM]` / `{CODE}.{MM}-` prefix, never the phase-token + * leading integer. + * + * Sibling of tests/adr-612-collision-characterization.test.cjs (the PR-0 anchor): + * that file locks the CURRENT M-NN collapse (`normalizePhaseName('2-01.02-01') + * === '02'`); this file locks the bracket grammar that resolves the same + * `(milestone, phase, subphase, plan)` identity to exactly one tuple on the + * gated bracket path. + * + * All assertions are BEHAVIORAL: call the exported function, assert its typed + * output. No source-grep. The example tables mirror ADR §3; the two fast-check + * blocks are the generative round-trip / disk-display bijection properties the + * #612 approval requires (ADR Decision 4). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fc = require('fast-check'); + +const core = require('../gsd-core/bin/lib/phase-id.cjs'); + +const p2 = (n) => String(n).padStart(2, '0'); + +// ─── ADR §3 round-trip example table (doc-parity) ─────────────────────────── +const TABLE = [ + { display: '[GSD.02] 05.03-01', dir: 'GSD.02-05.03-feature' }, + { display: '[GSD.02] 05', dir: 'GSD.02-05-feature' }, + { display: '[CK.01] 12.04', dir: 'CK.01-12.04-feature' }, +]; + +describe('bracket grammar: emit/render round-trip pair (ADR §3)', () => { + test('render(parse(display)) === display', () => { + for (const { display } of TABLE) { + assert.strictEqual(core.renderPhaseId(core.parsePhaseId(display)), display); + } + }); + + test('toDir(parse(display), slug) === dir', () => { + for (const { display, dir } of TABLE) { + assert.strictEqual(core.toDir(core.parsePhaseId(display), 'feature'), dir); + } + }); + + test('parse is idempotent across surfaces: parse(dir) and parse(display) agree on the tuple', () => { + for (const { display, dir } of TABLE) { + const a = core.parsePhaseId(display); + const b = core.parsePhaseId(dir); + assert.strictEqual( + `${b.project}.${b.milestone}-${b.phase}`, + `${a.project}.${a.milestone}-${a.phase}`, + ); + } + }); +}); + +// ─── ADR §1 collision-test acceptance (the full 5-tuple, post-fix) ─────────── +describe('bracket grammar: full 5-tuple parse (ADR §1 acceptance)', () => { + test('parsePhaseId resolves a complete milestone/phase/subphase/plan identity', () => { + const parsed = core.parsePhaseId('GSD.02-05.03-01'); + assert.deepStrictEqual(parsed, { + project: 'GSD', + milestone: '02', // from the bracket/dir prefix (READING-B), not the leading int + phase: '05', + subphase: '03', + plan: '01', + }); + assert.strictEqual(core.renderPhaseId(parsed), '[GSD.02] 05.03-01'); + assert.strictEqual(core.toDir(parsed, 'some-feature'), 'GSD.02-05.03-some-feature'); + }); + + test('a plan without a subphase round-trips (display -[LL] with no .SS)', () => { + const id = core.parsePhaseId('[GSD.02] 05-01'); + assert.strictEqual(id.subphase, undefined); + assert.strictEqual(id.plan, '01'); + assert.strictEqual(core.renderPhaseId(id), '[GSD.02] 05-01'); + }); + + test('a bare dir/token arg with no trailing segment parses (contract #2 "bare bracket arg")', () => { + // `GSD.02-05` — the on-disk/CLI token with neither sub-phase, plan, nor slug. + const id = core.parsePhaseId('GSD.02-05'); + assert.deepStrictEqual(id, { project: 'GSD', milestone: '02', phase: '05' }); + assert.strictEqual(core.renderPhaseId(id), '[GSD.02] 05'); + }); +}); + +// ─── 3+-digit numeric width (re-review Minor 1) ───────────────────────────── +// The fast-check generators below (numArb) span 1–999 so the round-trip and +// disk↔display properties genuinely exercise 3+-digit tokens; this concrete +// case pins the reviewer's hand-traced example deterministically. A 3+-digit +// milestone must survive pad2() un-truncated, carry no leading zero, and match +// CANONICAL_NUMERIC_RE's `[1-9]\d{2,}` branch inside toDir. +describe('bracket grammar: 3+-digit milestone width (re-review Minor 1)', () => { + test("'[GSD.100] 05' round-trips, renders, and toDirs without truncation", () => { + const id = core.parsePhaseId('[GSD.100] 05'); + assert.deepStrictEqual(id, { project: 'GSD', milestone: '100', phase: '05' }); + assert.strictEqual(core.renderPhaseId(id), '[GSD.100] 05'); + assert.strictEqual(core.toDir(id, 'feature'), 'GSD.100-05-feature'); + }); +}); + +// ─── Strict-reject: parsePhaseId rejects non-canonical input (review B1) ──── +// render(parse(x)) === x must hold for every WELL-FORMED display string +// (ADR-612 Decision 4). A permissive match that accepts unpadded numbers or +// multi-space separators falsifies that contract the moment the accepted +// string differs from what renderPhaseId would emit for the same tuple — +// parsePhaseId must reject rather than silently normalize. +describe('bracket grammar: parsePhaseId rejects non-canonical input (review B1)', () => { + test('unpadded / over-padded / multi-space display forms are rejected', () => { + assert.throws(() => core.parsePhaseId('[GSD.5] 5'), /parsePhaseId: not canonical/); + assert.throws(() => core.parsePhaseId('[GSD.005] 05'), /parsePhaseId: not canonical/); + assert.throws(() => core.parsePhaseId('[GSD.02] 05'), /parsePhaseId: not canonical/); + }); + + test('leading/trailing whitespace is rejected by the anchors (no .trim() tolerance)', () => { + assert.throws(() => core.parsePhaseId(' [GSD.02] 05'), /parsePhaseId: not a bracket phase id/); + assert.throws(() => core.parsePhaseId('[GSD.02] 05 '), /parsePhaseId: not a bracket phase id/); + }); + + test('unpadded dir/token forms are rejected', () => { + assert.throws(() => core.parsePhaseId('GSD.2-5'), /parsePhaseId: not canonical/); + assert.throws(() => core.parsePhaseId('GSD.02-05-1'), /parsePhaseId: not canonical/); + }); + + test('a canonical form still parses cleanly (no false-positive rejection)', () => { + assert.deepStrictEqual(core.parsePhaseId('[GSD.02] 05'), { project: 'GSD', milestone: '02', phase: '05' }); + assert.deepStrictEqual(core.parsePhaseId('GSD.02-05'), { project: 'GSD', milestone: '02', phase: '05' }); + }); +}); + +// ─── Bare/ambiguous tokens are rejected — only the bracket parser throws ───── +describe('bracket grammar: parsePhaseId rejects ambiguous non-bracket tokens (ADR conservative default #8a)', () => { + test("bare '02-04' (the cross-subsystem ambiguity) is rejected by the bracket parser", () => { + // '02-04' has no bracket and no {CODE}.{MM}- dot-prefix, so it matches + // neither branch — the new bracket parser throws rather than guess a tuple. + // The rejection lives ONLY here; normalizePhaseName still returns it intact + // (see adr-612-collision-characterization.test.cjs), so no existing path + // gains a throw. + assert.throws(() => core.parsePhaseId('02-04'), /not a bracket phase id/); + // The legacy reader is untouched by this rejection. + assert.strictEqual(core.normalizePhaseName('02-04'), '02-04'); + }); + + test('other non-bracket forms are rejected', () => { + assert.throws(() => core.parsePhaseId('05'), /not a bracket phase id/); + assert.throws(() => core.parsePhaseId('2-01'), /not a bracket phase id/); + assert.throws(() => core.parsePhaseId(''), /not a bracket phase id/); + }); +}); + +// ─── READING-B milestone source (gated on 'bracket'; legacy paths intact) ──── +describe('bracket grammar: getMilestoneFromPhaseId READING-B', () => { + test("milestone comes from the [PROJECT.MM] prefix, not the phase-token leading int", () => { + // 'GSD.02-05.03' → milestone 02 (v2.0), NOT phase 05 (v5.0). ADR Decision 6. + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.02-05.03', 'bracket'), 'v2.0'); + }); + + test('sentinel milestone ranges (0.x / 999.x) resolve to null', () => { + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.00-01', 'bracket'), null); + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.999-01', 'bracket'), null); + }); + + test('a bracket-convention call on a non-bracket string returns null (no throw)', () => { + // Negative branch: convention === 'bracket' but the string has no + // {CODE}.{MM} prefix → null, not an exception. + assert.strictEqual(core.getMilestoneFromPhaseId('2-01', 'bracket'), null); + }); + + test("legacy M-NN path is byte-unchanged when convention is absent / not 'bracket'", () => { + // READING-A leading-int rule, current behavior — must not regress. + assert.strictEqual(core.getMilestoneFromPhaseId('2-01'), 'v2.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('CK-2-01'), 'v2.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('2-01', 'milestone-prefixed'), 'v2.0'); + // Sentinel + non-milestone forms preserved on the legacy path. + assert.strictEqual(core.getMilestoneFromPhaseId('0-01'), null); + assert.strictEqual(core.getMilestoneFromPhaseId('05'), null); + }); +}); + +// ─── extractPhaseToken: bracket dir form (gated on convention) ─────────────── +// The bracket dir `{CODE}.{MM}-{PP}` is string-indistinguishable from the legacy +// #2043 letter-prefixed-decimal family (`P0.3-2`) when the code ends in a digit, +// so the new bracket reader fires ONLY under an explicit `convention` arg — the +// same gating decision as getMilestoneFromPhaseId's READING-B. Convention-less +// callers keep the legacy reading byte-identical (pinned across the whole +// numeric-tail family in tests/phase-id.test.cjs). +describe('bracket grammar: extractPhaseToken (bracket path gated on convention)', () => { + test("extracts the phase token PP[.SS] from a bracket dir under convention 'bracket'", () => { + assert.strictEqual(core.extractPhaseToken('CK.02-02.01-slug', 'bracket'), '02.01'); + assert.strictEqual(core.extractPhaseToken('GSD.02-05-feature', 'bracket'), '05'); + assert.strictEqual(core.extractPhaseToken('GSD.02-05.03-01', 'bracket'), '05.03'); // plan is not part of the token + }); + + test('the bracket path is OFF by default: a convention-less call keeps the legacy reading', () => { + // Without the signal the bracket branch is skipped; `GSD.02` matches neither + // the leading-int nor the letter-prefix legacy rule, so the whole dir name is + // returned unchanged (prior behaviour) rather than parsed as a bracket dir. + assert.strictEqual(core.extractPhaseToken('GSD.02-05.03-01'), 'GSD.02-05.03-01'); + }); + + test('legacy code-prefixed dirs still extract as before (no regression)', () => { + assert.strictEqual(core.extractPhaseToken('CK-01-foo'), 'CK-01'); + assert.strictEqual(core.extractPhaseToken('02-04-some-slug'), '02-04'); + }); +}); + +// ─── comparator: existing comparePhaseNum orders extracted bracket tokens ───── +// ADR §Phases PR-1 row names "comparator". No NEW comparator code is required: +// bracket phase tokens flow through extractPhaseToken (which yields the +// dot-decimal `PP[.SS]` form), and comparePhaseNum's existing numeric+decimal +// branch already orders that form. Cross-MILESTONE ordering (a milestone- +// qualified key) is a resolution/lookup concern deferred to PR-2 +// (bracketQualifiedKey), not core grammar — the last case below shows the +// boundary: two tokens from different milestones compare equal because the +// extracted token is milestone-blind by construction. +describe('bracket grammar: comparePhaseNum orders extracted bracket phase tokens', () => { + const tok = (d) => core.extractPhaseToken(d, 'bracket'); + test('phase order: 05 < 12', () => { + assert.ok(core.comparePhaseNum(tok('GSD.02-05'), tok('GSD.02-12')) < 0); + assert.ok(core.comparePhaseNum(tok('GSD.02-12'), tok('GSD.02-05')) > 0); + }); + + test('sub-phase order: 05.03 < 05.10, and a bare phase sorts before its sub-phases', () => { + assert.ok(core.comparePhaseNum(tok('GSD.02-05.03'), tok('GSD.02-05.10')) < 0); + assert.ok(core.comparePhaseNum(tok('GSD.02-05'), tok('GSD.02-05.03')) < 0); + }); + + test('the extracted token is milestone-blind: same PP[.SS] across milestones compares equal (PR-2 owns qualified ordering)', () => { + assert.strictEqual(core.comparePhaseNum(tok('GSD.02-05.03'), tok('CK.09-05.03')), 0); + }); +}); + +// ─── sentinel guard: isSentinelPhaseId / SENTINEL_RANGES ──────────────────── +describe('bracket grammar: sentinel guard', () => { + test('SENTINEL_RANGES are the {0, 999} milestone ranges', () => { + assert.deepStrictEqual([...core.SENTINEL_RANGES], [0, 999]); + }); + + test('isSentinelPhaseId is true for milestone 0 / 999 across forms', () => { + // Bracket forms: milestone in the `{CODE}.{MM}` prefix (gated on convention). + assert.strictEqual(core.isSentinelPhaseId('GSD.999-01', 'bracket'), true); + assert.strictEqual(core.isSentinelPhaseId('GSD.00-01', 'bracket'), true); + // Legacy/bare leading-int forms need no convention. + assert.strictEqual(core.isSentinelPhaseId('999.1'), true); + assert.strictEqual(core.isSentinelPhaseId('0.1'), true); + }); + + test('isSentinelPhaseId is false for ordinary milestones and for tokens with no leading integer', () => { + assert.strictEqual(core.isSentinelPhaseId('GSD.02-05', 'bracket'), false); + assert.strictEqual(core.isSentinelPhaseId('2-01'), false); + // Negative branch: a string with no leading integer at all → false. + assert.strictEqual(core.isSentinelPhaseId('feature-branch'), false); + }); + + test('the bracket sentinel path is OFF by default: a convention-less #1324 dir is not a sentinel', () => { + // `P0.0-foundation` is a real #1324 letter-prefixed phase, NOT milestone-0 + // sentinel. Auto-detecting the `P0`/`.0` prefix would be a false positive + // (the same root ambiguity gated in extractPhaseToken), so without the + // convention signal the legacy leading-int rule applies and returns false. + assert.strictEqual(core.isSentinelPhaseId('P0.0-foundation'), false); + assert.strictEqual(core.isSentinelPhaseId('P0.999-x'), false); + }); + + // SENTINEL_RANGES is the two DISCRETE values {0, 999} (an `.includes()` + // membership test), not an inclusive numeric range — so a milestone just + // inside either boundary (1, 998) and one just past the upper boundary + // (1000) are all ordinary, non-sentinel milestones. Locks that boundary + // shape across both the bracket and legacy reading paths. + test('milestones 1, 998, and 1000 are NOT sentinels (boundary probe on the {0, 999} discrete set)', () => { + assert.strictEqual(core.isSentinelPhaseId('GSD.01-01', 'bracket'), false); + assert.strictEqual(core.isSentinelPhaseId('GSD.998-01', 'bracket'), false); + assert.strictEqual(core.isSentinelPhaseId('GSD.1000-01', 'bracket'), false); + assert.strictEqual(core.isSentinelPhaseId('1-01'), false); + assert.strictEqual(core.isSentinelPhaseId('998-01'), false); + assert.strictEqual(core.isSentinelPhaseId('1000-01'), false); + }); + + test('getMilestoneFromPhaseId resolves 1, 998, and 1000 to real milestones (not the sentinel null)', () => { + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.01-01', 'bracket'), 'v1.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.998-01', 'bracket'), 'v998.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('GSD.1000-01', 'bracket'), 'v1000.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('1-01'), 'v1.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('998-01'), 'v998.0'); + assert.strictEqual(core.getMilestoneFromPhaseId('1000-01'), 'v1000.0'); + }); +}); + +// ─── slug guard: toDir never emits a path-traversal slug ──────────────────── +describe('bracket grammar: toDir slug guard', () => { + test('a hostile slug is sanitized to a safe filesystem token', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + const dir = core.toDir(id, '../../etc/passwd'); + assert.ok(!dir.includes('/'), `dir must not contain a path separator: ${dir}`); + assert.ok(!dir.includes('..'), `dir must not contain '..': ${dir}`); + assert.strictEqual(dir, 'GSD.02-05-etc-passwd'); + }); + + test('a clean slug is preserved (round-trip unaffected)', () => { + assert.strictEqual(core.toDir(core.parsePhaseId('[GSD.02] 05.03-01'), 'feature'), 'GSD.02-05.03-feature'); + }); +}); + +// ─── toDir field validation: every interpolated PhaseId field is checked +// (review M1) ────────────────────────────────────────────────────────────── +// PhaseId is a structural (not nominal) type: nothing stops a caller from +// hand-building one and skipping parsePhaseId entirely. Only the slug was +// ever guarded, so a hand-built id with a hostile project/milestone/phase +// still reached the on-disk path unsanitized — a live traversal, not merely +// a theoretical one. +describe('bracket grammar: toDir validates every interpolated field (review M1)', () => { + test('a path-traversal project is rejected', () => { + const id = { project: '../../etc', milestone: '02', phase: '05' }; + assert.throws(() => core.toDir(id, 'feature'), /toDir:.*project/); + }); + + test('a lowercase project is rejected (does not match the project_code grammar)', () => { + const id = { project: 'gsd', milestone: '02', phase: '05' }; + assert.throws(() => core.toDir(id, 'feature'), /toDir:.*project/); + }); + + test('an unpadded milestone is rejected (not the canonical pad2 shape)', () => { + const id = { project: 'GSD', milestone: '5', phase: '05' }; + assert.throws(() => core.toDir(id, 'feature'), /toDir:.*milestone/); + }); + + test('a non-numeric phase is rejected', () => { + const id = { project: 'GSD', milestone: '02', phase: '../etc' }; + assert.throws(() => core.toDir(id, 'feature'), /toDir:.*phase/); + }); + + test('a non-numeric subphase is rejected when present', () => { + const id = { project: 'GSD', milestone: '02', phase: '05', subphase: '../etc' }; + assert.throws(() => core.toDir(id, 'feature'), /toDir:.*subphase/); + }); +}); + +// ─── toDir slug emptiness: no dangling trailing hyphen (review M2) ────────── +describe('bracket grammar: toDir rejects a slug that sanitizes to empty (review M2)', () => { + test('a slug of only punctuation is rejected rather than emitting a trailing hyphen', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + assert.throws(() => core.toDir(id, '!!!'), /toDir:.*slug/); + assert.throws(() => core.toDir(id, '...'), /toDir:.*slug/); + }); +}); + +// ─── toDir all-digit slug: collides with the plan grammar (review M3) ─────── +describe('bracket grammar: toDir rejects an all-digit slug (review M3)', () => { + test("a slug of '2026' is rejected — it would re-parse as a plan, not a slug", () => { + const id = core.parsePhaseId('[GSD.02] 05'); + assert.throws(() => core.toDir(id, '2026'), /toDir:.*slug/); + }); +}); + +// ─── toDir slug type: non-string slugs are rejected, not stringified (nit) ── +describe('bracket grammar: toDir rejects a non-string slug', () => { + test('undefined and null are rejected rather than coerced to the literal word', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + assert.throws(() => core.toDir(id, undefined), /toDir:.*slug/); + assert.throws(() => core.toDir(id, null), /toDir:.*slug/); + }); +}); + +// ═══ m1: deterministic boundary coverage (re-review m1) ═════════════════════ +// The fast-check domain (1–999, [a-z0-9] slugs) exercises the grammar's bounds +// only INCIDENTALLY. This section PINS them: the 2↔3-digit numeric-width +// boundary, a leading-zero 3-digit value, abusive slug content (null byte, +// control chars, unicode, absolute paths), whitespace-only, and very-long +// input. Every input below is a hand-written literal (never seeded from the +// renderer), and every expectation is the compiled lib's CURRENT behavior — +// this closes a proof gap, not a behavior gap. Placed against the toDir slug/ +// field-validation cluster above so the absolute-path cases sit next to the +// `../../etc` traversal test they extend. + +// ─── m1.1: numeric-width boundary 99/100/101 (identity grammar) ────────────── +// pad2() passes a ≥3-digit value through un-truncated and it carries no leading +// zero, so 99/100/101 are all canonical at the milestone/phase/subphase +// positions: parse→render round-trips and toDir emits them byte-for-byte +// (CANONICAL_NUMERIC_RE's `[1-9]\d{2,}` branch admits 100/101). The plan +// position is IDENTITY-symmetric too (parse/render accept all three) but is a +// filename-surface dimension only — toDir drops it from the dir string. +describe('bracket grammar: numeric-width boundary 99/100/101 (review m1)', () => { + test('milestone width 99/100/101 round-trips through display, dir, and toDir', () => { + for (const n of ['99', '100', '101']) { + assert.deepStrictEqual(core.parsePhaseId(`[GSD.${n}] 05`), { project: 'GSD', milestone: n, phase: '05' }, n); + assert.strictEqual(core.renderPhaseId(core.parsePhaseId(`[GSD.${n}] 05`)), `[GSD.${n}] 05`, n); + assert.strictEqual(core.parsePhaseId(`GSD.${n}-05`).milestone, n, n); + assert.strictEqual(core.toDir(core.parsePhaseId(`[GSD.${n}] 05`), 'feat'), `GSD.${n}-05-feat`, n); + } + }); + + test('phase width 99/100/101 round-trips through display, dir, and toDir', () => { + for (const n of ['99', '100', '101']) { + assert.deepStrictEqual(core.parsePhaseId(`[GSD.02] ${n}`), { project: 'GSD', milestone: '02', phase: n }, n); + assert.strictEqual(core.renderPhaseId(core.parsePhaseId(`[GSD.02] ${n}`)), `[GSD.02] ${n}`, n); + assert.strictEqual(core.parsePhaseId(`GSD.02-${n}`).phase, n, n); + assert.strictEqual(core.toDir(core.parsePhaseId(`[GSD.02] ${n}`), 'feat'), `GSD.02-${n}-feat`, n); + } + }); + + test('subphase width 99/100/101 round-trips through display, dir, and toDir', () => { + for (const n of ['99', '100', '101']) { + assert.strictEqual(core.parsePhaseId(`[GSD.02] 05.${n}`).subphase, n, n); + assert.strictEqual(core.renderPhaseId(core.parsePhaseId(`[GSD.02] 05.${n}`)), `[GSD.02] 05.${n}`, n); + assert.strictEqual(core.parsePhaseId(`GSD.02-05.${n}`).subphase, n, n); + assert.strictEqual(core.toDir(core.parsePhaseId(`[GSD.02] 05.${n}`), 'feat'), `GSD.02-05.${n}-feat`, n); + } + }); + + test('plan width 99/100/101: parse/render accept all three (identity-symmetric); toDir drops the plan', () => { + for (const n of ['99', '100', '101']) { + // Identity grammar is symmetric at the plan position — plan >=100 is + // accepted and round-trips (the plan-position cap lives ONLY in the + // read-token source, pinned in m1.2 below, NOT in parsePhaseId). + assert.strictEqual(core.parsePhaseId(`[GSD.02] 05-${n}`).plan, n, n); + assert.strictEqual(core.renderPhaseId(core.parsePhaseId(`[GSD.02] 05-${n}`)), `[GSD.02] 05-${n}`, n); + assert.strictEqual(core.parsePhaseId(`GSD.02-05-${n}`).plan, n, n); + // toDir emits the dir with NO plan segment (plan is filename-surface only), + // so all three widths collapse to the same slug-bearing dir. + assert.strictEqual(core.toDir(core.parsePhaseId(`[GSD.02] 05-${n}`), 'feat'), 'GSD.02-05-feat', n); + } + }); +}); + +// ─── m1.2: read-token width is POSITIONAL (plan capped, others admit 3+) ────── +// BRACKET_PHASE_TOKEN_SOURCE is the PR-2 READ-tolerance source, applied after +// the `{CODE}.` prefix is stripped, so its run is MM-PP[.SS][-LL]. milestone +// (leading, unbounded), phase (dash-1) and subphase (dot) are delimiter- +// disambiguated and admit the canonical `[1-9]\d{2,}` width; the slug-adjacent +// plan (dash-2) consumes the single-owner #2232 continuation seam `\d{2}(?!\d)`, +// so a plan >=100 is DELIBERATELY out of the token grammar. This is the landed +// positional divergence (see the block at the foot of +// tests/continuation-grammar-parity.test.cjs), pinned here at the 99/100/101 +// boundary — asymmetry expected, NOT symmetry with the other positions. +describe('bracket grammar: read-token width is positional at 99/100/101 (review m1)', () => { + const tok = (run) => run.match(new RegExp(`^${core.BRACKET_PHASE_TOKEN_SOURCE}`))?.[0]; + + test('milestone / phase / subphase absorb 99, 100, and 101', () => { + for (const n of ['99', '100', '101']) { + assert.strictEqual(tok(`${n}-05`), `${n}-05`, `milestone ${n} (leading, unbounded)`); + assert.strictEqual(tok(`02-${n}`), `02-${n}`, `phase ${n} (dash-1, delimiter-disambiguated)`); + assert.strictEqual(tok(`02-05.${n}`), `02-05.${n}`, `subphase ${n} (dot, delimiter-disambiguated)`); + } + }); + + test('plan (dash-2, slug-adjacent) absorbs 99 but NOT 100/101 — the #2232 cap holds', () => { + assert.strictEqual(tok('02-05-99'), '02-05-99', 'a canonical 2-digit plan is absorbed'); + assert.strictEqual(tok('02-05-100'), '02-05', 'plan 100 is capped out of the token run'); + assert.strictEqual(tok('02-05-101'), '02-05', 'plan 101 is capped out of the token run'); + }); +}); + +// ─── m1.3: leading-zero 3-digit value (007) is non-canonical everywhere ─────── +// pad2('007') === '07' (parseInt drops the leading zeros), so the re-render / +// re-emit can never match the input — parsePhaseId rejects '007' as not- +// canonical at milestone/phase/subphase/plan, in BOTH the display and dir forms. +describe('bracket grammar: leading-zero 3-digit value (007) is rejected (review m1)', () => { + test('display form rejects 007 in milestone / phase / subphase / plan', () => { + for (const s of ['[GSD.007] 05', '[GSD.02] 007', '[GSD.02] 05.007', '[GSD.02] 05-007']) { + assert.throws(() => core.parsePhaseId(s), /parsePhaseId: not canonical/, s); + } + }); + + test('dir form rejects 007 in milestone / phase / subphase / plan', () => { + for (const s of ['GSD.007-05', 'GSD.02-007', 'GSD.02-05.007', 'GSD.02-05-007']) { + assert.throws(() => core.parsePhaseId(s), /parsePhaseId: not canonical/, s); + } + }); +}); + +// ─── m1.4: slug content abuse — null byte / control / unicode ───────────────── +// Two layers, each pinned independently: +// READ (parsePhaseId): a dir trailing segment is a read-tolerant slug, DROPPED +// from the identity tuple. Any non-line-terminator content — null byte, +// control char, accented letter, emoji — is accepted and dropped (never +// stored, so it cannot smuggle a bad identity, and is never mis-read as a +// plan). A LINE TERMINATOR (\n / \r) is rejected outright because the dir +// regex's `.+` cannot cross it. +// EMIT (toDir): the allow-list sanitizer `.replace(/[^a-z0-9]+/g,'-')` +// collapses every non-[a-z0-9] run to a single hyphen (null / control / tab / +// newline / path separator alike) and drops non-ASCII bytes; content that +// sanitizes to nothing is rejected rather than emitting a dangling hyphen. +describe('bracket grammar: slug content abuse — null byte / control / unicode (review m1)', () => { + const EMOJI = '\u{1F4A5}'; // 💥 + const ACCENTED = 'café'; // café + + test('parsePhaseId drops an abusive (non-line-terminator) trailing slug, keeping a clean tuple', () => { + for (const bad of ['foo\x00bar', 'foo\x07bar', 'foo\tbar', ACCENTED, EMOJI]) { + const id = core.parsePhaseId(`GSD.02-05-${bad}`); + assert.deepStrictEqual(id, { project: 'GSD', milestone: '02', phase: '05' }, JSON.stringify(bad)); + } + }); + + test('a line terminator (\\n / \\r) in the slug position is rejected by the anchors', () => { + assert.throws(() => core.parsePhaseId('GSD.02-05-foo\nbar'), /not a bracket phase id/); + assert.throws(() => core.parsePhaseId('GSD.02-05-foo\rbar'), /not a bracket phase id/); + }); + + test('toDir sanitizes abusive slug content to a safe [a-z0-9-] token', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + assert.strictEqual(core.toDir(id, 'foo\x00bar'), 'GSD.02-05-foo-bar', 'null byte → hyphen'); + assert.strictEqual(core.toDir(id, 'foo\x07bar'), 'GSD.02-05-foo-bar', 'control char → hyphen'); + assert.strictEqual(core.toDir(id, 'a\nb'), 'GSD.02-05-a-b', 'newline → hyphen (safe on emit, unlike read)'); + assert.strictEqual(core.toDir(id, 'a\rb'), 'GSD.02-05-a-b', 'carriage return → hyphen'); + assert.strictEqual(core.toDir(id, ACCENTED), 'GSD.02-05-caf', 'non-ASCII dropped, trailing hyphen stripped'); + }); + + test('toDir rejects a slug that sanitizes to empty (emoji-only / null-only) rather than a dangling hyphen', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + assert.throws(() => core.toDir(id, EMOJI), /toDir:.*slug/); + assert.throws(() => core.toDir(id, '\x00'), /toDir:.*slug/); + }); +}); + +// ─── m1.5: absolute-path slug or project ───────────────────────────────────── +// Sibling of the `../../etc/passwd` traversal test above (toDir slug guard) and +// the hand-built-id field-validation block (review M1): the absolute-path +// (leading `/`) shapes, pinned next to the traversal shapes they extend. +describe('bracket grammar: absolute-path slug or project (review m1)', () => { + test('an absolute-path slug is sanitized — leading slash and separators collapse away', () => { + const id = core.parsePhaseId('[GSD.02] 05'); + const dir = core.toDir(id, '/etc/passwd'); + assert.ok(!dir.includes('/'), `dir must not contain a path separator: ${dir}`); + assert.strictEqual(dir, 'GSD.02-05-etc-passwd'); + }); + + test('an absolute-path project on a hand-built id is rejected by PROJECT_ID_RE', () => { + assert.throws(() => core.toDir({ project: '/etc/passwd', milestone: '02', phase: '05' }, 'feat'), /toDir:.*project/); + assert.throws(() => core.toDir({ project: '/etc', milestone: '02', phase: '05' }, 'feat'), /toDir:.*project/); + }); + + test('an absolute-path string is not a bracket phase id', () => { + assert.throws(() => core.parsePhaseId('/etc/passwd'), /not a bracket phase id/); + }); + + test('an absolute-path slug embedded in a dir string is dropped, leaving a clean tuple (no `/` in any field)', () => { + const id = core.parsePhaseId('GSD.02-05-/etc/passwd'); + assert.deepStrictEqual(id, { project: 'GSD', milestone: '02', phase: '05' }); + }); +}); + +// ─── m1.6: whitespace-only input ───────────────────────────────────────────── +// (The empty string is already pinned above under the ambiguous-token block; +// these are the non-empty all-whitespace forms.) +describe('bracket grammar: whitespace-only input is rejected (review m1)', () => { + test("' ', ' ', '\\t', '\\n', and '\\t\\n' all reject as not-a-bracket-phase-id", () => { + for (const s of [' ', ' ', '\t', '\n', '\t\n']) { + assert.throws(() => core.parsePhaseId(s), /not a bracket phase id/, JSON.stringify(s)); + } + }); +}); + +// ─── m1.7: very-long input (ReDoS smoke) ───────────────────────────────────── +// Behavioral (not timing) assertions: the grammar's regexes are linear — no +// nested quantifier over an overlapping class — so a 10k-char input resolves +// promptly; a catastrophic-backtracking regression would fail the run by +// timeout rather than by assertion. +describe('bracket grammar: very-long input handled promptly (review m1)', () => { + const BIG = 'a'.repeat(10000); + + test('a 10k-char garbage string rejects promptly', () => { + assert.throws(() => core.parsePhaseId(BIG), /not a bracket phase id/); + }); + + test('a partial-then-fail prefix (open bracket + 10k digits) rejects promptly', () => { + assert.throws(() => core.parsePhaseId(`[GSD.${'0'.repeat(10000)}`), /not a bracket phase id/); + }); + + test('a 10k-char slug in a dir string parses (slug dropped) without hanging', () => { + assert.deepStrictEqual(core.parsePhaseId(`GSD.02-05-${BIG}`), { project: 'GSD', milestone: '02', phase: '05' }); + }); + + test('toDir sanitizes a 10k-char slug promptly to the expected token', () => { + const dir = core.toDir(core.parsePhaseId('[GSD.02] 05'), BIG); + assert.strictEqual(dir, `GSD.02-05-${BIG}`); + assert.strictEqual(dir.length, 'GSD.02-05-'.length + 10000); + }); +}); + +// ─── canonical token/heading sources for the downstream read path (PR-2) ───── +describe('bracket grammar: exported canonical sources (drift-guard owner)', () => { + test('BRACKET_PHASE_TOKEN_SOURCE matches the bracket numeric run (dot-or-dash sub-separator)', () => { + const re = new RegExp(`^${core.BRACKET_PHASE_TOKEN_SOURCE}$`); + // The bracket dir/heading numeric run: MM-PP[.SS] (dash milestone↔phase, dot phase↔subphase). + assert.ok(re.test('02-05.03'), 'MM-PP.SS run'); + assert.ok(re.test('02-05'), 'MM-PP run'); + assert.ok(re.test('05.03'), 'PP.SS phase token'); + assert.ok(re.test('05'), 'bare phase'); + assert.ok(re.test('12A'), 'letter variant'); + assert.ok(!re.test('slug'), 'non-numeric is not a token'); + }); + + test('PHASE_HEADING_PREFIX_SRC matches a bracket-or-Phase heading intro, not a bare number', () => { + const re = new RegExp(`^${core.PHASE_HEADING_PREFIX_SRC}`); + assert.ok(re.test('[GSD.02] 05: Title'), 'bracket prefix'); + assert.ok(re.test('Phase 5: Title'), 'Phase prefix'); + assert.ok(re.test('[GSD.02] Phase 5: Title'), 'bracket + Phase prefix'); + assert.ok(!re.test('05: Title'), 'a bare number is not a phase heading intro'); + }); +}); + +// ─── fast-check generative properties (ADR Decision 4) ────────────────────── +// A genuinely generative project code over the repo's [A-Z][A-Z0-9_]* grammar. +const projectArb = fc + .tuple( + fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split('')), + fc.array(fc.constantFrom(...'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_'.split('')), { maxLength: 5 }), + ) + .map(([head, rest]) => head + rest.join('')); +// A clean slug over [a-z0-9], filtered to guarantee at least one letter: the +// mixed digit+letter shape (review M3) exercises alphanumeric words like +// 'v2ui' without ever generating an all-digit slug, which toDir now rejects +// (an all-digit slug collides with the plan grammar). Restricted to +// lowercase-plus-digit input so the slug guard's toLowerCase()/replace() is a +// no-op — safeSlug === slug holds, keeping the disk↔display bijection's dir- +// string equality assertion exact. +const slugArb = fc + .array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz0123456789'.split('')), { minLength: 1, maxLength: 8 }) + .map((cs) => cs.join('')) + .filter((s) => /[a-z]/.test(s)); +// Spans 1–999 so the property domain genuinely includes 3+-digit milestone/ +// phase/subphase/plan tokens (re-review Minor 1). pad2() passes a ≥3-digit +// value through unchanged (no truncation) and it carries no leading zero, so +// both the render round-trip and CANONICAL_NUMERIC_RE's dedicated `[1-9]\d{2,}` +// branch (exercised via toDir in the bijection property) hold at that width. +const numArb = fc.integer({ min: 1, max: 999 }); +const optNumArb = fc.option(numArb, { nil: undefined }); + +describe('bracket grammar — properties (fast-check)', () => { + test('round-trip: renderPhaseId(parsePhaseId(display)) === display for every well-formed display', () => { + fc.assert( + fc.property(projectArb, numArb, numArb, optNumArb, optNumArb, (proj, mm, pp, ss, ll) => { + let display = `[${proj}.${p2(mm)}] ${p2(pp)}`; + if (ss !== undefined) display += `.${p2(ss)}`; + if (ll !== undefined) display += `-${p2(ll)}`; + return core.renderPhaseId(core.parsePhaseId(display)) === display; + }), + ); + }); + + test('disk↔display bijection: toDir(parse(display), slug) is the canonical dir and re-parses to the same identity', () => { + fc.assert( + fc.property(projectArb, numArb, numArb, optNumArb, slugArb, (proj, mm, pp, ss, slug) => { + let display = `[${proj}.${p2(mm)}] ${p2(pp)}`; + if (ss !== undefined) display += `.${p2(ss)}`; + const id = core.parsePhaseId(display); + const dir = core.toDir(id, slug); + const expectedDir = `${proj}.${p2(mm)}-${p2(pp)}${ss !== undefined ? '.' + p2(ss) : ''}-${slug}`; + if (dir !== expectedDir) return false; + const back = core.parsePhaseId(dir); + return ( + back.project === id.project && + back.milestone === id.milestone && + back.phase === id.phase && + back.subphase === id.subphase && + // A valid (letter-bearing) slug must never be read back as a plan + // (review M3) — the bijection holds on the full identity tuple, + // not just the milestone/phase/subphase dimensions. + back.plan === undefined + ); + }), + ); + }); + + // Non-canonical property (review B1): the round-trip property above only + // ever feeds parsePhaseId a string produced by p2()-padding, so it cannot + // exercise the rejection path at all. This property starts from a KNOWN + // canonical display string and applies one structural mutation — stripping + // a pad (milestone, phase, OR subphase), doubling the bracket/phase separator + // space, over-padding a field, or adding stray whitespace — asserting + // parsePhaseId throws on every one. The milestone/phase/subphase integers are + // restricted to 1-9 here so `p2()` always actually pads (e.g. '05', not '42'); + // otherwise the "unpad" mutation could regenerate the same canonical string, + // making the mutation a no-op instead of a genuine probe. `includeSub` (a + // generated boolean) decides whether the canonical carries a `.SS` subphase; + // the subphase-pad mutations (re-review Minor 2) force one in so there is + // always a `.SS` to mutate, while the non-subphase mutations keep their + // original no-subphase coverage whenever `includeSub` is false. + const singleDigitArb = fc.integer({ min: 1, max: 9 }); + const mutationArb = fc.constantFrom( + 'unpad-milestone', + 'unpad-phase', + 'unpad-subphase', + 'overpad-milestone', + 'overpad-phase', + 'overpad-subphase', + 'double-space', + 'leading-space', + 'trailing-space', + ); + + test('non-canonical mutations of a canonical display string are always rejected', () => { + fc.assert( + fc.property( + projectArb, + singleDigitArb, + singleDigitArb, + singleDigitArb, + fc.boolean(), + mutationArb, + (proj, mm, pp, ss, includeSub, mutation) => { + const mmP = p2(mm); + const ppP = p2(pp); + const ssP = p2(ss); + const isSubMutation = mutation === 'unpad-subphase' || mutation === 'overpad-subphase'; + // A subphase-pad mutation needs a `.SS` to act on, so force one in for + // those cases; otherwise the generated boolean decides, preserving the + // original no-subphase mutation coverage. + const hasSub = includeSub || isSubMutation; + const subCanon = hasSub ? `.${ssP}` : ''; + const canonical = `[${proj}.${mmP}] ${ppP}${subCanon}`; + let mutated; + switch (mutation) { + case 'unpad-milestone': mutated = `[${proj}.${mm}] ${ppP}${subCanon}`; break; + case 'unpad-phase': mutated = `[${proj}.${mmP}] ${pp}${subCanon}`; break; + case 'unpad-subphase': mutated = `[${proj}.${mmP}] ${ppP}.${ss}`; break; + case 'overpad-milestone': mutated = `[${proj}.0${mmP}] ${ppP}${subCanon}`; break; + case 'overpad-phase': mutated = `[${proj}.${mmP}] 0${ppP}${subCanon}`; break; + case 'overpad-subphase': mutated = `[${proj}.${mmP}] ${ppP}.0${ssP}`; break; + case 'double-space': mutated = `[${proj}.${mmP}] ${ppP}${subCanon}`; break; + case 'leading-space': mutated = ` ${canonical}`; break; + case 'trailing-space': mutated = `${canonical} `; break; + default: throw new Error(`unreachable mutation: ${mutation}`); + } + // Sanity: every mutation above must actually change the string, or + // the property would (correctly) fail to throw and falsely indict + // the implementation instead of the generator. + if (mutated === canonical) return false; + try { + core.parsePhaseId(mutated); + return false; // did not throw — the mutation slipped through + } catch { + return true; + } + }, + ), + ); + }); +}); + +// ─── the #2232 reconciliation, generatively ───────────────────────────────── +// BRACKET_PHASE_TOKEN_SOURCE is the READ side; toDir is the EMIT side. The +// example tables pin specific strings, but the contract that actually matters +// is metamorphic and spans the pair: for every id the emit path can produce, +// the read path must collect exactly that id's numeric run — no more (the +// #2232 over-collection) and no less (the under-collection a verbatim +// exactly-2 cap would cause at 3+-digit widths). Tying the two together means a +// future change to either side fails here rather than drifting silently, which +// is the whole point of consuming the single-owner seam. +describe('bracket grammar — read/emit agreement on the token run (#2232)', () => { + // A slug whose FIRST WORD is a >=3-digit number — the #2232 bug class itself + // (roadmap phase "2026 Photos & Performance" → slug "2026-photos-…"). The + // existing slugArb generates a single [a-z0-9] word and so can never produce + // this shape; the collision only exists when a digit-run sits at a segment + // boundary. Bounded at >=100 because a 2-digit first word is genuinely + // ambiguous against a canonical plan — the seam's known, accepted limit, + // identical on the M-NN path. + const numberLeadingSlugArb = fc + .tuple( + fc.integer({ min: 100, max: 9999 }), + fc.array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyz'.split('')), { minLength: 1, maxLength: 6 }), + ) + .map(([lead, word]) => `${lead}-${word.join('')}`); + + test('a number-leading slug never bleeds into the token run', () => { + fc.assert( + fc.property(projectArb, numArb, numArb, optNumArb, numberLeadingSlugArb, (proj, mm, pp, ss, slug) => { + const display = `[${proj}.${p2(mm)}] ${p2(pp)}${ss !== undefined ? '.' + p2(ss) : ''}`; + const id = core.parsePhaseId(display); + const dir = core.toDir(id, slug); + + // The run the emit path actually wrote, independent of the read regex. + const expectedRun = `${p2(mm)}-${p2(pp)}${ss !== undefined ? '.' + p2(ss) : ''}`; + + // The read path, applied the way a PR-2 reader would: strip the + // `{CODE}.` prefix, then collect the run with the exported source. + const runInput = dir.slice(`${proj}.`.length); + const collected = runInput.match(new RegExp(`^${core.BRACKET_PHASE_TOKEN_SOURCE}`))?.[0]; + if (collected !== expectedRun) return false; + + // And the strict parser agrees the slug is a slug, not a plan — the + // #2232 failure mode was the reader and the parser disagreeing about + // where the identity ends. + const back = core.parsePhaseId(dir); + return back.phase === p2(pp) && back.milestone === p2(mm) && back.plan === undefined; + }), + ); + }); +}); diff --git a/tests/continuation-grammar-parity.test.cjs b/tests/continuation-grammar-parity.test.cjs index 1dc7f1c54..b08cdfc03 100644 --- a/tests/continuation-grammar-parity.test.cjs +++ b/tests/continuation-grammar-parity.test.cjs @@ -20,12 +20,14 @@ * "is this segment absorbed as a continuation?" MUST equal * `isPhaseContinuationSegment(segment)`. * - * Surfaces covered (the five #2043 sites): + * Surfaces covered (the five #2043 sites, plus the #612 bracket read path): * 1. phase-id.cjs extractPhaseToken * 2. validate.cjs PHASE_TOKEN_FROM_DIR_RE * 3. validate.cjs canonicalPlanStem * 4. core-utils.cjs extractCanonicalPlanId (paired plan component) * 5. roadmap-parser.cjs getMilestonePhaseFilter → isDirInMilestone (hyphenated mode) + * 6. phase-id.cjs BRACKET_PHASE_TOKEN_SOURCE (slug-adjacent position only — + * see the divergence block at the foot of this file) */ const { test, describe } = require('node:test'); @@ -110,6 +112,20 @@ describe('#2232 continuation-grammar parity — every consuming surface agrees', owner, `extractCanonicalPlanId(${JSON.stringify(planFile)}) diverged from the owner`, ); + + // ── Surface 6: #612 BRACKET_PHASE_TOKEN_SOURCE (slug-adjacent position) ── + // The bracket run is MM-PP[.SS][-LL]; `-LL` is the only position a slug + // word can collide with, so it is the position #2232 owns. Same shape as + // surface 1 with the bracket's extra milestone level: `01-14--slug…` + // puts at dash-2, exactly where a year over-collected before. + const bracketDir = `01-14-${seg}-photos-performance`; + const bracketToken = bracketDir.match(new RegExp(phaseId.BRACKET_PHASE_TOKEN_SOURCE))?.[0]; + assert.strictEqual( + bracketToken === `01-14-${seg}`, + owner, + `BRACKET_PHASE_TOKEN_SOURCE on ${JSON.stringify(bracketDir)} collected ` + + `${JSON.stringify(bracketToken)} — diverged from the owner at the slug-adjacent position`, + ); }); } }); @@ -158,3 +174,51 @@ describe('#2232 continuation-grammar parity — roadmap isDirInMilestone (hyphen }); } }); + +// ─── #612: the DELIBERATE divergence, pinned ──────────────────────────────── +// Surface 6 consumes the owner at the slug-adjacent position (above), but is +// deliberately WIDER at the other positions. That is a divergence, so per the +// Generative Fix Divergence rule it gets pinned here rather than left to a +// comment: if someone later "unifies" the bracket run onto the exactly-2 cap, +// or re-widens the slug-adjacent position back to `\d+`, one of these fails and +// points them at the rationale in phase-id.cts. +// +// The policy is stated independently of the regex: bracket's non-slug-adjacent +// positions are DELIMITER-disambiguated (a grammar-required field separator; a +// dot no slug can contain), not heuristically recognized, so they carry the +// canonical width toDir emits — while #2232's cap defends the one position that +// sits against a slug. +describe('#612 bracket divergence — wider only where the delimiter disambiguates', () => { + const tokenOf = (s) => s.match(new RegExp(phaseId.BRACKET_PHASE_TOKEN_SOURCE))?.[0]; + + test('the #2232 repro cannot reopen on the bracket path', () => { + // The review's scenario: roadmap phase "2026 Photos & Performance" at + // phase 14 → slug leads with a year. The token is the phase, not the year. + assert.strictEqual(tokenOf('01-14-2026-photos-performance'), '01-14'); + assert.strictEqual(tokenOf('01-14.03-2026-photos-performance'), '01-14.03'); + }); + + test('3+-digit phase and sub-phase — widths toDir emits — stay recognized', () => { + // Both are rejected by a verbatim exactly-2 cap; both are canonical per + // CANONICAL_NUMERIC_RE, so under-collecting them would break the read path + // against ids the emit path produces. + assert.strictEqual(tokenOf('02-105-slug'), '02-105', '3-digit phase (dash-1)'); + assert.strictEqual(tokenOf('05.100'), '05.100', '3-digit sub-phase (dot)'); + assert.strictEqual(tokenOf('01-2026-photos'), '01-2026', 'a 4-digit phase is unambiguous at dash-1'); + }); + + test('the divergence is bounded: a PLAN >=100 is out of the grammar (#2232 policy verbatim)', () => { + // The accepted trade-off. Stated as a test so it is a decision on record, + // not an accident: the slug-adjacent position cannot be widened without + // reopening the year collision. + assert.strictEqual(tokenOf('02-05-100'), '02-05', 'a 3-digit plan is not absorbed'); + assert.strictEqual(tokenOf('02-05-01'), '02-05-01', 'a canonical 2-digit plan is absorbed'); + }); + + test('an over-padded field is not canonical, so it is not collected', () => { + // `014` matches neither canonical branch (leading zero + 3 digits), which is + // what parsePhaseId rejects too — the read side under-collects rather than + // inventing a field the parser would refuse. + assert.strictEqual(tokenOf('01-014-slug'), '01'); + }); +}); diff --git a/tests/phase-id.test.cjs b/tests/phase-id.test.cjs index c084a2524..b37c45e43 100644 --- a/tests/phase-id.test.cjs +++ b/tests/phase-id.test.cjs @@ -209,6 +209,27 @@ describe('extractPhaseToken', () => { assert.strictEqual(phaseId.extractPhaseToken('M1-2-brain'), 'M1-2'); }); + // #612/#2249: the #2043/#1324 letter-prefixed-decimal family has a NUMERIC-tail + // variant (`P0.3-2`) the #1324 pins above never covered — every tail there is + // non-numeric (`-tenant`, `-gate`) or hyphen-only (`M1-2`). PR-1 added a bracket + // dir reader `{CODE}.{MM}-{PP}` to extractPhaseToken; because that shape is + // string-indistinguishable from this family when the code ends in a digit, the + // reader is GATED on an explicit `convention` arg. This characterization locks + // the convention-less (legacy) reading byte-identical across the WHOLE family — + // single- AND multi-digit tails — so the gate can never silently regress it. + // (The multi-digit rows are precisely the ones no discriminator-tightening fix + // could have preserved: `P0.12-34` stays ambiguous with a padded bracket dir, + // whereas the convention gate is complete.) + test('preserves the #2043 numeric-tail letter-prefixed family (convention-less, byte-identical)', () => { + assert.strictEqual(phaseId.extractPhaseToken('P0.3-2-tenant'), 'P0.3-2'); + assert.strictEqual(phaseId.extractPhaseToken('P1.2-3'), 'P1.2-3'); + assert.strictEqual(phaseId.extractPhaseToken('A0.1-2'), 'A0.1-2'); + assert.strictEqual(phaseId.extractPhaseToken('X9.9-9-name'), 'X9.9-9'); + assert.strictEqual(phaseId.extractPhaseToken('P0.12-34-name'), 'P0.12-34'); + assert.strictEqual(phaseId.extractPhaseToken('P0.34-56-name'), 'P0.34-56'); + assert.strictEqual(phaseId.extractPhaseToken('P0X.3-2'), 'P0X.3-2'); + }); + test('returns the full dirName when no numeric token found', () => { assert.strictEqual(phaseId.extractPhaseToken('no-numeric'), 'no-numeric'); assert.strictEqual(phaseId.extractPhaseToken('alpha'), 'alpha');