fix(ci): make the two rg-based static checks actually run

.fallowrc.json

@@ -39,6 +39,7 @@
     "examples/test-app/**",
     "scripts/perf/**",
     "scripts/layering/**",
+    "scripts/di-seams/**",
     "scripts/maestro-conformance/corpus/**",
     "apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests.xctestplan",
     "scripts/write-xcuitest-cache-metadata.mjs",

.github/workflows/ci.yml

@@ -28,8 +28,9 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  # Text-only assertions run before the toolchain setup, so a grep failure does
-  # not wait on an install.
+  # The Swift trailing-comma assertion is text-only and runs before the toolchain setup, so a
+  # grep failure does not wait on an install. The DI-seams check below needs a real TypeScript
+  # runtime (#1976 / PR #2006), so it runs after Setup toolchain instead.
   lint:
     name: Lint & Format
     runs-on: ubuntu-latest
@@ -38,23 +39,50 @@ jobs:
       - name: Checkout
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
 
+      # #1976: ripgrep is never installed on ubuntu-latest, so `rg` failed with "command not
+      # found" (exit 127) on every run, and `if rg ...; then ... fi` cannot distinguish that
+      # from "no matches" (exit 1) — both read as false, so the step passed without the
+      # assertion ever executing. Rewritten against `grep`, which every runner ships, with the
+      # match/no-match/error exit codes handled explicitly so a broken scan fails loudly instead
+      # of silently passing.
       - name: Disallow trailing commas before closing parenthesis in Swift
         run: |
-          if rg -nU --glob '*.swift' ',\s*\n\s*\)' apple/runner; then
-            echo "Found trailing commas before ')' in Swift files. This syntax requires Swift 6.1+ and breaks older Xcode toolchains."
+          mapfile -d '' -t swift_files < <(git ls-files -z -- 'apple/runner' | grep -z '\.swift$')
+          if [ "${#swift_files[@]}" -eq 0 ]; then
+            echo "No apple/runner/*.swift files are tracked; the trailing-comma check has nothing to scan." >&2
             exit 1
           fi
-
-      - name: Fail if test-only DI seams reappear in production code
-        run: |
-          if rg '\?\s*:\s*typeof\s+' src/ --glob '!**/__tests__/**' --glob '!*.test.ts'; then
-            echo "Found test-only DI seams (optional typeof params) in production code."
+          set +e
+          grep -PzoH ',\s*\n\s*\)' "${swift_files[@]}"
+          status=$?
+          set -e
+          if [ "$status" -eq 0 ]; then
+            echo "Found trailing commas before ')' in Swift files. This syntax requires Swift 6.1+ and breaks older Xcode toolchains."
+            exit 1
+          elif [ "$status" -ne 1 ]; then
+            echo "grep exited $status while scanning apple/runner for trailing commas; treating an unreadable scan as a failure instead of a silent pass."
             exit 1
           fi
 
       - name: Setup toolchain
         uses: ./.github/actions/setup-node-pnpm
 
+      # Same false-green shape as the Swift check above (#1976). An earlier revision of this
+      # gate (PR #2006, first review pass) fixed the exit-code handling but kept the ban/allow
+      # decision as a regex that exempted matches by the *spelling* of the typeof target
+      # (`typeof fetch` always passed, SCREAMING_SNAKE_CASE targets always passed) — a name-based
+      # semantic allowlist that would silently pass a new, genuinely test-only `typeof fetch` seam
+      # anywhere in the tree while banning an equally legitimate seam under any other name.
+      # scripts/di-seams instead checks each match against an explicit, typed, per-site allowlist
+      # (scripts/di-seams/approved.ts) keyed by (file, field, typeof-target): a triple is exempt
+      # only because it was individually reviewed and named, never because of how it is spelled.
+      # The gate fails just as hard on a stale approval (one whose triple no longer matches
+      # anything) as on an unapproved seam, so the allowlist can't drift out of sync with the code
+      # it describes. See scripts/di-seams/model.ts and its tests.
+      - name: Fail if test-only DI seams reappear in production code
+        uses: ./.github/actions/run-gate
+        with: { gate: di-seams }
+
       - name: Run oxlint
         uses: ./.github/actions/run-gate
         with: { gate: lint }

package.json

@@ -137,6 +137,7 @@
     "check:coverage-changed": "node --experimental-strip-types scripts/coverage-changed/run.ts",
     "check:coverage-changed:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts",
     "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/*.test.ts && node --experimental-strip-types scripts/layering/check.ts",
