diff --git a/.changeset/humble-wasps-rally.md b/.changeset/humble-wasps-rally.md new file mode 100644 index 000000000..7c46cceef --- /dev/null +++ b/.changeset/humble-wasps-rally.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4877 +--- +**verify plan-structure stops warning that a logical-OR fallback swallows a failure** — the R4 scan read "||" as a pipe, so the standard "git cat-file -e || echo missing" ghost-control idiom was flagged as a swallowed failure when "||" is exactly the construct handling it; single pipes (including zsh "|&") still warn. (#4774) diff --git a/src/verify.cts b/src/verify.cts index dd9ce2329..c512b9edf 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -922,8 +922,15 @@ function scanQuantitativeCriteria(content: string): { errors: string[]; warnings // 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. + // #4774: the lookahead keeps `||` — a logical OR, the construct that + // HANDLES the failure (e.g. the `git cat-file -e || echo missing` + // ghost-control idiom, reachable here when prose apostrophes leave the + // segment's quote state unclosed) — quiet, while `|` and `|&` (a real + // pipeline stage, stderr-merged or not) still warn. Accepted trade-off: + // a QUOTED `||` literal ahead of a real pipe (``git grep 'a||b' f | wc -l``) + // also goes quiet — the lookahead cannot reach past the doubled pipe. // allow-adhoc-markdown: shell pipeline stage boundary, not a table cell (#4024) - if (/\bgit\s+[a-z][^\n|]*\|/.test(seg)) { + if (/\bgit\s+[a-z][^\n|]*\|(?!\|)/.test(seg)) { record( warnings, 'R4', diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index 091f24f25..222be1915 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -5199,6 +5199,109 @@ describe('#4024: scanQuantitativeCriteria — pure unit tests', () => { `R4 is warn-only, got errors: ${JSON.stringify(result.errors)}`); }); + // #4774 — `||` is a logical OR: the construct REACTS to a non-zero exit, so + // warning that the failure is "swallowed" inverts the semantics and fires on + // the git cat-file -e || echo missing ghost-control idiom. The R4 + // regex must require the pipe NOT to be doubled. + // + // Reachability (adversarial review): splitShellSegments already splits at + // UNQUOTED ||, so the plain form never reaches R4 — the false positive + // reaches it when prose leaves the segment's quote state unclosed (an + // apostrophe: "doesn't"). These tests use that shape, which is genuinely + // red under the old regex. + test('#4774 R4: a logical-OR fallback is a handled failure, not a swallowed one (apostrophe-confused prose)', () => { + const content = makeCriteriaPlan( + " - Doesn't assume tags: `git cat-file -e $sha || echo missing` exits 0 (ghost control: a bogus hash must print \"missing\").", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + !result.warnings.some(w => w.includes('[plan-criteria R4]')), + `|| must not read as a pipeline stage boundary, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + test('#4774 R4: a bare logical-OR fallback stays quiet in apostrophe prose (issue row 1)', () => { + const content = makeCriteriaPlan( + " - Doesn't assume history: `git log --oneline || echo none` prints at least the header line.", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + !result.warnings.some(w => w.includes('[plan-criteria R4]')), + `got: ${JSON.stringify(result.warnings)}`, + ); + }); + + test('#4774 R4: logical-OR into a block stays quiet (issue row 8)', () => { + const content = makeCriteriaPlan( + " - Doesn't assume a clean tree: `git status --porcelain || { echo dirty; exit 1; }` must not print \"dirty\".", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + !result.warnings.some(w => w.includes('[plan-criteria R4]')), + `got: ${JSON.stringify(result.warnings)}`, + ); + }); + + test('#4774 R4: real single-pipe pipelines still warn (issue rows 3/4)', () => { + // One criterion per test: the scanner dedupes findings BY MESSAGE, so two + // R4-worthy lines report a single warning — per-line is the suite's idiom. + for (const line of [ + ' - `git rev-parse HEAD | cut -c1-10` is 10 chars long.', + ' - `git diff --name-only | wc -l` is 0.', + ]) { + const result = scanQuantitativeCriteria(makeCriteriaPlan(line, '')); + assert.ok( + result.warnings.some(w => w.includes('[plan-criteria R4]')), + `single-pipe stage must still warn, got: ${JSON.stringify(result.warnings)}`, + ); + } + }); + + test('#4774 R4: a pipeline boundary before the OR still warns (issue row 6)', () => { + const content = makeCriteriaPlan( + " - Doesn't assume tags: `git log --oneline | head -5 || echo none` prints 5 lines.", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.warnings.some(w => w.includes('[plan-criteria R4]')), + `git|head is a real non-final stage, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + test('#4774 R4: a |& stderr-merge is a real pipeline and still warns (issue row 7)', () => { + const content = makeCriteriaPlan( + " - Doesn't assume tags: `git grep -l \"pattern\" |& wc -l` is 0.", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + result.warnings.some(w => w.includes('[plan-criteria R4]')), + `|& merges stderr into the pipeline — git is still non-final, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + test('#4774 R4: a quoted || ahead of a real pipe goes quiet — pinned accepted trade-off', () => { + // Adversarial review Finding 3: the lookahead cannot reach past a doubled + // pipe, so a QUOTED '||' literal inside a grep pattern ahead of a real + // pipeline boundary is no longer flagged. This is a deliberate, disclosed + // trade-off (warn-only, narrow class, and the quoted form is exactly the + // case where "||" is most likely to be DATA, not control). Pinned here so + // a future grammar change makes the trade-off decision consciously. + const content = makeCriteriaPlan( + " - `git grep 'a||b' f | wc -l` is 0.", + '', + ); + const result = scanQuantitativeCriteria(content); + assert.ok( + !result.warnings.some(w => w.includes('[plan-criteria R4]')), + `pinned trade-off: quoted || ahead of a real pipe is quiet; got: ${JSON.stringify(result.warnings)}`, + ); + }); + // 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(