From 7638d47565b0e8e4acc9178a045c8383028dd4c2 Mon Sep 17 00:00:00 2001 From: ayushcodes10 Date: Sat, 12 Sep 2026 13:30:03 +0530 Subject: [PATCH] Use the replace fallback for AST cache writes The AST extraction cache writes its own temp file and replace, a separate code path from the shared atomic writer, and only caught PermissionError. This was the exact write named in the report: worker failed for a file with WinError 17 while replacing a cache entry under the output directory's ast cache. Also covers the stat index write, which had no Windows fallback at all. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh --- graphify/cache.py | 12 +++--------- tests/test_cache.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/graphify/cache.py b/graphify/cache.py index d6060ed2b..a70cff03d 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -17,6 +17,7 @@ from pathlib import Path # absolute path ("/shared/graphify-out"). Single source of truth in graphify.paths # (#1423); re-exported here as _GRAPHIFY_OUT for the existing call sites. from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT +from graphify.paths import os_replace_with_fallback as _os_replace_with_fallback # AST cache entries are the output of graphify's own extractor code, so they # are only valid for the version that wrote them: keying purely on file @@ -399,7 +400,7 @@ def _flush_stat_index() -> None: try: os.write(fd, json.dumps(on_disk, separators=(",", ":")).encode()) os.close(fd) - os.replace(tmp, p) + _os_replace_with_fallback(tmp, p) except Exception: try: os.close(fd) @@ -1126,14 +1127,7 @@ def save_cached(path: Path, result: dict, root: Path = Path("."), kind: str = "a try: os.write(fd, json.dumps(on_disk).encode()) os.close(fd) - try: - os.replace(tmp_path, entry) - except PermissionError: - # Windows: os.replace can fail with WinError 5 if the target is - # briefly locked. Fall back to copy-then-delete. - import shutil - shutil.copy2(tmp_path, entry) - os.unlink(tmp_path) + _os_replace_with_fallback(tmp_path, entry) except Exception: try: os.close(fd) diff --git a/tests/test_cache.py b/tests/test_cache.py index f01a1cd29..a1328d0a5 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -221,6 +221,37 @@ def test_save_cached_relativizes_source_file(tmp_path): assert edge_sources == {"src/foo.py"} +def test_save_cached_survives_a_winerror_17_replace(tmp_path, monkeypatch): + """#3508: on some Windows/filesystem combinations, `os.replace` raises + WinError 17 ("cannot move to a different disk drive") even for a temp file + and destination in the same directory on the same drive -- reported for + exactly this AST cache write path. WinError 17 is a plain OSError, not a + PermissionError, so it must still trigger the copy-then-delete fallback.""" + import os + from graphify.cache import save_cached, file_hash, cache_dir + + src = tmp_path / "foo.py" + src.write_text("def x(): pass\n") + + real_replace = os.replace + + 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) + try: + save_cached(src, {"nodes": [], "edges": []}, root=tmp_path, kind="ast") + finally: + monkeypatch.setattr(os, "replace", real_replace) + + h = file_hash(src, tmp_path) + entry = cache_dir(tmp_path, "ast") / f"{h}.json" + assert entry.exists() + assert not any(p.name.endswith(".tmp") for p in entry.parent.iterdir()) + + def test_load_cached_absolutizes_source_file(tmp_path): """``load_cached`` returns the same absolute-path shape that a fresh extraction produces, so consumers don't need to special-case cache