fix(installer): never brick the install when a self-update swap fails

apps/bootstrap-installer/src-tauri/src/update.rs

@@ -629,27 +629,10 @@ async fn install_macos_app_update(
         ));
     }
 
-    let moved_old = if target_app.exists() {
-        match tokio::fs::rename(target_app, &old).await {
-            Ok(()) => true,
-            Err(_) => {
-                remove_dir_if_exists(target_app).await;
-                false
-            }
-        }
-    } else {
-        false
-    };
-    if let Err(err) = tokio::fs::rename(&tmp, target_app).await {
-        if moved_old {
-            let _ = tokio::fs::rename(&old, target_app).await;
-        }
-        return Err(anyhow!(
-            "installing updated app at {}: {err}",
-            target_app.display()
-        ));
-    }
-    remove_dir_if_exists(&old).await;
+    // Atomic-as-possible swap with rollback. Extracted so the invariant
+    // (target is never left deleted-with-no-replacement) can be unit-tested
+    // without ditto / a real .app bundle.
+    swap_in_new_bundle(&tmp, target_app, &old).await?;
 
     let _ = Command::new("/usr/bin/xattr")
         .arg("-dr")
@@ -662,6 +645,42 @@ async fn install_macos_app_update(
     Ok(target_app.to_path_buf())
 }
 
+/// Move a freshly-staged bundle (`tmp`) into place at `target`, parking any
+/// existing bundle at `old` so the move can succeed (macOS `rename` won't
+/// overwrite a non-empty directory).
+///
+/// Invariant: on ANY failure path, `target` is left pointing at a working
+/// bundle — either the original (rolled back from `old`) or untouched — and we
+/// never delete the running app with no replacement in place. The staged `tmp`
+/// copy is cleaned up on failure.
+async fn swap_in_new_bundle(tmp: &Path, target: &Path, old: &Path) -> Result<()> {
+    let moved_old = if target.exists() {
+        if let Err(err) = tokio::fs::rename(target, old).await {
+            // Could not move the existing app aside. Leave it untouched and
+            // bail — a failed update must not brick the install.
+            remove_dir_if_exists(tmp).await;
+            return Err(anyhow!(
+                "could not move existing app aside at {} (leaving it in place): {err}",
+                target.display()
+            ));
+        }
+        true
+    } else {
+        false
+    };
+    if let Err(err) = tokio::fs::rename(tmp, target).await {
+        // Restore the original app from the backup so the user keeps a working
+        // install, and clean up the staged copy.
+        if moved_old {
+            let _ = tokio::fs::rename(old, target).await;
+        }
+        remove_dir_if_exists(tmp).await;
+        return Err(anyhow!("installing updated app at {}: {err}", target.display()));
+    }
+    remove_dir_if_exists(old).await;
+    Ok(())
+}
+
 #[cfg(not(target_os = "macos"))]
 async fn install_macos_app_update(
     _app: &AppHandle,
@@ -795,4 +814,96 @@ mod tests {
         );
         assert_eq!(target_app_from_args(["--target-app", "/tmp/not-an-app"]), None);
     }
+
+    // Helpers for the swap tests: make a throwaway dir tree we can rename.
+    fn unique_tmp_dir(tag: &str) -> PathBuf {
+        let base = std::env::temp_dir().join(format!(
+            "hermes-swap-test-{tag}-{}-{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        std::fs::create_dir_all(&base).unwrap();
+        base
+    }
+
+    fn write_marker(dir: &Path, contents: &str) {
+        std::fs::create_dir_all(dir).unwrap();
+        std::fs::write(dir.join("marker.txt"), contents).unwrap();
+    }
+
+    #[tokio::test]
+    async fn swap_installs_new_bundle_and_cleans_up() {
+        let base = unique_tmp_dir("ok");
+        let target = base.join("Hermes.app");
+        let tmp = base.join("Hermes.app.hermes-update-new");
+        let old = base.join("Hermes.app.hermes-update-old");
+        write_marker(&target, "OLD");
+        write_marker(&tmp, "NEW");
+
+        swap_in_new_bundle(&tmp, &target, &old).await.unwrap();
+
+        // New bundle is now at target; staging + backup dirs are gone.
+        assert_eq!(
+            std::fs::read_to_string(target.join("marker.txt")).unwrap(),
+            "NEW"
+        );
+        assert!(!tmp.exists(), "staged copy should be cleaned up");
+        assert!(!old.exists(), "backup should be cleaned up on success");
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
+    #[tokio::test]
+    async fn swap_failure_never_leaves_target_missing() {
+        // Regression guard for the catastrophic path: the move-aside of the
+        // existing app fails AND the staged bundle can't be installed. The
+        // buggy version deleted `target` when move-aside failed and then
+        // skipped rollback, bricking the install. The fixed version must leave
+        // the original app intact on disk.
+        //
+        // Trigger both failures deterministically:
+        //  - `old` is a NON-EMPTY dir  -> rename(target, old) fails
+        //  - `tmp` does not exist       -> rename(tmp, target) fails
+        let base = unique_tmp_dir("fail");
+        let target = base.join("Hermes.app");
+        let tmp = base.join("Hermes.app.hermes-update-new"); // intentionally absent
+        let old = base.join("Hermes.app.hermes-update-old");
+        write_marker(&target, "OLD");
+        write_marker(&old, "OCCUPIED"); // non-empty => rename(target,old) fails
+
+        let result = swap_in_new_bundle(&tmp, &target, &old).await;
+
+        assert!(result.is_err(), "swap should fail when neither move can complete");
+        assert!(target.exists(), "original app must NOT be deleted on failure");
+        assert_eq!(
+            std::fs::read_to_string(target.join("marker.txt")).unwrap(),
+            "OLD",
+            "original app contents must be intact after a failed swap"
+        );
+        let _ = std::fs::remove_dir_all(&base);
+    }
+
+    #[tokio::test]
+    async fn swap_rolls_back_when_install_step_fails() {
+        // Move-aside succeeds but installing the staged bundle fails (tmp
+        // absent). The original must be rolled back from `old` to `target`.
+        let base = unique_tmp_dir("rollback");
+        let target = base.join("Hermes.app");
+        let tmp = base.join("Hermes.app.hermes-update-new"); // absent
+        let old = base.join("Hermes.app.hermes-update-old");
+        write_marker(&target, "OLD");
+
+        let result = swap_in_new_bundle(&tmp, &target, &old).await;
+
+        assert!(result.is_err());
+        assert!(target.exists(), "original must be restored after failed install");
+        assert_eq!(
+            std::fs::read_to_string(target.join("marker.txt")).unwrap(),
+            "OLD"
+        );
+        assert!(!old.exists(), "backup should be rolled back, not left behind");
+        let _ = std::fs::remove_dir_all(&base);
+    }
 }