Skip to content

Inspect turn hooks before runtime preparation#29945

Open
jif-oai wants to merge 1 commit into
jif/route-mcp-elicitation-idsfrom
jif/inspect-hooks-before-runtime-preparation
Open

Inspect turn hooks before runtime preparation#29945
jif-oai wants to merge 1 commit into
jif/route-mcp-elicitation-idsfrom
jif/inspect-hooks-before-runtime-preparation

Conversation

@jif-oai

@jif-oai jif-oai commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • split input-hook inspection from history recording
  • run session-start and user-input hooks before runtime preparation can begin
  • preserve the existing model-history order by buffering hook context until after initial context is recorded
  • flush inspected input on blocked and aborted setup paths

Why

A runtime snapshot must not publish for a turn that hooks stop before sampling. Separating inspection from recording establishes that decision boundary without rewriting history or moving hook context ahead of the normal initial developer context.

@jif-oai jif-oai requested a review from a team as a code owner June 25, 2026 01:32

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

Copy link
Copy Markdown
Contributor

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: e522746dcf

ℹ️ 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".

.await;
return Ok(None);
}
let (inspected_input, blocked_input) = inspect_inputs(&sess, &turn_context, &input).await;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Add integration coverage for hook reordering

This reorders SessionStart/UserPromptSubmit inspection relative to the rest of turn setup and adds blocked/aborted setup paths, but the commit only changes production code. Because this is agent turn logic, please add an integration test under core/tests/suite that exercises the new ordering and persistence behavior so future changes do not regress it.

AGENTS.md reference: AGENTS.md:L112-L118

Useful? React with 👍 / 👎.

{
accepted_user_input = true;
}
inspected_input.push((input_item, hook_outcome));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Preserve transcript order for queued prompt hooks

When two user prompts are queued while the model is running, this loop now buffers every hook outcome and record_inspected_inputs is called only after inspect_inputs returns. That means the UserPromptSubmit hook for the second queued prompt runs before the first accepted prompt has been written to history/transcript_path; previously each accepted item was recorded before inspecting the next one. Hooks that read the transcript to decide whether to allow/block later queued prompts can now make decisions against stale history.

Useful? React with 👍 / 👎.

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.

1 participant