puter/paseo
1
0
Fork 0
mirror of https://github.com/getpaseo/paseo.git synced 2026-10-26 19:22:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(server): stop sweeping materialized images by age

Browse Source 
The 30-day sweep reintroduced the bug this PR fixes, on a longer fuse.
mtime cannot distinguish an image a transcript still references from an
abandoned one, and reading a transcript never touches the file, so an
image you still look at ages out and disappears.

It was also a regression on hosts whose temp dir is not reaped on a
schedule. There these images survived indefinitely; adding our own
deleter made them mortal.

Nothing deletes them now. Content addressing bounds growth to distinct
images rather than renders. Bounding the directory properly needs to be
reference-aware, against the agent timelines that point into it.
This commit is contained in:
Christoph Leiter
2026-07-30 06:05:50 +02:00
parent ba42900d1a
commit a6f975f49a
2 changed files with 24 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/server/src/server/agent/providers/provider-image-output.test.ts
+8 -19
View File
@@ -179,28 +179,17 @@ describe("materializeProviderImage", () => {
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", () => {
// The whole point of moving out of the temp dir is that nothing deletes these
// behind the user's back. An age-based sweep would reintroduce that on a
// longer fuse, because mtime cannot tell a referenced image from an abandoned
// one and reading a transcript never touches the file.
test("keeps images no matter how old they are", () => {
const image = materializeProviderImage({ data: "YWJjMTIz", mimeType: "image/png" });
const longAgoSeconds = Date.now() / 1000 - 31 * 24 * 60 * 60;
const longAgoSeconds = Date.now() / 1000 - 365 * 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" });
// A fresh daemon process materializing an unrelated image: the oldest point
// at which a startup sweep would run.
__resetMaterializedImageAttachmentDirForTests();
materializeProviderImage({ data: "Z2hpNzg5", mimeType: "image/png" });
packages/server/src/server/agent/providers/provider-image-output.ts
+16 -41
View File
@@ -24,15 +24,23 @@ export interface MaterializedProviderImage {
const PROVIDER_IMAGE_ATTACHMENT_DIR = "paseo-attachments";
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
// Materialized images used to live in os.tmpdir(), which the OS may reap (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;
// whose bytes were gone, and nothing could re-create them.
//
// Nothing deletes these now, deliberately. An age-based sweep here would only
// move the same bug to a longer fuse — mtime cannot distinguish an image a
// transcript still references from an abandoned one, and reading a transcript
// does not touch the file. It would also be a regression on hosts whose temp
// dir is not reaped on a schedule, where these images currently survive
// indefinitely. Bounding this directory needs to be reference-aware, against
// the agent timelines that point into it.
//
// Growth is bounded in practice by content addressing: filenames are a hash of
// the bytes, so re-emitting or replaying an image reuses its existing file.
let materializedImageAttachmentDir: string | null = null;
let sweptMaterializedImageAttachments = false;
function canReuseMaterializedImageAttachmentDir(dir: string): boolean {
try {
@@ -47,38 +55,6 @@ 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 &&
@@ -91,14 +67,12 @@ function getMaterializedImageAttachmentDir(): string {
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. */
/** Test-only hook: the resolved directory is process-scoped. */
export function __resetMaterializedImageAttachmentDirForTests(): void {
materializedImageAttachmentDir = null;
sweptMaterializedImageAttachments = false;
}
function getImageExtension(mimeType: string): string {
@@ -134,7 +108,8 @@ function normalizeImageData(mimeType: string, data: string): { mimeType: string;
// 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.
// within a single daemon process — which is also why this directory does not
// grow per render, only per distinct image.
export function materializeProviderImage(image: {
data: string;
mimeType: string | null;