golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity
5,137 Commits 4 Branches 2 Tags
2076d450d725c472e9497b96aaa192d5540fe2f4
Commit Graph

7 Commits

Author SHA1 Message Date
Tom Boucher
636ec92107 refactor(#3185): phase enumeration has one owner and a decidable scope (#3222) 
* 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>
 2026-08-08 14:22:10 -04:00
Tom Boucher
343835facc refactor(#3183): route live-plan counting through scanPhasePlans (#3199) 
* refactor(#3183): route live-plan counting through scanPhasePlans

scanPhasePlans becomes the sole owner of the live-plan derivation. Twenty-one
independent re-derivations across seven modules now route through it, and
scripts/lint-plan-count-drift.cjs reports zero, scanning the whole repo rather
than an allowlist (ADR-3180 Decision 4a).

The epic scoped this at three copies. A whole-repo guard found twenty-six sites
across nine files, so Phase 1 absorbs every live-plan re-derivation and Phase 3
narrows to window plus sentinel enumeration.

Two sites are exempt with a documented reason rather than a bare allowlist:
audit.cts scans one quick task's own directory for a single completion record,
and gsd2-import.cts reads a foreign GSD-2 tasks/ layout during a one-time
import. Neither is a phase directory.

scanPhasePlans gains allPlanFiles (pre-supersession) alongside planFiles so one
owner answers both questions: verify.cts's numbering-gap check wants every plan
on disk, its pairing check wants the live set. Both fields are additive.

Highest-severity fix: cmdPhasePlanIndex, which feeds execute-phase wave
scheduling, was scheduling status:superseded plans into waves and reporting zero
plans for the post-#3139 nested layout.

filterPlanFiles and filterSummaryFiles are deleted; getPhaseFileStats orphaned
them and only their own tests still called them.

New leaf module src/planning-scope.cts carries the frozen SCOPE discriminator,
with its six-gate ripple closed: gitignore, inventory manifest, INVENTORY.md and
the CONTEXT.md glossary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

* docs(#3183): amend ADR-3180 for the Phase 1/3 boundary re-slice

The contract held; the phase boundary did not. The whole-repo drift guard found
26 re-derivations across 9 files against the epic's estimate of 3, and
cmdProgressRender re-derives both enumeration and plan counting on adjacent
lines, so DW4 was unsatisfiable within Phase 1's original file scope.

Records the amended scope, scanPhasePlans's new allPlanFiles field,
findOrphanSummaries, the two documented exemptions, the re-derived Tier-2
table, and the describeNonCanonicalPlans trap for later phases.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

* fix(#3183): complete the canonical pairing rule and gate the naming diagnostic

The remote runner went red with 13 deterministic failures on both lanes,
and they were right: replacing verify.cts's canonicalPlanStem pairing with
summaryCandidates dropped a case the bespoke rule covered. A plan carrying a
descriptive slug after its id (68-01-scaffolding-PLAN.md) pairs with its
canonical-stem summary (68-01-SUMMARY.md), and summaryCandidates generated no
such candidate, so the plan read unsummarized.

The fix is to complete the one rule rather than restore a second:
summaryCandidates gains a canonical-id candidate, narrowed to fire only when an
id pair was actually extracted. countMatchedSummaries, findUnsummarizedPlans
and findOrphanSummaries all inherit it. The two-plans-one-summary collision
behaviour of the original rule is preserved deliberately and documented in
place.

Second defect, independently root-caused while verifying: routing the #2893
naming diagnostic through scanPhasePlans exposed it to the loose /PLAN/i
fallback, which is correct for counting and wrong for a naming check — a
non-canonically-named file was accepted as a valid plan and the diagnostic
went silent. cmdPhasesList, cmdFindPhase and cmdPhasePlanIndex now intersect
with a strict isCanonicalPlanFile predicate before reporting names.

Same class as the describeNonCanonicalPlans trap already recorded in ADR-3180:
a question about file naming wants the physical, strictly-matched set; only a
question about outstanding work wants the live set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

* chore(#3183): register planning-scope.cjs in the eslint migration list

tests/repo-invariants.test.cjs asserts every bin/lib/*.cjs is linted xor
ignored per its ADR-457 migration state. The new planning-scope module closed
five of the six .cts ripple gates - gitignore, inventory manifest, INVENTORY.md
and the CONTEXT.md glossary - but not eslint, because that one is enforced by a
test rather than by lint:ci, so the local pipeline stayed green while it was
missing.

Generated from src/planning-scope.cts, so the .cjs is ignored and the .cts is
linted, matching every other migrated module.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

* fix(#3183): replace the plan-count drift detector with a literal tokenizer

CodeQL reported 4 high-severity js/redos alerts on REGEX_LITERAL_MD_RE, the
backtracking regex that finds "a regex literal mentioning PLAN/SUMMARY and an
escaped \.md". Five review rounds found it had two defects, not one:

  - EXPONENTIAL, then CUBIC. Its "any char" atom `(?:\\.|[^/\r\n])` let a `\.`
    pair be consumed either as one escape or as two class characters, which is
    exponential backtracking: 27,464ms on `"/\.mdplan" + "\.".repeat(28) + "X"`.
    Excluding `\` from the class killed that but left a cubic path — 23ms at
    N=200, 172ms at N=400, 1362ms at N=800 on `"/" + "PLAN\.md".repeat(N)` with
    no closing `/`. This guard is the last stage of `npm run lint:ci`, which CI
    runs on fork pull requests, so a crafted src/*.cts could stall the job.
  - A DETECTION HOLE. A character class holding a bare, unescaped `/` — e.g.
    `/SUMMARY[^/]*\.md$/`, an ordinary path-excluding filter — terminated the
    literal at that `/`, so the scan never reached `\.md` and the guard missed
    it entirely. (Classes holding an ESCAPED `\/` were already matched; the
    tests cover those separately as parity, not as regressions.)

Both defects have one root cause: regex-literal grammar — `\x` escapes, and
`/` inside `[...]` not terminating — is not expressible in a backtracking
regex. So the detector is now a tokenizer, not a regex.

readRegexLiteralAt reads the literal at a given `/` in a single left-to-right
pass with no backtracking, treating escapes as two-character units and
suppressing the `/` terminator inside a character class. findRegexLiteralMdMatch
restarts it at every `/` on the line, preserving the old "find anywhere"
behaviour; MAX_REGEX_LITERAL_LEN (400) bounds each read — including the
trailing-flag scan — which keeps the whole-line cost linear.

Results: cubic shape flat at 0.06-0.39ms out to N=3200 (25KB), exponential
shape 0.01ms at 28 reps and 0.00ms at 64, and the bare-`/` class shapes are now
caught. Differential against the old regex over 28,474 lines (those matching
FILENAME_TEST_RE but not PLAN_SUMMARY_LITERAL_RE, across src/tests/scripts/
gsd-core/bin/eslint-rules, excluding 265 lines with >6 backslashes on which the
old regex hangs): 6 differences, all the tokenizer returning the fuller or
newly-correct literal, 0 old-only misses. The `\.md` token stays
case-insensitive, matching the `/i` the old regex carried.

Also closes three holes in the same new file:

  - walk() tested entry.isFile(), false for a symlink, so a symlinked
    src/*.cts was silently unscanned — an evasion of a guard whose stated
    principle (ADR-3180 Decision 4a) is whole-repo discovery with no allowlist.
    It now resolves symlinks, but confined: file links must resolve inside the
    repo root, directory links inside the scanned dir itself. Every sibling
    drift guard in scripts/ uses the Dirent classification and never follows
    links, so following them unconfined would have made this the only linter
    able to read outside the tree — on fork PRs an arbitrary out-of-repo read
    whose matched fragments reach a public CI log. The narrower directory rule
    additionally stops `src/up -> ..` from sweeping the whole repo, and the
    skip list is now checked against resolved paths so `src/g -> ../.git`
    cannot reach .git/** or node_modules/**. Real paths are de-duplicated and
    files reported canonically, so a symlink alias cannot shift which
    FUNCTION_SCOPED_EXEMPTIONS key applies.
  - Both the reported fragment and the reported FILE PATH are attacker-
    controlled source text written straight to a CI log, and git permits
    control bytes in a filename. Both are now escaped — C0/C1/DEL plus the
    bidi and zero-width controls — so a crafted literal or filename cannot
    recolour the log, overwrite a line with CR, or fabricate a line that looks
    like this guard's own success output.

Regression coverage in tests/plan-count-single-owner.test.cjs: a child-process
probe over both pathological shapes (catastrophic backtracking is synchronous
and would freeze the suite rather than fail one test), the bare-`/` class
shapes verified to fail against the parent-commit blob, root-confinement tests
covering the outside-file, outside-directory, cycle, broken-link and duplicate
cases, direct isInsideRoot coverage including the sibling-prefix case that a
bare startsWith would let through, sanitizeForReport coverage, and
limit-1/limit/limit+1 coverage of MAX_REGEX_LITERAL_LEN derived from the
exported constant. The earlier structural assertion was dropped — it checked
for the substring `[^/`, which respelling the class as `[^\r\n/]` defeats
while staying exponential.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

* chore(#3183): backfill changeset PR number

Restores b77931869, which a force-push during the ReDoS remediation dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012qYy4ZWif3sscQyMsup6Ma

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
 2026-08-08 01:20:55 -04:00
Tom Boucher
ef823ca9d9 fix(#2830): propagate a halted plan to its transitive dependents (#3038) 
* test(#2830): add failing regression tests for halted-plan dependent blocking

Add tests/fix-2830-halted-plan-dependents.test.cjs covering direct,
transitive (2 and 3 hop), and diamond dependents of a halted plan across
both independent "which plans are incomplete" readers (phase-plan-index's
cmdPhasePlanIndex and findPhaseInternal/searchPhaseInDir), the negative
case (an unrelated decoupled plan stays runnable), and a parity check that
the two readers agree. Uses only modules that already exist at this
commit (gsd-tools.cjs via subprocess, the pre-existing phase-locator.cjs)
so the test file loads and runs cleanly on a fresh clone of this exact
commit. These fail against current behavior: neither reader has any
concept of a halted plan or a blocked_by/runnable view yet.

* fix(#2830): a halted plan no longer leaves its dependents on the runnable work list

A plan that reaches a designed stop still writes a SUMMARY, so both
"which plans are incomplete" readers saw it as an ordinary completion and
reported its dependents as ordinary runnable work — never checking
whether an upstream plan had halted rather than finished.

- New `status: halted` frontmatter value, documented in all four SUMMARY
  templates alongside the existing `status: complete`.
- New shared src/plan-dependency-graph.cts: a single computeHaltPropagation
  pass that both phase.cts's cmdPhasePlanIndex (wave-grouping) and
  phase-locator.cts's searchPhaseInDir (the phase-location primitive, ~50
  dependent symbols across 5 command routers) now call, so the
  two-implementation divergence that caused this bug cannot recur. It
  accepts an optional precomputedOrder so cmdPhasePlanIndex — which already
  runs Kahn's algorithm in computeDependencyLevels for wave assignment —
  passes that order straight through instead of a second traversal;
  searchPhaseInDir (no prior traversal) lets the module derive its own.
  The two small duplicated predicates each reader would otherwise carry
  (is this status "halted"?, which summary file matches which plan id?)
  are centralized in the same module as isHaltedStatus/buildSummaryFileIndex.
- Additive fields only: `halted`/`blocked_by`/`runnable` on
  cmdPhasePlanIndex's plans[] and top level, `halted_plans`/`blocked_by`/
  `runnable_plans` on searchPhaseInDir's result. The pre-existing
  `incomplete`/`incomplete_plans` fields are unchanged in meaning and
  membership.
- execute-phase.md's discover_and_group_plans step now also skips any
  plan whose `blocked_by` is non-empty, reporting it by name with its
  blocking chain, in addition to (not instead of) the existing
  has_summary skip rule.

Extends tests/fix-2830-halted-plan-dependents.test.cjs (introduced in the
prior commit) with direct unit coverage of computeHaltPropagation
(including the precomputedOrder call shape) and a fast-check property
test — both only possible once this commit's new module exists.

Closes #2830

* fix(#2830): surface the halt-aware view from init execute-phase

The adopted work made phase-locator compute halted_plans / blocked_by /
runnable_plans, but cmdInitExecutePhase builds its output by explicitly
enumerating fields, so all three were computed and then silently dropped at
the exact consumer the issue names as regressed.

Forwards them additively -- incomplete_plans and incomplete_count keep their
name, type and semantics byte-for-byte -- and adds the same three empty
defaults to the roadmap-only fallback so the shape is consistent in both
branches. Covered by a new test that drives the real CLI end to end rather
than the locator function, since the locator already worked.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#2830): fail closed on dependency cycles and stop the templates inviting the defect

Three review findings, all fixed:

- BLOCKER (isolated adversarial). Cycle participants never reach indegree 0 in
  the Kahn pass, so they were excluded from the topological order, never visited
  by the forward pass, and vanished from blocked_by entirely -- i.e. reported as
  runnable. The wave-grouping reader hard-fails on a cycle so it never hit this,
  but the phase-location reader does not, so init execute-phase offered a plan
  depending directly on a halted plan. Reproduced, then fixed in the shared
  engine so every consumer is safe regardless of pre-checks: a node absent from
  the order is now blocked with a deterministic, non-empty named cause. A plan
  silently missing from both blocked_by and runnable is the exact disappearance
  this issue exists to prevent.

- MAJOR (isolated adversarial). All four summary templates showed the field as
  an inline comment on the value line. Frontmatter parsing does not strip
  trailing comments, so an executor copying the templates' own presentation
  wrote a halt that parsed as a non-halted string, silently reproducing the
  original bug. Guidance moved off the value line, and the halt predicate now
  tolerates an unquoted trailing comment.

- HARD standards violation. A test regex-matched child-process stderr prose for
  /cycle/i, which CONTRIBUTING bans. Replaced with the structured failure signal
  plus a differential assertion (same fixture without the cycle edge must
  succeed), so it stays cycle-specific without matching prose.

Also folds the duplicated read-summary-and-check-halted wrapper out of both
readers into the shared module -- centralizing only the predicate left the exact
two-copies-that-drift pattern the module exists to prevent -- and commits the
artifact-types documentation for the new status value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#2830): stop the property generator hanging the whole suite

The remote runner did not fail -- it hung. Two containers sat in this file for
31+ minutes, and an earlier attempt ran 9 hours before I killed it. The runner
passes --test-timeout=0, so nothing ever reaps it: this would have hung CI
indefinitely, not reported a failure.

Root cause: the DAG generator built edges by rejection --

  from: fc.integer({ min: 0, max: n - 1 })
  to:   fc.integer({ min: 0, max: n - 1 })
  .filter(({ from, to }) => from < to)

With n === 1 both integers are forced to 0, so the predicate is unsatisfiable
and fast-check retries value generation forever. n is drawn from 1..12 and
fast-check biases toward boundary values, so n === 1 is reached almost at once.

This also explains why the failing-first run completed normally while the fixed
run hung: before the fix the graph module did not exist, so the property test
threw on import and never reached generation. It only starts hanging once the
code under test works.

Generates the DAG by construction instead -- `to` is drawn strictly above
`from`, with the degenerate single-node case short-circuited to an empty edge
list -- so no rejection sampling is involved. Switches the import to the shared
fast-check setup so the seed and run count are pinned per CONTRIBUTING, and adds
a bounded regression guard that samples the arbitrary directly, so a future
reintroduction fails loudly instead of hanging.

Verified: the file now completes in 2 seconds, 29 tests started and 29 finished,
zero failures, against an indefinite hang before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#2830): restore the depends_on display contract and acknowledge the workflow growth

Full-suite run surfaced two things the focused harnesses could not.

1. Regression of a pinned pre-existing contract (#3785). A refactor routed the
   EMITTED depends_on field through the new dependency resolver, which also
   consults the canonical-prefix map. The original consulted the plan map only,
   so a short canonical prefix passed through verbatim -- '24-01' stayed
   '24-01' rather than becoming '24-01-auth-hardening'. The emitted field is a
   DISPLAY mapping, not the DAG resolution, and #3785 pins that. Reverted with
   a comment recording why it must not use the resolver; full resolution is
   still used for the wave DAG and halt propagation, which is what needs it.

2. The workflow file grew 518 bytes without an acknowledgment, from the
   halt-aware skip rule and the widened parse contract. Acknowledged.

Note on where the acknowledgment landed: the guidance is to add a NEW fragment,
but execute-phase.md is already named by an existing fragment and the linter
hard-fails when two ack sources name the same path. Appending to the owning
fragment, following its own established multi-PR pattern, was the only
lint-clean option.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#2830): backfill changeset pr number

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-08-04 06:47:46 -04:00
Tom Boucher
67e2ff7b25 fix(#2855): scope the phase-locator archived-milestone fallback to the active workstream (#3008) 
* test(#2855): add failing-first regression test for cross-workstream archive leak

Covers findPhaseInternal/getArchivedPhaseDirs in src/phase-locator.cts
resolving a pending workstream phase to an unrelated workstream's (or
flat-mode's) archived phase because the archive fallback hardcodes the
project-root .planning/milestones/ tree. Fails against the current
implementation; the fix lands in a follow-up commit.

* fix(#2855): scope phase-locator archived-milestone fallback to the active workstream

findPhaseInternal and getArchivedPhaseDirs in src/phase-locator.cts hardcoded
the project-root .planning/milestones/ tree when falling back to search
archived phases, ignoring GSD_WORKSTREAM. A pending phase in one workstream
whose own phases/ directory didn't exist yet would silently resolve to a
same-numbered phase archived under an unrelated workstream's (or flat-mode's)
history, complete with stale plan/summary counts and an archived status.

Route the archive fallback through planningDir(cwd) instead — the same
workstream-aware helper the active-phase search (three lines above) and the
archive-write path (archivePhaseDirectories in milestone.cts) already use.
Flat/non-workstream projects are unaffected: planningDir(cwd) with no
GSD_WORKSTREAM resolves to the same root .planning path as before.

Also switch the reported relBase/basePath from a hardcoded
'.planning/milestones/...' literal to path.relative(cwd, archivePath), so the
paths returned to callers stay consistent with wherever the archive actually
resolved to (root or workstream-scoped).

* chore(#2855): add changeset for phase-locator workstream archive fix

* fix(#2855): normalize getArchivedPhaseDirs basePath to posix separators

Orthogonal code-review finding: findPhaseInternal's relBase/directory field
was explicitly toPosixPath-normalized, but getArchivedPhaseDirs's basePath
used a bare path.relative() call, leaving it native-separator on Windows —
an inconsistency between two sibling "relative path from cwd" report fields
introduced by the same #2855 fix. Wrap basePath in toPosixPath to match, and
update the two existing assertions that compared basePath against path.join
output (which would break on Windows now that the field is guaranteed posix)
to compare against forward-slash literals instead, matching how the sibling
`directory` field is already asserted elsewhere in this suite.

* refactor(#2855): share archive-directory resolution between findPhaseInternal and getArchivedPhaseDirs

Orthogonal code-review finding: the two functions carried independent copies
of the same resolve-milestonesDir-then-enumerate-archive-dirs logic — the
exact shape that let the original #2855 bug (hardcoded root path) exist in
one copy while the workstream-aware active-phase search sat three lines
above it. Extract listArchiveVersionDirs(cwd) as the single seam both
functions now consume, so a future change to how the archive tree is located
only needs to happen once. Byte-for-behaviour preserved: readSubdirectories
and searchPhaseInDir already self-contain their own try/catch and never
throw, so moving the iteration outside the old inline try block changes
nothing observable (verified via manual repro scripts covering leak
prevention, positive resolution, flat-mode parity, and multi-milestone
reverse-sort ordering).

* test(#2855): demonstrate ROADMAP.md presence does not affect the archive-leak guard

Orthogonal code-review (spec axis) finding: issue #2855's AC1 states the
guard must hold "regardless of whether workstream A's roadmap already lists
the phase and when it doesn't yet" — an explicit two-value dimension that
had no direct test coverage; it was only inferable by reading
findPhaseInternal's source and confirming it never touches ROADMAP.md.
Add a parametrized test creating the workstream's ROADMAP.md with and
without a matching Phase heading, asserting the archive-leak guard resolves
identically (null) either way.

* chore(#2855): backfill changeset PR number to 3008

---------

Co-authored-by: sim <sim@local>
 2026-08-02 19:24:53 -04:00
Tom Boucher
5f609762ed fix(#2237): fail loud on ambiguous bare-number phase directory collision (#2262) 
* fix(#2237): fail loud on ambiguous bare-number phase directory collision

When two unrelated projects share a .planning/phases/ tree, a bare phase
number silently resolved to the first 0N-* directory found — risking
cross-project file writes. The fix detects multiple matches for the same
phase number and surfaces an ambiguous_matches result instead of silently
taking the first.

Changes:
- src/phase-locator.cts: searchPhaseInDir uses filter() + ambiguity check
- src/phase.cts: cmdFindPhase same pattern
- src/init.cts: cmdInitPhaseOp surfaces ambiguous_matches in the result
- tests/phase-locator.test.cjs: 3 regression tests

* docs: backfill changeset PR number (#2262)

* merge: keep up to date with next

* fix: regenerate stale capability-registry after next merge
 2026-07-14 15:17:14 -04:00
Tom Boucher
8c3d934a90 refactor(#1294): T-final — delete the core.cjs re-export spine (epic #1267 complete) (#1295) 
* refactor(#1294): T-final — delete the core.cjs re-export spine (epic #1267 complete)

After T0–T6 nothing imports core, so retire the spine and its scaffolding:
- delete src/core.cts (and the gitignored gsd-core/bin/lib/core.cjs artifact;
  remove its .gitignore + eslint-ignore entries)
- delete scripts/lint-core-spine-imports.cjs + its allowlist; drop it from the
  package.json lint:ci chain
- regenerate docs/INVENTORY-MANIFEST.json (drops the core.cjs surface)
- sweep stale references: CONTEXT.md glossary back-compat clauses (spine retired,
  callers import the leaf directly), planning-config.md CONFIG_DEFAULTS owner,
  and false present-tense core.cjs claims in leaf-module docstrings

The ADR-857 decomposition is complete: the former Core god-module is fully
dissolved into its leaf modules; no re-export spine remains. No behaviour change.

Closes #1294

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(#1294): migrate the computed-path core.cjs importers the literal grep missed

bin/install.js used require(path.join(_gsdLibDir, 'core.cjs')) (a computed
path, and bin/install.js was never in the convergence lint's scan roots), and
~8 test files referenced core.cjs via path.join/readFileSync/existsSync/FILE_ARG
forms the literal-string migration grep missed. Route install.js's symbols to
their leaves (RUNTIME_PROFILE_MAP->model-catalog, resolveTierEntry/EFFORT_SET->
model-resolver) and repoint/adjust the test references to the leaves. Recovers
the 161 'Cannot find module core.cjs' failures from the spine deletion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
 2026-06-15 18:50:46 -04:00
Tom Boucher
dd81e3d120 refactor(#881): extract phase-locator fs-search into phase-locator.cts (#882) 
ADR-857 rollout phase 2d. Move the phase-directory search/location functions
(searchPhaseInDir, findPhaseInternal, getArchivedPhaseDirs) + their interfaces
(PhaseSearchResult, ArchivedPhaseDir) out of core.cts into a new module
src/phase-locator.cts. core.cts re-exports all three (callers unchanged).

Cycle-free: phase-locator depends only on leaves (phase-id for token/name
matching, core-utils for fs-scan/path helpers, planning-workspace for
planningDir) — unblocked by the core-utils leaf (2c). This completes the
phase-search split: parsing in phase-id (2a), fs-search in phase-locator.

New-CLI-module checklist done (.gitignore, eslint, INVENTORY 94->95 + row,
manifest, ARCHITECTURE, CONTEXT.md "Phase Locator Module"). Adds
tests/phase-locator.test.cjs (37 tests: behavioral + shim-identity +
adversarial phase-dir fixtures).

Gates: lint, code-review, security-review, codex adversarial-review (0
findings; verbatim move checksum-verified). Mac 4078 pass; clean-build docker
12972 pass, 0 fail.

Closes #881

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
 2026-06-08 14:38:26 -04:00