From a6a6f6656be49f7a5a99922ac09520f9d8ae6a05 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 18 Sep 2026 18:27:51 +0000 Subject: [PATCH] Stage reconcile restore copies so a failed rollback cannot wipe the workspace restore_workspace used to clear the live tree before copy_tree. If that copy failed, rollback returned false with the original files already gone. Stage under .arbiter-reconcile-snapshots first; only then replace contents. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 10 +++++++++ include/reconcile.h | 3 +++ src/reconcile.cpp | 25 ++++++++++++++++++++- tests/test_reconcile.cpp | 47 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..46d4d270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **Reconcile rollback no longer wipes the workspace on a failed restore.** + `restore_workspace` now copies the snapshot into a staging directory + under `.arbiter-reconcile-snapshots` before clearing the live tree. + If that copy fails (unreadable snapshot, I/O error), the original + files stay in place and restore returns false. Previously + `clear_dir_contents` ran first, so a subsequent `copy_tree` failure + left the workspace empty while `run_reconcile` reported `failed` + rather than `rolled_back`. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/include/reconcile.h b/include/reconcile.h index 69afd93e..1bcb2c80 100644 --- a/include/reconcile.h +++ b/include/reconcile.h @@ -159,6 +159,9 @@ VerificationEvidence run_verification(const ReconcileSpec& spec, bool snapshot_workspace(const std::string& root, const std::string& dest, std::string* err); +// Copy `snapshot` into `root`. The snapshot is staged first so a +// failed copy cannot clear the live tree; only a successful stage +// replaces workspace contents (`.arbiter-reconcile-snapshots` is kept). bool restore_workspace(const std::string& snapshot, const std::string& root, std::string* err); diff --git a/src/reconcile.cpp b/src/reconcile.cpp index 0179806b..7cfc6bf9 100644 --- a/src/reconcile.cpp +++ b/src/reconcile.cpp @@ -926,8 +926,31 @@ bool restore_workspace(const std::string& snapshot, if (err) *err = "workspace root is missing"; return false; } + + // Stage the snapshot copy first. clear_dir_contents(root) is not + // reversible, so a copy_tree failure after clearing used to leave + // the live workspace empty (or partial) even though restore + // returned false. Staging lives under .arbiter-reconcile-snapshots + // so clear_dir_contents skips it (same rule as the `pre` snapshot). + const fs::path snap_dir = fs::path(root) / ".arbiter-reconcile-snapshots"; + const fs::path staging = snap_dir / ".restore-staging"; + fs::create_directories(snap_dir, ec); + if (ec) { + if (err) *err = "cannot create snapshot dir: " + ec.message(); + return false; + } + fs::remove_all(staging, ec); + if (!copy_tree(snapshot, staging.string(), err)) { + fs::remove_all(staging, ec); + return false; + } + clear_dir_contents(root); - return copy_tree(snapshot, root, err); + const bool ok = copy_tree(staging.string(), root, err); + if (ok) fs::remove_all(staging, ec); + // On a failed install into root, leave staging in place so the + // staged tree is still recoverable under .restore-staging. + return ok; } std::string format_reconcile_brief(const StateContract& c, diff --git a/tests/test_reconcile.cpp b/tests/test_reconcile.cpp index 2864feaa..5c250740 100644 --- a/tests/test_reconcile.cpp +++ b/tests/test_reconcile.cpp @@ -227,6 +227,53 @@ TEST_CASE("broken tests never satisfy") { CHECK_FALSE(r.verification.passed); } +TEST_CASE("restore_workspace does not wipe the live tree when snapshot copy fails") { + TempDir dir; + const fs::path root = dir.path / "ws"; + const fs::path snap = dir.path / "snap"; + fs::create_directories(root); + fs::create_directories(snap); + write_file(root / "keep.txt", "original\n"); + write_file(snap / "secret.txt", "from-snap\n"); + fs::permissions(snap / "secret.txt", fs::perms::none); + + std::string err; + CHECK_FALSE(restore_workspace(snap.string(), root.string(), &err)); + CHECK(fs::exists(root / "keep.txt")); + CHECK_FALSE(fs::exists(root / "secret.txt")); + { + std::ifstream in(root / "keep.txt"); + std::string body((std::istreambuf_iterator(in)), {}); + CHECK(body == "original\n"); + } + + std::error_code ec; + fs::permissions(snap / "secret.txt", + fs::perms::owner_read | fs::perms::owner_write, ec); +} + +TEST_CASE("restore_workspace replaces live files from a readable snapshot") { + TempDir dir; + const fs::path root = dir.path / "ws"; + const fs::path snap = dir.path / "snap"; + fs::create_directories(root); + fs::create_directories(snap); + write_file(root / "keep.txt", "original\n"); + write_file(root / "debris.py", "partial\n"); + write_file(snap / "keep.txt", "restored\n"); + + std::string err; + CHECK(restore_workspace(snap.string(), root.string(), &err)); + CHECK(fs::exists(root / "keep.txt")); + CHECK_FALSE(fs::exists(root / "debris.py")); + { + std::ifstream in(root / "keep.txt"); + std::string body((std::istreambuf_iterator(in)), {}); + CHECK(body == "restored\n"); + } + CHECK_FALSE(fs::exists(root / ".arbiter-reconcile-snapshots" / ".restore-staging")); +} + TEST_CASE("rollbackOnFailure restores pre-run tree") { TempDir dir; write_file(dir.path / "keep.txt", "original\n");