Fail invalid plugin registration gates loudly (#72577)

* fix plugin registration gate failures
2026-05-06 07:30:43 +00:00 · 2026-04-27 15:46:50 +10:00
parent 85148f3b20
commit 06b3e4ef8a
4 changed files with 131 additions and 46 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai
 - Cron: classify isolated runs as errors from structured embedded-run execution-denial metadata, with final-output marker fallback for `SYSTEM_RUN_DENIED`, `INVALID_REQUEST`, and approval-binding refusals, so blocked commands no longer appear green in cron history. Fixes #67172; carries forward #67186. Thanks @oc-gh-dr, @hclsys, and @1yihui.
 - Onboarding/GitHub Copilot: add manifest-owned `--github-copilot-token` support for non-interactive setup, including env fallback, tokenRef storage in ref mode, saved-profile reuse, and current Copilot default-model wiring. Refs #50002 and supersedes #50003. Thanks @scottgl9.
 - Gateway/install: add a validated `--wrapper`/`OPENCLAW_WRAPPER` service install path that persists executable LaunchAgent/systemd wrappers across forced reinstalls, updates, and doctor repairs instead of falling back to raw node/bun `ProgramArguments`. Fixes #69400. (#72445) Thanks @willtmc.
+- Plugins: fail plugin registration when loader-owned acceptance gates reject missing hook names or memory-only capability registration from non-memory plugins, surfacing the issue through plugin status and doctor instead of silently dropping the registration. Fixes #72459. Thanks @1fanwang and @amknight.
 - macOS Gateway: write launchd services with a state-dir `WorkingDirectory`, use a durable state-dir temp path instead of freezing macOS session `TMPDIR`, create that temp directory before bootstrap, and label abort-shaped launchd exits as `SIGABRT/abort` in status output. Fixes #53679 and #70223; refs #71848. Thanks @dlturock, @stammi922, and @palladius.
 - Exec approvals: accept runtime-owned `source: "allow-always"` and `commandText` allowlist metadata in gateway and node approval-set payloads so Control UI round-trips no longer fail with `unexpected property 'source'`. Fixes #60000; carries forward #60064. Thanks @sd1471123, @sharkqwy, and @luoyanglang.
 - Exec/node: skip approval-plan preparation for full-trust `host=node` runs so interpreter and script commands no longer fail with `SYSTEM_RUN_DENIED: approval cannot safely bind` when effective policy is `security=full` and `ask=off`. Fixes #48457 and duplicate #69251. Thanks @ajtran303, @jaserNo1, @Blakeshannon, @lesliefag, and @AvIsBeastMC.
--- a/docs/plugins/sdk-testing.md
+++ b/docs/plugins/sdk-testing.md
@@ -81,6 +81,19 @@ describe("my-channel target resolution", () => {

 ## Testing patterns

+### Testing registration contracts
+
+Unit tests that pass a hand-written `api` mock to `register(api)` do not exercise
+OpenClaw's loader acceptance gates. Add at least one loader-backed smoke test
+for each registration surface your plugin depends on, especially hooks and
+exclusive capabilities such as memory.
+
+The real loader fails plugin registration when required metadata is missing or a
+plugin calls a capability API it does not own. For example,
+`api.registerHook(...)` requires a hook name, and
+`api.registerMemoryCapability(...)` requires the plugin manifest or exported
+entry to declare `kind: "memory"`.
+
 ### Unit testing a channel plugin

 ```typescript
--- a/src/plugins/loader.test.ts
+++ b/src/plugins/loader.test.ts
@@ -66,6 +66,7 @@ import {
 import {
  buildMemoryPromptSection,
  clearMemoryPluginState,
+  getMemoryCapabilityRegistration,
  getMemoryRuntime,
  listActiveMemoryPublicArtifacts,
  listMemoryCorpusSupplements,
@@ -3121,6 +3122,93 @@ module.exports = { id: "throws-after-import", register() {} };`,
    clearPluginInteractiveHandlers();
  });

+  it("fails plugin registration when a hook is missing its required name", () => {
+    useNoBundledPlugins();
+    const plugin = writePlugin({
+      id: "nameless-hook",
+      filename: "nameless-hook.cjs",
+      body: `module.exports = {
+        id: "nameless-hook",
+        register(api) {
+          api.registerHook("gateway:startup", () => {});
+        },
+      };`,
+    });
+
+    clearInternalHooks();
+
+    const registry = loadOpenClawPlugins({
+      cache: false,
+      workspaceDir: plugin.dir,
+      config: {
+        plugins: {
+          load: { paths: [plugin.file] },
+          allow: ["nameless-hook"],
+        },
+      },
+      onlyPluginIds: ["nameless-hook"],
+    });
+
+    const record = registry.plugins.find((entry) => entry.id === "nameless-hook");
+    expect(record?.status).toBe("error");
+    expect(record?.failurePhase).toBe("register");
+    expect(record?.error).toContain("hook registration missing name");
+    expect(registry.hooks).toEqual([]);
+    expect(getRegisteredEventKeys()).toEqual([]);
+    expect(
+      registry.diagnostics.some(
+        (diag) =>
+          diag.pluginId === "nameless-hook" &&
+          diag.level === "error" &&
+          diag.message.includes("hook registration missing name"),
+      ),
+    ).toBe(true);
+
+    clearInternalHooks();
+  });
+
+  it("fails plugin registration when a non-memory plugin registers a memory capability", () => {
+    useNoBundledPlugins();
+    const plugin = writePlugin({
+      id: "invalid-memory-capability",
+      filename: "invalid-memory-capability.cjs",
+      body: `module.exports = {
+        id: "invalid-memory-capability",
+        register(api) {
+          api.registerMemoryCapability({
+            promptBuilder: () => ["should not register"],
+          });
+        },
+      };`,
+    });
+
+    const registry = loadOpenClawPlugins({
+      cache: false,
+      workspaceDir: plugin.dir,
+      config: {
+        plugins: {
+          load: { paths: [plugin.file] },
+          allow: ["invalid-memory-capability"],
+        },
+      },
+      onlyPluginIds: ["invalid-memory-capability"],
+    });
+
+    const record = registry.plugins.find((entry) => entry.id === "invalid-memory-capability");
+    expect(record?.status).toBe("error");
+    expect(record?.failurePhase).toBe("register");
+    expect(record?.error).toContain("only memory plugins can register a memory capability");
+    expect(getMemoryCapabilityRegistration()).toBeUndefined();
+    expect(
+      registry.diagnostics.some(
+        (diag) =>
+          diag.pluginId === "invalid-memory-capability" &&
+          diag.level === "error" &&
+          diag.message.includes("only memory plugins can register a memory capability"),
+      ),
+    ).toBe(true);
+  });
+
  it("can scope bundled provider loads to deepseek without hanging", () => {
    const scoped = loadOpenClawPlugins({
      cache: false,
--- a/src/plugins/registry.ts
+++ b/src/plugins/registry.ts
@@ -235,6 +235,17 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
    registry.diagnostics.push(diag);
  };

+  const throwRegistrationError = (message: string): never => {
+    throw new Error(message);
+  };
+
+  const requireRegistrationValue = (value: string | undefined, message: string): string => {
+    if (!value) {
+      throw new Error(message);
+    }
+    return value;
+  };
+
  const registerCodexAppServerExtensionFactory = (
    record: PluginRecord,
    factory: Parameters<OpenClawPluginApi["registerCodexAppServerExtensionFactory"]>[0],
@@ -414,23 +425,17 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
    const eventList = Array.isArray(events) ? events : [events];
    const normalizedEvents = eventList.map((event) => event.trim()).filter(Boolean);
    const entry = opts?.entry ?? null;
-    const name = entry?.hook.name ?? opts?.name?.trim();
-    if (!name) {
-      pushDiagnostic({
-        level: "warn",
-        pluginId: record.id,
-        source: record.source,
-        message: "hook registration missing name",
-      });
-      return;
-    }
-    const existingHook = registry.hooks.find((entry) => entry.entry.hook.name === name);
+    const hookName = requireRegistrationValue(
+      entry?.hook.name ?? opts?.name?.trim(),
+      "hook registration missing name",
+    );
+    const existingHook = registry.hooks.find((entry) => entry.entry.hook.name === hookName);
    if (existingHook) {
      pushDiagnostic({
        level: "error",
        pluginId: record.id,
        source: record.source,
-        message: `hook already registered: ${name} (${existingHook.pluginId})`,
+        message: `hook already registered: ${hookName} (${existingHook.pluginId})`,
      });
      return;
    }
@@ -441,7 +446,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
          ...entry,
          hook: {
            ...entry.hook,
-            name,
+            name: hookName,
            description,
            source: "openclaw-plugin",
            pluginId: record.id,
@@ -453,7 +458,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
        }
      : {
          hook: {
-            name,
+            name: hookName,
            description,
            source: "openclaw-plugin",
            pluginId: record.id,
@@ -466,7 +471,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
          invocation: { enabled: true },
        };

-    record.hookNames.push(name);
+    record.hookNames.push(hookName);
    registry.hooks.push({
      pluginId: record.id,
      entry: hookEntry,
@@ -483,7 +488,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
      return;
    }

-    const previousRegistrations = activePluginHookRegistrations.get(name) ?? [];
+    const previousRegistrations = activePluginHookRegistrations.get(hookName) ?? [];
    for (const registration of previousRegistrations) {
      unregisterInternalHook(registration.event, registration.handler);
    }
@@ -496,10 +501,10 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
      registerInternalHook(event, handler);
      nextRegistrations.push({ event, handler });
    }
-    activePluginHookRegistrations.set(name, nextRegistrations);
+    activePluginHookRegistrations.set(hookName, nextRegistrations);
    const rollbackEntries = pluginHookRollback.get(record.id) ?? [];
    rollbackEntries.push({
-      name,
+      name: hookName,
      previousRegistrations: [...previousRegistrations],
    });
    pluginHookRollback.set(record.id, rollbackEntries);
@@ -1562,13 +1567,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
              },
              registerMemoryCapability: (capability) => {
                if (!hasKind(record.kind, "memory")) {
-                  pushDiagnostic({
-                    level: "error",
-                    pluginId: record.id,
-                    source: record.source,
-                    message: "only memory plugins can register a memory capability",
-                  });
-                  return;
+                  throwRegistrationError("only memory plugins can register a memory capability");
                }
                if (
                  Array.isArray(record.kind) &&
@@ -1588,13 +1587,9 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
              },
              registerMemoryPromptSection: (builder) => {
                if (!hasKind(record.kind, "memory")) {
-                  pushDiagnostic({
-                    level: "error",
-                    pluginId: record.id,
-                    source: record.source,
-                    message: "only memory plugins can register a memory prompt section",
-                  });
-                  return;
+                  throwRegistrationError(
+                    "only memory plugins can register a memory prompt section",
+                  );
                }
                if (
                  Array.isArray(record.kind) &&
@@ -1620,13 +1615,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
              },
              registerMemoryFlushPlan: (resolver) => {
                if (!hasKind(record.kind, "memory")) {
-                  pushDiagnostic({
-                    level: "error",
-                    pluginId: record.id,
-                    source: record.source,
-                    message: "only memory plugins can register a memory flush plan",
-                  });
-                  return;
+                  throwRegistrationError("only memory plugins can register a memory flush plan");
                }
                if (
                  Array.isArray(record.kind) &&
@@ -1646,13 +1635,7 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) {
              },
              registerMemoryRuntime: (runtime) => {
                if (!hasKind(record.kind, "memory")) {
-                  pushDiagnostic({
-                    level: "error",
-                    pluginId: record.id,
-                    source: record.source,
-                    message: "only memory plugins can register a memory runtime",
-                  });
-                  return;
+                  throwRegistrationError("only memory plugins can register a memory runtime");
                }
                if (
                  Array.isArray(record.kind) &&