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..f0001a97 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,16 @@ 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 +- `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 +- 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 - `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..ca96813a 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,239 @@ 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'); + }); + + 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 46fcf7ea..3717359d 100644 --- a/src/lib/validator.ts +++ b/src/lib/validator.ts @@ -586,13 +586,66 @@ 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 (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(' '); +} + +/** + * 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 { + // 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] ?? []; - 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; + if (isCanonical) return 'exec'; + sawLegacy = true; + } + } + if (sawLegacy) return 'legacy'; + return 'absent'; } export async function checkSettingsHooks( @@ -629,8 +682,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 +694,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 +714,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)`,