mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Centralize TOML config deserialization (#58198)
## What changed - Add a non-inlined `TryFrom<toml::Value>` implementation for `ConfigToml` and route callers through it to share the generated deserializer in `codex-config`. - Reuse the ignored-field deserialization helper for strict validation, returning path-aware errors while preserving existing fallbacks for CLI validation and ignored-config warnings. GitOrigin-RevId: 0208b314416ad4d3d7fbf3a30560a0c8c4035abd
This commit is contained in:
committed by
copyberry
parent
2f1e4f99e0
commit
b91cedc080
@@ -145,9 +145,7 @@ impl ConfigManager {
|
||||
})?,
|
||||
};
|
||||
|
||||
let effective = layers.effective_config();
|
||||
let mut effective_config_toml: ConfigToml = effective
|
||||
.try_into()
|
||||
let mut effective_config_toml = ConfigToml::try_from(layers.effective_config())
|
||||
.map_err(|err| ConfigManagerError::toml("invalid configuration", err))?;
|
||||
layers
|
||||
.requirements_toml()
|
||||
@@ -861,7 +859,7 @@ fn toml_value_to_value(value: &TomlValue) -> anyhow::Result<toml_edit::Value> {
|
||||
}
|
||||
|
||||
fn validate_config(value: &TomlValue) -> anyhow::Result<()> {
|
||||
let config: ConfigToml = value.clone().try_into()?;
|
||||
let config = ConfigToml::try_from(value.clone())?;
|
||||
if config.approval_policy == Some(AskForApproval::UnlessTrusted) {
|
||||
anyhow::bail!(
|
||||
"approval_policy = \"untrusted\" is no longer supported; remove this setting"
|
||||
|
||||
@@ -846,6 +846,16 @@ pub fn effective_sandbox_mode(
|
||||
}
|
||||
}
|
||||
|
||||
impl TryFrom<toml::Value> for ConfigToml {
|
||||
type Error = toml::de::Error;
|
||||
|
||||
// Keep the generated TOML deserializer in this crate, shared by all callers.
|
||||
#[inline(never)]
|
||||
fn try_from(value: toml::Value) -> Result<Self, Self::Error> {
|
||||
value.try_into()
|
||||
}
|
||||
}
|
||||
|
||||
impl ConfigToml {
|
||||
/// Derive the effective permission profile from sandbox config.
|
||||
///
|
||||
|
||||
@@ -663,6 +663,7 @@ fn validate_cli_overrides_strictly(
|
||||
) -> io::Result<()> {
|
||||
let _guard = AbsolutePathBufGuard::new(base_dir);
|
||||
let ignored_path = ignored_toml_value_fields::<ConfigToml>(cli_overrides_layer.clone())
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.chain(unknown_tui_toml_value_path(cli_overrides_layer))
|
||||
.next()
|
||||
@@ -1634,7 +1635,7 @@ pub fn resolve_relative_paths_in_config_toml(
|
||||
}
|
||||
// The typed round trip resolves AbsolutePathBuf fields against this layer's directory.
|
||||
let _guard = AbsolutePathBufGuard::new(base_dir);
|
||||
let Ok(resolved) = value_from_config_toml.clone().try_into::<ConfigToml>() else {
|
||||
let Ok(resolved) = ConfigToml::try_from(value_from_config_toml.clone()) else {
|
||||
return Ok(value_from_config_toml);
|
||||
};
|
||||
drop(_guard);
|
||||
|
||||
@@ -75,18 +75,8 @@ fn config_error_from_ignored_toml_value_fields_for_source<T: DeserializeOwned +
|
||||
} 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);
|
||||
if !path_segments.is_empty() {
|
||||
ignored_paths.push(path_segments);
|
||||
}
|
||||
};
|
||||
let deserializer = serde_ignored::Deserializer::new(value, &mut ignored_callback);
|
||||
let result: Result<T, _> = serde_path_to_error::deserialize(deserializer);
|
||||
|
||||
match result {
|
||||
Ok(_) => unknown_field_error_from_paths(source, contents, ignored_paths)
|
||||
match ignored_toml_value_fields::<T>(value) {
|
||||
Ok(ignored_paths) => 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) => {
|
||||
@@ -128,19 +118,19 @@ pub(crate) fn unknown_tui_toml_value_path(value: &TomlValue) -> Vec<Vec<String>>
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn ignored_toml_value_fields<T: DeserializeOwned>(value: TomlValue) -> Vec<Vec<String>> {
|
||||
pub(crate) fn ignored_toml_value_fields<T: DeserializeOwned>(
|
||||
value: TomlValue,
|
||||
) -> Result<Vec<Vec<String>>, serde_path_to_error::Error<toml::de::Error>> {
|
||||
let mut ignored_paths = Vec::new();
|
||||
let result: Result<T, _> = serde_ignored::deserialize(value, |ignored_path| {
|
||||
let path_segments = ignored_path_segments(&ignored_path);
|
||||
if !path_segments.is_empty() {
|
||||
ignored_paths.push(path_segments);
|
||||
let mut record_path = |path: serde_ignored::Path<'_>| {
|
||||
let segments = ignored_path_segments(&path);
|
||||
if !segments.is_empty() {
|
||||
ignored_paths.push(segments);
|
||||
}
|
||||
});
|
||||
if result.is_err() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
ignored_paths
|
||||
};
|
||||
let deserializer = serde_ignored::Deserializer::new(value, &mut record_path);
|
||||
serde_path_to_error::deserialize::<_, T>(deserializer)?;
|
||||
Ok(ignored_paths)
|
||||
}
|
||||
|
||||
pub(crate) fn unknown_feature_toml_value_field(value: &TomlValue) -> Option<String> {
|
||||
@@ -246,6 +236,7 @@ pub(crate) fn ignored_config_warning(
|
||||
let effective = config.effective_config();
|
||||
let unknown_features = unknown_feature_toml_value_path(&effective);
|
||||
for path in ignored_toml_value_fields::<ConfigToml>(effective)
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.chain(unknown_features)
|
||||
{
|
||||
@@ -286,12 +277,14 @@ pub(crate) fn ignored_config_warning(
|
||||
.cloned();
|
||||
fields.extend(
|
||||
ignored_toml_value_fields::<ConfigRequirementsToml>(value)
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(|path| (format!("requirements ({source})"), path)),
|
||||
);
|
||||
if let Some(network) = permissions_network {
|
||||
fields.extend(
|
||||
ignored_toml_value_fields::<NetworkRequirementsToml>(network)
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(|path| {
|
||||
let mut full_path = vec!["permissions".to_string(), "network".to_string()];
|
||||
|
||||
@@ -1634,7 +1634,7 @@ impl ConfigBuilder {
|
||||
|
||||
async fn config_toml_from_layers(layers: &ConfigLayerStack) -> std::io::Result<ConfigToml> {
|
||||
// The loader resolves paths relative to each layer's file before deserialization.
|
||||
match layers.effective_config().try_into() {
|
||||
match ConfigToml::try_from(layers.effective_config()) {
|
||||
Ok(config_toml) => Ok(config_toml),
|
||||
Err(err) => {
|
||||
if let Some(config_error) = codex_config::first_layer_config_error::<ConfigToml>(
|
||||
@@ -2240,8 +2240,7 @@ pub fn deserialize_config_toml_with_base(
|
||||
// This guard ensures that any relative paths that is deserialized into an
|
||||
// [AbsolutePathBuf] is resolved against `config_base_dir`.
|
||||
let _guard = AbsolutePathBufGuard::new(config_base_dir);
|
||||
root_value
|
||||
.try_into()
|
||||
ConfigToml::try_from(root_value)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
|
||||
}
|
||||
|
||||
@@ -4924,11 +4923,8 @@ impl Config {
|
||||
&self,
|
||||
profile_name: &str,
|
||||
) -> std::io::Result<PermissionProfileToml> {
|
||||
let cfg: ConfigToml = self
|
||||
.config_layer_stack
|
||||
.effective_config()
|
||||
.try_into()
|
||||
.map_err(|err| {
|
||||
let cfg =
|
||||
ConfigToml::try_from(self.config_layer_stack.effective_config()).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidInput,
|
||||
format!(
|
||||
@@ -4952,11 +4948,8 @@ impl Config {
|
||||
) -> std::io::Result<Option<NetworkProxySpec>> {
|
||||
let profile_allows_network_proxy =
|
||||
profile_allows_configured_network_proxy(permission_profile);
|
||||
let cfg: ConfigToml = self
|
||||
.config_layer_stack
|
||||
.effective_config()
|
||||
.try_into()
|
||||
.map_err(|err| {
|
||||
let cfg =
|
||||
ConfigToml::try_from(self.config_layer_stack.effective_config()).map_err(|err| {
|
||||
std::io::Error::new(
|
||||
ErrorKind::InvalidInput,
|
||||
format!(
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::config::Config;
|
||||
use crate::config::TokenBudgetConfig;
|
||||
use crate::config::resolve_token_budget_config;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use codex_config::config_toml::ConfigToml;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::account::PlanType;
|
||||
@@ -42,10 +43,7 @@ pub(super) fn apply_experimental_context(
|
||||
}
|
||||
|
||||
if config.token_budget.is_none() {
|
||||
let config_toml = config
|
||||
.config_layer_stack
|
||||
.effective_config()
|
||||
.try_into()
|
||||
let config_toml = ConfigToml::try_from(config.config_layer_stack.effective_config())
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
|
||||
config.token_budget = resolve_token_budget_config(&config_toml, &config.features)?;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user