Skip to content

fix(core): emit hooks for apply_patch edits#18391

Merged
fcoury-oai merged 4 commits intomainfrom
fcoury/fix-apply-patch-hooks
Apr 22, 2026
Merged

fix(core): emit hooks for apply_patch edits#18391
fcoury-oai merged 4 commits intomainfrom
fcoury/fix-apply-patch-hooks

Conversation

@fcoury-oai
Copy link
Copy Markdown
Contributor

Fixes #16732.

Why

apply_patch is Codex's primary file edit path, but it was not emitting PreToolUse or PostToolUse hook events. That meant hook-based policy, auditing, and write coordination could observe shell commands while missing the actual file mutation performed by apply_patch.

The issue also exposed that the hook runtime serialized command hook payloads with tool_name: "Bash" unconditionally. Even if apply_patch supplied hook payloads, hooks would either fail to match it directly or receive misleading stdin that identified the edit as a Bash tool call.

What Changed

  • Added PreToolUse and PostToolUse payload support to ApplyPatchHandler.
  • Exposed the raw patch body as tool_input.command for both JSON/function and freeform apply_patch calls.
  • Taught tool hook payloads to carry a handler-supplied hook-facing tool_name.
  • Preserved existing shell compatibility by continuing to emit Bash for shell-like tools.
  • Serialized the selected hook tool_name into hook stdin instead of hardcoding Bash.
  • Relaxed the generated hook command input schema so tool_name can represent tools other than Bash.

Verification

Added focused handler coverage for:

  • JSON/function apply_patch calls producing a PreToolUse payload.
  • Freeform apply_patch calls producing a PreToolUse payload.
  • Successful apply_patch output producing a PostToolUse payload.
  • Shell and exec_command handlers continuing to expose Bash.

Added end-to-end hook coverage for:

  • A PreToolUse hook matching ^apply_patch$ blocking the patch before the target file is created.
  • A PostToolUse hook matching ^apply_patch$ receiving the patch input and tool response, then adding context to the follow-up model request.
  • Non-participating tools such as the plan tool continuing not to emit PreToolUse/PostToolUse hook events.

Also validated manually with a live codex exec smoke test using an isolated temp workspace and temp CODEX_HOME. The smoke test confirmed that a real apply_patch edit emits PreToolUse/PostToolUse with tool_name: "apply_patch", a shell command still emits tool_name: "Bash", and a denying PreToolUse hook prevents the blocked patch file from being created.

@fcoury-oai fcoury-oai force-pushed the fcoury/fix-apply-patch-hooks branch from 6a53c79 to 7e25c99 Compare April 17, 2026 20:42
@fcoury-oai
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@openai openai deleted a comment from Zoner12 Apr 17, 2026
Copy link
Copy Markdown

e-fu commented Apr 20, 2026

This PR would unblock an important real workflow for us.

We’ve been porting an Elixir/Phoenix hook setup from another agent into Codex.

What we confirmed locally:

  • PreToolUse for Bash is working in our environment.
  • We can visibly block mix test and mix dialyzer and force mix test.json / mix dialyzer.json instead.
  • Plugin skills are loading correctly.
  • The missing piece is edit-time validation after Codex file edits.

Our workflow depends on post-edit checks like:

  • mix format on the edited file
  • mix compile --warnings-as-errors
  • targeted mix test.json for the corresponding test file
  • framework-specific checks such as mix ash.codegen --check

Right now that workflow cannot be implemented cleanly because the file edit path does not emit hook events, so we had to fall back to a weaker Stop-hook design. That loses precision, runs later than we want, and is much less ergonomic for iterative coding.

So from a user/workflow perspective: this is not just a nice-to-have hook expansion. It’s the difference between:

  • immediate post-edit validation that keeps the loop tight
  • versus delayed whole-turn validation that is noisier and less targeted

If this lands and apply_patch emits PreToolUse / PostToolUse with a non-Bash tool_name, that would materially improve Codex for edit-centric code-quality workflows.

junpei-9898 pushed a commit to junpei-9898/phasegate that referenced this pull request Apr 20, 2026
調査結果: openai/codex#18391 が既に open + 1 approval + no blockers で merge 待ち状態。
内容的に ApplyPatchHandler が PreToolUse/PostToolUse hook を発火するよう修正済みで、
Phasegate が必要とする fix に一致。追加 PR は送らず、issue に実装履歴と上流 fix 後の
追従手順を記載。

- 実装履歴テーブルを追加 (Wave 1 ~ C-6 の版履歴)
- 上流 fix 後の Phasegate 側対応手順を記載 (テンプレート matcher 拡張 / payload 採取)
- 関連リンクに PR #18391 を追加

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread codex-rs/core/tests/suite/hooks.rs Outdated
Copy link
Copy Markdown
Collaborator

@abhinav-oai abhinav-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also wire up the PermissionRequest hook to be able to match the apply_patch tool?

I think we just need an impl of permission_request_payload() and minor schema change so the logic in tools/orchestrator.rs kicks in

@fcoury-oai fcoury-oai force-pushed the fcoury/fix-apply-patch-hooks branch from 7e25c99 to 17f9354 Compare April 21, 2026 21:39
@fcoury-oai fcoury-oai requested a review from a team as a code owner April 21, 2026 21:39
Teach apply_patch to provide PreToolUse and PostToolUse payloads for hook dispatch, covering both JSON and freeform patch calls.

Pass handler-supplied hook names through runtime serialization so existing Bash matchers keep working and apply_patch can be matched directly.
@fcoury-oai fcoury-oai force-pushed the fcoury/fix-apply-patch-hooks branch from 17f9354 to 40c1a7f Compare April 21, 2026 22:13
@fcoury-oai fcoury-oai requested a review from abhinav-oai April 21, 2026 22:30
@fcoury-oai
Copy link
Copy Markdown
Contributor Author

I implemented the compatibility pieces for apply_patch hooks.

The behavior now is:

  • hook stdin stays canonical: tool_name remains apply_patch
  • matcher selection recognizes apply_patch, Write, and Edit for apply_patch calls
  • a handler only runs once if its regex matches multiple aliases, like apply_patch|Write|Edit
  • PermissionRequest supports apply_patch, with the patch body in tool_input.command

I also exercised this locally and the hook config matched PreToolUse and PermissionRequest via ^Write$, and PostToolUse via ^Edit$, while all hook payloads still logged tool_name: apply_patch.

Copy link
Copy Markdown
Collaborator

@abhinav-oai abhinav-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@fcoury-oai fcoury-oai merged commit 09ebc34 into main Apr 22, 2026
25 checks passed
@fcoury-oai fcoury-oai deleted the fcoury/fix-apply-patch-hooks branch April 22, 2026 01:00
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ApplyPatchHandler doesn't emit PreToolUse/PostToolUse hook event. Hooks only fire for Bash tool.

5 participants