From ebaecace2bd33230c4f27698e73a64e624828366 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 9 Jul 2026 19:34:34 +0200 Subject: [PATCH] Keep Pi text-only sessions from bricking on images (#1960) * fix(pi): avoid raw images for text-only models Pi text-only models reject image content, and Pi persists the rejected raw image in session history. Materialize those image prompts to local files and send path hints while preserving raw images for vision-capable models. * fix(pi): require explicit vision support Default unknown Pi model capabilities to the text-hint path so raw image forwarding only happens for models that declare image input. Also keep materialized image files in a private per-user temp directory with restrictive file modes. * fix(pi): harden materialized image temp dirs * test(server): accept per-user image attachment dirs * fix(server): use private temp dirs for materialized images * fix(server): refresh missing image temp cache * test(server): clean image temp dirs --- docs/providers.md | 2 + .../claude/agent.image-rendering.test.ts | 71 +++++++++------ .../providers/codex-app-server-agent.test.ts | 4 +- .../server/agent/providers/pi/agent.test.ts | 89 +++++++++++++++++++ .../src/server/agent/providers/pi/agent.ts | 39 ++++++-- .../provider-image-output.posix.test.ts | 24 +++++ .../providers/provider-image-output.test.ts | 38 +++++++- .../agent/providers/provider-image-output.ts | 45 ++++++++-- 8 files changed, 270 insertions(+), 42 deletions(-) create mode 100644 packages/server/src/server/agent/providers/provider-image-output.posix.test.ts diff --git a/docs/providers.md b/docs/providers.md index 6af942119..a0da9fa3a 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -26,6 +26,8 @@ Pi is a process-backed provider. Paseo requires the user to have the `pi` binary Paseo's per-agent and daemon-wide system prompts are passed to Pi with `--append-system-prompt`, so Pi keeps its default coding prompt while receiving Paseo's additional instructions. +Pi model records expose input capabilities through `model.input`. Only send raw RPC `images` when the current model explicitly includes `"image"` in that list. Text-only Pi/OMP models reject image content and persist the rejected image in JSONL history, so image prompts for those models must be materialized to a local file and passed as a text path hint instead. + Pi MCP support depends on the open-source `pi-mcp-adapter` extension being loaded for the agent cwd. Probe with Pi RPC `get_commands`; the adapter registers an extension command named `mcp` (often with `sourceInfo.source` containing `pi-mcp-adapter`). When Paseo injects MCP servers into Pi, write a per-agent MCP config and pass it with `--mcp-config` instead of modifying user or project MCP files. For local HTTP servers such as Paseo's own `/mcp/agents` endpoint, explicitly disable adapter OAuth (`auth: false`, `oauth: false`) in the generated config. Pi import discovery reads Pi's persisted JSONL session files because Pi RPC does not expose a recent-session listing command. Resume and full history hydration still go through `pi --mode rpc` using the session file as `nativeHandle`. diff --git a/packages/server/src/server/agent/providers/claude/agent.image-rendering.test.ts b/packages/server/src/server/agent/providers/claude/agent.image-rendering.test.ts index 444019cfb..66891e01b 100644 --- a/packages/server/src/server/agent/providers/claude/agent.image-rendering.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.image-rendering.test.ts @@ -9,6 +9,8 @@ import { ClaudeAgentClient } from "./agent.js"; const ONE_BY_ONE_PNG_BASE64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+X1r0AAAAASUVORK5CYII="; +const MATERIALIZED_PNG_PATH_PATTERN = + /paseo-attachments(?:-[^\\/]+)?[\\/](?:[^\\/]+[\\/])?[0-9a-f]{64}\.png$/; interface ClaudeImageTestSession { translateMessageToEvents(message: SDKMessage): AgentStreamEvent[]; @@ -163,12 +165,17 @@ describe("Claude tool_result image rendering", () => { const [imageMessage, ...extraImages] = imageMessages(timelineItems); expect(extraImages).toEqual([]); - const source = markdownImageSource(imageMessage); - expect(source).toMatch(/paseo-attachments[\\/][0-9a-f]{64}\.png$/); - expect(existsSync(source)).toBe(true); - expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); - - rmSync(source, { force: true }); + let source: string | undefined; + try { + source = markdownImageSource(imageMessage); + expect(source).toMatch(MATERIALIZED_PNG_PATH_PATTERN); + expect(existsSync(source)).toBe(true); + expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); + } finally { + if (source) { + rmSync(source, { force: true }); + } + } }); test("replays the image as assistant markdown through history conversion", async () => { @@ -179,12 +186,17 @@ describe("Claude tool_result image rendering", () => { const [imageMessage, ...extraImages] = imageMessages(items); expect(extraImages).toEqual([]); - const source = markdownImageSource(imageMessage); - expect(source).toMatch(/paseo-attachments[\\/][0-9a-f]{64}\.png$/); - expect(existsSync(source)).toBe(true); - expect(JSON.stringify(items)).not.toContain(ONE_BY_ONE_PNG_BASE64); - - rmSync(source, { force: true }); + let source: string | undefined; + try { + source = markdownImageSource(imageMessage); + expect(source).toMatch(MATERIALIZED_PNG_PATH_PATTERN); + expect(existsSync(source)).toBe(true); + expect(JSON.stringify(items)).not.toContain(ONE_BY_ONE_PNG_BASE64); + } finally { + if (source) { + rmSync(source, { force: true }); + } + } }); test("keeps base64 out of an errored tool_result that carries an image", async () => { @@ -198,12 +210,17 @@ describe("Claude tool_result image rendering", () => { const [imageMessage, ...extraImages] = imageMessages(timelineItems); expect(extraImages).toEqual([]); - const source = markdownImageSource(imageMessage); - expect(existsSync(source)).toBe(true); - expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); - expect(JSON.stringify(events)).toContain("[image]"); - - rmSync(source, { force: true }); + let source: string | undefined; + try { + source = markdownImageSource(imageMessage); + expect(existsSync(source)).toBe(true); + expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); + expect(JSON.stringify(events)).toContain("[image]"); + } finally { + if (source) { + rmSync(source, { force: true }); + } + } }); test("emits one image message per image block in a multi-image tool_result", async () => { @@ -216,12 +233,16 @@ describe("Claude tool_result image rendering", () => { .map((event) => (event as { item: AgentTimelineItem }).item); const sources = imageMessages(timelineItems).map(markdownImageSource); - expect(sources).toHaveLength(2); - // Identical bytes materialize to one content-hashed file (idempotent), one message per block. - expect(new Set(sources).size).toBe(1); - expect(existsSync(sources[0])).toBe(true); - expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); - - rmSync(sources[0], { force: true }); + try { + expect(sources).toHaveLength(2); + // Identical bytes materialize to one content-hashed file (idempotent), one message per block. + expect(new Set(sources).size).toBe(1); + expect(existsSync(sources[0])).toBe(true); + expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); + } finally { + for (const source of sources) { + rmSync(source, { force: true }); + } + } }); }); 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 6e0d1d9c8..835d979a2 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 @@ -2354,7 +2354,7 @@ describe("Codex app-server provider", () => { expect(event.item.text).not.toContain("data:image"); expect(event.item.text).not.toContain(ONE_BY_ONE_PNG_BASE64); const source = markdownImageSource(event.item.text); - expect(source).toMatch(/paseo-attachments[\\/].+\.png$/); + expect(source).toMatch(/paseo-attachments(?:-[^\\/]+)?[\\/].+\.png$/); expect(existsSync(source)).toBe(true); rmSync(source, { force: true }); }); @@ -2470,7 +2470,7 @@ describe("Codex app-server provider", () => { } expect(JSON.stringify(events)).not.toContain(ONE_BY_ONE_PNG_BASE64); const source = markdownImageSource(imageEvent.item.text); - expect(source).toMatch(/paseo-attachments[\\/].+\.png$/); + expect(source).toMatch(/paseo-attachments(?:-[^\\/]+)?[\\/].+\.png$/); expect(existsSync(source)).toBe(true); rmSync(source, { force: true }); appServer.assertNoErrors(); diff --git a/packages/server/src/server/agent/providers/pi/agent.test.ts b/packages/server/src/server/agent/providers/pi/agent.test.ts index 5b6450b2e..3f1ef5d3f 100644 --- a/packages/server/src/server/agent/providers/pi/agent.test.ts +++ b/packages/server/src/server/agent/providers/pi/agent.test.ts @@ -6,6 +6,7 @@ import { mkdtempSync, openSync, readSync, + rmSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -17,6 +18,9 @@ import type { AgentSession, AgentSessionConfig, AgentStreamEvent } from "../../a import { PiRpcAgentClient, PiRpcAgentSession, transformPiModels } from "./agent.js"; import { FakePi } from "./test-utils/fake-pi.js"; +const ONE_BY_ONE_PNG_BASE64 = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/p9sAAAAASUVORK5CYII="; + function createClient(pi = new FakePi()): PiRpcAgentClient { return new PiRpcAgentClient({ logger: pino({ level: "silent" }), @@ -658,6 +662,91 @@ describe("PiRpcAgentSession", () => { expect(fakeSession.setThinkingLevelRequests).toEqual(["high"]); }); + test("materializes image prompts as text hints for text-only Pi models", async () => { + const { pi, session } = await createSession(); + const fakeSession = pi.latestSession(); + fakeSession.setModelResult = { + provider: "openrouter", + id: "openai/gpt-oss-20b:free", + name: "OpenAI: gpt-oss-20b (free)", + input: ["text"], + }; + + await session.setModel("openrouter/openai/gpt-oss-20b:free"); + await session.startTurn([ + { type: "text", text: "Describe this image." }, + { type: "image", data: ONE_BY_ONE_PNG_BASE64, mimeType: "image/png" }, + ]); + + let imagePath: string | undefined; + try { + expect(fakeSession.prompts).toHaveLength(1); + const prompt = fakeSession.prompts[0]!; + expect(prompt.imageCount).toBe(0); + expect(prompt.message).toContain("Describe this image."); + expect(prompt.message).not.toContain(ONE_BY_ONE_PNG_BASE64); + imagePath = prompt.message.match(/\[Image available at: (.+)\]/)?.[1]; + expect(imagePath).toBeTypeOf("string"); + expect(imagePath).toMatch( + /paseo-attachments(?:-[^\\/]+)?[\\/](?:[^\\/]+[\\/])?[0-9a-f]{64}\.png$/, + ); + expect(existsSync(imagePath!)).toBe(true); + } finally { + if (imagePath) { + rmSync(imagePath, { force: true }); + } + } + }); + + test("materializes image prompts when Pi model capabilities are unknown", async () => { + const { pi, session } = await createSession(); + const fakeSession = pi.latestSession(); + + await session.startTurn([ + { type: "text", text: "Describe this image." }, + { type: "image", data: ONE_BY_ONE_PNG_BASE64, mimeType: "image/png" }, + ]); + + let imagePath: string | undefined; + try { + expect(fakeSession.prompts).toHaveLength(1); + const prompt = fakeSession.prompts[0]!; + expect(prompt.imageCount).toBe(0); + expect(prompt.message).toContain("Describe this image."); + imagePath = prompt.message.match(/\[Image available at: (.+)\]/)?.[1]; + expect(imagePath).toBeTypeOf("string"); + expect(existsSync(imagePath!)).toBe(true); + } finally { + if (imagePath) { + rmSync(imagePath, { force: true }); + } + } + }); + + test("forwards raw image prompts for vision-capable Pi models", async () => { + const { pi, session } = await createSession(); + const fakeSession = pi.latestSession(); + fakeSession.setModelResult = { + provider: "openai", + id: "gpt-4o", + name: "GPT-4o", + input: ["text", "image"], + }; + + await session.setModel("openai/gpt-4o"); + await session.startTurn([ + { type: "text", text: "Describe this image." }, + { type: "image", data: ONE_BY_ONE_PNG_BASE64, mimeType: "image/png" }, + ]); + + expect(fakeSession.prompts).toEqual([ + { + message: "Describe this image.", + imageCount: 1, + }, + ]); + }); + test("fails the active turn when the Pi process exits mid-turn", async () => { const { pi, session, events } = await createSession(); diff --git a/packages/server/src/server/agent/providers/pi/agent.ts b/packages/server/src/server/agent/providers/pi/agent.ts index affcac79d..ec16b765e 100644 --- a/packages/server/src/server/agent/providers/pi/agent.ts +++ b/packages/server/src/server/agent/providers/pi/agent.ts @@ -56,6 +56,7 @@ import { streamPiHistory, type PiCapturedUserMessageEntry, } from "./history-mapper.js"; +import { materializeProviderImage } from "../provider-image-output.js"; import { PiCliRuntime } from "./cli-runtime.js"; import { revertPiConversation } from "./rewind.js"; import { listPiImportableSessions, readPiImportSessionConfig } from "./session-descriptor.js"; @@ -362,13 +363,33 @@ function toAgentUsage(stats: PiSessionStats): AgentUsage | undefined { }; } -function convertPromptInput(prompt: AgentPromptInput): PiPromptPayload { +function piModelSupportsImageInput(model: PiModel | null | undefined): boolean { + return model?.input?.includes("image") === true; +} + +function renderTextOnlyImageHint(image: { data: string; mimeType: string }): string { + try { + const materialized = materializeProviderImage({ + data: image.data, + mimeType: image.mimeType, + }); + return `[Image available at: ${materialized.path}]`; + } catch (error) { + return `[Image attachment omitted: failed to write local file (${toDiagnosticErrorMessage(error)})]`; + } +} + +function convertPromptInput( + prompt: AgentPromptInput, + options: { model: PiModel | null | undefined }, +): PiPromptPayload { if (typeof prompt === "string") { return { text: prompt }; } const textParts: string[] = []; const images: PiImageContent[] = []; + const forwardImages = piModelSupportsImageInput(options.model); for (const block of prompt) { if (block.type === "text") { @@ -377,11 +398,15 @@ function convertPromptInput(prompt: AgentPromptInput): PiPromptPayload { } if (block.type === "image") { - images.push({ - type: "image", - data: block.data, - mimeType: block.mimeType, - }); + if (forwardImages) { + images.push({ + type: "image", + data: block.data, + mimeType: block.mimeType, + }); + } else { + textParts.push(renderTextOnlyImageHint(block)); + } continue; } @@ -1046,7 +1071,7 @@ export class PiRpcAgentSession implements AgentSession { throw new Error("A Pi turn is already active"); } - const payload = convertPromptInput(prompt); + const payload = convertPromptInput(prompt, { model: this.state.model }); const turnId = randomUUID(); this.activeTurnId = turnId; diff --git a/packages/server/src/server/agent/providers/provider-image-output.posix.test.ts b/packages/server/src/server/agent/providers/provider-image-output.posix.test.ts new file mode 100644 index 000000000..1f4da6e04 --- /dev/null +++ b/packages/server/src/server/agent/providers/provider-image-output.posix.test.ts @@ -0,0 +1,24 @@ +import { existsSync, rmSync, statSync } from "node:fs"; +import path from "node:path"; +import { describe, expect, test } from "vitest"; + +import { materializeProviderImage } from "./provider-image-output.js"; + +describe.skipIf(process.platform === "win32")("materializeProviderImage", () => { + test("writes image attachments under a private temp directory", () => { + const materialized = materializeProviderImage({ + data: "YWJjMTIz", + mimeType: "image/png", + }); + const attachmentDir = path.dirname(materialized.path); + + try { + expect(path.basename(attachmentDir)).toMatch(/^paseo-attachments-/); + expect(existsSync(materialized.path)).toBe(true); + expect(statSync(attachmentDir).mode & 0o777).toBe(0o700); + expect(statSync(materialized.path).mode & 0o777).toBe(0o600); + } finally { + rmSync(attachmentDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/server/src/server/agent/providers/provider-image-output.test.ts b/packages/server/src/server/agent/providers/provider-image-output.test.ts index 71d28e1d4..49ffcf94b 100644 --- a/packages/server/src/server/agent/providers/provider-image-output.test.ts +++ b/packages/server/src/server/agent/providers/provider-image-output.test.ts @@ -1,14 +1,17 @@ +import { existsSync, rmSync } from "node:fs"; +import path from "node:path"; import { describe, expect, test } from "vitest"; import { isProviderImageMarkdown, + materializeProviderImage, renderProviderImageOutputAsAssistantMarkdown, } from "./provider-image-output.js"; const HASH = "a".repeat(64); -function renderImageMarkdown(path: string): string { - const item = renderProviderImageOutputAsAssistantMarkdown({ path }); +function renderImageMarkdown(imagePath: string): string { + const item = renderProviderImageOutputAsAssistantMarkdown({ path: imagePath }); if (!item || item.type !== "assistant_message") { throw new Error("Expected provider image output to render as assistant markdown."); } @@ -18,6 +21,12 @@ function renderImageMarkdown(path: string): string { describe("isProviderImageMarkdown", () => { test("matches the markdown emitted for a materialized attachment", () => { expect(isProviderImageMarkdown(`![Image](/tmp/paseo-attachments/${HASH}.png)`)).toBe(true); + expect(isProviderImageMarkdown(`![Image](/tmp/paseo-attachments-a1B2c3/${HASH}.png)`)).toBe( + true, + ); + expect(isProviderImageMarkdown(`![Image](/tmp/paseo-attachments/user-1000/${HASH}.png)`)).toBe( + true, + ); expect(isProviderImageMarkdown(`![shot](/var/folders/x/paseo-attachments/${HASH}.webp)`)).toBe( true, ); @@ -48,3 +57,28 @@ describe("isProviderImageMarkdown", () => { expect(isProviderImageMarkdown("see the chart: ![chart](x.png)")).toBe(false); }); }); + +describe("materializeProviderImage", () => { + test("recreates the private temp directory if the cached directory is removed", () => { + const first = materializeProviderImage({ + data: "YWJjMTIz", + mimeType: "image/png", + }); + const firstDir = path.dirname(first.path); + expect(existsSync(first.path)).toBe(true); + + rmSync(firstDir, { recursive: true, force: true }); + + const second = materializeProviderImage({ + data: "ZGVmNDU2", + mimeType: "image/png", + }); + const secondDir = path.dirname(second.path); + + try { + expect(existsSync(second.path)).toBe(true); + } finally { + rmSync(secondDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/server/src/server/agent/providers/provider-image-output.ts b/packages/server/src/server/agent/providers/provider-image-output.ts index d02c58185..649e5bdfe 100644 --- a/packages/server/src/server/agent/providers/provider-image-output.ts +++ b/packages/server/src/server/agent/providers/provider-image-output.ts @@ -18,6 +18,39 @@ export interface MaterializedProviderImage { } const PROVIDER_IMAGE_ATTACHMENT_DIR = "paseo-attachments"; +const PROVIDER_IMAGE_ATTACHMENT_DIR_PREFIX = `${PROVIDER_IMAGE_ATTACHMENT_DIR}-`; +const PRIVATE_ATTACHMENT_DIR_MODE = 0o700; +const MATERIALIZED_IMAGE_FILE_MODE = 0o600; + +let materializedImageAttachmentDir: string | null = null; + +function canReuseMaterializedImageAttachmentDir(dir: string): boolean { + try { + const stats = fsSync.lstatSync(dir); + if (!stats.isDirectory()) { + return false; + } + fsSync.chmodSync(dir, PRIVATE_ATTACHMENT_DIR_MODE); + return true; + } catch { + return false; + } +} + +function getMaterializedImageAttachmentDir(): string { + if ( + materializedImageAttachmentDir && + canReuseMaterializedImageAttachmentDir(materializedImageAttachmentDir) + ) { + return materializedImageAttachmentDir; + } + + materializedImageAttachmentDir = fsSync.mkdtempSync( + path.join(os.tmpdir(), PROVIDER_IMAGE_ATTACHMENT_DIR_PREFIX), + ); + fsSync.chmodSync(materializedImageAttachmentDir, PRIVATE_ATTACHMENT_DIR_MODE); + return materializedImageAttachmentDir; +} function getImageExtension(mimeType: string): string { switch (mimeType) { @@ -49,20 +82,20 @@ function normalizeImageData(mimeType: string, data: string): { mimeType: string; } // Filenames are a content hash of the bytes so re-materializing the same image -// is idempotent: history replay reuses the existing temp file instead of leaking -// a fresh one on every load. +// within a process reuses the existing temp file instead of leaking a fresh one +// for repeated image blocks or history replay. export function materializeProviderImage(image: { data: string; mimeType: string | null; }): MaterializedProviderImage { - const attachmentsDir = path.join(os.tmpdir(), PROVIDER_IMAGE_ATTACHMENT_DIR); - fsSync.mkdirSync(attachmentsDir, { recursive: true }); + const attachmentsDir = getMaterializedImageAttachmentDir(); const normalized = normalizeImageData(image.mimeType ?? "image/png", image.data); const bytes = Buffer.from(normalized.data, "base64"); const extension = getImageExtension(normalized.mimeType); const hash = createHash("sha256").update(bytes).digest("hex"); const filePath = path.join(attachmentsDir, `${hash}.${extension}`); - fsSync.writeFileSync(filePath, bytes); + fsSync.writeFileSync(filePath, bytes, { mode: MATERIALIZED_IMAGE_FILE_MODE }); + fsSync.chmodSync(filePath, MATERIALIZED_IMAGE_FILE_MODE); return { path: filePath }; } @@ -71,7 +104,7 @@ export function materializeProviderImage(image: { // keeps user-authored text from being mistaken for a provider image during history replay. The // separator still accepts old doubled-backslash Windows history; new Windows output uses file URIs. const PROVIDER_IMAGE_MARKDOWN = new RegExp( - `^!\\[[^\\]]*\\]\\([^)]*${PROVIDER_IMAGE_ATTACHMENT_DIR}[/\\\\]+[0-9a-f]{64}\\.[a-z0-9]+\\)`, + `^!\\[[^\\]]*\\]\\([^)]*${PROVIDER_IMAGE_ATTACHMENT_DIR}(?:-[^/\\\\)]+)?[/\\\\]+(?:[^/\\\\)]+[/\\\\]+)?[0-9a-f]{64}\\.[a-z0-9]+\\)`, ); export function isProviderImageMarkdown(text: string): boolean {