Desktop plugin loader: hung imports time out, timers die on disable, duplicate ids error, broken reloads retire stale code

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/<id>`); `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: <T>(path: string, opts?: PluginRestOptions) => pluginRest<T>(pluginId, path, opts),
     socket: (path, onMessage) => track(pluginSocket(pluginId, path, onMessage)),
     os: createPluginOs(pluginId),

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<string, number | undefined>
+
+  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, () => 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'

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 — `<hermes home>/desktop-plugins/<name>/plugin.js`
@@ -56,6 +59,11 @@ interface LoadOptions {
 /** Live runtime plugins: id -> disposers (unload/reload support). */
 const loaded = new Map<string, (() => 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<typeof setTimeout> | undefined
 
     try {
-      mod = await import(/* @vite-ignore */ url)
+      mod = await Promise.race([
+        import(/* @vite-ignore */ url) as Promise<{ default?: HermesPlugin }>,
+        new Promise<never>((_, 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<boolean> {
       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<void> {
         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 {