mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-21 22:32:22 +03:00
fix(vision-bridge): nested tool_result images + provider-prefix credential check (#12903)
* fix(vision-bridge): extract/replace images nested inside tool_result content
Claude Code sends tool_result images as {type:"image",source:{base64}}
nested inside a tool_result's content array, not as top-level content
parts. The vision-bridge guardrail's extractImageParts filtered nested
hits out (!p.nested), so these images were silently dropped — a
text-only executor then received a request with no image and returned
HTTP 400.
Port the path-based nested extraction/replace fix:
- MediaPart gains a path field: the key/index chain from
message.content[partIndex] down to the media object itself.
- inspect() tracks the path through recursion; pushPart stamps it.
- extractImageParts drops the !p.nested gate and emits path for nested
hits (extract↔replace contract preserved: same order, every hit
replaceable).
- replaceImageParts rewrites via detectMediaParts: top-level hits swap
their content slot, nested hits walk MediaPart.path via the new
replaceObjectAtPath helper.
- ensureBase64ImagesForClaudeWire skips nested hits (.filter(!p.path))
to keep its sequential index map aligned.
TDD: 7 failing tests (path field, nested extract, nested replace,
document order) → 47/47 pass. typecheck:core clean.
* fix(vision-bridge): resolve provider prefix to node id for credential check
Re-land 932002580 (2026-08-19), which was never merged: it branched off
7acddd91a and fell outside the group-D reimplementation range (4f01fba68
re-picked only cf4dfc868). The same root cause now surfaces on the reroute
path (visionBridgeRerouteTextOnly=true): hasUsableCredentialsForModel
queried provider_connections with the bare node prefix "skhynix" → 0 rows
→ false → getBestVisionModel discarded the configured fixed model and
auto-selected cloudflare-playground/moonshotai/kimi-k2.7-code → Playwright
chromium missing → 502 on every image-bearing request.
- resolveProviderCredentialIds: literal prefix + prefix-index mapped node
id (no-op dedup), composed after #10760's alias→canonical
resolveProviderId.
- getPrefixToNode: 60s-cached getProviderPrefixIndex lookup, fail-open
null.
- hasUsableCredentialsForModel: loop the resolved provider ids and return
true when any has a usable active connection; noauth empty-set
semantics (#10702) preserved.
TDD: resolveProviderCredentialIds 4/4 + skhynix node-id integration test
(RED confirmed: false !== true on the reroute regression). Focused
regression green: visionBridgeCredentials 10/10, vision-bridge reroute/
credentials suite 12/12, vision-bridge policy/mode/cache 18/18,
visionBridgeRouter 16/16. typecheck:core clean.
---------
Co-authored-by: Jihyun Son <jihyun.son@sk.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -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)
|
||||
@@ -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<string, unknown>,
|
||||
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<string, unknown> | 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<string, unknown> | 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<string, unknown>).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<string, unknown>;
|
||||
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<string, unknown> | 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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -84,19 +84,73 @@ function loadProvidersModule(): Promise<typeof import("@/lib/db/providers")> {
|
||||
return providersModulePromise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Credential candidate provider ids for `hasUsableCredentialsForModel`.
|
||||
*
|
||||
* A compatible provider node (openai-compatible-chat-<uuid>) 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<string, string> | 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<string, string> } | null = null;
|
||||
const PREFIX_INDEX_TTL_MS = 60_000;
|
||||
async function getPrefixToNode(): Promise<Map<string, string> | 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-<uuid>` 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<boolean | null> {
|
||||
const rawProvider = typeof model === "string" ? model.split("/")[0]?.trim() : "";
|
||||
@@ -105,17 +159,28 @@ export async function hasUsableCredentialsForModel(model: string): Promise<boole
|
||||
const isNoAuth = isNoAuthProviderKey(rawProvider, provider);
|
||||
try {
|
||||
const { getProviderConnections } = await loadProvidersModule();
|
||||
const connections = await getProviderConnections({ provider, isActive: true });
|
||||
if (!Array.isArray(connections)) return null;
|
||||
// Empty active set: keyed providers are definitively unusable; no-auth
|
||||
// providers still work through the synthetic "noauth" connection.
|
||||
if (connections.length === 0) return isNoAuth;
|
||||
// No-auth rows store no API key (authType "noauth" + empty apiKey would
|
||||
// fail the generic key check) — only a terminal status blocks them.
|
||||
if (isNoAuth) {
|
||||
return !connections.some((c: any) => 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;
|
||||
}
|
||||
|
||||
@@ -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<MediaPart["shape"]> = new Set([
|
||||
"image_url",
|
||||
@@ -200,18 +208,22 @@ const REPLACEABLE_IMAGE_SHAPES: ReadonlySet<MediaPart["shape"]> = 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<RequestBody> {
|
||||
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<string, unknown>;
|
||||
replaceObjectAtPath(container, path, {
|
||||
type: replacementTextType,
|
||||
text: description,
|
||||
});
|
||||
}
|
||||
|
||||
message.content = newContent;
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
function replaceObjectAtPath(
|
||||
container: Record<string, unknown>,
|
||||
path: (string | number)[],
|
||||
replacement: Record<string, unknown>
|
||||
): void {
|
||||
let node: unknown = container;
|
||||
for (let i = 0; i < path.length - 1; i++) {
|
||||
const next = (node as Record<string, unknown> | null | undefined)?.[path[i] as string];
|
||||
if (next == null || typeof next !== "object") return;
|
||||
node = next;
|
||||
}
|
||||
(node as Record<string, unknown>)[path[path.length - 1] as string] = replacement;
|
||||
}
|
||||
|
||||
118
tests/unit/guardrails/visionBridgeCredentialsPrefix.test.ts
Normal file
118
tests/unit/guardrails/visionBridgeCredentialsPrefix.test.ts
Normal file
@@ -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-<uuid>) 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 = "<prefix>" → 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"
|
||||
);
|
||||
});
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user