golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#3511): scope every phase-directory scan to the phase it belongs to (#3535)

Browse Source
This commit is contained in:
Tom Boucher
2026-08-15 07:02:33 -04:00
committed by GitHub
parent 3d17569d5b
commit 59e7a677fe
30 changed files with 1903 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
src/verification.cts
View File

@@ -43,7 +43,7 @@ import { execGit } from './shell-command-projection.cjs';
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
const { output, error } = io;
const { extractPhaseToken } = phaseId;
const { extractPhaseToken, scopeToPhase } = phaseId;
const { extractFrontmatter } = frontmatterMod;
const { SCOPE } = planningScopeMod;
type Scope = planningScopeMod.Scope;
@@ -309,10 +309,20 @@ interface ResolveVerificationFileOptions {
* (`12-review-VERIFICATION.md`) — a regression this option closes.
*
* Omitted / empty when the token cannot be derived: falls back to plain
* alphabetically-first among all dashed candidates (the original pre-#3357
* behavior), never to null.
* alphabetically-first among the SCOPED dashed candidates (see
* `phaseDirName` below), never to null.
*/
phaseToken?: string;
/**
* #3511 reconciliation: the phase directory's own basename (the same value
* every call site already passes through `extractPhaseToken` to derive
* `phaseToken` above) — needed separately because the fallback below scopes
* by `isPhaseArtifact(fileName, phaseDirName)`, not by `phaseToken`.
*
* Omitted: the fallback degrades to the plain (unscoped) alphabetically-first
* pick — the original pre-#3357 behavior — never to null.
*/
phaseDirName?: string;
}
/**
@@ -332,22 +342,59 @@ type ResolveUatFileOptions = ResolveVerificationFileOptions;
* `bareName` is the artifact filename WITHOUT the leading dash (`'UAT.md'`);
* a "dashed" candidate is any entry ending `-${bareName}`.
*
* Selection order (identical to resolveVerificationFile's documented tiers,
* generalized off the suffix):
* Selection order:
* 1. `options.phaseToken` given and `<phaseToken>-${bareName}` is among
* the candidates — that exact file always wins: it is THIS phase's own
* artifact, and no other candidate (whichever phase's token it carries)
* can outrank it (#3492 / #3518).
* 2. Fallback — no exact phase-token match (or no token given):
* alphabetically first of ALL dashed candidates (deterministic —
* `entries` is an unsorted readdir listing whose order is
* filesystem-dependent, so the sort is what makes the answer
* machine-independent). Load-bearing: a phase whose only artifact is
* non-canonically named must keep resolving to it, not to null — the
* fix must not turn "found an artifact" into "found nothing".
* 3. `options.allowBare` only — a bare `${bareName}`, ranked BELOW both of
* the above (a dashed file names its phase; a bare one does not).
* Reached only when (1) and (2) found no dashed candidate at all.
* 2. Fallback — no exact phase-token match (or no token given): alphabetically
* first of the dashed candidates that are THIS phase's own, per
* `scopeToPhase(candidates, options.phaseDirName)` (#3511 reconciliation,
* below). Load-bearing: a phase whose only artifact is non-canonically
* named must keep resolving to it, not to null — this fix must not turn
* "found an artifact" into "found nothing" for anyone. A
* non-canonically-named artifact of THIS phase (e.g.
* `03-CORRECTION-VERIFICATION.md` in `03-foo`) still passes
* `isPhaseArtifact` (it names phase 03, same as the directory), so it
* is still returned here.
* 3. `options.allowBare` only — a bare `${bareName}`, ranked BELOW both
* of the above. Rationale: a dashed file names its phase, a bare one
* does not, so a dashed file (canonical or not) is always the better
* answer when both exist. Reached when neither (1) nor (2) found any
* candidate — including when (2)'s scoping filtered every dashed
* candidate out as belonging to some OTHER phase.
*
* #3511 RECONCILIATION with `isPhaseArtifact` (`src/phase-id.cts`): that
* predicate's own docblock used to flag this fallback as an open gap — its
* aggregate scans exclude a cross-phase stray, but this single-pick resolver
* did not, so it could return a stray as THE artifact while the aggregate
* scans correctly ignored it. Closed by scoping step (2) above through
* `scopeToPhase` (`src/phase-id.cts`, itself built on `isPhaseArtifact`):
* `options.phaseDirName` threads the phase directory's basename in, and the
* fallback now filters candidates through `scopeToPhase(candidates,
* phaseDirName)` before picking alphabetically-first. This does NOT reopen
* the #3357 guarantee — that guarantee is "a phase whose only report is
* non-canonically named must keep working", and a non-canonically-named
* artifact of THIS phase still passes `isPhaseArtifact` (it is membership by
* phase number, not by canonical shape), so it is still returned. Only a
* file belonging to a DIFFERENT phase is now excluded — and excluding it is
* correct: returning another phase's artifact as this phase's own is worse
* than reporting none (confidently wrong beats honestly empty).
* The fail-safe now lives entirely inside `isPhaseArtifact`, not in
* `scopeToPhase` (which is a plain filter with no unfiltered fallback):
* (a) when phase-number membership cannot be determined for `phaseDirName` at
* all (no reliable token — the zero-token directory case), every candidate is
* treated as belonging to the phase; (b) the `firstLetterPrefixed`
* bracket-ambiguity case, where a letter-prefixed-decimal dir is
* string-indistinguishable from a bracket-dir token, also includes
* everything rather than guess; (c) a token-less filename (bare
* `${bareName}`) is accepted by directory containment alone. Outside
* those cases, when scoping DOES remove every dashed candidate — a real
* cross-phase stray, or a phase whose own artifact is genuinely absent — the
* fallback below correctly falls through to `allowBare`/`null`: reporting no
* artifact, not another phase's. `options.phaseDirName` omitted entirely skips
* the filter outright (the ternary below), which is unscoped, pre-#3511
* behavior.
*
* Pure — takes an already-read directory listing and does no I/O of its own,
* so every call site keeps its existing `fsImpl` seam and no-throw contract
@@ -364,7 +411,16 @@ function resolvePhaseArtifactFile(
const thisPhaseFile = `${options.phaseToken}-${bareName}`;
if (candidates.includes(thisPhaseFile)) return thisPhaseFile;
}
return candidates[0];
// #3511: scope the fallback to files that belong to THIS phase, so a
// stray cross-phase file can no longer outrank a return of null.
// `phaseDirName` omitted, or membership undeterminable for it, →
// unscoped `candidates` (pre-#3511 behavior); otherwise strays are
// filtered out, and if that leaves nothing the code falls through to
// `allowBare`/`null` deliberately.
const scoped = options.phaseDirName
? scopeToPhase(candidates, options.phaseDirName)
: candidates;
if (scoped.length > 0) return scoped[0];
}
if (options.allowBare && entries.includes(bareName)) return bareName;
return null;
@@ -384,9 +440,11 @@ function resolvePhaseArtifactFile(
* single resolver both now call (#3473 F2).
*
* Selection order: see `resolvePhaseArtifactFile` (the shared core this
* delegates to since #3518) — phase-token-pinned, then alphabetically-first
* dashed fallback, then (allowBare only) a bare `VERIFICATION.md`. Behavior
* is byte-identical to the pre-#3518 standalone implementation.
* delegates to since #3518, itself phase-scoped since #3511) —
* phase-token-pinned, then phase-scoped alphabetically-first dashed
* fallback, then (allowBare only) a bare `VERIFICATION.md`. #3518 extracted
* this into the shared core without changing behavior; #3511's
* `phaseDirName` scoping now lives inside that shared core rather than here.
*/
function resolveVerificationFile(
entries: string[],
@@ -411,7 +469,11 @@ function resolveVerificationFile(
* is consumed downstream by workflows that then read the named file, so a
* wrong path routes UAT state from another phase.
*
* Deterministic by construction: same answer on every machine.
* Deterministic by construction: same answer on every machine. Phase-scoped
* (#3511): passing `options.phaseDirName` filters the alphabetically-first
* fallback (tier 2) to artifacts that belong to THIS phase — see
* `resolvePhaseArtifactFile` for the full selection order and scoping
* rationale.
*/
function resolveUatFile(
entries: string[],
@@ -477,9 +539,11 @@ function findStaleVerificationSummary(
const phaseFiles = fsImpl.readdirSync(phaseDir);
// #3492: pin selection to THIS phase's own token so a stray cross-phase
// or sentinel-numbered canonically-shaped file cannot outrank this
// phase's own (possibly non-canonical) report.
const phaseToken = extractPhaseToken(path.basename(phaseDir));
const verificationFile = resolveVerificationFile(phaseFiles, { phaseToken });
// phase's own (possibly non-canonical) report. #3511: phaseDirName scopes
// the fallback path to this same phase (see resolveVerificationFile docs).
const phaseDirName = path.basename(phaseDir);
const phaseToken = extractPhaseToken(phaseDirName);
const verificationFile = resolveVerificationFile(phaseFiles, { phaseToken, phaseDirName });
if (!verificationFile) return { determined: true, stale: false };
const summaryFiles = (scanPhasePlans(phaseDir) as { summaryFiles: string[] }).summaryFiles
@@ -522,8 +586,8 @@ function findStaleVerificationSummary(
* Behavior:
* 1. Find the phase's verification report via `resolveVerificationFile`
* (canonical `<phase-token>-VERIFICATION.md` preferred; falls back to the
* alphabetically-first `*-VERIFICATION.md` when none is canonical — #3357).
* If none → status 'missing'.
* alphabetically-first `*-VERIFICATION.md` that belongs to THIS phase when
* none is canonical — #3357/#3511). If none → status 'missing'.
* 2. Extract `status` from FRONTMATTER ONLY via the shared extractFrontmatter
* parser (DEFECT.FRONTMATTER-SCALAR-BROAD-GREP fix — parser anchors at byte 0).
* If no frontmatter block or no `status` key → status 'missing'.
@@ -569,8 +633,9 @@ function readVerificationStatus(
// #3492: pin selection to THIS phase's own token (already derived above
// for the routed command argument) so a stray cross-phase or
// sentinel-numbered canonically-shaped file cannot outrank this phase's
// own (possibly non-canonical) report.
verificationFile = resolveVerificationFile(entries, { phaseToken });
// own (possibly non-canonical) report. #3511: baseName also scopes the
// fallback path to this same phase (see resolveVerificationFile docs).
verificationFile = resolveVerificationFile(entries, { phaseToken, phaseDirName: baseName });
} catch {
// Directory unreadable → treat as missing
verificationFile = null;
@@ -775,8 +840,9 @@ function cmdVerificationResolveFile(cwd: string, phaseDirArg: string | undefined
let verificationPath = '';
try {
const entries = fs.readdirSync(phaseDir);
const phaseToken = extractPhaseToken(path.basename(phaseDir));
const verificationFile = resolveVerificationFile(entries, { allowBare: true, phaseToken });
const phaseDirName = path.basename(phaseDir);
const phaseToken = extractPhaseToken(phaseDirName);
const verificationFile = resolveVerificationFile(entries, { allowBare: true, phaseToken, phaseDirName });
if (verificationFile) {
verificationPath = path.join(phaseDir, verificationFile);
}