fix(providers): treat a degraded cached catalog as a failed model sync (#10862)

Obrigado — root cause preciso: a rota sync-models só reconhecia a degradação para local_catalog, não para o fallback de cache com warning, então uma chave expirada (401) virava silenciosamente "Nenhum modelo novo foi adicionado" em vez de um erro visível.

Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos):
- typecheck:core limpo, complexity/cognitive-complexity dentro do baseline
- tests/unit/sync-models-degraded-cached-catalog-9683.test.ts — 6/6 passando (payloads reais do models/route.ts)
- Suítes model-sync/provider-models/sync-models/siliconflow — 181/181 passando, incluindo as 3 asserções pré-existentes #5460/#5465

changelog.d/fixes/10862-sync-models-degraded-cached-catalog.md

@@ -0,0 +1 @@
+- **fix(providers):** importing models with an expired API key now surfaces the credential error instead of reporting "No new models were added". The Import button posts to `/api/providers/{id}/sync-models`, which self-fetches the models route; that route does not fail on an upstream 401 but degrades to a catalog it already has, preferring the cache and using the local catalog only when there is no cache. A provider that imported successfully once therefore has a cache, so an expired key produced `{ source: "cache", warning: "Models probe failed (401) — using cached catalog" }` with HTTP 200 — and the #5460/#5465 degradation guard only recognised the `local_catalog` branch, so model-sync accepted it as a successful discovery, found every cached model already imported, and returned the empty-diff result. Retest does not go through this path, which is why it failed correctly and made the import look like a genuine "nothing to do". The existing rule — a degraded discovery must not be persisted as the synced catalog — is now applied to the branch it missed rather than special-casing 401/403, discriminating on the warning the fallback builder always attaches (an ordinary non-refresh cache hit attaches none, and model-sync always requests `refresh=true`). `isDegradedLocalCatalog` keeps its exact meaning and its existing tests

src/app/api/providers/[id]/sync-models/degradedLocalCatalog.ts

