diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0ddbf426e..572093d84 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -961,6 +961,14 @@ impl Session { }), }); } + for message in &config.startup_warnings { + post_session_configured_events.push(Event { + id: "".to_owned(), + msg: EventMsg::Warning(WarningEvent { + message: message.clone(), + }), + }); + } maybe_push_unstable_features_warning(&config, &mut post_session_configured_events); let auth = auth.as_ref(); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d9ddc5944..7de6f00ea 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -21,6 +21,7 @@ use crate::config::types::UriBasedFileOpener; use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; +use crate::config_loader::ConstrainedWithSource; use crate::config_loader::LoaderOverrides; use crate::config_loader::McpServerIdentity; use crate::config_loader::McpServerRequirement; @@ -116,6 +117,9 @@ pub struct Config { /// requirements). pub config_layer_stack: ConfigLayerStack, + /// Warnings collected during config load that should be shown on startup. + pub startup_warnings: Vec, + /// Optional override of model selection. pub model: Option, @@ -601,6 +605,41 @@ fn constrain_mcp_servers( }) } +fn apply_requirement_constrained_value( + field_name: &'static str, + configured_value: T, + constrained_value: &mut ConstrainedWithSource, + startup_warnings: &mut Vec, +) -> std::io::Result<()> +where + T: Clone + std::fmt::Debug + Send + Sync, +{ + if let Err(err) = constrained_value.set(configured_value) { + let fallback_value = constrained_value.get().clone(); + tracing::warn!( + error = %err, + ?fallback_value, + requirement_source = ?constrained_value.source, + "configured value is disallowed by requirements; falling back to required value for {field_name}" + ); + let message = format!( + "Configured value for `{field_name}` is disallowed by requirements; falling back to required value {fallback_value:?}. Details: {err}" + ); + startup_warnings.push(message); + + constrained_value.set(fallback_value).map_err(|fallback_err| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "configured value for `{field_name}` is disallowed by requirements ({err}); fallback to a requirement-compliant value also failed ({fallback_err})" + ), + ) + })?; + } + + Ok(()) +} + fn mcp_server_matches_requirement( requirement: &McpServerRequirement, server: &McpServerConfig, @@ -1320,6 +1359,7 @@ impl Config { ) -> std::io::Result { let requirements = config_layer_stack.requirements().clone(); let user_instructions = Self::load_instructions(Some(&codex_home)); + let mut startup_warnings = Vec::new(); // Destructure ConfigOverrides fully to ensure all overrides are applied. let ConfigOverrides { @@ -1586,12 +1626,18 @@ impl Config { enforce_residency, } = requirements; - constrained_approval_policy - .set(approval_policy) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; - constrained_sandbox_policy - .set(sandbox_policy) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; + apply_requirement_constrained_value( + "approval_policy", + approval_policy, + &mut constrained_approval_policy, + &mut startup_warnings, + )?; + apply_requirement_constrained_value( + "sandbox_mode", + sandbox_policy, + &mut constrained_sandbox_policy, + &mut startup_warnings, + )?; let mcp_servers = constrain_mcp_servers(cfg.mcp_servers.clone(), mcp_servers.as_ref()) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; @@ -1604,6 +1650,7 @@ impl Config { model_provider_id, model_provider, cwd: resolved_cwd, + startup_warnings, approval_policy: constrained_approval_policy.value, sandbox_policy: constrained_sandbox_policy.value, enforce_residency: enforce_residency.value, @@ -3853,6 +3900,7 @@ model_verbosity = "high" codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), config_layer_stack: Default::default(), + startup_warnings: Vec::new(), history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, @@ -3940,6 +3988,7 @@ model_verbosity = "high" codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), config_layer_stack: Default::default(), + startup_warnings: Vec::new(), history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, @@ -4042,6 +4091,7 @@ model_verbosity = "high" codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), config_layer_stack: Default::default(), + startup_warnings: Vec::new(), history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, @@ -4130,6 +4180,7 @@ model_verbosity = "high" codex_home: fixture.codex_home(), log_dir: fixture.codex_home().join("log"), config_layer_stack: Default::default(), + startup_warnings: Vec::new(), history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, @@ -4681,7 +4732,7 @@ mcp_oauth_callback_port = 5678 } #[tokio::test] - async fn explicit_sandbox_mode_still_errors_when_disallowed_by_requirements() + async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> std::io::Result<()> { let codex_home = TempDir::new()?; std::fs::write( @@ -4700,19 +4751,15 @@ mcp_oauth_callback_port = 5678 enforce_residency: None, }; - let err = ConfigBuilder::default() + let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .fallback_cwd(Some(codex_home.path().to_path_buf())) .cloud_requirements(CloudRequirementsLoader::new( async move { Some(requirements) }, )) .build() - .await - .expect_err("explicit disallowed mode should still fail"); - assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); - let message = err.to_string(); - assert!(message.contains("invalid value for `sandbox_mode`")); - assert!(message.contains("set by cloud requirements")); + .await?; + assert_eq!(*config.sandbox_policy.get(), SandboxPolicy::ReadOnly); Ok(()) } @@ -4749,7 +4796,7 @@ trust_level = "untrusted" } #[tokio::test] - async fn explicit_approval_policy_still_errors_when_disallowed_by_requirements() + async fn explicit_approval_policy_falls_back_when_disallowed_by_requirements() -> std::io::Result<()> { let codex_home = TempDir::new()?; std::fs::write( @@ -4758,7 +4805,7 @@ trust_level = "untrusted" "#, )?; - let err = ConfigBuilder::default() + let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .fallback_cwd(Some(codex_home.path().to_path_buf())) .cloud_requirements(CloudRequirementsLoader::new(async { @@ -4768,12 +4815,8 @@ trust_level = "untrusted" }) })) .build() - .await - .expect_err("explicit disallowed approval policy should fail"); - assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); - let message = err.to_string(); - assert!(message.contains("invalid value for `approval_policy`")); - assert!(message.contains("set by cloud requirements")); + .await?; + assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest); Ok(()) } } diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/collab.rs index 3110204d2..e6f0c3c95 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/collab.rs @@ -1168,6 +1168,37 @@ mod tests { #[tokio::test] async fn build_agent_spawn_config_uses_turn_context_values() { + fn pick_allowed_approval_policy( + constraint: &crate::config::Constrained, + base: AskForApproval, + ) -> AskForApproval { + let candidates = [ + AskForApproval::Never, + AskForApproval::UnlessTrusted, + AskForApproval::OnRequest, + AskForApproval::OnFailure, + ]; + candidates + .into_iter() + .find(|candidate| *candidate != base && constraint.can_set(candidate).is_ok()) + .unwrap_or(base) + } + + fn pick_allowed_sandbox_policy( + constraint: &crate::config::Constrained, + base: SandboxPolicy, + ) -> SandboxPolicy { + let candidates = [ + SandboxPolicy::new_read_only_policy(), + SandboxPolicy::new_workspace_write_policy(), + SandboxPolicy::DangerFullAccess, + ]; + candidates + .into_iter() + .find(|candidate| *candidate != base && constraint.can_set(candidate).is_ok()) + .unwrap_or(base) + } + let (_session, mut turn) = make_session_and_context().await; let base_instructions = BaseInstructions { text: "base".to_string(), @@ -1181,8 +1212,14 @@ mod tests { let temp_dir = tempfile::tempdir().expect("temp dir"); turn.cwd = temp_dir.path().to_path_buf(); turn.codex_linux_sandbox_exe = Some(PathBuf::from("/bin/echo")); - turn.approval_policy = AskForApproval::Never; - turn.sandbox_policy = SandboxPolicy::DangerFullAccess; + turn.approval_policy = pick_allowed_approval_policy( + &turn.config.approval_policy, + *turn.config.approval_policy.get(), + ); + turn.sandbox_policy = pick_allowed_sandbox_policy( + &turn.config.sandbox_policy, + turn.config.sandbox_policy.get().clone(), + ); let config = build_agent_spawn_config(&base_instructions, &turn, 0).expect("spawn config"); let mut expected = (*turn.config).clone();