fix(desktop): resolve plugin SDK namespaces lazily so disk plugins load in production builds - #107303
Conversation
runtime.ts captured the SDK namespaces (the plugin SDK, React and the two jsx runtimes) in a module-scope object literal, and that module sits in an import cycle: sdk/index -> @/contrib/* -> contrib/runtime-loader -> sdk/runtime -> sdk/index. In an unbundled (dev) graph the namespace object is live, so the capture works. In a production bundle the bundler emits the SDK namespace as a hoisted `var` whose assignment lands AFTER the literal that reads it, so the captured value is `undefined` (no TDZ error) and `Object.keys(GLOBALS[globalKey])` in `shimUrl()` throws "Cannot convert undefined or null to object". That throw happens inside `loadRuntimePlugin()` -- via `unsupportedImports()` -> `sdkImportMap()` -> `shimUrl()`, which run for every source before it is evaluated -- so it is content-independent: EVERY plugin loaded from $HERMES_HOME/desktop-plugins/<id>/plugin.js (and the desktop/plugin.js half of a unified package) fails to load, showing status "failed" in Capabilities -> Plugins. Resolve the namespaces at call time instead: `installPluginSdk()` and the shim builder only ever run once the app is up, so reading them there is always safe and statement ordering can no longer matter. Repro (production build only): build apps/desktop for production, drop any plugin.js into ~/.hermes/desktop-plugins/<id>/, start the app -> [plugins] runtime load failed (<id>) TypeError: Cannot convert undefined or null to object (.../assets/sdk-<hash>.js:5). Note: a vitest unit test cannot catch this (dev module graph keeps the namespace live); the faithful guard is a production-build smoke test that loads a fixture plugin through loadRuntimePlugin().
Competing fixes for the same regression (#107288 / #107291 / #107304 / #107312 — every disk desktop plugin fails after #107212): #107301 breaks the |
…ad in production builds The module-scope GLOBALS capture sat in an import cycle (sdk/index -> contrib/* -> contrib/runtime-loader -> sdk/runtime -> sdk/index). Production bundlers may evaluate it before the captured bindings are assigned, so Object.keys() threw 'Cannot convert undefined or null to object' and blocked EVERY disk plugin before any plugin code ran. Diagnosis credit: NousResearch#107303's byte-offset forensics. Namespaces are now read at call time via pluginNamespaces(), when module evaluation is long over. As a second layer, shimSource() emits a shim that fails loudly at import time, naming the specifier and the missing global, instead of killing map construction for unrelated plugins.
…ad in production builds Root cause: a top-level capturing the namespace import was emitted by the Rolldown bundler before the import's namespace variable was populated in the output bundle. This left as , causing every runtime (disk) plugin's shim blob to crash with in Object.keys(). Fix: replace the eager with a function that resolves the bindings at call time, after all imports are initialized. Both installPluginSdk() and shimUrl() now call this function lazily. Upstream tracking: NousResearch/hermes-agent issues NousResearch#107312, NousResearch#107304, NousResearch#107336, NousResearch#107288, NousResearch#107291, NousResearch#107352; PRs NousResearch#107303, NousResearch#107301, NousResearch#107309, NousResearch#107338, NousResearch#107405. This commit is a local fix — REMOVE when upstream merges one of those PRs.
…ad in production builds Root cause: a top-level capturing the namespace import was emitted by the Rolldown bundler before the import's namespace variable was populated in the output bundle. This left as , causing every runtime (disk) plugin's shim blob to crash with in Object.keys(). Fix: replace the eager with a function that resolves the bindings at call time, after all imports are initialized. Both installPluginSdk() and shimUrl() now call this function lazily. Upstream tracking: NousResearch/hermes-agent issues NousResearch#107312, NousResearch#107304, NousResearch#107336, NousResearch#107288, NousResearch#107291, NousResearch#107352; PRs NousResearch#107303, NousResearch#107301, NousResearch#107309, NousResearch#107338, NousResearch#107405. This commit is a local fix — REMOVE when upstream merges one of those PRs.
|
Independent Linux verification of #107303 using the test companion in #107405 at On Fedora 44 x86_64, Node 22.23.1 and Chromium headless shell 151.0.7922.34:
Follow-up quick check: rebased the two existing commits onto main For #107405's CI-placement question, I suggest a small Scope: disposable checkout and production Vite graph in Chromium, not installed Linux Electron/package acceptance. No running Desktop or gateway was changed. Full-suite and hosted-CI success are not claimed; existing Vite warnings remained, and Playwright used its Ubuntu fallback browser build on Fedora. Thanks for the fix and complementary tests. No competing implementation proposed. Verification and this draft were AI-assisted; recorded outputs and the rendered smoke screenshot were checked. |
|
Thanks — this fix matches exactly what I independently traced and hotfixed on the v0.21.1 packaged build (macOS arm64). Same root cause, same shape of fix:
One extra data point for the release checklist: the follow-on symptom users may see after a partial guard ( Thanks for the quick fix! 🎉 |
What breaks
In a production (bundled) build of the desktop app, every plugin loaded from disk fails to load:
$HERMES_HOME/desktop-plugins/<id>/plugin.jsand thedesktop/plugin.jshalf of a unified package alike. Capabilities → Plugins → Desktop plugins shows the row asfailedwith:It is not plugin-specific — the throw happens before any plugin code runs, so there is no plugin-side workaround.
Root cause
apps/desktop/src/sdk/runtime.tscaptured the SDK namespaces in a module-scope object literal:That module sits in an import cycle:
Object.keys(sdk)works — which is why this is invisible in dev, in vitest, and in review.var, and the bundler may order the two top-level statements either way. In the shipped 0.20.4 bundle it emits them like this (byte offsets inside one minified chunk):var Lg={__HERMES_PLUGIN_SDK__:Db,__HERMES_REACT__:J,__HERMES_REACT_JSX__:Y,…}var Db=t({…})— the plugin-SDK namespace itselfSo
Lg.__HERMES_PLUGIN_SDK__isundefinedwhen the literal is evaluated (var⇒ no TDZ error), and the firstObject.keys(GLOBALS[globalKey])insideshimUrl()throws exactlyTypeError: Cannot convert undefined or null to object.Why every disk plugin dies
loadRuntimePlugin()runsinstallPluginSdk()and thenunsupportedImports(source)→sdkImportMap()→shimUrl()for every source, before evaluating it, so the failure is content-independent.Reproduction (production build only)
apps/desktopfor production and run the packaged app.plugin.jsinto~/.hermes/desktop-plugins/<id>/.desktop.logshows:The fix
Resolve the namespaces at call time —
installPluginSdk()and the shim builder only ever run once the app is up, so reading them there is always safe and statement ordering stops mattering.installPluginSdk()now reads a freshpluginNamespaces(), andshimUrl()reads it per call.Testing note (honest)
A vitest unit test cannot catch this: in the dev module graph the namespace object is live, so the old code passes too — the regression is bundler-ordering-only. Two options, reviewer's call: rely on the structural fix (module scope no longer captures the namespaces at all), or add a production-build smoke test that loads a fixture plugin through
loadRuntimePlugin()and asserts it registers, which is the faithful reproducer.Impact
Any user updating to a build with this ordering loses every on-disk desktop plugin. Same-shape siblings worth an audit while you are here: any other module-scope capture of a value from the
sdk/index↔contribcycle.Fixes #107304