From 74fadc28d63d57f295d8591ff36e2b96c4034203 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Tue, 12 May 2026 17:18:07 +0700 Subject: [PATCH 1/4] fix(doctor): detect new Claude Code hook exec-form schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLI 2.3.1 → 2.3.2 · plugin 2.4.10 → 2.4.11 `checkSettingsHooks` was matching the legacy shell form only — looking for `"onebrain checkpoint stop"` inside the `command` field. Claude Code's current hook schema splits this into `{command: "onebrain", args: ["checkpoint", "stop"]}`, so the substring check missed every canonical entry and `/doctor` false-flagged both required hooks as missing on vaults whose `register-hooks` had already migrated them. Changes: - New `effectiveCommand()` joins `command + args[]` so a single substring check handles both schemas - New `detectHookForm()` returns `exec` / `legacy` / `absent` — legacy entries now warn "--fix will migrate to exec form" instead of being invisible - Stale-hook sweep also uses the joined effective command so a stale reference hidden inside `args[]` is no longer missed - `--fix` path unchanged: `runRegisterHooks` already migrates legacy shell-form to exec form via its existing `rewriteIfShellForm` pass - 6 new unit tests cover canonical exec, legacy shell, bash-wrapper, absent, mixed-group, and qmd-skip cases - doctor SKILL.md updated to describe the effective-command rule --- .../onebrain/.claude-plugin/plugin.json | 2 +- .../plugins/onebrain/skills/doctor/SKILL.md | 6 +- CHANGELOG.md | 10 +- PLUGIN-CHANGELOG.md | 6 +- package.json | 2 +- src/lib/index.test.ts | 147 ++++++++++++++++++ src/lib/validator.ts | 64 +++++++- 7 files changed, 224 insertions(+), 13 deletions(-) diff --git a/.claude/plugins/onebrain/.claude-plugin/plugin.json b/.claude/plugins/onebrain/.claude-plugin/plugin.json index 9d58e747..84b082ee 100644 --- a/.claude/plugins/onebrain/.claude-plugin/plugin.json +++ b/.claude/plugins/onebrain/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "onebrain", - "version": "2.4.10", + "version": "2.4.11", "description": "OneBrain — Where human and AI thinking become one. A powerful thinking partner powered by AI synergy.", "author": { "name": "OneBrain Contributors" diff --git a/.claude/plugins/onebrain/skills/doctor/SKILL.md b/.claude/plugins/onebrain/skills/doctor/SKILL.md index 63ad1a89..08e63358 100644 --- a/.claude/plugins/onebrain/skills/doctor/SKILL.md +++ b/.claude/plugins/onebrain/skills/doctor/SKILL.md @@ -112,14 +112,14 @@ Run all applicable checks based on flags (default: all). Collect findings before **OneBrain hooks:** - Read `[vault]/.claude/settings.json` (vault-level settings — the `.claude/` folder inside the vault, not `~/.claude/settings.json`) - Allowed events: only `Stop` and `PostToolUse` (the latter conditional on `qmd_collection`). -- Check required `Stop` hook: entry exists under `hooks.Stop` and command contains `checkpoint stop` → ✅ / 🔴 missing or wrong -- Sweep all other hook events (PreCompact, PostCompact, UserPromptSubmit, SessionStart, etc.): any entry whose command contains `onebrain` → 🟡 stale onebrain hook under non-allowed event — suggest running /update to remove it. Non-onebrain entries under those events are user-added and must be preserved (not flagged). +- Check required `Stop` hook: entry exists under `hooks.Stop` and its **effective command** (the `command` field joined with `args[]` by spaces — so both legacy `{command: "onebrain checkpoint stop"}` and exec-form `{command: "onebrain", args: ["checkpoint", "stop"]}` reduce to the same string) contains `onebrain checkpoint stop` → ✅ / 🔴 missing or wrong +- Sweep all other hook events (PreCompact, PostCompact, UserPromptSubmit, SessionStart, etc.): any entry whose effective command contains `onebrain` → 🟡 stale onebrain hook under non-allowed event — suggest running /update to remove it. Non-onebrain entries under those events are user-added and must be preserved (not flagged). **qmd PostToolUse hook (only when `qmd_collection` is set in vault.yml):** - If `qmd_collection` is absent in vault.yml: skip this entire check - If `qmd_collection` is present: - Check `which qmd` (macOS/Linux) or `where qmd` (Windows): qmd binary must be installed → ✅ / 🔴 "qmd not installed — qmd_collection is set but binary is missing; run `/qmd setup` to reinstall" - - Read `[vault]/.claude/settings.json` (same file used for the Stop hook); check that `hooks.PostToolUse` contains an entry whose `command` contains `qmd-reindex` → ✅ / 🔴 "PostToolUse qmd hook missing in settings.json — run /update to register" + - Read `[vault]/.claude/settings.json` (same file used for the Stop hook); check that `hooks.PostToolUse` contains an entry whose effective command (see Stop hook note above for the legacy + exec-form merge rule) contains `onebrain qmd-reindex` → ✅ / 🔴 "PostToolUse qmd hook missing in settings.json — run /update to register" ### Scheduler Health (added 2026-05-12) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a4e57a8..151c8ef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,5 @@ --- -latest_version: 2.3.1 +latest_version: 2.3.2 released: 2026-05-12 --- @@ -13,6 +13,14 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +## v2.3.2 — fix(doctor): detect new Claude Code hook exec-form schema + +- `checkSettingsHooks` now joins `command` + `args[]` into the effective command string before substring matching, so canonical exec-form hooks (`{command: "onebrain", args: ["checkpoint", "stop"]}`) are no longer false-flagged as missing +- New `detectHookForm` helper classifies each matching entry as `exec` (canonical), `legacy` (shell-form, wrapper, etc.), or `absent` — legacy entries now warn with "--fix will migrate to exec form" instead of staying invisible +- Stale-hook sweep also uses the joined effective command, so a stale `onebrain` reference hidden in `args[]` of a wrapper entry is no longer missed +- Adds 6 unit tests covering exec form, legacy shell form, bash-wrapper form, absent entries, and qmd-conditional skipping +- No behavior change for vaults already in canonical exec form (the common case post-v2.3.0) + ## v2.3.1 — feat(scheduler): hook-style command mode for direct CLI scheduling - `ScheduleEntry.command` field added: schedule any CLI binary using the same `command + args[]` shape as Claude Code hooks diff --git a/PLUGIN-CHANGELOG.md b/PLUGIN-CHANGELOG.md index 4a801fa0..e9b776c0 100644 --- a/PLUGIN-CHANGELOG.md +++ b/PLUGIN-CHANGELOG.md @@ -1,5 +1,5 @@ --- -latest_version: 2.4.10 +latest_version: 2.4.11 released: 2026-05-12 --- @@ -11,6 +11,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). > **Versioning:** Plugin version is tracked in `plugin.json`. Bump when ANY harness config changes — skills, agents, hooks, INSTRUCTIONS, Gemini settings, slash commands, etc. > For CLI binary (`@onebrain-ai/cli`) changes, see [CHANGELOG.md](CHANGELOG.md). +## v2.4.11 — 2026-05-12 + +- docs(doctor): update SKILL.md hook-check description to match new validator behavior — effective command = `command` joined with `args[]`, so canonical exec-form `{command: "onebrain", args: ["checkpoint", "stop"]}` is recognized alongside legacy shell form + ## v2.4.10 — 2026-05-12 - feat(pause): new `/pause` skill saves snapshot of long-running work to `07-logs/pause/`; non-terminal — does not clear context diff --git a/package.json b/package.json index 8ebbad28..53673070 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@onebrain-ai/cli", - "version": "2.3.1", + "version": "2.3.2", "description": "CLI for OneBrain — personal AI OS for Obsidian with persistent memory, 24+ skills, and Claude Code integration", "keywords": [ "onebrain", diff --git a/src/lib/index.test.ts b/src/lib/index.test.ts index 8bbda0d1..fce2097f 100644 --- a/src/lib/index.test.ts +++ b/src/lib/index.test.ts @@ -8,6 +8,7 @@ import { checkFolders, checkOrphanCheckpoints, checkQmdEmbeddings, + checkSettingsHooks, checkVaultYml, loadVaultConfig, } from './index.js'; @@ -401,3 +402,149 @@ describe('checkOrphanCheckpoints', () => { expect(result.message).toContain('1'); }); }); + +// --------------------------------------------------------------------------- +// checkSettingsHooks — exec/legacy schema detection +// --------------------------------------------------------------------------- + +describe('checkSettingsHooks — hook schema detection', () => { + let dir: string; + + const configWithQmd: VaultConfig = { + folders: { + inbox: '00-inbox', + projects: '01-projects', + areas: '02-areas', + knowledge: '03-knowledge', + resources: '04-resources', + agent: '05-agent', + archive: '06-archive', + logs: '07-logs', + }, + qmd_collection: 'ob-test', + checkpoint: { messages: 15, minutes: 30 }, + }; + + beforeEach(async () => { + dir = await makeTmpDir(); + await mkdir(join(dir, '.claude'), { recursive: true }); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + async function writeSettings(json: unknown): Promise { + await writeFile(join(dir, '.claude', 'settings.json'), JSON.stringify(json), 'utf8'); + } + + const allowList = ['Bash(onebrain *)']; + + it('accepts canonical exec form for Stop and qmd hooks', async () => { + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }, + ], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('ok'); + }); + + it('accepts legacy shell form for both required hooks but warns to migrate', async () => { + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [{ matcher: '', hooks: [{ command: 'onebrain checkpoint stop' }] }], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain qmd-reindex' }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('warn'); + expect(result.details?.some((d) => d.includes('Stop hook in legacy shell form'))).toBe(true); + expect( + result.details?.some((d) => d.includes('PostToolUse (qmd) hook in legacy shell form')), + ).toBe(true); + }); + + it('warns "missing" when no entry matches at all', async () => { + await writeSettings({ + permissions: { allow: allowList }, + hooks: { Stop: [{ matcher: '', hooks: [{ command: 'echo hi' }] }] }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('warn'); + expect(result.details?.some((d) => d === 'Stop hook missing')).toBe(true); + expect(result.details?.some((d) => d === 'PostToolUse (qmd) hook missing')).toBe(true); + }); + + it('treats non-canonical exec form (e.g. bash wrapper) as legacy', async () => { + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { + matcher: '', + hooks: [{ command: 'bash', args: ['-lc', 'onebrain checkpoint stop'] }], + }, + ], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('warn'); + expect(result.details?.some((d) => d.includes('Stop hook in legacy shell form'))).toBe(true); + }); + + it('skips qmd hook check when qmd_collection is absent', async () => { + const { qmd_collection: _qmd, ...rest } = configWithQmd; + const noQmd: VaultConfig = rest; + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, noQmd); + expect(result.status).toBe('ok'); + }); + + it('matches required hook even when an extra unrelated entry shares the group', async () => { + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { + matcher: '', + hooks: [ + { command: 'echo', args: ['unrelated'] }, + { command: 'onebrain', args: ['checkpoint', 'stop'] }, + ], + }, + ], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('ok'); + }); +}); diff --git a/src/lib/validator.ts b/src/lib/validator.ts index 46fcf7ea..c6666805 100644 --- a/src/lib/validator.ts +++ b/src/lib/validator.ts @@ -586,13 +586,57 @@ const REQUIRED_PERMISSION = 'Bash(onebrain *)'; const STALE_HOOK_SUBSTRINGS = ['checkpoint-hook.sh', 'session-init.sh']; interface SettingsForCheck { - hooks?: Record }>>; + hooks?: Record< + string, + Array<{ matcher?: string; hooks?: Array<{ command?: string; args?: string[] }> }> + >; permissions?: { allow?: string[] }; } -function hookPresent(settings: SettingsForCheck, event: string, cmdSubstring: string): boolean { +/** + * Effective command string for a hook entry. + * + * Tolerates both schemas Claude Code accepts: + * - legacy shell form: `{ command: "onebrain checkpoint stop" }` + * - new exec form: `{ command: "onebrain", args: ["checkpoint", "stop"] }` + * + * Both reduce to the same space-joined string, so a single substring check + * works for either. (register-hooks migrates legacy → exec, but stale + * settings.json files may still hold legacy entries until that runs.) + */ +function effectiveCommand(h: { command?: string; args?: string[] }): string { + const parts: string[] = []; + if (h.command) parts.push(h.command); + if (h.args && h.args.length > 0) parts.push(...h.args); + return parts.join(' '); +} + +/** + * Form of the matching hook entry, if any: + * - 'exec' — canonical exec form: `{ command: "onebrain", args: [...] }` + * - 'legacy' — any matching entry that is not in canonical exec form + * (shell-form, wrapper like `bash -c …`, missing args[], etc.). + * Working but should be migrated via `doctor --fix`. + * - 'absent' — no entry matches the substring. + */ +type HookForm = 'exec' | 'legacy' | 'absent'; + +const CANONICAL_HOOK_COMMAND = 'onebrain'; + +function detectHookForm( + settings: SettingsForCheck, + event: string, + cmdSubstring: string, +): HookForm { const groups = settings.hooks?.[event] ?? []; - return groups.some((g) => g.hooks?.some((h) => (h.command ?? '').includes(cmdSubstring))); + for (const g of groups) { + for (const h of g.hooks ?? []) { + if (!effectiveCommand(h).includes(cmdSubstring)) continue; + const isCanonical = h.command === CANONICAL_HOOK_COMMAND && (h.args?.length ?? 0) > 0; + return isCanonical ? 'exec' : 'legacy'; + } + } + return 'absent'; } export async function checkSettingsHooks( @@ -629,8 +673,11 @@ export async function checkSettingsHooks( // Check required hooks for (const { event, cmdSubstring } of REQUIRED_HOOKS) { - if (!hookPresent(settings, event, cmdSubstring)) { + const form = detectHookForm(settings, event, cmdSubstring); + if (form === 'absent') { warnings.push(`${event} hook missing`); + } else if (form === 'legacy') { + warnings.push(`${event} hook in legacy shell form — --fix will migrate to exec form`); } else { confirmedHooks.push(`${event} ✓`); } @@ -638,8 +685,13 @@ export async function checkSettingsHooks( // PostToolUse (qmd) — conditional on qmd_collection if (config.qmd_collection) { - if (!hookPresent(settings, 'PostToolUse', QMD_HOOK_SUBSTRING)) { + const form = detectHookForm(settings, 'PostToolUse', QMD_HOOK_SUBSTRING); + if (form === 'absent') { warnings.push('PostToolUse (qmd) hook missing'); + } else if (form === 'legacy') { + warnings.push( + 'PostToolUse (qmd) hook in legacy shell form — --fix will migrate to exec form', + ); } else { confirmedHooks.push('PostToolUse ✓'); } @@ -653,7 +705,7 @@ export async function checkSettingsHooks( const groups = settings.hooks?.[event] ?? []; for (const g of groups) { for (const h of g.hooks ?? []) { - const cmd = h.command ?? ''; + const cmd = effectiveCommand(h); if (!ALLOWED_HOOK_EVENTS.has(event) && cmd.includes(ONEBRAIN_COMMAND_SUBSTRING)) { warnings.push( `stale ${event} hook found (onebrain CLI only registers Stop + PostToolUse)`, From e3f5675959e0a694a2f91f7d8a89845045465664 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Tue, 12 May 2026 17:45:26 +0700 Subject: [PATCH 2/4] style: apply biome format to validator + tests --- CLAUDE.md | 8 ++++++++ src/lib/index.test.ts | 12 +++--------- src/lib/validator.ts | 6 +----- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bb468bea..317a5c9b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,3 +6,11 @@ All tool names in INSTRUCTIONS.md use Claude Code conventions directly. ## Agent Instructions @.claude/plugins/onebrain/INSTRUCTIONS.md + +## Spec & plan path convention (overrides superpower defaults) + +This repo is the **OneBrain CLI** working directory. When `superpowers:brainstorming` runs here, save design docs to `/01-projects/onebrain/cli/YYYY-MM-DD--design.md`. When `superpowers:writing-plans` runs here, save plans to `/01-projects/onebrain/cli/YYYY-MM-DD--plan.md`. Use date-prefixed naming (matches existing CLI history · 63 files). + +Vault location: `/Users/keng/Library/Mobile Documents/iCloud~md~obsidian/Documents/ob-1/` + +Full convention (all OneBrain projects): see `/01-projects/onebrain/shared/path-convention.md` diff --git a/src/lib/index.test.ts b/src/lib/index.test.ts index fce2097f..1ecf2f3b 100644 --- a/src/lib/index.test.ts +++ b/src/lib/index.test.ts @@ -444,9 +444,7 @@ describe('checkSettingsHooks — hook schema detection', () => { await writeSettings({ permissions: { allow: allowList }, hooks: { - Stop: [ - { matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }, - ], + Stop: [{ matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }], PostToolUse: [ { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, ], @@ -462,9 +460,7 @@ describe('checkSettingsHooks — hook schema detection', () => { permissions: { allow: allowList }, hooks: { Stop: [{ matcher: '', hooks: [{ command: 'onebrain checkpoint stop' }] }], - PostToolUse: [ - { matcher: 'Write|Edit', hooks: [{ command: 'onebrain qmd-reindex' }] }, - ], + PostToolUse: [{ matcher: 'Write|Edit', hooks: [{ command: 'onebrain qmd-reindex' }] }], }, }); @@ -515,9 +511,7 @@ describe('checkSettingsHooks — hook schema detection', () => { await writeSettings({ permissions: { allow: allowList }, hooks: { - Stop: [ - { matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }, - ], + Stop: [{ matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }], }, }); diff --git a/src/lib/validator.ts b/src/lib/validator.ts index c6666805..eadeb1f9 100644 --- a/src/lib/validator.ts +++ b/src/lib/validator.ts @@ -623,11 +623,7 @@ type HookForm = 'exec' | 'legacy' | 'absent'; const CANONICAL_HOOK_COMMAND = 'onebrain'; -function detectHookForm( - settings: SettingsForCheck, - event: string, - cmdSubstring: string, -): HookForm { +function detectHookForm(settings: SettingsForCheck, event: string, cmdSubstring: string): HookForm { const groups = settings.hooks?.[event] ?? []; for (const g of groups) { for (const h of g.hooks ?? []) { From e4e7fa014ec7d380d2551ee944374e0d4f25b8bf Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Tue, 12 May 2026 17:46:24 +0700 Subject: [PATCH 3/4] revert: drop unrelated CLAUDE.md change from format commit --- CLAUDE.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 317a5c9b..bb468bea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,11 +6,3 @@ All tool names in INSTRUCTIONS.md use Claude Code conventions directly. ## Agent Instructions @.claude/plugins/onebrain/INSTRUCTIONS.md - -## Spec & plan path convention (overrides superpower defaults) - -This repo is the **OneBrain CLI** working directory. When `superpowers:brainstorming` runs here, save design docs to `/01-projects/onebrain/cli/YYYY-MM-DD--design.md`. When `superpowers:writing-plans` runs here, save plans to `/01-projects/onebrain/cli/YYYY-MM-DD--plan.md`. Use date-prefixed naming (matches existing CLI history · 63 files). - -Vault location: `/Users/keng/Library/Mobile Documents/iCloud~md~obsidian/Documents/ob-1/` - -Full convention (all OneBrain projects): see `/01-projects/onebrain/shared/path-convention.md` From 417d1ca62a7d7703269af3d4d7f338bf4e957741 Mon Sep 17 00:00:00 2001 From: Suppaseth Charoenkarnka Date: Tue, 12 May 2026 17:54:06 +0700 Subject: [PATCH 4/4] fix(doctor): address review feedback on hook schema detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - detectHookForm now scans all matching entries per event and reports exec if any matching entry is canonical, only falling back to legacy when no canonical exists. Handles partial-migration state where a stale legacy entry was left behind alongside a new canonical one - effectiveCommand filters non-string args entries — hand-edited settings.json with stray null/numbers can't produce ghost matches - 4 new tests: partial-migration duplicates, mixed Stop+PostToolUse migration, stale exec-form event detection, defensive args filter --- CHANGELOG.md | 4 +- src/lib/index.test.ts | 96 +++++++++++++++++++++++++++++++++++++++++++ src/lib/validator.ts | 19 +++++++-- 3 files changed, 115 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 151c8ef0..f0001a97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `checkSettingsHooks` now joins `command` + `args[]` into the effective command string before substring matching, so canonical exec-form hooks (`{command: "onebrain", args: ["checkpoint", "stop"]}`) are no longer false-flagged as missing - New `detectHookForm` helper classifies each matching entry as `exec` (canonical), `legacy` (shell-form, wrapper, etc.), or `absent` — legacy entries now warn with "--fix will migrate to exec form" instead of staying invisible +- `detectHookForm` scans all matching entries per event: if any entry is in canonical exec form, the hook is reported as exec even when a legacy duplicate also matches (handles partial-migration state where a stale legacy entry was left behind alongside the new canonical one) +- `effectiveCommand` filters non-string `args[]` entries — hand-edited `settings.json` with stray `null`/numbers can't produce ghost substring matches - Stale-hook sweep also uses the joined effective command, so a stale `onebrain` reference hidden in `args[]` of a wrapper entry is no longer missed -- Adds 6 unit tests covering exec form, legacy shell form, bash-wrapper form, absent entries, and qmd-conditional skipping +- 10 unit tests covering exec, legacy shell, bash-wrapper, absent, partial migration, mixed-state Stop+PostToolUse, stale exec-form events, defensive args filtering, and qmd-conditional skipping - No behavior change for vaults already in canonical exec form (the common case post-v2.3.0) ## v2.3.1 — feat(scheduler): hook-style command mode for direct CLI scheduling diff --git a/src/lib/index.test.ts b/src/lib/index.test.ts index 1ecf2f3b..ca96813a 100644 --- a/src/lib/index.test.ts +++ b/src/lib/index.test.ts @@ -541,4 +541,100 @@ describe('checkSettingsHooks — hook schema detection', () => { const result = await checkSettingsHooks(dir, configWithQmd); expect(result.status).toBe('ok'); }); + + it('reports exec when canonical and legacy duplicates coexist (partial migration)', async () => { + // Mid-migration state: a legacy shell-form entry was left behind when a + // new canonical entry was added. The canonical one is what actually + // fires, so the check should treat the hook as exec — not legacy. + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { + matcher: '', + hooks: [ + { command: 'onebrain checkpoint stop' }, + { command: 'onebrain', args: ['checkpoint', 'stop'] }, + ], + }, + ], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('ok'); + expect(result.details?.some((d) => d.includes('legacy shell form'))).toBe(false); + }); + + it('evaluates Stop and PostToolUse independently in a mixed migration state', async () => { + // Stop already migrated to exec form, qmd still on legacy. Should fire + // exactly one warning (qmd legacy), not two. + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [{ matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }], + PostToolUse: [{ matcher: 'Write|Edit', hooks: [{ command: 'onebrain qmd-reindex' }] }], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('warn'); + expect( + result.details?.some((d) => d.includes('PostToolUse (qmd) hook in legacy shell form')), + ).toBe(true); + expect(result.details?.some((d) => d.includes('Stop hook'))).toBe(false); + }); + + it('flags a stale exec-form onebrain hook under a disallowed event', async () => { + // Before this fix, the stale-hook sweep only inspected `command`. An + // exec-form onebrain entry under PreCompact (where it has no business + // running) was masked because the verb hid in args[]. The sweep now + // uses effectiveCommand, so this stale event surfaces correctly. + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [{ matcher: '', hooks: [{ command: 'onebrain', args: ['checkpoint', 'stop'] }] }], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + PreCompact: [{ matcher: '', hooks: [{ command: 'onebrain', args: ['some-stale-verb'] }] }], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + expect(result.status).toBe('warn'); + expect(result.details?.some((d) => d.includes('stale PreCompact hook'))).toBe(true); + }); + + it('ignores non-string args entries (defensive against hand-edited settings.json)', async () => { + // settings.json is user-editable JSON, so args could carry a stray + // null/number that would otherwise spread into the joined effective + // command and produce a ghost match. + await writeSettings({ + permissions: { allow: allowList }, + hooks: { + Stop: [ + { + matcher: '', + hooks: [ + { + command: 'onebrain', + args: ['checkpoint', null as unknown as string, 'stop'], + }, + ], + }, + ], + PostToolUse: [ + { matcher: 'Write|Edit', hooks: [{ command: 'onebrain', args: ['qmd-reindex'] }] }, + ], + }, + }); + + const result = await checkSettingsHooks(dir, configWithQmd); + // Filtered effective command is `onebrain checkpoint stop` → matches. + expect(result.status).toBe('ok'); + }); }); diff --git a/src/lib/validator.ts b/src/lib/validator.ts index eadeb1f9..3717359d 100644 --- a/src/lib/validator.ts +++ b/src/lib/validator.ts @@ -606,8 +606,13 @@ interface SettingsForCheck { */ function effectiveCommand(h: { command?: string; args?: string[] }): string { const parts: string[] = []; - if (h.command) parts.push(h.command); - if (h.args && h.args.length > 0) parts.push(...h.args); + if (typeof h.command === 'string' && h.command.length > 0) parts.push(h.command); + // settings.json is user-edited JSON, so `args` may carry non-string entries + // despite the typed interface — filter defensively before joining so a stray + // null/number can't produce ghost substring matches. + if (Array.isArray(h.args)) { + for (const a of h.args) if (typeof a === 'string' && a.length > 0) parts.push(a); + } return parts.join(' '); } @@ -624,14 +629,22 @@ type HookForm = 'exec' | 'legacy' | 'absent'; const CANONICAL_HOOK_COMMAND = 'onebrain'; function detectHookForm(settings: SettingsForCheck, event: string, cmdSubstring: string): HookForm { + // Scan ALL matching entries — if any one is in canonical exec form, report + // 'exec' even when a legacy duplicate also matches. This handles partial + // migrations where a new canonical entry was added before the legacy one + // was removed: the canonical entry is what actually fires, so it should + // win the form classification. + let sawLegacy = false; const groups = settings.hooks?.[event] ?? []; for (const g of groups) { for (const h of g.hooks ?? []) { if (!effectiveCommand(h).includes(cmdSubstring)) continue; const isCanonical = h.command === CANONICAL_HOOK_COMMAND && (h.args?.length ?? 0) > 0; - return isCanonical ? 'exec' : 'legacy'; + if (isCanonical) return 'exec'; + sawLegacy = true; } } + if (sawLegacy) return 'legacy'; return 'absent'; }