mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-26 19:22:51 +00:00
fix(server): materialize provider images under the paseo home
Provider images were materialized into a per-process directory created with mkdtemp(os.tmpdir(), "paseo-attachments-"). The OS reaps that location -- macOS clears /var/folders after roughly three days -- so every image in a transcript older than the reap window was silently orphaned: the stored markdown still pointed at a path whose bytes no longer existed, and nothing could recreate them. Materialize into resolvePaseoHome()/paseo-attachments/ instead. The directory keeps its 0700 mode and the files their 0600 mode, and the basename is unchanged so PROVIDER_IMAGE_MARKDOWN still matches history written by older builds (the optional "-<suffix>" group covers the old mkdtemp shape). A stable directory is no longer self-limiting, so add a best-effort sweep of entries older than 30 days, run once per process on the first image materialized. Re-materializing an image rewrites it and refreshes its mtime, so images still being rendered keep themselves alive and only genuinely cold ones age out. This also makes the existing content-hash filename deduplication work across daemon restarts; previously it only held within a single process. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
d1ce2b77f7
commit
14ec5a8ae9
@@ -1,24 +1,49 @@
|
||||
import { existsSync, rmSync, statSync } from "node:fs";
|
||||
import { existsSync, mkdtempSync, rmSync, statSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, test } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, test } from "vitest";
|
||||
|
||||
import { materializeProviderImage } from "./provider-image-output.js";
|
||||
import {
|
||||
__resetMaterializedImageAttachmentDirForTests,
|
||||
materializeProviderImage,
|
||||
} from "./provider-image-output.js";
|
||||
|
||||
describe.skipIf(process.platform === "win32")("materializeProviderImage", () => {
|
||||
test("writes image attachments under a private temp directory", () => {
|
||||
const originalPaseoHome = process.env.PASEO_HOME;
|
||||
let testHome: string | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
testHome = mkdtempSync(path.join(os.tmpdir(), "paseo-provider-images-posix-"));
|
||||
process.env.PASEO_HOME = testHome;
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalPaseoHome === undefined) {
|
||||
delete process.env.PASEO_HOME;
|
||||
} else {
|
||||
process.env.PASEO_HOME = originalPaseoHome;
|
||||
}
|
||||
if (testHome) {
|
||||
rmSync(testHome, { recursive: true, force: true });
|
||||
testHome = null;
|
||||
}
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
});
|
||||
|
||||
test("writes image attachments under a private directory in the paseo home", () => {
|
||||
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 });
|
||||
}
|
||||
// Exactly "paseo-attachments" — no mkdtemp suffix. The old per-process temp
|
||||
// directory is what let the OS reap these images out from under the app.
|
||||
expect(path.basename(attachmentDir)).toBe("paseo-attachments");
|
||||
expect(attachmentDir).toBe(path.join(testHome as string, "paseo-attachments"));
|
||||
expect(existsSync(materialized.path)).toBe(true);
|
||||
expect(statSync(attachmentDir).mode & 0o777).toBe(0o700);
|
||||
expect(statSync(materialized.path).mode & 0o777).toBe(0o600);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { existsSync, rmSync } from "node:fs";
|
||||
import { existsSync, mkdtempSync, readdirSync, rmSync, statSync, utimesSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, test } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, test } from "vitest";
|
||||
|
||||
import {
|
||||
__resetMaterializedImageAttachmentDirForTests,
|
||||
isProviderImageMarkdown,
|
||||
materializeProviderImage,
|
||||
renderProviderImageOutputAsAssistantMarkdown,
|
||||
@@ -10,6 +12,25 @@ import {
|
||||
|
||||
const HASH = "a".repeat(64);
|
||||
|
||||
// Materialized images live under the paseo home now, so without this every test
|
||||
// run would read and delete the developer's real provider images.
|
||||
const originalPaseoHome = process.env.PASEO_HOME;
|
||||
let testHome: string | null = null;
|
||||
|
||||
function useTempPaseoHome(): string {
|
||||
testHome = mkdtempSync(path.join(os.tmpdir(), "paseo-provider-images-"));
|
||||
process.env.PASEO_HOME = testHome;
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
return testHome;
|
||||
}
|
||||
|
||||
function attachmentsDir(): string {
|
||||
if (!testHome) {
|
||||
throw new Error("useTempPaseoHome() must run before reading the attachments dir.");
|
||||
}
|
||||
return path.join(testHome, "paseo-attachments");
|
||||
}
|
||||
|
||||
function renderImageMarkdown(imagePath: string): string {
|
||||
const item = renderProviderImageOutputAsAssistantMarkdown({ path: imagePath });
|
||||
if (!item || item.type !== "assistant_message") {
|
||||
@@ -90,26 +111,99 @@ describe("isProviderImageMarkdown", () => {
|
||||
});
|
||||
|
||||
describe("materializeProviderImage", () => {
|
||||
test("recreates the private temp directory if the cached directory is removed", () => {
|
||||
beforeEach(() => {
|
||||
useTempPaseoHome();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalPaseoHome === undefined) {
|
||||
delete process.env.PASEO_HOME;
|
||||
} else {
|
||||
process.env.PASEO_HOME = originalPaseoHome;
|
||||
}
|
||||
if (testHome) {
|
||||
rmSync(testHome, { recursive: true, force: true });
|
||||
testHome = null;
|
||||
}
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
});
|
||||
|
||||
test("recreates the private 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 });
|
||||
rmSync(path.dirname(first.path), { 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 });
|
||||
expect(existsSync(second.path)).toBe(true);
|
||||
});
|
||||
|
||||
test("stores images in a stable directory under the paseo home", () => {
|
||||
const result = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
|
||||
// A fixed directory, not a per-process mkdtemp one — the old shape is what
|
||||
// let the OS reap these images out from under the app after ~3 days.
|
||||
expect(path.dirname(result.path)).toBe(attachmentsDir());
|
||||
expect(path.basename(path.dirname(result.path))).toBe("paseo-attachments");
|
||||
expect(statSync(attachmentsDir()).mode & 0o777).toBe(0o700);
|
||||
expect(statSync(result.path).mode & 0o777).toBe(0o600);
|
||||
});
|
||||
|
||||
test("is idempotent for the same bytes across daemon restarts", () => {
|
||||
const first = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
|
||||
// A fresh daemon process: module state is cleared but the directory persists.
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
const second = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
|
||||
expect(second.path).toBe(first.path);
|
||||
expect(readdirSync(attachmentsDir())).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("emits markdown that isProviderImageMarkdown still recognizes", () => {
|
||||
const item = renderProviderImageOutputAsAssistantMarkdown(
|
||||
{ data: "YWJjMTIz", mimeType: "image/png" },
|
||||
{ materialize: materializeProviderImage },
|
||||
);
|
||||
|
||||
if (!item || item.type !== "assistant_message") {
|
||||
throw new Error("Expected provider image output to render as assistant markdown.");
|
||||
}
|
||||
expect(isProviderImageMarkdown(item.text)).toBe(true);
|
||||
});
|
||||
|
||||
test("sweeps images older than the retention window and keeps fresh ones", () => {
|
||||
const stale = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
const fresh = materializeProviderImage({ data: "ZGVmNDU2", mimeType: "image/png" });
|
||||
|
||||
const longAgoSeconds = Date.now() / 1000 - 31 * 24 * 60 * 60;
|
||||
utimesSync(stale.path, longAgoSeconds, longAgoSeconds);
|
||||
|
||||
// The sweep runs once per process, on the first image materialized.
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
materializeProviderImage({ data: "Z2hpNzg5", mimeType: "image/png" });
|
||||
|
||||
expect(existsSync(stale.path)).toBe(false);
|
||||
expect(existsSync(fresh.path)).toBe(true);
|
||||
});
|
||||
|
||||
test("re-materializing a stale image keeps it alive", () => {
|
||||
const image = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
const longAgoSeconds = Date.now() / 1000 - 31 * 24 * 60 * 60;
|
||||
utimesSync(image.path, longAgoSeconds, longAgoSeconds);
|
||||
|
||||
// Rendering it again rewrites the bytes, refreshing mtime before any sweep.
|
||||
materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
|
||||
__resetMaterializedImageAttachmentDirForTests();
|
||||
materializeProviderImage({ data: "Z2hpNzg5", mimeType: "image/png" });
|
||||
|
||||
expect(existsSync(image.path)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import * as fsSync from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import type { AgentTimelineItem } from "../agent-sdk-types.js";
|
||||
import { resolvePaseoHome } from "../../paseo-home.js";
|
||||
|
||||
export interface ProviderImageOutput {
|
||||
path?: string | null;
|
||||
@@ -17,12 +17,22 @@ export interface MaterializedProviderImage {
|
||||
path: string;
|
||||
}
|
||||
|
||||
// The directory basename stays "paseo-attachments" because PROVIDER_IMAGE_MARKDOWN
|
||||
// below matches on it, and history written by older builds used
|
||||
// mkdtemp(os.tmpdir(), "paseo-attachments-") — the optional "-<suffix>" group in
|
||||
// that regex is what lets one pattern match both shapes.
|
||||
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;
|
||||
// Materialized images used to live in os.tmpdir(), which the OS reaps (macOS
|
||||
// clears /var/folders after ~3 days). That silently orphaned every image in any
|
||||
// transcript older than the reap window: the markdown still pointed at a path
|
||||
// whose bytes were gone, and nothing could re-create them. They now live under
|
||||
// the paseo home, so this sweep is what bounds the directory instead.
|
||||
const MATERIALIZED_IMAGE_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000;
|
||||
|
||||
let materializedImageAttachmentDir: string | null = null;
|
||||
let sweptMaterializedImageAttachments = false;
|
||||
|
||||
function canReuseMaterializedImageAttachmentDir(dir: string): boolean {
|
||||
try {
|
||||
@@ -37,6 +47,38 @@ function canReuseMaterializedImageAttachmentDir(dir: string): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
// Runs once per process, on the first image materialized. Re-materializing an
|
||||
// image rewrites it, refreshing mtime, so an image still being rendered keeps
|
||||
// itself alive and only genuinely cold ones age out.
|
||||
function sweepMaterializedImageAttachments(dir: string): void {
|
||||
if (sweptMaterializedImageAttachments) {
|
||||
return;
|
||||
}
|
||||
sweptMaterializedImageAttachments = true;
|
||||
|
||||
const cutoffMs = Date.now() - MATERIALIZED_IMAGE_MAX_AGE_MS;
|
||||
let entries: fsSync.Dirent[];
|
||||
try {
|
||||
entries = fsSync.readdirSync(dir, { withFileTypes: true });
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const entry of entries) {
|
||||
if (!entry.isFile()) {
|
||||
continue;
|
||||
}
|
||||
const filePath = path.join(dir, entry.name);
|
||||
try {
|
||||
if (fsSync.statSync(filePath).mtimeMs < cutoffMs) {
|
||||
fsSync.rmSync(filePath, { force: true });
|
||||
}
|
||||
} catch {
|
||||
// Raced with another writer or already gone. Sweeping is best-effort.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function getMaterializedImageAttachmentDir(): string {
|
||||
if (
|
||||
materializedImageAttachmentDir &&
|
||||
@@ -45,11 +87,18 @@ function getMaterializedImageAttachmentDir(): string {
|
||||
return materializedImageAttachmentDir;
|
||||
}
|
||||
|
||||
materializedImageAttachmentDir = fsSync.mkdtempSync(
|
||||
path.join(os.tmpdir(), PROVIDER_IMAGE_ATTACHMENT_DIR_PREFIX),
|
||||
);
|
||||
fsSync.chmodSync(materializedImageAttachmentDir, PRIVATE_ATTACHMENT_DIR_MODE);
|
||||
return materializedImageAttachmentDir;
|
||||
const dir = path.join(resolvePaseoHome(), PROVIDER_IMAGE_ATTACHMENT_DIR);
|
||||
fsSync.mkdirSync(dir, { recursive: true, mode: PRIVATE_ATTACHMENT_DIR_MODE });
|
||||
fsSync.chmodSync(dir, PRIVATE_ATTACHMENT_DIR_MODE);
|
||||
materializedImageAttachmentDir = dir;
|
||||
sweepMaterializedImageAttachments(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
/** Test-only hook: the sweep and the resolved directory are process-scoped. */
|
||||
export function __resetMaterializedImageAttachmentDirForTests(): void {
|
||||
materializedImageAttachmentDir = null;
|
||||
sweptMaterializedImageAttachments = false;
|
||||
}
|
||||
|
||||
function getImageExtension(mimeType: string): string {
|
||||
@@ -81,9 +130,11 @@ function normalizeImageData(mimeType: string, data: string): { mimeType: string;
|
||||
return { mimeType, data };
|
||||
}
|
||||
|
||||
// Filenames are a content hash of the bytes so re-materializing the same image
|
||||
// within a process reuses the existing temp file instead of leaking a fresh one
|
||||
// for repeated image blocks or history replay.
|
||||
// Filenames are a content hash of the bytes, and the directory is now stable
|
||||
// across daemon restarts, so re-materializing the same image reuses the existing
|
||||
// file instead of leaking a fresh one for repeated image blocks or history
|
||||
// replay. Under the old per-process temp directory this deduplication only held
|
||||
// within a single daemon process.
|
||||
export function materializeProviderImage(image: {
|
||||
data: string;
|
||||
mimeType: string | null;
|
||||
|
||||
Reference in New Issue
Block a user