diff --git a/codex-rs/app-server/src/config_manager_service_tests.rs b/codex-rs/app-server/src/config_manager_service_tests.rs index 67add1c33f..4775cb9c85 100644 --- a/codex-rs/app-server/src/config_manager_service_tests.rs +++ b/codex-rs/app-server/src/config_manager_service_tests.rs @@ -469,6 +469,9 @@ async fn invalid_user_value_written_if_overridden_by_managed() { CloudRequirementsLoader::default(), ); + // Write validation checks the effective merged config. The invalid user + // value is allowed here because the managed layer still supplies the + // effective approval policy. let result = service .write_value(ConfigValueWriteParams { file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()), diff --git a/codex-rs/config/src/lenient.rs b/codex-rs/config/src/lenient.rs index 45256ec3c4..ab06304797 100644 --- a/codex-rs/config/src/lenient.rs +++ b/codex-rs/config/src/lenient.rs @@ -1,6 +1,13 @@ use serde::de::DeserializeOwned; use toml::Value as TomlValue; +/// Deserializes TOML while turning invalid enum-valued settings into warnings. +/// +/// Serde reports enum failures as normal deserialization errors, which would +/// otherwise reject the whole assembled config. Config files are user-editable, +/// so this helper keeps the strongly typed destination type and handles enum +/// leniency at the load boundary: deserialize, find the first invalid enum +/// value, remove only that TOML key, record a startup warning, and retry. pub(crate) fn deserialize_with_enum_warnings( mut value: TomlValue, ) -> Result<(TomlValue, T, Vec), toml::de::Error> @@ -10,6 +17,9 @@ where let mut warnings = Vec::new(); loop { + // serde_path_to_error gives us the exact config path that failed, so + // we can remove the offending TOML value without making invalid enum + // state representable inside ConfigToml or its nested structs. match serde_path_to_error::deserialize(value.clone()) { Ok(parsed) => return Ok((value, parsed, warnings)), Err(err) => { @@ -30,10 +40,20 @@ where } } +/// Detects Serde's enum-variant failures without swallowing unrelated errors. +/// +/// toml::de::Error does not expose a structured enum-kind discriminator, so the +/// loader keeps the match deliberately narrow and lets all other parse and type +/// errors flow through the existing config-error path. fn is_unknown_variant_error(err: &toml::de::Error) -> bool { err.message().contains("unknown variant") } +/// Removes the failed TOML key identified by serde_path_to_error. +/// +/// Config enum fields live in tables rather than arrays, so the path traversal +/// intentionally follows table keys only. If the path cannot be removed, the +/// caller falls back to returning the original deserialization error. fn remove_value_at_path(value: &mut TomlValue, path: &str) -> Option { let mut parts = path.split('.').peekable(); let mut current = value; diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index e313757159..7a9a08c7b4 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -213,6 +213,12 @@ impl ConfigLayerStack { self } + /// 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. pub fn with_additional_startup_warnings(mut self, warnings: Vec) -> Self { if warnings.is_empty() { return self; @@ -311,6 +317,13 @@ impl ConfigLayerStack { merged } + /// 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. + /// 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. pub fn deserialize_effective_config_with_warnings( &self, ) -> Result<(TomlValue, T, Vec), toml::de::Error> diff --git a/codex-rs/core/src/config/config_loader_tests.rs b/codex-rs/core/src/config/config_loader_tests.rs index 413fe0a081..3301b832ca 100644 --- a/codex-rs/core/src/config/config_loader_tests.rs +++ b/codex-rs/core/src/config/config_loader_tests.rs @@ -143,6 +143,8 @@ notification_method = "loudly" ) .await?; + // The invalid enum leaves are removed before ConfigToml is produced, but + // unrelated valid settings still survive in the effective config. let (effective_config, _config_toml, enum_warnings): (TomlValue, ConfigToml, Vec) = layers.deserialize_effective_config_with_warnings()?; let expected_config = toml::from_str::( diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 524da299c6..5b00d34da0 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -967,6 +967,10 @@ impl ConfigBuilder { // relative paths to absolute paths based on the parent folder of the // respective config file, so we should be safe to deserialize without // AbsolutePathBufGuard here. + // + // Invalid enum-valued settings are dropped during deserialization and + // returned as warnings. This keeps ConfigToml strict and prevents raw + // invalid values from leaking into downstream config consumers. let (_merged_toml, config_toml, enum_warnings) = match config_layer_stack .deserialize_effective_config_with_warnings::() { @@ -1227,6 +1231,9 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides( .await?; let _guard = AbsolutePathBufGuard::new(codex_home); + // This helper returns ConfigToml directly, so enum warnings are intentionally + // not surfaced here. The full ConfigBuilder path is responsible for + // attaching them to startup warnings. let (_merged_toml, cfg, _enum_warnings) = config_layer_stack .deserialize_effective_config_with_warnings::() .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))