From 92dfc56e287ec4b42ebd0f623c3a21cd851a3c68 Mon Sep 17 00:00:00 2001 From: L4XB Date: Tue, 15 Sep 2026 22:30:28 +0200 Subject: [PATCH 1/2] fix(detect): relativize manifest keys that spell root differently ``_to_relative_for_storage`` compares text. A stored absolute key naming a file under ``root`` through a symlinked checkout path, or with different case on a case-insensitive filesystem, relativizes to ``../...`` and is kept absolute for ever. The next save writes the fresh relative key beside it, so the manifest describes the same file twice and the stale row never expires. When the lexical answer says out-of-root, ask the filesystem before accepting it: walk the key's ancestors for the one that ``os.path.samefile`` matches the resolved root, and join the remainder with its original casing. Keys with no such ancestor still round-trip absolute, and the lexical fast path is unchanged, so in-root keys pay nothing. --- graphify/detect.py | 44 ++++++++++++++++++-- tests/test_detect.py | 99 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 3 deletions(-) diff --git a/graphify/detect.py b/graphify/detect.py index eec8aaf1d2..200fead8b9 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -2111,6 +2111,40 @@ def _nfc(s: str) -> str: return unicodedata.normalize("NFC", s) +def _anchor_under_root(key_path: Path, root_res: Path) -> "str | None": + """Relativize ``key_path`` against ``root_res`` by identity, not by text. + + ``os.path.relpath`` is lexical and case-sensitive. Windows paths are not, + and a default macOS volume is not either, so a stored key whose case + differs from the resolved root ("C:\\Users\\Foo" against "C:\\Users\\foo", + a parent directory renamed only in case) reads as out-of-root and stays + absolute for ever. An upgrade that changes the key convention then leaves + the old key beside the new one rather than replacing it, and the manifest + ends up describing the same tree twice (#3581). + + Walks the key's ancestors looking for the one that IS ``root_res`` + according to the filesystem, and joins the remainder with its original + casing intact. Returns None when no ancestor matches, which is the + genuinely out-of-root case. Only reached after the lexical attempt has + already failed, so the ordinary in-root path costs nothing. + """ + parts: list[str] = [] + current = key_path + while True: + parent = current.parent + if parent == current: + return None + parts.append(current.name) + current = parent + try: + if os.path.samefile(current, root_res): + return "/".join(reversed(parts)) + except OSError: + # An ancestor we cannot stat cannot be shown to be root; leave the + # key absolute, as it was before this fallback existed. + return None + + def _to_relative_for_storage(key: str, root: Path) -> str: """Return ``key`` as a forward-slash relative path from ``root``. @@ -2135,15 +2169,19 @@ def _to_relative_for_storage(key: str, root: Path) -> str: if not p.is_absolute(): return key try: - base = _nfc(str(Path(root).resolve())) - rel = os.path.relpath(_nfc(str(p)), base) + root_res = Path(root).resolve() + rel = os.path.relpath(_nfc(str(p)), _nfc(str(root_res))) except (ValueError, OSError): return key # outside root (e.g. Windows cross-drive) # ``os.path.relpath`` happily produces ``../foo`` for paths outside # root; mirror the prior ``relative_to``-raises-ValueError semantics by # keeping out-of-root entries in their absolute form. if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): - return key + # Lexically out of root. Before accepting that, ask the filesystem: + # the key may denote a path under root that merely spells it + # differently (see :func:`_anchor_under_root`). + anchored = _anchor_under_root(p, root_res) + return anchored if anchored is not None else key return rel.replace(os.sep, "/") diff --git a/tests/test_detect.py b/tests/test_detect.py index 22099029a5..b5f1e417c3 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -2537,6 +2537,105 @@ def test_save_manifest_out_of_root_keeps_absolute(tmp_path): outside.unlink(missing_ok=True) +# --- #3581: a stored key that spells root differently is still in root ------- +# ``os.path.relpath`` compares text. A filesystem compares identity. When a +# stored absolute key denotes a path under ``root`` that merely spells it +# differently (a case-variant on Windows/APFS, a symlinked checkout), the +# lexical answer is ``../...`` and the key would stay absolute for ever, so an +# upgrade that changes the key convention leaves the stale key sitting beside +# the new relative one and the manifest describes the same tree twice. + +def test_save_manifest_relativizes_key_reached_through_symlinked_root( + requires_symlinks, tmp_path +): + """A legacy key naming the corpus through a symlink to ``root`` relativizes + to the same key the fresh scan produces, so the row is replaced rather than + duplicated. Symlinks make this reproducible on every filesystem; case + variants (the reported shape) only do so on a case-insensitive one.""" + import json + from graphify.detect import save_manifest + + root = tmp_path / "repo" + (root / "pkg").mkdir(parents=True) + mod = root / "pkg" / "mod.py" + mod.write_text("def x(): pass\n") + link = tmp_path / "checkout" + link.symlink_to(root, target_is_directory=True) + + manifest_path = tmp_path / "graphify-out" / "manifest.json" + manifest_path.parent.mkdir(parents=True) + manifest_path.write_text(json.dumps({ + str(link / "pkg" / "mod.py"): {"mtime": 0.0, "ast_hash": "stale", "semantic_hash": ""}, + }), encoding="utf-8") + + save_manifest({"code": [str(mod)]}, str(manifest_path), kind="ast", root=root) + + raw = json.loads(manifest_path.read_text(encoding="utf-8")) + assert set(raw) == {"pkg/mod.py"}, ( + f"the symlinked key names a file under root and must collapse onto the " + f"relative key, got {sorted(raw)}" + ) + + +def test_save_manifest_legacy_case_variant_key_does_not_duplicate(tmp_path): + """The reported shape: a manifest carried across an upgrade holds absolute + keys whose root differs only in case. On the filesystems the reporter runs + (Windows, default macOS) those name the very same files, so they must + collapse onto the new relative keys. On a case-sensitive filesystem they + are genuinely different files and must both survive, absolute. Assert the + one that actually applies here rather than skipping half the platforms.""" + import json + from graphify.detect import save_manifest + + root = tmp_path / "Repo" + (root / "pkg").mkdir(parents=True) + mod = root / "pkg" / "mod.py" + mod.write_text("def x(): pass\n") + variant_key = str(tmp_path / "repo" / "pkg" / "mod.py") + case_insensitive = (tmp_path / "repo").exists() + + manifest_path = tmp_path / "graphify-out" / "manifest.json" + manifest_path.parent.mkdir(parents=True) + manifest_path.write_text(json.dumps({ + variant_key: {"mtime": 0.0, "ast_hash": "stale", "semantic_hash": ""}, + }), encoding="utf-8") + + save_manifest({"code": [str(mod)]}, str(manifest_path), kind="ast", root=root) + + raw = json.loads(manifest_path.read_text(encoding="utf-8")) + if case_insensitive: + assert set(raw) == {"pkg/mod.py"}, ( + f"the case variant names the same file and must not survive as a " + f"second, absolute key, got {sorted(raw)}" + ) + else: + assert set(raw) == {"pkg/mod.py", variant_key}, ( + f"on a case-sensitive filesystem the variant is a different file " + f"and must keep its absolute key, got {sorted(raw)}" + ) + + +def test_anchor_under_root_returns_none_when_no_ancestor_is_root(tmp_path): + """The fallback must only fire on a path it can prove lies under ``root``. + + Both out-of-root shapes are unit-tested here because ``save_manifest`` + cannot reach the second one: it prunes rows whose file no longer exists + before relativizing, so a key naming a vanished tree never gets this far. + """ + from graphify.detect import _anchor_under_root + + root = tmp_path / "repo" + (root / "pkg").mkdir(parents=True) + sibling = tmp_path / "other" / "pkg" / "mod.py" + sibling.parent.mkdir(parents=True) + sibling.write_text("pass\n") + + assert _anchor_under_root(sibling, root.resolve()) is None + assert _anchor_under_root(tmp_path / "gone" / "pkg" / "mod.py", root.resolve()) is None + # ...and a path that genuinely is under root still anchors. + assert _anchor_under_root(root / "pkg" / "mod.py", root.resolve()) == "pkg/mod.py" + + def test_detect_incremental_portable_across_paths(tmp_path): """End-to-end: a manifest written at one root must be readable from a different absolute prefix (the cross-machine case #777 is about). From e84dfabde169ac8a292cd2623c0775f88f2dc4e7 Mon Sep 17 00:00:00 2001 From: L4XB Date: Tue, 15 Sep 2026 23:59:24 +0200 Subject: [PATCH 2/2] test(detect): give the case-variant cell a file to anchor against on Linux The case-sensitive branch asserted the variant key stays absolute, but on a case-sensitive filesystem that path does not exist at all, so save_manifest's existing "drop rows whose file is gone" prune removed the row before anything relativized it. The branch was measuring that prune rather than the anchoring decision, and failed on every CI job. Create the variant file when the probe says case-sensitive, so what the branch measures is that `os.path.samefile` refuses to anchor a genuinely different file. Verified against the same shape locally with two distinct directory names: the fresh key relativizes, the other keeps its absolute key. --- tests/test_detect.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_detect.py b/tests/test_detect.py index b5f1e417c3..89e31d3bfd 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -2591,8 +2591,16 @@ def test_save_manifest_legacy_case_variant_key_does_not_duplicate(tmp_path): (root / "pkg").mkdir(parents=True) mod = root / "pkg" / "mod.py" mod.write_text("def x(): pass\n") - variant_key = str(tmp_path / "repo" / "pkg" / "mod.py") - case_insensitive = (tmp_path / "repo").exists() + variant_root = tmp_path / "repo" + variant_key = str(variant_root / "pkg" / "mod.py") + case_insensitive = variant_root.exists() + if not case_insensitive: + # On a case-sensitive filesystem the variant names a file that is simply + # absent, and save_manifest drops rows whose file is gone before it + # relativizes anything. Give it a real file, so what the assertion below + # measures is the anchoring decision rather than that prune. + (variant_root / "pkg").mkdir(parents=True) + (variant_root / "pkg" / "mod.py").write_text("def y(): pass\n") manifest_path = tmp_path / "graphify-out" / "manifest.json" manifest_path.parent.mkdir(parents=True)