mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 19:02:17 +03:00
fix(guardrails): keep auto combos exempt from the vision bridge credential guard (#12373)
getBestVisionModel() validates a configured fixedModel with hasUsableCredentialsForModel() before short-circuiting (#8430). auto / auto/* ids are virtual combos with no provider row, so that check always reported a confirmed false and the combo was silently discarded in favour of global auto-selection — it never got the chance to rotate its members. This mirrors the exemption the reroute guard in visionBridge.ts already carries; concrete fixedModel ids keep the #8430 fall-through unchanged. Validated in a combined worktree with all 25 PRs of this batch boarded together: typecheck:core clean, 443/443 node-runner tests plus 14/14 vitest across every test file the batch touches, and check-changelog-integrity, check:cycles (418 files), check:provider-consistency (272 REGISTRY entries, 355 canonical providers), check:docs-counts, check:docs-sync (42 locales) and check-file-size all green. Thanks @pacocartones.
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- **fix(guardrails):** keep `auto`/`auto/*` virtual combos exempt from the Vision Bridge `fixedModel` credential guard so a combo target is passed through instead of silently falling back to global auto-selection ([#12237](https://github.com/diegosouzapw/OmniRoute/issues/12237))
|
||||
@@ -231,7 +231,9 @@ async function getVisionCapableModels(
|
||||
};
|
||||
});
|
||||
|
||||
return candidates.filter((candidate): candidate is VisionModelCandidate => candidate !== null);
|
||||
return candidates.filter(
|
||||
(candidate): candidate is VisionModelCandidate => candidate !== null
|
||||
);
|
||||
})
|
||||
);
|
||||
|
||||
@@ -271,6 +273,51 @@ function selectBestModel(
|
||||
return scored[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* (#12237) `auto` / `auto/*` ids are VIRTUAL combos: there is no provider
|
||||
* row for "auto", so the credential check always reports `false` for them.
|
||||
* Member-level credentials are enforced downstream when the combo
|
||||
* dispatches (mirrors the reroute guard in visionBridge.ts), so a virtual
|
||||
* combo must not be discarded by the #8430 short-circuit — otherwise the
|
||||
* combo silently falls through to auto-selection and never rotates. It is
|
||||
* still subject to the pool check in `getBestVisionModel`: when the ENTIRE
|
||||
* vision pool is unusable there is nothing the combo could dispatch to, and
|
||||
* returning the combo id would let a raw image reach a text-only backend
|
||||
* (#8430).
|
||||
*
|
||||
* Returns the combo id when `fixedModel` is virtual, `undefined` otherwise.
|
||||
*/
|
||||
function resolveVirtualCombo(fixedModel: string | undefined): string | undefined {
|
||||
return fixedModel === "auto" || fixedModel?.startsWith("auto/") ? fixedModel : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a live selection-cache entry for `cacheKey`.
|
||||
*
|
||||
* Returns the id to hand back: the cached member for a concrete target, or
|
||||
* `virtualCombo` once the cached member proves it still has usable
|
||||
* credentials (the cache never re-validates credentials, and the caller
|
||||
* exempts virtual combos from that check). A missing or expired entry yields
|
||||
* `null`; an entry whose member is no longer available or usable is dropped
|
||||
* so the pool is rescanned.
|
||||
*/
|
||||
async function resolveCachedSelection(
|
||||
cacheKey: string,
|
||||
virtualCombo: string | undefined,
|
||||
deps: VisionBridgeRouterDeps
|
||||
): Promise<string | null> {
|
||||
const cached = selectionCache.get(cacheKey);
|
||||
if (!cached || cached.expiresAt <= Date.now()) return null;
|
||||
|
||||
if (await cachedModelRemainsAvailable(cached.modelId, deps)) {
|
||||
if (!virtualCombo) return cached.modelId;
|
||||
const checkCreds = deps.hasUsableCredentials ?? hasUsableCredentialsForModel;
|
||||
if ((await checkCreds(cached.modelId)) !== false) return virtualCombo;
|
||||
}
|
||||
selectionCache.delete(cacheKey);
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the best vision model for image description.
|
||||
* Respects fixed model override if configured, but validates it has usable
|
||||
@@ -283,12 +330,15 @@ export async function getBestVisionModel(
|
||||
deps: VisionBridgeRouterDeps = {}
|
||||
): Promise<string | null> {
|
||||
const fullConfig = { ...DEFAULT_ROUTER_CONFIG, ...config };
|
||||
const virtualCombo = resolveVirtualCombo(fullConfig.fixedModel);
|
||||
|
||||
// If fixed model is configured, validate it has usable credentials first.
|
||||
// (#8430) An unreachable fixedModel (e.g. the default "openai/gpt-4o-mini"
|
||||
// on an instance with no OpenAI connection/key) must not short-circuit the
|
||||
// credential check — fall through to auto-selection instead.
|
||||
if (fullConfig.fixedModel) {
|
||||
// (#12237) A virtual combo is exempt here and goes through the pool
|
||||
// selection below instead; see `resolveVirtualCombo`.
|
||||
if (fullConfig.fixedModel && !virtualCombo) {
|
||||
const checkCreds = deps.hasUsableCredentials ?? hasUsableCredentialsForModel;
|
||||
const usable = await checkCreds(fullConfig.fixedModel);
|
||||
// Only skip credential validation when the check is indeterminate (null).
|
||||
@@ -304,13 +354,8 @@ export async function getBestVisionModel(
|
||||
fullConfig.excludedModels.length > 0
|
||||
? `excl:${[...fullConfig.excludedModels].sort().join(",")}`
|
||||
: "default";
|
||||
const cached = selectionCache.get(cacheKey);
|
||||
if (cached && cached.expiresAt > Date.now()) {
|
||||
if (await cachedModelRemainsAvailable(cached.modelId, deps)) {
|
||||
return cached.modelId;
|
||||
}
|
||||
selectionCache.delete(cacheKey);
|
||||
}
|
||||
const cachedPick = await resolveCachedSelection(cacheKey, virtualCombo, deps);
|
||||
if (cachedPick) return cachedPick;
|
||||
|
||||
// Get all vision-capable candidates
|
||||
const candidates = await getVisionCapableModels(deps);
|
||||
@@ -329,7 +374,9 @@ export async function getBestVisionModel(
|
||||
expiresAt: Date.now() + fullConfig.selectionCacheTtlMs,
|
||||
});
|
||||
|
||||
return best.fullName;
|
||||
// A virtual combo is returned as-is once the pool proves at least one
|
||||
// vision-capable member is usable; it rotates its own members downstream.
|
||||
return virtualCombo ?? best.fullName;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -80,8 +80,65 @@ test("getBestVisionModel — should exclude specified models", async () => {
|
||||
test("getBestVisionModel — excludes a candidate with no usable active connection", async () => {
|
||||
// Every candidate reports a confirmed-unusable connection (`false`) ->
|
||||
// no candidate survives -> returns null instead of an unreachable default.
|
||||
const model = await getBestVisionModel({}, { hasUsableCredentials: async () => false });
|
||||
assert.equal(model, null);
|
||||
});
|
||||
|
||||
// `auto` / `auto/*` ids are VIRTUAL combos: there is no provider row for
|
||||
// "auto", so hasUsableCredentialsForModel reports a confirmed `false` for the
|
||||
// combo id itself while the pool members remain usable (indeterminate here).
|
||||
const virtualComboOnlyUnusable = async (fullModelId: string) =>
|
||||
fullModelId === "auto" || fullModelId.startsWith("auto/") ? false : null;
|
||||
|
||||
test("getBestVisionModel — keeps an auto/* virtual-combo fixedModel when its credential check is false (#12237)", async () => {
|
||||
// The #8430 short-circuit must not discard the combo — member credentials
|
||||
// are enforced downstream when the combo dispatches (same exemption as the
|
||||
// reroute guard in visionBridge.ts).
|
||||
const fixedModel = "auto/vision";
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{ fixedModel },
|
||||
{ hasUsableCredentials: virtualComboOnlyUnusable }
|
||||
);
|
||||
assert.equal(model, fixedModel);
|
||||
});
|
||||
|
||||
test('getBestVisionModel — keeps a bare "auto" fixedModel when its credential check is false (#12237)', async () => {
|
||||
const model = await getBestVisionModel(
|
||||
{ fixedModel: "auto" },
|
||||
{ hasUsableCredentials: virtualComboOnlyUnusable }
|
||||
);
|
||||
assert.equal(model, "auto");
|
||||
});
|
||||
|
||||
test("getBestVisionModel — keeps an auto/* virtual-combo fixedModel on a cached pool selection (#12237)", async () => {
|
||||
// Warm the selection cache with a pool pick, then ask for the combo: the
|
||||
// cache-hit branch must still hand back the combo, not the cached member.
|
||||
const warm = await getBestVisionModel({}, { hasUsableCredentials: virtualComboOnlyUnusable });
|
||||
assert.ok(warm);
|
||||
const model = await getBestVisionModel(
|
||||
{ fixedModel: "auto/vision" },
|
||||
{ hasUsableCredentials: virtualComboOnlyUnusable }
|
||||
);
|
||||
assert.equal(model, "auto/vision");
|
||||
});
|
||||
|
||||
test("getBestVisionModel — discards an auto/* virtual-combo fixedModel when the ENTIRE vision pool is unusable (#8430)", async () => {
|
||||
// The exemption only bypasses the credential check on the virtual id. With
|
||||
// no usable vision-capable member anywhere, the combo has nothing to
|
||||
// dispatch to and must fall through to `null` so the caller describes
|
||||
// instead of forwarding a raw image to a text-only backend.
|
||||
const model = await getBestVisionModel(
|
||||
{ fixedModel: "auto/vision" },
|
||||
{ hasUsableCredentials: async () => false }
|
||||
);
|
||||
assert.equal(model, null);
|
||||
});
|
||||
|
||||
test("getBestVisionModel — still falls through when a concrete fixedModel has no usable credentials (#8430)", async () => {
|
||||
// Regression guard for the exemption above: a non-virtual fixedModel with
|
||||
// a confirmed-unusable credential check must still be discarded.
|
||||
const model = await getBestVisionModel(
|
||||
{ fixedModel: "openai/gpt-4o-mini" },
|
||||
{ hasUsableCredentials: async () => false }
|
||||
);
|
||||
assert.equal(model, null);
|
||||
@@ -105,20 +162,17 @@ test("getBestVisionModel — does not query live catalogs for providers without
|
||||
assert.equal(catalogCalls, 0);
|
||||
});
|
||||
|
||||
test(
|
||||
"getBestVisionModel — selects a credentialed candidate over an uncredentialed higher-priority one",
|
||||
async () => {
|
||||
// openai (priority 50, would normally win) has no usable connection;
|
||||
// every other vision-capable provider does.
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{
|
||||
hasUsableCredentials: async (fullModelId) => fullModelId.split("/")[0] !== "openai",
|
||||
}
|
||||
);
|
||||
assert.equal(model.startsWith("openai/"), false);
|
||||
}
|
||||
);
|
||||
test("getBestVisionModel — selects a credentialed candidate over an uncredentialed higher-priority one", async () => {
|
||||
// openai (priority 50, would normally win) has no usable connection;
|
||||
// every other vision-capable provider does.
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{
|
||||
hasUsableCredentials: async (fullModelId) => fullModelId.split("/")[0] !== "openai",
|
||||
}
|
||||
);
|
||||
assert.equal(model.startsWith("openai/"), false);
|
||||
});
|
||||
|
||||
test("getBestVisionModel — excludes static models missing from an authoritative live catalog", async () => {
|
||||
const model = await getBestVisionModel(
|
||||
@@ -188,17 +242,14 @@ test("getFallbackModels — should respect max fallback attempts", async () => {
|
||||
assert.ok(fallbacks.length <= 2);
|
||||
});
|
||||
|
||||
test(
|
||||
"getFallbackModels — does not include candidates with a confirmed-unusable connection",
|
||||
async () => {
|
||||
const fallbacks = await getFallbackModels(
|
||||
"openai/gpt-4o-mini",
|
||||
{},
|
||||
{ hasUsableCredentials: async (fullModelId) => fullModelId.split("/")[0] !== "anthropic" }
|
||||
);
|
||||
assert.ok(!fallbacks.some((m) => m.startsWith("anthropic/")));
|
||||
}
|
||||
);
|
||||
test("getFallbackModels — does not include candidates with a confirmed-unusable connection", async () => {
|
||||
const fallbacks = await getFallbackModels(
|
||||
"openai/gpt-4o-mini",
|
||||
{},
|
||||
{ hasUsableCredentials: async (fullModelId) => fullModelId.split("/")[0] !== "anthropic" }
|
||||
);
|
||||
assert.ok(!fallbacks.some((m) => m.startsWith("anthropic/")));
|
||||
});
|
||||
|
||||
test("getFallbackModels — excludes fallbacks missing from an authoritative live catalog", async () => {
|
||||
const fallbacks = await getFallbackModels(
|
||||
|
||||
Reference in New Issue
Block a user