diff --git a/codex-rs/cli/tests/features.rs b/codex-rs/cli/tests/features.rs index 9ce0314e32..6da9fd9325 100644 --- a/codex-rs/cli/tests/features.rs +++ b/codex-rs/cli/tests/features.rs @@ -27,13 +27,19 @@ fn codex_command(codex_home: &Path) -> Result { #[test] fn strict_config_rejects_unknown_config_override() -> Result<()> { - let codex_home = TempDir::new()?; - - let mut cmd = codex_command(codex_home.path())?; - cmd.args(["--strict-config", "-c", "foo=bar", "exec", "hello"]) - .assert() - .failure() - .stderr(contains("unknown configuration field")); + for (config_override, ignored_path) in [ + ("foo=bar", "foo"), + ("tui.bogus_key_xyz=1", "tui.bogus_key_xyz"), + ] { + let codex_home = TempDir::new()?; + let mut cmd = codex_command(codex_home.path())?; + cmd.args(["--strict-config", "-c", config_override, "exec", "hello"]) + .assert() + .failure() + .stderr(contains(format!( + "unknown configuration field `{ignored_path}`" + ))); + } Ok(()) } @@ -73,6 +79,12 @@ fn interactive_validates_config_before_requiring_terminal() -> Result<()> { "unknown_key = true", "unknown configuration field", ), + ( + &["--strict-config"], + "config.toml", + "[tui]\nbogus_key_xyz = 1", + "unknown configuration field `tui.bogus_key_xyz`", + ), ( &["--strict-config", "-c", "foo=bar"], "config.toml", diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 33854b0270..e524076d7b 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -45,6 +45,7 @@ use crate::strict_config::config_error_from_ignored_toml_value_fields; use crate::strict_config::ignored_config_warning; use crate::strict_config::ignored_toml_value_fields; use crate::strict_config::unknown_feature_toml_value_field; +use crate::strict_config::unknown_tui_toml_value_path; use crate::thread_config::ThreadConfigContext; use crate::thread_config::ThreadConfigLoader; use codex_file_system::ExecutorFileSystem; @@ -653,16 +654,13 @@ fn validate_cli_overrides_strictly( base_dir: &Path, ) -> io::Result<()> { let _guard = AbsolutePathBufGuard::new(base_dir); - if let Some(path) = ignored_toml_value_fields::(cli_overrides_layer.clone()).first() - { - let ignored_path = path.join("."); - return Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("unknown configuration field `{ignored_path}` in -c/--config override"), - )); - } - - if let Some(ignored_path) = unknown_feature_toml_value_field(cli_overrides_layer) { + let ignored_path = ignored_toml_value_fields::(cli_overrides_layer.clone()) + .into_iter() + .chain(unknown_tui_toml_value_path(cli_overrides_layer)) + .next() + .map(|path| path.join(".")) + .or_else(|| unknown_feature_toml_value_field(cli_overrides_layer)); + if let Some(ignored_path) = ignored_path { return Err(io::Error::new( io::ErrorKind::InvalidData, format!("unknown configuration field `{ignored_path}` in -c/--config override"), diff --git a/codex-rs/config/src/strict_config.rs b/codex-rs/config/src/strict_config.rs index 038adbf988..b8e8f8d88d 100644 --- a/codex-rs/config/src/strict_config.rs +++ b/codex-rs/config/src/strict_config.rs @@ -14,15 +14,17 @@ use crate::diagnostics::span_for_toml_key_path; use crate::diagnostics::text_range_from_span; use crate::format_config_layer_source; use crate::requirements_layers::strip_cloud_auth_requirements; +use crate::types::Tui; use codex_features::is_known_feature_key; use codex_utils_absolute_path::AbsolutePathBufGuard; use serde::de::DeserializeOwned; +use std::any::TypeId; use std::collections::BTreeSet; use std::fmt::Write; use std::path::Path; use toml::Value as TomlValue; -pub fn config_error_from_ignored_toml_fields( +pub fn config_error_from_ignored_toml_fields( path: impl AsRef, contents: &str, ) -> Option { @@ -35,7 +37,7 @@ pub fn config_error_from_ignored_toml_fields( } } -pub(crate) fn config_error_from_ignored_toml_value_fields( +pub(crate) fn config_error_from_ignored_toml_value_fields( path: impl AsRef, contents: &str, value: TomlValue, @@ -47,7 +49,9 @@ pub(crate) fn config_error_from_ignored_toml_value_fields( ) } -pub(crate) fn config_error_from_ignored_toml_value_fields_for_source_name( +pub(crate) fn config_error_from_ignored_toml_value_fields_for_source_name< + T: DeserializeOwned + 'static, +>( source_name: &str, contents: &str, value: TomlValue, @@ -59,12 +63,17 @@ pub(crate) fn config_error_from_ignored_toml_value_fields_for_source_name( +fn config_error_from_ignored_toml_value_fields_for_source( source: ConfigDiagnosticSource<'_>, contents: &str, value: TomlValue, ) -> Option { let unknown_feature_paths = unknown_feature_toml_value_path(&value); + let unknown_tui_paths = if TypeId::of::() == TypeId::of::() { + unknown_tui_toml_value_path(&value) + } else { + Vec::new() + }; let mut ignored_paths = Vec::new(); let mut ignored_callback = |ignored_path: serde_ignored::Path<'_>| { let path_segments = ignored_path_segments(&ignored_path); @@ -77,6 +86,7 @@ fn config_error_from_ignored_toml_value_fields_for_source( match result { Ok(_) => unknown_field_error_from_paths(source, contents, ignored_paths) + .or_else(|| unknown_field_error_from_paths(source, contents, unknown_tui_paths)) .or_else(|| unknown_field_error_from_paths(source, contents, unknown_feature_paths)), Err(err) => { let path_hint = err.path().clone(); @@ -94,6 +104,29 @@ fn config_error_from_ignored_toml_value_fields_for_source( } } +pub(crate) fn unknown_tui_toml_value_path(value: &TomlValue) -> Vec> { + // `serde_ignored` cannot observe keys consumed by the flattened notification settings. + // Round-trip the TUI config so accepted input keys stay derived from `Tui` itself. + let Some(tui_value) = value.as_table().and_then(|root| root.get("tui")) else { + return Vec::new(); + }; + let Some(configured_tui) = tui_value.as_table() else { + return Vec::new(); + }; + let Ok(tui) = tui_value.clone().try_into::() else { + return Vec::new(); + }; + let Ok(TomlValue::Table(serialized_tui)) = TomlValue::try_from(tui) else { + return Vec::new(); + }; + + configured_tui + .keys() + .filter(|key| !serialized_tui.contains_key(*key)) + .map(|key| vec!["tui".to_string(), key.clone()]) + .collect() +} + pub(crate) fn ignored_toml_value_fields(value: TomlValue) -> Vec> { let mut ignored_paths = Vec::new(); let result: Result = serde_ignored::deserialize(value, |ignored_path| { diff --git a/codex-rs/config/src/strict_config_tests.rs b/codex-rs/config/src/strict_config_tests.rs index 4587ac0217..22a10b48ef 100644 --- a/codex-rs/config/src/strict_config_tests.rs +++ b/codex-rs/config/src/strict_config_tests.rs @@ -6,6 +6,7 @@ use crate::config_toml::ConfigToml; use crate::diagnostics::TextPosition; use crate::diagnostics::TextRange; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; use std::path::PathBuf; #[test] @@ -91,6 +92,82 @@ foo = true"#; ); } +#[test] +fn strict_config_rejects_unknown_tui_key() { + let path = Path::new("/tmp/config.toml"); + let contents = r#" +[tui] +bogus_key_xyz = 1"#; + + let error = config_error_from_ignored_toml_fields::(path, contents) + .expect("unknown TUI key error"); + + assert_eq!( + error, + ConfigError::new( + path.to_path_buf(), + TextRange { + start: TextPosition { line: 3, column: 1 }, + end: TextPosition { + line: 3, + column: 13, + }, + }, + "unknown configuration field `tui.bogus_key_xyz`", + ) + ); +} + +#[test] +fn strict_config_accepts_tui_notification_settings() { + let path = Path::new("/tmp/config.toml"); + + for contents in [ + "[tui]\nnotifications = false\n", + "[tui]\nnotification_method = \"bel\"\n", + "[tui]\nnotification_condition = \"always\"\n", + ] { + assert_eq!( + config_error_from_ignored_toml_fields::(path, contents), + None + ); + } +} + +#[test] +fn generic_validation_does_not_apply_config_toml_tui_fields() { + let path = Path::new("/tmp/config.toml"); + let contents = "[tui]\nplugin_key = 1\n"; + + assert_eq!( + config_error_from_ignored_toml_fields::>>( + path, contents, + ), + None + ); +} + +#[test] +fn wrapped_config_toml_still_rejects_unknown_feature_key() { + let path = Path::new("/tmp/config.toml"); + let contents = "[features]\nfoo = true\n"; + + let error = config_error_from_ignored_toml_fields::>(path, contents) + .expect("unknown feature error"); + + assert_eq!( + error, + ConfigError::new( + path.to_path_buf(), + TextRange { + start: TextPosition { line: 2, column: 1 }, + end: TextPosition { line: 2, column: 3 }, + }, + "unknown configuration field `features.foo`", + ) + ); +} + #[test] fn strict_config_accepts_tool_registry_config() { let path = Path::new("/tmp/config.toml");