fix(canvas): bound disk I/O in readCanvases when agents emit many files (greptile P1)
Pre-fix: every .json file in .ao/canvases/ was lstat-ed, read, JSON-parsed,
and Zod-validated before slice(0, 32) was applied. A buggy agent emitting
1000 canvas files at 256KB cap each would trigger 256MB of disk I/O on
every 5-second poll.
Fix: two-pass discovery + capped read loop:
1. Cheap pass — lstat every .json, filter non-regular + oversized files,
collect {name, mtimeMs}. No reads, no JSON.parse, no Zod.
2. Sort survivors by mtime desc (newest first). Preserves the "newest 32
wins" property the cap was meant to enforce.
3. Capped pass — read + parse + validate in mtime order, break early once
CANVAS_MAX_PER_SESSION valid canvases are collected. Hard ceiling at
CANVAS_MAX_PER_SESSION * 4 file reads so a pile of invalid newer files
can't starve the loop into reading everything anyway.
Final sort still uses canvas-supplied updatedAt (the UI ordering), which
may differ from mtime if an agent rewrites a file without bumping the
canvas's updatedAt.
Adds regression test: writes 200 canvas files with monotonic mtimes,
asserts result is the 32 newest by mtime (not arbitrary OS order).
20 canvas-log tests pass (19 -> 20).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
68a70747fc
commit
04785dd9c0
|
|
@ -91,6 +91,32 @@ describe("readCanvases", () => {
|
|||
expect(result[0]?.id).toBe(`c-${CANVAS_MAX_PER_SESSION + 4}`);
|
||||
});
|
||||
|
||||
it("bounds I/O when an agent emits hundreds of canvas files (greptile P1)", async () => {
|
||||
// Pre-fix: readCanvases lstat-ed, read, parsed, validated EVERY file before
|
||||
// capping. 200 files × 256 KB = 50+ MB of reads per 5s poll. Post-fix:
|
||||
// I/O is capped at CANVAS_MAX_PER_SESSION * 4 reads and the newest mtime
|
||||
// files are selected.
|
||||
const N = 200;
|
||||
for (let i = 0; i < N; i++) {
|
||||
const c: CanvasArtifact = {
|
||||
...validMarkdown,
|
||||
id: `c-${String(i).padStart(3, "0")}`,
|
||||
};
|
||||
const filePath = join(canvasDir, `c-${String(i).padStart(3, "0")}.json`);
|
||||
await writeFile(filePath, JSON.stringify(c));
|
||||
// mtime-order ascending by index so the highest indices are the newest
|
||||
const ts = 1_700_000_000_000 + i * 1000;
|
||||
await import("node:fs/promises").then((fs) => fs.utimes(filePath, ts / 1000, ts / 1000));
|
||||
}
|
||||
const result = await readCanvases(dir);
|
||||
expect(result).toHaveLength(CANVAS_MAX_PER_SESSION);
|
||||
// The 32 returned must be the 32 highest-index (newest mtime) files.
|
||||
const ids = new Set(result.map((c) => c.id));
|
||||
for (let i = N - CANVAS_MAX_PER_SESSION; i < N; i++) {
|
||||
expect(ids.has(`c-${String(i).padStart(3, "0")}`)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("drops file canvases that use the reserved core- prefix", async () => {
|
||||
const reserved: CanvasArtifact = { ...validMarkdown, id: "core-git-diff" };
|
||||
await writeFile(join(canvasDir, "evil.json"), JSON.stringify(reserved));
|
||||
|
|
|
|||
|
|
@ -38,12 +38,19 @@ export async function readCanvases(workspacePath: string): Promise<CanvasArtifac
|
|||
throw err;
|
||||
}
|
||||
|
||||
const canvases: CanvasArtifact[] = [];
|
||||
// Two-pass to bound I/O when a buggy agent emits 1000s of canvas files.
|
||||
// Pass 1 (cheap, all entries): lstat to filter non-regular + oversized,
|
||||
// collect {name, mtimeMs} for the survivors. lstat is cheap; readFile +
|
||||
// JSON.parse + Zod validate is the expensive part we want to bound.
|
||||
// Pass 2 (capped): sort by mtime desc, read + validate the newest, break
|
||||
// once CANVAS_MAX_PER_SESSION valid canvases are collected. Hard ceiling
|
||||
// CANVAS_MAX_PER_SESSION * 4 file reads so the loop can't be starved by a
|
||||
// pile of invalid newer files into reading everything anyway.
|
||||
type Candidate = { name: string; mtimeMs: number };
|
||||
const candidates: Candidate[] = [];
|
||||
for (const entry of entries) {
|
||||
if (!entry.endsWith(".json")) continue;
|
||||
const filePath = join(dir, entry);
|
||||
|
||||
let size: number;
|
||||
try {
|
||||
const st = await lstat(filePath);
|
||||
// Reject anything that isn't a regular file. lstat (not stat) avoids
|
||||
|
|
@ -53,14 +60,31 @@ export async function readCanvases(workspacePath: string): Promise<CanvasArtifac
|
|||
console.warn(`canvas: dropping ${entry} (not a regular file)`);
|
||||
continue;
|
||||
}
|
||||
size = st.size;
|
||||
if (st.size > CANVAS_MAX_FILE_BYTES) {
|
||||
console.warn(`canvas: dropping ${entry} (${st.size} bytes exceeds ${CANVAS_MAX_FILE_BYTES})`);
|
||||
continue;
|
||||
}
|
||||
candidates.push({ name: entry, mtimeMs: st.mtimeMs });
|
||||
} catch {
|
||||
continue;
|
||||
// file vanished between readdir and lstat, or unreadable — skip silently
|
||||
}
|
||||
if (size > CANVAS_MAX_FILE_BYTES) {
|
||||
console.warn(`canvas: dropping ${entry} (${size} bytes exceeds ${CANVAS_MAX_FILE_BYTES})`);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Newest-first so the read-loop can early-break at CANVAS_MAX_PER_SESSION
|
||||
// and still keep the newest files (the design intent of the cap).
|
||||
candidates.sort((a, b) => b.mtimeMs - a.mtimeMs);
|
||||
|
||||
const readBudget = CANVAS_MAX_PER_SESSION * 4;
|
||||
const canvases: CanvasArtifact[] = [];
|
||||
let reads = 0;
|
||||
for (const { name } of candidates) {
|
||||
if (canvases.length >= CANVAS_MAX_PER_SESSION) break;
|
||||
if (reads >= readBudget) {
|
||||
console.warn(`canvas: stopping at ${reads} reads (budget exhausted, ${candidates.length - reads} skipped)`);
|
||||
break;
|
||||
}
|
||||
reads++;
|
||||
const filePath = join(dir, name);
|
||||
|
||||
let raw: string;
|
||||
try {
|
||||
|
|
@ -73,27 +97,31 @@ export async function readCanvases(workspacePath: string): Promise<CanvasArtifac
|
|||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
} catch {
|
||||
console.warn(`canvas: dropping ${entry} (invalid JSON)`);
|
||||
console.warn(`canvas: dropping ${name} (invalid JSON)`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const result = CanvasArtifactSchema.safeParse(parsed);
|
||||
if (!result.success) {
|
||||
console.warn(`canvas: dropping ${entry} (schema invalid: ${result.error.issues[0]?.message ?? "unknown"})`);
|
||||
console.warn(`canvas: dropping ${name} (schema invalid: ${result.error.issues[0]?.message ?? "unknown"})`);
|
||||
continue;
|
||||
}
|
||||
// The "core-" id prefix is reserved for canvases synthesized by core
|
||||
// (e.g. core-git-diff). Dropping it here prevents an agent file canvas
|
||||
// from shadowing the trusted synthesized version in API responses.
|
||||
if (result.data.id.startsWith("core-")) {
|
||||
console.warn(`canvas: dropping ${entry} (id "${result.data.id}" uses reserved "core-" prefix)`);
|
||||
console.warn(`canvas: dropping ${name} (id "${result.data.id}" uses reserved "core-" prefix)`);
|
||||
continue;
|
||||
}
|
||||
canvases.push(result.data);
|
||||
}
|
||||
|
||||
// Final sort by updatedAt (the canvas-supplied timestamp). mtime ordering
|
||||
// was used to bound I/O; updatedAt is what the UI cares about and may
|
||||
// differ if an agent rewrites a file without changing the canvas's
|
||||
// updatedAt field.
|
||||
canvases.sort((a, b) => (a.updatedAt < b.updatedAt ? 1 : -1));
|
||||
return canvases.slice(0, CANVAS_MAX_PER_SESSION);
|
||||
return canvases;
|
||||
}
|
||||
|
||||
export async function synthesizeGitDiffCanvas(
|
||||
|
|
|
|||
Loading…
Reference in New Issue