mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
[codex] Route live thread writes through ThreadStore (#18882)
Begin migrating the thread write codepaths to ThreadStore. This starts using ThreadStore inside of core session code, not only in the app server code. Rework the interfaces around thread recording/persistence. We're left with the following: * `ThreadManager`: owns the process-level registry of loaded threads and handles cross-thread orchestration: start, resume, fork, lookup, remove, and route ops to running CodexThreads. * `CodexThread`: represents one loaded/running thread from the outside. It is the handle app-server and callers use to submit ops, inspect session metadata, and shut the thread down. * `LiveThread`: session-owned persistence lifecycle handle for one active thread. Core session code uses it to append rollout items, materialize lazy persistence, flush, shutdown, discard init-failed writers, and load that thread’s persisted history. * `ThreadStore`: storage backend abstraction. It answers “how are threads persisted, read, listed, updated, archived?” Local and remote implementations live behind this trait. * `LocalThreadStore`: local ThreadStore implementation. It owns the file/sqlite-specific details and keeps RolloutRecorder as a local implementation detail. This is a few too many Thread abstractions for my liking, but they do all represent different concepts / needs / layers. Migration note: in places where the core code explicitly requires a path, rather than a thread ID, throw an error if we're running with a remote store. Cover the new local live-writer lifecycle with focused tests and preserve app-server thread-start behavior, including ephemeral pathless sessions.
This commit is contained in:
23 files changed
+1789
-889
No files matched your search
@@ -5,7 +5,7 @@ use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::ThreadMemoryMode;
|
||||
use codex_protocol::protocol::ThreadNameUpdatedEvent;
|
||||
use codex_rollout::StateDbHandle;
|
||||
use codex_rollout::ARCHIVED_SESSIONS_SUBDIR;
|
||||
use codex_rollout::append_rollout_item_to_path;
|
||||
use codex_rollout::append_thread_name;
|
||||
use codex_rollout::find_archived_thread_path_by_id_str;
|
||||
@@ -13,6 +13,7 @@ use codex_rollout::find_thread_path_by_id_str;
|
||||
use codex_rollout::read_session_meta_line;
|
||||
|
||||
use super::LocalThreadStore;
|
||||
use super::live_writer;
|
||||
use crate::ReadThreadParams;
|
||||
use crate::StoredThread;
|
||||
use crate::ThreadStoreError;
|
||||
@@ -53,7 +54,7 @@ pub(super) async fn update_thread_metadata(
|
||||
.await?;
|
||||
}
|
||||
|
||||
let state_db_ctx = open_state_db_for_direct_thread_lookup(store).await;
|
||||
let state_db_ctx = store.state_db().await;
|
||||
codex_rollout::state_db::reconcile_rollout(
|
||||
state_db_ctx.as_deref(),
|
||||
resolved_rollout_path.path.as_path(),
|
||||
@@ -65,7 +66,7 @@ pub(super) async fn update_thread_metadata(
|
||||
)
|
||||
.await;
|
||||
|
||||
read_thread::read_thread(
|
||||
match read_thread::read_thread(
|
||||
store,
|
||||
ReadThreadParams {
|
||||
thread_id,
|
||||
@@ -74,6 +75,18 @@ pub(super) async fn update_thread_metadata(
|
||||
},
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(thread) => Ok(thread),
|
||||
Err(_) => {
|
||||
read_thread::read_thread_by_rollout_path(
|
||||
store,
|
||||
resolved_rollout_path.path,
|
||||
params.include_archived,
|
||||
/*include_history*/ false,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn apply_thread_name(
|
||||
@@ -127,15 +140,6 @@ async fn apply_thread_memory_mode(
|
||||
})
|
||||
}
|
||||
|
||||
async fn open_state_db_for_direct_thread_lookup(store: &LocalThreadStore) -> Option<StateDbHandle> {
|
||||
codex_state::StateRuntime::init(
|
||||
store.config.sqlite_home.clone(),
|
||||
store.config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.ok()
|
||||
}
|
||||
|
||||
fn memory_mode_as_str(mode: ThreadMemoryMode) -> &'static str {
|
||||
match mode {
|
||||
ThreadMemoryMode::Enabled => "enabled",
|
||||
@@ -148,6 +152,11 @@ async fn resolve_rollout_path(
|
||||
thread_id: ThreadId,
|
||||
include_archived: bool,
|
||||
) -> ThreadStoreResult<ResolvedRolloutPath> {
|
||||
if let Ok(path) = live_writer::rollout_path(store, thread_id).await {
|
||||
let archived = rollout_path_is_archived(store, path.as_path());
|
||||
return Ok(ResolvedRolloutPath { path, archived });
|
||||
}
|
||||
|
||||
let active_path =
|
||||
find_thread_path_by_id_str(store.config.codex_home.as_path(), &thread_id.to_string())
|
||||
.await
|
||||
@@ -179,6 +188,10 @@ async fn resolve_rollout_path(
|
||||
})
|
||||
}
|
||||
|
||||
fn rollout_path_is_archived(store: &LocalThreadStore, path: &std::path::Path) -> bool {
|
||||
path.starts_with(store.config.codex_home.join(ARCHIVED_SESSIONS_SUBDIR))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -187,6 +200,8 @@ mod tests {
|
||||
use uuid::Uuid;
|
||||
|
||||
use super::*;
|
||||
use crate::ResumeThreadParams;
|
||||
use crate::ThreadEventPersistenceMode;
|
||||
use crate::ThreadMetadataPatch;
|
||||
use crate::ThreadStore;
|
||||
use crate::local::LocalThreadStore;
|
||||
@@ -231,11 +246,18 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_sets_memory_mode_on_active_rollout() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let config = test_config(home.path());
|
||||
let store = LocalThreadStore::new(config.clone());
|
||||
let uuid = Uuid::from_u128(302);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path =
|
||||
write_session_file(home.path(), "2025-01-03T14-30-00", uuid).expect("session file");
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
home.path().to_path_buf(),
|
||||
config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
@@ -254,6 +276,51 @@ mod tests {
|
||||
assert_eq!(appended["type"], "session_meta");
|
||||
assert_eq!(appended["payload"]["id"], thread_id.to_string());
|
||||
assert_eq!(appended["payload"]["memory_mode"], "disabled");
|
||||
let memory_mode = runtime
|
||||
.get_thread_memory_mode(thread_id)
|
||||
.await
|
||||
.expect("thread memory mode should be readable");
|
||||
assert_eq!(memory_mode.as_deref(), Some("disabled"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_uses_live_rollout_path_for_external_resume() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let external_home = TempDir::new().expect("external temp dir");
|
||||
let store = LocalThreadStore::new(test_config(home.path()));
|
||||
let uuid = Uuid::from_u128(307);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let path = write_session_file(external_home.path(), "2025-01-03T14-45-00", uuid)
|
||||
.expect("external session file");
|
||||
|
||||
store
|
||||
.resume_thread(ResumeThreadParams {
|
||||
thread_id,
|
||||
rollout_path: Some(path.clone()),
|
||||
history: None,
|
||||
include_archived: true,
|
||||
event_persistence_mode: ThreadEventPersistenceMode::Limited,
|
||||
})
|
||||
.await
|
||||
.expect("resume external live thread");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
memory_mode: Some(ThreadMemoryMode::Disabled),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: false,
|
||||
})
|
||||
.await
|
||||
.expect("set memory mode on external live thread");
|
||||
|
||||
assert_eq!(thread.thread_id, thread_id);
|
||||
assert!(thread.rollout_path.is_some());
|
||||
let appended = last_rollout_item(path.as_path());
|
||||
assert_eq!(appended["type"], "session_meta");
|
||||
assert_eq!(appended["payload"]["memory_mode"], "disabled");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -385,6 +452,70 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thread_metadata_keeps_live_archived_thread_archived_in_sqlite() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let store = LocalThreadStore::new(config.clone());
|
||||
let uuid = Uuid::from_u128(308);
|
||||
let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id");
|
||||
let archived_path = write_archived_session_file(home.path(), "2025-01-03T16-30-00", uuid)
|
||||
.expect("archived session file");
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
home.path().to_path_buf(),
|
||||
config.model_provider_id.clone(),
|
||||
)
|
||||
.await
|
||||
.expect("state db should initialize");
|
||||
runtime
|
||||
.mark_backfill_complete(/*last_watermark*/ None)
|
||||
.await
|
||||
.expect("backfill should be complete");
|
||||
codex_rollout::state_db::reconcile_rollout(
|
||||
Some(runtime.as_ref()),
|
||||
archived_path.as_path(),
|
||||
config.model_provider_id.as_str(),
|
||||
/*builder*/ None,
|
||||
&[],
|
||||
/*archived_only*/ Some(true),
|
||||
/*new_thread_memory_mode*/ None,
|
||||
)
|
||||
.await;
|
||||
store
|
||||
.resume_thread(ResumeThreadParams {
|
||||
thread_id,
|
||||
rollout_path: Some(archived_path.clone()),
|
||||
history: None,
|
||||
include_archived: true,
|
||||
event_persistence_mode: ThreadEventPersistenceMode::Limited,
|
||||
})
|
||||
.await
|
||||
.expect("resume archived live thread");
|
||||
|
||||
let thread = store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some("Live archived title".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: true,
|
||||
})
|
||||
.await
|
||||
.expect("set archived thread name");
|
||||
|
||||
assert!(thread.archived_at.is_some());
|
||||
assert!(
|
||||
runtime
|
||||
.get_thread(thread_id)
|
||||
.await
|
||||
.expect("get metadata")
|
||||
.expect("metadata")
|
||||
.archived_at
|
||||
.is_some()
|
||||
);
|
||||
}
|
||||
|
||||
fn last_rollout_item(path: &std::path::Path) -> Value {
|
||||
let last_line = std::fs::read_to_string(path)
|
||||
.expect("read rollout")
|
||||
|
||||
Reference in new issue
Block a user