diff --git a/CHANGELOG.md b/CHANGELOG.md index c6ad0e1d5d..de1c87efae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ ### πŸ”§ Bug Fixes +- **thinking / runtime-config (module-graph fix):** operator-configured proxy settings that are hydrated at **boot** but read **per-request** were silently ignored in production. Next.js compiles `instrumentation.ts` (boot hydration via `applyRuntimeSettings` / restore hooks) as a **separate webpack module graph** from the app-route / open-sse executors, so a module-local `let _config` singleton is **duplicated** β€” the boot copy is hydrated but the request path reads a different, un-hydrated copy. Live VPS validation proved the Thinking-Budget hydration ran to completion at boot yet `base.ts` still saw the `passthrough` default (this is why #5312 fix A stayed broken even after the boot-wiring fix). Fixed by backing the singletons with `globalThis` (the pattern `systemPrompt.ts` already uses for the Global System Prompt, #2470), so all module-graph copies share one instance: **`thinkingBudget.ts`** (the dashboard Thinking-Budget mode now reaches the executor), **`backgroundTaskDetector.ts`** (the opt-in background-model degradation now actually fires on requests), and **`systemTransforms.ts`** (operator pipeline overrides now reach the request path). `payloadRules.ts` was already safe (it lazily self-loads from the DB per request, #2986). Regression guards: `tests/unit/thinking-budget-globalthis-5312.test.ts` + `tests/unit/runtime-config-globalthis-5312.test.ts` (assert globalThis-backed sharing; a module-local `let` fails them). ([#5312](https://github.com/diegosouzapw/OmniRoute/issues/5312)) + - **thinking (Claude OAuth):** restore the proxy-level **Thinking-Budget** config on startup. The dashboard mode (`auto`/`custom`/`adaptive`) is persisted under `settings.thinkingBudget`, but the boot-time hydration (`hydrateThinkingBudgetConfig`) was only wired into `src/server-init.ts` β€” an **unused module that never runs in production** β€” so the operator's choice silently reverted to the `passthrough` default on every restart (#5312 fix A was non-functional, even though its direct unit test passed). The hydration now runs in the real boot path (`src/instrumentation-node.ts`), alongside the Global System Prompt restore. Surfaced by live Anthropic-OAuth validation on the VPS. Regression guard: `tests/unit/thinking-budget-boot-wiring-5312.test.ts` (asserts the production boot module calls the hydration, not just the function in isolation). ([#5312](https://github.com/diegosouzapw/OmniRoute/issues/5312)) - **translator/chatcore (hardening):** re-apply two defensive review-fixes that were dropped in a branch rebuild before #5661 / #5662 landed. (1) `mergeConsecutiveSameRoleContents` (OpenAIβ†’Gemini) now shallow-copies each entry and its `parts` array instead of pushing the input reference, so the consecutive-same-role merge never mutates the caller's objects. (2) `defaultClaudeToolType` (Claude tool defaults) now passes any non-object array entry (`null` / primitive) through unchanged instead of spreading it into a fabricated `{ type: "custom", … }` tool. No behavior change on real payloads (Gemini contents are freshly built; Claude tools are always objects); both properties are now locked by regression tests in `tests/unit/translator-gemini-consecutive-role-2191.test.ts` and `tests/unit/claude-tool-type-default-2195.test.ts`. diff --git a/open-sse/services/backgroundTaskDetector.ts b/open-sse/services/backgroundTaskDetector.ts index c767c4c7f7..20ac799c5d 100644 --- a/open-sse/services/backgroundTaskDetector.ts +++ b/open-sse/services/backgroundTaskDetector.ts @@ -65,12 +65,27 @@ const DEFAULT_DEGRADATION_MAP: Record = { // ── State ─────────────────────────────────────────────────────────────────── -let _config: DegradationConfig = { - enabled: false, // Disabled by default β€” user must opt in - degradationMap: { ...DEFAULT_DEGRADATION_MAP }, - detectionPatterns: [...DEFAULT_DETECTION_PATTERNS], - stats: { detected: 0, tokensSaved: 0 }, -}; +// Backed by globalThis so the singleton is shared across the SEPARATE webpack +// module graphs Next.js builds for `instrumentation.ts` (boot-time hydration via +// applyRuntimeSettings β†’ setBackgroundDegradationConfig) and the app-route / +// open-sse executors (per-request reads in the chat handler). A module-local `let` +// is duplicated per graph, so the operator's opt-in (`enabled:true`) applied at boot +// never reaches the request path β€” the degradation silently never fires (the +// #5312-class module-graph bug). Mirrors systemPrompt.ts (#2470) and thinkingBudget.ts. +const GLOBAL_KEY = "__omniroute_backgroundDegradation_config__"; +const _store = globalThis as unknown as Record; + +function getConfig(): DegradationConfig { + if (!_store[GLOBAL_KEY]) { + _store[GLOBAL_KEY] = { + enabled: false, // Disabled by default β€” user must opt in + degradationMap: { ...DEFAULT_DEGRADATION_MAP }, + detectionPatterns: [...DEFAULT_DETECTION_PATTERNS], + stats: { detected: 0, tokensSaved: 0 }, + }; + } + return _store[GLOBAL_KEY]!; +} // ── Config Management ─────────────────────────────────────────────────────── @@ -78,10 +93,10 @@ let _config: DegradationConfig = { * Set the background degradation config (called from settings API or startup). */ export function setBackgroundDegradationConfig(config: Partial): void { - _config = { - ..._config, + _store[GLOBAL_KEY] = { + ...getConfig(), ...config, - stats: _config.stats, // preserve stats across config changes + stats: getConfig().stats, // preserve stats across config changes }; } @@ -90,10 +105,10 @@ export function setBackgroundDegradationConfig(config: Partial + const matched = getConfig().detectionPatterns.some((pattern) => systemContent.includes(pattern.toLowerCase()) ); @@ -224,9 +239,9 @@ export function isBackgroundTask( export function getDegradedModel(originalModel: string): string { if (!originalModel) return originalModel; - const degraded = _config.degradationMap[originalModel]; + const degraded = getConfig().degradationMap[originalModel]; if (degraded) { - _config.stats.detected++; + getConfig().stats.detected++; return degraded; } diff --git a/open-sse/services/systemTransforms.ts b/open-sse/services/systemTransforms.ts index a647a6c967..09eb210887 100644 --- a/open-sse/services/systemTransforms.ts +++ b/open-sse/services/systemTransforms.ts @@ -449,7 +449,23 @@ function resolveProviderConfig( // Runtime singleton. // ──────────────────────────────────────────────────────────────────────────── -let _systemTransformsConfig: SystemTransformsConfig = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; +// Backed by globalThis so the config is shared across the SEPARATE webpack module +// graphs Next.js builds for `instrumentation.ts` (boot-time hydration via +// applyRuntimeSettings β†’ setSystemTransformsConfig) and the app-route / open-sse +// executors (per-request reads in base.ts / claudeCodeCompatible.ts). A module-local +// `let` is duplicated per graph, so a Settings-UI override applied at boot never +// reaches the request path (the #5312-class module-graph bug; the protective +// compiled default still runs, but operator customizations were silently dropped). +// Mirrors systemPrompt.ts (#2470) and thinkingBudget.ts (#5312). +const GLOBAL_KEY = "__omniroute_systemTransforms_config__"; +const _store = globalThis as unknown as Record; + +function getStore(): SystemTransformsConfig { + if (!_store[GLOBAL_KEY]) { + _store[GLOBAL_KEY] = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; + } + return _store[GLOBAL_KEY]!; +} /** * Replace the active system-transforms config. Called from @@ -460,14 +476,14 @@ let _systemTransformsConfig: SystemTransformsConfig = DEFAULT_SYSTEM_TRANSFORMS_ */ export function setSystemTransformsConfig(input: unknown): void { if (!input || typeof input !== "object") { - _systemTransformsConfig = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; + _store[GLOBAL_KEY] = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; return; } const candidate = input as Record; // Legacy shape: { enabled, pipeline } β†’ migrate to per-provider map. if ("pipeline" in candidate && Array.isArray(candidate.pipeline)) { - _systemTransformsConfig = { + _store[GLOBAL_KEY] = { providers: { ...DEFAULT_SYSTEM_TRANSFORMS_CONFIG.providers, [PROVIDER_CC_BRIDGE]: { @@ -500,17 +516,17 @@ export function setSystemTransformsConfig(input: unknown): void { next.providers[providerId] = providerDefault; } } - _systemTransformsConfig = next; + _store[GLOBAL_KEY] = next; return; } - _systemTransformsConfig = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; + _store[GLOBAL_KEY] = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; } export function getSystemTransformsConfig(): SystemTransformsConfig { - return _systemTransformsConfig; + return getStore(); } export function resetSystemTransformsConfig(): void { - _systemTransformsConfig = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; + _store[GLOBAL_KEY] = DEFAULT_SYSTEM_TRANSFORMS_CONFIG; } diff --git a/open-sse/services/thinkingBudget.ts b/open-sse/services/thinkingBudget.ts index a16f3b432d..812c00aa35 100644 --- a/open-sse/services/thinkingBudget.ts +++ b/open-sse/services/thinkingBudget.ts @@ -56,8 +56,24 @@ export const DEFAULT_THINKING_CONFIG = { effortLevel: "medium", } satisfies ThinkingBudgetConfig; -// In-memory config (loaded from DB on startup, or default) -let _config: ThinkingBudgetConfig = { ...DEFAULT_THINKING_CONFIG }; +// In-memory config (loaded from DB on startup, or default). +// +// Backed by globalThis so the singleton is shared across the SEPARATE webpack +// module graphs Next.js builds for `instrumentation.ts` (boot-time hydration via +// hydrateThinkingBudgetConfig) and the app-route / open-sse executors (per-request +// reads in base.ts). A plain module-level `let` is DUPLICATED per graph, so the +// boot hydration would land on the instrumentation graph's copy and never reach +// base.ts β€” exactly the #5312 fix-A break proven on the VPS. Mirrors the same +// globalThis pattern systemPrompt.ts already uses for the Global System Prompt (#2470). +const GLOBAL_KEY = "__omniroute_thinkingBudget_config__"; +const _store = globalThis as unknown as Record; + +function getConfig(): ThinkingBudgetConfig { + if (!_store[GLOBAL_KEY]) { + _store[GLOBAL_KEY] = { ...DEFAULT_THINKING_CONFIG }; + } + return _store[GLOBAL_KEY]!; +} function toRecord(value: unknown): JsonRecord { return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : {}; @@ -72,14 +88,14 @@ function getStringField(record: JsonRecord, key: string): string { * Set the thinking budget config (called from settings API or startup) */ export function setThinkingBudgetConfig(config: Partial) { - _config = { ...DEFAULT_THINKING_CONFIG, ...config }; + _store[GLOBAL_KEY] = { ...DEFAULT_THINKING_CONFIG, ...config }; } /** * Get current thinking budget config */ export function getThinkingBudgetConfig() { - return { ..._config }; + return { ...getConfig() }; } /** @@ -195,7 +211,7 @@ export function applyThinkingBudget( body: unknown, config: Partial | null = null ) { - const cfg = config || _config; + const cfg = config || getConfig(); if (!body || typeof body !== "object") return body; // Early exit: strip ALL reasoning/thinking params for models that don't support them. diff --git a/tests/unit/runtime-config-globalthis-5312.test.ts b/tests/unit/runtime-config-globalthis-5312.test.ts new file mode 100644 index 0000000000..6b58c0c2ad --- /dev/null +++ b/tests/unit/runtime-config-globalthis-5312.test.ts @@ -0,0 +1,77 @@ +/** + * TDD regression for the #5312-class module-graph bug in the runtime-settings + * config singletons that are hydrated at boot (instrumentation graph, via + * applyRuntimeSettings) but read per-request (open-sse executor graph). + * + * Next.js compiles `instrumentation.ts` as a SEPARATE webpack module graph from the + * app-route / open-sse executors, so a module-local `let _config` is duplicated per + * graph β€” a boot-time hydration never reaches the request path. Two runtime-settings + * targets had this bug (audited): backgroundTaskDetector (opt-in degradation silently + * never fired) and systemTransforms (operator overrides silently dropped). Both are + * now globalThis-backed (payloadRules was already safe via lazy DB self-load, #2986). + * + * These tests assert globalThis-backing: a value written directly to the shared slot + * (simulating a hydrate in the instrumentation graph) MUST be observable through the + * getter, and the setter MUST write that same slot. A module-local `let` fails both. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + setBackgroundDegradationConfig, + getBackgroundDegradationConfig, +} from "../../open-sse/services/backgroundTaskDetector.ts"; +import { + setSystemTransformsConfig, + getSystemTransformsConfig, + resetSystemTransformsConfig, +} from "../../open-sse/services/systemTransforms.ts"; + +const store = globalThis as unknown as Record; +const BG_KEY = "__omniroute_backgroundDegradation_config__"; +const ST_KEY = "__omniroute_systemTransforms_config__"; + +test.afterEach(() => { + setBackgroundDegradationConfig({ enabled: false }); + resetSystemTransformsConfig(); +}); + +test("#5312-class: backgroundDegradation setter writes the globalThis-shared slot", () => { + setBackgroundDegradationConfig({ enabled: true }); + assert.equal( + (store[BG_KEY] as { enabled?: boolean })?.enabled, + true, + "config must live on globalThis so the operator opt-in survives Next's separate module graphs" + ); +}); + +test("#5312-class: backgroundDegradation getter observes a cross-graph hydrate", () => { + store[BG_KEY] = { + enabled: true, + degradationMap: { "claude-opus-4-8": "claude-haiku-4-5" }, + detectionPatterns: [], + stats: { detected: 0, tokensSaved: 0 }, + }; + assert.equal( + getBackgroundDegradationConfig().enabled, + true, + "getter must read globalThis; a module-local `let _config` would keep enabled=false (opt-in dead on the request path)" + ); +}); + +test("#5312-class: systemTransforms setter writes the globalThis-shared slot", () => { + setSystemTransformsConfig({ providers: { claude: { enabled: false, pipeline: [] } } }); + assert.ok( + store[ST_KEY], + "systemTransforms config must live on globalThis so operator overrides survive Next's module graphs" + ); +}); + +test("#5312-class: systemTransforms getter observes a cross-graph hydrate", () => { + const marker = { providers: { claude: { enabled: false, pipeline: [] } } }; + store[ST_KEY] = marker; + assert.equal( + getSystemTransformsConfig().providers.claude.enabled, + false, + "getter must read globalThis; a module-local `let` would keep the compiled default (operator override lost)" + ); +}); diff --git a/tests/unit/thinking-budget-globalthis-5312.test.ts b/tests/unit/thinking-budget-globalthis-5312.test.ts new file mode 100644 index 0000000000..89dcbe685a --- /dev/null +++ b/tests/unit/thinking-budget-globalthis-5312.test.ts @@ -0,0 +1,53 @@ +/** + * TDD regression for #5312 (fix A β€” module-graph fix): the Thinking-Budget config + * singleton MUST be backed by globalThis, not a module-local `let _config`. + * + * Next.js compiles `instrumentation.ts` as a SEPARATE webpack module graph from the + * app-route / open-sse executors. A module-local singleton hydrated at boot (in the + * instrumentation graph, by hydrateThinkingBudgetConfig) is a DIFFERENT object than + * the one `base.ts` reads per-request β€” so the operator's dashboard Thinking-Budget + * mode silently never took effect in production (proven live on the VPS: register() + * + registerNodejs() ran and hydrate returned true, yet base.ts still read the + * default). Backing the config with globalThis (mirroring systemPrompt.ts #2470) + * shares the one instance across graphs. + * + * These tests assert the storage is globalThis-backed. A module-local `let` fails + * both: (1) setThinkingBudgetConfig would not populate the shared slot, and (2) + * getThinkingBudgetConfig would not observe a value written to the shared slot by + * "another module graph". + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + setThinkingBudgetConfig, + getThinkingBudgetConfig, + DEFAULT_THINKING_CONFIG, +} from "../../open-sse/services/thinkingBudget.ts"; + +const GLOBAL_KEY = "__omniroute_thinkingBudget_config__"; +const store = globalThis as unknown as Record; + +test.afterEach(() => { + setThinkingBudgetConfig(DEFAULT_THINKING_CONFIG); +}); + +test("#5312: setThinkingBudgetConfig writes to the globalThis-shared slot", () => { + setThinkingBudgetConfig({ mode: "auto" }); + assert.equal( + store[GLOBAL_KEY]?.mode, + "auto", + "config must live on globalThis so it is shared across Next's separate module graphs (instrumentation vs routes)" + ); +}); + +test("#5312: getThinkingBudgetConfig reads the globalThis-shared slot (a cross-graph hydrate reaches readers)", () => { + // Simulate a hydrate that happened in a DIFFERENT webpack module graph (the + // instrumentation graph) by writing the shared globalThis slot directly. + store[GLOBAL_KEY] = { mode: "custom", customBudget: 8192 }; + assert.equal( + getThinkingBudgetConfig().mode, + "custom", + "getter must read globalThis; a module-local `let _config` would not see the cross-graph hydration (the #5312 fix-A break)" + ); + assert.equal(getThinkingBudgetConfig().customBudget, 8192); +});