Sanitize config enums after merging layers

Defer invalid enum handling until after config layers are assembled. This keeps layer loading raw, removes invalid enum values from the final effective config, and reports warnings with the dotted field path and invalid value only.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-05-05 03:13:40 +03:00
co-authored by Codex
parent 5e1dbff17e
commit d6e2ff811b
5 changed files with 63 additions and 211 deletions
+28 -118
View File
@@ -39,126 +39,89 @@ enum Lenient<T> {
Failure,
}
pub(crate) fn sanitize_config_toml_enums(
value: &mut TomlValue,
source: impl Into<String>,
) -> Vec<String> {
let source = source.into();
pub(crate) fn sanitize_config_toml_enums(value: &mut TomlValue) -> Vec<String> {
let mut warnings = Vec::new();
let Some(table) = value.as_table_mut() else {
return warnings;
};
sanitize_config_table(table, &source, &mut warnings);
sanitize_config_table(table, &mut warnings);
warnings
}
fn sanitize_config_table(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
remove_invalid_enum::<AskForApproval>(
table,
"approval_policy",
"approval_policy",
source,
warnings,
);
fn sanitize_config_table(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
remove_invalid_enum::<AskForApproval>(table, "approval_policy", "approval_policy", warnings);
remove_invalid_enum::<ApprovalsReviewer>(
table,
"approvals_reviewer",
"approvals_reviewer",
source,
warnings,
);
remove_invalid_enum::<SandboxMode>(table, "sandbox_mode", "sandbox_mode", source, warnings);
remove_invalid_enum::<SandboxMode>(table, "sandbox_mode", "sandbox_mode", warnings);
remove_invalid_enum::<ReasoningEffort>(
table,
"model_reasoning_effort",
"model_reasoning_effort",
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
table,
"plan_mode_reasoning_effort",
"plan_mode_reasoning_effort",
source,
warnings,
);
remove_invalid_enum::<ReasoningSummary>(
table,
"model_reasoning_summary",
"model_reasoning_summary",
source,
warnings,
);
remove_invalid_enum::<Verbosity>(
table,
"model_verbosity",
"model_verbosity",
source,
warnings,
);
remove_invalid_enum::<Personality>(table, "personality", "personality", source, warnings);
remove_invalid_enum::<ServiceTier>(table, "service_tier", "service_tier", source, warnings);
remove_invalid_enum::<Verbosity>(table, "model_verbosity", "model_verbosity", warnings);
remove_invalid_enum::<Personality>(table, "personality", "personality", warnings);
remove_invalid_enum::<ServiceTier>(table, "service_tier", "service_tier", warnings);
remove_invalid_enum::<ForcedLoginMethod>(
table,
"forced_login_method",
"forced_login_method",
source,
warnings,
);
remove_invalid_enum::<AuthCredentialsStoreMode>(
table,
"cli_auth_credentials_store",
"cli_auth_credentials_store",
source,
warnings,
);
remove_invalid_enum::<OAuthCredentialsStoreMode>(
table,
"mcp_oauth_credentials_store",
"mcp_oauth_credentials_store",
source,
warnings,
);
remove_invalid_enum::<UriBasedFileOpener>(
table,
"file_opener",
"file_opener",
source,
warnings,
);
remove_invalid_enum::<UriBasedFileOpener>(table, "file_opener", "file_opener", warnings);
remove_invalid_enum::<ThreadStoreProbe>(
table,
"experimental_thread_store",
"experimental_thread_store",
source,
warnings,
);
remove_invalid_enum::<WebSearchMode>(table, "web_search", "web_search", source, warnings);
remove_invalid_enum::<WebSearchMode>(table, "web_search", "web_search", warnings);
if let Some(tui) = table.get_mut("tui").and_then(TomlValue::as_table_mut) {
remove_invalid_enum::<NotificationMethod>(
tui,
"notification_method",
"tui.notification_method",
source,
warnings,
);
remove_invalid_enum::<NotificationCondition>(
tui,
"notification_condition",
"tui.notification_condition",
source,
warnings,
);
remove_invalid_enum::<AltScreenMode>(
tui,
"alternate_screen",
"tui.alternate_screen",
source,
warnings,
);
}
@@ -171,7 +134,6 @@ fn sanitize_config_table(
shell,
"inherit",
"shell_environment_policy.inherit",
source,
warnings,
);
}
@@ -181,25 +143,17 @@ fn sanitize_config_table(
windows,
"sandbox",
"windows.sandbox",
source,
warnings,
);
}
if let Some(realtime) = table.get_mut("realtime").and_then(TomlValue::as_table_mut) {
remove_invalid_enum::<RealtimeWsVersion>(
realtime,
"version",
"realtime.version",
source,
warnings,
);
remove_invalid_enum::<RealtimeWsMode>(realtime, "type", "realtime.type", source, warnings);
remove_invalid_enum::<RealtimeWsVersion>(realtime, "version", "realtime.version", warnings);
remove_invalid_enum::<RealtimeWsMode>(realtime, "type", "realtime.type", warnings);
remove_invalid_enum::<RealtimeTransport>(
realtime,
"transport",
"realtime.transport",
source,
warnings,
);
}
@@ -214,7 +168,6 @@ fn sanitize_config_table(
web_search_tool,
"search_context_size",
"tools.web_search.search_context_size",
source,
warnings,
);
if let Some(user_location) = web_search_tool
@@ -225,7 +178,6 @@ fn sanitize_config_table(
user_location,
"type",
"tools.web_search.user_location.type",
source,
warnings,
);
}
@@ -238,25 +190,20 @@ fn sanitize_config_table(
project,
"trust_level",
&format!("projects.{name}.trust_level"),
source,
warnings,
);
}
}
}
sanitize_profiles(table, source, warnings);
sanitize_permissions(table, source, warnings);
sanitize_mcp_servers(table, source, warnings);
sanitize_plugins(table, source, warnings);
sanitize_otel(table, source, warnings);
sanitize_profiles(table, warnings);
sanitize_permissions(table, warnings);
sanitize_mcp_servers(table, warnings);
sanitize_plugins(table, warnings);
sanitize_otel(table, warnings);
}
fn sanitize_profiles(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
fn sanitize_profiles(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
let Some(profiles) = table.get_mut("profiles").and_then(TomlValue::as_table_mut) else {
return;
};
@@ -268,70 +215,60 @@ fn sanitize_profiles(
profile,
"service_tier",
&format!("profiles.{name}.service_tier"),
source,
warnings,
);
remove_invalid_enum::<AskForApproval>(
profile,
"approval_policy",
&format!("profiles.{name}.approval_policy"),
source,
warnings,
);
remove_invalid_enum::<ApprovalsReviewer>(
profile,
"approvals_reviewer",
&format!("profiles.{name}.approvals_reviewer"),
source,
warnings,
);
remove_invalid_enum::<SandboxMode>(
profile,
"sandbox_mode",
&format!("profiles.{name}.sandbox_mode"),
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
profile,
"model_reasoning_effort",
&format!("profiles.{name}.model_reasoning_effort"),
source,
warnings,
);
remove_invalid_enum::<ReasoningEffort>(
profile,
"plan_mode_reasoning_effort",
&format!("profiles.{name}.plan_mode_reasoning_effort"),
source,
warnings,
);
remove_invalid_enum::<ReasoningSummary>(
profile,
"model_reasoning_summary",
&format!("profiles.{name}.model_reasoning_summary"),
source,
warnings,
);
remove_invalid_enum::<Verbosity>(
profile,
"model_verbosity",
&format!("profiles.{name}.model_verbosity"),
source,
warnings,
);
remove_invalid_enum::<Personality>(
profile,
"personality",
&format!("profiles.{name}.personality"),
source,
warnings,
);
remove_invalid_enum::<WebSearchMode>(
profile,
"web_search",
&format!("profiles.{name}.web_search"),
source,
warnings,
);
if let Some(windows) = profile.get_mut("windows").and_then(TomlValue::as_table_mut) {
@@ -339,18 +276,13 @@ fn sanitize_profiles(
windows,
"sandbox",
&format!("profiles.{name}.windows.sandbox"),
source,
warnings,
);
}
}
}
fn sanitize_permissions(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
fn sanitize_permissions(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
let Some(permissions) = table
.get_mut("permissions")
.and_then(TomlValue::as_table_mut)
@@ -369,7 +301,6 @@ fn sanitize_permissions(
remove_invalid_map_enums::<NetworkDomainPermissionToml>(
domains,
&format!("permissions.{profile_name}.network.domains"),
source,
warnings,
);
}
@@ -380,18 +311,13 @@ fn sanitize_permissions(
remove_invalid_map_enums::<NetworkUnixSocketPermissionToml>(
unix_sockets,
&format!("permissions.{profile_name}.network.unix_sockets"),
source,
warnings,
);
}
}
}
fn sanitize_mcp_servers(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
fn sanitize_mcp_servers(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
let Some(mcp_servers) = table
.get_mut("mcp_servers")
.and_then(TomlValue::as_table_mut)
@@ -406,7 +332,6 @@ fn sanitize_mcp_servers(
server,
"default_tools_approval_mode",
&format!("mcp_servers.{server_name}.default_tools_approval_mode"),
source,
warnings,
);
if let Some(tools) = server.get_mut("tools").and_then(TomlValue::as_table_mut) {
@@ -416,7 +341,6 @@ fn sanitize_mcp_servers(
tool,
"approval_mode",
&format!("mcp_servers.{server_name}.tools.{tool_name}.approval_mode"),
source,
warnings,
);
}
@@ -425,11 +349,7 @@ fn sanitize_mcp_servers(
}
}
fn sanitize_plugins(
table: &mut TomlMap<String, TomlValue>,
source: &str,
warnings: &mut Vec<String>,
) {
fn sanitize_plugins(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
let Some(plugins) = table.get_mut("plugins").and_then(TomlValue::as_table_mut) else {
return;
};
@@ -451,7 +371,6 @@ fn sanitize_plugins(
&format!(
"plugins.{plugin_name}.mcp_servers.{server_name}.default_tools_approval_mode"
),
source,
warnings,
);
if let Some(tools) = server.get_mut("tools").and_then(TomlValue::as_table_mut) {
@@ -463,7 +382,6 @@ fn sanitize_plugins(
&format!(
"plugins.{plugin_name}.mcp_servers.{server_name}.tools.{tool_name}.approval_mode"
),
source,
warnings,
);
}
@@ -473,24 +391,22 @@ fn sanitize_plugins(
}
}
fn sanitize_otel(table: &mut TomlMap<String, TomlValue>, source: &str, warnings: &mut Vec<String>) {
fn sanitize_otel(table: &mut TomlMap<String, TomlValue>, warnings: &mut Vec<String>) {
let Some(otel) = table.get_mut("otel").and_then(TomlValue::as_table_mut) else {
return;
};
remove_invalid_enum::<OtelHttpProtocol>(otel, "protocol", "otel.protocol", source, warnings);
remove_invalid_enum::<OtelExporterKind>(otel, "exporter", "otel.exporter", source, warnings);
remove_invalid_enum::<OtelHttpProtocol>(otel, "protocol", "otel.protocol", warnings);
remove_invalid_enum::<OtelExporterKind>(otel, "exporter", "otel.exporter", warnings);
remove_invalid_enum::<OtelExporterKind>(
otel,
"trace_exporter",
"otel.trace_exporter",
source,
warnings,
);
remove_invalid_enum::<OtelExporterKind>(
otel,
"metrics_exporter",
"otel.metrics_exporter",
source,
warnings,
);
}
@@ -498,7 +414,6 @@ fn sanitize_otel(table: &mut TomlMap<String, TomlValue>, source: &str, warnings:
fn remove_invalid_map_enums<T>(
table: &mut TomlMap<String, TomlValue>,
path: &str,
source: &str,
warnings: &mut Vec<String>,
) where
T: DeserializeOwned,
@@ -511,11 +426,7 @@ fn remove_invalid_map_enums<T>(
let Some(value) = table.remove(&key) else {
continue;
};
warnings.push(invalid_enum_warning(
source,
&format!("{path}.{key}"),
&value,
));
warnings.push(invalid_enum_warning(&format!("{path}.{key}"), &value));
}
}
@@ -523,7 +434,6 @@ fn remove_invalid_enum<T>(
table: &mut TomlMap<String, TomlValue>,
key: &str,
path: &str,
source: &str,
warnings: &mut Vec<String>,
) where
T: DeserializeOwned,
@@ -537,7 +447,7 @@ fn remove_invalid_enum<T>(
let Some(value) = table.remove(key) else {
return;
};
warnings.push(invalid_enum_warning(source, path, &value));
warnings.push(invalid_enum_warning(path, &value));
}
fn invalid_enum<T>(value: &TomlValue) -> bool
@@ -550,8 +460,8 @@ where
}
}
fn invalid_enum_warning(source: &str, path: &str, value: &TomlValue) -> String {
format!("Ignoring invalid config value in {source} at {path}: {value}")
fn invalid_enum_warning(path: &str, value: &TomlValue) -> String {
format!("Ignoring invalid config value at {path}: {value}")
}
#[derive(serde::Deserialize)]
+11 -78
View File
@@ -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::sanitize_config_toml_enums;
use crate::merge::merge_toml_values;
use crate::overrides::build_cli_overrides_layer;
use crate::project_root_markers::default_project_root_markers;
@@ -174,8 +173,6 @@ pub async fn load_config_layers_state(
)?)
};
let mut startup_warnings = Vec::new();
// Include an entry for the "system" config folder, loading its config.toml,
// if it exists.
let system_config_toml_file = system_config_toml_file_with_overrides(&overrides)?;
@@ -189,12 +186,7 @@ pub async fn load_config_layers_state(
)
})
.await?;
push_config_layer_with_enum_warnings(
system_layer,
&mut layers,
&mut startup_warnings,
CONFIG_TOML_FILE,
);
layers.push(system_layer);
// Add a layer for $CODEX_HOME/config.toml so folder-derived resources such
// as rules/ can still be discovered. When user config is ignored, preserve
@@ -218,14 +210,9 @@ pub async fn load_config_layers_state(
})
.await?
};
push_config_layer_with_enum_warnings(
user_layer,
&mut layers,
&mut startup_warnings,
CONFIG_TOML_FILE,
);
layers.push(user_layer);
let checked_stack_warnings = cwd.is_some();
let mut startup_warnings = None;
if let Some(cwd) = cwd {
let mut merged_so_far = TomlValue::Table(toml::map::Map::new());
for layer in &layers {
@@ -283,7 +270,7 @@ pub async fn load_config_layers_state(
)
.await?;
layers.extend(project_layers.layers);
startup_warnings.extend(project_layers.startup_warnings);
startup_warnings = Some(project_layers.startup_warnings);
}
// Add a layer for runtime overrides from the CLI or UI, if any exist.
@@ -319,16 +306,10 @@ pub async fn load_config_layers_state(
})?;
let managed_config =
resolve_relative_paths_in_config_toml(config.managed_config, managed_parent)?;
let managed_layer = ConfigLayerEntry::new(
layers.push(ConfigLayerEntry::new(
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: config.file },
managed_config,
);
push_config_layer_with_enum_warnings(
managed_layer,
&mut layers,
&mut startup_warnings,
CONFIG_TOML_FILE,
);
));
}
if let Some(config) = managed_config_from_mdm {
// As a general rule, config from MDM should _not_ include relative
@@ -338,17 +319,11 @@ pub async fn load_config_layers_state(
// value for base_dir, so codex_home is as good a value as any.
let managed_config =
resolve_relative_paths_in_config_toml(config.managed_config, codex_home)?;
let managed_layer = ConfigLayerEntry::new_with_raw_toml(
layers.push(ConfigLayerEntry::new_with_raw_toml(
ConfigLayerSource::LegacyManagedConfigTomlFromMdm,
managed_config,
config.raw_toml,
);
push_config_layer_with_enum_warnings(
managed_layer,
&mut layers,
&mut startup_warnings,
CONFIG_TOML_FILE,
);
));
}
let config_layer_stack = ConfigLayerStack::new(
@@ -357,48 +332,12 @@ pub async fn load_config_layers_state(
config_requirements_toml.into_toml(),
)?
.with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules);
Ok(if checked_stack_warnings || !startup_warnings.is_empty() {
config_layer_stack.with_startup_warnings(startup_warnings)
} else {
config_layer_stack
Ok(match startup_warnings {
Some(startup_warnings) => config_layer_stack.with_startup_warnings(startup_warnings),
None => config_layer_stack,
})
}
fn push_config_layer_with_enum_warnings(
mut layer: ConfigLayerEntry,
layers: &mut Vec<ConfigLayerEntry>,
startup_warnings: &mut Vec<String>,
config_toml_file: &str,
) {
startup_warnings.extend(sanitize_config_toml_enums(
&mut layer.config,
config_warning_source(&layer.name, config_toml_file),
));
layers.push(layer);
}
fn config_warning_source(layer: &ConfigLayerSource, config_toml_file: &str) -> String {
match layer {
ConfigLayerSource::System { file } => {
format!("system config {}", file.as_path().display())
}
ConfigLayerSource::User { file } => {
format!("user config {}", file.as_path().display())
}
ConfigLayerSource::Project { dot_codex_folder } => format!(
"project config {}",
dot_codex_folder.as_path().join(config_toml_file).display()
),
ConfigLayerSource::SessionFlags => "session config overrides".to_string(),
ConfigLayerSource::LegacyManagedConfigTomlFromFile { file } => {
format!("managed config {}", file.as_path().display())
}
ConfigLayerSource::Mdm { .. } | ConfigLayerSource::LegacyManagedConfigTomlFromMdm => {
"managed device config".to_string()
}
}
}
fn insert_layer_by_precedence(layers: &mut Vec<ConfigLayerEntry>, layer: ConfigLayerEntry) {
match layers
.iter()
@@ -1085,12 +1024,6 @@ async fn load_project_layers(
};
let mut config = config;
let ignored_project_config_keys = sanitize_project_config(&mut config);
if disabled_reason.is_none() {
startup_warnings.extend(sanitize_config_toml_enums(
&mut config,
format!("project config {}", config_file.as_path().display()),
));
}
let config =
resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?;
if disabled_reason.is_none() && !ignored_project_config_keys.is_empty() {
+17
View File
@@ -4,6 +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::sanitize_config_toml_enums;
use super::merge::merge_toml_values;
use codex_app_server_protocol::ConfigLayer;
use codex_app_server_protocol::ConfigLayerMetadata;
@@ -212,6 +213,16 @@ impl ConfigLayerStack {
self
}
pub fn with_additional_startup_warnings(mut self, warnings: Vec<String>) -> Self {
if warnings.is_empty() {
return self;
}
self.startup_warnings
.get_or_insert_with(Vec::new)
.extend(warnings);
self
}
pub fn startup_warnings(&self) -> Option<&[String]> {
self.startup_warnings.as_deref()
}
@@ -300,6 +311,12 @@ impl ConfigLayerStack {
merged
}
pub fn effective_config_with_warnings(&self) -> (TomlValue, Vec<String>) {
let mut merged = self.effective_config();
let warnings = sanitize_config_toml_enums(&mut merged);
(merged, warnings)
}
/// Returns field origins for the merged config-layer view.
///
/// Requirement sources are tracked separately and are not included here.
@@ -143,7 +143,7 @@ notification_method = "loudly"
)
.await?;
let effective_config = layers.effective_config();
let (effective_config, enum_warnings) = layers.effective_config_with_warnings();
let expected_config = toml::from_str::<TomlValue>(
r#"
model = "gpt-5-codex"
@@ -153,21 +153,12 @@ model = "gpt-5-codex"
)
.expect("expected config should parse");
let expected_startup_warnings = vec![
format!(
"Ignoring invalid config value in user config {} at sandbox_mode: \"make-it-so\"",
config_path.display()
),
format!(
"Ignoring invalid config value in user config {} at tui.notification_method: \"loudly\"",
config_path.display()
),
"Ignoring invalid config value at sandbox_mode: \"make-it-so\"".to_string(),
"Ignoring invalid config value at tui.notification_method: \"loudly\"".to_string(),
];
assert_eq!(
(
effective_config,
layers.startup_warnings().unwrap_or_default().to_vec()
),
(effective_config, enum_warnings),
(expected_config, expected_startup_warnings)
);
Ok(())
+3 -2
View File
@@ -963,7 +963,8 @@ impl ConfigBuilder {
.unwrap_or(&codex_config::NoopThreadConfigLoader),
)
.await?;
let merged_toml = config_layer_stack.effective_config();
let (merged_toml, enum_warnings) = config_layer_stack.effective_config_with_warnings();
let config_layer_stack = config_layer_stack.with_additional_startup_warnings(enum_warnings);
// Note that each layer in ConfigLayerStack should have resolved
// relative paths to absolute paths based on the parent folder of the
@@ -1225,7 +1226,7 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides(
)
.await?;
let merged_toml = config_layer_stack.effective_config();
let (merged_toml, _enum_warnings) = config_layer_stack.effective_config_with_warnings();
let cfg = deserialize_config_toml_with_base(merged_toml, codex_home).map_err(|e| {
tracing::error!("Failed to deserialize overridden config: {e}");
e