From 43de93a39cdff3e4cfd211b3e1deaf0bc917b359 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 3 Jul 2026 03:01:18 -0700 Subject: [PATCH] fix(cli): add snapshot --no-clean to stop wiping prior output Every `snapshot` run unconditionally deleted existing .png/.jpg files in the output directory before capturing new ones, so two consecutive runs with different --at sets clobbered each other with no way to opt out. Extracted the wipe into cleanSnapshotDir(), matching the existing computeSnapshotTimes/tailFrameTime extraction pattern in this file so it's unit-testable directly, and gated it behind a new --clean flag (default true, preserving current behavior) following the same default-true/ --no-x-to-opt-out convention already established by --end/--no-end. --- packages/cli/src/commands/snapshot.test.ts | 43 ++++++++++++++++++- packages/cli/src/commands/snapshot.ts | 48 +++++++++++++++++----- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/commands/snapshot.test.ts b/packages/cli/src/commands/snapshot.test.ts index 0537f1705d..a54098b92f 100644 --- a/packages/cli/src/commands/snapshot.test.ts +++ b/packages/cli/src/commands/snapshot.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from "vitest"; -import { computeSnapshotTimes, tailFrameTime } from "./snapshot.js"; +import { mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { cleanSnapshotDir, computeSnapshotTimes, tailFrameTime } from "./snapshot.js"; describe("tailFrameTime", () => { it("backs off ~3% of duration so the final frame isn't the blank exact-end", () => { @@ -59,3 +62,41 @@ describe("computeSnapshotTimes (FINDING [7]: tail is always captured)", () => { expect(appendedTail).toBe(false); }); }); + +// Regression: every `snapshot` run unconditionally wiped the output +// directory's existing .png/.jpg files before capturing new ones, so two +// consecutive runs with different --at sets clobbered each other with no +// way to opt out. cleanSnapshotDir is now a separate, callable-or-skippable +// step (see the --clean/--no-clean flag) instead of being inlined and +// unconditional. +describe("cleanSnapshotDir", () => { + it("deletes existing image files in the directory", () => { + const dir = mkdtempSync(join(tmpdir(), "snapshot-clean-")); + writeFileSync(join(dir, "frame-1.png"), "stub"); + writeFileSync(join(dir, "frame-2.jpg"), "stub"); + try { + cleanSnapshotDir(dir); + expect(readdirSync(dir)).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("only removes image files, leaving other files untouched", () => { + const dir = mkdtempSync(join(tmpdir(), "snapshot-clean-scope-")); + writeFileSync(join(dir, "frame.png"), "stub"); + writeFileSync(join(dir, "descriptions.md"), "keep me"); + try { + cleanSnapshotDir(dir); + expect(readdirSync(dir)).toEqual(["descriptions.md"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("does not throw when the directory does not exist", () => { + expect(() => + cleanSnapshotDir(join(tmpdir(), "snapshot-clean-missing-nonexistent")), + ).not.toThrow(); + }); +}); diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts index 42d408659f..e18be1727a 100644 --- a/packages/cli/src/commands/snapshot.ts +++ b/packages/cli/src/commands/snapshot.ts @@ -1,7 +1,15 @@ // fallow-ignore-file complexity import { spawn } from "node:child_process"; import { defineCommand } from "citty"; -import { existsSync, mkdtempSync, readFileSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdtempSync, + readdirSync, + readFileSync, + mkdirSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { resolve, join, relative, isAbsolute, basename } from "node:path"; import { resolveProject } from "../utils/project.js"; @@ -177,6 +185,25 @@ export function computeSnapshotTimes( return { times: times.map(round), appendedTail: false }; } +/** + * Deletes existing .png/.jpg/.jpeg files from a snapshot output directory. + * Pure enough to unit test directly: two consecutive `snapshot` runs with + * different --at sets otherwise clobber each other's output with no way to + * opt out, since every run wiped the directory unconditionally before this + * was made a separate, disableable step (see the `clean` flag). + */ +export function cleanSnapshotDir(dir: string): void { + try { + for (const file of readdirSync(dir)) { + if (/\.(png|jpg|jpeg)$/i.test(file)) { + rmSync(join(dir, file), { force: true }); + } + } + } catch { + /* best-effort — proceed even if cleanup fails */ + } +} + /** * Render key frames from a composition as PNG screenshots. * The agent can Read these to verify its output visually. @@ -190,6 +217,7 @@ async function captureSnapshots( outputDir?: string; angle?: Camera; includeEnd?: boolean; + clean?: boolean; }, ): Promise { const { bundleToSingleHtml } = await import("@hyperframes/core/compiler"); @@ -343,16 +371,7 @@ async function captureSnapshots( const snapshotDir = opts.outputDir ?? join(projectDir, "snapshots"); mkdirSync(snapshotDir, { recursive: true }); - try { - const { readdirSync } = await import("node:fs"); - for (const file of readdirSync(snapshotDir)) { - if (/\.(png|jpg|jpeg)$/i.test(file)) { - rmSync(join(snapshotDir, file), { force: true }); - } - } - } catch { - /* best-effort — proceed even if cleanup fails */ - } + if (opts.clean !== false) cleanSnapshotDir(snapshotDir); // Chrome-headless ignores programmatic