fix(desktop): ship the bundled Node's npm on Windows (#16381)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

AGENTS.md

@@ -704,6 +704,6 @@ When adding code that needs a new string, decide up front which rule it falls un
 
 - Electron desktop app name in dev (macOS) — `npm run desktop` shows the app as "Electron" in the Dock unless `scripts/brand-dev-electron.mjs` (wired as the `predesktop` hook) has run. There are **three independent name sources** and they must all be set; getting one wrong looks like the fix silently not working. (1) `app.name` — Electron-internal, drives the menu bar, About panel and `app.getPath("userData")`. It comes from `productName` in `electron/package.json`, read by Electron's `default_app` in dev and `lib/browser/init` when packaged. Note `default_app` only reads `<arg>/package.json`, so `npm run desktop` must point electron at the `electron/` **directory** — `electron electron/main.mjs` makes it probe `electron/main.mjs/package.json`, miss, and leave `app.name` at the host bundle default. (2) `CFBundleDisplayName` / `CFBundleName` in the running bundle's Info.plist — what `lsappinfo` and `NSRunningApplication.localizedName` report. (3) **The `.app` directory name — this is what the Dock tooltip actually shows.** macOS prefers the bundle's filesystem name over the plist keys; `/Applications/DBeaver.app` displays as "DBeaver" despite `CFBundleName = "DBeaver Community"`. So patching only the plist is NOT enough — the script also renames `node_modules/electron/dist/Electron.app` → `<productName>.app` and rewrites `node_modules/electron/path.txt` to match (`getElectronPath()` in `node_modules/electron/index.js` joins path.txt onto `dist/` and silently re-downloads Electron ~100 MB if it doesn't resolve, so the two must move together). `CFBundleExecutable` is deliberately left as `Electron` — `/Applications/Antigravity.app` ships that exact value and still displays correctly, so it only affects `ps`/Activity Monitor. Editing the plist does not break code signing: Electron's dist is ad-hoc *linker-signed* (`Info.plist=not bound`, `Sealed Resources=none`), so the signature covers only the Mach-O. `npm run build:desktop` is unaffected by the rename — electron-builder packages from `~/Library/Caches/electron/electron-v*.zip`, never from `node_modules/electron/dist`. The packaged app never had the problem: electron-builder emits `<productName>.app` with matching plist keys. Already-running instances keep the name they launched with, so quit and relaunch when verifying.
 
