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
This commit is contained in:
Vincent Koc 2026-03-13 19:13:35 -07:00 committed by GitHub
parent 27e863ce40
commit bcbfbb831e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 140 additions and 11 deletions

View File

@ -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");
});
});

View File

@ -148,15 +148,22 @@ function resolveAdapterCapabilities(
const ADAPTERS_BY_CHANNEL_ACCOUNT = new Map<string, SessionBindingAdapter>();
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: {

View File

@ -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({

View File

@ -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;

View File

@ -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,