Resolve graph.json from an ancestor directory, plus two independent fixes - #3296
Resolve graph.json from an ancestor directory, plus two independent fixes#3296bobthearsonist wants to merge 7 commits into
Conversation
Passing follow_symlinks=True declares intent to follow links outside the scan root, but collect_files applied _resolves_under_root unconditionally, rejecting every file reached through such a link. That made the flag inert for its only documented use case: a directory of links to scattered source dirs. Skip containment when the caller explicitly opted in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Read commands defaulted to <GRAPHIFY_OUT>/graph.json relative to cwd, so query/path/explain/affected/god-nodes failed from any subdirectory of the scan root -- a package subdir, a git worktree -- even when a usable graph sat one or more levels up. The workaround was a shell wrapper function, which only ever worked in the one shell that sourced it and did nothing for tools that exec graphify directly. Walk up to the nearest readable, non-empty <GRAPHIFY_OUT>/graph.json when the configured path is not present here, and name the resolved graph on stderr (stdout carries the answer and is parsed by callers). Candidates that exist but are empty, unreadable, or a dangling link are skipped so the walk continues rather than returning a path that fails to parse. An explicit --graph still wins and is passed through untouched even when it does not exist; a graph present in cwd is returned with no filesystem walk, keeping the common case byte-identical; and an absolute GRAPHIFY_OUT disables the walk, since it names one fixed location rather than a per-root convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`explain` resolved an ambiguous label by refusing to answer: when the winning match tier spanned several source files it listed them and exited 1. Indexing git worktrees beside their canonical repo makes that tie the normal case -- the same symbol legitimately exists as matching/Foo.py and matching-1234-ga-merge/Foo.py -- so the command failed even when standing in the worktree whose copy was obviously meant. Prefer the candidate whose top-level directory is the one cwd is in, derived from the graph's own scan root the way `affected` derives it. When cwd singles out nothing, warn on stderr naming every alternative and answer anyway, matching the "warning: ... was ambiguous" style `path` already uses. Ambiguity now travels on stderr so stdout stays parseable. The fallback picks the lowest source_file rather than the first match, so the answer stays stable under graph iteration order -- the ordering bug the previous hard failure existed to prevent is still covered, without making the command unusable. `deduplicate_by_label` is deliberately left off: it conflates same-label symbols across files, which is the opposite of the distinction being drawn here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
default_graph_json is documented as the package-wide fallback, but only the CLI walked up to an ancestor graph; serve, build, prs and benchmark got a cwd-relative path that does not exist outside the scan root. The MCP server inherits the client's cwd, which is routinely a git worktree, so it failed where the CLI succeeded from the identical directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A multi-root workspace graph covering many repositories plus the worktrees under active development runs well past 512 MiB. Tripping the cap fails both the CLI and the MCP server with "could not load graph.json", which reads as corruption rather than as a configurable limit. The cap guards against absurd input, not against a legitimate graph. GRAPHIFY_MAX_GRAPH_BYTES still overrides. It is not a sufficient answer on its own: a machine environment variable does not reach processes that are already running, so the stack stays broken until every client is restarted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fast-path check told agents to look for graphify-out/graph.json "relative to the current working directory". In a git worktree or any nested subdirectory that file is absent while a perfectly good graph sits one or more levels up, so an agent following the skill concludes no graph exists and falls back to grep or to rebuilding one. The read commands already resolve upward and report which graph they used, so the reliable check is to run one rather than to stat a fixed path. Applied to every client variant, since the mistake is identical in all of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The PreToolUse guard checked out_path("graph.json") relative to the current
directory. Agents run from wherever they happen to be — a git worktree, a
package subdirectory — while the graph sits at the scan root above them, so
the guard silently never fired in exactly the places it was needed and the
agent grepped a codebase that had a graph one level up.
Mirrors what the read commands already do. Still fails open: any error
resolves to None and the guard prints nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Formal verification. 4 change(s) tested, no difference found (not proven).
Graphify review — findings
Makes read commands (explain, affected, god-nodes, path) resolve a missing default graph by walking up to the nearest ancestor's graph.json, while an explicit --graph is always used verbatim even when absent. Extends the hook guards (search/read/gemini) to fire when a graph exists in an ancestor rather than only in cwd, so agents running from worktrees or subdirectories still get nudged. Disambiguates explain when several files define the same label by preferring the top-level directory the caller is standing in, falling back to a stderr warning plus the lowest-sorted source_file for a stable pick.
Worth a look
- SSRF guard bypassed when follow_symlinks=True in collect_files —
graphify/extract.py:7080· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- collect_files no longer enforces root containment when following symlinks —
graphify/extract.py:7081· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Default graph lookup trusts attacker-controlled ancestor directories —
graphify/cli.py:108· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- follow_symlinks bypasses root containment and includes files outside the scan root —
graphify/extract.py:7078· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- follow_symlinks=True disables containment, enabling path traversal outside root —
graphify/extract.py:7080· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3056 functions depend on the 739 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 490 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
build_from_json()— 187 callers, 18 callees - new:
build_merge()— 46 callers, 14 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
save_manifest()— 40 callers, 11 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_bash()— 41 callers, 10 callees - …and 74 more — each is listed as a finding
Verification — 3056 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 2836 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_check\_skill\_version.
The verifier did not have enough to check \_check\_skill\_version, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `skill_dst` is annotated `Path` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).
The verifier ran both versions of \_run\_cli on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in find\_import\_cycles (not a proof).
The verifier ran both versions of find\_import\_cycles on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_infer\_merge\_root.
The verifier did not have enough to check \_infer\_merge\_root, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `graph_path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify build\_merge.
The verifier did not have enough to check build\_merge, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify merge\_raw\_extraction.
The verifier did not have enough to check merge\_raw\_extraction, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `graph_path` is annotated `str | Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify prefix\_graph\_for\_global.
The verifier did not have enough to check prefix\_graph\_for\_global, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 115 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous
Could not verify: Could not verify \_absolutize\_ids\_in.
The verifier did not have enough to check \_absolutize\_ids\_in, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_flush\_stat\_index (not a proof).
The verifier ran both versions of \_flush\_stat\_index on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_relativize\_ids\_in.
The verifier did not have enough to check \_relativize\_ids\_in, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `'str | Path'` — outside the synthesizable primitive/collection set
No difference found (not proven): No behavior difference found in \_rewrite\_strings (not a proof).
The verifier ran both versions of \_rewrite\_strings on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 81 more finding(s) on lines outside this diff (see the check run).
| @@ -7075,9 +7075,15 @@ def _canon(nid: str) -> str: | |||
|
|
|||
| def collect_files(target: Path, *, follow_symlinks: bool = False, root: Path | None = None) -> list[Path]: | |||
There was a problem hiding this comment.
collect_files()
fans out to 7 callees (efferent coupling); 17 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
In a multi-repo workspace the graph lives at a parent root — here
C:/Repositories/graphify-out/covering 23 repos. Queries from a child repo or worktree failed with "graph file not found", because resolution only ever looked in cwd.Five commits make ancestor resolution consistent: CLI, non-CLI callers,
hook-guard, and the skill doc.Two independent fixes:
follow_symlinks=Truewas overridden by a root-containment check, so linked trees were silently skipped rather than followed.graph.jsonsize cap rejected an 859 MB workspace graph. Raised to 2 GiB.Verified on Windows against a 331k-node, 23-repo graph, resolving from the CLI, PowerShell, and the MCP server.