Skip to content

fix(desktop): resolve runtime plugin SDK namespaces lazily and break sdk cycle - #107753

Closed
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/desktop-sdk-runtime-cycle-107288
Closed

JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/desktop-sdk-runtime-cycle-107288

Conversation

@JoaoMarcos44

Copy link
Copy Markdown
Contributor

Summary

Resolves #107288 by breaking the static module cycle between sdk/index.ts and contrib/runtime-loader.ts, and making runtime plugin SDK namespace resolution lazy and null-safe.

Problem & Mechanism

Following the Capabilities → Plugins consolidation (#107212 and subsequent consolidation in commit 248ff2d3e8), sdk/index.ts was caught in a static module cycle:

sdk/index.ts
  → app/skills/index.tsx (re-exports SkillsView)
  → app/skills/plugins-tab.tsx (statically imported discoverRuntimePlugins)
  → contrib/runtime-loader.ts (statically imported sdk/runtime.ts)
  → sdk/runtime.ts (statically imported sdk/index.ts)
  → sdk/index.ts

In production and packaged desktop builds, Rolldown's module topological sorting emitted sdk/runtime.ts's GLOBALS object literal before the sdk export namespace was evaluated (hoisted unassigned var / TDZ). Consequently:

  • GLOBALS.__HERMES_PLUGIN_SDK__ was captured as undefined at module evaluation.
  • On desktop startup, when loading any runtime (disk) plugin, contrib/runtime-loader.ts:113 calls installPluginSdk(), which runs sdkImportMap() and shimUrl('__HERMES_PLUGIN_SDK__').
  • shimUrl executed Object.keys(GLOBALS[globalKey]) on undefined, throwing TypeError: Cannot convert undefined or null to object.
  • Every disk plugin failed to load on boot regardless of contents.

Solution

  1. Cycle Break: In apps/desktop/src/app/skills/plugins-tab.tsx, removed the top-level static import import { discoverRuntimePlugins } from '@/contrib/runtime-loader'. discoverRuntimePlugins is only called inside rescanAll() (an asynchronous action triggered by the user clicking the Rescan button or after an agent plugin update), so it is now loaded lazily:

    const { discoverRuntimePlugins } = await import('@/contrib/runtime-loader')
    await discoverRuntimePlugins()

    This severs the sole static link from app/skills to contrib/runtime-loader.ts, reducing paths from sdk/index.ts to contrib/runtime-loader.ts to 0.

  2. Call-Time Namespace Evaluation: In apps/desktop/src/sdk/runtime.ts, converted GLOBALS properties to getters:

    const GLOBALS = {
      get __HERMES_PLUGIN_SDK__() { return sdk },
      get __HERMES_REACT__() { return React },
      get __HERMES_REACT_JSX__() { return jsxRuntime },
      get __HERMES_REACT_JSX_DEV__() { return jsxDevRuntime }
    } as const

    When installPluginSdk() executes, Object.assign(globalThis, GLOBALS) evaluates the getters at call time, assigning the fully initialized namespace objects.

  3. Null-Guarded Shim Generation: In shimUrl(), guarded Object.keys(namespace ?? {}) against nullish values to ensure graceful link-time handling instead of throwing in the host loader.

Validation

  • Import Graph Analysis: Verified via graph traversal that directed paths from sdk/index.ts to contrib/runtime-loader.ts and sdk/runtime.ts are 0.
  • Unit Tests:
    • Added apps/desktop/src/sdk/runtime.test.ts testing global namespace assignment, import map construction, and safe evaluation.
    • Added test case to apps/desktop/src/app/skills/plugins-tab.test.tsx verifying rescan behavior.
    • All tests in plugins-tab.test.tsx (17 tests) and runtime.test.ts (3 tests) pass.
  • Typecheck & Lint:
    • npm run typecheck (tsc for renderer, electron, e2e): passed (exit code 0).
    • npm run lint (eslint across src/ and electron/): passed (0 errors).

Fixes #107288.

…sdk cycle

Break the static import cycle between sdk/index.ts and contrib/runtime-loader.ts by lazily importing discoverRuntimePlugins in plugins-tab.tsx upon manual rescan.

Additionally, make GLOBALS in sdk/runtime.ts evaluate namespaces through getters at call time and guard Object.keys in shimUrl against uninitialized namespaces, preventing TypeError when loading runtime desktop plugins.

Fixes NousResearch#107288.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Sep 10, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #107303 (earliest open fix for #107288 / #107352); #107518, #107603 and #107644 target the same bug. This PR also breaks the static import cycle in plugins-tab.tsx, which the others do not, so reviewers may prefer it or fold that piece into the canonical PR. Flagging the cluster so a maintainer picks one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: #107212 puts sdk/index.ts in a module cycle — every runtime (disk) desktop plugin fails with "Cannot convert undefined or null to object"

2 participants