fix(combos): synchronize allowedProviders and allow invariant overrides on update (#13951)

* fix(combos): synchronize allowedProviders and allow invariant overrides

* docs: add PR reference to changelog fragment #13951

* fix(combos): only widen existing allowedProviders/allowedModelFamilies restrictions on edit

The dashboard's combo edit save unconditionally set overrideAllowedProviders
and unconditionally nulled allowedModelFamilies. When a combo had NO prior
allowedProviders restriction, the PUT route unioned the (empty) current
restriction with the new step providers, synthesizing a brand-new allowlist
out of nothing — the opposite of "no restriction" — so a later add-a-provider
update would start failing COMBO_008 where it previously succeeded.

- Gate the server-side union in PUT /api/combos/[id] so it only widens an
  ALREADY non-empty allowedProviders restriction; a combo with no restriction
  stays unrestricted.
- Only clear allowedModelFamilies when a new step's family actually falls
  outside the existing restriction, instead of always nulling it on any edit.
- Derive the provider id via resolveCanonicalProviderModel instead of a naive
  model.split('/')[0], so self-aliased no-auth providers (e.g. 'opencode' ->
  'opencode-zen') resolve to their real routing provider.
- Add a PUT-route-level regression test covering both the "no prior
  restriction stays unrestricted" and "prior restriction gets unioned" cases
  (the existing combo-update-invariants test only exercised
  combosDb.updateCombo() directly, bypassing this route branch).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* refactor(combos): extract computeAllowedRestrictionSync to keep handleSave under the complexity ratchet

The #13951 gating fix pushed handleSave's cyclomatic complexity past the frozen
new-code ceiling (16 > 15). Moving the allowedProviders/allowedModelFamilies sync
into a module-level helper restores complexityNewCode=0 with no behavior change.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

changelog.d/fixes/13951-combo-update-invariants.md

@@ -0,0 +1 @@
+- fix(combos): synchronize allowedProviders and allow invariant override when updating combos from dashboard ([#13951](https://github.com/diegosouzapw/OmniRoute/pull/13951))

src/app/(dashboard)/dashboard/combos/page.tsx

@@ -84,6 +84,8 @@ import {
 } from "@/lib/combos/intelligentRouting";
 import { getComboStepTarget } from "@/lib/combos/steps";
 import { DEAD_COMBO_CONFIG_KEYS } from "@/lib/combos/deadConfigKeys";
+import { modelFamily } from "@/lib/combos/invariants";
+import { resolveCanonicalProviderModel } from "@omniroute/open-sse/services/model.ts";
 import { resolveServerErrorMessage } from "@/lib/api/serverErrorMessage";
 import { useTranslations } from "next-intl";
 
@@ -653,6 +655,55 @@ function normalizeModelEntry(entry) {
   };
 }
 
+/**
+ * On an existing-combo edit, work out how the dashboard save should synchronize
+ * `allowedProviders` / `allowedModelFamilies` against the combo's new step list so
+ * adding a step across providers never triggers COMBO_008 (#13951). Both restrictions
+ * are only ever WIDENED or left untouched here — never synthesized from no restriction,
+ * and never wiped just because the combo happens to have one.
+ */
+function computeAllowedRestrictionSync(
+  isEdit: boolean,
+  combo: { allowedProviders?: unknown; allowedModelFamilies?: unknown } | null | undefined,
+  models: Array<{ providerId?: string; model?: string }>
+): { allowedProviders?: string[]; allowedModelFamilies?: null; overrideAllowedProviders?: true } {
+  if (!isEdit) return {};
+  const result: {
+    allowedProviders?: string[];
+    allowedModelFamilies?: null;
+    overrideAllowedProviders?: true;
+  } = { overrideAllowedProviders: true };
+
+  const existingProviders = Array.isArray(combo?.allowedProviders) ? combo.allowedProviders : [];
+  if (existingProviders.length > 0) {
+    const stepProviders = models
+      .map((m) => {
+        if (m.providerId) return m.providerId;
+        if (typeof m.model !== "string" || !m.model.includes("/")) return "";
+        const [aliasOrProvider, ...rest] = m.model.split("/");
+        return resolveCanonicalProviderModel(aliasOrProvider, rest.join("/")).provider || "";
+      })
+      .filter((p): p is string => Boolean(p));
+    result.allowedProviders = Array.from(new Set([...existingProviders, ...stepProviders]));
+  }
+
+  // Only clear the family restriction when a new step actually violates it (#13951).
+  const existingFamilies = Array.isArray(combo?.allowedModelFamilies)
+    ? combo.allowedModelFamilies
+    : [];
+  if (existingFamilies.length > 0) {
+    const allowedFamilies = new Set(existingFamilies);
+    const stepViolatesFamilies = models.some((m) => {
+      const family = typeof m.model === "string" ? modelFamily(m.model) : null;
+      return !family || !allowedFamilies.has(family);
+    });
+    if (stepViolatesFamilies) result.allowedModelFamilies = null;
+  }
+
+  return result;
+}
+
+
 function getModelString(entry) {
   if (typeof entry === "string") return entry;
   if (entry?.kind === "combo-ref") return entry.comboName;
@@ -3027,6 +3078,10 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
       strategy,
     };
 
+    // When editing an existing combo from the dashboard form, synchronize allowedProviders
+    // and clear legacy family restrictions so adding steps across providers never triggers COMBO_008
+    Object.assign(saveData, computeAllowedRestrictionSync(isEdit, combo, models));
+
     // Per-combo description (#5005). Free-text, optional, persisted in combo data.
     if (description.trim()) {
       saveData.description = description.trim();

src/app/api/combos/[id]/route.ts

@@ -6,6 +6,7 @@ import { syncToCloud } from "@/lib/cloudSync";
 import { validateCompositeTiersConfig } from "@/lib/combos/compositeTiers";
 import { normalizeComboModels } from "@/lib/combos/steps";
 import { validateComboDAG, clampComboDepth } from "@omniroute/open-sse/services/combo.ts";
+import { resolveCanonicalProviderModel } from "@omniroute/open-sse/services/model.ts";
 import { updateComboSchema } from "@/shared/validation/schemas";
 import { requiresQuotaOnlyComboRefExecute } from "@/shared/validation/schemas/combo";
 import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
@@ -137,6 +138,32 @@ export async function PUT(request, { params }) {
           }),
         }
       : normalizedUpdate;
+
+    if (body.overrideAllowedProviders === true) {
+      delete body.overrideAllowedProviders;
+      const currentProviders = Array.isArray(currentCombo.allowedProviders)
+        ? currentCombo.allowedProviders
+        : [];
+      // Only widen an EXISTING restriction (#13951/COMBO_008). When the combo
+      // currently has no allowedProviders restriction, currentProviders is
+      // empty and unioning it with the new step providers would synthesize a
+      // brand-new allowlist out of nothing — the opposite of "no restriction".
+      if (body.models && body.allowedProviders === undefined && currentProviders.length > 0) {
+        const stepProviders = (
+          body.models as Array<{ providerId?: string; provider?: string; model?: string }>
+        )
+          .map((m) => {
+            if (m.providerId) return m.providerId;
+            if (m.provider) return m.provider;
+            if (typeof m.model !== "string" || !m.model.includes("/")) return "";
+            const [aliasOrProvider, ...rest] = m.model.split("/");
+            return resolveCanonicalProviderModel(aliasOrProvider, rest.join("/")).provider || "";
+          })
+          .filter((p): p is string => Boolean(p));
+        body.allowedProviders = Array.from(new Set([...currentProviders, ...stepProviders]));
+      }
+    }
+
     const nextComboState = {
       ...currentCombo,
       ...body,

src/lib/combos/invariants.ts

@@ -14,10 +14,19 @@ const FAMILY_PATTERNS: ReadonlyArray<[string, RegExp]> = [
 ];
 
 function strings(value: unknown): string[] {
-  return Array.isArray(value) ? value.filter((item): item is string => typeof item === "string") : [];
+  return Array.isArray(value)
+    ? value.filter((item): item is string => typeof item === "string")
+    : [];
 }
 
-function modelFamily(model: string): string | null {
+/**
+ * Detect the model "family" (gpt/claude/gemini/...) from a bare or
+ * provider-prefixed model id. Exported for callers that need to know
+ * whether a candidate step would actually violate an existing
+ * `allowedModelFamilies` restriction (#13951) rather than only the
+ * `validateComboInvariant` throw path below.
+ */
+export function modelFamily(model: string): string | null {
   const bare = model.slice(model.lastIndexOf("/") + 1);
   return FAMILY_PATTERNS.find(([, pattern]) => pattern.test(bare))?.[0] ?? null;
 }

src/shared/validation/schemas/combo.ts

@@ -432,8 +432,9 @@ export const updateComboSchema = z
     // so the one endpoint a client can flip it through stripped the field and
     // a visibility-only update was rejected as empty. #12836
     isHidden: z.boolean().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(),
+    allowedProviders: z.array(z.string().trim().min(1).max(200)).max(100).optional().nullable(),
+    allowedModelFamilies: z.array(z.string().trim().min(1).max(100)).max(100).optional().nullable(),
+    overrideAllowedProviders: z.boolean().optional(),
     // Nullable like `description` and `context_length` above: an absent field means
     // "leave unchanged" because updateCombo merges over the stored record, so clearing
     // one needs an explicit null for updateCombo's null-means-delete pass (#12158).

tests/unit/combo-put-route-allowed-providers.test.ts

@@ -0,0 +1,85 @@
+// #13951 — route-level regression coverage for the PUT /api/combos/[id]
+// overrideAllowedProviders sync path. tests/unit/combo-update-invariants.test.ts
+// only exercises combosDb.updateCombo() directly, bypassing the PUT route
+// branch this test targets.
+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-put-route-"));
+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 comboRoute = await import("../../src/app/api/combos/[id]/route.ts");
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+});
+
+function put(id: string, body: Record<string, unknown>) {
+  return new Request(`http://localhost/api/combos/${id}`, {
+    method: "PUT",
+    headers: { "content-type": "application/json" },
+    body: JSON.stringify(body),
+  });
+}
+
+test("PUT with overrideAllowedProviders on a combo with NO prior restriction stays unrestricted", async () => {
+  const combo = await combosDb.createCombo({
+    name: "unrestricted-combo",
+    strategy: "priority",
+    models: [{ provider: "claude", model: "claude-sonnet-5" }],
+  });
+  assert.ok(combo?.id);
+  assert.equal((combo as { allowedProviders?: string[] }).allowedProviders, undefined);
+
+  const response = await comboRoute.PUT(
+    put(combo.id, {
+      name: "unrestricted-combo",
+      models: [
+        { provider: "claude", model: "claude-sonnet-5" },
+        { provider: "openai", model: "gpt-5" },
+      ],
+      overrideAllowedProviders: true,
+    }),
+    { params: Promise.resolve({ id: combo.id }) }
+  );
+  assert.equal(response.status, 200);
+
+  const stored = (await combosDb.getComboById(combo.id)) as { allowedProviders?: string[] };
+  // The combo had no restriction before the edit — it must still have none
+  // afterwards. Synthesizing allowedProviders=["claude","openai"] here would
+  // be the #13951 regression: a later add-a-provider update would start
+  // failing COMBO_008 where it previously succeeded.
+  assert.equal(stored.allowedProviders, undefined);
+});
+
+test("PUT with overrideAllowedProviders on a combo with an EXISTING restriction unions the new step providers", async () => {
+  const combo = await combosDb.createCombo({
+    name: "restricted-combo",
+    strategy: "priority",
+    allowedProviders: ["claude"],
+    models: [{ provider: "claude", model: "claude-sonnet-5" }],
+  });
+  assert.ok(combo?.id);
+
+  const response = await comboRoute.PUT(
+    put(combo.id, {
+      name: "restricted-combo",
+      models: [
+        { provider: "claude", model: "claude-sonnet-5" },
+        { provider: "openai", model: "gpt-5" },
+      ],
+      overrideAllowedProviders: true,
+    }),
+    { params: Promise.resolve({ id: combo.id }) }
+  );
+  assert.equal(response.status, 200);
+
+  const stored = (await combosDb.getComboById(combo.id)) as { allowedProviders?: string[] };
+  assert.deepEqual([...(stored.allowedProviders ?? [])].sort(), ["claude", "openai"]);
+});

tests/unit/combo-update-invariants.test.ts

@@ -0,0 +1,89 @@
+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";
+import { updateComboSchema } from "../../src/shared/validation/schemas/combo.ts";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-combo-invariants-"));
+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");
+
+async function resetStorage() {
+  core.resetDbInstance();
+  if (fs.existsSync(TEST_DATA_DIR)) {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+  }
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(async () => {
+  await resetStorage();
+});
+
+test.after(async () => {
+  core.resetDbInstance();
+  if (fs.existsSync(TEST_DATA_DIR)) {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+  }
+});
+
+test("updateComboSchema accepts nullable allowedProviders, allowedModelFamilies, and overrideAllowedProviders", () => {
+  const parsedNulls = updateComboSchema.safeParse({
+    allowedProviders: null,
+    allowedModelFamilies: null,
+    overrideAllowedProviders: true,
+  });
+  assert.equal(parsedNulls.success, true);
+  if (parsedNulls.success) {
+    assert.equal(parsedNulls.data.allowedProviders, null);
+    assert.equal(parsedNulls.data.allowedModelFamilies, null);
+    assert.equal(parsedNulls.data.overrideAllowedProviders, true);
+  }
+
+  const parsedArray = updateComboSchema.safeParse({
+    allowedProviders: ["claude", "antigravity"],
+    allowedModelFamilies: ["claude"],
+  });
+  assert.equal(parsedArray.success, true);
+});
+
+test("updateCombo allows updating allowedProviders and clearing with null", async () => {
+  const combo = await combosDb.createCombo({
+    name: "claude-combo",
+    allowedProviders: ["claude"],
+    models: [{ provider: "claude", model: "claude-sonnet-5" }],
+  });
+  assert.ok(combo?.id);
+
+  // Updating models to include a new provider with expanded allowedProviders succeeds
+  const updated = await combosDb.updateCombo(String(combo.id), {
+    allowedProviders: ["claude", "antigravity"],
+    models: [
+      { provider: "claude", model: "claude-sonnet-5" },
+      { provider: "antigravity", model: "claude-sonnet-4-6" },
+    ],
+  });
+  assert.ok(updated);
+  const typedUpdated = updated as {
+    allowedProviders?: string[];
+    models: Array<{ providerId?: string }>;
+  };
+  assert.deepEqual(typedUpdated.allowedProviders, ["claude", "antigravity"]);
+  assert.equal(typedUpdated.models.length, 2);
+
+  // Clearing allowedProviders with null succeeds and removes the invariant restriction
+  const cleared = await combosDb.updateCombo(String(combo.id), {
+    allowedProviders: null,
+    models: [{ provider: "openrouter", model: "nvidia/nemotron-3.5-lightning:free" }],
+  });
+  assert.ok(cleared);
+  const typedCleared = cleared as {
+    allowedProviders?: string[];
+    models: Array<{ providerId?: string }>;
+  };
+  assert.equal(typedCleared.allowedProviders, undefined);
+  assert.equal(typedCleared.models[0]?.providerId, "openrouter");
+});