mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
Warn about ignored configuration settings (#44691)
## Why Unrecognized settings can be silently ignored, leaving typos or deprecated configuration unnoticed. ## What changed - Collect warnings for unrecognized fields in effective configuration and requirements layers, including unknown feature keys in configuration. - Report sources and field names without configuration values, show up to three entries, and provide migration hints for `network_proxy`, `allowed_permissions`, and `include_view_image_tool`. - Emit project-specific warnings when starting a thread and suppress repeated startup warnings in the app server and TUI. ## Testing Add coverage for merged configuration, disabled layers, startup and project warning delivery, omission of configuration values, and warning deduplication. GitOrigin-RevId: 464cef87882502b19865ae10b50b6f82defb7296
This commit is contained in:
1 parent
9e22e74e8d
commit
e53c444964
11 files changed
+305
-44
No files matched your search
@@ -1424,10 +1424,23 @@ impl ThreadRequestProcessor {
|
|||||||
.map_err(|err| config_load_error(&err))?;
|
.map_err(|err| config_load_error(&err))?;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread config can include project-local warnings absent at initialization.
|
||||||
|
let mut config_warnings = config
|
||||||
|
.startup_warnings
|
||||||
|
.iter()
|
||||||
|
.map(|summary| ConfigWarningNotification {
|
||||||
|
summary: summary.clone(),
|
||||||
|
details: None,
|
||||||
|
path: None,
|
||||||
|
range: None,
|
||||||
|
})
|
||||||
|
.collect::<Vec<_>>();
|
||||||
if let Ok(Some(err)) =
|
if let Ok(Some(err)) =
|
||||||
codex_core::check_execpolicy_for_warnings(&config.config_layer_stack).await
|
codex_core::check_execpolicy_for_warnings(&config.config_layer_stack).await
|
||||||
{
|
{
|
||||||
let notification = crate::exec_policy_config_warning(&err);
|
config_warnings.push(crate::exec_policy_config_warning(&err));
|
||||||
|
}
|
||||||
|
for notification in config_warnings {
|
||||||
if !initial_config_warnings.contains(¬ification) {
|
if !initial_config_warnings.contains(¬ification) {
|
||||||
listener_task_context
|
listener_task_context
|
||||||
.outgoing
|
.outgoing
|
||||||
|
|||||||
@@ -1,7 +1,76 @@
|
|||||||
use std::process::Command;
|
//! Config errors and startup/project warnings through the public API.
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
|
use app_test_support::TestAppServer;
|
||||||
|
use codex_app_server_protocol::ConfigWarningNotification;
|
||||||
|
use codex_app_server_protocol::ThreadStartParams;
|
||||||
|
use codex_core::config::set_project_trust_level;
|
||||||
|
use codex_protocol::config_types::TrustLevel;
|
||||||
|
use pretty_assertions::assert_ne;
|
||||||
|
use std::process::Command;
|
||||||
|
use std::time::Duration;
|
||||||
use tempfile::TempDir;
|
use tempfile::TempDir;
|
||||||
|
use tokio::time::timeout;
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn ignored_config_fields_emit_startup_and_project_warnings() -> Result<()> {
|
||||||
|
let home = TempDir::new()?;
|
||||||
|
std::fs::write(
|
||||||
|
home.path().join("config.toml"),
|
||||||
|
"network_proxy = { nested = 'private_value' }",
|
||||||
|
)?;
|
||||||
|
std::fs::write(
|
||||||
|
home.path().join("requirements.toml"),
|
||||||
|
"allowed_permissions = [':read-only']",
|
||||||
|
)?;
|
||||||
|
let read_timeout = Duration::from_secs(30);
|
||||||
|
let mut server = TestAppServer::builder()
|
||||||
|
.with_codex_home(home.path())
|
||||||
|
.build_initialized_with_timeout(read_timeout)
|
||||||
|
.await?;
|
||||||
|
let startup_warning: ConfigWarningNotification =
|
||||||
|
timeout(read_timeout, server.read_notification("configWarning")).await??;
|
||||||
|
let summary = &startup_warning.summary;
|
||||||
|
assert!(summary.contains("`network_proxy` is ignored."));
|
||||||
|
assert!(summary.contains("`allowed_permissions` is ignored."));
|
||||||
|
assert!(!summary.contains("private_value"));
|
||||||
|
|
||||||
|
server.start_thread(ThreadStartParams::default()).await?;
|
||||||
|
// Config warnings are sent before the thread/start response. Other startup
|
||||||
|
// warnings (e.g. missing bubblewrap on Linux) may still be buffered here.
|
||||||
|
while server
|
||||||
|
.pending_notification_methods()
|
||||||
|
.iter()
|
||||||
|
.any(|method| method == "configWarning")
|
||||||
|
{
|
||||||
|
let warning: ConfigWarningNotification =
|
||||||
|
timeout(read_timeout, server.read_notification("configWarning")).await??;
|
||||||
|
assert_ne!(
|
||||||
|
warning, startup_warning,
|
||||||
|
"thread/start repeated the startup warning"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let project = TempDir::new()?;
|
||||||
|
std::fs::create_dir(project.path().join(".git"))?;
|
||||||
|
std::fs::create_dir(project.path().join(".codex"))?;
|
||||||
|
std::fs::write(
|
||||||
|
project.path().join(".codex/config.toml"),
|
||||||
|
"project_setting = 'private_value'",
|
||||||
|
)?;
|
||||||
|
set_project_trust_level(home.path(), project.path(), TrustLevel::Trusted)?;
|
||||||
|
server
|
||||||
|
.start_thread(ThreadStartParams {
|
||||||
|
cwd: Some(project.path().display().to_string()),
|
||||||
|
..Default::default()
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
let warning: ConfigWarningNotification =
|
||||||
|
timeout(read_timeout, server.read_notification("configWarning")).await??;
|
||||||
|
assert!(warning.summary.contains("`project_setting` is ignored."));
|
||||||
|
assert!(!warning.summary.contains("private_value"));
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn strict_config_rejects_unknown_config_fields_for_standalone_app_server() -> Result<()> {
|
fn strict_config_rejects_unknown_config_fields_for_standalone_app_server() -> Result<()> {
|
||||||
|
|||||||
@@ -34,7 +34,8 @@ use crate::state::ConfigLoadOptions;
|
|||||||
use crate::state::LoaderOverrides;
|
use crate::state::LoaderOverrides;
|
||||||
use crate::state::validate_enabled_config_layers;
|
use crate::state::validate_enabled_config_layers;
|
||||||
use crate::strict_config::config_error_from_ignored_toml_value_fields;
|
use crate::strict_config::config_error_from_ignored_toml_value_fields;
|
||||||
use crate::strict_config::ignored_toml_value_field;
|
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_feature_toml_value_field;
|
||||||
use crate::thread_config::ThreadConfigContext;
|
use crate::thread_config::ThreadConfigContext;
|
||||||
use crate::thread_config::ThreadConfigLoader;
|
use crate::thread_config::ThreadConfigLoader;
|
||||||
@@ -242,8 +243,7 @@ pub async fn load_config_layers_state(
|
|||||||
let loaded_config_layers =
|
let loaded_config_layers =
|
||||||
layer_io::load_config_layers_internal(fs, codex_home, overrides.clone(), strict_config)
|
layer_io::load_config_layers_internal(fs, codex_home, overrides.clone(), strict_config)
|
||||||
.await?;
|
.await?;
|
||||||
let mut startup_warnings = (!loaded_config_layers.startup_warnings.is_empty())
|
let mut startup_warnings = loaded_config_layers.startup_warnings.clone();
|
||||||
.then(|| loaded_config_layers.startup_warnings.clone());
|
|
||||||
if !ignore_managed_requirements {
|
if !ignore_managed_requirements {
|
||||||
requirements_layers.extend(system_requirements_layer);
|
requirements_layers.extend(system_requirements_layer);
|
||||||
requirements_layers.extend(bundle_requirements_layers);
|
requirements_layers.extend(bundle_requirements_layers);
|
||||||
@@ -257,7 +257,7 @@ pub async fn load_config_layers_state(
|
|||||||
}
|
}
|
||||||
|
|
||||||
let mut config_requirements_toml =
|
let mut config_requirements_toml =
|
||||||
compose_requirements(requirements_layers)?.unwrap_or_default();
|
compose_requirements(requirements_layers.clone())?.unwrap_or_default();
|
||||||
// Remote app servers enforce auth policy for their workspaces; do not let local
|
// Remote app servers enforce auth policy for their workspaces; do not let local
|
||||||
// requirements reintroduce authentication restrictions for those workspaces.
|
// requirements reintroduce authentication restrictions for those workspaces.
|
||||||
if overrides.ignore_login_requirements {
|
if overrides.ignore_login_requirements {
|
||||||
@@ -436,9 +436,7 @@ pub async fn load_config_layers_state(
|
|||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
layers.extend(project_layers.layers);
|
layers.extend(project_layers.layers);
|
||||||
startup_warnings
|
startup_warnings.extend(project_layers.startup_warnings);
|
||||||
.get_or_insert_with(Vec::new)
|
|
||||||
.extend(project_layers.startup_warnings);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add a layer for runtime overrides from the CLI or UI, if any exist.
|
// Add a layer for runtime overrides from the CLI or UI, if any exist.
|
||||||
@@ -521,10 +519,11 @@ pub async fn load_config_layers_state(
|
|||||||
config_requirements_toml.into_toml(),
|
config_requirements_toml.into_toml(),
|
||||||
)?
|
)?
|
||||||
.with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules);
|
.with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules);
|
||||||
Ok(match startup_warnings {
|
startup_warnings.extend(ignored_config_warning(
|
||||||
Some(startup_warnings) => config_layer_stack.with_startup_warnings(startup_warnings),
|
&config_layer_stack,
|
||||||
None => config_layer_stack,
|
&requirements_layers,
|
||||||
})
|
));
|
||||||
|
Ok(config_layer_stack.with_startup_warnings(startup_warnings))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn load_user_config_layer(
|
async fn load_user_config_layer(
|
||||||
@@ -676,8 +675,9 @@ fn validate_cli_overrides_strictly(
|
|||||||
base_dir: &Path,
|
base_dir: &Path,
|
||||||
) -> io::Result<()> {
|
) -> io::Result<()> {
|
||||||
let _guard = AbsolutePathBufGuard::new(base_dir);
|
let _guard = AbsolutePathBufGuard::new(base_dir);
|
||||||
if let Some(ignored_path) = ignored_toml_value_field::<ConfigToml>(cli_overrides_layer.clone())
|
if let Some(path) = ignored_toml_value_fields::<ConfigToml>(cli_overrides_layer.clone()).first()
|
||||||
{
|
{
|
||||||
|
let ignored_path = path.join(".");
|
||||||
return Err(io::Error::new(
|
return Err(io::Error::new(
|
||||||
io::ErrorKind::InvalidData,
|
io::ErrorKind::InvalidData,
|
||||||
format!("unknown configuration field `{ignored_path}` in -c/--config override"),
|
format!("unknown configuration field `{ignored_path}` in -c/--config override"),
|
||||||
|
|||||||
@@ -102,11 +102,7 @@ impl ComposableRequirementsLayer {
|
|||||||
let mut regular_toml = parse_layer_toml(&toml, &source)?;
|
let mut regular_toml = parse_layer_toml(&toml, &source)?;
|
||||||
|
|
||||||
// These fields can only be set locally; ignore them before validating cloud policy.
|
// These fields can only be set locally; ignore them before validating cloud policy.
|
||||||
if matches!(source, RequirementSource::EnterpriseManaged { .. }) {
|
strip_cloud_auth_requirements(&source, &mut regular_toml);
|
||||||
for field in LOCAL_ONLY_AUTH_REQUIREMENTS {
|
|
||||||
remove_top_level_field(&mut regular_toml, field);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Provider fragments can be incomplete until all requirements layers
|
// Provider fragments can be incomplete until all requirements layers
|
||||||
// are merged. Resolve explicit paths while their source base is still
|
// are merged. Resolve explicit paths while their source base is still
|
||||||
@@ -278,6 +274,14 @@ fn strip_special_fields(layer_toml: &mut TomlValue) {
|
|||||||
remove_nested_field_and_prune_empty(layer_toml, &["auto_review", "required_on_models"]);
|
remove_nested_field_and_prune_empty(layer_toml, &["auto_review", "required_on_models"]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn strip_cloud_auth_requirements(source: &RequirementSource, value: &mut TomlValue) {
|
||||||
|
if matches!(source, RequirementSource::EnterpriseManaged { .. }) {
|
||||||
|
for field in LOCAL_ONLY_AUTH_REQUIREMENTS {
|
||||||
|
remove_top_level_field(value, field);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option<TomlValue> {
|
fn remove_top_level_field(value: &mut TomlValue, key: &str) -> Option<TomlValue> {
|
||||||
value.as_table_mut()?.remove(key)
|
value.as_table_mut()?.remove(key)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,5 +6,6 @@ mod rules;
|
|||||||
mod stack;
|
mod stack;
|
||||||
|
|
||||||
pub use layer::RequirementsLayerEntry;
|
pub use layer::RequirementsLayerEntry;
|
||||||
|
pub(crate) use layer::strip_cloud_auth_requirements;
|
||||||
pub use stack::compose_requirements;
|
pub use stack::compose_requirements;
|
||||||
pub use stack::compose_requirements_for_hostname;
|
pub use stack::compose_requirements_for_hostname;
|
||||||
@@ -1,5 +1,10 @@
|
|||||||
//! Strict config validation built on top of serde's ignored-field tracking.
|
//! Strict config validation built on top of serde's ignored-field tracking.
|
||||||
|
|
||||||
|
use crate::CONFIG_TOML_FILE;
|
||||||
|
use crate::ConfigLayerStack;
|
||||||
|
use crate::ConfigRequirementsToml;
|
||||||
|
use crate::RequirementsLayerEntry;
|
||||||
|
use crate::config_toml::ConfigToml;
|
||||||
use crate::diagnostics::ConfigDiagnosticSource;
|
use crate::diagnostics::ConfigDiagnosticSource;
|
||||||
use crate::diagnostics::ConfigError;
|
use crate::diagnostics::ConfigError;
|
||||||
use crate::diagnostics::config_error_from_toml_for_source;
|
use crate::diagnostics::config_error_from_toml_for_source;
|
||||||
@@ -7,8 +12,13 @@ use crate::diagnostics::default_range;
|
|||||||
use crate::diagnostics::span_for_config_path;
|
use crate::diagnostics::span_for_config_path;
|
||||||
use crate::diagnostics::span_for_toml_key_path;
|
use crate::diagnostics::span_for_toml_key_path;
|
||||||
use crate::diagnostics::text_range_from_span;
|
use crate::diagnostics::text_range_from_span;
|
||||||
|
use crate::format_config_layer_source;
|
||||||
|
use crate::requirements_layers::strip_cloud_auth_requirements;
|
||||||
use codex_features::is_known_feature_key;
|
use codex_features::is_known_feature_key;
|
||||||
|
use codex_utils_absolute_path::AbsolutePathBufGuard;
|
||||||
use serde::de::DeserializeOwned;
|
use serde::de::DeserializeOwned;
|
||||||
|
use std::collections::BTreeSet;
|
||||||
|
use std::fmt::Write;
|
||||||
use std::path::Path;
|
use std::path::Path;
|
||||||
use toml::Value as TomlValue;
|
use toml::Value as TomlValue;
|
||||||
|
|
||||||
@@ -84,7 +94,7 @@ fn config_error_from_ignored_toml_value_fields_for_source<T: DeserializeOwned>(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn ignored_toml_value_field<T: DeserializeOwned>(value: TomlValue) -> Option<String> {
|
pub(crate) fn ignored_toml_value_fields<T: DeserializeOwned>(value: TomlValue) -> Vec<Vec<String>> {
|
||||||
let mut ignored_paths = Vec::new();
|
let mut ignored_paths = Vec::new();
|
||||||
let result: Result<T, _> = serde_ignored::deserialize(value, |ignored_path| {
|
let result: Result<T, _> = serde_ignored::deserialize(value, |ignored_path| {
|
||||||
let path_segments = ignored_path_segments(&ignored_path);
|
let path_segments = ignored_path_segments(&ignored_path);
|
||||||
@@ -93,13 +103,10 @@ pub(crate) fn ignored_toml_value_field<T: DeserializeOwned>(value: TomlValue) ->
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
if result.is_err() {
|
if result.is_err() {
|
||||||
return None;
|
return Vec::new();
|
||||||
}
|
}
|
||||||
|
|
||||||
ignored_paths
|
ignored_paths
|
||||||
.into_iter()
|
|
||||||
.next()
|
|
||||||
.map(|path_segments| path_segments.join("."))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn unknown_feature_toml_value_field(value: &TomlValue) -> Option<String> {
|
pub(crate) fn unknown_feature_toml_value_field(value: &TomlValue) -> Option<String> {
|
||||||
@@ -195,6 +202,111 @@ fn push_ignored_path_segments(path: &serde_ignored::Path<'_>, segments: &mut Vec
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) fn ignored_config_warning(
|
||||||
|
config: &ConfigLayerStack,
|
||||||
|
requirements: &[RequirementsLayerEntry],
|
||||||
|
) -> Option<String> {
|
||||||
|
let mut fields = BTreeSet::new();
|
||||||
|
// Validate the merged config so incomplete layer fragments (e.g. a provider
|
||||||
|
// override) do not hide diagnostics.
|
||||||
|
let effective = config.effective_config();
|
||||||
|
let unknown_features = unknown_feature_toml_value_path(&effective);
|
||||||
|
for path in ignored_toml_value_fields::<ConfigToml>(effective)
|
||||||
|
.into_iter()
|
||||||
|
.chain(unknown_features)
|
||||||
|
{
|
||||||
|
// Origins only track leaf values; an ignored field can be a whole table.
|
||||||
|
for layer in config.layers_high_to_low().filter(|layer| {
|
||||||
|
path.iter()
|
||||||
|
.try_fold(&layer.config, |value, key| match value {
|
||||||
|
toml::Value::Array(items) => {
|
||||||
|
key.parse::<usize>().ok().and_then(|i| items.get(i))
|
||||||
|
}
|
||||||
|
_ => value.get(key),
|
||||||
|
})
|
||||||
|
.is_some()
|
||||||
|
}) {
|
||||||
|
fields.insert((
|
||||||
|
format_config_layer_source(&layer.name, CONFIG_TOML_FILE),
|
||||||
|
path.clone(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Requirements use a different schema and lose unknown fields during
|
||||||
|
// composition. Inspect their original layers, with the same path bases.
|
||||||
|
// Normal loading remains responsible for reporting parse/type errors.
|
||||||
|
for layer in requirements {
|
||||||
|
let Ok((source, mut value, base_dir)) = layer.clone().into_raw_parts() else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
let _guard = base_dir
|
||||||
|
.as_ref()
|
||||||
|
.map(|base_dir| AbsolutePathBufGuard::new(base_dir.as_path()));
|
||||||
|
strip_cloud_auth_requirements(&source, &mut value);
|
||||||
|
fields.extend(
|
||||||
|
ignored_toml_value_fields::<ConfigRequirementsToml>(value)
|
||||||
|
.into_iter()
|
||||||
|
.map(|path| (format!("requirements ({source})"), path)),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
if fields.is_empty() {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let count = fields.len();
|
||||||
|
let setting = if count == 1 { "setting" } else { "settings" };
|
||||||
|
let mut warning = format!(
|
||||||
|
"Codex is ignoring {count} unrecognized configuration {setting}. Check for typos or deprecated settings."
|
||||||
|
);
|
||||||
|
for (source, path) in fields.iter().take(3) {
|
||||||
|
let source = bounded_label(source);
|
||||||
|
let key = bounded_label(&path.join("."));
|
||||||
|
let hint = match path
|
||||||
|
.iter()
|
||||||
|
.map(String::as_str)
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.as_slice()
|
||||||
|
{
|
||||||
|
["network_proxy"] => {
|
||||||
|
" Use [permissions.<name>.network] for network settings, or [experimental_network] in requirements.toml for enforced network policy."
|
||||||
|
}
|
||||||
|
["allowed_permissions"] => {
|
||||||
|
" Use [allowed_permission_profiles] and default_permissions; the old allowlist is ignored even when both forms are present."
|
||||||
|
}
|
||||||
|
["include_view_image_tool"]
|
||||||
|
| ["features", "include_view_image_tool"]
|
||||||
|
| ["profiles", _, "include_view_image_tool"]
|
||||||
|
| ["profiles", _, "features", "include_view_image_tool"] => {
|
||||||
|
" Use [features].view_image to configure the image tool."
|
||||||
|
}
|
||||||
|
_ => "",
|
||||||
|
};
|
||||||
|
let _ = write!(warning, "\n {source}: `{key}` is ignored.{hint}");
|
||||||
|
}
|
||||||
|
if fields.len() > 3 {
|
||||||
|
let remaining = fields.len() - 3;
|
||||||
|
let _ = write!(warning, "\n ... and {remaining} more ignored settings.");
|
||||||
|
}
|
||||||
|
Some(warning)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn bounded_label(value: &str) -> String {
|
||||||
|
let mut label = String::new();
|
||||||
|
for character in value.chars() {
|
||||||
|
if label.len() >= 200 {
|
||||||
|
label.push('…');
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if character.is_control() {
|
||||||
|
label.extend(character.escape_default());
|
||||||
|
} else {
|
||||||
|
label.push(character);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
label
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
#[path = "strict_config_tests.rs"]
|
#[path = "strict_config_tests.rs"]
|
||||||
mod tests;
|
mod tests;
|
||||||
@@ -1,4 +1,7 @@
|
|||||||
use super::*;
|
use super::*;
|
||||||
|
use crate::ConfigLayerEntry;
|
||||||
|
use crate::ConfigLayerSource;
|
||||||
|
use crate::ConfigRequirements;
|
||||||
use crate::config_toml::ConfigToml;
|
use crate::config_toml::ConfigToml;
|
||||||
use crate::diagnostics::TextPosition;
|
use crate::diagnostics::TextPosition;
|
||||||
use crate::diagnostics::TextRange;
|
use crate::diagnostics::TextRange;
|
||||||
@@ -150,3 +153,51 @@ collapsed = true"#;
|
|||||||
|
|
||||||
assert_eq!(error, None);
|
assert_eq!(error, None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn layer(source: ConfigLayerSource, contents: &str) -> ConfigLayerEntry {
|
||||||
|
ConfigLayerEntry::new(source, toml::from_str(contents).unwrap())
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn checks_merged_config_and_ignores_disabled_layers() {
|
||||||
|
let layers = vec![
|
||||||
|
layer(
|
||||||
|
ConfigLayerSource::EnterpriseManaged {
|
||||||
|
id: "cfg".into(),
|
||||||
|
name: "Defaults".into(),
|
||||||
|
},
|
||||||
|
r#"
|
||||||
|
[model_providers.custom]
|
||||||
|
name = "Custom"
|
||||||
|
wire_api = "responses"
|
||||||
|
unknown_timeout = 1
|
||||||
|
[model_providers.custom.http_headers]
|
||||||
|
custom_header = "accepted"
|
||||||
|
"#,
|
||||||
|
),
|
||||||
|
ConfigLayerEntry::new_disabled(
|
||||||
|
ConfigLayerSource::SessionFlags,
|
||||||
|
toml::from_str("disabled_unknown = true").unwrap(),
|
||||||
|
"not trusted",
|
||||||
|
),
|
||||||
|
layer(
|
||||||
|
ConfigLayerSource::SessionFlags,
|
||||||
|
r#"
|
||||||
|
[model_providers.custom]
|
||||||
|
unknown_timeout = 42
|
||||||
|
[profiles.work.features]
|
||||||
|
include_view_image_tool = false
|
||||||
|
"#,
|
||||||
|
),
|
||||||
|
];
|
||||||
|
let config = ConfigLayerStack::new(
|
||||||
|
layers,
|
||||||
|
ConfigRequirements::default(),
|
||||||
|
ConfigRequirementsToml::default(),
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
ignored_config_warning(&config, &[]).unwrap(),
|
||||||
|
"Codex is ignoring 3 unrecognized configuration settings. Check for typos or deprecated settings.\n enterprise-managed (Defaults, cfg): `model_providers.custom.unknown_timeout` is ignored.\n session-flags: `model_providers.custom.unknown_timeout` is ignored.\n session-flags: `profiles.work.features.include_view_image_tool` is ignored. Use [features].view_image to configure the image tool."
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -3727,7 +3727,7 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<
|
|||||||
tokio::fs::create_dir_all(nested.join(".codex")).await?;
|
tokio::fs::create_dir_all(nested.join(".codex")).await?;
|
||||||
tokio::fs::write(
|
tokio::fs::write(
|
||||||
nested.join(".codex").join(CONFIG_TOML_FILE),
|
nested.join(".codex").join(CONFIG_TOML_FILE),
|
||||||
r#"foo = "child"
|
r#"model = "child"
|
||||||
profile = "ignored"
|
profile = "ignored"
|
||||||
"#,
|
"#,
|
||||||
)
|
)
|
||||||
@@ -3749,7 +3749,7 @@ profile = "ignored"
|
|||||||
tokio::fs::write(
|
tokio::fs::write(
|
||||||
&untrusted_config_path,
|
&untrusted_config_path,
|
||||||
format!(
|
format!(
|
||||||
r#"foo = "user"
|
r#"model = "user"
|
||||||
{untrusted_config_contents}"#
|
{untrusted_config_contents}"#
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
@@ -3774,7 +3774,7 @@ profile = "ignored"
|
|||||||
"expected untrusted project layer to be disabled"
|
"expected untrusted project layer to be disabled"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
project_layers_untrusted[0].config.get("foo"),
|
project_layers_untrusted[0].config.get("model"),
|
||||||
Some(&TomlValue::String("child".to_string()))
|
Some(&TomlValue::String("child".to_string()))
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
@@ -3782,7 +3782,7 @@ profile = "ignored"
|
|||||||
"expected unsupported project config keys to be ignored even when the layer is disabled"
|
"expected unsupported project config keys to be ignored even when the layer is disabled"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
layers_untrusted.effective_config().get("foo"),
|
layers_untrusted.effective_config().get("model"),
|
||||||
Some(&TomlValue::String("user".to_string()))
|
Some(&TomlValue::String("user".to_string()))
|
||||||
);
|
);
|
||||||
let empty_warnings: &[String] = &[];
|
let empty_warnings: &[String] = &[];
|
||||||
@@ -3792,7 +3792,7 @@ profile = "ignored"
|
|||||||
tokio::fs::create_dir_all(&codex_home_unknown).await?;
|
tokio::fs::create_dir_all(&codex_home_unknown).await?;
|
||||||
tokio::fs::write(
|
tokio::fs::write(
|
||||||
codex_home_unknown.join(CONFIG_TOML_FILE),
|
codex_home_unknown.join(CONFIG_TOML_FILE),
|
||||||
r#"foo = "user"
|
r#"model = "user"
|
||||||
"#,
|
"#,
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
@@ -3816,7 +3816,7 @@ profile = "ignored"
|
|||||||
"expected unknown-trust project layer to be disabled"
|
"expected unknown-trust project layer to be disabled"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
project_layers_unknown[0].config.get("foo"),
|
project_layers_unknown[0].config.get("model"),
|
||||||
Some(&TomlValue::String("child".to_string()))
|
Some(&TomlValue::String("child".to_string()))
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
@@ -3824,7 +3824,7 @@ profile = "ignored"
|
|||||||
"expected unsupported project config keys to be ignored even when the layer is disabled"
|
"expected unsupported project config keys to be ignored even when the layer is disabled"
|
||||||
);
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
layers_unknown.effective_config().get("foo"),
|
layers_unknown.effective_config().get("model"),
|
||||||
Some(&TomlValue::String("user".to_string()))
|
Some(&TomlValue::String("user".to_string()))
|
||||||
);
|
);
|
||||||
assert_eq!(layers_unknown.startup_warnings(), Some(empty_warnings));
|
assert_eq!(layers_unknown.startup_warnings(), Some(empty_warnings));
|
||||||
|
|||||||
@@ -241,6 +241,9 @@ impl ChatWidget {
|
|||||||
if self.warning_display_state.startup_complete {
|
if self.warning_display_state.startup_complete {
|
||||||
self.on_warning(message);
|
self.on_warning(message);
|
||||||
} else if self.warning_display_state.should_display(&message) {
|
} else if self.warning_display_state.should_display(&message) {
|
||||||
|
self.warning_display_state
|
||||||
|
.startup_config_warnings
|
||||||
|
.insert(message.clone());
|
||||||
self.add_to_history(history_cell::StartupWarningsCell::new(vec![message]));
|
self.add_to_history(history_cell::StartupWarningsCell::new(vec![message]));
|
||||||
self.request_redraw();
|
self.request_redraw();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1008,26 +1008,31 @@ async fn config_warning_during_turn_remains_inline() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn live_app_server_config_warning_prefixes_summary() {
|
async fn startup_config_warning_is_not_repeated_by_thread() {
|
||||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||||
|
let message = "Codex is ignoring 1 unrecognized configuration setting. Check for typos or deprecated settings.";
|
||||||
|
|
||||||
chat.handle_server_notification(
|
for notification in [
|
||||||
ServerNotification::ConfigWarning(ConfigWarningNotification {
|
ServerNotification::ConfigWarning(ConfigWarningNotification {
|
||||||
summary: "Invalid configuration; using defaults.".to_string(),
|
summary: message.to_string(),
|
||||||
details: None,
|
details: None,
|
||||||
path: None,
|
path: None,
|
||||||
range: None,
|
range: None,
|
||||||
}),
|
}),
|
||||||
/*replay_kind*/ None,
|
ServerNotification::Warning(WarningNotification {
|
||||||
);
|
thread_id: Some("thread-1".to_string()),
|
||||||
|
message: message.to_string(),
|
||||||
|
}),
|
||||||
|
] {
|
||||||
|
chat.handle_server_notification(notification, /*replay_kind*/ None);
|
||||||
|
}
|
||||||
|
|
||||||
let cells = drain_insert_history_transcript(&mut rx);
|
let cells = drain_insert_history_transcript(&mut rx);
|
||||||
assert_eq!(cells.len(), 1, "expected one warning history cell");
|
assert_eq!(cells.len(), 1);
|
||||||
let rendered = lines_to_single_string(&cells[0]);
|
insta::assert_snapshot!(lines_to_single_string(&cells[0]), @"
|
||||||
assert!(
|
⚠ Codex is ignoring 1 unrecognized configuration setting. Check for typos or
|
||||||
rendered.contains("Invalid configuration; using defaults."),
|
deprecated settings.
|
||||||
"expected config warning summary, got {rendered}"
|
");
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -8,13 +8,16 @@ const FALLBACK_MODEL_METADATA_WARNING_SUFFIX: &str =
|
|||||||
pub(super) struct WarningDisplayState {
|
pub(super) struct WarningDisplayState {
|
||||||
/// Completed resume history does not end startup; active work does.
|
/// Completed resume history does not end startup; active work does.
|
||||||
pub(super) startup_complete: bool,
|
pub(super) startup_complete: bool,
|
||||||
|
/// Initialization config warnings may also arrive as ordinary thread warnings.
|
||||||
|
pub(super) startup_config_warnings: HashSet<String>,
|
||||||
fallback_model_metadata_slugs: HashSet<String>,
|
fallback_model_metadata_slugs: HashSet<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl WarningDisplayState {
|
impl WarningDisplayState {
|
||||||
pub(super) fn should_display(&mut self, message: &str) -> bool {
|
pub(super) fn should_display(&mut self, message: &str) -> bool {
|
||||||
fallback_model_metadata_warning_slug(message)
|
!self.startup_config_warnings.contains(message)
|
||||||
.is_none_or(|slug| self.fallback_model_metadata_slugs.insert(slug.to_string()))
|
&& fallback_model_metadata_warning_slug(message)
|
||||||
|
.is_none_or(|slug| self.fallback_model_metadata_slugs.insert(slug.to_string()))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user