From 44f774fe413f3e7074886379e94fba7fe3be1d26 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Wed, 17 Dec 2025 21:02:19 -0700 Subject: [PATCH 1/6] feat: add support for image file @mentions - Add resolveImageMentions to convert @/path/to/image.png mentions to base64 data URLs - Integrate image resolution for newTask, askResponse, editMessageConfirm, queueMessage - Add binary file detection to skip including binary content as text context - Support png, jpg, jpeg, webp formats with 20 image limit per message - Add unit and integration tests for image mention handling --- .../__tests__/resolveImageMentions.spec.ts | 89 +++++++++++++ src/core/mentions/index.ts | 8 +- src/core/mentions/resolveImageMentions.ts | 117 ++++++++++++++++++ ...eHandler.imageMentions.integration.spec.ts | 77 ++++++++++++ .../__tests__/webviewMessageHandler.spec.ts | 36 ++++++ src/core/webview/webviewMessageHandler.ts | 32 ++++- 6 files changed, 353 insertions(+), 6 deletions(-) create mode 100644 src/core/mentions/__tests__/resolveImageMentions.spec.ts create mode 100644 src/core/mentions/resolveImageMentions.ts create mode 100644 src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts diff --git a/src/core/mentions/__tests__/resolveImageMentions.spec.ts b/src/core/mentions/__tests__/resolveImageMentions.spec.ts new file mode 100644 index 00000000000..7aad639677e --- /dev/null +++ b/src/core/mentions/__tests__/resolveImageMentions.spec.ts @@ -0,0 +1,89 @@ +import * as path from "path" + +import { resolveImageMentions } from "../resolveImageMentions" + +vi.mock("fs/promises", () => { + return { + default: { + readFile: vi.fn(), + }, + readFile: vi.fn(), + } +}) + +import * as fs from "fs/promises" + +const mockReadFile = vi.mocked(fs.readFile) + +describe("resolveImageMentions", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("should append a data URL when a local png mention is present", async () => { + mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + + const result = await resolveImageMentions({ + text: "Please look at @/assets/cat.png", + images: [], + cwd: "/workspace", + }) + + expect(mockReadFile).toHaveBeenCalledWith(path.resolve("/workspace", "assets/cat.png")) + expect(result.text).toBe("Please look at @/assets/cat.png") + expect(result.images).toEqual([`data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}`]) + }) + + it("should ignore non-image mentions", async () => { + const result = await resolveImageMentions({ + text: "See @/src/index.ts", + images: [], + cwd: "/workspace", + }) + + expect(mockReadFile).not.toHaveBeenCalled() + expect(result.images).toEqual([]) + }) + + it("should skip unreadable files (fail-soft)", async () => { + mockReadFile.mockRejectedValue(new Error("ENOENT")) + + const result = await resolveImageMentions({ + text: "See @/missing.webp", + images: [], + cwd: "/workspace", + }) + + expect(result.images).toEqual([]) + }) + + it("should respect rooIgnoreController", async () => { + mockReadFile.mockResolvedValue(Buffer.from("jpg-bytes")) + const rooIgnoreController = { + validateAccess: vi.fn().mockReturnValue(false), + } + + const result = await resolveImageMentions({ + text: "See @/secret.jpg", + images: [], + cwd: "/workspace", + rooIgnoreController, + }) + + expect(rooIgnoreController.validateAccess).toHaveBeenCalledWith("secret.jpg") + expect(mockReadFile).not.toHaveBeenCalled() + expect(result.images).toEqual([]) + }) + + it("should dedupe when mention repeats", async () => { + mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + + const result = await resolveImageMentions({ + text: "@/a.png and again @/a.png", + images: [], + cwd: "/workspace", + }) + + expect(result.images).toHaveLength(1) + }) +}) diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index f038b5b7836..1ee94c2b86c 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -274,7 +274,13 @@ async function getFileOrFolderContent( const stats = await fs.stat(absPath) if (stats.isFile()) { - if (rooIgnoreController && !rooIgnoreController.validateAccess(absPath)) { + // Avoid trying to include image binary content as text context. + // Image mentions are handled separately via image attachment flow. + const isBinary = await isBinaryFile(absPath).catch(() => false) + if (isBinary) { + return `(Binary file ${mentionPath} omitted)` + } + if (rooIgnoreController && !rooIgnoreController.validateAccess(unescapedPath)) { return `(File ${mentionPath} is ignored by .rooignore)` } try { diff --git a/src/core/mentions/resolveImageMentions.ts b/src/core/mentions/resolveImageMentions.ts new file mode 100644 index 00000000000..53de9df67ed --- /dev/null +++ b/src/core/mentions/resolveImageMentions.ts @@ -0,0 +1,117 @@ +import * as path from "path" +import * as fs from "fs/promises" + +import { mentionRegexGlobal, unescapeSpaces } from "../../shared/context-mentions" + +const MAX_IMAGES_PER_MESSAGE = 20 + +const SUPPORTED_IMAGE_EXTENSIONS = new Set([".png", ".jpg", ".jpeg", ".webp"]) + +function getMimeTypeFromExtension(extLower: string): string | undefined { + if (extLower === ".png") return "image/png" + if (extLower === ".jpg" || extLower === ".jpeg") return "image/jpeg" + if (extLower === ".webp") return "image/webp" + return undefined +} + +export interface ResolveImageMentionsOptions { + text: string + images?: string[] + cwd: string + rooIgnoreController?: { validateAccess: (filePath: string) => boolean } +} + +export interface ResolveImageMentionsResult { + text: string + images: string[] +} + +function isPathWithinCwd(absPath: string, cwd: string): boolean { + const rel = path.relative(cwd, absPath) + return rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel) +} + +function dedupePreserveOrder(values: string[]): string[] { + const seen = new Set() + const result: string[] = [] + for (const v of values) { + if (seen.has(v)) continue + seen.add(v) + result.push(v) + } + return result +} + +/** + * Resolves local image file mentions like `@/path/to/image.png` found in `text` into `data:image/...;base64,...` + * and appends them to the outgoing `images` array. + * + * - Only supports local workspace-relative mentions (must start with `/`). + * - Only supports: png, jpg, jpeg, webp. + * - Leaves `text` unchanged. + * - Respects `.rooignore` via `rooIgnoreController.validateAccess` when provided. + */ +export async function resolveImageMentions({ + text, + images, + cwd, + rooIgnoreController, +}: ResolveImageMentionsOptions): Promise { + const existingImages = Array.isArray(images) ? images : [] + if (existingImages.length >= MAX_IMAGES_PER_MESSAGE) { + return { text, images: existingImages.slice(0, MAX_IMAGES_PER_MESSAGE) } + } + + const mentions = Array.from(text.matchAll(mentionRegexGlobal)) + .map((m) => m[1]) + .filter(Boolean) + if (mentions.length === 0) { + return { text, images: existingImages } + } + + const imageMentions = mentions.filter((mention) => { + if (!mention.startsWith("/")) return false + const relPath = unescapeSpaces(mention.slice(1)) + const ext = path.extname(relPath).toLowerCase() + return SUPPORTED_IMAGE_EXTENSIONS.has(ext) + }) + + if (imageMentions.length === 0) { + return { text, images: existingImages } + } + + const newImages: string[] = [] + for (const mention of imageMentions) { + if (existingImages.length + newImages.length >= MAX_IMAGES_PER_MESSAGE) { + break + } + + const relPath = unescapeSpaces(mention.slice(1)) + const absPath = path.resolve(cwd, relPath) + if (!isPathWithinCwd(absPath, cwd)) { + continue + } + + if (rooIgnoreController && !rooIgnoreController.validateAccess(relPath)) { + continue + } + + const ext = path.extname(relPath).toLowerCase() + const mimeType = getMimeTypeFromExtension(ext) + if (!mimeType) { + continue + } + + try { + const buffer = await fs.readFile(absPath) + const base64 = buffer.toString("base64") + newImages.push(`data:${mimeType};base64,${base64}`) + } catch { + // Fail-soft: skip unreadable/missing files. + continue + } + } + + const merged = dedupePreserveOrder([...existingImages, ...newImages]).slice(0, MAX_IMAGES_PER_MESSAGE) + return { text, images: merged } +} diff --git a/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts new file mode 100644 index 00000000000..24af44d2b9b --- /dev/null +++ b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts @@ -0,0 +1,77 @@ +import * as fs from "fs/promises" +import * as path from "path" + +// Must mock dependencies before importing the handler module. +vi.mock("../../../api/providers/fetchers/modelCache") + +import { webviewMessageHandler } from "../webviewMessageHandler" +import type { ClineProvider } from "../ClineProvider" + +vi.mock("vscode", () => ({ + window: { + showInformationMessage: vi.fn(), + showErrorMessage: vi.fn(), + }, + workspace: { + workspaceFolders: [{ uri: { fsPath: "/mock/workspace" } }], + }, +})) + +describe("webviewMessageHandler - image mentions (integration)", () => { + it("resolves image mentions for newTask and passes images to createTask", async () => { + const tmpRoot = await fs.mkdtemp(path.join(process.cwd(), "tmp-image-mentions-")) + try { + const imgBytes = Buffer.from("png-bytes") + await fs.writeFile(path.join(tmpRoot, "cat.png"), imgBytes) + + const mockProvider = { + cwd: tmpRoot, + getCurrentTask: vi.fn().mockReturnValue(undefined), + createTask: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + } as unknown as ClineProvider + + await webviewMessageHandler(mockProvider, { + type: "newTask", + text: "Please look at @/cat.png", + images: [], + } as any) + + expect(mockProvider.createTask).toHaveBeenCalledWith("Please look at @/cat.png", [ + `data:image/png;base64,${imgBytes.toString("base64")}`, + ]) + } finally { + await fs.rm(tmpRoot, { recursive: true, force: true }) + } + }) + + it("resolves image mentions for askResponse and passes images to handleWebviewAskResponse", async () => { + const tmpRoot = await fs.mkdtemp(path.join(process.cwd(), "tmp-image-mentions-")) + try { + const imgBytes = Buffer.from("jpg-bytes") + await fs.writeFile(path.join(tmpRoot, "cat.jpg"), imgBytes) + + const handleWebviewAskResponse = vi.fn() + const mockProvider = { + cwd: tmpRoot, + getCurrentTask: vi.fn().mockReturnValue({ + cwd: tmpRoot, + handleWebviewAskResponse, + }), + } as unknown as ClineProvider + + await webviewMessageHandler(mockProvider, { + type: "askResponse", + askResponse: "messageResponse", + text: "Please look at @/cat.jpg", + images: [], + } as any) + + expect(handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Please look at @/cat.jpg", [ + `data:image/jpeg;base64,${imgBytes.toString("base64")}`, + ]) + } finally { + await fs.rm(tmpRoot, { recursive: true, force: true }) + } + }) +}) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index f724558ae6a..b113195bfdc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -96,6 +96,15 @@ vi.mock("../../../utils/fs") vi.mock("../../../utils/path") vi.mock("../../../utils/globalContext") +vi.mock("../../mentions/resolveImageMentions", () => ({ + resolveImageMentions: vi.fn(async ({ text, images }: { text: string; images?: string[] }) => ({ + text, + images: [...(images ?? []), "data:image/png;base64,from-mention"], + })), +})) + +import { resolveImageMentions } from "../../mentions/resolveImageMentions" + describe("webviewMessageHandler - requestLmStudioModels", () => { beforeEach(() => { vi.clearAllMocks() @@ -138,6 +147,33 @@ describe("webviewMessageHandler - requestLmStudioModels", () => { }) }) +describe("webviewMessageHandler - image mentions", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("should resolve image mentions for askResponse payloads", async () => { + const mockHandleWebviewAskResponse = vi.fn() + vi.mocked(mockClineProvider.getCurrentTask).mockReturnValue({ + cwd: "/mock/workspace", + rooIgnoreController: undefined, + handleWebviewAskResponse: mockHandleWebviewAskResponse, + } as any) + + await webviewMessageHandler(mockClineProvider, { + type: "askResponse", + askResponse: "messageResponse", + text: "See @/img.png", + images: [], + }) + + expect(vi.mocked(resolveImageMentions)).toHaveBeenCalled() + expect(mockHandleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "See @/img.png", [ + "data:image/png;base64,from-mention", + ]) + }) +}) + describe("webviewMessageHandler - requestOllamaModels", () => { beforeEach(() => { vi.clearAllMocks() diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index c08504c576d..1fdb1c6e51f 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -52,6 +52,7 @@ import { exportSettings, importSettingsWithFeedback } from "../config/importExpo import { getOpenAiModels } from "../../api/providers/openai" import { getVsCodeLmModels } from "../../api/providers/vscode-lm" import { openMention } from "../mentions" +import { resolveImageMentions } from "../mentions/resolveImageMentions" import { getWorkspacePath } from "../../utils/path" import { Mode, defaultModeSlug } from "../../shared/modes" import { getModels, flushModels } from "../../api/providers/fetchers/modelCache" @@ -77,6 +78,19 @@ export const webviewMessageHandler = async ( const getCurrentCwd = () => { return provider.getCurrentTask()?.cwd || provider.cwd } + + const resolveIncomingImages = async (payload: { text?: string; images?: string[] }) => { + const text = payload.text ?? "" + const images = payload.images + const currentTask = provider.getCurrentTask() + const resolved = await resolveImageMentions({ + text, + images, + cwd: getCurrentCwd(), + rooIgnoreController: currentTask?.rooIgnoreController, + }) + return resolved + } /** * Shared utility to find message indices based on timestamp. * When multiple messages share the same timestamp (e.g., after condense), @@ -503,7 +517,8 @@ export const webviewMessageHandler = async ( // agentically running promises in old instance don't affect our new // task. This essentially creates a fresh slate for the new task. try { - await provider.createTask(message.text, message.images) + const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + await provider.createTask(resolved.text, resolved.images) // Task created successfully - notify the UI to reset await provider.postMessageToWebview({ type: "invoke", invoke: "newChat" }) } catch (error) { @@ -520,7 +535,12 @@ export const webviewMessageHandler = async ( break case "askResponse": - provider.getCurrentTask()?.handleWebviewAskResponse(message.askResponse!, message.text, message.images) + { + const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + provider + .getCurrentTask() + ?.handleWebviewAskResponse(message.askResponse!, resolved.text, resolved.images) + } break case "updateSettings": @@ -1842,11 +1862,12 @@ export const webviewMessageHandler = async ( break case "editMessageConfirm": if (message.messageTs && message.text) { + const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) await handleEditMessageConfirm( message.messageTs, - message.text, + resolved.text, message.restoreCheckpoint, - message.images, + resolved.images, ) } break @@ -2991,7 +3012,8 @@ export const webviewMessageHandler = async ( */ case "queueMessage": { - provider.getCurrentTask()?.messageQueueService.addMessage(message.text ?? "", message.images) + const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + provider.getCurrentTask()?.messageQueueService.addMessage(resolved.text, resolved.images) break } case "removeQueuedMessage": { From 826275d106c931d7358e7cb78565ea45fcfaf979 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 18 Dec 2025 10:21:43 -0700 Subject: [PATCH 2/6] test: normalize edit-dialog images to empty array --- .../webview/__tests__/ClineProvider.spec.ts | 22 +++++++++---------- .../webviewMessageHandler.checkpoint.spec.ts | 2 +- .../__tests__/webviewMessageHandler.spec.ts | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 85e144290f0..1b70e2eb4bf 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1305,7 +1305,7 @@ describe("ClineProvider", () => { messageTs: 4000, text: "Edited message content", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming edit through the dialog @@ -3034,7 +3034,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 3000, text: "Edited message with preserved images", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate confirmation @@ -3090,7 +3090,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 3000, text: "Edited message with file attachment", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3144,7 +3144,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3186,7 +3186,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3244,14 +3244,14 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message 1", hasCheckpoint: false, - images: undefined, + images: [], }) expect(mockPostMessage).toHaveBeenCalledWith({ type: "showEditMessageDialog", messageTs: 4000, text: "Edited message 2", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming both edits @@ -3438,7 +3438,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 5000, text: "Edited non-existent message", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3532,7 +3532,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3625,7 +3625,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: largeEditedContent, hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit @@ -3833,7 +3833,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: futureTimestamp + 1000, text: "Edited future message", hasCheckpoint: false, - images: undefined, + images: [], }) // Simulate user confirming the edit diff --git a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts index a0687d6cc19..423bab542f9 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts @@ -124,7 +124,7 @@ describe("webviewMessageHandler - checkpoint operations", () => { operation: "edit", editData: { editedContent: "Edited checkpoint message", - images: undefined, + images: [], apiConversationHistoryIndex: 0, }, }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index b113195bfdc..385b79efd9f 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -718,7 +718,7 @@ describe("webviewMessageHandler - message dialog preferences", () => { messageTs: 123456789, text: "edited content", hasCheckpoint: false, - images: undefined, + images: [], }) }) }) From bc49d923e70801094e8f7bea22e4d26b86ad2ff0 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 18 Dec 2025 10:24:57 -0700 Subject: [PATCH 3/6] test: fix edit dialog image expectation --- .../webview/__tests__/ClineProvider.spec.ts | 28 +++++++++---------- .../__tests__/webviewMessageHandler.spec.ts | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 1b70e2eb4bf..f73cf0eb588 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1305,7 +1305,7 @@ describe("ClineProvider", () => { messageTs: 4000, text: "Edited message content", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming edit through the dialog @@ -3034,7 +3034,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 3000, text: "Edited message with preserved images", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate confirmation @@ -3048,7 +3048,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { expect(mockCline.overwriteClineMessages).toHaveBeenCalledWith([mockMessages[0]]) expect(mockCline.overwriteApiConversationHistory).toHaveBeenCalledWith([{ ts: 1000 }]) // Verify submitUserMessage was called with the edited content - expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with preserved images", undefined) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with preserved images", []) }) test("handles editing messages with file attachments", async () => { @@ -3090,7 +3090,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 3000, text: "Edited message with file attachment", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit @@ -3101,7 +3101,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }) expect(mockCline.overwriteClineMessages).toHaveBeenCalled() - expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with file attachment", undefined) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith("Edited message with file attachment", []) }) }) @@ -3144,7 +3144,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit @@ -3186,7 +3186,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit @@ -3244,14 +3244,14 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message 1", hasCheckpoint: false, - images: [], + images: undefined, }) expect(mockPostMessage).toHaveBeenCalledWith({ type: "showEditMessageDialog", messageTs: 4000, text: "Edited message 2", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming both edits @@ -3438,7 +3438,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 5000, text: "Edited non-existent message", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit @@ -3532,7 +3532,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: "Edited message", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit @@ -3625,14 +3625,14 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: 2000, text: largeEditedContent, hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit await messageHandler({ type: "editMessageConfirm", messageTs: 2000, text: largeEditedContent }) expect(mockCline.overwriteClineMessages).toHaveBeenCalled() - expect(mockCline.submitUserMessage).toHaveBeenCalledWith(largeEditedContent, undefined) + expect(mockCline.submitUserMessage).toHaveBeenCalledWith(largeEditedContent, []) }) test("handles deleting messages with large payloads", async () => { @@ -3833,7 +3833,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { messageTs: futureTimestamp + 1000, text: "Edited future message", hasCheckpoint: false, - images: [], + images: undefined, }) // Simulate user confirming the edit diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 385b79efd9f..b113195bfdc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -718,7 +718,7 @@ describe("webviewMessageHandler - message dialog preferences", () => { messageTs: 123456789, text: "edited content", hasCheckpoint: false, - images: [], + images: undefined, }) }) }) From 24b1f6edcc3ea31af70026d6d64905b238fb1f8d Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 18 Dec 2025 10:54:45 -0700 Subject: [PATCH 4/6] fix: align image mentions with read_file behavior - Add support for all 11 image formats (.gif, .svg, .bmp, .ico, .tiff, .tif, .avif) - Add per-file size validation using validateImageForProcessing - Add total memory tracking with ImageMemoryTracker - Add supportsImages check for model compatibility - Reuse IMAGE_MIME_TYPES and SUPPORTED_IMAGE_FORMATS from imageHelpers - Add JSDoc for resolveIncomingImages helper - Update integration tests to use os.tmpdir() instead of process.cwd() - Add comprehensive test coverage for new validation behavior --- .../__tests__/resolveImageMentions.spec.ts | 129 ++++++++++++++++++ src/core/mentions/resolveImageMentions.ts | 82 +++++++++-- .../webviewMessageHandler.checkpoint.spec.ts | 4 + .../webviewMessageHandler.edit.spec.ts | 4 + ...eHandler.imageMentions.integration.spec.ts | 81 ++++++++++- .../__tests__/webviewMessageHandler.spec.ts | 4 + src/core/webview/webviewMessageHandler.ts | 7 + 7 files changed, 294 insertions(+), 17 deletions(-) diff --git a/src/core/mentions/__tests__/resolveImageMentions.spec.ts b/src/core/mentions/__tests__/resolveImageMentions.spec.ts index 7aad639677e..9ae9fb3fdaf 100644 --- a/src/core/mentions/__tests__/resolveImageMentions.spec.ts +++ b/src/core/mentions/__tests__/resolveImageMentions.spec.ts @@ -6,18 +6,60 @@ vi.mock("fs/promises", () => { return { default: { readFile: vi.fn(), + stat: vi.fn(), }, readFile: vi.fn(), + stat: vi.fn(), } }) +vi.mock("../../tools/helpers/imageHelpers", () => ({ + SUPPORTED_IMAGE_FORMATS: [ + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".svg", + ".bmp", + ".ico", + ".tiff", + ".tif", + ".avif", + ], + IMAGE_MIME_TYPES: { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".gif": "image/gif", + ".webp": "image/webp", + ".svg": "image/svg+xml", + ".bmp": "image/bmp", + ".ico": "image/x-icon", + ".tiff": "image/tiff", + ".tif": "image/tiff", + ".avif": "image/avif", + }, + validateImageForProcessing: vi.fn(), + ImageMemoryTracker: vi.fn().mockImplementation(() => ({ + getTotalMemoryUsed: vi.fn().mockReturnValue(0), + addMemoryUsage: vi.fn(), + })), + DEFAULT_MAX_IMAGE_FILE_SIZE_MB: 5, + DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB: 20, +})) + import * as fs from "fs/promises" +import { validateImageForProcessing } from "../../tools/helpers/imageHelpers" const mockReadFile = vi.mocked(fs.readFile) +const mockValidateImage = vi.mocked(validateImageForProcessing) describe("resolveImageMentions", () => { beforeEach(() => { vi.clearAllMocks() + // Default: validation passes + mockValidateImage.mockResolvedValue({ isValid: true, sizeInMB: 0.1 }) }) it("should append a data URL when a local png mention is present", async () => { @@ -29,11 +71,36 @@ describe("resolveImageMentions", () => { cwd: "/workspace", }) + expect(mockValidateImage).toHaveBeenCalled() expect(mockReadFile).toHaveBeenCalledWith(path.resolve("/workspace", "assets/cat.png")) expect(result.text).toBe("Please look at @/assets/cat.png") expect(result.images).toEqual([`data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}`]) }) + it("should support gif images (matching read_file)", async () => { + mockReadFile.mockResolvedValue(Buffer.from("gif-bytes")) + + const result = await resolveImageMentions({ + text: "See @/animation.gif", + images: [], + cwd: "/workspace", + }) + + expect(result.images).toEqual([`data:image/gif;base64,${Buffer.from("gif-bytes").toString("base64")}`]) + }) + + it("should support svg images (matching read_file)", async () => { + mockReadFile.mockResolvedValue(Buffer.from("svg-bytes")) + + const result = await resolveImageMentions({ + text: "See @/icon.svg", + images: [], + cwd: "/workspace", + }) + + expect(result.images).toEqual([`data:image/svg+xml;base64,${Buffer.from("svg-bytes").toString("base64")}`]) + }) + it("should ignore non-image mentions", async () => { const result = await resolveImageMentions({ text: "See @/src/index.ts", @@ -86,4 +153,66 @@ describe("resolveImageMentions", () => { expect(result.images).toHaveLength(1) }) + + it("should skip images when supportsImages is false", async () => { + mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + + const result = await resolveImageMentions({ + text: "See @/cat.png", + images: [], + cwd: "/workspace", + supportsImages: false, + }) + + expect(mockReadFile).not.toHaveBeenCalled() + expect(result.images).toEqual([]) + }) + + it("should skip images that exceed size limits", async () => { + mockValidateImage.mockResolvedValue({ + isValid: false, + reason: "size_limit", + notice: "Image too large", + }) + + const result = await resolveImageMentions({ + text: "See @/huge.png", + images: [], + cwd: "/workspace", + }) + + expect(mockValidateImage).toHaveBeenCalled() + expect(mockReadFile).not.toHaveBeenCalled() + expect(result.images).toEqual([]) + }) + + it("should skip images that would exceed memory limit", async () => { + mockValidateImage.mockResolvedValue({ + isValid: false, + reason: "memory_limit", + notice: "Would exceed memory limit", + }) + + const result = await resolveImageMentions({ + text: "See @/large.png", + images: [], + cwd: "/workspace", + }) + + expect(result.images).toEqual([]) + }) + + it("should pass custom size limits to validation", async () => { + mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + + await resolveImageMentions({ + text: "See @/cat.png", + images: [], + cwd: "/workspace", + maxImageFileSize: 10, + maxTotalImageSize: 50, + }) + + expect(mockValidateImage).toHaveBeenCalledWith(expect.any(String), true, 10, 50, 0) + }) }) diff --git a/src/core/mentions/resolveImageMentions.ts b/src/core/mentions/resolveImageMentions.ts index 53de9df67ed..ae93d4e8978 100644 --- a/src/core/mentions/resolveImageMentions.ts +++ b/src/core/mentions/resolveImageMentions.ts @@ -2,23 +2,28 @@ import * as path from "path" import * as fs from "fs/promises" import { mentionRegexGlobal, unescapeSpaces } from "../../shared/context-mentions" +import { + SUPPORTED_IMAGE_FORMATS, + IMAGE_MIME_TYPES, + validateImageForProcessing, + ImageMemoryTracker, + DEFAULT_MAX_IMAGE_FILE_SIZE_MB, + DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB, +} from "../tools/helpers/imageHelpers" const MAX_IMAGES_PER_MESSAGE = 20 -const SUPPORTED_IMAGE_EXTENSIONS = new Set([".png", ".jpg", ".jpeg", ".webp"]) - -function getMimeTypeFromExtension(extLower: string): string | undefined { - if (extLower === ".png") return "image/png" - if (extLower === ".jpg" || extLower === ".jpeg") return "image/jpeg" - if (extLower === ".webp") return "image/webp" - return undefined -} - export interface ResolveImageMentionsOptions { text: string images?: string[] cwd: string rooIgnoreController?: { validateAccess: (filePath: string) => boolean } + /** Whether the current model supports images. Defaults to true. */ + supportsImages?: boolean + /** Maximum size per image file in MB. Defaults to 5MB. */ + maxImageFileSize?: number + /** Maximum total size of all images in MB. Defaults to 20MB. */ + maxTotalImageSize?: number } export interface ResolveImageMentionsResult { @@ -42,26 +47,52 @@ function dedupePreserveOrder(values: string[]): string[] { return result } +/** + * Checks if a file extension is a supported image format. + * Uses the same format list as read_file tool. + */ +function isSupportedImageExtension(extension: string): boolean { + return SUPPORTED_IMAGE_FORMATS.includes(extension.toLowerCase() as (typeof SUPPORTED_IMAGE_FORMATS)[number]) +} + +/** + * Gets the MIME type for an image extension. + * Uses the same mapping as read_file tool. + */ +function getMimeType(extension: string): string | undefined { + return IMAGE_MIME_TYPES[extension.toLowerCase()] +} + /** * Resolves local image file mentions like `@/path/to/image.png` found in `text` into `data:image/...;base64,...` * and appends them to the outgoing `images` array. * - * - Only supports local workspace-relative mentions (must start with `/`). - * - Only supports: png, jpg, jpeg, webp. - * - Leaves `text` unchanged. - * - Respects `.rooignore` via `rooIgnoreController.validateAccess` when provided. + * Behavior matches the read_file tool: + * - Supports the same image formats: png, jpg, jpeg, gif, webp, svg, bmp, ico, tiff, avif + * - Respects per-file size limits (default 5MB) + * - Respects total memory limits (default 20MB) + * - Skips images if model doesn't support them + * - Respects `.rooignore` via `rooIgnoreController.validateAccess` when provided */ export async function resolveImageMentions({ text, images, cwd, rooIgnoreController, + supportsImages = true, + maxImageFileSize = DEFAULT_MAX_IMAGE_FILE_SIZE_MB, + maxTotalImageSize = DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB, }: ResolveImageMentionsOptions): Promise { const existingImages = Array.isArray(images) ? images : [] if (existingImages.length >= MAX_IMAGES_PER_MESSAGE) { return { text, images: existingImages.slice(0, MAX_IMAGES_PER_MESSAGE) } } + // If model doesn't support images, skip image processing entirely + if (!supportsImages) { + return { text, images: existingImages } + } + const mentions = Array.from(text.matchAll(mentionRegexGlobal)) .map((m) => m[1]) .filter(Boolean) @@ -73,14 +104,16 @@ export async function resolveImageMentions({ if (!mention.startsWith("/")) return false const relPath = unescapeSpaces(mention.slice(1)) const ext = path.extname(relPath).toLowerCase() - return SUPPORTED_IMAGE_EXTENSIONS.has(ext) + return isSupportedImageExtension(ext) }) if (imageMentions.length === 0) { return { text, images: existingImages } } + const imageMemoryTracker = new ImageMemoryTracker() const newImages: string[] = [] + for (const mention of imageMentions) { if (existingImages.length + newImages.length >= MAX_IMAGES_PER_MESSAGE) { break @@ -97,15 +130,34 @@ export async function resolveImageMentions({ } const ext = path.extname(relPath).toLowerCase() - const mimeType = getMimeTypeFromExtension(ext) + const mimeType = getMimeType(ext) if (!mimeType) { continue } + // Validate image size limits (matches read_file behavior) try { + const validationResult = await validateImageForProcessing( + absPath, + supportsImages, + maxImageFileSize, + maxTotalImageSize, + imageMemoryTracker.getTotalMemoryUsed(), + ) + + if (!validationResult.isValid) { + // Skip this image due to size/memory limits, but continue processing others + continue + } + const buffer = await fs.readFile(absPath) const base64 = buffer.toString("base64") newImages.push(`data:${mimeType};base64,${base64}`) + + // Track memory usage + if (validationResult.sizeInMB) { + imageMemoryTracker.addMemoryUsage(validationResult.sizeInMB) + } } catch { // Fail-soft: skip unreadable/missing files. continue diff --git a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts index 423bab542f9..2b2b0f78b1d 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts @@ -55,6 +55,10 @@ describe("webviewMessageHandler - checkpoint operations", () => { contextProxy: { globalStorageUri: { fsPath: "/test/storage" }, }, + getState: vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }), } }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts index dfbce361e45..d1d7bba1283 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.edit.spec.ts @@ -70,6 +70,10 @@ describe("webviewMessageHandler - Edit Message with Timestamp Fallback", () => { globalStorageUri: { fsPath: "/mock/storage" }, }, log: vi.fn(), + getState: vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }), } as unknown as ClineProvider }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts index 24af44d2b9b..6908e1dd1bc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts @@ -1,5 +1,6 @@ import * as fs from "fs/promises" import * as path from "path" +import * as os from "os" // Must mock dependencies before importing the handler module. vi.mock("../../../api/providers/fetchers/modelCache") @@ -17,9 +18,46 @@ vi.mock("vscode", () => ({ }, })) +// Mock imageHelpers to avoid fs.stat issues in integration tests +vi.mock("../../tools/helpers/imageHelpers", () => ({ + SUPPORTED_IMAGE_FORMATS: [ + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".svg", + ".bmp", + ".ico", + ".tiff", + ".tif", + ".avif", + ], + IMAGE_MIME_TYPES: { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".gif": "image/gif", + ".webp": "image/webp", + ".svg": "image/svg+xml", + ".bmp": "image/bmp", + ".ico": "image/x-icon", + ".tiff": "image/tiff", + ".tif": "image/tiff", + ".avif": "image/avif", + }, + validateImageForProcessing: vi.fn().mockResolvedValue({ isValid: true, sizeInMB: 0.001 }), + ImageMemoryTracker: vi.fn().mockImplementation(() => ({ + getTotalMemoryUsed: vi.fn().mockReturnValue(0), + addMemoryUsage: vi.fn(), + })), + DEFAULT_MAX_IMAGE_FILE_SIZE_MB: 5, + DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB: 20, +})) + describe("webviewMessageHandler - image mentions (integration)", () => { it("resolves image mentions for newTask and passes images to createTask", async () => { - const tmpRoot = await fs.mkdtemp(path.join(process.cwd(), "tmp-image-mentions-")) + const tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "roo-image-mentions-")) try { const imgBytes = Buffer.from("png-bytes") await fs.writeFile(path.join(tmpRoot, "cat.png"), imgBytes) @@ -29,6 +67,10 @@ describe("webviewMessageHandler - image mentions (integration)", () => { getCurrentTask: vi.fn().mockReturnValue(undefined), createTask: vi.fn().mockResolvedValue(undefined), postMessageToWebview: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }), } as unknown as ClineProvider await webviewMessageHandler(mockProvider, { @@ -46,7 +88,7 @@ describe("webviewMessageHandler - image mentions (integration)", () => { }) it("resolves image mentions for askResponse and passes images to handleWebviewAskResponse", async () => { - const tmpRoot = await fs.mkdtemp(path.join(process.cwd(), "tmp-image-mentions-")) + const tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "roo-image-mentions-")) try { const imgBytes = Buffer.from("jpg-bytes") await fs.writeFile(path.join(tmpRoot, "cat.jpg"), imgBytes) @@ -58,6 +100,10 @@ describe("webviewMessageHandler - image mentions (integration)", () => { cwd: tmpRoot, handleWebviewAskResponse, }), + getState: vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }), } as unknown as ClineProvider await webviewMessageHandler(mockProvider, { @@ -74,4 +120,35 @@ describe("webviewMessageHandler - image mentions (integration)", () => { await fs.rm(tmpRoot, { recursive: true, force: true }) } }) + + it("resolves gif image mentions (matching read_file behavior)", async () => { + const tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "roo-image-mentions-")) + try { + const imgBytes = Buffer.from("gif-bytes") + await fs.writeFile(path.join(tmpRoot, "animation.gif"), imgBytes) + + const mockProvider = { + cwd: tmpRoot, + getCurrentTask: vi.fn().mockReturnValue(undefined), + createTask: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }), + } as unknown as ClineProvider + + await webviewMessageHandler(mockProvider, { + type: "newTask", + text: "See @/animation.gif", + images: [], + } as any) + + expect(mockProvider.createTask).toHaveBeenCalledWith("See @/animation.gif", [ + `data:image/gif;base64,${imgBytes.toString("base64")}`, + ]) + } finally { + await fs.rm(tmpRoot, { recursive: true, force: true }) + } + }) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index b113195bfdc..8e8401eb924 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -150,6 +150,10 @@ describe("webviewMessageHandler - requestLmStudioModels", () => { describe("webviewMessageHandler - image mentions", () => { beforeEach(() => { vi.clearAllMocks() + mockClineProvider.getState = vi.fn().mockResolvedValue({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }) }) it("should resolve image mentions for askResponse payloads", async () => { diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 1fdb1c6e51f..4dca1253e42 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -79,15 +79,22 @@ export const webviewMessageHandler = async ( return provider.getCurrentTask()?.cwd || provider.cwd } + /** + * Resolves image file mentions in incoming messages. + * Matches read_file behavior: respects size limits and model capabilities. + */ const resolveIncomingImages = async (payload: { text?: string; images?: string[] }) => { const text = payload.text ?? "" const images = payload.images const currentTask = provider.getCurrentTask() + const state = await provider.getState() const resolved = await resolveImageMentions({ text, images, cwd: getCurrentCwd(), rooIgnoreController: currentTask?.rooIgnoreController, + maxImageFileSize: state.maxImageFileSize, + maxTotalImageSize: state.maxTotalImageSize, }) return resolved } From f2d15548e8591040ed1b98ad3370d52b8660d7fb Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Thu, 18 Dec 2025 15:34:28 -0700 Subject: [PATCH 5/6] refactor: reuse existing imageHelpers functions per review feedback - Use isSupportedImageFormat() from imageHelpers instead of local isSupportedImageExtension() - Use readImageAsDataUrlWithBuffer() instead of manual fs.readFile + base64 conversion - Remove local getMimeType() as readImageAsDataUrlWithBuffer handles MIME type internally - Update tests to mock the new dependencies --- .../__tests__/resolveImageMentions.spec.ts | 99 +++++++++---------- src/core/mentions/resolveImageMentions.ts | 34 +------ ...eHandler.imageMentions.integration.spec.ts | 48 +++------ 3 files changed, 61 insertions(+), 120 deletions(-) diff --git a/src/core/mentions/__tests__/resolveImageMentions.spec.ts b/src/core/mentions/__tests__/resolveImageMentions.spec.ts index 9ae9fb3fdaf..cbba7005b80 100644 --- a/src/core/mentions/__tests__/resolveImageMentions.spec.ts +++ b/src/core/mentions/__tests__/resolveImageMentions.spec.ts @@ -2,44 +2,27 @@ import * as path from "path" import { resolveImageMentions } from "../resolveImageMentions" -vi.mock("fs/promises", () => { - return { - default: { - readFile: vi.fn(), - stat: vi.fn(), - }, - readFile: vi.fn(), - stat: vi.fn(), - } -}) +const SUPPORTED_IMAGE_FORMATS = [ + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".svg", + ".bmp", + ".ico", + ".tiff", + ".tif", + ".avif", +] as const vi.mock("../../tools/helpers/imageHelpers", () => ({ - SUPPORTED_IMAGE_FORMATS: [ - ".png", - ".jpg", - ".jpeg", - ".gif", - ".webp", - ".svg", - ".bmp", - ".ico", - ".tiff", - ".tif", - ".avif", - ], - IMAGE_MIME_TYPES: { - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".gif": "image/gif", - ".webp": "image/webp", - ".svg": "image/svg+xml", - ".bmp": "image/bmp", - ".ico": "image/x-icon", - ".tiff": "image/tiff", - ".tif": "image/tiff", - ".avif": "image/avif", - }, + isSupportedImageFormat: vi.fn((ext: string) => + [".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg", ".bmp", ".ico", ".tiff", ".tif", ".avif"].includes( + ext.toLowerCase(), + ), + ), + readImageAsDataUrlWithBuffer: vi.fn(), validateImageForProcessing: vi.fn(), ImageMemoryTracker: vi.fn().mockImplementation(() => ({ getTotalMemoryUsed: vi.fn().mockReturnValue(0), @@ -49,10 +32,9 @@ vi.mock("../../tools/helpers/imageHelpers", () => ({ DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB: 20, })) -import * as fs from "fs/promises" -import { validateImageForProcessing } from "../../tools/helpers/imageHelpers" +import { validateImageForProcessing, readImageAsDataUrlWithBuffer } from "../../tools/helpers/imageHelpers" -const mockReadFile = vi.mocked(fs.readFile) +const mockReadImageAsDataUrl = vi.mocked(readImageAsDataUrlWithBuffer) const mockValidateImage = vi.mocked(validateImageForProcessing) describe("resolveImageMentions", () => { @@ -63,7 +45,8 @@ describe("resolveImageMentions", () => { }) it("should append a data URL when a local png mention is present", async () => { - mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + const dataUrl = `data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("png-bytes") }) const result = await resolveImageMentions({ text: "Please look at @/assets/cat.png", @@ -72,13 +55,14 @@ describe("resolveImageMentions", () => { }) expect(mockValidateImage).toHaveBeenCalled() - expect(mockReadFile).toHaveBeenCalledWith(path.resolve("/workspace", "assets/cat.png")) + expect(mockReadImageAsDataUrl).toHaveBeenCalledWith(path.resolve("/workspace", "assets/cat.png")) expect(result.text).toBe("Please look at @/assets/cat.png") - expect(result.images).toEqual([`data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}`]) + expect(result.images).toEqual([dataUrl]) }) it("should support gif images (matching read_file)", async () => { - mockReadFile.mockResolvedValue(Buffer.from("gif-bytes")) + const dataUrl = `data:image/gif;base64,${Buffer.from("gif-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("gif-bytes") }) const result = await resolveImageMentions({ text: "See @/animation.gif", @@ -86,11 +70,12 @@ describe("resolveImageMentions", () => { cwd: "/workspace", }) - expect(result.images).toEqual([`data:image/gif;base64,${Buffer.from("gif-bytes").toString("base64")}`]) + expect(result.images).toEqual([dataUrl]) }) it("should support svg images (matching read_file)", async () => { - mockReadFile.mockResolvedValue(Buffer.from("svg-bytes")) + const dataUrl = `data:image/svg+xml;base64,${Buffer.from("svg-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("svg-bytes") }) const result = await resolveImageMentions({ text: "See @/icon.svg", @@ -98,7 +83,7 @@ describe("resolveImageMentions", () => { cwd: "/workspace", }) - expect(result.images).toEqual([`data:image/svg+xml;base64,${Buffer.from("svg-bytes").toString("base64")}`]) + expect(result.images).toEqual([dataUrl]) }) it("should ignore non-image mentions", async () => { @@ -108,12 +93,12 @@ describe("resolveImageMentions", () => { cwd: "/workspace", }) - expect(mockReadFile).not.toHaveBeenCalled() + expect(mockReadImageAsDataUrl).not.toHaveBeenCalled() expect(result.images).toEqual([]) }) it("should skip unreadable files (fail-soft)", async () => { - mockReadFile.mockRejectedValue(new Error("ENOENT")) + mockReadImageAsDataUrl.mockRejectedValue(new Error("ENOENT")) const result = await resolveImageMentions({ text: "See @/missing.webp", @@ -125,7 +110,8 @@ describe("resolveImageMentions", () => { }) it("should respect rooIgnoreController", async () => { - mockReadFile.mockResolvedValue(Buffer.from("jpg-bytes")) + const dataUrl = `data:image/jpeg;base64,${Buffer.from("jpg-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("jpg-bytes") }) const rooIgnoreController = { validateAccess: vi.fn().mockReturnValue(false), } @@ -138,12 +124,13 @@ describe("resolveImageMentions", () => { }) expect(rooIgnoreController.validateAccess).toHaveBeenCalledWith("secret.jpg") - expect(mockReadFile).not.toHaveBeenCalled() + expect(mockReadImageAsDataUrl).not.toHaveBeenCalled() expect(result.images).toEqual([]) }) it("should dedupe when mention repeats", async () => { - mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + const dataUrl = `data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("png-bytes") }) const result = await resolveImageMentions({ text: "@/a.png and again @/a.png", @@ -155,7 +142,8 @@ describe("resolveImageMentions", () => { }) it("should skip images when supportsImages is false", async () => { - mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + const dataUrl = `data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("png-bytes") }) const result = await resolveImageMentions({ text: "See @/cat.png", @@ -164,7 +152,7 @@ describe("resolveImageMentions", () => { supportsImages: false, }) - expect(mockReadFile).not.toHaveBeenCalled() + expect(mockReadImageAsDataUrl).not.toHaveBeenCalled() expect(result.images).toEqual([]) }) @@ -182,7 +170,7 @@ describe("resolveImageMentions", () => { }) expect(mockValidateImage).toHaveBeenCalled() - expect(mockReadFile).not.toHaveBeenCalled() + expect(mockReadImageAsDataUrl).not.toHaveBeenCalled() expect(result.images).toEqual([]) }) @@ -203,7 +191,8 @@ describe("resolveImageMentions", () => { }) it("should pass custom size limits to validation", async () => { - mockReadFile.mockResolvedValue(Buffer.from("png-bytes")) + const dataUrl = `data:image/png;base64,${Buffer.from("png-bytes").toString("base64")}` + mockReadImageAsDataUrl.mockResolvedValue({ dataUrl, buffer: Buffer.from("png-bytes") }) await resolveImageMentions({ text: "See @/cat.png", diff --git a/src/core/mentions/resolveImageMentions.ts b/src/core/mentions/resolveImageMentions.ts index ae93d4e8978..0a0344348f1 100644 --- a/src/core/mentions/resolveImageMentions.ts +++ b/src/core/mentions/resolveImageMentions.ts @@ -1,10 +1,9 @@ import * as path from "path" -import * as fs from "fs/promises" import { mentionRegexGlobal, unescapeSpaces } from "../../shared/context-mentions" import { - SUPPORTED_IMAGE_FORMATS, - IMAGE_MIME_TYPES, + isSupportedImageFormat, + readImageAsDataUrlWithBuffer, validateImageForProcessing, ImageMemoryTracker, DEFAULT_MAX_IMAGE_FILE_SIZE_MB, @@ -47,22 +46,6 @@ function dedupePreserveOrder(values: string[]): string[] { return result } -/** - * Checks if a file extension is a supported image format. - * Uses the same format list as read_file tool. - */ -function isSupportedImageExtension(extension: string): boolean { - return SUPPORTED_IMAGE_FORMATS.includes(extension.toLowerCase() as (typeof SUPPORTED_IMAGE_FORMATS)[number]) -} - -/** - * Gets the MIME type for an image extension. - * Uses the same mapping as read_file tool. - */ -function getMimeType(extension: string): string | undefined { - return IMAGE_MIME_TYPES[extension.toLowerCase()] -} - /** * Resolves local image file mentions like `@/path/to/image.png` found in `text` into `data:image/...;base64,...` * and appends them to the outgoing `images` array. @@ -104,7 +87,7 @@ export async function resolveImageMentions({ if (!mention.startsWith("/")) return false const relPath = unescapeSpaces(mention.slice(1)) const ext = path.extname(relPath).toLowerCase() - return isSupportedImageExtension(ext) + return isSupportedImageFormat(ext) }) if (imageMentions.length === 0) { @@ -129,12 +112,6 @@ export async function resolveImageMentions({ continue } - const ext = path.extname(relPath).toLowerCase() - const mimeType = getMimeType(ext) - if (!mimeType) { - continue - } - // Validate image size limits (matches read_file behavior) try { const validationResult = await validateImageForProcessing( @@ -150,9 +127,8 @@ export async function resolveImageMentions({ continue } - const buffer = await fs.readFile(absPath) - const base64 = buffer.toString("base64") - newImages.push(`data:${mimeType};base64,${base64}`) + const { dataUrl } = await readImageAsDataUrlWithBuffer(absPath) + newImages.push(dataUrl) // Track memory usage if (validationResult.sizeInMB) { diff --git a/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts index 6908e1dd1bc..277e56626ad 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.imageMentions.integration.spec.ts @@ -18,42 +18,18 @@ vi.mock("vscode", () => ({ }, })) -// Mock imageHelpers to avoid fs.stat issues in integration tests -vi.mock("../../tools/helpers/imageHelpers", () => ({ - SUPPORTED_IMAGE_FORMATS: [ - ".png", - ".jpg", - ".jpeg", - ".gif", - ".webp", - ".svg", - ".bmp", - ".ico", - ".tiff", - ".tif", - ".avif", - ], - IMAGE_MIME_TYPES: { - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".gif": "image/gif", - ".webp": "image/webp", - ".svg": "image/svg+xml", - ".bmp": "image/bmp", - ".ico": "image/x-icon", - ".tiff": "image/tiff", - ".tif": "image/tiff", - ".avif": "image/avif", - }, - validateImageForProcessing: vi.fn().mockResolvedValue({ isValid: true, sizeInMB: 0.001 }), - ImageMemoryTracker: vi.fn().mockImplementation(() => ({ - getTotalMemoryUsed: vi.fn().mockReturnValue(0), - addMemoryUsage: vi.fn(), - })), - DEFAULT_MAX_IMAGE_FILE_SIZE_MB: 5, - DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB: 20, -})) +// Mock imageHelpers - use actual implementations for functions that need real file access +vi.mock("../../tools/helpers/imageHelpers", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + validateImageForProcessing: vi.fn().mockResolvedValue({ isValid: true, sizeInMB: 0.001 }), + ImageMemoryTracker: vi.fn().mockImplementation(() => ({ + getTotalMemoryUsed: vi.fn().mockReturnValue(0), + addMemoryUsage: vi.fn(), + })), + } +}) describe("webviewMessageHandler - image mentions (integration)", () => { it("resolves image mentions for newTask and passes images to createTask", async () => { From 63112052fe7c3b8f9db0443b9c3f18a510ab1d52 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 7 Jan 2026 15:09:54 +0000 Subject: [PATCH 6/6] refactor: remove unused SUPPORTED_IMAGE_FORMATS constant from test file --- .../__tests__/resolveImageMentions.spec.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/core/mentions/__tests__/resolveImageMentions.spec.ts b/src/core/mentions/__tests__/resolveImageMentions.spec.ts index cbba7005b80..747c778819f 100644 --- a/src/core/mentions/__tests__/resolveImageMentions.spec.ts +++ b/src/core/mentions/__tests__/resolveImageMentions.spec.ts @@ -2,20 +2,6 @@ import * as path from "path" import { resolveImageMentions } from "../resolveImageMentions" -const SUPPORTED_IMAGE_FORMATS = [ - ".png", - ".jpg", - ".jpeg", - ".gif", - ".webp", - ".svg", - ".bmp", - ".ico", - ".tiff", - ".tif", - ".avif", -] as const - vi.mock("../../tools/helpers/imageHelpers", () => ({ isSupportedImageFormat: vi.fn((ext: string) => [".png", ".jpg", ".jpeg", ".gif", ".webp", ".svg", ".bmp", ".ico", ".tiff", ".tif", ".avif"].includes(