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

enhance(#2572): run the verify-summary artifact check against phase SUMMARYs (#2685)

Browse Source 
* enhance(#2572): run the verify-summary artifact check against phase SUMMARYs (W025)

The artifact<->git check has existed since the beginning but was only ever
pointed at .planning/research/SUMMARY.md (new-project.md:1145,
new-milestone.md:425). Phase summaries -- the ones that actually claim
"I created these files" -- were never checked.

- extract verifySummaryCore from cmdVerifySummary: same checks, lifted out of
  the output() wrapper so callers consume {passed, checks, errors} directly
  instead of shelling out and re-parsing JSON; cmdVerifySummary is now a thin
  adapter over it
- validate.health gains advisory W025 per phase SUMMARY with missing files

Advisory only: appends to warnings[], never touches status escalation beyond
the channel's own warning semantics, the repair set, or readVerificationStatus.

Resolves both open questions from triage: (a) commits_exist is deliberately NOT
surfaced -- its hash pattern matches any hex-shaped token in prose, too loose to
show a user; (b) a phase carries N per-plan summaries plus a legacy bare
SUMMARY.md, so all of them are checked via the repo-wide filter.

* chore(#2572): add changeset fragment

* feat(#2572): move the SUMMARY artifact check to phase completion

Responds to the #2685 review. Three substantive changes.

Seam (Blocker 2). The check now runs in cmdPhaseComplete, the seam the
issue body cited (src/phase.cts:~1745), not validate.health. That channel
does exist: cmdPhaseComplete declares warnings[], populates it from the
UAT/VERIFICATION pre-scan, and emits it. The cycle objection raised
against the earlier deviation holds for state.cts only -- verify.cts has
no transitive import path to phase.cts, so phase.cts -> verify.cjs adds
no cycle (verified over every src/*.cts). Moving it also retires the
retroactive firing across all historical phases: this fires once, at
completion, for the phase being completed.

Extraction (Blocker 1). Pattern 2 now excludes [ and ] from its path
class. The SUMMARY templates prescribe a YAML flow sequence
(key-files.created: [a.ts, b.ts]) and the label matches case-insensitively,
so the class previously captured the literal [ and produced a candidate
that can never exist on disk -- firing on healthy projects built from the
templates GSD itself ships. Stripping frontmatter was the other offered
remedy; measured across all three shipped templates it is a no-op on top
of the exclusion, so it is not carried. Consequence named in-code: the
key-files block still is not read, which needs a real frontmatter parse.

Also narrowed to the noise classes confirmed in review -- globs, bare
hostnames, and paths resolving outside the project are skipped rather
than reported, and the containment guard the old comment claimed now
actually exists.

Budget (Majors 1 and 3). verifySummaryCore takes a checkCommits option;
phase completion passes false, so the discarded git cat-file probes are
not spawned at all. It also passes Infinity, so every referenced file is
reported instead of the first two -- a summary listing twelve files of
which nine are missing now says nine, not zero. The verb keeps its
historical 2-file default.

Tests (Major 2). The vacuous fixtures are gone with the health block.
The replacements use /-bearing paths that genuinely extract, and each
fix was mutation-checked: un-anchoring pattern 2, dropping the glob,
hostname or containment filter, forcing commit checking on, and
re-capping at 2 each fail at least one test.

* docs(#2572): describe the phase-completion SUMMARY artifact check

The W025 text under /gsd-health is withdrawn with the health seam; the
check is documented where it now runs, under `phase complete` in
docs/CLI-TOOLS.md.

Both the docs and the changeset previously overclaimed: they said a
referenced file not on disk is warned about, while at most two candidates
per SUMMARY were ever examined. The cap is gone at this seam, so the
claim now holds -- and the text states the limits that remain, rather
than leaving them to be discovered: the key-files frontmatter block is
not read, commit hashes are not resolved, and globs, URLs, bare
hostnames and out-of-project paths are skipped rather than reported.

---------

Co-authored-by: CI Rebase Check <ci@gsd-redux>
This commit is contained in:
Rezolv
2026-07-28 18:31:33 -04:00
committed by GitHub
parent 6932fb16d7
commit 7e8f6a6d7d
6 changed files with 567 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
src/phase.cts
View File

@@ -62,6 +62,11 @@ import uatPredicate = require('./uat-predicate.cjs');
const { evaluateUatPassed } = uatPredicate;
// eslint-disable-next-line @typescript-eslint/no-require-imports -- verification.cjs is an export= CommonJS module
import verificationMod = require('./verification.cjs');
// #2572: the artifact↔disk core behind the `verify-summary` verb. `verify.cts`
// has no transitive import path back to `phase.cts`, so this edge introduces no
// cycle (the reverse edge, `state.cts → verify.cjs`, would).
// eslint-disable-next-line @typescript-eslint/no-require-imports -- verify.cjs is an export= CommonJS module
import verifyMod = require('./verify.cjs');
const { readVerificationStatus } = verificationMod;
const { planningDir, withPlanningLock, listAvailableWorkstreams, getActiveWorkstream } =
@@ -1757,6 +1762,50 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
* warnings are surfaced this run, not a blocked or corrupted completion. */
}
// #2572: artifact↔disk advisory for the SUMMARYs of the phase being completed.
//
// A SUMMARY asserts "I created these files". Nothing checked that claim for
// phase summaries — the `verify-summary` verb has existed since the beginning
// but was only ever pointed at `.planning/research/SUMMARY.md`. An interrupted
// or over-reported phase therefore counted toward 100% silently.
//
// Joins the same ADVISORY channel as the pre-scan above: findings land in
// `warnings[]` (rendered by execute-phase.md's "If has_warnings is true"
// step), never in the completion GATE (readVerificationStatus below).
// Completion is never blocked.
//
// `checkCommits: false` — only the file-existence half is surfaced here, so
// the `git cat-file` probes would be spawned and their result discarded. The
// hash pattern is a loose `\b[0-9a-f]{7,40}\b` that matches any hex-shaped
// token in prose, too noisy to put in front of a user even as a warning.
//
// `Infinity` — report every referenced file, not the CLI verb's default first
// two, so a phase that lists twelve files and landed three says so. The verb
// keeps its 2-file default; only this caller opts out of the cap.
try {
const phaseDirRel = phaseInfo['directory'] as string;
// `summaries` arrives pre-sorted from the phase locator, so warning order is
// deterministic across platforms rather than readdir-dependent.
const summaryNames = (phaseInfo['summaries'] as string[] | undefined) || [];
for (const summaryName of summaryNames) {
const v = verifyMod.verifySummaryCore(
cwd,
`${phaseDirRel}/${summaryName}`,
Infinity,
{ checkCommits: false },
);
const missing = v.checks.files_created.missing;
if (missing.length > 0) {
warnings.push(
`${summaryName}: references ${missing.length} file(s) not on disk: ${missing.join(', ')}`,
);
}
}
} catch {
/* best-effort, same posture as the #2245 pre-scan above: an unreadable
* SUMMARY means one fewer advisory this run, never a blocked completion. */
}
let nextPhaseNum: string | null = null;
let nextPhaseName: string | null = null;
let isLastPhase = true;

129
src/verify.cts
View File

@@ -63,21 +63,52 @@ const { MODEL_PROFILES } = modelProfilesMod;
void stripShippedMilestones;
void detectSchemaFiles;
function cmdVerifySummary(
interface SummaryVerification {
passed: boolean;
checks: {
summary_exists: boolean;
files_created: { checked: number; found: number; missing: string[] };
commits_exist: boolean;
self_check: string;
};
errors: string[];
}
/**
* Pure core of `verify-summary` (#2572).
*
* Same artifact↔git checks the CLI verb has always run, lifted out of the
* `output()` wrapper so other verbs can consume the structured
* `{ passed, checks, errors }` contract directly instead of shelling out and
* re-parsing JSON. `cmdVerifySummary` is now a thin adapter over this.
*
* Never throws and never writes to stdout: a missing SUMMARY, a non-repo, or an
* unresolvable commit all come back as structured `false`/`missing` values.
*
* Caveat for callers surfacing `commits_exist`: the hash pattern is a loose
* `\b[0-9a-f]{7,40}\b`, so any hex-shaped token in the prose counts as a
* candidate. That is cheap as an advisory signal and unacceptable as a gate.
*
* @param checkFileCount How many extracted candidates to probe. Defaults to 2 —
* the value the CLI verb has always used. Pass `Infinity` to probe every
* candidate (see `cmdPhaseComplete`, which reports on all of them).
* @param opts.checkCommits When `false`, the `git cat-file` probes are skipped
* entirely and `commits_exist` comes back `false` meaning *not checked*.
* Callers that do not surface `commits_exist` should pass `false` so this
* stays a pure-filesystem check with no subprocess cost.
*/
function verifySummaryCore(
cwd: string,
summaryPath: string,
checkFileCount: number | undefined,
raw: boolean,
): void {
if (!summaryPath) {
error('summary-path required');
}
checkFileCount?: number,
opts?: { checkCommits?: boolean },
): SummaryVerification {
const fullPath = path.join(cwd, summaryPath);
const checkCount = checkFileCount || 2;
const checkCommits = opts?.checkCommits !== false;
if (!fs.existsSync(fullPath)) {
const result = {
return {
passed: false,
checks: {
summary_exists: false,
@@ -87,24 +118,74 @@ function cmdVerifySummary(
},
errors: ['SUMMARY.md not found'],
};
output(result, raw, 'failed');
return;
}
const content = fs.readFileSync(fullPath, 'utf-8');
const errors: string[] = [];
const projectRoot = path.resolve(cwd);
/**
* Is `candidate` plausibly a repo-relative file this check should probe?
*
* Deliberately narrowing. This is an ADVISORY, so the two error directions are
* not symmetric: a false positive tells a user their healthy project is
* missing a file that was never claimed, while a false negative just means one
* reference goes unprobed. Every rejection below is a noise class confirmed on
* #2685; when in doubt, skip rather than warn.
*/
const isProbableProjectFile = (candidate: string): boolean => {
// Only repo-relative paths — a bare filename is too ambiguous to locate.
if (!candidate.includes('/')) return false;
// URLs, protocol-relative links, and any other scheme.
if (candidate.startsWith('http') || candidate.startsWith('//')) return false;
if (/^[a-z][a-z0-9+.-]*:\/\//i.test(candidate)) return false;
// Globs name a set, not a file: `src/**/*.cts` is never "missing".
if (/[*?]/.test(candidate)) return false;
// Bare hostnames (`docs.example.com/guide.html`). A repo-relative path's
// first segment is a directory name, which in practice contains a dot only
// when it is a dotfile directory (`.github/`, `.changeset/`, `.planning/`)
// — i.e. the dot is at index 0. A dot anywhere later marks a hostname.
const firstSegment = candidate.split('/')[0] || '';
if (firstSegment.indexOf('.') > 0) return false;
// Containment guard: a `../`-bearing reference must not turn this advisory
// into a filesystem existence probe outside the project.
const resolved = path.resolve(projectRoot, candidate);
if (resolved !== projectRoot && !resolved.startsWith(projectRoot + path.sep)) return false;
return true;
};
// Pattern 2 excludes `[` and `]` from its path class (#2685 Blocker 1). All
// three SUMMARY templates prescribe a YAML flow sequence for `key-files`:
//
// key-files:
// created: [src/auth/login.ts, src/auth/session.ts]
//
// and the label matches `(?:Created|Modified|…):` case-insensitively. Without
// the bracket exclusion the class captures the literal `[` as part of the
// first path, yielding `[src/auth/login.ts` — a candidate that can never exist
// on disk. That fired on healthy projects built from GSD's own shipped
// template. The exclusion also stops a markdown list in the body from
// reintroducing the same artifact.
//
// Stripping frontmatter first was the other remedy offered on #2685. It is a
// verified no-op on top of this exclusion — measured identical extraction
// across all three shipped templates — because the exclusion already makes a
// flow-sequence line contribute nothing. Consequence worth naming: the
// `key-files` block, the most authoritative statement of what a phase created,
// is still not read. Recovering it needs a real frontmatter parse, which is
// deliberately left as a follow-up rather than smuggled in here.
const mentionedFiles = new Set<string>();
const patterns = [
/`([^`]+\.[a-zA-Z]+)`/g,
/(?:Created|Modified|Added|Updated|Edited):\s*`?([^\s`]+\.[a-zA-Z]+)`?/gi,
/(?:Created|Modified|Added|Updated|Edited):\s*`?([^\s`[\]]+\.[a-zA-Z]+)`?/gi,
];
for (const pattern of patterns) {
let m: RegExpExecArray | null;
while ((m = pattern.exec(content)) !== null) {
const filePath = m[1];
if (filePath && !filePath.startsWith('http') && filePath.includes('/')) {
if (filePath && isProbableProjectFile(filePath)) {
mentionedFiles.add(filePath);
}
}
@@ -113,13 +194,13 @@ function cmdVerifySummary(
const filesToCheck = Array.from(mentionedFiles).slice(0, checkCount);
const missing: string[] = [];
for (const file of filesToCheck) {
if (!fs.existsSync(path.join(cwd, file))) {
if (!fs.existsSync(path.resolve(projectRoot, file))) {
missing.push(file);
}
}
const commitHashPattern = /\b[0-9a-f]{7,40}\b/g;
const hashes = content.match(commitHashPattern) || [];
const hashes = checkCommits ? content.match(commitHashPattern) || [] : [];
let commitsExist = false;
if (hashes.length > 0) {
for (const hash of hashes.slice(0, 3)) {
@@ -157,8 +238,21 @@ function cmdVerifySummary(
};
const passed = missing.length === 0 && selfCheck !== 'failed';
const result = { passed, checks, errors };
output(result, raw, passed ? 'passed' : 'failed');
return { passed, checks, errors };
}
/** CLI adapter over verifySummaryCore — arg guard + output shaping only. */
function cmdVerifySummary(
cwd: string,
summaryPath: string,
checkFileCount: number | undefined,
raw: boolean,
): void {
if (!summaryPath) {
error('summary-path required');
}
const result = verifySummaryCore(cwd, summaryPath, checkFileCount);
output(result, raw, result.passed ? 'passed' : 'failed');
}
/**
@@ -2526,6 +2620,7 @@ export = {
scanNegativeGrepCommentEcho,
scanFileWideNegativeGateConflict,
cmdVerifySummary,
verifySummaryCore,
cmdVerifyPlanStructure,
cmdVerifyPhaseCompleteness,
cmdVerifyReferences,