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

fix(#3611): decode entity-escaped ampersands and split shell segments quote-aware (#3693)

Browse Source 
* test(#3611): pin entity-escaped ampersand chains in the negative-grep gate

* fix(#3611): decode entity-escaped ampersands before the negative-grep gate scans

* chore(#3611): add changeset

* test(#3611): pin entity chains in the 968 detector and quote-aware splits

* fix(#3611): quote-aware segment split + entity decode in both plan gates

* chore(#3611): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-19 19:53:45 -04:00
committed by GitHub
parent bad1f045b1
commit 7fc1561806
3 changed files with 197 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/zesty-ibex-chatter.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3693
---
**`verify plan-structure` no longer false-flags positively-asserted literals in entity-escaped verify chains** — planners emit `&amp;&amp;` as the chain operator, which the negative-grep gate's segment splitter did not recognize, so a `= 0` clause poisoned `-ge 3` clauses joined to it and pushed authors toward suppressing a real gate. The gate now scans the decoded text the shell would actually run. (#3611)

82
src/verify.cts
View File

@@ -289,15 +289,78 @@ function cmdVerifySummary(
* verify gate fails on the comment echo rather than a real regression. Conservative:
* errors only on a confidently-extracted QUOTED literal; ambiguous (bareword) → warning.
*/
/**
* Decode entity-escaped ampersands (&amp; → &) — #3611. Planners emit
* <automated> bodies with `&amp;&amp;` as the chain operator (66 occurrences
* vs 0 literal in the reporting repo), and the executing agent reads the
* decoded (rendered) form. Every downstream scan — segment split,
* zero-comparison, literal harvest, echo matching — must operate on the same
* decoded text or a negative clause (`= 0`) poisons the literals of a
* POSITIVE clause (-ge 3) joined to it. Shared by both plan-discipline
* scanners so the two gates cannot drift apart again.
*/
function decodeEntityAmps(s: string): string {
return s.replace(/&amp;/g, '&');
}
/**
* Split one shell line into &&/|| segments, QUOTE-AWARE (#3611 adversarial
* review): an operator inside a quoted literal (`grep -c 'a&&b'`) is part of
* the pattern, not a chain operator — a quote-blind split destroys the
* literal and silently disarms the gate for exactly the plans that spell
* patterns with ampersands. Backslash escapes count inside double quotes
* (POSIX single quotes have none, and over-staying a single-quoted span can
* only miss a split, never invent one).
*/
function splitShellSegments(line: string): string[] {
const segments: string[] = [];
let current = '';
let quote: string | null = null;
for (let i = 0; i < line.length; i++) {
const ch = line[i];
if (quote === "'") {
if (ch === "'") quote = null;
current += ch;
continue;
}
if (quote === '"') {
if (ch === '\\') {
current += ch + (line[i + 1] ?? '');
i++;
continue;
}
if (ch === '"') quote = null;
current += ch;
continue;
}
if (ch === "'" || ch === '"') {
quote = ch;
current += ch;
continue;
}
if ((ch === '&' && line[i + 1] === '&') || (ch === '|' && line[i + 1] === '|')) {
segments.push(current.trim());
current = '';
i++;
continue;
}
current += ch;
}
segments.push(current.trim());
return segments.filter((s) => s !== '');
}
function scanNegativeGrepCommentEcho(content: string): { errors: string[]; warnings: string[] } {
const errors: string[] = [];
const warnings: string[] = [];
// Normalize newlines; join backslash line-continuations so a verify command wrapped
// across lines (grep ... \ <newline> == 0) is still seen as one segment.
const text = (content || '')
// #3611: decode entity-escaped ampersands (see decodeEntityAmps) so every
// downstream scan reads the same decoded text the executing agent reads.
const text = decodeEntityAmps((content || '')
.replace(/\r\n/g, '\n')
.replace(/\r/g, '\n')
.replace(/\\\n/g, ' ');
.replace(/\\\n/g, ' '));
// 1. Allowlisted literals: <!-- planner-discipline-allow: LIT -->
const allow = new Set<string>();
@@ -348,7 +411,7 @@ function scanNegativeGrepCommentEcho(content: string): { errors: string[]; warni
// poisoning a negative gate (`== 0`) sharing the same physical line.
const seenErr = new Set<string>();
const seenWarn = new Set<string>();
const segments = text.split('\n').flatMap((line) => line.split(/\s*(?:&&|\|\|)\s*/));
const segments = text.split('\n').flatMap(splitShellSegments);
for (const seg of segments) {
if (!/grep(?:\s+-{1,2}[A-Za-z])/.test(seg) || !zeroCmp(seg)) continue;
countGrepRe.lastIndex = 0;
@@ -396,10 +459,13 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[]
const warnings: string[] = [];
// Normalize newlines; join backslash line-continuations (same as #429).
const text = (content || '')
// #3611: the SAME entity decode as the #429 scanner — the two gates share
// the caller and the input; a decode on one side only let an entity-escaped
// chain poison this detector's harvest exactly the same way.
const text = decodeEntityAmps((content || '')
.replace(/\r\n/g, '\n')
.replace(/\r/g, '\n')
.replace(/\\\n/g, ' ');
.replace(/\\\n/g, ' '));
// Allowlisted patterns: <!-- planner-region-allow: PAT -->
const allow = new Set<string>();
@@ -547,10 +613,8 @@ function scanFileWideNegativeGateConflict(content: string): { warnings: string[]
for (let ai = 0; ai < tasks.length; ai++) {
const taskA = tasks[ai];
// Split gate text into shell segments (split on && / || within lines).
const segments = taskA.gateText.split('\n').flatMap(line =>
line.split(/\s*(?:&&|\|\|)\s*/),
);
// Split gate text into shell segments (quote-aware, shared with #429 — #3611).
const segments = taskA.gateText.split('\n').flatMap(splitShellSegments);
for (const seg of segments) {
if (!/grep/.test(seg)) continue;

119
tests/verify.test.cjs
View File

@@ -2737,6 +2737,24 @@ describe('scanFileWideNegativeGateConflict — pure unit tests', () => {
});
// Case 2: region-scoped via sed → NO warn
test('case 1b — entity-escaped chain: positive clause pattern must not be harvested as a file-wide ban (#3611)', () => {
// Task A bans banned_thing file-wide (== 0) AND positively asserts
// required_thing (-ge 1), joined by &amp;&amp;. Task B mentions only
// required_thing. Pre-fix, the literal-only split kept the chain as ONE
// segment: zeroCmp saw the == 0 and the harvest took BOTH patterns,
// falsely warning that B conflicts with a file-wide ban on required_thing.
const content = makeTwoTaskPlan({
taskAGate: "grep -c 'banned_thing' app/page.py == 0 &amp;&amp; grep -c 'required_thing' app/page.py -ge 1",
taskBAction: 'Introduce required_thing usage the plan asserts positively.',
});
const result = scan(content);
assert.strictEqual(
result.warnings.filter(w => w.includes('#968')).length,
0,
`a positively-asserted pattern (-ge 1) joined by &amp;&amp; must not warn as a file-wide ban, got: ${JSON.stringify(result.warnings)}`,
);
});
test('case 2 — region-scoped via sed pipe → NO warn', () => {
const content = makeTwoTaskPlan({
taskAGate: "! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh'",
@@ -4204,6 +4222,107 @@ describe('scanNegativeGrepCommentEcho — pure unit tests', () => {
assert.ok(!result.errors[0].includes('presentTok'), `error must NOT name presentTok, got: ${result.errors[0]}`);
});
test('case 12b — mixed gates joined by entity-escaped &amp;&amp;: no false positive for the positive token (#3611)', () => {
// #3611: planners emit <automated> bodies with the ampersands entity-escaped
// (&amp;&amp;). The literal-only segment splitter did not match that spelling,
// so the negative clause's `= 0` poisoned count-grep literals from the
// POSITIVE clause in the same chain — a `-ge 3` literal flagged as forbidden.
// Identical to case 12 except for the ampersand spelling.
const lines = [
'---',
'phase: 01-test',
'plan: 01',
'type: execute',
'wave: 1',
'depends_on: []',
'files_modified: [file.ts]',
'autonomous: true',
'must_haves:',
' - AC1',
'---',
'',
'<task>',
'<name>Entity-escaped chain task</name>',
'<action>',
'Use presentTok for the new pattern.',
'Do not use absentTok any more.',
'</action>',
"<verify><automated>test \"$(grep -c 'absentTok' f)\" = 0 &amp;&amp; test \"$(grep -c 'presentTok' f)\" -ge 3</automated></verify>",
'<done>Done</done>',
'</task>',
].join('\n');
const verify = require(VERIFY_CJS);
const result = verify.scanNegativeGrepCommentEcho(lines);
assert.strictEqual(result.errors.length, 1, `expected exactly 1 error (absentTok only), got: ${JSON.stringify(result.errors)}`);
assert.ok(result.errors[0].includes('absentTok'), `error must name absentTok, got: ${result.errors[0]}`);
assert.ok(!result.errors[0].includes('presentTok'), `a positively-asserted literal (-ge 3) must never be flagged regardless of ampersand spelling, got: ${result.errors[0]}`);
});
test('case 12c — entity-escaped literals and action echoes decode consistently (#3611)', () => {
// A literal that itself contains &amp; (e.g. "a&amp;b" as the grep pattern)
// and an action echo carrying the same entity spelling must still match
// after the decode — the flag stays correct for entity-bearing literals.
const lines = [
'---',
'phase: 01-test',
'plan: 01',
'type: execute',
'wave: 1',
'depends_on: []',
'files_modified: [file.ts]',
'autonomous: true',
'must_haves:',
' - AC1',
'---',
'',
'<task>',
'<name>Entity literal task</name>',
'<action>',
'Remove the old a&amp;b join.',
'</action>',
"<verify><automated>grep -c 'a&amp;b' f == 0</automated></verify>",
'<done>Done</done>',
'</task>',
].join('\n');
const verify = require(VERIFY_CJS);
const result = verify.scanNegativeGrepCommentEcho(lines);
assert.strictEqual(result.errors.length, 1, `the entity-bearing literal must still flag its action echo, got: ${JSON.stringify(result.errors)}`);
assert.ok(result.errors[0].includes('a&b'), `error must carry the decoded literal a&b, got: ${result.errors[0]}`);
});
test('case 12d — a quoted literal CONTAINING && is not shattered by the segment split (#3611 review)', () => {
// A negative grep banning a boolean shape (`grep -c 'a&&b' f == 0`) and an
// action echo mentioning a&&b. The split must be quote-aware: splitting on
// the operator inside the quotes would destroy the literal and silently
// disarm the gate — exactly the plans that spell patterns with ampersands.
const lines = [
'---',
'phase: 01-test',
'plan: 01',
'type: execute',
'wave: 1',
'depends_on: []',
'files_modified: [file.ts]',
'autonomous: true',
'must_haves:',
' - AC1',
'---',
'',
'<task>',
'<name>Quoted operator literal task</name>',
'<action>',
'Remove the a&&b join.',
'</action>',
"<verify><automated>grep -c 'a&&b' f == 0</automated></verify>",
'<done>Done</done>',
'</task>',
].join('\n');
const verify = require(VERIFY_CJS);
const result = verify.scanNegativeGrepCommentEcho(lines);
assert.strictEqual(result.errors.length, 1, `the quoted a&&b literal must still flag its action echo, got: ${JSON.stringify(result.errors)}`);
assert.ok(result.errors[0].includes('a&&b'), `error must carry the intact literal, got: ${result.errors[0]}`);
});
test('case 13 — grep -c -F (separate count+fixed flags) extracts literal', () => {
// Bug 2: grep -c -F 'LIT' was not extracted by the old regex that required -c
// immediately before the pattern without intervening flags.