From cd81b2ab9868f0f33c39581495a288b0285bfa8a Mon Sep 17 00:00:00 2001 From: Hamsa_M <116961508+hamsa0x7@users.noreply.github.com> Date: Fri, 3 Jul 2026 00:00:49 +0530 Subject: [PATCH] fix(discovery): resolve duplicate /v1 paths and redirect aborts (#5904) Integrated into release/v3.8.44. Thanks @hamsa0x7 for diagnosing the doubled /v1 discovery path and the REDIRECT_BLOCKED probe-loop abort (#5899). De-scoped to the discovery fix (the #5903 session-affinity work is handled by #5943) and added Rule #18 regression guards. --- CHANGELOG.md | 2 +- config/quality/file-size-baseline.json | 3 +- src/app/api/providers/[id]/models/route.ts | 9 +- tests/unit/provider-models-route.test.ts | 124 +++++++++++++++++++++ 4 files changed, 135 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 070d8dae6e..7484b1c082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ _TBD_ ### 🔧 Bug Fixes -_TBD_ +- **fix(providers): Api Airforce model discovery no longer produces a doubled `/v1` path** — a base URL ending in `/v1/chat/completions` (e.g. `https://api.airforce/v1/chat/completions`) was only stripped of `/chat/completions`, leaving a trailing `/v1` that the endpoint builder then doubled into `…/v1/v1/models`. That 308 redirect was surfaced as `REDIRECT_BLOCKED` and aborted the whole discovery probe loop before the correct `…/v1/models` candidate. The `/v1` suffix is now stripped independently (guarding a host literally named `v1`), and a `REDIRECT_BLOCKED` on one candidate continues to the next endpoint instead of aborting. Regression guards: `tests/unit/provider-models-route.test.ts`. ([#5904](https://github.com/diegosouzapw/OmniRoute/pull/5904) — thanks [@hamsa0x7](https://github.com/hamsa0x7)). Also reported/fixed independently by [@anki1kr](https://github.com/anki1kr) in [#5920](https://github.com/diegosouzapw/OmniRoute/pull/5920). ### 📝 Maintenance diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 2756977aa9..69c2eacabe 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -294,7 +294,7 @@ "_rebaseline_2026_06_27_5193_antigravity_test": "#5193 own test growth: oauth-providers-config.test.ts 870->873 (+3: antigravity projectId assertion + 50ms tick for the now fire-and-forget onboarding, matching the no-PKCE/no-openid flow).", "tests/unit/oauth-providers-config.test.ts": 873, "tests/unit/perplexity-web.test.ts": 959, - "tests/unit/provider-models-route.test.ts": 1628, + "tests/unit/provider-models-route.test.ts": 1752, "tests/unit/provider-validation-specialty.test.ts": 2874, "_rebaseline_pr4613_compatible_provider_groups": "Reconcile #4613 already-merged growth: providers-page-utils.test.ts 1004->1052 (+48, buildCompatibleProviderGroups partition unit test). Fast-gate PR->release does not run check:file-size, so this surfaced post-merge.", "tests/unit/providers-page-utils.test.ts": 1052, @@ -356,6 +356,7 @@ "_rebaseline_2026_06_17_4107_pending_reaper": "PR #4107 own growth: usageHistory.ts 854->934 (+80 = orphaned-pending-request reaper — sweepStalePendingRequests() evicts pending details older than 15min + a hard 5000 cap, plus an unref'd 5min sweep timer wired lazily into trackPendingRequest). Fixes an unbounded memory leak where abnormally-terminated requests left payload previews in pendingById forever. Cohesive with the existing pending-request bookkeeping (mirrors the normal removal path: decrement counters + cleanup buckets); not extractable.", "_rebaseline_2026_06_17_4116_combo_hedge_listener": "combo.ts: +9 lines from #4116 (detach per-target listener from shared hedge abort signal to fix a listener leak). Behavior-preserving cleanup; 5289 -> 5298.", "_rebaseline_2026_06_20_4355_gpt5x_pro_pricing": "PR #4355 own growth: pricing.ts 1581->1592 (+11 = pure-data pricing rows for openai gpt-5.5-pro + gpt-5.4-pro, closing the $0 gap that tripped the catalog pricing gate after the #4324 sweep added them to the registry; -pro mirrors its base family tier). provider-models-route.test.ts 1616->1618 (+2 = test-only alignment to the intentional opencode-go discovery behavior: owned_by stamp + T39 two-endpoint fail-path fetchCalls). Both are data/test-only; not extractable.", + "_rebaseline_2026_07_02_5899_airforce_v1_discovery": "PR #5904 own growth: provider-models-route.test.ts 1628->1752 (+124 = test-only Rule #18 regression guards for the Api Airforce /v1/v1/models discovery bug (#5899): (a) a baseUrl ending in /v1/chat/completions must probe .../v1/models not the doubled .../v1/v1/models, and the host-guard case http://v1; (b) a REDIRECT_BLOCKED on one candidate must continue to the next endpoint instead of aborting the probe loop. Both guards fail on the pre-fix code. Test-only additions cohesive with the existing provider-models discovery suite (shared seedConnection/callRoute harness); not separately extractable without duplicating the harness.", "_rebaseline_2026_06_19_4293_codex_spark_scope": "PR #4293 (isolate Codex Spark quota scope) own growth, MEASURED on the actual merged tree (release/v3.8.30 + #4293). Production: auth.ts 2219->2279 (+60) threads requestedModel into Codex quota-policy/headroom/preflight/P2C scoring so normal Codex and GPT-5.3-Codex-Spark windows are evaluated independently; chatCore.ts 5116->5125 (+9) passes the failing model scope into Codex 429 failover (markCodexScopeRateLimited) instead of a connection-wide rateLimitedUntil write; accountFallback.ts 1727->1731 (+4) scopes Codex model-lock keys to codex vs spark. Heavy parsing/display logic lives in new leaf helpers under the cap (codexQuotaScopes.ts, codexUsageQuotas.ts, codexFailover.ts). Tests: account-fallback-service 1544->1569, executor-codex 1336->1339, sse-auth 1527->1553, usage-service-hardening 1612->1633 (added Spark-scope regression coverage). Cohesive wiring at existing selection/failover lockout boundaries; not extractable.", "_rebaseline_2026_06_20_4447_openai_gpt41mini_o_mini_pricing": "PR #4447 own growth: pricing.ts 1592->1620 (+28 = pure-data pricing rows closing the null/$0 gap for registry-exposed OpenAI ids gpt-4.1-mini, gpt-4.1-nano, o3-mini, o4-mini that tripped the catalog pricing gate; getPricingForModel does an exact lookup, so a missing key resolves to null. Official OpenAI per-1M prices + the table's derived-field convention (reasoning=output*1.5, cache_creation=input, cached=official). Restore-green for a pre-existing release/v3.8.32 red surfaced by #4432's __RUN_ALL__ run. Cohesive data; not extractable.", "_rebaseline_2026_06_20_web_cookie_validator_shadow_fix": "validation.ts 4518->4522 (+4 = move the generic web-cookie validateWebCookieProvider dispatch from the TOP of validateProviderApiKey to a FALLBACK after SPECIALTY_VALIDATORS, plus a comment, so #4023's generic AUTH_007 ping no longer shadows the rich per-provider validators (grok-web #3474 IP-reputation/Cloudflare, chatgpt-web cf-mitigated, claude/gemini/copilot/qwen/t3-web). Restores provider-validation-specialty.test.ts (112/112) while keeping web-cookie-auth007 (5/5). Behavior fix at an existing dispatch boundary; not extractable.", diff --git a/src/app/api/providers/[id]/models/route.ts b/src/app/api/providers/[id]/models/route.ts index c217c45636..532edc74dd 100755 --- a/src/app/api/providers/[id]/models/route.ts +++ b/src/app/api/providers/[id]/models/route.ts @@ -533,7 +533,9 @@ export async function GET( base = base.slice(0, -17); } else if (base.endsWith("/completions")) { base = base.slice(0, -12); - } else if (base.endsWith("/v1")) { + } + + if (base.endsWith("/v1") && !base.endsWith("://v1")) { base = base.slice(0, -3); } @@ -576,6 +578,11 @@ export async function GET( } } catch (err: any) { if (err.message === "auth_failed") break; // Don't try other endpoints if auth failed + + if (err?.code === "REDIRECT_BLOCKED") { + continue; // Try next endpoint + } + const status = getSafeOutboundFetchErrorStatus(err); if (status) { throw err; diff --git a/tests/unit/provider-models-route.test.ts b/tests/unit/provider-models-route.test.ts index 0dd23c0f6e..f83c81e5a3 100644 --- a/tests/unit/provider-models-route.test.ts +++ b/tests/unit/provider-models-route.test.ts @@ -320,6 +320,130 @@ test("provider models route discovers SiliconFlow models from configured China b ]); }); +test("provider models route handles local hostnames named 'v1' correctly", async () => { + const connection = await seedConnection("openai-compatible-local-v1", { + apiKey: "sk-local", + providerSpecificData: { + baseUrl: "http://v1/chat/completions", + }, + }); + const seenUrls: string[] = []; + + globalThis.fetch = async (url) => { + seenUrls.push(String(url)); + return Response.json({ + data: [{ id: "local-v1-model", name: "Local v1 Model" }], + }); + }; + + const response = await callRoute(connection.id); + const body = (await response.json()) as any; + + assert.equal(response.status, 200); + assert.equal(body.source, "api"); + assert.deepEqual(seenUrls, ["http://v1/v1/models"]); +}); + +test("provider models route correctly strips standard /v1 paths", async () => { + const connection = await seedConnection("openai-compatible-standard-v1", { + apiKey: "sk-standard", + providerSpecificData: { + baseUrl: "https://api.openai.com/v1", + }, + }); + const seenUrls: string[] = []; + + globalThis.fetch = async (url) => { + seenUrls.push(String(url)); + return Response.json({ + data: [{ id: "standard-model", name: "Standard Model" }], + }); + }; + + const response = await callRoute(connection.id); + const body = (await response.json()) as any; + + assert.equal(response.status, 200); + assert.equal(body.source, "api"); + assert.deepEqual(seenUrls, ["https://api.openai.com/v1/models"]); +}); + +test("provider models route strips /v1 when it precedes /chat/completions (#5899 no double /v1)", async () => { + // Regression for #5899 (Api Airforce): a baseUrl of the form + // "https://api.airforce/v1/chat/completions" must probe ".../v1/models" — NOT + // ".../v1/v1/models". The old `else if` strip chain only removed + // "/chat/completions", leaving a trailing "/v1" that the endpoint builder then + // doubled, producing a 308 redirect that aborted discovery. + const connection = await seedConnection("openai-compatible-airforce-v1", { + apiKey: "sk-airforce", + providerSpecificData: { + baseUrl: "https://api.airforce/v1/chat/completions", + }, + }); + const seenUrls: string[] = []; + + globalThis.fetch = async (url) => { + seenUrls.push(String(url)); + return Response.json({ + data: [{ id: "airforce-model", name: "Airforce Model" }], + }); + }; + + const response = await callRoute(connection.id); + const body = (await response.json()) as any; + + assert.equal(response.status, 200); + assert.equal(body.source, "api"); + // First probed endpoint must have a single /v1 — no ".../v1/v1/models". + assert.equal(seenUrls[0], "https://api.airforce/v1/models"); + assert.ok( + !seenUrls.some((u) => u.includes("/v1/v1/")), + `no endpoint should contain a doubled /v1: ${JSON.stringify(seenUrls)}` + ); +}); + +test("provider models route continues probing past a REDIRECT_BLOCKED endpoint (#5899)", async () => { + // Regression for #5899: a REDIRECT_BLOCKED error on one candidate endpoint must + // not abort the whole probe loop — discovery should fall through to the next + // endpoint instead of surfacing an empty catalog. + const connection = await seedConnection("openai-compatible-redirect-v1", { + apiKey: "sk-redirect", + providerSpecificData: { + baseUrl: "https://redirect.example", + }, + }); + const seenUrls: string[] = []; + + globalThis.fetch = async (url) => { + const u = String(url); + seenUrls.push(u); + // First candidate ".../v1/models" answers with a real 308 redirect → + // safeOutboundFetch throws a SafeOutboundFetchError(REDIRECT_BLOCKED). The old + // code re-threw on it (status 503) and aborted the loop; the fix `continue`s. + if (u === "https://redirect.example/v1/models") { + return new Response(null, { + status: 308, + headers: { location: "https://redirect.example/models" }, + }); + } + return Response.json({ + data: [{ id: "redirect-model", name: "Redirect Model" }], + }); + }; + + const response = await callRoute(connection.id); + const body = (await response.json()) as any; + + assert.equal(response.status, 200); + // Without the REDIRECT_BLOCKED `continue`, discovery aborted and fell back to a + // non-api catalog. The fix lets it reach the next endpoint and return live models. + assert.equal(body.source, "api"); + assert.ok( + seenUrls.length >= 2, + `expected the loop to continue past REDIRECT_BLOCKED: ${JSON.stringify(seenUrls)}` + ); +}); + test("provider models route returns static catalog entries for providers with hardcoded models", async () => { const connection = await seedConnection("bailian-coding-plan", { apiKey: "bailian-key",