review-collect: plain paths under --literal-pathspecs; whole-millisecond timeout override (review fix)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

lib/review-collect.ts

@@ -114,7 +114,7 @@ export const CONSUMER_SCAN_PATHSPEC_BYTES = 16_000;
 
 export function consumerScanTimeoutMs(): number {
   const v = Number(process.env.GSTACK_CONSUMER_SCAN_TIMEOUT_MS);
-  return Number.isFinite(v) && v > 0 ? v : CONSUMER_SCAN_TIMEOUT_MS;
+  return Number.isFinite(v) && v >= 1 ? Math.floor(v) : CONSUMER_SCAN_TIMEOUT_MS;
 }
 
 export type GrepFailure = { timedOut: boolean; status: number | null; err: string };
@@ -123,7 +123,7 @@ export type GrepFailure = { timedOut: boolean; status: number | null; err: strin
  * The lines `git grep -I -F -o --null` prints for `patterns` at `rev` ('' is the working tree with untracked files), for
  * the files `keep` accepts, in some order. Every pattern holds its own extension, so a `-l` pass for the extensions alone
  * lists every file that can match; the full pattern list then reads only those files, CONSUMER_SCAN_PATHSPEC_BYTES of
- * literal pathspecs per call. Each call gets what is left of `deadline` (epoch ms).
+ * paths per call under --literal-pathspecs (an inherited GIT_GLOB_PATHSPECS makes git refuse, which fails closed). Each call gets what is left of `deadline` (epoch ms).
  */
 export function grepConsumers(root: string, patterns: readonly string[], rev: string, keep: (file: string) => boolean, deadline: number, chunkBytes = CONSUMER_SCAN_PATHSPEC_BYTES): string[] | GrepFailure {
   const where = rev ? [rev, '--'] : ['--untracked', '--'];
@@ -131,12 +131,12 @@ export function grepConsumers(root: string, patterns: readonly string[], rev: st
   const listed = gitUntil(root, ['grep', '-I', '-F', '-l', '-z', '--no-color', ...exts, ...where], deadline);
   if (listed.status !== 0 && listed.status !== 1) return listed;
   const files = listed.out.split('\0').filter(Boolean).map(f => (rev ? f.slice(rev.length + 1) : f)).filter(keep);
-  const flags = ['grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', '--no-color', ...patterns.flatMap(b => ['-e', b]), ...where];
+  const flags = ['--literal-pathspecs', 'grep', '-I', '-F', '-o', '--null', '--no-line-number', '--no-column', '--no-color', ...patterns.flatMap(b => ['-e', b]), ...where];
   const lines: string[] = [];
   for (let i = 0; i < files.length;) {
     const chunk: string[] = [];
     for (let bytes = 0; i < files.length; i++) {
-      const spec = `:(literal)${files[i]}`;
+      const spec = files[i]!;
       if (chunk.length && bytes + spec.length > chunkBytes) break;
       chunk.push(spec);
       bytes += spec.length;

test/review-collect.test.ts

@@ -3,7 +3,7 @@ import { spawnSync } from 'node:child_process';
 import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
-import { collectCommit, collectLive, CONSUMER_SCAN_CAP, CONSUMER_SCAN_TIMEOUT_MS, grepConsumers, loadRegistryAt, scanConsumers } from '../lib/review-collect';
+import { collectCommit, collectLive, CONSUMER_SCAN_CAP, CONSUMER_SCAN_TIMEOUT_MS, consumerScanTimeoutMs, grepConsumers, loadRegistryAt, scanConsumers } from '../lib/review-collect';
 import { depthOfCommitInput } from '../lib/review-depth';
 
 const dirs: string[] = [];
@@ -239,6 +239,21 @@ describe('GST-544: the consumer scan reads only files that can match, under one
     expect(scanConsumers(r.dir, ['data/rows.json'], [r.head], false, r.dir, failed, CONSUMER_SCAN_TIMEOUT_MS)).toBe('all');
     expect(failed[0]!.detail).toMatch(new RegExp(`^git grep exited \\d+: .*\\(deadline ${CONSUMER_SCAN_TIMEOUT_MS / 1000} s\\)$`));
   });
+  const withEnv = (name: string, value: string, fn: () => void) => {
+    const saved = process.env[name];
+    process.env[name] = value;
+    try { fn(); } finally { if (saved === undefined) delete process.env[name]; else process.env[name] = saved; }
+  };
+  // Value: protects=an inherited GIT_LITERAL_PATHSPECS (git --literal-pathspecs exports it to children) cannot empty the narrowed
+  //   search and drop data to docs; a fractional override cannot crash spawnSync; fails_when=the pathspecs use magic the variable
+  //   turns off, or the timeout is not an integer; seam=the process environment
+  test('an inherited GIT_LITERAL_PATHSPECS keeps the same lines; a fractional timeout override is whole milliseconds', () => {
+    const r = mixed();
+    withEnv('GIT_LITERAL_PATHSPECS', '1', () => {
+      for (const at of [r.head, '']) expect((grepConsumers(r.dir, patterns, at, () => true, Date.now() + 10_000) as string[]).filter(Boolean).sort()).toEqual(wholeTree(r.dir, at));
+    });
+    withEnv('GSTACK_CONSUMER_SCAN_TIMEOUT_MS', '1500.5', () => expect(consumerScanTimeoutMs()).toBe(1500));
+  });
   // Value: protects=a slow scan ends in its own fail-closed consumer_scan_failed, not in the parent killing gstack-review-plan;
   //   fails_when=the scan deadline reaches the timeout the dispatch and preflight callers give the whole plan; seam=caller source
   test('the default scan deadline leaves a minute of the plan timeout its callers set', () => {