Preserve config layer fallback for invalid enums

This commit is contained in:
Ahmed Ibrahim
2026-05-07 00:33:05 +03:00
parent 646f9baf7e
commit 5684d7b5bd
2 changed files with 71 additions and 8 deletions
+25 -8
View File
@@ -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<String>) -> 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<T>(
&self,
) -> Result<(TomlValue, T, Vec<String>), 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::<T>(normalized_layer)?;
warnings.extend(layer_warnings);
merge_toml_values(&mut merged, &sanitized_layer);
}
let (sanitized_effective, typed_effective, effective_warnings) =
deserialize_with_enum_warnings::<T>(merged)?;
warnings.extend(effective_warnings);
Ok((sanitized_effective, typed_effective, warnings))
}
/// Returns field origins for the merged config-layer view.
@@ -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<String>) =
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");