From b41445d092ec5c0bcf60be88cc4f739445dfbd61 Mon Sep 17 00:00:00 2001 From: breken Date: Mon, 5 Oct 2026 11:17:04 -0700 Subject: [PATCH] :bug: Keep background plugins registered when another plugin opens (#11929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading a plugin closed every non-background plugin and then emptied the runtime registry, dropping background plugins (allowBackground, such as the MCP plugin) that were left running. The registry is what routes a plugin iframe's postMessage traffic to its plugin and what ɵunloadPlugin searches, so after the user opened any other plugin the MCP plugin never received task requests from its UI (MCP tool calls timed out) and could no longer be unloaded. Keep background plugins in the registry and drop only the plugins that were closed. AI-assisted-by: claude-opus-5.5 Signed-off-by: breken-ai <312387581+breken-ai@users.noreply.github.com> Co-authored-by: breken-ai <312387581+breken-ai@users.noreply.github.com> --- .serena/memories/plugins/core.md | 2 +- plugins/CHANGELOG.md | 1 + .../src/lib/load-plugin.spec.ts | 42 +++++++++++++++++++ .../plugins-runtime/src/lib/load-plugin.ts | 10 ++++- 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/.serena/memories/plugins/core.md b/.serena/memories/plugins/core.md index b042c34aa8..64f4643860 100644 --- a/.serena/memories/plugins/core.md +++ b/.serena/memories/plugins/core.md @@ -26,7 +26,7 @@ ## Lifecycle -- Loading a plugin closes existing non-background plugins and resets the runtime registry. Be careful around `allowBackground` semantics when changing load/close behavior. +- Loading a plugin closes existing non-background plugins and removes them from the runtime registry. Background plugins (`allowBackground`) keep running and must stay registered: the registry routes their UI `postMessage` traffic by sender iframe and is what `ɵunloadPlugin` searches. - If sandbox evaluation fails, the runtime marks the error as plugin-originated, closes the plugin, and rethrows. - `plugin-manager` removes event listeners, timers, intervals, and modal state on close, and marks the plugin destroyed. Listener callbacks check that flag because Penpot events can fire after close. diff --git a/plugins/CHANGELOG.md b/plugins/CHANGELOG.md index 5c1e07dd74..4b45df68b0 100644 --- a/plugins/CHANGELOG.md +++ b/plugins/CHANGELOG.md @@ -16,6 +16,7 @@ - **plugins-runtime**: Removed the premature deep-hardening of the host plugin context, which froze shared host functions (including `Function.prototype`) before SES override taming, causing `TypeError: Cannot assign to read only property 'toString'` on later host-side function extension. Related to #11001. - **plugins-runtime**: The plugin modal no longer shows two resize grips in Firefox. Firefox now shows only its native grip. Closes #11795. - **plugins-runtime**: Fixed the `fontFamilies` token property mapping so `Shape.applyToken(token, ["fontFamilies"])` resolves to the canonical `:font-family` attribute and applied-token readback exposes the documented `fontFamilies` key instead of the undocumented singular `fontFamily`. Closes #11405. +- **plugins-runtime**: Opening a plugin no longer drops running background plugins (`allowBackground`, such as the MCP plugin) from the runtime registry. They kept running, but their UI messages were no longer delivered to the plugin and they could no longer be unloaded, so MCP tasks timed out after another plugin was opened. ## 1.5.0 (2026-07-08) diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts index 810ebd54b3..779619279c 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.spec.ts @@ -3,6 +3,7 @@ import { loadPlugin, ɵloadPlugin, ɵloadPluginByUrl, + ɵunloadPlugin, setContextBuilder, getPlugins, } from './load-plugin'; @@ -200,6 +201,47 @@ describe('plugin-loader', () => { expect(mockPluginApi1.plugin.sendMessage).not.toHaveBeenCalled(); }); + it('should keep background plugins registered when loading another plugin', async () => { + const backgroundIframeWindow = { nodeType: 1 } as unknown as Window; + const backgroundClose = vi.fn(); + const backgroundPluginApi = { + plugin: { + close: backgroundClose, + sendMessage: vi.fn(), + }, + iframeWindow: backgroundIframeWindow, + manifest: { + ...manifest, + pluginId: 'background-plugin', + allowBackground: true, + }, + } as unknown as Awaited>; + + vi.mocked(createPlugin).mockResolvedValue(backgroundPluginApi); + await loadPlugin(manifest); + + vi.mocked(createPlugin).mockResolvedValue(mockPluginApi); + await loadPlugin(manifest); + + expect(backgroundClose).not.toHaveBeenCalled(); + expect(getPlugins()).toContain(backgroundPluginApi); + + const event = new MessageEvent('message', { data: 'from-background' }); + Object.defineProperty(event, 'source', { value: backgroundIframeWindow }); + window.dispatchEvent(event); + + expect(backgroundPluginApi.plugin.sendMessage).toHaveBeenCalledWith( + 'from-background', + ); + + ɵunloadPlugin('background-plugin'); + expect(backgroundClose).toHaveBeenCalledTimes(1); + + // the runtime's close callback deregisters the plugin + vi.mocked(createPlugin).mock.calls[0][2](); + expect(getPlugins()).not.toContain(backgroundPluginApi); + }); + it('should load plugin using ɵloadPlugin', async () => { await ɵloadPlugin(manifest); diff --git a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts index dfd44f4971..e22e62c70c 100644 --- a/plugins/libs/plugins-runtime/src/lib/load-plugin.ts +++ b/plugins/libs/plugins-runtime/src/lib/load-plugin.ts @@ -17,14 +17,20 @@ export function setContextBuilder(builder: ContextBuilder) { export const getPlugins = () => plugins; const closeAllPlugins = () => { + // Background plugins keep running, so they must stay registered: the + // registry routes their UI messages and lets them be unloaded later. + const backgroundPlugins: typeof plugins = []; + plugins.forEach((pluginApi) => { /* eslint-disable @typescript-eslint/no-explicit-any */ - if (!(pluginApi.manifest as any)?.allowBackground) { + if ((pluginApi.manifest as any)?.allowBackground) { + backgroundPlugins.push(pluginApi); + } else { pluginApi.plugin.close(); } }); - plugins = []; + plugins = backgroundPlugins; }; window.addEventListener('message', (event) => {