From 77dbfb961d9748b540c3a98c4dcd8a7e4cf933df Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 2 Aug 2026 23:43:32 -0400 Subject: [PATCH] fix(#2645): persist verification verdicts so deletion cannot raise completeness (#3016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2645): pin failing-first regression coverage for verification-deletion ledger Deleting a *-VERIFICATION.md file after a failing gaps_found/human_needed verdict was recorded silently raises reported workstream completion — the deletion is indistinguishable from "verifier never ran" at the verdict-lookup layer. These tests pin the correct behavior (deletion must never raise completed_phases/progress_percent) and fail against the current code, which has no such protection. * fix(#2645): persist verification verdicts across deletion in workstream rollup FAILING_VERIFICATION_STATUSES gated phase completeness on a verdict read fresh from *-VERIFICATION.md on every call. Deleting that file made "verifier found gaps, report later deleted" indistinguishable from "verifier never ran" (both read the internal 'missing' sentinel), so removing evidence silently raised completed_phases/progress_percent. Persist the last real verdict observed per phase key in a ledger file at the workstream directory level (outside every phase directory, so the same deletion that triggers the hole cannot also erase the memory of it). The ledger is consulted only when a live read comes back 'missing' and is updated whenever a real verdict is observed, so a genuinely re-verified phase is never permanently pinned, and verifier-disabled or not-yet-verified phases are unaffected (no ledger entry is ever created for them). Ledger-winner selection walks phase directories in the same sorted order buildWorkstreamInventory's own duplicate-directory tie-break uses, so a stale same-numbered directory can never clobber the live directory's remembered verdict on an exact mtime tie. Adds fault-injection coverage for the ledger read/write per CONTRIBUTING.md's filesystem-writes rules. * fix(#2645): redesign verification ledger to fail closed, not open The two-state ledger read (any read/parse failure -> "nothing remembered") failed OPEN: deleting the ledger alongside the report, or simply corrupting it while the report was already gone, degraded to the same 'missing' sentinel this issue exists to stop trusting -- silently reopening the completion-inflation hole one level up. Redesigned as a three-state read distinguishing 'absent' (no ledger file at all -- ENOENT specifically, disambiguated from a broken symlink via lstatSync) from 'corrupt' (file exists but unreadable/unparseable/ wrong shape) from 'ok'. Only 'absent' behaves as pre-fix (ungated) -- deliberate, since every existing project is in that state for every workstream on the day this ships. 'corrupt' and 'ok' both fail closed for a phase with no trustworthy entry, via a new internal sentinel 'unrecorded' added to FAILING_VERIFICATION_STATUSES. A corrupt ledger is not a permanent wedge: it is only overwritten when a real verdict is actually observed (never patched with an empty object), so re-verifying even one phase repairs the file. The ledger write is now atomic (temp file + rename, mirroring broken-windows.cts's writeLedgerAtomic/renameWithRetry shape) so this fix does not itself produce the corrupt files it now treats as security-relevant. Disclosed, accepted residual gap, pinned as an explicit test: deleting the ledger file itself (not just the report) still returns a workstream to the pre-adoption 'absent' state. This is inherent to any design where a wholly-absent store must be safe by default -- the alternative is gating every never-verified phase in every project on upgrade. Rail B is prospective only; a phase deleted before this fix shipped cannot be retroactively recovered. Also: dropped 'stale' from the Row 9 property test's REAL_STATUSES (it does not round-trip through readVerificationStatus as written, so including it claimed coverage the test did not have), converted two try/finally test bodies to t.after(), and added the remaining CONTRIBUTING.md fault-injection cases (broken symlink, missing parent directory, rename failure, temp-file cleanup). * fix(#2645): share the rollup winner selection to close a scoping gap BLOCKER: the ledger-winner selection in workstream-inventory.cts compared raw mtimes with no milestone-scoping filter, while the builder's own rollupDirByKey filters out-of-milestone directories before comparing. In a scoped workstream, a stale out-of-milestone duplicate-key directory with a newer mtime could win the ledger's selection while losing the builder's -- so deleting the LIVE directory's report never consulted the ledger, reopening #2645's hole for the phase that actually counts toward completed_phases, reachable with a plain rm. Extracted pickRollupWinners as the single shared implementation both rollupDirByKey and the ledger's winner selection now call, with the identical scoping filter -- two independent hand-written copies of "pick the winner" is what produced the divergence; one implementation makes the bug class structurally impossible rather than merely tested against. Added a unit-level proof (synthetic same-key entries with opposing inclusion/mtime) and an integration-level proof (a scoped workstream asserting an out-of-milestone verdict is never written into the ledger). Also: disclosed a third residual limitation in the changeset (editing a ledger entry by hand plants a permanent false verdict -- worse than deleting the ledger, since it looks like genuine history; not made tamper-proof, that's scope creep here); fixed a non-ENOENT lstatSync failure falling open to 'absent' instead of failing closed like every other path in that function; and fixed two test bugs a real gsd-test run caught -- Row 11's write-failure mock matched only the final ledger path, but the atomic-write refactor moved the real write target to a temp file, so the mock silently stopped intercepting anything and the test's own assertion caught its own staleness. * test(#2645): relabel Row 19 honestly and pin the lstat fail-closed fix Row 19 used two DIFFERENT phase keys (1-old, 2-new), so it never exercised the same-key collision the milestone-scoping blocker fix addresses -- the pre-fix, unshared ledger-winner code would have satisfied it too. Its docstring called it the integration-level proof of the blocker; it is not. Relabeled both rows accurately: Row 18 (synthetic same-key data) is now stated as the only row that proves the collision end to end, and Row 19 is described for what it genuinely covers -- a distinctly-keyed out-of-milestone phase's verdict never reaching the ledger, real coverage but not the collision case. Extensive probing (documented in 10-diagnosis.md) could not construct a natural directory-naming pair that shares a rollup key while diverging in milestone membership under the current roadmap-parser implementation, so the collision proof stays unit-level by necessity, not convenience. Also added a test pinning the lstat fail-closed fix: readVerificationLedger disambiguates a broken symlink (ENOENT from readFileSync) from genuine absence via a follow-up lstatSync call, and only lstatSync itself reporting ENOENT is proof of absence. A double-fault (readFileSync ENOENT, lstatSync a DIFFERENT code) cannot occur on a real filesystem, so it's monkeypatched directly -- without a test, a future edit could re-widen that catch back to "any lstat failure means absent" and fall open again silently. * chore(#2645): backfill changeset PR number to 3016 --------- Co-authored-by: sim --- .../2645-verification-deletion-ledger.md | 9 + src/workstream-inventory-builder.cts | 78 +- src/workstream-inventory.cts | 295 ++++++- tests/workstream-inventory.test.cjs | 758 +++++++++++++++++- 4 files changed, 1121 insertions(+), 19 deletions(-) create mode 100644 .changeset/2645-verification-deletion-ledger.md diff --git a/.changeset/2645-verification-deletion-ledger.md b/.changeset/2645-verification-deletion-ledger.md new file mode 100644 index 000000000..b2a2e6342 --- /dev/null +++ b/.changeset/2645-verification-deletion-ledger.md @@ -0,0 +1,9 @@ +--- +type: Fixed +pr: 3016 +--- +**Deleting a phase's verification report can no longer inflate workstream completion once that report has been seen.** Removing a `*-VERIFICATION.md` file after a failing `gaps_found` or `human_needed` verdict was recorded used to be indistinguishable from never having verified the phase at all, so `completed_phases` and `progress_percent` silently rose. `workstream status`/`list`/`progress` now remember the last real verdict observed per phase in a new `.verification-ledger.json` file alongside each workstream's `STATE.md`, so a failing verdict a prior read has already seen can't be erased by deleting its report. This adds a small write side effect to those previously read-only commands, and the file is a new tracked artifact under `.planning/workstreams//` for projects that commit their planning docs. + +The ledger fails **closed**, not open: once a workstream has adopted it (the ledger file exists), a phase with no remembered entry — including one whose ledger entry can't be read because the file is corrupt or unreadable — is treated as not-yet-verified-and-blocking, not as safe-to-complete. A workstream that has never used the verifier is untouched (no ledger file is ever created for it), which is what keeps every existing project from dropping to `in_progress` the moment this ships. + +**Three limitations, disclosed rather than silently left:** this is prospective only — a phase verified and its report deleted *before* this fix ships has no ledger entry and can't be recovered retroactively. Deleting the ledger file itself, not just the report, still returns that phase to pre-adoption behavior; this is inherent to any design where a wholly-absent ledger must be safe (the alternative is gating every never-verified phase in every project on upgrade), and is not something ledger design alone can close. And the ledger is not tamper-proof: anyone with write access to `.planning/workstreams//.verification-ledger.json` can hand-edit an entry to `"passed"` and the remembered value is trusted indefinitely — this is a *different* and arguably worse way to inflate completion than deleting the ledger (which at least resets to a visibly pre-adoption, untracked state), since an edited entry looks like genuine durable history. Integrity-checking the ledger's own content is out of scope for this fix. (#2645) diff --git a/src/workstream-inventory-builder.cts b/src/workstream-inventory-builder.cts index 0baef82b0..aca4afeed 100644 --- a/src/workstream-inventory-builder.cts +++ b/src/workstream-inventory-builder.cts @@ -21,8 +21,60 @@ function toPosixPath(p: string): string { * EXPLICIT failing verdicts the verifier emits — `missing`/`unknown` (verifier * off / not yet run) and `stale` (mtime-derived, #2348) are intentionally left * untouched so verifier-disabled projects do not regress to never-complete. + * + * #2645: `'unrecorded'` is NOT a verdict the verifier itself ever emits — it + * is an internal sentinel `workstream-inventory.cts`'s verification-deletion + * ledger substitutes for `'missing'` once that workstream has ADOPTED the + * ledger (a `.verification-ledger.json` file exists for it) but has no + * remembered entry for this specific phase. Pre-adoption (no ledger file at + * all) still resolves to plain `'missing'`, which stays OUTSIDE this set — + * that is what keeps a project untouched by this fix until it actually uses + * the verifier at least once. Post-adoption, an unrecorded phase fails + * CLOSED (counted here) rather than open, so a corrupt or evidence-absent + * ledger entry can no longer be read as "safe to complete" the way a bare + * `'missing'` sentinel is. */ -const FAILING_VERIFICATION_STATUSES = new Set(['gaps_found', 'human_needed']); +const FAILING_VERIFICATION_STATUSES = new Set(['gaps_found', 'human_needed', 'unrecorded']); + +/** + * #2562 / Bug #2445 / #2645 review: pick ONE winning item per key from a + * PRE-SORTED list — newest `mtimeMs` wins; on an exact tie the incumbent + * (first-in-sort-order) wins, since only a STRICTLY greater mtime replaces + * it. `includeItem` lets a caller exclude items before comparison (e.g. + * out-of-milestone directories) — critically, the filter runs BEFORE the + * mtime comparison, so an excluded item can never win a tie or a comparison + * against an included one. + * + * Extracted as the SINGLE shared implementation after a #2645 review found + * two independently-written copies of this exact rule had silently + * diverged: `workstream-inventory.cts`'s ledger-winner selection compared + * raw mtimes with no scoping filter, while this module's own `rollupDirByKey` + * (below) filtered out-of-milestone directories first. A stale out-of- + * milestone directory with a newer mtime than the live in-milestone one + * (plausible after a checkout/rebase resets mtimes) could then win the + * LEDGER's selection while losing the BUILDER's — reopening #2645's own + * hole for the phase that actually counts toward `completed_phases`, + * reachable with a plain `rm` and no ledger tampering. A comment asserting + * two hand-written copies "use the same rule" is not a guarantee they do; + * one shared function is. + */ +export function pickRollupWinners( + sortedItems: T[], + keyOf: (item: T) => string, + mtimeOf: (item: T) => number, + includeItem: (item: T) => boolean = () => true, +): Map { + const winners = new Map(); + for (const item of sortedItems) { + if (!includeItem(item)) continue; + const key = keyOf(item); + const incumbent = winners.get(key); + if (incumbent === undefined || mtimeOf(item) > mtimeOf(incumbent)) { + winners.set(key, item); + } + } + return winners; +} export function isCompletedInventory(status: unknown): boolean { const s = (typeof status === 'string' @@ -220,20 +272,16 @@ export function buildWorkstreamInventory(inputs: BuildWorkstreamInventoryInputs) // each add to the numerator while the denominator counts distinct phases — // pushing completed_phases past it, where the old `Math.min` cap silently // rounded the result up to 100% and hid an unstarted phase. Newest-on-disk - // wins, mirroring state.cts's #2445 de-duplication. - const rollupDirByKey = new Map(); - for (const dir of [...phaseDirNames].sort()) { - const entry = countsMap.get(dir); - if (scoped && entry?.inMilestone === false) continue; - const key = entry?.phaseKey ?? dir; - const incumbent = rollupDirByKey.get(key); - if (incumbent === undefined) { - rollupDirByKey.set(key, dir); - continue; - } - const incumbentMtime = countsMap.get(incumbent)?.mtimeMs ?? 0; - if ((entry?.mtimeMs ?? 0) > incumbentMtime) rollupDirByKey.set(key, dir); - } + // wins, mirroring state.cts's #2445 de-duplication. `pickRollupWinners` is + // the SHARED implementation `workstream-inventory.cts`'s ledger-winner + // selection also calls, so the two can never independently diverge again + // (#2645 review). + const rollupDirByKey = pickRollupWinners( + [...phaseDirNames].sort(), + (dir) => countsMap.get(dir)?.phaseKey ?? dir, + (dir) => countsMap.get(dir)?.mtimeMs ?? 0, + (dir) => !(scoped && countsMap.get(dir)?.inMilestone === false), + ); const rollupDirs = new Set(rollupDirByKey.values()); const phases: PhaseStatus[] = []; diff --git a/src/workstream-inventory.cts b/src/workstream-inventory.cts index 08dbed24b..9de9cfd25 100644 --- a/src/workstream-inventory.cts +++ b/src/workstream-inventory.cts @@ -34,7 +34,7 @@ const { phaseKeyFromDir, phaseKeyFromProse, parentPhaseKey } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- roadmap-parser.cjs is an export= CommonJS module import roadmapParserMod = require('./roadmap-parser.cjs'); const { getMilestonePhaseFilter, isMilestoneShippedInRoadmap } = roadmapParserMod; -import { buildWorkstreamInventory, isCompletedInventory } from './workstream-inventory-builder.cjs'; +import { buildWorkstreamInventory, isCompletedInventory, pickRollupWinners } from './workstream-inventory-builder.cjs'; import type { WorkstreamInventory, StateProjection, MilestoneShippedSignal } from './workstream-inventory-builder.cjs'; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -199,6 +199,208 @@ function countPhaseFiles(phaseDir: string): PhaseFileCounts { return { planCount: scan.planCount, summaryCount: scan.summaryCount }; } +// ─── #2645: verification-deletion ledger ─────────────────────────────────── +// +// #2562's completeness gate (`FAILING_VERIFICATION_STATUSES`, +// workstream-inventory-builder.cts) reads a phase's verification verdict +// fresh from `*-VERIFICATION.md` on every call. That makes "verifier ran, +// found gaps, report later deleted" indistinguishable from "verifier never +// ran" — both collapse to the same `'missing'` sentinel +// (verification.cts's `missingResult()`), which is deliberately NOT in the +// failing set (so verifier-disabled projects can still reach 100%). Deleting +// a failing report is therefore sufficient to silently raise the reported +// completion percentage — a Goodhart hole (#2645). +// +// The fix persists the last REAL (non-'missing') verdict this module has +// ever observed per phase key, in a small ledger file living at the +// WORKSTREAM directory level — never inside the phase directory whose file +// is the thing being deleted, so the same `rm` that triggers the hole +// cannot also erase the memory of it. When a live read comes back 'missing', +// the ledger is consulted as a fallback; when a live read comes back with a +// real verdict — including a later 'passed' that supersedes an earlier +// failing one — the ledger is updated to match, so a genuinely re-verified +// phase is never permanently pinned. +// +// #2645 review: a NAIVE two-state read ("got entries, or nothing") fails +// OPEN — any read/parse failure degraded to `{}`, which is indistinguishable +// from "genuinely never verified", so corrupting the ledger (or deleting it +// alongside the report) silently reopened the exact hole this fix exists to +// close, one level up. THREE states, not two: +// +// 1. `'absent'` — no `.verification-ledger.json` for this workstream at +// all. This is the ONLY state that behaves exactly as pre-#2645 (a +// phase with no live report reads `'missing'`, ungated). Deliberate: +// on the day this ships, EVERY existing project is in this state for +// EVERY workstream, and gating here would drop them all to +// `in_progress` at once. A workstream stays here forever if the +// verifier is never actually used on it (criterion 2/3 — no entry is +// ever written for a `'missing'` live read, so the file itself is +// never created). +// 2. `'corrupt'` — the file exists but could not be read or parsed (I/O +// error, invalid JSON, wrong shape). This is NOT the same as absent: +// an unreadable file is evidence something existed. Treated identically +// to "present, no entry for this phase" below — fails CLOSED, not open. +// 3. `'ok'` — the file exists and parsed. A phase with an entry uses +// it; a phase WITHOUT one is "present, no entry" — this workstream has +// adopted the ledger (some phase in it has a real verdict on record), +// so an unobserved phase can no longer default to the pre-adoption +// "ungated" behavior, or the same evidence-erasure hole reopens for +// THIS phase specifically. Fails CLOSED: resolves to the `'unrecorded'` +// sentinel (`workstream-inventory-builder.cts`'s +// `FAILING_VERIFICATION_STATUSES`), not `'missing'`. +// +// Corrupt-ledger recovery: a corrupt ledger is NOT a permanent wedge. Any +// phase with a REAL live verdict on disk still writes/repairs the ledger on +// this same call (the corrupt content is fully overwritten, never patched), +// so re-running the verifier for even one phase heals the file. A phase with +// no live report and no way to re-verify stays `'unrecorded'` (gated) until +// someone re-verifies it — a deliberate, disclosed cost of failing closed, +// not an accidental one. +// +// Disclosed, ACCEPTED residual gap (not closed by this fix, and not closable +// by ledger design alone): deleting the ledger FILE ITSELF (not just the +// phase's report) returns a workstream to state 1 (`'absent'`) and restores +// pre-#2645 behavior for it. Any durable store that can fail open when +// absent has this property at its own root — the ledger raises the bar from +// "delete one file" to "delete two files in two different directories, +// including one this issue's own reproduction never needed to touch", but a +// deliberately absent ledger is indistinguishable from a never-adopted one +// by design (criterion 2/3 depend on that same indistinguishability). Rail B +// is PROSPECTIVE ONLY: a phase verified and its report deleted BEFORE this +// fix ships has no ledger entry to fall back on and cannot be retroactively +// recovered. +interface VerificationLedger { [phaseKey: string]: string; } + +interface VerificationLedgerRead { + state: 'absent' | 'corrupt' | 'ok'; + entries: VerificationLedger; +} + +function verificationLedgerPath(wsDir: string): string { + return path.join(wsDir, '.verification-ledger.json'); +} + +function readVerificationLedger(wsDir: string): VerificationLedgerRead { + const ledgerPath = verificationLedgerPath(wsDir); + let raw: string; + try { + raw = fs.readFileSync(ledgerPath, 'utf-8'); + } catch (err) { + // `fs.readFileSync` FOLLOWS symlinks, so a broken symlink at this path + // (the entry exists, its target does not) reports the EXACT SAME + // `ENOENT` as genuine absence — `code` alone cannot distinguish + // "nothing was ever here" from "something is here and cannot be read". + // `fs.lstatSync` does NOT follow symlinks, so it still finds the + // symlink entry itself even when its target is gone. Only when NEITHER + // call finds anything is this genuinely `'absent'` (pre-adoption); a + // present-but-broken symlink is evidence something existed and must + // fail CLOSED like any other unreadable ledger, not fall open. + const code = (err as NodeJS.ErrnoException)?.code; + if (code === 'ENOENT') { + try { + fs.lstatSync(ledgerPath); + return { state: 'corrupt', entries: {} }; // a symlink entry exists; its target does not + } catch (lstatErr) { + // #2645 review: every OTHER failure path in this function fails + // CLOSED — this one must too. A failed `lstatSync` is only proof of + // absence when IT ALSO reports `ENOENT`; anything else (a raced + // permission change, a path component that became inaccessible + // between the two calls, …) is not evidence the file was never + // there, and a bare `catch {}` here would silently fall OPEN exactly + // like the two-state design this fix replaced. + const lstatCode = (lstatErr as NodeJS.ErrnoException)?.code; + if (lstatCode === 'ENOENT') return { state: 'absent', entries: {} }; // truly nothing at this path + return { state: 'corrupt', entries: {} }; + } + } + // Any OTHER read failure (EACCES, EISDIR, …) also means the path EXISTS + // in some form but this process cannot see its content right now — that + // is corruption from this reader's point of view, not absence, and must + // fail closed rather than silently falling back to the ungated + // pre-adoption behavior. + return { state: 'corrupt', entries: {} }; + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return { state: 'corrupt', entries: {} }; + } + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return { state: 'corrupt', entries: {} }; + } + const out: VerificationLedger = {}; + for (const [key, value] of Object.entries(parsed as Record)) { + if (typeof value === 'string') out[key] = value; + } + return { state: 'ok', entries: out }; +} + +// #2645 review: Windows can transiently hold the rename target busy (AV +// scanners, indexers) — the SAME retry shape `broken-windows.cts`'s +// `writeLedgerAtomic`/`renameWithRetry` already uses for its own ledger +// write, mirrored here rather than imported (that function is private to +// its module) so this fix does not widen its own blast radius by exporting +// a new cross-module utility. +const LEDGER_RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']); +const LEDGER_RENAME_MAX_ATTEMPTS = 5; +const LEDGER_RENAME_BACKOFF_MS = 25; + +function renameVerificationLedgerWithRetry(tmpPath: string, finalPath: string): void { + let lastErr: unknown; + for (let attempt = 0; attempt < LEDGER_RENAME_MAX_ATTEMPTS; attempt++) { + try { + fs.renameSync(tmpPath, finalPath); + return; + } catch (err: unknown) { + lastErr = err; + const code = (err && typeof err === 'object' && 'code' in err) ? String((err as { code?: unknown }).code) : ''; + if (code && LEDGER_RENAME_RETRY_ERRNOS.has(code) && attempt < LEDGER_RENAME_MAX_ATTEMPTS - 1) { + // Exponential-ish backoff: 25ms, 50ms, 100ms, 200ms. Transient + // Windows locks usually clear well inside that window. + const delay = LEDGER_RENAME_BACKOFF_MS * Math.pow(2, attempt); + const start = Date.now(); + while (Date.now() - start < delay) { + // Deliberate short busy-wait — no async/timer seam is available + // in this synchronous read path. + } + continue; + } + throw err; + } + } + throw lastErr; +} + +function writeVerificationLedger(wsDir: string, ledger: VerificationLedger): void { + // #2645 review: atomic write. A crash or a concurrent read mid-write + // against `verificationLedgerPath(wsDir)` directly would leave (or briefly + // expose) TRUNCATED JSON — and now that `'corrupt'` carries real semantic + // weight (it fails CLOSED, holding every phase with no live report at + // `'unrecorded'`), producing a corrupt file ourselves is a self-inflicted + // version of the exact failure mode this fix exists to survive. Write to a + // sibling temp file in the SAME directory (same filesystem — `rename` is + // only atomic within one) and rename into place: a reader can only ever + // observe the prior complete content or the new complete content. + const finalPath = verificationLedgerPath(wsDir); + const tmpPath = `${finalPath}.${process.pid}.tmp`; + try { + fs.mkdirSync(wsDir, { recursive: true }); + fs.writeFileSync(tmpPath, `${JSON.stringify(ledger, null, 2)}\n`, 'utf-8'); + renameVerificationLedgerWithRetry(tmpPath, finalPath); + } catch { + // Best-effort persistence: a missing parent directory that `mkdirSync` + // itself cannot create, a read-only filesystem, a write failure, or a + // rename failure that exhausts its retries must not break inventory + // reads, which are otherwise pure. Losing this observation only + // re-opens the pre-#2645 window for THIS run; the next successful read + // while a report is on disk repairs it. Clean up a half-written temp + // file so repeated failures cannot accumulate orphaned + // `.verification-ledger.json..tmp` files. + try { fs.unlinkSync(tmpPath); } catch { /* nothing to clean up, or cleanup itself failed — not fatal */ } + } +} + function readStateProjection(statePath: string): StateProjection { try { const stateContent = fs.readFileSync(statePath, 'utf-8'); @@ -366,7 +568,18 @@ function inspectWorkstream(cwd: string, name: string, options: InspectWorkstream // Collect per-phase file counts (+ canonical key, milestone membership, // verification verdict). `phaseKey` lets the builder de-duplicate stale // same-numbered directories (Bug #2445's scenario) in the rollup. - const phaseFilesCounts = phaseDirNames.map(dir => { + // + // #2645 review: built from `[...phaseDirNames].sort()`, NOT the raw + // `phaseDirNames` (unsorted `readdirSync` order — `readSubdirectories`'s + // default). `buildWorkstreamInventory`'s own de-dup (`rollupDirByKey`, + // workstream-inventory-builder.cts) iterates the SAME sorted order; the + // ledger-winner tie-break below (incumbent wins unless a later entry has a + // STRICTLY newer mtime) only agrees with the builder's winner on an exact + // mtime tie if both walk entries in the same order. Iterating unsorted + // input here let the two independently pick different "winning" + // directories for one phase key on a tie — reopening the stale-duplicate- + // clobbers-live-verdict hole criterion 4 exists to close. + const rawPhaseEntries = [...phaseDirNames].sort().map(dir => { const phaseDir = path.join(p.phases, dir); const counts = countPhaseFiles(phaseDir); return { @@ -376,7 +589,83 @@ function inspectWorkstream(cwd: string, name: string, options: InspectWorkstream planCount: counts.planCount, summaryCount: counts.summaryCount, inMilestone: isDirInCurrentMilestone(dir), - verificationStatus: readVerificationStatus(phaseDir).status, + liveVerificationStatus: readVerificationStatus(phaseDir).status, + }; + }); + + // #2645: only the directory Bug #2445's de-dup rollup would actually pick + // for a phase key may read or write that key's ledger entry. Letting every + // same-keyed directory (including a stale leftover) write would let a + // stale duplicate's stale verdict clobber the live directory's remembered + // one. + // + // #2645 review — CORRECTED: an earlier version of this comment claimed the + // milestone-scoping exclusion (`scoped && entry.inMilestone === false`) + // was safe to drop here as "out of scope", and hand-wrote a scoping-free + // tie-break rule. That was a real bug, not a scope call: in a SCOPED + // workstream, a stale OUT-of-milestone directory sharing a phase key with + // the live IN-milestone one can have a newer mtime (plausible after a + // checkout/rebase resets mtimes) and would then win THIS selection while + // the builder's own `rollupDirByKey` — which DOES apply the scoping filter + // — picks the live directory instead. `isLedgerWinner` would then be false + // for the live directory, so deleting ITS `*-VERIFICATION.md` would never + // consult the ledger and would reopen #2645's exact hole for the phase + // that actually counts toward `completed_phases` — reachable with a plain + // `rm`, no ledger tampering required. Fixed by calling the SAME shared + // `pickRollupWinners` the builder's `rollupDirByKey` now also calls, with + // the identical scoping filter, rather than a second hand-written copy. + const ledgerWinnerByKey = pickRollupWinners( + rawPhaseEntries, + (entry) => entry.phaseKey, + (entry) => entry.mtimeMs, + (entry) => !(scoped && entry.inMilestone === false), + ); + + const ledgerRead = readVerificationLedger(wsDir); + // Both `'corrupt'` and `'ok'` start from whatever entries could actually be + // trusted (empty for `'corrupt'` — nothing in an unparseable file is + // trusted) and get REPAIRED below by any real verdict this call observes; + // only `'absent'` skips the ledger mechanism entirely (pre-adoption). + const verificationLedger = ledgerRead.entries; + let ledgerDirty = false; + for (const winner of ledgerWinnerByKey.values()) { + if (winner.liveVerificationStatus === 'missing') continue; + if (verificationLedger[winner.phaseKey] !== winner.liveVerificationStatus) { + verificationLedger[winner.phaseKey] = winner.liveVerificationStatus; + ledgerDirty = true; + } + } + // A `'corrupt'` read that observes no real verdict this call has nothing to + // repair with — writing an empty `{}` would DESTROY whatever the corrupt + // file's bytes might still hold (a human could recover it by hand; this + // fix must not foreclose that). Only write when there is something real to + // persist, exactly as for `'absent'`/`'ok'`. + if (ledgerDirty) writeVerificationLedger(wsDir, verificationLedger); + + const phaseFilesCounts = rawPhaseEntries.map(entry => { + const isLedgerWinner = ledgerWinnerByKey.get(entry.phaseKey) === entry; + let verificationStatus = entry.liveVerificationStatus; + if (entry.liveVerificationStatus === 'missing' && isLedgerWinner) { + if (ledgerRead.state === 'absent') { + // State 1: pre-adoption. Exactly today's behavior — 'missing' is + // NOT in FAILING_VERIFICATION_STATUSES, so this does not gate. + verificationStatus = 'missing'; + } else { + // States 2/3 ('corrupt' or 'ok'): this workstream has adopted the + // ledger. A remembered entry wins; no entry fails CLOSED to the + // 'unrecorded' sentinel rather than falling open to 'missing'. + const remembered = verificationLedger[entry.phaseKey]; + verificationStatus = remembered !== undefined ? remembered : 'unrecorded'; + } + } + return { + directory: entry.directory, + phaseKey: entry.phaseKey, + mtimeMs: entry.mtimeMs, + planCount: entry.planCount, + summaryCount: entry.summaryCount, + inMilestone: entry.inMilestone, + verificationStatus, }; }); diff --git a/tests/workstream-inventory.test.cjs b/tests/workstream-inventory.test.cjs index 6312bc077..82f810260 100644 --- a/tests/workstream-inventory.test.cjs +++ b/tests/workstream-inventory.test.cjs @@ -11,7 +11,7 @@ const fs = require('fs'); const path = require('path'); const { cleanup } = require('./helpers.cjs'); const { createFixture, seedWorkstream } = require('./fixtures/index.cjs'); -const { buildWorkstreamInventory, isCompletedInventory } = require('../gsd-core/bin/lib/workstream-inventory-builder.cjs'); +const { buildWorkstreamInventory, isCompletedInventory, pickRollupWinners } = require('../gsd-core/bin/lib/workstream-inventory-builder.cjs'); const { inspectWorkstream } = require('../gsd-core/bin/lib/workstream-inventory.cjs'); const { VERIFIER_STATUSES } = require('../gsd-core/bin/lib/verification.cjs'); const { phaseKeyFromDir, phaseKeyFromProse, phaseKeyFromToken } = require('../gsd-core/bin/lib/phase-id.cjs'); @@ -1006,3 +1006,759 @@ describe('#2562 — a shipped marker its own artifacts contradict is not asserte assert.equal(inv.milestone_shipped_unverified, true); }); }); + +// #2645 — deleting a *-VERIFICATION.md must never raise reported completion. +// #2562 gated completeness on the verdict READ FRESH from disk every call: +// "verifier ran gaps_found, report later deleted" and "verifier never ran" +// both read as the internal 'missing' sentinel, and only the fresh read was +// consulted — so deleting the evidence file alone was enough to silently +// raise completed_phases/progress_percent. The fix persists the last REAL +// verdict observed per phase key in a ledger file living at the WORKSTREAM +// level (`.verification-ledger.json`, sibling to STATE.md/ROADMAP.md), +// consulted only when the live read comes back 'missing'. +describe('#2645 — deleting a verification report must not raise completeness', () => { + let tmpDir; + before(() => { tmpDir = createFixture(); }); + after(() => cleanup(tmpDir)); + + const FLAT_STATE = 'status: executing\n'; + function flatRoadmap(rows) { + return [ + '# Roadmap', '', '## Progress', '', + '| Phase | Plans Complete | Status | Completed |', + '| --- | --- | --- | --- |', + ...rows, + '', + ].join('\n'); + } + + function writePhase(wsDir, slug, { plans = 1, summaries = 1, verification } = {}) { + const dir = path.join(wsDir, 'phases', slug); + fs.mkdirSync(dir, { recursive: true }); + for (let i = 1; i <= plans; i++) fs.writeFileSync(path.join(dir, `0${i}-PLAN.md`), '# plan\n'); + for (let i = 1; i <= summaries; i++) fs.writeFileSync(path.join(dir, `0${i}-SUMMARY.md`), '# summary\n'); + if (verification) fs.writeFileSync(path.join(dir, '01-VERIFICATION.md'), `---\nstatus: ${verification}\n---\n`); + return dir; + } + + function verificationFilePath(wsDir, slug) { + return path.join(wsDir, 'phases', slug, '01-VERIFICATION.md'); + } + + // Row 1 — failing-first regression, the issue's own reproduction sequence. + test('deleting a gaps_found report after it was observed does not raise completeness (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-gaps' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + + const before1 = inspectWorkstream(tmpDir, 'ws-2645-gaps', { active: null }); + assert.ok(before1); + assert.equal(before1.phases[0].status, 'in_progress', 'gaps_found must not count as complete'); + assert.equal(before1.completed_phases, 0); + assert.equal(before1.progress_percent, 0); + + fs.unlinkSync(verificationFilePath(wsDir, '1-foo')); + + const after1 = inspectWorkstream(tmpDir, 'ws-2645-gaps', { active: null }); + assert.ok(after1); + assert.equal(after1.phases[0].status, 'in_progress', + 'deleting the failing report must not flip the phase to complete'); + assert.equal(after1.completed_phases, 0, 'deleting evidence must never raise completed_phases'); + assert.equal(after1.progress_percent, 0, 'deleting evidence must never raise progress_percent'); + }); + + // Row 2 — the other FAILING_VERIFICATION_STATUSES member. + test('deleting a human_needed report after it was observed does not raise completeness (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-human' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'human_needed' }); + + inspectWorkstream(tmpDir, 'ws-2645-human', { active: null }); // observe while present + fs.unlinkSync(verificationFilePath(wsDir, '1-foo')); + + const after = inspectWorkstream(tmpDir, 'ws-2645-human', { active: null }); + assert.ok(after); + assert.equal(after.phases[0].status, 'in_progress'); + assert.equal(after.completed_phases, 0); + assert.equal(after.progress_percent, 0); + }); + + // Row 3 — criterion 2: verifier-disabled projects (no report ever written) + // must still be able to reach 100%. + test('a phase that was never verified still reaches complete (#2645, criterion 2)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-never' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1 }); // no verification file, ever + + const inv = inspectWorkstream(tmpDir, 'ws-2645-never', { active: null }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'complete'); + assert.equal(inv.completed_phases, 1); + assert.equal(inv.progress_percent, 100); + }); + + // Row 4 — criterion 3: same on-disk shape as row 3 (no file), across TWO + // reads, must behave IDENTICALLY — the ledger must not newly gate a phase + // that was simply never verified in the first place. + test('a not-yet-verified phase is not newly gated by the ledger (#2645, criterion 3)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-not-yet' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1 }); + + const first = inspectWorkstream(tmpDir, 'ws-2645-not-yet', { active: null }); + const second = inspectWorkstream(tmpDir, 'ws-2645-not-yet', { active: null }); + assert.equal(first.progress_percent, 100); + assert.equal(second.progress_percent, 100, 'a second read must not change the outcome'); + assert.equal(second.phases[0].status, 'complete'); + }); + + // Row 5 — recovery: a genuinely re-verified phase must not be pinned by an + // earlier failing verdict the ledger remembers. + test('a re-verified passed phase is not pinned by an earlier failing ledger entry (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-recover' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + inspectWorkstream(tmpDir, 'ws-2645-recover', { active: null }); // observe gaps_found + + // Re-verify: overwrite the report with a passing verdict. + fs.writeFileSync(verificationFilePath(wsDir, '1-foo'), '---\nstatus: passed\n---\n'); + const reverified = inspectWorkstream(tmpDir, 'ws-2645-recover', { active: null }); + assert.equal(reverified.phases[0].status, 'complete', 'a genuine re-verify must count'); + + // Delete the now-passing report — the ledger's newest real verdict is + // 'passed', which is not in the failing set, so this must stay complete. + fs.unlinkSync(verificationFilePath(wsDir, '1-foo')); + const afterDelete = inspectWorkstream(tmpDir, 'ws-2645-recover', { active: null }); + assert.equal(afterDelete.phases[0].status, 'complete', + 'the ledger must hold the newest verdict, not an earlier failing one'); + }); + + // Row 6 — criterion 4: the ledger must not live inside the phase directory + // whose file is the one being deleted. + test('ledger file lives outside the phase directory it protects (#2645, criterion 4)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-location' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + const phaseDir = writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + inspectWorkstream(tmpDir, 'ws-2645-location', { active: null }); + + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + assert.ok(fs.existsSync(ledgerPath), 'the ledger must be persisted somewhere'); + assert.ok(!ledgerPath.startsWith(phaseDir + path.sep) && ledgerPath !== phaseDir, + 'the ledger must not live inside the phase directory'); + + // Deleting the ENTIRE phase directory (not just the report) must not + // touch the ledger — this is what "not the same file" is protecting + // against in the realistic case. + cleanup(phaseDir); + assert.ok(fs.existsSync(ledgerPath), 'removing the phase directory must not remove the ledger'); + }); + + // Row 7 — Bug #2445 dedup safety: a stale duplicate directory sharing the + // same phase key must not be able to clobber the WINNING (newest) + // directory's ledger entry with its own stale/leftover verdict. + test('a stale duplicate directory cannot clobber the ledger entry of the live directory (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-dupe' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + '| 2. Bar | 0/1 | Not started | - |', + ])); + // Stale leftover directory (older mtime), leftover gaps_found report. + const staleDir = writePhase(wsDir, '01-foo-old', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const oldTime = new Date('2020-01-01T00:00:00Z'); + fs.utimesSync(staleDir, oldTime, oldTime); + // Live/winning directory (newer mtime), currently passed. + const liveDir = writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'passed' }); + const newTime = new Date('2026-01-01T00:00:00Z'); + fs.utimesSync(liveDir, newTime, newTime); + + const before7 = inspectWorkstream(tmpDir, 'ws-2645-dupe', { active: null }); + assert.ok(before7); + assert.equal(before7.completed_phases, 1, 'the winning (newest) directory is passed'); + + // Delete the WINNING directory's report. If the stale directory's + // gaps_found had clobbered the ledger, this phase key would now + // incorrectly read gaps_found and never recover. It must instead read + // the winning directory's own remembered 'passed'. + fs.unlinkSync(path.join(liveDir, '01-VERIFICATION.md')); + + const after7 = inspectWorkstream(tmpDir, 'ws-2645-dupe', { active: null }); + assert.ok(after7); + assert.equal(after7.completed_phases, 1, + "the ledger must remember the WINNING directory's passed verdict, not the stale duplicate's gaps_found"); + }); + + // Row 8 — boundary: an EXACT mtime tie between two same-keyed directories. + // The builder's own tie-break (`rollupDirByKey`) walks + // `[...phaseDirNames].sort()` and keeps the incumbent on a tie + // (first-in-sort-order wins, since only a STRICTLY newer mtime replaces + // it). The ledger's winner selection must walk the SAME sorted order so + // the two deterministically agree on which directory wins — not merely + // "some directory wins" (a prior version of this test asserted only + // `completed_phases === 0 || 1`, which is true regardless of agreement and + // caught nothing; both `01-foo-a`/`01-foo-b` sort deterministically, so the + // outcome here is not a coin flip). + test('an exact mtime tie resolves the ledger winner by sort order, matching the builder (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-tie' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + ])); + const tieTime = new Date('2025-06-01T00:00:00Z'); + // '01-foo-a' sorts before '01-foo-b' — with equal mtimes, BOTH the + // builder's rollup and the ledger's winner selection must keep the + // incumbent '01-foo-a' (passed), never adopt '01-foo-b' (gaps_found). + const dirA = writePhase(wsDir, '01-foo-a', { plans: 1, summaries: 1, verification: 'passed' }); + fs.utimesSync(dirA, tieTime, tieTime); + const dirB = writePhase(wsDir, '01-foo-b', { plans: 1, summaries: 1, verification: 'gaps_found' }); + fs.utimesSync(dirB, tieTime, tieTime); + + assert.doesNotThrow(() => inspectWorkstream(tmpDir, 'ws-2645-tie', { active: null })); + const inv = inspectWorkstream(tmpDir, 'ws-2645-tie', { active: null }); + assert.ok(inv); + // Deterministic, not "either is fine": the builder's own rollup counts + // exactly the sort-order incumbent (01-foo-a, passed) as complete. + assert.equal(inv.completed_phases, 1, + 'the sort-order incumbent (01-foo-a, passed) must be the one the builder counts complete'); + + // Concrete cross-check that the LEDGER's winner selection agrees with the + // builder's, not just that this run's numbers happen to match: delete + // 01-foo-a's report (unlink bumps the directory's mtime — restore it to + // the exact tie value so the SECOND read still sees a genuine tie, not a + // newest-mtime win). If the ledger's winner selection had instead picked + // 01-foo-b (the bug this test guards — unsorted iteration disagreeing + // with the builder's sorted `rollupDirByKey`), this phase key's + // remembered verdict would be 'gaps_found' and the phase would flip to + // in_progress. It must instead stay complete, because the ledger + // remembers 01-foo-a's 'passed' — the same directory the builder uses. + fs.unlinkSync(path.join(dirA, '01-VERIFICATION.md')); + fs.utimesSync(dirA, tieTime, tieTime); // restore the tie the unlink disturbed + const afterDelete = inspectWorkstream(tmpDir, 'ws-2645-tie', { active: null }); + assert.equal(afterDelete.completed_phases, 1, + "the ledger must have remembered the sort-order incumbent's 'passed' verdict, not the other directory's gaps_found"); + }); + + // Row 9 — property: whatever sequence of REAL verdicts is observed for a + // phase key, once the file goes missing the ledger must replay exactly the + // LAST one observed — never an earlier one, never a synthesized value. + // + // #2645 review: `'stale'` is deliberately EXCLUDED here, not merely + // forgotten. Writing a literal `status: stale` frontmatter value does NOT + // round-trip as `'stale'` — `readVerificationStatus` + // (`src/verification.cts`) explicitly excludes `'stale'` from its raw-file + // routing lookup (`rawStatus !== 'stale'`) and falls through to the + // "Unknown value" branch, returning `'unknown'` instead. A genuine + // `'stale'` verdict is only reachable via `findStaleVerificationSummary`'s + // mtime comparison (a SUMMARY file newer than the VERIFICATION file), not + // by writing the word into the file. Including `'stale'` in this array + // without accounting for that would silently substitute `'unknown'` on + // every iteration and still pass (both are non-failing) — a docstring + // claiming "real verdicts" coverage it does not actually exercise. + // `'stale'` handling is explicitly out of scope for this issue (#2348) and + // is not gated by `FAILING_VERIFICATION_STATUSES` either way. + test('property: the ledger always replays the most recently observed real verdict after deletion (#2645)', () => { + const REAL_STATUSES = ['passed', 'gaps_found', 'human_needed', 'unknown']; + const FAILING = new Set(['gaps_found', 'human_needed']); + fc.assert(fc.property( + fc.array(fc.constantFrom(...REAL_STATUSES), { minLength: 1, maxLength: 6 }), + (sequence) => { + const wsName = `ws-2645-prop-${Math.random().toString(36).slice(2)}`; + const wsDir = seedWorkstream(tmpDir, { name: wsName }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + ])); + const dir = writePhase(wsDir, '1-foo', { plans: 1, summaries: 1 }); + const reportPath = path.join(dir, '01-VERIFICATION.md'); + + let lastReal = null; + for (const status of sequence) { + fs.writeFileSync(reportPath, `---\nstatus: ${status}\n---\n`); + inspectWorkstream(tmpDir, wsName, { active: null }); // observe + lastReal = status; + } + fs.unlinkSync(reportPath); + + const inv = inspectWorkstream(tmpDir, wsName, { active: null }); + const expectComplete = !FAILING.has(lastReal); + assert.equal(inv.phases[0].status === 'complete', expectComplete, + `after observing ${JSON.stringify(sequence)} then deleting, status must reflect the last real verdict (${lastReal})`); + + cleanup(wsDir); + }, + ), { numRuns: 25 }); + }); + + // Row 10 — CONTRIBUTING.md "Filesystem writes and installers": code that + // writes under `.planning` needs fault-injection coverage. A corrupted + // `.verification-ledger.json` (bad JSON, or valid JSON of the wrong shape) + // must degrade to "nothing remembered", never throw and never break the + // read-only commands this ledger is a side effect of. + test('a corrupted ledger file degrades to no memory instead of throwing (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-corrupt-ledger' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + // Not valid JSON at all. + fs.writeFileSync(ledgerPath, 'not json{{{'); + assert.doesNotThrow(() => inspectWorkstream(tmpDir, 'ws-2645-corrupt-ledger', { active: null })); + + // Valid JSON, wrong shape (array instead of object) — must also degrade, + // not partially trust it. + fs.writeFileSync(ledgerPath, '["gaps_found"]'); + const inv = inspectWorkstream(tmpDir, 'ws-2645-corrupt-ledger', { active: null }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', 'the live gaps_found read still governs regardless of ledger corruption'); + }); + + // Row 10b — the path Row 10 does NOT exercise (a live report is present in + // both its cases, so the live read governs regardless of ledger state). + // With the report ALSO absent, a corrupt ledger must fail CLOSED — an + // unreadable/malformed ledger for an ADOPTED workstream (the ledger file + // exists) is treated as "present, no trustworthy entry", exactly like + // "present, no entry for this phase", NOT as "absent" (pre-adoption). This + // is the fail-open→fail-closed distinction the maintainer's review required. + test('a corrupted ledger with no live report fails closed instead of completing (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-corrupt-no-report' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + // Adopt the ledger for real (a genuine observation), matching the shape + // of an actually-used workstream, THEN corrupt it and remove the report — + // this is the realistic "ledger existed, now can't be trusted, AND the + // evidence that would let the live read cover for it is also gone" case. + const before = inspectWorkstream(tmpDir, 'ws-2645-corrupt-no-report', { active: null }); + assert.equal(before.completed_phases, 0, 'guard: starts correctly gated by the live gaps_found verdict'); + + fs.unlinkSync(path.join(wsDir, 'phases', '1-foo', '01-VERIFICATION.md')); + fs.writeFileSync(ledgerPath, 'not json{{{'); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-corrupt-no-report', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', + 'a corrupt ledger for an adopted workstream must fail closed, not silently permit completion'); + assert.equal(inv.completed_phases, 0, 'the percentage must not rise just because the ledger became unreadable'); + }); + + // Row 13 — "delete the ledger" case #1: the ledger file is removed, but + // the phase's report is STILL PRESENT and still fails verification. This + // must never raise completeness (the live read governs regardless of + // ledger presence), and the deletion must be SELF-HEALING: the very next + // read that observes the still-present real verdict recreates the ledger. + test('deleting only the ledger file (report still present) does not raise completeness and self-heals (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-delete-ledger-only' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + inspectWorkstream(tmpDir, 'ws-2645-delete-ledger-only', { active: null }); // adopt + assert.ok(fs.existsSync(ledgerPath), 'guard: the ledger must exist before deleting it is meaningful'); + + fs.unlinkSync(ledgerPath); + const afterLedgerDelete = inspectWorkstream(tmpDir, 'ws-2645-delete-ledger-only', { active: null }); + assert.equal(afterLedgerDelete.completed_phases, 0, + 'the still-present gaps_found report must keep this gated regardless of the ledger'); + assert.ok(fs.existsSync(ledgerPath), 'a read that observes a real verdict must recreate/self-heal the ledger'); + }); + + // Row 14 — the DISCLOSED, ACCEPTED residual gap, pinned deliberately so it + // is never mistaken for a silent regression: deleting the report AND the + // ledger TOGETHER, after a failing verdict was already observed and + // recorded, returns this phase key to the pre-adoption `'absent'` ledger + // state — indistinguishable, by design, from a workstream that never used + // the verifier at all (criteria 2/3 require exactly that indistinguishability + // for a workstream that HASN'T adopted the ledger). This is the "prospective + // only" limitation named in the changeset: any durable store that must fail + // OPEN when wholly absent (to avoid gating every pre-existing project on + // upgrade) has this property at its own root. The bar is raised from "delete + // one file" to "delete two files in two different directories, one of which + // this issue's own reproduction never needed to touch" — not eliminated. + test('deleting the report AND the ledger together reopens the pre-adoption window (documented, not a regression) (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-delete-both' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + inspectWorkstream(tmpDir, 'ws-2645-delete-both', { active: null }); // adopt + observe gaps_found + assert.ok(fs.existsSync(ledgerPath)); + + fs.unlinkSync(path.join(wsDir, 'phases', '1-foo', '01-VERIFICATION.md')); + fs.unlinkSync(ledgerPath); + + const inv = inspectWorkstream(tmpDir, 'ws-2645-delete-both', { active: null }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'complete', + 'documented limitation: removing BOTH files returns this phase to the pre-adoption/never-verified state — ' + + 'this is the accepted "prospective only" boundary, not a bug in this fix'); + }); + + // Row 11 — fault injection per CONTRIBUTING.md: read-only target directory + // / partial write failure. Monkeypatch `fs.writeFileSync` to throw only for + // the ledger's WRITE TARGET (delegating everything else to the real + // implementation, since STATE.md/ROADMAP.md/phase files also go through + // the same fs module) — `inspectWorkstream` must still return a valid + // inventory rather than propagating the write failure. Restored via + // `t.after()` (never `try/finally` in a test body — CONTRIBUTING.md:344 — + // and never `chmod 0o000`, which root bypasses in CI). + // + // #2645 review: the write target is the ledger's TEMP file + // (`..tmp`), not `ledgerPath` itself — the write is + // atomic (temp file + rename, #2645 review). Matching only the exact + // final path here made this mock a no-op once atomicity landed (the real + // write always succeeded, so the assertion that follows failed): fixed to + // match anything starting with `ledgerPath`, which covers the temp file + // regardless of its exact suffix. + test('a write failure on the ledger file does not break inspectWorkstream (#2645)', (t) => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-write-fail' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + const originalWriteFileSync = fs.writeFileSync; + fs.writeFileSync = (targetPath, ...rest) => { + if (typeof targetPath === 'string' && targetPath.startsWith(ledgerPath)) { + throw new Error('injected: simulated read-only target directory'); + } + return originalWriteFileSync(targetPath, ...rest); + }; + t.after(() => { fs.writeFileSync = originalWriteFileSync; }); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-write-fail', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', 'the inventory is still correct even though persistence failed'); + assert.ok(!fs.existsSync(ledgerPath), 'the failed write must not have left a partial/corrupt ledger file'); + const leftoverTmp = fs.readdirSync(wsDir).filter(name => name.startsWith('.verification-ledger.json.') && name.endsWith('.tmp')); + assert.deepEqual(leftoverTmp, [], 'a failed temp-file write must not leave an orphaned temp file behind'); + }); + + // Row 12 — the read-side counterpart: a read failure on the ledger path + // specifically (e.g. permission denied) must also degrade rather than + // throw, and must not mask the live on-disk verdict for THIS call. + test('a read failure on the ledger file does not break inspectWorkstream (#2645)', (t) => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-read-fail' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | Complete | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + // Seed a ledger entry via a real (unmocked) observation first. + inspectWorkstream(tmpDir, 'ws-2645-read-fail', { active: null }); + assert.ok(fs.existsSync(ledgerPath), 'guard: the ledger must exist before the read-failure case is meaningful'); + + const originalReadFileSync = fs.readFileSync; + fs.readFileSync = (targetPath, ...rest) => { + if (typeof targetPath === 'string' && targetPath === ledgerPath) { + throw new Error('injected: simulated permission denied'); + } + return originalReadFileSync(targetPath, ...rest); + }; + t.after(() => { fs.readFileSync = originalReadFileSync; }); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-read-fail', { active: null }); }); + assert.ok(inv); + // The ledger read failed (treated as 'corrupt', not 'absent', since the + // file DOES exist) — this workstream has adopted the ledger, so this + // falls CLOSED. The report is still live on disk with a real + // (non-missing) gaps_found verdict though, so the phase is correctly + // in_progress from the LIVE read regardless — the fail-closed path isn't + // even reached for this phase (its live read isn't 'missing'). + assert.equal(inv.phases[0].status, 'in_progress'); + }); + + // Row 15 — CONTRIBUTING.md fault-injection: broken symlink. `readFileSync` + // FOLLOWS symlinks, so a broken symlink at the ledger path reports the + // SAME `ENOENT` as genuine absence via errno alone — `readVerificationLedger` + // disambiguates with `lstatSync` (which does NOT follow symlinks) before + // concluding `'absent'`. This is the realistic worst case: the durable + // memory is unreadable AND there is no live report to fall back on. + test('a broken symlink at the ledger path fails closed instead of falling open to pre-adoption (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-broken-symlink' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + const before = inspectWorkstream(tmpDir, 'ws-2645-broken-symlink', { active: null }); // adopt + assert.equal(before.completed_phases, 0, 'guard: gated by the live gaps_found verdict'); + + fs.unlinkSync(path.join(wsDir, 'phases', '1-foo', '01-VERIFICATION.md')); + fs.unlinkSync(ledgerPath); + fs.symlinkSync(path.join(wsDir, 'does-not-exist-target'), ledgerPath); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-broken-symlink', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', + 'a broken symlink is evidence something existed — it must fail closed, not fall open to pre-adoption behavior'); + assert.equal(inv.completed_phases, 0, 'the percentage must not rise because the ledger became a broken symlink'); + }); + + // Row 15b — pins the lstat fail-closed fix specifically. The broken-symlink + // disambiguation in `readVerificationLedger` calls `fs.lstatSync` after + // `fs.readFileSync` reports `ENOENT`, to tell "genuinely absent" from "a + // symlink entry exists, its target does not". A REVIEW caught that the + // `catch` around that `lstatSync` call originally treated ANY lstat + // failure as proof of absence — but only `lstatSync` ITSELF reporting + // `ENOENT` is genuine proof; a DIFFERENT lstat failure (a raced permission + // change, a path component that became inaccessible between the two + // calls, …) is not evidence the file was never there, and must fail + // CLOSED like every other path in that function. Double-fault both calls + // to pin this: `readFileSync` throws `ENOENT` (as it does for a genuinely + // missing path or file), `lstatSync` throws something else (`EACCES`) — + // this combination is impossible to produce with a real filesystem state + // (if the path is genuinely gone, `lstatSync` reports `ENOENT` too), so it + // is monkeypatched directly rather than constructed on disk; this is what + // "a future edit could re-widen that catch and fall open again silently" + // means without a test — the double-fault CANNOT be exercised any other + // way. Restored via `t.after()`. + test('a non-ENOENT lstat failure during broken-symlink disambiguation fails closed, not open (#2645)', (t) => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-lstat-double-fault' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + const before = inspectWorkstream(tmpDir, 'ws-2645-lstat-double-fault', { active: null }); // adopt + assert.equal(before.completed_phases, 0, 'guard: gated by the live gaps_found verdict'); + fs.unlinkSync(path.join(wsDir, 'phases', '1-foo', '01-VERIFICATION.md')); + + const originalReadFileSync = fs.readFileSync; + const originalLstatSync = fs.lstatSync; + fs.readFileSync = (targetPath, ...rest) => { + if (typeof targetPath === 'string' && targetPath === ledgerPath) { + throw Object.assign(new Error('injected: simulated missing ledger'), { code: 'ENOENT' }); + } + return originalReadFileSync(targetPath, ...rest); + }; + fs.lstatSync = (targetPath, ...rest) => { + if (typeof targetPath === 'string' && targetPath === ledgerPath) { + throw Object.assign(new Error('injected: simulated raced permission fault'), { code: 'EACCES' }); + } + return originalLstatSync(targetPath, ...rest); + }; + t.after(() => { + fs.readFileSync = originalReadFileSync; + fs.lstatSync = originalLstatSync; + }); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-lstat-double-fault', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', + 'a non-ENOENT lstat failure is not proof of absence — it must fail closed (corrupt), not fall open (absent)'); + assert.equal(inv.completed_phases, 0, 'the percentage must not rise because of an ambiguous double-fault read'); + }); + + // Row 16 — CONTRIBUTING.md fault-injection: missing/uncreatable parent + // directory for the write side. Monkeypatch `fs.mkdirSync` to throw only + // for the workstream directory (delegating everything else), simulating a + // parent directory `writeVerificationLedger` cannot ensure exists. + test('an uncreatable parent directory on write does not break inspectWorkstream (#2645)', (t) => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-no-parent-dir' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + const originalMkdirSync = fs.mkdirSync; + fs.mkdirSync = (targetPath, ...rest) => { + if (typeof targetPath === 'string' && targetPath === wsDir) { + throw new Error('injected: simulated missing/uncreatable parent directory'); + } + return originalMkdirSync(targetPath, ...rest); + }; + t.after(() => { fs.mkdirSync = originalMkdirSync; }); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-no-parent-dir', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', 'the live report still governs even though persistence failed'); + assert.ok(!fs.existsSync(ledgerPath), 'a write that could not ensure its directory must not leave a ledger file'); + }); + + // Row 17 — CONTRIBUTING.md fault-injection: rename failure and temp-file + // cleanup, now applicable because the write is atomic (temp file + rename, + // #2645 review). Monkeypatch `fs.renameSync` to throw only for the ledger's + // rename — the temp file must be written, the rename must fail, and the + // orphaned temp file must be cleaned up rather than accumulating. + test('a rename failure during the atomic ledger write cleans up the temp file (#2645)', (t) => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-rename-fail' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), FLAT_STATE); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), flatRoadmap([ + '| 1. Foo | 1/1 | In Progress | - |', + ])); + writePhase(wsDir, '1-foo', { plans: 1, summaries: 1, verification: 'gaps_found' }); + const ledgerPath = path.join(wsDir, '.verification-ledger.json'); + + const originalRenameSync = fs.renameSync; + fs.renameSync = (src, dest) => { + if (typeof dest === 'string' && dest === ledgerPath) { + throw Object.assign(new Error('injected: simulated rename failure'), { code: 'EPERM' }); + } + return originalRenameSync(src, dest); + }; + t.after(() => { fs.renameSync = originalRenameSync; }); + + let inv; + assert.doesNotThrow(() => { inv = inspectWorkstream(tmpDir, 'ws-2645-rename-fail', { active: null }); }); + assert.ok(inv); + assert.equal(inv.phases[0].status, 'in_progress', 'the live report still governs even though persistence failed'); + assert.ok(!fs.existsSync(ledgerPath), 'a failed rename must not leave a ledger file at the final path'); + const leftoverTmp = fs.readdirSync(wsDir).filter(name => name.startsWith('.verification-ledger.json.') && name.endsWith('.tmp')); + assert.deepEqual(leftoverTmp, [], 'a failed rename must not leave an orphaned temp file behind'); + }); + + // Row 18 — BLOCKER regression, and THIS IS THE ONLY ROW THAT PROVES IT + // END TO END. `pickRollupWinners` (`workstream-inventory-builder.cts`) is + // the SINGLE shared implementation both `buildWorkstreamInventory`'s + // `rollupDirByKey` and `inspectWorkstream`'s ledger-winner selection now + // call. An earlier version of the ledger selection was a SEPARATE + // hand-written copy that omitted the `includeItem` (milestone-scoping) + // filter — so in a scoped workstream, a stale OUT-of-milestone directory + // sharing a phase key with the live IN-milestone one, with a newer mtime + // (plausible after a checkout/rebase resets mtimes), could win the + // LEDGER's selection while losing the BUILDER's. `isLedgerWinner` would + // then be false for the live directory, so deleting ITS + // `*-VERIFICATION.md` would never consult the ledger — reopening #2645's + // hole for the phase that actually counts toward `completed_phases`, + // reachable with a plain `rm`, no ledger tampering. + // + // This test constructs that EXACT collision directly — one synthetic key + // shared by two entries, one included (in-milestone) and one excluded + // (out-of-milestone), the excluded one given the larger mtime — and + // proves `pickRollupWinners` applies the filter BEFORE comparing mtimes. + // + // Row 19 (below) does NOT reproduce this same-key collision through the + // real `phaseKeyFromDir` / `isDirInCurrentMilestone` pipeline — extensive + // probing (documented in `10-diagnosis.md`'s "Fourth note") found no + // directory-naming pair that shares a rollup key while diverging in + // milestone membership under the CURRENT roadmap-parser implementation; + // every membership-determining path collapses to the same phase-number + // extraction the rollup key already uses. Row 19 instead pins a narrower, + // real, DISTINCTLY-keyed guarantee (an out-of-milestone phase's verdict is + // never written into the ledger at all) — genuinely useful coverage, but + // NOT a substitute for this row: it does not exercise the collision path, + // and the pre-fix code would have passed it too. + test('pickRollupWinners: an excluded (out-of-milestone) item can never win over an included one, even with a newer mtime (#2645)', () => { + const liveInMilestone = { key: 'shared', mtimeMs: 100, inMilestone: true, label: 'live' }; + const staleOutOfMilestone = { key: 'shared', mtimeMs: 999999, inMilestone: false, label: 'stale' }; // far newer mtime, but excluded + const scoped = true; + + const winners = pickRollupWinners( + [liveInMilestone, staleOutOfMilestone], // pre-sorted input, as both real call sites provide + (item) => item.key, + (item) => item.mtimeMs, + (item) => !(scoped && item.inMilestone === false), + ); + + assert.equal(winners.get('shared'), liveInMilestone, + 'the excluded stale directory must never win the key, regardless of its mtime advantage'); + assert.equal(winners.get('shared').label, 'live'); + + // The mirror image, sorted the OTHER way — order-of-iteration must not + // matter once the filter is applied; only inclusion + mtime should. + const winnersReversed = pickRollupWinners( + [staleOutOfMilestone, liveInMilestone], + (item) => item.key, + (item) => item.mtimeMs, + (item) => !(scoped && item.inMilestone === false), + ); + assert.equal(winnersReversed.get('shared'), liveInMilestone); + + // Unscoped (scoped=false): the exclusion never engages, so the newer + // mtime wins normally — pinning that the filter is scoping-conditional, + // not an unconditional "in-milestone always wins" rule. + const unscoped = false; + const winnersUnscoped = pickRollupWinners( + [liveInMilestone, staleOutOfMilestone], + (item) => item.key, + (item) => item.mtimeMs, + (item) => !(unscoped && item.inMilestone === false), + ); + assert.equal(winnersUnscoped.get('shared'), staleOutOfMilestone, + 'when scoping is off, the plain newest-mtime rule applies with no exclusion'); + }); + + // Row 19 — NOT the collision blocker (see Row 18's comment — this does + // not reproduce it and the pre-fix code would pass this too). What this + // DOES cover: in a genuinely SCOPED workstream (roadmap has a Milestone + // column, STATE.md carries `milestone:`), a DISTINCTLY-keyed + // out-of-milestone phase's real verdict must never be written into the + // ledger at all — proving the REAL `inspectWorkstream` read/write path + // threads `scoped`/`inMilestone` into the ledger write gate correctly, end + // to end, for the (much more common) non-colliding case. `1-old` (v1.0) + // and `2-new` (v2.0) are DIFFERENT phase keys, not a shared one. + test('milestone scoping: a distinctly-keyed out-of-milestone phase is never written into the ledger (#2645)', () => { + const wsDir = seedWorkstream(tmpDir, { name: 'ws-2645-scoped-ledger' }); + fs.writeFileSync(path.join(wsDir, 'STATE.md'), 'milestone: v2.0\nstatus: executing\n'); + fs.writeFileSync(path.join(wsDir, 'ROADMAP.md'), [ + '# Roadmap', '', '## Progress', '', + '| Phase | Milestone | Plans Complete | Status | Completed |', + '| --- | --- | --- | --- | --- |', + '| 1. Old | v1.0 | 1/1 | Complete | - |', + '| 2. New | v2.0 | 1/1 | In Progress | - |', + '', + ].join('\n')); + writePhase(wsDir, '1-old', { plans: 1, summaries: 1, verification: 'gaps_found' }); // prior milestone + writePhase(wsDir, '2-new', { plans: 1, summaries: 1, verification: 'gaps_found' }); // current milestone + + const inv = inspectWorkstream(tmpDir, 'ws-2645-scoped-ledger', { active: null }); + assert.ok(inv); + // Guard: confirm scoping is actually active and phase 1 really is + // excluded from the rollup — otherwise this test would not exercise + // anything. + assert.equal(inv.roadmap_phase_count, 1, 'guard: denominator = v2.0 phases only, scoping is active'); + + const ledgerRaw = fs.readFileSync(path.join(wsDir, '.verification-ledger.json'), 'utf-8'); + const ledger = JSON.parse(ledgerRaw); + assert.ok(!('01' in ledger) && !('1' in ledger), + 'the out-of-milestone phase (1-old) must never be written into the ledger, regardless of its key form'); + assert.ok(('02' in ledger) || ('2' in ledger), + 'the in-milestone phase (2-new) must be recorded normally'); + }); +});