From d42194459852260f98394e03ac9bc6d9197d82b9 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 16 Aug 2026 00:52:19 +0000 Subject: [PATCH] fix(mcp): preserved image tool results - Forwarded MCP image blocks to the agent and TUI without copying their base64 payloads. - Retained existing text and resource formatting around image blocks. - Added regression coverage for mixed text and image tool results. Fixes #8687 --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/mcp/tool-bridge.ts | 48 ++++++++++++------- .../coding-agent/test/mcp-reconnect.test.ts | 19 +++++++- 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f61f4fc3d..bff63cbba 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,10 @@ - Kept automatic model selection on paid `xai/grok-4.5` when only `XAI_API_KEY` is set, instead of preferring SuperGrok `xai-oauth/grok-4.5`. Explicit `xai-oauth/grok-4.5` still works with that paid key. - Stopped sending presence/frequency penalties and stop sequences to xAI reasoning models such as `grok-4.5`, which reject them. +### Fixed + +- Preserved MCP `ImageContent` tool-result blocks so vision-capable models and the TUI can inspect returned images instead of receiving only a text placeholder ([#8687](https://github.com/can1357/oh-my-pi/issues/8687)). + ## [17.3.4] - 2026-08-14 ### Changed diff --git a/packages/coding-agent/src/mcp/tool-bridge.ts b/packages/coding-agent/src/mcp/tool-bridge.ts index b7ac90a47..c5e14f9a4 100644 --- a/packages/coding-agent/src/mcp/tool-bridge.ts +++ b/packages/coding-agent/src/mcp/tool-bridge.ts @@ -4,7 +4,7 @@ * Converts MCP tool definitions to CustomTool format for the agent. */ import type { AgentToolUpdateCallback } from "@oh-my-pi/pi-agent-core"; -import type { TSchema } from "@oh-my-pi/pi-ai"; +import type { ImageContent, TextContent, TSchema } from "@oh-my-pi/pi-ai"; import { normalizeSchemaForMCP } from "@oh-my-pi/pi-ai/utils/schema"; import { logger, untilAborted } from "@oh-my-pi/pi-utils"; import { INTENT_FIELD } from "@oh-my-pi/pi-wire"; @@ -191,30 +191,40 @@ export interface MCPToolDetails { meta?: OutputMeta; } /** - * Format MCP content for LLM consumption. + * Convert MCP content to agent content while retaining image payloads. */ -function formatMCPContent(content: MCPContent[]): string { - const parts: string[] = []; +function formatMCPContent(content: MCPContent[]): Array { + const blocks: Array = []; + let text = ""; + const flushText = () => { + if (!text) return; + blocks.push({ type: "text", text }); + text = ""; + }; + const appendText = (value: string) => { + text += text ? `\n\n${value}` : value; + }; for (const item of content) { switch (item.type) { case "text": - parts.push(item.text); + appendText(item.text); break; case "image": - parts.push(`[Image: ${item.mimeType}]`); + flushText(); + blocks.push(item); break; case "resource": - if (item.resource.text) { - parts.push(`[Resource: ${item.resource.uri}]\n${item.resource.text}`); - } else { - parts.push(`[Resource: ${item.resource.uri}]`); - } + appendText( + item.resource.text + ? `[Resource: ${item.resource.uri}]\n${item.resource.text}` + : `[Resource: ${item.resource.uri}]`, + ); break; } } - - return parts.join("\n\n"); + flushText(); + return blocks.length > 0 ? blocks : [{ type: "text", text: "" }]; } /** Build a CustomToolResult from a callTool response. */ @@ -225,7 +235,7 @@ function buildResult( provider?: string, providerName?: string, ): CustomToolResult { - const text = formatMCPContent(result.content); + const content = formatMCPContent(result.content); const details: MCPToolDetails = { serverName, mcpToolName, @@ -235,8 +245,14 @@ function buildResult( provider, providerName, }; - const contentText = result.isError ? `Error: ${text}` : text; - const toolResult: CustomToolResult = { content: [{ type: "text", text: contentText }], details }; + if (result.isError) { + if (content[0]?.type === "text") { + content[0] = { type: "text", text: `Error: ${content[0].text}` }; + } else { + content.unshift({ type: "text", text: "Error:" }); + } + } + const toolResult: CustomToolResult = { content, details }; if (result.isError) { toolResult.isError = true; } diff --git a/packages/coding-agent/test/mcp-reconnect.test.ts b/packages/coding-agent/test/mcp-reconnect.test.ts index 4576ffffc..680971f82 100644 --- a/packages/coding-agent/test/mcp-reconnect.test.ts +++ b/packages/coding-agent/test/mcp-reconnect.test.ts @@ -6,7 +6,12 @@ import { isRetriableConnectionError, MCPTool, } from "@oh-my-pi/pi-coding-agent/mcp/tool-bridge"; -import type { MCPServerConnection, MCPToolCallResult, MCPTransport } from "@oh-my-pi/pi-coding-agent/mcp/types"; +import type { + MCPImageContent, + MCPServerConnection, + MCPToolCallResult, + MCPTransport, +} from "@oh-my-pi/pi-coding-agent/mcp/types"; import { ToolAbortError } from "@oh-my-pi/pi-coding-agent/tools/tool-errors"; import { logger } from "@oh-my-pi/pi-utils"; @@ -146,6 +151,18 @@ describe("MCPTool.execute retry on connection error", () => { expect(result.content[0]).toEqual({ type: "text", text: "ok" }); }); + it("preserves image blocks returned by MCP tools", async () => { + const image: MCPImageContent = { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }; + const transport = mockTransport(async () => ({ + content: [{ type: "text", text: "Screenshot captured" }, image], + })); + const tool = new MCPTool(makeConnection(transport), TOOL_DEF); + + const result = await tool.execute("call-1", {}, noop, noCtx); + + expect(result.content).toEqual([{ type: "text", text: "Screenshot captured" }, image]); + }); + it("retries on transport closed and rebinding succeeds", async () => { let oldCalls = 0; let newCalls = 0;