From bb4de3926f49098fabf063f69bb319428e8f6263 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Thu, 9 Jul 2026 23:49:09 -0700
Subject: [PATCH] fix(test-harness): return promptly when a timed-out child
 leaves pipe-holding orphans

proc.kill() only signals the sh -c wrapper; the claude child survives as an
orphan that inherited our stdout/stderr pipes, blocking the stream drain until
it exits (observed: a 600s spawn timeout stretching to 1431s and tripping bun's
per-test timeout with no result). On timeout, cancel the stdout reader; race
the stderr drain against child exit + 5s grace. Streamed transcript lines
survive the cancel, so callers still get their evidence.

Regression test: test/session-runner-timeout.test.ts (fake claude spawns a
pipe-holding orphan; fails in 30s without the fix, passes in 8s with it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 test/helpers/session-runner.ts      | 19 +++++++-
 test/session-runner-timeout.test.ts | 73 +++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 test/session-runner-timeout.test.ts

diff --git a/test/helpers/session-runner.ts b/test/helpers/session-runner.ts
index c29c59022..88933d739 100644
--- a/test/helpers/session-runner.ts
+++ b/test/helpers/session-runner.ts
@@ -201,6 +201,12 @@ export async function runSkillTest(options: {
   const timeoutId = setTimeout(() => {
     timedOut = true;
     proc.kill();
+    // proc.kill() only signals the `sh -c` wrapper. The claude child it
+    // spawned can survive as an orphan that inherited our stdout/stderr
+    // pipes, so without cancel() the read loop below blocks until the
+    // orphan finally exits (observed: a 600s timeout stretching past 1400s
+    // and tripping bun's per-test timeout instead of returning a result).
+    reader.cancel().catch(() => { /* stream already closed */ });
   }, timeout);
 
   // Stream NDJSON from stdout for real-time progress
@@ -289,7 +295,18 @@ export async function runSkillTest(options: {
     collectedLines.push(buf);
   }
 
-  stderr = await stderrPromise;
+  // Same orphan hazard as stdout: an orphaned grandchild holding stderr open
+  // would block the drain forever. Race it against child exit + a short grace
+  // window; the normal path (pipes close with the child) still wins the race
+  // and keeps full stderr.
+  stderr = await Promise.race([
+    stderrPromise,
+    (async () => {
+      await proc.exited;
+      await new Promise((r) => setTimeout(r, 5_000));
+      return '';
+    })(),
+  ]);
   const exitCode = await proc.exited;
   clearTimeout(timeoutId);
 
diff --git a/test/session-runner-timeout.test.ts b/test/session-runner-timeout.test.ts
new file mode 100644
index 000000000..b77780e0d
--- /dev/null
+++ b/test/session-runner-timeout.test.ts
@@ -0,0 +1,73 @@
+import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
+import { runSkillTest } from './helpers/session-runner';
+import * as fs from 'fs';
+import * as path from 'path';
+import * as os from 'os';
+
+// Regression test for the runSkillTest timeout path (free tier — no API call,
+// the spawned "claude" is a local fake).
+//
+// proc.kill() only signals the `sh -c` wrapper; the claude child it spawned
+// survives as an orphan that inherited our stdout/stderr pipes. Before the
+// fix, the runner then blocked on the pipe drain until the orphan exited —
+// observed as a 600s spawn timeout stretching past 1400s and tripping bun's
+// per-test timeout in skill-e2e-autoplan-dual-voice.test.ts. The fix cancels
+// the stdout reader on timeout and races the stderr drain against child exit
+// plus a short grace window.
+
+const ORPHAN_LINGER_SECS = 45; // without the fix the runner blocks this long
+
+describe.skipIf(process.platform === 'win32')('session-runner timeout path', () => {
+  let fixtureBin: string;
+  let workDir: string;
+
+  beforeAll(() => {
+    fixtureBin = fs.mkdtempSync(path.join(os.tmpdir(), 'fake-claude-bin-'));
+    workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'session-runner-timeout-'));
+
+    // Fake claude: emits minimal stream-json, spawns a pipe-holding orphan,
+    // then lingers in the foreground. Killing the sh wrapper leaves both this
+    // process and its background child alive, holding the pipes open.
+    const fakeClaude = path.join(fixtureBin, 'claude');
+    fs.writeFileSync(
+      fakeClaude,
+      `#!/bin/sh
+cat > /dev/null &
+echo '{"type":"system","subtype":"init"}'
+echo '{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Bash","input":{"command":"echo hi"}}]}}'
+sleep ${ORPHAN_LINGER_SECS} &
+exec sleep ${ORPHAN_LINGER_SECS}
+`,
+      { mode: 0o755 },
+    );
+  });
+
+  afterAll(() => {
+    for (const dir of [fixtureBin, workDir]) {
+      if (dir && fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test(
+    'returns promptly when a killed child leaves a pipe-holding orphan',
+    async () => {
+      const started = Date.now();
+      const result = await runSkillTest({
+        testName: 'session-runner-timeout-orphan',
+        workingDirectory: workDir,
+        prompt: 'irrelevant — the fake claude ignores stdin',
+        timeout: 3_000,
+        env: { PATH: `${fixtureBin}:${process.env.PATH ?? ''}` },
+      });
+      const wall = Date.now() - started;
+
+      expect(result.exitReason).toBe('timeout');
+      // Streamed lines collected before the kill must survive the cancel.
+      expect(result.transcript.some((e: any) => e?.type === 'assistant')).toBe(true);
+      // Without the fix the runner blocks until the orphan exits (~${ORPHAN_LINGER_SECS}s).
+      // Timeout (3s) + stderr grace (5s) + slack must stay well under that.
+      expect(wall).toBeLessThan(20_000);
+    },
+    30_000,
+  );
+});