diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index ab3d5fffc5..afeb27696a 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -16111,6 +16111,81 @@ "title": "ModelProviderCapabilitiesReadResponse", "type": "object" }, + "ModelProviderError": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback.", + "oneOf": [ + { + "properties": { + "currentModelProvider": { + "type": "string" + }, + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderMismatch" + ], + "title": "ModelProviderMismatchv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "currentModelProvider", + "modelProvider", + "type" + ], + "title": "ModelProviderMismatchv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderNotConfigured" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "type" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "requiredModelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderPolicyConflict" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "requiredModelProvider", + "type" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderError", + "type": "object" + } + ], + "title": "ModelProviderError" + }, "ModelRerouteReason": { "enum": [ "highRiskCyberActivity" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 28a7518cc4..b6a76228a4 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -12103,6 +12103,81 @@ "title": "ModelProviderCapabilitiesReadResponse", "type": "object" }, + "ModelProviderError": { + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback.", + "oneOf": [ + { + "properties": { + "currentModelProvider": { + "type": "string" + }, + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderMismatch" + ], + "title": "ModelProviderMismatchv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "currentModelProvider", + "modelProvider", + "type" + ], + "title": "ModelProviderMismatchv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderNotConfigured" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "type" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "requiredModelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderPolicyConflict" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "requiredModelProvider", + "type" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderError", + "type": "object" + } + ], + "title": "ModelProviderError" + }, "ModelRerouteReason": { "enum": [ "highRiskCyberActivity" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ModelProviderError.json b/codex-rs/app-server-protocol/schema/json/v2/ModelProviderError.json new file mode 100644 index 0000000000..e479bd5509 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/ModelProviderError.json @@ -0,0 +1,75 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback.", + "oneOf": [ + { + "properties": { + "currentModelProvider": { + "type": "string" + }, + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderMismatch" + ], + "title": "ModelProviderMismatchv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "currentModelProvider", + "modelProvider", + "type" + ], + "title": "ModelProviderMismatchv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderNotConfigured" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "type" + ], + "title": "ModelProviderNotConfiguredv2::ModelProviderError", + "type": "object" + }, + { + "properties": { + "modelProvider": { + "type": "string" + }, + "requiredModelProvider": { + "type": "string" + }, + "type": { + "enum": [ + "modelProviderPolicyConflict" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderErrorType", + "type": "string" + } + }, + "required": [ + "modelProvider", + "requiredModelProvider", + "type" + ], + "title": "ModelProviderPolicyConflictv2::ModelProviderError", + "type": "object" + } + ], + "title": "ModelProviderError" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index cc3b51ecbd..4c7ea92a77 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index 3668daa519..24ce1c7ad9 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ModelProviderError.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ModelProviderError.ts new file mode 100644 index 0000000000..1c3931ae3f --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ModelProviderError.ts @@ -0,0 +1,8 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +/** + * Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback. + */ +export type ModelProviderError = { "type": "modelProviderMismatch", modelProvider: string, currentModelProvider: string, } | { "type": "modelProviderNotConfigured", modelProvider: string, } | { "type": "modelProviderPolicyConflict", modelProvider: string, requiredModelProvider: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 3a130bde44..4ec94e59a4 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -329,6 +329,7 @@ export type { ModelListParams } from "./ModelListParams"; export type { ModelListResponse } from "./ModelListResponse"; export type { ModelProviderCapabilitiesReadParams } from "./ModelProviderCapabilitiesReadParams"; export type { ModelProviderCapabilitiesReadResponse } from "./ModelProviderCapabilitiesReadResponse"; +export type { ModelProviderError } from "./ModelProviderError"; export type { ModelRerouteReason } from "./ModelRerouteReason"; export type { ModelReroutedNotification } from "./ModelReroutedNotification"; export type { ModelSafetyBufferingUpdatedNotification } from "./ModelSafetyBufferingUpdatedNotification"; diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index 07447b4190..e43b9656fb 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -133,6 +133,7 @@ pub fn generate_ts_with_options( export_client_responses(out_dir)?; ClientNotification::export_all_to(out_dir)?; crate::UserVerificationRpcError::export_all_to(out_dir)?; + crate::ModelProviderError::export_all_to(out_dir)?; ServerRequest::export_all_to(out_dir)?; export_server_responses(out_dir)?; @@ -234,6 +235,10 @@ pub fn generate_json_with_experimental(out_dir: &Path, experimental_api: bool) - out_dir, "v2::UserVerificationRpcError", )?); + schemas.push(write_json_schema::( + out_dir, + "v2::ModelProviderError", + )?); schemas.extend(export_client_param_schemas(out_dir)?); schemas.extend(export_client_response_schemas(out_dir)?); schemas.extend(export_server_param_schemas(out_dir)?); diff --git a/codex-rs/app-server-protocol/src/protocol/v2/mod.rs b/codex-rs/app-server-protocol/src/protocol/v2/mod.rs index 37b6f5ad20..0ecd549909 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/mod.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/mod.rs @@ -22,6 +22,7 @@ mod item; mod mcp; mod memory; mod model; +mod model_provider_error; mod notification; mod permissions; mod plugin; @@ -64,6 +65,7 @@ pub use item::*; pub use mcp::*; pub use memory::*; pub use model::*; +pub use model_provider_error::*; pub use notification::*; pub use permissions::*; pub use plugin::*; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error.rs b/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error.rs new file mode 100644 index 0000000000..ee8cc7b746 --- /dev/null +++ b/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error.rs @@ -0,0 +1,70 @@ +//! Recoverable provider failures shared by resume and inference admission. + +use crate::JsonSchema; +use crate::TS; +use serde::Deserialize; +use serde::Serialize; + +/// Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[serde( + tag = "type", + rename_all = "camelCase", + rename_all_fields = "camelCase" +)] +#[ts( + tag = "type", + rename_all = "camelCase", + rename_all_fields = "camelCase", + export_to = "v2/" +)] +pub enum ModelProviderError { + ModelProviderMismatch { + #[schemars(rename = "modelProvider")] + model_provider: String, + #[schemars(rename = "currentModelProvider")] + current_model_provider: String, + }, + ModelProviderNotConfigured { + #[schemars(rename = "modelProvider")] + model_provider: String, + }, + ModelProviderPolicyConflict { + #[schemars(rename = "modelProvider")] + model_provider: String, + #[schemars(rename = "requiredModelProvider")] + required_model_provider: String, + }, +} + +impl ModelProviderError { + pub fn title(&self) -> &'static str { + match self { + Self::ModelProviderMismatch { .. } => "Model provider mismatch", + Self::ModelProviderNotConfigured { .. } => "Model provider unavailable", + Self::ModelProviderPolicyConflict { .. } => "Model provider blocked by policy", + } + } +} + +impl std::fmt::Display for ModelProviderError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::ModelProviderMismatch { model_provider, current_model_provider } => write!(f, + "This chat uses {model_provider}, but your current provider is {current_model_provider}. Switch to {model_provider} to continue."), + Self::ModelProviderNotConfigured { model_provider } => write!(f, + "This chat uses model provider “{model_provider}”, which is no longer configured. Restore this provider’s configuration to continue."), + Self::ModelProviderPolicyConflict { model_provider, required_model_provider } + if model_provider != required_model_provider => write!(f, + "This chat uses model provider “{model_provider}”, but your organization requires “{required_model_provider}”. Contact your administrator or start a new chat using the required provider."), + Self::ModelProviderPolicyConflict { .. } => f.write_str( + "This chat’s model provider settings no longer match your organization’s requirements. Contact your administrator or start a new chat using the current provider settings."), + } + } +} + +impl std::error::Error for ModelProviderError {} + +#[cfg(test)] +#[path = "model_provider_error_tests.rs"] +mod tests; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error_tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error_tests.rs new file mode 100644 index 0000000000..0537ec6a21 --- /dev/null +++ b/codex-rs/app-server-protocol/src/protocol/v2/model_provider_error_tests.rs @@ -0,0 +1,53 @@ +//! Checks that generated provider-error schemas describe the serialized wire payloads. + +use super::ModelProviderError; +use pretty_assertions::assert_eq; +use std::collections::BTreeSet; + +#[test] +fn provider_error_schema_matches_wire_fields() -> anyhow::Result<()> { + let schema = serde_json::to_value(schemars::schema_for!(ModelProviderError))?; + for error in [ + ModelProviderError::ModelProviderMismatch { + model_provider: "old-provider".to_string(), + current_model_provider: "current-provider".to_string(), + }, + ModelProviderError::ModelProviderNotConfigured { + model_provider: "old-provider".to_string(), + }, + ModelProviderError::ModelProviderPolicyConflict { + model_provider: "old-provider".to_string(), + required_model_provider: "required-provider".to_string(), + }, + ] { + let wire = serde_json::to_value(&error)?; + let variant = schema["oneOf"] + .as_array() + .expect("tagged variants") + .iter() + .find(|variant| variant["properties"]["type"]["enum"][0] == wire["type"]) + .expect("schema variant for wire discriminator"); + let wire_fields: BTreeSet<_> = wire + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + let schema_fields: BTreeSet<_> = variant["properties"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + let required_fields: BTreeSet<_> = variant["required"] + .as_array() + .unwrap() + .iter() + .map(|value| value.as_str().unwrap()) + .collect(); + assert_eq!(schema_fields, wire_fields); + assert_eq!(required_fields, wire_fields); + assert_eq!(serde_json::from_value::(wire)?, error); + } + Ok(()) +} diff --git a/codex-rs/app-server-protocol/src/schema_fixtures.rs b/codex-rs/app-server-protocol/src/schema_fixtures.rs index d0f36db800..83e8126ba3 100644 --- a/codex-rs/app-server-protocol/src/schema_fixtures.rs +++ b/codex-rs/app-server-protocol/src/schema_fixtures.rs @@ -64,6 +64,7 @@ pub fn generate_typescript_schema_fixture_subtree_for_tests() -> Result(&mut files, &mut seen)?; + collect_typescript_fixture_file::(&mut files, &mut seen)?; collect_typescript_fixture_file::(&mut files, &mut seen)?; visit_typescript_fixture_dependencies(&mut files, &mut seen, |visitor| { visit_server_response_types(visitor); diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 3024269d72..acb7a9ac21 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -295,7 +295,7 @@ any files were compressed; failures are reported through existing logs and metri There are no progress notifications or cancellation API. Clients sharing this Codex home must support compressed rollout files, including shared histories. -## Managed model provider requirements +## Model provider admission `configRequirements/read` reports `supportsIndependentSpeedModes: true` when `features.fast_mode` and `features.ultrafast_mode` independently control Fast and @@ -307,8 +307,18 @@ Existing threads retain their provider configuration. Input RPCs reject requests `model_provider` or `model_providers` requirements no longer match that configuration, or cannot be loaded. This covers turn start/steer, review, compaction, manual queue start, and active goal updates. Realtime connections use separate routing configuration and are not checked here. -Interrupt, realtime stop, and goal pause/clear remain available. User and project -configuration changes alone do not invalidate existing threads. +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. ## Managed network requirements diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 96e30332b7..461920b677 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -1,3 +1,4 @@ +use codex_app_server_protocol::ModelProviderError; use codex_arg0::Arg0DispatchPaths; use codex_cloud_config::managed_config_bundle_loader; use codex_config::CloudConfigBundleLoader; @@ -38,12 +39,6 @@ use tracing::warn; #[path = "application_network.rs"] pub(crate) mod application_network; -#[derive(Debug, thiserror::Error)] -#[error( - "Your organization's required model provider settings changed. Restart Codex to apply them; this request was not sent" -)] -pub(crate) struct ModelProviderRequirementsChanged; - /// Shared app-server entry point for loading effective Codex configuration. #[derive(Clone)] pub(crate) struct ConfigManager { @@ -408,7 +403,12 @@ impl ConfigManager { if selection_changed || definition_changed { return Err(std::io::Error::new( std::io::ErrorKind::PermissionDenied, - ModelProviderRequirementsChanged, + ModelProviderError::ModelProviderPolicyConflict { + model_provider: current.model_provider_id.clone(), + required_model_provider: requirements + .model_provider + .unwrap_or_else(|| current.model_provider_id.clone()), + }, )); } Ok(()) @@ -430,6 +430,27 @@ 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( @@ -514,6 +535,89 @@ impl ConfigManager { }) } + /// Validates saved and requested providers before resume overrides or policy can replace them. + pub(crate) async fn load_for_resume( + &self, + saved_provider: Option<&str>, + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + cwd: Option, + ) -> std::io::Result { + let requested_provider = typesafe_overrides.model_provider.clone().or_else(|| { + request_overrides + .as_ref() + .and_then(|values| values.get("model_provider")) + .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); + self.load_with_validation( + &self.current_cli_overrides(), + request_overrides, + typesafe_overrides, + cwd, + |config, _overrides, layers| { + let requested_provider = requested_provider + .as_deref() + .or(config.model_provider.as_deref()) + .unwrap_or("openai"); + // Validate the saved route even when a client sends a provider override. + for provider in [ + saved_provider.unwrap_or(requested_provider), + requested_provider, + ] { + if let Some(required) = layers.required_model_provider() + && required != provider + { + return Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + ModelProviderError::ModelProviderPolicyConflict { + model_provider: provider.to_owned(), + required_model_provider: required.to_owned(), + }, + )); + } + if !config.model_providers.contains_key(provider) + && !built_in_model_providers(config.openai_base_url.clone()) + .contains_key(provider) + { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + ModelProviderError::ModelProviderNotConfigured { + model_provider: provider.to_owned(), + }, + )); + } + } + 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, + }, + )); + } + } + Ok(()) + }, + ) + .await + .and_then(|config| { + config.validate_windows_mxc_requirement()?; + Ok(config) + }) + } + /// Reload sources using the task's session flags before materializing config. pub(crate) async fn load_permission_config_for_thread( &self, @@ -558,11 +662,33 @@ impl ConfigManager { #[instrument(level = "trace", skip_all)] pub(crate) async fn load_with_cli_overrides( + &self, + cli_overrides: &[(String, TomlValue)], + request_overrides: Option>, + typesafe_overrides: ConfigOverrides, + fallback_cwd: Option, + ) -> std::io::Result { + self.load_with_validation( + cli_overrides, + request_overrides, + typesafe_overrides, + fallback_cwd, + |_, _, _| Ok(()), + ) + .await + } + + async fn load_with_validation( &self, cli_overrides: &[(String, TomlValue)], request_overrides: Option>, mut typesafe_overrides: ConfigOverrides, fallback_cwd: Option, + validate: impl FnOnce( + &codex_config::config_toml::ConfigToml, + &ConfigOverrides, + &ConfigLayerStack, + ) -> std::io::Result<()>, ) -> std::io::Result { let mcp_ema_cancellation = self.capture_mcp_ema_cancellation(); let policy_load = self.refresh_application_network_policy().await?; @@ -593,7 +719,7 @@ impl ConfigManager { .fallback_cwd(fallback_cwd) .cloud_config_bundle(policy_load.cloud_config.clone()) .thread_config_loader(Arc::clone(&self.thread_config_loader)) - .build() + .build_with_validation(validate) .await; let mut config = result?; self.check_application_policy_load(&policy_load)?; 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 a58138b3bf..1f2129d69d 100644 --- a/codex-rs/app-server/src/config_manager_provider_tests.rs +++ b/codex-rs/app-server/src/config_manager_provider_tests.rs @@ -1,4 +1,4 @@ -//! Checks retained providers against managed policy without reloading session configuration. +//! Checks retained providers against managed policy and the current provider selection. use super::*; use anyhow::Result; @@ -261,3 +261,62 @@ 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/message_processor_gateway_oauth_tests.rs b/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs index 2859336dbf..7dd6c9ed81 100644 --- a/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs +++ b/codex-rs/app-server/src/message_processor_gateway_oauth_tests.rs @@ -266,11 +266,23 @@ delivery = {{ kind = "header", name = "X-Gateway-Authorization" }} } }) .await?; - assert_eq!(error, JSONRPCErrorError { - code: -32600, - message: "failed to load configuration: Your organization's required model provider settings changed. Restart Codex to apply them; this request was not sent".to_string(), - data: None, - }); + assert_eq!( + error, + JSONRPCErrorError { + code: -32600, + message: codex_app_server_protocol::ModelProviderError::ModelProviderPolicyConflict { + model_provider: "gateway".into(), + required_model_provider: "openai".into(), + } + .to_string(), + data: Some(serde_json::to_value( + codex_app_server_protocol::ModelProviderError::ModelProviderPolicyConflict { + model_provider: "gateway".into(), + required_model_provider: "openai".into(), + } + )?), + } + ); assert!(server.received_requests().await.unwrap().is_empty()); processor.shutdown_threads().await; processor.drain_background_tasks().await; diff --git a/codex-rs/app-server/src/request_processors/config_errors.rs b/codex-rs/app-server/src/request_processors/config_errors.rs index 1dd9d55ef3..591af88656 100644 --- a/codex-rs/app-server/src/request_processors/config_errors.rs +++ b/codex-rs/app-server/src/request_processors/config_errors.rs @@ -14,6 +14,19 @@ fn cloud_config_bundle_load_error(err: &std::io::Error) -> Option<&CloudConfigBu } pub(crate) fn config_load_error(err: &std::io::Error) -> JSONRPCErrorError { + let mut current = err + .get_ref() + .map(|error| error as &(dyn std::error::Error + 'static)); + while let Some(source) = current { + if let Some(provider_error) = + source.downcast_ref::() + { + let mut error = invalid_request(provider_error.to_string()); + error.data = serde_json::to_value(provider_error).ok(); + return error; + } + current = source.source(); + } let data = cloud_config_bundle_load_error(err).map(|cloud_error| { let mut data = serde_json::json!({ "reason": "cloudConfigBundle", 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 bada44de51..82aeaf7f46 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(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(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 08aa330bb0..1ef252dcbf 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -247,12 +247,23 @@ fn merge_persisted_resume_metadata( typesafe_overrides: &mut ConfigOverrides, persisted_metadata: &ThreadMetadata, ) { - if has_model_resume_override(request_overrides.as_ref(), typesafe_overrides) { + let has_model_override = + has_model_resume_override(request_overrides.as_ref(), typesafe_overrides); + + // Model and reasoning overrides do not imply a provider change. + if typesafe_overrides.model_provider.is_none() + && !request_overrides + .as_ref() + .is_some_and(|overrides| overrides.contains_key("model_provider")) + { + typesafe_overrides.model_provider = Some(persisted_metadata.model_provider.clone()); + } + + if has_model_override { return; } typesafe_overrides.model = persisted_metadata.model.clone(); - typesafe_overrides.model_provider = Some(persisted_metadata.model_provider.clone()); if let Some(reasoning_effort) = persisted_metadata.reasoning_effort.as_ref() { request_overrides.get_or_insert_with(HashMap::new).insert( @@ -292,7 +303,9 @@ fn has_model_resume_override( ) -> bool { typesafe_overrides.model.is_some() || typesafe_overrides.model_provider.is_some() - || request_overrides.is_some_and(|overrides| overrides.contains_key("model")) + || request_overrides.is_some_and(|overrides| { + overrides.contains_key("model") || overrides.contains_key("model_provider") + }) || request_overrides .is_some_and(|overrides| overrides.contains_key("model_reasoning_effort")) } @@ -2564,7 +2577,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(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; self.submit_core_op(request_id, thread.as_ref(), Op::Compact) @@ -4252,7 +4265,32 @@ impl ThreadRequestProcessor { drop(_thread_list_state_permit); let config = self .config_manager - .load_for_cwd(request_overrides, typesafe_overrides, history_cwd) + .load_for_resume( + resume_source_thread + .as_ref() + .map(|thread| thread.model_provider.as_str()) + .filter(|provider| !provider.is_empty()) + .or_else(|| { + config_state + .persisted_metadata + .as_ref() + .map(|metadata| metadata.model_provider.as_str()) + }) + .or_else(|| match &thread_history { + InitialHistory::Resumed(resumed) => { + resumed.history.iter().find_map(|item| match item { + RolloutItem::SessionMeta(meta) => { + meta.meta.model_provider.as_deref() + } + _ => None, + }) + } + _ => None, + }), + request_overrides, + typesafe_overrides, + history_cwd, + ) .await .map_err(|err| config_load_error(&err))?; *prepared_config = Some(PreparedResumeConfig { @@ -4710,6 +4748,10 @@ impl ThreadRequestProcessor { mismatch_details.join("; ") ); } + self.config_manager + .check_thread_model_provider_for_input(existing_thread.config().await.as_ref()) + .await + .map_err(|err| config_load_error(&err))?; let redact_resume_payloads = should_redact_thread_resume_payloads(app_server_client_name.as_deref()); let include_turns = !params.exclude_turns; diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 3dc3432876..9ec1298cb9 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -858,7 +858,10 @@ mod thread_processor_behavior_tests { ); assert_eq!(typesafe_overrides.model, Some("gpt-5.2-codex".to_string())); - assert_eq!(typesafe_overrides.model_provider, None); + assert_eq!( + typesafe_overrides.model_provider, + Some("mock_provider".to_string()) + ); assert_eq!( request_overrides, Some(HashMap::from([( @@ -870,8 +873,7 @@ mod thread_processor_behavior_tests { } #[test] - fn merge_persisted_resume_metadata_skips_persisted_values_when_model_overridden() -> Result<()> - { + fn merge_persisted_resume_metadata_preserves_provider_when_model_overridden() -> Result<()> { let mut request_overrides = Some(HashMap::from([( "model".to_string(), serde_json::Value::String("gpt-5.2-codex".to_string()), @@ -887,7 +889,10 @@ mod thread_processor_behavior_tests { ); assert_eq!(typesafe_overrides.model, None); - assert_eq!(typesafe_overrides.model_provider, None); + assert_eq!( + typesafe_overrides.model_provider, + Some("mock_provider".to_string()) + ); assert_eq!( request_overrides, Some(HashMap::from([( @@ -922,7 +927,7 @@ mod thread_processor_behavior_tests { } #[test] - fn merge_persisted_resume_metadata_skips_persisted_values_when_reasoning_effort_overridden() + fn merge_persisted_resume_metadata_preserves_provider_when_reasoning_effort_overridden() -> Result<()> { let mut request_overrides = Some(HashMap::from([( "model_reasoning_effort".to_string(), @@ -939,7 +944,10 @@ mod thread_processor_behavior_tests { ); assert_eq!(typesafe_overrides.model, None); - assert_eq!(typesafe_overrides.model_provider, None); + assert_eq!( + typesafe_overrides.model_provider, + Some("mock_provider".to_string()) + ); assert_eq!( request_overrides, Some(HashMap::from([( 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 a2f5b5ad68..be77be34d6 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(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(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 e5be25d12d..f310c4ac65 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -338,7 +338,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(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)?; @@ -508,7 +508,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(&request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; if let Some(tool_output) = ¶ms.tool_output { @@ -1037,7 +1037,7 @@ impl TurnRequestProcessor { self.ensure_direct_input_allowed(request_id, thread.as_ref()) .await?; self.config_manager - .check_thread_model_provider(thread.config().await.as_ref()) + .check_thread_model_provider_for_input(thread.config().await.as_ref()) .await .map_err(|error| config_load_error(&error))?; diff --git a/codex-rs/app-server/tests/suite/logging.rs b/codex-rs/app-server/tests/suite/logging.rs index ce6590ed2c..348d6a8786 100644 --- a/codex-rs/app-server/tests/suite/logging.rs +++ b/codex-rs/app-server/tests/suite/logging.rs @@ -14,9 +14,12 @@ use codex_app_server_protocol::ApprovalsReviewer; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigValueWriteParams; +use codex_app_server_protocol::ConfigWriteResponse; use codex_app_server_protocol::InitializeCapabilities; use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::LoginAccountResponse; +use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::ServerRequest; @@ -289,6 +292,16 @@ supports_websockets = false let mut thread_ids = Vec::new(); for provider in ["bearer_provider", "mock_provider"] { + let config_id = app_server + .send_config_value_write_request(ConfigValueWriteParams { + file_path: None, + key_path: "model_provider".into(), + value: json!(provider), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await?; + let _: ConfigWriteResponse = app_server.read_response(config_id).await?; let thread = app_server .start_thread(ThreadStartParams { model_provider: Some(provider.into()), 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 aa4eb5b3bb..bd5f991044 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -1010,12 +1010,14 @@ 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, 1); + assert_eq!(resume_config_loads, 2); } else { assert!( - resume_config_loads >= 2, - "changed persisted metadata must reload config" + resume_config_loads >= 4, + "changed persisted metadata must reload both provider and thread 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 5fb2fa4e0a..f5bda49c8a 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 @@ -1,17 +1,15 @@ -//! Verifies provider requirements at app-server input and queue admission boundaries. +//! Verifies provider selection and requirements at app-server input and queue admission boundaries. 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; @@ -44,6 +42,7 @@ 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, @@ -80,7 +79,17 @@ base_url = "{}/v1" other.uri(), ); let requirements_path = home.path().join("requirements.toml"); - std::fs::write(&requirements_path, &requirements)?; + 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)?; + } let mut server = TestAppServer::builder() .with_codex_home(home.path()) .with_json_logging("warn") @@ -127,13 +136,31 @@ base_url = "{}/v1" } else { requirements.replace(gateway.uri(), &other.uri()) }; - std::fs::write(requirements_path, updated)?; + 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)?; + codex_app_server_protocol::ModelProviderError::ModelProviderPolicyConflict { + model_provider: "gateway".into(), + required_model_provider: required_selection.into(), + } + }; let expected_error = JSONRPCErrorError { code: -32600, - message: "failed to load configuration: Your organization's required model provider settings changed. Restart Codex to apply them; this request was not sent".to_string(), - data: None, + message: provider_error.to_string(), + data: Some(serde_json::to_value(provider_error)?), }; for (method, params) in [ + ("thread/resume", json!({"threadId": started.thread.id})), ( "thread/goal/set", json!({"threadId": started.thread.id, "objective": "Blocked objective"}), @@ -256,46 +283,71 @@ base_url = "{}/v1" } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn local_config_changes_do_not_block_existing_threads() -> Result<()> { +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_sequence( + let requests = responses::mount_sse_once( &provider, - vec![ - responses::sse(vec![responses::ev_completed("first")]), - responses::sse(vec![responses::ev_completed("second")]), - ], + responses::sse(vec![responses::ev_completed("restored")]), ) .await; - for local_settings in ["model_provider = 'openai'", "invalid toml !!!"] { - std::fs::write(home.path().join("config.toml"), local_settings)?; - let id = server - .send_turn_start_request(TurnStartParams { - thread_id: started.thread.id.clone(), - input: vec![UserInput::Text { - text: "Continue on the original provider.".into(), - text_elements: Vec::new(), - }], - ..Default::default() - }) - .await?; - let _: TurnStartResponse = server.read_response(id).await?; - server - .read_stream_until_notification_message("turn/completed") - .await?; - } - assert_eq!(requests.requests().len(), 2); + 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_do_not_block_existing_thread_turn() -> Result<()> { +async fn malformed_system_defaults_reject_input_until_provider_can_be_resolved() -> Result<()> { let provider = MockServer::start().await; let home = TempDir::new()?; MockResponsesConfig::new(&provider.uri()).write(home.path())?; @@ -310,7 +362,7 @@ async fn malformed_system_defaults_do_not_block_existing_thread_turn() -> Result .build() .await?, ); - let mut client = in_process::start(InProcessStartArgs { + let client = in_process::start(InProcessStartArgs { arg0_paths: Arg0DispatchPaths::default(), config, cli_overrides: Vec::new(), @@ -365,23 +417,9 @@ async fn malformed_system_defaults_do_not_block_existing_thread_turn() -> Result }, }) .await? - .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); + .expect_err("cannot verify the selected provider from malformed configuration"); + assert_eq!(response.code, -32600); + assert!(requests.requests().is_empty()); 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 07f05dd7e3..8badd11648 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -6782,3 +6782,221 @@ async fn setup_rollout_fixture(codex_home: &Path, server_uri: &str) -> Result 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, + )?; + if policy_conflict { + let config_path = home.path().join("config.toml"); + 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" + ), + )?; + std::fs::write( + home.path().join("requirements.toml"), + "model_provider = 'mock_provider'", + )?; + } + let mut client = TestAppServer::builder() + .with_codex_home(home.path()) + .build_initialized() + .await?; + let expected = if policy_conflict { + codex_app_server_protocol::ModelProviderError::ModelProviderPolicyConflict { + model_provider: "old_provider".into(), + required_model_provider: "mock_provider".into(), + } + } else { + codex_app_server_protocol::ModelProviderError::ModelProviderNotConfigured { + model_provider: "old_provider".into(), + } + }; + // Model and reasoning overrides must not discard the provider being validated. + for mut params in [ + ThreadResumeParams::default(), + ThreadResumeParams { + model: Some("gpt-5.1-codex".into()), + ..Default::default() + }, + ThreadResumeParams { + config: Some(std::collections::HashMap::from([( + "model".into(), + json!("gpt-5.1-codex"), + )])), + ..Default::default() + }, + ThreadResumeParams { + config: Some(std::collections::HashMap::from([( + "model_reasoning_effort".into(), + json!("high"), + )])), + ..Default::default() + }, + ] { + params.thread_id = thread_id.clone(); + let id = client.send_thread_resume_request(params).await?; + let error = client + .read_stream_until_error_message(RequestId::Integer(id)) + .await?; + assert_eq!( + error.error, + JSONRPCErrorError { + code: -32600, + message: expected.to_string(), + data: Some(serde_json::to_value(&expected)?), + } + ); + } + assert!( + server + .received_requests() + .await + .expect("mock server should record received requests") + .iter() + .all(|request| !request.url.path().ends_with("/responses")) + ); + // Failure leaves the history readable and does not create a live session with a new provider. + let id = client + .send_raw_request( + "thread/read", + Some(json!({"threadId": 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"); + // Provider overrides must not bypass admission for the saved provider. + let id = client + .send_thread_resume_request(ThreadResumeParams { + thread_id, + model: Some("gpt-5.1-codex".into()), + model_provider: typed_override.then(|| "mock_provider".into()), + config: (!typed_override).then(|| { + std::collections::HashMap::from([("model_provider".into(), json!("mock_provider"))]) + }), + ..Default::default() + }) + .await?; + let error = client + .read_stream_until_error_message(RequestId::Integer(id)) + .await?; + assert_eq!(error.error.data, Some(serde_json::to_value(expected)?)); + 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<()> { + 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 mut client = TestAppServer::builder() + .with_codex_home(home.path()) + .build_initialized() + .await?; + let params = ThreadResumeParams { + thread_id, + ..Default::default() + }; + let id = client.send_thread_resume_request(params.clone()).await?; + let error = client + .read_stream_until_error_message(RequestId::Integer(id)) + .await?; + assert_eq!( + error + .error + .data + .as_ref() + .expect("provider error should include structured data")["type"], + if configured { + "modelProviderMismatch" + } else { + "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 = 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 resumed: ThreadResumeResponse = client.read_response(id).await?; + assert_eq!(resumed.model_provider, "old_provider"); + Ok(()) +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 74750d0a13..eb949b8a46 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1558,10 +1558,23 @@ impl ConfigBuilder { pub async fn build(self) -> std::io::Result { // Keep the large config-loading future off small runtime thread stacks. - Box::pin(self.build_inner()).await + // Avoid an additional async frame for ordinary config loads. + Box::pin(self.build_inner(|_, _, _| Ok(()))).await } - async fn build_inner(self) -> std::io::Result { + /// Validates the same loaded configuration that will be materialized, before provider + /// selection or initialization. Validation cannot alter the loaded configuration. + pub async fn build_with_validation( + self, + validate: impl FnOnce(&ConfigToml, &ConfigOverrides, &ConfigLayerStack) -> std::io::Result<()>, + ) -> std::io::Result { + Box::pin(self.build_inner(validate)).await + } + + async fn build_inner( + self, + validate: impl FnOnce(&ConfigToml, &ConfigOverrides, &ConfigLayerStack) -> std::io::Result<()>, + ) -> std::io::Result { let Self { codex_home, cli_overrides, @@ -1602,6 +1615,7 @@ impl ConfigBuilder { ) .await?; let config_toml = config_toml_from_layers(&config_layer_stack).await?; + validate(&config_toml, &harness_overrides, &config_layer_stack)?; Config::load_config_with_layer_stack( LOCAL_FS.as_ref(), config_toml, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index cbad3bf375..a2d219157e 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -8702,7 +8702,7 @@ async fn prompt_edit_reverts_earlier_and_first_visible_prompts_in_place_scenario filename_ts, "2025-01-05T12:00:00Z", "unused preview", - Some("test-provider"), + Some(config.model_provider_id.as_str()), /*git_info*/ None, ) .expect("materialized rollout should be created"); diff --git a/codex-rs/tui/tests/suite/provider_defaults.rs b/codex-rs/tui/tests/suite/provider_defaults.rs index 3ae9c8532e..4a622a11c9 100644 --- a/codex-rs/tui/tests/suite/provider_defaults.rs +++ b/codex-rs/tui/tests/suite/provider_defaults.rs @@ -2,6 +2,7 @@ use super::focus_palette::PtyCodex; use super::focus_palette::write_test_config; +use anyhow::Context; use anyhow::Result; use anyhow::ensure; use app_test_support::create_fake_rollout; @@ -92,12 +93,24 @@ async fn history_lookup_uses_server_provider_with_local_and_embedded_servers() - ]); } let mut terminal = PtyCodex::start_cli(&cwd, home, &args)?; - let (expected, excluded) = if embedded || explicit { + let (expected, excluded) = if action == "resume" && last && explicit { + // The explicit provider selects the saved chat, but the daemon independently + // checks whether its configured provider can activate that chat. + ( + "This chat uses openai, but your current provider is server-provider.", + "Server provider history", + ) + } else if embedded || explicit { ("Client provider history", "Server provider history") } else { ("Server provider history", "Client provider history") }; - terminal.wait_for_screen(expected)?; + terminal.wait_for_screen(expected).with_context(|| { + format!( + "{action}: embedded={embedded}, last={last}, explicit={explicit}; screen:\n{}", + terminal.screen_contents() + ) + })?; ensure!( !terminal.screen_contains(excluded), "{action}: unexpected provider history" diff --git a/sdk/python/src/openai_codex/generated/v2_all.py b/sdk/python/src/openai_codex/generated/v2_all.py index e4d90fe916..c3a2c63d9c 100644 --- a/sdk/python/src/openai_codex/generated/v2_all.py +++ b/sdk/python/src/openai_codex/generated/v2_all.py @@ -2786,6 +2786,59 @@ class ModelProviderCapabilitiesReadResponse(BaseModel): web_search: Annotated[bool, Field(alias="webSearch")] +class ModelProviderMismatchModelProviderError(BaseModel): + model_config = ConfigDict( + populate_by_name=True, + ) + current_model_provider: Annotated[str, Field(alias="currentModelProvider")] + model_provider: Annotated[str, Field(alias="modelProvider")] + type: Annotated[ + Literal["modelProviderMismatch"], + Field(title="ModelProviderMismatchv2::ModelProviderErrorType"), + ] + + +class ModelProviderNotConfiguredModelProviderError(BaseModel): + model_config = ConfigDict( + populate_by_name=True, + ) + model_provider: Annotated[str, Field(alias="modelProvider")] + type: Annotated[ + Literal["modelProviderNotConfigured"], + Field(title="ModelProviderNotConfiguredv2::ModelProviderErrorType"), + ] + + +class ModelProviderPolicyConflictModelProviderError(BaseModel): + model_config = ConfigDict( + populate_by_name=True, + ) + model_provider: Annotated[str, Field(alias="modelProvider")] + required_model_provider: Annotated[str, Field(alias="requiredModelProvider")] + type: Annotated[ + Literal["modelProviderPolicyConflict"], + Field(title="ModelProviderPolicyConflictv2::ModelProviderErrorType"), + ] + + +class ModelProviderError( + RootModel[ + ModelProviderMismatchModelProviderError + | ModelProviderNotConfiguredModelProviderError + | ModelProviderPolicyConflictModelProviderError + ] +): + root: Annotated[ + ModelProviderMismatchModelProviderError + | ModelProviderNotConfiguredModelProviderError + | ModelProviderPolicyConflictModelProviderError, + Field( + description="Structured JSON-RPC error data. Unknown errors must retain the client's normal fallback.", + title="ModelProviderError", + ), + ] + + class ModelRerouteReason(RootModel[Literal["highRiskCyberActivity"]]): root: Literal["highRiskCyberActivity"] diff --git a/sdk/python/tests/test_contract_generation.py b/sdk/python/tests/test_contract_generation.py index 11acc14205..178f5c6a64 100644 --- a/sdk/python/tests/test_contract_generation.py +++ b/sdk/python/tests/test_contract_generation.py @@ -119,3 +119,26 @@ def test_schema_generation_failure_does_not_update_python(monkeypatch): with pytest.raises(subprocess.CalledProcessError): runpy.run_path(str(script), run_name="__main__") assert calls == ["cargo"] + + +@pytest.mark.parametrize( + "payload", + [ + {"type": "modelProviderNotConfigured", "modelProvider": "old-provider"}, + { + "type": "modelProviderMismatch", + "modelProvider": "old-provider", + "currentModelProvider": "current-provider", + }, + { + "type": "modelProviderPolicyConflict", + "modelProvider": "old-provider", + "requiredModelProvider": "required-provider", + }, + ], +) +def test_model_provider_errors_round_trip_wire_names(payload): + from openai_codex.generated.v2_all import ModelProviderError + + error = ModelProviderError.model_validate(payload) + assert error.model_dump(by_alias=True) == payload