From 04ac8723b9d5a0040b06fd628080a571f7793d27 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 4 Sep 2026 21:04:49 -0400 Subject: [PATCH] fix(#4024): flag quantitative-criteria trap shapes in verify plan-structure (#4288) * 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 // 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: . 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 --- .changeset/proud-deer-jump.md | 5 + src/verify.cts | 234 ++++++++++++++++++++++ tests/verify.test.cjs | 359 ++++++++++++++++++++++++++++++++++ 3 files changed, 598 insertions(+) create mode 100644 .changeset/proud-deer-jump.md diff --git a/.changeset/proud-deer-jump.md b/.changeset/proud-deer-jump.md new file mode 100644 index 000000000..bac028b2d --- /dev/null +++ b/.changeset/proud-deer-jump.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4288 +--- +**`verify plan-structure` now flags quantitative acceptance criteria that are traps at HEAD** — plans whose criteria used an exact `grep -c` count, a bulk "all N tests were observed failing" claim, an unquoted $VAR in command position, `wc` output compared by string equality, or a relative `HEAD~N` git anchor passed verification while the criterion was unsatisfiable or vacuous before any work began. (#4024) diff --git a/src/verify.cts b/src/verify.cts index 84ee1db18..556cbdf61 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -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 — ``, ``, and + * `` blocks. Prose elsewhere in the plan (e.g. an explanatory + * ``) 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: `` (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: , , . A + // block contains its 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(); + const allowRe = //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): 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(); + const seenWarn = new Set(); + + 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: `^..`.', + 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 (`^..`) 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, diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index c2d193ba3..0d0fa3629 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -4798,3 +4798,362 @@ describe('allowlist-syntax parity: doc marker == runtime marker', () => { }); }); } + +// ─── #4024: quantitative acceptance criteria measured shapes ───────────────── +// +// scanQuantitativeCriteria is the third plan-discipline scanner in the +// cmdVerifyPlanStructure family (after #429's comment-echo gate and #968's +// file-wide negative-gate conflict detector). It judges whether quantitative +// acceptance criteria are WRITTEN in a shape that is provably a trap at HEAD — +// an exact line-count out of `grep -c`, a bulk all-N observed-failing claim, +// an unquoted $VAR in command position, a fallible command swallowed by a +// non-final pipeline stage, `wc` output compared by string equality, or a +// `git diff`/`git log` anchored to whatever commit happened to land. +// +// Every rule has a corrected arm asserted here: a rule that fires on its own +// fix is a refusal, not a rule (issue #4024, "Every rule needs a corrected arm"). + +const CRITERIA_PLAN_HEAD = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [some/file.ts]', + 'autonomous: true', + 'must_haves:', + ' truths:', + ' - "something is true"', + '---', + '', +]; + +function makeCriteriaPlan(criteriaBody, taskVerify) { + return CRITERIA_PLAN_HEAD.concat([ + '', + criteriaBody, + '', + '', + '', + ' Task 1: Do something', + ' some/file.ts', + ' Do the thing', + ` ${taskVerify || 'echo ok'}`, + ' Thing is done', + '', + ]).join('\n'); +} + +describe('#4024: scanQuantitativeCriteria — pure unit tests', () => { + let scanQuantitativeCriteria; + + // Self-contained bindings: this block sits below the file's fold-points, + // whose nested scopes own the earlier VERIFY_CJS consts and omit `before` + // from the top-level destructure. Bind both locally rather than relying + // on file position. + const before = require('node:test').before; + const VERIFY_CJS_LOCAL = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'verify.cjs'); + + before(() => { + const verify = require(VERIFY_CJS_LOCAL); + scanQuantitativeCriteria = verify.scanQuantitativeCriteria; + }); + + // Row 1 — issue reproduction shape 1 (phase 444 row 13) + test('#4024 R1: exact grep -c count (== 2) in acceptance_criteria is an error', () => { + const content = makeCriteriaPlan( + " - `grep -c 'unassignedTabs' store.ts` returns exactly 2.", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R1]')), + `expected an R1 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Row 1 shell spelling of the same trap + test('#4024 R1: shell exact-count comparison (grep -c ... == 2) is an error', () => { + const content = makeCriteriaPlan( + ' - `grep -c \'unassignedTabs\' store.ts == 2` passes.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R1]')), + `expected an R1 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Rows 4+5 — corrected arms: hedged counts, and the == 1 presence idiom + test('#4024 R1 corrected arm: hedged counts and == 1 / == 0 gates stay clean', () => { + for (const criterion of [ + "`grep -c 'presentTok' f == 1` passes.", + "`grep -c 'absentTok' f == 0` passes.", + "`grep -c 'tok' f` returns at least 2.", + "`grep -c 'tok' f` is >= 1.", + ]) { + const result = scanQuantitativeCriteria(makeCriteriaPlan(` - ${criterion}`, '')); + assert.deepStrictEqual( + result.errors.filter(e => e.includes('[plan-criteria R1]')), + [], + `hedged/presence criterion must stay clean: ${criterion}, got: ${JSON.stringify(result.errors)}`, + ); + } + }); + + // Row 2 — issue reproduction shape 2 (phase 444 row 6) + test('#4024 R2: bulk all-N observed-failing claim is an error', () => { + const content = makeCriteriaPlan( + ' - All seven tests were observed FAILING before the implementation existed.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R2]')), + `expected an R2 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Row 3 — phase 444 row 14 spelling + test('#4024 R2: N-through-M observed-failing claim is an error', () => { + const content = makeCriteriaPlan( + ' - Tests 8 through 20 were each observed FAILING before their rule existed.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R2]')), + `expected an R2 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Row 6 — corrected arm: the prescription row 6 itself recorded + test('#4024 R2 corrected arm: per-test discrimination wording stays clean', () => { + const content = makeCriteriaPlan( + ' - Each test discriminates: it fails before the change and passes after it.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.deepStrictEqual(result.errors, [], + `corrected wording must stay clean, got: ${JSON.stringify(result.errors)}`); + // Subset claims without the all-N / N-through-M subject are also clean + // (the corrected arm still says "were observed failing" about a subset). + const subset = scanQuantitativeCriteria(makeCriteriaPlan( + ' - The two tests that pin the rule were observed failing before the change.', '')); + assert.deepStrictEqual(subset.errors, [], + `subset wording must stay clean, got: ${JSON.stringify(subset.errors)}`); + }); + + // Row 7 — phase 443 row 8: $NOKEY in command position + test('#4024 R3: unquoted $VAR in command position is an error', () => { + const content = makeCriteriaPlan( + ' - `$NOKEY npx tsx scripts/check.ts` exits 0 (NOKEY="env -u KEY").', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R3]')), + `expected an R3 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Row 8 — corrected arm: prefix written inline + test('#4024 R3 corrected arm: inline env prefix stays clean', () => { + const content = makeCriteriaPlan( + ' - `env -u KEY npx tsx scripts/check.ts` exits 0.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.deepStrictEqual(result.errors, [], + `inline prefix must stay clean, got: ${JSON.stringify(result.errors)}`); + }); + + // Row 9 — phase 443 rows 6/9: fallible git swallowed by a non-final stage + test('#4024 R4: fallible git in non-final pipeline stage warns (never errors)', () => { + const content = makeCriteriaPlan( + ' - `git grep -l "pattern" | wc -l` is 0.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.warnings.some(w => w.includes('[plan-criteria R4]')), + `expected an R4 warning, got: ${JSON.stringify(result.warnings)}`, + ); + assert.deepStrictEqual(result.errors, [], + `R4 is warn-only, got errors: ${JSON.stringify(result.errors)}`); + }); + + // Row 10 — phase 443 row 24: BSD wc pads, grep -x 0 never matches + test('#4024 R5: wc output compared by grep -x string equality is an error', () => { + const content = makeCriteriaPlan( + ' - `git status --porcelain | wc -l | grep -x 0` succeeds.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R5]')), + `expected an R5 error, got: ${JSON.stringify(result.errors)}`, + ); + }); + + // Row 11 — corrected arm: numeric comparison + test('#4024 R5 corrected arm: numeric test on captured count stays clean', () => { + const content = makeCriteriaPlan( + ' - `n=$(git status --porcelain | wc -l); test "$n" -eq 0` succeeds.', + '', + ); + const result = scanQuantitativeCriteria(content); + assert.deepStrictEqual(result.errors, [], + `numeric test must stay clean, got: ${JSON.stringify(result.errors)}`); + }); + + // Row 12 — phase 443 row 25: HEAD~1 names whatever landed last + test('#4024 R6: relative HEAD~N anchor is an error', () => { + for (const criterion of [ + '`git diff HEAD~1 --stat` shows no deletions.', + '`git log HEAD~2..HEAD --oneline` lists only this session\'s commits.', + ]) { + const result = scanQuantitativeCriteria(makeCriteriaPlan(` - ${criterion}`, '')); + assert.ok( + result.errors.some(e => e.includes('[plan-criteria R6]')), + `expected an R6 error for: ${criterion}, got: ${JSON.stringify(result.errors)}`, + ); + } + }); + + // Rows 13+14 — corrected arm (explicit range) and ambiguous shape (bare diff) + test('#4024 R6 corrected arm + bare-diff warning', () => { + const fixed = scanQuantitativeCriteria(makeCriteriaPlan( + ' - `git diff abc1234^..abc1234 --stat` shows no deletions.', '')); + assert.deepStrictEqual(fixed.errors.filter(e => e.includes('[plan-criteria R6]')), [], + `explicit sha range must stay clean, got: ${JSON.stringify(fixed.errors)}`); + + const bare = scanQuantitativeCriteria(makeCriteriaPlan( + ' - `git diff` shows no deletions.', '')); + assert.ok( + bare.warnings.some(w => w.includes('[plan-criteria R6]')), + `bare git diff must warn, got: ${JSON.stringify(bare.warnings)}`, + ); + assert.deepStrictEqual(bare.errors, [], + `bare git diff is warn-only, got: ${JSON.stringify(bare.errors)}`); + }); + + // Rows 15+16 — the legitimate exit + test('#4024 allow marker suppresses the flagged rule; empty reason is not honored', () => { + const flagged = makeCriteriaPlan( + ' - `grep -c \'unassignedTabs\' store.ts` returns exactly 2.', + '', + ); + const withMarker = flagged.replace( + '', + '\n ', + ); + const r1 = scanQuantitativeCriteria(withMarker); + assert.deepStrictEqual(r1.errors, [], + `allow marker with a reason must suppress R1, got: ${JSON.stringify(r1.errors)}`); + + const emptyReason = flagged.replace( + '', + '\n ', + ); + const r2 = scanQuantitativeCriteria(emptyReason); + assert.ok( + r2.errors.some(e => e.includes('[plan-criteria R1]')), + `empty reason must NOT suppress R1, got: ${JSON.stringify(r2.errors)}`, + ); + }); + + // Rows 17+18 — fail open / negative space + test('#4024 fail-open: text outside criteria zones and plans without criteria are silent', () => { + // Zone-scope proof: an -only trap must not be judged. + const actionOnly = CRITERIA_PLAN_HEAD.concat([ + '', + ' Task 1', + ' f', + ' Run `git diff HEAD~1` to inspect the previous session.', + ' echo ok', + ' Done', + '', + ]).join('\n'); + const rAction = scanQuantitativeCriteria(actionOnly); + assert.deepStrictEqual(rAction.errors, [], + `-only text is not judged, got: ${JSON.stringify(rAction.errors)}`); + assert.deepStrictEqual(rAction.warnings, [], + `-only text yields no warnings, got: ${JSON.stringify(rAction.warnings)}`); + + const noCriteria = scanQuantitativeCriteria(validPlanContent()); + assert.deepStrictEqual(noCriteria.errors, []); + assert.deepStrictEqual(noCriteria.warnings, []); + }); + + // Row 19 — normalization: CRLF + entity-escaped chains + test('#4024 normalization: CRLF and && chains are read decoded', () => { + const content = makeCriteriaPlan( + " - `grep -c 'a' f == 2 && git diff HEAD~1 --stat` passes.", + '', + ).replace(/\n/g, '\r\n'); + const result = scanQuantitativeCriteria(content); + assert.ok(result.errors.some(e => e.includes('[plan-criteria R1]')), + `entity-escaped chain must still trip R1, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors.some(e => e.includes('[plan-criteria R6]')), + `entity-escaped chain must still trip R6, got: ${JSON.stringify(result.errors)}`); + }); +}); + +describe('#4024: verify plan-structure — quantitative criteria gate (e2e)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-test'), { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + function writePlan(content) { + const planPath = path.join(tmpDir, '.planning', 'phases', '01-test', '01-01-PLAN.md'); + fs.writeFileSync(planPath, content); + return 'verify plan-structure .planning/phases/01-test/01-01-PLAN.md'; + } + + // Row 20 — the issue's reproduction, verbatim shapes + test('#4024 e2e: issue reproduction plan is invalid', () => { + const cmd = writePlan(makeCriteriaPlan([ + ' - `grep -c \'someIdentifier\' src/some/file.ts` returns exactly 2.', + ' - All seven tests were observed FAILING before the implementation existed.', + ].join('\n'), '')); + const result = runGsdTools(cmd, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.valid, false, `trap criteria must invalidate the plan, errors: ${JSON.stringify(out.errors)}`); + assert.ok(out.errors.length >= 2, `both shapes must be flagged, got: ${JSON.stringify(out.errors)}`); + }); + + test('#4024 e2e: corrected-arm plan stays valid', () => { + const cmd = writePlan(makeCriteriaPlan([ + ' - `grep -c \'presentTok\' f == 1` passes.', + ' - Each test discriminates: it fails before the change and passes after it.', + ].join('\n'), '')); + const result = runGsdTools(cmd, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.valid, true, `corrected plan must stay valid, errors: ${JSON.stringify(out.errors)}`); + assert.deepStrictEqual(out.errors, []); + }); + + test('#4024 e2e: allow marker restores validity', () => { + const cmd = writePlan(makeCriteriaPlan([ + ' ', + ' - `grep -c \'someIdentifier\' src/some/file.ts` returns exactly 2.', + ].join('\n'), '')); + const result = runGsdTools(cmd, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.valid, true, `allow marker must restore validity, errors: ${JSON.stringify(out.errors)}`); + assert.deepStrictEqual(out.errors, []); + }); +});