mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Use managed defaults for TUI threads (#30147)
## Why #29683 exposes managed defaults for new-thread model settings through `configRequirements/read` without applying them server-wide. The TUI is an app-server client, so it should explicitly consume those defaults when it creates a fresh thread. This lets plain `codex` start on the managed model while preserving the existing ability to change model settings within the thread. ## What changed - Read `requirements.models.newThread` during TUI app-server bootstrap. - Apply the managed model, reasoning effort, and service tier to the initial fresh thread and subsequent `/new` or `/clear` threads. - Keep explicit launch overrides above the managed defaults. - Normalize the managed `fast` service tier to the `priority` request value. - Leave resumed and forked threads unchanged. The application logic lives in a small TUI-only module; app-server `thread/start` behavior remains unchanged for other clients. ## User experience - Plain `codex` starts with the managed new-thread settings. - A user can still change settings with `/model` or the existing service-tier controls. - Starting another fresh thread reapplies the managed defaults. - Explicit launch choices such as `codex -m <model>` continue to win. ## Validation - `just test -p codex-tui managed_new_thread_defaults` - `just fix -p codex-tui` Depends on #29683.
This commit is contained in:
committed by
GitHub
Unverified
parent
79a8ffdbf7
commit
cf36c688b3
+13
-1
@@ -51,6 +51,7 @@ use crate::legacy_core::config::ConfigBuilder;
|
||||
use crate::legacy_core::config::ConfigOverrides;
|
||||
use crate::legacy_core::config::PermissionProfileSnapshot;
|
||||
use crate::legacy_core::config::edit::ConfigEditsBuilder;
|
||||
use crate::managed_new_thread_defaults::apply_managed_new_thread_defaults;
|
||||
use crate::model_catalog::ModelCatalog;
|
||||
use crate::model_migration::ModelMigrationOutcome;
|
||||
use crate::model_migration::migration_copy_for_models;
|
||||
@@ -794,7 +795,18 @@ impl App {
|
||||
None => app_server.bootstrap(&config).await?,
|
||||
};
|
||||
let bootstrap_ms = bootstrap.duration.as_millis();
|
||||
let mut model = bootstrap.default_model;
|
||||
if matches!(
|
||||
&session_selection,
|
||||
SessionSelection::StartFresh | SessionSelection::Exit
|
||||
) {
|
||||
apply_managed_new_thread_defaults(
|
||||
&mut config,
|
||||
app_server.managed_new_thread_defaults(),
|
||||
&cli_kv_overrides,
|
||||
&harness_overrides,
|
||||
);
|
||||
}
|
||||
let mut model = config.model.clone().unwrap_or(bootstrap.default_model);
|
||||
let available_models = bootstrap.available_models;
|
||||
let remote_connection = crate::status::remote_connection::remote_connection_status_value(
|
||||
&app_server_target,
|
||||
|
||||
@@ -539,7 +539,13 @@ impl App {
|
||||
self.refresh_in_memory_config_from_disk_best_effort("starting a new thread")
|
||||
.await;
|
||||
let model = self.chat_widget.current_model().to_string();
|
||||
let config = self.fresh_session_config();
|
||||
let mut config = self.fresh_session_config();
|
||||
apply_managed_new_thread_defaults(
|
||||
&mut config,
|
||||
app_server.managed_new_thread_defaults(),
|
||||
&self.cli_kv_overrides,
|
||||
&self.harness_overrides,
|
||||
);
|
||||
let summary = session_summary(
|
||||
self.chat_widget.token_usage(),
|
||||
self.chat_widget.thread_id(),
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigRequirementsReadResponse;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::ExternalAgentConfigDetectParams;
|
||||
use codex_app_server_protocol::ExternalAgentConfigDetectResponse;
|
||||
@@ -39,6 +40,7 @@ use codex_app_server_protocol::MemoryResetResponse;
|
||||
use codex_app_server_protocol::Model as ApiModel;
|
||||
use codex_app_server_protocol::ModelListParams;
|
||||
use codex_app_server_protocol::ModelListResponse;
|
||||
use codex_app_server_protocol::NewThreadModelDefaults;
|
||||
use codex_app_server_protocol::RateLimitSnapshot;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ReviewDelivery;
|
||||
@@ -178,6 +180,7 @@ pub(crate) struct AppServerSession {
|
||||
thread_settings_update_supported: bool,
|
||||
default_model: Option<String>,
|
||||
available_models: Vec<ModelPreset>,
|
||||
managed_new_thread_defaults: Option<NewThreadModelDefaults>,
|
||||
external_agent_config_import_completion_pending: AtomicBool,
|
||||
}
|
||||
|
||||
@@ -222,6 +225,7 @@ impl AppServerSession {
|
||||
thread_settings_update_supported: true,
|
||||
default_model: None,
|
||||
available_models: Vec::new(),
|
||||
managed_new_thread_defaults: None,
|
||||
external_agent_config_import_completion_pending: AtomicBool::new(false),
|
||||
}
|
||||
}
|
||||
@@ -260,6 +264,21 @@ impl AppServerSession {
|
||||
pub(crate) async fn bootstrap(&mut self, config: &Config) -> Result<AppServerBootstrap> {
|
||||
let started_at = Instant::now();
|
||||
let account = self.read_account().await?;
|
||||
let requirements_request_id = self.next_request_id();
|
||||
let requirements: ConfigRequirementsReadResponse = self
|
||||
.client
|
||||
.request_typed(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: requirements_request_id,
|
||||
params: None,
|
||||
})
|
||||
.await
|
||||
.map_err(|err| {
|
||||
bootstrap_request_error("configRequirements/read failed during TUI bootstrap", err)
|
||||
})?;
|
||||
self.managed_new_thread_defaults = requirements
|
||||
.requirements
|
||||
.and_then(|requirements| requirements.models)
|
||||
.and_then(|models| models.new_thread);
|
||||
let model_request_id = self.next_request_id();
|
||||
let models: ModelListResponse = self
|
||||
.client
|
||||
@@ -350,6 +369,10 @@ impl AppServerSession {
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn managed_new_thread_defaults(&self) -> Option<&NewThreadModelDefaults> {
|
||||
self.managed_new_thread_defaults.as_ref()
|
||||
}
|
||||
|
||||
/// Fetches the current account info without refreshing the auth token.
|
||||
///
|
||||
/// Used by both `bootstrap` (to populate the initial UI) and `get_login_status`
|
||||
|
||||
@@ -137,6 +137,7 @@ mod line_truncation;
|
||||
pub(crate) mod live_wrap;
|
||||
pub use live_wrap::RowBuilder;
|
||||
mod local_chatgpt_auth;
|
||||
mod managed_new_thread_defaults;
|
||||
mod markdown;
|
||||
mod markdown_render;
|
||||
mod markdown_stream;
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::legacy_core::config::ConfigOverrides;
|
||||
use codex_app_server_protocol::NewThreadModelDefaults;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
pub(crate) fn apply_managed_new_thread_defaults(
|
||||
config: &mut Config,
|
||||
defaults: Option<&NewThreadModelDefaults>,
|
||||
cli_kv_overrides: &[(String, TomlValue)],
|
||||
harness_overrides: &ConfigOverrides,
|
||||
) {
|
||||
let Some(defaults) = defaults else {
|
||||
return;
|
||||
};
|
||||
// Managed values are defaults rather than enforcement. Preserve explicit launch choices from
|
||||
// dedicated flags such as `-m` (`harness_overrides`) and generic `-c key=value` settings
|
||||
// (`cli_kv_overrides`), then fill only the fields that were not selected for this invocation.
|
||||
// Model and reasoning effort are a compatibility-sensitive pair, so an explicit override of
|
||||
// either opts out of both managed values. For example, `codex -m gpt-5.4` keeps that model and
|
||||
// its existing/default effort, while `-c model_reasoning_effort=low` does not switch to the
|
||||
// managed model. Service tier remains independent and is resolved against the selected model
|
||||
// before the thread starts.
|
||||
let has_cli_override = |key: &str| cli_kv_overrides.iter().any(|(path, _value)| path == key);
|
||||
let has_explicit_model_settings = harness_overrides.model.is_some()
|
||||
|| has_cli_override("model")
|
||||
|| has_cli_override("model_reasoning_effort");
|
||||
|
||||
if !has_explicit_model_settings && let Some(model) = defaults.model.as_ref() {
|
||||
config.model = Some(model.clone());
|
||||
}
|
||||
if !has_explicit_model_settings
|
||||
&& let Some(reasoning_effort) = defaults.model_reasoning_effort.as_ref()
|
||||
{
|
||||
config.model_reasoning_effort = Some(reasoning_effort.clone());
|
||||
}
|
||||
if harness_overrides.service_tier.is_none()
|
||||
&& !has_cli_override("service_tier")
|
||||
&& let Some(service_tier) = defaults.service_tier.as_ref()
|
||||
{
|
||||
config.service_tier = Some(
|
||||
ServiceTier::from_request_value(service_tier)
|
||||
.map(|tier| tier.request_value().to_string())
|
||||
.unwrap_or_else(|| service_tier.clone()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "managed_new_thread_defaults_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,110 @@
|
||||
use super::*;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
async fn test_config() -> Config {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir").keep();
|
||||
ConfigBuilder::default()
|
||||
.codex_home(codex_home)
|
||||
.build()
|
||||
.await
|
||||
.expect("config")
|
||||
}
|
||||
|
||||
fn defaults() -> NewThreadModelDefaults {
|
||||
NewThreadModelDefaults {
|
||||
model: Some("managed-model".to_string()),
|
||||
model_reasoning_effort: Some(ReasoningEffort::High),
|
||||
service_tier: Some("fast".to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn applies_managed_defaults_to_a_new_thread_config() {
|
||||
let mut actual = test_config().await;
|
||||
actual.model = Some("configured-model".to_string());
|
||||
actual.model_reasoning_effort = Some(ReasoningEffort::Low);
|
||||
actual.service_tier = Some("flex".to_string());
|
||||
let mut expected = actual.clone();
|
||||
expected.model = Some("managed-model".to_string());
|
||||
expected.model_reasoning_effort = Some(ReasoningEffort::High);
|
||||
expected.service_tier = Some(ServiceTier::Fast.request_value().to_string());
|
||||
|
||||
apply_managed_new_thread_defaults(
|
||||
&mut actual,
|
||||
Some(&defaults()),
|
||||
&[],
|
||||
&ConfigOverrides::default(),
|
||||
);
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_model_skips_managed_model_and_reasoning_effort() {
|
||||
let mut actual = test_config().await;
|
||||
actual.model = Some("explicit-model".to_string());
|
||||
actual.model_reasoning_effort = None;
|
||||
actual.service_tier = Some("flex".to_string());
|
||||
let mut expected = actual.clone();
|
||||
expected.service_tier = Some(ServiceTier::Fast.request_value().to_string());
|
||||
let harness_overrides = ConfigOverrides {
|
||||
model: Some("explicit-model".to_string()),
|
||||
..ConfigOverrides::default()
|
||||
};
|
||||
|
||||
apply_managed_new_thread_defaults(&mut actual, Some(&defaults()), &[], &harness_overrides);
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_reasoning_effort_skips_managed_model_and_reasoning_effort() {
|
||||
let mut actual = test_config().await;
|
||||
actual.model = Some("configured-model".to_string());
|
||||
actual.model_reasoning_effort = Some(ReasoningEffort::Low);
|
||||
actual.service_tier = Some("flex".to_string());
|
||||
let mut expected = actual.clone();
|
||||
expected.service_tier = Some(ServiceTier::Fast.request_value().to_string());
|
||||
let cli_kv_overrides = vec![(
|
||||
"model_reasoning_effort".to_string(),
|
||||
TomlValue::String("low".to_string()),
|
||||
)];
|
||||
|
||||
apply_managed_new_thread_defaults(
|
||||
&mut actual,
|
||||
Some(&defaults()),
|
||||
&cli_kv_overrides,
|
||||
&ConfigOverrides::default(),
|
||||
);
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_launch_overrides_take_precedence() {
|
||||
let mut actual = test_config().await;
|
||||
actual.model = Some("explicit-model".to_string());
|
||||
actual.model_reasoning_effort = Some(ReasoningEffort::Low);
|
||||
actual.service_tier = Some("flex".to_string());
|
||||
let expected = actual.clone();
|
||||
let cli_kv_overrides = vec![(
|
||||
"model_reasoning_effort".to_string(),
|
||||
TomlValue::String("low".to_string()),
|
||||
)];
|
||||
let harness_overrides = ConfigOverrides {
|
||||
model: Some("explicit-model".to_string()),
|
||||
service_tier: Some(Some("flex".to_string())),
|
||||
..ConfigOverrides::default()
|
||||
};
|
||||
|
||||
apply_managed_new_thread_defaults(
|
||||
&mut actual,
|
||||
Some(&defaults()),
|
||||
&cli_kv_overrides,
|
||||
&harness_overrides,
|
||||
);
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
Reference in New Issue
Block a user