From 07067841b5c574cae4cfbd265559da604f17a958 Mon Sep 17 00:00:00 2001 From: Ravi Tharuma <25951435+RaviTharuma@users.noreply.github.com> Date: Fri, 24 Jul 2026 16:44:00 +0200 Subject: [PATCH] feat(combo): enforce provider and model family invariants (#8304) * feat(combo): enforce provider and model family invariants Closes #8279 Co-Authored-By: Ravi Tharuma * 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 --------- Co-authored-by: Ravi Tharuma Co-authored-by: Ravi Tharuma Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- src/app/api/combos/[id]/route.ts | 4 ++ src/app/api/combos/route.ts | 4 ++ src/lib/api/comboErrorResponse.ts | 1 + src/lib/combos/invariants.ts | 65 +++++++++++++++++++ src/lib/db/combos.ts | 8 +++ src/lib/db/jsonMigration.ts | 7 +- src/shared/constants/errorCodes.ts | 6 ++ src/shared/validation/schemas/combo.ts | 7 +- .../unit/combo-routes-composite-tiers.test.ts | 46 +++++++++++++ tests/unit/db-combos-crud.test.ts | 29 +++++++++ tests/unit/json-migration-combos.test.ts | 30 +++++++++ 11 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 src/lib/combos/invariants.ts diff --git a/src/app/api/combos/[id]/route.ts b/src/app/api/combos/[id]/route.ts index 4946006266..5ec69f1385 100644 --- a/src/app/api/combos/[id]/route.ts +++ b/src/app/api/combos/[id]/route.ts @@ -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); } diff --git a/src/app/api/combos/route.ts b/src/app/api/combos/route.ts index 10acd647b7..7bd3baebef 100644 --- a/src/app/api/combos/route.ts +++ b/src/app/api/combos/route.ts @@ -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 }); } diff --git a/src/lib/api/comboErrorResponse.ts b/src/lib/api/comboErrorResponse.ts index 11436ca0ba..1ee66d4c17 100644 --- a/src/lib/api/comboErrorResponse.ts +++ b/src/lib/api/comboErrorResponse.ts @@ -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 diff --git a/src/lib/combos/invariants.ts b/src/lib/combos/invariants.ts new file mode 100644 index 0000000000..302bf61296 --- /dev/null +++ b/src/lib/combos/invariants.ts @@ -0,0 +1,65 @@ +type JsonRecord = Record; + +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` + ); + } + }); +} diff --git a/src/lib/db/combos.ts b/src/lib/db/combos.ts index a658801473..4314319792 100644 --- a/src/lib/db/combos.ts +++ b/src/lib/db/combos.ts @@ -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; @@ -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( diff --git a/src/lib/db/jsonMigration.ts b/src/lib/db/jsonMigration.ts index c927cd582b..89bad747ed 100644 --- a/src/lib/db/jsonMigration.ts +++ b/src/lib/db/jsonMigration.ts @@ -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).strategy ); } - const normalizedCombo: Record = { + const normalizedCombo: Record = 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, diff --git a/src/shared/constants/errorCodes.ts b/src/shared/constants/errorCodes.ts index 471f76bb0e..8a0eb4094c 100644 --- a/src/shared/constants/errorCodes.ts +++ b/src/shared/constants/errorCodes.ts @@ -187,6 +187,12 @@ export const ERROR_CODES: Record = { 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: { diff --git a/src/shared/validation/schemas/combo.ts b/src/shared/validation/schemas/combo.ts index 73d3824ba9..62340fea15 100644 --- a/src/shared/validation/schemas/combo.ts +++ b/src/shared/validation/schemas/combo.ts @@ -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 && diff --git a/tests/unit/combo-routes-composite-tiers.test.ts b/tests/unit/combo-routes-composite-tiers.test.ts index 2abda3e5ab..bb1cbc0f93 100644 --- a/tests/unit/combo-routes-composite-tiers.test.ts +++ b/tests/unit/combo-routes-composite-tiers.test.ts @@ -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"); +}); diff --git a/tests/unit/db-combos-crud.test.ts b/tests/unit/db-combos-crud.test.ts index 69b754d693..986ab7d76e 100644 --- a/tests/unit/db-combos-crud.test.ts +++ b/tests/unit/db-combos-crud.test.ts @@ -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", diff --git a/tests/unit/json-migration-combos.test.ts b/tests/unit/json-migration-combos.test.ts index 36ef86bd51..978f9109ae 100644 --- a/tests/unit/json-migration-combos.test.ts +++ b/tests/unit/json-migration-combos.test.ts @@ -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 + ); +});