fix(core): emit hooks for apply_patch edits#18391
Conversation
6a53c79 to
7e25c99
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
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:
Our workflow depends on post-edit checks like:
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 So from a user/workflow perspective: this is not just a nice-to-have hook expansion. It’s the difference between:
If this lands and |
調査結果: 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>
abhinav-oai
left a comment
There was a problem hiding this comment.
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
7e25c99 to
17f9354
Compare
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.
17f9354 to
40c1a7f
Compare
|
I implemented the compatibility pieces for apply_patch hooks. The behavior now is:
I also exercised this locally and the hook config matched PreToolUse and PermissionRequest via |
Fixes #16732.
Why
apply_patchis Codex's primary file edit path, but it was not emittingPreToolUseorPostToolUsehook events. That meant hook-based policy, auditing, and write coordination could observe shell commands while missing the actual file mutation performed byapply_patch.The issue also exposed that the hook runtime serialized command hook payloads with
tool_name: "Bash"unconditionally. Even ifapply_patchsupplied 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
PreToolUseandPostToolUsepayload support toApplyPatchHandler.tool_input.commandfor both JSON/function and freeformapply_patchcalls.tool_name.Bashfor shell-like tools.tool_nameinto hook stdin instead of hardcodingBash.tool_namecan represent tools other thanBash.Verification
Added focused handler coverage for:
apply_patchcalls producing aPreToolUsepayload.apply_patchcalls producing aPreToolUsepayload.apply_patchoutput producing aPostToolUsepayload.exec_commandhandlers continuing to exposeBash.Added end-to-end hook coverage for:
PreToolUsehook matching^apply_patch$blocking the patch before the target file is created.PostToolUsehook matching^apply_patch$receiving the patch input and tool response, then adding context to the follow-up model request.PreToolUse/PostToolUsehook events.Also validated manually with a live
codex execsmoke test using an isolated temp workspace and tempCODEX_HOME. The smoke test confirmed that a realapply_patchedit emitsPreToolUse/PostToolUsewithtool_name: "apply_patch", a shell command still emitstool_name: "Bash", and a denyingPreToolUsehook prevents the blocked patch file from being created.