Skip to content

fix(hooks): narrow inject matchers — Mac CPU fix D #1479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
automagik-genie merged 1 commit into from
Apr 29, 2026
Merged

fix(hooks): narrow inject matchers — Mac CPU fix D #1479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 95 additions & 1 deletion src/hooks/__tests__/inject.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`);
Expand Down Expand Up @@ -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');
});
});
});
135 changes: 94 additions & 41 deletions src/hooks/inject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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',
Expand All @@ -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<boolean> {
let settings: Record<string, unknown> = {};

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<Record<string, unknown>> {
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));
});
}
Comment on lines +91 to +98

@gemini-code-assist gemini-code-assist Bot Apr 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The short-circuit logic in allEventsAlreadyInjected is too lenient. It returns true if it finds at least one matching genie entry, but it does not verify that the timeout matches DISPATCH_TIMEOUT, nor does it ensure the absence of stale genie entries (e.g., an old * matcher remaining alongside a new SendMessage matcher). This could lead to the injection process skipping necessary updates or leaving redundant dispatcher invocations in settings.json if the configuration is in a partially updated state.

function allEventsAlreadyInjected(existingHooks: HooksConfig, hooksConfig: HooksConfig): boolean {
  return DISPATCHED_EVENTS.every((event) => {
    const existing = existingHooks[event] ?? [];
    const desired = hooksConfig[event][0];
    const desiredCommand = desired.hooks[0].command;
    const desiredMatcher = desired.matcher;

    const genieEntries = existing.filter((m) => m.hooks?.some((h) => isGenieDispatchCommand(h.command)));
    if (genieEntries.length === 0) return false;

    // Ensure all genie entries for this event match the desired configuration
    return genieEntries.every((m) =>
      m.matcher === desiredMatcher &&
      m.hooks.every((h) =>
        !isGenieDispatchCommand(h.command) ||
        (h.command === desiredCommand && h.timeout === DISPATCH_TIMEOUT)
      )
    );
  });
}


// 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;

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restrict pruning to deprecated hook events only

pruneObsoleteGenieEntries currently treats every event outside DISPATCHED_EVENTS as obsolete, so a re-inject removes genie hook dispatch entries from still-supported events like UserPromptSubmit/Stop (handlers are registered in src/hooks/index.ts). In practice, teams that manually enabled dispatch for those events will silently lose those hooks the next time injectTeamHooks runs, which is a functional regression introduced by this cleanup path.

Useful? React with 👍 / 👎.

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,

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid rewriting shared matcher blocks in-place

When a matcher block contains both the genie dispatch hook and user-defined hooks, refreshMatcherEntries rewrites the entire block’s matcher to genieEntry.matcher. For PostToolUse, this changes * to SendMessage and unintentionally narrows user hooks in the same block, so their commands stop running for other tools after reinjection. The matcher update should be isolated to genie’s own entry instead of mutating shared entries.

Useful? React with 👍 / 👎.

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<boolean> {
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;

Expand Down
47 changes: 38 additions & 9 deletions src/hooks/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,44 @@ export interface Handler {
fn: (payload: HookPayload) => Promise<HandlerResult>;
}

/** 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<Record<HookEventName, string>> = {
// 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<string>([
'PreToolUse',
Expand Down
Loading