Merge PR #8688: fix(mcp): preserve image tool results (@roboomp)
This commit is contained in:
@@ -94,6 +94,10 @@
|
||||
- Fixed external thinking being offered on xAI reasoning-only Responses models (grok-4 family) that reject `reasoning.effort`, where the private scratchpad ran alongside native reasoning instead of replacing it.
|
||||
- Fixed the extension tool-call handler timeout rendering outside a titled section in `/settings` by registering its Extensions group on the Tools tab.
|
||||
|
||||
### 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
|
||||
|
||||
@@ -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<TextContent | ImageContent> {
|
||||
const blocks: Array<TextContent | ImageContent> = [];
|
||||
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<MCPToolDetails> {
|
||||
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<MCPToolDetails> = { 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<MCPToolDetails> = { content, details };
|
||||
if (result.isError) {
|
||||
toolResult.isError = true;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user