mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 15:59:30 +00:00
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
This commit is contained in:
parent
3ea875b5d8
commit
e7ca90e3af
9 changed files with 189 additions and 1 deletions
|
|
@ -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 <name>` flags them, so a
|
||||
hook that never runs is diagnosable.
|
||||
|
||||
| Event | When it fires |
|
||||
| ------------------------ | ---------------------------------------------------------- |
|
||||
| `command:new` | `/new` command issued |
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
||||
|
|
|
|||
|
|
@ -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."));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,6 +115,7 @@ describe("onboard-hooks", () => {
|
|||
? undefined
|
||||
: "missing requirements") as HookStatusEntry["blockedReason"],
|
||||
...params,
|
||||
unknownEvents: [],
|
||||
source: "openclaw-bundled" as const,
|
||||
pluginId: undefined,
|
||||
homepage: undefined,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
35
src/hooks/internal-hook-types.test.ts
Normal file
35
src/hooks/internal-hook-types.test.ts
Normal file
|
|
@ -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);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -52,17 +52,19 @@ describe("loader", () => {
|
|||
sourceDir?: string;
|
||||
hookName: string;
|
||||
handlerCode?: string;
|
||||
events?: string[];
|
||||
}): Promise<string> {
|
||||
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");
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue