From 66330cc7241295e51e263e1f50eb197bb0e40709 Mon Sep 17 00:00:00 2001 From: anhtahaylove <37265396+anhtahaylove@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:27:38 +0700 Subject: [PATCH] fix(compression): pass a URL object when spawning the LLMLingua worker (#13093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `node:worker_threads` only treats a `URL` instance as a `file:` URL — a string must be a relative path. The silent `catch {}` in `pump()` made this degrade compression to a passthrough while still reporting success, which is the worst shape for it. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the other 13 PRs of this batch — zero merge conflicts between them. - `typecheck:core` clean - complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline - 71 focused assertions green across the 13 test files this batch adds or touches ⚠️ base-red inherited: #12732 — `Docs Gates (fast-path)`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098). None of them touch this diff. Thanks @anhtahaylove — the root-cause write-up, the measured before/after numbers and the red-before-green proof on every one of these made the batch reviewable as a unit. --- changelog.d/fixes/llmlingua-worker-spawn.md | 1 + .../compression/engines/llmlingua/worker.ts | 7 +- .../llmlingua-worker-spawn-12822.test.ts | 77 +++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/llmlingua-worker-spawn.md create mode 100644 tests/unit/compression/llmlingua-worker-spawn-12822.test.ts diff --git a/changelog.d/fixes/llmlingua-worker-spawn.md b/changelog.d/fixes/llmlingua-worker-spawn.md new file mode 100644 index 0000000000..865f19a32e --- /dev/null +++ b/changelog.d/fixes/llmlingua-worker-spawn.md @@ -0,0 +1 @@ +- fix(compression): spawn the LLMLingua worker with a file URL object so compression actually runs instead of silently failing open on Node diff --git a/open-sse/services/compression/engines/llmlingua/worker.ts b/open-sse/services/compression/engines/llmlingua/worker.ts index 00ba3e1255..0994e1a935 100644 --- a/open-sse/services/compression/engines/llmlingua/worker.ts +++ b/open-sse/services/compression/engines/llmlingua/worker.ts @@ -234,7 +234,12 @@ function ensureWorker(): Worker { const { workerFile, execArgv } = resolveWorkerFile(); const absoluteWorkerFile = path.resolve(workerFile); - const w = new Worker(pathToFileURL(absoluteWorkerFile).href, { execArgv }); + // Pass the URL OBJECT, not `.href`. `new Worker()` treats a plain string as a + // filesystem path, so a "file://..." string is looked up literally and throws + // ERR_WORKER_PATH (a string arg must start with ./ or ../). Only a URL instance + // is interpreted as a file: URL. Spawn failures are swallowed by pump()'s catch, + // so getting this wrong silently disables compression instead of erroring. + const w = new Worker(pathToFileURL(absoluteWorkerFile), { execArgv }); w.on("message", (reply: WorkerReply) => { const entry = pending.get(reply.id); diff --git a/tests/unit/compression/llmlingua-worker-spawn-12822.test.ts b/tests/unit/compression/llmlingua-worker-spawn-12822.test.ts new file mode 100644 index 0000000000..360a1f2037 --- /dev/null +++ b/tests/unit/compression/llmlingua-worker-spawn-12822.test.ts @@ -0,0 +1,77 @@ +/** + * Regression guard for #12822: the LLMLingua worker must actually spawn on Node. + * + * Root cause: `new Worker(pathToFileURL(file).href, ...)` passes a STRING. Node treats a + * string argument as a filesystem path (it must start with ./ or ../), so a "file://..." + * string is looked up literally and throws ERR_WORKER_PATH. Only a URL INSTANCE is + * interpreted as a file: URL. + * + * Why it was invisible: pump() wraps ensureWorker() in `catch {}` and fails open, so the + * spawn crash silently degraded every compression call to a passthrough instead of erroring. + * + * This test asserts the Node contract directly against a real Worker, so it fails on the + * old `.href` spelling and passes on the URL object. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { Worker } from "node:worker_threads"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const WORKER_SRC = path.resolve( + here, + "../../../open-sse/services/compression/engines/llmlingua/worker.ts" +); + +function spawnWith(arg: string | URL): Promise { + return new Promise((resolve, reject) => { + let w: Worker; + try { + w = new Worker(arg, {}); + } catch (err) { + reject(err); + return; + } + w.on("error", reject); + w.on("exit", () => resolve()); + }); +} + +test("a file: URL STRING is rejected by node:worker_threads (the #12822 crash)", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-worker-")); + const child = path.join(dir, "child.mjs"); + fs.writeFileSync(child, "process.exit(0);\n"); + + await assert.rejects( + () => spawnWith(pathToFileURL(child).href), + (err: NodeJS.ErrnoException) => err.code === "ERR_WORKER_PATH", + "passing .href must fail — this is exactly what shipped and was swallowed by the fail-open catch" + ); + + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test("a file: URL OBJECT spawns cleanly", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-worker-")); + const child = path.join(dir, "child.mjs"); + fs.writeFileSync(child, "process.exit(0);\n"); + + await spawnWith(pathToFileURL(child)); + + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test("worker.ts passes the URL object, not .href", () => { + const code = fs.readFileSync(WORKER_SRC, "utf8"); + assert.ok( + /new Worker\(\s*pathToFileURL\([A-Za-z0-9_]+\)\s*,/.test(code), + "ensureWorker must pass the URL instance to new Worker()" + ); + assert.ok( + !/new Worker\(\s*pathToFileURL\([A-Za-z0-9_]+\)\.href/.test(code), + "ensureWorker must not pass pathToFileURL(...).href — that throws ERR_WORKER_PATH" + ); +});