From 5a402babdcc90ff1d5cb3bd6ac8b59ee8288d764 Mon Sep 17 00:00:00 2001 From: Telov <53878242+Telov@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:48:53 +0300 Subject: [PATCH] fix(desktop): ship the bundled Node's npm on Windows (#16381) Co-authored-by: Claude Opus 5 --- AGENTS.md | 2 +- electron-builder.config.mjs | 131 +++++++++++++++++++++++++++++------- electron/main.mjs | 20 ++++++ 3 files changed, 126 insertions(+), 27 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 936157a940..1255f08445 100644 --- a/AGENTS.md +++ b/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 `/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` → `.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 `.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--` tarball from `https://nodejs.org/dist/v/` 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 → `/node/`. `electron/main.mjs::injectBundledNode()` prepends the platform-appropriate bin dir to `PATH` (POSIX: `/node/bin`; Windows: `/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--` tarball from `https://nodejs.org/dist/v/` 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 → `/node/`. `electron/main.mjs::injectBundledNode()` prepends the platform-appropriate bin dir to `PATH` (POSIX: `/node/bin`; Windows: `/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 `/node_modules/npm` and hits this exactly; POSIX tarballs put it at `/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 ` 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. diff --git a/electron-builder.config.mjs b/electron-builder.config.mjs index f529292c0d..8548a82a13 100644 --- a/electron-builder.config.mjs +++ b/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 - * /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. + * /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/` → `/node/`, and the Windows Node zip puts npm + * at `/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 `/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: `/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 ` 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/). diff --git a/electron/main.mjs b/electron/main.mjs index 94231a4405..6806479836 100644 --- a/electron/main.mjs +++ b/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) {