From eb2d3c6416e0db91b462f1853445145fceb43d89 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Tue, 6 Oct 2026 22:03:36 +0000 Subject: [PATCH] Preserve live TUI settings when configuration reloads fail (#51510) ## Why Live preferences can be newer than the last successful configuration load. Starting a new thread after a reload failure could overwrite those preferences with stale settings. ## What changed Carry local settings alongside the new-session configuration, retaining the live settings when reloading fails and applying the staged settings when the new thread starts. During resume and agent selection, replace staged local settings only when the directory-trust check actually reloads configuration. ## Testing Add a regression test that changes a live rendering preference, writes invalid `config.toml`, and verifies that loading a new thread's configuration preserves the live local settings. GitOrigin-RevId: 4ac3c70de6d627e1609cc0a5d4ba5df5a61a8b20 --- codex-rs/tui/src/app/agents_overview.rs | 7 ++-- codex-rs/tui/src/app/config_persistence.rs | 21 ++++++++++- codex-rs/tui/src/app/new_session.rs | 13 ++++--- codex-rs/tui/src/app/resume_config.rs | 36 ++++++++++--------- codex-rs/tui/src/app/session_lifecycle.rs | 4 +-- .../tui/src/app/tests/projectless_tests.rs | 2 +- 6 files changed, 55 insertions(+), 28 deletions(-) diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 41fe62b3bd..f72a9caaa9 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -566,7 +566,7 @@ impl App { } }; if !unloaded && started.is_none() { - if let Err(control) = self + match self .confirm_directory_trust( tui, app_server, @@ -580,9 +580,10 @@ impl App { ) .await { - return Ok(control); + Ok(Some(reloaded)) => local_settings = reloaded, + Ok(None) => {} + Err(control) => return Ok(control), } - local_settings = self.local_settings.reloaded(&resume_config); } // Folder selection and trust prompts can replace or clear the loading frame. if startup_draft.is_none() { diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index f9a2d59f01..5e8bf63ef3 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -1826,7 +1826,7 @@ enabled = false .await; app.refresh_in_memory_config_from_disk().await?; - let new_config = app.load_new_session_config(&app_server).await?; + let (new_config, _) = app.load_new_session_config(&app_server).await?; let permission_config = app .rebuild_config_for_permission_profile(":workspace") .await?; @@ -1991,6 +1991,25 @@ theme = "dracula" Ok(()) } + #[tokio::test] + async fn new_thread_keeps_live_settings_after_failed_reload() -> Result<()> { + let mut app = make_test_app().await; + let server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let home = tempdir()?; + app.config.codex_home = home.path().to_path_buf().abs(); + let config_path = home.path().join("config.toml"); + std::fs::write(&config_path, "[tui]\ntheme = 'dracula'\n")?; + app.refresh_in_memory_config_from_disk().await?; + // A live setting can be newer than the last successful disk load. + app.local_settings.tui.rendering.math = false; + std::fs::write(config_path, "[broken")?; + + let (_, settings) = app.load_new_session_config(&server).await?; + assert_eq!(settings, app.local_settings); + server.shutdown().await?; + Ok(()) + } + #[tokio::test] async fn rebuild_config_for_resume_or_fallback_uses_current_config_on_same_cwd_error() -> Result<()> { diff --git a/codex-rs/tui/src/app/new_session.rs b/codex-rs/tui/src/app/new_session.rs index 6e72d34b67..ffc00b9754 100644 --- a/codex-rs/tui/src/app/new_session.rs +++ b/codex-rs/tui/src/app/new_session.rs @@ -53,7 +53,7 @@ impl App { pub(super) async fn load_new_session_config( &mut self, app_server: &AppServerSession, - ) -> Result { + ) -> Result<(Config, crate::local_settings::LocalSettings)> { let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); let defaults_cwd = match app_server.thread_params_mode() { crate::app_server_session::ThreadParamsMode::Embedded => cwd.as_path(), @@ -67,11 +67,14 @@ impl App { ) .await?; // Stage local preferences and permission carryover without changing the active task. - let mut config = match self.rebuild_config_for_cwd(cwd).await { - Ok(config) => config, + let (mut config, local_settings) = match self.rebuild_config_for_cwd(cwd).await { + Ok(config) => { + let settings = self.local_settings.reloaded(&config); + (config, settings) + } Err(err) => { tracing::warn!(%err, "failed to refresh local settings before a new thread"); - self.config.clone() + (self.config.clone(), self.local_settings.clone()) } }; if let Some(defaults) = defaults.as_ref() { @@ -93,6 +96,6 @@ impl App { &self.harness_overrides, ); } - Ok(config) + Ok((config, local_settings)) } } diff --git a/codex-rs/tui/src/app/resume_config.rs b/codex-rs/tui/src/app/resume_config.rs index c2ab1a9f84..30add17ec1 100644 --- a/codex-rs/tui/src/app/resume_config.rs +++ b/codex-rs/tui/src/app/resume_config.rs @@ -130,19 +130,22 @@ impl App { None }; let trust_cwd = resume_config.0.cwd.to_path_buf(); - self.confirm_directory_trust( - tui, - app_server, - &mut resume_config.0, - &trust_cwd, - crate::onboarding::DirectoryTrustOptions { - resumed_thread: resumed_thread.as_ref(), - ..Default::default() - }, - /*startup_draft*/ None, - ) - .await?; - resume_config.1 = self.local_settings.reloaded(&resume_config.0); + if let Some(local_settings) = self + .confirm_directory_trust( + tui, + app_server, + &mut resume_config.0, + &trust_cwd, + crate::onboarding::DirectoryTrustOptions { + resumed_thread: resumed_thread.as_ref(), + ..Default::default() + }, + /*startup_draft*/ None, + ) + .await? + { + resume_config.1 = local_settings; + } Ok(resume_config) } @@ -154,12 +157,12 @@ impl App { cwd: &Path, options: crate::onboarding::DirectoryTrustOptions<'_>, mut startup_draft: Option<&mut StartupDraftPump>, - ) -> std::result::Result<(), AppRunControl> { + ) -> std::result::Result, AppRunControl> { // Keep the existing explicit remote --cd gate, including retries after cancellation. // Other remote destinations await authoritative trust-root metadata. let cwd = if self.app_server_target.uses_remote_workspace() { let Some(cwd) = app_server.remote_cwd_override() else { - return Ok(()); + return Ok(None); }; cwd } else { @@ -261,7 +264,8 @@ impl App { } } } + return Ok(Some(self.local_settings.reloaded(config))); } - Ok(()) + Ok(None) } } diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index e8cc5b4afa..61550ab90b 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -988,7 +988,7 @@ impl App { // Start a fresh in-memory session while preserving resumability via persisted rollout // history. If an initial message is provided, `enqueue_primary_thread_session` suppresses it // until the new session is configured and any replayed turns have been rendered. - let mut config = match self.load_new_session_config(app_server).await { + let (mut config, local_settings) = match self.load_new_session_config(app_server).await { Ok(config) => config, Err(err) => { if let Some(message) = initial_user_message { @@ -1034,7 +1034,7 @@ impl App { Some(started.session.thread_id), ) .await; - self.local_settings = self.local_settings.reloaded(&config); + self.local_settings = local_settings; self.refresh_server_version_overview_notice(CODEX_CLI_VERSION); self.config = config; self.remember_launch_permissions(); diff --git a/codex-rs/tui/src/app/tests/projectless_tests.rs b/codex-rs/tui/src/app/tests/projectless_tests.rs index bef8b88536..d85d8a0b56 100644 --- a/codex-rs/tui/src/app/tests/projectless_tests.rs +++ b/codex-rs/tui/src/app/tests/projectless_tests.rs @@ -163,7 +163,7 @@ sandbox = "unelevated" }) .await??; response.single_request(); - let next = app.load_new_session_config(&server).await?; + let (next, _) = app.load_new_session_config(&server).await?; assert_eq!( next.permissions.permission_profile(), &PermissionProfile::workspace_write()