enhance(#3464): exec() detection widening, citation-debt cleanup — Phase 8 (#4171)

* feat(#3464): widen no-source-grep to detect regex.exec() on tracked text Adds an execCall kind alongside the existing regexTest detection -- regex.exec(tracked) was invisible to the rule while regex.test(tracked) was already caught, despite both reading a source-derived string through a regex. Measured: 4 previously-invisible sites across 2 files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(#3464): migrate 4 sites newly flagged by the exec() widening docs-hooks-table-parity.test.cjs's three regex-extraction loops are site-scoped marked (source-text-is-the-product) -- the dynamic preToolEvent/postToolEvent dialect branching they mirror is explicitly documented as not statically parseable, so a literal-pattern mirror is the practical minimum-cost check. no-bare-gsd-tools-command-position.test.cjs's readRouterVerbs() now requires HOST_COMMAND_ROUTERS directly instead of regex-walking gsd-tools.cjs's source text -- the same accessor three other suites already use. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3464): pay down 6 grandfathered uncited allow-test-rule markers Two were genuinely load-bearing (suppressing a real detected violation) and just needed a citation added -- phase6-capstone-conformance.test.cjs, runtime-name-policy.test.cjs, both now (#3464). Four were dead-weight file-header markers suppressing nothing -- each file's real effective sites are covered by separate, already-cited markers elsewhere in the same file. Deleted outright rather than cited, per Phase 1's own precedent (remove non-load-bearing markers instead of grandfathering them forever) -- codex-config.test.cjs (two copies), gsd-check-update-worker-platform-gate.test.cjs, orphaned-hooks.test.cjs, settings-jsonc.test.cjs. allowlist.json: 134 -> 128 entries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore(#3464): re-baseline effective-exemption ceiling to 84 The exec() widening's 3 newly-marked sites are now suppressed and counted; ceiling rises 81 -> 84, the exact measured high-water mark. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#3464): correct citation and restore a wrongly-deleted marker Two review corrections, both found by the orthogonal review pass: - docs-hooks-table-parity.test.cjs's 3 new exec() markers cited #3464 (mechanically "the phase that widened the rule") when the file's own established, correct reference is #3839 (the issue this whole test exists to enforce, already cited in its file header) -- fixed to match. - gsd-check-update-worker-platform-gate.test.cjs's deleted file-header marker was NOT dead weight: its codeOnly() helper wraps readFileSync and is called inline as an assert argument, a genuine source-grep pattern on real .cjs/.js source that the rule cannot currently see (helper-function indirection is a distinct blind spot from anything Phase 7/8 measured) -- CONTRIBUTING.md is explicit that "unverified" is not the same as "vestigial." Restored, site-scoped this time (directly above codeOnly(), not as an inert file-header comment) and cited (#3103, the issue the file's own docstring already references). codex-config.test.cjs's two deletions and orphaned-hooks.test.cjs's / settings-jsonc.test.cjs's deletions were independently re-verified and stand: their flagged lines read generated .toml/.json OUTPUT, not source, or have no residual pattern at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-02 08:11:23 -04:00
parent 9b77320580
commit 2131fe13f3
12 changed files with 128 additions and 50 deletions
--- a/eslint-rules/no-source-grep.cjs
+++ b/eslint-rules/no-source-grep.cjs
@@ -6,7 +6,8 @@
 * Flags variables bound to readFileSync() of a .cjs/.cts/.js/.mjs/.mts/.ts
 * source path that later have a text-search method called on them, whether
 * directly, via a bounded chain of derived bindings (`const b = f(a)`,
- * `b = a`, ...), or via `regex.test(tracked)` / `/lit/.test(tracked)`.
+ * `b = a`, ...), or via `regex.test(tracked)` / `/lit/.test(tracked)` /
+ * `regex.exec(tracked)` / `/lit/.exec(tracked)` (#3464 phase 8).
 *
 * Variable identity is resolved through real lexical scope (ESLint
 * `Variable` objects via `sourceCode.scopeManager`/`getScope`), not by name
@@ -235,7 +236,7 @@ const rule = {
    ],
    messages: {
      noSourceGrep:
-        'Source-grep test: do not read source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and call .includes/.match/.matchAll/.startsWith/.indexOf/.split/.replace/.search (or regex.test()) on the result. Use require() to run the module instead. Add // allow-test-rule: <reason> (#NNN) directly above (or trailing) the flagged line to suppress just that site.',
+        'Source-grep test: do not read source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and call .includes/.match/.matchAll/.startsWith/.indexOf/.split/.replace/.search (or regex.test() / regex.exec()) on the result. Use require() to run the module instead. Add // allow-test-rule: <reason> (#NNN) directly above (or trailing) the flagged line to suppress just that site.',
      // Diagnostic-only companion to `noSourceGrep`, emitted ONLY when the
      // `neutralizeSuppression` schema option is set (see its doc comment
      // and `reportUnlessSuppressed` above) -- never fires with the real
@@ -600,6 +601,8 @@ const rule = {
          pendingCalls.push({ node, kind: 'textMethod' });
        } else if (propName === 'test') {
          pendingCalls.push({ node, kind: 'regexTest' });
+        } else if (propName === 'exec') {
+          pendingCalls.push({ node, kind: 'execCall' });
        }
      },
      'Program:exit'() {
@@ -711,8 +714,13 @@ const rule = {
            continue;
          }

-          // kind === 'regexTest': re.test(tracked) or /lit/.test(tracked).
-          // The tracked variable is the ARGUMENT here, not the callee object.
+          // kind === 'regexTest' / 'execCall': re.test(tracked) or
+          // /lit/.test(tracked), and identically re.exec(tracked) or
+          // /lit/.exec(tracked) (#3464 phase 8) -- both return a
+          // regex-shaped result, but what matters here is only that the
+          // ARGUMENT (not the callee object) may carry the tracked source
+          // text, so the receiver/argument classification is shared
+          // byte-for-byte between the two kinds.
          const looksLikeRegexReceiver =
            obj.type === 'Identifier' || (obj.type === 'Literal' && !!obj.regex);
          if (!looksLikeRegexReceiver) continue;
--- a/scripts/lint-allow-test-rule-refs.allowlist.json
+++ b/scripts/lint-allow-test-rule-refs.allowlist.json
@@ -24,7 +24,6 @@
  "tests/code-review-pipeline-regression.test.cjs :: source-text-is-the-product",
  "tests/code-review.test.cjs :: source-text-is-the-product",
  "tests/codebuddy-install.test.cjs :: source-text-is-the-product",
-  "tests/codex-config.test.cjs :: source-text-is-the-product",
  "tests/command-contract.test.cjs :: source-text-is-the-product",
  "tests/commands.test.cjs :: source-text-is-the-product",
  "tests/config-field-docs.test.cjs :: docs-parity",
@@ -52,7 +51,6 @@
  "tests/frontmatter-cli.test.cjs :: source-text-is-the-product",
  "tests/gates-taxonomy.test.cjs :: source-text-is-the-product",
  "tests/git-base-branch.test.cjs :: source-text-is-the-product",
-  "tests/gsd-check-update-worker-platform-gate.test.cjs :: structural-regression-guard",
  "tests/gsd-researcher-app-aware.test.cjs :: source-text-is-the-product",
  "tests/gsd-researcher-flow-diagram.test.cjs :: source-text-is-the-product",
  "tests/gsd-settings-advanced.test.cjs :: source-text-is-the-product",
@@ -74,13 +72,11 @@
  "tests/next-safety-gates.test.cjs :: source-text-is-the-product",
  "tests/next-up-clear-order.test.cjs :: source-text-is-the-product",
  "tests/no-hardcoded-home-gsd-tools.test.cjs :: source-text-is-the-product",
-  "tests/orphaned-hooks.test.cjs :: structural-regression-guard",
  "tests/package-legitimacy-gate.test.cjs :: source-text-is-the-product",
  "tests/parallel-dependent-plans.test.cjs :: source-text-is-the-product",
  "tests/path-replacement.test.cjs :: source-text-is-the-product",
  "tests/phase.test.cjs :: source-text-is-the-product",
  "tests/phase6-capability-docs.test.cjs :: source-text-is-the-product",
-  "tests/phase6-capstone-conformance.test.cjs :: source-text-is-the-product",
  "tests/phase6-planning-capabilities.test.cjs :: source-text-is-the-product",
  "tests/plan-bounce.test.cjs :: source-text-is-the-product",
  "tests/plan-phase-drift-guard.test.cjs :: source-text-is-the-product",
@@ -105,7 +101,6 @@
  "tests/research-agent-profiles.test.cjs :: source-text-is-the-product",
  "tests/roadmap.test.cjs :: source-text-is-the-product",
  "tests/runtime-launcher-parity.test.cjs :: structural-regression-guard",
-  "tests/runtime-name-policy.test.cjs :: source-text-is-the-product",
  "tests/scan-command.test.cjs :: source-text-is-the-product",
  "tests/secret-scan-lint.security.test.cjs :: source-text-is-the-product",
  "tests/secure-phase.test.cjs :: source-text-is-the-product",
@@ -113,7 +108,6 @@
  "tests/security-scan.security.test.cjs :: source-text-is-the-product",
  "tests/seed-scan-new-milestone.test.cjs :: source-text-is-the-product",
  "tests/settings-integrations.test.cjs :: source-text-is-the-product",
-  "tests/settings-jsonc.test.cjs :: structural-regression-guard",
  "tests/skill-frontmatter-contract.test.cjs :: source-text-is-the-product",
  "tests/spawn-liveness-banner.test.cjs :: source-text-is-the-product",
  "tests/state.test.cjs :: source-text-is-the-product",
--- a/scripts/lint-allow-test-rule-refs.effective-ceiling.json
+++ b/scripts/lint-allow-test-rule-refs.effective-ceiling.json
@@ -1,4 +1,4 @@
 {
-  "maxSites": 81,
+  "maxSites": 84,
  "grace": 2
 }
--- a/tests/codex-config.test.cjs
+++ b/tests/codex-config.test.cjs
@@ -1,8 +1,3 @@
-// allow-test-rule: source-text-is-the-product
-// Workflow .md / agent .md / command .md / reference .md files — their text
-// IS what the runtime loads. Testing text content tests the deployed contract.
-// Per CONTRIBUTING.md exception matrix.
-
 /**
 * GSD Tools Tests - codex-config.cjs
 *
@@ -3355,11 +3350,6 @@ test('writeNonClaudeDefaults function exists and is a no-op for Claude (#2834)',
 {
  const { describe: __foldDescribe } = require('node:test');
  __foldDescribe("folded:bug-2639-codex-toml-neutralization (consolidation epic #1969 H3 W4 #3336)", () => {
-// allow-test-rule: source-text-is-the-product
-// Workflow .md / agent .md / command .md / reference .md files — their text
-// IS what the runtime loads. Testing text content tests the deployed contract.
-// Per CONTRIBUTING.md exception matrix.
-
 /**
 * Regression: issue #2639 — Codex install generated agent TOMLs with stale
 * Claude-specific references (CLAUDE.md, .claude/skills/, .claudeignore).
--- a/tests/docs-hooks-table-parity.test.cjs
+++ b/tests/docs-hooks-table-parity.test.cjs
@@ -91,16 +91,19 @@ function registeredHookEvents() {
  let m;
  // Literal hook-spec array (the Kimi mirror of the settings.json wiring).
  const specRe = /event:\s*'([A-Za-z]+)',\s*command:\s*cmd\('([^']+)'\)/g;
+  // allow-test-rule: source-text-is-the-product (#3839)
  while ((m = specRe.exec(src)) !== null) add(path.basename(m[2]), m[1]);
  // Probe lines paired with a literal event:
  // settings.hooks.<Event>.some(… referencesHook(…, '<name>'))
  const probeRe = /settings\.hooks\.([A-Za-z]+)\.some\(\(entry: HookGroup\) =>\s*\n\s*entry\.hooks && entry\.hooks\.some\(\(h: HookEntry\) => referencesHook\(h as Record<string, unknown>, '([^']+)'\)/g;
+  // allow-test-rule: source-text-is-the-product (#3839)
  while ((m = probeRe.exec(src)) !== null) add(m[2], m[1]);
  // Probe lines paired with the runtime-resolved variables — statically
  // resolved to their non-Gemini canonical events (docs document the
  // canonical Claude/GS wiring; BeforeTool/AfterTool are the Gemini twins):
  // const preToolEvent = hookEvents === 'gemini' ? 'BeforeTool' : 'PreToolUse'
  const dynRe = /settings\.hooks\[(preToolEvent|postToolEvent)\]\.some\(\(entry: HookGroup\) =>\s*\n\s*entry\.hooks && entry\.hooks\.some\(\(h: HookEntry\) => referencesHook\(h as Record<string, unknown>, '([^']+)'\)/g;
+  // allow-test-rule: source-text-is-the-product (#3839)
  while ((m = dynRe.exec(src)) !== null) add(m[2], m[1] === 'preToolEvent' ? 'PreToolUse' : 'PostToolUse');
  return map;
 }
--- a/tests/eslint-rules.test.cjs
+++ b/tests/eslint-rules.test.cjs
@@ -696,6 +696,97 @@ describe('no-source-grep rule — widening (#3502)', () => {
      invalid: [],
    });
  });
+
+  // One RuleTester case per row of the widen-regex.exec()-detection matrix,
+  // epic #3464 phase 8: `regex.exec(tracked)` must be flagged the same way
+  // `regex.test(tracked)` already is, sharing the identical
+  // looksLikeRegexReceiver / trackedInfo(args[0]) detection path.
+
+  test('#3464p8 row 1: re.exec(trackedSrc) — flagged (new .exec() detection)', () => {
+    ruleTester.run('no-source-grep', noSourceGrep, {
+      valid: [],
+      invalid: [
+        {
+          code: `
+            const fs = require('fs');
+            const path = require('path');
+            const re = /foo/;
+            const trackedSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'x.cjs'), 'utf8');
+            re.exec(trackedSrc);
+          `,
+          filename: 'tests/foo.test.cjs',
+          errors: [{ messageId: 'noSourceGrep' }],
+        },
+      ],
+    });
+  });
+
+  test('#3464p8 row 2: re.test(trackedSrc) — still flagged (unchanged baseline)', () => {
+    ruleTester.run('no-source-grep', noSourceGrep, {
+      valid: [],
+      invalid: [
+        {
+          code: `
+            const fs = require('fs');
+            const path = require('path');
+            const re = /foo/;
+            const trackedSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'x.cjs'), 'utf8');
+            re.test(trackedSrc);
+          `,
+          filename: 'tests/foo.test.cjs',
+          errors: [{ messageId: 'noSourceGrep' }],
+        },
+      ],
+    });
+  });
+
+  test('#3464p8 row 3: re.exec(untrackedString) — not flagged (argument is not source-derived)', () => {
+    ruleTester.run('no-source-grep', noSourceGrep, {
+      valid: [
+        {
+          code: `
+            const re = /foo/;
+            const untrackedString = 'hello';
+            re.exec(untrackedString);
+          `,
+          filename: 'tests/foo.test.cjs',
+        },
+      ],
+      invalid: [],
+    });
+  });
+
+  test('#3464p8 row 4: someObj.exec(trackedSrc) — not flagged (receiver is not a bare Identifier/regex literal)', () => {
+    ruleTester.run('no-source-grep', noSourceGrep, {
+      valid: [
+        {
+          code: `
+            const fs = require('fs');
+            const path = require('path');
+            const trackedSrc = fs.readFileSync(path.join(__dirname, '..', 'src', 'x.cjs'), 'utf8');
+            getRegex().exec(trackedSrc);
+          `,
+          filename: 'tests/foo.test.cjs',
+        },
+      ],
+      invalid: [],
+    });
+  });
+
+  test('#3464p8 row 5: re.exec() with zero arguments — not flagged, does not throw', () => {
+    ruleTester.run('no-source-grep', noSourceGrep, {
+      valid: [
+        {
+          code: `
+            const re = /foo/;
+            re.exec();
+          `,
+          filename: 'tests/foo.test.cjs',
+        },
+      ],
+      invalid: [],
+    });
+  });
 });

 // ─── no-source-grep site-scoped suppression (#3508 / Phase 4 of #3464) ──────
--- a/tests/gsd-check-update-worker-platform-gate.test.cjs
+++ b/tests/gsd-check-update-worker-platform-gate.test.cjs
@@ -22,11 +22,6 @@
 * is the minimum-cost contract.
 */

-// allow-test-rule: structural-regression-guard
-// structural assertion on spawn-options shape; the behavior
-// (Windows-only shell resolution) is platform-gated at runtime and cannot be
-// reached on POSIX CI without a Windows lane.
-
 'use strict';

 const { test, describe } = require('node:test');
@@ -40,6 +35,12 @@ const PROJECTION_PATH = path.join(
  __dirname, '..', 'gsd-core', 'bin', 'lib', 'shell-command-projection.cjs',
 );

+// allow-test-rule: structural-regression-guard (#3103)
+// This helper feeds real source (via readFileSync) into structural
+// assertions below. The behavior it guards — Windows-only shell
+// resolution — is platform-gated at runtime and cannot be reached on
+// POSIX CI without a Windows lane, so a structural assertion on the
+// spawn-options shape is the minimum-cost contract.
 function codeOnly(file) {
  return fs.readFileSync(file, 'utf8')
    // eslint-disable-next-line local/no-unbounded-quantifier -- parses this repo's own bounded hooks/lib source, not adversarial input
--- a/tests/no-bare-gsd-tools-command-position.test.cjs
+++ b/tests/no-bare-gsd-tools-command-position.test.cjs
@@ -61,26 +61,26 @@ const SCAN_DIRS = [
 ];

 // Derive the verb set the bare-call guard matches against. Most top-level
-// verbs live in the host-command router table as `'verb': routeHandler` entries
-// (~70); this reads those dynamically so new router verbs are covered the moment
-// they land. A handful of verbs are dispatched as FAMILIES (their own
-// `command === 'verb'` arm, not a route-table entry): `query` (line ~2876),
-// `intel`, `verify`, and `graphify`. These are stable, documented families, so
-// they are supplemented explicitly here rather than parsed from the help string
-// (whose prose mixes real verbs with English words like "for"/"output"/"working",
-// producing noise). If a family verb is ever promoted into the route table the
-// union dedupes harmlessly; if a NEW family verb is added it must be added here.
+// verbs live in the host-command router table (`HOST_COMMAND_ROUTERS`, ~70
+// entries, exported by gsd-tools.cjs for exactly this kind of test); this
+// reads that real exported object directly so new router verbs are covered
+// the moment they land. A handful of verbs are dispatched as FAMILIES (their
+// own `command === 'verb'` arm, not a route-table entry): `query` (line
+// ~2876), `intel`, `verify`, and `graphify`. These are stable, documented
+// families, so they are supplemented explicitly here rather than parsed from
+// the help string (whose prose mixes real verbs with English words like
+// "for"/"output"/"working", producing noise). If a family verb is ever
+// promoted into the route table the union dedupes harmlessly; if a NEW
+// family verb is added it must be added here.
 //
 // Sorted longest-first so a hyphenated verb (`verify-summary`) is preferred over
 // its prefix (`verify`) — the exact ordering bug that let `verify-summary` slip
 // past a fixed 6-verb list during the first #2751 pass.
 const FAMILY_DISPATCHED_VERBS = ['query', 'intel', 'verify', 'graphify'];
 function readRouterVerbs() {
-  const src = fs.readFileSync(ROUTER_PATH, 'utf8');
-  const re = /(?:'([a-z][a-z-]*)'|([a-z][a-z-]*))\s*:\s*route[A-Z]\w*/g;
+  const { HOST_COMMAND_ROUTERS } = require(ROUTER_PATH);
  const verbs = new Set(FAMILY_DISPATCHED_VERBS);
-  let m;
-  while ((m = re.exec(src)) !== null) verbs.add(m[1] || m[2]);
+  for (const verb of Object.keys(HOST_COMMAND_ROUTERS)) verbs.add(verb);
  return [...verbs].sort((a, b) => b.length - a.length);
 }

--- a/tests/orphaned-hooks.test.cjs
+++ b/tests/orphaned-hooks.test.cjs
@@ -1,7 +1,3 @@
-// allow-test-rule: structural-regression-guard
-// Reads hook .js or bin/install.js source to assert structural invariants
-// (search array order, function wiring, path constants) that cannot be
-// verified by observing runtime outputs alone. Per CONTRIBUTING.md exception matrix.
 /**
 * Regression test for #1750: orphaned hook files from removed features
 * (e.g., gsd-intel-*.js) should NOT be flagged as stale by gsd-check-update.js.
--- a/tests/phase6-capstone-conformance.test.cjs
+++ b/tests/phase6-capstone-conformance.test.cjs
@@ -247,7 +247,7 @@ describe('ADR-857 phase 6 — capabilities must not bake install paths into the

  test('generated capability-registry.cjs contains no ~/.claude install path', () => {
    const reg = fs.readFileSync(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'capability-registry.cjs'), 'utf8');
-    // allow-test-rule: source-text-is-the-product
+    // allow-test-rule: source-text-is-the-product (#3464)
    const leakLines = reg.split(/\r?\n/).map((l, i) => [i + 1, l]).filter(([, l]) => LEAK.test(l)).map(([n]) => n);
    assert.deepEqual(leakLines, [],
      `capability-registry.cjs leaks ~/.claude install paths at line(s) ${leakLines.join(', ')} — the registry is copied verbatim to non-Claude runtimes (only workflow .md files are path-converted at install). Make the source capability fragment path-free.`);
--- a/tests/runtime-name-policy.test.cjs
+++ b/tests/runtime-name-policy.test.cjs
@@ -55,7 +55,7 @@ describe('runtime-name-policy windsurf alias parity — manifest vs FALLBACK_ALI
  test('manifest and FALLBACK_ALIASES windsurf alias sets are identical', () => {
    // Read FALLBACK_ALIASES from source to detect manual drift before a build.
    const srcPath = path.join(ROOT, 'src', 'runtime-name-policy.cts');
-    // allow-test-rule: source-text-is-the-product
+    // allow-test-rule: source-text-is-the-product (#3464)
    // FALLBACK_ALIASES source text IS the product contract for runtimes that can't load the manifest at runtime; verifying
    // both surfaces contain the same windsurf aliases catches manual-mirror drift.
    const src = fs.readFileSync(srcPath, 'utf8');
--- a/tests/settings-jsonc.test.cjs
+++ b/tests/settings-jsonc.test.cjs
@@ -1,8 +1,3 @@
-// allow-test-rule: structural-regression-guard
-// Reads hook .js or bin/install.js source to assert structural invariants
-// (search array order, function wiring, path constants) that cannot be
-// verified by observing runtime outputs alone. Per CONTRIBUTING.md exception matrix.
-
 /**
 * GSD Tools Tests - settings.json JSONC (JSON with comments) support
 *