mirror of
https://github.com/garrytan/gstack.git
synced 2026-07-11 02:13:47 +02:00
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>
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user