mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(api): warn (never reject) when a combo name shadows a real model id (#8530)
POST /api/combos and PUT /api/combos/[id] had zero validation or observability when a combo name collided with a real model id, and sseModelService.getComboForModel() always resolves the combo first. That combo-first precedence is not a bug: #6940 documents a combo named after a bare model id (e.g. `gpt-5.5`) as the supported mechanism for per-model provider fallback, reusing the #3227/#3233 machinery and covered by tests/unit/responses-combo-resolution-3227.test.ts and tests/unit/combo-name-codex-responses-rewrite.test.ts. Hard-rejecting a colliding name (as #8530's literal acceptance criteria requested) would regress that documented workflow. Instead, both routes now attach a non-blocking `warning` field (`COMBO_NAME_SHADOWS_MODEL`) to the create/rename response when the name collides with a real model id, and a new boot-time scan (scanComboModelNameCollisionsAtBoot in src/instrumentation-node.ts) logs a startup warning enumerating existing collisions — so an operator who hits this by accident has a signal, while the #6940-sanctioned pattern keeps working exactly as before. New tests/unit/combo-model-name-collision-8530.test.ts proves both: the sanctioned shapes (create/rename to a colliding name) still return 201/200 with the warning attached, and non-colliding names get no warning field at all.
This commit is contained in:
1
changelog.d/fixes/8530-combo-model-name-collision.md
Normal file
1
changelog.d/fixes/8530-combo-model-name-collision.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(api): surface a non-blocking warning + startup scan when a combo name shadows a real model id, instead of silently routing with zero signal (#8530)
|
||||
@@ -128,6 +128,37 @@ Auto-scoring selects best provider/model per request
|
||||
| `src/sse/handlers/chat.ts` | Integration: auto prefix short-circuit |
|
||||
| `src/shared/constants/providers.ts` | `SYSTEM_PROVIDERS.auto` system entry |
|
||||
|
||||
## Combo Names That Match a Real Model Id
|
||||
|
||||
A combo whose `name` is identical to a bare model id (e.g. a combo named
|
||||
`gpt-5.5`) is an **intentional, supported pattern**, not a bug: it is the
|
||||
mechanism for per-model-id provider fallback documented in
|
||||
[#6940](https://github.com/diegosouzapw/OmniRoute/issues/6940). Because combo
|
||||
resolution is checked before bare-model-id resolution
|
||||
(`getComboForModel()` in `src/sse/services/model.ts`), a request for the bare
|
||||
id `gpt-5.5` is routed through the combo's targets (e.g.
|
||||
`acme-responses/gpt-5.5`, `backup-responses/gpt-5.5`) instead of straight to
|
||||
a single provider — this reuses the combo-before-rewrite precedence built for
|
||||
[#3227/#3233](https://github.com/diegosouzapw/OmniRoute/issues/3227) and is
|
||||
regression-tested by `tests/unit/responses-combo-resolution-3227.test.ts` and
|
||||
`tests/unit/combo-name-codex-responses-rewrite.test.ts`.
|
||||
|
||||
Creating or renaming a combo to a name that shadows a real model id is
|
||||
**never rejected** — doing so would break this documented workflow. Instead
|
||||
(#8530), `POST /api/combos` and `PUT /api/combos/[id]` attach a non-blocking
|
||||
`warning` field to the response when the (new) name collides with a real
|
||||
model id:
|
||||
|
||||
```json
|
||||
{ "warning": { "code": "COMBO_NAME_SHADOWS_MODEL", "modelId": "gpt-5.5", "providerId": "openai" } }
|
||||
```
|
||||
|
||||
At boot, `scanComboModelNameCollisionsAtBoot()`
|
||||
(`src/instrumentation-node.ts`) also logs a one-line `[STARTUP]` warning
|
||||
enumerating every existing combo that shadows a model id, so operators who
|
||||
hit this by accident (rather than intentionally, per #6940) have a signal.
|
||||
The detection helper lives in `src/lib/combos/modelNameCollision.ts`.
|
||||
|
||||
## How It Works (Persisted Auto-Combos)
|
||||
|
||||
The Auto-Combo Engine dynamically selects the best provider/model for each request using a **12-factor scoring function** (defined in `open-sse/services/autoCombo/scoring.ts` → `DEFAULT_WEIGHTS`). All weights sum to **1.0**.
|
||||
|
||||
@@ -18,6 +18,7 @@ import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { QUOTA_MODEL_PREFIX } from "@/lib/quota/quotaModelNaming";
|
||||
import { comboErrorResponse } from "@/lib/api/comboErrorResponse";
|
||||
import { ComboInvariantError } from "@/lib/combos/invariants";
|
||||
import { buildComboNameCollisionWarning } from "@/lib/combos/modelNameCollision";
|
||||
|
||||
// Minimal shape for the fields we read off a combo row in this route.
|
||||
// `getComboById` returns a structurally `JsonRecord`-typed object, so we
|
||||
@@ -249,7 +250,13 @@ export async function PUT(request, { params }) {
|
||||
// Auto sync to Cloud if enabled
|
||||
await syncToCloudIfEnabled();
|
||||
|
||||
return NextResponse.json(combo);
|
||||
// #8530: a combo renamed to a real model id is a supported pattern
|
||||
// (#6940 — bare-model-id provider fallback), so it is never rejected.
|
||||
// Surface it as a non-blocking warning instead of silently shadowing it.
|
||||
const warning = comboName
|
||||
? buildComboNameCollisionWarning(String(comboName))
|
||||
: null;
|
||||
return NextResponse.json(warning ? { ...combo, warning } : combo);
|
||||
} catch (error) {
|
||||
if (error instanceof ComboInvariantError) {
|
||||
return comboErrorResponse("COMBO_008", 400, { reason: error.message }, request);
|
||||
|
||||
@@ -17,6 +17,7 @@ import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { comboErrorResponse } from "@/lib/api/comboErrorResponse";
|
||||
import { computeComboContextLength } from "@/lib/combos/comboContext";
|
||||
import { ComboInvariantError } from "@/lib/combos/invariants";
|
||||
import { buildComboNameCollisionWarning } from "@/lib/combos/modelNameCollision";
|
||||
|
||||
// GET /api/combos - Get all combos
|
||||
export async function GET(request: Request) {
|
||||
@@ -120,7 +121,12 @@ export async function POST(request) {
|
||||
// Auto sync to Cloud if enabled
|
||||
await syncToCloudIfEnabled();
|
||||
|
||||
return NextResponse.json(combo, { status: 201 });
|
||||
// #8530: a combo named after a real model id is a supported pattern
|
||||
// (#6940 — bare-model-id provider fallback), so it is never rejected.
|
||||
// Surface it as a non-blocking warning so the dashboard/API caller can
|
||||
// confirm it was intentional instead of silently shadowing the model.
|
||||
const warning = buildComboNameCollisionWarning(name);
|
||||
return NextResponse.json(warning ? { ...combo, warning } : combo, { status: 201 });
|
||||
} catch (error) {
|
||||
if (error instanceof ComboInvariantError) {
|
||||
return comboErrorResponse("COMBO_008", 400, { reason: error.message }, request);
|
||||
|
||||
@@ -212,6 +212,36 @@ export async function warmModelCatalogCache(): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* #8530: enumerate existing combos whose name shadows a real model id and
|
||||
* log a startup warning. Never rejects/throws — #6940 documents a combo
|
||||
* named after a bare model id as the supported mechanism for per-model
|
||||
* provider fallback, so a collision here is expected in some deployments;
|
||||
* this only gives operators who hit it accidentally a signal.
|
||||
*
|
||||
* Exported (rather than left inline in registerNodejs()) so it can be unit
|
||||
* tested directly without exercising the rest of the startup sequence.
|
||||
*/
|
||||
export async function scanComboModelNameCollisionsAtBoot(): Promise<void> {
|
||||
try {
|
||||
const [{ getCombos }, { scanCombosForModelCollisions }] = await Promise.all([
|
||||
import("@/lib/db/combos"),
|
||||
import("@/lib/combos/modelNameCollision"),
|
||||
]);
|
||||
const collisions = scanCombosForModelCollisions(await getCombos());
|
||||
if (collisions.length > 0) {
|
||||
console.warn(
|
||||
`[STARTUP] ${collisions.length} combo(s) share a name with a real model id (#8530) — ` +
|
||||
"intentional per #6940 bare-model-id fallback, but confirm each is expected: " +
|
||||
collisions.map((c) => `${c.comboName}→${c.providerId}/${c.modelId}`).join(", ")
|
||||
);
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
console.warn("[STARTUP] Could not scan combos for model-name collisions (non-fatal):", msg);
|
||||
}
|
||||
}
|
||||
|
||||
export async function registerNodejs(): Promise<void> {
|
||||
markServerStarting();
|
||||
|
||||
@@ -264,6 +294,8 @@ export async function registerNodejs(): Promise<void> {
|
||||
console.warn("[STARTUP] Could not clear stale crash cooldowns (non-fatal):", msg);
|
||||
}
|
||||
|
||||
await scanComboModelNameCollisionsAtBoot();
|
||||
|
||||
const [
|
||||
{ initGracefulShutdown },
|
||||
{ initApiBridgeServer },
|
||||
|
||||
95
src/lib/combos/modelNameCollision.ts
Normal file
95
src/lib/combos/modelNameCollision.ts
Normal file
@@ -0,0 +1,95 @@
|
||||
/**
|
||||
* Combo-name / model-id collision detection (#8530).
|
||||
*
|
||||
* #6940 (closed by the maintainer) documents a combo named identically to a
|
||||
* bare model id — e.g. combo `gpt-5.5` fanning out to
|
||||
* `acme-responses/gpt-5.5`, `backup-responses/gpt-5.5` — as THE supported
|
||||
* mechanism for per-model provider fallback on bare Responses model ids
|
||||
* (reusing the #3227/#3233 combo-before-rewrite precedence, which is
|
||||
* regression-tested in `tests/unit/responses-combo-resolution-3227.test.ts`
|
||||
* and `tests/unit/combo-name-codex-responses-rewrite.test.ts`).
|
||||
*
|
||||
* So a colliding name is NOT rejected — it is a supported, intentional
|
||||
* pattern. This module only makes the collision *observable*: callers use it
|
||||
* to attach a non-blocking warning to the create/rename response and to the
|
||||
* boot-time scan, instead of silently shadowing the model with zero signal.
|
||||
*/
|
||||
import { PROVIDER_MODELS } from "@/shared/constants/models";
|
||||
|
||||
export interface ComboModelCollision {
|
||||
providerId: string;
|
||||
modelId: string;
|
||||
}
|
||||
|
||||
let cachedIndex: Map<string, ComboModelCollision> | null = null;
|
||||
|
||||
/**
|
||||
* Bare model id -> first provider that registers it. Built once from the
|
||||
* (lazily-generated, then cached) provider registry and memoized for the
|
||||
* process lifetime — the registry is static compiled-in config, not
|
||||
* runtime/DB state, so it never needs invalidation.
|
||||
*/
|
||||
function getModelIdIndex(): Map<string, ComboModelCollision> {
|
||||
if (cachedIndex) return cachedIndex;
|
||||
const index = new Map<string, ComboModelCollision>();
|
||||
for (const [providerId, models] of Object.entries(PROVIDER_MODELS)) {
|
||||
for (const model of models) {
|
||||
if (!index.has(model.id)) {
|
||||
index.set(model.id, { providerId, modelId: model.id });
|
||||
}
|
||||
}
|
||||
}
|
||||
cachedIndex = index;
|
||||
return index;
|
||||
}
|
||||
|
||||
/** Test-only: force the memoized registry index to rebuild on next call. */
|
||||
export function __resetModelNameCollisionCacheForTest(): void {
|
||||
cachedIndex = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the provider that registers `name` as a bare model id, or `null`
|
||||
* when `name` does not collide with any known real model id.
|
||||
*/
|
||||
export function findCollidingModel(name: string): ComboModelCollision | null {
|
||||
if (!name) return null;
|
||||
return getModelIdIndex().get(name) ?? null;
|
||||
}
|
||||
|
||||
/** Machine-readable warning payload attached to POST/PUT combo responses. */
|
||||
export function buildComboNameCollisionWarning(
|
||||
name: string
|
||||
): { code: "COMBO_NAME_SHADOWS_MODEL"; modelId: string; providerId: string } | null {
|
||||
const collision = findCollidingModel(name);
|
||||
if (!collision) return null;
|
||||
return {
|
||||
code: "COMBO_NAME_SHADOWS_MODEL",
|
||||
modelId: collision.modelId,
|
||||
providerId: collision.providerId,
|
||||
};
|
||||
}
|
||||
|
||||
/** Shape of the minimal combo record the boot-time scan needs. */
|
||||
export interface ComboLike {
|
||||
name?: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Boot-time scan (see `src/instrumentation-node.ts`): enumerates existing
|
||||
* combos whose name shadows a real model id, for a startup log — never a
|
||||
* hard failure, since the shadowing pattern is intentional per #6940.
|
||||
*/
|
||||
export function scanCombosForModelCollisions(
|
||||
combos: readonly ComboLike[]
|
||||
): Array<{ comboName: string; providerId: string; modelId: string }> {
|
||||
const results: Array<{ comboName: string; providerId: string; modelId: string }> = [];
|
||||
for (const combo of combos) {
|
||||
if (typeof combo.name !== "string" || combo.name.length === 0) continue;
|
||||
const collision = findCollidingModel(combo.name);
|
||||
if (collision) {
|
||||
results.push({ comboName: combo.name, ...collision });
|
||||
}
|
||||
}
|
||||
return results;
|
||||
}
|
||||
190
tests/unit/combo-model-name-collision-8530.test.ts
Normal file
190
tests/unit/combo-model-name-collision-8530.test.ts
Normal file
@@ -0,0 +1,190 @@
|
||||
// #8530 — combo name / model id collision guard.
|
||||
//
|
||||
// #6940 (closed by the maintainer) documents a combo named identically to a
|
||||
// bare model id (e.g. combo `gpt-5.5`) as THE supported mechanism for
|
||||
// per-model provider fallback — see `tests/unit/responses-combo-resolution-3227.test.ts`
|
||||
// and `tests/unit/combo-name-codex-responses-rewrite.test.ts` for the
|
||||
// combo-before-rewrite precedence this relies on. So creation/rename must
|
||||
// NEVER hard-reject a colliding name (that would regress #6940); it must
|
||||
// only make the collision observable via a non-blocking `warning` field.
|
||||
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-combo-collision-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const combosDb = await import("../../src/lib/db/combos.ts");
|
||||
const createRoute = await import("../../src/app/api/combos/route.ts");
|
||||
const comboRoute = await import("../../src/app/api/combos/[id]/route.ts");
|
||||
const collision = await import("../../src/lib/combos/modelNameCollision.ts");
|
||||
|
||||
// A model id that really is registered by multiple providers (verified via
|
||||
// PROVIDER_MODELS at write time — see open-sse/config/providers/*).
|
||||
const REAL_MODEL_ID = "gpt-5.5";
|
||||
// Not a registered bare model id anywhere in the provider registry.
|
||||
const NON_COLLIDING_NAME = "not-a-real-model-8530-guard-probe";
|
||||
|
||||
interface ComboResponseBody {
|
||||
name?: string;
|
||||
warning?: { code: string; modelId: string; providerId: string };
|
||||
}
|
||||
|
||||
async function resetStorage() {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
function makeCreateRequest(body: unknown) {
|
||||
return new Request("http://localhost/api/combos", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}
|
||||
|
||||
function makeUpdateRequest(body: unknown) {
|
||||
return new Request("http://localhost/api/combos/combo-1", {
|
||||
method: "PUT",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("POST /api/combos: name colliding with a real model id is created (#6940 pattern), with a warning", async () => {
|
||||
const response = await createRoute.POST(
|
||||
makeCreateRequest({
|
||||
name: REAL_MODEL_ID,
|
||||
strategy: "priority",
|
||||
models: [
|
||||
{ providerId: "codex", model: REAL_MODEL_ID },
|
||||
{ providerId: "openai", model: REAL_MODEL_ID },
|
||||
],
|
||||
})
|
||||
);
|
||||
const body = (await response.json()) as ComboResponseBody;
|
||||
|
||||
// Legitimate/sanctioned shape MUST still pass — never a 4xx (would regress #6940).
|
||||
assert.equal(response.status, 201);
|
||||
assert.equal(body.name, REAL_MODEL_ID);
|
||||
const stored = await combosDb.getComboByName(REAL_MODEL_ID);
|
||||
assert.equal(stored?.name, REAL_MODEL_ID);
|
||||
|
||||
// But it is now observable via a non-blocking warning.
|
||||
assert.equal(body.warning?.code, "COMBO_NAME_SHADOWS_MODEL");
|
||||
assert.equal(body.warning?.modelId, REAL_MODEL_ID);
|
||||
assert.equal(typeof body.warning?.providerId, "string");
|
||||
});
|
||||
|
||||
test("POST /api/combos: non-colliding name is created with no warning field at all", async () => {
|
||||
const response = await createRoute.POST(
|
||||
makeCreateRequest({
|
||||
name: NON_COLLIDING_NAME,
|
||||
strategy: "priority",
|
||||
models: [{ providerId: "claude", model: "claude-sonnet-4-6" }],
|
||||
})
|
||||
);
|
||||
const body = (await response.json()) as ComboResponseBody;
|
||||
|
||||
assert.equal(response.status, 201);
|
||||
assert.equal(body.name, NON_COLLIDING_NAME);
|
||||
assert.equal("warning" in body, false);
|
||||
});
|
||||
|
||||
test("PUT /api/combos/[id]: renaming to a real model id is applied (#6940 pattern), with a warning", async () => {
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "claude-plain",
|
||||
models: [{ provider: "claude", model: "claude-sonnet-4-6" }],
|
||||
});
|
||||
|
||||
const response = await comboRoute.PUT(makeUpdateRequest({ name: REAL_MODEL_ID }), {
|
||||
params: Promise.resolve({ id: combo.id }),
|
||||
});
|
||||
const body = (await response.json()) as ComboResponseBody;
|
||||
|
||||
// Legitimate/sanctioned rename MUST still pass — never a 4xx.
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(body.name, REAL_MODEL_ID);
|
||||
const stored = await combosDb.getComboByName(REAL_MODEL_ID);
|
||||
assert.equal(stored?.id, combo.id);
|
||||
|
||||
assert.equal(body.warning?.code, "COMBO_NAME_SHADOWS_MODEL");
|
||||
assert.equal(body.warning?.modelId, REAL_MODEL_ID);
|
||||
});
|
||||
|
||||
test("PUT /api/combos/[id]: renaming to a non-colliding name has no warning field", async () => {
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "claude-plain",
|
||||
models: [{ provider: "claude", model: "claude-sonnet-4-6" }],
|
||||
});
|
||||
|
||||
const response = await comboRoute.PUT(makeUpdateRequest({ name: NON_COLLIDING_NAME }), {
|
||||
params: Promise.resolve({ id: combo.id }),
|
||||
});
|
||||
const body = (await response.json()) as ComboResponseBody;
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(body.name, NON_COLLIDING_NAME);
|
||||
assert.equal("warning" in body, false);
|
||||
});
|
||||
|
||||
test("scanCombosForModelCollisions: reports existing combos that shadow a real model id", () => {
|
||||
const results = collision.scanCombosForModelCollisions([
|
||||
{ name: REAL_MODEL_ID },
|
||||
{ name: NON_COLLIDING_NAME },
|
||||
{ name: "" },
|
||||
]);
|
||||
assert.equal(results.length, 1);
|
||||
assert.equal(results[0].comboName, REAL_MODEL_ID);
|
||||
assert.equal(results[0].modelId, REAL_MODEL_ID);
|
||||
});
|
||||
|
||||
test("findCollidingModel: returns null for a name with no real-model-id collision", () => {
|
||||
assert.equal(collision.findCollidingModel(NON_COLLIDING_NAME), null);
|
||||
});
|
||||
|
||||
test("scanComboModelNameCollisionsAtBoot: logs a startup warning enumerating existing collisions", async () => {
|
||||
await combosDb.createCombo({
|
||||
name: REAL_MODEL_ID,
|
||||
models: [{ provider: "codex", model: REAL_MODEL_ID }],
|
||||
});
|
||||
await combosDb.createCombo({
|
||||
name: NON_COLLIDING_NAME,
|
||||
models: [{ provider: "claude", model: "claude-sonnet-4-6" }],
|
||||
});
|
||||
|
||||
const { scanComboModelNameCollisionsAtBoot } = await import("../../src/instrumentation-node.ts");
|
||||
const originalWarn = console.warn;
|
||||
const warnings: unknown[][] = [];
|
||||
console.warn = (...args: unknown[]) => {
|
||||
warnings.push(args);
|
||||
};
|
||||
try {
|
||||
await scanComboModelNameCollisionsAtBoot();
|
||||
} finally {
|
||||
console.warn = originalWarn;
|
||||
}
|
||||
|
||||
const collisionWarning = warnings.find((args) =>
|
||||
String(args[0]).includes("share a name with a real model id (#8530)")
|
||||
);
|
||||
assert.ok(collisionWarning, "expected a startup warning about the model-name collision");
|
||||
assert.ok(String(collisionWarning![0]).includes(REAL_MODEL_ID));
|
||||
assert.ok(
|
||||
!String(collisionWarning![0]).includes(NON_COLLIDING_NAME),
|
||||
"non-colliding combo must not be reported"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user