Repository navigation
Complete v0.2.4 restore reliability - #6
Conversation
- Restore audit (Docs/Restore/V0_2_4_RESTORE_AUDIT.md) - Dry-run support (root restore --dry-run) - Pre-restore validation (platform, Nix, features, .drv checks) - Partial failure recovery with auto-rollback - Strengthened drift detection in root status - Restore event ledger (RestorePlanned, RestoreRecovered) - Restore error normalization - Smoke test docs and restore reference notes - 7 new tests
Record Restore Failed events on validation failure, add auto-rollback and lock-edge-case tests, regenerate crate versions in Cargo.lock, and update the restore audit so it matches the implemented system. Co-authored-by: Sergio Rovira <sgr0691@users.noreply.github.com>
Use store_paths.values(), drop needless returns in restore recovery, and collapse identical Restore error-formatter branches. Co-authored-by: Sergio Rovira <sgr0691@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 040ae574cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !adapter.profile_exists() { | ||
| return Err(anyhow::anyhow!( | ||
| "Restore validation failed: Root profile does not exist.\n\ | ||
| Run 'root init' to create the profile." |
There was a problem hiding this comment.
Allow restore to create a missing profile
When restoring after ~/.root has been removed, RealNixAdapter::profile_exists() is false, so this rejects the restore before any package can recreate the profile. The suggested root init does not resolve the error because init() only creates Root directories and deliberately does not create profiles/default; consequently a clean/disaster-recovery restore remains impossible until an unrelated package is installed first.
Useful? React with 👍 / 👎.
| if let Some(ref snapshot) = pre_restore_snapshot { | ||
| match attempt_rollback_to_snapshot(adapter, snapshot) { | ||
| Ok(()) => { |
There was a problem hiding this comment.
Restore metadata files during automatic recovery
If profile reconciliation reaches the metadata writes, save_lock_v2(target_lock) can succeed and the following Rootfile write can fail, for example because of a permission or disk error. The resulting error enters this recovery path, but attempt_rollback_to_snapshot only changes the Nix profile; it neither restores root.lock nor the Rootfile, after which the code incorrectly reports that both were preserved and leaves the target lock paired with the old Rootfile/profile.
Useful? React with 👍 / 👎.
| if lock.platform != current_platform { | ||
| drift_details.push(DriftIssue { |
There was a problem hiding this comment.
Mark platform mismatches unhealthy
When the lockfile platform differs but the package/profile comparisons otherwise pass, this appends a drift issue without setting healthy = false. The report therefore returns healthy: true and state: "Healthy" while simultaneously reporting platform-mismatch, making structured status consumers treat the newly detected drift as healthy.
Useful? React with 👍 / 👎.
…alth Allow restore to create a missing Nix profile, rewrite Rootfile and root.lock from the pre-restore snapshot on automatic recovery, and mark platform-mismatch as unhealthy in root status. Co-authored-by: Sergio Rovira <sgr0691@users.noreply.github.com>
Closes the remaining work on restore reliability so v0.2.4 can ship.
The original
v0.2.4-restore-reliabilitybranch had the product changes (--dry-run, pre-restore validation, automatic rollback, events, docs) but was never PR’d, never CI’d, and still listed crate versions as 0.2.3 inCargo.lock. The restore audit still described the pre-0.2.4 system.What this PR adds on top of that branch
restore()records aRestore/Failedevent whenrestore_validatefails, instead of returning silently.test_restore_partial_failure_rolls_back_profileinstallsfd, restores a lock of[ripgrep, missing_pkg], and asserts profile + Rootfile +root.lockstay onfdwithRestoreRecovered.Commands::Restore { lock, dry_run }compiles again;--dry-runis covered.Cargo.lock— workspace crates are 0.2.4 (dependency versions unchanged).Docs/Restore/V0_2_4_RESTORE_AUDIT.mdmatches the implemented flow and marks 9.1 / 9.7 closed.for_kv_map, needless returns, and identical error-formatter branches.Codex review follow-ups
restore_validateno longer rejects a missing~/.root/profiles/default. Restore treats it as empty andnix profile addcreates it.attempt_rollback_to_snapshotnow rewritesroot.lockand Rootfile from the snapshot, not only the Nix profile.root statussetshealthy = falseandNeedsAttentiononplatform-mismatch.Local CI
All passed.
Still deferred (called out in the audit, not this PR)
Nix ops are not transactional, no Nix timeout during restore, no restore-from-snapshot-file, v1 installables can be unpinned, no snapshot retention.
Tagging
v0.2.4and publishing the GitHub release is a post-merge step.