Harden reviewer gate, edit recovery, and spawn/tool-call robustness

agents/__tests__/gate-paths-parity.test.ts

@@ -188,13 +188,11 @@ describe('gate-path helpers — inline copies match canonical exports', () => {
     const pathInputs: string[] = [
       // reviewable source (expect true)
       'src/foo.ts',
-      // __tests__/ path (false)
+      // Tests are first-class reviewable files.
       'src/__tests__/foo.ts',
-      // .test.ts path (false)
       'src/foo.test.ts',
-      // JS-flavored test/spec files (false) — parity must hold across the
-      // broadened (?:tsx?|jsx?|mjs|cjs) exclusion so a `.test.js`/`.spec.mjs`
-      // test file is not classified as reviewable source by the inline copy.
+      // JS-flavored test/spec files are reviewable and remain separately
+      // identifiable as coverage evidence.
       'src/foo.test.js',
       'src/foo.spec.mjs',
       'src/foo.test.cjs',
@@ -234,7 +232,7 @@ describe('gate-path helpers — inline copies match canonical exports', () => {
     // selectReviewableGateFiles normalizes + filters + dedupes internally, so
     // pass raw path lists.
     const listInputs: string[][] = [
-      // mixed source + bookkeeping: only source survives, normalized + deduped
+      // mixed source + tests + bookkeeping: source and tests survive
       [
         'src/foo.ts',
         './src/foo.ts',
@@ -367,12 +365,12 @@ describe('gate-path helpers — canonical export behavior', () => {
     expect(normalizeGateFilePath('   ')).toBe('')
   })
 
-  test('selectReviewableGateFiles keeps reviewable source and drops tests/generated/docs/data/bookkeeping', () => {
+  test('selectReviewableGateFiles keeps source and tests and drops generated/docs/data/bookkeeping', () => {
     const selected = selectReviewableGateFiles([
       // reviewable source (kept): .ts and .py
       'src/foo.ts',
       'scripts/tool.py',
-      // __tests__/ and .test/.spec (dropped)
+      // __tests__/ and .test/.spec (kept)
       'src/__tests__/foo.ts',
       'src/foo.test.ts',
       'src/foo.spec.ts',
@@ -392,7 +390,13 @@ describe('gate-path helpers — canonical export behavior', () => {
       'evals/case.ts',
       '.agents/sessions/slug/STATE.json',
     ])
-    expect(selected).toEqual(['src/foo.ts', 'scripts/tool.py'])
+    expect(selected).toEqual([
+      'src/foo.ts',
+      'scripts/tool.py',
+      'src/__tests__/foo.ts',
+      'src/foo.test.ts',
+      'src/foo.spec.ts',
+    ])
     // empty list -> []
     expect(selectReviewableGateFiles([])).toEqual([])
   })
@@ -421,7 +425,11 @@ describe('gate-path helpers — canonical export behavior', () => {
       'src/__tests__/feature.ts',
     ]
 
-    expect(selectReviewableGateFiles(inputs)).toEqual(['src/feature.ts'])
+    expect(selectReviewableGateFiles(inputs)).toEqual([
+      'src/feature.ts',
+      'src/feature.test.ts',
+      'src/__tests__/feature.ts',
+    ])
     expect(selectCoverageEvidenceFiles(inputs)).toEqual([
       'src/feature.test.ts',
       'src/__tests__/feature.ts',
@@ -443,12 +451,9 @@ describe('gate-path helpers — canonical export behavior', () => {
     ).toEqual(['src/feature.test.ts', 'src/__tests__/feature.ts'])
   })
 
-  test('JS-flavored test files are excluded from reviewable source and treated as coverage evidence', () => {
-    // RF-3/RF-9: widen the test/spec exclusion so JS-flavored test files
-    // (foo.test.mjs/.cjs/.jsx, foo.spec.*) are treated as tests rather than
-    // reviewable source. Without this, the reviewable include regex (which
-    // already accepts mjs|cjs|jsx as source extensions) would classify a JS
-    // test file as reviewable source.
+  test('JS-flavored test files are reviewable and treated as coverage evidence', () => {
+    // Tests participate in reviewer attestation while remaining identifiable
+    // as coverage evidence.
     for (const testFile of [
       'src/foo.test.mjs',
       'src/foo.test.cjs',
@@ -457,7 +462,7 @@ describe('gate-path helpers — canonical export behavior', () => {
       'src/foo.spec.cjs',
       'src/foo.spec.jsx',
     ]) {
-      expect(isReviewableGateFile(testFile)).toBe(false)
+      expect(isReviewableGateFile(testFile)).toBe(true)
       expect(isCoverageEvidenceFile(testFile)).toBe(true)
     }
     // Non-test JS-flavored source stays reviewable and is not coverage evidence.

agents/__tests__/gate-paths.test.ts

@@ -134,7 +134,7 @@ describe('gate-paths helpers', () => {
     expect(gateFileSetsEqual(['a', 'b'], ['a', 'c'])).toBe(false)
   })
 
-  test('isReviewableGateFile includes real source and excludes tests/generated/docs/data/bookkeeping', () => {
+  test('isReviewableGateFile includes source and tests while excluding generated/docs/data/bookkeeping', () => {
     // Reviewable source extensions (expect true).
     for (const reviewable of [
       'src/a.ts',
@@ -148,19 +148,19 @@ describe('gate-paths helpers', () => {
       'cmd/main.go',
       'svc/App.java',
       'app/Main.kt',
-    ]) {
-      expect(isReviewableGateFile(reviewable)).toBe(true)
-    }
-
-    // Tests, generated code, docs, data/config, env, and bookkeeping dirs
-    // (expect false).
-    for (const excluded of [
       'src/__tests__/a.ts',
       'src/a.test.ts',
       'src/a.spec.tsx',
       'src/a.test.mjs',
       'src/a.spec.cjs',
       'src/a.test.jsx',
+    ]) {
+      expect(isReviewableGateFile(reviewable)).toBe(true)
+    }
+
+    // Generated code, docs, data/config, env, and bookkeeping dirs
+    // (expect false).
+    for (const excluded of [
       'src/a.generated.ts',
       'src/a.generated.tsx',
       'README.md',
@@ -206,22 +206,22 @@ describe('gate-paths helpers', () => {
     }
   })
 
-  test('selectReviewableGateFiles normalizes, dedupes, drops empties, keeps only reviewable source', () => {
+  test('selectReviewableGateFiles normalizes, dedupes, and keeps source and tests', () => {
     const cwd = process.cwd().replace(/\\/g, '/').replace(/\/+$/, '')
     const result = selectReviewableGateFiles([
       'src/a.ts',
       './src/a.ts', // dedupes with src/a.ts after normalization
       `${cwd}/src/a.ts`, // in-cwd absolute -> src/a.ts (dedupe)
       'src\\b.ts', // backslashes -> src/b.ts
-      'src/a.test.ts', // test file excluded
+      'src/a.test.ts', // test file included
       'src/a.generated.ts', // generated excluded
       'README.md', // doc excluded
       'docs/guide.ts', // docs/ excluded
       '/etc/passwd', // absolute-outside-cwd -> '' dropped
       '', // empty dropped
       '   ', // whitespace dropped
     ])
-    expect(result).toEqual(['src/a.ts', 'src/b.ts'])
+    expect(result).toEqual(['src/a.ts', 'src/b.ts', 'src/a.test.ts'])
   })
 
   test('selectCoverageEvidenceFiles normalizes, dedupes, and keeps only test files', () => {

agents/__tests__/gate-reviewer-parity.test.ts

@@ -0,0 +1,142 @@
+import { readFileSync } from 'node:fs'
+
+import { describe, expect, test } from 'bun:test'
+
+import { isTestCoverageReviewerFinding } from '../base2/gate-reviewer'
+
+// Parity: inline base2 mirror must match the gate-reviewer export used for
+// exclusive all-coverage → test-writer routing.
+type GateReviewerHelpers = {
+  isTestCoverageReviewerFinding: (text: string) => boolean
+}
+
+type GateReviewerFunctionName = keyof GateReviewerHelpers
+type InlineHelperFactory = () => GateReviewerHelpers
+
+const INLINE_HELPER_NAMES: GateReviewerFunctionName[] = [
+  'isTestCoverageReviewerFinding',
+]
+
+function extractInlineFunctionSource(
+  source: string,
+  functionName: string,
+): string {
+  const declarationStart = source.indexOf(`function ${functionName}(`)
+  if (declarationStart < 0) {
+    throw new Error(`Unable to find inline ${functionName} declaration`)
+  }
+
+  const bodyStart = source.indexOf('{', declarationStart)
+  if (bodyStart < 0) {
+    throw new Error(`Unable to find inline ${functionName} body`)
+  }
+
+  let depth = 0
+  for (let index = bodyStart; index < source.length; index += 1) {
+    const character = source[index]
+    if (character === '{') depth += 1
+    if (character === '}') depth -= 1
+    if (depth === 0) {
+      return source.slice(declarationStart, index + 1)
+    }
+  }
+
+  throw new Error(`Unable to find end of inline ${functionName} declaration`)
+}
+
+function loadInlineGateReviewerHelpers(): GateReviewerHelpers {
+  const base2Source = readFileSync(
+    new URL('../base2/base2.ts', import.meta.url),
+    'utf8',
+  )
+  const transpiler = new Bun.Transpiler({ loader: 'ts' })
+  const base2JavaScript = transpiler.transformSync(base2Source)
+  const helperSource = INLINE_HELPER_NAMES.map((functionName) =>
+    extractInlineFunctionSource(base2JavaScript, functionName),
+  ).join('\n\n')
+  const buildHelpers = new Function(
+    `"use strict";\n${helperSource}\nreturn { isTestCoverageReviewerFinding }`,
+  ) as InlineHelperFactory
+
+  return buildHelpers()
+}
+
+describe('gate-reviewer helpers — inline copies match canonical exports', () => {
+  test('isTestCoverageReviewerFinding parity across representative inputs', () => {
+    const inlineHelpers = loadInlineGateReviewerHelpers()
+
+    const inputs: unknown[] = [
+      // coverage-only findings (expect true)
+      'BLOCKING: test coverage missing for changed behavior (add a case to the relevant *.test.ts)',
+      'test coverage is insufficient',
+      'TEST COVERAGE missing',
+      'BLOCKING: coverage gap: add a case to src/foo.test.ts for the new behavior',
+      'coverage missing; extend widget.test.tsx',
+      // generic test/coverage mentions that keep the repair-editor path (false)
+      'BLOCKING: add tests for the parser',
+      'BLOCKING: this path is not tested',
+      'BLOCKING: coverage of edge cases is unclear',
+      'BLOCKING: update foo.test.ts to match the new API',
+      'BLOCKING: fix the null dereference in parse()',
+      '',
+      '   ',
+      // non-string inputs (false)
+      undefined,
+      null,
+      42,
+      {},
+      ['test coverage'],
+    ]
+
+    for (const input of inputs) {
+      expect(
+        inlineHelpers.isTestCoverageReviewerFinding(input as string),
+      ).toBe(isTestCoverageReviewerFinding(input as string))
+    }
+  })
+})
+
+// Direct behavioral coverage for the canonical gate-reviewer.ts export (as
+// opposed to the parity suite above, which only asserts the inline base2 copy
+// matches the export). These assertions also keep the export consumed so it
+// is not dead code.
+describe('gate-reviewer helpers — canonical export behavior', () => {
+  test('isTestCoverageReviewerFinding keys on the test-coverage bigram or coverage plus a .test.* token', () => {
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: test coverage missing for changed behavior (add a case to the relevant *.test.ts)',
+      ),
+    ).toBe(true)
+    expect(isTestCoverageReviewerFinding('TEST COVERAGE missing')).toBe(true)
+    expect(
+      isTestCoverageReviewerFinding('coverage missing; extend widget.test.tsx'),
+    ).toBe(true)
+  })
+
+  test('isTestCoverageReviewerFinding stays conservative for generic test/coverage mentions', () => {
+    expect(
+      isTestCoverageReviewerFinding('BLOCKING: add tests for the parser'),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: coverage of edge cases is unclear',
+      ),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: update foo.test.ts to match the new API',
+      ),
+    ).toBe(false)
+    expect(isTestCoverageReviewerFinding('')).toBe(false)
+  })
+
+  test('isTestCoverageReviewerFinding rejects non-string inputs', () => {
+    expect(
+      isTestCoverageReviewerFinding(undefined as unknown as string),
+    ).toBe(false)
+    expect(isTestCoverageReviewerFinding(null as unknown as string)).toBe(
+      false,
+    )
+    expect(isTestCoverageReviewerFinding(42 as unknown as string)).toBe(false)
+  })
+})

agents/__tests__/gate-reviewer.test.ts

@@ -8,6 +8,7 @@ import {
   collectReviewerFindingRecords,
   detectReviewerCrash,
   getReviewerFinalizationVerdict,
+  isTestCoverageReviewerFinding,
   stripReviewerPreamble,
 } from '../base2/gate-reviewer'
 
@@ -611,4 +612,64 @@ describe('gate-reviewer helpers', () => {
       ),
     ).toBe('')
   })
+
+  test('isTestCoverageReviewerFinding keys on the test-coverage bigram or coverage plus a .test.* token', () => {
+    // The synthetic coverage blocker from collectReviewerBlockers must classify
+    // as coverage so the all-coverage set routes exclusively to test-writer.
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: test coverage missing for changed behavior (add a case to the relevant *.test.ts)',
+      ),
+    ).toBe(true)
+    expect(
+      isTestCoverageReviewerFinding('test coverage is insufficient'),
+    ).toBe(true)
+    expect(isTestCoverageReviewerFinding('TEST COVERAGE missing')).toBe(true)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: coverage gap: add a case to src/foo.test.ts for the new behavior',
+      ),
+    ).toBe(true)
+    expect(
+      isTestCoverageReviewerFinding('coverage missing; extend widget.test.tsx'),
+    ).toBe(true)
+  })
+
+  test('isTestCoverageReviewerFinding stays conservative for generic test/coverage mentions', () => {
+    // Generic requirements mentioning test(s)/tested or bare coverage must
+    // keep routing to repair-editor (status quo).
+    expect(
+      isTestCoverageReviewerFinding('BLOCKING: add tests for the parser'),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding('BLOCKING: this path is not tested'),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: coverage of edge cases is unclear',
+      ),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: update foo.test.ts to match the new API',
+      ),
+    ).toBe(false)
+    expect(
+      isTestCoverageReviewerFinding(
+        'BLOCKING: fix the null dereference in parse()',
+      ),
+    ).toBe(false)
+    expect(isTestCoverageReviewerFinding('')).toBe(false)
+  })
+
+  test('isTestCoverageReviewerFinding rejects non-string inputs', () => {
+    expect(
+      isTestCoverageReviewerFinding(undefined as unknown as string),
+    ).toBe(false)
+    expect(isTestCoverageReviewerFinding(null as unknown as string)).toBe(
+      false,
+    )
+    expect(isTestCoverageReviewerFinding(42 as unknown as string)).toBe(false)
+    expect(isTestCoverageReviewerFinding({} as unknown as string)).toBe(false)
+  })
 })