-- Electron desktop `node` / `npm` / `npx` PATH bridging — when the packaged `.app` is launched from Finder/Spotlight on macOS, the OS gives it a minimal PATH (`/usr/bin:/bin`). Homebrew, nvm, asdf installs of Node.js are invisible to spawned subprocesses. Two breakages flow from that: (1) backend launcher scripts that do `spawn("node", ...)` can't find Node; (2) most stdio MCP marketplace entries (Slack, GitHub, Figma, etc.) use `command: "npx"`, and when the agent-server tries to spawn them the missing `npx` makes the spawn fail with ENOENT — the SDK reports it as an `error_kind: "connection"` MCP test failure, which the install modal renders as `MCP$TEST_ERROR_CONNECTION` ("Could not reach the server. Check the URL and server type."), a misleading error since no URL is involved. **First fix attempt — DOES NOT WORK for stdio MCPs:** wrap `node`/`npm`/`npx` with thin shell scripts that run Electron with `ELECTRON_RUN_AS_NODE=1` against the package's CLI JS. That bridges the ENOENT but stdio JSON-RPC servers spawned through the wrapper exit with `McpError: Connection closed` before completing the MCP handshake — Electron-as-Node has subtly different stdin/stdout pipe semantics from a vanilla `node` binary when used as a stdio child of a windowed process. **Working fix:** bundle the real Node.js distribution. `scripts/download-node.mjs` downloads the official `node-v<ver>-<platform>-<arch>` tarball from `https://nodejs.org/dist/v<ver>/` into `resources/node/` (gitignored), prunes `include/`, `share/`, docs, and `node_modules/corepack` to keep the size down (~130 MB on Linux x64, dominated by the Node binary itself). Default pin: `NODE_BUNDLE_VERSION = "22.12.0"` (the repo's `engines.node` floor; every 22.x build shares the Electron 42 ABI); override with `NODE_VERSION=`. `electron-builder.config.mjs` ships `resources/node/` as an extraResource → `<Resources>/node/`. `electron/main.mjs::injectBundledNode()` prepends the platform-appropriate bin dir to `PATH` (POSIX: `<Resources>/node/bin`; Windows: `<Resources>/node/`) so subsequent spawns of `node`/`npm`/`npx` resolve to real binaries with full stdio fidelity. It also `chmod +x`'s the binaries on POSIX because electron-builder doesn't always preserve the bit. `injectBundledNode()` is a no-op when `!app.isPackaged` (dev `npm run desktop` uses the developer's system node). `build:desktop` and `build:desktop:universal` both run `download-node.mjs` after `download-uv.mjs`. If the bundled dir is missing at runtime, `injectBundledNode()` logs a loud `[desktop]` warning instead of silently leaving PATH bare.
+- Electron desktop `node` / `npm` / `npx` PATH bridging — when the packaged `.app` is launched from Finder/Spotlight on macOS, the OS gives it a minimal PATH (`/usr/bin:/bin`). Homebrew, nvm, asdf installs of Node.js are invisible to spawned subprocesses. Two breakages flow from that: (1) backend launcher scripts that do `spawn("node", ...)` can't find Node; (2) most stdio MCP marketplace entries (Slack, GitHub, Figma, etc.) use `command: "npx"`, and when the agent-server tries to spawn them the missing `npx` makes the spawn fail with ENOENT — the SDK reports it as an `error_kind: "connection"` MCP test failure, which the install modal renders as `MCP$TEST_ERROR_CONNECTION` ("Could not reach the server. Check the URL and server type."), a misleading error since no URL is involved. **First fix attempt — DOES NOT WORK for stdio MCPs:** wrap `node`/`npm`/`npx` with thin shell scripts that run Electron with `ELECTRON_RUN_AS_NODE=1` against the package's CLI JS. That bridges the ENOENT but stdio JSON-RPC servers spawned through the wrapper exit with `McpError: Connection closed` before completing the MCP handshake — Electron-as-Node has subtly different stdin/stdout pipe semantics from a vanilla `node` binary when used as a stdio child of a windowed process. **Working fix:** bundle the real Node.js distribution. `scripts/download-node.mjs` downloads the official `node-v<ver>-<platform>-<arch>` tarball from `https://nodejs.org/dist/v<ver>/` into `resources/node/` (gitignored), prunes `include/`, `share/`, docs, and `node_modules/corepack` to keep the size down (~130 MB on Linux x64, dominated by the Node binary itself). Default pin: `NODE_BUNDLE_VERSION = "22.12.0"` (the repo's `engines.node` floor; every 22.x build shares the Electron 42 ABI); override with `NODE_VERSION=`. `electron-builder.config.mjs` ships `resources/node/` as an extraResource → `<Resources>/node/`. `electron/main.mjs::injectBundledNode()` prepends the platform-appropriate bin dir to `PATH` (POSIX: `<Resources>/node/bin`; Windows: `<Resources>/node/`) so subsequent spawns of `node`/`npm`/`npx` resolve to real binaries with full stdio fidelity. It also `chmod +x`'s the binaries on POSIX because electron-builder doesn't always preserve the bit. `injectBundledNode()` is a no-op when `!app.isPackaged` (dev `npm run desktop` uses the developer's system node). `build:desktop` and `build:desktop:universal` both run `download-node.mjs` after `download-uv.mjs`. If the bundled dir is missing at runtime, `injectBundledNode()` logs a loud `[desktop]` warning instead of silently leaving PATH bare. **extraResources will not copy the distribution's root `node_modules`:** `app-builder-lib/src/util/filter.ts::createFilter` returns `false` for any entry whose path relative to the copy root is exactly `node_modules`, *before* the `filter` patterns are consulted, so no `filter` value can opt back in. The Windows Node zip puts npm at `<root>/node_modules/npm` and hits this exactly; POSIX tarballs put it at `<root>/lib/node_modules/npm` and are unaffected — which is why this only broke Windows. Shipped result: a working `node.exe` beside `npm.cmd`/`npx.cmd` shims pointing at a missing `node_modules\npm\bin\npx-cli.js`, so every `npx -y <pkg>` spawn dies with `MODULE_NOT_FOUND` *and* shadows the user's own npm, since the dir is PREPENDED to PATH. The `afterPack` hook (`restoreBundledNodeNpm`) copies that directory into the packed output and then hard-fails the build if `npm-cli.js` still isn't there, mirroring the check `download-node.mjs` already runs on the source tree.
 
 - Cloud conversation resume gating: when a cloud conversation is closed from the UI (`pauseCloudSandbox` is called), the conversation's `conversation_url` is NOT cleared -- it still points to the old sandbox host. `WebSocketProviderWrapper` must suppress the URL (pass `null` to `ConversationWebSocketProvider`) while `sandbox_status === "PAUSED"`, otherwise the WebSocket immediately tries the stale URL before the sandbox wakes. Symmetrically, `useActiveConversation`'s refetch interval must fast-poll (3 s) on both `!conversation_url` AND `sandbox_status === "PAUSED"` -- checking only the missing URL would leave the hook on the 30 s interval while the sandbox is resuming. The resume sequence: navigate -> sandbox PAUSED detected -> `resumeCloudSandbox` called (in `conversation.tsx`) -> fast-poll detects RUNNING -> `conversationUrl` unblocked -> WebSocket connects.