@@ -19,3 +19,44 @@ export function isDegradedLocalCatalog(modelsData: {
     typeof modelsData?.source === "string" ? modelsData.source.trim().toLowerCase() : "";
   return source === "local_catalog" && modelsData?.intentional !== true;
 }
+
+/**
+ * #9683 — the same degradation, one branch further up.
+ *
+ * When remote discovery fails, the models route falls back to the CACHED
+ * catalog if it has one and only falls back to the local catalog when it does
+ * not (`buildDiscoveryFallbackResponse`). A provider that was imported
+ * successfully once therefore has a cache, so an expired key produced
+ * `source: "cache"` + a warning and HTTP 200 — model-sync treated that as a
+ * successful discovery, found every cached model already imported, and reported
+ * "No new models were added" instead of the credential error. Retest, which
+ * does not go through this path, failed correctly, which is what made the
+ * import look like a real "nothing to do".
+ *
+ * The discriminator is the warning: the fallback builder always attaches one,
+ * while the ordinary cache hit (`maybeReturnCachedDiscovery`, a non-refresh
+ * read) attaches none. Model-sync always requests `refresh=true`, so the one
+ * warning-carrying cache response it can observe is a degraded one.
+ */
+export function isDegradedCachedCatalog(modelsData: {
+  source?: unknown;
+  warning?: unknown;
+}): boolean {
+  const source =
+    typeof modelsData?.source === "string" ? modelsData.source.trim().toLowerCase() : "";
+  if (source !== "cache") return false;
+  return typeof modelsData?.warning === "string" && modelsData.warning.trim().length > 0;
+}
+
+/**
+ * Either degraded shape. Model-sync must refuse to treat these as a successful
+ * discovery: persisting them would silently pin a stale catalog and hide the
+ * real failure from the operator.
+ */
+export function isDegradedDiscovery(modelsData: {
+  source?: unknown;
+  intentional?: unknown;
+  warning?: unknown;
+}): boolean {
+  return isDegradedLocalCatalog(modelsData) || isDegradedCachedCatalog(modelsData);
+}

src/app/api/providers/[id]/sync-models/route.ts

@@ -22,7 +22,7 @@ import { autoSyncCodexProfilesFromLiveCatalog } from "@/lib/cli-helper/codexProf
 import { autoSyncClaudeProfilesFromLiveCatalog } from "@/lib/cli-helper/claudeProfileAutoSync";
 import { providerUsesCuratedModelsOnly } from "@/lib/providers/modelListingCapability";
 import { GET as getProviderModels } from "../models/route";
-import { isDegradedLocalCatalog } from "./degradedLocalCatalog";
+import { isDegradedDiscovery } from "./degradedLocalCatalog";
 import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 type JsonRecord = Record<string, unknown>;
@@ -465,9 +465,9 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
 
     const modelSource = toNonEmptyString(modelsData.source)?.toLowerCase() || "unknown";
     const modelWarning = toNonEmptyString(modelsData.warning);
-    if (isDegradedLocalCatalog(modelsData)) {
+    if (isDegradedDiscovery(modelsData)) {
       const responseError =
-        modelWarning || "Remote model discovery failed; local catalog fallback not synced";
+        modelWarning || "Remote model discovery failed; catalog fallback not synced";
       await saveCallLog({
         method: "GET",
         path: `/api/providers/${id}/models`,

tests/unit/sync-models-degraded-cached-catalog-9683.test.ts

@@ -0,0 +1,121 @@
+/**
+ * #9683 — "Importing Models" reported success with an expired API key.
+ *
+ * The import button posts to `/api/providers/{id}/sync-models`, which self-fetches
+ * `/api/providers/{id}/models?refresh=true`. On an upstream 401 that route does not
+ * fail: it falls back to a catalog it already has. It prefers the CACHE and only uses
+ * the local catalog when there is no cache, so a provider that imported successfully
+ * once takes the cache branch — `{ source: "cache", warning: "…(401)…" }` with HTTP
+ * 200. `isDegradedLocalCatalog` only recognised `local_catalog`, so model-sync treated
+ * that as a successful discovery, found every cached model already imported and
+ * answered "No new models were added" instead of surfacing the credential error.
+ *
+ * The warning is the discriminator: `buildDiscoveryFallbackResponse` always attaches
+ * one, while the ordinary non-refresh cache hit attaches none.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { isDegradedLocalCatalog, isDegradedCachedCatalog, isDegradedDiscovery } =
+  await import("../../src/app/api/providers/[id]/sync-models/degradedLocalCatalog.ts");
+
+// The exact payload shapes `src/app/api/providers/[id]/models/route.ts` emits from
+// `buildCachedDiscoveryResponse(cacheWarning)` on a failed probe.
+const AUTH_FAILED_CACHE = {
+  source: "cache",
+  warning: "Models probe failed (401) — using cached catalog",
+};
+const BEDROCK_AUTH_FAILED_CACHE = {
+  source: "cache",
+  warning: "Auth failed (403) — using cached catalog",
+};
+const UNAVAILABLE_CACHE = {
+  source: "cache",
+  warning: "API unavailable — using cached catalog",
+};
+// `maybeReturnCachedDiscovery()` — an ordinary cache hit, built with no warning.
+const HEALTHY_CACHE = { source: "cache" };
+
+test("#9683: a 401 that degraded to the cached catalog is a failed discovery", () => {
+  assert.equal(
+    isDegradedCachedCatalog(AUTH_FAILED_CACHE),
+    true,
+    "an expired key must not be reported to the operator as a successful import"
+  );
+  assert.equal(isDegradedCachedCatalog(BEDROCK_AUTH_FAILED_CACHE), true);
+  assert.equal(isDegradedDiscovery(AUTH_FAILED_CACHE), true);
+});
+
+test("#9683: any warning-carrying cache fallback is degraded, not only auth", () => {
+  // The rule model-sync already applies to `local_catalog` is not auth-specific:
+  // a degraded discovery must not be persisted as the synced catalog.
+  assert.equal(isDegradedCachedCatalog(UNAVAILABLE_CACHE), true);
+  assert.equal(isDegradedDiscovery(UNAVAILABLE_CACHE), true);
+});
+
+test("#9683: an ordinary cache hit stays a success", () => {
+  assert.equal(isDegradedCachedCatalog(HEALTHY_CACHE), false);
+  assert.equal(isDegradedDiscovery(HEALTHY_CACHE), false);
+  for (const blank of ["", "   "]) {
+    assert.equal(
+      isDegradedCachedCatalog({ source: "cache", warning: blank }),
+      false,
+      "a blank warning is not a degradation signal"
+    );
+  }
+  assert.equal(isDegradedCachedCatalog({ source: "cache", warning: 42 }), false);
+});
+
+test("#9683: only the cache source is judged by this predicate", () => {
+  assert.equal(isDegradedCachedCatalog({ source: "api", warning: "anything" }), false);
+  assert.equal(isDegradedCachedCatalog({ source: "local_catalog", warning: "x" }), false);
+  assert.equal(isDegradedCachedCatalog({}), false);
+  assert.equal(isDegradedCachedCatalog({ source: "" }), false);
+  assert.equal(isDegradedCachedCatalog({ source: "  CACHE  ", warning: "x" }), true);
+});
+
+// ── #5460/#5465 must be unchanged ─────────────────────────────────────────
+
+test("#9683: the local-catalog rule is untouched", () => {
+  assert.equal(isDegradedLocalCatalog({ source: "local_catalog", intentional: true }), false);
+  assert.equal(isDegradedLocalCatalog({ source: "local_catalog", intentional: false }), true);
+  assert.equal(isDegradedLocalCatalog({ source: "cache", intentional: false }), false);
+
+  assert.equal(
+    isDegradedDiscovery({ source: "local_catalog", intentional: true }),
+    false,
+    "a provider whose local catalog is its only discovery source still syncs"
+  );
+  assert.equal(
+    isDegradedDiscovery({
+      source: "local_catalog",
+      intentional: true,
+      warning: "reka has no remote /models endpoint",
+    }),
+    false,
+    "an intentional local catalog is not degraded just because it carries a warning"
+  );
+  assert.equal(isDegradedDiscovery({ source: "local_catalog", intentional: false }), true);
+});
+
+// ── Wiring ────────────────────────────────────────────────────────────────
+// The predicate is only half the fix: the route has to consult the combined one.
+// This assertion fails on the pre-fix tree, where the guard read
+// `isDegradedLocalCatalog(modelsData)` and so never saw the cache fallback.
+
+test("#9683: the sync-models route gates on the combined predicate", async () => {
+  const { readFileSync } = await import("node:fs");
+  const source = readFileSync(
+    new URL("../../src/app/api/providers/[id]/sync-models/route.ts", import.meta.url),
+    "utf8"
+  );
+  assert.match(
+    source,
+    /if\s*\(\s*isDegradedDiscovery\(modelsData\)\s*\)/,
+    "model-sync must refuse both degraded shapes, not just local_catalog"
+  );
+  assert.ok(
+    !/isDegradedLocalCatalog\(modelsData\)/.test(source),
+    "the narrow predicate must no longer be the route's only gate"
+  );
+});