From e6c206d19d4e9e2735e96d05b2fd3e1b92b36d44 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 13 May 2025 19:22:16 -0700 Subject: [PATCH] fix: tighten up some logic around session timestamps and ids (#922) * update `SessionConfigured` event to include the UUID for the session * show the UUID in the Rust TUI * use local timestamps in log files instead of UTC * include timestamps in log file names for easier discovery --- codex-rs/Cargo.lock | 13 ++++ codex-rs/core/Cargo.toml | 4 +- codex-rs/core/src/codex.rs | 20 +++-- codex-rs/core/src/protocol.rs | 15 +++- codex-rs/core/src/rollout.rs | 21 ++--- codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/chatwidget.rs | 12 +-- .../tui/src/conversation_history_widget.rs | 14 ++-- codex-rs/tui/src/history_cell.rs | 78 ++++++++++--------- 9 files changed, 101 insertions(+), 77 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 15a62983..d67a2df7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -639,6 +639,7 @@ dependencies = [ "tui-input", "tui-markdown", "tui-textarea", + "uuid", ] [[package]] @@ -2275,6 +2276,15 @@ dependencies = [ "libc", ] +[[package]] +name = "num_threads" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c7398b9c8b70908f6371f47ed36737907c87c52af34c268fed0bf0ceb92ead9" +dependencies = [ + "libc", +] + [[package]] name = "object" version = "0.32.2" @@ -3686,7 +3696,9 @@ checksum = "8a7619e19bc266e0f9c5e6686659d394bc57973859340060a69221e57dbc0c40" dependencies = [ "deranged", "itoa", + "libc", "num-conv", + "num_threads", "powerfmt", "serde", "time-core", @@ -4097,6 +4109,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9" dependencies = [ "getrandom 0.3.2", + "serde", ] [[package]] diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 6154d91d..e7a93d3d 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -31,7 +31,7 @@ reqwest = { version = "0.12", features = ["json", "stream"] } serde = { version = "1", features = ["derive"] } serde_json = "1" thiserror = "2.0.12" -time = { version = "0.3", features = ["formatting", "macros"] } +time = { version = "0.3", features = ["formatting", "local-offset", "macros"] } tokio = { version = "1", features = [ "io-std", "macros", @@ -44,7 +44,7 @@ toml = "0.8.20" tracing = { version = "0.1.41", features = ["log"] } tree-sitter = "0.25.3" tree-sitter-bash = "0.23.3" -uuid = { version = "1", features = ["v4"] } +uuid = { version = "1", features = ["serde", "v4"] } [target.'cfg(target_os = "linux")'.dependencies] libc = "0.2.172" diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 82296ccb..26e1f665 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -30,6 +30,7 @@ use tracing::error; use tracing::info; use tracing::trace; use tracing::warn; +use uuid::Uuid; use crate::WireApi; use crate::client::ModelClient; @@ -62,6 +63,7 @@ use crate::protocol::InputItem; use crate::protocol::Op; use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; +use crate::protocol::SessionConfiguredEvent; use crate::protocol::Submission; use crate::rollout::RolloutRecorder; use crate::safety::SafetyCheck; @@ -596,13 +598,15 @@ async fn submission_loop( // Attempt to create a RolloutRecorder *before* moving the // `instructions` value into the Session struct. - let rollout_recorder = match RolloutRecorder::new(instructions.clone()).await { - Ok(r) => Some(r), - Err(e) => { - tracing::warn!("failed to initialise rollout recorder: {e}"); - None - } - }; + let session_id = Uuid::new_v4(); + let rollout_recorder = + match RolloutRecorder::new(session_id, instructions.clone()).await { + Ok(r) => Some(r), + Err(e) => { + tracing::warn!("failed to initialise rollout recorder: {e}"); + None + } + }; sess = Some(Arc::new(Session { client, @@ -622,7 +626,7 @@ async fn submission_loop( // ack let events = std::iter::once(Event { id: sub.id.clone(), - msg: EventMsg::SessionConfigured { model }, + msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id, model }), }) .chain(mcp_connection_errors.into_iter()); for event in events { diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 1069a904..e4b83826 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use mcp_types::CallToolResult; use serde::Deserialize; use serde::Serialize; +use uuid::Uuid; use crate::model_provider_info::ModelProviderInfo; @@ -323,10 +324,7 @@ pub enum EventMsg { }, /// Ack the client's configure message. - SessionConfigured { - /// Tell the client what model is being queried. - model: String, - }, + SessionConfigured(SessionConfiguredEvent), McpToolCallBegin { /// Identifier so this can be paired with the McpToolCallEnd event. @@ -429,6 +427,15 @@ pub enum EventMsg { }, } +#[derive(Debug, Default, Clone, Deserialize, Serialize)] +pub struct SessionConfiguredEvent { + /// Unique id for this session. + pub session_id: Uuid, + + /// Tell the client what model is being queried. + pub model: String, +} + /// User's decision in response to an ExecApprovalRequest. #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] #[serde(rename_all = "snake_case")] diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index 2a45222a..7a014f40 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -37,8 +37,8 @@ struct SessionMeta { /// Rollouts are recorded as JSONL and can be inspected with tools such as: /// /// ```ignore -/// $ jq -C . ~/.codex/sessions/rollout-2025-05-07-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl -/// $ fx ~/.codex/sessions/rollout-2025-05-07-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl +/// $ jq -C . ~/.codex/sessions/rollout-2025-05-07T17-24-21-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl +/// $ fx ~/.codex/sessions/rollout-2025-05-07T17-24-21-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl /// ``` #[derive(Clone)] pub(crate) struct RolloutRecorder { @@ -49,12 +49,12 @@ impl RolloutRecorder { /// Attempt to create a new [`RolloutRecorder`]. If the sessions directory /// cannot be created or the rollout file cannot be opened we return the /// error so the caller can decide whether to disable persistence. - pub async fn new(instructions: Option) -> std::io::Result { + pub async fn new(uuid: Uuid, instructions: Option) -> std::io::Result { let LogFileInfo { file, session_id, timestamp, - } = create_log_file()?; + } = create_log_file(uuid)?; // Build the static session metadata JSON first. let timestamp_format: &[FormatItem] = format_description!( @@ -154,18 +154,19 @@ struct LogFileInfo { timestamp: OffsetDateTime, } -fn create_log_file() -> std::io::Result { +fn create_log_file(session_id: Uuid) -> std::io::Result { // Resolve ~/.codex/sessions and create it if missing. let mut dir = codex_dir()?; dir.push(SESSIONS_SUBDIR); fs::create_dir_all(&dir)?; - // Generate a v4 UUID – matches the JS CLI implementation. - let session_id = Uuid::new_v4(); - let timestamp = OffsetDateTime::now_utc(); + let timestamp = OffsetDateTime::now_local() + .map_err(|e| IoError::new(ErrorKind::Other, format!("failed to get local time: {e}")))?; - // Custom format for YYYY-MM-DD. - let format: &[FormatItem] = format_description!("[year]-[month]-[day]"); + // Custom format for YYYY-MM-DDThh-mm-ss. Use `-` instead of `:` for + // compatibility with filesystems that do not allow colons in filenames. + let format: &[FormatItem] = + format_description!("[year]-[month]-[day]T[hour]-[minute]-[second]"); let date_str = timestamp .format(format) .map_err(|e| IoError::new(ErrorKind::Other, format!("failed to format timestamp: {e}")))?; diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 230cbd2b..4bd23015 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -42,3 +42,4 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] } tui-input = "0.11.1" tui-markdown = "0.3.3" tui-textarea = "0.7.0" +uuid = { version = "1" } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c9a04b7b..accb7305 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -102,8 +102,6 @@ impl ChatWidget<'_> { config, }; - let _ = chat_widget.submit_welcome_message(); - if initial_prompt.is_some() || !initial_images.is_empty() { let text = initial_prompt.unwrap_or_default(); let _ = chat_widget.submit_user_message_with_images(text, initial_images); @@ -161,12 +159,6 @@ impl ChatWidget<'_> { } } - fn submit_welcome_message(&mut self) -> std::result::Result<(), SendError> { - self.conversation_history.add_welcome_message(&self.config); - self.request_redraw()?; - Ok(()) - } - fn submit_user_message( &mut self, text: String, @@ -215,10 +207,10 @@ impl ChatWidget<'_> { ) -> std::result::Result<(), SendError> { let Event { id, msg } = event; match msg { - EventMsg::SessionConfigured { model } => { + EventMsg::SessionConfigured(event) => { // Record session information at the top of the conversation. self.conversation_history - .add_session_info(&self.config, model); + .add_session_info(&self.config, event); self.request_redraw()?; } EventMsg::AgentMessage { message } => { diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index 70e7b6c4..f7a94059 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -3,6 +3,7 @@ use crate::history_cell::HistoryCell; use crate::history_cell::PatchEventType; use codex_core::config::Config; use codex_core::protocol::FileChange; +use codex_core::protocol::SessionConfiguredEvent; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use ratatui::prelude::*; @@ -162,8 +163,11 @@ impl ConversationHistoryWidget { self.scroll_position = usize::MAX; } - pub fn add_welcome_message(&mut self, config: &Config) { - self.add_to_history(HistoryCell::new_welcome_message(config)); + /// Note `model` could differ from `config.model` if the agent decided to + /// use a different model than the one requested by the user. + pub fn add_session_info(&mut self, config: &Config, event: SessionConfiguredEvent) { + let is_first_event = self.history.is_empty(); + self.add_to_history(HistoryCell::new_session_info(config, event, is_first_event)); } pub fn add_user_message(&mut self, message: String) { @@ -195,12 +199,6 @@ impl ConversationHistoryWidget { self.add_to_history(HistoryCell::new_patch_event(event_type, changes)); } - /// Note `model` could differ from `config.model` if the agent decided to - /// use a different model than the one requested by the user. - pub fn add_session_info(&mut self, config: &Config, model: String) { - self.add_to_history(HistoryCell::new_session_info(config, model)); - } - pub fn add_active_exec_command(&mut self, call_id: String, command: Vec) { self.add_to_history(HistoryCell::new_active_exec_command(call_id, command)); } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 4f4259aa..23ce6667 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2,6 +2,7 @@ use codex_ansi_escape::ansi_escape_line; use codex_common::elapsed::format_duration; use codex_core::config::Config; use codex_core::protocol::FileChange; +use codex_core::protocol::SessionConfiguredEvent; use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; @@ -94,29 +95,50 @@ pub(crate) enum HistoryCell { const TOOL_CALL_MAX_LINES: usize = 5; impl HistoryCell { - pub(crate) fn new_welcome_message(config: &Config) -> Self { - let mut lines: Vec> = vec![ - Line::from(vec![ - "OpenAI ".into(), - "Codex".bold(), - " (research preview)".dim(), - ]), - Line::from(""), - Line::from("codex session:".magenta().bold()), - ]; + pub(crate) fn new_session_info( + config: &Config, + event: SessionConfiguredEvent, + is_first_event: bool, + ) -> Self { + let SessionConfiguredEvent { model, session_id } = event; + if is_first_event { + let mut lines: Vec> = vec![ + Line::from(vec![ + "OpenAI ".into(), + "Codex".bold(), + " (research preview)".dim(), + ]), + Line::from(""), + Line::from(vec![ + "codex session".magenta().bold(), + " ".into(), + session_id.to_string().dim(), + ]), + ]; - let entries = vec![ - ("workdir", config.cwd.display().to_string()), - ("model", config.model.clone()), - ("provider", config.model_provider_id.clone()), - ("approval", format!("{:?}", config.approval_policy)), - ("sandbox", format!("{:?}", config.sandbox_policy)), - ]; - for (key, value) in entries { - lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()])); + let entries = vec![ + ("workdir", config.cwd.display().to_string()), + ("model", config.model.clone()), + ("provider", config.model_provider_id.clone()), + ("approval", format!("{:?}", config.approval_policy)), + ("sandbox", format!("{:?}", config.sandbox_policy)), + ]; + for (key, value) in entries { + lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()])); + } + lines.push(Line::from("")); + HistoryCell::WelcomeMessage { lines } + } else if config.model == model { + HistoryCell::SessionInfo { lines: vec![] } + } else { + let lines = vec![ + Line::from("model changed:".magenta().bold()), + Line::from(format!("requested: {}", config.model)), + Line::from(format!("used: {}", model)), + Line::from(""), + ]; + HistoryCell::SessionInfo { lines } } - lines.push(Line::from("")); - HistoryCell::WelcomeMessage { lines } } pub(crate) fn new_user_prompt(message: String) -> Self { @@ -296,20 +318,6 @@ impl HistoryCell { HistoryCell::ErrorEvent { lines } } - pub(crate) fn new_session_info(config: &Config, model: String) -> Self { - if config.model == model { - HistoryCell::SessionInfo { lines: vec![] } - } else { - let lines = vec![ - Line::from("model changed:".magenta().bold()), - Line::from(format!("requested: {}", config.model)), - Line::from(format!("used: {}", model)), - Line::from(""), - ]; - HistoryCell::SessionInfo { lines } - } - } - /// Create a new `PendingPatch` cell that lists the file‑level summary of /// a proposed patch. The summary lines should already be formatted (e.g. /// "A path/to/file.rs").