diff --git a/src/plugins/loader.test.ts b/src/plugins/loader.test.ts index 88951002ffe..aaab857e082 100644 --- a/src/plugins/loader.test.ts +++ b/src/plugins/loader.test.ts @@ -1770,6 +1770,60 @@ module.exports = { id: "throws-after-import", register() {} };`, clearInternalHooks(); }); + it("rolls back global hook and command side effects when registration fails", async () => { + useNoBundledPlugins(); + const plugin = writePlugin({ + id: "failing-side-effects", + filename: "failing-side-effects.cjs", + body: `module.exports = { + id: "failing-side-effects", + register(api) { + api.registerHook( + "gateway:startup", + (event) => { + event.messages.push("should-not-run"); + }, + { name: "failing-side-effects-hook" }, + ); + api.registerCommand({ + name: "failme", + description: "Fail me", + handler: async () => ({ text: "nope" }), + }); + throw new Error("boom"); + }, + };`, + }); + + clearInternalHooks(); + clearPluginCommands(); + + const registry = loadOpenClawPlugins({ + cache: false, + workspaceDir: plugin.dir, + config: { + plugins: { + load: { paths: [plugin.file] }, + allow: ["failing-side-effects"], + }, + }, + onlyPluginIds: ["failing-side-effects"], + }); + + expect(registry.plugins.find((entry) => entry.id === "failing-side-effects")?.status).toBe( + "error", + ); + expect(getRegisteredEventKeys()).toEqual([]); + expect(getPluginCommandSpecs()).toEqual([]); + + const event = createInternalHookEvent("gateway", "startup", "gateway:startup"); + await triggerInternalHook(event); + expect(event.messages).toEqual([]); + + clearInternalHooks(); + clearPluginCommands(); + }); + it("can scope bundled provider loads to deepseek without hanging", () => { const scoped = loadOpenClawPlugins({ cache: false, diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index 5f668a280cd..581ae019dac 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -1562,6 +1562,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi const { registry, createApi, + rollbackPluginGlobalSideEffects, registerReload, registerNodeHostCommand, registerSecurityAuditCollector, @@ -2236,6 +2237,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi registry.plugins.push(record); seenIds.set(pluginId, candidate.origin); } catch (err) { + rollbackPluginGlobalSideEffects(record.id); restorePluginRegistry(registry, registrySnapshot); restoreRegisteredAgentHarnesses(previousAgentHarnesses); restoreRegisteredCompactionProviders(previousCompactionProviders); diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index ed555ced8d0..565b1edf3fe 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -23,6 +23,7 @@ import { resolveUserPath } from "../utils.js"; import { buildPluginApi } from "./api-builder.js"; import { normalizeRegisteredChannelPlugin } from "./channel-validation.js"; import { registerPluginCommand, validatePluginCommandDefinition } from "./command-registration.js"; +import { clearPluginCommandsForPlugin } from "./command-registry-state.js"; import { getRegisteredCompactionProvider, registerCompactionProvider, @@ -173,9 +174,13 @@ const activePluginHookRegistrations = resolveGlobalSingleton< Map[1] }>> >(ACTIVE_PLUGIN_HOOK_REGISTRATIONS_KEY, () => new Map()); +type HookRegistration = { event: string; handler: Parameters[1] }; +type HookRollbackEntry = { name: string; previousRegistrations: HookRegistration[] }; + export function createPluginRegistry(registryParams: PluginRegistryParams) { const registry = createEmptyPluginRegistry(); const coreGatewayMethods = new Set(Object.keys(registryParams.coreGatewayHandlers ?? {})); + const pluginHookRollback = new Map(); const pushDiagnostic = (diag: PluginDiagnostic) => { registry.diagnostics.push(diag); @@ -303,6 +308,12 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { nextRegistrations.push({ event, handler }); } activePluginHookRegistrations.set(name, nextRegistrations); + const rollbackEntries = pluginHookRollback.get(record.id) ?? []; + rollbackEntries.push({ + name, + previousRegistrations: [...previousRegistrations], + }); + pluginHookRollback.set(record.id, rollbackEntries); }; const registerGatewayMethod = ( @@ -1413,9 +1424,37 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { }); }; + const rollbackPluginGlobalSideEffects = (pluginId: string) => { + if (registryParams.activateGlobalSideEffects === false) { + return; + } + + clearPluginCommandsForPlugin(pluginId); + + const hookRollbackEntries = pluginHookRollback.get(pluginId) ?? []; + for (const entry of hookRollbackEntries.toReversed()) { + const activeRegistrations = activePluginHookRegistrations.get(entry.name) ?? []; + for (const registration of activeRegistrations) { + unregisterInternalHook(registration.event, registration.handler); + } + + if (entry.previousRegistrations.length === 0) { + activePluginHookRegistrations.delete(entry.name); + continue; + } + + for (const registration of entry.previousRegistrations) { + registerInternalHook(registration.event, registration.handler); + } + activePluginHookRegistrations.set(entry.name, [...entry.previousRegistrations]); + } + pluginHookRollback.delete(pluginId); + }; + return { registry, createApi, + rollbackPluginGlobalSideEffects, pushDiagnostic, registerTool, registerChannel,