fix(guardrails): stop Vision Bridge from re-selecting a model locked after a 404 (#12111)

changelog.d/fixes/12111-vision-bridge-model-lockout.md

@@ -0,0 +1 @@
+- fix(guardrails): stop Vision Bridge from re-selecting a model locked after a 404 (#12111)

src/lib/guardrails/visionBridgeCredentials.ts

@@ -7,6 +7,7 @@
 
 import { resolveProviderId } from "@/shared/constants/providers";
 import { isNoAuthProviderKey } from "@/shared/utils/noAuthProviders";
+import { SYNTHETIC_NOAUTH_CONNECTION_ID } from "@omniroute/open-sse/services/autoCombo/resilienceCandidateFilter.ts";
 
 /**
  * True when a provider connection can actually authenticate upstream.
@@ -119,3 +120,51 @@ export async function hasUsableCredentialsForModel(model: string): Promise<boole
     return null;
   }
 }
+
+/** A minimal reference to a usable provider connection, for per-connection lockout checks. */
+export interface UsableConnectionRef {
+  id: string;
+}
+
+/**
+ * Resolve the individual usable connections for `model`'s provider (#12111).
+ *
+ * `hasUsableCredentialsForModel` collapses this same data to a single
+ * boolean, which is enough to know a provider is reachable at all but not
+ * enough to know whether one *specific* model is servable: `isModelLocked`
+ * (open-sse/services/accountFallback.ts) is scoped per provider+connection+
+ * model, so callers that need to exclude a locked model must check it
+ * against each connection that could actually serve it — dropping the model
+ * only when every one of those connections has it locked (mirrors
+ * `isConnectionEligibleForModel` in
+ * open-sse/services/autoCombo/resilienceCandidateFilter.ts).
+ *
+ * Returns `null` on the same indeterminate cases as
+ * `hasUsableCredentialsForModel` (credential store unavailable) so callers
+ * can fail open identically. No-auth providers with no stored connection row
+ * resolve to the synthetic "noauth" connection id, matching the id
+ * `lockModel`/`isModelLocked` use for those providers elsewhere in the
+ * resilience layer.
+ */
+export async function getUsableConnectionsForModel(
+  model: string
+): Promise<UsableConnectionRef[] | null> {
+  const rawProvider = typeof model === "string" ? model.split("/")[0]?.trim() : "";
+  if (!rawProvider) return null;
+  const provider = resolveProviderId(rawProvider);
+  const isNoAuth = isNoAuthProviderKey(rawProvider, provider);
+  try {
+    const { getProviderConnections } = await loadProvidersModule();
+    const connections = await getProviderConnections({ provider, isActive: true });
+    if (!Array.isArray(connections)) return null;
+    if (connections.length === 0) {
+      return isNoAuth ? [{ id: SYNTHETIC_NOAUTH_CONNECTION_ID }] : [];
+    }
+    const usable = isNoAuth
+      ? connections.filter((c: any) => !hasTerminalConnectionStatus(c))
+      : connections.filter((c: any) => isProviderConnectionUsable(c));
+    return usable.map((c: any) => ({ id: String(c.id) }));
+  } catch {
+    return null;
+  }
+}

src/lib/guardrails/visionBridgeRouter.ts

@@ -7,8 +7,16 @@ import { getResolvedModelCapabilities } from "@/lib/modelCapabilities";
 import { getActiveSyncedCatalog } from "@/lib/db/models/activeSyncedCatalog";
 import { PROVIDER_MODELS } from "@omniroute/open-sse/config/providerModels";
 import { getRegisteredProviderEffortBaseModelId } from "@omniroute/open-sse/utils/registeredEffortVariants.ts";
-import { hasUsableCredentialsForModel } from "./visionBridgeCredentials";
+import {
+  hasUsableCredentialsForModel,
+  getUsableConnectionsForModel,
+} from "./visionBridgeCredentials";
 import { isVisionBridgeForcedModel } from "@/shared/constants/visionBridgeDefaults";
+import { resolveProviderId } from "@/shared/constants/providers";
+import {
+  isModelLocked,
+  getAllModelLockouts,
+} from "@omniroute/open-sse/services/accountFallback.ts";
 
 export interface VisionModelCandidate {
   modelId: string;
@@ -109,6 +117,12 @@ function calculateSuccessRate(modelId: string): number {
 export interface VisionBridgeRouterDeps {
   hasUsableCredentials?: (model: string) => Promise<boolean | null>;
   getActiveSyncedCatalog?: (provider: string) => Promise<VisionModelCatalog>;
+  /**
+   * (#12111) Per-connection model-lockout check, defaulting to the real
+   * `accountFallback.isModelLocked`. Injectable for the same reason as
+   * `hasUsableCredentials`: `node:test` has no supported ESM module-mocking.
+   */
+  isModelLocked?: (provider: string, connectionId: string, model: string) => boolean;
 }
 
 export interface VisionModelCatalog {
@@ -150,6 +164,53 @@ function createCatalogModelPredicate(
   };
 }
 
+/**
+ * connectionIds worth probing for a `(providerAlias, modelId)` lockout check:
+ * the provider's DB-known usable connections, plus any connectionId that
+ * already has an active lockout entry for this provider (#12111) — a 404
+ * lock (`accountFallback.lockModel`) can target a connectionId the DB-backed
+ * lookup does not surface (e.g. it predates a reconnect, or the credential
+ * check path a caller injected does not go through the same DB rows), and
+ * missing it would silently fail the exclusion open.
+ */
+function collectLockoutConnectionIds(providerAlias: string): string[] {
+  const canonicalProvider = resolveProviderId(providerAlias);
+  return getAllModelLockouts()
+    .filter((entry) => entry.provider === canonicalProvider)
+    .map((entry) => entry.connectionId);
+}
+
+/**
+ * (#12111) True unless `modelId` is locked (a post-404 model lockout, see
+ * `accountFallback.lockModel`) on every connection that could actually serve
+ * it. `isModelLocked` is scoped per provider+connection+model, so a single
+ * locked connection must not exclude a model that's still reachable through
+ * another connection on the same provider — mirrors
+ * `isConnectionEligibleForModel` in
+ * open-sse/services/autoCombo/resilienceCandidateFilter.ts. Fails open (never
+ * excludes) when nothing is known about the provider's connections, matching
+ * `hasUsableCredentialsForModel`'s existing fail-open contract — this check
+ * only narrows an already-credentialed candidate, it never widens the pool.
+ */
+async function isModelUsableGivenLockouts(
+  providerAlias: string,
+  modelId: string,
+  deps: VisionBridgeRouterDeps
+): Promise<boolean> {
+  const checkLocked = deps.isModelLocked ?? isModelLocked;
+  const dbConnections = await getUsableConnectionsForModel(`${providerAlias}/${modelId}`);
+  if (dbConnections === null) return true; // indeterminate credential store — fail open
+
+  const candidateIds = new Set(dbConnections.map((conn) => conn.id));
+  for (const id of collectLockoutConnectionIds(providerAlias)) candidateIds.add(id);
+  if (candidateIds.size === 0) return true; // nothing known about this provider's connections
+
+  for (const id of candidateIds) {
+    if (!checkLocked(providerAlias, id, modelId)) return true;
+  }
+  return false;
+}
+
 async function cachedModelRemainsAvailable(
   fullModelId: string,
   deps: VisionBridgeRouterDeps
@@ -162,6 +223,8 @@ async function cachedModelRemainsAvailable(
   const registryModel = PROVIDER_MODELS[providerAlias]?.find((model) => model.id === modelId);
   if (!registryModel) return false;
 
+  if (!(await isModelUsableGivenLockouts(providerAlias, modelId, deps))) return false;
+
   const catalog = await readActiveCatalog(providerAlias, deps);
   return createCatalogModelPredicate(providerAlias, catalog)(registryModel);
 }
@@ -193,13 +256,27 @@ async function getVisionCapableModels(
       });
       if (visionModels.length === 0) return [];
 
-      const usableModels = (
+      const credentialedModels = (
         await Promise.all(
           visionModels.map(async (model) =>
             (await checkCreds(`${providerAlias}/${model.id}`)) === false ? null : model
           )
         )
       ).filter((model): model is (typeof visionModels)[number] => model !== null);
+      if (credentialedModels.length === 0) return [];
+
+      // (#12111) A healthy provider connection does not mean every model on
+      // it is servable: chatCore.ts locks one specific model for 120s on a
+      // 404 while leaving the connection active, so the credential check
+      // above never sees it. Drop only the models locked on every usable
+      // connection for this provider.
+      const usableModels = (
+        await Promise.all(
+          credentialedModels.map(async (model) =>
+            (await isModelUsableGivenLockouts(providerAlias, model.id, deps)) ? model : null
+          )
+        )
+      ).filter((model): model is (typeof credentialedModels)[number] => model !== null);
       if (usableModels.length === 0) return [];
 
       const catalog = await readActiveCatalog(providerAlias, deps);

tests/unit/guardrails/visionBridge12111Repro.test.ts

@@ -0,0 +1,64 @@
+/**
+ * TDD repro for issue #12111: Vision Bridge auto-router can select a model
+ * already locked after a 404.
+ *
+ * getVisionCapableModels() (src/lib/guardrails/visionBridgeRouter.ts) filters
+ * candidates only on the registry vision flag and hasUsableCredentialsForModel
+ * (connection-scoped). It never consults isModelLocked
+ * (open-sse/services/accountFallback.ts), which the provider layer sets on a
+ * 404 "model not found" (open-sse/handlers/chatCore.ts). This test locks a
+ * vision-capable model exactly as chatCore.ts would after a 404, then asks
+ * getBestVisionModel() for a pick while forcing every other vision-capable
+ * provider to look uncredentialed (mirroring the reporter's setup: only one
+ * provider connection is actually usable) -- the locked model must not win.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { getBestVisionModel, clearSelectionCache } =
+  await import("../../../src/lib/guardrails/visionBridgeRouter.ts");
+const { lockModel, clearAllModelLockouts, isModelLocked } =
+  await import("../../../open-sse/services/accountFallback.ts");
+
+test.beforeEach(() => {
+  clearSelectionCache();
+  clearAllModelLockouts();
+});
+
+test("getBestVisionModel must not select a model locked after a 404 (#12111)", async () => {
+  const provider = "nvidia";
+  const connectionId = "conn-nvidia-1";
+  // The issue's original log line named "moonshotai/kimi-k2.6"; the registry
+  // has since renamed that entry to "kimi-k3" (open-sse/config/providers/
+  // registry/nvidia/index.ts) but it resolves the same way: it is the first
+  // vision-capable nvidia model in registry order, so it is still the model
+  // getBestVisionModel picks first when only nvidia is credentialed.
+  const modelId = "moonshotai/kimi-k3";
+  const fullModelId = `${provider}/${modelId}`;
+
+  // Reproduce the exact runtime event from the issue log line:
+  // "[provider] Node <redacted> model not found (404) for <model>
+  //  - locking model for 120s (connection stays active)"
+  lockModel(provider, connectionId, modelId, "not_found", 120_000);
+  assert.equal(
+    isModelLocked(provider, connectionId, modelId),
+    true,
+    "sanity check: accountFallback must report the model as locked"
+  );
+
+  // Only the nvidia provider looks credentialed -- mirrors the reporter's
+  // setup where the NVIDIA connection tests 200 all day (connection-scoped
+  // credential check passes) but the specific model 404s for the account.
+  const model = await getBestVisionModel(
+    {},
+    { hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
+  );
+
+  assert.notEqual(
+    model,
+    fullModelId,
+    "getBestVisionModel selected a model that accountFallback has locked after " +
+      "a 404 -- getVisionCapableModels() never consults isModelLocked " +
+      "(src/lib/guardrails/visionBridgeRouter.ts)"
+  );
+});

tests/unit/guardrails/visionBridgeRouter.test.ts

@@ -25,6 +25,10 @@ const {
   getLatencyStats,
 } = await import("../../../src/lib/guardrails/visionBridgeRouter.ts");
 const { PROVIDER_MODELS } = await import("../../../open-sse/config/providerModels.ts");
+const { lockModel, clearAllModelLockouts, isModelLocked } =
+  await import("../../../open-sse/services/accountFallback.ts");
+const { createProviderConnection, deleteProviderConnectionsByProvider } =
+  await import("../../../src/lib/db/providers.ts");
 type VisionBridgeRouterDepsT =
   import("../../../src/lib/guardrails/visionBridgeRouter.ts").VisionBridgeRouterDeps;
 
@@ -294,3 +298,103 @@ test("getLatencyStats — should return latency statistics", () => {
   assert.equal(stats["model-a"].avg, 110);
   assert.equal(stats["model-a"].successRate, 1);
 });
+
+// ── model-lockout exclusion (#12111) ────────────────────────────────────────
+// getVisionCapableModels() must consult accountFallback's per-connection
+// model lockout (set by chatCore.ts on a 404) in addition to the credential
+// check, and drop a model only when every usable connection has it locked —
+// see tests/unit/guardrails/visionBridge12111Repro.test.ts for the original
+// end-to-end reproduction against the exact reporter setup. These cases
+// exercise the same production code path (getBestVisionModel →
+// getVisionCapableModels → isModelUsableGivenLockouts) with a synthetic
+// registry entry, following the pattern in "accepts a registry model whose
+// liveCatalogIds match upstream" above.
+
+test("getBestVisionModel — excludes a model locked on its only usable connection (#12111)", async () => {
+  const provider = "__vision-bridge-lockout-test-1__";
+  const connectionId = "conn-1";
+  const modelId = "synthetic-vision-model";
+  PROVIDER_MODELS[provider] = [
+    { id: modelId, name: "Synthetic Vision Model", supportsVision: true },
+  ];
+  clearAllModelLockouts();
+  lockModel(provider, connectionId, modelId, "not_found", 120_000);
+
+  try {
+    const model = await getBestVisionModel(
+      {},
+      { hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
+    );
+    assert.notEqual(model, `${provider}/${modelId}`);
+  } finally {
+    delete PROVIDER_MODELS[provider];
+    clearAllModelLockouts();
+  }
+});
+
+test("getBestVisionModel — keeps a model locked on one connection while a second connection stays usable (#12111)", async () => {
+  const provider = "__vision-bridge-lockout-test-2__";
+  const modelId = "synthetic-vision-model";
+  PROVIDER_MODELS[provider] = [
+    { id: modelId, name: "Synthetic Vision Model", supportsVision: true },
+  ];
+  clearAllModelLockouts();
+
+  const lockedConn = await createProviderConnection({
+    provider,
+    authType: "apikey",
+    apiKey: "sk-test-locked",
+  });
+  const openConn = await createProviderConnection({
+    provider,
+    authType: "apikey",
+    apiKey: "sk-test-open",
+  });
+  lockModel(provider, (lockedConn as { id: string }).id, modelId, "not_found", 120_000);
+  // Sanity: the OTHER connection must not itself be locked.
+  assert.equal(isModelLocked(provider, (openConn as { id: string }).id, modelId), false);
+
+  try {
+    const model = await getBestVisionModel(
+      {},
+      { hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
+    );
+    assert.equal(
+      model,
+      `${provider}/${modelId}`,
+      "a model locked on only ONE of two usable connections must stay selectable"
+    );
+  } finally {
+    delete PROVIDER_MODELS[provider];
+    clearAllModelLockouts();
+    await deleteProviderConnectionsByProvider(provider);
+  }
+});
+
+test("getBestVisionModel — drops a cached selection once it becomes locked mid-window (#12111)", async () => {
+  const provider = "__vision-bridge-lockout-test-3__";
+  const connectionId = "conn-1";
+  const modelId = "synthetic-vision-model";
+  PROVIDER_MODELS[provider] = [
+    { id: modelId, name: "Synthetic Vision Model", supportsVision: true },
+  ];
+  clearAllModelLockouts();
+  const deps = { hasUsableCredentials: async (id: string) => id.startsWith(`${provider}/`) };
+
+  try {
+    // First call populates the 60s selection cache with the only candidate.
+    assert.equal(await getBestVisionModel({}, deps), `${provider}/${modelId}`);
+
+    // The model 404s and gets locked mid-cache-window, exactly like chatCore.ts.
+    lockModel(provider, connectionId, modelId, "not_found", 120_000);
+
+    // A cache hit that never re-validates lockouts would keep returning the
+    // now-locked model for up to 60s of further failing requests (the
+    // reporter's complaint); it must fall through to "no usable candidate".
+    assert.equal(await getBestVisionModel({}, deps), null);
+  } finally {
+    delete PROVIDER_MODELS[provider];
+    clearAllModelLockouts();
+    clearSelectionCache();
+  }
+});