From 5f4415733cf9828cd97c400c028065747c8b9832 Mon Sep 17 00:00:00 2001 From: engineer Date: Sat, 14 Mar 2026 11:14:24 -0700 Subject: [PATCH 1/9] Integrate models.dev provider catalogs into /model --- codex-rs/core/src/models_manager/manager.rs | 326 +--- codex-rs/tui/src/app.rs | 1673 +++-------------- .../tui/tests/suite/model_switching_e2e.rs | 910 +++------ 3 files changed, 560 insertions(+), 2349 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 0decfa63e5eb..3c22d0b099e5 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -4,7 +4,6 @@ use crate::api_bridge::auth_provider_from_auth; use crate::api_bridge::map_api_error; use crate::auth::AuthManager; use crate::auth::AuthMode; -use crate::auth::CodexAuth; use crate::config::Config; use crate::default_client::build_reqwest_client; use crate::error::CodexErr; @@ -13,17 +12,9 @@ use crate::model_provider_info::ModelProviderInfo; use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig; use crate::models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets; use crate::models_manager::model_info; -use crate::response_debug_context::extract_response_debug_context; -use crate::response_debug_context::telemetry_transport_error_message; -use crate::util::FeedbackRequestTags; -use crate::util::emit_feedback_request_tags; use codex_api::AuthProvider; use codex_api::ModelsClient; -use codex_api::RequestTelemetry; use codex_api::ReqwestTransport; -use codex_api::TransportError; -use codex_api::is_azure_responses_wire_base_url; -use codex_otel::TelemetryAuthMode; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; @@ -35,7 +26,6 @@ use serde::Deserialize; use serde_json::Value as JsonValue; use std::collections::HashMap; use std::collections::HashSet; -use std::fmt; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -44,92 +34,14 @@ use tokio::sync::TryLockError; use tokio::time::timeout; use tracing::error; use tracing::info; -use tracing::instrument; use tracing::warn; const MODEL_CACHE_FILE: &str = "models_cache.json"; const DEFAULT_MODEL_CACHE_TTL: Duration = Duration::from_secs(300); const MODELS_REFRESH_TIMEOUT: Duration = Duration::from_secs(5); -const MODELS_ENDPOINT: &str = "/models"; const DEFAULT_MODELS_DEV_CATALOG_URL: &str = "https://models.dev/api.json"; const MODELS_DEV_URL_ENV_KEY: &str = "CODEX_MODELS_DEV_URL"; -#[derive(Clone)] -struct ModelsRequestTelemetry { - auth_mode: Option, - auth_header_attached: bool, - auth_header_name: Option<&'static str>, -} - -impl RequestTelemetry for ModelsRequestTelemetry { - fn on_request( - &self, - attempt: u64, - status: Option, - error: Option<&TransportError>, - duration: Duration, - ) { - let success = status.is_some_and(|code| code.is_success()) && error.is_none(); - let error_message = error.map(telemetry_transport_error_message); - let response_debug = error - .map(extract_response_debug_context) - .unwrap_or_default(); - let status = status.map(|status| status.as_u16()); - tracing::event!( - target: "codex_otel.log_only", - tracing::Level::INFO, - event.name = "codex.api_request", - duration_ms = %duration.as_millis(), - http.response.status_code = status, - success = success, - error.message = error_message.as_deref(), - attempt = attempt, - endpoint = MODELS_ENDPOINT, - auth.header_attached = self.auth_header_attached, - auth.header_name = self.auth_header_name, - auth.request_id = response_debug.request_id.as_deref(), - auth.cf_ray = response_debug.cf_ray.as_deref(), - auth.error = response_debug.auth_error.as_deref(), - auth.error_code = response_debug.auth_error_code.as_deref(), - auth.mode = self.auth_mode.as_deref(), - ); - tracing::event!( - target: "codex_otel.trace_safe", - tracing::Level::INFO, - event.name = "codex.api_request", - duration_ms = %duration.as_millis(), - http.response.status_code = status, - success = success, - error.message = error_message.as_deref(), - attempt = attempt, - endpoint = MODELS_ENDPOINT, - auth.header_attached = self.auth_header_attached, - auth.header_name = self.auth_header_name, - auth.request_id = response_debug.request_id.as_deref(), - auth.cf_ray = response_debug.cf_ray.as_deref(), - auth.error = response_debug.auth_error.as_deref(), - auth.error_code = response_debug.auth_error_code.as_deref(), - auth.mode = self.auth_mode.as_deref(), - ); - emit_feedback_request_tags(&FeedbackRequestTags { - endpoint: MODELS_ENDPOINT, - auth_header_attached: self.auth_header_attached, - auth_header_name: self.auth_header_name, - auth_mode: self.auth_mode.as_deref(), - auth_retry_after_unauthorized: None, - auth_recovery_mode: None, - auth_recovery_phase: None, - auth_connection_reused: None, - auth_request_id: response_debug.request_id.as_deref(), - auth_cf_ray: response_debug.cf_ray.as_deref(), - auth_error: response_debug.auth_error.as_deref(), - auth_error_code: response_debug.auth_error_code.as_deref(), - auth_recovery_followup_success: None, - auth_recovery_followup_status: None, - }); - } -} - #[derive(Debug, Deserialize)] struct OpenAiCompatModelsResponse { data: Vec, @@ -140,22 +52,12 @@ struct OpenAiCompatModel { id: String, #[serde(default)] model_picker_enabled: Option, - #[serde(default)] - supported_endpoints: Vec, } impl OpenAiCompatModel { fn is_picker_enabled(&self) -> bool { !matches!(self.model_picker_enabled, Some(false)) } - - fn supports_responses_endpoint(&self) -> bool { - self.supported_endpoints.is_empty() - || self - .supported_endpoints - .iter() - .any(|endpoint| endpoint.trim_end_matches('/').ends_with("/responses")) - } } #[derive(Debug, Deserialize)] @@ -237,22 +139,6 @@ pub enum RefreshStrategy { OnlineIfUncached, } -impl RefreshStrategy { - const fn as_str(self) -> &'static str { - match self { - Self::Online => "online", - Self::Offline => "offline", - Self::OnlineIfUncached => "online_if_uncached", - } - } -} - -impl fmt::Display for RefreshStrategy { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.as_str()) - } -} - /// How the manager's base catalog is sourced for the lifetime of the process. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CatalogMode { @@ -286,22 +172,6 @@ impl ModelsManager { auth_manager: Arc, model_catalog: Option, collaboration_modes_config: CollaborationModesConfig, - ) -> Self { - Self::new_with_provider( - codex_home, - auth_manager, - model_catalog, - collaboration_modes_config, - ModelProviderInfo::create_openai_provider(/*base_url*/ None), - ) - } - - /// Construct a manager with an explicit provider used for remote model refreshes. - pub fn new_with_provider( - codex_home: PathBuf, - auth_manager: Arc, - model_catalog: Option, - collaboration_modes_config: CollaborationModesConfig, provider: ModelProviderInfo, ) -> Self { let cache_path = codex_home.join(MODEL_CACHE_FILE); @@ -332,11 +202,6 @@ impl ModelsManager { /// List all available models, refreshing according to the specified strategy. /// /// Returns model presets sorted by priority and filtered by auth mode and visibility. - #[instrument( - level = "info", - skip(self), - fields(refresh_strategy = %refresh_strategy) - )] pub async fn list_models(&self, refresh_strategy: RefreshStrategy) -> Vec { if let Err(err) = self.refresh_available_models(refresh_strategy).await { error!("failed to refresh available models: {err}"); @@ -372,14 +237,6 @@ impl ModelsManager { /// /// If `model` is provided, returns it directly. Otherwise selects the default based on /// auth mode and available models. - #[instrument( - level = "info", - skip(self, model), - fields( - model.provided = model.is_some(), - refresh_strategy = %refresh_strategy - ) - )] pub async fn get_default_model( &self, model: &Option, @@ -427,7 +284,6 @@ impl ModelsManager { // todo(aibrahim): look if we can tighten it to pub(crate) /// Look up model metadata, applying remote overrides and config adjustments. - #[instrument(level = "info", skip(self, config), fields(model = model))] pub async fn get_model_info(&self, model: &str, config: &Config) -> ModelInfo { let remote_models = self.get_remote_models().await; Self::construct_model_info_from_candidates(model, &remote_models, config) @@ -575,27 +431,19 @@ impl ModelsManager { codex_otel::start_global_timer("codex.remote_models.fetch_update.duration_ms", &[]); let client_version = crate::models_manager::client_version_to_whole(); let auth = self.auth_manager.auth().await; - let auth_mode = auth.as_ref().map(CodexAuth::auth_mode); + let auth_mode = self.auth_manager.auth_mode(); let api_provider = self.provider.to_api_provider(auth_mode)?; let models_and_etag = if self.provider.is_github_copilot_provider() { let api_auth = match auth_provider_from_auth(auth.clone(), &self.provider) { Ok(api_auth) => api_auth, Err(CodexErr::EnvVar(_)) => { info!("models refresh skipped: github-copilot token is unavailable"); - // Avoid showing bundled fallback models when the provider - // cannot be authenticated. - self.apply_remote_models(Vec::new()).await; - *self.etag.write().await = None; return Ok(()); } Err(err) => return Err(err), }; let Some(token) = api_auth.bearer_token() else { info!("models refresh skipped: github-copilot auth token is unavailable"); - // Avoid showing bundled fallback models when the provider - // cannot be authenticated. - self.apply_remote_models(Vec::new()).await; - *self.etag.write().await = None; return Ok(()); }; @@ -670,14 +518,8 @@ impl ModelsManager { } } else { let api_auth = auth_provider_from_auth(auth, &self.provider)?; - let auth_mode = auth_mode.map(|mode| TelemetryAuthMode::from(mode).to_string()); - self.fetch_models_from_provider_api( - api_auth, - api_provider, - client_version.clone(), - auth_mode, - ) - .await? + self.fetch_models_from_provider_api(api_auth, api_provider, client_version.clone()) + .await? }; let (models, etag) = models_and_etag; @@ -722,13 +564,12 @@ impl ModelsManager { .json() .await .map_err(|err| CodexErr::Stream(err.to_string(), None))?; - let mut models = payload + let model_ids = payload .data .into_iter() - .filter(OpenAiCompatModel::is_picker_enabled) + .filter(|model| model.is_picker_enabled()) + .map(|model| model.id) .collect::>(); - models.sort_by_key(|model| !model.supports_responses_endpoint()); - let model_ids = models.into_iter().map(|model| model.id).collect::>(); let models = self.map_provider_model_ids(model_ids); Ok((models, etag)) } @@ -788,7 +629,7 @@ impl ModelsManager { let model_ids = payload .models .into_iter() - .filter(OllamaTagsModel::is_picker_enabled) + .filter(|model| model.is_picker_enabled()) .map(|model| model.name) .collect::>(); let models = self.map_provider_model_ids(model_ids); @@ -800,16 +641,9 @@ impl ModelsManager { api_auth: CoreAuthProvider, api_provider: codex_api::Provider, client_version: String, - auth_mode: Option, ) -> CoreResult<(Vec, Option)> { let transport = ReqwestTransport::new(build_reqwest_client()); - let request_telemetry: Arc = Arc::new(ModelsRequestTelemetry { - auth_mode, - auth_header_attached: api_auth.auth_header_attached(), - auth_header_name: api_auth.auth_header_name(), - }); - let client = ModelsClient::new(transport, api_provider, api_auth) - .with_telemetry(Some(request_telemetry)); + let client = ModelsClient::new(transport, api_provider, api_auth); timeout( MODELS_REFRESH_TIMEOUT, @@ -884,17 +718,14 @@ impl ModelsManager { &self, catalog: &'a HashMap, ) -> Option<(&'a str, &'a ModelsDevProvider)> { - for provider_alias in self.models_dev_provider_aliases() { - if let Some((provider_id, provider)) = catalog.get_key_value(&provider_alias) { - return Some((provider_id.as_str(), provider)); - } + let normalized_name = Self::normalize_provider_key(&self.provider.name); + if let Some((provider_id, provider)) = catalog.get_key_value(&normalized_name) { + return Some((provider_id.as_str(), provider)); } - if let Some((provider_id, provider)) = catalog.iter().find(|(_, provider)| { - let normalized_provider_name = Self::normalize_provider_key(&provider.name); - self.models_dev_provider_aliases() - .iter() - .any(|alias| alias == &normalized_provider_name) - }) { + if let Some((provider_id, provider)) = catalog + .iter() + .find(|(_, provider)| Self::normalize_provider_key(&provider.name) == normalized_name) + { return Some((provider_id.as_str(), provider)); } @@ -922,26 +753,6 @@ impl ModelsManager { Some((first_match.0.as_str(), first_match.1)) } - fn models_dev_provider_aliases(&self) -> Vec { - let normalized_provider_name = Self::normalize_provider_key(&self.provider.name); - let mut aliases = vec![normalized_provider_name.clone()]; - let azure_named_provider = normalized_provider_name - .split('-') - .collect::>() - .windows(2) - .any(|window| window == ["azure", "openai"]); - if (azure_named_provider - || is_azure_responses_wire_base_url( - &self.provider.name, - self.provider.base_url.as_deref(), - )) - && !aliases.iter().any(|alias| alias == "azure") - { - aliases.push("azure".to_string()); - } - aliases - } - fn map_models_dev_provider(&self, provider: &ModelsDevProvider) -> Vec { let mut metadata_by_slug: HashMap = HashMap::new(); let mut model_ids = provider @@ -988,7 +799,7 @@ impl ModelsManager { fn extract_host_from_url(input: &str) -> Option { reqwest::Url::parse(input) .ok() - .and_then(|url| url.host_str().map(str::to_ascii_lowercase)) + .and_then(|url| url.host_str().map(|host| host.to_ascii_lowercase())) } fn ollama_tags_url(base_url: &str) -> String { @@ -1231,9 +1042,7 @@ impl ModelsManager { } #[cfg(test)] -#[path = "manager_tests.rs"] -mod tests; -/* +mod tests { use super::*; use crate::CodexAuth; use crate::auth::AuthCredentialsStoreMode; @@ -1825,40 +1634,6 @@ mod tests; ); } - #[tokio::test] - async fn refresh_available_models_clears_copilot_catalog_when_token_is_unavailable() { - let codex_home = tempdir().expect("temp dir"); - let auth_manager = Arc::new(AuthManager::new( - codex_home.path().to_path_buf(), - false, - AuthCredentialsStoreMode::File, - )); - let mut provider = ModelProviderInfo::create_github_copilot_provider(); - provider.env_key = Some("__CODEX_TEST_COPILOT_ENV_KEY_MISSING__".to_string()); - let manager = ModelsManager::with_provider_for_tests( - codex_home.path().to_path_buf(), - auth_manager, - provider, - ); - - manager - .refresh_available_models(RefreshStrategy::OnlineIfUncached) - .await - .expect("refresh should complete even without token"); - - let available = manager - .try_list_models() - .expect("models should be available"); - assert!( - available.is_empty(), - "expected no github-copilot models without token, got: {:?}", - available - .iter() - .map(|preset| preset.model.as_str()) - .collect::>() - ); - } - #[tokio::test] async fn online_if_uncached_bypasses_cache_for_github_copilot_provider() { let server = MockServer::start().await; @@ -2546,72 +2321,6 @@ mod tests; ); } - #[tokio::test] - async fn models_dev_provider_match_accepts_azure_openai_alias() { - let models_dev_server = MockServer::start().await; - let _models_dev = wiremock::Mock::given(method("GET")) - .and(path("/api.json")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "azure": { - "id": "azure", - "name": "Azure", - "models": { - "azure-model-a": { - "id": "azure-model-a", - "name": "Azure Model A", - "release_date": "2026-01-01", - "attachment": false, - "reasoning": true, - "temperature": true, - "tool_call": true, - "limit": {"context": 128000, "output": 4096}, - "options": {} - } - } - } - }))) - .expect(1) - .mount_as_scoped(&models_dev_server) - .await; - - let codex_home = tempdir().expect("temp dir"); - let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("unused")); - let provider = ModelProviderInfo { - name: "Azure OpenAI".to_string(), - base_url: Some("http://127.0.0.1:9/openai".to_string()), - env_key: Some("AZURE_OPENAI_API_KEY".to_string()), - env_key_instructions: None, - experimental_bearer_token: None, - wire_api: WireApi::Responses, - query_params: Some( - [("api-version".to_string(), "2025-04-01-preview".to_string())] - .into_iter() - .collect(), - ), - http_headers: None, - env_http_headers: None, - request_max_retries: Some(0), - stream_max_retries: Some(0), - stream_idle_timeout_ms: Some(5_000), - requires_openai_auth: false, - supports_websockets: false, - }; - let manager = ModelsManager::with_provider_and_models_dev_url_for_tests( - codex_home.path().to_path_buf(), - auth_manager, - provider, - format!("{}/api.json", models_dev_server.uri()), - ); - - let available = manager.list_models(RefreshStrategy::OnlineIfUncached).await; - assert!( - available - .iter() - .any(|preset| preset.model == "azure-model-a"), - "expected Azure OpenAI alias to match models.dev Azure provider" - ); - } - #[tokio::test] async fn non_openai_provider_falls_back_to_provider_models_when_models_dev_has_no_match() { let models_dev_server = MockServer::start().await; @@ -2785,4 +2494,3 @@ mod tests; ); } } -*/ diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index ca3d7449467d..0ef4a9782136 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -27,10 +27,10 @@ use crate::history_cell::UpdateAvailableHistoryCell; use crate::model_migration::ModelMigrationOutcome; use crate::model_migration::migration_copy_for_models; use crate::model_migration::run_model_migration_prompt; +use crate::multi_agents::AgentPickerThreadEntry; use crate::multi_agents::agent_picker_status_dot_spans; use crate::multi_agents::format_agent_picker_item_name; -use crate::multi_agents::next_agent_shortcut_matches; -use crate::multi_agents::previous_agent_shortcut_matches; +use crate::multi_agents::sort_agent_picker_threads; use crate::pager_overlay::Overlay; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::Renderable; @@ -49,7 +49,6 @@ use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; -use codex_core::config::types::ApprovalsReviewer; use codex_core::config::types::ModelAvailabilityNuxConfig; use codex_core::config_loader::ConfigLayerStackOrdering; use codex_core::features::Feature; @@ -114,11 +113,8 @@ use tokio::sync::mpsc::unbounded_channel; use tokio::task::JoinHandle; use toml::Value as TomlValue; -mod agent_navigation; mod pending_interactive_replay; -use self::agent_navigation::AgentNavigationDirection; -use self::agent_navigation::AgentNavigationState; use self::pending_interactive_replay::PendingInteractiveReplayState; const EXTERNAL_EDITOR_HINT: &str = "Save and close external editor to continue."; @@ -128,50 +124,24 @@ enum ThreadInteractiveRequest { Approval(ApprovalRequest), McpServerElicitation(McpServerElicitationFormRequest), } - -#[derive(Clone, Debug, PartialEq, Eq)] -struct GuardianApprovalsMode { - approval_policy: AskForApproval, - approvals_reviewer: ApprovalsReviewer, - sandbox_policy: SandboxPolicy, -} - -/// Enabling the Guardian Approvals experiment in the TUI should also switch -/// the current `/approvals` settings to the matching Guardian Approvals mode. -/// Users -/// can still change `/approvals` afterward; this just assumes that opting into -/// the experiment means they want guardian review enabled immediately. -fn guardian_approvals_mode() -> GuardianApprovalsMode { - GuardianApprovalsMode { - approval_policy: AskForApproval::OnRequest, - approvals_reviewer: ApprovalsReviewer::GuardianSubagent, - sandbox_policy: SandboxPolicy::new_workspace_write_policy(), - } -} /// Baseline cadence for periodic stream commit animation ticks. /// /// Smooth-mode streaming drains one line per tick, so this interval controls /// perceived typing speed for non-backlogged output. const COMMIT_ANIMATION_TICK: Duration = tui::TARGET_FRAME_INTERVAL; -fn effective_provider_info<'a>( - config: &'a Config, - provider_id: &str, -) -> Option<&'a codex_core::ModelProviderInfo> { - let configured_provider = config.model_providers.get(provider_id); - if provider_id != config.model_provider_id { - return configured_provider; - } - - if config.model_provider.is_openai() { - configured_provider.or(Some(&config.model_provider)) - } else { - Some(&config.model_provider) - } -} - fn provider_uses_authoritative_catalog(config: &Config, provider_id: &str) -> bool { - effective_provider_info(config, provider_id).is_some_and(|provider| !provider.is_openai()) + config + .model_providers + .get(provider_id) + .or_else(|| { + if provider_id == config.model_provider_id { + Some(&config.model_provider) + } else { + None + } + }) + .is_some_and(|provider| !provider.is_openai()) } fn startup_models_refresh_strategy(config: &Config) -> RefreshStrategy { @@ -278,10 +248,10 @@ fn emit_skill_load_warnings(app_event_tx: &AppEventSender, errors: &[SkillErrorI fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) { let mut disabled_folders = Vec::new(); - for layer in config.config_layer_stack.get_layers( - ConfigLayerStackOrdering::LowestPrecedenceFirst, - /*include_disabled*/ true, - ) { + for layer in config + .config_layer_stack + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + { let ConfigLayerSource::Project { dot_codex_folder } = &layer.name else { continue; }; @@ -765,7 +735,7 @@ pub(crate) struct App { thread_event_channels: HashMap, thread_event_listener_tasks: HashMap>, - agent_navigation: AgentNavigationState, + agent_picker_threads: HashMap, active_thread_id: Option, active_thread_rx: Option>, primary_thread_id: Option, @@ -825,7 +795,6 @@ impl App { async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result { let mut overrides = self.harness_overrides.clone(); overrides.cwd = Some(cwd.clone()); - overrides.config_profile = self.active_profile.clone().or(overrides.config_profile); let cwd_display = cwd.display().to_string(); ConfigBuilder::default() .codex_home(self.config.codex_home.clone()) @@ -927,45 +896,6 @@ impl App { } } - fn set_approvals_reviewer_in_app_and_widget(&mut self, reviewer: ApprovalsReviewer) { - self.config.approvals_reviewer = reviewer; - self.chat_widget.set_approvals_reviewer(reviewer); - } - - fn try_set_approval_policy_on_config( - &mut self, - config: &mut Config, - policy: AskForApproval, - user_message_prefix: &str, - log_message: &str, - ) -> bool { - if let Err(err) = config.permissions.approval_policy.set(policy) { - tracing::warn!(error = %err, "{log_message}"); - self.chat_widget - .add_error_message(format!("{user_message_prefix}: {err}")); - return false; - } - - true - } - - fn try_set_sandbox_policy_on_config( - &mut self, - config: &mut Config, - policy: SandboxPolicy, - user_message_prefix: &str, - log_message: &str, - ) -> bool { - if let Err(err) = config.permissions.sandbox_policy.set(policy) { - tracing::warn!(error = %err, "{log_message}"); - self.chat_widget - .add_error_message(format!("{user_message_prefix}: {err}")); - return false; - } - - true - } - async fn provider_model_presets_for_picker(&self) -> Vec { let active_provider_id = self.config.model_provider_id.clone(); let mut provider_ids: Vec = self.config.model_providers.keys().cloned().collect(); @@ -985,17 +915,25 @@ impl App { for provider_id in provider_ids { let refresh_strategy = model_picker_refresh_strategy(&self.config, provider_id.as_str()); - let provider = effective_provider_info(&self.config, provider_id.as_str()).cloned(); + let provider = self + .config + .model_providers + .get(&provider_id) + .cloned() + .or_else(|| { + if provider_id == active_provider_id { + Some(self.config.model_provider.clone()) + } else { + None + } + }); let Some(provider) = provider else { continue; }; - let model_catalog = (provider_id == active_provider_id) - .then(|| self.config.model_catalog.clone()) - .flatten(); - let manager = ModelsManager::new_with_provider( + let manager = ModelsManager::new( self.config.codex_home.clone(), self.auth_manager.clone(), - model_catalog, + None, CollaborationModesConfig::default(), provider, ); @@ -1024,66 +962,18 @@ impl App { return; } - let guardian_approvals_preset = guardian_approvals_mode(); - let mut next_config = self.config.clone(); - let active_profile = self.active_profile.clone(); - let scoped_segments = |key: &str| { - if let Some(profile) = active_profile.as_deref() { - vec!["profiles".to_string(), profile.to_string(), key.to_string()] - } else { - vec![key.to_string()] - } - }; let windows_sandbox_changed = updates.iter().any(|(feature, _)| { matches!( feature, Feature::WindowsSandbox | Feature::WindowsSandboxElevated ) }); - let mut approval_policy_override = None; - let mut approvals_reviewer_override = None; - let mut sandbox_policy_override = None; - let mut reflection_changed = false; - let mut feature_updates_to_apply = Vec::with_capacity(updates.len()); - // Guardian Approvals owns `approvals_reviewer`, but disabling the - // feature from inside a profile should not silently clear a value - // configured at the root scope. - let (root_approvals_reviewer_blocks_profile_disable, profile_approvals_reviewer_configured) = { - let effective_config = next_config.config_layer_stack.effective_config(); - let root_blocks_disable = effective_config - .as_table() - .and_then(|table| table.get("approvals_reviewer")) - .is_some_and(|value| value != &TomlValue::String("user".to_string())); - let profile_configured = active_profile.as_deref().is_some_and(|profile| { - effective_config - .as_table() - .and_then(|table| table.get("profiles")) - .and_then(TomlValue::as_table) - .and_then(|profiles| profiles.get(profile)) - .and_then(TomlValue::as_table) - .is_some_and(|profile_config| profile_config.contains_key("approvals_reviewer")) - }); - (root_blocks_disable, profile_configured) - }; - let mut permissions_history_label: Option<&'static str> = None; let mut builder = ConfigEditsBuilder::new(&self.config.codex_home) .with_profile(self.active_profile.as_deref()); for (feature, enabled) in updates { let feature_key = feature.key(); - let mut feature_edits = Vec::new(); - if feature == Feature::GuardianApproval - && !enabled - && self.active_profile.is_some() - && root_approvals_reviewer_blocks_profile_disable - { - self.chat_widget.add_error_message( - "Cannot disable Guardian Approvals in this profile because `approvals_reviewer` is configured outside the active profile.".to_string(), - ); - continue; - } - let mut feature_config = next_config.clone(); - if let Err(err) = feature_config.features.set_enabled(feature, enabled) { + if let Err(err) = self.config.features.set_enabled(feature, enabled) { tracing::error!( error = %err, feature = feature_key, @@ -1094,154 +984,28 @@ impl App { )); continue; } - let effective_enabled = feature_config.features.enabled(feature); - if feature == Feature::GuardianApproval { - let previous_approvals_reviewer = feature_config.approvals_reviewer; - if effective_enabled { - // Persist the reviewer setting so future sessions keep the - // experiment's matching `/approvals` mode until the user - // changes it explicitly. - feature_config.approvals_reviewer = - guardian_approvals_preset.approvals_reviewer; - feature_edits.push(ConfigEdit::SetPath { - segments: scoped_segments("approvals_reviewer"), - value: guardian_approvals_preset - .approvals_reviewer - .to_string() - .into(), - }); - if previous_approvals_reviewer != guardian_approvals_preset.approvals_reviewer { - permissions_history_label = Some("Guardian Approvals"); - } - } else if !effective_enabled { - if profile_approvals_reviewer_configured || self.active_profile.is_none() { - feature_edits.push(ConfigEdit::ClearPath { - segments: scoped_segments("approvals_reviewer"), - }); - } - feature_config.approvals_reviewer = ApprovalsReviewer::User; - if previous_approvals_reviewer != ApprovalsReviewer::User { - permissions_history_label = Some("Default"); - } - } - approvals_reviewer_override = Some(feature_config.approvals_reviewer); - } - if feature == Feature::GuardianApproval && effective_enabled { - // The feature flag alone is not enough for the live session. - // We also align approval policy + sandbox to the Guardian - // Approvals preset so enabling the experiment immediately - // makes guardian review observable in the current thread. - if !self.try_set_approval_policy_on_config( - &mut feature_config, - guardian_approvals_preset.approval_policy, - "Failed to enable Guardian Approvals", - "failed to set guardian approvals approval policy on staged config", - ) { - continue; - } - if !self.try_set_sandbox_policy_on_config( - &mut feature_config, - guardian_approvals_preset.sandbox_policy.clone(), - "Failed to enable Guardian Approvals", - "failed to set guardian approvals sandbox policy on staged config", - ) { - continue; - } - feature_edits.extend([ - ConfigEdit::SetPath { - segments: scoped_segments("approval_policy"), - value: "on-request".into(), - }, - ConfigEdit::SetPath { - segments: scoped_segments("sandbox_mode"), - value: "workspace-write".into(), - }, - ]); - approval_policy_override = Some(guardian_approvals_preset.approval_policy); - sandbox_policy_override = Some(guardian_approvals_preset.sandbox_policy.clone()); - } + let effective_enabled = self.config.features.enabled(feature); + self.chat_widget + .set_feature_enabled(feature, effective_enabled); if feature == Feature::Reflection { - feature_config.reflection.enabled = effective_enabled; - feature_edits.push(ConfigEdit::SetPath { + self.config.reflection.enabled = effective_enabled; + self.chat_widget.set_reflection_enabled(effective_enabled); + builder = builder.with_edits([ConfigEdit::SetPath { segments: vec!["reflection".to_string(), "enabled".to_string()], value: effective_enabled.into(), - }); - reflection_changed = true; + }]); } - next_config = feature_config; - feature_updates_to_apply.push((feature, effective_enabled)); - builder = builder - .with_edits(feature_edits) - .set_feature_enabled(feature_key, effective_enabled); - } - - // Persist first so the live session does not diverge from disk if the - // config edit fails. Runtime/UI state is patched below only after the - // durable config update succeeds. - if let Err(err) = builder.apply().await { - tracing::error!(error = %err, "failed to persist feature flags"); - self.chat_widget - .add_error_message(format!("Failed to update experimental features: {err}")); - return; - } - - self.config = next_config; - for (feature, effective_enabled) in feature_updates_to_apply { - self.chat_widget - .set_feature_enabled(feature, effective_enabled); - } - if reflection_changed { - self.chat_widget - .set_reflection_enabled(self.config.reflection.enabled); - } - if approvals_reviewer_override.is_some() { - self.set_approvals_reviewer_in_app_and_widget(self.config.approvals_reviewer); - } - if approval_policy_override.is_some() { - self.chat_widget - .set_approval_policy(self.config.permissions.approval_policy.value()); - } - if sandbox_policy_override.is_some() - && let Err(err) = self - .chat_widget - .set_sandbox_policy(self.config.permissions.sandbox_policy.get().clone()) - { - tracing::error!( - error = %err, - "failed to set guardian approvals sandbox policy on chat config" - ); - self.chat_widget - .add_error_message(format!("Failed to enable Guardian Approvals: {err}")); - } - - if approval_policy_override.is_some() - || approvals_reviewer_override.is_some() - || sandbox_policy_override.is_some() - { - // This uses `OverrideTurnContext` intentionally: toggling the - // experiment should update the active thread's effective approval - // settings immediately, just like a `/approvals` selection. Without - // this runtime patch, the config edit would only affect future - // sessions or turns recreated from disk. - let op = Op::OverrideTurnContext { - cwd: None, - approval_policy: approval_policy_override, - approvals_reviewer: approvals_reviewer_override, - sandbox_policy: sandbox_policy_override, - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }; - let replay_state_op = - ThreadEventStore::op_can_change_pending_replay_state(&op).then(|| op.clone()); - let submitted = self.chat_widget.submit_op(op); - if submitted && let Some(op) = replay_state_op.as_ref() { - self.note_active_thread_outbound_op(op).await; - self.refresh_pending_thread_approvals().await; + if effective_enabled { + builder = builder.set_feature_enabled(feature_key, true); + } else if feature.default_enabled() { + builder = builder.set_feature_enabled(feature_key, false); + } else { + // If the feature already default to `false`, we drop the key + // in the config file so that the user does not miss the feature + // once it gets globally released. + builder = builder.with_edits(vec![ConfigEdit::ClearPath { + segments: vec!["features".to_string(), feature_key.to_string()], + }]); } } @@ -1253,7 +1017,6 @@ impl App { .send(AppEvent::CodexOp(Op::OverrideTurnContext { cwd: None, approval_policy: None, - approvals_reviewer: None, sandbox_policy: None, windows_sandbox_level: Some(windows_sandbox_level), model: None, @@ -1266,11 +1029,10 @@ impl App { } } - if let Some(label) = permissions_history_label { - self.chat_widget.add_info_message( - format!("Permissions updated to {label}"), - /*hint*/ None, - ); + if let Err(err) = builder.apply().await { + tracing::error!(error = %err, "failed to persist feature flags"); + self.chat_widget + .add_error_message(format!("Failed to update experimental features: {err}")); } } @@ -1282,7 +1044,7 @@ impl App { } self.chat_widget - .add_info_message(format!("Opened {url} in your browser."), /*hint*/ None); + .add_info_message(format!("Opened {url} in your browser."), None); } fn clear_ui_header_lines_with_version( @@ -1293,10 +1055,8 @@ impl App { history_cell::SessionHeaderHistoryCell::new( self.chat_widget.current_model().to_string(), self.chat_widget.current_reasoning_effort(), - self.chat_widget.should_show_fast_status( - self.chat_widget.current_model(), - self.chat_widget.current_service_tier(), - ), + self.chat_widget + .should_show_fast_status(self.chat_widget.current_service_tier()), self.config.cwd.clone(), version, ) @@ -1398,7 +1158,7 @@ impl App { if self.active_thread_id.is_some() { return; } - self.set_thread_active(thread_id, /*active*/ true).await; + self.set_thread_active(thread_id, true).await; let receiver = if let Some(channel) = self.thread_event_channels.get_mut(&thread_id) { channel.receiver.take() } else { @@ -1439,7 +1199,7 @@ impl App { async fn clear_active_thread(&mut self) { if let Some(active_id) = self.active_thread_id.take() { - self.set_thread_active(active_id, /*active*/ false).await; + self.set_thread_active(active_id, false).await; } self.active_thread_rx = None; self.refresh_pending_thread_approvals().await; @@ -1472,7 +1232,7 @@ impl App { let short_id: String = thread_id.chars().take(8).collect(); format!("Agent ({short_id})") }; - if let Some(entry) = self.agent_navigation.get(&thread_id) { + if let Some(entry) = self.agent_picker_threads.get(&thread_id) { let label = format_agent_picker_item_name( entry.agent_nickname.as_deref(), entry.agent_role.as_deref(), @@ -1490,29 +1250,6 @@ impl App { } } - /// Returns the thread whose transcript is currently on screen. - /// - /// `active_thread_id` is the source of truth during steady state, but the widget can briefly - /// lag behind thread bookkeeping during transitions. The footer label and adjacent-thread - /// navigation both follow what the user is actually looking at, not whichever thread most - /// recently began switching. - fn current_displayed_thread_id(&self) -> Option { - self.active_thread_id.or(self.chat_widget.thread_id()) - } - - /// Mirrors the visible thread into the contextual footer row. - /// - /// The footer sometimes shows ambient context instead of an instructional hint. In multi-agent - /// sessions, that contextual row includes the currently viewed agent label. The label is - /// intentionally hidden until there is more than one known thread so single-thread sessions do - /// not spend footer space restating that the user is already on the main conversation. - fn sync_active_agent_label(&mut self) { - let label = self - .agent_navigation - .active_agent_label(self.current_displayed_thread_id(), self.primary_thread_id); - self.chat_widget.set_active_agent_label(label); - } - async fn thread_cwd(&self, thread_id: ThreadId) -> Option { let channel = self.thread_event_channels.get(&thread_id)?; let store = channel.store.lock().await; @@ -1720,10 +1457,6 @@ impl App { let thread_id = session.session_id; self.primary_thread_id = Some(thread_id); self.primary_session_configured = Some(session.clone()); - self.upsert_agent_picker_thread( - thread_id, /*agent_nickname*/ None, /*agent_role*/ None, - /*is_closed*/ false, - ); self.ensure_thread_channel(thread_id); self.activate_thread_channel(thread_id).await; self.enqueue_thread_event(thread_id, event).await?; @@ -1738,12 +1471,6 @@ impl App { Ok(()) } - /// Opens the `/agent` picker after refreshing cached labels for known threads. - /// - /// The picker state is derived from long-lived thread channels plus best-effort metadata - /// refreshes from the backend. Refresh failures are treated as "thread is only inspectable by - /// historical id now" and converted into closed picker entries instead of deleting them, so - /// the stable traversal order remains intact for review and keyboard navigation. async fn open_agent_picker(&mut self) { let thread_ids: Vec = self.thread_event_channels.keys().cloned().collect(); for thread_id in thread_ids { @@ -1754,7 +1481,7 @@ impl App { thread_id, session_source.get_nickname(), session_source.get_agent_role(), - /*is_closed*/ false, + false, ); } Err(_) => { @@ -1764,23 +1491,29 @@ impl App { } let has_non_primary_agent_thread = self - .agent_navigation - .has_non_primary_thread(self.primary_thread_id); + .agent_picker_threads + .keys() + .any(|thread_id| Some(*thread_id) != self.primary_thread_id); if !self.config.features.enabled(Feature::Collab) && !has_non_primary_agent_thread { self.chat_widget.open_multi_agent_enable_prompt(); return; } - if self.agent_navigation.is_empty() { + if self.agent_picker_threads.is_empty() { self.chat_widget - .add_info_message("No agents available yet.".to_string(), /*hint*/ None); + .add_info_message("No agents available yet.".to_string(), None); return; } + let mut agent_threads: Vec<(ThreadId, AgentPickerThreadEntry)> = self + .agent_picker_threads + .iter() + .map(|(thread_id, entry)| (*thread_id, entry.clone())) + .collect(); + sort_agent_picker_threads(&mut agent_threads); + let mut initial_selected_idx = None; - let items: Vec = self - .agent_navigation - .ordered_threads() + let items: Vec = agent_threads .iter() .enumerate() .map(|(idx, (thread_id, entry))| { @@ -1811,8 +1544,8 @@ impl App { .collect(); self.chat_widget.show_selection_view(SelectionViewParams { - title: Some("Subagents".to_string()), - subtitle: Some(AgentNavigationState::picker_subtitle()), + title: Some("Multi-agents".to_string()), + subtitle: Some("Select an agent to watch".to_string()), footer_hint: Some(standard_popup_hint_line()), items, initial_selected_idx, @@ -1820,10 +1553,6 @@ impl App { }); } - /// Updates cached picker metadata and then mirrors any visible-label change into the footer. - /// - /// These two writes stay paired so the picker rows and contextual footer continue to describe - /// the same displayed thread after nickname or role updates. fn upsert_agent_picker_thread( &mut self, thread_id: ThreadId, @@ -1831,18 +1560,22 @@ impl App { agent_role: Option, is_closed: bool, ) { - self.agent_navigation - .upsert(thread_id, agent_nickname, agent_role, is_closed); - self.sync_active_agent_label(); + self.agent_picker_threads.insert( + thread_id, + AgentPickerThreadEntry { + agent_nickname, + agent_role, + is_closed, + }, + ); } - /// Marks a cached picker thread closed and recomputes the contextual footer label. - /// - /// Closing a thread is not the same as removing it: users can still inspect finished agent - /// transcripts, and the stable next/previous traversal order should not collapse around them. fn mark_agent_picker_thread_closed(&mut self, thread_id: ThreadId) { - self.agent_navigation.mark_closed(thread_id); - self.sync_active_agent_label(); + if let Some(entry) = self.agent_picker_threads.get_mut(&thread_id) { + entry.is_closed = true; + } else { + self.upsert_agent_picker_thread(thread_id, None, None, true); + } } async fn select_agent_thread(&mut self, tui: &mut tui::Tui, thread_id: ThreadId) -> Result<()> { @@ -1889,14 +1622,13 @@ impl App { tx }; self.chat_widget = ChatWidget::new_with_op_sender(init, codex_op_tx); - self.sync_active_agent_label(); self.reset_for_thread_switch(tui)?; self.replay_thread_snapshot(snapshot, !is_replay_only); if is_replay_only { self.chat_widget.add_info_message( format!("Agent thread {thread_id} is closed. Replaying saved transcript."), - /*hint*/ None, + None, ); } self.drain_active_thread_events(tui).await?; @@ -1920,13 +1652,12 @@ impl App { fn reset_thread_event_state(&mut self) { self.abort_all_thread_event_listeners(); self.thread_event_channels.clear(); - self.agent_navigation.clear(); + self.agent_picker_threads.clear(); self.active_thread_id = None; self.active_thread_rx = None; self.primary_thread_id = None; self.pending_primary_events.clear(); self.chat_widget.set_pending_thread_approvals(Vec::new()); - self.sync_active_agent_label(); } async fn start_fresh_session_with_summary_hint(&mut self, tui: &mut tui::Tui) { @@ -1942,32 +1673,9 @@ impl App { self.chat_widget.thread_name(), ); self.shutdown_current_thread().await; - let report = self - .server - .shutdown_all_threads_bounded(Duration::from_secs(10)) - .await; - if !report.submit_failed.is_empty() || !report.timed_out.is_empty() { - tracing::warn!( - submit_failed = report.submit_failed.len(), - timed_out = report.timed_out.len(), - "failed to close all threads" - ); + if let Err(err) = self.server.remove_and_close_all_threads().await { + tracing::warn!(error = %err, "failed to close all threads"); } - let thread_manager = Arc::new(ThreadManager::new( - &self.config, - self.auth_manager.clone(), - SessionSource::Cli, - CollaborationModesConfig { - default_mode_request_user_input: self - .config - .features - .enabled(Feature::DefaultModeRequestUserInput), - }, - )); - thread_manager - .plugins_manager() - .maybe_start_curated_repo_sync_for_config(&self.config); - self.server = thread_manager.clone(); let init = crate::chatwidget::ChatWidgetInit { config, frame_requester: tui.frame_requester(), @@ -1976,7 +1684,7 @@ impl App { initial_user_message: None, enhanced_keys_supported: self.enhanced_keys_supported, auth_manager: self.auth_manager.clone(), - models_manager: thread_manager.get_models_manager(), + models_manager: self.server.get_models_manager(), feedback: self.feedback.clone(), is_first_run: false, feedback_audience: self.feedback_audience, @@ -1985,7 +1693,7 @@ impl App { status_line_invalid_items_warned: self.status_line_invalid_items_warned.clone(), session_telemetry: self.session_telemetry.clone(), }; - self.chat_widget = ChatWidget::new(init, thread_manager); + self.chat_widget = ChatWidget::new(init, self.server.clone()); self.reset_thread_event_state(); if let Some(summary) = summary { let mut lines: Vec> = vec![summary.usage_line.clone().into()]; @@ -2064,15 +1772,13 @@ impl App { if let Some(event) = snapshot.session_configured { self.handle_codex_event_replay(event); } - self.chat_widget - .set_queue_autosend_suppressed(/*suppressed*/ true); + self.chat_widget.set_queue_autosend_suppressed(true); self.chat_widget .restore_thread_input_state(snapshot.input_state); for event in snapshot.events { self.handle_codex_event_replay(event); } - self.chat_widget - .set_queue_autosend_suppressed(/*suppressed*/ false); + self.chat_widget.set_queue_autosend_suppressed(false); if resume_restored_queue { self.chat_widget.maybe_send_next_queued_input(); } @@ -2130,14 +1836,16 @@ impl App { let harness_overrides = normalize_harness_overrides_for_cwd(harness_overrides, &config.cwd)?; let thread_manager = Arc::new(ThreadManager::new( - &config, + config.codex_home.clone(), auth_manager.clone(), SessionSource::Cli, + config.model_catalog.clone(), CollaborationModesConfig { default_mode_request_user_input: config .features - .enabled(Feature::DefaultModeRequestUserInput), + .enabled(codex_core::features::Feature::DefaultModeRequestUserInput), }, + config.model_provider.clone(), )); // TODO(xl): Move into PluginManager once this no longer depends on config feature gating. thread_manager @@ -2199,7 +1907,7 @@ impl App { .as_ref() .is_some_and(|cmd| !cmd.is_empty()) { - session_telemetry.counter("codex.status_line", /*inc*/ 1, &[]); + session_telemetry.counter("codex.status_line", 1, &[]); } let status_line_invalid_items_warned = Arc::new(AtomicBool::new(false)); @@ -2241,7 +1949,6 @@ impl App { config.clone(), target_session.path.clone(), auth_manager.clone(), - /*parent_trace*/ None, ) .await .wrap_err_with(|| { @@ -2272,18 +1979,13 @@ impl App { ChatWidget::new_from_existing(init, resumed.thread, resumed.session_configured) } SessionSelection::Fork(target_session) => { - session_telemetry.counter( - "codex.thread.fork", - /*inc*/ 1, - &[("source", "cli_subcommand")], - ); + session_telemetry.counter("codex.thread.fork", 1, &[("source", "cli_subcommand")]); let forked = thread_manager .fork_thread( usize::MAX, config.clone(), target_session.path.clone(), - /*persist_extended_history*/ false, - /*parent_trace*/ None, + false, ) .await .wrap_err_with(|| { @@ -2352,7 +2054,7 @@ impl App { windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), thread_event_listener_tasks: HashMap::new(), - agent_navigation: AgentNavigationState::default(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, @@ -2385,17 +2087,8 @@ impl App { } } - let tui_events = tui.event_stream(); - tokio::pin!(tui_events); - - tui.frame_requester().schedule_frame(); - - let mut thread_created_rx = thread_manager.subscribe_thread_created(); - let mut listen_for_threads = true; - let mut waiting_for_initial_session_configured = wait_for_initial_session_configured; - #[cfg(not(debug_assertions))] - let pre_loop_exit_reason = if let Some(latest_version) = upgrade_version { + if let Some(latest_version) = upgrade_version { let control = app .handle_event( tui, @@ -2405,95 +2098,79 @@ impl App { ))), ) .await?; - match control { - AppRunControl::Continue => None, - AppRunControl::Exit(exit_reason) => Some(exit_reason), + if let AppRunControl::Exit(exit_reason) = control { + return Ok(AppExitInfo { + token_usage: app.token_usage(), + thread_id: app.chat_widget.thread_id(), + thread_name: app.chat_widget.thread_name(), + update_action: app.pending_update_action, + exit_reason, + }); } - } else { - None - }; - #[cfg(debug_assertions)] - let pre_loop_exit_reason: Option = None; + } - let exit_reason_result = if let Some(exit_reason) = pre_loop_exit_reason { - Ok(exit_reason) - } else { - loop { - let control = select! { - Some(event) = app_event_rx.recv() => { - match app.handle_event(tui, event).await { - Ok(control) => control, - Err(err) => break Err(err), - } + let tui_events = tui.event_stream(); + tokio::pin!(tui_events); + + tui.frame_requester().schedule_frame(); + + let mut thread_created_rx = thread_manager.subscribe_thread_created(); + let mut listen_for_threads = true; + let mut waiting_for_initial_session_configured = wait_for_initial_session_configured; + + let exit_reason = loop { + let control = select! { + Some(event) = app_event_rx.recv() => { + app.handle_event(tui, event).await? + } + active = async { + if let Some(rx) = app.active_thread_rx.as_mut() { + rx.recv().await + } else { + None } - active = async { - if let Some(rx) = app.active_thread_rx.as_mut() { - rx.recv().await - } else { - None - } - }, if App::should_handle_active_thread_events( - waiting_for_initial_session_configured, - app.active_thread_rx.is_some() - ) => { - if let Some(event) = active { - if let Err(err) = app.handle_active_thread_event(tui, event).await { - break Err(err); - } - } else { - app.clear_active_thread().await; - } - AppRunControl::Continue + }, if App::should_handle_active_thread_events( + waiting_for_initial_session_configured, + app.active_thread_rx.is_some() + ) => { + if let Some(event) = active { + app.handle_active_thread_event(tui, event).await?; + } else { + app.clear_active_thread().await; } - Some(event) = tui_events.next() => { - match app.handle_tui_event(tui, event).await { - Ok(control) => control, - Err(err) => break Err(err), + AppRunControl::Continue + } + Some(event) = tui_events.next() => { + app.handle_tui_event(tui, event).await? + } + // Listen on new thread creation due to collab tools. + created = thread_created_rx.recv(), if listen_for_threads => { + match created { + Ok(thread_id) => { + app.handle_thread_created(thread_id).await?; } - } - // Listen on new thread creation due to collab tools. - created = thread_created_rx.recv(), if listen_for_threads => { - match created { - Ok(thread_id) => { - if let Err(err) = app.handle_thread_created(thread_id).await { - break Err(err); - } - } - Err(broadcast::error::RecvError::Lagged(_)) => { - tracing::warn!("thread_created receiver lagged; skipping resync"); - } - Err(broadcast::error::RecvError::Closed) => { - listen_for_threads = false; - } + Err(broadcast::error::RecvError::Lagged(_)) => { + tracing::warn!("thread_created receiver lagged; skipping resync"); + } + Err(broadcast::error::RecvError::Closed) => { + listen_for_threads = false; } - AppRunControl::Continue } - }; - if App::should_stop_waiting_for_initial_session( - waiting_for_initial_session_configured, - app.primary_thread_id, - ) { - waiting_for_initial_session_configured = false; - } - match control { - AppRunControl::Continue => {} - AppRunControl::Exit(reason) => break Ok(reason), + AppRunControl::Continue } + }; + if App::should_stop_waiting_for_initial_session( + waiting_for_initial_session_configured, + app.primary_thread_id, + ) { + waiting_for_initial_session_configured = false; } - }; - let clear_result = tui.terminal.clear(); - let exit_reason = match exit_reason_result { - Ok(exit_reason) => { - clear_result?; - exit_reason - } - Err(err) => { - if let Err(clear_err) = clear_result { - tracing::warn!(error = %clear_err, "failed to clear terminal UI"); - } - return Err(err); + match control { + AppRunControl::Continue => {} + AppRunControl::Exit(reason) => break reason, } }; + tui.terminal.clear()?; Ok(AppExitInfo { token_usage: app.token_usage(), thread_id: app.chat_widget.thread_id(), @@ -2570,19 +2247,13 @@ impl App { self.start_fresh_session_with_summary_hint(tui).await; } AppEvent::ClearUi => { - self.clear_terminal_ui(tui, /*redraw_header*/ false)?; + self.clear_terminal_ui(tui, false)?; self.reset_app_ui_state_after_clear(); self.start_fresh_session_with_summary_hint(tui).await; } AppEvent::OpenResumePicker => { - match crate::resume_picker::run_resume_picker( - tui, - &self.config, - /*show_all*/ false, - ) - .await? - { + match crate::resume_picker::run_resume_picker(tui, &self.config, false).await? { SessionSelection::Resume(target_session) => { let current_cwd = self.config.cwd.clone(); let resume_cwd = match crate::resolve_cwd_for_resume_or_fork( @@ -2592,7 +2263,7 @@ impl App { target_session.thread_id, &target_session.path, CwdPromptAction::Resume, - /*allow_prompt*/ true, + true, ) .await? { @@ -2626,7 +2297,6 @@ impl App { resume_config.clone(), target_session.path.clone(), self.auth_manager.clone(), - /*parent_trace*/ None, ) .await { @@ -2677,7 +2347,7 @@ impl App { AppEvent::ForkCurrentSession => { self.session_telemetry.counter( "codex.thread.fork", - /*inc*/ 1, + 1, &[("source", "slash_command")], ); let summary = session_summary( @@ -2695,13 +2365,7 @@ impl App { if path.exists() { match self .server - .fork_thread( - usize::MAX, - self.config.clone(), - path.clone(), - /*persist_extended_history*/ false, - /*parent_trace*/ None, - ) + .fork_thread(usize::MAX, self.config.clone(), path.clone(), false) .await { Ok(forked) => { @@ -2862,9 +2526,6 @@ impl App { url, is_installed, is_enabled, - suggest_reason: None, - suggestion_type: None, - elicitation_target: None, }); } AppEvent::OpenUrlInBrowser { url } => { @@ -2954,7 +2615,7 @@ impl App { AppEvent::OpenWindowsSandboxFallbackPrompt { preset } => { self.session_telemetry.counter( "codex.windows_sandbox.fallback_prompt_shown", - /*inc*/ 1, + 1, &[], ); self.chat_widget.clear_windows_sandbox_setup_status(); @@ -3148,7 +2809,7 @@ impl App { self.chat_widget .add_to_history(history_cell::new_info_event( format!("Sandbox read access granted for {}", path.display()), - /*hint*/ None, + None, )); } }, @@ -3194,7 +2855,6 @@ impl App { Op::OverrideTurnContext { cwd: None, approval_policy: None, - approvals_reviewer: None, sandbox_policy: None, windows_sandbox_level: Some(windows_sandbox_level), model: None, @@ -3218,7 +2878,6 @@ impl App { Op::OverrideTurnContext { cwd: None, approval_policy: Some(preset.approval), - approvals_reviewer: Some(self.config.approvals_reviewer), sandbox_policy: Some(preset.sandbox.clone()), windows_sandbox_level: Some(windows_sandbox_level), model: None, @@ -3265,17 +2924,17 @@ impl App { model, effort, } => { - let profile = self.active_profile.clone(); + let profile = self.active_profile.as_deref(); let selected_provider = provider - .clone() - .unwrap_or_else(|| self.config.model_provider_id.clone()); + .as_deref() + .unwrap_or(self.config.model_provider_id.as_str()); let provider_changed = selected_provider != self.config.model_provider_id; let mut builder = ConfigEditsBuilder::new(&self.config.codex_home) - .with_profile(profile.as_deref()) + .with_profile(profile) .set_model(Some(model.as_str()), effort); if let Some(provider_id) = provider.as_deref() { - let segments = if let Some(profile) = profile.as_deref() { + let segments = if let Some(profile) = profile { vec![ "profiles".to_string(), profile.to_string(), @@ -3292,9 +2951,6 @@ impl App { match builder.apply().await { Ok(()) => { - if provider_changed { - self.start_fresh_session_with_summary_hint(tui).await; - } let effort_label = effort .map(|selected_effort| selected_effort.to_string()) .unwrap_or_else(|| "default".to_string()); @@ -3302,7 +2958,7 @@ impl App { "Selected model: {model}, provider: {selected_provider}, effort: {effort_label}" ); let mut message = if provider_changed { - format!("Applied model selection {selected_provider}/{model}") + format!("Saved model selection {selected_provider}/{model}") } else { format!("Model changed to {model}") }; @@ -3311,21 +2967,21 @@ impl App { message.push_str(label); } if provider_changed { - message.push_str(". Provider change applied in a fresh session"); + message.push_str(". Start a new session to apply provider change"); } - if let Some(profile) = profile.as_deref() { + if let Some(profile) = profile { message.push_str(" for "); message.push_str(profile); message.push_str(" profile"); } - self.chat_widget.add_info_message(message, /*hint*/ None); + self.chat_widget.add_info_message(message, None); } Err(err) => { tracing::error!( error = %err, "failed to persist model selection" ); - if let Some(profile) = profile.as_deref() { + if let Some(profile) = profile { self.chat_widget.add_error_message(format!( "Failed to save model/provider for profile `{profile}`: {err}" )); @@ -3353,7 +3009,7 @@ impl App { message.push_str(profile); message.push_str(" profile"); } - self.chat_widget.add_info_message(message, /*hint*/ None); + self.chat_widget.add_info_message(message, None); } Err(err) => { tracing::error!( @@ -3389,7 +3045,7 @@ impl App { message.push_str(profile); message.push_str(" profile"); } - self.chat_widget.add_info_message(message, /*hint*/ None); + self.chat_widget.add_info_message(message, None); } Err(err) => { tracing::error!(error = %err, "failed to persist fast mode selection"); @@ -3436,7 +3092,7 @@ impl App { let selection = name.unwrap_or_else(|| "System default".to_string()); self.chat_widget.add_info_message( format!("Realtime {} set to {selection}", kind.noun()), - /*hint*/ None, + None, ); } } @@ -3456,20 +3112,14 @@ impl App { self.chat_widget.restart_realtime_audio_device(kind); } AppEvent::UpdateAskForApprovalPolicy(policy) => { - let mut config = self.config.clone(); - if !self.try_set_approval_policy_on_config( - &mut config, - policy, - "Failed to set approval policy", - "failed to set approval policy on app config", - ) { + self.runtime_approval_policy_override = Some(policy); + if let Err(err) = self.config.permissions.approval_policy.set(policy) { + tracing::warn!(%err, "failed to set approval policy on app config"); + self.chat_widget + .add_error_message(format!("Failed to set approval policy: {err}")); return Ok(AppRunControl::Continue); } - self.config = config; - self.runtime_approval_policy_override = - Some(self.config.permissions.approval_policy.value()); - self.chat_widget - .set_approval_policy(self.config.permissions.approval_policy.value()); + self.chat_widget.set_approval_policy(policy); } AppEvent::UpdateSandboxPolicy(policy) => { #[cfg(target_os = "windows")] @@ -3480,16 +3130,12 @@ impl App { ); let policy_for_chat = policy.clone(); - let mut config = self.config.clone(); - if !self.try_set_sandbox_policy_on_config( - &mut config, - policy, - "Failed to set sandbox policy", - "failed to set sandbox policy on app config", - ) { + if let Err(err) = self.config.permissions.sandbox_policy.set(policy) { + tracing::warn!(%err, "failed to set sandbox policy on app config"); + self.chat_widget + .add_error_message(format!("Failed to set sandbox policy: {err}")); return Ok(AppRunControl::Continue); } - self.config = config; if let Err(err) = self.chat_widget.set_sandbox_policy(policy_for_chat) { tracing::warn!(%err, "failed to set sandbox policy on chat config"); self.chat_widget @@ -3529,36 +3175,6 @@ impl App { } } } - AppEvent::UpdateApprovalsReviewer(policy) => { - self.config.approvals_reviewer = policy; - self.chat_widget.set_approvals_reviewer(policy); - let profile = self.active_profile.as_deref(); - let segments = if let Some(profile) = profile { - vec![ - "profiles".to_string(), - profile.to_string(), - "approvals_reviewer".to_string(), - ] - } else { - vec!["approvals_reviewer".to_string()] - }; - if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .with_profile(profile) - .with_edits([ConfigEdit::SetPath { - segments, - value: policy.to_string().into(), - }]) - .apply() - .await - { - tracing::error!( - error = %err, - "failed to persist approvals reviewer update" - ); - self.chat_widget - .add_error_message(format!("Failed to save approvals reviewer: {err}")); - } - } AppEvent::UpdateFeatureFlags { updates } => { self.update_feature_flags(updates).await; } @@ -3582,7 +3198,7 @@ impl App { } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .set_hide_full_access_warning(/*acknowledged*/ true) + .set_hide_full_access_warning(true) .apply() .await { @@ -3597,7 +3213,7 @@ impl App { } AppEvent::PersistWorldWritableWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .set_hide_world_writable_warning(/*acknowledged*/ true) + .set_hide_world_writable_warning(true) .apply() .await { @@ -3612,7 +3228,7 @@ impl App { } AppEvent::PersistRateLimitSwitchPromptHidden => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .set_hide_rate_limit_model_nudge(/*acknowledged*/ true) + .set_hide_rate_limit_model_nudge(true) .apply() .await { @@ -3825,7 +3441,7 @@ impl App { lines.push(Line::from("")); } if let Some(rule_line) = - crate::bottom_pane::format_requested_permissions_rule(&permissions) + crate::bottom_pane::format_additional_permissions_rule(&permissions) { lines.push(Line::from(vec![ "Permission rule: ".into(), @@ -4008,7 +3624,7 @@ impl App { format!( "Agent thread {closed_thread_id} closed. Switched back to main thread." ), - /*hint*/ None, + None, ); } else { self.clear_active_thread().await; @@ -4047,7 +3663,7 @@ impl App { thread_id, config_snapshot.session_source.get_nickname(), config_snapshot.session_source.get_agent_role(), - /*is_closed*/ false, + false, ); let event = Event { id: String::new(), @@ -4059,7 +3675,6 @@ impl App { model_provider_id: config_snapshot.model_provider_id, service_tier: config_snapshot.service_tier, approval_policy: config_snapshot.approval_policy, - approvals_reviewer: config_snapshot.approvals_reviewer, sandbox_policy: config_snapshot.sandbox_policy, cwd: config_snapshot.cwd, reasoning_effort: config_snapshot.reasoning_effort, @@ -4215,49 +3830,11 @@ impl App { fn reset_external_editor_state(&mut self, tui: &mut tui::Tui) { self.chat_widget .set_external_editor_state(ExternalEditorState::Closed); - self.chat_widget.set_footer_hint_override(/*items*/ None); + self.chat_widget.set_footer_hint_override(None); tui.frame_requester().schedule_frame(); } async fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) { - // Some terminals, especially on macOS, encode Option+Left/Right as Option+b/f unless - // enhanced keyboard reporting is available. We only treat those word-motion fallbacks as - // agent-switch shortcuts when the composer is empty so we never steal the expected - // editing behavior for moving across words inside a draft. - let allow_agent_word_motion_fallback = !self.enhanced_keys_supported - && self.chat_widget.composer_text_with_pending().is_empty(); - if self.overlay.is_none() - && self.chat_widget.no_modal_or_popup_active() - // Alt+Left/Right are also natural word-motion keys in the composer. Keep agent - // fast-switch available only once the draft is empty so editing behavior wins whenever - // there is text on screen. - && self.chat_widget.composer_text_with_pending().is_empty() - && previous_agent_shortcut_matches(key_event, allow_agent_word_motion_fallback) - { - if let Some(thread_id) = self.agent_navigation.adjacent_thread_id( - self.current_displayed_thread_id(), - AgentNavigationDirection::Previous, - ) { - let _ = self.select_agent_thread(tui, thread_id).await; - } - return; - } - if self.overlay.is_none() - && self.chat_widget.no_modal_or_popup_active() - // Mirror the previous-agent rule above: empty drafts may use these keys for thread - // switching, but non-empty drafts keep them for expected word-wise cursor motion. - && self.chat_widget.composer_text_with_pending().is_empty() - && next_agent_shortcut_matches(key_event, allow_agent_word_motion_fallback) - { - if let Some(thread_id) = self.agent_navigation.adjacent_thread_id( - self.current_displayed_thread_id(), - AgentNavigationDirection::Next, - ) { - let _ = self.select_agent_thread(tui, thread_id).await; - } - return; - } - match key_event { KeyEvent { code: KeyCode::Char('t'), @@ -4279,7 +3856,7 @@ impl App { if !self.chat_widget.can_run_ctrl_l_clear_now() { return; } - if let Err(err) = self.clear_terminal_ui(tui, /*redraw_header*/ false) { + if let Err(err) = self.clear_terminal_ui(tui, false) { tracing::warn!(error = %err, "failed to clear terminal UI"); self.chat_widget .add_error_message(format!("Failed to clear terminal UI: {err}")); @@ -4392,13 +3969,11 @@ mod tests { use crate::app_backtrack::BacktrackState; use crate::app_backtrack::user_count; use crate::chatwidget::tests::make_chatwidget_manual_with_sender; - use crate::chatwidget::tests::set_chatgpt_auth; use crate::file_search::FileSearchManager; use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::UserHistoryCell; use crate::history_cell::new_session_info; - use crate::multi_agents::AgentPickerThreadEntry; use assert_matches::assert_matches; use codex_core::CodexAuth; use codex_core::GITHUB_COPILOT_PROVIDER_ID; @@ -4414,7 +3989,6 @@ mod tests { use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::openai_models::ModelAvailabilityNux; - use codex_protocol::openai_models::ModelsResponse; use codex_protocol::protocol::AgentMessageDeltaEvent; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; @@ -4434,48 +4008,15 @@ mod tests { use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::prelude::Line; - use serial_test::serial; - use std::env; - use std::ffi::OsString; use std::io::Read; use std::io::Write; use std::net::TcpListener; use std::path::PathBuf; use std::sync::Arc; - use std::sync::LazyLock; - use std::sync::Mutex; use std::sync::atomic::AtomicBool; use tempfile::tempdir; use tokio::time; - static MODELS_DEV_ENV_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); - - struct EnvVarGuard { - key: &'static str, - original: Option, - } - - impl EnvVarGuard { - fn set(key: &'static str, value: &str) -> Self { - let original = env::var_os(key); - unsafe { - env::set_var(key, value); - } - Self { key, original } - } - } - - impl Drop for EnvVarGuard { - fn drop(&mut self) { - unsafe { - match &self.original { - Some(value) => env::set_var(self.key, value), - None => env::remove_var(self.key), - } - } - } - } - #[test] fn normalize_harness_overrides_resolves_relative_add_dirs() -> Result<()> { let temp_dir = tempdir()?; @@ -4623,7 +4164,6 @@ mod tests { request_max_retries: None, stream_max_retries: None, stream_idle_timeout_ms: None, - websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }, @@ -4637,163 +4177,7 @@ mod tests { } #[tokio::test] - async fn model_picker_refresh_prefers_effective_active_provider_over_stale_provider_map() - -> Result<()> { - let codex_home = tempdir()?; - let mut config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .build() - .await?; - config.model_provider_id = "azure-local".to_string(); - config.model_provider = codex_core::ModelProviderInfo { - name: "Azure OpenAI".to_string(), - base_url: Some("https://proxy.local/v1".to_string()), - env_key: Some("AZURE_TEST_KEY".to_string()), - env_key_instructions: None, - experimental_bearer_token: None, - wire_api: codex_core::WireApi::Responses, - query_params: None, - http_headers: None, - env_http_headers: None, - request_max_retries: None, - stream_max_retries: None, - stream_idle_timeout_ms: None, - websocket_connect_timeout_ms: None, - requires_openai_auth: false, - supports_websockets: false, - }; - config.model_providers.insert( - "azure-local".to_string(), - codex_core::ModelProviderInfo::create_openai_provider(None), - ); - - assert_eq!( - startup_models_refresh_strategy(&config), - RefreshStrategy::OnlineIfUncached - ); - assert_eq!( - model_picker_refresh_strategy(&config, "azure-local"), - RefreshStrategy::OnlineIfUncached - ); - Ok(()) - } - - #[tokio::test] - #[serial] - async fn provider_model_presets_for_picker_prefers_effective_active_provider_over_stale_provider_map() - -> Result<()> { - let listener = TcpListener::bind("127.0.0.1:0")?; - let addr = listener.local_addr()?; - let models_dev_url = format!("http://{addr}/api.json"); - let models_dev_body = serde_json::json!({ - "azure": { - "id": "azure", - "name": "Azure OpenAI", - "api": format!("http://{addr}/proxy/v1"), - "models": { - "azure-model-a": { - "id": "azure-model-a", - "name": "azure-model-a", - "release_date": "2026-01-01", - "attachment": false, - "reasoning": true, - "temperature": true, - "tool_call": true, - "limit": {"context": 128000, "output": 4096}, - "options": {} - }, - "azure-model-b": { - "id": "azure-model-b", - "name": "azure-model-b", - "release_date": "2026-01-01", - "attachment": false, - "reasoning": true, - "temperature": true, - "tool_call": true, - "limit": {"context": 128000, "output": 4096}, - "options": {} - } - } - } - }) - .to_string(); - let server = std::thread::spawn(move || { - let (mut stream, _) = listener.accept().expect("accept /api.json connection"); - let mut request = Vec::new(); - let mut chunk = [0_u8; 1024]; - loop { - let bytes_read = stream.read(&mut chunk).expect("read request bytes"); - if bytes_read == 0 { - break; - } - request.extend_from_slice(&chunk[..bytes_read]); - if request.windows(4).any(|window| window == b"\r\n\r\n") { - break; - } - } - - let request = String::from_utf8_lossy(&request).to_string(); - let response = format!( - "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", - models_dev_body.len(), - models_dev_body - ); - stream - .write_all(response.as_bytes()) - .expect("write /api.json response"); - request - }); - - let mut app = make_test_app().await; - app.auth_manager = codex_core::test_support::auth_manager_from_auth( - CodexAuth::from_api_key("azure-test-token"), - ); - - let active_provider = codex_core::ModelProviderInfo { - name: "Azure OpenAI".to_string(), - base_url: Some(format!("http://{addr}/proxy/v1")), - env_key: Some("AZURE_TEST_KEY".to_string()), - env_key_instructions: None, - experimental_bearer_token: None, - wire_api: codex_core::WireApi::Responses, - query_params: None, - http_headers: None, - env_http_headers: None, - request_max_retries: None, - stream_max_retries: None, - stream_idle_timeout_ms: None, - websocket_connect_timeout_ms: None, - requires_openai_auth: false, - supports_websockets: false, - }; - app.config.model_provider_id = "azure-local".to_string(); - app.config.model_provider = active_provider; - app.config.model_providers = HashMap::from([( - "azure-local".to_string(), - codex_core::ModelProviderInfo::create_openai_provider(None), - )]); - - let _env_lock = MODELS_DEV_ENV_LOCK.lock().expect("lock models.dev env var"); - let _models_dev_url = EnvVarGuard::set("CODEX_MODELS_DEV_URL", &models_dev_url); - - let presets = app.provider_model_presets_for_picker().await; - let request = server.join().expect("models.dev server should complete"); - - assert!( - request.starts_with("GET /api.json "), - "expected GET /api.json request, got:\n{request}" - ); - assert!( - presets.iter().any(|preset| { - preset.provider_id == "azure-local" && preset.model.model == "azure-model-b" - }), - "expected active provider picker refresh to use effective custom provider" - ); - Ok(()) - } - - #[tokio::test] - async fn provider_model_presets_for_picker_fetches_copilot_models_for_inactive_provider() + async fn provider_model_presets_for_picker_fetches_copilot_models_for_inactive_provider() -> Result<()> { let listener = TcpListener::bind("127.0.0.1:0")?; let addr = listener.local_addr()?; @@ -4867,42 +4251,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn provider_model_presets_for_picker_preserves_active_custom_catalog() -> Result<()> { - let mut app = make_test_app().await; - let provider_id = "copilot-local".to_string(); - let provider = codex_core::ModelProviderInfo { - env_key: Some("__CODEX_TEST_COPILOT_ENV_KEY_MISSING__".to_string()), - ..codex_core::ModelProviderInfo::create_github_copilot_provider() - }; - app.config.model_provider_id = provider_id.clone(); - app.config.model_provider = provider.clone(); - app.config - .model_providers - .insert(provider_id.clone(), provider.clone()); - - let gpt_model = - codex_core::test_support::construct_model_info_offline("gpt-5.3-codex", &app.config); - let claude_model = - codex_core::test_support::construct_model_info_offline("claude-opus-4.6", &app.config); - app.config.model_catalog = Some(ModelsResponse { - models: vec![gpt_model, claude_model], - }); - - let presets = app.provider_model_presets_for_picker().await; - let custom_provider_models = presets - .iter() - .filter(|preset| preset.provider_id == provider_id) - .map(|preset| preset.model.model.clone()) - .collect::>(); - - assert_eq!( - custom_provider_models, - vec!["gpt-5.3-codex".to_string(), "claude-opus-4.6".to_string()] - ); - Ok(()) - } - #[test] fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() { assert_eq!( @@ -5020,7 +4368,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5193,7 +4540,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5271,7 +4617,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5353,7 +4698,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5434,7 +4778,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5509,7 +4852,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5623,7 +4965,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5693,7 +5034,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5797,7 +5137,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5874,7 +5213,6 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5976,14 +5314,13 @@ mod tests { assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); assert_eq!( - app.agent_navigation.get(&thread_id), + app.agent_picker_threads.get(&thread_id), Some(&AgentPickerThreadEntry { agent_nickname: None, agent_role: None, is_closed: true, }) ); - assert_eq!(app.agent_navigation.ordered_thread_ids(), vec![thread_id]); Ok(()) } @@ -5993,18 +5330,20 @@ mod tests { let thread_id = ThreadId::new(); app.thread_event_channels .insert(thread_id, ThreadEventChannel::new(1)); - app.agent_navigation.upsert( + app.agent_picker_threads.insert( thread_id, - Some("Robie".to_string()), - Some("explorer".to_string()), - false, + AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: false, + }, ); app.open_agent_picker().await; assert_eq!(app.thread_event_channels.contains_key(&thread_id), true); assert_eq!( - app.agent_navigation.get(&thread_id), + app.agent_picker_threads.get(&thread_id), Some(&AgentPickerThreadEntry { agent_nickname: Some("Robie".to_string()), agent_role: Some("explorer".to_string()), @@ -6017,7 +5356,6 @@ mod tests { #[tokio::test] async fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Result<()> { let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; - let _ = app.config.features.disable(Feature::Collab); app.open_agent_picker().await; app.chat_widget @@ -6037,16 +5375,21 @@ mod tests { .map(|line| line.to_string()) .collect::>() .join("\n"); - assert!(rendered.contains("Subagents will be enabled in the next session.")); + assert!(rendered.contains("Multi-agent will be enabled in the next session.")); Ok(()) } #[tokio::test] - async fn update_feature_flags_enabling_guardian_selects_guardian_approvals() -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; + async fn update_feature_flags_enabling_guardian_persists_only_the_feature_flag() -> Result<()> { + let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await; let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf(); - let guardian_approvals = guardian_approvals_mode(); + let current_session_policy = app + .chat_widget + .config_ref() + .permissions + .approval_policy + .value(); app.update_feature_flags(vec![(Feature::GuardianApproval, true)]) .await; @@ -6058,13 +5401,9 @@ mod tests { .features .enabled(Feature::GuardianApproval) ); - assert_eq!( - app.config.approvals_reviewer, - guardian_approvals.approvals_reviewer - ); assert_eq!( app.config.permissions.approval_policy.value(), - guardian_approvals.approval_policy + current_session_policy ); assert_eq!( app.chat_widget @@ -6072,379 +5411,35 @@ mod tests { .permissions .approval_policy .value(), - guardian_approvals.approval_policy - ); - assert_eq!( - app.chat_widget - .config_ref() - .permissions - .sandbox_policy - .get(), - &guardian_approvals.sandbox_policy - ); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - guardian_approvals.approvals_reviewer + current_session_policy ); assert_eq!(app.runtime_approval_policy_override, None); - assert_eq!(app.runtime_sandbox_policy_override, None); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: Some(guardian_approvals.approval_policy), - approvals_reviewer: Some(guardian_approvals.approvals_reviewer), - sandbox_policy: Some(guardian_approvals.sandbox_policy.clone()), - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - ); - let cell = match app_event_rx.try_recv() { - Ok(AppEvent::InsertHistoryCell(cell)) => cell, - other => panic!("expected InsertHistoryCell event, got {other:?}"), - }; - let rendered = cell - .display_lines(120) - .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"); - assert!(rendered.contains("Permissions updated to Guardian Approvals")); - - let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(config.contains("guardian_approval = true")); - assert!(config.contains("approvals_reviewer = \"guardian_subagent\"")); - assert!(config.contains("approval_policy = \"on-request\"")); - assert!(config.contains("sandbox_mode = \"workspace-write\"")); - Ok(()) - } - - #[tokio::test] - async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restores_default() - -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = "approvals_reviewer = \"guardian_subagent\"\napproval_policy = \"on-request\"\nsandbox_mode = \"workspace-write\"\n\n[features]\nguardian_approval = true\n"; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - app.config - .features - .set_enabled(Feature::GuardianApproval, true)?; - app.chat_widget - .set_feature_enabled(Feature::GuardianApproval, true); - app.config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::GuardianSubagent); - app.config - .permissions - .approval_policy - .set(AskForApproval::OnRequest)?; - app.config - .permissions - .sandbox_policy - .set(SandboxPolicy::new_workspace_write_policy())?; - app.chat_widget - .set_approval_policy(AskForApproval::OnRequest); - app.chat_widget - .set_sandbox_policy(SandboxPolicy::new_workspace_write_policy())?; - - app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) - .await; - - assert!(!app.config.features.enabled(Feature::GuardianApproval)); assert!( - !app.chat_widget - .config_ref() - .features - .enabled(Feature::GuardianApproval) - ); - assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User); - assert_eq!( - app.config.permissions.approval_policy.value(), - AskForApproval::OnRequest - ); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - ApprovalsReviewer::User - ); - assert_eq!(app.runtime_approval_policy_override, None); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: None, - approvals_reviewer: Some(ApprovalsReviewer::User), - sandbox_policy: None, - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - ); - let cell = match app_event_rx.try_recv() { - Ok(AppEvent::InsertHistoryCell(cell)) => cell, - other => panic!("expected InsertHistoryCell event, got {other:?}"), - }; - let rendered = cell - .display_lines(120) - .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"); - assert!(rendered.contains("Permissions updated to Default")); - - let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(!config.contains("guardian_approval = true")); - assert!(!config.contains("approvals_reviewer =")); - assert!(config.contains("approval_policy = \"on-request\"")); - assert!(config.contains("sandbox_mode = \"workspace-write\"")); - Ok(()) - } - - #[tokio::test] - async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review_policy() - -> Result<()> { - let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - let guardian_approvals = guardian_approvals_mode(); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = "approvals_reviewer = \"user\"\n"; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - app.config.approvals_reviewer = ApprovalsReviewer::User; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::User); - - app.update_feature_flags(vec![(Feature::GuardianApproval, true)]) - .await; - - assert!(app.config.features.enabled(Feature::GuardianApproval)); - assert_eq!( - app.config.approvals_reviewer, - guardian_approvals.approvals_reviewer - ); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - guardian_approvals.approvals_reviewer - ); - assert_eq!( - app.config.permissions.approval_policy.value(), - guardian_approvals.approval_policy - ); - assert_eq!( - app.chat_widget - .config_ref() - .permissions - .sandbox_policy - .get(), - &guardian_approvals.sandbox_policy - ); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: Some(guardian_approvals.approval_policy), - approvals_reviewer: Some(guardian_approvals.approvals_reviewer), - sandbox_policy: Some(guardian_approvals.sandbox_policy.clone()), - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) + op_rx.try_recv().is_err(), + "feature toggle should not patch the active session" ); let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(config.contains("approvals_reviewer = \"guardian_subagent\"")); assert!(config.contains("guardian_approval = true")); - assert!(config.contains("approval_policy = \"on-request\"")); - assert!(config.contains("sandbox_mode = \"workspace-write\"")); + assert!(!config.contains("approval_policy")); Ok(()) } #[tokio::test] - async fn update_feature_flags_disabling_guardian_clears_manual_review_policy_without_history() - -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = "approvals_reviewer = \"user\"\napproval_policy = \"on-request\"\nsandbox_mode = \"workspace-write\"\n\n[features]\nguardian_approval = true\n"; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - app.config - .features - .set_enabled(Feature::GuardianApproval, true)?; - app.chat_widget - .set_feature_enabled(Feature::GuardianApproval, true); - app.config.approvals_reviewer = ApprovalsReviewer::User; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::User); - - app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) - .await; - - assert!(!app.config.features.enabled(Feature::GuardianApproval)); - assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - ApprovalsReviewer::User - ); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: None, - approvals_reviewer: Some(ApprovalsReviewer::User), - sandbox_policy: None, - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - ); - assert!( - app_event_rx.try_recv().is_err(), - "manual review should not emit a permissions history update when the effective state stays default" - ); - - let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(!config.contains("guardian_approval = true")); - assert!(!config.contains("approvals_reviewer =")); - Ok(()) - } - - #[tokio::test] - async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_review_policy() - -> Result<()> { + async fn update_feature_flags_disabling_guardian_clears_only_the_feature_flag() -> Result<()> { let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await; let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf(); - let guardian_approvals = guardian_approvals_mode(); - app.active_profile = Some("guardian".to_string()); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = "profile = \"guardian\"\napprovals_reviewer = \"user\"\n"; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - app.config.approvals_reviewer = ApprovalsReviewer::User; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::User); - - app.update_feature_flags(vec![(Feature::GuardianApproval, true)]) - .await; - - assert!(app.config.features.enabled(Feature::GuardianApproval)); - assert_eq!( - app.config.approvals_reviewer, - guardian_approvals.approvals_reviewer - ); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - guardian_approvals.approvals_reviewer - ); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: Some(guardian_approvals.approval_policy), - approvals_reviewer: Some(guardian_approvals.approvals_reviewer), - sandbox_policy: Some(guardian_approvals.sandbox_policy.clone()), - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - ); - - let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - let config_value = toml::from_str::(&config)?; - let profile_config = config_value - .as_table() - .and_then(|table| table.get("profiles")) - .and_then(TomlValue::as_table) - .and_then(|profiles| profiles.get("guardian")) - .and_then(TomlValue::as_table) - .expect("guardian profile should exist"); - assert_eq!( - config_value - .as_table() - .and_then(|table| table.get("approvals_reviewer")), - Some(&TomlValue::String("user".to_string())) - ); - assert_eq!( - profile_config.get("approvals_reviewer"), - Some(&TomlValue::String("guardian_subagent".to_string())) - ); - Ok(()) - } - - #[tokio::test] - async fn update_feature_flags_disabling_guardian_in_profile_allows_inherited_user_reviewer() - -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - app.active_profile = Some("guardian".to_string()); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = r#" -profile = "guardian" -approvals_reviewer = "user" - -[profiles.guardian] -approvals_reviewer = "guardian_subagent" - -[profiles.guardian.features] -guardian_approval = true -"#; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); + std::fs::write( + codex_home.path().join("config.toml"), + "[features]\nguardian_approval = true\n", + )?; app.config .features .set_enabled(Feature::GuardianApproval, true)?; app.chat_widget .set_feature_enabled(Feature::GuardianApproval, true); - app.config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::GuardianSubagent); + let current_session_policy = app.config.permissions.approval_policy.value(); app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) .await; @@ -6456,117 +5451,19 @@ guardian_approval = true .features .enabled(Feature::GuardianApproval) ); - assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - ApprovalsReviewer::User - ); - assert_eq!( - op_rx.try_recv(), - Ok(Op::OverrideTurnContext { - cwd: None, - approval_policy: None, - approvals_reviewer: Some(ApprovalsReviewer::User), - sandbox_policy: None, - windows_sandbox_level: None, - model: None, - effort: None, - summary: None, - service_tier: None, - collaboration_mode: None, - personality: None, - }) - ); - let cell = match app_event_rx.try_recv() { - Ok(AppEvent::InsertHistoryCell(cell)) => cell, - other => panic!("expected InsertHistoryCell event, got {other:?}"), - }; - let rendered = cell - .display_lines(120) - .into_iter() - .map(|line| line.to_string()) - .collect::>() - .join("\n"); - assert!(rendered.contains("Permissions updated to Default")); - - let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(!config.contains("guardian_approval = true")); - assert!(!config.contains("guardian_subagent")); - assert_eq!( - toml::from_str::(&config)? - .as_table() - .and_then(|table| table.get("approvals_reviewer")), - Some(&TomlValue::String("user".to_string())) - ); - Ok(()) - } - - #[tokio::test] - async fn update_feature_flags_disabling_guardian_in_profile_keeps_inherited_non_user_reviewer_enabled() - -> Result<()> { - let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - app.active_profile = Some("guardian".to_string()); - let config_toml_path = AbsolutePathBuf::try_from(codex_home.path().join("config.toml"))?; - let config_toml = "profile = \"guardian\"\napprovals_reviewer = \"guardian_subagent\"\n\n[features]\nguardian_approval = true\n"; - std::fs::write(config_toml_path.as_path(), config_toml)?; - let user_config = toml::from_str::(config_toml)?; - app.config.config_layer_stack = app - .config - .config_layer_stack - .with_user_config(&config_toml_path, user_config); - app.config - .features - .set_enabled(Feature::GuardianApproval, true)?; - app.chat_widget - .set_feature_enabled(Feature::GuardianApproval, true); - app.config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent; - app.chat_widget - .set_approvals_reviewer(ApprovalsReviewer::GuardianSubagent); - - app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) - .await; - - assert!(app.config.features.enabled(Feature::GuardianApproval)); - assert!( - app.chat_widget - .config_ref() - .features - .enabled(Feature::GuardianApproval) - ); assert_eq!( - app.config.approvals_reviewer, - ApprovalsReviewer::GuardianSubagent - ); - assert_eq!( - app.chat_widget.config_ref().approvals_reviewer, - ApprovalsReviewer::GuardianSubagent + app.config.permissions.approval_policy.value(), + current_session_policy ); + assert_eq!(app.runtime_approval_policy_override, None); assert!( op_rx.try_recv().is_err(), - "disabling an inherited non-user reviewer should not patch the active session" - ); - let app_events = std::iter::from_fn(|| app_event_rx.try_recv().ok()).collect::>(); - assert!( - !app_events.iter().any(|event| match event { - AppEvent::InsertHistoryCell(cell) => cell - .display_lines(120) - .iter() - .any(|line| line.to_string().contains("Permissions updated to")), - _ => false, - }), - "blocking disable with inherited guardian review should not emit a permissions history update: {app_events:?}" + "feature toggle should not patch the active session" ); let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?; - assert!(config.contains("guardian_approval = true")); - assert_eq!( - toml::from_str::(&config)? - .as_table() - .and_then(|table| table.get("approvals_reviewer")), - Some(&TomlValue::String("guardian_subagent".to_string())) - ); + assert!(!config.contains("guardian_approval = true")); + assert!(!config.contains("approval_policy")); Ok(()) } @@ -6715,11 +5612,13 @@ guardian_approval = true } app.thread_event_channels .insert(agent_thread_id, agent_channel); - app.agent_navigation.upsert( + app.agent_picker_threads.insert( agent_thread_id, - Some("Robie".to_string()), - Some("explorer".to_string()), - false, + AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: false, + }, ); app.refresh_pending_thread_approvals().await; @@ -6759,7 +5658,6 @@ guardian_approval = true model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::OnRequest, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), cwd: PathBuf::from("/tmp/agent"), reasoning_effort: None, @@ -6772,11 +5670,13 @@ guardian_approval = true }, ), ); - app.agent_navigation.upsert( + app.agent_picker_threads.insert( agent_thread_id, - Some("Robie".to_string()), - Some("explorer".to_string()), - false, + AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: false, + }, ); app.enqueue_thread_event( @@ -6980,7 +5880,6 @@ guardian_approval = true model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: Some(ReasoningEffortConfig::High), @@ -7051,32 +5950,6 @@ guardian_approval = true assert_snapshot!("clear_ui_after_long_transcript_fresh_header_only", rendered); } - #[tokio::test] - async fn clear_ui_header_shows_fast_status_only_for_gpt54() { - let mut app = make_test_app().await; - app.config.cwd = PathBuf::from("/tmp/project"); - app.chat_widget.set_model("gpt-5.4"); - app.chat_widget - .set_reasoning_effort(Some(ReasoningEffortConfig::XHigh)); - app.chat_widget - .set_service_tier(Some(codex_protocol::config_types::ServiceTier::Fast)); - set_chatgpt_auth(&mut app.chat_widget); - - let rendered = app - .clear_ui_header_lines_with_version(80, "") - .iter() - .map(|line| { - line.spans - .iter() - .map(|span| span.content.as_ref()) - .collect::() - }) - .collect::>() - .join("\n"); - - assert_snapshot!("clear_ui_header_fast_status_gpt54_only", rendered); - } - async fn make_test_app() -> App { let (chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; let config = chat_widget.config_ref().clone(); @@ -7123,7 +5996,7 @@ guardian_approval = true windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), thread_event_listener_tasks: HashMap::new(), - agent_navigation: AgentNavigationState::default(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, @@ -7183,7 +6056,7 @@ guardian_approval = true windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), thread_event_listener_tasks: HashMap::new(), - agent_navigation: AgentNavigationState::default(), + agent_picker_threads: HashMap::new(), active_thread_id: None, active_thread_rx: None, primary_thread_id: None, @@ -7664,7 +6537,6 @@ guardian_approval = true model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: next_cwd.clone(), reasoning_effort: None, @@ -7721,32 +6593,6 @@ guardian_approval = true Ok(()) } - #[tokio::test] - async fn rebuild_config_for_cwd_preserves_active_profile() -> Result<()> { - let mut app = make_test_app().await; - let codex_home = tempdir()?; - app.config.codex_home = codex_home.path().to_path_buf(); - app.active_profile = Some("copilot".to_string()); - std::fs::write( - codex_home.path().join("config.toml"), - r#" -model_provider = "openai" -model = "gpt-5.4" - -[profiles.copilot] -model_provider = "github-copilot" -model = "claude-opus-4.6" -"#, - )?; - - let rebuilt = app.rebuild_config_for_cwd(app.config.cwd.clone()).await?; - - assert_eq!(rebuilt.active_profile.as_deref(), Some("copilot")); - assert_eq!(rebuilt.model_provider_id, "github-copilot"); - assert_eq!(rebuilt.model.as_deref(), Some("claude-opus-4.6")); - Ok(()) - } - #[tokio::test] async fn sync_tui_theme_selection_updates_chat_widget_config_copy() { let mut app = make_test_app().await; @@ -7806,7 +6652,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7866,7 +6711,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7959,7 +6803,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -8025,7 +6868,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -8106,7 +6948,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -8234,7 +7075,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -8304,7 +7144,6 @@ model = "claude-opus-4.6" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, - approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, diff --git a/codex-rs/tui/tests/suite/model_switching_e2e.rs b/codex-rs/tui/tests/suite/model_switching_e2e.rs index 22ea2f1ba193..f184c4ec379f 100644 --- a/codex-rs/tui/tests/suite/model_switching_e2e.rs +++ b/codex-rs/tui/tests/suite/model_switching_e2e.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::io::Read; use std::io::Write; use std::net::TcpListener; -use std::net::TcpStream; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -33,36 +32,7 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( eprintln!("skipping integration test because codex binary is unavailable"); return Ok(()); }; - let codex_home = tempdir_with_ollama_config(&repo_root, "gpt-5.3-codex")?; - let (first_base_url, first_server) = spawn_models_server(serde_json::json!({ - "object": "list", - "data": [ - {"id": "gpt-5.3-codex", "object": "model"}, - {"id": "claude-opus-4.6", "object": "model"} - ] - }))?; - let (first_models_dev_url, first_models_dev_server) = - spawn_models_dev_catalog_server(serde_json::json!({ - "github-copilot": { - "id": "github-copilot", - "name": "GitHub Copilot", - "api": "https://api.githubcopilot.com/v1", - "models": { - "gpt-5.3-codex": {"id": "gpt-5.3-codex", "name": "gpt-5.3-codex"} - } - }, - "lmstudio": { - "id": "lmstudio", - "name": "LM Studio", - "api": "http://127.0.0.1:1234/v1", - "models": { - "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} - } - } - }))?; - let mut first_env = HashMap::new(); - first_env.insert("CODEX_OSS_BASE_URL".to_string(), first_base_url); - first_env.insert("CODEX_MODELS_DEV_URL".to_string(), first_models_dev_url); + let codex_home = tempdir_with_catalog_and_config(&repo_root)?; let first_output = run_codex_cli_with_filter( &codex_cli, @@ -71,58 +41,16 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( "gpt-5.3-codex", "claude", None, - first_env, + HashMap::new(), ) .await .context("switch from gpt-5.3-codex to claude-opus-4.6")?; assert_model_and_provider_in_config( codex_home.path(), "claude-opus-4.6", - "ollama", + "github-copilot", &first_output, )?; - let first_requests = first_server - .join() - .expect("first model server should join")?; - anyhow::ensure!( - first_requests - .iter() - .any(|line| is_models_endpoint_request(line)), - "expected GET /v1/models in first run, got: {first_requests:?}" - ); - let _first_models_dev_requests = first_models_dev_server - .join() - .expect("first models.dev server should join")?; - - let (second_base_url, second_server) = spawn_models_server(serde_json::json!({ - "object": "list", - "data": [ - {"id": "gpt-5.3-codex", "object": "model"}, - {"id": "claude-opus-4.6", "object": "model"} - ] - }))?; - let (second_models_dev_url, second_models_dev_server) = - spawn_models_dev_catalog_server(serde_json::json!({ - "github-copilot": { - "id": "github-copilot", - "name": "GitHub Copilot", - "api": "https://api.githubcopilot.com/v1", - "models": { - "gpt-5.3-codex": {"id": "gpt-5.3-codex", "name": "gpt-5.3-codex"} - } - }, - "lmstudio": { - "id": "lmstudio", - "name": "LM Studio", - "api": "http://127.0.0.1:1234/v1", - "models": { - "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} - } - } - }))?; - let mut second_env = HashMap::new(); - second_env.insert("CODEX_OSS_BASE_URL".to_string(), second_base_url); - second_env.insert("CODEX_MODELS_DEV_URL".to_string(), second_models_dev_url); let second_output = run_codex_cli_with_filter( &codex_cli, @@ -131,28 +59,16 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( "claude-opus-4.6", "gpt-5.3-codex", None, - second_env, + HashMap::new(), ) .await .context("switch back from claude-opus-4.6 to gpt-5.3-codex")?; assert_model_and_provider_in_config( codex_home.path(), "gpt-5.3-codex", - "ollama", + "github-copilot", &second_output, )?; - let second_requests = second_server - .join() - .expect("second model server should join")?; - anyhow::ensure!( - second_requests - .iter() - .any(|line| is_models_endpoint_request(line)), - "expected GET /v1/models in second run, got: {second_requests:?}" - ); - let _second_models_dev_requests = second_models_dev_server - .join() - .expect("second models.dev server should join")?; Ok(()) } @@ -199,7 +115,9 @@ async fn model_picker_ignores_openai_compat_models_disabled_for_picker() -> Resu let requests = server.join().expect("model listing server should join")?; assert!( - requests.iter().any(|line| is_models_endpoint_request(line)), + requests + .iter() + .any(|line| line.starts_with("GET /v1/models ")), "expected at least one GET /v1/models request, got: {requests:?}" ); @@ -245,7 +163,9 @@ async fn ollama_model_picker_uses_local_models_endpoint_and_switches() -> Result let requests = server.join().expect("model listing server should join")?; assert!( - requests.iter().any(|line| is_models_endpoint_request(line)), + requests + .iter() + .any(|line| line.starts_with("GET /v1/models ")), "expected at least one GET /v1/models request, got: {requests:?}" ); @@ -275,7 +195,7 @@ async fn ollama_model_picker_does_not_switch_to_bundled_gpt_when_discovery_fails codex_home.path(), &repo_root, "llama3.2:latest", - "github-copilot/not-a-real-model", + "gpt-5", None, env, ) @@ -285,7 +205,9 @@ async fn ollama_model_picker_does_not_switch_to_bundled_gpt_when_discovery_fails let requests = server.join().expect("discovery server should join")?; assert!( - requests.iter().any(|line| is_models_endpoint_request(line)), + requests + .iter() + .any(|line| line.starts_with("GET /v1/models ")), "expected GET /v1/models request, got: {requests:?}" ); @@ -343,7 +265,7 @@ async fn ollama_model_switch_then_prompt_uses_responses_api() -> Result<()> { anyhow::ensure!( requests .iter() - .any(|request| is_models_endpoint_request(request)), + .any(|request| request.starts_with("GET /v1/models ")), "expected GET /v1/models request; got requests: {requests:?}" ); let responses_request = requests @@ -377,47 +299,45 @@ async fn copilot_model_switch_then_prompt_uses_responses_api_without_cli_provide }; let prompt = "When the first gpt model was released?"; let answer_text = "The first GPT model (GPT-1) was released in June 2018."; - let startup_model = "gpt-5"; - let selected_model = "claude-4.6-opus"; let (base_url, server) = spawn_openai_compat_models_and_responses_server( serde_json::json!({ "object": "list", "data": [ { - "id": startup_model, + "id": "copilot-test-a", "object": "model", "model_picker_enabled": true, "supported_endpoints": ["/responses"] }, { - "id": selected_model, + "id": "copilot-test-b", "object": "model", "model_picker_enabled": true, "supported_endpoints": ["/responses"] } ] }), - selected_model, + "copilot-test-b", answer_text, )?; - let codex_home = tempdir_with_github_copilot_config( + let codex_home = tempdir_with_local_copilot_config( &repo_root, - startup_model, + "copilot-test-a", base_url.as_str(), - &[startup_model, selected_model], + &["copilot-test-a", "copilot-test-b"], )?; let mut env = HashMap::new(); env.insert( - "GITHUB_COPILOT_TOKEN".to_string(), + "COPILOT_TEST_KEY".to_string(), "test-copilot-token".to_string(), ); let output = run_codex_cli_with_filter( &codex_cli, codex_home.path(), &repo_root, - startup_model, - selected_model, + "copilot-test-a", + "copilot-test-b", Some(prompt), env, ) @@ -425,24 +345,18 @@ async fn copilot_model_switch_then_prompt_uses_responses_api_without_cli_provide .context("switch model via /model and run prompt against local Copilot responses API")?; assert_model_and_provider_in_config( codex_home.path(), - selected_model, - "github-copilot", + "copilot-test-b", + "copilot-local", &output, )?; let requests = server.join().expect("model/responses server should join")?; - anyhow::ensure!( - requests - .iter() - .any(|request| is_models_endpoint_request(request)), - "expected GET /v1/models request against localhost Copilot provider; got requests: {requests:?}" - ); let responses_request = requests .iter() .find(|request| request.starts_with("POST /v1/responses ")) .context("missing POST /v1/responses request")?; anyhow::ensure!( - responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), + responses_request.contains("\"model\":\"copilot-test-b\""), "expected switched model in /responses request body; request: {responses_request}" ); anyhow::ensure!( @@ -468,14 +382,13 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( let prompt = "When the first gpt model was released?"; let answer_text = "The first GPT model (GPT-1) was released in June 2018."; - let startup_model = "gpt-5"; - let selected_model = "claude-4.6-opus"; + let selected_model = "claude-opus-4.6"; let (base_url, server) = spawn_openai_compat_models_with_chat_completions_fallback_server( serde_json::json!({ "object": "list", "data": [ { - "id": startup_model, + "id": "copilot-test-a", "object": "model", "model_picker_enabled": true, "supported_endpoints": ["/responses"] @@ -491,23 +404,23 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( selected_model, answer_text, )?; - let codex_home = tempdir_with_github_copilot_config( + let codex_home = tempdir_with_local_copilot_config( &repo_root, - startup_model, + "copilot-test-a", base_url.as_str(), - &[startup_model, selected_model], + &["copilot-test-a", selected_model], )?; let mut env = HashMap::new(); env.insert( - "GITHUB_COPILOT_TOKEN".to_string(), + "COPILOT_TEST_KEY".to_string(), "test-copilot-token".to_string(), ); let output = run_codex_cli_with_filter( &codex_cli, codex_home.path(), &repo_root, - startup_model, + "copilot-test-a", selected_model, Some(prompt), env, @@ -517,7 +430,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( assert_model_and_provider_in_config( codex_home.path(), selected_model, - "github-copilot", + "copilot-local", &output, )?; anyhow::ensure!( @@ -526,18 +439,12 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( ); let requests = server.join().expect("model/responses server should join")?; - anyhow::ensure!( - requests - .iter() - .any(|request| is_models_endpoint_request(request)), - "expected GET /v1/models request against localhost Copilot provider; got requests: {requests:?}" - ); let responses_request = requests .iter() .find(|request| request.starts_with("POST /v1/responses ")) .context("missing POST /v1/responses request")?; anyhow::ensure!( - responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), + responses_request.contains("\"model\":\"claude-opus-4.6\""), "expected switched model in /responses request body; request: {responses_request}" ); @@ -546,7 +453,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( .find(|request| request.starts_with("POST /v1/chat/completions ")) .context("missing POST /v1/chat/completions request")?; anyhow::ensure!( - chat_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), + chat_request.contains("\"model\":\"claude-opus-4.6\""), "expected switched model in /chat/completions request body; request: {chat_request}" ); anyhow::ensure!( @@ -674,181 +581,53 @@ async fn models_dev_provider_config_parses_custom_provider() -> Result<()> { Ok(()) } -#[tokio::test] -async fn cross_provider_model_switch_applies_immediately_without_manual_new_session() -> Result<()> -{ - // run_codex_cli_with_filter() does not work on Windows due to PTY limitations. - if cfg!(windows) { - return Ok(()); - } +fn tempdir_with_catalog_and_config(repo_root: &Path) -> Result { + let codex_home = tempfile::tempdir()?; - let repo_root = codex_utils_cargo_bin::repo_root()?; - let Some(codex_cli) = find_codex_cli(&repo_root) else { - eprintln!("skipping integration test because codex binary is unavailable"); - return Ok(()); - }; + let source_catalog_path = codex_utils_cargo_bin::find_resource!("../core/models.json")?; + let source_catalog = std::fs::read_to_string(&source_catalog_path)?; + let mut source_catalog: JsonValue = serde_json::from_str(&source_catalog)?; + let models = source_catalog + .get_mut("models") + .and_then(JsonValue::as_array_mut) + .context("models array missing")?; + let template = models.first().cloned().context("models array is empty")?; - let startup_model = "azure-model-a"; - let selected_model = "claude-4.6-opus"; - let prompt = "Name the provider handling this prompt."; - let azure_answer = "Azure should not handle the post-switch prompt."; - let copilot_answer = "Copilot provider handled this prompt immediately."; + let gpt_model = model_from_template(&template, "gpt-5.3-codex", "gpt-5.3-codex", 0)?; + let claude_model = model_from_template(&template, "claude-opus-4.6", "claude-opus-4.6", 1)?; + *models = vec![gpt_model, claude_model]; - let (azure_base_url, azure_server) = spawn_openai_compat_models_and_responses_server( - serde_json::json!({ - "object": "list", - "data": [ - {"id": startup_model, "object": "model"}, - {"id": "azure-model-b", "object": "model"} - ] - }), - startup_model, - azure_answer, - )?; - let (copilot_base_url, copilot_server) = spawn_openai_compat_models_and_responses_server( - serde_json::json!({ - "object": "list", - "data": [ - {"id": selected_model, "object": "model"} - ] - }), - selected_model, - copilot_answer, - )?; - let (models_dev_url, models_dev_server) = spawn_models_dev_catalog_server(serde_json::json!({ - "azure": { - "id": "azure", - "name": "Azure", - "api": azure_base_url.clone(), - "models": { - startup_model: {"id": startup_model, "name": startup_model}, - "azure-model-b": {"id": "azure-model-b", "name": "azure-model-b"} - } - }, - "github-copilot": { - "id": "github-copilot", - "name": "GitHub Copilot", - "api": copilot_base_url.clone(), - "models": { - selected_model: {"id": selected_model, "name": selected_model} - } - }, - "ollama": { - "id": "ollama", - "name": "Ollama", - "api": "http://127.0.0.1:11434/v1", - "models": { - "llama3.2:latest": {"id": "llama3.2:latest", "name": "llama3.2:latest"} - } - }, - "lmstudio": { - "id": "lmstudio", - "name": "LM Studio", - "api": "http://127.0.0.1:1234/v1", - "models": { - "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} - } - } - }))?; - let codex_home = tempdir_with_dual_provider_config( - &repo_root, - startup_model, - azure_base_url.as_str(), - copilot_base_url.as_str(), + let custom_catalog_path = codex_home.path().join("catalog.json"); + std::fs::write( + &custom_catalog_path, + serde_json::to_string(&source_catalog)?, )?; - let mut env = HashMap::new(); - env.insert("AZURE_TEST_KEY".to_string(), "test-azure-token".to_string()); - env.insert( - "GITHUB_COPILOT_TOKEN".to_string(), - "test-copilot-token".to_string(), - ); - env.insert("CODEX_MODELS_DEV_URL".to_string(), models_dev_url); - - let output = run_codex_cli_with_filter( - &codex_cli, - codex_home.path(), - &repo_root, - startup_model, - selected_model, - Some(prompt), - env, - ) - .await - .context("cross-provider switch via /model should apply immediately in-session")?; - - let azure_requests = azure_server - .join() - .expect("azure test server should join")?; - let copilot_requests = copilot_server - .join() - .expect("copilot test server should join")?; - let models_dev_requests = models_dev_server - .join() - .expect("models.dev server should join")?; - - anyhow::ensure!( - copilot_requests - .iter() - .any(|request| is_models_endpoint_request(request)), - "expected Copilot provider /models discovery request; got requests: {copilot_requests:?}" - ); - anyhow::ensure!( - models_dev_requests - .iter() - .any(|request| request.starts_with("GET /api.json ")), - "expected GET /api.json request; got requests: {models_dev_requests:?}" - ); - - assert_model_and_provider_in_config( - codex_home.path(), - selected_model, - "github-copilot", - &output, - )?; - anyhow::ensure!( - output.contains(copilot_answer), - "expected Copilot answer after in-session provider switch, got: {output}" - ); - anyhow::ensure!( - !output.contains("Start a new session to apply provider change"), - "did not expect restart hint after provider switch, got: {output}" - ); + let repo_root_display = repo_root.display(); + let catalog_display = custom_catalog_path.display(); + let config_contents = format!( + r#"model_provider = "github-copilot" +model = "gpt-5.3-codex" +model_catalog_json = "{catalog_display}" - let copilot_responses_request = copilot_requests - .iter() - .find(|request| request.starts_with("POST /v1/responses ")) - .context("missing Copilot POST /v1/responses request after provider switch")?; - anyhow::ensure!( - copilot_responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), - "expected selected model in Copilot /responses request body; request: {copilot_responses_request}" - ); - anyhow::ensure!( - copilot_responses_request.contains(prompt), - "expected prompt text in Copilot /responses request body; request: {copilot_responses_request}" - ); - anyhow::ensure!( - !azure_requests - .iter() - .any(|request| request.starts_with("POST /v1/responses ")), - "did not expect post-switch /responses requests against Azure provider; requests: {azure_requests:?}" +[projects."{repo_root_display}"] +trust_level = "trusted" +"# ); + std::fs::write(codex_home.path().join("config.toml"), config_contents)?; - Ok(()) + Ok(codex_home) } fn tempdir_with_ollama_config(repo_root: &Path, model: &str) -> Result { let codex_home = tempfile::tempdir()?; - let repo_root_display = repo_root.display().to_string(); - let repo_root_toml = toml::Value::String(repo_root_display).to_string(); - let model_toml = toml::Value::String(model.to_string()).to_string(); + let repo_root_display = repo_root.display(); let config_contents = format!( r#"model_provider = "ollama" -model = {model_toml} -cli_auth_credentials_store = "file" +model = "{model}" -[projects.{repo_root_toml}] +[projects."{repo_root_display}"] trust_level = "trusted" "# ); @@ -857,7 +636,7 @@ trust_level = "trusted" Ok(codex_home) } -fn tempdir_with_github_copilot_config( +fn tempdir_with_local_copilot_config( repo_root: &Path, model: &str, base_url: &str, @@ -887,25 +666,20 @@ fn tempdir_with_github_copilot_config( serde_json::to_string(&source_catalog)?, )?; - let repo_root_display = repo_root.display().to_string(); - let repo_root_toml = toml::Value::String(repo_root_display).to_string(); - let catalog_display = custom_catalog_path.display().to_string(); - let catalog_toml = toml::Value::String(catalog_display).to_string(); - let model_toml = toml::Value::String(model.to_string()).to_string(); - let base_url_toml = toml::Value::String(base_url.to_string()).to_string(); + let repo_root_display = repo_root.display(); + let catalog_display = custom_catalog_path.display(); let config_contents = format!( - r#"model_provider = "github-copilot" -model = {model_toml} -model_catalog_json = {catalog_toml} -cli_auth_credentials_store = "file" + r#"model_provider = "copilot-local" +model = "{model}" +model_catalog_json = "{catalog_display}" -[model_providers.github-copilot] +[model_providers.copilot-local] name = "GitHub Copilot" -base_url = {base_url_toml} -env_key = "GITHUB_COPILOT_TOKEN" +base_url = "{base_url}" +env_key = "COPILOT_TEST_KEY" wire_api = "responses" -[projects.{repo_root_toml}] +[projects."{repo_root_display}"] trust_level = "trusted" "# ); @@ -924,64 +698,18 @@ fn tempdir_with_models_dev_provider_config( ) -> Result { let codex_home = tempfile::tempdir()?; - let repo_root_display = repo_root.display().to_string(); - let repo_root_toml = toml::Value::String(repo_root_display).to_string(); - let provider_id_toml = toml::Value::String(provider_id.to_string()).to_string(); - let provider_name_toml = toml::Value::String(provider_name.to_string()).to_string(); - let model_toml = toml::Value::String(model.to_string()).to_string(); - let base_url_toml = toml::Value::String(base_url.to_string()).to_string(); - let env_key_toml = toml::Value::String(env_key.to_string()).to_string(); + let repo_root_display = repo_root.display(); let config_contents = format!( - r#"model_provider = {provider_id_toml} -model = {model_toml} -cli_auth_credentials_store = "file" + r#"model_provider = "{provider_id}" +model = "{model}" [model_providers.{provider_id}] -name = {provider_name_toml} -base_url = {base_url_toml} -env_key = {env_key_toml} -wire_api = "responses" - -[projects.{repo_root_toml}] -trust_level = "trusted" -"# - ); - std::fs::write(codex_home.path().join("config.toml"), config_contents)?; - - Ok(codex_home) -} - -fn tempdir_with_dual_provider_config( - repo_root: &Path, - startup_model: &str, - azure_base_url: &str, - copilot_base_url: &str, -) -> Result { - let codex_home = tempfile::tempdir()?; - - let repo_root_display = repo_root.display().to_string(); - let repo_root_toml = toml::Value::String(repo_root_display).to_string(); - let startup_model_toml = toml::Value::String(startup_model.to_string()).to_string(); - let azure_base_url_toml = toml::Value::String(azure_base_url.to_string()).to_string(); - let copilot_base_url_toml = toml::Value::String(copilot_base_url.to_string()).to_string(); - let config_contents = format!( - r#"model_provider = "azure-local" -model = {startup_model_toml} -cli_auth_credentials_store = "file" - -[model_providers.azure-local] -name = "Azure" -base_url = {azure_base_url_toml} -env_key = "AZURE_TEST_KEY" -wire_api = "responses" - -[model_providers.github-copilot] -name = "GitHub Copilot" -base_url = {copilot_base_url_toml} -env_key = "GITHUB_COPILOT_TOKEN" +name = "{provider_name}" +base_url = "{base_url}" +env_key = "{env_key}" wire_api = "responses" -[projects.{repo_root_toml}] +[projects."{repo_root_display}"] trust_level = "trusted" "# ); @@ -990,10 +718,6 @@ trust_level = "trusted" Ok(codex_home) } -fn is_models_endpoint_request(request_line: &str) -> bool { - request_line.starts_with("GET /v1/models ") || request_line.starts_with("GET /v1/models?") -} - fn spawn_models_server( response_json: serde_json::Value, ) -> Result<(String, thread::JoinHandle>>)> { @@ -1113,7 +837,7 @@ fn spawn_failing_ollama_discovery_server() let request_line = request.lines().next().unwrap_or_default().to_string(); requests.push(request_line.clone()); - let response = if is_models_endpoint_request(&request_line) + let response = if request_line.starts_with("GET /v1/models ") || request_line.starts_with("GET /api/tags ") { "HTTP/1.1 500 Internal Server Error\r\nContent-Type: application/json\r\nConnection: close\r\nContent-Length: 24\r\n\r\n{\"error\":\"test failure\"}" @@ -1192,30 +916,6 @@ async fn run_codex_cli_with_filter( filter: &str, prompt_after_switch: Option<&str>, extra_env: HashMap, -) -> Result { - run_codex_cli_with_filter_options( - codex_cli, - codex_home, - cwd, - startup_model_hint, - filter, - prompt_after_switch, - extra_env, - true, - ) - .await -} - -#[expect(clippy::too_many_arguments)] -async fn run_codex_cli_with_filter_options( - codex_cli: &Path, - codex_home: &Path, - cwd: &Path, - startup_model_hint: &str, - filter: &str, - prompt_after_switch: Option<&str>, - extra_env: HashMap, - send_escape_before_prompt: bool, ) -> Result { let mut env = HashMap::new(); env.insert("CODEX_HOME".to_string(), codex_home.display().to_string()); @@ -1251,8 +951,6 @@ async fn run_codex_cli_with_filter_options( let writer_tx = session.writer_sender(); let writer_for_input = writer_tx.clone(); let (startup_ready_tx, mut startup_ready_rx) = watch::channel(false); - let (reasoning_prompt_tx, mut reasoning_prompt_rx) = watch::channel(false); - const REASONING_CONFIRM_HINT: &str = "Press enter to confirm or esc to go back"; let filter = filter.to_string(); let prompt_after_switch = prompt_after_switch.map(std::borrow::ToOwned::to_owned); let input_task = tokio::spawn(async move { @@ -1268,8 +966,7 @@ async fn run_codex_cli_with_filter_options( } }) .await; - // Allow provider model refresh to complete before opening `/model`. - sleep(Duration::from_millis(2200)).await; + sleep(Duration::from_millis(500)).await; type_text_with_stabilization(&writer_for_input, "/model").await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; @@ -1277,31 +974,11 @@ async fn run_codex_cli_with_filter_options( type_text_with_stabilization(&writer_for_input, &filter).await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; - // Some models open an effort submenu after model selection. - // Confirm only when that submenu is actually shown. - let reasoning_prompt_visible = timeout(Duration::from_millis(1400), async { - loop { - if *reasoning_prompt_rx.borrow() { - break true; - } - if reasoning_prompt_rx.changed().await.is_err() { - break false; - } - } - }) - .await - .unwrap_or(false); - if reasoning_prompt_visible { - sleep(Duration::from_millis(200)).await; - let _ = writer_for_input.send(vec![b'\r']).await; - } if let Some(prompt) = prompt_after_switch { - // Allow model/effort selection to settle before prompt input. + // Allow model/effort selection to settle and close the picker before prompt input. sleep(Duration::from_millis(900)).await; - if send_escape_before_prompt { - let _ = writer_for_input.send(vec![27]).await; - sleep(Duration::from_millis(700)).await; - } + let _ = writer_for_input.send(vec![27]).await; + sleep(Duration::from_millis(700)).await; type_text_with_stabilization(&writer_for_input, &prompt).await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; @@ -1336,13 +1013,6 @@ async fn run_codex_cli_with_filter_options( let _ = startup_ready_tx.send(true); } } - if !*reasoning_prompt_tx.borrow() - && output - .windows(REASONING_CONFIRM_HINT.len()) - .any(|window| window == REASONING_CONFIRM_HINT.as_bytes()) - { - let _ = reasoning_prompt_tx.send(true); - } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -1382,48 +1052,24 @@ async fn run_codex_cli_with_filter_options( } fn find_codex_cli(cwd: &Path) -> Option { - // Only trust `cargo_bin("codex")` when Cargo exported the current test-run - // binary path. Otherwise the assert_cmd fallback can resolve a stale - // `target/debug/codex` artifact from an older build, which masks current TUI - // changes in these interactive regressions. - if cargo_test_exported_bin_path("codex") - && let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") - { + if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { return Some(path); } - - if let Some(path) = sibling_test_binary("codex") { - return Some(path); + if ensure_fallback_codex_binary_is_built(cwd).is_err() { + return None; } - - if ensure_fallback_codex_binary_is_built(cwd).is_ok() { - return sibling_test_binary("codex").or_else(|| { - let fallback = debug_target_binary(cwd, "codex"); - fallback.is_file().then_some(fallback) - }); + let fallback = cwd.join("codex-rs/target/debug/codex"); + if fallback.is_file() { + return Some(fallback); } - None } -fn cargo_test_exported_bin_path(name: &str) -> bool { - let env_key = format!("CARGO_BIN_EXE_{name}"); - if std::env::var_os(&env_key).is_some() { - return true; - } - let underscore_env_key = format!("CARGO_BIN_EXE_{}", name.replace('-', "_")); - std::env::var_os(underscore_env_key).is_some() -} - fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { static BUILD_RESULT: OnceLock> = OnceLock::new(); let result = BUILD_RESULT.get_or_init(|| { let status = Command::new("cargo") - .env("CARGO_INCREMENTAL", "0") - .env("CARGO_PROFILE_DEV_DEBUG", "0") .arg("build") - .arg("-p") - .arg("codex-cli") .arg("--bin") .arg("codex") .current_dir(repo_root.join("codex-rs")) @@ -1443,88 +1089,6 @@ fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { } } -fn sibling_test_binary(binary: &str) -> Option { - let current_exe = std::env::current_exe().ok()?; - let profile_dir = current_exe.parent()?.parent()?; - let candidate = profile_dir.join(format!("{binary}{}", std::env::consts::EXE_SUFFIX)); - candidate.is_file().then_some(candidate) -} - -fn debug_target_binary(repo_root: &Path, binary: &str) -> PathBuf { - repo_root - .join("codex-rs") - .join("target") - .join("debug") - .join(format!("{binary}{}", std::env::consts::EXE_SUFFIX)) -} - -fn read_http_request(stream: &mut TcpStream) -> Result)>> { - stream - .set_read_timeout(Some(Duration::from_secs(3))) - .context("failed to set read timeout")?; - - let mut raw = Vec::new(); - let mut chunk = [0_u8; 1024]; - loop { - match stream.read(&mut chunk) { - Ok(0) => break, - Ok(bytes_read) => { - raw.extend_from_slice(&chunk[..bytes_read]); - if raw.windows(4).any(|window| window == b"\r\n\r\n") { - break; - } - } - Err(err) - if err.kind() == std::io::ErrorKind::WouldBlock - || err.kind() == std::io::ErrorKind::TimedOut => - { - break; - } - Err(err) => return Err(err.into()), - } - } - - let Some(header_end) = raw - .windows(4) - .position(|window| window == b"\r\n\r\n") - .map(|index| index + 4) - else { - return Ok(None); - }; - let headers = String::from_utf8_lossy(&raw[..header_end]).to_string(); - let request_line = headers - .lines() - .next() - .context("missing request line")? - .to_string(); - let content_length = headers - .lines() - .find_map(|line| { - let lower = line.to_ascii_lowercase(); - lower - .strip_prefix("content-length:") - .and_then(|value| value.trim().parse::().ok()) - }) - .unwrap_or(0); - - let mut body = raw[header_end..].to_vec(); - while body.len() < content_length { - match stream.read(&mut chunk) { - Ok(0) => break, - Ok(bytes_read) => body.extend_from_slice(&chunk[..bytes_read]), - Err(err) - if err.kind() == std::io::ErrorKind::WouldBlock - || err.kind() == std::io::ErrorKind::TimedOut => - { - break; - } - Err(err) => return Err(err.into()), - } - } - - Ok(Some((request_line, body))) -} - fn spawn_openai_compat_models_and_responses_server( models_response_json: serde_json::Value, response_model: &str, @@ -1557,13 +1121,72 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage { match listener.accept() { Ok((mut stream, _)) => { - let Some((request_line, body_bytes)) = read_http_request(&mut stream)? else { - continue; - }; + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .context("failed to set read timeout")?; + + let mut raw_request = Vec::new(); + let mut chunk = [0_u8; 1024]; + loop { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => { + raw_request.extend_from_slice(&chunk[..bytes_read]); + if raw_request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } + + let header_end = raw_request + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|index| index + 4) + .context("HTTP headers terminator not found")?; + let headers = String::from_utf8_lossy(&raw_request[..header_end]).to_string(); + let request_line = headers + .lines() + .next() + .context("missing request line")? + .to_string(); + + let content_length = headers + .lines() + .find_map(|line| { + let lower = line.to_ascii_lowercase(); + lower + .strip_prefix("content-length:") + .and_then(|value| value.trim().parse::().ok()) + }) + .unwrap_or(0); + + let mut body_bytes = raw_request[header_end..].to_vec(); + while body_bytes.len() < content_length { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => body_bytes.extend_from_slice(&chunk[..bytes_read]), + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } + let body = String::from_utf8_lossy(&body_bytes).to_string(); requests.push(format!("{request_line}\n{body}")); - let response = if is_models_endpoint_request(&request_line) { + let response = if request_line.starts_with("GET /v1/models ") { format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", models_response_body.len(), @@ -1587,7 +1210,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(20)); + idle_deadline = Some(Instant::now() + Duration::from_secs(8)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); @@ -1601,18 +1224,28 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage Ok((format!("http://{address}/v1"), handle)) } -fn spawn_models_dev_catalog_server( - response_json: serde_json::Value, +fn spawn_openai_compat_models_with_chat_completions_fallback_server( + models_response_json: serde_json::Value, + fallback_model: &str, + answer_text: &str, ) -> Result<(String, thread::JoinHandle>>)> { let listener = TcpListener::bind("127.0.0.1:0")?; let address = listener.local_addr()?; - let response_body = response_json.to_string(); + let models_response_body = models_response_json.to_string(); + let fallback_model_json = serde_json::to_string(fallback_model)?; + let answer_json = serde_json::to_string(answer_text)?; + let responses_error = format!( + "{{\"error\":{{\"message\":\"model {fallback_model} does not support Responses API.\",\"code\":\"unsupported_api_for_model\"}}}}" + ); + let chat_completions_response = format!( + "{{\"id\":\"chatcmpl-fallback-1\",\"choices\":[{{\"index\":0,\"message\":{{\"role\":\"assistant\",\"content\":{answer_json}}},\"finish_reason\":\"stop\"}}],\"model\":{fallback_model_json}}}" + ); let handle = thread::spawn(move || { listener .set_nonblocking(true) .context("failed to set nonblocking listener")?; let mut requests = Vec::new(); - let hard_deadline = Instant::now() + Duration::from_secs(20); + let hard_deadline = Instant::now() + Duration::from_secs(30); let mut idle_deadline: Option = None; while Instant::now() < hard_deadline && idle_deadline @@ -1622,16 +1255,17 @@ fn spawn_models_dev_catalog_server( match listener.accept() { Ok((mut stream, _)) => { stream - .set_read_timeout(Some(Duration::from_secs(2))) + .set_read_timeout(Some(Duration::from_secs(3))) .context("failed to set read timeout")?; - let mut request = Vec::new(); + + let mut raw_request = Vec::new(); let mut chunk = [0_u8; 1024]; loop { match stream.read(&mut chunk) { Ok(0) => break, Ok(bytes_read) => { - request.extend_from_slice(&chunk[..bytes_read]); - if request.windows(4).any(|window| window == b"\r\n\r\n") { + raw_request.extend_from_slice(&chunk[..bytes_read]); + if raw_request.windows(4).any(|window| window == b"\r\n\r\n") { break; } } @@ -1645,78 +1279,47 @@ fn spawn_models_dev_catalog_server( } } - let request = String::from_utf8_lossy(&request).to_string(); - let request_line = request.lines().next().unwrap_or_default().to_string(); - requests.push(request_line.clone()); - - let response = if request_line.starts_with("GET /api.json ") { - format!( - "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", - response_body.len(), - response_body - ) - } else { - "HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" - .to_string() - }; - stream - .write_all(response.as_bytes()) - .context("failed to write models.dev response")?; - stream - .flush() - .context("failed to flush models.dev response")?; - - idle_deadline = Some(Instant::now() + Duration::from_secs(2)); - } - Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { - thread::sleep(Duration::from_millis(20)); - } - Err(err) => return Err(err.into()), - } - } - Ok(requests) - }); - - Ok((format!("http://{address}/api.json"), handle)) -} + let header_end = raw_request + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|index| index + 4) + .context("HTTP headers terminator not found")?; + let headers = String::from_utf8_lossy(&raw_request[..header_end]).to_string(); + let request_line = headers + .lines() + .next() + .context("missing request line")? + .to_string(); + + let content_length = headers + .lines() + .find_map(|line| { + let lower = line.to_ascii_lowercase(); + lower + .strip_prefix("content-length:") + .and_then(|value| value.trim().parse::().ok()) + }) + .unwrap_or(0); + + let mut body_bytes = raw_request[header_end..].to_vec(); + while body_bytes.len() < content_length { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => body_bytes.extend_from_slice(&chunk[..bytes_read]), + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } -fn spawn_openai_compat_models_with_chat_completions_fallback_server( - models_response_json: serde_json::Value, - fallback_model: &str, - answer_text: &str, -) -> Result<(String, thread::JoinHandle>>)> { - let listener = TcpListener::bind("127.0.0.1:0")?; - let address = listener.local_addr()?; - let models_response_body = models_response_json.to_string(); - let fallback_model_json = serde_json::to_string(fallback_model)?; - let answer_json = serde_json::to_string(answer_text)?; - let responses_error = format!( - "{{\"error\":{{\"message\":\"model {fallback_model} does not support Responses API.\",\"code\":\"unsupported_api_for_model\"}}}}" - ); - let chat_completions_response = format!( - "{{\"id\":\"chatcmpl-fallback-1\",\"choices\":[{{\"index\":0,\"message\":{{\"role\":\"assistant\",\"content\":{answer_json}}},\"finish_reason\":\"stop\"}}],\"model\":{fallback_model_json}}}" - ); - let handle = thread::spawn(move || { - listener - .set_nonblocking(true) - .context("failed to set nonblocking listener")?; - let mut requests = Vec::new(); - let hard_deadline = Instant::now() + Duration::from_secs(30); - let mut idle_deadline: Option = None; - while Instant::now() < hard_deadline - && idle_deadline - .map(|deadline| Instant::now() < deadline) - .unwrap_or(true) - { - match listener.accept() { - Ok((mut stream, _)) => { - let Some((request_line, body_bytes)) = read_http_request(&mut stream)? else { - continue; - }; let body = String::from_utf8_lossy(&body_bytes).to_string(); requests.push(format!("{request_line}\n{body}")); - let response = if is_models_endpoint_request(&request_line) { + let response = if request_line.starts_with("GET /v1/models ") { format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", models_response_body.len(), @@ -1746,7 +1349,7 @@ fn spawn_openai_compat_models_with_chat_completions_fallback_server( .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(20)); + idle_deadline = Some(Instant::now() + Duration::from_secs(8)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); @@ -1760,15 +1363,13 @@ fn spawn_openai_compat_models_with_chat_completions_fallback_server( Ok((format!("http://{address}/v1"), handle)) } -type ResponsesServerHandle = thread::JoinHandle>>; - fn spawn_models_dev_and_responses_server( models_dev_provider_id: &str, models_dev_provider_name: &str, model_ids: &[&str], response_model: &str, answer_text: &str, -) -> Result<(String, String, ResponsesServerHandle)> { +) -> Result<(String, String, thread::JoinHandle>>)> { let listener = TcpListener::bind("127.0.0.1:0")?; let address = listener.local_addr()?; @@ -1828,9 +1429,68 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage { match listener.accept() { Ok((mut stream, _)) => { - let Some((request_line, body_bytes)) = read_http_request(&mut stream)? else { - continue; - }; + stream + .set_read_timeout(Some(Duration::from_secs(3))) + .context("failed to set read timeout")?; + + let mut raw_request = Vec::new(); + let mut chunk = [0_u8; 1024]; + loop { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => { + raw_request.extend_from_slice(&chunk[..bytes_read]); + if raw_request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } + + let header_end = raw_request + .windows(4) + .position(|window| window == b"\r\n\r\n") + .map(|index| index + 4) + .context("HTTP headers terminator not found")?; + let headers = String::from_utf8_lossy(&raw_request[..header_end]).to_string(); + let request_line = headers + .lines() + .next() + .context("missing request line")? + .to_string(); + + let content_length = headers + .lines() + .find_map(|line| { + let lower = line.to_ascii_lowercase(); + lower + .strip_prefix("content-length:") + .and_then(|value| value.trim().parse::().ok()) + }) + .unwrap_or(0); + + let mut body_bytes = raw_request[header_end..].to_vec(); + while body_bytes.len() < content_length { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => body_bytes.extend_from_slice(&chunk[..bytes_read]), + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } + let body = String::from_utf8_lossy(&body_bytes).to_string(); requests.push(format!("{request_line}\n{body}")); @@ -1858,7 +1518,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(20)); + idle_deadline = Some(Instant::now() + Duration::from_secs(8)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); @@ -1869,7 +1529,11 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage Ok(requests) }); - Ok((provider_api, format!("http://{address}/api.json"), handle)) + Ok(( + provider_api.clone(), + format!("http://{address}/api.json"), + handle, + )) } async fn type_text_with_stabilization(writer: &tokio::sync::mpsc::Sender>, text: &str) { From 50e134564e65e4cf9fa29fe7da2dea0a3c8edbba Mon Sep 17 00:00:00 2001 From: engineer Date: Sat, 14 Mar 2026 18:55:44 -0700 Subject: [PATCH 2/9] Fix Copilot localhost model discovery and in-session switch coverage --- .../tui/tests/suite/model_switching_e2e.rs | 529 +++++++++++++++--- 1 file changed, 452 insertions(+), 77 deletions(-) diff --git a/codex-rs/tui/tests/suite/model_switching_e2e.rs b/codex-rs/tui/tests/suite/model_switching_e2e.rs index f184c4ec379f..6e71f009882c 100644 --- a/codex-rs/tui/tests/suite/model_switching_e2e.rs +++ b/codex-rs/tui/tests/suite/model_switching_e2e.rs @@ -32,7 +32,36 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( eprintln!("skipping integration test because codex binary is unavailable"); return Ok(()); }; - let codex_home = tempdir_with_catalog_and_config(&repo_root)?; + let codex_home = tempdir_with_ollama_config(&repo_root, "gpt-5.3-codex")?; + let (first_base_url, first_server) = spawn_models_server(serde_json::json!({ + "object": "list", + "data": [ + {"id": "gpt-5.3-codex", "object": "model"}, + {"id": "claude-opus-4.6", "object": "model"} + ] + }))?; + let (first_models_dev_url, first_models_dev_server) = + spawn_models_dev_catalog_server(serde_json::json!({ + "github-copilot": { + "id": "github-copilot", + "name": "GitHub Copilot", + "api": "https://api.githubcopilot.com/v1", + "models": { + "gpt-5.3-codex": {"id": "gpt-5.3-codex", "name": "gpt-5.3-codex"} + } + }, + "lmstudio": { + "id": "lmstudio", + "name": "LM Studio", + "api": "http://127.0.0.1:1234/v1", + "models": { + "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} + } + } + }))?; + let mut first_env = HashMap::new(); + first_env.insert("CODEX_OSS_BASE_URL".to_string(), first_base_url); + first_env.insert("CODEX_MODELS_DEV_URL".to_string(), first_models_dev_url); let first_output = run_codex_cli_with_filter( &codex_cli, @@ -41,16 +70,58 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( "gpt-5.3-codex", "claude", None, - HashMap::new(), + first_env, ) .await .context("switch from gpt-5.3-codex to claude-opus-4.6")?; assert_model_and_provider_in_config( codex_home.path(), "claude-opus-4.6", - "github-copilot", + "ollama", &first_output, )?; + let first_requests = first_server + .join() + .expect("first model server should join")?; + anyhow::ensure!( + first_requests + .iter() + .any(|line| is_models_endpoint_request(line)), + "expected GET /v1/models in first run, got: {first_requests:?}" + ); + let _first_models_dev_requests = first_models_dev_server + .join() + .expect("first models.dev server should join")?; + + let (second_base_url, second_server) = spawn_models_server(serde_json::json!({ + "object": "list", + "data": [ + {"id": "gpt-5.3-codex", "object": "model"}, + {"id": "claude-opus-4.6", "object": "model"} + ] + }))?; + let (second_models_dev_url, second_models_dev_server) = + spawn_models_dev_catalog_server(serde_json::json!({ + "github-copilot": { + "id": "github-copilot", + "name": "GitHub Copilot", + "api": "https://api.githubcopilot.com/v1", + "models": { + "gpt-5.3-codex": {"id": "gpt-5.3-codex", "name": "gpt-5.3-codex"} + } + }, + "lmstudio": { + "id": "lmstudio", + "name": "LM Studio", + "api": "http://127.0.0.1:1234/v1", + "models": { + "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} + } + } + }))?; + let mut second_env = HashMap::new(); + second_env.insert("CODEX_OSS_BASE_URL".to_string(), second_base_url); + second_env.insert("CODEX_MODELS_DEV_URL".to_string(), second_models_dev_url); let second_output = run_codex_cli_with_filter( &codex_cli, @@ -59,16 +130,28 @@ async fn model_picker_search_switches_and_persists_across_restarts() -> Result<( "claude-opus-4.6", "gpt-5.3-codex", None, - HashMap::new(), + second_env, ) .await .context("switch back from claude-opus-4.6 to gpt-5.3-codex")?; assert_model_and_provider_in_config( codex_home.path(), "gpt-5.3-codex", - "github-copilot", + "ollama", &second_output, )?; + let second_requests = second_server + .join() + .expect("second model server should join")?; + anyhow::ensure!( + second_requests + .iter() + .any(|line| is_models_endpoint_request(line)), + "expected GET /v1/models in second run, got: {second_requests:?}" + ); + let _second_models_dev_requests = second_models_dev_server + .join() + .expect("second models.dev server should join")?; Ok(()) } @@ -115,9 +198,7 @@ async fn model_picker_ignores_openai_compat_models_disabled_for_picker() -> Resu let requests = server.join().expect("model listing server should join")?; assert!( - requests - .iter() - .any(|line| line.starts_with("GET /v1/models ")), + requests.iter().any(|line| is_models_endpoint_request(line)), "expected at least one GET /v1/models request, got: {requests:?}" ); @@ -163,9 +244,7 @@ async fn ollama_model_picker_uses_local_models_endpoint_and_switches() -> Result let requests = server.join().expect("model listing server should join")?; assert!( - requests - .iter() - .any(|line| line.starts_with("GET /v1/models ")), + requests.iter().any(|line| is_models_endpoint_request(line)), "expected at least one GET /v1/models request, got: {requests:?}" ); @@ -195,7 +274,7 @@ async fn ollama_model_picker_does_not_switch_to_bundled_gpt_when_discovery_fails codex_home.path(), &repo_root, "llama3.2:latest", - "gpt-5", + "github-copilot/gpt-5", None, env, ) @@ -205,9 +284,7 @@ async fn ollama_model_picker_does_not_switch_to_bundled_gpt_when_discovery_fails let requests = server.join().expect("discovery server should join")?; assert!( - requests - .iter() - .any(|line| line.starts_with("GET /v1/models ")), + requests.iter().any(|line| is_models_endpoint_request(line)), "expected GET /v1/models request, got: {requests:?}" ); @@ -265,7 +342,7 @@ async fn ollama_model_switch_then_prompt_uses_responses_api() -> Result<()> { anyhow::ensure!( requests .iter() - .any(|request| request.starts_with("GET /v1/models ")), + .any(|request| is_models_endpoint_request(request)), "expected GET /v1/models request; got requests: {requests:?}" ); let responses_request = requests @@ -320,7 +397,7 @@ async fn copilot_model_switch_then_prompt_uses_responses_api_without_cli_provide "copilot-test-b", answer_text, )?; - let codex_home = tempdir_with_local_copilot_config( + let codex_home = tempdir_with_github_copilot_config( &repo_root, "copilot-test-a", base_url.as_str(), @@ -329,7 +406,7 @@ async fn copilot_model_switch_then_prompt_uses_responses_api_without_cli_provide let mut env = HashMap::new(); env.insert( - "COPILOT_TEST_KEY".to_string(), + "GITHUB_COPILOT_TOKEN".to_string(), "test-copilot-token".to_string(), ); let output = run_codex_cli_with_filter( @@ -346,7 +423,7 @@ async fn copilot_model_switch_then_prompt_uses_responses_api_without_cli_provide assert_model_and_provider_in_config( codex_home.path(), "copilot-test-b", - "copilot-local", + "github-copilot", &output, )?; @@ -382,7 +459,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( let prompt = "When the first gpt model was released?"; let answer_text = "The first GPT model (GPT-1) was released in June 2018."; - let selected_model = "claude-opus-4.6"; + let selected_model = "claude-4.6-opus"; let (base_url, server) = spawn_openai_compat_models_with_chat_completions_fallback_server( serde_json::json!({ "object": "list", @@ -404,7 +481,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( selected_model, answer_text, )?; - let codex_home = tempdir_with_local_copilot_config( + let codex_home = tempdir_with_github_copilot_config( &repo_root, "copilot-test-a", base_url.as_str(), @@ -413,7 +490,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( let mut env = HashMap::new(); env.insert( - "COPILOT_TEST_KEY".to_string(), + "GITHUB_COPILOT_TOKEN".to_string(), "test-copilot-token".to_string(), ); let output = run_codex_cli_with_filter( @@ -430,7 +507,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( assert_model_and_provider_in_config( codex_home.path(), selected_model, - "copilot-local", + "github-copilot", &output, )?; anyhow::ensure!( @@ -444,7 +521,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( .find(|request| request.starts_with("POST /v1/responses ")) .context("missing POST /v1/responses request")?; anyhow::ensure!( - responses_request.contains("\"model\":\"claude-opus-4.6\""), + responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), "expected switched model in /responses request body; request: {responses_request}" ); @@ -453,7 +530,7 @@ async fn copilot_chat_completions_only_model_switch_then_prompt_preserves_model( .find(|request| request.starts_with("POST /v1/chat/completions ")) .context("missing POST /v1/chat/completions request")?; anyhow::ensure!( - chat_request.contains("\"model\":\"claude-opus-4.6\""), + chat_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), "expected switched model in /chat/completions request body; request: {chat_request}" ); anyhow::ensure!( @@ -581,42 +658,167 @@ async fn models_dev_provider_config_parses_custom_provider() -> Result<()> { Ok(()) } -fn tempdir_with_catalog_and_config(repo_root: &Path) -> Result { - let codex_home = tempfile::tempdir()?; +#[tokio::test] +async fn cross_provider_model_switch_applies_immediately_without_manual_new_session() -> Result<()> +{ + // run_codex_cli_with_filter() does not work on Windows due to PTY limitations. + if cfg!(windows) { + return Ok(()); + } - let source_catalog_path = codex_utils_cargo_bin::find_resource!("../core/models.json")?; - let source_catalog = std::fs::read_to_string(&source_catalog_path)?; - let mut source_catalog: JsonValue = serde_json::from_str(&source_catalog)?; - let models = source_catalog - .get_mut("models") - .and_then(JsonValue::as_array_mut) - .context("models array missing")?; - let template = models.first().cloned().context("models array is empty")?; + let repo_root = codex_utils_cargo_bin::repo_root()?; + let Some(codex_cli) = find_codex_cli(&repo_root) else { + eprintln!("skipping integration test because codex binary is unavailable"); + return Ok(()); + }; - let gpt_model = model_from_template(&template, "gpt-5.3-codex", "gpt-5.3-codex", 0)?; - let claude_model = model_from_template(&template, "claude-opus-4.6", "claude-opus-4.6", 1)?; - *models = vec![gpt_model, claude_model]; + let startup_model = "azure-model-a"; + let selected_model = "claude-4.6-opus"; + let prompt = "Name the provider handling this prompt."; + let azure_answer = "Azure should not handle the post-switch prompt."; + let copilot_answer = "Copilot provider handled this prompt immediately."; - let custom_catalog_path = codex_home.path().join("catalog.json"); - std::fs::write( - &custom_catalog_path, - serde_json::to_string(&source_catalog)?, + let (azure_base_url, azure_server) = spawn_openai_compat_models_and_responses_server( + serde_json::json!({ + "object": "list", + "data": [ + {"id": startup_model, "object": "model"}, + {"id": "azure-model-b", "object": "model"} + ] + }), + startup_model, + azure_answer, + )?; + let (copilot_base_url, copilot_server) = spawn_openai_compat_models_and_responses_server( + serde_json::json!({ + "object": "list", + "data": [ + {"id": selected_model, "object": "model"} + ] + }), + selected_model, + copilot_answer, + )?; + let (models_dev_url, models_dev_server) = spawn_models_dev_catalog_server(serde_json::json!({ + "azure": { + "id": "azure", + "name": "Azure", + "api": azure_base_url.clone(), + "models": { + startup_model: {"id": startup_model, "name": startup_model}, + "azure-model-b": {"id": "azure-model-b", "name": "azure-model-b"} + } + }, + "github-copilot": { + "id": "github-copilot", + "name": "GitHub Copilot", + "api": copilot_base_url.clone(), + "models": { + selected_model: {"id": selected_model, "name": selected_model} + } + }, + "ollama": { + "id": "ollama", + "name": "Ollama", + "api": "http://127.0.0.1:11434/v1", + "models": { + "llama3.2:latest": {"id": "llama3.2:latest", "name": "llama3.2:latest"} + } + }, + "lmstudio": { + "id": "lmstudio", + "name": "LM Studio", + "api": "http://127.0.0.1:1234/v1", + "models": { + "qwen2.5-coder:7b": {"id": "qwen2.5-coder:7b", "name": "qwen2.5-coder:7b"} + } + } + }))?; + let codex_home = tempdir_with_dual_provider_config( + &repo_root, + startup_model, + azure_base_url.as_str(), + copilot_base_url.as_str(), )?; - let repo_root_display = repo_root.display(); - let catalog_display = custom_catalog_path.display(); - let config_contents = format!( - r#"model_provider = "github-copilot" -model = "gpt-5.3-codex" -model_catalog_json = "{catalog_display}" + let mut env = HashMap::new(); + env.insert("AZURE_TEST_KEY".to_string(), "test-azure-token".to_string()); + env.insert( + "GITHUB_COPILOT_TOKEN".to_string(), + "test-copilot-token".to_string(), + ); + env.insert("CODEX_MODELS_DEV_URL".to_string(), models_dev_url); -[projects."{repo_root_display}"] -trust_level = "trusted" -"# + let output = run_codex_cli_with_filter( + &codex_cli, + codex_home.path(), + &repo_root, + startup_model, + selected_model, + Some(prompt), + env, + ) + .await + .context("cross-provider switch via /model should apply immediately in-session")?; + + let azure_requests = azure_server + .join() + .expect("azure test server should join")?; + let copilot_requests = copilot_server + .join() + .expect("copilot test server should join")?; + let models_dev_requests = models_dev_server + .join() + .expect("models.dev server should join")?; + + anyhow::ensure!( + copilot_requests + .iter() + .any(|request| is_models_endpoint_request(request)), + "expected Copilot provider /models discovery request; got requests: {copilot_requests:?}" + ); + anyhow::ensure!( + models_dev_requests + .iter() + .any(|request| request.starts_with("GET /api.json ")), + "expected GET /api.json request; got requests: {models_dev_requests:?}" ); - std::fs::write(codex_home.path().join("config.toml"), config_contents)?; - Ok(codex_home) + assert_model_and_provider_in_config( + codex_home.path(), + selected_model, + "github-copilot", + &output, + )?; + anyhow::ensure!( + output.contains(copilot_answer), + "expected Copilot answer after in-session provider switch, got: {output}" + ); + anyhow::ensure!( + !output.contains("Start a new session to apply provider change"), + "did not expect restart hint after provider switch, got: {output}" + ); + + let copilot_responses_request = copilot_requests + .iter() + .find(|request| request.starts_with("POST /v1/responses ")) + .context("missing Copilot POST /v1/responses request after provider switch")?; + anyhow::ensure!( + copilot_responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), + "expected selected model in Copilot /responses request body; request: {copilot_responses_request}" + ); + anyhow::ensure!( + copilot_responses_request.contains(prompt), + "expected prompt text in Copilot /responses request body; request: {copilot_responses_request}" + ); + anyhow::ensure!( + !azure_requests + .iter() + .any(|request| request.starts_with("POST /v1/responses ")), + "did not expect post-switch /responses requests against Azure provider; requests: {azure_requests:?}" + ); + + Ok(()) } fn tempdir_with_ollama_config(repo_root: &Path, model: &str) -> Result { @@ -626,6 +828,7 @@ fn tempdir_with_ollama_config(repo_root: &Path, model: &str) -> Result let config_contents = format!( r#"model_provider = "ollama" model = "{model}" +cli_auth_credentials_store = "file" [projects."{repo_root_display}"] trust_level = "trusted" @@ -636,7 +839,7 @@ trust_level = "trusted" Ok(codex_home) } -fn tempdir_with_local_copilot_config( +fn tempdir_with_github_copilot_config( repo_root: &Path, model: &str, base_url: &str, @@ -669,14 +872,15 @@ fn tempdir_with_local_copilot_config( let repo_root_display = repo_root.display(); let catalog_display = custom_catalog_path.display(); let config_contents = format!( - r#"model_provider = "copilot-local" + r#"model_provider = "github-copilot" model = "{model}" model_catalog_json = "{catalog_display}" +cli_auth_credentials_store = "file" -[model_providers.copilot-local] +[model_providers.github-copilot] name = "GitHub Copilot" base_url = "{base_url}" -env_key = "COPILOT_TEST_KEY" +env_key = "GITHUB_COPILOT_TOKEN" wire_api = "responses" [projects."{repo_root_display}"] @@ -702,6 +906,7 @@ fn tempdir_with_models_dev_provider_config( let config_contents = format!( r#"model_provider = "{provider_id}" model = "{model}" +cli_auth_credentials_store = "file" [model_providers.{provider_id}] name = "{provider_name}" @@ -718,6 +923,45 @@ trust_level = "trusted" Ok(codex_home) } +fn tempdir_with_dual_provider_config( + repo_root: &Path, + startup_model: &str, + azure_base_url: &str, + copilot_base_url: &str, +) -> Result { + let codex_home = tempfile::tempdir()?; + + let repo_root_display = repo_root.display(); + let config_contents = format!( + r#"model_provider = "azure-local" +model = "{startup_model}" +cli_auth_credentials_store = "file" + +[model_providers.azure-local] +name = "Azure" +base_url = "{azure_base_url}" +env_key = "AZURE_TEST_KEY" +wire_api = "responses" + +[model_providers.github-copilot] +name = "GitHub Copilot" +base_url = "{copilot_base_url}" +env_key = "GITHUB_COPILOT_TOKEN" +wire_api = "responses" + +[projects."{repo_root_display}"] +trust_level = "trusted" +"# + ); + std::fs::write(codex_home.path().join("config.toml"), config_contents)?; + + Ok(codex_home) +} + +fn is_models_endpoint_request(request_line: &str) -> bool { + request_line.starts_with("GET /v1/models ") || request_line.starts_with("GET /v1/models?") +} + fn spawn_models_server( response_json: serde_json::Value, ) -> Result<(String, thread::JoinHandle>>)> { @@ -837,7 +1081,7 @@ fn spawn_failing_ollama_discovery_server() let request_line = request.lines().next().unwrap_or_default().to_string(); requests.push(request_line.clone()); - let response = if request_line.starts_with("GET /v1/models ") + let response = if is_models_endpoint_request(&request_line) || request_line.starts_with("GET /api/tags ") { "HTTP/1.1 500 Internal Server Error\r\nContent-Type: application/json\r\nConnection: close\r\nContent-Length: 24\r\n\r\n{\"error\":\"test failure\"}" @@ -916,6 +1160,29 @@ async fn run_codex_cli_with_filter( filter: &str, prompt_after_switch: Option<&str>, extra_env: HashMap, +) -> Result { + run_codex_cli_with_filter_options( + codex_cli, + codex_home, + cwd, + startup_model_hint, + filter, + prompt_after_switch, + extra_env, + true, + ) + .await +} + +async fn run_codex_cli_with_filter_options( + codex_cli: &Path, + codex_home: &Path, + cwd: &Path, + startup_model_hint: &str, + filter: &str, + prompt_after_switch: Option<&str>, + extra_env: HashMap, + send_escape_before_prompt: bool, ) -> Result { let mut env = HashMap::new(); env.insert("CODEX_HOME".to_string(), codex_home.display().to_string()); @@ -951,6 +1218,8 @@ async fn run_codex_cli_with_filter( let writer_tx = session.writer_sender(); let writer_for_input = writer_tx.clone(); let (startup_ready_tx, mut startup_ready_rx) = watch::channel(false); + let (reasoning_prompt_tx, mut reasoning_prompt_rx) = watch::channel(false); + const REASONING_CONFIRM_HINT: &str = "Press enter to confirm or esc to go back"; let filter = filter.to_string(); let prompt_after_switch = prompt_after_switch.map(std::borrow::ToOwned::to_owned); let input_task = tokio::spawn(async move { @@ -966,7 +1235,8 @@ async fn run_codex_cli_with_filter( } }) .await; - sleep(Duration::from_millis(500)).await; + // Allow provider model refresh to complete before opening `/model`. + sleep(Duration::from_millis(2200)).await; type_text_with_stabilization(&writer_for_input, "/model").await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; @@ -974,11 +1244,31 @@ async fn run_codex_cli_with_filter( type_text_with_stabilization(&writer_for_input, &filter).await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; + // Some models open an effort submenu after model selection. + // Confirm only when that submenu is actually shown. + let reasoning_prompt_visible = timeout(Duration::from_millis(1400), async { + loop { + if *reasoning_prompt_rx.borrow() { + break true; + } + if reasoning_prompt_rx.changed().await.is_err() { + break false; + } + } + }) + .await + .unwrap_or(false); + if reasoning_prompt_visible { + sleep(Duration::from_millis(200)).await; + let _ = writer_for_input.send(vec![b'\r']).await; + } if let Some(prompt) = prompt_after_switch { - // Allow model/effort selection to settle and close the picker before prompt input. + // Allow model/effort selection to settle before prompt input. sleep(Duration::from_millis(900)).await; - let _ = writer_for_input.send(vec![27]).await; - sleep(Duration::from_millis(700)).await; + if send_escape_before_prompt { + let _ = writer_for_input.send(vec![27]).await; + sleep(Duration::from_millis(700)).await; + } type_text_with_stabilization(&writer_for_input, &prompt).await; sleep(Duration::from_millis(120)).await; let _ = writer_for_input.send(vec![b'\r']).await; @@ -1013,6 +1303,13 @@ async fn run_codex_cli_with_filter( let _ = startup_ready_tx.send(true); } } + if !*reasoning_prompt_tx.borrow() + && output + .windows(REASONING_CONFIRM_HINT.len()) + .any(|window| window == REASONING_CONFIRM_HINT.as_bytes()) + { + let _ = reasoning_prompt_tx.send(true); + } } Err(tokio::sync::broadcast::error::RecvError::Closed) => break exit_rx.await, Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => {} @@ -1052,17 +1349,16 @@ async fn run_codex_cli_with_filter( } fn find_codex_cli(cwd: &Path) -> Option { - if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { - return Some(path); - } - if ensure_fallback_codex_binary_is_built(cwd).is_err() { - return None; - } - let fallback = cwd.join("codex-rs/target/debug/codex"); - if fallback.is_file() { - return Some(fallback); + // Always build a fresh local binary once per test process so PTY E2E + // assertions exercise the current source tree instead of stale artifacts. + if ensure_fallback_codex_binary_is_built(cwd).is_ok() { + let fallback = cwd.join("codex-rs/target/debug/codex"); + if fallback.is_file() { + return Some(fallback); + } } - None + + codex_utils_cargo_bin::cargo_bin("codex").ok() } fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { @@ -1186,7 +1482,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage let body = String::from_utf8_lossy(&body_bytes).to_string(); requests.push(format!("{request_line}\n{body}")); - let response = if request_line.starts_with("GET /v1/models ") { + let response = if is_models_endpoint_request(&request_line) { format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", models_response_body.len(), @@ -1210,7 +1506,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(8)); + idle_deadline = Some(Instant::now() + Duration::from_secs(20)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); @@ -1224,6 +1520,85 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage Ok((format!("http://{address}/v1"), handle)) } +fn spawn_models_dev_catalog_server( + response_json: serde_json::Value, +) -> Result<(String, thread::JoinHandle>>)> { + let listener = TcpListener::bind("127.0.0.1:0")?; + let address = listener.local_addr()?; + let response_body = response_json.to_string(); + let handle = thread::spawn(move || { + listener + .set_nonblocking(true) + .context("failed to set nonblocking listener")?; + let mut requests = Vec::new(); + let hard_deadline = Instant::now() + Duration::from_secs(20); + let mut idle_deadline: Option = None; + while Instant::now() < hard_deadline + && idle_deadline + .map(|deadline| Instant::now() < deadline) + .unwrap_or(true) + { + match listener.accept() { + Ok((mut stream, _)) => { + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .context("failed to set read timeout")?; + let mut request = Vec::new(); + let mut chunk = [0_u8; 1024]; + loop { + match stream.read(&mut chunk) { + Ok(0) => break, + Ok(bytes_read) => { + request.extend_from_slice(&chunk[..bytes_read]); + if request.windows(4).any(|window| window == b"\r\n\r\n") { + break; + } + } + Err(err) + if err.kind() == std::io::ErrorKind::WouldBlock + || err.kind() == std::io::ErrorKind::TimedOut => + { + break; + } + Err(err) => return Err(err.into()), + } + } + + let request = String::from_utf8_lossy(&request).to_string(); + let request_line = request.lines().next().unwrap_or_default().to_string(); + requests.push(request_line.clone()); + + let response = if request_line.starts_with("GET /api.json ") { + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + response_body.len(), + response_body + ) + } else { + "HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + .to_string() + }; + stream + .write_all(response.as_bytes()) + .context("failed to write models.dev response")?; + stream + .flush() + .context("failed to flush models.dev response")?; + + idle_deadline = Some(Instant::now() + Duration::from_secs(2)); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(20)); + } + Err(err) => return Err(err.into()), + } + } + Ok(requests) + }); + + Ok((format!("http://{address}/api.json"), handle)) +} + fn spawn_openai_compat_models_with_chat_completions_fallback_server( models_response_json: serde_json::Value, fallback_model: &str, @@ -1319,7 +1694,7 @@ fn spawn_openai_compat_models_with_chat_completions_fallback_server( let body = String::from_utf8_lossy(&body_bytes).to_string(); requests.push(format!("{request_line}\n{body}")); - let response = if request_line.starts_with("GET /v1/models ") { + let response = if is_models_endpoint_request(&request_line) { format!( "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", models_response_body.len(), @@ -1349,7 +1724,7 @@ fn spawn_openai_compat_models_with_chat_completions_fallback_server( .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(8)); + idle_deadline = Some(Instant::now() + Duration::from_secs(20)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); @@ -1518,7 +1893,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage .flush() .context("failed to flush test server response")?; - idle_deadline = Some(Instant::now() + Duration::from_secs(8)); + idle_deadline = Some(Instant::now() + Duration::from_secs(20)); } Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { thread::sleep(Duration::from_millis(20)); From a47ba21bb23641f8db35280e068a17b47d6a0f8d Mon Sep 17 00:00:00 2001 From: engineer Date: Sat, 14 Mar 2026 21:59:33 -0700 Subject: [PATCH 3/9] fix(model-switch): apply provider changes immediately and tighten catalog fallback --- codex-rs/core/src/models_manager/manager.rs | 42 +++++ codex-rs/tui/src/app.rs | 161 ++++++++++++++++++-- 2 files changed, 194 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 3c22d0b099e5..f9cc3a6a8fd8 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -438,12 +438,20 @@ impl ModelsManager { Ok(api_auth) => api_auth, Err(CodexErr::EnvVar(_)) => { info!("models refresh skipped: github-copilot token is unavailable"); + // Avoid showing bundled fallback models when the provider + // cannot be authenticated. + self.apply_remote_models(Vec::new()).await; + *self.etag.write().await = None; return Ok(()); } Err(err) => return Err(err), }; let Some(token) = api_auth.bearer_token() else { info!("models refresh skipped: github-copilot auth token is unavailable"); + // Avoid showing bundled fallback models when the provider + // cannot be authenticated. + self.apply_remote_models(Vec::new()).await; + *self.etag.write().await = None; return Ok(()); }; @@ -1634,6 +1642,40 @@ mod tests { ); } + #[tokio::test] + async fn refresh_available_models_clears_copilot_catalog_when_token_is_unavailable() { + let codex_home = tempdir().expect("temp dir"); + let auth_manager = Arc::new(AuthManager::new( + codex_home.path().to_path_buf(), + false, + AuthCredentialsStoreMode::File, + )); + let mut provider = ModelProviderInfo::create_github_copilot_provider(); + provider.env_key = Some("__CODEX_TEST_COPILOT_ENV_KEY_MISSING__".to_string()); + let manager = ModelsManager::with_provider_for_tests( + codex_home.path().to_path_buf(), + auth_manager, + provider, + ); + + manager + .refresh_available_models(RefreshStrategy::OnlineIfUncached) + .await + .expect("refresh should complete even without token"); + + let available = manager + .try_list_models() + .expect("models should be available"); + assert!( + available.is_empty(), + "expected no github-copilot models without token, got: {:?}", + available + .iter() + .map(|preset| preset.model.as_str()) + .collect::>() + ); + } + #[tokio::test] async fn online_if_uncached_bypasses_cache_for_github_copilot_provider() { let server = MockServer::start().await; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0ef4a9782136..d703fe0f9680 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -84,6 +84,7 @@ use codex_protocol::protocol::TokenUsage; use codex_utils_absolute_path::AbsolutePathBuf; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; +use color_eyre::eyre::eyre; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -957,6 +958,122 @@ impl App { .open_model_popup_with_provider_presets(presets); } + async fn apply_provider_switch_immediately( + &mut self, + tui: &mut tui::Tui, + provider_id: &str, + model: &str, + effort: Option, + ) -> Result<()> { + let current_cwd = self.config.cwd.clone(); + let mut updated_config = self + .rebuild_config_for_resume_or_fallback(¤t_cwd, current_cwd.clone()) + .await?; + self.apply_runtime_policy_overrides(&mut updated_config); + + let provider = updated_config + .model_providers + .get(provider_id) + .cloned() + .or_else(|| self.config.model_providers.get(provider_id).cloned()) + .ok_or_else(|| eyre!("Model provider `{provider_id}` not found in configuration"))?; + updated_config.model_provider_id = provider_id.to_string(); + updated_config.model_provider = provider; + updated_config.model = Some(model.to_string()); + updated_config.model_reasoning_effort = effort; + let replacement_server = Arc::new(ThreadManager::new( + updated_config.codex_home.clone(), + self.auth_manager.clone(), + SessionSource::Cli, + updated_config.model_catalog.clone(), + CollaborationModesConfig { + default_mode_request_user_input: updated_config + .features + .enabled(Feature::DefaultModeRequestUserInput), + }, + updated_config.model_provider.clone(), + )); + replacement_server + .plugins_manager() + .maybe_start_curated_repo_sync_for_config(&updated_config); + let previous_server = Arc::clone(&self.server); + + // Keep the selection used by resume/new-session bootstraps aligned with what the user picked. + self.chat_widget.set_model(model); + self.on_update_reasoning_effort(effort); + + if let Some(rollout_path) = self.chat_widget.rollout_path() + && rollout_path.exists() + { + match replacement_server + .resume_thread_from_rollout( + updated_config.clone(), + rollout_path.clone(), + self.auth_manager.clone(), + ) + .await + { + Ok(resumed) => { + self.shutdown_current_thread().await; + if let Err(err) = previous_server.remove_and_close_all_threads().await { + tracing::warn!(error = %err, "failed to close previous threads"); + } + self.server = replacement_server; + self.config = updated_config; + tui.set_notification_method(self.config.tui_notification_method); + self.file_search.update_search_dir(self.config.cwd.clone()); + let init = + self.chatwidget_init_for_forked_or_resumed_thread(tui, self.config.clone()); + self.chat_widget = ChatWidget::new_from_existing( + init, + resumed.thread, + resumed.session_configured, + ); + self.reset_thread_event_state(); + self.refresh_status_line(); + tui.frame_requester().schedule_frame(); + return Ok(()); + } + Err(err) => { + tracing::warn!( + error = %err, + path = %rollout_path.display(), + "failed to resume active thread after provider switch; starting a fresh session" + ); + } + } + } + + self.shutdown_current_thread().await; + if let Err(err) = previous_server.remove_and_close_all_threads().await { + tracing::warn!(error = %err, "failed to close previous threads"); + } + self.server = replacement_server; + self.config = updated_config; + let model = self.chat_widget.current_model().to_string(); + let init = crate::chatwidget::ChatWidgetInit { + config: self.fresh_session_config(), + frame_requester: tui.frame_requester(), + app_event_tx: self.app_event_tx.clone(), + initial_user_message: None, + enhanced_keys_supported: self.enhanced_keys_supported, + auth_manager: self.auth_manager.clone(), + models_manager: self.server.get_models_manager(), + feedback: self.feedback.clone(), + is_first_run: false, + feedback_audience: self.feedback_audience, + model: Some(model), + startup_tooltip_override: None, + status_line_invalid_items_warned: self.status_line_invalid_items_warned.clone(), + session_telemetry: self.session_telemetry.clone(), + }; + self.chat_widget = ChatWidget::new(init, self.server.clone()); + self.reset_thread_event_state(); + tui.frame_requester().schedule_frame(); + self.refresh_status_line(); + Ok(()) + } + async fn update_feature_flags(&mut self, updates: Vec<(Feature, bool)>) { if updates.is_empty() { return; @@ -2924,17 +3041,17 @@ impl App { model, effort, } => { - let profile = self.active_profile.as_deref(); + let profile = self.active_profile.clone(); let selected_provider = provider - .as_deref() - .unwrap_or(self.config.model_provider_id.as_str()); + .clone() + .unwrap_or_else(|| self.config.model_provider_id.clone()); let provider_changed = selected_provider != self.config.model_provider_id; let mut builder = ConfigEditsBuilder::new(&self.config.codex_home) - .with_profile(profile) + .with_profile(profile.as_deref()) .set_model(Some(model.as_str()), effort); if let Some(provider_id) = provider.as_deref() { - let segments = if let Some(profile) = profile { + let segments = if let Some(profile) = profile.as_deref() { vec![ "profiles".to_string(), profile.to_string(), @@ -2951,13 +3068,39 @@ impl App { match builder.apply().await { Ok(()) => { + let provider_applied_immediately = if provider_changed { + match self + .apply_provider_switch_immediately( + tui, + selected_provider.as_str(), + model.as_str(), + effort, + ) + .await + { + Ok(()) => true, + Err(err) => { + tracing::error!( + error = %err, + provider = %selected_provider, + model, + "failed to apply provider switch immediately" + ); + false + } + } + } else { + true + }; let effort_label = effort .map(|selected_effort| selected_effort.to_string()) .unwrap_or_else(|| "default".to_string()); tracing::info!( "Selected model: {model}, provider: {selected_provider}, effort: {effort_label}" ); - let mut message = if provider_changed { + let mut message = if provider_changed && provider_applied_immediately { + format!("Model changed to {selected_provider}/{model}") + } else if provider_changed { format!("Saved model selection {selected_provider}/{model}") } else { format!("Model changed to {model}") @@ -2966,10 +3109,10 @@ impl App { message.push(' '); message.push_str(label); } - if provider_changed { + if provider_changed && !provider_applied_immediately { message.push_str(". Start a new session to apply provider change"); } - if let Some(profile) = profile { + if let Some(profile) = profile.as_deref() { message.push_str(" for "); message.push_str(profile); message.push_str(" profile"); @@ -2981,7 +3124,7 @@ impl App { error = %err, "failed to persist model selection" ); - if let Some(profile) = profile { + if let Some(profile) = profile.as_deref() { self.chat_widget.add_error_message(format!( "Failed to save model/provider for profile `{profile}`: {err}" )); From 9b39df242cd35c8be856a7b9c9cbe9fddbd81f0f Mon Sep 17 00:00:00 2001 From: engineer Date: Mon, 16 Mar 2026 14:08:41 -0700 Subject: [PATCH 4/9] fix: preserve provider-backed model switching (#12) --- codex-rs/core/src/models_manager/manager.rs | 119 ++++++++++++++++++-- codex-rs/tui/src/app.rs | 27 +++++ 2 files changed, 137 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index f9cc3a6a8fd8..08dff42f4942 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -15,6 +15,7 @@ use crate::models_manager::model_info; use codex_api::AuthProvider; use codex_api::ModelsClient; use codex_api::ReqwestTransport; +use codex_api::is_azure_responses_wire_base_url; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; @@ -52,12 +53,22 @@ struct OpenAiCompatModel { id: String, #[serde(default)] model_picker_enabled: Option, + #[serde(default)] + supported_endpoints: Vec, } impl OpenAiCompatModel { fn is_picker_enabled(&self) -> bool { !matches!(self.model_picker_enabled, Some(false)) } + + fn supports_responses_endpoint(&self) -> bool { + self.supported_endpoints.is_empty() + || self + .supported_endpoints + .iter() + .any(|endpoint| endpoint.trim_end_matches('/').ends_with("/responses")) + } } #[derive(Debug, Deserialize)] @@ -572,12 +583,13 @@ impl ModelsManager { .json() .await .map_err(|err| CodexErr::Stream(err.to_string(), None))?; - let model_ids = payload + let mut models = payload .data .into_iter() .filter(|model| model.is_picker_enabled()) - .map(|model| model.id) .collect::>(); + models.sort_by_key(|model| !model.supports_responses_endpoint()); + let model_ids = models.into_iter().map(|model| model.id).collect::>(); let models = self.map_provider_model_ids(model_ids); Ok((models, etag)) } @@ -726,14 +738,17 @@ impl ModelsManager { &self, catalog: &'a HashMap, ) -> Option<(&'a str, &'a ModelsDevProvider)> { - let normalized_name = Self::normalize_provider_key(&self.provider.name); - if let Some((provider_id, provider)) = catalog.get_key_value(&normalized_name) { - return Some((provider_id.as_str(), provider)); + for provider_alias in self.models_dev_provider_aliases() { + if let Some((provider_id, provider)) = catalog.get_key_value(&provider_alias) { + return Some((provider_id.as_str(), provider)); + } } - if let Some((provider_id, provider)) = catalog - .iter() - .find(|(_, provider)| Self::normalize_provider_key(&provider.name) == normalized_name) - { + if let Some((provider_id, provider)) = catalog.iter().find(|(_, provider)| { + let normalized_provider_name = Self::normalize_provider_key(&provider.name); + self.models_dev_provider_aliases() + .iter() + .any(|alias| alias == &normalized_provider_name) + }) { return Some((provider_id.as_str(), provider)); } @@ -761,6 +776,26 @@ impl ModelsManager { Some((first_match.0.as_str(), first_match.1)) } + fn models_dev_provider_aliases(&self) -> Vec { + let normalized_provider_name = Self::normalize_provider_key(&self.provider.name); + let mut aliases = vec![normalized_provider_name.clone()]; + let azure_named_provider = normalized_provider_name + .split('-') + .collect::>() + .windows(2) + .any(|window| window == ["azure", "openai"]); + if (azure_named_provider + || is_azure_responses_wire_base_url( + &self.provider.name, + self.provider.base_url.as_deref(), + )) + && !aliases.iter().any(|alias| alias == "azure") + { + aliases.push("azure".to_string()); + } + aliases + } + fn map_models_dev_provider(&self, provider: &ModelsDevProvider) -> Vec { let mut metadata_by_slug: HashMap = HashMap::new(); let mut model_ids = provider @@ -2363,6 +2398,72 @@ mod tests { ); } + #[tokio::test] + async fn models_dev_provider_match_accepts_azure_openai_alias() { + let models_dev_server = MockServer::start().await; + let _models_dev = wiremock::Mock::given(method("GET")) + .and(path("/api.json")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "azure": { + "id": "azure", + "name": "Azure", + "models": { + "azure-model-a": { + "id": "azure-model-a", + "name": "Azure Model A", + "release_date": "2026-01-01", + "attachment": false, + "reasoning": true, + "temperature": true, + "tool_call": true, + "limit": {"context": 128000, "output": 4096}, + "options": {} + } + } + } + }))) + .expect(1) + .mount_as_scoped(&models_dev_server) + .await; + + let codex_home = tempdir().expect("temp dir"); + let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("unused")); + let provider = ModelProviderInfo { + name: "Azure OpenAI".to_string(), + base_url: Some("http://127.0.0.1:9/openai".to_string()), + env_key: Some("AZURE_OPENAI_API_KEY".to_string()), + env_key_instructions: None, + experimental_bearer_token: None, + wire_api: WireApi::Responses, + query_params: Some( + [("api-version".to_string(), "2025-04-01-preview".to_string())] + .into_iter() + .collect(), + ), + http_headers: None, + env_http_headers: None, + request_max_retries: Some(0), + stream_max_retries: Some(0), + stream_idle_timeout_ms: Some(5_000), + requires_openai_auth: false, + supports_websockets: false, + }; + let manager = ModelsManager::with_provider_and_models_dev_url_for_tests( + codex_home.path().to_path_buf(), + auth_manager, + provider, + format!("{}/api.json", models_dev_server.uri()), + ); + + let available = manager.list_models(RefreshStrategy::OnlineIfUncached).await; + assert!( + available + .iter() + .any(|preset| preset.model == "azure-model-a"), + "expected Azure OpenAI alias to match models.dev Azure provider" + ); + } + #[tokio::test] async fn non_openai_provider_falls_back_to_provider_models_when_models_dev_has_no_match() { let models_dev_server = MockServer::start().await; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d703fe0f9680..80976cdd491e 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -796,6 +796,7 @@ impl App { async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result { let mut overrides = self.harness_overrides.clone(); overrides.cwd = Some(cwd.clone()); + overrides.config_profile = self.active_profile.clone().or(overrides.config_profile); let cwd_display = cwd.display().to_string(); ConfigBuilder::default() .codex_home(self.config.codex_home.clone()) @@ -6736,6 +6737,32 @@ mod tests { Ok(()) } + #[tokio::test] + async fn rebuild_config_for_cwd_preserves_active_profile() -> Result<()> { + let mut app = make_test_app().await; + let codex_home = tempdir()?; + app.config.codex_home = codex_home.path().to_path_buf(); + app.active_profile = Some("copilot".to_string()); + std::fs::write( + codex_home.path().join("config.toml"), + r#" +model_provider = "openai" +model = "gpt-5.4" + +[profiles.copilot] +model_provider = "github-copilot" +model = "claude-opus-4.6" +"#, + )?; + + let rebuilt = app.rebuild_config_for_cwd(app.config.cwd.clone()).await?; + + assert_eq!(rebuilt.active_profile.as_deref(), Some("copilot")); + assert_eq!(rebuilt.model_provider_id, "github-copilot"); + assert_eq!(rebuilt.model.as_deref(), Some("claude-opus-4.6")); + Ok(()) + } + #[tokio::test] async fn sync_tui_theme_selection_updates_chat_widget_config_copy() { let mut app = make_test_app().await; From f6bda8970c9cf622e194ba80d200a3f95a1211e3 Mon Sep 17 00:00:00 2001 From: engineer Date: Tue, 17 Mar 2026 05:10:12 -0700 Subject: [PATCH 5/9] codex: fix PR #32 hosted test regressions --- codex-rs/core/tests/common/test_codex.rs | 1 + .../tui/tests/suite/model_switching_e2e.rs | 100 +++++++++++++----- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 11ddddc27e04..f13fd1abe06b 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -192,6 +192,7 @@ impl TestCodexBuilder { &config, codex_core::test_support::auth_manager_from_auth(auth.clone()), SessionSource::Exec, + config.model_catalog.clone(), CollaborationModesConfig::default(), ) } else { diff --git a/codex-rs/tui/tests/suite/model_switching_e2e.rs b/codex-rs/tui/tests/suite/model_switching_e2e.rs index 6e71f009882c..53b4b27161c7 100644 --- a/codex-rs/tui/tests/suite/model_switching_e2e.rs +++ b/codex-rs/tui/tests/suite/model_switching_e2e.rs @@ -824,13 +824,15 @@ async fn cross_provider_model_switch_applies_immediately_without_manual_new_sess fn tempdir_with_ollama_config(repo_root: &Path, model: &str) -> Result { let codex_home = tempfile::tempdir()?; - let repo_root_display = repo_root.display(); + let repo_root_display = repo_root.display().to_string(); + let repo_root_toml = toml::Value::String(repo_root_display).to_string(); + let model_toml = toml::Value::String(model.to_string()).to_string(); let config_contents = format!( r#"model_provider = "ollama" -model = "{model}" +model = {model_toml} cli_auth_credentials_store = "file" -[projects."{repo_root_display}"] +[projects.{repo_root_toml}] trust_level = "trusted" "# ); @@ -869,21 +871,25 @@ fn tempdir_with_github_copilot_config( serde_json::to_string(&source_catalog)?, )?; - let repo_root_display = repo_root.display(); - let catalog_display = custom_catalog_path.display(); + let repo_root_display = repo_root.display().to_string(); + let repo_root_toml = toml::Value::String(repo_root_display).to_string(); + let catalog_display = custom_catalog_path.display().to_string(); + let catalog_toml = toml::Value::String(catalog_display).to_string(); + let model_toml = toml::Value::String(model.to_string()).to_string(); + let base_url_toml = toml::Value::String(base_url.to_string()).to_string(); let config_contents = format!( r#"model_provider = "github-copilot" -model = "{model}" -model_catalog_json = "{catalog_display}" +model = {model_toml} +model_catalog_json = {catalog_toml} cli_auth_credentials_store = "file" [model_providers.github-copilot] name = "GitHub Copilot" -base_url = "{base_url}" +base_url = {base_url_toml} env_key = "GITHUB_COPILOT_TOKEN" wire_api = "responses" -[projects."{repo_root_display}"] +[projects.{repo_root_toml}] trust_level = "trusted" "# ); @@ -902,19 +908,25 @@ fn tempdir_with_models_dev_provider_config( ) -> Result { let codex_home = tempfile::tempdir()?; - let repo_root_display = repo_root.display(); + let repo_root_display = repo_root.display().to_string(); + let repo_root_toml = toml::Value::String(repo_root_display).to_string(); + let provider_id_toml = toml::Value::String(provider_id.to_string()).to_string(); + let provider_name_toml = toml::Value::String(provider_name.to_string()).to_string(); + let model_toml = toml::Value::String(model.to_string()).to_string(); + let base_url_toml = toml::Value::String(base_url.to_string()).to_string(); + let env_key_toml = toml::Value::String(env_key.to_string()).to_string(); let config_contents = format!( - r#"model_provider = "{provider_id}" -model = "{model}" + r#"model_provider = {provider_id_toml} +model = {model_toml} cli_auth_credentials_store = "file" [model_providers.{provider_id}] -name = "{provider_name}" -base_url = "{base_url}" -env_key = "{env_key}" +name = {provider_name_toml} +base_url = {base_url_toml} +env_key = {env_key_toml} wire_api = "responses" -[projects."{repo_root_display}"] +[projects.{repo_root_toml}] trust_level = "trusted" "# ); @@ -931,25 +943,29 @@ fn tempdir_with_dual_provider_config( ) -> Result { let codex_home = tempfile::tempdir()?; - let repo_root_display = repo_root.display(); + let repo_root_display = repo_root.display().to_string(); + let repo_root_toml = toml::Value::String(repo_root_display).to_string(); + let startup_model_toml = toml::Value::String(startup_model.to_string()).to_string(); + let azure_base_url_toml = toml::Value::String(azure_base_url.to_string()).to_string(); + let copilot_base_url_toml = toml::Value::String(copilot_base_url.to_string()).to_string(); let config_contents = format!( r#"model_provider = "azure-local" -model = "{startup_model}" +model = {startup_model_toml} cli_auth_credentials_store = "file" [model_providers.azure-local] name = "Azure" -base_url = "{azure_base_url}" +base_url = {azure_base_url_toml} env_key = "AZURE_TEST_KEY" wire_api = "responses" [model_providers.github-copilot] name = "GitHub Copilot" -base_url = "{copilot_base_url}" +base_url = {copilot_base_url_toml} env_key = "GITHUB_COPILOT_TOKEN" wire_api = "responses" -[projects."{repo_root_display}"] +[projects.{repo_root_toml}] trust_level = "trusted" "# ); @@ -1349,16 +1365,27 @@ async fn run_codex_cli_with_filter_options( } fn find_codex_cli(cwd: &Path) -> Option { - // Always build a fresh local binary once per test process so PTY E2E - // assertions exercise the current source tree instead of stale artifacts. + if let Some(path) = sibling_test_binary("codex") { + return Some(path); + } + + if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { + return Some(path); + } + + let fallback = debug_target_binary(cwd, "codex"); + if fallback.is_file() { + return Some(fallback); + } + if ensure_fallback_codex_binary_is_built(cwd).is_ok() { - let fallback = cwd.join("codex-rs/target/debug/codex"); - if fallback.is_file() { - return Some(fallback); - } + return sibling_test_binary("codex").or_else(|| { + let fallback = debug_target_binary(cwd, "codex"); + fallback.is_file().then_some(fallback) + }); } - codex_utils_cargo_bin::cargo_bin("codex").ok() + None } fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { @@ -1366,6 +1393,8 @@ fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { let result = BUILD_RESULT.get_or_init(|| { let status = Command::new("cargo") .arg("build") + .arg("-p") + .arg("codex-cli") .arg("--bin") .arg("codex") .current_dir(repo_root.join("codex-rs")) @@ -1385,6 +1414,21 @@ fn ensure_fallback_codex_binary_is_built(repo_root: &Path) -> Result<()> { } } +fn sibling_test_binary(binary: &str) -> Option { + let current_exe = std::env::current_exe().ok()?; + let profile_dir = current_exe.parent()?.parent()?; + let candidate = profile_dir.join(format!("{binary}{}", std::env::consts::EXE_SUFFIX)); + candidate.is_file().then_some(candidate) +} + +fn debug_target_binary(repo_root: &Path, binary: &str) -> PathBuf { + repo_root + .join("codex-rs") + .join("target") + .join("debug") + .join(format!("{binary}{}", std::env::consts::EXE_SUFFIX)) +} + fn spawn_openai_compat_models_and_responses_server( models_response_json: serde_json::Value, response_model: &str, From 07f692e7d1c1ebb927606c0ceae3f8aaa5448502 Mon Sep 17 00:00:00 2001 From: engineer Date: Wed, 18 Mar 2026 10:01:37 -0700 Subject: [PATCH 6/9] Fix runtime model switching and normalize Copilot Claude tool wrappers Apply cross-provider /model selections to the live TUI session instead of forcing a fresh session before the change takes effect. Update the app and chat widget runtime config immediately, keep the persisted config change, and align the TUI regression tests with the intended in-session behavior. Also fix the GitHub Copilot chat/completions fallback path for Claude-family models. Instead of collapsing fallback replies into a single assistant text message, detect Claude-style / wrapper blocks and synthesize native ResponseItem::FunctionCall and ResponseItem::FunctionCallOutput items so Codex handles them like first-class tool activity. Preserve surrounding assistant prose as normal assistant messages and add focused client-side regression tests for wrapper conversion and plain-text passthrough. This replaces the UI-only workaround direction with provider-boundary normalization, which is the correct place to adapt non-Responses payloads into Codex's native item model. (cherry picked from commit ec5be6b44e3658da90e89cf955bdbcacb5b6ffcb) --- codex-rs/core/src/client.rs | 106 ++++++++++++++++-- codex-rs/core/src/client_tests.rs | 56 +++++++++ .../core/src/stream_events_utils_tests.rs | 1 + codex-rs/tui/src/app.rs | 69 ++++++++++++ codex-rs/tui/src/chatwidget.rs | 5 + codex-rs/tui/src/chatwidget/tests.rs | 17 +-- 6 files changed, 238 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 79ac7e1bb8dd..9fe8a85d4fc4 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -65,6 +65,7 @@ use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::config_types::ServiceTier; use codex_protocol::config_types::Verbosity as VerbosityConfig; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; @@ -1163,18 +1164,13 @@ impl ModelClientSession { None, ) })?; + let completion_items = synthesize_chat_completions_output_items(&assistant_text); let (tx_event, rx_event) = mpsc::channel(8); let _ = tx_event.try_send(Ok(ResponseEvent::Created)); - let _ = tx_event.try_send(Ok(ResponseEvent::OutputItemDone(ResponseItem::Message { - id: None, - role: "assistant".to_string(), - content: vec![ContentItem::OutputText { - text: assistant_text, - }], - end_turn: None, - phase: None, - }))); + for item in completion_items { + let _ = tx_event.try_send(Ok(ResponseEvent::OutputItemDone(item))); + } let _ = tx_event.try_send(Ok(ResponseEvent::Completed { response_id, token_usage: None, @@ -1588,6 +1584,98 @@ fn extract_chat_completions_text(response_json: &JsonValue) -> Option { }) } +fn synthesize_chat_completions_output_items(text: &str) -> Vec { + let parsed = parse_claude_tool_wrapper_blocks(text); + if parsed.is_empty() { + return vec![assistant_message_item(text.to_string())]; + } + + let mut items = Vec::new(); + for item in parsed { + items.push(item); + } + items +} + +fn assistant_message_item(text: String) -> ResponseItem { + ResponseItem::Message { + id: None, + role: "assistant".to_string(), + content: vec![ContentItem::OutputText { text }], + end_turn: None, + phase: None, + } +} + +fn parse_claude_tool_wrapper_blocks(text: &str) -> Vec { + let mut items = Vec::new(); + let mut remaining = text; + let mut tool_index = 0usize; + + while let Some(start) = remaining.find("") { + let before = &remaining[..start]; + if !before.is_empty() { + items.push(assistant_message_item(before.to_string())); + } + + let after_open = &remaining[start + "".len()..]; + let Some(end_call) = after_open.find("") else { + return vec![assistant_message_item(text.to_string())]; + }; + let tool_call_payload = after_open[..end_call].trim(); + let after_call = &after_open[end_call + "".len()..]; + + let Some(result_start) = after_call.find("") else { + return vec![assistant_message_item(text.to_string())]; + }; + let between = &after_call[..result_start]; + if !between.is_empty() { + items.push(assistant_message_item(between.to_string())); + } + + let after_result_open = &after_call[result_start + "".len()..]; + let Some(end_result) = after_result_open.find("") else { + return vec![assistant_message_item(text.to_string())]; + }; + let tool_result_payload = after_result_open[..end_result].trim(); + remaining = &after_result_open[end_result + "".len()..]; + + let Ok(tool_call_json) = serde_json::from_str::(tool_call_payload) else { + return vec![assistant_message_item(text.to_string())]; + }; + let Some(name) = tool_call_json + .get("name") + .and_then(JsonValue::as_str) + .map(ToString::to_string) + else { + return vec![assistant_message_item(text.to_string())]; + }; + let arguments = tool_call_json + .get("arguments") + .cloned() + .unwrap_or(JsonValue::Object(serde_json::Map::new())); + let call_id = format!("chat-completions-tool-call-{tool_index}"); + tool_index += 1; + items.push(ResponseItem::FunctionCall { + id: None, + name, + namespace: None, + arguments: arguments.to_string(), + call_id: call_id.clone(), + }); + items.push(ResponseItem::FunctionCallOutput { + call_id, + output: FunctionCallOutputPayload::from_text(tool_result_payload.to_string()), + }); + } + + if !remaining.is_empty() { + items.push(assistant_message_item(remaining.to_string())); + } + + items +} + fn map_response_stream( api_stream: S, session_telemetry: SessionTelemetry, diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 441a34864577..2f10e244e0ef 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -1,9 +1,12 @@ use super::AuthRequestTelemetryContext; use super::ModelClient; use super::PendingUnauthorizedRetry; +use super::synthesize_chat_completions_output_items; use super::UnauthorizedRecoveryExecution; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; +use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; @@ -116,3 +119,56 @@ fn auth_request_telemetry_context_tracks_attached_auth_and_retry_phase() { assert_eq!(auth_context.recovery_mode, Some("managed")); assert_eq!(auth_context.recovery_phase, Some("refresh_token")); } + +#[test] +fn synthesize_chat_completions_output_items_converts_claude_tool_wrappers() { + let items = synthesize_chat_completions_output_items( + "before{\"name\":\"shell\",\"arguments\":{\"command\":\"pwd\"}}okafter", + ); + + assert_eq!(items.len(), 4); + assert!(matches!( + &items[0], + ResponseItem::Message { role, content, .. } + if role == "assistant" + && matches!( + content.first(), + Some(codex_protocol::models::ContentItem::OutputText { text }) if text == "before" + ) + )); + assert!(matches!( + &items[1], + ResponseItem::FunctionCall { name, arguments, .. } + if name == "shell" && arguments == "{\"command\":\"pwd\"}" + )); + assert!(matches!( + &items[2], + ResponseItem::FunctionCallOutput { output, .. } + if matches!(&output.body, FunctionCallOutputBody::Text(text) if text == "ok") + )); + assert!(matches!( + &items[3], + ResponseItem::Message { role, content, .. } + if role == "assistant" + && matches!( + content.first(), + Some(codex_protocol::models::ContentItem::OutputText { text }) if text == "after" + ) + )); +} + +#[test] +fn synthesize_chat_completions_output_items_leaves_plain_text_unchanged() { + let items = synthesize_chat_completions_output_items("plain assistant text"); + + assert_eq!(items.len(), 1); + assert!(matches!( + &items[0], + ResponseItem::Message { role, content, .. } + if role == "assistant" + && matches!( + content.first(), + Some(codex_protocol::models::ContentItem::OutputText { text }) if text == "plain assistant text" + ) + )); +} diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index bfebb8902c52..2d76780bae96 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -68,6 +68,7 @@ fn last_assistant_message_from_item_returns_none_for_plan_only_hidden_message() assert_eq!(last_assistant_message_from_item(&item, true), None); } + #[tokio::test] async fn save_image_generation_result_saves_base64_to_png_in_temp_dir() { let expected_path = std::env::temp_dir().join("ig_save_base64.png"); diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 80976cdd491e..23988bda0f19 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3047,6 +3047,18 @@ impl App { .clone() .unwrap_or_else(|| self.config.model_provider_id.clone()); let provider_changed = selected_provider != self.config.model_provider_id; + if provider_changed { + if let Some(provider_info) = + self.config.model_providers.get(&selected_provider).cloned() + { + self.config.model_provider = provider_info.clone(); + self.chat_widget.config_mut().model_provider = provider_info; + } + self.config.model_provider_id = selected_provider.clone(); + self.chat_widget.config_mut().model_provider_id = selected_provider.clone(); + self.chat_widget.set_model(&model); + self.refresh_status_line(); + } let mut builder = ConfigEditsBuilder::new(&self.config.codex_home) .with_profile(profile.as_deref()) @@ -6763,6 +6775,63 @@ model = "claude-opus-4.6" Ok(()) } + #[tokio::test] + async fn persist_model_selection_switches_provider_in_runtime() -> Result<()> { + let mut app = make_test_app().await; + let codex_home = tempdir()?; + app.config.codex_home = codex_home.path().to_path_buf(); + std::fs::write( + codex_home.path().join("config.toml"), + r#" +model_provider = "openai" +model = "gpt-5.4" +"#, + )?; + + let copilot_provider = codex_core::ModelProviderInfo::create_github_copilot_provider(); + app.config + .model_providers + .insert("github-copilot".to_string(), copilot_provider.clone()); + app.chat_widget + .config_mut() + .model_providers + .insert("github-copilot".to_string(), copilot_provider); + + let mut tui = crate::tui::tests::create_test_tui()?; + let control = app + .handle_event( + &mut tui, + AppEvent::PersistModelSelection { + provider: Some("github-copilot".to_string()), + model: "claude-4.6-opus".to_string(), + effort: None, + }, + ) + .await?; + + assert!(matches!(control, AppRunControl::Continue)); + assert_eq!(app.config.model_provider_id, "github-copilot"); + assert_eq!(app.config.model.as_deref(), Some("gpt-5.4")); + assert_eq!( + app.chat_widget.config_ref().model_provider_id, + "github-copilot" + ); + assert_eq!( + app.chat_widget.config_ref().model.as_deref(), + Some("claude-4.6-opus") + ); + assert_eq!(app.chat_widget.current_model(), "claude-4.6-opus"); + + let rebuilt = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await?; + assert_eq!(rebuilt.model_provider_id, "github-copilot"); + assert_eq!(rebuilt.model.as_deref(), Some("claude-4.6-opus")); + + Ok(()) + } + #[tokio::test] async fn sync_tui_theme_selection_updates_chat_widget_config_copy() { let mut app = make_test_app().await; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index de56ab883cab..c9e7f10730bb 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -8227,6 +8227,7 @@ impl ChatWidget { /// Set the model in the widget's config copy and stored collaboration mode. pub(crate) fn set_model(&mut self, model: &str) { + self.config.model = Some(model.to_string()); self.current_collaboration_mode = self.current_collaboration_mode.with_updates( Some(model.to_string()), /*effort*/ None, @@ -9438,6 +9439,10 @@ impl ChatWidget { &self.config } + pub(crate) fn config_mut(&mut self) -> &mut Config { + &mut self.config + } + #[cfg(test)] pub(crate) fn status_line_text(&self) -> Option { self.bottom_pane.status_line_text() diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 809f99b51660..dd5268387d23 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -8327,7 +8327,7 @@ async fn model_picker_switches_to_azure_model_and_back_without_runtime_switch() } #[tokio::test] -async fn model_picker_switches_from_gpt5_3_to_copilot_claude_without_runtime_switch() { +async fn model_picker_switches_from_gpt5_3_to_copilot_claude_with_runtime_switch() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; chat.config.model_provider_id = "azure".to_string(); chat.set_model("gpt-5.3-codex"); @@ -8393,16 +8393,19 @@ async fn model_picker_switches_from_gpt5_3_to_copilot_claude_without_runtime_swi "expected persisted Copilot Claude model selection; events: {switch_events:?}" ); assert!( - !switch_events + switch_events .iter() - .any(|event| matches!(event, AppEvent::UpdateModel(_))), - "did not expect in-session model update for provider switch; events: {switch_events:?}" + .any(|event| matches!(event, AppEvent::UpdateModel(model) if model == "claude-opus-4.6")), + "expected in-session model update for provider switch; events: {switch_events:?}" ); assert!( - !switch_events + switch_events .iter() - .any(|event| matches!(event, AppEvent::UpdateReasoningEffort(_))), - "did not expect in-session reasoning update for provider switch; events: {switch_events:?}" + .any(|event| matches!( + event, + AppEvent::UpdateReasoningEffort(Some(ReasoningEffortConfig::Medium)) + )), + "expected in-session reasoning update for provider switch; events: {switch_events:?}" ); } From adc0fd6fa4ffb2d233e4fedc4d53067a387879ae Mon Sep 17 00:00:00 2001 From: engineer Date: Wed, 18 Mar 2026 22:12:46 -0700 Subject: [PATCH 7/9] Fix reasoning selection for Azure model aliases (cherry picked from commit 13637ca23802ec9e7ac7c06dfe7efa759b5c7250) --- codex-rs/core/src/models_manager/manager.rs | 19 ++- .../core/src/models_manager/manager_tests.rs | 83 +++++++++++ .../tui/tests/suite/model_switching_e2e.rs | 137 ++++++++++++++++++ 3 files changed, 231 insertions(+), 8 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 08dff42f4942..ab7df1b64bc3 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -341,10 +341,7 @@ impl ModelsManager { candidates: &[ModelInfo], config: &Config, ) -> ModelInfo { - // First use the normal longest-prefix match. If that misses, allow a narrowly scoped - // retry for namespaced slugs like `custom/gpt-5.3-codex`. - let remote = Self::find_model_by_longest_prefix(model, candidates) - .or_else(|| Self::find_model_by_namespaced_suffix(model, candidates)); + let remote = Self::find_metadata_candidate(model, candidates); let model_info = if let Some(remote) = remote { ModelInfo { slug: model.to_string(), @@ -357,6 +354,15 @@ impl ModelsManager { model_info::with_config_overrides(model_info, config) } + /// Reuse the same metadata lookup rules for both direct model selection and provider-backed + /// model listings. + fn find_metadata_candidate(model: &str, candidates: &[ModelInfo]) -> Option { + // First use the normal longest-prefix match. If that misses, allow a narrowly scoped + // retry for namespaced slugs like `custom/gpt-5.3-codex`. + Self::find_model_by_longest_prefix(model, candidates) + .or_else(|| Self::find_model_by_namespaced_suffix(model, candidates)) + } + /// Refresh models if the provided ETag differs from the cached ETag. /// /// Uses `Online` strategy to fetch latest models when ETags differ. @@ -864,10 +870,7 @@ impl ModelsManager { return None; } - let mut candidate = bundled_models - .iter() - .find(|bundled| bundled.slug == model_id) - .cloned() + let mut candidate = Self::find_metadata_candidate(&model_id, &bundled_models) .unwrap_or_else(|| model_info::model_info_from_slug(&model_id)); candidate.slug = model_id.clone(); if candidate.display_name.is_empty() { diff --git a/codex-rs/core/src/models_manager/manager_tests.rs b/codex-rs/core/src/models_manager/manager_tests.rs index 70b42637b00f..69a0142d162a 100644 --- a/codex-rs/core/src/models_manager/manager_tests.rs +++ b/codex-rs/core/src/models_manager/manager_tests.rs @@ -5,6 +5,7 @@ use crate::config::ConfigBuilder; use crate::model_provider_info::WireApi; use chrono::Utc; use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::openai_models::ReasoningEffort; use core_test_support::responses::mount_models_once; use pretty_assertions::assert_eq; use serde_json::json; @@ -827,6 +828,88 @@ async fn models_dev_provider_match_uses_base_url_host_when_name_is_custom() { ); } +#[tokio::test] +async fn models_dev_provider_alias_model_inherits_canonical_reasoning_metadata() { + let models_dev_server = MockServer::start().await; + let _models_dev = wiremock::Mock::given(method("GET")) + .and(path("/api.json")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "azure": { + "id": "azure", + "name": "Azure", + "api": "https://azure.example.com/openai", + "env": ["AZURE_OPENAI_API_KEY"], + "models": { + "azure/gpt-5.4-pro": { + "id": "azure/gpt-5.4-pro", + "name": "Azure GPT 5.4 Pro", + "release_date": "2026-01-01", + "attachment": false, + "reasoning": true, + "temperature": true, + "tool_call": true, + "limit": {"context": 128000, "output": 4096}, + "options": {} + } + } + } + }))) + .expect(1) + .mount_as_scoped(&models_dev_server) + .await; + + let codex_home = tempdir().expect("temp dir"); + let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("unused")); + let provider = ModelProviderInfo { + name: "Azure OpenAI".to_string(), + base_url: Some("https://azure.example.com/openai".to_string()), + env_key: Some("AZURE_OPENAI_API_KEY".to_string()), + env_key_instructions: None, + experimental_bearer_token: None, + wire_api: WireApi::Responses, + query_params: Some( + [("api-version".to_string(), "2025-04-01-preview".to_string())] + .into_iter() + .collect(), + ), + http_headers: None, + env_http_headers: None, + request_max_retries: Some(0), + stream_max_retries: Some(0), + stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, + requires_openai_auth: false, + supports_websockets: false, + }; + let manager = ModelsManager::with_provider_and_models_dev_url_for_tests( + codex_home.path().to_path_buf(), + auth_manager, + provider, + format!("{}/api.json", models_dev_server.uri()), + ); + + let available = manager.list_models(RefreshStrategy::OnlineIfUncached).await; + let preset = available + .iter() + .find(|preset| preset.model == "azure/gpt-5.4-pro") + .expect("expected Azure alias model to be listed"); + + assert_eq!(preset.default_reasoning_effort, ReasoningEffort::Medium); + assert_eq!( + preset + .supported_reasoning_efforts + .iter() + .map(|preset| preset.effort) + .collect::>(), + vec![ + ReasoningEffort::Low, + ReasoningEffort::Medium, + ReasoningEffort::High, + ReasoningEffort::XHigh, + ] + ); +} + #[tokio::test] async fn non_openai_provider_falls_back_to_provider_models_when_models_dev_has_no_match() { let models_dev_server = MockServer::start().await; diff --git a/codex-rs/tui/tests/suite/model_switching_e2e.rs b/codex-rs/tui/tests/suite/model_switching_e2e.rs index 53b4b27161c7..d13661d606e4 100644 --- a/codex-rs/tui/tests/suite/model_switching_e2e.rs +++ b/codex-rs/tui/tests/suite/model_switching_e2e.rs @@ -622,6 +622,93 @@ async fn models_dev_provider_model_switch_then_prompt_uses_selected_model() -> R Ok(()) } +#[tokio::test] +async fn models_dev_provider_alias_model_switch_then_prompt_uses_selected_reasoning_effort() +-> Result<()> { + if cfg!(windows) { + return Ok(()); + } + + let repo_root = codex_utils_cargo_bin::repo_root()?; + let Some(codex_cli) = find_codex_cli(&repo_root) else { + eprintln!("skipping integration test because codex binary is unavailable"); + return Ok(()); + }; + + let prompt = "What year did GPT-1 launch?"; + let answer_text = "GPT-1 launched in 2018."; + let startup_model = "azure/gpt-5.4"; + let selected_model = "azure/gpt-5.4-pro"; + let selected_reasoning = "high"; + let provider_id = "azure-local"; + + let (provider_base_url, models_dev_url, server) = spawn_models_dev_and_responses_server( + "azure", + "Azure", + &[startup_model, selected_model], + selected_model, + answer_text, + )?; + let codex_home = tempdir_with_models_dev_provider_config( + &repo_root, + provider_id, + "Azure", + startup_model, + provider_base_url.as_str(), + "AZURE_TEST_KEY", + )?; + + let mut env = HashMap::new(); + env.insert("AZURE_TEST_KEY".to_string(), "test-azure-token".to_string()); + env.insert("CODEX_MODELS_DEV_URL".to_string(), models_dev_url); + + let output = run_codex_cli_with_filter_and_reasoning( + &codex_cli, + codex_home.path(), + &repo_root, + startup_model, + selected_model, + Some(selected_reasoning), + Some(prompt), + env, + ) + .await + .context("switch models.dev Azure alias via /model, pick reasoning, and run prompt")?; + let requests = server + .join() + .expect("models.dev/responses server should join")?; + + anyhow::ensure!( + requests + .iter() + .any(|request| request.starts_with("GET /api.json ")), + "expected GET /api.json request; got requests: {requests:?}; output: {output}" + ); + assert_model_and_provider_in_config(codex_home.path(), selected_model, provider_id, &output)?; + anyhow::ensure!( + output.contains("2018"), + "expected answer to contain 2018, got output: {output}" + ); + let responses_request = requests + .iter() + .find(|request| request.starts_with("POST /v1/responses ")) + .context("missing POST /v1/responses request")?; + anyhow::ensure!( + responses_request.contains(format!("\"model\":\"{selected_model}\"").as_str()), + "expected switched model in /responses request body; request: {responses_request}" + ); + anyhow::ensure!( + responses_request.contains("\"reasoning\":{\"effort\":\"high\"}"), + "expected selected reasoning effort in /responses request body; request: {responses_request}" + ); + anyhow::ensure!( + responses_request.contains(prompt), + "expected prompt text in /responses request body; request: {responses_request}" + ); + + Ok(()) +} + #[tokio::test] async fn models_dev_provider_config_parses_custom_provider() -> Result<()> { let repo_root = codex_utils_cargo_bin::repo_root()?; @@ -1183,6 +1270,31 @@ async fn run_codex_cli_with_filter( cwd, startup_model_hint, filter, + None, + prompt_after_switch, + extra_env, + true, + ) + .await +} + +async fn run_codex_cli_with_filter_and_reasoning( + codex_cli: &Path, + codex_home: &Path, + cwd: &Path, + startup_model_hint: &str, + filter: &str, + reasoning_filter: Option<&str>, + prompt_after_switch: Option<&str>, + extra_env: HashMap, +) -> Result { + run_codex_cli_with_filter_options( + codex_cli, + codex_home, + cwd, + startup_model_hint, + filter, + reasoning_filter, prompt_after_switch, extra_env, true, @@ -1196,6 +1308,7 @@ async fn run_codex_cli_with_filter_options( cwd: &Path, startup_model_hint: &str, filter: &str, + reasoning_filter: Option<&str>, prompt_after_switch: Option<&str>, extra_env: HashMap, send_escape_before_prompt: bool, @@ -1237,6 +1350,7 @@ async fn run_codex_cli_with_filter_options( let (reasoning_prompt_tx, mut reasoning_prompt_rx) = watch::channel(false); const REASONING_CONFIRM_HINT: &str = "Press enter to confirm or esc to go back"; let filter = filter.to_string(); + let reasoning_filter = reasoning_filter.map(std::borrow::ToOwned::to_owned); let prompt_after_switch = prompt_after_switch.map(std::borrow::ToOwned::to_owned); let input_task = tokio::spawn(async move { // Wait for startup to finish before dispatching `/model`. @@ -1276,6 +1390,29 @@ async fn run_codex_cli_with_filter_options( .unwrap_or(false); if reasoning_prompt_visible { sleep(Duration::from_millis(200)).await; + if let Some(reasoning_filter) = reasoning_filter.as_ref() { + match reasoning_filter.as_str() { + "medium" => {} + "high" => { + let _ = writer_for_input.send(b"\x1b[B".to_vec()).await; + sleep(Duration::from_millis(120)).await; + } + "xhigh" => { + let _ = writer_for_input.send(b"\x1b[B".to_vec()).await; + sleep(Duration::from_millis(120)).await; + let _ = writer_for_input.send(b"\x1b[B".to_vec()).await; + sleep(Duration::from_millis(120)).await; + } + "low" => { + let _ = writer_for_input.send(b"\x1b[A".to_vec()).await; + sleep(Duration::from_millis(120)).await; + } + _ => { + type_text_with_stabilization(&writer_for_input, reasoning_filter).await; + sleep(Duration::from_millis(120)).await; + } + } + } let _ = writer_for_input.send(vec![b'\r']).await; } if let Some(prompt) = prompt_after_switch { From 2d5254b06497c22363c340c1cc73d9a6a8ea77f3 Mon Sep 17 00:00:00 2001 From: engineer Date: Thu, 19 Mar 2026 09:57:40 -0700 Subject: [PATCH 8/9] ci: avoid expired upstream npm staging artifacts (cherry picked from commit 8b7779520d39ccbd0bea17e6f0a33131be05e960) --- .github/workflows/ci.yml | 12 ++- codex-cli/scripts/install_native_deps.py | 26 ++++-- scripts/stage_npm_packages.py | 103 +++++++++++++++++------ scripts/test_stage_npm_packages.py | 40 +++++++-- 4 files changed, 140 insertions(+), 41 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0588d01a78c1..d2ebeb51e230 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,8 +36,16 @@ jobs: GH_TOKEN: ${{ github.token }} run: | set -euo pipefail - # Use a rust-release version that includes all native binaries. - CODEX_VERSION=0.74.0 + # Discover a live rust-release run instead of pinning an expired artifact set. + CODEX_VERSION=$(gh run list \ + -R openai/codex \ + --workflow .github/workflows/rust-release.yml \ + --json headBranch,status,conclusion \ + --jq 'map(select(.status == "completed" and .conclusion == "success" and (.headBranch | startswith("rust-v"))))[0].headBranch | sub("^rust-v"; "")') + if [ -z "$CODEX_VERSION" ] || [ "$CODEX_VERSION" = "null" ]; then + echo "Unable to resolve a live rust-release version" >&2 + exit 1 + fi OUTPUT_DIR="${RUNNER_TEMP}" python3 ./scripts/stage_npm_packages.py \ --release-version "$CODEX_VERSION" \ diff --git a/codex-cli/scripts/install_native_deps.py b/codex-cli/scripts/install_native_deps.py index 58fbd370fc15..d7eace6f7cd3 100755 --- a/codex-cli/scripts/install_native_deps.py +++ b/codex-cli/scripts/install_native_deps.py @@ -169,13 +169,13 @@ def main() -> int: if not workflow_url: workflow_url = DEFAULT_WORKFLOW_URL - workflow_id = workflow_url.rstrip("/").split("/")[-1] - print(f"Downloading native artifacts from workflow {workflow_id}...") + workflow_repo, workflow_id = _parse_workflow_reference(workflow_url) + print(f"Downloading native artifacts from {workflow_repo} workflow {workflow_id}...") - with _gha_group(f"Download native artifacts from workflow {workflow_id}"): + with _gha_group(f"Download native artifacts from {workflow_repo} workflow {workflow_id}"): with tempfile.TemporaryDirectory(prefix="codex-native-artifacts-") as artifacts_dir_str: artifacts_dir = Path(artifacts_dir_str) - _download_artifacts(workflow_id, artifacts_dir) + _download_artifacts(workflow_repo, workflow_id, artifacts_dir) install_binary_components( artifacts_dir, vendor_dir, @@ -259,7 +259,21 @@ def fetch_rg( return [results[target] for target in targets] -def _download_artifacts(workflow_id: str, dest_dir: Path) -> None: +def _parse_workflow_reference(workflow_url: str) -> tuple[str, str]: + parsed = urlparse(workflow_url) + parts = parsed.path.strip("/").split("/") + if parsed.netloc != "github.com" or len(parts) < 5 or parts[2] != "actions" or parts[3] != "runs": + raise ValueError(f"Unsupported workflow URL: {workflow_url}") + + repo = f"{parts[0]}/{parts[1]}" + workflow_id = parts[4] + if not workflow_id.isdigit(): + raise ValueError(f"Unsupported workflow URL: {workflow_url}") + + return repo, workflow_id + + +def _download_artifacts(workflow_repo: str, workflow_id: str, dest_dir: Path) -> None: cmd = [ "gh", "run", @@ -267,7 +281,7 @@ def _download_artifacts(workflow_id: str, dest_dir: Path) -> None: "--dir", str(dest_dir), "--repo", - "openai/codex", + workflow_repo, workflow_id, ] subprocess.check_call(cmd) diff --git a/scripts/stage_npm_packages.py b/scripts/stage_npm_packages.py index 0e074d545465..5386201277cd 100755 --- a/scripts/stage_npm_packages.py +++ b/scripts/stage_npm_packages.py @@ -7,6 +7,7 @@ import importlib.util import json import os +import re import shutil import subprocess import tempfile @@ -17,7 +18,7 @@ BUILD_SCRIPT = REPO_ROOT / "codex-cli" / "scripts" / "build_npm_package.py" INSTALL_NATIVE_DEPS = REPO_ROOT / "codex-cli" / "scripts" / "install_native_deps.py" WORKFLOW_NAME = ".github/workflows/rust-release.yml" -GITHUB_REPO = "openai/codex" +DEFAULT_GITHUB_REPO = "openai/codex" _SPEC = importlib.util.spec_from_file_location("codex_build_npm_package", BUILD_SCRIPT) if _SPEC is None or _SPEC.loader is None: @@ -78,34 +79,84 @@ def expand_packages(packages: list[str]) -> list[str]: return expanded +def normalize_github_repo(value: str | None) -> str | None: + if not value: + return None + + stripped = value.strip() + if not stripped: + return None + + if re.fullmatch(r"[^/]+/[^/]+", stripped): + return stripped.removesuffix(".git") + + ssh_match = re.match(r"git@github\.com:(?P[^\s]+?)(?:\.git)?$", stripped) + if ssh_match: + return ssh_match.group("repo") + + https_match = re.match(r"https://github\.com/(?P[^\s]+?)(?:\.git)?/?$", stripped) + if https_match: + return https_match.group("repo") + + return None + + +def get_github_repo() -> str: + env_repo = normalize_github_repo(os.environ.get("GITHUB_REPOSITORY")) + if env_repo: + return env_repo + + try: + remote_url = subprocess.check_output( + ["git", "config", "--get", "remote.origin.url"], + cwd=REPO_ROOT, + text=True, + ).strip() + except subprocess.CalledProcessError: + return DEFAULT_GITHUB_REPO + + return normalize_github_repo(remote_url) or DEFAULT_GITHUB_REPO + + def resolve_release_workflow(version: str) -> dict: release_branch = f"rust-v{version}" - stdout = subprocess.check_output( - [ - "gh", - "run", - "list", - "-R", - GITHUB_REPO, - "--branch", - release_branch, - "--json", - "workflowName,url,headSha", - "--workflow", - WORKFLOW_NAME, - "--jq", - "first(.[])", - ], - cwd=REPO_ROOT, - text=True, + candidate_repos = [get_github_repo()] + if DEFAULT_GITHUB_REPO not in candidate_repos: + candidate_repos.append(DEFAULT_GITHUB_REPO) + + for github_repo in candidate_repos: + try: + stdout = subprocess.check_output( + [ + "gh", + "run", + "list", + "-R", + github_repo, + "--branch", + release_branch, + "--json", + "workflowName,url,headSha", + "--workflow", + WORKFLOW_NAME, + "--jq", + "first(.[])", + ], + cwd=REPO_ROOT, + text=True, + ) + except subprocess.CalledProcessError: + continue + + workflow = json.loads(stdout or "null") + if workflow: + return workflow + + repo_list = ", ".join(candidate_repos) + raise RuntimeError( + "Unable to find rust-release workflow for version " + f"{version} in any of [{repo_list}] (branch {release_branch}, workflow {WORKFLOW_NAME})." ) - workflow = json.loads(stdout or "null") - if not workflow: - raise RuntimeError( - "Unable to find rust-release workflow for version " - f"{version} in {GITHUB_REPO} (branch {release_branch}, workflow {WORKFLOW_NAME})." - ) - return workflow def resolve_workflow_url(version: str, override: str | None) -> tuple[str, str | None]: diff --git a/scripts/test_stage_npm_packages.py b/scripts/test_stage_npm_packages.py index f262f13fe315..1510bbfe0d99 100644 --- a/scripts/test_stage_npm_packages.py +++ b/scripts/test_stage_npm_packages.py @@ -15,13 +15,14 @@ class ResolveReleaseWorkflowTests(unittest.TestCase): - def test_queries_upstream_repo_for_release_workflow(self) -> None: + def test_queries_detected_repo_for_release_workflow(self) -> None: + repo = "dzianisv/codex" expected = { "workflowName": "rust-release", - "url": "https://github.com/openai/codex/actions/runs/20345806534", + "url": f"https://github.com/{repo}/actions/runs/20345806534", "headSha": "5b9d9a60d74c8ee2cb34d60fb14b71990e8318ea", } - with patch.object( + with patch.object(MODULE, "get_github_repo", return_value=repo), patch.object( MODULE.subprocess, "check_output", return_value=MODULE.json.dumps(expected), @@ -31,17 +32,42 @@ def test_queries_upstream_repo_for_release_workflow(self) -> None: self.assertEqual(workflow, expected) cmd = check_output.call_args.args[0] self.assertEqual(cmd[:4], ["gh", "run", "list", "-R"]) - self.assertEqual(cmd[4], MODULE.GITHUB_REPO) + self.assertEqual(cmd[4], repo) self.assertIn("--branch", cmd) self.assertEqual(cmd[cmd.index("--branch") + 1], "rust-v0.74.0") self.assertIn("--workflow", cmd) self.assertEqual(cmd[cmd.index("--workflow") + 1], MODULE.WORKFLOW_NAME) - def test_missing_workflow_error_mentions_repo_and_branch(self) -> None: - with patch.object(MODULE.subprocess, "check_output", return_value=""): + def test_falls_back_to_upstream_when_current_repo_has_no_workflow(self) -> None: + repo = "dzianisv/codex" + expected = { + "workflowName": "rust-release", + "url": "https://github.com/openai/codex/actions/runs/20345806534", + "headSha": "5b9d9a60d74c8ee2cb34d60fb14b71990e8318ea", + } + with patch.object(MODULE, "get_github_repo", return_value=repo), patch.object( + MODULE.subprocess, + "check_output", + side_effect=["", MODULE.json.dumps(expected)], + ) as check_output: + workflow = MODULE.resolve_release_workflow("0.74.0") + + self.assertEqual(workflow, expected) + first_cmd = check_output.call_args_list[0].args[0] + second_cmd = check_output.call_args_list[1].args[0] + self.assertEqual(first_cmd[4], repo) + self.assertEqual(second_cmd[4], MODULE.DEFAULT_GITHUB_REPO) + + def test_missing_workflow_error_mentions_candidate_repos_and_branch(self) -> None: + repo = "dzianisv/codex" + with patch.object(MODULE, "get_github_repo", return_value=repo), patch.object( + MODULE.subprocess, + "check_output", + return_value="", + ): with self.assertRaisesRegex( RuntimeError, - r"openai/codex.*rust-v0\.74\.0.*\.github/workflows/rust-release\.yml", + r"dzianisv/codex, openai/codex.*rust-v0\.74\.0.*\.github/workflows/rust-release\.yml", ): MODULE.resolve_release_workflow("0.74.0") From 3e7b4b6490e2d26c843820b998896c393e1d83db Mon Sep 17 00:00:00 2001 From: engineer Date: Thu, 19 Mar 2026 19:22:27 -0700 Subject: [PATCH 9/9] Consolidate model switching fixes and CI staging --- codex-rs/core/src/client_tests.rs | 2 +- codex-rs/core/src/models_manager/manager.rs | 36 ++++- .../core/src/stream_events_utils_tests.rs | 1 - codex-rs/core/tests/common/test_codex.rs | 1 - codex-rs/tui/src/app.rs | 148 +++++++++++++++--- codex-rs/tui/src/chatwidget/tests.rs | 16 +- codex-rs/tui/src/tui.rs | 11 ++ .../tui/tests/suite/model_switching_e2e.rs | 6 +- 8 files changed, 177 insertions(+), 44 deletions(-) diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 2f10e244e0ef..38ad5ccdd165 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -1,8 +1,8 @@ use super::AuthRequestTelemetryContext; use super::ModelClient; use super::PendingUnauthorizedRetry; -use super::synthesize_chat_completions_output_items; use super::UnauthorizedRecoveryExecution; +use super::synthesize_chat_completions_output_items; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; use codex_protocol::models::FunctionCallOutputBody; diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index ab7df1b64bc3..45845dbfe0a1 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -210,6 +210,23 @@ impl ModelsManager { } } + /// Construct a manager with an explicit provider used for remote model refreshes. + pub fn new_with_provider( + codex_home: PathBuf, + auth_manager: Arc, + model_catalog: Option, + collaboration_modes_config: CollaborationModesConfig, + provider: ModelProviderInfo, + ) -> Self { + Self::new( + codex_home, + auth_manager, + model_catalog, + collaboration_modes_config, + provider, + ) + } + /// List all available models, refreshing according to the specified strategy. /// /// Returns model presets sorted by priority and filtered by auth mode and visibility. @@ -592,7 +609,7 @@ impl ModelsManager { let mut models = payload .data .into_iter() - .filter(|model| model.is_picker_enabled()) + .filter(OpenAiCompatModel::is_picker_enabled) .collect::>(); models.sort_by_key(|model| !model.supports_responses_endpoint()); let model_ids = models.into_iter().map(|model| model.id).collect::>(); @@ -655,7 +672,7 @@ impl ModelsManager { let model_ids = payload .models .into_iter() - .filter(|model| model.is_picker_enabled()) + .filter(OllamaTagsModel::is_picker_enabled) .map(|model| model.name) .collect::>(); let models = self.map_provider_model_ids(model_ids); @@ -848,7 +865,7 @@ impl ModelsManager { fn extract_host_from_url(input: &str) -> Option { reqwest::Url::parse(input) .ok() - .and_then(|url| url.host_str().map(|host| host.to_ascii_lowercase())) + .and_then(|url| url.host_str().map(str::to_ascii_lowercase)) } fn ollama_tags_url(base_url: &str) -> String { @@ -1166,6 +1183,7 @@ mod tests { request_max_retries: Some(0), stream_max_retries: Some(0), stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, } @@ -1186,7 +1204,7 @@ mod tests { auth_manager, None, CollaborationModesConfig::default(), - ModelProviderInfo::create_openai_provider(), + ModelProviderInfo::create_openai_provider(/*base_url*/ None), ); let known_slug = manager .get_remote_models() @@ -1227,7 +1245,7 @@ mod tests { models: vec![overlay], }), CollaborationModesConfig::default(), - ModelProviderInfo::create_openai_provider(), + ModelProviderInfo::create_openai_provider(/*base_url*/ None), ); let model_info = manager @@ -1261,7 +1279,7 @@ mod tests { models: vec![remote], }), CollaborationModesConfig::default(), - ModelProviderInfo::create_openai_provider(), + ModelProviderInfo::create_openai_provider(/*base_url*/ None), ); let namespaced_model = "custom/gpt-image".to_string(); @@ -1287,7 +1305,7 @@ mod tests { auth_manager, None, CollaborationModesConfig::default(), - ModelProviderInfo::create_openai_provider(), + ModelProviderInfo::create_openai_provider(/*base_url*/ None), ); let known_slug = manager .get_remote_models() @@ -2306,6 +2324,7 @@ mod tests { request_max_retries: Some(0), stream_max_retries: Some(0), stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }; @@ -2382,6 +2401,7 @@ mod tests { request_max_retries: Some(0), stream_max_retries: Some(0), stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }; @@ -2448,6 +2468,7 @@ mod tests { request_max_retries: Some(0), stream_max_retries: Some(0), stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }; @@ -2514,6 +2535,7 @@ mod tests { request_max_retries: Some(0), stream_max_retries: Some(0), stream_idle_timeout_ms: Some(5_000), + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }; diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index 2d76780bae96..bfebb8902c52 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -68,7 +68,6 @@ fn last_assistant_message_from_item_returns_none_for_plan_only_hidden_message() assert_eq!(last_assistant_message_from_item(&item, true), None); } - #[tokio::test] async fn save_image_generation_result_saves_base64_to_png_in_temp_dir() { let expected_path = std::env::temp_dir().join("ig_save_base64.png"); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index f13fd1abe06b..11ddddc27e04 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -192,7 +192,6 @@ impl TestCodexBuilder { &config, codex_core::test_support::auth_manager_from_auth(auth.clone()), SessionSource::Exec, - config.model_catalog.clone(), CollaborationModesConfig::default(), ) } else { diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 23988bda0f19..d5df3be42447 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -30,7 +30,6 @@ use crate::model_migration::run_model_migration_prompt; use crate::multi_agents::AgentPickerThreadEntry; use crate::multi_agents::agent_picker_status_dot_spans; use crate::multi_agents::format_agent_picker_item_name; -use crate::multi_agents::sort_agent_picker_threads; use crate::pager_overlay::Overlay; use crate::render::highlight::highlight_bash_to_lines; use crate::render::renderable::Renderable; @@ -49,6 +48,8 @@ use codex_core::config::ConfigBuilder; use codex_core::config::ConfigOverrides; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; +#[cfg(test)] +use codex_core::config::types::ApprovalsReviewer; use codex_core::config::types::ModelAvailabilityNuxConfig; use codex_core::config_loader::ConfigLayerStackOrdering; use codex_core::features::Feature; @@ -90,6 +91,7 @@ use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use ratatui::style::Stylize; use ratatui::text::Line; +use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::Wrap; use std::collections::BTreeMap; @@ -983,16 +985,14 @@ impl App { updated_config.model = Some(model.to_string()); updated_config.model_reasoning_effort = effort; let replacement_server = Arc::new(ThreadManager::new( - updated_config.codex_home.clone(), + &updated_config, self.auth_manager.clone(), SessionSource::Cli, - updated_config.model_catalog.clone(), CollaborationModesConfig { default_mode_request_user_input: updated_config .features .enabled(Feature::DefaultModeRequestUserInput), }, - updated_config.model_provider.clone(), )); replacement_server .plugins_manager() @@ -1011,13 +1011,21 @@ impl App { updated_config.clone(), rollout_path.clone(), self.auth_manager.clone(), + None, ) .await { Ok(resumed) => { self.shutdown_current_thread().await; - if let Err(err) = previous_server.remove_and_close_all_threads().await { - tracing::warn!(error = %err, "failed to close previous threads"); + let report = previous_server + .shutdown_all_threads_bounded(Duration::from_secs(5)) + .await; + if !report.submit_failed.is_empty() || !report.timed_out.is_empty() { + tracing::warn!( + submit_failed = report.submit_failed.len(), + timed_out = report.timed_out.len(), + "failed to close previous threads" + ); } self.server = replacement_server; self.config = updated_config; @@ -1046,8 +1054,15 @@ impl App { } self.shutdown_current_thread().await; - if let Err(err) = previous_server.remove_and_close_all_threads().await { - tracing::warn!(error = %err, "failed to close previous threads"); + let report = previous_server + .shutdown_all_threads_bounded(Duration::from_secs(5)) + .await; + if !report.submit_failed.is_empty() || !report.timed_out.is_empty() { + tracing::warn!( + submit_failed = report.submit_failed.len(), + timed_out = report.timed_out.len(), + "failed to close previous threads" + ); } self.server = replacement_server; self.config = updated_config; @@ -1173,8 +1188,10 @@ impl App { history_cell::SessionHeaderHistoryCell::new( self.chat_widget.current_model().to_string(), self.chat_widget.current_reasoning_effort(), - self.chat_widget - .should_show_fast_status(self.chat_widget.current_service_tier()), + self.chat_widget.should_show_fast_status( + self.chat_widget.current_model(), + self.chat_widget.current_service_tier(), + ), self.config.cwd.clone(), version, ) @@ -1628,7 +1645,34 @@ impl App { .iter() .map(|(thread_id, entry)| (*thread_id, entry.clone())) .collect(); - sort_agent_picker_threads(&mut agent_threads); + let primary_thread_id = self.primary_thread_id; + agent_threads.sort_by(|(left_id, left_entry), (right_id, right_entry)| { + let left_is_primary = primary_thread_id == Some(*left_id); + let right_is_primary = primary_thread_id == Some(*right_id); + let left_name = format_agent_picker_item_name( + left_entry.agent_nickname.as_deref(), + left_entry.agent_role.as_deref(), + left_is_primary, + ); + let right_name = format_agent_picker_item_name( + right_entry.agent_nickname.as_deref(), + right_entry.agent_role.as_deref(), + right_is_primary, + ); + ( + left_is_primary, + !left_entry.is_closed, + left_name, + left_id.to_string(), + ) + .cmp(&( + right_is_primary, + !right_entry.is_closed, + right_name, + right_id.to_string(), + )) + .reverse() + }); let mut initial_selected_idx = None; let items: Vec = agent_threads @@ -1791,8 +1835,16 @@ impl App { self.chat_widget.thread_name(), ); self.shutdown_current_thread().await; - if let Err(err) = self.server.remove_and_close_all_threads().await { - tracing::warn!(error = %err, "failed to close all threads"); + let report = self + .server + .shutdown_all_threads_bounded(Duration::from_secs(5)) + .await; + if !report.submit_failed.is_empty() || !report.timed_out.is_empty() { + tracing::warn!( + submit_failed = report.submit_failed.len(), + timed_out = report.timed_out.len(), + "failed to close all threads" + ); } let init = crate::chatwidget::ChatWidgetInit { config, @@ -1954,16 +2006,14 @@ impl App { let harness_overrides = normalize_harness_overrides_for_cwd(harness_overrides, &config.cwd)?; let thread_manager = Arc::new(ThreadManager::new( - config.codex_home.clone(), + &config, auth_manager.clone(), SessionSource::Cli, - config.model_catalog.clone(), CollaborationModesConfig { default_mode_request_user_input: config .features .enabled(codex_core::features::Feature::DefaultModeRequestUserInput), }, - config.model_provider.clone(), )); // TODO(xl): Move into PluginManager once this no longer depends on config feature gating. thread_manager @@ -2067,6 +2117,7 @@ impl App { config.clone(), target_session.path.clone(), auth_manager.clone(), + None, ) .await .wrap_err_with(|| { @@ -2104,6 +2155,7 @@ impl App { config.clone(), target_session.path.clone(), false, + None, ) .await .wrap_err_with(|| { @@ -2415,6 +2467,7 @@ impl App { resume_config.clone(), target_session.path.clone(), self.auth_manager.clone(), + None, ) .await { @@ -2483,7 +2536,7 @@ impl App { if path.exists() { match self .server - .fork_thread(usize::MAX, self.config.clone(), path.clone(), false) + .fork_thread(usize::MAX, self.config.clone(), path.clone(), false, None) .await { Ok(forked) => { @@ -2644,6 +2697,9 @@ impl App { url, is_installed, is_enabled, + suggest_reason: None, + suggestion_type: None, + elicitation_target: None, }); } AppEvent::OpenUrlInBrowser { url } => { @@ -3331,6 +3387,36 @@ impl App { } } } + AppEvent::UpdateApprovalsReviewer(policy) => { + self.config.approvals_reviewer = policy; + self.chat_widget.set_approvals_reviewer(policy); + let profile = self.active_profile.as_deref(); + let segments = if let Some(profile) = profile { + vec![ + "profiles".to_string(), + profile.to_string(), + "approvals_reviewer".to_string(), + ] + } else { + vec!["approvals_reviewer".to_string()] + }; + if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) + .with_profile(profile) + .with_edits([ConfigEdit::SetPath { + segments, + value: policy.to_string().into(), + }]) + .apply() + .await + { + tracing::error!( + error = %err, + "failed to persist approvals reviewer update" + ); + self.chat_widget + .add_error_message(format!("Failed to save approvals reviewer: {err}")); + } + } AppEvent::UpdateFeatureFlags { updates } => { self.update_feature_flags(updates).await; } @@ -3597,11 +3683,11 @@ impl App { lines.push(Line::from("")); } if let Some(rule_line) = - crate::bottom_pane::format_additional_permissions_rule(&permissions) + crate::bottom_pane::format_requested_permissions_rule(&permissions) { lines.push(Line::from(vec![ "Permission rule: ".into(), - rule_line.cyan(), + Span::from(rule_line).cyan(), ])); } self.overlay = Some(Overlay::new_static_with_renderables( @@ -3831,6 +3917,7 @@ impl App { model_provider_id: config_snapshot.model_provider_id, service_tier: config_snapshot.service_tier, approval_policy: config_snapshot.approval_policy, + approvals_reviewer: config_snapshot.approvals_reviewer, sandbox_policy: config_snapshot.sandbox_policy, cwd: config_snapshot.cwd, reasoning_effort: config_snapshot.reasoning_effort, @@ -4320,6 +4407,7 @@ mod tests { request_max_retries: None, stream_max_retries: None, stream_idle_timeout_ms: None, + websocket_connect_timeout_ms: None, requires_openai_auth: false, supports_websockets: false, }, @@ -4524,6 +4612,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -4696,6 +4785,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -4773,6 +4863,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -4854,6 +4945,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -4934,6 +5026,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5008,6 +5101,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5121,6 +5215,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5190,6 +5285,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5293,6 +5389,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5369,6 +5466,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, @@ -5814,6 +5912,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::OnRequest, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_workspace_write_policy(), cwd: PathBuf::from("/tmp/agent"), reasoning_effort: None, @@ -6036,6 +6135,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: Some(ReasoningEffortConfig::High), @@ -6693,6 +6793,7 @@ mod tests { model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: next_cwd.clone(), reasoning_effort: None, @@ -6811,7 +6912,7 @@ model = "gpt-5.4" assert!(matches!(control, AppRunControl::Continue)); assert_eq!(app.config.model_provider_id, "github-copilot"); - assert_eq!(app.config.model.as_deref(), Some("gpt-5.4")); + assert_eq!(app.config.model.as_deref(), Some("claude-4.6-opus")); assert_eq!( app.chat_widget.config_ref().model_provider_id, "github-copilot" @@ -6891,6 +6992,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -6950,6 +7052,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7042,6 +7145,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7107,6 +7211,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7187,6 +7292,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7314,6 +7420,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/home/user/project"), reasoning_effort: None, @@ -7383,6 +7490,7 @@ model = "gpt-5.4" model_provider_id: "test-provider".to_string(), service_tier: None, approval_policy: AskForApproval::Never, + approvals_reviewer: ApprovalsReviewer::User, sandbox_policy: SandboxPolicy::new_read_only_policy(), cwd: PathBuf::from("/tmp/project"), reasoning_effort: None, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index dd5268387d23..9e6a35140c3f 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -8393,18 +8393,16 @@ async fn model_picker_switches_from_gpt5_3_to_copilot_claude_with_runtime_switch "expected persisted Copilot Claude model selection; events: {switch_events:?}" ); assert!( - switch_events - .iter() - .any(|event| matches!(event, AppEvent::UpdateModel(model) if model == "claude-opus-4.6")), + switch_events.iter().any( + |event| matches!(event, AppEvent::UpdateModel(model) if model == "claude-opus-4.6") + ), "expected in-session model update for provider switch; events: {switch_events:?}" ); assert!( - switch_events - .iter() - .any(|event| matches!( - event, - AppEvent::UpdateReasoningEffort(Some(ReasoningEffortConfig::Medium)) - )), + switch_events.iter().any(|event| matches!( + event, + AppEvent::UpdateReasoningEffort(Some(ReasoningEffortConfig::Medium)) + )), "expected in-session reasoning update for provider switch; events: {switch_events:?}" ); } diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 9d521d1aa6f1..fce53b53fddf 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -544,3 +544,14 @@ impl Tui { Ok(None) } } + +#[cfg(test)] +pub(crate) mod tests { + use super::*; + use ratatui::backend::CrosstermBackend; + + pub(crate) fn create_test_tui() -> Result { + let terminal = custom_terminal::Terminal::with_options(CrosstermBackend::new(stdout()))?; + Ok(Tui::new(terminal)) + } +} diff --git a/codex-rs/tui/tests/suite/model_switching_e2e.rs b/codex-rs/tui/tests/suite/model_switching_e2e.rs index d13661d606e4..58f85a3c149f 100644 --- a/codex-rs/tui/tests/suite/model_switching_e2e.rs +++ b/codex-rs/tui/tests/suite/model_switching_e2e.rs @@ -2085,11 +2085,7 @@ data: {{\"type\":\"response.completed\",\"response\":{{\"id\":\"resp-1\",\"usage Ok(requests) }); - Ok(( - provider_api.clone(), - format!("http://{address}/api.json"), - handle, - )) + Ok((provider_api, format!("http://{address}/api.json"), handle)) } async fn type_text_with_stabilization(writer: &tokio::sync::mpsc::Sender>, text: &str) {