Dashboard › Code › Session 0EXEoNU0U7NR
0EXEoNU0U7NRpSyL4You are reviewing a PR for the loreai project. The PR fixes a CI build failure where fossilize (a tool that creates standalone Node.js binaries) was receiving all platform names as a single comma-joined string instead of separate arguments.
The fix switches from CLI invocation to using fossilize's programmatic API directly.
Please review the changes critically. Look for:
Here is the diff:
File: packages/gateway/script/build-binary-sea.ts
The file adds these imports at the top (after line 46):
// Fossilize imports
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { fileURLToPath as nodeFileURLToPath } from "node:url";
// @ts-ignore - fossilize doesn't have proper typedefs but works at runtime
import fossilize from "../../../node_modules/.pnpm/fossilize@0.8.1/node_modules/fossilize/dist/impl-7RS2FY5C.js";
And replaces the CLI invocation (lines ~582-629) with:
// Use fossilize's programmatic API
const fossilizeContext = {
process,
os,
fs,
path,
};
const fossilizeFlags = {
nodeVersion: "lts",
platforms: targets.map(fossilizeTarget),
noBundle: true,
holePunch: true,
outputName: "lore",
outDir: distBinDir,
assetManifest: manifestPath,
// Don't sign here - we'll handle signing/renaming manually like before
sign: false,
concurrencyLimit: 3,
};
console.log(
`→ fossilize: ${targets.length} platform(s), ${Object.keys(manifest).length} asset(s)`,
);
await fossilize.default(fossilizeContext, fossilizeFlags, bundlePath);
Note: The old code used spawnSync (synchronous), the new code uses await (async). Check if the calling context supports async.
Also note: The import path ...
The intent is correct — fixing a bug where platform names were passed as a single comma-joined string instead of separate arguments. However, the implementation has several serious issues that would make this PR unacceptable as-is.
import fossilize from "../../../node_modules/.pnpm/fossilize@0.8.1/node_modules/fossilize/dist/impl-7RS2FY5C.js";
This is the single biggest problem. It has three fragility layers:
fossilize@0.8.1 — the version-specific directory. Any pnpm update fossilize or lockfile change breaks this.impl-7RS2FY5C.js — a content-hashed filename from fossilize's build. Any fossilize release changes this hash.../../../node_modules/.pnpm/ — hardcoded relative traversal into pnpm's internal store structure, which pnpm explicitly considers an implementation detail.This is especially ironic because the old code already solved this problem correctly:
const gatewayRequire = createRequire(`${packageDir}/`);
const fossilizePkgDir = dirname(gatewayRequire.resolve("fossilize/package.json"));
The fix should use the same dynamic resolution approach to find fossilize's programmatic entry point. For example:
const fossilizePkgDir = dirname(gatewayRequire.resolve("fossilize/package.json"));
const fossilize = await import(join(fossilizePkgDir, "dist", "index.js"));
// or read fossilize's package.json "exports"/"main" to find the entry
If the issue is that fossilize doesn't export its programmatic API cleanly, that's worth documenting — but the solution is to resolve the path dynamically, not to hardcode it.