diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a7fdc40..ddb0ceaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/src/serena/hooks.py b/src/serena/hooks.py index 971a2fec..659fae63 100644 --- a/src/serena/hooks.py +++ b/src/serena/hooks.py @@ -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." ), ) diff --git a/test/serena/test_hooks.py b/test/serena/test_hooks.py index 25408f0d..2bef04f0 100644 --- a/test/serena/test_hooks.py +++ b/test/serena/test_hooks.py @@ -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)]: