perf(build): stop unpacking node_modules wholesale from the Windows asar

apps/desktop/src/wsl/DesktopWslEnvironment.ts

@@ -229,15 +229,18 @@ const NODE_PTY_PROBE_SCRIPT = (
 printf 'nodeVersion:%s\\n' "$(node -p 'process.versions.node' 2>/dev/null)"
 printf 'resolvedPath:%s\\n' "$PATH"
 cd ${shellQuote(linuxServerDir)} && node <<'NODE' >/dev/null 2>&1
-// The server bundle externalizes its deps to node_modules, and the WSL Node
-// can't read inside app.asar, so confirm those deps are unpacked on the real
-// filesystem before reporting the backend healthy. "effect" is the framework
-// every server module imports; resolving it validates the whole node_modules
-// tree. Exit 3 marks this distinct from a node-pty problem so the caller can
-// report it accurately instead of letting the server crash on
-// ERR_MODULE_NOT_FOUND at launch (which, in wsl-only mode, would just fail to
-// launch with no fallback).
-try { require.resolve("effect"); } catch (_e) { process.exit(3); }
+// The WSL Node can't read inside app.asar, so confirm what the server needs is
+// unpacked on the real filesystem before reporting the backend healthy. Exit 3
+// marks this distinct from a node-pty prebuild problem so the caller can report
+// it accurately instead of letting the server crash on ERR_MODULE_NOT_FOUND at
+// launch (which, in wsl-only mode, would just fail to launch with no fallback).
+//
+// The sentinel must be a package the CLI bundle leaves external. It used to be
+// "effect", back when the bundle externalized its runtime deps and the whole
+// node_modules tree was unpacked. The bundle now inlines its JS dependencies,
+// so "effect" no longer exists on disk and only the native packages do —
+// resolving node-pty is what actually validates the unpacked tree.
+try { require.resolve("node-pty/package.json"); } catch (_e) { process.exit(3); }
 const fs = require("node:fs");
 const path = require("node:path");
 const pkgDir = path.dirname(require.resolve("node-pty/package.json"));
@@ -462,11 +465,12 @@ const ensureNodePtyImpl = (
       } as const;
     }
 
