diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 9ff641e115..d58c5c742b 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -18,6 +18,7 @@ fn spawn_startup_thread_start( app_server: &AppServerSession, local_settings: crate::local_settings::LocalSettings, config: Config, + launch_choices: crate::app_server_session::StartupLaunchChoices, app_event_tx: AppEventSender, worktree: Option, ) { @@ -25,7 +26,6 @@ fn spawn_startup_thread_start( let thread_params_mode = app_server.thread_params_mode(); let remote_cwd_override = app_server.remote_cwd_override().map(Path::to_path_buf); let thread_tool_transport = app_server.thread_tool_transport(); - let model_provider_override = app_server.model_provider_override.clone(); tokio::spawn(async move { let result = crate::app_server_session::start_thread_with_request_handle( request_handle, @@ -34,7 +34,7 @@ fn spawn_startup_thread_start( thread_params_mode, remote_cwd_override, thread_tool_transport, - model_provider_override, + launch_choices, ) .await .and_then(|started| { @@ -331,6 +331,11 @@ impl App { &harness_overrides, ); } + let mut launch_choices = crate::app_server_session::StartupLaunchChoices::from_launch( + &cli_kv_overrides, + &harness_overrides, + &loader_overrides, + ); let mut model = startup_model(&config, &bootstrap, startup_defaults.server_defaults_read); let available_models = bootstrap.available_models; let remote_connection = crate::status::remote_connection::remote_connection_status_value( @@ -361,6 +366,7 @@ impl App { tui, &mut config, &local_settings, + &mut launch_choices, model.as_str(), &app_event_tx, &available_models, @@ -461,6 +467,7 @@ impl App { &app_server, local_settings.clone(), config.clone(), + launch_choices, app_event_tx.clone(), managed_worktree.clone(), ); diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index 08f8668c58..d0f3b31669 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -217,6 +217,7 @@ pub(super) fn target_preset_for_upgrade<'a>( pub(super) fn apply_accepted_model_migration( config: &mut Config, + launch_choices: &mut crate::app_server_session::StartupLaunchChoices, app_event_tx: &AppEventSender, from_model: String, target_model: String, @@ -227,6 +228,8 @@ pub(super) fn apply_accepted_model_migration( to_model: target_model.clone(), }); + launch_choices.model = Some(target_model.clone()); + launch_choices.effort = Some(serde_json::json!(target_default_effort)); config.model = Some(target_model.clone()); config.model_reasoning_effort = Some(target_default_effort.clone()); app_event_tx.send(AppEvent::UpdateModel(target_model.clone())); @@ -310,6 +313,7 @@ pub(super) async fn handle_model_migration_prompt_if_needed( tui: &mut tui::Tui, config: &mut Config, local_settings: &crate::local_settings::LocalSettings, + launch_choices: &mut crate::app_server_session::StartupLaunchChoices, model: &str, app_event_tx: &AppEventSender, available_models: &[ModelPreset], @@ -367,6 +371,7 @@ pub(super) async fn handle_model_migration_prompt_if_needed( ModelMigrationOutcome::Accepted => { apply_accepted_model_migration( config, + launch_choices, app_event_tx, model.to_string(), target_model.clone(), diff --git a/codex-rs/tui/src/app/tests/disconnect_tests.rs b/codex-rs/tui/src/app/tests/disconnect_tests.rs index 2cdaea45e3..2daf07b457 100644 --- a/codex-rs/tui/src/app/tests/disconnect_tests.rs +++ b/codex-rs/tui/src/app/tests/disconnect_tests.rs @@ -367,7 +367,7 @@ async fn lost_initial_thread_reply_keeps_startup_draft_offline() -> Result<()> { ThreadParamsMode::Remote, /*remote_cwd_override*/ None, session.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await; let mut tui = crate::tui::test_support::make_test_tui()?; diff --git a/codex-rs/tui/src/app/tests/model_catalog.rs b/codex-rs/tui/src/app/tests/model_catalog.rs index f4f19c5352..273725c2f2 100644 --- a/codex-rs/tui/src/app/tests/model_catalog.rs +++ b/codex-rs/tui/src/app/tests/model_catalog.rs @@ -439,6 +439,7 @@ async fn accepted_model_migration_persists_target_default_reasoning_effort() -> let app_event_tx = AppEventSender::new(tx_raw); apply_accepted_model_migration( &mut config, + &mut crate::app_server_session::StartupLaunchChoices::default(), &app_event_tx, current_model, upgrade.id, diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index 908d621183..341bf6e643 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -64,7 +64,7 @@ async fn daybreak_command_persists_and_confirms_each_selection() -> Result<()> { crate::app_server_session::ThreadParamsMode::Embedded, /*remote_cwd_override*/ None, server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert!(startup.session.daybreak_enabled); @@ -1024,7 +1024,7 @@ async fn external_transport_registers_dynamic_tools_and_finds_task_mentions() -> crate::app_server_session::ThreadParamsMode::Embedded, /*remote_cwd_override*/ None, app_server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert!(startup.task_tools_available); @@ -1305,7 +1305,7 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() crate::app_server_session::ThreadParamsMode::Embedded, /*remote_cwd_override*/ None, app_server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert!(startup.task_tools_available); @@ -1696,7 +1696,7 @@ async fn older_external_server_starts_without_unsupported_dynamic_tools_or_histo crate::app_server_session::ThreadParamsMode::Embedded, /*remote_cwd_override*/ None, app_server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert!(!startup.task_tools_available); diff --git a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs index 22e27ed3f1..85e337487e 100644 --- a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs +++ b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs @@ -175,21 +175,50 @@ async fn run_until_thread_start( #[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"), - ("profile_model", true, "profile-model", "high"), - ("profile_effort", true, "server-model", "low"), - ("managed", true, "managed-model", "medium"), + for (choice, managed, expected_model, expected_effort, expected_provider) in [ + ("saved", false, None, None, None), + ("cli_model", true, Some("cli-model"), None, None), + ("kv_model", false, Some("kv-model"), None, None), + ( + "migration", + false, + Some("upgraded-model"), + Some("high"), + None, + ), + ("cli_effort", true, None, Some("low"), None), + ("cli_provider", false, None, None, Some("openai")), + ("kv_provider", false, None, None, Some("openai")), + ( + "profile_model", + false, + Some("profile-model"), + Some("high"), + None, + ), + ( + "profile_effort", + false, + Some("server-model"), + Some("low"), + None, + ), + ("managed", true, None, None, None), ] { 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", + r#" +model = "client-model" +model_reasoning_effort = "low" +model_provider = "client-provider" +[model_providers.client-provider] +name = "Client provider" +base_url = "http://127.0.0.1:9/v1" +wire_api = "responses" +requires_openai_auth = false +"#, )?; std::fs::write( server_home.path().join("config.toml"), @@ -206,6 +235,13 @@ async fn fresh_startup_uses_server_defaults_with_explicit_and_managed_precedence let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests(); match choice { "cli_model" => harness_overrides.model = Some("cli-model".to_string()), + "kv_model" => cli_kv_overrides.extend([ + ("model".into(), "earlier-model".into()), + ("model".into(), "kv-model".into()), + ]), + "migration" => harness_overrides.model = Some("old-model".into()), + "cli_provider" => harness_overrides.model_provider = Some("openai".into()), + "kv_provider" => cli_kv_overrides.push(("model_provider".into(), "openai".into())), "cli_effort" => cli_kv_overrides.push(( "model_reasoning_effort".to_string(), TomlValue::String("low".to_string()), @@ -225,6 +261,11 @@ async fn fresh_startup_uses_server_defaults_with_explicit_and_managed_precedence } _ => {} } + let mut launch_choices = crate::app_server_session::StartupLaunchChoices::from_launch( + &cli_kv_overrides, + &harness_overrides, + &loader_overrides, + ); let mut config = ConfigBuilder::default() .codex_home(client_home.path().to_path_buf()) .loader_overrides(loader_overrides) @@ -249,7 +290,7 @@ async fn fresh_startup_uses_server_defaults_with_explicit_and_managed_precedence ) .await?; server = server.with_remote_cwd_override(Some(server_config.cwd.to_path_buf())); - let bootstrap = server.bootstrap(&config).await?; + server.bootstrap(&config).await?; assert!( prepare_fresh_startup_config( &mut config, @@ -261,32 +302,42 @@ async fn fresh_startup_uses_server_defaults_with_explicit_and_managed_precedence .await? .server_defaults_read ); - let selected_model = startup_model(&config, &bootstrap, /*server_defaults_read*/ true); - let started = crate::app_server_session::start_thread_with_request_handle( + if choice == "migration" { + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + apply_accepted_model_migration( + &mut config, + &mut launch_choices, + &AppEventSender::new(tx), + "old-model".into(), + "upgraded-model".into(), + ReasoningEffortConfig::High, + ); + } + 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(), - /*model_provider_override*/ None, + launch_choices, ) .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"] + starts[0]["model"].clone(), + starts[0]["config"].get("model_reasoning_effort").cloned(), + starts[0]["modelProvider"].clone() ), ( - &serde_json::json!(expected_model), - &serde_json::json!(expected_effort) + serde_json::json!(expected_model), + expected_effort.map(serde_json::Value::from), + serde_json::json!(expected_provider), ), "{choice}" ); - assert_eq!(started.session.model, expected_model, "{choice}"); assert_eq!( recorded_params(&requests, "config/read") .into_iter() @@ -372,7 +423,7 @@ async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Res server.thread_params_mode(), server.remote_cwd_override().map(Path::to_path_buf), server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert_eq!(started.session.model, selected_model); @@ -389,7 +440,7 @@ async fn fresh_startup_reads_destination_and_cleared_model_uses_catalog() -> Res 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"); + assert!(starts[0]["config"].get("model_reasoning_effort").is_none()); let (mut app, _, _) = make_test_app_with_channels().await; app.chat_widget.handle_thread_session_quiet(started.session); if !remote { @@ -457,10 +508,7 @@ async fn fresh_startup_falls_back_only_for_unsupported_config_read() -> Result<( &starts[0]["model"], &starts[0]["config"]["model_reasoning_effort"] ), - ( - &serde_json::json!("client-model"), - &serde_json::json!("low") - ) + (&serde_json::Value::Null, &serde_json::Value::Null) ); tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???; } @@ -520,10 +568,7 @@ async fn startup_reads_server_defaults_before_starting_thread() -> Result<()> { &starts[0]["model"], &starts[0]["config"]["model_reasoning_effort"] ), - ( - &serde_json::json!("server-model"), - &serde_json::json!("high") - ), + (&serde_json::Value::Null, &serde_json::Value::Null), ); tokio::time::timeout(Duration::from_secs(/*secs*/ 15), proxy).await???; Ok(()) diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index ac9657391c..432a082c82 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -13,6 +13,7 @@ pub(crate) mod provider_selection; mod provider_selection_tests; mod realtime; mod rollout_history; +mod startup_launch; mod thread_list; mod web_search; @@ -25,6 +26,7 @@ pub(crate) use history::HISTORY_ITEM_SCAN_LIMIT; pub(crate) use history::HistoryHydrationScope; pub(crate) use history::INITIAL_HISTORY_TURN_LIMIT; pub(crate) use history::thread_items_page_params; +pub(crate) use startup_launch::StartupLaunchChoices; use crate::app_event::PermissionProfileSelection; use crate::app_event_sender::AppEventSender; @@ -1729,7 +1731,7 @@ pub(crate) async fn start_thread_with_request_handle( thread_params_mode: ThreadParamsMode, remote_cwd_override: Option, thread_tool_transport: ThreadToolTransport, - model_provider_override: Option, + launch_choices: StartupLaunchChoices, ) -> Result { let request_id = RequestId::String(format!("startup-thread-start-{}", Uuid::new_v4())); let mut params = thread_start_params_from_config( @@ -1738,7 +1740,7 @@ pub(crate) async fn start_thread_with_request_handle( remote_cwd_override.as_deref(), /*session_start_source*/ None, ); - params.model_provider = model_provider_override.or(params.model_provider); + launch_choices.configure(&mut params); params.daybreak_enabled = (config.daybreak_enabled && !config.ephemeral).then_some(true); thread_tool_transport.configure(&mut params); let (response, _history_support, task_tools_available) = diff --git a/codex-rs/tui/src/app_server_session/prompt_history_tests.rs b/codex-rs/tui/src/app_server_session/prompt_history_tests.rs index 28092d3321..6743f85a16 100644 --- a/codex-rs/tui/src/app_server_session/prompt_history_tests.rs +++ b/codex-rs/tui/src/app_server_session/prompt_history_tests.rs @@ -70,7 +70,7 @@ async fn lifecycle_metadata_uses_local_prompt_history() -> Result<()> { mode, /*remote_cwd_override*/ None, app_server.thread_tool_transport(), - /*model_provider_override*/ None, + crate::app_server_session::StartupLaunchChoices::default(), ) .await?; assert_eq!(startup.session.message_history, expected); diff --git a/codex-rs/tui/src/app_server_session/startup_launch.rs b/codex-rs/tui/src/app_server_session/startup_launch.rs new file mode 100644 index 0000000000..b9cfaad187 --- /dev/null +++ b/codex-rs/tui/src/app_server_session/startup_launch.rs @@ -0,0 +1,71 @@ +//! First-thread model overrides come from invocation inputs, independently of local config. +//! Explicit profile launches retain their existing resolved-config path until server profile +//! selection is available for connected launches. + +use crate::legacy_core::config::ConfigOverrides; +use codex_app_server_protocol::ThreadStartParams; +use codex_config::LoaderOverrides; + +#[derive(Default)] +pub(crate) struct StartupLaunchChoices { + pub(crate) model: Option, + pub(crate) effort: Option, + provider: Option, + selected_profile: bool, +} + +impl StartupLaunchChoices { + pub(crate) fn from_launch( + cli_overrides: &[(String, toml::Value)], + harness_overrides: &ConfigOverrides, + loader_overrides: &LoaderOverrides, + ) -> Self { + if loader_overrides.user_config_profile.is_some() { + return Self { + provider: harness_overrides.model_provider.clone(), + selected_profile: true, + ..Self::default() + }; + } + let value = |key| { + cli_overrides + .iter() + .rev() + .find(|(path, _)| path == key) + .map(|(_, value)| value) + }; + Self { + model: harness_overrides.model.clone().or_else(|| { + value("model") + .and_then(toml::Value::as_str) + .map(str::to_owned) + }), + effort: value("model_reasoning_effort") + .and_then(|value| serde_json::to_value(value).ok()), + provider: harness_overrides.model_provider.clone().or_else(|| { + value("model_provider") + .and_then(toml::Value::as_str) + .map(str::to_owned) + }), + selected_profile: false, + } + } + + pub(super) fn configure(self, params: &mut ThreadStartParams) { + if self.selected_profile { + params.model_provider = self.provider.or(params.model_provider.take()); + return; + } + params.model = self.model; + params.model_provider = self.provider; + if let Some(overrides) = params.config.as_mut() { + overrides.remove("model_reasoning_effort"); + } + if let Some(effort) = self.effort { + params + .config + .get_or_insert_default() + .insert("model_reasoning_effort".into(), effort); + } + } +}