From b0088303fda3938a87bc6bed4c85a513769b74a7 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 3 Jul 2026 00:03:44 +0200 Subject: [PATCH] Render MCP screenshot images without timeline base64 --- .../src/server/agent/mcp-server.test.ts | 87 ++++++++++++++++++- .../providers/codex-app-server-agent.test.ts | 82 +++++++++++++++++ .../agent/providers/codex-app-server-agent.ts | 42 ++++++++- .../providers/codex/tool-call-mapper.test.ts | 38 ++++++++ .../agent/providers/codex/tool-call-mapper.ts | 56 +++++++++++- .../src/server/browser-tools/tools.test.ts | 18 +++- .../server/src/server/browser-tools/tools.ts | 13 ++- 7 files changed, 327 insertions(+), 9 deletions(-) diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index 00c96da61..8bfee67b2 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -63,6 +63,13 @@ interface LooseStructuredContent { [key: string]: unknown; } +interface LooseContentBlock { + type: string; + text?: string; + data?: string; + mimeType?: string; +} + interface RegisteredMcpTool { inputSchema: LooseInputSchema; outputSchema?: unknown; @@ -71,18 +78,18 @@ interface RegisteredMcpTool { extra?: unknown, ) => Promise<{ structuredContent: LooseStructuredContent; - content?: Array<{ type: string; text?: string }>; + content?: LooseContentBlock[]; }>; handler?: (input: unknown) => Promise<{ structuredContent: LooseStructuredContent; - content?: Array<{ type: string; text?: string }>; + content?: LooseContentBlock[]; }>; } interface RegisteredMcpToolWithHandler extends RegisteredMcpTool { handler: (input: unknown) => Promise<{ structuredContent: LooseStructuredContent; - content?: Array<{ type: string; text?: string }>; + content?: LooseContentBlock[]; }>; } @@ -725,6 +732,80 @@ describe("browser MCP tools", () => { } }); + it("returns screenshot pixels as image content and keeps structured content metadata-only", async () => { + const agentManager = new BoundaryAgentManagerFake(); + const agentStorage = new BoundaryAgentStorageFake(); + const broker = new FakeBrowserToolsBroker({ + requestId: "req-browser-screenshot", + ok: true, + result: { + command: "screenshot", + browserId: "11111111-1111-4111-8111-111111111111", + mimeType: "image/png", + dataBase64: "iVBORw0KGgo=", + width: 800, + height: 600, + }, + }); + const server = await createAgentMcpServer({ + agentManager: agentManager as AgentManager, + agentStorage: agentStorage as AgentStorage, + providerSnapshotManager: + new BoundaryProviderSnapshotManagerFake() as unknown as ProviderSnapshotManager, + browserToolsBroker: broker as BrowserToolsBroker, + callerAgentId: "agent-1", + logger, + }); + const client = await connectInMemoryMcpClient(server); + + try { + const response = await client.callTool({ + name: "browser_screenshot", + arguments: { browserId: "11111111-1111-4111-8111-111111111111" }, + }); + + expect(broker.calls).toEqual([ + { + agentId: "agent-1", + cwd: REPO_CWD, + workspaceId: BROWSER_WORKSPACE_ID, + command: { + command: "screenshot", + args: { + browserId: "11111111-1111-4111-8111-111111111111", + fullPage: false, + }, + }, + }, + ]); + expect(response.content).toEqual([ + { type: "text", text: "Captured browser screenshot (800x600)." }, + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + ]); + expect(response.structuredContent).toEqual({ + ok: true, + result: { + command: "screenshot", + browserId: "11111111-1111-4111-8111-111111111111", + mimeType: "image/png", + width: 800, + height: 600, + }, + context: { + agentId: "agent-1", + cwd: REPO_CWD, + workspaceId: BROWSER_WORKSPACE_ID, + browserId: "11111111-1111-4111-8111-111111111111", + }, + }); + expect(JSON.stringify(response.structuredContent)).not.toContain("iVBORw0KGgo="); + expect(JSON.stringify(response.structuredContent)).not.toContain("dataBase64"); + } finally { + await client.close(); + await server.close(); + } + }); + it("keeps browser tools registered when browser tools are disabled", async () => { const { agentManager, agentStorage, spies } = createTestDeps(); spies.agentManager.getAgent.mockReturnValue({ diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index 3a82f7410..c95665f84 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -2359,6 +2359,88 @@ describe("Codex app-server provider", () => { rmSync(source, { force: true }); }); + test("emits mcpToolCall image content as a completed tool call plus assistant markdown image", () => { + const session = createSession(); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + + asInternals(session).handleNotification("item/completed", { + item: { + id: "mcp-browser-screenshot", + type: "mcpToolCall", + status: "completed", + server: "paseo", + tool: "browser_screenshot", + arguments: { browserId: "11111111-1111-4111-8111-111111111111" }, + result: { + content: [ + { type: "text", text: "Captured browser screenshot (1x1)." }, + { type: "image", data: ONE_BY_ONE_PNG_BASE64, mimeType: "image/png" }, + ], + structuredContent: { + ok: true, + result: { + command: "screenshot", + browserId: "11111111-1111-4111-8111-111111111111", + mimeType: "image/png", + width: 1, + height: 1, + }, + }, + }, + }, + }); + + expect(events.length).toBe(2); + expect(events[0]).toEqual({ + type: "timeline", + provider: "codex", + turnId: "test-turn", + item: { + type: "tool_call", + callId: "mcp-browser-screenshot", + name: "paseo.browser_screenshot", + status: "completed", + error: null, + detail: { + type: "unknown", + input: { browserId: "11111111-1111-4111-8111-111111111111" }, + output: { + content: [ + { type: "text", text: "Captured browser screenshot (1x1)." }, + { type: "text", text: "[image]" }, + ], + structuredContent: { + ok: true, + result: { + command: "screenshot", + browserId: "11111111-1111-4111-8111-111111111111", + mimeType: "image/png", + width: 1, + height: 1, + }, + }, + }, + }, + }, + }); + expect(events[1]).toMatchObject({ + type: "timeline", + provider: "codex", + turnId: "test-turn", + item: { type: "assistant_message" }, + }); + const imageEvent = events[1]; + if (imageEvent?.type !== "timeline" || imageEvent.item.type !== "assistant_message") { + throw new Error("Expected assistant image timeline event"); + } + expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); + const source = markdownImageSource(imageEvent.item.text); + expect(source).toMatch(/paseo-attachments[\\/].+\.png$/); + expect(existsSync(source)).toBe(true); + rmSync(source, { force: true }); + }); + test("ignores incomplete imageGeneration thread items without failing the turn", () => { const session = createSession(); const events: AgentStreamEvent[] = []; diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 41d91f7d2..4681703c2 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -49,6 +49,7 @@ import { curateAgentActivity } from "../activity-curator.js"; import { mapCodexToolCallEnvelope, mapCodexToolCallFromThreadItem, + splitCodexMcpToolResultImages, } from "./codex/tool-call-mapper.js"; import { checkProviderLaunchAvailable, @@ -1696,6 +1697,39 @@ export function threadItemToTimeline( } } +function mcpToolResultImagesToTimeline(item: unknown): AgentTimelineItem[] { + const itemRecord = toObjectRecord(item); + if (!itemRecord) { + return []; + } + const normalizedType = normalizeCodexThreadItemType( + typeof itemRecord.type === "string" ? itemRecord.type : undefined, + ); + if (normalizedType !== "mcpToolCall") { + return []; + } + + const { images } = splitCodexMcpToolResultImages(itemRecord.result); + return images + .map((image) => + renderProviderImageOutputAsAssistantMarkdown(image, { + materialize: materializeProviderImage, + }), + ) + .filter((timelineItem): timelineItem is AgentTimelineItem => timelineItem !== null); +} + +function threadItemToTimelineEntries( + item: unknown, + options?: { includeUserMessage?: boolean; cwd?: string | null }, +): AgentTimelineItem[] { + const timelineItem = threadItemToTimeline(item, options); + if (!timelineItem) { + return []; + } + return [timelineItem, ...mcpToolResultImagesToTimeline(item)]; +} + const CodexThreadReadResponseSchema = z .object({ thread: z @@ -1735,8 +1769,7 @@ async function loadCodexThreadHistoryTimeline(params: { const timeline: PersistedTimelineEntry[] = []; for (const turn of response.thread.turns) { for (const item of turn.items) { - const timelineItem = threadItemToTimeline(item, { cwd: params.cwd }); - if (timelineItem) { + for (const timelineItem of threadItemToTimelineEntries(item, { cwd: params.cwd })) { const timestamp = readCodexHistoryTimestamp(item) ?? readCodexTurnHistoryTimestamp(turn, timelineItem); timeline.push({ @@ -4848,10 +4881,15 @@ export class CodexAppServerAgentSession implements AgentSession { } this.warnOnIncompleteEditToolCall(timelineItem, "item_completed", parsed.item); } + const imageItems = mcpToolResultImagesToTimeline(parsed.item); this.emitEvent({ type: "timeline", provider: CODEX_PROVIDER, item: timelineItem }); if (timelineItem.type === "assistant_message") { this.pendingAssistantMessageBoundary = true; } + for (const imageItem of imageItems) { + this.emitEvent({ type: "timeline", provider: CODEX_PROVIDER, item: imageItem }); + this.pendingAssistantMessageBoundary = true; + } if (itemId) { this.emittedItemCompletedIds.add(itemId); this.emittedItemStartedIds.delete(itemId); diff --git a/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts b/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts index bb69ee2e7..af4fab521 100644 --- a/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts +++ b/packages/server/src/server/agent/providers/codex/tool-call-mapper.test.ts @@ -703,6 +703,44 @@ describe("codex tool-call mapper", () => { }); }); + it("replaces mcp image result blocks with placeholder text in tool output", () => { + const item = expectMapped( + mapCodexToolCallFromThreadItem({ + type: "mcpToolCall", + id: "codex-browser-screenshot", + status: "completed", + server: "paseo", + tool: "browser_screenshot", + arguments: { browserId: "11111111-1111-4111-8111-111111111111" }, + result: { + content: [ + { type: "text", text: "Captured browser screenshot (1x1)." }, + { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, + ], + }, + }), + ); + + expect(item).toEqual({ + type: "tool_call", + callId: "codex-browser-screenshot", + name: "paseo.browser_screenshot", + status: "completed", + error: null, + detail: { + type: "unknown", + input: { browserId: "11111111-1111-4111-8111-111111111111" }, + output: { + content: [ + { type: "text", text: "Captured browser screenshot (1x1)." }, + { type: "text", text: "[image]" }, + ], + }, + }, + }); + expect(JSON.stringify(item)).not.toContain("iVBORw0KGgo="); + }); + it("normalizes codex paseo_voice.speak mcp calls and extracts spoken text", () => { const item = expectMapped( mapCodexToolCallFromThreadItem({ diff --git a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts index c4bdf9077..4979b7649 100644 --- a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts +++ b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts @@ -76,6 +76,16 @@ interface CodexResolvedToolCall { cwd: string | null; } +export interface CodexMcpToolResultImage { + data: string; + mimeType: string; +} + +interface CodexMcpToolResultImagesSplit { + images: CodexMcpToolResultImage[]; + output: unknown; +} + function toToolCallTimelineItem(envelope: CodexResolvedToolCall): ToolCallTimelineItem { const name = envelope.toolKind === "speak" ? ("speak" as const) : envelope.name; const parsedDetail = deriveCodexToolDetail({ @@ -596,6 +606,50 @@ function buildMcpToolName(server: string | undefined, tool: string): string { return trimmedTool; } +function readMcpImageContent(block: unknown): CodexMcpToolResultImage | null { + if (!isRecord(block)) { + return null; + } + if (block.type !== "image") { + return null; + } + if (typeof block.data !== "string" || typeof block.mimeType !== "string") { + return null; + } + return { + data: block.data, + mimeType: block.mimeType, + }; +} + +export function splitCodexMcpToolResultImages(result: unknown): CodexMcpToolResultImagesSplit { + if (!isRecord(result) || !Array.isArray(result.content)) { + return { images: [], output: result }; + } + + const images: CodexMcpToolResultImage[] = []; + const content = result.content.map((block) => { + const image = readMcpImageContent(block); + if (!image) { + return block; + } + images.push(image); + return { type: "text", text: "[image]" }; + }); + + if (images.length === 0) { + return { images, output: result }; + } + + return { + images, + output: { + ...result, + content, + }, + }; +} + function toNullableObject(value: Record): Record | null { return Object.keys(value).length > 0 ? value : null; } @@ -855,7 +909,7 @@ function mapMcpToolCallItem( } const name = buildMcpToolName(item.server, tool); const input = item.arguments ?? null; - const output = item.result ?? null; + const { output } = splitCodexMcpToolResultImages(item.result ?? null); const error = item.error ?? null; const status = normalizeToolCallStatus(item.status, error, output); diff --git a/packages/server/src/server/browser-tools/tools.test.ts b/packages/server/src/server/browser-tools/tools.test.ts index 8c724d82b..edfa66555 100644 --- a/packages/server/src/server/browser-tools/tools.test.ts +++ b/packages/server/src/server/browser-tools/tools.test.ts @@ -224,6 +224,13 @@ const routedToolCases = [ { type: "text", text: "Captured browser screenshot (800x600)." }, { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, ], + structuredResult: { + command: "screenshot", + browserId: BROWSER_ID, + mimeType: "image/png", + width: 800, + height: 600, + }, }, { name: "logs", @@ -270,6 +277,13 @@ const routedToolCases = [ { type: "text", text: "Captured browser screenshot (390x1200)." }, { type: "image", data: "iVBORw0KGgo=", mimeType: "image/png" }, ], + structuredResult: { + command: "screenshot", + browserId: BROWSER_ID, + mimeType: "image/png", + width: 390, + height: 1200, + }, }, { name: "upload", @@ -667,7 +681,7 @@ describe("registerBrowserTools", () => { test.each(routedToolCases)( "$name routes browser id in command args and workspace id in the envelope", - async ({ toolName, input, command, payload, content }) => { + async ({ toolName, input, command, payload, content, structuredResult }) => { const harness = new BrowserToolHarness(); harness.broker.setResponse(payload); @@ -684,7 +698,7 @@ describe("registerBrowserTools", () => { expect(response.content).toEqual(content); expect(response.structuredContent).toEqual({ ok: payload.ok, - result: payload.result, + result: structuredResult ?? payload.result, context: { agentId: "agent-1", cwd: "/repo", diff --git a/packages/server/src/server/browser-tools/tools.ts b/packages/server/src/server/browser-tools/tools.ts index 2fbc664c2..981bbdc46 100644 --- a/packages/server/src/server/browser-tools/tools.ts +++ b/packages/server/src/server/browser-tools/tools.ts @@ -680,7 +680,7 @@ function browserToolResult(params: { content: browserToolSuccessContent(payload), structuredContent: { ok: true, - result: payload.result, + result: browserToolStructuredResult(payload.result), context, }, }; @@ -696,6 +696,17 @@ function browserToolResult(params: { }; } +function browserToolStructuredResult( + result: Extract["result"], +): Extract["result"] | Record { + if (result.command !== "screenshot") { + return result; + } + + const { dataBase64: _dataBase64, ...metadata } = result; + return metadata; +} + function browserToolSuccessContent( payload: Extract, ): PaseoToolResult["content"] {