mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
#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).
This commit is contained in:
committed by
GitHub
parent
7d07be9b20
commit
cded5146f0
@@ -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))
|
||||
|
||||
|
||||
@@ -60,20 +60,40 @@ const BUILT_IN_ALIASES: Record<string, string> = {
|
||||
};
|
||||
|
||||
// ── Custom Aliases (persisted via Settings API) ─────────────────────────────
|
||||
let _customAliases: Record<string, string> = {};
|
||||
//
|
||||
// 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<string, string> | undefined
|
||||
>;
|
||||
|
||||
function customAliases(): Record<string, string> {
|
||||
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<string, string>): void {
|
||||
_customAliases = { ...aliases };
|
||||
_aliasStore[CUSTOM_ALIASES_GLOBAL_KEY] = { ...aliases };
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current custom aliases.
|
||||
*/
|
||||
export function getCustomAliases(): Record<string, string> {
|
||||
return { ..._customAliases };
|
||||
return { ...customAliases() };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -81,7 +101,7 @@ export function getCustomAliases(): Record<string, string> {
|
||||
* Custom aliases take precedence over built-in.
|
||||
*/
|
||||
export function getAllAliases(): Record<string, string> {
|
||||
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;
|
||||
|
||||
71
tests/unit/model-aliases-globalthis-5777.test.ts
Normal file
71
tests/unit/model-aliases-globalthis-5777.test.ts
Normal file
@@ -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<string, Record<string, string> | 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);
|
||||
});
|
||||
Reference in New Issue
Block a user