Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/serena/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 132 additions & 0 deletions test/serena/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)."""

Expand Down
Loading