diff --git a/.changeset/jolly-jaguars-caper.md b/.changeset/jolly-jaguars-caper.md new file mode 100644 index 000000000..d6353e835 --- /dev/null +++ b/.changeset/jolly-jaguars-caper.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3222 +--- +**Progress, stats, and phase listings now stay within the current milestone.** `progress`, `stats`, and `phases list` no longer count backlog (`999.*`) or pre-milestone (`0-*`) directories as current-milestone phases, and `phases clear` / `milestone complete` no longer delete or archive those directories. `phases list --phase` and `--include-archived` are unaffected, since they intentionally look up or list beyond the current milestone. (#3185) diff --git a/CONTEXT.md b/CONTEXT.md index b76f809c0..e11bf76f9 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -27,7 +27,7 @@ Module owning phase-effort estimation and its calibration against measured reali Module owning the canonical phase-verification status projection shared by phase transition, progress, manager, autonomous, and closeout readiness paths. `readVerificationStatus(phaseDir, opts?)` reads the first `*-VERIFICATION.md` frontmatter `status`, maps it through `VERIFICATION_ROUTING_TABLE`, and fail-closes — only `{passed}` satisfies the canonical gate; `missing`/`unknown`/`gaps_found`/`human_needed`/`stale` all route away from "complete" (#1522). `findStaleVerificationSummary` flags a SUMMARY newer than the VERIFICATION file (status `stale`). Both honor a no-throw, degrade-to-safe contract (any FS error → `missing` / not-stale) and an injectable `opts.fs` seam. Source of truth: `gsd-core/bin/lib/verification.cjs` (generated from `src/verification.cts`). ### Phase Locator Module -Module owning phase-directory search and location: active-phase discovery against the `.planning/phases/` tree (`searchPhaseInDir`, `findPhaseInternal`) and archived-phase-dir enumeration (`getArchivedPhaseDirs`), matching phase ids/tokens against the filesystem. Depends only on leaf modules (`phase-id` for token/name matching, `core-utils` for fs-scan/path helpers, `planning-workspace` for `planningDir`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2d (#881); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-locator.cjs` (generated from `src/phase-locator.cts`). Since #2830, `searchPhaseInDir` also parses each plan's `depends_on` and each completed plan's SUMMARY `status` and calls Plan Dependency Graph Module's `computeHaltPropagation` to populate `halted_plans`/`blocked_by`/`runnable_plans` — additive fields; `incomplete_plans` keeps its pre-#2830 meaning unchanged. +Module owning phase-directory search and location: active-phase discovery against the `.planning/phases/` tree (`searchPhaseInDir`, `findPhaseInternal`) and archived-phase-dir enumeration (`getArchivedPhaseDirs`), matching phase ids/tokens against the filesystem. Depends only on leaf modules (`phase-id` for token/name matching, `core-utils` for fs-scan/path helpers, `planning-workspace` for `planningDir`) — no `loadConfig`, no other core dependency. Extracted from the Core module per ADR-857 rollout phase 2d (#881); the `core.cjs` re-export spine was retired in epic #1267, so callers import this leaf directly. Source of truth: `gsd-core/bin/lib/phase-locator.cjs` (generated from `src/phase-locator.cts`). Since #2830, `searchPhaseInDir` also parses each plan's `depends_on` and each completed plan's SUMMARY `status` and calls Plan Dependency Graph Module's `computeHaltPropagation` to populate `halted_plans`/`blocked_by`/`runnable_plans` — additive fields; `incomplete_plans` keeps its pre-#2830 meaning unchanged. Since #3185 (ADR-3180 Decision 1, Phase 3), the module also owns `listMilestonePhaseDirs(phasesDir, { cwd, ws, versionOverride, phaseIdConvention })`, the single canonical owner of milestone-scoped phase-directory enumeration: it applies the current milestone's `ROADMAP.md` window (via `getMilestonePhaseFilter`) and then the canonical `isSentinelPhaseId` sentinel filter, in that order, over the raw `phasesDir` directory listing. It returns `{ value: string[], scope }`, where `scope` is the `SCOPE` enum from `src/planning-scope.cts` (`complete`/`truncated`/`unscoped`/`unreadable`), so a caller can distinguish a genuinely empty milestone from an enumeration that could not be scoped. Consumed by `query progress`, `stats`, and the bare `phases list`, all of which need "which phases belong to this milestone." `phases list --phase` and `--include-archived` (lookup/archive questions) read the unscoped physical directory set and do not call this owner. `phases clear` and `milestone complete`'s phase-archival move call `isSentinelPhaseId` directly instead — they must sweep every non-sentinel phase directory regardless of milestone window, so they take the sentinel filter without this owner's window scoping. ### Plan Dependency Graph Module Module owning the single halt-propagation engine over a plan's `depends_on` DAG (#2830). **Domain term: _halted_** — a plan that reached a designed stop (a gate failure, a spike concluding without expanding, or any other intentional non-completion) and wrote a SUMMARY recording that fact via `status: halted` in its frontmatter, as opposed to `status: complete` (ordinary finish) or no SUMMARY at all (not yet attempted). **Domain term: _blocked_** — a plan whose `depends_on` chain reaches a halted plan, directly or transitively; distinct from merely _incomplete_ (no SUMMARY yet) — an ordinary in-progress/not-yet-started dependency does not block. `computeHaltPropagation(nodes: {id, resolvedDependsOn, halted}[])` performs exactly one Kahn's-algorithm topological pass and returns `{order, visited, blockedBy}`, where `blockedBy` maps a plan id to the de-duplicated set of halted plan ids transitively upstream of it (diamond-safe, any depth). This is the SHARED engine both of the two independent "which plans are incomplete" readers call — `phase.cts`'s wave-grouping (`cmdPhasePlanIndex`) and `phase-locator.cts`'s phase-location primitive (`searchPhaseInDir`) — so the two-implementation divergence that caused #2830 (one parsed `depends_on` for waves only, the other never parsed it at all) cannot recur: each caller resolves its own raw `depends_on` tokens to canonical ids before calling in, but the graph traversal itself exists in exactly one place. Pure — no I/O, no config; each caller does its own file reads (a plan's frontmatter, a completed plan's SUMMARY `status`) and fails open (treats an unreadable/malformed file as "not halted"/"no deps") rather than throwing. Source of truth: `gsd-core/bin/lib/plan-dependency-graph.cjs` (generated from `src/plan-dependency-graph.cts`). diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index b7e1bff5f..9e584f39c 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -140,8 +140,32 @@ node gsd-tools.cjs phase-plan-index # List phases with filtering node gsd-tools.cjs phases list [--type planned|executed|all] [--phase N] [--include-archived] + +# Archive (or, with --force, permanently delete) every current phase directory — +# used by /gsd-new-milestone before roadmapping the next cycle +node gsd-tools.cjs phases clear [--confirm] [--force] [--archive-version ] ``` +### Milestone-scoped phase listing (`phases list`) + +The bare `phases list` (no `--phase`, no `--include-archived`) is scoped to the +current milestone's `ROADMAP.md` window **and** filtered through the canonical +sentinel predicate: `999.*` backlog directories and `0-*` pre-milestone +directories are not listed as current-milestone phases. `--phase ` (a direct +lookup) and `--include-archived` (an archive listing) are deliberately **not** +scoped or sentinel-filtered — they answer "does this phase exist" and "what has +ever existed here," not "what belongs to this milestone," so they still see +sentinel and out-of-window directories. + +### `phases clear` and sentinel directories + +`phases clear` moves (or, with `--force` and no prior archive, permanently +deletes) every phase directory under `.planning/phases/` except sentinels. It +now excludes both `999.*` (backlog) and `0-*` (pre-milestone) directories via +the same canonical sentinel predicate `phases list` uses — previously its own +regex excluded `999` but not `0`, so a `0-*` directory could be destroyed on +this irreversible path. + ### Phase SUMMARY artifact check A phase `SUMMARY.md` asserts which files the phase created or modified. On @@ -585,6 +609,8 @@ node gsd-tools.cjs requirements mark-complete **Unstarted-phase guard.** Before archiving, the command scans the ROADMAP scoped for `` and refuses if any `### Phase N:` heading in that slice has no matching phase directory on disk (`disk_status: no_directory`). Phase 0 (pre-milestone) and Phase 999 (backlog) sentinels are excluded. The guard runs whenever `--force` is absent, independent of `STATE.md`'s `milestone:` field — if that field is present but does not match ``, a WARNING naming both values is emitted to stderr and the scan still runs (#2946). Pass `--force` to override. +**Sentinel directories are never archived.** The phase-directory move performed when `--no-archive-phases` is absent is now filtered through the same canonical sentinel predicate as `phases list` and `phases clear`: `999.*` (backlog) and `0-*` (pre-milestone) directories are left in place rather than moved into `.planning/milestones/-phases/`. Previously this path was scoped only by the milestone window, with no sentinel filter, so a sentinel directory sitting inside the window could be archived along with the milestone's real phases. + --- ## Agent Skills @@ -670,7 +696,15 @@ node gsd-tools.cjs progress [json|table|bar] # Progress as typed JSON surface (#455) node gsd-tools.cjs progress --json +``` +Both `stats` and `progress` are scoped to the current milestone's `ROADMAP.md` +window and sentinel-filtered: `999.*` backlog directories and `0-*` +pre-milestone directories are not counted as current-milestone phases, and the +aggregate completion percentage no longer reads `100` while phases from the +active window are still outstanding. + +```bash # Complete a todo node gsd-tools.cjs todo complete diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 495f2cd50..5e6a40f36 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -476,6 +476,8 @@ If any category is non-empty you are prompted with `[R] Resolve` / `[A] Acknowle > **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 --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 `` prints a WARNING and still runs the guard (#2946). +> **Sentinel directories stay put.** Moving phase directories into the archive (the default, unless `--no-archive-phases` is passed) now excludes `999.*` (backlog) and `0-*` (pre-milestone) directories via the same sentinel predicate the unstarted-phase guard already uses. Previously the archive move was scoped only by the milestone window, so a sentinel directory sitting inside that window could be archived along with the milestone's own phases. + --- ### `/gsd-milestone-summary` @@ -662,6 +664,8 @@ Show status, next steps, and automatically advance to the next logical workflow - Phase needs verification → runs `/gsd-verify-work` - All phases complete → suggests `/gsd-complete-milestone` +Status reporting is scoped to the current milestone's `ROADMAP.md` window and sentinel-filtered: `999.*` backlog directories and `0-*` pre-milestone directories are not counted as current-milestone phases, so the reported progress percentage no longer holds at `100` while phases in the active window are still outstanding. + ```bash /gsd-progress # "Where am I? What's next?" with auto-routing /gsd-progress --next # Advance to next step automatically @@ -942,6 +946,8 @@ Display project statistics. /gsd-stats # Project metrics dashboard ``` +Scoped to the current milestone's `ROADMAP.md` window and sentinel-filtered: `999.*` backlog directories and `0-*` pre-milestone directories are not counted as current-milestone phases. + ### `/gsd-profile-user` Generate a developer behavioral profile from Claude Code session analysis across 8 dimensions (communication style, decision patterns, debugging approach, UX preferences, vendor choices, frustration triggers, learning style, explanation depth). Produces artifacts that personalize Claude's responses. diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index 36b06f66c..8658e0979 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -319,7 +319,7 @@ Ideas that aren't ready for active planning go into the backlog using 999.x numb /gsd-capture --backlog "Mobile responsive" # Creates 999.2-mobile-responsive/ ``` -Backlog items get full phase directories, so you can use `/gsd-discuss-phase 999.1` to explore an idea further or `/gsd-plan-phase 999.1` when it's ready. +Backlog items get full phase directories, so you can use `/gsd-discuss-phase 999.1` to explore an idea further or `/gsd-plan-phase 999.1` when it's ready. Backlog directories (and the `0-*` pre-milestone directory some projects carry) are excluded from `/gsd-progress`, `/gsd-stats`, and phase listings for the current milestone — they stay out of the active phase sequence for counting purposes too, not just for planning. **Review and promote** with `/gsd-review-backlog` — it shows all backlog items and lets you promote (move to active sequence), keep (leave in backlog), or remove (delete). diff --git a/docs/adr/3180-planning-semantic-model-single-owner.md b/docs/adr/3180-planning-semantic-model-single-owner.md index 8da2c7044..0c4428a9b 100644 --- a/docs/adr/3180-planning-semantic-model-single-owner.md +++ b/docs/adr/3180-planning-semantic-model-single-owner.md @@ -351,3 +351,122 @@ ADR predicted lands as written, plus two the prediction did not contain: **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. + +### Amendment 3 — Phase 3 (#3185) validation: the contract held; the load-bearing bug was upstream of enumeration itself + +Decision 2's contract **held** for its third consumer: `listMilestonePhaseDirs` returns +`ScopedResult` unchanged, and `SCOPE` needed no new member — every case Phase 3 hit +(a genuinely empty milestone, a truncated window, an unscoped/legacy ROADMAP, an unreadable +ROADMAP) was already one of the four frozen values. + +**Declared deviation from Decision 1's provisional signature.** Decision 1 locked +`listMilestonePhaseDirs(roadmapContent: string, phasesDir: string, deps?): ScopedResult`. +That signature cannot work: the milestone window needs `cwd` (to read `STATE.md` for the active +milestone version) and `ws` (workstream scoping), and `getMilestonePhaseFilter` — the post-#3184 +canonical owner of "read the ROADMAP and resolve the window" — reads `ROADMAP.md` itself rather +than accepting its content as an argument. Threading a pre-read `roadmapContent` string past that +owner would reintroduce a second ROADMAP-reading path beside it, which is exactly the divergence +class this epic removes. Shipped signature: +`listMilestonePhaseDirs(phasesDir, { cwd, ws, versionOverride, phaseIdConvention })`. This is a +signature change, not a contract change — `ScopedResult` and `SCOPE` are unaffected, so it does +not require re-litigating Decision 2. + +**The copy count was a lower bound, a third consecutive time.** The epic scoped enumeration at +**4 copies**. The Decision 4(a) whole-repo guard (`scripts/lint-phase-enumeration-drift.cjs`) found +**54 violations**: 23 sentinel re-derivations across 8 modules, in three regex variants plus four +integer-comparison forms, now consolidated onto the canonical `isSentinelPhaseId` +(`SENTINEL_RANGES [0, 999]`); plus 31 unscoped `phasesDir` reads. Most of the 23 sentinel +re-derivations tested only `999`, so Phase 0 previously slipped through every one of them. + +**The load-bearing finding: the sentinel exclusion lived on the wrong set.** The pre-existing +sentinel exclusion was applied to the ROADMAP HEADING set (`### Phase N:` entries), not to +phase-directory names — but `getMilestonePhaseFilter` degrades to a literal pass-all `() => true` +when that heading set is empty, per Decision 3's documented "over-inclusive, never +under-inclusive" promise. The exclusion was therefore unreachable exactly when it was needed: a +backlog or pre-milestone directory has no corresponding ROADMAP heading to exclude by, so the +filter that was supposed to keep it out degraded to accepting everything instead. This is the same +path #3167 named, and it is why `cmdStats` already called `getMilestonePhaseFilter` and still +listed backlog directories — calling the filter was not enough while the filter's own pass-all +degrade could not distinguish "no phases in this milestone" from "no heading to test this directory +against." The fix applies the sentinel test to **directory names**, unconditionally, after the +window filter runs rather than folding it into the window filter's heading-matching logic. The +narrowing is sentinel-only: pass-all still stands for every non-sentinel directory the window +filter cannot place, so Decision 3's promise is narrowed minimally, not revoked (Decision 3 / +Hyrum's Law). + +**#3161 is subsumed alongside #3167, as the Tier-2 table predicted.** #3161 ("aggregate percent +reports 100 while plans are outstanding") shared the same upstream cause: `cmdStats`'s and +`cmdProgressRender`'s `totalPlans`/`totalSummaries` accumulation now iterates the single owner's +scoped, sentinel-filtered `dirs` set (`listMilestonePhaseDirs`'s `value`) instead of an unscoped +`readdirSync` of the phases directory, so a `999.*`/`0-*` directory with its own already-summarized +plans can no longer inflate `totalSummaries` (or `totalPlans`) against a milestone that has not +actually finished — the same backlog-dir listing bug row 3 named, manifesting in the percent +aggregate rather than the phase list. + +**Two destructive-path defects the sweep exposed.** `phases clear` carried the fifth sentinel copy +and its third regex variant (`/^999(?:\.|$)/`) — it excluded `999` but not `0`, so a `0-*` +pre-milestone directory was deleted (or, pre-#1871, hard-removed) on this irreversible path. +`milestone complete`'s phase-archival move had no sentinel filter at all on its stats/dry-run/move +paths — only the milestone window — so a sentinel directory sitting inside the window's phase range +could be archived alongside the milestone's own phases. Both now route through +`isSentinelPhaseId` directly (not through `listMilestonePhaseDirs`, since both need every +non-sentinel directory regardless of milestone window — see the generalized rule below). + +**An unadvertised but correct Tier-2 change.** Phase 0 directories now drop out of +`progress`/`stats`/`phases list` alongside Phase 999, because the canonical predicate treats both +sentinels alike and the engine-wide convention (#1580) already declares both as sentinel ranges, +while `roadmap analyze` (Phase 2) already honored it. This was not separately predicted by Decision +3's table — it falls out of routing every reader through one predicate that was already correct. + +**The guard's own false positive, worth recording.** The first version of +`scripts/lint-phase-enumeration-drift.cjs` flagged JSDoc comments and inline comments that merely +*documented* that the code below already called the canonical owner — matching sentinel-shaped +regex literals inside prose, not code. It is now comment-aware (skips block/line comments before +matching). Recorded because a guard that reports prose as drift trains its readers to reflexively +exempt documentation, which is the opposite of Decision 4(a)'s whole-repo, no-allowlist intent. + +**The generalized exemption rule**, restated from Amendment 1's file-naming case in this +derivation's terms: a **lookup**, **diagnostic**, **archival**, or **mutation** pass wants the +physical directory set — every non-sentinel directory on disk, regardless of milestone window. +Only "which phases belong to the current milestone" wants the scoped set `listMilestonePhaseDirs` +returns. `phases list --phase N` and `--include-archived` (lookup/archive) and `phases clear` / +`milestone complete` (destructive mutation) take the first; `progress`, `stats`, and the bare +`phases list` take the second. + +**Decision 3's Tier-2 table, re-derived for Phase 3** per its own contingency clause: + +| Command surface | Output change | +|---|---| +| `query progress`, `stats`, bare `phases list` | `999.*` backlog and `0-*` pre-milestone directories are no longer listed or counted as current-milestone phases; the aggregate completion percentage stops reading `100` while phases in the active window are still outstanding | +| `phases clear` | no longer deletes/archives a `0-*` pre-milestone directory — previously excluded `999` but not `0` on this irreversible path | +| `milestone complete` | its phase-archival move no longer sweeps sentinel directories into `.planning/milestones/-phases/` alongside the milestone's own phases | +| `stats` P0.0 plan-count correction — **not predicted** | `isDirInMilestone` could not match a #1324 letter-prefixed-decimal directory (`P0.0-foundation`) to its own `### Phase P0.0:` ROADMAP heading, so `stats` reported that phase with `plans: 0` while its directory held real plan files. Fixed inline as part of the same sweep; not a Decision-1 owner change, but a defect the whole-repo guard's investigation surfaced in the same code path | + +**A single owner is not always a single RULE — the `0.x` split.** The sharpest finding of this +phase, and a correction to how Decision 1 reads. An isolated security review observed that +`isSentinelPhaseId` classifies `0.1` / `00.1` as sentinel milestone 0 (its `/^0*(\d+)/` backtracks +to capture `0`), and that this looked wrong against #2554. Changing the canonical predicate to +exempt `0.x` made the suite fail **six** tests, because two PINNED contracts disagree — and both +are right, because they ask different questions: + +| Contract | Question | Verdict on `0.x` | +|---|---|---| +| #2554 (`roadmap-parser.test.cjs`) | is this directory part of the current milestone's phase SET? | **count it** — a `00.1-` dir declared as `### Phase 00.1:` is a real phase | +| #2949 (`issue-2949-phase-complete-stage3-sentinel.test.cjs`) | must this phase COMPLETE before the milestone can close? | **sentinel** — a `0.x` must not block `is_last_phase` | + +No single global predicate answers both. The resolution is layered, not unified: `isSentinelPhaseId` +keeps its semantics (`0.x` IS a sentinel, satisfying #2949), and the milestone-WINDOW layer keeps a +narrower 999-only rule (satisfying #2554), carried as a function-scoped guard exemption with a +written reason rather than a second silent copy. + +**The lesson for Phases 4 and 5:** "one owner per derivation" governs *who computes an answer*, not +*how many questions share it*. Before folding a call site onto a canonical predicate, establish +which question that site asks — an over-broad canonical rule is as much a defect as a divergent +copy, and it fails in a worse way, because it looks like consolidation. Note also that the +security review's data-completeness concern here was *inference* about intent, while #2949 is +*pinned* intent; where the two conflict, the pinned contract wins and the review finding is +recorded as adjudicated rather than fixed. + +**Scope note.** Phase 3 is the last consumer of the enumeration/window layer; Phases 4 and 5 build +on the completion and state-extraction derivations respectively and do not depend on +`listMilestonePhaseDirs`. diff --git a/package.json b/package.json index 6048786b7..dcdcab570 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,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 && node scripts/lint-milestone-window-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 && node scripts/lint-phase-enumeration-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", diff --git a/scripts/lint-milestone-window-drift.cjs b/scripts/lint-milestone-window-drift.cjs index 49aa81685..94fd31c6e 100644 --- a/scripts/lint-milestone-window-drift.cjs +++ b/scripts/lint-milestone-window-drift.cjs @@ -195,6 +195,32 @@ function readStringLiteralAt(line, start) { return null; } +/** + * Strip comment text from a line before detection. A guard that fires on a + * COMMENT — including a comment documenting that the code below uses the + * canonical owner, or prose quoting this guard's own detector shapes — reports + * prose as drift and trains readers to add exemptions for documentation. + * Handles the three shapes that appear in this codebase: a whole-line + * block-comment continuation (`*` or `/*` leading), a `//` line comment, and + * a trailing `//` after code. Mirrors `lint-phase-enumeration-drift.cjs`'s + * own copy (not shared — each guard applies it at a slightly different point + * in its detection pipeline). + * + * Deliberately simple and conservative: it does not attempt full block-comment + * state tracking across lines (this is a per-line scan, same tradeoff the + * sibling guards document). A `//` inside a string literal would be stripped + * early — accepted, because the effect is to UNDER-report on a pathological + * line, never to over-report prose as drift. + */ +function stripComments(line) { + const trimmed = line.trim(); + // Whole-line block comment or JSDoc continuation. + if (trimmed.startsWith('*') || trimmed.startsWith('/*') || trimmed.startsWith('//')) return ''; + // Trailing line comment after code. + const idx = line.indexOf('//'); + return idx === -1 ? line : line.slice(0, idx); +} + /** * The first literal (regex OR quoted/backtick string) on `line` whose text * contains a HEADING_QUANTIFIER_RE match — the "smoking gun" fragment worth @@ -202,7 +228,10 @@ function readStringLiteralAt(line, start) { * 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). + * than a thrown error if a future line splits them). Takes the RAW `line` + * (not comment-stripped) so a reported fragment still shows the actual source + * text — comment-stripping is applied only to the detection decision, never + * to the reported fragment. */ function extractFragment(line) { for (let i = 0; i < line.length; i++) { @@ -233,8 +262,11 @@ function findMilestoneWindowDrift(text, relPath) { 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)); + const code = stripComments(line); + if (!code.trim()) continue; + + if (!HEADING_QUANTIFIER_RE.test(code)) continue; + const isMilestoneWindowToken = PHASE_LOOKAHEAD_RE.test(code) || (VERSION_TOKEN_RE.test(code) && MARKER_EMOJI_RE.test(code)); if (!isMilestoneWindowToken) continue; if (exemptFunctions && exemptFunctions.has(currentFunction)) continue; @@ -297,4 +329,5 @@ module.exports = { FUNCTION_SCOPED_EXEMPTIONS, readStringLiteralAt, extractFragment, + stripComments, }; diff --git a/scripts/lint-phase-enumeration-drift.cjs b/scripts/lint-phase-enumeration-drift.cjs new file mode 100644 index 000000000..81ea0bb50 --- /dev/null +++ b/scripts/lint-phase-enumeration-drift.cjs @@ -0,0 +1,457 @@ +#!/usr/bin/env node +'use strict'; + +/** + * Anti-divergence drift guard for the PHASE-ENUMERATION seam (epic #3180, + * issue #3185, ADR-3180 Decision 1 row "Phase enumeration"). + * + * `src/phase-locator.cts` is the SINGLE canonical owner of "which phase + * directories belong to the current milestone" — `listMilestonePhaseDirs`. + * `src/phase-id.cts` is the SINGLE canonical owner of "is this phase id a + * reserved sentinel (Phase 0 / Phase 999.x)" — `isSentinelPhaseId` / + * `SENTINEL_RANGES`. Before these existed the derivation had four + * independent implementations, and the sentinel rule had five copies across + * three different regexes that disagreed about Phase 0 (see + * `listMilestonePhaseDirs`'s own doc comment). Every other module that + * hand-rolls a `readdirSync` over the phases directory, or hand-rolls a + * `999`-shaped sentinel test, is a re-derivation that can silently drift + * from one of these two owners — the same generative-fix-divergence class + * `lint-plan-count-drift.cjs` and `lint-milestone-window-drift.cjs` exist to + * remove, now applied to the phase-enumeration seam. + * + * 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 and + * `lint-milestone-window-drift.cjs`'s precedent. + * + * TWO INDEPENDENT DETECTORS. A line matching EITHER is a violation: + * + * DETECTOR 1 (enumeration): a single source line carrying BOTH + * (a) `readdirSync`, AND + * (b) a phases-directory token — the identifier `phasesDir`, or a + * quoted/backticked `'phases'` string. + * This is the shape every consumer used before routing through the owner. + * + * DETECTOR 2 (sentinel): a single source line carrying a sentinel-range + * literal outside the owner — + * - a bare `999` inside a regex literal or a quoted/backticked string + * (e.g. `/^999\b/`, `/^999(?:\.|$)/`, `'999'`), OR + * - a numeric comparison against 999 (`=== 999`, `== 999`, `!== 999`), + * or a bare reference to `SENTINEL_RANGES` outside its owner. + * Deliberately NARROW, mirroring the sibling guards' token discipline: the + * `999` must be a STANDALONE digit run (no digit immediately before or + * after it, inside the literal or right after the comparison operator), so + * unrelated arithmetic (`total + 1999`, `=== 9990`) never fires — only a + * line that actually spells the reserved sentinel value fires. + * + * Owner files (exempt by construction — each not only DEFINES its half of + * the grammar but composes it at internal call sites that are the canonical + * implementation, not copies of it): + * - `src/phase-locator.cts` — defines `listMilestonePhaseDirs`, the + * enumeration owner, and legitimately calls `readdirSync` on the phases + * dir inside it. + * - `src/phase-id.cts` — defines `isSentinelPhaseId` and + * `SENTINEL_RANGES`, the sentinel owner. + * + * FUNCTION-SCOPED EXEMPTIONS (per ADR-3180 Decision 4(a) — a written reason, + * never a bare file allowlist, so an unrelated re-derivation added anywhere + * ELSE in these same files is still caught). Generalizing #3183's rule ("a + * diagnostic about file NAMING wants the physical set; only a question about + * outstanding WORK wants the live set"): a LOOKUP, DIAGNOSTIC or ARCHIVAL + * pass wants the physical set; only "which phases belong to this milestone" + * wants the scoped set. + * - `src/roadmap.cts` `cmdRoadmapAnalyze`: builds `_phaseDirNames` as a + * heading->directory LOOKUP INDEX, not a milestone enumeration. It must + * see the PHYSICAL set so a heading already scoped by + * `extractCurrentMilestoneScoped` can find its directory; filtering it + * through the owner would scope the same set twice. + * - `src/verify.cts` `collectDiskPhases`: a DRIFT DIAGNOSTIC comparing + * what is on disk against what the ROADMAP declares. It wants the + * physical set by definition — scoping it would make the diagnostic + * unable to see the very drift it exists to report. + * - `src/verify.cts` `cmdValidateHealth`: a project-wide HEALTH-CHECK sweep + * (config drift, phase-directory naming, duplicate-directory collisions, + * unsummarized-plan detection) — same "sweep everything, report gaps" + * shape as `collectDiskPhases` and the `audit.cts` scanners; it must see + * every phase directory regardless of milestone window to catch a + * naming/duplicate defect wherever it lives. + * - `src/verify.cts` `cmdVerifySchemaDrift`: resolves ONE caller-supplied + * `phase` argument to its directory (falling back to an exact-name + * match) — a single-phase LOOKUP, not a current-milestone enumeration. + * - `src/init.cts` `detectHasPriorPhases`: answers "has this project EVER + * completed a phase", explicitly excluding the current one. A history + * probe across all milestones, not a current-milestone enumeration. + * - `src/init.cts` `detectUiPhaseActive`: its `readdirSync` targets a + * SINGLE already-resolved phase directory's own FILES + * (`phases/`) to check for a `*-UI-SPEC.md` file — not the + * phases directory itself. It never enumerates which phases exist at + * all, so it is not this derivation, only shaped like it textually. + * - `src/init.cts` `cmdInitMilestoneOp`: `diskPhaseDirs` is a heading-> + * directory LOOKUP INDEX (same role as `cmdRoadmapAnalyze`'s + * `_phaseDirNames` below) — the ROADMAP heading scan just above it + * already scopes `roadmapPhaseNumbers` to the current milestone, so this + * map must see the PHYSICAL set to resolve each heading's phase number + * to its actual directory name; scoping it again would look up inside + * an already-scoped set for no benefit. Its sibling readdirSync (the + * no-ROADMAP-headings-found fallback, where there is no heading scope + * to look inside) is a real current-milestone enumeration and is routed + * through the owner, not exempted. + * - `src/milestone.cts` `archivePhaseDirectories`: archival MOVES the + * physical set. Scoping it would silently leave out-of-window + * directories behind in the live tree. + * - `src/milestone.cts` `cmdMilestoneComplete`: its one remaining + * unrouted readdirSync (`phaseDirEntries`, the unstarted-phase guard) + * is a token-match LOOKUP against ROADMAP headings already scoped by + * `sliceMilestoneWindow`/`extractCurrentMilestone` above it — same + * "physical set feeds an already-scoped lookup" shape as + * `cmdRoadmapAnalyze`'s `_phaseDirNames`. Its three OTHER former + * readdirSync call sites (the stats aggregation, the dry-run archive + * preview, and the real archive-move loop) all genuinely asked "which + * phases belong to the current milestone" and are routed through the + * owner with the resolved `version` as `versionOverride`. + * - `src/milestone.cts` `cmdPhasesClear`: a destructive CLEAR that must + * remove every live phase directory except sentinels, regardless of + * milestone window — `new-milestone` runs this to wipe the ENTIRE + * phases tree before starting fresh, not just the outgoing milestone's + * slice. Scoping it to one milestone's window would silently leave + * out-of-window directories behind instead of clearing/archiving them. + * - `src/phase.cts` `cmdPhasesList`: its `--phase ` lookup and + * `--include-archived` merge are phase LOCATION and archive + * enumeration, not current-milestone enumeration; both legitimately + * read the physical set. Its ENUMERATION path routes through the owner. + * - `src/roadmap-parser.cts` `getMilestonePhaseFilter`: its two heading/ + * bullet scans that seed `milestonePhaseNums` deliberately use the local + * `999`-only literal, NOT `isSentinelPhaseId`. That canonical predicate + * additionally treats a leading `0` as sentinel milestone 0 (via its + * `/^0*(\d+)/` backtrack), which would swallow #2554's decimal phase ids + * ("00.1" is a real phase, not milestone 0). This scan asks a narrower + * question — "which phase ids does this milestone's window declare" — + * where only the 999 icebox range is excluded. + * - `src/state.cts` `cmdStateValidate` ("Gate 1: Validate STATE.md against + * filesystem"): resolves ONE directory — the disk match for STATE.md's + * own `Current Phase` field — by prefix, a single-phase LOOKUP, not an + * enumeration of the current milestone's phase set. + * - `src/state.cts` `cmdStateSync` ("Gate 2: Sync STATE.md from filesystem + * ground truth"): a ground-truth RECONCILIATION pass, same family as + * `collectDiskPhases` below — it deliberately scans every phase + * directory on disk (minus #1514 retired-phase exclusion) so STATE.md's + * rewritten counters reflect the true disk state, not a re-derivation of + * "which phases belong to the current milestone" the way its sibling + * `buildStateFrontmatter` computes (that one IS routed through the + * owner, scoped to the stored milestone, because it exists specifically + * to answer the milestone-scoped question at STATE.md construction + * time). + * - `src/state.cts` `cmdStateRebuild`: its nested `phaseInventoryProvider` + * deliberately does NOT route through `listMilestonePhaseDirs`. `state + * rebuild` is a RECONCILIATION pass against ground truth — it must see + * every phase directory on disk so an orphan STATE.md row for a phase + * that no longer exists (or sits outside the current milestone window) is + * dropped. Scoping this enumeration would make the rebuild silently + * preserve stale rows instead of dropping them, and a non-`readdirSync`- + * shaped owner call also cannot propagate the original fault message the + * #3057 B1 contract requires to surface verbatim. + * - `src/phase.cts` `cmdPhaseNextDecimal`: computes the next free decimal + * sub-phase id (e.g. `2.3`) by scanning EVERY on-disk directory and the + * WHOLE ROADMAP (not milestone-scoped) for existing `2.N` ids — an id + * collision it must avoid can come from any milestone, so it needs the + * physical set, matching the CREATE-adjacent exemption category. + * - `src/phase.cts` `cmdPhasePlanIndex`: resolves ONE caller-supplied + * `phase` id to its directory — a single-phase LOCATION lookup, not an + * enumeration of the current milestone's phase set. + * - `src/phase.cts` `cmdPhaseInsert`: the same next-free-decimal-id scan as + * `cmdPhaseNextDecimal` (id collisions can come from any milestone), + * immediately followed by creating the new phase directory — a CREATE + * operation, physical set by definition. + * - `src/phase.cts` `renameDecimalPhases`, `renameIntegerPhases`: RENAME + * mutations. Each `readdirSync` targets a SINGLE just-renamed phase + * directory's own FILES (`phasesDir/newDirName`) to rename the files + * inside it to match — not an enumeration of the phases directory at + * all; only shaped like one because `phasesDir` is a substring of the + * joined path. + * - `src/audit.cts` `scanUatGaps`, `scanVerificationGaps`, + * `scanContextQuestions`, `scanDeferredItems`: the pre-milestone-close + * audit gate (`gsd-tools.cjs audit-open`, called by `/gsd:complete- + * milestone`'s pre-close gate). Each deliberately SWEEPS EVERY phase + * directory on disk to report open UAT/VERIFICATION/CONTEXT/deferred-item + * gaps — the audit's whole purpose is catching stragglers before a + * milestone closes, so scoping it to the current milestone's window + * would hide exactly the drift (e.g. a still-open item in a phase that + * somehow fell outside the window) it exists to surface. + * - `src/roadmap-upgrade.cts` `computeMigrationPlan`: a legacy-id-to- + * milestone-prefixed-id MIGRATION. It must see and rename EVERY existing + * phase directory across every milestone in one pass (a legacy phase + * number can legitimately collide across milestones — that ambiguity is + * exactly what the migration resolves) — the physical set by definition. + * - `src/smart-entry.cts` `detectVerifyFailed`: resolves ONE phase — the + * current phase from STATE.md, falling back to the highest-numbered + * directory when STATE.md has none — to check its own verify/UAT + * artifacts. A single-phase LOOKUP (with an explicit fallback rule of + * its own), not a current-milestone enumeration. + * - `src/commands.cts` `cmdHistoryDigest`: explicitly builds `allPhaseDirs` + * as archived-milestone dirs (via `getArchivedPhaseDirs`) PLUS every + * live phase directory, to produce a project-wide historical digest + * spanning every milestone ever shipped — the union is a strict + * superset of any one milestone's window by design; scoping the live + * half would silently drop history the digest exists to preserve. + * + * The tree-walk / root-confinement / regex-literal-tokenizer / sanitizer + * machinery is SHARED with the sibling drift guards via + * `scripts/lib/drift-scan.cjs` (ADR-3180 Decision 4) — see that module for + * the `isInsideRoot` case-sensitivity note, the `walk` symlink-confinement + * rationale, and the `readRegexLiteralAt` ReDoS-avoidance rationale. + * `readStringLiteralAt` below is the same style, written locally for + * quoted/backticked strings, mirroring `lint-milestone-window-drift.cjs`'s + * own local copy (not shared — each guard's literal-bearing shape differs). + * + * KNOWN, ACCEPTED limits of a per-line textual scan (same tradeoff the + * sibling drift guards document): a re-derivation whose detector tokens are + * split across two DIFFERENT lines with no single line carrying both is not + * caught by this narrow shape. That is left to code review and the design's + * identity tests, not this regex. + */ + +const path = require('node:path'); +const driftScan = require('./lib/drift-scan.cjs'); +const { readRegexLiteralAt, MAX_REGEX_LITERAL_LEN, sanitizeForReport, scanTree } = driftScan; + +// (1a) The enumeration primitive itself. +const READDIR_SYNC_RE = /readdirSync/; + +// (1b1) The phases-directory identifier every routed call site used to bind +// its `readdirSync` target to. +const PHASES_DIR_ID_RE = /\bphasesDir\b/; + +// (1b2) A quoted/backticked `'phases'` string — the other shape a phases-dir +// path segment takes at a call site that builds the path inline instead of +// through a `phasesDir` local. +const PHASES_STRING_RE = /['"`]phases['"`]/; + +// (2b) A numeric comparison against the sentinel value, or a bare reference +// to the owner's exported range constant used outside the owner. The digit +// run is anchored on both sides (`\b`, and no digit can precede it inside +// the `\s*` gap immediately after the operator) so `=== 9990` / `=== 19999` +// never fire — only the standalone value `999` does. +const SENTINEL_COMPARISON_RE = /(?:===|==|!==)\s*999\b|\bSENTINEL_RANGES\b/; + +// A standalone `999` inside an already-located literal's text: no digit +// immediately before or after, so `1999`/`9990`/`19999` inside a string or +// regex literal never fire — only the literal spelling of the reserved +// sentinel value does. +const STANDALONE_999_RE = /(? ({ file: rel, ...d })); + }, + }); +} + +function main() { + const root = path.join(__dirname, '..'); + const violations = scanRepo(root); + if (violations.length === 0) { + process.stdout.write('ok phase-enumeration-drift: no unsanctioned phase-enumeration re-derivations outside phase-locator.cts / phase-id.cts\n'); + return; + } + process.stderr.write('phase-enumeration-drift: independent re-derivation(s) of phase enumeration found.\n'); + process.stderr.write('Use src/phase-locator.cjs `listMilestonePhaseDirs` instead of re-deriving a phases-directory\n'); + process.stderr.write('readdirSync, and src/phase-id.cjs `isSentinelPhaseId` instead of re-deriving a `999` sentinel test:\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 = { + findPhaseEnumerationDrift, + scanRepo, + READDIR_SYNC_RE, + PHASES_DIR_ID_RE, + PHASES_STRING_RE, + SENTINEL_COMPARISON_RE, + STANDALONE_999_RE, + OWNER_FILES, + FUNCTION_SCOPED_EXEMPTIONS, + readStringLiteralAt, + hasSentinelLiteral, + extractFragment, + stripComments, +}; diff --git a/scripts/lint-plan-count-drift.cjs b/scripts/lint-plan-count-drift.cjs index 10fc5ff10..7b53d10f5 100644 --- a/scripts/lint-plan-count-drift.cjs +++ b/scripts/lint-plan-count-drift.cjs @@ -198,6 +198,32 @@ function findRegexLiteralMdMatch(line) { return null; } +/** + * Strip comment text from a line before detection. A guard that fires on a + * COMMENT — including a comment documenting that the code below uses the + * canonical owner, or prose quoting this guard's own detector shapes — reports + * prose as drift and trains readers to add exemptions for documentation. + * Handles the three shapes that appear in this codebase: a whole-line + * block-comment continuation (`*` or `/*` leading), a `//` line comment, and + * a trailing `//` after code. Mirrors `lint-phase-enumeration-drift.cjs`'s + * own copy (not shared — each guard applies it at a slightly different point + * in its detection pipeline). + * + * Deliberately simple and conservative: it does not attempt full block-comment + * state tracking across lines (this is a per-line scan, same tradeoff the + * sibling guards document). A `//` inside a string literal would be stripped + * early — accepted, because the effect is to UNDER-report on a pathological + * line, never to over-report prose as drift. + */ +function stripComments(line) { + const trimmed = line.trim(); + // Whole-line block comment or JSDoc continuation. + if (trimmed.startsWith('*') || trimmed.startsWith('/*') || trimmed.startsWith('//')) return ''; + // Trailing line comment after code. + const idx = line.indexOf('//'); + return idx === -1 ? line : line.slice(0, idx); +} + /** * 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 @@ -214,9 +240,12 @@ function findPlanCountDrift(text, relPath) { const fnMatch = TOP_LEVEL_FUNCTION_RE.exec(line); if (fnMatch) currentFunction = fnMatch[1]; - if (!FILENAME_TEST_RE.test(line)) continue; - const quoted = PLAN_SUMMARY_LITERAL_RE.exec(line); - const found = quoted ? quoted[0] : findRegexLiteralMdMatch(line); + const code = stripComments(line); + if (!code.trim()) continue; + + if (!FILENAME_TEST_RE.test(code)) continue; + const quoted = PLAN_SUMMARY_LITERAL_RE.exec(code); + const found = quoted ? quoted[0] : findRegexLiteralMdMatch(code); if (!found) continue; if (exemptFunctions && exemptFunctions.has(currentFunction)) continue; @@ -279,4 +308,5 @@ module.exports = { MAX_REGEX_LITERAL_LEN, isInsideRoot, sanitizeForReport, + stripComments, }; diff --git a/src/commands.cts b/src/commands.cts index 2cde54eb1..83b275f49 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -21,13 +21,13 @@ import coreUtilsMod = require('./core-utils.cjs'); const { toPosixPath, generateSlugInternal, extractOneLinerFromBody } = coreUtilsMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, comparePhaseNum, extractPhaseToken, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; +const { normalizePhaseName, comparePhaseNum, extractPhaseToken, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); -const { getArchivedPhaseDirs, findPhaseInternal } = phaseLocatorMod; +const { getArchivedPhaseDirs, findPhaseInternal, listMilestonePhaseDirs } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); -const { extractCurrentMilestone, stripShippedMilestones: _stripShippedMilestones, getMilestoneInfo, getMilestonePhaseFilter, getRoadmapPhaseInternal } = roadmapParserMod; +const { extractCurrentMilestone, stripShippedMilestones: _stripShippedMilestones, getMilestoneInfo, getRoadmapPhaseInternal } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import modelResolverMod = require('./model-resolver.cjs'); const { resolveModelInternal, resolveModelForTier, resolveProviderEscalation, resolveEffortInternal, resolveFastModeInternal, resolveEffortForTier, resolveGranularityInternal, assertValidGranularityOverride } = modelResolverMod; @@ -1565,10 +1565,16 @@ function cmdProgressRender(cwd: string, format: string | undefined, raw: boolean const phases: PhaseProgress[] = []; let totalPlans = 0; let totalSummaries = 0; + let phaseScope: string | null = null; try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort((a, b) => comparePhaseNum(a, b)); + // #3185 (ADR-3180 Decision 1): the single owner applies the milestone + // window AND the sentinel filter and returns dirs already sorted by + // comparePhaseNum. This command previously read the phases directory + // directly with neither, which is why `query progress` listed 999.* + // backlog directories as current-milestone phases (#3167). + const { value: dirs, scope } = listMilestonePhaseDirs(phasesDir, { cwd }); + phaseScope = scope; for (const dir of dirs) { const dm = dir.match(/^(\d+(?:\.\d+)*)-?(.*)/); @@ -1619,6 +1625,9 @@ function cmdProgressRender(cwd: string, format: string | undefined, raw: boolean total_plans: totalPlans, total_summaries: totalSummaries, percent, + // #3185 (ADR-3180 Decision 2): the enumeration's scope, so a consumer + // can tell a genuinely-empty milestone from one it could not scope. + phase_scope: phaseScope, }, raw, undefined); } } @@ -1856,7 +1865,6 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { const reqPath = planningPaths(cwd).requirements; const statePath = planningPaths(cwd).state; const milestone = getMilestoneInfo(cwd); - const isDirInMilestone = getMilestonePhaseFilter(cwd) as (dir: string) => boolean; // Phase & plan stats (reuse progress pattern) const phasesByNumber = new Map(); let totalPlans = 0; let totalSummaries = 0; + let phaseScope: string | null = null; try { const roadmapRaw = platformReadSync(roadmapPath); @@ -1879,6 +1888,12 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { const headingPattern = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:\s*([^\n]+)/gi; let match: RegExpExecArray | null; while ((match = headingPattern.exec(roadmapContent)) !== null) { + // #3185: the heading seed carried no sentinel filter, so a + // `### Phase 999.1:` backlog heading produced a stats row even with no + // directory on disk. Uses the canonical predicate (phase-id.cts), not a + // local literal — the rule had five copies and three regex variants + // before this phase, disagreeing about Phase 0. + if (isSentinelPhaseId(match[1])) continue; const key = normalizePhaseName(match[1]); phasesByNumber.set(key, { number: key, @@ -1891,12 +1906,13 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { } catch { /* intentionally empty */ } try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries - .filter(e => e.isDirectory()) - .map(e => e.name) - .filter(isDirInMilestone) - .sort((a, b) => comparePhaseNum(a, b)); + // #3185 (ADR-3180 Decision 1): route through the single owner. This + // previously applied the milestone window but NOT a directory-level + // sentinel filter — and getMilestonePhaseFilter degrades to a pass-all + // predicate when its heading set is empty, at which point every directory + // on disk passed, backlog included (#3167). + const { value: dirs, scope } = listMilestonePhaseDirs(phasesDir, { cwd }); + phaseScope = scope; for (const dir of dirs) { // Use extractPhaseToken to correctly parse M-NN-style and code-prefixed dir names. @@ -1991,6 +2007,9 @@ function cmdStats(cwd: string, format: string | undefined, raw: boolean): void { git_commits: gitCommits, git_first_commit_date: gitFirstCommitDate, last_activity: lastActivity, + // #3185 (ADR-3180 Decision 2): the enumeration's scope, so a consumer + // can tell a genuinely-empty milestone from one it could not scope. + phase_scope: phaseScope, }; if (format === 'table') { diff --git a/src/init.cts b/src/init.cts index bd3f45fe0..e449d05c7 100644 --- a/src/init.cts +++ b/src/init.cts @@ -79,16 +79,15 @@ const { const { output, error } = io; const { loadConfig, loadConfigResolved } = configLoader; const { resolveModelInternal, resolveGranularityInternal, assertValidGranularityOverride } = modelResolver; -const { findPhaseInternal } = phaseLocator; +const { findPhaseInternal, listMilestonePhaseDirs } = phaseLocator; const { getRoadmapPhaseInternal, getMilestoneInfo, - getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { escapeRegex, normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery } = phaseId; +const { escapeRegex, normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery, isSentinelPhaseId } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -1212,19 +1211,11 @@ function cmdInitNewMilestone(cwd: string, raw: boolean, options: Record; const latestCompleted = getLatestCompletedMilestone(cwd); const phasesDir = path.join(planningDir(cwd), 'phases'); - let phaseDirCount = 0; - - try { - if (fs.existsSync(phasesDir)) { - const isDirInMilestone = getMilestonePhaseFilter(cwd); - phaseDirCount = fs - .readdirSync(phasesDir, { withFileTypes: true }) - .filter((entry) => entry.isDirectory() && isDirInMilestone(entry.name)) - .length; - } - } catch { - /* intentionally empty */ - } + // #3185 (ADR-3180 Decision 1): "how many phase directories belong to the + // CURRENT milestone" is exactly the scoped question listMilestonePhaseDirs + // owns — routed through it instead of a local readdirSync + hand-rolled + // window filter (which also never excluded sentinels, unlike the owner). + const phaseDirCount = listMilestonePhaseDirs(phasesDir, { cwd }).value.length; const wf = (config.workflow ?? {}) as Record; @@ -2012,7 +2003,8 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:`, 'gi'); let m: RegExpExecArray | null; while ((m = phasePattern.exec(currentSection)) !== null) { - if (/^999(?:\.|$)/.test(m[1])) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(m[1])) continue; roadmapPhaseNumbers.push(m[1]); } } catch { @@ -2050,8 +2042,12 @@ function cmdInitMilestoneOp(cwd: string, raw: boolean): void { } } else { try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries.filter((e) => e.isDirectory()).map((e) => e.name); + // #3185 (ADR-3180 Decision 1): the ROADMAP heading scan above found no + // current-milestone phase headings — fall back to asking the canonical + // owner "which phase directories belong to the current milestone" + // directly, instead of a hand-rolled readdirSync over every directory + // on disk (which also never excluded sentinels, unlike the owner). + const dirs = listMilestonePhaseDirs(phasesDir, { cwd }).value; phaseCount = dirs.length; for (const dir of dirs) { try { @@ -2152,18 +2148,13 @@ function cmdInitManager(cwd: string, raw: boolean): void { const rawContent = fs.readFileSync(paths.roadmap, 'utf-8'); const content = extractCurrentMilestone(rawContent, cwd); const phasesDir = paths.phases; - const isDirInMilestone = getMilestonePhaseFilter(cwd); - const _phaseDirEntries = (() => { - try { - return fs - .readdirSync(phasesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory()) - .map((e) => e.name); - } catch { - return []; - } - })(); + // #3185 (ADR-3180 Decision 1): "which phase directories belong to the + // CURRENT milestone" is the scoped question listMilestonePhaseDirs owns — + // routed through it instead of a hand-rolled readdirSync + a separate + // getMilestonePhaseFilter window check (which also never excluded + // sentinels, unlike the owner). + const _phaseDirEntries = listMilestonePhaseDirs(phasesDir, { cwd }).value; const _checkboxStates = new Map(); const _cbPattern = new RegExp(`-\\s*\\[(x| )\\]\\s*.*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})[:\\s]`, 'gi'); @@ -2213,8 +2204,7 @@ function cmdInitManager(cwd: string, raw: boolean): void { ); try { - const dirs = _phaseDirEntries.filter(isDirInMilestone); - const dirMatch = dirs.find((d) => phaseTokenMatches(d, normalized)); + const dirMatch = _phaseDirEntries.find((d) => phaseTokenMatches(d, normalized)); if (dirMatch) { const fullDir = path.join(phasesDir, dirMatch); @@ -2387,7 +2377,8 @@ function cmdInitManager(cwd: string, raw: boolean): void { const recommendedActions: Record[] = []; for (const phase of phases) { if (phase['disk_status'] === 'complete') continue; - if (/^999(?:\.|$)/.test(phase['number'] as string)) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(phase['number'])) continue; if (phase['disk_status'] === 'executed') { recommendedActions.push({ @@ -2455,7 +2446,8 @@ function cmdInitManager(cwd: string, raw: boolean): void { return true; }); - const nonBacklogPhases = phases.filter((p) => !/^999(?:\.|$)/.test(p['number'] as string)); + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + const nonBacklogPhases = phases.filter((p) => !isSentinelPhaseId(p['number'] as string)); const completedCount = nonBacklogPhases.filter((p) => p['phase_complete'] === true).length; const sanitizeFlags = (rawVal: unknown): string => { @@ -2806,21 +2798,16 @@ function cmdInitProgress(cwd: string, raw: boolean, options: Record(); try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries - .filter((e) => e.isDirectory()) - .map((e) => e.name) - .filter(isDirInMilestone) - .sort((a, b) => { - const pa = a.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - const pb = b.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})`, 'i')); - if (!pa || !pb) return a.localeCompare(b); - return parseInt(pa[1], 10) - parseInt(pb[1], 10); - }); + // #3185 (ADR-3180 Decision 1): "which phase directories belong to the + // CURRENT milestone" — routed through the canonical owner instead of a + // hand-rolled readdirSync + isDirInMilestone filter + local sort (which + // also never excluded sentinels, unlike the owner; the final `phases` + // array is re-sorted below anyway, so dropping the local sort here is + // behavior-preserving). + const dirs = listMilestonePhaseDirs(phasesDir, { cwd }).value; for (const dir of dirs) { const dirMatch = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i')); diff --git a/src/milestone.cts b/src/milestone.cts index df8e5582d..c64aa9ad0 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -26,7 +26,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, phaseTokenMatches, PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; +const { escapeRegex, normalizePhaseName, phaseTokenMatches, PHASE_NUMBER_TOKEN_SOURCE, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); const { @@ -44,6 +44,9 @@ const { extractOneLinerFromBody, countMatchedSummaries } = coreUtilsMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- plan-scan.cjs is an export= CommonJS module import planScanMod = require('./plan-scan.cjs'); const { scanPhasePlans } = planScanMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-locator.cjs is an export= CommonJS module +import phaseLocatorMod = require('./phase-locator.cjs'); +const { listMilestonePhaseDirs } = phaseLocatorMod; const { planningPaths } = planningWorkspace; const { extractFrontmatter } = frontmatterMod; const { writeStateMd } = stateMod; @@ -653,8 +656,8 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo // milestone completion. Mirrors the engine-wide sentinel convention // (phase-id getMilestoneFromPhaseId, roadmap-command-router SENTINELS, // the #1445 /^999/ progress filters). (#1580) - const major = parseInt(phaseNum, 10); - if (major === 0 || major === 999) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this local check already covered both 0 and 999; now delegates to the single canonical owner. + if (isSentinelPhaseId(phaseNum)) continue; const normalized = normalizePhaseName(phaseNum); // A phase has disk_status: 'no_directory' when no phase directory // with a matching token exists on disk. Use the same phaseTokenMatches @@ -686,15 +689,14 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo const accomplishments: string[] = []; try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries - .filter((e) => e.isDirectory()) - .map((e) => e.name) - .sort(); + // #3185 (ADR-3180 Decision 1): "which phase directories belong to the + // CURRENT milestone" — routed through the canonical owner (with the + // explicit `version` this command already resolved) instead of a + // hand-rolled readdirSync + isDirInMilestone filter, which also never + // excluded sentinels, unlike the owner. + const dirs = listMilestonePhaseDirs(phasesDir, { cwd, versionOverride: version }).value; for (const dir of dirs) { - if (!isDirInMilestone(dir)) continue; - phaseCount++; // #3183: canonical plan/summary sets (root+nested, superseded-excluded) // from the single owner, rather than a root-only hand-rolled readdirSync @@ -745,14 +747,10 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo if (options.dryRun) { const phaseDirsToArchive: string[] = []; if (options.archivePhases !== false) { - try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - for (const e of entries) { - if (e.isDirectory() && isDirInMilestone(e.name)) { - phaseDirsToArchive.push(e.name); - } - } - } catch { /* phasesDir missing — nothing to archive */ } + // #3185 (ADR-3180 Decision 1): same routed derivation as the stats loop + // above — the dry-run preview must list exactly what the real archive + // pass below would move. + phaseDirsToArchive.push(...listMilestonePhaseDirs(phasesDir, { cwd, versionOverride: version }).value); } const dryRunResult = { dry_run: true, @@ -876,10 +874,12 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo const phaseArchiveDir = path.join(archiveDir, `${version}-phases`); platformEnsureDir(phaseArchiveDir); - const phaseEntries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const phaseDirNames = phaseEntries.filter((e) => e.isDirectory()).map((e) => e.name); + // #3185 (ADR-3180 Decision 1): same routed derivation as the stats + // loop above — only the CURRENT milestone's phase directories move, + // never a sentinel or an out-of-window directory left for a later + // milestone. + const phaseDirNames = listMilestonePhaseDirs(phasesDir, { cwd, versionOverride: version }).value; for (const dir of phaseDirNames) { - if (!isDirInMilestone(dir)) continue; retryRenameSync(path.join(phasesDir, dir), path.join(phaseArchiveDir, dir)); archivedCount++; } @@ -949,7 +949,13 @@ function cmdPhasesClear(cwd: string, raw: boolean, args: string[]): void { if (fs.existsSync(phasesDir)) { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries.filter((e) => e.isDirectory() && !/^999(?:\.|$)/.test(e.name)); + // #3185 (ADR-3180 Decision 1): this carried the FIFTH copy of the + // sentinel rule and its THIRD regex variant — `/^999(?:\.|$)/` — which + // excluded 999 but NOT 0. Because this is the DESTRUCTIVE path, that + // divergence meant a `0-*` directory `roadmap analyze` preserves as a + // sentinel was DELETED here. Routed through the canonical predicate so + // every reader of "is this a sentinel phase" agrees by construction. + const dirs = entries.filter((e) => e.isDirectory() && !isSentinelPhaseId(e.name)); if (dirs.length > 0 && !confirm) { error( diff --git a/src/phase-lifecycle.cts b/src/phase-lifecycle.cts index e0f4177f5..0466a4c35 100644 --- a/src/phase-lifecycle.cts +++ b/src/phase-lifecycle.cts @@ -21,6 +21,9 @@ */ import { findTableWithColumns } from './markdown-table.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-id.cjs is an export= CommonJS module +import phaseIdMod = require('./phase-id.cjs'); +const { isSentinelPhaseId } = phaseIdMod; /** Result of deriveProgressFromRoadmap. */ export interface RoadmapProgress { @@ -90,10 +93,11 @@ export function deriveProgressFromRoadmap(roadmapContent: string): RoadmapProgre const completed = allRows.filter((r) => /^complete$/i.test((r['Status'] ?? '').trim())).length; completedPhases = completed > 0 ? completed : null; - // Data rows only (exclude 999.x backlog phases). Mirrors init.cts /^999(?:\.|$)/ filter. + // Data rows only (exclude sentinel phases 0 and 999.x). + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. const dataRows = allRows.filter((r) => { const phase = (r['Phase'] ?? '').trim(); - return /^\d/.test(phase) && !/^999\b/.test(phase); + return /^\d/.test(phase) && !isSentinelPhaseId(phase); }); totalPhases = dataRows.length > 0 ? dataRows.length : null; diff --git a/src/phase-locator.cts b/src/phase-locator.cts index 92f4b0d47..826db6974 100644 --- a/src/phase-locator.cts +++ b/src/phase-locator.cts @@ -20,7 +20,7 @@ import fs from 'node:fs'; import path from 'node:path'; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdModule = require('./phase-id.cjs'); -const { normalizePhaseName, phaseTokenMatches, extractPhaseToken } = phaseIdModule; +const { normalizePhaseName, phaseTokenMatches, extractPhaseToken, isSentinelPhaseId, comparePhaseNum } = phaseIdModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import coreUtilsModule = require('./core-utils.cjs'); const { readSubdirectories, getPhaseFileStats, extractCanonicalPlanId, toPosixPath, findUnsummarizedPlans } = coreUtilsModule; @@ -33,6 +33,13 @@ const { extractFrontmatter } = frontmatterModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import planDependencyGraphModule = require('./plan-dependency-graph.cjs'); const { computeHaltPropagation, buildSummaryFileIndex, isSummaryFileHalted } = planDependencyGraphModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import roadmapParserModule = require('./roadmap-parser.cjs'); +const { getMilestonePhaseFilter } = roadmapParserModule; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planningScopeMod = require('./planning-scope.cjs'); +const { SCOPE } = planningScopeMod; +type Scope = planningScopeMod.Scope; // ─── Phase search types ─────────────────────────────────────────────────────── @@ -287,6 +294,86 @@ function findPhaseInternal(cwd: string, phase: unknown): PhaseSearchResult | nul return null; } +/** + * #3185 (epic #3180 Phase 3, ADR-3180 Decision 1 row "Phase enumeration"): + * the SINGLE canonical owner of "which phase directories belong to the current + * milestone". Applies the milestone window AND the sentinel filter, in that + * order, and returns the surviving directory names. + * + * Before this existed the derivation had four independent implementations and + * only `cmdRoadmapAnalyze` carried both halves; `cmdProgressRender`, + * `cmdStats` and `cmdPhasesList` each carried neither or one. + * + * TWO THINGS THIS GETS RIGHT THAT A HEADING-SIDE FILTER CANNOT: + * + * 1. The sentinel test runs against DIRECTORY NAMES and is UNCONDITIONAL. + * `getMilestonePhaseFilter` excludes sentinels from its ROADMAP HEADING + * set, but when that set is empty it degrades to a literal `() => true` + * pass-all predicate and never consults the heading set at all — so its + * own sentinel exclusion becomes unreachable exactly when it is needed, + * and every directory on disk (backlog included) is reported as a + * current-milestone phase. That degrade is the #3167 symptom path. + * + * 2. The sentinel predicate is the canonical `isSentinelPhaseId` + * (`src/phase-id.cts`, SENTINEL_RANGES [0, 999]), not a local literal. + * The rule had five copies and three different regexes before this phase, + * and they disagreed about Phase 0. + * + * The pass-all degrade is narrowed MINIMALLY: it stays over-inclusive for + * non-sentinel directories, so a project whose window declares no phases + * still sees its real phase directories. Only sentinels are refused. + * + * `scope` distinguishes a REAL empty from a NON-answer (ADR-3180 Decision 2): + * an absent `phasesDir` is a real empty (a new project genuinely has no + * phases) and inherits the window's scope, whereas a `phasesDir` that exists + * but cannot be read is UNREADABLE. + */ +function listMilestonePhaseDirs( + phasesDir: string, + opts: { + cwd?: string; + ws?: string | null; + versionOverride?: string | null; + phaseIdConvention?: string | null; + } = {}, +): { value: string[]; scope: Scope } { + const { cwd, ws = null, versionOverride = null, phaseIdConvention = null } = opts; + + // Without a cwd there is nothing to scope AGAINST — the caller asked for an + // unscoped read, which is a real answer (mirrors extractCurrentMilestoneScoped's + // row 1). Sentinels are still refused: they are never milestone phases. + let inWindow: (dirName: string) => boolean = () => true; + let scope: Scope = SCOPE.COMPLETE; + if (cwd) { + const filter = getMilestonePhaseFilter(cwd, versionOverride, phaseIdConvention, ws); + inWindow = filter; + scope = filter.scope; + } + + // An ABSENT phases dir is a real empty, not a failure: a freshly-created + // project genuinely has no phase directories yet. Distinguishing this from + // the unreadable case below is the whole point of the scope discriminator. + if (!fs.existsSync(phasesDir)) return { value: [], scope }; + + let names: string[]; + try { + names = fs.readdirSync(phasesDir, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name); + } catch { + // The directory EXISTS but could not be read (EACCES/EIO). An empty list + // here is a NON-answer and must not be reported as "this milestone has no + // phases" — that collapse is the defect class this epic removes. + return { value: [], scope: SCOPE.UNREADABLE }; + } + + const value = names + .filter((name) => inWindow(name) && !isSentinelPhaseId(name, phaseIdConvention ?? undefined)) + .sort((a, b) => comparePhaseNum(a, b)); + + return { value, scope }; +} + function getArchivedPhaseDirs(cwd: string): ArchivedPhaseDir[] { // #2855: same workstream-scoped resolution as findPhaseInternal above, via // the shared listArchiveVersionDirs helper. `phase.list --include-archived` @@ -314,4 +401,5 @@ export = { searchPhaseInDir, findPhaseInternal, getArchivedPhaseDirs, + listMilestonePhaseDirs, }; diff --git a/src/phase.cts b/src/phase.cts index ed2cf23fd..eb227e552 100644 --- a/src/phase.cts +++ b/src/phase.cts @@ -42,10 +42,10 @@ const { } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- phase-locator.cjs is an export= CommonJS module import phaseLocatorMod = require('./phase-locator.cjs'); -const { findPhaseInternal, getArchivedPhaseDirs } = phaseLocatorMod; +const { findPhaseInternal, getArchivedPhaseDirs, listMilestonePhaseDirs } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- roadmap-parser.cjs is an export= CommonJS module import roadmapParserMod = require('./roadmap-parser.cjs'); -const { stripShippedMilestones, extractCurrentMilestone, getMilestonePhaseFilter, currentMilestoneRawRanges, withPhaseSection } = roadmapParserMod; +const { stripShippedMilestones, extractCurrentMilestone, currentMilestoneRawRanges, withPhaseSection } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module import planningWorkspace = require('./planning-workspace.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module @@ -209,26 +209,51 @@ function cmdPhasesList(cwd: string, options: PhaseListOptions, raw: boolean): vo } try { - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - let dirs: string[] = entries.filter((e) => e.isDirectory()).map((e) => e.name); - - if (includeArchived) { - const archived = getArchivedPhaseDirs(cwd); - for (const a of archived) { - dirs.push(`${a.name} [${a.milestone}]`); - } - } - - dirs.sort((a, b) => comparePhaseNum(a, b)); + // #3185 (ADR-3180 Decision 1): only the ENUMERATION routes through the + // single owner. The two other modes below ask genuinely DIFFERENT + // questions and are exempt by documented reason, never by a file + // allowlist (ADR-3180 Decision 4a): + // + // --phase locating ONE phase by token is phase LOCATION, a + // question src/phase-locator.cts already owns via + // findPhaseInternal/searchPhaseInDir. Scoping it to + // the current milestone would make an out-of-window + // phase report "Phase not found". + // --include-archived archived directories are BY DEFINITION from other + // milestones; filtering them through the CURRENT + // milestone window would return nothing at all. + // + // Generalizing #3183's rule ("a diagnostic about file NAMING wants the + // physical set; only a question about outstanding WORK wants the live + // set"): a LOOKUP wants the physical set; only "which phases belong to + // this milestone" wants the scoped set. + const archivedLabels: string[] = includeArchived + ? getArchivedPhaseDirs(cwd).map((a) => `${a.name} [${a.milestone}]`) + : []; + let dirs: string[]; + // #3185 (ADR-3180 Decision 2): the enumeration's scope, so a consumer + // can tell a genuinely-empty milestone from one it could not scope. Only + // the ENUMERATION path scopes anything; the LOOKUP path below has no + // enumeration to report a scope for. + let phaseScope: string | null = null; if (phase) { + // LOOKUP (b): search the physical set, plus archived when asked. + const lookupPool = [...readSubdirectories(phasesDir, true), ...archivedLabels]; const normalized = normalizePhaseName(phase); - const match = dirs.find((d) => phaseTokenMatches(d, normalized)); + const match = lookupPool.find((d) => phaseTokenMatches(d, normalized)); if (!match) { output({ files: [], count: 0, phase_dir: null, error: 'Phase not found' }, raw, ''); return; } dirs = [match]; + } else { + // ENUMERATION (a): milestone-scoped and sentinel-filtered, plus + // archived when asked (c). + const enumerated = listMilestonePhaseDirs(phasesDir, { cwd }); + phaseScope = enumerated.scope; + dirs = [...enumerated.value, ...archivedLabels]; + dirs.sort((a, b) => comparePhaseNum(a, b)); } if (type) { @@ -274,13 +299,18 @@ function cmdPhasesList(cwd: string, options: PhaseListOptions, raw: boolean): vo files, count: files.length, phase_dir: phase ? dirs[0].replace(/^\d+(?:\.\d+)*-?/, '') : null, + // #3185 (ADR-3180 Decision 2): the enumeration's scope, so a consumer + // can tell a genuinely-empty milestone from one it could not scope. + phase_scope: phaseScope, }; if (warnings.length) result['warning'] = warnings.join(' | '); output(result, raw, files.join('\n')); return; } - output({ directories: dirs, count: dirs.length }, raw, dirs.join('\n')); + // #3185 (ADR-3180 Decision 2): the enumeration's scope, so a consumer + // can tell a genuinely-empty milestone from one it could not scope. + output({ directories: dirs, count: dirs.length, phase_scope: phaseScope }, raw, dirs.join('\n')); } catch (e) { const msg = e instanceof Error ? e.message : String(e); error('Failed to list phases: ' + msg); @@ -979,11 +1009,13 @@ function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?: while ((m = headerPattern.exec(content)) !== null) { const num = parseInt(m[1], 10); - if (num !== 999) usedPhaseNums.add(num); + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!isSentinelPhaseId(num)) usedPhaseNums.add(num); } while ((m = bulletPattern.exec(content)) !== null) { const num = parseInt(m[1], 10); - if (num !== 999) usedPhaseNums.add(num); + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!isSentinelPhaseId(num)) usedPhaseNums.add(num); } // 3) On-disk phase directories (e.g. phases/11-foo/ with no header yet) @@ -994,7 +1026,8 @@ function cmdPhaseAdd(cwd: string, description: string, raw: boolean, customId?: const match = entry.match(dirNumPattern); if (!match) continue; const num = parseInt(match[1], 10); - if (num !== 999) usedPhaseNums.add(num); + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!isSentinelPhaseId(num)) usedPhaseNums.add(num); } } @@ -1072,7 +1105,8 @@ function cmdPhaseAddBatch(cwd: string, descriptions: string[], raw: boolean): vo let m: RegExpExecArray | null; while ((m = phasePattern.exec(content)) !== null) { const num = parseInt(m[1], 10); - if (num === 999) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(num)) continue; if (num > maxPhase) maxPhase = num; } const phasesOnDisk = path.join(planningDir(cwd), 'phases'); @@ -1082,7 +1116,8 @@ function cmdPhaseAddBatch(cwd: string, descriptions: string[], raw: boolean): vo const match = entry.match(dirNumPattern); if (!match) continue; const num = parseInt(match[1], 10); - if (num === 999) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(num)) continue; if (num > maxPhase) maxPhase = num; } } @@ -1376,7 +1411,8 @@ function renameIntegerPhases( const m = dir.match(/^(\d+)([A-Z])?(?:\.(\d+))?-(.+)$/i); if (!m) return null; const dirInt = parseInt(m[1], 10); - return dirInt > removedInt && dirInt !== 999 + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + return dirInt > removedInt && !isSentinelPhaseId(dirInt) ? { dir, oldInt: dirInt, @@ -1418,7 +1454,8 @@ function renameIntegerPhases( function decrementRoadmapPhaseNumber(raw: string, removedInt: number): string { const num = parseInt(raw, 10); - if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!Number.isInteger(num) || num <= removedInt || isSentinelPhaseId(num)) return raw; return String(num - 1); } @@ -1426,13 +1463,15 @@ function decrementRoadmapPhaseToken(raw: string, removedInt: number): string { const match = String(raw).match(/^(\d+)(\.\d+)?$/); if (!match) return raw; const num = parseInt(match[1], 10); - if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!Number.isInteger(num) || num <= removedInt || isSentinelPhaseId(num)) return raw; return `${num - 1}${match[2] || ''}`; } function decrementRoadmapPaddedPhaseNumber(raw: string, removedInt: number): string { const num = parseInt(raw, 10); - if (!Number.isInteger(num) || num <= removedInt || num === 999) return raw; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!Number.isInteger(num) || num <= removedInt || isSentinelPhaseId(num)) return raw; return String(num - 1).padStart(raw.length, '0'); } @@ -1621,7 +1660,8 @@ function updateRoadmapAfterPhaseRemoval( const m = phaseCellShapeRe.exec(row['Phase'] ?? ''); if (!m) return false; const num = parseInt(m[1], 10); - if (!Number.isInteger(num) || num <= removedInt || num === 999) return false; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!Number.isInteger(num) || num <= removedInt || isSentinelPhaseId(num)) return false; processedOrdinalRows.add(index); matchedRowIndex = index; return true; @@ -2590,18 +2630,20 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { } try { - const isDirInMilestone = getMilestonePhaseFilter(cwd); - const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); - const dirs = entries - .filter((e) => e.isDirectory()) - .map((e) => e.name) - .filter(isDirInMilestone) - .sort((a, b) => comparePhaseNum(a, b)); + // #3185 (ADR-3180 Decision 1): "which phase directories belong to + // the CURRENT milestone" — routed through the canonical owner + // instead of a hand-rolled readdirSync + isDirInMilestone filter + // (which also never excluded sentinels on its own, unlike the + // owner; the per-directory isSentinelPhaseId check below stays as a + // defensive second check against the REGEX-EXTRACTED token, which + // is not necessarily identical to the raw directory name). + const dirs = listMilestonePhaseDirs(phasesDir, { cwd }).value; for (const dir of dirs) { const dm = dir.match(new RegExp(`^(${PHASE_NUMBER_TOKEN_SOURCE})-?(.*)`, 'i')); if (dm) { - if (/^999(?:\.|$)/.test(dm[1])) continue; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(dm[1])) continue; if (comparePhaseNum(dm[1], phaseNum) > 0) { nextPhaseNum = dm[1]; nextPhaseName = dm[2] || null; @@ -2645,9 +2687,8 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void { let pm: RegExpExecArray | null; while ((pm = phasePattern.exec(roadmapForPhases)) !== null) { // #2786: skip sentinel phase ids (999.x backlog, 0.x drafts) — stage 1 - // already skips 999 dirs on disk; stage 2's heading scan must not - // advance into backlog headings. Mirrors the /^999(?:\.|$)/ guard - // stage 1 uses at line 2536, but via isSentinelPhaseId for both ranges. + // already skips sentinel dirs on disk via isSentinelPhaseId (#3185); + // stage 2's heading scan must not advance into backlog headings either. if (isSentinelPhaseId(pm[1])) continue; if (comparePhaseNum(pm[1], phaseNum) > 0) { nextPhaseNum = pm[1]; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 13e2aa2cb..eb4d463ca 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -28,6 +28,8 @@ const { // #2121: roadmapPhaseLookupSources now lives in phase-id.cjs (single owner of // the lookup-source ordering); imported here rather than defined locally. roadmapPhaseLookupSources, + extractPhaseToken, + isSentinelPhaseId, } = phaseIdModule; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); @@ -643,7 +645,8 @@ function findRoadmapBulletPhaseInContent(content: string, phaseNum: unknown, pha function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { if (!phaseNum) return null; const normalizedPhase = stripProjectCodePrefix(phaseNum); - if (/^999(?:\.|$)/.test(normalizedPhase)) return null; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(normalizedPhase)) return null; // Resolved INSIDE the try for the same reason as getMilestoneInfo below: planningDir // throws a plain Error for an invalid GSD_WORKSTREAM/GSD_PROJECT segment, and resolving // it outside let that escape uncaught, crashing every caller for a malformed workstream @@ -958,7 +961,11 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p for (const h of tokenizeHeadings(roadmap)) { if (h.level < 2 || h.level > 4) continue; const pm = phaseHeadingPattern.exec(h.text); - // Exclude 999.x backlog phases from milestone phase set. Mirrors init.cts filter. + // #3185: deliberately NOT isSentinelPhaseId here. That predicate treats a + // leading 0 as sentinel milestone 0, which would swallow the #2554 decimal + // phase ids ("00.1" is a real phase, not milestone 0). This scan asks a + // narrower question -- "which phase ids does this milestone's window + // declare" -- where only the 999 icebox range is excluded. if (pm && !/^999\b/.test(pm[1])) milestonePhaseNums.add(pm[1]); } // #2199: also count bullet/checkbox phase entries (`- [ ] **Phase N — name**`) @@ -973,6 +980,11 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p const scanner = new RegExp(BULLET_PHASE_LINE_PATTERN.source, 'gim'); const roadmapUnfenced = stripFencedCode(roadmap).text; while ((bm = scanner.exec(roadmapUnfenced)) !== null) { + // #3185: deliberately NOT isSentinelPhaseId here. That predicate treats a + // leading 0 as sentinel milestone 0, which would swallow the #2554 decimal + // phase ids ("00.1" is a real phase, not milestone 0). This scan asks a + // narrower question -- "which phase ids does this milestone's window + // declare" -- where only the 999 icebox range is excluded. if (!/^999\b/.test(bm[1])) milestonePhaseNums.add(bm[1]); } } @@ -1039,6 +1051,20 @@ function getMilestonePhaseFilter(cwd: string, versionOverride?: string | null, p const sm = stripped.match(numericRe); if (sm && normalized.has(normalizePhaseIdSegments(sm[1]).toLowerCase())) return true; } + // #3185: last resort — ask the CANONICAL phase-id token extractor. The + // three attempts above are all leading-DIGIT or bare-alnum shapes, so none + // of them can match a #1324 letter-prefixed-DECIMAL directory + // (`P0.0-foundation`) against its own `### Phase P0.0:` heading: numericRe + // needs a leading digit, `customMatch` stops at the `.` and yields `P0`, + // and stripProjectCodePrefix needs a dash before the digit. The observable + // symptom was `stats` reporting such a phase with plans: 0 while its + // directory held plan files, because the heading seeded the row but the + // directory never folded in. extractPhaseToken is #2121's single owner of + // "what is this directory's phase token", so this defers to it rather than + // widening a fourth bespoke regex here. Additive: it can only ADMIT a + // directory, never exclude one the attempts above already matched. + const token = extractPhaseToken(dirName); + if (token && normalized.has(normalizePhaseIdSegments(String(token)).toLowerCase())) return true; return false; } (isDirInMilestone as MilestonePhaseFilter).phaseCount = milestonePhaseNums.size; diff --git a/src/roadmap.cts b/src/roadmap.cts index 263781928..75d8b6398 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -14,7 +14,7 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseTokenMatches, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources } = phaseIdMod; +const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseTokenMatches, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; @@ -215,7 +215,8 @@ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: s * phase resolution as `roadmap.get-phase` — not a milestone-only subset. */ function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | null { - if (/^999(?:\.|$)/.test(stripProjectCodePrefix(phaseNum))) return null; + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(stripProjectCodePrefix(phaseNum))) return null; const roadmapPath = planningPaths(cwd).roadmap; // Read directly rather than gating on fs.existsSync: existsSync returns false // on EACCES/EIO too, which would mask an UNREADABLE roadmap as "missing" and @@ -247,7 +248,8 @@ function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | nu // ─── cmdRoadmapGetPhase ─────────────────────────────────────────────────────── function cmdRoadmapGetPhase(cwd: string, phaseNum: string, raw: boolean): void { - if (/^999(?:\.|$)/.test(stripProjectCodePrefix(phaseNum))) { + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (isSentinelPhaseId(stripProjectCodePrefix(phaseNum))) { output({ found: false, phase_number: phaseNum }, raw, ''); return; } @@ -338,17 +340,21 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { }> = []; let match: RegExpExecArray | null; - // Phase 0 (pre-milestone) and Phase 999 (backlog) are sentinels, not real - // phases. They legitimately have no directory and must never be surfaced as - // current/next phase or counted in phase_count. Mirrors the engine-wide - // sentinel convention (phase-id getMilestoneFromPhaseId, roadmap-command-router - // SENTINELS, the #1445 /^999/ progress filters). (#1580) - const isSentinelPhase = (num: string): boolean => { - const major = parseInt(num, 10); - return major === 0 || major === 999; - }; + // #3185 (ADR-3180 Decision 1): the local `isSentinelPhase` closure was a + // fourth independent copy of the sentinel rule (`parseInt(num,10) === 0 || + // === 999`). Deleted in favour of the canonical `isSentinelPhaseId` + // (src/phase-id.cts, SENTINEL_RANGES) so the engine-wide convention #1580 + // describes has exactly one implementation. Phase 0 (pre-milestone) and + // Phase 999 (backlog) are sentinels, not real phases: they legitimately + // have no directory and must never be surfaced as current/next phase or + // counted in phase_count. // Build phase directory lookup once (O(1) readdir instead of O(N) per phase) + // #3185 exemption (documented reason, not a file allowlist — ADR-3180 + // Decision 4a): this is a heading->directory LOOKUP INDEX, not a milestone + // enumeration. It must see the PHYSICAL set so a heading already scoped by + // extractCurrentMilestoneScoped above can find its directory; filtering it + // through listMilestonePhaseDirs would scope the same set twice. const _phaseDirNames = (() => { try { return fs.readdirSync(phasesDir, { withFileTypes: true }) @@ -359,7 +365,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { while ((match = phasePattern.exec(content)) !== null) { const phaseNum = match[1]; - if (isSentinelPhase(phaseNum)) continue; + if (isSentinelPhaseId(phaseNum)) continue; const phaseName = match[2].replace(/\(INSERTED\)/i, '').trim(); // Extract goal from the section @@ -476,7 +482,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { checklistPhases.add(checklistMatch[1]); } const detailPhases = new Set(phases.map(p => p.number)); - const missingDetails = [...checklistPhases].filter(p => !detailPhases.has(p) && !isSentinelPhase(p)); + const missingDetails = [...checklistPhases].filter(p => !detailPhases.has(p) && !isSentinelPhaseId(p)); const result = { milestones, diff --git a/src/state.cts b/src/state.cts index 634c443b5..6bbaa3822 100644 --- a/src/state.cts +++ b/src/state.cts @@ -16,10 +16,10 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir } = phaseIdMod; +const { escapeRegex, parsePhaseFromProse, PHASE_NUMBER_TOKEN_SOURCE, phaseKeyFromToken, phaseKeyFromDir, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); -const { getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone, isMilestoneBoundedInRoadmap, hasMilestoneSectioning } = roadmapParserMod; +const { getMilestoneInfo, 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'); @@ -34,6 +34,9 @@ import scanPhasePlans = require('./plan-scan.cjs'); import planningScopeMod = require('./planning-scope.cjs'); const { SCOPE } = planningScopeMod; // eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseLocatorMod = require('./phase-locator.cjs'); +const { listMilestonePhaseDirs } = phaseLocatorMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports import stateTransitionMod = require('./state-transition.cjs'); const { transitionCore, applyStatePreservation, sliceCurrentPositionSection } = stateTransitionMod; type StateTransitionIntent = stateTransitionMod.StateTransitionIntent; @@ -748,11 +751,14 @@ function cmdStateUpdateProgress(cwd: string, raw: boolean): void { let totalPlans = 0; let totalSummaries = 0; - if (fs.existsSync(phasesDir)) { - const isDirInMilestone = getMilestonePhaseFilter(cwd) as (dir: string) => boolean; - const phaseDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name) - .filter(isDirInMilestone); + { + // #3185 (ADR-3180 Decision 1): "which phase directories belong to the + // CURRENT milestone" — routed through the canonical owner instead of a + // hand-rolled readdirSync + isDirInMilestone filter (which also never + // excluded sentinels, unlike the owner). The owner already handles an + // absent phasesDir as a real empty, so the fs.existsSync guard folds + // into it. + const phaseDirs = listMilestonePhaseDirs(phasesDir, { cwd }).value; for (const dir of phaseDirs) { const { planCount, summaryCount } = scanPhasePlans(path.join(phasesDir, dir)); totalPlans += planCount; @@ -1697,10 +1703,11 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto // #3017: scope the milestone filter to the STORED milestone when available, // so a state.* write doesn't auto-derive (and mis-bind) to a different // milestone's heading and clobber the stored value + progress counts. - const isDirInMilestone = getMilestonePhaseFilter(cwd, storedMilestone ?? undefined) as (dir: string) => boolean; - const allMatchingDirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()).map(e => e.name) - .filter(isDirInMilestone); + // #3185 (ADR-3180 Decision 1): "which phase directories belong to the + // CURRENT (stored) milestone" — routed through the canonical owner + // instead of a hand-rolled readdirSync + isDirInMilestone filter + // (which also never excluded sentinels, unlike the owner). + const allMatchingDirs = listMilestonePhaseDirs(phasesDir, { cwd, versionOverride: storedMilestone ?? null }).value; // Bug #2445: when stale phase dirs from a prior milestone remain in // .planning/phases/ alongside new dirs with the same phase number, @@ -1756,8 +1763,9 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto // Only count tokens that contain at least one digit — excludes // pure-word section headings (Overview, Details) while keeping // numeric phases (01, 05.1) and project-code IDs (PROJ-42). - // Also exclude 999.x backlog phases. Mirrors init.cts filter. - if (!/\d/.test(m[1]) || /^999\b/.test(m[1])) continue; + // Also exclude sentinel phases (0 and 999.x backlog). + // #3185: canonical sentinel predicate (SENTINEL_RANGES [0,999]) — this was a local 999-only literal that admitted Phase 0. + if (!/\d/.test(m[1]) || isSentinelPhaseId(m[1])) continue; // #1514: retired/folded phases are struck through in the ROADMAP; // exclude them from the denominator (they can never be completed). if (retiredPhaseNums.has(phaseKeyFromToken(m[1]))) continue; @@ -3179,6 +3187,11 @@ function cmdStateRebuild(cwd: string, options: StateRebuildOptions, raw: boolean try { const phasesDir = path.join(planningPaths(cwd).planning, 'phases'); if (!fs.existsSync(phasesDir) || !fs.statSync(phasesDir).isDirectory()) return { ok: true, phases: [] }; + // #3185: deliberately NOT listMilestonePhaseDirs. `state rebuild` is a + // RECONCILIATION pass against ground truth -- it must see every phase + // directory on disk so an orphan STATE.md row for a phase that no longer + // exists (or sits outside the current window) is dropped. Scoping this + // would make the rebuild silently preserve stale rows. const entries = fs.readdirSync(phasesDir); const records: PhaseInventoryRecord[] = []; for (const entry of entries) { diff --git a/src/uat.cts b/src/uat.cts index 6d75e22e6..2d34a8d32 100644 --- a/src/uat.cts +++ b/src/uat.cts @@ -21,9 +21,6 @@ const { collectSection, tokenizeHeadings } = markdownSectionizer; import markdownTable = require('./markdown-table.cjs'); const { splitTableRow, isDelimiterRow } = markdownTable; // eslint-disable-next-line @typescript-eslint/no-require-imports -import roadmapParser = require('./roadmap-parser.cjs'); -const { getMilestonePhaseFilter } = roadmapParser; -// eslint-disable-next-line @typescript-eslint/no-require-imports import coreUtils = require('./core-utils.cjs'); const { toPosixPath } = coreUtils; // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -37,7 +34,7 @@ import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocator = require('./phase-locator.cjs'); -const { getArchivedPhaseDirs } = phaseLocator; +const { getArchivedPhaseDirs, listMilestonePhaseDirs } = phaseLocator; import { requireSafePath, sanitizeForDisplay } from './security.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module import configLoader = require('./config-loader.cjs'); @@ -105,21 +102,20 @@ function cmdAuditUat(cwd: string, raw: boolean): void { error('No phases directory found in planning directory'); } - const isDirInMilestone = getMilestonePhaseFilter(cwd); const results: UatFileResult[] = []; // Active dirs are milestone-filtered; archived dirs deliberately are NOT. - // getMilestonePhaseFilter derives the CURRENT milestone's phase numbers from - // ROADMAP.md, and archived phases belong to past milestones by definition — so - // applying it to them discards every one and silently reinstates the bug. + // listMilestonePhaseDirs derives the CURRENT milestone's phase directories + // (window + sentinel filtered) from ROADMAP.md, and archived phases belong + // to past milestones by definition — so applying it to them discards every + // one and silently reinstates the bug. const scanTargets: { dir: string; phaseDir: string; milestone?: string }[] = []; if (hasActivePhases) { - const dirs = fs.readdirSync(phasesDir, { withFileTypes: true }) - .filter(e => e.isDirectory()) - .map(e => e.name) - .filter(isDirInMilestone) - .sort(); + // #3185 (ADR-3180 Decision 1): routed through the canonical owner + // instead of a hand-rolled readdirSync + isDirInMilestone filter, which + // also never excluded sentinels, unlike the owner. + const dirs = listMilestonePhaseDirs(phasesDir, { cwd }).value; for (const dir of dirs) { scanTargets.push({ dir, phaseDir: path.join(phasesDir, dir) }); } diff --git a/src/workstream-inventory.cts b/src/workstream-inventory.cts index 6612fe7dd..4e3519492 100644 --- a/src/workstream-inventory.cts +++ b/src/workstream-inventory.cts @@ -78,11 +78,26 @@ function workstreamsRoot(cwd: string): string { return path.join(planningRoot(cwd), 'workstreams'); } -function countRoadmapPhases(roadmapPath: string, fallbackCount: number): number { +/** + * #3185 (ADR-3180 Decision 1): count the phases the CURRENT milestone + * declares, not every `Phase` heading in the file. + * + * This previously matched `^#{2,4}\s+Phase\s+…` across the whole ROADMAP with + * no milestone window and no sentinel filter, so it counted 999.* backlog and + * Phase 0 headings and spanned every milestone the document had ever had. + * `getMilestonePhaseFilter` already computes exactly this number for the + * scoped window (`phaseCount`, sentinel-filtered), and `inspectWorkstream` in + * this same file already passes a resolved `currentVersion` to it — this + * function was the sibling copy that never got the fix. + */ +function countRoadmapPhases(roadmapPath: string, fallbackCount: number, cwd?: string, ws?: string | null, versionOverride?: string | null): number { try { - const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); - const matches = roadmapContent.match(/^#{2,4}\s+Phase\s+[\w][\w.-]*/gm); - return matches ? matches.length : fallbackCount; + if (!fs.existsSync(roadmapPath)) return fallbackCount; + if (!cwd) return fallbackCount; + const filter = getMilestonePhaseFilter(cwd, versionOverride ?? null, null, ws ?? null); + // A pass-all degrade (phaseCount 0) means the window declared no phases — + // fall back rather than reporting a confident zero. + return filter.phaseCount > 0 ? filter.phaseCount : fallbackCount; } catch { return fallbackCount; } @@ -717,7 +732,7 @@ function inspectWorkstream(cwd: string, name: string, options: InspectWorkstream // declares in its Progress table but never scaffolded — the heading-only // count drops them, even when other headings exist. Union the declared rows // with the phase directories so neither source can silently shrink it. - let fallbackPhaseCount = countRoadmapPhases(p.roadmap, phaseDirNames.length); + let fallbackPhaseCount = countRoadmapPhases(p.roadmap, phaseDirNames.length, cwd, name, currentVersion); if (!scoped && progressRows.length > 0) { const union = new Set(progressRows.map(row => row.key)); for (const entry of phaseFilesCounts) union.add(entry.phaseKey); diff --git a/tests/enumeration-drift-guard.test.cjs b/tests/enumeration-drift-guard.test.cjs new file mode 100644 index 000000000..019ceb4d3 --- /dev/null +++ b/tests/enumeration-drift-guard.test.cjs @@ -0,0 +1,110 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +/** + * Unit coverage for the phase-ENUMERATION drift guard + * (scripts/lint-phase-enumeration-drift.cjs, epic #3180, issue #3185, + * ADR-3180 Decision 1 row "Phase enumeration"). + * + * Modelled on tests/phase-id-drift-guard.test.cjs: this guard was previously + * exercised only via `lint:ci`, with no unit test of its own pure functions. + * Behavioral throughout: assertions drive `findPhaseEnumerationDrift` / + * `scanRepo` / `stripComments` directly — no `readFileSync().includes()` in + * a test body. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const { + findPhaseEnumerationDrift, + scanRepo, + stripComments, +} = require(path.join(ROOT, 'scripts', 'lint-phase-enumeration-drift.cjs')); + +describe('#3185 phase-enumeration drift scanner: findPhaseEnumerationDrift (pure)', () => { + test('a clean line carrying neither detector shape is not flagged', () => { + assert.deepEqual( + findPhaseEnumerationDrift("const { value: dirs } = listMilestonePhaseDirs(phasesDir, { cwd });", 'src/fake.cts'), + [], + ); + }); + + test('an injected fs.readdirSync(phasesDir, ...) line IS flagged', () => { + const v = findPhaseEnumerationDrift( + "const names = fs.readdirSync(phasesDir, { withFileTypes: true });", + 'src/fake.cts', + ); + assert.equal(v.length, 1); + assert.equal(v[0].line, 1); + }); + + test('an injected /^999(?:\\.|$)/ sentinel literal IS flagged', () => { + const v = findPhaseEnumerationDrift( + "const isBacklog = /^999(?:\\.|$)/.test(dirName);", + 'src/fake.cts', + ); + assert.equal(v.length, 1); + assert.equal(v[0].line, 1); + }); + + test('a COMMENT containing either shape is NOT flagged', () => { + const readdirInComment = findPhaseEnumerationDrift( + "// legacy code used to do fs.readdirSync(phasesDir, { withFileTypes: true })", + 'src/fake.cts', + ); + assert.deepEqual(readdirInComment, []); + + const sentinelInComment = findPhaseEnumerationDrift( + "// the old regex was /^999(?:\\.|$)/ before the single owner existed", + 'src/fake.cts', + ); + assert.deepEqual(sentinelInComment, []); + + const blockCommentContinuation = findPhaseEnumerationDrift( + " * mirrors fs.readdirSync(phasesDir, { withFileTypes: true }) from before", + 'src/fake.cts', + ); + assert.deepEqual(blockCommentContinuation, []); + }); + + test('an unrelated numeric such as const n = total + 1999 is NOT flagged', () => { + assert.deepEqual( + findPhaseEnumerationDrift('const n = total + 1999;', 'src/fake.cts'), + [], + ); + }); +}); + +describe('#3185 phase-enumeration drift scanner: stripComments (pure)', () => { + test('a whole-line // comment strips to empty', () => { + assert.equal(stripComments('// just a comment'), ''); + }); + + test('a JSDoc continuation line strips to empty', () => { + assert.equal(stripComments(' * some doc text'), ''); + }); + + test('a trailing // comment after code strips only the comment portion', () => { + assert.equal(stripComments('const x = 1; // trailing note'), 'const x = 1; '); + }); + + test('a line with no comment is returned unchanged', () => { + const line = 'const dirs = listMilestonePhaseDirs(phasesDir, { cwd });'; + assert.equal(stripComments(line), line); + }); +}); + +describe('#3185 phase-enumeration drift scanner: the live repo is clean', () => { + test('scanRepo(repoRoot) returns zero unsanctioned re-derivations', () => { + const violations = scanRepo(ROOT); + assert.deepEqual( + violations, + [], + 'unsanctioned phase-enumeration re-derivation(s) — route through listMilestonePhaseDirs / isSentinelPhaseId:\n' + + violations.map((d) => ` ${d.file}:${d.line} ${d.found}`).join('\n'), + ); + }); +}); diff --git a/tests/enumeration-single-owner.test.cjs b/tests/enumeration-single-owner.test.cjs new file mode 100644 index 000000000..c88e1e6a0 --- /dev/null +++ b/tests/enumeration-single-owner.test.cjs @@ -0,0 +1,486 @@ +/** + * Tests for the phase-ENUMERATION single-owner contract (#3185, epic #3180 + * Phase 3, ADR-3180 Decision 1 row "Phase enumeration"). + * Matrix: .gsd/phase/refactor-3185-phase-enumeration-single-owner/50-test-matrix.md + * + * This file lands FAILING-FIRST. Every test below asserts the behavior the + * design requires and that the tree does NOT have yet, so the remote runner + * records a real RED before the fix. + * + * Rows covered here are the ones with direct code evidence in the design: + * - row 36 `progress` lists 999.* backlog dirs as current-milestone phases + * - row 39 a `0-*` sentinel directory is listed by `stats`/`progress` + * - row 14 negative space: `P0.0-foundation` is a REAL phase (#1324 + * letter-prefixed-decimal family), not sentinel milestone 0 + * - row 38 a `### Phase 999.1:` ROADMAP heading produces a `stats` row + * - row 43 `phases clear --confirm` DELETES a `0-*` directory, because + * cmdPhasesClear carries its own 5th sentinel copy + * (`/^999(?:\.|$)/`) excluding 999 but NOT 0 — while + * `roadmap analyze` treats Phase 0 as a sentinel to preserve. + * + * Assertions are on `--json` output (a typed surface), never on rendered + * prose — CONTRIBUTING.md bans raw-text matching on test outputs. Row 43 also + * carries the NEGATIVE PROOF that the directory still exists on disk, because + * an exit code alone cannot distinguish "refused" from "deleted and reported + * success". + */ + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { + createTempProject, + createTempGitProject, + cleanup, + runGsdTools, +} = require('./helpers.cjs'); +const { SCOPE } = require('../gsd-core/bin/lib/planning-scope.cjs'); +const { isSentinelPhaseId } = require('../gsd-core/bin/lib/phase-id.cjs'); + +// ─── Fixture helpers ─────────────────────────────────────────────────────── + +function planningDirOf(cwd) { + return path.join(cwd, '.planning'); +} + +function writeRoadmap(cwd, lines) { + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + fs.writeFileSync(path.join(planningDirOf(cwd), 'ROADMAP.md'), lines.join('\n')); +} + +function writeState(cwd, fields) { + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + const lines = ['---']; + for (const [k, v] of Object.entries(fields)) lines.push(`${k}: ${v}`); + lines.push('---', ''); + fs.writeFileSync(path.join(planningDirOf(cwd), 'STATE.md'), lines.join('\n')); +} + +/** Create `/.planning/phases//` and optionally seed files. */ +function makePhaseDir(cwd, dirName, files = {}) { + const dir = path.join(planningDirOf(cwd), 'phases', dirName); + fs.mkdirSync(dir, { recursive: true }); + for (const [name, content] of Object.entries(files)) { + fs.writeFileSync(path.join(dir, name), content); + } + return dir; +} + +/** A ROADMAP with a real current milestone plus a 999 backlog heading. */ +function roadmapWithBacklog() { + return [ + '# Roadmap', + '', + '## v1.0 Current 🚧', + '', + '### Phase 1: Foundation', + '', + '**Goal:** lay the foundation', + '', + '### Phase 999.1: Icebox item', + '', + '**Goal:** someday', + '', + ]; +} + +function parseJson(result, label) { + assert.ok(result.success, `${label} should exit 0: ${result.error || ''}`); + try { + return JSON.parse(result.output); + } catch (err) { + throw new Error(`${label} did not emit parseable JSON: ${err.message}`); + } +} + +// ═════════════════════════════════════════════════════════════════════════ +// Row 36 — `progress` must not list 999.* backlog dirs as milestone phases +// ═════════════════════════════════════════════════════════════════════════ + +test('progress does not list a 999 backlog directory as a current-milestone phase', (t) => { + const cwd = createTempProject('gsd-phase-enum-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '999.1-icebox', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => n.startsWith('999')), + `999.* backlog directories must not appear as current-milestone phases; got ${JSON.stringify(numbers)}`, + ); + assert.ok( + numbers.includes('01') || numbers.includes('1'), + `the real in-window phase must still be listed; got ${JSON.stringify(numbers)}`, + ); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// Row 39 / row 14 — phase-0 sentinel vs the letter-prefixed-decimal family +// ═════════════════════════════════════════════════════════════════════════ + +test('progress does not list a phase-0 sentinel directory', (t) => { + const cwd = createTempProject('gsd-phase-enum-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '0-prep', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => /^0+$/.test(n)), + `phase-0 sentinel directories must not appear as milestone phases; got ${JSON.stringify(numbers)}`, + ); +}); + +test('a letter-prefixed decimal directory is NOT read as a phase-0 sentinel', (t) => { + // Negative space (matrix row 14): `P0.0-foundation` is a real phase from the + // #1324 letter-prefixed-decimal family, NOT sentinel milestone 0. A sentinel + // rule that strips a bare letter prefix would silently delete this family + // from three commands. + const cwd = createTempProject('gsd-phase-enum-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, [ + '# Roadmap', + '', + '## v1.0 Current 🚧', + '', + '### Phase P0.0: Foundation', + '', + '**Goal:** real work', + '', + ]); + makePhaseDir(cwd, 'P0.0-foundation', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + numbers.length > 0, + 'a letter-prefixed decimal phase must survive the sentinel filter, not be dropped as milestone 0', + ); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// Row 38 — a 999 ROADMAP heading must not produce a `stats` row +// ═════════════════════════════════════════════════════════════════════════ + +test('stats does not emit a row for a 999 backlog heading with no directory', (t) => { + const cwd = createTempProject('gsd-phase-enum-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('stats json', cwd), 'stats json'); + + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => n.startsWith('999')), + `a 999 heading must not seed a stats row; got ${JSON.stringify(numbers)}`, + ); +}); + +test('stats does not list a phase-0 sentinel directory', (t) => { + const cwd = createTempProject('gsd-phase-enum-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '0-prep', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('stats json', cwd), 'stats json'); + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => /^0+$/.test(n)), + `phase-0 sentinel directories must not appear in stats; got ${JSON.stringify(numbers)}`, + ); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// Row 43 — the destructive path: `phases clear` must not delete a sentinel +// ═════════════════════════════════════════════════════════════════════════ + +test('phases clear preserves a phase-0 sentinel directory, with negative proof on disk', (t) => { + // cmdPhasesClear carries the FIFTH copy of the sentinel rule and the THIRD + // regex variant — `/^999(?:\.|$)/` — which excludes 999 but NOT 0. So a + // `0-*` directory that `roadmap analyze` treats as a preserved sentinel is + // DELETED here. The exit code alone cannot show this, so the load-bearing + // assertion is that the directory still exists afterwards. + const cwd = createTempGitProject('gsd-phase-enum-clear-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + const zeroDir = makePhaseDir(cwd, '0-prep', { '01-PLAN.md': '# Plan\n' }); + const backlogDir = makePhaseDir(cwd, '999.1-icebox', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + + runGsdTools('phases clear --confirm --force', cwd); + + assert.ok( + fs.existsSync(zeroDir), + 'a phase-0 sentinel directory must be preserved by `phases clear`, exactly as the 999 sentinel already is', + ); + assert.ok( + fs.existsSync(backlogDir), + 'the 999 sentinel directory must remain preserved (unchanged behavior)', + ); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// Rows 7-9 — the PASS-ALL degrade, which is where the defect actually lives +// +// When the milestone window yields NO phase headings, getMilestonePhaseFilter +// degrades to a literal `() => true` predicate and stops consulting its +// heading set at all — so its `/^999\b/` heading-side sentinel exclusion +// becomes unreachable and every directory on disk is reported as a +// current-milestone phase. A fixture that contains phase headings keeps the +// filter ACTIVE and never exercises this path, which is why the sibling +// phase-0 test above passes today for the wrong reason. +// ═════════════════════════════════════════════════════════════════════════ + +/** A ROADMAP whose current milestone declares NO phases — the pass-all path. */ +function roadmapWithNoPhaseHeadings() { + return [ + '# Roadmap', + '', + '## v1.0 Current 🚧', + '', + 'This milestone has been declared but has no phases yet.', + '', + ]; +} + +test('pass-all degrade still excludes a 999 sentinel directory', (t) => { + const cwd = createTempProject('gsd-phase-enum-passall-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithNoPhaseHeadings()); + makePhaseDir(cwd, '999.1-icebox', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => n.startsWith('999')), + `the pass-all degrade must still filter sentinels; got ${JSON.stringify(numbers)}`, + ); +}); + +test('pass-all degrade still excludes a phase-0 sentinel directory', (t) => { + const cwd = createTempProject('gsd-phase-enum-passall-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithNoPhaseHeadings()); + makePhaseDir(cwd, '0-prep', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + !numbers.some((n) => /^0+$/.test(n)), + `the pass-all degrade must still filter phase-0 sentinels; got ${JSON.stringify(numbers)}`, + ); +}); + +test('pass-all degrade stays over-inclusive for NON-sentinel directories', (t) => { + // Negative space (matrix row 9): the narrowing is sentinel-only. The + // documented "over-inclusive, never under-inclusive" promise in + // getMilestonePhaseFilter's own comment is narrowed minimally, not revoked — + // an ordinary phase directory must still be listed when the window declares + // no phases, or a project mid-migration loses its real phases from view. + const cwd = createTempProject('gsd-phase-enum-passall-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithNoPhaseHeadings()); + makePhaseDir(cwd, '04-thing', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + const numbers = (report.phases || []).map((p) => String(p.number)); + assert.ok( + numbers.length > 0, + 'an ordinary phase directory must still be listed under the pass-all degrade', + ); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// `phases list` — ADR-3180 Decision 4(c): the identity assertion belongs at +// EACH consumer's own observable output, not only at progress/stats. +// ═════════════════════════════════════════════════════════════════════════ + +test('phases list excludes a 999 backlog dir and a 0-sentinel dir, keeps the real one', (t) => { + const cwd = createTempProject('gsd-phase-enum-list-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '999.1-icebox', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '0-prep', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('phases list', cwd), 'phases list'); + const dirs = report.directories || []; + + assert.ok( + !dirs.some((d) => d.startsWith('999')), + `999.* backlog directories must not appear in phases list; got ${JSON.stringify(dirs)}`, + ); + assert.ok( + !dirs.some((d) => /^0(?:[-.]|$)/.test(d)), + `phase-0 sentinel directories must not appear in phases list; got ${JSON.stringify(dirs)}`, + ); + assert.ok( + dirs.includes('01-foundation'), + `the real in-window phase directory must still be listed; got ${JSON.stringify(dirs)}`, + ); +}); + +test('phases list --phase still finds an out-of-window phase (documented exemption)', (t) => { + const cwd = createTempProject('gsd-phase-enum-list-lookup-'); + t.after(() => cleanup(cwd)); + + // v1.0 is current; phase 5 belongs to a different (later) milestone and is + // never mentioned in v1.0's window, so it is genuinely OUT-OF-WINDOW. + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, [ + '# Roadmap', + '', + '## v1.0 Current 🚧', + '', + '### Phase 1: Foundation', + '', + '## v2.0 Next', + '', + '### Phase 5: Later Work', + '', + ]); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '05-later-work', { '01-PLAN.md': '# Plan\n' }); + + const result = runGsdTools('phases list --phase 5', cwd); + const report = parseJson(result, 'phases list --phase 5'); + + assert.strictEqual( + report.error, + undefined, + `--phase lookup must find an out-of-window phase by design (ADR-3180 Decision 4a); got ${JSON.stringify(report)}`, + ); + assert.deepStrictEqual(report.directories, ['05-later-work']); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// `phase_scope` — epic #3180 Done-when #7: the enumeration path can fail +// distinguishably. Mirrors the TRUNCATED fixture shape from +// tests/milestone-window-single-owner.test.cjs's buildTruncatedFixture: an +// ACTIVE milestone heading with no phase headings inside its own window, +// followed by a later milestone heading whose section DOES carry phase +// headings -- the window closes before the phase region. +// ═════════════════════════════════════════════════════════════════════════ + +function writeTruncatedRoadmap(cwd) { + writeRoadmap(cwd, [ + '# Roadmap', + '', + '## v3.0 In Progress 🚧', + '', + 'Some preamble notes. No phase headings here.', + '', + '## v4.0 Next', + '', + '### Phase 1: Foo', + '', + '### Phase 2: Bar', + '', + ]); +} + +test('progress json phase_scope is not complete when the milestone window is truncated', (t) => { + const cwd = createTempProject('gsd-phase-enum-scope-truncated-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v3.0' }); + writeTruncatedRoadmap(cwd); + makePhaseDir(cwd, '1-foo', { '01-PLAN.md': '# Plan\n' }); + makePhaseDir(cwd, '2-bar', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + assert.notStrictEqual( + report.phase_scope, + SCOPE.COMPLETE, + `a truncated window must not report phase_scope: "complete"; got ${JSON.stringify(report.phase_scope)}`, + ); +}); + +test('progress json phase_scope is complete for a healthy, in-window fixture', (t) => { + const cwd = createTempProject('gsd-phase-enum-scope-complete-'); + t.after(() => cleanup(cwd)); + + writeState(cwd, { milestone: 'v1.0' }); + writeRoadmap(cwd, roadmapWithBacklog()); + makePhaseDir(cwd, '01-foundation', { '01-PLAN.md': '# Plan\n' }); + + const report = parseJson(runGsdTools('progress json', cwd), 'progress json'); + assert.strictEqual(report.phase_scope, SCOPE.COMPLETE); +}); + +// ═════════════════════════════════════════════════════════════════════════ +// isSentinelPhaseId boundary (#3185, reverted 5fcb7a5a2): #2554 and #2949 +// are pinned contracts that ask DIFFERENT QUESTIONS about `0.x` ids, so one +// global predicate cannot answer both: +// - #2554 (tests/roadmap-parser.test.cjs) asks "is this directory part of +// the current milestone's phase set?" -- a `00.1-` directory +// declared as `### Phase 00.1:` MUST be counted there. +// - #2949 (tests/issue-2949-phase-complete-stage3-sentinel.test.cjs) asks +// "must this phase be completed before the milestone can close?" -- a +// `0.x` id IS a sentinel for that question, and must not block +// `is_last_phase=true`. +// isSentinelPhaseId answers the #2949 (completion) question, so `0.x` stays +// a sentinel here. The #2554 (milestone-window) question is answered by a +// narrower, 999-only rule inside getMilestonePhaseFilter in +// src/roadmap-parser.cts, not by this predicate. The two questions are +// deliberately answered at two different layers. +// ═════════════════════════════════════════════════════════════════════════ + +const SENTINEL_IDS = [ + '0', + '00', + '0-prep', + '00-prep', + '0.1', + '00.1', + '0.1-slug', + '999', + '999.1', + '999.1-icebox', + 'GSD-999-icebox', +]; +const NON_SENTINEL_IDS = ['01-foundation', 'P0.0-foundation', '9990-x', '998-x', '1000-x', '04-thing']; + +test('isSentinelPhaseId: sentinel boundary table', () => { + for (const id of SENTINEL_IDS) { + assert.strictEqual(isSentinelPhaseId(id), true, `expected sentinel: ${JSON.stringify(id)}`); + } +}); + +test('isSentinelPhaseId: NOT-sentinel boundary table', () => { + for (const id of NON_SENTINEL_IDS) { + assert.strictEqual(isSentinelPhaseId(id), false, `expected NOT sentinel: ${JSON.stringify(id)}`); + } +});