From 215e49b1a2f5155a4328864ddc85e9ff87f88dcc Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 30 Jul 2026 05:29:25 +0800 Subject: [PATCH] fix(hooks): report eventless hooks as not ready (#116083) Fixes #72370 Release note: hooks check now reports selected hooks without declared events as not ready instead of ready. --- src/cli/hooks-cli.test.ts | 41 +++++++++++++++++++++++++++++ src/hooks/hooks-status.test.ts | 48 ++++++++++++++++++++++++++++++++++ src/hooks/hooks-status.ts | 8 +++--- 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 src/hooks/hooks-status.test.ts diff --git a/src/cli/hooks-cli.test.ts b/src/cli/hooks-cli.test.ts index caad296a8fdf..2f4ff54953ca 100644 --- a/src/cli/hooks-cli.test.ts +++ b/src/cli/hooks-cli.test.ts @@ -85,6 +85,20 @@ function createPluginManagedHookReport(): HookStatusReport { }; } +function createEventlessHookReport(): HookStatusReport { + return { + ...report, + hooks: [ + { + ...expectDefined(report.hooks[0], "report.hooks[0] test invariant"), + events: [], + loadable: false, + blockedReason: "no events defined", + }, + ], + }; +} + describe("hooks cli formatting", () => { it("labels hooks list output", () => { const output = formatHooksList(report, {}); @@ -97,6 +111,33 @@ describe("hooks cli formatting", () => { expect(output).toContain("Hooks Status"); }); + it("classifies eventless hooks as not ready in human check output", () => { + const output = formatHooksCheck(createEventlessHookReport(), {}); + + expect(output).toContain("Ready: 0"); + expect(output).toContain("Not ready: 1"); + expect(output).toContain("session-memory - no events defined"); + }); + + it("classifies eventless hooks as not eligible in JSON check output", () => { + const output = JSON.parse(formatHooksCheck(createEventlessHookReport(), { json: true })); + + expect(output).toMatchObject({ + total: 1, + eligible: 0, + notEligible: 1, + hooks: { + eligible: [], + notEligible: [ + { + name: "session-memory", + blockedReason: "no events defined", + }, + ], + }, + }); + }); + it("labels plugin-managed hooks with plugin id", () => { const pluginReport = createPluginManagedHookReport(); diff --git a/src/hooks/hooks-status.test.ts b/src/hooks/hooks-status.test.ts new file mode 100644 index 000000000000..8489dc7b7c66 --- /dev/null +++ b/src/hooks/hooks-status.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { buildWorkspaceHookStatus } from "./hooks-status.js"; +import type { HookEntry } from "./types.js"; + +function createHookEntry(params: { + source: HookEntry["hook"]["source"]; + events: string[]; +}): HookEntry { + return { + hook: { + name: "session-memory", + description: "Save session context to memory", + source: params.source, + filePath: `/tmp/${params.source}/HOOK.md`, + baseDir: `/tmp/${params.source}`, + handlerPath: `/tmp/${params.source}/handler.js`, + }, + frontmatter: {}, + metadata: { events: params.events }, + }; +} + +describe("hook status", () => { + it("reports an eventless managed winner as not loadable", () => { + const report = buildWorkspaceHookStatus("/tmp/workspace", { + entries: [ + createHookEntry({ + source: "openclaw-managed", + events: [], + }), + createHookEntry({ + source: "openclaw-workspace", + events: ["command:new"], + }), + ], + }); + + expect(report.hooks).toHaveLength(1); + expect(report.hooks[0]).toMatchObject({ + source: "openclaw-managed", + events: [], + enabledByConfig: true, + requirementsSatisfied: true, + loadable: false, + blockedReason: "no events defined", + }); + }); +}); diff --git a/src/hooks/hooks-status.ts b/src/hooks/hooks-status.ts index ad775817156b..73fa7f218bba 100644 --- a/src/hooks/hooks-status.ts +++ b/src/hooks/hooks-status.ts @@ -42,7 +42,7 @@ export type HookStatusEntry = { enabledByConfig: boolean; requirementsSatisfied: boolean; loadable: boolean; - blockedReason?: HookEnableStateReason | "missing requirements"; + blockedReason?: HookEnableStateReason | "missing requirements" | "no events defined"; managedByPlugin: boolean; requirements: Requirements; missing: Requirements; @@ -114,9 +114,11 @@ function buildHookStatus( }); const enabledByConfig = enableState.enabled; - const loadable = enabledByConfig && requirementsSatisfied; + const hasEvents = events.length > 0; + const loadable = enabledByConfig && requirementsSatisfied && hasEvents; const blockedReason = - enableState.reason ?? (requirementsSatisfied ? undefined : "missing requirements"); + enableState.reason ?? + (!requirementsSatisfied ? "missing requirements" : hasEvents ? undefined : "no events defined"); return { name: entry.hook.name,