diff --git a/.changeset/lucky-wasps-chatter.md b/.changeset/lucky-wasps-chatter.md new file mode 100644 index 000000000..a703d4e1d --- /dev/null +++ b/.changeset/lucky-wasps-chatter.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1062 +--- +**The planner now blocks plans that would self-trip their own verify gate** — when an acceptance criterion negative-greps for a literal (`grep -c 'LIT' file == 0`) and that same literal appears verbatim in an `` body, plan creation now fails at write time instead of letting the executor waste cycles on a comment-text echo at commit time. Unquoted/ambiguous grep targets warn instead of failing; add `` to allowlist a legitimate occurrence. (#1062) diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 5905ad140..bc46bbace 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -190,6 +190,14 @@ Every task has four required fields: **Grep gate hygiene:** `grep -c` counts comments, so header prose can be self-invalidating. Use `grep -v '^#' | grep -c token`. Bare `== 0` gates on unfiltered files are forbidden. + +**Comment-text discipline (HARD GATE, #429):** A literal an acceptance criterion negative-greps for (`grep -c 'LIT' file == 0`) must NOT appear verbatim in any `` body — JSDoc samples, head-comment references, or "what NOT to do" snippets echo into the written file and trip the executor's commit-time gate. `validate_plan` (`verify.plan-structure`) fails plan creation on violation. Rephrase the literal by concept, or — when it must legitimately appear — add an allowlist marker on its own line: + +`` + +Full rules + worked examples: @gsd-core/references/planner-antipatterns.md ("Comment-Text Discipline"). + + **:** Acceptance criteria - measurable state of completion. - Good: "Valid credentials return 200 + JWT cookie, invalid credentials return 401" - Bad: "Authentication is complete" diff --git a/docs/AGENTS.md b/docs/AGENTS.md index beb3a0f55..6f9cdb484 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -173,6 +173,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp - Includes `read_first` and `acceptance_criteria` sections - Groups plans into dependency waves - Performs reachability check to validate plan steps reference accessible files and APIs (v1.32) +- Enforces a comment-text discipline HARD GATE at plan-write time (`verify.plan-structure`): a literal that an acceptance criterion negative-greps for (`grep -c 'LIT' file == 0`) must not appear verbatim in an `` body; violations fail plan creation. Use `` to allowlist a legitimate occurrence. (#429) --- diff --git a/gsd-core/references/planner-antipatterns.md b/gsd-core/references/planner-antipatterns.md index db203fbcd..1005c5cce 100644 --- a/gsd-core/references/planner-antipatterns.md +++ b/gsd-core/references/planner-antipatterns.md @@ -87,3 +87,44 @@ A plan should not interleave multiple checkpoint types with implementation tasks - "will be wired later", "dynamic in future phase", "skip for now" If a decision from CONTEXT.md says "display cost calculated from billing table in impulses", the plan must deliver exactly that. Not "static label /min" as a "v1". If the phase is too complex, recommend a phase split instead of silently reducing scope. + +## Comment-Text Discipline (HARD GATE) + +> Enforced at plan-write time by `verify.plan-structure` (the `validate_plan` step). Issue #429. + +When an `` or `` block uses a **negative grep** — `grep -c 'LITERAL' file == 0`, meaning "this literal must NOT appear in the file" — that same `LITERAL` must not appear verbatim anywhere in an `` body. Verbatim code blocks, JSDoc samples, head-comment references, and "what NOT to do" illustrations get echoed into the file the executor writes, so the executor's commit-time gate fails on the *comment text*, not on a real code regression. The work is correct; the gate output is semantically wrong; the executor wastes cycles and learns to distrust the gate. + +**The gate:** plan creation FAILS (error, `valid: false`) when a confidently-extracted (quoted) negative-grep literal also appears in an `` block. When the grep literal is unquoted and cannot be extracted unambiguously, the gate WARNS instead of failing (so you still get the plan, with the risk surfaced). + +### Bad — JSDoc sample echoes the forbidden literal + +```xml + + + Add a `?from=` query param to the share link. Do NOT reintroduce the old + `?from=` referrer hack the JSDoc warned about. + + grep -c '?from=' src/animal-detail.tsx == 0 + +``` + +### Good — rephrase the comment by concept + +```xml + + + Add the share-link query param. Do NOT reintroduce the legacy referrer hack. + + grep -c '?from=' src/animal-detail.tsx == 0 + +``` + +### Allowlist escape hatch + +When the literal MUST appear in the plan body verbatim — e.g. the plan documents the test file that exercises the gate itself, or the literal is part of the verification command's own grep regex — add a marker on its own line so the gate skips that literal: + +``` + +``` + +One marker per literal. The marker exempts only the exact literal it names. diff --git a/src/verify.cts b/src/verify.cts index 96ab08575..94587dfed 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -150,6 +150,104 @@ function cmdVerifySummary( output(result, raw, passed ? 'passed' : 'failed'); } +/** + * Issue #429 — negative-grep comment-text echo gate. + * A literal that an acceptance criterion negative-greps for (grep -c 'LIT' file == 0) + * must not also appear verbatim inside an body, or the executor's commit-time + * verify gate fails on the comment echo rather than a real regression. Conservative: + * errors only on a confidently-extracted QUOTED literal; ambiguous (bareword) → warning. + */ +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 ... \ == 0) is still seen as one segment. + const text = (content || '') + .replace(/\r\n/g, '\n') + .replace(/\r/g, '\n') + .replace(/\\\n/g, ' '); + + // 1. Allowlisted literals: + const allow = new Set(); + const allowRe = //g; + let am: RegExpExecArray | null; + while ((am = allowRe.exec(text)) !== null) allow.add(am[1]); + + // Zero-equality comparison (the negative grep). The required leading whitespace + // before the operator distinguishes a shell comparison (`[ $c == 0 ]`, `... == 0`, + // always spaced) from an assignment (`VAR=0`, never spaced) and naturally excludes + // `>= 0`, `<= 0`, `!= 0`, `!== 0`, `=== 0`. + const zeroCmp = (s: string): boolean => + /\s==?\s*0\b/.test(s) || /-eq\s+0\b/.test(s) || /\bequals\s+0\b/.test(s); + + // A grep invocation using a count flag (-c / -cF / -Fc / --count), capturing the + // search pattern (first quoted token, else first bareword) after a run of options. + // The options run lets `grep -c -F 'LIT'`, `grep -F -c 'LIT'`, `grep -c -e 'LIT'` + // and `grep --count 'LIT'` all resolve to the LIT pattern. + 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); + // `grep -cv 'pat' == 0` counts NON-matching lines, so == 0 there asserts "all lines + // match" — a POSITIVE gate, not our negative gate. Skip inverted greps. + const optsHaveInvert = (opts: string): boolean => + /(?:^|\s)-[A-Za-z]*v[A-Za-z]*(?=\s|$)/.test(opts) || /--invert-match\b/.test(opts); + // Bareword sanity: a real grep target, not a stray operator/number/flag. + const plausibleBare = (s: string): boolean => /[A-Za-z0-9_]/.test(s) && !/^[-=!<>0-9]+$/.test(s); + + // 2. text to scan, with negative-grep COMMAND SPANS removed (only the + // command, not the whole line) so a pasted verify command does not self-flag + // while a prose echo on the same line is still caught. + const cmdSpanRe = + /grep(?:\s+-{1,2}[A-Za-z][A-Za-z-]*)+\s+(?:'[^']*'|"[^"]*"|[^\s'"|>&;]+)[^\n]*?(?:==|-eq|=)\s*0\b/g; + const actionZones: string[] = []; + const actionRe = /([\s\S]*?)<\/action>/g; + let acm: RegExpExecArray | null; + while ((acm = actionRe.exec(text)) !== null) actionZones.push(acm[1]); + const scannableActionText = actionZones.map((zone) => zone.replace(cmdSpanRe, ' ')).join('\n'); + + // 3. Per shell SEGMENT (split lines on && / ||) extract count-grep literals and + // check echoes. Per-segment splitting keeps a positive gate (`== 1`) from + // poisoning a negative gate (`== 0`) sharing the same physical line. + const seenErr = new Set(); + const seenWarn = new Set(); + const segments = text.split('\n').flatMap((line) => line.split(/\s*(?:&&|\|\|)\s*/)); + for (const seg of segments) { + if (!/grep(?:\s+-{1,2}[A-Za-z])/.test(seg) || !zeroCmp(seg)) continue; + countGrepRe.lastIndex = 0; + const quotedLits: string[] = []; + const bareLits: string[] = []; + let m: RegExpExecArray | null; + while ((m = countGrepRe.exec(seg)) !== null) { + if (!optsHaveCount(m[1]) || optsHaveInvert(m[1])) continue; // need count, not invert (-cv is positive) + if (m[2] !== undefined) quotedLits.push(m[2]); + else if (m[3] !== undefined) quotedLits.push(m[3]); + else if (m[4] !== undefined && plausibleBare(m[4])) bareLits.push(m[4]); + } + for (const quoted of quotedLits) { + if (!quoted || allow.has(quoted) || seenErr.has(quoted)) continue; + if (scannableActionText.includes(quoted)) { + seenErr.add(quoted); + errors.push( + `Plan body contains forbidden literal "${quoted}" in an block, but an acceptance criterion negative-greps for it (grep -c ... == 0). Rephrase the literal by concept, remove it from the plan body, or add if it must legitimately appear.`, + ); + } + } + if (quotedLits.length === 0) { + for (const bare of bareLits) { + if (allow.has(bare) || seenWarn.has(bare)) continue; + if (scannableActionText.includes(bare)) { + seenWarn.add(bare); + warnings.push( + `Possible comment-text echo (#429): negative-grep target "${bare}" is unquoted so its literal could not be extracted unambiguously, but it appears in an block. Quote the grep literal and add an allowlist marker if the echo is intended, or rephrase by concept.`, + ); + } + } + } + } + return { errors, warnings }; +} + function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): void { if (!filePath) { error('file path required'); @@ -208,6 +306,10 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo errors.push('Has checkpoint tasks but autonomous is not false'); } + const echoScan = scanNegativeGrepCommentEcho(content); + errors.push(...echoScan.errors); + warnings.push(...echoScan.warnings); + output( { valid: errors.length === 0, @@ -1733,6 +1835,7 @@ function cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void { } export = { + scanNegativeGrepCommentEcho, cmdVerifySummary, cmdVerifyPlanStructure, cmdVerifyPhaseCompleteness, diff --git a/tests/issue-429-comment-text-gate.test.cjs b/tests/issue-429-comment-text-gate.test.cjs new file mode 100644 index 000000000..e0771de5d --- /dev/null +++ b/tests/issue-429-comment-text-gate.test.cjs @@ -0,0 +1,711 @@ +// allow-test-rule: source-text-is-the-product +// Issue #429: the gate logic is tested behaviorally via the exported pure +// function + runGsdTools; the discipline rule + allowlist escape hatch are +// asserted against the agent/reference .md whose text IS the deployed contract. + +'use strict'; + +const { test, describe, before, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); + +// Build path to built verify.cjs +const VERIFY_CJS = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'verify.cjs'); + +// fast-check: loaded at top level so skip flags evaluate correctly +let fc; +try { fc = require('fast-check'); } catch { fc = null; } +// Build path to agent/reference files +const PLANNER_MD = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); +const ANTIPATTERNS_MD = path.join(__dirname, '..', 'gsd-core', 'references', 'planner-antipatterns.md'); + +// ─── Fixtures ────────────────────────────────────────────────────────────────── + +function makePlan({ negativeGrep, actionEcho, allowlistMarker, positiveGrep } = {}) { + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [src/animal-detail.tsx]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# Test Plan', + '', + ]; + + if (allowlistMarker) { + lines.push(allowlistMarker, ''); + } + + lines.push(''); + lines.push('Test task'); + lines.push(''); + if (actionEcho) { + lines.push(actionEcho); + } else { + lines.push('Do the work.'); + } + lines.push(''); + + if (positiveGrep) { + lines.push(`${positiveGrep}`); + } else if (negativeGrep) { + lines.push(`${negativeGrep}`); + } else { + lines.push('npm test'); + } + + lines.push('Task complete'); + lines.push(''); + + return lines.join('\n'); +} + +// ─── Group 1: pure-function unit tests ──────────────────────────────────────── + +describe('scanNegativeGrepCommentEcho — pure unit tests', () => { + let scanNegativeGrepCommentEcho; + + before(() => { + const verify = require(VERIFY_CJS); + scanNegativeGrepCommentEcho = verify.scanNegativeGrepCommentEcho; + }); + + test('case 1 — regression Plan 12-04: action echoes the forbidden literal', () => { + const content = makePlan({ + negativeGrep: "grep -c '?from=' src/animal-detail.tsx == 0", + actionEcho: 'Do NOT reintroduce the old ?from= referrer hack.', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `expected 1 error, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('?from='), `error should mention ?from=, got: ${result.errors[0]}`); + }); + + test('case 2 — regression Plan 11-04: JSDoc head-comment echoes CardModalHost', () => { + const content = makePlan({ + negativeGrep: "grep -c 'CardModalHost' file == 0", + actionEcho: '* @see CardModalHost for the deprecated pattern.', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `expected 1 error, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('CardModalHost'), `error should mention CardModalHost, got: ${result.errors[0]}`); + }); + + test('case 3 — regression Plan 12-02: head-comment echoes .catch(() => null) (regex-special chars)', () => { + const content = makePlan({ + negativeGrep: "grep -c '.catch(() => null)' file == 0", + actionEcho: '// Old pattern: .catch(() => null)', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `expected 1 error, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('.catch(() => null)'), `error should mention the literal, got: ${result.errors[0]}`); + }); + + test('case 4 — boundary: positive count gate (== 60) must NOT be flagged (AC#2)', () => { + const content = makePlan({ + positiveGrep: "grep -c '= makeParallel(' file == 60", + actionEcho: 'Use makeParallel() for concurrent processing.', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, `positive count gate must not flag, errors: ${JSON.stringify(result.errors)}`); + }); + + test('case 5 — no echo: literal only in verify, not in action', () => { + const content = makePlan({ + negativeGrep: "grep -c 'LEGACY_TOKEN' file == 0", + actionEcho: 'Remove the old token handling.', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, 'should be no errors'); + assert.strictEqual(result.warnings.length, 0, 'should be no warnings'); + }); + + test('case 6 — allowlist marker suppresses the error', () => { + const content = makePlan({ + negativeGrep: "grep -c '?from=' src/animal-detail.tsx == 0", + actionEcho: 'Do NOT reintroduce the old ?from= referrer hack.', + allowlistMarker: '', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, `allowlist should suppress error, got: ${JSON.stringify(result.errors)}`); + }); + + test('case 7 — ambiguous unquoted bareword echo: warning not error', () => { + const content = makePlan({ + negativeGrep: 'grep -c badToken file == 0', + actionEcho: 'Remove badToken from codebase.', + }); + const result = scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, `ambiguous token must not error, got: ${JSON.stringify(result.errors)}`); + assert.strictEqual(result.warnings.length, 1, `ambiguous token should warn once, got: ${JSON.stringify(result.warnings)}`); + assert.ok(result.warnings[0].includes('badToken'), `warning should mention badToken, got: ${result.warnings[0]}`); + }); + + test('case 8 — negative-grep command inside an does NOT self-flag', () => { + // action tells executor to ADD the verify command — the grep itself is in the action + // but there is no echo of selfToken outside the grep command + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Add verify command', + '', + "Add this to the CI script: grep -c 'selfToken' file == 0", + '', + 'npm test', + 'Done', + '', + ].join('\n'); + const verify = require(VERIFY_CJS); + const r = verify.scanNegativeGrepCommentEcho(lines); + assert.strictEqual(r.errors.length, 0, `grep command in action must not self-flag, errors: ${JSON.stringify(r.errors)}`); + }); + + test('case 9 — CRLF newlines are normalized', () => { + const content = makePlan({ + negativeGrep: "grep -c '?from=' src/animal-detail.tsx == 0", + actionEcho: 'Do NOT reintroduce the old ?from= referrer hack.', + }); + const crlfContent = content.split('\n').join('\r\n'); + const result = scanNegativeGrepCommentEcho(crlfContent); + assert.strictEqual(result.errors.length, 1, `CRLF content should still find error, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('?from=')); + }); + + test('case 10 — multiple distinct echoed literals each produce their own error', () => { + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Multi literal task', + '', + "Remove tokA and tokB from the codebase.", + '', + "grep -c 'tokA' file == 0 && grep -c 'tokB' file == 0", + 'Done', + '', + ].join('\n'); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(lines); + assert.strictEqual(result.errors.length, 2, `expected 2 errors (one per literal), got: ${JSON.stringify(result.errors)}`); + }); + + test('case 11 — != 0 and >= 0 are NOT negative gates', () => { + const verify = require(VERIFY_CJS); + const content1 = makePlan({ + negativeGrep: "grep -c 'nz' file != 0", + actionEcho: 'Ensure nz is present.', + }); + const r1 = verify.scanNegativeGrepCommentEcho(content1); + assert.strictEqual(r1.errors.length, 0, `!= 0 must not trigger, errors: ${JSON.stringify(r1.errors)}`); + + const content2 = makePlan({ + negativeGrep: "grep -c 'nz' file >= 0", + actionEcho: 'Ensure nz is present.', + }); + const r2 = verify.scanNegativeGrepCommentEcho(content2); + assert.strictEqual(r2.errors.length, 0, `>= 0 must not trigger, errors: ${JSON.stringify(r2.errors)}`); + }); + + // ── Bug-fix regression tests (adversarial-review findings) ─────────────────── + + test('case 12 — mixed positive+negative on one line: no false positive for positive gate token', () => { + // Bug 1: mixed positive+negative greps on one physical line — presentTok is a + // *positive* gate (== 1) and absentTok is a *negative* gate (== 0). Only absentTok + // should be flagged; presentTok must not produce a spurious error. + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Mixed gate task', + '', + 'Use presentTok for the new pattern.', + 'Do not use absentTok any more.', + '', + "grep -c 'presentTok' f == 1 && grep -c 'absentTok' f == 0", + 'Done', + '', + ].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'), `error must NOT name presentTok, 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. + const verify = require(VERIFY_CJS); + const content = makePlan({ + negativeGrep: "grep -c -F '.catch(() => null)' f == 0", + actionEcho: '// Old pattern: .catch(() => null)', + }); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `grep -c -F must extract literal, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('.catch(() => null)'), `error must name the literal, got: ${result.errors[0]}`); + }); + + test('case 14 — grep -F -c (reversed flag order) extracts literal', () => { + // Bug 2: grep -F -c 'LIT' — count flag not in the first position after grep. + const verify = require(VERIFY_CJS); + const content = makePlan({ + negativeGrep: "grep -F -c 'CardModalHost' f == 0", + actionEcho: '* @see CardModalHost for the deprecated pattern.', + }); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `grep -F -c must extract literal, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('CardModalHost'), `error must name CardModalHost, got: ${result.errors[0]}`); + }); + + test('case 15 — grep --count (long option) extracts literal', () => { + // Bug 2: grep --count 'LIT' was not matched by the old -c pattern. + const verify = require(VERIFY_CJS); + const content = makePlan({ + negativeGrep: "grep --count 'longCountTok' f == 0", + actionEcho: 'Remove longCountTok from the codebase.', + }); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, `grep --count must extract literal, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('longCountTok'), `error must name longCountTok, got: ${result.errors[0]}`); + }); + + test('case 16 — same-line command span stripped but prose echo on same line is still caught', () => { + // Bug 3: the old code filtered entire lines; a line with a pasted grep command AND + // a prose echo would be dropped, silencing the error. Only the command SPAN should + // be stripped; prose on the same line that echoes the token must still be detected. + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Span strip task', + '', + // Single line: pasted command PLUS a prose mention of spanTok outside the command + "Run grep -c 'spanTok' f == 0 to confirm; note spanTok must be gone.", + '', + "grep -c 'spanTok' f == 0", + 'Done', + '', + ].join('\n'); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(lines); + assert.strictEqual(result.errors.length, 1, `prose echo outside command span must still be caught, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('spanTok'), `error must name spanTok, got: ${result.errors[0]}`); + }); + + test('case 17 — command-only action (no prose echo) still does NOT self-flag', () => { + // Bug 3 regression guard: when the ONLY occurrence of the token in an action is + // inside the grep command span itself, no error should fire. + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Solo command task', + '', + "grep -c 'soloTok' file == 0", + '', + "grep -c 'soloTok' file == 0", + 'Done', + '', + ].join('\n'); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(lines); + assert.strictEqual(result.errors.length, 0, `command-only action must not self-flag, errors: ${JSON.stringify(result.errors)}`); + }); + + test('case 18 — multi-line backslash continuation in verify command is joined and detected', () => { + // Bug 4: a verify command split with trailing backslash was not joined, so the + // == 0 appeared on a continuation line without the grep prefix → missed. + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [file.ts]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Multi-line verify task', + '', + 'Remove mlTok from all modules.', + '', + 'grep -c \'mlTok\' file \\\n == 0', + 'Done', + '', + ].join('\n'); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(lines); + assert.strictEqual(result.errors.length, 1, `backslash-continued verify must be detected, got: ${JSON.stringify(result.errors)}`); + assert.ok(result.errors[0].includes('mlTok'), `error must name mlTok, got: ${result.errors[0]}`); + }); + + // ── (A) assignment is not a gate ────────────────────────────────────────────── + + test('case 19 — bare STATUS=0 assignment after semicolon is not a negative gate', () => { + // grep -c '...' f > /dev/null; STATUS=0 is an assignment, not a == 0 gate. + // deprecatedTok is echoed in the action but the verify line has no == 0 gate, + // so no error should fire. + const content = makePlan({ + negativeGrep: "grep -c 'deprecatedTok' src/m.ts > /dev/null; STATUS=0", + actionEcho: 'Remove deprecatedTok from the module.', + }); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, [ + 'assignment after semicolon must not be treated as a negative gate,', + `errors: ${JSON.stringify(result.errors)}`, + ].join(' ')); + }); + + test('case 19b — positive control: spaced == 0 IS a gate and fires when token is echoed', () => { + // Same plan as case 19 but the verify line now uses the real == 0 gate form. + // deprecatedTok is echoed in the action → expect exactly 1 error. + const content = makePlan({ + negativeGrep: "grep -c 'deprecatedTok' src/m.ts == 0", + actionEcho: 'Remove deprecatedTok from the module.', + }); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 1, [ + 'spaced == 0 gate with echoed token must produce exactly 1 error,', + `errors: ${JSON.stringify(result.errors)}`, + ].join(' ')); + assert.ok(result.errors[0].includes('deprecatedTok'), `error must name deprecatedTok, got: ${result.errors[0]}`); + }); + + // ── (B) inverted count is not a negative gate ───────────────────────────────── + + test('case 20 — grep -cv with == 0 is NOT a negative gate', () => { + // -cv counts non-matching lines; "== 0" on a -cv result is a positive assertion + // (all lines match), which is out of scope for the negative-grep gate rule. + // invTok is echoed in the action but no error should fire. + const content = makePlan({ + negativeGrep: "grep -cv 'invTok' file == 0", + actionEcho: 'Ensure every line contains invTok.', + }); + const verify = require(VERIFY_CJS); + const result = verify.scanNegativeGrepCommentEcho(content); + assert.strictEqual(result.errors.length, 0, [ + 'grep -cv counts non-matching lines; == 0 is a positive assertion — must not flag,', + `errors: ${JSON.stringify(result.errors)}`, + ].join(' ')); + }); +}); + +// ─── Group 2: end-to-end via runGsdTools ────────────────────────────────────── + +describe('scanNegativeGrepCommentEcho — end-to-end via verify plan-structure', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('e2e case 1 — echoed literal causes valid:false', () => { + const planContent = makePlan({ + negativeGrep: "grep -c '?from=' src/animal-detail.tsx == 0", + actionEcho: 'Do NOT reintroduce the old ?from= referrer hack.', + }); + const planDir = path.join(tmpDir, '.planning', 'phases', '01-test'); + fs.mkdirSync(planDir, { recursive: true }); + fs.writeFileSync(path.join(planDir, '01-01-PLAN.md'), planContent); + + const result = runGsdTools('verify plan-structure .planning/phases/01-test/01-01-PLAN.md', tmpDir); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, false, `expected valid:false, got: ${JSON.stringify(output)}`); + assert.ok( + output.errors.some(e => e.includes('?from=')), + `expected an error mentioning ?from=, got: ${JSON.stringify(output.errors)}`, + ); + }); + + test('e2e case 2 — allowlist marker causes valid:true', () => { + const planContent = makePlan({ + negativeGrep: "grep -c '?from=' src/animal-detail.tsx == 0", + actionEcho: 'Do NOT reintroduce the old ?from= referrer hack.', + allowlistMarker: '', + }); + const planDir = path.join(tmpDir, '.planning', 'phases', '01-test'); + fs.mkdirSync(planDir, { recursive: true }); + fs.writeFileSync(path.join(planDir, '01-01-PLAN.md'), planContent); + + const result = runGsdTools('verify plan-structure .planning/phases/01-test/01-01-PLAN.md', tmpDir); + const output = JSON.parse(result.output); + assert.strictEqual(output.valid, true, `expected valid:true with allowlist, got: ${JSON.stringify(output)}`); + }); +}); + +// ─── Group 3: doc-contract (source-text-is-the-product) ─────────────────────── + +describe('doc-contract: agent/reference .md files carry the deployed contract text', () => { + test('gsd-planner.md contains block', () => { + const content = fs.readFileSync(PLANNER_MD, 'utf8'); + assert.ok(content.includes(''), 'gsd-planner.md must contain '); + }); + + test('gsd-planner.md contains a usage example (`, + }); + const r2 = scanNegativeGrepCommentEcho(withMarker); + assert.strictEqual(r2.errors.length, 0, [ + `allowlist marker "${ALLOW_PREFIX} parityTok -->" must suppress error,`, + `got: ${JSON.stringify(r2.errors)}`, + ].join(' ')); + }); +});