From cded5146f072f7043b4bfa9d50ea9e2f650b6782 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Wed, 1 Jul 2026 09:11:13 -0300 Subject: [PATCH] fix(model-aliases): back custom-alias store with globalThis (#5777 follow-up) (#5821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #5777 self-healed the GET /api/settings/model-aliases symptom at the route layer, but the root cause remained: modelDeprecation.ts held _customAliases in a plain module-level let, which webpack duplicates across the startup and app-route module graphs (same class as #5312). Startup hydration landed on one copy; the API route read the other (empty) one. Back the store with globalThis (__omniroute_customAliases__) so both instances share one store — the exact pattern already used by thinkingBudget.ts/backgroundTaskDetector.ts (#5312). The route-layer DB self-heal from #5777 stays as a harmless fallback. Extends #5777 (thanks @jleonar2). Regression: tests/unit/model-aliases-globalthis-5777.test.ts (fails on the plain-let store: never populates globalThis, never reads a sibling instance's write). --- CHANGELOG.md | 2 +- open-sse/services/modelDeprecation.ts | 38 +++++++--- .../model-aliases-globalthis-5777.test.ts | 71 +++++++++++++++++++ 3 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 tests/unit/model-aliases-globalthis-5777.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bcafea25a3..169468a9f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,7 +32,7 @@ ### 🔧 Bug Fixes -- **settings (model aliases — self-heal after restart):** the Settings → Routing page showed "No exact-match aliases configured" after a server restart even though the aliases were persisted in the DB. Aliases are held in a module-local `_customAliases` map in `modelDeprecation.ts` that the boot path hydrates, but Next.js compiles the app-route module graph separately from the startup graph (the same webpack chunk-splitting class as #5312), so the `GET /api/settings/model-aliases` handler read a different, un-hydrated copy. The handler now self-heals: when its in-memory alias map is empty it reads `settings.modelAliases` from the DB (via the existing `getSettings()` db module — no raw SQL in the route) and repopulates the map, so the UI reflects the persisted aliases on the first GET after a restart. Regression guard: `tests/unit/model-aliases-settings-route-selfheal.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2)) +- **settings (model aliases — self-heal after restart):** the Settings → Routing page showed "No exact-match aliases configured" after a server restart even though the aliases were persisted in the DB. Aliases are held in a module-local `_customAliases` map in `modelDeprecation.ts` that the boot path hydrates, but Next.js compiles the app-route module graph separately from the startup graph (the same webpack chunk-splitting class as #5312), so the `GET /api/settings/model-aliases` handler read a different, un-hydrated copy. The handler now self-heals: when its in-memory alias map is empty it reads `settings.modelAliases` from the DB (via the existing `getSettings()` db module — no raw SQL in the route) and repopulates the map, so the UI reflects the persisted aliases on the first GET after a restart. Follow-up: the root cause is now also fixed — the `_customAliases` store in `modelDeprecation.ts` is backed by `globalThis` (key `__omniroute_customAliases__`), so the startup and app-route module graphs share **one** store and the route reads the boot-hydrated aliases directly (the DB self-heal remains as a harmless fallback), mirroring the same `globalThis` singleton pattern already applied to `thinkingBudget.ts`/`backgroundTaskDetector.ts` (#5312). Regression guards: `tests/unit/model-aliases-settings-route-selfheal.test.ts` + `tests/unit/model-aliases-globalthis-5777.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2)) - **providers (grok-cli token auto-refresh):** grok-cli OAuth tokens were never proactively refreshed before their real expiry. `mapTokens` hardcoded `expiresIn: 21600` (6 h) regardless of the token's actual lifetime, so the persisted `expiresAt` was always "now + 6 h" and the proactive `tokenHealthCheck` sweep (refresh when `expiresAt - now < 5 min`) fired 6 h after import instead of shortly before the token really expired. `mapTokens` now computes `expiresIn` from the authoritative `expires_at` field in `~/.grok/auth.json` (ISO → epoch-seconds) with a fallback to the JWT `exp` claim (payload-only decode, no signature trust); the hardcoded `21600` is kept only when neither is present. An already-expired token (real `expires_at`/`exp` in the past) is now clamped to a positive `expiresIn` via `Math.max(1, …)`, so the import route stores a near-future `expiresAt` and AutoCombo refreshes the connection instead of reading a past date and excluding it outright. Regression guards: 5 cases in `tests/unit/grok-cli-oauth.test.ts` (JWT `exp`, JSON `expires_at`, the `21600` fallback, and the two expired-token clamps). ([#5775](https://github.com/diegosouzapw/OmniRoute/pull/5775) — thanks [@Chewji9875](https://github.com/Chewji9875)) diff --git a/open-sse/services/modelDeprecation.ts b/open-sse/services/modelDeprecation.ts index b38b76c36f..cf965abf34 100644 --- a/open-sse/services/modelDeprecation.ts +++ b/open-sse/services/modelDeprecation.ts @@ -60,20 +60,40 @@ const BUILT_IN_ALIASES: Record = { }; // ── Custom Aliases (persisted via Settings API) ───────────────────────────── -let _customAliases: Record = {}; +// +// Backed by globalThis so the singleton store is shared across the SEPARATE webpack +// module graphs Next.js builds for `instrumentation.ts` (boot-time hydration via +// applyRuntimeSettings → setCustomAliases) and the app-route `GET /api/settings/model-aliases`. +// A plain module-level `let` is DUPLICATED per graph, so startup hydration lands on the +// instrumentation graph's copy while the API route reads an empty copy — the exact +// symptom #5777 patched at the route layer. Migrating the store to globalThis fixes the +// root cause (both instances read/write one store), mirroring the #5312 pattern already +// applied to thinkingBudget.ts and backgroundTaskDetector.ts (and systemPrompt.ts #2470). +const CUSTOM_ALIASES_GLOBAL_KEY = "__omniroute_customAliases__"; +const _aliasStore = globalThis as unknown as Record< + string, + Record | undefined +>; + +function customAliases(): Record { + if (!_aliasStore[CUSTOM_ALIASES_GLOBAL_KEY]) { + _aliasStore[CUSTOM_ALIASES_GLOBAL_KEY] = {}; + } + return _aliasStore[CUSTOM_ALIASES_GLOBAL_KEY]!; +} /** * Set custom aliases (called from settings API or startup). */ export function setCustomAliases(aliases: Record): void { - _customAliases = { ...aliases }; + _aliasStore[CUSTOM_ALIASES_GLOBAL_KEY] = { ...aliases }; } /** * Get current custom aliases. */ export function getCustomAliases(): Record { - return { ..._customAliases }; + return { ...customAliases() }; } /** @@ -81,7 +101,7 @@ export function getCustomAliases(): Record { * Custom aliases take precedence over built-in. */ export function getAllAliases(): Record { - return { ...BUILT_IN_ALIASES, ..._customAliases }; + return { ...BUILT_IN_ALIASES, ...customAliases() }; } /** @@ -95,7 +115,8 @@ export function resolveModelAlias(modelId: string): string { if (!modelId) return modelId; // Check custom aliases first (higher priority) - if (_customAliases[modelId]) return _customAliases[modelId]; + const custom = customAliases(); + if (custom[modelId]) return custom[modelId]; // Then check built-in if (BUILT_IN_ALIASES[modelId]) return BUILT_IN_ALIASES[modelId]; @@ -129,15 +150,16 @@ export function isDeprecated(modelId: string): boolean { * Add a custom alias. */ export function addCustomAlias(from: string, to: string): void { - _customAliases[from] = to; + customAliases()[from] = to; } /** * Remove a custom alias. */ export function removeCustomAlias(from: string): boolean { - if (_customAliases[from]) { - delete _customAliases[from]; + const custom = customAliases(); + if (custom[from]) { + delete custom[from]; return true; } return false; diff --git a/tests/unit/model-aliases-globalthis-5777.test.ts b/tests/unit/model-aliases-globalthis-5777.test.ts new file mode 100644 index 0000000000..cfa79239e0 --- /dev/null +++ b/tests/unit/model-aliases-globalthis-5777.test.ts @@ -0,0 +1,71 @@ +/** + * #5777 follow-up — root-cause regression guard for the custom-alias store. + * + * The standalone production build makes webpack emit TWO copies of + * `open-sse/services/modelDeprecation.ts`: one hydrated at startup + * (`applyRuntimeSettings` → `setCustomAliases`) for request routing, and one used by + * `GET /api/settings/model-aliases`. When the store was a plain module-level + * `let _customAliases`, each copy had its own state, so the API route read an empty + * map after restart (#5777). The store is now backed by `globalThis` so BOTH module + * instances share one object — the same #5312 pattern used by thinkingBudget.ts / + * backgroundTaskDetector.ts. + * + * These tests fail on the old plain-`let` implementation: it never touches globalThis + * (test 1) and never reads a value another instance wrote to globalThis (test 2). + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const GLOBAL_KEY = "__omniroute_customAliases__"; +const g = globalThis as unknown as Record | undefined>; + +const modelDeprecation = await import("../../open-sse/services/modelDeprecation.ts"); + +test.beforeEach(() => { + delete g[GLOBAL_KEY]; +}); + +test.after(() => { + delete g[GLOBAL_KEY]; +}); + +test("#5777: setCustomAliases writes through globalThis (store is not a per-module let)", () => { + modelDeprecation.setCustomAliases({ "old-model": "new-model" }); + + // A plain module-level `let` would never populate globalThis; the globalThis-backed + // store does. This is what lets a second webpack module instance see the write. + assert.deepEqual( + g[GLOBAL_KEY], + { "old-model": "new-model" }, + "custom aliases must live on globalThis so both webpack module graphs share them" + ); +}); + +test("#5777: reads reflect a value written by another module instance via globalThis", () => { + // Simulate the OTHER webpack module instance (the startup/instrumentation graph) + // hydrating the shared store — this module instance did NOT call setCustomAliases. + g[GLOBAL_KEY] = { "claude-opus-4-8": "mimo/mimo-v2.5-pro" }; + + // With the plain-`let` store this module's own copy would still be empty (the #5777 + // bug). With the globalThis backing the read reflects the other instance's write. + assert.deepEqual(modelDeprecation.getCustomAliases(), { + "claude-opus-4-8": "mimo/mimo-v2.5-pro", + }); + assert.equal(modelDeprecation.resolveModelAlias("claude-opus-4-8"), "mimo/mimo-v2.5-pro"); + assert.equal( + modelDeprecation.getAllAliases()["claude-opus-4-8"], + "mimo/mimo-v2.5-pro", + "getAllAliases must merge the globalThis-backed custom aliases over built-ins" + ); +}); + +test("#5777: addCustomAlias / removeCustomAlias mutate the shared globalThis store", () => { + modelDeprecation.setCustomAliases({}); + modelDeprecation.addCustomAlias("foo", "bar"); + assert.equal(g[GLOBAL_KEY]?.foo, "bar", "addCustomAlias must mutate the globalThis store"); + assert.equal(modelDeprecation.resolveModelAlias("foo"), "bar"); + + assert.equal(modelDeprecation.removeCustomAlias("foo"), true); + assert.equal(g[GLOBAL_KEY]?.foo, undefined, "removeCustomAlias must clear it from globalThis"); + assert.equal(modelDeprecation.removeCustomAlias("missing"), false); +});