core: support extension-owned turn items - #31283
Conversation
b6e8ca7 to
f15b9a9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8b56118f
ℹ️ 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".
| (item, Vec::new()) | ||
| } | ||
| }; | ||
| session.emit_turn_item_completed(turn.as_ref(), item).await; |
There was a problem hiding this comment.
Persist completed extension items for replay
When an extension emits a canonical item with no legacy fallback (the API allows legacy_events to be empty), this ItemCompleted(TurnItem::Extension) is delivered live but is dropped from rollout storage because rollout/src/policy.rs only keeps completed Plan/Sleep items. After resuming from that rollout, the extension-owned item is gone, so the new canonical extension item path is not durable unless every extension also invents a legacy event; include TurnItem::Extension(_) in the persisted completion policy.
AGENTS.md reference: AGENTS.md:L102-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
no, we are purposely not doing this in this PR and deferring it for #30188 - we'll be switching to ItemCompleted(<turn_item>) for paginated threads generally then
6466cfa to
022f3c0
Compare
| num_last_images_to_include: Option<usize>, | ||
| } | ||
|
|
||
| fn legacy_end_event(item: &ImageGenerationItem) -> EventMsg { |
There was a problem hiding this comment.
should this persist TurnItem::Extension and drop legacy events?
There was a problem hiding this comment.
will be doing as part of this PR! #30188
022f3c0 to
ca76515
Compare
Description
This PR adds a
codex-extension-itemscrate for extension-ownedTurnItemschemas, and updates standalone image generation to start using it viaTurnItem::Extension.This gives us a way to prevent Core from having to be aware of all extension items. App-server still exposes the existing public
ThreadItem::ImageGenerationshape, now by wrapping the same sharedimage_generation::ImageGenerationItemtype.The new
codex-extension-itemscrate is necessary because the image gen extension item is used by:codex-image-generation-extension, which produces it.codex-tools / core, which carry it generically.codex-protocol, which serializes it into lifecycle events and rollouts.app-server protocol, which wraps it in publicThreadItem::ImageGenerationWe keep the hosted Responses API image generation as
TurnItem::ImageGenerationbecause core still owns its persistence and legacy fanout.Before
Standalone image generation is implemented as an extension, but its item representation previously lived in the core protocol. This sets the precedent that core is aware of all extension items, which would be good to avoid.
After
Future extension items can have typed app-server APIs without adding a new
TurnItemvariant,ExtensionTurnItemvariant, or core emitter match arm.What changed
codex-extension-itemswith the closedExtensionItemenum and sharedimage_generation::ImageGenerationItemschema.TurnItem::Extension(ExtensionItem)andExtensionTurnItem::Extension { item, legacy_events }paths.ImageGenerationBegin/ImageGenerationEndevents explicitly.ItemStarted/ItemCompletedbefore extension-provided legacy events.Follow-up
Standalone web search still uses its typed special-case path. Migrating it later would let
ExtensionTurnItemcollapse into a single extension-item struct.