Skip to content

feat(blueprint): add snapshot prune/delete commands for retention management #5453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vasanth53 wants to merge 6 commits into
base: main
Choose a base branch
from
+318 −1
Open

feat(blueprint): add snapshot prune/delete commands for retention management #5453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e76755
feat(blueprint): add snapshot prune/delete commands for retention man…
vasanth53 Jun 15, 2026
fd79ea0
Merge branch 'main' into feat/snapshot-retention-management
vasanth53 Jun 16, 2026
f46d36c
Merge branch 'main' into feat/snapshot-retention-management
vasanth53 Jun 18, 2026
87cc2fa
Merge branch 'main' into feat/snapshot-retention-management
vasanth53 Jun 19, 2026
bafcd45
Merge branch 'main' into feat/snapshot-retention-management
vasanth53 Jun 22, 2026
c174d90
Merge branch 'main' into feat/snapshot-retention-management
vasanth53 Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 109 additions & 1 deletion nemoclaw/src/blueprint/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import YAML from "yaml";
import { validateEndpointUrl } from "./ssrf.js";
import { buildSubprocessEnv } from "../lib/subprocess-env.js";
import { DASHBOARD_PORT } from "../lib/ports.js";
import { deleteSnapshot, listSnapshots, pruneSnapshots } from "./snapshot.js";

type Action = "plan" | "apply" | "status" | "rollback";
type SnapshotsAction = "list" | "prune" | "delete";

type RollbackPlanSource = { sandbox_name?: unknown };
type UnknownRecord = { [key: string]: unknown };
Expand Down Expand Up @@ -66,6 +68,10 @@ function isAction(value: string | undefined): value is Action {
return value === "plan" || value === "apply" || value === "status" || value === "rollback";
}

function isSnapshotsAction(value: string | undefined): value is SnapshotsAction {
return value === "list" || value === "prune" || value === "delete";
}

function isObjectLike(value: unknown): value is UnknownRecord {
if (typeof value !== "object" || value === null || Array.isArray(value)) {
return false;
Expand Down Expand Up @@ -951,8 +957,12 @@ export async function main(argv: string[] = process.argv.slice(2)): Promise<void
}

if (!action) {
if (rawAction === "snapshots") {
await actionSnapshots(argv.slice(1));
return;
}
throw new Error(
`Unknown action '${rawAction ?? "(missing)"}'. Use: plan, apply, status, rollback`,
`Unknown action '${rawAction ?? "(missing)"}'. Use: plan, apply, status, rollback, snapshots`,
);
}

Expand Down Expand Up @@ -998,3 +1008,101 @@ export async function main(argv: string[] = process.argv.slice(2)): Promise<void
break;
}
}

// ── Snapshot Management ─────────────────────────────────────────

function snapshotsUsage(): string {
return "Usage: snapshots <list|prune|delete> [options]\n" +
"\n" +
"Subcommands:\n" +
" list List all snapshots\n" +
" prune --keep <N> Keep N most recent snapshots, delete the rest\n" +
" delete --path <path> Delete a specific snapshot by path\n" +
"\n" +
"Examples:\n" +
" snapshots list\n" +
" snapshots prune --keep 3\n" +
" snapshots delete --path ~/.nemoclaw/snapshots/20260101T000000Z\n";
}

