fix(dispatch): persist analytics before exit (os.Exit race)#78
Conversation
Dogfooding revealed PostToolUse analytics almost never persisted: dispatch launched `go recordAnalytics(...)` then immediately called os.Exit(), which kills the goroutine before its SQLite open+insert completes — the write lost the race every time. As a result the events table never grew via the live hook path. Now dispatch runs recordAnalytics in a goroutine but WAITS up to analyticsRecordTimeout (2s) for it before exiting; on timeout it proceeds fail-open (ARCH-1). A WAL insert is single-digit ms, so the wait is invisible in practice and only bounds a locked/slow DB so a stuck write can't hang the tool call. Verified live: after wiring hookwise into settings.json, real PostToolUse hooks now record events (events table grew 1330→1333 from this session's tool calls); a manual PostToolUse dispatch also persists. Before this fix the same dispatches recorded nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 44 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
1 participant
Summary
Dogfooding caught a real bug:
hookwise dispatchalmost never persisted analytics. It launchedgo recordAnalytics(...)then immediately calledos.Exit(), which terminates all goroutines — so the SQLite open+insert lost the race with process exit every time. The whole analytics half of hookwise was effectively dark in the live hook path.Fix
Run
recordAnalyticsin a goroutine (so a locked/slow DB can't hang the hook forever) but wait up toanalyticsRecordTimeout(2s) for it before exiting. Fail-open on timeout (ARCH-1). A WAL insert is single-digit ms, so the wait is invisible in practice.Verified live (end-to-end)
After wiring hookwise into
~/.claude/settings.json, realPostToolUsehooks now record events — the events table grew 1330 → 1333 from this session's own tool calls. A manualPostToolUsedispatch also persists (POSTPROOFrow landed). Before this fix, the identical dispatches recorded nothing. (PreToolUsecorrectly doesn't create an event row — events are recorded onPostToolUseby design.)Note on testing
The bug is a process-level race (
os.Exitvs goroutine), not unit-testable inpackage main. The verification is the live end-to-end proof above. Unit/contract gates green.Found while bringing hookwise live for daily use (the post-Dolt dogfooding effort).
🤖 Generated with Claude Code