From b9d15e96955e745fcf397fb9c2465a6d74510380 Mon Sep 17 00:00:00 2001 From: Ousama Benyounes Date: Tue, 22 Sep 2026 21:53:38 +0000 Subject: [PATCH 1/4] fix(hooks): list the built-in hooks each tool really receives `hooks list` rendered builtinHookDefs('claude') for every tool, so the built-in block described Claude's hook set no matter which tool the row was for: Copilot's SessionEnd hook never appeared, while tools that receive no hooks at all were credited with six. The displayed set is now derived from what each tool actually receives through reconciliation, and a tool with no hook surface is omitted rather than shown an invented list. Fixes #717 --- docs/usage-guide.md | 2 + docs/usage-guide.zh-CN.md | 2 + src/__tests__/hooks-cmd.test.ts | 140 +++++++++++++++++++++++++-- src/__tests__/openclaw-hooks.test.ts | 34 +++++++ src/builtin-hooks.ts | 62 ++++++++++++ src/hooks-cmd.ts | 56 +++++++++-- src/hooks.ts | 23 +++++ 7 files changed, 299 insertions(+), 20 deletions(-) diff --git a/docs/usage-guide.md b/docs/usage-guide.md index 81767a5d..ccfd03aa 100644 --- a/docs/usage-guide.md +++ b/docs/usage-guide.md @@ -1409,6 +1409,8 @@ teamai hooks inject # Re-inject teamai hooks remove # Remove ``` +`hooks list` prints the built-in set per tool, because the set is not universal: Copilot also gets `SessionEnd`, OMP's extension covers four events without the `Skill` / `TodoWrite` matchers, OpenClaw maps only `SessionStart` + `UserPromptSubmit`, and Hermes only `SessionStart`. Tools the hook pipeline installs nothing for (e.g. JoyCode) are omitted, and so is Kiro — its `SessionStart` command is embedded as `hooks.agentSpawn` by the agent sync, so it exists only for the agents you actually synced. + The inject and remove commands only touch tools you actually have installed (i.e. whose `~/./` root directory already exists). They never create root directories for tools listed in `toolPaths` but not installed. On Windows, the built-in hook dispatch commands that shell out through bash (e.g. Claude, Codex, Cursor, Copilot CLI) reference Git Bash by absolute path — standard install locations first, then the `HKLM\SOFTWARE\GitForWindows` registry as fallback — so they never resolve to the WSL `bash.exe` launcher; if Git Bash cannot be found they degrade to bare `bash`. diff --git a/docs/usage-guide.zh-CN.md b/docs/usage-guide.zh-CN.md index 504a93f9..75977e3d 100644 --- a/docs/usage-guide.zh-CN.md +++ b/docs/usage-guide.zh-CN.md @@ -1369,6 +1369,8 @@ teamai hooks inject # 重新注入 teamai hooks remove # 移除 ``` +`hooks list` 按工具分别列出内置 hooks，因为各工具的集合并不相同：Copilot 额外有 `SessionEnd`，OMP 扩展覆盖四个事件且没有 `Skill` / `TodoWrite` matcher，OpenClaw 只映射 `SessionStart` + `UserPromptSubmit`，Hermes 只有 `SessionStart`。hook 注入流程不会为其安装任何内置 hook 的工具（如 JoyCode）不会列出；Kiro 也不列出——它的 `SessionStart` 由 agent 同步以 `hooks.agentSpawn` 形式内嵌，只存在于你实际同步过的 agent 中。 + inject 和 remove 只会操作你实际已安装的工具（即 `~/./` 根目录已存在的工具）。对于 `toolPaths` 中已配置但未安装的工具，命令不会为其凭空创建根目录。 在 Windows 上，经由 bash 执行的内置 hook 派发命令（如 Claude、Codex、Cursor、Copilot CLI）会以绝对路径引用 Git Bash——先查标准安装位置，再回退到 `HKLM\SOFTWARE\GitForWindows` 注册表——从而避免解析到 WSL 的 `bash.exe`；若找不到 Git Bash，则退回裸 `bash`。 diff --git a/src/__tests__/hooks-cmd.test.ts b/src/__tests__/hooks-cmd.test.ts index 79a7d894..106fc6a4 100644 --- a/src/__tests__/hooks-cmd.test.ts +++ b/src/__tests__/hooks-cmd.test.ts @@ -22,7 +22,7 @@ vi.mock('../hooks.js', async () => { }); vi.mock('../resources/hooks.js', () => ({ - parseTeamHooks: vi.fn(), + parseTeamHooksConfig: vi.fn(), })); vi.mock('../utils/logger.js', () => ({ @@ -39,7 +39,7 @@ vi.mock('../utils/logger.js', () => ({ import { autoDetectInit } from '../config.js'; import { getHookStatus, reconcileHooks, reconcileHooksToAllTools, reconcileTeamHooksForConfig, sweepLegacyProjectHooks, hasInstalledCodexTrustGatedTool } from '../hooks.js'; -import { parseTeamHooks } from '../resources/hooks.js'; +import { parseTeamHooksConfig } from '../resources/hooks.js'; import { log } from '../utils/logger.js'; import { hooksInject, hooksRemove, hooksList } from '../hooks-cmd.js'; import { TeamaiConfigSchema } from '../types.js'; @@ -51,7 +51,12 @@ const mockedReconcileStandalone = reconcileHooks as Mock; const mockedReconcile = reconcileHooksToAllTools as Mock; const mockedReconcileForConfig = reconcileTeamHooksForConfig as Mock; const mockedHasCodexTrustGated = hasInstalledCodexTrustGatedTool as Mock; -const mockedParseTeamHooks = parseTeamHooks as Mock; +const mockedParseTeamHooks = parseTeamHooksConfig as Mock; + +/** hooks.yaml parse result: team defs (B) plus the optional builtin override. */ +function hooksYaml(defs: unknown[], builtin?: unknown) { + return { defs, builtin }; +} const mockedLog = log as unknown as { info: Mock; success: Mock; warn: Mock; error: Mock; debug: Mock }; const mockLocalConfig = { @@ -85,6 +90,19 @@ function copilotConfig() { }; } +/** Hook lines listed under `:` in the built-in (A) block. */ +function builtinBlock(text: string, tool: string): string[] | undefined { + const lines = text.split('\n'); + const start = lines.findIndex((l) => /^ {2}\S/.test(l) && l.trim().slice(0, -1).split(', ').includes(tool)); + if (start === -1) return undefined; + const block: string[] = []; + for (const line of lines.slice(start + 1)) { + if (!line.startsWith(' ')) break; + block.push(line.trim()); + } + return block; +} + function mockHome(home: string): () => void { const originalHome = process.env.HOME; process.env.HOME = home; @@ -102,7 +120,7 @@ beforeEach(() => { mockedReconcile.mockResolvedValue(undefined); mockedReconcileForConfig.mockResolvedValue(undefined); mockedHasCodexTrustGated.mockResolvedValue(false); - mockedParseTeamHooks.mockResolvedValue(TEAM_DEFS); + mockedParseTeamHooks.mockResolvedValue(hooksYaml(TEAM_DEFS)); }); describe('hooksInject', () => { @@ -179,9 +197,9 @@ describe('hooksInject', () => { describe('hooksList', () => { it('prints built-in hooks and team hooks from hooks.yaml', async () => { - mockedParseTeamHooks.mockResolvedValue([ + mockedParseTeamHooks.mockResolvedValue(hooksYaml([ { source: 'team', key: 'lint', event: 'Stop', command: 'npm run lint', description: '[teamai:hook:lint] lint', tools: ['claude'] }, - ]); + ])); const out: string[] = []; const spy = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); try { @@ -199,10 +217,10 @@ describe('hooksList', () => { }); it('prints the roles restriction next to the tools one', async () => { - mockedParseTeamHooks.mockResolvedValue([ + mockedParseTeamHooks.mockResolvedValue(hooksYaml([ { source: 'team', key: 'guard-tf', event: 'PreToolUse', matcher: 'Bash', command: 'guard-tf.sh', description: '[teamai:hook:guard-tf] x', roles: ['devops'] }, { source: 'team', key: 'lint', event: 'Stop', command: 'npm run lint', description: '[teamai:hook:lint] lint' }, - ]); + ])); const out: string[] = []; const spy = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); try { @@ -216,11 +234,11 @@ describe('hooksList', () => { }); it('prints the projects restriction next to the roles one', async () => { - mockedParseTeamHooks.mockResolvedValue([ + mockedParseTeamHooks.mockResolvedValue(hooksYaml([ { source: 'team', key: 'checkout-lint', event: 'Stop', command: 'echo checkout', description: '[teamai:hook:checkout-lint] x', projects: ['checkout'] }, { source: 'team', key: 'both', event: 'Stop', command: 'echo both', description: '[teamai:hook:both] x', roles: ['frontend'], projects: ['checkout', 'billing'] }, { source: 'team', key: 'nobody', event: 'Stop', command: 'echo none', description: '[teamai:hook:nobody] x', projects: [] }, - ]); + ])); const out: string[] = []; const spy = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); try { @@ -457,6 +475,108 @@ describe('hooksList', () => { 'copilot', ); }); + + it('prints the built-in hook set of each listed tool, including Copilot SessionEnd', async () => { + const originalCopilotHome = process.env.COPILOT_HOME; + process.env.COPILOT_HOME = COPILOT_HOME_FIXTURE; + const out: string[] = []; + const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); + mockedAutoDetectInit.mockResolvedValue({ + localConfig: { ...mockLocalConfig, enabledAgents: ['copilot'] }, + teamConfig: copilotConfig(), + }); + + try { + await hooksList({}); + } finally { + if (originalCopilotHome === undefined) delete process.env.COPILOT_HOME; + else process.env.COPILOT_HOME = originalCopilotHome; + consoleLog.mockRestore(); + } + + // Copilot's built-in set carries an extra SessionEnd entry that no other + // tool has; listing a hardcoded tool's set hides it even though inject + // really installs it. + const text = out.join('\n'); + expect(text).toContain(' copilot:'); + expect(text).toContain('SessionEnd'); + expect(text).toContain('hook-dispatch session-end'); + }); + + it('hides a built-in hook the team disabled in hooks.yaml', async () => { + // The reconcile engine applies `builtin.disabled`, so a hook listed + // here that is no longer in the settings file would be a lie. + mockedParseTeamHooks.mockResolvedValue(hooksYaml([], { disabled: ['Hook dispatch stop'] })); + const out: string[] = []; + const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); + + try { + await hooksList({}); + } finally { + consoleLog.mockRestore(); + } + + const text = out.join('\n'); + const builtin = text.slice(text.indexOf('Built-in hooks (A)'), text.indexOf('Team hooks (B)')); + const claude = builtinBlock(builtin, 'claude') ?? []; + expect(claude).toHaveLength(5); + expect(claude.join('\n')).not.toContain('Stop →'); + }); + + it('lists only the built-in hooks a tool actually receives (#717)', async () => { + const out: string[] = []; + const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); + mockedAutoDetectInit.mockResolvedValue({ + localConfig: mockLocalConfig, + teamConfig: { + toolPaths: { + claude: { settings: '.claude/settings.json', skills: '.claude/skills' }, + // Standalone adapters: each covers a narrower slice of the + // built-in set than the settings-file tools. + hermes: { skills: '.hermes/skills' }, + omp: { skills: '.omp/skills' }, + openclaw: { skills: '.openclaw/skills' }, + // No built-in hook from the hook pipeline. + joycode: { skills: '.joycode/skills' }, + kiro: { skills: '.kiro/skills', agents: '.kiro/agents' }, + }, + }, + }); + + try { + await hooksList({}); + } finally { + consoleLog.mockRestore(); + } + + const text = out.join('\n'); + const builtin = text.slice(text.indexOf('Built-in hooks (A)'), text.indexOf('Team hooks (B)')); + + // Claude is reconciled through its settings file: the whole set. + expect(builtinBlock(builtin, 'claude')).toHaveLength(6); + // Hermes installs a single on_session_start script running the raw + // dispatch command (hermes-hooks.ts). + expect(builtinBlock(builtin, 'hermes')).toEqual([ + 'SessionStart → teamai hook-dispatch session-start --tool >/dev/null 2>&1 || true', + ]); + // OMP's extension subscribes to four events and has no matcher-scoped + // PostToolUse pass (omp-hooks.ts). + const omp = builtinBlock(builtin, 'omp') ?? []; + expect(omp).toHaveLength(4); + expect(omp.join('\n')).not.toContain('[Skill]'); + expect(omp.join('\n')).not.toContain('[TodoWrite]'); + // OpenClaw's handler maps session:start and command:new only + // (openclaw-hooks.ts EVENT_MAP). + expect(builtinBlock(builtin, 'openclaw')).toEqual([ + 'SessionStart → teamai hook-dispatch session-start --tool ', + 'UserPromptSubmit → teamai hook-dispatch prompt-submit --tool ', + ]); + // JoyCode has no hook surface, and Kiro's session-start command is + // embedded per agent by the agent sync instead of the hook pipeline: + // neither may be listed. + expect(builtinBlock(builtin, 'joycode')).toBeUndefined(); + expect(builtinBlock(builtin, 'kiro')).toBeUndefined(); + }); }); describe('hooksRemove', () => { diff --git a/src/__tests__/openclaw-hooks.test.ts b/src/__tests__/openclaw-hooks.test.ts index aafaf684..4e8c1468 100644 --- a/src/__tests__/openclaw-hooks.test.ts +++ b/src/__tests__/openclaw-hooks.test.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; import { injectOpenClawHooks, removeOpenClawHooks, OPENCLAW_HOOK_DIR } from '../openclaw-hooks.js'; +import { reconcileHooksToAllTools } from '../hooks.js'; let tmpDir: string; let wsDir: string; @@ -75,3 +76,36 @@ describe('removeOpenClawHooks', () => { await expect(removeOpenClawHooks(hooksDir)).resolves.toBeUndefined(); }); }); + +describe('reconcileHooksToAllTools routes the OpenClaw family to its adapter', () => { + // `hooks inject` / `init` / `pull` all go through this path. Without an + // OpenClaw branch it skipped the claw variants for lack of a `settings` + // path, so their hooks were only ever written by the legacy migration. + const toolPaths = { openclaw: { skills: '.openclaw/skills' } } as Record; + + it('injects, then removeAll deletes, the workspace hook dir', async () => { + const manifest = path.join(tmpDir, 'managed-hooks.json'); + const hookDir = path.join(wsDir, 'hooks', OPENCLAW_HOOK_DIR); + + await reconcileHooksToAllTools(toolPaths, tmpDir, [], manifest); + expect(fs.existsSync(path.join(hookDir, 'handler.ts'))).toBe(true); + + await reconcileHooksToAllTools(toolPaths, tmpDir, [], manifest, { removeAll: true }); + expect(fs.existsSync(hookDir)).toBe(false); + }); + + it('does nothing when the workspace cannot be resolved', async () => { + delete process.env.OPENCLAW_STATE_DIR; + const home = path.join(tmpDir, 'empty-home'); + fs.mkdirSync(home, { recursive: true }); + const prevHome = process.env.HOME; + process.env.HOME = home; + try { + await reconcileHooksToAllTools(toolPaths, home, [], path.join(tmpDir, 'managed-hooks.json')); + } finally { + if (prevHome === undefined) delete process.env.HOME; + else process.env.HOME = prevHome; + } + expect(fs.existsSync(path.join(home, '.openclaw'))).toBe(false); + }); +}); diff --git a/src/builtin-hooks.ts b/src/builtin-hooks.ts index 59e56077..05793928 100644 --- a/src/builtin-hooks.ts +++ b/src/builtin-hooks.ts @@ -398,6 +398,68 @@ export function builtinHookDefs(tool: string): HookDef[] { })); } +/** + * Built-in hooks each non-settings tool really receives from the hook + * reconciliation pipeline, and how the installed artifact runs them. + * + * Tools driven by a settings/hooks file get the full `builtinHookDefs(tool)` + * set through `reconcileHooks`. The adapters below own their own format, each + * cover a narrower slice, and spawn the dispatcher directly — so the shell + * wrapper the settings tools carry would misreport what is on disk. + * + * A tool in neither place is not listed: JoyCode has no hook surface at all, + * and Kiro's session-start command is embedded per agent by the agent sync + * (`renderForKiro`), so it exists only for agents that were actually synced + * rather than coming from the hook pipeline. + */ +const ADAPTER_BUILTIN_HOOKS: Record = { + // hermes-hooks.ts registers one on_session_start script whose single line is + // the dispatch command with errors swallowed (buildReportScript). + hermes: { keys: ['Hook dispatch session-start'], suffix: ' >/dev/null 2>&1 || true' }, + // omp-hooks.ts subscribes to four OMP extension events and spawns the + // dispatcher with argv; `tool_result` carries no matcher, so the Skill / + // TodoWrite passes do not exist there. + omp: { + keys: [ + 'Hook dispatch session-start', + 'Hook dispatch stop', + 'Hook dispatch post-tool-use wildcard', + 'Hook dispatch prompt-submit', + ], + }, + // opencode-hooks.ts covers the same four events plus the matcher-scoped + // post-tool-use passes (TOOL_MATCHER), i.e. the whole built-in set. + opencode: { keys: BUILTIN_HOOK_SPECS.map((spec) => spec.key) }, + // openclaw-hooks.ts EVENT_MAP maps session:start and command:new only, and + // its generated handler spawns the dispatcher with argv. Only `openclaw`: + // the other claw variants share its workspace resolver, so reconciliation + // does not route them (see reconcileHooksToAllTools). + openclaw: { keys: ['Hook dispatch session-start', 'Hook dispatch prompt-submit'] }, +}; + +/** + * Built-in hook definitions a tool actually receives, for reporting + * (`teamai hooks list`). + * + * `settingsDriven` tools go through the settings-file reconcile path and get + * the full set; the others are limited to what their own adapter installs, and + * a tool the pipeline never installs a built-in hook for gets an empty list so + * callers can omit it instead of advertising hooks it never receives (#717). + */ +export function installedBuiltinHookDefs(tool: string, settingsDriven: boolean): HookDef[] { + if (settingsDriven) return builtinHookDefs(tool); + const adapter = ADAPTER_BUILTIN_HOOKS[tool]; + if (!adapter) return []; + return BUILTIN_HOOK_SPECS.filter((spec) => adapter.keys.includes(spec.key)).map((spec) => ({ + source: 'builtin' as const, + key: spec.key, + event: spec.event, + matcher: spec.matcher, + command: getRawDispatchCommand(spec.dispatchEvent, tool, spec.matcher) + (adapter.suffix ?? ''), + description: `${TEAMAI_HOOK_DESCRIPTION_PREFIX} ${spec.key}`, + })); +} + /** §4.8 team override of built-in hooks. Only whitelisted fields are honored. */ export interface BuiltinHookOverride { /** Built-in hook keys to disable (drop entirely). */ diff --git a/src/hooks-cmd.ts b/src/hooks-cmd.ts index 98915ea4..7afa3b2c 100644 --- a/src/hooks-cmd.ts +++ b/src/hooks-cmd.ts @@ -1,10 +1,10 @@ import path from 'node:path'; import { autoDetectInit } from './config.js'; import { reconcileHooks, reconcileHooksToAllTools, reconcileTeamHooksForConfig, sweepLegacyProjectHooks, getHookStatus, hasInstalledCodexTrustGatedTool, codexTrustReminder, type HookStatus } from './hooks.js'; -import { builtinHookDefs } from './builtin-hooks.js'; -import { parseTeamHooks } from './resources/hooks.js'; +import { applyBuiltinOverride, installedBuiltinHookDefs } from './builtin-hooks.js'; +import { parseTeamHooksConfig } from './resources/hooks.js'; import { log } from './utils/logger.js'; -import type { GlobalOptions } from './types.js'; +import type { GlobalOptions, HookDef } from './types.js'; import { COPILOT_TOOL_ID, getManagedHooksPath, @@ -22,6 +22,8 @@ interface HookListRow { tool: string; status: HookListStatus; settingsPath: string; + /** Built-in hooks this tool really receives (empty = no hook surface). */ + builtinDefs: HookDef[]; } function formatDisplayPath(settingsPath: string): string { @@ -95,6 +97,10 @@ export async function hooksList(_options: GlobalOptions): Promise { // `~/.qoder-cn` vs `/.qoder`) would otherwise be probed in the *other* // build's file and always reported missing. const hookScopedPaths = scopedToolPaths(teamConfig, { ...localConfig, scope: hookScope }); + // The team's `builtin:` block can disable built-in hooks (§4.8); the + // reconcile engine applies it, so the listing must too or it shows hooks + // that were just removed from the settings files. + const { defs: teamDefs, builtin: builtinOverride } = await parseTeamHooksConfig(localConfig.repo.localPath); const rows: HookListRow[] = []; // One settings file is one install, so list it once, for the target that owns // it — the same rule the write path applies. Qoder CN shares Qoder's project @@ -131,29 +137,59 @@ export async function hooksList(_options: GlobalOptions): Promise { tool, status: await pathExists(extFile) ? 'installed' : 'missing', settingsPath: formatDisplayPath(extFile), + builtinDefs: installedBuiltinHookDefs(tool, false), }); continue; } if (!hookPath) { - rows.push({ tool, status: 'not configured', settingsPath: 'no settings configured' }); + rows.push({ + tool, + status: 'not configured', + settingsPath: 'no settings configured', + builtinDefs: installedBuiltinHookDefs(tool, false), + }); continue; } rows.push({ tool, status: await getHookStatus(hookPath, tool), settingsPath: formatDisplayPath(hookPath), + builtinDefs: applyBuiltinOverride(installedBuiltinHookDefs(tool, true), builtinOverride), }); } console.log(formatHooksList(rows)); - const teamDefs = await parseTeamHooks(localConfig.repo.localPath); - console.log(''); - console.log('Built-in hooks (A) — teamai operational (injected into every tool):'); - for (const d of builtinHookDefs('claude')) { - const matcher = d.matcher && d.matcher !== '*' ? ` [${d.matcher}]` : ''; - console.log(` ${d.event}${matcher} → ${d.command}`); + console.log('Built-in hooks (A) — teamai operational, per tool:'); + // The built-in set is per tool, not universal: Copilot carries an extra + // SessionEnd entry, the dispatch command differs for ZCode (raw) and the + // shell-dependent GUI tools (PATH wrapper), and the standalone adapters + // (Hermes, OMP, OpenClaw) install only part of the set. Rendering one + // hardcoded tool's set both hid hooks that `hooks inject` really installs + // and advertised hooks tools without that surface never receive (#717), so + // tools with no built-in hooks at all are omitted here. Tools whose set is + // identical once the tool id is folded out share one block, so the listing + // stays short instead of repeating the same rows per tool. + const builtinGroups = new Map(); + for (const { tool, builtinDefs } of rows) { + if (builtinDefs.length === 0) continue; + const lines = builtinDefs.map((d) => { + const matcher = d.matcher && d.matcher !== '*' ? ` [${d.matcher}]` : ''; + const command = d.command.split(`--tool ${tool}`).join('--tool '); + return ` ${d.event}${matcher} → ${command}`; + }); + const key = lines.join('\n'); + const group = builtinGroups.get(key); + if (group) group.tools.push(tool); + else builtinGroups.set(key, { tools: [tool], lines }); + } + if (builtinGroups.size === 0) { + console.log(' (none)'); + } + for (const group of builtinGroups.values()) { + console.log(` ${group.tools.join(', ')}:`); + for (const line of group.lines) console.log(line); } console.log(''); diff --git a/src/hooks.ts b/src/hooks.ts index 59175a67..2c99138e 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1482,6 +1482,29 @@ export async function reconcileHooksToAllTools( } continue; } + // OpenClaw has no settings hook list either: its hook is a HOOK.md + + // handler.ts pair under the resolved workspace dir. Route it to that + // adapter, which no-ops when the workspace cannot be resolved, so an + // uninstalled OpenClaw never grows a config dir. Only `openclaw` itself: + // resolveOpenclawWorkspaceDir resolves the OpenClaw workspace, so routing + // the other claw variants here would make them overwrite that one handler + // with each other's --tool value. + if (tool === 'openclaw') { + if (opts.settingsOnly) continue; + try { + if (opts.removeAll) { + const { removeOpenClawHooks, resolveOpenclawWorkspaceDir } = await import('./openclaw-hooks.js'); + const wsDir = await resolveOpenclawWorkspaceDir(); + if (wsDir) await removeOpenClawHooks(path.join(wsDir, 'hooks')); + } else { + const { injectOpenClawHooks } = await import('./openclaw-hooks.js'); + await injectOpenClawHooks(undefined, tool); + } + } catch (e) { + log.warn(`Failed to reconcile OpenClaw hooks for ${tool}: ${(e as Error).message}`); + } + continue; + } // OpenCode has no settings.json hook list; it auto-loads JS/TS plugins from // its config dirs. Route it to the plugin-file adapter instead of the // settings-based path. From b2923768867af07b0ac23e1d67815734e4d69dd8 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Tue, 22 Sep 2026 22:04:37 +0000 Subject: [PATCH 2/4] fix(hooks): report adapter-driven tools by their generated artifact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hooks list` probed a settings file per tool, so Hermes, OpenCode and OpenClaw — which reconciliation installs as one generated script/plugin/ handler each — fell through to the generic branch and were reported as "not configured" even right after `hooks inject` wrote their hook. OMP already had a bespoke branch for this. Resolve that artifact per adapter and use its presence as the status, the same rule OMP used, so the status column matches the built-in block. Co-Authored-By: Claude Opus 5 --- src/__tests__/hooks-cmd.test.ts | 25 +++++++++++++++++ src/hermes-hooks.ts | 2 +- src/hooks-cmd.ts | 49 +++++++++++++++++++++++++++------ 3 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/__tests__/hooks-cmd.test.ts b/src/__tests__/hooks-cmd.test.ts index 106fc6a4..eccee073 100644 --- a/src/__tests__/hooks-cmd.test.ts +++ b/src/__tests__/hooks-cmd.test.ts @@ -523,6 +523,31 @@ describe('hooksList', () => { expect(claude.join('\n')).not.toContain('Stop →'); }); + it('reports adapter-driven tools by their generated artifact, not "not configured"', async () => { + // Hermes / OpenCode / OMP / OpenClaw have no settings file to probe: + // reconciliation writes one generated artifact each, so its presence + // is the status. Falling through to the generic branch printed + // "not configured" for tools the pipeline does install hooks for. + const restoreHome = mockHome('/home/testuser'); + const out: string[] = []; + const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); + mockedAutoDetectInit.mockResolvedValue({ + localConfig: mockLocalConfig, + teamConfig: { toolPaths: { hermes: { skills: '.hermes/skills' } } }, + }); + + try { + await hooksList({}); + } finally { + restoreHome(); + consoleLog.mockRestore(); + } + + const text = out.join('\n'); + expect(text).toContain('~/.hermes/hooks/teamai-status-report.sh'); + expect(text).not.toContain('no settings configured'); + }); + it('lists only the built-in hooks a tool actually receives (#717)', async () => { const out: string[] = []; const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); diff --git a/src/hermes-hooks.ts b/src/hermes-hooks.ts index 7ef6b970..76186215 100644 --- a/src/hermes-hooks.ts +++ b/src/hermes-hooks.ts @@ -24,7 +24,7 @@ import { const REPORT_EVENT = 'on_session_start'; /** Absolute path to the generated status-report shell script. */ -function getReportScriptPath(): string { +export function getReportScriptPath(): string { return path.join(getHermesHome(), 'hooks', 'teamai-status-report.sh'); } diff --git a/src/hooks-cmd.ts b/src/hooks-cmd.ts index 7afa3b2c..11adfb3b 100644 --- a/src/hooks-cmd.ts +++ b/src/hooks-cmd.ts @@ -54,6 +54,37 @@ function formatHooksList(rows: HookListRow[]): string { return lines.join('\n'); } +/** + * Path of the single generated artifact an adapter-driven tool's built-in hooks + * live in, or null when the tool is reconciled through a settings file (or its + * target location cannot be resolved, e.g. no OpenClaw workspace on this + * machine). Presence of that file is the tool's whole install status. + */ +async function adapterHookArtifact( + tool: string, + baseDir: string, + scope: 'user' | 'project', +): Promise { + if (tool === 'omp') { + const { resolveOmpExtensionsDir, OMP_HOOK_FILE } = await import('./omp-hooks.js'); + return path.join(resolveOmpExtensionsDir(), OMP_HOOK_FILE); + } + if (tool === 'opencode') { + const { resolveOpencodePluginDir, OPENCODE_HOOK_FILE } = await import('./opencode-hooks.js'); + return path.join(resolveOpencodePluginDir(baseDir, scope), OPENCODE_HOOK_FILE); + } + if (tool === 'hermes') { + const { getReportScriptPath } = await import('./hermes-hooks.js'); + return getReportScriptPath(); + } + if (tool === 'openclaw') { + const { resolveOpenclawWorkspaceDir, OPENCLAW_HOOK_DIR } = await import('./openclaw-hooks.js'); + const workspace = await resolveOpenclawWorkspaceDir(); + return workspace ? path.join(workspace, 'hooks', OPENCLAW_HOOK_DIR, 'HOOK.md') : null; + } + return null; +} + /** * Handler for `teamai hooks inject`. * Reconciles built-in (A) + team (B) hooks into all configured AI tool settings. @@ -127,16 +158,15 @@ export async function hooksList(_options: GlobalOptions): Promise { if (seenSettingsFiles.has(hookPath)) continue; seenSettingsFiles.add(hookPath); } - // OMP has no settings/hooks file to parse: its hooks are a single - // generated extension under the user agent dir, so presence of the - // file (with our marker) is the whole status. - if (tool === 'omp') { - const { resolveOmpExtensionsDir, OMP_HOOK_FILE } = await import('./omp-hooks.js'); - const extFile = path.join(resolveOmpExtensionsDir(), OMP_HOOK_FILE); + // The adapter-driven tools have no settings/hooks file to parse: each + // installs a single generated artifact, so its presence is the whole + // status. + const artifact = await adapterHookArtifact(tool, baseDir, localConfig.scope); + if (artifact) { rows.push({ tool, - status: await pathExists(extFile) ? 'installed' : 'missing', - settingsPath: formatDisplayPath(extFile), + status: await pathExists(artifact) ? 'installed' : 'missing', + settingsPath: formatDisplayPath(artifact), builtinDefs: installedBuiltinHookDefs(tool, false), }); continue; @@ -154,6 +184,9 @@ export async function hooksList(_options: GlobalOptions): Promise { tool, status: await getHookStatus(hookPath, tool), settingsPath: formatDisplayPath(hookPath), + // The override is applied to the settings-driven defs only: the + // standalone adapters generate a fixed handler and ignore it, so + // filtering their rows would hide hooks they still install. builtinDefs: applyBuiltinOverride(installedBuiltinHookDefs(tool, true), builtinOverride), }); } From d84b5f913e1223220f9bec23a4b1135b75e622f2 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Tue, 22 Sep 2026 22:13:40 +0000 Subject: [PATCH 3/4] fix(hooks): status against the effective built-in set and the user plugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two status-column defects the per-tool listing exposed: - `getHookStatus` checked the unmodified `builtinHookDefs(tool)`, so a built-in the team disabled through hooks.yaml was still expected on disk and every tool read `missing` right after a correct reconciliation. It now takes the same §4.8 override reconciliation applies. - The OpenCode artifact was probed under the config scope's base dir, but `reconcileOpencodePlugin` always installs the single plugin under the user path, so a project-scope config reported `missing` after a successful injection. Co-Authored-By: Claude Opus 5 --- src/__tests__/hooks-cmd.test.ts | 39 ++++++++++++++++++++++++++++----- src/hooks-cmd.ts | 14 +++++------- src/hooks.ts | 12 ++++++++-- 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/__tests__/hooks-cmd.test.ts b/src/__tests__/hooks-cmd.test.ts index eccee073..8ade1508 100644 --- a/src/__tests__/hooks-cmd.test.ts +++ b/src/__tests__/hooks-cmd.test.ts @@ -269,14 +269,17 @@ describe('hooksList', () => { expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join('/home/testuser', '.claude/settings.json'), 'claude', + undefined, ); expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join('/home/testuser', '.claude-internal/settings.json'), 'claude-internal', + undefined, ); expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join('/home/testuser', '.cursor/hooks.json'), 'cursor', + undefined, ); const output = consoleLog.mock.calls.map((call) => String(call[0])).join('\n'); @@ -315,10 +318,12 @@ describe('hooksList', () => { expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join('/home/testuser', '.claude/settings.json'), 'claude', + undefined, ); expect(mockedGetHookStatus).not.toHaveBeenCalledWith( path.join('/path/to/project', '.claude/settings.json'), 'claude', + undefined, ); } finally { restoreHome(); @@ -354,10 +359,12 @@ describe('hooksList', () => { expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join('/home/testuser', '.qoder-cn', 'settings.json'), 'qoder-cn', + undefined, ); expect(mockedGetHookStatus).not.toHaveBeenCalledWith( path.join('/home/testuser', '.qoder', 'settings.json'), 'qoder-cn', + undefined, ); }); @@ -386,8 +393,8 @@ describe('hooksList', () => { } const shared = path.join(projectRoot, '.qoder', 'settings.json'); - expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder'); - expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder-cn'); + expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder', undefined); + expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder-cn', undefined); }); // #667: ownership of the file shared by Qoder and Qoder CN follows the @@ -420,9 +427,9 @@ describe('hooksList', () => { } const shared = path.join(projectRoot, '.qoder', 'settings.json'); - expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder-cn'); + expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder-cn', undefined); // `qoder` is not enabled, so it must not claim — and mis-probe — the file. - expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder'); + expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder', undefined); }); // The same rule with no whitelist: `disabledAgents` alone moves ownership. @@ -449,8 +456,8 @@ describe('hooksList', () => { } const shared = path.join(projectRoot, '.qoder', 'settings.json'); - expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder-cn'); - expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder'); + expect(mockedGetHookStatus).toHaveBeenCalledWith(shared, 'qoder-cn', undefined); + expect(mockedGetHookStatus).not.toHaveBeenCalledWith(shared, 'qoder', undefined); }); it('lists standalone Copilot hooks under COPILOT_HOME', async () => { @@ -473,6 +480,7 @@ describe('hooksList', () => { expect(mockedGetHookStatus).toHaveBeenCalledWith( path.join(COPILOT_HOME_FIXTURE, 'hooks/teamai.json'), 'copilot', + undefined, ); }); @@ -548,6 +556,25 @@ describe('hooksList', () => { expect(text).not.toContain('no settings configured'); }); + it('checks tool status against the overridden built-in set', async () => { + // Reconciliation applies `builtin.disabled` when writing, so a status + // check that still expects the disabled hook reads `missing` forever. + mockedParseTeamHooks.mockResolvedValue(hooksYaml([], { disabled: ['Hook dispatch stop'] })); + const consoleLog = vi.spyOn(console, 'log').mockImplementation(() => undefined); + + try { + await hooksList({}); + } finally { + consoleLog.mockRestore(); + } + + expect(mockedGetHookStatus).toHaveBeenCalledWith( + expect.any(String), + 'claude', + { disabled: ['Hook dispatch stop'] }, + ); + }); + it('lists only the built-in hooks a tool actually receives (#717)', async () => { const out: string[] = []; const consoleLog = vi.spyOn(console, 'log').mockImplementation((m?: unknown) => { out.push(String(m)); }); diff --git a/src/hooks-cmd.ts b/src/hooks-cmd.ts index 11adfb3b..ad65d9de 100644 --- a/src/hooks-cmd.ts +++ b/src/hooks-cmd.ts @@ -60,18 +60,16 @@ function formatHooksList(rows: HookListRow[]): string { * target location cannot be resolved, e.g. no OpenClaw workspace on this * machine). Presence of that file is the tool's whole install status. */ -async function adapterHookArtifact( - tool: string, - baseDir: string, - scope: 'user' | 'project', -): Promise { +async function adapterHookArtifact(tool: string): Promise { if (tool === 'omp') { const { resolveOmpExtensionsDir, OMP_HOOK_FILE } = await import('./omp-hooks.js'); return path.join(resolveOmpExtensionsDir(), OMP_HOOK_FILE); } if (tool === 'opencode') { + // reconcileOpencodePlugin always installs the single plugin under the + // user path, whatever the config scope, so probe there. const { resolveOpencodePluginDir, OPENCODE_HOOK_FILE } = await import('./opencode-hooks.js'); - return path.join(resolveOpencodePluginDir(baseDir, scope), OPENCODE_HOOK_FILE); + return path.join(resolveOpencodePluginDir(getUserHome(), 'user'), OPENCODE_HOOK_FILE); } if (tool === 'hermes') { const { getReportScriptPath } = await import('./hermes-hooks.js'); @@ -161,7 +159,7 @@ export async function hooksList(_options: GlobalOptions): Promise { // The adapter-driven tools have no settings/hooks file to parse: each // installs a single generated artifact, so its presence is the whole // status. - const artifact = await adapterHookArtifact(tool, baseDir, localConfig.scope); + const artifact = await adapterHookArtifact(tool); if (artifact) { rows.push({ tool, @@ -182,7 +180,7 @@ export async function hooksList(_options: GlobalOptions): Promise { } rows.push({ tool, - status: await getHookStatus(hookPath, tool), + status: await getHookStatus(hookPath, tool, builtinOverride), settingsPath: formatDisplayPath(hookPath), // The override is applied to the settings-driven defs only: the // standalone adapters generate a fixed handler and ignore it, so diff --git a/src/hooks.ts b/src/hooks.ts index 2c99138e..23327a20 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1158,11 +1158,19 @@ export async function removeHooks(settingsPath: string, tool?: string): Promise< * Report whether the current built-in (A) hook set is present in a tool settings * file. Computed against the unified HookDef model: every built-in entry for the * tool must already exist on disk. + * + * `builtinOverride` is the team's §4.8 override. Reconciliation applies it when + * writing, so the status check must apply it too — otherwise a hook the team + * disabled is still expected on disk and every tool reads as `missing`. */ -export async function getHookStatus(settingsPath: string, tool?: string): Promise { +export async function getHookStatus( + settingsPath: string, + tool?: string, + builtinOverride?: BuiltinHookOverride, +): Promise { const toolName = tool ?? 'claude'; const expanded = expandHome(settingsPath); - const defs = builtinHookDefs(toolName); + const defs = applyBuiltinOverride(builtinHookDefs(toolName), builtinOverride); const format = detectFormat(toolName); if (format === 'cursor') { From ebc4c3e5f20ca2604824414f83378635c6a6c703 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Tue, 22 Sep 2026 22:20:05 +0000 Subject: [PATCH 4/4] fix(hooks): require every generated file before reporting installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The adapter status probe accepted a single file, so an OpenClaw installation missing its handler.ts — the file HOOK.md points at, without which no hook runs — still read as `installed`. Check every generated file the adapter writes and report `installed` only when all are present. Co-Authored-By: Claude Opus 5 --- src/hooks-cmd.ts | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/hooks-cmd.ts b/src/hooks-cmd.ts index ad65d9de..87698e7e 100644 --- a/src/hooks-cmd.ts +++ b/src/hooks-cmd.ts @@ -55,30 +55,35 @@ function formatHooksList(rows: HookListRow[]): string { } /** - * Path of the single generated artifact an adapter-driven tool's built-in hooks - * live in, or null when the tool is reconciled through a settings file (or its - * target location cannot be resolved, e.g. no OpenClaw workspace on this - * machine). Presence of that file is the tool's whole install status. + * Generated files an adapter-driven tool's built-in hooks live in, or null when + * the tool is reconciled through a settings file (or its target location cannot + * be resolved, e.g. no OpenClaw workspace on this machine). The tool counts as + * installed only when every one of them is present, so a half-written + * installation does not read as `installed`. */ -async function adapterHookArtifact(tool: string): Promise { +async function adapterHookArtifacts(tool: string): Promise { if (tool === 'omp') { const { resolveOmpExtensionsDir, OMP_HOOK_FILE } = await import('./omp-hooks.js'); - return path.join(resolveOmpExtensionsDir(), OMP_HOOK_FILE); + return [path.join(resolveOmpExtensionsDir(), OMP_HOOK_FILE)]; } if (tool === 'opencode') { // reconcileOpencodePlugin always installs the single plugin under the // user path, whatever the config scope, so probe there. const { resolveOpencodePluginDir, OPENCODE_HOOK_FILE } = await import('./opencode-hooks.js'); - return path.join(resolveOpencodePluginDir(getUserHome(), 'user'), OPENCODE_HOOK_FILE); + return [path.join(resolveOpencodePluginDir(getUserHome(), 'user'), OPENCODE_HOOK_FILE)]; } if (tool === 'hermes') { const { getReportScriptPath } = await import('./hermes-hooks.js'); - return getReportScriptPath(); + return [getReportScriptPath()]; } if (tool === 'openclaw') { const { resolveOpenclawWorkspaceDir, OPENCLAW_HOOK_DIR } = await import('./openclaw-hooks.js'); const workspace = await resolveOpenclawWorkspaceDir(); - return workspace ? path.join(workspace, 'hooks', OPENCLAW_HOOK_DIR, 'HOOK.md') : null; + if (!workspace) return null; + // The engine needs both halves: the HOOK.md descriptor and the handler + // it points at. + const dir = path.join(workspace, 'hooks', OPENCLAW_HOOK_DIR); + return [path.join(dir, 'HOOK.md'), path.join(dir, 'handler.ts')]; } return null; } @@ -159,12 +164,13 @@ export async function hooksList(_options: GlobalOptions): Promise { // The adapter-driven tools have no settings/hooks file to parse: each // installs a single generated artifact, so its presence is the whole // status. - const artifact = await adapterHookArtifact(tool); - if (artifact) { + const artifacts = await adapterHookArtifacts(tool); + if (artifacts) { + const present = await Promise.all(artifacts.map((file) => pathExists(file))); rows.push({ tool, - status: await pathExists(artifact) ? 'installed' : 'missing', - settingsPath: formatDisplayPath(artifact), + status: present.every(Boolean) ? 'installed' : 'missing', + settingsPath: formatDisplayPath(artifacts[0] as string), builtinDefs: installedBuiltinHookDefs(tool, false), }); continue;