From 48ef4ed5e91ce054c4c065c3e7d916367b2786d6 Mon Sep 17 00:00:00 2001 From: Felipe Date: Tue, 28 Apr 2026 20:41:45 -0300 Subject: [PATCH] =?UTF-8?q?fix(hooks):=20narrow=20inject=20matchers=20?= =?UTF-8?q?=E2=80=94=20Mac=20CPU=20fix=20D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The team settings.json hook config was wiring SessionStart, SessionEnd, TeammateIdle, and TaskCompleted with matcher='*' even though zero handlers exist for those events. PostToolUse was wired with '*' but only one handler (runtime-emit-msg, matcher /^SendMessage$/) exists. Result: every fire of any of these events caused a wasted bun cold-start to run the hook-dispatch entrypoint that did nothing. Fix: - types.ts: replace flat DISPATCHED_EVENTS array with DISPATCHED_EVENT_MATCHERS map. Only PreToolUse ('*') and PostToolUse ('SendMessage') are wired. DISPATCHED_EVENTS retained as a derived array for backward-compat. - inject.ts: buildHooksConfig() now uses per-event matchers. injectIntoFile() refactored into helpers (readSettings, allEventsAlreadyInjected, hasNoObsoleteGenieEntries, pruneObsoleteGenieEntries, refreshMatcherEntries, upsertGenieEntry) — each with single responsibility and no cognitive complexity warnings. - Pruning: cleans up SessionStart/SessionEnd/TeammateIdle/TaskCompleted from any pre-fix-D installed settings.json. - Refresh: updates PostToolUse matcher '*' → 'SendMessage' on next inject. - User-defined hooks under obsolete events are PRESERVED (only genie's own dispatch entries are pruned). Validation: 10/10 src/hooks/__tests__/inject.test.ts pass (6 prior + 4 new covering: matcher map shape, PostToolUse SendMessage narrowing, obsolete event pruning, user-hook preservation). tsc --noEmit clean. biome clean. Fix D 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: this PR - B/E: pending — needsSeed cache (lower priority post-C); session-sync cache file Co-Authored-By: Claude Opus 4.7 --- src/hooks/__tests__/inject.test.ts | 96 +++++++++++++++++++- src/hooks/inject.ts | 135 ++++++++++++++++++++--------- src/hooks/types.ts | 47 ++++++++-- 3 files changed, 227 insertions(+), 51 deletions(-) diff --git a/src/hooks/__tests__/inject.test.ts b/src/hooks/__tests__/inject.test.ts index 6f8f7497f..5b3f35b2d 100644 --- a/src/hooks/__tests__/inject.test.ts +++ b/src/hooks/__tests__/inject.test.ts @@ -4,7 +4,7 @@ import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { injectTeamHooks, isTeamHooked } from '../inject.js'; -import { DISPATCHED_EVENTS } from '../types.js'; +import { DISPATCHED_EVENTS, DISPATCHED_EVENT_MATCHERS } from '../types.js'; describe('hook injection', () => { const testDir = join(tmpdir(), `genie-hook-test-${Date.now()}`); @@ -106,4 +106,98 @@ describe('hook injection', () => { const result = await isTeamHooked('test-team'); expect(result).toBe(true); }); + + // Mac-CPU fix D — narrow matchers + drop empty events + describe('Mac-CPU fix D — narrowed matchers + dropped empty events', () => { + test('DISPATCHED_EVENT_MATCHERS only wires events that have handlers', () => { + // PreToolUse + PostToolUse are the only events with registered handlers + // (UserPromptSubmit and Stop have handlers too, but inject path is + // claude-only and those aren't currently wired through this layer) + expect(Object.keys(DISPATCHED_EVENT_MATCHERS).sort()).toEqual(['PostToolUse', 'PreToolUse']); + // Empty-handler events MUST NOT be wired + expect(DISPATCHED_EVENT_MATCHERS).not.toHaveProperty('SessionStart'); + expect(DISPATCHED_EVENT_MATCHERS).not.toHaveProperty('SessionEnd'); + expect(DISPATCHED_EVENT_MATCHERS).not.toHaveProperty('TeammateIdle'); + expect(DISPATCHED_EVENT_MATCHERS).not.toHaveProperty('TaskCompleted'); + }); + + test('PostToolUse is wired with SendMessage matcher (not "*")', async () => { + await injectTeamHooks('test-team'); + const settingsPath = join(testDir, 'teams', 'test-team', 'settings.json'); + const settings = JSON.parse(await readFile(settingsPath, 'utf-8')); + // The genie entry for PostToolUse must be SendMessage, not '*' + const postToolUseEntries = settings.hooks.PostToolUse; + expect(postToolUseEntries).toBeDefined(); + const genieEntry = postToolUseEntries.find((e: { matcher?: string }) => e.matcher === 'SendMessage'); + expect(genieEntry).toBeDefined(); + // No genie entry should have '*' matcher under PostToolUse + const wildcardGenie = postToolUseEntries.find( + (e: { matcher?: string; hooks?: Array<{ command?: string }> }) => + e.matcher === '*' && e.hooks?.some((h) => h.command?.includes('hook dispatch')), + ); + expect(wildcardGenie).toBeUndefined(); + }); + + test('injectIntoFile prunes obsolete genie entries (SessionStart, etc.) on re-inject', async () => { + const teamDir = join(testDir, 'teams', 'test-team'); + await mkdir(teamDir, { recursive: true }); + const settingsPath = join(teamDir, 'settings.json'); + + // Simulate pre-fix-D settings: SessionStart/SessionEnd/TeammateIdle/TaskCompleted + // wired with the genie dispatch command + const stalePath = '/path/to/genie/src/genie.ts'; + const staleCmd = `bun run '${stalePath}' hook dispatch`; + await writeFile( + settingsPath, + JSON.stringify({ + hooks: { + PreToolUse: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + PostToolUse: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + SessionStart: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + SessionEnd: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + TeammateIdle: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + TaskCompleted: [{ matcher: '*', hooks: [{ type: 'command', command: staleCmd, timeout: 15 }] }], + }, + }), + ); + + const result = await injectTeamHooks('test-team'); + expect(result).toBe(true); // re-injected (changes detected) + + const settings = JSON.parse(await readFile(settingsPath, 'utf-8')); + // Obsolete events with only-genie entries should be DELETED entirely + expect(settings.hooks.SessionStart).toBeUndefined(); + expect(settings.hooks.SessionEnd).toBeUndefined(); + expect(settings.hooks.TeammateIdle).toBeUndefined(); + expect(settings.hooks.TaskCompleted).toBeUndefined(); + // Active events should remain + expect(settings.hooks.PreToolUse).toBeDefined(); + expect(settings.hooks.PostToolUse).toBeDefined(); + // PostToolUse matcher must be narrowed + expect(settings.hooks.PostToolUse[0].matcher).toBe('SendMessage'); + }); + + test('injectIntoFile preserves user-defined hooks under obsolete events', async () => { + const teamDir = join(testDir, 'teams', 'test-team'); + await mkdir(teamDir, { recursive: true }); + const settingsPath = join(teamDir, 'settings.json'); + + // User has their own SessionStart hook (not genie's) — must be preserved + await writeFile( + settingsPath, + JSON.stringify({ + hooks: { + SessionStart: [{ matcher: '*', hooks: [{ type: 'command', command: 'echo user-hook', timeout: 5 }] }], + }, + }), + ); + + await injectTeamHooks('test-team'); + + const settings = JSON.parse(await readFile(settingsPath, 'utf-8')); + // User's SessionStart hook MUST survive + expect(settings.hooks.SessionStart).toBeDefined(); + expect(settings.hooks.SessionStart[0].hooks[0].command).toBe('echo user-hook'); + }); + }); }); diff --git a/src/hooks/inject.ts b/src/hooks/inject.ts index dcd521c8b..b90989071 100644 --- a/src/hooks/inject.ts +++ b/src/hooks/inject.ts @@ -10,7 +10,7 @@ import { mkdir, readFile, writeFile } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { DISPATCHED_EVENTS } from './types.js'; +import { DISPATCHED_EVENTS, DISPATCHED_EVENT_MATCHERS } from './types.js'; interface HookEntry { type: string; @@ -56,10 +56,13 @@ function buildHooksConfig(): HooksConfig { const hooks: HooksConfig = {}; const dispatchCommand = buildDispatchCommand(); - for (const event of DISPATCHED_EVENTS) { + // Mac-CPU fix D — wire each event with its declared matcher. + // Events absent from DISPATCHED_EVENT_MATCHERS are NOT wired (avoids + // useless `bun` cold-starts for events with zero handlers). + for (const [event, matcher] of Object.entries(DISPATCHED_EVENT_MATCHERS)) { hooks[event] = [ { - matcher: '*', + matcher, hooks: [ { type: 'command', @@ -74,56 +77,106 @@ function buildHooksConfig(): HooksConfig { return hooks; } -/** - * Inject genie hook dispatch into a settings.json file. - * Preserves existing non-hook settings. Overwrites existing hooks. - */ -async function injectIntoFile(settingsPath: string): Promise { - let settings: Record = {}; - - if (existsSync(settingsPath)) { - try { - const content = await readFile(settingsPath, 'utf-8'); - settings = JSON.parse(content); - } catch { - // Corrupted or empty — start fresh - } +/** Read existing settings (or start fresh on missing/corrupt). */ +async function readSettings(settingsPath: string): Promise> { + if (!existsSync(settingsPath)) return {}; + try { + return JSON.parse(await readFile(settingsPath, 'utf-8')); + } catch { + return {}; } +} - const hooksConfig = buildHooksConfig(); +/** True if every dispatched event already matches both desired matcher AND command. */ +function allEventsAlreadyInjected(existingHooks: HooksConfig, hooksConfig: HooksConfig): boolean { + return DISPATCHED_EVENTS.every((event) => { + const existing = existingHooks[event]; + const desiredCommand = hooksConfig[event][0].hooks[0].command; + const desiredMatcher = hooksConfig[event][0].matcher; + return existing?.some((m) => m.matcher === desiredMatcher && m.hooks?.some((h) => h.command === desiredCommand)); + }); +} - // Check if already injected (avoid unnecessary writes) - const existingHooks = settings.hooks as HooksConfig | undefined; - if (existingHooks) { - const allInjected = DISPATCHED_EVENTS.every((event) => { - const existing = existingHooks[event]; - const desiredCommand = hooksConfig[event][0].hooks[0].command; - return existing?.some((m) => m.hooks?.some((h) => h.command === desiredCommand)); - }); - if (allInjected) { - return false; // already injected +/** True if no obsolete events (removed from DISPATCHED_EVENT_MATCHERS) still carry a genie entry. */ +function hasNoObsoleteGenieEntries(existingHooks: HooksConfig): boolean { + return Object.keys(existingHooks).every((event) => { + if (DISPATCHED_EVENTS.includes(event as never)) return true; + const entries = existingHooks[event]; + return !entries?.some((m) => m.hooks?.some((h) => isGenieDispatchCommand(h.command))); + }); +} + +/** + * Mac-CPU fix D — prune genie-dispatch entries from events that are no + * longer in DISPATCHED_EVENT_MATCHERS (SessionStart/SessionEnd/TeammateIdle/ + * TaskCompleted). User-defined hooks under those events are preserved. + */ +function pruneObsoleteGenieEntries(mergedHooks: HooksConfig): void { + for (const event of Object.keys(mergedHooks)) { + if (DISPATCHED_EVENTS.includes(event as never)) continue; + const cleaned = (mergedHooks[event] ?? []) + .map((matcher) => ({ + ...matcher, + hooks: matcher.hooks?.filter((hook) => !isGenieDispatchCommand(hook.command)), + })) + .filter((matcher) => (matcher.hooks?.length ?? 0) > 0); + if (cleaned.length === 0) { + delete mergedHooks[event]; + } else { + mergedHooks[event] = cleaned; } } +} - // Merge genie hook entries into existing hooks (preserve user-defined hooks) - const mergedHooks: HooksConfig = existingHooks ? { ...existingHooks } : {}; - for (const event of DISPATCHED_EVENTS) { - const genieEntry = hooksConfig[event][0]; - const existingEntries = (mergedHooks[event] ?? []).map((matcher) => ({ +/** + * Refresh existing matcher entries: any matcher with a genie-dispatch hook + * inside it gets its `matcher` field rewritten to the desired value (so + * PostToolUse '*' → 'SendMessage' on next inject) and its command + timeout + * refreshed. + */ +function refreshMatcherEntries(entries: HookMatcher[], genieEntry: HookMatcher): HookMatcher[] { + return entries.map((matcher) => { + const hasGenieHook = matcher.hooks?.some((h) => isGenieDispatchCommand(h.command)); + return { ...matcher, + matcher: hasGenieHook ? genieEntry.matcher : matcher.matcher, hooks: matcher.hooks?.map((hook) => isGenieDispatchCommand(hook.command) ? { ...hook, command: genieEntry.hooks[0].command, timeout: DISPATCH_TIMEOUT } : hook, ), - })); - // Only add if not already present - const alreadyPresent = existingEntries.some((m) => m.hooks?.some((h) => isGenieDispatchCommand(h.command))); - if (!alreadyPresent) { - mergedHooks[event] = [...existingEntries, genieEntry]; - } else { - mergedHooks[event] = existingEntries; - } + }; + }); +} + +/** Add or refresh the genie entry for one event in-place on mergedHooks. */ +function upsertGenieEntry(mergedHooks: HooksConfig, event: string, genieEntry: HookMatcher): void { + const existingEntries = refreshMatcherEntries(mergedHooks[event] ?? [], genieEntry); + const alreadyPresent = existingEntries.some((m) => m.hooks?.some((h) => isGenieDispatchCommand(h.command))); + mergedHooks[event] = alreadyPresent ? existingEntries : [...existingEntries, genieEntry]; +} + +/** + * Inject genie hook dispatch into a settings.json file. + * Preserves existing non-hook settings. Overwrites existing hooks. + */ +async function injectIntoFile(settingsPath: string): Promise { + const settings = await readSettings(settingsPath); + const hooksConfig = buildHooksConfig(); + const existingHooks = settings.hooks as HooksConfig | undefined; + + if ( + existingHooks && + allEventsAlreadyInjected(existingHooks, hooksConfig) && + hasNoObsoleteGenieEntries(existingHooks) + ) { + return false; // already injected and clean — nothing to do + } + + const mergedHooks: HooksConfig = existingHooks ? { ...existingHooks } : {}; + pruneObsoleteGenieEntries(mergedHooks); + for (const event of DISPATCHED_EVENTS) { + upsertGenieEntry(mergedHooks, event, hooksConfig[event][0]); } settings.hooks = mergedHooks; diff --git a/src/hooks/types.ts b/src/hooks/types.ts index 31e43631d..09395fc1d 100644 --- a/src/hooks/types.ts +++ b/src/hooks/types.ts @@ -71,15 +71,44 @@ export interface Handler { fn: (payload: HookPayload) => Promise; } -/** The hook events that CC settings.json supports for the dispatch command. */ -export const DISPATCHED_EVENTS: HookEventName[] = [ - 'PreToolUse', - 'PostToolUse', - 'SessionStart', - 'SessionEnd', - 'TeammateIdle', - 'TaskCompleted', -]; +/** + * Hook events that CC settings.json wires to `genie hook dispatch`, mapped + * to the per-event tool-name matcher. + * + * Mac-CPU fix D — narrow matchers + drop empty events. + * + * Previous DISPATCHED_EVENTS list wired SessionStart/SessionEnd/TeammateIdle/ + * TaskCompleted with matcher='*' even though zero handlers exist for those + * events — every fire was a wasted `bun` cold-start (each start runs the full + * hook-dispatch entrypoint and PG init). Combined with PostToolUse:* (only + * `runtime-emit-msg` exists, and only matches `SendMessage`), the inject + * config was producing dozens of useless dispatcher invocations per user + * action on a busy dev machine. + * + * The matcher value is the CC-settings `matcher` field — `*` means all + * tools, otherwise an exact tool name (or pipe-separated list). + * + * To add a new dispatched event: register the handler in `index.ts` AND + * add the (event, matcher) pair here. To deprecate: remove from this map + * — `injectIntoFile` will prune existing entries on the next inject. + */ +export const DISPATCHED_EVENT_MATCHERS: Partial> = { + // PreToolUse handlers: branch-guard (Bash), orchestration-guard (Bash), + // brain-inject (.*), freshness (Read), audit-context (Write|Edit), + // identity-inject (SendMessage), auto-spawn (SendMessage), + // runtime-emit-tool (.*), session-sync-tool (.*) — broad coverage. + PreToolUse: '*', + // PostToolUse handler: runtime-emit-msg matches `^SendMessage$` only. + // Wiring '*' caused the dispatcher to run on every Bash/Read/Write/Edit + // post-use even though those produce no event — pure waste. + PostToolUse: 'SendMessage', +}; + +/** + * Convenience array — derived from DISPATCHED_EVENT_MATCHERS. + * Kept so callers that need the event list (without matcher) are unchanged. + */ +export const DISPATCHED_EVENTS: HookEventName[] = Object.keys(DISPATCHED_EVENT_MATCHERS) as HookEventName[]; const BLOCKING_EVENTS = new Set([ 'PreToolUse',