refactor(#3186): one shared phase-completion predicate, disk-strict (#3306)

* docs(#3186): record the disk-strict completion decision in ADR-3180 7.4

The maintainer decided #2957 on 2026-08-08: disk state is authoritative and a
ROADMAP checkbox is a human annotation with no machine authority. Section 7.4
still carried the OPEN QUESTION and was marked blocked, so the contract said one
thing and the tracker another.

Recorded per section 7's own rule - a behavior not stated there is not decided,
and amending a rule is an ADR amendment rather than a code change with a comment.
The decision comment names Phase 4's PR as the carrier of this edit and makes it
an acceptance criterion that the text be in the tree before implementation
begins, so this lands first, alone, ahead of any code.

Also clears the stale blocked-on-2957 row in the guard roster.

* refactor(#3186): one shared phase-completion predicate, disk-strict

isPhaseComplete in verification.cts becomes the single owner. It calls
readVerificationStatus UNCONDITIONALLY - plan count is not a precondition - so a
zero-plan phase with a passing VERIFICATION.md is complete. That is #3168: init
gated the read on a plan count and synthesized a not_required sentinel, so
phase.complete succeeded while init.manager reported incomplete for the same
phase.

The guard, built and run before scope was fixed per Amendment 3, found 9
re-derivations where the ADR named 3. Four were unnamed, including one in the
prompt layer: mvp-phase.md ORed a ticked checkbox with disk status, which under
disk-strict is the divergence itself.

Per the #2957 decision, a ticked ROADMAP checkbox is a human annotation with no
machine authority. The overrides in roadmap analyze and init manager are deleted
rather than generalized; the user's checkbox stays in ROADMAP.md, only its
authority goes.

scanPhasePlans.completed and buildWorkstreamInventory are deliberately NOT folded
- they answer 'are all plans summarized', which is a different question, and
folding them would either over-report completion or invert the dependency
direction between Phase 1's owner and this one.

Verified on the remote runner.

* fix(#3186): close seven review findings and record the missing-verdict rule

The isolated review reproduced a write-path regression I introduced: migrating
cmdRoadmapUpdatePlanProgress dropped its summaryCount>=planCount gate, so a phase
with a fresh passing verification plus a newly-added unsummarized plan reported
complete AND wrote a checkbox into ROADMAP.md while phase complete refused. The
owner stays right per 7.4 - plan count is not a completion precondition - so the
gate is restored at the write site as an explicit composition, mirroring the
separate 2648 unexecuted-plan gate cmdPhaseComplete already carries.

The spec axis was right that my 0.x-split reasoning was too permissive. The 2957
decision names buildStateFrontmatter as one of the three that must converge, and
buildWorkstreamInventory combined a summaries-met local with verification data to
decide the same verdict - Decision 4(c)'s named bypass, and it reproduced 3168 in
a third surface. Both now route through the owner. The raw scanPhasePlans helper
stays: it answers are-plans-summarized, which genuinely is a different question.

Maintainer decision recorded in 7.4: a missing verdict is not a passing one, so
an absent VERIFICATION.md means not complete everywhere. That retires 2645's
verifier-disabled tolerance and inverts its Goodhart incentive - deleting the
evidence now lowers completion instead of raising it.

Guard hardened: block-form count gates and algebraic restatements are caught, and
the header now discloses its remaining limits instead of overclaiming.

Verified on the remote runner.

* fix(#3186): route state sync through the owner and catch bare completed reads

The matrix found 52 failures. 51 were fixtures asserting the old semantics: a
phase with plans and summaries but no VERIFICATION.md used to count complete and
correctly no longer does. Each fixture now carries a passing verification where
that is what the test was actually about, rather than having its assertion
weakened.

The 52nd was a real 10th re-derivation the guard could not see. cmdStateSync
destructured scanPhasePlans().completed directly - a bare field read, not a
comparison - and used it as a completion verdict, so state sync and state json
disagreed on completed_phases for identical disk state. Routed through the owner.

Guard gains shape (d): any read of .completed off a scanPhasePlans() result
outside plan-scan.cts, in chained, destructured and indirect forms, function
scoped with no line window. It cannot tell a summaries-met read from a completion
read - that is data flow - so it flags every one and requires a written-reason
exemption, which is the same discipline shapes a-c already use. The blind spot is
disclosed in the header rather than overclaimed.

The emitted-attribution failure was also mine, not pre-existing: the mvp-phase.md
checkbox-OR removal moves emitted bytes, acknowledged in tests/emitted-drift-acks.

Verified on the remote runner.

* test(#3186): give the nested-plans sync fixture a passing verification

Last 3 matrix failures were one failure echoing up two describe levels. Phase
01-alpha had plans and summaries but no VERIFICATION.md, so under disk-strict
completed stayed 0 and no Progress change was emitted - correct new behavior, not
a regression.

Added the passing verification rather than dropping the Progress expectation, so
the test still covers what #3257 is about: that a nested plans/ layout is counted
and not undercounted. Probe against the built lib confirms
Progress: 0% -> 50% alongside Total Plans in Phase: 0 -> 3.

* chore(#3186): backfill changeset PR number

pr:0 placeholder replaced with the real number now that #3306 exists.

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-10 10:18:29 -04:00
committed by GitHub
parent 96a82bbffb
commit e201cde73c
25 changed files with 2849 additions and 207 deletions

View File

@@ -721,6 +721,8 @@ Interactive command center for managing multiple phases from one terminal.
/gsd-manager --analyze-deps # Scan ROADMAP phases for dependency relationships before parallel execution
```
**Phase completion is disk-strict (ADR-3180 §7.4, issue #3186).** A phase's status here — and in `roadmap analyze`, `roadmap update-plan-progress`, and `phase complete` — is decided by one rule: a passing `*-VERIFICATION.md` on disk, checked unconditionally (plan count is never a precondition, so a zero-plan phase with a passing verification reports complete). A ticked `- [x]` checkbox in `ROADMAP.md` is a human annotation only; it carries no machine authority and is never consulted for these commands' completion verdicts. `roadmap update-plan-progress` additionally withholds writing the checkbox/completion date while any plan in the phase has no matching `*-SUMMARY.md`, mirroring `phase complete`'s own coverage gate.
**Checkpoint Heartbeats (#2410):**
Background `execute-phase` runs emit `[checkpoint]` markers at every wave and plan

View File

@@ -278,7 +278,7 @@ This section is that written rule. It is **normative**, and it is what the guard
**Note on #3204.** Routing `buildStateFrontmatter` through the owner will not by itself fix #3204: its defect is the discriminator one layer *above* enumeration — "is the ROADMAP's phase count safe to trust" — which misclassifies ordinary `## Overview` / `## Progress` headings as milestone sectioning. That discriminator **is** §7.1's `isMilestoneBoundedInRoadmap`. The enumeration routing and the discriminator replacement must ship together or the defect survives the consolidation.
#### 7.4 Phase completion — *Required — Phase 4, blocked*
#### 7.4 Phase completion — *Required — Phase 4 (decided: disk-strict)*
**Question.** Is phase `P` complete?
@@ -286,9 +286,29 @@ This section is that written rule. It is **normative**, and it is what the guard
**Rule.** `readVerificationStatus` is called **unconditionally**. Plan count is not a precondition: a phase with zero plans and a passing `*-VERIFICATION.md` is complete. The read path and the write path share this predicate, so "`phase.complete` succeeds while `init.manager` reports incomplete" is unrepresentable for identical input.
**OPEN QUESTION — does a ROADMAP checkbox override disk state? (#2957).** There are **three** completion implementations, not the two the epic recorded: `cmdPhaseComplete`, `buildPhaseCompletionProjection`, and `buildStateFrontmatter`, which computes completed phases from plan scanning alone and never consults the ROADMAP checkbox that `cmdRoadmapAnalyze` deliberately honors. Checkbox-override versus disk-strict is a **product decision**, and it is not made here.
**DECIDED — disk state is authoritative; a ROADMAP checkbox does not override it (#2957, maintainer decision 2026-08-08).** This replaces the OPEN QUESTION this section previously carried, per §7's own rule that a behavior not stated here is not decided.
**Forcing function.** Phase 4's drift guard fails while more than one completion predicate exists. It cannot be satisfied by consolidating two of three and leaving the third, and Phase 4 must not ship before #2957 is decided — a shared predicate that silently adopts whichever semantics its author happened to hold is a product decision made by typing order.
There are **three** completion implementations, not the two the epic recorded: `cmdPhaseComplete`, `buildPhaseCompletionProjection`, and `buildStateFrontmatter`, which computes completed phases from plan scanning alone and never consults the ROADMAP checkbox that `cmdRoadmapAnalyze` deliberately honors. The resolution:
1. **A ticked ROADMAP checkbox is a human annotation with no machine authority.** `cmdRoadmapAnalyze`'s deliberate honoring of it is the **divergent** behavior here, and it is **removed** rather than generalized.
2. `buildStateFrontmatter`'s existing plan-scanning semantics therefore become **canonical**, and all three implementations route through the one predicate.
3. The shared predicate derives completion from plan and verification state **on disk**, per the Rule above — `readVerificationStatus` unconditionally, plan count not a precondition.
**Why disk-strict.** A stale tick asserting completion over contradicting disk state is precisely the confidently-wrong answer this epic exists to remove, and it is the same failure shape as the `100`-percent aggregate (#3161): a well-formed, plausible value that no caller can distinguish from a real one.
**Tier-2 consequence (Decision 3).** A phase marked complete *solely* by a ticked checkbox — no passing `*-VERIFICATION.md`, plans outstanding — **stops reporting complete from `roadmap analyze`**. The break is deliberate and ships with its own breaking-change call-out, changeset fragment and docs update in Phase 4's PR.
**A missing verdict is not a passing one (decided 2026-08-10, maintainer).** `isPhaseComplete` is `verification.status === 'passed'`, so an **absent** `*-VERIFICATION.md` means **not complete** — everywhere, including `workstream list` / `status` / `progress`.
This retires #2645's deliberate boundary, which kept `missing` / `unknown` / `stale` out of the failing set specifically so a verifier-disabled project would not report 0% forever. Recorded rather than absorbed silently, because the consequence is real: **a project that never runs the verifier now reports its phases incomplete.**
It also closes #2645's Goodhart hole from the opposite side. That issue's symptom was that *deleting* a `*-VERIFICATION.md` **raised** the reported percentage — the metric could be improved by destroying the evidence. Under disk-strict, deleting it **lowers** completion, so the incentive inverts and the ledger's deletion-memory role is no longer load-bearing.
**Forcing function.** Phase 4's drift guard fails while more than one completion predicate exists. It cannot be satisfied by consolidating two of three and leaving the third.
**Consumers that must route through the owner.** `cmdPhaseComplete`, `buildPhaseCompletionProjection`, `buildStateFrontmatter` (the three #2957 names), plus `cmdRoadmapAnalyze`, `cmdInitManager`, `cmdRoadmapUpdatePlanProgress`, `buildWorkstreamInventory`, and the prompt layer's `mvp-phase.md` — **nine re-derivations found by the guard where this section named three.**
**A write-path gate is not a second predicate.** `cmdRoadmapUpdatePlanProgress` writes a completion checkbox, and completion alone does not mean every plan was executed — `readVerificationStatus`'s staleness check compares summary mtimes, never plan count. It therefore ANDs the owner's verdict with an explicit plan-coverage gate, mirroring the separate #2648 unexecuted-plan gate `cmdPhaseComplete` already carries. That composition is sanctioned; re-deriving completion beside it is not.
#### 7.5 Live-plan counting — *Enforced (Phase 1), with a known representation gap*
@@ -377,7 +397,7 @@ One row per derivation. A blank owner is a derivation whose contract is locked (
| Milestone windowing (§7.1) | `roadmap-parser.cts` | `lint-milestone-window-drift.cjs` | `src/` | enforced |
| Milestone identity (§7.2) | `roadmap-parser.cts` (Phase 6) | same guard, token set widened by Phase 6 | `src/` | enforced |
| Phase enumeration (§7.3) | `phase-locator.cts` | `lint-phase-enumeration-drift.cjs` | `src/` | enforced — the state writers included (#3185) |
| Phase completion (§7.4) | `verification.cts` (Phase 4) | Phase 4 | `src/` | blocked on #2957 |
| Phase completion (§7.4) | `verification.cts` (Phase 4) | Phase 4 | `src/` | contract decided (disk-strict, #2957); migration is Phase 4 |
| Live-plan counting (§7.5) | `plan-scan.cts` | `lint-plan-count-drift.cjs` | `src/` | enforced |
| Live-plan counting, prompt layer (§7.5) | — (Phase 8) | `lint-planning-prompt-drift.cjs` | `gsd-core/workflows`, `commands`, `agents`, `skills` | ratcheted, 7 sites |
| Completion ratio (§7.6) | `phase-lifecycle.cts` | `lint-completion-ratio-drift.cjs` | `src/` | arithmetic + rule 3 enforced; rule 4 is Phase 7 |