diff --git a/graphify/build.py b/graphify/build.py index f72067b1..3f7aa8fc 100644 --- a/graphify/build.py +++ b/graphify/build.py @@ -31,6 +31,7 @@ from pathlib import Path import networkx as nx from .ids import make_id, normalize_id as _normalize_id from .paths import default_graph_json as _default_graph_json +from .paths import is_absolute_any_platform as _is_abs from .validate import validate_extraction @@ -282,7 +283,7 @@ def _norm_source_file(p: str | None, root: str | None = None) -> str | None: if not p: return p p = p.replace("\\", "/") - if root and os.path.isabs(p): + if root and _is_abs(p): try: p = Path(p).relative_to(root).as_posix() except ValueError: @@ -314,7 +315,7 @@ def _abs_identity(p: str | None, root: str | None = None) -> str | None: return None q = p.replace("\\", "/") pp = Path(q) - if not pp.is_absolute() and root: + if not _is_abs(q) and root: pp = Path(root) / q try: return pp.resolve().as_posix() @@ -486,7 +487,7 @@ def _derive_prune_root(prune_sources: "list[str]", stored_sfs: "set[str]") -> "s rel_sfs = [ sf.replace("\\", "/") for sf in stored_sfs - if sf and isinstance(sf, str) and not os.path.isabs(sf.replace("\\", "/")) + if sf and isinstance(sf, str) and not _is_abs(sf.replace("\\", "/")) ] if not rel_sfs: return None @@ -495,7 +496,7 @@ def _derive_prune_root(prune_sources: "list[str]", stored_sfs: "set[str]") -> "s if not p or not isinstance(p, str): continue q = p.replace("\\", "/") - if not os.path.isabs(q): + if not _is_abs(q): continue hits = {q[: -len(s) - 1] for s in rel_sfs if q.endswith("/" + s)} if len(hits) > 1: @@ -608,8 +609,13 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict: continue sf_norm = _norm_source_file(str(sf), root) or str(sf) rel = Path(sf_norm) - if rel.is_absolute(): - continue # can't relativize (no/failed root) — leave id untouched + if _is_abs(sf_norm): + # Can't relativize (no/failed root) — leave the id untouched rather + # than bake an on-disk path into it. Tested for BOTH platforms: a + # graph built on Linux/CI carries POSIX-absolute source_files that + # WindowsPath.is_absolute() calls relative, which leaked the whole + # build directory into node IDs when updated on Windows (#2618). + continue if not rel.name: # source_file equals the scan root, so _norm_source_file relativized it # to Path('.') — a project-level node with no per-file identity to remap. @@ -642,7 +648,7 @@ def _semantic_id_remap(nodes: list, root: str | None) -> dict: # id registration. It is the longest form, so it goes first (greedy # prefix stripping, same ordering rule as _old_file_stems). sf_raw = str(sf).replace("\\", "/") - if sf_raw != sf_norm and os.path.isabs(sf_raw): + if sf_raw != sf_norm and _is_abs(sf_raw): abs_stem = make_id(_file_stem(Path(sf_raw))) if abs_stem and abs_stem != new_stem and abs_stem not in old_forms: old_forms.insert(0, abs_stem) @@ -685,8 +691,9 @@ def graph_has_legacy_ids(nodes: list, root: str | Path | None = None, sample: in sf = node.get("source_file") if not nid or not isinstance(nid, str) or not sf: continue - rel = Path(_norm_source_file(str(sf), _r) or str(sf)) - if rel.is_absolute(): + sf_norm = _norm_source_file(str(sf), _r) or str(sf) + rel = Path(sf_norm) + if _is_abs(sf_norm): continue if not rel.name: continue # source_file == scan root -> Path('.'), no file stem (#1618) @@ -1047,7 +1054,7 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat if not sf: continue rel = Path(str(sf)) - if rel.is_absolute(): + if _is_abs(str(sf)): continue new_stem = make_id(_fs(rel)) if str(attrs.get("label", "")) == rel.name: diff --git a/graphify/paths.py b/graphify/paths.py index ef0cedb5..a48688e2 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -21,7 +21,7 @@ import os import re import stat import tempfile -from pathlib import Path, PurePosixPath +from pathlib import Path, PurePosixPath, PureWindowsPath GRAPHIFY_OUT = os.environ.get("GRAPHIFY_OUT", "graphify-out") @@ -303,6 +303,41 @@ def default_graph_json() -> str: return str(out_path("graph.json")) +def is_absolute_any_platform(p: "str | Path | None") -> bool: + """Whether *p* is absolute under POSIX **or** Windows rules. + + ``Path.is_absolute()`` and ``os.path.isabs()`` answer for the HOST os only, + which is the wrong question for a path that was *stored* — a ``source_file`` + in ``graph.json``, a ``prune_sources`` entry, a cache key. Those travel + between machines (build in Docker/CI, update on a Windows workstation, or + the reverse), so the host's rules do not describe the string in hand: + + - On Windows, ``WindowsPath("/home/ci/repo/docs/a.md").is_absolute()`` is + False — no drive letter — so a Linux-built graph's absolute paths read as + relative and get baked into node IDs or joined under the scan root (#2618). + - On POSIX, ``PosixPath("C:/Users/u/a.md").is_absolute()`` is False for the + mirror-image reason (#2197, #1789). + + ``os.path.isabs`` is additionally not stable across supported interpreters: + Python 3.13 changed ``ntpath.isabs`` so a path starting with a single slash + is no longer absolute, where 3.10–3.12 said it was. The project supports + >=3.10, so a guard written on it silently means different things per version. + + Answering for both platforms is the conservative choice for stored paths: + treating a path as absolute at worst declines to relativize it (the string is + kept as-is), whereas treating an absolute path as relative corrupts identity. + Covers drive-letter, UNC, and POSIX-root forms with either separator. + + NOTE: this is for STORED/portable paths. Code resolving a path against the + real local filesystem (``cli``, ``detect``, ``hooks``) must keep using + ``Path.is_absolute()`` — there the host's rules are exactly right. + """ + if not p: + return False + s = str(p) + return PurePosixPath(s).is_absolute() or PureWindowsPath(s).is_absolute() + + def nfc(s: str) -> str: """NFC-normalize a path string. diff --git a/tests/test_build.py b/tests/test_build.py index e51da3ff..416e17bc 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1,6 +1,7 @@ import json from pathlib import Path import networkx as nx +import pytest from networkx.readwrite import json_graph from graphify.build import build_from_json, build, build_merge, edge_data, edge_datas, dedupe_edges, dedupe_nodes @@ -1427,3 +1428,65 @@ def test_build_from_json_prunes_dangling_hyperedge_members(capsys): assert set(hes) == {"he_partial"}, "an all-dangling hyperedge must be dropped" assert hes["he_partial"]["nodes"] == ["alpha", "beta"] assert "he_all_ghost" in capsys.readouterr().err + + +# --- foreign-absolute source_file must not leak into IDs -------------------- +# A graph.json is portable: built in Docker/Linux CI, updated on a Windows +# workstation, or the reverse. Every guard that asks "is this stored path +# absolute?" therefore has to answer for BOTH platforms. Host-only tests +# (Path.is_absolute / os.path.isabs) silently call the other platform's +# absolute paths relative, which bakes a build directory into node IDs (the +# mirror of #2197, reported as #2618) or joins it under the scan root. +# +# os.path.isabs is doubly unsafe here: Python 3.13 changed ntpath.isabs so a +# single leading slash is no longer absolute, where 3.10-3.12 said it was, so +# the same guard meant different things across supported interpreters. + +FOREIGN_ABSOLUTE_SOURCE_FILES = [ + "/home/ci/build/repo/docs/api/README.md", # POSIX-absolute (Linux/Docker build) + "C:/Users/u/repo/docs/api/README.md", # Windows-absolute, forward slashes +] + + +@pytest.mark.parametrize("sf", FOREIGN_ABSOLUTE_SOURCE_FILES) +def test_semantic_rekey_skips_absolute_from_either_platform(sf): + """#2618: an absolute source_file is left alone whichever OS wrote it.""" + from graphify.build import _semantic_id_remap + nodes = [{"id": "api_readme", "source_file": sf, "type": "document"}] + assert _semantic_id_remap(nodes, None) == {}, ( + f"{sf!r} leaked its on-disk path into the node ID" + ) + + +@pytest.mark.parametrize("sf", FOREIGN_ABSOLUTE_SOURCE_FILES) +def test_graph_has_legacy_ids_skips_absolute_from_either_platform(sf): + """The legacy-ID probe derives a stem from source_file, so it must skip an + absolute path rather than mint a stem out of the whole build directory.""" + from graphify.build import graph_has_legacy_ids + nodes = [{"id": "api_readme", "source_file": sf, + "type": "document", "source_location": "L1"}] + assert graph_has_legacy_ids(nodes, root=None) is False + + +def test_norm_source_file_relativizes_a_posix_absolute_path(): + """A Linux-built graph's absolute source_file must relativize against the + matching root regardless of the host running the update.""" + from graphify.build import _norm_source_file + assert _norm_source_file( + "/home/ci/build/repo/docs/api/README.md", "/home/ci/build/repo" + ) == "docs/api/README.md" + + +def test_derive_prune_root_recovers_root_from_posix_absolute_prune_sources(): + """The prune-root recovery skips any prune source it thinks is relative. + + With a host-only absoluteness test, every POSIX-absolute prune source was + skipped on Windows, the root came back None, and prune/replace silently + no-opped — the failure mode of #1151 / #2446 / #2012, reached from the other + direction. + """ + from graphify.build import _derive_prune_root + stored = {"docs/a.md", "/home/ci/build/repo/docs/b.md"} + assert _derive_prune_root( + ["/home/ci/build/repo/docs/a.md"], stored + ) == "/home/ci/build/repo" diff --git a/tests/test_paths.py b/tests/test_paths.py index e0e1a2f0..12359006 100644 --- a/tests/test_paths.py +++ b/tests/test_paths.py @@ -97,3 +97,50 @@ def test_disambiguate_path_proximity_same_dir() -> None: "pkg/a/caller.py", ) assert winner == "near" + + +# --- cross-platform absoluteness for STORED paths --------------------------- +# Path.is_absolute()/os.path.isabs() answer for the host OS, which is the wrong +# question for a path read out of graph.json: graphs are built in Docker/CI and +# updated on Windows (and vice versa), so the string in hand may follow the +# other platform's rules. Getting this wrong bakes an on-disk path into node IDs +# (#2197, #1789) or joins an absolute path under the scan root. + +@pytest.mark.parametrize("path", [ + "/home/ci/build/repo/docs/api/README.md", # POSIX absolute + "/", # POSIX root + "C:/Users/u/repo/docs/a.md", # Windows drive, forward slashes + r"C:\Users\u\repo\docs\a.md", # Windows drive, backslashes + r"\\server\share\docs\a.md", # UNC + "//server/share/docs/a.md", # UNC, forward slashes +]) +def test_is_absolute_any_platform_accepts_both_conventions(path): + from graphify.paths import is_absolute_any_platform + assert is_absolute_any_platform(path) is True, ( + f"{path!r} is absolute on some platform and must be treated as such" + ) + + +@pytest.mark.parametrize("path", [ + "docs/api/README.md", + r"docs\api\README.md", + "README.md", + ".", + "", + None, + r"\foo", # Windows root-relative (no drive) — not absolute anywhere +]) +def test_is_absolute_any_platform_rejects_relative(path): + from graphify.paths import is_absolute_any_platform + assert is_absolute_any_platform(path) is False + + +def test_is_absolute_any_platform_is_host_independent(): + """The whole point: the answer must not depend on which OS is running. + + Both spellings are absolute somewhere, so both must return True on every + host — that is exactly what Path.is_absolute() fails to do. + """ + from graphify.paths import is_absolute_any_platform + assert is_absolute_any_platform("/home/ci/x.md") + assert is_absolute_any_platform("C:/Users/u/x.md")