From ff3c82c8a9d0195f2cc92f6ac0492fd972d7acdd Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 29 Sep 2026 03:13:42 +0000 Subject: [PATCH] Preserve server reasoning summary and verbosity settings in the TUI (#49144) ## Why Forwarding local defaults as request overrides can replace server defaults or saved thread settings for reasoning summaries and verbosity. ## What changed Only forward `model_reasoning_summary` and `model_verbosity` as shared thread request overrides when their winning configuration layer is a session flag or an explicitly selected user profile. Use the effective values from that layer, and omit values whose winning source is ordinary user or project configuration. Preserve the existing reasoning-summary defaults for new embedded threads. ## Testing Add coverage for profile, project, and CLI precedence. Extend outbound model request tests to verify that remote-mode thread start and resume preserve server reasoning summary and verbosity settings despite conflicting local values. GitOrigin-RevId: 5e14254a51ed7506118a46847bb7466ab58908ef --- codex-rs/tui/src/app_server_session.rs | 111 ++++++++--- .../reasoning_defaults_tests.rs | 181 +++++++++++------- 2 files changed, 197 insertions(+), 95 deletions(-) diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 286acd3af6..dc90d8e373 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1836,18 +1836,6 @@ fn config_request_overrides_from_config( .as_ref() .map(std::string::ToString::to_string), ); - insert( - "model_reasoning_summary", - config - .model_reasoning_summary - .map(|summary| summary.to_string()), - ); - insert( - "model_verbosity", - config - .model_verbosity - .map(|verbosity| verbosity.to_string()), - ); insert( "personality", personality_opt_out_only(config.personality).map(|personality| personality.to_string()), @@ -1856,6 +1844,23 @@ fn config_request_overrides_from_config( "web_search", Some(config.web_search_mode.value().to_string()), ); + // Only winning launch choices may replace server defaults or saved thread settings. + let origins = config.config_layer_stack.origins(); + let effective = config.config_layer_stack.effective_config(); + for key in ["model_reasoning_summary", "model_verbosity"] { + if origins.get(key).is_some_and(|origin| { + matches!( + origin.name, + ConfigLayerSource::SessionFlags + | ConfigLayerSource::User { + profile: Some(_), + .. + } + ) + }) { + overrides.insert(key.to_string(), serde_json::json!(effective[key])); + } + } if config.bypass_hook_trust { overrides.insert("bypass_hook_trust".to_string(), true.into()); } @@ -3381,12 +3386,11 @@ mod tests { let string = |value: &str| serde_json::Value::String(value.to_string()); let expected_config = HashMap::from([ ("model_reasoning_effort".to_string(), string("high")), - ("model_reasoning_summary".to_string(), string("detailed")), - ("model_verbosity".to_string(), string("low")), ("web_search".to_string(), string("disabled")), ("bypass_hook_trust".to_string(), true.into()), ]); let mut expected_start_config = expected_config.clone(); + expected_start_config.insert("model_reasoning_summary".to_string(), string("detailed")); expected_start_config.insert( "features".to_string(), serde_json::json!({"concurrent_reasoning_summaries": false}), @@ -3396,6 +3400,71 @@ mod tests { assert_eq!(fork.config, Some(expected_config)); } + #[tokio::test] + async fn config_overrides_forward_explicit_summary_and_verbosity() -> Result<()> { + let home = tempfile::tempdir()?; + let workspace = home.path().join("workspace"); + std::fs::create_dir_all(workspace.join(".codex"))?; + std::fs::write( + home.path().join("config.toml"), + format!( + "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n[projects.{}]\ntrust_level = \"trusted\"\n", + toml::Value::String(workspace.to_string_lossy().into_owned()), + ), + )?; + let profile = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?; + std::fs::write( + &profile, + "model_reasoning_summary = \"detailed\"\nmodel_verbosity = \"high\"\n", + )?; + for (selected_profile, project, cli, expected) in [ + (false, false, false, [None, None]), + (true, false, false, [Some("detailed"), Some("high")]), + (false, false, true, [Some("auto"), Some("medium")]), + (true, false, true, [Some("auto"), Some("medium")]), + (true, true, false, [None, None]), + (true, true, true, [Some("auto"), Some("medium")]), + ] { + std::fs::write( + workspace.join(".codex/config.toml"), + if project { + "model_reasoning_summary = \"concise\"\nmodel_verbosity = \"low\"\n" + } else { + "" + }, + )?; + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .harness_overrides(ConfigOverrides { + cwd: Some(workspace.clone()), + ..ConfigOverrides::default() + }) + .loader_overrides(codex_config::LoaderOverrides { + user_config_path: selected_profile.then(|| profile.clone()), + user_config_profile: selected_profile.then(|| "work".parse().unwrap()), + ..codex_config::LoaderOverrides::without_managed_config_for_tests() + }) + .cli_overrides(if cli { + vec![ + ("model_reasoning_summary".to_string(), "auto".into()), + ("model_verbosity".to_string(), "medium".into()), + ] + } else { + Vec::new() + }) + .build() + .await?; + let overrides = + config_request_overrides_from_config(&config).expect("config overrides"); + assert_eq!( + ["model_reasoning_summary", "model_verbosity"] + .map(|key| overrides.get(key).and_then(serde_json::Value::as_str)), + expected, + ); + } + Ok(()) + } + #[tokio::test] async fn thread_resume_params_can_restore_persisted_model_settings() { let temp_dir = tempfile::tempdir().expect("tempdir"); @@ -3418,16 +3487,10 @@ mod tests { assert_eq!(params.model_provider, None); assert_eq!( params.config, - Some(HashMap::from([ - ( - "model_reasoning_summary".to_string(), - serde_json::Value::String("detailed".to_string()), - ), - ( - "web_search".to_string(), - serde_json::Value::String("cached".to_string()), - ), - ])) + Some(HashMap::from([( + "web_search".to_string(), + serde_json::Value::String("cached".to_string()), + )])) ); } diff --git a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs index 189dfce2bd..9813f2b9a1 100644 --- a/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs +++ b/codex-rs/tui/src/app_server_session/reasoning_defaults_tests.rs @@ -1,58 +1,63 @@ -//! Verify embedded TUI reasoning configuration and metadata on its outbound model request. +//! Verify TUI reasoning defaults and server-owned settings on outbound model requests. use super::*; use crate::legacy_core::config::ConfigBuilder; use codex_app_server_protocol::ServerNotification; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::Verbosity; use core_test_support::responses; use pretty_assertions::assert_eq; use serde_json::json; #[tokio::test] -async fn embedded_reasoning_defaults_reach_responses() -> Result<()> { - for (settings, summary, stream_options) in [ - ("", json!(null), json!(null)), +async fn reasoning_defaults_reach_responses() -> Result<()> { + for (settings, summary, stream_options, mode) in [ + ("", json!(null), json!(null), ThreadParamsMode::Embedded), ( "[features]\nconcurrent_reasoning_summaries = false", json!(null), json!(null), + ThreadParamsMode::Embedded, ), ( "[features]\nconcurrent_reasoning_summaries = true", json!(null), json!(null), + ThreadParamsMode::Embedded, ), ( "model_reasoning_summary = 'detailed'", json!("detailed"), json!(null), + ThreadParamsMode::Embedded, ), ( "model_reasoning_summary = 'detailed'\n[features]\nconcurrent_reasoning_summaries = true", json!("detailed"), json!({"reasoning_summary_delivery": "sequential_cutoff"}), + ThreadParamsMode::Embedded, ), ( "model_reasoning_summary = 'none'\n[features]\nconcurrent_reasoning_summaries = true", json!(null), json!(null), + ThreadParamsMode::Embedded, + ), + ( + "model_reasoning_summary = 'detailed'\nmodel_verbosity = 'high'", + json!("detailed"), + json!(null), + ThreadParamsMode::Remote, ), ] { let server = responses::start_mock_server().await; - let response = responses::mount_sse_once( - &server, - responses::sse(vec![ - responses::ev_response_created("response"), - responses::ev_completed("response"), - ]), - ) - .await; let home = tempfile::tempdir()?; let base_url = server.uri(); std::fs::write( home.path().join("config.toml"), format!( r#" -model = "gpt-5.2" +model = "gpt-5.5" model_provider = "reasoning-test" {settings} [model_providers.reasoning-test] @@ -64,72 +69,106 @@ stream_max_retries = 0 "# ), )?; - let config = ConfigBuilder::default() + let mut config = ConfigBuilder::default() .codex_home(home.path().to_path_buf()) .build() .await?; let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?; + app_server.thread_params_mode = mode; + if mode == ThreadParamsMode::Remote { + config.model_reasoning_summary = Some(ReasoningSummary::Concise); + config.model_verbosity = Some(Verbosity::Low); + } let started = app_server.start_thread(&config).await?; - let turn = app_server - .turn_start( - started.session.thread_id, - "tui-user-message".to_string(), - vec![UserInput::Text { - text: "hello".to_string(), - text_elements: Vec::new(), - }], - config.cwd.to_path_buf(), - /*approval_policy*/ None, - /*approvals_reviewer*/ None, - TurnPermissionsOverride::Preserve, - &config.workspace_roots, - started.session.model.clone(), - /*effort*/ None, - /*summary*/ None, - /*service_tier*/ None, - /*collaboration_mode*/ None, - /*personality*/ None, - /*output_schema*/ None, - ) - .await?; - tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async { - while let Some(event) = app_server.next_event().await { - if let AppServerEvent::ServerNotification(notification) = event - && let ServerNotification::TurnCompleted(completed) = *notification - { - assert_eq!( - completed.turn.status, - codex_app_server_protocol::TurnStatus::Completed - ); - return; + for resume in [false, true] { + if resume { + if mode != ThreadParamsMode::Remote { + break; } + app_server.shutdown().await?; + app_server = crate::start_embedded_app_server_for_picker(&config).await?; + app_server.thread_params_mode = mode; + app_server + .resume_thread( + &crate::local_settings::LocalSettings::from(&config), + config.clone(), + started.session.thread_id, + ResumeModelSettings::RestoreFromThread, + ) + .await?; } - panic!("app-server disconnected before completing the turn"); - }) - .await?; - let body = response.single_request().body_json(); - assert_eq!( - json!({"summary": body["reasoning"]["summary"], "stream_options": body["stream_options"]}), - json!({"summary": summary, "stream_options": stream_options}), - "settings: {settings}" - ); - let metadata: serde_json::Value = serde_json::from_str( - body["client_metadata"]["x-codex-turn-metadata"] - .as_str() - .expect("canonical turn metadata"), - )?; - assert_eq!( - ( - &metadata["thread_id"], - &metadata["turn_id"], - &metadata["turn_trigger"], - ), - ( - &json!(started.session.thread_id), - &json!(turn.turn.id), - &json!("user"), + let response = responses::mount_sse_once( + &server, + responses::sse(vec![ + responses::ev_response_created("response"), + responses::ev_completed("response"), + ]), ) - ); + .await; + let turn = app_server + .turn_start( + started.session.thread_id, + "tui-user-message".to_string(), + vec![UserInput::Text { + text: "hello".to_string(), + text_elements: Vec::new(), + }], + config.cwd.to_path_buf(), + /*approval_policy*/ None, + /*approvals_reviewer*/ None, + TurnPermissionsOverride::Preserve, + &config.workspace_roots, + started.session.model.clone(), + /*effort*/ None, + /*summary*/ None, + /*service_tier*/ None, + /*collaboration_mode*/ None, + /*personality*/ None, + /*output_schema*/ None, + ) + .await?; + tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 30), async { + while let Some(event) = app_server.next_event().await { + if let AppServerEvent::ServerNotification(notification) = event + && let ServerNotification::TurnCompleted(completed) = *notification + { + assert_eq!( + completed.turn.status, + codex_app_server_protocol::TurnStatus::Completed + ); + return; + } + } + panic!("app-server disconnected before completing the turn"); + }) + .await?; + let body = response.single_request().body_json(); + assert_eq!( + json!({"summary": body["reasoning"]["summary"], "stream_options": body["stream_options"]}), + json!({"summary": summary, "stream_options": stream_options}), + "settings: {settings}" + ); + if mode == ThreadParamsMode::Remote { + assert_eq!(body["text"]["verbosity"], json!("high")); + } + let metadata: serde_json::Value = serde_json::from_str( + body["client_metadata"]["x-codex-turn-metadata"] + .as_str() + .expect("canonical turn metadata"), + )?; + assert_eq!( + ( + &metadata["thread_id"], + &metadata["turn_id"], + &metadata["turn_trigger"], + ), + ( + &json!(started.session.thread_id), + &json!(turn.turn.id), + &json!("user"), + ) + ); + } app_server.shutdown().await?; } Ok(())