From ca2e2d1e653784de993c4d748d4eb826a21c91ea Mon Sep 17 00:00:00 2001 From: Gemini Agent Date: Wed, 15 Apr 2026 13:15:25 +0100 Subject: [PATCH] fix(init): use canonical PostToolUse hook event (fixes #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runInit()` was writing the hook under `PostToolExecution`, which isn't a valid Claude Code hook event. Result: `claude /doctor` flagged an unknown settings key, and the hook never fired, so `events.json` stayed empty and the pet had no tool activity to react to. Also fixes the entry shape — Claude Code's canonical form is `{matcher, hooks: [{type, command}]}`, not the flat `{matcher, command}`. Changes: - `PostToolExecution` → `PostToolUse` everywhere in init/uninstall. - Writes entries in the wrapped canonical shape. - Migrates any legacy `PostToolExecution` entries onto `PostToolUse` on re-init, so existing users upgrade cleanly without losing the hook. - Uninstall sweeps both keys so lingering legacy entries are cleaned too. - Codachi-detection helper now matches both the wrapped and legacy flat shapes when deciding whether to replace vs preserve an entry. Adds `src/init.test.ts` (11 tests) covering the shape, key, idempotency, legacy migration, unrelated-hook preservation, and uninstall cleanup. Full suite: 381 tests pass (was 370). Co-Authored-By: Claude Opus 4.6 (1M context) --- node_modules/.package-lock.json | 5 +- package-lock.json | 7 +- src/init.test.ts | 183 ++++++++++++++++++++++++++++++++ src/init.ts | 67 ++++++++---- 4 files changed, 241 insertions(+), 21 deletions(-) create mode 100644 src/init.test.ts diff --git a/node_modules/.package-lock.json b/node_modules/.package-lock.json index 96c4abd..c6fec93 100644 --- a/node_modules/.package-lock.json +++ b/node_modules/.package-lock.json @@ -1,6 +1,6 @@ { "name": "codachi", - "version": "0.1.0", + "version": "0.2.0", "lockfileVersion": 3, "requires": true, "packages": { @@ -193,6 +193,7 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-25.5.0.tgz", "integrity": "sha512-jp2P3tQMSxWugkCUKLRPVUpGaL5MVFwF8RDuSRztfwgN1wmqJeMSbKlnEtQqU8UrhTmzEmZdu2I6v2dpp7XIxw==", "dev": true, + "peer": true, "dependencies": { "undici-types": "~7.18.0" } @@ -1193,6 +1194,7 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-5.4.21.tgz", "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -1274,6 +1276,7 @@ "resolved": "https://registry.npmjs.org/vitest/-/vitest-2.1.9.tgz", "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, + "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/package-lock.json b/package-lock.json index 64b38fc..dee35b5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codachi", - "version": "0.1.0", + "version": "0.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codachi", - "version": "0.1.0", + "version": "0.2.0", "license": "MIT", "bin": { "codachi": "dist/index.js", @@ -862,6 +862,7 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-25.5.0.tgz", "integrity": "sha512-jp2P3tQMSxWugkCUKLRPVUpGaL5MVFwF8RDuSRztfwgN1wmqJeMSbKlnEtQqU8UrhTmzEmZdu2I6v2dpp7XIxw==", "dev": true, + "peer": true, "dependencies": { "undici-types": "~7.18.0" } @@ -1876,6 +1877,7 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-5.4.21.tgz", "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -1957,6 +1959,7 @@ "resolved": "https://registry.npmjs.org/vitest/-/vitest-2.1.9.tgz", "integrity": "sha512-MSmPM9REYqDGBI8439mA4mWhV5sKmDlBKWIYbA3lRb2PTHACE0mgKwA8yQ2xq9vxDTuk4iPrECBAEW2aoFXY0Q==", "dev": true, + "peer": true, "dependencies": { "@vitest/expect": "2.1.9", "@vitest/mocker": "2.1.9", diff --git a/src/init.test.ts b/src/init.test.ts new file mode 100644 index 0000000..a11b424 --- /dev/null +++ b/src/init.test.ts @@ -0,0 +1,183 @@ +/** + * Tests for runInit / runUninstall — covers the settings.json shape codachi + * writes, migration of legacy PostToolExecution entries, idempotency, and + * preservation of user-configured hooks from other tools. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +let tmpHome: string; + +beforeEach(() => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'codachi-init-')); + vi.spyOn(os, 'homedir').mockReturnValue(tmpHome); + // Pretend we're running from an npx cache so detectMode() returns bin mode — + // that keeps the assertions stable across machines. + process.argv = ['node', path.join(os.tmpdir(), '_npx', 'x', 'bin', 'codachi'), 'init']; +}); + +afterEach(() => { + fs.rmSync(tmpHome, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +async function importFresh() { + vi.resetModules(); + return (await import('./init.js')) as typeof import('./init.js'); +} + +function readSettings(): Record { + return JSON.parse(fs.readFileSync(path.join(tmpHome, '.claude', 'settings.json'), 'utf8')); +} + +describe('runInit', () => { + it('writes the hook under PostToolUse (not PostToolExecution)', async () => { + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.hooks).toBeDefined(); + expect(s.hooks.PostToolUse).toBeDefined(); + expect(s.hooks.PostToolExecution).toBeUndefined(); + }); + + it('writes hook entries in the canonical {matcher, hooks: [{type, command}]} shape', async () => { + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + const entry = s.hooks.PostToolUse[0]; + expect(entry).toMatchObject({ + matcher: '', + hooks: [{ type: 'command', command: 'codachi-hook' }], + }); + }); + + it('sets the statusLine command', async () => { + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.statusLine).toEqual({ type: 'command', command: 'codachi' }); + }); + + it('migrates legacy PostToolExecution entries onto PostToolUse', async () => { + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ + hooks: { + PostToolExecution: [{ matcher: '', command: 'codachi-hook' }], + }, + }), + ); + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.hooks.PostToolExecution).toBeUndefined(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks[0].command).toBe('codachi-hook'); + }); + + it('is idempotent — running twice leaves exactly one codachi entry', async () => { + const { runInit } = await importFresh(); + runInit(); + runInit(); + const s = readSettings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + }); + + it('replaces a legacy flat-shape codachi entry with the wrapped form', async () => { + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ + hooks: { + PostToolUse: [{ matcher: '', command: 'codachi-hook' }], + }, + }), + ); + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.hooks.PostToolUse).toHaveLength(1); + expect(s.hooks.PostToolUse[0].hooks).toEqual([ + { type: 'command', command: 'codachi-hook' }, + ]); + }); + + it('preserves unrelated PostToolUse entries from other tools', async () => { + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + const otherHook = { + matcher: 'Bash', + hooks: [{ type: 'command', command: '/some/other-hook.sh' }], + }; + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ hooks: { PostToolUse: [otherHook] } }), + ); + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.hooks.PostToolUse).toHaveLength(2); + expect(s.hooks.PostToolUse).toContainEqual(otherHook); + }); + + it('preserves unrelated top-level settings keys', async () => { + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ mcpServers: { foo: { type: 'sse', url: 'http://x' } } }), + ); + const { runInit } = await importFresh(); + runInit(); + const s = readSettings(); + expect(s.mcpServers).toEqual({ foo: { type: 'sse', url: 'http://x' } }); + }); +}); + +describe('runUninstall', () => { + it('removes the codachi hook from PostToolUse', async () => { + const { runInit, runUninstall } = await importFresh(); + runInit(); + runUninstall(); + const s = readSettings(); + expect(s.hooks).toBeUndefined(); + expect(s.statusLine).toBeUndefined(); + }); + + it('also cleans up any lingering PostToolExecution entries', async () => { + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ + statusLine: { type: 'command', command: 'codachi' }, + hooks: { + PostToolExecution: [{ matcher: '', command: 'codachi-hook' }], + }, + }), + ); + const { runUninstall } = await importFresh(); + runUninstall(); + const s = readSettings(); + expect(s.hooks).toBeUndefined(); + expect(s.statusLine).toBeUndefined(); + }); + + it('preserves other hooks during uninstall', async () => { + const otherHook = { + matcher: 'Write', + hooks: [{ type: 'command', command: '/other/pre.sh' }], + }; + fs.mkdirSync(path.join(tmpHome, '.claude'), { recursive: true }); + fs.writeFileSync( + path.join(tmpHome, '.claude', 'settings.json'), + JSON.stringify({ hooks: { PreToolUse: [otherHook] } }), + ); + const { runInit, runUninstall } = await importFresh(); + runInit(); + runUninstall(); + const s = readSettings(); + expect(s.hooks.PreToolUse).toEqual([otherHook]); + expect(s.hooks.PostToolUse).toBeUndefined(); + }); +}); diff --git a/src/init.ts b/src/init.ts index 55a96cf..1c8ba3e 100644 --- a/src/init.ts +++ b/src/init.ts @@ -1,7 +1,7 @@ /** * codachi init — auto-configure Claude Code's ~/.claude/settings.json. * - * Adds a statusLine entry and a PostToolExecution hook. The commands it writes + * Adds a statusLine entry and a PostToolUse hook. The commands it writes * depend on how codachi is running: * * - When installed globally or via `npx codachi init` (argv[1] resolved to @@ -13,7 +13,8 @@ * if the clone is not on PATH. * * Idempotent: if a codachi statusLine / hook already exists, it updates in - * place rather than duplicating. + * place rather than duplicating. Also migrates any legacy PostToolExecution + * entries from pre-fix installs onto the canonical PostToolUse key. */ import fs from 'node:fs'; import path from 'node:path'; @@ -47,6 +48,20 @@ function detectMode(): { statusCmd: string; hookCmd: string; mode: 'bin' | 'loca }; } +const isCodachiCommand = (cmd: unknown): boolean => + typeof cmd === 'string' && /codachi(-hook)?|codachi[\\/]dist[\\/]hook/.test(cmd); + +const isCodachiEntry = (h: unknown): boolean => { + const hook = h as Record; + if (isCodachiCommand(hook.command)) return true; + if (Array.isArray(hook.hooks)) { + return (hook.hooks as Record[]).some( + inner => isCodachiCommand(inner.command), + ); + } + return false; +}; + export function runInit(): void { const { statusCmd, hookCmd, mode } = detectMode(); @@ -61,15 +76,30 @@ export function runInit(): void { settings.statusLine = { type: 'command', command: statusCmd }; const hooks = (settings.hooks ?? {}) as Record; - const postHooks = Array.isArray(hooks.PostToolExecution) ? hooks.PostToolExecution : []; - // Replace any existing codachi hook rather than duplicating. - const cleaned = postHooks.filter((h: unknown) => { - const hook = h as Record; - return !(typeof hook.command === 'string' && /codachi(-hook)?|codachi[\\/]dist[\\/]hook/.test(hook.command)); + // Migrate any legacy `PostToolExecution` entries (from versions before + // this fix) onto the canonical `PostToolUse` key so upgrading users don't + // end up with a stale hook that never fires. + if (Array.isArray(hooks.PostToolExecution)) { + const legacy = hooks.PostToolExecution; + delete hooks.PostToolExecution; + hooks.PostToolUse = [ + ...(Array.isArray(hooks.PostToolUse) ? hooks.PostToolUse : []), + ...legacy, + ]; + } + + const postHooks = Array.isArray(hooks.PostToolUse) ? hooks.PostToolUse : []; + + // Replace any existing codachi hook rather than duplicating. Match both + // the canonical wrapped form ({matcher, hooks: [{type, command}]}) and + // any legacy flat form ({matcher, command}). + const cleaned = postHooks.filter(h => !isCodachiEntry(h)); + cleaned.push({ + matcher: '', + hooks: [{ type: 'command', command: hookCmd }], }); - cleaned.push({ matcher: '', command: hookCmd }); - hooks.PostToolExecution = cleaned; + hooks.PostToolUse = cleaned; settings.hooks = hooks; fs.mkdirSync(path.dirname(SETTINGS_FILE), { recursive: true }); @@ -105,16 +135,17 @@ export function runUninstall(): void { changed = true; } - // Remove codachi hook from PostToolExecution. + // Remove codachi hook from PostToolUse (and any leftover PostToolExecution). const hooks = settings.hooks as Record | undefined; - if (hooks?.PostToolExecution && Array.isArray(hooks.PostToolExecution)) { - const before = hooks.PostToolExecution.length; - hooks.PostToolExecution = hooks.PostToolExecution.filter((h: unknown) => { - const hook = h as Record; - return !(typeof hook.command === 'string' && /codachi(-hook)?|codachi[\\/]dist[\\/]hook/.test(hook.command)); - }); - if (hooks.PostToolExecution.length < before) changed = true; - if (hooks.PostToolExecution.length === 0) delete hooks.PostToolExecution; + if (hooks) { + for (const key of ['PostToolUse', 'PostToolExecution'] as const) { + const entries = hooks[key]; + if (!Array.isArray(entries)) continue; + const before = entries.length; + hooks[key] = entries.filter(h => !isCodachiEntry(h)); + if (hooks[key].length < before) changed = true; + if (hooks[key].length === 0) delete hooks[key]; + } if (Object.keys(hooks).length === 0) delete settings.hooks; }