diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6f0a8a4cb6..c6c9ecc425 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -343,16 +343,13 @@ be loaded. This covers turn start/steer, review, compaction, manual queue start, updates. Realtime connections use separate routing configuration and are not checked here. Interrupt, realtime stop, and goal pause/clear remain available. -Resume and input RPCs also reject a thread whose saved provider differs from the current -configured provider, including profile and app-server launch overrides. The JSON-RPC error's -`data` contains `ModelProviderError`: `modelProviderMismatch` has `modelProvider` and -`currentModelProvider`, `modelProviderNotConfigured` identifies a missing provider, and -`modelProviderPolicyConflict` identifies a mandatory-policy conflict. Clients can still use -`thread/read` to display history and disable the composer with the error's explanation. -Changing the resume request's provider does not bypass validation of the saved provider; -switch the configured provider back to continue the chat. Input is also rejected when the -current provider configuration cannot be loaded. Older clients retain their normal error -fallback for unrecognized structured error variants. +Resume and input use the thread's saved provider even when a different provider is the default. +A cold resume resolves the saved provider from configuration. A loaded thread uses its retained +provider configuration and checks current managed policy before accepting input. The JSON-RPC +error's `data` contains `modelProviderNotConfigured` for a missing provider or +`modelProviderPolicyConflict` for a mandatory-policy conflict. Clients can use `thread/read` +to display history when input is blocked. A resume request that explicitly selects a +different provider cannot change the thread's saved provider. ## Managed network requirements diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 0165e4c5f1..3495e6c31e 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -434,27 +434,6 @@ impl ConfigManager { Ok(CloudStartupConfig { bootstrap, config }) } - /// Admits resumed threads and input only on the currently selected provider. - pub(crate) async fn check_thread_model_provider_for_input( - &self, - current: &Config, - ) -> std::io::Result<()> { - self.check_thread_model_provider(current).await?; - let configured = self - .load_latest_config(Some(current.cwd.to_path_buf())) - .await?; - if current.model_provider_id != configured.model_provider_id { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - ModelProviderError::ModelProviderMismatch { - model_provider: current.model_provider_id.clone(), - current_model_provider: configured.model_provider_id, - }, - )); - } - Ok(()) - } - /// Installs local policy before cloud access; failed policy loads block traffic while /// non-strict startup remains available for configuration repair. pub(crate) async fn load_startup_config( @@ -544,7 +523,7 @@ impl ConfigManager { &self, saved_provider: Option<&str>, request_overrides: Option>, - typesafe_overrides: ConfigOverrides, + mut typesafe_overrides: ConfigOverrides, cwd: Option, ) -> std::io::Result { let requested_provider = typesafe_overrides.model_provider.clone().or_else(|| { @@ -554,11 +533,9 @@ impl ConfigManager { .and_then(serde_json::Value::as_str) .map(str::to_owned) }); - // Resolve the selection before the saved provider is applied as a resume override. - let current_provider = self - .load_latest_config(typesafe_overrides.cwd.clone().or_else(|| cwd.clone())) - .await - .map(|config| config.model_provider_id); + if let Some(saved) = saved_provider { + typesafe_overrides.model_provider = Some(saved.to_owned()); + } self.load_with_validation( &self.current_cli_overrides(), request_overrides, @@ -567,6 +544,7 @@ impl ConfigManager { |config, _overrides, layers| { let requested_provider = requested_provider .as_deref() + .or(saved_provider) .or(config.model_provider.as_deref()) .unwrap_or("openai"); // Validate the saved route even when a client sends a provider override. @@ -597,20 +575,13 @@ impl ConfigManager { )); } } - let current_provider = current_provider?; - for provider in [ - saved_provider.unwrap_or(requested_provider), - requested_provider, - ] { - if provider != current_provider { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - ModelProviderError::ModelProviderMismatch { - model_provider: provider.to_owned(), - current_model_provider: current_provider, - }, - )); - } + if let Some(saved) = saved_provider + && saved != requested_provider + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("Cannot change this thread's model provider from `{saved}` to `{requested_provider}` on resume."), + )); } Ok(()) }, diff --git a/codex-rs/app-server/src/config_manager_provider_tests.rs b/codex-rs/app-server/src/config_manager_provider_tests.rs index 1f2129d69d..b8996c7e0e 100644 --- a/codex-rs/app-server/src/config_manager_provider_tests.rs +++ b/codex-rs/app-server/src/config_manager_provider_tests.rs @@ -261,62 +261,3 @@ async fn catalog_reload_uses_latest_model_provider() -> Result<()> { assert_eq!(refreshed.model_provider_id, "openai"); Ok(()) } - -/// Invocation overrides and selected profiles still define the current provider. -#[test_case("model_provider", "gateway"; "explicit_provider")] -#[test_case("profile", "gateway-profile"; "selected_profile")] -#[tokio::test] -async fn input_provider_check_uses_effective_selection(key: &str, value: &str) -> Result<()> { - let home = tempdir()?; - std::fs::write( - home.path().join("config.toml"), - r#" -model_provider = "openai" -[model_providers.gateway] -name = "Gateway" -"#, - )?; - let mut loader_overrides = LoaderOverrides::without_managed_config_for_tests(); - let cli_overrides = if key == "profile" { - std::fs::write( - home.path().join(format!("{value}.config.toml")), - "model_provider = 'gateway'\n[model_providers.gateway]\nname = 'Gateway'\n", - )?; - loader_overrides.user_config_path = Some(AbsolutePathBuf::from_absolute_path( - home.path().join(format!("{value}.config.toml")), - )?); - loader_overrides.user_config_profile = Some(value.parse()?); - Vec::new() - } else { - vec![(key.into(), TomlValue::String(value.into()))] - }; - let manager = ConfigManager::new_for_tests( - home.path().to_path_buf(), - cli_overrides, - loader_overrides, - CloudConfigBundleLoader::default(), - ); - let current = manager - .load_latest_config(Some(home.path().to_path_buf())) - .await?; - assert_eq!(current.model_provider_id, "gateway"); - manager - .check_thread_model_provider_for_input(¤t) - .await?; - let default_manager = - ConfigManager::without_managed_config_for_tests(home.path().to_path_buf()); - let error = default_manager - .check_thread_model_provider_for_input(¤t) - .await - .unwrap_err(); - assert_eq!( - error - .get_ref() - .and_then(|error| error.downcast_ref::()), - Some(&ModelProviderError::ModelProviderMismatch { - model_provider: "gateway".into(), - current_model_provider: "openai".into(), - }) - ); - Ok(()) -} diff --git a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs index 82aeaf7f46..bada44de51 100644 --- a/codex-rs/app-server/src/request_processors/thread_goal_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_goal_processor.rs @@ -162,7 +162,7 @@ impl ThreadGoalRequestProcessor { && let Ok(thread) = self.thread_manager.get_thread(thread_id).await { self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; } diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index d1253a06a8..a2da8e7251 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -2608,7 +2608,7 @@ impl ThreadRequestProcessor { let (_, thread) = self.load_thread(&thread_id).await?; ensure_direct_input_allowed(thread.as_ref()).await?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; self.submit_core_op(request_id, thread.as_ref(), Op::Compact) @@ -4790,7 +4790,7 @@ impl ThreadRequestProcessor { ); } self.config_manager - .check_thread_model_provider_for_input(existing_thread.config().await.as_ref()) + .check_thread_model_provider(existing_thread.config().await.as_ref()) .await .map_err(|err| config_load_error(&err))?; let redact_resume_payloads = diff --git a/codex-rs/app-server/src/request_processors/thread_queue_processor.rs b/codex-rs/app-server/src/request_processors/thread_queue_processor.rs index be77be34d6..a2f5b5ad68 100644 --- a/codex-rs/app-server/src/request_processors/thread_queue_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_queue_processor.rs @@ -193,7 +193,7 @@ impl ThreadQueueRequestProcessor { let thread = loaded_thread .ok_or_else(|| invalid_request("resume the thread before starting a queued message"))?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; let submission = self diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 2667f98dd6..45a1f5e12b 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -194,7 +194,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; match self @@ -389,7 +389,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; let (review_request, display_text) = Self::review_request_from_target(params.target)?; @@ -559,7 +559,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(&request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; if let Some(tool_output) = ¶ms.tool_output { @@ -1095,7 +1095,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider_for_input(thread.config().await.as_ref()) + .check_thread_model_provider(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index bd5f991044..aa4eb5b3bb 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -1010,14 +1010,12 @@ async fn resume_revalidates_persisted_thread_after_config_load( (resumed.thread.git_info, resumed.thread.project_id), (expected_git_info.clone(), expected_project_id.clone()) ); - // Each resume attempt loads the current provider selection and then the - // saved thread config. Changed metadata must trigger another full attempt. if matches!(mutation, ResumeMutation::Unchanged) { - assert_eq!(resume_config_loads, 2); + assert_eq!(resume_config_loads, 1); } else { assert!( - resume_config_loads >= 4, - "changed persisted metadata must reload both provider and thread config" + resume_config_loads >= 2, + "changed persisted metadata must reload config" ); } turn_result?; diff --git a/codex-rs/app-server/tests/suite/v2/model_provider_enforcement_tests.rs b/codex-rs/app-server/tests/suite/v2/model_provider_enforcement_tests.rs index 03a1b192d6..5b402d1e39 100644 --- a/codex-rs/app-server/tests/suite/v2/model_provider_enforcement_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/model_provider_enforcement_tests.rs @@ -4,12 +4,14 @@ use anyhow::Result; use app_test_support::MockResponsesConfig; use app_test_support::TestAppServer; use codex_app_server::in_process; +use codex_app_server::in_process::InProcessServerEvent; use codex_app_server::in_process::InProcessStartArgs; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ThreadQueueAddParams; use codex_app_server_protocol::ThreadQueueAddResponse; use codex_app_server_protocol::ThreadQueueListParams; @@ -42,7 +44,6 @@ use wiremock::MockServer; #[test_case("other"; "selection_changes")] #[test_case("gateway"; "definition_changes")] -#[test_case("default"; "default_selection_changes")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn provider_requirement_changes_reject_inputs_to_existing_threads( required_selection: &str, @@ -79,17 +80,7 @@ base_url = "{}/v1" other.uri(), ); let requirements_path = home.path().join("requirements.toml"); - if required_selection == "default" { - std::fs::write( - &requirements_path, - requirements.replace("model_provider = \"gateway\"", ""), - )?; - let path = home.path().join("config.toml"); - let config = std::fs::read_to_string(&path)?; - std::fs::write(path, config.replace("mock_provider", "gateway"))?; - } else { - std::fs::write(&requirements_path, &requirements)?; - } + std::fs::write(&requirements_path, &requirements)?; let mut server = TestAppServer::builder() .with_codex_home(home.path()) .with_json_logging("warn") @@ -136,24 +127,12 @@ base_url = "{}/v1" } else { requirements.replace(gateway.uri(), &other.uri()) }; - let provider_error = if required_selection == "default" { - let path = home.path().join("config.toml"); - let config = std::fs::read_to_string(&path)?; - std::fs::write( - path, - config.replace("model_provider = \"gateway\"", "model_provider = \"other\""), - )?; - codex_app_server_protocol::ModelProviderError::ModelProviderMismatch { - model_provider: "gateway".into(), - current_model_provider: "other".into(), - } - } else { - std::fs::write(requirements_path, updated)?; + std::fs::write(requirements_path, updated)?; + let provider_error = codex_app_server_protocol::ModelProviderError::ModelProviderPolicyConflict { model_provider: "gateway".into(), required_model_provider: required_selection.into(), - } - }; + }; let expected_error = JSONRPCErrorError { code: -32600, message: provider_error.to_string(), @@ -287,71 +266,7 @@ base_url = "{}/v1" } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn provider_selection_can_be_restored_for_existing_threads() -> Result<()> { - let provider = MockServer::start().await; - let home = TempDir::new()?; - MockResponsesConfig::new(&provider.uri()).write(home.path())?; - let path = home.path().join("config.toml"); - let original = std::fs::read_to_string(&path)?; - let mut server = TestAppServer::builder() - .with_codex_home(home.path()) - .build_initialized() - .await?; - let started = server.start_thread(ThreadStartParams::default()).await?; - let requests = responses::mount_sse_once( - &provider, - responses::sse(vec![responses::ev_completed("restored")]), - ) - .await; - std::fs::write( - &path, - original.replace( - "model_provider = \"mock_provider\"", - "model_provider = \"openai\"", - ), - )?; - let params = TurnStartParams { - thread_id: started.thread.id.clone(), - input: vec![UserInput::Text { - text: "Continue on the original provider.".into(), - text_elements: Vec::new(), - }], - ..Default::default() - }; - let id = server.send_turn_start_request(params.clone()).await?; - let error = server - .read_stream_until_error_message(RequestId::Integer(id)) - .await?; - assert_eq!( - error.error.data, - Some(serde_json::to_value( - codex_app_server_protocol::ModelProviderError::ModelProviderMismatch { - model_provider: "mock_provider".into(), - current_model_provider: "openai".into(), - } - )?) - ); - assert!(requests.requests().is_empty()); - let id = server - .send_raw_request( - "thread/read", - Some(json!({"threadId": started.thread.id, "includeTurns": false})), - ) - .await?; - let read: codex_app_server_protocol::ThreadReadResponse = server.read_response(id).await?; - assert_eq!(read.thread.model_provider, "mock_provider"); - std::fs::write(path, original)?; - let id = server.send_turn_start_request(params).await?; - let _: TurnStartResponse = server.read_response(id).await?; - server - .read_stream_until_notification_message("turn/completed") - .await?; - assert_eq!(requests.requests().len(), 1); - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn malformed_system_defaults_reject_input_until_provider_can_be_resolved() -> Result<()> { +async fn malformed_system_defaults_do_not_block_existing_thread_turn() -> Result<()> { let provider = MockServer::start().await; let home = TempDir::new()?; MockResponsesConfig::new(&provider.uri()).write(home.path())?; @@ -366,7 +281,7 @@ async fn malformed_system_defaults_reject_input_until_provider_can_be_resolved() .build() .await?, ); - let client = in_process::start(InProcessStartArgs { + let mut client = in_process::start(InProcessStartArgs { arg0_paths: Arg0DispatchPaths::default(), config, cli_overrides: Vec::new(), @@ -421,9 +336,23 @@ async fn malformed_system_defaults_reject_input_until_provider_can_be_resolved() }, }) .await? - .expect_err("cannot verify the selected provider from malformed configuration"); - assert_eq!(response.code, -32600); - assert!(requests.requests().is_empty()); + .expect("turn/start should succeed"); + let _: TurnStartResponse = serde_json::from_value(response)?; + timeout(Duration::from_secs(/*secs*/ 60), async { + loop { + let Some(event) = client.next_event().await else { + anyhow::bail!("app-server stopped before turn/completed"); + }; + if let InProcessServerEvent::ServerNotification(notification) = event + && let ServerNotification::TurnCompleted(completed) = notification.as_ref() + && completed.thread_id == started.thread.id + { + return Ok::<(), anyhow::Error>(()); + } + } + }) + .await??; + assert_eq!(requests.requests().len(), 1); client.shutdown().await?; Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 24d4b57e19..6bfef75126 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -6842,6 +6842,17 @@ async fn setup_rollout_fixture(codex_home: &Path, server_uri: &str) -> Result Result { + create_fake_rollout( + home, + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved history", + Some("old_provider"), + /*git_info*/ None, + ) +} + #[test_case::test_case(false, false; "missing_definition_config_override")] #[test_case::test_case(false, true; "missing_definition_typed_override")] #[test_case::test_case(true, false; "policy_conflict_config_override")] @@ -6854,14 +6865,7 @@ async fn thread_resume_returns_provider_error_even_with_provider_override( let server = create_mock_responses_server_repeating_assistant("Done").await; let home = TempDir::new()?; mock_responses_config(&server.uri()).write(home.path())?; - let thread_id = create_fake_rollout( - home.path(), - "2025-01-05T12-00-00", - "2025-01-05T12:00:00Z", - "Saved history", - Some("old_provider"), - /*git_info*/ None, - )?; + let thread_id = create_saved_gateway_rollout(home.path())?; if policy_conflict { let config_path = home.path().join("config.toml"); let config = std::fs::read_to_string(&config_path)?; @@ -6963,34 +6967,12 @@ async fn thread_resume_returns_provider_error_even_with_provider_override( Ok(()) } -#[test_case::test_case(false; "missing_definition")] -#[test_case::test_case(true; "provider_mismatch")] #[tokio::test] -async fn thread_resume_recovers_after_provider_configuration_is_restored( - configured: bool, -) -> Result<()> { +async fn thread_resume_recovers_after_provider_configuration_is_restored() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let home = TempDir::new()?; mock_responses_config(&server.uri()).write(home.path())?; - let thread_id = create_fake_rollout( - home.path(), - "2025-01-05T12-00-00", - "2025-01-05T12:00:00Z", - "Saved history", - Some("old_provider"), - /*git_info*/ None, - )?; - let config_path = home.path().join("config.toml"); - if configured { - let config = std::fs::read_to_string(&config_path)?; - let server_uri = server.uri(); - std::fs::write( - &config_path, - format!( - "{config}\n[model_providers.old_provider]\nname = 'Original provider'\nbase_url = '{server_uri}/v1'\n" - ), - )?; - } + let thread_id = create_saved_gateway_rollout(home.path())?; let mut client = TestAppServer::builder() .with_codex_home(home.path()) .build_initialized() @@ -7008,54 +6990,27 @@ async fn thread_resume_recovers_after_provider_configuration_is_restored( .error .data .as_ref() - .expect("provider error should include structured data")["type"], - if configured { - "modelProviderMismatch" - } else { - "modelProviderNotConfigured" - } + .and_then(|data| data["type"].as_str()), + Some("modelProviderNotConfigured") ); - if configured { - for overrides in [ - ThreadResumeParams { - model_provider: Some("mock_provider".into()), - ..params.clone() - }, - ThreadResumeParams { - config: Some(std::collections::HashMap::from([( - "model_provider".into(), - json!("mock_provider"), - )])), - ..params.clone() - }, - ] { - let id = client.send_thread_resume_request(overrides).await?; - let overridden = client - .read_stream_until_error_message(RequestId::Integer(id)) - .await?; - assert_eq!(overridden.error, error.error); - } - } - let id = client - .send_raw_request( - "thread/read", - Some(json!({"threadId": params.thread_id, "includeTurns": true})), - ) - .await?; - let read: codex_app_server_protocol::ThreadReadResponse = client.read_response(id).await?; - assert_eq!(read.thread.model_provider, "old_provider"); + + let config_path = home.path().join("config.toml"); let config = std::fs::read_to_string(&config_path)?; - let restored = if configured { - config.replace( - "model_provider = \"mock_provider\"", - "model_provider = \"old_provider\"", - ) - } else { - config.replace("mock_provider", "old_provider") - }; - std::fs::write(config_path, restored)?; - let id = client.send_thread_resume_request(params).await?; + let server_uri = server.uri(); + std::fs::write( + &config_path, + format!( + "{config}\n[model_providers.old_provider]\nname = 'Original provider'\nbase_url = '{server_uri}/v1'\n" + ), + )?; + let id = client + .send_thread_resume_request(ThreadResumeParams { + model: Some("gpt-5.2-codex".into()), + ..params + }) + .await?; let resumed: ThreadResumeResponse = client.read_response(id).await?; assert_eq!(resumed.model_provider, "old_provider"); + assert_eq!(resumed.model, "gpt-5.2-codex"); Ok(()) } diff --git a/codex-rs/tui/src/app/tests/startup.rs b/codex-rs/tui/src/app/tests/startup.rs index 90a89fba54..876aac0017 100644 --- a/codex-rs/tui/src/app/tests/startup.rs +++ b/codex-rs/tui/src/app/tests/startup.rs @@ -2057,117 +2057,106 @@ async fn provider_resume_retries_via_picker_preserve_fresh_errors_and_drafts() - } async fn check_provider_resume_retries(reopen_via: ProviderResumeRoute) -> Result<()> { - for provider_configured in [false, true] { - let mut app = Box::pin(make_test_app()).await; - let home = app.config.codex_home.clone(); - let definition = "[model_providers.missing_provider]\nname = 'Saved provider'\nbase_url = 'http://127.0.0.1:12345/v1'\nwire_api = 'responses'\n"; - let config = if provider_configured { - format!("[tui]\nresume_cwd = 'current'\n{definition}") - } else { - "[tui]\nresume_cwd = 'current'\n".to_string() - }; - std::fs::write(home.join("config.toml"), config)?; - crate::legacy_core::config::set_project_trust_level( + let mut app = Box::pin(make_test_app()).await; + let home = app.config.codex_home.clone(); + let definition = "[model_providers.missing_provider]\nname = 'Saved provider'\nbase_url = 'http://127.0.0.1:12345/v1'\nwire_api = 'responses'\n"; + let config = "[tui]\nresume_cwd = 'current'\n"; + std::fs::write(home.join("config.toml"), config)?; + crate::legacy_core::config::set_project_trust_level( + home.as_path(), + app.config.cwd.as_path(), + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(std::io::Error::other)?; + let thread_id = ThreadId::from_string( + &app_test_support::create_fake_rollout( home.as_path(), - app.config.cwd.as_path(), - codex_protocol::config_types::TrustLevel::Trusted, + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved provider history", + Some("missing_provider"), + /*git_info*/ None, ) - .map_err(std::io::Error::other)?; - let thread_id = ThreadId::from_string( - &app_test_support::create_fake_rollout( - home.as_path(), - "2025-01-05T12-00-00", - "2025-01-05T12:00:00Z", - "Saved provider history", - Some("missing_provider"), - /*git_info*/ None, - ) - .map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?, - )?; - let mut server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; - let mut tui = crate::tui::test_support::make_test_tui()?; - let target = crate::resume_picker::SessionTarget { - backend_id: server.backend_id(), - path: None, - thread_id, - cwd: None, - history_mode: None, - }; - Box::pin(app.resume_target_session(&mut tui, &mut server, target.clone())).await?; - assert!(app.chat_widget.has_provider_error()); - assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); - let turns = app.thread_event_channels[&thread_id] - .store - .lock() - .await - .snapshot() - .turns; - assert!(serde_json::to_string(&turns)?.contains("Saved provider history")); - assert!(!app.ignore_same_thread_resume(&target)); - app.chat_widget - .apply_external_edit("Preserved draft".into()); - for current_provider in ["other_provider", "missing_provider"] { - if !matches!(reopen_via, ProviderResumeRoute::SameThread) { - let other_id = ThreadId::from_string( - &app_test_support::create_fake_rollout( - home.as_path(), - "2025-01-05T13-00-00", - "2025-01-05T13:00:00Z", - "Other chat", - Some("openai"), - /*git_info*/ None, - ) - .expect("other chat"), - )?; - Box::pin(app.select_agents_overview_thread(&mut tui, &mut server, other_id)) - .await?; - assert_eq!(app.chat_widget.thread_id(), Some(other_id)); - assert!( - app.agents_overview.input_states[&thread_id] - .provider_error - .is_some() - ); - } - std::fs::write( - home.join("config.toml"), - format!( - "model_provider = '{current_provider}'\n[tui]\nresume_cwd = 'current'\n{definition}\n{}", - definition.replace("missing_provider", "other_provider") - ), + .map_err(|err| color_eyre::eyre::eyre!(err.to_string()))?, + )?; + let mut server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + let target = crate::resume_picker::SessionTarget { + backend_id: server.backend_id(), + path: None, + thread_id, + cwd: None, + history_mode: None, + }; + Box::pin(app.resume_target_session(&mut tui, &mut server, target.clone())).await?; + assert!(app.chat_widget.has_provider_error()); + assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); + let turns = app.thread_event_channels[&thread_id] + .store + .lock() + .await + .snapshot() + .turns; + assert!(serde_json::to_string(&turns)?.contains("Saved provider history")); + assert!(!app.ignore_same_thread_resume(&target)); + app.chat_widget + .apply_external_edit("Preserved draft".into()); + for provider_configured in [false, true] { + if !matches!(reopen_via, ProviderResumeRoute::SameThread) { + let other_id = ThreadId::from_string( + &app_test_support::create_fake_rollout( + home.as_path(), + "2025-01-05T13-00-00", + "2025-01-05T13:00:00Z", + "Other chat", + Some("openai"), + /*git_info*/ None, + ) + .expect("other chat"), )?; - match reopen_via { - ProviderResumeRoute::Overview => { - Box::pin(app.select_agents_overview_thread(&mut tui, &mut server, thread_id)) - .await?; - } - ProviderResumeRoute::Picker => { - Box::pin(app.apply_resume_picker_selection( - &mut tui, - &mut server, - SessionSelection::Resume(target.clone()), - )) - .await?; - } - ProviderResumeRoute::SameThread => { - Box::pin(app.resume_target_session(&mut tui, &mut server, target.clone())) - .await?; - } - } - let input = app.chat_widget.capture_thread_input_state().unwrap(); - let expected_error = (current_provider != "missing_provider").then(|| { - codex_app_server_protocol::ModelProviderError::ModelProviderMismatch { - model_provider: "missing_provider".into(), - current_model_provider: current_provider.into(), - } - }); - assert_eq!(input.provider_error, expected_error); - assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); - assert_eq!( - app.chat_widget.composer_text_with_pending(), - "Preserved draft" + Box::pin(app.select_agents_overview_thread(&mut tui, &mut server, other_id)).await?; + assert_eq!(app.chat_widget.thread_id(), Some(other_id)); + assert!( + app.agents_overview.input_states[&thread_id] + .provider_error + .is_some() ); } - server.shutdown().await?; + let saved_definition = if provider_configured { definition } else { "" }; + std::fs::write( + home.join("config.toml"), + format!("model_provider = 'ollama'\n{config}{saved_definition}"), + )?; + match reopen_via { + ProviderResumeRoute::Overview => { + Box::pin(app.select_agents_overview_thread(&mut tui, &mut server, thread_id)) + .await?; + } + ProviderResumeRoute::Picker => { + Box::pin(app.apply_resume_picker_selection( + &mut tui, + &mut server, + SessionSelection::Resume(target.clone()), + )) + .await?; + } + ProviderResumeRoute::SameThread => { + Box::pin(app.resume_target_session(&mut tui, &mut server, target.clone())).await?; + } + } + let input = app.chat_widget.capture_thread_input_state().unwrap(); + let expected_error = (!provider_configured).then(|| { + codex_app_server_protocol::ModelProviderError::ModelProviderNotConfigured { + model_provider: "missing_provider".into(), + } + }); + assert_eq!(input.provider_error, expected_error); + assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "Preserved draft" + ); } + server.shutdown().await?; Ok(()) } diff --git a/codex-rs/tui/src/app_server_session/provider_selection_tests.rs b/codex-rs/tui/src/app_server_session/provider_selection_tests.rs index 1322067ff1..330267905e 100644 --- a/codex-rs/tui/src/app_server_session/provider_selection_tests.rs +++ b/codex-rs/tui/src/app_server_session/provider_selection_tests.rs @@ -183,7 +183,7 @@ requires_openai_auth = false } #[tokio::test] -async fn resume_discovers_other_providers_but_rejects_mismatched_resume() -> Result<()> { +async fn resume_discovers_and_reopens_the_saved_gateway() -> Result<()> { let home = tempfile::tempdir()?; write_provider_config(home.path())?; let original = ConfigBuilder::default() @@ -250,18 +250,15 @@ async fn resume_discovers_other_providers_but_rejects_mismatched_resume() -> Res .await? .is_none() ); - let error = server + let resumed = server .resume_thread( &local_settings, config.clone(), parent, ResumeModelSettings::RestoreFromThread, ) - .await - .expect_err("a different configured provider must block resume"); - assert!(error.to_string().contains( - "This chat uses server-provider, but your current provider is client-provider. Switch to server-provider to continue." - )); + .await?; + assert_eq!(resumed.session.model_provider_id, "server-provider"); assert_eq!( server .start_thread(&config) diff --git a/codex-rs/tui/tests/suite/provider_defaults.rs b/codex-rs/tui/tests/suite/provider_defaults.rs index 1786316c3c..16e8f344f3 100644 --- a/codex-rs/tui/tests/suite/provider_defaults.rs +++ b/codex-rs/tui/tests/suite/provider_defaults.rs @@ -94,7 +94,13 @@ async fn resume_lists_all_providers_and_fork_keeps_provider_filter() -> Result<( } let case = format!("{action}: embedded={embedded}, last={last}, explicit={explicit}"); let mut terminal = PtyCodex::start_cli(&cwd, home, &args)?; - if action == "resume" { + if action == "resume" && last && explicit { + // --last finds the newest thread across providers. An explicit override + // cannot switch that thread from server-provider to openai. + terminal + .wait_for_screen("Cannot change this thread's model provider") + .with_context(|| format!("{case}; screen:\n{}", terminal.screen_contents()))?; + } else if action == "resume" { terminal .wait_for_screen("Server provider history") .with_context(|| format!("{case}; screen:\n{}", terminal.screen_contents()))?;