From 359aba59c7b362a5efaa4f0cd4d482d48ee2df66 Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Fri, 7 Aug 2026 02:33:28 +0200 Subject: [PATCH] fix(build): colocateLlmlinguaOptionals skip-check treated a Next-traced stub as fully copied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- scripts/build/colocateOptionals.mjs | 48 ++++++++++++++++++++++----- tests/unit/colocate-optionals.test.ts | 35 +++++++++++++++++++ 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/scripts/build/colocateOptionals.mjs b/scripts/build/colocateOptionals.mjs index 91548954b0..7ea96a8530 100644 --- a/scripts/build/colocateOptionals.mjs +++ b/scripts/build/colocateOptionals.mjs @@ -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}`); } } diff --git a/tests/unit/colocate-optionals.test.ts b/tests/unit/colocate-optionals.test.ts index b9832c580b..a2d54dca99 100644 --- a/tests/unit/colocate-optionals.test.ts +++ b/tests/unit/colocate-optionals.test.ts @@ -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"]);