Preserve TUI sessions while starting replacement threads (#42671)

## Why

Starting a replacement thread can fail while reading configuration or calling
`thread/start`. The active thread and any composed input should remain available
when that happens. Remote app servers also need new threads to use their own
effective defaults instead of stale client-side model settings.

## What changed

- Read effective server configuration before starting a replacement thread,
  while preserving explicit model and reasoning-effort launch settings.
- Keep the current thread, configuration, subscriptions, and composer input
  intact until `thread/start` succeeds.
- Fall back to local defaults when an older server does not support
  `config/read`.

## Testing

Added coverage for server and managed defaults, explicit launch overrides,
remote working directories, older servers, and failures during configuration
reads or thread startup.

GitOrigin-RevId: 507a918ea2bf79c9f0adbb189bc611ecf57b757a
This commit is contained in:
Eric Traut
2026-09-04 04:45:44 +00:00
committed by copyberry
parent ea2046f36d
commit b995d06050
5 changed files with 396 additions and 16 deletions
+1
View File
@@ -226,6 +226,7 @@ mod input;
mod loaded_threads;
mod misalignment_policy;
mod model_defaults;
mod new_session;
mod pending_interactive_replay;
mod permission_shortcuts;
mod pets;
+80
View File
@@ -0,0 +1,80 @@
//! New-session configuration from server defaults and explicit launch settings.
//!
//! Build the replacement configuration without replacing the active task's settings.
use super::*;
use codex_config::ConfigLayerSource;
impl App {
pub(super) async fn load_new_session_config(
&mut self,
app_server: &AppServerSession,
) -> Result<Config> {
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(),
crate::app_server_session::ThreadParamsMode::Remote => {
app_server.remote_cwd_override().unwrap_or(Path::new("."))
}
};
// config/read resolves relative paths on the server. With no remote launch override,
// "." uses the same server process directory as thread/start's omitted cwd.
let defaults = match crate::config_update::read_effective_config(
app_server.request_handle(),
defaults_cwd.display().to_string(),
)
.await
{
Ok(response) => Some(response.config),
Err(err)
if matches!(
err.downcast_ref::<TypedRequestError>(),
Some(TypedRequestError::Server { source, .. })
if source.code == -32601
|| source.code == -32600
&& source.message.contains("config/read")
&& (source.message.contains("unknown variant")
|| source.message.contains("unknown method"))
) =>
{
// Older servers can still start threads using the existing local defaults.
None
}
Err(err) => return Err(err),
};
// 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,
Err(err) => {
tracing::warn!(%err, "failed to refresh local settings before a new thread");
self.config.clone()
}
};
self.apply_runtime_policy_overrides(&mut config, RuntimePolicyOverrideScope::All);
config.service_tier = self.chat_widget.configured_service_tier();
if let Some(defaults) = defaults {
// A remote server cannot resolve this invocation's explicitly selected local profile.
let has_launch_setting = |key: &str| {
self.cli_kv_overrides.iter().any(|(path, _)| path == key)
|| config.config_layer_stack.layers_high_to_low().any(|layer| {
layer.disabled_reason.is_none()
&& matches!(
layer.name,
ConfigLayerSource::User {
profile: Some(_),
..
}
)
&& layer.config.get(key).is_some()
})
};
if self.harness_overrides.model.is_none() && !has_launch_setting("model") {
config.model = defaults.model;
}
if !has_launch_setting("model_reasoning_effort") {
config.model_reasoning_effort = defaults.model_reasoning_effort;
}
}
Ok(config)
}
}
+26 -14
View File
@@ -831,10 +831,18 @@ 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.
self.refresh_in_memory_config_from_disk_best_effort("starting a new thread")
.await;
let model = self.chat_widget.current_model().to_string();
let mut config = self.fresh_session_config();
let mut config = match self.load_new_session_config(app_server).await {
Ok(config) => config,
Err(err) => {
if let Some(message) = initial_user_message {
self.chat_widget.restore_user_message_to_composer(message);
}
self.chat_widget
.add_error_message(format!("Failed to read new session defaults: {err}"));
tui.frame_requester().schedule_frame();
return;
}
};
apply_managed_new_thread_defaults(
&mut config,
app_server.managed_new_thread_defaults(),
@@ -847,15 +855,6 @@ impl App {
self.chat_widget.thread_name(),
self.chat_widget.rollout_path().as_deref(),
);
self.shutdown_current_thread(app_server).await;
let tracked_thread_ids: Vec<ThreadId> =
self.thread_event_channels.keys().copied().collect();
for thread_id in tracked_thread_ids {
if let Err(err) = app_server.thread_unsubscribe(thread_id).await {
tracing::warn!("failed to unsubscribe tracked thread {thread_id}: {err}");
}
}
self.config = config.clone();
match app_server
.start_thread_with_session_start_source(
&config,
@@ -865,6 +864,17 @@ impl App {
.await
{
Ok(mut started) => {
self.shutdown_current_thread(app_server).await;
let tracked_thread_ids: Vec<ThreadId> =
self.thread_event_channels.keys().copied().collect();
for thread_id in tracked_thread_ids {
if let Err(err) = app_server.thread_unsubscribe(thread_id).await {
tracing::warn!("failed to unsubscribe tracked thread {thread_id}: {err}");
}
}
self.local_settings = crate::local_settings::LocalSettings::from(&config);
self.config = config;
let name_error = if let Some(name) = new_thread_name {
match app_server
.thread_set_name(started.session.thread_id, name.clone())
@@ -913,7 +923,9 @@ impl App {
self.chat_widget.add_error_message(format!(
"Failed to start a fresh session through the app server: {err}"
));
self.config.model = Some(model);
if let Some(message) = initial_user_message {
self.chat_widget.restore_user_message_to_composer(message);
}
}
}
tui.frame_requester().schedule_frame();
@@ -0,0 +1,248 @@
//! Replacement starts use server defaults and preserve the current task on failure.
use super::*;
use pretty_assertions::assert_eq;
#[tokio::test]
async fn replacement_uses_server_defaults_and_preserves_explicit_launch_settings() -> Result<()> {
for (managed_effort, explicit, expected_model, expected_effort) in [
(None, "saved", "server-model", "high"),
(Some("medium"), "model", "cli-model", "high"),
(Some("medium"), "effort", "server-model", "low"),
(None, "profile_model", "profile-model", "high"),
(None, "profile_effort", "server-model", "low"),
(Some(""), "profile", "managed-model", "low"),
(Some("medium"), "profile", "managed-model", "medium"),
] {
let (mut app, _events, _ops) = make_test_app_with_channels().await;
let server_home = tempdir()?;
let client_home = tempdir()?;
std::fs::write(
server_home.path().join("work.config.toml"),
"model = \"server-model\"\nmodel_reasoning_effort = \"high\"\n",
)?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\n",
)?;
if let Some(effort) = managed_effort {
let effort = if effort.is_empty() {
String::new()
} else {
format!("model_reasoning_effort = {effort:?}\n")
};
std::fs::write(
server_home.path().join("requirements.toml"),
format!("[models.new_thread]\nmodel = \"managed-model\"\n{effort}"),
)?;
}
let mut server_config = app.config.clone();
server_config.codex_home = server_home.path().to_path_buf().abs();
server_config.sqlite = SqliteConfig::new_for_testing(server_home.path().abs());
let (mut server, requests, proxy) = start_recording_app_server_with_history(
&server_config,
HistoryCapabilities::Current,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Remote,
LoaderOverrides {
user_config_path: Some(server_home.path().join("work.config.toml").abs()),
user_config_profile: Some("work".parse()?),
system_requirements_path: Some(server_home.path().join("requirements.toml")),
..LoaderOverrides::default()
},
)
.await?;
server = server.with_remote_cwd_override(Some(server_config.cwd.to_path_buf()));
server.bootstrap(&server_config).await?;
app.config.codex_home = client_home.path().to_path_buf().abs();
app.config.sqlite = SqliteConfig::new_for_testing(client_home.path().abs());
app.chat_widget
.handle_thread_session_quiet(test_thread_session(
ThreadId::new(),
client_home.path().to_path_buf(),
));
match explicit {
"model" => app.harness_overrides.model = Some("cli-model".to_string()),
"effort" => app.cli_kv_overrides.push((
"model_reasoning_effort".to_string(),
TomlValue::String("low".to_string()),
)),
profile @ ("profile" | "profile_model" | "profile_effort") => {
let path = client_home.path().join("work.config.toml");
std::fs::write(
&path,
match profile {
"profile_model" => "model = \"profile-model\"\n",
"profile_effort" => "model_reasoning_effort = \"low\"\n",
_ => "model = \"profile-model\"\nmodel_reasoning_effort = \"low\"\n",
},
)?;
app.loader_overrides.user_config_path = Some(path.abs());
app.loader_overrides.user_config_profile = Some("work".parse()?);
}
_ => {}
}
let mut tui = crate::tui::test_support::make_test_tui()?;
app.start_fresh_session_with_summary_hint(
&mut tui,
&mut server,
/*session_start_source*/ None,
/*initial_user_message*/ None,
/*new_thread_name*/ None,
)
.await;
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1);
assert_eq!(
(
&starts[0]["model"],
&starts[0]["config"]["model_reasoning_effort"]
),
(
&serde_json::json!(expected_model),
&serde_json::json!(expected_effort)
),
);
assert_eq!(
recorded_params(&requests, "config/read"),
vec![serde_json::json!({"cwd": server_config.cwd.display().to_string()})],
);
if explicit == "saved" {
let rendered = render_bottom_popup(&app.chat_widget, /*width*/ 80)
.replace(&server_config.cwd.display().to_string(), "<PROJECT>");
insta::assert_snapshot!(rendered, @r"
› Ask Codex to do anything
server-model high · <PROJECT>
");
}
server.shutdown().await?;
proxy.await??;
}
Ok(())
}
#[tokio::test]
async fn replacement_failure_keeps_current_task_and_restores_input() -> Result<()> {
for fail_read in [false, true] {
let (mut app, mut events, _ops) = make_test_app_with_channels().await;
let home = tempdir()?;
app.config.codex_home = home.path().to_path_buf().abs();
app.config.sqlite = SqliteConfig::new_for_testing(home.path().abs());
let current = ThreadId::new();
app.enqueue_primary_thread_session(
test_thread_session(current, app.config.cwd.to_path_buf()),
Vec::new(),
)
.await?;
while events.try_recv().is_ok() {}
let original_config = app.config.clone();
let (mut server, requests, proxy) = start_recording_app_server_with_history(
&app.config,
HistoryCapabilities::ThreadStartFails,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
if fail_read {
std::fs::write(home.path().join("config.toml"), "invalid = [")?;
}
let mut tui = crate::tui::test_support::make_test_tui()?;
app.start_fresh_session_with_summary_hint(
&mut tui,
&mut server,
/*session_start_source*/ None,
Some("keep this request".into()),
/*new_thread_name*/ None,
)
.await;
assert_eq!(app.chat_widget.thread_id(), Some(current));
assert_eq!(app.active_thread_id, Some(current));
assert!(app.active_thread_rx.is_some());
assert_eq!(app.config, original_config);
assert_eq!(
app.chat_widget.composer_text_with_pending(),
"keep this request"
);
assert!(recorded_params(&requests, "thread/unsubscribe").is_empty());
assert_eq!(
recorded_params(&requests, "thread/start").len(),
usize::from(!fail_read)
);
let errors = std::iter::from_fn(|| events.try_recv().ok())
.filter_map(|event| match event {
AppEvent::InsertHistoryCell(cell) => {
Some(lines_to_single_string(&cell.display_lines(/*width*/ 1000)))
}
_ => None,
})
.collect::<Vec<_>>()
.join("\n");
if fail_read {
insta::assert_snapshot!(errors, @"■ Failed to read new session defaults: config/read failed in TUI");
} else {
insta::assert_snapshot!(errors, @"■ Failed to start a fresh session through the app server: thread/start failed during TUI bootstrap: thread/start failed: replacement unavailable (code -32603)");
}
server.shutdown().await?;
proxy.await??;
}
Ok(())
}
#[tokio::test]
async fn replacement_preserves_remote_launch_paths_and_older_servers() -> Result<()> {
for (remote_cwd, capabilities) in [
(None, HistoryCapabilities::Current),
(Some(PathBuf::from(".")), HistoryCapabilities::Current),
(None, HistoryCapabilities::ConfigReadUnsupported(-32601)),
(None, HistoryCapabilities::ConfigReadUnsupported(-32600)),
] {
let (mut app, _events, _ops) = make_test_app_with_channels().await;
let home = tempdir()?;
app.config.codex_home = home.path().to_path_buf().abs();
app.config.sqlite = SqliteConfig::new_for_testing(home.path().abs());
std::fs::write(
home.path().join("config.toml"),
"model = \"configured-model\"\n",
)?;
let (server, requests, proxy) = start_recording_app_server_with_history(
&app.config,
capabilities,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Remote,
LoaderOverrides::default(),
)
.await?;
let mut server = server.with_remote_cwd_override(remote_cwd.clone());
let mut tui = crate::tui::test_support::make_test_tui()?;
app.start_fresh_session_with_summary_hint(
&mut tui,
&mut server,
/*session_start_source*/ None,
/*initial_user_message*/ None,
/*new_thread_name*/ None,
)
.await;
assert_eq!(
recorded_params(&requests, "config/read"),
vec![serde_json::json!({"cwd": "."}),]
);
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1);
assert_eq!(
(&starts[0]["cwd"], &starts[0]["model"]),
(
&serde_json::json!(remote_cwd),
&serde_json::json!("configured-model")
),
);
assert_eq!(app.chat_widget.current_model(), "configured-model");
server.shutdown().await?;
proxy.await??;
}
Ok(())
}
@@ -49,6 +49,8 @@ enum HistoryCapabilities {
LegacyDynamicToolsAndHistory,
ForkHydrationFails,
ThreadListFails,
ThreadStartFails,
ConfigReadUnsupported(i64),
}
/// Returns and resets `(thread/loaded/list, thread/read)` request counts.
@@ -78,6 +80,7 @@ pub(super) async fn start_recording_app_server(
blocked_thread_list,
failed_thread_name,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await
}
@@ -91,6 +94,7 @@ pub(super) async fn start_recording_remote_app_server(
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Remote,
LoaderOverrides::default(),
)
.await
}
@@ -102,6 +106,7 @@ async fn start_recording_app_server_with_history(
mut blocked_thread_list: Option<(ThreadId, oneshot::Sender<()>, oneshot::Receiver<()>)>,
failed_thread_name: Option<&'static str>,
thread_params_mode: crate::app_server_session::ThreadParamsMode,
loader_overrides: LoaderOverrides,
) -> Result<RecordingAppServer> {
let state_db =
crate::init_state_db_for_app_server_target(config, &crate::AppServerTarget::Embedded)
@@ -110,7 +115,7 @@ async fn start_recording_app_server_with_history(
codex_arg0::Arg0DispatchPaths::default(),
config.clone(),
Vec::new(),
codex_config::LoaderOverrides::default(),
loader_overrides,
/*strict_config*/ false,
codex_config::CloudConfigBundleLoader::default(),
codex_feedback::CodexFeedback::new(),
@@ -185,7 +190,30 @@ async fn start_recording_app_server_with_history(
.is_some_and(|tools| {
tools.iter().any(|tool| tool["type"] == "namespace")
});
let response = if request.method == "thread/list"
let response = if let HistoryCapabilities::ConfigReadUnsupported(code) =
history_capabilities
&& request.method == "config/read"
{
JSONRPCMessage::Error(JSONRPCError {
id: request_id,
error: JSONRPCErrorError {
code,
data: None,
message: "unknown variant `config/read`".to_string(),
},
})
} else if history_capabilities == HistoryCapabilities::ThreadStartFails
&& request.method == "thread/start"
{
JSONRPCMessage::Error(JSONRPCError {
id: request_id,
error: JSONRPCErrorError {
code: -32603,
data: None,
message: "replacement unavailable".to_string(),
},
})
} else if request.method == "thread/list"
&& std::mem::take(&mut reject_thread_list)
{
JSONRPCMessage::Error(JSONRPCError {
@@ -1013,6 +1041,7 @@ async fn older_external_server_starts_without_unsupported_dynamic_tools_or_histo
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -1863,6 +1892,7 @@ async fn remote_legacy_history_start_negotiates_once_for_resume_and_fork() -> Re
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -1955,6 +1985,7 @@ async fn remote_legacy_history_start_retries_unsupported_paginated_variant() ->
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -1986,6 +2017,7 @@ async fn assert_remote_legacy_history_retry(request: LegacyHistoryRequest) -> Re
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -2053,6 +2085,7 @@ async fn paginated_fork_survives_post_response_hydration_failure() -> Result<()>
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -2391,6 +2424,7 @@ async fn agents_overview_stop_uses_history_mode_for_turn_lookup() -> Result<()>
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
@@ -2437,6 +2471,7 @@ async fn agents_overview_seeds_loaded_threads_when_recent_listing_is_unavailable
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
let started = app_server.start_thread(&app.config).await?;
@@ -2514,6 +2549,7 @@ async fn agents_overview_stop_uses_full_history_after_legacy_negotiation() -> Re
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
app_server.start_thread(&app.config).await?;
@@ -3392,3 +3428,6 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> {
.join()
.expect("session lifecycle request test thread")
}
#[path = "new_session_tests.rs"]
mod new_session_tests;