fix: handle early codex exec exit (#8825)

Fixes CodexExec to avoid missing early process exits by registering the
exit handler up front and deferring the error until after stdout is
drained, and adds a regression test that simulates a fast-exit child
while still producing output so hangs are caught.
This commit is contained in:
Thibault Sottiaux
2026-01-07 08:54:27 -08:00
committed by GitHub
parent 4cef89a122
commit 0d788e6263
2 changed files with 84 additions and 14 deletions

View File

@@ -153,6 +153,14 @@ export class CodexExec {
});
}
const exitPromise = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>(
(resolve) => {
child.once("exit", (code, signal) => {
resolve({ code, signal });
});
},
);
const rl = readline.createInterface({
input: child.stdout,
crlfDelay: Infinity,
@@ -164,21 +172,13 @@ export class CodexExec {
yield line as string;
}
const exitCode = new Promise((resolve, reject) => {
child.once("exit", (code) => {
if (code === 0) {
resolve(code);
} else {
const stderrBuffer = Buffer.concat(stderrChunks);
reject(
new Error(`Codex Exec exited with code ${code}: ${stderrBuffer.toString("utf8")}`),
);
}
});
});
if (spawnError) throw spawnError;
await exitCode;
const { code, signal } = await exitPromise;
if (code !== 0 || signal) {
const stderrBuffer = Buffer.concat(stderrChunks);
const detail = signal ? `signal ${signal}` : `code ${code ?? 1}`;
throw new Error(`Codex Exec exited with ${detail}: ${stderrBuffer.toString("utf8")}`);
}
} finally {
rl.close();
child.removeAllListeners();

View File

@@ -0,0 +1,70 @@
import * as child_process from "node:child_process";
import { EventEmitter } from "node:events";
import { PassThrough } from "node:stream";
import { describe, expect, it } from "@jest/globals";
jest.mock("node:child_process", () => {
const actual = jest.requireActual<typeof import("node:child_process")>("node:child_process");
return { ...actual, spawn: jest.fn() };
});
const _actualChildProcess =
jest.requireActual<typeof import("node:child_process")>("node:child_process");
const spawnMock = child_process.spawn as jest.MockedFunction<typeof _actualChildProcess.spawn>;
class FakeChildProcess extends EventEmitter {
stdin = new PassThrough();
stdout = new PassThrough();
stderr = new PassThrough();
killed = false;
kill(): boolean {
this.killed = true;
return true;
}
}
function createEarlyExitChild(exitCode = 2): FakeChildProcess {
const child = new FakeChildProcess();
setImmediate(() => {
child.stderr.write("boom");
child.emit("exit", exitCode, null);
setImmediate(() => {
child.stdout.end();
child.stderr.end();
});
});
return child;
}
const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
describe("CodexExec", () => {
it("rejects when exit happens before stdout closes", async () => {
const { CodexExec } = await import("../src/exec");
const child = createEarlyExitChild();
spawnMock.mockReturnValue(child as unknown as child_process.ChildProcess);
const exec = new CodexExec("codex");
const runPromise = (async () => {
for await (const _ of exec.run({ input: "hi" })) {
// no-op
}
})().then(
() => ({ status: "resolved" as const }),
(error) => ({ status: "rejected" as const, error }),
);
const result = await Promise.race([
runPromise,
delay(500).then(() => ({ status: "timeout" as const })),
]);
expect(result.status).toBe("rejected");
if (result.status === "rejected") {
expect(result.error).toBeInstanceOf(Error);
expect(result.error.message).toMatch(/Codex Exec exited/);
}
});
});