From 1bad0a0804adbd6a85c733bf1c3af594c649530e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 5 Jun 2026 14:18:58 +0200 Subject: [PATCH] Simplify SQLite journal mode plumbing --- codex-rs/app-server/src/in_process.rs | 7 ++++--- codex-rs/app-server/src/lib.rs | 2 +- codex-rs/core/src/config/mod.rs | 14 ++++--------- codex-rs/core/src/rollout.rs | 4 ---- codex-rs/core/src/state_db_bridge.rs | 2 +- codex-rs/rollout/src/config.rs | 11 ----------- codex-rs/rollout/src/recorder_tests.rs | 3 ++- codex-rs/rollout/src/state_db.rs | 21 ++++++++++++++------ codex-rs/state/src/runtime.rs | 27 +++++++++++++------------- codex-rs/tui/src/lib.rs | 17 +++++++++------- 10 files changed, 51 insertions(+), 57 deletions(-) diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 448a389851..56cd55ade6 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -763,9 +763,10 @@ mod tests { ) -> InProcessClientHandle { let codex_home = TempDir::new().expect("temp dir"); let config = Arc::new(build_test_config(codex_home.path()).await); - let state_db = codex_rollout::state_db::try_init(config.as_ref()) - .await - .expect("state db should initialize for in-process test"); + let state_db = + codex_rollout::state_db::try_init(config.as_ref(), config.sqlite_journal_mode) + .await + .expect("state db should initialize for in-process test"); let args = InProcessStartArgs { arg0_paths: Arg0DispatchPaths::default(), config, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 0689a79f00..55b973a864 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -531,7 +531,7 @@ pub async fn run_main_with_transport_options( } _ => None, }; - let state_db = match rollout_state_db::try_init(&config).await { + let state_db = match rollout_state_db::try_init(&config, config.sqlite_journal_mode).await { Ok(state_db) => Some(state_db), Err(err) => { return Err(std::io::Error::other(format!( diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 0afa13c50f..526d80fdcd 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -253,15 +253,6 @@ fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option { } } -fn resolve_sqlite_journal_mode( - mode: SqliteJournalModeToml, -) -> codex_state::RuntimeSqliteJournalMode { - match mode { - SqliteJournalModeToml::Wal => codex_state::RuntimeSqliteJournalMode::Wal, - SqliteJournalModeToml::Delete => codex_state::RuntimeSqliteJournalMode::Delete, - } -} - fn resolve_cli_auth_credentials_store_mode( configured: AuthCredentialsStoreMode, package_version: &str, @@ -3293,7 +3284,10 @@ impl Config { .sqlite .as_ref() .and_then(|sqlite| sqlite.journal_mode) - .map(resolve_sqlite_journal_mode) + .map(|mode| match mode { + SqliteJournalModeToml::Wal => codex_state::RuntimeSqliteJournalMode::Wal, + SqliteJournalModeToml::Delete => codex_state::RuntimeSqliteJournalMode::Delete, + }) .unwrap_or_default(); let original_permission_profile = permission_profile.clone(); apply_requirement_constrained_value( diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index f8b7b6c905..73fbe5b5e4 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -32,10 +32,6 @@ impl codex_rollout::RolloutConfigView for Config { self.sqlite_home.as_path() } - fn sqlite_journal_mode(&self) -> codex_state::RuntimeSqliteJournalMode { - self.sqlite_journal_mode - } - fn cwd(&self) -> &std::path::Path { self.cwd.as_path() } diff --git a/codex-rs/core/src/state_db_bridge.rs b/codex-rs/core/src/state_db_bridge.rs index 78d3cb11f9..ee06b65c85 100644 --- a/codex-rs/core/src/state_db_bridge.rs +++ b/codex-rs/core/src/state_db_bridge.rs @@ -4,5 +4,5 @@ pub use codex_rollout::state_db::StateDbHandle; use crate::config::Config; pub async fn init_state_db(config: &Config) -> Option { - rollout_state_db::init(config).await + rollout_state_db::init(config, config.sqlite_journal_mode).await } diff --git a/codex-rs/rollout/src/config.rs b/codex-rs/rollout/src/config.rs index 947e0a4262..420949bfbd 100644 --- a/codex-rs/rollout/src/config.rs +++ b/codex-rs/rollout/src/config.rs @@ -5,9 +5,6 @@ use std::sync::Arc; pub trait RolloutConfigView { fn codex_home(&self) -> &Path; fn sqlite_home(&self) -> &Path; - fn sqlite_journal_mode(&self) -> codex_state::RuntimeSqliteJournalMode { - codex_state::RuntimeSqliteJournalMode::default() - } fn cwd(&self) -> &Path; fn model_provider_id(&self) -> &str; fn generate_memories(&self) -> bool; @@ -67,10 +64,6 @@ impl RolloutConfigView for &T { (*self).sqlite_home() } - fn sqlite_journal_mode(&self) -> codex_state::RuntimeSqliteJournalMode { - (*self).sqlite_journal_mode() - } - fn cwd(&self) -> &Path { (*self).cwd() } @@ -93,10 +86,6 @@ impl RolloutConfigView for Arc { self.as_ref().sqlite_home() } - fn sqlite_journal_mode(&self) -> codex_state::RuntimeSqliteJournalMode { - self.as_ref().sqlite_journal_mode() - } - fn cwd(&self) -> &Path { self.as_ref().cwd() } diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index 07029abe37..3c8ed23e1f 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -127,7 +127,8 @@ async fn state_db_init_backfills_before_returning() -> anyhow::Result<()> { .join("\n"); fs::write(&rollout_path, format!("{jsonl}\n"))?; - let runtime = crate::state_db::init(&test_config(home.path())) + let config = test_config(home.path()); + let runtime = crate::state_db::init(&config, codex_state::RuntimeSqliteJournalMode::default()) .await .expect("state db should initialize"); diff --git a/codex-rs/rollout/src/state_db.rs b/codex-rs/rollout/src/state_db.rs index 6726af1070..9ad714fd80 100644 --- a/codex-rs/rollout/src/state_db.rs +++ b/codex-rs/rollout/src/state_db.rs @@ -38,11 +38,14 @@ const STARTUP_BACKFILL_WAIT_TIMEOUT: Duration = Duration::from_secs(2); /// This is the process entry point for local state: it opens the SQLite-backed /// runtime, applies rollout metadata backfills as needed, and returns the /// initialized handle. -pub async fn init(config: &impl RolloutConfigView) -> Option { +pub async fn init( + config: &impl RolloutConfigView, + sqlite_journal_mode: codex_state::RuntimeSqliteJournalMode, +) -> Option { match try_init_with_roots( config.codex_home().to_path_buf(), config.sqlite_home().to_path_buf(), - config.sqlite_journal_mode(), + sqlite_journal_mode, config.model_provider_id().to_string(), ) .await @@ -59,11 +62,14 @@ pub async fn init(config: &impl RolloutConfigView) -> Option { /// /// Prefer [`init`] unless the caller needs to surface the exact failure after /// tracing or UI setup has completed. -pub async fn try_init(config: &impl RolloutConfigView) -> anyhow::Result { +pub async fn try_init( + config: &impl RolloutConfigView, + sqlite_journal_mode: codex_state::RuntimeSqliteJournalMode, +) -> anyhow::Result { try_init_with_roots( config.codex_home().to_path_buf(), config.sqlite_home().to_path_buf(), - config.sqlite_journal_mode(), + sqlite_journal_mode, config.model_provider_id().to_string(), ) .await @@ -217,7 +223,10 @@ fn emit_startup_warning(message: &str) { /// /// Unlike [`init`], this helper does not run rollout backfill. It is for /// optional local reads from non-owning contexts such as remote app-server mode. -pub async fn get_state_db(config: &impl RolloutConfigView) -> Option { +pub async fn get_state_db( + config: &impl RolloutConfigView, + sqlite_journal_mode: codex_state::RuntimeSqliteJournalMode, +) -> Option { let state_path = codex_state::state_db_path(config.sqlite_home()); if !tokio::fs::try_exists(&state_path).await.unwrap_or(false) { codex_state::record_fallback( @@ -230,7 +239,7 @@ pub async fn get_state_db(config: &impl RolloutConfigView) -> Option SqlxSqliteJournalMode { - match self { - Self::Wal => SqlxSqliteJournalMode::Wal, - Self::Delete => SqlxSqliteJournalMode::Delete, - } - } -} - #[derive(Clone, Copy)] struct RuntimeDbSpec { label: &'static str, @@ -373,11 +364,17 @@ fn base_sqlite_options( path: &Path, journal_mode: RuntimeSqliteJournalMode, ) -> SqliteConnectOptions { + let (journal_mode, synchronous) = match journal_mode { + RuntimeSqliteJournalMode::Wal => (SqlxSqliteJournalMode::Wal, SqliteSynchronous::Normal), + RuntimeSqliteJournalMode::Delete => { + (SqlxSqliteJournalMode::Delete, SqliteSynchronous::Full) + } + }; SqliteConnectOptions::new() .filename(path) .create_if_missing(true) - .journal_mode(journal_mode.to_sqlx()) - .synchronous(SqliteSynchronous::Normal) + .journal_mode(journal_mode) + .synchronous(synchronous) .busy_timeout(Duration::from_secs(5)) .log_statements(LevelFilter::Off) } @@ -706,7 +703,7 @@ mod tests { } #[tokio::test] - async fn open_state_sqlite_honors_delete_journal_mode() { + async fn open_state_sqlite_honors_delete_journal_policy() { let codex_home = unique_temp_dir(); tokio::fs::create_dir_all(&codex_home) .await @@ -726,8 +723,12 @@ mod tests { .fetch_one(&pool) .await .expect("query journal mode"); + let synchronous: i64 = sqlx::query_scalar("PRAGMA synchronous") + .fetch_one(&pool) + .await + .expect("query synchronous mode"); - assert_eq!(journal_mode, "delete"); + assert_eq!((journal_mode, synchronous), ("delete".to_string(), 2)); pool.close().await; let _ = tokio::fs::remove_dir_all(codex_home).await; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index b53ec0f83d..489044f2d6 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -346,14 +346,17 @@ async fn init_state_db_for_app_server_target( app_server_target: &AppServerTarget, ) -> std::io::Result> { match app_server_target { - AppServerTarget::Embedded => state_db::try_init(config).await.map(Some).map_err(|err| { - std::io::Error::other(LocalStateDbStartupError::new( - codex_state::state_db_path(config.sqlite_home.as_path()), - err.to_string(), - )) - }), + AppServerTarget::Embedded => state_db::try_init(config, config.sqlite_journal_mode) + .await + .map(Some) + .map_err(|err| { + std::io::Error::other(LocalStateDbStartupError::new( + codex_state::state_db_path(config.sqlite_home.as_path()), + err.to_string(), + )) + }), AppServerTarget::LocalDaemon { .. } | AppServerTarget::Remote { .. } => { - Ok(state_db::get_state_db(config).await) + Ok(state_db::get_state_db(config, config.sqlite_journal_mode).await) } } }