From f4ae2d0e54fc9f4154610b19ba541c1ab1f24f66 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 5 May 2026 17:30:37 -0700 Subject: [PATCH 1/2] Move message history out of core --- codex-rs/Cargo.lock | 14 ++ codex-rs/Cargo.toml | 1 + codex-rs/app-server-client/src/lib.rs | 3 - codex-rs/core/src/lib.rs | 5 - codex-rs/core/src/session/handlers.rs | 55 -------- codex-rs/core/src/session/session.rs | 22 +--- codex-rs/core/src/session/turn.rs | 1 - codex-rs/exec/src/lib.rs | 2 - .../tests/event_processor_with_json_output.rs | 2 - codex-rs/mcp-server/src/codex_tool_runner.rs | 1 - codex-rs/mcp-server/src/outgoing_message.rs | 10 -- codex-rs/message-history/BUILD.bazel | 6 + codex-rs/message-history/Cargo.toml | 24 ++++ .../src/lib.rs} | 55 +++++--- .../src/tests.rs} | 37 ++---- codex-rs/protocol/src/lib.rs | 1 - codex-rs/protocol/src/message_history.rs | 11 -- codex-rs/protocol/src/protocol.rs | 43 ------ codex-rs/rollout-trace/src/protocol_event.rs | 2 - codex-rs/rollout/src/policy.rs | 1 - codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/app.rs | 2 - codex-rs/tui/src/app/config_persistence.rs | 3 +- codex-rs/tui/src/app/event_dispatch.rs | 6 + codex-rs/tui/src/app/tests.rs | 33 ++--- codex-rs/tui/src/app/thread_events.rs | 3 +- codex-rs/tui/src/app/thread_routing.rs | 123 ++++++++---------- codex-rs/tui/src/app/thread_session_state.rs | 9 +- codex-rs/tui/src/app_command.rs | 15 --- codex-rs/tui/src/app_event.rs | 15 ++- codex-rs/tui/src/app_server_session.rs | 29 +++-- codex-rs/tui/src/bottom_pane/chat_composer.rs | 4 +- .../src/bottom_pane/chat_composer_history.rs | 96 ++++++-------- codex-rs/tui/src/chatwidget.rs | 20 ++- .../chatwidget/tests/composer_submission.rs | 27 ++-- .../tui/src/chatwidget/tests/exec_flow.rs | 7 +- .../src/chatwidget/tests/history_replay.rs | 27 ++-- .../tui/src/chatwidget/tests/permissions.rs | 6 +- .../tui/src/chatwidget/tests/plan_mode.rs | 6 +- .../src/chatwidget/tests/slash_commands.rs | 49 +++---- .../src/chatwidget/tests/status_and_layout.rs | 3 +- codex-rs/tui/src/history_cell.rs | 3 +- codex-rs/tui/src/session_state.rs | 9 +- codex-rs/tui/tests/fixtures/oss-story.jsonl | 2 +- 44 files changed, 312 insertions(+), 482 deletions(-) create mode 100644 codex-rs/message-history/BUILD.bazel create mode 100644 codex-rs/message-history/Cargo.toml rename codex-rs/{core/src/message_history.rs => message-history/src/lib.rs} (90%) rename codex-rs/{core/src/message_history_tests.rs => message-history/src/tests.rs} (88%) delete mode 100644 codex-rs/protocol/src/message_history.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cd3e93c9cb2c..8f28a07539c7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3110,6 +3110,19 @@ dependencies = [ "wiremock", ] +[[package]] +name = "codex-message-history" +version = "0.0.0" +dependencies = [ + "codex-config", + "pretty_assertions", + "serde", + "serde_json", + "tempfile", + "tokio", + "tracing", +] + [[package]] name = "codex-model-provider" version = "0.0.0" @@ -3652,6 +3665,7 @@ dependencies = [ "codex-install-context", "codex-login", "codex-mcp", + "codex-message-history", "codex-model-provider", "codex-model-provider-info", "codex-models-manager", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index ce3e91626dd1..5dcd8c2a2e90 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -169,6 +169,7 @@ codex-keyring-store = { path = "keyring-store" } codex-linux-sandbox = { path = "linux-sandbox" } codex-lmstudio = { path = "lmstudio" } codex-login = { path = "login" } +codex-message-history = { path = "message-history" } codex-memories-mcp = { path = "memories/mcp" } codex-memories-read = { path = "memories/read" } codex-memories-write = { path = "memories/write" } diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 0911cc448dd3..7f793cbd0c7e 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -72,12 +72,9 @@ pub mod legacy_core { pub use codex_core::DEFAULT_AGENTS_MD_FILENAME; pub use codex_core::LOCAL_AGENTS_MD_FILENAME; pub use codex_core::McpManager; - pub use codex_core::append_message_history_entry; pub use codex_core::check_execpolicy_for_warnings; pub use codex_core::format_exec_policy_error_with_source; pub use codex_core::grant_read_root_non_elevated; - pub use codex_core::lookup_message_history_entry; - pub use codex_core::message_history_metadata; pub use codex_core::web_search_detail; pub mod config { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index cbada9a26ced..21891e7434f9 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -61,14 +61,9 @@ pub use codex_mcp::SandboxState; mod mcp_openai_file; mod mcp_tool_call; pub(crate) mod mention_syntax; -pub(crate) mod message_history; pub(crate) mod utils; pub use mention_syntax::PLUGIN_TEXT_MENTION_SIGIL; pub use mention_syntax::TOOL_MENTION_SIGIL; -pub use message_history::HistoryEntry as MessageHistoryEntry; -pub use message_history::append_entry as append_message_history_entry; -pub use message_history::history_metadata as message_history_metadata; -pub use message_history::lookup as lookup_message_history_entry; pub use utils::path_utils; pub mod personality_migration; pub(crate) mod plugins; diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 612eaf5d6593..5326111909c9 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -471,53 +471,6 @@ pub async fn dynamic_tool_response(sess: &Arc, id: String, response: Dy sess.notify_dynamic_tool_response(&id, response).await; } -pub async fn add_to_history(sess: &Arc, config: &Arc, text: String) { - let id = sess.conversation_id; - let config = Arc::clone(config); - tokio::spawn(async move { - if let Err(e) = crate::message_history::append_entry(&text, &id, &config).await { - warn!("failed to append to message history: {e}"); - } - }); -} - -pub async fn get_history_entry_request( - sess: &Arc, - config: &Arc, - sub_id: String, - offset: usize, - log_id: u64, -) { - let config = Arc::clone(config); - let sess_clone = Arc::clone(sess); - - tokio::spawn(async move { - // Run lookup in blocking thread because it does file IO + locking. - let entry_opt = tokio::task::spawn_blocking(move || { - crate::message_history::lookup(log_id, offset, &config) - }) - .await - .unwrap_or(None); - - let event = Event { - id: sub_id, - msg: EventMsg::GetHistoryEntryResponse( - codex_protocol::protocol::GetHistoryEntryResponseEvent { - offset, - log_id, - entry: entry_opt.map(|e| codex_protocol::message_history::HistoryEntry { - conversation_id: e.session_id, - ts: e.ts, - text: e.text, - }), - }, - ), - }; - - sess_clone.send_event_raw(event).await; - }); -} - pub async fn refresh_mcp_servers(sess: &Arc, refresh_config: McpServerRefreshConfig) { let mut guard = sess.pending_mcp_server_refresh_config.lock().await; *guard = Some(refresh_config); @@ -1087,14 +1040,6 @@ pub(super) async fn submission_loop( dynamic_tool_response(&sess, id, response).await; false } - Op::AddToHistory { text } => { - add_to_history(&sess, &config, text).await; - false - } - Op::GetHistoryEntryRequest { offset, log_id } => { - get_history_entry_request(&sess, &config, sub.id.clone(), offset, log_id).await; - false - } Op::ListMcpTools => { list_mcp_tools(&sess, &config, sub.id.clone()).await; false diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 23621f87cfdf..8864115eb285 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -444,19 +444,6 @@ impl Session { )); let state_db_ctx = if config.ephemeral { None } else { state_db }; - let is_subagent = session_configuration.session_source.is_non_root_agent(); - let history_meta_fut = async { - if is_subagent { - (0, 0) - } else { - crate::message_history::history_metadata(&config).await - } - } - .instrument(info_span!( - "session_init.history_metadata", - otel.name = "session_init.history_metadata", - session_init.is_subagent = is_subagent, - )); let auth_manager_clone = Arc::clone(&auth_manager); let config_for_mcp = Arc::clone(&config); let mcp_manager_for_mcp = Arc::clone(&mcp_manager); @@ -479,11 +466,8 @@ impl Session { )); // Join all independent futures. - let ( - thread_persistence_result, - (history_log_id, history_entry_count), - (auth, mcp_servers, auth_statuses), - ) = tokio::join!(thread_persistence_fut, history_meta_fut, auth_and_mcp_fut); + let (thread_persistence_result, (auth, mcp_servers, auth_statuses)) = + tokio::join!(thread_persistence_fut, auth_and_mcp_fut); let mut live_thread_init = LiveThreadInitGuard::new(thread_persistence_result.map_err(|e| { @@ -885,8 +869,6 @@ impl Session { active_permission_profile: session_configuration.active_permission_profile(), cwd: session_configuration.cwd.clone(), reasoning_effort: session_configuration.collaboration_mode.reasoning_effort(), - history_log_id, - history_entry_count, initial_messages, network_proxy: session_network_proxy.filter(|_| { Self::managed_network_proxy_active_for_permission_profile( diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 8ebbb0a8ac3a..8044d3347649 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1505,7 +1505,6 @@ pub(super) fn realtime_text_for_event(msg: &EventMsg) -> Option { | EventMsg::DeprecationNotice(_) | EventMsg::StreamError(_) | EventMsg::TurnDiff(_) - | EventMsg::GetHistoryEntryResponse(_) | EventMsg::McpListToolsResponse(_) | EventMsg::ListSkillsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index d61346f1d09e..18496a12b6ae 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -1144,8 +1144,6 @@ fn session_configured_from_thread_response( active_permission_profile, cwd, reasoning_effort, - history_log_id: 0, - history_entry_count: 0, initial_messages: None, network_proxy: None, rollout_path, diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 4b01ccccd124..ddace00ef06c 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -118,8 +118,6 @@ fn session_configured_produces_thread_started_event() { active_permission_profile: None, cwd: test_path_buf("/tmp/project").abs(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, initial_messages: None, network_proxy: None, rollout_path: None, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 4070cc7e1194..8f285c8c7574 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -351,7 +351,6 @@ async fn run_codex_tool_session_inner( | EventMsg::TurnDiff(_) | EventMsg::WebSearchBegin(_) | EventMsg::WebSearchEnd(_) - | EventMsg::GetHistoryEntryResponse(_) | EventMsg::PlanUpdate(_) | EventMsg::TurnAborted(_) | EventMsg::UserMessage(_) diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index eb66ea061996..c414857644af 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -308,8 +308,6 @@ mod tests { active_permission_profile: None, cwd: test_path_buf("/home/user/project").abs(), reasoning_effort: Some(ReasoningEffort::default()), - history_log_id: 1, - history_entry_count: 1000, initial_messages: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), @@ -353,8 +351,6 @@ mod tests { active_permission_profile: None, cwd: test_path_buf("/home/user/project").abs(), reasoning_effort: Some(ReasoningEffort::default()), - history_log_id: 1, - history_entry_count: 1000, initial_messages: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), @@ -392,8 +388,6 @@ mod tests { "permission_profile": session_configured_event.permission_profile, "cwd": test_path_buf("/home/user/project"), "reasoning_effort": session_configured_event.reasoning_effort, - "history_log_id": session_configured_event.history_log_id, - "history_entry_count": session_configured_event.history_entry_count, "rollout_path": rollout_file.path().to_path_buf(), } }); @@ -421,8 +415,6 @@ mod tests { active_permission_profile: None, cwd: test_path_buf("/home/user/project").abs(), reasoning_effort: Some(ReasoningEffort::default()), - history_log_id: 1, - history_entry_count: 1000, initial_messages: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), @@ -461,8 +453,6 @@ mod tests { "permission_profile": session_configured_event.permission_profile, "cwd": test_path_buf("/home/user/project"), "reasoning_effort": session_configured_event.reasoning_effort, - "history_log_id": session_configured_event.history_log_id, - "history_entry_count": session_configured_event.history_entry_count, "rollout_path": rollout_file.path().to_path_buf(), } }); diff --git a/codex-rs/message-history/BUILD.bazel b/codex-rs/message-history/BUILD.bazel new file mode 100644 index 000000000000..70df76cf77f9 --- /dev/null +++ b/codex-rs/message-history/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "message-history", + crate_name = "codex_message_history", +) diff --git a/codex-rs/message-history/Cargo.toml b/codex-rs/message-history/Cargo.toml new file mode 100644 index 000000000000..8b03e9a2010c --- /dev/null +++ b/codex-rs/message-history/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "codex-message-history" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_message_history" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-config = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["fs", "rt"] } +tracing = { workspace = true, features = ["log"] } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/codex-rs/core/src/message_history.rs b/codex-rs/message-history/src/lib.rs similarity index 90% rename from codex-rs/core/src/message_history.rs rename to codex-rs/message-history/src/lib.rs index 3458ec73068b..0d85cb8fe3fd 100644 --- a/codex-rs/core/src/message_history.rs +++ b/codex-rs/message-history/src/lib.rs @@ -5,16 +5,14 @@ //! JSON-Lines tooling. Each record has the following schema: //! //! ````text -//! {"conversation_id":"","ts":,"text":""} +//! {"session_id":"","ts":,"text":""} //! ```` //! -//! To minimise the chance of interleaved writes when multiple processes are +//! To minimize the chance of interleaved writes when multiple processes are //! appending concurrently, callers should *prepare the full line* (record + //! trailing `\n`) and write it with a **single `write(2)` system call** while //! the file descriptor is opened with the `O_APPEND` flag. POSIX guarantees //! that writes up to `PIPE_BUF` bytes are atomic in that case. -//! Note: `conversation_id` stores the thread id; the field name is preserved for -//! backwards compatibility with existing history files. use std::fs::File; use std::fs::OpenOptions; @@ -26,6 +24,7 @@ use std::io::Seek; use std::io::SeekFrom; use std::io::Write; use std::path::Path; +use std::path::PathBuf; use serde::Deserialize; use serde::Serialize; @@ -34,11 +33,9 @@ use std::time::Duration; use tokio::fs; use tokio::io::AsyncReadExt; -use crate::config::Config; +use codex_config::types::History; use codex_config::types::HistoryPersistence; -use codex_utils_absolute_path::AbsolutePathBuf; -use codex_protocol::ThreadId; #[cfg(unix)] use std::os::unix::fs::OpenOptionsExt; #[cfg(unix)] @@ -60,7 +57,24 @@ pub struct HistoryEntry { pub text: String, } -fn history_filepath(config: &Config) -> AbsolutePathBuf { +#[derive(Debug, Clone, PartialEq)] +pub struct HistoryConfig { + pub codex_home: PathBuf, + pub persistence: HistoryPersistence, + pub max_bytes: Option, +} + +impl HistoryConfig { + pub fn new(codex_home: impl Into, history: &History) -> Self { + Self { + codex_home: codex_home.into(), + persistence: history.persistence, + max_bytes: history.max_bytes, + } + } +} + +fn history_filepath(config: &HistoryConfig) -> PathBuf { config.codex_home.join(HISTORY_FILENAME) } @@ -79,8 +93,12 @@ fn history_filepath(config: &Config) -> AbsolutePathBuf { /// Returns an I/O error if the history file cannot be opened/created, the /// system clock is before the Unix epoch, or the exclusive lock cannot be /// acquired after [`MAX_RETRIES`] attempts. -pub async fn append_entry(text: &str, conversation_id: &ThreadId, config: &Config) -> Result<()> { - match config.history.persistence { +pub async fn append_entry( + text: &str, + conversation_id: impl std::fmt::Display, + config: &HistoryConfig, +) -> Result<()> { + match config.persistence { HistoryPersistence::SaveAll => { // Save everything: proceed. } @@ -128,7 +146,7 @@ pub async fn append_entry(text: &str, conversation_id: &ThreadId, config: &Confi // Ensure permissions. ensure_owner_only_permissions(&history_file).await?; - let history_max_bytes = config.history.max_bytes; + let history_max_bytes = config.max_bytes; // Perform a blocking write under an advisory write lock using std::fs. tokio::task::spawn_blocking(move || -> Result<()> { @@ -256,7 +274,7 @@ fn trim_target_bytes(max_bytes: u64, newest_entry_len: u64) -> u64 { /// `(0, 0)` when the file does not exist or its metadata cannot be read. If /// metadata succeeds but the file cannot be opened or scanned, returns /// `(log_id, 0)` so callers can still detect that a history file exists. -pub async fn history_metadata(config: &Config) -> (u64, usize) { +pub async fn history_metadata(config: &HistoryConfig) -> (u64, usize) { let path = history_filepath(config); history_metadata_for_file(&path).await } @@ -271,7 +289,7 @@ pub async fn history_metadata(config: &Config) -> (u64, usize) { /// This function is synchronous because it acquires a shared advisory file lock /// via `File::try_lock_shared`. Callers on an async runtime should wrap it in /// `spawn_blocking`. -pub fn lookup(log_id: u64, offset: usize, config: &Config) -> Option { +pub fn lookup(log_id: u64, offset: usize, config: &HistoryConfig) -> Option { let path = history_filepath(config); lookup_history_entry(&path, log_id, offset) } @@ -300,7 +318,7 @@ async fn ensure_owner_only_permissions(_file: &File) -> Result<()> { async fn history_metadata_for_file(path: &Path) -> (u64, usize) { let log_id = match fs::metadata(path).await { - Ok(metadata) => history_log_id(&metadata).unwrap_or(0), + Ok(metadata) => log_identity(&metadata).unwrap_or(0), Err(e) if e.kind() == std::io::ErrorKind::NotFound => return (0, 0), Err(_) => return (0, 0), }; @@ -347,7 +365,7 @@ fn lookup_history_entry(path: &Path, log_id: u64, offset: usize) -> Option Option Option { +fn log_identity(metadata: &std::fs::Metadata) -> Option { use std::os::unix::fs::MetadataExt; Some(metadata.ino()) } #[cfg(windows)] -fn history_log_id(metadata: &std::fs::Metadata) -> Option { +fn log_identity(metadata: &std::fs::Metadata) -> Option { use std::os::windows::fs::MetadataExt; Some(metadata.creation_time()) } #[cfg(not(any(unix, windows)))] -fn history_log_id(_metadata: &std::fs::Metadata) -> Option { +fn log_identity(_metadata: &std::fs::Metadata) -> Option { None } #[cfg(test)] -#[path = "message_history_tests.rs"] mod tests; diff --git a/codex-rs/core/src/message_history_tests.rs b/codex-rs/message-history/src/tests.rs similarity index 88% rename from codex-rs/core/src/message_history_tests.rs rename to codex-rs/message-history/src/tests.rs index de89a3eb9c95..88f0b7e00734 100644 --- a/codex-rs/core/src/message_history_tests.rs +++ b/codex-rs/message-history/src/tests.rs @@ -1,6 +1,5 @@ use super::*; -use crate::config::ConfigBuilder; -use codex_protocol::ThreadId; +use codex_config::types::History; use pretty_assertions::assert_eq; use std::fs::File; use std::io::Write; @@ -88,14 +87,9 @@ async fn lookup_uses_stable_log_id_after_appends() { #[tokio::test] async fn append_entry_trims_history_when_beyond_max_bytes() { let codex_home = TempDir::new().expect("create temp dir"); - - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .build() - .await - .expect("load config"); - - let conversation_id = ThreadId::new(); + let mut history = History::default(); + let mut config = HistoryConfig::new(codex_home.path(), &history); + let conversation_id = "conversation-id"; let entry_one = "a".repeat(200); let entry_two = "b".repeat(200); @@ -109,8 +103,8 @@ async fn append_entry_trims_history_when_beyond_max_bytes() { let first_len = std::fs::metadata(&history_path).expect("metadata").len(); let limit_bytes = first_len + 10; - config.history.max_bytes = - Some(usize::try_from(limit_bytes).expect("limit should fit into usize")); + history.max_bytes = Some(usize::try_from(limit_bytes).expect("limit should fit into usize")); + config = HistoryConfig::new(codex_home.path(), &history); append_entry(&entry_two, &conversation_id, &config) .await @@ -135,14 +129,9 @@ async fn append_entry_trims_history_when_beyond_max_bytes() { #[tokio::test] async fn append_entry_trims_history_to_soft_cap() { let codex_home = TempDir::new().expect("create temp dir"); - - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .build() - .await - .expect("load config"); - - let conversation_id = ThreadId::new(); + let mut history = History::default(); + let mut config = HistoryConfig::new(codex_home.path(), &history); + let conversation_id = "conversation-id"; let short_entry = "a".repeat(200); let long_entry = "b".repeat(400); @@ -165,10 +154,11 @@ async fn append_entry_trims_history_to_soft_cap() { .checked_sub(short_entry_len) .expect("second entry length should be larger than first entry length"); - config.history.max_bytes = Some( + history.max_bytes = Some( usize::try_from((2 * long_entry_len) + (short_entry_len / 2)) .expect("max bytes should fit into usize"), ); + config = HistoryConfig::new(codex_home.path(), &history); append_entry(&long_entry, &conversation_id, &config) .await @@ -185,10 +175,7 @@ async fn append_entry_trims_history_to_soft_cap() { assert_eq!(entries[0].text, long_entry); let pruned_len = std::fs::metadata(&history_path).expect("metadata").len(); - let max_bytes = config - .history - .max_bytes - .expect("max bytes should be configured") as u64; + let max_bytes = config.max_bytes.expect("max bytes should be configured") as u64; assert!(pruned_len <= max_bytes); diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index 175c92331f25..b5a6c3779a75 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -14,7 +14,6 @@ pub mod exec_output; pub mod items; pub mod mcp; pub mod memory_citation; -pub mod message_history; pub mod models; pub mod network_policy; pub mod num_format; diff --git a/codex-rs/protocol/src/message_history.rs b/codex-rs/protocol/src/message_history.rs deleted file mode 100644 index 0d8bd8df4e72..000000000000 --- a/codex-rs/protocol/src/message_history.rs +++ /dev/null @@ -1,11 +0,0 @@ -use schemars::JsonSchema; -use serde::Deserialize; -use serde::Serialize; -use ts_rs::TS; - -#[derive(Serialize, Deserialize, Debug, Clone, JsonSchema, TS)] -pub struct HistoryEntry { - pub conversation_id: String, - pub ts: u64, - pub text: String, -} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 3675806e6085..4b32a18fa5b4 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -34,7 +34,6 @@ use crate::mcp::Resource as McpResource; use crate::mcp::ResourceTemplate as McpResourceTemplate; use crate::mcp::Tool as McpTool; use crate::memory_citation::MemoryCitation; -use crate::message_history::HistoryEntry; use crate::models::ActivePermissionProfile; use crate::models::BaseInstructions; use crate::models::ContentItem; @@ -723,18 +722,6 @@ pub enum Op { response: DynamicToolResponse, }, - /// Append an entry to the persistent cross-session message history. - /// - /// Note the entry is not guaranteed to be logged if the user has - /// history disabled, it matches the list of "sensitive" patterns, etc. - AddToHistory { - /// The message text to be stored. - text: String, - }, - - /// Request a single history entry identified by `log_id` + `offset`. - GetHistoryEntryRequest { offset: usize, log_id: u64 }, - /// Request the list of MCP tools available across all configured servers. /// Reply is delivered via `EventMsg::McpListToolsResponse`. ListMcpTools, @@ -896,8 +883,6 @@ impl Op { Self::UserInputAnswer { .. } => "user_input_answer", Self::RequestPermissionsResponse { .. } => "request_permissions_response", Self::DynamicToolResponse { .. } => "dynamic_tool_response", - Self::AddToHistory { .. } => "add_to_history", - Self::GetHistoryEntryRequest { .. } => "get_history_entry_request", Self::ListMcpTools => "list_mcp_tools", Self::RefreshMcpServers { .. } => "refresh_mcp_servers", Self::ReloadUserConfig => "reload_user_config", @@ -1448,9 +1433,6 @@ pub enum EventMsg { TurnDiff(TurnDiffEvent), - /// Response to GetHistoryEntryRequest. - GetHistoryEntryResponse(GetHistoryEntryResponseEvent), - /// List of MCP tools available to the agent. McpListToolsResponse(McpListToolsResponseEvent), @@ -3251,15 +3233,6 @@ pub struct TurnDiffEvent { pub unified_diff: String, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] -pub struct GetHistoryEntryResponseEvent { - pub offset: usize, - pub log_id: u64, - /// The entry at the requested offset, if available and parseable. - #[serde(skip_serializing_if = "Option::is_none")] - pub entry: Option, -} - #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct McpListToolsResponseEvent { /// Fully qualified tool name -> tool definition. @@ -3504,12 +3477,6 @@ pub struct SessionConfiguredEvent { #[serde(skip_serializing_if = "Option::is_none")] pub reasoning_effort: Option, - /// Identifier of the history log file (inode on Unix, 0 otherwise). - pub history_log_id: u64, - - /// Current number of entries in the history log. - pub history_entry_count: usize, - /// Optional initial messages (as events) for resumed sessions. /// When present, UIs can use these to seed the history. #[serde(skip_serializing_if = "Option::is_none")] @@ -3551,8 +3518,6 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent { active_permission_profile: Option, cwd: AbsolutePathBuf, reasoning_effort: Option, - history_log_id: u64, - history_entry_count: usize, initial_messages: Option>, network_proxy: Option, rollout_path: Option, @@ -3583,8 +3548,6 @@ impl<'de> Deserialize<'de> for SessionConfiguredEvent { active_permission_profile: wire.active_permission_profile, cwd: wire.cwd, reasoning_effort: wire.reasoning_effort, - history_log_id: wire.history_log_id, - history_entry_count: wire.history_entry_count, initial_messages: wire.initial_messages, network_proxy: wire.network_proxy, rollout_path: wire.rollout_path, @@ -5341,8 +5304,6 @@ mod tests { active_permission_profile: None, cwd: test_path_buf("/home/user/project").abs(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, initial_messages: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), @@ -5361,8 +5322,6 @@ mod tests { "permission_profile": permission_profile, "cwd": test_path_buf("/home/user/project"), "reasoning_effort": "medium", - "history_log_id": 0, - "history_entry_count": 0, "rollout_path": format!("{}", rollout_file.path().display()), } }); @@ -5383,8 +5342,6 @@ mod tests { "type": "read-only" }, "cwd": cwd, - "history_log_id": 0, - "history_entry_count": 0, }); let event: SessionConfiguredEvent = serde_json::from_value(value)?; diff --git a/codex-rs/rollout-trace/src/protocol_event.rs b/codex-rs/rollout-trace/src/protocol_event.rs index 542073342ea5..6edc5d623fa0 100644 --- a/codex-rs/rollout-trace/src/protocol_event.rs +++ b/codex-rs/rollout-trace/src/protocol_event.rs @@ -260,7 +260,6 @@ pub(crate) fn tool_runtime_trace_event(event: &EventMsg) -> Option Option<&'static s | EventMsg::PatchApplyUpdated(_) | EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) - | EventMsg::GetHistoryEntryResponse(_) | EventMsg::McpListToolsResponse(_) | EventMsg::ListSkillsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) diff --git a/codex-rs/rollout/src/policy.rs b/codex-rs/rollout/src/policy.rs index aeb92ade1180..f6b2ff4f6834 100644 --- a/codex-rs/rollout/src/policy.rs +++ b/codex-rs/rollout/src/policy.rs @@ -155,7 +155,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option { | EventMsg::PatchApplyBegin(_) | EventMsg::PatchApplyUpdated(_) | EventMsg::TurnDiff(_) - | EventMsg::GetHistoryEntryResponse(_) | EventMsg::McpListToolsResponse(_) | EventMsg::RealtimeConversationListVoicesResponse(_) | EventMsg::McpStartupUpdate(_) diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index c5538c02ed89..5ef87bd9b929 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -42,6 +42,7 @@ codex-feedback = { workspace = true } codex-file-search = { workspace = true } codex-git-utils = { workspace = true } codex-login = { workspace = true } +codex-message-history = { workspace = true } codex-model-provider = { workspace = true } codex-model-provider-info = { workspace = true } codex-models-manager = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0ecbd02d61b4..38b4dc940ec9 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -41,13 +41,11 @@ use crate::history_cell::HistoryCell; use crate::history_cell::UpdateAvailableHistoryCell; use crate::key_hint::KeyBindingListExt; use crate::keymap::RuntimeKeymap; -use crate::legacy_core::append_message_history_entry; use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; use crate::legacy_core::config::edit::ConfigEdit; use crate::legacy_core::config::edit::ConfigEditsBuilder; -use crate::legacy_core::lookup_message_history_entry; #[cfg(target_os = "windows")] use crate::legacy_core::windows_sandbox::WindowsSandboxLevelExt; use crate::model_catalog::ModelCatalog; diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 69eba70cf971..e50c2782ca87 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -648,8 +648,7 @@ mod tests { cwd: next_cwd.clone().abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 37fcdb4251b5..6702f933f990 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -326,6 +326,12 @@ impl App { AppEvent::CodexOp(op) => { self.submit_active_thread_op(app_server, op).await?; } + AppEvent::AppendMessageHistoryEntry { thread_id, text } => { + self.append_message_history_entry(thread_id, text); + } + AppEvent::LookupMessageHistoryEntry { offset, log_id } => { + self.lookup_message_history_entry(offset, log_id).await?; + } AppEvent::ApproveRecentAutoReviewDenial { thread_id, id } => { self.chat_widget .approve_recent_auto_review_denial(thread_id, id); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 0e59964c3bd0..bc816250fc7b 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -548,18 +548,10 @@ async fn history_lookup_response_is_routed_to_requesting_thread() -> Result<()> let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; let thread_id = ThreadId::new(); - let handled = app - .try_handle_local_history_op( - thread_id, - &Op::GetHistoryEntryRequest { - offset: 0, - log_id: 1, - }, - ) + app.active_thread_id = Some(thread_id); + app.lookup_message_history_entry(/*offset*/ 0, /*log_id*/ 1) .await?; - assert!(handled); - let app_event = tokio::time::timeout(Duration::from_secs(1), app_event_rx.recv()) .await .expect("history lookup should emit an app event") @@ -3665,8 +3657,7 @@ async fn render_clear_ui_header_after_long_transcript_for_snapshot() -> String { cwd: test_path_buf("/tmp/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::High), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }; @@ -3911,8 +3902,7 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), } @@ -4453,8 +4443,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }; @@ -4517,8 +4506,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); @@ -4610,8 +4598,7 @@ async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); @@ -5007,8 +4994,7 @@ async fn new_session_requests_shutdown_for_previous_conversation() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }; @@ -5129,8 +5115,7 @@ async fn clear_only_ui_reset_preserves_chat_session_state() { cwd: test_path_buf("/tmp/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index 759adf92286a..56477c18cfc6 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -354,8 +354,7 @@ mod tests { cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), } diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index df6f01e8bd14..b5e88ea7ad4f 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -394,10 +394,6 @@ impl App { ) -> Result<()> { crate::session_log::log_outbound_op(&op); - if self.try_handle_local_history_op(thread_id, &op).await? { - return Ok(()); - } - if self .try_resolve_app_server_request(app_server, thread_id, &op) .await? @@ -422,70 +418,62 @@ impl App { Ok(()) } - /// Spawn a background task that fetches MCP server status from the app-server - /// via paginated RPCs, then delivers the result back through - /// `AppEvent::McpInventoryLoaded`. - /// - /// The spawned task is fire-and-forget: no `JoinHandle` is stored, so a stale - /// result may arrive after the user has moved on. We currently accept that - /// tradeoff because the effect is limited to stale inventory output in history, - /// while request-token invalidation would add cross-cutting async state for a - /// low-severity path. - pub(super) async fn try_handle_local_history_op( - &mut self, - thread_id: ThreadId, - op: &AppCommand, - ) -> Result { - match op { - AppCommand::AddToHistory { text } => { - let text = text.to_string(); - let config = self.chat_widget.config_ref().clone(); - tokio::spawn(async move { - if let Err(err) = append_message_history_entry(&text, &thread_id, &config).await - { - tracing::warn!( - thread_id = %thread_id, - error = %err, - "failed to append to message history" - ); - } - }); - Ok(true) + /// Persist prompt text in the local cross-session message history. + pub(super) fn append_message_history_entry(&self, thread_id: ThreadId, text: String) { + let history_config = codex_message_history::HistoryConfig::new( + self.chat_widget.config_ref().codex_home.clone(), + &self.chat_widget.config_ref().history, + ); + tokio::spawn(async move { + if let Err(err) = + codex_message_history::append_entry(&text, thread_id, &history_config).await + { + tracing::warn!( + thread_id = %thread_id, + error = %err, + "failed to append to message history" + ); } - AppCommand::GetHistoryEntryRequest { offset, log_id } => { - let config = self.chat_widget.config_ref().clone(); - let app_event_tx = self.app_event_tx.clone(); - let offset = *offset; - let log_id = *log_id; - tokio::spawn(async move { - let entry_opt = tokio::task::spawn_blocking(move || { - lookup_message_history_entry(log_id, offset, &config) - }) - .await - .unwrap_or_else(|err| { - tracing::warn!(error = %err, "history lookup task failed"); - None - }); + }); + } - app_event_tx.send(AppEvent::ThreadHistoryEntryResponse { - thread_id, - event: HistoryLookupResponse { - offset, - log_id, - entry: entry_opt.map(|entry| { - codex_protocol::message_history::HistoryEntry { - conversation_id: entry.session_id, - ts: entry.ts, - text: entry.text, - } - }), - }, - }); - }); - Ok(true) - } - _ => Ok(false), - } + /// Fetch one local cross-session message history entry for the active thread. + pub(super) async fn lookup_message_history_entry( + &mut self, + offset: usize, + log_id: u64, + ) -> Result<()> { + let Some(thread_id) = self.active_thread_id else { + self.chat_widget + .add_error_message("No active thread is available.".to_string()); + return Ok(()); + }; + + let history_config = codex_message_history::HistoryConfig::new( + self.chat_widget.config_ref().codex_home.clone(), + &self.chat_widget.config_ref().history, + ); + let app_event_tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let entry_opt = tokio::task::spawn_blocking(move || { + codex_message_history::lookup(log_id, offset, &history_config) + }) + .await + .unwrap_or_else(|err| { + tracing::warn!(error = %err, "history lookup task failed"); + None + }); + + app_event_tx.send(AppEvent::ThreadHistoryEntryResponse { + thread_id, + event: HistoryLookupResponse { + offset, + log_id, + entry: entry_opt.map(|entry| entry.text), + }, + }); + }); + Ok(()) } pub(super) async fn try_submit_active_thread_op_via_app_server( @@ -922,8 +910,7 @@ impl App { } else if rollout_path.is_some() { session.model.clear(); } - session.history_log_id = 0; - session.history_entry_count = 0; + session.message_history = None; session.rollout_path = rollout_path; self.upsert_agent_picker_thread( thread_id, diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index 3a898b82a3ba..65faa114f113 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -73,8 +73,7 @@ impl App { cwd: thread.cwd.clone(), instruction_source_paths: Vec::new(), reasoning_effort: self.chat_widget.current_reasoning_effort(), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: thread.path.clone(), }); @@ -93,8 +92,7 @@ impl App { } else if thread.path.is_some() { session.model.clear(); } - session.history_log_id = 0; - session.history_entry_count = 0; + session.message_history = None; session } @@ -150,8 +148,7 @@ mod tests { cwd: cwd.abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), } diff --git a/codex-rs/tui/src/app_command.rs b/codex-rs/tui/src/app_command.rs index 8633da04bf7f..dea4a175c1c1 100644 --- a/codex-rs/tui/src/app_command.rs +++ b/codex-rs/tui/src/app_command.rs @@ -104,13 +104,6 @@ pub(crate) enum AppCommand { Review { target: ReviewTarget, }, - AddToHistory { - text: String, - }, - GetHistoryEntryRequest { - offset: usize, - log_id: u64, - }, ApproveGuardianDeniedAction { event: GuardianAssessmentEvent, }, @@ -276,14 +269,6 @@ impl AppCommand { Self::Review { target } } - pub(crate) fn add_to_history(text: String) -> Self { - Self::AddToHistory { text } - } - - pub(crate) fn history_lookup(offset: usize, log_id: u64) -> Self { - Self::GetHistoryEntryRequest { offset, log_id } - } - pub(crate) fn approve_guardian_denied_action(event: GuardianAssessmentEvent) -> Self { Self::ApproveGuardianDeniedAction { event } } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index c88ff1711568..39801ee91026 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -28,7 +28,6 @@ use codex_app_server_protocol::SkillsListResponse; use codex_app_server_protocol::ThreadGoalStatus; use codex_file_search::FileMatch; use codex_protocol::ThreadId; -use codex_protocol::message_history::HistoryEntry; use codex_protocol::openai_models::ModelPreset; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_approval_presets::ApprovalPreset; @@ -68,7 +67,7 @@ pub(crate) enum ThreadGoalSetMode { pub(crate) struct HistoryLookupResponse { pub(crate) offset: usize, pub(crate) log_id: u64, - pub(crate) entry: Option, + pub(crate) entry: Option, } impl RealtimeAudioDeviceKind { @@ -150,6 +149,18 @@ pub(crate) enum AppEvent { event: HistoryLookupResponse, }, + /// Persist a submitted prompt in the cross-session message history. + AppendMessageHistoryEntry { + thread_id: ThreadId, + text: String, + }, + + /// Fetch a persistent cross-session message history entry by offset. + LookupMessageHistoryEntry { + offset: usize, + log_id: u64, + }, + /// Start a new session. NewSession, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 67e486986817..e3610c1b0e05 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -4,11 +4,9 @@ //! request/response plumbing out of `App` and `ChatWidget`. use crate::bottom_pane::FeedbackAudience; -#[cfg(test)] -use crate::legacy_core::append_message_history_entry; use crate::legacy_core::config::Config; -use crate::legacy_core::message_history_metadata; use crate::permission_compat::legacy_compatible_permission_profile; +use crate::session_state::MessageHistoryMetadata; use crate::session_state::ThreadSessionState; use crate::status::StatusAccountDisplay; use crate::status::plan_type_display_name; @@ -1467,8 +1465,9 @@ async fn thread_session_state_from_thread_response( .map(ThreadId::from_string) .transpose() .map_err(|err| format!("forked_from_id is invalid: {err}"))?; - let (history_log_id, history_entry_count) = message_history_metadata(config).await; - let history_entry_count = u64::try_from(history_entry_count).unwrap_or(u64::MAX); + let history_config = + codex_message_history::HistoryConfig::new(config.codex_home.clone(), &config.history); + let (log_id, entry_count) = codex_message_history::history_metadata(&history_config).await; Ok(ThreadSessionState { thread_id, forked_from_id, @@ -1484,8 +1483,10 @@ async fn thread_session_state_from_thread_response( cwd, instruction_source_paths, reasoning_effort, - history_log_id, - history_entry_count, + message_history: Some(MessageHistoryMetadata { + log_id, + entry_count, + }), network_proxy: None, rollout_path, }) @@ -1958,10 +1959,13 @@ mod tests { let config = build_config(&temp_dir).await; let thread_id = ThreadId::new(); - append_message_history_entry("older", &thread_id, &config) + let history_config = + codex_message_history::HistoryConfig::new(config.codex_home.clone(), &config.history); + + codex_message_history::append_entry("older", &thread_id, &history_config) .await .expect("history append should succeed"); - append_message_history_entry("newer", &thread_id, &config) + codex_message_history::append_entry("newer", &thread_id, &history_config) .await .expect("history append should succeed"); @@ -1985,8 +1989,11 @@ mod tests { .await .expect("session should map"); - assert_ne!(session.history_log_id, 0); - assert_eq!(session.history_entry_count, 2); + let metadata = session + .message_history + .expect("session should include message-history metadata"); + assert_ne!(metadata.log_id, 0); + assert_eq!(metadata.entry_count, 2); } #[tokio::test] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 32e8c092fa65..3c793800d8e7 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -849,8 +849,8 @@ impl ChatComposer { && self.remote_image_urls.is_empty() } - /// Record the history metadata advertised by `SessionConfiguredEvent` so - /// that the composer can navigate cross-session history. + /// Record local persistent-history metadata so the composer can navigate + /// cross-session history. pub(crate) fn set_history_metadata(&mut self, log_id: u64, entry_count: usize) { self.history.set_metadata(log_id, entry_count); } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index 52ae811226b5..e168534ae342 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -15,7 +15,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; -use crate::app_command::AppCommand as Op; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::MentionBinding; @@ -105,11 +104,11 @@ impl HistoryEntry { /// the chat composer. This struct is intentionally decoupled from the /// rendering widget so the logic remains isolated and easier to test. pub(crate) struct ChatComposerHistory { - /// Identifier of the history log as reported by `SessionConfiguredEvent`. - history_log_id: Option, + /// Identifier of the persistent history log used for stale lookup rejection. + persistent_log_id: Option, /// Number of entries already present in the persistent cross-session /// history file when the session started. - history_entry_count: usize, + persistent_entry_count: usize, /// Messages submitted by the user *during this UI session* (newest at END). /// Local entries retain full draft state (text elements, image paths, pending pastes, remote image URLs). @@ -216,8 +215,8 @@ impl ChatComposerHistory { /// metadata-free lets the composer reset and reuse this helper across session lifecycles. pub fn new() -> Self { Self { - history_log_id: None, - history_entry_count: 0, + persistent_log_id: None, + persistent_entry_count: 0, local_history: Vec::new(), fetched_history: HashMap::new(), history_cursor: None, @@ -232,8 +231,8 @@ impl ChatComposerHistory { /// because offsets only make sense within one history log snapshot. Reusing old offsets after a /// log-id change would allow a stale async response to hydrate the wrong prompt. pub fn set_metadata(&mut self, log_id: u64, entry_count: usize) { - self.history_log_id = Some(log_id); - self.history_entry_count = entry_count; + self.persistent_log_id = Some(log_id); + self.persistent_entry_count = entry_count; self.fetched_history.clear(); self.local_history.clear(); self.history_cursor = None; @@ -298,7 +297,7 @@ impl ChatComposerHistory { /// history recall. If callers moved the cursor into the middle of a recalled entry and still /// forced navigation, users would lose normal vertical movement within the draft. pub fn should_handle_navigation(&self, text: &str, cursor: usize) -> bool { - if self.history_entry_count == 0 && self.local_history.is_empty() { + if self.persistent_entry_count == 0 && self.local_history.is_empty() { return false; } @@ -320,11 +319,11 @@ impl ChatComposerHistory { /// Handles Up by moving toward older entries in the combined history space. /// /// Local entries can be returned immediately, while missing persistent entries emit a - /// `GetHistoryEntryRequest` and return `None` until the response arrives. Calling this while + /// `LookupMessageHistoryEntry` and return `None` until the response arrives. Calling this while /// Ctrl+R search is active intentionally exits search traversal. pub fn navigate_up(&mut self, app_event_tx: &AppEventSender) -> Option { self.search = None; - let total_entries = self.history_entry_count + self.local_history.len(); + let total_entries = self.persistent_entry_count + self.local_history.len(); if total_entries == 0 { return None; } @@ -346,7 +345,7 @@ impl ChatComposerHistory { /// search state and resumes normal shell-style browsing. pub fn navigate_down(&mut self, app_event_tx: &AppEventSender) -> Option { self.search = None; - let total_entries = self.history_entry_count + self.local_history.len(); + let total_entries = self.persistent_entry_count + self.local_history.len(); if total_entries == 0 { return None; } @@ -385,7 +384,7 @@ impl ChatComposerHistory { entry: Option, app_event_tx: &AppEventSender, ) -> HistoryEntryResponse { - if self.history_log_id != Some(log_id) { + if self.persistent_log_id != Some(log_id) { return HistoryEntryResponse::Ignored; } @@ -517,7 +516,7 @@ impl ChatComposerHistory { // --------------------------------------------------------------------- fn total_entries(&self) -> usize { - self.history_entry_count + self.local_history.len() + self.persistent_entry_count + self.local_history.len() } fn search_start_offset( @@ -588,8 +587,8 @@ impl ChatComposerHistory { if self.search_matches(&entry) && self.search_result_is_unique(&entry) { return self.search_match(offset, entry); } - } else if offset < self.history_entry_count - && let Some(log_id) = self.history_log_id + } else if offset < self.persistent_entry_count + && let Some(log_id) = self.persistent_log_id { if let Some(search) = self.search.as_mut() { search.awaiting = Some(PendingHistorySearch { @@ -598,7 +597,7 @@ impl ChatComposerHistory { boundary_if_exhausted, }); } - app_event_tx.send(AppEvent::CodexOp(Op::history_lookup(offset, log_id))); + app_event_tx.send(AppEvent::LookupMessageHistoryEntry { offset, log_id }); return HistorySearchResult::Pending; } @@ -618,9 +617,9 @@ impl ChatComposerHistory { } fn entry_at_cached_offset(&self, offset: usize) -> Option { - if offset >= self.history_entry_count { + if offset >= self.persistent_entry_count { self.local_history - .get(offset - self.history_entry_count) + .get(offset - self.persistent_entry_count) .cloned() } else { self.fetched_history.get(&offset).cloned() @@ -702,11 +701,11 @@ impl ChatComposerHistory { global_idx: usize, app_event_tx: &AppEventSender, ) -> Option { - if global_idx >= self.history_entry_count { + if global_idx >= self.persistent_entry_count { // Local entry. if let Some(entry) = self .local_history - .get(global_idx - self.history_entry_count) + .get(global_idx - self.persistent_entry_count) .cloned() { self.last_history_text = Some(entry.text.clone()); @@ -715,8 +714,11 @@ impl ChatComposerHistory { } else if let Some(entry) = self.fetched_history.get(&global_idx).cloned() { self.last_history_text = Some(entry.text.clone()); return Some(entry); - } else if let Some(log_id) = self.history_log_id { - app_event_tx.send(AppEvent::CodexOp(Op::history_lookup(global_idx, log_id))); + } else if let Some(log_id) = self.persistent_log_id { + app_event_tx.send(AppEvent::LookupMessageHistoryEntry { + offset: global_idx, + log_id, + }); } None } @@ -838,16 +840,11 @@ mod tests { // Verify that a history lookup request was sent. let event = rx.try_recv().expect("expected AppEvent to be sent"); - let AppEvent::CodexOp(op) = event else { + let AppEvent::LookupMessageHistoryEntry { offset, log_id } = event else { panic!("unexpected event variant"); }; - assert_eq!( - Op::GetHistoryEntryRequest { - log_id: 1, - offset: 2, - }, - op - ); + assert_eq!(offset, 2); + assert_eq!(log_id, 1); // Inject the async response. assert_eq!( @@ -865,16 +862,11 @@ mod tests { // Verify second lookup request for offset 1. let event2 = rx.try_recv().expect("expected second event"); - let AppEvent::CodexOp(op) = event2 else { + let AppEvent::LookupMessageHistoryEntry { offset, log_id } = event2 else { panic!("unexpected event variant"); }; - assert_eq!( - Op::GetHistoryEntryRequest { - log_id: 1, - offset: 1, - }, - op - ); + assert_eq!(offset, 1); + assert_eq!(log_id, 1); assert_eq!( HistoryEntryResponse::Found(HistoryEntry::new("older".to_string())), @@ -1101,16 +1093,13 @@ mod tests { &tx ) ); - let AppEvent::CodexOp(op) = rx.try_recv().expect("expected latest lookup") else { + let AppEvent::LookupMessageHistoryEntry { offset, log_id } = + rx.try_recv().expect("expected latest lookup") + else { panic!("unexpected event variant"); }; - assert_eq!( - Op::GetHistoryEntryRequest { - log_id: 1, - offset: 2, - }, - op - ); + assert_eq!(offset, 2); + assert_eq!(log_id, 1); assert_eq!( HistoryEntryResponse::Search(HistorySearchResult::Pending), @@ -1121,16 +1110,13 @@ mod tests { &tx ) ); - let AppEvent::CodexOp(op) = rx.try_recv().expect("expected next lookup") else { + let AppEvent::LookupMessageHistoryEntry { offset, log_id } = + rx.try_recv().expect("expected next lookup") + else { panic!("unexpected event variant"); }; - assert_eq!( - Op::GetHistoryEntryRequest { - log_id: 1, - offset: 1, - }, - op - ); + assert_eq!(offset, 1); + assert_eq!(log_id, 1); assert_eq!( HistoryEntryResponse::Search(HistorySearchResult::Found(HistoryEntry::new( diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 0b58a913cb66..9edc1d6cf907 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2043,10 +2043,9 @@ impl ChatWidget { self.visible_user_turn_count = 0; self.copy_history_evicted_by_rollback = false; self.saw_copy_source_this_turn = false; - let history_entry_count = - usize::try_from(session.history_entry_count).unwrap_or(usize::MAX); + let history_metadata = session.message_history.unwrap_or_default(); self.bottom_pane - .set_history_metadata(session.history_log_id, history_entry_count); + .set_history_metadata(history_metadata.log_id, history_metadata.entry_count); self.set_skills(/*skills*/ None); self.session_network_proxy = session.network_proxy.clone(); let previous_thread_id = self.thread_id; @@ -4034,7 +4033,7 @@ impl ChatWidget { entry, } = event; self.bottom_pane - .on_history_entry_response(log_id, offset, entry.map(|e| e.text)); + .on_history_entry_response(log_id, offset, entry); } fn on_shutdown_complete(&mut self) { @@ -5564,7 +5563,7 @@ impl ChatWidget { ) -> QueueDrain { let drain = self.submit_shell_command(command); if drain == QueueDrain::Stop { - self.submit_op(AppCommand::add_to_history(history_text.to_string())); + self.append_message_history_entry(history_text.to_string()); } drain } @@ -5895,7 +5894,7 @@ impl ChatWidget { } }; if let Some(history_text) = history_text { - self.submit_op(AppCommand::add_to_history(history_text)); + self.append_message_history_entry(history_text); } if let Some(pending_steer) = pending_steer { @@ -10522,6 +10521,15 @@ impl ChatWidget { true } + fn append_message_history_entry(&self, text: String) { + let Some(thread_id) = self.thread_id else { + tracing::warn!("failed to append to message history: no active thread id"); + return; + }; + self.app_event_tx + .send(AppEvent::AppendMessageHistoryEntry { thread_id, text }); + } + pub(crate) fn prepare_local_op_submission(&mut self, op: &AppCommand) { if matches!(op, AppCommand::Interrupt) && self.agent_turn_running { if let Some(controller) = self.stream_controller.as_mut() { diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 76b2b29bc092..c3311a0991c3 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -29,8 +29,7 @@ async fn submission_preserves_text_elements_and_local_images() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -133,8 +132,7 @@ async fn submission_includes_configured_permission_profile() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -183,8 +181,7 @@ async fn submission_keeps_profile_when_legacy_projection_is_external() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -225,8 +222,7 @@ async fn submission_with_remote_and_local_images_keeps_local_placeholder_numberi cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -319,8 +315,7 @@ async fn enter_with_only_remote_images_submits_user_turn() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -383,8 +378,7 @@ async fn shift_enter_with_only_remote_images_does_not_submit_user_turn() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -422,8 +416,7 @@ async fn enter_with_only_remote_images_does_not_submit_when_modal_is_active() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -461,8 +454,7 @@ async fn enter_with_only_remote_images_does_not_submit_when_input_disabled() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -503,8 +495,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index c9789be14d45..ccc5db956a35 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -958,8 +958,7 @@ async fn bang_shell_enter_while_task_running_submits_run_user_shell_command() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -977,8 +976,8 @@ async fn bang_shell_enter_while_task_running_submits_run_user_shell_command() { other => panic!("expected RunUserShellCommand op, got {other:?}"), } assert_matches!( - op_rx.try_recv(), - Ok(Op::AddToHistory { text }) if text == "!echo hi" + rx.try_recv(), + Ok(AppEvent::AppendMessageHistoryEntry { text, .. }) if text == "!echo hi" ); assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 8c31175998cd..9b94803f8205 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -31,8 +31,7 @@ async fn resumed_initial_messages_render_history() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -102,8 +101,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -171,8 +169,7 @@ async fn replayed_user_message_preserves_remote_image_urls() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -271,8 +268,7 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { cwd: expected_cwd.clone(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: None, }; @@ -328,8 +324,7 @@ async fn session_configured_external_sandbox_keeps_external_runtime_policy() { cwd: test_path_buf("/home/user/external").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: None, }; @@ -367,8 +362,7 @@ async fn replayed_user_message_with_only_remote_images_renders_history_cell() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -422,8 +416,7 @@ async fn replayed_user_message_with_only_local_images_renders_history_cell() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -693,8 +686,7 @@ async fn replayed_reasoning_item_hides_raw_reasoning_when_disabled() { cwd: test_project_path().abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: None, }); @@ -739,8 +731,7 @@ async fn replayed_reasoning_item_shows_raw_reasoning_when_enabled() { cwd: test_project_path().abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: None, }); diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 09595a11ba3d..df3615c0fd3a 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -586,8 +586,7 @@ async fn permissions_selection_marks_auto_review_current_after_session_configure cwd: test_project_path().abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); @@ -634,8 +633,7 @@ async fn permissions_selection_marks_auto_review_current_with_custom_workspace_w cwd, instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), }); diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index 8af07a6aa93e..51f04e167958 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -1219,8 +1219,7 @@ async fn submit_user_message_emits_structured_plugin_mentions_from_bindings() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }; @@ -1463,8 +1462,7 @@ async fn plan_slash_command_with_args_submits_prompt_in_plan_mode() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: None, }; diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 3b6b0e7ff2ca..bd41f01d2dfb 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -38,14 +38,16 @@ fn recall_latest_after_clearing(chat: &mut ChatWidget) -> String { chat.bottom_pane.composer_text() } -fn next_add_to_history_op(op_rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> String { +fn next_add_to_history_event(rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> String { loop { - match op_rx.try_recv() { - Ok(Op::AddToHistory { text }) => return text, + match rx.try_recv() { + Ok(AppEvent::AppendMessageHistoryEntry { text, .. }) => return text, Ok(_) => continue, - Err(TryRecvError::Empty) => panic!("expected AddToHistory op but queue was empty"), + Err(TryRecvError::Empty) => { + panic!("expected AppendMessageHistoryEntry event but queue was empty") + } Err(TryRecvError::Disconnected) => { - panic!("expected AddToHistory op but channel closed") + panic!("expected AppendMessageHistoryEntry event but channel closed") } } } @@ -116,15 +118,6 @@ async fn queued_slash_review_with_args_dispatches_after_active_turn() { complete_turn_with_message(&mut chat, "turn-1", Some("done")); match op_rx.try_recv() { - Ok(Op::AddToHistory { .. }) => match op_rx.try_recv() { - Ok(Op::Review { target }) => assert_eq!( - target, - ReviewTarget::Custom { - instructions: "check regressions".to_string(), - } - ), - other => panic!("expected queued /review to submit review op, got {other:?}"), - }, Ok(Op::Review { target }) => assert_eq!( target, ReviewTarget::Custom { @@ -152,7 +145,7 @@ async fn queued_slash_review_with_args_restores_for_edit() { #[tokio::test] async fn queued_bang_shell_dispatches_after_active_turn() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); handle_turn_started(&mut chat, "turn-1"); @@ -171,10 +164,7 @@ async fn queued_bang_shell_dispatches_after_active_turn() { Ok(Op::RunUserShellCommand { command }) => assert_eq!(command, "echo hi"), other => panic!("expected queued shell command op, got {other:?}"), } - assert_matches!( - op_rx.try_recv(), - Ok(Op::AddToHistory { text }) if text == "!echo hi" - ); + assert_eq!(next_add_to_history_event(&mut rx), "!echo hi"); assert!(chat.queued_user_messages.is_empty()); } @@ -217,7 +207,7 @@ async fn queued_empty_bang_shell_reports_help_when_dequeued_and_drains_next_inpu #[tokio::test] async fn queued_bang_shell_waits_for_user_shell_completion_before_next_input() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); handle_turn_started(&mut chat, "turn-1"); @@ -230,10 +220,7 @@ async fn queued_bang_shell_waits_for_user_shell_completion_before_next_input() { Ok(Op::RunUserShellCommand { command }) => assert_eq!(command, "echo hi"), other => panic!("expected queued shell command op, got {other:?}"), } - assert_matches!( - op_rx.try_recv(), - Ok(Op::AddToHistory { text }) if text == "!echo hi" - ); + assert_eq!(next_add_to_history_event(&mut rx), "!echo hi"); assert_eq!(chat.queued_user_messages.len(), 1); let begin = begin_exec_with_source( @@ -412,10 +399,10 @@ async fn queued_inline_rename_does_not_drain_again_before_turn_started() { ), other => panic!("expected first queued message after /rename, got {other:?}"), } - assert_matches!( - op_rx.try_recv(), - Ok(Op::AddToHistory { text }) if text == "first after rename" - ); + assert!(events.iter().any(|event| matches!( + event, + AppEvent::AppendMessageHistoryEntry { text, .. } if text == "first after rename" + ))); assert_eq!( chat.queued_user_message_texts(), vec!["second after rename"] @@ -945,7 +932,7 @@ fn merged_history_record_remaps_override_image_placeholders() { #[tokio::test] async fn interrupted_merged_message_history_encodes_mentions_once() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.thread_id = Some(ThreadId::new()); chat.on_task_started(); chat.on_agent_message_delta("Final answer line\n".to_string()); @@ -977,7 +964,7 @@ async fn interrupted_merged_message_history_encodes_mentions_once() { other => panic!("expected user turn, got {other:?}"), } let encoded = "use [$figma](app://figma) now"; - assert_eq!(next_add_to_history_op(&mut op_rx), encoded); + assert_eq!(next_add_to_history_event(&mut rx), encoded); chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); next_interrupt_op(&mut op_rx); @@ -997,7 +984,7 @@ async fn interrupted_merged_message_history_encodes_mentions_once() { } other => panic!("expected resubmitted user turn, got {other:?}"), } - assert_eq!(next_add_to_history_op(&mut op_rx), encoded); + assert_eq!(next_add_to_history_event(&mut rx), encoded); } #[tokio::test] 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 96b2b681b6f1..dcb72ed3b2b9 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1863,8 +1863,7 @@ async fn session_configured_clears_goal_status_footer() { cwd: test_path_buf("/home/user/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: Some(ReasoningEffortConfig::default()), - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(rollout_file.path().to_path_buf()), }); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index a9dd59e7941a..256345f67afe 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -3830,8 +3830,7 @@ mod tests { cwd: test_path_buf("/tmp/project").abs(), instruction_source_paths: Vec::new(), reasoning_effort: None, - history_log_id: 0, - history_entry_count: 0, + message_history: None, network_proxy: None, rollout_path: Some(PathBuf::new()), } diff --git a/codex-rs/tui/src/session_state.rs b/codex-rs/tui/src/session_state.rs index ec0f7789d716..b5b8ef7fea34 100644 --- a/codex-rs/tui/src/session_state.rs +++ b/codex-rs/tui/src/session_state.rs @@ -17,6 +17,12 @@ pub(crate) struct SessionNetworkProxyRuntime { pub(crate) socks_addr: String, } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct MessageHistoryMetadata { + pub(crate) log_id: u64, + pub(crate) entry_count: usize, +} + #[derive(Debug, Clone, PartialEq)] pub(crate) struct ThreadSessionState { pub(crate) thread_id: ThreadId, @@ -38,8 +44,7 @@ pub(crate) struct ThreadSessionState { pub(crate) cwd: AbsolutePathBuf, pub(crate) instruction_source_paths: Vec, pub(crate) reasoning_effort: Option, - pub(crate) history_log_id: u64, - pub(crate) history_entry_count: u64, + pub(crate) message_history: Option, pub(crate) network_proxy: Option, pub(crate) rollout_path: Option, } diff --git a/codex-rs/tui/tests/fixtures/oss-story.jsonl b/codex-rs/tui/tests/fixtures/oss-story.jsonl index 72d0fc40f496..a62182f4879c 100644 --- a/codex-rs/tui/tests/fixtures/oss-story.jsonl +++ b/codex-rs/tui/tests/fixtures/oss-story.jsonl @@ -2,7 +2,7 @@ {"ts":"2025-08-10T03:12:26.500Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-10T03:12:26.502Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] resume_path: None"} {"ts":"2025-08-10T03:12:26.502Z","dir":"to_tui","kind":"app_event","variant":"Redraw"} -{"ts":"2025-08-10T03:12:26.519Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"8f7c4ac2-6141-42da-b4d5-7032a8e8df3b","model":"gpt-oss:20b","history_log_id":2532619,"history_entry_count":355}}} +{"ts":"2025-08-10T03:12:26.519Z","dir":"to_tui","kind":"codex_event","payload":{"id":"0","msg":{"type":"session_configured","session_id":"8f7c4ac2-6141-42da-b4d5-7032a8e8df3b","model":"gpt-oss:20b"}}} {"ts":"2025-08-10T03:12:26.520Z","dir":"to_tui","kind":"insert_history","lines":9} {"ts":"2025-08-10T03:12:26.520Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} {"ts":"2025-08-10T03:12:26.520Z","dir":"to_tui","kind":"app_event","variant":"RequestRedraw"} From 69c15d5df784297868cc9a99876e31d0ae3f2bd1 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 5 May 2026 18:57:33 -0700 Subject: [PATCH 2/2] codex: address PR review feedback (#21278) --- codex-rs/message-history/Cargo.toml | 2 +- codex-rs/tui/src/app/event_dispatch.rs | 9 ++- codex-rs/tui/src/app/tests.rs | 3 +- codex-rs/tui/src/app/thread_routing.rs | 9 +-- codex-rs/tui/src/app_event.rs | 1 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 10 ++- .../src/bottom_pane/chat_composer_history.rs | 67 ++++++++++++++----- codex-rs/tui/src/bottom_pane/mod.rs | 11 ++- codex-rs/tui/src/chatwidget.rs | 7 +- 9 files changed, 86 insertions(+), 33 deletions(-) diff --git a/codex-rs/message-history/Cargo.toml b/codex-rs/message-history/Cargo.toml index 8b03e9a2010c..34bffd687a27 100644 --- a/codex-rs/message-history/Cargo.toml +++ b/codex-rs/message-history/Cargo.toml @@ -15,7 +15,7 @@ workspace = true codex-config = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -tokio = { workspace = true, features = ["fs", "rt"] } +tokio = { workspace = true, features = ["fs", "io-util", "rt"] } tracing = { workspace = true, features = ["log"] } [dev-dependencies] diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6702f933f990..c5858b9b7b9c 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -329,8 +329,13 @@ impl App { AppEvent::AppendMessageHistoryEntry { thread_id, text } => { self.append_message_history_entry(thread_id, text); } - AppEvent::LookupMessageHistoryEntry { offset, log_id } => { - self.lookup_message_history_entry(offset, log_id).await?; + AppEvent::LookupMessageHistoryEntry { + thread_id, + offset, + log_id, + } => { + self.lookup_message_history_entry(thread_id, offset, log_id) + .await?; } AppEvent::ApproveRecentAutoReviewDenial { thread_id, id } => { self.chat_widget diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index bc816250fc7b..487e965e7dfc 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -548,8 +548,7 @@ async fn history_lookup_response_is_routed_to_requesting_thread() -> Result<()> let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; let thread_id = ThreadId::new(); - app.active_thread_id = Some(thread_id); - app.lookup_message_history_entry(/*offset*/ 0, /*log_id*/ 1) + app.lookup_message_history_entry(thread_id, /*offset*/ 0, /*log_id*/ 1) .await?; let app_event = tokio::time::timeout(Duration::from_secs(1), app_event_rx.recv()) diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index b5e88ea7ad4f..00c05922932a 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -437,18 +437,13 @@ impl App { }); } - /// Fetch one local cross-session message history entry for the active thread. + /// Fetch one local cross-session message history entry for the requesting thread. pub(super) async fn lookup_message_history_entry( &mut self, + thread_id: ThreadId, offset: usize, log_id: u64, ) -> Result<()> { - let Some(thread_id) = self.active_thread_id else { - self.chat_widget - .add_error_message("No active thread is available.".to_string()); - return Ok(()); - }; - let history_config = codex_message_history::HistoryConfig::new( self.chat_widget.config_ref().codex_home.clone(), &self.chat_widget.config_ref().history, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 39801ee91026..4ee405f49525 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -157,6 +157,7 @@ pub(crate) enum AppEvent { /// Fetch a persistent cross-session message history entry by offset. LookupMessageHistoryEntry { + thread_id: ThreadId, offset: usize, log_id: u64, }, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 3c793800d8e7..0084e4604f88 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -207,6 +207,7 @@ use crate::render::RectExt; use crate::render::renderable::Renderable; use crate::slash_command::SlashCommand; use crate::style::user_message_style; +use codex_protocol::ThreadId; use codex_protocol::models::local_image_label_text; use codex_protocol::user_input::ByteRange; use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; @@ -851,8 +852,13 @@ impl ChatComposer { /// Record local persistent-history metadata so the composer can navigate /// cross-session history. - pub(crate) fn set_history_metadata(&mut self, log_id: u64, entry_count: usize) { - self.history.set_metadata(log_id, entry_count); + pub(crate) fn set_history_metadata( + &mut self, + thread_id: ThreadId, + log_id: u64, + entry_count: usize, + ) { + self.history.set_metadata(thread_id, log_id, entry_count); } /// Integrate an asynchronous response to an on-demand history lookup. diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index e168534ae342..6a490e81ec19 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -19,6 +19,7 @@ use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::MentionBinding; use crate::mention_codec::decode_history_mentions; +use codex_protocol::ThreadId; use codex_protocol::user_input::TextElement; /// A composer history entry that can rehydrate draft state. @@ -104,6 +105,8 @@ impl HistoryEntry { /// the chat composer. This struct is intentionally decoupled from the /// rendering widget so the logic remains isolated and easier to test. pub(crate) struct ChatComposerHistory { + /// Thread that owns persistent lookup responses for this metadata snapshot. + thread_id: Option, /// Identifier of the persistent history log used for stale lookup rejection. persistent_log_id: Option, /// Number of entries already present in the persistent cross-session @@ -215,6 +218,7 @@ impl ChatComposerHistory { /// metadata-free lets the composer reset and reuse this helper across session lifecycles. pub fn new() -> Self { Self { + thread_id: None, persistent_log_id: None, persistent_entry_count: 0, local_history: Vec::new(), @@ -230,7 +234,8 @@ impl ChatComposerHistory { /// This clears fetched entries, local entries, navigation cursors, and active search state /// because offsets only make sense within one history log snapshot. Reusing old offsets after a /// log-id change would allow a stale async response to hydrate the wrong prompt. - pub fn set_metadata(&mut self, log_id: u64, entry_count: usize) { + pub fn set_metadata(&mut self, thread_id: ThreadId, log_id: u64, entry_count: usize) { + self.thread_id = Some(thread_id); self.persistent_log_id = Some(log_id); self.persistent_entry_count = entry_count; self.fetched_history.clear(); @@ -588,7 +593,7 @@ impl ChatComposerHistory { return self.search_match(offset, entry); } } else if offset < self.persistent_entry_count - && let Some(log_id) = self.persistent_log_id + && let (Some(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) { if let Some(search) = self.search.as_mut() { search.awaiting = Some(PendingHistorySearch { @@ -597,7 +602,11 @@ impl ChatComposerHistory { boundary_if_exhausted, }); } - app_event_tx.send(AppEvent::LookupMessageHistoryEntry { offset, log_id }); + app_event_tx.send(AppEvent::LookupMessageHistoryEntry { + thread_id, + offset, + log_id, + }); return HistorySearchResult::Pending; } @@ -714,8 +723,9 @@ impl ChatComposerHistory { } else if let Some(entry) = self.fetched_history.get(&global_idx).cloned() { self.last_history_text = Some(entry.text.clone()); return Some(entry); - } else if let Some(log_id) = self.persistent_log_id { + } else if let (Some(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) { app_event_tx.send(AppEvent::LookupMessageHistoryEntry { + thread_id, offset: global_idx, log_id, }); @@ -796,6 +806,11 @@ mod tests { use pretty_assertions::assert_eq; use tokio::sync::mpsc::unbounded_channel; + fn test_thread_id() -> ThreadId { + ThreadId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8") + .expect("thread id should parse") + } + #[test] fn duplicate_submissions_are_not_recorded() { let mut history = ChatComposerHistory::new(); @@ -832,7 +847,8 @@ mod tests { let mut history = ChatComposerHistory::new(); // Pretend there are 3 persistent entries. - history.set_metadata(/*log_id*/ 1, /*entry_count*/ 3); + let thread_id = test_thread_id(); + history.set_metadata(thread_id, /*log_id*/ 1, /*entry_count*/ 3); // First Up should request offset 2 (latest) and await async data. assert!(history.should_handle_navigation("", /*cursor*/ 0)); @@ -840,9 +856,15 @@ mod tests { // Verify that a history lookup request was sent. let event = rx.try_recv().expect("expected AppEvent to be sent"); - let AppEvent::LookupMessageHistoryEntry { offset, log_id } = event else { + let AppEvent::LookupMessageHistoryEntry { + thread_id: response_thread_id, + offset, + log_id, + } = event + else { panic!("unexpected event variant"); }; + assert_eq!(response_thread_id, thread_id); assert_eq!(offset, 2); assert_eq!(log_id, 1); @@ -862,9 +884,15 @@ mod tests { // Verify second lookup request for offset 1. let event2 = rx.try_recv().expect("expected second event"); - let AppEvent::LookupMessageHistoryEntry { offset, log_id } = event2 else { + let AppEvent::LookupMessageHistoryEntry { + thread_id: response_thread_id, + offset, + log_id, + } = event2 + else { panic!("unexpected event variant"); }; + assert_eq!(response_thread_id, thread_id); assert_eq!(offset, 1); assert_eq!(log_id, 1); @@ -1009,7 +1037,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut history = ChatComposerHistory::new(); - history.set_metadata(/*log_id*/ 1, /*entry_count*/ 3); + history.set_metadata(test_thread_id(), /*log_id*/ 1, /*entry_count*/ 3); assert_eq!( HistorySearchResult::Pending, @@ -1082,7 +1110,8 @@ mod tests { let tx = AppEventSender::new(tx); let mut history = ChatComposerHistory::new(); - history.set_metadata(/*log_id*/ 1, /*entry_count*/ 3); + let thread_id = test_thread_id(); + history.set_metadata(thread_id, /*log_id*/ 1, /*entry_count*/ 3); assert_eq!( HistorySearchResult::Pending, @@ -1093,11 +1122,15 @@ mod tests { &tx ) ); - let AppEvent::LookupMessageHistoryEntry { offset, log_id } = - rx.try_recv().expect("expected latest lookup") + let AppEvent::LookupMessageHistoryEntry { + thread_id: response_thread_id, + offset, + log_id, + } = rx.try_recv().expect("expected latest lookup") else { panic!("unexpected event variant"); }; + assert_eq!(response_thread_id, thread_id); assert_eq!(offset, 2); assert_eq!(log_id, 1); @@ -1110,11 +1143,15 @@ mod tests { &tx ) ); - let AppEvent::LookupMessageHistoryEntry { offset, log_id } = - rx.try_recv().expect("expected next lookup") + let AppEvent::LookupMessageHistoryEntry { + thread_id: response_thread_id, + offset, + log_id, + } = rx.try_recv().expect("expected next lookup") else { panic!("unexpected event variant"); }; + assert_eq!(response_thread_id, thread_id); assert_eq!(offset, 1); assert_eq!(log_id, 1); @@ -1137,7 +1174,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut history = ChatComposerHistory::new(); - history.set_metadata(/*log_id*/ 1, /*entry_count*/ 4); + history.set_metadata(test_thread_id(), /*log_id*/ 1, /*entry_count*/ 4); assert_eq!( HistorySearchResult::Pending, @@ -1256,7 +1293,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut history = ChatComposerHistory::new(); - history.set_metadata(/*log_id*/ 1, /*entry_count*/ 3); + history.set_metadata(test_thread_id(), /*log_id*/ 1, /*entry_count*/ 3); history .fetched_history .insert(1, HistoryEntry::new("command2".to_string())); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 7b0694e0b346..d11b01cc9e60 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -36,6 +36,7 @@ use codex_core_skills::model::SkillMetadata; use codex_features::Features; use codex_file_search::FileMatch; use codex_plugin::PluginCapabilitySummary; +use codex_protocol::ThreadId; use codex_protocol::user_input::TextElement; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -1420,8 +1421,14 @@ impl BottomPane { // --- History helpers --- - pub(crate) fn set_history_metadata(&mut self, log_id: u64, entry_count: usize) { - self.composer.set_history_metadata(log_id, entry_count); + pub(crate) fn set_history_metadata( + &mut self, + thread_id: ThreadId, + log_id: u64, + entry_count: usize, + ) { + self.composer + .set_history_metadata(thread_id, log_id, entry_count); } pub(crate) fn flush_paste_burst_if_due(&mut self) -> bool { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9edc1d6cf907..c8fccbf8f2b0 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2044,8 +2044,11 @@ impl ChatWidget { self.copy_history_evicted_by_rollback = false; self.saw_copy_source_this_turn = false; let history_metadata = session.message_history.unwrap_or_default(); - self.bottom_pane - .set_history_metadata(history_metadata.log_id, history_metadata.entry_count); + self.bottom_pane.set_history_metadata( + session.thread_id, + history_metadata.log_id, + history_metadata.entry_count, + ); self.set_skills(/*skills*/ None); self.session_network_proxy = session.network_proxy.clone(); let previous_thread_id = self.thread_id;