From fe05acad23cb30fabcd942fbaabac358c32afd8b Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 30 Apr 2026 21:24:59 -0700 Subject: [PATCH 01/29] Make thread store process-scoped (#19474) - Build one app-server process ThreadStore from startup config and share it with ThreadManager and CodexMessageProcessor. - Remove per-thread/fork store reconstruction so effective thread config cannot switch the persistence backend. - Add params to ThreadStore create/resume for specifying thread metadata, since otherwise the metadata from store creation would be used (incorrectly). --- .../app-server/src/bespoke_event_handling.rs | 7 +- .../app-server/src/codex_message_processor.rs | 94 ++++---- codex-rs/app-server/src/message_processor.rs | 7 + .../app-server/tests/suite/v2/thread_read.rs | 7 + .../tests/suite/v2/thread_resume.rs | 73 +++++-- codex-rs/core/src/agent/control.rs | 74 +++---- codex-rs/core/src/agent/control_tests.rs | 18 +- codex-rs/core/src/codex_thread.rs | 12 ++ codex-rs/core/src/personality_migration.rs | 7 +- codex-rs/core/src/prompt_debug.rs | 4 +- codex-rs/core/src/session/mod.rs | 2 + codex-rs/core/src/session/session.rs | 18 ++ codex-rs/core/src/session/tests.rs | 31 ++- .../core/src/session/tests/guardian_tests.rs | 2 +- codex-rs/core/src/test_support.rs | 6 +- codex-rs/core/src/thread_manager.rs | 159 ++++++++------ codex-rs/core/src/thread_manager_tests.rs | 142 +++++++++++-- .../src/tools/handlers/multi_agents_tests.rs | 172 ++++----------- codex-rs/core/tests/common/test_codex.rs | 11 +- codex-rs/core/tests/suite/client.rs | 3 +- codex-rs/core/tests/suite/code_mode.rs | 1 - codex-rs/core/tests/suite/compact_remote.rs | 1 - .../core/tests/suite/compact_resume_fork.rs | 12 +- codex-rs/core/tests/suite/fork_thread.rs | 3 - .../core/tests/suite/permissions_messages.rs | 1 - .../core/tests/suite/realtime_conversation.rs | 1 - codex-rs/core/tests/suite/resume_warning.rs | 1 - codex-rs/core/tests/suite/search_tool.rs | 1 - codex-rs/core/tests/suite/skills.rs | 5 +- .../tests/suite/unstable_features_warning.rs | 2 - codex-rs/core/tests/suite/window_headers.rs | 1 - codex-rs/mcp-server/src/codex_tool_runner.rs | 6 +- codex-rs/mcp-server/src/message_processor.rs | 2 + codex-rs/memories/write/src/runtime.rs | 2 - codex-rs/rollout/src/metadata.rs | 26 +-- codex-rs/rollout/src/metadata_tests.rs | 20 +- codex-rs/rollout/src/state_db.rs | 35 ++- codex-rs/thread-manager-sample/src/main.rs | 3 +- codex-rs/thread-store/src/lib.rs | 2 + .../thread-store/src/local/archive_thread.rs | 8 +- .../thread-store/src/local/create_thread.rs | 18 +- codex-rs/thread-store/src/local/helpers.rs | 8 + .../thread-store/src/local/list_threads.rs | 27 ++- .../thread-store/src/local/live_writer.rs | 18 +- codex-rs/thread-store/src/local/mod.rs | 200 +++++++++++++++++- .../thread-store/src/local/read_thread.rs | 115 +++++----- .../thread-store/src/local/test_support.rs | 11 +- .../src/local/unarchive_thread.rs | 10 +- .../src/local/update_thread_metadata.rs | 23 +- codex-rs/thread-store/src/remote/helpers.rs | 7 + codex-rs/thread-store/src/remote/mod.rs | 147 +++++++++++++ .../remote/proto/codex.thread_store.v1.proto | 2 + .../src/remote/proto/codex.thread_store.v1.rs | 4 + codex-rs/thread-store/src/types.rs | 17 ++ codex-rs/tui/src/app/tests.rs | 1 + 55 files changed, 1076 insertions(+), 514 deletions(-) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index e8a6cb9cc0..628034da72 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -2687,12 +2687,7 @@ mod tests { thread_id: conversation_id, thread: conversation, .. - } = thread_manager - .start_thread( - config.clone(), - codex_core::thread_store_from_config(&config), - ) - .await?; + } = thread_manager.start_thread(config.clone()).await?; let thread_state = new_thread_state(); let thread_watch_manager = ThreadWatchManager::new(); let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY); diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c447d2c576..55b7166862 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -259,7 +259,6 @@ use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::NetworkProxyAuditMetadata; -use codex_core::config::ThreadStoreConfig; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::exec::ExecCapturePolicy; @@ -378,12 +377,10 @@ use codex_state::ThreadMetadata; use codex_state::ThreadMetadataBuilder; use codex_state::log_db::LogDbLayer; use codex_thread_store::ArchiveThreadParams as StoreArchiveThreadParams; -use codex_thread_store::InMemoryThreadStore; use codex_thread_store::ListThreadsParams as StoreListThreadsParams; use codex_thread_store::LocalThreadStore; use codex_thread_store::ReadThreadByRolloutPathParams as StoreReadThreadByRolloutPathParams; use codex_thread_store::ReadThreadParams as StoreReadThreadParams; -use codex_thread_store::RemoteThreadStore; use codex_thread_store::SortDirection as StoreSortDirection; use codex_thread_store::StoredThread; use codex_thread_store::ThreadMetadataPatch as StoreThreadMetadataPatch; @@ -691,18 +688,11 @@ pub(crate) struct CodexMessageProcessorArgs { /// go through `config_manager`. pub(crate) config: Arc, pub(crate) config_manager: ConfigManager, + pub(crate) thread_store: Arc, pub(crate) feedback: CodexFeedback, pub(crate) log_db: Option, } -fn thread_store_from_config(config: &Config) -> Arc { - match &config.experimental_thread_store { - ThreadStoreConfig::Local => Arc::new(configured_local_thread_store(config)), - ThreadStoreConfig::Remote { endpoint } => Arc::new(RemoteThreadStore::new(endpoint)), - ThreadStoreConfig::InMemory { id } => InMemoryThreadStore::for_id(id), - } -} - fn environment_selection_error_message(err: CodexErr) -> String { match err { CodexErr::InvalidRequest(message) => message, @@ -710,10 +700,6 @@ fn environment_selection_error_message(err: CodexErr) -> String { } } -fn configured_local_thread_store(config: &Config) -> LocalThreadStore { - LocalThreadStore::new(codex_rollout::RolloutConfig::from_view(config)) -} - impl CodexMessageProcessor { async fn instruction_sources_from_config(config: &Config) -> Vec { codex_core::AgentsMdManager::new(config) @@ -830,6 +816,7 @@ impl CodexMessageProcessor { arg0_paths, config, config_manager, + thread_store, feedback, log_db, } = args; @@ -839,7 +826,7 @@ impl CodexMessageProcessor { outgoing: outgoing.clone(), analytics_events_client, arg0_paths, - thread_store: thread_store_from_config(&config), + thread_store, config, config_manager, active_login: Arc::new(Mutex::new(None)), @@ -2586,7 +2573,6 @@ impl CodexMessageProcessor { let imported_thread = self .thread_manager .start_thread_with_options(StartThreadOptions { - thread_store: thread_store_from_config(&config), config, initial_history: InitialHistory::Forked(rollout_items), session_source: None, @@ -2784,7 +2770,6 @@ impl CodexMessageProcessor { } = listener_task_context .thread_manager .start_thread_with_options(StartThreadOptions { - thread_store: thread_store_from_config(&config), config, initial_history: match session_start_source .unwrap_or(codex_app_server_protocol::ThreadStartSource::Startup) @@ -4348,7 +4333,6 @@ impl CodexMessageProcessor { .thread_manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), thread_history, self.auth_manager.clone(), persist_extended_history, @@ -4503,27 +4487,20 @@ impl CodexMessageProcessor { request_id: &ConnectionRequestId, params: &ThreadResumeParams, ) -> Result { - if let Ok(existing_thread_id) = ThreadId::from_string(¶ms.thread_id) - && let Ok(existing_thread) = self.thread_manager.get_thread(existing_thread_id).await - { - if params.history.is_some() { + let running_thread = if params.history.is_some() { + if let Ok(existing_thread_id) = ThreadId::from_string(¶ms.thread_id) + && self + .thread_manager + .get_thread(existing_thread_id) + .await + .is_ok() + { return Err(invalid_request(format!( "cannot resume thread {existing_thread_id} with history while it is already running" ))); } - - if let (Some(requested_path), Some(active_path)) = ( - params.path.as_ref(), - existing_thread.rollout_path().as_ref(), - ) && requested_path != active_path - { - return Err(invalid_request(format!( - "cannot resume running thread {existing_thread_id} with mismatched path: requested `{}`, active `{}`", - requested_path.display(), - active_path.display() - ))); - } - + None + } else if params.path.is_some() { let source_thread = self .read_stored_thread_for_resume( ¶ms.thread_id, @@ -4531,12 +4508,45 @@ impl CodexMessageProcessor { /*include_history*/ true, ) .await?; + let existing_thread_id = source_thread.thread_id; + if let Ok(existing_thread) = self.thread_manager.get_thread(existing_thread_id).await { + if let (Some(requested_path), Some(active_path)) = ( + params.path.as_ref(), + existing_thread.rollout_path().as_ref(), + ) && requested_path != active_path + { + return Err(invalid_request(format!( + "cannot resume running thread {existing_thread_id} with stale path: requested `{}`, active `{}`", + requested_path.display(), + active_path.display() + ))); + } + Some((existing_thread_id, existing_thread, source_thread)) + } else { + None + } + } else if let Ok(existing_thread_id) = ThreadId::from_string(¶ms.thread_id) + && let Ok(existing_thread) = self.thread_manager.get_thread(existing_thread_id).await + { + let source_thread = self + .read_stored_thread_for_resume( + ¶ms.thread_id, + /*path*/ None, + /*include_history*/ true, + ) + .await?; if source_thread.thread_id != existing_thread_id { return Err(invalid_request(format!( "cannot resume running thread {existing_thread_id} from source thread {}", source_thread.thread_id ))); } + Some((existing_thread_id, existing_thread, source_thread)) + } else { + None + }; + + if let Some((existing_thread_id, existing_thread, source_thread)) = running_thread { let history_items = source_thread .history .as_ref() @@ -4731,11 +4741,10 @@ impl CodexMessageProcessor { async fn read_stored_thread_for_new_fork( &self, - thread_store: &dyn ThreadStore, thread_id: ThreadId, include_history: bool, ) -> Result { - thread_store + self.thread_store .read_thread(StoreReadThreadParams { thread_id, include_archived: true, @@ -4938,7 +4947,6 @@ impl CodexMessageProcessor { let fallback_model_provider = config.model_provider_id.clone(); let instruction_sources = Self::instruction_sources_from_config(&config).await; - let fork_thread_store = thread_store_from_config(&config); let NewThread { thread_id, @@ -4950,7 +4958,6 @@ impl CodexMessageProcessor { .fork_thread_from_history( ForkSnapshot::Interrupted, config, - fork_thread_store.clone(), InitialHistory::Resumed(ResumedHistory { conversation_id: source_thread_id, history: history_items.clone(), @@ -4986,11 +4993,7 @@ impl CodexMessageProcessor { let mut thread = if let Some(fork_rollout_path) = session_configured.rollout_path.as_ref() { let stored_thread = self - .read_stored_thread_for_new_fork( - fork_thread_store.as_ref(), - thread_id, - include_turns, - ) + .read_stored_thread_for_new_fork(thread_id, include_turns) .await?; self.stored_thread_to_api_thread( stored_thread, @@ -7250,7 +7253,6 @@ impl CodexMessageProcessor { .fork_thread( ForkSnapshot::Interrupted, config.clone(), - thread_store_from_config(&config), rollout_path, /*persist_extended_history*/ false, self.request_trace_context(request_id).await, diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 0480aba8c8..7b394c3d8c 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -65,6 +65,7 @@ use codex_arg0::Arg0DispatchPaths; use codex_chatgpt::connectors; use codex_core::ThreadManager; use codex_core::config::Config; +use codex_core::thread_store_from_config; use codex_exec_server::EnvironmentManager; use codex_features::Feature; use codex_feedback::CodexFeedback; @@ -285,12 +286,17 @@ impl MessageProcessor { auth_manager.set_external_auth(Arc::new(ExternalAuthRefreshBridge { outgoing: outgoing.clone(), })); + // The thread store is intentionally process-scoped. Config reloads can + // affect per-thread behavior, but they must not move newly started, + // resumed, or forked threads to a different persistence backend/root. + let thread_store = thread_store_from_config(config.as_ref()); let thread_manager = Arc::new(ThreadManager::new( config.as_ref(), auth_manager.clone(), session_source, environment_manager, Some(analytics_events_client.clone()), + Arc::clone(&thread_store), )); thread_manager .plugins_manager() @@ -304,6 +310,7 @@ impl MessageProcessor { arg0_paths, config: Arc::clone(&config), config_manager: config_manager.clone(), + thread_store, feedback, log_db, }); diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 8427b482be..feedded6f4 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -48,6 +48,7 @@ use codex_protocol::models::BaseInstructions; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource as ProtocolSessionSource; +use codex_protocol::protocol::ThreadMemoryMode; use codex_protocol::protocol::UserMessageEvent; use codex_protocol::user_input::ByteRange; use codex_protocol::user_input::TextElement; @@ -56,6 +57,7 @@ use codex_thread_store::CreateThreadParams; use codex_thread_store::InMemoryThreadStore; use codex_thread_store::ThreadEventPersistenceMode; use codex_thread_store::ThreadMetadataPatch; +use codex_thread_store::ThreadPersistenceMetadata; use codex_thread_store::ThreadStore; use codex_thread_store::UpdateThreadMetadataParams; use core_test_support::responses; @@ -1028,6 +1030,11 @@ async fn seed_pathless_store_thread( source: ProtocolSessionSource::Cli, base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + metadata: ThreadPersistenceMetadata { + cwd: None, + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Disabled, + }, event_persistence_mode: ThreadEventPersistenceMode::default(), }) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index d9f5f039de..48673387b8 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -71,6 +71,7 @@ use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; use serde_json::json; use std::fs::FileTimes; +use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -1669,7 +1670,7 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { } #[tokio::test] -async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Result<()> { +async fn thread_resume_uses_path_over_thread_id_when_thread_is_running() -> Result<()> { let server = responses::start_mock_server().await; let first_body = responses::sse(vec![ responses::ev_response_created("resp-1"), @@ -1749,24 +1750,71 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul ) .await??; - let resume_id = primary + let other_thread_id = ThreadId::new().to_string(); + let stale_path = rollout_path(codex_home.path(), "2025-01-01T00-00-00", &thread_id); + std::fs::create_dir_all(stale_path.parent().expect("stale path parent"))?; + let thread_uuid = Uuid::parse_str(&thread_id)?; + let mut stale_file = std::fs::File::create(&stale_path)?; + let stale_meta = json!({ + "timestamp": "2025-01-01T00:00:00Z", + "type": "session_meta", + "payload": { + "id": thread_uuid, + "timestamp": "2025-01-01T00:00:00Z", + "cwd": codex_home.path(), + "originator": "test_originator", + "cli_version": "test_version", + "source": "cli", + "model_provider": "test-provider", + }, + }); + writeln!(stale_file, "{stale_meta}")?; + let stale_user_event = json!({ + "timestamp": "2025-01-01T00:00:00Z", + "type": "event_msg", + "payload": { + "type": "user_message", + "message": "stale history", + "kind": "plain", + }, + }); + writeln!(stale_file, "{stale_user_event}")?; + + let stale_resume_id = primary .send_thread_resume_request(ThreadResumeParams { - thread_id: thread_id.clone(), - path: Some(PathBuf::from("/tmp/does-not-match-running-rollout.jsonl")), + thread_id: other_thread_id.clone(), + path: Some(stale_path), ..Default::default() }) .await?; - let resume_err: JSONRPCError = timeout( + let stale_resume_err: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, - primary.read_stream_until_error_message(RequestId::Integer(resume_id)), + primary.read_stream_until_error_message(RequestId::Integer(stale_resume_id)), ) .await??; assert!( - resume_err.error.message.contains("mismatched path"), + stale_resume_err.error.message.contains("stale path"), "unexpected resume error: {}", - resume_err.error.message + stale_resume_err.error.message ); + let resume_by_path_id = primary + .send_thread_resume_request(ThreadResumeParams { + thread_id: other_thread_id.clone(), + path: thread.path, + ..Default::default() + }) + .await?; + let resume_by_path_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + primary.read_stream_until_response_message(RequestId::Integer(resume_by_path_id)), + ) + .await??; + let ThreadResumeResponse { + thread: resumed, .. + } = to_response::(resume_by_path_resp)?; + assert_eq!(resumed.id, thread_id); + primary .interrupt_turn_and_wait_for_aborted(thread_id, running_turn.id, DEFAULT_READ_TIMEOUT) .await?; @@ -2463,7 +2511,7 @@ async fn thread_resume_surfaces_cloud_requirements_load_errors() -> Result<()> { } #[tokio::test] -async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { +async fn thread_resume_uses_path_over_invalid_thread_id() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; @@ -2523,13 +2571,6 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { thread: resumed, .. } = to_response::(resume_resp)?; assert_eq!(resumed.id, thread.id); - let resumed_path = resumed.path.as_ref().expect("resumed thread path"); - let original_path = thread.path.as_ref().expect("original thread path"); - assert_eq!( - normalized_existing_path(resumed_path)?, - normalized_existing_path(original_path)? - ); - assert_eq!(resumed.status, ThreadStatus::Idle); Ok(()) } diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 09a9d4c148..705d2d168f 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -5,16 +5,12 @@ use crate::agent::role::DEFAULT_ROLE_NAME; use crate::agent::role::resolve_role_config; use crate::agent::status::is_final; use crate::codex_thread::ThreadConfigSnapshot; -use crate::find_archived_thread_path_by_id_str; -use crate::find_thread_path_by_id_str; -use crate::rollout::RolloutRecorder; use crate::session::emit_subagent_session_started; use crate::session_prefix::format_subagent_context_line; use crate::session_prefix::format_subagent_notification_message; use crate::shell_snapshot::ShellSnapshot; -use crate::thread_manager::ResumeThreadFromRolloutOptions; +use crate::thread_manager::ResumeThreadWithHistoryOptions; use crate::thread_manager::ThreadManagerState; -use crate::thread_manager::thread_store_from_config; use crate::thread_rollout_truncation::truncate_rollout_to_last_n_fork_turns; use codex_features::Feature; use codex_protocol::AgentPath; @@ -27,6 +23,7 @@ use codex_protocol::models::ResponseItem; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::InterAgentCommunication; use codex_protocol::protocol::Op; +use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -34,6 +31,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::user_input::UserInput; use codex_rollout::state_db; use codex_state::DirectionalThreadSpawnEdgeStatus; +use codex_thread_store::ReadThreadParams; use serde::Serialize; use std::collections::HashMap; use std::collections::VecDeque; @@ -235,7 +233,6 @@ impl AgentControl { state .spawn_new_thread_with_source( config.clone(), - thread_store_from_config(&config), self.clone(), session_source, /*persist_extended_history*/ false, @@ -246,15 +243,7 @@ impl AgentControl { ) .await? } - (None, _) => { - state - .spawn_new_thread( - config.clone(), - thread_store_from_config(&config), - self.clone(), - ) - .await? - } + (None, _) => state.spawn_new_thread(config.clone(), self.clone()).await?, }; agent_metadata.agent_id = Some(new_thread.thread_id); reservation.commit(agent_metadata.clone()); @@ -377,23 +366,21 @@ impl AgentControl { parent_thread.codex.session.flush_rollout().await?; } - let rollout_path = parent_thread - .as_ref() - .and_then(|parent_thread| parent_thread.rollout_path()) - .or(find_thread_path_by_id_str( - config.codex_home.as_path(), - &parent_thread_id.to_string(), - ) - .await?) + let parent_history = state + .read_stored_thread(ReadThreadParams { + thread_id: parent_thread_id, + include_archived: true, + include_history: true, + }) + .await? + .history .ok_or_else(|| { CodexErr::Fatal(format!( - "parent thread rollout unavailable for fork: {parent_thread_id}" + "parent thread history unavailable for fork: {parent_thread_id}" )) })?; - let mut forked_rollout_items = RolloutRecorder::get_rollout_history(&rollout_path) - .await? - .get_rollout_items(); + let mut forked_rollout_items = parent_history.items; if let SpawnAgentForkMode::LastNTurns(last_n_turns) = fork_mode { forked_rollout_items = truncate_rollout_to_last_n_fork_turns(&forked_rollout_items, *last_n_turns); @@ -436,7 +423,6 @@ impl AgentControl { state .fork_thread_with_source( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(forked_rollout_items), self.clone(), session_source, @@ -576,24 +562,26 @@ impl AgentControl { let inherited_exec_policy = self .inherited_exec_policy_for_source(&state, Some(&session_source), &config) .await; - let rollout_path = - match find_thread_path_by_id_str(config.codex_home.as_path(), &thread_id.to_string()) - .await? - { - Some(rollout_path) => rollout_path, - None => find_archived_thread_path_by_id_str( - config.codex_home.as_path(), - &thread_id.to_string(), - ) - .await? - .ok_or_else(|| CodexErr::ThreadNotFound(thread_id))?, - }; + let stored_thread = state + .read_stored_thread(ReadThreadParams { + thread_id, + include_archived: true, + include_history: true, + }) + .await?; + let history = stored_thread + .history + .ok_or_else(|| CodexErr::ThreadNotFound(thread_id))? + .items; let resumed_thread = state - .resume_thread_from_rollout_with_source(ResumeThreadFromRolloutOptions { + .resume_thread_with_history_with_source(ResumeThreadWithHistoryOptions { config: config.clone(), - thread_store: thread_store_from_config(&config), - rollout_path, + initial_history: InitialHistory::Resumed(ResumedHistory { + conversation_id: thread_id, + history, + rollout_path: stored_thread.rollout_path, + }), agent_control: self.clone(), session_source, inherited_shell_snapshot, diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 6a86000f96..7ef2120d5c 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -7,7 +7,6 @@ use crate::config::Config; use crate::config::ConfigBuilder; use crate::context::ContextualUserFragment; use crate::context::SubagentNotification; -use crate::thread_manager::thread_store_from_config; use assert_matches::assert_matches; use codex_features::Feature; use codex_login::CodexAuth; @@ -27,6 +26,7 @@ use codex_protocol::protocol::TurnCompleteEvent; use codex_protocol::protocol::TurnStartedEvent; use codex_thread_store::ArchiveThreadParams; use codex_thread_store::LocalThreadStore; +use codex_thread_store::LocalThreadStoreConfig; use codex_thread_store::ThreadStore; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -109,7 +109,7 @@ impl AgentControlHarness { async fn start_thread(&self) -> (ThreadId, Arc) { let new_thread = self .manager - .start_thread(self.config.clone(), thread_store_from_config(&self.config)) + .start_thread(self.config.clone()) .await .expect("start thread"); (new_thread.thread_id, new_thread.thread) @@ -610,10 +610,7 @@ async fn spawn_agent_can_fork_parent_thread_history_with_sanitized_items() { Some("Child subagent guidance.".to_string()); let new_thread = harness .manager - .start_thread( - parent_config.clone(), - thread_store_from_config(&parent_config), - ) + .start_thread(parent_config.clone()) .await .expect("start parent thread"); let parent_thread_id = new_thread.thread_id; @@ -956,7 +953,7 @@ async fn spawn_agent_respects_max_threads_limit() { let control = manager.agent_control(); let _ = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); @@ -1313,10 +1310,7 @@ async fn multi_agent_v2_completion_queues_message_for_direct_parent() { let _ = tester_config.features.enable(Feature::MultiAgentV2); let tester_thread_id = harness .manager - .start_thread( - tester_config.clone(), - thread_store_from_config(&tester_config), - ) + .start_thread(tester_config.clone()) .await .expect("tester thread should start") .thread_id; @@ -1701,7 +1695,7 @@ async fn resume_agent_from_rollout_reads_archived_rollout_path() { .shutdown_live_agent(child_thread_id) .await .expect("child shutdown should succeed"); - let store = LocalThreadStore::new(codex_rollout::RolloutConfig::from_view(&harness.config)); + let store = LocalThreadStore::new(LocalThreadStoreConfig::from_config(&harness.config)); store .archive_thread(ArchiveThreadParams { thread_id: child_thread_id, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 511db769b8..cc83c0a7c1 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -25,6 +25,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::Submission; use codex_protocol::protocol::ThreadMemoryMode; @@ -93,6 +94,7 @@ pub struct CodexThreadTurnContextOverrides { pub struct CodexThread { pub(crate) codex: Codex, pub(crate) session_source: SessionSource, + session_configured: SessionConfiguredEvent, rollout_path: Option, out_of_band_elicitation_count: Mutex, _watch_registration: WatchRegistration, @@ -103,6 +105,7 @@ pub struct CodexThread { impl CodexThread { pub(crate) fn new( codex: Codex, + session_configured: SessionConfiguredEvent, rollout_path: Option, session_source: SessionSource, watch_registration: WatchRegistration, @@ -110,6 +113,7 @@ impl CodexThread { Self { codex, session_source, + session_configured, rollout_path, out_of_band_elicitation_count: Mutex::new(0), _watch_registration: watch_registration, @@ -377,6 +381,14 @@ impl CodexThread { self.rollout_path.clone() } + pub(crate) fn session_configured(&self) -> SessionConfiguredEvent { + self.session_configured.clone() + } + + pub(crate) fn is_running(&self) -> bool { + !self.codex.tx_sub.is_closed() + } + pub async fn guardian_trunk_rollout_path(&self) -> Option { self.codex .session diff --git a/codex-rs/core/src/personality_migration.rs b/codex-rs/core/src/personality_migration.rs index 19bf00284f..5227cf07e3 100644 --- a/codex-rs/core/src/personality_migration.rs +++ b/codex-rs/core/src/personality_migration.rs @@ -3,6 +3,7 @@ use codex_config::config_toml::ConfigToml; use codex_protocol::config_types::Personality; use codex_thread_store::ListThreadsParams; use codex_thread_store::LocalThreadStore; +use codex_thread_store::LocalThreadStoreConfig; use codex_thread_store::ThreadSortKey; use codex_thread_store::ThreadStore; use std::io; @@ -60,12 +61,10 @@ pub async fn maybe_migrate_personality( } async fn has_recorded_sessions(codex_home: &Path, default_provider: &str) -> io::Result { - let store = LocalThreadStore::new(codex_rollout::RolloutConfig { + let store = LocalThreadStore::new(LocalThreadStoreConfig { codex_home: codex_home.to_path_buf(), sqlite_home: codex_home.to_path_buf(), - cwd: codex_home.to_path_buf(), - model_provider_id: default_provider.to_string(), - generate_memories: false, + default_model_provider_id: default_provider.to_string(), }); if has_threads(&store, /*archived*/ false).await? { return Ok(true); diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index 2a91f2e54c..d4f3130129 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -41,9 +41,9 @@ pub async fn build_prompt_input( SessionSource::Exec, Arc::new(EnvironmentManager::new(EnvironmentManagerArgs::new(local_runtime_paths)).await), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); - let thread_store = thread_store_from_config(&config); - let thread = thread_manager.start_thread(config, thread_store).await?; + let thread = thread_manager.start_thread(config).await?; let output = build_prompt_input_from_session(thread.thread.codex.session.as_ref(), input).await; let shutdown = thread.thread.shutdown_and_wait().await; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index b08d31c205..577852cc66 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -136,6 +136,7 @@ use codex_thread_store::LiveThreadInitGuard; use codex_thread_store::LocalThreadStore; use codex_thread_store::ResumeThreadParams; use codex_thread_store::ThreadEventPersistenceMode; +use codex_thread_store::ThreadPersistenceMetadata; use codex_thread_store::ThreadStore; use codex_utils_output_truncation::TruncationPolicy; use futures::future::BoxFuture; @@ -347,6 +348,7 @@ use codex_protocol::protocol::SkillMetadata as ProtocolSkillMetadata; use codex_protocol::protocol::SkillToolDependency as ProtocolSkillToolDependency; use codex_protocol::protocol::StreamErrorEvent; use codex_protocol::protocol::Submission; +use codex_protocol::protocol::ThreadMemoryMode; use codex_protocol::protocol::TokenCountEvent; use codex_protocol::protocol::TokenUsage; use codex_protocol::protocol::TokenUsageInfo; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 6c6cc22a0e..2c08caff48 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -400,6 +400,15 @@ impl Session { text: session_configuration.base_instructions.clone(), }, dynamic_tools: session_configuration.dynamic_tools.clone(), + metadata: ThreadPersistenceMetadata { + cwd: Some(config.cwd.to_path_buf()), + model_provider: config.model_provider_id.clone(), + memory_mode: if config.memories.generate_memories { + ThreadMemoryMode::Enabled + } else { + ThreadMemoryMode::Disabled + }, + }, event_persistence_mode, }, ) @@ -413,6 +422,15 @@ impl Session { rollout_path: resumed_history.rollout_path.clone(), history: Some(resumed_history.history.clone()), include_archived: true, + metadata: ThreadPersistenceMetadata { + cwd: Some(config.cwd.to_path_buf()), + model_provider: config.model_provider_id.clone(), + memory_mode: if config.memories.generate_memories { + ThreadMemoryMode::Enabled + } else { + ThreadMemoryMode::Disabled + }, + }, event_persistence_mode, }, ) diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 068e47bd71..30e90bcc93 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1679,9 +1679,6 @@ async fn fork_startup_context_then_first_turn_diff_snapshot() -> anyhow::Result< .fork_thread( usize::MAX, fork_config.clone(), - std::sync::Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(&fork_config), - )), rollout_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -2720,6 +2717,7 @@ async fn wait_for_thread_rollback_failed(rx: &async_channel::Receiver) -> } async fn attach_thread_persistence(session: &mut Session) -> PathBuf { + let config = session.get_config().await; let live_thread = LiveThread::create( Arc::clone(&session.services.thread_store), CreateThreadParams { @@ -2728,6 +2726,15 @@ async fn attach_thread_persistence(session: &mut Session) -> PathBuf { source: SessionSource::Exec, base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + metadata: ThreadPersistenceMetadata { + cwd: Some(config.cwd.to_path_buf()), + model_provider: config.model_provider_id.clone(), + memory_mode: if config.memories.generate_memories { + ThreadMemoryMode::Enabled + } else { + ThreadMemoryMode::Disabled + }, + }, event_persistence_mode: ThreadEventPersistenceMode::Limited, }, ) @@ -3392,7 +3399,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(config.as_ref()), + codex_thread_store::LocalThreadStoreConfig::from_config(config.as_ref()), )), codex_rollout_trace::ThreadTraceContext::disabled(), ) @@ -3539,7 +3546,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { state_db: None, live_thread: None, thread_store: Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(config.as_ref()), + codex_thread_store::LocalThreadStoreConfig::from_config(config.as_ref()), )), model_client: ModelClient::new( Some(auth_manager.clone()), @@ -3711,7 +3718,7 @@ async fn make_session_with_config_and_rx( Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(config.as_ref()), + codex_thread_store::LocalThreadStoreConfig::from_config(config.as_ref()), )), codex_rollout_trace::ThreadTraceContext::disabled(), ) @@ -4574,6 +4581,7 @@ async fn shutdown_complete_does_not_append_to_thread_store_after_shutdown() { let (mut session, _turn_context) = make_session_and_context().await; let store = Arc::new(codex_thread_store::InMemoryThreadStore::default()); let thread_store: Arc = store.clone(); + let config = session.get_config().await; let live_thread = LiveThread::create( Arc::clone(&thread_store), CreateThreadParams { @@ -4582,6 +4590,15 @@ async fn shutdown_complete_does_not_append_to_thread_store_after_shutdown() { source: SessionSource::Exec, base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + metadata: ThreadPersistenceMetadata { + cwd: Some(config.cwd.to_path_buf()), + model_provider: config.model_provider_id.clone(), + memory_mode: if config.memories.generate_memories { + ThreadMemoryMode::Enabled + } else { + ThreadMemoryMode::Disabled + }, + }, event_persistence_mode: ThreadEventPersistenceMode::Limited, }, ) @@ -4968,7 +4985,7 @@ where state_db: None, live_thread: None, thread_store: Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(config.as_ref()), + codex_thread_store::LocalThreadStoreConfig::from_config(config.as_ref()), )), model_client: ModelClient::new( Some(Arc::clone(&auth_manager)), diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 080ba79bdb..7f9673255d 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -729,7 +729,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_watcher = Arc::new(SkillsWatcher::noop()); let thread_store = Arc::new(codex_thread_store::LocalThreadStore::new( - codex_rollout::RolloutConfig::from_view(&config), + codex_thread_store::LocalThreadStoreConfig::from_config(&config), )); let CodexSpawnOk { codex, .. } = Codex::spawn(CodexSpawnArgs { diff --git a/codex-rs/core/src/test_support.rs b/codex-rs/core/src/test_support.rs index 34ed487fc5..6dbcf7a464 100644 --- a/codex-rs/core/src/test_support.rs +++ b/codex-rs/core/src/test_support.rs @@ -20,7 +20,6 @@ use codex_models_manager::test_support::get_model_offline_for_tests; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; -use codex_thread_store::ThreadStore; use once_cell::sync::Lazy; use crate::ThreadManager; @@ -77,18 +76,16 @@ pub fn thread_manager_with_models_provider_and_home( pub async fn start_thread_with_user_shell_override( thread_manager: &ThreadManager, config: Config, - thread_store: Arc, user_shell_override: crate::shell::Shell, ) -> codex_protocol::error::Result { thread_manager - .start_thread_with_user_shell_override_for_tests(config, thread_store, user_shell_override) + .start_thread_with_user_shell_override_for_tests(config, user_shell_override) .await } pub async fn resume_thread_from_rollout_with_user_shell_override( thread_manager: &ThreadManager, config: Config, - thread_store: Arc, rollout_path: PathBuf, auth_manager: Arc, user_shell_override: crate::shell::Shell, @@ -96,7 +93,6 @@ pub async fn resume_thread_from_rollout_with_user_shell_override( thread_manager .resume_thread_from_rollout_with_user_shell_override_for_tests( config, - thread_store, rollout_path, auth_manager, user_shell_override, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 085edfd532..c42b7f0c25 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -28,6 +28,7 @@ use codex_login::AuthManager; use codex_login::CodexAuth; use codex_model_provider::create_model_provider; use codex_model_provider_info::ModelProviderInfo; +use codex_model_provider_info::OPENAI_PROVIDER_ID; use codex_models_manager::manager::RefreshStrategy; use codex_models_manager::manager::SharedModelsManager; use codex_protocol::ThreadId; @@ -50,12 +51,15 @@ use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnAbortedEvent; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::W3cTraceContext; -use codex_rollout::RolloutConfig; use codex_state::DirectionalThreadSpawnEdgeStatus; use codex_thread_store::InMemoryThreadStore; use codex_thread_store::LocalThreadStore; +use codex_thread_store::LocalThreadStoreConfig; +use codex_thread_store::ReadThreadParams; use codex_thread_store::RemoteThreadStore; +use codex_thread_store::StoredThread; use codex_thread_store::ThreadStore; +use codex_thread_store::ThreadStoreError; use codex_utils_absolute_path::AbsolutePathBuf; use futures::StreamExt; use futures::stream::FuturesUnordered; @@ -211,7 +215,6 @@ pub struct ThreadManager { pub struct StartThreadOptions { pub config: Config, - pub thread_store: Arc, pub initial_history: InitialHistory, pub session_source: Option, pub dynamic_tools: Vec, @@ -221,10 +224,9 @@ pub struct StartThreadOptions { pub environments: Vec, } -pub(crate) struct ResumeThreadFromRolloutOptions { +pub(crate) struct ResumeThreadWithHistoryOptions { pub(crate) config: Config, - pub(crate) thread_store: Arc, - pub(crate) rollout_path: PathBuf, + pub(crate) initial_history: InitialHistory, pub(crate) agent_control: AgentControl, pub(crate) session_source: SessionSource, pub(crate) inherited_shell_snapshot: Option>, @@ -244,6 +246,7 @@ pub(crate) struct ThreadManagerState { plugins_manager: Arc, mcp_manager: Arc, skills_watcher: Arc, + thread_store: Arc, session_source: SessionSource, analytics_events_client: Option, // Captures submitted ops for testing purpose when test mode is enabled. @@ -263,9 +266,9 @@ pub fn build_models_manager( pub fn thread_store_from_config(config: &Config) -> Arc { match &config.experimental_thread_store { - ThreadStoreConfig::Local => { - Arc::new(LocalThreadStore::new(RolloutConfig::from_view(config))) - } + ThreadStoreConfig::Local => Arc::new(LocalThreadStore::new( + LocalThreadStoreConfig::from_config(config), + )), ThreadStoreConfig::Remote { endpoint } => Arc::new(RemoteThreadStore::new(endpoint)), ThreadStoreConfig::InMemory { id } => InMemoryThreadStore::for_id(id), } @@ -278,6 +281,7 @@ impl ThreadManager { session_source: SessionSource, environment_manager: Arc, analytics_events_client: Option, + thread_store: Arc, ) -> Self { let codex_home = config.codex_home.clone(); let restriction_product = session_source.restriction_product(); @@ -303,6 +307,7 @@ impl ThreadManager { plugins_manager, mcp_manager, skills_watcher, + thread_store, auth_manager, session_source, analytics_events_client, @@ -363,6 +368,14 @@ impl ThreadManager { restriction_product, )); let skills_watcher = build_skills_watcher(Arc::clone(&skills_manager)); + // This test constructor has no Config input. Tests that need a non-local + // process store should construct ThreadManager::new with an explicit store. + let thread_store: Arc = + Arc::new(LocalThreadStore::new(LocalThreadStoreConfig { + codex_home: codex_home.clone(), + sqlite_home: codex_home.clone(), + default_model_provider_id: OPENAI_PROVIDER_ID.to_string(), + })); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -374,6 +387,7 @@ impl ThreadManager { plugins_manager, mcp_manager, skills_watcher, + thread_store, auth_manager, session_source: SessionSource::Exec, analytics_events_client: None, @@ -517,16 +531,11 @@ impl ThreadManager { Ok(subtree_thread_ids) } - pub async fn start_thread( - &self, - config: Config, - thread_store: Arc, - ) -> CodexResult { + pub async fn start_thread(&self, config: Config) -> CodexResult { // Box delegated thread-spawn futures so these convenience wrappers do // not inline the full spawn path into every caller's async state. Box::pin(self.start_thread_with_tools( config, - thread_store, Vec::new(), /*persist_extended_history*/ false, )) @@ -536,7 +545,6 @@ impl ThreadManager { pub async fn start_thread_with_tools( &self, config: Config, - thread_store: Arc, dynamic_tools: Vec, persist_extended_history: bool, ) -> CodexResult { @@ -546,7 +554,6 @@ impl ThreadManager { ); Box::pin(self.start_thread_with_options(StartThreadOptions { config, - thread_store, initial_history: InitialHistory::New, session_source: None, dynamic_tools, @@ -567,7 +574,6 @@ impl ThreadManager { .unwrap_or_else(|| self.state.session_source.clone()); Box::pin(self.state.spawn_thread_with_source( options.config, - options.thread_store, options.initial_history, Arc::clone(&self.state.auth_manager), self.agent_control(), @@ -587,7 +593,6 @@ impl ThreadManager { pub async fn resume_thread_from_rollout( &self, config: Config, - thread_store: Arc, rollout_path: PathBuf, auth_manager: Arc, parent_trace: Option, @@ -595,7 +600,6 @@ impl ThreadManager { let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?; Box::pin(self.resume_thread_with_history( config, - thread_store, initial_history, auth_manager, /*persist_extended_history*/ false, @@ -607,7 +611,6 @@ impl ThreadManager { pub async fn resume_thread_with_history( &self, config: Config, - thread_store: Arc, initial_history: InitialHistory, auth_manager: Arc, persist_extended_history: bool, @@ -619,7 +622,6 @@ impl ThreadManager { ); Box::pin(self.state.spawn_thread( config, - thread_store, initial_history, auth_manager, self.agent_control(), @@ -636,7 +638,6 @@ impl ThreadManager { pub(crate) async fn start_thread_with_user_shell_override_for_tests( &self, config: Config, - thread_store: Arc, user_shell_override: crate::shell::Shell, ) -> CodexResult { let environments = default_thread_environment_selections( @@ -645,7 +646,6 @@ impl ThreadManager { ); Box::pin(self.state.spawn_thread( config, - thread_store, InitialHistory::New, Arc::clone(&self.state.auth_manager), self.agent_control(), @@ -662,7 +662,6 @@ impl ThreadManager { pub(crate) async fn resume_thread_from_rollout_with_user_shell_override_for_tests( &self, config: Config, - thread_store: Arc, rollout_path: PathBuf, auth_manager: Arc, user_shell_override: crate::shell::Shell, @@ -674,7 +673,6 @@ impl ThreadManager { ); Box::pin(self.state.spawn_thread( config, - thread_store, initial_history, auth_manager, self.agent_control(), @@ -754,7 +752,6 @@ impl ThreadManager { &self, snapshot: S, config: Config, - thread_store: Arc, path: PathBuf, persist_extended_history: bool, parent_trace: Option, @@ -767,7 +764,6 @@ impl ThreadManager { self.fork_thread_from_history( snapshot, config, - thread_store, history, persist_extended_history, parent_trace, @@ -780,7 +776,6 @@ impl ThreadManager { &self, snapshot: S, config: Config, - thread_store: Arc, history: InitialHistory, persist_extended_history: bool, parent_trace: Option, @@ -791,7 +786,6 @@ impl ThreadManager { self.fork_thread_with_initial_history( snapshot.into(), config, - thread_store, history, persist_extended_history, parent_trace, @@ -803,7 +797,6 @@ impl ThreadManager { &self, snapshot: ForkSnapshot, config: Config, - thread_store: Arc, history: InitialHistory, persist_extended_history: bool, parent_trace: Option, @@ -816,7 +809,6 @@ impl ThreadManager { ); Box::pin(self.state.spawn_thread( config, - thread_store, history, Arc::clone(&self.state.auth_manager), self.agent_control(), @@ -865,6 +857,31 @@ impl ThreadManagerState { } } + pub(crate) async fn read_stored_thread( + &self, + params: ReadThreadParams, + ) -> CodexResult { + let thread_id = params.thread_id; + self.thread_store + .read_thread(params) + .await + .map_err(|err| match err { + ThreadStoreError::ThreadNotFound { thread_id } => { + CodexErr::ThreadNotFound(thread_id) + } + ThreadStoreError::InvalidRequest { message } => { + if message.starts_with("no rollout found for thread id ") { + CodexErr::ThreadNotFound(thread_id) + } else { + CodexErr::Fatal(format!( + "failed to read stored thread {thread_id}: invalid thread-store request: {message}" + )) + } + } + err => CodexErr::Fatal(format!("failed to read stored thread {thread_id}: {err}")), + }) + } + /// Send an operation to a thread by ID. pub(crate) async fn send_op(&self, thread_id: ThreadId, op: Op) -> CodexResult { let thread = self.get_thread(thread_id).await?; @@ -896,12 +913,10 @@ impl ThreadManagerState { pub(crate) async fn spawn_new_thread( &self, config: Config, - thread_store: Arc, agent_control: AgentControl, ) -> CodexResult { Box::pin(self.spawn_new_thread_with_source( config, - thread_store, agent_control, self.session_source.clone(), /*persist_extended_history*/ false, @@ -917,7 +932,6 @@ impl ThreadManagerState { pub(crate) async fn spawn_new_thread_with_source( &self, config: Config, - thread_store: Arc, agent_control: AgentControl, session_source: SessionSource, persist_extended_history: bool, @@ -931,7 +945,6 @@ impl ThreadManagerState { }); Box::pin(self.spawn_thread_with_source( config, - thread_store, InitialHistory::New, Arc::clone(&self.auth_manager), agent_control, @@ -948,25 +961,22 @@ impl ThreadManagerState { .await } - pub(crate) async fn resume_thread_from_rollout_with_source( + pub(crate) async fn resume_thread_with_history_with_source( &self, - options: ResumeThreadFromRolloutOptions, + options: ResumeThreadWithHistoryOptions, ) -> CodexResult { - let ResumeThreadFromRolloutOptions { + let ResumeThreadWithHistoryOptions { config, - thread_store, - rollout_path, + initial_history, agent_control, session_source, inherited_shell_snapshot, inherited_exec_policy, } = options; - let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?; let environments = default_thread_environment_selections(self.environment_manager.as_ref(), &config.cwd); Box::pin(self.spawn_thread_with_source( config, - thread_store, initial_history, Arc::clone(&self.auth_manager), agent_control, @@ -987,7 +997,6 @@ impl ThreadManagerState { pub(crate) async fn fork_thread_with_source( &self, config: Config, - thread_store: Arc, initial_history: InitialHistory, agent_control: AgentControl, session_source: SessionSource, @@ -1001,7 +1010,6 @@ impl ThreadManagerState { }); Box::pin(self.spawn_thread_with_source( config, - thread_store, initial_history, Arc::clone(&self.auth_manager), agent_control, @@ -1023,7 +1031,6 @@ impl ThreadManagerState { pub(crate) async fn spawn_thread( &self, config: Config, - thread_store: Arc, initial_history: InitialHistory, auth_manager: Arc, agent_control: AgentControl, @@ -1036,7 +1043,6 @@ impl ThreadManagerState { ) -> CodexResult { Box::pin(self.spawn_thread_with_source( config, - thread_store, initial_history, auth_manager, agent_control, @@ -1057,7 +1063,6 @@ impl ThreadManagerState { pub(crate) async fn spawn_thread_with_source( &self, config: Config, - thread_store: Arc, initial_history: InitialHistory, auth_manager: Arc, agent_control: AgentControl, @@ -1072,6 +1077,27 @@ impl ThreadManagerState { user_shell_override: Option, ) -> CodexResult { let is_resumed_thread = matches!(&initial_history, InitialHistory::Resumed(_)); + if let InitialHistory::Resumed(resumed) = &initial_history { + let mut threads = self.threads.write().await; + if let Some(thread) = threads.get(&resumed.conversation_id).cloned() { + if thread.is_running() { + if let Some(requested_rollout_path) = resumed.rollout_path.as_deref() + && thread.rollout_path().as_deref() != Some(requested_rollout_path) + { + return Err(CodexErr::InvalidRequest(format!( + "thread {} is already running with a different rollout path", + resumed.conversation_id + ))); + } + return Ok(NewThread { + thread_id: resumed.conversation_id, + session_configured: thread.session_configured(), + thread, + }); + } + threads.remove(&resumed.conversation_id); + } + } let environment = selected_primary_environment(self.environment_manager.as_ref(), &environments)?; let watch_registration = match environment.as_ref() { @@ -1115,7 +1141,7 @@ impl ThreadManagerState { parent_trace, environments, analytics_events_client: self.analytics_events_client.clone(), - thread_store, + thread_store: Arc::clone(&self.thread_store), }) .await?; let new_thread = self @@ -1147,20 +1173,31 @@ impl ThreadManagerState { } }; - let thread = Arc::new(CodexThread::new( - codex, - session_configured.rollout_path.clone(), - session_source, - watch_registration, - )); - let mut threads = self.threads.write().await; - threads.insert(thread_id, thread.clone()); + { + let mut threads = self.threads.write().await; + if let std::collections::hash_map::Entry::Vacant(e) = threads.entry(thread_id) { + let thread = Arc::new(CodexThread::new( + codex, + session_configured.clone(), + session_configured.rollout_path.clone(), + session_source, + watch_registration, + )); + e.insert(thread.clone()); + return Ok(NewThread { + thread_id, + thread, + session_configured, + }); + } + } - Ok(NewThread { - thread_id, - thread, - session_configured, - }) + if let Err(err) = codex.shutdown_and_wait().await { + warn!("failed to shut down duplicate thread {thread_id}: {err}"); + } + Err(CodexErr::InvalidRequest(format!( + "thread {thread_id} is already running" + ))) } pub(crate) fn notify_thread_created(&self, thread_id: ThreadId) { diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index c79321b94b..2fe2f97bb3 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -161,8 +161,7 @@ fn fork_thread_accepts_legacy_usize_snapshot_argument() { ) { let _future = manager.fork_thread( usize::MAX, - config.clone(), - thread_store_from_config(&config), + config, path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -263,12 +262,12 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); let thread_1 = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start first thread") .thread_id; let thread_2 = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start second thread") .thread_id; @@ -314,7 +313,6 @@ async fn start_thread_accepts_explicit_environment_when_default_environment_is_d let thread = manager .start_thread_with_options(StartThreadOptions { - thread_store: thread_store_from_config(&config), config: config.clone(), initial_history: InitialHistory::New, session_source: None, @@ -347,10 +345,8 @@ async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() { config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); - let thread_store = thread_store_from_config(&config); let thread = manager .start_thread_with_options(StartThreadOptions { - thread_store, config, initial_history: InitialHistory::New, session_source: Some(SessionSource::Internal( @@ -393,6 +389,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let selected_cwd = AbsolutePathBuf::try_from(config.cwd.as_path().join("selected")).expect("absolute path"); @@ -401,11 +398,8 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { cwd: selected_cwd.clone(), }]; let default_cwd = config.cwd.clone(); - let thread_store = thread_store_from_config(&config); - let source = manager .start_thread_with_options(StartThreadOptions { - thread_store: Arc::clone(&thread_store), config: config.clone(), initial_history: InitialHistory::New, session_source: None, @@ -437,7 +431,6 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { let resumed = manager .resume_thread_from_rollout( config.clone(), - Arc::clone(&thread_store), rollout_path.clone(), auth_manager, /*parent_trace*/ None, @@ -459,7 +452,6 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { .fork_thread( ForkSnapshot::Interrupted, config, - thread_store, rollout_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -478,6 +470,117 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { assert_ne!(forked_turn.environments[0].cwd, selected_cwd); } +#[tokio::test] +async fn resume_active_thread_from_rollout_returns_running_thread() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config().await; + config.codex_home = temp_dir.path().join("codex-home").abs(); + config.cwd = config.codex_home.abs(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let manager = ThreadManager::new( + &config, + auth_manager.clone(), + SessionSource::Exec, + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + /*analytics_events_client*/ None, + thread_store_from_config(&config), + ); + + let source = manager + .start_thread(config.clone()) + .await + .expect("start source thread"); + source.thread.ensure_rollout_materialized().await; + source + .thread + .flush_rollout() + .await + .expect("flush source rollout"); + let rollout_path = source + .thread + .rollout_path() + .expect("source rollout path should exist"); + + let resumed = manager + .resume_thread_from_rollout( + config, + rollout_path, + auth_manager, + /*parent_trace*/ None, + ) + .await + .expect("resume active source thread"); + assert_eq!(resumed.thread_id, source.thread_id); + assert!(Arc::ptr_eq(&resumed.thread, &source.thread)); + + source + .thread + .shutdown_and_wait() + .await + .expect("shutdown source thread"); +} + +#[tokio::test] +async fn resume_stopped_thread_from_rollout_spawns_new_thread() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config().await; + config.codex_home = temp_dir.path().join("codex-home").abs(); + config.cwd = config.codex_home.abs(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let manager = ThreadManager::new( + &config, + auth_manager.clone(), + SessionSource::Exec, + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + /*analytics_events_client*/ None, + thread_store_from_config(&config), + ); + + let source = manager + .start_thread(config.clone()) + .await + .expect("start source thread"); + source.thread.ensure_rollout_materialized().await; + source + .thread + .flush_rollout() + .await + .expect("flush source rollout"); + let rollout_path = source + .thread + .rollout_path() + .expect("source rollout path should exist"); + source + .thread + .shutdown_and_wait() + .await + .expect("shutdown source thread"); + + let resumed = manager + .resume_thread_from_rollout( + config, + rollout_path, + auth_manager, + /*parent_trace*/ None, + ) + .await + .expect("resume stopped source thread"); + assert_eq!(resumed.thread_id, source.thread_id); + assert!(!Arc::ptr_eq(&resumed.thread, &source.thread)); + + resumed + .thread + .shutdown_and_wait() + .await + .expect("shutdown resumed thread"); +} + #[tokio::test] async fn new_uses_active_provider_for_model_refresh() { let server = MockServer::start().await; @@ -499,6 +602,7 @@ async fn new_uses_active_provider_for_model_refresh() { SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let _ = manager.list_models(RefreshStrategy::Online).await; @@ -709,12 +813,12 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let source = manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(vec![ RolloutItem::ResponseItem(user_msg("hello")), RolloutItem::ResponseItem(assistant_msg("partial")), @@ -741,7 +845,6 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor .fork_thread( ForkSnapshot::Interrupted, config.clone(), - thread_store_from_config(&config), source_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -812,12 +915,12 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let source = manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(vec![ RolloutItem::EventMsg(EventMsg::TurnStarted(TurnStartedEvent { turn_id: "turn-explicit".to_string(), @@ -855,7 +958,6 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { .fork_thread( ForkSnapshot::Interrupted, config.clone(), - thread_store_from_config(&config), source_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -904,12 +1006,12 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let source = manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(vec![ RolloutItem::ResponseItem(user_msg("hello")), RolloutItem::ResponseItem(assistant_msg("partial")), @@ -934,7 +1036,6 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ .fork_thread( ForkSnapshot::Interrupted, config.clone(), - thread_store_from_config(&config), source_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -975,7 +1076,6 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ .fork_thread( ForkSnapshot::Interrupted, config.clone(), - thread_store_from_config(&config), forked_path, /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -1042,12 +1142,12 @@ async fn resumed_thread_activates_paused_goal_and_continues_on_request() -> anyh SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let source = manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(vec![RolloutItem::ResponseItem(user_msg("keep working"))]), auth_manager.clone(), /*persist_extended_history*/ false, @@ -1072,12 +1172,12 @@ async fn resumed_thread_activates_paused_goal_and_continues_on_request() -> anyh /*token_budget*/ None, ) .await?; + source.thread.shutdown_and_wait().await?; manager.remove_thread(&source.thread_id).await; let resumed = manager .resume_thread_from_rollout( config.clone(), - thread_store_from_config(&config), source_path, auth_manager, /*parent_trace*/ None, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index e55bd4fa28..61dc77eb36 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -5,7 +5,6 @@ use crate::config::DEFAULT_AGENT_MAX_DEPTH; use crate::function_tool::FunctionCallError; use crate::session::tests::make_session_and_context; use crate::session_prefix::format_subagent_notification_message; -use crate::thread_manager::thread_store_from_config; use crate::tools::context::ToolOutput; use crate::tools::handlers::multi_agents_v2::CloseAgentHandler as CloseAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::FollowupTaskHandler as FollowupTaskHandlerV2; @@ -297,10 +296,7 @@ async fn spawn_agent_fork_context_rejects_agent_type_override() { let role_name = install_role_with_model_override(&mut turn).await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -332,10 +328,7 @@ async fn spawn_agent_fork_context_rejects_child_model_overrides() { let (mut session, turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -370,10 +363,7 @@ async fn multi_agent_v2_spawn_fork_turns_all_rejects_agent_type_override() { let role_name = install_role_with_model_override(&mut turn).await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -416,10 +406,7 @@ async fn multi_agent_v2_spawn_defaults_to_full_fork_and_rejects_child_model_over let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -460,10 +447,7 @@ async fn multi_agent_v2_spawn_partial_fork_turns_allows_agent_type_override() { let role_name = install_role_with_model_override(&mut turn).await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -546,10 +530,7 @@ async fn multi_agent_v2_spawn_requires_task_name() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -583,10 +564,7 @@ async fn multi_agent_v2_spawn_rejects_legacy_items_field() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -646,10 +624,7 @@ async fn multi_agent_v2_spawn_returns_path_and_send_message_accepts_relative_pat let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -746,10 +721,7 @@ async fn multi_agent_v2_spawn_rejects_legacy_fork_context() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -788,10 +760,7 @@ async fn multi_agent_v2_spawn_rejects_invalid_fork_turns_string() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -830,10 +799,7 @@ async fn multi_agent_v2_spawn_rejects_zero_fork_turns() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -872,10 +838,7 @@ async fn multi_agent_v2_send_message_accepts_root_target_from_child() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -951,10 +914,7 @@ async fn multi_agent_v2_followup_task_rejects_root_target_from_child() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1035,10 +995,7 @@ async fn multi_agent_v2_list_agents_returns_completed_status_and_last_task_messa let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1132,10 +1089,7 @@ async fn multi_agent_v2_list_agents_filters_by_relative_path_prefix() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1222,10 +1176,7 @@ async fn multi_agent_v2_list_agents_omits_closed_agents() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1289,10 +1240,7 @@ async fn multi_agent_v2_send_message_rejects_legacy_items_field() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1348,10 +1296,7 @@ async fn multi_agent_v2_send_message_rejects_interrupt_parameter() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1424,10 +1369,7 @@ async fn multi_agent_v2_followup_task_completion_notifies_parent_on_every_turn() let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1562,10 +1504,7 @@ async fn multi_agent_v2_followup_task_rejects_legacy_items_field() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1618,10 +1557,7 @@ async fn multi_agent_v2_interrupted_turn_does_not_notify_parent() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1698,10 +1634,7 @@ async fn multi_agent_v2_spawn_omits_agent_id_when_named() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1740,10 +1673,7 @@ async fn multi_agent_v2_spawn_surfaces_task_name_validation_errors() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -1957,7 +1887,7 @@ async fn multi_agent_v2_spawn_agent_ignores_configured_max_depth() { .enable(Feature::MultiAgentV2) .expect("test config should allow feature update"); let root = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -2082,7 +2012,7 @@ async fn send_input_interrupts_before_prompt() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2124,7 +2054,7 @@ async fn send_input_accepts_structured_items() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2219,7 +2149,7 @@ async fn resume_agent_noops_for_active_agent() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2260,7 +2190,6 @@ async fn resume_agent_restores_closed_agent_and_accepts_send_input() { let thread = manager .resume_thread_with_history( config.clone(), - thread_store_from_config(&config), InitialHistory::Forked(vec![RolloutItem::ResponseItem(ResponseItem::Message { id: None, role: "user".to_string(), @@ -2425,10 +2354,7 @@ async fn multi_agent_v2_wait_agent_accepts_timeout_only_argument() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -2605,7 +2531,7 @@ async fn wait_agent_times_out_when_status_is_not_final() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2648,7 +2574,7 @@ async fn wait_agent_clamps_short_timeouts_to_minimum() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2686,7 +2612,7 @@ async fn wait_agent_returns_final_status_without_timeout() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -2736,10 +2662,7 @@ async fn multi_agent_v2_wait_agent_returns_summary_for_mailbox_activity() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -2830,10 +2753,7 @@ async fn multi_agent_v2_wait_agent_returns_for_already_queued_mail() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -2911,10 +2831,7 @@ async fn multi_agent_v2_wait_agent_wakes_on_any_mailbox_notification() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -3002,10 +2919,7 @@ async fn multi_agent_v2_wait_agent_does_not_return_completed_content() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -3091,10 +3005,7 @@ async fn multi_agent_v2_close_agent_accepts_task_name_target() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -3153,10 +3064,7 @@ async fn multi_agent_v2_close_agent_rejects_root_target_and_id() { let (mut session, mut turn) = make_session_and_context().await; let manager = thread_manager(); let root = manager - .start_thread( - (*turn.config).clone(), - thread_store_from_config(turn.config.as_ref()), - ) + .start_thread((*turn.config).clone()) .await .expect("root thread should start"); session.services.agent_control = manager.agent_control(); @@ -3206,7 +3114,7 @@ async fn close_agent_submits_shutdown_and_returns_previous_status() { session.services.agent_control = manager.agent_control(); let config = turn.config.as_ref().clone(); let thread = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("start thread"); let agent_id = thread.thread_id; @@ -3250,7 +3158,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr .expect("test config should allow sqlite"); let parent = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("parent thread should start"); let parent_thread_id = parent.thread_id; @@ -3381,7 +3289,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr ); let operator = manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("operator thread should start"); let operator_session = operator.thread.codex.session.clone(); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 374116e3fe..291a0795ce 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -430,6 +430,7 @@ impl TestCodexBuilder { SessionSource::Exec, Arc::clone(&environment_manager), /*analytics_events_client*/ None, + thread_store_from_config(&config), ) } else { codex_core::test_support::thread_manager_with_models_provider_and_home( @@ -449,7 +450,6 @@ impl TestCodexBuilder { codex_core::test_support::resume_thread_from_rollout_with_user_shell_override( thread_manager.as_ref(), config.clone(), - thread_store_from_config(&config), path, auth_manager, user_shell_override, @@ -461,7 +461,6 @@ impl TestCodexBuilder { let auth_manager = codex_core::test_support::auth_manager_from_auth(auth); Box::pin(thread_manager.resume_thread_from_rollout( config.clone(), - thread_store_from_config(&config), path, auth_manager, /*parent_trace*/ None, @@ -473,18 +472,12 @@ impl TestCodexBuilder { codex_core::test_support::start_thread_with_user_shell_override( thread_manager.as_ref(), config.clone(), - thread_store_from_config(&config), user_shell_override, ), ) .await? } - (None, None) => { - Box::pin( - thread_manager.start_thread(config.clone(), thread_store_from_config(&config)), - ) - .await? - } + (None, None) => Box::pin(thread_manager.start_thread(config.clone())).await?, }; Ok(TestCodex { diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 9ed83605c2..f4960af550 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1107,9 +1107,10 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { SessionSource::Exec, Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); let NewThread { thread: codex, .. } = thread_manager - .start_thread(config.clone(), thread_store_from_config(&config)) + .start_thread(config.clone()) .await .expect("create new conversation"); diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index c000469aa7..af94252c02 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -2556,7 +2556,6 @@ async fn code_mode_can_call_hidden_dynamic_tools() -> Result<()> { .thread_manager .start_thread_with_tools( base_test.config.clone(), - codex_core::thread_store_from_config(&base_test.config), vec![DynamicToolSpec { namespace: Some("codex_app".to_string()), name: "hidden_dynamic_tool".to_string(), diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 30884cc0a9..b145506d86 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -445,7 +445,6 @@ async fn remote_compact_filters_deferred_dynamic_tools() -> Result<()> { .thread_manager .start_thread_with_tools( test.config.clone(), - codex_core::thread_store_from_config(&test.config), dynamic_tools, /*persist_extended_history*/ false, ) diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index 24f77fefdc..354e9a6a03 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -149,6 +149,7 @@ async fn compact_resume_and_fork_preserve_model_history_view() { "compact+resume test expects base path {base_path:?} to exist", ); + shutdown_conversation(&base).await; let resumed = resume_conversation(&manager, &config, base_path).await; user_turn(&resumed, "AFTER_RESUME").await; let resumed_path = fetch_conversation_path(&resumed); @@ -304,6 +305,7 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() "second compact test expects base path {base_path:?} to exist", ); + shutdown_conversation(&base).await; let resumed = resume_conversation(&manager, &config, base_path).await; user_turn(&resumed, "AFTER_RESUME").await; let resumed_path = fetch_conversation_path(&resumed); @@ -323,6 +325,7 @@ async fn compact_resume_after_second_compaction_preserves_history() -> Result<() "second compact test expects forked path {forked_path:?} to exist", ); + shutdown_conversation(&forked).await; let resumed_again = resume_conversation(&manager, &config, forked_path).await; user_turn(&resumed_again, AFTER_SECOND_RESUME).await; @@ -815,6 +818,13 @@ fn fetch_conversation_path(conversation: &Arc) -> std::path::PathBu conversation.rollout_path().expect("rollout path") } +async fn shutdown_conversation(conversation: &Arc) { + conversation + .shutdown_and_wait() + .await + .expect("shutdown conversation"); +} + async fn resume_conversation( manager: &ThreadManager, config: &Config, @@ -825,7 +835,6 @@ async fn resume_conversation( ); Box::pin(manager.resume_thread_from_rollout( config.clone(), - codex_core::thread_store_from_config(config), path, auth_manager, /*parent_trace*/ None, @@ -845,7 +854,6 @@ async fn fork_thread( Box::pin(manager.fork_thread( nth_user_message, config.clone(), - codex_core::thread_store_from_config(config), path, /*persist_extended_history*/ false, /*parent_trace*/ None, diff --git a/codex-rs/core/tests/suite/fork_thread.rs b/codex-rs/core/tests/suite/fork_thread.rs index 50e0dc1862..19ed2a2088 100644 --- a/codex-rs/core/tests/suite/fork_thread.rs +++ b/codex-rs/core/tests/suite/fork_thread.rs @@ -100,7 +100,6 @@ async fn fork_thread_twice_drops_to_first_message() { .fork_thread( ForkSnapshot::TruncateBeforeNthUserMessage(1), config_for_fork.clone(), - codex_core::thread_store_from_config(&config_for_fork), base_path.clone(), /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -125,7 +124,6 @@ async fn fork_thread_twice_drops_to_first_message() { .fork_thread( ForkSnapshot::TruncateBeforeNthUserMessage(0), config_for_fork.clone(), - codex_core::thread_store_from_config(&config_for_fork), fork1_path.clone(), /*persist_extended_history*/ false, /*parent_trace*/ None, @@ -194,7 +192,6 @@ async fn fork_thread_from_history_does_not_require_source_rollout_path() { .fork_thread_from_history( ForkSnapshot::Interrupted, test.config.clone(), - codex_core::thread_store_from_config(&test.config), InitialHistory::Resumed(ResumedHistory { conversation_id: test.session_configured.session_id, history: source_items.clone(), diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index d64349c7f3..bb93d5cbf8 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -496,7 +496,6 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { .fork_thread( ForkSnapshot::Interrupted, fork_config.clone(), - codex_core::thread_store_from_config(&fork_config), rollout_path, /*persist_extended_history*/ false, /*parent_trace*/ None, diff --git a/codex-rs/core/tests/suite/realtime_conversation.rs b/codex-rs/core/tests/suite/realtime_conversation.rs index 93d1f29b43..96aa979f9a 100644 --- a/codex-rs/core/tests/suite/realtime_conversation.rs +++ b/codex-rs/core/tests/suite/realtime_conversation.rs @@ -1671,7 +1671,6 @@ async fn conversation_startup_context_current_thread_selects_many_turns_by_budge .thread_manager .resume_thread_with_history( test.config.clone(), - codex_core::thread_store_from_config(&test.config), InitialHistory::Forked(history), auth_manager_from_auth(CodexAuth::from_api_key("dummy")), /*persist_extended_history*/ false, diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index 9b2faba367..cb545df351 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -106,7 +106,6 @@ async fn emits_warning_when_resumed_model_differs() { } = thread_manager .resume_thread_with_history( config.clone(), - codex_core::thread_store_from_config(&config), initial_history, auth_manager, /*persist_extended_history*/ false, diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 5b3c540ef3..b4edf24668 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -793,7 +793,6 @@ async fn tool_search_returns_deferred_dynamic_tool_and_routes_follow_up_call() - .thread_manager .start_thread_with_tools( base_test.config.clone(), - codex_core::thread_store_from_config(&base_test.config), vec![dynamic_tool], /*persist_extended_history*/ false, ) diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index f6db9a7098..a68af6a1e2 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -246,10 +246,9 @@ async fn list_skills_skips_cwd_roots_when_environment_disabled() -> Result<()> { )?, )), /*analytics_events_client*/ None, + thread_store_from_config(&config), ); - let new_thread = thread_manager - .start_thread(config.clone(), thread_store_from_config(&config)) - .await?; + let new_thread = thread_manager.start_thread(config.clone()).await?; let cwd = config.cwd.to_path_buf(); new_thread diff --git a/codex-rs/core/tests/suite/unstable_features_warning.rs b/codex-rs/core/tests/suite/unstable_features_warning.rs index 973a23a466..66a7366589 100644 --- a/codex-rs/core/tests/suite/unstable_features_warning.rs +++ b/codex-rs/core/tests/suite/unstable_features_warning.rs @@ -44,7 +44,6 @@ async fn emits_warning_when_unstable_features_enabled_via_config() { } = thread_manager .resume_thread_with_history( config.clone(), - codex_core::thread_store_from_config(&config), InitialHistory::New, auth_manager, /*persist_extended_history*/ false, @@ -92,7 +91,6 @@ async fn suppresses_warning_when_configured() { } = thread_manager .resume_thread_with_history( config.clone(), - codex_core::thread_store_from_config(&config), InitialHistory::New, auth_manager, /*persist_extended_history*/ false, diff --git a/codex-rs/core/tests/suite/window_headers.rs b/codex-rs/core/tests/suite/window_headers.rs index eca8a1fce9..de52821839 100644 --- a/codex-rs/core/tests/suite/window_headers.rs +++ b/codex-rs/core/tests/suite/window_headers.rs @@ -71,7 +71,6 @@ async fn window_id_advances_after_compact_persists_on_resume_and_resets_on_fork( .fork_thread( /*snapshot*/ 0usize, resumed.config.clone(), - codex_core::thread_store_from_config(&resumed.config), rollout_path, /*persist_extended_history*/ false, /*parent_trace*/ None, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 79b43122d9..62d8b14fbf 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -13,7 +13,6 @@ use codex_core::CodexThread; use codex_core::NewThread; use codex_core::ThreadManager; use codex_core::config::Config as CodexConfig; -use codex_core::thread_store_from_config; use codex_protocol::ThreadId; use codex_protocol::protocol::AgentMessageEvent; use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; @@ -69,10 +68,7 @@ pub async fn run_codex_tool_session( thread_id, thread, session_configured, - } = match thread_manager - .start_thread(config.clone(), thread_store_from_config(&config)) - .await - { + } = match thread_manager.start_thread(config.clone()).await { Ok(res) => res, Err(e) => { let result = CallToolResult { diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 0e9a1435fc..99076650cc 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use codex_arg0::Arg0DispatchPaths; use codex_core::ThreadManager; use codex_core::config::Config; +use codex_core::thread_store_from_config; use codex_exec_server::EnvironmentManager; use codex_login::AuthManager; use codex_login::default_client::USER_AGENT_SUFFIX; @@ -65,6 +66,7 @@ impl MessageProcessor { SessionSource::Mcp, environment_manager, /*analytics_events_client*/ None, + thread_store_from_config(config.as_ref()), )); Self { outgoing, diff --git a/codex-rs/memories/write/src/runtime.rs b/codex-rs/memories/write/src/runtime.rs index 255fb718f2..737fb67870 100644 --- a/codex-rs/memories/write/src/runtime.rs +++ b/codex-rs/memories/write/src/runtime.rs @@ -8,7 +8,6 @@ use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::content_items_to_text; use codex_core::resolve_installation_id; -use codex_core::thread_store_from_config; use codex_features::Feature; use codex_login::AuthManager; use codex_login::CodexAuth; @@ -237,7 +236,6 @@ impl MemoryStartupContext { } = self .thread_manager .start_thread_with_options(StartThreadOptions { - thread_store: thread_store_from_config(&config), config, initial_history: InitialHistory::New, session_source: Some(SessionSource::Internal( diff --git a/codex-rs/rollout/src/metadata.rs b/codex-rs/rollout/src/metadata.rs index 58d55a887d..e7a25f0cda 100644 --- a/codex-rs/rollout/src/metadata.rs +++ b/codex-rs/rollout/src/metadata.rs @@ -1,6 +1,5 @@ use crate::ARCHIVED_SESSIONS_SUBDIR; use crate::SESSIONS_SUBDIR; -use crate::config::RolloutConfigView; use crate::list; use crate::list::parse_timestamp_uuid_from_filename; use crate::recorder::RolloutRecorder; @@ -135,7 +134,8 @@ pub async fn extract_metadata_from_rollout( pub(crate) async fn backfill_sessions( runtime: &codex_state::StateRuntime, - config: &impl RolloutConfigView, + codex_home: &Path, + default_provider: &str, ) { let metric_client = codex_otel::global(); let timer = metric_client @@ -146,7 +146,7 @@ pub(crate) async fn backfill_sessions( Err(err) => { warn!( "failed to read backfill state at {}: {err}", - config.codex_home().display() + codex_home.display() ); BackfillState::default() } @@ -159,7 +159,7 @@ pub(crate) async fn backfill_sessions( Err(err) => { warn!( "failed to claim backfill worker at {}: {err}", - config.codex_home().display() + codex_home.display() ); return; } @@ -167,7 +167,7 @@ pub(crate) async fn backfill_sessions( if !claimed { info!( "state db backfill already running at {}; skipping duplicate worker", - config.codex_home().display() + codex_home.display() ); return; } @@ -176,7 +176,7 @@ pub(crate) async fn backfill_sessions( Err(err) => { warn!( "failed to read claimed backfill state at {}: {err}", - config.codex_home().display() + codex_home.display() ); BackfillState { status: BackfillStatus::Running, @@ -188,15 +188,15 @@ pub(crate) async fn backfill_sessions( if let Err(err) = runtime.mark_backfill_running().await { warn!( "failed to mark backfill running at {}: {err}", - config.codex_home().display() + codex_home.display() ); } else { backfill_state.status = BackfillStatus::Running; } } - let sessions_root = config.codex_home().join(SESSIONS_SUBDIR); - let archived_root = config.codex_home().join(ARCHIVED_SESSIONS_SUBDIR); + let sessions_root = codex_home.join(SESSIONS_SUBDIR); + let archived_root = codex_home.join(ARCHIVED_SESSIONS_SUBDIR); let mut rollout_paths: Vec = Vec::new(); for (root, archived) in [(sessions_root, false), (archived_root, true)] { if !tokio::fs::try_exists(&root).await.unwrap_or(false) { @@ -205,7 +205,7 @@ pub(crate) async fn backfill_sessions( match collect_rollout_paths(&root).await { Ok(paths) => { rollout_paths.extend(paths.into_iter().map(|path| BackfillRolloutPath { - watermark: backfill_watermark_for_path(config.codex_home(), &path), + watermark: backfill_watermark_for_path(codex_home, &path), path, archived, })); @@ -232,7 +232,7 @@ pub(crate) async fn backfill_sessions( for batch in rollout_paths.chunks(BACKFILL_BATCH_SIZE) { for rollout in batch { stats.scanned = stats.scanned.saturating_add(1); - match extract_metadata_from_rollout(&rollout.path, config.model_provider_id()).await { + match extract_metadata_from_rollout(&rollout.path, default_provider).await { Ok(outcome) => { if outcome.parse_errors > 0 && let Some(ref metric_client) = metric_client @@ -309,7 +309,7 @@ pub(crate) async fn backfill_sessions( { warn!( "failed to checkpoint backfill at {}: {err}", - config.codex_home().display() + codex_home.display() ); } else { last_watermark = Some(last_entry.watermark.clone()); @@ -322,7 +322,7 @@ pub(crate) async fn backfill_sessions( { warn!( "failed to mark backfill complete at {}: {err}", - config.codex_home().display() + codex_home.display() ); } diff --git a/codex-rs/rollout/src/metadata_tests.rs b/codex-rs/rollout/src/metadata_tests.rs index 8a149313dc..c94cd0be7e 100644 --- a/codex-rs/rollout/src/metadata_tests.rs +++ b/codex-rs/rollout/src/metadata_tests.rs @@ -1,7 +1,6 @@ #![allow(warnings, clippy::all)] use super::*; -use crate::config::RolloutConfig; use chrono::DateTime; use chrono::NaiveDateTime; use chrono::Timelike; @@ -24,16 +23,6 @@ use std::path::PathBuf; use tempfile::tempdir; use uuid::Uuid; -fn test_config(codex_home: PathBuf) -> RolloutConfig { - RolloutConfig { - sqlite_home: codex_home.clone(), - cwd: codex_home.clone(), - codex_home, - model_provider_id: "test-provider".to_string(), - generate_memories: true, - } -} - #[tokio::test] async fn extract_metadata_from_rollout_uses_session_meta() { let dir = tempdir().expect("tempdir"); @@ -210,8 +199,7 @@ async fn backfill_sessions_resumes_from_watermark_and_marks_complete() { )) .await; - let config = test_config(codex_home.clone()); - backfill_sessions(runtime.as_ref(), &config).await; + backfill_sessions(runtime.as_ref(), codex_home.as_path(), "test-provider").await; let first_id = ThreadId::from_string(&first_uuid.to_string()).expect("first thread id"); let second_id = ThreadId::from_string(&second_uuid.to_string()).expect("second thread id"); @@ -278,8 +266,7 @@ async fn backfill_sessions_preserves_existing_git_branch_and_fills_missing_git_f .await .expect("existing metadata upsert"); - let config = test_config(codex_home.clone()); - backfill_sessions(runtime.as_ref(), &config).await; + backfill_sessions(runtime.as_ref(), codex_home.as_path(), "test-provider").await; let persisted = runtime .get_thread(thread_id) @@ -313,8 +300,7 @@ async fn backfill_sessions_normalizes_cwd_before_upsert() { .await .expect("initialize runtime"); - let config = test_config(codex_home.clone()); - backfill_sessions(runtime.as_ref(), &config).await; + backfill_sessions(runtime.as_ref(), codex_home.as_path(), "test-provider").await; let thread_id = ThreadId::from_string(&thread_uuid.to_string()).expect("thread id"); let stored = runtime diff --git a/codex-rs/rollout/src/state_db.rs b/codex-rs/rollout/src/state_db.rs index ae87bff73e..41b59c9760 100644 --- a/codex-rs/rollout/src/state_db.rs +++ b/codex-rs/rollout/src/state_db.rs @@ -25,9 +25,23 @@ pub type StateDbHandle = Arc; /// Initialize the state runtime for thread state persistence and backfill checks. pub async fn init(config: &impl RolloutConfigView) -> Option { let config = RolloutConfig::from_view(config); + init_with_roots( + config.codex_home, + config.sqlite_home, + config.model_provider_id, + ) + .await +} + +/// Initialize the state runtime for a local thread store. +pub async fn init_with_roots( + codex_home: PathBuf, + sqlite_home: PathBuf, + default_model_provider_id: String, +) -> Option { let runtime = match codex_state::StateRuntime::init( - config.sqlite_home.clone(), - config.model_provider_id.clone(), + sqlite_home.clone(), + default_model_provider_id.clone(), ) .await { @@ -35,7 +49,7 @@ pub async fn init(config: &impl RolloutConfigView) -> Option { Err(err) => { warn!( "failed to initialize state runtime at {}: {err}", - config.sqlite_home.display() + sqlite_home.display() ); return None; } @@ -45,16 +59,20 @@ pub async fn init(config: &impl RolloutConfigView) -> Option { Err(err) => { warn!( "failed to read backfill state at {}: {err}", - config.codex_home.display() + codex_home.display() ); return None; } }; if backfill_state.status != codex_state::BackfillStatus::Complete { let runtime_for_backfill = runtime.clone(); - let config = config.clone(); tokio::spawn(async move { - metadata::backfill_sessions(runtime_for_backfill.as_ref(), &config).await; + metadata::backfill_sessions( + runtime_for_backfill.as_ref(), + codex_home.as_path(), + default_model_provider_id.as_str(), + ) + .await; }); } Some(runtime) @@ -487,7 +505,7 @@ pub async fn read_repair_rollout_path( pub async fn apply_rollout_items( context: Option<&codex_state::StateRuntime>, rollout_path: &Path, - _default_provider: &str, + default_provider: &str, builder: Option<&ThreadMetadataBuilder>, items: &[RolloutItem], stage: &str, @@ -511,6 +529,9 @@ pub async fn apply_rollout_items( } }, }; + if builder.model_provider.is_none() { + builder.model_provider = Some(default_provider.to_string()); + } builder.rollout_path = rollout_path.to_path_buf(); builder.cwd = normalize_cwd_for_state_db(&builder.cwd); if let Err(err) = ctx diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 56c71ab392..2f004717e7 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -118,12 +118,13 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { SessionSource::Exec, environment_manager, /*analytics_events_client*/ None, + Arc::clone(&thread_store), ); let NewThread { thread_id, thread, .. } = thread_manager - .start_thread(config, thread_store) + .start_thread(config) .await .context("start Codex thread")?; diff --git a/codex-rs/thread-store/src/lib.rs b/codex-rs/thread-store/src/lib.rs index b1adf5e743..52b7f5ea1f 100644 --- a/codex-rs/thread-store/src/lib.rs +++ b/codex-rs/thread-store/src/lib.rs @@ -19,6 +19,7 @@ pub use in_memory::InMemoryThreadStoreCalls; pub use live_thread::LiveThread; pub use live_thread::LiveThreadInitGuard; pub use local::LocalThreadStore; +pub use local::LocalThreadStoreConfig; pub use remote::RemoteThreadStore; pub use store::ThreadStore; pub use types::AppendThreadItemsParams; @@ -37,5 +38,6 @@ pub use types::StoredThreadHistory; pub use types::ThreadEventPersistenceMode; pub use types::ThreadMetadataPatch; pub use types::ThreadPage; +pub use types::ThreadPersistenceMetadata; pub use types::ThreadSortKey; pub use types::UpdateThreadMetadataParams; diff --git a/codex-rs/thread-store/src/local/archive_thread.rs b/codex-rs/thread-store/src/local/archive_thread.rs index 0dd65df72a..5df1d5b761 100644 --- a/codex-rs/thread-store/src/local/archive_thread.rs +++ b/codex-rs/thread-store/src/local/archive_thread.rs @@ -48,7 +48,7 @@ pub(super) async fn archive_thread( } })?; - if let Some(ctx) = codex_rollout::state_db::get_state_db(&store.config).await { + if let Some(ctx) = store.state_db().await { let _ = ctx .mark_archived(thread_id, archived_path.as_path(), Utc::now()) .await; @@ -130,7 +130,7 @@ mod tests { write_session_file(home.path(), "2025-01-03T12-00-00", uuid).expect("session file"); let runtime = codex_state::StateRuntime::init( home.path().to_path_buf(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -144,10 +144,10 @@ mod tests { Utc::now(), SessionSource::Cli, ); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().to_path_buf(); builder.cli_version = Some("test_version".to_string()); - let metadata = builder.build(config.model_provider_id.as_str()); + let metadata = builder.build(config.default_model_provider_id.as_str()); runtime .upsert_thread(&metadata) .await diff --git a/codex-rs/thread-store/src/local/create_thread.rs b/codex-rs/thread-store/src/local/create_thread.rs index 69f19adc6a..e444e5c91d 100644 --- a/codex-rs/thread-store/src/local/create_thread.rs +++ b/codex-rs/thread-store/src/local/create_thread.rs @@ -3,7 +3,9 @@ use crate::CreateThreadParams; use crate::ThreadEventPersistenceMode; use crate::ThreadStoreError; use crate::ThreadStoreResult; +use codex_protocol::protocol::ThreadMemoryMode; use codex_rollout::EventPersistenceMode; +use codex_rollout::RolloutConfig; use codex_rollout::RolloutRecorder; use codex_rollout::RolloutRecorderParams; @@ -11,9 +13,23 @@ pub(super) async fn create_thread( store: &LocalThreadStore, params: CreateThreadParams, ) -> ThreadStoreResult { + let cwd = params + .metadata + .cwd + .clone() + .ok_or_else(|| ThreadStoreError::InvalidRequest { + message: "local thread store requires a cwd".to_string(), + })?; + let config = RolloutConfig { + codex_home: store.config.codex_home.clone(), + sqlite_home: store.config.sqlite_home.clone(), + cwd, + model_provider_id: params.metadata.model_provider.clone(), + generate_memories: matches!(params.metadata.memory_mode, ThreadMemoryMode::Enabled), + }; let state_db_ctx = store.state_db().await; let recorder = RolloutRecorder::new( - &store.config, + &config, RolloutRecorderParams::new( params.thread_id, params.forked_from_id, diff --git a/codex-rs/thread-store/src/local/helpers.rs b/codex-rs/thread-store/src/local/helpers.rs index e7ca821773..0cbf94da8c 100644 --- a/codex-rs/thread-store/src/local/helpers.rs +++ b/codex-rs/thread-store/src/local/helpers.rs @@ -13,6 +13,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GitInfo; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; +use codex_rollout::ARCHIVED_SESSIONS_SUBDIR; use codex_rollout::ThreadItem; use codex_state::ThreadMetadata; @@ -51,6 +52,13 @@ pub(super) fn scoped_rollout_path( } } +pub(super) fn rollout_path_is_archived(codex_home: &Path, path: &Path) -> bool { + path.starts_with(codex_home.join(ARCHIVED_SESSIONS_SUBDIR)) + || path + .components() + .any(|component| component.as_os_str() == OsStr::new(ARCHIVED_SESSIONS_SUBDIR)) +} + pub(super) fn matching_rollout_file_name( rollout_path: &Path, thread_id: ThreadId, diff --git a/codex-rs/thread-store/src/local/list_threads.rs b/codex-rs/thread-store/src/local/list_threads.rs index 42c0e8144b..037bd25085 100644 --- a/codex-rs/thread-store/src/local/list_threads.rs +++ b/codex-rs/thread-store/src/local/list_threads.rs @@ -39,8 +39,16 @@ pub(super) async fn list_threads( SortDirection::Asc => codex_rollout::SortDirection::Asc, SortDirection::Desc => codex_rollout::SortDirection::Desc, }; + let rollout_config = RolloutConfig { + codex_home: store.config.codex_home.clone(), + sqlite_home: store.config.sqlite_home.clone(), + cwd: store.config.codex_home.clone(), + model_provider_id: store.config.default_model_provider_id.clone(), + generate_memories: false, + }; let page = list_rollout_threads( - &store.config, + &rollout_config, + store.config.default_model_provider_id.as_str(), ¶ms, cursor.as_ref(), sort_key, @@ -60,7 +68,7 @@ pub(super) async fn list_threads( stored_thread_from_rollout_item( item, params.archived, - store.config.model_provider_id.as_str(), + store.config.default_model_provider_id.as_str(), ) }) .collect::>(); @@ -99,6 +107,7 @@ pub(super) async fn list_threads( async fn list_rollout_threads( config: &RolloutConfig, + default_model_provider_id: &str, params: &ListThreadsParams, cursor: Option<&codex_rollout::Cursor>, sort_key: codex_rollout::ThreadSortKey, @@ -114,7 +123,7 @@ async fn list_rollout_threads( params.allowed_sources.as_slice(), params.model_providers.as_deref(), params.cwd_filters.as_deref(), - config.model_provider_id.as_str(), + default_model_provider_id, params.search_term.as_deref(), ) .await @@ -128,7 +137,7 @@ async fn list_rollout_threads( params.allowed_sources.as_slice(), params.model_providers.as_deref(), params.cwd_filters.as_deref(), - config.model_provider_id.as_str(), + default_model_provider_id, params.search_term.as_deref(), ) .await @@ -142,7 +151,7 @@ async fn list_rollout_threads( params.allowed_sources.as_slice(), params.model_providers.as_deref(), params.cwd_filters.as_deref(), - config.model_provider_id.as_str(), + default_model_provider_id, params.search_term.as_deref(), ) .await @@ -156,7 +165,7 @@ async fn list_rollout_threads( params.allowed_sources.as_slice(), params.model_providers.as_deref(), params.cwd_filters.as_deref(), - config.model_provider_id.as_str(), + default_model_provider_id, params.search_term.as_deref(), ) .await @@ -230,7 +239,7 @@ mod tests { let runtime = codex_state::StateRuntime::init( home.path().to_path_buf(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -245,10 +254,10 @@ mod tests { created_at, SessionSource::Cli, ); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().to_path_buf(); builder.cli_version = Some("test_version".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.title = "needle title".to_string(); metadata.first_user_message = Some("plain preview".to_string()); runtime diff --git a/codex-rs/thread-store/src/local/live_writer.rs b/codex-rs/thread-store/src/local/live_writer.rs index fd8cd93c79..643207b59d 100644 --- a/codex-rs/thread-store/src/local/live_writer.rs +++ b/codex-rs/thread-store/src/local/live_writer.rs @@ -1,6 +1,8 @@ use std::path::PathBuf; use codex_protocol::ThreadId; +use codex_protocol::protocol::ThreadMemoryMode; +use codex_rollout::RolloutConfig; use codex_rollout::RolloutRecorder; use codex_rollout::RolloutRecorderParams; use codex_rollout::builder_from_items; @@ -55,9 +57,23 @@ pub(super) async fn resume_thread( let state_builder = history .as_deref() .and_then(|items| builder_from_items(items, rollout_path.as_path())); + let cwd = params + .metadata + .cwd + .clone() + .ok_or_else(|| ThreadStoreError::InvalidRequest { + message: "local thread store requires a cwd".to_string(), + })?; + let config = RolloutConfig { + codex_home: store.config.codex_home.clone(), + sqlite_home: store.config.sqlite_home.clone(), + cwd, + model_provider_id: params.metadata.model_provider.clone(), + generate_memories: matches!(params.metadata.memory_mode, ThreadMemoryMode::Enabled), + }; let state_db_ctx = store.state_db().await; let recorder = RolloutRecorder::new( - &store.config, + &config, RolloutRecorderParams::resume( rollout_path, create_thread::event_persistence_mode(params.event_persistence_mode), diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs index a246a41ae5..04dd8b2490 100644 --- a/codex-rs/thread-store/src/local/mod.rs +++ b/codex-rs/thread-store/src/local/mod.rs @@ -12,7 +12,6 @@ mod test_support; use async_trait::async_trait; use codex_protocol::ThreadId; -use codex_rollout::RolloutConfig; use codex_rollout::RolloutRecorder; use codex_rollout::StateDbHandle; use std::collections::HashMap; @@ -41,11 +40,33 @@ use crate::UpdateThreadMetadataParams; /// Local filesystem/SQLite-backed implementation of [`ThreadStore`]. #[derive(Clone)] pub struct LocalThreadStore { - pub(super) config: RolloutConfig, + pub(super) config: LocalThreadStoreConfig, live_recorders: Arc>>, state_db: Arc>, } +/// Process-scoped configuration for local thread storage. +/// +/// This describes where local storage lives. New-thread rollout metadata such +/// as cwd, provider, and memory mode is supplied when live persistence is opened. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct LocalThreadStoreConfig { + pub codex_home: PathBuf, + pub sqlite_home: PathBuf, + /// Provider used only when older local metadata does not contain one. + pub default_model_provider_id: String, +} + +impl LocalThreadStoreConfig { + pub fn from_config(config: &impl codex_rollout::RolloutConfigView) -> Self { + Self { + codex_home: config.codex_home().to_path_buf(), + sqlite_home: config.sqlite_home().to_path_buf(), + default_model_provider_id: config.model_provider_id().to_string(), + } + } +} + impl std::fmt::Debug for LocalThreadStore { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("LocalThreadStore") @@ -55,8 +76,8 @@ impl std::fmt::Debug for LocalThreadStore { } impl LocalThreadStore { - /// Create a local store from the rollout configuration used by existing local persistence. - pub fn new(config: RolloutConfig) -> Self { + /// Create a local store from process-scoped local storage configuration. + pub fn new(config: LocalThreadStoreConfig) -> Self { Self { config, live_recorders: Arc::new(Mutex::new(HashMap::new())), @@ -68,7 +89,13 @@ impl LocalThreadStore { pub async fn state_db(&self) -> Option { self.state_db .get_or_try_init(|| async { - codex_rollout::state_db::init(&self.config).await.ok_or(()) + codex_rollout::state_db::init_with_roots( + self.config.codex_home.clone(), + self.config.sqlite_home.clone(), + self.config.default_model_provider_id.clone(), + ) + .await + .ok_or(()) }) .await .ok() @@ -176,6 +203,16 @@ impl ThreadStore for LocalThreadStore { params: LoadThreadHistoryParams, ) -> ThreadStoreResult { if let Ok(rollout_path) = live_writer::rollout_path(self, params.thread_id).await { + if !params.include_archived + && helpers::rollout_path_is_archived( + self.config.codex_home.as_path(), + rollout_path.as_path(), + ) + { + return Err(ThreadStoreError::InvalidRequest { + message: format!("thread {} is archived", params.thread_id), + }); + } return read_thread::read_thread_by_rollout_path( self, rollout_path, @@ -251,11 +288,13 @@ mod tests { use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::ThreadMemoryMode; use codex_protocol::protocol::UserMessageEvent; use tempfile::TempDir; use super::*; use crate::ThreadEventPersistenceMode; + use crate::ThreadPersistenceMetadata; use crate::local::test_support::test_config; use crate::local::test_support::write_archived_session_file; use crate::local::test_support::write_session_file; @@ -309,6 +348,26 @@ mod tests { ); } + #[tokio::test] + async fn create_thread_rejects_missing_cwd() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + let thread_id = ThreadId::default(); + let mut params = create_thread_params(thread_id); + params.metadata.cwd = None; + + let err = store + .create_thread(params) + .await + .expect_err("local thread store should require cwd"); + + assert!(matches!( + err, + ThreadStoreError::InvalidRequest { message } + if message == "local thread store requires a cwd" + )); + } + #[tokio::test] async fn discard_thread_drops_unmaterialized_live_writer() { let home = TempDir::new().expect("temp dir"); @@ -387,6 +446,7 @@ mod tests { rollout_path: None, history: None, include_archived: true, + metadata: thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, }) .await @@ -427,6 +487,63 @@ mod tests { assert!(err.to_string().contains("already has a live local writer")); } + #[tokio::test] + async fn resume_thread_rejects_duplicate_live_writer() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + let thread_id = ThreadId::default(); + + store + .create_thread(create_thread_params(thread_id)) + .await + .expect("create live thread"); + let rollout_path = store + .live_rollout_path(thread_id) + .await + .expect("live rollout path"); + let err = store + .resume_thread(ResumeThreadParams { + thread_id, + rollout_path: Some(rollout_path), + history: None, + include_archived: true, + metadata: thread_metadata(), + event_persistence_mode: ThreadEventPersistenceMode::Limited, + }) + .await + .expect_err("duplicate live resume should fail"); + assert!(matches!(err, ThreadStoreError::InvalidRequest { .. })); + assert!(err.to_string().contains("already has a live local writer")); + } + + #[tokio::test] + async fn resume_thread_rejects_missing_cwd() { + let home = TempDir::new().expect("temp dir"); + let store = LocalThreadStore::new(test_config(home.path())); + let uuid = uuid::Uuid::from_u128(407); + let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id"); + let rollout_path = + write_session_file(home.path(), "2025-01-04T11-30-00", uuid).expect("session file"); + let err = store + .resume_thread(ResumeThreadParams { + thread_id, + rollout_path: Some(rollout_path), + history: None, + include_archived: true, + metadata: ThreadPersistenceMetadata { + cwd: None, + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Enabled, + }, + event_persistence_mode: ThreadEventPersistenceMode::Limited, + }) + .await + .expect_err("missing cwd should fail"); + + assert!(matches!(err, ThreadStoreError::InvalidRequest { .. })); + assert!(err.to_string().contains("requires a cwd")); + } + #[tokio::test] async fn load_history_uses_live_writer_rollout_path() { let home = TempDir::new().expect("temp dir"); @@ -443,6 +560,7 @@ mod tests { rollout_path: Some(rollout_path), history: None, include_archived: true, + metadata: thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, }) .await @@ -475,6 +593,46 @@ mod tests { })); } + #[tokio::test] + async fn read_thread_uses_live_writer_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::Uuid::from_u128(406); + let thread_id = ThreadId::from_string(&uuid.to_string()).expect("valid thread id"); + let rollout_path = write_session_file(external_home.path(), "2025-01-04T11-00-00", uuid) + .expect("external session file"); + + store + .resume_thread(ResumeThreadParams { + thread_id, + rollout_path: Some(rollout_path.clone()), + history: None, + include_archived: true, + metadata: thread_metadata(), + event_persistence_mode: ThreadEventPersistenceMode::Limited, + }) + .await + .expect("resume live thread"); + + let thread = store + .read_thread(ReadThreadParams { + thread_id, + include_archived: false, + include_history: true, + }) + .await + .expect("read external live thread"); + + assert_eq!(thread.rollout_path, Some(rollout_path)); + assert!(thread.history.expect("history").items.iter().any(|item| { + matches!( + item, + RolloutItem::EventMsg(EventMsg::UserMessage(event)) if event.message == "Hello from user" + ) + })); + } + #[tokio::test] async fn load_history_uses_live_writer_rollout_path_for_archived_source() { let home = TempDir::new().expect("temp dir"); @@ -490,6 +648,7 @@ mod tests { rollout_path: Some(rollout_path), history: None, include_archived: true, + metadata: thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, }) .await @@ -506,12 +665,32 @@ mod tests { .await .expect("flush live thread"); - let history = store + let err = store + .read_thread(ReadThreadParams { + thread_id, + include_archived: false, + include_history: false, + }) + .await + .expect_err("active-only read should reject archived live thread"); + assert!(matches!(err, ThreadStoreError::InvalidRequest { .. })); + + let err = store .load_history(LoadThreadHistoryParams { thread_id, include_archived: false, }) .await + .expect_err("active-only history should reject archived live thread"); + assert!(matches!(err, ThreadStoreError::InvalidRequest { .. })); + assert!(err.to_string().contains("archived")); + + let history = store + .load_history(LoadThreadHistoryParams { + thread_id, + include_archived: true, + }) + .await .expect("load archived live history"); assert!(history.items.iter().any(|item| { @@ -574,10 +753,19 @@ mod tests { source: SessionSource::Exec, base_instructions: BaseInstructions::default(), dynamic_tools: Vec::new(), + metadata: thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, } } + fn thread_metadata() -> ThreadPersistenceMetadata { + ThreadPersistenceMetadata { + cwd: Some(std::env::current_dir().expect("cwd")), + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Enabled, + } + } + fn user_message_item(message: &str) -> RolloutItem { RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent { message: message.to_string(), diff --git a/codex-rs/thread-store/src/local/read_thread.rs b/codex-rs/thread-store/src/local/read_thread.rs index 169a8ce6db..8b3d3160db 100644 --- a/codex-rs/thread-store/src/local/read_thread.rs +++ b/codex-rs/thread-store/src/local/read_thread.rs @@ -16,8 +16,10 @@ use codex_state::ThreadMetadata; use super::LocalThreadStore; use super::helpers::distinct_thread_metadata_title; use super::helpers::git_info_from_parts; +use super::helpers::rollout_path_is_archived; use super::helpers::set_thread_name_from_title; use super::helpers::stored_thread_from_rollout_item; +use super::live_writer; use crate::ReadThreadParams; use crate::StoredThread; use crate::StoredThreadHistory; @@ -30,7 +32,12 @@ pub(super) async fn read_thread( ) -> ThreadStoreResult { let thread_id = params.thread_id; if let Some(metadata) = read_sqlite_metadata(store, thread_id).await - && (params.include_archived || metadata.archived_at.is_none()) + && (params.include_archived + || (metadata.archived_at.is_none() + && !rollout_path_is_archived( + store.config.codex_home.as_path(), + metadata.rollout_path.as_path(), + ))) && (!params.include_history || sqlite_rollout_path_can_load_history_for_thread( store, @@ -44,6 +51,7 @@ pub(super) async fn read_thread( && let Some(rollout_path) = thread.rollout_path.clone() && let Ok(mut rollout_thread) = read_thread_from_rollout_path(store, rollout_path).await && rollout_thread.thread_id == thread_id + && (params.include_archived || rollout_thread.archived_at.is_none()) && !rollout_thread.preview.is_empty() { if thread.name.is_some() { @@ -153,6 +161,17 @@ async fn resolve_rollout_path( thread_id: codex_protocol::ThreadId, include_archived: bool, ) -> ThreadStoreResult> { + if let Ok(path) = live_writer::rollout_path(store, thread_id).await + && tokio::fs::try_exists(path.as_path()).await.map_err(|err| { + ThreadStoreError::InvalidRequest { + message: format!("failed to check rollout path for thread id {thread_id}: {err}"), + } + })? + && (include_archived || !rollout_path_is_archived(store.config.codex_home.as_path(), &path)) + { + return Ok(Some(path)); + } + if include_archived { match find_thread_path_by_id_str(store.config.codex_home.as_path(), &thread_id.to_string()) .await @@ -185,21 +204,25 @@ async fn read_thread_from_rollout_path( let Some(item) = read_thread_item_from_rollout(path.clone()).await else { return stored_thread_from_session_meta(store, path).await; }; - let archived = path.starts_with( - store - .config - .codex_home - .join(codex_rollout::ARCHIVED_SESSIONS_SUBDIR), - ); - let mut thread = - stored_thread_from_rollout_item(item, archived, store.config.model_provider_id.as_str()) - .ok_or_else(|| ThreadStoreError::Internal { - message: format!("failed to read thread id from {}", path.display()), - })?; - thread.forked_from_id = read_session_meta_line(path.as_path()) - .await - .ok() - .and_then(|meta_line| meta_line.meta.forked_from_id); + let archived = rollout_path_is_archived(store.config.codex_home.as_path(), path.as_path()); + let mut thread = stored_thread_from_rollout_item( + item, + archived, + store.config.default_model_provider_id.as_str(), + ) + .ok_or_else(|| ThreadStoreError::Internal { + message: format!("failed to read thread id from {}", path.display()), + })?; + if let Ok(meta_line) = read_session_meta_line(path.as_path()).await { + thread.forked_from_id = meta_line.meta.forked_from_id; + if let Some(model_provider) = meta_line + .meta + .model_provider + .filter(|provider| !provider.is_empty()) + { + thread.model_provider = model_provider; + } + } if let Ok(Some(title)) = find_thread_name_by_id(store.config.codex_home.as_path(), &thread.thread_id).await { @@ -225,7 +248,7 @@ async fn read_sqlite_metadata( ) -> Option { let runtime = StateRuntime::init( store.config.sqlite_home.clone(), - store.config.model_provider_id.clone(), + store.config.default_model_provider_id.clone(), ) .await .ok()?; @@ -254,7 +277,7 @@ async fn stored_thread_from_sqlite_metadata( preview: metadata.first_user_message.clone().unwrap_or_default(), name, model_provider: if metadata.model_provider.is_empty() { - store.config.model_provider_id.clone() + store.config.default_model_provider_id.clone() } else { metadata.model_provider }, @@ -294,12 +317,7 @@ async fn stored_thread_from_session_meta( .map_err(|err| ThreadStoreError::Internal { message: format!("failed to read thread {}: {err}", path.display()), })?; - let archived = path.starts_with( - store - .config - .codex_home - .join(codex_rollout::ARCHIVED_SESSIONS_SUBDIR), - ); + let archived = rollout_path_is_archived(store.config.codex_home.as_path(), path.as_path()); Ok(stored_thread_from_meta_line( store, meta_line, path, archived, )) @@ -327,7 +345,7 @@ fn stored_thread_from_meta_line( .meta .model_provider .filter(|provider| !provider.is_empty()) - .unwrap_or_else(|| store.config.model_provider_id.clone()), + .unwrap_or_else(|| store.config.default_model_provider_id.clone()), model: None, reasoning_effort: None, created_at, @@ -459,7 +477,7 @@ mod tests { write_session_file(home.path(), "2025-01-03T12-00-00", uuid).expect("session file"); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -469,10 +487,10 @@ mod tests { Utc::now(), SessionSource::Cli, ); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.git_branch = Some("sqlite-branch".to_string()); runtime - .upsert_thread(&builder.build(config.model_provider_id.as_str())) + .upsert_thread(&builder.build(config.default_model_provider_id.as_str())) .await .expect("state db upsert should succeed"); @@ -606,16 +624,16 @@ mod tests { write_session_file(home.path(), "2025-01-03T12-00-00", uuid).expect("session file"); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); let mut builder = ThreadMetadataBuilder::new(thread_id, rollout_path, Utc::now(), SessionSource::Cli); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().to_path_buf(); builder.cli_version = Some("test_version".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.title = "Saved title".to_string(); metadata.first_user_message = Some("Hello from user".to_string()); runtime @@ -674,7 +692,7 @@ mod tests { let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -684,9 +702,9 @@ mod tests { Utc::now(), SessionSource::Cli, ); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().join("sqlite-workspace"); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.title = "Saved title".to_string(); metadata.first_user_message = Some("Hello from sqlite".to_string()); runtime @@ -707,6 +725,7 @@ mod tests { assert_eq!(thread.rollout_path, Some(rollout_path)); assert_eq!(thread.preview, "Hello from rollout"); assert_eq!(thread.name, Some("Saved title".to_string())); + assert_eq!(thread.model_provider, "rollout-provider"); assert_eq!(thread.cwd, rollout_cwd); } @@ -761,7 +780,7 @@ mod tests { let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -774,7 +793,7 @@ mod tests { builder.model_provider = Some("sqlite-provider".to_string()); builder.cwd = home.path().join("workspace"); builder.cli_version = Some("sqlite-cli".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.title = "Command-only thread".to_string(); runtime .upsert_thread(&metadata) @@ -815,7 +834,7 @@ mod tests { let stale_path = external.path().join("missing-rollout.jsonl"); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -826,7 +845,7 @@ mod tests { SessionSource::Cli, ); builder.model_provider = Some("stale-sqlite-provider".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.first_user_message = Some("stale sqlite preview".to_string()); runtime .upsert_thread(&metadata) @@ -845,7 +864,7 @@ mod tests { assert_eq!(thread.thread_id, thread_id); assert_eq!(thread.rollout_path, Some(rollout_path)); assert_eq!(thread.preview, "Hello from user"); - assert_eq!(thread.model_provider, config.model_provider_id); + assert_eq!(thread.model_provider, config.default_model_provider_id); let history = thread.history.expect("history should load"); assert_eq!(history.thread_id, thread_id); assert_eq!(history.items.len(), 2); @@ -866,14 +885,14 @@ mod tests { .expect("other session file"); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); let mut builder = ThreadMetadataBuilder::new(thread_id, stale_path, Utc::now(), SessionSource::Cli); builder.model_provider = Some("wrong-sqlite-provider".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.first_user_message = Some("wrong sqlite preview".to_string()); runtime .upsert_thread(&metadata) @@ -892,7 +911,7 @@ mod tests { assert_eq!(thread.thread_id, thread_id); assert_eq!(thread.rollout_path, Some(rollout_path)); assert_eq!(thread.preview, "Hello from user"); - assert_eq!(thread.model_provider, config.model_provider_id); + assert_eq!(thread.model_provider, config.default_model_provider_id); let history = thread.history.expect("history should load"); assert_eq!(history.thread_id, thread_id); assert_eq!(history.items.len(), 2); @@ -964,7 +983,7 @@ mod tests { .join(format!("rollout-2025-01-03T12-00-00-{uuid}.jsonl")); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -977,7 +996,7 @@ mod tests { builder.model_provider = Some("sqlite-provider".to_string()); builder.cwd = external.path().join("workspace"); builder.cli_version = Some("sqlite-cli".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.title = "SQLite title".to_string(); metadata.first_user_message = Some("SQLite preview".to_string()); metadata.model = Some("sqlite-model".to_string()); @@ -1022,14 +1041,14 @@ mod tests { .join(format!("rollout-2025-01-03T12-00-00-{uuid}.jsonl")); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); let mut builder = ThreadMetadataBuilder::new(thread_id, rollout_path, Utc::now(), SessionSource::Cli); builder.archived_at = Some(Utc::now()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.first_user_message = Some("Archived SQLite preview".to_string()); runtime .upsert_thread(&metadata) @@ -1077,7 +1096,7 @@ mod tests { .expect("archived session file"); let runtime = codex_state::StateRuntime::init( config.sqlite_home.clone(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -1088,7 +1107,7 @@ mod tests { SessionSource::Cli, ); builder.archived_at = Some(Utc::now()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.first_user_message = Some("Archived SQLite preview".to_string()); runtime .upsert_thread(&metadata) diff --git a/codex-rs/thread-store/src/local/test_support.rs b/codex-rs/thread-store/src/local/test_support.rs index 4656503a6a..98321880ff 100644 --- a/codex-rs/thread-store/src/local/test_support.rs +++ b/codex-rs/thread-store/src/local/test_support.rs @@ -4,16 +4,15 @@ use std::path::Path; use std::path::PathBuf; use codex_rollout::ARCHIVED_SESSIONS_SUBDIR; -use codex_rollout::RolloutConfig; use uuid::Uuid; -pub(super) fn test_config(codex_home: &Path) -> RolloutConfig { - RolloutConfig { +use super::LocalThreadStoreConfig; + +pub(super) fn test_config(codex_home: &Path) -> LocalThreadStoreConfig { + LocalThreadStoreConfig { codex_home: codex_home.to_path_buf(), sqlite_home: codex_home.to_path_buf(), - cwd: codex_home.to_path_buf(), - model_provider_id: "test-provider".to_string(), - generate_memories: true, + default_model_provider_id: "test-provider".to_string(), } } diff --git a/codex-rs/thread-store/src/local/unarchive_thread.rs b/codex-rs/thread-store/src/local/unarchive_thread.rs index 17e484bace..8a3ab2960a 100644 --- a/codex-rs/thread-store/src/local/unarchive_thread.rs +++ b/codex-rs/thread-store/src/local/unarchive_thread.rs @@ -71,7 +71,7 @@ pub(super) async fn unarchive_thread( message: format!("failed to update unarchived thread timestamp: {err}"), })?; - if let Some(ctx) = codex_rollout::state_db::get_state_db(&store.config).await { + if let Some(ctx) = store.state_db().await { let _ = ctx .mark_unarchived(thread_id, restored_path.as_path()) .await; @@ -88,7 +88,7 @@ pub(super) async fn unarchive_thread( stored_thread_from_rollout_item( item, /*archived*/ false, - store.config.model_provider_id.as_str(), + store.config.default_model_provider_id.as_str(), ) .ok_or_else(|| ThreadStoreError::Internal { message: format!( @@ -154,7 +154,7 @@ mod tests { .expect("archived session file"); let runtime = codex_state::StateRuntime::init( home.path().to_path_buf(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -168,10 +168,10 @@ mod tests { Utc::now(), SessionSource::Cli, ); - builder.model_provider = Some(config.model_provider_id.clone()); + builder.model_provider = Some(config.default_model_provider_id.clone()); builder.cwd = home.path().to_path_buf(); builder.cli_version = Some("test_version".to_string()); - let mut metadata = builder.build(config.model_provider_id.as_str()); + let mut metadata = builder.build(config.default_model_provider_id.as_str()); metadata.archived_at = Some(metadata.updated_at); runtime .upsert_thread(&metadata) diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index 52c937c6bb..fba0172525 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -58,7 +58,7 @@ pub(super) async fn update_thread_metadata( codex_rollout::state_db::reconcile_rollout( state_db_ctx.as_deref(), resolved_rollout_path.path.as_path(), - store.config.model_provider_id.as_str(), + store.config.default_model_provider_id.as_str(), /*builder*/ None, &[], /*archived_only*/ resolved_rollout_path.archived.then_some(true), @@ -203,6 +203,7 @@ mod tests { use crate::ResumeThreadParams; use crate::ThreadEventPersistenceMode; use crate::ThreadMetadataPatch; + use crate::ThreadPersistenceMetadata; use crate::ThreadStore; use crate::local::LocalThreadStore; use crate::local::test_support::test_config; @@ -254,7 +255,7 @@ mod tests { 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(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -299,6 +300,7 @@ mod tests { rollout_path: Some(path.clone()), history: None, include_archived: true, + metadata: test_thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, }) .await @@ -400,7 +402,7 @@ mod tests { .expect("archived session file"); let runtime = codex_state::StateRuntime::init( home.path().to_path_buf(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -411,7 +413,7 @@ mod tests { codex_rollout::state_db::reconcile_rollout( Some(runtime.as_ref()), archived_path.as_path(), - config.model_provider_id.as_str(), + config.default_model_provider_id.as_str(), /*builder*/ None, &[], /*archived_only*/ Some(true), @@ -463,7 +465,7 @@ mod tests { .expect("archived session file"); let runtime = codex_state::StateRuntime::init( home.path().to_path_buf(), - config.model_provider_id.clone(), + config.default_model_provider_id.clone(), ) .await .expect("state db should initialize"); @@ -474,7 +476,7 @@ mod tests { codex_rollout::state_db::reconcile_rollout( Some(runtime.as_ref()), archived_path.as_path(), - config.model_provider_id.as_str(), + config.default_model_provider_id.as_str(), /*builder*/ None, &[], /*archived_only*/ Some(true), @@ -487,6 +489,7 @@ mod tests { rollout_path: Some(archived_path.clone()), history: None, include_archived: true, + metadata: test_thread_metadata(), event_persistence_mode: ThreadEventPersistenceMode::Limited, }) .await @@ -516,6 +519,14 @@ mod tests { ); } + fn test_thread_metadata() -> ThreadPersistenceMetadata { + ThreadPersistenceMetadata { + cwd: Some(std::env::current_dir().expect("cwd")), + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Enabled, + } + } + fn last_rollout_item(path: &std::path::Path) -> Value { let last_line = std::fs::read_to_string(path) .expect("read rollout") diff --git a/codex-rs/thread-store/src/remote/helpers.rs b/codex-rs/thread-store/src/remote/helpers.rs index 8b0a739225..74b3ac7763 100644 --- a/codex-rs/thread-store/src/remote/helpers.rs +++ b/codex-rs/thread-store/src/remote/helpers.rs @@ -25,6 +25,7 @@ use crate::StoredThread; use crate::StoredThreadHistory; use crate::ThreadEventPersistenceMode; use crate::ThreadMetadataPatch; +use crate::ThreadPersistenceMetadata; use crate::ThreadSortKey; use crate::ThreadStoreError; use crate::ThreadStoreResult; @@ -186,6 +187,12 @@ pub(super) fn dynamic_tools_json( serialize_json_vec(dynamic_tools, "dynamic_tool") } +pub(super) fn thread_persistence_metadata_json( + metadata: &ThreadPersistenceMetadata, +) -> ThreadStoreResult { + serialize_json(metadata, "thread_persistence_metadata") +} + pub(super) fn rollout_items_json(items: &[RolloutItem]) -> ThreadStoreResult> { serialize_json_vec(items, "rollout_item") } diff --git a/codex-rs/thread-store/src/remote/mod.rs b/codex-rs/thread-store/src/remote/mod.rs index a3befac4e5..3e74a45f4b 100644 --- a/codex-rs/thread-store/src/remote/mod.rs +++ b/codex-rs/thread-store/src/remote/mod.rs @@ -69,6 +69,7 @@ impl ThreadStore for RemoteThreadStore { params.event_persistence_mode, ) .into(), + metadata_json: helpers::thread_persistence_metadata_json(¶ms.metadata)?, }; self.client() .await? @@ -96,6 +97,7 @@ impl ThreadStore for RemoteThreadStore { params.event_persistence_mode, ) .into(), + metadata_json: helpers::thread_persistence_metadata_json(¶ms.metadata)?, }; self.client() .await? @@ -260,3 +262,148 @@ impl ThreadStore for RemoteThreadStore { helpers::stored_thread_from_proto(thread) } } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use codex_protocol::ThreadId; + use codex_protocol::models::BaseInstructions; + use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::ThreadMemoryMode; + use pretty_assertions::assert_eq; + use tokio::sync::mpsc; + use tonic::Request; + use tonic::Response; + use tonic::Status; + use tonic::transport::Server; + + use super::*; + use crate::ThreadEventPersistenceMode; + use crate::ThreadPersistenceMetadata; + use proto::thread_store_server; + use proto::thread_store_server::ThreadStoreServer; + + enum RecordedRequest { + Create(proto::CreateThreadRequest), + Resume(proto::ResumeThreadRequest), + } + + struct TestServer { + requests_tx: mpsc::UnboundedSender, + } + + #[tonic::async_trait] + impl thread_store_server::ThreadStore for TestServer { + async fn create_thread( + &self, + request: Request, + ) -> Result, Status> { + self.requests_tx + .send(RecordedRequest::Create(request.into_inner())) + .expect("record create request"); + Ok(Response::new(proto::Empty {})) + } + + async fn resume_thread( + &self, + request: Request, + ) -> Result, Status> { + self.requests_tx + .send(RecordedRequest::Resume(request.into_inner())) + .expect("record resume request"); + Ok(Response::new(proto::Empty {})) + } + + async fn list_threads( + &self, + _request: Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented")) + } + } + + async fn test_store() -> (RemoteThreadStore, mpsc::UnboundedReceiver) { + let (requests_tx, requests_rx) = mpsc::unbounded_channel(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind test server"); + let addr = listener.local_addr().expect("test server addr"); + + tokio::spawn(async move { + Server::builder() + .add_service(ThreadStoreServer::new(TestServer { requests_tx })) + .serve_with_incoming(tokio_stream::wrappers::TcpListenerStream::new(listener)) + .await + .expect("test server"); + }); + + ( + RemoteThreadStore::new(format!("http://{addr}")), + requests_rx, + ) + } + + #[tokio::test] + async fn create_thread_forwards_metadata() { + let (store, mut requests_rx) = test_store().await; + let metadata = ThreadPersistenceMetadata { + cwd: Some(PathBuf::from("/workspace")), + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Enabled, + }; + + store + .create_thread(CreateThreadParams { + thread_id: ThreadId::new(), + forked_from_id: None, + source: SessionSource::Exec, + base_instructions: BaseInstructions::default(), + dynamic_tools: Vec::new(), + metadata: metadata.clone(), + event_persistence_mode: ThreadEventPersistenceMode::Limited, + }) + .await + .expect("create thread"); + + let Some(RecordedRequest::Create(request)) = requests_rx.recv().await else { + panic!("expected create request"); + }; + assert_eq!( + serde_json::from_str::(&request.metadata_json) + .expect("metadata json"), + metadata + ); + } + + #[tokio::test] + async fn resume_thread_forwards_metadata() { + let (store, mut requests_rx) = test_store().await; + let metadata = ThreadPersistenceMetadata { + cwd: Some(PathBuf::from("/workspace")), + model_provider: "test-provider".to_string(), + memory_mode: ThreadMemoryMode::Disabled, + }; + + store + .resume_thread(ResumeThreadParams { + thread_id: ThreadId::new(), + rollout_path: None, + history: None, + include_archived: false, + metadata: metadata.clone(), + event_persistence_mode: ThreadEventPersistenceMode::Limited, + }) + .await + .expect("resume thread"); + + let Some(RecordedRequest::Resume(request)) = requests_rx.recv().await else { + panic!("expected resume request"); + }; + assert_eq!( + serde_json::from_str::(&request.metadata_json) + .expect("metadata json"), + metadata + ); + } +} diff --git a/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.proto b/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.proto index 06a3cbd874..7c797f139a 100644 --- a/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.proto +++ b/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.proto @@ -31,6 +31,7 @@ message CreateThreadRequest { string base_instructions_json = 4; repeated string dynamic_tools_json = 5; ThreadEventPersistenceMode event_persistence_mode = 6; + string metadata_json = 7; } message ResumeThreadRequest { @@ -40,6 +41,7 @@ message ResumeThreadRequest { bool has_history = 4; bool include_archived = 5; ThreadEventPersistenceMode event_persistence_mode = 6; + string metadata_json = 7; } message AppendThreadItemsRequest { diff --git a/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.rs b/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.rs index bf5612764c..a210ef8766 100644 --- a/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.rs +++ b/codex-rs/thread-store/src/remote/proto/codex.thread_store.v1.rs @@ -22,6 +22,8 @@ pub struct CreateThreadRequest { pub dynamic_tools_json: ::prost::alloc::vec::Vec<::prost::alloc::string::String>, #[prost(enumeration = "ThreadEventPersistenceMode", tag = "6")] pub event_persistence_mode: i32, + #[prost(string, tag = "7")] + pub metadata_json: ::prost::alloc::string::String, } #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct ResumeThreadRequest { @@ -37,6 +39,8 @@ pub struct ResumeThreadRequest { pub include_archived: bool, #[prost(enumeration = "ThreadEventPersistenceMode", tag = "6")] pub event_persistence_mode: i32, + #[prost(string, tag = "7")] + pub metadata_json: ::prost::alloc::string::String, } #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)] pub struct AppendThreadItemsRequest { diff --git a/codex-rs/thread-store/src/types.rs b/codex-rs/thread-store/src/types.rs index f019bcb29a..85bde023bd 100644 --- a/codex-rs/thread-store/src/types.rs +++ b/codex-rs/thread-store/src/types.rs @@ -26,6 +26,19 @@ pub enum ThreadEventPersistenceMode { Extended, } +/// Thread-scoped metadata used when opening live persistence. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ThreadPersistenceMetadata { + /// Effective working directory for environment-backed threads. + /// + /// `None` means the thread has no filesystem/environment context. + pub cwd: Option, + /// Model provider associated with the thread. + pub model_provider: String, + /// Memory mode associated with the live thread. + pub memory_mode: MemoryMode, +} + /// Parameters required to create a persisted thread. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct CreateThreadParams { @@ -39,6 +52,8 @@ pub struct CreateThreadParams { pub base_instructions: BaseInstructions, /// Dynamic tools available to the thread at startup. pub dynamic_tools: Vec, + /// Metadata captured for the newly created thread. + pub metadata: ThreadPersistenceMetadata, /// Whether persistence should include the extended event surface. pub event_persistence_mode: ThreadEventPersistenceMode, } @@ -54,6 +69,8 @@ pub struct ResumeThreadParams { pub history: Option>, /// Whether archived threads may be reopened. pub include_archived: bool, + /// Metadata for future writes appended to the resumed live thread. + pub metadata: ThreadPersistenceMetadata, /// Whether persistence should include the extended event surface. pub event_persistence_mode: ThreadEventPersistenceMode, } diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 9caa4a93a6..47f57cd82c 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1592,6 +1592,7 @@ fn update_memory_settings_updates_current_thread_memory_mode() -> Result<()> { let (mut app, _app_event_rx, _op_rx) = Box::pin(make_test_app_with_channels()).await; let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf().abs(); + app.config.sqlite_home = codex_home.path().to_path_buf(); // Seed the previous setting so this test exercises the thread-mode update path. app.config.memories.generate_memories = true; From d898cc8f3f41217a1020fe5e3255047465076e25 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 30 Apr 2026 22:42:07 -0700 Subject: [PATCH 02/29] Format multi-day goal durations in the TUI (#20558) ## Why Goal mode shows elapsed time in compact hour/minute form. That is easy to scan for shorter runs, but once a goal runs past 24 hours, large hour counts become harder to read at a glance. ## What changed Updated `codex-rs/tui/src/goal_display.rs` so unbudgeted goal elapsed time keeps the existing compact format below one day, then switches to a day-aware format once the elapsed time reaches 24 hours: - `23h 59m` - `1d 0h 0m` - `2d 23h 42m` The formatter now covers the 24-hour boundary in unit tests, and the TUI status-line snapshot for a completed elapsed goal now exercises the multi-day display. ## Verification - `cargo test -p codex-tui` Here's my longest-running test task: image --- ...__status_line_goal_complete_elapsed_footer.snap | 2 +- .../tui/src/chatwidget/tests/status_and_layout.rs | 12 +++++++----- codex-rs/tui/src/goal_display.rs | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_complete_elapsed_footer.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_complete_elapsed_footer.snap index 9b9ba2c999..97253d2ec9 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_complete_elapsed_footer.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_goal_complete_elapsed_footer.snap @@ -6,4 +6,4 @@ expression: normalized_backend_snapshot(terminal.backend()) " " "› Ask Codex to do anything " " " -" gpt-5.4 Goal achieved (30m) " +" gpt-5.4 Goal achieved (2d 23h 42m) " diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index bce32210d9..36031e9832 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1626,16 +1626,18 @@ async fn status_line_goal_complete_elapsed_footer_snapshot() { chat.show_welcome_banner = false; chat.config.tui_status_line = Some(vec!["model-name".to_string()]); chat.refresh_status_line(); + let mut goal = test_thread_goal( + codex_app_server_protocol::ThreadGoalStatus::Complete, + /*token_budget*/ None, + /*tokens_used*/ 40_000, + ); + goal.time_used_seconds = 2 * 24 * 60 * 60 + 23 * 60 * 60 + 42 * 60; chat.handle_server_notification( ServerNotification::ThreadGoalUpdated( codex_app_server_protocol::ThreadGoalUpdatedNotification { thread_id: "thread-1".to_string(), turn_id: None, - goal: test_thread_goal( - codex_app_server_protocol::ThreadGoalStatus::Complete, - /*token_budget*/ None, - /*tokens_used*/ 40_000, - ), + goal, }, ), /*replay_kind*/ None, diff --git a/codex-rs/tui/src/goal_display.rs b/codex-rs/tui/src/goal_display.rs index 1fdcadd902..d0455054b7 100644 --- a/codex-rs/tui/src/goal_display.rs +++ b/codex-rs/tui/src/goal_display.rs @@ -15,6 +15,12 @@ pub(crate) fn format_goal_elapsed_seconds(seconds: i64) -> String { let hours = minutes / 60; let remaining_minutes = minutes % 60; + if hours >= 24 { + let days = hours / 24; + let remaining_hours = hours % 24; + return format!("{days}d {remaining_hours}h {remaining_minutes}m"); + } + if remaining_minutes == 0 { format!("{hours}h") } else { @@ -64,6 +70,14 @@ mod tests { assert_eq!(format_goal_elapsed_seconds(30 * 60), "30m"); assert_eq!(format_goal_elapsed_seconds(90 * 60), "1h 30m"); assert_eq!(format_goal_elapsed_seconds(2 * 60 * 60), "2h"); + let just_before_one_day = 24 * 60 * 60 - 1; + assert_eq!(format_goal_elapsed_seconds(just_before_one_day), "23h 59m"); + + let one_day = 24 * 60 * 60; + assert_eq!(format_goal_elapsed_seconds(one_day), "1d 0h 0m"); + + let almost_three_days = 2 * 24 * 60 * 60 + 23 * 60 * 60 + 42 * 60; + assert_eq!(format_goal_elapsed_seconds(almost_three_days), "2d 23h 42m"); } fn test_thread_goal(token_budget: Option, tokens_used: i64) -> ThreadGoal { From a93c89f4972d9c6a493688f3f4d35aad74c498e1 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 30 Apr 2026 22:42:48 -0700 Subject: [PATCH 03/29] Color TUI statusline from active theme (#19631) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Users have shared that the TUI can feel too visually flat because themes mostly show up in code syntax highlighting. The configurable statusline is a natural place to make the active theme more visible, while still letting users keep the existing monotone statusline if they prefer it. ## What Changed - Added a statusline styling helper that builds the rendered statusline from `(StatusLineItem, text)` segments, preserving item identity while keeping the plain text output unchanged. - Derived foreground accent colors from the active syntax theme by looking up TextMate scopes through the existing syntax highlighter, with conservative ANSI fallbacks when a scope does not provide a foreground. - Tuned theme-derived colors to keep the accents visible without making the statusline feel overly bright. - Added `[tui].status_line_use_colors`, defaulting to `true`, plus a separated `/statusline` toggle so users can enable or disable theme-derived statusline colors from the setup UI. - Updated the live statusline and `/statusline` preview to use the same styled builder, while keeping terminal-title preview text plain. - Kept statusline separators and active-agent add-ons subdued while removing blanket dimming from the whole passive statusline. ## Verification - `cargo test -p codex-tui status_line` - `cargo test -p codex-tui theme_picker` - `cargo test -p codex-tui foreground_style_for_scopes` - `cargo test -p codex-tui` - `cargo test -p codex-config` - `cargo test -p codex-core status_line_use_colors` - `cargo insta pending-snapshots --manifest-path tui/Cargo.toml` ## Visual Screenshot 2026-04-30 at 6 16 08 PM Screenshot 2026-04-30 at 6 16 02 PM --- codex-rs/config/src/types.rs | 5 + codex-rs/core/config.schema.json | 5 + codex-rs/core/src/config/config_tests.rs | 37 +++ codex-rs/core/src/config/edit.rs | 8 + codex-rs/core/src/config/mod.rs | 8 + codex-rs/thread-manager-sample/src/main.rs | 1 + codex-rs/tui/src/app/event_dispatch.rs | 26 +- codex-rs/tui/src/app/tests.rs | 6 +- codex-rs/tui/src/app_event.rs | 4 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 1 - codex-rs/tui/src/bottom_pane/footer.rs | 24 +- codex-rs/tui/src/bottom_pane/mod.rs | 2 + .../src/bottom_pane/multi_select_picker.rs | 224 +++++++++++++-- ..._snapshot_uses_runtime_preview_values.snap | 6 +- .../tui/src/bottom_pane/status_line_setup.rs | 108 +++++-- .../tui/src/bottom_pane/status_line_style.rs | 270 ++++++++++++++++++ .../src/bottom_pane/status_surface_preview.rs | 26 +- codex-rs/tui/src/bottom_pane/title_setup.rs | 2 + codex-rs/tui/src/chatwidget.rs | 8 +- ...tatus_line_setup_popup_hardcoded_only.snap | 6 +- ...ts__status_line_setup_popup_live_only.snap | 6 +- ..._tests__status_line_setup_popup_mixed.snap | 6 +- .../tui/src/chatwidget/status_surfaces.rs | 41 ++- .../src/chatwidget/tests/status_and_layout.rs | 2 +- .../tests/status_surface_previews.rs | 2 +- codex-rs/tui/src/render/highlight.rs | 66 +++++ codex-rs/tui/src/theme_picker.rs | 27 +- 27 files changed, 789 insertions(+), 138 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/status_line_style.rs diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 43d79ed901..91925fbeb4 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -634,6 +634,11 @@ pub struct Tui { #[serde(default)] pub status_line: Option>, + /// Color status line items with colors derived from the active syntax theme. + /// Defaults to `true`. + #[serde(default = "default_true")] + pub status_line_use_colors: bool, + /// Ordered list of terminal title item identifiers. /// /// When set, the TUI renders the selected items into the terminal window/tab title. diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index d7b7bba6a0..c8397418da 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2528,6 +2528,11 @@ }, "type": "array" }, + "status_line_use_colors": { + "default": true, + "description": "Color status line items with colors derived from the active syntax theme. Defaults to `true`.", + "type": "boolean" + }, "terminal_resize_reflow_max_rows": { "default": null, "description": "Trim terminal resize-reflow replay to the most recent rendered terminal rows when the transcript exceeds this cap. Omit to use Codex's terminal-specific default. Set to `0` to keep all rendered rows.", diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 14affc4fc3..aeee21cf70 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -552,6 +552,7 @@ fn config_toml_deserializes_model_availability_nux() { vim_mode_default: false, alternate_screen: AltScreenMode::default(), status_line: None, + status_line_use_colors: true, terminal_title: None, theme: None, keymap: TuiKeymap::default(), @@ -566,6 +567,37 @@ fn config_toml_deserializes_model_availability_nux() { ); } +#[test] +fn config_toml_status_line_use_colors_defaults_to_enabled() { + let toml = r#" +[tui] +"#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for TUI config"); + + assert!( + cfg.tui + .expect("tui config should deserialize") + .status_line_use_colors + ); +} + +#[test] +fn config_toml_deserializes_status_line_use_colors_disabled() { + let toml = r#" +[tui] +status_line_use_colors = false +"#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for TUI config"); + + assert!( + !cfg.tui + .expect("tui config should deserialize") + .status_line_use_colors + ); +} + #[test] fn config_toml_deserializes_terminal_resize_reflow_config() { let toml = r#" @@ -2095,6 +2127,7 @@ fn tui_config_missing_notifications_field_defaults_to_enabled() { vim_mode_default: false, alternate_screen: AltScreenMode::Auto, status_line: None, + status_line_use_colors: true, terminal_title: None, theme: None, keymap: TuiKeymap::default(), @@ -6421,6 +6454,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { tool_suggest: ToolSuggestConfig::default(), tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_status_line_use_colors: true, tui_terminal_title: None, tui_theme: None, otel: OtelConfig::default(), @@ -6618,6 +6652,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { tool_suggest: ToolSuggestConfig::default(), tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_status_line_use_colors: true, tui_terminal_title: None, tui_theme: None, otel: OtelConfig::default(), @@ -6769,6 +6804,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { tool_suggest: ToolSuggestConfig::default(), tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_status_line_use_colors: true, tui_terminal_title: None, tui_theme: None, otel: OtelConfig::default(), @@ -6905,6 +6941,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { tool_suggest: ToolSuggestConfig::default(), tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_status_line_use_colors: true, tui_terminal_title: None, tui_theme: None, otel: OtelConfig::default(), diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 80b54aeaf0..8d4128900d 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -104,6 +104,14 @@ pub fn status_line_items_edit(items: &[String]) -> ConfigEdit { } } +/// Produces a config edit that sets `[tui].status_line_use_colors`. +pub fn status_line_use_colors_edit(enabled: bool) -> ConfigEdit { + ConfigEdit::SetPath { + segments: vec!["tui".to_string(), "status_line_use_colors".to_string()], + value: value(enabled), + } +} + /// Produces a config edit that sets `[tui].terminal_title` to an explicit ordered list. /// /// The array is written even when it is empty so "disabled title updates" stays diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 29f158058b..83b8d78b8b 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -524,6 +524,9 @@ pub struct Config { /// When unset, the TUI defaults to: `model-with-reasoning` and `current-dir`. pub tui_status_line: Option>, + /// Whether to color status line items with colors from the active syntax theme. + pub tui_status_line_use_colors: bool, + /// Ordered list of terminal title item identifiers for the TUI. /// /// When unset, the TUI defaults to: `activity` and `project`. @@ -3011,6 +3014,11 @@ impl Config { .map(|t| t.alternate_screen) .unwrap_or_default(), tui_status_line: cfg.tui.as_ref().and_then(|t| t.status_line.clone()), + tui_status_line_use_colors: cfg + .tui + .as_ref() + .map(|t| t.status_line_use_colors) + .unwrap_or(true), tui_terminal_title: cfg.tui.as_ref().and_then(|t| t.terminal_title.clone()), tui_theme: cfg.tui.as_ref().and_then(|t| t.theme.clone()), terminal_resize_reflow, diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 2f004717e7..757f79bfa9 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -191,6 +191,7 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R model_availability_nux: ModelAvailabilityNuxConfig::default(), tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_status_line_use_colors: true, tui_terminal_title: None, tui_theme: None, terminal_resize_reflow: TerminalResizeReflowConfig::default(), diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 01bae109d7..b2cd1e3e05 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1785,22 +1785,29 @@ impl App { tui.frame_requester().schedule_frame(); } } - AppEvent::StatusLineSetup { items } => { + AppEvent::StatusLineSetup { + items, + use_theme_colors, + } => { let ids = items.iter().map(ToString::to_string).collect::>(); - let edit = crate::legacy_core::config::edit::status_line_items_edit(&ids); + let items_edit = crate::legacy_core::config::edit::status_line_items_edit(&ids); + let colors_edit = + crate::legacy_core::config::edit::status_line_use_colors_edit(use_theme_colors); let apply_result = ConfigEditsBuilder::new(&self.config.codex_home) - .with_edits([edit]) + .with_edits([items_edit, colors_edit]) .apply() .await; match apply_result { Ok(()) => { self.config.tui_status_line = Some(ids.clone()); - self.chat_widget.setup_status_line(items); + self.config.tui_status_line_use_colors = use_theme_colors; + self.chat_widget.setup_status_line(items, use_theme_colors); } Err(err) => { - tracing::error!(error = %err, "failed to persist status line items; keeping previous selection"); - self.chat_widget - .add_error_message(format!("Failed to save status line items: {err}")); + tracing::error!(error = %err, "failed to persist status line settings; keeping previous selection"); + self.chat_widget.add_error_message(format!( + "Failed to save status line settings: {err}" + )); } } } @@ -1857,15 +1864,20 @@ impl App { crate::render::highlight::set_syntax_theme(theme); } self.sync_tui_theme_selection(name); + self.refresh_status_line(); } Err(err) => { self.restore_runtime_theme_from_config(); + self.refresh_status_line(); tracing::error!(error = %err, "failed to persist theme selection"); self.chat_widget .add_error_message(format!("Failed to save theme: {err}")); } } } + AppEvent::SyntaxThemePreviewed => { + self.refresh_status_line(); + } AppEvent::OpenKeymapActionMenu { context, action } => { self.chat_widget .open_keymap_action_menu(context, action, &self.keymap); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 47f57cd82c..84500e3edf 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1199,8 +1199,10 @@ async fn replayed_interrupted_turn_restores_queued_input_to_composer() { #[tokio::test] async fn token_usage_update_refreshes_status_line_with_runtime_context_window() { let mut app = make_test_app().await; - app.chat_widget - .setup_status_line(vec![crate::bottom_pane::StatusLineItem::ContextWindowSize]); + app.chat_widget.setup_status_line( + vec![crate::bottom_pane::StatusLineItem::ContextWindowSize], + /*use_theme_colors*/ true, + ); assert_eq!(app.chat_widget.status_line_text(), None); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index f2b2b19f69..5ae99e088b 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -808,6 +808,7 @@ pub(crate) enum AppEvent { /// Apply a user-confirmed status-line item ordering/selection. StatusLineSetup { items: Vec, + use_theme_colors: bool, }, /// Dismiss the status-line setup UI without changing config. StatusLineSetupCancelled, @@ -828,6 +829,9 @@ pub(crate) enum AppEvent { name: String, }, + /// Runtime syntax theme preview changed; refresh theme-derived UI colors. + SyntaxThemePreviewed, + /// Open set/remove actions for the selected keymap action. OpenKeymapActionMenu { context: String, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 3c255ada22..9e0ba8dc7e 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -4267,7 +4267,6 @@ impl ChatComposer { let status_line_active = uses_passive_footer_status_layout(&footer_props); let combined_status_line = if status_line_active { passive_footer_status_line(&footer_props) - .map(ratatui::prelude::Stylize::dim) } else { None }; diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index a28aa4a1ca..9c4036b564 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -684,7 +684,7 @@ fn footer_from_props_lines( // Passive footer context can come from the configurable status line, the // active agent label, or both combined. if let Some(status_line) = passive_footer_status_line(props) { - return vec![status_line.dim()]; + return vec![status_line]; } match props.mode { FooterMode::QuitShortcutReminder => { @@ -755,10 +755,10 @@ pub(crate) fn passive_footer_status_line(props: &FooterProps) -> Option max_left - && let Some(line) = passive_status_line - .as_ref() - .map(|line| line.clone().dim()) - .map(|line| { - truncate_line_with_ellipsis_if_overflow(line, max_left as usize) - }) + && let Some(line) = passive_status_line.as_ref().map(|line| { + truncate_line_with_ellipsis_if_overflow(line.clone(), max_left as usize) + }) { left_width = line.width() as u16; truncated_status_line = Some(line); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 1264bc987c..df97d8d653 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -53,6 +53,7 @@ mod mcp_server_elicitation; mod multi_select_picker; mod request_user_input; mod status_line_setup; +mod status_line_style; mod status_surface_preview; mod title_setup; pub(crate) use action_required_title::ACTION_REQUIRED_PREVIEW_PREFIX; @@ -67,6 +68,7 @@ pub(crate) use approval_overlay::format_requested_permissions_rule; pub(crate) use mcp_server_elicitation::McpServerElicitationFormRequest; pub(crate) use mcp_server_elicitation::McpServerElicitationOverlay; pub(crate) use request_user_input::RequestUserInputOverlay; +pub(crate) use status_line_style::status_line_from_segments; mod bottom_pane_view; #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs index 0082109b7b..6b046e7ea0 100644 --- a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs +++ b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs @@ -18,8 +18,22 @@ //! app_event_tx, //! ) //! .items(vec![ -//! MultiSelectItem { id: "a".into(), name: "Item A".into(), description: None, enabled: true }, -//! MultiSelectItem { id: "b".into(), name: "Item B".into(), description: None, enabled: false }, +//! MultiSelectItem { +//! id: "a".into(), +//! name: "Item A".into(), +//! description: None, +//! enabled: true, +//! orderable: true, +//! section_break_after: false, +//! }, +//! MultiSelectItem { +//! id: "b".into(), +//! name: "Item B".into(), +//! description: None, +//! enabled: false, +//! orderable: true, +//! section_break_after: false, +//! }, //! ]) //! .on_confirm(|selected_ids, tx| { /* handle confirmation */ }) //! .build(); @@ -64,6 +78,8 @@ const SEARCH_PLACEHOLDER: &str = "Type to search"; /// Prefix displayed before the search query (mimics a command prompt). const SEARCH_PROMPT_PREFIX: &str = "> "; +const SECTION_BREAK_ROW: &str = " ───────────────────────"; + /// Direction for reordering items in the list. enum Direction { Up, @@ -89,7 +105,6 @@ pub type PreviewCallback = Box Option Self { + Self { + id: String::new(), + name: String::new(), + description: None, + enabled: false, + orderable: true, + section_break_after: false, + } + } +} + +struct BuiltRows { + rows: Vec, + state: ScrollState, } /// A multi-select picker widget with fuzzy search and optional reordering. @@ -240,33 +279,61 @@ impl MultiSelectPicker { } /// Calculates the height needed for the row list area. - fn rows_height(&self, rows: &[GenericDisplayRow]) -> u16 { - rows.len().clamp(1, MAX_POPUP_ROWS).try_into().unwrap_or(1) + fn rows_height(&self, rows: &BuiltRows) -> u16 { + rows.rows + .len() + .clamp(1, MAX_POPUP_ROWS) + .try_into() + .unwrap_or(1) } /// Builds the display rows for all currently visible (filtered) items. /// /// Each row shows: `› [x] Item Name` where `›` indicates cursor position /// and `[x]` or `[ ]` indicates enabled/disabled state. - fn build_rows(&self) -> Vec { - self.filtered_indices - .iter() - .enumerate() - .filter_map(|(visible_idx, actual_idx)| { - self.items.get(*actual_idx).map(|item| { - let is_selected = self.state.selected_idx == Some(visible_idx); - let prefix = if is_selected { '›' } else { ' ' }; - let marker = if item.enabled { 'x' } else { ' ' }; - let item_name = truncate_text(&item.name, ITEM_NAME_TRUNCATE_LEN); - let name = format!("{prefix} [{marker}] {item_name}"); - GenericDisplayRow { - name, - description: item.description.clone(), - ..Default::default() - } - }) - }) - .collect() + fn build_rows(&self) -> BuiltRows { + let mut rows = Vec::new(); + let mut visible_to_row = Vec::with_capacity(self.filtered_indices.len()); + for (visible_idx, actual_idx) in self.filtered_indices.iter().enumerate() { + let Some(item) = self.items.get(*actual_idx) else { + continue; + }; + visible_to_row.push(rows.len()); + let is_selected = self.state.selected_idx == Some(visible_idx); + let prefix = if is_selected { '›' } else { ' ' }; + let marker = if item.enabled { 'x' } else { ' ' }; + let item_name = truncate_text(&item.name, ITEM_NAME_TRUNCATE_LEN); + let name = format!("{prefix} [{marker}] {item_name}"); + rows.push(GenericDisplayRow { + name, + description: item.description.clone(), + ..Default::default() + }); + + if item.section_break_after && visible_idx + 1 < self.filtered_indices.len() { + rows.push(GenericDisplayRow { + name: SECTION_BREAK_ROW.to_string(), + is_disabled: true, + ..Default::default() + }); + } + } + + let selected_idx = self + .state + .selected_idx + .and_then(|visible_idx| visible_to_row.get(visible_idx).copied()); + let scroll_top = visible_to_row + .get(self.state.scroll_top) + .copied() + .unwrap_or(0); + BuiltRows { + rows, + state: ScrollState { + selected_idx, + scroll_top, + }, + } } /// Moves the selection cursor up, wrapping to the bottom if at the top. @@ -351,12 +418,24 @@ impl MultiSelectPicker { return; } + if !self + .items + .get(actual_idx) + .is_some_and(|item| item.orderable) + { + return; + } + let new_idx = match direction { Direction::Up if actual_idx > 0 => actual_idx - 1, Direction::Down if actual_idx + 1 < len => actual_idx + 1, _ => return, }; + if !self.items.get(new_idx).is_some_and(|item| item.orderable) { + return; + } + // move item in underlying list self.items.swap(actual_idx, new_idx); @@ -570,8 +649,8 @@ impl Renderable for MultiSelectPicker { render_rows_single_line( render_area, buf, - &rows, - &self.state, + &rows.rows, + &rows.state, render_area.height as usize, "no matches", ); @@ -793,3 +872,96 @@ pub(crate) fn match_item( } None } + +#[cfg(test)] +mod tests { + use super::*; + use crate::app_event::AppEvent; + use pretty_assertions::assert_eq; + use tokio::sync::mpsc::unbounded_channel; + + fn test_picker(items: Vec) -> MultiSelectPicker { + let (tx, _rx) = unbounded_channel::(); + MultiSelectPicker::builder( + "Test".to_string(), + /*subtitle*/ None, + AppEventSender::new(tx), + ) + .items(items) + .enable_ordering() + .build() + } + + fn item(id: &str, orderable: bool, section_break_after: bool) -> MultiSelectItem { + MultiSelectItem { + id: id.to_string(), + name: id.to_string(), + orderable, + section_break_after, + ..Default::default() + } + } + + #[test] + fn non_orderable_items_cannot_move_or_be_crossed() { + let mut picker = test_picker(vec![ + item( + "theme-colors", + /*orderable*/ false, + /*section_break_after*/ true, + ), + item( + "model", /*orderable*/ true, /*section_break_after*/ false, + ), + item( + "branch", /*orderable*/ true, /*section_break_after*/ false, + ), + ]); + + picker.move_selected_item(Direction::Down); + assert_eq!( + picker + .items + .iter() + .map(|item| item.id.as_str()) + .collect::>(), + vec!["theme-colors", "model", "branch"] + ); + + picker.move_down(); + picker.move_selected_item(Direction::Up); + assert_eq!( + picker + .items + .iter() + .map(|item| item.id.as_str()) + .collect::>(), + vec!["theme-colors", "model", "branch"] + ); + } + + #[test] + fn section_break_after_item_renders_separator_row() { + let picker = test_picker(vec![ + item( + "theme-colors", + /*orderable*/ false, + /*section_break_after*/ true, + ), + item( + "model", /*orderable*/ true, /*section_break_after*/ false, + ), + ]); + + let rows = picker.build_rows(); + + assert_eq!( + rows.rows + .iter() + .map(|row| row.name.as_str()) + .collect::>(), + vec!["› [ ] theme-colors", SECTION_BREAK_ROW, " [ ] model"] + ); + assert_eq!(rows.state.selected_idx, Some(0)); + } +} diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap index ff93e83748..d29d964d81 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap @@ -8,14 +8,14 @@ expression: "render_lines(&view, 72)" Type to search > -› [x] model Current model name +› [x] Use theme colors Apply colors from the active /theme + ─────────────────────── + [x] model Current model name [x] current-dir Current working directory [x] git-branch Current Git branch (omitted when unavaila… [ ] model-with-reasoning Current model name with reasoning level [ ] project-name Project name (omitted when unavailable) [ ] run-state Compact session run-state text (Ready, Wo… - [ ] context-remaining Percentage of context window remaining (o… - [ ] context-used Percentage of context window used (omitte… gpt-5-codex · ~/codex-rs · jif/statusline-preview Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc diff --git a/codex-rs/tui/src/bottom_pane/status_line_setup.rs b/codex-rs/tui/src/bottom_pane/status_line_setup.rs index ea522f3bf0..5dd79f35e3 100644 --- a/codex-rs/tui/src/bottom_pane/status_line_setup.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_setup.rs @@ -35,6 +35,8 @@ use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewData; use crate::bottom_pane::status_surface_preview::StatusSurfacePreviewItem; use crate::render::renderable::Renderable; +const STATUS_LINE_USE_THEME_COLORS_ITEM_ID: &str = "status-line-use-theme-colors"; + /// Available items that can be displayed in the status line. /// /// Each variant represents a piece of information that can be shown at the @@ -45,7 +47,7 @@ use crate::render::renderable::Renderable; /// - Git-related items only show when in a git repository /// - Context/limit items only show when data is available from the API /// - Session ID only shows after a session has started -#[derive(EnumIter, EnumString, Display, Debug, Clone, Eq, PartialEq, Ord, PartialOrd)] +#[derive(EnumIter, EnumString, Display, Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd)] #[strum(serialize_all = "kebab_case")] pub(crate) enum StatusLineItem { /// The current model name. @@ -118,7 +120,7 @@ pub(crate) enum StatusLineItem { impl StatusLineItem { /// User-visible description shown in the popup. - pub(crate) fn description(&self) -> &'static str { + pub(crate) fn description(self) -> &'static str { match self { StatusLineItem::ModelName => "Current model name", StatusLineItem::ModelWithReasoning => "Current model name with reasoning level", @@ -200,17 +202,27 @@ impl StatusLineSetupView { /// /// * `status_line_items` - Currently configured item IDs (in display order), /// or `None` to start with all items disabled + /// * `use_theme_colors` - Whether the preview and saved status line use colors from + /// the active theme /// * `app_event_tx` - Event sender for dispatching configuration changes /// /// Items from `status_line_items` are shown first (in order) and marked as /// enabled. Remaining items are appended and marked as disabled. pub(crate) fn new( status_line_items: Option<&[String]>, + use_theme_colors: bool, preview_data: StatusSurfacePreviewData, app_event_tx: AppEventSender, ) -> Self { let mut used_ids = HashSet::new(); - let mut items = Vec::new(); + let mut items = vec![MultiSelectItem { + id: STATUS_LINE_USE_THEME_COLORS_ITEM_ID.to_string(), + name: "Use theme colors".to_string(), + description: Some("Apply colors from the active /theme".to_string()), + enabled: use_theme_colors, + orderable: false, + section_break_after: true, + }]; if let Some(selected_items) = status_line_items.as_ref() { for id in *selected_items { @@ -246,21 +258,31 @@ impl StatusLineSetupView { .items(items) .enable_ordering() .on_preview(move |items| { - preview_data.line_for_items( + let use_theme_colors = items + .iter() + .find(|item| item.id == STATUS_LINE_USE_THEME_COLORS_ITEM_ID) + .map(|item| item.enabled) + .unwrap_or(true); + preview_data.status_line_for_items( items .iter() .filter(|item| item.enabled) - .filter_map(|item| item.id.parse::().ok()) - .map(StatusLineItem::preview_item), + .filter_map(|item| item.id.parse::().ok()), + use_theme_colors, ) }) .on_confirm(|ids, app_event| { + let use_theme_colors = ids + .iter() + .any(|id| id == STATUS_LINE_USE_THEME_COLORS_ITEM_ID); let items = ids .iter() - .map(|id| id.parse::()) - .collect::, _>>() - .unwrap_or_default(); - app_event.send(AppEvent::StatusLineSetup { items }); + .filter_map(|id| id.parse::().ok()) + .collect::>(); + app_event.send(AppEvent::StatusLineSetup { + items, + use_theme_colors, + }); }) .on_cancel(|app_event| { app_event.send(AppEvent::StatusLineSetupCancelled); @@ -276,6 +298,8 @@ impl StatusLineSetupView { name: item.to_string(), description: Some(item.description().to_string()), enabled, + orderable: true, + section_break_after: false, } } } @@ -415,23 +439,29 @@ mod tests { name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, MultiSelectItem { id: StatusLineItem::CurrentDir.to_string(), name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, ]; assert_eq!( - preview_data.line_for_items( - items - .iter() - .filter_map(|item| item.id.parse::().ok()) - .map(StatusLineItem::preview_item), + line_text( + preview_data.status_line_for_items( + items + .iter() + .filter_map(|item| item.id.parse::().ok()), + /*use_theme_colors*/ true, + ) ), - Some(Line::from("gpt-5 · /repo")) + Some("gpt-5 · /repo".to_string()) ); } @@ -447,23 +477,29 @@ mod tests { name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, MultiSelectItem { id: StatusLineItem::GitBranch.to_string(), name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, ]; assert_eq!( - preview_data.line_for_items( - items - .iter() - .filter_map(|item| item.id.parse::().ok()) - .map(StatusLineItem::preview_item), + line_text( + preview_data.status_line_for_items( + items + .iter() + .filter_map(|item| item.id.parse::().ok()), + /*use_theme_colors*/ true, + ) ), - Some(Line::from("gpt-5 · feat/awesome-feature")) + Some("gpt-5 · feat/awesome-feature".to_string()) ); } @@ -485,23 +521,29 @@ mod tests { name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, MultiSelectItem { id: StatusLineItem::ThreadTitle.to_string(), name: String::new(), description: None, enabled: true, + orderable: true, + section_break_after: false, }, ]; assert_eq!( - preview_data.line_for_items( - items - .iter() - .filter_map(|item| item.id.parse::().ok()) - .map(StatusLineItem::preview_item), + line_text( + preview_data.status_line_for_items( + items + .iter() + .filter_map(|item| item.id.parse::().ok()), + /*use_theme_colors*/ true, + ) ), - Some(Line::from("gpt-5 · Roadmap cleanup")) + Some("gpt-5 · Roadmap cleanup".to_string()) ); } @@ -514,6 +556,7 @@ mod tests { StatusLineItem::CurrentDir.to_string(), StatusLineItem::GitBranch.to_string(), ]), + /*use_theme_colors*/ true, StatusSurfacePreviewData::from_iter([ ( StatusLineItem::ModelName.preview_item(), @@ -560,4 +603,13 @@ mod tests { .collect::>() .join("\n") } + + fn line_text(line: Option>) -> Option { + line.map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + } } diff --git a/codex-rs/tui/src/bottom_pane/status_line_style.rs b/codex-rs/tui/src/bottom_pane/status_line_style.rs new file mode 100644 index 0000000000..1449256a64 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/status_line_style.rs @@ -0,0 +1,270 @@ +//! Theme-derived styling for the configurable footer statusline. + +use ratatui::prelude::Stylize; +use ratatui::style::Color; +use ratatui::style::Style; +use ratatui::text::Line; +use ratatui::text::Span; + +use super::status_line_setup::StatusLineItem; +use crate::render::highlight::foreground_style_for_scopes; + +const STATUS_LINE_SEPARATOR: &str = " · "; +const STATUS_LINE_COLOR_SATURATION_PERCENT: u16 = 85; +const STATUS_LINE_COLOR_BRIGHTNESS_PERCENT: u16 = 100; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum StatusLineAccent { + Model, + Path, + Branch, + State, + Usage, + Limit, + Metadata, + Mode, + Thread, + Progress, +} + +impl StatusLineAccent { + fn for_item(item: StatusLineItem) -> Self { + match item { + StatusLineItem::ModelName | StatusLineItem::ModelWithReasoning => Self::Model, + StatusLineItem::CurrentDir | StatusLineItem::ProjectRoot => Self::Path, + StatusLineItem::GitBranch => Self::Branch, + StatusLineItem::Status => Self::State, + StatusLineItem::ContextRemaining + | StatusLineItem::ContextUsed + | StatusLineItem::ContextWindowSize + | StatusLineItem::UsedTokens + | StatusLineItem::TotalInputTokens + | StatusLineItem::TotalOutputTokens => Self::Usage, + StatusLineItem::FiveHourLimit | StatusLineItem::WeeklyLimit => Self::Limit, + StatusLineItem::CodexVersion | StatusLineItem::SessionId => Self::Metadata, + StatusLineItem::FastMode => Self::Mode, + StatusLineItem::ThreadTitle => Self::Thread, + StatusLineItem::TaskProgress => Self::Progress, + } + } + + fn scopes(self) -> &'static [&'static str] { + match self { + Self::Model => &["entity.name.type", "support.type", "variable"], + Self::Path => &["string", "markup.underline.link"], + Self::Branch => &["entity.name.function", "entity.name.tag"], + Self::State => &["keyword.control", "keyword"], + Self::Usage => &["constant.numeric", "constant"], + Self::Limit => &["constant.language", "storage.type"], + Self::Metadata => &["comment", "constant.other"], + Self::Mode => &["storage.modifier", "keyword.operator"], + Self::Thread => &["markup.heading", "entity.name.section"], + Self::Progress => &["markup.inserted", "constant.numeric"], + } + } + + fn fallback_style(self) -> Style { + match self { + Self::Model | Self::State | Self::Metadata | Self::Mode => Style::default().cyan(), + Self::Path | Self::Usage | Self::Progress => Style::default().green(), + Self::Branch | Self::Limit | Self::Thread => Style::default().magenta(), + } + } +} + +pub(crate) fn status_line_from_segments( + segments: I, + use_theme_colors: bool, +) -> Option> +where + I: IntoIterator, +{ + status_line_from_segments_with_resolver(segments, use_theme_colors, |accent| { + foreground_style_for_scopes(accent.scopes()) + }) +} + +fn status_line_from_segments_with_resolver( + segments: I, + use_theme_colors: bool, + theme_style_for_accent: F, +) -> Option> +where + I: IntoIterator, + F: Fn(StatusLineAccent) -> Option