diff --git a/graphify/cache.py b/graphify/cache.py index bdb6cbd1e0..833fdaa861 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -22,7 +22,8 @@ # are only valid for the version that wrote them: keying purely on file # content means extractor fixes shipped in a new release keep serving stale # pre-fix results. The AST cache is therefore namespaced by package version -# (cache/ast/v{version}/), with entries from other versions removed on first +# and cache-key schema (cache/ast/v{version}-s{schema}/), with entries from +# other versions or schemas removed on first # use. The semantic cache is deliberately NOT versioned — its entries are # produced by the LLM from file contents, and invalidating them on every # release would re-bill extraction for unchanged files. @@ -33,6 +34,9 @@ except Exception: _EXTRACTOR_VERSION = "unknown" +# Bump when AST cache-key semantics change independently of the package version. +_AST_CACHE_SCHEMA = 2 + # Version dirs already swept this process — cleanup runs once per (base, version). _cleaned_ast_dirs: set[str] = set() @@ -416,9 +420,10 @@ def file_hash(path: Path, root: Path = Path("."), cache_root: "Path | None" = No (see :func:`_stat_sig_fresh`) — so two different contents can never share a digest. Index is flushed atomically at process exit. - Using a relative path (not absolute) makes cache entries portable across - machines and checkout directories, so shared caches and CI work correctly. - Falls back to the resolved absolute path if the file is outside root. + Using the walked path relative to root keeps distinct symlink aliases from + sharing an extraction entry while preserving portability across machines + and checkout directories. Falls back to the resolved path when the walked + path cannot be expressed relative to root. For Markdown files (.md), only the body below the YAML frontmatter is hashed, so metadata-only changes (e.g. reviewed, status, tags) do not invalidate the cache. @@ -443,10 +448,35 @@ def file_hash(path: Path, root: Path = Path("."), cache_root: "Path | None" = No # path served whichever was computed first — making file_hash order-dependent # and poisoning the persisted stat-index across runs (#1989). Store one digest # per salt so alternating roots don't force re-reads. + resolved_root = root.resolve() try: - salt = resolved.relative_to(Path(root).resolve()).as_posix().lower() + resolved_rel = resolved.relative_to(resolved_root) except ValueError: + # Preserve the existing fallback for a target outside the corpus. An + # in-root symlink to such a target is excluded by collect_files(), but + # direct cache callers still rely on the resolved external identity. salt = resolved.as_posix().lower() + else: + walked = Path(os.path.abspath(p)) + walked_root = Path(os.path.abspath(root)) + try: + walked_rel = walked.relative_to(walked_root) + except ValueError: + # extract() resolves its operational root, while paths collected + # through a symlinked scan root retain that walked spelling. Find + # the lexical ancestor representing the resolved corpus root so a + # leaf symlink still contributes its own relative path to the key. + walked_rel = None + for parent in walked.parents: + try: + if parent.resolve() == resolved_root: + walked_rel = walked.relative_to(parent) + break + except OSError: + continue + if walked_rel is None: + walked_rel = resolved_rel + salt = walked_rel.as_posix().lower() st: "os.stat_result | None" = None try: @@ -612,6 +642,29 @@ def _normalize_source_file_value(src: "str | Path", root_resolved: Path) -> str: return rel.replace(os.sep, "/") +def _semantic_entry_matches_path(result: dict, path: Path, root: Path) -> bool: + """Whether cached semantic groups belong to the requested walked path. + + Before walked paths entered the cache salt, a symlink could overwrite its + target's unversioned semantic entry. Rejecting that mismatched legacy + payload makes the next extraction self-heal instead of replaying it forever. + """ + expected = _normalize_path(Path(os.path.abspath(path))) + for bucket in ("nodes", "edges", "hyperedges"): + for item in result.get(bucket, []): + if not isinstance(item, dict): + continue + source = item.get("source_file") + if not source: + continue + source_path = Path(source) + if not source_path.is_absolute(): + source_path = Path(root) / source_path + if _normalize_path(Path(os.path.abspath(source_path))) != expected: + return False + return True + + # Storage marker standing in for the absolute root a cached id/path was minted # under (#2257). Extractors mint node ids from the path STRING they are handed # (``_make_id(str(path))``, ``_file_node_id(path)``), so a cache entry written @@ -844,9 +897,10 @@ def cache_dir(root: Path = Path("."), kind: str = "ast", "semantic-deep" (#1894). Separate subdirectories prevent semantic cache entries from overwriting AST cache entries for the same source_file (#582). - AST entries live in graphify-out/cache/ast/v{version}/ — namespaced by - graphify version because they depend on extractor code, not just file - contents. Semantic entries are still NOT version-namespaced (re-extraction + AST entries live in graphify-out/cache/ast/v{version}-s{schema}/, namespaced + by graphify version and cache-key schema because they depend on extractor + code and key semantics, not just file contents. Semantic entries are still + NOT version-namespaced (re-extraction costs LLM calls, #1252): they live in graphify-out/cache/semantic/, with deep-mode entries beside them in graphify-out/cache/semantic-deep/. @@ -859,7 +913,7 @@ def cache_dir(root: Path = Path("."), kind: str = "ast", base = _out if _out.is_absolute() else Path(root).resolve() / _out d = base / "cache" / kind if kind == "ast": - d = d / f"v{_EXTRACTOR_VERSION}" + d = d / f"v{_EXTRACTOR_VERSION}-s{_AST_CACHE_SCHEMA}" _cleanup_stale_ast_entries(d.parent, d) elif prompt_fp: d = d / f"p{prompt_fp}" @@ -938,6 +992,12 @@ def load_cached(path: Path, root: Path = Path("."), kind: str = "ast", # across chunks without losing the truncated one (it stays partial). if not allow_partial and isinstance(result, dict) and result.get("partial"): return None + if ( + kind.startswith("semantic") + and isinstance(result, dict) + and not _semantic_entry_matches_path(result, Path(path), Path(root)) + ): + return None if legacy_hit: _legacy_semantic_hits += 1 # Re-anchor relative source_file fields so callers see the same @@ -1290,7 +1350,8 @@ def save_semantic_cache( from collections import defaultdict kind = "semantic" if mode is None else f"semantic-{mode}" - root_path = Path(root).resolve() + root_walked = _normalize_path(Path(os.path.abspath(root))) + root_resolved = _normalize_path(Path(root).resolve()) def _normalized(item: dict) -> dict: """Copy of ``item`` with a portable ``source_file`` (#2197). @@ -1305,7 +1366,9 @@ def _normalized(item: dict) -> dict: src = item.get("source_file") if not src: return item - norm = _normalize_source_file_value(src, root_path) + norm = _normalize_source_file_value(src, root_walked) + if Path(norm).is_absolute() and root_walked != root_resolved: + norm = _normalize_source_file_value(src, root_resolved) if norm != src: item = {**item, "source_file": norm} return item @@ -1327,10 +1390,23 @@ def _normalized(item: dict) -> dict: if src: by_file[src]["hyperedges"].append(h) - def resolved_source_path(value: str | Path) -> Path: + def source_path(value: str | Path) -> Path: + """Return the normalized walked identity for a semantic group.""" path = Path(value) if not path.is_absolute(): - path = root_path / path + path = root_walked / path + elif root_walked != root_resolved: + normalized = _normalize_path(Path(os.path.abspath(path))) + try: + relative = normalized.relative_to(root_resolved) + except ValueError: + pass + else: + path = root_walked / relative + return _normalize_path(Path(os.path.abspath(path))) + + def resolved_source_path(value: str | Path) -> Path: + path = source_path(value) try: return path.resolve() except (OSError, RuntimeError): @@ -1340,18 +1416,18 @@ def resolved_source_path(value: str | Path) -> Path: allowed_paths = None if allowed_source_files is not None: - allowed_paths = {resolved_source_path(path) for path in allowed_source_files} + allowed_paths = {source_path(path) for path in allowed_source_files} partial_paths = None if partial_source_files is not None: - partial_paths = {resolved_source_path(path) for path in partial_source_files} + partial_paths = {source_path(path) for path in partial_source_files} # A chunk that truncated to an EMPTY parse contributes no grouped items, # so its file is absent from by_file and the write loop below would never # stamp it partial — leaving a prior clean slice looking complete (#1950 # empty-parse gap). Seed an empty group for each named partial file that # isn't already present, so the loop merges its existing entry and stamps - # it partial. Keyed by the resolved path (deduped against present groups). - _present = {resolved_source_path(k) for k in by_file} + # it partial. Keyed by walked path (deduped against present groups). + _present = {source_path(k) for k in by_file} for _pp in partial_paths: if _pp not in _present: by_file[str(_pp)] # defaultdict: create an empty {nodes,edges,hyperedges} @@ -1359,7 +1435,9 @@ def resolved_source_path(value: str | Path) -> Path: def group_skipped(fpath: str) -> bool: """Mirror the write-loop skip condition for one source_file group.""" p = resolved_source_path(fpath) - return not p.is_file() or (allowed_paths is not None and p not in allowed_paths) + return not p.is_file() or ( + allowed_paths is not None and source_path(fpath) not in allowed_paths + ) # Dangling-reference pruning (#1916). A node group is skipped by the write # loop below when its source_file is not a real file (ghost path) or is @@ -1415,9 +1493,10 @@ def hyperedge_dangles(h: dict) -> bool: saved = 0 skipped_not_file = 0 for fpath, result in by_file.items(): + cache_path = source_path(fpath) p = resolved_source_path(fpath) if p.is_file(): - if allowed_paths is not None and p not in allowed_paths: + if allowed_paths is not None and cache_path not in allowed_paths: warnings.warn( "semantic cache skipped out-of-scope source_file " f"{fpath!r}; the file was not dispatched for extraction", @@ -1437,7 +1516,7 @@ def hyperedge_dangles(h: dict) -> bool: # markers ride through, so is_partial below re-detects it) rather # than a later clean slice silently replacing it and promoting the # half-file to complete. - prev = load_cached(p, root, kind=kind, cache_root=cache_root, + prev = load_cached(cache_path, root, kind=kind, cache_root=cache_root, prompt=prompt, prompt_file=prompt_file, allow_legacy=False, allow_partial=True) _prev_partial = bool(prev.get("partial")) if prev else False @@ -1458,13 +1537,13 @@ def hyperedge_dangles(h: dict) -> bool: # complete re-extraction (merge_existing=False) overwrites the # content-hash key with a non-partial entry that then serves normally. is_partial = ( - (partial_paths is not None and p in partial_paths) + (partial_paths is not None and cache_path in partial_paths) or _group_has_partial_marker(result) or _prev_partial ) if is_partial: result = {**result, "partial": True} - save_cached(p, result, root, kind=kind, cache_root=cache_root, + save_cached(cache_path, result, root, kind=kind, cache_root=cache_root, prompt=prompt, prompt_file=prompt_file) saved += 1 else: diff --git a/tests/test_cache.py b/tests/test_cache.py index 67135b3932..920526dc18 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -585,6 +585,29 @@ def test_ast_cache_invalidated_on_version_bump(tmp_path, monkeypatch): ) +def test_ast_cache_schema_rejects_same_version_legacy_collision( + tmp_path, monkeypatch +): + """A key-schema change must not replay a poisoned same-version AST entry.""" + import json + import graphify.cache as cache_mod + + target = tmp_path / "real.py" + target.write_text("value = 1\n") + monkeypatch.setattr(cache_mod, "_EXTRACTOR_VERSION", "0.9.46", raising=False) + old_dir = tmp_path / cache_mod._GRAPHIFY_OUT / "cache" / "ast" / "v0.9.46" + old_dir.mkdir(parents=True) + old_hash = file_hash(target, tmp_path) + (old_dir / f"{old_hash}.json").write_text(json.dumps({ + "nodes": [{"id": "alias", "source_file": "alias.py"}], + "edges": [], + })) + monkeypatch.setattr(cache_mod, "_cleaned_ast_dirs", set(), raising=False) + + assert load_cached(target, root=tmp_path, kind="ast") is None + assert not old_dir.exists() + + def test_ast_cache_version_bump_cleans_stale_entries(tmp_path, monkeypatch): """Upgrading removes AST entries left behind by previous versions so the cache directory does not grow one full copy per release.""" @@ -682,6 +705,246 @@ def test_save_cached_in_root_symlink_keeps_symlink_name(tmp_path): ) +def test_file_hash_distinguishes_walked_symlink_paths_portably( + requires_symlinks, tmp_path +): + """Aliases of one target need separate portable extraction-cache keys.""" + from graphify import cache as cache_mod + + _reset_stat_index() + hashes_by_root = [] + for dirname in ("repo_a", "repo_b"): + root = tmp_path / dirname + (root / "sub").mkdir(parents=True) + target = root / "real.py" + target.write_text("def value():\n return 1\n") + aliases = (root / "alias.py", root / "sub" / "link.py") + aliases[0].symlink_to(target) + aliases[1].symlink_to(target) + + hashes = tuple(file_hash(path, root) for path in (target, *aliases)) + assert len(set(hashes)) == 3 + assert len(cache_mod._stat_index[str(target.resolve())]["hashes"]) == 3 + hashes_by_root.append(hashes) + + assert hashes_by_root[0] == hashes_by_root[1] + + +def test_file_hash_keeps_resolved_fallback_for_external_symlink( + requires_symlinks, tmp_path +): + """An out-of-root target retains the existing resolved-path identity.""" + _reset_stat_index() + root = tmp_path / "repo" + root.mkdir() + target = tmp_path / "external.py" + target.write_text("external = True\n") + alias = root / "external.py" + alias.symlink_to(target) + + assert file_hash(alias, root) == file_hash(target, root) + + +def test_warm_cache_keeps_target_and_symlink_sources_distinct( + requires_symlinks, tmp_path, monkeypatch +): + """#2832: a warm cache must not move target nodes onto its symlink.""" + from collections import Counter + + import graphify.extract as extract_mod + + _reset_stat_index() + physical_root = tmp_path / "repo" + (physical_root / "sub").mkdir(parents=True) + target = physical_root / "real.py" + target.write_text("def value():\n return 1\n") + alias = physical_root / "sub" / "link.py" + alias.symlink_to(target) + root = tmp_path / "scan" + root.symlink_to(physical_root, target_is_directory=True) + + paths = extract_mod.collect_files(root) + assert [path.relative_to(root).as_posix() for path in paths] == [ + "real.py", + "sub/link.py", + ] + + cold = extract_mod.extract(paths, cache_root=root, root=root, parallel=False) + misses = [] + real_extract = extract_mod._safe_extract_with_xaml_root + + def counting_extract(extractor, path, extract_root): + misses.append(path) + return real_extract(extractor, path, extract_root) + + monkeypatch.setattr(extract_mod, "_safe_extract_with_xaml_root", counting_extract) + warm = extract_mod.extract(paths, cache_root=root, root=root, parallel=False) + + assert misses == [] + cold_counts = Counter(n.get("source_file") for n in cold["nodes"]) + warm_counts = Counter(n.get("source_file") for n in warm["nodes"]) + assert warm_counts == cold_counts + assert len(cold_counts) == 2 + assert {Path(source).name for source in cold_counts} == {"real.py", "link.py"} + + +def test_semantic_cache_self_heals_legacy_symlink_collision( + requires_symlinks, tmp_path +): + """A poisoned legacy entry misses once, then walked groups round-trip.""" + import json + + from graphify.cache import check_semantic_cache, save_semantic_cache + + _reset_stat_index() + physical_root = tmp_path / "repo" + physical_root.mkdir() + (physical_root / "real.md").write_text("# Shared\n") + (physical_root / "alias.md").symlink_to(physical_root / "real.md") + root = tmp_path / "scan" + root.symlink_to(physical_root, target_is_directory=True) + target = root / "real.md" + alias = root / "alias.md" + + legacy_hash = file_hash(target, root) + legacy_entry = cache_dir(root, "semantic") / f"{legacy_hash}.json" + legacy_entry.write_text(json.dumps({ + "nodes": [{"id": "alias-old", "source_file": "alias.md"}], + "edges": [], + })) + + nodes, _, _, uncached = check_semantic_cache( + [str(target), str(alias)], root=root + ) + assert nodes == [] + assert uncached == [str(target), str(alias)] + + saved = save_semantic_cache( + [ + {"id": "real", "source_file": str(target)}, + {"id": "alias", "source_file": str(alias)}, + ], + [], + root=root, + ) + stored_sources = [] + for path in (target, alias): + entry = cache_dir(root, "semantic") / f"{file_hash(path, root)}.json" + stored_sources.append(json.loads(entry.read_text())["nodes"][0]["source_file"]) + nodes, _, _, uncached = check_semantic_cache( + [str(target), str(alias)], root=root + ) + + assert saved == 2 + assert stored_sources == ["real.md", "alias.md"] + assert [node["id"] for node in nodes] == ["real", "alias"] + assert uncached == [] + + +def test_semantic_symlink_policy_uses_walked_identity( + requires_symlinks, tmp_path +): + """Alias authorization and partial state must not leak to its target.""" + from graphify.cache import load_cached, save_semantic_cache + + _reset_stat_index() + target = tmp_path / "real.md" + target.write_text("# Shared\n") + alias = tmp_path / "alias.md" + alias.symlink_to(target) + + with pytest.warns(RuntimeWarning, match="out-of-scope source_file 'alias.md'"): + saved = save_semantic_cache( + [ + {"id": "real", "source_file": "real.md"}, + {"id": "alias", "source_file": "alias.md"}, + ], + [], + root=tmp_path, + allowed_source_files=[target], + partial_source_files=[alias], + ) + + target_entry = load_cached( + target, root=tmp_path, kind="semantic", allow_partial=True + ) + assert saved == 1 + assert target_entry is not None + assert target_entry.get("partial") is not True + assert load_cached(alias, root=tmp_path, kind="semantic") is None + + +def test_semantic_symlink_root_accepts_resolved_policy_paths_without_alias_leak( + requires_symlinks, tmp_path +): + """Resolved root spellings apply only to the matching walked identity.""" + from graphify.cache import load_cached, save_semantic_cache + + _reset_stat_index() + physical_root = tmp_path / "repo" + physical_root.mkdir() + target = physical_root / "real.md" + target.write_text("# Shared\n") + alias = physical_root / "alias.md" + alias.symlink_to(target) + root = tmp_path / "scan" + root.symlink_to(physical_root, target_is_directory=True) + walked_target = root / "real.md" + walked_alias = root / "alias.md" + + with pytest.warns(RuntimeWarning, match="out-of-scope source_file"): + saved = save_semantic_cache( + [ + {"id": "real", "source_file": str(walked_target)}, + {"id": "alias", "source_file": str(walked_alias)}, + ], + [], + root=root, + allowed_source_files=[walked_target.resolve()], + partial_source_files=[walked_target.resolve()], + ) + + target_entry = load_cached( + walked_target, root=root, kind="semantic", allow_partial=True + ) + assert saved == 1 + assert target_entry is not None + assert [node["id"] for node in target_entry["nodes"]] == ["real"] + assert target_entry["partial"] is True + assert load_cached(walked_target, root=root, kind="semantic") is None + assert load_cached(walked_alias, root=root, kind="semantic") is None + + +def test_semantic_symlink_root_keeps_external_policy_path_absolute( + requires_symlinks, tmp_path +): + """An allowed absolute source outside a symlinked root stays external.""" + from graphify.cache import load_cached, save_semantic_cache + + _reset_stat_index() + physical_root = tmp_path / "repo" + physical_root.mkdir() + root = tmp_path / "scan" + root.symlink_to(physical_root, target_is_directory=True) + external = tmp_path / "external.md" + external.write_text("# External\n") + + saved = save_semantic_cache( + [{"id": "external", "source_file": str(external)}], + [], + root=root, + allowed_source_files=[external], + partial_source_files=[external], + ) + + entry = load_cached(external, root=root, kind="semantic", allow_partial=True) + assert saved == 1 + assert entry is not None + assert Path(entry["nodes"][0]["source_file"]) == external + assert entry["partial"] is True + assert load_cached(external, root=root, kind="semantic") is None + + def test_semantic_prune_removes_orphan_entries(tmp_path): """Changing a file's content leaves the old content-hash entry orphaned; pruning against the new live hash removes the stale entry and keeps the