From bbe93ab8a86874b8d0b9c812e4e622b26b92708f Mon Sep 17 00:00:00 2001
From: kshitij <82637225+kshitijk4poor@users.noreply.github.com>
Date: Sat, 1 Aug 2026 16:49:42 +0530
Subject: [PATCH] fix(update): restore mid-swap backup before clearing
 leftovers in staging
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 2 review HIGH (empirically reproduced): a hard kill between
os.rename(dst, backup) and os.rename(staging, dst) leaves dst missing and
the backup as the ONLY copy of that entry. On retry, _stage_replacement
deleted that backup as a 'leftover' BEFORE staging the fresh copy — so a
staging failure (disk exhaustion is likeliest exactly after writing a full
staging copy) left a hole in the install with nothing to roll back to.

Restore the backup to dst first when dst is missing; it's a same-filesystem
rename. Mutation-verified: removing the restore makes the new test fail.
---
 hermes_cli/update_cmd.py                      |  8 ++++
 tests/hermes_cli/test_update_zip_two_phase.py | 38 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py
index 4d24b25379c13..c1331475d6a9a 100644
--- a/hermes_cli/update_cmd.py
+++ b/hermes_cli/update_cmd.py
@@ -613,6 +613,14 @@ def _stage_replacement(src: str, dst: str) -> str:
     """
     staging = f"{dst}.hermes-update-staging"
     backup = f"{dst}.hermes-update-old"
+    # A previous run may have died between "move dst aside" and "move staging
+    # in" — leaving dst missing and the backup as the ONLY copy of that entry.
+    # Restore it before clearing leftovers: deleting the backup first and then
+    # failing to stage (disk exhaustion is likely right after writing a full
+    # staging copy) would leave a hole in the install with nothing to roll
+    # back to. The restore is a same-filesystem rename — instant and safe.
+    if not os.path.exists(dst) and os.path.exists(backup):
+        os.rename(backup, dst)
     for leftover in (staging, backup):
         if os.path.isdir(leftover):
             shutil.rmtree(leftover, ignore_errors=True)
diff --git a/tests/hermes_cli/test_update_zip_two_phase.py b/tests/hermes_cli/test_update_zip_two_phase.py
index a6844deed8264..012463ce20f4c 100644
--- a/tests/hermes_cli/test_update_zip_two_phase.py
+++ b/tests/hermes_cli/test_update_zip_two_phase.py
@@ -348,3 +348,41 @@ def test_patched_is_windows_reaches_the_venv_path_derivation():
         venv_python_path("/nope/venv", windows=True).as_posix()
         == "/nope/venv/Scripts/python.exe"
     )
+
+
+# ---------------------------------------------------------------------------
+# Crash between "move dst aside" and "move staging in" (Phase 2 review HIGH)
+# ---------------------------------------------------------------------------
+
+def test_staging_restores_backup_when_dst_is_missing(tmp_path, monkeypatch):
+    """A previous run that died mid-swap leaves dst missing and the backup as
+    the ONLY copy of that entry. On retry, _stage_replacement must restore
+    the backup to dst BEFORE clearing leftovers — otherwise a staging failure
+    right after (disk exhaustion is likeliest exactly then) leaves a hole in
+    the install with nothing to roll back to."""
+    live, new = tmp_path / "live", tmp_path / "new"
+    live.mkdir()
+    _live_tree(new, {"agent": "new"})
+    # Simulate the crashed state: dst gone, backup holds the old tree.
+    backup = live / "agent.hermes-update-old"
+    backup.mkdir()
+    (backup / "version.txt").write_text("old")
+
+    # Staging fails (disk full) on the fresh copy.
+    def boom(src, dst, *a, **kw):
+        raise OSError(28, "No space left on device")
+
+    monkeypatch.setattr(update_cmd.shutil, "copytree", boom)
+    with pytest.raises(OSError):
+        update_cmd._stage_replacement(str(new / "agent"), str(live / "agent"))
+    monkeypatch.undo()
+
+    # The old tree must have been restored to dst before the failure.
+    assert (live / "agent" / "version.txt").read_text() == "old"
+    assert not backup.exists()
+
+    # And a clean retry completes the update normally.
+    staged = _stage_all(live, new, ["agent"])
+    update_cmd._commit_staged_replacements(staged)
+    assert (live / "agent" / "version.txt").read_text() == "new"
+    assert not [p for p in os.listdir(live) if "hermes-update" in p]