diff --git a/src/serena/hooks.py b/src/serena/hooks.py index a93221932..a7343d176 100644 --- a/src/serena/hooks.py +++ b/src/serena/hooks.py @@ -243,18 +243,46 @@ def reset(self) -> None: #: are caught alongside ``read_file``, while ``write_file``/``edit_file`` are not. _READ_FILE_VERB_SUBSTRINGS: tuple[str, ...] = ("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 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 name = self._tool_name if "file" not in name: return False diff --git a/test/serena/test_hooks.py b/test/serena/test_hooks.py index 25408f0d0..d72f9a1ff 100644 --- a/test/serena/test_hooks.py +++ b/test/serena/test_hooks.py @@ -512,6 +512,138 @@ def test_cleanup_is_idempotent(self, tmp_path: Path): SessionEndCleanupHook(HookClient.CLAUDE_CODE).execute() +class TestCodexExecCommandDetection: + """Tests for Codex ``exec_command`` grep/read classification (issue #1394). + + When the Codex client sends shell work as ``exec_command`` with the actual + command under ``tool_input.cmd``, the hook must inspect that field and count + the call against the appropriate counter. + """ + + def _make_codex_input(self, cmd: str, session_id: str = "codex-session") -> dict: + return { + "session_id": session_id, + "tool_name": "exec_command", + "tool_input": {"cmd": cmd}, + } + + # --- is_grep_tool --- + + def test_rg_is_grep(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input('rg "SomeSymbol" packages/server'))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_tool() + + def test_grep_is_grep(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input('grep -r "foo" .'))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_tool() + + def test_path_prefixed_grep_is_grep(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input('/usr/bin/grep -r "foo" .'))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_tool() + + def test_ag_is_grep(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("ag SomeSymbol src/"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_tool() + + # --- is_read_file_tool --- + + def test_sed_is_read(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("sed -n '1,220p' packages/server/foo.ts"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_read_file_tool() + + def test_cat_is_read(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("cat README.md"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_read_file_tool() + + def test_head_is_read(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("head -n 40 src/main.py"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_read_file_tool() + + def test_tail_is_read(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("tail -n 20 src/main.py"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_read_file_tool() + + # --- negatives --- + + def test_ls_is_neither(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("ls -la src/"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert not hook.is_grep_tool() + assert not hook.is_read_file_tool() + + def test_echo_is_neither(self, tmp_path: Path): + with patch("sys.stdin", _make_stdin(self._make_codex_input("echo hello"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert not hook.is_grep_tool() + assert not hook.is_read_file_tool() + + def test_non_exec_command_tool_unaffected(self, tmp_path: Path): + """For Codex, non-exec_command tools still fall through to substring matching.""" + with patch("sys.stdin", _make_stdin(_base_input(tool_name="grep_search"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + hook = PreToolUseRemindAboutSerenaHook(HookClient.CODEX) + assert hook.is_grep_tool() + + # --- end-to-end: deny fires after threshold for Codex exec_command --- + + def test_deny_after_threshold_codex_rg(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): + """Repeated ``exec_command`` calls with ``rg`` should trip the grep deny for Codex.""" + for _ in range(ToolUseCounter._GREP_USES_THRESHOLD): + with patch("sys.stdin", _make_stdin(self._make_codex_input('rg "Symbol" src/'))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + PreToolUseRemindAboutSerenaHook(HookClient.CODEX).execute() + + output = capsys.readouterr().out.strip() + assert output, "expected a deny to be emitted for Codex rg calls" + result = json.loads(output) + assert result["hookSpecificOutput"]["permissionDecision"] == "deny" + assert "grep" in result["hookSpecificOutput"]["additionalContext"].lower() + + def test_deny_after_threshold_codex_sed(self, tmp_path: Path, capsys: pytest.CaptureFixture[str]): + """Repeated ``exec_command`` calls with ``sed`` should trip the read deny for Codex.""" + for _ in range(ToolUseCounter._READ_FILE_USES_THRESHOLD): + with patch("sys.stdin", _make_stdin(self._make_codex_input("sed -n '1,50p' src/main.py"))), patch( + "serena.hooks.serena_home_dir", str(tmp_path) + ): + PreToolUseRemindAboutSerenaHook(HookClient.CODEX).execute() + + output = capsys.readouterr().out.strip() + assert output, "expected a deny to be emitted for Codex sed calls" + result = json.loads(output) + assert result["hookSpecificOutput"]["permissionDecision"] == "deny" + assert "read file" in result["hookSpecificOutput"]["additionalContext"].lower() + + class TestHookCli: """Tests for the Click CLI entry point (serena-hooks)."""