Skip to content

Commit abe5708

Browse files
committed
fix(hooks): keep a hook's broken pipe from crashing the run
The payload was written to a lifecycle hook's stdin without awaiting the write. A shell hook that handles one lifecycle kind exits at once on the other, closing its end of the pipe; a run summary larger than the pipe buffers, as a long session's is, then fails with EPIPE, and the unhandled rejection took the process down at the end of a finished run. The write is awaited, a broken pipe is recorded as the hook's outcome, and the run ends as it did.
1 parent a76f1c6 commit abe5708

2 files changed

Lines changed: 91 additions & 3 deletions

File tree

‎src/session/hooks.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
import { describe, expect, test } from "bun:test";
2+
import { mkdtemp, writeFile } from "node:fs/promises";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
25
import type { ReactorEmittedEvent } from "@intx/inference";
36
import {
7+
createLifecycleHookManager,
48
createTurnContextCollector,
59
HOOK_PAYLOAD_TOOL_RESULT_CHARS,
10+
type RunSummary,
611
} from "./hooks.js";
712

813
function event(type: string, data: unknown): ReactorEmittedEvent {
@@ -60,3 +65,53 @@ describe("createTurnContextCollector tool result truncation", () => {
6065
expect(turn?.toolResults[0]?.content).toBe(smallOutput);
6166
});
6267
});
68+
69+
describe("lifecycle hook payload delivery", () => {
70+
test("a hook that exits without reading a large payload is a hook outcome, not a crash", async () => {
71+
// A shell hook that handles only postTurn exits at once on postRun; the
72+
// run summary of a long session is megabytes, far past what the pipe
73+
// buffers, so the write lands on a closed pipe.
74+
const directory = await mkdtemp(join(tmpdir(), "corbits-hook-"));
75+
const path = join(directory, "post-turn-only.sh");
76+
await writeFile(path, 'case "$1" in postTurn) cat > /dev/null ;; esac\n');
77+
const errors: string[] = [];
78+
const manager = createLifecycleHookManager({
79+
hooks: [{ id: path, name: "post-turn-only.sh", type: "shell", path }],
80+
logError: (message) => errors.push(message),
81+
});
82+
const summary: RunSummary = {
83+
task: "x".repeat(2_000_000),
84+
status: "done",
85+
startedAt: 0,
86+
finishedAt: 1,
87+
durationMs: 1,
88+
turnsUsed: 0,
89+
tokenUsage: {
90+
input: 0,
91+
output: 0,
92+
cacheRead: 0,
93+
cacheWrite: 0,
94+
thinking: 0,
95+
},
96+
turns: [],
97+
toolCallCount: 0,
98+
};
99+
let unhandled: unknown = null;
100+
const onUnhandled = (reason: unknown) => {
101+
unhandled = reason;
102+
};
103+
process.on("unhandledRejection", onUnhandled);
104+
try {
105+
await manager.dispatchPostRun(summary);
106+
await new Promise((resolve) => setTimeout(resolve, 200));
107+
} finally {
108+
process.off("unhandledRejection", onUnhandled);
109+
}
110+
// Whether the pipe breaks before the payload is buffered varies run to
111+
// run; what must not vary is that neither outcome takes the process down.
112+
expect(unhandled).toBeNull();
113+
expect(errors).toEqual([]);
114+
const [status] = manager.getStatuses();
115+
expect(status?.lastExitStatus?.code).toBe(0);
116+
});
117+
});

‎src/session/hooks.ts‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { FileSink } from "bun";
12
import { mkdir, readdir, writeFile } from "node:fs/promises";
23
import { createHash } from "node:crypto";
34
import { homedir, tmpdir } from "node:os";
@@ -410,19 +411,51 @@ async function runLifecycleHook(
410411
stdout: "ignore",
411412
stderr: "pipe",
412413
});
413-
proc.stdin.write(JSON.stringify(payload));
414-
proc.stdin.end();
414+
const delivered = await deliverPayload(proc.stdin, JSON.stringify(payload));
415415
const [exitCode, stderr] = await Promise.all([
416416
proc.exited,
417417
new Response(proc.stderr).text(),
418418
]);
419419
return {
420420
code: exitCode,
421421
signal: proc.signalCode,
422-
stderr,
422+
stderr: delivered
423+
? stderr
424+
: `${stderr}${stderr.length > 0 && !stderr.endsWith("\n") ? "\n" : ""}hook exited without reading its payload\n`,
423425
};
424426
}
425427

428+
/**
429+
* Writes the payload to the hook and says whether the hook took it. A hook
430+
* that exits before reading — a shell hook that handles one lifecycle kind
431+
* and ignores the other — closes its end of the pipe, and a payload larger
432+
* than the pipe buffers (a long session's run summary) then fails with
433+
* EPIPE. Unawaited, that rejection was fatal to the whole process at the end
434+
* of a finished run. It is the hook's outcome, not the run's.
435+
*/
436+
async function deliverPayload(
437+
stdin: FileSink,
438+
payload: string,
439+
): Promise<boolean> {
440+
try {
441+
await stdin.write(payload);
442+
await stdin.end();
443+
return true;
444+
} catch (err: unknown) {
445+
if (isBrokenPipe(err)) return false;
446+
throw err;
447+
}
448+
}
449+
450+
function isBrokenPipe(err: unknown): boolean {
451+
return (
452+
typeof err === "object" &&
453+
err !== null &&
454+
"code" in err &&
455+
(err as { code?: unknown }).code === "EPIPE"
456+
);
457+
}
458+
426459
function hookCommand(hook: LifecycleHook, kind: HookKind): string[] {
427460
return ["sh", hook.path, kind];
428461
}

0 commit comments

Comments
 (0)