From d7d98fe356d72b1aa255817680245c433c03d9c5 Mon Sep 17 00:00:00 2001 From: Hernan Javier Ardila Sanchez Date: Mon, 10 Aug 2026 08:25:18 +0200 Subject: [PATCH] fix(guardrails): vision bridge reroute/pool/self-loop fixes (#9946) - auto/best-vision and auto/pro-vision now resolve to the vision CATEGORY (candidate filter by capability) instead of the flat smart variant, so the vision-bridge describe/reroute target can actually see images (resolveBuiltinAutoSpec in builtinCatalog). - vision candidate pool excludes registry entries whose catalog OVERSTATES vision support (opencode-go/opencode-zen/tokenrouter are forced through the vision bridge by isVisionBridgeForcedModel) in both the auto-combo candidate filter (suffixComposition) and the vision router (visionBridgeRouter). - reroute guard: an auto/* target is a virtual combo; a missing 'auto' provider row (hasUsableCredentials=false) must never block the reroute. - claude-wire backends (minimax, zai, ...) reject remote image URLs (MiniMax 403 2013): ensureBase64ImagesForClaudeWire resolves URLs to base64 before rerouting, and the describe self-loop normalizes to base64 for those targets (isClaudeWireFormatModel). - self-loop describe uses a real DB-backed key (resolveSelfLoopApiKey) instead of the sk_omniroute sentinel rejected by REQUIRE_API_KEY instances, and bypasses the runtime's hooked global fetch via undici (ProxyFetch with a dead local proxy would otherwise break every describe); compression is disabled on the self-loop sub-request so image payloads are never mangled. Tests: vision-bridge-auto-reroute (2), vision-bridge-selfloop-key (4), vision-bridge-claude-wire (6), builtin-vision-spec (4), vision-filter-excludes-forced (4). Co-authored-by: herjarsa --- open-sse/services/autoCombo/builtinCatalog.ts | 68 ++++++- .../services/autoCombo/suffixComposition.ts | 12 +- src/lib/guardrails/visionBridge.ts | 28 ++- src/lib/guardrails/visionBridgeHelpers.ts | 170 ++++++++++++++++-- src/lib/guardrails/visionBridgeRouter.ts | 3 +- .../autoCombo/builtin-vision-spec.test.ts | 44 +++++ .../vision-filter-excludes-forced.test.ts | 39 ++++ .../vision-bridge-auto-reroute.test.ts | 103 +++++++++++ .../vision-bridge-claude-wire.test.ts | 126 +++++++++++++ .../vision-bridge-selfloop-key.test.ts | 64 +++++++ 10 files changed, 635 insertions(+), 22 deletions(-) create mode 100644 tests/unit/autoCombo/builtin-vision-spec.test.ts create mode 100644 tests/unit/autoCombo/vision-filter-excludes-forced.test.ts create mode 100644 tests/unit/guardrails/vision-bridge-auto-reroute.test.ts create mode 100644 tests/unit/guardrails/vision-bridge-claude-wire.test.ts create mode 100644 tests/unit/guardrails/vision-bridge-selfloop-key.test.ts diff --git a/open-sse/services/autoCombo/builtinCatalog.ts b/open-sse/services/autoCombo/builtinCatalog.ts index f2f12f6328..adaca208b3 100644 --- a/open-sse/services/autoCombo/builtinCatalog.ts +++ b/open-sse/services/autoCombo/builtinCatalog.ts @@ -1,6 +1,6 @@ import type { AutoVariant } from "./autoPrefix"; import { VALID_VARIANTS } from "./autoPrefix"; -import { parseAutoSuffix } from "./suffixComposition"; +import { parseAutoSuffix, type AutoCategory, type AutoTier } from "./suffixComposition"; import { isValidModelFamily, AUTO_FAMILY_IDS } from "./modelFamily"; export { AUTO_FAMILY_IDS }; @@ -112,13 +112,71 @@ export function isPaidTierAutoId(autoId: string): boolean { return parsed.valid && parsed.tier === "pro"; } -export async function createBuiltinAutoCombo(modelStr: string, suffix: string) { - const { createVirtualAutoCombo } = await import("./virtualFactory.ts"); +/** + * Resolved spec for a built-in `auto/*` id: either a flat variant (legacy) or + * a category/tier overlay (#4235 Phase B). Category `vision`/`multimodal` adds + * a candidate filter so the virtual combo only scores vision-capable models. + */ +export type BuiltinAutoSpec = + | { variant: AutoVariant | undefined } + | { category: AutoCategory; tier?: AutoTier }; + +/** + * Vision-flavored flat ids that MUST resolve to the `vision` category (candidate + * filter by capability), not to a flat variant: the vision-bridge guardrail and + * its self-loop depend on `auto/best-vision` picking a model that can actually + * see images. Mapping it to `smart` scored ALL candidates and resolved to + * text-only models (e.g. deepseek-v4-flash-free), breaking every describe call. + */ +const VISION_CATEGORY_AUTO_IDS: Record = { + "auto/best-vision": { category: "vision" }, + "auto/pro-vision": { category: "vision", tier: "pro" }, +}; + +/** + * Pure resolver for a built-in `auto/*` id. Extracted from + * `createBuiltinAutoCombo` so the catalog mapping is unit-testable without + * materializing a virtual combo (which requires the DB). + */ +export function resolveBuiltinAutoSpec(modelStr: string, suffix: string): BuiltinAutoSpec { + const visionSpec = VISION_CATEGORY_AUTO_IDS[modelStr]; + if (visionSpec) return visionSpec; const resolved = resolveAutoVariant(modelStr, suffix); if (resolved.recognized) { - const spec = modelStr === "auto/best-free" ? { tier: "free" as const } : undefined; - const virtualCombo = await createVirtualAutoCombo(resolved.variant, spec); + return { variant: resolved.variant }; + } + + const parsed = parseAutoSuffix(suffix); + if (parsed.valid) { + return { + category: parsed.category as AutoCategory, + ...(parsed.tier ? { tier: parsed.tier } : {}), + }; + } + + return { variant: undefined }; +} + +export async function createBuiltinAutoCombo(modelStr: string, suffix: string) { + const { createVirtualAutoCombo } = await import("./virtualFactory.ts"); + const spec = resolveBuiltinAutoSpec(modelStr, suffix); + + if ("category" in spec) { + // #4235 Phase B category/tier path (incl. vision ids like auto/best-vision). + const virtualCombo = await createVirtualAutoCombo(undefined, { + category: spec.category, + ...(spec.tier ? { tier: spec.tier } : {}), + }); + virtualCombo.name = modelStr; + virtualCombo.id = modelStr; + return virtualCombo; + } + + if ("variant" in spec && spec.variant !== undefined) { + const virtualCombo = await createVirtualAutoCombo(spec.variant, { + ...(modelStr === "auto/best-free" ? { tier: "free" as const } : {}), + }); virtualCombo.name = modelStr; virtualCombo.id = modelStr; return virtualCombo; diff --git a/open-sse/services/autoCombo/suffixComposition.ts b/open-sse/services/autoCombo/suffixComposition.ts index fc5e11a2e6..1a288cc500 100644 --- a/open-sse/services/autoCombo/suffixComposition.ts +++ b/open-sse/services/autoCombo/suffixComposition.ts @@ -20,6 +20,7 @@ import type { AutoVariant } from "./autoPrefix"; import { classifyTier } from "../tierResolver"; import { getResolvedModelCapabilities } from "@/lib/modelCapabilities"; import { isVisionModelId } from "@/shared/constants/visionModels"; +import { isVisionBridgeForcedModel } from "@/shared/constants/visionBridgeDefaults"; export type AutoCategory = "coding" | "reasoning" | "vision" | "chat" | "multimodal"; export type AutoTier = "fast" | "cheap" | "floor" | "free" | "reliable" | "pro"; @@ -111,9 +112,16 @@ export function buildAutoCandidateFilter( checks.push((c) => { try { const caps = getResolvedModelCapabilities({ provider: c.provider, model: c.model }); - return caps.supportsVision === true || isVisionModelId(c.model); + const capable = + caps.supportsVision === true || isVisionModelId(c.model); + if (!capable) return false; + // #vison-pool: registry entries whose catalog OVERSTATES vision support + // (opencode-go/opencode-zen/tokenrouter — the backend models are text-only) + // are forced through the vision bridge by isVisionBridgeForcedModel. + // They must never be selected as the vision-capable candidate itself. + return !isVisionBridgeForcedModel(`${c.provider}/${c.model}`); } catch { - return isVisionModelId(c.model); + return isVisionModelId(c.model) && !isVisionBridgeForcedModel(`${c.provider}/${c.model}`); } }); } diff --git a/src/lib/guardrails/visionBridge.ts b/src/lib/guardrails/visionBridge.ts index f48ebd72b8..bc7f7efd86 100644 --- a/src/lib/guardrails/visionBridge.ts +++ b/src/lib/guardrails/visionBridge.ts @@ -13,7 +13,9 @@ import { callVisionModel as defaultCallVisionModel, composeVisionPrompt, replaceImageParts, + ensureBase64ImagesForClaudeWire, } from "./visionBridgeHelpers"; +import { fetch as undiciFetch } from "undici"; import { getVisionBridgeConfig, isVisionBridgeForcedModel, @@ -298,14 +300,27 @@ export class VisionBridgeGuardrail extends BaseGuardrail { const bestUsable = await checkCreds(bestModel); // Only block the reroute when we KNOW the target is unusable (false). // `null` (no DB / tests) fails open so existing unit tests keep working. - if (bestUsable === false) { + // `auto/*` ids (e.g. auto/best-vision) are VIRTUAL combos: credentials + // resolve through their member models at request time, so a missing + // "auto" provider row (hasUsableCredentialsForModel → false) must + // never block the reroute. + if (bestUsable === false && !bestModel.startsWith("auto/")) { context.log?.warn?.( "VISION_BRIDGE", `Vision reroute target ${bestModel} has no usable credentials; describing images instead of hijacking ${model}` ); } else { + // Claude-wire backends (minimax, zai, …) reject remote image URLs + // (MiniMax 403 2013); resolve them to base64 before rerouting so + // the rerouted request can actually be processed upstream. Use + // undici fetch to bypass the runtime's hooked global fetch. + const rerouteBody = await ensureBase64ImagesForClaudeWire( + body as Parameters[0], + bestModel, + undiciFetch as unknown as typeof fetch + ); const modifiedBody = { - ...(body as Record), + ...(rerouteBody as Record), model: bestModel, }; return { @@ -347,7 +362,14 @@ export class VisionBridgeGuardrail extends BaseGuardrail { // targets what the user actually asked instead of a generic caption. const lastUserText = extractLastUserText(messages); const composedPrompt = composeVisionPrompt(config.prompt, lastUserText, runtime.taskAware); - const describeConfig = { ...config, prompt: composedPrompt }; + // Bypass the runtime's hooked global fetch (ProxyFetch) for the self-loop + // describe call — a dead local proxy (127.0.0.1:8317) would otherwise break + // every describe. Tests inject their own callVisionModel. + const describeConfig = { + ...config, + prompt: composedPrompt, + fetchImpl: undiciFetch as unknown as typeof fetch, + }; // Shared describe cache (sha256 of contentRef+prompt+model): the same image // with the same prompt/model is described once per TTL. Failures are never diff --git a/src/lib/guardrails/visionBridgeHelpers.ts b/src/lib/guardrails/visionBridgeHelpers.ts index b8eb432017..c44e6bb2f9 100644 --- a/src/lib/guardrails/visionBridgeHelpers.ts +++ b/src/lib/guardrails/visionBridgeHelpers.ts @@ -6,6 +6,8 @@ import { fetchRemoteImage } from "@/shared/network/remoteImageFetch"; import { getRuntimePorts } from "@/lib/runtime/ports"; import { resolveSelfLoopBearer } from "@/shared/middleware/chatBodyAdmission"; import { getBestVisionModel, getFallbackModels, recordLatency } from "./visionBridgeRouter"; +import { REGISTRY } from "@omniroute/open-sse/config/providers"; +import { fetch as undiciFetch } from "undici"; /** * Provider to environment variable mapping for API key resolution. */ @@ -15,6 +17,27 @@ const PROVIDER_API_KEY_MAP: Record = { openai: "OPENAI_API_KEY", }; +// Providers whose wire format is Anthropic Messages ("claude"). Anthropic +// accepts `source: { type: "url" }` for images, but most claude-format backends +// (MiniMax, Z.AI, …) do NOT — they reject remote URLs (MiniMax: 403 code +// 2013). The vision bridge must deliver images as base64 for these targets, +// both in the describe self-loop and in the rerouted payload. +const CLAUDE_WIRE_PROVIDERS = new Set( + Object.entries(REGISTRY) + .filter(([, entry]) => entry.format === "claude") + .map(([id]) => id.toLowerCase()) +); + +/** + * True when `provider/model` targets a Claude-Messages wire format backend + * that cannot ingest remote image URLs and needs base64 instead. + */ +export function isClaudeWireFormatModel(model: string | null | undefined): boolean { + if (!model || typeof model !== "string") return false; + const provider = model.includes("/") ? model.split("/")[0].trim().toLowerCase() : ""; + return CLAUDE_WIRE_PROVIDERS.has(provider); +} + /** * Resolve API key based on model provider (issue #2232). * @@ -46,6 +69,43 @@ export function resolveProviderApiKey(model: string, explicitKey?: string): stri return process.env[envVar] || ""; } +let selfLoopKeyPromise: Promise | null = null; + +/** + * Resolve a real API key for the OmniRoute SELF-LOOP describe call. + * + * The `sk_omniroute` sentinel works only when REQUIRE_API_KEY is disabled; on + * REQUIRE_API_KEY instances it is rejected with 401 "Missing API key", which + * silently breaks every vision-bridge describe. Priority: + * 1. VISION_BRIDGE_API_KEY env (already handled by resolveProviderApiKey — + * kept here for the injected-resolver test path). + * 2. Injected resolver (tests) or the DB-backed `getOrCreateApiKey()` — + * memoized so at most one key is created per process. + * 3. `sk_omniroute` as a final fallback (local mode without auth). + */ +export async function resolveSelfLoopApiKey(resolver?: () => Promise): Promise { + const envKey = (process.env.VISION_BRIDGE_API_KEY || "").trim(); + if (envKey) return envKey; + if (resolver) { + const key = (await resolver()).trim(); + if (key) return key; + return "sk_omniroute"; + } + if (!selfLoopKeyPromise) { + selfLoopKeyPromise = (async () => { + try { + const { getOrCreateApiKey } = await import("@/shared/services/apiKeyResolver"); + const key = await getOrCreateApiKey(); + if (typeof key === "string" && key.trim().length > 0) return key.trim(); + } catch { + /* fall through */ + } + return "sk_omniroute"; + })(); + } + return selfLoopKeyPromise; +} + /** * Resolve the OpenAI-compatible base URL for non-Anthropic vision bridge calls * (issue #2232). @@ -152,6 +212,68 @@ export function extractImageParts(messages: RequestMessage[]): ImagePart[] { })); } +// Undici fetch with a browser-ish User-Agent: Wikimedia (and other CDNs) +// reject requests without a UA with HTTP 400, silently breaking remote image +// downloads in the describe path. +const VISION_BRIDGE_UA_FETCH: typeof fetch = ((input: RequestInfo | URL, init?: RequestInit) => + undiciFetch(input as string | URL, { + ...(init as Parameters[1]), + headers: { + "user-agent": "omniroute-vision-bridge", + ...((init?.headers as Record | undefined) ?? {}), + }, + })) as typeof fetch; + +/** + * Resolve every image part in the body to a base64 data URI when the target + * model speaks the Claude wire format (remote URLs unsupported by most + * claude-format backends, e.g. MiniMax 403 2013). Fail-open: an image that + * cannot be fetched is left untouched. + */ +export async function ensureBase64ImagesForClaudeWire( + body: RequestBody, + model: string, + fetchImpl: typeof fetch = VISION_BRIDGE_UA_FETCH +): Promise { + if (!isClaudeWireFormatModel(model)) return body; + const parts = extractImageParts(body.messages as RequestMessage[]); + if (parts.length === 0) return body; + + const resolved = await Promise.all( + parts.map(async (part) => { + const normalized = resolveImageAsDataUri(part.imageUrl); + if (normalized.startsWith("data:")) return null; // already base64 + try { + return await fetchRemoteImageAsDataUri(normalized, new AbortController().signal, fetchImpl); + } catch { + return null; // fail-open: keep the original part + } + }) + ); + + // Map sequential image index → resolved data URI (null = keep original). + const byIndex = new Map(); + parts.forEach((part, i) => { + if (resolved[i]) byIndex.set(i, resolved[i] as string); + }); + if (byIndex.size === 0) return body; + + const result = structuredClone(body) as RequestBody; + let imageIndex = 0; + for (const message of result.messages ?? []) { + if (!message || !Array.isArray(message.content)) continue; + for (const part of message.content as RequestContentPart[]) { + if (part.type !== "image_url" && part.type !== "image") continue; + const dataUri = byIndex.get(imageIndex); + imageIndex++; + if (dataUri) { + (part as { image_url?: { url: string } }).image_url = { url: dataUri }; + } + } + } + return result; +} + /** * Resolve image URL to data URI format for vision model. * - HTTP/HTTPS URLs: passed through as-is @@ -178,8 +300,17 @@ export function resolveImageAsDataUri(imageUrl: string): string { return `data:image/png;base64,${imageUrl}`; } -async function fetchRemoteImageAsDataUri(imageUrl: string, signal: AbortSignal): Promise { - const remoteImage = await fetchRemoteImage(imageUrl, { signal }); +async function fetchRemoteImageAsDataUri( + imageUrl: string, + signal: AbortSignal, + fetchImpl: typeof fetch = VISION_BRIDGE_UA_FETCH +): Promise { + const remoteImage = await fetchRemoteImage(imageUrl, { + signal, + // Bypass the runtime's hooked global fetch (ProxyFetch) — a dead local + // proxy (e.g. 127.0.0.1:8317) would otherwise break the download. + fetchImpl, + }); const mediaType = remoteImage.contentType.split(";")[0]?.trim() || "image/png"; return `data:${mediaType};base64,${remoteImage.buffer.toString("base64")}`; } @@ -187,7 +318,8 @@ async function fetchRemoteImageAsDataUri(imageUrl: string, signal: AbortSignal): async function normalizeVisionImageInput( imageInput: string, isAnthropic: boolean, - signal: AbortSignal + signal: AbortSignal, + fetchImpl?: typeof fetch ): Promise { const normalizedImage = resolveImageAsDataUri(imageInput); @@ -195,7 +327,7 @@ async function normalizeVisionImageInput( isAnthropic && (normalizedImage.startsWith("http://") || normalizedImage.startsWith("https://")) ) { - return fetchRemoteImageAsDataUri(normalizedImage, signal); + return fetchRemoteImageAsDataUri(normalizedImage, signal, fetchImpl); } return normalizedImage; @@ -206,6 +338,8 @@ export interface VisionModelConfig { prompt: string; timeoutMs: number; maxImages: number; + /** Injectable fetch (tests). Defaults to undici fetch to bypass the runtime's hooked global fetch. */ + fetchImpl?: typeof fetch; } /** Task-aware focus hint (codex-vision-proxy pattern): steer the description @@ -481,17 +615,27 @@ async function callVisionModelSingle( // Resolve API key based on provider const resolvedApiKey = resolveProviderApiKey(config.model, apiKey); + // Production callers (VisionBridgeGuardrail) inject undici fetch to bypass + // the runtime's hooked global fetch (ProxyFetch). Defaults to globalThis.fetch + // so existing unit tests that mock it keep working. + const fetchImpl = config.fetchImpl ?? globalThis.fetch; - // Detect provider from model identifier + // Detect provider from model identifier. Claude-wire targets (minimax, zai, + // …) cannot ingest remote image URLs — normalize to base64 so the self-loop + // body reaches the backend as a data URI (the OpenAI→claude translator only + // preserves data URIs as base64; remote URLs become source.url which these + // backends reject). const isAnthropic = config.model.startsWith("anthropic/"); + const requiresBase64 = isAnthropic || isClaudeWireFormatModel(config.model); try { // Extract model name from provider/model format const modelName = config.model.includes("/") ? config.model.split("/")[1] : config.model; const normalizedImageInput = await normalizeVisionImageInput( imageDataUri, - isAnthropic, - controller.signal + requiresBase64, + controller.signal, + fetchImpl ); let response: Response; @@ -510,7 +654,7 @@ async function callVisionModelSingle( base64Data = matches[2]; } - response = await fetch(`${anthropicBaseUrl}/v1/messages`, { + response = await fetchImpl(`${anthropicBaseUrl}/v1/messages`, { method: "POST", signal: controller.signal, headers: { @@ -561,8 +705,9 @@ async function callVisionModelSingle( // Build headers with optional recursion guard for self-loop calls. // When routing through OmniRoute's own API, omit the vision-bridge // guardrail on the sub-request to prevent infinite recursion. - // Use sk_omniroute as fallback for self-loop if no API key is resolved. - const selfLoopApiKey = resolvedApiKey || "sk_omniroute"; + // Use a real DB-backed key for self-loop (sk_omniroute is rejected by + // REQUIRE_API_KEY instances with 401 "Missing API key"). + const selfLoopApiKey = resolvedApiKey || (await resolveSelfLoopApiKey()); const headers: Record = { "Content-Type": "application/json", // Explicit JSON opt-in: without `Accept: application/json` OmniRoute's @@ -583,6 +728,9 @@ async function callVisionModelSingle( // `sk_omniroute` sentinel OR the operator-configured env key), so // external clients cannot use it to bypass admission. headers["x-omniroute-admission-bypass"] = "internal"; + // The compression pipeline must not touch the image payload of the + // self-loop describe call (stacked RTK/Caveman can mangle data URIs). + headers["x-omniroute-compression"] = "off"; // The admission bypass honors the env key when set (REQUIRE_API_KEY=true // deployments) and the `sk_omniroute` sentinel otherwise. Force the same // resolved credential so the bypass holds even when a real vision key is @@ -590,7 +738,7 @@ async function callVisionModelSingle( headers["Authorization"] = `Bearer ${resolveSelfLoopBearer()}`; } - response = await fetch(`${baseUrl}/chat/completions`, { + response = await fetchImpl(`${baseUrl}/chat/completions`, { method: "POST", signal: controller.signal, headers, diff --git a/src/lib/guardrails/visionBridgeRouter.ts b/src/lib/guardrails/visionBridgeRouter.ts index 9ea04025e9..d7f6b8a62f 100644 --- a/src/lib/guardrails/visionBridgeRouter.ts +++ b/src/lib/guardrails/visionBridgeRouter.ts @@ -6,6 +6,7 @@ import { getResolvedModelCapabilities } from "@/lib/modelCapabilities"; import { PROVIDER_MODELS, PROVIDER_ID_TO_ALIAS } from "@omniroute/open-sse/config/providerModels"; import { hasUsableCredentialsForModel } from "./visionBridgeCredentials"; +import { isVisionBridgeForcedModel } from "@/shared/constants/visionBridgeDefaults"; export interface VisionModelCandidate { modelId: string; @@ -133,7 +134,7 @@ async function getVisionCapableModels( const fullModelId = `${providerAlias}/${model.id}`; const caps = getResolvedModelCapabilities(fullModelId); - if (caps.supportsVision === true) { + if (caps.supportsVision === true && !isVisionBridgeForcedModel(fullModelId)) { checks.push( checkCreds(fullModelId).then((usable) => { // Only a confirmed `false` excludes a candidate — `null` (indeterminate, diff --git a/tests/unit/autoCombo/builtin-vision-spec.test.ts b/tests/unit/autoCombo/builtin-vision-spec.test.ts new file mode 100644 index 0000000000..fc65a50a40 --- /dev/null +++ b/tests/unit/autoCombo/builtin-vision-spec.test.ts @@ -0,0 +1,44 @@ +/** + * Regression: `auto/best-vision` must resolve to the `vision` CATEGORY (candidate + * filter by vision capability), not to the flat `smart` variant. + * + * Root cause on runtime v3.8.49: AUTO_TEMPLATE_VARIANTS mapped + * `"auto/best-vision": "smart"`, so the virtual combo scored ALL candidates + * (verified: it resolved to text-only `deepseek-v4-flash-free`), making the + * vision-bridge describe/reroute target useless. + * + * Runs under Vitest (the autoCombo suite is Vitest-only in this repo). + */ +import { describe, it, expect } from "vitest"; +import { resolveBuiltinAutoSpec } from "../../../open-sse/services/autoCombo/builtinCatalog"; + +describe("resolveBuiltinAutoSpec — vision category ids", () => { + it("auto/best-vision resolves to category vision (not smart variant)", () => { + expect(resolveBuiltinAutoSpec("auto/best-vision", "best-vision")).toEqual({ + category: "vision", + }); + }); + + it("auto/pro-vision resolves to category vision + tier pro", () => { + expect(resolveBuiltinAutoSpec("auto/pro-vision", "pro-vision")).toEqual({ + category: "vision", + tier: "pro", + }); + }); + + it("legacy flat variants keep their variant mapping", () => { + expect(resolveBuiltinAutoSpec("auto/best-coding", "best-coding")).toEqual({ + variant: "coding", + }); + expect(resolveBuiltinAutoSpec("auto/fast", "fast")).toEqual({ variant: "fast" }); + expect(resolveBuiltinAutoSpec("auto/chat", "chat")).toEqual({ variant: undefined }); + }); + + it("category:tier suffix still resolves via parseAutoSuffix", () => { + expect(resolveBuiltinAutoSpec("auto/coding:fast", "coding:fast")).toEqual({ + category: "coding", + tier: "fast", + }); + expect(resolveBuiltinAutoSpec("auto/vision", "vision")).toEqual({ category: "vision" }); + }); +}); \ No newline at end of file diff --git a/tests/unit/autoCombo/vision-filter-excludes-forced.test.ts b/tests/unit/autoCombo/vision-filter-excludes-forced.test.ts new file mode 100644 index 0000000000..15d45de6dc --- /dev/null +++ b/tests/unit/autoCombo/vision-filter-excludes-forced.test.ts @@ -0,0 +1,39 @@ +/** + * Regression: the vision-category candidate filter must exclude registry + * entries whose catalog OVERSTATES vision support (opencode-go/opencode-zen/ + * tokenrouter backends are text-only and are forced through the vision bridge + * by isVisionBridgeForcedModel). Otherwise `auto/best-vision` pools include + * models that can never process images (e.g. deepseek-v4-flash-max), breaking + * the vision bridge describe/reroute. + */ +import { describe, it, expect } from "vitest"; +import { buildAutoCandidateFilter } from "../../../open-sse/services/autoCombo/suffixComposition"; + +describe("buildAutoCandidateFilter — vision category", () => { + it("keeps genuinely vision-capable models", () => { + const filter = buildAutoCandidateFilter("vision"); + expect(filter).not.toBeNull(); + // MiniMax M3 is a real multimodal model (format claude, supportsVision: true). + expect(filter?.({ provider: "minimax", model: "MiniMax-M3" })).toBe(true); + }); + + it("rejects models whose catalog entry overstates vision (forced through the bridge)", () => { + const filter = buildAutoCandidateFilter("vision"); + // opencode-go/deepseek-v4-flash-max is in FORCED_VISION_BRIDGE_MODELS — + // the catalog claims vision but the backend is text-only. + expect(filter?.({ provider: "opencode-go", model: "deepseek-v4-flash-max" })).toBe(false); + expect(filter?.({ provider: "opencode-go", model: "deepseek-v4-flash" })).toBe(false); + expect(filter?.({ provider: "opencode-zen", model: "deepseek-v4-flash" })).toBe(false); + }); + + it("rejects models with no confirmed vision support", () => { + const filter = buildAutoCandidateFilter("vision"); + // Unknown catalog entry → no confirmed vision → must be rejected. + expect(filter?.({ provider: "acme", model: "acme-text" })).toBe(false); + }); + + it("non-vision categories are unaffected", () => { + const filter = buildAutoCandidateFilter("coding"); + expect(filter).toBeNull(); + }); +}); diff --git a/tests/unit/guardrails/vision-bridge-auto-reroute.test.ts b/tests/unit/guardrails/vision-bridge-auto-reroute.test.ts new file mode 100644 index 0000000000..638969b42b --- /dev/null +++ b/tests/unit/guardrails/vision-bridge-auto-reroute.test.ts @@ -0,0 +1,103 @@ +/** + * Regression: the vision-bridge reroute must work when the configured vision + * model is an `auto/*` virtual id, and the describe path must only run when the + * vision pool is genuinely empty. + * + * Upstream v3.8.50 resolves `auto/*` fixedModels through the vision router + * pool; the guardrail-level guard (`bestUsable === false && !auto/*`) keeps the + * reroute from being blocked when the router returns an unresolved auto id. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { VisionBridgeGuardrail } = await import("../../../src/lib/guardrails/visionBridge.ts"); +const { resetGuardrailsForTests } = await import("../../../src/lib/guardrails/registry.ts"); +import type { GuardrailContext } from "../../../src/lib/guardrails/base.ts"; +import type { VisionModelConfig } from "../../../src/lib/guardrails/visionBridgeHelpers.ts"; + +let mockSettings: Record = {}; +let visionCallCount = 0; +let credentialsMock: (model: string) => Promise = async () => null; + +function createGuardrail(options?: Parameters[0]) { + return new VisionBridgeGuardrail({ + ...options, + deps: { + getSettings: async () => mockSettings, + callVisionModel: async (_imageDataUri: string, _config: VisionModelConfig) => { + visionCallCount++; + return "described"; + }, + hasUsableCredentials: credentialsMock, + ...(options?.deps ?? {}), + }, + }); +} + +function createContext(overrides: Partial = {}): GuardrailContext { + return { model: "deepseek/deepseek-chat", log: console, ...overrides }; +} + +function imagePayload(overrides: Record = {}): Record { + return { + model: "deepseek/deepseek-chat", + messages: [ + { + role: "user", + content: [ + { type: "text", text: "What is in this image?" }, + { type: "image_url", image_url: { url: "https://example.com/cat.png" } }, + ], + }, + ], + ...overrides, + }; +} + +function baseSettings() { + return { + visionBridgeEnabled: true, + visionBridgeModel: "auto/best-vision", + visionBridgePrompt: "Describe this image concisely.", + visionBridgeTimeout: 30000, + visionBridgeMaxImages: 10, + }; +} + +test.beforeEach(() => { + resetGuardrailsForTests({ registerDefaults: false }); + visionCallCount = 0; + credentialsMock = async () => null; + mockSettings = baseSettings(); +}); + +test("VB-REROUTE-AUTO: auto/best-vision resolves through the router pool and reroutes (no describe)", async () => { + // Provider "auto" has no credential rows → hasUsableCredentials=false for the + // raw auto id; the router must still resolve a pool model and reroute. + credentialsMock = async (model: string) => (model.startsWith("auto/") ? false : null); + + const guardrail = createGuardrail(); + const result = await guardrail.preCall(imagePayload(), createContext()); + + assert.strictEqual(result.block, false); + assert.strictEqual(visionCallCount, 0, "describe path must not run when a vision target exists"); + assert.ok(result.modifiedPayload, "payload must be modified"); + const body = result.modifiedPayload as Record; + // The reroute points the request at the resolved vision model from the pool. + assert.notStrictEqual(body.model, "deepseek/deepseek-chat"); + assert.deepEqual((result.meta as Record).rerouted, true); +}); + +test("VB-REROUTE-AUTO: falls back to describe only when the ENTIRE vision pool is unusable", async () => { + // Every vision candidate is confirmed unusable → nothing to reroute to → the + // describe path runs (existing behavior). + credentialsMock = async () => false; + + const guardrail = createGuardrail(); + const result = await guardrail.preCall(imagePayload(), createContext()); + + assert.strictEqual(result.block, false); + assert.strictEqual(visionCallCount, 1, "describe path must run when no vision target is usable"); + const body = result.modifiedPayload as Record; + assert.strictEqual(body.model, "deepseek/deepseek-chat"); +}); diff --git a/tests/unit/guardrails/vision-bridge-claude-wire.test.ts b/tests/unit/guardrails/vision-bridge-claude-wire.test.ts new file mode 100644 index 0000000000..93f6a7e680 --- /dev/null +++ b/tests/unit/guardrails/vision-bridge-claude-wire.test.ts @@ -0,0 +1,126 @@ +/** + * Regression: claude-wire format vision targets (MiniMax, Z.AI, Kimi, …) + * reject remote image URLs (MiniMax 403 code 2013). The vision bridge must + * normalize remote URLs to base64 data URIs for these targets. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { + isClaudeWireFormatModel, + ensureBase64ImagesForClaudeWire, +} = await import("../../../src/lib/guardrails/visionBridgeHelpers.ts"); + +test("isClaudeWireFormatModel: true for anthropic and claude-format registry providers", () => { + assert.strictEqual(isClaudeWireFormatModel("anthropic/claude-sonnet-4"), true); + assert.strictEqual(isClaudeWireFormatModel("minimax/MiniMax-M3"), true); + assert.strictEqual(isClaudeWireFormatModel("zai/glm-5"), true); + assert.strictEqual(isClaudeWireFormatModel("claude/claude-opus"), true); + assert.strictEqual(isClaudeWireFormatModel("wafer/wafer-model"), true); +}); + +test("isClaudeWireFormatModel: false for openai-format providers", () => { + assert.strictEqual(isClaudeWireFormatModel("openai/gpt-4o-mini"), false); + assert.strictEqual(isClaudeWireFormatModel("kiro/minimax-m2.5"), false); + assert.strictEqual(isClaudeWireFormatModel("auto/best-vision"), false); + assert.strictEqual(isClaudeWireFormatModel(null), false); +}); + +test("ensureBase64ImagesForClaudeWire: passthrough for non-claude-wire models", async () => { + const body = { + model: "openai/gpt-4o-mini", + messages: [ + { + role: "user", + content: [ + { type: "text", text: "hi" }, + { type: "image_url", image_url: { url: "https://example.com/a.png" } }, + ], + }, + ], + }; + const out = await ensureBase64ImagesForClaudeWire(body, "openai/gpt-4o-mini"); + assert.strictEqual(out, body, "non-claude-wire body must be returned untouched"); +}); + +test("ensureBase64ImagesForClaudeWire: keeps data-URI images as-is", async () => { + const dataUri = "data:image/png;base64,iVBORw0KGgo="; + const body = { + model: "minimax/MiniMax-M3", + messages: [ + { + role: "user", + content: [{ type: "image_url", image_url: { url: dataUri } }], + }, + ], + }; + const out = await ensureBase64ImagesForClaudeWire(body, "minimax/MiniMax-M3"); + const part = out.messages[0].content[0]; + assert.strictEqual(part.image_url.url, dataUri); +}); + +test("ensureBase64ImagesForClaudeWire: resolves remote URLs to base64 for claude-wire targets", async () => { + const pngBase64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg=="; + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => + new Response(new Uint8Array(Buffer.from(pngBase64, "base64")), { + status: 200, + headers: { "content-type": "image/png" }, + }); + + try { + const body = { + model: "minimax/MiniMax-M3", + messages: [ + { + role: "user", + content: [ + { type: "text", text: "What is this?" }, + { type: "image_url", image_url: { url: "https://example.com/cat.png" } }, + ], + }, + ], + }; + const out = await ensureBase64ImagesForClaudeWire( + body, + "minimax/MiniMax-M3", + async () => + new Response(new Uint8Array(Buffer.from(pngBase64, "base64")), { + status: 200, + headers: { "content-type": "image/png" }, + }) + ); + const part = out.messages[0].content[1]; + assert.ok( + part.image_url.url.startsWith("data:image/png;base64,"), + "remote URL must be resolved to a base64 data URI" + ); + assert.ok(part.image_url.url.includes(pngBase64)); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("ensureBase64ImagesForClaudeWire: fail-open when the remote fetch fails", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => { + throw new Error("network down"); + }; + + try { + const body = { + model: "minimax/MiniMax-M3", + messages: [ + { + role: "user", + content: [{ type: "image_url", image_url: { url: "https://example.com/cat.png" } }], + }, + ], + }; + const out = await ensureBase64ImagesForClaudeWire(body, "minimax/MiniMax-M3"); + const part = out.messages[0].content[0]; + assert.strictEqual(part.image_url.url, "https://example.com/cat.png"); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/tests/unit/guardrails/vision-bridge-selfloop-key.test.ts b/tests/unit/guardrails/vision-bridge-selfloop-key.test.ts new file mode 100644 index 0000000000..9d190ab701 --- /dev/null +++ b/tests/unit/guardrails/vision-bridge-selfloop-key.test.ts @@ -0,0 +1,64 @@ +/** + * Regression: the vision-bridge SELF-LOOP must authenticate with a real + * DB-backed API key, not the `sk_omniroute` sentinel. + * + * Root cause on runtime v3.8.49: `callVisionModelSingle` used + * `resolvedApiKey || "sk_omniroute"` for the Authorization header of the + * OmniRoute self-loop request. On instances with REQUIRE_API_KEY enabled the + * runtime rejects `sk_omniroute` with 401 "Missing API key", so EVERY + * vision-bridge describe call failed and image requests were never processed. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { resolveSelfLoopApiKey } = await import( + "../../../src/lib/guardrails/visionBridgeHelpers.ts" +); + +test("uses VISION_BRIDGE_API_KEY when set", async () => { + const previous = process.env.VISION_BRIDGE_API_KEY; + process.env.VISION_BRIDGE_API_KEY = "sk-operator-key"; + try { + const key = await resolveSelfLoopApiKey(async () => "sk-db-key"); + assert.strictEqual(key, "sk-operator-key"); + } finally { + if (previous === undefined) delete process.env.VISION_BRIDGE_API_KEY; + else process.env.VISION_BRIDGE_API_KEY = previous; + } +}); + +test("falls back to the injected resolver (DB key) when no env key is set", async () => { + const previous = process.env.VISION_BRIDGE_API_KEY; + delete process.env.VISION_BRIDGE_API_KEY; + try { + const key = await resolveSelfLoopApiKey(async () => "sk-real-db-key"); + assert.strictEqual(key, "sk-real-db-key"); + } finally { + if (previous === undefined) delete process.env.VISION_BRIDGE_API_KEY; + else process.env.VISION_BRIDGE_API_KEY = previous; + } +}); + +test("never returns the sk_omniroute sentinel when a real key is resolvable", async () => { + const previous = process.env.VISION_BRIDGE_API_KEY; + delete process.env.VISION_BRIDGE_API_KEY; + try { + const key = await resolveSelfLoopApiKey(async () => "sk-db-key"); + assert.notStrictEqual(key, "sk_omniroute"); + } finally { + if (previous === undefined) delete process.env.VISION_BRIDGE_API_KEY; + else process.env.VISION_BRIDGE_API_KEY = previous; + } +}); + +test("falls back to sk_omniroute only when nothing else is available", async () => { + const previous = process.env.VISION_BRIDGE_API_KEY; + delete process.env.VISION_BRIDGE_API_KEY; + try { + const key = await resolveSelfLoopApiKey(async () => ""); + assert.strictEqual(key, "sk_omniroute"); + } finally { + if (previous === undefined) delete process.env.VISION_BRIDGE_API_KEY; + else process.env.VISION_BRIDGE_API_KEY = previous; + } +});