From 314b6f1870640fd7a1f01d8ca239785bf4a7a9f7 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:20:28 -0300 Subject: [PATCH] fix(resilience): respect connection cooldown stored as numeric epoch (#3954) (#3995) rate_limited_until is a TEXT column, but setConnectionRateLimitUntil (Antigravity full-quota path) persists a raw epoch number that SQLite coerces to a numeric string ("1781696905131.0"). The selection predicate isAccountUnavailable then did new Date("1781696905131.0") -> NaN, so the cooling connection was never skipped and the router kept dispatching to rate-limited accounts. Normalize numeric-epoch strings (and number/Date/ISO) via a shared cooldownUntilMs() helper in isAccountUnavailable / getEarliestRateLimitedUntil / filterAvailableAccounts / parseFutureDateMs. ISO behavior preserved. --- CHANGELOG.md | 1 + config/quality/file-size-baseline.json | 5 +- open-sse/services/accountFallback.ts | 29 ++++- src/sse/services/auth.ts | 5 +- tests/unit/cooldown-epoch-string-3954.test.ts | 112 ++++++++++++++++++ 5 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 tests/unit/cooldown-epoch-string-3954.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bbf15f386a..bdb026d84b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ ### πŸ› Fixed +- **fix(resilience): respect connection cooldown stored as a numeric epoch (router kept hammering 429 accounts)** β€” the router kept dispatching to connections still inside their rate-limit cooldown, causing client timeouts and "connection cooldown isn't respected" reports. Root cause: `rate_limited_until` is a `TEXT` column, but the Antigravity full-quota path (`setConnectionRateLimitUntil`) persists a raw epoch **number**, which SQLite coerces to a numeric string like `"1781696905131.0"`. The account-selection predicate then did `new Date("1781696905131.0")` β†’ `Invalid Date` β†’ `NaN`, so `NaN > Date.now()` was false and the cooling connection was never skipped. The cooldown read predicates (`isAccountUnavailable`, `getEarliestRateLimitedUntil`, `filterAvailableAccounts`, `parseFutureDateMs`) now normalize numeric-epoch strings as well as ISO strings/Date/number via a shared `cooldownUntilMs()` helper β€” ISO behavior is unchanged. ([#3954](https://github.com/diegosouzapw/OmniRoute/issues/3954)) - **fix(compression/memory): stop memory + compression from poisoning the upstream prompt cache** β€” with compression and/or memory enabled, requests to caching providers (Anthropic-family) missed the prompt cache on every turn, multiplying cost. Two root causes: (1) memory injection prepended the retrieved memories β€” which **vary per user query** β€” at index 0 of the message array, shifting the entire cacheable prefix every turn; memory is now inserted just before the last user message when the request carries `cache_control` breakpoints, keeping the cacheable prefix (system prompt + prior turns) byte-stable. (2) the cache-aware `skipSystemPrompt` flag computed by `getCacheAwareStrategy()` was dropped by `selectCompressionStrategy()` (which can only return a mode), so the system prompt could still be compressed under caching; a new `resolveCacheAwareConfig()` now forces `preserveSystemPrompt` on for caching requests. ([#3936](https://github.com/diegosouzapw/OmniRoute/pull/3936), closes [#3890](https://github.com/diegosouzapw/OmniRoute/issues/3890) β€” thanks @xenstar / @diegosouzapw) - **fix(providers): register BytePlus ModelArk so its API key can be added** β€” adding a BytePlus (`ark-…`) key reported "invalid". `byteplus` was present in the provider catalog (`APIKEY_PROVIDERS`) but **never registered in the routing registry**, so key validation fell through to `{ unsupported: true }` β†’ HTTP 400 β†’ the UI rendered every key as invalid (and the provider was unusable for inference). Added a registry entry modeled on the existing Volcengine Ark provider: OpenAI-compatible format, base `https://ark.ap-southeast.bytepluses.com/api/v3` (region `ap-southeast-1`), `Authorization: Bearer` auth, seeded with the catalog's advertised models (Seed 2.0, Kimi K2 Thinking, GLM 4.7, GPT-OSS-120B). ([#3935](https://github.com/diegosouzapw/OmniRoute/pull/3935), closes [#3877](https://github.com/diegosouzapw/OmniRoute/issues/3877) β€” thanks @nikohd12 / @diegosouzapw) - **fix(providers): Nous Research key validation no longer fails on a stale probe model** β€” adding a valid Nous Research API key reported "invalid" even though the same key worked via the portal's copy-shell `curl`. The validation probe sent `model: "nousresearch/hermes-4-70b"`, which Nous does not serve, so the API returned `400` and the validator (which only treated `200`/`429` as success) reported the key invalid. The probe now uses the real `Hermes-4-70B` slug, and any non-auth 4xx (`400`/`404`/`422`) is treated as a valid key (the request shape was wrong, not the credentials) β€” mirroring the longcat/nvidia validators so a future model rename can't re-break key validation. ([#3934](https://github.com/diegosouzapw/OmniRoute/pull/3934), closes [#3881](https://github.com/diegosouzapw/OmniRoute/issues/3881) β€” thanks @FerLuisxd / @diegosouzapw) diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 5054b664f1..c6dcfe61c2 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -15,6 +15,7 @@ "_rebaseline_2026_06_15_3929_vertex_media": "PR #3929 own growth: audioSpeech.ts 952->965 (+13) and videoGeneration.ts 1026->1078 (+52) = vertex/* media branches (Gemini TTS, Veo predictLongRunning poll) wired into the speech/video handlers; new logic lives in open-sse/executors/vertexMedia.ts (341, under cap). Cohesive media-provider feature.", "_rebaseline_2026_06_15_3879_redact_thinking": "PR #3879 + #3921 reconcile: AddApiKeyModal.tsx 843->845 (+2 = merging #3879's CcCompatibleRequestDefaultsFields (context1m + opt-in redact-thinking toggle) into #3921's preset-input block in the cc-compatible settings group). Cohesive UI; not extractable.", "_rebaseline_2026_06_15_3890_cache_preserve": "Issue #3890 own growth: chatCore.ts 5815->5823 (+8 = wire resolveCacheAwareConfig() into the compression apply step so the system prompt is never compressed in a caching context β€” honors the cache-aware skipSystemPrompt flag that selectCompressionStrategy could not carry). Cohesive cache-preservation guard at the existing compression chokepoint; not extractable.", + "_rebaseline_2026_06_16_3954_cooldown_epoch": "Issue #3954 own growth: accountFallback.ts 1708->1727 (+19 = a shared cooldownUntilMs() normalizer + its use in isAccountUnavailable/getEarliestRateLimitedUntil/filterAvailableAccounts so a rate_limited_until persisted as a numeric-epoch string is honored, not parsed to NaN) and auth.ts 2216->2219 (+3 = parseFutureDateMs reuses cooldownUntilMs). Cohesive cooldown read-path hardening at the existing chokepoints; one helper, not extractable.", "_rebaseline_2026_06_15_3938_perplexity_v218": "PR #3938 own growth: perplexity-web.ts 868->939 (+71 = rebuild buildPplxRequestBody to mirror the current www.perplexity.ai schematized request body β€” version 2.18, use_schematized_api + the full supported_block_use_cases list, dsl_query, shared requestId for frontend_uuid/client_search_results_cache_key, last_backend_uuid only on follow-ups β€” plus the x-perplexity-request-* / x-request-id headers replacing the stale X-App-ApiVersion pair that triggered HTTP 400). Cohesive upstream-schema sync in a single executor; not extractable.", "cap": 800, "frozen": { @@ -40,7 +41,7 @@ "open-sse/mcp-server/schemas/tools.ts": 1437, "open-sse/mcp-server/server.ts": 1457, "open-sse/mcp-server/tools/advancedTools.ts": 1118, - "open-sse/services/accountFallback.ts": 1708, + "open-sse/services/accountFallback.ts": 1727, "open-sse/services/batchProcessor.ts": 828, "open-sse/services/browserBackedChat.ts": 850, "open-sse/services/claudeCodeCompatible.ts": 1202, @@ -119,7 +120,7 @@ "src/shared/services/cliRuntime.ts": 1090, "src/shared/validation/schemas.ts": 2523, "src/sse/handlers/chat.ts": 1425, - "src/sse/services/auth.ts": 2216 + "src/sse/services/auth.ts": 2219 }, "_rebaseline_2026_06_09": "Re-baseline consciente pre-release v3.8.19: 9 arquivos cresceram durante o ciclo (features mergeadas: RequestLoggerV2 +281 request-logger rework, stream +101, combo +73, chatCore +45, catalog +32 fable-5/catalog-flag, callLogs +4, accountFallback +2, usageHistory novo 840) + core.ts +7 (fix resetAllDbModuleState, PR 3536). A catraca segue valendo destes valores β€” proximo crescimento falha. Decisao: encolher (esp. RequestLoggerV2/chatCore) e a issue #3501 ficam para o ciclo seguinte.", "_rebaseline_2026_06_11_phase1f": "Phase 1f (#3501): ProviderDetailPageClient.tsx 4948β†’4062 (-886 LOC); 3 novos hooks extraΓ­dos. useProviderConnections.ts=954 acima do cap=800 β€” justificado: extraΓ§Γ£o direta do god-component (zero lΓ³gica nova), prΓ³pria reduΓ§Γ£o do cliente supera o custo. useProviderSettings.ts=263 e useProviderModels.ts=154 jΓ‘ abaixo do cap.", diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index 729ee8b433..01f73c22ec 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -1576,12 +1576,31 @@ export function checkFallbackError( // ─── Account State Management ─────────────────────────────────────────────── +/** + * Normalize a stored cooldown timestamp to epoch milliseconds. + * + * `rate_limited_until` is a TEXT column, but some write paths persist a raw + * epoch NUMBER (e.g. `setConnectionRateLimitUntil` on the Antigravity full-quota + * path). SQLite TEXT affinity coerces it to a numeric string like + * "1781696905131.0", which `new Date(...)` cannot parse (β†’ NaN). Accept numeric + * epoch strings/numbers as well as ISO strings and Date objects (#3954). + */ +export function cooldownUntilMs(value: string | number | Date | null | undefined): number { + if (value === null || value === undefined || value === "") return NaN; + if (value instanceof Date) return value.getTime(); + if (typeof value === "number") return value; + const raw = value.trim(); + if (/^\d+(\.\d+)?$/.test(raw)) return Number(raw); + return new Date(raw).getTime(); +} + /** * Check if account is currently unavailable (cooldown not expired) */ export function isAccountUnavailable(unavailableUntil: string | Date | null | undefined): boolean { if (!unavailableUntil) return false; - return new Date(unavailableUntil).getTime() > Date.now(); + const ms = cooldownUntilMs(unavailableUntil); + return Number.isFinite(ms) && ms > Date.now(); } /** @@ -1601,8 +1620,8 @@ export function getEarliestRateLimitedUntil( const now = Date.now(); for (const acc of accounts) { if (!acc.rateLimitedUntil) continue; - const until = new Date(acc.rateLimitedUntil).getTime(); - if (until <= now) continue; + const until = cooldownUntilMs(acc.rateLimitedUntil); + if (!Number.isFinite(until) || until <= now) continue; if (!earliest || until < earliest) earliest = until; } if (!earliest) return null; @@ -1640,8 +1659,8 @@ export function filterAvailableAccounts( return accounts.filter((acc) => { if (excludeId && acc.id === excludeId) return false; if (acc.rateLimitedUntil) { - const until = new Date(acc.rateLimitedUntil).getTime(); - if (until > now) return false; + const until = cooldownUntilMs(acc.rateLimitedUntil); + if (Number.isFinite(until) && until > now) return false; } return true; }); diff --git a/src/sse/services/auth.ts b/src/sse/services/auth.ts index fad736f8de..740ba0c1f8 100644 --- a/src/sse/services/auth.ts +++ b/src/sse/services/auth.ts @@ -21,6 +21,7 @@ import { isAccountUnavailable, getUnavailableUntil, getEarliestRateLimitedUntil, + cooldownUntilMs, formatRetryAfter, checkFallbackError, isModelLocked, @@ -496,7 +497,9 @@ export function evaluateQuotaLimitPolicy( function parseFutureDateMs(value: string | null): number | null { if (!value) return null; - const ms = new Date(value).getTime(); + // Tolerate numeric-epoch strings (e.g. "1781696905131.0") as well as ISO + // strings β€” the rate_limited_until TEXT column can hold either (#3954). + const ms = cooldownUntilMs(value); if (!Number.isFinite(ms) || ms <= Date.now()) return null; return ms; } diff --git a/tests/unit/cooldown-epoch-string-3954.test.ts b/tests/unit/cooldown-epoch-string-3954.test.ts new file mode 100644 index 0000000000..ddc3af23c7 --- /dev/null +++ b/tests/unit/cooldown-epoch-string-3954.test.ts @@ -0,0 +1,112 @@ +/** + * TDD regression for #3954: the router keeps selecting rate-limited (429) + * accounts because the connection cooldown is not respected. + * + * Root cause: `rate_limited_until` is a TEXT column, but the Antigravity + * full-quota path (`setConnectionRateLimitUntil`) writes a raw epoch NUMBER. + * SQLite TEXT affinity coerces it to a numeric string like "1781696905131.0". + * The selection filter `isAccountUnavailable` then does + * `new Date("1781696905131.0")` β†’ Invalid Date β†’ NaN, so `NaN > Date.now()` + * is false and the still-cooling connection is NOT skipped β†’ client timeouts. + * + * The fix hardens the cooldown read predicates to tolerate numeric-epoch + * strings (in addition to ISO strings / Date / number), without changing the + * write path. ISO behavior is preserved (regression guard). + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-3954-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const { isAccountUnavailable, getEarliestRateLimitedUntil, filterAvailableAccounts } = await import( + "../../open-sse/services/accountFallback.ts" +); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +const HOUR = 3_600_000; + +// ── Pure predicate: the exact chokepoint account selection uses ────────────── + +test("#3954 isAccountUnavailable: numeric-epoch string (future) is treated as unavailable", () => { + assert.equal(isAccountUnavailable(String(Date.now() + HOUR)), true); +}); + +test("#3954 isAccountUnavailable: SQLite REALβ†’TEXT '.0' epoch string (future) is unavailable", () => { + assert.equal(isAccountUnavailable(`${Date.now() + HOUR}.0`), true); +}); + +test("#3954 isAccountUnavailable: numeric-epoch string in the past is available again", () => { + assert.equal(isAccountUnavailable(String(Date.now() - HOUR)), false); + assert.equal(isAccountUnavailable(`${Date.now() - HOUR}.0`), false); +}); + +test("#3954 isAccountUnavailable: ISO strings still work (no regression)", () => { + assert.equal(isAccountUnavailable(new Date(Date.now() + HOUR).toISOString()), true); + assert.equal(isAccountUnavailable(new Date(Date.now() - HOUR).toISOString()), false); +}); + +test("#3954 isAccountUnavailable: empty/null/Date inputs behave", () => { + assert.equal(isAccountUnavailable(null), false); + assert.equal(isAccountUnavailable(undefined), false); + assert.equal(isAccountUnavailable(""), false); + assert.equal(isAccountUnavailable(new Date(Date.now() + HOUR)), true); +}); + +test("#3954 getEarliestRateLimitedUntil: honors numeric-epoch-string cooldowns", () => { + const soon = Date.now() + 30_000; + const earliest = getEarliestRateLimitedUntil([ + { rateLimitedUntil: String(Date.now() + 90_000) }, + { rateLimitedUntil: String(soon) }, + ]); + assert.equal(earliest, new Date(soon).toISOString()); +}); + +test("#3954 filterAvailableAccounts: excludes a numeric-epoch-string future cooldown", () => { + const accts = [ + { id: "cooling", rateLimitedUntil: String(Date.now() + HOUR) }, + { id: "healthy", rateLimitedUntil: null }, + ]; + const available = filterAvailableAccounts(accts as never); + assert.deepEqual( + available.map((a) => a.id), + ["healthy"] + ); +}); + +// ── End-to-end: the write coercion that triggers the real bug ─────────────── + +test("#3954 setConnectionRateLimitUntil stores a numeric string that selection still honors", async () => { + const conn = await providersDb.createProviderConnection({ + provider: "antigravity", + authType: "oauth", + name: "AG 3954", + }); + const connId = (conn as { id: string }).id; + providersDb.setConnectionRateLimitUntil(connId, Date.now() + HOUR); + + const db = core.getDbInstance() as unknown as { + prepare: (sql: string) => { get: (id: string) => { rate_limited_until: unknown } | undefined }; + }; + const row = db + .prepare("SELECT rate_limited_until FROM provider_connections WHERE id = ?") + .get(connId); + const stored = row?.rate_limited_until; + + // It is persisted in the numeric-string form (NOT ISO) β€” this is the trap. + assert.ok( + /^\d+(\.\d+)?$/.test(String(stored)), + `expected numeric epoch string, got ${String(stored)}` + ); + // The selection filter must still treat this connection as unavailable. + assert.equal(isAccountUnavailable(String(stored)), true); +});