Restore the destination if the fallback swap fails

The fallback removed whatever sat at the destination and then
renamed the new content into place as two separate steps, so a
failure in the second step left nothing at that path at all, worse
than the original write it was trying to protect. Now renames the
existing destination aside as a backup instead of unlinking it, and
renames the backup straight back if the final rename fails, so a
mid swap failure restores the original rather than losing it. Found
by the graphify review bot on PR 3509.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
(cherry picked from commit b56c77a107)

graphify/paths.py

@@ -42,12 +42,13 @@ def os_replace_with_fallback(src: "str | Path", dst: "str | Path") -> None:
     e.g. install.py's managed skill symlinks, #3286, rely on exactly this).
     A naive ``shutil.copy2(src, dst)`` does the opposite when ``dst`` is a
     symlink: opening it for writing follows the link and overwrites its
-    TARGET's content instead. So the fallback copies to a fresh temp file
-    in ``dst``'s directory first, then removes whatever is at ``dst`` (link
-    or file) and renames the temp copy into place -- matching replace's
-    "whatever was there is gone, a plain file replaces it" semantics, and
-    keeping the exposure window to two fast metadata operations rather than
-    however long the copy itself takes.
+    TARGET's content instead. So the fallback copies to a fresh temp file in
+    ``dst``'s directory first, renames whatever is currently at ``dst`` (link
+    or file) aside as a backup rather than deleting it outright, and only
+    then renames the temp copy into ``dst``'s place -- matching replace's
+    "whatever was there is gone, a plain file replaces it" semantics. If that
+    final rename fails, the backup is renamed straight back so a mid-swap
+    failure leaves the original in place rather than leaving ``dst`` missing.
     """
     try:
         os.replace(src, dst)
@@ -57,15 +58,31 @@ def os_replace_with_fallback(src: "str | Path", dst: "str | Path") -> None:
             raise
     import shutil
     dst = os.fspath(dst)
-    fd, tmp_copy = tempfile.mkstemp(dir=os.path.dirname(dst) or ".", prefix=".gfy-replace-", suffix=".tmp")
+    dst_dir = os.path.dirname(dst) or "."
+    fd, tmp_copy = tempfile.mkstemp(dir=dst_dir, prefix=".gfy-replace-", suffix=".tmp")
     os.close(fd)
     try:
         shutil.copy2(src, tmp_copy)
+        backup = None
+        if os.path.lexists(dst):
+            bfd, backup = tempfile.mkstemp(dir=dst_dir, prefix=".gfy-replace-bak-", suffix=".tmp")
+            os.close(bfd)
+            os.unlink(backup)  # reserve the name only; rename needs it free on Windows
+            os.rename(dst, backup)  # a plain rename moves a symlink itself, never its target
         try:
-            os.unlink(dst)
-        except FileNotFoundError:
-            pass
-        os.rename(tmp_copy, dst)
+            os.rename(tmp_copy, dst)
+        except BaseException:
+            if backup is not None:
+                try:
+                    os.rename(backup, dst)
+                except OSError:
+                    pass  # best-effort restore; the swap failure below still propagates
+            raise
+        if backup is not None:
+            try:
+                os.unlink(backup)
+            except OSError:
+                pass
     except BaseException:
         try:
             os.unlink(tmp_copy)

tests/test_atomic_writes.py

@@ -227,6 +227,47 @@ def test_os_replace_with_fallback_replaces_a_symlink_destination_in_place(tmp_pa
     assert not src.exists()
 
 
+def test_os_replace_with_fallback_restores_destination_on_final_rename_failure(tmp_path, monkeypatch):
+    """The fallback removes the existing destination before renaming the new
+    content into place; if that final rename then fails for any reason, the
+    original content must be restored rather than leaving the destination
+    missing -- a bare unlink-then-rename with no recovery would silently
+    destroy the previous file on a mid-swap failure."""
+    from graphify.paths import os_replace_with_fallback
+
+    dst = tmp_path / "dst.json"
+    dst.write_text("ORIGINAL", encoding="utf-8")
+    src = tmp_path / "src.tmp"
+    src.write_text("NEW", encoding="utf-8")
+
+    def flaky_replace(a, b):
+        exc = OSError("cannot move to a different disk drive")
+        exc.winerror = 17
+        raise exc
+
+    real_rename = os.rename
+    calls = {"n": 0}
+
+    def flaky_rename(a, b):
+        calls["n"] += 1
+        # First rename swaps dst aside as a backup (must succeed so there's
+        # something to restore); force the second -- landing the new
+        # content -- to fail.
+        if calls["n"] == 2:
+            raise OSError("simulated failure landing the new content")
+        return real_rename(a, b)
+
+    monkeypatch.setattr(os, "replace", flaky_replace)
+    monkeypatch.setattr(os, "rename", flaky_rename)
+    with pytest.raises(OSError, match="simulated failure landing the new content"):
+        os_replace_with_fallback(str(src), str(dst))
+
+    assert dst.exists(), "destination must not be left missing after a failed swap"
+    assert dst.read_text(encoding="utf-8") == "ORIGINAL"
+    leftover = {p.name for p in tmp_path.iterdir()}
+    assert leftover == {"dst.json", "src.tmp"}, f"unexpected leftover files: {leftover}"
+
+
 def test_write_json_atomic_ensure_ascii_false_preserves_utf8(tmp_path):
     from graphify.paths import write_json_atomic