diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index 7a9a08c7b4..371e862b70 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -215,10 +215,10 @@ impl ConfigLayerStack { /// Appends warnings discovered after the raw layers have been assembled. /// - /// Invalid enum warnings are produced while deserializing the effective - /// config, so they are not tied to any single raw layer load. Keeping this - /// merge point on the stack lets callers surface them through the same - /// startup-warning channel used by file-level config diagnostics. + /// Invalid enum warnings are produced while preparing the typed effective + /// config, after the stack has already been constructed. Keeping this merge + /// point on the stack lets callers surface them through the same startup + /// warning channel used by file-level config diagnostics. pub fn with_additional_startup_warnings(mut self, warnings: Vec) -> Self { if warnings.is_empty() { return self; @@ -321,16 +321,33 @@ impl ConfigLayerStack { /// /// This is the boundary where user-editable config stays lenient for /// invalid enum values while consumers still receive a fully typed config. - /// The returned TOML value is the sanitized effective config, with invalid - /// enum keys removed, so callers that inspect raw values see the same shape - /// that was actually deserialized. + /// Each layer is sanitized before it is merged so a higher-precedence + /// invalid enum value falls back to a valid lower-precedence value. pub fn deserialize_effective_config_with_warnings( &self, ) -> Result<(TomlValue, T, Vec), toml::de::Error> where T: serde::de::DeserializeOwned, { - deserialize_with_enum_warnings(self.effective_config()) + let mut merged = TomlValue::Table(toml::map::Map::new()); + let mut warnings = Vec::new(); + + for layer in self.get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ false, + ) { + let mut normalized_layer = TomlValue::Table(toml::map::Map::new()); + merge_toml_values(&mut normalized_layer, &layer.config); + let (sanitized_layer, _typed_layer, layer_warnings) = + deserialize_with_enum_warnings::(normalized_layer)?; + warnings.extend(layer_warnings); + merge_toml_values(&mut merged, &sanitized_layer); + } + + let (sanitized_effective, typed_effective, effective_warnings) = + deserialize_with_enum_warnings::(merged)?; + warnings.extend(effective_warnings); + Ok((sanitized_effective, typed_effective, warnings)) } /// Returns field origins for the merged config-layer view. diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 78649b18b8..07da2813ed 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -25,6 +25,7 @@ use codex_config::loader::load_config_layers_state; use codex_config::loader::load_requirements_toml; use codex_config::version_for_toml; use codex_exec_server::LOCAL_FS; +use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WebSearchMode; use codex_protocol::models::PermissionProfile; @@ -175,6 +176,51 @@ theme = "loudly" Ok(()) } +#[tokio::test] +async fn invalid_higher_precedence_enum_falls_back_to_lower_layer() -> anyhow::Result<()> { + let tmp = tempdir().expect("tempdir"); + let system_config_path = tmp.path().join("system_config.toml"); + std::fs::write(&system_config_path, r#"sandbox_mode = "workspace-write""#) + .expect("write system config"); + std::fs::write( + tmp.path().join(CONFIG_TOML_FILE), + r#"sandbox_mode = "make-it-so""#, + ) + .expect("write user config"); + + let cwd = AbsolutePathBuf::try_from(tmp.path()).expect("cwd"); + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + tmp.path(), + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides { + system_config_path: Some(system_config_path), + ..LoaderOverrides::without_managed_config_for_tests() + }, + CloudRequirementsLoader::default(), + &codex_config::NoopThreadConfigLoader, + ) + .await?; + + let (effective_config, config_toml, enum_warnings): (TomlValue, ConfigToml, Vec) = + layers.deserialize_effective_config_with_warnings()?; + + assert_eq!( + ( + effective_config.get("sandbox_mode"), + config_toml.sandbox_mode, + enum_warnings, + ), + ( + Some(&TomlValue::String("workspace-write".to_string())), + Some(SandboxMode::WorkspaceWrite), + vec!["Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string()], + ) + ); + Ok(()) +} + #[tokio::test] async fn invalid_enum_values_do_not_poison_relative_path_resolution() -> anyhow::Result<()> { let tmp = tempdir().expect("tempdir");