diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f013f9923..82ed6a04d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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://:*` 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 `` 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: ""` 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 `` 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:2099–2139`), 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:2143–2183`), 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)) diff --git a/package.json b/package.json index e6b2be3fb3..3ff935039c 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/scripts/build/pack-artifact-policy.ts b/scripts/build/pack-artifact-policy.ts index 12ed81d7a4..91701acc3e 100644 --- a/scripts/build/pack-artifact-policy.ts +++ b/scripts/build/pack-artifact-policy.ts @@ -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", ]; diff --git a/tests/unit/cli-runtime-imports-packaged-5227.test.ts b/tests/unit/cli-runtime-imports-packaged-5227.test.ts new file mode 100644 index 0000000000..26567c4080 --- /dev/null +++ b/tests/unit/cli-runtime-imports-packaged-5227.test.ts @@ -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(); + // 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(); + 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(", ")}` + ); +}); diff --git a/tests/unit/pack-artifact-policy.test.ts b/tests/unit/pack-artifact-policy.test.ts index 08b78d4a5b..cb37b2b66f 100644 --- a/tests/unit/pack-artifact-policy.test.ts +++ b/tests/unit/pack-artifact-policy.test.ts @@ -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", ]); });