Skip to content

V3/codex hook - #3305

Closed
gemini133 wants to merge 5 commits into
rtk-ai:developfrom
gemini133:v3/codex-hook
Closed

gemini133 wants to merge 5 commits into
rtk-ai:developfrom
gemini133:v3/codex-hook

Conversation

@gemini133

Copy link
Copy Markdown

Summary

  • Adds native Codex PreToolUse hook handling for Bash commands.
  • Adds rtk init --codex --auto-patch install/uninstall support for
    $CODEX_HOME/hooks.json.
  • Emits Codex-compatible allow, deny, and no-decision responses.

Extends #377 (feat(init): add Codex CLI support via AGENTS.md + RTK.md workflow) from advisory setup files to Codex's native hook protocol.

Problem

Codex hook output has stricter schema requirements than Claude-style rewrite
responses. In particular, Codex rejects updatedInput unless the same response
also has permissionDecision: "allow". A deny response must include a reason
and must not include updatedInput.

Without native support, Codex Bash commands either stay raw or risk invalid hook
JSON when RTK attempts to rewrite them.

Behavior Changes

Codex PreToolUse case Before After
Default/ask rule, command git status A rewrite response can be invalid for Codex. stdout {}; Codex runs raw git status.
Explicit allow Bash(git *), command git status Command stays raw. Emits permissionDecision: "allow" and updatedInput.command = "rtk git status".
Explicit deny Bash(git status) Deny cannot be represented safely by the old path. Emits permissionDecision: "deny" with a reason and no updatedInput.
rtk init --codex --auto-patch with existing hooks No native RTK Codex hook upsert. Adds/removes only the RTK-managed entry; foreign entries remain.

Implementation

  • Adds rtk hook codex.
  • Adds Codex-specific hook response construction in src/hooks/hook_cmd.rs.
  • Adds Codex hook constants and an _rtk_managed marker for install hygiene.
  • Adds Codex install, upsert, and uninstall helpers in src/hooks/init.rs.
  • Preserves non-Bash and malformed payloads as no-decision responses.

Compatibility Notes

  • No updatedInput is emitted unless Codex receives
    permissionDecision: "allow".
  • Deny responses include permissionDecisionReason and omit updatedInput.
  • Existing non-RTK hooks.json entries are preserved.
  • Protocol stderr cleanliness is handled by the separate hook-warning cleanup
    PR; the Codex schema behavior here is independent.

Test Plan

  • cargo test -q codex
  • cargo fmt --all --check
  • cargo clippy --all-targets
  • cargo test
  • git diff --check

Expected installed behavior:

default/no explicit allow
=> {}

allow rule Bash(git *)
=> permissionDecision=allow with updatedInput.command="rtk git status"

deny rule Bash(git status)
=> permissionDecision=deny with no updatedInput

ahundt added 5 commits June 23, 2026 19:11
Codex CLI added PreToolUse (Bash matcher) hook events in v0.117.0,
matching Claude Code's protocol byte-for-byte for both stdin payload
(tool_input.command + tool_name + hook_event_name) and the
hookSpecificOutput.permissionDecision/updatedInput response schema.
Until now RTK only integrated with Codex via AGENTS.md + RTK.md file
references, which left Claude/Gemini parity broken for the no-1-tool-
config case where users want RTK to rewrite Bash commands automatically.

Why this hook flow is safe alongside other Codex hooks:

  developers.openai.com/codex/hooks: "Matching hooks from multiple
  files all run. Multiple matching command hooks for the same event
  are launched concurrently, so one hook cannot prevent another
  matching hook from starting."

So unlike Claude rtk-ai#1515, Codex has no hook-collision bug to engineer
around. We APPEND a single PreToolUse entry to ~/.codex/hooks.json
and leave any foreign entries the user already has (audit loggers,
other rewrite tools, ...) untouched. Each RTK-owned hook entry
carries a `_rtk_managed: true` marker so uninstall can identify it
even after path edits.

Changes:

* src/hooks/hook_cmd.rs (+228 LOC, +14 tests, 0 failed):
  - new `run_codex()` reuses process_claude_payload() since the
    response schema matches; mirrors run_cursor's `{}` fail-open
    pattern (empty/parse-error/skip/ignore all emit "{}\n").
  - new `run_codex_inner` test helper + 13 unit tests covering:
    happy-path rewrite, schema parity with Claude (byte-for-byte
    assertion), non-Bash tool rejection, compound rewrites, env
    prefix preservation, substitution-skip CVE class, file-redirect
    skip, empty-command/missing-tool-input fallback, extra Codex
    session fields ignored.

* src/main.rs (+5 LOC):
  - new HookCommands::Codex variant routes to run_codex().

* src/hooks/constants.rs (+12 LOC):
  - CODEX_HOOK_COMMAND, CODEX_HOOKS_JSON, CODEX_HOOK_RTK_MARKER.

