From 595912c0081579f7dda04fe8c79206c0ff95021f Mon Sep 17 00:00:00 2001 From: sujeito-operator Date: Wed, 19 Aug 2026 22:17:32 +0000 Subject: [PATCH 1/2] Return tool-call images instead of dropping them decodeToolCallResponsePayload kept only inputText items and fell back to "OK", so an image-only plugin tool result reached the model as the word "OK" with the image discarded. Decode inputImage data URLs into images and render them as tool result blocks in the claude-code, pi and acp bridges. --- .../agent-runtime/src/pi/bridge/tool-proxy.ts | 12 +- packages/plugin-sdk/src/provider-bridge.ts | 3 + .../src/bridge-kit/bridge-tool-calls.test.ts | 144 ++++++++++++++++++ .../src/bridge-kit/bridge-tool-calls.ts | 80 +++++++++- .../pending-tool-call-tracker.test.ts | 12 +- .../bridge-kit/pending-tool-call-tracker.ts | 3 + .../provider-acp/src/bridge/bridge.test.ts | 1 + plugins/provider-acp/src/bridge/bridge.ts | 8 +- .../src/bridge/tool-proxy-mcp.test.ts | 34 ++++- .../provider-acp/src/bridge/tool-proxy-mcp.ts | 16 +- .../bridge/__tests__/tool-proxy-mcp.test.ts | 27 ++++ .../src/bridge/tool-proxy-mcp.ts | 10 +- 12 files changed, 332 insertions(+), 18 deletions(-) create mode 100644 packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.test.ts diff --git a/packages/agent-runtime/src/pi/bridge/tool-proxy.ts b/packages/agent-runtime/src/pi/bridge/tool-proxy.ts index 4ccacef050..32830f1d51 100644 --- a/packages/agent-runtime/src/pi/bridge/tool-proxy.ts +++ b/packages/agent-runtime/src/pi/bridge/tool-proxy.ts @@ -1,5 +1,9 @@ import { Type, type TSchema } from "@earendil-works/pi-ai"; import type { ToolDefinition } from "@earendil-works/pi-coding-agent"; +import { + buildBridgeToolCallContent, + type BridgeToolCallImage, +} from "@bb/provider-bridge-protocol/bridge-kit"; export interface DynamicToolDefinition { name: string; @@ -10,7 +14,11 @@ export interface DynamicToolDefinition { export type ToolCallForwarder = ( toolName: string, args: Record, -) => Promise<{ content: string; isError?: boolean }>; +) => Promise<{ + content: string; + images?: BridgeToolCallImage[]; + isError?: boolean; +}>; /** * Builds Pi-compatible ToolDefinition objects from dynamic tool definitions @@ -34,7 +42,7 @@ export function buildDynamicTools( ) { const result = await forwardToolCall(def.name, params); return { - content: [{ type: "text" as const, text: result.content }], + content: buildBridgeToolCallContent(result), details: {}, ...(result.isError ? { isError: true } : {}), }; diff --git a/packages/plugin-sdk/src/provider-bridge.ts b/packages/plugin-sdk/src/provider-bridge.ts index a84ed17784..5d22971f68 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, buildShellEnvOverrides, createBridgeIo, createBridgeLineHandler, @@ -143,6 +144,8 @@ export { } from "@bb/provider-bridge-protocol/bridge-kit"; export type { BridgeJsonRpcResponse, + BridgeToolCallContent, + BridgeToolCallImage, BridgeToolCallRequest, BuildInteractiveResponseArgs, DecodedInteractiveRequest, 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..63d320b38b --- /dev/null +++ b/packages/provider-bridge-protocol/src/bridge-kit/bridge-tool-calls.test.ts @@ -0,0 +1,144 @@ +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", 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: "", + 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", + images: [{ data: PNG, mimeType: "image/jpeg" }], + isError: false, + }); + }); + + 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", + 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,", + images: [], + isError: false, + }); + }); + + it("falls back to OK only when there is neither text nor image", () => { + expect( + decodeToolCallResponsePayload({ success: true, contentItems: [] }), + ).toEqual({ content: "OK", images: [], isError: false }); + expect(decodeToolCallResponsePayload({ nope: true })).toEqual({ + content: "OK", + images: [], + isError: false, + }); + }); +}); + +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("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..99b571e475 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,90 @@ 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; + images: BridgeToolCallImage[]; isError: boolean; } { const parsed = providerToolCallResponseSchema.safeParse(result); if (!parsed.success) { - return { content: "OK", isError: false }; + return { content: "OK", images: [], isError: false }; } - 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 images: BridgeToolCallImage[] = []; + for (const item of parsed.data.contentItems) { + if (item.type === "inputText") { + texts.push(item.text); + continue; + } + const image = decodeImageDataUrl(item.imageUrl); + if (image === null) { + texts.push(item.imageUrl); + continue; + } + images.push(image); + } + const text = texts.join("\n"); return { - content: text || "OK", + // "OK" stands in for an empty result, not for a dropped one: an image-only + // result says what happened through `images` and must not be relabelled. + content: text === "" && images.length === 0 ? "OK" : text, + images, isError: !parsed.data.success, }; } + +/** + * 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; + images?: BridgeToolCallImage[]; +}): BridgeToolCallContent[] { + 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..3fe06b3c99 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,11 @@ describe("createPendingToolCallTracker", () => { }, }), ).toBe(true); - await expect(result).resolves.toEqual({ content: "hello", isError: false }); + await expect(result).resolves.toEqual({ + content: "hello", + images: [], + isError: false, + }); }); it("settles a pending call from an error response", async () => { @@ -151,7 +155,11 @@ describe("createPendingToolCallTracker", () => { }, }), ).toBe(true); - await expect(resultB).resolves.toEqual({ content: "b", isError: false }); + await expect(resultB).resolves.toEqual({ + content: "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..e37ced488f 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,14 @@ import { decodeToolCallResponsePayload, type BridgeJsonRpcResponse, + type BridgeToolCallImage, type BridgeToolCallRequest, } from "./bridge-tool-calls.js"; export interface BridgeToolCallResult { content: string; + /** 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..18b8f867d1 100644 --- a/plugins/provider-acp/src/bridge/bridge.test.ts +++ b/plugins/provider-acp/src/bridge/bridge.test.ts @@ -1450,6 +1450,7 @@ describe("acp bridge", () => { await expect(bridgeCall).resolves.toEqual({ content: "environment directory updated", + images: [], isError: false, ok: true, }); diff --git a/plugins/provider-acp/src/bridge/bridge.ts b/plugins/provider-acp/src/bridge/bridge.ts index 382b867f13..d3d594d1e2 100644 --- a/plugins/provider-acp/src/bridge/bridge.ts +++ b/plugins/provider-acp/src/bridge/bridge.ts @@ -29,6 +29,7 @@ import { runBridgeRequest, withoutBridgeRuntimeEnv, type BridgeJsonRpcResponse, + type BridgeToolCallImage, BRIDGE_INBOUND_REQUEST_METHODS, BRIDGE_JSON_RPC_ERRORS, BRIDGE_NOTIFICATION_METHODS, @@ -359,7 +360,12 @@ async function forwardDynamicToolCall(args: { threadId: string; tool: string; }): Promise< - | { ok: true; content: string; isError?: boolean } + | { + ok: true; + content: string; + 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..7ad1477b8c 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,29 @@ 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 no `images` key, so it also pins that an older + // packaged bridge still decodes. 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: "", + 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..fbaf42d1b1 100644 --- a/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts +++ b/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts @@ -1,4 +1,5 @@ import { + buildBridgeToolCallContent, dynamicToolSchema, type DynamicTool, } from "@get-bb/plugin-sdk/provider-bridge"; @@ -60,13 +61,24 @@ type BridgeRequestPayload = }; type BridgeToolCallResponse = - | { ok: true; content: string; isError?: boolean } + | { + ok: true; + content: string; + images: { data: string; mimeType: string }[]; + isError?: boolean; + } | { ok: false; error: string }; const bridgeToolCallResponseSchema = z.union([ z.object({ ok: z.literal(true), content: z.string(), + // Defaulted rather than optional: this socket's two ends ship together, but + // the MCP process is re-executed from the packaged artifact and a missing + // key here would otherwise throw instead of degrading to a text result. + 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() }), @@ -315,7 +327,7 @@ async function handleRequest( return; } writeResult(message.id, { - content: [{ type: "text", text: result.content }], + content: buildBridgeToolCallContent(result), ...(result.isError ? { isError: true } : {}), }); } catch (error) { 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..5049269535 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,31 @@ 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: "", + 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..52facc33bf 100644 --- a/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts +++ b/plugins/provider-claude-code/src/bridge/tool-proxy-mcp.ts @@ -1,4 +1,6 @@ import { + buildBridgeToolCallContent, + type BridgeToolCallImage, type DynamicTool, } from "@get-bb/plugin-sdk/provider-bridge"; import type { McpSdkServerConfigWithInstance } from "@anthropic-ai/claude-agent-sdk"; @@ -15,7 +17,11 @@ export type DynamicToolDefinition = DynamicTool; export type ToolCallForwarder = ( toolName: string, args: Record, -) => Promise<{ content: string; isError?: boolean }>; +) => Promise<{ + content: string; + images?: BridgeToolCallImage[]; + isError?: boolean; +}>; export function buildBridgeMcpServer( dynamicTools: DynamicToolDefinition[], @@ -56,7 +62,7 @@ export function buildBridgeMcpServer( request.params.arguments ?? {}, ); return { - content: [{ type: "text" as const, text: result.content }], + content: buildBridgeToolCallContent(result), ...(result.isError ? { isError: true } : {}), }; }); From 9ef36eaeff5626591e0d738b2742a6ce6c6d78c2 Mon Sep 17 00:00:00 2001 From: Michael Yong Date: Thu, 20 Aug 2026 11:41:19 -0700 Subject: [PATCH 2/2] Harden tool-call image bridging --- docs/api_to_audit.md | 12 +++ .../pi/bridge/__tests__/tool-proxy.test.ts | 47 ++++++++++ .../agent-runtime/src/pi/bridge/tool-proxy.ts | 8 +- packages/plugin-sdk/src/provider-bridge.ts | 4 +- .../src/bridge-kit/bridge-tool-calls.test.ts | 85 ++++++++++++++++++- .../src/bridge-kit/bridge-tool-calls.ts | 40 +++++++-- .../pending-tool-call-tracker.test.ts | 2 + .../bridge-kit/pending-tool-call-tracker.ts | 3 + .../provider-acp/src/bridge/bridge.test.ts | 14 +-- plugins/provider-acp/src/bridge/bridge.ts | 6 +- .../src/bridge/tool-proxy-mcp.test.ts | 7 +- .../provider-acp/src/bridge/tool-proxy-mcp.ts | 32 ++++--- .../src/delta-translation.test.ts | 40 ++++++++- plugins/provider-acp/src/delta-translation.ts | 9 +- .../bridge/__tests__/tool-proxy-mcp.test.ts | 7 ++ .../src/bridge/tool-proxy-mcp.ts | 11 ++- 16 files changed, 282 insertions(+), 45 deletions(-) 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 32830f1d51..573bb58642 100644 --- a/packages/agent-runtime/src/pi/bridge/tool-proxy.ts +++ b/packages/agent-runtime/src/pi/bridge/tool-proxy.ts @@ -2,6 +2,7 @@ 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"; @@ -16,6 +17,7 @@ export type ToolCallForwarder = ( args: Record, ) => Promise<{ content: string; + contentBlocks?: BridgeToolCallContent[]; images?: BridgeToolCallImage[]; isError?: boolean; }>; @@ -41,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: 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 5d22971f68..b39c33b7c7 100644 --- a/packages/plugin-sdk/src/provider-bridge.ts +++ b/packages/plugin-sdk/src/provider-bridge.ts @@ -112,7 +112,7 @@ export type { export { bashArgsSchema, bridgeRequestEnvelopeSchema, - buildBridgeToolCallContent, + buildBridgeToolCallContent as experimental_buildBridgeToolCallContent, buildShellEnvOverrides, createBridgeIo, createBridgeLineHandler, @@ -144,8 +144,6 @@ export { } from "@bb/provider-bridge-protocol/bridge-kit"; export type { BridgeJsonRpcResponse, - BridgeToolCallContent, - BridgeToolCallImage, BridgeToolCallRequest, BuildInteractiveResponseArgs, DecodedInteractiveRequest, 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 index 63d320b38b..5598065702 100644 --- 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 @@ -16,7 +16,15 @@ describe("decodeToolCallResponsePayload", () => { { type: "inputText", text: "second" }, ], }), - ).toEqual({ content: "first\nsecond", images: [], isError: false }); + ).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 @@ -31,6 +39,7 @@ describe("decodeToolCallResponsePayload", () => { }), ).toEqual({ content: "", + contentBlocks: [{ type: "image", data: PNG, mimeType: "image/png" }], images: [{ data: PNG, mimeType: "image/png" }], isError: false, }); @@ -47,11 +56,32 @@ describe("decodeToolCallResponsePayload", () => { }), ).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({ @@ -75,6 +105,7 @@ describe("decodeToolCallResponsePayload", () => { }), ).toEqual({ content: "https://example.com/a.png", + contentBlocks: [{ type: "text", text: "https://example.com/a.png" }], images: [], isError: false, }); @@ -90,6 +121,7 @@ describe("decodeToolCallResponsePayload", () => { }), ).toEqual({ content: "data:image/png;base64,", + contentBlocks: [{ type: "text", text: "data:image/png;base64," }], images: [], isError: false, }); @@ -98,13 +130,44 @@ describe("decodeToolCallResponsePayload", () => { it("falls back to OK only when there is neither text nor image", () => { expect( decodeToolCallResponsePayload({ success: true, contentItems: [] }), - ).toEqual({ content: "OK", images: [], isError: false }); - expect(decodeToolCallResponsePayload({ nope: true })).toEqual({ + ).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", () => { @@ -129,6 +192,22 @@ describe("buildBridgeToolCallContent", () => { ]); }); + 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" }, 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 99b571e475..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 @@ -132,7 +132,7 @@ export type BridgeToolCallContent = | { type: "text"; text: string } | { type: "image"; data: string; mimeType: string }; -const IMAGE_DATA_URL = /^data:([^;,]+);base64,(.*)$/s; +const IMAGE_DATA_URL = /^data:(.+);base64,(.+)$/s; /** * Splits `data:;base64,` into the parts a tool result carries. @@ -154,36 +154,60 @@ function decodeImageDataUrl(imageUrl: string): BridgeToolCallImage | null { export function decodeToolCallResponsePayload(result: unknown): { content: string; + contentBlocks: BridgeToolCallContent[]; images: BridgeToolCallImage[]; isError: boolean; } { const parsed = providerToolCallResponseSchema.safeParse(result); if (!parsed.success) { - return { content: "OK", images: [], isError: false }; + return { + content: "Invalid tool call response", + contentBlocks: [{ type: "text", text: "Invalid tool call response" }], + images: [], + isError: true, + }; } 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 { - // "OK" stands in for an empty result, not for a dropped one: an image-only - // result says what happened through `images` and must not be relabelled. - content: text === "" && images.length === 0 ? "OK" : text, + // 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: !parsed.data.success, + isError, }; } @@ -193,8 +217,12 @@ export function decodeToolCallResponsePayload(result: unknown): { */ 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 }); 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 3fe06b3c99..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 @@ -69,6 +69,7 @@ describe("createPendingToolCallTracker", () => { ).toBe(true); await expect(result).resolves.toEqual({ content: "hello", + contentBlocks: [{ type: "text", text: "hello" }], images: [], isError: false, }); @@ -157,6 +158,7 @@ describe("createPendingToolCallTracker", () => { ).toBe(true); 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 e37ced488f..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,12 +1,15 @@ 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 18b8f867d1..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,7 @@ describe("acp bridge", () => { await expect(bridgeCall).resolves.toEqual({ content: "environment directory updated", + contentBlocks: [{ type: "text", text: "environment directory updated" }], images: [], isError: false, ok: true, @@ -2231,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 d3d594d1e2..01f519acb1 100644 --- a/plugins/provider-acp/src/bridge/bridge.ts +++ b/plugins/provider-acp/src/bridge/bridge.ts @@ -29,7 +29,6 @@ import { runBridgeRequest, withoutBridgeRuntimeEnv, type BridgeJsonRpcResponse, - type BridgeToolCallImage, BRIDGE_INBOUND_REQUEST_METHODS, BRIDGE_JSON_RPC_ERRORS, BRIDGE_NOTIFICATION_METHODS, @@ -45,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, @@ -363,6 +366,7 @@ async function forwardDynamicToolCall(args: { | { ok: true; content: string; + contentBlocks: BridgeToolCallContent[]; images: BridgeToolCallImage[]; isError?: boolean; } 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 7ad1477b8c..ceb34a43b2 100644 --- a/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts +++ b/plugins/provider-acp/src/bridge/tool-proxy-mcp.test.ts @@ -151,14 +151,17 @@ describe("bb-bridge MCP server keeps long tool calls alive", () => { expect(fakeBridge.requests).toHaveLength(1); }, 20_000); - // The response above carries no `images` key, so it also pins that an older - // packaged bridge still decodes. This one carries an image-only result. + // 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" }], }, }); diff --git a/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts b/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts index fbaf42d1b1..452c6ada7c 100644 --- a/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts +++ b/plugins/provider-acp/src/bridge/tool-proxy-mcp.ts @@ -1,6 +1,6 @@ import { - buildBridgeToolCallContent, dynamicToolSchema, + experimental_buildBridgeToolCallContent, type DynamicTool, } from "@get-bb/plugin-sdk/provider-bridge"; import { createConnection } from "node:net"; @@ -60,22 +60,25 @@ type BridgeRequestPayload = tool: string; }; -type BridgeToolCallResponse = - | { - ok: true; - content: string; - images: { data: string; mimeType: string }[]; - isError?: boolean; - } - | { ok: false; error: string }; - const bridgeToolCallResponseSchema = z.union([ z.object({ ok: z.literal(true), content: z.string(), - // Defaulted rather than optional: this socket's two ends ship together, but - // the MCP process is re-executed from the packaged artifact and a missing - // key here would otherwise throw instead of degrading to a text result. + 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([]), @@ -83,6 +86,7 @@ const bridgeToolCallResponseSchema = z.union([ }), z.object({ ok: z.literal(false), error: z.string() }), ]); +type BridgeToolCallResponse = z.infer; interface JsonRpcMessage { id?: string | number; @@ -327,7 +331,7 @@ async function handleRequest( return; } writeResult(message.id, { - content: buildBridgeToolCallContent(result), + 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 5049269535..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 @@ -122,6 +122,13 @@ describe("buildBridgeMcpServer", () => { ], async () => ({ content: "", + contentBlocks: [ + { + type: "image" as const, + data: "iVBORw0KGgo=", + mimeType: "image/png", + }, + ], images: [{ data: "iVBORw0KGgo=", mimeType: "image/png" }], }), ); 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 52facc33bf..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,7 +1,6 @@ import { - buildBridgeToolCallContent, - type BridgeToolCallImage, 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"; @@ -14,12 +13,16 @@ 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; - images?: BridgeToolCallImage[]; + contentBlocks?: BridgeToolCallContent[]; isError?: boolean; }>; @@ -62,7 +65,7 @@ export function buildBridgeMcpServer( request.params.arguments ?? {}, ); return { - content: buildBridgeToolCallContent(result), + content: experimental_buildBridgeToolCallContent(result), ...(result.isError ? { isError: true } : {}), }; });