Skip to content

Commit b191cef

Browse files
fix(loop-context): don't crash when an --on-exceed hook ignores stdin (#524)
runOnExceedHook() piped the escalation decision to the hook script's stdin with no 'error' listener on the stream itself. If the script exits without ever reading stdin -- a completely ordinary shape for a notify script (e.g. one that just does `echo escalated`) -- writing to that already-closed pipe raises an 'error' event on the stream (EPIPE on POSIX, EOF on Windows). Node treats an unhandled stream 'error' as an uncaught exception by default, crashing the whole loop-context process, even though this hook is documented and coded as fire-and-forget: the crash happens after the circuit breaker has already made its decision, so it turns a successful --check run into a hard failure for a reason entirely unrelated to the ledger. Attach a no-op 'error' listener on child.stdin so a write-after-close failure is absorbed instead of escalating past the 'close' handler, which already resolves the promise regardless of how the hook exited. Test plan: added a regression test with a hook script that never reads stdin, using a large ledger error string so the piped decision JSON reliably exceeds the OS pipe buffer (removing timing flakiness from the repro) -- confirmed it fails with the exact "write EOF" crash against the pre-fix code and passes with the fix. Full clean rebuild + npm test: 52/52 passing. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent c39ca41 commit b191cef

4 files changed

Lines changed: 54 additions & 0 deletions

File tree

tools/loop-context/dist/cli.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,14 @@ function runOnExceedHook(script, decision) {
276276
resolve();
277277
});
278278
child.on('close', () => resolve());
279+
// The hook script may exit without ever reading stdin (e.g. a notify
280+
// script that ignores its input). Writing to that already-closed pipe
281+
// then raises an 'error' event (EPIPE on POSIX, EOF on Windows) on the
282+
// stream itself; left unhandled, Node treats that as an uncaught
283+
// exception and crashes the whole process. This hook is documented as
284+
// fire-and-forget, so a write failure here must not escalate past the
285+
// 'close' handler above, which resolves regardless.
286+
child.stdin.on('error', () => { });
279287
child.stdin.write(JSON.stringify(decision));
280288
child.stdin.end();
281289
});

tools/loop-context/src/cli.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,14 @@ function runOnExceedHook(script: string, decision: BreakerDecision): Promise<voi
302302
resolve();
303303
});
304304
child.on('close', () => resolve());
305+
// The hook script may exit without ever reading stdin (e.g. a notify
306+
// script that ignores its input). Writing to that already-closed pipe
307+
// then raises an 'error' event (EPIPE on POSIX, EOF on Windows) on the
308+
// stream itself; left unhandled, Node treats that as an uncaught
309+
// exception and crashes the whole process. This hook is documented as
310+
// fire-and-forget, so a write failure here must not escalate past the
311+
// 'close' handler above, which resolves regardless.
312+
child.stdin.on('error', () => {});
305313
child.stdin.write(JSON.stringify(decision));
306314
child.stdin.end();
307315
});

tools/loop-context/test/cli.test.mjs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { resolveDailyBudgetFromPattern } from '../dist/budget-resolver.js';
1010
const testDir = path.dirname(fileURLToPath(import.meta.url));
1111
const cli = path.join(testDir, '../dist/cli.js');
1212
const onExceedCaptureScript = path.join(testDir, 'fixtures/on-exceed-capture.mjs');
13+
const onExceedIgnoreStdinScript = path.join(testDir, 'fixtures/on-exceed-ignore-stdin.mjs');
1314

1415
async function freshDir() {
1516
return mkdtemp(path.join(tmpdir(), 'loop-context-cli-daily-'));
@@ -28,6 +29,9 @@ function runCli(args, input = stagnationLedger) {
2829
return spawnSync(process.execPath, [cli, ...args], {
2930
input,
3031
encoding: 'utf8',
32+
// Default 1MB is too small once a decision's JSON echoes back a large
33+
// error string (see the --on-exceed EPIPE regression test below).
34+
maxBuffer: 20 * 1024 * 1024,
3135
});
3236
}
3337

@@ -154,3 +158,31 @@ test('cli --on-exceed is not invoked when the run continues', async () => {
154158
assert.equal(r.status, 0);
155159
await assert.rejects(() => readFile(outFile, 'utf8'));
156160
});
161+
162+
test('cli --on-exceed does not crash when the hook script exits without reading stdin', async () => {
163+
const dir = await freshDir();
164+
const outFile = path.join(dir, 'ran.marker');
165+
166+
// A large first-line error survives errorSignature() unshortened, so the
167+
// piped decision JSON is big enough to exceed the OS pipe buffer -- this
168+
// makes the write-after-close failure (EPIPE on POSIX, EOF on Windows)
169+
// land reliably instead of racing the hook process's own startup time.
170+
const bigError = 'e'.repeat(5_000_000);
171+
const bigLedger = JSON.stringify({
172+
goal: 'x',
173+
attempts: [
174+
{ iteration: 1, action: 'a', outcome: 'failure', error: bigError },
175+
{ iteration: 2, action: 'a', outcome: 'failure', error: bigError },
176+
{ iteration: 3, action: 'a', outcome: 'failure', error: bigError },
177+
],
178+
});
179+
180+
const r = runCli(
181+
['--check', '--on-exceed', `node ${onExceedIgnoreStdinScript} ${outFile}`, '--json'],
182+
bigLedger,
183+
);
184+
185+
assert.equal(r.status, 2, `expected a clean escalation exit, not a crash. stderr: ${r.stderr}`);
186+
assert.doesNotMatch(r.stderr, /Uncaught|EPIPE|EOF/);
187+
assert.equal(await readFile(outFile, 'utf8'), 'ran');
188+
});
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { writeFileSync } from 'node:fs';
2+
3+
// Deliberately never reads stdin -- simulates an operator's notify script
4+
// that ignores its input entirely. Writes a marker first so the test can
5+
// confirm this process actually ran despite draining nothing.
6+
writeFileSync(process.argv[2], 'ran');

0 commit comments

Comments
 (0)