From 8959421e2db7dc05dbe0438704cf8e81dd45bdf5 Mon Sep 17 00:00:00 2001
From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
Date: Sun, 16 Aug 2026 04:03:41 +0800
Subject: [PATCH] fix(updater): keep the backup when the app cannot be put back
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`replace_and_relaunch` moves the current bundle into the stage, then
renames the replacement into its place. Between those two lines there is
no app at all, and the backup inside the stage is the only copy.

If the second rename failed, the restore was attempted with `let _ =` and
the stage was deleted regardless. A restore that failed therefore took
the backup with it: nothing at `current`, no copy left, and an error that
named only the swap. That is a reinstall.

The launch-failure arm ten lines below already does this correctly — it
matches on the restore, reports it in the message, and leaves the stage
alone so the backup survives. This one now does the same. Two arms of the
same function disagreeing about how much care a missing app deserves is
the whole of the bug.

The new test covers the swap-failure path, which had none: a replacement
that was never extracted fails the second rename, and the previous app
has to be back at `current` with its own marker. Removing the restore
fails it.

What it does not cover is the restore *itself* failing — the seams here
are `launch` and `report`, and neither runs between the two renames, so
forcing that would mean injecting the rename. The correctness of that arm
rests on mirroring the tested one below it.
---
 src/bin/tty7-updater.rs | 50 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/bin/tty7-updater.rs b/src/bin/tty7-updater.rs
index be2a8824..7c20b775 100644
--- a/src/bin/tty7-updater.rs
+++ b/src/bin/tty7-updater.rs
@@ -327,9 +327,21 @@ mod macos {
         }
 
         if let Err(error) = fs::rename(replacement, current) {
-            let _ = fs::rename(&backup, current);
-            let _ = fs::remove_dir_all(stage);
-            let result = Err(format!("putting the staged app in place: {error}"));
+            // The same care the launch-failure arm below takes, and for a
+            // sharper reason: `current` has already been moved into the stage,
+            // so the backup is the only app there is. Dropping the stage
+            // without checking the restore first deleted it — leaving nothing
+            // at `current`, and an error that mentioned only the swap.
+            let result = match fs::rename(&backup, current) {
+                Ok(()) => {
+                    let _ = fs::remove_dir_all(stage);
+                    Err(format!("putting the staged app in place: {error}"))
+                }
+                Err(restore) => Err(format!(
+                    "putting the staged app in place: {error}; \
+                     restoring the previous app: {restore}"
+                )),
+            };
             report(&result);
             return result;
         }
@@ -475,6 +487,38 @@ mod macos {
             assert!(!root.path().join(".tty7.app.tty7-update-backup").exists());
         }
 
+        #[test]
+        fn a_failed_swap_puts_the_previous_app_back() {
+            // The window this covers: `current` has already been renamed into
+            // the stage, so between the two renames there is no app at all.
+            // A replacement that is not there fails the second one.
+            let root = tempfile::tempdir().unwrap();
+            let current = root.path().join("tty7.app");
+            let stage = root.path().join("stage");
+            bundle(&current, "old");
+            fs::create_dir_all(&stage).unwrap();
+            let replacement = stage.join("never-extracted.app");
+
+            let error = replace_and_relaunch(
+                &current,
+                &replacement,
+                &stage,
+                |_| panic!("nothing should be launched when the swap failed"),
+                |_| (),
+            )
+            .unwrap_err();
+
+            assert!(
+                error.starts_with("putting the staged app in place:"),
+                "{error}"
+            );
+            assert!(
+                current.exists(),
+                "the previous app must be back where it was"
+            );
+            assert_eq!(fs::read_to_string(current.join("marker")).unwrap(), "old");
+        }
+
         #[test]
         fn failed_launch_restores_and_relaunches_the_previous_app() {
             let root = tempfile::tempdir().unwrap();