feat(cli): add snapshot create/list/restore commands - #1892
Merged
Conversation
Expose the backup/restore plumbing from sandbox-state.ts as user-facing commands so users can manage sandbox state independently of rebuild: nemoclaw <name> snapshot create Create a timestamped snapshot nemoclaw <name> snapshot list List available snapshots nemoclaw <name> snapshot restore [ts] Restore from a snapshot Closes #1891 Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a sandbox-scoped Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant CLI as "nemoclaw CLI"
participant OpenShell as "openshell"
participant SandboxState as "sandboxState"
participant FS as "Filesystem/BackupDir"
User->>CLI: "<sandbox> snapshot create"
CLI->>OpenShell: "sandbox list" (captureOpenshell)
OpenShell-->>CLI: running list / error
CLI->>SandboxState: backupSandboxState(sandbox)
SandboxState-->>FS: write timestamped backup dir
SandboxState-->>CLI: success / failure (+ failed dirs)
CLI-->>User: "Snapshot created" / error
User->>CLI: "<sandbox> snapshot list"
CLI->>SandboxState: listBackups(sandbox)
SandboxState-->>FS: read backup metadata
SandboxState-->>CLI: list entries
CLI-->>User: prints snapshot list
User->>CLI: "<sandbox> snapshot restore [ts]"
CLI->>SandboxState: resolve backup (exact / prefix / latest)
SandboxState->>FS: read chosen backup
SandboxState->>FS: restore files into sandbox
SandboxState-->>CLI: success / partial / failure (+ failed dirs)
CLI-->>User: print restore result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/nemoclaw.ts (1)
1680-1766: Tighten argument validation for snapshot subcommands.
create/list/restorecurrently accept unexpected trailing args silently. Rejecting extras will make scripting behavior deterministic.Proposed fix
switch (subcommand) { case "create": { + if (subArgs.length > 1) { + console.error(" Usage: nemoclaw " + sandboxName + " snapshot create"); + process.exit(1); + } const isLive = captureOpenshell(["sandbox", "list"], { ignoreError: true }); @@ case "list": { + if (subArgs.length > 1) { + console.error(" Usage: nemoclaw " + sandboxName + " snapshot list"); + process.exit(1); + } const backups = sandboxState.listBackups(sandboxName); @@ case "restore": { + if (subArgs.length > 2) { + console.error(" Usage: nemoclaw " + sandboxName + " snapshot restore [timestamp]"); + process.exit(1); + } const timestamp = subArgs[1] || null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 1680 - 1766, The snapshot subcommands currently ignore unexpected trailing arguments; before handling the switch on subcommand (or at the top of each case), validate subArgs so only allowed argument counts are accepted: for "create" and "list" require subArgs.length === 1 (no extras), and for "restore" allow subArgs.length === 1 or 2 (optional timestamp) but reject >2; on violation print a concise error (e.g., "Unexpected arguments") and the usage lines and call process.exit(1). Update the logic around the existing subcommand/subArgs handling (refer to variables subcommand, subArgs and the "create"/"list"/"restore" case blocks) to perform these checks before executing backup/restore operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nemoclaw.ts`:
- Around line 1683-1687: The current logic uses captureOpenshell(["sandbox",
"list"]) and treats any failure the same as the sandbox not running; update the
branch to first check the openshell call result (the captureOpenshell return
value, e.g., isLive.success / isLive.exitCode / isLive.error or absence of
isLive) and if the command failed, log a clear connectivity/gateway error
including the underlying isLive error/output and exit; only when the command
succeeded run parseLiveSandboxNames(isLive.output || "") and then, if liveNames
does not contain sandboxName, log the existing "Sandbox ... is not running"
message and exit.
- Around line 1725-1734: The restore lookup currently uses
b.backupPath.includes(timestamp) which can return multiple partial matches;
update the logic in the sandbox restore block (where sandboxState.listBackups,
sandboxName, timestamp and backupPath are used) to disambiguate: collect all
candidates where b.timestamp === timestamp OR the backup path basename/last
segment equals timestamp (e.g., path.endsWith('/' + timestamp) or compare
path.split('/').pop()), then if zero matches keep the existing error, if more
than one match print an “ambiguous snapshot” error listing the matching
backupPath values and exit, and only set backupPath when exactly one match is
found.
---
Nitpick comments:
In `@src/nemoclaw.ts`:
- Around line 1680-1766: The snapshot subcommands currently ignore unexpected
trailing arguments; before handling the switch on subcommand (or at the top of
each case), validate subArgs so only allowed argument counts are accepted: for
"create" and "list" require subArgs.length === 1 (no extras), and for "restore"
allow subArgs.length === 1 or 2 (optional timestamp) but reject >2; on violation
print a concise error (e.g., "Unexpected arguments") and the usage lines and
call process.exit(1). Update the logic around the existing subcommand/subArgs
handling (refer to variables subcommand, subArgs and the
"create"/"list"/"restore" case blocks) to perform these checks before executing
backup/restore operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1bb41dc-eb5f-404d-85a3-b3c795a492cb
📒 Files selected for processing (1)
src/nemoclaw.ts
Tests the full snapshot create/list/restore lifecycle: - Create snapshot, verify it appears in list - Modify sandbox state, create second snapshot - Restore latest snapshot, verify state recovered - Restore first snapshot by timestamp - Verify no credentials in snapshot directories Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/test-snapshot-commands.sh`:
- Around line 129-131: The snapshot timestamp extraction currently allows an
empty SNAPSHOT_TIMESTAMP and continues, making the by-timestamp restore path
optional; change this so that if SNAPSHOT_TIMESTAMP is empty the script fails
immediately with a non-zero exit and clear error (i.e., do not use "|| true"),
and after performing the restore-by-timestamp verify that SECOND_MARKER is
removed (or that restored content matches expected) and if not, print an error
and exit non-zero; update the logic around SNAPSHOT_TIMESTAMP, the restore
invocation that uses it, and the post-restore checks for SECOND_MARKER to
enforce hard failures on parse or restore mismatches (also apply the same strict
behavior in the similar block referenced around lines 170-189).
- Around line 209-214: The help-check currently inspects HELP_OUTPUT for
"snapshot create" and "snapshot restore" but omits "snapshot list", so update
the conditional that checks HELP_OUTPUT (the grep chain around HELP_OUTPUT) to
also verify "snapshot list" is present before calling pass "snapshot help shows
create/list/restore"; ensure the same HELP_OUTPUT variable and success message
are used so the assertion matches the message.
- Around line 144-165: The test currently does a no-op latest restore because
the sandbox already contains the second snapshot; before calling `nemoclaw
"${SANDBOX_NAME}" snapshot restore` mutate the workspace (for example overwrite
or delete the file referenced by `SECOND_MARKER`) so the restore must actually
change state, then run the restore and assert failures on post-restore checks by
replacing the non-failing `info` branch with a hard `fail` when `SECOND_CHECK`
does not equal `SECOND_CONTENT`; reference `RESTORE_OUTPUT`, `SECOND_MARKER`,
`SECOND_CHECK`, and `SECOND_CONTENT` to locate where to inject the perturbation
and change the assertion handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 070a5367-08ea-44c2-a6f3-0beb44db98b9
📒 Files selected for processing (2)
.github/workflows/nightly-e2e.yamltest/e2e/test-snapshot-commands.sh
cv
approved these changes
Apr 15, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/nemoclaw.ts (2)
1765-1769:⚠️ Potential issue | 🟡 MinorHandle OpenShell query failures before reporting “not running.”
If
openshell sandbox listfails here, this branch still falls through to the “not running” message, which is misleading for gateway/runtime connectivity failures. CheckisLive.statusfirst and exit with a query failure message before parsing names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 1765 - 1769, The code currently assumes captureOpenshell succeeded and reports "not running" incorrectly; before calling parseLiveSandboxNames, check the captureOpenshell result (isLive.status or equivalent) and if the query failed log a clear query failure error (including any isLive.error/output details) and exit, otherwise proceed to parseLiveSandboxNames and the existing sandboxName check; update the block around captureOpenshell/parseLiveSandboxNames to bail on query failure instead of falling through to the "not running" message.
1807-1815:⚠️ Potential issue | 🟠 MajorDisambiguate snapshot selection before restoring.
backupPath.includes(timestamp)can match multiple snapshots or unrelated path segments, so this can restore the wrong backup. Collect exact candidates, fail on ambiguity, and only continue when there is exactly one match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 1807 - 1815, Replace the single .find(...) with a filtering step that collects exact candidates (e.g., use sandboxState.listBackups(sandboxName).filter(...)) rather than using backupPath.includes(timestamp); then check candidates.length: if 0, print the existing "No snapshot matching" message and exit; if >1, print an "Ambiguous snapshot" error listing the matching candidates and exit; only when candidates.length === 1 set backupPath = candidates[0].backupPath. Keep the checks using the same variables (timestamp, sandboxName, sandboxState.listBackups, backupPath) so the logic is localized to this block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/nemoclaw.ts`:
- Around line 1825-1826: Call ensureLiveSandboxOrExit(sandboxName) before
invoking sandboxState.restoreSandboxState to preflight that the target sandbox
is running; modify sandboxSnapshot to be async and await it in the dispatch path
so you can await ensureLiveSandboxOrExit. Specifically, in sandboxSnapshot (the
function that currently calls restoreSandboxState), add an await
ensureLiveSandboxOrExit(sandboxName) immediately prior to the
console.log/restore call, change sandboxSnapshot's signature to async, and
update the code that dispatches/awaits sandboxSnapshot to await the returned
promise. This ensures you validate the sandbox presence before
restoreSandboxState runs.
---
Duplicate comments:
In `@src/nemoclaw.ts`:
- Around line 1765-1769: The code currently assumes captureOpenshell succeeded
and reports "not running" incorrectly; before calling parseLiveSandboxNames,
check the captureOpenshell result (isLive.status or equivalent) and if the query
failed log a clear query failure error (including any isLive.error/output
details) and exit, otherwise proceed to parseLiveSandboxNames and the existing
sandboxName check; update the block around
captureOpenshell/parseLiveSandboxNames to bail on query failure instead of
falling through to the "not running" message.
- Around line 1807-1815: Replace the single .find(...) with a filtering step
that collects exact candidates (e.g., use
sandboxState.listBackups(sandboxName).filter(...)) rather than using
backupPath.includes(timestamp); then check candidates.length: if 0, print the
existing "No snapshot matching" message and exit; if >1, print an "Ambiguous
snapshot" error listing the matching candidates and exit; only when
candidates.length === 1 set backupPath = candidates[0].backupPath. Keep the
checks using the same variables (timestamp, sandboxName,
sandboxState.listBackups, backupPath) so the logic is localized to this block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cfb0ed04-0dbb-479c-a2c9-4e8eae18fd47
📒 Files selected for processing (1)
src/nemoclaw.ts
Comment on lines
+1825
to
+1826
| console.log(` Restoring snapshot into '${sandboxName}'...`); | ||
| const result = sandboxState.restoreSandboxState(sandboxName, backupPath); |
Contributor
There was a problem hiding this comment.
Preflight the target sandbox before restore.
restoreSandboxState() only checks SSH after it finds local state directories. For snapshots with no backed-up dirs, it returns success immediately, so this command can print a successful restore even when '${sandboxName}' is stopped or missing. Call ensureLiveSandboxOrExit(sandboxName) before restoring; that will require making sandboxSnapshot async and awaiting it in the dispatch path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/nemoclaw.ts` around lines 1825 - 1826, Call
ensureLiveSandboxOrExit(sandboxName) before invoking
sandboxState.restoreSandboxState to preflight that the target sandbox is
running; modify sandboxSnapshot to be async and await it in the dispatch path so
you can await ensureLiveSandboxOrExit. Specifically, in sandboxSnapshot (the
function that currently calls restoreSandboxState), add an await
ensureLiveSandboxOrExit(sandboxName) immediately prior to the
console.log/restore call, change sandboxSnapshot's signature to async, and
update the code that dispatches/awaits sandboxSnapshot to await the returned
promise. This ensures you validate the sandbox presence before
restoreSandboxState runs.
- Handle openshell query failures separately from "sandbox not running" - Disambiguate snapshot matching: reject ambiguous partial timestamps - Fail hard on timestamp parse failure in E2E instead of skipping - Perturb workspace before latest restore so it's not a no-op - Assert snapshot list in help check alongside create/restore Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
3 tasks
cv
pushed a commit
that referenced
this pull request
Apr 15, 2026
## Summary - Document `nemoclaw <name> snapshot create/list/restore` commands (from #1892) - Document `nemoclaw <name> policy-remove` command (from #1822) - Document `nemoclaw <name> rebuild` command with version staleness detection (from #1870) - Document `nemoclaw backup-all` command (from #1870) - Update `status` to mention live enforced policy display (from #1896) - Update `connect` and `status` to mention version staleness warnings (from #1870) - Update `destroy` to reference snapshots and rebuild as alternatives - Add snapshot commands section to backup-restore page - Add `policy-remove` to customize-network-policy page - Add "Sandbox is running an outdated agent version" troubleshooting entry - Bump doc version switcher through 0.0.16 - Regenerate agent skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit hooks pass - [ ] Verify rendered pages in docs build output 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added documentation for policy preset removal command with selection flow * Documented new rebuild command for sandbox upgrades while preserving workspace state * Introduced snapshot commands for creating, listing, and restoring workspace backups * Added bulk backup capability for multiple sandboxes * Enhanced agent version warnings in connect and status commands with remediation guidance * Expanded troubleshooting guide with outdated agent version resolution steps <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exposes the backup/restore plumbing from
sandbox-state.ts(added in #1870) as user-facing CLI commands, per feedback from @bpelfrey:New commands
Example workflow
Implementation
Thin wrappers around existing functions in
src/lib/sandbox-state.ts:backupSandboxState()→snapshot createrestoreSandboxState()→snapshot restorelistBackups()/getLatestBackup()→snapshot listNo new modules or dependencies — just CLI dispatch wiring.
Test plan
nemoclaw <name> snapshot createon a live sandboxnemoclaw <name> snapshot listshows the snapshotnemoclaw <name> snapshot restorerestores stateCloses #1891
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Tests
Chores