From ba61d1fe4db9d5b552c7b9a5402183266e6af56f Mon Sep 17 00:00:00 2001 From: tsouth89 Date: Sun, 9 Aug 2026 16:17:30 -0400 Subject: [PATCH 1/4] perf(build): stop unpacking node_modules wholesale from the Windows asar A Windows installer built from main writes 14,687 files, of which 13,875 are loose node_modules files under app.asar.unpacked. Only 20 of them are native .node binaries. For contrast, the entire Electron runtime -- several hundred MB -- is 22 files, because it stays inside the archive. That file count costs twice. NSIS install time tracks file count, not bytes. And every one of those files is a separate open/stat/scan the first time the server starts after an install, which is exactly when the OS file cache is cold and the on-access virus scanner is not. The blanket `**/node_modules/**` unpack exists because the CLI bundle externalizes its runtime dependencies, and the WSL backend launches plain `wsl.exe -- node`, which cannot read inside an asar. So every external dep has to be a real file on disk. Invert the bundler's rule: bundle everything except the packages that genuinely cannot be inlined -- native addons, the JS wrappers that dlopen them, and the Bun-only entry points that resolve `bun:*` specifiers -- then narrow asarUnpack to exactly that set. Measured on this tree, win/nsis x64: files written at install 14,687 -> 1,192 (-92%) loose node_modules files 13,875 -> 370 native .node binaries 20 -> 20 installer size 145.0 MiB -> 138.9 MiB Cold start improves by the same mechanism. Extracting each build's payload to a fresh directory (so the files have never been read) and booting the server: server boot to "Listening on" 9044ms / 10160ms -> 3667ms / 3779ms module load only (--version) 6521 / 6238 / 6208ms -> 761 / 659 / 654ms Run order was alternated between builds to keep cache and scanner state from favouring either one. The desktop main window is not created until the backend answers HTTP, so that ~6s comes straight off a cold launch. Both consumers now derive from one list in scripts/lib/cli-external-packages.ts. They cannot drift, and the drift is worth guarding: 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. `node-gyp-build-optional-packages` hit exactly this while writing the patch -- matched as external by the `node-gyp-build` prefix, missed by a glob without a trailing wildcard, and invisible on the platform being tested on. Verified the way this can actually fail: extracted app.asar.unpacked into a directory with no node_modules ancestor -- what plain node sees under WSL -- and booted the server there. Migrations ran, it listened on 127.0.0.1, and no module failed to resolve. node-pty, ffi-rs, msgpackr-extract and @ff-labs/fff-node all load from that isolated tree. --- apps/server/vite.config.ts | 19 +++---- scripts/build-desktop-artifact.ts | 23 +++++--- scripts/lib/cli-external-packages.test.ts | 64 +++++++++++++++++++++++ scripts/lib/cli-external-packages.ts | 56 ++++++++++++++++++++ 4 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 scripts/lib/cli-external-packages.test.ts create mode 100644 scripts/lib/cli-external-packages.ts diff --git a/apps/server/vite.config.ts b/apps/server/vite.config.ts index 521654f3279..84b38f26658 100644 --- a/apps/server/vite.config.ts +++ b/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"; diff --git a/scripts/build-desktop-artifact.ts b/scripts/build-desktop-artifact.ts index a30b6d4a90a..12b19a5b886 100644 --- a/scripts/build-desktop-artifact.ts +++ b/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", diff --git a/scripts/lib/cli-external-packages.test.ts b/scripts/lib/cli-external-packages.test.ts new file mode 100644 index 00000000000..67365a60b8d --- /dev/null +++ b/scripts/lib/cli-external-packages.test.ts @@ -0,0 +1,64 @@ +import { assert, describe, it } from "@effect/vitest"; + +import { + CLI_EXTERNAL_PACKAGE_PREFIXES, + CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS, + shouldBundleCliDependency, +} from "./cli-external-packages.ts"; + +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*/**/*"); + }); +}); diff --git a/scripts/lib/cli-external-packages.ts b/scripts/lib/cli-external-packages.ts new file mode 100644 index 00000000000..ae41ecf3b5f --- /dev/null +++ b/scripts/lib/cli-external-packages.ts @@ -0,0 +1,56 @@ +/** + * 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. + */ +export const CLI_EXTERNAL_PACKAGE_PREFIXES = [ + // Native addons (.node), and the JS wrappers that dlopen them by real path. + "node-pty", + "ffi-rs", + "@yuuang/", + "@ff-labs/", + "@clerk/electron-passkeys", + "@msgpackr-extract/", + "msgpackr-extract", + "node-gyp-build", + "node-addon-api", + // Bun-only entry points: reached through a runtime-conditional dynamic import + // and resolving `bun:*` specifiers, which do not exist when bundling for Node. + "@effect/platform-bun", + "@effect/sql-sqlite-bun", +] 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, +); From 0aacacd8196977df4f7b63a57ca3ed6db92ffc7a Mon Sep 17 00:00:00 2001 From: tsouth89 Date: Sun, 9 Aug 2026 17:38:30 -0400 Subject: [PATCH 2/4] fix(build): keep the dependency closure of external packages external Real WSL testing on this branch found a case the hand-maintained list could not catch by inspection. node-gyp-build-optional-packages is external, so it is loaded from the real filesystem, so its own `require` resolves from the real filesystem too. It requires detect-libc, which was not on the list and therefore got bundled into the CLI bundle -- present only inside app.asar. The Windows primary reads that transparently under ELECTRON_RUN_AS_NODE and resolves it; plain node under WSL cannot. msgpackr-extract failed through the same chain. Measured under Ubuntu 24.04 with Linux node v24.18.0 against the packaged tree: before: MISSING (cjs) msgpackr-extract [MODULE_NOT_FOUND] detect-libc MISSING (cjs) node-gyp-build-optional-packages [MODULE_NOT_FOUND] after : no resolution failures The general rule is that an external package's entire runtime dependency closure must be external. That is not something to maintain by staring at a list, so it is now a test: it walks each runtime-external package's declared dependencies transitively and fails if any would be bundled away. Writing that test surfaced a distinction the single list had flattened. The Bun-only entries are external for a build-time reason -- they resolve `bun:*` specifiers that do not exist when bundling for Node -- and Node never loads them, so their closure genuinely does not need to be external. The native packages are external for a runtime reason and theirs does. The list is split along that line, and the closure test applies only to the runtime set. Adds 6 files to the installer (1,192 -> 1,198). Native binaries and installer size are unchanged. --- scripts/lib/cli-external-packages.test.ts | 55 +++++++++++++++++++++++ scripts/lib/cli-external-packages.ts | 34 ++++++++++++-- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/scripts/lib/cli-external-packages.test.ts b/scripts/lib/cli-external-packages.test.ts index 67365a60b8d..23e06b49f0d 100644 --- a/scripts/lib/cli-external-packages.test.ts +++ b/scripts/lib/cli-external-packages.test.ts @@ -3,6 +3,7 @@ import { assert, describe, it } from "@effect/vitest"; import { CLI_EXTERNAL_PACKAGE_PREFIXES, CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS, + CLI_RUNTIME_EXTERNAL_PREFIXES, shouldBundleCliDependency, } from "./cli-external-packages.ts"; @@ -62,3 +63,57 @@ describe("CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS", () => { 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. +describe("external package dependency closure", () => { + // Must be runtime-external specifically: the dependency has to exist on disk. + const isExternal = (name: string) => + CLI_RUNTIME_EXTERNAL_PREFIXES.some((prefix) => name.startsWith(prefix)); + + it("keeps every runtime dependency of an external package external too", async () => { + const { createRequire } = await import("node:module"); + const require = createRequire(import.meta.url); + + const violations: string[] = []; + const seen = new Set(); + // Only the runtime-external set. The build-only entries resolve `bun:*` + // and are never loaded by Node, so their closure is irrelevant here. + const queue: string[] = CLI_RUNTIME_EXTERNAL_PREFIXES.filter((prefix) => !prefix.endsWith("/")); + + for (const name of queue) { + if (seen.has(name)) continue; + seen.add(name); + + let manifest: { dependencies?: Record }; + try { + manifest = require(`${name}/package.json`); + } catch { + // Not installed on this platform (or reached only by prefix); nothing + // to check. The globs still cover it if it does get installed. + continue; + } + + for (const dependency of Object.keys(manifest.dependencies ?? {})) { + if (!isExternal(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(", ")}`, + ); + }); +}); diff --git a/scripts/lib/cli-external-packages.ts b/scripts/lib/cli-external-packages.ts index ae41ecf3b5f..1477b382f68 100644 --- a/scripts/lib/cli-external-packages.ts +++ b/scripts/lib/cli-external-packages.ts @@ -17,8 +17,17 @@ * a package's platform-specific siblings — `node-gyp-build` covers * `node-gyp-build-optional-packages`, `@yuuang/` covers every `ffi-rs-*` binding. */ -export const CLI_EXTERNAL_PACKAGE_PREFIXES = [ - // Native addons (.node), and the JS wrappers that dlopen them by real path. +/** + * 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/", @@ -28,12 +37,29 @@ export const CLI_EXTERNAL_PACKAGE_PREFIXES = [ "msgpackr-extract", "node-gyp-build", "node-addon-api", - // Bun-only entry points: reached through a runtime-conditional dynamic import - // and resolving `bun:*` specifiers, which do not exist when bundling for Node. + // 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; From 45b45176a5198897ba05f4330f1d28f2daffbf8e Mon Sep 17 00:00:00 2001 From: tsouth89 Date: Sun, 9 Aug 2026 18:41:35 -0400 Subject: [PATCH 3/4] test(build): make the closure guard actually read the dependency closure The guard added in the previous commit could pass without checking anything. It resolved manifests with `require("/package.json")` from scripts/lib, and swallowed resolution failures as "not installed on this platform". Under pnpm isolation that catch swallowed nearly everything. Probed from scripts/lib, every seed failed with MODULE_NOT_FOUND -- node-pty, msgpackr-extract, ffi-rs, node-gyp-build, detect-libc, node-addon-api. Probed from apps/server, only its direct dependencies resolved; the transitive packages that actually caused the WSL breakage still did not. `exports` maps are a second hole: @ff-labs/fff-node refuses the /package.json subpath with ERR_PACKAGE_PATH_NOT_EXPORTED, which the same catch treated as absent. Seeding the queue from the prefix strings was wrong for a second reason: the filter dropped every prefix ending in "/", so "@yuuang/", "@ff-labs/" and "@msgpackr-extract/" were never visited even where resolution worked. Read the manifests off disk from the pnpm store instead. That is the same tree asarUnpack globs target, it reaches transitive packages, and it is not subject to resolution or exports semantics. Seeds now come from what is installed and matches a prefix, so scoped prefixes are covered. Added a guard test that fails unless node-pty, node-gyp-build-optional-packages and detect-libc are actually found, because a closure check that reads nothing is worse than no check -- it reports success. Verified by mutation: removing detect-libc from the list fails with "node-gyp-build-optional-packages -> detect-libc", the real bug. The previous version of this test passed with detect-libc removed. --- scripts/lib/cli-external-packages.test.ts | 140 ++++++++++++++++------ 1 file changed, 105 insertions(+), 35 deletions(-) diff --git a/scripts/lib/cli-external-packages.test.ts b/scripts/lib/cli-external-packages.test.ts index 23e06b49f0d..cb684a7b10a 100644 --- a/scripts/lib/cli-external-packages.test.ts +++ b/scripts/lib/cli-external-packages.test.ts @@ -1,4 +1,11 @@ +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, @@ -7,6 +14,14 @@ import { 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"]) { @@ -74,46 +89,101 @@ describe("CLI_EXTERNAL_PACKAGE_UNPACK_GLOBS", () => { // // Found the hard way: node-gyp-build-optional-packages requires detect-libc, // which was bundled. Windows was fine; WSL got MODULE_NOT_FOUND. -describe("external package dependency closure", () => { - // Must be runtime-external specifically: the dependency has to exist on disk. - const isExternal = (name: string) => +it.layer(NodeServices.layer)("external package dependency closure", (it) => { + // Read manifests off disk from the pnpm store rather than resolving them. + // `require("/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", + ); + + const installed = new Map(); + if (!(yield* fileSystem.exists(storeDir))) return installed; + + for (const entry of yield* fileSystem.readDirectory(storeDir)) { + const modulesDir = path.join(storeDir, entry, "node_modules"); + if (!(yield* fileSystem.exists(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* fileSystem.exists(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("keeps every runtime dependency of an external package external too", async () => { - const { createRequire } = await import("node:module"); - const require = createRequire(import.meta.url); - - const violations: string[] = []; - const seen = new Set(); - // Only the runtime-external set. The build-only entries resolve `bun:*` - // and are never loaded by Node, so their closure is irrelevant here. - const queue: string[] = CLI_RUNTIME_EXTERNAL_PREFIXES.filter((prefix) => !prefix.endsWith("/")); - - for (const name of queue) { - if (seen.has(name)) continue; - seen.add(name); - - let manifest: { dependencies?: Record }; - try { - manifest = require(`${name}/package.json`); - } catch { - // Not installed on this platform (or reached only by prefix); nothing - // to check. The globs still cover it if it does get installed. - continue; + 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(); + // 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); - for (const dependency of Object.keys(manifest.dependencies ?? {})) { - if (!isExternal(dependency)) { - violations.push(`${name} -> ${dependency}`); + 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); } - 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(", ")}`, - ); - }); + assert.deepStrictEqual( + violations, + [], + `these dependencies of external packages would be bundled away and fail to resolve under WSL: ${violations.join(", ")}`, + ); + }), + ); }); From 12a5bf026f2fd5efde257aec14e9b1e9783a423c Mon Sep 17 00:00:00 2001 From: tsouth89 Date: Sun, 9 Aug 2026 19:18:16 -0400 Subject: [PATCH 4/4] fix(test): tolerate non-directory entries in the pnpm store The closure guard walked node_modules/.pnpm and built a node_modules path under each entry. The store also contains a regular file, lock.yaml, so that path is rooted in a file rather than a directory. Linux raises ENOTDIR from the access call; Windows quietly reports false. The test therefore passed locally and failed on CI -- itself an instance of the platform asymmetry this file exists to catch. Existence checks now treat any failure as absence. --- scripts/lib/cli-external-packages.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/scripts/lib/cli-external-packages.test.ts b/scripts/lib/cli-external-packages.test.ts index cb684a7b10a..3504540e956 100644 --- a/scripts/lib/cli-external-packages.test.ts +++ b/scripts/lib/cli-external-packages.test.ts @@ -106,12 +106,19 @@ it.layer(NodeServices.layer)("external package dependency closure", (it) => { "../../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(); - if (!(yield* fileSystem.exists(storeDir))) return installed; + if (!(yield* isPresent(storeDir))) return installed; for (const entry of yield* fileSystem.readDirectory(storeDir)) { const modulesDir = path.join(storeDir, entry, "node_modules"); - if (!(yield* fileSystem.exists(modulesDir))) continue; + if (!(yield* isPresent(modulesDir))) continue; for (const owner of yield* fileSystem.readDirectory(modulesDir)) { const names = owner.startsWith("@") @@ -123,7 +130,7 @@ it.layer(NodeServices.layer)("external package dependency closure", (it) => { for (const name of names) { if (installed.has(name)) continue; const manifestPath = path.join(modulesDir, name, "package.json"); - if (!(yield* fileSystem.exists(manifestPath))) continue; + if (!(yield* isPresent(manifestPath))) continue; installed.set(name, decodeManifest(yield* fileSystem.readFileString(manifestPath))); } }