A convergence audit showed the `<tag>[\s\S]*?</tag>` lazy-scan ReDoS was pervasive
(a dozen+ bespoke copies across roadmap-parser/check-command-router/verify), each
a distinct quadratic vector on a large document with unclosed tags. Rather than
whack-a-mole, single-source them (maintainer-directed):
- markdown-sectionizer: extractTaggedBlocks now shares one ReDoS-safe
`taggedBlockPattern` (stop-at-next-open, bounded optional attributes) and gains
a `stripTaggedBlocks` companion for block removal.
- roadmap-parser: 3 `<details>` strips -> stripTaggedBlocks (behavior-identical —
no <details> here carries attributes).
- verify: actionZones + both <task> loops + their nested <name>/<files>/gate/req
extractions -> extractTaggedBlocks (behavior byte-equivalent, verified).
- check-command-router: the objective|tasks?|action alternation hardened in place
(distinct multi-tag shape); HTML-comment strip gains a `$` fallback.
Every vector now linear (<3ms on 1.5MB adversarial); real content unchanged
(end-to-end verify/roadmap resolution + task extraction confirmed). The only
remaining `<!--…-->` scan (uat.cts:201) is anchored + non-global — one scan, safe.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review caught that the prior commit bounded only the paren tag clause and left
the SIBLING bracket-prefix `(?:\[[^\]]+\]\s*)?` (same host regexes, before Phase)
UNBOUNDED — the identical quadratic reachable via a `[...]` run (measured ~16s at
1.7MB). Bound `[^\]]+`/`[^\]]*` -> {1,200}/{0,200} across all 19 phase/milestone
heading prefixes. Comprehensive re-measurement now shows EVERY vector linear
(bracket/paren/id/name/milestone all ~2-44ms at 2.45MB; bracket scaling
2k->2ms, 4k->5ms, 8k->10ms). Also: update the #1729 literal-mirror parity test
off its stale unbounded constant, and add limit-1 (199) boundary coverage.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The canonical OPTIONAL_PHASE_TAG_SOURCE tag clause `(?:\s*\([^)\n]*\))?` (and its
inlined literal mirrors across 11 modules) had an UNBOUNDED body, making the
optional-group + /g header scan quadratic on adversarial ROADMAP.md/STATE.md — a
long run of `(` after a header ran ~18.8s at 1.7MB. Bound the body to {0,200} in
the constant AND every mirror in lockstep (the #1729 "both forms change together"
contract), so the scan is linear: the same 1.7MB input now resolves in ~9ms
(measured), while real tags (a handful of chars) still match and a 201-char tag
is rejected. Added a #2128 boundary regression to the #1729 suite.
Pre-existing (byte-identical before/after the Phase 4 migrations); folded in at
maintainer direction rather than deferred.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Correctness review of the Phase 4 guard found the allowlist over-broad and the
scanner/guards evadable. Fixed all findings:
- Migrate 9 sites that were wrongly sanctioned: their regex is the PURE canonical
token (`\d+[A-Z]?(?:\.\d+)*`, no variant), byte-identical to already-migrated
siblings. The old justification argued against swapping to the extractPhaseToken()
FUNCTION (behavior-risky) — but the guard only wants the same regex built from
the SOURCE string (byte-equal, zero risk). Coverage is now 32 migrated / 5
sanctioned, not the overstated 23 / 14 (audit.cts x3, uat.cts, init.cts x4,
roadmap-upgrade.cts). Each conversion proven byte-equal (.source + .flags).
- Harden the drift detector: also catch the `[0-9]`-in-place-of-`\d` variant;
document the accepted limits (cross-line split, semantic restructuring —
covered by the identity guard + review, not a text scan).
- Sanction robustness: a `phase-id-owner:` marker now counts only inside a `//`
comment (a bare substring in a string no longer suppresses a real flag), and
the preceding-line window skips blank lines (an auto-formatter's blank line no
longer reactivates the flag).
- roadmap-parser.cts:462 comment: corrected — that regex carries no /i flag, so
its [A-Za-z] class does real case work (matches state.cts:1409's rationale).
- Identity guard: surface require failures instead of silently skipping, and
floor coverage at >75% of consumer modules (inspects 156/157).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route 23 literal re-derivations of the canonical phase-number token through
phase-id.cjs `PHASE_NUMBER_TOKEN_SOURCE` (via new RegExp). Each conversion was
proven BYTE-IDENTICAL (old.source === new.source && old.flags === new.flags), so
the runtime regexes are unchanged — zero behavior change by construction.
The remaining 14 phase-token sites are genuine but context-specific and stay
literal with a `// phase-id-owner: <reason>` sanction: dir-name parses whose
dash-continuation semantics differ from extractPhaseToken, and the [A-Za-z]
case-variant / [.-] dot-or-dash separator forms that are not source-byte-equal
to the canonical token.
Scanner (`npm run check:phase-id-drift`) is now green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the ADR-2121-locked canonical functions to src/phase-id.cts. No consumer
behavior changes — Phases 2-4 migrate the divergent call sites against them.
- parsePhaseFromProse: anchored prose parser. A phase is returned only when the
STATE.md "Phase:" field VALUE begins with a phase token, so
"Milestone v0.5 complete" yields { phase: null } instead of "5" (the #2111
root cause: the old unanchored \b(\d+..)\b mined the minor-version digit).
Name extraction (parenthetical / em-dash tail, minus status words) unchanged.
- stripConfiguredProjectCodePrefix / isForeignPrefixedPhaseQuery: config-aware
prefix policy. A foreign prefix (MEM-01 when the configured code is LKML) is
preserved rather than collapsed to a bare numeric phase — the #2104 fix's
canonical home (consumed later, outside this epic's critical path).
- roadmapPhaseLookupSources: moved from roadmap-parser.cts so phase-id.cts is
the single owner of the exact -> numeric -> prefix-tolerant ordering.
roadmap-parser.cts now imports it (behavior-identical); its two now-unused
imports (phaseMarkdownRegexSourceExact, OPTIONAL_PROJECT_CODE_PREFIX_SOURCE)
are dropped.
Tests: subject-named suites in tests/phase-id.test.cjs covering the ADR
boundary set (v0.5, v1.0, MEM-01, AB-29, bare 29, zero-padded 029) plus two
fast-check properties: the #2111 "Milestone vX.Y complete never yields a phase"
invariant and a parse/normalize property.
Extend-never-mutate: the 12 pre-existing phase-id.cts exports are unchanged.
Closes#2124
Refs #2121
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A phase whose slug's first word is a bare single digit (dir
`46-6-rs-pipeline-orchestrator`, roadmap phase "6 Rs Pipeline Orchestrator" →
slug `6-rs-…`) had its phase token over-collected as `46-6` instead of `46`, so
`gsd-tools` phase-by-number lookups (init.plan-phase, init.phase-op, and
downstream execute/verify/ship) resolved phase_dir=null / has_context=false.
Root cause: an over-broad "looks numeric" test (`/^\d/`, `\d+(?:-\d+)*`)
classified token segments and could not distinguish a legitimate zero-padded
sub-phase segment (`01`, `02`) from a single-digit slug word (`6`). Zero-padded
phase/sub-phase segments are always ≥2 digits, so requiring ≥2 digits is the
structural distinguisher. Applied consistently across every same-class
implementation the triage identified (fixing extractPhaseToken alone leaves the
health-check and milestone-filter subsystems exposed):
- src/phase-id.cts extractPhaseToken — a pure-numeric leading segment continues
only with ≥2-digit segments; a letter-prefixed milestone id (`M1`) still
admits its single-digit sub-phase (`M1-2`).
- src/validate.cts PHASE_TOKEN_FROM_DIR_RE (W005/W006/W007 health checks) and
canonicalPlanStem (I001 plan/summary pairing).
- src/roadmap-parser.cts isDirInMilestone numericRe (getMilestonePhaseFilter —
highest blast radius: a false negative silently excludes a phase from the
milestone).
- src/core-utils.cts + its verbatim duplicate src/phase.cts extractCanonicalPlanId.
Regression tests added across tests/{phase-id,health-validation,core-utils,
roadmap-parser}.test.cjs using the shared `46-6-rs-…` fixture, asserting the
token resolves to `46` (not `46-6`) while legit multi-segment tokens
(`01-02`, `02-03-04`, `M1-2`, `68-01`, `02-01`) are unchanged. The
roadmap-parser test exercises the real getMilestonePhaseFilter end-to-end.
The residual ≥2-digit-leading-slug case (e.g. `NN-2024-roadmap`) stays out of
scope — subsumed by the #612 / #565 phase-ID convention work, per the issue.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#1729): resolve phase headers with a pre-colon parenthetical tag
A phase header may carry a parenthetical tag between the number and the
colon, e.g. `### Phase 26 (Cluster B): Title`. Every phase-header regex
built `Phase\s+<num>` immediately against the colon delimiter, so the
tagged phase was invisible: the resolver returned found:false and, just
as bad, the capture-all enumeration/parse paths (roadmap analyze,
milestone listing + milestone-scope filter, verify, init/import, state
total_phases, validate, the command router, preamble stripping, and the
phase-remove renumbering rewrite) silently dropped, miscounted, or
failed to renumber it — wrong phase_count, progress_percent, next_phase,
or corrupt numbering after a removal.
The fix tolerates the tag at the header seam. Parameterized resolver
sites compose the exported OPTIONAL_PHASE_TAG_SOURCE fragment; literal
enumeration sites inline its character-for-character mirror
`(?:\s*\([^)\n]*\))?`, placed immediately before the colon so it cannot
alter an existing match (optional, single-line, one paren pair, no
capture-group shift). In the renumber-on-removal rewrite the tag is
folded into the re-emitted suffix capture so it survives verbatim. Both
forms are documented to change together and a drift-guard test asserts
their behavioral equivalence over a header corpus.
Deliberately excluded: roadmap-upgrade.cts (legacy one-time migration),
where tolerating the tag would silently drop it on header rewrite — that
needs its own data-preserving treatment. Known boundaries left for
follow-up: checklist/bullet-style phase entries (`- [ ] Phase N (tag):`)
and a malformed space-before-colon variant, both pre-existing.
Validated empirically against the issue's reproduction: `roadmap
get-phase 26` resolves and `roadmap analyze` lists Phase 26 with the tag
excluded from the name (phase_count 2, next 26); an all-tagged versioned
roadmap now scopes correctly instead of falling back to a pass-all
filter. Regression coverage in tests/phase.test.cjs asserts resolver
parity (pre- vs post-colon), padding tolerance (#3537), decimal
sub-phases, no cross-phase false match, the shared seam, enumeration
coherence, renumber-preserves-tag, and seam/mirror drift. Full unit
suite green (7291 pass, 0 fail); eslint + regression-name +
resolution-provenance + changeset lints pass. Reviewed by Codex
(no critical/high; the two enumeration misses it surfaced are folded in).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(#1729): add changeset for pre-colon phase-tag fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(#1445,#1446): exclude 999.x backlog phases from milestone totals; allow total_phases downward correction
#1445: deriveProgressFromRoadmap (phase-lifecycle.cts), the roadmapPhaseCount
loop (state.cts), and getMilestonePhaseFilter (roadmap-parser.cts) all now
skip phase tokens matching /^999\b/ — consistent with the existing init.cts
filter. 999.x backlog dirs are consequently excluded from phaseDirs too.
#1446: shouldPreserveExistingProgress (state-document.cts) no longer includes
total_phases in its ratchet check. total_phases always takes the freshly
derived value; only completed_phases, total_plans, and completed_plans retain
ratchet behaviour.
Regression tests added for both bugs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: add changeset for #1445/#1446 progress-backlog-exclusion-and-ratchet
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(#1445,#1446): rename test files to fix-NNN convention; fix changeset pr: null
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes all three inline copies of the fenced-code state machine from
src/roadmap-parser.cts and replaces them with calls to
tokenizeHeadings() from the canonical markdown-sectionizer seam
(ADR-1372 T4).
Changes:
- Drop stripFencedLines() function (copy 1 of 3 — standalone helper)
- Rewrite computeSectionEnd() using tokenizeHeadings() offsets into
the original content (copy 2 — inline fence loop); the returned
character offset is preserved exactly for all inputs
- Rewrite getMilestonePhaseFilter() versionOverride fence loop using
tokenizeHeadings() (copy 3 — inline); sectionEnd offset preserved
- Replace stripFencedLines(roadmap) + unanchored phasePattern.exec()
with tokenizeHeadings(roadmap) filtered by level and phase-heading
pattern; headings in inline HTML comments (<!-- ## Phase N: -->)
are no longer mis-counted (anchored ATX detection is correct)
- Add import { tokenizeHeadings } from ./markdown-sectionizer.cjs
Offset preservation: tokenizeHeadings() records h.offset as the
character index of '#' in the ORIGINAL content; computeSectionEnd()
and the versionOverride path both use h.offset directly as the
section-end character offset — no stripping, no shift.
Corpus head-to-head: 29/30 slots are byte-identical to origin/next.
The 1 diff (getMilestonePhaseFilter phaseCount for the HTML-comment
fixture: 3→2) is an improvement: the old unanchored regex counted
"## Phase 998:" embedded in "<!-- ## Phase 998: ... -->" mid-line;
tokenizeHeadings() correctly requires '#' at line-start (ATX rule).
No existing test asserts on that count; feat-3594 passes unchanged.
All 122 roadmap/milestone/phase tests green.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(#1294): T-final — delete the core.cjs re-export spine (epic #1267 complete)
After T0–T6 nothing imports core, so retire the spine and its scaffolding:
- delete src/core.cts (and the gitignored gsd-core/bin/lib/core.cjs artifact;
remove its .gitignore + eslint-ignore entries)
- delete scripts/lint-core-spine-imports.cjs + its allowlist; drop it from the
package.json lint:ci chain
- regenerate docs/INVENTORY-MANIFEST.json (drops the core.cjs surface)
- sweep stale references: CONTEXT.md glossary back-compat clauses (spine retired,
callers import the leaf directly), planning-config.md CONFIG_DEFAULTS owner,
and false present-tense core.cjs claims in leaf-module docstrings
The ADR-857 decomposition is complete: the former Core god-module is fully
dissolved into its leaf modules; no re-export spine remains. No behaviour change.
Closes#1294
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#1294): migrate the computed-path core.cjs importers the literal grep missed
bin/install.js used require(path.join(_gsdLibDir, 'core.cjs')) (a computed
path, and bin/install.js was never in the convergence lint's scan roots), and
~8 test files referenced core.cjs via path.join/readFileSync/existsSync/FILE_ARG
forms the literal-string migration grep missed. Route install.js's symbols to
their leaves (RUNTIME_PROFILE_MAP->model-catalog, resolveTierEntry/EFFORT_SET->
model-resolver) and repoint/adjust the test references to the leaves. Recovers
the 161 'Cannot find module core.cjs' failures from the spine deletion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#1203): gate free-form ROADMAP deprecation warning on phase_id_convention
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: backfill changeset PR number for #1218
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(#1203): use canonical gsd-tools roadmap upgrade command in warning
Match the migration command string to the canonical form used in
verify.cts and roadmap-command-router.cts (gsd-tools roadmap upgrade
--convention milestone-prefixed, dry-run by default) instead of the
non-canonical 'gsd roadmap upgrade --apply'.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ADR-857 rollout phase 2b. Move the 6 ROADMAP.md-parsing functions
(stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone,
getRoadmapPhaseInternal, getMilestoneInfo, getMilestonePhaseFilter) + their
interfaces out of core.cts into a new leaf module src/roadmap-parser.cts.
core.cts re-exports them (behavior-preserving); ~9 external callers unchanged.
Resolves the parse/write straddle: roadmap.cts (ROADMAP.md mutation) now imports
its 3 parsing helpers from roadmap-parser.cjs directly instead of reaching
through core. roadmap-parser depends only on leaves (phase-id, planning-workspace,
shell-command-projection) — cycle-free, enabled by phase 2a.
New-CLI-module checklist done (.gitignore, eslint, INVENTORY 92->93 + row,
manifest, ARCHITECTURE, CONTEXT.md "Roadmap Parser Module"). Adds
tests/roadmap-parser.test.cjs (46 tests: behavioral + shim-identity +
adversarial ROADMAP.md fixtures).
Adversarial review surfaced a pre-existing fence-blindness bug in
getMilestonePhaseFilter (matches phase headings inside fenced code blocks);
filed as #875 and left for a separate fix (out of scope for this
behavior-preserving extraction). The two fenced-fixture tests characterize the
current behavior with a #875 reference and flip when it's fixed.
Gates: lint, code-review, security-review, codex adversarial-review (0
correctness findings). gsd-test: clean-build docker + Mac green; the full-suite
docker run's "X is not a function" errors on re-exported symbols were local
incremental-tsc staleness (verified: clean rebuild of the affected files = 195
pass, 0 fail; Mac = 4001 pass).
Closes#870
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
