fix(db): drop runRetention from getConnection — Mac CPU fix A (hook-dispatch coldstart) - #1475
Merged
Merged
Conversation
Every `genie hook dispatch` bun fork was running the full unbounded-table retention DELETEs on PG connect. With hundreds of hook forks per minute on a busy Mac dev machine, this was the dominant CPU consumer behind the .18 100%-CPU report (along with PG pool exhaustion: "sorry, too many clients already" in scheduler.log line 95912+). Fix: - Export runRetention from db.ts (was private) - Remove the runRetention call from runPostConnectSetup so hook-dispatch forks never trigger it - Wire a periodic retentionTimer in scheduler-daemon (1h cadence, fire-immediately-then-interval) - Cleanup in daemon stop() The retentionRan intra-process flag stays as a guard for the daemon's own startup-then-timer path. Validation: 43/43 src/lib/db.test.ts pass (added 3 new tests for the new contract: runRetention exported, no longer called from runPostConnectSetup, and scheduler-daemon owns the timer). This is fix A of the 5-step .19 Mac-CPU root-cause plan (A→E). A is the highest single-CPU-win, smallest-diff piece. B/C/D/E follow in separate PRs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request moves the database retention logic from the connection setup phase to a periodic timer within the scheduler-daemon. By exporting runRetention and managing its execution via a one-hour interval in the long-lived daemon process, the system avoids redundant DELETE operations during high-frequency connection events, which previously led to high CPU usage and database pool exhaustion. Tests have been updated to verify the new export and the daemon's ownership of the timer. I have no feedback to provide.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 821fcf19fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+2624
to
+2626
| const { getConnection, runRetention } = await import('./db.js'); | ||
| const sql = await getConnection(); | ||
| await runRetention(sql); |
There was a problem hiding this comment.
startDaemon is designed to run with dependency overrides, but the new retention task hard-codes import('./db.js') and calls getConnection() directly instead of deps.getConnection(). In contexts that pass mocked deps (e.g., src/lib/scheduler-daemon.test.ts startDaemon tests), this bypasses the mock and can open a real DB connection or trigger db.ts auto-start behavior (genie serve) unexpectedly, which breaks isolation and introduces side effects outside the daemon under test.
Useful? React with 👍 / 👎.
namastex888
added a commit
that referenced
this pull request
Apr 28, 2026
Every `genie hook dispatch` bun fork was running runMigrations() + needsSeed()+runSeed() on first PG connect. needsSeed() loops all 92 ~/.claude/teams entries on every call. With hundreds of hook forks per minute on a busy Mac dev machine, this was the second-largest contributor to the .18 100%-CPU regression (after fix A — the retention DELETEs). Fix: - dispatch-command.ts:dispatchAction() sets GENIE_SKIP_DB_BOOT=1 BEFORE calling dispatch(stdin), so the first getConnection() inside any handler skips migrations + seed. - db.ts:runPostConnectSetup() honors GENIE_SKIP_DB_BOOT alongside isTestMode via a unified `skipBoot` guard. Correctness: - The long-lived `genie serve` daemon owns migrations + seed at startup; short-lived hook forks must not re-run them. Schema is already there. - Handlers that need DB still get a working connection — they just skip the boot-time setup the daemon already handled. - Daemon and CLI code paths never enter dispatchAction(), so the env flag scope is correctly hook-only. Validation: 45/45 src/lib/db.test.ts pass (43 prior + 2 new for the new contract). tsc --noEmit clean. Fix C of the 5-step .19 Mac-CPU root-cause plan (A→E): - A: ✓ #1475 — drop runRetention from getConnection - B: pending — cache needsSeed via teams-mtime marker (separately) - C: this PR - D: pending — narrow hook matchers (PostToolUse=SendMessage only) - E: pending — persist session-sync cache file Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Apr 28, 2026
namastex888
added a commit
that referenced
this pull request
Apr 29, 2026
Per-process Map<executorId, sessionId> in session-sync.ts caches NOTHING across `genie hook dispatch` bun forks (each fork starts with empty Map). Result: every cold-start hook fork did 3 DB round-trips (getAgentByName + getExecutor + audit) even when the (executorId, sessionId) pair was already-reconciled by a previous fork. Fix: - Disk-backed cache at ~/.genie/cache/session-sync.json (overridable via __GENIE_SESSION_SYNC_CACHE_FILE for tests) - loadDiskCache() runs once per fork BEFORE any DB call, replacing empty Map with previously-persisted (executorId, sessionId) pairs - persistDiskCache() writes after every cache mutation (atomic via write-temp + rename) - MAX_CACHE_ENTRIES=1000 with insertion-order trim — bounded growth - Concurrency: rename races can lose one fork's writes for OTHER executor entries (benign: at most occasional redundant DB syncs from later forks) - Corrupt cache file is tolerated — falls through to DB After this PR, hook forks have ZERO DB calls in the steady state for already-reconciled sessions (only on actual rotation or first capture). Validation: 18/18 src/hooks/__tests__/session-sync.test.ts pass (14 prior + 4 new: persist-after-reconcile, cold-start-loads-from-disk, cache-miss- falls-through, corrupt-file-tolerated). tsc --noEmit clean. biome clean. Fix E of the 5-step .19 Mac-CPU root-cause plan (A→E): - A: shipped #1475 — drop runRetention from getConnection - filewatch: shipped #1474 — chokidar replacement - C: shipped #1476 — GENIE_SKIP_DB_BOOT for hook dispatch - D: open #1479 — narrow inject matchers - E: this PR - B: in flight (twin) — needsSeed mtime cache (lower priority post-C) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
namastex888
added a commit
that referenced
this pull request
Apr 29, 2026
Per-process Map<executorId, sessionId> in session-sync.ts caches NOTHING across `genie hook dispatch` bun forks (each fork starts with empty Map). Result: every cold-start hook fork did 3 DB round-trips (getAgentByName + getExecutor + audit) even when the (executorId, sessionId) pair was already-reconciled by a previous fork. Fix: - Disk-backed cache at ~/.genie/cache/session-sync.json (overridable via __GENIE_SESSION_SYNC_CACHE_FILE for tests) - loadDiskCache() runs once per fork BEFORE any DB call, replacing empty Map with previously-persisted (executorId, sessionId) pairs - persistDiskCache() writes after every cache mutation (atomic via write-temp + rename) - MAX_CACHE_ENTRIES=1000 with insertion-order trim — bounded growth - Concurrency: rename races can lose one fork's writes for OTHER executor entries (benign: at most occasional redundant DB syncs from later forks) - Corrupt cache file is tolerated — falls through to DB After this PR, hook forks have ZERO DB calls in the steady state for already-reconciled sessions (only on actual rotation or first capture). Validation: 18/18 src/hooks/__tests__/session-sync.test.ts pass (14 prior + 4 new: persist-after-reconcile, cold-start-loads-from-disk, cache-miss- falls-through, corrupt-file-tolerated). tsc --noEmit clean. biome clean. Fix E of the 5-step .19 Mac-CPU root-cause plan (A→E): - A: shipped #1475 — drop runRetention from getConnection - filewatch: shipped #1474 — chokidar replacement - C: shipped #1476 — GENIE_SKIP_DB_BOOT for hook dispatch - D: open #1479 — narrow inject matchers - E: this PR - B: in flight (twin) — needsSeed mtime cache (lower priority post-C) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
namastex888
added a commit
that referenced
this pull request
Apr 29, 2026
* fix(hooks): persist session-sync cache to disk — Mac CPU fix E Per-process Map<executorId, sessionId> in session-sync.ts caches NOTHING across `genie hook dispatch` bun forks (each fork starts with empty Map). Result: every cold-start hook fork did 3 DB round-trips (getAgentByName + getExecutor + audit) even when the (executorId, sessionId) pair was already-reconciled by a previous fork. Fix: - Disk-backed cache at ~/.genie/cache/session-sync.json (overridable via __GENIE_SESSION_SYNC_CACHE_FILE for tests) - loadDiskCache() runs once per fork BEFORE any DB call, replacing empty Map with previously-persisted (executorId, sessionId) pairs - persistDiskCache() writes after every cache mutation (atomic via write-temp + rename) - MAX_CACHE_ENTRIES=1000 with insertion-order trim — bounded growth - Concurrency: rename races can lose one fork's writes for OTHER executor entries (benign: at most occasional redundant DB syncs from later forks) - Corrupt cache file is tolerated — falls through to DB After this PR, hook forks have ZERO DB calls in the steady state for already-reconciled sessions (only on actual rotation or first capture). Validation: 18/18 src/hooks/__tests__/session-sync.test.ts pass (14 prior + 4 new: persist-after-reconcile, cold-start-loads-from-disk, cache-miss- falls-through, corrupt-file-tolerated). tsc --noEmit clean. biome clean. Fix E of the 5-step .19 Mac-CPU root-cause plan (A→E): - A: shipped #1475 — drop runRetention from getConnection - filewatch: shipped #1474 — chokidar replacement - C: shipped #1476 — GENIE_SKIP_DB_BOOT for hook dispatch - D: open #1479 — narrow inject matchers - E: this PR - B: in flight (twin) — needsSeed mtime cache (lower priority post-C) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(hooks): skip disk cache I/O in test mode (post-rebase fix-up) After rebasing fix E onto current dev (post #1479/#1482/#1483/#1484 merges), the pre-existing session-sync (Gap 2) tests started failing because loadDiskCache() on first call would read the production ~/.genie/cache/session-sync.json — populated by earlier real-mode runs of the handler. Stale (executorId, sessionId) entries caused the in-memory cache to short-circuit BEFORE the test fixtures' mocked DB calls were made, so audit-event emissions never fired. Fix: loadDiskCache() and persistDiskCache() now skip ALL disk I/O when: - ANY _deps field is non-null (test installed mocks), OR - NODE_ENV=test or BUN_ENV=test UNLESS the test explicitly set __GENIE_SESSION_SYNC_CACHE_FILE via _setCacheFileForTest (the new fix-E tests opt in this way). Mirrors the existing test-mode skip in shouldSkipSync. Validation: 18/18 src/hooks/__tests__/session-sync.test.ts pass. Both the 14 pre-existing tests and the 4 new fix-E tests work — pre-existing tests get clean cache state; fix-E tests use their isolated tmp-dir cache file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Apr 29, 2026
namastex888
added a commit
that referenced
this pull request
Apr 29, 2026
…ok entry (#1489) * docs(brainstorm): hookify-third-party-absorption design + wish (delivery #2) Crystallized brainstorm + scaffolded wish for delivery #2 of the genie hookify umbrella (delivery #1 shipped as PR #1485). Delivery #2 makes genie the only Claude Code hook entry: absorbs existing foreign hooks (Token Optimizer, ultratoken, future plugins) via a one-time settings.json rewrite + subprocess-passthrough handlers, and lets operators deploy custom hooks (e.g. rlmx run on every Bash) as plain TS code in .genie/hooks/. Three-tier scoping (per-team > per-repo > global), trust- allowlisted, with reload + test for inner-loop iteration. Council reviewed (sentinel + architect + ergonomist + operator) — all four push-backs folded into the design + wish: - Sentinel: trust allowlist required (filesystem presence is not consent), versioned absorb snapshots, two-mode env capture (probe + offline) with denylist, threat model documented (single-operator machine). - Architect: Handler interface gains version/source/manifest_path discriminated union; registry migrates const handlers to let registryRef ReadonlyArray Handler; loud shadowing instead of silent. - Ergonomist: defineHook() config-object scaffold; genie hook reload + test ship with this delivery; rename absorb to import; broken hooks loud in list. - Operator: versioned snapshots last 10; per-team archive lifecycle; 5 ms passthrough is a measured SLO with bench + alert template. Plan-review fix-loop 1 closed all 6 reviewer gaps (trust threat model, probe offline fallback, registry mutation contract, per-team archive lifecycle, Handler vNext strategy, broken-hook recovery). WISH.md plan-reviewed SHIP. 4 execution groups, 3 waves: - Wave 1: G1 foundation (registry + Handler v1 + loader + trust gate) - Wave 2 parallel: G2 operator inner loop + G3 foreign-hook absorption - Wave 3: G4 lifecycle wiring + telemetry/microbench + docs + delivery report Files: - .genie/brainstorms/hookify-third-party-absorption/DRAFT.md - .genie/brainstorms/hookify-third-party-absorption/DESIGN.md - .genie/wishes/hookify-third-party-absorption/WISH.md Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(brainstorm): patch hookify-third-party-absorption wish for Mac CPU follow-ups Three Mac CPU follow-up PRs landed on dev after delivery #1 merged (PR #1475 retention, #1476 GENIE_SKIP_DB_BOOT, #1481 narrowed matchers). Wish plan shape unchanged but two implementation realities now apply: 1. DISPATCHED_EVENTS renamed to DISPATCHED_EVENT_MATCHERS in src/hooks/types.ts and shrunk from 6 events to 2 (PreToolUse plus PostToolUse:SendMessage). Group 3's import logic must consult the new constant; foreign hooks on un-wired events (PreCompact, SessionStart, etc.) cannot be absorbed and must be left in place with explicit reporting in --dry-run. 2. Tools that invoke hook code outside genie serve set GENIE_SKIP_DB_BOOT=1 to mirror the bun-fallback path's behavior (genie hook test, scaffold validation). Added [left-in-place] reporting requirement to import --dry-run plus a new acceptance criterion covering the three-status output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 of 6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix A of the 5-step .19 Mac-CPU root-cause plan (A→E). Highest single-CPU-win, smallest diff.
Every
genie hook dispatchbun fork was running the full unbounded-table retention DELETEs on PG connect. With hundreds of hook forks per minute on a busy Mac dev machine, this was the dominant CPU consumer behind the .18 100%-CPU report — along with PG pool exhaustion ("sorry, too many clients already" in scheduler.log line 95912+ today).Root cause
db.ts:665calledrunRetentionfromrunPostConnectSetup, gated only by an in-processretentionRanflag. Since eachgenie hook dispatchinvocation is a freshbunfork, the flag resets every time → retention DELETEs fire on every PreToolUse / UserPromptSubmit / PostToolUse cold-start.What this PR does
runRetentionfromsrc/lib/db.ts(was private)runRetentioncall fromrunPostConnectSetupso hook-dispatch forks never trigger retentionsrc/lib/scheduler-daemon.ts: 1h cadence, fires immediately on daemon startup, dynamically imports + callsrunRetention. Cleanup instop()src/lib/db.test.tswith 3 new tests asserting the new contract (runRetention exported, no longer in runPostConnectSetup, daemon owns the timer)The
retentionRanintra-process flag stays as a guard for the daemon's own startup-then-timer path.Validation
bun test src/lib/db.test.ts— 43/43 pass (40 original + 3 new)bun x tsc --noEmitcleanCompanion PRs in this Mac-CPU sprint
fs.watch(partial mitigation; FSEvents fanout)needsSeedvia teams-mtime marker (avoid 92-team loop on every fork)GENIE_SKIP_DB_BOOTenv for hook-dispatch entry point (skip migrations + seed entirely)PostToolUse=SendMessageonly, drop empty events)Mitigation guidance for Mac users until .19 cuts
genie serve stop # kills the daemon — no scheduler/capture, but stops the freeze(Note: the bug exists on
@latest4.260423.10 too — it's not unique to .18. Pinning Mac users to @latest does NOT help.)