From 84e117c5c2b781785e91a3436575eef10b6418b4 Mon Sep 17 00:00:00 2001 From: octo-patch Date: Tue, 28 Apr 2026 12:26:37 +0800 Subject: [PATCH 1/8] fix(hooks): detect grep/read shell commands in Codex exec_command (fixes #1394) When the Codex client sends shell work via `exec_command`, the tool name is always `exec_command` and the actual command is under `tool_input.cmd`. The previous `is_grep_tool` / `is_read_file_tool` checks inspected only the tool name and therefore never fired for Codex grep/read workflows. Add `_GREP_SHELL_COMMANDS` and `_READ_SHELL_COMMANDS` frozensets and a helper `_get_exec_command_name()` that extracts the basename of the first token in `cmd`. Both detection methods now use these to classify `exec_command` calls when the client is `HookClient.CODEX`, leaving all other client paths unchanged. Co-Authored-By: Octopus --- CHANGELOG.md | 3 +++ src/serena/hooks.py | 32 +++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6747ece8..5a7fdc40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Status of the `main` branch. Changes prior to the next official version change will appear here. +* 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. + # v1.2.0 (2026-04-27) * General: diff --git a/src/serena/hooks.py b/src/serena/hooks.py index a9322193..971a2fec 100644 --- a/src/serena/hooks.py +++ b/src/serena/hooks.py @@ -241,20 +241,50 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): #: ingest. Conservative on purpose: only verbs that strongly imply *reading* #: a file, never *modifying* one — so ``view_file``/``open_file``/``show_file`` #: are caught alongside ``read_file``, while ``write_file``/``edit_file`` are not. - _READ_FILE_VERB_SUBSTRINGS: tuple[str, ...] = ("read", "view", "open", "show") + _READ_FILE_VERB_SUBSTRINGS: frozenset[str] = frozenset(("read", "view", "open", "show")) + + #: Shell commands that perform grep-like search; used to classify Codex + #: ``exec_command`` calls whose ``cmd`` field starts with one of these. + _GREP_SHELL_COMMANDS: frozenset[str] = frozenset(("grep", "rg", "ag", "ack", "fgrep", "egrep")) + + #: Shell commands that perform file-read operations; used to classify Codex + #: ``exec_command`` calls whose ``cmd`` field starts with one of these. + _READ_SHELL_COMMANDS: frozenset[str] = frozenset(("cat", "head", "tail", "sed", "less", "more", "bat")) def __init__(self, client: HookClient): 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. + + 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 is_grep_tool(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 + # heuristic for other clients return "grep" in self._tool_name def is_read_file_tool(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 + # heuristic for other clients name = self._tool_name if "file" not in name: return False From d7852341f7f92747db8c95b78fdba3866a673b99 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Thu, 30 Apr 2026 13:20:40 +0200 Subject: [PATCH 2/8] Refactoring: Make FilenameMatcher accept file extensions instead of patterns Makes the matching both simpler and faster --- .../adding_new_language_support_guide.md | 2 +- src/solidlsp/ls_config.py | 149 +++++++++--------- 2 files changed, 78 insertions(+), 73 deletions(-) diff --git a/.serena/memories/adding_new_language_support_guide.md b/.serena/memories/adding_new_language_support_guide.md index 8ffa4684..bafa7526 100644 --- a/.serena/memories/adding_new_language_support_guide.md +++ b/.serena/memories/adding_new_language_support_guide.md @@ -97,7 +97,7 @@ class Language(str, Enum): match self: # Existing cases... case self.NEW_LANGUAGE: - return FilenameMatcher("*.newlang", "*.nl") # File extensions + return FilenameMatcher(".newlang", ".nl") # File extensions ``` ### 2.2 Update Language Server Factory diff --git a/src/solidlsp/ls_config.py b/src/solidlsp/ls_config.py index eb527dc3..030a6f71 100644 --- a/src/solidlsp/ls_config.py +++ b/src/solidlsp/ls_config.py @@ -2,7 +2,6 @@ Configuration objects for language servers """ -import fnmatch from collections.abc import Iterable from dataclasses import dataclass, field from enum import Enum @@ -13,15 +12,21 @@ if TYPE_CHECKING: class FilenameMatcher: - def __init__(self, *patterns: str) -> None: + def __init__(self, *file_extensions: str) -> None: """ - :param patterns: fnmatch-compatible patterns + :param file_extensions: file extensions, e.g., `.py, .yml` """ - self.patterns = patterns + self._file_extensions = file_extensions def is_relevant_filename(self, fn: str) -> bool: - for pattern in self.patterns: - if fnmatch.fnmatch(fn, pattern): + for ext in self._file_extensions: + if fn.endswith(ext): + return True + return False + + def string_contains_relevant_filename(self, string: str) -> bool: + for ext in self._file_extensions: + if ext in string: return True return False @@ -151,7 +156,7 @@ class Language(str, Enum): """ ANSIBLE = "ansible" """Ansible language server (experimental) using @ansible/ansible-language-server. - Supports *.yaml and *.yml files (same extensions as YAML, hence experimental). + Supports .yaml and .yml files (same extensions as YAML, hence experimental). Must be explicitly specified in project.yml. Requires Node.js and npm. Requires ``ansible`` in PATH for full functionality. """ @@ -210,134 +215,134 @@ class Language(str, Enum): def get_source_fn_matcher(self) -> FilenameMatcher: match self: case self.PYTHON | self.PYTHON_JEDI | self.PYTHON_TY: - return FilenameMatcher("*.py", "*.pyi") + return FilenameMatcher(".py", ".pyi") case self.JAVA: - return FilenameMatcher("*.java") + return FilenameMatcher(".java") case self.TYPESCRIPT | self.TYPESCRIPT_VTS: # see https://github.com/oraios/serena/issues/204 path_patterns = [] for prefix in ["c", "m", ""]: for postfix in ["x", ""]: for base_pattern in ["ts", "js"]: - path_patterns.append(f"*.{prefix}{base_pattern}{postfix}") + path_patterns.append(f".{prefix}{base_pattern}{postfix}") return FilenameMatcher(*path_patterns) case self.CSHARP | self.CSHARP_OMNISHARP: - return FilenameMatcher("*.cs") + return FilenameMatcher(".cs") case self.RUST: - return FilenameMatcher("*.rs") + return FilenameMatcher(".rs") case self.GO: - return FilenameMatcher("*.go") + return FilenameMatcher(".go") case self.RUBY: - return FilenameMatcher("*.rb", "*.erb") + return FilenameMatcher(".rb", ".erb") case self.RUBY_SOLARGRAPH: - return FilenameMatcher("*.rb") + return FilenameMatcher(".rb") case self.CPP | self.CPP_CCLS: - return FilenameMatcher("*.cpp", "*.h", "*.hpp", "*.c", "*.hxx", "*.cc", "*.cxx") + return FilenameMatcher(".cpp", ".h", ".hpp", ".c", ".hxx", ".cc", ".cxx") case self.KOTLIN: - return FilenameMatcher("*.kt", "*.kts") + return FilenameMatcher(".kt", ".kts") case self.DART: - return FilenameMatcher("*.dart") + return FilenameMatcher(".dart") case self.PHP | self.PHP_PHPACTOR: - return FilenameMatcher("*.php") + return FilenameMatcher(".php") case self.R: - return FilenameMatcher("*.R", "*.r", "*.Rmd", "*.Rnw") + return FilenameMatcher(".R", ".r", ".Rmd", ".Rnw") case self.PERL: - return FilenameMatcher("*.pl", "*.pm", "*.t") + return FilenameMatcher(".pl", ".pm", ".t") case self.CLOJURE: - return FilenameMatcher("*.clj", "*.cljs", "*.cljc", "*.edn") # codespell:ignore edn + return FilenameMatcher(".clj", ".cljs", ".cljc", ".edn") # codespell:ignore edn case self.ELIXIR: - return FilenameMatcher("*.ex", "*.exs") + return FilenameMatcher(".ex", ".exs") case self.ELM: - return FilenameMatcher("*.elm") + return FilenameMatcher(".elm") case self.TERRAFORM: - return FilenameMatcher("*.tf", "*.tfvars", "*.tfstate") + return FilenameMatcher(".tf", ".tfvars", ".tfstate") case self.SWIFT: - return FilenameMatcher("*.swift") + return FilenameMatcher(".swift") case self.BASH: - return FilenameMatcher("*.sh", "*.bash") + return FilenameMatcher(".sh", ".bash") case self.CRYSTAL: - return FilenameMatcher("*.cr") + return FilenameMatcher(".cr") case self.YAML: - return FilenameMatcher("*.yaml", "*.yml") + return FilenameMatcher(".yaml", ".yml") case self.JSON: - return FilenameMatcher("*.json", "*.jsonc") + return FilenameMatcher(".json", ".jsonc") case self.TOML: - return FilenameMatcher("*.toml") + return FilenameMatcher(".toml") case self.ZIG: - return FilenameMatcher("*.zig", "*.zon") + return FilenameMatcher(".zig", ".zon") case self.LUA: - return FilenameMatcher("*.lua") + return FilenameMatcher(".lua") case self.LUAU: - return FilenameMatcher("*.luau") + return FilenameMatcher(".luau") case self.NIX: - return FilenameMatcher("*.nix") + return FilenameMatcher(".nix") case self.ERLANG: - return FilenameMatcher("*.erl", "*.hrl", "*.escript", "*.config", "*.app", "*.app.src") + return FilenameMatcher(".erl", ".hrl", ".escript", ".config", ".app", ".app.src") case self.OCAML: - return FilenameMatcher("*.ml", "*.mli", "*.re", "*.rei") + return FilenameMatcher(".ml", ".mli", ".re", ".rei") case self.AL: - return FilenameMatcher("*.al", "*.dal") + return FilenameMatcher(".al", ".dal") case self.FSHARP: - return FilenameMatcher("*.fs", "*.fsx", "*.fsi") + return FilenameMatcher(".fs", ".fsx", ".fsi") case self.REGO: - return FilenameMatcher("*.rego") + return FilenameMatcher(".rego") case self.MARKDOWN: - return FilenameMatcher("*.md", "*.markdown") + return FilenameMatcher(".md", ".markdown") case self.SCALA: - return FilenameMatcher("*.scala", "*.sbt") + return FilenameMatcher(".scala", ".sbt") case self.JULIA: - return FilenameMatcher("*.jl") + return FilenameMatcher(".jl") case self.FORTRAN: return FilenameMatcher( - "*.f90", "*.F90", "*.f95", "*.F95", "*.f03", "*.F03", "*.f08", "*.F08", "*.f", "*.F", "*.for", "*.FOR", "*.fpp", "*.FPP" + ".f90", ".F90", ".f95", ".F95", ".f03", ".F03", ".f08", ".F08", ".f", ".F", ".for", ".FOR", ".fpp", ".FPP" ) case self.HASKELL: - return FilenameMatcher("*.hs", "*.lhs") + return FilenameMatcher(".hs", ".lhs") case self.HAXE: - return FilenameMatcher("*.hx") + return FilenameMatcher(".hx") case self.LEAN4: - return FilenameMatcher("*.lean") + return FilenameMatcher(".lean") case self.VUE: - path_patterns = ["*.vue"] + path_patterns = [".vue"] for prefix in ["c", "m", ""]: for postfix in ["x", ""]: for base_pattern in ["ts", "js"]: - path_patterns.append(f"*.{prefix}{base_pattern}{postfix}") + path_patterns.append(f".{prefix}{base_pattern}{postfix}") return FilenameMatcher(*path_patterns) case self.POWERSHELL: - return FilenameMatcher("*.ps1", "*.psm1", "*.psd1") + return FilenameMatcher(".ps1", ".psm1", ".psd1") case self.PASCAL: - return FilenameMatcher("*.pas", "*.pp", "*.lpr", "*.dpr", "*.dpk", "*.inc") + return FilenameMatcher(".pas", ".pp", ".lpr", ".dpr", ".dpk", ".inc") case self.GROOVY: - return FilenameMatcher("*.groovy", "*.gvy") + return FilenameMatcher(".groovy", ".gvy") case self.MATLAB: - return FilenameMatcher("*.m", "*.mlx", "*.mlapp") + return FilenameMatcher(".m", ".mlx", ".mlapp") case self.HLSL: return FilenameMatcher( - "*.hlsl", - "*.hlsli", - "*.fx", - "*.fxh", - "*.cginc", - "*.compute", - "*.shader", - "*.glsl", - "*.vert", - "*.frag", - "*.geom", - "*.tesc", - "*.tese", - "*.comp", - "*.wgsl", + ".hlsl", + ".hlsli", + ".fx", + ".fxh", + ".cginc", + ".compute", + ".shader", + ".glsl", + ".vert", + ".frag", + ".geom", + ".tesc", + ".tese", + ".comp", + ".wgsl", ) case self.SYSTEMVERILOG: - return FilenameMatcher("*.sv", "*.svh", "*.v", "*.vh") + return FilenameMatcher(".sv", ".svh", ".v", ".vh") case self.SOLIDITY: - return FilenameMatcher("*.sol") + return FilenameMatcher(".sol") case self.ANSIBLE: - return FilenameMatcher("*.yaml", "*.yml") + return FilenameMatcher(".yaml", ".yml") case self.MSL: - return FilenameMatcher("*.mrc") + return FilenameMatcher(".mrc") case _: raise ValueError(f"Unhandled language: {self}") From 7df64bcee043ef19c13facc05a2532e70bad10cc Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Thu, 30 Apr 2026 13:22:05 +0200 Subject: [PATCH 3/8] Extended Language to distinguish between programming and non-programming languages --- src/solidlsp/ls_config.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/solidlsp/ls_config.py b/src/solidlsp/ls_config.py index 030a6f71..e50db397 100644 --- a/src/solidlsp/ls_config.py +++ b/src/solidlsp/ls_config.py @@ -162,10 +162,11 @@ class Language(str, Enum): """ @classmethod - def iter_all(cls, include_experimental: bool = False) -> Iterable[Self]: + def iter_all(cls, include_experimental: bool = False, include_non_programming_languages: bool = True) -> Iterable[Self]: for lang in cls: if include_experimental or not lang.is_experimental(): - yield lang + if include_non_programming_languages or lang.is_programming_language(): + yield lang def is_experimental(self) -> bool: """ @@ -192,6 +193,12 @@ class Language(str, Enum): self.SOLIDITY, } + def is_programming_language(self) -> bool: + """Whether the supported language should be considered a programming language. + Solidlsp supports languages like markdown or json, this method returns False for them. + """ + return self not in frozenset((self.MARKDOWN, self.JSON, self.TOML, self.YAML, self.ANSIBLE)) + def __str__(self) -> str: return self.value From 66d223c9e1e4fc31430cdb2e95bf8835ef227c9f Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Thu, 30 Apr 2026 13:30:37 +0200 Subject: [PATCH 4/8] 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 --- CHANGELOG.md | 1 + src/serena/hooks.py | 73 +++++++++++++++++++++++---------------- test/serena/test_hooks.py | 8 ++--- 3 files changed, 49 insertions(+), 33 deletions(-) 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)]: From 8b858d1cab8263f3123529d0c5df5f01b5ddebcc Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 4 May 2026 15:08:59 +0200 Subject: [PATCH 5/8] FilenameMatcher: fix in new method string_contains_relevant_filename --- src/solidlsp/ls_config.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/solidlsp/ls_config.py b/src/solidlsp/ls_config.py index be6fd7e4..f04f4817 100644 --- a/src/solidlsp/ls_config.py +++ b/src/solidlsp/ls_config.py @@ -2,6 +2,7 @@ Configuration objects for language servers """ +import re from collections.abc import Iterable from dataclasses import dataclass, field from enum import Enum @@ -25,8 +26,12 @@ class FilenameMatcher: return False def string_contains_relevant_filename(self, string: str) -> bool: + """:return: whether ``string`` contains an occurrence of any registered extension as + a *complete* extension — i.e. the extension must either end the string or be followed + by a non-extension-character (anything other than a letter, digit, or underscore). + """ for ext in self._file_extensions: - if ext in string: + if re.search(rf"{re.escape(ext)}(?:\W|$)", string): return True return False From 4b0865c33b9e78a24c246a6ccb97147cff4ede42 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 4 May 2026 15:09:36 +0200 Subject: [PATCH 6/8] Hooks: improve heuristics for code file detection --- src/serena/hooks.py | 33 ++++++++----- test/serena/test_hooks.py | 98 ++++++++++++++++++++++++++++++++++----- 2 files changed, 107 insertions(+), 24 deletions(-) diff --git a/src/serena/hooks.py b/src/serena/hooks.py index 63d14c28..02f022e9 100644 --- a/src/serena/hooks.py +++ b/src/serena/hooks.py @@ -256,15 +256,16 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): super().__init__(client) self._tool_call_counter = self.ToolUseCounter.load(self) - # 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. + # extract a shell ``cmd``/``command`` field (Codex ``exec_command``-style payloads): + # split into command name (basename, lowercased) and the remaining argument string + # so both bare names (``rg``) and path-prefixed invocations (``/usr/bin/grep``) are + # normalised the same way. Stays ``None`` when no shell command is present. self._command: str | None = None self._command_name: str | None = None self._command_args_str: str | None = None + # extract a direct file-path field (Claude Code's ``Read``/``Edit``/``Write`` tool + # payloads pass the target as ``file_path`` rather than via a shell command). + self._file_path: 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: @@ -272,15 +273,23 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): if len(cmd_split) > 1: self._command_args_str = cmd_split[1] self._command_name = os.path.basename(cmd_split[0]).lower() + file_path = self._tool_input.get("file_path") or self._tool_input.get("filePath") or "" + self._file_path = str(file_path).strip() or None - 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: + def _call_targets_code_file(self) -> bool: + """:return: whether the call's payload references a source-code file. Heuristic that + scans every payload field that may hold a path — the shell-command argument string + (e.g. Codex ``exec_command``) and the direct ``file_path`` field (Claude Code's + ``Read``) — for a substring matching any supported language's source extensions. + """ + candidates = [s for s in (self._command_args_str, self._file_path) if s] + if not candidates: 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 + for candidate in candidates: + if file_matcher.string_contains_relevant_filename(candidate): + return True return False def is_grep_call(self) -> bool: @@ -303,7 +312,7 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): 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() + return self.is_read_call() and self._call_targets_code_file() def execute(self) -> None: # gate the entire hook on the rate-limit window: while we are within diff --git a/test/serena/test_hooks.py b/test/serena/test_hooks.py index 50fcec77..25c4ece5 100644 --- a/test/serena/test_hooks.py +++ b/test/serena/test_hooks.py @@ -23,11 +23,31 @@ def _make_stdin(data: dict) -> StringIO: return StringIO(json.dumps(data)) -def _base_input(tool_name: str = "grep_search", session_id: str = "test-session-123") -> dict: +def _base_input( + tool_name: str = "grep_search", + session_id: str = "test-session-123", + tool_input: dict | None = None, +) -> dict: return { "session_id": session_id, "tool_name": tool_name, - "tool_input": {"query": "foo"}, + "tool_input": tool_input if tool_input is not None else {"query": "foo"}, + } + + +def _read_input(tool_name: str = "read", session_id: str = "test-session-123", file_path: str = "src/foo.py") -> dict: + """Build a hook payload for a read-style tool call against a code file. + + Read-style tools are gated on :meth:`PreToolUseRemindAboutSerenaHook._call_targets_code_file`, + which scans the payload for a path with a known source-code extension. Tests that exercise + the read-counter path therefore must supply such a path; we use ``file_path`` (the field + Claude Code's ``Read`` tool emits) by default, which the hook also picks up for VS Code- + shaped payloads in non-Claude-Code clients. + """ + return { + "session_id": session_id, + "tool_name": tool_name, + "tool_input": {"file_path": file_path}, } @@ -77,14 +97,22 @@ class TestPreToolUseRemindAboutSerenaHook: 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``).""" + """Claude Code uses the exact tool name ``Read`` (lowercased to ``read``); the call + only counts as a *code-file* read when the payload points at a source file. + """ 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)): + with ( + patch("sys.stdin", _make_stdin(_read_input(tool_name=name))), + patch("serena.hooks.serena_home_dir", str(tmp_path)), + ): hook = PreToolUseRemindAboutSerenaHook(HookClient.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``.""" + """Non-Claude-Code clients accept any read-style verb (``read``/``view``/``open``/``show``) + combined with ``file``; the call still only counts as a *code-file* read when the payload + points at a source file. + """ cases = [ # canonical names ("read_file", True), @@ -100,10 +128,23 @@ class TestPreToolUseRemindAboutSerenaHook: ("edit_file", False), ] 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)): + with ( + patch("sys.stdin", _make_stdin(_read_input(tool_name=name))), + patch("serena.hooks.serena_home_dir", str(tmp_path)), + ): hook = PreToolUseRemindAboutSerenaHook(HookClient.VSCODE) assert hook.is_read_code_file_call() == expected, f"is_read_file_tool() wrong for {name} (vscode)" + def test_read_non_code_file_does_not_count_as_code_read(self, tmp_path: Path): + """A ``Read`` call against a non-source file (e.g. plain text) must NOT trip the + code-file read counter, even though the tool name itself is recognised as a read. + """ + payload = _read_input(tool_name="read", file_path="notes/todo.txt") + with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE) + assert hook.is_read_call() is True + assert hook.is_read_code_file_call() is False + def test_serena_tool_detection(self, tmp_path: Path): for name, expected in [("mcp_serena_find_symbol", True), ("serena_overview", True), ("grep_search", False)]: with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)): @@ -142,16 +183,21 @@ class TestPreToolUseRemindAboutSerenaHook: assert "grep" in hook_output["additionalContext"].lower() def test_deny_output_after_threshold_reads(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): - """After reaching the read file threshold, the hook should output a deny.""" + """After reaching the read file threshold, the hook should output a deny. + + The read counter only ticks up when the payload actually targets a source-code file, + so ``_read_input`` is used to supply a plausible ``.py`` ``file_path``. + """ for _ in range(ToolUseCounter._READ_FILE_USES_THRESHOLD): - with patch("sys.stdin", _make_stdin(_base_input("read"))), patch("serena.hooks.serena_home_dir", str(tmp_path)): + with patch("sys.stdin", _make_stdin(_read_input("read"))), patch("serena.hooks.serena_home_dir", str(tmp_path)): PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE).execute() output = capsys.readouterr().out.strip() result = json.loads(output) hook_output = result["hookSpecificOutput"] assert hook_output["permissionDecision"] == "deny" - assert "read file" in hook_output["additionalContext"].lower() + assert "read" in hook_output["additionalContext"].lower() + assert "code file" in hook_output["additionalContext"].lower() def test_serena_tool_resets_counters(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): """Using a Serena tool should reset counters, so the threshold is not reached.""" @@ -297,7 +343,7 @@ class TestToolUseCounter: counter.last_read_file_use_timestamp = now - timedelta(seconds=1) counter.n_recent_read_file_uses = 1 - hook = self._make_hook_stub("read_file", now) + hook = self._make_hook_stub("read_file", now, file_path="src/foo.py") counter.update(hook) assert counter.n_recent_read_file_uses == 2 @@ -308,11 +354,24 @@ class TestToolUseCounter: counter.last_read_file_use_timestamp = now - timedelta(seconds=ToolUseCounter._READ_FILE_RESET_PERIOD_SECONDS + 1) counter.n_recent_read_file_uses = 2 - hook = self._make_hook_stub("read_file", now) + hook = self._make_hook_stub("read_file", now, file_path="src/foo.py") counter.update(hook) assert counter.n_recent_read_file_uses == 1 + def test_update_does_not_count_read_of_non_code_file(self): + """A read call whose payload points at a non-source file (e.g. ``.txt``) must + leave the read-file counter untouched — the per-tool counter is for *code* reads. + """ + counter = ToolUseCounter() + now = datetime.now() + + hook = self._make_hook_stub("read_file", now, file_path="notes/todo.txt") + counter.update(hook) + + assert counter.n_recent_read_file_uses == 0 + assert counter.last_read_file_use_timestamp is None + def test_serena_tool_resets_all_counters(self): counter = ToolUseCounter( n_recent_grep_uses=2, @@ -393,18 +452,33 @@ class TestToolUseCounter: assert counter.last_deny_timestamp == base @staticmethod - def _make_hook_stub(tool_name: str, timestamp: datetime) -> PreToolUseRemindAboutSerenaHook: + def _make_hook_stub( + tool_name: str, + timestamp: datetime, + file_path: str | None = None, + command_args_str: str | None = None, + ) -> PreToolUseRemindAboutSerenaHook: """Create a minimal stub that satisfies ToolUseCounter.update without reading stdin. Uses ``HookClient.VSCODE`` so that ``is_grep_tool`` / ``is_read_file_tool`` apply the substring-matching branch — the counter tests below feed verbose tool names like ``grep_search`` / ``read_file`` which are only recognized under the non-Claude-Code branch (Claude Code uses exact names ``grep`` / ``read``). + + :param file_path: optional payload path; when set to a source-extension path (e.g. + ``src/foo.py``), the stub will be classified as a *code-file* read by + :meth:`PreToolUseRemindAboutSerenaHook._call_targets_code_file`. + :param command_args_str: optional shell-argument tail; equivalent to ``file_path`` but + populates the ``cmd``-derived field instead of ``file_path``. """ stub = object.__new__(PreToolUseRemindAboutSerenaHook) stub._tool_name = tool_name.lower() stub._client = HookClient.VSCODE stub.triggered_at_timestamp = timestamp + stub._command = None + stub._command_name = None + stub._command_args_str = command_args_str + stub._file_path = file_path return stub From 6eb3587dcc66a848cb32b9d9c24e51d52aab7522 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Mon, 4 May 2026 16:59:37 +0200 Subject: [PATCH 7/8] Hooks: adjust output to work for codex, extended docs --- docs/02-usage/030_clients.md | 64 ++++++++++++++++- src/serena/hooks.py | 136 +++++++++++++++++++++++++++-------- test/serena/test_hooks.py | 122 +++++++++++++++++++++++-------- 3 files changed, 260 insertions(+), 62 deletions(-) diff --git a/docs/02-usage/030_clients.md b/docs/02-usage/030_clients.md index cbfe0d4e..a9378826 100644 --- a/docs/02-usage/030_clients.md +++ b/docs/02-usage/030_clients.md @@ -318,9 +318,67 @@ The Codex app does not start a session in the project's directory, so when using asking Codex to "Activate the current dir as project using serena" at the start of each session (though Codex might do this automatically). -**Hooks.** The Codex hook system is less refined than of VSCode or Claude Code, but Codex also displays less drift -and is better in using Serena's tools out of the box. If you use the Codex app, you can consider setting up the -SessionStart hook (see above) to remind the agent to activate the project directory. +**Hooks.** +Codex supports lifecycle hooks; see the +[Codex hooks documentation](https://developers.openai.com/codex/hooks) for details. To enable +Serena's hooks for Codex, add this feature flag to `~/.codex/config.toml`: + +```toml +[features] +codex_hooks = true +``` + +Then create `~/.codex/hooks.json` with the following content: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "serena-hooks remind --client=codex" + } + ] + } + ], + "SessionStart": [ + { + "matcher": "startup|resume", + "hooks": [ + { + "type": "command", + "command": "serena-hooks activate --client=codex" + } + ] + } + ], + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "serena-hooks cleanup --client=codex" + } + ] + } + ] + } +} +``` + +The hooks will: + +- **`activate`**: Prompt the agent to activate the current project and read Serena's instructions + when a Codex session starts or resumes. +- **`remind`**: Nudge the agent to use Serena's symbolic tools when it makes too many consecutive + code-search or code-file-read calls without using Serena tools in between. +- **`cleanup`**: Clean up hook session data when the session ends. + +The `PreToolUse` matcher is intentionally restricted to `Bash`. The Serena reminder hook for Codex +tracks shell-based grep and code-file reads, so running it for every tool call is unnecessary. ## Claude Desktop diff --git a/src/serena/hooks.py b/src/serena/hooks.py index 02f022e9..153f13b3 100644 --- a/src/serena/hooks.py +++ b/src/serena/hooks.py @@ -13,7 +13,6 @@ 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") @@ -67,15 +66,16 @@ class PreToolUseHook(Hook, ABC): permission_decision_reason: str additional_context: str = "" - def to_json_string(self) -> str: + def to_json_string(self, client: HookClient) -> str: hook_output = { "hookSpecificOutput": { "hookEventName": "PreToolUse", "permissionDecision": self.permission_decision, "permissionDecisionReason": self.permission_decision_reason, - "additionalContext": self.additional_context, } } + if client != HookClient.CODEX: + hook_output["hookSpecificOutput"]["additionalContext"] = self.additional_context return json.dumps(hook_output) def is_serena_tool(self) -> bool: @@ -245,18 +245,70 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): _READ_FILE_VERB_SUBSTRINGS: frozenset[str] = frozenset(("read", "view", "open", "show")) #: Shell commands that perform grep-like search; used to classify Codex - #: ``exec_command`` calls whose ``cmd`` field starts with one of these. + #: shell-command tool calls whose ``cmd`` or ``command`` field starts with one of these. _GREP_SHELL_COMMANDS: frozenset[str] = frozenset(("grep", "rg", "ag", "ack", "fgrep", "egrep")) #: Shell commands that perform file-read operations; used to classify Codex - #: ``exec_command`` calls whose ``cmd`` field starts with one of these. - _READ_SHELL_COMMANDS: frozenset[str] = frozenset(("cat", "head", "tail", "sed", "less", "more", "bat")) + #: shell-command tool calls whose ``cmd`` or ``command`` field starts with one of these. + _READ_SHELL_COMMANDS: frozenset[str] = frozenset(("cat", "head", "tail", "sed", "less", "more", "bat", "get-content", "gc")) + + #: file suffixes for source-like files where symbolic tools are usually more + #: appropriate than repeated raw reads. Lowercase and extension-only. + _CODE_FILE_EXTENSIONS: frozenset[str] = frozenset( + ( + ".al", + ".bash", + ".c", + ".clj", + ".cljs", + ".cpp", + ".cs", + ".css", + ".dart", + ".elm", + ".ex", + ".exs", + ".fs", + ".fsx", + ".go", + ".groovy", + ".h", + ".hpp", + ".hs", + ".html", + ".java", + ".jl", + ".js", + ".jsx", + ".kt", + ".kts", + ".lean", + ".lua", + ".m", + ".matlab", + ".php", + ".ps1", + ".py", + ".r", + ".rb", + ".rs", + ".scala", + ".sh", + ".sol", + ".svelte", + ".swift", + ".ts", + ".tsx", + ".vue", + ".zig", + ) + ) def __init__(self, client: HookClient): super().__init__(client) self._tool_call_counter = self.ToolUseCounter.load(self) - # extract a shell ``cmd``/``command`` field (Codex ``exec_command``-style payloads): + # extract a shell ``cmd``/``command`` field (Codex shell-command-style payloads): # split into command name (basename, lowercased) and the remaining argument string # so both bare names (``rg``) and path-prefixed invocations (``/usr/bin/grep``) are # normalised the same way. Stays ``None`` when no shell command is present. @@ -276,26 +328,10 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): file_path = self._tool_input.get("file_path") or self._tool_input.get("filePath") or "" self._file_path = str(file_path).strip() or None - def _call_targets_code_file(self) -> bool: - """:return: whether the call's payload references a source-code file. Heuristic that - scans every payload field that may hold a path — the shell-command argument string - (e.g. Codex ``exec_command``) and the direct ``file_path`` field (Claude Code's - ``Read``) — for a substring matching any supported language's source extensions. - """ - candidates = [s for s in (self._command_args_str, self._file_path) if s] - if not candidates: - return False - for lang in Language.iter_all(include_non_programming_languages=False): - file_matcher = lang.get_source_fn_matcher() - for candidate in candidates: - if file_matcher.string_contains_relevant_filename(candidate): - return True - return False - 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": + if self._client == HookClient.CODEX and self._is_shell_command_call(): return self._command_name in self._GREP_SHELL_COMMANDS # heuristic for other clients return "grep" in self._tool_name @@ -303,7 +339,7 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): 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": + if self._client == HookClient.CODEX and self._is_shell_command_call(): return self._command_name in self._READ_SHELL_COMMANDS # heuristic for other clients name = self._tool_name @@ -311,8 +347,48 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): return False return any(verb in name for verb in self._READ_FILE_VERB_SUBSTRINGS) + def _is_shell_command_call(self) -> bool: + """:return: whether the hook payload represents a shell-command tool call.""" + return self._command_name is not None + + def is_read_file_call(self) -> bool: + """:return: whether the tool call reads a file-like target.""" + return self.is_read_call() + def is_read_code_file_call(self) -> bool: - return self.is_read_call() and self._call_targets_code_file() + """:return: whether the tool call reads a source-like file target.""" + if not self.is_read_file_call(): + return False + + if self._file_path is not None: + return self._is_code_file_path(self._file_path) + + if self._client == HookClient.CODEX and self._command_args_str is not None: + return any(self._is_code_file_path(argument) for argument in self._iter_shell_path_arguments()) + + return True + + def _iter_shell_path_arguments(self) -> list[str]: + """:return: path-like shell arguments with quoting and common options removed.""" + arguments: list[str] = [] + if self._command_args_str is None: + return arguments + + for raw_argument in self._command_args_str.split(): + argument = raw_argument.strip().strip("'\"") + if not argument or argument.startswith("-"): + continue + arguments.append(argument) + + return arguments + + @classmethod + def _is_code_file_path(cls, file_path: str) -> bool: + """:return: whether ``file_path`` has a source-like suffix.""" + cleaned_path = file_path.strip().strip("'\"") + if not cleaned_path: + return False + return Path(cleaned_path).suffix.lower() in cls._CODE_FILE_EXTENSIONS def execute(self) -> None: # gate the entire hook on the rate-limit window: while we are within @@ -351,7 +427,7 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): # ensures we never reach this branch within the rate-limit window. self._tool_call_counter.reset() self._tool_call_counter.last_deny_timestamp = self.triggered_at_timestamp - click.echo(output_data.to_json_string()) + click.echo(output_data.to_json_string(self._client)) self._tool_call_counter.save(self) def _build_grep_deny(self) -> "PreToolUseHook.OutputData": @@ -368,10 +444,10 @@ class PreToolUseRemindAboutSerenaHook(PreToolUseHook): def _build_code_read_deny(self) -> "PreToolUseHook.OutputData": return self.OutputData( permission_decision="deny", - permission_decision_reason="Too many consecutive read calls of code files without using symbolic tools. " + permission_decision_reason="Too many consecutive read calls of files without using symbolic tools. " "You can continue using read now if needed, the counter was reset.", additional_context=( - "You were using many read calls on code files recently. Consider using Serena's symbolic " + "You were using many read calls on 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." ), ) @@ -449,7 +525,7 @@ class PreToolUseAutoApproveSerenaHook(PreToolUseHook): permission_decision="allow", permission_decision_reason=f"Auto-approved: Serena tool call while client is in {self._permission_mode} mode.", ) - click.echo(output_data.to_json_string()) + click.echo(output_data.to_json_string(self._client)) _client_option = click.option( diff --git a/test/serena/test_hooks.py b/test/serena/test_hooks.py index 25c4ece5..3bed4ec4 100644 --- a/test/serena/test_hooks.py +++ b/test/serena/test_hooks.py @@ -36,14 +36,7 @@ def _base_input( def _read_input(tool_name: str = "read", session_id: str = "test-session-123", file_path: str = "src/foo.py") -> dict: - """Build a hook payload for a read-style tool call against a code file. - - Read-style tools are gated on :meth:`PreToolUseRemindAboutSerenaHook._call_targets_code_file`, - which scans the payload for a path with a known source-code extension. Tests that exercise - the read-counter path therefore must supply such a path; we use ``file_path`` (the field - Claude Code's ``Read`` tool emits) by default, which the hook also picks up for VS Code- - shaped payloads in non-Claude-Code clients. - """ + """Build a hook payload for a read-style tool call against a file.""" return { "session_id": session_id, "tool_name": tool_name, @@ -96,22 +89,34 @@ class TestPreToolUseRemindAboutSerenaHook: hook = PreToolUseRemindAboutSerenaHook(HookClient.VSCODE) assert hook.is_grep_call() == expected, f"is_grep_tool() wrong for {name} (vscode)" + def test_grep_tool_detection_codex_shell_commands(self, tmp_path: Path): + """Codex shell-command tools are classified by the command embedded in their payload.""" + cases = [ + ("exec_command", {"cmd": "rg -n foo README.md"}, True), + ("functions.shell_command", {"command": "rg -n foo README.md"}, True), + ("functions.shell_command", {"command": "Get-Content README.md"}, False), + ] + for tool_name, tool_input, expected in cases: + with ( + patch("sys.stdin", _make_stdin(_base_input(tool_name=tool_name, tool_input=tool_input))), + patch("serena.hooks.serena_home_dir", str(tmp_path)), + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_call() == expected, f"is_grep_tool() wrong for {tool_name} / {tool_input}" + def test_read_file_tool_detection_claude_code(self, tmp_path: Path): - """Claude Code uses the exact tool name ``Read`` (lowercased to ``read``); the call - only counts as a *code-file* read when the payload points at a source file. - """ + """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(_read_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)), ): hook = PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE) - assert hook.is_read_code_file_call() == expected, f"is_read_file_tool() wrong for {name} (claude-code)" + assert hook.is_read_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``; the call still only counts as a *code-file* read when the payload - points at a source file. + combined with ``file``. """ cases = [ # canonical names @@ -133,18 +138,45 @@ class TestPreToolUseRemindAboutSerenaHook: patch("serena.hooks.serena_home_dir", str(tmp_path)), ): hook = PreToolUseRemindAboutSerenaHook(HookClient.VSCODE) - assert hook.is_read_code_file_call() == expected, f"is_read_file_tool() wrong for {name} (vscode)" + assert hook.is_read_file_call() == expected, f"is_read_file_tool() wrong for {name} (vscode)" - def test_read_non_code_file_does_not_count_as_code_read(self, tmp_path: Path): - """A ``Read`` call against a non-source file (e.g. plain text) must NOT trip the - code-file read counter, even though the tool name itself is recognised as a read. - """ + def test_read_non_code_file_counts_as_file_read(self, tmp_path: Path): + """A ``Read`` call against a non-source file still counts as a file read.""" payload = _read_input(tool_name="read", file_path="notes/todo.txt") with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): hook = PreToolUseRemindAboutSerenaHook(HookClient.CLAUDE_CODE) assert hook.is_read_call() is True + assert hook.is_read_file_call() is True assert hook.is_read_code_file_call() is False + def test_read_code_file_detection(self, tmp_path: Path): + """Only source-like read targets count for the code-read reminder.""" + cases = [ + (HookClient.CLAUDE_CODE, _read_input(tool_name="read", file_path="README.md"), False), + (HookClient.CLAUDE_CODE, _read_input(tool_name="read", file_path="src/foo.py"), True), + (HookClient.CODEX, _base_input("functions.shell_command", tool_input={"command": "Get-Content README.md"}), False), + (HookClient.CODEX, _base_input("functions.shell_command", tool_input={"command": "Get-Content src/foo.py"}), True), + ] + for client, payload, expected in cases: + with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): + hook = PreToolUseRemindAboutSerenaHook(client) + assert hook.is_read_code_file_call() == expected, f"is_read_code_file_call() wrong for {client} / {payload}" + + def test_read_file_tool_detection_codex_shell_commands(self, tmp_path: Path): + """Codex shell-command tools count PowerShell file reads, including non-code files.""" + cases = [ + ("exec_command", {"cmd": "cat README.md"}, True), + ("functions.shell_command", {"command": "Get-Content README.md"}, True), + ("functions.shell_command", {"command": "rg -n foo README.md"}, False), + ] + for tool_name, tool_input, expected in cases: + with ( + patch("sys.stdin", _make_stdin(_base_input(tool_name=tool_name, tool_input=tool_input))), + patch("serena.hooks.serena_home_dir", str(tmp_path)), + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_read_file_call() == expected, f"is_read_file_tool() wrong for {tool_name} / {tool_input}" + def test_serena_tool_detection(self, tmp_path: Path): for name, expected in [("mcp_serena_find_symbol", True), ("serena_overview", True), ("grep_search", False)]: with patch("sys.stdin", _make_stdin(_base_input(tool_name=name))), patch("serena.hooks.serena_home_dir", str(tmp_path)): @@ -182,11 +214,24 @@ class TestPreToolUseRemindAboutSerenaHook: assert hook_output["permissionDecision"] == "deny" assert "grep" in hook_output["additionalContext"].lower() + def test_deny_output_after_threshold_greps_codex_shell_command(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): + """After reaching the grep threshold, Codex ``functions.shell_command`` emits a deny.""" + payload = _base_input("functions.shell_command", tool_input={"command": "rg -n foo README.md"}) + for _ in range(ToolUseCounter._GREP_USES_THRESHOLD): + with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): + PreToolUseRemindAboutSerenaHook(HookClient.CODEX).execute() + + output = capsys.readouterr().out.strip() + result = json.loads(output) + hook_output = result["hookSpecificOutput"] + assert hook_output["permissionDecision"] == "deny" + assert "additionalContext" not in hook_output + assert "grep" in hook_output["permissionDecisionReason"].lower() + def test_deny_output_after_threshold_reads(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): """After reaching the read file threshold, the hook should output a deny. - The read counter only ticks up when the payload actually targets a source-code file, - so ``_read_input`` is used to supply a plausible ``.py`` ``file_path``. + ``_read_input`` supplies the direct ``file_path`` field emitted by file-read tools. """ for _ in range(ToolUseCounter._READ_FILE_USES_THRESHOLD): with patch("sys.stdin", _make_stdin(_read_input("read"))), patch("serena.hooks.serena_home_dir", str(tmp_path)): @@ -197,7 +242,29 @@ class TestPreToolUseRemindAboutSerenaHook: hook_output = result["hookSpecificOutput"] assert hook_output["permissionDecision"] == "deny" assert "read" in hook_output["additionalContext"].lower() - assert "code file" in hook_output["additionalContext"].lower() + assert "files" in hook_output["additionalContext"].lower() + + def test_no_deny_after_threshold_get_content_markdown_codex(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): + """PowerShell ``Get-Content`` calls on markdown files do not count for the code-read reminder.""" + payload = _base_input("functions.shell_command", tool_input={"command": "Get-Content README.md"}) + for _ in range(ToolUseCounter._READ_FILE_USES_THRESHOLD): + with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): + PreToolUseRemindAboutSerenaHook(HookClient.CODEX).execute() + + assert capsys.readouterr().out == "" + + def test_deny_output_after_threshold_get_content_code_file_codex(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): + """PowerShell ``Get-Content`` calls on source files count for the code-read reminder.""" + payload = _base_input("functions.shell_command", tool_input={"command": "Get-Content src/foo.py"}) + for _ in range(ToolUseCounter._READ_FILE_USES_THRESHOLD): + with patch("sys.stdin", _make_stdin(payload)), patch("serena.hooks.serena_home_dir", str(tmp_path)): + PreToolUseRemindAboutSerenaHook(HookClient.CODEX).execute() + + result = json.loads(capsys.readouterr().out.strip()) + hook_output = result["hookSpecificOutput"] + assert hook_output["permissionDecision"] == "deny" + assert "additionalContext" not in hook_output + assert "read" in hook_output["permissionDecisionReason"].lower() def test_serena_tool_resets_counters(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): """Using a Serena tool should reset counters, so the threshold is not reached.""" @@ -359,10 +426,8 @@ class TestToolUseCounter: assert counter.n_recent_read_file_uses == 1 - def test_update_does_not_count_read_of_non_code_file(self): - """A read call whose payload points at a non-source file (e.g. ``.txt``) must - leave the read-file counter untouched — the per-tool counter is for *code* reads. - """ + def test_update_ignores_read_of_non_code_file(self): + """A read call whose payload points at a non-source file does not increment the code-read counter.""" counter = ToolUseCounter() now = datetime.now() @@ -465,9 +530,8 @@ class TestToolUseCounter: ``grep_search`` / ``read_file`` which are only recognized under the non-Claude-Code branch (Claude Code uses exact names ``grep`` / ``read``). - :param file_path: optional payload path; when set to a source-extension path (e.g. - ``src/foo.py``), the stub will be classified as a *code-file* read by - :meth:`PreToolUseRemindAboutSerenaHook._call_targets_code_file`. + :param file_path: optional payload path; the hook classifies read-style tools as + file reads regardless of the path extension. :param command_args_str: optional shell-argument tail; equivalent to ``file_path`` but populates the ``cmd``-derived field instead of ``file_path``. """ From 9f4e29c8b0d53e6fcc43633d49f4e72bcf37d1d2 Mon Sep 17 00:00:00 2001 From: Michael Panchenko Date: Tue, 5 May 2026 17:31:15 +0200 Subject: [PATCH 8/8] Changelog, readme, project.yml - minor changes --- .serena/project.yml | 22 ++++++++++++++++------ CHANGELOG.md | 4 ++-- README.md | 7 +++++-- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/.serena/project.yml b/.serena/project.yml index aa5d12e3..c49f5a03 100644 --- a/.serena/project.yml +++ b/.serena/project.yml @@ -87,12 +87,13 @@ encoding: utf-8 # Set this to a list of mode names to always include the respective modes for this project. base_modes: -# list of mode names that are to be activated by default. +# list of mode names that are to be activated by default, overriding the setting in the global configuration. # The full set of modes to be activated is base_modes (from global config) + default_modes + added_modes. -# If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. +# If the setting is undefined/empty, the default_modes from the global configuration (serena_config.yml) apply. # Otherwise, this overrides the setting from the global configuration (serena_config.yml). +# Therefore, you can set this to [] if you do not want the default modes defined in the global config to apply +# for this project. # This setting can, in turn, be overridden by CLI parameters (--mode). -# Set this to [] to not use the default modes defined in the global config for this project. # See https://oraios.github.io/serena/02-usage/050_configuration.html#modes default_modes: @@ -137,9 +138,18 @@ ignored_memory_patterns: [] # No documentation on options means no options are available. ls_specific_settings: {} -# list of mode names to be activated additionally for this project. +# list of mode names to be activated additionally for this project, e.g. ["query-projects"] # The full set of modes to be activated is base_modes (from global config) + default_modes + added_modes. -# Otherwise, this setting overrides the global configuration. -# Set this to a list of mode names to always include the respective modes for this project. # See https://oraios.github.io/serena/02-usage/050_configuration.html#modes added_modes: + +# list of additional workspace folder paths for cross-package reference support (e.g. in monorepos). +# Paths can be absolute or relative to the project root. +# Each folder is registered as an LSP workspace folder, enabling language servers to discover +# symbols and references across package boundaries. +# Currently supported for: TypeScript. +# Example: +# additional_workspace_folders: +# - ../sibling-package +# - ../shared-lib +additional_workspace_folders: [] diff --git a/CHANGELOG.md b/CHANGELOG.md index d5d71bc5..8e76ce72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,14 +7,14 @@ Status of the `main` branch. Changes prior to the next official version change w Instead, they can define `added_modes` to add modes on top of base and default modes. See updated [documentation on modes](https://oraios.github.io/serena/02-usage/050_configuration.html#modes). - Serena's default configuration now uses `interactive` and `editing` as `base_modes` instead of as `default_modes`. - - Add cross-package reference support via `additional_workspace_folders` setting (currently implemented for TypeScript). * JetBrains: - Add new tools: - `jet_brains_list_inspections`: Lists available IDE inspections (akin to diagnostics), optionally filtered by language or group - `jet_brains_run_inspections`: Runs IDE inspections on a file and returns the results -* LSP Tools: +* LSP Backend: + - Add cross-package reference support via `additional_workspace_folders` setting (currently implemented for TypeScript). - Add new tools: - `find_declaration`: Finds the declaration/definition of a symbol - `find_implementations`: Finds the implementations of an interface or abstract method diff --git a/README.md b/README.md index b0909f06..3bd53899 100644 --- a/README.md +++ b/README.md @@ -145,11 +145,14 @@ without reading entire files. | find referencing symbols | yes | yes | | search in project dependencies | -- | yes | | type hierarchy | -- | yes | -| find declaration | yes | yes | -| find implementations | yes | yes | +| find declaration | yes* | yes | +| find implementations | yes** | yes | | query external projects | yes | yes | | diagnostics/inspections | yes | yes | +*: Will generally not work for declarations in external dependencies.
+**: Only available for some languages, limited by the language server functionality. + ### Refactoring Without precise refactoring tools, agents are forced to resort to unreliable and expensive search and replace operations.