›_ Codex Issues
Codex Issues / #20204

Inconsistent PreToolUse hook coverage across tool handlers (most tools never emit hook events)

Open 💬 10 comments Opened Apr 29, 2026 by Harihara04sudhan
💡 Likely answer: A maintainer (github-actions[bot], contributor) responded on this thread — see the highlighted reply below.

Summary

codex-rs/core/src/tools/registry.rs dispatches PreToolUse / PermissionRequest / PostToolUse generically: any handler that returns Some(payload) from pre_tool_use_payload will surface a hook event. Today only shell (Bash), unified_exec, apply_patch, and mcp opt in. Every other tool handler in core/src/tools/handlers/ falls back to the trait default (None), so hooks never fire for those tools.

The result is that the hooks system, which is otherwise a clean general purpose interception mechanism, behaves as if it were shell and edit-only. Anyone building tooling on top of Codex hooks (observability, audit logging, policy enforcement, telemetry, debugging instrumentation, third-party integrations) silently loses coverage on a large portion of the agent's tool surface.

Affected handlers (don't currently emit hook events)

From codex-rs/core/src/tools/handlers/:

  • Filesystem reads: list_dir, view_image
  • MCP resource access: mcp_resource (asymmetric with mcp tool calls, which do emit)
  • Agent task plan / objectives: plan (update_plan), goal (create_goal / update_goal / get_goal)
  • Batch sub-agent orchestration: agent_jobs (spawn_agents_on_csv)
  • Tool discovery: tool_search, tool_suggest
  • Per-agent control: multi_agents/* (5 sub-handlers), multi_agents_v2/* (7 sub-handlers)
  • Web search: no handler implementation in core/src/web_search.rs

The asymmetry between apply_patch (file edits: emits hooks) and list_dir / view_image (file reads: silent) is particularly noticeable for any tool that wants a complete view of agent activity.

Why this matters

Codex hooks are a public extension point. Their value depends on coverage being consistent across tools. With today's gaps:

  • Observability tools cannot record what files the agent listed or what plans it updated.
  • Audit logs are incomplete: apply_patch is logged, but the update_plan that decided to apply the patch is not.
  • Policy / safety tooling cannot enforce constraints on agent reasoning surface (update_plan, goal) or on multi-agent orchestration.
  • Telemetry / debugging instrumentation has blind spots on the same boundaries.

This is a consistency gap in an existing public API surface.

Proposed change

Add pre_tool_use_payload implementations to the handlers above. The pattern is already established in apply_patch.rs:317-322 and mcp.rs. Each addition is 5 to 15 lines of Rust.

A working PoC is available on a fork branch:

The PoC covers 8 of the missing handlers:

  • list_dir.rs
  • view_image.rs
  • mcp_resource.rs
  • plan.rs
  • goal.rs
  • agent_jobs.rs
  • tool_search.rs
  • tool_suggest.rs

For handlers that wrap multiple sub-tools (mcp_resource, goal), the raw function arguments are forwarded as JSON. For handlers with a single args schema (list_dir, view_image, tool_search), the parsed fields are surfaced explicitly.

Diff size: 16 files changed, 503 insertions, 0 deletions (8 handler patches plus 8 test additions).

Tests: 10 new unit tests cover the new pre_tool_use_payload paths, asserting each handler returns the expected PreToolUsePayload shape (and None for non-Function payloads where applicable). Pattern follows apply_patch_tests.rs. Full codex-core lib test count: 1641 to 1651, all passing on the patched branch (after rebasing on current main).

Live verification: Built cargo build -p codex-cli --release and ran the patched binary in Codex Desktop. Captured a PreToolUse tool=update_plan hook event for the first time, confirming the dispatch path is correct end-to-end. Without this patch, update_plan calls go through silently.

Out of scope

Two pieces are intentionally deferred to keep the first PR reviewable:

  1. multi_agents/* (5 sub-handlers) and multi_agents_v2/* (7 sub-handlers) for fine-grained agent control. The big-picture orchestration entry (spawn_agents_on_csv) is already covered via agent_jobs.rs in this PR. Per-agent control can land in a focused follow-up.
  2. web_search: no handler implementation exists today; needs a new handler skeleton in core/src/web_search.rs. Better as its own issue + PR.

Tested against

  • openai/codex main (current head as of filing).
  • Codex CLI 0.125.0 baseline plus patched build.
  • Codex Desktop on macOS (Apple Silicon).

Open questions

  1. Is there an internal roadmap for hook coverage that this overlaps with?
  2. Are there any handlers on the deferred list (multi_agents, web_search) where you'd prefer not to emit hooks for design reasons (privacy, operational noise, etc.)?
  3. Preference on PR shape: single PR for all 8, or split by category?
  4. Naming convention for hook payloads: should update_plan's HookToolName be "update_plan" or pulled from the canonical tool registry name?

Happy to iterate on the design before any code lands.

View original on GitHub ↗

10 Comments

github-actions[bot] contributor · 2 months ago

Potential duplicates detected. Please review them and close your issue if it is a duplicate.

  • #18491

Powered by Codex Action

Harihara04sudhan · 2 months ago

Not a duplicate of #18491, though there is partial overlap. Reasons:

  1. #18491 makes two distinct asks: (a) expand PreToolUse to non-Bash tools, and (b) implement the updatedInput rewrite path in the hook response. This issue covers (a) only and does not propose any changes to updatedInput semantics.
  2. This issue is narrower and concrete: it lists 8 specific handlers (list_dir, view_image, mcp_resource, plan, goal, agent_jobs, tool_search, tool_suggest) with a working PoC, tests, and an explicit out-of-scope list (multi_agents/*, multi_agents_v2/*, web_search), rather than "all tool calls".
  3. The example list in #18491 (read_file, grep, apply_patch) is partially obsolete on current main: apply_patch already emits hook events.

The bot likely matched on shared keywords (PreToolUse, hooks, tool handlers). Happy to coordinate with #18491's author if maintainers prefer to consolidate, but flagging as a separate scope so the implementation half can move forward independently.

gabrimatic contributor · 2 months ago

I have a working fix for this on a fork branch.

Branch: https://github.com/gabrimatic/codex/tree/fix/expand-pre-tool-use-hook-coverage
Commit: https://github.com/gabrimatic/codex/commit/784846956c369ee287ae672c9ae31b5ae5081169
Diff vs openai/codex@main: https://github.com/openai/codex/compare/main...gabrimatic:codex:fix/expand-pre-tool-use-hook-coverage

Approach

Implemented pre_tool_use_payload for the eight handlers identified in the issue, following the existing pattern in apply_patch.rs:317-322 and mcp.rs:

  • list_dir
  • view_image
  • mcp_resource (asymmetric with mcp today; emits a payload per resolved tool name — list_mcp_resources / list_mcp_resource_templates / read_mcp_resource)
  • update_plan (plan.rs)
  • get_goal / create_goal / update_goal (goal.rs)
  • spawn_agents_on_csv (agent_jobs.rs)
  • tool_search
  • tool_suggest

The hook tool name is HookToolName::new(invocation.tool_name.display()), so handlers backed by multiple tool names report the specific tool that was invoked. The tool_input is the parsed JSON arguments, mirroring mcp.rs. A small shared helper, hook_tool_input_from_arguments, lives in tools/handlers/mod.rs: empty arguments become an empty object so hook scripts can rely on a JSON object shape, and inputs that don't parse as JSON are surfaced as a string fallback. Non-Function/ToolSearch payloads short-circuit to None, matching the existing unified_exec and mcp implementations.

No existing handler logic is modified — purely additive.

Tests

Each handler gets a unit test that asserts:

  • pre_tool_use_payload returns the expected tool_name and parsed tool_input for a Function (or ToolSearch) payload.
  • A non-matching payload variant returns None.

goal also locks in the empty-object fallback for get_goal (which has no arguments).

Verification

cargo test -p codex-core --lib -- tools::handlers     # 153 passed, 0 failed
cargo test -p codex-core --lib -- pre_tool_use        # 25 passed, 0 failed (8 new)
cargo clippy -p codex-core --tests --all-features -- -D warnings   # clean
cargo +nightly fmt --check                                          # clean

Scope

13 files, 589 insertions, 0 deletions. All additions, no modifications to existing logic.

Multi-agent v1/v2 sub-handlers and web_search (which has no handler implementation) are intentionally out of scope here — happy to follow up in a separate change if there's appetite. This change matches the eight-handler scope described in the issue.

Per the contributing guidelines I'm not opening a PR; please let me know if you'd like me to do so under invitation, or if a maintainer would like to land the fix directly from the branch.

oxysoft · 2 months ago

Indexed this hook ticket in the umbrella tracker: #21753

Goal: collect the scattered Codex hook requests and bugs into one parity matrix for Full Claude Code Hook Parity (29+), while preserving this issue as the detailed thread for its specific behavior.

AgenticThinkingUK · 2 months ago

This is the exact gap that makes hook based governance hard to trust.

If some tool paths emit hooks and others do not, external tools think they are seeing the agent action stream when they are only seeing part of it. That is worse than an unsupported surface because the blind spot is not obvious.

Two things would make this much safer:

  1. Each registered tool handler should either emit PreToolUse/PostToolUse or explicitly declare why it does not.
  2. Codex should expose a small runtime capability report so collectors can tell the difference between covered, unsupported, and known blind spot before trusting the event stream.

That would let Codex events map cleanly into a neutral runtime evidence envelope, including AgentHook compatible collectors, without pretending the hook surface is complete while coverage is still handler dependent.

Harihara04sudhan · 2 months ago

@gabrimatic Thank you for picking this up, your fork looks solid. The shared hook_tool_input_from_arguments helper is a much cleaner approach than my original per-handler PoC, and the per-tool-name dispatch in mcp_resource is exactly right.

I'd love to co-author with you on the PR upstream, with you as the primary author since the implementation is yours. To make that easy, here's what I can take off your plate:

Rebase onto current main. Your branch is now 424 commits behind upstream; I can rebase, resolve any conflicts (likely around the recent handler restructuring), and run the existing hook test suite to confirm coverage holds.
Add coverage tests. I have an integration test scaffold from my earlier PoC that exercises each of the 8 handlers and asserts the PreToolUse payload shape (tool name, parsed input). Happy to fold those in.
Open the PR and shepherd it through CI / review feedback.
If you'd prefer to drive the PR yourself, I'm equally happy to help in any other shape to review your branch, add tests in a follow-up commit, file companion documentation, whatever moves it forward. The goal is just getting hook parity merged so the blind-spot problem in #21753 (which @AgenticThinkingUK and others highlighted on the security side) closes for these 8 handlers.

Drop a comment or reach out to [harisudhan@armoriq.io](mailto:harisudhan@armoriq.io) or @Harihara04sudhan on GitHub and we can coordinate.

AgenticThinkingUK · 2 months ago

Good to see this moving.

From the runtime-evidence side, the key thing is full coverage across the tool surface, not just shell/edit paths. These handlers are exactly where audit, policy, and approval systems otherwise get blind spots.

Happy to test the PR against an AgentHook-compatible collector once it is open.

gabrimatic contributor · 2 months ago

Thanks @Harihara04sudhan, I appreciate it.

I rebased and ported the change onto current main here:
https://github.com/gabrimatic/codex/tree/fix/pre-tool-use-hook-coverage-current-main

The refreshed branch is 1 commit ahead of upstream main and no longer carries the old 424-commit drift. I also adjusted the implementation for the current per-tool handler layout: goal and MCP resource handlers are now split, request_plugin_install has replaced the old tool suggestion path, and list_dir is no longer present on current main.

Local verification:

  • just fmt
  • just fix -p codex-core
  • cargo test -p codex-core --lib -- pre_tool_use_payload (26 passed)

I did not open an upstream PR yet because docs/contributing.md says external PRs are by invitation only. If a maintainer is comfortable inviting this one, I can open the PR from the refreshed branch. I’m also happy to co-author with you: if you want to add your integration scaffold on top, the branch is ready for that.

Kaspre · 2 months ago

Adding one more handler to the missing-coverage list: Code Mode exec (codex-rs/core/src/tools/code_mode/execute_handler.rs at rust-v0.130.0).

CodeModeExecuteHandler::pre_tool_use_payload is unimplemented and inherits the default None from ToolHandler. Same root cause as the 8 handlers already in this issue, but worth calling out separately because Code Mode is the freeform JavaScript execution surface — a model dispatching tools.exec_command, tools.apply_patch, etc. through Code Mode silently bypasses any hook-based policy/audit/sandboxing that's in place for the direct shell/edit paths.

Probe verification: a model routing a SHA-256 prompt through Code Mode (custom_tool_call name: "exec", input = raw JS calling tools.exec_command) shows zero PreToolUse events fired, vs. the same intent routed via direct function_call: exec_command which DOES fire the hook correctly.

Proposed fix branch (3-file diff, ready to cherry-pick):

Adds:

  • pre_tool_use_payload on CodeModeExecuteHandler extracting raw JS from ToolPayload::Custom { input } as tool_input.command (same convention as apply_patch/shell_command/unified_exec)
  • HookToolName::code_mode_exec() constructor — canonical name code_mode_exec distinct from Bash, with exec as matcher alias (mirrors apply_patch's alias pattern)
  • Unit tests for both Custom and non-Custom payloads (revives the orphaned execute_handler_tests.rs file)

Happy to fold this into the in-flight branch (cc @gabrimatic, @Harihara04sudhan) — the patch is small and self-contained.

Ar9av · 1 month ago

For comparison baseline: agent-manual (https://github.com/Ar9av/agent-manual) documents hook coverage across Claude Code, Gemini CLI, and Codex.

Claude Code fires PreToolUse/PostToolUse consistently for all built-in tools including apply_patch (fixed in PR #18391) but coverage for MCP tools varies. Gemini CLI fires BeforeTool/AfterTool for both native and MCP tools and includes mcp_context and original_request_name fields in the stdin payload to distinguish them.

If the inconsistency here is specifically around MCP vs native tool hooks, Gemini CLI's approach of including MCP metadata in every tool hook (rather than separate event types) might be worth considering as a path forward.