-    // Server dependencies (e.g. "effect") couldn't be resolved on the WSL
-    // filesystem — a packaging regression, since the server bundle needs its
-    // node_modules unpacked from the asar. Fatal so wsl-only mode falls back to
-    // Windows and dual mode surfaces the reason inline, instead of the server
-    // crash-looping on ERR_MODULE_NOT_FOUND once it actually launches.
+    // The packages the server bundle leaves external (node-pty and the other
+    // native addons) couldn't be resolved on the WSL filesystem — a packaging
+    // regression, since those must be unpacked from the asar. Fatal so wsl-only
+    // mode falls back to Windows and dual mode surfaces the reason inline,
+    // instead of the server crash-looping on ERR_MODULE_NOT_FOUND once it
+    // actually launches.
     if (probe.exitCode === 3) {
       return {
         ok: false,

apps/server/vite.config.ts

@@ -5,16 +5,17 @@ import baseConfig from "../../vite.config.ts";
 import { loadRepoEnv } from "../../scripts/lib/public-config.ts";
 import packageJson from "./package.json" with { type: "json" };
 
-const bundledPackagePrefixes = [
-  "@pierre/diffs",
-  "@t3tools/",
-  "effect-acp",
-  "effect-codex-app-server",
-];
+// The bundle used to inline only workspace packages, leaving every third-party
+// runtime dep external. External deps must exist on the real filesystem (the WSL
+// backend runs plain `wsl.exe -- node`, which cannot read inside an asar), so the
+// desktop build unpacked `**\/node_modules\/**` wholesale: 13,875 loose files to
+// support 20 native binaries. NSIS install time tracks file count, not bytes.
+//
+// Inverted here — bundle everything except the packages that genuinely cannot be
+// inlined. See scripts/lib/cli-external-packages.ts for what earns an exemption.
+import { shouldBundleCliDependency } from "../../scripts/lib/cli-external-packages.ts";
 
-export function shouldBundleCliDependency(id: string): boolean {
-  return bundledPackagePrefixes.some((prefix) => id.startsWith(prefix));
-}
+export { shouldBundleCliDependency };
 
 const repoEnv = loadRepoEnv();
 const cliBuildChannel = packageJson.version.includes("-nightly.") ? "nightly" : "latest";

scripts/build-desktop-artifact.ts

@@ -17,6 +17,7 @@ import {
   type WebAssetBrand,
 } from "./lib/brand-assets.ts";
 import { getDefaultBuildArch } from "./lib/build-target-arch.ts";
+import { CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS } from "./lib/cli-external-packages.ts";
 import { loadRepoEnv } from "./lib/public-config.ts";
 import { resolveCatalogDependencies } from "./lib/resolve-catalog.ts";
 
@@ -633,13 +634,21 @@ export const DESKTOP_FILE_EXCLUSIONS = [
   // are dead weight. The trailing dash keeps the SDK's own JS package.
   "!**/node_modules/@anthropic-ai/claude-agent-sdk-*/**/*",
 ] as const;
-// The WSL backend launches the server with plain `wsl.exe -- node`, which
-// cannot read inside an asar archive — and the server bundle externalizes its
-// runtime deps, so the whole node_modules tree must be unpacked, not just the
-// bundle (otherwise ERR_MODULE_NOT_FOUND: "Cannot find package 'effect'").
-// The Windows primary backend reads the same files through the asar redirect,
-// so nothing is duplicated.
-export const WINDOWS_ASAR_UNPACK = ["apps/server/dist/**", "**/node_modules/**"] as const;
+// The WSL backend launches the server with plain `wsl.exe -- node`, which cannot
+// read inside an asar archive, so everything it loads must be on the real
+// filesystem. This used to unpack `**\/node_modules\/**` wholesale, because the
+// server bundle externalized its runtime deps and the Linux Node would fail with
+// ERR_MODULE_NOT_FOUND ("Cannot find package 'effect'") before it even reached
+// node-pty.
+//
+// The CLI bundle now inlines its JS dependencies, so the only things that still
+// have to be loose are the server bundle itself and the packages the bundle
+// leaves external — derived from the same list the bundler uses, so the two
+// cannot drift apart.
+export const WINDOWS_ASAR_UNPACK = [
+  "apps/server/dist/**",
+  ...CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS,
+] as const;
 export const DESKTOP_EXTRA_RESOURCES = [
   {
     from: "apps/desktop/prod-resources/resource-monitor",

scripts/lib/cli-external-packages.test.ts

@@ -0,0 +1,196 @@
+import * as NodeURL from "node:url";
+
+import * as NodeServices from "@effect/platform-node/NodeServices";
+import { assert, describe, it } from "@effect/vitest";
+import * as Effect from "effect/Effect";
+import * as FileSystem from "effect/FileSystem";
+import * as Path from "effect/Path";
+import * as Schema from "effect/Schema";
+
+import {
+  CLI_EXTERNAL_PACKAGE_PREFIXES,
+  CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS,
+  CLI_RUNTIME_EXTERNAL_PREFIXES,
+  shouldBundleCliDependency,
+} from "./cli-external-packages.ts";
+
+// Only the field this test cares about; decoding ignores everything else.
+const PackageManifest = Schema.Struct({
+  dependencies: Schema.optional(Schema.Record(Schema.String, Schema.String)),
+});
+type PackageManifest = typeof PackageManifest.Type;
+
+const decodeManifest = Schema.decodeUnknownSync(Schema.fromJsonString(PackageManifest));
+
+describe("shouldBundleCliDependency", () => {
+  it("bundles ordinary runtime dependencies", () => {
+    for (const id of ["effect", "@effect/platform", "hono", "@t3tools/shared/hostProcess"]) {
+      assert.strictEqual(shouldBundleCliDependency(id), true, id);
+    }
+  });
+
+  it("never bundles node: builtins", () => {
+    assert.strictEqual(shouldBundleCliDependency("node:fs"), false);
+  });
+
+  it("leaves native addons and their dlopen wrappers external", () => {
+    for (const id of [
+      "node-pty",
+      "ffi-rs",
+      "@yuuang/ffi-rs-win32-x64-msvc",
+      "@ff-labs/fff-node",
+      "@clerk/electron-passkeys",
+      "msgpackr-extract",
+      "@msgpackr-extract/msgpackr-extract-win32-x64",
+    ]) {
+      assert.strictEqual(shouldBundleCliDependency(id), false, id);
+    }
+  });
+
+  it("leaves bun-only entry points external", () => {
+    assert.strictEqual(shouldBundleCliDependency("@effect/platform-bun"), false);
+    assert.strictEqual(shouldBundleCliDependency("@effect/sql-sqlite-bun"), false);
+  });
+
+  // The real package is `node-gyp-build-optional-packages`, reached by prefix.
+  // Matching it as external while failing to unpack it is invisible on the
+  // Windows primary (which reads app.asar) and breaks only under WSL.
+  it("treats prefix-matched siblings as external", () => {
+    assert.strictEqual(shouldBundleCliDependency("node-gyp-build-optional-packages"), false);
+  });
+});
+
+describe("CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS", () => {
+  it("unpacks every external prefix from both the top level and the pnpm store", () => {
+    for (const prefix of CLI_EXTERNAL_PACKAGE_PREFIXES) {
+      assert.include(CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS, `node_modules/${prefix}*/**/*`, prefix);
+      assert.include(
+        CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS,
+        `node_modules/.pnpm/**/node_modules/${prefix}*/**/*`,
+        prefix,
+      );
+    }
+  });
+
+  // Without the trailing `*` the globs stop covering prefix-matched siblings,
+  // which is exactly how a package ends up external but not unpacked.
+  it("keeps the trailing wildcard that matches prefix siblings", () => {
+    assert.include(CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS, "node_modules/node-gyp-build*/**/*");
+  });
+});
+
+// The failure this guards is invisible on Windows and fatal under WSL.
+//
+// An external package is loaded from the real filesystem, so its own `require`
+// also resolves from the real filesystem. If one of its dependencies was
+// bundled away instead of left external, that dependency exists only inside
+// app.asar — which the Windows primary reads transparently under
+// ELECTRON_RUN_AS_NODE, and plain `node` under WSL cannot.
+//
+// Found the hard way: node-gyp-build-optional-packages requires detect-libc,
+// which was bundled. Windows was fine; WSL got MODULE_NOT_FOUND.
+it.layer(NodeServices.layer)("external package dependency closure", (it) => {
+  // Read manifests off disk from the pnpm store rather than resolving them.
+  // `require("<name>/package.json")` cannot do this job: under pnpm isolation a
+  // transitive package (detect-libc, msgpackr-extract, ffi-rs) is not reachable
+  // by name from this file at all, and an `exports` map can refuse the
+  // `/package.json` subpath outright (@ff-labs/fff-node). Both surface as "not
+  // installed", which would let this test skip everything and pass while
+  // checking nothing. The store is also what asarUnpack globs target, so this
+  // reads the same tree the build packages.
+  const readInstalledPackages = Effect.gen(function* () {
+    const fileSystem = yield* FileSystem.FileSystem;
+    const path = yield* Path.Path;
+    const storeDir = path.resolve(
+      path.dirname(NodeURL.fileURLToPath(import.meta.url)),
+      "../../node_modules/.pnpm",
+    );
+
+    // The store holds regular files too (lock.yaml), so a path built under one
+    // raises ENOTDIR rather than reporting absence. That throws on Linux while
+    // Windows quietly returns false, which is exactly the kind of difference
+    // this test exists to catch, so treat any failure as "not there".
+    const isPresent = (candidate: string) =>
+      fileSystem.exists(candidate).pipe(Effect.orElseSucceed(() => false));
+
+    const installed = new Map<string, PackageManifest>();
+    if (!(yield* isPresent(storeDir))) return installed;
+
+    for (const entry of yield* fileSystem.readDirectory(storeDir)) {
+      const modulesDir = path.join(storeDir, entry, "node_modules");
+      if (!(yield* isPresent(modulesDir))) continue;
+
+      for (const owner of yield* fileSystem.readDirectory(modulesDir)) {
+        const names = owner.startsWith("@")
+          ? (yield* fileSystem.readDirectory(path.join(modulesDir, owner))).map(
+              (scoped) => `${owner}/${scoped}`,
+            )
+          : [owner];
+
+        for (const name of names) {
+          if (installed.has(name)) continue;
+          const manifestPath = path.join(modulesDir, name, "package.json");
+          if (!(yield* isPresent(manifestPath))) continue;
+          installed.set(name, decodeManifest(yield* fileSystem.readFileString(manifestPath)));
+        }
+      }
+    }
+    return installed;
+  }).pipe(Effect.cached, Effect.runSync);
+
+  // Runtime-external only. The build-only entries resolve `bun:*` and are never
+  // loaded by Node, so their closure genuinely does not need to be external.
+  const isRuntimeExternal = (name: string) =>
+    CLI_RUNTIME_EXTERNAL_PREFIXES.some((prefix) => name.startsWith(prefix));
+
+  it.effect("finds the runtime-external packages on disk", () =>
+    Effect.gen(function* () {
+      const installed = yield* readInstalledPackages;
+      const found = [...installed.keys()].filter(isRuntimeExternal);
+
+      // Without this the closure check below can pass vacuously: if nothing is
+      // read, nothing is checked. These are the packages whose closure actually
+      // broke WSL, so require them by name.
+      for (const required of ["node-pty", "node-gyp-build-optional-packages", "detect-libc"]) {
+        assert.ok(
+          found.includes(required),
+          `expected ${required} in the pnpm store; the closure check is only meaningful if it can read these (found ${found.length})`,
+        );
+      }
+    }),
+  );
+
+  it.effect("keeps every runtime dependency of an external package external too", () =>
+    Effect.gen(function* () {
+      const installed = yield* readInstalledPackages;
+      const violations: string[] = [];
+      const seen = new Set<string>();
+      // Seeded from what is actually installed and matches a prefix, so scoped
+      // prefixes like "@yuuang/" and "@ff-labs/" are covered too. Seeding from
+      // the prefix strings themselves would skip every scoped entry, since a
+      // prefix is not a package name.
+      const queue = [...installed.keys()].filter(isRuntimeExternal);
+
+      for (const name of queue) {
+        if (seen.has(name)) continue;
+        seen.add(name);
+
+        const manifest = installed.get(name);
+        if (!manifest) continue;
+
+        for (const dependency of Object.keys(manifest.dependencies ?? {})) {
+          if (!isRuntimeExternal(dependency)) {
+            violations.push(`${name} -> ${dependency}`);
+          }
+          if (!seen.has(dependency)) queue.push(dependency);
+        }
+      }
+
+      assert.deepStrictEqual(
+        violations,
+        [],
+        `these dependencies of external packages would be bundled away and fail to resolve under WSL: ${violations.join(", ")}`,
+      );
+    }),
+  );
+});

scripts/lib/cli-external-packages.ts

@@ -0,0 +1,82 @@
+/**
+ * The single source of truth for packages the server CLI bundle must NOT inline.
+ *
+ * Two consumers derive from this list, and they must never disagree:
+ *
+ * - apps/server/vite.config.ts decides what stays external to the bundle.
+ * - scripts/build-desktop-artifact.ts decides what gets unpacked out of the asar.
+ *
+ * A package that is external but not unpacked still resolves on the Windows
+ * primary, which runs under ELECTRON_RUN_AS_NODE and reads app.asar
+ * transparently. It fails only under WSL, where the backend is launched as plain
+ * `wsl.exe -- node` and cannot read inside an archive. That asymmetry makes the
+ * drift invisible on the platform you are most likely to test on, which is why
+ * both consumers derive from one list instead of maintaining their own.
+ *
+ * Entries are matched as prefixes (`id.startsWith(prefix)`), so they also cover
+ * a package's platform-specific siblings — `node-gyp-build` covers
+ * `node-gyp-build-optional-packages`, `@yuuang/` covers every `ffi-rs-*` binding.
+ */
+/**
+ * External because Node actually loads them from disk at runtime.
+ *
+ * Native addons (.node), the JS wrappers that dlopen them by real path, and —
+ * critically — the ordinary JS packages those wrappers require. An external
+ * package is loaded from the real filesystem, so its own `require` also
+ * resolves from the real filesystem; a dependency that was bundled away exists
+ * only inside app.asar and is unreachable there. This closure is enforced by a
+ * test, not by inspection.
+ */
+export const CLI_RUNTIME_EXTERNAL_PREFIXES = [
+  "node-pty",
+  "ffi-rs",
+  "@yuuang/",
+  "@ff-labs/",
+  "@clerk/electron-passkeys",
+  "@msgpackr-extract/",
+  "msgpackr-extract",
+  "node-gyp-build",
+  "node-addon-api",
+  // Required by node-gyp-build-optional-packages. Not native, but in the
+  // closure: without it, WSL gets MODULE_NOT_FOUND while Windows is fine.
+  "detect-libc",
+] as const;
+
+/**
+ * External only so the bundler never has to resolve them.
+ *
+ * These are reached through a runtime-conditional dynamic import that Node
+ * never takes, and they resolve `bun:*` specifiers that do not exist when
+ * bundling for Node. Because Node never loads them, their dependency closure
+ * does not need to be external — only the entry point must stay unbundled.
+ */
+export const CLI_BUILD_ONLY_EXTERNAL_PREFIXES = [
+  "@effect/platform-bun",
+  "@effect/sql-sqlite-bun",
+] as const;
+
+export const CLI_EXTERNAL_PACKAGE_PREFIXES = [
+  ...CLI_RUNTIME_EXTERNAL_PREFIXES,
+  ...CLI_BUILD_ONLY_EXTERNAL_PREFIXES,
+] as const;
+
+/** True when the CLI bundle should inline `id` rather than leave it external. */
+export function shouldBundleCliDependency(id: string): boolean {
+  if (id.startsWith("node:")) return false;
+  return !CLI_EXTERNAL_PACKAGE_PREFIXES.some((prefix) => id.startsWith(prefix));
+}
+
+/**
+ * asar-unpack globs covering every external package.
+ *
+ * The trailing `*` is what keeps these aligned with the prefix matching above:
+ * without it, `node-gyp-build` would be left external by the bundler and then
+ * not unpacked, because the real package is `node-gyp-build-optional-packages`.
+ *
+ * pnpm stores real files under `.pnpm` and symlinks the top-level names, so both
+ * paths are unpacked for the link target to exist on disk.
+ */
+export const CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS = CLI_EXTERNAL_PACKAGE_PREFIXES.flatMap(
+  (prefix) =>
+    [`node_modules/${prefix}*/**/*`, `node_modules/.pnpm/**/node_modules/${prefix}*/**/*`] as const,
+);