fix: MCP image preview thumbnails and save_image tool functionality

- 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
This commit is contained in:
Roo Code 2026-01-22 09:22:53 +00:00
parent 7f1cab989f
commit 953f037c7f
4 changed files with 70 additions and 41 deletions

View file

@ -319,9 +319,19 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> {
response: outputText || (images.length > 0 ? `[${images.length} image(s)]` : ""),
})
toolResultPretty =
(toolResult.isError ? "Error:\n" : "") +
(outputText || (images.length > 0 ? `[${images.length} image(s) received]` : ""))
// Build the result text
let resultText = outputText || ""
// Include image data URLs in the text response so the agent can use them with save_image tool
if (images.length > 0) {
const imageDataSection = images
.map((img, index) => `<image_${index + 1}>\n${img}\n</image_${index + 1}>`)
.join("\n\n")
const imageInfo = `\n\n[${images.length} image(s) received - data URLs provided below for use with save_image tool]\n\n${imageDataSection}`
resultText = resultText ? resultText + imageInfo : imageInfo.trim()
}
toolResultPretty = (toolResult.isError ? "Error:\n" : "") + resultText
}
// Send completion status

View file

@ -618,14 +618,12 @@ describe("useMcpToolTool", () => {
mockProviderRef.deref.mockReturnValue({
getMcpHub: () => ({
callTool: vi.fn().mockResolvedValue(mockToolResult),
getAllServers: vi
.fn()
.mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshot", description: "Get screenshot" }],
},
]),
getAllServers: vi.fn().mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshot", description: "Get screenshot" }],
},
]),
}),
postMessageToWebview: vi.fn(),
})
@ -637,9 +635,13 @@ describe("useMcpToolTool", () => {
})
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_request_started")
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "[1 image(s) received]", [
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJ",
])
expect(mockTask.say).toHaveBeenCalledWith(
"mcp_server_response",
expect.stringContaining(
"[1 image(s) received - data URLs provided below for use with save_image tool]",
),
["data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJ"],
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("with 1 image(s)"))
})
@ -693,9 +695,11 @@ describe("useMcpToolTool", () => {
})
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_request_started")
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "Node name: Button", [
"data:image/png;base64,base64imagedata",
])
expect(mockTask.say).toHaveBeenCalledWith(
"mcp_server_response",
expect.stringContaining("Node name: Button"),
["data:image/png;base64,base64imagedata"],
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("with 1 image(s)"))
})
@ -732,14 +736,12 @@ describe("useMcpToolTool", () => {
mockProviderRef.deref.mockReturnValue({
getMcpHub: () => ({
callTool: vi.fn().mockResolvedValue(mockToolResult),
getAllServers: vi
.fn()
.mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshot", description: "Get screenshot" }],
},
]),
getAllServers: vi.fn().mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshot", description: "Get screenshot" }],
},
]),
}),
postMessageToWebview: vi.fn(),
})
@ -751,9 +753,13 @@ describe("useMcpToolTool", () => {
})
// Should not double-prefix the data URL
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "[1 image(s) received]", [
"data:image/jpeg;base64,/9j/4AAQSkZJRg==",
])
expect(mockTask.say).toHaveBeenCalledWith(
"mcp_server_response",
expect.stringContaining(
"[1 image(s) received - data URLs provided below for use with save_image tool]",
),
["data:image/jpeg;base64,/9j/4AAQSkZJRg=="],
)
})
it("should handle multiple images in response", async () => {
@ -794,14 +800,12 @@ describe("useMcpToolTool", () => {
mockProviderRef.deref.mockReturnValue({
getMcpHub: () => ({
callTool: vi.fn().mockResolvedValue(mockToolResult),
getAllServers: vi
.fn()
.mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshots", description: "Get screenshots" }],
},
]),
getAllServers: vi.fn().mockReturnValue([
{
name: "figma-server",
tools: [{ name: "get_screenshots", description: "Get screenshots" }],
},
]),
}),
postMessageToWebview: vi.fn(),
})
@ -812,10 +816,13 @@ describe("useMcpToolTool", () => {
pushToolResult: mockPushToolResult,
})
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "[2 image(s) received]", [
"data:image/png;base64,image1data",
"data:image/png;base64,image2data",
])
expect(mockTask.say).toHaveBeenCalledWith(
"mcp_server_response",
expect.stringContaining(
"[2 image(s) received - data URLs provided below for use with save_image tool]",
),
["data:image/png;base64,image1data", "data:image/png;base64,image2data"],
)
expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("with 2 image(s)"))
})
})

View file

@ -119,6 +119,7 @@ interface ChatRowProps {
onFollowUpUnmount?: () => void
isFollowUpAnswered?: boolean
isFollowUpAutoApprovalPaused?: boolean
mcpResponseImages?: string[]
editable?: boolean
hasCheckpoint?: boolean
}
@ -173,6 +174,7 @@ export const ChatRowContent = ({
onBatchFileResponse,
isFollowUpAnswered,
isFollowUpAutoApprovalPaused,
mcpResponseImages,
}: ChatRowContentProps) => {
const { t, i18n } = useTranslation()
@ -1627,7 +1629,7 @@ export const ChatRowContent = ({
server={server}
useMcpServer={useMcpServer}
alwaysAllowMcp={alwaysAllowMcp}
images={message.images}
images={mcpResponseImages ?? message.images}
/>
)}
</div>

View file

@ -1327,6 +1327,15 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
return <BrowserSessionStatusRow key={messageOrGroup.ts} message={messageOrGroup} />
}
// For use_mcp_server ask messages, find the corresponding mcp_server_response to get images
let mcpResponseImages: string[] | undefined
if (messageOrGroup.type === "ask" && messageOrGroup.ask === "use_mcp_server") {
const mcpResponse = modifiedMessages.find(
(m) => m.ts > messageOrGroup.ts && m.say === "mcp_server_response",
)
mcpResponseImages = mcpResponse?.images
}
// regular message
return (
<ChatRow
@ -1342,6 +1351,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
onBatchFileResponse={handleBatchFileResponse}
isFollowUpAnswered={messageOrGroup.isAnswered === true || messageOrGroup.ts === currentFollowUpTs}
isFollowUpAutoApprovalPaused={isFollowUpAutoApprovalPaused}
mcpResponseImages={mcpResponseImages}
editable={
messageOrGroup.type === "ask" &&
messageOrGroup.ask === "tool" &&