Repository navigation
[codex] Add threadCatalog metadata subscriptions - #26009
btraut-openai wants to merge 7 commits into
Conversation
btraut-openai
changed the title
300f2d6 to
6880449
Compare
6880449 to
bbb721b
Compare
bbb721b to
7d62042
Compare
![chatgpt-codex-connector[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d620423f2
ℹ️ 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".
7d62042 to
e031dc6
Compare
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
e031dc6 to
6806826
Compare
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
3d970d4 to
7954ae9
Compare
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
7954ae9 to
f100d8f
Compare
btraut-openai
changed the title
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
f100d8f to
7baa30b
Compare
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
dc0c72d to
07dff7c
Compare
jif-oai
left a comment
jif-oai
left a comment
There was a problem hiding this comment.
This is getting large because catalog publication is spread across LiveThread, core, listener tasks, and individual request handlers. That makes the invariant difficult to make complete and ordered. We already miss direct store mutations and have multiple mapping/delivery paths.
Can we first introduce one store-owned catalog change stream (upsert/delete/...) or any equivalent. Then make app-server only handle subscriber fanout? wdyt?
07dff7c to
5c7d2ca
Compare
5815824 to
68ac705
Compare
|
Codex: > Can we first introduce one store-owned catalog change stream (upsert/delete/...) or any equivalent? I kept this PR surgical instead. A store-owned stream would require changing every store implementation and defining delete/tombstone semantics alongside the existing |
# Conflicts: # codex-rs/tui/src/app/app_server_event_targets.rs
Add revision-based bootstrap ordering, refresh spawned threads after listener attachment, preserve catalog notifications under in-process backpressure, and correct the generated recency timestamp type.
Carry root session identity through StoredThread summaries and make catalog unsubscribe wait for in-flight notification delivery.
68ac705 to
c1b099d
Compare
|
Codex: Closing this. Jif is right: the store should own one ordered catalog change stream, and app-server should only fan it out. This PR spread that invariant across too many mutation paths and stopped being reviewable. I’m going to split it into a store-owned change stream first, then a thin app-server subscription on top. |
|
Codex: First replacement is #29894. It is store-only and adds no app-server or schema surface. I’m leaving the fanout/API PR until this ownership boundary is reviewed. |
2 participants
Why
Sidebar clients have an awkward choice today: fetch one page with
thread/listand miss activity in threads outside that page, or resume every thread and pay for detailed runtime subscriptions they do not need.This adds a metadata-only catalog subscription so clients can keep a small paginated sidebar view while still hearing about future persisted thread changes.
thread/listremains the way to fetch existing catalog data;threadCatalog/subscribeis only for future metadata mutations.What changed
The app-server v2 protocol now exposes
threadCatalog/subscribe,threadCatalog/unsubscribe, andthreadCatalog/changed. Catalog change notifications carry a completeThreadSummarywith sidebar row metadata, including ids, preview/name, cwd, created/updated/recency timestamps in seconds and milliseconds, archive state, git info, source, and parent thread id when known. They intentionally exclude turns, items, messages, deltas, tool state, status, and loaded runtime state.The implementation publishes summaries from persisted thread creation and metadata update paths. Listing, reading, and resuming threads do not make them recent; meaningful thread activity and metadata mutations do. Ephemeral in-memory threads stay out of this catalog subscription rather than adding a second app-server-owned shadow catalog.
How it works
Think of
thread/listas asking, “What are the first N persisted rows in my sidebar right now?”threadCatalog/subscribeasks, “From now on, tell me when any persisted thread row changes, even if I did not load that row yet.”Clients should start buffering
threadCatalog/changednotifications before sending the subscribe request, wait for the subscribe response, callthread/list, then apply the buffered summaries idempotently. Reconnecting clients should resubscribe and refetch their visible page because catalog subscriptions do not replay offline changes.codex-apps follow-up should subscribe before the initial sidebar list, apply its existing filters when summaries arrive, upsert/reorder/remove rows on
threadCatalog/changed, unsubscribe when the view is disposed, and refetch on reconnect.Verification
Regenerated app-server schema fixtures and ran the focused protocol tests, app-server catalog subscription tests, and thread resume regression locally. Also ran scoped Rust fix/clippy for touched crates, Rust formatting, and whitespace checks.