Skip to content

fix(cli): run Node runtime guard before heavy import chain (#12296) - #13253

Merged
diegosouzapw merged 1 commit into
release/v3.8.51from
fix/12296-regex-flags-crash
Sep 12, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.51from
fix/12296-regex-flags-crash

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Root cause

bin/omniroute.mjs imports getNodeRuntimeSupport/getNodeRuntimeWarning from
bin/nodeRuntimeSupport.mjs to detect an unsupported Node.js runtime, but never actually
calls either function — only bin/cli/commands/serve.mjs and bin/cli/commands/doctor.mjs
do, deep inside command handlers that are unreachable if the startup import chain itself crashes
first.

And it does crash first: right after generating STORAGE_ENCRYPTION_KEY, the script does
await import("tsx/esm") and then loads Commander's ~70 command modules (bin/cli/program.mjs),
which pull in ora -> the hoisted string-width@8.x. That package's index.js has top-level
regex literals using the ES2024 v (Unicode-set) flag, e.g. /^\p{RGI_Emoji}$/v. On a Node/V8
build that predates v-flag support, that literal fails to even parse, and V8 throws exactly
SyntaxError: Invalid regular expression flags — with no flag value in the message, which is why
it looked untraceable. Same class of failure hit pnpm (pnpm/pnpm#11546), root-caused there as
running a too-new dependency on an unsupported/old Node major.

Fix

Call the existing getNodeRuntimeSupport()/getNodeRuntimeWarning() check right after the
--version/-V fast path in bin/omniroute.mjs, before the heavy tsx/esm + command-
registration import chain. When the runtime is incompatible, print a clear, actionable message
(including a note that a stale/distro-packaged nodejs binary is the most common cause) and
process.exit(1). The check is skipped for the same read-only invocations
shouldProvisionStorageKey already exempts (--help/-h, help/completion) — those still
need the full command registry to render their own output, so an incompatible runtime crashing
there is a separate, pre-existing limitation this fix does not attempt to solve.

The existing calls in serve.mjs/doctor.mjs are left as-is (harmless duplicate checks).

Regression test

tests/unit/issue-12296-node-runtime-guard.test.ts — asserts bin/omniroute.mjs actually calls
getNodeRuntimeSupport/getNodeRuntimeWarning, and that the call happens before
await import("tsx/esm").

RED (before fix):

✖ bin/omniroute.mjs invokes the Node runtime compatibility guard before the heavy tsx/esm + command-registration import chain
AssertionError [ERR_ASSERTION]: getNodeRuntimeSupport()/getNodeRuntimeWarning() is imported in
bin/omniroute.mjs but never called there ...
  actual: -1, expected: -1 (notStrictEqual), tests 1, pass 0, fail 1

GREEN (after fix):

✔ bin/omniroute.mjs invokes the Node runtime compatibility guard before the heavy tsx/esm + command-registration import chain
tests 1, pass 1, fail 0

Gates run

  • node scripts/check/check-file-size.mjs — OK
  • node scripts/check/check-complexity.mjs — OK (2798 violations vs baseline 3218)
  • node scripts/check/check-cognitive-complexity.mjs — OK (1265 violations vs baseline 1437)
  • npm run typecheck:core — clean (exit 0)
  • npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files> — clean (bin/** is excluded from ESLint's scope by project config; the new test file lints clean)
  • New regression test — PASS (RED→GREEN evidence above)
  • Existing CLI-entrypoint-touching tests re-run individually (not just the new test):
    cli-entrypoint, cli-version-fastpath, cli-storage-key-provision,
    cli-storage-key-bootstrap, bin-omniroute-mcp, cli-data-dir-env,
    cli-data-dir-env-loading, cli-env-collision, cli-update-notifier,
    cli-update-global-paths-3295, reset-password-cli-6261-6258,
    cli-electron-to-cli-migration-server-env-7302, termux-android-cache-dir,
    cli-memory, node-runtime-support — all pass.
    mcp-stdio-json-purity fails identically on both the fixed and the pre-fix (baseline) code
    under the current heavy multi-worktree devbox load (61s timeout waiting for the MCP subprocess
    to answer on stdout) — confirmed pre-existing/unrelated by reverting to HEAD:bin/omniroute.mjs
    and re-running the same test, which fails with the same timeout and message.
  • node scripts/check/check-changelog-integrity.mjs — OK

⚠️ base-red inherited: #12732 — unit #12058, integration codex-cache, package-artifact, tarball-smoke, agent-skills-sync

Closes #12296

bin/omniroute.mjs imported getNodeRuntimeSupport/getNodeRuntimeWarning but
never called them. On a Node/V8 build predating v-flag regex support, the
subsequent tsx/esm + Commander command-registration chain pulls in ora ->
string-width, whose top-level v-flag regex literals fail to parse, throwing
a bare 'Invalid regular expression flags' SyntaxError instead of a clear
runtime-support message. Call the guard right after the --version fast path
(and before the heavy import chain), skipping it for the same read-only
invocations shouldProvisionStorageKey already exempts.
@diegosouzapw
diegosouzapw merged commit 8d52dca into release/v3.8.51 Sep 12, 2026
15 of 22 checks passed
Githab-capibara added a commit to Githab-capibara/OmniRoute that referenced this pull request Sep 17, 2026
…nference#12296) (cheaper-inference#13253)

Merged as part of the 39-PR owner batch of 2026-09-11, validated as a unit.

Boarded into one consolidated worktree cut from `release/v3.8.51` with the other 38 — zero conflicts between them.

- ESLint over every changed file: no errors (the only finding was one suppression entry the batch emptied, pruned on cheaper-inference#13243)
- `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK
- complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437 — both under baseline
- 256 assertions green: 246 under node:test and 10 under vitest, which is where `tests/unit/**/*.test.tsx` actually runs
- `check-file-size`: `chatCore.ts` rebaselined 6144 → 6146 for cheaper-inference#13278 and cheaper-inference#13276, annotated and landed on cheaper-inference#13243

⚠️ base-red inherited: cheaper-inference#12732 — the provider count (356 in the docs vs the 358 the modules define) and `open-sse/utils/stream.ts` at 3115 > frozen 3098 both reproduce on the pure tip with zero contribution from this batch.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nference#12296) (cheaper-inference#13253)

Merged as part of the 39-PR owner batch of 2026-09-11, validated as a unit.

Boarded into one consolidated worktree cut from `release/v3.8.51` with the other 38 — zero conflicts between them.

- ESLint over every changed file: no errors (the only finding was one suppression entry the batch emptied, pruned on cheaper-inference#13243)
- `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, within baseline); `check:changelog-integrity` OK
- complexity 2821 / baseline 3218 and cognitive-complexity 1272 / baseline 1437 — both under baseline
- 256 assertions green: 246 under node:test and 10 under vitest, which is where `tests/unit/**/*.test.tsx` actually runs
- `check-file-size`: `chatCore.ts` rebaselined 6144 → 6146 for cheaper-inference#13278 and cheaper-inference#13276, annotated and landed on cheaper-inference#13243

⚠️ base-red inherited: cheaper-inference#12732 — the provider count (356 in the docs vs the 358 the modules define) and `open-sse/utils/stream.ts` at 3115 > frozen 3098 both reproduce on the pure tip with zero contribution from this batch.
@diegosouzapw
diegosouzapw deleted the fix/12296-regex-flags-crash branch October 18, 2026 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(startup): "Invalid regular expression flags" crash right after STORAGE_ENCRYPTION_KEY generation on first run

1 participant