fix(resolution): repoint cross-extension re-exports without leaking an absolute path (#1814)

This commit is contained in:
Alpha Nury
2026-07-18 12:04:23 +01:00
committed by safishamsi
parent a86666ae94
commit d56b70a451
6 changed files with 376 additions and 12 deletions
+1
View File
@@ -6,6 +6,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu
- Feat: opt-in strict PreToolUse hook that actually makes agents use the graph. The installed Claude Code hook has always *nudged* the agent to run `graphify query` before reading raw files, but a nudge is advisory `additionalContext` the model routinely walks past mid-task. `graphify install --project --strict` (or `graphify claude install --strict`) now installs a hook that *blocks* the first raw source read of a session (`permissionDecision: "deny"`) with a redirect to `graphify query`, then downgrades to the soft nudge — so it fires at most once per session and can never strand the agent (the next read proceeds even if no query ran, or if `graphify query` itself failed). Running any `graphify query`/`explain`/`path` refreshes a short-lived "recently oriented" stamp that suppresses the block. Strict mode is Claude Code only (Bash-grep and Glob stay nudge-only; Gemini/Codex/OpenCode can't hard-block and are unchanged); `GRAPHIFY_HOOK_STRICT=1`/`0` toggles it at runtime without a reinstall. Default installs are unchanged (soft nudge).
- Fix: the PreToolUse hook stops crying wolf (#1840), which applies to the default soft nudge too. It no longer fires for reads of files **outside** the indexed project (a common false trigger, e.g. a `~/.claude/.../SKILL.md` read), and when the graph is **stale for the target file** (the file changed after the last build, or `graphify watch` flagged the tree) it softens to a non-mandatory nudge that suggests `graphify update` instead of demanding the query. Gating is ~3 `stat` calls — no corpus walk — so it stays fast on large monorepos, and fails open on any error.
- Fix: a same-basename cross-extension re-export no longer manufactures a phantom self-cycle (#1814, thanks @Greg-Moskalenko). A typed `.ts` wrapper that re-exports a hand-written `.mjs` runtime (`export { N } from "./foo.mjs"`) had `foo.ts` and `foo.mjs` collapse onto one base file id (the id stem drops the extension), and while `_disambiguate_colliding_node_ids` correctly salts the two file *nodes* apart (`foo_ts_foo` / `foo_mjs_foo`), the re-export *edge* keyed its target salt by the importer's own source file — mis-pointing the `./foo.mjs` target back at `foo.ts`, a `source == target` self-loop reported as a 1-file import cycle in `GRAPH_REPORT.md`. During disambiguation an import/re-export edge now carries the resolved target file as a *transient* salt key, so the salt lands on the real sibling node (generalizing the C/ObjC `.h`-sibling carve-out from #1475 to every language and to `re_exports`) and the phantom cycle disappears. That hint has no downstream reader and holds an absolute path, so it is popped once consumed and never persisted — and the graph serializer drops it as a backstop — keeping graph.json deterministic and byte-identical across checkout locations. Node ids are unchanged (the residual was purely at the edge layer). One caveat: a graph written by a *pre-fix* build still records the stale self-loop, and because `graphify update` only re-extracts changed files, an unchanged wrapper keeps that edge until it is next edited or a `--force` full rebuild runs — though any stale absolute hint a pre-fix graph happened to persist is dropped on the next build regardless. (The extension-aware-id alternative was rejected: it would rewrite every file and symbol id and force a full-rebuild migration in lockstep with the skill/validation id spec, #1033.)
## 0.9.18 (2026-07-17)
+7 -1
View File
@@ -706,7 +706,12 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
tgt = norm_to_id.get(_normalize_id(tgt), tgt)
if src not in node_set or tgt not in node_set:
continue # skip edges to external/stdlib nodes - expected, not an error
attrs = {k: v for k, v in edge.items() if k not in ("source", "target")}
# `target_file` is a transient import-disambiguation salt hint (#1814)
# with no downstream reader; it holds an absolute path, so it must never
# be persisted. Disambiguation already pops it off fresh extractions —
# dropping it here as well keeps a pre-fix graph's stale absolute hint
# from surviving an incremental build_merge, which re-serializes base
# edges through here without re-running disambiguation.
# Sanitize numeric edge fields (#1960): an explicit ``"weight": null`` in
# the extraction JSON survives ``.get("weight", 1.0)`` (the key is present,
# so the default never applies) and reaches Louvain/Leiden as None,
@@ -716,6 +721,7 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
# strings, NaN/inf, negatives — while numeric strings coerce cleanly.
# Repair (not drop) the key so graph.json round-trips a clean value and a
# cluster-only/--update reload never re-ingests the null.
attrs = {k: v for k, v in edge.items() if k not in ("source", "target", "target_file")}
for _num_key in ("weight", "confidence_score"):
if _num_key in attrs:
try:
+10 -2
View File
@@ -309,7 +309,7 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p
resolved = _resolve_js_import_target(raw, str_path)
if resolved is not None:
tgt_nid, resolved_path = resolved
edges.append({
edge = {
"source": file_nid,
"target": tgt_nid,
"relation": "imports_from",
@@ -318,7 +318,15 @@ def _import_js(node, source: bytes, file_nid: str, stem: str, edges: list, str_p
"source_file": str_path,
"source_location": f"L{node.start_point[0] + 1}",
"weight": 1.0,
})
}
# Stamp the resolved target file so a same-basename cross-extension
# sibling (foo.ts importing/re-exporting ./foo.mjs) keys its target salt
# by the TARGET's file rather than the importer's. Both files collapse to
# the base id `foo`; without this the salted lookup mis-points the target
# back onto the importer's own variant, a phantom self-loop (#1814).
if resolved_path is not None:
edge["target_file"] = str(resolved_path)
edges.append(edge)
# Emit symbol-level edges for named imports/re-exports from local/aliased files.
# e.g. `import { Foo, type Bar } from './bar'` → file → Foo, file → Bar (EXTRACTED)
+15 -5
View File
@@ -1258,11 +1258,11 @@ def _dynamic_import_js(node, source: bytes, caller_nid: str, str_path: str, edge
resolved = _resolve_js_import_target(raw, str_path)
if resolved is None:
break
tgt_nid, _ = resolved
tgt_nid, resolved_path = resolved
pair = (caller_nid, tgt_nid)
if pair not in seen_dyn_pairs:
seen_dyn_pairs.add(pair)
edges.append({
edge = {
"source": caller_nid,
"target": tgt_nid,
# A deferred `import(...)` is a real dependency, so keep it as an
@@ -1276,7 +1276,12 @@ def _dynamic_import_js(node, source: bytes, caller_nid: str, str_path: str, edge
"source_file": str_path,
"source_location": f"L{node.start_point[0] + 1}",
"weight": 1.0,
})
}
# Key the target salt by the resolved target file so a same-basename
# cross-extension sibling isn't mis-salted onto the importer (#1814).
if resolved_path is not None:
edge["target_file"] = str(resolved_path)
edges.append(edge)
break
return True
@@ -1570,7 +1575,7 @@ def _require_imports_js(node, source: bytes, file_nid: str, stem: str, edges: li
continue
tgt_nid, resolved_path = resolved
line = node.start_point[0] + 1
edges.append({
edge = {
"source": file_nid,
"target": tgt_nid,
"relation": "imports_from",
@@ -1579,7 +1584,12 @@ def _require_imports_js(node, source: bytes, file_nid: str, stem: str, edges: li
"source_file": str_path,
"source_location": f"L{line}",
"weight": 1.0,
})
}
# Key the target salt by the resolved target file so a same-basename
# cross-extension sibling isn't mis-salted onto the importer (#1814).
if resolved_path is not None:
edge["target_file"] = str(resolved_path)
edges.append(edge)
found = True
# Symbol-level edges for destructured / accessor binders.
+32 -4
View File
@@ -637,6 +637,12 @@ def _disambiguate_colliding_node_ids(
node["id"] = new_id
if not remap:
# No colliding ids to salt apart, but the transient `target_file` hint an
# importer stamps on every resolved import (#1814) still has to be dropped
# here — this early exit skips the edge loop below, so without it a
# non-colliding import would carry its absolute path into graph.json.
for edge in edges:
edge.pop("target_file", None)
return
unambiguous_remaps: dict[str, str] = {}
@@ -671,7 +677,20 @@ def _disambiguate_colliding_node_ids(
for edge in edges:
edge_source_key = _source_key(str(edge.get("source_file", "")), root)
source_key = (edge.get("source", ""), edge_source_key)
target_key = (edge.get("target", ""), edge_source_key)
# An import/re-export edge's target is a FILE node that can collapse with a
# same-basename cross-extension sibling (foo.ts vs foo.mjs, #1814). Keying
# its target salt by the IMPORTER's own source_file mis-points it back at the
# importer's variant (a self-loop). When the emitter stamped the resolved
# target file, key the target salt by THAT file so the salt lands on the
# correct sibling. Generalizes the #1475 C/ObjC header carve-out (below) to
# every language and to re_exports. `pop` it as we consume it: this is the
# hint's only reader, and its absolute path must not persist into graph.json.
target_file = edge.pop("target_file", None)
if target_file and edge.get("relation") in ("imports", "imports_from", "re_exports"):
target_edge_key = _source_key(str(target_file), root)
else:
target_edge_key = edge_source_key
target_key = (edge.get("target", ""), target_edge_key)
if source_key in remap:
edge["source"] = remap[source_key]
elif edge.get("source") in unambiguous_remaps:
@@ -777,12 +796,12 @@ def _apply_symbol_resolution_facts(
for edge in edges
}
def add_edge(source: str, target: str, relation: str, context: str, line: int, source_path: Path) -> None:
def add_edge(source: str, target: str, relation: str, context: str, line: int, source_path: Path, target_file: str | None = None) -> None:
key = (source, target, relation, context or "")
if key in existing_edges:
return
existing_edges.add(key)
edges.append({
edge = {
"source": source,
"target": target,
"relation": relation,
@@ -791,7 +810,13 @@ def _apply_symbol_resolution_facts(
"source_file": str(source_path),
"source_location": f"L{line}",
"weight": 1.0,
})
}
# A re-export edge's target is a FILE node that can collapse with a
# same-basename cross-extension sibling; stamp the resolved target file so
# the id-disambiguation salt is keyed by the TARGET, not the importer (#1814).
if target_file is not None:
edge["target_file"] = target_file
edges.append(edge)
for declaration in facts.declarations:
ensure_symbol_node(declaration.file_path, declaration.name, declaration.line)
@@ -837,6 +862,7 @@ def _apply_symbol_resolution_facts(
"export",
star_fact.line,
star_fact.file_path,
target_file=str(path_by_resolved.get(target_path, target_path)),
)
for namespace_fact in facts.namespace_exports:
@@ -867,6 +893,7 @@ def _apply_symbol_resolution_facts(
"export",
namespace_fact.line,
namespace_fact.file_path,
target_file=str(path_by_resolved.get(target_path, target_path)),
)
for export_fact in facts.exports:
@@ -891,6 +918,7 @@ def _apply_symbol_resolution_facts(
"export",
export_fact.line,
export_fact.file_path,
target_file=str(path_by_resolved.get(origin[0], origin[0])),
)
def resolve_exported_origin(target_path: Path, imported_name: str, seen: set[tuple[Path, str]] | None = None) -> tuple[Path, str]:
@@ -0,0 +1,311 @@
"""Same-basename cross-extension re-exports must not collapse to a self-cycle (#1814).
A hand-written ``.mjs`` plain-ESM runtime plus a thin typed ``.ts`` wrapper that
re-exports it (``export { N } from "./foo.mjs"``) is a common convention. Because
``_file_stem`` drops the extension, ``foo.ts`` and ``foo.mjs`` both collapse to the
base file id ``foo`` at extract time. ``_disambiguate_colliding_node_ids`` salts the
two NODES apart correctly (``foo_ts_foo`` / ``foo_mjs_foo``), but the file-level
re-export edge (and the ``re_exports`` export edge) keyed its target salt by the
IMPORTER's own source_file, mis-pointing the ``./foo.mjs`` target back onto the
importer's own variant — a phantom ``foo.ts -> foo.ts`` self-loop reported as a
1-file import cycle.
These lock: the salted node ids stay unchanged (the fix does NOT make ids
extension-aware — Option A rejected, #1033), the re-export edge lands on the
sibling node, and no phantom 1-file cycle survives.
"""
from __future__ import annotations
import json
from pathlib import Path
from graphify.build import build
from graphify.export import to_json
from graphify.extract import extract
def _write(path: Path, text: str) -> Path:
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(text, encoding="utf-8")
return path
def _node_id_by_label(result: dict, label: str) -> str:
ids = [n["id"] for n in result["nodes"] if n.get("label") == label]
assert len(ids) == 1, f"expected exactly one node labelled {label!r}; got {ids}"
return ids[0]
def _reexport_like_edges(result: dict) -> list[dict]:
return [
e for e in result["edges"]
if e.get("relation") in ("imports_from", "re_exports")
]
def test_cross_ext_reexport_emits_no_self_loop(tmp_path: Path):
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=tmp_path)
self_loops = [
e for e in _reexport_like_edges(result)
if e.get("source") == e.get("target")
]
assert not self_loops, (
f"cross-extension re-export produced a phantom self-loop; got {self_loops}"
)
def test_cross_ext_reexport_target_is_the_sibling_node(tmp_path: Path):
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=tmp_path)
foo_ts = _node_id_by_label(result, "foo.ts")
foo_mjs = _node_id_by_label(result, "foo.mjs")
# The scheme stays as-is: extension dropped from the base stem, siblings salted
# apart by source path. The fix must NOT make node ids extension-aware (#1814
# Option A rejected).
assert foo_ts == "foo_ts_foo"
assert foo_mjs == "foo_mjs_foo"
file_level = [
e for e in result["edges"]
if e.get("relation") == "imports_from" and e.get("source") == foo_ts
]
assert file_level, "no file-level re-export edge from foo.ts was emitted"
assert all(e.get("target") == foo_mjs for e in file_level), (
f"file-level re-export must target the .mjs sibling node {foo_mjs!r}; "
f"got {[e.get('target') for e in file_level]}"
)
# The symbol-provenance re_exports (context='export') edge must also point at
# the sibling, never back at the importer.
export_edges = [
e for e in result["edges"]
if e.get("relation") == "re_exports" and e.get("context") == "export"
and e.get("source") == foo_ts
]
assert export_edges, "no re_exports export edge from foo.ts was emitted"
assert all(e.get("target") == foo_mjs for e in export_edges), (
f"re_exports export edge must target the .mjs sibling node {foo_mjs!r}; "
f"got {[e.get('target') for e in export_edges]}"
)
def test_cross_ext_reexport_no_phantom_import_cycle(tmp_path: Path):
import networkx as nx
from graphify.analyze import find_import_cycles
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=tmp_path)
graph = nx.DiGraph()
for node in result["nodes"]:
graph.add_node(node["id"], **{k: v for k, v in node.items() if k != "id"})
for edge in result["edges"]:
graph.add_edge(
edge["source"],
edge["target"],
**{k: v for k, v in edge.items() if k not in ("source", "target")},
)
assert find_import_cycles(graph) == [], (
"cross-extension re-export must not manufacture a file-level import cycle"
)
def test_same_basename_three_colliding_siblings_reexport_selects_named_variant(
tmp_path: Path,
):
"""With three same-basename siblings that all collapse to the base id ``foo``
(``foo.mjs`` / ``foo.cjs`` / ``foo.ts``), keying the re-export target by the
RESOLVED target file — not the importer's file — must land on the specifically
named ``./foo.mjs`` variant, proving the fix is a real per-file selection and
not a binary coin-flip between two colliders.
(The issue's illustrative trio uses ``foo.d.mts``, but a ``.d.mts`` stem keeps
its ``.d`` segment and so does NOT collide with ``foo`` — it cannot exercise
multi-variant salt selection. ``foo.cjs`` is the realistic dual-format sibling
that genuinely collides.)
"""
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
cjs = _write(tmp_path / "foo.cjs", "module.exports.M = 2;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, cjs, ts], cache_root=tmp_path)
foo_ts = _node_id_by_label(result, "foo.ts")
foo_mjs = _node_id_by_label(result, "foo.mjs")
foo_cjs = _node_id_by_label(result, "foo.cjs")
assert foo_mjs != foo_cjs != foo_ts
file_level = [
e for e in result["edges"]
if e.get("relation") == "imports_from" and e.get("source") == foo_ts
]
assert file_level, "no file-level re-export edge from foo.ts was emitted"
assert all(e.get("target") == foo_mjs for e in file_level), (
f"re-export of './foo.mjs' must resolve to the .mjs node {foo_mjs!r}, not "
f"the .cjs sibling {foo_cjs!r}; got {[e.get('target') for e in file_level]}"
)
self_loops = [
e for e in _reexport_like_edges(result)
if e.get("source") == e.get("target")
]
assert not self_loops, f"unexpected self-loop among siblings; got {self_loops}"
# --------------------------------------------------------------------------- #
# The ``target_file`` the fix stamps on import/re-export edges is a transient
# extraction-time disambiguation salt hint (its only reader is the salt lookup
# in ``_disambiguate_colliding_node_ids``). It carries an ABSOLUTE filesystem
# path, so it must never survive its consumer onto a persisted edge: leaking it
# into graph.json breaks determinism across checkout locations and the
# cross-machine merge/global-graph portability the codebase engineered for. The
# following lock that the hint is stripped after disambiguation and never
# reaches graph.json — on the raw-dump extract path AND the build path — and
# that a persisted absolute hint from a pre-fix graph is dropped on the next
# build rather than carried forward.
# --------------------------------------------------------------------------- #
def test_disambiguation_strips_transient_target_file_hint(tmp_path: Path):
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=tmp_path)
leaked = [e for e in result["edges"] if "target_file" in e]
assert not leaked, (
f"the target_file salt hint must not survive disambiguation onto an "
f"edge (it carries an absolute path with no downstream reader); got {leaked}"
)
def test_target_file_hint_stripped_even_without_a_collision(tmp_path: Path):
# No same-basename collision here, so `_disambiguate_colliding_node_ids`
# takes its early `if not remap: return` exit before the edge loop. An
# ordinary import still stamps target_file at extraction, so that early
# exit must strip it too — otherwise every non-colliding import leaks an
# absolute path.
util = _write(tmp_path / "util.ts", "export const helper = 1;\n")
main = _write(tmp_path / "main.ts", 'import { helper } from "./util";\n')
result = extract([util, main], cache_root=tmp_path)
leaked = [e for e in result["edges"] if "target_file" in e]
assert not leaked, (
f"a non-colliding import leaked the transient target_file hint; got {leaked}"
)
def test_graph_json_has_no_target_file_and_no_absolute_path(tmp_path: Path):
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=tmp_path)
graph = build([result], root=tmp_path)
out = tmp_path / "graph.json"
to_json(graph, {}, str(out), force=True)
raw = out.read_text(encoding="utf-8")
data = json.loads(raw)
leaked = [link for link in data["links"] if "target_file" in link]
assert not leaked, f"absolute target_file persisted into graph.json links: {leaked}"
assert str(tmp_path.resolve()) not in raw, (
"graph.json leaked an absolute checkout path (source_file is relativized, "
"but the target_file hint was serialized verbatim)"
)
def test_graph_json_is_checkout_location_independent(tmp_path: Path):
"""Building the byte-identical repo at two different absolute locations must
yield identical graph.json edges. A leaked absolute target_file differs by
its checkout prefix and would defeat the cross-machine merge/global-graph
portability the codebase is built around."""
def _links_built_at(dirname: str) -> list[dict]:
d = tmp_path / dirname
d.mkdir()
mjs = _write(d / "foo.mjs", "export const N = 1;\n")
ts = _write(d / "foo.ts", 'export { N } from "./foo.mjs";\n')
result = extract([mjs, ts], cache_root=d)
graph = build([result], root=d)
out = d / "graph.json"
to_json(graph, {}, str(out), force=True)
links = json.loads(out.read_text(encoding="utf-8"))["links"]
return sorted(
links,
key=lambda link: (
str(link.get("source")),
str(link.get("target")),
str(link.get("relation")),
),
)
assert _links_built_at("loc_a") == _links_built_at("loc_bbbb_longer"), (
"graph.json edges differ across checkout locations — an absolute path leaked"
)
def test_build_drops_persisted_target_file_from_a_pre_fix_graph(tmp_path: Path):
# A graph.json written by a pre-fix build carries an absolute target_file on
# its import edges. On the next (incremental) build those base edges are
# re-serialized through build(), which does NOT re-run disambiguation — so
# the serializer itself must drop the persisted absolute path rather than
# carry a foreign checkout prefix forward into the updated graph.
legacy_chunk = {
"nodes": [
{"id": "foo_ts_foo", "label": "foo.ts",
"source_file": "foo.ts", "file_type": "code"},
{"id": "foo_mjs_foo", "label": "foo.mjs",
"source_file": "foo.mjs", "file_type": "code"},
],
"edges": [
{
"source": "foo_ts_foo",
"target": "foo_mjs_foo",
"relation": "imports_from",
"context": "re-export",
"confidence": "EXTRACTED",
"source_file": "foo.ts",
"target_file": "/some/other/checkout/foo.mjs",
"weight": 1.0,
}
],
}
graph = build([legacy_chunk], root=tmp_path)
assert graph.number_of_edges() == 1, "the base import edge should survive the merge"
for _src, _tgt, data in graph.edges(data=True):
assert "target_file" not in data, (
f"build() carried a persisted absolute target_file into the graph: {data}"
)
def test_target_file_hint_never_written_to_the_ast_cache(tmp_path: Path):
"""The hint is emitted only on JS/TS-family edges, and those suffixes bypass
the AST cache entirely (``_JS_CACHE_BYPASS_SUFFIXES``). A warm/relocated
cache therefore can never carry a foreign absolute target_file that would
miss the disambiguation salt. Lock that no AST cache entry stores it."""
mjs = _write(tmp_path / "foo.mjs", "export const N = 1;\n")
ts = _write(tmp_path / "foo.ts", 'export { N } from "./foo.mjs";\n')
extract([mjs, ts], cache_root=tmp_path)
ast_dir = tmp_path / "graphify-out" / "cache" / "ast"
entries = list(ast_dir.rglob("*.json")) if ast_dir.exists() else []
for entry in entries:
payload = json.loads(entry.read_text(encoding="utf-8"))
for edge in payload.get("edges", []):
assert "target_file" not in edge, (
f"AST cache entry {entry.name} stored a non-portable target_file "
f"hint: {edge}"
)