From 7f66492e262770f71d50cea9712ce258a607d6c5 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 11:48:38 -0700 Subject: [PATCH] Plugins: reserve shared registry names --- src/plugins/loader.test.ts | 147 +++++++++++++++++++++++++++++++++++++ src/plugins/registry.ts | 42 +++++++++++ 2 files changed, 189 insertions(+) diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index c37cfbfd46c..ac6ff410268 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -986,6 +986,153 @@ describe("loadOpenClawPlugins", () => { expect(httpPlugin?.httpRoutes).toBe(1); }); + it("rejects duplicate plugin-visible hook names", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "hook-owner-a", + filename: "hook-owner-a.cjs", + body: `module.exports = { id: "hook-owner-a", register(api) { + api.registerHook("gateway:startup", () => {}, { name: "shared-hook" }); +} };`, + }); + const second = writePlugin({ + id: "hook-owner-b", + filename: "hook-owner-b.cjs", + body: `module.exports = { id: "hook-owner-b", register(api) { + api.registerHook("gateway:startup", () => {}, { name: "shared-hook" }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["hook-owner-a", "hook-owner-b"], + }, + }, + }); + + expect(registry.hooks.filter((entry) => entry.entry.hook.name === "shared-hook")).toHaveLength( + 1, + ); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "hook-owner-b" && + diag.message === "hook already registered: shared-hook (hook-owner-a)", + ), + ).toBe(true); + }); + + it("rejects duplicate plugin service ids", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "service-owner-a", + filename: "service-owner-a.cjs", + body: `module.exports = { id: "service-owner-a", register(api) { + api.registerService({ id: "shared-service", start() {} }); +} };`, + }); + const second = writePlugin({ + id: "service-owner-b", + filename: "service-owner-b.cjs", + body: `module.exports = { id: "service-owner-b", register(api) { + api.registerService({ id: "shared-service", start() {} }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["service-owner-a", "service-owner-b"], + }, + }, + }); + + expect(registry.services.filter((entry) => entry.service.id === "shared-service")).toHaveLength( + 1, + ); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "service-owner-b" && + diag.message === "service already registered: shared-service (service-owner-a)", + ), + ).toBe(true); + }); + + it("requires plugin CLI registrars to declare explicit command roots", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "cli-missing-metadata", + filename: "cli-missing-metadata.cjs", + body: `module.exports = { id: "cli-missing-metadata", register(api) { + api.registerCli(() => {}); +} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["cli-missing-metadata"], + }, + }); + + expect(registry.cliRegistrars).toHaveLength(0); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "cli-missing-metadata" && + diag.message === "cli registration missing explicit commands metadata", + ), + ).toBe(true); + }); + + it("rejects duplicate plugin CLI command roots", () => { + useNoBundledPlugins(); + const first = writePlugin({ + id: "cli-owner-a", + filename: "cli-owner-a.cjs", + body: `module.exports = { id: "cli-owner-a", register(api) { + api.registerCli(() => {}, { commands: ["shared-cli"] }); +} };`, + }); + const second = writePlugin({ + id: "cli-owner-b", + filename: "cli-owner-b.cjs", + body: `module.exports = { id: "cli-owner-b", register(api) { + api.registerCli(() => {}, { commands: ["shared-cli"] }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + cache: false, + config: { + plugins: { + load: { paths: [first.file, second.file] }, + allow: ["cli-owner-a", "cli-owner-b"], + }, + }, + }); + + expect(registry.cliRegistrars).toHaveLength(1); + expect(registry.cliRegistrars[0]?.pluginId).toBe("cli-owner-a"); + expect( + registry.diagnostics.some( + (diag) => + diag.level === "error" && + diag.pluginId === "cli-owner-b" && + diag.message === "cli command already registered: shared-cli (cli-owner-a)", + ), + ).toBe(true); + }); + it("registers http routes", () => { useNoBundledPlugins(); const plugin = writePlugin({ diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index ca987dc8e79..c1c63cc96cb 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -238,6 +238,16 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { }); return; } + const existingHook = registry.hooks.find((entry) => entry.entry.hook.name === name); + if (existingHook) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `hook already registered: ${name} (${existingHook.pluginId})`, + }); + return; + } const description = entry?.hook.description ?? opts?.description ?? ""; const hookEntry: HookEntry = entry @@ -473,6 +483,28 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { opts?: { commands?: string[] }, ) => { const commands = (opts?.commands ?? []).map((cmd) => cmd.trim()).filter(Boolean); + if (commands.length === 0) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: "cli registration missing explicit commands metadata", + }); + return; + } + const existing = registry.cliRegistrars.find((entry) => + entry.commands.some((command) => commands.includes(command)), + ); + if (existing) { + const overlap = commands.find((command) => existing.commands.includes(command)); + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `cli command already registered: ${overlap ?? commands[0]} (${existing.pluginId})`, + }); + return; + } record.cliCommands.push(...commands); registry.cliRegistrars.push({ pluginId: record.id, @@ -487,6 +519,16 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { if (!id) { return; } + const existing = registry.services.find((entry) => entry.service.id === id); + if (existing) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `service already registered: ${id} (${existing.pluginId})`, + }); + return; + } record.services.push(id); registry.services.push({ pluginId: record.id,