diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index ba116ec0e8..131433208a 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -40,7 +40,6 @@ pub(crate) use review::routes_approval_to_guardian; pub(crate) use review::spawn_approval_request_review; pub(crate) use review_session::GuardianReviewSessionManager; -const GUARDIAN_PREFERRED_MODEL: &str = "codex-auto-review"; pub(crate) const GUARDIAN_REVIEW_TIMEOUT: Duration = Duration::from_secs(90); pub(crate) const GUARDIAN_REVIEWER_NAME: &str = "guardian"; pub(crate) const MAX_CONSECUTIVE_GUARDIAN_DENIALS_PER_TURN: u32 = 3; diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 3e88f9b08f..2908492ddd 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -659,9 +659,10 @@ pub(super) async fn run_guardian_review_session( fallback } }; + let preferred_model_id = turn.provider.approval_review_preferred_model(); let preferred_model = available_models .iter() - .find(|preset| preset.model == super::GUARDIAN_PREFERRED_MODEL); + .find(|preset| preset.model == preferred_model_id); let (guardian_model, guardian_reasoning_effort) = if let Some(preset) = preferred_model { let reasoning_effort = preferred_reasoning_effort( preset @@ -670,10 +671,7 @@ pub(super) async fn run_guardian_review_session( .any(|effort| effort.effort == codex_protocol::openai_models::ReasoningEffort::Low), Some(preset.default_reasoning_effort), ); - ( - super::GUARDIAN_PREFERRED_MODEL.to_string(), - reasoning_effort, - ) + (preferred_model_id.to_string(), reasoning_effort) } else { let reasoning_effort = preferred_reasoning_effort( turn.model_info diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index e647796cbf..7601ef44c3 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -22,6 +22,9 @@ use codex_config::types::McpServerConfig; use codex_exec_server::LOCAL_FS; use codex_features::Feature; use codex_model_provider::create_model_provider; +use codex_model_provider_info::AMAZON_BEDROCK_GPT_5_4_MODEL_ID; +use codex_model_provider_info::AMAZON_BEDROCK_PROVIDER_ID; +use codex_model_provider_info::ModelProviderInfo; use codex_network_proxy::NetworkProxyConfig; use codex_protocol::ThreadId; use codex_protocol::approvals::NetworkApprovalProtocol; @@ -29,6 +32,7 @@ use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::ContentItem; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; +use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; @@ -2336,6 +2340,35 @@ async fn guardian_review_session_config_uses_parent_active_model_instead_of_hard assert_eq!(guardian_config.model, Some("active-model".to_string())); } +#[tokio::test] +async fn guardian_review_session_config_keeps_bedrock_provider_for_bedrock_gpt_5_4() { + let mut parent_config = test_config().await; + parent_config.model_provider_id = AMAZON_BEDROCK_PROVIDER_ID.to_string(); + parent_config.model_provider = + ModelProviderInfo::create_amazon_bedrock_provider(/*aws*/ None); + + let guardian_config = build_guardian_review_session_config_for_test( + &parent_config, + /*live_network_config*/ None, + AMAZON_BEDROCK_GPT_5_4_MODEL_ID, + Some(ReasoningEffort::Low), + ) + .expect("guardian config"); + + assert_eq!( + ( + guardian_config.model, + guardian_config.model_provider_id, + guardian_config.model_provider, + ), + ( + Some(AMAZON_BEDROCK_GPT_5_4_MODEL_ID.to_string()), + AMAZON_BEDROCK_PROVIDER_ID.to_string(), + ModelProviderInfo::create_amazon_bedrock_provider(/*aws*/ None), + ) + ); +} + #[tokio::test] async fn guardian_review_session_config_uses_requirements_guardian_policy_config() { let codex_home = tempfile::tempdir().expect("create temp dir"); diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 6856b80b71..65f71851d6 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -37,6 +37,7 @@ pub const OPENAI_PROVIDER_ID: &str = "openai"; pub const CHATGPT_CODEX_BASE_URL: &str = "https://chatgpt.com/backend-api/codex"; const AMAZON_BEDROCK_PROVIDER_NAME: &str = "Amazon Bedrock"; pub const AMAZON_BEDROCK_PROVIDER_ID: &str = "amazon-bedrock"; +pub const AMAZON_BEDROCK_GPT_5_4_MODEL_ID: &str = "openai.gpt-5.4"; pub const AMAZON_BEDROCK_DEFAULT_BASE_URL: &str = "https://bedrock-mantle.us-east-1.api.aws/openai/v1"; const AMAZON_BEDROCK_MANTLE_CLIENT_AGENT_HEADER: &str = "x-amzn-mantle-client-agent"; diff --git a/codex-rs/model-provider/src/amazon_bedrock/catalog.rs b/codex-rs/model-provider/src/amazon_bedrock/catalog.rs index dc286fe6d6..d6fe1de095 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/catalog.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/catalog.rs @@ -1,3 +1,4 @@ +use codex_model_provider_info::AMAZON_BEDROCK_GPT_5_4_MODEL_ID; use codex_models_manager::model_info::BASE_INSTRUCTIONS; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::ServiceTier; @@ -18,7 +19,6 @@ use codex_protocol::openai_models::WebSearchToolType; const GPT_OSS_CONTEXT_WINDOW: i64 = 128_000; const GPT_5_4_CONTEXT_WINDOW: i64 = 272_000; const GPT_5_4_MAX_CONTEXT_WINDOW: i64 = 1_000_000; -const GPT_5_4_CMB_MODEL_ID: &str = "openai.gpt-5.4"; pub(crate) fn static_model_catalog() -> ModelsResponse { ModelsResponse { @@ -40,7 +40,7 @@ pub(crate) fn static_model_catalog() -> ModelsResponse { fn gpt_5_4_cmb_bedrock_model(priority: i32) -> ModelInfo { ModelInfo { - slug: GPT_5_4_CMB_MODEL_ID.to_string(), + slug: AMAZON_BEDROCK_GPT_5_4_MODEL_ID.to_string(), display_name: "gpt-5.4".to_string(), description: Some("Strong model for everyday coding.".to_string()), default_reasoning_level: Some(ReasoningEffort::Medium), @@ -155,7 +155,7 @@ mod tests { let catalog = static_model_catalog(); assert_eq!(catalog.models.len(), 3); - assert_eq!(catalog.models[0].slug, GPT_5_4_CMB_MODEL_ID); + assert_eq!(catalog.models[0].slug, AMAZON_BEDROCK_GPT_5_4_MODEL_ID); assert_eq!(catalog.models[1].slug, "openai.gpt-oss-120b"); assert_eq!(catalog.models[2].slug, "openai.gpt-oss-20b"); } @@ -166,7 +166,7 @@ mod tests { let cmb_model = catalog .models .iter() - .find(|model| model.slug == GPT_5_4_CMB_MODEL_ID) + .find(|model| model.slug == AMAZON_BEDROCK_GPT_5_4_MODEL_ID) .expect("Bedrock catalog should include GPT-5.4 CMB"); assert_eq!( diff --git a/codex-rs/model-provider/src/amazon_bedrock/mod.rs b/codex-rs/model-provider/src/amazon_bedrock/mod.rs index adca7d7d91..3940f73fcd 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/mod.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/mod.rs @@ -9,6 +9,7 @@ use codex_api::Provider; use codex_api::SharedAuthProvider; use codex_login::AuthManager; use codex_login::CodexAuth; +use codex_model_provider_info::AMAZON_BEDROCK_GPT_5_4_MODEL_ID; use codex_model_provider_info::ModelProviderAwsAuthInfo; use codex_model_provider_info::ModelProviderInfo; use codex_models_manager::manager::SharedModelsManager; @@ -62,6 +63,10 @@ impl ModelProvider for AmazonBedrockModelProvider { } } + fn approval_review_preferred_model(&self) -> &'static str { + AMAZON_BEDROCK_GPT_5_4_MODEL_ID + } + fn auth_manager(&self) -> Option> { None } @@ -140,4 +145,16 @@ mod tests { } ); } + + #[test] + fn approval_review_preferred_model_uses_bedrock_gpt_5_4() { + let provider = AmazonBedrockModelProvider::new( + ModelProviderInfo::create_amazon_bedrock_provider(/*aws*/ None), + ); + + assert_eq!( + provider.approval_review_preferred_model(), + AMAZON_BEDROCK_GPT_5_4_MODEL_ID + ); + } } diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index 8e1d37b29d..1ef7c22962 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -70,6 +70,10 @@ impl std::error::Error for ProviderAccountError {} pub type ProviderAccountResult = std::result::Result; +/// Default model used for automatic approval review when a provider does not +/// require a backend-specific model ID. +pub const DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL: &str = "codex-auto-review"; + /// Runtime provider abstraction used by model execution. /// /// Implementations own provider-specific behavior for a model backend. The @@ -85,6 +89,13 @@ pub trait ModelProvider: fmt::Debug + Send + Sync { ProviderCapabilities::default() } + /// Returns the preferred model used for automatic approval review. + /// + /// Providers that require backend-specific model IDs should override this. + fn approval_review_preferred_model(&self) -> &'static str { + DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL + } + /// Returns whether requests made through this provider should include attestation. fn supports_attestation(&self) -> bool { false @@ -350,6 +361,19 @@ mod tests { assert_eq!(provider.capabilities(), ProviderCapabilities::default()); } + #[test] + fn configured_provider_uses_default_approval_review_preferred_model() { + let provider = create_model_provider( + ModelProviderInfo::create_openai_provider(/*base_url*/ None), + /*auth_manager*/ None, + ); + + assert_eq!( + provider.approval_review_preferred_model(), + DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL + ); + } + #[tokio::test] async fn configured_provider_runtime_base_url_uses_configured_base_url() { let provider = create_model_provider(