From 56b9d00335f202a32ee16e8a976ea71be69805c8 Mon Sep 17 00:00:00 2001 From: Ravi Tharuma <25951435+RaviTharuma@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:48:07 +0200 Subject: [PATCH] fix(docker): warn when OMNIROUTE_MEMORY_MB disagrees with NODE_OPTIONS heap (#10818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged — validated together with a batch of related RaviTharuma PRs in one combined worktree (typecheck:core clean, complexity/file-size/changelog gates green, focused tests passing). Thanks for the contribution! --- .../fixes/10353-memory-heap-conflict-warn.md | 1 + docs/reference/ENVIRONMENT.md | 2 +- scripts/build/runtime-env.mjs | 53 ++++++++++++++ scripts/dev/run-standalone.mjs | 14 ++-- tests/unit/10353-heap-limit-conflict.test.ts | 73 +++++++++++++++++++ 5 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 changelog.d/fixes/10353-memory-heap-conflict-warn.md create mode 100644 tests/unit/10353-heap-limit-conflict.test.ts diff --git a/changelog.d/fixes/10353-memory-heap-conflict-warn.md b/changelog.d/fixes/10353-memory-heap-conflict-warn.md new file mode 100644 index 0000000000..c52b7cc15c --- /dev/null +++ b/changelog.d/fixes/10353-memory-heap-conflict-warn.md @@ -0,0 +1 @@ +- **fix(docker):** warn at boot when `OMNIROUTE_MEMORY_MB` disagrees with `NODE_OPTIONS --max-old-space-size`, and document that the standalone/Docker launcher appends `OMNIROUTE_MEMORY_MB` last ([#10353](https://github.com/diegosouzapw/OmniRoute/issues/10353)) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index b35d7c97d9..73a0c8a08d 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -843,7 +843,7 @@ The logging system writes to both stdout and rotated log files. All configuratio | Variable | Default | Description | | -------------------------- | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `OMNIROUTE_MEMORY_MB` | _auto_ | Runtime V8 heap limit (MB). When unset, calibrated dynamically (~35% of system RAM, clamped to `[512, 4096]`); `512` is only the floor when total memory can't be read. Set explicitly to override. Docker standalone and `omniroute serve` use it to set `--max-old-space-size`. | +| `OMNIROUTE_MEMORY_MB` | _auto_ | **Recommended** Docker/standalone V8 heap limit (MB). When unset, calibrated dynamically (~35% of system RAM, clamped to `[512, 4096]`); `512` is only the floor when total memory can't be read. On `run-standalone.mjs` (Docker CMD), an **explicit** value is appended as `--max-old-space-size` and **wins** over a conflicting NODE_OPTIONS heap flag (V8 last-flag). `omniroute serve` still prefers an existing NODE_OPTIONS heap (#5238). Do not set both to different numbers — the process logs a warn naming both values and the winner. | | `PROMPT_CACHE_MAX_SIZE` | `50` | Max cached system prompt entries. | | `PROMPT_CACHE_MAX_BYTES` | `2097152` (2 MB) | Max total prompt cache size. | | `PROMPT_CACHE_TTL_MS` | `300000` (5 min) | Prompt cache entry TTL. | diff --git a/scripts/build/runtime-env.mjs b/scripts/build/runtime-env.mjs index d8dbe45765..e4eec02ed1 100644 --- a/scripts/build/runtime-env.mjs +++ b/scripts/build/runtime-env.mjs @@ -49,6 +49,59 @@ export function envHasExplicitHeapFlag(env) { return String(sourceEnv?.NODE_OPTIONS || "").includes(MAX_OLD_SPACE_FLAG); } +/** Last `--max-old-space-size=` value in NODE_OPTIONS, or null if absent. */ +export function parseNodeOptionsHeapMb(nodeOptions) { + const matches = [...String(nodeOptions || "").matchAll(/--max-old-space-size=(\d+)/g)]; + if (matches.length === 0) return null; + const parsed = Number.parseInt(matches[matches.length - 1][1], 10); + return Number.isFinite(parsed) ? parsed : null; +} + +/** + * True when OMNIROUTE_MEMORY_MB is an explicit in-range integer (not the + * unset/invalid fallback). Docker images set this; Compose may also set + * NODE_OPTIONS — #10353 needs to know both knobs were intentionally present. + */ +export function envHasExplicitOmnirouteMemoryMb(env) { + const sourceEnv = arguments.length === 0 ? process.env : env; + const parsed = Number.parseInt(String(sourceEnv?.OMNIROUTE_MEMORY_MB ?? ""), 10); + return Number.isFinite(parsed) && parsed >= 64 && parsed <= 16384; +} + +/** + * Docker `run-standalone.mjs` appends `--max-old-space-size` from + * OMNIROUTE_MEMORY_MB. V8 last-flag semantics mean that appended value wins + * over an earlier NODE_OPTIONS heap. Warn once when both are set and disagree + * so env dumps stop looking like NODE_OPTIONS is in effect (#10353). + * + * @returns {boolean} true when a warn was emitted + */ +export function warnConflictingHeapLimits(env, omnirouteMb, log = console.warn) { + const nodeMb = parseNodeOptionsHeapMb(env?.NODE_OPTIONS); + if (nodeMb == null || !envHasExplicitOmnirouteMemoryMb(env)) return false; + if (nodeMb === omnirouteMb) return false; + log( + `[omniroute] heap limit conflict: OMNIROUTE_MEMORY_MB=${omnirouteMb} disagrees with NODE_OPTIONS --max-old-space-size=${nodeMb}. ` + + `run-standalone.mjs / Docker appends OMNIROUTE_MEMORY_MB last, so the effective V8 heap is ${omnirouteMb} MB. ` + + `Set only OMNIROUTE_MEMORY_MB (recommended) or make both values match.` + ); + return true; +} + +/** + * NODE_OPTIONS string for Docker / run-standalone.mjs. + * Explicit OMNIROUTE_MEMORY_MB always appends (wins). Otherwise keep an + * existing NODE_OPTIONS heap flag (#5238). Otherwise append the fallback. + */ +export function buildStandaloneNodeOptions(env = process.env, omnirouteMb) { + const existing = String(env?.NODE_OPTIONS || "").trim(); + if (envHasExplicitOmnirouteMemoryMb(env)) { + return `${existing} ${MAX_OLD_SPACE_FLAG}=${omnirouteMb}`.trim(); + } + if (existing.includes(MAX_OLD_SPACE_FLAG)) return existing; + return `${existing} ${MAX_OLD_SPACE_FLAG}=${omnirouteMb}`.trim(); +} + /** * Assemble the NODE_OPTIONS string for the spawned server, preserving any flags * the user already exported. #5238: `omniroute serve` used to UNCONDITIONALLY diff --git a/scripts/dev/run-standalone.mjs b/scripts/dev/run-standalone.mjs index 531322da77..0f26e804ad 100644 --- a/scripts/dev/run-standalone.mjs +++ b/scripts/dev/run-standalone.mjs @@ -5,6 +5,8 @@ import { resolveRuntimePorts, withRuntimePortEnv, resolveMaxOldSpaceMb, + warnConflictingHeapLimits, + buildStandaloneNodeOptions, spawnWithForwardedSignals, } from "../build/runtime-env.mjs"; import { bootstrapEnv } from "../build/bootstrap-env.mjs"; @@ -13,13 +15,13 @@ const env = bootstrapEnv(); const runtimePorts = resolveRuntimePorts(env); const childEnv = withRuntimePortEnv(env, runtimePorts); -// #2939: honor OMNIROUTE_MEMORY_MB (default 512), the same knob -// `omniroute serve` uses, so Docker users can control the server heap under -// load / large SQLite DBs. A trailing --max-old-space-size wins, so this -// overrides the image fallback without clobbering any other NODE_OPTIONS flags. +// #2939 / #10353: OMNIROUTE_MEMORY_MB is the Docker/standalone heap knob. +// When it is set, we append --max-old-space-size last (V8 last-flag wins). +// When it is unset and NODE_OPTIONS already pins the heap, keep NODE_OPTIONS +// (#5238). Warn when both are set and the numbers disagree. const maxOldSpaceMb = resolveMaxOldSpaceMb(childEnv.OMNIROUTE_MEMORY_MB); -childEnv.NODE_OPTIONS = - `${childEnv.NODE_OPTIONS || ""} --max-old-space-size=${maxOldSpaceMb}`.trim(); +warnConflictingHeapLimits(childEnv, maxOldSpaceMb); +childEnv.NODE_OPTIONS = buildStandaloneNodeOptions(childEnv, maxOldSpaceMb); // Prefer the WS-aware wrapper (server-ws.mjs) over the bare Next standalone // server.js: it installs the trusted peer-IP stamp (scripts/dev/peer-stamp.mjs) diff --git a/tests/unit/10353-heap-limit-conflict.test.ts b/tests/unit/10353-heap-limit-conflict.test.ts new file mode 100644 index 0000000000..c1c7a1d7c3 --- /dev/null +++ b/tests/unit/10353-heap-limit-conflict.test.ts @@ -0,0 +1,73 @@ +/** + * #10353 — warn when OMNIROUTE_MEMORY_MB disagrees with NODE_OPTIONS heap. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { + parseNodeOptionsHeapMb, + envHasExplicitOmnirouteMemoryMb, + warnConflictingHeapLimits, + buildStandaloneNodeOptions, +} = await import("../../scripts/build/runtime-env.mjs"); + +test("parseNodeOptionsHeapMb reads the last heap flag", () => { + assert.equal(parseNodeOptionsHeapMb(""), null); + assert.equal(parseNodeOptionsHeapMb("--enable-source-maps"), null); + assert.equal(parseNodeOptionsHeapMb("--max-old-space-size=512"), 512); + assert.equal( + parseNodeOptionsHeapMb("--max-old-space-size=512 --max-old-space-size=2048"), + 2048 + ); +}); + +test("envHasExplicitOmnirouteMemoryMb requires an in-range integer", () => { + assert.equal(envHasExplicitOmnirouteMemoryMb({}), false); + assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "" }), false); + assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "abc" }), false); + assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "32" }), false); + assert.equal(envHasExplicitOmnirouteMemoryMb({ OMNIROUTE_MEMORY_MB: "2048" }), true); +}); + +test("#10353 dual-set disagree → warn + OMNIROUTE_MEMORY_MB wins", () => { + const messages: string[] = []; + const env = { + NODE_OPTIONS: "--max-old-space-size=512", + OMNIROUTE_MEMORY_MB: "2048", + }; + assert.equal(warnConflictingHeapLimits(env, 2048, (m: string) => messages.push(m)), true); + assert.match(messages[0], /OMNIROUTE_MEMORY_MB=2048/); + assert.match(messages[0], /--max-old-space-size=512/); + assert.match(messages[0], /effective V8 heap is 2048 MB/); + assert.equal( + buildStandaloneNodeOptions(env, 2048), + "--max-old-space-size=512 --max-old-space-size=2048" + ); +}); + +test("#10353 only one knob set → no conflict warn", () => { + const messages: string[] = []; + const log = (m: string) => messages.push(m); + assert.equal( + warnConflictingHeapLimits({ NODE_OPTIONS: "--max-old-space-size=512" }, 512, log), + false + ); + assert.equal( + warnConflictingHeapLimits({ OMNIROUTE_MEMORY_MB: "2048" }, 2048, log), + false + ); + assert.equal( + warnConflictingHeapLimits( + { NODE_OPTIONS: "--max-old-space-size=1024", OMNIROUTE_MEMORY_MB: "1024" }, + 1024, + log + ), + false + ); + assert.equal(messages.length, 0); +}); + +test("#10353 unset OMNIROUTE_MEMORY_MB keeps NODE_OPTIONS heap", () => { + const env = { NODE_OPTIONS: "--max-old-space-size=8192" }; + assert.equal(buildStandaloneNodeOptions(env, 512), "--max-old-space-size=8192"); +});