From a78882d8b8d9359650482bed83d26313fad6ad34 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Sun, 22 Mar 2026 21:48:34 +0100 Subject: [PATCH] fix(coding-agent): enforce safe auto-resized image limits closes #2055 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/cli/file-processor.ts | 4 + packages/coding-agent/src/core/tools/read.ts | 23 ++- .../coding-agent/src/utils/image-resize.ts | 171 ++++++------------ .../test/image-processing.test.ts | 48 +++-- .../test/image-resize-callers.test.ts | 53 ++++++ 6 files changed, 161 insertions(+), 139 deletions(-) create mode 100644 packages/coding-agent/test/image-resize-callers.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0ddd4ab0..9f34ac76 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed auto-resized image handling to enforce the inline image size limit on the final base64 payload, return text-only fallbacks when resizing cannot produce a safe image, and avoid falling back to the original image in `read` and `@file` auto-resize paths ([#2055](https://github.com/badlogic/pi-mono/issues/2055)) - Fixed `pi update` for git packages to skip destructive reset, clean, and reinstall steps when the fetched target already matches the local checkout ([#2503](https://github.com/badlogic/pi-mono/issues/2503)) - Fixed print and JSON mode to take over stdout during non-interactive startup, keeping package-manager and other incidental chatter off protocol/output stdout ([#2482](https://github.com/badlogic/pi-mono/issues/2482)) diff --git a/packages/coding-agent/src/cli/file-processor.ts b/packages/coding-agent/src/cli/file-processor.ts index 8b676419..96a47c26 100644 --- a/packages/coding-agent/src/cli/file-processor.ts +++ b/packages/coding-agent/src/cli/file-processor.ts @@ -57,6 +57,10 @@ export async function processFileArguments(fileArgs: string[], options?: Process if (autoResizeImages) { const resized = await resizeImage({ type: "image", data: base64Content, mimeType }); + if (!resized) { + text += `[Image omitted: could not be resized below the inline image size limit.]\n`; + continue; + } dimensionNote = formatDimensionNote(resized); attachment = { type: "image", diff --git a/packages/coding-agent/src/core/tools/read.ts b/packages/coding-agent/src/core/tools/read.ts index d23b4313..bf531950 100644 --- a/packages/coding-agent/src/core/tools/read.ts +++ b/packages/coding-agent/src/core/tools/read.ts @@ -160,13 +160,22 @@ export function createReadToolDefinition( if (autoResizeImages) { // Resize image if needed before sending it back to the model. const resized = await resizeImage({ type: "image", data: base64, mimeType }); - const dimensionNote = formatDimensionNote(resized); - let textNote = `Read image file [${resized.mimeType}]`; - if (dimensionNote) textNote += `\n${dimensionNote}`; - content = [ - { type: "text", text: textNote }, - { type: "image", data: resized.data, mimeType: resized.mimeType }, - ]; + if (!resized) { + content = [ + { + type: "text", + text: `Read image file [${mimeType}]\n[Image omitted: could not be resized below the inline image size limit.]`, + }, + ]; + } else { + const dimensionNote = formatDimensionNote(resized); + let textNote = `Read image file [${resized.mimeType}]`; + if (dimensionNote) textNote += `\n${dimensionNote}`; + content = [ + { type: "text", text: textNote }, + { type: "image", data: resized.data, mimeType: resized.mimeType }, + ]; + } } else { content = [ { type: "text", text: `Read image file [${mimeType}]` }, diff --git a/packages/coding-agent/src/utils/image-resize.ts b/packages/coding-agent/src/utils/image-resize.ts index 69647c66..8c14d0b6 100644 --- a/packages/coding-agent/src/utils/image-resize.ts +++ b/packages/coding-agent/src/utils/image-resize.ts @@ -5,7 +5,7 @@ import { loadPhoton } from "./photon.js"; export interface ImageResizeOptions { maxWidth?: number; // Default: 2000 maxHeight?: number; // Default: 2000 - maxBytes?: number; // Default: 4.5MB (below Anthropic's 5MB limit) + maxBytes?: number; // Default: 4.5MB of base64 payload (below Anthropic's 5MB limit) jpegQuality?: number; // Default: 80 } @@ -19,7 +19,7 @@ export interface ResizedImage { wasResized: boolean; } -// 4.5MB - provides headroom below Anthropic's 5MB limit +// 4.5MB of base64 payload. Provides headroom below Anthropic's 5MB limit. const DEFAULT_MAX_BYTES = 4.5 * 1024 * 1024; const DEFAULT_OPTIONS: Required = { @@ -29,43 +29,42 @@ const DEFAULT_OPTIONS: Required = { jpegQuality: 80, }; -/** Helper to pick the smaller of two buffers */ -function pickSmaller( - a: { buffer: Uint8Array; mimeType: string }, - b: { buffer: Uint8Array; mimeType: string }, -): { buffer: Uint8Array; mimeType: string } { - return a.buffer.length <= b.buffer.length ? a : b; +interface EncodedCandidate { + data: string; + encodedSize: number; + mimeType: string; +} + +function encodeCandidate(buffer: Uint8Array, mimeType: string): EncodedCandidate { + const data = Buffer.from(buffer).toString("base64"); + return { + data, + encodedSize: Buffer.byteLength(data, "utf-8"), + mimeType, + }; } /** - * Resize an image to fit within the specified max dimensions and file size. - * Returns the original image if it already fits within the limits. + * Resize an image to fit within the specified max dimensions and encoded file size. + * Returns null if the image cannot be resized below maxBytes. * * Uses Photon (Rust/WASM) for image processing. If Photon is not available, - * returns the original image unchanged. + * returns null. * * Strategy for staying under maxBytes: * 1. First resize to maxWidth/maxHeight * 2. Try both PNG and JPEG formats, pick the smaller one * 3. If still too large, try JPEG with decreasing quality - * 4. If still too large, progressively reduce dimensions + * 4. If still too large, progressively reduce dimensions until 1x1 */ -export async function resizeImage(img: ImageContent, options?: ImageResizeOptions): Promise { +export async function resizeImage(img: ImageContent, options?: ImageResizeOptions): Promise { const opts = { ...DEFAULT_OPTIONS, ...options }; const inputBuffer = Buffer.from(img.data, "base64"); + const inputBase64Size = Buffer.byteLength(img.data, "utf-8"); const photon = await loadPhoton(); if (!photon) { - // Photon not available, return original image - return { - data: img.data, - mimeType: img.mimeType, - originalWidth: 0, - originalHeight: 0, - width: 0, - height: 0, - wasResized: false, - }; + return null; } let image: ReturnType | undefined; @@ -79,9 +78,8 @@ export async function resizeImage(img: ImageContent, options?: ImageResizeOption const originalHeight = image.get_height(); const format = img.mimeType?.split("/")[1] ?? "png"; - // Check if already within all limits (dimensions AND size) - const originalSize = inputBuffer.length; - if (originalWidth <= opts.maxWidth && originalHeight <= opts.maxHeight && originalSize <= opts.maxBytes) { + // Check if already within all limits (dimensions AND encoded size) + if (originalWidth <= opts.maxWidth && originalHeight <= opts.maxHeight && inputBase64Size < opts.maxBytes) { return { data: img.data, mimeType: img.mimeType ?? `image/${format}`, @@ -106,114 +104,57 @@ export async function resizeImage(img: ImageContent, options?: ImageResizeOption targetHeight = opts.maxHeight; } - // Helper to resize and encode in both formats, returning the smaller one - function tryBothFormats( - width: number, - height: number, - jpegQuality: number, - ): { buffer: Uint8Array; mimeType: string } { + function tryEncodings(width: number, height: number, jpegQualities: number[]): EncodedCandidate[] { const resized = photon!.resize(image!, width, height, photon!.SamplingFilter.Lanczos3); try { - const pngBuffer = resized.get_bytes(); - const jpegBuffer = resized.get_bytes_jpeg(jpegQuality); - - return pickSmaller( - { buffer: pngBuffer, mimeType: "image/png" }, - { buffer: jpegBuffer, mimeType: "image/jpeg" }, - ); + const candidates: EncodedCandidate[] = [encodeCandidate(resized.get_bytes(), "image/png")]; + for (const quality of jpegQualities) { + candidates.push(encodeCandidate(resized.get_bytes_jpeg(quality), "image/jpeg")); + } + return candidates; } finally { resized.free(); } } - // Try to produce an image under maxBytes - const qualitySteps = [85, 70, 55, 40]; - const scaleSteps = [1.0, 0.75, 0.5, 0.35, 0.25]; + const qualitySteps = Array.from(new Set([opts.jpegQuality, 85, 70, 55, 40])); + let currentWidth = targetWidth; + let currentHeight = targetHeight; - let best: { buffer: Uint8Array; mimeType: string }; - let finalWidth = targetWidth; - let finalHeight = targetHeight; - - // First attempt: resize to target dimensions, try both formats - best = tryBothFormats(targetWidth, targetHeight, opts.jpegQuality); - - if (best.buffer.length <= opts.maxBytes) { - return { - data: Buffer.from(best.buffer).toString("base64"), - mimeType: best.mimeType, - originalWidth, - originalHeight, - width: finalWidth, - height: finalHeight, - wasResized: true, - }; - } - - // Still too large - try JPEG with decreasing quality - for (const quality of qualitySteps) { - best = tryBothFormats(targetWidth, targetHeight, quality); - - if (best.buffer.length <= opts.maxBytes) { - return { - data: Buffer.from(best.buffer).toString("base64"), - mimeType: best.mimeType, - originalWidth, - originalHeight, - width: finalWidth, - height: finalHeight, - wasResized: true, - }; - } - } - - // Still too large - reduce dimensions progressively - for (const scale of scaleSteps) { - finalWidth = Math.round(targetWidth * scale); - finalHeight = Math.round(targetHeight * scale); - - if (finalWidth < 100 || finalHeight < 100) { - break; - } - - for (const quality of qualitySteps) { - best = tryBothFormats(finalWidth, finalHeight, quality); - - if (best.buffer.length <= opts.maxBytes) { + while (true) { + const candidates = tryEncodings(currentWidth, currentHeight, qualitySteps); + for (const candidate of candidates) { + if (candidate.encodedSize < opts.maxBytes) { return { - data: Buffer.from(best.buffer).toString("base64"), - mimeType: best.mimeType, + data: candidate.data, + mimeType: candidate.mimeType, originalWidth, originalHeight, - width: finalWidth, - height: finalHeight, + width: currentWidth, + height: currentHeight, wasResized: true, }; } } + + if (currentWidth === 1 && currentHeight === 1) { + break; + } + + const nextWidth = currentWidth === 1 ? 1 : Math.max(1, Math.floor(currentWidth * 0.75)); + const nextHeight = currentHeight === 1 ? 1 : Math.max(1, Math.floor(currentHeight * 0.75)); + if (nextWidth === currentWidth && nextHeight === currentHeight) { + break; + } + + currentWidth = nextWidth; + currentHeight = nextHeight; } - // Last resort: return smallest version we produced - return { - data: Buffer.from(best.buffer).toString("base64"), - mimeType: best.mimeType, - originalWidth, - originalHeight, - width: finalWidth, - height: finalHeight, - wasResized: true, - }; + return null; } catch { - // Failed to load image - return { - data: img.data, - mimeType: img.mimeType, - originalWidth: 0, - originalHeight: 0, - width: 0, - height: 0, - wasResized: false, - }; + return null; } finally { if (image) { image.free(); diff --git a/packages/coding-agent/test/image-processing.test.ts b/packages/coding-agent/test/image-processing.test.ts index 18140a5e..4dba6f2d 100644 --- a/packages/coding-agent/test/image-processing.test.ts +++ b/packages/coding-agent/test/image-processing.test.ts @@ -52,12 +52,13 @@ describe("resizeImage", () => { { maxWidth: 100, maxHeight: 100, maxBytes: 1024 * 1024 }, ); - expect(result.wasResized).toBe(false); - expect(result.data).toBe(TINY_PNG); - expect(result.originalWidth).toBe(2); - expect(result.originalHeight).toBe(2); - expect(result.width).toBe(2); - expect(result.height).toBe(2); + expect(result).not.toBeNull(); + expect(result!.wasResized).toBe(false); + expect(result!.data).toBe(TINY_PNG); + expect(result!.originalWidth).toBe(2); + expect(result!.originalHeight).toBe(2); + expect(result!.width).toBe(2); + expect(result!.height).toBe(2); }); it("should resize image exceeding dimension limits", async () => { @@ -66,26 +67,38 @@ describe("resizeImage", () => { { maxWidth: 50, maxHeight: 50, maxBytes: 1024 * 1024 }, ); - expect(result.wasResized).toBe(true); - expect(result.originalWidth).toBe(100); - expect(result.originalHeight).toBe(100); - expect(result.width).toBeLessThanOrEqual(50); - expect(result.height).toBeLessThanOrEqual(50); + expect(result).not.toBeNull(); + expect(result!.wasResized).toBe(true); + expect(result!.originalWidth).toBe(100); + expect(result!.originalHeight).toBe(100); + expect(result!.width).toBeLessThanOrEqual(50); + expect(result!.height).toBeLessThanOrEqual(50); }); it("should resize image exceeding byte limit", async () => { const originalBuffer = Buffer.from(LARGE_PNG_200x200, "base64"); const originalSize = originalBuffer.length; - // Set maxBytes to less than the original image size + // Set maxBytes to less than the original encoded image size const result = await resizeImage( { type: "image", data: LARGE_PNG_200x200, mimeType: "image/png" }, - { maxWidth: 2000, maxHeight: 2000, maxBytes: Math.floor(originalSize / 2) }, + { maxWidth: 2000, maxHeight: 2000, maxBytes: Math.floor(LARGE_PNG_200x200.length * 0.9) }, ); // Should have tried to reduce size - const resultBuffer = Buffer.from(result.data, "base64"); + expect(result).not.toBeNull(); + const resultBuffer = Buffer.from(result!.data, "base64"); expect(resultBuffer.length).toBeLessThan(originalSize); + expect(result!.data.length).toBeLessThan(LARGE_PNG_200x200.length); + }); + + it("should return null when image cannot be resized below maxBytes", async () => { + const result = await resizeImage( + { type: "image", data: LARGE_PNG_200x200, mimeType: "image/png" }, + { maxWidth: 2000, maxHeight: 2000, maxBytes: 1 }, + ); + + expect(result).toBeNull(); }); it("should handle JPEG input", async () => { @@ -94,9 +107,10 @@ describe("resizeImage", () => { { maxWidth: 100, maxHeight: 100, maxBytes: 1024 * 1024 }, ); - expect(result.wasResized).toBe(false); - expect(result.originalWidth).toBe(2); - expect(result.originalHeight).toBe(2); + expect(result).not.toBeNull(); + expect(result!.wasResized).toBe(false); + expect(result!.originalWidth).toBe(2); + expect(result!.originalHeight).toBe(2); }); }); diff --git a/packages/coding-agent/test/image-resize-callers.test.ts b/packages/coding-agent/test/image-resize-callers.test.ts new file mode 100644 index 00000000..5d1c1a79 --- /dev/null +++ b/packages/coding-agent/test/image-resize-callers.test.ts @@ -0,0 +1,53 @@ +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../src/utils/image-resize.js", () => ({ + resizeImage: vi.fn(), + formatDimensionNote: vi.fn(() => undefined), +})); + +import { processFileArguments } from "../src/cli/file-processor.js"; +import { createReadTool } from "../src/core/tools/read.js"; +import { resizeImage } from "../src/utils/image-resize.js"; + +const TINY_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg=="; + +describe("image resize callers", () => { + let testDir: string; + + beforeEach(() => { + testDir = join(tmpdir(), `image-resize-callers-${Date.now()}`); + mkdirSync(testDir, { recursive: true }); + vi.mocked(resizeImage).mockReset(); + vi.mocked(resizeImage).mockResolvedValue(null); + }); + + afterEach(() => { + rmSync(testDir, { recursive: true, force: true }); + }); + + it("read tool returns text-only output when auto-resize cannot produce a safe image", async () => { + const imagePath = join(testDir, "test.png"); + writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + const tool = createReadTool(testDir); + const result = await tool.execute("test-read-image", { path: imagePath }); + + expect(result.content).toHaveLength(1); + expect(result.content[0].type).toBe("text"); + expect((result.content[0] as { type: "text"; text: string }).text).toContain("Image omitted"); + }); + + it("file processor omits image attachments when auto-resize cannot produce a safe image", async () => { + const imagePath = join(testDir, "test.png"); + writeFileSync(imagePath, Buffer.from(TINY_PNG_BASE64, "base64")); + + const result = await processFileArguments([imagePath]); + + expect(result.images).toHaveLength(0); + expect(result.text).toContain("Image omitted"); + }); +});