+    "check:di-seams": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/di-seams/*.test.ts && node --experimental-strip-types scripts/di-seams/check.ts",
     "depgraph": "node --experimental-strip-types scripts/depgraph/build.ts",
     "depgraph:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/depgraph/model.test.ts scripts/depgraph/affected.test.ts",
     "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --fail-on-issues",

scripts/check-affected/checks.ts

@@ -39,6 +39,7 @@ export const CHECK_CATALOG: readonly CheckSpec[] = [
   // make every root-checkout validation install it implicitly.
   gate('test-app-typecheck', 'Expo test app typecheck', 'test-app:typecheck', false),
   gate('layering', 'Import-direction layering guard', 'check:layering'),
+  gate('di-seams', 'Test-only DI seam guard', 'check:di-seams'),
   gate('fallow', 'Fallow code-quality audit', 'check:fallow'),
   gate('mcp-metadata', 'MCP registry metadata sync', 'check:mcp-metadata'),
   gate('build', 'Build (tsdown + declarations)', 'build'),

scripts/check-affected/model.ts

@@ -34,6 +34,7 @@ export type CheckId =
   | 'typecheck'
   | 'test-app-typecheck'
   | 'layering'
+  | 'di-seams'
   | 'fallow'
   | 'mcp-metadata'
   | 'build'
@@ -94,6 +95,7 @@ export const ALL_CHECKS: readonly CheckId[] = [
   'typecheck',
   'test-app-typecheck',
   'layering',
+  'di-seams',
   'fallow',
   'mcp-metadata',
   'build',

scripts/di-seams/check.ts

@@ -0,0 +1,66 @@
+// `pnpm check:di-seams` — fails if a test-only DI seam (an optional `field?: typeof X`
+// parameter that exists only to let a test inject an alternate implementation) reappears in
+// production code without a `// di-seam-approved: <reason>` comment directly above it. See
+// model.ts for how a match and its approval are found; #1976 / PR #2006 for why approval lives
+// as a comment on the declaration rather than in an external table.
+
+import { execFileSync } from 'node:child_process';
+import fs from 'node:fs';
+import path from 'node:path';
+import { pathToFileURL } from 'node:url';
+import { checkSeams, findSeamMatches, type SourceFile } from './model.ts';
+
+const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], {
+  encoding: 'utf8',
+}).trim();
+
+function listProductionSourceFiles(): string[] {
+  const out = execFileSync('git', ['ls-files', '--', 'src'], { cwd: repoRoot, encoding: 'utf8' });
+  return out
+    .split('\n')
+    .filter(Boolean)
+    .filter((file) => !file.includes('/__tests__/') && !file.endsWith('.test.ts'));
+}
+
+function readSources(files: readonly string[]): SourceFile[] {
+  return files.map((file) => ({
+    path: file,
+    source: fs.readFileSync(path.join(repoRoot, file), 'utf8'),
+  }));
+}
+
+export function main(): number {
+  const files = readSources(listProductionSourceFiles());
+  const matches = findSeamMatches(files);
+  const approved = matches.filter((match) => match.approvalReason !== null);
+  const { violations } = checkSeams(matches);
+
+  if (violations.length === 0) {
+    process.stdout.write(
+      `DI-seam guard: OK — ${files.length} production src/ files scanned, ` +
+        `${approved.length} approved seam(s) found, no unapproved seams.\n`,
+    );
+    return 0;
+  }
+
+  process.stderr.write(
+    `Found ${violations.length} test-only DI seam(s) (optional typeof params) in production code:\n`,
+  );
+  for (const violation of violations) {
+    process.stderr.write(`  ${violation.file}:${violation.line}: ${violation.text}\n`);
+    process.stderr.write(
+      `::error file=${violation.file},line=${violation.line},title=Test-only DI seam::` +
+        `${violation.text}\n`,
+    );
+  }
+  process.stderr.write(
+    '\nIf this is a deliberate, reviewed injection seam and not a leftover test seam, add a ' +
+      '`// di-seam-approved: <reason>` comment directly above the declaration — do not broaden ' +
+      'the pattern in model.ts to exempt it by name.\n\n',
+  );
+  return 1;
+}
+
+if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) {
+  process.exit(main());
+}

scripts/di-seams/model.test.ts

@@ -0,0 +1,176 @@
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+import { checkSeams, findSeamMatches } from './model.ts';
+
+test('findSeamMatches finds an unapproved optional typeof property with its line number', () => {
+  const matches = findSeamMatches([
+    { path: 'a.ts', source: 'type T = {\n  dispatch?: typeof dispatchCommand;\n};\n' },
+  ]);
+  assert.deepEqual(matches, [
+    {
+      file: 'a.ts',
+      line: 2,
+      field: 'dispatch',
+      target: 'dispatchCommand',
+      text: 'dispatch?: typeof dispatchCommand;',
+      approvalReason: null,
+    },
+  ]);
+});
+
+test('findSeamMatches ignores a required (non-optional) field', () => {
+  const matches = findSeamMatches([
+    { path: 'a.ts', source: 'type T = { dispatch: typeof dispatchCommand };' },
+  ]);
+  assert.deepEqual(matches, []);
+});
+
+test('findSeamMatches finds a bare optional function parameter, not just object-type fields', () => {
+  const matches = findSeamMatches([
+    { path: 'a.ts', source: 'function f(dispatch?: typeof dispatchCommand) {}\n' },
+  ]);
+  assert.equal(matches.length, 1);
+  assert.equal(matches[0]?.field, 'dispatch');
+  assert.equal(matches[0]?.target, 'dispatchCommand');
+});
+
+// AST-based matching finds this for free — no multiline-specific handling needed, unlike a
+// text-based scan (PR #2006 review, round 2).
+test('findSeamMatches finds a declaration whose `?:` and `typeof` land on different lines', () => {
+  const matches = findSeamMatches([
+    { path: 'a.ts', source: 'type T = {\n  dispatch?:\n    typeof dispatchCommand;\n};\n' },
+  ]);
+  assert.equal(matches.length, 1);
+  assert.equal(matches[0]?.field, 'dispatch');
+  assert.equal(matches[0]?.target, 'dispatchCommand');
+});
+
+test('findSeamMatches attaches an immediately-preceding di-seam-approved comment', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source:
+        'type T = {\n  // di-seam-approved: legitimate, reviewed\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches.length, 1);
+  assert.equal(matches[0]?.approvalReason, 'legitimate, reviewed');
+});
+
+test('findSeamMatches joins a multi-line di-seam-approved comment block into one reason', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source:
+        'type T = {\n  // di-seam-approved: first line of the reason\n  // second line of the reason\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches[0]?.approvalReason, 'first line of the reason second line of the reason');
+});
+
+test('findSeamMatches does not treat a marker on an earlier, unrelated field as approving this one', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source:
+        'type T = {\n  // di-seam-approved: approves otherField only\n  otherField: string;\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches.length, 1);
+  assert.equal(matches[0]?.field, 'dispatch');
+  assert.equal(matches[0]?.approvalReason, null);
+});
+
+test('findSeamMatches requires the marker text itself, not just a nearby comment', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source:
+        'type T = {\n  // just a plain comment, not a marker\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches[0]?.approvalReason, null);
+});
+
+// PR #2006 review: a bare marker with no reason is a bypass, not a review — must not approve.
+test('findSeamMatches rejects a di-seam-approved marker with no reason text', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source: 'type T = {\n  // di-seam-approved:\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches[0]?.approvalReason, null);
+});
+
+test('findSeamMatches rejects a di-seam-approved marker whose reason is only whitespace', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'a.ts',
+      source: 'type T = {\n  // di-seam-approved:   \n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  assert.equal(matches[0]?.approvalReason, null);
+});
+
+test('checkSeams flags a declaration whose only marker has no reason text', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'src/x.ts',
+      source: 'type T = {\n  // di-seam-approved:\n  fetchImpl?: typeof fetch;\n};\n',
+    },
+  ]);
+  const { violations } = checkSeams(matches);
+  assert.equal(violations.length, 1);
+  assert.equal(violations[0]?.field, 'fetchImpl');
+});
+
+test('checkSeams passes an approved match and flags an unapproved one', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'src/x.ts',
+      source:
+        'type T = {\n  // di-seam-approved: reviewed\n  fetchImpl?: typeof fetch;\n  dispatch?: typeof dispatchCommand;\n};\n',
+    },
+  ]);
+  const { violations } = checkSeams(matches);
+  assert.equal(violations.length, 1);
+  assert.equal(violations[0]?.field, 'dispatch');
+});
+
+// This is the exact failure mode PR #2006's review flagged in the name-based version: approving
+// one `fetchImpl?: typeof fetch` must not silently approve a second, different one.
+test('checkSeams flags a second, unmarked occurrence of an approved field/target pair', () => {
+  const matches = findSeamMatches([
+    {
+      path: 'src/x.ts',
+      source:
+        'type T = {\n  // di-seam-approved: reviewed\n  fetchImpl?: typeof fetch;\n};\ntype U = {\n  fetchImpl?: typeof fetch;\n};\n',
+    },
+  ]);
+  const { violations } = checkSeams(matches);
+  assert.equal(violations.length, 1);
+  assert.equal(violations[0]?.line, 6);
+});
+
+// PR #2006 review, round 3: a global positional table breaks on any unrelated line shift. A
+// code-local marker has nothing to resync — reordering unrelated declarations around an approved
+// one must not affect it.
+test('checkSeams stays passing when unrelated code is inserted above an approved declaration', () => {
+  const before = findSeamMatches([
+    {
+      path: 'src/x.ts',
+      source: 'type T = {\n  // di-seam-approved: reviewed\n  fetchImpl?: typeof fetch;\n};\n',
+    },
+  ]);
+  const after = findSeamMatches([
+    {
+      path: 'src/x.ts',
+      source:
+        '// an unrelated new import or declaration lands here\nconst unrelated = 1;\n\ntype T = {\n  // di-seam-approved: reviewed\n  fetchImpl?: typeof fetch;\n};\n',
+    },
+  ]);
+  assert.deepEqual(checkSeams(before).violations, []);
+  assert.deepEqual(checkSeams(after).violations, []);
+  assert.notEqual(before[0]?.line, after[0]?.line);
+});