diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index b6f0b8c85..49b0f9e8e 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -74,6 +74,8 @@ use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::registry::Registry; use tracing_subscriber::util::SubscriberInitExt; +const SQLITE_RECOVERY_CONFIG_WARNING_SUMMARY: &str = "Codex rebuilt its local database."; + mod analytics_utils; mod app_server_tracing; mod attestation; @@ -533,8 +535,8 @@ pub async fn run_main_with_transport_options( } _ => None, }; - let state_db = match rollout_state_db::try_init(&config).await { - Ok(state_db) => Some(state_db), + let state_db_init = match init_sqlite_state_db_with_fresh_start_on_corruption(&config).await { + Ok(state_db_init) => state_db_init, Err(err) => { return Err(std::io::Error::other(format!( "failed to initialize sqlite state runtime under {}: {err}", @@ -542,6 +544,15 @@ pub async fn run_main_with_transport_options( ))); } }; + let state_db = state_db_init.state_db; + if let Some(recovery_notice) = state_db_init.recovery_notice { + config_warnings.push(ConfigWarningNotification { + summary: SQLITE_RECOVERY_CONFIG_WARNING_SUMMARY.to_string(), + details: Some(recovery_notice.details), + path: None, + range: None, + }); + } if should_run_personality_migration { let effective_toml = config.config_layer_stack.effective_config(); @@ -1099,6 +1110,121 @@ pub async fn run_main_with_transport_options( Ok(()) } +struct SqliteRecoveryNotice { + details: String, +} + +struct RecoveredSqliteDatabase { + database_path: String, + backup_folder: String, +} + +struct StateDbInitResult { + state_db: Option, + recovery_notice: Option, +} + +async fn init_sqlite_state_db_with_fresh_start_on_corruption( + config: &Config, +) -> anyhow::Result { + let mut attempted_backups = HashSet::new(); + let mut recovered_databases = Vec::new(); + loop { + let err = match rollout_state_db::try_init(config).await { + Ok(state_db) => { + let recovery_notice = sqlite_recovery_notice(&recovered_databases); + if recovery_notice.is_some() { + emit_state_db_backup_warning(SQLITE_RECOVERY_CONFIG_WARNING_SUMMARY); + for recovered_database in &recovered_databases { + emit_state_db_backup_warning(&format!( + "Database path: {}", + recovered_database.database_path + )); + emit_state_db_backup_warning(&format!( + "Backup folder: {}", + recovered_database.backup_folder + )); + } + } + return Ok(StateDbInitResult { + state_db: Some(state_db), + recovery_notice, + }); + } + Err(err) => err, + }; + if !codex_state::is_sqlite_corruption_error(&err) { + return Err(err); + } + + let database_path = codex_state::runtime_db_path_for_corruption_error(&err) + .unwrap_or_else(|| codex_state::state_db_path(config.sqlite_home.as_path())); + if !attempted_backups.insert(database_path.clone()) { + return Err(anyhow::anyhow!( + "failed to initialize sqlite state runtime after moving damaged database file into a backup folder: {err}" + )); + } + + let original_error = err.to_string(); + emit_state_db_backup_warning(&format!( + "Codex local database at {} appears damaged. Moving it into a backup folder so the app server can rebuild it from saved data.", + database_path.display() + )); + let backups = codex_state::backup_runtime_db_for_fresh_start(database_path.as_path()) + .await + .map_err(|backup_err| { + anyhow::anyhow!( + "failed to move damaged sqlite state database files into a backup folder: {backup_err}; original error: {original_error}" + ) + })?; + for backup in &backups { + emit_state_db_backup_warning(&format!( + "Moved damaged Codex local database file {} to {}", + backup.original_path.display(), + backup.backup_path.display() + )); + } + if let Some(first_backup) = backups.first() + && let Some(backup_folder) = first_backup.backup_path.parent() + { + recovered_databases.push(RecoveredSqliteDatabase { + database_path: first_backup.original_path.display().to_string(), + backup_folder: backup_folder.display().to_string(), + }); + } + } +} + +fn sqlite_recovery_notice( + recovered_databases: &[RecoveredSqliteDatabase], +) -> Option { + if recovered_databases.is_empty() { + return None; + } + + let details = recovered_databases + .iter() + .map(|recovered_database| { + format!( + "Database path: {}\nBackup folder: {}", + recovered_database.database_path, recovered_database.backup_folder + ) + }) + .collect::>() + .join("\n\n"); + Some(SqliteRecoveryNotice { details }) +} + +fn emit_state_db_backup_warning(message: &str) { + warn!("{message}"); + if !tracing::dispatcher::has_been_set() { + #[allow(clippy::print_stderr)] + { + eprintln!("{message}"); + } + } +} + fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransport { match transport { AppServerTransport::Stdio => AppServerRpcTransport::Stdio, diff --git a/codex-rs/cli/src/doctor.rs b/codex-rs/cli/src/doctor.rs index d817f1a19..abb5daf97 100644 --- a/codex-rs/cli/src/doctor.rs +++ b/codex-rs/cli/src/doctor.rs @@ -2141,8 +2141,9 @@ async fn state_check(config: &Config) -> DoctorCheck { }; let mut check = DoctorCheck::new("state.paths", "state", status, summary).details(details); if status == CheckStatus::Fail { - check = check - .remediation("Back up CODEX_HOME, then remove or repair the affected SQLite database."); + check = check.remediation( + "Move the damaged SQLite database aside, then restart the interactive CLI or app server so it can rebuild that runtime database from saved data. Other entry points may not rebuild automatically.", + ); } check } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index b194a5ab1..13acf40d4 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -38,6 +38,7 @@ use codex_utils_cli::ProfileV2Name; use codex_utils_cli::SharedCliOptions; use codex_utils_cli::resume_hint; use owo_colors::OwoColorize; +use std::collections::HashSet; use std::io::IsTerminal; use std::path::PathBuf; use supports_color::Stream; @@ -2188,7 +2189,7 @@ async fn run_interactive_tui( remote_endpoint.clone(), ) }; - let mut attempted_repair = false; + let mut attempted_backups = HashSet::new(); loop { let err = match start_tui().await { Ok(exit_info) => return Ok(exit_info), @@ -2201,25 +2202,25 @@ async fn run_interactive_tui( local_state_db::print_locked_guidance(startup_error); return Ok(AppExitInfo::fatal(startup_error.to_string())); } - if attempted_repair { + if !local_state_db::is_auto_backup_recoverable(startup_error) { local_state_db::print_diagnostic_guidance(startup_error); return Ok(AppExitInfo::fatal(startup_error.to_string())); } - if !local_state_db::confirm_repair(startup_error)? { + if !attempted_backups.insert(startup_error.database_path().to_path_buf()) { local_state_db::print_diagnostic_guidance(startup_error); return Ok(AppExitInfo::fatal(startup_error.to_string())); } - match local_state_db::repair_files(startup_error).await { - Ok(backups) => local_state_db::print_repair_backups(&backups), - Err(repair_err) => { + local_state_db::print_auto_backup_start(startup_error); + match local_state_db::backup_files_for_fresh_start(startup_error).await { + Ok(backups) => local_state_db::confirm_fresh_start_rebuild(startup_error, &backups)?, + Err(backup_err) => { local_state_db::print_diagnostic_guidance(startup_error); return Ok(AppExitInfo::fatal(format!( - "failed to repair Codex local data automatically: {repair_err}" + "failed to move damaged Codex local database files into a backup folder automatically: {backup_err}" ))); } } - attempted_repair = true; } } diff --git a/codex-rs/cli/src/state_db_recovery.rs b/codex-rs/cli/src/state_db_recovery.rs index 7aeffaca3..89bb16643 100644 --- a/codex-rs/cli/src/state_db_recovery.rs +++ b/codex-rs/cli/src/state_db_recovery.rs @@ -1,10 +1,12 @@ -//! CLI recovery for local state database startup failures. +//! CLI handling for local state database startup failures. //! -//! This keeps user-facing repair and lock-contention handling out of the main +//! This keeps user-facing backup and lock-contention handling out of the main //! CLI dispatch path while preserving the TUI startup error as the boundary type. +use codex_state::RuntimeDbBackup; use codex_tui::LocalStateDbStartupError; -use std::path::PathBuf; +use std::io::IsTerminal; +use std::path::Path; pub(crate) fn startup_error(err: &std::io::Error) -> Option<&LocalStateDbStartupError> { err.get_ref() @@ -12,66 +14,60 @@ pub(crate) fn startup_error(err: &std::io::Error) -> Option<&LocalStateDbStartup } pub(crate) fn is_locked(detail: &str) -> bool { - let detail = detail.to_ascii_lowercase(); - detail.contains("database is locked") || detail.contains("database is busy") + codex_state::sqlite_error_detail_is_lock(detail) } -pub(crate) fn confirm_repair(startup_error: &LocalStateDbStartupError) -> std::io::Result { +pub(crate) fn is_corruption(detail: &str) -> bool { + codex_state::sqlite_error_detail_is_corruption(detail) +} + +pub(crate) fn is_auto_backup_recoverable(startup_error: &LocalStateDbStartupError) -> bool { + is_corruption(startup_error.detail()) || sqlite_home_is_blocking_file(startup_error) +} + +fn sqlite_home_is_blocking_file(startup_error: &LocalStateDbStartupError) -> bool { + startup_error + .database_path() + .parent() + .and_then(|path| std::fs::metadata(path).ok()) + .is_some_and(|metadata| metadata.is_file()) +} + +pub(crate) fn print_auto_backup_start(startup_error: &LocalStateDbStartupError) { eprintln!("Codex couldn't start because its local database appears to be damaged."); - eprintln!("Codex can try a safe repair by backing up those files and rebuilding them."); + eprintln!("Moving the damaged local database aside so Codex can rebuild it from saved data."); print_technical_details(startup_error); - crate::confirm("Repair Codex local data now? [y/N]: ") } -pub(crate) async fn repair_files( +pub(crate) async fn backup_files_for_fresh_start( startup_error: &LocalStateDbStartupError, -) -> std::io::Result> { - let state_db_path = startup_error.state_db_path(); - let sqlite_home = state_db_path.parent().ok_or_else(|| { - std::io::Error::other("state database path does not have a parent directory") - })?; - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map_or(0, |duration| duration.as_secs()); - let repair_suffix = format!("codex-repair-{timestamp}"); - let mut backups = Vec::new(); - - match tokio::fs::metadata(sqlite_home).await { - Ok(metadata) if metadata.is_dir() => {} - Ok(_) => { - backups.push(backup_path(sqlite_home, &repair_suffix).await?); - tokio::fs::create_dir_all(sqlite_home).await?; - } - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - tokio::fs::create_dir_all(sqlite_home).await?; - } - Err(err) => return Err(err), - } - - for path in codex_state::runtime_db_paths(sqlite_home) - .into_iter() - .flat_map(|db| sqlite_paths(db.path.as_path())) - { - if tokio::fs::try_exists(path.as_path()).await? { - backups.push(backup_path(path.as_path(), &repair_suffix).await?); - } - } - - if backups.is_empty() { - return Err(std::io::Error::other( - "no repairable Codex local data files were found", - )); - } - - Ok(backups) +) -> std::io::Result> { + codex_state::backup_runtime_db_for_fresh_start(startup_error.database_path()).await } -pub(crate) fn print_repair_backups(backups: &[PathBuf]) { - eprintln!("Backed up Codex local data before repair:"); - for backup in backups { - eprintln!(" {}", backup.display()); +pub(crate) fn confirm_fresh_start_rebuild( + startup_error: &LocalStateDbStartupError, + backups: &[RuntimeDbBackup], +) -> std::io::Result<()> { + eprintln!("Codex rebuilt its local database."); + eprintln!( + "Codex detected a damaged local database, moved it into a backup folder, and will continue startup with a fresh database." + ); + eprintln!("Database path: {}", startup_error.database_path().display()); + if let Some(backup_folder) = backup_folder(backups) { + eprintln!("Backup folder: {}", backup_folder.display()); + } else { + eprintln!("Backup folder: unavailable"); } - eprintln!("Retrying startup with rebuilt local data..."); + + if std::io::stdin().is_terminal() && std::io::stderr().is_terminal() { + eprintln!("Press Enter to continue."); + let mut input = String::new(); + std::io::stdin().read_line(&mut input)?; + } else { + eprintln!("Continuing startup with a fresh local database..."); + } + Ok(()) } pub(crate) fn print_diagnostic_guidance(startup_error: &LocalStateDbStartupError) { @@ -87,79 +83,50 @@ pub(crate) fn print_locked_guidance(startup_error: &LocalStateDbStartupError) { print_technical_details(startup_error); } -fn sqlite_paths(db_path: &std::path::Path) -> Vec { - let mut wal_path = db_path.as_os_str().to_os_string(); - wal_path.push("-wal"); - let mut shm_path = db_path.as_os_str().to_os_string(); - shm_path.push("-shm"); - vec![ - db_path.to_path_buf(), - PathBuf::from(wal_path), - PathBuf::from(shm_path), - ] -} - -async fn backup_path(path: &std::path::Path, repair_suffix: &str) -> std::io::Result { - let file_name = path.file_name().ok_or_else(|| { - std::io::Error::other(format!( - "cannot create a repair backup name for {}", - path.display() - )) - })?; - let mut sequence = 0; - loop { - let mut backup_name = file_name.to_os_string(); - backup_name.push(format!(".{repair_suffix}.{sequence}.bak")); - let backup_path = path.with_file_name(backup_name); - if !tokio::fs::try_exists(backup_path.as_path()).await? { - tokio::fs::rename(path, backup_path.as_path()).await?; - return Ok(backup_path); - } - sequence += 1; - } -} - fn print_technical_details(startup_error: &LocalStateDbStartupError) { eprintln!("Technical details:"); - eprintln!(" Location: {}", startup_error.state_db_path().display()); + eprintln!(" Location: {}", startup_error.database_path().display()); eprintln!(" Cause: {}", startup_error.detail()); } +fn backup_folder(backups: &[RuntimeDbBackup]) -> Option<&Path> { + backups.first()?.backup_path.parent() +} + #[cfg(test)] mod tests { use super::*; use pretty_assertions::assert_eq; + use std::path::PathBuf; use tempfile::TempDir; #[tokio::test] - async fn repair_backs_up_owned_database_files() -> std::io::Result<()> { + async fn backup_backs_up_only_failed_database_file() -> std::io::Result<()> { let temp_dir = TempDir::new()?; let state_path = codex_state::state_db_path(temp_dir.path()); - let logs_path = codex_state::logs_db_path(temp_dir.path()); - let goals_path = codex_state::goals_db_path(temp_dir.path()); - let state_sidecars = sqlite_paths(state_path.as_path()); + let failed_db_path = codex_state::logs_db_path(temp_dir.path()); tokio::fs::write(state_path.as_path(), b"state").await?; - tokio::fs::write(state_sidecars[1].as_path(), b"state-wal").await?; - tokio::fs::write(logs_path.as_path(), b"logs").await?; - tokio::fs::write(goals_path.as_path(), b"goals").await?; + tokio::fs::write(failed_db_path.as_path(), b"logs").await?; let startup_error = - LocalStateDbStartupError::new(state_path.clone(), "corrupt".to_string()); - let backups = repair_files(&startup_error).await?; + LocalStateDbStartupError::new(failed_db_path.clone(), "corrupt".to_string()); + let backups = backup_files_for_fresh_start(&startup_error).await?; - assert_eq!(backups.len(), 4); - assert!(!tokio::fs::try_exists(state_path.as_path()).await?); - assert!(!tokio::fs::try_exists(state_sidecars[1].as_path()).await?); - assert!(!tokio::fs::try_exists(logs_path.as_path()).await?); - assert!(!tokio::fs::try_exists(goals_path.as_path()).await?); - for backup in backups { - assert!(tokio::fs::try_exists(backup.as_path()).await?); - } + assert_eq!( + backups + .iter() + .map(|backup| &backup.original_path) + .collect::>(), + vec![&failed_db_path] + ); + assert!(!tokio::fs::try_exists(failed_db_path.as_path()).await?); + assert!(tokio::fs::try_exists(state_path.as_path()).await?); + assert!(tokio::fs::try_exists(backups[0].backup_path.as_path()).await?); Ok(()) } #[tokio::test] - async fn repair_replaces_blocking_sqlite_home_file() -> std::io::Result<()> { + async fn backup_replaces_blocking_sqlite_home_file() -> std::io::Result<()> { let temp_dir = TempDir::new()?; let sqlite_home = temp_dir.path().join("sqlite-home"); tokio::fs::write(sqlite_home.as_path(), b"not-a-directory").await?; @@ -168,18 +135,25 @@ mod tests { "File exists".to_string(), ); - let backups = repair_files(&startup_error).await?; + assert!(is_auto_backup_recoverable(&startup_error)); + let backups = backup_files_for_fresh_start(&startup_error).await?; assert_eq!(backups.len(), 1); assert!(tokio::fs::metadata(sqlite_home.as_path()).await?.is_dir()); - assert!(tokio::fs::try_exists(backups[0].as_path()).await?); + assert!(tokio::fs::try_exists(backups[0].backup_path.as_path()).await?); Ok(()) } #[test] - fn lock_failures_skip_repair() { - assert!(is_locked("database is locked")); - assert!(is_locked("database is busy")); - assert!(!is_locked("database disk image is malformed")); + fn backup_folder_uses_parent_of_first_backup_path() { + let backups = vec![RuntimeDbBackup { + original_path: PathBuf::from("/tmp/state_5.sqlite"), + backup_path: PathBuf::from("/tmp/db-backups/sqlite-1-0/state_5.sqlite"), + }]; + + assert_eq!( + backup_folder(&backups), + Some(Path::new("/tmp/db-backups/sqlite-1-0")) + ); } } diff --git a/codex-rs/rollout/src/state_db.rs b/codex-rs/rollout/src/state_db.rs index fa2905dd4..c21d7002b 100644 --- a/codex-rs/rollout/src/state_db.rs +++ b/codex-rs/rollout/src/state_db.rs @@ -5,6 +5,7 @@ use crate::list::SortDirection; use crate::list::ThreadSortKey; use crate::metadata; use crate::sqlite_metrics; +use anyhow::Context; use chrono::DateTime; use chrono::Utc; use codex_protocol::ThreadId; @@ -50,7 +51,7 @@ pub async fn init(config: &impl RolloutConfigView) -> Option { { Ok(runtime) => Some(runtime), Err(err) => { - emit_startup_warning(&format!("failed to initialize state runtime: {err}")); + emit_startup_warning(&format!("failed to initialize state runtime: {err:#}")); None } } @@ -109,9 +110,9 @@ async fn try_init_with_roots_inner( let runtime = codex_state::StateRuntime::init(sqlite_home.clone(), default_model_provider_id.clone()) .await - .map_err(|err| { - anyhow::anyhow!( - "failed to initialize state runtime at {}: {err}", + .with_context(|| { + format!( + "failed to initialize state runtime at {}", sqlite_home.display() ) })?; @@ -128,7 +129,10 @@ async fn try_init_with_roots_inner( backfill_gate_started.elapsed(), &backfill_gate_result, ); - backfill_gate_result?; + if let Err(err) = backfill_gate_result { + runtime.close().await; + return Err(err); + } Ok(runtime) } diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index cb8c711e6..264b737e9 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -57,15 +57,21 @@ pub use runtime::GoalStore; pub use runtime::GoalUpdate; pub use runtime::MemoryStore; pub use runtime::RemoteControlEnrollmentRecord; +pub use runtime::RuntimeDbBackup; pub use runtime::RuntimeDbPath; pub use runtime::ThreadFilterOptions; +pub use runtime::backup_runtime_db_for_fresh_start; pub use runtime::goals_db_filename; pub use runtime::goals_db_path; +pub use runtime::is_sqlite_corruption_error; pub use runtime::logs_db_filename; pub use runtime::logs_db_path; pub use runtime::memories_db_filename; pub use runtime::memories_db_path; +pub use runtime::runtime_db_path_for_corruption_error; pub use runtime::runtime_db_paths; +pub use runtime::sqlite_error_detail_is_corruption; +pub use runtime::sqlite_error_detail_is_lock; pub use runtime::sqlite_integrity_check; pub use runtime::state_db_filename; pub use runtime::state_db_path; diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 5ae41bcd8..2937bf0c2 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -62,6 +62,7 @@ mod backfill; mod goals; mod logs; mod memories; +mod recovery; mod remote_control; #[cfg(test)] mod test_support; @@ -72,6 +73,12 @@ pub use goals::GoalAccountingOutcome; pub use goals::GoalStore; pub use goals::GoalUpdate; pub use memories::MemoryStore; +pub use recovery::RuntimeDbBackup; +pub use recovery::backup_runtime_db_for_fresh_start; +pub use recovery::is_sqlite_corruption_error; +pub use recovery::runtime_db_path_for_corruption_error; +pub use recovery::sqlite_error_detail_is_corruption; +pub use recovery::sqlite_error_detail_is_lock; pub use remote_control::RemoteControlEnrollmentRecord; pub use threads::ThreadFilterOptions; @@ -200,6 +207,7 @@ impl StateRuntime { Ok(db) => Arc::new(db), Err(err) => { warn!("failed to open logs db at {}: {err}", logs_path.display()); + close_sqlite_pools(&[pool.as_ref()]).await; return Err(err); } }; @@ -208,6 +216,7 @@ impl StateRuntime { Ok(db) => Arc::new(db), Err(err) => { warn!("failed to open goals db at {}: {err}", goals_path.display()); + close_sqlite_pools(&[pool.as_ref(), logs_pool.as_ref()]).await; return Err(err); } }; @@ -224,6 +233,7 @@ impl StateRuntime { "failed to open memories db at {}: {err}", memories_path.display() ); + close_sqlite_pools(&[pool.as_ref(), logs_pool.as_ref(), goals_pool.as_ref()]).await; return Err(err); } }; @@ -236,7 +246,16 @@ impl StateRuntime { started.elapsed(), &backfill_state_result, ); - backfill_state_result?; + if let Err(err) = backfill_state_result { + close_sqlite_pools(&[ + pool.as_ref(), + logs_pool.as_ref(), + goals_pool.as_ref(), + memories_pool.as_ref(), + ]) + .await; + return Err(err); + } let started = Instant::now(); let thread_updated_at_millis_result: anyhow::Result> = sqlx::query_scalar("SELECT MAX(threads.updated_at_ms) FROM threads") @@ -250,7 +269,19 @@ impl StateRuntime { started.elapsed(), &thread_updated_at_millis_result, ); - let thread_updated_at_millis = thread_updated_at_millis_result?; + let thread_updated_at_millis = match thread_updated_at_millis_result { + Ok(value) => value, + Err(err) => { + close_sqlite_pools(&[ + pool.as_ref(), + logs_pool.as_ref(), + goals_pool.as_ref(), + memories_pool.as_ref(), + ]) + .await; + return Err(err); + } + }; let thread_updated_at_millis = thread_updated_at_millis.unwrap_or(0); let runtime = Arc::new(Self { thread_goals: GoalStore::new(Arc::clone(&goals_pool)), @@ -283,6 +314,14 @@ impl StateRuntime { &self.memories } + /// Close all SQLite pools and wait for outstanding pool workers to exit. + pub async fn close(&self) { + self.memories.close().await; + self.thread_goals.close().await; + self.logs_pool.close().await; + self.pool.close().await; + } + pub async fn clear_memory_data_in_sqlite_home(sqlite_home: &Path) -> anyhow::Result { let memories_path = MEMORIES_DB.path(sqlite_home); if !tokio::fs::try_exists(&memories_path).await? { @@ -302,6 +341,12 @@ impl StateRuntime { } } +async fn close_sqlite_pools(pools: &[&SqlitePool]) { + for pool in pools { + pool.close().await; + } +} + fn base_sqlite_options(path: &Path) -> SqliteConnectOptions { SqliteConnectOptions::new() .filename(path) @@ -367,7 +412,8 @@ async fn open_sqlite( started.elapsed(), &pool_result, ); - let pool = pool_result?; + let pool = pool_result + .map_err(|source| recovery::RuntimeDbInitError::new(spec.label, "open", path, source))?; let started = Instant::now(); let migrate_result = migrator.run(&pool).await.map_err(anyhow::Error::from); crate::telemetry::record_init_result( @@ -377,7 +423,10 @@ async fn open_sqlite( started.elapsed(), &migrate_result, ); - migrate_result?; + if let Err(source) = migrate_result { + pool.close().await; + return Err(recovery::RuntimeDbInitError::new(spec.label, "migrate", path, source).into()); + } Ok(pool) } diff --git a/codex-rs/state/src/runtime/goals.rs b/codex-rs/state/src/runtime/goals.rs index d0fab47d0..e5769ccd1 100644 --- a/codex-rs/state/src/runtime/goals.rs +++ b/codex-rs/state/src/runtime/goals.rs @@ -11,6 +11,10 @@ impl GoalStore { pub(crate) fn new(pool: Arc) -> Self { Self { pool } } + + pub(crate) async fn close(&self) { + self.pool.close().await; + } } pub struct GoalUpdate { diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index 310150079..895c25435 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -35,6 +35,10 @@ impl MemoryStore { Self { pool, state_pool } } + pub(crate) async fn close(&self) { + self.pool.close().await; + } + /// Deletes all persisted memory state in one transaction. /// /// This removes every `stage1_outputs` row and all `jobs` rows for the diff --git a/codex-rs/state/src/runtime/recovery.rs b/codex-rs/state/src/runtime/recovery.rs new file mode 100644 index 000000000..d9cf0bcdc --- /dev/null +++ b/codex-rs/state/src/runtime/recovery.rs @@ -0,0 +1,243 @@ +//! Backup-and-rebuild support for Codex runtime SQLite databases. +//! +//! Codex keeps several independent runtime SQLite databases under one SQLite +//! home. When SQLite reports that one of them is corrupt, automatic recovery +//! moves only that database file and its sidecars into a backup folder so the +//! other databases keep their data. + +use std::borrow::Cow; +use std::path::Path; +use std::path::PathBuf; + +const BACKUP_DIR_NAME: &str = "db-backups"; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct RuntimeDbBackup { + /// Path where the runtime database or sidecar lived before it was moved. + pub original_path: PathBuf, + /// Path where the runtime database or sidecar was backed up. + pub backup_path: PathBuf, +} + +#[derive(Debug)] +pub(crate) struct RuntimeDbInitError { + label: &'static str, + operation: &'static str, + path: PathBuf, + source: anyhow::Error, +} + +impl RuntimeDbInitError { + pub(crate) fn new( + label: &'static str, + operation: &'static str, + path: &Path, + source: anyhow::Error, + ) -> Self { + Self { + label, + operation, + path: path.to_path_buf(), + source, + } + } + + fn path(&self) -> &Path { + self.path.as_path() + } +} + +impl std::fmt::Display for RuntimeDbInitError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "failed to {} {} at {}: {}", + self.operation, + self.label, + self.path.display(), + self.source + ) + } +} + +impl std::error::Error for RuntimeDbInitError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(self.source.as_ref()) + } +} + +/// Move one Codex runtime SQLite database out of the way so that database can +/// be recreated without discarding unrelated runtime databases. +pub async fn backup_runtime_db_for_fresh_start( + db_path: &Path, +) -> std::io::Result> { + let sqlite_home = db_path.parent().ok_or_else(|| { + std::io::Error::other(format!( + "database path does not have a parent directory: {}", + db_path.display() + )) + })?; + match tokio::fs::metadata(sqlite_home).await { + Ok(metadata) if metadata.is_dir() => backup_runtime_db_files(db_path).await, + Ok(_) => backup_blocking_sqlite_home(sqlite_home).await, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + tokio::fs::create_dir_all(sqlite_home).await?; + Err(std::io::Error::other(format!( + "no Codex runtime database files were found to back up for {}", + db_path.display() + ))) + } + Err(err) => Err(err), + } +} + +pub fn runtime_db_path_for_corruption_error(err: &anyhow::Error) -> Option { + if !is_sqlite_corruption_error(err) { + return None; + } + err.chain() + .find_map(|source| source.downcast_ref::()) + .map(|err| err.path().to_path_buf()) +} + +pub fn is_sqlite_corruption_error(err: &anyhow::Error) -> bool { + err.chain().any(sqlite_error_source_is_corruption) +} + +fn sqlite_error_source_is_corruption(source: &(dyn std::error::Error + 'static)) -> bool { + let Some(err) = source.downcast_ref::() else { + return false; + }; + let sqlx::Error::Database(database_error) = err else { + return false; + }; + sqlite_error_detail_is_corruption(database_error.message()) + || database_error + .code() + .is_some_and(sqlite_database_code_is_corruption) +} + +fn sqlite_database_code_is_corruption(code: Cow<'_, str>) -> bool { + matches!( + code.as_ref().to_ascii_lowercase().as_str(), + "11" | "26" | "sqlite_corrupt" | "sqlite_notadb" + ) +} + +pub fn sqlite_error_detail_is_corruption(detail: &str) -> bool { + let detail = detail.to_ascii_lowercase(); + detail.contains("database disk image is malformed") + || detail.contains("database schema is malformed") + || detail.contains("database is corrupt") + || detail.contains("file is not a database") + || detail.contains("sqlite_corrupt") + || detail.contains("sqlite_notadb") + || detail.contains("(code: 11)") + || detail.contains("(code: 26)") +} + +pub fn sqlite_error_detail_is_lock(detail: &str) -> bool { + let detail = detail.to_ascii_lowercase(); + detail.contains("database is locked") || detail.contains("database is busy") +} + +async fn backup_runtime_db_files(db_path: &Path) -> std::io::Result> { + let sqlite_home = db_path.parent().ok_or_else(|| { + std::io::Error::other(format!( + "database path does not have a parent directory: {}", + db_path.display() + )) + })?; + backup_sqlite_paths(sqlite_home, sqlite_paths(db_path)).await +} + +async fn backup_sqlite_paths( + sqlite_home: &Path, + paths: impl IntoIterator, +) -> std::io::Result> { + let backup_dir = create_unique_backup_dir(sqlite_home.join(BACKUP_DIR_NAME).as_path()).await?; + let mut backups = Vec::new(); + + for path in paths { + if tokio::fs::try_exists(path.as_path()).await? { + let backup_path = backup_dir.join(file_name(path.as_path())?); + tokio::fs::rename(path.as_path(), backup_path.as_path()).await?; + backups.push(RuntimeDbBackup { + original_path: path, + backup_path, + }); + } + } + + if backups.is_empty() { + let _ = tokio::fs::remove_dir(backup_dir).await; + return Err(std::io::Error::other( + "no Codex runtime database files were found to back up", + )); + } + + Ok(backups) +} + +async fn backup_blocking_sqlite_home(sqlite_home: &Path) -> std::io::Result> { + let parent = sqlite_home.parent().ok_or_else(|| { + std::io::Error::other(format!( + "cannot create a backup folder for {}", + sqlite_home.display() + )) + })?; + let mut backup_dir_name = file_name(sqlite_home)?.to_os_string(); + backup_dir_name.push(format!(".{BACKUP_DIR_NAME}")); + let backup_parent = parent.join(backup_dir_name); + let backup_dir = create_unique_backup_dir(backup_parent.as_path()).await?; + let backup_path = backup_dir.join(file_name(sqlite_home)?); + tokio::fs::rename(sqlite_home, backup_path.as_path()).await?; + tokio::fs::create_dir_all(sqlite_home).await?; + Ok(vec![RuntimeDbBackup { + original_path: sqlite_home.to_path_buf(), + backup_path, + }]) +} + +fn sqlite_paths(db_path: &Path) -> Vec { + let mut wal_path = db_path.as_os_str().to_os_string(); + wal_path.push("-wal"); + let mut shm_path = db_path.as_os_str().to_os_string(); + shm_path.push("-shm"); + vec![ + db_path.to_path_buf(), + PathBuf::from(wal_path), + PathBuf::from(shm_path), + ] +} + +async fn create_unique_backup_dir(backup_parent: &Path) -> std::io::Result { + tokio::fs::create_dir_all(backup_parent).await?; + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |duration| duration.as_secs()); + let mut sequence = 0_u32; + loop { + let backup_dir = backup_parent.join(format!("sqlite-{timestamp}-{sequence}")); + match tokio::fs::create_dir(backup_dir.as_path()).await { + Ok(()) => return Ok(backup_dir), + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { + sequence += 1; + } + Err(err) => return Err(err), + } + } +} + +fn file_name(path: &Path) -> std::io::Result<&std::ffi::OsStr> { + path.file_name().ok_or_else(|| { + std::io::Error::other(format!( + "cannot create a backup name for {}", + path.display() + )) + }) +} + +#[cfg(test)] +#[path = "recovery_tests.rs"] +mod tests; diff --git a/codex-rs/state/src/runtime/recovery_tests.rs b/codex-rs/state/src/runtime/recovery_tests.rs new file mode 100644 index 000000000..7424dbcfe --- /dev/null +++ b/codex-rs/state/src/runtime/recovery_tests.rs @@ -0,0 +1,105 @@ +use super::*; +use crate::runtime::test_support::unique_temp_dir; +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn backup_moves_only_requested_runtime_db_files_to_backup_folder() -> std::io::Result<()> { + let sqlite_home = unique_temp_dir(); + tokio::fs::create_dir_all(sqlite_home.as_path()).await?; + let runtime_paths = super::super::runtime_db_paths(sqlite_home.as_path()); + let mut expected_paths = Vec::new(); + for db_path in runtime_paths.iter().map(|db| db.path.as_path()) { + for path in sqlite_paths(db_path) { + tokio::fs::write(path.as_path(), path.display().to_string()).await?; + expected_paths.push(path); + } + } + let failed_db_path = super::super::logs_db_path(sqlite_home.as_path()); + let failed_paths = sqlite_paths(failed_db_path.as_path()); + + let backups = backup_runtime_db_for_fresh_start(failed_db_path.as_path()).await?; + + assert_eq!(backups.len(), failed_paths.len()); + for path in &failed_paths { + assert!(!tokio::fs::try_exists(path.as_path()).await?); + } + for path in expected_paths + .iter() + .filter(|path| !failed_paths.contains(path)) + { + assert!(tokio::fs::try_exists(path.as_path()).await?); + } + for backup in backups { + assert!( + backup + .backup_path + .starts_with(sqlite_home.join(BACKUP_DIR_NAME)) + ); + assert!(tokio::fs::try_exists(backup.backup_path.as_path()).await?); + } + Ok(()) +} + +#[tokio::test] +async fn backup_replaces_blocking_sqlite_home_file() -> std::io::Result<()> { + let temp_dir = unique_temp_dir(); + tokio::fs::create_dir_all(temp_dir.as_path()).await?; + let sqlite_home = temp_dir.join("sqlite-home"); + tokio::fs::write(sqlite_home.as_path(), b"not-a-directory").await?; + + let backups = backup_runtime_db_for_fresh_start( + super::super::state_db_path(sqlite_home.as_path()).as_path(), + ) + .await?; + + assert_eq!(backups.len(), 1); + assert!(tokio::fs::metadata(sqlite_home.as_path()).await?.is_dir()); + assert!( + backups[0] + .backup_path + .starts_with(temp_dir.join(format!("sqlite-home.{BACKUP_DIR_NAME}"))) + ); + assert!(tokio::fs::try_exists(backups[0].backup_path.as_path()).await?); + Ok(()) +} + +#[test] +fn sqlite_error_detail_classifies_corruption_and_lock_errors() { + assert!(sqlite_error_detail_is_corruption("file is not a database")); + assert!(sqlite_error_detail_is_corruption( + "error returned from database: (code: 11) database disk image is malformed" + )); + assert!(!sqlite_error_detail_is_corruption("database is locked")); + assert!(sqlite_error_detail_is_lock("database is locked")); + assert!(sqlite_error_detail_is_lock("database is busy")); +} + +#[tokio::test] +async fn runtime_db_path_for_corruption_error_returns_failed_database_path() -> std::io::Result<()> +{ + let sqlite_home = unique_temp_dir(); + tokio::fs::create_dir_all(sqlite_home.as_path()).await?; + let path = super::super::state_db_path(sqlite_home.as_path()); + tokio::fs::write(path.as_path(), b"not sqlite").await?; + + let err = match super::super::StateRuntime::init(sqlite_home, "openai".to_string()).await { + Ok(_) => panic!("malformed sqlite should fail to initialize"), + Err(err) => err, + }; + + assert_eq!(runtime_db_path_for_corruption_error(&err), Some(path)); + Ok(()) +} + +#[test] +fn runtime_db_path_for_corruption_error_ignores_corrupt_word_in_path() { + let path = PathBuf::from("/tmp/sqlite_corrupt/state_5.sqlite"); + let err = anyhow::Error::new(RuntimeDbInitError::new( + "state DB", + "open", + path.as_path(), + anyhow::anyhow!("permission denied"), + )); + + assert_eq!(runtime_db_path_for_corruption_error(&err), None); +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 281e4a52a..4c0918387 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -352,9 +352,11 @@ async fn init_state_db_for_app_server_target( ) -> std::io::Result> { match app_server_target { AppServerTarget::Embedded => state_db::try_init(config).await.map(Some).map_err(|err| { + let database_path = codex_state::runtime_db_path_for_corruption_error(&err) + .unwrap_or_else(|| codex_state::state_db_path(config.sqlite_home.as_path())); std::io::Error::other(LocalStateDbStartupError::new( - codex_state::state_db_path(config.sqlite_home.as_path()), - err.to_string(), + database_path, + format!("{err:#}"), )) }), AppServerTarget::LocalDaemon { .. } | AppServerTarget::Remote { .. } => { @@ -2990,6 +2992,37 @@ mod tests { ); Ok(()) } + + #[tokio::test] + async fn embedded_state_db_corruption_preserves_failed_database_for_cli_recovery() + -> color_eyre::Result<()> { + let temp_dir = TempDir::new()?; + let mut config = build_config(&temp_dir).await?; + let sqlite_home = temp_dir.path().join("sqlite-home"); + std::fs::create_dir_all(&sqlite_home)?; + let logs_db_path = codex_state::logs_db_path(&sqlite_home); + std::fs::write(&logs_db_path, "not a sqlite database")?; + config.sqlite_home = sqlite_home; + + let err = + match init_state_db_for_app_server_target(&config, &AppServerTarget::Embedded).await { + Ok(_) => panic!("embedded startup should surface state db init failures"), + Err(err) => err, + }; + let startup_error = err + .get_ref() + .and_then(|err| err.downcast_ref::()) + .expect("state db startup failure should retain its typed context"); + + assert_eq!(startup_error.database_path(), logs_db_path.as_path()); + assert!( + codex_state::sqlite_error_detail_is_corruption(startup_error.detail()), + "startup error should preserve the SQLite corruption cause, got: {}", + startup_error.detail() + ); + Ok(()) + } + #[tokio::test] #[serial] async fn windows_shows_trust_prompt_with_sandbox() -> std::io::Result<()> { diff --git a/codex-rs/tui/src/startup_error.rs b/codex-rs/tui/src/startup_error.rs index 55b691d83..585cec4b2 100644 --- a/codex-rs/tui/src/startup_error.rs +++ b/codex-rs/tui/src/startup_error.rs @@ -3,24 +3,28 @@ use std::path::PathBuf; #[derive(Debug, thiserror::Error)] #[error( - "failed to initialize sqlite state db at {}: {detail}", - state_db_path.display() + "failed to initialize sqlite local db at {}: {detail}", + database_path.display() )] pub struct LocalStateDbStartupError { - state_db_path: PathBuf, + database_path: PathBuf, detail: String, } impl LocalStateDbStartupError { - pub fn new(state_db_path: PathBuf, detail: String) -> Self { + pub fn new(database_path: PathBuf, detail: String) -> Self { Self { - state_db_path, + database_path, detail, } } + pub fn database_path(&self) -> &Path { + self.database_path.as_path() + } + pub fn state_db_path(&self) -> &Path { - self.state_db_path.as_path() + self.database_path() } pub fn detail(&self) -> &str {