mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-13 00:15:05 +00:00
Hooks: read hook only fires on read of code files
Using heuristics to determine whether it's a read of a code file based on the command
This commit is contained in:
1 parent
7df64bcee0
commit
66d223c9e1
3 files changed
+49
-33
No files matched your search
@@ -4,6 +4,7 @@ Status of the `main` branch. Changes prior to the next official version change w
|
||||
|
||||
* Hooks:
|
||||
- Extension: heuristics for parsing commands and firing a hook on too many greps or reads. Important for clients that, unlike claude code, don't have dedicated grep/read tools.
|
||||
- Read hook now only fires on reads of code files (using heuristics to parse the read command string)
|
||||
|
||||
# v1.2.0 (2026-04-27)
|
||||
|
||||
|
||||
+44
-29
@@ -13,6 +13,7 @@ from typing import Literal, Self
|
||||
import click
|
||||
|
||||
from serena.util.cli_util import AutoRegisteringGroup
|
||||
from solidlsp.ls_config import Language
|
||||
|
||||
# copied from serena_config.py, we don't want to import anything here to keep the hook commands fast
|
||||
serena_home_dir = os.getenv("SERENA_HOME", "").strip() or str(Path.home() / ".serena")
|
||||
@@ -190,8 +191,8 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
return
|
||||
|
||||
now = hook.triggered_at_timestamp
|
||||
is_grep = hook.is_grep_tool()
|
||||
is_read = hook.is_read_file_tool()
|
||||
is_grep = hook.is_grep_call()
|
||||
is_code_file_read = hook.is_read_code_file_call()
|
||||
|
||||
# update grep counter
|
||||
if is_grep:
|
||||
@@ -203,7 +204,7 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
self.last_grep_use_timestamp = now
|
||||
|
||||
# update read file counter
|
||||
if is_read:
|
||||
if is_code_file_read:
|
||||
read_period = self._READ_FILE_RESET_PERIOD_SECONDS
|
||||
if (
|
||||
self.last_read_file_use_timestamp is not None
|
||||
@@ -216,7 +217,7 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
|
||||
# update combined non-symbolic counter — catches mixed bursts (e.g.
|
||||
# alternating grep/read) that neither per-tool counter would trip
|
||||
if is_grep or is_read:
|
||||
if is_grep or is_code_file_read:
|
||||
combined_period = self._NON_SYMBOLIC_RESET_PERIOD_SECONDS
|
||||
if (
|
||||
self.last_non_symbolic_use_timestamp is not None
|
||||
@@ -255,41 +256,55 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
super().__init__(client)
|
||||
self._tool_call_counter = self.ToolUseCounter.load(self)
|
||||
|
||||
def _get_exec_command_name(self) -> str:
|
||||
"""Extract the command name from an ``exec_command`` tool input.
|
||||
# Extract the command name from an ``exec_command`` tool input.
|
||||
#
|
||||
# We use the basename of the first whitespace-separated token of the
|
||||
# ``cmd`` field, lowercased, so that both bare names (``rg``) and
|
||||
# path-prefixed invocations (``/usr/bin/grep``) are normalised the same
|
||||
# way. Returns an empty string when the input is absent or empty.
|
||||
self._command: str | None = None
|
||||
self._command_name: str | None = None
|
||||
self._command_args_str: str | None = None
|
||||
if self._tool_input is not None:
|
||||
self._command = self._tool_input.get("cmd", self._tool_input.get("command", "")).strip()
|
||||
if self._command:
|
||||
cmd_split = self._command.split(maxsplit=1)
|
||||
if len(cmd_split) > 1:
|
||||
self._command_args_str = cmd_split[1]
|
||||
self._command_name = os.path.basename(cmd_split[0]).lower()
|
||||
|
||||
Returns the basename of the first whitespace-separated token of the
|
||||
``cmd`` field, lowercased, so that both bare names (``rg``) and
|
||||
path-prefixed invocations (``/usr/bin/grep``) are normalised the same
|
||||
way. Returns an empty string when the input is absent or empty.
|
||||
"""
|
||||
if self._tool_input is None:
|
||||
return ""
|
||||
cmd = str(self._tool_input.get("cmd", "") or "").strip()
|
||||
if not cmd:
|
||||
return ""
|
||||
first_token = cmd.split()[0]
|
||||
return os.path.basename(first_token).lower()
|
||||
def _cmd_args_contain_code_file(self) -> bool:
|
||||
"""Heuristic, looks whether the command string contains a substring that looks like a code extension."""
|
||||
if not self._command_args_str:
|
||||
return False
|
||||
for lang in Language.iter_all(include_non_programming_languages=False):
|
||||
file_matcher = lang.get_source_fn_matcher()
|
||||
if file_matcher.string_contains_relevant_filename(self._command_args_str):
|
||||
return True
|
||||
return False
|
||||
|
||||
def is_grep_tool(self) -> bool:
|
||||
def is_grep_call(self) -> bool:
|
||||
if self._client == HookClient.CLAUDE_CODE:
|
||||
return self._tool_name == "grep"
|
||||
if self._client == HookClient.CODEX and self._tool_name == "exec_command":
|
||||
return self._get_exec_command_name() in self._GREP_SHELL_COMMANDS
|
||||
return self._command_name in self._GREP_SHELL_COMMANDS
|
||||
# heuristic for other clients
|
||||
return "grep" in self._tool_name
|
||||
|
||||
def is_read_file_tool(self) -> bool:
|
||||
def is_read_call(self) -> bool:
|
||||
if self._client == HookClient.CLAUDE_CODE:
|
||||
return self._tool_name == "read"
|
||||
if self._client == HookClient.CODEX and self._tool_name == "exec_command":
|
||||
return self._get_exec_command_name() in self._READ_SHELL_COMMANDS
|
||||
return self._command_name in self._READ_SHELL_COMMANDS
|
||||
# heuristic for other clients
|
||||
name = self._tool_name
|
||||
if "file" not in name:
|
||||
return False
|
||||
return any(verb in name for verb in self._READ_FILE_VERB_SUBSTRINGS)
|
||||
|
||||
def is_read_code_file_call(self) -> bool:
|
||||
return self.is_read_call() and self._cmd_args_contain_code_file()
|
||||
|
||||
def execute(self) -> None:
|
||||
# gate the entire hook on the rate-limit window: while we are within
|
||||
# _MIN_DENY_INTERVAL_SECONDS of the last emitted deny, no counter
|
||||
@@ -310,14 +325,14 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
too_many_non_symbolic = self._tool_call_counter.too_many_recent_non_symbolic()
|
||||
|
||||
output_data: PreToolUseHook.OutputData | None = None
|
||||
if self.is_grep_tool() and too_many_greps:
|
||||
if self.is_grep_call() and too_many_greps:
|
||||
output_data = self._build_grep_deny()
|
||||
elif self.is_read_file_tool() and too_many_reads:
|
||||
output_data = self._build_read_deny()
|
||||
elif self.is_read_code_file_call() and too_many_reads:
|
||||
output_data = self._build_code_read_deny()
|
||||
elif too_many_greps:
|
||||
output_data = self._build_grep_deny()
|
||||
elif too_many_reads:
|
||||
output_data = self._build_read_deny()
|
||||
output_data = self._build_code_read_deny()
|
||||
elif too_many_non_symbolic:
|
||||
output_data = self._build_non_symbolic_deny()
|
||||
|
||||
@@ -341,13 +356,13 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook):
|
||||
),
|
||||
)
|
||||
|
||||
def _build_read_deny(self) -> "PreToolUseHook.OutputData":
|
||||
def _build_code_read_deny(self) -> "PreToolUseHook.OutputData":
|
||||
return self.OutputData(
|
||||
permission_decision="deny",
|
||||
permission_decision_reason="Too many consecutive read calls without using symbolic tools. "
|
||||
permission_decision_reason="Too many consecutive read calls of code files without using symbolic tools. "
|
||||
"You can continue using read now if needed, the counter was reset.",
|
||||
additional_context=(
|
||||
"You were using many read file calls recently. Consider using Serena's symbolic "
|
||||
"You were using many read calls on code files recently. Consider using Serena's symbolic "
|
||||
"mcp tools instead for more targeted reads. You can continue using read now if needed, the counter was reset."
|
||||
),
|
||||
)
|
||||
|
||||
@@ -68,21 +68,21 @@ class TestPreToolUseRemindAboutSerenaHook:
|
||||
for name, expected in [("grep", True), ("grep_search", False), ("mcp_grep", False), ("read", False)]:
|
||||
with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)):
|
||||
hook = PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE)
|
||||
assert hook.is_grep_tool() == expected, f"is_grep_tool() wrong for {name} (claude-code)"
|
||||
assert hook.is_grep_call() == expected, f"is_grep_tool() wrong for {name} (claude-code)"
|
||||
|
||||
def test_grep_tool_detection_non_claude_code(self, tmp_path: Path):
|
||||
"""Non-Claude-Code clients fall back to substring matching to cover verbose tool names."""
|
||||
for name, expected in [("grep_search", True), ("mcp_grep", True), ("read_file", False), ("serena_find", False)]:
|
||||
with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)):
|
||||
hook = PreToolUseRemindAboutSerenaHook(HookClient.VSCODE)
|
||||
assert hook.is_grep_tool() == expected, f"is_grep_tool() wrong for {name} (vscode)"
|
||||
assert hook.is_grep_call() == expected, f"is_grep_tool() wrong for {name} (vscode)"
|
||||
|
||||
def test_read_file_tool_detection_claude_code(self, tmp_path: Path):
|
||||
"""Claude Code uses the exact tool name ``Read`` (lowercased to ``read``)."""
|
||||
for name, expected in [("read", True), ("read_file", False), ("readFile", False), ("grep", False)]:
|
||||
with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)):
|
||||
hook = PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE)
|
||||
assert hook.is_read_file_tool() == expected, f"is_read_file_tool() wrong for {name} (claude-code)"
|
||||
assert hook.is_read_code_file_call() == expected, f"is_read_file_tool() wrong for {name} (claude-code)"
|
||||
|
||||
def test_read_file_tool_detection_non_claude_code(self, tmp_path: Path):
|
||||
"""Non-Claude-Code clients accept any read-style verb (``read``/``view``/``open``/``show``) combined with ``file``."""
|
||||
@@ -103,7 +103,7 @@ class TestPreToolUseRemindAboutSerenaHook:
|
||||
for name, expected in cases:
|
||||
with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)):
|
||||
hook = PreToolUseRemindAboutSerenaHook(HookClient.VSCODE)
|
||||
assert hook.is_read_file_tool() == expected, f"is_read_file_tool() wrong for {name} (vscode)"
|
||||
assert hook.is_read_code_file_call() == expected, f"is_read_file_tool() wrong for {name} (vscode)"
|
||||
|
||||
def test_serena_tool_detection(self, tmp_path: Path):
|
||||
for name, expected in [("mcp_serena_find_symbol", True), ("serena_overview", True), ("grep_search", False)]:
|
||||
|
||||
Reference in new issue
Block a user