From 003d982c83bb26ab4875539693c418723c5a9eb7 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Wed, 16 Sep 2026 04:26:44 -0500 Subject: [PATCH] fix(#4623): keep repo-wide planning docs out of the verification digest, and accept --files on verification.fingerprint (#4749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#4623): keep repo-wide planning docs out of the verification digest, and accept --files on verification.fingerprint Two defects in the covered-input fingerprint (#4155), one issue. 1. `computeCoveredDigest` hashed the whole bytes of every declared path uniformly, so `.planning/ROADMAP.md` and `.planning/REQUIREMENTS.md` — which every phase rewrites as ordinary bookkeeping, and which the closing phase's own `phase.complete` / `requirements mark-complete` rewrite AFTER the verifier ran — flipped every phase that declared them to `stale` on zero implementation change, and from there `isPhaseComplete` → `init.manager` → `complete-milestone`'s `ALL_PHASES_VERIFIED` gate. Fingerprint v2 leaves any direct child of a planning root out of the hash: `.planning/` itself, plus the phase's own planning root (the parent of its `phases/`, so `planningDir`'s `/` and `workstreams//` layouts are covered without the digest knowing what a workstream is — `sharedPlanningRoots` / `isSharedPlanningDoc`, defined by position rather than a name list so the set cannot drift; a root is accepted only when the phase dir sits under a `phases/` directory inside `.planning/`). Such a path is still validated exactly as every other covered path (confined, present, a regular file — the fail-closed contract is unchanged); only its bytes are ignored, and a declaration made only of shared documents fails closed like an empty one. A stored digest names its version, and `readVerificationStatus` now recomputes under THAT version (`parseFingerprintVersion`, `KNOWN_FINGERPRINT_VERSIONS`): a legacy v1 report keeps v1 semantics until it is re-fingerprinted, so the upgrade alone stales nothing; a version this build cannot recompute fails closed. 2. `verification.fingerprint` received a raw positional slice, so `--files a`, `--files "a,b"` and `--files a --files b` all put the literal token into the covered set and failed closed as "a covered file is missing, unreadable, or escapes the project root" — the message that convinced the reporting project the digest was permanently unrecomputable. `parseFingerprintFileArgs` accepts every form (plus `--files=a,b`, freely mixed with bare positionals), treats any other `--flag` and an empty `--files` value as usage errors that say so, and the phase-dir argument must now be an existing directory: omitting it used to take the first covered file as the phase dir and print a plausible digest over the rest at exit 0. Regression tests (tests/verification-status.test.cjs, #4623 block): the cross-phase case from the report, the same-phase `requirements mark-complete` / `phase.complete` cases from the thread, a workstream-scoped root, v1-preserved / unknown-version-stale, the fail-closed cases (missing, directory, escaping symlink, all-shared), every `--files` form against the bare form, the unknown-flag / empty-value / omitted-phase-dir errors, and AC5's zero-file error. Verified failing against the pre-fix source: 29 of 34 fail, the 7 that pass pin behaviour the fix must leave unchanged. Docs: CONTEXT.md Verification Module, agents/gsd-verifier.md's covered_files instruction (rewritten in place — the file sits 21 bytes under its LARGE hard cap), gsd-core/templates/verification-report.md. Fixes #4623 Emitted-Drift-Ack-Growth: gsd-verifier.md — the #4155 covered_files instruction now states that planning-root docs are digest-inert (#4623); +18 bytes, under the LARGE cap Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DCMY8P8s6dp4g3Rxu3nNAi * chore(#4623): set changeset fragment pr to 4749 --------- Co-authored-by: Claude Fable 5.1 Co-authored-by: Tom Boucher --- .changeset/bold-jaguars-wake.md | 5 + CONTEXT.md | 2 +- agents/gsd-verifier.md | 4 +- gsd-core/templates/verification-report.md | 2 +- src/verification.cts | 251 +++++++++- tests/verification-status.test.cjs | 545 +++++++++++++++++++++- 6 files changed, 795 insertions(+), 14 deletions(-) create mode 100644 .changeset/bold-jaguars-wake.md diff --git a/.changeset/bold-jaguars-wake.md b/.changeset/bold-jaguars-wake.md new file mode 100644 index 000000000..8c3dcf31e --- /dev/null +++ b/.changeset/bold-jaguars-wake.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4749 +--- +**A phase's verification no longer goes `stale` because a repo-wide planning document was rewritten** — `covered_digest` hashed `.planning/ROADMAP.md` and `.planning/REQUIREMENTS.md` byte-for-byte, so completing any phase (or the phase's own `phase.complete` / `requirements mark-complete` bookkeeping) flipped every verification that had declared them to `stale`, failed `complete-milestone`'s `ALL_PHASES_VERIFIED` gate, and forced an `override_closeout` for phases whose implementation had not changed. Fingerprint v2 leaves the repo-wide planning documents — the direct children of the planning root, including a workstream's own — out of the digest by construction (they are still validated, only their bytes are ignored); an existing v1 digest keeps its old meaning until the report is re-fingerprinted, so upgrading stales nothing. Separately, `query verification.fingerprint` now accepts `--files a`, `--files a,b` and repeated `--files` alongside the bare positional form, reports an unknown flag as a usage error instead of "a covered file is missing", and rejects a missing phase directory instead of printing a digest over the wrong set at exit 0. (#4623) diff --git a/CONTEXT.md b/CONTEXT.md index 9de41adf6..84ad3be60 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -30,7 +30,7 @@ Module owning phase create, rename, complete, remove, list, and plan-index opera Module owning phase-effort estimation and its calibration against measured reality (ADR-2629, epic #1952). Pure — no I/O, no config reads; the CLI seam (`src/estimate-cli.cts`, verbs `estimate-check` / `estimate-calibration`) owns reading `.planning/config.json` and `.planning/estimation-calibration.json`. Interface: `parseEstimate`/`renderEstimate` (the PLAN.md `estimate: {tokens, tasks, confidence}` block), `parseActuals`/`renderActuals` (the SUMMARY.md `actuals: {tokens, tasks, commits}` block), `deriveConfidence(sampleCount) → low|med|high`, `classifyAgainstBudget(estimate, budget) → {overBudget, ratio, recommendation, budgetValid}`, `computeCalibration(samples) → {factor, sampleCount, applied, confidence, clamped}`, `applyCalibration`, `parseCalibrationDocument`/`renderCalibrationDocument`, `extractFrontmatterBlock` (leading-`---`-anchored scalar-block reader; stays hand-rolled for its TYPE CONTRACT — it returns numeric-looking values as numbers so `parseEstimate`/`parseActuals` see the types they validate, whereas the migrated `extractFrontmatter`, ADR-3473 §8.1/#3881, resolves every scalar as a string under js-yaml's FAILSAFE_SCHEMA; migrating this function onto the shared parser is follow-on work under ADR-3473 §8.1, not done), `calibrationBasis` (returns `estimate.raw_tokens` when present, else `tokens` — calibration must measure actual/raw or the loop un-corrects itself), and `measureTokens` (a re-export of `prompt-budget`'s `estimateTokens`). **Domain terms: _raw_ vs _calibrated_ tokens** — the same token count in two mutually incompatible states, carried by the compile-time brands `RawTokens` (the planner's uncorrected projection, and the only legal calibration denominator) and `CalibratedTokens` (the projection with the project's factor applied, and the only figure meaningful against the budget), constructed at trust boundaries via `asRawTokens` / `asCalibratedTokens` (#2671). The brands erase at compile time — the emitted `.cjs`, the CLI JSON, and both frontmatter schemas are unchanged — and exist because mixing the two states was NOT catchable at runtime: both are positive integers of the same magnitude, and the mix-up shipped twice past a green ~26,800-test suite (#2631 factor², #2632 self-defeating loop). `asRawTokens` refuses a `CalibratedTokens` by design; the single legitimate crossover (a pre-#2632 plan whose `tokens` IS the raw projection) lives behind one commented assertion in `calibrationBasis`. Compile fixtures: `tests/fixtures/brand-typing/`. **_smart zone_** — the usable prefix of a model's context window before output quality degrades, expressed as the configurable `workflow.smart_zone_tokens` budget (default 100000, a *policy default* rather than a benchmark constant since the effective ceiling is model/task-dependent); **_estimate/actuals_** — a projected phase cost recorded at plan time and the measured cost recorded at completion, both on the **same `estimateTokens` scale** so their ratio measures the miss rather than a difference between two measurement methods. Two invariants: (1) every signal is **exogenous** — the correction routes on a measured actual/estimate ratio and `confidence` routes on a calibration sample count, never on a model's self-assessment (this project measured self-rated confidence and found it weak — `gsd-core/references/honest-verifier.md:25-29`; see `.out-of-scope/general-purpose-agent-prompt-skills.md`); (2) the over-budget flag is **advisory** — a warning plus a split recommendation, never a block. Calibration is median-of-ratios, clamped to `[0.5, 3.0]`, and inert below 3 samples. CLI seam verbs: `estimate-check` (classify one figure; `--calibrated` when the input already has the factor applied — omitting it squares the correction), `estimate-calibration` (report the current factor), `estimate-calibrate` (#2632 — pair every completed phase's PLAN `estimate` with its SUMMARY `actuals`, rebuild `.planning/estimation-calibration.json` idempotently, and report the result; this is what closes the loop). Source of truth: `gsd-core/bin/lib/phase-estimation.cjs` and `src/estimate-cli.cts`. Test anchors: `tests/phase-estimation.test.cjs`, `tests/estimate-calibrate.test.cjs`. ### Verification Module -Module owning the canonical phase-verification status projection shared by phase transition, progress, manager, autonomous, and closeout readiness paths. `readVerificationStatus(phaseDir, opts?)` reads the first `*-VERIFICATION.md` frontmatter `status`, maps it through `VERIFICATION_ROUTING_TABLE`, and fail-closes — only `{passed}` satisfies the canonical gate; `missing`/`unknown`/`gaps_found`/`human_needed`/`stale` all route away from "complete" (#1522). Staleness is one of TWO mutually exclusive checks, selected by whether the report DECLARES a fingerprint (#4155, i.e. either `covered_files` or `covered_digest` is present — a report that declares only one, or an empty/malformed pair, fails closed to `stale` rather than silently downgrading to the weaker legacy check): a report with a well-formed pair is checked by `computeCoveredDigest(projectRoot, coveredFiles)` — a deterministic sha256-of-sha256s over the SORTED, de-duplicated, `./`/`..`-canonicalized covered set, hashed by file BYTES always through the real `node:fs`, never a caller-injected `opts.fs` seam (`covered_files` spans the whole `projectRoot`, not just `.planning/`, so a caller-scoped containment wrapper narrower than `projectRoot` would reject legitimate covered files; the `realRel`-vs-`realRoot` re-check inside `computeCoveredDigest` is the actual confinement boundary, against `projectRoot`) — recomputed and compared to the stored digest. `allCurrentArtifactsCovered` additionally re-scans the LIVE phase directory (via `scanPhasePlans`, short-circuited so it only runs once the digest itself has matched, and fails closed to `false` on any scope other than `SCOPE.COMPLETE`) for every current `*-PLAN.md`/`*-SUMMARY.md` and requires each to be represented in `covered_files`: a plan or summary added to the phase AFTER verification, never declared, would otherwise pass the digest check untouched. ANY mismatch — digest, an uncovered current artifact, or a covered path that is missing, unreadable, or escapes `projectRoot` (`findProjectRoot(phaseDir)`, canonicalized via real `fs`, never the injected seam — it is a trusted caller-derived anchor, not attacker-influenced covered-input data) — routes to `stale`, fail-closed (`computeCoveredDigest` returns `null` for the whole set rather than a partial digest). A report with no fingerprint metadata at all (every report predating #4155) falls back to the legacy `findStaleVerificationSummary`, which flags a SUMMARY newer than the VERIFICATION file — unchanged. The verifier computes `covered_digest` via the CLI (`verification.fingerprint ...`, routed in `verification-command-router.cjs`), never by hand — this is deterministic math, not an LLM-estimated value. The legacy path alone keeps the module's original no-throw, degrade-to-safe contract (any FS error → `missing` / not-stale); the fingerprint path's contract is stricter (any FS error → `stale`), matching #4155's fail-closed design. `isPhaseComplete(phaseDir, deps?)` is the single canonical owner of "is phase P complete?" (ADR-3180 §7.4, issue #3186, disk-strict per #2957): it wraps `readVerificationStatus`, calling it UNCONDITIONALLY — plan count is never a precondition, so a zero-plan phase with a passing `*-VERIFICATION.md` is complete (#3168) — and returns `{ value: { complete, verification }, scope }`; `complete` is exactly `verification.status === 'passed'`. A ROADMAP checkbox carries no machine authority and is never consulted. `cmdPhaseComplete`, `buildPhaseCompletionProjection`, and `buildStateFrontmatter` all route through it. Source of truth: `gsd-core/bin/lib/verification.cjs` (generated from `src/verification.cts`). +Module owning the canonical phase-verification status projection shared by phase transition, progress, manager, autonomous, and closeout readiness paths. `readVerificationStatus(phaseDir, opts?)` reads the first `*-VERIFICATION.md` frontmatter `status`, maps it through `VERIFICATION_ROUTING_TABLE`, and fail-closes — only `{passed}` satisfies the canonical gate; `missing`/`unknown`/`gaps_found`/`human_needed`/`stale` all route away from "complete" (#1522). Staleness is one of TWO mutually exclusive checks, selected by whether the report DECLARES a fingerprint (#4155, i.e. either `covered_files` or `covered_digest` is present — a report that declares only one, or an empty/malformed pair, fails closed to `stale` rather than silently downgrading to the weaker legacy check): a report with a well-formed pair is checked by `computeCoveredDigest(projectRoot, coveredFiles)` — a deterministic sha256-of-sha256s over the SORTED, de-duplicated, `./`/`..`-canonicalized covered set, hashed by file BYTES always through the real `node:fs`, never a caller-injected `opts.fs` seam (`covered_files` spans the whole `projectRoot`, not just `.planning/`, so a caller-scoped containment wrapper narrower than `projectRoot` would reject legitimate covered files; the `realRel`-vs-`realRoot` re-check inside `computeCoveredDigest` is the actual confinement boundary, against `projectRoot`) — recomputed and compared to the stored digest. Fingerprint **v2** (#4623) leaves repo-wide planning documents — any DIRECT child of a planning root: `.planning/` itself, and the phase's own planning root (the parent of its `phases/`, so the `.planning//` and `workstreams//` layouts of `planningDir` are covered without the digest knowing what a workstream is; `sharedPlanningRoots` / `isSharedPlanningDoc`, defined by position so the set cannot drift as new top-level documents appear) — OUT OF THE HASH while still validating them exactly as every other covered path (confined to `projectRoot`, present, a regular file — the fail-closed contract is unchanged; only the bytes are ignored): every phase rewrites them as ordinary bookkeeping, and the closing phase's own `phase.complete` / `requirements mark-complete` writes them AFTER the verifier ran, so under v1 a phase staled itself and every sibling that had declared them. A declaration consisting ONLY of shared documents hashes nothing and is `null` (stale), like an empty one. A stored digest names its version (`v:sha256:…`) and is recomputed under THAT version (`parseFingerprintVersion`, `KNOWN_FINGERPRINT_VERSIONS`), never the current constant, so a legacy v1 report keeps v1 semantics (shared documents included) until it is re-fingerprinted, an upgrade alone stales nothing, and a version this build cannot recompute fails closed to `stale`. `allCurrentArtifactsCovered` additionally re-scans the LIVE phase directory (via `scanPhasePlans`, short-circuited so it only runs once the digest itself has matched, and fails closed to `false` on any scope other than `SCOPE.COMPLETE`) for every current `*-PLAN.md`/`*-SUMMARY.md` and requires each to be represented in `covered_files`: a plan or summary added to the phase AFTER verification, never declared, would otherwise pass the digest check untouched. ANY mismatch — digest, an uncovered current artifact, or a covered path that is missing, unreadable, or escapes `projectRoot` (`findProjectRoot(phaseDir)`, canonicalized via real `fs`, never the injected seam — it is a trusted caller-derived anchor, not attacker-influenced covered-input data) — routes to `stale`, fail-closed (`computeCoveredDigest` returns `null` for the whole set rather than a partial digest). A report with no fingerprint metadata at all (every report predating #4155) falls back to the legacy `findStaleVerificationSummary`, which flags a SUMMARY newer than the VERIFICATION file — unchanged. The verifier computes `covered_digest` via the CLI (`verification.fingerprint ...`, routed in `verification-command-router.cjs`; since #4623 the covered files are bare positionals or `--files a[,b]`, repeatable and mixable, the phase dir must be an existing directory — an omitted one used to hash the wrong set at exit 0 — and any other `--flag` is a usage error rather than a covered path), never by hand — this is deterministic math, not an LLM-estimated value. The legacy path alone keeps the module's original no-throw, degrade-to-safe contract (any FS error → `missing` / not-stale); the fingerprint path's contract is stricter (any FS error → `stale`), matching #4155's fail-closed design. `isPhaseComplete(phaseDir, deps?)` is the single canonical owner of "is phase P complete?" (ADR-3180 §7.4, issue #3186, disk-strict per #2957): it wraps `readVerificationStatus`, calling it UNCONDITIONALLY — plan count is never a precondition, so a zero-plan phase with a passing `*-VERIFICATION.md` is complete (#3168) — and returns `{ value: { complete, verification }, scope }`; `complete` is exactly `verification.status === 'passed'`. A ROADMAP checkbox carries no machine authority and is never consulted. `cmdPhaseComplete`, `buildPhaseCompletionProjection`, and `buildStateFrontmatter` all route through it. Source of truth: `gsd-core/bin/lib/verification.cjs` (generated from `src/verification.cts`). `SEAM.verification-isphasecomplete.owns=isPhaseComplete(phaseDir, deps?) is the single canonical owner of "is phase P complete?" (ADR-3180 §7.4, issue #3186)` `SEAM.verification-isphasecomplete.enforced-by=test:tests/verification-status.test.cjs` diff --git a/agents/gsd-verifier.md b/agents/gsd-verifier.md index c487d4e83..2fa3eb5e2 100644 --- a/agents/gsd-verifier.md +++ b/agents/gsd-verifier.md @@ -668,7 +668,7 @@ If `valid != true`, refuse to verify. Surface the discrepancy and ask the user t **ALWAYS use the Write tool to create files** — never use `Bash(cat << 'EOF')` or heredoc commands for file creation. -**#4155:** `covered_files`: every phase PLAN/SUMMARY (+superseded, nested `plans/`), mapped requirement, changed impl file — ROOT-relative. `gsd_run query verification.fingerprint {phaseDir} {file}...`, copy output — never hand-write `covered_digest`. +**#4155:** `covered_files`: every phase PLAN/SUMMARY (+superseded, nested `plans/`), changed impl file — ROOT-relative; planning-root docs are inert (#4623). `gsd_run query verification.fingerprint {phaseDir} {file}...`, copy output — never hand-write `covered_digest`. Create `.planning/phases/{phase_dir}/{phase_num}-VERIFICATION.md`: @@ -679,7 +679,7 @@ verified: YYYY-MM-DDTHH:MM:SSZ status: passed | gaps_found | human_needed score: N/M must-haves verified covered_files: [...] -covered_digest: "v1:sha256:..." +covered_digest: "v2:sha256:..." behavior_unverified: 0 # Count of ⚠️ PRESENT_BEHAVIOR_UNVERIFIED truths (present + wired, behavior not exercised); each is detailed in behavior_unverified_items below (and in human_verification when status is human_needed) overrides_applied: 0 # Count of PASSED (override) items included in score overrides: # Only if overrides exist — carried forward or newly added diff --git a/gsd-core/templates/verification-report.md b/gsd-core/templates/verification-report.md index ab52300aa..9c5a175e7 100644 --- a/gsd-core/templates/verification-report.md +++ b/gsd-core/templates/verification-report.md @@ -16,7 +16,7 @@ covered_files: # #4155 — see agents/gsd-verifier.md's "Create VERIFICATION.md" - .planning/phases/XX-name/{phase_num}-{plan}-PLAN.md - .planning/phases/XX-name/{phase_num}-{plan}-SUMMARY.md - src/{changed-file}.cts -covered_digest: "v1:sha256:{digest from verification.fingerprint}" +covered_digest: "v2:sha256:{digest from verification.fingerprint}" behavior_unverified: 0 # Count of ⚠️ PRESENT_BEHAVIOR_UNVERIFIED truths (present + wired, behavior not exercised) behavior_unverified_items: # Only if behavior_unverified > 0 — the truths above as structured items; emitted regardless of overall status - truth: "Observable truth whose state transition or cancellation/cleanup/ordering invariant no test exercises" diff --git a/src/verification.cts b/src/verification.cts index 43823a78a..c0cf7ab78 100644 --- a/src/verification.cts +++ b/src/verification.cts @@ -220,8 +220,98 @@ function canonicalizeCoveredFiles(files: readonly string[]): string[] { * Bump on any change to the digest's input shape (path list, hashing order, * per-file hash algorithm) so an old stored digest can never collide with a * differently-computed new one — a version mismatch is just a mismatch. + * + * Version history: + * v1 (#4155) — every covered path's whole bytes, uniformly. + * v2 (#4623) — repo-wide planning documents (`isSharedPlanningDoc`) are + * excluded from the hash by construction. + * + * A stored digest names its own version (`v:sha256:…`), and + * `readVerificationStatus` recomputes under the STORED version rather than + * this constant — so bumping it does not flip every already-verified phase + * to `stale` on upgrade. A legacy v1 report keeps v1 semantics, shared + * documents included, until it is re-fingerprinted; only a version outside + * `KNOWN_FINGERPRINT_VERSIONS` is unrecomputable and fails closed. */ -const FINGERPRINT_VERSION = 1; +const FINGERPRINT_VERSION = 2; +const KNOWN_FINGERPRINT_VERSIONS: ReadonlySet = new Set([1, 2]); + +/** + * #4623: the planning roots whose DIRECT children are repo-wide planning + * documents, as project-root-relative posix paths. Always `.planning`; plus + * the phase's OWN planning root when a phase directory is known — the parent + * of its `phases/` directory, which is how `planningDir` lays out every + * scope (`.planning`, `.planning/`, `.planning/workstreams/`, + * `.planning//workstreams/`; `planning-workspace.cts`). Derived + * from the phase's position rather than from a list of layouts so a + * workstream-scoped `ROADMAP.md` is recognised without this function + * knowing what a workstream is, and so `.planning/research/notes.md` is + * NOT mistaken for one — lexically the two are indistinguishable from + * `.planning//ROADMAP.md`. A phase directory that does not sit + * under the project root (unit fixtures at a bare tmpdir) contributes no + * extra root. + */ +function sharedPlanningRoots(projectRoot: string, phaseDir?: string | null): string[] { + const roots = ['.planning']; + if (phaseDir) { + const phasesDir = path.dirname(path.resolve(phaseDir)); + const planningDir = path.dirname(phasesDir); + const rel = normalizeRel(path.relative(path.resolve(projectRoot), planningDir)); + // Two structural checks, both load-bearing: the phase dir's PARENT must be + // the `phases/` directory `planningDir` lays every scope out with, and the + // derived root must sit inside `.planning/`. Without them any accepted + // directory — `/src/phases/01-fake` — would nominate `src` as a + // planning root and silently drop real implementation evidence from the + // digest (found by the cross-AI review of this change). A shape that fails + // either check contributes no extra root; `.planning` itself is already + // present. + if ( + path.basename(phasesDir) === 'phases' && + rel.startsWith('.planning/') && + !rel.includes('/../') && + !roots.includes(rel) + ) { + roots.push(rel); + } + } + return roots; +} + +/** + * #4623: a covered path names a repo-wide planning document when it sits + * DIRECTLY under one of `sharedPlanningRoots` — `ROADMAP.md`, + * `REQUIREMENTS.md`, `STATE.md`, `PROJECT.md`, `MILESTONES.md`, + * `config.json`, … — as opposed to a phase's own artifacts under + * `/phases//` or a research note under `.planning/research/`. + * Every phase rewrites these as ordinary bookkeeping (a roadmap checkbox, a + * requirement's traceability cell, STATE.md's position), so hashing their + * whole bytes into one phase's digest coupled every phase's staleness to + * every other phase's close — and to its OWN close, since `phase.complete` + * and `requirements mark-complete` write them after the verifier has + * already run. + * + * Defined by position, not by a name list, so the set cannot drift as new + * top-level planning documents appear (the tree already carries a dozen). + * `rel` is expected posix-normalized (`canonicalizeCoveredFiles`), so a + * `./.planning/ROADMAP.md` spelling has already collapsed to the bare form. + */ +function isSharedPlanningDoc(rel: string, roots: readonly string[] = ['.planning']): boolean { + if (rel === '' || rel.endsWith('/')) return false; + return roots.includes(path.posix.dirname(rel)); +} + +/** + * #4623: the fingerprint version a stored `covered_digest` was computed + * under, or `null` when the prefix is absent, malformed, or names a version + * this build cannot recompute (an unknown version is a mismatch by + * construction — the fail-closed shape `FINGERPRINT_VERSION`'s doc promises). + */ +function parseFingerprintVersion(digest: string): number | null { + const m = /^v(\d+):sha256:/.exec(digest); + if (!m) return null; + const version = Number(m[1]); + return KNOWN_FINGERPRINT_VERSIONS.has(version) ? version : null; +} /** * #4155: recompute the deterministic content fingerprint over a verifier's @@ -263,9 +353,22 @@ const FINGERPRINT_VERSION = 1; * confinement work (against `projectRoot`, the correct boundary for this * data), so no security property is lost by bypassing a narrower seam here. */ -function computeCoveredDigest(projectRoot: string, coveredFiles: readonly string[]): string | null { +function computeCoveredDigest( + projectRoot: string, + coveredFiles: readonly string[], + version: number = FINGERPRINT_VERSION, + opts: { phaseDir?: string | null } = {}, +): string | null { + // #4623: `version` selects the input shape to hash under — the CURRENT + // one for a fresh fingerprint (the CLI verb), or the STORED one when + // `readVerificationStatus` recomputes against a report's own digest. + // `opts.phaseDir` lets v2 recognise the phase's own planning root + // (`sharedPlanningRoots`); without it only `.planning/` itself is shared. + if (!KNOWN_FINGERPRINT_VERSIONS.has(version)) return null; const uniqueSorted = canonicalizeCoveredFiles(coveredFiles); if (uniqueSorted.length === 0) return null; + const sharedRoots = version >= 2 ? sharedPlanningRoots(projectRoot, opts.phaseDir) : []; + let hashed = 0; // Canonicalize the root ONCE — every candidate's realpath is checked against // this, not the possibly-symlinked `projectRoot` argument itself. Always via @@ -307,19 +410,33 @@ function computeCoveredDigest(projectRoot: string, coveredFiles: readonly string } const st = fs.statSync(real); if (!st.isFile()) return null; + // #4623 (v2+): a repo-wide planning document is VALIDATED exactly as + // every other covered path — confined, present, a regular file; the + // fail-closed contract above is unchanged — but its bytes contribute + // nothing to the digest. It may stay declared in `covered_files` (the + // verifier's instructions long said to list the mapped requirement, + // and every report already written does); its bookkeeping churn can + // no longer read as drift. + if (isSharedPlanningDoc(rel, sharedRoots)) continue; bytes = fs.readFileSync(real); } catch { return null; } const fileHash = crypto.createHash('sha256').update(bytes).digest('hex'); parts.push(`${rel}\n${fileHash}\n`); + hashed++; } + // #4623 (v2+): a declaration made ONLY of shared planning documents has no + // evidence in it at all — a constant digest over the header would satisfy + // the fingerprint pair while grounding the verification in nothing. Fail + // closed, the same way an empty declaration does. + if (version >= 2 && hashed === 0) return null; const aggregate = crypto .createHash('sha256') - .update(`v${FINGERPRINT_VERSION}\n${parts.join('')}`, 'utf-8') + .update(`v${version}\n${parts.join('')}`, 'utf-8') .digest('hex'); - return `v${FINGERPRINT_VERSION}:sha256:${aggregate}`; + return `v${version}:sha256:${aggregate}`; } /** @@ -963,9 +1080,20 @@ function readVerificationStatus( // short-circuit in turn: the live-directory re-scan (for a plan/summary // added AFTER verification and never declared in covered_files) only // runs once the digest itself has already matched. + // + // #4623: recompute under the STORED digest's own version, not the + // current constant — a v1 report written before the shared-document + // exclusion keeps v1 semantics rather than going stale on upgrade. An + // unknown version parses to `null`, which `computeCoveredDigest` + // refuses (returns `null`), so the compare below fails closed. + const storedVersion = + hasWellFormedFingerprint && typeof coveredDigestVal === 'string' + ? parseFingerprintVersion(coveredDigestVal) + : null; isStale = !hasWellFormedFingerprint || - computeCoveredDigest(findProjectRoot(phaseDir), coveredFilesVal) !== coveredDigestVal || + storedVersion === null || + computeCoveredDigest(findProjectRoot(phaseDir), coveredFilesVal, storedVersion, { phaseDir }) !== coveredDigestVal || !allCurrentArtifactsCovered(phaseDir, coveredFilesVal); } else { const staleCheck = findStaleVerificationSummary( @@ -1156,6 +1284,66 @@ function cmdVerificationResolveFile(cwd: string, phaseDirArg: string | undefined output({ verification_file: verificationPath }, raw, verificationPath); } +/** + * #4623: parse the argv tokens after `verification.fingerprint ` + * into a covered-file list. The router hands over a raw positional slice, + * so every `--files`-style form other `gsd-tools` verbs accept (`commit + * --files a b`, `docs/CLI-TOOLS.md`) used to reach `computeCoveredDigest` + * with the literal token `--files` — or an unsplit `"a,b"` — as a covered + * path, and the whole command failed closed with "a covered file is + * missing, unreadable, or escapes the project root". On the reporting + * project that message convinced two people the digest was permanently + * unrecomputable. + * + * Accepted, all equivalent and freely mixed: + * - bare positionals `a b` (the documented form, unchanged) + * - a single flag `--files a` + * - a comma-separated value `--files a,b` (also `--files=a,b`) + * - a repeated flag `--files a --files b` + * + * Only a `--files` VALUE is comma-split: a bare positional keeps its bytes, + * so the documented form's behaviour on a comma-bearing filename is + * unchanged. Any other `--flag` is an explicit usage error, never a path — + * a mis-typed flag must not fail as "file missing" again. (`--raw` never + * reaches here; the CLI entry point splices it out before routing.) + */ +function parseFingerprintFileArgs(tokens: readonly string[]): { files: string[] } | { error: string } { + const files: string[] = []; + const EMPTY_VALUE = '--files requires at least one path for verification.fingerprint (a path, or a comma-separated list)'; + const splitList = (value: string): string[] => + value + .split(',') + .map((s) => s.trim()) + .filter((s) => s.length > 0); + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i]; + if (token === '--files') { + const value = tokens[i + 1]; + if (value === undefined || value.startsWith('--')) { + return { error: '--files requires a value for verification.fingerprint (a path, or a comma-separated list)' }; + } + const list = splitList(value); + // An empty or all-comma value is a usage error, never a silent no-op — + // the caller would otherwise meet the generic zero-files error and go + // looking for a missing path. + if (list.length === 0) return { error: EMPTY_VALUE }; + files.push(...list); + i++; + } else if (token.startsWith('--files=')) { + const list = splitList(token.slice('--files='.length)); + if (list.length === 0) return { error: EMPTY_VALUE }; + files.push(...list); + } else if (token.startsWith('--')) { + return { + error: `unknown flag ${token} for verification.fingerprint (covered files are bare positionals or --files , repeatable)`, + }; + } else { + files.push(token); + } + } + return { files }; +} + /** * CLI command handler (#4155): compute the covered-input fingerprint the * verifier embeds in VERIFICATION.md frontmatter (`covered_files`, @@ -1171,7 +1359,13 @@ function cmdVerificationResolveFile(cwd: string, phaseDirArg: string | undefined * @param cwd - Current working directory. * @param phaseDirArg - Phase directory path (absolute or relative to cwd); * its project root is the base covered paths resolve against. - * @param files - Covered-input paths, relative to the project root. + * Must be an existing directory (#4623): with the + * phase dir omitted, the first covered file used to be + * taken as the phase dir and the rest hashed — a + * plausible digest over the wrong set, at exit 0. + * @param fileArgs - The argv tokens after the phase dir, parsed by + * `parseFingerprintFileArgs`: covered-input paths + * relative to the project root, bare or via `--files`. * @param raw - Whether to emit raw (non-JSON) output: just the * `covered_digest` string, so `VAR=$(gsd_run query * verification.fingerprint "$PHASE_DIR" ... --raw)` is @@ -1182,18 +1376,36 @@ function cmdVerificationResolveFile(cwd: string, phaseDirArg: string | undefined function cmdVerificationFingerprint( cwd: string, phaseDirArg: string | undefined, - files: string[], + fileArgs: readonly string[], raw: boolean, ): void { if (!phaseDirArg) { error('phase directory required for verification.fingerprint'); return; } + const phaseDir = path.resolve(cwd, phaseDirArg); + let phaseDirIsDir = false; + try { + phaseDirIsDir = fs.statSync(phaseDir).isDirectory(); + } catch { + // not found → not a directory + } + if (!phaseDirIsDir) { + error( + `phase directory not found: ${phaseDirArg} — verification.fingerprint takes the phase directory first, then the covered files`, + ); + return; + } + const parsed = parseFingerprintFileArgs(fileArgs); + if ('error' in parsed) { + error(parsed.error); + return; + } + const files = parsed.files; if (files.length === 0) { error('at least one covered file required for verification.fingerprint'); return; } - const phaseDir = path.resolve(cwd, phaseDirArg); const projectRoot = findProjectRoot(phaseDir); // canonicalizeCoveredFiles here is for the emitted `covered_files` field — // computeCoveredDigest canonicalizes its own `coveredFiles` argument @@ -1202,8 +1414,25 @@ function cmdVerificationFingerprint( // already-canonical list keeps that internal pass a cheap no-op rather // than a second meaningfully different canonicalization. const uniqueSorted = canonicalizeCoveredFiles(files); - const digest = computeCoveredDigest(projectRoot, uniqueSorted); + const digest = computeCoveredDigest(projectRoot, uniqueSorted, FINGERPRINT_VERSION, { phaseDir }); if (digest === null) { + // #4623: name the one null that is NOT a bad path — a declaration made + // only of shared planning documents hashes nothing under v2, and the + // generic message below would send the caller looking for a missing file + // that is not missing. Discriminated AFTER the v2 attempt, and only when a + // v1 pass over the same list (which hashes, and therefore validates, every + // path) succeeds: an all-shared list with a missing or directory member is + // a bad path first, and gets the generic message. + const sharedRoots = sharedPlanningRoots(projectRoot, phaseDir); + if ( + uniqueSorted.every((f) => isSharedPlanningDoc(f, sharedRoots)) && + computeCoveredDigest(projectRoot, uniqueSorted, 1) !== null + ) { + error( + `could not compute fingerprint — every covered file is a repo-wide planning document (direct children of ${sharedRoots.join(', ')} never enter the digest); declare the phase's own artifacts and implementation files`, + ); + return; + } error('could not compute fingerprint — a covered file is missing, unreadable, or escapes the project root'); return; } @@ -1222,5 +1451,9 @@ export = { cmdVerificationStatus, cmdVerificationResolveFile, computeCoveredDigest, + sharedPlanningRoots, + isSharedPlanningDoc, + parseFingerprintVersion, + parseFingerprintFileArgs, cmdVerificationFingerprint, }; diff --git a/tests/verification-status.test.cjs b/tests/verification-status.test.cjs index 706547baf..1db70f2ee 100644 --- a/tests/verification-status.test.cjs +++ b/tests/verification-status.test.cjs @@ -52,7 +52,12 @@ const { resolveUatFile, readVerificationStatus, findStaleVerificationSummary, + isPhaseComplete, computeCoveredDigest, + sharedPlanningRoots, + isSharedPlanningDoc, + parseFingerprintVersion, + parseFingerprintFileArgs, } = require('../gsd-core/bin/lib/verification.cjs'); // #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs. @@ -1759,7 +1764,8 @@ describe('#4155: computeCoveredDigest — direct unit coverage', () => { const d1 = computeCoveredDigest(root, ['a.txt', 'b.txt']); const d2 = computeCoveredDigest(root, ['b.txt', 'a.txt']); assert.equal(d1, d2); - assert.match(d1, /^v1:sha256:[0-9a-f]{64}$/); + // The version prefix is pinned by the #4623 block below; this test is about order-independence. + assert.match(d1, /^v\d+:sha256:[0-9a-f]{64}$/); }); test('a "./"-prefixed path and its bare equivalent → identical digest (canonicalized, not double-counted)', (t) => { @@ -2173,6 +2179,543 @@ describe('#4155: verification.fingerprint CLI', () => { }); }); +// ─── #4623: shared planning documents + fingerprint argv ───────────────────── +// +// Two defects, one issue. (1) `computeCoveredDigest` hashed the whole bytes of +// repo-wide planning documents (`.planning/ROADMAP.md`, `REQUIREMENTS.md`, …) +// into a phase's digest, so any phase's ordinary bookkeeping — including the +// closing phase's OWN checkbox flip — read as drift for every phase that had +// declared them. Fingerprint v2 excludes those documents by construction, and +// a stored v1 digest keeps v1 semantics so an upgrade does not stale every +// already-verified phase. (2) `verification.fingerprint` took a raw positional +// slice: every `--files` form failed closed as "a covered file is missing", +// and an omitted phase dir silently hashed the wrong set at exit 0. + +const NO_GIT_TIMES = { phaseCleanCommitTimesMs: () => new Map() }; + +function makePhase4623(projectDir, name) { + const phaseDir = path.join(projectDir, '.planning', 'phases', name); + fs.mkdirSync(phaseDir, { recursive: true }); + const num = name.split('-')[0]; + fs.writeFileSync(path.join(phaseDir, `${num}-01-PLAN.md`), `# Plan ${name}\n`); + fs.writeFileSync(path.join(phaseDir, `${num}-01-SUMMARY.md`), `# Summary ${name}\n`); + return { + phaseDir, + num, + ownFiles: [ + `.planning/phases/${name}/${num}-01-PLAN.md`, + `.planning/phases/${name}/${num}-01-SUMMARY.md`, + ], + }; +} + +function writeReport4623(phase, coveredFiles, digest) { + const sorted = [...new Set(coveredFiles)].sort(); + fs.writeFileSync( + path.join(phase.phaseDir, `${phase.num}-VERIFICATION.md`), + `---\nstatus: passed\ncovered_files:\n${sorted.map((f) => ` - ${f}`).join('\n')}\ncovered_digest: "${digest}"\n---\n`, + ); +} + +function writeSharedDocs4623(projectDir, { roadmapDone = false, reqDone = false } = {}) { + fs.writeFileSync( + path.join(projectDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n- [${roadmapDone ? 'x' : ' '}] **Phase 1: Alpha**\n- [ ] **Phase 2: Beta**\n`, + ); + fs.writeFileSync( + path.join(projectDir, '.planning', 'REQUIREMENTS.md'), + `# Requirements\n\n- [${reqDone ? 'x' : ' '}] **REQ-01**: The thing works\n\n| REQ-01 | Phase 1 | ${reqDone ? 'Complete' : 'Pending'} |\n`, + ); +} + +const SHARED_DOCS_4623 = ['.planning/ROADMAP.md', '.planning/REQUIREMENTS.md']; + +describe('#4623: isSharedPlanningDoc — a repo-wide planning document is a DIRECT child of .planning/', () => { + test('top-level planning documents are shared, whatever their name', () => { + for (const rel of [ + '.planning/ROADMAP.md', + '.planning/REQUIREMENTS.md', + '.planning/STATE.md', + '.planning/PROJECT.md', + '.planning/MILESTONES.md', + '.planning/config.json', + ]) { + assert.equal(isSharedPlanningDoc(rel), true, rel); + } + }); + + test('phase artifacts, nested planning files, implementation files, and a same-named root file are not', () => { + for (const rel of [ + '.planning/phases/01-example/01-01-PLAN.md', + '.planning/phases/01-example/01-VERIFICATION.md', + '.planning/research/notes.md', + '.planning/milestones/v1.0-ROADMAP.md', + 'src/thing.cts', + 'ROADMAP.md', + '.planning', + '.planning/ROADMAP.md/', + '', + ]) { + assert.equal(isSharedPlanningDoc(rel), false, JSON.stringify(rel)); + } + }); + + test('extra planning roots make their direct children shared; sharedPlanningRoots derives them from the phase dir', (t) => { + const roots = ['.planning', '.planning/workstreams/w']; + assert.equal(isSharedPlanningDoc('.planning/workstreams/w/ROADMAP.md', roots), true); + assert.equal(isSharedPlanningDoc('.planning/workstreams/w/phases/01-a/01-01-PLAN.md', roots), false); + assert.equal(isSharedPlanningDoc('.planning/workstreams/w/ROADMAP.md'), false, 'unknown root without the phase dir'); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-roots-')); + t.after(() => cleanup(root)); + assert.deepEqual(sharedPlanningRoots(root), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning', 'phases', '01-a')), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning', 'proj', 'phases', '01-a')), ['.planning', '.planning/proj']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning', 'proj', 'workstreams', 'w', 'phases', '01-a')), ['.planning', '.planning/proj/workstreams/w']); + // A phase dir outside the project root contributes nothing. + assert.deepEqual(sharedPlanningRoots(root, path.join(os.tmpdir(), 'elsewhere', 'phases', '01-a')), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, root), ['.planning']); + // Structural: the parent must be `phases/` and the root must sit inside .planning/ — an + // arbitrary accepted directory must never nominate its grandparent as a planning root. + assert.deepEqual(sharedPlanningRoots(root, path.join(root, 'src', 'phases', '01-fake')), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning', 'proj', 'notphases', '01-a')), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning', 'phases')), ['.planning']); + assert.deepEqual(sharedPlanningRoots(root, path.join(root, '.planning')), ['.planning']); + }); +}); + +describe('#4623: parseFingerprintVersion', () => { + test('reads the version prefix of a well-formed digest', () => { + assert.equal(parseFingerprintVersion('v1:sha256:' + 'a'.repeat(64)), 1); + assert.equal(parseFingerprintVersion('v2:sha256:' + 'a'.repeat(64)), 2); + }); + + test('an unknown, malformed, or absent version → null (fail closed)', () => { + assert.equal(parseFingerprintVersion('v9:sha256:' + 'a'.repeat(64)), null); + assert.equal(parseFingerprintVersion('sha256:' + 'a'.repeat(64)), null); + assert.equal(parseFingerprintVersion('v2:md5:' + 'a'.repeat(32)), null); + assert.equal(parseFingerprintVersion(''), null); + }); +}); + +describe('#4623: parseFingerprintFileArgs — every --files form the issue tried, plus the documented bare form', () => { + test('bare positionals pass through unchanged, commas included (AC4)', () => { + assert.deepEqual(parseFingerprintFileArgs(['a.rb', 'b.rb']), { files: ['a.rb', 'b.rb'] }); + // Only a --files VALUE is comma-split: the documented form keeps its bytes. + assert.deepEqual(parseFingerprintFileArgs(['a,b']), { files: ['a,b'] }); + assert.deepEqual(parseFingerprintFileArgs([]), { files: [] }); + }); + + test('--files (AC2)', () => { + assert.deepEqual(parseFingerprintFileArgs(['--files', 'fastlane/Fastfile']), { files: ['fastlane/Fastfile'] }); + }); + + test('--files "a,b" and --files=a,b split on commas, trimming and dropping empties (AC3)', () => { + assert.deepEqual(parseFingerprintFileArgs(['--files', 'a.rb,b.rb']), { files: ['a.rb', 'b.rb'] }); + assert.deepEqual(parseFingerprintFileArgs(['--files', ' a.rb , b.rb ,']), { files: ['a.rb', 'b.rb'] }); + assert.deepEqual(parseFingerprintFileArgs(['--files=a.rb,b.rb']), { files: ['a.rb', 'b.rb'] }); + }); + + test('--files a --files b collects every occurrence (AC3)', () => { + assert.deepEqual(parseFingerprintFileArgs(['--files', 'a.rb', '--files', 'b.rb']), { files: ['a.rb', 'b.rb'] }); + }); + + test('forms mix freely, in order', () => { + assert.deepEqual(parseFingerprintFileArgs(['x', '--files', 'a,b', 'y', '--files=c']), { + files: ['x', 'a', 'b', 'y', 'c'], + }); + }); + + test('an empty --files value (--files=, --files ",", --files "") is a usage error, never a silent no-op', () => { + for (const tokens of [['--files='], ['--files', ','], ['--files', ''], ['--files', ' , ']]) { + const parsed = parseFingerprintFileArgs(tokens); + assert.ok('error' in parsed, JSON.stringify(tokens)); + assert.match(parsed.error, /--files requires at least one path/); + } + }); + + test('--files with no value, or followed by another flag, is a usage error', () => { + for (const tokens of [['--files'], ['a', '--files'], ['--files', '--files', 'a']]) { + const parsed = parseFingerprintFileArgs(tokens); + assert.ok('error' in parsed, JSON.stringify(tokens)); + assert.match(parsed.error, /--files requires a value/); + } + }); + + test('any other --flag is an explicit usage error naming the flag, never a covered path', () => { + const parsed = parseFingerprintFileArgs(['a.rb', '--file', 'b.rb']); + assert.ok('error' in parsed); + assert.match(parsed.error, /unknown flag --file\b/); + assert.doesNotMatch(parsed.error, /missing, unreadable/); + }); +}); + +describe('#4623: computeCoveredDigest v2 — shared planning documents do not enter the digest', () => { + test('defaults to v2 and names the version in the digest', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-version-')); + t.after(() => cleanup(root)); + fs.writeFileSync(path.join(root, 'impl.txt'), 'x'); + assert.match(computeCoveredDigest(root, ['impl.txt']), /^v2:sha256:[0-9a-f]{64}$/); + assert.match(computeCoveredDigest(root, ['impl.txt'], 1), /^v1:sha256:[0-9a-f]{64}$/); + assert.notEqual(computeCoveredDigest(root, ['impl.txt']), computeCoveredDigest(root, ['impl.txt'], 1)); + }); + + test('an unknown version → null (fail closed, never a digest under guessed semantics)', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-unknown-version-')); + t.after(() => cleanup(root)); + fs.writeFileSync(path.join(root, 'impl.txt'), 'x'); + assert.equal(computeCoveredDigest(root, ['impl.txt'], 9), null); + assert.equal(computeCoveredDigest(root, ['impl.txt'], 0), null); + }); + + test('a byte change to .planning/ROADMAP.md or REQUIREMENTS.md leaves the v2 digest unchanged; a phase artifact or implementation change still moves it', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-shared-')); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning', 'phases', '01-a'), { recursive: true }); + fs.mkdirSync(path.join(root, 'src')); + writeSharedDocs4623(root); + fs.writeFileSync(path.join(root, '.planning', 'phases', '01-a', '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(root, 'src', 'thing.cts'), 'export const x = 1;\n'); + const covered = [...SHARED_DOCS_4623, '.planning/phases/01-a/01-01-PLAN.md', 'src/thing.cts']; + + const before = computeCoveredDigest(root, covered); + writeSharedDocs4623(root, { roadmapDone: true, reqDone: true }); + assert.equal(computeCoveredDigest(root, covered), before, 'shared-doc bookkeeping must not move a v2 digest'); + + fs.writeFileSync(path.join(root, '.planning', 'phases', '01-a', '01-01-PLAN.md'), '# Plan (edited)\n'); + const afterPlan = computeCoveredDigest(root, covered); + assert.notEqual(afterPlan, before, 'a phase artifact change must still move it'); + + fs.writeFileSync(path.join(root, 'src', 'thing.cts'), 'export const x = 2;\n'); + assert.notEqual(computeCoveredDigest(root, covered), afterPlan, 'an implementation change must still move it'); + }); + + test('a nested planning file (.planning/research/…) is NOT shared and still moves the digest', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-nested-')); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning', 'research'), { recursive: true }); + const note = path.join(root, '.planning', 'research', 'notes.md'); + fs.writeFileSync(note, 'v1'); + const before = computeCoveredDigest(root, ['.planning/research/notes.md']); + fs.writeFileSync(note, 'v2'); + assert.notEqual(computeCoveredDigest(root, ['.planning/research/notes.md']), before); + }); + + test('a declared shared document is validated like any other path — present: inert; missing, a directory, or an escaping symlink: null (fail closed, as v1)', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-shared-validated-')); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning', 'phases'), { recursive: true }); + fs.writeFileSync(path.join(root, 'impl.txt'), 'x'); + const withoutDecl = computeCoveredDigest(root, ['impl.txt']); + // Missing → null, exactly as v1. + assert.equal(computeCoveredDigest(root, ['impl.txt', '.planning/ROADMAP.md']), null); + fs.writeFileSync(path.join(root, '.planning', 'ROADMAP.md'), '# Roadmap\n'); + // Present → contributes nothing: same digest as if undeclared. + assert.equal(computeCoveredDigest(root, ['impl.txt', '.planning/ROADMAP.md']), withoutDecl); + // A directory directly under the root is not a document → null, as v1. + assert.equal(computeCoveredDigest(root, ['impl.txt', '.planning/phases']), null); + // An in-root symlink whose target escapes → null, as v1 (nothing is read either way, + // but the declaration is still an escape and still invalidates the set). + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-shared-outside-')); + t.after(() => cleanup(outside)); + fs.writeFileSync(path.join(outside, 'secret.txt'), 'not this project'); + fs.symlinkSync(path.join(outside, 'secret.txt'), path.join(root, '.planning', 'ESCAPE.md')); + assert.equal(computeCoveredDigest(root, ['impl.txt', '.planning/ESCAPE.md']), null); + }); + + test('a declaration made only of shared planning documents hashes nothing → null (fail closed, like an empty one)', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-all-shared-')); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning')); + writeSharedDocs4623(root); + assert.equal(computeCoveredDigest(root, SHARED_DOCS_4623), null); + // v1 still hashes them. + assert.match(computeCoveredDigest(root, SHARED_DOCS_4623, 1), /^v1:/); + }); + + test('the phase\'s own planning root is shared too: a workstream-scoped ROADMAP.md is inert, a research note beside it is not', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-workstream-')); + t.after(() => cleanup(root)); + const wsRoot = path.join(root, '.planning', 'workstreams', 'payments'); + const phaseDir = path.join(wsRoot, 'phases', '01-a'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.mkdirSync(path.join(root, '.planning', 'research'), { recursive: true }); + fs.writeFileSync(path.join(wsRoot, 'ROADMAP.md'), '- [ ] Phase 1\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(root, '.planning', 'research', 'notes.md'), 'v1'); + const covered = [ + '.planning/workstreams/payments/ROADMAP.md', + '.planning/workstreams/payments/phases/01-a/01-01-PLAN.md', + '.planning/research/notes.md', + ]; + assert.deepEqual(sharedPlanningRoots(root, phaseDir), ['.planning', '.planning/workstreams/payments']); + const before = computeCoveredDigest(root, covered, 2, { phaseDir }); + fs.writeFileSync(path.join(wsRoot, 'ROADMAP.md'), '- [x] Phase 1\n'); + assert.equal(computeCoveredDigest(root, covered, 2, { phaseDir }), before, 'the workstream roadmap is this phase\'s shared doc'); + // Without the phase dir the same path is NOT recognised (lexically it could be .planning//…). + assert.notEqual(computeCoveredDigest(root, covered, 2), before); + fs.writeFileSync(path.join(root, '.planning', 'research', 'notes.md'), 'v2'); + assert.notEqual(computeCoveredDigest(root, covered, 2, { phaseDir }), before, 'a nested research note is evidence'); + }); + + test('a shared-looking path that escapes the root still fails the whole set', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-escape-')); + t.after(() => cleanup(root)); + fs.writeFileSync(path.join(root, 'impl.txt'), 'x'); + assert.equal(computeCoveredDigest(root, ['impl.txt', '../.planning/ROADMAP.md']), null); + }); + + test('v1 semantics are preserved on request: a shared-doc change still moves a v1 digest', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4623-v1-')); + t.after(() => cleanup(root)); + fs.mkdirSync(path.join(root, '.planning')); + fs.writeFileSync(path.join(root, 'impl.txt'), 'x'); + writeSharedDocs4623(root); + const before = computeCoveredDigest(root, ['impl.txt', ...SHARED_DOCS_4623], 1); + writeSharedDocs4623(root, { roadmapDone: true }); + assert.notEqual(computeCoveredDigest(root, ['impl.txt', ...SHARED_DOCS_4623], 1), before); + }); +}); + +describe('#4623: readVerificationStatus — shared planning documents no longer stale a phase', () => { + const { runGsdTools } = require('./helpers.cjs'); + + function fingerprintViaCli(projectDir, phase, coveredFiles) { + const res = runGsdTools(['verification', 'fingerprint', phase.phaseDir, ...coveredFiles], projectDir); + assert.equal(res.success, true, `expected success, got: ${res.output}${res.error}`); + return JSON.parse(res.output).covered_digest; + } + + test('AC1 cross-phase: completing phase A (roadmap + requirement bookkeeping) leaves phase B passed; B\'s own artifact change still stales B alone', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const a = makePhase4623(projectDir, '01-alpha'); + const b = makePhase4623(projectDir, '02-beta'); + const aCovered = [...a.ownFiles, ...SHARED_DOCS_4623]; + const bCovered = [...b.ownFiles, ...SHARED_DOCS_4623]; + writeReport4623(a, aCovered, fingerprintViaCli(projectDir, a, aCovered)); + writeReport4623(b, bCovered, fingerprintViaCli(projectDir, b, bCovered)); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + assert.equal(readVerificationStatus(b.phaseDir, NO_GIT_TIMES).status, 'passed'); + + // Phase A closes: its roadmap checkbox and its requirement flip. + writeSharedDocs4623(projectDir, { roadmapDone: true, reqDone: true }); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed', 'the closing phase itself'); + assert.equal(readVerificationStatus(b.phaseDir, NO_GIT_TIMES).status, 'passed', 'the untouched sibling phase'); + assert.equal(isPhaseComplete(b.phaseDir).value.complete, true); + + // Real drift in B is still caught, and only in B. + fs.appendFileSync(path.join(b.phaseDir, '02-01-PLAN.md'), '\nchanged\n'); + assert.equal(readVerificationStatus(b.phaseDir, NO_GIT_TIMES).status, 'stale'); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + }); + + test('same-phase: the phase\'s own `requirements mark-complete` and `phase.complete` writes do not stale it', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const a = makePhase4623(projectDir, '01-alpha'); + const covered = [...a.ownFiles, ...SHARED_DOCS_4623]; + writeReport4623(a, covered, fingerprintViaCli(projectDir, a, covered)); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + + // requirements mark-complete: checkbox + traceability cell. + writeSharedDocs4623(projectDir, { reqDone: true }); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + // phase.complete: the phase's own roadmap status cell. + writeSharedDocs4623(projectDir, { reqDone: true, roadmapDone: true }); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + }); + + test('a legacy v1 report keeps v1 semantics: passed while untouched, stale on a shared-doc edit, and the CLI re-fingerprint is the remedy', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const a = makePhase4623(projectDir, '01-alpha'); + const covered = [...a.ownFiles, ...SHARED_DOCS_4623]; + const v1 = computeCoveredDigest(projectDir, covered, 1); + writeReport4623(a, covered, v1); + // The upgrade alone must not stale an intact v1 report. + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + + writeSharedDocs4623(projectDir, { roadmapDone: true }); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'stale', 'v1 hashed the shared docs; honour that'); + + // The documented remedy: recompute through the CLI, paste the result. + const v2 = fingerprintViaCli(projectDir, a, covered); + assert.match(v2, /^v2:/); + writeReport4623(a, covered, v2); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed'); + writeSharedDocs4623(projectDir, { roadmapDone: true, reqDone: true }); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'passed', 'and it lasts'); + }); + + test('workstream scope through the READ path: the workstream\'s own ROADMAP.md flip leaves its phase passed; its PLAN edit stales it', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + const wsRoot = path.join(projectDir, '.planning', 'workstreams', 'payments'); + const phaseDir = path.join(wsRoot, 'phases', '01-alpha'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(wsRoot, 'ROADMAP.md'), '- [ ] **Phase 1: Alpha**\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + const phase = { phaseDir, num: '01' }; + const covered = [ + '.planning/workstreams/payments/ROADMAP.md', + '.planning/workstreams/payments/phases/01-alpha/01-01-PLAN.md', + '.planning/workstreams/payments/phases/01-alpha/01-01-SUMMARY.md', + ]; + writeReport4623(phase, covered, fingerprintViaCli(projectDir, phase, covered)); + assert.equal(readVerificationStatus(phaseDir, NO_GIT_TIMES).status, 'passed'); + fs.writeFileSync(path.join(wsRoot, 'ROADMAP.md'), '- [x] **Phase 1: Alpha**\n'); + assert.equal(readVerificationStatus(phaseDir, NO_GIT_TIMES).status, 'passed', 'workstream roadmap bookkeeping'); + fs.appendFileSync(path.join(phaseDir, '01-01-PLAN.md'), 'changed\n'); + assert.equal(readVerificationStatus(phaseDir, NO_GIT_TIMES).status, 'stale'); + }); + + test('a phase directory that is not /phases/ nominates no extra root: implementation evidence beside it stays hashed', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + const fakePhase = path.join(projectDir, 'src', 'phases', '01-fake'); + fs.mkdirSync(fakePhase, { recursive: true }); + fs.writeFileSync(path.join(projectDir, 'src', 'evidence.cts'), 'export const x = 1;\n'); + fs.writeFileSync(path.join(fakePhase, '01-01-PLAN.md'), '# Plan\n'); + const covered = ['src/evidence.cts', 'src/phases/01-fake/01-01-PLAN.md']; + const digest = computeCoveredDigest(projectDir, covered, 2, { phaseDir: fakePhase }); + fs.writeFileSync( + path.join(fakePhase, '01-VERIFICATION.md'), + `---\nstatus: passed\ncovered_files:\n${covered.map((f) => ` - ${f}`).join('\n')}\ncovered_digest: "${digest}"\n---\n`, + ); + assert.equal(readVerificationStatus(fakePhase, NO_GIT_TIMES).status, 'passed'); + fs.writeFileSync(path.join(projectDir, 'src', 'evidence.cts'), 'export const x = 2;\n'); + assert.equal(readVerificationStatus(fakePhase, NO_GIT_TIMES).status, 'stale', 'src/ must never be treated as a planning root'); + }); + + test('a digest under an unknown fingerprint version is stale (fail closed)', (t) => { + const projectDir = createTempGitProject(); + t.after(() => cleanup(projectDir)); + const a = makePhase4623(projectDir, '01-alpha'); + writeReport4623(a, a.ownFiles, 'v9:sha256:' + 'a'.repeat(64)); + assert.equal(readVerificationStatus(a.phaseDir, NO_GIT_TIMES).status, 'stale'); + }); +}); + +describe('#4623: verification.fingerprint CLI — --files forms and the phase-dir guard', () => { + const { runGsdTools } = require('./helpers.cjs'); + + function setup() { + const projectDir = createTempGitProject(); + const a = makePhase4623(projectDir, '01-alpha'); + fs.mkdirSync(path.join(projectDir, 'fastlane')); + fs.writeFileSync(path.join(projectDir, 'fastlane', 'Fastfile'), 'lane :x do end\n'); + fs.writeFileSync(path.join(projectDir, 'a.rb'), 'a\n'); + fs.writeFileSync(path.join(projectDir, 'b.rb'), 'b\n'); + return { projectDir, a }; + } + + function run(projectDir, phaseDir, ...tokens) { + return runGsdTools(['verification', 'fingerprint', phaseDir, ...tokens], projectDir); + } + + function expectJson(res) { + assert.equal(res.success, true, `expected success, got: ${res.output}${res.error}`); + return JSON.parse(res.output); + } + + test('AC2: --files a produces the same covered_files/covered_digest as the bare positional form', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + const positional = expectJson(run(projectDir, a.phaseDir, 'fastlane/Fastfile')); + const flagged = expectJson(run(projectDir, a.phaseDir, '--files', 'fastlane/Fastfile')); + assert.deepEqual(flagged, positional); + assert.deepEqual(positional.covered_files, ['fastlane/Fastfile']); + }); + + test('AC3: --files "a,b" and --files a --files b both resolve to covered_files [a, b], canonicalized like the bare form (AC4)', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + const positional = expectJson(run(projectDir, a.phaseDir, 'b.rb', 'a.rb')); + assert.deepEqual(positional.covered_files, ['a.rb', 'b.rb']); + assert.deepEqual(expectJson(run(projectDir, a.phaseDir, '--files', 'a.rb,b.rb')), positional); + assert.deepEqual(expectJson(run(projectDir, a.phaseDir, '--files', 'a.rb', '--files', 'b.rb')), positional); + assert.deepEqual(expectJson(run(projectDir, a.phaseDir, '--files=b.rb,a.rb')), positional); + assert.deepEqual(expectJson(run(projectDir, a.phaseDir, 'a.rb', '--files', 'b.rb')), positional); + assert.equal(positional.covered_digest, computeCoveredDigest(projectDir, ['a.rb', 'b.rb'])); + }); + + test('--raw with --files prints just the digest', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + const res = run(projectDir, a.phaseDir, '--files', 'a.rb,b.rb', '--raw'); + assert.equal(res.success, true, `expected success, got: ${res.output}${res.error}`); + assert.equal(res.output.trim(), computeCoveredDigest(projectDir, ['a.rb', 'b.rb'])); + }); + + test('AC5: a phase directory with zero covered files still fails closed with the existing error', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + const res = run(projectDir, a.phaseDir); + assert.equal(res.success, false); + assert.match(`${res.output}${res.error}`, /at least one covered file required/); + }); + + test('an unrecognized flag is a usage error naming the flag — not "a covered file is missing"', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + const res = run(projectDir, a.phaseDir, '--file', 'a.rb'); + assert.equal(res.success, false); + assert.match(`${res.output}${res.error}`, /unknown flag --file\b/); + assert.doesNotMatch(`${res.output}${res.error}`, /missing, unreadable/); + }); + + test('an omitted phase dir (first argument is a covered file) is an error, not a plausible digest over the wrong set at exit 0', (t) => { + const { projectDir } = setup(); + t.after(() => cleanup(projectDir)); + const res = runGsdTools(['verification', 'fingerprint', 'a.rb', 'b.rb', '--raw'], projectDir); + assert.equal(res.success, false); + assert.match(`${res.output}${res.error}`, /phase directory not found/); + assert.doesNotMatch(res.output, /^v\d+:sha256:/); + }); + + test('a declaration made only of shared planning documents is a named error, not "file missing"', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const res = run(projectDir, a.phaseDir, '--files', SHARED_DOCS_4623.join(',')); + assert.equal(res.success, false); + assert.match(`${res.output}${res.error}`, /every covered file is a repo-wide planning document/); + assert.doesNotMatch(`${res.output}${res.error}`, /missing, unreadable/); + }); + + test('an all-shared declaration with a missing or directory member is a bad path first (generic error), not the named all-shared error', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const res = run(projectDir, a.phaseDir, '.planning/ROADMAP.md', '.planning/MISSING.md'); + assert.equal(res.success, false); + assert.match(`${res.output}${res.error}`, /missing, unreadable/); + assert.doesNotMatch(`${res.output}${res.error}`, /every covered file is a repo-wide planning document/); + const dir = run(projectDir, a.phaseDir, '.planning/ROADMAP.md', '.planning/phases'); + assert.equal(dir.success, false); + assert.match(`${dir.output}${dir.error}`, /missing, unreadable/); + }); + + test('a declared shared document stays listed in covered_files, and the emitted v2 digest survives its rewrite', (t) => { + const { projectDir, a } = setup(); + t.after(() => cleanup(projectDir)); + writeSharedDocs4623(projectDir); + const covered = [...a.ownFiles, ...SHARED_DOCS_4623]; + const first = expectJson(run(projectDir, a.phaseDir, '--files', covered.join(','))); + assert.deepEqual(first.covered_files, [...covered].sort()); + assert.match(first.covered_digest, /^v2:/); + assert.equal(first.covered_digest, computeCoveredDigest(projectDir, covered)); + + writeSharedDocs4623(projectDir, { roadmapDone: true, reqDone: true }); + const second = expectJson(run(projectDir, a.phaseDir, ...covered)); + assert.equal(second.covered_digest, first.covered_digest); + }); +}); + // ─── #2617: next_command runtime projection ────────────────────────────────── // // Regression tests for #2617 — verification-status `next_command` bypassed the