[codex] Implement remote thread store methods#19008
Conversation
![chatgpt-codex-connector[bot]](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: d663af75e8
ℹ️ 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".
| async fn state_db(&self, _thread_id: ThreadId) -> ThreadStoreResult<Option<StateDbHandle>> { | ||
| Err(not_implemented("state_db")) | ||
| Err(ThreadStoreError::Internal { | ||
| message: "remote thread store does not expose local state databases".to_string(), | ||
| }) |
There was a problem hiding this comment.
Ok(None) for remote local-only thread fields
When experimental_thread_store_endpoint is set, core now uses RemoteThreadStore, but state_db/rollout_path return ThreadStoreError::Internal. Session init calls these with ? (core/src/session/session.rs lines 300 and 376), so non-ephemeral thread startup fails immediately under remote thread store configuration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ack, it doesn't fully work yet with remote
| message: status.message().to_string(), | ||
| }, | ||
| tonic::Code::NotFound => ThreadStoreError::InvalidRequest { |
There was a problem hiding this comment.
remote_status_to_error converts tonic::Code::NotFound into InvalidRequest. thread/read uses ThreadNotFound as the non-error “missing persisted thread” path before falling back to live thread state (app-server/src/codex_message_processor.rs lines 3901-3911 and 3969-3973). This mapping turns missing persisted records into hard errors.
Useful? React with 👍 / 👎.
| if let Some(endpoint) = config.experimental_thread_store_endpoint.as_deref() { | ||
| warn!( | ||
| "experimental_thread_store_endpoint `{endpoint}` is configured, but remote thread write APIs are not implemented yet; using local thread store" | ||
| ); | ||
| return Arc::new(RemoteThreadStore::new(endpoint)); |
There was a problem hiding this comment.
ThreadManager now always selects RemoteThreadStore when the endpoint is set, but app-server resume logic still reconstructs history from local files (resume_thread_from_rollout uses find_thread_path_by_id_str + RolloutRecorder::get_rollout_history). In remote-only deployments, unloaded thread resume-by-id returns “no rollout found” and cannot recover persisted threads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
ack, it doesn't fully work yet with remote
wiltzius-openai
force-pushed
the
wiltzius/codex/thread-store-remote-impls
branch
from
d663af7 to
b9ee3bd
Compare
wiltzius-openai
force-pushed
the
wiltzius/codex/thread-store-remote-impls
branch
from
b9ee3bd to
2067566
Compare
wiltzius-openai
changed the base branch from
wiltzius/codex/thread-store-remote-base-9938
to
main
wiltzius-openai
force-pushed
the
wiltzius/codex/thread-store-remote-impls
branch
from
2067566 to
88f927a
Compare
2 participants
No description provided.