* test(#3184): failing-first milestone-window single-owner suite Covers the 50 input classes in the phase test matrix: scope classification (genuinely-empty vs truncated vs unscoped vs unreadable), the section-end owner's level boundaries, consumer-output identity per ADR-3180 Decision 4(c), the milestone.complete refusal with negative proof that no directory moved, the version-token boundary defect, drift-guard behavior, and three fast-check properties over document-shaped generators. Committed alone so the remote runner records the failure before the fix lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * refactor(#3184): milestone windowing routes through one owner Three copies of the milestone section-end walk lived in roadmap-parser.cts — two distinct computeSectionEnd function nodes plus an inline third in getMilestonePhaseFilter's versionOverride branch. computeMilestoneSectionEnd is now the sole owner and the other two are deleted, not kept in sync by comment. The whole-repo drift guard found what the epic did not: state.cts held three more re-derivations of the same vocabulary — two byte-identical milestone bounding checks carrying a defect neither reported copy has (no boundary after the version token, so v2.0 matched inside v2.0.1), and a milestone-sectioning predicate. All three route through the owner now. A composition-level duplicate appeared inside this change's own first pass: getMilestonePhaseFilter and cmdMilestoneComplete each re-assembled a window out of the owner's primitives, and had already diverged on whether to skip a closed milestone heading. sliceMilestoneWindow is the one composition. Windows now carry the ADR-3180 SCOPE discriminator, so a truncated window is distinguishable from a genuinely empty milestone — those were output-identical, which is the whole failure class. roadmap analyze emits it (#3165), and milestone complete refuses to archive on anything but COMPLETE rather than pass-all moving every phase directory on disk (#3166). The pass-all degrade is preserved where its premise holds: making the filter deny-all would trade a silent over-inclusive answer for a silent under-inclusive one on the read paths that count with it. extractCurrentMilestone keeps its signature — 200+ affected symbols across 41 files and 25 process flows — and is a one-line wrapper over the scoped owner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * fix(#3184): fence-aware phase detection and one heading-selection owner Review fixes from the two orthogonal passes. The blocker: hasPhaseEntries matched ATX phase headings fence-aware via tokenizeHeadings but tested the #2199 bullet form against un-stripped markdown, so a fenced EXAMPLE of the bullet syntax counted as a real phase. A genuinely empty milestone then classified TRUNCATED and milestone complete refused a legitimate archive — a false positive in the destructive direction, worse than the defect this phase set out to fix. Both that path and getMilestonePhaseFilter own pre-existing bullet scan now run on stripFencedCode, since leaving one meant the owner file gave two different answers to the same question. The selection rule — locate, prefer the non-closed heading, else the first — had been written three more times inside the file whose thesis is single ownership. selectMilestoneHeading owns it; all three sites route through it. The copies were behaviorally identical, so this is de-duplication with no observable change, verified by probing that all three paths select the same heading. roadmap analyze emitting a scope no consumer read left #3165's actual symptom alive, so Route 0 in next.md now treats a non-complete scope as scan-failed rather than as a clean empty scan, and the ADR amendment no longer overstates what shipped. Also: the scope refusal moved above the archive-directory create, so a refusal leaves nothing on disk; the versionOverride comment names all four consumers; COMMANDS.md documents the new guard beside its sibling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * test(#2658): exclude the changelog from the malformed-path scan The gate walks every emitted .md/.js/.cjs file in an installed tree and asserts none contains `.claude/.trae/rules` or `.trae/.trae/rules`. CHANGELOG.md ships into that tree, and its #2658 entry quotes both malformed paths while describing the fix that removed them — so the release note documenting the fix trips the fix's own regression test. Red on next before this branch. The installer is correct: a probe over a real --trae --local install found 621 emitted files, exactly one hit, and it was gsd-core/CHANGELOG.md. The scan scope was the defect, not the product. Excluded by exact relative path rather than by loosening the patterns or skipping all markdown — the emitted agent and command markdown is precisely what #2658 was about, so the gate stays strong everywhere it matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * test(#3184): regenerate install-tree fixtures for the shared drift scanner scripts/lib/ ships in the npm package and installer, so extracting the shared tree-walk into scripts/lib/drift-scan.cjs adds one path to every runtime's install tree. Regenerated via npm run gen:install-tree; the delta is exactly that one path per fixture. The two drift guards themselves do not ship (scripts/lint-*.cjs is excluded), so only the extracted library moves. This matches the existing scripts/lib/allowlist-ratchet.cjs precedent, which is likewise a lint-only helper carried in the shipped tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * fix(#3184): restore the #730 sub-milestone boundary and narrow the refusal The remote runner caught two regressions this branch introduced. Both were mine, and neither review pass found them — only running the existing suite did. The version-token boundary. I replaced locateMilestoneHeadings' \b with (?![\w.-]), reasoning that v2.0 matching inside v2.0.1 was the same defect #2562 fixed in isMilestoneShippedInRoadmap. It is not the same question. A milestone state of v8.0 legitimately selects the '## v8.0-B' sub-milestone section over a closed v8.0-A sibling (#730), and \b is what allows it while the stricter boundary forbids it — nine tests in roadmap-phase-fallback said so. Reverted to \b; the state.cts consolidation is now a straight merge with no behavior change, and the v2.0/v2.0.1 ambiguity is left exactly as it was. The ADR amendment and the design doc no longer claim otherwise. The refusal scope. I refused whenever the window was not COMPLETE, but #3166 is about the TRUNCATED window specifically — the heading is found and the section closes before the phase region, so pass-all archives everything. UNREADABLE and UNSCOPED are pre-existing, legitimately handled states, and refusing on them broke 'handles missing ROADMAP.md gracefully' and three archive tests. Narrowed to TRUNCATED; docs corrected to match. One of the new tests was also wrong: its fixture gave the shipped and current milestones' phases the same numeric id, and the filter matches on that id, so it could not have distinguished the two windows. Fixture corrected to exercise what it claims to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * fix(#3184): enumerate drift-scan.cjs for uninstall The installer copies scripts/lib/ wholesale, but uninstall removes an explicit set — deliberately, so a user's own helpers in that directory survive. The extracted drift-scan.cjs was copied in and never enumerated, so it outlived uninstall, left the directory non-empty, and the rmdir that follows failed. Added to GSD_SCRIPTS_LIB_FILES, following allowlist-ratchet.cjs, which is likewise a lint-only helper that ships there and is enumerated. Verified with a real install-then-uninstall into a temp target: scripts/lib/ held exactly the three GSD files and was gone afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * test(#3184): assert install and uninstall agree on scripts/lib and scripts/changeset Found while shipping this phase, and fixed here rather than noted. install() copies scripts/lib/ and scripts/changeset/ into the target WHOLESALE — the comment at the copy site literally says "and any future lib helpers". uninstall() removes them by hardcoded enumeration, deliberately, so a user's own helpers in those directories survive. A wholesale writer paired with an enumerated remover cannot stay in sync by construction: any file added to either directory ships to every user and is then orphaned in their repo forever, since it survives uninstall, leaves the directory non-empty, and the rmdir that follows fails. Nothing reported this. 31,225 tests were green over it. That is the same divergence class this epic exists to delete, sitting in the installer, so it gets the same remedy CLAUDE.md prescribes for it: a parity assertion that fails the moment the two surfaces disagree. The test compares each directory's real contents against its enumeration and names the offending file plus the constant to add it to. Both enumerations are hoisted to module scope and exported, so the test asserts on the actual arrays rather than pattern-matching the installer's source — no allow-test-rule annotation needed. Proven non-vacuous both ways: empty diff on the current tree, correct report when an unenumerated file is injected. scripts/changeset/ turned out to carry the identical defect and is covered too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 * chore(#3184): backfill changeset PR number Also narrows the wording to match the shipped behavior: the refusal fires on a truncated window specifically, not on any non-complete scope. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015kfkRFNUESoBspUYcAQaT3 --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/serene-elks-sing.md
Normal file
5
.changeset/serene-elks-sing.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Changed
|
||||
pr: 3209
|
||||
---
|
||||
**A truncated milestone window is no longer reported as an empty milestone** — `roadmap analyze` now emits a `scope` field (`complete`/`truncated`/`unscoped`/`unreadable`) so `phase_count: 0` from a genuinely fresh milestone is distinguishable from a window that closed before reaching the roadmap's phase sections, and `milestone complete` refuses to archive on a truncated window instead of moving every phase directory in the project. (#3184)
|
||||
@@ -181,7 +181,7 @@ Canonical GFM table parsing + schema registry seam (`gsd-core/bin/lib/markdown-t
|
||||
Shared fail-loud `Result<T>` and per-surface write-set contracts (`gsd-core/bin/lib/write-set.cjs`, generated from `src/write-set.cts`; ADR-2143 §5/§6, epic #2143). Pure, Node built-ins only, no I/O. Exports: `Result<T>` (`{ok:true,value}\|{ok:false,reason}` — ADR-2143 §5 fail-loud parse shape, never a bare `null` a caller can mistake for "empty but fine"; the single source of truth `markdown-table.cjs` re-exports so its existing importers are unaffected; deliberately distinct from command-routing-hub's dispatch `Result` `{ok,data\|kind}`); `WriteOutcome` (`{surface: string, applied: boolean}` — one surface's outcome within a multi-surface write); `WriteSet` (`WriteOutcome[]`); `writeSetComplete(ws) → boolean` (true only when the set is non-empty AND every surface applied — ADR-2143 §6's "no OR-into-one-flag" rule: a command that mutates more than one surface must not collapse independent surface outcomes into a single boolean, the anti-pattern that let a checkbox-only partial write (#2140) report full success). `milestone.cts`'s `requirements mark-complete` handler is the first consumer: it reports a `write_set` (`checkbox`/`traceability` surfaces) and `write_set_complete` alongside its existing `updated`/`marked_complete`/`already_complete`/`not_found`/`table_unmatched` fields, which remain computed exactly as before — the write-set is additive, structured ADR-2143 documentation of the same per-surface facts #2140's tactical fix already exposed via `table_unmatched`.
|
||||
|
||||
### Roadmap Parser Module
|
||||
Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`, `isMilestoneShippedInRoadmap`, `withPhaseSection`). Milestone shipped/active heading classification is owned here (#2562): `isMilestoneShippedInRoadmap(content, version)` answers "does the ROADMAP mark THIS milestone shipped" from heading and `<summary>` lines only — never a bullet that merely names the version — with the version token boundary-matched so `v2.0` does not match inside `v2.0.1`. `extractCurrentMilestone` and `getMilestonePhaseFilter` take an optional trailing workstream name so their `planningDir` resolution targets `.planning/workstreams/<ws>/`; omitted, it resolves exactly as before (including the `GSD_WORKSTREAM` fallback). `getMilestonePhaseFilter` exposes `versionScoped`, true only when the returned phase set really is one milestone's — consumers must not read `phaseCount` as a current-milestone denominator otherwise — and `versionSectionFound`, true whenever the requested version's section was located at all. The two differ precisely for a located-but-EMPTY section: it falls through to the zero-count pass-all degrade, which resets `versionScoped` to false, leaving `versionSectionFound` the only surviving evidence that the milestone exists rather than being absent. `missingExplicitVersion` covers the complementary shape (versioned roadmap, no section for this version). `withPhaseSection(content, phaseId, edit)` resolves a phase's `### Phase N` detail-section heading via the #2121 phase-id source (`phaseMarkdownRegexSource`) and delegates to the markdown-sectionizer seam's `withSection`, so a per-phase ROADMAP edit is bounded to that phase's own section (ADR-2143 §4). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`, `markdown-sectionizer`, and — since #1881 — `unusable-input` for the out-of-band diagnostic) — no `loadConfig`, no other core dependency. An unreadable ROADMAP.md is reported rather than collapsed into the same sentinel as a genuinely absent one; absence itself stays silent, and neither lookup gains a throw (the #2245 audit records that `src/state.cts` removed its defensive try/catch on the strength of `getMilestoneInfo` never throwing). Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`).
|
||||
Module owning ROADMAP.md parsing: shipped-milestone slicing, current-milestone extraction, milestone/phase lookups, and milestone-phase filtering (`stripShippedMilestones`, `extractCurrentMilestone`, `replaceInCurrentMilestone`, `getRoadmapPhaseInternal`, `getMilestoneInfo`, `getMilestonePhaseFilter`, `isMilestoneShippedInRoadmap`, `withPhaseSection`). Milestone shipped/active heading classification is owned here (#2562): `isMilestoneShippedInRoadmap(content, version)` answers "does the ROADMAP mark THIS milestone shipped" from heading and `<summary>` lines only — never a bullet that merely names the version — with the version token boundary-matched so `v2.0` does not match inside `v2.0.1`. `extractCurrentMilestone` and `getMilestonePhaseFilter` take an optional trailing workstream name so their `planningDir` resolution targets `.planning/workstreams/<ws>/`; omitted, it resolves exactly as before (including the `GSD_WORKSTREAM` fallback). `getMilestonePhaseFilter` exposes `versionScoped`, true only when the returned phase set really is one milestone's — consumers must not read `phaseCount` as a current-milestone denominator otherwise — and `versionSectionFound`, true whenever the requested version's section was located at all. The two differ precisely for a located-but-EMPTY section: it falls through to the zero-count pass-all degrade, which resets `versionScoped` to false, leaving `versionSectionFound` the only surviving evidence that the milestone exists rather than being absent. `missingExplicitVersion` covers the complementary shape (versioned roadmap, no section for this version). `withPhaseSection(content, phaseId, edit)` resolves a phase's `### Phase N` detail-section heading via the #2121 phase-id source (`phaseMarkdownRegexSource`) and delegates to the markdown-sectionizer seam's `withSection`, so a per-phase ROADMAP edit is bounded to that phase's own section (ADR-2143 §4). Depends only on leaf modules (`phase-id`, `planning-workspace`, `shell-command-projection`, `markdown-sectionizer`, and — since #1881 — `unusable-input` for the out-of-band diagnostic) — no `loadConfig`, no other core dependency. An unreadable ROADMAP.md is reported rather than collapsed into the same sentinel as a genuinely absent one; absence itself stays silent, and neither lookup gains a throw (the #2245 audit records that `src/state.cts` removed its defensive try/catch on the strength of `getMilestoneInfo` never throwing). Milestone WINDOWING — which headings bound a milestone — is owned here as of #3184 (epic #3180 Phase 2, ADR-3180 Decision 1): `computeMilestoneSectionEnd` (the section-end walk, formerly duplicated as two distinct nested `computeSectionEnd` functions plus an inline third copy in `getMilestonePhaseFilter`'s `versionOverride` branch), `locateMilestoneHeadings` (heading location, version token boundary-matched with `(?![\w.-])` so `v2.0` does not match inside `v2.0.1` — `\b` does, because `.` is a non-word character), `sliceMilestoneWindow` (the one composition of locate → prefer-non-closed → section-end, so a consumer cannot re-assemble its own window from the primitives), and `isMilestoneBoundedInRoadmap` (the named predicate replacing two byte-identical re-derivations in `state.cts`). `extractCurrentMilestoneScoped` is the real extractor and returns the Planning Scope Module's `ScopedResult`; `extractCurrentMilestone` remains a one-line wrapper over `.value` because its blast radius is CRITICAL (200+ affected symbols, 20 direct callers) and its signature must not move. `getMilestonePhaseFilter` gains a `scope` field: its pass-all degrade is PRESERVED where its premise holds (a genuinely-empty, freshly-declared milestone reports `SCOPE.COMPLETE`) and is now labelled where it does not (`SCOPE.TRUNCATED` when the window reached no phase entries while the document has them), so the destructive consumer — `milestone.complete`, which MOVES phase directories — can refuse instead of archiving every phase directory on disk (#3166). The filter's function behavior is deliberately unchanged: making it deny-all on a non-COMPLETE scope would trade a silent over-inclusive answer for a silent under-inclusive one on the read paths that count with it. Extracted from the Core module per ADR-857 rollout phase 2b (#870), resolving the ROADMAP.md parse/write straddle so the Roadmap module (`roadmap.cjs`, which owns ROADMAP.md mutation) imports parsing directly instead of through Core; the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/roadmap-parser.cjs` (generated from `src/roadmap-parser.cts`).
|
||||
|
||||
### Core Utilities Module
|
||||
Module owning the shared low-level utility primitives extracted from Core: POSIX path normalization (`toPosixPath`), filesystem scanning (`detectSubRepos`, `readSubdirectories`, `getPhaseFileStats`, `pathExistsInternal`), plan/summary pairing helpers (`countMatchedSummaries`, `findUnsummarizedPlans`, `findOrphanSummaries`), and small pure helpers (`generateSlugInternal`, `extractOneLinerFromBody`, `extractCanonicalPlanId`, `timeAgo`). `filterPlanFiles`/`filterSummaryFiles` were retired by #3183 (ADR-3180 Decision 2) — `getPhaseFileStats` no longer re-derives plan/summary filename matching locally; it now sources `plans`/`summaries` (plus a `scope` field, `COMPLETE`/`TRUNCATED`/`UNREADABLE`) directly from `scanPhasePlans` (`src/plan-scan.cts`), the single owner of live-plan counting. Depends only on Node built-ins and already-leafed modules (`phase-id` for `comparePhaseNum`, `planning-workspace` for `findContextMdIn`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2c (#877) as the shared leaf that unblocks the phase-locator fs-search extraction (2d); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/core-utils.cjs` (generated from `src/core-utils.cts`).
|
||||
|
||||
@@ -361,6 +361,21 @@ const GSD_HOOK_LIB_FILES = ['git-cmd.js', 'gsd-graphify-rebuild.sh', 'cursor-wor
|
||||
*/
|
||||
const SHARED_HOOKS_DIR_DEFAULT = 'hooks';
|
||||
|
||||
// #3184 — GSD-managed file enumerations for scripts/changeset/ and scripts/lib/
|
||||
// uninstall. The install-side copy of both directories is wholesale ("copy every
|
||||
// file present"), so these enumerations MUST be kept in parity with the real
|
||||
// directory contents or an added file ships on install and then orphans on
|
||||
// uninstall (survives removal, keeps the dir non-empty, blocks its rmdir).
|
||||
// Hoisted to module scope (and exported below) so tests/install.test.cjs can
|
||||
// assert parity against fs.readdirSync(scripts/lib) / fs.readdirSync(scripts/changeset)
|
||||
// without source-grepping this file.
|
||||
const GSD_CHANGESET_FILES = [
|
||||
'cli.cjs', 'parse.cjs', 'render.cjs', 'serialize.cjs',
|
||||
'github-release-notes.cjs', 'lint.cjs', 'new.cjs',
|
||||
'README.md', // documentation only — not user-authored
|
||||
];
|
||||
const GSD_SCRIPTS_LIB_FILES = ['cli-exit.cjs', 'allowlist-ratchet.cjs', 'drift-scan.cjs'];
|
||||
|
||||
/**
|
||||
* Resolve a runtime's shared-hooks directory name from its descriptor.
|
||||
*
|
||||
@@ -8642,13 +8657,8 @@ function uninstall(isGlobal, runtime = DEFAULT_RUNTIME) {
|
||||
// Any file NOT in this set is user-owned and must survive uninstall.
|
||||
// After removing GSD files, attempt to rmdir — if the directory is still
|
||||
// non-empty (user has custom helpers) it stays; otherwise it goes cleanly.
|
||||
const GSD_CHANGESET_FILES = [
|
||||
'cli.cjs', 'parse.cjs', 'render.cjs', 'serialize.cjs',
|
||||
'github-release-notes.cjs', 'lint.cjs', 'new.cjs',
|
||||
'README.md', // documentation only — not user-authored
|
||||
];
|
||||
const GSD_SCRIPTS_LIB_FILES = ['cli-exit.cjs', 'allowlist-ratchet.cjs'];
|
||||
|
||||
// GSD_CHANGESET_FILES / GSD_SCRIPTS_LIB_FILES are module-scoped (#3184) so
|
||||
// tests can assert their parity against the real directory contents.
|
||||
const changesetUninstallDir = path.join(targetDir, 'scripts', 'changeset');
|
||||
if (fs.existsSync(changesetUninstallDir)) {
|
||||
let removedChangeset = 0;
|
||||
@@ -13482,6 +13492,10 @@ module.exports = {
|
||||
// #3023 — shared hook bundle directory name, descriptor-driven
|
||||
SHARED_HOOKS_DIR_DEFAULT,
|
||||
resolveSharedHooksDirName,
|
||||
// #3184 — uninstall-side GSD-managed file enumerations, exported for
|
||||
// parity assertions against the wholesale-copy source directories
|
||||
GSD_CHANGESET_FILES,
|
||||
GSD_SCRIPTS_LIB_FILES,
|
||||
convertSlashCommandsToCodexSkillMentions,
|
||||
convertClaudeCommandToCodexSkill,
|
||||
convertClaudeCommandToKimiSkill,
|
||||
|
||||
@@ -184,6 +184,39 @@ node gsd-tools.cjs roadmap analyze
|
||||
node gsd-tools.cjs roadmap update-plan-progress <N>
|
||||
```
|
||||
|
||||
### Milestone window scope (`roadmap analyze`)
|
||||
|
||||
`roadmap analyze` scopes its phase list to the current milestone's section of
|
||||
`ROADMAP.md`. Its JSON output carries a `scope` field describing how much of the
|
||||
intended input that scoping actually saw:
|
||||
|
||||
| `scope` | Meaning |
|
||||
|---|---|
|
||||
| `complete` | The window was computed over the whole intended input. `phase_count: 0` here is a **real** answer — a freshly-declared milestone genuinely has no phases yet. |
|
||||
| `truncated` | The milestone's heading was found, but its window closed before reaching the document's phase region — typically because a closed-milestone heading sits between the active milestone and its `### Phase N:` sections. `phase_count: 0` here is a **non**-answer. |
|
||||
| `unscoped` | No milestone version could be resolved (or its section is absent) on a ROADMAP that does use versioned milestones, so the result is not milestone-scoped. |
|
||||
| `unreadable` | `ROADMAP.md` could not be read. |
|
||||
|
||||
Before this field existed, all four cases produced the same well-formed
|
||||
`phase_count: 0` with no error, so a consumer could not tell a genuinely empty
|
||||
milestone from a scoping failure. Branch on `scope`, not on `phase_count` alone.
|
||||
|
||||
A ROADMAP with no versioned milestone headings at all (the free-form legacy
|
||||
shape) reports `complete`: the whole document *is* the milestone there.
|
||||
|
||||
### `milestone complete` refuses an untrustworthy window
|
||||
|
||||
`milestone complete` archives `ROADMAP.md`/`REQUIREMENTS.md` and **moves phase
|
||||
directories** — a one-way door. When the milestone window's `scope` is
|
||||
`truncated` — the milestone heading was found but its section closes before
|
||||
reaching any phase entries, even though the ROADMAP has phase entries
|
||||
elsewhere — phase scoping cannot be trusted, and the command now refuses
|
||||
rather than falling back to an over-inclusive filter that would archive every
|
||||
phase directory in the project. `unreadable` (no ROADMAP.md at all) and
|
||||
`unscoped` (no section for this version) are pre-existing, legitimately
|
||||
handled states and are not refused here. Pass `--force` to override, the same
|
||||
affordance the unstarted-phase guard uses.
|
||||
|
||||
---
|
||||
|
||||
## Config Commands
|
||||
|
||||
@@ -472,6 +472,8 @@ If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowle
|
||||
|
||||
> **Note:** the `deferred-items.md` category is the per-phase SCOPE BOUNDARY log a phase agent writes when it finds a defect it should not fix. It is a different artifact from the `## Deferred Items` section `[A]` writes into `STATE.md`, which records what you acknowledged at close.
|
||||
|
||||
> **Truncated-window guard.** Archiving also refuses when the milestone's ROADMAP window is truncated — `Cannot mark milestone complete: the ROADMAP window for "<version>" is truncated`. This is the case where the milestone's heading is found but its section closes before reaching the roadmap's `### Phase N:` region (typically a closed-milestone heading sitting in between), which previously degraded to an over-inclusive filter and archived *every* phase directory in the project rather than the milestone's own. An unreadable ROADMAP.md or a version with no matching section at all are pre-existing, legitimately-handled states and are not refused here. Same override as below: `gsd-tools milestone complete <version> --force`. A window that is genuinely empty — a freshly-declared milestone with no phases yet — is *not* affected and still completes normally.
|
||||
|
||||
> **Unstarted-phase guard.** Archiving refuses if the milestone's ROADMAP still lists a phase with no phase directory on disk — `Cannot mark milestone complete: ROADMAP lists N unstarted phase(s)`. If a phase was intentionally deferred or merged without a directory, run `gsd-tools milestone complete <version> --force` (the `/gsd-complete-milestone` workflow runs the underlying command without `--force`, so use the CLI directly to override). A `STATE.md` `milestone:` value that does not match `<version>` prints a WARNING and still runs the guard (#2946).
|
||||
|
||||
---
|
||||
|
||||
@@ -295,3 +295,59 @@ plan report as a naming violation — the diagnostic reads non-membership as a d
|
||||
`allPlanFiles`. The general rule: a **diagnostic about file naming** wants the physical set; only a
|
||||
question about outstanding *work* wants the live set. Later phases must make that choice explicitly
|
||||
per call site rather than swapping in `planFiles` mechanically.
|
||||
|
||||
### Amendment 2 — Phase 2 (#3184) validation: the contract held; the copy count was low again
|
||||
|
||||
Decision 2's contract needed **no change** for its second consumer: `SCOPE`'s four values covered
|
||||
every row of the windowing derivation's behavior table, including the two rows the epic's text does
|
||||
not distinguish (a free-form legacy ROADMAP with no versioned milestones is `COMPLETE`, not
|
||||
`UNSCOPED` — whole-document genuinely *is* the milestone there). Phase 2 adds no member and changes
|
||||
no semantics. `src/planning-scope.cts` needed no edit, so Phase 2 carries no `.cts` six-gate ripple.
|
||||
|
||||
**What Phase 2 found.** The epic and this ADR both scope milestone windowing at **three** copies, all
|
||||
inside `roadmap-parser.cts`. Building the Decision 4(a) whole-repo guard found **two more**, in a
|
||||
different module and one function down: `state.cts` `buildStateFrontmatter` and `syncStateFrontmatter`
|
||||
each hand-roll `^#{1,3}\s+(?!Phase\s+\S).*${escapeRegex(version)}` to answer "is this milestone
|
||||
bounded to a versioned ROADMAP heading" — the heading-location half of the derivation, byte-identical
|
||||
to each other. This is Phase 1's finding repeating with a different derivation: **the epic's copy
|
||||
counts are a lower bound derived from the reported issues, and the whole-repo guard is what makes them
|
||||
real.** Both sites now call the owner's `isMilestoneBoundedInRoadmap`, which is a straight
|
||||
consolidation of the two identical `state.cts` regexes onto `locateMilestoneHeadings` with **no
|
||||
behavior change** — which is all it should ever have been.
|
||||
|
||||
**A boundary tightening was tried and reverted.** A first pass at `locateMilestoneHeadings` swapped its
|
||||
`\b` version-token boundary for the stricter `(?![\w.-])` used by `isMilestoneShippedInRoadmap`
|
||||
(#2562), reasoning that `v2.0` should not match inside `v2.0.1` anywhere windowing happens. That broke
|
||||
`extractCurrentMilestoneScoped`'s #730 contract: a milestone STATE of `v8.0` legitimately selects the
|
||||
`## v8.0-B …` active sub-milestone heading over a closed `v8.0-A` sibling (`0` is a word character, `-`
|
||||
is not, so `\b` matches; `(?![\w.-])` does not, because `-` is in its excluded set). `\b` is restored in
|
||||
`locateMilestoneHeadings`; the stricter boundary stays local to `isMilestoneShippedInRoadmap` and to the
|
||||
#730 `detailsVersionBoundary`, which answer a narrower question ("is exactly this milestone shipped" /
|
||||
"which Phase Details section is exactly this one's version token's") than "which heading does this
|
||||
milestone STATE select." The consolidation itself (three `roadmap-parser.cts` copies plus the two
|
||||
`state.cts` copies onto one owner) is behavior-preserving.
|
||||
|
||||
**A composition-level re-derivation, caught in review of this phase's own diff.** Decision 4(c)
|
||||
anticipated a consumer post-*filtering* an owner's result. The shape that actually appeared is its
|
||||
mirror: two sites re-*assembling* a window out of the owner's primitives —
|
||||
`locateMilestoneHeadings` → pick a heading → `computeMilestoneSectionEnd` → slice — in
|
||||
`getMilestonePhaseFilter`'s `versionOverride` branch and in `milestone.cts`'s unstarted-phase guard.
|
||||
Both call the canonical owner at every step, so the drift guard and an owner-level identity test are
|
||||
both green, and the two compositions had **already diverged** on whether to skip a closed milestone
|
||||
heading. Decision 4(c) is therefore read to cover **assembly as well as post-processing**: where a
|
||||
derivation has a composition, the composition is itself an owner. Added as
|
||||
`sliceMilestoneWindow`; both sites route through it.
|
||||
|
||||
**Decision 3's Tier-2 table, re-derived for Phase 2** per its own contingency clause. The row this
|
||||
ADR predicted lands as written, plus two the prediction did not contain:
|
||||
|
||||
| Command surface | Output change |
|
||||
|---|---|
|
||||
| `roadmap analyze` | gains a `scope` field. `phase_count: 0` is still emitted verbatim — what changes is that a sibling field now says whether that zero is an answer. Stated precisely because the first draft of this row claimed the count itself changed, which is not what shipped |
|
||||
| `/gsd:progress --next` Route 0 | `gsd-core/workflows/next.md` treats a non-`complete` scope as scan-failed (warn + fall through to the prior-phase check) instead of looping a phase list the scan could not populate. Without this the new field would be a diagnostic no consumer reads, and #3165's actual symptom — the resume invariant reporting clean because it could not run — would still reproduce |
|
||||
| `milestone complete` | refuses (unless `--force`) when the window's scope is `TRUNCATED` — the milestone heading was found but its section closes before reaching any phase entries, even though the ROADMAP has phase entries elsewhere — instead of pass-all archiving every phase directory on disk (#3166). `UNREADABLE` and `UNSCOPED` are pre-existing, legitimately-handled states (`missingExplicitVersion` errors where that matters; a missing ROADMAP.md has its own documented graceful path) and are not refused here. |
|
||||
| `milestone complete` unstarted-phase guard — **not predicted** | the guard scoped its window by STATE.md's `milestone:` field while the filter beside it scoped by the `version` argument; the two could disagree, and the guard under-detected unstarted phases on the destructive path. Both now use the `version` argument. |
|
||||
|
||||
**Scope note.** Phase 3 (enumeration) inherits a window layer that is now single-owner and
|
||||
scope-carrying; its own guard starts from a green windowing baseline, exactly as Phase 1 left plan
|
||||
counting clean for Phase 3.
|
||||
|
||||
@@ -104,11 +104,22 @@ Illustrative bash:
|
||||
```bash
|
||||
INCOMPLETE_PHASE=""
|
||||
ROADMAP_JSON=$(gsd_run query roadmap.analyze)
|
||||
ROADMAP_SCOPE=$(echo "$ROADMAP_JSON" | jq -r '.scope // "complete"')
|
||||
if [ $? -ne 0 ] || [ -z "$ROADMAP_JSON" ]; then
|
||||
echo "⚠ WARNING: resume-incomplete-phase scan could not run (roadmap.analyze failed)." >&2
|
||||
echo " The incomplete-phase invariant (#160) could not be verified." >&2
|
||||
echo " Proceeding to prior-phase completeness check — review project state carefully." >&2
|
||||
# Fall through to prior_phase_completeness rather than silently skipping
|
||||
elif [ "$ROADMAP_SCOPE" != "complete" ]; then
|
||||
# #3184/#3165: roadmap.analyze succeeded and returned a well-formed document,
|
||||
# but its milestone window did not see all of its input, so `.phases[]` is a
|
||||
# NON-answer rather than a real empty. Looping it would run the invariant over
|
||||
# a phase list the scan could not populate and report "clean" — the silent
|
||||
# disarm #3165 reports. Treated as scan-failed, same as an outright failure.
|
||||
echo "⚠ WARNING: resume-incomplete-phase scan could not be scoped (roadmap.analyze scope: $ROADMAP_SCOPE)." >&2
|
||||
echo " The milestone window did not cover the whole ROADMAP, so the phase list is incomplete." >&2
|
||||
echo " The incomplete-phase invariant (#160) could not be verified — review project state carefully." >&2
|
||||
# Fall through to prior_phase_completeness rather than silently passing
|
||||
else
|
||||
for PHASE_NUM in $(echo "$ROADMAP_JSON" | jq -r '.phases[] | (.number // .phase_number // empty)'); do
|
||||
PHASE_JSON=$(gsd_run query find-phase "$PHASE_NUM")
|
||||
@@ -338,6 +349,7 @@ Resume with: `/gsd:progress --next --auto` once resolved.
|
||||
- [ ] `--no-resume`: Route 0 skipped, prior_phase_completeness defer prompt runs as before
|
||||
- [ ] `--force`: everything skipped (Gates, Route 0, prior_phase_completeness) → straight to `determine_next_action`
|
||||
- [ ] Scan uses `gsd_run` (canonical resolver form); errors are surfaced rather than suppressed
|
||||
- [ ] A `roadmap.analyze` result whose `scope` is not `complete` is treated as scan-failed (warn + fall through), never as a clean empty scan (#3184/#3165)
|
||||
- [ ] Predicate is plans-without-summaries (`plans.length > summaries.length`) — consistent with `determine_next_action` Route 4
|
||||
- [ ] Next action correctly determined from routing rules
|
||||
- [ ] Command invoked immediately without user confirmation
|
||||
|
||||
@@ -108,7 +108,7 @@
|
||||
"lint": "eslint . --cache --cache-location node_modules/.cache/eslint/",
|
||||
"lint:fix": "eslint . --fix",
|
||||
"lint:table-schema-drift": "node scripts/lint-table-schema-drift.cjs",
|
||||
"lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs",
|
||||
"lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs",
|
||||
"lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs",
|
||||
"lint:regression-names": "node scripts/lint-regression-test-names.cjs",
|
||||
"lint:descriptions": "node scripts/lint-descriptions.cjs",
|
||||
|
||||
278
scripts/lib/drift-scan.cjs
Normal file
278
scripts/lib/drift-scan.cjs
Normal file
@@ -0,0 +1,278 @@
|
||||
#!/usr/bin/env node
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Shared anti-divergence scanner machinery (epic #3180, ADR-3180 Decision 4).
|
||||
*
|
||||
* Extracted from `scripts/lint-plan-count-drift.cjs` (#3183) so that
|
||||
* `scripts/lint-milestone-window-drift.cjs` (#3184) and any future
|
||||
* `scripts/lint-<derivation>-drift.cjs` guard consume ONE tree-walk /
|
||||
* root-confinement / literal-tokenizer / sanitizer implementation instead of
|
||||
* each copying it verbatim — the exact generative-fix-divergence class this
|
||||
* epic exists to remove, now applied to the guards themselves (see
|
||||
* `.gsd/phase/refactor-3184-milestone-window-single-owner/40-design.md`,
|
||||
* "Rejected: let the new drift guard copy Phase 1's tree-walk /
|
||||
* root-confinement / sanitizer").
|
||||
*
|
||||
* `lint-plan-count-drift.cjs` re-exports every symbol it exported before this
|
||||
* extraction (`readRegexLiteralAt`, `MAX_REGEX_LITERAL_LEN`, `isInsideRoot`,
|
||||
* `sanitizeForReport`), so `tests/plan-count-single-owner.test.cjs` — the
|
||||
* ReDoS and root-confinement regression net for this exact machinery —
|
||||
* continues to pass unchanged and is the regression net for this move too.
|
||||
*/
|
||||
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
// Longest regex literal this scanner will consider, in characters. Real
|
||||
// plan/summary filename filters (and milestone-window regex literals) are far
|
||||
// shorter; the bound is what keeps the scan linear. The tokenizer restarts at
|
||||
// every `/` on the line (so that a literal preceded by a stray unpaired `/`
|
||||
// is still found, matching the previous regex's "find anywhere" behaviour),
|
||||
// which without a per-literal bound would be quadratic on a pathological
|
||||
// line. With it the whole-line cost is O(n * MAX_REGEX_LITERAL_LEN) with no
|
||||
// backtracking at all.
|
||||
const MAX_REGEX_LITERAL_LEN = 400;
|
||||
|
||||
// Directory names this scanner never descends into or reports out of —
|
||||
// `.git` (repo internals, e.g. a persisted CI token in `.git/config`),
|
||||
// `node_modules` (thousands of third-party files, none of them authored
|
||||
// source), `dist` (build output). Named once and used at BOTH skip sites
|
||||
// below: the cheap `entry.name` fast path in `walk`, and the resolved-path
|
||||
// component check in `isUnderSkippedDir` — a symlink whose OWN name is not
|
||||
// in this set but whose target resolves through a directory that IS (e.g.
|
||||
// `src/g -> ../.git`, `src/nm -> ../node_modules`) must still be skipped, or
|
||||
// the name-only check is a trivial bypass.
|
||||
const SKIP_DIR_NAMES = new Set(['node_modules', 'dist', '.git']);
|
||||
|
||||
/**
|
||||
* Read the JS regex literal starting at `line[start]` (which must be `/`).
|
||||
* Returns `{ text, end }` — `text` includes the delimiters and any trailing
|
||||
* flags, `end` is the index one past the literal — or null if no literal
|
||||
* closes within MAX_REGEX_LITERAL_LEN characters.
|
||||
*
|
||||
* Single left-to-right pass, no backtracking. It models the two constructs a
|
||||
* backtracking pattern gets wrong, which is why this is a tokenizer and not a
|
||||
* regex:
|
||||
* - `\x` escapes consume BOTH characters, so an escaped `\/` never
|
||||
* terminates the literal;
|
||||
* - inside a `[...]` character class a bare `/` does NOT terminate, so
|
||||
* `/PLAN[\\/].*\.md$/` is one literal rather than two fragments. The
|
||||
* previous regex silently MISSED every re-derivation using a
|
||||
* cross-platform path-separator class for exactly this reason.
|
||||
*/
|
||||
function readRegexLiteralAt(line, start) {
|
||||
if (line[start] !== '/') return null;
|
||||
const limit = Math.min(line.length, start + MAX_REGEX_LITERAL_LEN);
|
||||
let inClass = false;
|
||||
for (let i = start + 1; i < limit; i++) {
|
||||
const ch = line[i];
|
||||
if (ch === '\\') {
|
||||
i++; // escape consumes the next character, whatever it is
|
||||
continue;
|
||||
}
|
||||
if (ch === '\r' || ch === '\n') return null; // a literal cannot span lines
|
||||
if (ch === '[') {
|
||||
inClass = true;
|
||||
} else if (ch === ']') {
|
||||
inClass = false;
|
||||
} else if (ch === '/' && !inClass) {
|
||||
// Trailing flags are bounded by the SAME `limit` as the literal body
|
||||
// itself (not `line.length`) — a literal followed by an unbounded run
|
||||
// of lowercase letters must not make `text` grow past
|
||||
// MAX_REGEX_LITERAL_LEN either.
|
||||
let end = i + 1;
|
||||
while (end < limit && line[end] >= 'a' && line[end] <= 'z') end++;
|
||||
return { text: line.slice(start, end), end };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// Symlinks report `isDirectory()`/`isFile()` as false on the Dirent from
|
||||
// `readdirSync`, so a symlinked `.cts` (or a symlinked directory containing
|
||||
// one) was previously invisible to this scanner — an evasion of a guard
|
||||
// whose stated design principle (ADR-3180 Decision 4a) is whole-repo
|
||||
// discovery with no allowlist. Resolve each entry with `fs.statSync` (which
|
||||
// follows symlinks) to classify it, skipping broken links. `ctx.visitedRealDirs`
|
||||
// guards against a symlink cycle sending `walk` into infinite recursion.
|
||||
//
|
||||
// Every sibling drift guard in `scripts/` that does NOT import this module
|
||||
// (`lint-phase-id-drift.cjs`, `lint-package-identity-drift.cjs`,
|
||||
// `lint-portable-timeout.cjs`, `lint-test-file-count.cjs`,
|
||||
// `lint-allow-test-rule-refs.cjs`) uses the `Dirent` classification straight
|
||||
// off `readdirSync` and does NOT follow symlinks at all. This scanner follows
|
||||
// them so a symlinked source file cannot evade ADR-3180 Decision 4a's
|
||||
// whole-repo discovery — root confinement (`isInsideRoot` below) is the price
|
||||
// of doing so: without it, a symlink planted anywhere under a scan directory
|
||||
// could walk this scanner out to read and report arbitrary files elsewhere on
|
||||
// disk.
|
||||
//
|
||||
// DIRECTORY vs FILE symlinks are confined to two DIFFERENT roots, tracked as
|
||||
// `ctx.scanDirRoot` (the realpath of the current top-level scan-dir entry,
|
||||
// e.g. `<realRoot>/src`) vs `ctx.realRoot` (the whole repo):
|
||||
// - a DIRECTORY symlink is descended ONLY if its resolved realpath is
|
||||
// inside `ctx.scanDirRoot` — NOT merely inside `ctx.realRoot`. Without
|
||||
// this, `src/up -> ..` (or `-> <realRoot>`) resolves inside the repo
|
||||
// root and `walk` descends the ENTIRE repo, reporting violations under
|
||||
// paths like `tests/not-src.cts` or `docs/other.cts` — files the caller's
|
||||
// scan-dir list scopes it OUT of. This is a deliberate, fail-CLOSED
|
||||
// trade-off: a directory symlink pointing elsewhere INSIDE the repo (but
|
||||
// outside the scan directory) is simply not followed. The alternative —
|
||||
// descending it — is exactly the whole-repo sweep this rule exists to
|
||||
// prevent, and a fork PR could use that sweep to redden `lint:ci` on
|
||||
// files this guard was never meant to read. The narrower rule is worth
|
||||
// more than the missed edge case.
|
||||
// - a FILE symlink is still scanned if its resolved realpath is inside
|
||||
// `ctx.realRoot` (the whole repo, not just the scan directory) — this is
|
||||
// what keeps `src/alias.cts -> vendor/real.cts` covered (test (f)): an
|
||||
// aliased file genuinely is part of the compiled surface even when its
|
||||
// real target lives outside `src/`, and it is still reported under its
|
||||
// canonical (real) path.
|
||||
//
|
||||
// A resolved path is inside a root only if it IS that root or begins with
|
||||
// root + separator — a plain `startsWith(root)` would also accept a sibling
|
||||
// directory whose name merely starts with the root's name (`/repo-evil`).
|
||||
function isInsideRoot(realPath, realRoot) {
|
||||
return realPath === realRoot || realPath.startsWith(realRoot + path.sep);
|
||||
}
|
||||
|
||||
// True when `realPath` (already confirmed inside `realRoot` by `isInsideRoot`)
|
||||
// resolves THROUGH a skip-list directory anywhere along its path relative to
|
||||
// the root — not just when `realPath` itself IS one. This is what closes the
|
||||
// symlink bypass the `entry.name` fast path alone cannot: `walk` tests
|
||||
// `entry.name` (the symlink's OWN name in its parent directory), but a
|
||||
// symlink named something innocuous can still RESOLVE into `.git` /
|
||||
// `node_modules` / `dist` (`src/g -> ../.git`, `src/leak.cts ->
|
||||
// ../.git/config`, `src/nm -> ../node_modules`) — `isInsideRoot` alone admits
|
||||
// all three, because every one of those real paths is still under the root.
|
||||
function isUnderSkippedDir(realPath, realRoot) {
|
||||
const rel = path.relative(realRoot, realPath);
|
||||
return rel.split(path.sep).some((segment) => SKIP_DIR_NAMES.has(segment));
|
||||
}
|
||||
|
||||
// `ctx.scanExt` is a `Set` of file extensions (e.g. `.cts`/`.ts`/`.mts`) the
|
||||
// caller wants reported — threaded through `ctx` rather than as a positional
|
||||
// parameter so recursive `walk(full, acc, ctx)` calls stay unchanged.
|
||||
function walk(dir, acc, ctx) {
|
||||
let entries;
|
||||
try {
|
||||
entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return acc;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (SKIP_DIR_NAMES.has(entry.name)) continue; // cheap fast path
|
||||
let stat;
|
||||
try {
|
||||
stat = entry.isSymbolicLink() ? fs.statSync(full) : entry;
|
||||
} catch {
|
||||
continue; // broken symlink target
|
||||
}
|
||||
let realPath;
|
||||
try {
|
||||
realPath = fs.realpathSync(full);
|
||||
} catch {
|
||||
continue; // broken symlink target (race, or a link stat() followed but realpath cannot)
|
||||
}
|
||||
if (stat.isDirectory()) {
|
||||
// Directories (symlinked or real) are confined to the CURRENT scan
|
||||
// directory root, not merely the repo root — see the comment above
|
||||
// `isInsideRoot` for why (`src/up -> ..` whole-repo sweep).
|
||||
if (!isInsideRoot(realPath, ctx.scanDirRoot)) continue;
|
||||
if (isUnderSkippedDir(realPath, ctx.realRoot)) continue; // symlink resolves through a skipped dir
|
||||
if (ctx.visitedRealDirs.has(realPath)) continue; // symlink cycle guard
|
||||
ctx.visitedRealDirs.add(realPath);
|
||||
walk(full, acc, ctx);
|
||||
} else if (stat.isFile() && ctx.scanExt.has(path.extname(entry.name))) {
|
||||
// Files are confined to the whole repo root — a symlinked FILE whose
|
||||
// real target lives outside the scan directory but inside the repo
|
||||
// (e.g. `src/alias.cts -> vendor/real.cts`) is still part of the
|
||||
// compiled surface and must be scanned.
|
||||
if (!isInsideRoot(realPath, ctx.realRoot)) continue;
|
||||
if (isUnderSkippedDir(realPath, ctx.realRoot)) continue; // symlink resolves through a skipped dir
|
||||
if (ctx.visitedRealFiles.has(realPath)) continue; // two symlinks, same real file
|
||||
ctx.visitedRealFiles.add(realPath);
|
||||
acc.push(realPath);
|
||||
}
|
||||
}
|
||||
return acc;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whole-repo tree-walk driver shared by every `lint-<derivation>-drift.cjs`
|
||||
* guard. Resolves `root`, walks each of `scanDirs` filtered to `scanExt`
|
||||
* (symlink-following, root-confined, cycle-guarded — see `walk` above), reads
|
||||
* each discovered file, and calls `onFile(relPath, text)` for it — `relPath`
|
||||
* is repo-relative and already the file's canonical (real) path, so a
|
||||
* per-file exemption keyed on `relPath` matches consistently regardless of
|
||||
* which symlink reached it.
|
||||
*
|
||||
* `onFile` returns an array of violation objects (or an empty array / null /
|
||||
* undefined for "no violations in this file"); `scanTree` flattens them all
|
||||
* into one returned array. Pure I/O orchestration — detection logic lives
|
||||
* entirely in the caller's `onFile`.
|
||||
*/
|
||||
function scanTree({ root, scanDirs, scanExt, onFile }) {
|
||||
const violations = [];
|
||||
let realRoot;
|
||||
try {
|
||||
realRoot = fs.realpathSync(root);
|
||||
} catch {
|
||||
return violations; // root itself does not exist / is unreadable
|
||||
}
|
||||
for (const dir of scanDirs) {
|
||||
const scanDirPath = path.join(root, dir);
|
||||
let scanDirRoot;
|
||||
try {
|
||||
scanDirRoot = fs.realpathSync(scanDirPath);
|
||||
} catch {
|
||||
continue; // scan directory itself does not exist / is unreadable
|
||||
}
|
||||
const ctx = { realRoot, scanDirRoot, scanExt, visitedRealDirs: new Set(), visitedRealFiles: new Set() };
|
||||
for (const file of walk(scanDirPath, [], ctx)) {
|
||||
const rel = path.relative(realRoot, file);
|
||||
let text;
|
||||
try {
|
||||
text = fs.readFileSync(file, 'utf8');
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
const found = onFile(rel, text);
|
||||
if (found && found.length > 0) violations.push(...found);
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
// A reported fragment AND a reported file path are both attacker-controlled
|
||||
// source text on a fork PR (a repo can legally track a filename containing
|
||||
// control bytes, so the path is exactly as attacker-controlled as the
|
||||
// fragment), and both are written straight to a CI log. Replace C0/C1
|
||||
// control bytes (ANSI escapes included) with a visible \xNN, AND the
|
||||
// non-Latin-1 formatting/bidi/line-separator codepoints below with \uNNNN, so
|
||||
// a crafted literal or filename cannot rewrite the terminal rendering of the
|
||||
// report or hide/reorder its own text:
|
||||
// - U+200B-U+200F: zero-width space/joiners and directional marks
|
||||
// - U+2028/U+2029: Unicode LINE SEPARATOR / PARAGRAPH SEPARATOR (line
|
||||
// breaks a `\n`-only log scan would not catch)
|
||||
// - U+202A-U+202E: bidi embedding/override controls (RLO etc.)
|
||||
// - U+2066-U+2069: bidi isolate controls
|
||||
function sanitizeForReport(text) {
|
||||
return text
|
||||
// eslint-disable-next-line no-control-regex -- the control range IS the target
|
||||
.replace(/[\x00-\x1f\x7f-\x9f]/g, (c) => '\\x' + c.charCodeAt(0).toString(16).padStart(2, '0'))
|
||||
.replace(/[\u200B-\u200F\u2028\u2029\u202A-\u202E\u2066-\u2069]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0'));
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
SKIP_DIR_NAMES,
|
||||
isInsideRoot,
|
||||
isUnderSkippedDir,
|
||||
walk,
|
||||
readRegexLiteralAt,
|
||||
MAX_REGEX_LITERAL_LEN,
|
||||
sanitizeForReport,
|
||||
scanTree,
|
||||
};
|
||||
300
scripts/lint-milestone-window-drift.cjs
Normal file
300
scripts/lint-milestone-window-drift.cjs
Normal file
@@ -0,0 +1,300 @@
|
||||
#!/usr/bin/env node
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Anti-divergence drift guard for the milestone-WINDOWING seam
|
||||
* (epic #3180, issue #3184, ADR-3180 "Planning Semantic Model Single Owner").
|
||||
*
|
||||
* `src/roadmap-parser.cts` is the SINGLE canonical owner of "where does a
|
||||
* given milestone's ROADMAP section begin and end" — `computeMilestoneSectionEnd`,
|
||||
* `locateMilestoneHeadings`, and `isMilestoneBoundedInRoadmap`. Every other
|
||||
* module that hand-rolls a heading-level quantifier together with the
|
||||
* milestone-boundary shape (a non-Phase heading carrying a version token or a
|
||||
* shipped/active marker) is a re-derivation that can silently drift from the
|
||||
* owner — the exact defect class #2562 fixed in one copy and never reached
|
||||
* the other two (design doc: `currentMilestoneRawRanges::computeSectionEnd`
|
||||
* carried a "keep in sync" comment that was already evidence the risk was
|
||||
* known, not controlled).
|
||||
*
|
||||
* Per ADR-3180 Decision 4(a) this guard discovers call sites by SCANNING THE
|
||||
* WHOLE `src/` TREE, not by consulting an allowlist of known files — an
|
||||
* allowlist only measures re-derivations in files someone remembered to
|
||||
* list. Exemptions below are FUNCTION-SCOPED with a written reason, never a
|
||||
* bare file allowlist, mirroring `lint-plan-count-drift.cjs`'s precedent.
|
||||
*
|
||||
* Detection is intentionally NARROW, mirroring the plan-count-drift and
|
||||
* phase-id-drift precedents: a line is a re-derivation when it carries BOTH,
|
||||
* in ONE source line:
|
||||
* (a) a markdown heading-level quantifier token — `#{N,M}`, e.g. `#{1,3}`,
|
||||
* `#{2,3}`, `#{2,4}` — inside either a regex literal or a
|
||||
* quoted/backticked string, AND
|
||||
* (b) a milestone-window token: either the negative-lookahead phase
|
||||
* exclusion (`(?!Phase` / `(?!Phase\s+\S)`) or the milestone
|
||||
* boundary-marker set — a `v\d+\.\d+`-shaped version token appearing
|
||||
* together with any of the ✅ 📋 🚧 shipped/active markers.
|
||||
* Token (b) is deliberately narrow: a PHASE-heading regex (`#{2,4}\s*Phase`)
|
||||
* carries (a) alone, constantly, throughout this codebase (phase-numbering,
|
||||
* plan-index, wave-scheduling call sites) and must NOT be flagged — it asks
|
||||
* "where is phase N's heading", a different, already-single-owned question
|
||||
* (#2121). Only a line that ALSO carries the milestone-boundary shape — the
|
||||
* one `computeMilestoneSectionEnd`/`locateMilestoneHeadings` compute — is a
|
||||
* candidate re-derivation of THIS derivation.
|
||||
*
|
||||
* Both `(a)` and `(b)` must be readable through JS regex-literal AND
|
||||
* string/template-literal escaping: the two pre-#3184 `state.cts`
|
||||
* re-derivations this design is modelled on were
|
||||
* `new RegExp(\`^#{1,3}\\s+(?!Phase\\s+\\S)...\`)` — i.e. the SAME source
|
||||
* text as a real `/.../ ` regex literal, just doubly backslash-escaped
|
||||
* because it lives inside a template literal. `HEADING_QUANTIFIER_RE` and
|
||||
* `PHASE_LOOKAHEAD_RE` match either escaping level unchanged (no backslash
|
||||
* appears inside `#{`/`}`/`(?!Phase`'s literal characters); `VERSION_TOKEN_RE`
|
||||
* explicitly tolerates ONE or TWO backslashes before each `d`/`.` for exactly
|
||||
* this reason. Regex-LITERAL boundaries (used only to extract a reportable
|
||||
* `found` fragment, never for detection itself, which tests the raw line) are
|
||||
* located via the shared `readRegexLiteralAt` tokenizer
|
||||
* (`scripts/lib/drift-scan.cjs`) — a single left-to-right, no-backtracking
|
||||
* pass — never a backtracking "find the regex literal" regex (CodeQL js/redos
|
||||
* runs on `lint:ci`; see that module's own header for the full rationale).
|
||||
* `readStringLiteralAt` below is the same style, written locally for
|
||||
* quoted/backticked strings (not shared — `lint-plan-count-drift.cjs` has no
|
||||
* equivalent need, since its own literal-bearing shape is regex-only).
|
||||
*
|
||||
* Owner file (exempt by construction): `src/roadmap-parser.cts` — it not only
|
||||
* DEFINES this grammar but composes `#{1,3}` with `(?!Phase...)`/marker
|
||||
* alternations at several internal call sites (`computeMilestoneSectionEnd`,
|
||||
* `locateMilestoneHeadings`, `extractCurrentMilestoneScoped`'s
|
||||
* `anyMilestonePattern`/`anyMilestoneOrDetails`) that are the canonical
|
||||
* implementation, not copies of it.
|
||||
*
|
||||
* The tree-walk / root-confinement / regex-literal-tokenizer / sanitizer
|
||||
* machinery is SHARED with `scripts/lint-plan-count-drift.cjs` via
|
||||
* `scripts/lib/drift-scan.cjs` (ADR-3180 Decision 4, design doc's own
|
||||
* "Rejected: let the new drift guard copy Phase 1's tree-walk /
|
||||
* root-confinement / sanitizer") — see that module for the `isInsideRoot`
|
||||
* case-sensitivity note, the `walk` symlink-confinement rationale, and the
|
||||
* `readRegexLiteralAt` ReDoS-avoidance rationale.
|
||||
*
|
||||
* KNOWN, ACCEPTED limits of a per-line textual scan (same tradeoff the
|
||||
* sibling drift guards document): a re-derivation whose `(a)`/`(b)` tokens
|
||||
* are split across two DIFFERENT lines with no single line carrying both is
|
||||
* not caught by this narrow shape, nor is one routed through dynamic
|
||||
* dispatch. That is left to code review and the design's identity test
|
||||
* (ADR-3180 Decision 4b/4c), not this regex.
|
||||
*/
|
||||
|
||||
const path = require('node:path');
|
||||
const driftScan = require('./lib/drift-scan.cjs');
|
||||
const { readRegexLiteralAt, MAX_REGEX_LITERAL_LEN, sanitizeForReport, scanTree } = driftScan;
|
||||
|
||||
// (a) A markdown heading-level quantifier: `#{N,M}` — e.g. `#{1,3}`,
|
||||
// `#{2,3}`, `#{2,4}`. Bounded to 1-2 digit levels (real Markdown headings
|
||||
// never exceed level 6) so this stays a small, fixed, linear test — no
|
||||
// unbounded quantifier, nothing for CodeQL js/redos to flag.
|
||||
const HEADING_QUANTIFIER_RE = /#\{\d{1,2},\d{1,2}\}/;
|
||||
|
||||
// (b1) The negative-lookahead phase exclusion `computeMilestoneSectionEnd`/
|
||||
// `locateMilestoneHeadings` use to skip `### Phase N: …` headings while
|
||||
// scanning for the NEXT milestone boundary.
|
||||
const PHASE_LOOKAHEAD_RE = /\(\?!Phase\b/;
|
||||
|
||||
// (b2) A `v\d+\.\d+`-shaped version token, tolerant of ONE or TWO backslash
|
||||
// escaping levels (a bare regex literal carries `\d`/`\.` with a single
|
||||
// backslash; a template-literal regex SOURCE string carries the SAME source
|
||||
// text doubly-escaped, `\\d`/`\\.`, because the template literal's own
|
||||
// backslash must itself be escaped in the .cts source) and an OPTIONAL
|
||||
// capturing group immediately around the digit run (`v(\d+)\.\d+`, the shape
|
||||
// `roadmap-command-router.cts`'s `MILESTONE_RE` actually uses to capture the
|
||||
// major version number).
|
||||
const VERSION_TOKEN_RE = /v\(?\\{1,2}d\+\)?\\{1,2}\.\\{1,2}d\+/;
|
||||
|
||||
// (b2) The milestone shipped/active marker set `isClosedMilestoneHeading`/
|
||||
// `computeMilestoneSectionEnd` test for. `(b)` fires when this appears on the
|
||||
// SAME line as a VERSION_TOKEN_RE match — a version token alone is not
|
||||
// milestone-boundary-specific (plenty of non-heading code compares version
|
||||
// strings), and a marker alone is not either (it can appear in unrelated
|
||||
// prose-matching code); together, on one line, they are the boundary shape.
|
||||
const MARKER_EMOJI_RE = /[✅📋🚧]/u;
|
||||
|
||||
// Authored TypeScript source only (the generated bin/lib/*.cjs mirror it).
|
||||
const SCAN_DIRS = ['src'];
|
||||
const SCAN_EXT = new Set(['.cts', '.ts', '.mts']);
|
||||
|
||||
// The canonical owner defines the grammar; it is exempt by construction (see
|
||||
// header comment for why its OWN internal composition of these tokens is not
|
||||
// a re-derivation).
|
||||
const OWNER_FILE = path.join('src', 'roadmap-parser.cts');
|
||||
|
||||
// Per ADR-3180 Decision 4(a): NOT a bare file allowlist — each entry below is
|
||||
// scoped to the SPECIFIC function asking a documented, DIFFERENT question, so
|
||||
// an unrelated re-derivation added anywhere else in these same files is still
|
||||
// caught. Mirrors `lint-plan-count-drift.cjs`'s FUNCTION_SCOPED_EXEMPTIONS
|
||||
// mechanism.
|
||||
//
|
||||
// - roadmap-command-router.cts checkW021: `MILESTONE_RE` CLASSIFIES a
|
||||
// single heading LINE as "is this a milestone heading, and if so what is
|
||||
// its major version" for the W021 phase/milestone-prefix-mismatch check
|
||||
// — it is a per-line classifier consumed one line at a time via
|
||||
// `content.split('\n')`, with no concept of a section END at all. It
|
||||
// never computes "where does this milestone's content stop" — the
|
||||
// question `computeMilestoneSectionEnd` answers — so it cannot diverge
|
||||
// from that computation; it answers a narrower, different question this
|
||||
// derivation does not own.
|
||||
// - verify.cts checkMilestonePrefixMismatches: `sectionRx` ENUMERATES
|
||||
// every milestone heading in the document to build a list of
|
||||
// `{version, start, end}` sections (each section's `end` is provisionally
|
||||
// "rest of document" until the NEXT heading is found, then backfilled) —
|
||||
// it is answering "what are ALL the milestone sections", to check every
|
||||
// phase against its OWN enclosing milestone, not "where does THIS ONE
|
||||
// milestone (the current/asserted one) end" — `computeMilestoneSectionEnd`
|
||||
// takes a single heading and returns a single boundary; this function
|
||||
// never calls anything with that shape. (Design brief named this
|
||||
// `cmdValidateConsistency` — the code actually lives in the sibling
|
||||
// function `checkMilestonePrefixMismatches`, called from
|
||||
// `cmdValidateHealth`; `cmdValidateConsistency` itself does not contain
|
||||
// `sectionRx`. Exempted here under its ACTUAL containing function.) Also:
|
||||
// `sectionRx` (`/^#{1,3}\s+(?:\[[^\]]{1,200}\]\s*)?.*v(\d+\.\d+)/gim`)
|
||||
// does not itself carry token (b) as this guard defines it (no
|
||||
// `(?!Phase` lookahead, no marker-emoji pairing) — this exemption
|
||||
// currently documents intent rather than suppressing a live match.
|
||||
const FUNCTION_SCOPED_EXEMPTIONS = new Map([
|
||||
[path.join('src', 'roadmap-command-router.cts'), new Set(['checkW021'])],
|
||||
[path.join('src', 'verify.cts'), new Set(['checkMilestonePrefixMismatches'])],
|
||||
]);
|
||||
|
||||
// Optional `export ` modifier, mirroring `lint-plan-count-drift.cjs`'s
|
||||
// TOP_LEVEL_FUNCTION_RE — only a column-0 top-level `function` declaration
|
||||
// updates the current-function tracker; a nested/arrow function does not
|
||||
// reset it, matching every FUNCTION_SCOPED_EXEMPTIONS entry above (all
|
||||
// top-level `function` declarations).
|
||||
const TOP_LEVEL_FUNCTION_RE = /^(?:export\s+)?function\s+([A-Za-z0-9_]+)\s*\(/;
|
||||
|
||||
/**
|
||||
* Read the quoted or backtick-delimited string/template literal starting at
|
||||
* `line[start]` (which must be `'`, `"`, or `` ` ``). Returns `{ text, end }`
|
||||
* — `text` includes both delimiters, `end` is the index one past the literal
|
||||
* — or null if no matching close quote is found within MAX_REGEX_LITERAL_LEN
|
||||
* characters. Same single left-to-right, no-backtracking, escape-aware style
|
||||
* as the shared `readRegexLiteralAt` (`\x` escapes consume both characters,
|
||||
* so an escaped quote never terminates the literal early) — written locally
|
||||
* because `lint-plan-count-drift.cjs` has no equivalent need (its literal
|
||||
* shape is regex-only), so it does not belong in the shared module.
|
||||
*/
|
||||
function readStringLiteralAt(line, start) {
|
||||
const quote = line[start];
|
||||
if (quote !== "'" && quote !== '"' && quote !== '`') return null;
|
||||
const limit = Math.min(line.length, start + MAX_REGEX_LITERAL_LEN);
|
||||
for (let i = start + 1; i < limit; i++) {
|
||||
const ch = line[i];
|
||||
if (ch === '\\') {
|
||||
i++; // escape consumes the next character, whatever it is
|
||||
continue;
|
||||
}
|
||||
if (ch === '\r' || ch === '\n') return null; // a literal cannot span lines in this per-line scan
|
||||
if (ch === quote) return { text: line.slice(start, i + 1), end: i + 1 };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The first literal (regex OR quoted/backtick string) on `line` whose text
|
||||
* contains a HEADING_QUANTIFIER_RE match — the "smoking gun" fragment worth
|
||||
* reporting, mirroring `findRegexLiteralMdMatch`'s role in the sibling guard.
|
||||
* Falls back to a bounded, trimmed slice of the raw line when the tokens are
|
||||
* not both inside one located literal (not currently reachable against this
|
||||
* repo — see the header comment's per-file audit — but a fail-safe rather
|
||||
* than a thrown error if a future line splits them).
|
||||
*/
|
||||
function extractFragment(line) {
|
||||
for (let i = 0; i < line.length; i++) {
|
||||
const ch = line[i];
|
||||
let literal = null;
|
||||
if (ch === '/') literal = readRegexLiteralAt(line, i);
|
||||
else if (ch === "'" || ch === '"' || ch === '`') literal = readStringLiteralAt(line, i);
|
||||
if (!literal) continue;
|
||||
if (HEADING_QUANTIFIER_RE.test(literal.text)) return literal.text;
|
||||
i = literal.end - 1; // resume scanning just past this literal
|
||||
}
|
||||
return line.trim().slice(0, MAX_REGEX_LITERAL_LEN);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: find every unsanctioned milestone-window re-derivation in `text`.
|
||||
* `relPath` is the repo-relative path, used both to report file:line and to
|
||||
* apply the narrow, function-scoped exemptions above.
|
||||
* Returns [{ line, found }].
|
||||
*/
|
||||
function findMilestoneWindowDrift(text, relPath) {
|
||||
const out = [];
|
||||
const lines = text.split('\n');
|
||||
const exemptFunctions = FUNCTION_SCOPED_EXEMPTIONS.get(relPath) || null;
|
||||
let currentFunction = null;
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
const fnMatch = TOP_LEVEL_FUNCTION_RE.exec(line);
|
||||
if (fnMatch) currentFunction = fnMatch[1];
|
||||
|
||||
if (!HEADING_QUANTIFIER_RE.test(line)) continue;
|
||||
const isMilestoneWindowToken = PHASE_LOOKAHEAD_RE.test(line) || (VERSION_TOKEN_RE.test(line) && MARKER_EMOJI_RE.test(line));
|
||||
if (!isMilestoneWindowToken) continue;
|
||||
|
||||
if (exemptFunctions && exemptFunctions.has(currentFunction)) continue;
|
||||
|
||||
out.push({ line: i + 1, found: extractFragment(line) });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* Scan the authored source tree and return every unsanctioned re-derivation,
|
||||
* each annotated with the repo-relative file path.
|
||||
*/
|
||||
function scanRepo(root) {
|
||||
return scanTree({
|
||||
root,
|
||||
scanDirs: SCAN_DIRS,
|
||||
scanExt: SCAN_EXT,
|
||||
onFile(rel, text) {
|
||||
// `rel` is already the REAL (canonical) path (scanTree resolves
|
||||
// symlinks before calling onFile), so this comparison — and
|
||||
// FUNCTION_SCOPED_EXEMPTIONS above, also keyed on `rel` — match
|
||||
// consistently regardless of which symlink reached the file.
|
||||
if (rel === OWNER_FILE) return [];
|
||||
return findMilestoneWindowDrift(text, rel).map((d) => ({ file: rel, ...d }));
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function main() {
|
||||
const root = path.join(__dirname, '..');
|
||||
const violations = scanRepo(root);
|
||||
if (violations.length === 0) {
|
||||
process.stdout.write('ok milestone-window-drift: no unsanctioned milestone-window re-derivations outside roadmap-parser.cts\n');
|
||||
return;
|
||||
}
|
||||
process.stderr.write('milestone-window-drift: independent re-derivation(s) of milestone-window bounding found.\n');
|
||||
process.stderr.write('Use src/roadmap-parser.cjs `computeMilestoneSectionEnd` / `locateMilestoneHeadings` /\n');
|
||||
process.stderr.write('`isMilestoneBoundedInRoadmap` instead of re-deriving the milestone heading/boundary regex:\n');
|
||||
for (const d of violations) {
|
||||
// `d.file` is exactly as attacker-controlled as `d.found`: a repo can
|
||||
// legally track a filename containing control bytes / bidi overrides,
|
||||
// and it is a fork-PR-authored value reaching a CI log the same way the
|
||||
// matched literal does — sanitize it at the same reporting boundary.
|
||||
process.stderr.write(` ${sanitizeForReport(d.file)}:${d.line} ${sanitizeForReport(d.found)}\n`);
|
||||
}
|
||||
process.exitCode = 1;
|
||||
}
|
||||
|
||||
if (require.main === module) main();
|
||||
|
||||
module.exports = {
|
||||
findMilestoneWindowDrift,
|
||||
scanRepo,
|
||||
HEADING_QUANTIFIER_RE,
|
||||
PHASE_LOOKAHEAD_RE,
|
||||
VERSION_TOKEN_RE,
|
||||
MARKER_EMOJI_RE,
|
||||
OWNER_FILE,
|
||||
FUNCTION_SCOPED_EXEMPTIONS,
|
||||
readStringLiteralAt,
|
||||
extractFragment,
|
||||
};
|
||||
@@ -47,18 +47,14 @@
|
||||
* across two DIFFERENT lines with no single line carrying both, is not
|
||||
* caught by this narrow shape. That is left to code review, not this regex.
|
||||
*
|
||||
* `isInsideRoot`'s root-confinement check (used by the symlink-following
|
||||
* `walk`, below) is an EXACT string comparison, deliberately not
|
||||
* case-normalized. On a case-insensitive filesystem (macOS default; not CI,
|
||||
* which is ubuntu) a symlink whose target is a case-VARIANT of an in-root
|
||||
* path — a path the OS itself would still resolve to the same file — is
|
||||
* REJECTED by this exact comparison and silently left unscanned. This is a
|
||||
* fail-CLOSED miss (a re-derivation goes unreported), never an escape (never
|
||||
* a wrongly-admitted outside-root read), so it is left as-is: making the
|
||||
* comparison case-insensitive would WEAKEN confinement (a resolved path
|
||||
* merely case-differing from a sibling-of-root name, `/repo-Evil` vs
|
||||
* `/repo-evil`, could then be wrongly admitted) to fix a gap that only ever
|
||||
* under-reports on a platform this guard is not gated on.
|
||||
* The tree-walk / root-confinement / regex-literal-tokenizer / sanitizer
|
||||
* machinery below is SHARED with `scripts/lint-milestone-window-drift.cjs`
|
||||
* (#3184) via `scripts/lib/drift-scan.cjs` — see that module for the
|
||||
* `isInsideRoot` case-sensitivity note, the `walk` symlink-confinement
|
||||
* rationale, and the `readRegexLiteralAt` tokenizer's ReDoS-avoidance
|
||||
* rationale. It is deliberately NOT duplicated here a second time (ADR-3180
|
||||
* Decision 4's own "Rejected" list: "let the new drift guard copy Phase 1's
|
||||
* tree-walk / root-confinement / sanitizer").
|
||||
*
|
||||
* A regex literal longer than MAX_REGEX_LITERAL_LEN (400) characters is not
|
||||
* read, and is therefore not caught. That bound is what keeps the scan
|
||||
@@ -74,8 +70,9 @@
|
||||
* survive, and the reason the detector is now a tokenizer.
|
||||
*/
|
||||
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const driftScan = require('./lib/drift-scan.cjs');
|
||||
const { readRegexLiteralAt, MAX_REGEX_LITERAL_LEN, isInsideRoot, sanitizeForReport, scanTree } = driftScan;
|
||||
|
||||
// A `.filter(` call on the line — the shape every current re-derivation uses
|
||||
// to turn a directory listing into a plan-or-summary subset. Kept as its own
|
||||
@@ -96,15 +93,6 @@ const FILENAME_TEST_RE = /\.(?:filter|test|match|exec|endsWith|startsWith|includ
|
||||
// closing quote must match).
|
||||
const PLAN_SUMMARY_LITERAL_RE = /(['"])-?(?:PLAN|SUMMARY)\.md\1/;
|
||||
|
||||
// Longest regex literal this scanner will consider, in characters. Real
|
||||
// plan/summary filename filters are far shorter; the bound is what keeps the
|
||||
// scan linear. The tokenizer restarts at every `/` on the line (so that a
|
||||
// literal preceded by a stray unpaired `/` is still found, matching the
|
||||
// previous regex's "find anywhere" behaviour), which without a per-literal
|
||||
// bound would be quadratic on a pathological line. With it the whole-line
|
||||
// cost is O(n * MAX_REGEX_LITERAL_LEN) with no backtracking at all.
|
||||
const MAX_REGEX_LITERAL_LEN = 400;
|
||||
|
||||
// The two tokens that, appearing together INSIDE one regex literal, make it a
|
||||
// plan/summary filename filter. `\.md` is matched as literal source text, not
|
||||
// as a pattern, so there is nothing here to backtrack.
|
||||
@@ -115,17 +103,6 @@ const ESCAPED_MD_TOKEN = '\\.md';
|
||||
const SCAN_DIRS = ['src'];
|
||||
const SCAN_EXT = new Set(['.cts', '.ts', '.mts']);
|
||||
|
||||
// Directory names this scanner never descends into or reports out of —
|
||||
// `.git` (repo internals, e.g. a persisted CI token in `.git/config`),
|
||||
// `node_modules` (thousands of third-party files, none of them authored
|
||||
// source), `dist` (build output). Named once and used at BOTH skip sites
|
||||
// below: the cheap `entry.name` fast path in `walk`, and the resolved-path
|
||||
// component check in `isUnderSkippedDir` — a symlink whose OWN name is not
|
||||
// in this set but whose target resolves through a directory that IS (e.g.
|
||||
// `src/g -> ../.git`, `src/nm -> ../node_modules`) must still be skipped, or
|
||||
// the name-only check is a trivial bypass.
|
||||
const SKIP_DIR_NAMES = new Set(['node_modules', 'dist', '.git']);
|
||||
|
||||
// The canonical owner defines the grammar; it is exempt by construction.
|
||||
const OWNER_FILE = path.join('src', 'plan-scan.cts');
|
||||
|
||||
@@ -197,50 +174,6 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([
|
||||
// FUNCTION_SCOPED_EXEMPTIONS entry above to take effect.
|
||||
const TOP_LEVEL_FUNCTION_RE = /^(?:export\s+)?function\s+([A-Za-z0-9_]+)\s*\(/;
|
||||
|
||||
/**
|
||||
* Read the JS regex literal starting at `line[start]` (which must be `/`).
|
||||
* Returns `{ text, end }` — `text` includes the delimiters and any trailing
|
||||
* flags, `end` is the index one past the literal — or null if no literal
|
||||
* closes within MAX_REGEX_LITERAL_LEN characters.
|
||||
*
|
||||
* Single left-to-right pass, no backtracking. It models the two constructs a
|
||||
* backtracking pattern gets wrong, which is why this is a tokenizer and not a
|
||||
* regex:
|
||||
* - `\x` escapes consume BOTH characters, so an escaped `\/` never
|
||||
* terminates the literal;
|
||||
* - inside a `[...]` character class a bare `/` does NOT terminate, so
|
||||
* `/PLAN[\\/].*\.md$/` is one literal rather than two fragments. The
|
||||
* previous regex silently MISSED every re-derivation using a
|
||||
* cross-platform path-separator class for exactly this reason.
|
||||
*/
|
||||
function readRegexLiteralAt(line, start) {
|
||||
if (line[start] !== '/') return null;
|
||||
const limit = Math.min(line.length, start + MAX_REGEX_LITERAL_LEN);
|
||||
let inClass = false;
|
||||
for (let i = start + 1; i < limit; i++) {
|
||||
const ch = line[i];
|
||||
if (ch === '\\') {
|
||||
i++; // escape consumes the next character, whatever it is
|
||||
continue;
|
||||
}
|
||||
if (ch === '\r' || ch === '\n') return null; // a literal cannot span lines
|
||||
if (ch === '[') {
|
||||
inClass = true;
|
||||
} else if (ch === ']') {
|
||||
inClass = false;
|
||||
} else if (ch === '/' && !inClass) {
|
||||
// Trailing flags are bounded by the SAME `limit` as the literal body
|
||||
// itself (not `line.length`) — a literal followed by an unbounded run
|
||||
// of lowercase letters must not make `text` grow past
|
||||
// MAX_REGEX_LITERAL_LEN either.
|
||||
let end = i + 1;
|
||||
while (end < limit && line[end] >= 'a' && line[end] <= 'z') end++;
|
||||
return { text: line.slice(start, end), end };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The regex literal on `line` that mentions PLAN or SUMMARY together with an
|
||||
* escaped `.md` suffix — e.g. `/-PLAN\.md$/`, `/^PLAN-\d+.*\.md$/i`,
|
||||
@@ -265,113 +198,6 @@ function findRegexLiteralMdMatch(line) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Symlinks report `isDirectory()`/`isFile()` as false on the Dirent from
|
||||
// `readdirSync`, so a symlinked `src/*.cts` (or a symlinked directory
|
||||
// containing one) was previously invisible to this scanner — an evasion of a
|
||||
// guard whose stated design principle (ADR-3180 Decision 4a) is whole-repo
|
||||
// discovery with no allowlist. Resolve each entry with `fs.statSync` (which
|
||||
// follows symlinks) to classify it, skipping broken links. `ctx.visitedRealDirs`
|
||||
// guards against a symlink cycle sending `walk` into infinite recursion.
|
||||
//
|
||||
// Every sibling drift guard in `scripts/` (`lint-phase-id-drift.cjs`,
|
||||
// `lint-package-identity-drift.cjs`, `lint-portable-timeout.cjs`,
|
||||
// `lint-test-file-count.cjs`, `lint-allow-test-rule-refs.cjs`) uses the
|
||||
// `Dirent` classification straight off `readdirSync` and does NOT follow
|
||||
// symlinks at all. This guard follows them so a symlinked `src/*.cts` cannot
|
||||
// evade ADR-3180 Decision 4a's whole-repo discovery — root confinement
|
||||
// (`isInsideRoot` below) is the price of doing so: without it, a symlink
|
||||
// planted anywhere under `src/` could walk this scanner out to read and
|
||||
// report arbitrary files elsewhere on disk.
|
||||
//
|
||||
// DIRECTORY vs FILE symlinks are confined to two DIFFERENT roots, tracked as
|
||||
// `ctx.scanDirRoot` (the realpath of the current top-level SCAN_DIRS entry,
|
||||
// e.g. `<realRoot>/src`) vs `ctx.realRoot` (the whole repo):
|
||||
// - a DIRECTORY symlink is descended ONLY if its resolved realpath is
|
||||
// inside `ctx.scanDirRoot` — NOT merely inside `ctx.realRoot`. Without
|
||||
// this, `src/up -> ..` (or `-> <realRoot>`) resolves inside the repo
|
||||
// root and `walk` descends the ENTIRE repo, reporting violations under
|
||||
// paths like `tests/not-src.cts` or `docs/other.cts` — files the header
|
||||
// above says the scan is scoped OUT of (`SCAN_DIRS`). This is a
|
||||
// deliberate, fail-CLOSED trade-off: a directory symlink pointing
|
||||
// elsewhere INSIDE the repo (but outside the scan directory) is simply
|
||||
// not followed. The alternative — descending it — is exactly the
|
||||
// whole-repo sweep this rule exists to prevent, and a fork PR could use
|
||||
// that sweep to redden `lint:ci` on files this guard was never meant to
|
||||
// read. The narrower rule is worth more than the missed edge case.
|
||||
// - a FILE symlink is still scanned if its resolved realpath is inside
|
||||
// `ctx.realRoot` (the whole repo, not just the scan directory) — this is
|
||||
// what keeps `src/alias.cts -> vendor/real.cts` covered (test (f)): an
|
||||
// aliased file genuinely is part of the compiled surface even when its
|
||||
// real target lives outside `src/`, and it is still reported under its
|
||||
// canonical (real) path.
|
||||
//
|
||||
// A resolved path is inside a root only if it IS that root or begins with
|
||||
// root + separator — a plain `startsWith(root)` would also accept a sibling
|
||||
// directory whose name merely starts with the root's name (`/repo-evil`).
|
||||
function isInsideRoot(realPath, realRoot) {
|
||||
return realPath === realRoot || realPath.startsWith(realRoot + path.sep);
|
||||
}
|
||||
|
||||
// True when `realPath` (already confirmed inside `realRoot` by `isInsideRoot`)
|
||||
// resolves THROUGH a skip-list directory anywhere along its path relative to
|
||||
// the root — not just when `realPath` itself IS one. This is what closes the
|
||||
// symlink bypass the `entry.name` fast path alone cannot: `walk` tests
|
||||
// `entry.name` (the symlink's OWN name in its parent directory), but a
|
||||
// symlink named something innocuous can still RESOLVE into `.git` /
|
||||
// `node_modules` / `dist` (`src/g -> ../.git`, `src/leak.cts ->
|
||||
// ../.git/config`, `src/nm -> ../node_modules`) — `isInsideRoot` alone admits
|
||||
// all three, because every one of those real paths is still under the root.
|
||||
function isUnderSkippedDir(realPath, realRoot) {
|
||||
const rel = path.relative(realRoot, realPath);
|
||||
return rel.split(path.sep).some((segment) => SKIP_DIR_NAMES.has(segment));
|
||||
}
|
||||
|
||||
function walk(dir, acc, ctx) {
|
||||
let entries;
|
||||
try {
|
||||
entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return acc;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (SKIP_DIR_NAMES.has(entry.name)) continue; // cheap fast path
|
||||
let stat;
|
||||
try {
|
||||
stat = entry.isSymbolicLink() ? fs.statSync(full) : entry;
|
||||
} catch {
|
||||
continue; // broken symlink target
|
||||
}
|
||||
let realPath;
|
||||
try {
|
||||
realPath = fs.realpathSync(full);
|
||||
} catch {
|
||||
continue; // broken symlink target (race, or a link stat() followed but realpath cannot)
|
||||
}
|
||||
if (stat.isDirectory()) {
|
||||
// Directories (symlinked or real) are confined to the CURRENT scan
|
||||
// directory root, not merely the repo root — see the comment above
|
||||
// `isInsideRoot` for why (`src/up -> ..` whole-repo sweep).
|
||||
if (!isInsideRoot(realPath, ctx.scanDirRoot)) continue;
|
||||
if (isUnderSkippedDir(realPath, ctx.realRoot)) continue; // symlink resolves through a skipped dir
|
||||
if (ctx.visitedRealDirs.has(realPath)) continue; // symlink cycle guard
|
||||
ctx.visitedRealDirs.add(realPath);
|
||||
walk(full, acc, ctx);
|
||||
} else if (stat.isFile() && SCAN_EXT.has(path.extname(entry.name))) {
|
||||
// Files are confined to the whole repo root — a symlinked FILE whose
|
||||
// real target lives outside the scan directory but inside the repo
|
||||
// (e.g. `src/alias.cts -> vendor/real.cts`) is still part of the
|
||||
// compiled surface and must be scanned.
|
||||
if (!isInsideRoot(realPath, ctx.realRoot)) continue;
|
||||
if (isUnderSkippedDir(realPath, ctx.realRoot)) continue; // symlink resolves through a skipped dir
|
||||
if (ctx.visitedRealFiles.has(realPath)) continue; // two symlinks, same real file
|
||||
ctx.visitedRealFiles.add(realPath);
|
||||
acc.push(realPath);
|
||||
}
|
||||
}
|
||||
return acc;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: find every unsanctioned plan/summary-filter re-derivation in `text`.
|
||||
* `relPath` is the repo-relative path, used both to report file:line and to
|
||||
@@ -405,62 +231,19 @@ function findPlanCountDrift(text, relPath) {
|
||||
* each annotated with the repo-relative file path.
|
||||
*/
|
||||
function scanRepo(root) {
|
||||
const violations = [];
|
||||
let realRoot;
|
||||
try {
|
||||
realRoot = fs.realpathSync(root);
|
||||
} catch {
|
||||
return violations; // root itself does not exist / is unreadable
|
||||
}
|
||||
for (const dir of SCAN_DIRS) {
|
||||
const scanDirPath = path.join(root, dir);
|
||||
let scanDirRoot;
|
||||
try {
|
||||
scanDirRoot = fs.realpathSync(scanDirPath);
|
||||
} catch {
|
||||
continue; // scan directory itself does not exist / is unreadable
|
||||
}
|
||||
const ctx = { realRoot, scanDirRoot, visitedRealDirs: new Set(), visitedRealFiles: new Set() };
|
||||
for (const file of walk(scanDirPath, [], ctx)) {
|
||||
// `file` is already the REAL path (walk pushes realPath, not the
|
||||
// symlink path), so `rel` is the file's single canonical location
|
||||
// regardless of which symlink reached it — this is what makes
|
||||
// FUNCTION_SCOPED_EXEMPTIONS/OWNER_FILE, which are keyed on the
|
||||
// repo-relative path, match consistently.
|
||||
const rel = path.relative(realRoot, file);
|
||||
if (rel === OWNER_FILE) continue;
|
||||
let text;
|
||||
try {
|
||||
text = fs.readFileSync(file, 'utf8');
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
for (const d of findPlanCountDrift(text, rel)) {
|
||||
violations.push({ file: rel, ...d });
|
||||
}
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
// Both a `found` fragment AND a reported file path are attacker-controlled
|
||||
// source text on a fork PR (a repo can legally track a filename containing
|
||||
// control bytes, so the path is exactly as attacker-controlled as the
|
||||
// fragment — see the call sites in main() below), and both are written
|
||||
// straight to a CI log. Replace C0/C1 control bytes (ANSI escapes included)
|
||||
// with a visible \xNN, AND the non-Latin-1 formatting/bidi/line-separator
|
||||
// codepoints below with \uNNNN, so a crafted literal or filename cannot
|
||||
// rewrite the terminal rendering of the report or hide/reorder its own text:
|
||||
// - U+200B-U+200F: zero-width space/joiners and directional marks
|
||||
// - U+2028/U+2029: Unicode LINE SEPARATOR / PARAGRAPH SEPARATOR (line
|
||||
// breaks a `\n`-only log scan would not catch)
|
||||
// - U+202A-U+202E: bidi embedding/override controls (RLO etc.)
|
||||
// - U+2066-U+2069: bidi isolate controls
|
||||
function sanitizeForReport(text) {
|
||||
return text
|
||||
// eslint-disable-next-line no-control-regex -- the control range IS the target
|
||||
.replace(/[\x00-\x1f\x7f-\x9f]/g, (c) => '\\x' + c.charCodeAt(0).toString(16).padStart(2, '0'))
|
||||
.replace(/[\u200B-\u200F\u2028\u2029\u202A-\u202E\u2066-\u2069]/g, (c) => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0'));
|
||||
return scanTree({
|
||||
root,
|
||||
scanDirs: SCAN_DIRS,
|
||||
scanExt: SCAN_EXT,
|
||||
onFile(rel, text) {
|
||||
// `rel` is already the REAL (canonical) path (scanTree resolves
|
||||
// symlinks before calling onFile), so this comparison — and
|
||||
// FUNCTION_SCOPED_EXEMPTIONS above, also keyed on `rel` — match
|
||||
// consistently regardless of which symlink reached the file.
|
||||
if (rel === OWNER_FILE) return [];
|
||||
return findPlanCountDrift(text, rel).map((d) => ({ file: rel, ...d }));
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function main() {
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
"milestone-helper.test.cjs",
|
||||
"milestone-prefixed-convention.test.cjs",
|
||||
"milestone-summary.test.cjs",
|
||||
"milestone-window-single-owner.test.cjs",
|
||||
"milestone.test.cjs"
|
||||
],
|
||||
"issue": "TBD"
|
||||
|
||||
@@ -29,7 +29,15 @@ import phaseIdMod = require('./phase-id.cjs');
|
||||
const { escapeRegex, normalizePhaseName, phaseTokenMatches, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import roadmapParserMod = require('./roadmap-parser.cjs');
|
||||
const { getMilestonePhaseFilter, extractCurrentMilestone, getMilestoneInfo } = roadmapParserMod;
|
||||
const {
|
||||
getMilestonePhaseFilter,
|
||||
extractCurrentMilestone,
|
||||
getMilestoneInfo,
|
||||
sliceMilestoneWindow,
|
||||
} = roadmapParserMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import planningScopeMod = require('./planning-scope.cjs');
|
||||
const { SCOPE } = planningScopeMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import coreUtilsMod = require('./core-utils.cjs');
|
||||
const { extractOneLinerFromBody, countMatchedSummaries } = coreUtilsMod;
|
||||
@@ -520,18 +528,40 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
|
||||
const today = realClock.localToday();
|
||||
const milestoneName = options.name || version;
|
||||
|
||||
// Ensure archive directory exists (skipped in dry-run — no mutations)
|
||||
if (!options.dryRun) {
|
||||
platformEnsureDir(archiveDir);
|
||||
}
|
||||
|
||||
// Scope stats and accomplishments to only the phases belonging to the
|
||||
// current milestone's ROADMAP. Uses the shared filter from roadmap-parser.cjs
|
||||
// (same logic used by cmdPhasesList and other callers).
|
||||
// #3184 review finding: this scope computation + refusal MUST run BEFORE
|
||||
// `platformEnsureDir(archiveDir)` below — a refused run (scope not COMPLETE,
|
||||
// no --force) must be a true no-op on disk, and creating the archive
|
||||
// directory first left an empty directory behind even on refusal.
|
||||
const isDirInMilestone = getMilestonePhaseFilter(cwd, version);
|
||||
if (isDirInMilestone.missingExplicitVersion) {
|
||||
error(`no phases found for milestone ${version} in ROADMAP.md`);
|
||||
}
|
||||
// #3184/#3166: `milestone complete` is the ONE-WAY-DOOR consumer of the
|
||||
// milestone window (ROADMAP/REQUIREMENTS archived, phase directories
|
||||
// MOVED). #3166 is specifically the TRUNCATED case: the milestone's
|
||||
// heading IS found but its section closes before the phase region, and the
|
||||
// phase filter degrades to pass-all (see getMilestonePhaseFilter above) —
|
||||
// silently archiving every phase directory on disk. UNREADABLE (no
|
||||
// ROADMAP.md at all) and UNSCOPED (no section for this version) are
|
||||
// pre-existing, legitimately-handled states — `missingExplicitVersion`
|
||||
// above already errors where that matters, and a missing ROADMAP.md has
|
||||
// its own documented graceful path — so only TRUNCATED is refused here.
|
||||
// The read-path consumers keep the pass-all degrade for every scope
|
||||
// (ADR-3180 Decision 3's Rejected section: deny-all there would trade one
|
||||
// silent wrong answer for another); this write path refuses on TRUNCATED
|
||||
// alone, positioned before `platformEnsureDir` so a refusal stays a no-op
|
||||
// on disk.
|
||||
if (isDirInMilestone.scope === SCOPE.TRUNCATED && !options.force) {
|
||||
error(
|
||||
`Cannot mark milestone complete: the ROADMAP window for "${version}" is truncated ` +
|
||||
`(the milestone heading was found but its section ends before reaching any phase ` +
|
||||
`entries, even though the ROADMAP has phase entries elsewhere), so phase scoping ` +
|
||||
`cannot be trusted for this destructive operation. Re-run with --force to override.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Guard: prevent marking complete when ROADMAP still lists phases that have
|
||||
// no directory on disk (disk_status: no_directory). This catches the case
|
||||
@@ -587,7 +617,21 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
|
||||
}
|
||||
|
||||
const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const scopedContent = extractCurrentMilestone(roadmapContent, cwd);
|
||||
// #3184/#2946: scope the unstarted-phase guard to the same `version`
|
||||
// window `getMilestonePhaseFilter` used above, NOT to
|
||||
// extractCurrentMilestone's own STATE.md-derived window — those two
|
||||
// can disagree (that disagreement is exactly what the WARNING above
|
||||
// detects), and scoping this guard to the wrong window under-detects
|
||||
// unstarted phases on the destructive completion path. Calls the same
|
||||
// sliceMilestoneWindow owner getMilestonePhaseFilter's versionOverride
|
||||
// branch calls (a prior pass here re-composed locate+select+section-end
|
||||
// locally, which review caught as a second, disagreeing derivation of
|
||||
// the same window — ADR-3180 Decision 4(c)); falls back to
|
||||
// extractCurrentMilestone's whole-document result only for the
|
||||
// free-form (no versioned milestones anywhere) shape, where both
|
||||
// windows converge to the same value regardless of which version drove
|
||||
// the lookup.
|
||||
const scopedContent = sliceMilestoneWindow(roadmapContent, version) ?? extractCurrentMilestone(roadmapContent, cwd);
|
||||
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
|
||||
const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi');
|
||||
const noDirectoryPhases: string[] = [];
|
||||
@@ -737,6 +781,14 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
|
||||
return;
|
||||
}
|
||||
|
||||
// Ensure archive directory exists. Deliberately placed AFTER the dry-run
|
||||
// early return and every refusal/guard above (missingExplicitVersion, the
|
||||
// scope refusal, the unstarted-phase guard) — #3184 review finding: this
|
||||
// used to run before those checks, so a refused run still left an empty
|
||||
// archive directory behind. Reaching this point means the run is
|
||||
// committed to mutating.
|
||||
platformEnsureDir(archiveDir);
|
||||
|
||||
// Archive ROADMAP.md
|
||||
if (fs.existsSync(roadmapPath)) {
|
||||
const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
|
||||
@@ -36,8 +36,12 @@ import { platformReadSync } from './shell-command-projection.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import unusableInputMod = require('./unusable-input.cjs');
|
||||
const { UNUSABLE_REASON, warnUnusableInput } = unusableInputMod;
|
||||
import { tokenizeHeadings, stripTaggedBlocks, withSection } from './markdown-sectionizer.cjs';
|
||||
import { tokenizeHeadings, stripTaggedBlocks, withSection, stripFencedCode } from './markdown-sectionizer.cjs';
|
||||
import type { HeadingToken } from './markdown-sectionizer.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import planningScopeMod = require('./planning-scope.cjs');
|
||||
const { SCOPE } = planningScopeMod;
|
||||
type Scope = planningScopeMod.Scope;
|
||||
|
||||
// ─── Roadmap milestone scoping ───────────────────────────────────────────────
|
||||
|
||||
@@ -97,7 +101,194 @@ function isMilestoneShippedInRoadmap(content: string, version: string): boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the current milestone section from ROADMAP.md by positive lookup.
|
||||
* #3184 (epic #3180 Phase 2): the sole owner of "where does this milestone
|
||||
* heading's section end". Lifted from `currentMilestoneRawRanges`'s prior
|
||||
* inline copy — the only one of three byte-identical copies that carried a
|
||||
* "keep in sync" comment (evidence the risk was known, not controlled).
|
||||
* `extractCurrentMilestoneScoped`, `currentMilestoneRawRanges`, and
|
||||
* `getMilestonePhaseFilter`'s versionOverride branch all call this instead of
|
||||
* re-deriving it.
|
||||
*/
|
||||
function computeMilestoneSectionEnd(content: string, headingText: string, headingStart: number): number {
|
||||
const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length;
|
||||
const afterHeading = headingStart + headingText.length;
|
||||
// Use tokenizeHeadings (fence-aware, offsets into original content) to find
|
||||
// the next stop boundary without re-implementing fence detection. T4 seam migration.
|
||||
const headings = tokenizeHeadings(content);
|
||||
for (const h of headings) {
|
||||
if (h.offset <= headingStart) continue;
|
||||
if (h.offset < afterHeading) continue;
|
||||
if (h.level > level) continue;
|
||||
// Mirrors old stopPattern: level-bounded, not a Phase heading, milestone marker
|
||||
if (/^Phase\s+\S/i.test(h.text)) continue;
|
||||
if (!/v\d+\.\d+|✅|📋|🚧/i.test(h.text)) continue;
|
||||
return h.offset;
|
||||
}
|
||||
return content.length;
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: the sole milestone-heading locator. Boundary-matched on the version
|
||||
* token with `\b`, NOT the stricter `(?![\w.-])`: this function keeps `\b`
|
||||
* because a milestone STATE legitimately selects its own sub-milestone
|
||||
* heading (`v8.0` matching `## v8.0-B …` — `0` is a word char, `-` is not, so
|
||||
* `\b` matches) — that is deliberate, load-bearing behavior (#730). The
|
||||
* stricter `(?![\w.-])` boundary answers a DIFFERENT question — "is exactly
|
||||
* this milestone shipped" (`isMilestoneShippedInRoadmap`) / "which Phase
|
||||
* Details section belongs to exactly this one's version token"
|
||||
* (`detailsVersionBoundary`) — and applying it here breaks #730 sub-milestone
|
||||
* selection. `extractCurrentMilestoneScoped`, `currentMilestoneRawRanges`,
|
||||
* and `getMilestonePhaseFilter`'s versionOverride branch all consume this
|
||||
* instead of re-deriving their own heading-location regex.
|
||||
*/
|
||||
function locateMilestoneHeadings(content: string, version: string): RegExpExecArray[] {
|
||||
const escapedVersion = escapeRegex(version);
|
||||
const pattern = new RegExp(
|
||||
`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`,
|
||||
'gmi',
|
||||
);
|
||||
const matches: RegExpExecArray[] = [];
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = pattern.exec(content)) !== null) {
|
||||
matches.push(m);
|
||||
}
|
||||
return matches;
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: named predicate replacing the two `state.cts` re-derivations
|
||||
* (`buildStateFrontmatter`, `syncStateFrontmatter`) that each hand-rolled the
|
||||
* same "is this version bounded to a versioned ROADMAP heading" regex. A
|
||||
* straight consolidation of the two identical `state.cts` regexes onto the
|
||||
* shared `locateMilestoneHeadings` owner — no behavior change.
|
||||
*/
|
||||
function isMilestoneBoundedInRoadmap(content: string, version: string): boolean {
|
||||
return locateMilestoneHeadings(content, version).length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: does this ROADMAP carry ANY versioned milestone heading (`v1.2`-style
|
||||
* token on a level 1-3 non-Phase heading), independent of any particular
|
||||
* version. `extractCurrentMilestoneScoped` (free-form-vs-versioned row 3/4
|
||||
* classification) and `getMilestonePhaseFilter` (the deprecation warning +
|
||||
* the same row 3/4 classification for its versionOverride branch) each
|
||||
* hand-rolled this identically — the guard does not catch intra-owner-file
|
||||
* copies by construction, so this was found by review instead.
|
||||
*/
|
||||
function hasVersionedMilestones(content: string): boolean {
|
||||
return /^#{1,3}\s+.*v\d+\.\d+/mi.test(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184/#2828/#1761: does this ROADMAP use milestone SECTIONING at all — i.e.
|
||||
* does it carry any non-Phase heading at level 2-3? Deliberately weaker than
|
||||
* `hasVersionedMilestones`: this needs to distinguish a FLAT unmilestoned
|
||||
* roadmap (Phase headings only, where a whole-document phase count is
|
||||
* correct) from a MILESTONED-but-unbounded one (where that count conflates
|
||||
* sibling milestones, #1761) — that distinction is load-bearing and must not
|
||||
* be collapsed into the versioned-milestone check. Owned here so the
|
||||
* milestone heading vocabulary has one home; routes `state.cts`'s
|
||||
* `buildStateFrontmatter` #2828 guard instead of a third hand-rolled copy.
|
||||
*/
|
||||
function hasMilestoneSectioning(content: string): boolean {
|
||||
return /^#{2,3}\s+(?!Phase\s+\S)/mi.test(content);
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: the sole "which heading is this milestone's" rule — locate the version's
|
||||
* headings, prefer the first that is not marked CLOSED/SHIPPED, else fall back to the
|
||||
* first match. Returns null when the version has no heading at all.
|
||||
*
|
||||
* Extracted because three sites had written this same two-line selection
|
||||
* independently (sliceMilestoneWindow, extractCurrentMilestoneScoped,
|
||||
* currentMilestoneRawRanges) — the composition-level divergence ADR-3180
|
||||
* Decision 4(c) covers: calling the owner's primitives and re-assembling the
|
||||
* result locally is indistinguishable from re-deriving it.
|
||||
*/
|
||||
function selectMilestoneHeading(content: string, version: string): RegExpExecArray | null {
|
||||
const matches = locateMilestoneHeadings(content, version);
|
||||
if (matches.length === 0) return null;
|
||||
return matches.find((m) => !isClosedMilestoneHeading(m[1])) ?? matches[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: the sole "give me this version's window" composition. Delegates
|
||||
* heading selection to `selectMilestoneHeading` (the sole selection owner)
|
||||
* and then to `computeMilestoneSectionEnd` for the slice. Returns null when
|
||||
* the version has no heading at all, so callers can distinguish "no such
|
||||
* milestone section" from "empty section".
|
||||
*
|
||||
* Review finding (post-merge of this phase's first pass): `getMilestonePhaseFilter`'s
|
||||
* versionOverride branch and `cmdMilestoneComplete`'s unstarted-phase guard
|
||||
* had each independently composed `locateMilestoneHeadings` +
|
||||
* `computeMilestoneSectionEnd` into a window — the SAME derivation written
|
||||
* twice, and they disagreed (one skipped CLOSED headings, the other did not)
|
||||
* — exactly the composition-level divergence ADR-3180 Decision 4(c) warns
|
||||
* about: calling the owner and then re-assembling the result locally is
|
||||
* indistinguishable from re-deriving it. Both sites now call this instead.
|
||||
*/
|
||||
function sliceMilestoneWindow(content: string, version: string): string | null {
|
||||
const selected = selectMilestoneHeading(content, version);
|
||||
if (selected === null) return null;
|
||||
return content.slice(selected.index, computeMilestoneSectionEnd(content, selected[0], selected.index));
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: counts RAW phase references — a `#{2,4} Phase <id>:` heading
|
||||
* (fence-aware via `tokenizeHeadings`) or a `#2199` bullet entry — BEFORE any
|
||||
* sentinel filter. Used for BOTH sides of `classifyMilestoneWindow`'s row-8
|
||||
* comparison (does the window contain phase entries; does the document).
|
||||
* Deliberately does NOT filter `999.x`/Phase 0 sentinels: the question here
|
||||
* is "did the window reach the phase region", not "how many real phases
|
||||
* exist" — a window containing only sentinel phases still reached the
|
||||
* region and must read COMPLETE, not TRUNCATED.
|
||||
*/
|
||||
function hasPhaseEntries(markdown: string): boolean {
|
||||
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
|
||||
const phaseHeadingPattern = /^(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/i;
|
||||
for (const h of tokenizeHeadings(markdown)) {
|
||||
if (h.level < 2 || h.level > 4) continue;
|
||||
if (phaseHeadingPattern.test(h.text)) return true;
|
||||
}
|
||||
// #3184 review finding: the bullet fallback must be fence-aware too, or a
|
||||
// FENCED markdown EXAMPLE of the `- [ ] **Phase N — Name**` syntax (e.g. a
|
||||
// doc showing the convention) counts as a real phase entry. Strip fences
|
||||
// through the canonical seam before testing, matching tokenizeHeadings'
|
||||
// fence-awareness above.
|
||||
return BULLET_PHASE_LINE_PATTERN.test(stripFencedCode(markdown).text);
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: pure decision table (no I/O, no regex construction from caller
|
||||
* data) implementing the design's Behavior table rows 1-8 (the remaining
|
||||
* rows 9-17 reduce to one of these six through how the caller constructs its
|
||||
* input, not additional branches here). Kernighan's Law fired during design:
|
||||
* `getMilestonePhaseFilter` is already cyclomatic 36, so this discriminator
|
||||
* is extracted as its own named, separately-testable function rather than
|
||||
* inlined.
|
||||
*/
|
||||
function classifyMilestoneWindow(input: {
|
||||
readable: boolean;
|
||||
versionResolved: boolean;
|
||||
hasVersionedMilestones: boolean;
|
||||
headingFound: boolean;
|
||||
windowHasPhaseEntries: boolean;
|
||||
documentHasPhaseEntries: boolean;
|
||||
}): Scope {
|
||||
const { readable, versionResolved, hasVersionedMilestones, headingFound, windowHasPhaseEntries, documentHasPhaseEntries } = input;
|
||||
return (
|
||||
!readable ? SCOPE.UNREADABLE : // row 2
|
||||
!versionResolved && !hasVersionedMilestones ? SCOPE.COMPLETE : // row 3: free-form legacy roadmap
|
||||
!versionResolved && hasVersionedMilestones ? SCOPE.UNSCOPED : // row 4
|
||||
versionResolved && !headingFound ? SCOPE.UNSCOPED : // row 5
|
||||
headingFound && !windowHasPhaseEntries && documentHasPhaseEntries ? SCOPE.TRUNCATED : // row 8
|
||||
SCOPE.COMPLETE // rows 6, 7
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the current milestone section from ROADMAP.md by positive lookup,
|
||||
* carrying a `scope` discriminator (ADR-3180 Decision 2) alongside the value.
|
||||
*
|
||||
* @param content - ROADMAP.md content.
|
||||
* @param cwd - Project working directory, used to read the companion STATE.md
|
||||
@@ -106,9 +297,18 @@ function isMilestoneShippedInRoadmap(content: string, version: string): boolean
|
||||
* `.planning/workstreams/<ws>/` instead of the project root. Omitted (the
|
||||
* default) preserves the prior `planningDir(cwd)` resolution exactly,
|
||||
* including its `GSD_WORKSTREAM` env fallback.
|
||||
*
|
||||
* #3184: `extractCurrentMilestone`'s CRITICAL blast radius (200+ affected
|
||||
* symbols, 20 direct callers) means its signature and return type do not
|
||||
* change. This is the real owner; `extractCurrentMilestone` becomes a
|
||||
* one-line wrapper returning `.value` so every existing caller is untouched.
|
||||
*/
|
||||
function extractCurrentMilestone(content: string, cwd?: string, ws?: string | null): string {
|
||||
if (!cwd) return stripShippedMilestones(content);
|
||||
function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: string | null): { value: string; scope: Scope } {
|
||||
if (!cwd) {
|
||||
// Row 1: a deliberate unscoped read (no cwd supplied) is a real answer —
|
||||
// the caller asked for no scoping, so whole-document is COMPLETE.
|
||||
return { value: stripShippedMilestones(content), scope: SCOPE.COMPLETE };
|
||||
}
|
||||
|
||||
let version: string | null = null;
|
||||
try {
|
||||
@@ -129,18 +329,33 @@ function extractCurrentMilestone(content: string, cwd?: string, ws?: string | nu
|
||||
}
|
||||
}
|
||||
|
||||
if (!version) return stripShippedMilestones(content);
|
||||
const versionResolved = version !== null;
|
||||
// #3184: routed through the shared owner (was an inline copy — see the
|
||||
// twin copy in `getMilestonePhaseFilter`, the intra-owner-file duplicate
|
||||
// review caught since the drift guard exempts this file by construction).
|
||||
const versionedMilestonesPresent = hasVersionedMilestones(content);
|
||||
|
||||
const escapedVersion = escapeRegex(version);
|
||||
const sectionPattern = new RegExp(
|
||||
`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`,
|
||||
'gmi'
|
||||
);
|
||||
if (!version) {
|
||||
const value = stripShippedMilestones(content);
|
||||
return {
|
||||
value,
|
||||
scope: classifyMilestoneWindow({
|
||||
readable: true,
|
||||
versionResolved,
|
||||
hasVersionedMilestones: versionedMilestonesPresent,
|
||||
headingFound: false,
|
||||
windowHasPhaseEntries: hasPhaseEntries(value),
|
||||
documentHasPhaseEntries: hasPhaseEntries(value),
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(content));
|
||||
const summaryPattern = new RegExp(
|
||||
`<summary[^>]*>([^<]*${escapedVersion}[^<]*)<\\/summary>`,
|
||||
`<summary[^>]*>([^<]*${escapeRegex(version)}[^<]*)<\\/summary>`,
|
||||
'i'
|
||||
);
|
||||
const headingMatches = [...content.matchAll(sectionPattern)];
|
||||
const headingMatches = locateMilestoneHeadings(content, version);
|
||||
|
||||
if (headingMatches.length === 0) {
|
||||
const summaryMatch = content.match(summaryPattern);
|
||||
@@ -161,39 +376,46 @@ function extractCurrentMilestone(content: string, cwd?: string, ws?: string | nu
|
||||
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
|
||||
.replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '')
|
||||
.replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, '');
|
||||
return preamble + content.slice(detailsOpenIdx, detailsEnd);
|
||||
const value = preamble + content.slice(detailsOpenIdx, detailsEnd);
|
||||
return {
|
||||
value,
|
||||
scope: classifyMilestoneWindow({
|
||||
readable: true,
|
||||
versionResolved,
|
||||
hasVersionedMilestones: versionedMilestonesPresent,
|
||||
headingFound: true,
|
||||
windowHasPhaseEntries: hasPhaseEntries(value),
|
||||
documentHasPhaseEntries,
|
||||
}),
|
||||
};
|
||||
}
|
||||
}
|
||||
return stripShippedMilestones(content);
|
||||
const value = stripShippedMilestones(content);
|
||||
return {
|
||||
value,
|
||||
scope: classifyMilestoneWindow({
|
||||
readable: true,
|
||||
versionResolved,
|
||||
hasVersionedMilestones: versionedMilestonesPresent,
|
||||
headingFound: false,
|
||||
windowHasPhaseEntries: hasPhaseEntries(value),
|
||||
documentHasPhaseEntries,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
const allMatches = headingMatches;
|
||||
|
||||
const isClosed = isClosedMilestoneHeading;
|
||||
const firstMatch = allMatches[0];
|
||||
const selected = allMatches.find((m) => !isClosed(m[1])) || firstMatch;
|
||||
// #3184: selection collapses to the sole owner; `allMatches` is still needed
|
||||
// below (offsets, detailsMatch search), so only the selection itself routes
|
||||
// through `selectMilestoneHeading` rather than the whole block.
|
||||
const selected = selectMilestoneHeading(content, version)!;
|
||||
|
||||
const sectionStart = selected.index;
|
||||
|
||||
const computeSectionEnd = (headingText: string, headingStart: number): number => {
|
||||
const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length;
|
||||
const afterHeading = headingStart + headingText.length;
|
||||
// Use tokenizeHeadings (fence-aware, offsets into original content) to find
|
||||
// the next stop boundary without re-implementing fence detection. T4 seam migration.
|
||||
const headings = tokenizeHeadings(content);
|
||||
for (const h of headings) {
|
||||
if (h.offset <= headingStart) continue;
|
||||
if (h.offset < afterHeading) continue;
|
||||
if (h.level > level) continue;
|
||||
// Mirrors old stopPattern: level-bounded, not a Phase heading, milestone marker
|
||||
if (/^Phase\s+\S/i.test(h.text)) continue;
|
||||
if (!/v\d+\.\d+|✅|📋|🚧/i.test(h.text)) continue;
|
||||
return h.offset;
|
||||
}
|
||||
return content.length;
|
||||
};
|
||||
|
||||
const sectionEnd = computeSectionEnd(selected[0], sectionStart);
|
||||
const sectionEnd = computeMilestoneSectionEnd(content, selected[0], sectionStart);
|
||||
|
||||
const anyMilestonePattern = /^#{1,3}\s+(?!Phase\s+\S)(?:.*v\d+\.\d+|✅|📋|🚧)/im;
|
||||
const firstMilestoneMatch = content.match(anyMilestonePattern);
|
||||
@@ -231,7 +453,7 @@ function extractCurrentMilestone(content: string, cwd?: string, ws?: string | nu
|
||||
const detailsStart = detailsMatch.index ?? 0;
|
||||
detailsSection = content.slice(
|
||||
detailsStart,
|
||||
computeSectionEnd(detailsMatch[0], detailsStart),
|
||||
computeMilestoneSectionEnd(content, detailsMatch[0], detailsStart),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -250,9 +472,31 @@ function extractCurrentMilestone(content: string, cwd?: string, ws?: string | nu
|
||||
.replace(currentSectionHasPhaseDetails ? /^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim : /$/, '')
|
||||
.replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, '');
|
||||
|
||||
return detailsSection
|
||||
const value = detailsSection
|
||||
? preamble + currentSection + '\n' + detailsSection
|
||||
: preamble + currentSection;
|
||||
|
||||
return {
|
||||
value,
|
||||
scope: classifyMilestoneWindow({
|
||||
readable: true,
|
||||
versionResolved,
|
||||
hasVersionedMilestones: versionedMilestonesPresent,
|
||||
headingFound: true,
|
||||
windowHasPhaseEntries: hasPhaseEntries(value),
|
||||
documentHasPhaseEntries,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* #3184: thin wrapper preserving `extractCurrentMilestone`'s exact signature
|
||||
* and return type — CRITICAL blast radius (20 direct callers), so the type
|
||||
* stays `string`. `extractCurrentMilestoneScoped` is the real owner; callers
|
||||
* that need to branch on scope opt in to it directly.
|
||||
*/
|
||||
function extractCurrentMilestone(content: string, cwd?: string, ws?: string | null): string {
|
||||
return extractCurrentMilestoneScoped(content, cwd, ws).value;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -603,6 +847,13 @@ type MilestonePhaseFilter = ((dirName: string) => boolean) & {
|
||||
* the current milestone's.
|
||||
*/
|
||||
versionSectionFound: boolean;
|
||||
/**
|
||||
* #3184 (ADR-3180 Decision 2): the same window-classification carried by
|
||||
* `extractCurrentMilestoneScoped`. The filter's FUNCTION behavior is
|
||||
* UNCHANGED by this field — pass-all still passes all; a destructive
|
||||
* consumer (`cmdMilestoneComplete`) reads `scope` to refuse instead.
|
||||
*/
|
||||
scope: Scope;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -629,13 +880,23 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
let missingExplicitVersion = false;
|
||||
let versionScoped = false;
|
||||
let versionSectionFound = false;
|
||||
let scope: Scope = SCOPE.UNREADABLE;
|
||||
try {
|
||||
const roadmapPath = path.join(planningDir(cwd, ws), 'ROADMAP.md');
|
||||
const roadmapContent = platformReadSync(roadmapPath);
|
||||
if (roadmapContent === null) throw new Error('missing');
|
||||
let roadmap = extractCurrentMilestone(roadmapContent, cwd, ws);
|
||||
const scopedResult = extractCurrentMilestoneScoped(roadmapContent, cwd, ws);
|
||||
let roadmap = scopedResult.value;
|
||||
// Default: the filter's window IS extractCurrentMilestoneScoped's own
|
||||
// window (reused verbatim, not re-derived — ADR-3180 Decision 4c).
|
||||
// Overwritten below when `versionOverride` scopes to a DIFFERENT window.
|
||||
scope = scopedResult.scope;
|
||||
|
||||
const hasVersionedMilestonesGlobal = /^#{1,3}\s+.*v\d+\.\d+/mi.test(roadmapContent);
|
||||
// #3184: routed through the shared owner (was an inline copy — see the
|
||||
// twin copy in `extractCurrentMilestoneScoped`, the intra-owner-file
|
||||
// duplicate review caught since the drift guard exempts this file by
|
||||
// construction).
|
||||
const hasVersionedMilestonesGlobal = hasVersionedMilestones(roadmapContent);
|
||||
const hasPhaseHeadings = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+[\w]/i.test(roadmapContent);
|
||||
if (!hasVersionedMilestonesGlobal && hasPhaseHeadings && phaseIdConvention === 'milestone-prefixed') {
|
||||
console.warn(
|
||||
@@ -646,51 +907,48 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
}
|
||||
|
||||
if (versionOverride) {
|
||||
const escapedVersion = escapeRegex(versionOverride);
|
||||
const sectionPattern = new RegExp(`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}[^\\n]*)`, 'mi');
|
||||
let sectionMatch = roadmapContent.match(sectionPattern);
|
||||
// #3184: route the whole "locate headings -> pick the active one ->
|
||||
// section-end" composition through the single owner (sliceMilestoneWindow)
|
||||
// instead of assembling it here. This branch used to be a bare `.match()`
|
||||
// — first hit, no closed-heading skip, no version-token boundary — and a
|
||||
// review pass caught it independently re-composing the SAME primitives
|
||||
// `cmdMilestoneComplete`'s guard composed, disagreeing on closed-heading
|
||||
// skipping. Now both sites call one function. Boundary-matched
|
||||
// (`(?![\w.-])`) and closed-heading-skipping is a declared Tier-2 change
|
||||
// affecting every caller that passes `versionOverride`: `roadmap.analyze`
|
||||
// / `milestone complete` (this module, `cmdMilestoneComplete` in
|
||||
// milestone.cts), `inspectWorkstream` (workstream-inventory.cts:518,
|
||||
// via `currentVersion`), and `buildStateFrontmatter` (state.cts:1700,
|
||||
// via `storedMilestone`).
|
||||
const sliced = sliceMilestoneWindow(roadmapContent, versionOverride);
|
||||
|
||||
if (!sectionMatch) {
|
||||
const summaryPat = new RegExp(`<summary[^>]*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i');
|
||||
const summaryHit = roadmapContent.match(summaryPat);
|
||||
if (summaryHit) {
|
||||
const beforeSummary = roadmapContent.slice(0, summaryHit.index);
|
||||
const detailsIdx = beforeSummary.lastIndexOf('<details');
|
||||
if (detailsIdx !== -1) {
|
||||
sectionMatch = null;
|
||||
}
|
||||
}
|
||||
}
|
||||
const documentHasPhaseEntries = hasPhaseEntries(stripShippedMilestones(roadmapContent));
|
||||
|
||||
if (!sectionMatch) {
|
||||
const hasVersionedMilestones = /^#{1,3}\s+(?!Phase\s+\S).*v\d+\.\d+/mi.test(roadmapContent);
|
||||
if (sliced !== null) {
|
||||
versionScoped = true;
|
||||
versionSectionFound = true;
|
||||
roadmap = sliced;
|
||||
} else {
|
||||
const escapedVersion = escapeRegex(versionOverride);
|
||||
const versionInSummary = new RegExp(`<summary[^>]*>[^<]*${escapedVersion}[^<]*<\\/summary>`, 'i').test(roadmapContent);
|
||||
if (hasVersionedMilestones && !versionInSummary) {
|
||||
if (hasVersionedMilestonesGlobal && !versionInSummary) {
|
||||
roadmap = '';
|
||||
missingExplicitVersion = true;
|
||||
}
|
||||
} else {
|
||||
versionScoped = true;
|
||||
versionSectionFound = true;
|
||||
const sectionStart = sectionMatch.index!;
|
||||
const headingLevel = (sectionMatch[1].match(/^(#{1,3})\s/) ?? ['', '#'])[1].length;
|
||||
const afterHeading = sectionStart + sectionMatch[0].length;
|
||||
// Use tokenizeHeadings (fence-aware, offsets into original content) to find
|
||||
// the next milestone-boundary heading. T4 seam migration.
|
||||
const allHeadings = tokenizeHeadings(roadmapContent);
|
||||
let sectionEnd = roadmapContent.length;
|
||||
for (const h of allHeadings) {
|
||||
if (h.offset < afterHeading) continue;
|
||||
if (h.level > headingLevel) continue;
|
||||
if (/^Phase\s+\S/i.test(h.text)) continue;
|
||||
if (!/v\d+\.\d+|✅|📋|🚧/i.test(h.text)) continue;
|
||||
sectionEnd = h.offset;
|
||||
break;
|
||||
}
|
||||
|
||||
const currentSection = roadmapContent.slice(sectionStart, sectionEnd);
|
||||
roadmap = currentSection;
|
||||
// else: version appears only inside a `<summary>`, or there are no
|
||||
// versioned milestones anywhere — `roadmap` keeps
|
||||
// extractCurrentMilestoneScoped's own (STATE-scoped) result, matching
|
||||
// the pre-existing summary-block / free-form fallback shape.
|
||||
}
|
||||
|
||||
scope = classifyMilestoneWindow({
|
||||
readable: true,
|
||||
versionResolved: true,
|
||||
hasVersionedMilestones: hasVersionedMilestonesGlobal,
|
||||
headingFound: sliced !== null,
|
||||
windowHasPhaseEntries: hasPhaseEntries(roadmap),
|
||||
documentHasPhaseEntries,
|
||||
});
|
||||
}
|
||||
|
||||
// Use tokenizeHeadings (fence-aware) instead of stripFencedLines + regex.
|
||||
@@ -706,10 +964,15 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
// #2199: also count bullet/checkbox phase entries (`- [ ] **Phase N — name**`)
|
||||
// so a bullet-house-style ROADMAP populates the milestone phase set instead of
|
||||
// collapsing to a zero-count pass-all filter.
|
||||
// #3184 review finding: this scan must be fence-aware like `hasPhaseEntries`
|
||||
// above — otherwise a fenced markdown EXAMPLE of the bullet syntax inflates
|
||||
// milestonePhaseNums / phaseCount. Strip fences through the canonical seam
|
||||
// first.
|
||||
{
|
||||
let bm: RegExpExecArray | null;
|
||||
const scanner = new RegExp(BULLET_PHASE_LINE_PATTERN.source, 'gim');
|
||||
while ((bm = scanner.exec(roadmap)) !== null) {
|
||||
const roadmapUnfenced = stripFencedCode(roadmap).text;
|
||||
while ((bm = scanner.exec(roadmapUnfenced)) !== null) {
|
||||
if (!/^999\b/.test(bm[1])) milestonePhaseNums.add(bm[1]);
|
||||
}
|
||||
}
|
||||
@@ -719,7 +982,9 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
* any failure milestonePhaseNums stays empty, which below already
|
||||
* degrades to the same pass-all filter this function returns when a
|
||||
* ROADMAP genuinely has zero recognizable phase headings — a safe,
|
||||
* non-corrupting (over-inclusive, never under-inclusive) degrade. */
|
||||
* non-corrupting (over-inclusive, never under-inclusive) degrade.
|
||||
* #3184: `scope` was set to SCOPE.UNREADABLE before the try (row 2) and
|
||||
* is left as-is here — the read/parse fault IS the unreadable case. */
|
||||
}
|
||||
|
||||
if (milestonePhaseNums.size === 0) {
|
||||
@@ -731,6 +996,10 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
// `versionScoped` is reset here — this is the only surviving evidence that
|
||||
// the current milestone exists in the ROADMAP and simply has no phases yet.
|
||||
passAll.versionSectionFound = versionSectionFound;
|
||||
// #3184: the filter's FUNCTION behavior is unchanged — pass-all still
|
||||
// passes all. `scope` is the decidable signal a destructive consumer
|
||||
// reads to refuse instead (ADR-3180 Decision 3's two-tier policy).
|
||||
passAll.scope = scope;
|
||||
return passAll;
|
||||
}
|
||||
|
||||
@@ -776,6 +1045,7 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
(isDirInMilestone as MilestonePhaseFilter).missingExplicitVersion = missingExplicitVersion;
|
||||
(isDirInMilestone as MilestonePhaseFilter).versionScoped = versionScoped;
|
||||
(isDirInMilestone as MilestonePhaseFilter).versionSectionFound = versionSectionFound;
|
||||
(isDirInMilestone as MilestonePhaseFilter).scope = scope;
|
||||
return isDirInMilestone as MilestonePhaseFilter;
|
||||
}
|
||||
|
||||
@@ -785,12 +1055,12 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p
|
||||
* writer) so they cannot touch a backticked prose literal, a Backlog entry, or a
|
||||
* same-numbered phase in a shipped milestone.
|
||||
*
|
||||
* Mirrors the region selection in `extractCurrentMilestone` (version detection →
|
||||
* active heading → next milestone boundary → optional Phase Details section).
|
||||
* Returns null when there is no versioned active milestone; callers then fall
|
||||
* back to whole-content mutation (the prior behaviour).
|
||||
*
|
||||
* NOTE: keep the region logic here in sync with extractCurrentMilestone.
|
||||
* Mirrors the region selection in `extractCurrentMilestoneScoped` (version
|
||||
* detection → active heading → next milestone boundary → optional Phase
|
||||
* Details section) — both consume the same `locateMilestoneHeadings` /
|
||||
* `computeMilestoneSectionEnd` owner (#3184), so there is no separate copy to
|
||||
* keep in sync. Returns null when there is no versioned active milestone;
|
||||
* callers then fall back to whole-content mutation (the prior behaviour).
|
||||
*/
|
||||
function currentMilestoneRawRanges(
|
||||
content: string,
|
||||
@@ -813,33 +1083,16 @@ function currentMilestoneRawRanges(
|
||||
}
|
||||
if (!version) return null;
|
||||
|
||||
const escapedVersion = escapeRegex(version);
|
||||
const sectionPattern = new RegExp(
|
||||
`(^#{1,3}\\s+(?!Phase\\s+\\S).*${escapedVersion}\\b[^\\n]*)`,
|
||||
'gmi',
|
||||
);
|
||||
const headingMatches = [...content.matchAll(sectionPattern)];
|
||||
const headingMatches = locateMilestoneHeadings(content, version);
|
||||
if (headingMatches.length === 0) return null;
|
||||
|
||||
const isClosed = isClosedMilestoneHeading;
|
||||
const firstMatch = headingMatches[0];
|
||||
const selected = headingMatches.find((m) => !isClosed(m[1])) || firstMatch;
|
||||
// #3184: selection collapses to the sole owner; `headingMatches` is still
|
||||
// needed below for the detailsMatch search over all headings.
|
||||
const selected = selectMilestoneHeading(content, version)!;
|
||||
const sectionStart = selected.index ?? 0;
|
||||
|
||||
const computeSectionEnd = (headingText: string, headingStart: number): number => {
|
||||
const level = (headingText.match(/^(#{1,3})\s/) ?? ['', '#'])[1].length;
|
||||
const afterHeading = headingStart + headingText.length;
|
||||
for (const h of tokenizeHeadings(content)) {
|
||||
if (h.offset <= headingStart) continue;
|
||||
if (h.offset < afterHeading) continue;
|
||||
if (h.level > level) continue;
|
||||
if (/^Phase\s+\S/i.test(h.text)) continue;
|
||||
if (!/v\d+\.\d+|✅|📋|🚧/i.test(h.text)) continue;
|
||||
return h.offset;
|
||||
}
|
||||
return content.length;
|
||||
};
|
||||
const sectionEnd = computeSectionEnd(selected[0], sectionStart);
|
||||
const sectionEnd = computeMilestoneSectionEnd(content, selected[0], sectionStart);
|
||||
|
||||
const selectedVersionToken = selected[1].match(
|
||||
/v\d+(?:\.\d+)+(?:[-.][A-Za-z0-9]+)*/i,
|
||||
@@ -857,7 +1110,7 @@ function currentMilestoneRawRanges(
|
||||
let details: { start: number; end: number } | null = null;
|
||||
if (detailsMatch) {
|
||||
const detailsStart = detailsMatch.index ?? 0;
|
||||
details = { start: detailsStart, end: computeSectionEnd(detailsMatch[0], detailsStart) };
|
||||
details = { start: detailsStart, end: computeMilestoneSectionEnd(content, detailsMatch[0], detailsStart) };
|
||||
}
|
||||
|
||||
return { primary: { start: sectionStart, end: sectionEnd }, details };
|
||||
@@ -866,11 +1119,23 @@ function currentMilestoneRawRanges(
|
||||
export = {
|
||||
stripShippedMilestones,
|
||||
extractCurrentMilestone,
|
||||
extractCurrentMilestoneScoped,
|
||||
isMilestoneShippedInRoadmap,
|
||||
isMilestoneBoundedInRoadmap,
|
||||
replaceInCurrentMilestone,
|
||||
getRoadmapPhaseInternal,
|
||||
getMilestoneInfo,
|
||||
getMilestonePhaseFilter,
|
||||
currentMilestoneRawRanges,
|
||||
withPhaseSection,
|
||||
computeMilestoneSectionEnd,
|
||||
locateMilestoneHeadings,
|
||||
selectMilestoneHeading,
|
||||
classifyMilestoneWindow,
|
||||
// #3184: the sole "give me this version's window" composition — see its
|
||||
// own doc comment. milestone.cts's destructive-consumer guard consumes
|
||||
// this instead of composing locate+select+section-end itself.
|
||||
sliceMilestoneWindow,
|
||||
hasVersionedMilestones,
|
||||
hasMilestoneSectioning,
|
||||
};
|
||||
|
||||
@@ -20,7 +20,7 @@ import phaseLocatorMod = require('./phase-locator.cjs');
|
||||
const { findPhaseInternal } = phaseLocatorMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import roadmapParserModule = require('./roadmap-parser.cjs');
|
||||
const { stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = roadmapParserModule;
|
||||
const { stripShippedMilestones, extractCurrentMilestone, extractCurrentMilestoneScoped, replaceInCurrentMilestone } = roadmapParserModule;
|
||||
import { tokenizeHeadings } from './markdown-sectionizer.cjs';
|
||||
import { updateTableCell } from './markdown-table.cjs';
|
||||
import { platformWriteSync } from './shell-command-projection.cjs';
|
||||
@@ -309,7 +309,10 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void {
|
||||
}
|
||||
|
||||
const rawContent = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
const content = extractCurrentMilestone(rawContent, cwd);
|
||||
// #3184/#3165: use the scoped variant so a truncated window is a
|
||||
// distinguishable signal in the output instead of a silent `phase_count: 0`
|
||||
// indistinguishable from a genuinely empty milestone.
|
||||
const { value: content, scope } = extractCurrentMilestoneScoped(rawContent, cwd);
|
||||
const phasesDir = planningPaths(cwd).phases;
|
||||
|
||||
// Extract all phase headings: ## Phase N: Name or ### Phase N: Name
|
||||
@@ -486,6 +489,11 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void {
|
||||
current_phase: currentPhase ? currentPhase.number : null,
|
||||
next_phase: nextPhase ? nextPhase.number : null,
|
||||
missing_phase_details: missingDetails.length > 0 ? missingDetails : null,
|
||||
// #3184/#3165: distinguishes a genuinely empty milestone (`scope:
|
||||
// "complete"`, `phase_count: 0`) from a window that could not be fully
|
||||
// resolved (`"truncated"` / `"unscoped"` / `"unreadable"`) — those cases
|
||||
// were previously output-identical.
|
||||
scope,
|
||||
};
|
||||
|
||||
output(result, raw, undefined);
|
||||
|
||||
@@ -19,7 +19,7 @@ import phaseIdMod = require('./phase-id.cjs');
|
||||
const { escapeRegex, parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir } = phaseIdMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import roadmapParserMod = require('./roadmap-parser.cjs');
|
||||
const { getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone } = roadmapParserMod;
|
||||
const { getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone, isMilestoneBoundedInRoadmap, hasMilestoneSectioning } = roadmapParserMod;
|
||||
import { platformWriteSync, platformReadSync, platformEnsureDir, retryRenameSync, toPosixPath } from './shell-command-projection.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import planningWorkspace = require('./planning-workspace.cjs');
|
||||
@@ -1775,11 +1775,12 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto
|
||||
// downstream (mirrors the sync write-path guard).
|
||||
let milestoneBounded = true;
|
||||
if (milestone && roadmapRaw !== null) {
|
||||
const versionedHeading = new RegExp(
|
||||
`^#{1,3}\\s+(?!Phase\\s+\\S).*${escapeRegex(String(milestone).trim())}`,
|
||||
'mi',
|
||||
);
|
||||
milestoneBounded = versionedHeading.test(roadmapRaw);
|
||||
// #3184: routed through the single owner (roadmap-parser.cjs)
|
||||
// instead of a hand-rolled, unbounded-substring re-derivation —
|
||||
// the prior inline regex had no boundary assertion after the
|
||||
// version token, so `v2.0` matched inside `v2.0.1` (#2562-class
|
||||
// defect, design row 17).
|
||||
milestoneBounded = isMilestoneBoundedInRoadmap(roadmapRaw, String(milestone).trim());
|
||||
}
|
||||
// #2828: distinguish a FLAT unmilestoned roadmap (no milestone sectioning
|
||||
// at all — only Phase headings) from a MILESTONED-but-unbounded one
|
||||
@@ -1787,10 +1788,14 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto
|
||||
// On a flat roadmap the whole-doc count is correct (no sibling milestones to
|
||||
// conflate); on a sectioned-but-unbounded one it conflates siblings (#1761),
|
||||
// so fall back to phaseDirs.length.
|
||||
const hasMilestoneSectioning = roadmapRaw !== null
|
||||
&& /^#{2,3}\s+(?!Phase\s+\S)/mi.test(roadmapRaw);
|
||||
// #3184: routed through the single owner (roadmap-parser.cjs) —
|
||||
// deliberately weaker than isMilestoneBoundedInRoadmap above (no
|
||||
// version-token requirement); see hasMilestoneSectioning's own
|
||||
// doc comment for why that distinction is load-bearing.
|
||||
const roadmapHasMilestoneSectioning = roadmapRaw !== null
|
||||
&& hasMilestoneSectioning(roadmapRaw);
|
||||
const safeToUseRoadmapCount = milestoneBounded
|
||||
|| (roadmapPhaseCount > 0 && !hasMilestoneSectioning);
|
||||
|| (roadmapPhaseCount > 0 && !roadmapHasMilestoneSectioning);
|
||||
return {
|
||||
totalPhases: safeToUseRoadmapCount
|
||||
? Math.max(phaseDirs.length, roadmapPhaseCount)
|
||||
@@ -2986,8 +2991,10 @@ function cmdStateSync(cwd: string, options: StateSyncOptions | undefined, raw: b
|
||||
const versionStr = typeof fmVersion === 'string' && fmVersion.trim() ? fmVersion.trim() : null;
|
||||
let milestoneBounded = true;
|
||||
if (versionStr !== null && syncRoadmapRaw !== null) {
|
||||
const versionedHeading = new RegExp(`^#{1,3}\\s+(?!Phase\\s+\\S).*${escapeRegex(versionStr)}`, 'mi');
|
||||
milestoneBounded = versionedHeading.test(syncRoadmapRaw);
|
||||
// #3184: routed through the single owner (roadmap-parser.cjs) instead of
|
||||
// a hand-rolled, unbounded-substring re-derivation — see the identical
|
||||
// fix in buildStateFrontmatter above.
|
||||
milestoneBounded = isMilestoneBoundedInRoadmap(syncRoadmapRaw, versionStr);
|
||||
}
|
||||
let percent: number | null = null;
|
||||
if (!milestoneBounded) {
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"next.md": "#3184 (epic #3180 Phase 2): Route 0 (resume_incomplete_phase) grows by one branch that reads the new `scope` field `roadmap analyze` emits. Before this, Route 0 treated a well-formed `{phases: []}` as a clean scan and fell through to routing as though the incomplete-phase invariant (#160) had been checked and passed — the silent disarm #3165 reports, where a milestone window truncated by a closed-milestone heading yields an empty phase list with exit 0 and no error. The added `elif [ \"$ROADMAP_SCOPE\" != \"complete\" ]` arm routes that case into the SAME warn-and-fall-through branch the existing `roadmap.analyze failed` arm already uses, so a non-answer is no longer indistinguishable from a genuinely empty scan. The growth is the new arm's three warning lines, its explanatory comment, the `ROADMAP_SCOPE` assignment, and one success-criteria bullet. Adding this to the workflow rather than only to the CLI is deliberate: a spec review of this phase found that emitting `scope` without a consumer left #3165's actual symptom reproducing, so the field would have been a diagnostic nobody reads."
|
||||
}
|
||||
}
|
||||
@@ -210,7 +210,17 @@ describe('#2658: end-to-end --trae install never emits the malformed path (accep
|
||||
test('local install: no emitted .md/.js/.cjs file contains the malformed strings; the rules file is concrete', () => {
|
||||
const { configDir, root } = runMinimalInstall({ runtime: 'trae', scope: 'local' });
|
||||
try {
|
||||
const files = walk(configDir).filter((f) => /\.(md|js|cjs)$/.test(f));
|
||||
const files = walk(configDir)
|
||||
.filter((f) => /\.(md|js|cjs)$/.test(f))
|
||||
// gsd-core/CHANGELOG.md is excluded by exact relative path (not a blanket
|
||||
// .md skip — the emitted agent/command/workflow markdown this gate exists
|
||||
// to guard stays fully scanned). CHANGELOG.md legitimately QUOTES the
|
||||
// malformed `.claude/.trae/rules` / `.trae/.trae/rules` strings while
|
||||
// documenting the #2658 fix itself (#3006) — that historical-value
|
||||
// citation is not a regression of the installer's actual output. Verified
|
||||
// empirically: excluding only this one file drops the hit count to zero
|
||||
// across all 620 other emitted files.
|
||||
.filter((f) => f.split(path.sep).join('/').indexOf('gsd-core/CHANGELOG.md') === -1);
|
||||
assert.ok(files.length > 0, 'expected at least one emitted .md/.js/.cjs file');
|
||||
for (const file of files) {
|
||||
const content = fs.readFileSync(file, 'utf8');
|
||||
|
||||
1
tests/fixtures/install-tree/antigravity.json
vendored
1
tests/fixtures/install-tree/antigravity.json
vendored
@@ -415,6 +415,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/augment.json
vendored
1
tests/fixtures/install-tree/augment.json
vendored
@@ -485,6 +485,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/extract-learnings/SKILL.md",
|
||||
|
||||
@@ -483,5 +483,6 @@
|
||||
"scripts/gen-capability-registry.cjs",
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs"
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs"
|
||||
]
|
||||
|
||||
1
tests/fixtures/install-tree/claude.json
vendored
1
tests/fixtures/install-tree/claude.json
vendored
@@ -413,6 +413,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/cline.json
vendored
1
tests/fixtures/install-tree/cline.json
vendored
@@ -385,6 +385,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/extract-learnings/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/codebuddy.json
vendored
1
tests/fixtures/install-tree/codebuddy.json
vendored
@@ -485,6 +485,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
3
tests/fixtures/install-tree/codex.json
vendored
3
tests/fixtures/install-tree/codex.json
vendored
@@ -492,5 +492,6 @@
|
||||
"scripts/gen-capability-registry.cjs",
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs"
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs"
|
||||
]
|
||||
|
||||
1
tests/fixtures/install-tree/copilot.json
vendored
1
tests/fixtures/install-tree/copilot.json
vendored
@@ -384,6 +384,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/cursor.json
vendored
1
tests/fixtures/install-tree/cursor.json
vendored
@@ -391,6 +391,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/hermes.json
vendored
1
tests/fixtures/install-tree/hermes.json
vendored
@@ -414,6 +414,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd/DESCRIPTION.md",
|
||||
"skills/gsd/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/kilo.json
vendored
1
tests/fixtures/install-tree/kilo.json
vendored
@@ -488,6 +488,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/kimi-code.json
vendored
1
tests/fixtures/install-tree/kimi-code.json
vendored
@@ -414,6 +414,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/kimi.json
vendored
1
tests/fixtures/install-tree/kimi.json
vendored
@@ -450,6 +450,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/opencode.json
vendored
1
tests/fixtures/install-tree/opencode.json
vendored
@@ -488,6 +488,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-add-tests/SKILL.md",
|
||||
"skills/gsd-ai-integration-phase/SKILL.md",
|
||||
"skills/gsd-audit-fix/SKILL.md",
|
||||
|
||||
3
tests/fixtures/install-tree/pi.json
vendored
3
tests/fixtures/install-tree/pi.json
vendored
@@ -381,5 +381,6 @@
|
||||
"scripts/gen-capability-registry.cjs",
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs"
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs"
|
||||
]
|
||||
|
||||
1
tests/fixtures/install-tree/qwen.json
vendored
1
tests/fixtures/install-tree/qwen.json
vendored
@@ -414,6 +414,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/extract-learnings/SKILL.md",
|
||||
|
||||
1
tests/fixtures/install-tree/trae.json
vendored
1
tests/fixtures/install-tree/trae.json
vendored
@@ -382,6 +382,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/extract-learnings/SKILL.md",
|
||||
|
||||
3
tests/fixtures/install-tree/windsurf.json
vendored
3
tests/fixtures/install-tree/windsurf.json
vendored
@@ -384,5 +384,6 @@
|
||||
"scripts/gen-capability-registry.cjs",
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs"
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs"
|
||||
]
|
||||
|
||||
1
tests/fixtures/install-tree/zcode.json
vendored
1
tests/fixtures/install-tree/zcode.json
vendored
@@ -453,6 +453,7 @@
|
||||
"scripts/gen-loop-host-contract.cjs",
|
||||
"scripts/lib/allowlist-ratchet.cjs",
|
||||
"scripts/lib/cli-exit.cjs",
|
||||
"scripts/lib/drift-scan.cjs",
|
||||
"skills/gsd-ns-context/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/docs-update/SKILL.md",
|
||||
"skills/gsd-ns-context/skills/extract-learnings/SKILL.md",
|
||||
|
||||
@@ -49,6 +49,8 @@ const {
|
||||
configureKiloPermissions,
|
||||
selectRuntimesFromArgs,
|
||||
normalizeNodePath,
|
||||
GSD_CHANGESET_FILES,
|
||||
GSD_SCRIPTS_LIB_FILES,
|
||||
} = require('../bin/install.js');
|
||||
|
||||
const { getGlobalConfigDir } = require('../gsd-core/bin/lib/runtime-homes.cjs');
|
||||
@@ -11107,3 +11109,48 @@ describe('#3026: installer --help documents every accepted runtime flag', () =>
|
||||
`--help must document every accepted runtime flag; missing: ${missing.join(', ')}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3184: scripts/lib/ and scripts/changeset/ install/uninstall parity ────
|
||||
//
|
||||
// install() copies both source directories WHOLESALE (every file present —
|
||||
// see bin/install.js's "and any future lib helpers" comment), but uninstall()
|
||||
// removes files via an EXPLICIT hardcoded enumeration (GSD_SCRIPTS_LIB_FILES /
|
||||
// GSD_CHANGESET_FILES). Nothing keeps the two in sync: a file added to either
|
||||
// source directory ships to every install and then orphans on uninstall (it
|
||||
// survives removal, keeps the target dir non-empty, and blocks its rmdir).
|
||||
// This guard fails the moment the real directory outgrows the enumeration.
|
||||
|
||||
/**
|
||||
* Pure comparison: every file actually present in a source dir must be named
|
||||
* in the corresponding uninstall enumeration. Decoupled from fs so it can be
|
||||
* exercised directly with doctored input (see the probe in the PR report).
|
||||
*/
|
||||
function findUnenumeratedFiles(actualFiles, enumeratedFiles) {
|
||||
const enumerated = new Set(enumeratedFiles);
|
||||
return actualFiles.filter(f => !enumerated.has(f));
|
||||
}
|
||||
|
||||
describe('#3184: scripts/lib/ and scripts/changeset/ install/uninstall parity', () => {
|
||||
const REPO_ROOT = path.join(__dirname, '..');
|
||||
|
||||
function realFilesIn(relativeDir) {
|
||||
const dir = path.join(REPO_ROOT, relativeDir);
|
||||
return fs.readdirSync(dir).filter(entry => fs.statSync(path.join(dir, entry)).isFile());
|
||||
}
|
||||
|
||||
test('every file in scripts/lib/ is enumerated in GSD_SCRIPTS_LIB_FILES (bin/install.js)', () => {
|
||||
const unenumerated = findUnenumeratedFiles(realFilesIn(path.join('scripts', 'lib')), GSD_SCRIPTS_LIB_FILES);
|
||||
assert.deepEqual(unenumerated, [],
|
||||
`scripts/lib/${unenumerated.join(', scripts/lib/')} exist on disk but are missing from ` +
|
||||
`GSD_SCRIPTS_LIB_FILES in bin/install.js — add ${unenumerated.length === 1 ? 'it' : 'them'} to that ` +
|
||||
'array so uninstall() removes it (otherwise it ships to every install and orphans on uninstall).');
|
||||
});
|
||||
|
||||
test('every file in scripts/changeset/ is enumerated in GSD_CHANGESET_FILES (bin/install.js)', () => {
|
||||
const unenumerated = findUnenumeratedFiles(realFilesIn(path.join('scripts', 'changeset')), GSD_CHANGESET_FILES);
|
||||
assert.deepEqual(unenumerated, [],
|
||||
`scripts/changeset/${unenumerated.join(', scripts/changeset/')} exist on disk but are missing from ` +
|
||||
`GSD_CHANGESET_FILES in bin/install.js — add ${unenumerated.length === 1 ? 'it' : 'them'} to that ` +
|
||||
'array so uninstall() removes it (otherwise it ships to every install and orphans on uninstall).');
|
||||
});
|
||||
});
|
||||
|
||||
1285
tests/milestone-window-single-owner.test.cjs
Normal file
1285
tests/milestone-window-single-owner.test.cjs
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user