mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-03 22:02:08 +03:00
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: the boot-time hydration (applyRuntimeSettings / restore hooks) lands on the instrumentation graph's copy, but the request path (base.ts) reads a different, un-hydrated copy. Live VPS validation proved the Thinking-Budget hydrate ran to completion at boot yet base.ts still read the passthrough default — why #5312 fix A stayed broken after the boot-wiring fix. Back the singletons with globalThis (the pattern systemPrompt.ts already uses for #2470) so all graph copies share one instance: - thinkingBudget.ts — dashboard Thinking-Budget mode reaches the executor - backgroundTaskDetector.ts — opt-in background degradation actually fires - systemTransforms.ts — operator pipeline overrides reach the request path payloadRules.ts was already safe (lazy per-request DB self-load, #2986). Guards: thinking-budget-globalthis-5312 + runtime-config-globalthis-5312 (assert globalThis sharing; a module-local let fails them, RED->GREEN).
This commit is contained in:
committed by
GitHub
parent
edd9e25b59
commit
aa6264be84
@@ -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`.
|
||||
|
||||
@@ -65,12 +65,27 @@ const DEFAULT_DEGRADATION_MAP: Record<string, string> = {
|
||||
|
||||
// ── 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<string, DegradationConfig | undefined>;
|
||||
|
||||
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<DegradationConfig>): 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<DegradationConfig
|
||||
*/
|
||||
export function getBackgroundDegradationConfig(): DegradationConfig {
|
||||
return {
|
||||
..._config,
|
||||
degradationMap: { ..._config.degradationMap },
|
||||
detectionPatterns: [..._config.detectionPatterns],
|
||||
stats: { ..._config.stats },
|
||||
...getConfig(),
|
||||
degradationMap: { ...getConfig().degradationMap },
|
||||
detectionPatterns: [...getConfig().detectionPatterns],
|
||||
stats: { ...getConfig().stats },
|
||||
};
|
||||
}
|
||||
|
||||
@@ -101,7 +116,7 @@ export function getBackgroundDegradationConfig(): DegradationConfig {
|
||||
* Reset stats counters.
|
||||
*/
|
||||
export function resetStats(): void {
|
||||
_config.stats = { detected: 0, tokensSaved: 0 };
|
||||
getConfig().stats = { detected: 0, tokensSaved: 0 };
|
||||
}
|
||||
|
||||
// ── Detection ───────────────────────────────────────────────────────────────
|
||||
@@ -187,7 +202,7 @@ export function getBackgroundTaskReason(
|
||||
if (!systemContent) return null;
|
||||
|
||||
// Check against detection patterns
|
||||
const matched = _config.detectionPatterns.some((pattern) =>
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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<string, SystemTransformsConfig | undefined>;
|
||||
|
||||
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<string, unknown>;
|
||||
|
||||
// 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;
|
||||
}
|
||||
|
||||
@@ -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<string, ThinkingBudgetConfig | undefined>;
|
||||
|
||||
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<ThinkingBudgetConfig>) {
|
||||
_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<ThinkingBudgetConfig> | 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.
|
||||
|
||||
77
tests/unit/runtime-config-globalthis-5312.test.ts
Normal file
77
tests/unit/runtime-config-globalthis-5312.test.ts
Normal file
@@ -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<string, unknown>;
|
||||
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)"
|
||||
);
|
||||
});
|
||||
53
tests/unit/thinking-budget-globalthis-5312.test.ts
Normal file
53
tests/unit/thinking-budget-globalthis-5312.test.ts
Normal file
@@ -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<string, { mode?: string; customBudget?: number }>;
|
||||
|
||||
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);
|
||||
});
|
||||
Reference in New Issue
Block a user