fix(hooks): stop self-reporting benign BrokenPipeError to gateway (AI-GATEWAY-3J)#177
fix(hooks): stop self-reporting benign BrokenPipeError to gateway (AI-GATEWAY-3J)#177anonpran wants to merge 1 commit into
Conversation
…-GATEWAY-3J) The Claude Code / Codex / Copilot / Cursor hook scripts end main() by writing their JSON response to stdout via print(..., flush=True). When the host has already closed the read end of stdout (hook timeout, user cancel, session end, or a foreground PreToolUse approval-poll blocking up to 4h), the flush raises BrokenPipeError [Errno 32]. The broad `except Exception` caught it and self-reported it via log_error -> report_error_to_gateway -> /v1/hooks/errors -> Sentry, producing ~2200 noise events collapsed under one fingerprint. A secondary bug: the fallback print in the except re-hit the dead pipe and raised an UNHANDLED second BrokenPipeError (noisy non-zero exit). Fix (client-side only; the gateway is correct/fail-open and is untouched): - Add an _emit() helper that writes to stdout but treats a closed reader pipe as a benign no-op (swallows BrokenPipeError/OSError and redirects stdout to os.devnull so the interpreter-shutdown flush cannot re-raise). - Route every response write in main() through _emit(). - Split main()'s catch-all so `except BrokenPipeError` is intercepted first (benign, not reported) before `except Exception` (genuine errors are still reported and redacted via log_error). Cursor's WEB-4734 stderr redaction is preserved and guarded. Adds test_broken_pipe.py to all four hooks (12 tests): a broken pipe is not reported, real exceptions are still reported, _emit on a dead pipe does not raise. Follow-ups (out of scope): clamp the 4h foreground poll_approval_status that is the dominant trigger; add a CI job to execute these unittest files (static-checks currently runs only py_compile + pyflakes). Fixes AI-GATEWAY-3J Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| a benign no-op. The host may close the read end (timeout, cancel, session | ||
| end, blocked approval-poll) before we flush — that is not a hook error.""" | ||
| try: | ||
| sys.stdout.write(text + "\n") | ||
| sys.stdout.flush() | ||
| except (BrokenPipeError, OSError): | ||
| try: | ||
| sys.stdout = open(os.devnull, "w") | ||
| except Exception: |
There was a problem hiding this comment.
Silent
OSError swallow is broader than intended
_emit catches (BrokenPipeError, OSError), but BrokenPipeError is already a subclass of OSError, so the tuple is redundant and the bare OSError catch covers every OS-level I/O error on stdout — not just closed-pipe scenarios. A non-pipe OSError (e.g., ENOSPC on a redirected stdout, a permission error on a named pipe used for IPC) would be silently swallowed, stdout swapped to /dev/null, and no error reported anywhere. The same pattern is duplicated in codex/hooks/unbound.py, copilot/hooks/unbound.py, and cursor/unbound.py.
| except BrokenPipeError: | ||
| # Host closed the read end of our stdout pipe (timeout / cancel / | ||
| # session end / blocked approval-poll). Benign — do not self-report. | ||
| try: | ||
| sys.stdout = open(os.devnull, "w") | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Broken-pipe path is completely dark — no way to confirm the fix is working
The except BrokenPipeError handler redirects stdout to /dev/null and exits silently. Nothing is written to stderr, no counter is incremented, and no gateway call is made. Once deployed, the team can only verify the fix by watching AI-GATEWAY-3J go quiet in Sentry. If broken-pipe events spike again (e.g., a new 4h foreground poll timeout trigger), or if the fix has a regression in an edge case, there is no signal to detect it. Even a single print("broken pipe suppressed", file=sys.stderr) (or a non-Sentry lightweight counter via the existing gateway metrics path) would allow the team to confirm frequency post-deploy. This same gap exists in all four hooks.
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Broad
OSErrorcatch in_emit— PR intentionally swallowsBrokenPipeError/OSErroron stdout and redirects toos.devnull; Greptile P2 robustness concern, not a security defect. - Silent broken-pipe path (no observability) — Accepted trade-off for this fix; post-deploy validation via Sentry quieting; explicit follow-up deferred out of scope.
- Semgrep
insecure-file-permissions(chmod0o755/$BITS) — Pre-existing code on unchanged lines; not introduced by this diff. - Semgrep
sqlalchemy-execute-raw-query(cursor/unbound.py:510) — Pre-existing local SQLite usage; unchanged by this PR.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 4c1c7abd · 2026-06-23T11:24Z
Summary
Fixes Sentry AI-GATEWAY-3J —
HookError:general: [Hook:claude-code] Exception in main: [Errno 32] Broken pipe(~2200 occurrences, ongoing). This is client-side-only; the gateway is correct (fail-open, always 200) and is untouched.Root cause: the four coding-tool hooks end
main()by writing their JSON response to stdout viaprint(..., flush=True). When the host (Claude Code / Cursor / Copilot / Codex) has already closed the read end of stdout — hook timeout, user cancel, session end, or (dominant) a foregroundPreToolUseapproval-poll blocking up toAPPROVAL_TIMEOUT=4h— the flush raises PythonBrokenPipeError. The broadexcept Exceptionthen mistook this benign closed pipe for a crash and self-reported it vialog_error -> report_error_to_gateway -> POST /v1/hooks/errors -> Sentry. A secondary bug: the fallbackprintin theexceptre-hit the same dead pipe and raised an unhandled secondBrokenPipeError, so the hook exited noisily/non-zero instead of degrading gracefully.Changes
_emit(text)helper to each hook that writes to stdout but treats a closed reader pipe as a benign no-op (swallowsBrokenPipeError/OSError, redirects stdout toos.devnullso the interpreter-shutdown flush can't re-raise).main()through_emit().main()'s catch-all soexcept BrokenPipeError:is intercepted first (benign — not reported) beforeexcept Exception:(genuine errors are still reported and redacted vialog_error).redact_secrets(str(e), _cached_api_key)) is preserved and now guarded against a broken stderr pipe.os/sysalready present in all four) — frozen-binary drift-guard unaffected. The four hooks remain intentionally standalone (per-file_emitduplication is deliberate for PyInstaller vendoring).Files:
claude-code/hooks/unbound.py,codex/hooks/unbound.py,copilot/hooks/unbound.py,cursor/unbound.py(+ atest_broken_pipe.pybeside each).Test plan
unittest:BrokenPipeErroron the stdout write is not forwarded toreport_error_to_gateway/log_errormain()is still reported ("Exception in main: boom", categorygeneral)_emitto a dead pipe does not raise and swaps stdout to devnullexceptorder fails the suite (assertions are load-bearing)test_identityallow-list,binaryfrozen-session-start) reproduce on pristine HEADpy_compile/ast.parsecleanFollow-ups (out of scope here)
poll_approval_statuson thePreToolUsepath — the dominant trigger of the closed pipe (own enforcement blast radius; deferred).unittestfiles —static-checks.ymlcurrently runs onlypy_compile+pyflakes, so this behavior isn't CI-enforced.🤖 Generated with Claude Code
Note
Low Risk
Client-side hook I/O and error-handling only; genuine exceptions still report, with no gateway or policy behavior changes.
Overview
Stops AI-GATEWAY-3J noise by treating a closed stdout pipe as normal when the IDE hook host ends the session or times out before the hook flushes its JSON response.
Each of the four standalone
unbound.pyhooks (Claude Code, Codex, Copilot, Cursor) adds_emit, routes all hook responses through it instead ofprint(..., flush=True), and handlesBrokenPipeErrorinmain()before the genericexcept Exceptionso those cases are not sent tolog_error/ the gateway._emitswallows write/flush failures and redirects stdout toos.devnullso shutdown does not raise again; Cursor also guards stderr redaction prints the same way.Adds
test_broken_pipe.pybeside each hook (12 tests total) covering benign pipe behavior, real errors still reported, and dead-pipe_emitbehavior.Reviewed by Cursor Bugbot for commit 4c1c7ab. Bugbot is set up for automated code reviews on this repo. Configure here.
Greptile Summary
This PR fixes AI-GATEWAY-3J by stopping all four coding-tool hooks from self-reporting benign
BrokenPipeErrors that occur when the host closes the read end of stdout before the hook can flush its JSON response. The fix introduces an_emithelper that silently redirects stdout to/dev/nullon a closed pipe, and splitsmain()'s outer exception handler soBrokenPipeErroris caught first (discarded) and genuine errors remain reported vialog_error._emit(text)to all four hooks (claude-code,codex,copilot,cursor) with internal broken-pipe suppression and stdout-to-devnull swap, plus an outerexcept BrokenPipeErrorguard inmain()as a second layer of defence._emititself surviving a dead pipe without raising.Confidence Score: 4/5
Safe to merge — the core fix is structurally sound and well-tested, with the broken-pipe path correctly isolated from genuine error reporting in all four hooks.
The
_emitimplementation correctly suppresses the noisy broken-pipe reports and the exception ordering change ensures real errors are still forwarded to the gateway. The two gaps are:_emitcatches bareOSError(a superset ofBrokenPipeError), silently swallowing non-pipe I/O errors on stdout with no logging; and the new silent path has no observability — after deploy, the team can only confirm the fix worked by watching Sentry go quiet rather than monitoring a direct signal. Neither blocks correctness of the fix itself.All four
unbound.pyfiles share the same_emitpattern — any change to the OSError catch scope should be applied consistently across all of them.Important Files Changed
_emithelper and splitsexcept BrokenPipeErrorbeforeexcept Exception; core fix is correct, minor concern thatOSErrorcatch in_emitis broader than intended and the broken-pipe path has no observability_emitpattern and exception ordering as claude-code hook; same broadOSErrorcatch and silent broken-pipe path apply_emitpattern applied; copilot previously emitted{}not{"suppressOutput": true}on broken pipe — the outer BrokenPipeError handler now silently exits instead, consistent with the other hooks_emit+ broken-pipe handler; cursor-specific stderr redaction is now correctly guarded by its own try/except;handle_deny_and_exit()andsys.exit(2)still execute after a silent_emitfailure, which is acceptable since the host is gone_emitswaps stdout to devnull on dead pipe; tearDown correctly restoressys.stdoutget_api_keypatch correctly because codex reads viaos.getenv()directly (no call to aget_api_keyfunction before the guarded try block)patch.object(unbound.sys, "stderr", io.StringIO())guard in the real-exception test, correctly covering cursor's stderr redaction pathFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[main called] --> B[get api key] B --> C[stdin read] C --> D{input empty?} D -- yes --> E[_emit suppressOutput] D -- no --> F[json.loads event] F --> G{event type} G --> H[process event] H --> I[_emit response] E --> Z{BrokenPipeError in _emit?} I --> Z Z -- yes --> J[stdout = devnull, return silently - no report] Z -- no --> K[return normally] H --> Y{exception in main body?} Y -- BrokenPipeError --> L[outer BrokenPipeError handler - stdout devnull - no report] Y -- other Exception --> M[outer Exception handler - log_error then gateway - _emit suppressOutput]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[main called] --> B[get api key] B --> C[stdin read] C --> D{input empty?} D -- yes --> E[_emit suppressOutput] D -- no --> F[json.loads event] F --> G{event type} G --> H[process event] H --> I[_emit response] E --> Z{BrokenPipeError in _emit?} I --> Z Z -- yes --> J[stdout = devnull, return silently - no report] Z -- no --> K[return normally] H --> Y{exception in main body?} Y -- BrokenPipeError --> L[outer BrokenPipeError handler - stdout devnull - no report] Y -- other Exception --> M[outer Exception handler - log_error then gateway - _emit suppressOutput]Reviews (1): Last reviewed commit: "fix(hooks): stop self-reporting benign B..." | Re-trigger Greptile