Skip to content

[codex] Move hook request plumbing into hook runtime#23388

Merged
pakrym-oai merged 9 commits into
mainfrom
pakrym/hook-runtime-stop-hook
May 19, 2026
Merged

[codex] Move hook request plumbing into hook runtime#23388
pakrym-oai merged 9 commits into
mainfrom
pakrym/hook-runtime-stop-hook

Conversation

@pakrym-oai
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai commented May 19, 2026

Why

run_turn was still hand-building hook payloads and lifecycle events for a couple of hook paths. Most hook call sites already delegate request construction and event emission to hook_runtime, which keeps turn orchestration focused on model-flow decisions rather than hook plumbing.

This also keeps the legacy after_agent message extraction next to the legacy hook dispatch instead of leaving response-item walking in run_turn.

What changed

  • Added run_stop_hooks in hook_runtime to build StopRequest, emit preview start events, run the hook, and emit completion events.
  • Added run_legacy_after_agent_hook in hook_runtime to build and dispatch the legacy AfterAgent hook payload, including extracting input messages from response items.
  • Updated run_turn to call the hook runtime helpers and keep only the resulting continuation/block/stop decisions inline.
  • Removed the repeated pending session-start hook check from the run loop.

Validation

  • cargo test -p codex-core hook_runtime

@pakrym-oai pakrym-oai marked this pull request as ready for review May 19, 2026 01:15
@pakrym-oai pakrym-oai requested a review from a team as a code owner May 19, 2026 01:15
@pakrym-oai pakrym-oai changed the title [codex] Move stop hook runtime into hook helpers [codex] Move hook request plumbing into hook runtime May 19, 2026
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 55827a3141

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread codex-rs/core/src/session/turn.rs
let hooks = sess.hooks();
emit_hook_started_events(sess, turn_context, hooks.preview_stop(&request)).await;
let mut outcome = hooks.run_stop(request).await;
emit_hook_completed_events(sess, turn_context, std::mem::take(&mut outcome.hook_events)).await;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is something strange here... if I get it correctly. run_stop_hooks emit the internal stuff, drain (with the take) and then return StopOutcome type (with hook_events now empty). So things are partially consumed but, from what I read, this wasn't doing that before

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, codex overcooked a bit trying to avoid a clone.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Don't think it hurts anything.

@pakrym-oai pakrym-oai merged commit 9289b7c into main May 19, 2026
45 of 47 checks passed
@pakrym-oai pakrym-oai deleted the pakrym/hook-runtime-stop-hook branch May 19, 2026 15:41
@github-actions github-actions Bot locked and limited conversation to collaborators May 19, 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.

2 participants