Use server model defaults for fresh TUI startup (#43177)

## Why

Fresh startup could use saved client model and reasoning settings that differ from the app server's effective configuration. A cleared server model could also leave startup using a stale client model from bootstrap.

## What changed

- Reuse the new-session defaults reader at startup to load `model` and `model_reasoning_effort` through `config/read` before `thread/start`, using the appropriate embedded or remote working directory.
- Preserve explicit CLI and selected-profile settings, then apply managed new-thread defaults with the existing precedence rules.
- When the server has no configured model, select the startup model from its model catalog, falling back to the bootstrap default if the catalog is empty.
- Fall back to local defaults when older servers do not support `config/read`; stop startup before thread creation on other read failures.

## Testing

Add request-level tests for defaults and override precedence, working-directory selection, cleared-model fallback, request ordering, unsupported servers, and read failures.

GitOrigin-RevId: e094ba5d5501fdb34c393a0117b34dd93d8d4f2b
This commit is contained in:
Eric Traut
2026-09-06 07:20:54 +00:00
committed by copyberry
parent 6af345407d
commit 9587c9ef36
6 changed files with 581 additions and 53 deletions

View File

@@ -5,6 +5,66 @@
use super::*;
use codex_config::ConfigLayerSource;
pub(super) async fn read_new_session_defaults(
app_server: &AppServerSession,
cwd: &Path,
) -> Result<Option<codex_app_server_protocol::Config>> {
// 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.
match crate::config_update::read_effective_config(
app_server.request_handle(),
cwd.display().to_string(),
)
.await
{
Ok(response) => Ok(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.
Ok(None)
}
Err(err) => Err(err),
}
}
pub(super) fn overlay_new_session_defaults(
config: &mut Config,
defaults: &codex_app_server_protocol::Config,
cli_kv_overrides: &[(String, TomlValue)],
harness_overrides: &ConfigOverrides,
) {
// A remote server cannot resolve this invocation's explicitly selected local profile.
let has_launch_setting = |key: &str| {
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 harness_overrides.model.is_none() && !has_launch_setting("model") {
config.model = defaults.model.clone();
}
if !has_launch_setting("model_reasoning_effort") {
config.model_reasoning_effort = defaults.model_reasoning_effort.clone();
}
}
impl App {
pub(super) async fn load_new_session_config(
&mut self,
@@ -17,31 +77,7 @@ impl App {
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),
};
let defaults = read_new_session_defaults(app_server, defaults_cwd).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,
@@ -52,28 +88,13 @@ impl App {
};
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;
}
if let Some(defaults) = defaults.as_ref() {
overlay_new_session_defaults(
&mut config,
defaults,
&self.cli_kv_overrides,
&self.harness_overrides,
);
}
Ok(config)
}

View File

@@ -51,6 +51,58 @@ fn spawn_startup_thread_start(
});
}
pub(super) async fn prepare_fresh_startup_config(
config: &mut Config,
app_server: &AppServerSession,
cli_kv_overrides: &[(String, TomlValue)],
harness_overrides: &ConfigOverrides,
) -> Result<bool> {
let defaults_cwd = match app_server.thread_params_mode() {
crate::app_server_session::ThreadParamsMode::Embedded => config.cwd.as_path(),
crate::app_server_session::ThreadParamsMode::Remote => {
app_server.remote_cwd_override().unwrap_or(Path::new("."))
}
};
let defaults = super::new_session::read_new_session_defaults(app_server, defaults_cwd).await?;
if let Some(defaults) = defaults.as_ref() {
super::new_session::overlay_new_session_defaults(
config,
defaults,
cli_kv_overrides,
harness_overrides,
);
}
apply_managed_new_thread_defaults(
config,
app_server.managed_new_thread_defaults(),
cli_kv_overrides,
harness_overrides,
);
Ok(defaults.is_some())
}
pub(super) fn startup_model(
config: &Config,
bootstrap: &AppServerBootstrap,
server_defaults_read: bool,
) -> String {
config.model.clone().unwrap_or_else(|| {
if server_defaults_read {
// Bootstrap was seeded with local config, which may differ from a cleared server
// model. Use the server's model catalog when config/read returned model: null.
bootstrap
.available_models
.iter()
.find(|model| model.is_default)
.or_else(|| bootstrap.available_models.first())
.map(|model| model.model.clone())
.unwrap_or_else(|| bootstrap.default_model.clone())
} else {
bootstrap.default_model.clone()
}
})
}
impl App {
/// Recognizes queued requests before they become visible protected screens.
pub(super) fn has_queued_startup_protected_request(&self) -> bool {
@@ -166,12 +218,30 @@ impl App {
"connected app-server platform"
);
let bootstrap_ms = bootstrap.duration.as_millis();
if matches!(
let server_defaults_read = if matches!(
&session_selection,
SessionSelection::StartFresh
| SessionSelection::Exit
| SessionSelection::AgentsOverview
SessionSelection::StartFresh | SessionSelection::Exit
) {
match startup_draft
.run_until(
tui,
prepare_fresh_startup_config(
&mut config,
&app_server,
&cli_kv_overrides,
&harness_overrides,
),
)
.await
{
Ok(Ok(defaults_read)) => defaults_read,
Ok(Err(err)) => return shutdown_on_startup_error(app_server, err).await,
Err(err) => return shutdown_on_startup_error(app_server, err).await,
}
} else {
false
};
if matches!(&session_selection, SessionSelection::AgentsOverview) {
apply_managed_new_thread_defaults(
&mut config,
app_server.managed_new_thread_defaults(),
@@ -179,7 +249,7 @@ impl App {
&harness_overrides,
);
}
let mut model = config.model.clone().unwrap_or(bootstrap.default_model);
let mut model = startup_model(&config, &bootstrap, server_defaults_read);
let available_models = bootstrap.available_models;
let remote_connection = crate::status::remote_connection::remote_connection_status_value(
&app_server_target,

View File

@@ -51,6 +51,7 @@ enum HistoryCapabilities {
ThreadListFails,
ThreadStartFails,
ConfigReadUnsupported(i64),
ConfigReadFails,
}
/// Returns and resets `(thread/loaded/list, thread/read)` request counts.
@@ -202,6 +203,17 @@ async fn start_recording_app_server_with_history(
message: "unknown variant `config/read`".to_string(),
},
})
} else if history_capabilities == HistoryCapabilities::ConfigReadFails
&& request.method == "config/read"
{
JSONRPCMessage::Error(JSONRPCError {
id: request_id,
error: JSONRPCErrorError {
code: -32603,
data: None,
message: "config temporarily unavailable".to_string(),
},
})
} else if history_capabilities == HistoryCapabilities::ThreadStartFails
&& request.method == "thread/start"
{
@@ -3716,3 +3728,5 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> {
#[path = "new_session_tests.rs"]
mod new_session_tests;
#[path = "startup_defaults_tests.rs"]
mod startup_defaults_tests;

View File

@@ -0,0 +1,417 @@
//! Request-level coverage for the fresh-startup server defaults overlay.
use super::*;
use crate::app::startup::prepare_fresh_startup_config;
use crate::app::startup::startup_model;
use pretty_assertions::assert_eq;
async fn run_fresh_startup_for_test(
tui: &mut crate::tui::Tui,
server: AppServerSession,
config: Config,
bootstrap: AppServerBootstrap,
) -> Result<AppExitInfo> {
App::run(
tui,
server,
config.clone(),
config.cwd.to_path_buf(),
Vec::new(),
ConfigOverrides::default(),
LoaderOverrides::default(),
CloudConfigBundleLoader::default(),
/*initial_prompt*/ None,
Vec::new(),
SessionSelection::StartFresh,
codex_feedback::CodexFeedback::new(),
/*is_first_run*/ false,
/*should_prompt_windows_sandbox_nux_at_startup*/ false,
AppServerTarget::Embedded,
/*state_db*/ None,
Arc::new(EnvironmentManager::default_for_tests()),
Duration::ZERO,
Some(bootstrap),
/*startup_hooks_browser*/ None,
crate::startup_draft::tests::quiet_startup_test_pump(),
/*managed_worktree*/ None,
)
.await
}
async fn run_until_thread_start(
server: AppServerSession,
config: Config,
bootstrap: AppServerBootstrap,
requests: &RecordedRequests,
) -> Result<()> {
let mut tui = crate::tui::test_support::make_test_tui()?;
tui.pause_events();
let mut run = Box::pin(run_fresh_startup_for_test(
&mut tui, server, config, bootstrap,
));
tokio::time::timeout(Duration::from_secs(/*secs*/ 15), async {
loop {
if !recorded_params(requests, "thread/start").is_empty() {
return Ok::<(), color_eyre::eyre::Report>(());
}
tokio::select! {
result = &mut run => {
result?;
return Err(color_eyre::eyre::eyre!("startup exited before thread/start"));
}
() = tokio::time::sleep(Duration::from_millis(/*millis*/ 20)) => {}
}
}
})
.await??;
Ok(())
}
#[tokio::test]
async fn fresh_startup_uses_server_defaults_with_explicit_and_managed_precedence() -> Result<()> {
for (choice, managed, expected_model, expected_effort) in [
("saved", false, "server-model", "high"),
("cli_model", true, "cli-model", "high"),
("cli_effort", true, "server-model", "low"),
("profile_model", false, "profile-model", "high"),
("profile_effort", false, "server-model", "low"),
("managed", true, "managed-model", "medium"),
] {
let client_home = tempdir()?;
let server_home = tempdir()?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"client-model\"\nmodel_reasoning_effort = \"low\"\n",
)?;
std::fs::write(
server_home.path().join("config.toml"),
"model = \"server-model\"\nmodel_reasoning_effort = \"high\"\n",
)?;
if managed {
std::fs::write(
server_home.path().join("requirements.toml"),
"[models.new_thread]\nmodel = \"managed-model\"\nmodel_reasoning_effort = \"medium\"\n",
)?;
}
let mut harness_overrides = ConfigOverrides::default();
let mut cli_kv_overrides = Vec::new();
let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests();
match choice {
"cli_model" => harness_overrides.model = Some("cli-model".to_string()),
"cli_effort" => cli_kv_overrides.push((
"model_reasoning_effort".to_string(),
TomlValue::String("low".to_string()),
)),
"profile_model" | "profile_effort" => {
let path = client_home.path().join("work.config.toml");
std::fs::write(
&path,
if choice == "profile_model" {
"model = \"profile-model\"\n"
} else {
"model_reasoning_effort = \"low\"\n"
},
)?;
loader_overrides.user_config_path = Some(path.abs());
loader_overrides.user_config_profile = Some("work".parse()?);
}
_ => {}
}
let mut config = ConfigBuilder::default()
.codex_home(client_home.path().to_path_buf())
.loader_overrides(loader_overrides)
.cli_overrides(cli_kv_overrides.clone())
.harness_overrides(harness_overrides.clone())
.build()
.await?;
let mut server_config = 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("config.toml").abs()),
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()));
let bootstrap = server.bootstrap(&config).await?;
assert!(
prepare_fresh_startup_config(
&mut config,
&server,
&cli_kv_overrides,
&harness_overrides,
)
.await?
);
let selected_model = startup_model(&config, &bootstrap, /*server_defaults_read*/ true);
let started = crate::app_server_session::start_thread_with_request_handle(
server.request_handle(),
&crate::local_settings::LocalSettings::from(&config),
config,
server.thread_params_mode(),
server.remote_cwd_override().map(Path::to_path_buf),
server.thread_tool_transport(),
)
.await?;
assert_eq!(selected_model, expected_model, "{choice}");
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1, "{choice}");
assert_eq!(
(
&starts[0]["model"],
&starts[0]["config"]["model_reasoning_effort"]
),
(
&serde_json::json!(expected_model),
&serde_json::json!(expected_effort)
),
"{choice}"
);
assert_eq!(started.session.model, expected_model, "{choice}");
assert_eq!(
recorded_params(&requests, "config/read"),
vec![serde_json::json!({"cwd": server_config.cwd.display().to_string()})],
);
server.shutdown().await?;
proxy.await??;
}
Ok(())
}
#[tokio::test]
async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Result<()> {
for (remote, override_cwd) in [(false, false), (true, false), (true, true)] {
let client_home = tempdir()?;
let server_home = tempdir()?;
let destination = tempdir()?;
let launch_cwd = tempdir()?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"stale-client-model\"\nmodel_reasoning_effort = \"low\"\n",
)?;
std::fs::write(
server_home.path().join("config.toml"),
"model_reasoning_effort = \"high\"\n",
)?;
let mut config = ConfigBuilder::default()
.codex_home(client_home.path().to_path_buf())
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.harness_overrides(ConfigOverrides {
cwd: Some(destination.path().to_path_buf()),
..Default::default()
})
.build()
.await?;
let mut server_config = 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 mode = if remote {
crate::app_server_session::ThreadParamsMode::Remote
} else {
crate::app_server_session::ThreadParamsMode::Embedded
};
let (mut server, requests, proxy) = start_recording_app_server_with_history(
&server_config,
HistoryCapabilities::Current,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
mode,
LoaderOverrides {
user_config_path: Some(server_home.path().join("config.toml").abs()),
..LoaderOverrides::default()
},
)
.await?;
if override_cwd {
server = server.with_remote_cwd_override(Some(launch_cwd.path().to_path_buf()));
}
let bootstrap = server.bootstrap(&config).await?;
assert_eq!(bootstrap.default_model, "stale-client-model");
let defaults_read =
prepare_fresh_startup_config(&mut config, &server, &[], &ConfigOverrides::default())
.await?;
assert!(defaults_read);
assert_eq!(config.model, None);
let selected_model = startup_model(&config, &bootstrap, defaults_read);
assert_ne!(selected_model, "stale-client-model");
let started = crate::app_server_session::start_thread_with_request_handle(
server.request_handle(),
&crate::local_settings::LocalSettings::from(&config),
config,
server.thread_params_mode(),
server.remote_cwd_override().map(Path::to_path_buf),
server.thread_tool_transport(),
)
.await?;
assert_eq!(started.session.model, selected_model);
let starts = recorded_params(&requests, "thread/start");
assert_eq!(starts.len(), 1);
assert_eq!(starts[0]["model"], serde_json::Value::Null);
assert_eq!(starts[0]["config"]["model_reasoning_effort"], "high");
let (mut app, _, _) = make_test_app_with_channels().await;
app.chat_widget.handle_thread_session_quiet(started.session);
if !remote {
let rendered = render_bottom_popup(&app.chat_widget, /*width*/ 80)
.replace(&destination.path().display().to_string(), "<PROJECT>");
insta::assert_snapshot!(rendered, @r"
Ask Codex to do anything
gpt-6-astra high · <PROJECT>
");
}
let expected_cwd = if override_cwd {
launch_cwd.path().display().to_string()
} else if !remote {
destination.path().display().to_string()
} else {
".".to_string()
};
assert_eq!(
recorded_params(&requests, "config/read"),
vec![serde_json::json!({"cwd": expected_cwd})]
);
server.shutdown().await?;
proxy.await??;
}
Ok(())
}
#[tokio::test]
async fn fresh_startup_falls_back_only_for_unsupported_config_read() -> Result<()> {
for capabilities in [
HistoryCapabilities::ConfigReadUnsupported(-32601),
HistoryCapabilities::ConfigReadUnsupported(-32600),
] {
let home = tempdir()?;
std::fs::write(
home.path().join("config.toml"),
"model = \"client-model\"\nmodel_reasoning_effort = \"low\"\n",
)?;
let config = ConfigBuilder::default()
.codex_home(home.path().to_path_buf())
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.build()
.await?;
let (mut server, requests, proxy) = start_recording_app_server_with_history(
&config,
capabilities,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
let bootstrap = server.bootstrap(&config).await?;
run_until_thread_start(server, config, bootstrap, &requests).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!("client-model"),
&serde_json::json!("low")
)
);
tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???;
}
Ok(())
}
#[tokio::test]
async fn startup_reads_server_defaults_before_starting_thread() -> Result<()> {
let client_home = tempdir()?;
let server_home = tempdir()?;
std::fs::write(
client_home.path().join("config.toml"),
"model = \"client-model\"\nmodel_reasoning_effort = \"low\"\n",
)?;
std::fs::write(
server_home.path().join("config.toml"),
"model = \"server-model\"\nmodel_reasoning_effort = \"high\"\n",
)?;
let config = ConfigBuilder::default()
.codex_home(client_home.path().to_path_buf())
.loader_overrides(LoaderOverrides::without_managed_config_for_tests())
.build()
.await?;
let mut server_config = 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::Embedded,
LoaderOverrides {
user_config_path: Some(server_home.path().join("config.toml").abs()),
..LoaderOverrides::default()
},
)
.await?;
let bootstrap = server.bootstrap(&config).await?;
run_until_thread_start(server, config, bootstrap, &requests).await?;
{
let recorded = requests.lock().expect("request recorder lock");
let read = recorded
.iter()
.position(|request| request.method == "config/read")
.expect("startup must read server defaults");
let start = recorded
.iter()
.position(|request| request.method == "thread/start")
.expect("startup must start a thread");
assert!(read < start);
}
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!("server-model"),
&serde_json::json!("high")
),
);
tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???;
Ok(())
}
#[tokio::test]
async fn startup_read_failure_exits_before_thread_creation() -> Result<()> {
let (app, _, _) = make_test_app_with_channels().await;
let home = tempdir()?;
let mut config = app.config.clone();
config.codex_home = home.path().to_path_buf().abs();
config.sqlite = SqliteConfig::new_for_testing(home.path().abs());
let (mut server, requests, proxy) = start_recording_app_server_with_history(
&config,
HistoryCapabilities::ConfigReadFails,
/*blocked_thread_list*/ None,
/*failed_thread_name*/ None,
crate::app_server_session::ThreadParamsMode::Embedded,
LoaderOverrides::default(),
)
.await?;
let bootstrap = server.bootstrap(&config).await?;
let mut tui = crate::tui::test_support::make_test_tui()?;
let result = run_fresh_startup_for_test(&mut tui, server, config, bootstrap).await;
insta::assert_snapshot!(result.expect_err("startup read must fail").to_string(), @"config/read failed in TUI");
assert_eq!(recorded_params(&requests, "config/read").len(), 1);
assert!(recorded_params(&requests, "thread/start").is_empty());
proxy.await??;
Ok(())
}

View File

@@ -525,4 +525,4 @@ fn startup_draft_bottom_pane(
#[cfg(test)]
#[path = "startup_draft_tests.rs"]
mod tests;
pub(crate) mod tests;

View File

@@ -42,6 +42,12 @@ where
}
}
pub(crate) fn quiet_startup_test_pump() -> StartupDraftPump {
let mut pump = startup_test_pump(std::iter::empty());
pump.events = Box::pin(futures::stream::pending());
pump
}
#[test]
fn startup_draft_renders_full_empty_and_multiline_composer_frames() {
let mut pump = startup_test_pump(std::iter::empty());