fix(upgrade): warn that non-.openclaw sandbox data is not preserved on recreate (#7073) - #7093
Conversation
…n recreate (#7073) The pre-upgrade backup taken before a legacy-managed sandbox recreate scopes to /sandbox/.openclaw only. Files a user keeps elsewhere in the sandbox (e.g. /sandbox/user-data) are silently dropped by the recreate, since the recovery manifest restores .openclaw state alone. Print a warning in the prepared-backup-recovery summary (shown in both `upgrade-sandboxes --check` and the real recreate, before the destructive step) so users can back those paths up externally first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughPrepared backup recovery now reports the validated agent-specific state directory restored during recreation and warns that files outside it are not preserved. Tests cover multiple manifests, warning order, preservation boundaries, and automatic versus check modes. ChangesPrepared recovery scope
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/lib/actions/upgrade-sandboxes-recovery.test.ts`:
- Around line 193-204: Extend the recovery warning test around upgradeSandboxes
to cover both auto mode and check mode, using the public entrypoints. Capture
console warning and rebuildSandbox events in a shared sequence, then assert the
non-.openclaw preservation warnings occur before rebuilding in each mode while
retaining the existing message assertions.
🪄 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: Enterprise
Run ID: 6d0adf15-c98e-4a82-a5fa-5b19abc54e47
📒 Files selected for processing (2)
src/lib/actions/upgrade-sandboxes-recovery.test.tssrc/lib/actions/upgrade-sandboxes.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Exercise the automatic and check-only public paths. Prove the recovery warning precedes every destructive rebuild. Co-authored-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Derive each prepared-recovery warning from its validated manifest. Report OpenClaw and Hermes state paths accurately in mixed recovery. Co-authored-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/lib/actions/upgrade-sandboxes-recovery.test.ts`:
- Around line 238-267: The recovery test currently allows one console.log call
to satisfy multiple path assertions. Update the warning recorder around
upgradeSandboxes to capture each matching warning call as a distinct event, then
assert exactly one warning event for every statePath and verify the expected
warning content; ensure the assertions fail if multiple agent paths are merged
into one log call.
🪄 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: Enterprise
Run ID: abc88b7a-c8be-4a03-ad81-094eb3f3b841
📒 Files selected for processing (2)
src/lib/actions/upgrade-sandboxes-recovery.test.tssrc/lib/actions/upgrade-sandboxes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/upgrade-sandboxes.ts
Require one warning log call per recovered sandbox and one state path per warning. Co-authored-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Approved at exact head db7734a. The preservation warning is manifest-derived, emitted before destructive rebuild in both auto and check paths, and covered as one distinct warning per recovered sandbox. Focused validation passed 36/36; ordinary CI, both selected protected E2E scenarios, CodeRabbit, and the advisor are green with zero unresolved threads or actionable findings. The lack of an acknowledgement pause is consistent with this PR's explicit warning-only scope.
Context
Part of #7073. A legacy
v0.0.55 → currentupgrade recreates the managed sandbox and restores it from a pre-upgrade backup. That backup scopes to/sandbox/.openclawonly (seerebuild-backup-phase.tsmanifestdir: "/sandbox/.openclaw"), so any files the user keeps elsewhere in the sandbox (e.g./sandbox/user-data) are dropped by the recreate with no prior notice.I reproduced the loss on real hardware: after a legacy-recreate, non-
.openclawpaths are absent from the recreated sandbox while.openclawstate is restored intact.Change
Print a warning in the prepared-backup-recovery summary. It shows in both
upgrade-sandboxes --check(dry run) and the real recreate, immediately after the list of sandboxes that will be recovered — i.e. before the destructive step — so users can back up non-.openclawpaths externally first.Verification
upgradeSandboxesthrough the prepared-recovery branch and asserts the warning is emitted.--checkoutput.typecheck:cliand the fullupgrade-sandboxes-recoverysuite (35 tests) pass.Independent validation
The reporter independently ran the full
v0.0.55 → v0.0.63 → v0.0.81bridge on their own hardware and confirmed: the messaging bridge preserves the Slack channel (schema-v1messaging.planwritten on the v0.0.63 re-apply, survives to v0.0.81), while/sandbox/user-dataseeded before the final upgrade is still lost afterward — i.e. the non-.openclawdata loss this PR warns about is the tracked concern of the issue, and an external backup remains required.Scope
Deliberately narrow: a warning only. It does not change backup scope or restore behavior (widening the backup to cover arbitrary sandbox paths is a larger design decision about what the managed lifecycle owns). This just closes the silent-data-loss gap by making the boundary explicit before the irreversible recreate.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
/sandbox/user-data, and reminding users to back up those paths beforehand.Tests
#7073) verifying upgrade warning order and thatrebuildSandboxruns only in{ auto: true }mode (none in{ check: true }).Signed-off-by: Yanyun Liao yanyunl@nvidia.com