fix: reliably flush hook telemetry before process exit#515
Conversation
The hook binary is short-lived — bin/failproofai.mjs calls process.exit() the moment handleHookEvent returns — so events fired with un-awaited `void trackHookEvent(...)` (custom_hooks_loaded, convention_policies_loaded, the *_error events) were killed mid-flight and never reached PostHog. They only survived when a deny/instruct decision happened to add a trailing await; on the common allow path they were dropped entirely (verified: an allowed tool call sent ZERO events to a local capture sink). hook-telemetry.ts now tracks every in-flight POST in a pending set and exposes flushHookTelemetry(), which handleHookEvent awaits before returning (and bin's hook error path drains on throw). Delivery is now reliable regardless of decision. No change to which events fire — allow decisions still send nothing by design. Verified end-to-end against a local sink: the allow path now delivers its events, and all 12 CLIs emit hook_policy_triggered tagged with the correct cli on a deny. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Automated code review started - full review. Results will be posted here. |
|
Closing — folding this telemetry fix into the combined TUI + PostHog PR instead of shipping it standalone. |
2 participants
Problem
The hook binary is short-lived —
bin/failproofai.mjscallsprocess.exit()the momenthandleHookEventreturns. Events fired with un-awaitedvoid trackHookEvent(...)(custom_hooks_loaded,convention_policies_loaded, the*_errorevents) were killed mid-flight and never reached PostHog. They only survived when adeny/instructdecision happened to add a trailingawait.Verified: an allowed tool call through the installed binary sent zero events to a local capture sink — the loads/errors were dropped by
process.exit. This is the "telemetry not sending regularly" symptom.Fix
hook-telemetry.tsnow tracks every in-flight POST in a pending set and exposesflushHookTelemetry().handleHookEventawaits it before returning;bin's hook error path drains on throw. Delivery is now reliable regardless of decision.No coverage change — allow decisions still send nothing by design; this only makes the events that already fire land reliably.
Verification
flushHookTelemetryunit tests: awaits an un-awaited send, no-op when idle, never throws on reject.hook_policy_triggeredtagged with the correctclion a deny.Note: local Next.js build can't run inside a nested git worktree (Turbopack workspace-root limitation); the CLI bundle builds fine and CI runs the full build on a clean checkout.
🤖 Generated with Claude Code