Preserve replace semantics for a symlinked destination

os.replace atomically swaps whatever sits at the destination, a
symlink included, by replacing the link itself. install.py relies on
exactly that for its managed skill symlinks, per #3286. The
fallback's plain shutil.copy2 does the opposite when the destination
is a symlink: opening it for writing follows the link and overwrites
whatever it points at instead of the link. Copies to a fresh temp
file beside the destination first, then swaps the destination out
from under it with an unlink plus rename, matching replace's own
behavior and, as a side effect, shrinking the window where a reader
could see a half written file down to two fast metadata operations
rather than however long the copy itself takes. 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

graphify/paths.py

@@ -27,24 +27,52 @@ GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out")
 
 
 def os_replace_with_fallback(src: "str | Path", dst: "str | Path") -> None:
-    """``os.replace(src, dst)``, falling back to copy-then-delete for a known
-    set of Windows quirks (#3508) that raise even when ``src``/``dst`` are the
-    same directory on the same drive: ``PermissionError`` (WinError 5/32 --
+    """``os.replace(src, dst)``, falling back to a copy for a known set of
+    Windows quirks (#3508) that raise even when ``src``/``dst`` are the same
+    directory on the same drive: ``PermissionError`` (WinError 5/32 --
     destination briefly locked by another handle, antivirus, an open reader)
     and WinError 17 ("cannot move to a different disk drive", observed on some
     Windows/filesystem combinations despite textbook same-volume semantics).
     WinError 17 maps to a plain ``OSError`` in Python, not ``PermissionError``,
     so it's checked via ``winerror`` rather than the exception type. Any other
     failure is a real one and is re-raised.
+
+    ``os.replace`` atomically swaps whatever sits at ``dst`` -- including a
+    symlink, which it REPLACES in place rather than following (some callers,
+    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.
     """
     try:
         os.replace(src, dst)
+        return
     except OSError as exc:
         if not isinstance(exc, PermissionError) and getattr(exc, "winerror", None) != 17:
             raise
-        import shutil
-        shutil.copy2(src, dst)
-        os.unlink(src)
+    import shutil
+    dst = os.fspath(dst)
+    fd, tmp_copy = tempfile.mkstemp(dir=os.path.dirname(dst) or ".", prefix=".gfy-replace-", suffix=".tmp")
+    os.close(fd)
+    try:
+        shutil.copy2(src, tmp_copy)
+        try:
+            os.unlink(dst)
+        except FileNotFoundError:
+            pass
+        os.rename(tmp_copy, dst)
+    except BaseException:
+        try:
+            os.unlink(tmp_copy)
+        except OSError:
+            pass
+        raise
+    os.unlink(src)
 
 
 def _atomic_replace(path: "str | Path", write_fn) -> None:

tests/test_atomic_writes.py

@@ -6,6 +6,7 @@ directory then `os.replace`s it into place; on failure the original is untouched
 """
 import json
 import os
+import sys
 
 import pytest
 
@@ -192,6 +193,40 @@ def test_write_text_atomic_windows_winerror_17_fallback(tmp_path, monkeypatch):
     assert sorted(x.name for x in tmp_path.iterdir()) == ["graph.json"]
 
 
+@pytest.mark.skipif(sys.platform == "win32", reason="symlink setup differs on Windows")
+def test_os_replace_with_fallback_replaces_a_symlink_destination_in_place(tmp_path, monkeypatch):
+    """`os.replace` replaces a symlinked destination itself rather than
+    following it -- install.py relies on exactly this for managed skill
+    symlinks (#3286). 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. The #3508 fallback must preserve replace's
+    semantics, not copy2's, or a Windows quirk that triggers the fallback
+    would silently clobber whatever a managed symlink pointed at."""
+    from graphify.paths import os_replace_with_fallback
+
+    target = tmp_path / "shared_target.txt"
+    target.write_text("ORIGINAL SHARED CONTENT", encoding="utf-8")
+    link = tmp_path / "skill_link"
+    link.symlink_to(target)
+    src = tmp_path / "tmp_new.txt"
+    src.write_text("NEW CONTENT", encoding="utf-8")
+
+    def flaky_replace(a, b):
+        exc = OSError("cannot move to a different disk drive")
+        exc.winerror = 17
+        raise exc
+
+    monkeypatch.setattr(os, "replace", flaky_replace)
+    os_replace_with_fallback(str(src), str(link))
+
+    assert not link.is_symlink(), "link must be replaced by a plain file, not left as a symlink"
+    assert link.read_text(encoding="utf-8") == "NEW CONTENT"
+    assert target.read_text(encoding="utf-8") == "ORIGINAL SHARED CONTENT", (
+        "the shared target must be untouched -- a copy2-through-the-link would have clobbered it"
+    )
+    assert not src.exists()
+
+
 def test_write_json_atomic_ensure_ascii_false_preserves_utf8(tmp_path):
     from graphify.paths import write_json_atomic