fix(build): colocateLlmlinguaOptionals skip-check treated a Next-traced stub as fully copied

Debugging the omniroute-beta Docker rebuild: `npm run build` (and the
Dockerfile's own post-build verification) failed with
`Cannot find module '.../node_modules/@atjsh/llmlingua-2/dist/index.js'`.

Root cause, reproduced directly (both against a live Docker builder image
and in a unit test): Next.js's own standalone trace creates a stub
directory for `@atjsh/llmlingua-2` containing only `package.json` — it
references the package (a dynamically-imported optional dependency) but
can't fully bundle it. colocateLlmlinguaOptionals's skip checks (both the
closure-level early return and the per-package loop) only tested
`existsSync(dest)`, so that stub was indistinguishable from "already fully
co-located" — the function skipped copying the real `dist/` output
entirely, silently shipping a package with a manifest but no code.

Fix: check for the package's declared `main` entry file when it has one
(the real-world case for every actual SLM optional). Packages with no
`main` field fall back to comparing the destination's top-level entries
against the source's — correct both for genuinely multi-file packages and
for a metadata-only source (package.json is then its complete, faithfully-
copied contents), which the existing idempotency test exercises.

Covered by tests/unit/colocate-optionals.test.ts's new stub-reproduction
case (fails against the pre-fix code, passes after — confirmed directly)
plus the 6 pre-existing cases, all still green.
This commit is contained in:
Markus Hartung
2026-08-07 02:33:28 +02:00
parent 0bac1c8499
commit 359aba59c7
2 changed files with 74 additions and 9 deletions

View File

@@ -46,9 +46,43 @@
* fail-open, so this never throws into the install.
*/
import { cpSync, existsSync, mkdirSync, readFileSync } from "node:fs";
import { cpSync, existsSync, mkdirSync, readdirSync, readFileSync } from "node:fs";
import { dirname, join } from "node:path";
/**
* A package directory existing is not proof it was fully copied — Next.js's own
* standalone trace can create a stub directory containing only `package.json`
* for a package it references but doesn't fully bundle (e.g. a dynamically
* imported optional dependency it can't statically resolve). Both the
* closure-level and per-package skip checks below used to test bare directory
* existence, so that Next-created stub made colocateLlmlinguaOptionals believe
* the package was "already co-located" and skip copying its real `dist/`
* output entirely — silently shipping a package with a manifest but no code
* (breaks `require.resolve` at runtime).
*
* Check for the package's declared `main` entry file when it has one (the
* common case for real npm packages, including this closure's actual seeds).
* A package with no `main` field has no single file to check, so compare
* against the SOURCE package's own top-level entries instead: the copy is
* complete once every entry the source has is also present at dest — correct
* both for real multi-file packages and for a metadata-only source package
* (package.json is then the entire, faithfully-copied contents).
*/
function isPackageFullyCopied(srcDir, destDir) {
if (!existsSync(destDir)) return false;
let manifest;
try {
manifest = JSON.parse(readFileSync(join(destDir, "package.json"), "utf8"));
} catch {
return false; // no readable manifest — treat as not present
}
if (typeof manifest.main === "string" && manifest.main.trim()) {
return existsSync(join(destDir, manifest.main));
}
if (!existsSync(srcDir)) return true; // nothing to compare against — trust dest as-is
return readdirSync(srcDir).every((entry) => existsSync(join(destDir, entry)));
}
/**
* Entry packages of the SLM optional stack (the closure roots). `@huggingface/transformers` is
* deliberately absent — it is the pinned instance already present in `dist/node_modules`.
@@ -129,9 +163,7 @@ export function colocateLlmlinguaOptionals({
if (!existsSync(targetNm)) {
return {
skipped: true,
reason: targetNodeModulesDir
? "no target node_modules"
: "no standalone dist/node_modules",
reason: targetNodeModulesDir ? "no target node_modules" : "no standalone dist/node_modules",
};
}
@@ -146,7 +178,7 @@ export function colocateLlmlinguaOptionals({
// populated bundle must still receive any missing transitive dependencies.
if (
closure.length > 0 &&
closure.every((name) => existsSync(join(targetNm, name)))
closure.every((name) => isPackageFullyCopied(join(rootNm, name), join(targetNm, name)))
) {
return { skipped: true, reason: "already co-located" };
}
@@ -155,16 +187,14 @@ export function colocateLlmlinguaOptionals({
for (const name of closure) {
const dest = join(targetNm, name);
if (existsSync(dest)) continue;
if (isPackageFullyCopied(join(rootNm, name), dest)) continue;
try {
mkdirSync(dirname(dest), { recursive: true });
cpSync(join(rootNm, name), dest, { recursive: true });
copied++;
} catch (err) {
log(
` ⚠️ LLMLingua optional co-location failed for ${name}: ${err.message}`
);
log(` ⚠️ LLMLingua optional co-location failed for ${name}: ${err.message}`);
}
}

View File

@@ -175,6 +175,41 @@ test("colocateLlmlinguaOptionals skips when there is no standalone dist bundle",
}
});
test("colocateLlmlinguaOptionals fills a Next-traced stub (package.json only, no dist) instead of skipping it", () => {
// Reproduces a real build failure: Next.js's own standalone trace can create
// a stub directory for a dynamically-imported optional dependency it
// references but can't fully bundle — just package.json, no actual code.
// The old skip check (`existsSync(dest)`) treated that stub as "already
// co-located" and never copied the real dist/ output, so
// require.resolve('@atjsh/llmlingua-2') found a package.json with no
// matching main file at runtime.
const root = mkdtempSync(join(tmpdir(), "omniroute-colocate-stub-"));
try {
buildRoot(root);
const distNm = join(root, "dist", "node_modules");
mkPkg(distNm, "@huggingface/transformers", { version: "3.5.2" });
// Simulate the Next-traced stub: directory exists, package.json only.
const stubDir = join(distNm, "@atjsh", "llmlingua-2");
mkdirSync(stubDir, { recursive: true });
writeFileSync(
join(stubDir, "package.json"),
readFileSync(join(root, "node_modules", "@atjsh", "llmlingua-2", "package.json"), "utf8")
);
assert.ok(!existsSync(join(stubDir, "dist", "index.js")), "stub must start without dist/");
const result = colocateLlmlinguaOptionals({ rootDir: root });
assert.equal(result.skipped, false, "must not treat the stub as already co-located");
assert.ok(
existsSync(join(stubDir, "dist", "index.js")),
"the real dist/index.js must be filled in, not left missing behind the stub"
);
} finally {
rmSync(root, { recursive: true, force: true });
}
});
test("SEED_PACKAGES excludes transformers (it is a dist-pinned peer, not a seed)", () => {
assert.ok(!SEED_PACKAGES.includes("@huggingface/transformers"));
assert.deepEqual(SEED_PACKAGES, ["@atjsh/llmlingua-2", "@tensorflow/tfjs", "js-tiktoken"]);