From e7ca90e3afaf2539774b80ef399fc99dff3268dd Mon Sep 17 00:00:00 2001 From: Masato Hoshino Date: Mon, 6 Jul 2026 17:30:13 +0900 Subject: [PATCH] fix(hooks): flag hook event names that no core trigger emits (#99456) * fix(hooks): flag hook event names that no trigger site emits * docs(hooks): clarify bare family subscriptions in unknown-event note * fix(hooks): word unknown-event diagnostics around core-emitted keys * fix(hooks): apply unknown-event advisory to legacy config handlers too --- docs/automation/hooks.md | 8 +++++ src/cli/hooks-cli.test.ts | 20 +++++++++++ src/cli/hooks-cli.ts | 8 +++++ src/commands/onboard-hooks.test.ts | 1 + src/hooks/hooks-status.ts | 5 +++ src/hooks/internal-hook-types.test.ts | 35 +++++++++++++++++++ src/hooks/internal-hook-types.ts | 40 ++++++++++++++++++++++ src/hooks/loader.test.ts | 49 ++++++++++++++++++++++++++- src/hooks/loader.ts | 24 +++++++++++++ 9 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 src/hooks/internal-hook-types.test.ts diff --git a/docs/automation/hooks.md b/docs/automation/hooks.md index 33123b45007..2f879cdfed1 100644 --- a/docs/automation/hooks.md +++ b/docs/automation/hooks.md @@ -45,6 +45,14 @@ openclaw hooks info session-memory ## Event types +Hooks subscribe to a specific key from this table, or to a bare family name +(`command`, `session`, `agent`, `gateway`, `message`) to receive every action +in that family. OpenClaw core emits nothing else, so any other name is almost +always a typo that leaves the hook silently dead (only a plugin emitting a +custom event could fire it). The hook loader logs a warning for such names +(for example `command:nwe`), and `openclaw hooks info ` flags them, so a +hook that never runs is diagnosable. + | Event | When it fires | | ------------------------ | ---------------------------------------------------------- | | `command:new` | `/new` command issued | diff --git a/src/cli/hooks-cli.test.ts b/src/cli/hooks-cli.test.ts index 1895b299bc4..2e70aca57a3 100644 --- a/src/cli/hooks-cli.test.ts +++ b/src/cli/hooks-cli.test.ts @@ -20,6 +20,7 @@ const report: HookStatusReport = { emoji: "💾", homepage: "https://docs.openclaw.ai/automation/hooks#session-memory", events: ["command:new"], + unknownEvents: [], always: false, enabledByConfig: true, requirementsSatisfied: true, @@ -48,6 +49,7 @@ function createPluginManagedHookReport(): HookStatusReport { emoji: "🔗", homepage: undefined, events: ["command:new"], + unknownEvents: [], always: false, enabledByConfig: true, requirementsSatisfied: true, @@ -79,6 +81,24 @@ describe("hooks cli formatting", () => { expect(output).toContain("plugin:voice-call"); }); + it("warns about unknown events in hook info", () => { + const typoReport: HookStatusReport = { + workspaceDir: "/tmp/workspace", + managedHooksDir: "/tmp/hooks", + hooks: [ + { + ...report.hooks[0], + name: "typo-hook", + events: ["command:nwe", "command:new"], + unknownEvents: ["command:nwe"], + }, + ], + }; + + const output = formatHookInfo(typoReport, "typo-hook", {}); + expect(output).toContain("Event not emitted by core (likely typo): command:nwe"); + }); + it("shows plugin-managed details in hook info", () => { const pluginReport = createPluginManagedHookReport(); diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index f251f682ccc..2cab73d3166 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -191,6 +191,7 @@ export function formatHooksList(report: HookStatusReport, opts: HooksListOptions source: h.source, pluginId: h.pluginId, events: h.events, + unknownEvents: h.unknownEvents, homepage: h.homepage, missing: h.missing, managedByPlugin: h.managedByPlugin, @@ -300,6 +301,13 @@ export function formatHookInfo( if (hook.events.length > 0) { lines.push(`${theme.muted(" Events:")} ${hook.events.join(", ")}`); } + if (hook.unknownEvents.length > 0) { + lines.push( + theme.warn( + ` ⚠ Event${hook.unknownEvents.length === 1 ? "" : "s"} not emitted by core (likely typo): ${hook.unknownEvents.join(", ")}`, + ), + ); + } if (hook.managedByPlugin) { lines.push(theme.muted(" Managed by plugin; enable/disable via hooks CLI not available.")); } diff --git a/src/commands/onboard-hooks.test.ts b/src/commands/onboard-hooks.test.ts index 3192047fa09..5f3de3f7c22 100644 --- a/src/commands/onboard-hooks.test.ts +++ b/src/commands/onboard-hooks.test.ts @@ -115,6 +115,7 @@ describe("onboard-hooks", () => { ? undefined : "missing requirements") as HookStatusEntry["blockedReason"], ...params, + unknownEvents: [], source: "openclaw-bundled" as const, pluginId: undefined, homepage: undefined, diff --git a/src/hooks/hooks-status.ts b/src/hooks/hooks-status.ts index 598adf105db..4d7c33aa84a 100644 --- a/src/hooks/hooks-status.ts +++ b/src/hooks/hooks-status.ts @@ -5,6 +5,7 @@ import { evaluateEntryRequirementsForCurrentPlatform } from "../shared/entry-sta import type { RequirementConfigCheck, Requirements } from "../shared/requirements.js"; import { CONFIG_DIR } from "../utils.js"; import { hasBinary, isConfigPathTruthy } from "./config.js"; +import { isKnownInternalHookEventKey } from "./internal-hook-types.js"; import { resolveHookConfig, resolveHookEnableState, @@ -35,6 +36,8 @@ export type HookStatusEntry = { emoji?: string; homepage?: string; events: string[]; + /** Declared events no core trigger site emits (likely typos; fire only if a plugin emits them). */ + unknownEvents: string[]; always: boolean; enabledByConfig: boolean; requirementsSatisfied: boolean; @@ -96,6 +99,7 @@ function buildHookStatus( const enableState = resolveHookEnableState({ entry, config, hookConfig }); const always = entry.metadata?.always === true; const events = entry.metadata?.events ?? []; + const unknownEvents = events.filter((event) => !isKnownInternalHookEventKey(event)); const isEnvSatisfied = (envName: string) => Boolean(process.env[envName] || hookConfig?.env?.[envName]); const isConfigSatisfied = (pathStr: string) => isConfigPathTruthy(config, pathStr); @@ -127,6 +131,7 @@ function buildHookStatus( emoji, homepage, events, + unknownEvents, always, enabledByConfig, requirementsSatisfied, diff --git a/src/hooks/internal-hook-types.test.ts b/src/hooks/internal-hook-types.test.ts new file mode 100644 index 00000000000..1ff0af0cdb5 --- /dev/null +++ b/src/hooks/internal-hook-types.test.ts @@ -0,0 +1,35 @@ +// Internal hook event key validation tests. +import { describe, expect, it } from "vitest"; +import { + isKnownInternalHookEventKey, + KNOWN_INTERNAL_HOOK_EVENT_KEYS, +} from "./internal-hook-types.js"; + +describe("isKnownInternalHookEventKey", () => { + it("accepts every emitted type:action key", () => { + for (const key of KNOWN_INTERNAL_HOOK_EVENT_KEYS) { + expect(isKnownInternalHookEventKey(key), key).toBe(true); + } + }); + + it("accepts bare family keys that subscribe to every action", () => { + for (const family of ["command", "session", "agent", "gateway", "message"]) { + expect(isKnownInternalHookEventKey(family), family).toBe(true); + } + }); + + it("rejects typos, bare actions, and unknown families", () => { + for (const key of [ + "command:nwe", + "message:recieved", + "startup", // bare action without its family + "gateway:started", + "session:compact", // partial multi-segment action + "webhook:received", + "", + "COMMAND:NEW", + ]) { + expect(isKnownInternalHookEventKey(key), key).toBe(false); + } + }); +}); diff --git a/src/hooks/internal-hook-types.ts b/src/hooks/internal-hook-types.ts index 1e078f56439..ec4f202210e 100644 --- a/src/hooks/internal-hook-types.ts +++ b/src/hooks/internal-hook-types.ts @@ -1,6 +1,46 @@ // Internal hook types define runtime hook event families and payload contracts. export type InternalHookEventType = "command" | "session" | "agent" | "gateway" | "message"; +const KNOWN_INTERNAL_HOOK_EVENT_FAMILIES = [ + "command", + "session", + "agent", + "gateway", + "message", +] as const satisfies readonly InternalHookEventType[]; + +/** + * Event keys emitted by core trigger sites (see docs/automation/hooks.md + * events table — keep both in sync when adding a trigger). Hooks can also + * subscribe to a bare family key to receive every action of that family. + * Plugins can emit additional keys via the deprecated plugin-sdk/hook-runtime + * barrel, so anything outside this set is flagged as a likely typo + * (advisory), not rejected. + */ +export const KNOWN_INTERNAL_HOOK_EVENT_KEYS = [ + "agent:bootstrap", + "command:new", + "command:reset", + "command:stop", + "gateway:pre-restart", + "gateway:shutdown", + "gateway:startup", + "message:preprocessed", + "message:received", + "message:sent", + "message:transcribed", + "session:compact:after", + "session:compact:before", + "session:patch", +] as const; + +export function isKnownInternalHookEventKey(key: string): boolean { + return ( + (KNOWN_INTERNAL_HOOK_EVENT_KEYS as readonly string[]).includes(key) || + (KNOWN_INTERNAL_HOOK_EVENT_FAMILIES as readonly string[]).includes(key) + ); +} + export interface InternalHookEvent { /** The type of event (command, session, agent, gateway, etc.) */ type: InternalHookEventType; diff --git a/src/hooks/loader.test.ts b/src/hooks/loader.test.ts index 6af63442250..d1d5fd870e2 100644 --- a/src/hooks/loader.test.ts +++ b/src/hooks/loader.test.ts @@ -52,17 +52,19 @@ describe("loader", () => { sourceDir?: string; hookName: string; handlerCode?: string; + events?: string[]; }): Promise { const sourceDir = params.sourceDir ?? path.join(tmpDir, "hooks"); const hookDir = path.join(sourceDir, params.hookName); await fs.mkdir(hookDir, { recursive: true }); + const events = params.events ?? ["command:new"]; await fs.writeFile( path.join(hookDir, "HOOK.md"), [ "---", `name: ${params.hookName}`, `description: ${params.hookName} test hook`, - 'metadata: {"openclaw":{"events":["command:new"]}}', + `metadata: {"openclaw":{"events":${JSON.stringify(events)}}}`, "---", "", `# ${params.hookName}`, @@ -244,6 +246,51 @@ describe("loader", () => { expect(event.messages).toEqual(["keep-hook"]); }); + it("registers unknown event keys anyway (advisory warning, not a load failure)", async () => { + const hooksDir = path.join(tmpDir, "managed-hooks"); + await writeDiscoveredHook({ + sourceDir: hooksDir, + hookName: "typo-hook", + events: ["command:nwe", "command:new"], + }); + + const count = await loadInternalHooks( + { + hooks: { + internal: { + entries: { + "typo-hook": { enabled: true }, + }, + }, + }, + } satisfies OpenClawConfig, + tmpDir, + { managedHooksDir: hooksDir, bundledHooksDir: "/nonexistent/bundled/hooks" }, + ); + + // The typo'd key never fires, but validation is advisory: the hook still + // loads and its valid subscriptions keep working. + expect(count).toBe(1); + const keys = getRegisteredEventKeys(); + expect(keys).toContain("command:nwe"); + expect(keys).toContain("command:new"); + const event = createInternalHookEvent("command", "new", "test-session"); + await triggerInternalHook(event); + expect(event.messages).toEqual(["typo-hook"]); + }); + + it("registers legacy handler events with unknown keys anyway (advisory)", async () => { + const handlerPath = await writeHandlerModule("legacy-typo-handler.js"); + + const cfg = createEnabledHooksConfig([ + { event: "command:nwe", module: path.basename(handlerPath) }, + ]); + + const count = await loadInternalHooks(cfg, tmpDir); + expect(count).toBe(1); + expect(getRegisteredEventKeys()).toContain("command:nwe"); + }); + it("should load multiple handlers", async () => { // Create test handler modules const handler1Path = await writeHandlerModule("handler1.js"); diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 59deddcd37d..3dbd2b1cdfc 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -16,6 +16,7 @@ import { resolveGlobalSingleton } from "../shared/global-singleton.js"; import { shouldIncludeHook } from "./config.js"; import { hasConfiguredInternalHooks, resolveConfiguredInternalHookNames } from "./configured.js"; import { buildImportUrl } from "./import-url.js"; +import { isKnownInternalHookEventKey } from "./internal-hook-types.js"; import type { InternalHookHandler } from "./internal-hooks.js"; import { registerInternalHook, unregisterInternalHook } from "./internal-hooks.js"; import { getLegacyInternalHookHandlers } from "./legacy-config.js"; @@ -168,6 +169,20 @@ export async function loadInternalHooks( continue; } + // Core never emits keys outside the known set, so these are almost + // always typos that leave the hook silently dead (a plugin could emit + // custom keys via plugin-sdk/hook-runtime, hence advisory: warn but + // still register). + const unknownEvents = events.filter((event) => !isKnownInternalHookEventKey(event)); + if (unknownEvents.length > 0) { + log.warn( + `Hook '${safeLogValue(entry.hook.name)}' subscribes to event${unknownEvents.length === 1 ? "" : "s"} ` + + `${unknownEvents.map((event) => safeLogValue(event)).join(", ")} not emitted by OpenClaw core — ` + + `likely a typo; unless a plugin emits it, the hook never fires. ` + + `Known events: https://docs.openclaw.ai/automation/hooks`, + ); + } + for (const event of events) { registerInternalHook(event, handler); loadedHookRegistrations.push({ event, handler }); @@ -259,6 +274,15 @@ export async function loadInternalHooks( continue; } + // Same advisory typo check as directory-discovered hooks above. + if (!isKnownInternalHookEventKey(handlerConfig.event)) { + log.warn( + `Legacy hook handler ${safeLogValue(rawModule)} subscribes to event ` + + `${safeLogValue(handlerConfig.event)} not emitted by OpenClaw core — ` + + `likely a typo; unless a plugin emits it, the hook never fires. ` + + `Known events: https://docs.openclaw.ai/automation/hooks`, + ); + } registerInternalHook(handlerConfig.event, handler); loadedHookRegistrations.push({ event: handlerConfig.event, handler }); log.debug(