fix(coding-agent): normalize WebP on provider path

2026-08-11 23:07:21 -04:00
parent 2cd25fb4ee
commit 001460ee8a
6 changed files with 283 additions and 47 deletions
@@ -2,6 +2,10 @@

 ## [Unreleased]

+### Fixed
+
+- Fixed custom STB-backed vision providers retaining decoder metadata and sending WebP bytes that llama.cpp cannot decode. Image format is now detected from bytes, attached and historical WebP blocks are normalized on the actual provider-request path, and corrupt WebP is omitted instead of making the turn unrecoverable.
+
 ## [17.2.15] - 2026-08-12

 ### Added
@@ -25,9 +29,6 @@
 - Fixed `/handoff` losing local artifacts (plans, scratch files, research notes) by copying them across the handoff session boundary.
 - Replaced libarchive-based tar parsing with a hardened, in-process tar reader to prevent crashes and safely handle complex archive structures, symlinks, and sparse metadata.
 - Fixed `Ctrl+O` tool-output expansion failing to reach launch-completion messages wrapped in the hidden tool activity container.
-### Fixed
-
- Fixed custom STB-backed vision providers retaining decoder metadata and re-encoding resumed WebP images before provider requests.

 ## [17.2.14] - 2026-08-11

@@ -221,6 +221,7 @@ import { USER_TODO_EDIT_CUSTOM_TYPE } from "./tools/todo";
 import { ttsTool } from "./tools/tts";
 import { resolveActiveRepoContext } from "./utils/active-repo-context";
 import { EventBus } from "./utils/event-bus";
+import { normalizeProviderContextImagesForModel } from "./utils/image-loading";
 import { buildNamedToolChoice } from "./utils/tool-choice";
 import { VibeSessionRegistry } from "./vibe/runtime";
 import { buildWorkspaceTree, type WorkspaceTree } from "./workspace-tree";
@@ -3108,8 +3109,9 @@ async function createAgentSessionScoped(options: CreateAgentSessionOptions): Pro
 			return wrapSteeringForModel(withContext);
 		};
 		// Per-request provider-context transforms. Obfuscate FIRST so secrets are
-		// redacted from text before snapcompact rasterizes it into PNG frames, then
-		// clamp images to the active provider budget before the request is sent.
+		// redacted from text before snapcompact rasterizes it into PNG frames. Clamp
+		// to the provider budget before normalizing decoder-incompatible images so
+		// dropped historical images never pay a transcode cost.
 		const snapcompactSystemPromptMode = settings.get("snapcompact.systemPrompt");
 		const snapcompactInline =
 			snapcompactSystemPromptMode !== "none" || settings.get("snapcompact.toolResults")
