mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
feat(combo): enforce provider and model family invariants (#8304)
* feat(combo): enforce provider and model family invariants Closes #8279 Co-Authored-By: Ravi Tharuma <ravitharuma@users.noreply.github.com> * fix(combo): complete invariant enforcement paths Map invariant failures to structured API errors, correct target diagnostics, and validate restored combos inside the existing migration transaction. Co-Authored-By: Ravi Tharuma <noreply@github.com> --------- Co-authored-by: Ravi Tharuma <RaviTharuma@users.noreply.github.com> Co-authored-by: Ravi Tharuma <noreply@github.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -17,6 +17,7 @@ import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
|
||||
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";
|
||||
|
||||
// Minimal shape for the fields we read off a combo row in this route.
|
||||
// `getComboById` returns a structurally `JsonRecord`-typed object, so we
|
||||
@@ -250,6 +251,9 @@ export async function PUT(request, { params }) {
|
||||
|
||||
return NextResponse.json(combo);
|
||||
} catch (error) {
|
||||
if (error instanceof ComboInvariantError) {
|
||||
return comboErrorResponse("COMBO_008", 400, { reason: error.message }, request);
|
||||
}
|
||||
console.log("Error updating combo:", error);
|
||||
return comboErrorResponse("INTERNAL_001", 500, undefined, request);
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { comboErrorResponse } from "@/lib/api/comboErrorResponse";
|
||||
import { computeComboContextLength } from "@/lib/combos/comboContext";
|
||||
import { ComboInvariantError } from "@/lib/combos/invariants";
|
||||
|
||||
// GET /api/combos - Get all combos
|
||||
export async function GET(request: Request) {
|
||||
@@ -121,6 +122,9 @@ export async function POST(request) {
|
||||
|
||||
return NextResponse.json(combo, { status: 201 });
|
||||
} catch (error) {
|
||||
if (error instanceof ComboInvariantError) {
|
||||
return comboErrorResponse("COMBO_008", 400, { reason: error.message }, request);
|
||||
}
|
||||
console.log("Error creating combo:", error);
|
||||
return NextResponse.json({ error: "Failed to create combo" }, { status: 500 });
|
||||
}
|
||||
|
||||
@@ -32,6 +32,7 @@ export type ComboErrorCode =
|
||||
| "COMBO_005" // DAG cycle / depth overflow
|
||||
| "COMBO_006" // managed by Quota Share (409)
|
||||
| "COMBO_007" // not found (404)
|
||||
| "COMBO_008" // provider / model family invariant violation
|
||||
| "VALID_001" // generic invalid body
|
||||
| "VALID_002" // missing required field
|
||||
| "INTERNAL_001"; // fallback
|
||||
|
||||
65
src/lib/combos/invariants.ts
Normal file
65
src/lib/combos/invariants.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
export class ComboInvariantError extends Error {}
|
||||
|
||||
const FAMILY_PATTERNS: ReadonlyArray<[string, RegExp]> = [
|
||||
["gpt", /^gpt(?:-|$)/i],
|
||||
["claude", /^claude(?:-|$)/i],
|
||||
["gemini", /^gemini(?:-|$)/i],
|
||||
["glm", /^glm(?:-|$)/i],
|
||||
["kimi", /^kimi(?:-|$)/i],
|
||||
["llama", /^llama(?:-|$)/i],
|
||||
["minimax", /^minimax(?:-|$)/i],
|
||||
["mistral", /^(?:mistral|mixtral)(?:-|$)/i],
|
||||
];
|
||||
|
||||
function strings(value: unknown): string[] {
|
||||
return Array.isArray(value) ? value.filter((item): item is string => typeof item === "string") : [];
|
||||
}
|
||||
|
||||
function modelFamily(model: string): string | null {
|
||||
const bare = model.slice(model.lastIndexOf("/") + 1);
|
||||
return FAMILY_PATTERNS.find(([, pattern]) => pattern.test(bare))?.[0] ?? null;
|
||||
}
|
||||
|
||||
export function validateComboInvariant(combo: JsonRecord): void {
|
||||
const invariant =
|
||||
combo.invariant && typeof combo.invariant === "object" && !Array.isArray(combo.invariant)
|
||||
? (combo.invariant as JsonRecord)
|
||||
: {};
|
||||
const providers = new Set([
|
||||
...strings(combo.allowedProviders),
|
||||
...strings(invariant.allowedProviders),
|
||||
]);
|
||||
const families = new Set([
|
||||
...strings(combo.allowedModelFamilies),
|
||||
...strings(invariant.allowedModelFamilies),
|
||||
]);
|
||||
if (providers.size === 0 && families.size === 0) return;
|
||||
|
||||
const targets = Array.isArray(combo.models) ? combo.models : [];
|
||||
targets.forEach((value, index) => {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) return;
|
||||
const target = value as JsonRecord;
|
||||
if (target.kind === "combo-ref") return;
|
||||
const model = typeof target.model === "string" ? target.model : "";
|
||||
const provider =
|
||||
typeof target.providerId === "string"
|
||||
? target.providerId
|
||||
: typeof target.provider === "string"
|
||||
? target.provider
|
||||
: model.includes("/")
|
||||
? model.slice(0, model.indexOf("/"))
|
||||
: "";
|
||||
const family = modelFamily(model);
|
||||
if (
|
||||
(providers.size > 0 && !providers.has(provider)) ||
|
||||
(families.size > 0 && (!family || !families.has(family)))
|
||||
) {
|
||||
const targetName = model.includes("/") ? model : `${provider}/${model}`;
|
||||
throw new ComboInvariantError(
|
||||
`Combo "${String(combo.name ?? "unnamed")}" target ${index + 1} (${targetName}) violates its invariant`
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -9,6 +9,7 @@ import { invalidateDbCache } from "./readCache";
|
||||
import { invalidateReasoningRoutingRuleCache } from "./reasoningRoutingRules";
|
||||
import { normalizeComboRecord } from "@/lib/combos/steps";
|
||||
import { clearSessionModelHistoryForCombo } from "./contextHandoffs";
|
||||
import { validateComboInvariant } from "@/lib/combos/invariants";
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
@@ -184,6 +185,7 @@ export async function createCombo(data: JsonRecord) {
|
||||
typeof data.name === "string" ? [data.name] : []
|
||||
);
|
||||
|
||||
validateComboInvariant(combo);
|
||||
const contextCache = data.context_cache_protection ? 1 : 0;
|
||||
db.prepare(
|
||||
"INSERT INTO combos (id, name, data, sort_order, created_at, updated_at, context_cache_protection) VALUES (?, ?, ?, ?, ?, ?, ?)"
|
||||
@@ -227,6 +229,12 @@ export async function updateCombo(id: string, data: JsonRecord) {
|
||||
? merged["name"]
|
||||
: currentName;
|
||||
const normalizedMerged = normalizeStoredCombo({ ...merged, name: nextName }, db, [nextName]);
|
||||
validateComboInvariant({
|
||||
...normalizedMerged,
|
||||
...data,
|
||||
name: nextName,
|
||||
models: normalizedMerged.models,
|
||||
});
|
||||
const contextCacheProtection = normalizedMerged.context_cache_protection ? 1 : 0;
|
||||
|
||||
db.prepare(
|
||||
|
||||
@@ -13,6 +13,8 @@
|
||||
|
||||
import type { SqliteAdapter } from "./adapters/types";
|
||||
import { normalizeRoutingStrategy } from "@/shared/constants/routingStrategies";
|
||||
import { normalizeComboRecord } from "@/lib/combos/steps";
|
||||
import { validateComboInvariant } from "@/lib/combos/invariants";
|
||||
import {
|
||||
resolveImportedUsageAccountIdentity,
|
||||
resolveOrphanedUsageAccountIdentity,
|
||||
@@ -198,12 +200,13 @@ export function runJsonMigration(
|
||||
(config as Record<string, unknown>).strategy
|
||||
);
|
||||
}
|
||||
const normalizedCombo: Record<string, unknown> = {
|
||||
const normalizedCombo: Record<string, unknown> = normalizeComboRecord({
|
||||
...combo,
|
||||
strategy: normalizeRoutingStrategy(combo.strategy),
|
||||
config,
|
||||
sortOrder: typeof combo.sortOrder === "number" ? combo.sortOrder : index + 1,
|
||||
};
|
||||
});
|
||||
validateComboInvariant(normalizedCombo);
|
||||
insertCombo.run({
|
||||
id: normalizedCombo.id,
|
||||
name: normalizedCombo.name,
|
||||
|
||||
@@ -187,6 +187,12 @@ export const ERROR_CODES: Record<string, ErrorCodeDef> = {
|
||||
httpStatus: 404,
|
||||
category: "COMBO",
|
||||
},
|
||||
COMBO_008: {
|
||||
code: "COMBO_008",
|
||||
message: "Combo target violates its provider or model family invariant",
|
||||
httpStatus: 400,
|
||||
category: "COMBO",
|
||||
},
|
||||
|
||||
// ── Internal ──
|
||||
INTERNAL_001: {
|
||||
|
||||
@@ -288,7 +288,8 @@ export const createComboSchema = z.object({
|
||||
models: z.array(comboModelEntry).optional().default([]),
|
||||
strategy: comboStrategySchema.optional().default("priority"),
|
||||
config: comboRuntimeConfigSchema.optional(),
|
||||
allowedProviders: z.array(z.string().max(200)).optional(),
|
||||
allowedProviders: z.array(z.string().trim().min(1).max(200)).max(100).optional(),
|
||||
allowedModelFamilies: z.array(z.string().trim().min(1).max(100)).max(100).optional(),
|
||||
system_message: z.string().max(50000).optional(),
|
||||
tool_filter_regex: z.string().max(1000).optional(),
|
||||
context_cache_protection: z.boolean().optional(),
|
||||
@@ -346,7 +347,8 @@ export const updateComboSchema = z
|
||||
strategy: comboStrategySchema.optional(),
|
||||
config: comboRuntimeConfigSchema.optional(),
|
||||
isActive: z.boolean().optional(),
|
||||
allowedProviders: z.array(z.string().max(200)).optional(),
|
||||
allowedProviders: z.array(z.string().trim().min(1).max(200)).max(100).optional(),
|
||||
allowedModelFamilies: z.array(z.string().trim().min(1).max(100)).max(100).optional(),
|
||||
system_message: z.string().max(50000).optional(),
|
||||
tool_filter_regex: z.string().max(1000).optional(),
|
||||
context_cache_protection: z.boolean().optional(),
|
||||
@@ -367,6 +369,7 @@ export const updateComboSchema = z
|
||||
value.config === undefined &&
|
||||
value.isActive === undefined &&
|
||||
value.allowedProviders === undefined &&
|
||||
value.allowedModelFamilies === undefined &&
|
||||
value.system_message === undefined &&
|
||||
value.tool_filter_regex === undefined &&
|
||||
value.context_cache_protection === undefined &&
|
||||
|
||||
@@ -270,3 +270,49 @@ test("PUT /api/combos preserves legacy string combo refs during normalization",
|
||||
assert.equal(stored.models[0].kind, "combo-ref");
|
||||
assert.equal(stored.models[0].comboName, "child-ref");
|
||||
});
|
||||
|
||||
|
||||
test("POST /api/combos returns a structured 400 for invariant violations", async () => {
|
||||
const response = await createRoute.POST(
|
||||
makeCreateRequest({
|
||||
name: "invalid-create-invariant",
|
||||
allowedProviders: ["github"],
|
||||
allowedModelFamilies: ["gpt"],
|
||||
models: [{ provider: "zai", model: "glm-5" }],
|
||||
})
|
||||
);
|
||||
const body = await response.json();
|
||||
|
||||
assert.equal(response.status, 400);
|
||||
assert.equal(body.error.code, "COMBO_008");
|
||||
assert.equal(body.error.message, "Combo target violates its provider or model family invariant");
|
||||
assert.equal(
|
||||
body.error.details.reason,
|
||||
'Combo "invalid-create-invariant" target 1 (zai/glm-5) violates its invariant'
|
||||
);
|
||||
assert.equal(await combosDb.getComboByName("invalid-create-invariant"), null);
|
||||
});
|
||||
|
||||
test("PUT /api/combos returns a structured 400 for invariant violations", async () => {
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "valid-update-invariant",
|
||||
allowedProviders: ["github"],
|
||||
allowedModelFamilies: ["gpt"],
|
||||
models: [{ provider: "github", model: "gpt-5.4" }],
|
||||
});
|
||||
|
||||
const response = await comboRoute.PUT(
|
||||
makeUpdateRequest({ models: [{ provider: "moonshot", model: "kimi-k2" }] }),
|
||||
{ params: Promise.resolve({ id: combo.id }) }
|
||||
);
|
||||
const body = await response.json();
|
||||
const stored = await combosDb.getComboById(String(combo.id));
|
||||
|
||||
assert.equal(response.status, 400);
|
||||
assert.equal(body.error.code, "COMBO_008");
|
||||
assert.equal(
|
||||
body.error.details.reason,
|
||||
'Combo "valid-update-invariant" target 1 (moonshot/kimi-k2) violates its invariant'
|
||||
);
|
||||
assert.equal(stored?.models[0]?.model, "github/gpt-5.4");
|
||||
});
|
||||
|
||||
@@ -62,6 +62,35 @@ test("createCombo stores default strategy and supports lookup by id and name", a
|
||||
assert.deepEqual(await combosDb.getComboByName("Priority Combo"), combo);
|
||||
});
|
||||
|
||||
test("combo invariants reject invalid creates and updates atomically", async () => {
|
||||
await assert.rejects(
|
||||
combosDb.createCombo({
|
||||
name: "invalid-gpt-family",
|
||||
allowedProviders: ["github", "codex"],
|
||||
allowedModelFamilies: ["gpt"],
|
||||
models: [{ provider: "zai", model: "glm-5" }],
|
||||
}),
|
||||
/target 1 \(zai\/glm-5\) violates its invariant/i
|
||||
);
|
||||
assert.equal(await combosDb.getComboByName("invalid-gpt-family"), null);
|
||||
|
||||
const combo = await combosDb.createCombo({
|
||||
name: "gpt-family",
|
||||
allowedProviders: ["github", "codex"],
|
||||
allowedModelFamilies: ["gpt"],
|
||||
models: [{ provider: "github", model: "gpt-5.4" }],
|
||||
});
|
||||
|
||||
await assert.rejects(
|
||||
combosDb.updateCombo(String(combo.id), {
|
||||
models: [{ provider: "moonshot", model: "kimi-k2" }],
|
||||
}),
|
||||
/target 1 \(moonshot\/kimi-k2\) violates its invariant/i
|
||||
);
|
||||
const persisted = await combosDb.getComboById(String(combo.id));
|
||||
assert.equal((persisted?.models as Array<{ model: string }>)[0]?.model, "github/gpt-5.4");
|
||||
});
|
||||
|
||||
test("getCombos returns parsed combos in persisted sort order", async () => {
|
||||
await combosDb.createCombo({
|
||||
name: "Zulu",
|
||||
|
||||
@@ -143,3 +143,33 @@ test("runJsonMigration normalizes legacy combo strategy names at the import boun
|
||||
assert.equal(byId.get("combo-usage").config.strategy, "context-optimized");
|
||||
assert.equal(byId.get("combo-unknown").strategy, "priority");
|
||||
});
|
||||
|
||||
|
||||
test("runJsonMigration rejects invalid combo invariants atomically", () => {
|
||||
const db = core.getDbInstance();
|
||||
|
||||
assert.throws(
|
||||
() =>
|
||||
runJsonMigration(db, {
|
||||
settings: { importedBeforeFailure: true },
|
||||
combos: [
|
||||
{
|
||||
id: "invalid-import-combo",
|
||||
name: "invalid-import-combo",
|
||||
allowedProviders: ["github"],
|
||||
allowedModelFamilies: ["gpt"],
|
||||
models: [{ provider: "zai", model: "zai/glm-5" }],
|
||||
},
|
||||
],
|
||||
}),
|
||||
/target 1 \(zai\/glm-5\) violates its invariant/
|
||||
);
|
||||
|
||||
assert.equal(db.prepare("SELECT COUNT(*) count FROM combos").get().count, 0);
|
||||
assert.equal(
|
||||
db
|
||||
.prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
|
||||
.get("settings", "importedBeforeFailure"),
|
||||
undefined
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user