Skip to content

Commit 337ef77

Browse files
committed
Improve MCP image handling with size validation and UI enhancements
- Add early size check to prevent memory spikes from oversized images - Implement bounds checking for MCP settings (images per response, max size) - Add image count tooltips and improved accessibility in UI - Update translations for better user experience across EN/ES/FR - Refactor base64 validation with reusable regex constant
1 parent 51f2d82 commit 337ef77

File tree

10 files changed

+40
-12
lines changed

10 files changed

+40
-12
lines changed

src/core/tools/__tests__/useMcpToolTool.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -743,9 +743,9 @@ describe("useMcpToolTool", () => {
743743
expect(sayCall[2]).toHaveLength(1)
744744
expect(sayCall[2][0]).toContain(smallBase64)
745745

746-
// Should log warning about size exceeding limit
746+
// Should log warning about size exceeding limit (either early check or full validation)
747747
expect(consoleSpy).toHaveBeenCalledWith(
748-
expect.stringMatching(/MCP image exceeds size limit: .* > 1MB\. Image will be ignored\./),
748+
expect.stringMatching(/MCP image (likely exceeds size limit|exceeds size limit)/),
749749
)
750750

751751
expect(mockPushToolResult).toHaveBeenCalledWith(

src/core/tools/useMcpToolTool.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { McpExecutionStatus } from "@roo-code/types"
66
import { t } from "../../i18n"
77

88
const SUPPORTED_IMAGE_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"]
9+
const BASE64_REGEX = /^[A-Za-z0-9+/]*={0,2}$/
910

1011
interface McpToolParams {
1112
server_name?: string
@@ -216,8 +217,8 @@ async function processToolContent(toolResult: any, cline: Task): Promise<{ text:
216217

217218
// Get MCP settings from the extension's global state
218219
const state = await cline.providerRef.deref()?.getState()
219-
const maxImagesPerResponse = state?.mcpMaxImagesPerResponse ?? 20
220-
const maxImageSizeMB = state?.mcpMaxImageSizeMB ?? 10
220+
const maxImagesPerResponse = Math.max(1, Math.min(100, state?.mcpMaxImagesPerResponse ?? 20))
221+
const maxImageSizeMB = Math.max(0.1, Math.min(50, state?.mcpMaxImageSizeMB ?? 10))
221222

222223
toolResult.content.forEach((item: any) => {
223224
if (item.type === "text") {
@@ -240,9 +241,17 @@ async function processToolContent(toolResult: any, cline: Task): Promise<{ text:
240241
return
241242
}
242243

244+
// Quick size check before full validation to prevent memory spikes
245+
const approximateSizeMB = (item.data.length * 0.75) / (1024 * 1024)
246+
if (approximateSizeMB > maxImageSizeMB * 1.5) {
247+
console.warn(
248+
`MCP image likely exceeds size limit based on string length: ~${approximateSizeMB.toFixed(2)}MB`,
249+
)
250+
return
251+
}
252+
243253
// Basic validation for base64 format
244-
const base64Regex = /^[A-Za-z0-9+/]*={0,2}$/
245-
if (!base64Regex.test(item.data.replace(/\s/g, ""))) {
254+
if (!BASE64_REGEX.test(item.data.replace(/\s/g, ""))) {
246255
console.warn("Invalid MCP ImageContent: base64 data contains invalid characters")
247256
return
248257
}

webview-ui/src/components/chat/McpExecution.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ export const McpExecution = ({
218218
{(responseText && responseText.length > 0) || (images && images.length > 0) ? (
219219
<div className="flex items-center gap-1">
220220
{images && images.length > 0 && (
221-
<div className="flex items-center gap-1 text-xs text-vscode-descriptionForeground">
221+
<div
222+
className="flex items-center gap-1 text-xs text-vscode-descriptionForeground"
223+
title={t("execution.imageCountTooltip", { count: images.length })}>
222224
<span className="codicon codicon-file-media" />
223225
<span>{images.length}</span>
224226
</div>

webview-ui/src/components/common/Thumbnails.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React, { useState, useRef, useLayoutEffect, memo } from "react"
22
import { useWindowSize } from "react-use"
3+
import { useTranslation } from "react-i18next"
34
import { vscode } from "@src/utils/vscode"
45

56
interface ThumbnailsProps {
@@ -10,6 +11,7 @@ interface ThumbnailsProps {
1011
}
1112

1213
const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProps) => {
14+
const { t } = useTranslation("common")
1315
const [hoveredIndex, setHoveredIndex] = useState<number | null>(null)
1416
const [failedImages, setFailedImages] = useState<Set<number>>(new Set())
1517
const containerRef = useRef<HTMLDivElement>(null)
@@ -75,7 +77,7 @@ const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProp
7577
cursor: "pointer",
7678
}}
7779
onClick={() => handleImageClick(image)}
78-
title="Failed to load image">
80+
title={t("thumbnails.failedToLoad")}>
7981
<span
8082
className="codicon codicon-file-media"
8183
style={{
@@ -86,7 +88,7 @@ const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProp
8688
) : (
8789
<img
8890
src={image}
89-
alt={`Thumbnail ${index + 1}`}
91+
alt={t("thumbnails.altText", { index: index + 1 })}
9092
style={{
9193
width: 34,
9294
height: 34,

webview-ui/src/i18n/locales/en/common.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,5 +95,9 @@
9595
"months_ago": "{{count}} months ago",
9696
"year_ago": "a year ago",
9797
"years_ago": "{{count}} years ago"
98+
},
99+
"thumbnails": {
100+
"failedToLoad": "Failed to load image",
101+
"altText": "Thumbnail {{index}}"
98102
}
99103
}

webview-ui/src/i18n/locales/en/mcp.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@
6060
"execution": {
6161
"running": "Running",
6262
"completed": "Completed",
63-
"error": "Error"
63+
"error": "Error",
64+
"imageCountTooltip": "{{count}} image(s) in response"
6465
},
6566
"imageSettings": {
6667
"maxImagesLabel": "Max Images",

webview-ui/src/i18n/locales/es/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/es/mcp.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/fr/common.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/fr/mcp.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)