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
This commit is contained in:
Michael Bolin
2025-05-13 19:22:16 -07:00
committed by GitHub
parent 3c03c25e56
commit e6c206d19d
9 changed files with 101 additions and 77 deletions

13
codex-rs/Cargo.lock generated
View File

@@ -639,6 +639,7 @@ dependencies = [
"tui-input", "tui-input",
"tui-markdown", "tui-markdown",
"tui-textarea", "tui-textarea",
"uuid",
] ]
[[package]] [[package]]
@@ -2275,6 +2276,15 @@ dependencies = [
"libc", "libc",
] ]
[[package]]
name = "num_threads"
version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5c7398b9c8b70908f6371f47ed36737907c87c52af34c268fed0bf0ceb92ead9"
dependencies = [
"libc",
]
[[package]] [[package]]
name = "object" name = "object"
version = "0.32.2" version = "0.32.2"
@@ -3686,7 +3696,9 @@ checksum = "8a7619e19bc266e0f9c5e6686659d394bc57973859340060a69221e57dbc0c40"
dependencies = [ dependencies = [
"deranged", "deranged",
"itoa", "itoa",
"libc",
"num-conv", "num-conv",
"num_threads",
"powerfmt", "powerfmt",
"serde", "serde",
"time-core", "time-core",
@@ -4097,6 +4109,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9" checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9"
dependencies = [ dependencies = [
"getrandom 0.3.2", "getrandom 0.3.2",
"serde",
] ]
[[package]] [[package]]

View File

@@ -31,7 +31,7 @@ reqwest = { version = "0.12", features = ["json", "stream"] }
serde = { version = "1", features = ["derive"] } serde = { version = "1", features = ["derive"] }
serde_json = "1" serde_json = "1"
thiserror = "2.0.12" thiserror = "2.0.12"
time = { version = "0.3", features = ["formatting", "macros"] } time = { version = "0.3", features = ["formatting", "local-offset", "macros"] }
tokio = { version = "1", features = [ tokio = { version = "1", features = [
"io-std", "io-std",
"macros", "macros",
@@ -44,7 +44,7 @@ toml = "0.8.20"
tracing = { version = "0.1.41", features = ["log"] } tracing = { version = "0.1.41", features = ["log"] }
tree-sitter = "0.25.3" tree-sitter = "0.25.3"
tree-sitter-bash = "0.23.3" tree-sitter-bash = "0.23.3"
uuid = { version = "1", features = ["v4"] } uuid = { version = "1", features = ["serde", "v4"] }
[target.'cfg(target_os = "linux")'.dependencies] [target.'cfg(target_os = "linux")'.dependencies]
libc = "0.2.172" libc = "0.2.172"

View File

@@ -30,6 +30,7 @@ use tracing::error;
use tracing::info; use tracing::info;
use tracing::trace; use tracing::trace;
use tracing::warn; use tracing::warn;
use uuid::Uuid;
use crate::WireApi; use crate::WireApi;
use crate::client::ModelClient; use crate::client::ModelClient;
@@ -62,6 +63,7 @@ use crate::protocol::InputItem;
use crate::protocol::Op; use crate::protocol::Op;
use crate::protocol::ReviewDecision; use crate::protocol::ReviewDecision;
use crate::protocol::SandboxPolicy; use crate::protocol::SandboxPolicy;
use crate::protocol::SessionConfiguredEvent;
use crate::protocol::Submission; use crate::protocol::Submission;
use crate::rollout::RolloutRecorder; use crate::rollout::RolloutRecorder;
use crate::safety::SafetyCheck; use crate::safety::SafetyCheck;
@@ -596,13 +598,15 @@ async fn submission_loop(
// Attempt to create a RolloutRecorder *before* moving the // Attempt to create a RolloutRecorder *before* moving the
// `instructions` value into the Session struct. // `instructions` value into the Session struct.
let rollout_recorder = match RolloutRecorder::new(instructions.clone()).await { let session_id = Uuid::new_v4();
Ok(r) => Some(r), let rollout_recorder =
Err(e) => { match RolloutRecorder::new(session_id, instructions.clone()).await {
tracing::warn!("failed to initialise rollout recorder: {e}"); Ok(r) => Some(r),
None Err(e) => {
} tracing::warn!("failed to initialise rollout recorder: {e}");
}; None
}
};
sess = Some(Arc::new(Session { sess = Some(Arc::new(Session {
client, client,
@@ -622,7 +626,7 @@ async fn submission_loop(
// ack // ack
let events = std::iter::once(Event { let events = std::iter::once(Event {
id: sub.id.clone(), id: sub.id.clone(),
msg: EventMsg::SessionConfigured { model }, msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id, model }),
}) })
.chain(mcp_connection_errors.into_iter()); .chain(mcp_connection_errors.into_iter());
for event in events { for event in events {

View File

@@ -10,6 +10,7 @@ use std::path::PathBuf;
use mcp_types::CallToolResult; use mcp_types::CallToolResult;
use serde::Deserialize; use serde::Deserialize;
use serde::Serialize; use serde::Serialize;
use uuid::Uuid;
use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::ModelProviderInfo;
@@ -323,10 +324,7 @@ pub enum EventMsg {
}, },
/// Ack the client's configure message. /// Ack the client's configure message.
SessionConfigured { SessionConfigured(SessionConfiguredEvent),
/// Tell the client what model is being queried.
model: String,
},
McpToolCallBegin { McpToolCallBegin {
/// Identifier so this can be paired with the McpToolCallEnd event. /// 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. /// User's decision in response to an ExecApprovalRequest.
#[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)] #[derive(Debug, Default, Clone, Copy, Deserialize, Serialize)]
#[serde(rename_all = "snake_case")] #[serde(rename_all = "snake_case")]

View File

@@ -37,8 +37,8 @@ struct SessionMeta {
/// Rollouts are recorded as JSONL and can be inspected with tools such as: /// Rollouts are recorded as JSONL and can be inspected with tools such as:
/// ///
/// ```ignore /// ```ignore
/// $ jq -C . ~/.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-07-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl /// $ fx ~/.codex/sessions/rollout-2025-05-07T17-24-21-5973b6c0-94b8-487b-a530-2aeb6098ae0e.jsonl
/// ``` /// ```
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct RolloutRecorder { pub(crate) struct RolloutRecorder {
@@ -49,12 +49,12 @@ impl RolloutRecorder {
/// Attempt to create a new [`RolloutRecorder`]. If the sessions directory /// Attempt to create a new [`RolloutRecorder`]. If the sessions directory
/// cannot be created or the rollout file cannot be opened we return the /// cannot be created or the rollout file cannot be opened we return the
/// error so the caller can decide whether to disable persistence. /// error so the caller can decide whether to disable persistence.
pub async fn new(instructions: Option<String>) -> std::io::Result<Self> { pub async fn new(uuid: Uuid, instructions: Option<String>) -> std::io::Result<Self> {
let LogFileInfo { let LogFileInfo {
file, file,
session_id, session_id,
timestamp, timestamp,
} = create_log_file()?; } = create_log_file(uuid)?;
// Build the static session metadata JSON first. // Build the static session metadata JSON first.
let timestamp_format: &[FormatItem] = format_description!( let timestamp_format: &[FormatItem] = format_description!(
@@ -154,18 +154,19 @@ struct LogFileInfo {
timestamp: OffsetDateTime, timestamp: OffsetDateTime,
} }
fn create_log_file() -> std::io::Result<LogFileInfo> { fn create_log_file(session_id: Uuid) -> std::io::Result<LogFileInfo> {
// Resolve ~/.codex/sessions and create it if missing. // Resolve ~/.codex/sessions and create it if missing.
let mut dir = codex_dir()?; let mut dir = codex_dir()?;
dir.push(SESSIONS_SUBDIR); dir.push(SESSIONS_SUBDIR);
fs::create_dir_all(&dir)?; fs::create_dir_all(&dir)?;
// Generate a v4 UUID matches the JS CLI implementation. let timestamp = OffsetDateTime::now_local()
let session_id = Uuid::new_v4(); .map_err(|e| IoError::new(ErrorKind::Other, format!("failed to get local time: {e}")))?;
let timestamp = OffsetDateTime::now_utc();
// Custom format for YYYY-MM-DD. // Custom format for YYYY-MM-DDThh-mm-ss. Use `-` instead of `:` for
let format: &[FormatItem] = format_description!("[year]-[month]-[day]"); // 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 let date_str = timestamp
.format(format) .format(format)
.map_err(|e| IoError::new(ErrorKind::Other, format!("failed to format timestamp: {e}")))?; .map_err(|e| IoError::new(ErrorKind::Other, format!("failed to format timestamp: {e}")))?;

View File

@@ -42,3 +42,4 @@ tracing-subscriber = { version = "0.3.19", features = ["env-filter"] }
tui-input = "0.11.1" tui-input = "0.11.1"
tui-markdown = "0.3.3" tui-markdown = "0.3.3"
tui-textarea = "0.7.0" tui-textarea = "0.7.0"
uuid = { version = "1" }

View File

@@ -102,8 +102,6 @@ impl ChatWidget<'_> {
config, config,
}; };
let _ = chat_widget.submit_welcome_message();
if initial_prompt.is_some() || !initial_images.is_empty() { if initial_prompt.is_some() || !initial_images.is_empty() {
let text = initial_prompt.unwrap_or_default(); let text = initial_prompt.unwrap_or_default();
let _ = chat_widget.submit_user_message_with_images(text, initial_images); 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<AppEvent>> {
self.conversation_history.add_welcome_message(&self.config);
self.request_redraw()?;
Ok(())
}
fn submit_user_message( fn submit_user_message(
&mut self, &mut self,
text: String, text: String,
@@ -215,10 +207,10 @@ impl ChatWidget<'_> {
) -> std::result::Result<(), SendError<AppEvent>> { ) -> std::result::Result<(), SendError<AppEvent>> {
let Event { id, msg } = event; let Event { id, msg } = event;
match msg { match msg {
EventMsg::SessionConfigured { model } => { EventMsg::SessionConfigured(event) => {
// Record session information at the top of the conversation. // Record session information at the top of the conversation.
self.conversation_history self.conversation_history
.add_session_info(&self.config, model); .add_session_info(&self.config, event);
self.request_redraw()?; self.request_redraw()?;
} }
EventMsg::AgentMessage { message } => { EventMsg::AgentMessage { message } => {

View File

@@ -3,6 +3,7 @@ use crate::history_cell::HistoryCell;
use crate::history_cell::PatchEventType; use crate::history_cell::PatchEventType;
use codex_core::config::Config; use codex_core::config::Config;
use codex_core::protocol::FileChange; use codex_core::protocol::FileChange;
use codex_core::protocol::SessionConfiguredEvent;
use crossterm::event::KeyCode; use crossterm::event::KeyCode;
use crossterm::event::KeyEvent; use crossterm::event::KeyEvent;
use ratatui::prelude::*; use ratatui::prelude::*;
@@ -162,8 +163,11 @@ impl ConversationHistoryWidget {
self.scroll_position = usize::MAX; self.scroll_position = usize::MAX;
} }
pub fn add_welcome_message(&mut self, config: &Config) { /// Note `model` could differ from `config.model` if the agent decided to
self.add_to_history(HistoryCell::new_welcome_message(config)); /// 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) { 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)); 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<String>) { pub fn add_active_exec_command(&mut self, call_id: String, command: Vec<String>) {
self.add_to_history(HistoryCell::new_active_exec_command(call_id, command)); self.add_to_history(HistoryCell::new_active_exec_command(call_id, command));
} }

View File

@@ -2,6 +2,7 @@ use codex_ansi_escape::ansi_escape_line;
use codex_common::elapsed::format_duration; use codex_common::elapsed::format_duration;
use codex_core::config::Config; use codex_core::config::Config;
use codex_core::protocol::FileChange; use codex_core::protocol::FileChange;
use codex_core::protocol::SessionConfiguredEvent;
use ratatui::prelude::*; use ratatui::prelude::*;
use ratatui::style::Color; use ratatui::style::Color;
use ratatui::style::Modifier; use ratatui::style::Modifier;
@@ -94,29 +95,50 @@ pub(crate) enum HistoryCell {
const TOOL_CALL_MAX_LINES: usize = 5; const TOOL_CALL_MAX_LINES: usize = 5;
impl HistoryCell { impl HistoryCell {
pub(crate) fn new_welcome_message(config: &Config) -> Self { pub(crate) fn new_session_info(
let mut lines: Vec<Line<'static>> = vec![ config: &Config,
Line::from(vec![ event: SessionConfiguredEvent,
"OpenAI ".into(), is_first_event: bool,
"Codex".bold(), ) -> Self {
" (research preview)".dim(), let SessionConfiguredEvent { model, session_id } = event;
]), if is_first_event {
Line::from(""), let mut lines: Vec<Line<'static>> = vec![
Line::from("codex session:".magenta().bold()), 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![ let entries = vec![
("workdir", config.cwd.display().to_string()), ("workdir", config.cwd.display().to_string()),
("model", config.model.clone()), ("model", config.model.clone()),
("provider", config.model_provider_id.clone()), ("provider", config.model_provider_id.clone()),
("approval", format!("{:?}", config.approval_policy)), ("approval", format!("{:?}", config.approval_policy)),
("sandbox", format!("{:?}", config.sandbox_policy)), ("sandbox", format!("{:?}", config.sandbox_policy)),
]; ];
for (key, value) in entries { for (key, value) in entries {
lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()])); 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 { pub(crate) fn new_user_prompt(message: String) -> Self {
@@ -296,20 +318,6 @@ impl HistoryCell {
HistoryCell::ErrorEvent { lines } 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 filelevel summary of /// Create a new `PendingPatch` cell that lists the filelevel summary of
/// a proposed patch. The summary lines should already be formatted (e.g. /// a proposed patch. The summary lines should already be formatted (e.g.
/// "A path/to/file.rs"). /// "A path/to/file.rs").