From 951be1a8a1331110dc6e5a9d8071dbd045e859d0 Mon Sep 17 00:00:00 2001 From: xl-openai Date: Wed, 22 Apr 2026 22:50:44 -0700 Subject: [PATCH] feat: Warn and continue on unknown feature requirements (#19038) Requirements feature flags now fail open like config feature flags, but with a startup warning. image --- codex-rs/core/src/config/config_tests.rs | 52 +++++++++++++--- codex-rs/core/src/config/managed_features.rs | 65 ++++++++++++++++---- codex-rs/core/src/config/mod.rs | 6 +- 3 files changed, 101 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 5450c5266..820cd7e8b 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -7070,10 +7070,10 @@ async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io:: } #[tokio::test] -async fn feature_requirements_reject_collab_legacy_alias() { - let codex_home = TempDir::new().expect("tempdir"); +async fn feature_requirements_warn_on_collab_legacy_alias() -> std::io::Result<()> { + let codex_home = TempDir::new()?; - let err = ConfigBuilder::default() + let config = ConfigBuilder::without_managed_config_for_tests() .codex_home(codex_home.path().to_path_buf()) .cloud_requirements(CloudRequirementsLoader::new(async { Ok(Some(crate::config_loader::ConfigRequirementsToml { @@ -7084,15 +7084,49 @@ async fn feature_requirements_reject_collab_legacy_alias() { })) })) .build() - .await - .expect_err("legacy aliases should be rejected"); + .await?; - assert_eq!(err.kind(), std::io::ErrorKind::InvalidData); + assert!(config.features.enabled(Feature::Collab)); assert!( - err.to_string() - .contains("use canonical feature key `multi_agent`"), - "{err}" + config.startup_warnings.iter().any(|warning| { + warning.contains("Using legacy `features` requirement `collab`") + && warning.contains("prefer canonical feature key `multi_agent`") + }), + "{:?}", + config.startup_warnings ); + + Ok(()) +} + +#[tokio::test] +async fn feature_requirements_warn_and_ignore_unknown_feature() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + + let config = ConfigBuilder::without_managed_config_for_tests() + .codex_home(codex_home.path().to_path_buf()) + .cloud_requirements(CloudRequirementsLoader::new(async { + Ok(Some(crate::config_loader::ConfigRequirementsToml { + feature_requirements: Some(crate::config_loader::FeatureRequirementsToml { + entries: BTreeMap::from([("made_up_feature".to_string(), true)]), + }), + ..Default::default() + })) + })) + .build() + .await?; + + assert!( + config + .startup_warnings + .iter() + .any(|warning| warning + .contains("Ignoring unknown `features` requirement `made_up_feature`")), + "{:?}", + config.startup_warnings + ); + + Ok(()) } #[tokio::test] diff --git a/codex-rs/core/src/config/managed_features.rs b/codex-rs/core/src/config/managed_features.rs index 1265d5e71..2ebd4749a 100644 --- a/codex-rs/core/src/config/managed_features.rs +++ b/codex-rs/core/src/config/managed_features.rs @@ -31,13 +31,37 @@ impl ManagedFeatures { pub(crate) fn from_configured( configured_features: Features, feature_requirements: Option>, + ) -> std::io::Result { + Self::from_configured_with_optional_warnings( + configured_features, + feature_requirements, + /*startup_warnings*/ None, + ) + } + + pub(crate) fn from_configured_with_warnings( + configured_features: Features, + feature_requirements: Option>, + startup_warnings: &mut Vec, + ) -> std::io::Result { + Self::from_configured_with_optional_warnings( + configured_features, + feature_requirements, + Some(startup_warnings), + ) + } + + fn from_configured_with_optional_warnings( + configured_features: Features, + feature_requirements: Option>, + startup_warnings: Option<&mut Vec>, ) -> std::io::Result { let (pinned_features, source) = match feature_requirements { Some(Sourced { value: feature_requirements, source, }) => ( - parse_feature_requirements(feature_requirements, &source)?, + parse_feature_requirements(feature_requirements, &source, startup_warnings), Some(source), ), None => (BTreeMap::new(), None), @@ -171,7 +195,8 @@ fn feature_requirements_display(feature_requirements: &BTreeMap) fn parse_feature_requirements( feature_requirements: FeatureRequirementsToml, source: &RequirementSource, -) -> std::io::Result> { + mut startup_warnings: Option<&mut Vec>, +) -> BTreeMap { let mut pinned_features = BTreeMap::new(); for (key, enabled) in feature_requirements.entries { if let Some(feature) = canonical_feature_for_key(&key) { @@ -180,22 +205,34 @@ fn parse_feature_requirements( } if let Some(feature) = feature_for_key(&key) { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, + push_feature_requirement_warning( + &mut startup_warnings, format!( - "invalid `features` requirement `{key}` from {source}: use canonical feature key `{}`", + "Using legacy `features` requirement `{key}` from {source}; prefer canonical feature key `{}`", feature.key() ), - )); + ); + pinned_features.insert(feature, enabled); + continue; } - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!("invalid `features` requirement `{key}` from {source}"), - )); + push_feature_requirement_warning( + &mut startup_warnings, + format!("Ignoring unknown `features` requirement `{key}` from {source}"), + ); } - Ok(pinned_features) + pinned_features +} + +fn push_feature_requirement_warning( + startup_warnings: &mut Option<&mut Vec>, + message: String, +) { + tracing::warn!("{message}"); + if let Some(startup_warnings) = startup_warnings.as_deref_mut() { + startup_warnings.push(message); + } } fn explicit_feature_settings_in_config(cfg: &ConfigToml) -> Vec<(String, Feature, bool)> { @@ -272,7 +309,11 @@ pub(crate) fn validate_explicit_feature_settings_in_config_toml( return Ok(()); }; - let pinned_features = parse_feature_requirements(feature_requirements.clone(), source)?; + let pinned_features = parse_feature_requirements( + feature_requirements.clone(), + source, + /*startup_warnings*/ None, + ); if pinned_features.is_empty() { return Ok(()); } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 8d60307ec..5f77a6c5e 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1666,7 +1666,11 @@ impl Config { }, feature_overrides, ); - let features = ManagedFeatures::from_configured(configured_features, feature_requirements)?; + let features = ManagedFeatures::from_configured_with_warnings( + configured_features, + feature_requirements, + &mut startup_warnings, + )?; let windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg, &config_profile); let windows_sandbox_private_desktop = resolve_windows_sandbox_private_desktop(&cfg, &config_profile);