Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 193 additions & 0 deletions src/core/mentions/__tests__/resolveImageMentions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
import * as path from "path"

import { resolveImageMentions } from "../resolveImageMentions"

vi.mock("../../tools/helpers/imageHelpers", () => ({
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),
addMemoryUsage: vi.fn(),
})),
DEFAULT_MAX_IMAGE_FILE_SIZE_MB: 5,
DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB: 20,
}))

import { validateImageForProcessing, readImageAsDataUrlWithBuffer } from "../../tools/helpers/imageHelpers"

const mockReadImageAsDataUrl = vi.mocked(readImageAsDataUrlWithBuffer)
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 () => {
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",
images: [],
cwd: "/workspace",
})

expect(mockValidateImage).toHaveBeenCalled()
expect(mockReadImageAsDataUrl).toHaveBeenCalledWith(path.resolve("/workspace", "assets/cat.png"))
expect(result.text).toBe("Please look at @/assets/cat.png")
expect(result.images).toEqual([dataUrl])
})

it("should support gif images (matching read_file)", async () => {
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",
images: [],
cwd: "/workspace",
})

expect(result.images).toEqual([dataUrl])
})

it("should support svg images (matching read_file)", async () => {
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",
images: [],
cwd: "/workspace",
})

expect(result.images).toEqual([dataUrl])
})

it("should ignore non-image mentions", async () => {
const result = await resolveImageMentions({
text: "See @/src/index.ts",
images: [],
cwd: "/workspace",
})

expect(mockReadImageAsDataUrl).not.toHaveBeenCalled()
expect(result.images).toEqual([])
})

it("should skip unreadable files (fail-soft)", async () => {
mockReadImageAsDataUrl.mockRejectedValue(new Error("ENOENT"))

const result = await resolveImageMentions({
text: "See @/missing.webp",
images: [],
cwd: "/workspace",
})

expect(result.images).toEqual([])
})

it("should respect rooIgnoreController", async () => {
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),
}

const result = await resolveImageMentions({
text: "See @/secret.jpg",
images: [],
cwd: "/workspace",
rooIgnoreController,
})

expect(rooIgnoreController.validateAccess).toHaveBeenCalledWith("secret.jpg")
expect(mockReadImageAsDataUrl).not.toHaveBeenCalled()
expect(result.images).toEqual([])
})

it("should dedupe when mention repeats", async () => {
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",
images: [],
cwd: "/workspace",
})

expect(result.images).toHaveLength(1)
})

it("should skip images when supportsImages is false", async () => {
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",
images: [],
cwd: "/workspace",
supportsImages: false,
})

expect(mockReadImageAsDataUrl).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(mockReadImageAsDataUrl).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 () => {
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",
images: [],
cwd: "/workspace",
maxImageFileSize: 10,
maxTotalImageSize: 50,
})

expect(mockValidateImage).toHaveBeenCalledWith(expect.any(String), true, 10, 50, 0)
})
})
8 changes: 7 additions & 1 deletion src/core/mentions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
145 changes: 145 additions & 0 deletions src/core/mentions/resolveImageMentions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
import * as path from "path"

import { mentionRegexGlobal, unescapeSpaces } from "../../shared/context-mentions"
import {
isSupportedImageFormat,
readImageAsDataUrlWithBuffer,
validateImageForProcessing,
ImageMemoryTracker,
DEFAULT_MAX_IMAGE_FILE_SIZE_MB,
DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB,
} from "../tools/helpers/imageHelpers"

const MAX_IMAGES_PER_MESSAGE = 20

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 {
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<string>()
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.
*
* 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supportsImages defaults to true here, so if a caller forgets to pass the real model capability this will still read and base64-encode image mentions for non-vision models (extra memory + large payloads). Consider deriving/passing supportsImages from the active model info (or defaulting this to false so callers must opt-in explicitly).

Fix it with Roo Code or mention @roomote and request a fix.

maxImageFileSize = DEFAULT_MAX_IMAGE_FILE_SIZE_MB,
maxTotalImageSize = DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB,
}: ResolveImageMentionsOptions): Promise<ResolveImageMentionsResult> {
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)
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 isSupportedImageFormat(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
}

const relPath = unescapeSpaces(mention.slice(1))
const absPath = path.resolve(cwd, relPath)
if (!isPathWithinCwd(absPath, cwd)) {
continue
}

if (rooIgnoreController && !rooIgnoreController.validateAccess(relPath)) {
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 { dataUrl } = await readImageAsDataUrlWithBuffer(absPath)
newImages.push(dataUrl)

// Track memory usage
if (validationResult.sizeInMB) {
imageMemoryTracker.addMemoryUsage(validationResult.sizeInMB)
}
} catch {
// Fail-soft: skip unreadable/missing files.
continue
}
}

const merged = dedupePreserveOrder([...existingImages, ...newImages]).slice(0, MAX_IMAGES_PER_MESSAGE)
return { text, images: merged }
}
6 changes: 3 additions & 3 deletions src/core/webview/__tests__/ClineProvider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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", [])
})
})

Expand Down Expand Up @@ -3632,7 +3632,7 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => {
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 () => {
Expand Down
Loading
Loading