diff --git a/.gitignore b/.gitignore index f5417108ed..67edd02a0f 100644 --- a/.gitignore +++ b/.gitignore @@ -48,3 +48,12 @@ node_modules # so it stays on disk and is only untracked. Deliberately unanchored: an install # run from web/ leaves the same store one level down. .pnpm-store/ + +# otari hook setup's own generated settings: both embed a live Otari API key +# or master key directly in the hook command (see docs/agent-gates.md), so +# neither is safe to commit. Only these two files, not the whole .claude/ or +# .codex/ directory: .claude/settings.json (team-shared, committed on +# purpose) and any .claude/skills/ live under the same directory and must +# stay tracked. +.claude/settings.local.json +.codex/hooks.json diff --git a/docs/agent-gates.md b/docs/agent-gates.md index 8d586910f2..3723676185 100644 --- a/docs/agent-gates.md +++ b/docs/agent-gates.md @@ -19,12 +19,19 @@ This is the first slice. It ships: - Five gate types: `changed_path`, `command_match`, `command_if_changed`, `judge`, and `check_passed`. - `POST /api/v1/hooks/check`, evaluated against evidence the caller submits. -- `otari hook --harness claude-code`, a real installed command that reads a - Claude Code hook payload and calls the endpoint above. +- `otari hook --harness claude-code` and `otari hook --harness codex`, real + installed commands that read a Claude Code or Codex hook payload and call + the endpoint above. - `otari hook setup`, which registers it: writes a `PreToolUse` and a `Stop` - hook entry into Claude Code's own settings and, if this repo has no + hook entry into the harness's own settings (`.claude/settings.local.json` + for Claude Code, `.codex/hooks.json` for Codex) and, if this repo has no `.otari-gates.yml` yet, offers to scaffold a starter one. No reusable packs - yet, and no harness other than Claude Code. + yet. The Codex integration is newer and less exercised against a real + session than the Claude Code one; in particular, a session that runs + through Codex's own Code Mode does not yet get a `PreToolUse` dispatch at + all for a shell/apply_patch call it wraps in JS (openai/codex#23411, open + upstream), so only `Stop`'s own Git-status fallback and transcript scan + reach it today. This is a hook protocol, not a local filesystem reader: Otari never opens a caller's repository itself. The caller (an agent hook today; a native @@ -223,6 +230,30 @@ evidence it already collected for `changed_path` gates. message: This change may not follow the error-handling conventions; take a look. ``` +`judge_cli` is also optional: a string or ordered list naming which locally +installed CLI(s) may make this gate's own model call (`claude`, `codex` +today). Omitted (the default, and the only behavior before this field +existed), the caller picks whichever CLI its own invoking harness implies: +a Claude Code hook uses `claude`, a Codex hook uses `codex`, so most +policies never need this field at all. Naming one is for a gate that must +always use a specific CLI regardless of which harness invoked the hook (a +rubric that only works well with one model family, say); naming an ordered +list is a fallback preference, tried in order until one is actually found on +`PATH`, for a fleet where different machines have different CLIs installed. +`otari hook`'s own `--judge-cli`/`OTARI_HOOK_JUDGE_CLI` sets a session-wide +default between a gate's own preference and the harness default; see +"Choosing a judge CLI" below for the full precedence and both CLIs' own call +shape. + +```yaml + - id: follows-error-handling-pattern + type: judge + enforcement: advisory + rubric: Does this change follow the repository's error-handling conventions? + judge_cli: [claude, codex] + message: This change may not follow the error-handling conventions; take a look. +``` + Otari never calls a model itself, the same way it never reads a caller's repository for any other gate: the caller reads `rubric` from the parsed policy, builds its own prompt from it plus its own diff and transcript, runs @@ -234,46 +265,88 @@ the field table below). This route only relays that verdict into a call has neither a finished diff nor a full transcript to judge against yet, and submits no `judge_results` at all rather than an empty one: see the field table below for why that distinction matters), for every `judge` gate -in the local policy it shells out to `claude -p --model --tools "" ---strict-mcp-config` with a prompt built from the gate's `rubric`, `git diff +in the local policy it resolves which CLI to use (see "Choosing a judge CLI" +below), then calls it with a prompt built from the gate's `rubric`, `git diff HEAD`, and the assistant's own text replies from the session's transcript (never a `tool_use`/`tool_result` payload: a Bash call's own stdout or a Read's file contents dominate a raw transcript's bytes but carry no "why was this change made" signal a rubric can use), and requires exactly one JSON object back: `{"outcome": "pass" or "fail", "reasoning": "..."}`. This runs -against the machine's own Claude Code subscription, not billed through -Otari, from a dedicated `~/.otari/judge-workdir/` rather than the caller's -own repo: the prompt is fully self-contained text, so the call never needs -to run from the repo it is judging, and running it there anyway is how a -real early version of this recursed into itself (see below). `--tools "" ---strict-mcp-config` disable every built-in tool and MCP server for this one -call: the prompt embeds the diff and transcript verbatim, both -attacker-influenceable (a crafted diff or transcript could talk the model -into more than a verdict; see this gate type's own docstring above), and the -call never needs a tool to do its one job. `` defaults to Haiku, not -the session's own (often larger) default model: a judge call is a small, -structured pass/fail classification over bounded text, so it does not need a -frontier model, and every applicable judge gate costs one full invocation. -Override it with `--judge-model` or `OTARI_HOOK_JUDGE_MODEL` for a rubric -that genuinely needs more capability. The prompt goes over stdin, not as a -trailing argument: a diff or transcript can carry an embedded NUL byte, -which `subprocess` accepts as stdin input but raises `ValueError` for as an -argv element. A missing `claude` binary, a timeout, a launch failure -(`OSError`, e.g. `~/.otari/judge-workdir/` itself failing to create), a NUL -byte reaching this call some other way, a nonzero exit, or output that is -not that one JSON object all submit `outcome: "error"` rather than raising -(confirmed: an uncaught exception here used to exit `otari hook` before it -ever reached `httpx.post`, taking every other gate in the policy, -mechanical and required ones included, down with it), carrying the failure -detail (capped at 4,096 characters, the same length +against whatever subscription the resolved CLI is itself signed into, not +billed through Otari, from a dedicated `~/.otari/judge-workdir/` rather than +the caller's own repo: the prompt is fully self-contained text, so the call +never needs to run from the repo it is judging, and running it there anyway +is how a real early version of this recursed into itself (see below). +`` defaults to Haiku, not the session's own (often larger) default +model: a judge call is a small, structured pass/fail classification over +bounded text, so it does not need a frontier model, and every applicable +judge gate costs one full invocation. Override it with `--judge-model` or +`OTARI_HOOK_JUDGE_MODEL` for a rubric that genuinely needs more capability. +The prompt goes over stdin, not as a trailing argument, for both CLIs: a +diff or transcript can carry an embedded NUL byte, which `subprocess` +accepts as stdin input but raises `ValueError` for as an argv element. None +of the configured `judge_cli` candidates found on `PATH`, a timeout, a +launch failure (`OSError`, e.g. `~/.otari/judge-workdir/` itself failing to +create), a NUL byte reaching this call some other way, a nonzero exit, or +output that is not that one JSON object all submit `outcome: "error"` rather +than raising (confirmed: an uncaught exception here used to exit `otari +hook` before it ever reached `httpx.post`, taking every other gate in the +policy, mechanical and required ones included, down with it), carrying the +failure detail (capped at 4,096 characters, the same length `JudgeVerdictRequest.reasoning` itself caps at server-side, since an oversize `reasoning` would otherwise 422 the *whole* request and silently skip every other gate the same way) as `reasoning`; since the gate is always advisory, an `error` verdict can only ever warn, never block. One -exception: a nonzero exit whose message names `claude -p`'s own "prompt is -too long" rejection retries once with the transcript dropped entirely -(diff-only) before reporting `error`, since the transcript is supplementary -context and the diff is the evidence the rubric actually needs. +exception, `claude` only: a nonzero exit whose message names `claude -p`'s +own "prompt is too long" rejection retries once with the transcript dropped +entirely (diff-only) before reporting `error`, since the transcript is +supplementary context and the diff is the evidence the rubric actually +needs. `codex exec`'s own equivalent wording has not been confirmed against +a real call, so that backend gets no such retry yet, reporting `error` +directly on the same rejection instead. + +#### Choosing a judge CLI + +Precedence, most specific first: a gate's own `judge_cli`, then `otari +hook`'s own `--judge-cli`/`OTARI_HOOK_JUDGE_CLI`, then a default keyed on +`--harness` (`claude-code` → `claude`, `codex` → `codex`). Whichever list +that produces is tried in order; the first entry whose own binary +`shutil.which` finds on `PATH` is the one actually invoked, so a +multi-entry list is a fallback preference, not a "use all of these" +instruction, and an error message on a run where none resolve names every +candidate that was tried. + +The two backends' own call shape differs in what each can bind, including +what "no `--judge-model`/`OTARI_HOOK_JUDGE_MODEL` given" defaults to: + +- `claude`: `claude -p --model --tools "" --strict-mcp-config`, + `` defaulting to Haiku (`_HOOK_JUDGE_DEFAULT_MODEL`) with no + override; see the model paragraph above. `--tools ""`/`--strict-mcp-config` + disable every built-in tool and MCP server for this one call, dropping + their definitions from the prompt entirely: the prompt embeds the diff and + transcript verbatim, both attacker-influenceable (a crafted diff or + transcript could talk the model into more than a verdict; see this gate + type's own docstring above), and the call never needs a tool to do its one + job. +- `codex`: `codex exec - [--model ] --sandbox read-only + --ask-for-approval never --skip-git-repo-check --ephemeral --color never`. + `--model` is included only when an override was actually given: unlike + `claude`, this backend gets no hardcoded "small model" default, since + Codex's own model catalog has no equally stable name to pin (confirmed + against a real account: its own session history names a current default of + `gpt-6-astra`, not any of the "cheap tier" ids OpenAI's own docs name + elsewhere), omitting the flag entirely falls back to whatever model that + account already has configured as its own default, rather than risk naming + one it might reject outright. Codex also documents no flag to drop + tool/MCP definitions from the prompt the way `claude`'s two flags do, so + `--sandbox read-only`/`--ask-for-approval never` only bound what an + attempted tool call could *do*, not whether the model attempts one; the + isolated judge workdir this runs in already limits what a read-only + sandboxed attempt could see either way. `--skip-git-repo-check` is needed + because that workdir is a plain directory, not a repository, and + `--ephemeral` keeps this one-shot call from leaving a rollout file behind + for a later `Stop` event's own transcript scan to mistake for real session + evidence. A diff `otari hook` could not collect at all (`git diff HEAD` failing: no `HEAD` yet, a timeout, `git` itself missing) is kept distinct from one it @@ -303,15 +376,16 @@ it for that `Stop` event (confirmed against a real repo with such a file). `--judge-dry-run` (or `OTARI_HOOK_JUDGE_DRY_RUN`) runs everything up to the model call for real, policy parsing, `when_changed` filtering, diff and -transcript collection, but skips `claude -p` itself, submitting `outcome: -"error"` with a `reasoning` that estimates the prompt's size (`~4` chars per +transcript collection, `judge_cli` resolution, but skips the resolved CLI +itself, submitting `outcome: "error"` with a `reasoning` that names which +CLI(s) would have been tried and estimates the prompt's size (`~4` chars per token, a rough estimate, not a real tokenizer count, and not the same ratio the caps above are sized against) instead. Paired with the audit log below, this answers "how often would this actually fire, and roughly how large would each call be" without spending a single real model call: useful before turning a new or newly-scoped judge gate loose on a live session. -Every real `claude -p` attempt, dry-run or not, is appended to +Every real judge-CLI attempt, dry-run or not, is appended to `~/.otari/judge-calls.log` (one line per gate, before the call as `outcome: invoking` and again once it resolves), regardless of Otari's own database: this is a local, otari-hook-only audit trail, never the rubric, diff, @@ -703,9 +777,11 @@ blocking proves nothing about whether an interactive session's own `command_match`/`command_if_changed` gate is genuinely blocking. 1. Run `otari hook setup`. It writes both a `PreToolUse` hook entry and a - `Stop` hook entry into `.claude/settings.local.json` (personal, usually - gitignored by a global `~/.config/git/ignore`, so it never lands in a - PR), both pointing at this install's own + `Stop` hook entry into `.claude/settings.local.json` (personal, and this + repo's own `.gitignore` covers it specifically, since the generated + command embeds a live API key or master key: see "Registering it for + Codex" below for the same file for that harness), both pointing at this + install's own `otari hook --harness claude-code`; Claude Code passes its own `hook_event_name` in the payload, so one callback serves both. If this repo has no `.otari-gates.yml` yet, it offers to write a small starter @@ -728,9 +804,9 @@ blocking proves nothing about whether an interactive session's own every other hook or permission already in the file untouched. Safe to run again any time the policy or the credential changes. - `otari hook setup --harness claude-code` is currently the only harness; - `--api-key ` skips resolution and prompting outright, for a - non-interactive run. + `--harness codex` runs the same setup against Codex's own settings instead + (see "Registering it for Codex" below); `--api-key ` skips resolution + and prompting outright, for a non-interactive run. Three things about this are temporary, not deliberate design, and all trace back to one cause: this package installs into a per-project venv @@ -823,16 +899,64 @@ there: this endpoint accepts either, a hook needs nothing the master key uniquely grants, and a credential written into a settings file or a command line is one you should be able to rotate on its own. +### Registering it for Codex + +`otari hook setup --harness codex` writes the same pair of hook blocks into +`.codex/hooks.json` instead, naming this install's own `otari hook --harness +codex`. Personal, the same as `.claude/settings.local.json` above and for +the same reason: the generated command embeds a live API key or master key, +and this repo's own `.gitignore` covers this exact path specifically so it +never lands in a commit or a PR. + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "apply_patch|Bash|exec|code_mode_exec", + "hooks": [{"type": "command", "command": "/abs/path/to/.venv/bin/otari hook --harness codex -c /abs/path/to/config.yml"}] + } + ], + "Stop": [ + {"hooks": [{"type": "command", "command": "/abs/path/to/.venv/bin/otari hook --harness codex -c /abs/path/to/config.yml"}]} + ] + } +} +``` + +Codex's own edit tool, `apply_patch`, carries no bare `file_path` the way +Claude Code's `Edit`/`Write` do: the whole patch envelope arrives as +`tool_input.command`, and the path(s) it touches (a single call can touch +several) are read off its own `*** Add/Update/Delete File: ` header +lines. Its shell tool hook-dispatches under the same canonical name Claude +Code uses (`Bash`), but a turn run through Codex's own **Code Mode** wraps +any number of shell/apply_patch calls in one JS snippet under a tool named +`code_mode_exec` (Codex's own docs say a matcher may instead say `exec`, an +alias for that wire name; unconfirmed against a real dispatch, so the +generated matcher names both rather than depend on it); that whole snippet +is submitted as "the command" rather than parsed apart, so a `command_match` +gate still finds a forbidden phrase wherever it appears in it. As of this +writing, Code Mode's +own `PreToolUse` dispatch does not cover that surface at all +(openai/codex#23411, open upstream), confirmed against a real Codex Desktop +session that runs exclusively through Code Mode, so a policy relying only +on `PreToolUse` will not see those edits/commands until that lands; `Stop`'s +Git-status fallback and its own transcript scan (Codex's rollout JSONL, a +different shape from Claude Code's Message-API transcript) still do, since +neither depends on `PreToolUse` firing. + +### Known gaps + `otari hook` is a thin, harness-specific transport, not a second copy of the evaluator: it collects evidence and calls the endpoint above; every actual decision still comes from `gateway.agent_runtime`. What neither command does -yet: uninstall itself, probe whether it is correctly registered -(`otari status`, not built), or support a harness other than Claude Code. +yet: uninstall itself, or probe whether it is correctly registered +(`otari status`, not built). ## What's next `otari status`, to probe whether a hook is correctly registered without -re-running setup; a harness other than Claude Code; and sharing or -distributing a `check_passed` verifier across repos (a registry, reusable -"packs"), deliberately deferred rather than built alongside this first one +re-running setup; and sharing or distributing a `check_passed` verifier +across repos (a registry, reusable "packs"), deliberately deferred rather +than built alongside this first one (see that gate type's own section above). diff --git a/src/gateway/agent_runtime/domain/policy.py b/src/gateway/agent_runtime/domain/policy.py index 6828b253d3..988d480b5a 100644 --- a/src/gateway/agent_runtime/domain/policy.py +++ b/src/gateway/agent_runtime/domain/policy.py @@ -55,6 +55,14 @@ # time rather than a gate that silently blocks on a model's say-so. _JUDGE_ENFORCEMENTS = {"advisory"} +# Which locally-installed CLI(s) `otari hook` may use for a judge gate's own +# model call (JudgeGate.judge_cli); see that field's own docstring. Kept as +# its own set, not reused from anywhere `otari hook` itself defines, since +# this module stays dependency-free of that CLI-only concern (subprocess +# names, PATH resolution): a gate author only ever needs to know these two +# names exist, not how either is actually invoked. +_SUPPORTED_JUDGE_CLIS = {"claude", "codex"} + # A `**` in a forbidden glob crosses path segments by recursing over every # split point in the submitted path (domain/evaluators.py's _segments_match). # One is what every example in this codebase uses; more than that multiplies @@ -75,7 +83,7 @@ "changed_path": _COMMON_GATE_FIELDS | {"forbidden"}, "command_match": _COMMON_GATE_FIELDS | {"forbidden"}, "command_if_changed": _COMMON_GATE_FIELDS | {"when_changed", "require"}, - "judge": _COMMON_GATE_FIELDS | {"rubric", "when_changed"}, + "judge": _COMMON_GATE_FIELDS | {"rubric", "when_changed", "judge_cli"}, "check_passed": _COMMON_GATE_FIELDS | {"verifier", "when_changed"}, } @@ -320,6 +328,31 @@ def _parse_gate(raw: Any) -> GateSpec: judge_when_changed = _parse_glob_list( gate_id, "when_changed", _require_string_list(raw, "when_changed", gate_id, gate_type) ) + # judge_cli: optional, like when_changed above; absence means "no + # preference" (see JudgeGate's own docstring), not "always these two". + # Accepts a bare string as the one-entry case of the list form, not a + # different shape, since a gate author naming a single required CLI + # should not have to spell it as a one-item list. + judge_cli: tuple[str, ...] | None = None + if "judge_cli" in raw: + raw_judge_cli = raw["judge_cli"] + if isinstance(raw_judge_cli, str): + candidates = [raw_judge_cli] if raw_judge_cli else [] + elif isinstance(raw_judge_cli, list) and all(isinstance(item, str) for item in raw_judge_cli): + candidates = list(dict.fromkeys(raw_judge_cli)) + else: + candidates = [] + if not candidates: + raise PolicyError( + f"Gate {gate_id!r} (type 'judge'): 'judge_cli' must be a non-empty string or list of strings." + ) + unsupported = [item for item in candidates if item not in _SUPPORTED_JUDGE_CLIS] + if unsupported: + raise PolicyError( + f"Gate {gate_id!r}: judge_cli entries {unsupported!r} are not supported. " + f"Supported in this build: {', '.join(sorted(_SUPPORTED_JUDGE_CLIS))}." + ) + judge_cli = tuple(candidates) # enforcement_value is already proven "advisory" by the _JUDGE_ENFORCEMENTS # check above; cast documents that narrowing the same way the plain # Enforcement cast above documents its own. @@ -328,6 +361,7 @@ def _parse_gate(raw: Any) -> GateSpec: enforcement=cast(Literal["advisory"], enforcement_value), rubric=rubric, when_changed=tuple(judge_when_changed), + judge_cli=judge_cli, message=message, ) diff --git a/src/gateway/agent_runtime/domain/types.py b/src/gateway/agent_runtime/domain/types.py index 9259276fd9..c2e9d2f5cb 100644 --- a/src/gateway/agent_runtime/domain/types.py +++ b/src/gateway/agent_runtime/domain/types.py @@ -154,6 +154,19 @@ class JudgeGate: touched a matching path, so a rubric about, say, error-handling conventions is not re-judged, at real model-call cost, on a session that never touched application code. + + ``judge_cli`` is optional and names which locally-installed CLI(s) + ``otari hook`` may use to make the model call this gate needs, in + preference order; the first one whose own binary is found on ``PATH`` + wins. ``None`` (the default, and the only behavior a judge gate had + before this field existed) means no preference: the caller falls back to + whichever CLI its own invoking harness implies (Claude Code's hook -> + ``claude``, Codex's -> ``codex``). Naming one explicitly is what lets a + gate authored for, say, a Codex-only fleet require ``codex`` even when + invoked by a Claude Code hook, or list both so whichever is actually + installed on a given machine is used. This field changes nothing about + where the call happens: still entirely within ``otari hook``, never here + (see this gate's own opening paragraph). """ id: str @@ -161,6 +174,7 @@ class JudgeGate: rubric: str message: str when_changed: tuple[str, ...] = () + judge_cli: tuple[str, ...] | None = None type: Literal["judge"] = "judge" diff --git a/src/gateway/cli.py b/src/gateway/cli.py index afc94dc2aa..cef0b9f60a 100644 --- a/src/gateway/cli.py +++ b/src/gateway/cli.py @@ -10,6 +10,7 @@ import time from datetime import UTC, datetime from pathlib import Path +from typing import NamedTuple import click import uvicorn @@ -243,6 +244,53 @@ def gen_secret_key() -> None: # command_match gate gets before the command runs; see docs/agent-gates.md. _HOOK_COMMAND_TOOL_FIELDS = {"Bash": "command"} +# Codex hook-dispatches its own shell tool under the same canonical name +# Claude Code uses ("Bash"; confirmed against openai/codex's own +# HookToolName), plus, once a turn runs through Code Mode, "code_mode_exec": +# a freeform JS snippet that can wrap any number of tools.exec_command()/ +# tools.apply_patch() calls rather than naming a single command ("exec" is +# also accepted, in case a build reports the pre-canonicalization name). That +# whole snippet is kept as "the command" here rather than parsed apart: a +# forbidden phrase a command_match gate looks for still matches wherever it +# appears in it, and Code Mode's own PreToolUse dispatch is not complete yet +# (openai/codex#23411), so there is no reliable per-argument shape to parse +# even if it were worth the fragility. +_HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS = { + "claude-code": _HOOK_COMMAND_TOOL_FIELDS, + "codex": {"Bash": "command", "code_mode_exec": "command", "exec": "command"}, +} + +# Codex's own edit tool, sharing Claude Code's apply_patch envelope +# convention: unlike Edit/Write, tool_input carries no bare file_path; +# "command" holds the whole patch text, and the path(s) it touches are named +# on the envelope's own header lines instead (_hook_extract_patch_paths). +_CODEX_PATCH_TOOL_NAME = "apply_patch" +# A rename is its own two-line shape, not a fourth header verb: "*** Update +# File: " immediately followed by "*** Move to: ", +# neither one alone naming where the file ends up. +_PATCH_HEADER_RE = re.compile(r"^\*\*\* (?:(?:Add|Update|Delete) File|Move to): (.+)$", re.MULTILINE) + + +def _hook_extract_patch_paths(patch_text: str) -> list[str]: + """Target path(s) named in an apply_patch envelope's own header lines. + + One apply_patch call can touch several files, each named on its own + "*** Add/Update/Delete File: " header line; order-preserving and + de-duplicated, since a policy gate cares about the set of touched paths, + not how many headers happened to name each one. A rename's own "*** Move + to: " line is matched too, alongside the "Update File:" line naming + its old path that always precedes one: both the vacated and the landed-on + path are evidence a changed_path gate could care about, and reporting + only one would silently miss whichever gate is scoped to the other. + """ + seen: dict[str, None] = {} + for match in _PATCH_HEADER_RE.finditer(patch_text): + path = match.group(1).strip() + if path: + seen[path] = None + return list(seen) + + # Mirrors the Hook Server's own per-command bound (routes/hooks.py's # _MAX_COMMAND_LENGTH). A literal rather than an import: this command talks to # a gateway over HTTP that may be a different build, so the number it truncates @@ -467,6 +515,82 @@ def _hook_collect_transcript_commands(transcript_path: Path) -> list[str] | None return [command for tool_use_id, command in requested if tool_use_id is None or tool_use_id not in denied_ids] +# Codex's own equivalent of _PRETOOLUSE_DENIAL_MARKERS: no confirmed wrapper +# string for a PreToolUse-denied call has been observed in a real Codex +# transcript, so _hook_collect_codex_transcript_commands makes no attempt to +# exclude one. That is the same safer direction Claude Code's own denial +# handling argues for: keeping a denied command in evidence costs an +# occasional false "ran", never a missed "ran". +_CODEX_COMMAND_TOOL_NAMES = frozenset({"Bash", "shell", "local_shell", "exec_command"}) + + +def _hook_collect_codex_transcript_commands(transcript_path: Path) -> list[str] | None: + """Evidence for a `command_match`/`command_if_changed` gate on a Codex Stop event. + + Codex's own rollout file (its `transcript_path`) is a JSONL log of + `response_item` records, a different shape from Claude Code's own + Message-API transcript that `_hook_collect_transcript_commands` reads. A + classic shell call appears as a `function_call` item whose `arguments` is + a JSON-encoded string carrying a `command` field (a string, or an argv + list joined with spaces here); a turn run through Code Mode instead wraps + any number of shell/apply_patch calls in one `custom_tool_call` + (`name: "exec"`) whose `input` is the raw JavaScript that issued them. + That JS text is kept whole as "the command", the same choice + `_HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS` makes for a live PreToolUse call + and for the same reason: a forbidden phrase still matches wherever it + appears in it, with no per-argument parsing to get wrong. + + Returns None only when the transcript itself cannot be read, the same + sentinel `_hook_collect_transcript_commands` uses. + """ + try: + lines = transcript_path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + return None + + commands: list[str] = [] + for line in lines: + if not line.strip(): + continue + try: + record = json.loads(line) + except ValueError: + continue + if not isinstance(record, dict) or record.get("type") != "response_item": + continue + item = record.get("payload") + if not isinstance(item, dict): + continue + item_type = item.get("type") + if item_type == "custom_tool_call" and item.get("name") == "exec": + text = item.get("input") + if isinstance(text, str) and text: + commands.append(text) + elif item_type == "function_call" and item.get("name") in _CODEX_COMMAND_TOOL_NAMES: + raw_arguments = item.get("arguments") + # isinstance first, not a bare json.loads(... or "{}"): "arguments" is + # documented as a JSON-encoded string, but a malformed record or a + # future Codex shape carrying it pre-parsed (a dict/list) would + # otherwise reach json.loads and raise TypeError, which nothing here + # catches, crashing this whole Stop-event invocation instead of + # skipping the one record, the same fail-open contract every other + # per-record parse in this function already keeps. + if not isinstance(raw_arguments, str): + continue + try: + arguments = json.loads(raw_arguments) + except ValueError: + continue + if not isinstance(arguments, dict): + continue + command = arguments.get("command") + if isinstance(command, list): + command = " ".join(str(part) for part in command) + if isinstance(command, str) and command: + commands.append(command) + return commands + + # A judge gate's prompt is rubric + diff + transcript excerpt, each bounded # independently so one huge file or one long session can't build an unbounded # `claude -p` argv. Sized against a real measurement, not the "~4 chars/token" @@ -524,6 +648,16 @@ def _hook_collect_transcript_commands(transcript_path: Path) -> list[str] | None # one full invocation against the caller's own subscription (see # _hook_run_judge). Overridable per-invocation with --judge-model / # OTARI_HOOK_JUDGE_MODEL for a rubric that genuinely needs more capability. +# +# claude only: Codex's own model catalog has no equally stable "small model" +# name to hardcode the same way (confirmed against a real account: its own +# session history names a current default of "gpt-6-astra", not any of the +# "cheap tier" ids OpenAI's own docs name elsewhere, which is exactly the +# kind of drift a hardcoded guess here would silently go stale against). +# `_hook_run_judge` leaves `--model` off the codex backend's own invocation +# entirely when neither this nor --judge-model apply, falling back to +# whatever model that account already has configured as its own default, +# rather than risk naming one Codex might reject outright. _HOOK_JUDGE_DEFAULT_MODEL = "claude-haiku-4-5-20251001" _HOOK_JUDGE_PROMPT_TEMPLATE = """\ @@ -590,6 +724,47 @@ def _hook_extract_judge_transcript(transcript_path: Path) -> str: return "\n".join(texts) +def _hook_extract_codex_judge_transcript(transcript_path: Path) -> str: + """The assistant's own text replies from a Codex rollout, for a judge gate's prompt. + + Codex's equivalent of `_hook_extract_judge_transcript`: an assistant + reply is a `response_item` of type `message`, `role: "assistant"`, its + own text under `content[].type == "output_text"` (`input_text` is the + role Codex gives the other direction (developer/user turns), which + carry no judgment about this session's own work). + + Returns "" when the transcript cannot be read at all, or carries no + assistant text, the same as `_hook_extract_judge_transcript`. + """ + try: + lines = transcript_path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError: + return "" + + texts: list[str] = [] + for line in lines: + if not line.strip(): + continue + try: + record = json.loads(line) + except ValueError: + continue + if not isinstance(record, dict) or record.get("type") != "response_item": + continue + item = record.get("payload") + if not isinstance(item, dict) or item.get("type") != "message" or item.get("role") != "assistant": + continue + content = item.get("content") + if not isinstance(content, list): + continue + texts.extend( + block["text"] + for block in content + if isinstance(block, dict) and block.get("type") == "output_text" and isinstance(block.get("text"), str) + ) + return "\n".join(texts) + + def _hook_collect_diff(repo_root: Path) -> str | None: """The working tree's own diff against HEAD, for a judge gate's prompt. @@ -759,32 +934,28 @@ def _hook_strip_judge_code_fence(raw: str) -> str: _HOOK_MAX_JUDGE_REASONING_LENGTH = 4_096 -def _hook_call_claude_p(claude_path: str, model: str, prompt: str, *, deadline: float) -> tuple[str, str]: - """One `claude -p --model ` invocation; return (outcome, reasoning). +def _hook_run_judge_subprocess(argv: list[str], prompt: str, *, deadline: float, label: str) -> tuple[str, str]: + """Shared tail of every judge-CLI backend's own invocation: run `argv`, parse its + stdout as the one JSON verdict object the judge prompt demands; return (outcome, reasoning). + + Both `_hook_call_claude_p` and `_hook_call_codex_exec` build their own + `argv` (each backend's own flags are backend-specific: see each + function's own docstring for why) and hand it here for everything after + that: launching it, bounding it to what is left of the shared + `deadline`, and turning its stdout into a verdict. `label` (`"claude + -p"`/`"codex exec"`) names the backend in every message this produces, + the only difference in what each backend's own error/success text reads. outcome is always one of "pass"/"fail"/"error": a nonzero exit, a timeout, or output that is not the single JSON object the prompt demands are all "error", carrying the failure detail as reasoning rather than - raising. `claude -p`'s own "prompt is too long" rejection exits nonzero - with the message on stdout, not stderr (confirmed against a real call), - so the error detail falls back to stdout when stderr is empty. + raising. Runs with `cwd` set to `_hook_judge_workdir()`, never the repo being judged: see that function's own docstring for why (a real recursive incident) and why that is an isolated directory rather than a hooks-disabling flag. - `--tools ""` and `--strict-mcp-config`: this call's own prompt embeds the - diff and transcript verbatim, both attacker-influenceable (a crafted diff - or transcript could talk the model into more than a verdict; see the - `judge` gate type's own docstring on this), and it never needs a tool to - do its one job (read a prompt, emit one JSON object). `--tools ""` - disables every built-in tool; `--strict-mcp-config` with no `--mcp-config` - means no MCP server loads either, including one configured for the - repo being judged. Confirmed this still produces a normal verdict (and, - since it skips loading tool/MCP definitions into the system prompt, - measured cheaper than the same call without these flags). - `deadline` (a `time.monotonic()` timestamp, see `_HOOK_JUDGE_TOTAL_BUDGET_SECONDS`) is shared across every gate and retry in one run, not a fresh budget per call: already past it, this returns @@ -798,7 +969,7 @@ def _hook_call_claude_p(claude_path: str, model: str, prompt: str, *, deadline: try: result = subprocess.run( # noqa: S603 - fixed argv, no shell, resolved executable path - [claude_path, "--model", model, "--tools", "", "--strict-mcp-config", "-p"], + argv, input=prompt, cwd=_hook_judge_workdir(), capture_output=True, @@ -808,10 +979,10 @@ def _hook_call_claude_p(claude_path: str, model: str, prompt: str, *, deadline: check=False, ) except subprocess.TimeoutExpired: - return "error", f"claude -p did not respond within {min(_HOOK_JUDGE_TIMEOUT_SECONDS, remaining):.0f}s" + return "error", f"{label} did not respond within {min(_HOOK_JUDGE_TIMEOUT_SECONDS, remaining):.0f}s" except (OSError, ValueError) as exc: # OSError: `_hook_judge_workdir()`'s own `mkdir` (permissions, disk - # full) or the subprocess launch itself (`claude` disappearing + # full) or the subprocess launch itself (the binary disappearing # between `shutil.which` and this call). ValueError: an embedded NUL # byte, which a diff or transcript can carry (confirmed: `subprocess` # raises "embedded null byte" for one in an argv element, the reason @@ -819,33 +990,179 @@ def _hook_call_claude_p(claude_path: str, model: str, prompt: str, *, deadline: # argument). Both used to propagate uncaught, exiting `otari hook` # before it ever reached `httpx.post` and skipping every other gate # in the policy, mechanical and required ones included. - return "error", f"could not run claude -p ({exc})" + return "error", f"could not run {label} ({exc})" if result.returncode != 0: detail = result.stderr.strip() or result.stdout.strip() - return "error", f"claude -p exited {result.returncode}: {detail[:500]}" + return "error", f"{label} exited {result.returncode}: {detail[:500]}" try: verdict = json.loads(_hook_strip_judge_code_fence(result.stdout)) except ValueError: - return "error", f"claude -p did not return valid JSON: {result.stdout[:500]!r}" + return "error", f"{label} did not return valid JSON: {result.stdout[:500]!r}" outcome = verdict.get("outcome") if isinstance(verdict, dict) else None reasoning = verdict.get("reasoning") if isinstance(verdict, dict) else None if outcome not in ("pass", "fail") or not isinstance(reasoning, str): - return "error", f"claude -p returned an unrecognized verdict shape: {result.stdout[:500]!r}" + return "error", f"{label} returned an unrecognized verdict shape: {result.stdout[:500]!r}" return outcome, reasoning[:_HOOK_MAX_JUDGE_REASONING_LENGTH] +def _hook_call_claude_p(claude_path: str, model: str, prompt: str, *, deadline: float) -> tuple[str, str]: + """One `claude -p --model ` invocation; return (outcome, reasoning). + + `claude -p`'s own "prompt is too long" rejection exits nonzero with the + message on stdout, not stderr (confirmed against a real call), which is + why `_hook_run_judge_subprocess` falls back to stdout for its own error + detail when stderr is empty. + + `--tools ""` and `--strict-mcp-config`: this call's own prompt embeds the + diff and transcript verbatim, both attacker-influenceable (a crafted diff + or transcript could talk the model into more than a verdict; see the + `judge` gate type's own docstring on this), and it never needs a tool to + do its one job (read a prompt, emit one JSON object). `--tools ""` + disables every built-in tool; `--strict-mcp-config` with no `--mcp-config` + means no MCP server loads either, including one configured for the + repo being judged. Confirmed this still produces a normal verdict (and, + since it skips loading tool/MCP definitions into the system prompt, + measured cheaper than the same call without these flags). + """ + argv = [claude_path, "--model", model, "--tools", "", "--strict-mcp-config", "-p"] + return _hook_run_judge_subprocess(argv, prompt, deadline=deadline, label="claude -p") + + +def _hook_call_codex_exec(codex_path: str, model: str | None, prompt: str, *, deadline: float) -> tuple[str, str]: + """One `codex exec -` invocation; return (outcome, reasoning). + + `model` is `None` when `--judge-model`/`OTARI_HOOK_JUDGE_MODEL` named + none specifically (see + `_HOOK_JUDGE_DEFAULT_MODEL`'s own comment on why this backend gets no + hardcoded "small model" default the way `claude` does): `--model` is then + left off the invocation entirely, letting Codex fall back to whatever + model this account already has configured as its own default, rather + than risk naming one this build/account might reject outright. + + Codex's own non-interactive one-shot mode: the prompt goes over stdin + (`-` in place of a positional prompt argument, `codex exec`'s own way of + reading one), matching `_hook_call_claude_p`'s own choice for the same + reason (a diff or transcript can carry an embedded NUL byte, which + `subprocess` rejects in an argv element but not in piped input). + + `--sandbox read-only` and `--ask-for-approval never` keep this call from + taking any action even if the model attempts one: this call's own prompt + embeds the diff and transcript verbatim, both attacker-influenceable (see + JudgeGate's own docstring on this), and Codex documents no flag to drop + tool/MCP definitions from the prompt entirely the way `_hook_call_claude_p`'s + `--tools ""`/`--strict-mcp-config` do, so this only bounds what an + attempted tool call could *do*, not whether the model attempts one; the + isolated `_hook_judge_workdir()` this runs in already limits what a + read-only sandboxed attempt could see either way. `--skip-git-repo-check` + because that workdir is a plain directory, not a repository, and + `--ephemeral` so this one-shot call leaves no rollout file behind for a + future Stop event's own transcript scan to mistake for real session + evidence. + + outcome is always one of "pass"/"fail"/"error", the same contract + `_hook_call_claude_p` returns: `codex exec` prints only the final agent + message to stdout, progress to stderr, matched here by parsing that + stdout as the single JSON object the prompt demands. + + No equivalent to `_hook_call_claude_p`'s "prompt is too long" retry: + Codex's own rejection wording for an oversize prompt has not been + confirmed against a real call, so `_hook_run_judge` never applies that + retry to this backend rather than match a marker string that might never + fire. + """ + argv = [codex_path, "exec", "-"] + if model is not None: + argv += ["--model", model] + argv += [ + "--sandbox", + "read-only", + "--ask-for-approval", + "never", + "--skip-git-repo-check", + "--ephemeral", + "--color", + "never", + ] + return _hook_run_judge_subprocess(argv, prompt, deadline=deadline, label="codex exec") + + +# Binary name `shutil.which` resolves for each judge_cli backend. +# _hook_run_judge's own inline dispatch (not a dict of the two caller +# functions: their `model` parameter is optional for codex, required for +# claude, and a dict's value type would otherwise have to widen to the union +# of both, losing the distinction a type checker could otherwise hold onto) +# picks which one to call. +_JUDGE_CLI_BINARY_NAMES = {"claude": "claude", "codex": "codex"} + +# Which judge_cli backend a gate gets when neither it nor --judge-cli names +# one: whichever CLI the harness actually invoking this hook run is itself +# built on. See _hook_collect_judge_verdicts for the full precedence order. +_JUDGE_CLI_DEFAULT_BY_HARNESS = {"claude-code": ("claude",), "codex": ("codex",)} + + +def _hook_resolve_judge_cli(candidates: tuple[str, ...]) -> tuple[str, str] | None: + """First of `candidates` (in that order) whose own binary is found on PATH; None if none are.""" + for name in candidates: + binary = shutil.which(_JUDGE_CLI_BINARY_NAMES[name]) + if binary: + return name, binary + return None + + +def _parse_judge_cli(ctx: click.Context, param: click.Parameter, value: str | None) -> tuple[str, ...] | None: + """Parse `--judge-cli`/`OTARI_HOOK_JUDGE_CLI`: a comma-separated, ordered judge_cli override. + + None (unset) means "no session-wide override": a gate's own `judge_cli` + still wins over it either way, and a gate naming none of its own falls + back to the invoking harness's own default (`_JUDGE_CLI_DEFAULT_BY_HARNESS`). + """ + if value is None: + return None + names = tuple(name.strip() for name in value.split(",") if name.strip()) + unsupported = [name for name in names if name not in _JUDGE_CLI_BINARY_NAMES] + if not names or unsupported: + raise click.BadParameter( + f"must be a comma-separated list of: {', '.join(sorted(_JUDGE_CLI_BINARY_NAMES))} (got {value!r})." + ) + return names + + def _hook_run_judge( - rubric: str, diff: str, transcript: str, *, model: str, deadline: float, dry_run: bool = False + rubric: str, + diff: str, + transcript: str, + *, + judge_cli: tuple[str, ...], + model: str | None, + deadline: float, + dry_run: bool = False, ) -> tuple[str, str]: - """Invoke `claude -p --model ` for one judge gate's rubric; return (outcome, reasoning). + """Invoke the first available `judge_cli` backend for one judge gate's rubric; return (outcome, reasoning). Otari itself never calls a model (see JudgeGate's own docstring); this is - that call, made locally against the caller's own Claude Code - subscription, not billed through Otari. + that call, made locally against whatever subscription the resolved CLI + itself is signed into, not billed through Otari. + + `model` is the caller's own explicit choice (`--judge-model`/ + `OTARI_HOOK_JUDGE_MODEL`), or `None` for "no explicit choice, use this + backend's own default": `claude` gets `_HOOK_JUDGE_DEFAULT_MODEL` + (Haiku); `codex` gets none at all, `_hook_call_codex_exec` then omitting + `--model` entirely (see `_HOOK_JUDGE_DEFAULT_MODEL`'s own comment on why + the two are not symmetric here). Resolved after `judge_cli`, not before: + which default applies depends on which backend actually gets picked. + + `judge_cli` is the already-resolved preference order for this one gate + (a gate's own `judge_cli`, else `--judge-cli`/`OTARI_HOOK_JUDGE_CLI`, else + the invoking harness's own default; see `_hook_collect_judge_verdicts`). + `_hook_resolve_judge_cli` picks the first entry whose own binary is on + PATH; this is what lets a gate listing `judge_cli: [claude, codex]` still + get a verdict on a machine with only one of the two installed, and what + makes a bare, single-entry list behave exactly as a hardcoded "claude" + always did before this existed. `dry_run` skips the real call entirely, before ever touching `shutil.which` or `subprocess`: the wire contract has no fourth outcome to spell "this @@ -861,31 +1178,42 @@ def _hook_run_judge( entirely: the transcript is supplementary "why" context for a judge rubric, the diff is the primary evidence, so a diff-only retry is a strictly better fallback than reporting no verdict at all. Only for that - specific rejection, and only once: any other failure, or a rejection that - persists with no transcript left to drop, reports "error" as it always - has. + specific rejection, only once, and only against the `claude` backend + (`_hook_call_codex_exec`'s own docstring says why Codex gets no + equivalent yet): any other failure, or a rejection that persists with no + transcript left to drop, reports "error" as it always has. """ if dry_run: prompt = _hook_build_judge_prompt(rubric, diff, transcript) estimated_tokens = _hook_estimate_tokens(prompt) return ( "error", - f"--judge-dry-run: real claude -p call skipped; prompt would have been " + f"--judge-dry-run: real {'/'.join(judge_cli)} call skipped; prompt would have been " f"{len(prompt):,} chars (~{estimated_tokens:,} tokens estimated at " f"~{_HOOK_JUDGE_CHARS_PER_TOKEN_ESTIMATE} chars/token).", ) - claude_path = shutil.which("claude") - if not claude_path: - return "error", "the `claude` CLI was not found on PATH" + resolved = _hook_resolve_judge_cli(judge_cli) + if resolved is None: + tried = ", ".join(_JUDGE_CLI_BINARY_NAMES[name] for name in judge_cli) + return "error", f"none of the configured judge CLI(s) were found on PATH: {tried}" + backend, binary_path = resolved - outcome, reasoning = _hook_call_claude_p( - claude_path, model, _hook_build_judge_prompt(rubric, diff, transcript), deadline=deadline - ) - if outcome == "error" and transcript and _HOOK_JUDGE_PROMPT_TOO_LONG_MARKER in reasoning.lower(): - outcome, reasoning = _hook_call_claude_p( - claude_path, model, _hook_build_judge_prompt(rubric, diff, ""), deadline=deadline - ) + def call(prompt_text: str) -> tuple[str, str]: + if backend == "claude": + return _hook_call_claude_p( + binary_path, model if model is not None else _HOOK_JUDGE_DEFAULT_MODEL, prompt_text, deadline=deadline + ) + return _hook_call_codex_exec(binary_path, model, prompt_text, deadline=deadline) + + outcome, reasoning = call(_hook_build_judge_prompt(rubric, diff, transcript)) + if ( + backend == "claude" + and outcome == "error" + and transcript + and _HOOK_JUDGE_PROMPT_TOO_LONG_MARKER in reasoning.lower() + ): + outcome, reasoning = call(_hook_build_judge_prompt(rubric, diff, "")) return outcome, reasoning @@ -896,10 +1224,12 @@ def _hook_collect_judge_verdicts( transcript_path: str | None, changed_paths: list[str], *, - judge_model: str, + judge_model: str | None, judge_dry_run: bool = False, + harness: str = "claude-code", + judge_cli_override: tuple[str, ...] | None = None, ) -> list[dict[str, str]]: - """Run every applicable judge gate in the local policy, one `claude -p` call each. + """Run every applicable judge gate in the local policy, one model-CLI call each. Parses the policy locally with the same pure `domain.policy.parse_policy` the Hook Server itself uses, purely to find which gates are judge gates @@ -923,9 +1253,19 @@ def _hook_collect_judge_verdicts( `judge_dry_run` (see `hook`'s own `--judge-dry-run`) still runs this whole applicability check, still reads the diff and transcript, and still writes the same `_hook_log_judge_call` audit lines; only `_hook_run_judge` - itself skips the real `claude -p` call. This is what makes the resulting + itself skips the real model-CLI call. This is what makes the resulting log a real count of how often the model would have been invoked, not a guess: everything up to the call itself runs exactly as it would for real. + + `harness` picks which transcript format `transcript_path` is read as + (Claude Code's Message-API transcript vs. Codex's rollout JSONL). It also + supplies the *default* judge_cli order (`_JUDGE_CLI_DEFAULT_BY_HARNESS`) + for a gate that names none of its own: precedence, most specific first, + is a gate's own `JudgeGate.judge_cli`, then this call's own + `judge_cli_override` (`hook`'s own `--judge-cli`/`OTARI_HOOK_JUDGE_CLI`), + then that harness default. A gate or override naming more than one CLI is + an ordered fallback list, resolved by `_hook_resolve_judge_cli`: the first + entry whose own binary is on PATH is what actually gets invoked. """ try: spec = parse_policy(policy_yaml, source=str(gates_file)) @@ -959,7 +1299,8 @@ def _hook_collect_judge_verdicts( diff = _hook_collect_diff(repo_root) diff_collection_failed = diff is None diff = diff or "" - transcript = _hook_extract_judge_transcript(Path(transcript_path)) if transcript_path else "" + extract_transcript = _hook_extract_codex_judge_transcript if harness == "codex" else _hook_extract_judge_transcript + transcript = extract_transcript(Path(transcript_path)) if transcript_path else "" if len(transcript) > _HOOK_JUDGE_MAX_TRANSCRIPT_CHARS: click.echo( f"otari hook: transcript is {len(transcript):,} characters, over the " @@ -980,7 +1321,7 @@ def _hook_collect_judge_verdicts( results = [] for gate in judge_gates: - # Logged before the call, not after: a hung or killed `claude -p` + # Logged before the call, not after: a hung or killed model-CLI # invocation must still show up in the audit trail rather than # silently vanishing along with the process that would have logged # its outcome. @@ -988,12 +1329,19 @@ def _hook_collect_judge_verdicts( if diff_collection_failed: # No model call at all: a diff this gate cannot see is not # evidence to judge against, and every other pre-flight failure - # here (a missing `claude` binary, an exhausted time budget) - # already reports "error" without one either. + # here (no configured judge_cli found on PATH, an exhausted time + # budget) already reports "error" without one either. outcome, reasoning = "error", "could not collect the working tree diff" else: + judge_cli = gate.judge_cli or judge_cli_override or _JUDGE_CLI_DEFAULT_BY_HARNESS.get(harness, ("claude",)) outcome, reasoning = _hook_run_judge( - gate.rubric, diff, transcript, model=judge_model, deadline=deadline, dry_run=judge_dry_run + gate.rubric, + diff, + transcript, + judge_cli=judge_cli, + model=judge_model, + deadline=deadline, + dry_run=judge_dry_run, ) _hook_log_judge_call(repo_root, gate.id, outcome, detail=reasoning if judge_dry_run else None) results.append({"gate_id": gate.id, "outcome": outcome, "reasoning": reasoning}) @@ -1207,7 +1555,7 @@ def _hook_collect_check_verdicts( @cli.group(name="hook", invoke_without_command=True) @click.option( "--harness", - type=click.Choice(["claude-code"]), + type=click.Choice(["claude-code", "codex"]), default="claude-code", show_default=True, help="Agent integration sending this callback.", @@ -1224,9 +1572,23 @@ def _hook_collect_check_verdicts( @click.option( "--judge-model", envvar="OTARI_HOOK_JUDGE_MODEL", - default=_HOOK_JUDGE_DEFAULT_MODEL, - show_default=True, - help="Model `claude -p` uses for a judge gate's model call.", + default=None, + help=( + f"Model the resolved judge CLI uses for a judge gate's model call. Defaults to " + f"{_HOOK_JUDGE_DEFAULT_MODEL!r} when the resolved backend is claude; when it is codex, " + "left unset (that account's own default model applies) unless given explicitly here." + ), +) +@click.option( + "--judge-cli", + envvar="OTARI_HOOK_JUDGE_CLI", + default=None, + callback=_parse_judge_cli, + help=( + "Comma-separated, ordered judge-gate CLI backend(s) to try (claude, codex); the first one " + "found on PATH is used. A gate's own judge_cli overrides this; with neither set, defaults to " + "whichever CLI --harness itself implies." + ), ) @click.option( "--judge-dry-run", @@ -1246,7 +1608,8 @@ def hook( config: str | None, url: str | None, api_key: str | None, - judge_model: str, + judge_model: str | None, + judge_cli: tuple[str, ...] | None, judge_dry_run: bool, ) -> None: """Native callback entry point for a supported agent's hook protocol. @@ -1258,12 +1621,21 @@ def hook( harness-specific transport. See docs/agent-gates.md. Exit code is this harness's own protocol, not otari policy check's: - Claude Code's PreToolUse and Stop hooks both take 0 (proceed) or 2 (block, - stderr shown to the agent). Never blocks on a problem that is not a - required gate failing: a missing policy, an unreachable gateway, or a - missing credential all exit 0, with a message on stderr where there is + Claude Code's and Codex's PreToolUse and Stop hooks both take 0 (proceed) + or 2 (block, stderr shown to the agent). Never blocks on a problem that is + not a required gate failing: a missing policy, an unreachable gateway, or + a missing credential all exit 0, with a message on stderr where there is one worth surfacing. + `--harness` picks which payload/transcript shape is expected and which + tool names are read as an edit vs. a command (see + `_HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS`, `_CODEX_PATCH_TOOL_NAME`); Codex's + own Code Mode wraps shell/apply_patch calls in a JS snippet rather than + naming one tool, and its PreToolUse dispatch does not yet cover that + surface at all (openai/codex#23411), so a `changed_path`/`command_match` + gate scoped to `PreToolUse` will not see a Code Mode edit until upstream + fixes that; `Stop`'s own Git-status fallback and transcript scan still do. + A group, not a plain command, so `otari hook setup` can live alongside it: invoked with no subcommand (the shape every existing settings file already calls), it runs the callback above unchanged. @@ -1329,11 +1701,37 @@ def hook( if event == "PreToolUse": tool_name = payload.get("tool_name", "") tool_input = payload.get("tool_input") or {} + command_fields = _HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS.get(harness, _HOOK_COMMAND_TOOL_FIELDS) + is_apply_patch = harness == "codex" and tool_name == _CODEX_PATCH_TOOL_NAME # A tool call is either an edit or a shell command, never both, so at # most one of these evidence lists is ever populated per call. - path_field = _HOOK_EDIT_TOOL_PATH_FIELDS.get(tool_name) - command_field = _HOOK_COMMAND_TOOL_FIELDS.get(tool_name) - if path_field: + path_field = None if is_apply_patch else _HOOK_EDIT_TOOL_PATH_FIELDS.get(tool_name) + command_field = command_fields.get(tool_name) + if is_apply_patch: + # apply_patch carries no bare file_path the way Edit/Write do: + # tool_input["command"] is the whole patch envelope, one or more + # files named on its own header lines. + patch_text = tool_input.get("command") + if not patch_text: + return + resolved_paths = [] + for patch_path in _hook_extract_patch_paths(patch_text): + try: + # Joined onto `repo` (the call's own cwd), not resolved + # bare: an apply_patch header names its target relative to + # the tool call's own working directory, unlike Edit/ + # Write's always-absolute file_path. `Path.__truediv__` + # discards `repo` on its own if `patch_path` is already + # absolute, so both shapes resolve correctly here. See the + # Windows as_posix() note below: the same reason applies + # here, one target at a time. + resolved_paths.append((repo / patch_path).resolve().relative_to(root).as_posix()) + except ValueError: + continue # Outside the repo: nothing this policy can name. + if not resolved_paths: + return + changed_paths = resolved_paths + elif path_field: target = tool_input.get(path_field) if not target: return @@ -1375,14 +1773,17 @@ def hook( return changed_paths = collected - # transcript_path is Claude Code's own name for the session's JSONL - # transcript on disk. Absent, or unreadable, submits None rather than - # `[]`: `[]` means "collected, and there is none", which would let a - # required command_match/command_if_changed gate read a failed - # collection as a clean pass instead of the unresolved `unknown` it - # actually is (see docs/agent-gates.md). + # transcript_path is the session's JSONL transcript on disk (each + # harness's own name/format for it). Absent, or unreadable, submits + # None rather than `[]`: `[]` means "collected, and there is none", + # which would let a required command_match/command_if_changed gate + # read a failed collection as a clean pass instead of the unresolved + # `unknown` it actually is (see docs/agent-gates.md). transcript_path = payload.get("transcript_path") - commands = _hook_collect_transcript_commands(Path(transcript_path)) if transcript_path else None + collect_transcript_commands = ( + _hook_collect_codex_transcript_commands if harness == "codex" else _hook_collect_transcript_commands + ) + commands = collect_transcript_commands(Path(transcript_path)) if transcript_path else None command_scope = "session" if commands: # Same truncation the PreToolUse Bash branch applies to its one @@ -1410,6 +1811,8 @@ def hook( changed_paths, judge_model=judge_model, judge_dry_run=judge_dry_run, + harness=harness, + judge_cli_override=judge_cli, ) check_results = _hook_collect_check_verdicts(policy_yaml, gates_file, root, changed_paths) else: @@ -1502,7 +1905,7 @@ def hook( for gate in failing ) if blocked: - # stop_hook_active is Claude Code's own signal that this Stop is + # stop_hook_active is the harness's own signal that this Stop is # already the continuation a previous block forced. It matters because # Claude Code overrides a Stop hook that blocks eight times running # without progress, and then simply lets the turn end: a required gate @@ -1514,12 +1917,19 @@ def hook( # forbidden change. So keep blocking, and say plainly that the block # is finite, so the agent spends the remaining attempts fixing the # gate or telling the user it cannot, rather than retrying blind. - repeat_note = ( - "\n (already blocked once this turn; Claude Code overrides a Stop hook after 8 " - "consecutive blocks, so fix this now or say why you cannot.)" - if payload.get("stop_hook_active") - else "" - ) + # + # Codex's own retry cap (if it has a fixed one) has not been + # confirmed against a real session the way Claude Code's has, so its + # note names no specific number rather than guessing one. + if payload.get("stop_hook_active"): + repeat_note = ( + "\n (already blocked once this turn; Claude Code overrides a Stop hook after 8 " + "consecutive blocks, so fix this now or say why you cannot.)" + if harness == "claude-code" + else "\n (already blocked once this turn; fix this now or say why you cannot.)" + ) + else: + repeat_note = "" click.echo(f"otari hook: blocked ({harness}, {event}):\n{summary}{repeat_note}", err=True) raise SystemExit(2) # An advisory gate failed but nothing required did: warn without @@ -1535,8 +1945,8 @@ def hook( def _otari_binary_path() -> str: """Absolute path to this otari install's own binary. - Claude Code's hook subprocess does not inherit an activated shell's PATH, - so a bare "otari" often will not resolve. otari's own console-script + A hook subprocess (Claude Code's, Codex's) does not inherit an activated + shell's PATH, so a bare "otari" often will not resolve. otari's own console-script wrapper sits next to the interpreter running it (same venv/bin), which is what sys.executable already names. """ @@ -1647,10 +2057,31 @@ def _merge_hook_entry(settings_path: Path, event: str, command: str, *, matcher: return not updated +class _HookSetup(NamedTuple): + """Where one harness reads its own hook registration from, and the matcher vocabulary + (see _HOOK_EDIT_TOOL_PATH_FIELDS, _CODEX_PATCH_TOOL_NAME, _HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS) + its own PreToolUse dispatch expects. Codex's own upstream docs say "exec" is accepted as a + matcher alias for what its payload actually reports as tool_name "code_mode_exec"; that has + not been confirmed against a real dispatch, so `command_matcher` below names both literally + rather than depend on the alias translation actually being implemented. + """ + + settings_dir: str + settings_name: str + edit_matcher: str + command_matcher: str + + +_HOOK_SETUP_BY_HARNESS = { + "claude-code": _HookSetup(".claude", "settings.local.json", "Edit|Write|NotebookEdit", "Bash"), + "codex": _HookSetup(".codex", "hooks.json", "apply_patch", "Bash|exec|code_mode_exec"), +} + + @hook.command(name="setup") @click.option( "--harness", - type=click.Choice(["claude-code"]), + type=click.Choice(["claude-code", "codex"]), default="claude-code", show_default=True, help="Agent integration to configure.", @@ -1663,15 +2094,16 @@ def _merge_hook_entry(settings_path: Path, event: str, command: str, *, matcher: def hook_setup(harness: str, api_key: str | None) -> None: """Register otari hook in a supported agent's own settings. - Writes or updates a PreToolUse hook entry and a Stop hook entry in - .claude/settings.local.json (personal, gitignored, never committed) so - registering the Hook Server is not a manual JSON edit. Both point at the - same otari hook invocation; Claude Code passes its own hook_event_name in - the payload, so one callback serves either event. Offers to scaffold a - starter .otari-gates.yml when this repo has none yet, and picks the - PreToolUse matcher (whether it needs to cover Bash) from whatever gates - the policy turns out to have; Stop needs no matcher; see - docs/agent-gates.md for why both are registered unconditionally. + Writes or updates a PreToolUse hook entry and a Stop hook entry in the + harness's own personal, gitignored settings file (see + _HOOK_SETUP_BY_HARNESS) so registering the Hook Server is not a manual + JSON edit. Both point at the same otari hook invocation; the harness + passes its own hook_event_name in the payload, so one callback serves + either event. Offers to scaffold a starter .otari-gates.yml when this + repo has none yet, and picks the PreToolUse matcher (whether it needs to + cover a shell tool) from whatever gates the policy turns out to have; + Stop needs no matcher; see docs/agent-gates.md for why both are + registered unconditionally. """ root = _hook_find_repo_root(Path.cwd()) if root is None: @@ -1688,8 +2120,9 @@ def hook_setup(harness: str, api_key: str | None) -> None: f"passes until {gates_file.name} exists; see docs/agent-gates.md." ) + setup = _HOOK_SETUP_BY_HARNESS[harness] include_bash = _gates_file_allows_bash(gates_file) - matcher = "Edit|Write|NotebookEdit|Bash" if include_bash else "Edit|Write|NotebookEdit" + matcher = f"{setup.edit_matcher}|{setup.command_matcher}" if include_bash else setup.edit_matcher embedded_key = api_key if not embedded_key: @@ -1709,10 +2142,13 @@ def hook_setup(harness: str, api_key: str | None) -> None: command_parts += ["--api-key", embedded_key] command = shlex.join(command_parts) - settings_path = root / ".claude" / "settings.local.json" + settings_path = root / setup.settings_dir / setup.settings_name pretooluse_created = _merge_hook_entry(settings_path, "PreToolUse", command, matcher=matcher) click.echo(f"{'Added' if pretooluse_created else 'Updated'} the PreToolUse hook in {settings_path}.") - click.echo(f"Matcher: {matcher}" + ("" if include_bash else " (add a command_match gate to also cover Bash)")) + click.echo( + f"Matcher: {matcher}" + + ("" if include_bash else f" (add a command_match gate to also cover {setup.command_matcher})") + ) # Registered unconditionally, not only when the policy has a gate that # benefits: changed_path already falls back to `git status` on Stop diff --git a/tests/unit/agent_runtime/test_policy.py b/tests/unit/agent_runtime/test_policy.py index e25202c35a..8973a972ac 100644 --- a/tests/unit/agent_runtime/test_policy.py +++ b/tests/unit/agent_runtime/test_policy.py @@ -355,6 +355,62 @@ def test_judge_gate_rejects_an_explicitly_empty_when_changed() -> None: parse_policy(policy, source="test.yml") +def test_judge_gate_judge_cli_defaults_to_none() -> None: + """A judge gate that never mentions `judge_cli` keeps its pre-field behavior: no preference.""" + policy = ( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: judge\n enforcement: advisory\n rubric: r\n message: m\n" + ) + spec = parse_policy(policy, source="test.yml") + gate = spec.gates[0] + assert isinstance(gate, JudgeGate) + assert gate.judge_cli is None + + +def test_judge_gate_accepts_a_bare_judge_cli_string() -> None: + policy = ( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: judge\n enforcement: advisory\n rubric: r\n" + " judge_cli: codex\n message: m\n" + ) + spec = parse_policy(policy, source="test.yml") + gate = spec.gates[0] + assert isinstance(gate, JudgeGate) + assert gate.judge_cli == ("codex",) + + +def test_judge_gate_accepts_an_ordered_judge_cli_list_deduplicated() -> None: + policy = ( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: judge\n enforcement: advisory\n rubric: r\n" + " judge_cli: [codex, claude, codex]\n message: m\n" + ) + spec = parse_policy(policy, source="test.yml") + gate = spec.gates[0] + assert isinstance(gate, JudgeGate) + assert gate.judge_cli == ("codex", "claude") + + +def test_judge_gate_rejects_an_unsupported_judge_cli() -> None: + policy = ( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: judge\n enforcement: advisory\n rubric: r\n" + " judge_cli: gemini\n message: m\n" + ) + with pytest.raises(PolicyError, match="judge_cli"): + parse_policy(policy, source="test.yml") + + +def test_judge_gate_rejects_an_explicitly_empty_judge_cli_list() -> None: + policy = ( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: judge\n enforcement: advisory\n rubric: r\n" + " judge_cli: []\n message: m\n" + ) + with pytest.raises(PolicyError, match="judge_cli"): + parse_policy(policy, source="test.yml") + + def test_parses_a_valid_check_passed_policy() -> None: policy = ( 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' diff --git a/tests/unit/test_hook_cli.py b/tests/unit/test_hook_cli.py index 30e75e50e1..d6f5684da8 100644 --- a/tests/unit/test_hook_cli.py +++ b/tests/unit/test_hook_cli.py @@ -1133,7 +1133,11 @@ def fake_post(url: str, **kwargs: object) -> _FakeResponse: result = _invoke({"hook_event_name": "Stop", "cwd": str(judge_repo)}) assert result.exit_code == 0, result.output assert captured["json"]["judge_results"] == [ - {"gate_id": "follows-pattern", "outcome": "error", "reasoning": "the `claude` CLI was not found on PATH"} + { + "gate_id": "follows-pattern", + "outcome": "error", + "reasoning": "none of the configured judge CLI(s) were found on PATH: claude", + } ] diff --git a/tests/unit/test_hook_cli_codex.py b/tests/unit/test_hook_cli_codex.py new file mode 100644 index 0000000000..796da86460 --- /dev/null +++ b/tests/unit/test_hook_cli_codex.py @@ -0,0 +1,492 @@ +"""Unit tests for `otari hook --harness codex` and `otari hook setup --harness codex`. + +Codex's own payload/transcript shapes differ from Claude Code's (see +`_HOOK_COMMAND_TOOL_FIELDS_BY_HARNESS`, `_CODEX_PATCH_TOOL_NAME`, +`_hook_collect_codex_transcript_commands`, `_hook_extract_codex_judge_transcript` +in `gateway.cli`); this file covers those, the same way +`tests/unit/test_hook_cli.py` and `tests/unit/test_hook_setup_cli.py` cover +the Claude Code harness. Mocks the network boundary (httpx.post) and the Git +boundary (subprocess.run), same as test_hook_cli.py. +""" + +import json +import subprocess +from pathlib import Path +from typing import Any + +import httpx +import pytest +from click.testing import CliRunner + +import gateway.cli as gateway_cli + +_GATES_YAML = "schema_version: '1.0'\npolicy:\n id: test\ngates: []\n" + +_FAKE_OTARI_PATH = "/opt/otari/.venv/bin/otari" + + +class _FakeResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def raise_for_status(self) -> None: + pass + + def json(self) -> dict[str, Any]: + return self._payload + + +@pytest.fixture(autouse=True) +def _judge_log_in_tmp_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(gateway_cli, "_hook_judge_log_path", lambda: tmp_path / "judge-calls.log") + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + (tmp_path / ".git").mkdir() + (tmp_path / ".otari-gates.yml").write_text(_GATES_YAML, encoding="utf-8") + return tmp_path + + +def _invoke(payload: dict[str, Any], **extra_args: str) -> Any: + args = ["--api-key", "test-key", "--harness", "codex"] + for key, value in extra_args.items(): + args += [f"--{key.replace('_', '-')}", value] + return CliRunner().invoke(gateway_cli.hook, args, input=json.dumps(payload)) + + +# --- PreToolUse: apply_patch (Codex's own edit tool) ------------------------ + + +def test_pretooluse_extracts_a_single_path_from_an_apply_patch_envelope( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse( + { + "blocked": True, + "results": [{"gate_id": "g", "enforcement": "required", "outcome": "fail", "message": "no"}], + } + ) + + monkeypatch.setattr(httpx, "post", fake_post) + patch_text = "*** Begin Patch\n*** Update File: CHANGELOG.md\n@@\n-old\n+new\n*** End Patch" + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {"command": patch_text}, + } + result = _invoke(payload) + assert result.exit_code == 2, result.output + assert captured["json"]["changed_paths"] == ["CHANGELOG.md"] + assert captured["json"]["commands"] == [] + + +def test_pretooluse_apply_patch_covers_every_file_it_touches(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + patch_text = ( + "*** Begin Patch\n" + "*** Add File: src/new_module.py\n" + "+content\n" + "*** Update File: README.md\n" + "@@\n-old\n+new\n" + "*** Delete File: old_file.py\n" + "*** End Patch" + ) + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {"command": patch_text}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["changed_paths"] == ["src/new_module.py", "README.md", "old_file.py"] + + +def test_pretooluse_apply_patch_rename_reports_both_old_and_new_path( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + """A rename is "*** Update File: " immediately followed by "*** Move to: ", + neither line alone naming where the file ends up; a gate scoped to either path should see it. + """ + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + patch_text = ( + "*** Begin Patch\n*** Update File: old_name.py\n*** Move to: new_name.py\n@@\n-old\n+new\n*** End Patch" + ) + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {"command": patch_text}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["changed_paths"] == ["old_name.py", "new_name.py"] + + +def test_pretooluse_ignores_an_apply_patch_with_no_command(repo: Path) -> None: + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + + +def test_pretooluse_apply_patch_with_no_recognizable_header_is_a_no_op(repo: Path) -> None: + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {"command": "not a real patch envelope"}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + + +def test_pretooluse_apply_patch_path_outside_the_repo_is_skipped(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + patch_text = "*** Begin Patch\n*** Update File: /etc/passwd\n*** Update File: CHANGELOG.md\n*** End Patch" + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "apply_patch", + "tool_input": {"command": patch_text}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["changed_paths"] == ["CHANGELOG.md"] + + +# --- PreToolUse: shell / Code Mode exec ------------------------------------- + + +def test_pretooluse_submits_a_bash_command_for_codex(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse( + { + "blocked": True, + "results": [ + {"gate_id": "no-force-push", "enforcement": "required", "outcome": "fail", "message": "no"} + ], + } + ) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "Bash", + "tool_input": {"command": "git push --force"}, + } + result = _invoke(payload) + assert result.exit_code == 2, result.output + assert captured["json"]["commands"] == ["git push --force"] + + +def test_pretooluse_submits_a_code_mode_exec_snippet_whole(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + """Code Mode wraps any number of tools.exec_command()/tools.apply_patch() calls in one + JS snippet rather than naming a single command; the whole snippet is submitted as "the + command" so a forbidden phrase still matches wherever it appears, with nothing parsed + out of it. + """ + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + snippet = 'text(await tools.exec_command({cmd:"git status --short"}));' + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "code_mode_exec", + "tool_input": {"command": snippet}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] == [snippet] + + +def test_pretooluse_ignores_unhandled_codex_tools(repo: Path) -> None: + payload = { + "hook_event_name": "PreToolUse", + "cwd": str(repo), + "tool_name": "wait", + "tool_input": {"cell_id": "1"}, + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + + +# --- Stop: transcript command collection ------------------------------------ + + +def _response_item(payload: dict[str, Any]) -> str: + return json.dumps({"type": "response_item", "payload": payload}) + + +def test_stop_event_collects_classic_function_call_commands( + monkeypatch: pytest.MonkeyPatch, repo: Path, tmp_path: Path +) -> None: + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + transcript = tmp_path / "rollout.jsonl" + transcript.write_text( + "\n".join( + [ + _response_item( + {"type": "message", "role": "assistant", "content": [{"type": "output_text", "text": "hi"}]} + ), + _response_item( + {"type": "function_call", "name": "Bash", "arguments": json.dumps({"command": "git status"})} + ), + _response_item( + { + "type": "function_call", + "name": "shell", + "arguments": json.dumps({"command": ["ls", "-la"]}), + } + ), + _response_item({"type": "function_call", "name": "wait", "arguments": json.dumps({"cell_id": "1"})}), + ] + ) + + "\n", + encoding="utf-8", + ) + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = {"hook_event_name": "Stop", "cwd": str(repo), "transcript_path": str(transcript)} + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] == ["git status", "ls -la"] + + +def test_stop_event_skips_a_function_call_whose_arguments_is_not_a_json_string( + monkeypatch: pytest.MonkeyPatch, repo: Path, tmp_path: Path +) -> None: + """`arguments` is documented as a JSON-encoded string; a malformed record carrying it + pre-parsed (a dict, here) must be skipped, not crash json.loads with an uncaught TypeError. + """ + + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + transcript = tmp_path / "rollout.jsonl" + transcript.write_text( + "\n".join( + [ + _response_item({"type": "function_call", "name": "Bash", "arguments": {"command": "pwd"}}), + _response_item( + {"type": "function_call", "name": "Bash", "arguments": json.dumps({"command": "git status"})} + ), + ] + ) + + "\n", + encoding="utf-8", + ) + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = {"hook_event_name": "Stop", "cwd": str(repo), "transcript_path": str(transcript)} + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] == ["git status"] + + +def test_stop_event_collects_code_mode_exec_snippets_from_the_transcript( + monkeypatch: pytest.MonkeyPatch, repo: Path, tmp_path: Path +) -> None: + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + transcript = tmp_path / "rollout.jsonl" + snippet = 'text(await tools.exec_command({cmd:"npm install"}));' + transcript.write_text( + _response_item({"type": "custom_tool_call", "name": "exec", "input": snippet}) + "\n", + encoding="utf-8", + ) + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = {"hook_event_name": "Stop", "cwd": str(repo), "transcript_path": str(transcript)} + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] == [snippet] + + +def test_stop_event_skips_a_malformed_codex_transcript_line( + monkeypatch: pytest.MonkeyPatch, repo: Path, tmp_path: Path +) -> None: + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + transcript = tmp_path / "rollout.jsonl" + transcript.write_text( + "not json\n" + + _response_item({"type": "function_call", "name": "Bash", "arguments": '{"command": "pwd"}'}) + + "\n", + encoding="utf-8", + ) + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = {"hook_event_name": "Stop", "cwd": str(repo), "transcript_path": str(transcript)} + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] == ["pwd"] + + +def test_stop_event_codex_transcript_missing_submits_no_command_evidence( + monkeypatch: pytest.MonkeyPatch, repo: Path, tmp_path: Path +) -> None: + def fake_run(*args: object, **kwargs: object) -> subprocess.CompletedProcess[str]: + return subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + payload = { + "hook_event_name": "Stop", + "cwd": str(repo), + "transcript_path": str(tmp_path / "does-not-exist.jsonl"), + } + result = _invoke(payload) + assert result.exit_code == 0, result.output + assert captured["json"]["commands"] is None + + +# --- Judge gate transcript extraction --------------------------------------- + + +def test_codex_judge_transcript_extraction_keeps_only_assistant_output_text(tmp_path: Path) -> None: + transcript = tmp_path / "rollout.jsonl" + transcript.write_text( + "\n".join( + [ + _response_item( + {"type": "message", "role": "user", "content": [{"type": "input_text", "text": "do it"}]} + ), + _response_item( + { + "type": "message", + "role": "assistant", + "content": [{"type": "output_text", "text": "Reviewing now."}], + } + ), + _response_item({"type": "function_call", "name": "Bash", "arguments": "{}"}), + _response_item( + { + "type": "message", + "role": "assistant", + "content": [{"type": "output_text", "text": "Found an issue."}], + } + ), + ] + ) + + "\n", + encoding="utf-8", + ) + assert gateway_cli._hook_extract_codex_judge_transcript(transcript) == "Reviewing now.\nFound an issue." + + +def test_codex_judge_transcript_extraction_returns_empty_for_an_unreadable_file(tmp_path: Path) -> None: + assert gateway_cli._hook_extract_codex_judge_transcript(tmp_path / "missing.jsonl") == "" + + +# --- otari hook setup --harness codex --------------------------------------- + + +@pytest.fixture(autouse=True) +def _fixed_otari_path(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(gateway_cli, "_otari_binary_path", lambda: _FAKE_OTARI_PATH) + + +def _invoke_setup(*args: str, input: str | None = None) -> Any: # noqa: A002 - matches CliRunner's own kwarg name + return CliRunner().invoke(gateway_cli.hook, ["setup", "--harness", "codex", *args], input=input) + + +def test_setup_writes_codex_hooks_json_not_claude_settings(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + (tmp_path / ".git").mkdir() + monkeypatch.chdir(tmp_path) + result = _invoke_setup("--api-key", "k", input="n\n") + assert result.exit_code == 0, result.output + assert not (tmp_path / ".claude").exists() + settings = json.loads((tmp_path / ".codex" / "hooks.json").read_text(encoding="utf-8")) + entry = settings["hooks"]["PreToolUse"][0] + assert entry["matcher"] == "apply_patch" + assert entry["hooks"][0]["command"] == f"{_FAKE_OTARI_PATH} hook --harness codex --api-key k" + assert settings["hooks"]["Stop"][0]["hooks"][0]["command"] == f"{_FAKE_OTARI_PATH} hook --harness codex --api-key k" + + +def test_setup_matcher_covers_bash_and_exec_when_a_command_match_gate_exists( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + (tmp_path / ".git").mkdir() + monkeypatch.chdir(tmp_path) + (tmp_path / ".otari-gates.yml").write_text( + 'schema_version: "1.0"\npolicy:\n id: x\ngates:\n' + " - id: g\n type: command_match\n enforcement: required\n" + ' forbidden: ["npm"]\n message: m\n', + encoding="utf-8", + ) + result = _invoke_setup("--api-key", "k") + assert result.exit_code == 0, result.output + settings = json.loads((tmp_path / ".codex" / "hooks.json").read_text(encoding="utf-8")) + assert settings["hooks"]["PreToolUse"][0]["matcher"] == "apply_patch|Bash|exec|code_mode_exec" diff --git a/tests/unit/test_hook_judge_cli.py b/tests/unit/test_hook_judge_cli.py new file mode 100644 index 0000000000..76171bcff5 --- /dev/null +++ b/tests/unit/test_hook_judge_cli.py @@ -0,0 +1,345 @@ +"""Unit tests for judge-gate CLI backend selection: a gate's own `judge_cli`, +`--judge-cli`/`OTARI_HOOK_JUDGE_CLI`, and the invoking harness's own default, +in that precedence order (see `_hook_collect_judge_verdicts` in +`gateway.cli`). Complements `tests/unit/test_hook_cli.py` (which already +covers the `claude` backend's own call shape end to end) by covering the +selection mechanism itself and the `codex exec` backend's own call shape. +""" + +import json +import shutil +import subprocess +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import httpx +import pytest +from click.testing import CliRunner + +import gateway.cli as gateway_cli +from gateway.core.config import GatewayConfig + + +class _FakeResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def raise_for_status(self) -> None: + pass + + def json(self) -> dict[str, Any]: + return self._payload + + +@pytest.fixture(autouse=True) +def _judge_log_in_tmp_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.setattr(gateway_cli, "_hook_judge_log_path", lambda: tmp_path / "judge-calls.log") + + +@pytest.fixture(autouse=True) +def _config_stub(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + gateway_cli, "load_config", lambda config_path=None: GatewayConfig(master_key="test-master-key") + ) + + +def _gates_yaml(judge_cli: str | None = None) -> str: + judge_cli_line = f" judge_cli: {judge_cli}\n" if judge_cli is not None else "" + return ( + "schema_version: '1.0'\n" + "policy:\n id: test\n" + "gates:\n" + " - id: g\n" + " type: judge\n" + " enforcement: advisory\n" + " rubric: r\n" + f"{judge_cli_line}" + " message: m\n" + ) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + (tmp_path / ".git").mkdir() + return tmp_path + + +def _git_status_and_diff_run() -> Callable[..., subprocess.CompletedProcess[str]]: + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[:2] == ["git", "status"]: + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + if cmd[:2] == ["git", "diff"]: + return subprocess.CompletedProcess(args=cmd, returncode=0, stdout="", stderr="") + raise AssertionError(f"unexpected subprocess.run call before the judge CLI itself: {cmd}") + + return fake_run + + +def _invoke(payload: dict[str, Any], *, harness: str = "claude-code", extra: list[str] | None = None) -> Any: + args = ["--api-key", "test-key", "--harness", harness, *(extra or [])] + return CliRunner().invoke(gateway_cli.hook, args, input=json.dumps(payload)) + + +def _capture_post(monkeypatch: pytest.MonkeyPatch) -> dict[str, Any]: + captured: dict[str, Any] = {} + + def fake_post(url: str, **kwargs: object) -> _FakeResponse: + captured["json"] = kwargs.get("json") + return _FakeResponse({"blocked": False, "results": []}) + + monkeypatch.setattr(httpx, "post", fake_post) + return captured + + +def test_claude_code_harness_defaults_to_the_claude_backend(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + result = _git_status_and_diff_run()(cmd, **kwargs) if cmd[0] == "git" else None + if result is not None: + return result + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}" if name in ("claude", "codex") else None) + captured = _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code") + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/claude"] + assert captured["json"]["judge_results"] == [{"gate_id": "g", "outcome": "pass", "reasoning": "ok"}] + + +def test_codex_harness_defaults_to_the_codex_backend(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + result = _git_status_and_diff_run()(cmd, **kwargs) if cmd[0] == "git" else None + if result is not None: + return result + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}" if name in ("claude", "codex") else None) + captured = _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="codex") + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/codex"] + assert captured["json"]["judge_results"] == [{"gate_id": "g", "outcome": "pass", "reasoning": "ok"}] + + +def test_codex_exec_is_invoked_read_only_and_non_interactive(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + assert cmd[0] == "/usr/bin/codex" + assert cmd[1:3] == ["exec", "-"], "prompt goes over stdin, via the '-' pseudo-argument, not a trailing arg" + assert "--model" not in cmd, ( + "no --judge-model given and no stable 'small codex model' to default to (see " + "_HOOK_JUDGE_DEFAULT_MODEL's own comment): --model is left off, not guessed" + ) + assert cmd[cmd.index("--sandbox") + 1] == "read-only" + assert cmd[cmd.index("--ask-for-approval") + 1] == "never" + assert "--skip-git-repo-check" in cmd, "the judge workdir is a plain directory, not a Git repo" + assert "--ephemeral" in cmd, "a one-shot judge call must not leave a rollout file behind" + assert "input" in kwargs, "the prompt is piped over stdin, matching claude -p's own choice" + assert kwargs.get("cwd") == gateway_cli._hook_judge_workdir() + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "fail", "reasoning": "no"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/codex" if name == "codex" else None) + captured = _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="codex") + assert result.exit_code == 0, result.output + assert captured["json"]["judge_results"] == [{"gate_id": "g", "outcome": "fail", "reasoning": "no"}] + + +def test_claude_gets_the_haiku_default_model_with_no_override(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + assert cmd[cmd.index("--model") + 1] == gateway_cli._HOOK_JUDGE_DEFAULT_MODEL + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/claude" if name == "claude" else None) + _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code") + assert result.exit_code == 0, result.output + + +def test_judge_model_flag_overrides_the_default_for_the_codex_backend( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + assert cmd[cmd.index("--model") + 1] == "gpt-6-astra" + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/codex" if name == "codex" else None) + _capture_post(monkeypatch) + + result = _invoke( + {"hook_event_name": "Stop", "cwd": str(repo)}, harness="codex", extra=["--judge-model", "gpt-6-astra"] + ) + assert result.exit_code == 0, result.output + + +def test_a_gates_own_judge_cli_overrides_the_harness_default(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + """A gate authored to require codex gets codex even from a Claude Code hook.""" + (repo / ".otari-gates.yml").write_text(_gates_yaml(judge_cli="codex"), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}" if name in ("claude", "codex") else None) + _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code") + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/codex"] + + +def test_judge_cli_flag_overrides_the_harness_default_but_not_a_gates_own( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(judge_cli="claude"), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}" if name in ("claude", "codex") else None) + _capture_post(monkeypatch) + + # --judge-cli codex would win over the claude-code harness default, but + # the gate's own judge_cli: claude is more specific still and wins over both. + result = _invoke( + {"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code", extra=["--judge-cli", "codex"] + ) + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/claude"] + + +def test_judge_cli_flag_overrides_the_harness_default_when_the_gate_has_none( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(shutil, "which", lambda name: f"/usr/bin/{name}" if name in ("claude", "codex") else None) + _capture_post(monkeypatch) + + result = _invoke( + {"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code", extra=["--judge-cli", "codex"] + ) + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/codex"] + + +def test_judge_cli_falls_back_to_the_next_candidate_when_the_first_is_missing( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(judge_cli="[claude, codex]"), encoding="utf-8") + called_with: list[str] = [] + + def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + if cmd[0] == "git": + return _git_status_and_diff_run()(cmd, **kwargs) + called_with.append(cmd[0]) + return subprocess.CompletedProcess( + args=cmd, returncode=0, stdout=json.dumps({"outcome": "pass", "reasoning": "ok"}), stderr="" + ) + + monkeypatch.setattr(subprocess, "run", fake_run) + # Only codex is on PATH: claude is the preferred first candidate, but not available. + monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/codex" if name == "codex" else None) + _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code") + assert result.exit_code == 0, result.output + assert called_with == ["/usr/bin/codex"] + + +def test_reports_error_naming_every_candidate_tried_when_none_are_on_path( + monkeypatch: pytest.MonkeyPatch, repo: Path +) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(judge_cli="[claude, codex]"), encoding="utf-8") + monkeypatch.setattr(subprocess, "run", _git_status_and_diff_run()) + monkeypatch.setattr(shutil, "which", lambda name: None) + captured = _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="claude-code") + assert result.exit_code == 0, result.output + assert captured["json"]["judge_results"] == [ + { + "gate_id": "g", + "outcome": "error", + "reasoning": "none of the configured judge CLI(s) were found on PATH: claude, codex", + } + ] + + +def test_dry_run_message_names_the_resolved_harness_default(monkeypatch: pytest.MonkeyPatch, repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + monkeypatch.setattr(subprocess, "run", _git_status_and_diff_run()) + captured = _capture_post(monkeypatch) + + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, harness="codex", extra=["--judge-dry-run"]) + assert result.exit_code == 0, result.output + assert "real codex call skipped" in captured["json"]["judge_results"][0]["reasoning"] + + +def test_judge_cli_flag_rejects_an_unsupported_backend(repo: Path) -> None: + (repo / ".otari-gates.yml").write_text(_gates_yaml(), encoding="utf-8") + result = _invoke({"hook_event_name": "Stop", "cwd": str(repo)}, extra=["--judge-cli", "gemini"]) + assert result.exit_code != 0 + assert "claude" in result.output and "codex" in result.output