* test(#3185): failing-first phase-enumeration single-owner suite Covers the enumeration rows with direct code evidence: 999.* backlog dirs listed by progress/stats, the phase-0 sentinel divergence, the #1324 letter-prefixed-decimal negative space, and the destructive-path find — cmdPhasesClear carries a fifth sentinel copy (/^999(?:\.|$)/) that excludes 999 but not 0, so a 0-* directory roadmap.analyze preserves is deleted there. Also covers the pass-all degrade, which is where the defect actually lives: when the milestone window declares no phases the filter becomes a literal () => true and its heading-side sentinel exclusion is unreachable. A fixture carrying phase headings keeps the filter active and never reaches that path. Named for the derivation, not a module: the suite drives commands, phase, milestone, workstream-inventory and state, and both the phase and phase-locator buckets are already at the per-module test-file cap. Committed alone so the remote runner records the failure before the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * refactor(#3185): phase enumeration has one owner and a decidable scope Adds phase-locator.cts::listMilestonePhaseDirs as the single canonical owner of "which phase directories belong to the current milestone". It applies the milestone window AND the sentinel filter and returns a ScopedResult, so a caller can tell a genuinely-empty milestone from an enumeration that could not be scoped. The sentinel test now runs against DIRECTORY NAMES and is unconditional. getMilestonePhaseFilter excludes sentinels from its ROADMAP heading set, but degrades to a literal () => true pass-all predicate when that set is empty -- at which point the heading set is never consulted and its sentinel exclusion is unreachable exactly when it is needed. That degrade is the #3167 path, and it is why stats already used the filter and still listed backlog directories. The narrowing is sentinel-only: pass-all stays over-inclusive otherwise. Sentinel copies deleted, canonical isSentinelPhaseId adopted: - cmdRoadmapAnalyze's local closure (parseInt === 0 || === 999), 2 call sites - cmdPhasesClear's /^999(?:\.|$)/ -- the DESTRUCTIVE path, which excluded 999 but not 0, so a 0-* directory roadmap.analyze preserves was deleted cmdStats also seeded rows from ROADMAP headings with no sentinel filter, so a 999 heading produced a row with no directory; that seed is filtered now. cmdPhasesList routes only its ENUMERATION. --phase lookup searches the physical set (scoping it would report an out-of-window phase as not found) and --include-archived still merges archived dirs (they are by definition from other milestones). Both exempt by documented reason, never a file allowlist. Fixed inline, found while building: isDirInMilestone could not match a #1324 letter-prefixed-decimal directory (P0.0-foundation) to its own Phase P0.0 heading, so stats reported the phase with plans: 0 while its directory held plan files. Defers to phase-id's extractPhaseToken rather than widening a fourth bespoke regex; additive, so it can only admit directories. Refs #3180. Closes #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * refactor(#3185): route the last two enumeration re-derivations workstream-inventory countRoadmapPhases counted every `Phase` heading across the whole ROADMAP -- no window, no sentinel filter -- so it counted 999.* backlog and Phase 0 and spanned every milestone the document ever had. Its own caller already resolved a currentVersion and passed it to getMilestonePhaseFilter elsewhere in the same file; this was the sibling copy that never got the fix. state.cts phaseInventoryProvider enumerated phase dirs with its own /^(\d+)-(.+)$/ convention regex and neither filter, so a rebuilt STATE.md inventory carried backlog and sentinel directories as current-milestone phases. A non-COMPLETE enumeration scope now throws to the outer catch as a real scan failure rather than reporting a confident undercount, mirroring the per-phase scanPhasePlans contract beside it. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * refactor(#3185): consolidate 23 sentinel re-derivations onto one predicate The whole-repo drift guard (ADR-3180 Decision 4a, no file allowlist) found the sentinel rule re-implemented 23 times across 8 modules, in three regex variants plus four integer-comparison forms. Most tested 999 only, so Phase 0 slipped through them while roadmap.analyze and the engine-wide convention (#1580) both treat 0 and 999 alike. That disagreement is the defect class this epic removes. All 23 now call phase-id's isSentinelPhaseId (SENTINEL_RANGES [0,999]). Sites: init recommended-actions and backlog counts, milestone phase scan, the phase-lifecycle progress table, phase.cts used-number collection and the four renumber-on-remove guards, roadmap-parser's heading and bullet milestone counts, roadmap get-phase fallbacks, and state's heading denominator. Excluding Phase 0 at these sites is a deliberate behavior change and the point of the consolidation — several carried comments already saying 0 should be excluded while the literal beside them caught only 999. Adds scripts/lint-phase-enumeration-drift.cjs, wired into lint:ci. It scans the whole src/ tree with no file allowlist and reports both shapes: an independent phases-dir enumeration, and an independent sentinel literal. Exemptions are function-scoped with a written reason. The guard is comment-aware — its first pass flagged JSDoc and a comment documenting that the code below uses the canonical owner, which would have trained readers to exempt prose. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * refactor(#3185): resolve every phases-dir enumeration; drift guard reports zero Per-site triage of the 31 remaining whole-repo guard hits, applying the rule generalized from #3183's Amendment 1: a LOOKUP, DIAGNOSTIC, ARCHIVAL or MUTATION pass wants the physical set; only "which phases belong to this milestone" wants the scoped set. Routed (10): init new-milestone phase_dir_count, init milestone-op fallback count, init manager, init progress, milestone complete stats/dry-run/archive move, phase complete's next-phase scan, state update-progress, state frontmatter stats, and uat audit's active set. Exempt with a written function-scoped reason (never a file allowlist): the audit/UAT/verification sweeps that deliberately scan every directory to report gaps, phase create/insert/rename/renumber mutations, single-phase lookups, roadmap-upgrade's cross-milestone migration, cmdPhasesClear's whole-tree destructive pass, and the reads that list a phase dir's FILES rather than enumerating the phases dir at all. Latent defects fixed by the routing: sentinel directories leaked into cmdInitNewMilestone's phase_dir_count, cmdMilestoneComplete's stats, dry-run AND ARCHIVE MOVE, cmdStateUpdateProgress, buildStateFrontmatter and cmdAuditUat's active set — every one of those hand-rolled an isDirInMilestone filter with no sentinel exclusion, so `milestone complete` was archiving backlog directories. scripts/lint-phase-enumeration-drift.cjs now reports 0 re-derivations and npm run lint:ci is green. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * docs(#3185): document milestone-scoped enumeration and record ADR Amendment 3 Changeset fragment (Changed), CLI-TOOLS/COMMANDS/USER-GUIDE updates for the scoped output of progress, stats, phases list, phases clear and milestone complete, the CONTEXT.md Phase Locator glossary entry naming listMilestonePhaseDirs, and ADR-3180 Amendment 3. Amendment 3 records: the SCOPE contract held unchanged; the declared deviation from Decision 1's provisional signature (the window needs cwd/ws, which the locked roadmapContent parameter cannot supply); the copy count being a lower bound for the third consecutive phase (4 scoped vs 54 found); the load-bearing finding that the sentinel exclusion sat on the heading set and was unreachable under the pass-all degrade; the two destructive-path defects; and the generalized exemption rule. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * fix(#3185): wire scope to consumers; revert two wrong routings the suite caught Review + remote runner findings, all fixed: The three consumers computed the enumeration scope and threw it away, so TRUNCATED/UNSCOPED/UNREADABLE collapsed into the same output as COMPLETE -- reproducing this epic's own output-identical-failure defect one layer up. progress, stats and phases list now emit phase_scope (null on the phases list --phase lookup path, which performs no enumeration). Two routings were wrong and the suite proved it: roadmap-parser's two milestone phase-count scans are reverted to the 999-only literal. isSentinelPhaseId is BROADER than what it replaced: its legacy branch runs /^0*(\d+)/ over "00.1", which backtracks to capture 0, so it read #2554's decimal phase ids as sentinel milestone 0 and stopped counting them. state.cts phaseInventoryProvider is reverted to the physical disk scan. `state rebuild` is a RECONCILIATION pass -- scoping it made it throw on healthy trees whose fixture resolves no window, swallowed the raw readdirSync fault message #3057 B1 requires verbatim, and stopped it dropping orphan STATE.md rows, which is the job. Both are now function-scoped guard exemptions with written reasons, not silent reverts. This is the consolidation trap named in the epic: a canonical rule can cover MORE than the copy it replaces, and only real inputs show it. Adds phases list coverage, a scope-branch test, and a drift-guard unit suite; backports comment-awareness to the milestone-window and plan-count guards so all three siblings share one false-positive profile; names #3161 alongside #3167 in Amendment 3's subsumption record. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * fix(#3185): correct isSentinelPhaseId's decimal-zero misclassification An isolated security review caught this branch committing the epic's own sin: the over-broad predicate was worked around at ONE call site and left live at the destructive ones. isSentinelPhaseId's legacy branch ran /^0*(\d+)/, which backtracks so any id whose leading digit run is all zeros before a non-digit captures 0 -- "0.1", "00.1" and "0.2554" all read as sentinel milestone 0. Two pinned contracts disagree with that: #2554 requires "00.1" to be counted as a real phase, and the 999 icebox is a whole reserved milestone so "999.1" must stay sentinel. The rule is asymmetric and now says so explicitly: 999 is sentinel with or without a decimal part; 0 is sentinel only when bare. A decimal phase under either is a real phase for 0 and reserved for 999, because 999 reserves a MILESTONE while 0 reserves a PHASE. Fixing the owner lets the earlier workaround go: getMilestonePhaseFilter's two scans route through isSentinelPhaseId again and the guard exemption that existed only to accommodate the defect is deleted. The state.cts cmdStateRebuild exemption stays -- that one is a genuine reconciliation-wants-the-physical-set case. Also corrects tests/adr-612-bracket-grammar.test.cjs, which asserted isSentinelPhaseId('0.1') === true and so had encoded the defect as expected behavior. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * fix(#3185): keep isSentinelPhaseId's semantics — 0.x is layered, not wrong Reverts the previous commit. The remote suite failed six tests proving it wrong, and the reason is the sharpest finding of this phase. An isolated security review observed that isSentinelPhaseId reads 0.1 and 00.1 as sentinel milestone 0 and judged that a defect against #2554. Correcting the canonical predicate broke #2949. Both contracts are pinned and both are right, because they ask different questions: #2554 is this dir part of the current milestone's phase SET? -> count 00.1 #2949 must this phase COMPLETE before the milestone closes? -> 0.x sentinel No single global predicate answers both. isSentinelPhaseId keeps its semantics (0.x IS a sentinel, #2949), and the milestone-window layer keeps a narrower 999-only rule (#2554) as a function-scoped guard exemption with a written reason — not a second silent copy. That corrects how Decision 1 reads: "one owner per derivation" governs who computes an answer, not how many questions share it. An over-broad canonical rule is as much a defect as a divergent copy and fails worse, because it looks like consolidation. Recorded in Amendment 3 as the lesson for Phases 4 and 5. Where a review's inference about intent conflicts with a pinned contract, the pinned contract wins; the finding is adjudicated, not fixed. The boundary tables in the enumeration suite are corrected to assert 0.x IS a sentinel, with the layering explained. Refs #3180 #3185. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG * chore(#3185): set changeset fragment pr to 3222 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QELmgcSwcNBgbUs3kzJeqG --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -140,8 +140,32 @@ node gsd-tools.cjs phase-plan-index <phase>
|
||||
|
||||
# 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 <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 <N>` (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 <ids>
|
||||
|
||||
**Unstarted-phase guard.** Before archiving, the command scans the ROADMAP scoped for `<version>` 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 `<version>`, 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/<version>-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 <filename>
|
||||
|
||||
|
||||
@@ -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 <version> --force` (the `/gsd-complete-milestone` workflow runs the underlying command without `--force`, so use the CLI directly to override). A `STATE.md` `milestone:` value that does not match `<version>` prints a WARNING and still runs the guard (#2946).
|
||||
|
||||
> **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.
|
||||
|
||||
@@ -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).
|
||||
|
||||
|
||||
@@ -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<string[]>` 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<string[]>`.
|
||||
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<T>` 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/<version>-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-<slug>` 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`.
|
||||
|
||||
Reference in New Issue
Block a user