diff --git a/graphify/cli.py b/graphify/cli.py index 88f5b51ed..e01e83047 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -50,6 +50,23 @@ ), } }, ensure_ascii=False, separators=(",", ":")) + "\n" +# #2145: most code exploration happens inside spawned subagents (Task/Agent), +# which the Bash/Grep and Read/Glob guards never see — the parent spawns an +# "Explore" agent and that agent never sees the guard. Nudge the SPAWN so the +# subagent carries graphify orientation. Advisory only (never blocks a spawn). +_AGENT_NUDGE = json.dumps({ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "additionalContext": ( + 'MANDATORY: graphify-out/graph.json exists. This subagent MUST use ' + 'graphify before grepping or reading raw files: run ' + '`graphify query ""` (scoped subgraph), ' + '`graphify explain ""`, or `graphify path "" ""` to ' + 'orient first. Only read/grep raw files after graphify has oriented ' + 'you, or to modify/debug specific lines.' + ), + } +}, ensure_ascii=False, separators=(",", ":")) + "\n" # Strict-mode block (opt-in). Claude Code PreToolUse honors # hookSpecificOutput.permissionDecision == "deny" and shows permissionDecisionReason # to the model. Fires at most once per session (see _mark_session_denied) so it can @@ -527,6 +544,23 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None: sys.stdout.write(_READ_DENY) return sys.stdout.write(_READ_NUDGE) + elif kind == "agent": + # #2145: matcher "Task|Agent". A subagent spawn carries its brief in + # `prompt` (+ optional `description`). When a graph exists, nudge the + # spawn to orient via graphify — unless the prompt is already + # graphify-aware (the parent pasted graph output or included the rule), + # in which case the subagent is covered and we stay quiet. Nudge-only: + # a spawn is never blocked. Fails open on an absent graph / empty brief. + if not out_path("graph.json").is_file(): + return + brief = " ".join(str(t.get(k) or "") for k in ("prompt", "description")) + low = brief.lower() + if not low.strip(): + return + # Already oriented -> the subagent will use graphify; don't nag. + if "graphify" in low or "connections (" in low or "[extracted]" in low: + return + sys.stdout.write(_AGENT_NUDGE) except Exception: pass diff --git a/graphify/install.py b/graphify/install.py index c3503c2e8..344f0d9c7 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -291,14 +291,16 @@ def _print_project_git_add_hint(paths: list[Path]) -> None: def _claude_pretooluse_hooks(strict: bool = False) -> "list[dict]": """graphify's Claude/Codebuddy PreToolUse hooks, resolved at install time. - The command invokes `graphify hook-guard ` via the absolute exe - path (`_resolve_graphify_exe`), so it parses under sh, cmd.exe and PowerShell + The command invokes `graphify hook-guard ` via the absolute + exe path (`_resolve_graphify_exe`), so it parses under sh, cmd.exe and PowerShell alike — this is the #522 fix, and mirrors the codex hook. Matchers are - "Bash|Grep" and "Read|Glob" and the command always contains "graphify", so the - existing install/uninstall filters find and replace both old bash hooks and - these. "Grep" is in the search matcher because current Claude Code routes - content search through its dedicated Grep tool, not Bash (#1986) — a - Bash-only matcher never fired on the agent's primary search path. + "Bash|Grep", "Read|Glob" and "Task|Agent", and the command always contains + "graphify", so the existing install/uninstall filters find and replace both old + bash hooks and these. "Grep" is in the search matcher because current Claude Code + routes content search through its dedicated Grep tool, not Bash (#1986) — a + Bash-only matcher never fired on the agent's primary search path. "Task|Agent" + covers subagent spawns, where most exploration actually happens (#2145): the + parent's spawn would otherwise never see the guard. When ``strict`` is set, the read hook carries ``--strict`` so it blocks the first raw read per session (Claude Code only). The ``GRAPHIFY_HOOK_STRICT`` env @@ -313,6 +315,8 @@ def _claude_pretooluse_hooks(strict: bool = False) -> "list[dict]": "hooks": [{"type": "command", "command": f"{exe} hook-guard search"}]}, {"matcher": "Read|Glob", "hooks": [{"type": "command", "command": read_cmd}]}, + {"matcher": "Task|Agent", + "hooks": [{"type": "command", "command": f"{exe} hook-guard agent"}]}, ] def _skill_registration(skill_path: str = "~/.claude/skills/graphify/SKILL.md") -> str: return ( @@ -1725,11 +1729,11 @@ def _install_claude_hook(project_dir: Path, strict: bool = False) -> None: if not isinstance(pre_tool, list): _refuse_to_modify(settings_path) - hooks["PreToolUse"] = [h for h in pre_tool if not (isinstance(h, dict) and h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))] + hooks["PreToolUse"] = [h for h in pre_tool if not (isinstance(h, dict) and h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob", "Task|Agent") and "graphify" in str(h))] hooks["PreToolUse"].extend(_claude_pretooluse_hooks(strict=strict)) _write_settings_with_backup(settings_path, settings) _mode = " (strict)" if strict else "" - print(f" .claude/settings.json -> PreToolUse hooks registered (Bash|Grep search + Read/Glob){_mode}") + print(f" .claude/settings.json -> PreToolUse hooks registered (Bash|Grep search + Read/Glob + Task/Agent subagents){_mode}") def _uninstall_claude_hook(project_dir: Path) -> None: """Remove the graphify PreToolUse hook from .claude/settings.json and its local-only sibling .claude/settings.local.json. @@ -1749,7 +1753,7 @@ def _strip_graphify_hook(settings_path: Path) -> None: except json.JSONDecodeError: return pre_tool = settings.get("hooks", {}).get("PreToolUse", []) - filtered = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))] + filtered = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob", "Task|Agent") and "graphify" in str(h))] if len(filtered) == len(pre_tool): return settings["hooks"]["PreToolUse"] = filtered @@ -1894,7 +1898,7 @@ def _install_codebuddy_hook(project_dir: Path) -> None: if not isinstance(pre_tool, list): _refuse_to_modify(settings_path) - hooks["PreToolUse"] = [h for h in pre_tool if not (isinstance(h, dict) and h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))] + hooks["PreToolUse"] = [h for h in pre_tool if not (isinstance(h, dict) and h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob", "Task|Agent") and "graphify" in str(h))] hooks["PreToolUse"].extend(_claude_pretooluse_hooks()) _write_settings_with_backup(settings_path, settings) print(f" .codebuddy/settings.json -> PreToolUse hooks registered") @@ -1908,7 +1912,7 @@ def _uninstall_codebuddy_hook(project_dir: Path) -> None: except json.JSONDecodeError: return pre_tool = settings.get("hooks", {}).get("PreToolUse", []) - filtered = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))] + filtered = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob", "Task|Agent") and "graphify" in str(h))] if len(filtered) == len(pre_tool): return settings["hooks"]["PreToolUse"] = filtered diff --git a/tests/test_agent_hook.py b/tests/test_agent_hook.py new file mode 100644 index 000000000..d675c8a18 --- /dev/null +++ b/tests/test_agent_hook.py @@ -0,0 +1,127 @@ +"""The Task/Agent PreToolUse guard nudges subagent spawns toward the graph (#2145). + +Most code exploration happens inside spawned subagents (`Task`/`Agent`), which the +Bash|Grep and Read|Glob guards never see — the parent spawns an "Explore" agent and +that agent never sees the guard. The `graphify hook-guard agent` subcommand, wired +to matcher "Task|Agent", nudges the spawn to carry graphify orientation when a graph +exists, unless the prompt is already graphify-aware. It never blocks a spawn. +""" +import json +import os +import subprocess +import sys + +from graphify.__main__ import _claude_pretooluse_hooks + + +def _agent_matcher(): + hooks = _claude_pretooluse_hooks() + return next(h for h in hooks if h["matcher"] == "Task|Agent") + + +def _env(): + e = dict(os.environ) + e.pop("GRAPHIFY_OUT", None) + return e + + +def _run(tool_input, cwd, *, graph: bool, tool_name="Task"): + if graph: + (cwd / "graphify-out").mkdir(parents=True, exist_ok=True) + (cwd / "graphify-out" / "graph.json").write_text("{}", encoding="utf-8") + stdin = json.dumps({"tool_name": tool_name, "tool_input": tool_input}) + return subprocess.run( + [sys.executable, "-m", "graphify", "hook-guard", "agent"], + input=stdin, capture_output=True, text=True, cwd=cwd, env=_env(), + ) + + +def test_matcher_targets_task_and_agent(): + assert _agent_matcher()["matcher"] == "Task|Agent" + + +def test_command_is_hook_guard_agent(): + cmd = _agent_matcher()["hooks"][0]["command"] + assert "graphify" in cmd and "hook-guard agent" in cmd + + +def test_command_has_no_shell_syntax_or_backslashes(monkeypatch): + from graphify.__main__ import _resolve_graphify_exe + monkeypatch.setattr("shutil.which", lambda _name: r"C:\Users\me\graphify.EXE") + assert _resolve_graphify_exe() == "C:/Users/me/graphify.EXE" + cmd = next(h for h in _claude_pretooluse_hooks() + if h["matcher"] == "Task|Agent")["hooks"][0]["command"] + assert "\\" not in cmd + for token in ("$(", "case ", "[ -f", "&&", "||", ";;", "echo '"): + assert token not in cmd + + +def test_nudges_on_exploration_spawn_with_graph(tmp_path): + for tool_input in ( + {"prompt": "find all callers of PaymentService"}, + {"description": "explore auth", "prompt": "trace how login works across files"}, + {"prompt": "Where is the retry logic implemented?"}, + ): + out = _run(tool_input, tmp_path, graph=True).stdout + assert "graphify query" in out, f"{tool_input!r} should nudge" + + +def test_silent_without_graph(tmp_path): + out = _run({"prompt": "find all callers of X"}, tmp_path, graph=False).stdout + assert out.strip() == "" + + +def test_silent_when_prompt_already_graphify_aware(tmp_path): + for tool_input in ( + {"prompt": "Use `graphify query` to map the module, then summarize"}, + {"prompt": "Given these Connections (12) from the graph, refactor X"}, + {"prompt": "The [EXTRACTED] edges show a cycle; propose a fix"}, + ): + out = _run(tool_input, tmp_path, graph=True).stdout + assert out.strip() == "", f"{tool_input!r} already oriented; should stay quiet" + + +def test_silent_on_empty_brief(tmp_path): + out = _run({"prompt": ""}, tmp_path, graph=True).stdout + assert out.strip() == "" + + +def test_nudge_payload_is_valid_pretooluse_json(tmp_path): + out = _run({"prompt": "find callers of X"}, tmp_path, graph=True).stdout + payload = json.loads(out) + assert payload["hookSpecificOutput"]["hookEventName"] == "PreToolUse" + assert "graphify" in payload["hookSpecificOutput"]["additionalContext"] + + +def test_never_blocks_a_spawn(tmp_path): + r = _run({"prompt": "find callers of X"}, tmp_path, graph=True) + assert r.returncode == 0 + assert '"permissionDecision"' not in r.stdout + assert '"deny"' not in r.stdout + + +def test_fails_open_on_malformed_stdin(tmp_path): + (tmp_path / "graphify-out").mkdir() + (tmp_path / "graphify-out" / "graph.json").write_text("{}", encoding="utf-8") + r = subprocess.run( + [sys.executable, "-m", "graphify", "hook-guard", "agent"], + input="not json", capture_output=True, text=True, cwd=tmp_path, env=_env(), + ) + assert r.returncode == 0 + assert r.stdout.strip() == "" + + +def test_uninstall_filter_removes_our_hook_but_keeps_foreign(tmp_path): + # The uninstall/reinstall filter must strip graphify's own Task|Agent hook + # while preserving an unrelated Task|Agent hook another tool installed. + matcher_tuple = ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob", "Task|Agent") + pre_tool = [ + {"matcher": "Task|Agent", + "hooks": [{"type": "command", "command": "/x/graphify hook-guard agent"}]}, + {"matcher": "Task|Agent", + "hooks": [{"type": "command", "command": "some-other-tool run"}]}, + ] + filtered = [h for h in pre_tool + if not (h.get("matcher") in matcher_tuple and "graphify" in str(h))] + kept = [h["hooks"][0]["command"] for h in filtered] + assert kept == ["some-other-tool run"] diff --git a/tests/test_install.py b/tests/test_install.py index 8cea17b4c..7c33dad90 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -389,7 +389,9 @@ def test_claude_hook_install_idempotent_and_replaces_old_bash_hook(tmp_path): _install_claude_hook(tmp_path) # second install must not duplicate hooks = _json.loads(settings_path.read_text())["hooks"]["PreToolUse"] graphify_hooks = [h for h in hooks if "graphify" in str(h)] - assert len(graphify_hooks) == 2, "exactly the Bash + Read|Glob guards, no dupes" + assert len(graphify_hooks) == 3, \ + "exactly the Bash|Grep + Read|Glob + Task|Agent guards, no dupes" + assert {h["matcher"] for h in graphify_hooks} == {"Bash|Grep", "Read|Glob", "Task|Agent"} # the legacy bash payload must be gone assert not any("[ -f graphify-out" in h["hooks"][0]["command"] for h in graphify_hooks) diff --git a/tests/test_settings_merge.py b/tests/test_settings_merge.py index ab4cd562b..c4ca77461 100644 --- a/tests/test_settings_merge.py +++ b/tests/test_settings_merge.py @@ -87,7 +87,7 @@ def test_claude_install_preserves_existing_settings(tmp_path): pre_tool = result["hooks"]["PreToolUse"] assert seeded["hooks"]["PreToolUse"][0] in pre_tool graphify_hooks = [h for h in pre_tool if "graphify" in str(h)] - assert len(graphify_hooks) == 2 + assert len(graphify_hooks) == 3 # Bash|Grep + Read|Glob + Task|Agent (#2145) # strict=True lands on the read guard assert any(h["hooks"][0]["command"].endswith("--strict") for h in graphify_hooks)