* src/hooks/init.rs (+822 LOC, +14 tests):
  - lift the `--codex cannot be combined with --auto-patch` and
    `--codex cannot be combined with --no-patch` restrictions at
    dispatch_init.
  - `--codex --auto-patch` now ALSO writes ~/.codex/hooks.json
    (or .codex/hooks.json in project mode) in addition to the
    existing AGENTS.md + RTK.md file integration.
  - `--codex --no-patch` and bare `--codex` keep file-only behavior
    exactly as before — backward compatible.
  - new install_codex_hook() + upsert_codex_hook_json() pair: pure
    JSON transform that's idempotent (same rtk path -> Unchanged),
    upgradeable (path moved -> Updated, no duplicate), and refuses
    to clobber malformed JSON instead of silently overwriting.
  - new remove_codex_hook_from_json() handles uninstall: strips
    only RTK-marked entries, collapses empty containers, returns
    Some("") when the file becomes RTK-only so the caller deletes
    the file rather than leaving an empty {} behind.
  - uninstall_codex_at() updated to also remove RTK's hook entry
    from hooks.json (file deleted if it becomes empty; foreign
    entries kept verbatim).
  - 14 new tests covering: empty file → Added, repeat → Unchanged,
    foreign entry coexistence, marker-based update detection,
    unmarked legacy entry upgrade, malformed JSON refusal, foreign-
    only remove → None (no-op), mixed remove → strips only rtk,
    rtk-only remove → empty signal, disk-level idempotence, foreign-
    only uninstall preserves bytes, double-uninstall idempotence.
  - existing test_codex_mode_rejects_auto_patch + _rejects_no_patch
    rewritten as test_codex_mode_accepts_* (living docs of the v3
    contract change).

Live smoke (release binary, sandboxed HOME):

  $ echo '{"hook_event_name":"PreToolUse","tool_name":"Bash",
          "tool_input":{"command":"git status"}}' | rtk hook codex
  {"hookSpecificOutput":{"hookEventName":"PreToolUse",
   "permissionDecisionReason":"RTK auto-rewrite",
   "updatedInput":{"command":"rtk git status"}}}

  $ HOME=$TMP CODEX_HOME=$TMP/.codex rtk init -g --codex --auto-patch
  RTK configured for Codex CLI.
    RTK.md:     $TMP/.codex/RTK.md
    AGENTS.md:  @RTK.md reference added
    hooks.json: PreToolUse hook installed
    Next: run `codex` and open `/hooks` to review & trust the new RTK hook.

  $ rtk init -g --codex --uninstall
  RTK uninstalled for Codex CLI:
    - RTK.md
    - AGENTS.md: removed @RTK.md reference
    - hooks.json: $TMP/.codex/hooks.json

Tests: 2158 passed, 1 pre-existing CVE failure
       (test_dollar_substitution_passthrough, baseline on upstream/develop @ 047f454)

Refs: developers.openai.com/codex/hooks (PreToolUse spec)
      Issue: this is the v3 follow-on to the Claude PreToolUse handler;
      Gemini handler also exists. Now all three RTK-supported agents
      (Claude/Codex/Gemini) have a native hook integration.
Codex rejects a PreToolUse response that carries updatedInput unless
permissionDecision is exactly "allow". RTK's Codex handler reused the
Claude ask-compatible shape, so default and ask rewrites produced
updatedInput without allow and Codex reported:

  PreToolUse hook returned updatedInput without permissionDecision:allow

Split the shared PreToolUse rendering by host policy. Claude still emits
an updated input without permissionDecision when it wants the native prompt,
while Codex returns neutral {} unless the command matched an explicit allow
rule and can carry both permissionDecision:"allow" and updatedInput.

Make the Codex tests deterministic by injecting permission verdicts instead
of depending on local Claude settings. Also expose rewrite_cmd's evaluator to
tests so explicit deny can be asserted to win before unattestable-command
passthrough.

Verified against Codex's parser in codex-rs/hooks/src/engine/output_parser.rs.
Codex supports PreToolUse denial as hookSpecificOutput.permissionDecision
"deny" with a non-empty permissionDecisionReason, and it must not include
updatedInput. RTK was still inheriting Claude's deny behavior, where a deny
verdict is skipped so native Claude or fallthrough handling can decide.

Add an explicit DenyHandling policy to the shared PreToolUse renderer.
Claude keeps the existing skip and fallthrough behavior, while Codex renders
a valid deny response with no updatedInput. Add a regression test for the
deny shape so future changes do not reintroduce an unsupported ask or allow
combination.
Table-drive Codex allow, skip, and invalid-payload coverage, and share hooks.json PreToolUse extraction helpers in init tests. The tests keep the protocol invariant explicit: updatedInput is emitted only with permissionDecision allow, and deny responses omit updatedInput.

Tests:
- rtk cargo test -q codex
- rtk cargo fmt --all --check
- rtk git diff --check

Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
When uninstalling the Codex hook from a hooks.json file that only contains the
RTK-managed entry, leave an empty JSON object instead of deleting the file. This
preserves the observable cleanup result without adding a new file-deletion site
that trips the repository semgrep policy.

The uninstall path still removes only RTK-managed Codex PreToolUse entries and
continues to preserve foreign hooks.

Verification:
- cargo fmt --all -- --check
- cargo test -q codex
- cargo check -q
- git diff --check
@gemini133 gemini133 closed this Jul 30, 2026
@gemini133
gemini133 deleted the v3/codex-hook branch July 30, 2026 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants