extension-api: add TurnItemEmitter to tool calls#24813
Conversation
|
@codex review this |
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e770f71c0
ℹ️ 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".
| let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else { | ||
| return; | ||
| }; | ||
| session.emit_turn_item_completed(turn.as_ref(), item).await; |
There was a problem hiding this comment.
This is only using the low-level event emitter, so extension-emitted items skip the existing turn-item finalization path (TurnItemContributors plus host-owned handling like image-generation save/instruction wiring). Can we route this through one host-owned publish/finalize path, or narrow the API to item types that are already fully finalized by the extension?
There was a problem hiding this comment.
good catch, ty.
narrowed api for now, left comment about future work to reuse core turnitem finalization
|
@codex review this |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ 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". |
Why
Extension-contributed tools need to emit visible turn items through Codex's normal event and persistence pipeline.
What
TurnItemEmitterto extensionToolCalls and route the core implementation throughSession::emit_turn_item_*.Test Plan
just test -p codex-core -E 'test(passes_turn_fields_and_scoped_turn_item_emitter_to_extension_call)'