mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-13 18:52:18 +03:00
setLKGP() was only ever called on success — nothing invalidated a "last
known good provider" pin once that provider started failing, so a
*separate* subsequent request kept re-selecting the same just-failed
target via applyStrategyOrdering.ts's LKGP reordering.
Live incident: an OpenClaw request to combo "default" (routerStrategy:
lkgp) got a real reasoning + apply_patch tool call from
opencode-zen/big-pickle, then 3 separate follow-up requests over the
next ~2 minutes each independently re-selected the same big-pickle
target and each timed out with "504 Stream produced no non-ping SSE
event within 95000ms" before the client gave up — instead of failing
over to any of the combo's other 12 models.
Root cause confirmed via code read: circuit breaker and model lockout
deliberately don't react to this failure class (isStreamReadinessFailureErrorBody
exempts STREAM_READINESS_TIMEOUT/combo_target_timeout 504s from tripping
the provider breaker, and REQUEST_SCOPED_UPSTREAM_ERROR_CODES suppresses
model-lockout recording for the same class — both intentional, to avoid
poisoning a healthy provider on request-specific timing). Nothing else
in the system was clearing the stale LKGP pin, so it kept winning
target-selection ordering for every new top-level request.
Fix: add clearLKGP(comboName, modelId) to src/lib/db/settings/lkgp.ts,
export it through settings.ts/localDb.ts, and call it (mirroring the
existing setLKGP-on-success call pattern exactly, same two keys) in both
combo.ts's per-target failure paths -- handleComboChat's "Done retrying
this model" block and handleRoundRobinCombo's structurally identical
twin -- right where a target is finally given up on and the loop moves
to the next one.
TDD: new regression test in tests/unit/combo-routing-engine.test.ts
("clears LKGP after the last-known-good target fails") reproduces the
exact live scenario -- confirmed failing against the pre-fix code,
passing after. Added direct unit coverage for clearLKGP itself in
tests/unit/db-settings-crud.test.ts (deletes only the targeted key,
sibling keys survive; no-op on an unset key doesn't throw) and
registered the new export in db-settings-split.test.ts's public API
surface characterization test.
Test plan:
- Full combo/LKGP-related suite (combo-routing-engine, db-settings-crud,
db-settings-split, combo-strategy-fallbacks,
combo-selected-connection-success,
delete-provider-connection-invalidates-lkgp-8887, db-read-cache) --
183/183 passing.
- npx tsc --noEmit -- clean for all changed files (pre-existing unrelated
errors elsewhere in the same test files confirmed identical against a
pristine upstream/release/v3.8.50 checkout, zero diff at those lines).
- npm run lint -- clean (new test's any usage properly typed, not left
to inflate the file's frozen any-budget suppression).
⚠️ base-red inherited: #9985
92 lines
2.7 KiB
TypeScript
92 lines
2.7 KiB
TypeScript
/**
|
|
* Characterization test: settings.ts god-file decomposition.
|
|
* Verifies that:
|
|
* 1. toRecord in shared.ts has correct behavior (DB-free, pure function).
|
|
* 2. The host settings.ts still re-exports the full public API surface.
|
|
*/
|
|
|
|
import { describe, it } from "node:test";
|
|
import assert from "node:assert/strict";
|
|
|
|
// ── 1. shared.ts — toRecord ──────────────────────────────────────────────────
|
|
|
|
import { toRecord } from "../../src/lib/db/settings/shared.ts";
|
|
|
|
describe("toRecord", () => {
|
|
it("returns the object as-is when given a plain object", () => {
|
|
const obj = { a: 1, b: "two" };
|
|
assert.deepStrictEqual(toRecord(obj), obj);
|
|
});
|
|
|
|
it("returns {} for null", () => {
|
|
assert.deepStrictEqual(toRecord(null), {});
|
|
});
|
|
|
|
it("returns {} for undefined", () => {
|
|
assert.deepStrictEqual(toRecord(undefined), {});
|
|
});
|
|
|
|
it("returns {} for a string", () => {
|
|
assert.deepStrictEqual(toRecord("hello"), {});
|
|
});
|
|
|
|
it("returns {} for a number", () => {
|
|
assert.deepStrictEqual(toRecord(42), {});
|
|
});
|
|
|
|
it("returns {} for an array (arrays are objects but toRecord returns the array cast)", () => {
|
|
// toRecord casts arrays as JsonRecord — they ARE objects, so the cast succeeds.
|
|
const arr = [1, 2, 3];
|
|
assert.strictEqual(toRecord(arr), arr);
|
|
});
|
|
});
|
|
|
|
// ── 2. settings.ts — public API surface ─────────────────────────────────────
|
|
|
|
const settingsModule = await import("../../src/lib/db/settings.ts");
|
|
|
|
describe("settings.ts public API surface", () => {
|
|
const expectedFunctions = [
|
|
// Settings core
|
|
"getSettings",
|
|
"updateSettings",
|
|
"isCloudEnabled",
|
|
// Proxy helpers (exported)
|
|
"bumpProxyConfigGeneration",
|
|
// Proxy config
|
|
"getProxyConfig",
|
|
"getProxyForLevel",
|
|
"setProxyForLevel",
|
|
"deleteProxyForLevel",
|
|
"resolveProxyForConnection",
|
|
"setProxyConfig",
|
|
// Pricing (re-exported from ./settings/pricing)
|
|
"getPricing",
|
|
"getPricingWithSources",
|
|
"getPricingForModel",
|
|
"updatePricing",
|
|
"resetPricing",
|
|
"resetAllPricing",
|
|
// LKGP (re-exported from ./settings/lkgp)
|
|
"getLKGP",
|
|
"setLKGP",
|
|
"clearAllLKGP",
|
|
"clearLKGP",
|
|
// Cache metrics (re-exported from ./settings/cacheMetrics)
|
|
"getCacheMetrics",
|
|
"updateCacheMetrics",
|
|
"getCacheTrend",
|
|
"resetCacheMetrics",
|
|
] as const;
|
|
|
|
for (const name of expectedFunctions) {
|
|
it(`exports "${name}" as a function`, () => {
|
|
assert.strictEqual(
|
|
typeof (settingsModule as Record<string, unknown>)[name],
|
|
"function",
|
|
`Expected "${name}" to be exported as a function`
|
|
);
|
|
});
|
|
}
|
|
});
|