* fix(#2562): scope workstream progress/status to the current milestone
`workstream progress` / `workstream status` / `workstream list` share one
derivation that could report a workstream's CURRENT milestone as
"milestone complete" / 100% while phases in that milestone were unstarted,
in progress, or failing verification. Three coupled defects:
1. The shipped signal was project-lifetime, not milestone-scoped:
workstreamMilestoneShipped() returned true if ANY *-ROADMAP.md snapshot
existed or "SHIPPED" appeared anywhere in ROADMAP.md. Every prior shipped
milestone leaves a permanent collapsed <summary>✅ … SHIPPED</summary>
block, so any post-v1.0 workstream was pinned to "milestone complete"
forever (over-correction from #1913).
2. The denominator dropped declared-but-unscaffolded phases, and completed
PRIOR-milestone phase directories inflated the numerator, letting
progress_percent round to 100 while real work remained.
3. Phase completeness ignored the VERIFICATION verdict — SUMMARY >= PLAN
count alone marked a phase complete even with a human_needed verdict.
Fix: derive both numerator and denominator from artifacts scoped to the
current milestone. The current version comes from the workstream STATE.md
`milestone:` field (ROADMAP in-progress markers can be stale); the ROADMAP
`## Progress` table maps every phase — including dirless ones — to its
milestone, and the matching set is both the denominator and the directory
membership filter. The shipped signal now requires the CURRENT version's
archived ROADMAP snapshot (REQUIREMENTS snapshots are not accepted; they can
be written at milestone start) or the current milestone's own line marked
shipped. Phases with an explicit failing verdict (gaps_found/human_needed)
count as in_progress; missing/unknown/stale are left untouched so
verifier-disabled projects do not regress to never-complete.
Greenfield roadmaps with no versioned Progress table, and projects whose
current version cannot be determined, keep the prior behaviour.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(#2562): add changeset for workstream milestone-scoping fix
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(#2562): parse the Progress table via findTableWithColumns
The ad-hoc pipe-table regex tripped the local/no-adhoc-markdown-parsing
ESLint rule. Use the canonical markdown-table helper instead: the
milestone-grouped RoadmapProgress variant is located by its required
`Phase` + `Milestone` columns and cells are addressed by column NAME,
so the parser tolerates column reordering and injected columns. The
`flat` variant (no Milestone column) yields no attribution, which is
the intended fallback to legacy counting.
Behaviour is unchanged: verified against a real multi-workstream project
(same status/percent/phase and plan counts before and after).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#2562): count table-only phases in the unscoped denominator
Addresses the reporter's repro detail: a phase declared as a `## Progress`
table row with no `### Phase N` heading is missed by countRoadmapPhases
EVEN WHEN other headings exist — the heading regex counts 1 for a
"1 heading + 1 table-only" roadmap — not just in the zero-heading fallback
path. Milestone scoping did not cover this, because a flat Progress table
(no Milestone column) carries no per-phase attribution, so greenfield and
single-milestone projects kept the old heading-only denominator and the
declared phase silently vanished from it.
When milestone scoping cannot engage, the denominator is now the union of
the Progress table's declared phase numbers and the phase directories, so
neither source can shrink it. Verified against the reporter's minimal
fixture (phase 1: 1 PLAN + 1 SUMMARY + gaps_found; phase 2: table row only,
no heading, no dir), which now reports 0/2 at 0% across all four
table/STATE permutations instead of 1/1 at 100%.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(#2562): attribute dir-only sub-phases to their parent's milestone
A sub-phase directory inserted mid-milestone (e.g. `30.1-…` under a
table-declared phase 30) usually has no ROADMAP Progress-table row, so it
had no milestone attribution and was scoped out of the rollup entirely —
its completed work was invisible and it could never hold the percentage
below 100.
It now inherits its parent phase's milestone and joins BOTH sides of the
calculation. Both sides is the load-bearing part: adding it to the
numerator alone would let completed_phases exceed a denominator that never
counted it, cap back to 100% via Math.min, and reintroduce exactly the
defect this issue reports. A regression test pins that failure mode (all
declared phases complete + an in-progress dir-only sub-phase → 75%, not
100%).
Attribution is deliberately one-directional: a sub-phase counts only when
its PARENT is in the current milestone, so a follow-up created in a later
milestone under an older parent is excluded rather than misattributed —
conservative (under-count) rather than falsely inflating.
Verified on a real project: the reported workstream moves from 2/6 (33%)
to 3/7 (43%), the 3/7 being the honest figure — a completed sub-phase that
was previously invisible now counts, and so does its plan total.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(#2562): describe the denominator + sub-phase fixes in the changeset
The fragment was written at the first commit and only covered the three
original defects. Bring it up to date with what actually ships: the
table-only-phase denominator union (heading-only counting dropped a
declared phase even when other headings existed) and sub-phase milestone
inheritance across both sides of the calculation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(#2562): promote the canonical phase-key surface to phase-id
state.cts kept `phaseKeyFromToken`/`phaseKeyFromDir` private, so every other
module that had to compare two independently-derived phase references — a
ROADMAP table cell against a phase directory, say — wrote its own regex. That
is the defect class #2562 reports: a padded `01` and an unpadded `1-slug` land
in different key spaces and the comparison silently yields nothing.
Move the pair to the phase-id owner module and add `phaseKeyFromProse` (for
ROADMAP/STATE prose, markdown emphasis stripped) and `parentPhaseKey` (a
sub-phase's parent). state.cts imports them; its call sites are unchanged.
* fix(#2562): own milestone-shipped detection and accept a workstream scope
Three changes to the module that owns milestone parsing, so its consumers stop
reimplementing it:
- `isMilestoneShippedInRoadmap(content, version)` answers "does the ROADMAP mark
THIS milestone shipped" from heading and `<summary>` lines only. A bullet that
merely names the version (`- [x] 03-01: ship the v2.0 login endpoint`) is prose
about a phase, not a milestone verdict. The version token is boundary-matched
with `(?![\w.-])` — `\b` does not bound it, since `.` is a non-word character,
so a shipped `v2.0.1` heading would otherwise close `v2.0`.
- `extractCurrentMilestone` and `getMilestonePhaseFilter` take an optional
trailing workstream name and thread it to `planningDir(cwd, ws)`. A caller
iterating workstreams cannot set `GSD_WORKSTREAM` per iteration, which is what
the existing resolution falls back to. Omitted, resolution is unchanged.
- `getMilestonePhaseFilter` exposes `versionScoped`, true only when the phase set
really is one milestone's. On an unversioned roadmap `phaseCount` spans the
project's lifetime and must not be read as a current-milestone denominator.
The closed/active milestone-marker patterns were kept in three byte-identical
copies; they are hoisted to module scope as one `isClosedMilestoneHeading`.
* fix(#2562): derive membership and denominator from one phase-key space
The milestone scoping added earlier in this PR derived the ROADMAP table key and
the phase-directory key with two different regexes, and dropped rows it could not
attribute. Each of those was another way to reproduce the symptom this issue
reports — a rollup contradicting its own `phases[]` listing:
- a padded `| 01. … |` row never matched a `1-slug` directory (and a bespoke
`^0*(\d+…)` never matched `PROJ-05-…` at all), so phases fell out of the
milestone entirely and the percentage collapsed or pinned;
- a blank or malformed Milestone cell deleted the phase from BOTH sides, letting
an unstarted phase vanish and the remainder round to 100%;
- shipped detection scanned bullets, so any checkmarked line naming the version
closed the milestone;
- the numerator counted per-directory while the denominator counted distinct
phases, so a stale same-numbered directory (Bug #2445's scenario) pushed
`completed_phases` past the denominator, where `Math.min` capped it to 100%
and hid the unstarted phase.
Both sides now key off the phase-id owner module (`phaseKeyFromDir` /
`phaseKeyFromProse`), directory membership additionally consults
`getMilestonePhaseFilter` when that filter is genuinely version-scoped, and the
denominator is the union of the roadmap's declarations with the member
directories' keys — so `completed_phases <= denominator` holds by construction.
The Builder asserts it and throws; the `Math.min` cap survives only on the legacy
unscoped path, where the denominator is a heading count that cannot bound the
numerator. An unattributable row degrades over-inclusively (kept, never dropped),
matching the degrade direction roadmap-parser already commits to.
* test(#2562): boundary coverage for each milestone-scoping reproduction
One test per way the scoping could still report "milestone complete"/100% while
phases are incomplete: zero-padded rows vs padded dirs (and the mirror),
project-code-prefixed dirs, a blank/malformed Milestone cell, a checkmarked
bullet naming the version, a shipped `v2.0.1` heading against a current `v2.0`,
and a stale same-numbered directory. Plus the current milestone's own shipped
heading (the signal must survive the boundary fix), the Builder's
numerator-above-denominator throw, a parity check that every non-`passed`
verifier status blocks completeness, and a guard that scoping reads the
workstream's ROADMAP rather than the project root's.
Reverting only `src/` reddens six of them.
* docs(#2562): record the milestone-scoped semantics and its consumer impact
CONTEXT.md: the Workstream Inventory Module's completion fields now describe the
current milestone, not the workstream's lifetime; phase-id owns the canonical
phase-key surface; roadmap-parser owns milestone shipped/active classification
and takes an optional workstream scope.
Changeset: name the behaviour change explicitly — `roadmap_phase_count`,
`completed_phases` and `progress_percent` change meaning with no schema signal,
and `getOtherActiveWorkstreamInventories` filters on the derived status, so
consumers see real movement.
* fix(#2562): collapse every zero-padding spelling to one phase key
A property test over the key surface — table cell and directory decorated
INDEPENDENTLY, which is the point — found a divergence neither review named:
`padStart(2, '0')` is a no-op once the input is already ≥2 characters, so `5`
normalised to `05` while `005` stayed `005`. A `| 5. … |` row and a `005-slug`
directory therefore never compared equal, which is the same failure mode as the
padded-vs-unpadded blocker, one level down.
The strip belongs in `phaseKeyFromToken`, not in `normalizePhaseName`: applying
it to the latter regressed multi-decimal leading-zero plan IDs (`001.10-PLAN.md`
capture + wave assignment), which rely on its verbatim rendering. Confining it
to the key surface fixes the comparison and leaves rendering untouched.
Also tightens `isMilestoneShippedInRoadmap`'s patterns to anchored,
complementary character classes so an untrusted ROADMAP cannot drive
backtracking, and makes the project-code test discriminating — it previously
passed pre-fix, because an unresolvable key collapsed scoping to the whole
roadmap and happened to land on the same number. It now carries a
prior-milestone directory that a collapse would wrongly admit.
* fix(#2562): prefer the milestone-attributing Progress table; pin the seams
Three gaps the earlier self-check missed:
- Both RoadmapProgress variants carry a `Plans Complete` column, so probing it
first picked a FLAT table appearing earlier in the document over the
milestone-grouped one that actually carries the attribution. Every row came
back unattributed, was treated as current-milestone, and silently re-admitted
prior-milestone phases. The attributing shape is probed first; flipping the
order reddens the new test.
- `lint-phase-id-drift` exempts phase-id.cts by design, so it is silent on
`phaseKeyFromToken`'s own segment strip by construction — not evidence. Its
interaction with `stripProjectCodePrefix` (which runs AFTER) is pinned across
project codes and hyphenated ids, including the pre-existing `M1-46-6` vs
`M1-46-6-rs` asymmetry, which is `extractPhaseToken`'s #2043/#2232 slug-word
rule and not something to "fix" by accident.
- `listWorkstreamInventories` loops every workstream with no try/catch, so a
REACHABLE Builder-invariant throw would take down `workstream list`/`status`/
`progress` for all of them. The invariant test only exercised the pure Builder
with hand-built inputs. A test now drives `inspectWorkstream` over every
adversarial shape at once (prior-milestone dirs, three colliding duplicates,
a dirless declaration, an unattributed row, a project-code prefix, a dir-only
sub-phase) and asserts it does not throw and the invariant holds — so the
throw stays a contract assertion for external callers, not a runtime path.
Also covers the active-marker-wins rule (`## v2.0 — 🚧 IN PROGRESS … ✅` must not
mark shipped), which nothing exercised.
* fix(#2562): scope a declared-but-empty current milestone instead of falling back to history
The review's open MAJOR. `STATE.md`'s `milestone:` field updates the moment
`/gsd-new-milestone` writes the heading, while the `## Progress` table and phase
sections land later. In that window nothing attributes a phase to the current
milestone, `scoped` went false, and the fallback counted the project's ENTIRE
phase history as both numerator and denominator — a milestone with zero work
done reported 100% off its predecessors'. That is #2562's own symptom reached by
a different precondition, and none of the 16 tests covered it.
Reproduced first, four ROADMAP shapes, at `inspectWorkstream` rather than the
Builder — the Builder takes the scoping decision as an input, so a Builder-level
test proves it honours a flag, not that the derivation sets it. Three of the
four reported 2/2 100% with no phase of the current milestone begun.
Which signal witnesses the state depends on the ROADMAP's shape, and no single
one covers all three:
- `## v3.0` exists but declares no phases. `getMilestonePhaseFilter` DOES locate
the section and sets `versionScoped`, then the zero-phase pass-all degrade
resets it to false — erasing the only evidence the milestone exists. Neither
existing flag survives that path, so this adds `versionSectionFound`, set
beside `versionScoped` and deliberately preserved through the degrade.
- No section for this version at all, in a roadmap that versions its others —
the existing `missingExplicitVersion`, already exposed and tested.
- Unversioned headings, but a Progress table attributing every row elsewhere:
neither filter flag fires and the table is the only witness.
A ROADMAP that attributes NO versions anywhere matches none of them, which is
the point. Its rows parse with `version: null`, land in `currentMilestoneKeys`,
and never reach the new branch. `readCurrentMilestoneVersion` returns a non-null
version for very nearly every project (`getMilestoneInfo` defaults to `v1.0`),
so keying off `currentVersion` alone would have zeroed out every free-form
legacy project — the condition looks fussy for that reason. A test pins it.
Within an empty milestone, membership inverts: a directory belongs unless
another milestone's row claims it. Excluding everything would have dropped a
phase scaffolded before the roadmap caught up from BOTH sides of the rollup, and
hiding real work is the same class of defect as inventing it — this codebase
degrades over-inclusive, never under.
Scoping is now stated by the caller (`milestoneScoped`) rather than inferred
from `currentMilestonePhaseCount > 0`. That inference was the root cause: it
cannot represent a milestone that is scoped AND legitimately zero-phase, so the
Builder read "no phases yet" as "no scoping" and reopened the whole-history
path. The count-derived value stays the default for callers that say nothing.
A regression test also pins that a zero denominator does not trip the Builder's
`completed_phases <= denominator` throw, since `listWorkstreamInventories` has
no try/catch and a crash on every freshly-declared milestone would be worse than
a wrong percentage.
The changeset and CONTEXT.md no longer claim membership is derived in "ONE" /
"a SINGLE" phase-key space. `getMilestonePhaseFilter` still runs its own
`normalizePhaseIdSegments`; the signals are OR'd so a divergence can only widen
membership, but two normalisers coexist and the docs now say so.
* fix(#2562): cross-validate the shipped marker against the milestone's artifacts
`status: "milestone complete"` was asserted from the shipped marker alone, so a
single payload could report it beside `progress_percent: 67` — this issue's own
symptom, reached through `status` rather than the percentage.
The marker is now a claim checked against the milestone's own artifacts, and the
two signals are checked at DIFFERENT strengths because one check cannot serve
both. A `heading` marker (operator-typed, live ROADMAP) is refused on a short
completion ratio, which also catches phases declared but never scaffolded. A
`snapshot` marker is NOT ratio-gated: `milestone complete` moves the milestone's
phase dirs into `milestones/<version>-phases/` (milestone.cts:755-762) while
copying — never truncating — the live ROADMAP (:671-674), so a correctly
archived milestone reads 0/N by construction and a ratio gate would strip
`milestone complete` from every archived milestone. It is refused instead when
an in-milestone phase dir is still live and unfinished, reachable because
`milestone complete` does not advance STATE's `milestone:` field
(state-transition.cts:1335 vs :1224). `legacy` stays ungated — only reachable
when scoping is off.
A refused marker does not fall through to a STATE field claiming the same thing;
against contradicting artifacts neither source may report completion. The
refusal surfaces as `milestone_shipped_unverified` rather than staying silent,
distinct from `status_conflict` (derived-vs-field only).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* test(#2562): pin both marker strengths, the archived guard, and the owner modules
workstream-inventory: four tests, all four red against the prior src and green
with it. A live-ROADMAP SHIPPED heading over an incomplete milestone is refused;
an archived snapshot SURVIVES its phase dirs being moved out (the regression the
obvious single ratio-gate would cause — swapping the snapshot branch to that
gate reddens this AND the pre-existing `CURRENT-version snapshot marks the
milestone complete` at :321); an archived snapshot is refused once a phase is
reopened under it; and a refused marker is not re-asserted by a STATE field
claiming the same.
roadmap-parser: `isMilestoneShippedInRoadmap` gets unit coverage at its owner
module rather than only through the inventory that consumes it, plus two
characterisation tests for `getMilestonePhaseFilter`'s legacy call surface —
omitting the new trailing `ws` param is indistinguishable from `undefined`/`null`,
and the `GSD_WORKSTREAM` env fallback still resolves. These characterise the
call surface; they do not stand in for coverage of its individual callers.
phase-id: the `phaseKeyFrom*` / `parentPhaseKey` one-key-space contract, incl. a
property that padding a directory number never changes its key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* docs(#2562): record the two-strength shipped cross-check + milestone_shipped_unverified
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* fix(#2562): refuse an archived snapshot on a DIRTY archive, not just a live-unfinished dir
The snapshot arm checked `liveIncompletePhases > 0`, which misses the shape
@davesienkowski reproduced: a COMPLETE live dir beside a phase declared in the
Progress table with no directory. Nothing is live-and-unfinished, the marker
sails through, and `cmdWorkstreamProgress` returns
`{"status":"milestone complete","progress_percent":50}` — the reported symptom
verbatim, from one payload. Reproduced at 483a3ba30 before changing anything.
His diagnosis is the right one and better than mine: an in-milestone directory
outliving the archive means the archive is not CLEAN, and once that is true the
completion ratio is meaningful again. So the check is the conjunction — any live
in-milestone dir AND `completedPhases < effectivePhaseCount`. That strictly
subsumes the old predicate (an incomplete member dir is in the denominator and
not the numerator, so the ratio is always short when one exists) and leaves the
clean-archive guard green, since a clean archive has no live dirs at all.
Also corrects the module comment: the `scoped &&` prefix ungates all three
signals, not just `legacy`. That is correct behaviour — unscoped, the
denominator is the whole-roadmap count and membership is everything, so there is
no current-milestone artifact set to check a current-milestone claim against —
but the comment claimed otherwise. And the `milestone.cts` citations were ~28
lines stale after the rebase; they are now :700-702 (copy) and :783-790 (move),
re-verified against this head.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* fix(#2562): project milestone_shipped_unverified from list, status and progress
The inventory carried the field and every renderer dropped it — `workstream.cts`
was not in this PR's diff at all — so at the CLI a refused marker looked exactly
like no marker: a fallback `status` and nothing saying one was seen and rejected.
That is the silent collapse this issue is about, reintroduced one layer up, and
it made the changeset's "visible rather than silent" claim false at every
surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* test(#2562): pin the dirty-archive shape and the CLI projection
Five tests, all five red against the prior src and green with it.
The reviewer's repro at the builder: an archived snapshot with a COMPLETE live
dir beside a dirless declared phase must be refused, and status must not
contradict the percentage.
Four at the CLI via runGsdTools, the surface that was dropping the field rather
than the builder that already had it: `workstream progress`/`status`/`list` each
project `milestone_shipped_unverified: true` for that workstream, and a clean
archive still reports `false` with `status: "milestone complete"`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
* docs(#2562): correct the snapshot check, the scoped-only caveat and the CLI claim
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpFzuEHKTN1jaypSN44rzd
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
137f3fbb9c