fix(hooks): flag hook event names that no core trigger emits#99456
Conversation
openclaw-barnacle
Bot
added
docs
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 5:14 AM ET / 09:14 UTC. Summary PR surface: Source +77, Tests +103, Docs +8. Total +188 across 9 files. Reproducibility: yes. source-reproducible: current main registers every declared event string, while runtime dispatch only looks up the emitted family and Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the advisory diagnostic if maintainers accept the manual core event list, keeping trigger sites, the registry, and docs in sync. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main registers every declared event string, while runtime dispatch only looks up the emitted family and Is this the best way to solve the issue? Yes, advisory validation in loader/status is the best scoped fix because plugin-emitted custom events remain possible and fail-closed validation would be compatibility-sensitive. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 68e06b9ea148. Label changesLabel justifications:
Evidence reviewedPR surface: Source +77, Tests +103, Docs +8. Total +188 across 9 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
…w#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
…w#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
2 participants
What Problem This Solves
A hook's
HOOK.mdfrontmatter declares which internal events it subscribes to (metadata.openclaw.events), but the loader accepts any string:registerInternalHook(eventKey)registers arbitrary keys, and only an empty events array produces a warning (src/hooks/loader.ts). Since core is the only emitter of internal hook events, a misspelled name (command:nwe,message:recieved,gateway:started) loads "successfully" and then silently never fires — no loader warning, nothing inopenclaw hooks list/info, no way to tell a dead subscription from a hook that simply hasn't triggered yet. Debugging "my hook never runs" today means reading core source to discover the valid key set.Why This Change Was Made
The core-emitted key set is small, closed, and already documented (the events table in
docs/automation/hooks.md): core trigger sites emit exactly 14family:actionkeys, and hooks may also subscribe to a bare family (command,session,agent,gateway,message) to receive every action. Plugins can emit additional keys through the deprecatedplugin-sdk/hook-runtimebarrel (all bundled plugins emit only known keys — Telegram/Slack/Signal/WhatsApp emitmessage:received/message:sent), so a name outside the core set is almost always a typo — the diagnostics say so honestly ("not emitted by OpenClaw core — likely a typo; unless a plugin emits it, the hook never fires") and stay strictly advisory.This change makes that set checkable:
KNOWN_INTERNAL_HOOK_EVENT_KEYS+isKnownInternalHookEventKeyinsrc/hooks/internal-hook-types.ts(colocated with theInternalHookEventTypeunion; the list mirrors the docs table and each entry was verified against itscreateInternalHookEventcall site, including the dynamic ones —performGatewaySessionResetemits"new" | "reset", reset-command hooks emitResetCommandAction = "new" | "reset").HOOK.mdhooks and the legacyhooks.internal.handlersconfig shape. The hook is still registered unchanged — validation is advisory, zero behavior change, so plugin-emitted custom events and version skew degrade to today's behavior plus a warning rather than a refusal to load.HookStatusEntrygainsunknownEvents;openclaw hooks info <name>renders a⚠ Event not emitted by core (likely typo): …line and the JSON report includes the field, so the diagnosis is visible after load time too.docs/automation/hooks.mddocuments the behavior next to the events table.User Impact
Hook authors find out about dead subscriptions immediately (gateway log at load) and on demand (
openclaw hooks info), instead of shipping a hook that silently does nothing. Existing hooks with valid events see no change; hooks with typos keep loading exactly as before but are no longer silent about it.Evidence
HEAD:
f9b756725354debd4aa64cc62b36e411245b8e00(base upstream/maindf350e6720)Real behavior proof (no mocks — a real
HOOK.mddeclaring["command:nwe", "command:new"]loaded through the productionloadInternalHooks, then rendered by the production status builder and CLI; captured at607ca0ff76— the one commit since,f9b75672, only adds the same advisory to the legacyhooks.internal.handlerspath and does not touch the surfaces proven here):Focused tests (all green locally):
src/hooks/internal-hook-types.test.ts(new) — accept matrix for all 14 keys + 5 bare families; reject matrix for typos, bare actions, unknown families, case mismatchessrc/hooks/loader.test.ts— typo'd key still registers (advisory) and the valid subscription keeps firing; legacy config handlers get the same advisory treatmentsrc/cli/hooks-cli.test.ts—hooks inforenders the ⚠ linesrc/commands/onboard-hooks.test.ts— fixture updated for the new required fieldpnpm tsgo:core+pnpm tsgo:core:testcleanExternal structured review: Codex (GPT-5.5), 5 rounds — flagged docs wording (bare-family subscriptions), an overclaim in this body ("plugins cannot emit new keys" vs. the deprecated
plugin-sdk/hook-runtimere-export → diagnostics reworded to "not emitted by core / likely a typo"), and a missed sibling seam (the legacyhooks.internal.handlerspath registered events without the advisory check → covered); final pass clean.Known limits / Non-scope: advisory only — no fail-closed loading, no doctor check (the hooks CLI is the existing diagnostic surface), no change to the separate plugin-SDK hook API, no fuzzy matching/normalization of event names; no attempt to track plugin-emitted custom keys. New core trigger sites must add their key to the list + docs table (called out in a code comment at the list).
🤖 AI-assisted (Claude Fable 5 + Codex GPT-5.5 review loop). Prompt lineage: translate agentic-coding-tool hook-validation patterns (Claude Code hook-matcher fixes) into OpenClaw's internal hook loader; implementation, tests, review rounds, and the real-behavior proof were run locally as shown above.