From bcbfbb831e4e8dacce218b3cf6ac1e1f67fff63a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 19:13:35 -0700 Subject: [PATCH] Plugins: fail fast on channel and binding collisions (#45628) * Plugins: reject duplicate channel ids * Bindings: reject duplicate adapter registration * Plugins: fail on export id mismatch --- .../outbound/session-binding-service.test.ts | 20 ++++ src/infra/outbound/session-binding-service.ts | 17 +++- src/plugins/loader.test.ts | 94 +++++++++++++++++++ src/plugins/loader.ts | 10 +- src/plugins/registry.ts | 10 ++ 5 files changed, 140 insertions(+), 11 deletions(-) diff --git a/src/infra/outbound/session-binding-service.test.ts b/src/infra/outbound/session-binding-service.test.ts index 04a75d6e867..84fe5708802 100644 --- a/src/infra/outbound/session-binding-service.test.ts +++ b/src/infra/outbound/session-binding-service.test.ts @@ -198,4 +198,24 @@ describe("session binding service", () => { placements: [], }); }); + + it("rejects duplicate adapter registration for the same channel account", () => { + registerSessionBindingAdapter({ + channel: "discord", + accountId: "default", + bind: async (input) => createRecord(input), + listBySession: () => [], + resolveByConversation: () => null, + }); + + expect(() => + registerSessionBindingAdapter({ + channel: "Discord", + accountId: "DEFAULT", + bind: async (input) => createRecord(input), + listBySession: () => [], + resolveByConversation: () => null, + }), + ).toThrow("Session binding adapter already registered for discord:default"); + }); }); diff --git a/src/infra/outbound/session-binding-service.ts b/src/infra/outbound/session-binding-service.ts index ffbf04758e6..c155d3792b8 100644 --- a/src/infra/outbound/session-binding-service.ts +++ b/src/infra/outbound/session-binding-service.ts @@ -148,15 +148,22 @@ function resolveAdapterCapabilities( const ADAPTERS_BY_CHANNEL_ACCOUNT = new Map(); export function registerSessionBindingAdapter(adapter: SessionBindingAdapter): void { - const key = toAdapterKey({ - channel: adapter.channel, - accountId: adapter.accountId, - }); - ADAPTERS_BY_CHANNEL_ACCOUNT.set(key, { + const normalizedAdapter = { ...adapter, channel: adapter.channel.trim().toLowerCase(), accountId: normalizeAccountId(adapter.accountId), + }; + const key = toAdapterKey({ + channel: normalizedAdapter.channel, + accountId: normalizedAdapter.accountId, }); + const existing = ADAPTERS_BY_CHANNEL_ACCOUNT.get(key); + if (existing && existing !== adapter) { + throw new Error( + `Session binding adapter already registered for ${normalizedAdapter.channel}:${normalizedAdapter.accountId}`, + ); + } + ADAPTERS_BY_CHANNEL_ACCOUNT.set(key, normalizedAdapter); } export function unregisterSessionBindingAdapter(params: { diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 20d3fb22287..588def450af 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -825,6 +825,37 @@ describe("loadOpenClawPlugins", () => { expect(registry.diagnostics.some((d) => d.level === "error")).toBe(true); }); + it("fails when plugin export id mismatches manifest id", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "manifest-id", + filename: "manifest-id.cjs", + body: `module.exports = { id: "export-id", register() {} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["manifest-id"], + }, + }); + + const loaded = registry.plugins.find((entry) => entry.id === "manifest-id"); + expect(loaded?.status).toBe("error"); + expect(loaded?.error).toBe( + 'plugin id mismatch (config uses "manifest-id", export uses "export-id")', + ); + expect( + registry.diagnostics.some( + (entry) => + entry.level === "error" && + entry.pluginId === "manifest-id" && + entry.message === + 'plugin id mismatch (config uses "manifest-id", export uses "export-id")', + ), + ).toBe(true); + }); + it("registers channel plugins", () => { useNoBundledPlugins(); const plugin = writePlugin({ @@ -863,6 +894,69 @@ describe("loadOpenClawPlugins", () => { expect(channel).toBeDefined(); }); + it("rejects duplicate channel ids during plugin registration", () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "channel-dup", + filename: "channel-dup.cjs", + body: `module.exports = { id: "channel-dup", register(api) { + api.registerChannel({ + plugin: { + id: "demo", + meta: { + id: "demo", + label: "Demo Override", + selectionLabel: "Demo Override", + docsPath: "/channels/demo-override", + blurb: "override" + }, + capabilities: { chatTypes: ["direct"] }, + config: { + listAccountIds: () => [], + resolveAccount: () => ({ accountId: "default" }) + }, + outbound: { deliveryMode: "direct" } + } + }); + api.registerChannel({ + plugin: { + id: "demo", + meta: { + id: "demo", + label: "Demo Duplicate", + selectionLabel: "Demo Duplicate", + docsPath: "/channels/demo-duplicate", + blurb: "duplicate" + }, + capabilities: { chatTypes: ["direct"] }, + config: { + listAccountIds: () => [], + resolveAccount: () => ({ accountId: "default" }) + }, + outbound: { deliveryMode: "direct" } + } + }); +} };`, + }); + + const registry = loadRegistryFromSinglePlugin({ + plugin, + pluginConfig: { + allow: ["channel-dup"], + }, + }); + + expect(registry.channels.filter((entry) => entry.plugin.id === "demo")).toHaveLength(1); + expect( + registry.diagnostics.some( + (entry) => + entry.level === "error" && + entry.pluginId === "channel-dup" && + entry.message === "channel already registered: demo (channel-dup)", + ), + ).toBe(true); + }); + it("registers http routes with auth and match options", () => { useNoBundledPlugins(); const plugin = writePlugin({ diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 75882a5105b..18c0b4bfee2 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -779,12 +779,10 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi const register = resolved.register; if (definition?.id && definition.id !== record.id) { - registry.diagnostics.push({ - level: "warn", - pluginId: record.id, - source: record.source, - message: `plugin id mismatch (config uses "${record.id}", export uses "${definition.id}")`, - }); + pushPluginLoadError( + `plugin id mismatch (config uses "${record.id}", export uses "${definition.id}")`, + ); + continue; } record.name = definition?.name ?? record.name; diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index d45ff136a14..ca987dc8e79 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -419,6 +419,16 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { }); return; } + const existing = registry.channels.find((entry) => entry.plugin.id === id); + if (existing) { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: `channel already registered: ${id} (${existing.pluginId})`, + }); + return; + } record.channelIds.push(id); registry.channels.push({ pluginId: record.id,