diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 03d0e63e62..8c5b03a95e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -714,8 +714,8 @@ impl Session { // session only contains its own SessionMeta written by the recorder. let filtered: Vec<_> = rollout_items .iter() + .filter(|&item| !matches!(item, RolloutItem::SessionMeta(_))) .cloned() - .filter(|item| !matches!(item, RolloutItem::SessionMeta(_))) .collect(); if !filtered.is_empty() { self.persist_rollout_items(&filtered).await; diff --git a/codex-rs/core/src/codex_conversation.rs b/codex-rs/core/src/codex_conversation.rs index 8ebf30819b..6e220a0118 100644 --- a/codex-rs/core/src/codex_conversation.rs +++ b/codex-rs/core/src/codex_conversation.rs @@ -41,11 +41,11 @@ impl CodexConversation { self.rollout_path.clone() } - pub async fn flush_rollout(&self) -> std::io::Result<()> { - self.session.flush_rollout().await + pub async fn flush_rollout(&self) -> CodexResult<()> { + Ok(self.session.flush_rollout().await?) } - pub async fn set_session_name(&self, name: Option) -> std::io::Result<()> { - self.session.set_session_name(name).await + pub async fn set_session_name(&self, name: Option) -> CodexResult<()> { + Ok(self.session.set_session_name(name).await?) } } diff --git a/codex-rs/core/src/saved_sessions.rs b/codex-rs/core/src/saved_sessions.rs index ed6f294691..eb85019d6b 100644 --- a/codex-rs/core/src/saved_sessions.rs +++ b/codex-rs/core/src/saved_sessions.rs @@ -1,3 +1,4 @@ +use crate::error::Result; use crate::rollout::list::read_head_for_summary; use codex_protocol::ConversationId; use codex_protocol::protocol::SessionMetaLine; @@ -37,16 +38,19 @@ fn saved_sessions_path(codex_home: &Path) -> PathBuf { codex_home.join("saved_sessions.json") } -async fn load_saved_sessions_file(path: &Path) -> std::io::Result { +async fn load_saved_sessions_file(path: &Path) -> Result { match tokio::fs::read_to_string(path).await { - Ok(text) => serde_json::from_str(&text) - .map_err(|e| IoError::other(format!("failed to parse saved sessions: {e}"))), + Ok(text) => { + let parsed = serde_json::from_str(&text) + .map_err(|e| IoError::other(format!("failed to parse saved sessions: {e}")))?; + Ok(parsed) + } Err(err) if err.kind() == ErrorKind::NotFound => Ok(SavedSessionsFile::default()), - Err(err) => Err(err), + Err(err) => Err(err.into()), } } -async fn write_saved_sessions_file(path: &Path, file: &SavedSessionsFile) -> std::io::Result<()> { +async fn write_saved_sessions_file(path: &Path, file: &SavedSessionsFile) -> Result<()> { if let Some(parent) = path.parent() { tokio::fs::create_dir_all(parent).await?; } @@ -54,7 +58,8 @@ async fn write_saved_sessions_file(path: &Path, file: &SavedSessionsFile) -> std .map_err(|e| IoError::other(format!("failed to serialize saved sessions: {e}")))?; let tmp_path = path.with_extension("json.tmp"); tokio::fs::write(&tmp_path, json).await?; - tokio::fs::rename(tmp_path, path).await + tokio::fs::rename(tmp_path, path).await?; + Ok(()) } /// Create a new entry from the rollout's SessionMeta line. @@ -62,7 +67,7 @@ pub async fn build_saved_session_entry( name: String, rollout_path: PathBuf, model: String, -) -> std::io::Result { +) -> Result { let head = read_head_for_summary(&rollout_path).await?; let first = head.first().ok_or_else(|| { IoError::other(format!( @@ -94,10 +99,7 @@ pub async fn build_saved_session_entry( } /// Insert or replace a saved session entry in `saved_sessions.json`. -pub async fn upsert_saved_session( - codex_home: &Path, - entry: SavedSessionEntry, -) -> std::io::Result<()> { +pub async fn upsert_saved_session(codex_home: &Path, entry: SavedSessionEntry) -> Result<()> { let path = saved_sessions_path(codex_home); let mut file = load_saved_sessions_file(&path).await?; file.entries.insert(entry.name.clone(), entry); @@ -108,14 +110,14 @@ pub async fn upsert_saved_session( pub async fn resolve_saved_session( codex_home: &Path, name: &str, -) -> std::io::Result> { +) -> Result> { let path = saved_sessions_path(codex_home); let file = load_saved_sessions_file(&path).await?; Ok(file.entries.get(name).cloned()) } /// Return all saved sessions ordered by newest `saved_at` first. -pub async fn list_saved_sessions(codex_home: &Path) -> std::io::Result> { +pub async fn list_saved_sessions(codex_home: &Path) -> Result> { let path = saved_sessions_path(codex_home); let file = load_saved_sessions_file(&path).await?; let mut entries: Vec = file.entries.values().cloned().collect(); diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 3b70d17124..ede195de01 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -7,6 +7,8 @@ use crate::diff_render::DiffSummary; use crate::exec_command::strip_bash_lc_and_escape; use crate::file_search::FileSearchManager; use crate::history_cell::HistoryCell; +#[cfg(not(debug_assertions))] +use crate::history_cell::UpdateAvailableHistoryCell; use crate::model_migration::ModelMigrationOutcome; use crate::model_migration::migration_copy_for_config; use crate::model_migration::run_model_migration_prompt; @@ -25,9 +27,11 @@ use codex_common::model_presets::ModelUpgrade; use codex_common::model_presets::all_model_presets; use codex_core::AuthManager; use codex_core::ConversationManager; +use codex_core::SavedSessionEntry; use codex_core::build_saved_session_entry; use codex_core::config::Config; use codex_core::config::edit::ConfigEditsBuilder; +use codex_core::error::CodexErr; #[cfg(target_os = "windows")] use codex_core::features::Feature; use codex_core::model_family::find_family_for_model; @@ -55,9 +59,6 @@ use std::time::Duration; use tokio::select; use tokio::sync::mpsc::unbounded_channel; -#[cfg(not(debug_assertions))] -use crate::history_cell::UpdateAvailableHistoryCell; - const GPT_5_1_MIGRATION_AUTH_MODES: [AuthMode; 2] = [AuthMode::ChatGPT, AuthMode::ApiKey]; const ARCTICFOX_MIGRATION_AUTH_MODES: [AuthMode; 1] = [AuthMode::ChatGPT]; @@ -440,93 +441,68 @@ impl App { } async fn save_session(&mut self, requested_name: String) { - let name = requested_name.trim().to_string(); + let name = requested_name.clone().trim(); if name.is_empty() { self.chat_widget - .add_error_message("Usage: /save ".to_string()); - return; + .add_error_message("Usage: /save ".to_string()) } - let Some(conversation_id) = self.chat_widget.conversation_id() else { - self.chat_widget.add_error_message( - "Session is not ready yet; try /save again in a moment.".to_string(), - ); - return; - }; - let Some(rollout_path) = self.chat_widget.rollout_path() else { - self.chat_widget.add_error_message( - "Rollout path is not available yet; try /save again shortly.".to_string(), - ); - return; - }; - let conversation = match self.server.get_conversation(conversation_id).await { - Ok(conv) => conv, - Err(err) => { - tracing::error!( - conversation_id = %conversation_id, - error = %err, - "failed to look up conversation for /save" + match self.try_save_session(requested_name).await { + Ok(entry) => { + let name = &entry.name; + self.chat_widget.add_info_message( + format!( + "Saved session '{name}' (conversation {}).", entry.conversation_id + ), + Some(format!( + "Resume with `codex resume {name}` or fork with `codex fork {name}`.", + )), ); - self.chat_widget - .add_error_message(format!("Failed to save session '{name}': {err}")); - return; } - }; - if let Err(err) = conversation.flush_rollout().await { - tracing::error!( - conversation_id = %conversation_id, - error = %err, - "failed to flush rollout before /save" - ); - self.chat_widget - .add_error_message(format!("Failed to save session '{name}': {err}")); - return; + Err(error) => self + .chat_widget + .add_error_message(format!("Failed to save session '{name}': {error}")), } - if let Err(err) = conversation.set_session_name(Some(name.clone())).await { - tracing::error!( - conversation_id = %conversation_id, - error = %err, - "failed to update session name before /save" - ); - self.chat_widget - .add_error_message(format!("Failed to save session '{name}': {err}")); - return; + } + + async fn try_save_session( + &mut self, + requested_name: String, + ) -> Result { + // Normalize and validate the user-provided name early so downstream async work + // only runs for actionable requests. + let name = requested_name.trim(); + if name.is_empty() { + return Err("Usage: /save ".into()); } + + // Capture identifiers from the active chat widget; these are cheap and fast. + let conversation_id = self + .chat_widget + .conversation_id() + .ok_or_else(|| "Session is not ready yet; try /save again in a moment.".to_string())?; + let rollout_path = self.chat_widget.rollout_path().ok_or_else(|| { + "Rollout path is not available yet; try /save again shortly.".to_string() + })?; + + // Resolve the conversation handle; all subsequent operations use this shared reference. + let conversation = self.server.get_conversation(conversation_id).await?; + + // Ensure the rollout is fully flushed before snapshotting metadata. + conversation.flush_rollout().await?; + + // Persist the human-friendly name into the SessionMeta line. + conversation + .set_session_name(Some(name.to_string())) + .await?; + + // Build and persist the saved-session entry on disk. let model = self.chat_widget.config_ref().model.clone(); - let entry = match build_saved_session_entry(name.clone(), rollout_path.clone(), model).await - { - Ok(entry) => entry, - Err(err) => { - tracing::error!( - rollout = %rollout_path.display(), - error = %err, - "failed to build saved session entry" - ); - self.chat_widget - .add_error_message(format!("Failed to save session '{name}': {err}")); - return; - } - }; - if let Err(err) = upsert_saved_session(&self.config.codex_home, entry.clone()).await { - tracing::error!( - rollout = %rollout_path.display(), - error = %err, - "failed to persist saved session entry" - ); - self.chat_widget - .add_error_message(format!("Failed to save session '{name}': {err}")); - return; - } - let hint = format!( - "Resume with `codex resume {}` or fork with `codex fork {}`.", - entry.name, entry.name - ); - self.chat_widget.add_info_message( - format!( - "Saved session '{}' (conversation {}).", - entry.name, entry.conversation_id - ), - Some(hint), - ); + let entry = + build_saved_session_entry(name.to_string(), rollout_path.clone(), model).await?; + + upsert_saved_session(&self.config.codex_home, entry.clone()).await?; + + Ok(entry) } async fn handle_event(&mut self, tui: &mut tui::Tui, event: AppEvent) -> Result {