diff --git a/docs/api_to_audit.md b/docs/api_to_audit.md index 6ed96e2c40..6e97a4325b 100644 --- a/docs/api_to_audit.md +++ b/docs/api_to_audit.md @@ -5,6 +5,18 @@ entry here (see [AGENTS.md](../AGENTS.md), "Plugin API"). Dropping the prefix is the deliberate stabilization step: audit the entry, rename project-wide, and delete the entry in the same change. +## `experimental_buildBridgeToolCallContent` + +**What it does.** Converts a decoded bb tool-call response into the ordered +text and inline-image content blocks accepted by MCP and Pi tool result +contracts. It preserves a legacy aggregate text/images input while first-party +bridges migrate to ordered `contentBlocks`. + +**Audit before stabilizing.** Confirm that MCP and Pi continue sharing this +content-block vocabulary; decide whether legacy aggregate fields still need to +be accepted; and define any image MIME validation, decoding, or payload-size +policy at the server boundary before making the helper stable. + ## Host plugin foundation (`bb.hosts.experimental_client`, `ExperimentalHostClient.experimental_onWorkerExit`, `ExperimentalHostClient.experimental_onSignal`, `ExperimentalHostRpcContext.experimental_retainWorker`, `experimental_defineHostEntry`, and `experimental_createHostEntryHarness`) **What it does.** Lets one plugin package declare a singular `bb.host` Node diff --git a/packages/agent-runtime/src/pi/bridge/__tests__/tool-proxy.test.ts b/packages/agent-runtime/src/pi/bridge/__tests__/tool-proxy.test.ts index 9071226dc3..7768c55538 100644 --- a/packages/agent-runtime/src/pi/bridge/__tests__/tool-proxy.test.ts +++ b/packages/agent-runtime/src/pi/bridge/__tests__/tool-proxy.test.ts @@ -2,6 +2,53 @@ import { describe, expect, it } from "vitest"; import { buildDynamicTools } from "../tool-proxy.js"; describe("tool-proxy", () => { + it("returns ordered image and text content to Pi", async () => { + const [tool] = buildDynamicTools( + [ + { + name: "browser_screenshot", + description: "Capture the browser.", + inputSchema: { type: "object" }, + }, + ], + async () => ({ + content: "after", + contentBlocks: [ + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + { type: "text", text: "after" }, + ], + images: [{ data: "iVBORw0KGgo=", mimeType: "image/png" }], + }), + ); + + const result = await Reflect.apply(tool.execute, tool, [ + "call-1", + {}, + undefined, + ]); + expect(result.content).toEqual([ + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + { type: "text", text: "after" }, + ]); + }); + + it("throws forwarded failures so Pi marks the tool result as an error", async () => { + const [tool] = buildDynamicTools( + [ + { + name: "broken_tool", + description: "Fail.", + inputSchema: { type: "object" }, + }, + ], + async () => ({ content: "permission denied", isError: true }), + ); + + await expect( + Reflect.apply(tool.execute, tool, ["call-1", {}, undefined]), + ).rejects.toThrow("permission denied"); + }); + it("preserves required and optional scalar fields", () => { const [tool] = buildDynamicTools( [ diff --git a/packages/agent-runtime/src/pi/bridge/tool-proxy.ts b/packages/agent-runtime/src/pi/bridge/tool-proxy.ts index 4ccacef050..573bb58642 100644 --- a/packages/agent-runtime/src/pi/bridge/tool-proxy.ts +++ b/packages/agent-runtime/src/pi/bridge/tool-proxy.ts @@ -1,5 +1,10 @@ import { Type, type TSchema } from "@earendil-works/pi-ai"; import type { ToolDefinition } from "@earendil-works/pi-coding-agent"; +import { + buildBridgeToolCallContent, + type BridgeToolCallContent, + type BridgeToolCallImage, +} from "@bb/provider-bridge-protocol/bridge-kit"; export interface DynamicToolDefinition { name: string; @@ -10,7 +15,12 @@ export interface DynamicToolDefinition { export type ToolCallForwarder = ( toolName: string, args: Record, -) => Promise<{ content: string; isError?: boolean }>; +) => Promise<{ + content: string; + contentBlocks?: BridgeToolCallContent[]; + images?: BridgeToolCallImage[]; + isError?: boolean; +}>; /** * Builds Pi-compatible ToolDefinition objects from dynamic tool definitions @@ -33,10 +43,14 @@ export function buildDynamicTools( _signal: AbortSignal | undefined, ) { const result = await forwardToolCall(def.name, params); + if (result.isError) { + // Pi marks a tool result as failed only when execute throws; an + // extra isError property on AgentToolResult is ignored. + throw new Error(result.content || "Tool call failed"); + } return { - content: [{ type: "text" as const, text: result.content }], + content: buildBridgeToolCallContent(result), details: {}, - ...(result.isError ? { isError: true } : {}), }; }, } as ToolDefinition; diff --git a/packages/plugin-sdk/src/provider-bridge.ts b/packages/plugin-sdk/src/provider-bridge.ts index a84ed17784..b39c33b7c7 100644 --- a/packages/plugin-sdk/src/provider-bridge.ts +++ b/packages/plugin-sdk/src/provider-bridge.ts @@ -112,6 +112,7 @@ export type { export { bashArgsSchema, bridgeRequestEnvelopeSchema, + buildBridgeToolCallContent as experimental_buildBridgeToolCallContent, buildShellEnvOverrides, createBridgeIo, createBridgeLineHandler, diff --git a/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.test.ts b/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.test.ts new file mode 100644 index 0000000000..5598065702 --- /dev/null +++ b/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.test.ts @@ -0,0 +1,223 @@ +import { describe, expect, it } from "vitest"; +import { + buildBridgeToolCallContent, + decodeToolCallResponsePayload, +} from "./bridge-tool-calls.js"; + +const PNG = "iVBORw0KGgo="; + +describe("decodeToolCallResponsePayload", () => { + it("keeps text results unchanged", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputText", text: "first" }, + { type: "inputText", text: "second" }, + ], + }), + ).toEqual({ + content: "first\nsecond", + contentBlocks: [ + { type: "text", text: "first" }, + { type: "text", text: "second" }, + ], + images: [], + isError: false, + }); + }); + + // The bug: an image-only result decoded to the literal "OK" with the image + // dropped, so browser_screenshot reported success and returned nothing. + it("decodes an image-only result into an image rather than OK", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputImage", imageUrl: `data:image/png;base64,${PNG}` }, + ], + }), + ).toEqual({ + content: "", + contentBlocks: [{ type: "image", data: PNG, mimeType: "image/png" }], + images: [{ data: PNG, mimeType: "image/png" }], + isError: false, + }); + }); + + it("keeps both halves of a mixed text and image result", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputText", text: "captured" }, + { type: "inputImage", imageUrl: `data:image/jpeg;base64,${PNG}` }, + ], + }), + ).toEqual({ + content: "captured", + contentBlocks: [ + { type: "text", text: "captured" }, + { type: "image", data: PNG, mimeType: "image/jpeg" }, + ], + images: [{ data: PNG, mimeType: "image/jpeg" }], + isError: false, + }); + }); + + it("preserves interleaved text and image order", () => { + const decoded = decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputImage", imageUrl: `data:image/png;base64,${PNG}` }, + { type: "inputText", text: "between" }, + { type: "inputImage", imageUrl: `data:image/jpeg;base64,${PNG}` }, + ], + }); + + expect(buildBridgeToolCallContent(decoded)).toEqual([ + { type: "image", data: PNG, mimeType: "image/png" }, + { type: "text", text: "between" }, + { type: "image", data: PNG, mimeType: "image/jpeg" }, + ]); + }); + + it("reports an image result that failed as an error", () => { + expect( + decodeToolCallResponsePayload({ + success: false, + contentItems: [ + { type: "inputImage", imageUrl: `data:image/png;base64,${PNG}` }, + ], + }).isError, + ).toBe(true); + }); + + // A tool result contract carries inline base64 only, so a remote reference + // has to survive as text; dropping it is what this fix exists to stop. + it("keeps a non-data image url as text", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputImage", imageUrl: "https://example.com/a.png" }, + ], + }), + ).toEqual({ + content: "https://example.com/a.png", + contentBlocks: [{ type: "text", text: "https://example.com/a.png" }], + images: [], + isError: false, + }); + }); + + it("keeps a data url with an empty payload as text", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { type: "inputImage", imageUrl: "data:image/png;base64," }, + ], + }), + ).toEqual({ + content: "data:image/png;base64,", + contentBlocks: [{ type: "text", text: "data:image/png;base64," }], + images: [], + isError: false, + }); + }); + + it("falls back to OK only when there is neither text nor image", () => { + expect( + decodeToolCallResponsePayload({ success: true, contentItems: [] }), + ).toEqual({ + content: "OK", + contentBlocks: [{ type: "text", text: "OK" }], + images: [], + isError: false, + }); + }); + + it("surfaces empty failures and malformed payloads as errors", () => { + expect( + decodeToolCallResponsePayload({ success: false, contentItems: [] }), + ).toEqual({ + content: "Tool call failed", + contentBlocks: [{ type: "text", text: "Tool call failed" }], + images: [], + isError: true, + }); + expect(decodeToolCallResponsePayload({ nope: true })).toEqual({ + content: "Invalid tool call response", + contentBlocks: [{ type: "text", text: "Invalid tool call response" }], + images: [], + isError: true, + }); + }); + + it("accepts MIME parameters in an inline image data URL", () => { + expect( + decodeToolCallResponsePayload({ + success: true, + contentItems: [ + { + type: "inputImage", + imageUrl: `data:image/svg+xml;charset=utf-8;base64,${PNG}`, + }, + ], + }).images, + ).toEqual([{ data: PNG, mimeType: "image/svg+xml;charset=utf-8" }]); + }); +}); + +describe("buildBridgeToolCallContent", () => { + it("emits an image block alone when there is no text", () => { + expect( + buildBridgeToolCallContent({ + content: "", + images: [{ data: PNG, mimeType: "image/png" }], + }), + ).toEqual([{ type: "image", data: PNG, mimeType: "image/png" }]); + }); + + it("keeps text first when a result carries both", () => { + expect( + buildBridgeToolCallContent({ + content: "captured", + images: [{ data: PNG, mimeType: "image/png" }], + }), + ).toEqual([ + { type: "text", text: "captured" }, + { type: "image", data: PNG, mimeType: "image/png" }, + ]); + }); + + it("prefers ordered content blocks when present", () => { + expect( + buildBridgeToolCallContent({ + content: "legacy text", + contentBlocks: [ + { type: "image", data: PNG, mimeType: "image/png" }, + { type: "text", text: "after" }, + ], + images: [{ data: PNG, mimeType: "image/png" }], + }), + ).toEqual([ + { type: "image", data: PNG, mimeType: "image/png" }, + { type: "text", text: "after" }, + ]); + }); + + it("emits a lone text block for a text result", () => { + expect(buildBridgeToolCallContent({ content: "OK", images: [] })).toEqual([ + { type: "text", text: "OK" }, + ]); + }); + + // The pending-call failure paths resolve without an images key. + it("tolerates a result with no images key", () => { + expect(buildBridgeToolCallContent({ content: "transport closed" })).toEqual( + [{ type: "text", text: "transport closed" }], + ); + }); +}); diff --git a/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.ts b/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.ts index d31048cce6..63d8823a0d 100644 --- a/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.ts +++ b/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.ts @@ -117,22 +117,118 @@ export function decodeBridgeJsonRpcResponse( // Tool call response payload decoding // --------------------------------------------------------------------------- +/** An image on a tool call result, split out of an `inputImage` data URL. */ +export interface BridgeToolCallImage { + data: string; + mimeType: string; +} + +/** + * A tool result block in the one shape every consumer already accepts: MCP's + * `CallToolResult.content` (claude-code and acp) and pi's `AgentToolResult.content` + * declare the same two members with the same field names. + */ +export type BridgeToolCallContent = + | { type: "text"; text: string } + | { type: "image"; data: string; mimeType: string }; + +const IMAGE_DATA_URL = /^data:(.+);base64,(.+)$/s; + +/** + * Splits `data:;base64,` into the parts a tool result carries. + * Returns null for any other URL: both result contracts carry inline base64 and + * have nowhere to put a remote reference, so the caller keeps such a URL as text + * rather than dropping it. + */ +function decodeImageDataUrl(imageUrl: string): BridgeToolCallImage | null { + const match = IMAGE_DATA_URL.exec(imageUrl); + if (match === null) { + return null; + } + const [, mimeType, data] = match; + if (data.length === 0) { + return null; + } + return { data, mimeType }; +} + export function decodeToolCallResponsePayload(result: unknown): { content: string; + contentBlocks: BridgeToolCallContent[]; + images: BridgeToolCallImage[]; isError: boolean; } { const parsed = providerToolCallResponseSchema.safeParse(result); if (!parsed.success) { - return { content: "OK", isError: false }; + return { + content: "Invalid tool call response", + contentBlocks: [{ type: "text", text: "Invalid tool call response" }], + images: [], + isError: true, + }; } - const text = parsed.data.contentItems - .filter((item) => item.type === "inputText") - .map((item) => (item as { type: "inputText"; text: string }).text) - .join("\n"); + const texts: string[] = []; + const contentBlocks: BridgeToolCallContent[] = []; + const images: BridgeToolCallImage[] = []; + for (const item of parsed.data.contentItems) { + if (item.type === "inputText") { + texts.push(item.text); + if (item.text !== "") { + contentBlocks.push({ type: "text", text: item.text }); + } + continue; + } + const image = decodeImageDataUrl(item.imageUrl); + if (image === null) { + texts.push(item.imageUrl); + contentBlocks.push({ type: "text", text: item.imageUrl }); + continue; + } + images.push(image); + contentBlocks.push({ type: "image", ...image }); + } + const text = texts.join("\n"); + const isError = !parsed.data.success; + if (contentBlocks.length === 0) { + const fallback = isError ? "Tool call failed" : "OK"; + return { + content: fallback, + contentBlocks: [{ type: "text", text: fallback }], + images, + isError, + }; + } return { - content: text || "OK", - isError: !parsed.data.success, + // Keep the legacy aggregate fields for provider bridges that already use + // this published helper. New consumers use contentBlocks so interleaved + // text and images retain the plugin result's order. + content: text, + contentBlocks, + images, + isError, }; } + +/** + * Renders a decoded payload as tool result blocks, dropping empty text so an + * image-only result carries the image alone. + */ +export function buildBridgeToolCallContent(result: { + content: string; + contentBlocks?: BridgeToolCallContent[]; + images?: BridgeToolCallImage[]; +}): BridgeToolCallContent[] { + if (result.contentBlocks !== undefined) { + return result.contentBlocks; + } + const blocks: BridgeToolCallContent[] = []; + if (result.content !== "") { + blocks.push({ type: "text", text: result.content }); + } + for (const image of result.images ?? []) { + blocks.push({ type: "image", data: image.data, mimeType: image.mimeType }); + } + return blocks; +} diff --git a/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.test.ts b/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.test.ts index d07f765624..d3309cda9a 100644 --- a/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.test.ts +++ b/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.test.ts @@ -67,7 +67,12 @@ describe("createPendingToolCallTracker", () => { }, }), ).toBe(true); - await expect(result).resolves.toEqual({ content: "hello", isError: false }); + await expect(result).resolves.toEqual({ + content: "hello", + contentBlocks: [{ type: "text", text: "hello" }], + images: [], + isError: false, + }); }); it("settles a pending call from an error response", async () => { @@ -151,7 +156,12 @@ describe("createPendingToolCallTracker", () => { }, }), ).toBe(true); - await expect(resultB).resolves.toEqual({ content: "b", isError: false }); + await expect(resultB).resolves.toEqual({ + content: "b", + contentBlocks: [{ type: "text", text: "b" }], + images: [], + isError: false, + }); }); }); diff --git a/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.ts b/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.ts index 34a7e4d773..936e79959f 100644 --- a/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.ts +++ b/packages/provider-bridge-protocol/src/bridge-kit/pending-tool-call-tracker.ts @@ -1,11 +1,17 @@ import { decodeToolCallResponsePayload, type BridgeJsonRpcResponse, + type BridgeToolCallContent, + type BridgeToolCallImage, type BridgeToolCallRequest, } from "./bridge-tool-calls.js"; export interface BridgeToolCallResult { content: string; + /** Ordered provider result blocks; absent on local transport failures. */ + contentBlocks?: BridgeToolCallContent[]; + /** Absent on the failure paths below, which have no image to report. */ + images?: BridgeToolCallImage[]; isError?: boolean; } diff --git a/plugins/provider-acp/src/bridge/bridge.test.ts b/plugins/provider-acp/src/bridge/bridge.test.ts index dfe25301e4..bbdc60318d 100644 --- a/plugins/provider-acp/src/bridge/bridge.test.ts +++ b/plugins/provider-acp/src/bridge/bridge.test.ts @@ -100,8 +100,10 @@ function notifications(method: string): BridgeJsonRpcOutputMessage[] { * assembler per call over the full ordered capture keeps ids deterministic. */ function threadEvents(): Record[] { - return assembleCapturedThreadEvents(output.messages, "acp") as unknown as - Record[]; + return assembleCapturedThreadEvents( + output.messages, + "acp", + ) as unknown as Record[]; } /** The delta kinds the bridge put on the wire, in emission order. */ @@ -1351,9 +1353,7 @@ describe("acp bridge", () => { projectSlug, "mcp-approvals.json", ); - const approvals = JSON.parse( - readFileSync(approvalPath, "utf8"), - ) as unknown; + const approvals = JSON.parse(readFileSync(approvalPath, "utf8")) as unknown; expect(approvals).toEqual([ expect.stringMatching(`^${ACP_BRIDGE_MCP_SERVER_NAME}-[a-f0-9]{16}$`), ]); @@ -1450,6 +1450,8 @@ describe("acp bridge", () => { await expect(bridgeCall).resolves.toEqual({ content: "environment directory updated", + contentBlocks: [{ type: "text", text: "environment directory updated" }], + images: [], isError: false, ok: true, }); @@ -2230,8 +2232,7 @@ describe("acp bridge", () => { return params.threadId === threadId && Array.isArray(params.deltas) && params.deltas.some( - (delta) => - (delta as { kind?: unknown }).kind === "session.reset", + (delta) => (delta as { kind?: unknown }).kind === "session.reset", ) ? [index] : []; diff --git a/plugins/provider-acp/src/bridge/bridge.ts b/plugins/provider-acp/src/bridge/bridge.ts index 382b867f13..01f519acb1 100644 --- a/plugins/provider-acp/src/bridge/bridge.ts +++ b/plugins/provider-acp/src/bridge/bridge.ts @@ -44,6 +44,10 @@ import { createServer, type Server, type Socket } from "node:net"; import { dirname, isAbsolute, basename, relative, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { z } from "zod"; + +type DecodedToolCallResponse = ReturnType; +type BridgeToolCallContent = DecodedToolCallResponse["contentBlocks"][number]; +type BridgeToolCallImage = DecodedToolCallResponse["images"][number]; import { ACP_BRIDGE_NO_ACTIVE_TURN_ERROR_CODE, ACP_COMPACTION_COMPLETED_METHOD, @@ -359,7 +363,13 @@ async function forwardDynamicToolCall(args: { threadId: string; tool: string; }): Promise< - | { ok: true; content: string; isError?: boolean } + | { + ok: true; + content: string; + contentBlocks: BridgeToolCallContent[]; + images: BridgeToolCallImage[]; + isError?: boolean; + } | { ok: false; error: string } > { const session = sessionsByBbThreadId.get(args.threadId); diff --git a/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts b/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts index 90074b3bc4..ceb34a43b2 100644 --- a/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts +++ b/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts @@ -34,6 +34,7 @@ afterEach(async () => { /** A stand-in for the bridge's TCP dynamic-tool socket that answers late. */ async function listenFakeBridge(args: { responseDelayMs: number; + response?: unknown; }): Promise<{ port: number; server: Server; requests: unknown[] }> { const requests: unknown[] = []; const server = createServer((socket) => { @@ -51,10 +52,12 @@ async function listenFakeBridge(args: { return; } requests.push(request); + const response = args.response ?? { + ok: true, + content: '{"answers":{"Which?":"B"}}', + }; setTimeout(() => { - socket.end( - `${JSON.stringify({ ok: true, content: '{"answers":{"Which?":"B"}}' })}\n`, - ); + socket.end(`${JSON.stringify(response)}\n`); }, args.responseDelayMs); }); }); @@ -147,4 +150,32 @@ describe("bb-bridge MCP server keeps long tool calls alive", () => { expect(progressCount).toBeGreaterThan(0); expect(fakeBridge.requests).toHaveLength(1); }, 20_000); + + // The response above carries neither image field, pinning compatibility with + // the text-only socket shape. This one carries an image-only result. + it("relays an image-only result as an MCP image block", async () => { + const fakeBridge = await listenFakeBridge({ + responseDelayMs: 0, + response: { + ok: true, + content: "", + contentBlocks: [ + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + ], + images: [{ data: "iVBORw0KGgo=", mimeType: "image/png" }], + }, + }); + const client = await connectLikeOpenCode(fakeBridge.port); + + const result = await client.callTool( + { name: "AskUserQuestion", arguments: {} }, + CallToolResultSchema, + { timeout: 5_000 }, + ); + + expect(result.isError).toBeFalsy(); + expect(result.content).toEqual([ + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + ]); + }, 20_000); }); diff --git a/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts b/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts index 4501a679c5..452c6ada7c 100644 --- a/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts +++ b/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts @@ -1,5 +1,6 @@ import { dynamicToolSchema, + experimental_buildBridgeToolCallContent, type DynamicTool, } from "@get-bb/plugin-sdk/provider-bridge"; import { createConnection } from "node:net"; @@ -59,18 +60,33 @@ type BridgeRequestPayload = tool: string; }; -type BridgeToolCallResponse = - | { ok: true; content: string; isError?: boolean } - | { ok: false; error: string }; - const bridgeToolCallResponseSchema = z.union([ z.object({ ok: z.literal(true), content: z.string(), + contentBlocks: z + .array( + z.discriminatedUnion("type", [ + z.object({ type: z.literal("text"), text: z.string() }), + z.object({ + type: z.literal("image"), + data: z.string(), + mimeType: z.string(), + }), + ]), + ) + .optional(), + // The initialized response and older text-only responses omit images. + // Parsing them as an empty list keeps the re-executed packaged artifact + // compatible with that legacy socket shape. + images: z + .array(z.object({ data: z.string(), mimeType: z.string() })) + .default([]), isError: z.boolean().optional(), }), z.object({ ok: z.literal(false), error: z.string() }), ]); +type BridgeToolCallResponse = z.infer; interface JsonRpcMessage { id?: string | number; @@ -315,7 +331,7 @@ async function handleRequest( return; } writeResult(message.id, { - content: [{ type: "text", text: result.content }], + content: experimental_buildBridgeToolCallContent(result), ...(result.isError ? { isError: true } : {}), }); } catch (error) { diff --git a/plugins/provider-acp/src/delta-translation.test.ts b/plugins/provider-acp/src/delta-translation.test.ts index 847cdbc41c..147cc3c8ff 100644 --- a/plugins/provider-acp/src/delta-translation.test.ts +++ b/plugins/provider-acp/src/delta-translation.test.ts @@ -163,9 +163,11 @@ describe("acp delta translation (bridge-shared invariants)", () => { const startedItemId = startedEvents.find((event) => event.type === "item/started")?.type === "item/started" - ? (startedEvents.find( - (event) => event.type === "item/started", - ) as Extract).item.id + ? ( + startedEvents.find( + (event) => event.type === "item/started", + ) as Extract + ).item.id : ""; const terminalEvents = harness.translate( @@ -549,6 +551,38 @@ describe("acp delta translation (moved from the legacy adapter suite)", () => { ]); }); + it("summarizes inline image attachments from raw tool output", () => { + const events = startedHarness().translate( + updateEvent({ + sessionUpdate: "tool_call", + toolCallId: "call-image", + title: "Inspect image", + kind: "other", + status: "completed", + rawOutput: { + output: "", + attachments: [ + { + url: "data:image/svg+xml;charset=utf-8;base64,PHN2Zy8+", + contentType: "image/svg+xml", + }, + ], + }, + }), + ); + + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ + type: "item/completed", + item: { + type: "toolCall", + result: + '{"output":"","attachments":[{"url":"[image]","contentType":"image/svg+xml"}]}', + }, + }); + expect(JSON.stringify(events)).not.toContain("PHN2Zy8+"); + }); + it("translates diff tool calls into file changes", () => { const events = startedHarness().translate( updateEvent({ diff --git a/plugins/provider-acp/src/delta-translation.ts b/plugins/provider-acp/src/delta-translation.ts index 2fa78bd09e..4381fd129d 100644 --- a/plugins/provider-acp/src/delta-translation.ts +++ b/plugins/provider-acp/src/delta-translation.ts @@ -77,6 +77,8 @@ export interface AcpDeltaTranslationContext { const ASSISTANT_STREAM_KEY = "assistant"; const THOUGHT_STREAM_KEY = "thought"; +const INLINE_IMAGE_DATA_URL_PATTERN = + /data:image\/[a-z0-9.+-]+(?:;[^,]*)?;base64,[a-z0-9+/_=-]+/giu; const ACP_PLAN_STEP_STATUS_BY_ENTRY_STATUS = { pending: "pending", @@ -107,7 +109,12 @@ function extractAcpToolCallOutputText( if (event.rawOutput === undefined) { return undefined; } - const rawOutputText = extractResultText(event.rawOutput).trim(); + // Some ACP agents echo MCP image results as data-URL attachments in + // rawOutput. Keep the useful envelope, but do not persist or render the + // potentially multi-megabyte payload in the thread timeline. + const rawOutputText = extractResultText(event.rawOutput) + .replace(INLINE_IMAGE_DATA_URL_PATTERN, "[image]") + .trim(); return rawOutputText.length > 0 ? rawOutputText : undefined; } diff --git a/plugins/provider-claude-code/src/bridge/__tests__/tool-proxy-mcp.test.ts b/plugins/provider-claude-code/src/bridge/__tests__/tool-proxy-mcp.test.ts index 68b20e5e34..f661a7f10f 100644 --- a/plugins/provider-claude-code/src/bridge/__tests__/tool-proxy-mcp.test.ts +++ b/plugins/provider-claude-code/src/bridge/__tests__/tool-proxy-mcp.test.ts @@ -109,4 +109,38 @@ describe("buildBridgeMcpServer", () => { }); await client.close(); }); + + // Before this, an image-only result reached the model as the text "OK". + it("serves an image-only tool result as an MCP image block", async () => { + const server = buildBridgeMcpServer( + [ + { + name: "browser_screenshot", + description: "A PNG of the current page.", + inputSchema: { type: "object" }, + }, + ], + async () => ({ + content: "", + contentBlocks: [ + { + type: "image" as const, + data: "iVBORw0KGgo=", + mimeType: "image/png", + }, + ], + images: [{ data: "iVBORw0KGgo=", mimeType: "image/png" }], + }), + ); + const client = await connect(server); + + const result = await client.callTool({ + name: "browser_screenshot", + arguments: {}, + }); + expect(result.content).toEqual([ + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + ]); + await client.close(); + }); }); diff --git a/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts b/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts index 93300dd7da..5b3e2ce301 100644 --- a/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts +++ b/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts @@ -1,5 +1,6 @@ import { type DynamicTool, + experimental_buildBridgeToolCallContent, } from "@get-bb/plugin-sdk/provider-bridge"; import type { McpSdkServerConfigWithInstance } from "@anthropic-ai/claude-agent-sdk"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; @@ -12,10 +13,18 @@ export const BRIDGE_MCP_SERVER_NAME = "bb-bridge"; export type DynamicToolDefinition = DynamicTool; +type BridgeToolCallContent = + | { type: "text"; text: string } + | { type: "image"; data: string; mimeType: string }; + export type ToolCallForwarder = ( toolName: string, args: Record, -) => Promise<{ content: string; isError?: boolean }>; +) => Promise<{ + content: string; + contentBlocks?: BridgeToolCallContent[]; + isError?: boolean; +}>; export function buildBridgeMcpServer( dynamicTools: DynamicToolDefinition[], @@ -56,7 +65,7 @@ export function buildBridgeMcpServer( request.params.arguments ?? {}, ); return { - content: [{ type: "text" as const, text: result.content }], + content: experimental_buildBridgeToolCallContent(result), ...(result.isError ? { isError: true } : {}), }; });