mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 01:06:36 +00:00
fix(cache): stat-index must follow the cache location, not the key anchor
Completes #1774. The prior fix redirected the AST cache dir to CWD but file_hash still called _ensure_stat_index(root) without the cache location, so the hash fastpath's stat-index.json kept anchoring on the key-root (the analyzed corpus) — leaving a stray graphify-out/cache/ stat-index.json inside a writable foreign corpus even though the AST cache itself had moved to CWD. Thread cache_root through file_hash -> _ensure_stat_index (which already accepts it, #1747). Surfaced by an out-of-CWD parallel-extract edge case: the leak was masked in the in-process test suite because _stat_index_root is a set-once module global that an earlier test had already pinned. The regression test resets that global to simulate a fresh process and asserts the corpus stays clean while the stat index lands under CWD. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c3a42a35eb
commit
94d3099540
+8
-4
@@ -159,7 +159,7 @@ def _normalize_path(path: Path) -> Path:
|
||||
return Path(os.path.normcase(s))
|
||||
|
||||
|
||||
def file_hash(path: Path, root: Path = Path(".")) -> str:
|
||||
def file_hash(path: Path, root: Path = Path("."), cache_root: "Path | None" = None) -> str:
|
||||
"""SHA256 of file contents + path relative to root.
|
||||
|
||||
Uses a stat-based fastpath (size + mtime_ns) to skip full reads when the
|
||||
@@ -179,7 +179,11 @@ def file_hash(path: Path, root: Path = Path(".")) -> str:
|
||||
if not p.is_file():
|
||||
raise IsADirectoryError(f"file_hash requires a file, got: {p}")
|
||||
|
||||
_ensure_stat_index(root)
|
||||
# The stat index is a cache artifact, so it must follow the cache location
|
||||
# (cache_root), not the key-anchor root — otherwise it leaves a stray
|
||||
# graphify-out/cache/stat-index.json inside the analyzed source tree even when
|
||||
# the AST cache itself is redirected to CWD (#1774 completion).
|
||||
_ensure_stat_index(root, cache_root=cache_root)
|
||||
abs_key = str(p.resolve())
|
||||
st: "os.stat_result | None" = None
|
||||
try:
|
||||
@@ -377,7 +381,7 @@ def load_cached(path: Path, root: Path = Path("."), kind: str = "ast",
|
||||
"""
|
||||
location = cache_root if cache_root is not None else root
|
||||
try:
|
||||
h = file_hash(path, root)
|
||||
h = file_hash(path, root, cache_root=cache_root)
|
||||
except OSError:
|
||||
return None
|
||||
entry = cache_dir(location, kind) / f"{h}.json"
|
||||
@@ -428,7 +432,7 @@ def save_cached(path: Path, result: dict, root: Path = Path("."), kind: str = "a
|
||||
import copy as _copy
|
||||
on_disk = _copy.deepcopy(result)
|
||||
_relativize_source_files_in(on_disk, root)
|
||||
h = file_hash(p, root)
|
||||
h = file_hash(p, root, cache_root=cache_root)
|
||||
location = cache_root if cache_root is not None else root
|
||||
target_dir = cache_dir(location, kind)
|
||||
entry = target_dir / f"{h}.json"
|
||||
|
||||
@@ -17,9 +17,19 @@ from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
import graphify.extract as ex
|
||||
import graphify.cache as cache
|
||||
from graphify.cache import load_cached, file_hash
|
||||
|
||||
|
||||
def _reset_stat_index():
|
||||
"""The stat-index location is chosen once per process via a module global
|
||||
(#1747). Reset it so a test sees a fresh-process decision — otherwise an
|
||||
earlier test pins the location and masks where THIS extract would write it."""
|
||||
cache._stat_index_root = None
|
||||
cache._stat_index = {}
|
||||
cache._stat_index_dirty = False
|
||||
|
||||
|
||||
def _make_corpus(base: Path) -> Path:
|
||||
corpus = base / "corpus"
|
||||
corpus.mkdir()
|
||||
@@ -29,6 +39,7 @@ def _make_corpus(base: Path) -> Path:
|
||||
|
||||
|
||||
def test_default_cache_lands_in_cwd_not_source_tree(tmp_path, monkeypatch):
|
||||
_reset_stat_index()
|
||||
corpus = _make_corpus(tmp_path)
|
||||
work = tmp_path / "work"
|
||||
work.mkdir()
|
||||
@@ -37,13 +48,38 @@ def test_default_cache_lands_in_cwd_not_source_tree(tmp_path, monkeypatch):
|
||||
result = ex.extract([corpus / "a.py", corpus / "b.py"], parallel=False)
|
||||
|
||||
assert result["nodes"], "extraction should still produce nodes"
|
||||
# Nothing at all in the source tree — not the AST cache, and not the
|
||||
# stat-index.json the hash fastpath writes (which file_hash used to anchor on
|
||||
# the key-root, leaving a stray graphify-out/ in a writable corpus, #1774).
|
||||
assert not (corpus / "graphify-out").exists(), (
|
||||
"cache written into the analyzed source tree (#1774)"
|
||||
"cache/stat-index written into the analyzed source tree (#1774)"
|
||||
)
|
||||
assert (work / "graphify-out" / "cache").is_dir(), "cache should land under CWD"
|
||||
|
||||
|
||||
def test_default_cache_does_not_leave_stat_index_in_source_tree(tmp_path, monkeypatch):
|
||||
"""Fresh-process regression for the stat-index leak specifically: even for a
|
||||
WRITABLE out-of-CWD corpus (where the write would succeed), file_hash's
|
||||
stat-index must follow the cache location, not the key anchor (#1774)."""
|
||||
_reset_stat_index()
|
||||
corpus = _make_corpus(tmp_path)
|
||||
work = tmp_path / "elsewhere"
|
||||
work.mkdir()
|
||||
monkeypatch.chdir(work)
|
||||
|
||||
ex.extract([corpus / "a.py", corpus / "b.py"], parallel=False)
|
||||
# The stat index is buffered in memory and flushed at interpreter exit; force
|
||||
# the flush now so we can assert WHERE it lands.
|
||||
cache._flush_stat_index()
|
||||
|
||||
assert not (corpus / "graphify-out").exists(), "stat-index leaked into the corpus"
|
||||
assert (work / "graphify-out" / "cache" / "stat-index.json").exists(), (
|
||||
"stat-index should be written under the cache location (CWD)"
|
||||
)
|
||||
|
||||
|
||||
def test_explicit_cache_root_still_wins(tmp_path, monkeypatch):
|
||||
_reset_stat_index()
|
||||
corpus = _make_corpus(tmp_path)
|
||||
work = tmp_path / "work"
|
||||
work.mkdir()
|
||||
|
||||
Reference in New Issue
Block a user