mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 21:32:10 +03:00
fix(api): provider-models route — redirect→local-catalog fallback + custom-model merge (#6267, #6247) (#6449)
This commit is contained in:
committed by
GitHub
parent
cd4a720b7e
commit
2f4b793c3d
@@ -21,6 +21,9 @@
|
||||
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
- **fix(api):** the per-connection provider models route now degrades to the shipped catalog when a provider's `/models` endpoint answers with a redirect ([#6267](https://github.com/diegosouzapw/OmniRoute/issues/6267)) — a `qwen-web` import failed with a raw `Redirect blocked … (307)` 503. `safeOutboundFetch` throws `REDIRECT_BLOCKED` on the 307, `getSafeOutboundFetchErrorStatus` maps it to 503, and `buildDiscoveryErrorFallbackResponse` treated every 503 as a hard error — so the non-empty `getModelsByProviderId("qwen-web")` catalog was never surfaced. A models-endpoint redirect is not a fixable-config error (unlike `URL_GUARD_BLOCKED`/`INVALID_URL`, which stay hard errors), so it now falls back to the local/cached catalog before the 503 short-circuit. General fix — covers any config-driven provider that 307s. Regression guard: `tests/unit/provider-models-qwen-web-redirect-6267.test.ts`. (thanks @chirag127)
|
||||
- **fix(api):** the per-connection provider models route (MCP `list_models_catalog` + the dashboard import view) now merges USER-ADDED custom models into its response ([#6247](https://github.com/diegosouzapw/OmniRoute/issues/6247)) — custom models live in the `key_value` namespace `customModels`, which the live REST `/api/v1/models` already merges, but `src/app/api/providers/[id]/models/route.ts` never read `getCustomModels`, so custom models were dropped on both the discovery-success and local_catalog paths. They are now folded into the returned model list (deduped by id, stamped `owned_by: provider`), fixing MCP + the dashboard import view in one place. Regression guard: `tests/unit/provider-models-custom-merge-6247.test.ts`. (thanks @RCrushMe)
|
||||
|
||||
- **fix(providers):** GitLab Duo tool-calling follow-up turns no longer fail upstream with `422 {"detail":"Validation error"}` (tokens 0/0, rejected pre-inference). The #6234 tool-result-feedback fix serialized the **entire** multi-turn conversation into GitLab's single-file `code_suggestions` (`small_file`) generation endpoint — folded history that turn-N sent as an oversized `current_file.content_above_cursor` **and** duplicated verbatim into `user_instruction`, tripping the AI-Gateway's `small_file` validation guard. The executor now **bounds** that prompt: it keeps system + latest user message + the most-recent tool round (dropping older turns), caps oversized tool results, and stops duplicating the full prompt into `user_instruction` (which now carries only the short latest user message) — while still feeding the most-recent tool result back so the agent continues (`open-sse/executors/gitlab.ts`, [#6220](https://github.com/diegosouzapw/OmniRoute/issues/6220)). The unit test covers the bounding logic; the upstream 422→200 clearing is VPS-only (Hard Rule #18). Regression guard: `tests/unit/gitlab-tool-exchange-bounded-6220.test.ts`.
|
||||
|
||||
- **fix(i18n):** the provider-detail (`/dashboard/providers/[id]`) connection-status filter labels no longer render as `__MISSING__:All` / `__MISSING__:Active` / `__MISSING__:Error` / `__MISSING__:Banned` / `__MISSING__:CreditsExhausted` in non-English locales (notably pt-BR) ([#6290](https://github.com/diegosouzapw/OmniRoute/issues/6290)). Root cause was **not** the namespace mismatch the issue guessed — the `providers.filter*` keys resolve correctly in `en.json`; the debt lived in the locale mirrors (`src/i18n/messages/*.json`), where these five keys carried the `__MISSING__:` sync sentinel in ~15 locales and were absent entirely in ~26 others, so next-intl found the key and echoed the sentinel verbatim. All 40 non-English/-Chinese mirrors now ship real translations for the five `providers.filter*` labels. Regression guard: `tests/unit/i18n-provider-filter-keys-6290.test.ts`. (thanks @diegosouzapw)
|
||||
|
||||
@@ -71,7 +71,7 @@ import {
|
||||
parseGeminiModelsList,
|
||||
type GeminiDiscoveryModel,
|
||||
} from "@/lib/providerModels/geminiModelsParser";
|
||||
import { getSyncedAvailableModels } from "@/lib/db/models";
|
||||
import { getSyncedAvailableModels, getCustomModels } from "@/lib/db/models";
|
||||
import { fetchCursorAgentModels } from "@/lib/providerModels/cursorAgent";
|
||||
import {
|
||||
type JsonRecord,
|
||||
@@ -212,7 +212,38 @@ export async function GET(
|
||||
// Resolve proxy for this provider (provider-level → global → direct)
|
||||
const proxy = await resolveProxyForProvider(provider);
|
||||
|
||||
// #6247 — user-added custom models live in key_value namespace `customModels`
|
||||
// (getCustomModels). The live REST /api/v1/models merges them, but this
|
||||
// per-connection route (used by MCP list_models_catalog + the dashboard
|
||||
// import view) never did, so custom models were dropped on both the
|
||||
// discovery-success and local_catalog paths. Read them once here and fold
|
||||
// them into every models response via buildResponse below (dedup by id).
|
||||
let customModelsForProvider: Array<{ id: string; name?: string }> = [];
|
||||
try {
|
||||
const custom = await getCustomModels(provider);
|
||||
if (Array.isArray(custom)) {
|
||||
customModelsForProvider = custom as Array<{ id: string; name?: string }>;
|
||||
}
|
||||
} catch {
|
||||
// DB unavailable — proceed without custom models.
|
||||
}
|
||||
|
||||
const mergeCustomModels = (models: any[]) => {
|
||||
if (customModelsForProvider.length === 0) return models;
|
||||
const base = Array.isArray(models) ? models : [];
|
||||
const existing = new Set(
|
||||
base.map((m) => (m && typeof m.id === "string" ? m.id : null)).filter(Boolean)
|
||||
);
|
||||
const extra = customModelsForProvider
|
||||
.filter((m) => m && typeof m.id === "string" && m.id.length > 0 && !existing.has(m.id))
|
||||
.map((m) => ({ id: m.id, name: m.name || m.id, owned_by: provider }));
|
||||
return extra.length > 0 ? [...base, ...extra] : base;
|
||||
};
|
||||
|
||||
const buildResponse = (payload: any, statusConfig?: ResponseInit) => {
|
||||
if (payload.models && Array.isArray(payload.models)) {
|
||||
payload.models = mergeCustomModels(payload.models);
|
||||
}
|
||||
if (excludeHidden && payload.models && Array.isArray(payload.models)) {
|
||||
payload.models = payload.models.filter((m: any) => !getModelIsHidden(provider, m.id));
|
||||
}
|
||||
@@ -316,6 +347,16 @@ export async function GET(
|
||||
localWarning?: string;
|
||||
}
|
||||
) => {
|
||||
// #6267 — a models-endpoint redirect (307/308) is not a fixable-config
|
||||
// error. safeOutboundFetch throws REDIRECT_BLOCKED which
|
||||
// getSafeOutboundFetchErrorStatus maps to 503, but unlike the other 503
|
||||
// cases (URL_GUARD_BLOCKED / INVALID_URL, which are genuinely
|
||||
// unrecoverable and stay hard errors) a blocked redirect should degrade to
|
||||
// the local/cached catalog OmniRoute ships instead of surfacing a raw 503.
|
||||
// General fix — covers any config-driven provider that 307s (e.g. qwen-web).
|
||||
if (error instanceof SafeOutboundFetchError && error.code === "REDIRECT_BLOCKED") {
|
||||
return buildDiscoveryFallbackResponse(warnings);
|
||||
}
|
||||
const status = getSafeOutboundFetchErrorStatus(error);
|
||||
if (status === 400 || status === 503 || status === 504) return null;
|
||||
return buildDiscoveryFallbackResponse(warnings);
|
||||
|
||||
100
tests/unit/provider-models-custom-merge-6247.test.ts
Normal file
100
tests/unit/provider-models-custom-merge-6247.test.ts
Normal file
@@ -0,0 +1,100 @@
|
||||
// #6247 regression guard — the per-connection /api/providers/[id]/models route
|
||||
// (used by MCP list_models_catalog + the dashboard import view) must include
|
||||
// USER-ADDED custom models, not just the static/discovered catalog.
|
||||
//
|
||||
// Root cause: the route never read getCustomModels(provider) (custom models live
|
||||
// in key_value namespace `customModels`), so the live REST /api/v1/models merged
|
||||
// them but the per-connection route did not — on both the local_catalog and the
|
||||
// discovery-success paths. Fix: merge getCustomModels(provider) (dedup by id,
|
||||
// owned_by: provider) into the route's returned model list.
|
||||
//
|
||||
// Harness copied (minimal) from tests/unit/provider-models-route.test.ts — the
|
||||
// frozen file's own note says the seedConnection/callRoute harness is not
|
||||
// separately extractable, so a small local copy is acceptable.
|
||||
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-custom-merge-"));
|
||||
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 modelsDb = await import("../../src/lib/db/models.ts");
|
||||
const providerModelsRoute = await import("../../src/app/api/providers/[id]/models/route.ts");
|
||||
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
async function resetStorage() {
|
||||
globalThis.fetch = originalFetch;
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
interface SeedOverrides {
|
||||
authType?: string;
|
||||
name?: string;
|
||||
apiKey?: string;
|
||||
accessToken?: string;
|
||||
isActive?: boolean;
|
||||
testStatus?: string;
|
||||
providerSpecificData?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
async function seedConnection(provider: string, overrides: SeedOverrides = {}) {
|
||||
return providersDb.createProviderConnection({
|
||||
provider,
|
||||
authType: overrides.authType || "apikey",
|
||||
name: overrides.name || `${provider}-${Math.random().toString(16).slice(2, 8)}`,
|
||||
apiKey: overrides.apiKey,
|
||||
accessToken: overrides.accessToken,
|
||||
isActive: overrides.isActive ?? true,
|
||||
testStatus: overrides.testStatus || "active",
|
||||
providerSpecificData: overrides.providerSpecificData || {},
|
||||
});
|
||||
}
|
||||
|
||||
async function callRoute(connectionId: string, search = "") {
|
||||
return providerModelsRoute.GET(
|
||||
new Request(`http://localhost/api/providers/${connectionId}/models${search}`),
|
||||
{ params: { id: connectionId } }
|
||||
);
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
globalThis.fetch = originalFetch;
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("per-connection models route includes user-added custom models on the local_catalog path (#6247)", async () => {
|
||||
const connection = await seedConnection("aimlapi", { apiKey: "aiml-key" });
|
||||
|
||||
// A user-added custom model persisted under key_value namespace `customModels`.
|
||||
await modelsDb.addCustomModel("aimlapi", "my-org/custom-model-6247", "My Custom 6247");
|
||||
|
||||
// Force the local_catalog fallback: the live models probe fails, so the route
|
||||
// serves the local catalog (source local_catalog) — the path that dropped
|
||||
// custom models before the fix.
|
||||
globalThis.fetch = (async () => new Response("upstream down", { status: 500 })) as typeof fetch;
|
||||
|
||||
const response = await callRoute(connection.id);
|
||||
const body = (await response.json()) as {
|
||||
source?: string;
|
||||
models?: Array<{ id: string; owned_by?: string }>;
|
||||
};
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(body.source, "local_catalog");
|
||||
const custom = (body.models || []).find((m) => m.id === "my-org/custom-model-6247");
|
||||
// RED before the fix: the custom model is absent (route never read getCustomModels).
|
||||
assert.ok(custom, "user-added custom model must appear in the per-connection catalog");
|
||||
assert.equal(custom.owned_by, "aimlapi", "custom model must be stamped owned_by = provider");
|
||||
});
|
||||
103
tests/unit/provider-models-qwen-web-redirect-6267.test.ts
Normal file
103
tests/unit/provider-models-qwen-web-redirect-6267.test.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
// #6267 regression guard — a config-driven provider whose /models endpoint 307s
|
||||
// must degrade to the local catalog OmniRoute ships, not surface a raw 503.
|
||||
//
|
||||
// Root cause: safeOutboundFetch throws REDIRECT_BLOCKED on the 307 →
|
||||
// getSafeOutboundFetchErrorStatus maps it to 503 → buildDiscoveryErrorFallbackResponse
|
||||
// returned null for status 503 → re-throw → raw 503, hiding the non-empty
|
||||
// getModelsByProviderId("qwen-web") catalog. Fix: treat REDIRECT_BLOCKED as a
|
||||
// non-fixable-config error that degrades to the cached/local catalog.
|
||||
//
|
||||
// Harness copied (minimal) from tests/unit/provider-models-route.test.ts — the
|
||||
// frozen file's own note says the seedConnection/callRoute harness is not
|
||||
// separately extractable, so a small local copy is acceptable.
|
||||
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-qwen-web-redirect-"));
|
||||
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 providerModelsRoute = await import("../../src/app/api/providers/[id]/models/route.ts");
|
||||
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
async function resetStorage() {
|
||||
globalThis.fetch = originalFetch;
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
interface SeedOverrides {
|
||||
authType?: string;
|
||||
name?: string;
|
||||
apiKey?: string;
|
||||
accessToken?: string;
|
||||
isActive?: boolean;
|
||||
testStatus?: string;
|
||||
providerSpecificData?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
async function seedConnection(provider: string, overrides: SeedOverrides = {}) {
|
||||
return providersDb.createProviderConnection({
|
||||
provider,
|
||||
authType: overrides.authType || "apikey",
|
||||
name: overrides.name || `${provider}-${Math.random().toString(16).slice(2, 8)}`,
|
||||
apiKey: overrides.apiKey,
|
||||
accessToken: overrides.accessToken,
|
||||
isActive: overrides.isActive ?? true,
|
||||
testStatus: overrides.testStatus || "active",
|
||||
providerSpecificData: overrides.providerSpecificData || {},
|
||||
});
|
||||
}
|
||||
|
||||
async function callRoute(connectionId: string, search = "") {
|
||||
return providerModelsRoute.GET(
|
||||
new Request(`http://localhost/api/providers/${connectionId}/models${search}`),
|
||||
{ params: { id: connectionId } }
|
||||
);
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
globalThis.fetch = originalFetch;
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("qwen-web model import degrades to the local catalog when the /models endpoint 307s (#6267)", async () => {
|
||||
// A configured apiKey ensures the token gate passes and the config-driven
|
||||
// fetch is actually attempted (so we exercise the redirect path, not the
|
||||
// no-token fallback).
|
||||
const connection = await seedConnection("qwen-web", { apiKey: "qwen-web-cookie" });
|
||||
|
||||
// Upstream answers the models probe with a 307 to the login page — the exact
|
||||
// shape safeOutboundFetch rejects with REDIRECT_BLOCKED.
|
||||
globalThis.fetch = (async () =>
|
||||
new Response(null, {
|
||||
status: 307,
|
||||
headers: { location: "https://chat.qwen.ai/login" },
|
||||
})) as typeof fetch;
|
||||
|
||||
const response = await callRoute(connection.id);
|
||||
const body = (await response.json()) as {
|
||||
source?: string;
|
||||
models?: Array<{ id: string }>;
|
||||
};
|
||||
|
||||
// RED before the fix: raw 503 (Redirect blocked … (307)).
|
||||
assert.equal(response.status, 200, "a redirect on the models endpoint must not surface a 503");
|
||||
assert.equal(body.source, "local_catalog", "should fall back to the shipped catalog");
|
||||
const ids = (body.models || []).map((m) => m.id);
|
||||
assert.ok(
|
||||
ids.includes("qwen3.7-max"),
|
||||
`qwen-web catalog should be surfaced; got: ${ids.join(", ")}`
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user