diff --git a/apps/desktop/src/contrib/plugin.ts b/apps/desktop/src/contrib/plugin.ts index 0953c6816bd08..076d7b6f15fb6 100644 --- a/apps/desktop/src/contrib/plugin.ts +++ b/apps/desktop/src/contrib/plugin.ts @@ -90,6 +90,20 @@ export interface PluginContext { * callback) can never outlive the plugin the way a bare `host.onEvent` * there would. */ onEvent: (type: string, listener: GatewayEventListener) => () => void + /** Scoped timers: cleared when the plugin unloads/reloads/disables, so a + * poller cannot outlive the plugin the way a bare `setInterval` does (the + * host never sees a bare global — it is the author's leak). Each returns + * a disposer that cancels early. */ + setTimeout: (fn: () => void, ms: number) => () => void + setInterval: (fn: () => void, ms: number) => () => void + /** Scoped `addEventListener` on any target (window, document, a node): + * removed on unload/reload/disable. Returns a disposer. */ + addEventListener: ( + target: EventTarget, + type: string, + listener: EventListenerOrEventListenerObject, + options?: AddEventListenerOptions | boolean + ) => () => void /** REST to this plugin's own backend namespace (`/api/plugins/`); `path` * is relative ('/board'). The sanctioned door for a plugin that ships a * `plugin_api.py` — profile-aware, namespace-scoped by construction. Use @@ -201,6 +215,61 @@ function createPluginOs(pluginId: string): PluginOs { } } +/** Timers and DOM listeners a plugin takes out through `ctx`, retired as ONE + * tracked disposer. A fired timeout drops out of the set on its own, so a + * long-lived plugin firing many one-shots does not accumulate cleanups. */ +function createPluginLifetime(track: (dispose: () => void) => () => void) { + const cleanups = new Set<() => void>() + let tracked = false + + const scoped = (cleanup: () => void) => { + // Registered with the host on first use, so a plugin that never takes a + // timer or listener out adds nothing to its disposer list. + if (!tracked) { + tracked = true + track(() => { + cleanups.forEach(pending => pending()) + cleanups.clear() + }) + } + + cleanups.add(cleanup) + + return () => { + cleanups.delete(cleanup) + cleanup() + } + } + + return { + setTimeout: (fn: () => void, ms: number) => { + const clear = () => globalThis.clearTimeout(id) + + const id = globalThis.setTimeout(() => { + cleanups.delete(clear) + fn() + }, ms) + + return scoped(clear) + }, + setInterval: (fn: () => void, ms: number) => { + const id = globalThis.setInterval(fn, ms) + + return scoped(() => globalThis.clearInterval(id)) + }, + addEventListener: ( + target: EventTarget, + type: string, + listener: EventListenerOrEventListenerObject, + options?: AddEventListenerOptions | boolean + ) => { + target.addEventListener(type, listener, options) + + return scoped(() => target.removeEventListener(type, listener, options)) + } + } +} + /** Build the scoped context handed to a plugin's `register`. `onDispose` * receives every registration's disposer (the loader's unload/reload hook). */ export function createPluginContext(pluginId: string, onDispose?: (dispose: () => void) => void): PluginContext { @@ -219,6 +288,7 @@ export function createPluginContext(pluginId: string, onDispose?: (dispose: () = registerMany: cs => track(registry.registerMany(cs.map(scope))), onDispose: fn => void track(fn), onEvent: (type, listener) => track(onGatewayEvent(type, listener)), + ...createPluginLifetime(track), rest: (path: string, opts?: PluginRestOptions) => pluginRest(pluginId, path, opts), socket: (path, onMessage) => track(pluginSocket(pluginId, path, onMessage)), os: createPluginOs(pluginId), diff --git a/apps/desktop/src/contrib/runtime-loader.test.ts b/apps/desktop/src/contrib/runtime-loader.test.ts index a914a8f68b2a5..ec261239369e1 100644 --- a/apps/desktop/src/contrib/runtime-loader.test.ts +++ b/apps/desktop/src/contrib/runtime-loader.test.ts @@ -771,6 +771,198 @@ describe('remote static imports are refused (catalog trust)', () => { }) }) +describe('loader hardening: hangs, leaks, duplicate ids, stale incarnations', () => { + const root = '/local/.hermes/desktop-plugins' + const counters = globalThis as unknown as Record + + const withBlobReroute = () => { + const createObjectURL = vi + .spyOn(URL, 'createObjectURL') + .mockImplementation( + blob => + `data:text/javascript;base64,${Buffer.from((blob as unknown as { parts: string[] }).parts.join('')).toString('base64')}` + ) + + const revokeObjectURL = vi.spyOn(URL, 'revokeObjectURL').mockImplementation(() => undefined) + const RealBlob = globalThis.Blob + vi.stubGlobal( + 'Blob', + class { + parts: string[] + constructor(parts: string[]) { + this.parts = parts + } + } + ) + + return () => { + createObjectURL.mockRestore() + revokeObjectURL.mockRestore() + vi.stubGlobal('Blob', RealBlob) + } + } + + /** Root listing of standalone folders (listed in the given order) whose + * plugin.js text comes from `sources[folder]`. */ + const rootWith = (sources: Record string>, order = Object.keys(sources)) => { + desktopPluginsRoot.mockResolvedValue(root) + readDir.mockImplementation(async dir => { + if (dir === root) { + return { entries: order.map(name => ({ isDirectory: true, name, path: `${root}/${name}` })) } + } + + const name = order.find(folder => dir === `${root}/${folder}`) + + return name + ? { entries: [{ isDirectory: false, name: 'plugin.js', path: `${root}/${name}/plugin.js` }] } + : { entries: [] } + }) + readFileText.mockImplementation(async file => ({ text: sources[file.split('/').at(-2)!]() })) + watchPreviewFile.mockImplementation(async file => ({ id: `w-${file}` })) + } + + /** Yield until the loader has armed its import deadline (the scan reaches + * `import()` through a chain of awaited mocks, all microtasks). */ + const untilTimerArmed = async () => { + for (let i = 0; i < 1_000 && vi.getTimerCount() === 0; i += 1) { + await Promise.resolve() + } + } + + afterEach(() => { + vi.useRealTimers() + }) + + it('a plugin whose import never settles times out as ITS error; the rest of the scan still loads', async () => { + const restore = withBlobReroute() + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + counters.__afterHangRegister = 0 + + try { + rootWith({ + 'aaa-hang': () => 'await new Promise(() => {})\nexport default { id: "hang", register() {} }', + 'bbb-ok': () => 'export default { id: "after-hang", register() { globalThis.__afterHangRegister++ } }' + }) + + const scan = discoverRuntimePlugins() + await untilTimerArmed() + await vi.advanceTimersByTimeAsync(10_000) + await scan + + expect($pluginRecords.get()['aaa-hang']).toMatchObject({ status: 'error', file: `${root}/aaa-hang/plugin.js` }) + expect($pluginRecords.get()['aaa-hang']?.error).toMatch(/import timed out/) + expect(counters.__afterHangRegister).toBe(1) + expect($pluginRecords.get()['after-hang']).toMatchObject({ status: 'loaded' }) + } finally { + unloadRuntimePlugin('after-hang') + delete counters.__afterHangRegister + restore() + } + }) + + it('ctx.setInterval / ctx.addEventListener registrations die with the plugin on unload', async () => { + const restore = withBlobReroute() + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'setInterval', 'clearInterval'] }) + counters.__scopedTicks = 0 + counters.__scopedEvents = 0 + + try { + await loadRuntimePlugin( + `export default { + id: 'scoped-lifetime', + register(ctx) { + ctx.setInterval(() => { globalThis.__scopedTicks++ }, 1000) + ctx.addEventListener(window, 'hermes-probe', () => { globalThis.__scopedEvents++ }) + } + }`, + 'scoped-lifetime' + ) + + expect($pluginRecords.get()['scoped-lifetime']).toMatchObject({ status: 'loaded' }) + await vi.advanceTimersByTimeAsync(3_000) + window.dispatchEvent(new Event('hermes-probe')) + expect(counters.__scopedTicks).toBe(3) + expect(counters.__scopedEvents).toBe(1) + + unloadRuntimePlugin('scoped-lifetime') + await vi.advanceTimersByTimeAsync(3_000) + window.dispatchEvent(new Event('hermes-probe')) + expect(counters.__scopedTicks).toBe(3) + expect(counters.__scopedEvents).toBe(1) + } finally { + unloadRuntimePlugin('scoped-lifetime') + delete counters.__scopedTicks + delete counters.__scopedEvents + restore() + } + }) + + it('two folders claiming one id: the first (sorted) owns it, the later one errors on its own row', async () => { + const restore = withBlobReroute() + counters.__dupAlpha = 0 + counters.__dupBeta = 0 + + try { + // Listed beta-first: the loader sorts, so alpha still wins deterministically. + rootWith( + { + alpha: () => 'export default { id: "dup", register() { globalThis.__dupAlpha++ } }', + beta: () => 'export default { id: "dup", register() { globalThis.__dupBeta++ } }' + }, + ['beta', 'alpha'] + ) + + await discoverRuntimePlugins() + + expect(counters.__dupAlpha).toBe(1) + expect(counters.__dupBeta).toBe(0) + expect($pluginRecords.get().dup).toMatchObject({ status: 'loaded', file: `${root}/alpha/plugin.js` }) + expect($pluginRecords.get().beta).toMatchObject({ status: 'error', file: `${root}/beta/plugin.js` }) + expect($pluginRecords.get().beta?.error).toMatch(/duplicate id "dup", already loaded from .*alpha\/plugin\.js/) + } finally { + unloadRuntimePlugin('dup') + delete counters.__dupAlpha + delete counters.__dupBeta + restore() + } + }) + + it('a save that no longer loads retires the previous incarnation instead of leaving it live', async () => { + const restore = withBlobReroute() + counters.__staleHits = 0 + + let source = ` + import { host } from '@hermes/plugin-sdk' + export default { + id: 'stale', + register() { host.onEvent('bot_relay.outbox.pending', () => { globalThis.__staleHits++ }) } + } + ` + + try { + rootWith({ 'stale-folder': () => source }) + + await discoverRuntimePlugins() + emitGatewayEvent({ type: 'bot_relay.outbox.pending' } as never) + expect(counters.__staleHits).toBe(1) + expect($pluginRecords.get().stale).toMatchObject({ status: 'loaded' }) + + // Mid-edit save: the file on disk is now broken. + source = 'export default {' + await discoverRuntimePlugins() + + emitGatewayEvent({ type: 'bot_relay.outbox.pending' } as never) + expect(counters.__staleHits).toBe(1) + expect($pluginRecords.get().stale).toBeUndefined() + expect($pluginRecords.get()['stale-folder']).toMatchObject({ status: 'error' }) + } finally { + unloadRuntimePlugin('stale') + delete counters.__staleHits + restore() + } + }) +}) + describe('manual "Reload desktop plugins" (#91503)', () => { it('re-reads an already-known plugin.js path and swaps in the new module', async () => { const root = '/local/.hermes/desktop-plugins' diff --git a/apps/desktop/src/contrib/runtime-loader.ts b/apps/desktop/src/contrib/runtime-loader.ts index 2901a07207f75..6eab298ea23fc 100644 --- a/apps/desktop/src/contrib/runtime-loader.ts +++ b/apps/desktop/src/contrib/runtime-loader.ts @@ -7,8 +7,11 @@ * -> blob `import()` -> validate default HermesPlugin -> register(ctx) * * Loading the same plugin id again disposes the previous registrations first - * (agent rewrites a plugin file -> clean reload). Failures toast + log; a - * broken plugin can never take the app down. + * (agent rewrites a plugin file -> clean reload) — everything taken out + * through `ctx` (contributions, events, sockets, `ctx.setInterval`/ + * `ctx.addEventListener`); bare globals and module-scope state are the + * plugin's own. Failures toast + log; a broken plugin can never take the app + * down, and a module whose evaluation never settles times out on its own row. * * Sources today: the in-repo runtime example (`?raw`, proves the pipeline) * and the two on-disk doors — `/desktop-plugins//plugin.js` @@ -56,6 +59,11 @@ interface LoadOptions { /** Live runtime plugins: id -> disposers (unload/reload support). */ const loaded = new Map void)[]>() +/** Module evaluation deadline. A top-level `await` that never settles (a dead + * host, a gateway that is not up) would otherwise hang `import()` forever — + * and, through the disk scan's sequential loop, every plugin listed after it. */ +const IMPORT_TIMEOUT_MS = 10_000 + // Matches the specifier of a static `from '…'`, a side-effect `import '…'`, or // a dynamic `import('…')`. Deliberately loose — a sentence ending in `from`, a // quoted example, a commented-out import all match it — so a match is honoured @@ -265,10 +273,20 @@ export async function loadRuntimePlugin( const url = URL.createObjectURL(new Blob([rewriteSpecifiers(source)], { type: 'text/javascript' })) let mod: { default?: HermesPlugin } + let deadline: ReturnType | undefined try { - mod = await import(/* @vite-ignore */ url) + mod = await Promise.race([ + import(/* @vite-ignore */ url) as Promise<{ default?: HermesPlugin }>, + new Promise((_, reject) => { + deadline = setTimeout( + () => reject(new Error(`import timed out after ${IMPORT_TIMEOUT_MS / 1000}s — module evaluation never settled`)), + IMPORT_TIMEOUT_MS + ) + }) + ]) } finally { + clearTimeout(deadline) URL.revokeObjectURL(url) } @@ -300,6 +318,16 @@ export async function loadRuntimePlugin( return null } + // Two files claiming one id (a standalone install beside a unified-package + // copy): the FIRST loaded owns the id. Silently letting the second win + // disposed the first's registrations and made each file's hot-reload flip + // ownership; instead the later file errors on its own folder row. + const owner = $pluginRecords.get()[plugin.id] + + if (owner && owner.file !== options.file) { + throw new Error(`duplicate id "${plugin.id}", already loaded from ${owner.file ?? owner.kind}`) + } + const record = { id: plugin.id, name: plugin.name ?? plugin.id, @@ -536,15 +564,18 @@ async function loadDiskPlugin(entry: DiskPlugin): Promise { packageOrigin: entry.packageOrigin }) - // A hot-edit that changes `plugin.id`: loadRuntimePlugin only disposes the - // NEW id, so unload the previous incarnation here or its contributions + - // inventory row orphan. - if (id && prevId && prevId !== id) { + // loadRuntimePlugin only disposes the NEW id, so the previous incarnation + // is unloaded here when the file no longer yields it: a hot-edit that + // changes `plugin.id`, or a save that no longer loads at all (syntax + // error, timeout, duplicate). Otherwise the old module's contributions and + // its activate handle stay live beside the error row — the Plugins tab + // would show a broken file as "loaded" and re-enable stale code. + if (prevId && prevId !== id) { unloadRuntimePlugin(prevId) dropPlugin(prevId) } - entry.id = id ?? entry.id + entry.id = id // A fixing save under a different plugin id — drop the folder-named // error record so the inventory shows one row, not a ghost. @@ -660,7 +691,11 @@ async function scanDiskPlugins(reloadKnown = false): Promise { continue // Root missing (no plugins yet) — the poll/watch reconciles. } - for (const dir of entries.filter(e => e.isDirectory)) { + // Listing order is filesystem order; sorted so duplicate-id ownership + // (first loaded wins) is the same on every launch. + const folders = entries.filter(e => e.isDirectory).sort((a, b) => a.name.localeCompare(b.name)) + + for (const dir of folders) { let file: string | null try { diff --git a/website/docs/developer-guide/desktop-plugin-sdk.md b/website/docs/developer-guide/desktop-plugin-sdk.md index e1685208a9312..1723498d2314f 100644 --- a/website/docs/developer-guide/desktop-plugin-sdk.md +++ b/website/docs/developer-guide/desktop-plugin-sdk.md @@ -177,6 +177,12 @@ interface PluginContext { socket: (path: string, onMessage: (data: unknown) => void) => () => void /** Gateway event stream by type (`'*'` = all). Tracked: removed on unload/reload/disable. */ onEvent: (type: string, listener: (event: GatewayEvent) => void) => () => void + /** Any other cleanup to run on unload/reload/disable (store subscriptions, injected DOM). */ + onDispose: (fn: () => void) => void + /** Scoped timers and DOM listeners — cleared with the plugin. Each returns a disposer. */ + setTimeout: (fn: () => void, ms: number) => () => void + setInterval: (fn: () => void, ms: number) => () => void + addEventListener: (target: EventTarget, type: string, listener: EventListener, options?: AddEventListenerOptions | boolean) => () => void /** The curated OS door: native notification, open-external, reveal-in-file-manager, clipboard. */ os: PluginOs /** Plugin-scoped JSON persistence (keys live under `hermes.plugin..`). */ @@ -934,6 +940,17 @@ pipeline as a trust boundary. the canvas (width/height attributes, not just CSS) — panes resize constantly. - **Don't poll faster than a few seconds** with `host.request`; prefer `host.onEvent` / `ctx.socket` and let React Query dedupe. +- **Bare globals are not tracked.** `window.setInterval`, `window.addEventListener`, + a `