fix(#3086): apply #2667 .cmd-shim gate to deps.spawn + surface errorCode in review lanes (#3142)

* fix(#3086): apply #2667 .cmd-shim gate to deps.spawn + surface errorCode in review lanes

deps.spawn used shell:false with a bare binary name — on Windows, npm-installed
CLIs (gemini, codex, etc.) are .cmd shims that CreateProcess cannot start,
producing ENOENT + empty stderr. The review path then wrote an empty err file
and emitted a generic 'failed or returned empty output' stub.

Two fixes:
1. deps.spawn: detect .cmd/.bat on win32 and mediate through cmd.exe /d /s /c
   (same gate as runWithTimeout #2667, same explicit argv array).
2. runSpawnLane: surface errorCode (ENOENT, ETIMEDOUT) in the err file so the
   stub explains WHY the lane produced nothing.

* chore(#3086): backfill changeset PR number 3142

---------

Co-authored-by: sim <sim@local>

.changeset/nimble-sloths-jump.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3142
+---
+**Cross-AI reviewer lanes no longer silently drop on Windows** — `deps.spawn` used `shell: false` with a bare binary name, which fails with ENOENT on Windows .cmd shims (npm-installed CLIs). Now applies the #2667 `cmd.exe /d /s /c` shim gate. Spawn errors (ENOENT, ETIMEDOUT) are also surfaced in the reviewer err file instead of being silently dropped. (#3086)

gsd-core/bin/gsd-tools.cjs

@@ -1330,7 +1330,16 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load
     // cannot be interrupted by --test-force-exit and hangs a whole CI chunk to its 10-minute kill.
     const deps = {
       spawn: (binary, argv, opts) => {
-        const r = cp.spawnSync(binary, argv, {
+        // #3086: on Windows, reviewer CLIs (gemini, codex, etc.) are installed
+        // as .cmd shims. spawnSync with a bare name + shell:false fails with
+        // ENOENT (CreateProcess cannot start .cmd). Apply the same #2667 shim
+        // gate used in runWithTimeout: detect .cmd/.bat and mediate through
+        // cmd.exe /d /s /c with an explicit argv array (no shell:true).
+        const isWin = process.platform === 'win32';
+        const winShim = isWin && /\.(cmd|bat)$/i.test(path.basename(binary));
+        const spawnBinary = winShim ? (process.env.ComSpec || 'cmd.exe') : binary;
+        const spawnArgv = winShim ? ['/d', '/s', '/c', binary, ...argv] : argv;
+        const r = cp.spawnSync(spawnBinary, spawnArgv, {
           input: opts.input,
           encoding: 'utf8',
           timeout: opts.timeoutMs,

src/review-lane-runner.cts

@@ -664,7 +664,13 @@ function runSpawnLane(plan: SpawnPlan, deps: RunnerDeps, repoRoot: string): Lane
       : plan.argv;
 
   const out = deps.spawn(plan.binary, argv, { input, timeoutMs: plan.timeoutMs });
-  deps.writeFile(plan.errPath, out.stderr ?? '');
+  // #3086: surface spawn errors (ENOENT, ETIMEDOUT, etc.) that would otherwise
+  // be silently dropped — the review path read only stdout/stderr and treated
+  // an empty-stderr spawn failure as "the model had nothing to say".
+  const errContent = out.errorCode
+    ? `${out.stderr ?? ''}\n[spawn error: ${out.errorCode}]\n`
+    : (out.stderr ?? '');
+  deps.writeFile(plan.errPath, errContent);
 
   // `file-arg` lanes write the review themselves and their stdout is deliberately discarded (#1698).
   let review =

tests/review-lane-runner.test.cjs

@@ -663,3 +663,44 @@ describe('runner — orchestration', () => {
     }
   });
 });
+
+// #3086: spawn errors (ENOENT on Windows .cmd shims, ETIMEDOUT, etc.) must be
+// surfaced in the err file so the stub reviewer output explains WHY the lane
+// produced nothing, rather than silently dropping the error code.
+
+describe('runner — #3086: spawn errorCode surfaced in err file', () => {
+  test('a spawn ENOENT writes the error code to the err file', async () => {
+    const p = plan('gemini');
+    const d = deps({
+      spawn: () => ({ status: null, stdout: '', stderr: '', errorCode: 'ENOENT' }),
+    });
+    await runLane(p, d, { repoRoot: ROOT });
+    const errContent = d.files[p.errPath] || '';
+    assert.ok(errContent.includes('ENOENT'),
+      `err file must include the spawn error code; got: ${errContent}`);
+  });
+
+  test('a spawn ETIMEDOUT writes the error code to the err file', async () => {
+    const p = plan('codex');
+    const d = deps({
+      spawn: () => ({ status: null, stdout: '', stderr: '', errorCode: 'ETIMEDOUT' }),
+    });
+    await runLane(p, d, { repoRoot: ROOT });
+    const errContent = d.files[p.errPath] || '';
+    assert.ok(errContent.includes('ETIMEDOUT'),
+      `err file must include the spawn error code; got: ${errContent}`);
+  });
+
+  test('a successful spawn with stderr does NOT add a spawn error marker', async () => {
+    const p = plan('gemini');
+    const d = deps({
+      spawn: () => ({ status: 0, stdout: '## Review\nok', stderr: 'some warning', errorCode: undefined }),
+    });
+    await runLane(p, d, { repoRoot: ROOT });
+    const errContent = d.files[p.errPath] || '';
+    assert.ok(!errContent.includes('[spawn error:'),
+      `err file must NOT contain a spawn error marker on success; got: ${errContent}`);
+    assert.ok(errContent.includes('some warning'),
+      'legitimate stderr should still be written');
+  });
+});