fix: four production bugs — Windows crashes, ghost-merge collision, version probe
extract.py: clamp ProcessPoolExecutor max_workers to 61 on Windows (issue #1298). Python's ProcessPoolExecutor hard-caps at 61 on Windows via WaitForMultipleObjects; >61-core machines crashed on AST extraction. Clamp applied after all input paths (auto-compute, GRAPHIFY_MAX_WORKERS, --max-workers) to cover all three. build.py: skip ghost-merge when two AST nodes share (basename, label) key (issue #1257). When same-named symbols appear in same-named files across directories (e.g. two render() in two index.ts), last-writer-wins produced an arbitrary canonical node and mis-pointed all edges. Now tracked in _loc_collisions; ambiguous keys are skipped in Pass 2, leaving the ghost intact rather than merging into the wrong node. __main__.py: ignore OSError on unreadable .graphify_version probes (issue #1299). On restricted-permission installs or network mounts, .exists()/.read_text() raised PermissionError and crashed every graphify query/explain/path call at startup. All three FS probes now wrapped in try/except OSError: return. prs.py: resolve claude.cmd on Windows in prs.py claude-cli backend (issue #1288). The _call_llm and _call_claude_cli paths were already fixed; prs.py had the same bare ["claude", ...] call that fails on Windows npm installs with WinError 2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
813db19252
commit
b83086297f
+13
-3
@@ -94,9 +94,16 @@ def _enforce_graph_size_cap_or_exit(gp: Path) -> None:
|
||||
def _check_skill_version(skill_dst: Path) -> None:
|
||||
"""Warn if the installed skill is from an older graphify version."""
|
||||
version_file = skill_dst.parent / ".graphify_version"
|
||||
if not version_file.exists():
|
||||
try:
|
||||
if not version_file.exists():
|
||||
return
|
||||
except OSError:
|
||||
return
|
||||
if not skill_dst.exists():
|
||||
try:
|
||||
skill_exists = skill_dst.exists()
|
||||
except OSError:
|
||||
return
|
||||
if not skill_exists:
|
||||
print(" warning: skill dir exists but SKILL.md is missing. Run 'graphify install' to repair.")
|
||||
return
|
||||
# A progressive SKILL.md links to its references/ sidecar. If the body points
|
||||
@@ -108,7 +115,10 @@ def _check_skill_version(skill_dst: Path) -> None:
|
||||
body = ""
|
||||
if "references/" in body and not (skill_dst.parent / "references").exists():
|
||||
print(" warning: skill references/ sidecar is missing. Run 'graphify install' to repair.", file=sys.stderr)
|
||||
installed = version_file.read_text(encoding="utf-8").strip()
|
||||
try:
|
||||
installed = version_file.read_text(encoding="utf-8").strip()
|
||||
except OSError:
|
||||
return
|
||||
if installed != __version__:
|
||||
print(f" warning: skill is from graphify {installed}, package is {__version__}. Run 'graphify install' to update.", file=sys.stderr)
|
||||
|
||||
|
||||
+17
-3
@@ -165,9 +165,15 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
# _origin=="ast" as the canonical signal. AST nodes always win; any non-AST
|
||||
# node sharing (basename, label) with an AST node is a ghost.
|
||||
_loc_nodes: dict[tuple[str, str], str] = {} # (basename, label) -> canonical node id
|
||||
_loc_collisions: set[tuple[str, str]] = set() # keys shared by 2+ AST nodes
|
||||
_noloc_nodes: dict[tuple[str, str], str] = {} # (basename, label) -> ghost node id
|
||||
|
||||
# Pass 1: collect canonical nodes — AST-origin nodes take precedence over LLM nodes.
|
||||
# When 2+ AST nodes share a key (same-named symbols in same-named files across
|
||||
# directories, e.g. render in two index.ts), the key is ambiguous: merging a
|
||||
# ghost would pick an arbitrary winner via set-iteration order (#1257). Track
|
||||
# those keys so Pass 2 skips them — same conservatism as
|
||||
# _rewire_unique_stub_nodes, which only merges when exactly one real def exists.
|
||||
for nid in node_set:
|
||||
attrs = G.nodes[nid]
|
||||
label = str(attrs.get("label", "")).strip()
|
||||
@@ -175,10 +181,16 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
basename = Path(sf).name if sf else ""
|
||||
if not label or not basename:
|
||||
continue
|
||||
if attrs.get("source_location") or attrs.get("_origin") == "ast":
|
||||
is_ast = attrs.get("_origin") == "ast"
|
||||
if attrs.get("source_location") or is_ast:
|
||||
key = (basename, label)
|
||||
# AST-origin nodes always overwrite; non-AST only written if key unseen.
|
||||
if attrs.get("_origin") == "ast" or key not in _loc_nodes:
|
||||
if is_ast:
|
||||
# Two AST nodes on the same key is an ambiguous collision.
|
||||
if key in _loc_nodes and G.nodes[_loc_nodes[key]].get("_origin") == "ast":
|
||||
_loc_collisions.add(key)
|
||||
# AST-origin nodes always overwrite a prior non-AST entry.
|
||||
_loc_nodes[key] = nid
|
||||
elif key not in _loc_nodes:
|
||||
_loc_nodes[key] = nid
|
||||
|
||||
# Pass 2: find ghosts — non-AST nodes that have an AST canonical twin.
|
||||
@@ -192,6 +204,8 @@ def build_from_json(extraction: dict, *, directed: bool = False, root: str | Pat
|
||||
if not label or not basename:
|
||||
continue
|
||||
key = (basename, label)
|
||||
if key in _loc_collisions:
|
||||
continue # ambiguous key: no safe canonical winner, leave ghost intact
|
||||
if key in _loc_nodes and _loc_nodes[key] != nid:
|
||||
_noloc_nodes[key] = nid
|
||||
# For every ghost that has an AST counterpart, record a remap.
|
||||
|
||||
@@ -11565,6 +11565,14 @@ def _extract_parallel(
|
||||
cpu_cap = env_cap if env_cap is not None else (os.cpu_count() or 4)
|
||||
max_workers = min(cpu_cap, len(uncached_work))
|
||||
|
||||
# Windows ProcessPoolExecutor hard-caps at 61 workers (CPython limitation
|
||||
# tied to WaitForMultipleObjects). Clamp here so every path — auto-compute,
|
||||
# GRAPHIFY_MAX_WORKERS, and --max-workers — stays valid on >61-core boxes
|
||||
# (issue #1298). Guard against 0 from an empty work list.
|
||||
if sys.platform == "win32":
|
||||
max_workers = min(max_workers, 61)
|
||||
max_workers = max(max_workers, 1)
|
||||
|
||||
root_str = str(effective_root)
|
||||
work_items = [(idx, str(path), root_str) for idx, path in uncached_work]
|
||||
|
||||
|
||||
+5
-2
@@ -643,9 +643,12 @@ def triage_with_opus(prs: list[PRInfo], base: str) -> None:
|
||||
print("\n")
|
||||
|
||||
elif backend == "claude-cli":
|
||||
import subprocess as _sp
|
||||
import platform as _platform, shutil as _shutil, subprocess as _sp
|
||||
_claude = "claude"
|
||||
if _platform.system() == "Windows":
|
||||
_claude = _shutil.which("claude.cmd") or _shutil.which("claude") or "claude"
|
||||
proc = _sp.run(
|
||||
["claude", "-p", "--no-session-persistence"],
|
||||
[_claude, "-p", "--no-session-persistence"],
|
||||
input=prompt, capture_output=True, text=True, timeout=120,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
|
||||
@@ -149,6 +149,56 @@ def test_file_type_synonym_mapping():
|
||||
assert G.nodes["n3"]["file_type"] == "concept"
|
||||
|
||||
|
||||
def test_ghost_merge_unique_located_node_still_merges():
|
||||
"""#1145 ghost-merge: a semantic ghost collapses into the single AST node
|
||||
sharing its (basename, label), and edges re-point to the AST node."""
|
||||
ext = {
|
||||
"nodes": [
|
||||
{"id": "ast_render", "label": "render", "file_type": "code",
|
||||
"source_file": "src/app/index.ts", "source_location": "L10", "_origin": "ast"},
|
||||
{"id": "ghost_render", "label": "render", "file_type": "code",
|
||||
"source_file": "src/app/index.ts"},
|
||||
{"id": "caller", "label": "main", "file_type": "code",
|
||||
"source_file": "src/main.ts", "source_location": "L1", "_origin": "ast"},
|
||||
],
|
||||
"edges": [{"source": "caller", "target": "ghost_render", "relation": "calls",
|
||||
"confidence": "EXTRACTED", "source_file": "src/main.ts", "weight": 1.0}],
|
||||
"input_tokens": 0, "output_tokens": 0,
|
||||
}
|
||||
G = build_from_json(ext)
|
||||
assert "ghost_render" not in G.nodes()
|
||||
assert G.has_edge("caller", "ast_render")
|
||||
|
||||
|
||||
def test_ghost_merge_skipped_on_basename_collision():
|
||||
"""#1257: when two files with the same basename both define a symbol with the
|
||||
same label, the (basename, label) key is ambiguous and the semantic ghost
|
||||
must not be merged into an arbitrary one of them."""
|
||||
ext = {
|
||||
"nodes": [
|
||||
{"id": "a_render", "label": "render", "file_type": "code",
|
||||
"source_file": "src/a/index.ts", "source_location": "L10", "_origin": "ast"},
|
||||
{"id": "b_render", "label": "render", "file_type": "code",
|
||||
"source_file": "src/b/index.ts", "source_location": "L20", "_origin": "ast"},
|
||||
{"id": "ghost_render", "label": "render", "file_type": "code",
|
||||
"source_file": "src/a/index.ts"},
|
||||
{"id": "caller", "label": "main", "file_type": "code",
|
||||
"source_file": "src/main.ts", "source_location": "L1", "_origin": "ast"},
|
||||
],
|
||||
"edges": [{"source": "caller", "target": "ghost_render", "relation": "calls",
|
||||
"confidence": "EXTRACTED", "source_file": "src/main.ts", "weight": 1.0}],
|
||||
"input_tokens": 0, "output_tokens": 0,
|
||||
}
|
||||
G = build_from_json(ext)
|
||||
# The ghost survives: merging it into either a_render or b_render would
|
||||
# pick an arbitrary winner (set iteration order over node_set).
|
||||
assert "ghost_render" in G.nodes()
|
||||
assert G.number_of_nodes() == 4
|
||||
assert G.has_edge("caller", "ghost_render")
|
||||
assert not G.has_edge("caller", "a_render")
|
||||
assert not G.has_edge("caller", "b_render")
|
||||
|
||||
|
||||
def test_build_merge_preserves_call_edge_direction(tmp_path):
|
||||
"""Regression for #760.
|
||||
|
||||
|
||||
@@ -150,6 +150,33 @@ def test_check_skill_version_warns_on_missing_references(tmp_path, fake_bundle,
|
||||
assert "references/ sidecar is missing" in err
|
||||
|
||||
|
||||
def test_check_skill_version_ignores_permission_error(tmp_path, fake_bundle, monkeypatch, capsys):
|
||||
"""Unreadable version probes should not crash startup."""
|
||||
platform = fake_bundle
|
||||
_install(tmp_path, platform)
|
||||
skill_dir = tmp_path / ".claude" / "skills" / "graphify"
|
||||
skill = skill_dir / "SKILL.md"
|
||||
|
||||
# Drain install output so the no-warning assertions below see only what
|
||||
# _check_skill_version itself emits.
|
||||
capsys.readouterr()
|
||||
|
||||
original_exists = Path.exists
|
||||
|
||||
def guarded_exists(self):
|
||||
if self.name == ".graphify_version":
|
||||
raise PermissionError("denied")
|
||||
return original_exists(self)
|
||||
|
||||
monkeypatch.setattr(Path, "exists", guarded_exists)
|
||||
|
||||
mainmod._check_skill_version(skill)
|
||||
|
||||
out = capsys.readouterr()
|
||||
assert out.out == ""
|
||||
assert out.err == ""
|
||||
|
||||
|
||||
def test_hard_fail_when_bundle_dir_present_but_references_missing(tmp_path, monkeypatch):
|
||||
"""A bundle dir that exists but has no references/ subdir is a malformed
|
||||
package: exit 1 rather than silently shipping an empty sidecar.
|
||||
|
||||
Reference in New Issue
Block a user