fix(utils): retry transient Windows lock failures in atomic_replace

tests/test_atomic_replace_symlinks.py

@@ -21,6 +21,8 @@
 import pytest
 import yaml
 
+import utils
+
 # Ensure the repo root is importable when running via `pytest tests/...`.
 _REPO_ROOT = Path(__file__).resolve().parent.parent
 if str(_REPO_ROOT) not in sys.path:
@@ -88,6 +90,77 @@ def test_atomic_replace_accepts_pathlike_and_str(tmp_path: Path) -> None:
     assert target.read_text(encoding="utf-8") == "2"
 
 
+def _windows_lock_error(winerror: int) -> PermissionError:
+    exc = PermissionError(errno.EACCES, "destination is temporarily locked")
+    exc.winerror = winerror
+    return exc
+
+
+def test_windows_transient_replace_error_classification(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr(utils.os, "name", "nt")
+
+    for winerror in (5, 32, 33):
+        assert utils._is_transient_windows_replace_error(_windows_lock_error(winerror))
+
+    assert not utils._is_transient_windows_replace_error(_windows_lock_error(2))
+
+
+def test_atomic_replace_retries_transient_windows_lock(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    target = tmp_path / "target.yaml"
+    target.write_text("old", encoding="utf-8")
+    tmp = _write_tmp(tmp_path, "new")
+
+    real_replace = os.replace
+    attempts = 0
+    sleeps: list[float] = []
+
+    def flaky_replace(src: str, dst: str) -> None:
+        nonlocal attempts
+        attempts += 1
+        if attempts < 3:
+            raise _windows_lock_error(5)
+        real_replace(src, dst)
+
+    monkeypatch.setattr(utils, "_is_transient_windows_replace_error", lambda _exc: True)
+    monkeypatch.setattr(utils.os, "replace", flaky_replace)
+    monkeypatch.setattr(utils.time, "sleep", sleeps.append)
+
+    atomic_replace(tmp, target)
+
+    assert attempts == 3
+    assert sleeps == [0.05, 0.1]
+    assert target.read_text(encoding="utf-8") == "new"
+
+
+def test_atomic_replace_reraises_after_windows_retry_budget(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    target = tmp_path / "target.yaml"
+    target.write_text("old", encoding="utf-8")
+    tmp = _write_tmp(tmp_path, "new")
+
+    attempts = 0
+
+    def always_locked(_src: str, _dst: str) -> None:
+        nonlocal attempts
+        attempts += 1
+        raise _windows_lock_error(32)
+
+    monkeypatch.setattr(utils, "_WINDOWS_ATOMIC_REPLACE_RETRY_DELAYS", (0.0, 0.0))
+    monkeypatch.setattr(utils, "_is_transient_windows_replace_error", lambda _exc: True)
+    monkeypatch.setattr(utils.os, "replace", always_locked)
+    monkeypatch.setattr(utils.time, "sleep", lambda _delay: None)
+
+    with pytest.raises(PermissionError):
+        atomic_replace(tmp, target)
+
+    assert attempts == 3
+    assert target.read_text(encoding="utf-8") == "old"
+    assert tmp.exists(), "failed replacement must leave the temp file for caller cleanup"
+
+
 # ─── atomic_json_write / atomic_yaml_write wiring ──────────────────────────
 
 

utils.py

@@ -7,6 +7,7 @@
 import shutil
 import stat
 import tempfile
+import time
 from pathlib import Path
 from typing import Any, Union
 from urllib.parse import urlparse
@@ -16,6 +17,20 @@
 logger = logging.getLogger(__name__)
 
 
+# Windows can briefly deny replace/rename while antivirus, indexers, sync clients,
+# or a just-closed editor still holds the destination without FILE_SHARE_DELETE.
+# Keep retries bounded so a real ACL denial still surfaces to the caller.
+_WINDOWS_ATOMIC_REPLACE_RETRY_DELAYS = (0.05, 0.1, 0.2, 0.4, 0.8, 1.0)
+_WINDOWS_TRANSIENT_REPLACE_ERRORS = frozenset({5, 32, 33})
+
+
+def _is_transient_windows_replace_error(exc: OSError) -> bool:
+    return (
+        os.name == "nt"
+        and getattr(exc, "winerror", None) in _WINDOWS_TRANSIENT_REPLACE_ERRORS
+    )
+
+
 TRUTHY_STRINGS = frozenset({"1", "true", "yes", "on"})
 
 
@@ -111,28 +126,50 @@ def atomic_replace(tmp_path: Union[str, Path], target: Union[str, Path]) -> str:
     target_str = str(target)
     real_path = os.path.realpath(target_str) if os.path.islink(target_str) else target_str
     tmp_str = str(tmp_path)
-    try:
-        os.replace(tmp_str, real_path)
-    except OSError as exc:
-        if exc.errno not in (errno.EXDEV, errno.EBUSY):
-            raise
-        logger.debug(
-            "atomic_replace: %s -> %s failed with %s; falling back to copy",
-            tmp_str,
-            real_path,
-            errno.errorcode.get(exc.errno, exc.errno),
-        )
-        shutil.copyfile(tmp_str, real_path)
-        try:
-            shutil.copystat(tmp_str, real_path)
-        except OSError:
-            pass
+    retry_index = 0
+    while True:
         try:
-            with open(real_path, "rb") as f:
-                os.fsync(f.fileno())
-        except OSError:
-            pass
-        os.unlink(tmp_str)
+            os.replace(tmp_str, real_path)
+            break
+        except OSError as exc:
+            if (
+                _is_transient_windows_replace_error(exc)
+                and retry_index < len(_WINDOWS_ATOMIC_REPLACE_RETRY_DELAYS)
+            ):
+                delay = _WINDOWS_ATOMIC_REPLACE_RETRY_DELAYS[retry_index]
+                retry_index += 1
+                logger.debug(
+                    "atomic_replace: transient Windows lock %s for %s -> %s; "
+                    "retrying in %.2fs (%d/%d)",
+                    getattr(exc, "winerror", None),
+                    tmp_str,
+                    real_path,
+                    delay,
+                    retry_index,
+                    len(_WINDOWS_ATOMIC_REPLACE_RETRY_DELAYS),
+                )
+                time.sleep(delay)
+                continue
+            if exc.errno not in (errno.EXDEV, errno.EBUSY):
+                raise
+            logger.debug(
+                "atomic_replace: %s -> %s failed with %s; falling back to copy",
+                tmp_str,
+                real_path,
+                errno.errorcode.get(exc.errno, exc.errno),
+            )
+            shutil.copyfile(tmp_str, real_path)
+            try:
+                shutil.copystat(tmp_str, real_path)
+            except OSError:
+                pass
+            try:
+                with open(real_path, "rb") as f:
+                    os.fsync(f.fileno())
+            except OSError:
+                pass
+            os.unlink(tmp_str)
+            break
     return real_path