diff --git a/codex-rs/app-server/src/config_manager.rs b/codex-rs/app-server/src/config_manager.rs index 4fe02c5264..fd37f03b51 100644 --- a/codex-rs/app-server/src/config_manager.rs +++ b/codex-rs/app-server/src/config_manager.rs @@ -4,6 +4,7 @@ use codex_config::CloudConfigBundleLoader; use codex_config::ConfigLayerStack; use codex_config::LoaderOverrides; use codex_config::ThreadConfigLoader; +use codex_config::apply_default_gcp_auth; use codex_config::loader::load_config_layers_state; use codex_config::loader::load_managed_requirements_state; use codex_core::config::Config; @@ -15,6 +16,7 @@ use codex_login::AuthManager; use codex_login::default_client::set_default_client_residency_requirement; use codex_model_provider_info::AMAZON_BEDROCK_PROVIDER_ID; use codex_model_provider_info::AMAZON_BEDROCK_RUNTIME_PROVIDER_ID; +use codex_model_provider_info::GCP_PROVIDER_ID; use codex_model_provider_info::built_in_model_providers; use codex_model_provider_info::merge_configured_model_providers; use codex_utils_absolute_path::AbsolutePathBuf; @@ -371,16 +373,24 @@ impl ConfigManager { Some(provider) if matches!( current.model_provider_id.as_str(), - AMAZON_BEDROCK_PROVIDER_ID | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID + AMAZON_BEDROCK_PROVIDER_ID + | AMAZON_BEDROCK_RUNTIME_PROVIDER_ID + | GCP_PROVIDER_ID ) => { - // Bedrock requirements contain overrides of the built-in provider. - merge_configured_model_providers( + // Managed overrides must receive the same built-in defaults as startup config. + let mut provider = merge_configured_model_providers( built_in_model_providers(/*openai_base_url*/ None), HashMap::from([(current.model_provider_id.clone(), provider)]), ) .map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidData, message))? - .remove(¤t.model_provider_id) + .remove(¤t.model_provider_id); + if current.model_provider_id == GCP_PROVIDER_ID + && let Some(provider) = provider.as_mut() + { + apply_default_gcp_auth(provider, ¤t.cwd); + } + provider } Some(provider) => Some(provider), None => None, 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 86ee457514..b05521f5c0 100644 --- a/codex-rs/app-server/src/config_manager_provider_tests.rs +++ b/codex-rs/app-server/src/config_manager_provider_tests.rs @@ -14,13 +14,22 @@ use pretty_assertions::assert_eq; use tempfile::tempdir; use test_case::test_case; -#[test_case(AMAZON_BEDROCK_PROVIDER_ID; "bedrock")] -#[test_case(AMAZON_BEDROCK_RUNTIME_PROVIDER_ID; "bedrock_runtime")] +#[test_case(AMAZON_BEDROCK_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1"; "bedrock")] +#[test_case(AMAZON_BEDROCK_RUNTIME_PROVIDER_ID, "aws.region = 'us-west-2'", "us-west-2", "us-east-1"; "bedrock_runtime")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "example-project", "other-project"; "gcp_project")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nlocation = 'us-central1'", "us-central1", "us-east4"; "gcp_location")] +#[test_case(GCP_PROVIDER_ID, "project_id = 'example-project'\nauth.command = 'print-token'", "print-token", "other-token"; "gcp_auth")] #[tokio::test] -async fn provider_requirements_resolve_bedrock_overrides(provider_id: &str) -> Result<()> { +async fn provider_requirements_resolve_builtin_overrides( + provider_id: &str, + provider_config: &str, + original: &str, + replacement: &str, +) -> Result<()> { let home = tempdir()?; + let workspace = tempdir()?; let requirements = format!( - "model_provider = '{provider_id}'\n[model_providers.{provider_id}.aws]\nregion = 'us-west-2'" + "model_provider = '{provider_id}'\n[model_providers.{provider_id}]\n{provider_config}" ); let manager = ConfigManager::new_for_tests( home.path().to_path_buf(), @@ -28,7 +37,9 @@ async fn provider_requirements_resolve_bedrock_overrides(provider_id: &str) -> R LoaderOverrides::without_managed_config_for_tests(), CloudConfigBundleFixture::loader_with_enterprise_requirement(&requirements), ); - let current = manager.load_latest_config(/*fallback_cwd*/ None).await?; + let current = manager + .load_latest_config(Some(workspace.path().to_path_buf())) + .await?; manager.check_thread_model_provider(¤t).await?; let changed = ConfigManager::new_for_tests( @@ -36,7 +47,7 @@ async fn provider_requirements_resolve_bedrock_overrides(provider_id: &str) -> R Vec::new(), LoaderOverrides::without_managed_config_for_tests(), CloudConfigBundleFixture::loader_with_enterprise_requirement( - requirements.replace("us-west-2", "us-east-1"), + requirements.replace(original, replacement), ), ); assert_eq!( 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 dab5a6062f..147c1e084d 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 @@ -655,6 +655,8 @@ mod thread_processor_behavior_tests { supports_websockets: true, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; let config_manager = ConfigManager::new( diff --git a/codex-rs/app-server/tests/suite/v2/model_list_requirements_tests.rs b/codex-rs/app-server/tests/suite/v2/model_list_requirements_tests.rs index 6c2f24724d..de7e5af77f 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list_requirements_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list_requirements_tests.rs @@ -48,6 +48,32 @@ async fn list_models(server: &mut TestAppServer) -> Result { .await } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn model_list_accepts_unchanged_gcp_requirements() -> Result<()> { + let home = TempDir::new()?; + std::fs::write(home.path().join("config.toml"), "")?; + std::fs::write( + home.path().join("requirements.toml"), + r#" +model_provider = "gcp" +[model_providers.gcp] +project_id = "managed-project" +location = "us-central1" +"#, + )?; + let mut server = TestAppServer::builder() + .with_mock_chatgpt_backend() + .with_codex_home(home.path()) + .with_env_overrides(&[("OPENAI_API_KEY", None)]) + .build_initialized() + .await?; + + let initial = list_models(&mut server).await?; + assert!(!initial.data.is_empty()); + assert_eq!(list_models(&mut server).await?, initial); + Ok(()) +} + async fn sign_in(server: &mut TestAppServer) -> Result<()> { let id = server .send_login_account_api_key_request("sk-catalog-test") diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 71ad4e2bd8..9b6cd84638 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -30,6 +30,7 @@ pub use mcp_ema_cancellation::McpEmaCancellation; mod mcp_requirements; mod mcp_types; mod merge; +mod model_provider_defaults; mod model_provider_requirements; mod overrides; mod path_context; @@ -201,6 +202,7 @@ pub use merge::ShellEnvironmentPolicyFilterRepresentation; pub use merge::is_structured_feature_path; pub use merge::merge_toml_values; pub use merge::shell_environment_filter_entry; +pub use model_provider_defaults::apply_default_gcp_auth; pub use overrides::build_cli_overrides_layer; pub use path_context::ConfigPathContext; pub use plugin_edit::PluginConfigEdit; diff --git a/codex-rs/config/src/model_provider_defaults.rs b/codex-rs/config/src/model_provider_defaults.rs new file mode 100644 index 0000000000..ee00b9ea96 --- /dev/null +++ b/codex-rs/config/src/model_provider_defaults.rs @@ -0,0 +1,27 @@ +//! Applies runtime provider defaults using the resolved session working directory. + +use codex_model_provider_info::ModelProviderInfo; +use codex_protocol::config_types::ModelProviderAuthInfo; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::num::NonZeroU64; + +/// Supplies GCP command auth when no explicit auth override is configured. +pub fn apply_default_gcp_auth(provider: &mut ModelProviderInfo, cwd: &AbsolutePathBuf) { + provider.auth.get_or_insert_with(|| ModelProviderAuthInfo { + command: if cfg!(windows) { + "gcloud.cmd" + } else { + "gcloud" + } + .to_string(), + args: vec![ + "auth".into(), + "application-default".into(), + "print-access-token".into(), + ], + timeout_ms: NonZeroU64::new(30_000).unwrap_or(NonZeroU64::MIN), + refresh_interval_ms: 3_000_000, + // Registry construction must not depend on the process cwd, which may be deleted. + cwd: cwd.clone(), + }); +} diff --git a/codex-rs/config/src/thread_config.rs b/codex-rs/config/src/thread_config.rs index 0518ea887b..16a2e09ad6 100644 --- a/codex-rs/config/src/thread_config.rs +++ b/codex-rs/config/src/thread_config.rs @@ -317,6 +317,8 @@ mod tests { supports_websockets: true, supports_standalone_web_search: true, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } } diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto index b9f89c95c6..efd43c4d20 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto @@ -50,6 +50,8 @@ message ModelProvider { bool supports_websockets = 17; bool supports_standalone_web_search = 18; optional string model_catalog_url = 19; + optional string project_id = 20; + optional string location = 21; } message StringMap { diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs index 6ffc7bbdf6..2e5a3788e3 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs @@ -79,6 +79,10 @@ pub struct ModelProvider { pub supports_standalone_web_search: bool, #[prost(string, optional, tag = "19")] pub model_catalog_url: ::core::option::Option<::prost::alloc::string::String>, + #[prost(string, optional, tag = "20")] + pub project_id: ::core::option::Option<::prost::alloc::string::String>, + #[prost(string, optional, tag = "21")] + pub location: ::core::option::Option<::prost::alloc::string::String>, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct StringMap { diff --git a/codex-rs/config/src/thread_config/remote.rs b/codex-rs/config/src/thread_config/remote.rs index 0f2a436758..b9d8127da6 100644 --- a/codex-rs/config/src/thread_config/remote.rs +++ b/codex-rs/config/src/thread_config/remote.rs @@ -196,6 +196,8 @@ fn model_provider_from_proto( supports_websockets: provider.supports_websockets, supports_standalone_web_search: provider.supports_standalone_web_search, capabilities: None, + project_id: provider.project_id, + location: provider.location, include_internal_metadata: false, }; Ok((id, info)) @@ -216,6 +218,8 @@ fn model_provider_to_proto( auth, gateway_oauth: _, aws: _, + project_id, + location, wire_api, query_params, http_headers, @@ -251,6 +255,8 @@ fn model_provider_to_proto( requires_openai_auth, supports_websockets, supports_standalone_web_search, + project_id, + location, } } @@ -446,6 +452,8 @@ mod tests { #[test] fn model_provider_proto_roundtrips_through_domain_type() { let mut expected = expected_provider(); + expected.project_id = Some("vertex-project".to_string()); + expected.location = Some("us-central1".to_string()); expected.auth = None; expected.experimental_bearer_token = Some("synthetic-provider-token".into()); let proto = model_provider_to_proto("local", expected.clone()); @@ -520,6 +528,8 @@ mod tests { requires_openai_auth: false, supports_websockets: true, supports_standalone_web_search: true, + project_id: None, + location: None, }], features: HashMap::from([ ("plugins".to_string(), false), @@ -585,6 +595,8 @@ mod tests { gateway_oauth: None, aws: None, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a75092c17f..2c9b7a3f51 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3024,6 +3024,10 @@ "description": "Additional HTTP headers to include in requests to this provider where the (key, value) pairs are the header name and value.", "type": "object" }, + "location": { + "description": "Google Cloud location for the built-in GCP provider. Takes precedence over `GOOGLE_CLOUD_LOCATION`; defaults to `global` when neither is set.", + "type": "string" + }, "model_catalog_url": { "description": "Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery uses the Codex backend unless `base_url` overrides the inference endpoint.", "type": "string" @@ -3033,6 +3037,10 @@ "description": "Friendly display name.", "type": "string" }, + "project_id": { + "description": "Google Cloud project ID for the built-in GCP provider. Takes precedence over `GOOGLE_CLOUD_PROJECT` when deriving the Vertex AI endpoint.", + "type": "string" + }, "query_params": { "additionalProperties": { "type": "string" diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 5077994d69..171bcc3ba2 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -25,6 +25,7 @@ use codex_config::ResidencyRequirement; use codex_config::SandboxModeRequirement; use codex_config::Sourced; use codex_config::ThreadConfigLoader; +use codex_config::apply_default_gcp_auth; use codex_config::config_toml::CircuitBreakAction; use codex_config::config_toml::ConfigToml; use codex_config::config_toml::DEFAULT_PROJECT_DOC_MAX_BYTES; @@ -101,7 +102,6 @@ use codex_protocol::MemoryVersion; use codex_protocol::config_types::AltScreenMode; use codex_protocol::config_types::AutoCompactTokenLimitScope; use codex_protocol::config_types::ForcedLoginMethod; -use codex_protocol::config_types::ModelProviderAuthInfo; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; @@ -3995,21 +3995,8 @@ impl Config { merge_configured_model_providers(built_in_model_providers(openai_base_url), cfg.model_providers) .map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidData, message))?; - if let Some(provider) = model_providers.get_mut(GCP_PROVIDER_ID) - && provider.auth.is_none() - { - // Registry construction must not depend on the process cwd, which may be deleted. - provider.auth = Some(ModelProviderAuthInfo { - command: if cfg!(windows) { "gcloud.cmd" } else { "gcloud" }.to_string(), - args: vec![ - "auth".into(), - "application-default".into(), - "print-access-token".into(), - ], - timeout_ms: std::num::NonZeroU64::new(30_000).unwrap_or(std::num::NonZeroU64::MIN), - refresh_interval_ms: 3_000_000, - cwd: resolved_cwd.clone(), - }); + if let Some(provider) = model_providers.get_mut(GCP_PROVIDER_ID) { + apply_default_gcp_auth(provider, &resolved_cwd); } let model_provider_id = config_layer_stack.required_model_provider().map(str::to_string) diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 74b52a3cac..78215bf75d 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -93,6 +93,8 @@ async fn responses_stream_includes_subagent_header_on_review() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -240,6 +242,8 @@ async fn responses_stream_includes_subagent_header_on_other() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -368,6 +372,8 @@ async fn responses_respects_model_info_overrides_from_config() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 2c01fcb17d..d842f0fe5c 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1734,6 +1734,8 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -3240,6 +3242,8 @@ async fn azure_responses_request_does_not_store_and_preserves_prefixed_item_ids( supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -4088,6 +4092,8 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -4176,6 +4182,8 @@ async fn env_var_overrides_loaded_auth() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 288b8e208b..5f8c3bf717 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -2733,6 +2733,8 @@ fn websocket_provider_with_connect_timeout( supports_websockets: true, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } } diff --git a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs index c381b78cda..196e202523 100644 --- a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs +++ b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs @@ -85,6 +85,8 @@ async fn continue_after_stream_error() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; diff --git a/codex-rs/core/tests/suite/stream_no_completed.rs b/codex-rs/core/tests/suite/stream_no_completed.rs index 1823275ea4..5386eb2364 100644 --- a/codex-rs/core/tests/suite/stream_no_completed.rs +++ b/codex-rs/core/tests/suite/stream_no_completed.rs @@ -71,6 +71,8 @@ async fn retries_on_early_close() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; diff --git a/codex-rs/login/src/auth_env_telemetry.rs b/codex-rs/login/src/auth_env_telemetry.rs index 6ee0c43920..8f1c8ad8e1 100644 --- a/codex-rs/login/src/auth_env_telemetry.rs +++ b/codex-rs/login/src/auth_env_telemetry.rs @@ -80,6 +80,8 @@ mod tests { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 177d7c5917..7e635a392d 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -165,6 +165,12 @@ pub struct ModelProviderInfo { pub gateway_oauth: Option, /// AWS SigV4 auth configuration for this provider. pub aws: Option, + /// Google Cloud project ID for the built-in GCP provider. Takes precedence + /// over `GOOGLE_CLOUD_PROJECT` when deriving the Vertex AI endpoint. + pub project_id: Option, + /// Google Cloud location for the built-in GCP provider. Takes precedence + /// over `GOOGLE_CLOUD_LOCATION`; defaults to `global` when neither is set. + pub location: Option, /// Which wire protocol this provider expects. #[serde(default)] pub wire_api: WireApi, @@ -565,6 +571,8 @@ other non-default provider fields are not supported" supports_websockets: true, supports_standalone_web_search: true, capabilities: None, + project_id: None, + location: None, include_internal_metadata: true, } } @@ -605,6 +613,8 @@ other non-default provider fields are not supported" supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } } @@ -641,6 +651,8 @@ other non-default provider fields are not supported" requires_openai_auth: false, supports_websockets: false, supports_standalone_web_search: false, + project_id: None, + location: None, include_internal_metadata: false, } } @@ -765,14 +777,18 @@ pub fn merge_configured_model_providers( let base_url_override = provider.base_url.take(); let auth_override = provider.auth.take(); let http_headers_override = provider.http_headers.take(); + let project_id_override = provider.project_id.take(); + let location_override = provider.location.take(); if provider != ModelProviderInfo::default() { return Err(format!( "model_providers.{GCP_PROVIDER_ID} only supports changing \ -`base_url`, `auth`, and `http_headers`; other non-default provider fields are not supported" +`base_url`, `auth`, `http_headers`, `project_id`, and `location`; other non-default provider fields are not supported" )); } if let Some(built_in_provider) = model_providers.get_mut(GCP_PROVIDER_ID) { + built_in_provider.project_id = project_id_override; + built_in_provider.location = location_override; if let Some(base_url_override) = base_url_override { built_in_provider.base_url = Some(base_url_override); } @@ -836,6 +852,8 @@ pub fn create_oss_provider_with_base_url(base_url: &str, wire_api: WireApi) -> M supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } } diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index 10a70b66ee..6156d06513 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -97,6 +97,8 @@ base_url = "http://localhost:11434/v1" supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -136,6 +138,8 @@ query_params = { api-version = "2025-04-01-preview" } supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -179,6 +183,8 @@ supports_standalone_web_search = true supports_websockets: false, supports_standalone_web_search: true, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, }; @@ -369,6 +375,8 @@ fn test_create_amazon_bedrock_provider() { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } ); @@ -644,6 +652,8 @@ fn test_merge_configured_model_providers_applies_gcp_transport_overrides() { GCP_PROVIDER_ID.to_string(), ModelProviderInfo { base_url: Some("https://vertex.example.com/v1".to_string()), + project_id: Some("configured-project".to_string()), + location: Some("us-east4".to_string()), auth: Some(auth.clone()), http_headers: Some(maplit::hashmap! { "x-example-header".to_string() => "value".into(), @@ -657,6 +667,8 @@ fn test_merge_configured_model_providers_applies_gcp_transport_overrides() { .get_mut(GCP_PROVIDER_ID) .expect("GCP provider should be built in"); provider.base_url = Some("https://vertex.example.com/v1".to_string()); + provider.project_id = Some("configured-project".to_string()); + provider.location = Some("us-east4".to_string()); provider.auth = Some(auth); provider.http_headers = Some(maplit::hashmap! { "x-example-header".to_string() => "value".into(), diff --git a/codex-rs/model-provider/src/gcp/gcp_tests.rs b/codex-rs/model-provider/src/gcp/gcp_tests.rs index e2046dbe55..fb3fbed094 100644 --- a/codex-rs/model-provider/src/gcp/gcp_tests.rs +++ b/codex-rs/model-provider/src/gcp/gcp_tests.rs @@ -9,8 +9,12 @@ use super::*; #[test] fn vertex_base_url_defaults_to_global_location() { assert_eq!( - vertex_base_url(Some("example-project"), /*location*/ None) - .expect("project should resolve a Vertex endpoint"), + vertex_base_url( + &ModelProviderInfo::default(), + Some("example-project"), + /*environment_location*/ None + ) + .expect("project should resolve a Vertex endpoint"), "https://aiplatform.googleapis.com/v1/projects/example-project/locations/global/publishers/openai/v1" ); } @@ -18,31 +22,111 @@ fn vertex_base_url_defaults_to_global_location() { #[test] fn vertex_base_url_uses_explicit_location() { assert_eq!( - vertex_base_url(Some("example-project"), Some("us-central1")) - .expect("project and location should resolve a Vertex endpoint"), + vertex_base_url( + &ModelProviderInfo::default(), + Some("example-project"), + Some("us-central1") + ) + .expect("project and location should resolve a Vertex endpoint"), "https://aiplatform.googleapis.com/v1/projects/example-project/locations/us-central1/publishers/openai/v1" ); } #[test] fn vertex_base_url_explains_missing_project() { - let error = vertex_base_url(/*project*/ None, /*location*/ None) - .expect_err("missing project should be rejected"); + let error = vertex_base_url( + &ModelProviderInfo::default(), + /*environment_project*/ None, + /*environment_location*/ None, + ) + .expect_err("missing project should be rejected"); assert_eq!( error.to_string(), - "Missing environment variable: `GOOGLE_CLOUD_PROJECT`. Set GOOGLE_CLOUD_PROJECT to your Google Cloud project ID, or configure model_providers.gcp.base_url." + "Missing environment variable: `GOOGLE_CLOUD_PROJECT`. Set model_providers.gcp.project_id or GOOGLE_CLOUD_PROJECT to your Google Cloud project ID, or configure model_providers.gcp.base_url." ); } #[test] fn vertex_base_url_rejects_path_components() { - let error = vertex_base_url(Some("example-project/other"), /*location*/ None) - .expect_err("project must not add endpoint path components"); + let error = vertex_base_url( + &ModelProviderInfo::default(), + Some("example-project/other"), + /*environment_location*/ None, + ) + .expect_err("project must not add endpoint path components"); assert_eq!( error.to_string(), - "GOOGLE_CLOUD_PROJECT must contain only ASCII letters, digits, and hyphens" + "GOOGLE_CLOUD_PROJECT must be non-empty and contain only ASCII letters, digits, and hyphens" + ); +} + +#[test] +fn configured_vertex_endpoint_resolves_each_field_independently() { + for (project_id, location, expected) in [ + ( + Some("configured-project"), + Some("us-east4"), + "https://aiplatform.googleapis.com/v1/projects/configured-project/locations/us-east4/publishers/openai/v1", + ), + ( + Some("configured-project"), + None, + "https://aiplatform.googleapis.com/v1/projects/configured-project/locations/us-central1/publishers/openai/v1", + ), + ( + None, + Some("us-east4"), + "https://aiplatform.googleapis.com/v1/projects/environment-project/locations/us-east4/publishers/openai/v1", + ), + ] { + let info = ModelProviderInfo { + project_id: project_id.map(str::to_string), + location: location.map(str::to_string), + ..ModelProviderInfo::default() + }; + assert_eq!( + vertex_base_url(&info, Some("environment-project"), Some("us-central1")) + .expect("configured fields should take precedence over environment values"), + expected + ); + } +} + +#[test] +fn empty_configured_project_does_not_fall_back_to_environment() { + let info = ModelProviderInfo { + project_id: Some(String::new()), + ..ModelProviderInfo::default() + }; + let error = vertex_base_url(&info, Some("environment-project"), Some("us-central1")) + .expect_err("invalid explicit config should not fall back to environment values"); + + assert_eq!( + error.to_string(), + "model_providers.gcp.project_id must be non-empty and contain only ASCII letters, digits, and hyphens" + ); +} + +#[tokio::test] +async fn configured_project_and_location_build_api_provider() { + let provider = GcpModelProvider::new( + ModelProviderInfo { + project_id: Some("configured-project".to_string()), + location: Some("us-east4".to_string()), + ..ModelProviderInfo::create_gcp_provider() + }, + /*auth_manager*/ None, + ); + + assert_eq!( + provider + .api_provider() + .await + .expect("configured endpoint should build") + .base_url, + "https://aiplatform.googleapis.com/v1/projects/configured-project/locations/us-east4/publishers/openai/v1" ); } @@ -60,6 +144,8 @@ async fn configured_base_url_bypasses_project_discovery() { .try_into() .expect("current directory should be absolute"), }); + provider_info.project_id = Some("configured-project".to_string()); + provider_info.location = Some("us-east4".to_string()); let provider = GcpModelProvider::new(provider_info, /*auth_manager*/ None); assert_eq!( diff --git a/codex-rs/model-provider/src/gcp/mod.rs b/codex-rs/model-provider/src/gcp/mod.rs index 831b9e719c..fc1371d278 100644 --- a/codex-rs/model-provider/src/gcp/mod.rs +++ b/codex-rs/model-provider/src/gcp/mod.rs @@ -69,7 +69,7 @@ impl GcpModelProvider { let project = std::env::var(GOOGLE_CLOUD_PROJECT_ENV_VAR).ok(); let location = std::env::var(GOOGLE_CLOUD_LOCATION_ENV_VAR).ok(); - vertex_base_url(project.as_deref(), location.as_deref()).map(Some) + vertex_base_url(&self.info, project.as_deref(), location.as_deref()).map(Some) } } @@ -172,35 +172,66 @@ impl ModelProvider for GcpModelProvider { } } -fn vertex_base_url(project: Option<&str>, location: Option<&str>) -> Result { - let project = project +fn vertex_base_url( + info: &ModelProviderInfo, + environment_project: Option<&str>, + environment_location: Option<&str>, +) -> Result { + let project = info + .project_id + .as_deref() .map(str::trim) - .filter(|project| !project.is_empty()) + .or_else(|| { + environment_project + .map(str::trim) + .filter(|project| !project.is_empty()) + }) .ok_or_else(|| { CodexErr::EnvVar(EnvVarError { var: GOOGLE_CLOUD_PROJECT_ENV_VAR.to_string(), instructions: Some( - "Set GOOGLE_CLOUD_PROJECT to your Google Cloud project ID, or configure \ -model_providers.gcp.base_url." + "Set model_providers.gcp.project_id or GOOGLE_CLOUD_PROJECT to your Google \ +Cloud project ID, or configure model_providers.gcp.base_url." .to_string(), ), }) })?; - let location = location + let location = info + .location + .as_deref() .map(str::trim) - .filter(|location| !location.is_empty()) + .or_else(|| { + environment_location + .map(str::trim) + .filter(|location| !location.is_empty()) + }) .unwrap_or(DEFAULT_GOOGLE_CLOUD_LOCATION); for (name, value) in [ - (GOOGLE_CLOUD_PROJECT_ENV_VAR, project), - (GOOGLE_CLOUD_LOCATION_ENV_VAR, location), + ( + if info.project_id.is_some() { + "model_providers.gcp.project_id" + } else { + GOOGLE_CLOUD_PROJECT_ENV_VAR + }, + project, + ), + ( + if info.location.is_some() { + "model_providers.gcp.location" + } else { + GOOGLE_CLOUD_LOCATION_ENV_VAR + }, + location, + ), ] { - if !value - .chars() - .all(|character| character.is_ascii_alphanumeric() || character == '-') + if value.is_empty() + || !value + .chars() + .all(|character| character.is_ascii_alphanumeric() || character == '-') { return Err(CodexErr::InvalidRequest(format!( - "{name} must contain only ASCII letters, digits, and hyphens" + "{name} must be non-empty and contain only ASCII letters, digits, and hyphens" ))); } } diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index 49592497c3..15aeffe062 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -667,6 +667,8 @@ mod tests { supports_websockets: false, supports_standalone_web_search: false, capabilities: None, + project_id: None, + location: None, include_internal_metadata: false, } }