electron-builder.config.mjs

@@ -46,11 +46,14 @@
  * extraResources so Electron can inject it into PATH on startup.
  *
  * The bundled Node.js distribution (resources/node/) lands in
- * <Resources>/node/ via extraResources. Electron prepends its bin dir to
- * PATH at startup so backend scripts (`node scripts/ingress.mjs` etc.) and
- * stdio MCP servers spawned via `npx -y …` (Slack, GitHub, Figma, etc.)
- * can find a working node/npm/npx — the OS gives a Finder-launched .app
- * a minimal PATH (/usr/bin:/bin) that has none of those.
+ * <Resources>/node/ via extraResources — except for its root-level
+ * node_modules (npm itself), which electron-builder refuses to copy and the
+ * afterPack hook restores; see restoreBundledNodeNpm below.
+ *
+ * Electron prepends its bin dir to PATH at startup so backend scripts (`node
+ * scripts/ingress.mjs` etc.) and stdio MCP servers spawned via `npx -y …`
+ * (Slack, GitHub, Figma, etc.) can find a working node/npm/npx — the OS gives
+ * a Finder-launched .app a minimal PATH (/usr/bin:/bin) that has none of those.
  */
 
 import { cp, rm } from "node:fs/promises";
@@ -79,31 +82,39 @@ const rootPackageJson = JSON.parse(
   readFileSync(join(repoRoot, "package.json"), "utf8"),
 );
 
