mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-09 17:28:51 +00:00
fix(resilience): release combo session-stickiness pin on a terminal/quality-rejected account (#6692)
applySessionStickiness() gated the sticky pin only on 5h/weekly usage headroom, which is orthogonal to account availability, so a credits_exhausted/banned/ expired/rate-limited connection (or a quality-validation-rejected 200) kept being re-promoted forever, defeating failover for that conversation.
This commit is contained in:
parent
f4fb0d310b
commit
9704a78eef
5 changed files with 440 additions and 8 deletions
|
|
@ -28,6 +28,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
|||
- **fix(providers):** `fusion` combo strategy silently returned a panel member's raw answer instead of the configured `config.judgeModel` synthesis ([#6455](https://github.com/diegosouzapw/OmniRoute/issues/6455)) — `handleFusionChat()`'s single-survivor "degrade gracefully" path (added for #6454) returned the lone panel answer directly whenever only one panelist succeeded, regardless of whether an explicit `judgeModel` was configured; with the default `minPanel: 2` and a 2-model panel, any single flaky/rate-limited panelist forced this path on every request, so the configured judge (e.g. `auto/claude-opus`) was never invoked and the client-visible `.model` reflected whichever panelist happened to survive. The judge is now still invoked to synthesize a lone surviving answer whenever `judgeModel` is explicitly configured; the cheap direct-answer shortcut is kept only for the implicit case (no `judgeModel` set, where the "judge" is just `panel[0]`). Regression guard: `tests/unit/fusion-judge-model-6455.test.ts` + updated `tests/unit/combo-fusion-strategy.test.ts`. (thanks @chirag127)
|
||||
- **feat(combo):** sanitized diagnostic trace on an auto-combo terminal failure — instead of an opaque 503, a terminal combo failure now returns a whitelist-projected trace (candidate pool size, attempted count, excluded provider/reason codes, attempt order, and a terminal-reason code) via the new `errorResponseWithComboDiagnostics()`/`sanitizeComboDiagnostics()` in `open-sse/utils/error.ts` — provider/model ids and enumerated reason codes only, never keys/tokens/bodies, length- and count-capped. A reasoning-budget-exhausted panel now returns an actionable "increase max_tokens" message rather than a blind retry-limit 503. Regression guard: `tests/unit/combo-diagnostics-trace.test.ts`. ([#6545](https://github.com/diegosouzapw/OmniRoute/pull/6545)) — see PR. (thanks @developerjillur)
|
||||
- **fix(providers):** image/diffusion models discovered from an upstream catalog (e.g. HuggingFace's live `/v1/models`) are no longer advertised as chat models ([#6457](https://github.com/diegosouzapw/OmniRoute/issues/6457)) — the chat catalog builder defaulted synced models with no modality info to `endpoints: ["chat"]`, so `huggingface/stabilityai/stable-diffusion-xl-base-1.0` showed up in the chat `/v1/models` listing and returned `400 "not a chat model"` when called. `catalog.ts` now skips any synced model already registered as an image model for that provider (via the new `isRegisteredImageModel()`), leaving `getAllImageModels()` to list it with the correct `type: "image"`. Regression guard: `tests/unit/image-model-not-in-chat-catalog-6457.test.ts`.
|
||||
- **fix(resilience):** combo session stickiness never released a pin on a credits-exhausted/banned/expired account, permanently defeating failover for that conversation ([#6692](https://github.com/diegosouzapw/OmniRoute/issues/6692)) — `applySessionStickiness()` (`open-sse/services/combo/sessionStickiness.ts`) gated the sticky pin only on 5h/weekly usage-percentage headroom, which is orthogonal to account availability: a `credits_exhausted`/`banned`/`expired` connection, or one still inside its `rateLimitedUntil` cooldown, reports perfectly healthy headroom, so the pin was force-promoted back to the front of the target list on every subsequent turn. `clearStickyBinding()` also had zero call sites in `combo.ts`'s failure paths, so a quality-validation-rejected 200 (a masked daily-cap refusal) never released the pin either. The gate now also resolves the bound connection's terminal status/cooldown via a new injectable fetcher seam (fail-open on lookup errors, mirroring the existing saturation fetcher), and `combo.ts`'s two dispatchers (`handleComboChat`/`handleRoundRobinCombo`) release the pin immediately at both their connection-exhaustion classification point and their quality-validation-failure branch via the new `releaseStickyPinOnFailure()`. Regression guard: `tests/unit/repro-6692-sticky-terminal.test.ts` + extended `tests/unit/combo-session-stickiness.test.ts`.
|
||||
- **fix(test):** replace the bare `expect(true).toBe(true)` tautology in `playground-api-tab.test.tsx`'s SSE test and close the `check:test-masking` gap that let it slip through for a full cycle ([#6404](https://github.com/diegosouzapw/OmniRoute/issues/6404)) — a prior pass (#6548) had already swapped the literal to `expect(sendBtn).toBeDefined()`, but that stayed just as vacuous: the test's fetch mock returned an empty `/v1/models` list, so `ApiTab`'s Send button is always `disabled` (`!selectedModel`) and the SSE branch never runs — the "SSE infra is verified" comment was never true. The test now mocks a real model, drives the model `<select>` to enable Send, asserts `sendBtn.disabled === false` before clicking, and asserts the streamed SSE delta (`"Hello!"`) actually reached the response editor. Root cause on the detector side: `check-test-masking.mjs`'s tautology subcheck only compares base-vs-HEAD counts within a PR's own diff (`headExtTaut > baseExtTaut`) and no-ops locally when `GITHUB_BASE_SHA`/`GITHUB_BASE_REF` are unset ("sem base ref — pulando") — so a tautology merged once, or checked with a bare local run, was invisible forever after. Added a new always-on, PR-independent absolute-floor scan (`scanBareTautologies` + `countBareTautologies`) over every git-tracked test file for the bare `expect(true).toBe(true)` / `assert.equal(1,1)` / `assert.strictEqual(1,1)` patterns specifically (deliberately excluding `assert.ok(true)`, which has ~15 pre-existing verified-legitimate try/catch-fallback uses repo-wide and stays governed by the lenient diff-only subcheck) — verified zero pre-existing hits repo-wide once this file was fixed, so the new floor is safe to enforce unconditionally. Regression guard: `tests/unit/check-test-masking.test.ts` (new `scanBareTautologies`/`countBareTautologies` cases) + `tests/unit/ui/playground-api-tab.test.tsx`. (thanks @chirag127)
|
||||
- **fix(oauth):** Codex/ChatGPT (and every other OAuth provider) connection stays stuck showing "Auth Failed" even after a genuinely successful token refresh ([#6352](https://github.com/diegosouzapw/OmniRoute/issues/6352)) — `updateProviderCredentials()` (the shared `onPersist` callback for the manual "Refresh token" route, the reactive per-request refresh in `chat.ts`, and the Codex/Claude auth-file importers) correctly reused the stored `refresh_token`, persisted the new `access_token`, and replaced a rotated `refresh_token`, but never cleared the stale `testStatus`/`lastError*`/`errorCode` fields left over from a prior expired/invalid refresh or upstream 401/403 — only the separate background health-check sweep did that clearing. A successful refresh now resets `testStatus` to `"active"` and clears `lastError`, `lastErrorAt`, `lastErrorType`, `lastErrorSource`, and `errorCode` (an explicit `testStatus` from the caller still wins). Regression guard: `tests/unit/codex-oauth-refresh-persist-6352.test.ts`.
|
||||
- **perf(health):** short-TTL (1s) cache for the frequently-polled `GET /api/monitoring/health` payload — rebuilding it every request (DB reads + status aggregation across 8 subsystems) was wasteful under rapid polling; the cache stays near-real-time and is invalidated immediately on `DELETE` (circuit-breaker reset) so a manual reset is reflected at once. Regression guard: `tests/integration/monitoring-health-cache.test.ts`. ([#6553](https://github.com/diegosouzapw/OmniRoute/pull/6553)) — see PR. (thanks @developerjillur)
|
||||
|
|
|
|||
|
|
@ -67,6 +67,8 @@ import { getSessionConnection } from "./sessionManager.ts";
|
|||
import {
|
||||
applySessionStickiness,
|
||||
recordStickyBinding,
|
||||
clearStickyBinding,
|
||||
peekStickyConnectionId,
|
||||
resolveDisableSessionStickiness,
|
||||
} from "./combo/sessionStickiness.ts";
|
||||
import { selectQuotaShareTarget } from "./combo/quotaShareStrategy.ts";
|
||||
|
|
@ -204,6 +206,26 @@ export {
|
|||
validateComboDAG,
|
||||
} from "./combo/comboStructure.ts";
|
||||
|
||||
/**
|
||||
* #6692: release a session-stickiness pin the moment its bound connection is
|
||||
* the one that just failed. applySessionStickiness() only re-checks health on
|
||||
* the NEXT turn (lazily) — without this, a terminal/quality-rejected
|
||||
* connection stays pinned until that lazy recheck fires, and a masked
|
||||
* daily-cap 200-body rejection never trips the lazy recheck's DB-backed
|
||||
* testStatus gate at all (the connection row itself isn't marked unhealthy).
|
||||
* Exported for the two failure branches in handleComboChat + handleRoundRobinCombo.
|
||||
* peekStickyConnectionId guards against clearing an unrelated pin when the
|
||||
* failing target isn't actually the currently sticky-bound connection.
|
||||
*/
|
||||
export function releaseStickyPinOnFailure(
|
||||
messageHash: string | null | undefined,
|
||||
failedConnectionId: string | null | undefined
|
||||
): void {
|
||||
if (!messageHash || !failedConnectionId) return;
|
||||
if (peekStickyConnectionId(messageHash) !== failedConnectionId) return;
|
||||
clearStickyBinding(messageHash);
|
||||
}
|
||||
|
||||
const DEFAULT_MODEL_P95_MS: Record<string, number> = {
|
||||
"grok-4-fast-non-reasoning": 1143,
|
||||
"grok-4-1-fast-non-reasoning": 1244,
|
||||
|
|
@ -1669,6 +1691,10 @@ export async function handleComboChat({
|
|||
"COMBO",
|
||||
`Model ${modelStr} returned 200 but failed quality check: ${quality.reason}`
|
||||
);
|
||||
// #6692: a quality-rejected 200 never marks the connection row
|
||||
// unhealthy, so the sticky pin's lazy headroom recheck would never
|
||||
// catch it either — release it here, on the failing response.
|
||||
releaseStickyPinOnFailure(_sticky.messageHash, effectiveConnectionId);
|
||||
recordComboRequest(combo.name, modelStr, {
|
||||
success: false,
|
||||
latencyMs: Date.now() - startTime,
|
||||
|
|
@ -2027,6 +2053,10 @@ export async function handleComboChat({
|
|||
exhaustedLogLevel: "info",
|
||||
structuredError,
|
||||
});
|
||||
// #6692: this connection was just classified as provider/connection-level
|
||||
// exhausted — if it's the currently sticky-bound one, release the pin now
|
||||
// rather than waiting for the next turn's lazy headroom/status recheck.
|
||||
releaseStickyPinOnFailure(_sticky.messageHash, targetWithConnection.connectionId);
|
||||
|
||||
// #2101: Prevent infinite fallback loops with 400 Bad Request errors that are genuinely
|
||||
// body-specific (malformed JSON, bad format, missing required fields).
|
||||
|
|
@ -2759,6 +2789,19 @@ async function handleRoundRobinCombo({
|
|||
"COMBO-RR",
|
||||
`${modelStr} returned 200 but failed quality check: ${quality.reason}`
|
||||
);
|
||||
// #6692: same rationale as handleComboChat's quality-fail branch —
|
||||
// a quality-rejected 200 never marks the connection row unhealthy,
|
||||
// so release the sticky pin here rather than on the next turn.
|
||||
{
|
||||
const rrSelectedConnectionId =
|
||||
result.headers?.get("X-OmniRoute-Selected-Connection-Id") ||
|
||||
result.headers?.get("x-omniroute-selected-connection-id") ||
|
||||
undefined;
|
||||
releaseStickyPinOnFailure(
|
||||
_rrSessionSticky.messageHash,
|
||||
rrSelectedConnectionId || target.connectionId
|
||||
);
|
||||
}
|
||||
recordComboRequest(combo.name, modelStr, {
|
||||
success: false,
|
||||
latencyMs: Date.now() - startTime,
|
||||
|
|
@ -2985,6 +3028,8 @@ async function handleRoundRobinCombo({
|
|||
exhaustedLogLevel: "debug",
|
||||
structuredError,
|
||||
});
|
||||
// #6692: mirrors handleComboChat's exhaustion-point release above.
|
||||
releaseStickyPinOnFailure(_rrSessionSticky.messageHash, targetWithConnection.connectionId);
|
||||
|
||||
// Transient errors → mark in semaphore so round-robin stops stampeding this target.
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -27,6 +27,14 @@
|
|||
* orderTargetsByHeadroom (quotaStrategies.ts), so the open-sse leaf has no
|
||||
* static edge into src/lib/quota. For tests the fetcher is injected via
|
||||
* __setStickinessHeadroomFetcherForTests.
|
||||
* • Terminal-status gate (#6692): headroom alone is orthogonal to account
|
||||
* availability — a credits_exhausted/banned/expired connection (or one still
|
||||
* inside its rate-limit window) reports perfectly healthy 5h/weekly
|
||||
* utilization, so the headroom-only gate kept re-promoting a dead connection
|
||||
* forever. The connection's testStatus/rateLimitedUntil is now resolved via
|
||||
* the same dynamic-import-with-injectable-override seam (fail-open on lookup
|
||||
* errors, mirroring resolveSaturation) and gates the pin alongside headroom.
|
||||
* For tests the fetcher is injected via __setStickinessConnectionFetcherForTests.
|
||||
*
|
||||
* No barrel import — consistent with the other combo/* helpers.
|
||||
*
|
||||
|
|
@ -79,6 +87,83 @@ export function __setStickinessHeadroomFetcherForTests(fetcher: SaturationFetche
|
|||
_fetcherOverride = fetcher;
|
||||
}
|
||||
|
||||
// ─── Connection terminal-status gate (#6692) ─────────────────────────────────
|
||||
|
||||
/** Minimal connection health shape the terminal-status gate needs. */
|
||||
export interface StickyConnectionHealth {
|
||||
testStatus?: string | null;
|
||||
rateLimitedUntil?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Injectable connection-health fetcher seam (for unit tests).
|
||||
* Returns StickyConnectionHealth or undefined when unknown/lookup failed.
|
||||
*/
|
||||
export type ConnectionHealthFetcher = (
|
||||
connectionId: string,
|
||||
provider: string
|
||||
) => Promise<StickyConnectionHealth | undefined>;
|
||||
|
||||
/** Overrides the default connection-health fetcher for tests; null = use production fetcher. */
|
||||
let _connectionFetcherOverride: ConnectionHealthFetcher | null = null;
|
||||
|
||||
/** Test-only: inject the connection-health fetcher; pass null to restore default. */
|
||||
export function __setStickinessConnectionFetcherForTests(
|
||||
fetcher: ConnectionHealthFetcher | null
|
||||
): void {
|
||||
_connectionFetcherOverride = fetcher;
|
||||
}
|
||||
|
||||
/**
|
||||
* Statuses that mean the account is DURABLY dead, not just transiently rate
|
||||
* limited — mirrors TERMINAL_PIN_STATUSES used by the LKGP/context-cache pin
|
||||
* (combo.ts:558). Duplicated here (rather than imported) so this leaf keeps no
|
||||
* static edge into combo.ts, which itself imports this module.
|
||||
*/
|
||||
const TERMINAL_STICKY_STATUSES = new Set(["credits_exhausted", "banned", "expired"]);
|
||||
|
||||
/**
|
||||
* Resolve the sticky-bound connection's health by fetching its provider_connections
|
||||
* row. Uses the same dynamic-import pattern as resolveSaturation so this leaf has
|
||||
* no static dependency on src/lib/db. Fail-open (undefined) on any error.
|
||||
*/
|
||||
async function resolveConnectionHealth(
|
||||
connectionId: string,
|
||||
provider: string
|
||||
): Promise<StickyConnectionHealth | undefined> {
|
||||
if (_connectionFetcherOverride) return _connectionFetcherOverride(connectionId, provider);
|
||||
|
||||
try {
|
||||
const mod = await import("../../../src/lib/db/providers");
|
||||
const getProviderConnections = mod.getProviderConnections as (
|
||||
filter: Record<string, unknown>
|
||||
) => Promise<StickyConnectionHealth[]>;
|
||||
const connections = (await getProviderConnections({
|
||||
provider,
|
||||
isActive: true,
|
||||
})) as Array<StickyConnectionHealth & { id?: string }>;
|
||||
return connections.find((c) => c.id === connectionId);
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure: is the sticky-bound connection durably unhealthy right now? Fail-open
|
||||
* (false) when the connection is unknown — an unresolved lookup must never drop
|
||||
* a healthy pin.
|
||||
*/
|
||||
export function isStickyConnectionTerminallyUnhealthy(
|
||||
conn: StickyConnectionHealth | undefined,
|
||||
now: number
|
||||
): boolean {
|
||||
if (!conn) return false;
|
||||
const status = typeof conn.testStatus === "string" ? conn.testStatus : "";
|
||||
if (TERMINAL_STICKY_STATUSES.has(status)) return true;
|
||||
const rl = conn.rateLimitedUntil ? new Date(String(conn.rateLimitedUntil)).getTime() : 0;
|
||||
return Number.isFinite(rl) && rl > now;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the HeadroomSaturation for a connection by fetching both the 5h and
|
||||
* weekly utilisation signals. Uses the same dynamic-import pattern as
|
||||
|
|
@ -186,6 +271,17 @@ export function clearStickyBinding(messageHash: string): void {
|
|||
stickyMap.delete(messageHash);
|
||||
}
|
||||
|
||||
/**
|
||||
* Read-only peek at the connectionId currently bound to `messageHash`, without
|
||||
* mutating the store or checking TTL/health. Lets combo.ts's failure paths
|
||||
* confirm a just-failed target is the ACTUAL sticky-bound connection before
|
||||
* calling clearStickyBinding (#6692) — clearing on an unrelated target's
|
||||
* failure would drop a still-healthy pin.
|
||||
*/
|
||||
export function peekStickyConnectionId(messageHash: string): string | null {
|
||||
return stickyMap.get(messageHash)?.connectionId ?? null;
|
||||
}
|
||||
|
||||
/** Reset the entire store (for testing). */
|
||||
export function clearAllStickyBindings(): void {
|
||||
stickyMap.clear();
|
||||
|
|
@ -228,9 +324,10 @@ export interface ApplyStickinessResult {
|
|||
* Algorithm:
|
||||
* 1. Derive the message hash from the first user message.
|
||||
* 2. Look up the sticky binding for that hash.
|
||||
* 3. If found, fetch saturation for that connection.
|
||||
* 4. If headroom > threshold → move the matching target to index 0.
|
||||
* Otherwise → clear the binding (rebind on next success).
|
||||
* 3. If found, fetch saturation AND connection health for that connection.
|
||||
* 4. If headroom > threshold AND the connection is not durably unhealthy
|
||||
* (#6692: terminal testStatus / still rate-limited) → move the matching
|
||||
* target to index 0. Otherwise → clear the binding (rebind on next success).
|
||||
* 5. On any error → fall through unchanged (fail-open).
|
||||
*
|
||||
* In production the saturation fetcher is resolved via dynamic import of
|
||||
|
|
@ -272,13 +369,22 @@ export async function applySessionStickiness(
|
|||
return { targets: orderedTargets, messageHash, stuck: false };
|
||||
}
|
||||
|
||||
// Gate: headroom must be above threshold
|
||||
// Gate: headroom must be above threshold AND the connection must not be
|
||||
// durably unhealthy (#6692 — credits_exhausted/banned/expired/rate-limited
|
||||
// accounts report healthy 5h/weekly utilization, so headroom alone never
|
||||
// catches them).
|
||||
const stickyTarget = orderedTargets[stickyIdx];
|
||||
const sat = await resolveSaturation(connectionId, stickyTarget.provider);
|
||||
const [sat, connHealth] = await Promise.all([
|
||||
resolveSaturation(connectionId, stickyTarget.provider),
|
||||
resolveConnectionHealth(connectionId, stickyTarget.provider),
|
||||
]);
|
||||
const headroom = computeHeadroom(sat);
|
||||
|
||||
if (headroom <= STICKINESS_HEADROOM_THRESHOLD) {
|
||||
// Connection saturated — rebind on next success
|
||||
if (
|
||||
headroom <= STICKINESS_HEADROOM_THRESHOLD ||
|
||||
isStickyConnectionTerminallyUnhealthy(connHealth, Date.now())
|
||||
) {
|
||||
// Connection saturated or durably unhealthy — rebind on next success
|
||||
clearStickyBinding(messageHash);
|
||||
return { targets: orderedTargets, messageHash, stuck: false };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,12 +13,14 @@
|
|||
* - applySessionStickiness: no user message → normal ordering, no crash
|
||||
* - applySessionStickiness: different hashes → can map to different connections
|
||||
* - applySessionStickiness: saturation fetch error → fail-open
|
||||
* - recordStickyBinding / clearStickyBinding lifecycle
|
||||
* - applySessionStickiness: terminal connection status / rateLimitedUntil → rebind (#6692)
|
||||
* - recordStickyBinding / clearStickyBinding / peekStickyConnectionId lifecycle
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import type { HeadroomSaturation } from "../../open-sse/services/combo/headroomRanking.ts";
|
||||
import type { StickyConnectionHealth } from "../../open-sse/services/combo/sessionStickiness.ts";
|
||||
|
||||
const mod = await import("../../open-sse/services/combo/sessionStickiness.ts");
|
||||
const {
|
||||
|
|
@ -27,7 +29,9 @@ const {
|
|||
recordStickyBinding,
|
||||
clearStickyBinding,
|
||||
clearAllStickyBindings,
|
||||
peekStickyConnectionId,
|
||||
__setStickinessHeadroomFetcherForTests,
|
||||
__setStickinessConnectionFetcherForTests,
|
||||
STICKINESS_HEADROOM_THRESHOLD,
|
||||
} = mod;
|
||||
|
||||
|
|
@ -51,16 +55,26 @@ function injectSat(sat: HeadroomSaturation | undefined): void {
|
|||
__setStickinessHeadroomFetcherForTests(async (_id: string) => sat);
|
||||
}
|
||||
|
||||
function injectConnectionHealth(byId: Record<string, StickyConnectionHealth | undefined>): void {
|
||||
__setStickinessConnectionFetcherForTests(async (connectionId: string) => byId[connectionId]);
|
||||
}
|
||||
|
||||
// ─── Test lifecycle ───────────────────────────────────────────────────────────
|
||||
|
||||
test.beforeEach(() => {
|
||||
clearAllStickyBindings();
|
||||
// Default: healthy connection (headroom = 0.5)
|
||||
injectSat({ util5h: 0.3, util7d: 0.2 });
|
||||
// Default: unknown to the connection-health fetcher (fail-open, never terminal).
|
||||
// Without this override every test would hit the real dynamic-import → DB path
|
||||
// (resolveConnectionHealth's production branch), which is slow and non-deterministic
|
||||
// for a unit suite that otherwise makes zero DB calls.
|
||||
injectConnectionHealth({});
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
__setStickinessHeadroomFetcherForTests(null);
|
||||
__setStickinessConnectionFetcherForTests(null);
|
||||
});
|
||||
|
||||
// ─── deriveMessageHash ───────────────────────────────────────────────────────
|
||||
|
|
@ -320,3 +334,105 @@ test("messageHash is returned in result even when no binding exists", async () =
|
|||
assert.ok(r.messageHash !== null, "hash should be derivable and returned");
|
||||
assert.match(r.messageHash!, /^[a-f0-9]{16}$/);
|
||||
});
|
||||
|
||||
// ─── Terminal connection-status gate (#6692) ─────────────────────────────────
|
||||
//
|
||||
// Root cause: headroom (5h/weekly usage %) is orthogonal to account
|
||||
// availability — a credits_exhausted/banned/expired connection, or one still
|
||||
// inside its rateLimitedUntil cooldown, reports perfectly healthy headroom,
|
||||
// so the pre-fix headroom-only gate re-promoted a durably dead connection
|
||||
// forever. See tests/unit/repro-6692-sticky-terminal.test.ts for the full
|
||||
// end-to-end repro.
|
||||
|
||||
test("#6692: credits_exhausted sticky connection is rebound even with full headroom", async () => {
|
||||
injectSat({ util5h: 0.0, util7d: 0.0 }); // full headroom — would pass the old gate
|
||||
injectConnectionHealth({ "conn-A": { testStatus: "credits_exhausted" } });
|
||||
|
||||
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
|
||||
const messages = [{ role: "user", content: "Terminal status test" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
recordStickyBinding(hash, "conn-A");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
assert.equal(result.stuck, false, "credits_exhausted must release the pin");
|
||||
|
||||
// Binding must actually be cleared (not just skipped this call).
|
||||
assert.equal(peekStickyConnectionId(hash), null);
|
||||
});
|
||||
|
||||
test("#6692: banned / expired statuses also release the pin", async () => {
|
||||
for (const status of ["banned", "expired"]) {
|
||||
clearAllStickyBindings();
|
||||
injectSat({ util5h: 0.0, util7d: 0.0 });
|
||||
injectConnectionHealth({ "conn-A": { testStatus: status } });
|
||||
|
||||
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
|
||||
const messages = [{ role: "user", content: `Status ${status}` }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
recordStickyBinding(hash, "conn-A");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
assert.equal(result.stuck, false, `${status} must release the pin`);
|
||||
}
|
||||
});
|
||||
|
||||
test("#6692: connection still inside rateLimitedUntil window releases the pin", async () => {
|
||||
injectSat({ util5h: 0.0, util7d: 0.0 });
|
||||
const future = new Date(Date.now() + 60_000).toISOString();
|
||||
injectConnectionHealth({ "conn-A": { rateLimitedUntil: future } });
|
||||
|
||||
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
|
||||
const messages = [{ role: "user", content: "Cooling down" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
recordStickyBinding(hash, "conn-A");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
assert.equal(result.stuck, false, "an in-window rateLimitedUntil must release the pin");
|
||||
});
|
||||
|
||||
test("#6692: rateLimitedUntil in the past does NOT release the pin", async () => {
|
||||
injectSat({ util5h: 0.0, util7d: 0.0 });
|
||||
const past = new Date(Date.now() - 60_000).toISOString();
|
||||
injectConnectionHealth({ "conn-A": { rateLimitedUntil: past } });
|
||||
|
||||
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
|
||||
const messages = [{ role: "user", content: "Cooldown expired" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
recordStickyBinding(hash, "conn-A");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
assert.ok(result.stuck, "an expired rateLimitedUntil must not block reuse");
|
||||
assert.equal(result.targets[0].connectionId, "conn-A");
|
||||
});
|
||||
|
||||
test("#6692: connection-health fetch error → fail-open (pin preserved)", async () => {
|
||||
injectSat({ util5h: 0.0, util7d: 0.0 });
|
||||
__setStickinessConnectionFetcherForTests(async () => {
|
||||
throw new Error("db unavailable");
|
||||
});
|
||||
|
||||
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
|
||||
const messages = [{ role: "user", content: "Fetch error path" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
recordStickyBinding(hash, "conn-A");
|
||||
|
||||
// The production resolveConnectionHealth catches internally and returns
|
||||
// undefined (never throws) — mirrors resolveSaturation's own internal
|
||||
// try/catch. This test injects a THROWING override to prove
|
||||
// applySessionStickiness's outer try/catch also fails open end-to-end
|
||||
// (same behavior as the existing "saturation fetch error" case above:
|
||||
// total fail-open/no-op, never a crash).
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
assert.equal(result.stuck, false, "a connection-health fetch error must fail open, not crash");
|
||||
});
|
||||
|
||||
test("peekStickyConnectionId: reflects the current binding without mutating it", () => {
|
||||
const messages = [{ role: "user", content: "Peek test" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
|
||||
assert.equal(peekStickyConnectionId(hash), null, "no binding yet");
|
||||
recordStickyBinding(hash, "conn-peek");
|
||||
assert.equal(peekStickyConnectionId(hash), "conn-peek");
|
||||
// Peeking again must not clear or otherwise mutate the binding.
|
||||
assert.equal(peekStickyConnectionId(hash), "conn-peek");
|
||||
});
|
||||
|
|
|
|||
164
tests/unit/repro-6692-sticky-terminal.test.ts
Normal file
164
tests/unit/repro-6692-sticky-terminal.test.ts
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
/**
|
||||
* TDD repro probe for issue #6692 — "Combo session stickiness never releases a
|
||||
* credits-exhausted account (failover stops working)".
|
||||
*
|
||||
* Root cause (verified by code read): applySessionStickiness's ONLY eligibility
|
||||
* gate for keeping a sticky pin was a 5h/weekly usage-PERCENTAGE headroom check
|
||||
* (open-sse/services/combo/sessionStickiness.ts). It never inspected connection
|
||||
* testStatus / rateLimitedUntil / terminal state. clearStickyBinding()
|
||||
* (sessionStickiness.ts) had zero call sites anywhere in combo.ts's failure/
|
||||
* retry path (confirmed via repo-wide grep) — only the two internal branches
|
||||
* inside applySessionStickiness itself (connection missing from pool, or low
|
||||
* headroom) ever cleared a binding.
|
||||
*
|
||||
* This test reproduces the reported symptom using ONLY the real, exported
|
||||
* sessionStickiness.ts API, driven exactly the way combo.ts drives it in
|
||||
* production:
|
||||
* - recordStickyBinding() is called after a SUCCESSFUL turn (mirrors
|
||||
* combo.ts:1879 / combo.ts:2830).
|
||||
* - The bound connection's 5h/weekly utilization is healthy (fresh windows),
|
||||
* exactly as it would be for an account that is durably dead for a reason
|
||||
* ORTHOGONAL to 5h/weekly utilization (credits_exhausted / banned / a
|
||||
* per-day soft cap — reporter #2's exact scenario).
|
||||
* - The connection-health fetcher (the fix's new injectable seam, mirroring
|
||||
* __setStickinessHeadroomFetcherForTests) reports the bound connection as
|
||||
* credits_exhausted — exactly what a real provider_connections row would
|
||||
* report once markAccountUnavailable() has classified the terminal error.
|
||||
*
|
||||
* Expected (correct) behavior: once a sticky-bound connection is known
|
||||
* terminally unhealthy, the pin must release and failover must move to the
|
||||
* next healthy target.
|
||||
*
|
||||
* Actual (BEFORE the fix): applySessionStickiness re-promoted the dead
|
||||
* connection anyway, because headroom was the only gate it consulted — this
|
||||
* test failed RED on the pre-fix code (result.stuck === true, dead connection
|
||||
* stayed at position 0).
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import type { HeadroomSaturation } from "../../open-sse/services/combo/headroomRanking.ts";
|
||||
import type { StickyConnectionHealth } from "../../open-sse/services/combo/sessionStickiness.ts";
|
||||
|
||||
const mod = await import("../../open-sse/services/combo/sessionStickiness.ts");
|
||||
const {
|
||||
deriveMessageHash,
|
||||
applySessionStickiness,
|
||||
recordStickyBinding,
|
||||
clearAllStickyBindings,
|
||||
__setStickinessHeadroomFetcherForTests,
|
||||
__setStickinessConnectionFetcherForTests,
|
||||
} = mod;
|
||||
|
||||
function makeTarget(
|
||||
connectionId: string
|
||||
): import("../../open-sse/services/combo/types.ts").ResolvedComboTarget {
|
||||
return {
|
||||
kind: "model",
|
||||
stepId: `step-${connectionId}`,
|
||||
executionKey: `key-${connectionId}`,
|
||||
modelStr: `gpt-4/${connectionId}`,
|
||||
provider: "openai",
|
||||
providerId: null,
|
||||
connectionId,
|
||||
weight: 1,
|
||||
label: null,
|
||||
};
|
||||
}
|
||||
|
||||
function injectSat(sat: HeadroomSaturation | undefined): void {
|
||||
__setStickinessHeadroomFetcherForTests(async (_id: string) => sat);
|
||||
}
|
||||
|
||||
function injectConnectionHealth(byId: Record<string, StickyConnectionHealth | undefined>): void {
|
||||
__setStickinessConnectionFetcherForTests(async (connectionId: string) => byId[connectionId]);
|
||||
}
|
||||
|
||||
test.beforeEach(() => {
|
||||
clearAllStickyBindings();
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
__setStickinessHeadroomFetcherForTests(null);
|
||||
__setStickinessConnectionFetcherForTests(null);
|
||||
});
|
||||
|
||||
test("BUG #6692: sticky pin survives a terminal (credits_exhausted) account — headroom-only gate keeps re-promoting the dead connection instead of failing over", async () => {
|
||||
// Healthy 5h/weekly utilization (fresh windows) — exactly what a
|
||||
// credits_exhausted / banned / daily-soft-capped account reports on THESE
|
||||
// axes, since the terminal state is orthogonal to 5h/weekly percentage
|
||||
// utilization (issue #6692, both reporters).
|
||||
injectSat({ util5h: 0.05, util7d: 0.05 }); // headroom = 0.95, well above threshold
|
||||
|
||||
// The bound connection is durably dead (credits_exhausted); the fallback
|
||||
// connection is unknown to the health fetcher (fail-open → healthy).
|
||||
injectConnectionHealth({ "conn-exhausted": { testStatus: "credits_exhausted" } });
|
||||
|
||||
// Upstream strategy ordering already ranks conn-healthy first (a terminally
|
||||
// dead account naturally scores worse on the normal ordering signals too) —
|
||||
// conn-exhausted is only ever tried first because the sticky pin FORCE-
|
||||
// PROMOTES it. That force-promotion, not the base ordering, is the bug.
|
||||
const targets = [makeTarget("conn-healthy"), makeTarget("conn-exhausted")];
|
||||
const messages = [{ role: "user", content: "Multi-turn conversation, turn 1" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
assert.ok(hash, "hash must be derivable");
|
||||
|
||||
// Turn 1 (production: combo.ts success branch, e.g. combo.ts:1879):
|
||||
// conn-exhausted served the first turn successfully → sticky-bound.
|
||||
recordStickyBinding(hash, "conn-exhausted");
|
||||
|
||||
// Turn 2 (production): conn-exhausted now returns credits_exhausted /
|
||||
// banned / a masked daily-cap refusal — the provider_connections row is
|
||||
// updated by markAccountUnavailable() with testStatus=credits_exhausted,
|
||||
// which the injected connection-health fetcher above simulates.
|
||||
|
||||
// Turn 3 (production): the next request in the same conversation calls
|
||||
// applySessionStickiness() again. Desired behavior: the terminally dead
|
||||
// connection must NOT be re-promoted over the naturally-better ordering.
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
|
||||
assert.equal(
|
||||
result.stuck,
|
||||
false,
|
||||
"sticky pin must release once the bound connection is terminally exhausted, " +
|
||||
"not only when 5h/weekly headroom is low (issue #6692)"
|
||||
);
|
||||
assert.equal(
|
||||
result.targets[0].connectionId,
|
||||
"conn-healthy",
|
||||
"the terminally dead sticky pin must NOT be force-promoted over the naturally-ordered healthy target"
|
||||
);
|
||||
});
|
||||
|
||||
test("BUG #6692 (rate-limited variant): sticky pin releases while the bound connection is still inside its rateLimitedUntil window", async () => {
|
||||
injectSat({ util5h: 0.05, util7d: 0.05 });
|
||||
const future = new Date(Date.now() + 60_000).toISOString();
|
||||
injectConnectionHealth({ "conn-cooling": { rateLimitedUntil: future } });
|
||||
|
||||
const targets = [makeTarget("conn-healthy"), makeTarget("conn-cooling")];
|
||||
const messages = [{ role: "user", content: "Another conversation" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
|
||||
recordStickyBinding(hash, "conn-cooling");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
|
||||
assert.equal(result.stuck, false, "pin must release while the connection is still cooling down");
|
||||
assert.equal(result.targets[0].connectionId, "conn-healthy");
|
||||
});
|
||||
|
||||
test("healthy sticky connection (unknown to the connection-health fetcher) still stays pinned — fail-open preserved", async () => {
|
||||
injectSat({ util5h: 0.05, util7d: 0.05 });
|
||||
injectConnectionHealth({}); // unknown connection → fail-open → not terminal
|
||||
|
||||
const targets = [makeTarget("conn-a"), makeTarget("conn-b")];
|
||||
const messages = [{ role: "user", content: "Healthy conversation" }];
|
||||
const hash = deriveMessageHash(messages)!;
|
||||
|
||||
recordStickyBinding(hash, "conn-a");
|
||||
|
||||
const result = await applySessionStickiness(targets, messages);
|
||||
|
||||
assert.equal(result.stuck, true, "a healthy/unknown connection must remain pinned");
|
||||
assert.equal(result.targets[0].connectionId, "conn-a");
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue