Add experimental remote thread store config#18714
Conversation
2930d1d to
6205503
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6205503298
ℹ️ 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".
| match config.experimental_thread_store_endpoint.as_deref() { | ||
| Some(endpoint) => Arc::new(RemoteThreadStore::new(endpoint)), |
There was a problem hiding this comment.
Limit remote store selection to supported thread operations
configured_thread_store swaps in RemoteThreadStore for all thread APIs whenever experimental_thread_store_endpoint is set. But RemoteThreadStore currently only implements list_threads; methods like read_thread, update_thread_metadata, archive_thread, and unarchive_thread return not_implemented errors (thread-store/src/remote/mod.rs). This makes endpoints such as thread/read and thread/archive fail once the config is enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is a work in progress, it's OK for there to be failures in the remote case
| analytics_events_client, | ||
| arg0_paths, | ||
| thread_store: LocalThreadStore::new(codex_rollout::RolloutConfig::from_view(&config)), | ||
| thread_store: configured_thread_store(&config), |
There was a problem hiding this comment.
Route thread persistence to the configured remote store
The new config only changes CodexMessageProcessor.thread_store, but thread creation/runtime still persists through core sessions that hardcode LocalThreadStore::new(...) (core/src/session/session.rs). With experimental_thread_store_endpoint enabled, thread/start writes local state while thread/list/thread/read pull from remote, so newly updated threads can be missing or stale in API responses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is a work in progress, not done migrating the thread writes yet
6205503 to
d035ab9
Compare
Add experimental config to use remote thread store rather than local thread store implementation in app server