+/**
+ * Resolve the packaged app's Resources directory.
+ *
+ * On macOS it lives inside the `.app` bundle; on Linux/Windows it's a flat
+ * `resources/` subdirectory. We resolve both shapes from `context.appOutDir`
+ * + the productFilename.
+ */
+function packagedResourcesDir(context) {
+  const platform = context.electronPlatformName;
+  const productFilename = context.packager.appInfo.productFilename;
+  return platform === "darwin" || platform === "mas"
+    ? join(context.appOutDir, `${productFilename}.app`, "Contents", "Resources")
+    : join(context.appOutDir, "resources");
+}
+
+/**
+ * afterPack entry point. Invoked by electron-builder once per platform target
+ * after the unpacked directory has been populated but before installer-format
+ * packaging (DMG, NSIS, deb).
+ */
+async function afterPack(context) {
+  await stripBundledNodeModules(context);
+  await restoreBundledNodeNpm(context);
+}
+
 /**
  * Strip the auto-bundled node_modules from the packaged app, then restore
  * the small runtime closure of RUNTIME_PACKAGES.
  *
- * See the NODE_MODULES NOTE in the file header for the why. This is invoked
- * by electron-builder once per platform target after the unpacked directory
- * has been populated but before installer-format packaging (DMG, NSIS, deb).
- *
- * On macOS the app dir is inside a `.app` bundle; on Linux/Windows it's a
- * flat resources/ subdirectory. We resolve both shapes from
- * `context.appOutDir` + the productFilename.
+ * See the NODE_MODULES NOTE in the file header for the why.
  */
 async function stripBundledNodeModules(context) {
-  const platform = context.electronPlatformName;
-  const productFilename = context.packager.appInfo.productFilename;
-  const appDir =
-    platform === "darwin" || platform === "mas"
-      ? join(
-          context.appOutDir,
-          `${productFilename}.app`,
-          "Contents",
-          "Resources",
-          "app",
-        )
-      : join(context.appOutDir, "resources", "app");
+  const appDir = join(packagedResourcesDir(context), "app");
 
   const nm = join(appDir, "node_modules");
   if (existsSync(nm)) {
@@ -174,6 +185,72 @@ async function restoreRuntimeNodeModules(appDir) {
   );
 }
 
+/**
+ * Restore the bundled Node distribution's root-level `node_modules` (npm
+ * itself), which electron-builder silently refuses to copy, then verify the
+ * packed result.
+ *
+ * WHY THIS IS NEEDED — app-builder-lib's copy filter drops a `node_modules`
+ * directory sitting at the ROOT of a `from` dir, before any `filter` pattern
+ * is consulted (packages/app-builder-lib/src/util/filter.ts):
+ *
+ *     // filter the root node_modules, but not a subnode_modules
+ *     if (relative === "node_modules") {
+ *       return false
+ *     }
+ *
+ * That check is reached for extraResources too — `copyFiles()` builds the
+ * copy filter with the same `createFilter()`. Our extraResources entry copies
+ * `resources/node/` → `<Resources>/node/`, and the Windows Node zip puts npm
+ * at `<root>/node_modules/npm`, i.e. exactly `node_modules` relative to the
+ * copy root. So it is skipped, and no `filter` value can opt back in.
+ *
+ * POSIX tarballs put npm at `<root>/lib/node_modules/npm`, which is not the
+ * root entry and copies fine — which is why this only ever broke Windows and
+ * went unnoticed on macOS.
+ *
+ * Symptom when it goes wrong: `<Resources>/node/` ships a working `node.exe`
+ * next to `npm.cmd` / `npx.cmd` shims that exec
+ * `%~dp0\node_modules\npm\bin\npx-cli.js` — a file that isn't there. Every
+ * `npx -y <pkg>` spawn (stdio MCP servers, ACP servers) then dies with
+ * MODULE_NOT_FOUND, and because injectBundledNode() PREPENDS this directory
+ * to PATH it also shadows the user's own working npm installation.
+ */
+async function restoreBundledNodeNpm(context) {
+  const isWin = context.electronPlatformName === "win32";
+  const nodeDir = join(packagedResourcesDir(context), "node");
+
+  // No bundled Node at all — `npm run download-node` was skipped. That is a
+  // different (and already loudly reported) situation; main.mjs warns at
+  // startup. Don't turn it into a build failure here.
+  if (!existsSync(nodeDir)) return;
+
+  const srcNodeModules = join(repoRoot, "resources", "node", "node_modules");
+  const destNodeModules = join(nodeDir, "node_modules");
+  if (!existsSync(destNodeModules) && existsSync(srcNodeModules)) {
+    await cp(srcNodeModules, destNodeModules, { recursive: true });
+    const rel = relative(process.cwd(), destNodeModules);
+    // eslint-disable-next-line no-console -- electron-builder build log
+    console.log(
+      `[electron-builder] restored bundled Node node_modules: ${rel}`,
+    );
+  }
+
+  // Mirror the check scripts/download-node.mjs runs on the source tree, but
+  // against the packed output — the last point where a broken npm is still a
+  // build failure instead of a broken install.
+  const npmCli = isWin
+    ? join(nodeDir, "node_modules", "npm", "bin", "npm-cli.js")
+    : join(nodeDir, "lib", "node_modules", "npm", "bin", "npm-cli.js");
+  if (!existsSync(npmCli)) {
+    throw new Error(
+      `[electron-builder] packaged Node is missing npm-cli.js at ${npmCli} — ` +
+        "npm/npx would fail with MODULE_NOT_FOUND in the installed app. " +
+        "Run `npm run download-node` and rebuild.",
+    );
+  }
+}
+
 function getDirSizeBytes(dir) {
   // Synchronous walk so we can run it before the rm without async juggling
   // in the hook. The directory we're sizing is always small enough (<1 GB)
@@ -235,8 +312,10 @@ const config = {
   // Skip native-module rebuild — the app has no native deps.
   npmRebuild: false,
 
-  // Strip auto-bundled node_modules (see NODE_MODULES NOTE at top of file).
-  afterPack: stripBundledNodeModules,
+  // Strip auto-bundled node_modules (see NODE_MODULES NOTE at top of file),
+  // then restore the bundled Node distribution's own npm (see
+  // restoreBundledNodeNpm).
+  afterPack,
 
   // Files included in the packaged app.
   // Paths with `from` are relative to directories.app (electron/).

electron/main.mjs

@@ -186,6 +186,26 @@ function injectBundledNode() {
     return;
   }
 
+  // node.exe alone is not enough. npm / npx are wrapper scripts that exec
+  // npm's JS entry points out of the distribution's own node_modules, and
+  // that directory is the one piece electron-builder drops on Windows (see
+  // restoreBundledNodeNpm in electron-builder.config.mjs). Since we PREPEND
+  // this dir to PATH, a half-copied bundle doesn't just fail to help — it
+  // shadows the user's working npm with shims that die on MODULE_NOT_FOUND.
+  // Warn loudly, but still inject: `node` itself works and the backend
+  // launcher scripts need it.
+  const npmCli = isWin
+    ? join(nodeRoot, "node_modules", "npm", "bin", "npm-cli.js")
+    : join(nodeRoot, "lib", "node_modules", "npm", "bin", "npm-cli.js");
+  if (!existsSync(npmCli)) {
+    console.warn(
+      `[desktop] Bundled npm is incomplete — ${npmCli} is missing. ` +
+        "`npx`-launched subprocesses (stdio MCP servers, ACP servers) will " +
+        "fail with MODULE_NOT_FOUND, and this bundle shadows any npm already " +
+        "on PATH. Rebuild with `npm run download-node`.",
+    );
+  }
+
   // electron-builder doesn't always preserve the +x bit on POSIX. node, npm,
   // and npx need to be executable for shell PATH lookup to consider them.
   if (!isWin) {