fix(#4623): keep repo-wide planning docs out of the verification digest, and accept --files on verification.fingerprint (#4749)
* 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 `<project>/` and `workstreams/<ws>/` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DCMY8P8s6dp4g3Rxu3nNAi * chore(#4623): set changeset fragment pr to 4749 --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
@@ -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<N>: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<number> = 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/<project>`, `.planning/workstreams/<ws>`,
|
||||
* `.planning/<project>/workstreams/<ws>`; `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/<project>/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 — `<root>/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
|
||||
* `<root>/phases/<phase>/` 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 <phaseDir>`
|
||||
* 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 <a[,b]>, 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,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user