diff --git a/changelog.d/fixes/12903-vision-bridge-nested-images-prefix-credential.md b/changelog.d/fixes/12903-vision-bridge-nested-images-prefix-credential.md new file mode 100644 index 0000000000..3475ac1394 --- /dev/null +++ b/changelog.d/fixes/12903-vision-bridge-nested-images-prefix-credential.md @@ -0,0 +1 @@ +- fix(guardrails): Vision Bridge now extracts and replaces base64 images nested inside a `tool_result.content` array (the shape Claude Code uses), not just top-level content parts — previously these requests silently reached a vision-incapable provider and returned a 400. Also resolves a provider prefix that has no known alias (e.g. a custom OpenAI-compatible connection's model prefix) to the node id its credentials are actually stored under, instead of discarding the operator's fixed model and falling back to a no-auth candidate that fails (#12903) diff --git a/open-sse/utils/mediaParts.ts b/open-sse/utils/mediaParts.ts index 901b0a97de..0d34dc096f 100644 --- a/open-sse/utils/mediaParts.ts +++ b/open-sse/utils/mediaParts.ts @@ -25,6 +25,13 @@ export interface MediaPart { * replace top-level parts, so they must skip nested hits. */ nested: boolean; + /** + * Path from the top-level content part (`message.content[partIndex]`) to the + * media OBJECT itself. `[]` for top-level hits; for nested hits the keys / + * indexes to walk from the container part down to the media object (e.g. + * `["content", 0]` for an image inside a tool_result's content array). + */ + path: (string | number)[]; /** Original wire shape, for callers that need format-specific handling. */ shape: | "image_url" @@ -72,7 +79,8 @@ function pushPart( kind: MediaKind, ref: string, shape: MediaPart["shape"], - depth: number + depth: number, + path: (string | number)[] ): void { ctx.out.push({ kind, @@ -80,6 +88,7 @@ function pushPart( messageIndex: ctx.messageIndex, partIndex: ctx.partIndex, nested: depth > 0, + path, shape, }); if (ctx.stopAtKind === kind) ctx.found = true; @@ -90,12 +99,20 @@ function inspectImageShapes( obj: Record, type: string | undefined, ctx: DetectCtx, - depth: number + depth: number, + path: (string | number)[] ): boolean { if (type === "image_url" || type === "input_image") { const url = urlFrom(obj.image_url); if (url) { - pushPart(ctx, "image", url, type === "input_image" ? "input_image" : "image_url", depth); + pushPart( + ctx, + "image", + url, + type === "input_image" ? "input_image" : "image_url", + depth, + path + ); return true; } } @@ -103,13 +120,13 @@ function inspectImageShapes( const source = obj.source as Record | undefined; if (source?.type === "base64" && typeof source.data === "string") { const media = typeof source.media_type === "string" ? source.media_type : "image/png"; - pushPart(ctx, "image", `data:${media};base64,${source.data}`, "image_base64", depth); + pushPart(ctx, "image", `data:${media};base64,${source.data}`, "image_base64", depth, path); return true; } // Non-empty url required: an empty `source.url` is not an extractable image // (mirrors the guardrail's historical `if (url)` guard). if (source?.type === "url" && typeof source.url === "string" && source.url) { - pushPart(ctx, "image", source.url, "image_source_url", depth); + pushPart(ctx, "image", source.url, "image_source_url", depth, path); return true; } } @@ -126,26 +143,27 @@ function inspectAudioShapes( type: string | undefined, mediaType: unknown, ctx: DetectCtx, - depth: number + depth: number, + path: (string | number)[] ): boolean { if (type === "input_audio") { const audio = obj.input_audio as Record | undefined; if (typeof audio?.data === "string") { - pushPart(ctx, "audio", audio.data, "input_audio", depth); + pushPart(ctx, "audio", audio.data, "input_audio", depth, path); return true; } } if (type === "audio_url") { const url = urlFrom(obj.audio_url); if (url) { - pushPart(ctx, "audio", url, "audio_url", depth); + pushPart(ctx, "audio", url, "audio_url", depth, path); return true; } } if (typeof mediaType === "string" && mediaType.startsWith("audio/")) { const data = (obj.source as Record).data; if (typeof data === "string") { - pushPart(ctx, "audio", data, "audio_source", depth); + pushPart(ctx, "audio", data, "audio_source", depth, path); return true; } } @@ -158,19 +176,20 @@ function inspectVideoShapes( type: string | undefined, mediaType: unknown, ctx: DetectCtx, - depth: number + depth: number, + path: (string | number)[] ): boolean { if (type === "input_video") { const ref = urlFrom(obj.video_url ?? obj.input_video ?? obj.url); if (ref) { - pushPart(ctx, "video", ref, "input_video", depth); + pushPart(ctx, "video", ref, "input_video", depth, path); return true; } } if (type === "video_url") { const ref = urlFrom(obj.video_url); if (ref) { - pushPart(ctx, "video", ref, "video_url", depth); + pushPart(ctx, "video", ref, "video_url", depth, path); return true; } } @@ -181,13 +200,20 @@ function inspectVideoShapes( // Base64 must carry an explicit video MIME. This prevents a type:video wrapper // from relabelling arbitrary base64 content as MP4. if (videoMediaType && typeof source.data === "string") { - pushPart(ctx, "video", `data:${mediaType};base64,${source.data}`, "video_source", depth); + pushPart( + ctx, + "video", + `data:${mediaType};base64,${source.data}`, + "video_source", + depth, + path + ); return true; } const ref = urlFrom(source.url); const explicitAnthropicUrl = type === "video" && source.type === "url"; if (ref && (explicitAnthropicUrl || type === "video_source" || videoMediaType)) { - pushPart(ctx, "video", ref, "video_source", depth); + pushPart(ctx, "video", ref, "video_source", depth, path); return true; } } @@ -207,7 +233,8 @@ function inspectImageIndicators( type: string | undefined, mediaType: unknown, ctx: DetectCtx, - depth: number + depth: number, + path: (string | number)[] ): boolean { const lowerType = type?.toLowerCase(); const looksLikeImage = @@ -219,20 +246,31 @@ function inspectImageIndicators( const imageMediaType = typeof mediaType === "string" && mediaType.toLowerCase().startsWith("image/"); if (!looksLikeImage && !imageMediaType) return false; - pushPart(ctx, "image", urlFrom(obj.image_url ?? obj.input_image) ?? "", "image_indicator", depth); + pushPart( + ctx, + "image", + urlFrom(obj.image_url ?? obj.input_image) ?? "", + "image_indicator", + depth, + path + ); return true; } -function inspect(value: unknown, ctx: DetectCtx, depth: number): void { +function inspect(value: unknown, ctx: DetectCtx, depth: number, path: (string | number)[]): void { if (ctx.found || depth > MAX_DEPTH || value == null) return; if (typeof value === "string") { - if (value.startsWith("data:image/")) pushPart(ctx, "image", value, "data_uri_string", depth); - if (value.startsWith("data:video/")) pushPart(ctx, "video", value, "data_uri_string", depth); + if (value.startsWith("data:image/")) { + pushPart(ctx, "image", value, "data_uri_string", depth, path); + } + if (value.startsWith("data:video/")) { + pushPart(ctx, "video", value, "data_uri_string", depth, path); + } return; } if (Array.isArray(value)) { - for (const entry of value) { - inspect(entry, ctx, depth + 1); + for (let i = 0; i < value.length; i++) { + inspect(value[i], ctx, depth + 1, [...path, i]); if (ctx.found) return; } return; @@ -241,18 +279,18 @@ function inspect(value: unknown, ctx: DetectCtx, depth: number): void { const obj = value as Record; const type = typeof obj.type === "string" ? obj.type : undefined; - if (inspectImageShapes(obj, type, ctx, depth)) return; + if (inspectImageShapes(obj, type, ctx, depth, path)) return; const mediaType = (obj.source as Record | undefined)?.media_type; // Audio does not early-return: the same object can also carry image // indicators (bare `image_url`/`input_image` keys the legacy combo filter // matched) or nest image parts inside its payload. - inspectAudioShapes(obj, type, mediaType, ctx, depth); + inspectAudioShapes(obj, type, mediaType, ctx, depth, path); if (ctx.found) return; - if (inspectVideoShapes(obj, type, mediaType, ctx, depth)) return; - if (inspectImageIndicators(obj, type, mediaType, ctx, depth)) return; - for (const nested of Object.values(obj)) { - inspect(nested, ctx, depth + 1); + if (inspectVideoShapes(obj, type, mediaType, ctx, depth, path)) return; + if (inspectImageIndicators(obj, type, mediaType, ctx, depth, path)) return; + for (const [key, nested] of Object.entries(obj)) { + inspect(nested, ctx, depth + 1, [...path, key]); if (ctx.found) return; } } @@ -266,7 +304,7 @@ export function detectMediaParts( const content = messages[messageIndex]?.content; if (!Array.isArray(content)) continue; for (let partIndex = 0; partIndex < content.length; partIndex++) { - inspect(content[partIndex], { out, messageIndex, partIndex }, 0); + inspect(content[partIndex], { out, messageIndex, partIndex }, 0, []); } } return out; @@ -289,7 +327,7 @@ export function containsMediaKind( if (!Array.isArray(content)) continue; for (let partIndex = 0; partIndex < content.length; partIndex++) { const ctx: DetectCtx = { out, messageIndex, partIndex, stopAtKind: kind }; - inspect(content[partIndex], ctx, 0); + inspect(content[partIndex], ctx, 0, []); if (ctx.found) return true; } } diff --git a/src/lib/guardrails/visionBridgeCredentials.ts b/src/lib/guardrails/visionBridgeCredentials.ts index 9d25b45219..75d1919aba 100644 --- a/src/lib/guardrails/visionBridgeCredentials.ts +++ b/src/lib/guardrails/visionBridgeCredentials.ts @@ -84,19 +84,73 @@ function loadProvidersModule(): Promise { return providersModulePromise; } +/** + * Credential candidate provider ids for `hasUsableCredentialsForModel`. + * + * A compatible provider node (openai-compatible-chat-) is stored in + * `provider_connections` under its generated node id, while the + * operator-facing model id uses the node's configured public prefix + * (skhynix/HCP-...). Composed AFTER #10760's alias→canonical + * `resolveProviderId` step: the literal prefix alone misses the node row, so + * the prefix-index mapped node id is appended (deduped when the mapping is a + * no-op). Limitation: a node prefix that also collides with a catalog alias + * would already have been canonicalized by `resolveProviderId` and can miss + * here — no such collision exists in practice (reserved prefixes are + * filtered out of the index), noted for future maintainers. + */ +export function resolveProviderCredentialIds( + provider: string, + prefixToNode?: Map | null +): string[] { + const ids = [provider]; + const mapped = prefixToNode?.get(provider); + // Skip when the mapping is a no-op (already the literal provider). + if (mapped && mapped !== provider) ids.push(mapped); + return ids; +} + +// getBestVisionModel()/getFallbackModels() fan out to hasUsableCredentialsForModel +// once per vision-capable catalog entry via Promise.all. The prefix index reads +// the provider_nodes table on every call, so cache it briefly (same TTL order as +// the router's selection cache) to avoid N concurrent table reads per request. +let prefixIndexCache: { at: number; index: Map } | null = null; +const PREFIX_INDEX_TTL_MS = 60_000; +async function getPrefixToNode(): Promise | null> { + try { + const { getProviderPrefixIndex } = await import("@/lib/providerNodePrefixes"); + if (prefixIndexCache && Date.now() - prefixIndexCache.at < PREFIX_INDEX_TTL_MS) { + return prefixIndexCache.index; + } + const index = await getProviderPrefixIndex(); + prefixIndexCache = { at: Date.now(), index: index.prefixToNode }; + return index.prefixToNode; + } catch { + return null; + } +} + /** * Resolve whether `provider/model` has at least one usable active connection. * Returns `null` when the credential store is unavailable (unit tests / early boot). * - * The provider prefix is resolved alias→canonical id before querying - * `provider_connections` (the column stores the id, e.g. "opencode" for the - * "oc" alias — #10702: an alias-keyed query returned zero rows and excluded - * every candidate). No-auth providers (NOAUTH_PROVIDERS) need no stored API - * key: their effective credential is the synthetic "noauth" connection, so - * an empty active set is usable for them (unlike keyed providers). A stored - * row with a terminal status (disabled/banned/expired) still blocks the - * provider; any other row is treated as usable (the key requirement does not - * apply — a noauth row carries no API key by design). + * Two-step resolve before querying `provider_connections`: + * 1. alias→canonical id (#10702: the column stores the id, e.g. "opencode" + * for the "oc" alias — an alias-keyed query returned zero rows); + * 2. public prefix→node id via the provider-prefix index (#re-land 932002580: + * a compatible node's rows are stored under its generated + * `openai-compatible-chat-` id while the operator-facing model id + * uses the node's public prefix, e.g. "skhynix" — a bare-prefix query + * matched zero rows and made a credentialed, active connection report as + * unusable, so the Vision Bridge discarded the configured model). + * Both the literal segment and the mapped node id are queried; any usable + * active connection wins. + * + * No-auth providers (NOAUTH_PROVIDERS) need no stored API key: their effective + * credential is the synthetic "noauth" connection, so an empty active set is + * usable for them (unlike keyed providers). A stored row with a terminal + * status (disabled/banned/expired) still blocks the provider; any other row + * is treated as usable (the key requirement does not apply — a noauth row + * carries no API key by design). */ export async function hasUsableCredentialsForModel(model: string): Promise { const rawProvider = typeof model === "string" ? model.split("/")[0]?.trim() : ""; @@ -105,17 +159,28 @@ export async function hasUsableCredentialsForModel(model: string): Promise hasTerminalConnectionStatus(c)); + const prefixToNode = await getPrefixToNode(); + const providerIds = resolveProviderCredentialIds(provider, prefixToNode); + let sawStoredRow = false; + for (const providerId of providerIds) { + const connections = await getProviderConnections({ provider: providerId, isActive: true }); + if (!Array.isArray(connections)) return null; + // This candidate has no rows — the next candidate (mapped node id) may + // still hold the operator's connection. + if (connections.length === 0) continue; + sawStoredRow = true; + // No-auth rows store no API key (authType "noauth" + empty apiKey would + // fail the generic key check) — only a terminal status blocks them. + const usable = isNoAuth + ? !connections.some((c: any) => hasTerminalConnectionStatus(c)) + : connections.some((c: any) => isProviderConnectionUsable(c)); + if (usable) return true; } - return connections.some((c: any) => isProviderConnectionUsable(c)); + // No candidate has a stored row: keyed providers are definitively + // unusable; no-auth providers still work through the synthetic "noauth" + // connection (and a noauth provider with stored rows can only have been + // rejected via a terminal status in the loop above). + return isNoAuth && !sawStoredRow; } catch { return null; } diff --git a/src/lib/guardrails/visionBridgeHelpers.ts b/src/lib/guardrails/visionBridgeHelpers.ts index dd9b4d45e1..2d6a4fb05e 100644 --- a/src/lib/guardrails/visionBridgeHelpers.ts +++ b/src/lib/guardrails/visionBridgeHelpers.ts @@ -153,6 +153,13 @@ export interface ImagePart { partIndex: number; imageUrl: string; imageType: "image_url" | "image" | "url"; + /** + * For nested hits (image inside a container part, e.g. a tool_result's + * content array) the path from `message.content[partIndex]` to the image + * object itself — what replaceImageParts walks to splice it. Absent for + * top-level parts (plain partIndex splice). + */ + path?: (string | number)[]; } export interface RequestMessage { @@ -183,12 +190,13 @@ export type RequestContentPart = * executor instead of being described. */ /** - * Shapes `replaceImageParts` knows how to splice: top-level content parts - * whose `type` is `image_url`, `image`, or `input_image`. Everything else the - * detector reports (nested hits, `data_uri_string`, `image_indicator`) is - * combo-filter material only — extracting it would desync the positional - * description consumption in visionBridge (descriptions would shift onto the - * wrong images). + * Shapes `replaceImageParts` knows how to splice: content parts whose `type` + * is `image_url`, `image`, or `input_image` — at top level or nested (inside + * a container part such as a tool_result's content array; nested hits are + * spliced by walking `MediaPart.path`). Everything else the detector reports + * (`data_uri_string`, `image_indicator`) is combo-filter material only — + * extracting it would desync the positional description consumption in + * visionBridge (descriptions would shift onto the wrong images). */ const REPLACEABLE_IMAGE_SHAPES: ReadonlySet = new Set([ "image_url", @@ -200,18 +208,22 @@ const REPLACEABLE_IMAGE_SHAPES: ReadonlySet = new Set([ export function extractImageParts(messages: RequestMessage[]): ImagePart[] { // Delegates to the unified detector (open-sse/utils/mediaParts.ts) so the // guardrail and the combo compatibility filter share one source of truth. - // Extraction is ALLOWLISTED to top-level (non-nested) parts whose shape - // replaceImageParts can splice back — the extract↔replace contract: every - // extracted part MUST be replaceable, in the same order, or the positional - // descriptions shift onto the wrong images. + // Extraction is shaped-allowlisted and covers BOTH top-level parts and + // nested hits (image inside a container part, e.g. Claude Code's tool_result + // content array) whose shape replaceImageParts can splice back — the + // extract↔replace contract: every extracted part MUST be replaceable, in the + // same order, or the positional descriptions shift onto the wrong images. + // Nested hits carry `path` so the splice can walk the container; top-level + // hits rely on messageIndex/partIndex alone. return detectMediaParts(messages) - .filter((p) => p.kind === "image" && !p.nested && REPLACEABLE_IMAGE_SHAPES.has(p.shape)) + .filter((p) => p.kind === "image" && REPLACEABLE_IMAGE_SHAPES.has(p.shape)) .map((p) => ({ messageIndex: p.messageIndex, partIndex: p.partIndex, imageUrl: p.ref, imageType: p.shape === "image_base64" ? "image" : p.shape === "image_source_url" ? "url" : "image_url", + ...(p.nested ? { path: p.path } : {}), })); } @@ -239,7 +251,12 @@ export async function ensureBase64ImagesForClaudeWire( fetchImpl: typeof fetch = VISION_BRIDGE_UA_FETCH ): Promise { if (!isClaudeWireFormatModel(model)) return body; - const parts = extractImageParts(body.messages as RequestMessage[]); + // The splice below re-walks top-level content parts and swaps + // image_url/image fields by sequential index. Nested hits now carry + // `path` (extractImageParts emits them); this loop only handles top-level + // image_url/image parts, so skip nested hits to keep the index map aligned + // (a nested hit interleaved with top-level hits would desync the map). + const parts = extractImageParts(body.messages as RequestMessage[]).filter((p) => !p.path); if (parts.length === 0) return body; const resolved = await Promise.all( @@ -899,10 +916,6 @@ export interface RequestBody { [key: string]: unknown; } -/** - * Replace image content parts with text descriptions. - * Concatenates descriptions with labels: "[Image 1]: ..." - */ export function replaceImageParts( body: RequestBody, // #4012: a `null` entry means the describe call failed for that image — keep @@ -926,46 +939,61 @@ export function replaceImageParts( return result; } + // Splice via the unified detector so nested images (image inside a + // tool_result's content array, etc.) are replaced in the SAME order the + // guardrail extracted them (extract↔replace contract). Nested hits carry + // `path`, which the splice walks; top-level hits swap their content slot. + // `input_image` (Responses API) is read through a widened type but MUST be + // replaceable — extractImageParts allowlists it, and every extracted part + // needs a matching splice here. const replacementTextType: "text" | "input_text" = usesResponsesInput ? "input_text" : "text"; + const mediaParts = detectMediaParts(requestMessages).filter( + (p) => p.kind === "image" && REPLACEABLE_IMAGE_SHAPES.has(p.shape) + ); let descriptionIndex = 0; - - for (let msgIdx = 0; msgIdx < requestMessages.length; msgIdx++) { - const message = requestMessages[msgIdx]; - if (!message || !Array.isArray(message.content)) { + for (const part of mediaParts) { + const description = + descriptionIndex < descriptions.length ? descriptions[descriptionIndex++] : null; + if (description == null) { + // #4012: describe failed for this image — preserve the original image + // so a vision-capable upstream can still process it. continue; } - const newContent: RequestContentPart[] = []; + const message = requestMessages[part.messageIndex]; + if (!message || !Array.isArray(message.content)) continue; - for (const part of message.content) { - // `input_image` (Responses API) is read through a widened type: it is - // not part of the historical RequestContentPart union but MUST be - // replaceable — extractImageParts allowlists it, and every extracted - // part needs a matching splice here (extract↔replace contract). - const partType = (part as { type?: string } | null | undefined)?.type; - if (partType === "image_url" || partType === "image" || partType === "input_image") { - if (descriptionIndex < descriptions.length) { - const description = descriptions[descriptionIndex]; - descriptionIndex++; - if (description == null) { - // #4012: describe failed for this image — preserve the original - // image so a vision-capable upstream can still process it. - newContent.push(part as RequestContentPart); - } else { - newContent.push({ - type: replacementTextType, - text: description, - } as RequestContentPart); - } - } - } else { - newContent.push(part as RequestContentPart); - } + const path = part.path; + if (!path || path.length === 0) { + // Top-level part: swap the content slot itself with a text part. + (message.content as unknown[])[part.partIndex] = { + type: replacementTextType, + text: description, + }; + } else { + // Nested hit: walk the container part to the media object and splice it. + const container = message.content[part.partIndex] as Record; + replaceObjectAtPath(container, path, { + type: replacementTextType, + text: description, + }); } - - message.content = newContent; } return result; } + +function replaceObjectAtPath( + container: Record, + path: (string | number)[], + replacement: Record +): void { + let node: unknown = container; + for (let i = 0; i < path.length - 1; i++) { + const next = (node as Record | null | undefined)?.[path[i] as string]; + if (next == null || typeof next !== "object") return; + node = next; + } + (node as Record)[path[path.length - 1] as string] = replacement; +} diff --git a/tests/unit/guardrails/visionBridgeCredentialsPrefix.test.ts b/tests/unit/guardrails/visionBridgeCredentialsPrefix.test.ts new file mode 100644 index 0000000000..6e7bd6a33a --- /dev/null +++ b/tests/unit/guardrails/visionBridgeCredentialsPrefix.test.ts @@ -0,0 +1,118 @@ +/** + * Vision Bridge credential checks — provider-prefix → node-id resolution. + * + * Re-land of the lost fix 932002580 (2026-08-19, never merged). A compatible + * provider node (openai-compatible-chat-) is stored in + * `provider_connections` under its generated node id, while the + * operator-facing model id uses the node's configured public prefix + * (e.g. `skhynix/HCP-Vision-Latest`). `hasUsableCredentialsForModel` queried + * the bare prefix → 0 rows → false → `getBestVisionModel` discarded the + * configured fixed model and auto-selected a noauth candidate + * (cloudflare-playground/moonshotai/kimi-k2.7-code) → whole-request reroute + * to an unreachable executor → 502 on every image-bearing request. + * + * This suite lives in its OWN file/process on purpose: the fix caches the + * prefix index for 60s, and node:test runs each file in its own process — + * seeding must happen before the first index read in this process, so the + * integration test must not share a process with tests that warm the cache + * against an empty node table. + */ +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-vb-cred-prefix-")); + +// Set before any db import so getDbInstance() picks the temp dir. +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../../src/lib/db/core.ts"); +const providersDb = await import("../../../src/lib/db/providers.ts"); +const { hasUsableCredentialsForModel, resolveProviderCredentialIds } = + await import("../../../src/lib/guardrails/visionBridgeCredentials.ts"); + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +// ── resolveProviderCredentialIds (pure prefix→node id resolution) ─────────── + +test("resolveProviderCredentialIds returns the literal provider when no prefix mapping exists", () => { + assert.deepEqual(resolveProviderCredentialIds("openai", new Map()), ["openai"]); +}); + +test("resolveProviderCredentialIds appends the mapped node id for a known prefix", () => { + const prefixToNode = new Map([["skhynix", "openai-compatible-chat-abc-123"]]); + assert.deepEqual(resolveProviderCredentialIds("skhynix", prefixToNode), [ + "skhynix", + "openai-compatible-chat-abc-123", + ]); +}); + +test("resolveProviderCredentialIds dedupes when the mapping targets the literal provider", () => { + const prefixToNode = new Map([["openai", "openai"]]); + assert.deepEqual(resolveProviderCredentialIds("openai", prefixToNode), ["openai"]); +}); + +test("resolveProviderCredentialIds tolerates undefined prefix index", () => { + assert.deepEqual(resolveProviderCredentialIds("skhynix", undefined), ["skhynix"]); +}); + +// ── integration: node-id-stored connection found through the prefix (#re-land) ── + +test("prefix-keyed model finds a row stored under the compatible node id (skhynix reroute regression)", async () => { + await resetStorage(); + const uniq = `itest-${process.pid}-${Date.now()}`; + const nodeId = `openai-compatible-chat-${uniq}`; + const prefix = `skhynix-${uniq}`; + await providersDb.createProviderNode({ + id: nodeId, + type: "openai-compatible", + name: "SK hynix (test)", + prefix, + apiType: "chat", + baseUrl: "http://localhost:1/v1", + }); + + // Phase 1 (negative): only a banned connection under the node id — the + // prefix must map to the node and the terminal row must still block it. + await providersDb.createProviderConnection({ + provider: nodeId, + authType: "apikey", + apiKey: "sk-dead-key", + isActive: true, + testStatus: "banned", + }); + assert.equal( + await hasUsableCredentialsForModel(`${prefix}/HCP-Vision-Latest`), + false, + "a banned connection under the mapped node id must not count as usable" + ); + + // Phase 2 (positive): an active keyed connection under the node id must be + // found through the prefix mapping. Before the fix this queried + // provider = "" → 0 rows → false, so the Vision Bridge discarded + // the operator-configured model and auto-selected another provider. + await providersDb.createProviderConnection({ + provider: nodeId, + authType: "apikey", + apiKey: "sk-hynix-key", + isActive: true, + testStatus: "active", + }); + const usable = await hasUsableCredentialsForModel(`${prefix}/HCP-Vision-Latest`); + assert.equal( + usable, + true, + "prefix-keyed model must find the connection stored under the node id" + ); +}); diff --git a/tests/unit/guardrails/visionBridgeHelpers.extractImageParts.test.ts b/tests/unit/guardrails/visionBridgeHelpers.extractImageParts.test.ts index 70075efe91..baedfebea4 100644 --- a/tests/unit/guardrails/visionBridgeHelpers.extractImageParts.test.ts +++ b/tests/unit/guardrails/visionBridgeHelpers.extractImageParts.test.ts @@ -205,3 +205,59 @@ test("extractImageParts supports both base64 and url source blocks in one messag assert.strictEqual(result[1].imageType, "url"); assert.strictEqual(result[1].imageUrl, "https://example.com/B.png"); }); + +test("extractImageParts extracts a base64 image nested inside a tool_result content array", () => { + // Claude Code sends tool-result images as {type:"image", source:{base64}} + // inside the tool_result part's OWN content array (nested, not top-level). + const messages = [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_01", + content: [ + { + type: "image", + source: { type: "base64", media_type: "image/png", data: "AAA=" }, + }, + ], + }, + { type: "text", text: "[Image description]" }, + ], + }, + ] as unknown as RequestMessage[]; + + const result = extractImageParts(messages); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].messageIndex, 0); + assert.strictEqual(result[0].partIndex, 0); + assert.strictEqual(result[0].imageUrl, "data:image/png;base64,AAA="); + assert.strictEqual(result[0].imageType, "image"); + assert.deepStrictEqual(result[0].path, ["content", 0]); +}); + +test("extractImageParts keeps document order when nested and top-level images mix", () => { + const messages = [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_02", + content: [{ type: "image_url", image_url: { url: "https://example.com/in.png" } }], + }, + { type: "image_url", image_url: { url: "https://example.com/top.png" } }, + ], + }, + ] as unknown as RequestMessage[]; + + const result = extractImageParts(messages); + assert.strictEqual(result.length, 2); + // Nested hit first, with a path pointing into the tool_result content. + assert.strictEqual(result[0].imageUrl, "https://example.com/in.png"); + assert.deepStrictEqual(result[0].path, ["content", 0]); + // Top-level hit second, no path (plain partIndex splice). + assert.strictEqual(result[1].imageUrl, "https://example.com/top.png"); + assert.strictEqual(result[1].path, undefined); +}); diff --git a/tests/unit/guardrails/visionBridgeHelpers.replaceImageParts.test.ts b/tests/unit/guardrails/visionBridgeHelpers.replaceImageParts.test.ts index b572fb36a4..ed39c96d43 100644 --- a/tests/unit/guardrails/visionBridgeHelpers.replaceImageParts.test.ts +++ b/tests/unit/guardrails/visionBridgeHelpers.replaceImageParts.test.ts @@ -251,3 +251,100 @@ test("replaceImageParts handles mixed images and text", () => { assert.strictEqual(content[2].type, "text"); assert.strictEqual(content[2].text, "[Image 2]: Second image"); }); + +test("replaceImageParts replaces an image nested inside tool_result content", () => { + const body = { + model: "deepseek-ai/DeepSeek-V4-Flash-0731", + messages: [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "toolu_01", + content: [ + { + type: "image", + source: { type: "base64", media_type: "image/png", data: "AAA=" }, + }, + ], + }, + { type: "text", text: "[Image description]" }, + ], + }, + ], + }; + + const result = replaceImageParts(body, ["[Image 1]: A screenshot of the OmniRoute dashboard"]); + + const toolResult = result.messages[0].content[0] as { + type: string; + content: Array<{ type: string; text?: string }>; + }; + assert.strictEqual(toolResult.type, "tool_result"); + assert.deepStrictEqual(toolResult.content, [ + { type: "text", text: "[Image 1]: A screenshot of the OmniRoute dashboard" }, + ]); + // Sibling text preserved. + assert.strictEqual(result.messages[0].content[1].type, "text"); + assert.strictEqual(result.messages[0].content[1].text, "[Image description]"); +}); + +test("replaceImageParts consumes descriptions in document order for nested and top-level images", () => { + const body = { + model: "deepseek-ai/DeepSeek-V4-Flash-0731", + messages: [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "a", + content: [{ type: "image_url", image_url: { url: "https://x/a.png" } }], + }, + { type: "image_url", image_url: { url: "https://x/b.png" } }, + { + type: "tool_result", + tool_use_id: "c", + content: [{ type: "image_url", image_url: { url: "https://x/c.png" } }], + }, + ], + }, + ], + }; + + const result = replaceImageParts(body, ["DA", "DB", "DC"]); + const content = result.messages[0].content as Array<{ + type: string; + content?: Array<{ type: string; text?: string }>; + text?: string; + }>; + // part[0] tool_result → nested a → DA + assert.strictEqual(content[0].content?.[0].text, "DA"); + // part[1] top-level b → DB + assert.strictEqual(content[1].text, "DB"); + // part[2] tool_result → nested c → DC + assert.strictEqual(content[2].content?.[0].text, "DC"); +}); + +test("replaceImageParts keeps a nested image when its description is null (#4012)", () => { + const body = { + model: "deepseek-ai/DeepSeek-V4-Flash-0731", + messages: [ + { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "a", + content: [{ type: "image_url", image_url: { url: "https://x/a.png" } }], + }, + ], + }, + ], + }; + + const result = replaceImageParts(body, [null]); + const toolResult = result.messages[0].content[0] as { content: Array<{ type: string }> }; + assert.strictEqual(toolResult.content[0].type, "image_url"); +}); diff --git a/tests/unit/media-parts.test.ts b/tests/unit/media-parts.test.ts index 1facb94fc6..54bfb003e6 100644 --- a/tests/unit/media-parts.test.ts +++ b/tests/unit/media-parts.test.ts @@ -197,13 +197,36 @@ test("extractImageParts does not extract a data URI embedded in a text part", () assert.deepEqual(parts, []); }); -test("extractImageParts skips nested and indicator-only detections", () => { +test("detectMediaParts reports a path for nested images into the container", () => { + const parts = detectMediaParts( + msg([ + { + type: "tool_result", + tool_use_id: "t", + content: [ + { type: "image", source: { type: "base64", media_type: "image/png", data: "AAA" } }, + { type: "text", text: "note" }, + ], + }, + ]) + ); + assert.equal(parts.length, 1); + const p = parts[0]; + assert.equal(p.nested, true); + assert.equal(p.shape, "image_base64"); + assert.deepEqual(p.path, ["content", 0]); + assert.equal(p.messageIndex, 0); + assert.equal(p.partIndex, 0); +}); + +test("extractImageParts extracts nested images with replaceable shapes and skips indicators", () => { const parts = extractImageParts([ { role: "user", content: [ - // Image nested inside an audio payload: detector reports it (combo needs - // it) but the replacer cannot splice it — must not be extracted. + // Image nested inside an audio payload's cover art: the detector + // reports it AND the replacer can now splice it via `path` — it must + // be extracted and carried into replaceImageParts in order. { type: "input_audio", input_audio: { @@ -216,5 +239,7 @@ test("extractImageParts skips nested and indicator-only detections", () => { ], } as never, ]); - assert.deepEqual(parts, []); + assert.equal(parts.length, 1); + assert.equal(parts[0].imageUrl, "https://x/c.png"); + assert.deepEqual(parts[0].path, ["input_audio", "cover"]); });