Make enum config warnings advisory

codex-rs/config/src/lenient.rs

@@ -1,161 +1,141 @@
-use crate::config_toml::ConfigToml;
+use crate::schema::config_schema;
 use serde_json::Map as JsonMap;
 use serde_json::Value as JsonValue;
-use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::panic::AssertUnwindSafe;
 use std::panic::catch_unwind;
-use std::sync::OnceLock;
 use toml::Value as TomlValue;
 
-static ENUM_FIELD_SPECS: OnceLock<Vec<EnumFieldSpec>> = OnceLock::new();
-
-#[derive(Clone, Debug)]
-struct EnumFieldSpec {
-    path: Vec<PathSegment>,
-    allowed_values: BTreeSet<String>,
-    allows_non_string: bool,
-}
-
-/// One segment in a known enum-valued config path discovered from JSON Schema.
-#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
-enum PathSegment {
-    /// Match this literal table key.
-    Key(String),
-    /// Expand across every table child at this point, such as `profiles.*`.
-    AnyTable,
-}
-
 #[derive(Default)]
-struct ScalarChoices {
+struct EnumChoices {
     allowed_strings: BTreeSet<String>,
     allows_non_string: bool,
 }
 
-/// Deserializes config while turning known invalid enum-valued settings into warnings.
+enum PathElement {
+    Key(String),
+    Index(usize),
+}
+
+/// Return best-effort startup warnings for raw TOML values that look like invalid enums.
 ///
-/// The config structs use `serde_with::DefaultOnError` on their enum fields so
-/// an invalid field can default instead of rejecting the whole file. This helper
-/// uses the same generated JSON Schema that backs `config.schema.json` to find
-/// string enum-valued leaves. It then does a best-effort pass over the raw TOML:
-/// invalid enum leaves are deleted from the sanitized TOML and reported as
-/// startup warnings, while full shape/type validation stays with Serde. If the
-/// warning pass itself cannot run, config loading continues with the original
-/// TOML and no warnings.
-pub(crate) fn deserialize_with_enum_warnings(
-    value: TomlValue,
-) -> Result<(TomlValue, ConfigToml, Vec<String>), toml::de::Error> {
-    let (sanitized, warnings) = sanitize_for_enum_warnings(value);
-    let parsed = sanitized.clone().try_into::<ConfigToml>()?;
-    Ok((sanitized, parsed, warnings))
-}
-
-fn sanitize_for_enum_warnings(value: TomlValue) -> (TomlValue, Vec<String>) {
-    let original = value.clone();
+/// `DefaultOnError` is responsible for keeping config deserialization lenient.
+/// This pass is intentionally advisory: it walks the final merged TOML value
+/// against the generated config schema and reports enum-looking mismatches
+/// without changing the TOML that will be deserialized.
+pub(crate) fn enum_value_warnings(value: &TomlValue) -> Vec<String> {
     catch_unwind(AssertUnwindSafe(|| {
-        let mut sanitized = value;
+        let Ok(schema) = serde_json::to_value(config_schema()) else {
+            return Vec::new();
+        };
+        let definitions = schema.get("definitions").and_then(JsonValue::as_object);
+        let mut path = Vec::new();
+        let mut ref_stack = Vec::new();
         let mut warnings = Vec::new();
-        for spec in enum_field_specs() {
-            for path in matching_paths(&sanitized, &spec.path) {
-                remove_if_invalid(&mut sanitized, &path, spec, &mut warnings);
-            }
-        }
-        (sanitized, warnings)
+
+        collect_warnings(
+            value,
+            &schema,
+            definitions,
+            &mut path,
+            &mut ref_stack,
+            &mut warnings,
+        );
+
+        warnings
     }))
-    .unwrap_or((original, Vec::new()))
+    .unwrap_or_default()
 }
 
-fn enum_field_specs() -> &'static [EnumFieldSpec] {
-    ENUM_FIELD_SPECS.get_or_init(|| catch_unwind(build_enum_field_specs).unwrap_or_default())
-}
-
-fn build_enum_field_specs() -> Vec<EnumFieldSpec> {
-    let Ok(schema) = serde_json::to_value(crate::schema::config_schema()) else {
-        return Vec::new();
-    };
-    let definitions = schema.get("definitions").and_then(JsonValue::as_object);
-    let mut choices_by_path = BTreeMap::<Vec<PathSegment>, ScalarChoices>::new();
-    let mut path = Vec::new();
-    let mut ref_stack = Vec::new();
-
-    collect_enum_fields(
-        &schema,
-        definitions,
-        &mut path,
-        &mut choices_by_path,
-        &mut ref_stack,
-    );
-
-    choices_by_path
-        .into_iter()
-        .filter_map(|(path, choices)| {
-            if choices.allowed_strings.is_empty() {
-                return None;
-            }
-            Some(EnumFieldSpec {
-                path,
-                allowed_values: choices.allowed_strings,
-                allows_non_string: choices.allows_non_string,
-            })
-        })
-        .collect()
-}
-
-fn collect_enum_fields(
+fn collect_warnings(
+    value: &TomlValue,
     schema: &JsonValue,
     definitions: Option<&JsonMap<String, JsonValue>>,
-    path: &mut Vec<PathSegment>,
-    choices_by_path: &mut BTreeMap<Vec<PathSegment>, ScalarChoices>,
+    path: &mut Vec<PathElement>,
     ref_stack: &mut Vec<String>,
+    warnings: &mut Vec<String>,
 ) {
-    let mut choices = ScalarChoices::default();
-    collect_scalar_choices(schema, definitions, &mut choices, ref_stack);
-    if !choices.allowed_strings.is_empty() {
-        choices_by_path
-            .entry(path.clone())
-            .or_default()
-            .merge(choices);
+    let mut choices = EnumChoices::default();
+    collect_enum_choices(schema, definitions, ref_stack, &mut choices);
+    let invalid_enum_value = if let Some(value) = value.as_str() {
+        !choices.allowed_strings.contains(value)
+    } else {
+        !choices.allows_non_string
+    };
+    if !choices.allowed_strings.is_empty() && invalid_enum_value {
+        let warning = format!(
+            "Ignoring invalid config value at {}: {value}",
+            display_path(path)
+        );
+        if !warnings.contains(&warning) {
+            warnings.push(warning);
+        }
     }
 
     if let Some(definition) = resolve_definition(schema, definitions, ref_stack) {
-        collect_enum_fields(definition, definitions, path, choices_by_path, ref_stack);
+        collect_warnings(value, definition, definitions, path, ref_stack, warnings);
         ref_stack.pop();
     }
 
-    for child in schema_composition_children(schema) {
-        collect_enum_fields(child, definitions, path, choices_by_path, ref_stack);
+    for child_schema in schema_composition_children(schema) {
+        collect_warnings(value, child_schema, definitions, path, ref_stack, warnings);
     }
 
-    if let Some(properties) = schema.get("properties").and_then(JsonValue::as_object) {
-        for (key, child) in properties {
-            path.push(PathSegment::Key(key.clone()));
-            collect_enum_fields(child, definitions, path, choices_by_path, ref_stack);
+    let properties = schema.get("properties").and_then(JsonValue::as_object);
+    if let (Some(table), Some(properties)) = (value.as_table(), properties) {
+        for (key, child_schema) in properties {
+            let Some(child_value) = table.get(key) else {
+                continue;
+            };
+            path.push(PathElement::Key(key.clone()));
+            collect_warnings(
+                child_value,
+                child_schema,
+                definitions,
+                path,
+                ref_stack,
+                warnings,
+            );
             path.pop();
         }
     }
 
-    let Some(additional_properties) = schema.get("additionalProperties") else {
-        return;
-    };
-    if !additional_properties.is_object() {
-        return;
+    let additional_properties = schema
+        .get("additionalProperties")
+        .filter(|additional_properties| additional_properties.is_object());
+    if let (Some(table), Some(additional_properties)) = (value.as_table(), additional_properties) {
+        for (key, child_value) in table {
+            if properties.is_some_and(|properties| properties.contains_key(key)) {
+                continue;
+            }
+            path.push(PathElement::Key(key.clone()));
+            collect_warnings(
+                child_value,
+                additional_properties,
+                definitions,
+                path,
+                ref_stack,
+                warnings,
+            );
+            path.pop();
+        }
+    }
+
+    let items = schema.get("items");
+    if let (Some(array), Some(items)) = (value.as_array(), items) {
+        for (index, child_value) in array.iter().enumerate() {
+            path.push(PathElement::Index(index));
+            collect_warnings(child_value, items, definitions, path, ref_stack, warnings);
+            path.pop();
+        }
     }
-    path.push(PathSegment::AnyTable);
-    collect_enum_fields(
-        additional_properties,
-        definitions,
-        path,
-        choices_by_path,
-        ref_stack,
-    );
-    path.pop();
 }
 
-fn collect_scalar_choices(
+fn collect_enum_choices(
     schema: &JsonValue,
     definitions: Option<&JsonMap<String, JsonValue>>,
-    choices: &mut ScalarChoices,
     ref_stack: &mut Vec<String>,
+    choices: &mut EnumChoices,
 ) {
     choices
         .allowed_strings
@@ -163,12 +143,12 @@ fn collect_scalar_choices(
     choices.allows_non_string |= schema_allows_non_string(schema);
 
     if let Some(definition) = resolve_definition(schema, definitions, ref_stack) {
-        collect_scalar_choices(definition, definitions, choices, ref_stack);
+        collect_enum_choices(definition, definitions, ref_stack, choices);
         ref_stack.pop();
     }
 
-    for child in schema_composition_children(schema) {
-        collect_scalar_choices(child, definitions, choices, ref_stack);
+    for child_schema in schema_composition_children(schema) {
+        collect_enum_choices(child_schema, definitions, ref_stack, choices);
     }
 }
 
@@ -220,87 +200,26 @@ fn schema_composition_children(schema: &JsonValue) -> impl Iterator<Item = &Json
         .flatten()
 }
 
-/// Removes one known enum leaf when its raw TOML value does not match schema choices.
-fn remove_if_invalid(
-    value: &mut TomlValue,
-    path: &[String],
-    spec: &EnumFieldSpec,
-    warnings: &mut Vec<String>,
-) {
-    let Some(raw_value) = value_at_path(value, path) else {
-        return;
-    };
-    if let Some(raw_value) = raw_value.as_str() {
-        if spec.allowed_values.contains(raw_value) {
-            return;
-        }
-    } else if spec.allows_non_string {
-        return;
+fn display_path(path: &[PathElement]) -> String {
+    if path.is_empty() {
+        return "<root>".to_string();
     }
-    let Some(invalid_value) = remove_value_at_path(value, path) else {
-        return;
-    };
-    warnings.push(format!(
-        "Ignoring invalid config value at {}: {invalid_value}",
-        display_path(path)
-    ));
-}
 
-/// Expands a spec path into concrete TOML paths without holding borrows across mutation.
-fn matching_paths(value: &TomlValue, spec_path: &[PathSegment]) -> Vec<Vec<String>> {
-    let mut paths = vec![Vec::new()];
-    for segment in spec_path {
-        match segment {
-            PathSegment::Key(key) => {
-                for path in &mut paths {
-                    path.push(key.clone());
+    let mut output = String::new();
+    for element in path {
+        match element {
+            PathElement::Key(key) => {
+                if !output.is_empty() {
+                    output.push('.');
                 }
+                output.push_str(&display_key(key));
             }
-            PathSegment::AnyTable => {
-                let mut expanded = Vec::new();
-                for path in paths {
-                    let Some(table) = value_at_path(value, &path).and_then(TomlValue::as_table)
-                    else {
-                        continue;
-                    };
-                    expanded.extend(table.iter().filter_map(|(key, value)| {
-                        if !value.is_table() {
-                            return None;
-                        }
-                        let mut child = path.clone();
-                        child.push(key.clone());
-                        Some(child)
-                    }));
-                }
-                paths = expanded;
+            PathElement::Index(index) => {
+                output.push_str(&format!("[{index}]"));
             }
         }
     }
-    paths
-}
-
-fn value_at_path<'a>(value: &'a TomlValue, path: &[String]) -> Option<&'a TomlValue> {
-    let mut current = value;
-    for key in path {
-        current = current.as_table()?.get(key)?;
-    }
-    Some(current)
-}
-
-fn remove_value_at_path(value: &mut TomlValue, path: &[String]) -> Option<TomlValue> {
-    let (last, parents) = path.split_last()?;
-    let mut current = value;
-    for key in parents {
-        current = current.as_table_mut()?.get_mut(key)?;
-    }
-    current.as_table_mut()?.remove(last)
-}
-
-fn display_path(path: &[String]) -> String {
-    path.iter()
-        .map(|key| display_key(key))
-        .collect::<Vec<_>>()
-        .join(".")
+    output
 }
 
 fn display_key(key: &str) -> String {
@@ -314,10 +233,3 @@ fn display_key(key: &str) -> String {
     let escaped = key.replace('\\', "\\\\").replace('"', "\\\"");
     format!("\"{escaped}\"")
 }
-
-impl ScalarChoices {
-    fn merge(&mut self, other: ScalarChoices) {
-        self.allowed_strings.extend(other.allowed_strings);
-        self.allows_non_string |= other.allows_non_string;
-    }
-}

codex-rs/config/src/loader/mod.rs

@@ -15,7 +15,6 @@ use crate::diagnostics::ConfigError;
 use crate::diagnostics::config_error_from_toml;
 use crate::diagnostics::first_layer_config_error_from_entries as typed_first_layer_config_error_from_entries;
 use crate::diagnostics::io_error_from_config_error;
-use crate::lenient::deserialize_with_enum_warnings;
 use crate::merge::merge_toml_values;
 use crate::overrides::build_cli_overrides_layer;
 use crate::project_root_markers::default_project_root_markers;
@@ -870,15 +869,12 @@ pub fn resolve_relative_paths_in_config_toml(
     value_from_config_toml: TomlValue,
     base_dir: &Path,
 ) -> io::Result<TomlValue> {
-    // Use the serialize/deserialize round-trip to convert the
-    // `toml::Value` into a `ConfigToml` with `AbsolutePathBuf` fields resolved
-    // by the guard. Invalid enum values are ignored only for this typed
-    // projection; copy_shape_from_original preserves them in the raw layer so
-    // the final config load can still surface the startup warning.
+    // Use the serialize/deserialize round-trip to convert the `toml::Value`
+    // into a `ConfigToml` with `AbsolutePathBuf` fields resolved by the guard.
+    // copy_shape_from_original preserves raw non-path leaves so invalid enum
+    // values can still be reported from the final merged TOML.
     let _guard = AbsolutePathBufGuard::new(base_dir);
-    let Ok((_sanitized, resolved, _enum_warnings)) =
-        deserialize_with_enum_warnings(value_from_config_toml.clone())
-    else {
+    let Ok(resolved) = value_from_config_toml.clone().try_into::<ConfigToml>() else {
         return Ok(value_from_config_toml);
     };
     drop(_guard);
@@ -897,9 +893,9 @@ pub fn resolve_relative_paths_in_config_toml(
 }
 
 /// Ensure that every field in `original` is present in the returned
-/// `toml::Value`, taking the value from `resolved` where possible. This ensures
-/// the fields that we "removed" during the serialize/deserialize round-trip in
-/// `resolve_config_paths` are preserved, out of an abundance of caution.
+/// `toml::Value`, copying only the absolute path strings produced by the typed
+/// round-trip. Other leaves stay raw so lenient enum defaults do not overwrite
+/// the value that should produce a startup warning.
 fn copy_shape_from_original(original: &TomlValue, resolved: &TomlValue) -> TomlValue {
     match (original, resolved) {
         (TomlValue::Table(original_table), TomlValue::Table(resolved_table)) => {
@@ -921,7 +917,14 @@ fn copy_shape_from_original(original: &TomlValue, resolved: &TomlValue) -> TomlV
             }
             TomlValue::Array(items)
         }
-        (_, resolved_value) => resolved_value.clone(),
+        (TomlValue::String(original), TomlValue::String(resolved)) => {
+            if original != resolved && Path::new(resolved).is_absolute() {
+                TomlValue::String(resolved.clone())
+            } else {
+                TomlValue::String(original.clone())
+            }
+        }
+        (original_value, _) => original_value.clone(),
     }
 }
 

codex-rs/config/src/state.rs

@@ -4,7 +4,7 @@ use crate::config_requirements::ConfigRequirementsToml;
 use super::fingerprint::record_origins;
 use super::fingerprint::version_for_toml;
 use super::key_aliases::normalized_with_key_aliases;
-use super::lenient::deserialize_with_enum_warnings;
+use super::lenient::enum_value_warnings;
 use super::merge::merge_toml_values;
 use crate::config_toml::ConfigToml;
 use codex_app_server_protocol::ConfigLayer;
@@ -320,32 +320,16 @@ impl ConfigLayerStack {
 
     /// Deserializes the merged config-layer view and returns any soft warnings.
     ///
-    /// This is the boundary where user-editable config stays lenient for
-    /// invalid enum values while consumers still receive a fully typed config.
-    /// Each layer is sanitized before it is merged so a higher-precedence
-    /// invalid enum value falls back to a valid lower-precedence value.
+    /// Invalid enum-valued settings are handled by `DefaultOnError` during
+    /// deserialization. Warnings are collected from the final raw TOML view so
+    /// warning discovery remains advisory and cannot change config semantics.
     pub fn deserialize_effective_config_with_warnings(
         &self,
     ) -> Result<(TomlValue, ConfigToml, Vec<String>), toml::de::Error> {
-        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))
+        let merged = self.effective_config();
+        let warnings = enum_value_warnings(&merged);
+        let typed = merged.clone().try_into::<ConfigToml>()?;
+        Ok((merged, typed, warnings))
     }
 
     /// Returns field origins for the merged config-layer view.

codex-rs/core/src/config/config_loader_tests.rs

@@ -25,7 +25,6 @@ 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;
@@ -148,18 +147,21 @@ context_size = "galactic"
     )
     .await?;
 
-    // The invalid enum leaves are removed before ConfigToml is produced, but
-    // unrelated valid settings still survive in the effective config.
+    // The warning pass reads the final TOML value without mutating it, while
+    // DefaultOnError lets unrelated valid settings still deserialize.
     let (effective_config, _config_toml, enum_warnings) =
         layers.deserialize_effective_config_with_warnings()?;
     let expected_config = toml::from_str::<TomlValue>(
         r#"
 model = "gpt-5-codex"
+sandbox_mode = "make-it-so"
 
 [tui]
+notification_method = "loudly"
 theme = "loudly"
 
 [tools.web_search]
+context_size = "galactic"
 "#,
     )
     .expect("expected config should parse");
@@ -177,7 +179,7 @@ theme = "loudly"
 }
 
 #[tokio::test]
-async fn invalid_higher_precedence_enum_falls_back_to_lower_layer() -> anyhow::Result<()> {
+async fn invalid_higher_precedence_enum_warns_without_mutating_merge() -> 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""#)
@@ -213,8 +215,8 @@ async fn invalid_higher_precedence_enum_falls_back_to_lower_layer() -> anyhow::R
             enum_warnings,
         ),
         (
-            Some(&TomlValue::String("workspace-write".to_string())),
-            Some(SandboxMode::WorkspaceWrite),
+            Some(&TomlValue::String("make-it-so".to_string())),
+            None,
             vec!["Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string()],
         )
     );
@@ -325,6 +327,7 @@ sandbox_mode = "hold-my-coffee"
             r#"
 [profiles."alpha.beta"]
 model = "gpt-5-codex"
+sandbox_mode = "hold-my-coffee"
 "#,
         )
         .expect("expected config should parse"),

codex-rs/core/src/config/mod.rs

@@ -976,9 +976,8 @@ impl ConfigBuilder {
         // respective config file, so we should be safe to deserialize without
         // AbsolutePathBufGuard here.
         //
-        // Invalid enum-valued settings default at the typed field boundary and
-        // are removed from the merged TOML view when the best-effort scan can
-        // identify the offending leaf.
+        // Invalid enum-valued settings default at the typed field boundary.
+        // The warning scan reads the final merged TOML view without mutating it.
         let (_merged_toml, config_toml, enum_warnings) = match config_layer_stack
             .deserialize_effective_config_with_warnings()
         {