mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
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
This commit is contained in:
@@ -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 });
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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(``)).toBe(true);
|
||||
expect(isProviderImageMarkdown(``)).toBe(
|
||||
true,
|
||||
);
|
||||
expect(isProviderImageMarkdown(``)).toBe(
|
||||
true,
|
||||
);
|
||||
expect(isProviderImageMarkdown(``)).toBe(
|
||||
true,
|
||||
);
|
||||
@@ -48,3 +57,28 @@ describe("isProviderImageMarkdown", () => {
|
||||
expect(isProviderImageMarkdown("see the chart: ")).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 });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user