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
This commit is contained in:
Ian MacLeod
2026-10-06 23:19:16 +00:00
committed by copyberry
parent 044da98b4a
commit eb2d3c6416
6 changed files with 55 additions and 28 deletions
+4 -3
View File
@@ -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() {
+20 -1
View File
@@ -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<()> {
+8 -5
View File
@@ -53,7 +53,7 @@ impl App {
pub(super) async fn load_new_session_config(
&mut self,
app_server: &AppServerSession,
) -> Result<Config> {
) -> 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))
}
}
+20 -16
View File
@@ -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<Option<crate::local_settings::LocalSettings>, 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)
}
}
+2 -2
View File
@@ -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();
@@ -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()