🐛 Discard cancelled plugin loads after unloading

Cause:
- Disabling MCP during script loading could leave a late plugin instance
  attached to a stopped controller, blocking connection after re-enabling.

Fix:
- Track pending loads and discard instances cancelled by unload.
- Prevent duplicate global loads and preserve newer replacement instances.
- Clear pending state after failures so a subsequent load can retry.

Validation:
- Pass 84 runtime tests, the runtime build, lint and formatting checks.
- Reproduce the race with a delayed script in the browser, then verify
  re-enabling restores the session ID, themed indicator and MCP execution.

AI-assisted-by: gpt-6
Signed-off-by: Dominik Jain <dominik.jain@oraios-ai.de>
This commit is contained in:
Dominik Jain 2026-10-02 17:58:52 +00:00 committed by alonso.torres
parent 2bb6cd557e
commit 036f59266b
3 changed files with 87 additions and 4 deletions

View File

@ -2,7 +2,7 @@
### 🚀 Features
- **plugins-runtime, plugin-types**: Added global plugins (`scope: "global"`) that remain active across dashboard and workspace navigation until logout, with `penpotMgmt` for project and file discovery, file navigation and workspace events.
- **plugins-runtime, plugin-types**: Added global plugins (`scope: "global"`) that remain active across dashboard and workspace navigation until closed or logged out, with `penpotMgmt` for project and file discovery, file navigation and workspace events.
- **plugin-types:** Added `paddingType` (`'simple' | 'multiple'`) to flex and grid layouts and `marginType` (`'simple' | 'multiple'`) to layout children, exposing whether the four padding/margin sides are mirrored or honoured independently.
- **plugin-types**: Added `waitForLayoutUpdate` to wait until pending layout updates have finished. It rejects when the optional timeout elapses, defaulting to 30 seconds so a wait never hangs.
- **plugin-types**: Added `waitForLayoutUpdate` to the `Shape` interface to wait until the pending layout updates of a shape and its children have finished

View File

@ -1,6 +1,11 @@
import { describe, it, vi, expect, beforeAll } from 'vitest';
import 'ses';
import { loadPlugin, setContextBuilder, getPlugins } from './load-plugin';
import {
loadPlugin,
setContextBuilder,
getPlugins,
ɵunloadPlugin,
} from './load-plugin';
import type { Context } from '@penpot/plugin-types';
import type { Manifest } from './models/manifest.model.js';
@ -69,7 +74,7 @@ function makeHostFixture() {
// public penpot API. Plugin code must never see it (see B-2 below).
__internalSecret: 'host-internal',
} as unknown as Context;
return { context, listenerTypes, createRectangle, selection };
return { context, listeners, listenerTypes, createRectangle, selection };
}
function lastCompartmentGlobalThis(): Record<string, unknown> {
@ -171,6 +176,70 @@ describe('loadPlugin real initialization path (regression for #11001)', () => {
expect(fixture.createRectangle).toHaveBeenCalledTimes(1);
expect(lastCompartmentGlobalThis()['__selectionFrozen']).toBe(true);
});
it.each(['before', 'after'])(
'discards a cancelled global load that completes %s its replacement',
async (order) => {
const fixture = makeHostFixture();
setContextBuilder(() => fixture.context);
const manifest = {
...makeManifest('plugin.js', []),
host: 'https://plugins.test/',
scope: 'global' as const,
};
ɵunloadPlugin(manifest.pluginId);
let finishFetch!: (response: object) => void;
const response = new Promise((resolve) => {
finishFetch = resolve;
});
const fetch = vi.fn().mockReturnValue(response);
vi.stubGlobal('fetch', fetch);
try {
const cancelled = loadPlugin(manifest);
await loadPlugin(manifest);
expect(fetch).toHaveBeenCalledOnce();
ɵunloadPlugin(manifest.pluginId);
const replacement = () =>
loadPlugin({
...manifest,
host: '',
code: 'globalThis.marker = "replacement";',
});
if (order === 'after') await replacement();
finishFetch({
ok: true,
text: async () => 'globalThis.marker = "cancelled";',
});
await cancelled;
if (order === 'before') {
expect(getPlugins()).toHaveLength(0);
expect(fixture.listeners.size).toBe(0);
await replacement();
}
expect(getPlugins()).toHaveLength(1);
expect(lastCompartmentGlobalThis()['marker']).toBe('replacement');
expect(fixture.listeners.size).toBe(3);
} finally {
ɵunloadPlugin(manifest.pluginId);
vi.unstubAllGlobals();
}
},
);
it('allows retrying a global plugin after a failed load', async () => {
const fixture = makeHostFixture();
setContextBuilder(() => fixture.context);
const manifest = {
...makeManifest('throw new Error("load failed");', []),
scope: 'global' as const,
};
await expect(loadPlugin(manifest)).rejects.toThrow('load failed');
expect(getPlugins()).toHaveLength(0);
await loadPlugin({ ...manifest, code: 'globalThis.marker = "retry";' });
expect(lastCompartmentGlobalThis()['marker']).toBe('retry');
ɵunloadPlugin(manifest.pluginId);
expect(fixture.listeners.size).toBe(0);
});
it('keeps a global sandbox through navigation and other plugin loads, then closes it on logout', async () => {
const fixture = makeHostFixture();
let workspace = {

View File

@ -5,6 +5,7 @@ import { Manifest } from './models/manifest.model.js';
import { createPlugin } from './create-plugin.js';
let plugins: Awaited<ReturnType<typeof createPlugin>>[] = [];
const pendingPlugins = new Map<Manifest['pluginId'], symbol>();
export type ContextBuilder = (id: string) => Context;
@ -49,6 +50,7 @@ export const loadPlugin = async function (
closeCallback?: () => void,
apiExtensions?: object,
) {
const loadId = Symbol();
try {
const context = contextBuilder && contextBuilder(manifest.pluginId);
@ -58,6 +60,7 @@ export const loadPlugin = async function (
if (manifest.scope === 'global') {
if (
pendingPlugins.has(manifest.pluginId) ||
plugins.some(
(plugin) => plugin.manifest?.pluginId === manifest.pluginId,
)
@ -83,7 +86,9 @@ export const loadPlugin = async function (
// `createSandbox`'s proxy handler applies `ses.safeReturn` to values
// crossing into the sandbox. Compartment isolation and intrinsics
// hardening are performed by createSandbox, not here.
let plugin: Awaited<ReturnType<typeof createPlugin>> | undefined = undefined;
pendingPlugins.set(manifest.pluginId, loadId);
let plugin: Awaited<ReturnType<typeof createPlugin>> | undefined =
undefined;
plugin = await createPlugin(
context,
manifest,
@ -96,10 +101,18 @@ export const loadPlugin = async function (
},
apiExtensions,
);
if (pendingPlugins.get(manifest.pluginId) !== loadId) {
plugin.plugin.close();
return;
}
plugins.push(plugin);
} catch (error) {
if (manifest.scope !== 'global') closeAllPlugins();
throw error;
} finally {
if (pendingPlugins.get(manifest.pluginId) === loadId) {
pendingPlugins.delete(manifest.pluginId);
}
}
};
@ -117,6 +130,7 @@ export const ɵloadPluginByUrl = async function (manifestUrl: string) {
};
export const ɵunloadPlugin = function (id: Manifest['pluginId']) {
pendingPlugins.delete(id);
const plugin = plugins.find((plugin) => plugin.manifest.pluginId === id);
if (plugin) {