Skip to content

Commit 7e01d33

Browse files
tsouth89t3-code[bot]UtkarshUsernameshivamhwp
authored
perf(build): stop unpacking node_modules wholesale from the Windows asar (#5877)
Co-authored-by: tsouth89 <tsouth89@users.noreply.github.com> Co-authored-by: t3-code[bot] <t3-code[bot]@users.noreply.github.com> Co-authored-by: Utkarsh Patil <73941998+UtkarshUsername@users.noreply.github.com> Co-authored-by: Shivam Sharma <91240327+shivamhwp@users.noreply.github.com>
1 parent 9fd788b commit 7e01d33

6 files changed

Lines changed: 974 additions & 31 deletions

File tree

apps/desktop/src/wsl/DesktopWslEnvironment.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -229,15 +229,18 @@ const NODE_PTY_PROBE_SCRIPT = (
229229
printf 'nodeVersion:%s\\n' "$(node -p 'process.versions.node' 2>/dev/null)"
230230
printf 'resolvedPath:%s\\n' "$PATH"
231231
cd ${shellQuote(linuxServerDir)} && node <<'NODE' >/dev/null 2>&1
232-
// The server bundle externalizes its deps to node_modules, and the WSL Node
233-
// can't read inside app.asar, so confirm those deps are unpacked on the real
234-
// filesystem before reporting the backend healthy. "effect" is the framework
235-
// every server module imports; resolving it validates the whole node_modules
236-
// tree. Exit 3 marks this distinct from a node-pty problem so the caller can
237-
// report it accurately instead of letting the server crash on
238-
// ERR_MODULE_NOT_FOUND at launch (which, in wsl-only mode, would just fail to
239-
// launch with no fallback).
240-
try { require.resolve("effect"); } catch (_e) { process.exit(3); }
232+
// The WSL Node can't read inside app.asar, so confirm what the server needs is
233+
// unpacked on the real filesystem before reporting the backend healthy. Exit 3
234+
// marks this distinct from a node-pty prebuild problem so the caller can report
235+
// it accurately instead of letting the server crash on ERR_MODULE_NOT_FOUND at
236+
// launch (which, in wsl-only mode, would just fail to launch with no fallback).
237+
//
238+
// The sentinel must be a package the CLI bundle leaves external. It used to be
239+
// "effect", back when the bundle externalized its runtime deps and the whole
240+
// node_modules tree was unpacked. The bundle now inlines its JS dependencies,
241+
// so "effect" no longer exists on disk and only the native packages do —
242+
// resolving node-pty is what actually validates the unpacked tree.
243+
try { require.resolve("node-pty/package.json"); } catch (_e) { process.exit(3); }
241244
const fs = require("node:fs");
242245
const path = require("node:path");
243246
const pkgDir = path.dirname(require.resolve("node-pty/package.json"));
@@ -462,16 +465,17 @@ const ensureNodePtyImpl = (
462465
} as const;
463466
}
464467

465-
// Server dependencies (e.g. "effect") couldn't be resolved on the WSL
466-
// filesystem — a packaging regression, since the server bundle needs its
467-
// node_modules unpacked from the asar. Fatal so wsl-only mode falls back to
468-
// Windows and dual mode surfaces the reason inline, instead of the server
469-
// crash-looping on ERR_MODULE_NOT_FOUND once it actually launches.
468+
// The packages the server bundle leaves external (node-pty and the other
469+
// native addons) couldn't be resolved on the WSL filesystem — a packaging
470+
// regression, since those must be unpacked from the asar. Fatal so wsl-only
471+
// mode falls back to Windows and dual mode surfaces the reason inline,
472+
// instead of the server crash-looping on ERR_MODULE_NOT_FOUND once it
473+
// actually launches.
470474
if (probe.exitCode === 3) {
471475
return {
472476
ok: false,
473477
reason:
474-
"WSL server dependencies could not be loaded (for example \"effect\"). The server's bundled node_modules is not readable by the WSL distro's Node — this is a packaging problem with this build. Please report it.",
478+
'WSL server dependencies could not be loaded (for example "node-pty"). The native packages the server needs are not unpacked where the WSL distro\'s Node can read them — this is a packaging problem with this build. Please report it.',
475479
fatal: true,
476480
} as const;
477481
}

apps/server/vite.config.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,20 @@ import baseConfig from "../../vite.config.ts";
55
import { loadRepoEnv } from "../../scripts/lib/public-config.ts";
66
import packageJson from "./package.json" with { type: "json" };
77

8-
const bundledPackagePrefixes = [
9-
"@pierre/diffs",
10-
"@t3tools/",
11-
"effect-acp",
12-
"effect-codex-app-server",
13-
];
8+
// The bundle used to inline only workspace packages, leaving every third-party
9+
// runtime dep external. External deps must exist on the real filesystem (the WSL
10+
// backend runs plain `wsl.exe -- node`, which cannot read inside an asar), so the
11+
// desktop build unpacked `**\/node_modules\/**` wholesale: 13,875 loose files to
12+
// support 20 native binaries. NSIS install time tracks file count, not bytes.
13+
//
14+
// Inverted here — bundle everything except the packages that genuinely cannot be
15+
// inlined. See scripts/lib/cli-external-packages.ts for what earns an exemption.
16+
import {
17+
isExternalCliDependency,
18+
shouldBundleCliDependency,
19+
} from "../../scripts/lib/cli-external-packages.ts";
1420

15-
export function shouldBundleCliDependency(id: string): boolean {
16-
return bundledPackagePrefixes.some((prefix) => id.startsWith(prefix));
17-
}
21+
export { shouldBundleCliDependency };
1822

1923
const repoEnv = loadRepoEnv();
2024
const cliBuildChannel = packageJson.version.includes("-nightly.") ? "nightly" : "latest";
@@ -37,7 +41,14 @@ export default mergeConfig(
3741
sourcemap: true,
3842
clean: true,
3943
deps: {
44+
// Both halves are required. `alwaysBundle` forces the JS dependencies in
45+
// (declared deps are external by default, which is what this change is
46+
// undoing). `neverBundle` forces the native packages out: returning
47+
// false from `alwaysBundle` only means "no opinion", so a transitive
48+
// dependency would still be bundled — which silently inlined
49+
// msgpackr-extract and its loader, losing native acceleration.
4050
alwaysBundle: shouldBundleCliDependency,
51+
neverBundle: (id: string) => isExternalCliDependency(id),
4152
onlyBundle: false,
4253
},
4354
banner: {

scripts/build-desktop-artifact.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import * as NodeServices from "@effect/platform-node/NodeServices";
22
import { assert, it } from "@effect/vitest";
33
import * as ConfigProvider from "effect/ConfigProvider";
4+
import * as FileSystem from "effect/FileSystem";
5+
import * as Path from "effect/Path";
46
import * as Effect from "effect/Effect";
57
import * as Layer from "effect/Layer";
68
import * as Option from "effect/Option";
@@ -43,6 +45,8 @@ import {
4345
stageLinuxIconSize,
4446
STAGE_INSTALL_ARGS,
4547
WINDOWS_ASAR_UNPACK,
48+
ancestorNodeModulesPaths,
49+
copyDirectoryPreservingSymlinks,
4650
} from "./build-desktop-artifact.ts";
4751
import { BRAND_ASSET_PATHS } from "./lib/brand-assets.ts";
4852
import { HostProcessArchitecture, HostProcessPlatform } from "@t3tools/shared/hostProcess";
@@ -767,3 +771,85 @@ it.layer(NodeServices.layer)("build-desktop-artifact", (it) => {
767771
}),
768772
);
769773
});
774+
775+
// The self-containment check runs the packaged tree in a scratch directory. Its
776+
// own node_modules holds the unpacked externals and must be ignored, but any
777+
// node_modules *above* it would let Node's parent walk satisfy an import that is
778+
// missing from the package, so the probe refuses to run in that case.
779+
it("lists ancestor node_modules, nearest first, excluding the start directory", () => {
780+
assert.deepStrictEqual(ancestorNodeModulesPaths("C:\\tmp\\probe\\app", "\\"), [
781+
"C:\\tmp\\probe\\node_modules",
782+
"C:\\tmp\\node_modules",
783+
"C:\\node_modules",
784+
]);
785+
});
786+
787+
it("includes the filesystem root for posix paths", () => {
788+
assert.deepStrictEqual(ancestorNodeModulesPaths("/tmp/probe", "/"), [
789+
"/tmp/node_modules",
790+
"/node_modules",
791+
]);
792+
});
793+
794+
// A UNC root must keep its \\server\share prefix. Rebuilding it from segments
795+
// produced relative paths, which fs.exists resolves against the build cwd, so
796+
// the guard checked directories that do not exist and silently passed.
797+
it("keeps the prefix of a UNC path instead of going relative", () => {
798+
const paths = ancestorNodeModulesPaths("\\\\server\\share\\tmp\\app", "\\");
799+
for (const candidate of paths) {
800+
assert.ok(candidate.startsWith("\\\\server\\share"), candidate);
801+
}
802+
assert.deepStrictEqual(paths[0], "\\\\server\\share\\tmp\\node_modules");
803+
});
804+
805+
it.effect("rebases packaged links into the isolated tree", () =>
806+
Effect.gen(function* () {
807+
const fs = yield* FileSystem.FileSystem;
808+
const path = yield* Path.Path;
809+
const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-copy-symlinks-" });
810+
const source = path.join(root, "source");
811+
const destination = path.join(root, "destination");
812+
const packageDir = path.join(source, "node_modules/.pnpm/example@1/node_modules/example");
813+
const relativePackageLink = path.join(source, "node_modules/example-relative");
814+
const absolutePackageLink = path.join(source, "node_modules/example-absolute");
815+
816+
yield* fs.makeDirectory(packageDir, { recursive: true });
817+
yield* fs.writeFileString(path.join(packageDir, "index.js"), "module.exports = true;\n");
818+
yield* fs.symlink(
819+
path.join(".pnpm", "example@1", "node_modules", "example"),
820+
relativePackageLink,
821+
);
822+
yield* fs.symlink(packageDir, absolutePackageLink);
823+
824+
yield* copyDirectoryPreservingSymlinks(source, destination);
825+
826+
const copiedPackage = path.join(
827+
destination,
828+
"node_modules/.pnpm/example@1/node_modules/example",
829+
);
830+
const resolvedCopiedPackage = yield* fs.realPath(copiedPackage);
831+
assert.equal(
832+
yield* fs.readLink(path.join(destination, "node_modules/example-relative")),
833+
copiedPackage,
834+
);
835+
assert.equal(
836+
yield* fs.readLink(path.join(destination, "node_modules/example-absolute")),
837+
copiedPackage,
838+
);
839+
assert.equal(
840+
yield* fs.realPath(path.join(destination, "node_modules/example-relative")),
841+
resolvedCopiedPackage,
842+
);
843+
assert.equal(
844+
yield* fs.realPath(path.join(destination, "node_modules/example-absolute")),
845+
resolvedCopiedPackage,
846+
);
847+
}).pipe(Effect.provide(NodeServices.layer)),
848+
);
849+
850+
it("ignores trailing separators", () => {
851+
assert.deepStrictEqual(
852+
ancestorNodeModulesPaths("C:\\tmp\\probe\\app\\", "\\"),
853+
ancestorNodeModulesPaths("C:\\tmp\\probe\\app", "\\"),
854+
);
855+
});

0 commit comments

Comments
 (0)