fix(install): fire the search nudge on Claude Code's Grep tool, not just Bash (#1986)
ee1df22 narrowed the Claude Code search-guard matcher from "Glob|Grep" to
"Bash" on the premise that dedicated search tools were removed and searches
go through Bash. Current Claude Code routes content search through its
first-class Grep tool (its Bash tool description actively steers away from
shell grep), so the graphify-first nudge never fired on the agent's primary
exploration path and the graph was silently bypassed.
Three-part fix, per the issue's analysis:
- Matcher: "Bash" -> "Bash|Grep" in _claude_pretooluse_hooks. Glob already
fires the read nudge via "Read|Glob", so Grep was the only orphaned tool.
- Guard body: the hook-guard search branch only inspected tool_input.command,
which a Grep call doesn't carry (it has pattern/path/glob). A Grep-shaped
input (pattern present, no command) is now treated as a search — it IS one
by definition — and nudges whenever a fresh graph exists. The Bash
token-matching path is unchanged, and a command-carrying input never
triggers the Grep shape, so non-search Bash calls stay silent.
- Idempotency: "Bash|Grep" added to the four install/uninstall dedup filters
(claude + codebuddy), so upgrading replaces the stale "Bash" hook in place
instead of appending a duplicate — verified against a pre-fix settings.json.
Tests: new regression tests feed Grep-shaped tool_input through
hook-guard search and assert the nudge (with graph), silence (without),
valid PreToolUse JSON, and no blocking; plus a guard that a non-search Bash
command with a stray pattern key does not nudge. Existing matcher assertions
updated across test_search_hook/test_install/test_claude_md/test_codebuddy/
test_hook_strict. Hook+install suites: 397 passed. Full suite: 3224 passed;
the 13 failures are pre-existing on clean v8 in this environment.
Fixes #1986
This commit is contained in:
+10
-3
@@ -444,11 +444,18 @@ def _run_hook_guard(kind: str, strict: bool = False) -> None:
|
||||
try:
|
||||
if kind == "search":
|
||||
cmd_str = str(t.get("command", "") or "")
|
||||
# Same set the old `case` matched: *grep*, *ripgrep*, and rg/find/fd/
|
||||
# Two input shapes reach this guard (matcher "Bash|Grep", #1986):
|
||||
# the Bash tool carries `command`, while Claude Code's dedicated
|
||||
# Grep tool carries `pattern` (plus optional path/glob) and no
|
||||
# command — a Grep call IS a content search by definition, so it
|
||||
# nudges whenever a graph exists. For Bash, keep matching the same
|
||||
# set the old `case` matched: *grep*, *ripgrep*, and rg/find/fd/
|
||||
# ack/ag as a token (name followed by a space). Nudge-only, even in
|
||||
# strict mode — see the docstring.
|
||||
if any(tok in cmd_str for tok in ("grep", "ripgrep", "rg ", "find ", "fd ", "ack ", "ag ")) \
|
||||
and out_path("graph.json").is_file():
|
||||
is_grep_tool = not cmd_str and bool(t.get("pattern"))
|
||||
is_bash_search = any(tok in cmd_str for tok in (
|
||||
"grep", "ripgrep", "rg ", "find ", "fd ", "ack ", "ag "))
|
||||
if (is_grep_tool or is_bash_search) and out_path("graph.json").is_file():
|
||||
sys.stdout.write(_SEARCH_NUDGE)
|
||||
elif kind == "read":
|
||||
vals = [str(t.get("file_path") or ""), str(t.get("pattern") or ""), str(t.get("path") or "")]
|
||||
|
||||
+12
-9
@@ -290,9 +290,12 @@ def _claude_pretooluse_hooks(strict: bool = False) -> "list[dict]":
|
||||
|
||||
The command invokes `graphify hook-guard <search|read>` 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 stay "Bash"
|
||||
and "Read|Glob" and the command always contains "graphify", so the existing
|
||||
install/uninstall filters find and replace both old bash hooks and these.
|
||||
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.
|
||||
|
||||
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
|
||||
@@ -303,7 +306,7 @@ def _claude_pretooluse_hooks(strict: bool = False) -> "list[dict]":
|
||||
exe = f'"{exe}"'
|
||||
read_cmd = f"{exe} hook-guard read" + (" --strict" if strict else "")
|
||||
return [
|
||||
{"matcher": "Bash",
|
||||
{"matcher": "Bash|Grep",
|
||||
"hooks": [{"type": "command", "command": f"{exe} hook-guard search"}]},
|
||||
{"matcher": "Read|Glob",
|
||||
"hooks": [{"type": "command", "command": read_cmd}]},
|
||||
@@ -1645,11 +1648,11 @@ def _install_claude_hook(project_dir: Path, strict: bool = False) -> None:
|
||||
hooks = settings.setdefault("hooks", {})
|
||||
pre_tool = hooks.setdefault("PreToolUse", [])
|
||||
|
||||
hooks["PreToolUse"] = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Read|Glob") and "graphify" in str(h))]
|
||||
hooks["PreToolUse"] = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))]
|
||||
hooks["PreToolUse"].extend(_claude_pretooluse_hooks(strict=strict))
|
||||
settings_path.write_text(json.dumps(settings, indent=2), encoding="utf-8")
|
||||
_mode = " (strict)" if strict else ""
|
||||
print(f" .claude/settings.json -> PreToolUse hooks registered (Bash search + Read/Glob){_mode}")
|
||||
print(f" .claude/settings.json -> PreToolUse hooks registered (Bash|Grep search + Read/Glob){_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.
|
||||
@@ -1669,7 +1672,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", "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") and "graphify" in str(h))]
|
||||
if len(filtered) == len(pre_tool):
|
||||
return
|
||||
settings["hooks"]["PreToolUse"] = filtered
|
||||
@@ -1820,7 +1823,7 @@ def _install_codebuddy_hook(project_dir: Path) -> None:
|
||||
hooks = settings.setdefault("hooks", {})
|
||||
pre_tool = hooks.setdefault("PreToolUse", [])
|
||||
|
||||
hooks["PreToolUse"] = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Read|Glob") and "graphify" in str(h))]
|
||||
hooks["PreToolUse"] = [h for h in pre_tool if not (h.get("matcher") in ("Glob|Grep", "Bash", "Bash|Grep", "Read|Glob") and "graphify" in str(h))]
|
||||
hooks["PreToolUse"].extend(_claude_pretooluse_hooks())
|
||||
settings_path.write_text(json.dumps(settings, indent=2), encoding="utf-8")
|
||||
print(f" .codebuddy/settings.json -> PreToolUse hooks registered")
|
||||
@@ -1834,7 +1837,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", "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") and "graphify" in str(h))]
|
||||
if len(filtered) == len(pre_tool):
|
||||
return
|
||||
settings["hooks"]["PreToolUse"] = filtered
|
||||
|
||||
@@ -109,7 +109,7 @@ def test_install_creates_settings_json(tmp_path):
|
||||
assert settings_path.exists()
|
||||
settings = json.loads(settings_path.read_text())
|
||||
hooks = settings.get("hooks", {}).get("PreToolUse", [])
|
||||
assert any(h.get("matcher") == "Bash" for h in hooks)
|
||||
assert any(h.get("matcher") == "Bash|Grep" for h in hooks)
|
||||
|
||||
|
||||
def test_install_settings_json_idempotent(tmp_path):
|
||||
@@ -120,7 +120,7 @@ def test_install_settings_json_idempotent(tmp_path):
|
||||
settings_path = tmp_path / ".claude" / "settings.json"
|
||||
settings = json.loads(settings_path.read_text())
|
||||
hooks = settings.get("hooks", {}).get("PreToolUse", [])
|
||||
bash_hooks = [h for h in hooks if h.get("matcher") == "Bash" and "graphify" in str(h)]
|
||||
bash_hooks = [h for h in hooks if h.get("matcher") == "Bash|Grep" and "graphify" in str(h)]
|
||||
assert len(bash_hooks) == 1
|
||||
|
||||
|
||||
@@ -133,7 +133,7 @@ def test_uninstall_removes_settings_hook(tmp_path):
|
||||
if settings_path.exists():
|
||||
settings = json.loads(settings_path.read_text())
|
||||
hooks = settings.get("hooks", {}).get("PreToolUse", [])
|
||||
assert not any(h.get("matcher") == "Bash" and "graphify" in str(h) for h in hooks)
|
||||
assert not any(h.get("matcher") == "Bash|Grep" and "graphify" in str(h) for h in hooks)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -98,7 +98,7 @@ def test_codebuddy_install_hook_has_bash_matcher(tmp_path):
|
||||
codebuddy_install(tmp_path)
|
||||
settings = json.loads(_settings_path(tmp_path).read_text())
|
||||
hooks = settings["hooks"]["PreToolUse"]
|
||||
bash_hooks = [h for h in hooks if h.get("matcher") == "Bash"]
|
||||
bash_hooks = [h for h in hooks if h.get("matcher") == "Bash|Grep"]
|
||||
assert any("graphify" in str(h) for h in bash_hooks)
|
||||
|
||||
|
||||
|
||||
@@ -199,4 +199,4 @@ def test_install_hook_carries_strict_flag():
|
||||
assert read_strict.endswith("hook-guard read --strict")
|
||||
# search hook is unchanged either way
|
||||
for hooks in (soft, strict):
|
||||
assert next(h for h in hooks if h["matcher"] == "Bash")["hooks"][0]["command"].endswith("hook-guard search")
|
||||
assert next(h for h in hooks if h["matcher"] == "Bash|Grep")["hooks"][0]["command"].endswith("hook-guard search")
|
||||
|
||||
@@ -367,7 +367,7 @@ def test_claude_hook_is_shell_agnostic(tmp_path):
|
||||
_install_claude_hook(tmp_path)
|
||||
hooks = _json.loads((tmp_path / ".claude" / "settings.json").read_text())["hooks"]["PreToolUse"]
|
||||
matchers = {h["matcher"] for h in hooks}
|
||||
assert {"Bash", "Read|Glob"} <= matchers
|
||||
assert {"Bash|Grep", "Read|Glob"} <= matchers # Grep in the search matcher: #1986
|
||||
for h in hooks:
|
||||
cmd = h["hooks"][0]["command"]
|
||||
for token in ("$(", "case ", "[ -f", "&&", "||", ";;", "echo '"):
|
||||
|
||||
@@ -15,7 +15,7 @@ from graphify.__main__ import _claude_pretooluse_hooks
|
||||
|
||||
def _search_matcher():
|
||||
hooks = _claude_pretooluse_hooks()
|
||||
return next(h for h in hooks if h["matcher"] == "Bash")
|
||||
return next(h for h in hooks if h["matcher"] == "Bash|Grep")
|
||||
|
||||
|
||||
def _env():
|
||||
@@ -35,8 +35,22 @@ def _run(command, cwd, *, graph: bool):
|
||||
)
|
||||
|
||||
|
||||
def test_matcher_targets_bash():
|
||||
assert _search_matcher()["matcher"] == "Bash"
|
||||
def _run_grep_tool(tool_input, cwd, *, graph: bool):
|
||||
"""Feed a Grep-tool-shaped payload (pattern/path/glob, no command) to the guard."""
|
||||
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": "Grep", "tool_input": tool_input})
|
||||
return subprocess.run(
|
||||
[sys.executable, "-m", "graphify", "hook-guard", "search"],
|
||||
input=stdin, capture_output=True, text=True, cwd=cwd, env=_env(),
|
||||
)
|
||||
|
||||
|
||||
def test_matcher_targets_bash_and_grep():
|
||||
# #1986: content search goes through Claude Code's dedicated Grep tool, so
|
||||
# the matcher must cover it alongside Bash.
|
||||
assert _search_matcher()["matcher"] == "Bash|Grep"
|
||||
|
||||
|
||||
def test_hook_command_has_no_backslashes(monkeypatch):
|
||||
@@ -121,3 +135,51 @@ def test_honors_graphify_out_override(tmp_path):
|
||||
input=stdin, capture_output=True, text=True, cwd=tmp_path, env=env,
|
||||
)
|
||||
assert "graphify query" in r.stdout
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #1986: the dedicated Grep tool (pattern/path/glob, no command) must nudge too
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_grep_tool_input_nudges_with_graph(tmp_path):
|
||||
for tool_input in (
|
||||
{"pattern": "extract_corpus", "path": "."},
|
||||
{"pattern": "TODO"},
|
||||
{"pattern": "def main", "glob": "*.py"},
|
||||
{"pattern": "foo", "path": "src/", "glob": "**/*.ts"},
|
||||
):
|
||||
out = _run_grep_tool(tool_input, tmp_path, graph=True).stdout
|
||||
assert "graphify query" in out, f"Grep input {tool_input!r} should nudge"
|
||||
|
||||
|
||||
def test_grep_tool_input_silent_without_graph(tmp_path):
|
||||
out = _run_grep_tool({"pattern": "foo", "path": "."}, tmp_path, graph=False).stdout
|
||||
assert out.strip() == ""
|
||||
|
||||
|
||||
def test_grep_tool_nudge_is_valid_pretooluse_json(tmp_path):
|
||||
out = _run_grep_tool({"pattern": "foo", "path": "."}, tmp_path, graph=True).stdout
|
||||
payload = json.loads(out)
|
||||
assert payload["hookSpecificOutput"]["hookEventName"] == "PreToolUse"
|
||||
assert "graphify query" in payload["hookSpecificOutput"]["additionalContext"]
|
||||
|
||||
|
||||
def test_grep_tool_never_blocks(tmp_path):
|
||||
r = _run_grep_tool({"pattern": "foo", "path": "."}, tmp_path, graph=True)
|
||||
assert r.returncode == 0
|
||||
assert '"permissionDecision"' not in r.stdout
|
||||
assert '"deny"' not in r.stdout
|
||||
|
||||
|
||||
def test_bash_non_search_with_stray_pattern_key_does_not_nudge(tmp_path):
|
||||
"""A Bash tool_input carries `command`; the Grep-shape detection must not
|
||||
fire when a command is present but is not a search."""
|
||||
(tmp_path / "graphify-out").mkdir(parents=True, exist_ok=True)
|
||||
(tmp_path / "graphify-out" / "graph.json").write_text("{}", encoding="utf-8")
|
||||
stdin = json.dumps({"tool_input": {"command": "ls -la", "pattern": "x"}})
|
||||
r = subprocess.run(
|
||||
[sys.executable, "-m", "graphify", "hook-guard", "search"],
|
||||
input=stdin, capture_output=True, text=True, cwd=tmp_path, env=_env(),
|
||||
)
|
||||
assert r.stdout.strip() == ""
|
||||
|
||||
Reference in New Issue
Block a user