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

fix(#4024): flag quantitative-criteria trap shapes in verify plan-structure (#4288)

Browse Source 
* test(#4024): pin quantitative-criteria trap shapes for verify plan-structure

Rows 1-3 and 20 of the #4024 test matrix reproduce the issue's shapes
(exact grep -c counts, bulk all-N observed-failing claims) and are
expected to FAIL against unmodified next: nothing judges these shapes
today. Corrected-arm rows pin that each rule is silent on its own fix.

* fix(#4024): flag quantitative-criteria trap shapes in verify plan-structure

Add scanQuantitativeCriteria, the third plan-discipline scanner in the
cmdVerifyPlanStructure family (#429, #968). It judges criteria text in
<acceptance_criteria>/<automated>/<verify> blocks against a six-rule ban
list of shapes proven to be traps at HEAD: exact grep -c counts (R1),
bulk all-N observed-failing claims (R2), unquoted $VAR in command
position (R3), fallible git swallowed by a non-final pipeline stage (R4,
warn), wc output compared by string equality (R5), and relative HEAD~N
git anchors (R6; bare git diff warns). Legitimate exit:
<!-- plan-criteria-allow: R# - reason -->. Pure text scan, fail open.

* test(#4024): bind node:test before hook locally below the fold-point

* fix(#4024): R3 command-position anchor tolerates list bullets and inline-code backticks

* test(#4024): bind VERIFY_CJS locally in the unit block instead of relying on fold scope

* fix(#4024): R6 argument span ends at inline-code backtick or redirection

* fix(#4024): satisfy no-adhoc-markdown lint on the R4 stage-boundary regex

* chore(#4024): add changeset fragment

* chore(#4024): backfill PR number in changeset fragment

* fix(#4024): escape backticks in regex literals so drift-lint tokenizers keep function attribution

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-04 21:04:49 -04:00
committed by GitHub
parent 1ec4b38bd3
commit 04ac8723b9
3 changed files with 598 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

234
src/verify.cts
View File

@@ -745,6 +745,232 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[]
return { warnings, valid: true as const };
}
/**
* Issue #4024 — quantitative-criteria trap-shape scanner, the third plan-discipline
* gate in this family (after #429's comment-echo gate and #968's file-wide
* negative-gate conflict detector). #429/#968 judge `grep -c … == 0` shapes; this
* scanner judges the OTHER quantitative criterion shapes that are provably traps
* at HEAD, so a plan whose criteria are unsatisfiable-or-vacuous before any
* executor touches them no longer passes `verify plan-structure` clean.
*
* Scope: criteria text only — `<acceptance_criteria>`, `<automated>`, and
* `<verify>` blocks. Prose elsewhere in the plan (e.g. an explanatory
* `<action>`) is not judged (fail-open; see the negative-space section of the
* #4024 diagnosis).
*
* Ban list, each rule with a corrected arm asserted in tests (a rule that fires
* on its own fix is a refusal, not a rule):
* R1 (error) exact count out of `grep -c` with N ≥ 2 — `grep -c` counts LINES,
* not matches. Hedged counts (`>=`, "at least"), `== 0` (the #429/#968
* family) and the `== 1` presence idiom stay clean.
* R2 (error) bulk observed-failing claims — "all N tests … observed failing" /
* "tests N through M … each … observed failing". A test asserting a
* non-change cannot go red before the change exists. Per-test
* "discriminates" wording and subset claims stay clean.
* R3 (error) `$VAR` unquoted in command position — zsh does not word-split an
* unquoted expansion, the command exits 127, and any arm reading its
* status passes vacuously.
* R4 (warn) a fallible command (`git …`) in a non-final pipeline stage — the
* pipeline reports the LAST stage's status, so the failure is swallowed.
* Warn-only: the shape is ambiguous unless the criterion reads the status.
* R5 (error) `wc` output compared by string equality (`… | wc -l | grep -x 0`)
* — BSD `wc` pads its output, so the comparison never matches.
* R6 (error) `git diff`/`git log` with a relative `HEAD~N` anchor — it names
* whatever commit happened to land (another session's). Bare `git diff`
* with no range is a WARNING: a committed change produces no output and
* the check passes, but an uncommitted-tree check is a legitimate idiom.
*
* Legitimate exit: `<!-- plan-criteria-allow: R# - reason -->` (house style of
* `planner-discipline-allow`); the reason must be non-empty. Fail open: no
* criteria zones → no findings; every parse is text-only, nothing executes.
*/
function scanQuantitativeCriteria(content: string): { errors: string[]; warnings: string[] } {
const errors: string[] = [];
const warnings: string[] = [];
// Criteria zones: <acceptance_criteria>, <automated>, <verify>. A <verify>
// block contains its <automated> child, so the same segment may be harvested
// twice — dedupe findings by message below.
const zones: { text: string; prose: boolean }[] = [];
for (const tag of ['acceptance_criteria', 'automated', 'verify'] as const) {
for (const block of extractTaggedBlocks(content || '', tag)) {
zones.push({ text: block, prose: tag === 'acceptance_criteria' });
}
}
if (zones.length === 0) return { errors, warnings };
// Allow markers may sit anywhere in the plan (same as the sibling scanners).
// The reason must be non-empty: `R1 - -->` is not an auditable exit.
const allow = new Set<string>();
const allowRe = /<!--\s*plan-criteria-allow:\s*(R[1-6])\s+-\s+([^>]*\S)\s*-->/g;
let am: RegExpExecArray | null;
while ((am = allowRe.exec(content || '')) !== null) allow.add(am[1]);
const record = (bucket: string[], rule: string, message: string, seen: Set<string>): void => {
if (allow.has(rule) || seen.has(message)) return;
seen.add(message);
bucket.push(message);
};
// A count-grep invocation (grep with -c / --count), per the #429 idiom.
const countGrepRe =
/grep((?:\s+-{1,2}[A-Za-z][A-Za-z-]*)+)\s+(?:'[^']*'|"[^"]*"|[^\s|>&;]+)/g;
const optsHaveCount = (opts: string): boolean =>
/(?:^|\s)-[A-Za-z]*c[A-Za-z]*(?=\s|$)/.test(opts) || /--count\b/.test(opts);
const hasCountGrep = (s: string): boolean => {
countGrepRe.lastIndex = 0;
let m: RegExpExecArray | null;
while ((m = countGrepRe.exec(s)) !== null) {
if (optsHaveCount(m[1])) return true;
}
return false;
};
const seenErr = new Set<string>();
const seenWarn = new Set<string>();
for (const zone of zones) {
// Same normalization as the sibling scanners (#3611): newline join,
// backslash continuation join, entity-amp decode.
const text = decodeEntityAmps(zone.text
.replace(/\r\n/g, '\n')
.replace(/\r/g, '\n')
.replace(/\\\n/g, ' '));
// R2 — bulk observed-failing prose (acceptance_criteria only). The gap
// [^\n.]{0,120} keeps the subject and the claim adjacent within one
// sentence, so the corrected arm (a subset claim, or "each test
// discriminates") never fires.
if (zone.prose) {
const bulkRedRes = [
/\ball\s+(?:\d+|one|two|three|four|five|six|seven|eight|nine|ten|eleven|twelve)\s+tests\b[^\n.]{0,120}?observed\s+fail/i,
/\btests\s+\d+\s+through\s+\d+\b[^\n.]{0,120}?observed\s+fail/i,
];
for (const re of bulkRedRes) {
if (re.test(text)) {
record(
errors,
'R2',
'[plan-criteria R2] A bulk "all N tests / tests N through M were observed failing" criterion is ' +
'unsatisfiable whenever any of the N tests asserts a non-change, which cannot go red before the ' +
'change exists. Say "each test discriminates" (fails before the change, passes after), and attach a ' +
'mutation record for any test that cannot be seen red.',
seenErr,
);
}
}
}
for (const seg of text.split('\n').flatMap(splitShellSegments)) {
// R1 — exact count out of `grep -c` (N ≥ 2). Spaced `==`/`-eq` (the
// #429 zeroCmp idiom, excluding assignments and >=/<=/!=), or the prose
// "returns exactly N".
if (hasCountGrep(seg)) {
const exact: number[] = [];
let m: RegExpExecArray | null;
const shellCmp = /\s==?\s*(\d+)\b/g;
while ((m = shellCmp.exec(seg)) !== null) exact.push(parseInt(m[1], 10));
const eqCmp = /-eq\s+(\d+)\b/g;
while ((m = eqCmp.exec(seg)) !== null) exact.push(parseInt(m[1], 10));
const proseCmp = /\breturns?\s+exactly\s+(\d+)\b/gi;
while ((m = proseCmp.exec(seg)) !== null) exact.push(parseInt(m[1], 10));
if (exact.some((n) => n >= 2)) {
record(
errors,
'R1',
'[plan-criteria R1] An exact count out of `grep -c` is a trap: `grep -c` counts LINES, not matches, ' +
'so the criterion can be false at HEAD before any executor touches it. Use `>= 1`, `== 0`, or ' +
'`grep -n` and read the line numbers.',
seenErr,
);
}
}
// R3 — unquoted $VAR in command position (zsh does not word-split; the
// command exits 127 and any arm reading its status passes vacuously).
// The anchor tolerates a markdown list bullet and/or an inline-code
// backtick before the command — criteria are prose lines like
// "- `$NOKEY npx tsx x.ts` exits 0" — but nothing else: the expansion
// must still be the FIRST token of the command itself.
if (/^(?:[-*]\s*)?\u0060?\$[A-Z_][A-Z0-9_]*\s/.test(seg)) {
record(
errors,
'R3',
'[plan-criteria R3] `$VAR` unquoted in command position does not word-split under zsh — the shell ' +
'looks for a command literally named by the whole expansion and exits 127, so any arm reading its ' +
'status passes vacuously. Write the prefix inline or use a shell function.',
seenErr,
);
}
// R4 — a fallible `git …` in a non-final pipeline stage: the pipeline
// reports the LAST stage's status, so git's failure is swallowed.
// Warn-only: without a status assertion the shape is ambiguous.
// Not a markdown table: the negated-pipe class matches a SHELL pipeline
// stage boundary (git before the next `|`), the same shape the
// #429/#968 scanners use; there is no table row to parse.
// allow-adhoc-markdown: shell pipeline stage boundary, not a table cell (#4024)
if (/\bgit\s+[a-z][^\n|]*\|/.test(seg)) {
record(
warnings,
'R4',
'[plan-criteria R4] A fallible `git` in a non-final pipeline stage is swallowed — the pipeline reports ' +
'the last stage\'s status, so a broken command reads as clean. Capture the status first.',
seenWarn,
);
}
// R5 — `wc` output compared by string equality: BSD `wc` pads its output
// (seven spaces then the number), so `grep -x 0` can never match.
if (/\bwc\b/.test(seg) && /\bgrep\s+(?:-{1,2}[A-Za-z]*x[A-Za-z]*|--line-regexp)\s+['"]?\d+['"]?/.test(seg)) {
record(
errors,
'R5',
'[plan-criteria R5] `wc` output compared by string equality is unsatisfiable on BSD `wc`, which pads ' +
'its output before the number. Compare numerically: `test "$n" -eq 0`, or `tr -d \' \'` first.',
seenErr,
);
}
// R6 — relative HEAD~N anchors name whatever commit happened to land;
// a bare `git diff` with no range makes a committed change invisible.
if (/\bgit\s+(?:diff|log)\b[^\n|;&]*\bHEAD~\d+/.test(seg)) {
record(
errors,
'R6',
'[plan-criteria R6] `git diff`/`git log` with a relative `HEAD~N` anchor names whatever commit ' +
'happened to land last — in a repository with parallel sessions that is another session\'s commit. ' +
'Pin an explicit range: `<sha>^..<sha>`.',
seenErr,
);
} else {
// The argument span ends at the closing backtick of an inline-code
// span ("`git diff` shows ..."), at a redirection, or at a chain
// operator — trailing prose must never read as diff arguments.
const bareDiff = /\bgit\s+diff\b([^\u0060\n|;&<>]*)/.exec(seg);
if (bareDiff) {
const rest = bareDiff[1].trim();
// Bare or flags-only (no rev, range, or path): ambiguous between an
// uncommitted-tree check (legitimate) and a committed-change check
// (vacuously clean) — warn, never error.
if (rest === '' || /^-{1,2}[A-Za-z-]+$/.test(rest)) {
record(
warnings,
'R6',
'[plan-criteria R6] Bare `git diff` with no revision range produces no output for a COMMITTED ' +
'change, so the criterion passes vacuously. Pin an explicit range (`<sha>^..<sha>`) unless the ' +
'check is intentionally about the uncommitted tree.',
seenWarn,
);
}
}
}
}
}
return { errors, warnings };
}
// ─── Plan-task structure validation (#2444) ──────────────────────────────────
/**
@@ -995,6 +1221,13 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo
const conflictScan = scanFileWideNegativeGateConflict(content);
warnings.push(...conflictScan.warnings);
// #4024: quantitative-criteria trap shapes (exact grep -c counts, bulk
// observed-failing claims, $VAR command position, swallowed pipeline
// stages, wc string-equality, relative git anchors).
const quantScan = scanQuantitativeCriteria(content);
errors.push(...quantScan.errors);
warnings.push(...quantScan.warnings);
output(
{
valid: errors.length === 0,
@@ -2141,6 +2374,7 @@ function cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void {
export = {
scanNegativeGrepCommentEcho,
scanFileWideNegativeGateConflict,
scanQuantitativeCriteria,
cmdVerifySummary,
verifySummaryCore,
cmdVerifyPlanStructure,