diff --git a/.changeset/zesty-badgers-caper.md b/.changeset/zesty-badgers-caper.md new file mode 100644 index 000000000..2f911e84c --- /dev/null +++ b/.changeset/zesty-badgers-caper.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1320 +--- +**`verify plan-structure` warns on cross-task region-scope conflicts (#968)** — when a plan task's file-wide negative grep (`! grep -Eq 'PAT' file` / `grep -c 'PAT' file == 0`) bans a construct a sibling task legitimately requires elsewhere in the same file, plan validation now surfaces a warning pointing to the new region/function-scoped negative-gate idiom (documented in the gsd-planner guidance and the planner-antipatterns reference, with a worked banned-in-X / required-in-Y example). Warn-only: it never errors and never changes `valid`. (#1320) diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 89717878a..cdc7e4cb9 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -198,6 +198,10 @@ Every task has four required fields: Full rules + worked examples: @gsd-core/references/planner-antipatterns.md ("Comment-Text Discipline"). + +**Region-scoped negative gates (WARN, #968):** Region-scope a file-wide negative grep when a sibling task needs that construct elsewhere in the same file; `validate_plan` WARNS. See: @gsd-core/references/planner-antipatterns.md ("Region-Scoped Negative Gates"). + + **:** 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/reference/plan-md.md b/docs/reference/plan-md.md index e2de09fbc..38982e2d8 100644 --- a/docs/reference/plan-md.md +++ b/docs/reference/plan-md.md @@ -187,7 +187,7 @@ Plans that contain any checkpoint task must set `autonomous: false` in frontmatt | `` | Files the executor must read before touching anything — the file being modified, any source-of-truth pattern file, any file whose types or conventions must be replicated. | | `` | Concrete instructions with exact identifiers, file paths, function signatures, and expected values. Never says "align X with Y" without specifying the target state. Never contains fenced code blocks or full implementations. | | `` | A runnable command or check that proves the task succeeded. Must distinguish pass from fail — `echo "done"` is not valid. | -| `` | Verifiable conditions: grep-verifiable strings, command exit codes, observable behaviours. No subjective language ("looks correct", "properly configured"). | +| `` | Verifiable conditions: grep-verifiable strings, command exit codes, observable behaviours. No subjective language ("looks correct", "properly configured"). Negative greps (`! grep -Eq 'PAT' file`) are file-scoped — region-scope them (`sed -n`/`awk` range, then grep) when a sibling task needs the construct elsewhere in the same file (#968). | | `` | A short measurable statement of the completed outcome. | --- diff --git a/gsd-core/references/planner-antipatterns.md b/gsd-core/references/planner-antipatterns.md index 1005c5cce..26e5d1fad 100644 --- a/gsd-core/references/planner-antipatterns.md +++ b/gsd-core/references/planner-antipatterns.md @@ -128,3 +128,49 @@ When the literal MUST appear in the plan body verbatim — e.g. the plan documen ``` One marker per literal. The marker exempts only the exact literal it names. + +## Region-Scoped Negative Gates + +> Surfaced at plan-write time by `verify.plan-structure` (the `validate_plan` step), WARN-level. Issue #968. + +A **negative grep** — `! grep -Eq 'PAT' file` or `grep -c 'PAT' file == 0` — asserts a construct is absent. `grep` is file-scoped by nature: it has no notion of function or region. This breaks down when a phase splits one file across parallel tasks with legitimately opposite needs for the same construct in different regions: + +- **Task A** bans the construct file-wide — a synchronous factory must not block on a refresh: `! grep -Eq 'await .*refresh' app/page.py`. +- **Task B** legitimately requires it elsewhere in the same file — a post-reindex handler must `await bridge.refresh()` to repopulate state. + +Both occurrences are real, correct production code in different functions of one file. A file-wide negative grep cannot say "absent in function X, present in function Y", so the two gates are mutually unsatisfiable with a direct call — the executor is pushed into an indirection whose only purpose is to relocate the matched string out of the file (pure gate-appeasement, zero behaviour change). This is distinct from Comment-Text Discipline (#429): there is no comment echo and no allowlist helps — the construct must genuinely be present in one region and absent in another. + +**The fix:** region-scope the negative gate so "absent in region X" stops implying "absent file-wide." + +### Bad — file-wide ban unsatisfiable against a sibling's real code + +```xml + +! grep -Eq 'await .*refresh' app/page.py + +Add a post-reindex handler in app/page.py that awaits bridge.refresh(). +app/page.py +``` + +Task B writing `await bridge.refresh()` trips Task A's file-wide gate, though both are correct. + +### Good — scope the gate to the factory region + +```xml + +! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh' + +! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh' +``` + +The factory region is asserted clean; the reindex handler elsewhere in the same file keeps its required `await bridge.refresh()`. Both gates pass with no code restructuring. Prefer an AST/structural check or a focused unit test where region extraction is fragile. + +### When the split is intentional and unavoidable + +If region-scoping is genuinely impractical and the file split is intentional, suppress the warning with a marker naming the pattern: + +``` + +``` + +One marker per pattern. The marker exempts only the exact pattern it names. Prefer region-scoping over suppression. diff --git a/src/verify.cts b/src/verify.cts index ad3583865..0ba8ad168 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -253,6 +253,310 @@ function scanNegativeGrepCommentEcho(content: string): { errors: string[]; warni return { errors, warnings }; } +/** + * Issue #968 — file-wide negative-grep sibling conflict detector. + * A file-wide negative grep gate (! grep -Eq 'PAT' FILE or grep -c 'PAT' FILE == 0) + * bans a construct across the WHOLE file. When a sibling task in the same plan + * legitimately requires the same construct in the same file, the two gates are + * mutually unsatisfiable. This is a WARN-only check (never changes valid:false). + */ +function scanFileWideNegativeGateConflict(content: string): { warnings: string[]; valid: true } { + const warnings: string[] = []; + + // Normalize newlines; join backslash line-continuations (same as #429). + const text = (content || '') + .replace(/\r\n/g, '\n') + .replace(/\r/g, '\n') + .replace(/\\\n/g, ' '); + + // Allowlisted patterns: + const allow = new Set(); + const allowRe = //g; + let am: RegExpExecArray | null; + while ((am = allowRe.exec(text)) !== null) allow.add(am[1]); + + // Helper predicates (reused from #429 style). + // Zero-equality comparison: spaced == 0 or -eq 0. + const zeroCmp = (s: string): boolean => + /\s==?\s*0\b/.test(s) || /-eq\s+0\b/.test(s) || /\bequals\s+0\b/.test(s); + + // grep options include -c / --count + const optsHaveCount = (opts: string): boolean => + /(?:^|\s)-[A-Za-z]*c[A-Za-z]*(?=\s|$)/.test(opts) || /--count\b/.test(opts); + + // grep options include -v / --invert-match (inverted count is NOT a negative gate) + const optsHaveInvert = (opts: string): boolean => + /(?:^|\s)-[A-Za-z]*v[A-Za-z]*(?=\s|$)/.test(opts) || /--invert-match\b/.test(opts); + + // A bareword that is a plausible grep pattern (not a stray flag/number). + const plausibleBare = (s: string): boolean => /[A-Za-z0-9_]/.test(s) && !/^[-=!<>0-9]+$/.test(s); + + // Regex to extract grep arguments: opts run then PAT (quoted or bare). + const grepArgRe = + /grep((?:\s+-{1,2}[A-Za-z][A-Za-z-]*)+)\s+(?:'([^']*)'|"([^"]*)"|([^\s'"|>&;$()\[\]]+))/g; + + // FIX 1 (ReDoS): Linear-time "does reqText satisfy the grep pattern" — no RegExp execution. + // Never calls new RegExp, so no catastrophic backtracking is possible. + // + // Handles literal patterns and `.`/`.*/`.+`/`\s`-style wildcard gaps and `^`/`$` anchors. + // Patterns using character classes (`[…]`), alternation (`a|b`), or other regex constructs + // fall back to a conservative literal-substring check, so the detector may NOT warn on those + // (false-negative is the safe direction for a warn-only advisory). + const patternRequiredIn = (pat: string, reqText: string): boolean => { + const hay = (reqText || '').slice(0, 8000); // bound the haystack + if (!pat) return false; + // Strip ERE anchors — position constraints don't change whether the construct is required. + pat = pat.replace(/^\^/, '').replace(/\$$/, ''); + if (!pat) return false; + // Pure literal (no regex metacharacters): direct substring. + if (!/[.*+?^${}()|[\]\\]/.test(pat)) return hay.includes(pat); + const SENT = ' '; + // Replace simple wildcard gaps (\s* \w+ .* .+ .? bare .) with a sentinel. + let work = pat + .replace(/\\[sSwWdD][*+?]?/g, SENT) + .replace(/\.[*+?]/g, SENT) + .replace(/\./g, SENT); + work = work.replace(/\\(.)/g, '$1'); // de-escape \( \. etc → literal char + const joined = work.split(SENT).join(''); + // Unhandled regex constructs remain → safe literal-substring fallback on the raw pattern. + if (/[*+?^${}()|[\]]/.test(joined)) return hay.includes(pat); + const frags = work.split(SENT).filter(Boolean); + if (!frags.length) return false; // all-wildcard pattern → no meaningful requirement + let pos = 0; + for (const f of frags) { + const idx = hay.indexOf(f, pos); + if (idx === -1) return false; + pos = idx + f.length; + } + return true; + }; + + // FIX 2 (file basename over-match): exact normalized match; basename fallback ONLY for + // unqualified gate files (no path separator). + const normPath = (p: string): string => p.replace(/^\.\//, '').trim(); + + // File-wide discriminator: a token AFTER PAT that looks like a path. + // Paths have /, a file extension, or match a known task entry. + // Globs (containing *) are excluded (unresolvable — no warn). + const looksLikePath = (token: string): boolean => + !token.includes('*') && + (token.includes('/') || /\.[a-zA-Z]{1,6}$/.test(token)); + + // FIX 5 (hasLeadingNot): collapse to one command-boundary-anchored regex. + // Negation at a command boundary: start of segment, or after ; & | ( newline / then / do. + + // FIX 4 (isRegionScoped tightened): return true ONLY when grep is downstream of a + // sed line-range or awk range producer. Other pipe sources (cat, tac, etc.) are file-wide. + const isRegionScoped = (seg: string): boolean => { + if (!seg.includes('|')) return false; + const before = seg.slice(0, seg.lastIndexOf('|')); + // sed -n line/range extraction, e.g. sed -n '12,40p' FILE or sed -n '/a/,/b/p' FILE + if (/\bsed\s+-n\b/.test(before)) return true; + // awk range pattern, e.g. awk '/start/,/end/' FILE + if (/\bawk\b[^|]*\/[^/]*\/\s*,\s*\/[^/]*\//.test(before)) return true; + return false; + }; + + // Parse all blocks. + interface TaskInfo { + name: string; + files: string[]; // entries from + gateText: string; // ++ text + reqText: string; // + text (requirement side) + } + const taskRe = /]*>([\s\S]*?)<\/task>/g; + const tasks: TaskInfo[] = []; + let tm: RegExpExecArray | null; + while ((tm = taskRe.exec(text)) !== null) { + const tc = tm[1]; + // Extract task name. + const namem = tc.match(/([\s\S]*?)<\/name>/); + const name = namem ? namem[1].trim() : 'unnamed'; + // Extract entries. + const filesm = tc.match(/([\s\S]*?)<\/files>/); + const filesText = filesm ? filesm[1] : ''; + const files = filesText.split(/[,\s]+/).map(s => s.trim()).filter(Boolean); + // Gate text: //. + const gateFragments: string[] = []; + for (const tag of ['verify', 'automated', 'acceptance_criteria']) { + const re = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + let mm: RegExpExecArray | null; + while ((mm = re.exec(tc)) !== null) gateFragments.push(mm[1]); + } + // Requirement text: /. + const reqFragments: string[] = []; + for (const tag of ['action', 'acceptance_criteria']) { + const re = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + let mm: RegExpExecArray | null; + while ((mm = re.exec(tc)) !== null) reqFragments.push(mm[1]); + } + // Strip XML tags from gate text so segments containing embedded + // XML closing tags (e.g. cmd nested inside ) + // don't bleed into the file-path token extraction. + const rawGateText = gateFragments.join('\n'); + const gateText = rawGateText.replace(/<[^>]+>/g, ' '); + tasks.push({ + name, + files, + gateText, + reqText: reqFragments.join('\n'), + }); + } + + if (tasks.length < 2) return { warnings, valid: true }; + + // FIX 3 (extensionless known files): build a normalized set of ALL tasks' entries + // so that extensionless filenames like Dockerfile are also recognized as valid file tokens. + const knownFiles = new Set(); + for (const t of tasks) { + for (const f of t.files) knownFiles.add(normPath(f)); + } + + // Extended looksLikePath: accepts known entries even without an extension. + const isFileLike = (token: string): boolean => { + if (token.includes('*')) return false; // exclude globs + if (looksLikePath(token)) return true; + return knownFiles.has(normPath(token)); + }; + + // Dedup key: (taskAIdx, taskBIdx, pat, file) + const seen = new Set(); + + // For each task A, scan gate text for file-wide negative grep bans. + 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*/), + ); + + for (const seg of segments) { + if (!/grep/.test(seg)) continue; + + // FIX 5: Negation at a command boundary: start of segment, or after ; & | ( newline / then / do. + // Also handles ! negating an entire pipeline (e.g. ! cat FILE | grep ...). + const hasLeadingNot = + // Direct ! grep: negation immediately before grep keyword + /(?:^|[\n;&|(]|\bthen\b|\bdo\b)\s*!\s*grep/.test(seg) || + // Pipeline negation: ! at command boundary, grep appears in pipeline after | + (/(?:^|[\n;&|(]|\bthen\b|\bdo\b)\s*!\s*\w/.test(seg) && /\|\s*grep\b/.test(seg)); + + const hasCountZero = zeroCmp(seg); + + // Extract grep invocation and check for count. + grepArgRe.lastIndex = 0; + let pat: string | null = null; + let file: string | null = null; + let isBan = false; + + // FIX 4 helper: given a segment and the grep match end position, find the + // file argument. First try the token immediately after PAT; if none qualifies, + // try a cat/tac producer or < FILE redirect from the full segment. + const resolveFileArg = (segment: string, afterPatStr: string): string | null => { + // Primary: token immediately after PAT in the grep command + const fileM = afterPatStr.match(/^\s+([^\s'"|>&;$()\[\]]+)/); + const rawFile = fileM ? fileM[1] : null; + if (rawFile && isFileLike(rawFile)) return rawFile; + // FIX 4: For NON-region segments, also look for cat/tac producer or < FILE redirect + const catM = segment.match(/\b(?:cat|tac)\s+([^\s'"|>&;()]+)/); + if (catM && isFileLike(catM[1])) return catM[1]; + const redirM = segment.match(/<\s*([^\s'"|>&;()]+)/); + if (redirM && isFileLike(redirM[1])) return redirM[1]; + return null; + }; + + // If leading !, it might be a count or a direct !grep + if (hasLeadingNot && !hasCountZero) { + // Direct ! grep PAT FILE form: grep opts PAT FILE + // Extract PAT and FILE from the grep invocation + grepArgRe.lastIndex = 0; + let gm: RegExpExecArray | null; + while ((gm = grepArgRe.exec(seg)) !== null) { + const opts = gm[1]; + if (optsHaveInvert(opts)) continue; // -v form: not a ban + // PAT + const rawPat = gm[2] !== undefined ? gm[2] : + gm[3] !== undefined ? gm[3] : + gm[4] !== undefined && plausibleBare(gm[4]) ? gm[4] : null; + if (!rawPat) continue; + // FILE: next non-option token after PAT (or cat/tac/redirect in segment) + const afterPat = seg.slice((gm.index || 0) + gm[0].length); + const rawFile = resolveFileArg(seg, afterPat); + if (rawFile) { + pat = rawPat; + file = rawFile; + isBan = true; + } + } + } + + if (!isBan && hasCountZero) { + // count grep form: grep -c PAT FILE == 0 or [ $(grep -c PAT FILE) -eq 0 ] + grepArgRe.lastIndex = 0; + let gm: RegExpExecArray | null; + while ((gm = grepArgRe.exec(seg)) !== null) { + const opts = gm[1]; + if (!optsHaveCount(opts) || optsHaveInvert(opts)) continue; + const rawPat = gm[2] !== undefined ? gm[2] : + gm[3] !== undefined ? gm[3] : + gm[4] !== undefined && plausibleBare(gm[4]) ? gm[4] : null; + if (!rawPat) continue; + const afterPat = seg.slice((gm.index || 0) + gm[0].length); + const rawFile = resolveFileArg(seg, afterPat); + if (rawFile) { + pat = rawPat; + file = rawFile; + isBan = true; + } + } + } + + if (!isBan || !pat || !file) continue; + if (allow.has(pat)) continue; + // Skip if region-scoped (grep downstream of a sed/awk pipe — region extracted) + if (isRegionScoped(seg)) continue; + + // For each other task B: check if B's includes FILE AND B's reqText contains PAT + for (let bi = 0; bi < tasks.length; bi++) { + if (bi === ai) continue; + const taskB = tasks[bi]; + + // FIX 2: Exact normalized match; basename fallback ONLY for unqualified gate files. + const gateFile = normPath(file); + const bMatchesFile = taskB.files.some((bf) => { + const nbf = normPath(bf); + if (nbf === gateFile) return true; + // basename fallback only when the gate file is an unqualified bare filename (no dir separator) + if (!gateFile.includes('/') && path.basename(nbf) === gateFile) return true; + return false; + }); + if (!bMatchesFile) continue; + + // FIX 1: Use linear-time patternRequiredIn instead of new RegExp (ReDoS-safe). + const bRequiresPat = patternRequiredIn(pat, taskB.reqText); + if (!bRequiresPat) continue; + + const dedupeKey = `${ai}:${bi}:${pat}:${file}`; + if (seen.has(dedupeKey)) continue; + seen.add(dedupeKey); + + warnings.push( + `Region-scope conflict (#968): task "${taskA.name}" negative-greps "${pat}" file-wide on ${file}, ` + + `but sibling task "${taskB.name}" requires it in the same file. ` + + `A file-wide ban is unsatisfiable when a sibling needs the construct elsewhere — ` + + `region-scope task "${taskA.name}"'s gate (sed -n/awk range then grep) or use an AST/test check. ` + + `See planner-antipatterns.md "Region-Scoped Negative Gates", or add ` + + ` if intentional.`, + ); + } + } + } + + // This detector is warn-only: it never sets valid=false. + return { warnings, valid: true as const }; +} + function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): void { if (!filePath) { error('file path required'); @@ -315,6 +619,9 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo errors.push(...echoScan.errors); warnings.push(...echoScan.warnings); + const conflictScan = scanFileWideNegativeGateConflict(content); + warnings.push(...conflictScan.warnings); + output( { valid: errors.length === 0, @@ -1935,6 +2242,7 @@ function cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void { export = { scanNegativeGrepCommentEcho, + scanFileWideNegativeGateConflict, cmdVerifySummary, cmdVerifyPlanStructure, cmdVerifyPhaseCompleteness, diff --git a/tests/agent-size-baseline.json b/tests/agent-size-baseline.json index f6ec64c07..e320d007c 100644 --- a/tests/agent-size-baseline.json +++ b/tests/agent-size-baseline.json @@ -23,7 +23,7 @@ "gsd-pattern-mapper.md": 12487, "gsd-phase-researcher.md": 40638, "gsd-plan-checker.md": 42003, - "gsd-planner.md": 48892, + "gsd-planner.md": 49216, "gsd-project-researcher.md": 22014, "gsd-research-synthesizer.md": 13653, "gsd-roadmapper.md": 21781, diff --git a/tests/enh-968-region-scoped-negative-grep.test.cjs b/tests/enh-968-region-scoped-negative-grep.test.cjs new file mode 100644 index 000000000..5e76a4650 --- /dev/null +++ b/tests/enh-968-region-scoped-negative-grep.test.cjs @@ -0,0 +1,823 @@ +// allow-test-rule: source-text-is-the-product #968 +// Enhancement #968: region-scoped negative gate detector + guidance docs. +// Tests the pure function scanFileWideNegativeGateConflict exported from +// verify.cjs, plus CLI integration and doc-contract assertions. + +'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 { spawnSync } = require('node:child_process'); +const os = require('node:os'); +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'); + +// Build paths to doc files +const PLANNER_MD = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); +const ANTIPATTERNS_MD = path.join(__dirname, '..', 'gsd-core', 'references', 'planner-antipatterns.md'); +// PLAN_MD_REF removed — was unused (doc-contract cases test PLANNER_MD and ANTIPATTERNS_MD only) + +// ─── Fixture helpers ─────────────────────────────────────────────────────────── + +/** + * Build a minimal two-task plan fixture. + * taskA: file=app/page.py, gateText=the verify/acceptance_criteria block, actionText=action block + * taskB: file=app/page.py (default) or otherFile, action text + * allowlistMarker: optional HTML comment to insert at the top + */ +function makeTwoTaskPlan({ + taskAFile = 'app/page.py', + taskAGate = '! grep -Eq \'await .*refresh\' app/page.py', + taskAAction = 'Refactor the factory to be synchronous.', + taskBFile = 'app/page.py', + taskBAction = 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + allowlistMarker = '', +} = {}) { + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + `files_modified: [${taskAFile}, ${taskBFile}]`, + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# Test Plan', + '', + ]; + + if (allowlistMarker) { + lines.push(allowlistMarker, ''); + } + + // Task A: the one with the negative gate + lines.push(''); + lines.push('Task A: factory refactor'); + lines.push(`${taskAFile}`); + lines.push(`${taskAAction}`); + lines.push(`${taskAGate}`); + lines.push('Factory is synchronous.'); + lines.push(''); + lines.push(''); + + // Task B: the sibling that requires the construct + lines.push(''); + lines.push('Task B: reindex handler'); + lines.push(`${taskBFile}`); + lines.push(`${taskBAction}`); + lines.push('npm test'); + lines.push('Handler is in place.'); + lines.push(''); + + return lines.join('\n'); +} + +/** + * Build a single-task plan (no sibling). + */ +function makeSingleTaskPlan({ + taskFile = 'app/page.py', + taskGate = '! grep -Eq \'await .*refresh\' app/page.py', + taskAction = 'Refactor the factory to be synchronous.', +} = {}) { + return [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + `files_modified: [${taskFile}]`, + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# Test Plan', + '', + '', + 'Task A: factory refactor', + `${taskFile}`, + `${taskAction}`, + `${taskGate}`, + 'Factory is synchronous.', + '', + ].join('\n'); +} + +// ─── Group 1: pure-function unit tests ──────────────────────────────────────── + +describe('scanFileWideNegativeGateConflict — pure unit tests', () => { + let scan; + + before(() => { + const verify = require(VERIFY_CJS); + scan = verify.scanFileWideNegativeGateConflict; + assert.ok(typeof scan === 'function', 'scanFileWideNegativeGateConflict must be exported'); + }); + + // Case 1: basic WARN path — Task A bans PAT file-wide, Task B requires it in same file + test('case 1 — file-wide ban + sibling requires → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok(Array.isArray(result.warnings), 'must return { warnings: [] }'); + assert.ok( + result.warnings.length >= 1, + `expected at least 1 warning, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok( + result.warnings[0].includes('Region-scope conflict (#968)'), + `warning must mention Region-scope conflict (#968), got: ${result.warnings[0]}`, + ); + assert.ok( + result.warnings[0].includes('await .*refresh'), + `warning must mention the PAT, got: ${result.warnings[0]}`, + ); + assert.ok( + result.warnings[0].includes('app/page.py'), + `warning must mention the file, got: ${result.warnings[0]}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 2: region-scoped via sed → NO warn + 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'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sed-piped grep is region-scoped — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 2b: region-scoped via awk → NO warn + test('case 2b — region-scoped via awk pipe → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `awk-piped grep is region-scoped — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 3: single task file-wide ban, no sibling → NO warn + test('case 3 — single task, no sibling → NO warn', () => { + const content = makeSingleTaskPlan({ + taskGate: "! grep -Eq 'await .*refresh' app/page.py", + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `single task (no sibling) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 4: sibling requires PAT but lists a different file → NO warn + test('case 4 — sibling lists different file → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'app/page.py', + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBFile: 'app/other.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sibling with different file must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 5: sibling lists same file but action lacks PAT → NO warn + test('case 5 — sibling lists same file but action lacks PAT → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls bridge.sync() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sibling with no PAT in action must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 6: positive grep (no !) + sibling → NO warn (positive requirement, not a ban) + test('case 6 — positive grep (no !) + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -q 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `positive grep must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 7: inverted grep -v with ! + sibling → NO warn + test('case 7 — inverted grep -vq with ! + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -vq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `inverted grep (-v) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 8: allowlist marker present → NO warn + test('case 8 — allowlist marker suppresses warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + allowlistMarker: '', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `allowlist marker must suppress warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 9: one task both bans and requires same PAT in same file (no second task) → NO warn + test('case 9 — one task bans and requires PAT (no sibling) → NO warn', () => { + const content = makeSingleTaskPlan({ + taskGate: "! grep -Eq 'await .*refresh' app/page.py", + taskAction: 'Refactor to avoid await refresh, but note that bridge.refresh() is used later.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `single task (no sibling B) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 10: count form `grep -c 'PAT' FILE == 0` + sibling → WARN + test('case 10 — count form (grep -c PAT FILE == 0) + sibling → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -c 'await .*refresh' app/page.py == 0", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `count form (grep -c ... == 0) must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 11: bracket form `[ $(grep -c PAT FILE) -eq 0 ]` + sibling → WARN + test('case 11 — bracket form ([ $(grep -c PAT FILE) -eq 0 ]) + sibling → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "[ $(grep -c 'await .*refresh' app/page.py) -eq 0 ]", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `bracket form must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 12: CRLF variant of case 1 → WARN + test('case 12 — CRLF line endings → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const crlfContent = content.split('\n').join('\r\n'); + const result = scan(crlfContent); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `CRLF content must still warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 13: backslash line-continuation variant → WARN + test('case 13 — backslash line continuation → WARN', () => { + // Build manually to control exact line continuation + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [app/page.py]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Task A: factory refactor', + 'app/page.py', + 'Refactor the factory to be synchronous.', + // Gate split across lines with backslash continuation + "! grep -Eq 'await .*refresh' \\\napp/page.py", + 'Factory is synchronous.', + '', + '', + '', + 'Task B: reindex handler', + 'app/page.py', + 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + 'npm test', + 'Handler is in place.', + '', + ].join('\n'); + const result = scan(lines); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `backslash continuation must still warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 14: mixed line with positive gate AND a negative gate, sibling → WARN (on the negative only) + test('case 14 — mixed positive+negative on one segment + sibling → WARN for negative', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -c 'X' app/page.py == 1 && ! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `mixed line with negative gate + sibling must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 15: glob file arg `app/*.py` → NO warn (unresolvable path) + test('case 15 — glob file arg → NO warn (unresolvable)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/*.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `glob file arg must not warn (unresolvable), got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 16: invalid-regex PAT literal fallback → WARN, no exception + test('case 16 — invalid-regex PAT → literal fallback, WARN, no exception', () => { + // "await (refresh" — unbalanced paren, invalid regex + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await (refresh' app/page.py", + taskBAction: 'The handler calls await (refresh on bridge to repopulate state.', + }); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }, 'scan must not throw on invalid regex PAT'); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `invalid-regex PAT with literal match must warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 17: ReDoS-ish PAT (catastrophic backtracking) → no hang, no false warn. + // The sibling action is 5000 'a's — classic ReDoS trigger if we call new RegExp('(a+)+$'). + // Proof-of-no-hang: the test runner's own timeout catches it; a hanging test fails here. + // No timing assertion (flaky) — the linear patternRequiredIn implementation is microsecond-fast. + test('case 17 — catastrophic ReDoS pattern is instant, no hang, no false warn', () => { + const longAs = 'a'.repeat(5000); + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq '(a+)+$' app/page.py", + taskBAction: `Reindex handler that processes ${longAs} records and calls bridge.refresh().`, + }); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }, 'scan must not throw on ReDoS-ish PAT'); + // The literal '(a+)+$' is not present in the action text as a substring → no warn. + // (If new RegExp were used, this test would hang before reaching this assertion.) + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `catastrophic PAT '(a+)+$' not literally in action — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(Array.isArray(result.warnings), 'valid result shape'); + }); + + // Case 23 (mutation-catching): cat producer = file-wide → WARN; sed producer = region-scoped → NO warn + test('case 23a — cat pipe: ! cat app/page.py | grep -Eq PAT + sibling → WARN (file-wide via cat)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! cat app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `cat-piped grep is file-wide — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(result.valid !== false, 'valid must remain true even when #968 warns'); + }); + + test('case 23b — sed pipe: ! sed -n "12,40p" app/page.py | grep -Eq PAT + sibling → NO warn (region-scoped)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sed-piped grep is region-scoped — must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 24: awk region → NO warn + test('case 24 — awk region pipe: ! awk \'/^def make_page/,/^def /\' app/page.py | grep -Eq PAT + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `awk-piped grep is region-scoped — must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 25: basename non-over-match — different dirs, same basename → NO warn + test('case 25 — basename non-over-match: different dirs same filename → NO warn', () => { + // Task A bans on apps/web/config.py; Task B lists apps/admin/config.py + // Same basename "config.py" but different dirs → must NOT warn + const content = makeTwoTaskPlan({ + taskAFile: 'apps/web/config.py', + taskAGate: "! grep -Eq 'await .*refresh' apps/web/config.py", + taskBFile: 'apps/admin/config.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `different dirs (apps/web/config.py vs apps/admin/config.py) — same basename but must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 26: extensionless known file (Dockerfile) recognized via knownFiles → WARN + test('case 26 — extensionless known file (Dockerfile) via knownFiles → WARN', () => { + // Task A has ! grep -Eq 'FROM scratch' Dockerfile + // Dockerfile has no extension, so looksLikePath would miss it — but knownFiles should catch it + // Task B lists Dockerfile in and action requires 'FROM scratch' + const content = makeTwoTaskPlan({ + taskAFile: 'Dockerfile', + taskAGate: "! grep -Eq 'FROM scratch' Dockerfile", + taskBFile: 'Dockerfile', + taskBAction: 'Update the image base: FROM scratch ensures minimal surface area.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `Dockerfile (extensionless, known via ) should be recognized — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(result.valid !== false, 'valid must remain true'); + }); + + // Case 27: wildcard semantic match — patternRequiredIn handles .* correctly + test('case 27 — wildcard semantic match: "await .*refresh" (gate) warns when action has "await bridge.refresh()"', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `patternRequiredIn must match "await .*refresh" against "await bridge.refresh()" — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 4b: same-file positive control — sibling lists the SAME banned file + requires PAT → WARN + // Paired with case 4: proves the no-warn in case 4 is due to the file mismatch, not a dead detector. + test('case 4b — same-file positive control: sibling lists same file → WARN (proves case 4 no-warn is file-mismatch)', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'app/page.py', + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBFile: 'app/page.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `same-file sibling must warn — proves case 4's no-warn is due to file mismatch, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 7b: non-inverted positive control — without -v the ban IS detected → WARN + // Paired with case 7: proves the -v skip is what suppresses case 7. + test('case 7b — non-inverted positive control: ! grep -q (no -v) + sibling → WARN (proves case 7 no-warn is -v skip)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -q 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `non-inverted ! grep -q must warn — proves the -v flag is what suppresses case 7, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 25b: basename-fallback positive — bare unqualified filename matches sibling's qualified path → WARN + // Paired with case 25: proves the bare-name basename fallback at src ~line 525 actually fires. + // Case 25 only proves qualified paths don't over-match; this proves the bare fallback does fire. + test('case 25b — basename-fallback positive: bare gate file matches sibling qualified path → WARN (proves basename fallback fires)', () => { + // Task A gate uses bare "config.py" (no directory prefix — unqualified). + // Task B lists "apps/admin/config.py" (qualified). basename("apps/admin/config.py") === "config.py". + // The basename fallback (line 525) should match → WARN. + const content = makeTwoTaskPlan({ + taskAFile: 'config.py', + taskAGate: "! grep -Eq 'await .*refresh' config.py", + taskBFile: 'apps/admin/config.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `bare gate file "config.py" must match sibling "apps/admin/config.py" via basename fallback — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 28: anchored pattern warns after ^ strip — proves anchor stripping works + // Gate: ! grep -Eq '^FROM scratch' Dockerfile + // Sibling B lists Dockerfile, action requires 'FROM scratch' (no anchor in prose). + // Without anchor stripping, "^FROM scratch" would be treated as containing metacharacters + // and fall back to literal-substring: "^FROM scratch" not in B's prose → no warn. + // With anchor stripping, "FROM scratch" is the effective literal → found in B's prose → WARN. + test('case 28 — anchored pattern warns: ! grep -Eq \'^FROM scratch\' Dockerfile + sibling → WARN (proves ^ strip)', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'Dockerfile', + taskAGate: "! grep -Eq '^FROM scratch' Dockerfile", + taskBFile: 'Dockerfile', + taskBAction: 'Update the image base: FROM scratch ensures minimal surface area.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `anchored pattern "^FROM scratch" must warn after ^ is stripped — "FROM scratch" is in sibling action, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 29: alternation falls back conservatively — documents the known limitation. + // Gate: ! grep -Eq 'debug|trace' src/logger.ts + // Sibling B lists src/logger.ts, action says "remove debug calls" (contains "debug" but NOT "debug|trace"). + // patternRequiredIn sees unhandled `|` in joined frags → literal-substring fallback on raw pattern. + // "debug|trace" is NOT literally in B's prose → conservative NO warn. + // This is intentional: false-negative is the safe direction for a warn-only advisory. + test('case 29 — alternation conservative fallback: "debug|trace" → NO warn (documents alternation limitation)', () => { + // NOTE: This is intended conservative behavior, not a bug. + // patternRequiredIn falls back to literal-substring for patterns containing `|` (alternation), + // because safely expanding alternation without new RegExp would require a mini-parser. + // The literal "debug|trace" is not present verbatim in the action, so no warn fires. + // A planner who writes `debug|trace` gets no advisory — acceptable, since a false-negative + // is always safer than a false-positive for a warn-only gate. + const content = makeTwoTaskPlan({ + taskAFile: 'src/logger.ts', + taskAGate: "! grep -Eq 'debug|trace' src/logger.ts", + taskBFile: 'src/logger.ts', + taskBAction: 'Remove debug calls from the logger module to reduce noise.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `alternation pattern "debug|trace" must conservatively NOT warn — literal "debug|trace" not in action, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 18: empty content → no crash, no #968 warn + test('case 18 — empty content → no crash', () => { + let result; + assert.doesNotThrow(() => { + result = scan(''); + }); + assert.ok(Array.isArray(result.warnings), 'must return { warnings: [] }'); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + 'empty content must produce no #968 warn', + ); + }); + + // Case 18b: no-task plan → no crash + test('case 18b — no-task plan → no crash', () => { + const content = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: []', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# No tasks here', + ].join('\n'); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }); + assert.strictEqual(result.warnings.filter(w => w.includes('#968')).length, 0); + }); +}); + +// ─── Group 2: end-to-end via runGsdTools ────────────────────────────────────── + +describe('scanFileWideNegativeGateConflict — end-to-end via verify plan-structure', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // Case 19: integration — valid stays true despite warning (warn-only) + test('case 19 — integration: valid===true despite #968 warning', () => { + const planContent = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + 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 parsed = JSON.parse(result.output); + assert.strictEqual( + parsed.valid, + true, + `#968 is warn-only: valid must be true, got: ${JSON.stringify(parsed)}`, + ); + assert.ok( + parsed.warnings.some(w => w.includes('#968')), + `must have a #968 warning, got: ${JSON.stringify(parsed.warnings)}`, + ); + }); +}); + +// ─── Group 3: doc-contract ──────────────────────────────────────────────────── + +describe('doc-contract: guidance prose is in place', () => { + // Case 20: gsd-planner.md has the new guidance + test('case 20 — gsd-planner.md contains Region-scoped negative gates + reference', () => { + const content = fs.readFileSync(PLANNER_MD, 'utf8'); + assert.ok( + content.includes('Region-scoped negative gates'), + 'gsd-planner.md must include "Region-scoped negative gates"', + ); + assert.ok( + content.includes('planner-antipatterns.md'), + 'gsd-planner.md must reference planner-antipatterns.md', + ); + }); + + // Case 21: planner-antipatterns.md has the new section + test('case 21 — planner-antipatterns.md has ## Region-Scoped Negative Gates + examples', () => { + const content = fs.readFileSync(ANTIPATTERNS_MD, 'utf8'); + assert.ok( + content.includes('## Region-Scoped Negative Gates'), + 'planner-antipatterns.md must include "## Region-Scoped Negative Gates"', + ); + assert.ok( + content.includes('await .*refresh'), + 'planner-antipatterns.md must include the worked example pattern "await .*refresh"', + ); + // Verify sed or awk region example is present + const hasSedOrAwk = content.includes('sed -n') || content.includes('awk '); + assert.ok( + hasSedOrAwk, + 'planner-antipatterns.md must include sed-n or awk region example', + ); + }); +}); + +// ─── Group 4: AC3 executable proof ─────────────────────────────────────────── + +describe('AC3: executable proof — file-wide ban vs region-scoped simultaneously satisfiable', () => { + test('case 22 — grep/sed proof: both gates simultaneously satisfiable', () => { + // Check if grep and sed are available + const grepAvail = spawnSync('grep', ['--version']).status === 0; + const sedAvail = spawnSync('sed', ['--version']).status === 0 || + spawnSync('sed', ['-n', '1p', '/dev/null']).status === 0; + + if (!grepAvail || !sedAvail) { + // Skip gracefully if tools are unavailable + return; + } + + // Write a temp Python file with: + // def make_page(): — no await refresh + // async def reindex_handler(): — awaits bridge.refresh() + const tmpFile = path.join(os.tmpdir(), `gsd-968-proof-${process.pid}.py`); + const pyContent = [ + 'def make_page():', + ' """Synchronous factory — must not block on a refresh."""', + ' return {"title": "My Page"}', + '', + '', + 'async def reindex_handler():', + ' """Post-reindex callback — must await bridge.refresh() to repopulate state."""', + ' await bridge.refresh()', + ' return True', + ].join('\n'); + fs.writeFileSync(tmpFile, pyContent); + + try { + // (a) File-wide: grep -Eq 'await .*refresh' — should EXIT 0 (pattern found) + // This means a file-wide ban (! grep -Eq ...) WOULD FAIL + const fileWide = spawnSync('grep', ['-Eq', 'await .*refresh', tmpFile]); + assert.strictEqual( + fileWide.status, + 0, + 'grep file-wide should find the pattern (exits 0) — proving the file-wide ban would fail', + ); + + // (b) Region-scoped (make_page only): sed extracts lines 1-3, piped to grep → pattern NOT found + // The factory region is clean: ban PASSES + const makePageLines = spawnSync('sed', ['-n', '1,3p', tmpFile]); + assert.strictEqual(makePageLines.status, 0, 'sed should succeed'); + const makePageRegion = makePageLines.stdout.toString(); + + // Write to a temp file and grep it + const regionFile = path.join(os.tmpdir(), `gsd-968-region-${process.pid}.py`); + fs.writeFileSync(regionFile, makePageRegion); + try { + const regionBan = spawnSync('grep', ['-Eq', 'await .*refresh', regionFile]); + assert.strictEqual( + regionBan.status, + 1, + 'grep in make_page region should NOT find pattern (exits 1) — ban PASSES in factory region', + ); + + // (c) Region-scoped (reindex_handler): grep should FIND the pattern → requirement met + const reindexLines = spawnSync('sed', ['-n', '6,9p', tmpFile]); + const reindexRegion = reindexLines.stdout.toString(); + const reindexFile = path.join(os.tmpdir(), `gsd-968-reindex-${process.pid}.py`); + fs.writeFileSync(reindexFile, reindexRegion); + try { + const reindexCheck = spawnSync('grep', ['-Eq', 'await .*refresh', reindexFile]); + assert.strictEqual( + reindexCheck.status, + 0, + 'grep in reindex_handler region MUST find pattern (exits 0) — requirement met', + ); + } finally { + try { fs.unlinkSync(reindexFile); } catch { /* ignore */ } + } + } finally { + try { fs.unlinkSync(regionFile); } catch { /* ignore */ } + } + } finally { + try { fs.unlinkSync(tmpFile); } catch { /* ignore */ } + } + }); +});