diff --git a/sdk/typescript/src/exec.ts b/sdk/typescript/src/exec.ts index fb7ed54ad..582fc0bc9 100644 --- a/sdk/typescript/src/exec.ts +++ b/sdk/typescript/src/exec.ts @@ -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(); diff --git a/sdk/typescript/tests/exec.test.ts b/sdk/typescript/tests/exec.test.ts new file mode 100644 index 000000000..9c4b6c253 --- /dev/null +++ b/sdk/typescript/tests/exec.test.ts @@ -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("node:child_process"); + return { ...actual, spawn: jest.fn() }; +}); + +const _actualChildProcess = + jest.requireActual("node:child_process"); +const spawnMock = child_process.spawn as jest.MockedFunction; + +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/); + } + }); +});