export function actionSnapshots(argv: string[]): void {
const sub = argv.at(0);

if (!sub || sub === "--help" || sub === "-h") {
log(snapshotsUsage());
return;
}

if (!isSnapshotsAction(sub)) {
throw new Error(
`Unknown snapshots subcommand '${sub}'. Use: list, prune, delete`,
);
}

switch (sub) {
case "list": {
const snapshots = listSnapshots();
if (snapshots.length === 0) {
log("No snapshots found.");
return;
}
log(`Found ${snapshots.length} snapshot(s):\n`);
for (const snap of snapshots) {
log(` ${snap.timestamp}`);
log(` Path: ${snap.path}`);
log(` Source: ${snap.source}`);
log(` Files: ${snap.file_count}`);
log("");
}
break;
}
case "prune": {
let keep = -1;
for (let i = 1; i < argv.length; i++) {
if (argv[i] === "--keep") {
const val = argv[++i];
if (val === undefined) throw new Error("--keep requires a numeric value");
keep = Number.parseInt(val, 10);
if (!Number.isFinite(keep) || keep < 0) {
throw new Error("--keep must be a non-negative integer");
Comment on lines +1065 to +1067

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

--keep validation accepts malformed values.

Line 1065 uses Number.parseInt, so inputs like --keep 3abc or --keep 1.5 are accepted as 3 / 1, despite the “non-negative integer” requirement.

Suggested fix 
-          keep = Number.parseInt(val, 10);
-          if (!Number.isFinite(keep) || keep < 0) {
+          if (!/^\d+$/.test(val)) {
+            throw new Error("--keep must be a non-negative integer");
+          }
+          keep = Number.parseInt(val, 10);
+          if (!Number.isSafeInteger(keep) || keep < 0) {
             throw new Error("--keep must be a non-negative integer");
           }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nemoclaw/src/blueprint/runner.ts` around lines 1065 - 1067, The
`Number.parseInt` function at the keep variable assignment performs lenient
parsing and will accept malformed inputs like "3abc" or "1.5" by silently
truncating them to 3 and 1 respectively. To enforce the "non-negative integer"
requirement strictly, validate that the entire input string represents a pure
integer with no trailing characters or decimal points. You can do this by
checking that the parsed keep value, when converted back to a string, matches
the original trimmed input value, or by validating the input format using a
regex pattern that matches only optional whitespace and non-negative integer
digits before attempting to parse.

}
}
}
if (keep < 0) throw new Error("--keep is required for prune");

const { deleted, kept, failed } = pruneSnapshots(keep);
if (deleted.length === 0 && failed.length === 0) {
log(`Nothing to prune. ${kept.length} snapshot(s) kept (--keep=${String(keep)}).`);
return;
}
if (deleted.length > 0) {
log(`Pruned ${deleted.length} snapshot(s), kept ${kept.length}:\n`);
for (const path of deleted) {
log(` Deleted: ${path}`);
}
}
if (failed.length > 0) {
for (const path of failed) {
log(` Failed: ${path}`);
}
}
break;
}
case "delete": {
let snapshotPath: string | undefined;
for (let i = 1; i < argv.length; i++) {
if (argv[i] === "--path") {
snapshotPath = argv[++i];
}
}
if (!snapshotPath) throw new Error("--path is required for delete");

if (deleteSnapshot(snapshotPath)) {
log(`Deleted snapshot: ${snapshotPath}`);
} else {
throw new Error(`Failed to delete snapshot: ${snapshotPath}`);
}
Comment on lines +1092 to +1104

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

delete --path allows recursive deletion outside snapshots directory.

Lines 1092-1104 pass user-provided --path straight into recursive deletion. Without constraining it to ~/.nemoclaw/snapshots, this can remove arbitrary directories the user has permissions for.

Suggested fix 
-import { join, sep } from "node:path";
+import { join, resolve, sep } from "node:path";
...
     case "delete": {
       let snapshotPath: string | undefined;
       for (let i = 1; i < argv.length; i++) {
         if (argv[i] === "--path") {
           snapshotPath = argv[++i];
         }
       }
       if (!snapshotPath) throw new Error("--path is required for delete");
+
+      const snapshotsRoot = resolve(join(homedir(), ".nemoclaw", "snapshots"));
+      const targetPath = resolve(snapshotPath);
+      if (!targetPath.startsWith(snapshotsRoot + sep)) {
+        throw new Error(`--path must be under ${snapshotsRoot}`);
+      }

-      if (deleteSnapshot(snapshotPath)) {
-        log(`Deleted snapshot: ${snapshotPath}`);
+      if (deleteSnapshot(targetPath)) {
+        log(`Deleted snapshot: ${targetPath}`);
       } else {
-        throw new Error(`Failed to delete snapshot: ${snapshotPath}`);
+        throw new Error(`Failed to delete snapshot: ${targetPath}`);
       }
       break;
     }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nemoclaw/src/blueprint/runner.ts` around lines 1092 - 1104, The delete
command accepts a user-provided --path argument that is passed directly to
deleteSnapshot without validation, allowing deletion of arbitrary directories
outside the snapshots directory. After extracting snapshotPath from argv (around
line 1095), add validation to ensure the path is constrained to the snapshots
directory by resolving it to an absolute path and verifying it starts with the
expected snapshots directory path (typically ~/.nemoclaw/snapshots). Reject the
command with an appropriate error message if the resolved path attempts to
escape the snapshots directory. This validation must occur before the
deleteSnapshot function is called to prevent unauthorized recursive deletion of
directories outside the snapshots location.

break;
}
}
}
177 changes: 177 additions & 0 deletions nemoclaw/src/blueprint/snapshot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,12 @@ const {
rollbackFromSnapshot,
listSnapshots,
moveSync,
deleteSnapshot,
pruneSnapshots,
} = await import("./snapshot.js");

const { actionSnapshots } = await import("./runner.js");

const OPENCLAW_DIR = `${FAKE_HOME}/.openclaw`;
const SNAPSHOTS_DIR = `${FAKE_HOME}/.nemoclaw/snapshots`;

Expand Down Expand Up @@ -483,4 +487,177 @@ describe("snapshot", () => {
expect(listSnapshots()).toEqual([]);
});
});

describe("deleteSnapshot", () => {
it("removes a snapshot directory", () => {
addDir(`${SNAPSHOTS_DIR}/20260101T000000Z`);
addFile(`${SNAPSHOTS_DIR}/20260101T000000Z/snapshot.json`, "{}");

expect(deleteSnapshot(`${SNAPSHOTS_DIR}/20260101T000000Z`)).toBe(true);
expect(store.has(`${SNAPSHOTS_DIR}/20260101T000000Z`)).toBe(false);
});

it("returns false when path is a symlink", () => {
addSymlink(`${SNAPSHOTS_DIR}/evil`, "/attacker");

expect(deleteSnapshot(`${SNAPSHOTS_DIR}/evil`)).toBe(false);
});

it("returns true for non-existent path", () => {
expect(deleteSnapshot(`${SNAPSHOTS_DIR}/nonexistent`)).toBe(true);
});
});

describe("pruneSnapshots", () => {
it("keeps N snapshots and deletes the rest", () => {
addDir(`${SNAPSHOTS_DIR}/20260101T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260101T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260101T000000Z", source: OPENCLAW_DIR, file_count: 1, contents: [] }),
);
addDir(`${SNAPSHOTS_DIR}/20260201T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260201T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260201T000000Z", source: OPENCLAW_DIR, file_count: 1, contents: [] }),
);
addDir(`${SNAPSHOTS_DIR}/20260301T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260301T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260301T000000Z", source: OPENCLAW_DIR, file_count: 1, contents: [] }),
);

const result = pruneSnapshots(2);

expect(result.kept).toHaveLength(2);
expect(result.deleted).toHaveLength(1);
expect(result.failed).toHaveLength(0);
expect(result.kept[0]).toContain("20260301T000000Z");
expect(result.kept[1]).toContain("20260201T000000Z");
expect(result.deleted[0]).toContain("20260101T000000Z");
// Verify the deleted snapshot is actually removed from the store
expect(store.has(`${SNAPSHOTS_DIR}/20260101T000000Z`)).toBe(false);
expect(store.has(`${SNAPSHOTS_DIR}/20260201T000000Z`)).toBe(true);
expect(store.has(`${SNAPSHOTS_DIR}/20260301T000000Z`)).toBe(true);
});

it("returns empty deleted when keep >= snapshot count", () => {
addDir(`${SNAPSHOTS_DIR}/20260101T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260101T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260101T000000Z", source: OPENCLAW_DIR, file_count: 1, contents: [] }),
);

const result = pruneSnapshots(5);

expect(result.deleted).toHaveLength(0);
expect(result.failed).toHaveLength(0);
expect(result.kept).toHaveLength(1);
});

it("handles no snapshots gracefully", () => {
const result = pruneSnapshots(3);

expect(result.deleted).toHaveLength(0);
expect(result.failed).toHaveLength(0);
expect(result.kept).toHaveLength(0);
});

it("reports failures when deleteSnapshot returns false", () => {
addDir(`${SNAPSHOTS_DIR}/20260101T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260101T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260101T000000Z", source: OPENCLAW_DIR, file_count: 2, contents: [] }),
);
addDir(`${SNAPSHOTS_DIR}/20260201T000000Z`);
addFile(
`${SNAPSHOTS_DIR}/20260201T000000Z/snapshot.json`,
JSON.stringify({ timestamp: "20260201T000000Z", source: OPENCLAW_DIR, file_count: 2, contents: [] }),
);
// Make the older snapshot a symlink so deleteSnapshot rejects it
addSymlink(`${SNAPSHOTS_DIR}/20260101T000000Z`, "/attacker");

const result = pruneSnapshots(1);

expect(result.deleted).toHaveLength(0);
expect(result.failed).toHaveLength(1);
expect(result.kept).toHaveLength(1);
expect(result.failed[0]).toContain("20260101T000000Z");
expect(result.kept[0]).toContain("20260201T000000Z");
});
});

describe("actionSnapshots (CLI dispatch)", () => {
const stdoutChunks: string[] = [];

function captureStdout(): void {
vi.spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array) => {
stdoutChunks.push(String(chunk));
return true;
});
}

function stdoutText(): string {
return stdoutChunks.join("");
}

beforeEach(() => {
store.clear();
stdoutChunks.length = 0;
vi.clearAllMocks();
captureStdout();
});

afterEach(() => {
vi.restoreAllMocks();
});

it.each([["--help"], ["-h"], []])("shows usage for %j", (...argv) => {
actionSnapshots(argv);
expect(stdoutText()).toContain("Usage: snapshots <list|prune|delete>");
});

it("throws on unknown subcommand", () => {
expect(() => actionSnapshots(["bogus"])).toThrow(/Unknown snapshots subcommand/);
});

it("lists snapshots when none exist", () => {
actionSnapshots(["list"]);
expect(stdoutText()).toContain("No snapshots found.");
});

it("lists existing snapshots", () => {
const snapDir = `${FAKE_HOME}/.nemoclaw/snapshots/20260101T000000Z`;
addDir(snapDir);
addFile(snapDir + "/snapshot.json", JSON.stringify({ timestamp: "20260101T000000Z", source: FAKE_HOME, file_count: 1, contents: [] }));
actionSnapshots(["list"]);
expect(stdoutText()).toContain("20260101T000000Z");
});

it("prune --keep N deletes oldest", () => {
[1, 2, 3].forEach((n) => {
const ts = `2026010${n}T000000Z`;
addDir(`${FAKE_HOME}/.nemoclaw/snapshots/${ts}`);
addFile(`${FAKE_HOME}/.nemoclaw/snapshots/${ts}/snapshot.json`, JSON.stringify({ timestamp: ts, source: FAKE_HOME, file_count: 1, contents: [] }));
});
actionSnapshots(["prune", "--keep", "2"]);
expect(stdoutText()).toContain("Pruned 1 snapshot(s), kept 2");
});

it.each([["prune"], ["delete"]])("%s throws without required arg", (...argv) => {
expect(() => actionSnapshots(argv)).toThrow();
});

it("delete removes a snapshot by path", () => {
const sd = `${FAKE_HOME}/.nemoclaw/snapshots/20260101T000000Z`;
addDir(sd);
addFile(sd + "/snapshot.json", JSON.stringify({ timestamp: "20260101T000000Z", source: FAKE_HOME, file_count: 1, contents: [] }));
actionSnapshots(["delete", "--path", sd]);
expect(stdoutText()).toContain(`Deleted snapshot: ${sd}`);
});

it("delete succeeds on non-existent path", () => {
actionSnapshots(["delete", "--path", `${FAKE_HOME}/.nemoclaw/snapshots/nonexistent`]);
expect(stdoutText()).toContain("Deleted snapshot:");
});
});
});
32 changes: 32 additions & 0 deletions nemoclaw/src/blueprint/snapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,38 @@ function readStringArray(value: SnapshotManifestJson["contents"]): string[] {
: [];
}

export function deleteSnapshot(snapshotPath: string): boolean {
try {
rejectSymlinksOnPath(snapshotPath);
rmSync(snapshotPath, { recursive: true, force: true });
return true;
} catch {
return false;
}
}

export function pruneSnapshots(keep: number): { deleted: string[]; kept: string[]; failed: string[] } {
const snapshots = listSnapshots();
if (snapshots.length <= keep) {
return { deleted: [], kept: snapshots.map((s) => s.path), failed: [] };
}
Comment on lines +329 to +333

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle negative keep values defensively.

If keep is negative, slice(keep) returns elements from the end of the array, producing unexpected behavior (e.g., pruneSnapshots(-1) would keep all but the oldest snapshot instead of rejecting the input).

Consider clamping or throwing early:

🛡️ Suggested guard 
 export function pruneSnapshots(keep: number): { deleted: string[]; kept: string[]; failed: string[] } {
+  if (keep < 0) {
+    throw new Error(`Invalid keep count: ${keep}. Must be a non-negative integer.`);
+  }
   const snapshots = listSnapshots();
   if (snapshots.length <= keep) {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nemoclaw/src/blueprint/snapshot.ts` around lines 329 - 333, The
pruneSnapshots function does not validate the keep parameter for negative
values, and when keep is negative, the slice(keep) operation will behave
unexpectedly by returning elements from the end of the array. Add a defensive
guard at the beginning of the pruneSnapshots function to either throw an error
for invalid negative values or clamp the keep value to a minimum of 0 to ensure
predictable behavior.


const toDelete = snapshots.slice(keep);
const toKeep = snapshots.slice(0, keep);

const deleted: string[] = [];
const failed: string[] = [];
for (const snap of toDelete) {
if (deleteSnapshot(snap.path)) {
deleted.push(snap.path);
} else {
failed.push(snap.path);
}
}

return { deleted, kept: toKeep.map((s) => s.path), failed };
}

export function listSnapshots(): BlueprintSnapshotManifest[] {
let entries: Dirent[];
try {
Expand Down