Skip to content

fix(loop-context): don't crash when an --on-exceed hook ignores stdin - #524

Merged
cobusgreyling merged 1 commit into
cobusgreyling:mainfrom
KhaiTrang1995:fix/loop-context-on-exceed-stdin-epipe
Aug 17, 2026
Merged

fix(loop-context): don't crash when an --on-exceed hook ignores stdin#524
cobusgreyling merged 1 commit into
cobusgreyling:mainfrom
KhaiTrang1995:fix/loop-context-on-exceed-stdin-epipe

Conversation

@KhaiTrang1995

Copy link
Copy Markdown
Contributor

Problem

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.

Fix

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.

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>

@cobusgreyling cobusgreyling left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fire-and-forget --on-exceed hook must not crash the process on EPIPE when the script never reads stdin. No-op stdin error listener is the right fix; the large-payload test makes the race reliable.

@cobusgreyling
cobusgreyling merged commit b191cef into cobusgreyling:main Aug 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants