From f570960958fbb0e0ae6f7669ad82c00f2e28a19c Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:08:37 -0300 Subject: [PATCH 01/35] fix(oauth): persist and reuse rotated Codex OAuth refresh token (#6352) (#6602) --- CHANGELOG.md | 1 + src/sse/services/tokenRefresh.ts | 18 ++ .../codex-oauth-refresh-persist-6352.test.ts | 204 ++++++++++++++++++ 3 files changed, 223 insertions(+) create mode 100644 tests/unit/codex-oauth-refresh-persist-6352.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c56f7a43b2..c44b96b38e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ _Living section β€” bullets land here as PRs merge into `release/v3.8.47` (paral ### πŸ› Bug Fixes +- **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`. - **fix(resilience):** `headroom` combo routing did not always select the Codex account with the most free quota ([#6379](https://github.com/diegosouzapw/OmniRoute/issues/6379)) β€” `orderTargetsByHeadroom` (`open-sse/services/combo/quotaStrategies.ts`) already loaded the per-connection DB snapshot (with decrypted credentials) via `expandTargetsByQuotaAwareConnections`, but discarded it before calling `getSaturation`; for Codex, `fetchCodexSaturation` forwards straight to `fetchCodexQuota(connectionId, connection)`, which needs `connection` (or a prior `registerCodexConnection()` call, which never happens before headroom ranking runs) to read `accessToken` β€” so it returned `null` for every candidate, saturation failed open to `0` across the board, and ranking fell back to the original combo order regardless of actual free quota. `getSaturation()` and the headroom `SaturationFetcher` seam now accept and thread the loaded connection snapshot through to `fetchCodexQuota`. Kilo's dup flag vs #5903 was a false positive β€” that issue is about session-sticky reset-aware/least-used selection, not headroom's Codex saturation lookup. Regression guard: `tests/unit/headroom-codex-quota-snapshot-6379.test.ts`. (thanks @eidoog) - **fix(providers):** custom models a provider actually has are no longer dropped from the Free Provider Rankings when both **"Configured only"** and **"Available only"** filters are applied ([#6368](https://github.com/diegosouzapw/OmniRoute/issues/6368)), follow-up to #6150 β€” `freeProviderRankings.ts::getProviderModels()` only ever walked the static `open-sse/config/providerRegistry.ts` catalog, so a user-added custom model (e.g. a Puter `claude-fable-5` model saved as "Claude Fable 5") never entered the candidate model list the ranking scores against, and could never survive the #6150 configured/available filters even when actually configured and available. It now additively merges the provider's custom models (`db/models.ts::getCustomModels`) into that candidate list via a new pure, de-duping `mergeProviderModels()` helper, before scoring/filtering runs β€” catalog free/paid filtering elsewhere is untouched. Regression guard: `tests/unit/free-provider-rankings-custom-models-6368.test.ts`. (thanks @shabeer) - **fix(providers):** `cloudflare-ai` no longer silently drops image/non-text content parts ([#6390](https://github.com/diegosouzapw/OmniRoute/issues/6390)) β€” `transformRequest()`'s `flattenContent()` (added for #2539 to satisfy the Workers AI `/ai/v1/chat/completions` plain-string `content` requirement) mapped any non-text OpenAI content part (e.g. `image_url`) to `""` and joined the rest, so a request carrying an image quietly went out as text-only with the attachment gone and no error surfaced. It now throws a clear error on the first non-text part instead of dropping it silently, which the existing top-level `chatCore.ts` catch already routes through `buildErrorBody()`/`sanitizeErrorMessage()` (same pattern as `buildUrl()`'s missing-Account-ID error). Regression guard: `tests/unit/cloudflare-ai-image-parts-6390.test.ts`. diff --git a/src/sse/services/tokenRefresh.ts b/src/sse/services/tokenRefresh.ts index ab51be2742..05585ae301 100755 --- a/src/sse/services/tokenRefresh.ts +++ b/src/sse/services/tokenRefresh.ts @@ -119,6 +119,24 @@ export async function updateProviderCredentials(connectionId: string, newCredent if (newCredentials.accessToken) { updates.accessToken = newCredentials.accessToken; + // #6352: a successful refresh proves the connection is reachable and its + // refresh_token is valid again β€” clear any stale auth-failure state + // (testStatus/lastError*) left over from a prior expired/invalid refresh + // token or an upstream 401/403. Without this, a genuinely successful + // rotating-refresh (e.g. Codex/OpenAI) persisted the new access/refresh + // token while leaving the dashboard showing "Auth Failed" forever, + // because the error metadata was never reset here β€” only the health-check + // sweep (tokenHealthCheck.ts::checkConnection) did this clearing, so any + // OTHER caller of updateProviderCredentials (the manual refresh route, + // the reactive per-request refresh in chat.ts) looked like it "didn't + // pick up" the refreshed token. Explicit `newCredentials.testStatus` + // below still wins for callers that need a specific terminal state. + updates.testStatus = "active"; + updates.lastError = null; + updates.lastErrorAt = null; + updates.lastErrorType = null; + updates.lastErrorSource = null; + updates.errorCode = null; } if (newCredentials.refreshToken) { updates.refreshToken = newCredentials.refreshToken; diff --git a/tests/unit/codex-oauth-refresh-persist-6352.test.ts b/tests/unit/codex-oauth-refresh-persist-6352.test.ts new file mode 100644 index 0000000000..12f23cd67d --- /dev/null +++ b/tests/unit/codex-oauth-refresh-persist-6352.test.ts @@ -0,0 +1,204 @@ +/** + * #6352 β€” Codex/ChatGPT OAuth refresh: reuse + persist + rotate + clear stale + * "auth failed" state. + * + * Reported symptom: a ChatGPT Plus account added via Codex OAuth stops working + * after ~2 days and the dashboard's manual "Refresh token" button is reported + * as "not enough" β€” after clicking it the connection still shows `auth failed`. + * + * Root cause traced in this PR: `updateProviderCredentials()` + * (src/sse/services/tokenRefresh.ts) is the shared onPersist callback for every + * refresh entry point (the manual refresh route, the reactive per-request + * refresh in chat.ts's `checkAndRefreshToken`, the Codex auth-file importer). + * It correctly persists the new accessToken/refreshToken/expiresAt β€” but it + * NEVER cleared the stale auth-failure metadata (`testStatus`, `lastError`, + * `lastErrorType`, `lastErrorSource`, `errorCode`) left over from a prior + * expired/invalid refresh or upstream 401/403. Only the separate background + * health-check sweep (tokenHealthCheck.ts::checkConnection) did this clearing + * inline in its own onPersist callback. So a refresh that ACTUALLY succeeded + * β€” reusing the stored refresh_token, obtaining a fresh access_token, and even + * rotating in a new refresh_token β€” still left the connection displaying + * "Auth Failed" forever, because nothing ever reset the error columns. + * + * This test drives `checkAndRefreshToken("codex", ...)` β€” the exact function + * the real per-request refresh path (src/sse/handlers/chat.ts) calls β€” against + * a connection pre-seeded in a stale "auth failed" state, with a mocked Codex + * token endpoint. It asserts: + * (a) the refresh REUSES the stored refresh_token (request body assertion), + * (b) the refreshed access_token is PERSISTED back to the connection row, + * (c) a ROTATED refresh_token REPLACES the previously stored one, + * (d) the stale testStatus/lastError* auth-failure fields are CLEARED so the + * dashboard stops showing "Auth Failed" after a refresh that worked. + * + * (d) is the part that reproduces the reported bug: before the fix in + * src/sse/services/tokenRefresh.ts, this assertion fails (RED) because + * updateProviderCredentials left testStatus/lastError untouched. + */ +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-codex-refresh-6352-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-api-key-secret-6352"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const tokenRefresh = await import("../../src/sse/services/tokenRefresh.ts"); +const { OAUTH_ENDPOINTS } = await import("../../open-sse/config/constants.ts"); + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +function jsonResponse(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +type ConnectionRecord = { + id: string; + accessToken?: string | null; + refreshToken?: string | null; + testStatus?: string | null; + lastError?: string | null; + lastErrorType?: string | null; + lastErrorSource?: string | null; + errorCode?: string | null; +}; + +type FetchOptions = { body?: string }; + +async function withMockedFetch( + fetchImpl: (url: unknown, options?: FetchOptions) => Promise, + fn: () => Promise +): Promise { + const originalFetch = globalThis.fetch; + globalThis.fetch = fetchImpl as typeof fetch; + try { + return await fn(); + } finally { + globalThis.fetch = originalFetch; + } +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + await resetStorage(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("checkAndRefreshToken reuses the stored Codex refresh_token, persists the new access_token, rotates the refresh_token, and clears stale auth-failure state (#6352)", async () => { + const now = Date.now(); + + // Seed a connection in the exact "auth failed" state the issue describes: + // a prior refresh/request failure left testStatus/lastError* populated. + const connection = await providersDb.createProviderConnection({ + provider: "codex", + authType: "oauth", + name: "ChatGPT Plus (Codex OAuth)", + accessToken: "codex-stale-access", + refreshToken: "codex-stored-refresh-token", + // Already past the 5-minute Codex refresh lead β†’ checkAndRefreshToken refreshes. + expiresAt: new Date(now - 60_000).toISOString(), + testStatus: "invalid", + lastError: "Refresh token expired. Please re-authenticate this account.", + lastErrorAt: new Date(now - 120_000).toISOString(), + lastErrorType: "upstream_auth_error", + lastErrorSource: "oauth", + errorCode: "401", + } as unknown as Record); + const connectionId = (connection as ConnectionRecord).id; + + const capturedRequests: Array<{ url: string; body: string }> = []; + + await withMockedFetch( + async (url, options: FetchOptions = {}) => { + const body = String(options?.body ?? ""); + capturedRequests.push({ url: String(url), body }); + assert.equal(String(url), OAUTH_ENDPOINTS.openai.token); + + // (a) REUSE assertion: the refresh request must present the refresh_token + // that was actually stored on the connection β€” not a re-run of the full + // authorization_code flow, and not a stale/blank token. + const params = new URLSearchParams(body); + assert.equal(params.get("grant_type"), "refresh_token"); + assert.equal( + params.get("refresh_token"), + "codex-stored-refresh-token", + "must reuse the connection's stored refresh_token" + ); + + // Simulate OpenAI rotating in a brand-new refresh_token on this refresh. + return jsonResponse({ + access_token: "codex-fresh-access-token", + refresh_token: "codex-rotated-refresh-token", + expires_in: 3600, + }); + }, + async () => { + const connSnapshot = connection as ConnectionRecord; + const refreshed = await tokenRefresh.checkAndRefreshToken("codex", { + connectionId, + accessToken: connSnapshot.accessToken, + refreshToken: connSnapshot.refreshToken, + expiresAt: (connection as Record).expiresAt, + }); + + assert.equal(capturedRequests.length, 1, "the Codex token endpoint must be hit exactly once"); + + // The in-memory result returned to the caller carries the fresh tokens. + assert.equal(refreshed.accessToken, "codex-fresh-access-token"); + assert.equal(refreshed.refreshToken, "codex-rotated-refresh-token"); + + const stored = (await providersDb.getProviderConnectionById( + connectionId + )) as ConnectionRecord; + + // (b) PERSIST assertion: the refreshed access_token must be saved to the row. + assert.equal( + stored.accessToken, + "codex-fresh-access-token", + "the refreshed access_token must be persisted back to the connection" + ); + + // (c) ROTATION assertion: the new refresh_token must REPLACE the old one. + assert.equal( + stored.refreshToken, + "codex-rotated-refresh-token", + "a rotated refresh_token must replace the previously stored one" + ); + assert.notEqual( + stored.refreshToken, + "codex-stored-refresh-token", + "the stale, already-consumed refresh_token must not remain stored" + ); + + // (d) CLEAR-STALE-STATE assertion: a successful refresh must clear the + // auth-failure fields that drive the dashboard's "Auth Failed" badge. + // Before the fix these remained "invalid" / "upstream_auth_error" / "401" + // even though the token had genuinely refreshed. + assert.equal( + stored.testStatus, + "active", + "testStatus must clear to active after a successful refresh" + ); + // The read path (getProviderConnectionById) strips null-valued columns via + // cleanNulls(), so a cleared column surfaces as `undefined`, not `null` β€” + // both mean "cleared" here. + assert.equal(stored.lastError ?? null, null, "lastError must be cleared"); + assert.equal(stored.lastErrorType ?? null, null, "lastErrorType must be cleared"); + assert.equal(stored.lastErrorSource ?? null, null, "lastErrorSource must be cleared"); + assert.equal(stored.errorCode ?? null, null, "errorCode must be cleared"); + } + ); +}); From ebdfe727a602037ffe947477eab738e8cd7463ed Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:18:18 -0300 Subject: [PATCH 02/35] fix(test): replace tautology in playground-api-tab + make test-masking catch it (#6404) (#6603) playground-api-tab.test.tsx's SSE test always took the disabled-button branch (the fetch mock returned an empty model list) and asserted a tautology instead of exercising the SSE path it claims to verify. The test now selects a real model to enable Send, asserts it is actually enabled, and asserts the streamed SSE content reached the response editor. check-test-masking.mjs's tautology subcheck only compares base-vs-HEAD counts within a PR's own diff and no-ops entirely outside PR context (no GITHUB_BASE_SHA/REF) -- so a tautology merged once, or checked with a bare local run, stayed invisible forever after. Added an always-on absolute-floor scan (scanBareTautologies/countBareTautologies) over every tracked test file, scoped to the bare expect(true).toBe(true)/assert.equal(1,1) patterns that have zero legitimate uses in this codebase -- deliberately excluding assert.ok(true), which has ~15 pre-existing verified-legitimate try/catch-fallback uses and stays on the lenient diff-only path. --- CHANGELOG.md | 1 + scripts/check/check-test-masking.mjs | 103 +++++++++++++++++++++- tests/unit/check-test-masking.test.ts | 71 +++++++++++++++ tests/unit/ui/playground-api-tab.test.tsx | 44 ++++++--- 4 files changed, 205 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c44b96b38e..ecd488c6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ _Living section β€” bullets land here as PRs merge into `release/v3.8.47` (paral ### πŸ› Bug Fixes +- **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 `` 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`. - **fix(resilience):** `headroom` combo routing did not always select the Codex account with the most free quota ([#6379](https://github.com/diegosouzapw/OmniRoute/issues/6379)) β€” `orderTargetsByHeadroom` (`open-sse/services/combo/quotaStrategies.ts`) already loaded the per-connection DB snapshot (with decrypted credentials) via `expandTargetsByQuotaAwareConnections`, but discarded it before calling `getSaturation`; for Codex, `fetchCodexSaturation` forwards straight to `fetchCodexQuota(connectionId, connection)`, which needs `connection` (or a prior `registerCodexConnection()` call, which never happens before headroom ranking runs) to read `accessToken` β€” so it returned `null` for every candidate, saturation failed open to `0` across the board, and ranking fell back to the original combo order regardless of actual free quota. `getSaturation()` and the headroom `SaturationFetcher` seam now accept and thread the loaded connection snapshot through to `fetchCodexQuota`. Kilo's dup flag vs #5903 was a false positive β€” that issue is about session-sticky reset-aware/least-used selection, not headroom's Codex saturation lookup. Regression guard: `tests/unit/headroom-codex-quota-snapshot-6379.test.ts`. (thanks @eidoog) diff --git a/open-sse/config/imageRegistry.ts b/open-sse/config/imageRegistry.ts index 330a37c730..97f1dc60ce 100644 --- a/open-sse/config/imageRegistry.ts +++ b/open-sse/config/imageRegistry.ts @@ -723,6 +723,19 @@ export function getImageModelAliases() { return IMAGE_MODEL_ALIASES; } +/** + * #6457 β€” precise provider+modelId membership check against the image registry. + * Unlike getImageModelEntry() (which also resolves bare aliases and unprefixed + * ids by scanning every provider), this only answers "is `modelId` registered + * as an image model under this exact `providerId`?" β€” used by the chat catalog + * builder to keep upstream-discovered models (e.g. HuggingFace's live + * `/v1/models`, which returns image/diffusion models with no modality field) + * out of the chat listing when they are already known image-only models. + */ +export function isRegisteredImageModel(providerId, modelId) { + return Boolean(findImageModelConfig(providerId, modelId)); +} + export function getImageModelEntry(modelStr) { if (!modelStr) return null; diff --git a/src/app/api/v1/models/catalog.ts b/src/app/api/v1/models/catalog.ts index f4b1a57319..f6d9d25cc6 100644 --- a/src/app/api/v1/models/catalog.ts +++ b/src/app/api/v1/models/catalog.ts @@ -12,7 +12,7 @@ import { import { extractAliasBackedModels } from "./aliasBackedModels"; import { appendNoThinkingVariants } from "@omniroute/open-sse/utils/noThinkingAlias"; import { getAllEmbeddingModels } from "@omniroute/open-sse/config/embeddingRegistry"; -import { getAllImageModels } from "@omniroute/open-sse/config/imageRegistry"; +import { getAllImageModels, isRegisteredImageModel } from "@omniroute/open-sse/config/imageRegistry"; import { getAllRerankModels } from "@omniroute/open-sse/config/rerankRegistry"; import { getAllAudioModels } from "@omniroute/open-sse/config/audioRegistry"; import { getAllModerationModels } from "@omniroute/open-sse/config/moderationRegistry"; @@ -849,6 +849,18 @@ async function buildUnifiedModelsResponseCore( for (const sm of syncedModels) { if (!providerSupportsModel(canonicalProviderId, sm.id)) continue; if (getModelIsHidden(providerId, sm.id)) continue; + // #6457: some upstream discovery catalogs (e.g. HuggingFace's live + // `/v1/models`) return image/diffusion models with no modality info, + // so `endpoints` below would default to ["chat"] and misrepresent + // them as chat-capable. Skip any synced model that is already a + // registered image model for this provider β€” getAllImageModels() + // below adds the correctly-typed `type: "image"` entry instead. + if ( + isRegisteredImageModel(canonicalProviderId, sm.id) || + isRegisteredImageModel(providerId, sm.id) + ) { + continue; + } // #6328: apply hidePaidModels to synced provider rows too. Synced rows // rarely carry pricing metadata, so shouldHidePaid() falls through to // the FREE_MODEL_IDS_BY_PROVIDER catalog β€” providers with a curated diff --git a/tests/unit/image-model-not-in-chat-catalog-6457.test.ts b/tests/unit/image-model-not-in-chat-catalog-6457.test.ts new file mode 100644 index 0000000000..cbd9b2033e --- /dev/null +++ b/tests/unit/image-model-not-in-chat-catalog-6457.test.ts @@ -0,0 +1,102 @@ +// Regression test for #6457 β€” huggingface/stabilityai/stable-diffusion-xl-base-1.0 +// (an image/diffusion model) was listed as a CHAT model in GET /v1/models, so firing +// POST /v1/chat/completions with it hit the upstream and returned a raw HuggingFace +// "[400] The requested model '...' is not a chat model." error. +// +// Root cause: the synced-provider-models loop in catalog.ts (fed by live discovery β€” +// e.g. HuggingFace's own `/v1/models`) defaults a model's `endpoints` to `["chat"]` +// whenever the upstream discovery payload carries no modality/endpoint info, which is +// exactly what HuggingFace's live catalog returns for image models. That produced a +// second, bogus chat-typed entry for the SAME id already correctly listed with +// `type: "image"` by the imageRegistry loop β€” and catalogDedupe.ts keys on +// (id, type, subtype), so the two distinct-`type` entries both survived. +// +// Fix: skip a synced model in the chat-catalog loop when it is already a registered +// image model for that exact provider (open-sse/config/imageRegistry.ts +// isRegisteredImageModel()) β€” the imageRegistry loop still adds the correctly-typed +// `type: "image"` entry. + +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-image-chat-6457-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "catalog-test-secret-6457"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const modelsDb = await import("../../src/lib/db/models.ts"); +const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts"); + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +async function seedHuggingFaceConnection() { + return providersDb.createProviderConnection({ + provider: "huggingface", + authType: "apikey", + name: `huggingface-${Math.random().toString(16).slice(2, 8)}`, + apiKey: "hf-key", + isActive: true, + testStatus: "active", + }); +} + +test("#6457 image/diffusion model discovered via live sync is NOT listed as a chat model", async () => { + const connection = await seedHuggingFaceConnection(); + + // Simulate what HuggingFace's live `/v1/models` discovery persists for an + // image/diffusion model: no supportedEndpoints/modality info at all β€” the exact + // upstream shape that made the synced-models loop default to `["chat"]`. + await modelsDb.replaceSyncedAvailableModelsForConnection("huggingface", connection.id, [ + { id: "stabilityai/stable-diffusion-xl-base-1.0", name: "Stable Diffusion XL (HF)" }, + { id: "meta-llama/llama-3.1-8b-instruct", name: "Llama 3.1 8B" }, + ]); + + const response = await v1ModelsCatalog.getUnifiedModelsResponse( + new Request("http://localhost/api/v1/models") + ); + assert.equal(response.status, 200); + + const body = (await response.json()) as { + data: Array<{ id: string; type?: string }>; + }; + + const imageModelEntries = body.data.filter((m) => + m.id.includes("stabilityai/stable-diffusion-xl-base-1.0") + ); + + assert.ok(imageModelEntries.length > 0, "the image model must still be listed somewhere"); + for (const entry of imageModelEntries) { + assert.equal( + entry.type, + "image", + `every listing of the diffusion model must be type:"image", got ${JSON.stringify(entry)}` + ); + } + + // A real chat model synced alongside it must still be listed as chat (no `type`, + // per the OpenAI-compatible convention used throughout this catalog). + const chatModelEntries = body.data.filter((m) => + m.id.includes("meta-llama/llama-3.1-8b-instruct") + ); + assert.ok(chatModelEntries.length > 0, "the real chat model must still be listed"); + for (const entry of chatModelEntries) { + assert.equal(entry.type, undefined, "the real chat model must not carry a non-chat type"); + } +}); From 978e92e10481a53176351fd5463bb9021292d9ba Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:48:02 -0300 Subject: [PATCH 04/35] fix(providers): honor fusion config.judgeModel for final synthesis (#6455) (#6607) The fusion single-survivor degrade path (added for #6454) returned the lone panel answer directly whenever only one panelist succeeded, ignoring an explicitly configured judgeModel. With default minPanel=2 and a 2-model panel, any single flaky panelist forced this path every request, so the configured judge never ran and the response .model reflected a panel member. The judge is now still invoked to synthesize a lone surviving answer when judgeModel is explicitly configured; the direct-answer shortcut is kept only for the implicit case (no judgeModel, judge defaults to panel[0]). --- CHANGELOG.md | 1 + open-sse/services/fusion.ts | 20 ++- tests/unit/combo-fusion-strategy.test.ts | 40 +++++- tests/unit/fusion-judge-model-6455.test.ts | 142 +++++++++++++++++++++ 4 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 tests/unit/fusion-judge-model-6455.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 52488b1cac..3ea489b702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ _Living section β€” bullets land here as PRs merge into `release/v3.8.47` (paral ### πŸ› Bug Fixes +- **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) - **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(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 `` 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) diff --git a/open-sse/utils/toolCallArguments.ts b/open-sse/utils/toolCallArguments.ts index b5554c681a..9d7c797994 100644 --- a/open-sse/utils/toolCallArguments.ts +++ b/open-sse/utils/toolCallArguments.ts @@ -15,10 +15,32 @@ * A fuzzy suffix/prefix-overlap heuristic must NOT be used here: it silently * drops bytes from legitimate incremental deltas (turning `ll` into `l`, `xx` * into `x`), which trades a visible duplication bug for a silent truncation bug. + * + * A third, non-conformant shape some upstreams emit (#6459): the FULL + * `arguments` value delivered as an already-parsed JSON object/array instead + * of a JSON-encoded string (violates the OpenAI streaming contract, but seen + * from some Anthropic-shape-passthrough backends). Treating that as "not a + * string" and silently discarding it left `tool_use.input` empty upstream β€” + * or, when a caller re-serialized the buffer with plain string coercion + * instead of JSON, rendered literally as `[object Object]` in the client + * transcript. JSON.stringify it into a proper fragment instead of dropping it. */ +function normalizeIncomingFragment(incoming: unknown): string { + if (typeof incoming === "string") return incoming; + if (incoming == null) return ""; + if (typeof incoming === "object") { + try { + return JSON.stringify(incoming); + } catch { + return ""; + } + } + return ""; +} + export function appendToolCallArgumentDelta(current: unknown, incoming: unknown): string { const existing = typeof current === "string" ? current : ""; - const next = typeof incoming === "string" ? incoming : ""; + const next = normalizeIncomingFragment(incoming); if (!existing) return next; if (!next) return existing; diff --git a/tests/unit/anthropic-toolcall-args-6459.test.ts b/tests/unit/anthropic-toolcall-args-6459.test.ts new file mode 100644 index 0000000000..0a7d02d8cb --- /dev/null +++ b/tests/unit/anthropic-toolcall-args-6459.test.ts @@ -0,0 +1,161 @@ +/** + * Regression test for #6459: tool-call arguments render as + * `[object Object][object Object]` in the user-visible transcript when the + * upstream provider delivers the FULL `tool_calls[].function.arguments` value + * as an already-parsed JSON object (not a JSON-encoded string), which is what + * some Anthropic-shape-compatible backends do instead of following the OpenAI + * streaming contract. + * + * Before the fix, `appendToolCallArgumentDelta()` treated any non-string + * `incoming` fragment as an empty string, so the accumulated `argBuffer` + * never picked up the object at all β€” `openaiToClaudeResponse()` (the + * translator that builds the live /anthropic SSE stream, see + * `open-sse/translator/response/openai-to-claude.ts`) then emitted no + * `input_json_delta` for that chunk, and the client is left to coerce + * whatever partial data it has via string concatenation/`String(object)`, + * which is exactly how `[object Object]` sequences end up in the transcript. + * + * The fix: JSON.stringify() a non-string, non-null object/array fragment + * instead of discarding it, so the assembled `partial_json` is always valid + * JSON that parses back into the original structured value. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { openaiToClaudeResponse } from "../../open-sse/translator/response/openai-to-claude.ts"; + +function createState() { + return { toolCalls: new Map() }; +} + +function flatten(items: unknown[][]) { + return items.flatMap((item) => item || []); +} + +function assembleToolUseInput(events: Array>) { + const jsonDeltas = events.filter( + (e) => e?.type === "content_block_delta" && (e.delta as Record)?.type === "input_json_delta" + ); + const assembled = jsonDeltas + .map((e) => (e.delta as Record).partial_json as string) + .join(""); + return assembled; +} + +test("#6459: tool-call arguments delivered as a structured object (not a JSON string) render as the real object, not [object Object]", () => { + const state = createState(); + + const chunk1 = openaiToClaudeResponse( + { + id: "chatcmpl-6459", + model: "auto/claude-opus", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: "call_6459", + type: "function", + function: { name: "AskUserQuestion", arguments: "" }, + }, + ], + }, + finish_reason: null, + }, + ], + }, + state + ); + + // Non-conformant upstream: the FULL arguments value arrives as an already- + // parsed JS object (mirroring a nested tool_use.input structure), not a + // JSON-encoded string fragment. + const chunk2 = openaiToClaudeResponse( + { + id: "chatcmpl-6459", + model: "auto/claude-opus", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + function: { + arguments: { + questions: [ + { header: "Deploy target", options: [{ label: "staging" }] }, + { header: "Confirm rollback", options: [{ label: "yes" }, { label: "no" }] }, + ], + }, + }, + }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }, + state + ); + + const events = flatten([chunk1, chunk2]) as Array>; + const assembled = assembleToolUseInput(events); + + assert.ok(assembled.length > 0, "expected at least one input_json_delta with the tool args"); + assert.ok( + !assembled.includes("[object Object]"), + `assembled partial_json leaked a stringified-object coercion: ${assembled}` + ); + + let parsed: Record; + try { + parsed = JSON.parse(assembled); + } catch { + assert.fail(`assembled partial_json is not valid JSON β€” arguments object was corrupted: ${assembled}`); + } + + assert.ok(Array.isArray(parsed.questions), "questions array must survive as structured data"); + assert.equal(parsed.questions.length, 2); + assert.equal((parsed.questions[0] as Record).header, "Deploy target"); + assert.equal((parsed.questions[1] as Record).header, "Confirm rollback"); +}); + +test("#6459 no-regression: a plain text-only turn still translates normally", () => { + const state = createState(); + + const chunk1 = openaiToClaudeResponse( + { + id: "chatcmpl-6459-text", + model: "auto/claude-opus", + choices: [{ index: 0, delta: { role: "assistant" }, finish_reason: null }], + }, + state + ); + const chunk2 = openaiToClaudeResponse( + { + id: "chatcmpl-6459-text", + model: "auto/claude-opus", + choices: [{ index: 0, delta: { content: "Hello, world!" }, finish_reason: null }], + }, + state + ); + const chunk3 = openaiToClaudeResponse( + { + id: "chatcmpl-6459-text", + model: "auto/claude-opus", + choices: [{ index: 0, delta: {}, finish_reason: "stop" }], + }, + state + ); + + const events = flatten([chunk1, chunk2, chunk3]) as Array>; + const textDeltas = events.filter( + (e) => e?.type === "content_block_delta" && (e.delta as Record)?.type === "text_delta" + ); + + assert.equal(textDeltas.length, 1); + assert.equal((textDeltas[0].delta as Record).text, "Hello, world!"); +}); From 533016af368c7d07ba81075e616fcfd67491776b Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 7 Jul 2026 22:38:26 -0300 Subject: [PATCH 06/35] fix(providers): backfill #6454 CHANGELOG bullet + 11-member fusion regression guard (#6614) The fusion quorum-clamp/failure-detail root cause reported in #6454 was already fixed and merged via #6521 (open-sse/services/fusion.ts already carries Math.max(1, cfg.minPanel) + per-member failure reasons on this branch). That merge never landed a CHANGELOG bullet for #6454 itself. Backfills the missing bullet and adds a regression test at the exact repro scale (11-member fusion-free-style panel, 2 cooling / 9 healthy) to lock in that a cooling minority no longer sinks a healthy majority, while a genuinely all-failed panel still returns the documented 503. --- CHANGELOG.md | 1 + .../fusion-partial-panel-failure-6454.test.ts | 100 ++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 tests/unit/fusion-partial-panel-failure-6454.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e087578d4d..094e0e3de6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ _Living section β€” bullets land here as PRs merge into `release/v3.8.47` (paral ### πŸ› Bug Fixes - **fix(api):** tool-call arguments could render as `[object Object]` sequences instead of the real JSON through the `/anthropic` (Anthropic-shape `/messages`) routing path ([#6459](https://github.com/diegosouzapw/OmniRoute/issues/6459)) β€” `appendToolCallArgumentDelta()` (`open-sse/utils/toolCallArguments.ts`), the shared accumulator the streaming `openai-to-claude` response translator, `openai-responses` translator, and `responsesTransformer` all call to build up a tool call's `arguments`/`input_json_delta` buffer, treated any non-string `incoming` fragment as an empty string. Some upstreams deliver the full `tool_calls[].function.arguments` value as an already-parsed JSON object/array instead of the OpenAI-contracted JSON-encoded string; the old code silently discarded that fragment, leaving `tool_use.input` empty, and left downstream buffers open to a plain string coercion of the object (`[object Object]`) once client-side concatenation kicked in. `appendToolCallArgumentDelta()` now `JSON.stringify()`s a non-string, non-null object/array fragment into a valid JSON fragment instead of dropping it, so the assembled `partial_json` always parses back into the original structured value. Regression guard: `tests/unit/anthropic-toolcall-args-6459.test.ts`. (thanks @chirag127) +- **fix(providers):** `fusion` combo returned the opaque `"All fusion panel models failed"` 503 even when only a minority of panel members were actually cooling down / rate-limited, and a user-supplied `fusionTuning.minPanel=1` was silently overridden ([#6454](https://github.com/diegosouzapw/OmniRoute/issues/6454)) β€” `handleFusionChat()` hard-clamped the quorum floor via `Math.min(Math.max(2, cfg.minPanel), panel.length)`, so an operator-configured `minPanel=1` never took effect: `collectPanel()`'s straggler-grace timer only starts once `ok >= minPanel`, and with the floor forced to 2 a single fast success plus N slow-failing stragglers never reached quorum, so the panel sat waiting instead of degrading to the survivor. Per-member failure reasons (`straggler_dropped`/`timeout`/`threw`/`status_XXX`/`empty_content`/`unparseable`) were also logged server-side but never surfaced in the 503 body, leaving operators unable to tell a rate-limit fan-fail from a broader outage. Fixed by honoring `Math.max(1, cfg.minPanel)` and threading a `failures: Array<{ model, reason }>` collector into the 503 message (`model=reason` per entry) β€” production fix already merged via #6521; this entry backfills the missing CHANGELOG bullet and adds an 11-member, `fusion-free`-scale regression test matching the original repro shape (a cooling minority must not sink a healthy majority; a genuinely all-failed panel still returns the documented 503). Regression guard: `tests/unit/services/fusion-min-panel-and-failure-detail.test.ts` + `tests/unit/fusion-partial-panel-failure-6454.test.ts`. (thanks @chirag127) - **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) - **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(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 `` 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`. diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index d4eaf3ed70..94a090c87c 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -18,7 +18,12 @@ import { recordProviderFailure, selectLockoutCooldownMs, } from "./accountFallback.ts"; -import { errorResponse, unavailableResponse } from "../utils/error.ts"; +import { + errorResponse, + unavailableResponse, + errorResponseWithComboDiagnostics, +} from "../utils/error.ts"; +import type { ComboDiagnostics } from "../utils/error.ts"; import { buildTargetTimeoutRunner } from "./combo/targetTimeoutRunner.ts"; import { recordComboRequest, recordComboShadowRequest, getComboMetrics } from "./comboMetrics.ts"; import { @@ -1222,8 +1227,24 @@ export async function handleComboChat({ // 16 strategies (priority, weighted, etc.) that funnel through executeTarget. const quotaCutoffResetWindowConfig = resolveResetWindowConfig(config as Record); + // QA P0 diagnostics: record the order in which targets were actually attempted + // (provider/model ids only) so a terminal combo failure can report the attempt + // sequence alongside pool size + exhaustion reasons. Accumulates across set retries. + const comboAttemptOrder: Array<{ provider: string; model: string }> = []; + if (orderedTargets.length === 0) { - return comboModelNotFoundResponse("Combo has no executable targets"); + return errorResponseWithComboDiagnostics( + 404, + "Combo has no executable targets", + { + poolSize: 0, + attempted: 0, + excluded: [], + attemptOrder: [], + terminalReason: "no_executable_targets", + }, + { code: "model_not_found", type: "invalid_request_error" } + ); } scheduleShadowRouting( @@ -1306,6 +1327,23 @@ export async function handleComboChat({ let fallbackCount = 0; let recordedAttempts = 0; + // QA P0: assemble a sanitized diagnostic trace from the state already in scope + // (pool size + this set-try's exhausted providers/connections + attempt order + + // a terminal-reason code). Never touches keys/tokens β€” provider/model ids only. + const buildComboDiag = (terminalReason: string): ComboDiagnostics => ({ + poolSize: orderedTargets.length, + attempted: recordedAttempts, + excluded: [ + ...[...exhaustedProviders].map((p) => ({ provider: p, reason: "exhausted" })), + ...[...exhaustedConnections].map((c) => ({ + provider: "unknown", + reason: `exhausted_connection:${String(c).slice(0, 8)}`, + })), + ], + attemptOrder: comboAttemptOrder, + terminalReason, + }); + let globalResolve: ((res: Response) => void) | null = null; const globalPromise = new Promise((res) => { globalResolve = res; @@ -1442,7 +1480,23 @@ export async function handleComboChat({ "COMBO", `Maximum combo attempts (${MAX_GLOBAL_ATTEMPTS}) exceeded across all targets and fallbacks. Terminating loop to prevent runaway background requests.` ); - return { ok: false, response: errorResponse(503, "Maximum combo retry limit reached") }; + // Actionable failure instead of an opaque 503 when every candidate + // failed the same recoverable way. If the dominant cause was reasoning + // models exhausting a too-small max_tokens budget (no content output), + // retrying other models can't help β€” tell the caller to raise max_tokens. + const reasoningExhausted = /reasoning consumed \d+\/\d+ tokens/.test(lastError || ""); + return { + ok: false, + response: errorResponseWithComboDiagnostics( + 503, + reasoningExhausted + ? "All combo candidates exhausted their token budget on reasoning without producing content. Increase max_tokens β€” reasoning models need a larger budget to emit content." + : "Maximum combo retry limit reached", + buildComboDiag( + reasoningExhausted ? "reasoning_budget_exhausted" : "max_attempts_exceeded" + ) + ), + }; } // Predictive TTFT Circuit Breaker (skip slow models) @@ -1500,6 +1554,8 @@ export async function handleComboChat({ timestamp: Date.now(), strategy, }); + // QA P0 diagnostics: capture the attempt order (provider/model ids only). + comboAttemptOrder.push({ provider: provider ?? "unknown", model: modelStr }); // Deep clone the body to ensure context preservation and prevent mutations // from affecting other targets in the combo. structuredClone avoids the @@ -2244,15 +2300,11 @@ export async function handleComboChat({ latencyMs, fallbackCount, }); - return new Response( - JSON.stringify({ - error: { - message: "Service temporarily unavailable: all upstream accounts are inactive", - type: "service_unavailable", - code: "ALL_ACCOUNTS_INACTIVE", - }, - }), - { status: 503, headers: { "Content-Type": "application/json" } } + return errorResponseWithComboDiagnostics( + 503, + "Service temporarily unavailable: all upstream accounts are inactive", + buildComboDiag("all_accounts_inactive"), + { code: "ALL_ACCOUNTS_INACTIVE", type: "service_unavailable" } ); } @@ -2303,10 +2355,11 @@ export async function handleComboChat({ } log.warn("COMBO", `All models failed | ${msg}`); - return new Response(JSON.stringify({ error: { message: msg } }), { + return errorResponseWithComboDiagnostics( status, - headers: { "Content-Type": "application/json" }, - }); + msg, + buildComboDiag(lastError ?? "all_models_failed") + ); } return errorResponse(503, "Combo routing completed without an upstream response"); diff --git a/open-sse/utils/error.ts b/open-sse/utils/error.ts index 24354451f7..88efa6c67d 100644 --- a/open-sse/utils/error.ts +++ b/open-sse/utils/error.ts @@ -120,6 +120,88 @@ export function buildErrorBody( return body; } +/** + * Sanitized auto-combo diagnostic trace surfaced on a combo terminal failure. + * Contains ONLY provider/model ids, enumerated reason codes, and counts β€” never + * keys, tokens, cookies, credentials, or upstream bodies. Fields are length- and + * count-capped so the projection is safe to place in HTTP headers too. (QA P0: + * "Add a sanitized combo diagnostic trace … candidate pool count, excluded + * provider/model reasons, selected attempt order, terminal failure summary.") + */ +export interface ComboExclusion { + provider: string; + model?: string; + reason: string; +} +export interface ComboDiagnostics { + poolSize: number; + attempted: number; + excluded: ComboExclusion[]; + attemptOrder: Array<{ provider: string; model: string }>; + terminalReason: string; +} + +function clampDiagStr(v: unknown, max = 128): string { + return typeof v === "string" ? v.slice(0, max).replace(/[\r\n]+/g, " ") : ""; +} + +/** + * Whitelist projection β€” guarantees only id/reason string primitives + integer + * counts can escape, regardless of what the caller assembled. This is the secret + * containment boundary for the diagnostic trace. + */ +export function sanitizeComboDiagnostics(d: ComboDiagnostics): ComboDiagnostics { + return { + poolSize: Number.isFinite(d?.poolSize) ? d.poolSize : 0, + attempted: Number.isFinite(d?.attempted) ? d.attempted : 0, + excluded: (d?.excluded ?? []).slice(0, 64).map((e) => ({ + provider: clampDiagStr(e?.provider, 64), + ...(e?.model ? { model: clampDiagStr(e.model, 96) } : {}), + reason: clampDiagStr(e?.reason, 64), + })), + attemptOrder: (d?.attemptOrder ?? []) + .slice(0, 64) + .map((a) => ({ provider: clampDiagStr(a?.provider, 64), model: clampDiagStr(a?.model, 96) })), + terminalReason: clampDiagStr(d?.terminalReason, 200), + }; +} + +/** + * errorResponse variant that attaches a sanitized combo diagnostic trace as BOTH + * `x-omniroute-combo-*` headers and a `diagnostics` field in the OpenAI-shaped + * error body (extra field β€” backward-compatible with standard error parsers). + * `opts.code`/`opts.type` override the status-derived defaults (e.g. to preserve + * the `ALL_ACCOUNTS_INACTIVE` code on the 503 terminal path). + */ +export function errorResponseWithComboDiagnostics( + statusCode: number, + message: string, + diagnostics: ComboDiagnostics, + opts: { code?: string; type?: string } = {} +): Response { + const safe = sanitizeComboDiagnostics(diagnostics); + const body = buildErrorBody(statusCode, message) as ErrorResponseBody & { + diagnostics?: ComboDiagnostics; + }; + if (opts.code) body.error.code = opts.code; + if (opts.type) body.error.type = opts.type; + body.diagnostics = safe; + const excludedHeader = safe.excluded + .map((e) => `${e.provider}${e.model ? `/${e.model}` : ""}:${e.reason}`) + .join(",") + .slice(0, 900); + return new Response(JSON.stringify(body), { + status: statusCode, + headers: { + "Content-Type": "application/json", + "x-omniroute-combo-pool-size": String(safe.poolSize), + "x-omniroute-combo-attempted": String(safe.attempted), + "x-omniroute-combo-excluded": excludedHeader, + "x-omniroute-combo-terminal-reason": safe.terminalReason.slice(0, 200), + }, + }); +} + /** * Create error Response object (for non-streaming) * @param {number} statusCode - HTTP status code diff --git a/tests/unit/combo-diagnostics-trace.test.ts b/tests/unit/combo-diagnostics-trace.test.ts new file mode 100644 index 0000000000..be25e64c77 --- /dev/null +++ b/tests/unit/combo-diagnostics-trace.test.ts @@ -0,0 +1,81 @@ +/** + * QA P0 β€” sanitized auto-combo diagnostic trace. + * Guards the new `errorResponseWithComboDiagnostics` / `sanitizeComboDiagnostics` + * helpers: they must surface pool size + attempt order + exclusion reasons as + * both `x-omniroute-combo-*` headers and a `diagnostics` body field, while the + * sanitizer is the secret-containment boundary (only provider/model/reason ids + + * counts may ever escape β€” never keys/tokens/credentials). + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { errorResponseWithComboDiagnostics, sanitizeComboDiagnostics } = await import( + "../../open-sse/utils/error.ts" +); + +test("combo diagnostics: headers + body carry the sanitized trace (code override preserved)", async () => { + const res = errorResponseWithComboDiagnostics( + 503, + "all upstream accounts inactive", + { + poolSize: 3, + attempted: 2, + excluded: [{ provider: "openai", model: "gpt-x", reason: "exhausted" }], + attemptOrder: [{ provider: "openai", model: "gpt-x" }], + terminalReason: "all_accounts_inactive", + }, + { code: "ALL_ACCOUNTS_INACTIVE", type: "service_unavailable" } + ); + + assert.equal(res.status, 503); + assert.equal(res.headers.get("x-omniroute-combo-pool-size"), "3"); + assert.equal(res.headers.get("x-omniroute-combo-attempted"), "2"); + assert.match(res.headers.get("x-omniroute-combo-excluded") || "", /openai\/gpt-x:exhausted/); + assert.equal(res.headers.get("x-omniroute-combo-terminal-reason"), "all_accounts_inactive"); + + const body = await res.json(); + assert.equal(body.error.code, "ALL_ACCOUNTS_INACTIVE"); + assert.equal(body.error.type, "service_unavailable"); + assert.ok(body.diagnostics, "diagnostics field present in body"); + assert.equal(body.diagnostics.poolSize, 3); + assert.equal(body.diagnostics.attempted, 2); + assert.equal(body.diagnostics.terminalReason, "all_accounts_inactive"); + assert.equal(body.diagnostics.attemptOrder[0].provider, "openai"); +}); + +test("combo diagnostics: sanitizer caps sizes + keeps only the whitelist keys", () => { + const dirty = { + poolSize: 1, + attempted: 1, + excluded: Array.from({ length: 200 }, (_, i) => ({ + provider: "p" + i, + reason: "r".repeat(500), + })), + attemptOrder: Array.from({ length: 200 }, () => ({ provider: "p", model: "m" })), + terminalReason: "x".repeat(1000), + }; + const safe = sanitizeComboDiagnostics(dirty as never); + assert.ok(safe.excluded.length <= 64, "excluded capped at 64"); + assert.ok(safe.attemptOrder.length <= 64, "attemptOrder capped at 64"); + assert.ok(safe.excluded[0].reason.length <= 64, "reason length clamped"); + assert.ok(safe.terminalReason.length <= 200, "terminalReason length clamped"); + assert.deepEqual(Object.keys(safe.excluded[0]).sort(), ["provider", "reason"]); +}); + +test("combo diagnostics: secret containment β€” non-whitelisted fields never survive", () => { + const leaky = { + poolSize: 1, + attempted: 1, + excluded: [ + { provider: "openai", reason: "exhausted", apiKey: "sk-SECRET-KEY", token: "SECRET-TOK" }, + ], + attemptOrder: [{ provider: "openai", model: "m", accessToken: "SECRET-OAUTH" }], + terminalReason: "t", + }; + const safe = sanitizeComboDiagnostics(leaky as never); + const serialized = JSON.stringify(safe); + assert.ok(!serialized.includes("SECRET"), "no secret VALUES survive the projection"); + assert.ok(!serialized.includes("apiKey"), "no apiKey KEY survives"); + assert.ok(!serialized.includes("accessToken"), "no accessToken KEY survives"); + assert.ok(!serialized.includes("token"), "no token KEY survives"); +}); From ed0b9c73de3b1bdfd5e87ec94d66f198096beaf4 Mon Sep 17 00:00:00 2001 From: Jillur Rahman Date: Wed, 8 Jul 2026 10:36:03 +0600 Subject: [PATCH 23/35] perf(health): short-TTL cache for GET /api/monitoring/health (#6553) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merged β€” thank you, @developerjillur! Short-TTL (1s) cache for the frequently-polled GET /api/monitoring/health, invalidated on DELETE (circuit-breaker reset). Integrated into release/v3.8.47. --- CHANGELOG.md | 1 + src/app/api/monitoring/health/route.ts | 15 +++++ .../monitoring-health-cache.test.ts | 63 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 tests/integration/monitoring-health-cache.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ef97f259..333f0113c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ _Living section β€” bullets land here as PRs merge into `release/v3.8.47` (paral - **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(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 ` { + setBlockText(e.target.value); + setParamDirty(true); + }} + onBlur={() => saveModelParamFilters()} + placeholder="thinking, … (comma-separated)" + disabled={disabled} + className="mb-1 w-full rounded-lg border border-zinc-200 bg-white px-2.5 py-1.5 text-[11px] font-mono text-text-main placeholder:text-text-muted focus:border-primary focus:outline-none focus:ring-1 focus:ring-primary/30 dark:border-zinc-600 dark:bg-zinc-900" + /> +

+ {t("compatBlockedParamsHint") ?? "Blocked params (stripped from requests)"} + {paramSaving && " ● saving…"} +

+ +
+ { + setAllowText(e.target.value); + setParamDirty(true); + }} + onBlur={() => saveModelParamFilters()} + placeholder="reasoning, … (comma-separated)" + disabled={disabled} + className="mb-1 w-full rounded-lg border border-zinc-200 bg-white px-2.5 py-1.5 text-[11px] font-mono text-text-main placeholder:text-text-muted focus:border-primary focus:outline-none focus:ring-1 focus:ring-primary/30 dark:border-zinc-600 dark:bg-zinc-900" + /> +

+ {t("compatAllowedParamsHint") ?? "Allowed params (re-added after deny)"} +

+
+ +