@@ -3127,7 +3129,8 @@ async function createAgentSessionScoped(options: CreateAgentSessionOptions): Pro
 		const transformProviderContext = async (context: Context, transformModel: Model): Promise<Context> => {
 			let transformed = obfuscator ? obfuscateProviderContext(obfuscator, context) : context;
 			if (snapcompactInline) transformed = await snapcompactInline.transform(transformed, transformModel);
-			return clampProviderContextImages(transformed, transformModel);
+			transformed = clampProviderContextImages(transformed, transformModel);
+			return await normalizeProviderContextImagesForModel(transformed, transformModel);
 		};
 		const onPayload = async (payload: unknown, model?: Model) => {
 			return await extensionRunner.emitBeforeProviderRequest(payload, model);
@@ -3765,8 +3768,9 @@ async function createAgentSessionScoped(options: CreateAgentSessionOptions): Pro
 					convertToLlm: convertToLlmFinal,
 					transformContext: async messages => wrapSteeringForModel(messages),
 					transformProviderContext: async (context, transformModel) => {
-						const transformed = obfuscator ? obfuscateProviderContext(obfuscator, context) : context;
-						return clampProviderContextImages(transformed, transformModel);
+						let transformed = obfuscator ? obfuscateProviderContext(obfuscator, context) : context;
+						transformed = clampProviderContextImages(transformed, transformModel);
+						return await normalizeProviderContextImagesForModel(transformed, transformModel);
 					},
 					thinkingBudgets: agent.thinkingBudgets,
 					temperature: agent.temperature,
@@ -13,7 +13,7 @@ import type { LocalProtocolOptions } from "../internal-urls";
 import { deobfuscateSessionContext, obfuscateMessages } from "../secrets/message-transform";
 import type { SecretObfuscator } from "../secrets/obfuscator";
 import { stripPendingSecretPlaceholderSuffix } from "../secrets/placeholder";
-import { normalizeModelContextImages, normalizeModelContextMessages } from "../utils/image-loading";
+import { normalizeModelContextImages } from "../utils/image-loading";
 import { describeAttachedImagesForTextModel } from "../utils/image-vision-fallback";
 import { type CustomMessage, convertToLlm } from "./messages";
 import { IMAGE_ATTACHMENT_DESCRIPTION_TYPE } from "./queued-messages";
@@ -126,8 +126,7 @@ export class SessionProviderBoundary {
 	/** Converts session messages using the configured pre-LLM pipeline. */
 	async convertMessagesToLlm(messages: AgentMessage[], signal?: AbortSignal): Promise<Message[]> {
 		const transformedMessages = await this.#host.transformContext(messages, signal);
-		const converted = await this.#host.convertToLlm(transformedMessages);
-		return await normalizeModelContextMessages(converted, this.#host.model());
+		return await this.#host.convertToLlm(transformedMessages);
 	}

 	/** Applies session-level stream hooks and provider defaults to a side request. */
@@ -1,17 +1,25 @@
 import * as fs from "node:fs/promises";
-import type { ImageContent, Message, Model } from "@oh-my-pi/pi-ai";
-import { formatBytes, readImageMetadata, SUPPORTED_IMAGE_MIME_TYPES } from "@oh-my-pi/pi-utils";
+import type { Context, ImageContent, Message, Model, TextContent } from "@oh-my-pi/pi-ai";
+import { formatBytes, logger, readImageMetadata, SUPPORTED_IMAGE_MIME_TYPES } from "@oh-my-pi/pi-utils";
 import { LRUCache } from "@oh-my-pi/pi-utils/lru";
 import { resolveReadPath } from "../tools/path-utils";
 import { formatDimensionNote, type ImageResizeOptions, resizeImage } from "./image-resize";

 export const MAX_IMAGE_INPUT_BYTES = 20 * 1024 * 1024;
 export const SUPPORTED_INPUT_IMAGE_MIME_TYPES = SUPPORTED_IMAGE_MIME_TYPES;
-const MODEL_BOUNDARY_IMAGE_CACHE_LIMIT = 128;
+const MODEL_BOUNDARY_IMAGE_CACHE_MAX_SIZE = 64 * 1024 * 1024;
+const MODEL_BOUNDARY_IMAGE_CACHE_MAX_ENTRIES = 128;
 type NormalizedImagePayload = Pick<ImageContent, "data" | "mimeType">;
-const modelBoundaryImageCache = new LRUCache<string, Promise<NormalizedImagePayload>>({
-	max: MODEL_BOUNDARY_IMAGE_CACHE_LIMIT,
+const modelBoundaryImageCache = new LRUCache<string, NormalizedImagePayload | null>({
+	max: MODEL_BOUNDARY_IMAGE_CACHE_MAX_ENTRIES,
+	maxSize: MODEL_BOUNDARY_IMAGE_CACHE_MAX_SIZE,
+	sizeCalculation: payload => Math.max(1, payload?.data.length ?? 1),
 });
+const modelBoundaryImageNormalizations = new Map<string, Promise<NormalizedImagePayload | null>>();
+const UNDECODABLE_STB_IMAGE_OMISSION: TextContent = {
+	type: "text",
+	text: "[image omitted: WebP could not be decoded for this model]",
+};

 function hasWebPMagic(data: string): boolean {
 	const header = Buffer.from(data.slice(0, 16), "base64");
@@ -24,26 +32,47 @@ function isWebPImage(image: ImageContent): boolean {
 	return image.mimeType.toLowerCase() === "image/webp" || hasWebPMagic(image.data);
 }

-function modelBoundaryImageCacheKey(image: ImageContent): string {
-	return `${image.data.length}:${image.data.slice(0, 32)}:${image.data.slice(-32)}:${String(Bun.hash(image.data))}`;
+function modelBoundaryImageCacheKey(image: ImageContent, resize: ImageResizeOptions | undefined): string {
+	const resizeKey = JSON.stringify([
+		resize?.maxWidth,
+		resize?.maxHeight,
+		resize?.minDimension,
+		resize?.maxBytes,
+		resize?.jpegQuality,
+	]);
+	return `${resizeKey}:${image.mimeType}:${image.data.length}:${image.data.slice(0, 32)}:${image.data.slice(-32)}:${String(Bun.hash(image.data))}`;
 }

-async function memoizedStbImageNormalization(image: ImageContent): Promise<ImageContent> {
-	const key = modelBoundaryImageCacheKey(image);
-	let pending = modelBoundaryImageCache.get(key);
+async function memoizedStbImageNormalization(
+	image: ImageContent,
+	resize: ImageResizeOptions | undefined,
+): Promise<ImageContent | null> {
+	const key = modelBoundaryImageCacheKey(image, resize);
+	const cached = modelBoundaryImageCache.get(key);
+	if (cached !== undefined) return cached ? { ...image, ...cached } : null;
+
+	let pending = modelBoundaryImageNormalizations.get(key);
 	if (!pending) {
-		pending = resizeImage(image, { excludeWebP: true }).then(resized => {
-			if (resized.mimeType === "image/webp" || hasWebPMagic(resized.data)) {
-				throw new Error("Image normalization retained WebP for an STB-backed model");
-			}
-			return { data: resized.data, mimeType: resized.mimeType };
-		});
-		modelBoundaryImageCache.set(key, pending);
-		void pending.catch(() => {
-			if (modelBoundaryImageCache.peek(key) === pending) modelBoundaryImageCache.delete(key);
-		});
+		pending = resizeImage(image, { ...resize, excludeWebP: true })
+			.then(resized => {
+				if (resized.mimeType === "image/webp" || hasWebPMagic(resized.data)) {
+					throw new Error("Image normalization retained WebP for an STB-backed model");
+				}
+				return { data: resized.data, mimeType: resized.mimeType };
+			})
+			.catch(error => {
+				logger.warn("Dropping undecodable WebP for an STB-backed model", { error: String(error) });
+				return null;
+			})
+			.then(payload => {
+				modelBoundaryImageCache.set(key, payload);
+				return payload;
+			})
+			.finally(() => modelBoundaryImageNormalizations.delete(key));
+		modelBoundaryImageNormalizations.set(key, pending);
 	}
-	return { ...image, ...(await pending) };
+	const normalized = await pending;
+	return normalized ? { ...image, ...normalized } : null;
 }

 /**
@@ -163,15 +192,13 @@ export async function normalizeModelContextImages(
 	for (const image of images) {
 		try {
 			if (excludesWebP && isWebPImage(image)) {
-				normalized.push(await memoizedStbImageNormalization(image));
+				const converted = await memoizedStbImageNormalization(image, options?.resize);
+				if (converted) normalized.push(converted);
 				continue;
 			}
 			const resized = await resizeImage(image, resize);
 			normalized.push({ ...image, data: resized.data, mimeType: resized.mimeType });
-		} catch (error) {
-			if (excludesWebP && isWebPImage(image)) {
-				throw new Error("Failed to convert WebP image for an STB-backed model", { cause: error });
-			}
+		} catch {
 			// Preserve existing caller behavior for decode/resize failures: keep the
 			// user's image block rather than dropping it from the turn.
 			normalized.push(image);
@@ -191,23 +218,30 @@ export async function normalizeModelContextMessages(messages: Message[], model:
 	for (let messageIndex = 0; messageIndex < messages.length; messageIndex++) {
 		const message = messages[messageIndex]!;
 		if (typeof message.content === "string") continue;
-		const webpImages: ImageContent[] = [];
-		for (const part of message.content) {
-			if (part.type === "image" && isWebPImage(part)) webpImages.push(part);
+		let content: Array<(typeof message.content)[number]> | undefined;
+		for (let partIndex = 0; partIndex < message.content.length; partIndex++) {
+			const part = message.content[partIndex]!;
+			if (part.type !== "image" || !isWebPImage(part)) {
+				content?.push(part);
+				continue;
+			}
+			content ??= message.content.slice(0, partIndex);
+			const normalized = await memoizedStbImageNormalization(part, undefined);
+			content.push(normalized ?? UNDECODABLE_STB_IMAGE_OMISSION);
 		}
-		if (webpImages.length === 0) continue;
-		const normalized = await normalizeModelContextImages(webpImages, { model });
-		if (!normalized) continue;
-		let imageIndex = 0;
-		const content = message.content.map(part =>
-			part.type === "image" && isWebPImage(part) ? normalized[imageIndex++]! : part,
-		);
+		if (!content) continue;
 		output ??= messages.slice();
 		output[messageIndex] = { ...message, content } as Message;
 	}
 	return output ?? messages;
 }

+/** Normalizes historical image blocks in an ephemeral provider request. */
+export async function normalizeProviderContextImagesForModel(context: Context, model: Model): Promise<Context> {
+	const messages = await normalizeModelContextMessages(context.messages, model);
+	return messages === context.messages ? context : { ...context, messages };
+}
+
 export async function loadImageInput(options: LoadImageInputOptions): Promise<LoadedImageInput | null> {
 	const maxBytes = options.maxBytes ?? MAX_IMAGE_INPUT_BYTES;
 	const resolvedPath = options.resolvedPath ?? resolveReadPath(options.path, options.cwd);
@@ -181,6 +181,143 @@ describe("AgentSession message pipeline", () => {
 		expect(session.getImageAttachments()).toEqual([{ label: "Image #1", uri: "attachment://1", image: userImage }]);
 	});

+	it("normalizes historical WebP on the main provider request path", async () => {
+		using tempDir = TempDir.createSync("@pi-stb-main-path-");
+		const api = "test-stb-main-path";
+		const contexts: Context[] = [];
+		registerCustomApi(api, (_model, context) => {
+			contexts.push(context);
+			const stream = new AssistantMessageEventStream();
+			queueMicrotask(() => {
+				const message = createAssistantMessage("ok");
+				stream.push({ type: "text_delta", contentIndex: 0, delta: "ok", partial: message });
+				stream.push({ type: "done", reason: "stop", message });
+			});
+			return stream;
+		});
+		const seed = Buffer.from(
+			"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
+			"base64",
+		);
+		const webpData = Buffer.from(await new Bun.Image(seed).resize(200, 200).webp({ quality: 90 }).bytes()).toBase64();
+		const historicalImage: ImageContent = {
+			type: "image",
+			data: webpData,
+			// Confirm byte sniffing catches persisted blocks with stale metadata.
+			mimeType: "image/png",
+		};
+		const model = buildModel({
+			id: "stb-main-path",
+			name: "STB main path",
+			api,
+			provider: "managed-primary",
+			baseUrl: "http://127.0.0.1:8080/v1",
+			reasoning: false,
+			input: ["text", "image"],
+			imageInputDecoder: "stb",
+			cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
+			contextWindow: 4096,
+			maxTokens: 1024,
+		} as ModelSpec<Api>) as Model<Api>;
+		const authStorage = await AuthStorage.create(tempDir.join("auth.db"));
+		authStorage.setRuntimeApiKey(model.provider, "test-key");
+		const modelRegistry = new ModelRegistry(authStorage, tempDir.join("models.yml"));
+		const { session } = await createAgentSession({
+			cwd: tempDir.path(),
+			agentDir: tempDir.path(),
+			sessionManager: SessionManager.inMemory(tempDir.path()),
+			authStorage,
+			modelRegistry,
+			settings: Settings.isolated({ "compaction.enabled": false }),
+			model,
+			disableExtensionDiscovery: true,
+			skills: [],
+			contextFiles: [],
+			promptTemplates: [],
+			slashCommands: [],
+			enableMCP: false,
+			enableLsp: false,
+			skipPythonPreflight: true,
+			taskDepth: 1,
+			agentId: "SubAgent",
+		});
+		try {
+			session.agent.appendMessage({
+				role: "toolResult",
+				toolCallId: "read-1",
+				toolName: "read",
+				content: [{ type: "text", text: "screenshot" }, historicalImage],
+				isError: false,
+				timestamp: 1,
+			});
+
+			await session.sendUserMessage("continue");
+
+			expect(contexts).toHaveLength(1);
+			const outboundImages: ImageContent[] = [];
+			for (const message of contexts[0]!.messages) {
+				if (typeof message.content === "string") continue;
+				for (const part of message.content) {
+					if (part.type === "image") outboundImages.push(part);
+				}
+			}
+			expect(outboundImages).toHaveLength(1);
+			expect(outboundImages[0]!.mimeType).not.toBe("image/webp");
+			expect(Buffer.from(outboundImages[0]!.data.slice(0, 16), "base64").toString("ascii", 8, 12)).not.toBe("WEBP");
+			expect(historicalImage.mimeType).toBe("image/png");
+			expect(Buffer.from(historicalImage.data.slice(0, 16), "base64").toString("ascii", 8, 12)).toBe("WEBP");
+		} finally {
+			await session.dispose();
+			authStorage.close();
+		}
+	});
+
+	it("continues a user turn when an attached WebP is undecodable by an STB model", async () => {
+		const api = "test-stb-corrupt-attachment";
+		const contexts: Context[] = [];
+		registerCustomApi(api, (_model, context) => {
+			contexts.push(context);
+			const stream = new AssistantMessageEventStream();
+			queueMicrotask(() => {
+				const message = createAssistantMessage("ok");
+				stream.push({ type: "text_delta", contentIndex: 0, delta: "ok", partial: message });
+				stream.push({ type: "done", reason: "stop", message });
+			});
+			return stream;
+		});
+		const model = buildModel({
+			id: "stb-corrupt-attachment",
+			name: "STB corrupt attachment",
+			api,
+			provider: "managed-primary",
+			baseUrl: "http://127.0.0.1:8080/v1",
+			reasoning: false,
+			input: ["text", "image"],
+			imageInputDecoder: "stb",
+			cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
+			contextWindow: 4096,
+			maxTokens: 1024,
+		} as ModelSpec<Api>) as Model<Api>;
+		const session = new AgentSession({
+			agent: new Agent({ initialState: { model, systemPrompt: ["system"], messages: [], tools: [] } }),
+			sessionManager: SessionManager.inMemory(),
+			settings: Settings.isolated({ "compaction.enabled": false }),
+			modelRegistry: createModelRegistryStub() as never,
+		});
+		sessions.push(session);
+		const corrupt: ImageContent = {
+			type: "image",
+			data: Buffer.from("RIFF0000WEBPbroken-attachment").toBase64(),
+			mimeType: "image/webp",
+		};
+
+		await session.sendUserMessage([{ type: "text", text: "inspect this" }, corrupt]);
+
+		expect(contexts).toHaveLength(1);
+		const userMessage = contexts[0]!.messages.find(message => message.role === "user");
+		expect(userMessage?.content).toEqual([{ type: "text", text: "inspect this" }]);
+	});
+
 	it("keeps stored steering text raw while pre-LLM conversion wraps it", async () => {
 		const session = new AgentSession({
 			agent: createAgent(),
@@ -147,6 +147,39 @@ describe("normalizeModelContextImages model-aware WebP exclusion", () => {
 		expect(result?.[0]?.mimeType).toBe("image/webp");
 	});

+	test("honors resize options when caching STB image normalization", async () => {
+		const webp = { type: "image" as const, data: await makeRedWebP(400, 400), mimeType: "image/webp" };
+		const model = buildStbVisionModel("managed-primary");
+
+		const first = await normalizeModelContextImages([webp], {
+			model,
+			resize: { maxWidth: 120, maxHeight: 120, minDimension: 1 },
+		});
+		const second = await normalizeModelContextImages([webp], {
+			model,
+			resize: { maxWidth: 60, maxHeight: 60, minDimension: 1 },
+		});
+		const firstMetadata = await new Bun.Image(Buffer.from(first![0]!.data, "base64")).metadata();
+		const secondMetadata = await new Bun.Image(Buffer.from(second![0]!.data, "base64")).metadata();
+
+		expect(firstMetadata.width).toBe(120);
+		expect(secondMetadata.width).toBe(60);
+	});
+
+	test("drops an undecodable WebP attachment instead of rejecting the turn", async () => {
+		const corrupt = {
+			type: "image" as const,
+			data: Buffer.from("RIFF0000WEBPcorrupt").toBase64(),
+			mimeType: "image/webp",
+		};
+
+		const result = await normalizeModelContextImages([corrupt], {
+			model: buildStbVisionModel("managed-primary"),
+		});
+
+		expect(result).toEqual([]);
+	});
+
 	test("rewrites resumed tool-result WebP blocks at the STB provider boundary", async () => {
 		const original = {
 			type: "image" as const,
@@ -181,4 +214,32 @@ describe("normalizeModelContextImages model-aware WebP exclusion", () => {
 		// Provider-boundary normalization is ephemeral; persisted history is not mutated.
 		expect(messages[0]!.content[1]).toBe(original);
 	});
+
+	test("replaces an undecodable historical WebP with an omission note", async () => {
+		const corrupt = {
+			type: "image" as const,
+			data: Buffer.from("RIFF0000WEBPbad-history").toBase64(),
+			mimeType: "image/webp",
+		};
+		const messages = [
+			{
+				role: "toolResult" as const,
+				toolCallId: "read-corrupt",
+				toolName: "read",
+				content: [corrupt],
+				isError: false,
+				timestamp: 1,
+			},
+		];
+
+		const result = await normalizeModelContextMessages(messages, buildStbVisionModel("managed-primary"));
+		const resultMessage = result[0]!;
+		expect(resultMessage.role).toBe("toolResult");
+		if (resultMessage.role !== "toolResult") throw new Error("Expected tool result message");
+
+		expect(resultMessage.content).toEqual([
+			{ type: "text", text: "[image omitted: WebP could not be decoded for this model]" },
+		]);
+		expect(messages[0]!.content[0]).toBe(corrupt);
+	});
 });