fix(desktop): stop missing plugin entries from flooding IPC logs

2026-08-21 21:32:39 +00:00
parent 503d863fcd
commit 6a4e212ee0
2 changed files with 152 additions and 43 deletions
@@ -22,6 +22,7 @@ const readFileText = vi.fn<(path: string) => Promise<{ text: string; truncated?:
 const readPluginSource = vi.fn<(path: string) => Promise<{ text: string; truncated?: boolean }>>()
 const watchDirectory = vi.fn<(path: string) => Promise<{ id: string }>>()
 const watchPreviewFile = vi.fn<(path: string) => Promise<{ id: string }>>()
+const stopPreviewFileWatch = vi.fn<(id: string) => Promise<boolean>>()
 const onPreviewFileChanged = vi.fn()

 beforeEach(() => {
@@ -32,6 +33,8 @@ beforeEach(() => {
  readPluginSource.mockReset()
  watchDirectory.mockReset()
  watchPreviewFile.mockReset()
+  stopPreviewFileWatch.mockReset()
+  stopPreviewFileWatch.mockResolvedValue(true)
  onPreviewFileChanged.mockReset()
  getStatus.mockClear()
  ;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = {
@@ -40,6 +43,7 @@ beforeEach(() => {
    onPreviewFileChanged,
    readDir,
    readFileText,
+    stopPreviewFileWatch,
    watchDirectory,
    watchPreviewFile
  }
@@ -74,22 +78,28 @@ describe('scanDiskPlugins (#66899)', () => {
    expect(readDir).not.toHaveBeenCalled()
  })

-  it('probes desktop/plugin.js inside agent-plugin packages (unified packaging)', async () => {
+  it('treats a package without a Desktop half as metadata, not a throwing file read', async () => {
    desktopPluginsRoot.mockResolvedValue('/local/.hermes/desktop-plugins')
    agentPluginsRoot.mockResolvedValue('/local/.hermes/plugins')
-    readDir.mockImplementation(async dir =>
-      dir === '/local/.hermes/plugins'
-        ? { entries: [{ isDirectory: true, name: 'my-feature', path: '/local/.hermes/plugins/my-feature' }] }
-        : { entries: [] }
-    )
-    // No desktop half in this package — probe must target desktop/plugin.js.
-    readFileText.mockRejectedValue(new Error('ENOENT'))
+    readDir.mockImplementation(async dir => {
+      if (dir === '/local/.hermes/plugins') {
+        return { entries: [{ isDirectory: true, name: 'my-feature', path: '/local/.hermes/plugins/my-feature' }] }
+      }
+
+      if (dir === '/local/.hermes/plugins/my-feature') {
+        return {
+          entries: [{ isDirectory: false, name: 'plugin.yaml', path: '/local/.hermes/plugins/my-feature/plugin.yaml' }]
+        }
+      }
+
+      return { entries: [] }
+    })

    await discoverRuntimePlugins()

-    expect(readFileText).toHaveBeenCalledWith('/local/.hermes/plugins/my-feature/desktop/plugin.js')
-    // The Python half's files must never be probed as a desktop entry.
-    expect(readFileText).not.toHaveBeenCalledWith('/local/.hermes/plugins/my-feature/plugin.js')
+    expect(readDir).toHaveBeenCalledWith('/local/.hermes/plugins/my-feature')
+    expect(readDir).not.toHaveBeenCalledWith('/local/.hermes/plugins/my-feature/desktop')
+    expect(readFileText).not.toHaveBeenCalled()
  })

  it('still scans the standalone root when agentPluginsRoot is absent (older shell)', async () => {
@@ -106,11 +116,35 @@ describe('scanDiskPlugins (#66899)', () => {
  it('loads a unified desktop half OPT-IN: inventoried but not activated by default', async () => {
    desktopPluginsRoot.mockResolvedValue('/local/.hermes/desktop-plugins')
    agentPluginsRoot.mockResolvedValue('/local/.hermes/plugins')
-    readDir.mockImplementation(async dir =>
-      dir === '/local/.hermes/plugins'
-        ? { entries: [{ isDirectory: true, name: 'uni', path: '/local/.hermes/plugins/uni' }] }
-        : { entries: [] }
-    )
+    let desktopEntryPresent = true
+
+    readDir.mockImplementation(async dir => {
+      if (dir === '/local/.hermes/plugins') {
+        return { entries: [{ isDirectory: true, name: 'uni', path: '/local/.hermes/plugins/uni' }] }
+      }
+
+      if (dir === '/local/.hermes/plugins/uni') {
+        return {
+          entries: [{ isDirectory: true, name: 'desktop', path: '/local/.hermes/plugins/uni/desktop' }]
+        }
+      }
+
+      if (dir === '/local/.hermes/plugins/uni/desktop') {
+        return {
+          entries: desktopEntryPresent
+            ? [
+                {
+                  isDirectory: false,
+                  name: 'plugin.js',
+                  path: '/local/.hermes/plugins/uni/desktop/plugin.js'
+                }
+              ]
+            : []
+        }
+      }
+
+      return { entries: [] }
+    })

    const register = vi.fn()

@@ -154,6 +188,14 @@ describe('scanDiskPlugins (#66899)', () => {
      await setPluginEnabled('uni', true)
      expect(register).toHaveBeenCalledTimes(1)
      expect($pluginRecords.get().uni.status).toBe('loaded')
+
+      // Removing only desktop/plugin.js (while the Python package folder
+      // remains) unloads the previous Desktop registration instead of leaving
+      // a live ghost behind.
+      desktopEntryPresent = false
+      await discoverRuntimePlugins()
+      expect($pluginRecords.get().uni).toBeUndefined()
+      expect(stopPreviewFileWatch).toHaveBeenCalledWith('w-uni')
    } finally {
      createObjectURL.mockRestore()
      revokeObjectURL.mockRestore()
@@ -209,15 +251,30 @@ describe('plugin source reads (512 KiB preview-cap bug)', () => {
    }
  }

+  /** Two-level standalone-root listing the metadata-walk probe needs:
+   *  the root lists the package folder, the folder lists plugin.js. */
+  const standaloneRootWith = (name: string) => {
+    const folder = `/local/.hermes/desktop-plugins/${name}`
+
+    readDir.mockImplementation(async dir => {
+      if (dir === '/local/.hermes/desktop-plugins') {
+        return { entries: [{ isDirectory: true, name, path: folder }] }
+      }
+
+      if (dir === folder) {
+        return { entries: [{ isDirectory: false, name: 'plugin.js', path: `${folder}/plugin.js` }] }
+      }
+
+      return { entries: [] }
+    })
+  }
+
  it('loads the full source via readPluginSource when the shell offers it', async () => {
    ;(window.hermesDesktop as unknown as { readPluginSource: unknown }).readPluginSource = readPluginSource
    desktopPluginsRoot.mockResolvedValue('/local/.hermes/desktop-plugins')
    agentPluginsRoot.mockResolvedValue('')
-    readDir.mockResolvedValue({
-      entries: [{ isDirectory: true, name: 'big', path: '/local/.hermes/desktop-plugins/big' }]
-    })
-    // The existence probe still uses the preview read (metadata is enough
-    // there) — it may report truncation without failing the scan.
+    standaloneRootWith('big')
+    // The preview read would truncate this source — it must never be used.
    readFileText.mockResolvedValue({ text: '// first 512 KiB only', truncated: true })

    const register = vi.fn()
@@ -246,9 +303,7 @@ describe('plugin source reads (512 KiB preview-cap bug)', () => {
  it('older shell without readPluginSource: a truncated preview read fails LOUDLY, never evaluates', async () => {
    desktopPluginsRoot.mockResolvedValue('/local/.hermes/desktop-plugins')
    agentPluginsRoot.mockResolvedValue('')
-    readDir.mockResolvedValue({
-      entries: [{ isDirectory: true, name: 'huge', path: '/local/.hermes/desktop-plugins/huge' }]
-    })
+    standaloneRootWith('huge')
    // 512 KiB window of a larger file — parses fine, but is NOT the plugin.
    readFileText.mockResolvedValue({
      text: 'export default { id: "huge", register: () => { throw new Error("must never evaluate") } }',
@@ -276,9 +331,7 @@ describe('plugin source reads (512 KiB preview-cap bug)', () => {
  it('older shell, small plugin (not truncated): still loads through readFileText', async () => {
    desktopPluginsRoot.mockResolvedValue('/local/.hermes/desktop-plugins')
    agentPluginsRoot.mockResolvedValue('')
-    readDir.mockResolvedValue({
-      entries: [{ isDirectory: true, name: 'small', path: '/local/.hermes/desktop-plugins/small' }]
-    })
+    standaloneRootWith('small')

    const register = vi.fn()

@@ -233,8 +233,10 @@ interface DiskRoot {
  /** Root-level enable posture, forwarded to the loader (see LoadOptions). */
  defaultEnabled?: boolean
  dir: string
-  /** Resolve a scanned folder to its candidate plugin entry file. */
-  entry: (folderPath: string) => string
+  /** Path segments below each scanned package folder. Discovery walks
+   *  directory metadata to this file instead of throwing a content read for
+   *  every ordinary package that has no Desktop half. */
+  entrySegments: readonly string[]
 }

 /** Both scan roots, resolved fresh each pass (Electron-local, never the
@@ -251,7 +253,7 @@ async function diskRoots(): Promise<DiskRoot[]> {
  const standalone = await desktop.desktopPluginsRoot?.()

  if (standalone) {
-    roots.push({ dir: standalone, entry: folder => `${folder}/plugin.js` })
+    roots.push({ dir: standalone, entrySegments: ['plugin.js'] })
  }

  const unified = await desktop.agentPluginsRoot?.()
@@ -261,7 +263,7 @@ async function diskRoots(): Promise<DiskRoot[]> {
    // user allowlists the Python half (plugins.enabled), so the desktop half
    // matches that posture — inventoried in Settings → Plugins, off until
    // toggled. The standalone desktop-plugins door keeps its default-on trust.
-    roots.push({ defaultEnabled: false, dir: unified, entry: folder => `${folder}/desktop/plugin.js` })
+    roots.push({ defaultEnabled: false, dir: unified, entrySegments: ['desktop', 'plugin.js'] })
  }

  return roots
@@ -323,7 +325,10 @@ async function readPluginSourceText(file: string): Promise<string> {
  return result.text
 }

-async function loadDiskPlugin(entry: DiskPlugin): Promise<void> {
+/** Returns false when the entry file could not be read (vanished mid-read) so
+ *  the caller can reconcile/unload the registration instead of retaining a
+ *  live ghost for a missing entry. */
+async function loadDiskPlugin(entry: DiskPlugin): Promise<boolean> {
  const prevId = entry.id

  try {
@@ -349,11 +354,14 @@ async function loadDiskPlugin(entry: DiskPlugin): Promise<void> {
    if (id && id !== entry.origin) {
      dropOriginRecord(entry.origin, entry)
    }
+
+    return true
  } catch (error) {
    // An oversize source is a REAL failure the user must see (the silent
    // shape was the bug: a truncated file evaluated as a syntax error, or
-    // worse, as half a plugin). Everything else is a file vanishing
-    // mid-read — the next scan reconciles, stay quiet.
+    // worse, as half a plugin). It is a completed read of an existing file,
+    // so report it and keep the registration (true) — everything else is a
+    // file vanishing mid-read, where false lets the caller reconcile/unload.
    if (error instanceof PluginSourceOversizeError) {
      console.error(`[plugins] ${entry.origin}: ${error.message}`)
      notifyError(error, `Plugin "${entry.origin}" failed to load`)
@@ -365,10 +373,45 @@ async function loadDiskPlugin(entry: DiskPlugin): Promise<void> {
        status: 'error',
        error: error.message
      })
+
+      return true
    }
+
+    return false
  }
 }

+async function resolveDiskPluginEntry(
+  desktop: Window['hermesDesktop'],
+  folderPath: string,
+  segments: readonly string[]
+): Promise<string | null> {
+  let currentDir = folderPath
+
+  for (let index = 0; index < segments.length; index += 1) {
+    const { entries } = await desktop.readDir(currentDir)
+    const entry = entries.find(candidate => candidate.name === segments[index])
+
+    if (!entry) {
+      return null
+    }
+
+    const last = index === segments.length - 1
+
+    if (last) {
+      return entry.isDirectory ? null : entry.path
+    }
+
+    if (!entry.isDirectory) {
+      return null
+    }
+
+    currentDir = entry.path
+  }
+
+  return null
+}
+
 async function scanDiskPlugins(): Promise<void> {
  const desktop = window.hermesDesktop

@@ -399,19 +442,24 @@ async function scanDiskPlugins(): Promise<void> {
      }

      for (const dir of entries.filter(e => e.isDirectory)) {
-        const file = root.entry(dir.path)
+        let file: string | null
+
+        try {
+          file = await resolveDiskPluginEntry(desktop, dir.path, root.entrySegments)
+        } catch {
+          continue // Folder changed during the metadata walk; the next tick reconciles.
+        }
+
+        if (!file) {
+          continue // Ordinary agent package with no Desktop half — not an error.
+        }
+
        seen.add(file)

        if (disk.has(file)) {
          continue
        }

-        try {
-          await desktop.readFileText(file)
-        } catch {
-          continue // No entry file (yet) — not a plugin folder for this root.
-        }
-
        const record: DiskPlugin = {
          defaultEnabled: root.defaultEnabled,
          file,
@@ -421,7 +469,11 @@ async function scanDiskPlugins(): Promise<void> {
        }

        disk.set(file, record)
-        await loadDiskPlugin(record)
+
+        if (!(await loadDiskPlugin(record))) {
+          disk.delete(file)
+          continue
+        }

        try {
          record.watchId = (await desktop.watchPreviewFile(file)).id
@@ -485,7 +537,11 @@ export function watchRuntimePlugins(): void {

    for (const record of disk.values()) {
      if (record.watchId === id) {
-        void loadDiskPlugin(record)
+        void loadDiskPlugin(record).then(readable => {
+          if (!readable) {
+            void scanDiskPlugins()
+          }
+        })

        return
      }