From 4fe5518b2fdf553769c6fd324752df6a51ccff5e Mon Sep 17 00:00:00 2001 From: Job Chong Date: Thu, 27 Nov 2025 14:55:11 +0800 Subject: [PATCH 1/2] chore: improve rollout session init errors --- codex-rs/core/src/codex.rs | 5 +- codex-rs/core/src/rollout/error.rs | 162 +++++++++++++++++++++++++++++ codex-rs/core/src/rollout/mod.rs | 2 + 3 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 codex-rs/core/src/rollout/error.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index e3c5de5762..4b4daf8f19 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -102,6 +102,7 @@ use crate::protocol::TurnDiffEvent; use crate::protocol::WarningEvent; use crate::rollout::RolloutRecorder; use crate::rollout::RolloutRecorderParams; +use crate::rollout::map_session_init_error; use crate::shell; use crate::state::ActiveTurn; use crate::state::SessionServices; @@ -206,7 +207,7 @@ impl Codex { .await .map_err(|e| { error!("Failed to create session: {e:#}"); - CodexErr::InternalAgentDied + map_session_init_error(&e, &config.codex_home) })?; let conversation_id = session.conversation_id; @@ -508,7 +509,7 @@ impl Session { let rollout_recorder = rollout_recorder.map_err(|e| { error!("failed to initialize rollout recorder: {e:#}"); - anyhow::anyhow!("failed to initialize rollout recorder: {e:#}") + anyhow::Error::from(e) })?; let rollout_path = rollout_recorder.rollout_path.clone(); diff --git a/codex-rs/core/src/rollout/error.rs b/codex-rs/core/src/rollout/error.rs new file mode 100644 index 0000000000..138196fef2 --- /dev/null +++ b/codex-rs/core/src/rollout/error.rs @@ -0,0 +1,162 @@ +use std::io::ErrorKind; +use std::path::Path; + +use crate::error::CodexErr; +use crate::rollout::SESSIONS_SUBDIR; + +pub(crate) fn map_session_init_error(err: &anyhow::Error, codex_home: &Path) -> CodexErr { + if let Some(mapped) = err + .chain() + .filter_map(|cause| cause.downcast_ref::()) + .find_map(|io_err| map_rollout_io_error(io_err, codex_home)) + { + return mapped; + } + + CodexErr::Fatal(format!("Failed to initialize session: {err:#}")) +} + +fn map_rollout_io_error(io_err: &std::io::Error, codex_home: &Path) -> Option { + let sessions_dir = codex_home.join(SESSIONS_SUBDIR); + let hint = match io_err.kind() { + ErrorKind::PermissionDenied => format!( + "Codex cannot access session files at {} (permission denied). If sessions were created using sudo, fix ownership: sudo chown -R $(whoami) {}", + sessions_dir.display(), + codex_home.display() + ), + ErrorKind::NotFound => format!( + "Session storage missing at {}. Create the directory or choose a different Codex home.", + sessions_dir.display() + ), + ErrorKind::AlreadyExists => format!( + "Session storage path {} is blocked by an existing file. Remove or rename it so Codex can create sessions.", + sessions_dir.display() + ), + ErrorKind::InvalidData | ErrorKind::InvalidInput => format!( + "Session data under {} looks corrupt or unreadable. Clearing the sessions directory may help (this will remove saved conversations).", + sessions_dir.display() + ), + ErrorKind::IsADirectory | ErrorKind::NotADirectory => format!( + "Session storage path {} has an unexpected type. Ensure it is a directory Codex can use for session files.", + sessions_dir.display() + ), + _ => return None, + }; + + Some(CodexErr::Fatal(format!( + "{hint} (underlying error: {io_err})" + ))) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::rollout::SESSIONS_SUBDIR; + use std::path::PathBuf; + + fn io_error(kind: ErrorKind, msg: &str) -> anyhow::Error { + anyhow::Error::new(std::io::Error::new(kind, msg)) + } + + #[test] + fn startup_errors_propagate_context() { + let err = anyhow::anyhow!("rollout directory missing"); + let codex_home = PathBuf::from("/tmp/codex-home"); + let mapped = map_session_init_error(&err, &codex_home); + + match mapped { + CodexErr::Fatal(msg) => { + assert!( + msg.contains("Failed to initialize session"), + "expected startup prefix: {msg}" + ); + assert!( + msg.contains("rollout directory missing"), + "expected underlying cause: {msg}" + ); + } + other => panic!("expected fatal error mapping, got {other:?}"), + } + } + + #[test] + fn permission_denied_rollout_errors_surface_actionable_message() { + let err = io_error(ErrorKind::PermissionDenied, "no access"); + let codex_home = PathBuf::from("/tmp/codex-home"); + let mapped = map_session_init_error(&err, &codex_home); + let sessions_dir = codex_home.join(SESSIONS_SUBDIR); + + match mapped { + CodexErr::Fatal(msg) => { + let msg_lower = msg.to_lowercase(); + assert!( + msg.contains(&sessions_dir.display().to_string()), + "expected session path in message: {msg}" + ); + assert!( + msg_lower.contains("permission denied"), + "expected permission hint in message: {msg}" + ); + assert!( + msg.contains(&codex_home.display().to_string()), + "expected codex home in message: {msg}" + ); + } + other => panic!("expected fatal error mapping, got {other:?}"), + } + } + + #[test] + fn not_found_rollout_errors_explain_missing_sessions_dir() { + let err = io_error(ErrorKind::NotFound, "missing dir"); + let codex_home = PathBuf::from("/tmp/codex-home"); + let mapped = map_session_init_error(&err, &codex_home); + let sessions_dir = codex_home.join(SESSIONS_SUBDIR); + + match mapped { + CodexErr::Fatal(msg) => { + let msg_lower = msg.to_lowercase(); + assert!( + msg.contains(&sessions_dir.display().to_string()), + "expected session path in message: {msg}" + ); + assert!( + msg_lower.contains("missing") || msg_lower.contains("not found"), + "expected not-found hint in message: {msg}" + ); + assert!( + msg.contains("missing dir"), + "expected underlying cause in message: {msg}" + ); + } + other => panic!("expected fatal error mapping, got {other:?}"), + } + } + + #[test] + fn invalid_rollout_files_surface_corruption_hint() { + let err = io_error(ErrorKind::InvalidData, "bad json"); + let codex_home = PathBuf::from("/tmp/codex-home"); + let mapped = map_session_init_error(&err, &codex_home); + let sessions_dir = codex_home.join(SESSIONS_SUBDIR); + + match mapped { + CodexErr::Fatal(msg) => { + let msg_lower = msg.to_lowercase(); + assert!( + msg_lower.contains("corrupt") || msg_lower.contains("invalid"), + "expected corruption hint in message: {msg}" + ); + assert!( + msg.contains("bad json"), + "expected underlying cause in message: {msg}" + ); + assert!( + msg.contains(&sessions_dir.display().to_string()), + "expected session path in message: {msg}" + ); + } + other => panic!("expected fatal error mapping, got {other:?}"), + } + } +} diff --git a/codex-rs/core/src/rollout/mod.rs b/codex-rs/core/src/rollout/mod.rs index 23410bd4d0..540d204be3 100644 --- a/codex-rs/core/src/rollout/mod.rs +++ b/codex-rs/core/src/rollout/mod.rs @@ -7,11 +7,13 @@ pub const ARCHIVED_SESSIONS_SUBDIR: &str = "archived_sessions"; pub const INTERACTIVE_SESSION_SOURCES: &[SessionSource] = &[SessionSource::Cli, SessionSource::VSCode]; +pub(crate) mod error; pub mod list; pub(crate) mod policy; pub mod recorder; pub use codex_protocol::protocol::SessionMeta; +pub(crate) use error::map_session_init_error; pub use list::find_conversation_path_by_id_str; pub use recorder::RolloutRecorder; pub use recorder::RolloutRecorderParams; From e4bdf4478c2d04e80325132aec7fd9e1a1ea7530 Mon Sep 17 00:00:00 2001 From: Job Chong Date: Thu, 27 Nov 2025 15:37:55 +0800 Subject: [PATCH 2/2] Remove rollout error mapping tests --- codex-rs/core/src/rollout/error.rs | 113 ----------------------------- 1 file changed, 113 deletions(-) diff --git a/codex-rs/core/src/rollout/error.rs b/codex-rs/core/src/rollout/error.rs index 138196fef2..e924dd2d28 100644 --- a/codex-rs/core/src/rollout/error.rs +++ b/codex-rs/core/src/rollout/error.rs @@ -47,116 +47,3 @@ fn map_rollout_io_error(io_err: &std::io::Error, codex_home: &Path) -> Option anyhow::Error { - anyhow::Error::new(std::io::Error::new(kind, msg)) - } - - #[test] - fn startup_errors_propagate_context() { - let err = anyhow::anyhow!("rollout directory missing"); - let codex_home = PathBuf::from("/tmp/codex-home"); - let mapped = map_session_init_error(&err, &codex_home); - - match mapped { - CodexErr::Fatal(msg) => { - assert!( - msg.contains("Failed to initialize session"), - "expected startup prefix: {msg}" - ); - assert!( - msg.contains("rollout directory missing"), - "expected underlying cause: {msg}" - ); - } - other => panic!("expected fatal error mapping, got {other:?}"), - } - } - - #[test] - fn permission_denied_rollout_errors_surface_actionable_message() { - let err = io_error(ErrorKind::PermissionDenied, "no access"); - let codex_home = PathBuf::from("/tmp/codex-home"); - let mapped = map_session_init_error(&err, &codex_home); - let sessions_dir = codex_home.join(SESSIONS_SUBDIR); - - match mapped { - CodexErr::Fatal(msg) => { - let msg_lower = msg.to_lowercase(); - assert!( - msg.contains(&sessions_dir.display().to_string()), - "expected session path in message: {msg}" - ); - assert!( - msg_lower.contains("permission denied"), - "expected permission hint in message: {msg}" - ); - assert!( - msg.contains(&codex_home.display().to_string()), - "expected codex home in message: {msg}" - ); - } - other => panic!("expected fatal error mapping, got {other:?}"), - } - } - - #[test] - fn not_found_rollout_errors_explain_missing_sessions_dir() { - let err = io_error(ErrorKind::NotFound, "missing dir"); - let codex_home = PathBuf::from("/tmp/codex-home"); - let mapped = map_session_init_error(&err, &codex_home); - let sessions_dir = codex_home.join(SESSIONS_SUBDIR); - - match mapped { - CodexErr::Fatal(msg) => { - let msg_lower = msg.to_lowercase(); - assert!( - msg.contains(&sessions_dir.display().to_string()), - "expected session path in message: {msg}" - ); - assert!( - msg_lower.contains("missing") || msg_lower.contains("not found"), - "expected not-found hint in message: {msg}" - ); - assert!( - msg.contains("missing dir"), - "expected underlying cause in message: {msg}" - ); - } - other => panic!("expected fatal error mapping, got {other:?}"), - } - } - - #[test] - fn invalid_rollout_files_surface_corruption_hint() { - let err = io_error(ErrorKind::InvalidData, "bad json"); - let codex_home = PathBuf::from("/tmp/codex-home"); - let mapped = map_session_init_error(&err, &codex_home); - let sessions_dir = codex_home.join(SESSIONS_SUBDIR); - - match mapped { - CodexErr::Fatal(msg) => { - let msg_lower = msg.to_lowercase(); - assert!( - msg_lower.contains("corrupt") || msg_lower.contains("invalid"), - "expected corruption hint in message: {msg}" - ); - assert!( - msg.contains("bad json"), - "expected underlying cause in message: {msg}" - ); - assert!( - msg.contains(&sessions_dir.display().to_string()), - "expected session path in message: {msg}" - ); - } - other => panic!("expected fatal error mapping, got {other:?}"), - } - } -}