fix(cli): ship scripts/build/runtime-env.mjs in npm package (#5227) (#5230)

serve.mjs imports scripts/build/runtime-env.mjs (added with the #5213 heap
auto-calibration fix) but it was missing from package.json files whitelist,
breaking every global npm install at startup. Add it to files and guard with
a regression test that asserts all bin/ runtime imports of scripts/ are
packaged.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-28 13:03:08 -03:00
committed by GitHub
parent 3ee53cd561
commit 6143ffc021
5 changed files with 93 additions and 1 deletions

View File

@@ -21,6 +21,7 @@ _In development — bullets added per PR; finalized at release._
### 🔧 Bug Fixes
- **fix(cli): global npm install no longer fails with "Cannot find module scripts/build/runtime-env.mjs"** — the v3.8.39 heap auto-calibration fix made `bin/cli/commands/serve.mjs` import `scripts/build/runtime-env.mjs`, but that file was never added to the `files` whitelist in `package.json`, so the published npm tarball shipped the importing CLI without the imported module — breaking **every** `npm install -g omniroute` at startup (regression, all platforms). The file is now whitelisted (`npm pack` confirms it ships) and added to the pack-artifact policy allow/required lists, and a new regression test scans all `bin/**` entrypoints for runtime imports resolving under `scripts/` and asserts each is covered by the package `files` whitelist, so any future unpackaged CLI import fails CI instead of users' installs. ([#5227](https://github.com/diegosouzapw/OmniRoute/issues/5227) — thanks @PriyomSaha, @m-Yaghoubi, @jonlwheat2-gif)
- **fix(oauth): Antigravity refresh no longer nulls the stored refresh_token on an empty upstream response** — Google's OAuth token endpoint uses non-rotating refresh tokens: a refresh response normally OMITS `refresh_token` and occasionally returns it as an empty string. The Antigravity executor's `refreshCredentials` used `typeof tokens.refresh_token === "string" ? tokens.refresh_token : credentials.refreshToken`, and because `typeof "" === "string"` is true, an empty-string response overwrote the good token with `""` — nulling it on first refresh. The check now treats a non-string **or empty** value as absent and preserves the stored token, matching the canonical `refreshGoogleToken` (`tokens.refresh_token || refreshToken`) semantics. ([#3850](https://github.com/diegosouzapw/OmniRoute/issues/3850) — thanks @3xa228148)
- **fix(api): LAN/Tailscale dashboard access — `ws:` CSP scheme, GET-exempt version route, surface combo field errors** — three failures when opening the dashboard from a non-loopback host: (1) CSP `connect-src` allowed the `ws:` scheme only for loopback origins, blocking the dashboard's `ws://<lan-host>:*` Live WebSocket from LAN/Tailscale clients; the bare `ws:` scheme is now permitted (symmetric with the bare `wss:` already allowed), kept declarative in `next.config.mjs` with no global middleware (the project has none by design); (2) `GET /api/system/version` was blocked by `LOCAL_ONLY_API_PREFIXES` for all methods despite only `POST` spawning child processes (git/npm/pm2) — a new `LOCAL_ONLY_API_GET_EXEMPTIONS` set exempts safe read methods for this path while keeping `POST`/`PUT`/`PATCH`/`DELETE` strictly loopback-only; (3) `COMBO_002` validation errors only surfaced the generic message — `firstField`/`firstMessage` are now extracted from the first Zod issue and included in the response body. ([#5083](https://github.com/diegosouzapw/OmniRoute/issues/5083) — thanks @KooshaPari for the diagnosis and original PR #5084)
- **fix(sse): defer `</think>` close so it never leaks before `tool_calls` in Claude→OpenAI streaming** — when a Claude thinking block was followed by a tool_use block, the translator unconditionally emitted a `content: "</think>"` chunk at `content_block_stop`, injecting a spurious assistant text chunk immediately before the `tool_calls` delta and corrupting OpenAI-compatible clients (e.g. Kimi Coding). The close marker is now deferred: it is flushed at the first `text_delta` that follows the thinking block (preserving the #4633 / decolua/9router#454 behavior for Claude Code / Cursor) or at stream finish when no tool_calls were collected. Tool-use streams never get a `text_delta` after the thinking block, so `</think>` is never emitted into content before `tool_calls`. ([#5123](https://github.com/diegosouzapw/OmniRoute/issues/5123))
@@ -51,7 +52,7 @@ _In development — bullets added per PR; finalized at release._
### 📝 Maintenance
- **test(docker): de-brittle the #4076 builder-stage heap-ordering test (false-failing on a comment)**`dockerfile-build-heap-4076.test.ts` located the `npm run build` step via a raw `findIndex(/npm run build\b/)`, which matched a **comment line** in the builder stage (`# … npm run build fails with "Module not…"`, added with the v3.8.40 workspace-deps Docker fix) sitting before the `NODE_OPTIONS` heap line — making the test report the heap ceiling as set *after* the build and fail, even though the real `RUN … npm run build` correctly follows the `ENV NODE_OPTIONS` line. Instruction matching now skips Dockerfile comment lines (`# …`), so the ordering guard checks real `ENV`/`RUN` instructions only. The Dockerfile itself was already correct; this was a base-red blocking every PR into `release/v3.8.40`. ([#4076](https://github.com/diegosouzapw/OmniRoute/issues/4076))
- **test(docker): de-brittle the #4076 builder-stage heap-ordering test (false-failing on a comment)**`dockerfile-build-heap-4076.test.ts` located the `npm run build` step via a raw `findIndex(/npm run build\b/)`, which matched a **comment line** in the builder stage (`# … npm run build fails with "Module not…"`, added with the v3.8.40 workspace-deps Docker fix) sitting before the `NODE_OPTIONS` heap line — making the test report the heap ceiling as set _after_ the build and fail, even though the real `RUN … npm run build` correctly follows the `ENV NODE_OPTIONS` line. Instruction matching now skips Dockerfile comment lines (`# …`), so the ordering guard checks real `ENV`/`RUN` instructions only. The Dockerfile itself was already correct; this was a base-red blocking every PR into `release/v3.8.40`. ([#4076](https://github.com/diegosouzapw/OmniRoute/issues/4076))
- **test(combo): deterministic context-relay universal-handoff coverage** — covers the universal (provider-agnostic) session-handoff path in `context-relay` (`combo.ts:20992139`), which previously had only a definition-order assertion and a `TODO(phase-2)`. The test drives the real pipeline via session seams (`x-session-id``relayOptions.sessionId``maybeGenerateUniversalHandoff`) without live infrastructure. ([#5168](https://github.com/diegosouzapw/OmniRoute/pull/5168))
- **test(combo): end-to-end quota-share DRR routing-decision coverage (matrix parity)** — adds the missing E2E test for the `quota-share` strategy, driving the real `handleChat` → chatCore → `selectQuotaShareTarget` → executor pipeline via in-process seams and asserting which connection is dispatched. The DRR selector already had 29 unit tests; this closes the E2E gap and brings quota-share to parity with the 17-strategy public matrix. ([#5179](https://github.com/diegosouzapw/OmniRoute/pull/5179))
- **test(combo): deterministic context-relay codex quota-handoff coverage (closes last gap)** — covers the codex-specific handoff block of `context-relay` (`combo.ts:21432183`), which #5168 left documented-but-untested because it requires a `codex` connection. All seams (`fetchCodexQuota`, handoff generation, session relay) are mocked deterministically without live infra. ([#5195](https://github.com/diegosouzapw/OmniRoute/pull/5195))

View File

@@ -25,6 +25,7 @@
"bin/cli/runtime/",
"scripts/postinstall.mjs",
"scripts/build/postinstallSupport.mjs",
"scripts/build/runtime-env.mjs",
"scripts/build/colocateOptionals.mjs",
"scripts/build/sync-env.mjs",
"scripts/dev/responses-ws-proxy.mjs",

View File

@@ -106,6 +106,8 @@ export const PACK_ARTIFACT_ROOT_ALLOWED_EXACT_PATHS: string[] = [
"scripts/build/postinstall.mjs",
"scripts/build/postinstallSupport.mjs",
"scripts/build/colocateOptionals.mjs",
// #5227: imported at runtime by bin/cli/commands/serve.mjs (heap auto-calibration).
"scripts/build/runtime-env.mjs",
"scripts/build/sync-env.mjs",
"scripts/dev/responses-ws-proxy.mjs",
"scripts/dev/sync-env.mjs",
@@ -148,6 +150,7 @@ export const PACK_ARTIFACT_REQUIRED_PATHS: string[] = [
"scripts/build/postinstall.mjs",
"scripts/build/postinstallSupport.mjs",
"scripts/build/colocateOptionals.mjs",
"scripts/build/runtime-env.mjs",
"src/shared/utils/nodeRuntimeSupport.ts",
];

View File

@@ -0,0 +1,86 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { readFileSync, readdirSync, statSync } from "node:fs";
import { dirname, join, resolve, relative } from "node:path";
import { fileURLToPath } from "node:url";
/**
* Regression guard for #5227.
*
* The published npm package only ships the paths listed in package.json's
* `files` whitelist. If a CLI entrypoint under `bin/` imports a module under
* `scripts/` at runtime, that script MUST be covered by the whitelist —
* otherwise a global install (`npm install -g omniroute`) fails at startup with
* "Cannot find module .../scripts/build/runtime-env.mjs".
*
* #5227: `bin/cli/commands/serve.mjs` started importing
* `scripts/build/runtime-env.mjs` (added with the heap auto-calibration fix
* #5213) but the file was never added to `files`, breaking every global install.
*/
const REPO_ROOT = resolve(fileURLToPath(import.meta.url), "../../../");
function listMjsFiles(dir: string): string[] {
const out: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
if (statSync(full).isDirectory()) {
out.push(...listMjsFiles(full));
} else if (entry.endsWith(".mjs") || entry.endsWith(".js")) {
out.push(full);
}
}
return out;
}
/** Extract every `scripts/...`-resolving specifier statically/dynamically imported by a bin file. */
function scriptImports(binFile: string): string[] {
const src = readFileSync(binFile, "utf8");
const specifiers = new Set<string>();
// static: from "..." dynamic: import("...")
const re = /(?:from|import)\s*\(?\s*["']([^"']+)["']/g;
let m: RegExpExecArray | null;
while ((m = re.exec(src))) {
const spec = m[1];
if (!spec.startsWith(".")) continue; // only relative imports can reach scripts/
const resolved = resolve(dirname(binFile), spec);
const rel = relative(REPO_ROOT, resolved).split("\\").join("/");
if (rel.startsWith("scripts/")) specifiers.add(rel);
}
return [...specifiers];
}
/** A repo-relative path is published if it equals a whitelist entry or sits under a directory entry. */
function isCovered(relPath: string, files: string[]): boolean {
return files.some((entry) => {
if (entry.startsWith("!")) return false; // negations never add coverage
if (entry === relPath) return true;
const dirEntry = entry.endsWith("/") ? entry : entry + "/";
return relPath.startsWith(dirEntry);
});
}
test("#5227 — every scripts/* module imported by bin/ is in the npm files whitelist", () => {
const pkg = JSON.parse(readFileSync(join(REPO_ROOT, "package.json"), "utf8"));
const files: string[] = pkg.files || [];
assert.ok(files.length > 0, "package.json must declare a files whitelist");
const binDir = join(REPO_ROOT, "bin");
const imported = new Set<string>();
for (const binFile of listMjsFiles(binDir)) {
for (const spec of scriptImports(binFile)) imported.add(spec);
}
// Sanity: the known runtime imports must be detected, else the scanner is broken.
assert.ok(
imported.has("scripts/build/runtime-env.mjs"),
"scanner should detect serve.mjs's import of scripts/build/runtime-env.mjs"
);
const missing = [...imported].filter((p) => !isCovered(p, files));
assert.deepEqual(
missing,
[],
`bin/ imports these scripts/* modules that are NOT in package.json "files" (global install will fail): ${missing.join(", ")}`
);
});

View File

@@ -101,6 +101,7 @@ test("findMissingArtifactPaths flags missing root runtime files in the tarball",
"dist/webdav-handler.mjs",
"scripts/build/colocateOptionals.mjs",
"scripts/build/native-binary-compat.mjs",
"scripts/build/runtime-env.mjs",
"src/shared/utils/nodeRuntimeSupport.ts",
]);
});