From d6dac54a5d56fe4be69885e51f2d90fae88a169c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 10 Feb 2026 23:44:39 -0800 Subject: [PATCH] Extract `codex-config` from `codex-core` ## Why `codex-core` has accumulated config loading, requirements parsing, constraint logic, and config-layer state handling in one large crate. This refactor pulls that cohesive subsystem into a dedicated crate so we can reduce the compile/test surface area of `codex-core` and make future config work more isolated. This is part of the broader goal of right-sizing crates to reduce monolithic rebuild cost and improve incremental development speed. ## What Changed ### New crate - Added a new workspace crate: `codex-rs/config` (`codex-config`) - Added workspace wiring in `codex-rs/Cargo.toml` - Added dependency from `codex-core` to `codex-config` ### Moved config internals from `core` to `config` Moved these modules into `codex-config`: - `core/src/config/constraint.rs` -> `config/src/constraint.rs` - `core/src/config_loader/cloud_requirements.rs` -> `config/src/config_loader/cloud_requirements.rs` - `core/src/config_loader/config_requirements.rs` -> `config/src/config_loader/config_requirements.rs` - `core/src/config_loader/fingerprint.rs` -> `config/src/config_loader/fingerprint.rs` - `core/src/config_loader/merge.rs` -> `config/src/config_loader/merge.rs` - `core/src/config_loader/overrides.rs` -> `config/src/config_loader/overrides.rs` - `core/src/config_loader/requirements_exec_policy.rs` -> `config/src/config_loader/requirements_exec_policy.rs` - `core/src/config_loader/state.rs` -> `config/src/config_loader/state.rs` ### Removed shim modules in `core` After the move, the temporary one-line `pub use` shim files under `core/src/config_loader/` were deleted so history is a clean move/delete rather than introducing extra permanent forwarding modules. `core/src/config_loader/mod.rs` now imports/re-exports directly from `codex_config`, including direct use of `build_cli_overrides_layer` and test-only access to `version_for_toml`. ### Follow-on fixes for direct imports - Updated `core/src/config_loader/macos.rs` to use `super::{ConfigRequirementsToml, ConfigRequirementsWithSources, RequirementSource}`. - Updated `core/src/config_loader/tests.rs` imports to reference `crate::config_loader` re-exports and `codex_config` exec-policy TOML types. ## Behavior and API Notes - Config behavior is intended to be unchanged. - `codex-core` continues to expose the same config-loader-facing API surface to its internal callers via `core/src/config_loader/mod.rs` re-exports. - The main functional change is crate ownership and dependency direction, not config semantics. ## Validation Ran: - `cargo test -p codex-config` - `cargo test -p codex-core --no-run` - `cargo test -p codex-core config_loader::tests::load_requirements_toml_produces_expected_constraints` - `cargo test -p codex-core config_loader::tests::requirements_exec_policy_tests::parses_single_prefix_rule_from_raw_toml` - `just fmt` - `just fix -p codex-config -p codex-core` - `just fix -p codex-core` All listed commands completed successfully in this workspace environment. --- codex-rs/Cargo.lock | 22 +- codex-rs/Cargo.toml | 2 + codex-rs/config/BUILD.bazel | 6 + codex-rs/config/Cargo.toml | 26 ++ .../src/config_loader/cloud_requirements.rs | 0 .../src/config_loader/config_requirements.rs | 17 +- .../src/config_loader/fingerprint.rs | 2 +- .../src/config_loader/merge.rs | 0 codex-rs/config/src/config_loader/mod.rs | 29 +++ .../src/config_loader/overrides.rs | 2 +- .../config_loader/requirements_exec_policy.rs | 2 +- .../src/config_loader/state.rs | 0 codex-rs/config/src/constraint.rs | 241 +++++++++++++++++ codex-rs/config/src/lib.rs | 33 +++ codex-rs/core/Cargo.toml | 2 +- codex-rs/core/src/config/constraint.rs | 244 +----------------- codex-rs/core/src/config_loader/macos.rs | 6 +- codex-rs/core/src/config_loader/mod.rs | 51 ++-- codex-rs/core/src/config_loader/tests.rs | 18 +- codex-rs/tui/src/debug_config.rs | 86 +++--- 20 files changed, 456 insertions(+), 333 deletions(-) create mode 100644 codex-rs/config/BUILD.bazel create mode 100644 codex-rs/config/Cargo.toml rename codex-rs/{core => config}/src/config_loader/cloud_requirements.rs (100%) rename codex-rs/{core => config}/src/config_loader/config_requirements.rs (99%) rename codex-rs/{core => config}/src/config_loader/fingerprint.rs (97%) rename codex-rs/{core => config}/src/config_loader/merge.rs (100%) create mode 100644 codex-rs/config/src/config_loader/mod.rs rename codex-rs/{core => config}/src/config_loader/overrides.rs (94%) rename codex-rs/{core => config}/src/config_loader/requirements_exec_policy.rs (99%) rename codex-rs/{core => config}/src/config_loader/state.rs (100%) create mode 100644 codex-rs/config/src/constraint.rs create mode 100644 codex-rs/config/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 36ee957471..322b01c70a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1638,6 +1638,26 @@ dependencies = [ "toml 0.9.12+spec-1.1.0", ] +[[package]] +name = "codex-config" +version = "0.0.0" +dependencies = [ + "anyhow", + "codex-app-server-protocol", + "codex-execpolicy", + "codex-protocol", + "codex-utils-absolute-path", + "futures", + "multimap", + "pretty_assertions", + "serde", + "serde_json", + "sha2", + "thiserror 2.0.18", + "tokio", + "toml 0.9.12+spec-1.1.0", +] + [[package]] name = "codex-core" version = "0.0.0" @@ -1660,6 +1680,7 @@ dependencies = [ "codex-arg0", "codex-async-utils", "codex-client", + "codex-config", "codex-execpolicy", "codex-file-search", "codex-git", @@ -1696,7 +1717,6 @@ dependencies = [ "landlock", "libc", "maplit", - "multimap", "notify", "once_cell", "openssl-sys", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index e7b30353f2..6b7bcd9f56 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -16,6 +16,7 @@ members = [ "cloud-tasks-client", "cli", "common", + "config", "shell-command", "core", "hooks", @@ -83,6 +84,7 @@ codex-chatgpt = { path = "chatgpt" } codex-cli = { path = "cli"} codex-client = { path = "codex-client" } codex-common = { path = "common" } +codex-config = { path = "config" } codex-shell-command = { path = "shell-command" } codex-core = { path = "core" } codex-hooks = { path = "hooks" } diff --git a/codex-rs/config/BUILD.bazel b/codex-rs/config/BUILD.bazel new file mode 100644 index 0000000000..2b832782b0 --- /dev/null +++ b/codex-rs/config/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "config", + crate_name = "codex_config", +) diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml new file mode 100644 index 0000000000..1c804fb8db --- /dev/null +++ b/codex-rs/config/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "codex-config" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +codex-app-server-protocol = { workspace = true } +codex-execpolicy = { workspace = true } +codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } +futures = { workspace = true, features = ["alloc", "std"] } +multimap = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +sha2 = { workspace = true } +thiserror = { workspace = true } +toml = { workspace = true } + +[dev-dependencies] +anyhow = { workspace = true } +pretty_assertions = { workspace = true } +tokio = { workspace = true, features = ["full"] } diff --git a/codex-rs/core/src/config_loader/cloud_requirements.rs b/codex-rs/config/src/config_loader/cloud_requirements.rs similarity index 100% rename from codex-rs/core/src/config_loader/cloud_requirements.rs rename to codex-rs/config/src/config_loader/cloud_requirements.rs diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/config/src/config_loader/config_requirements.rs similarity index 99% rename from codex-rs/core/src/config_loader/config_requirements.rs rename to codex-rs/config/src/config_loader/config_requirements.rs index 6ba68afebe..76b8812c25 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/config/src/config_loader/config_requirements.rs @@ -10,8 +10,8 @@ use std::fmt; use super::requirements_exec_policy::RequirementsExecPolicy; use super::requirements_exec_policy::RequirementsExecPolicyToml; -use crate::config::Constrained; -use crate::config::ConstraintError; +use crate::Constrained; +use crate::ConstraintError; #[derive(Debug, Clone, PartialEq, Eq)] pub enum RequirementSource { @@ -80,7 +80,7 @@ pub struct ConfigRequirements { pub sandbox_policy: ConstrainedWithSource, pub web_search_mode: ConstrainedWithSource, pub mcp_servers: Option>>, - pub(crate) exec_policy: Option>, + pub exec_policy: Option>, pub enforce_residency: ConstrainedWithSource>, /// Managed network constraints derived from requirements. pub network: Option>, @@ -560,7 +560,6 @@ impl TryFrom for ConfigRequirements { #[cfg(test)] mod tests { use super::*; - use crate::config_loader::system_requirements_toml_file; use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; @@ -574,6 +573,12 @@ mod tests { cmd.iter().map(std::string::ToString::to_string).collect() } + fn requirements_toml_file() -> Result { + Ok(AbsolutePathBuf::try_from( + std::env::temp_dir().join("requirements.toml"), + )?) + } + fn with_unknown_source(toml: ConfigRequirementsToml) -> ConfigRequirementsWithSources { let ConfigRequirementsToml { allowed_approval_policies, @@ -732,7 +737,7 @@ mod tests { "#, )?; - let requirements_toml_file = system_requirements_toml_file()?; + let requirements_toml_file = requirements_toml_file()?; let source_location = RequirementSource::SystemRequirementsToml { file: requirements_toml_file, }; @@ -1149,7 +1154,7 @@ mod tests { ] "#; let config: ConfigRequirementsToml = from_str(toml_str)?; - let requirements_toml_file = system_requirements_toml_file()?; + let requirements_toml_file = requirements_toml_file()?; let source_location = RequirementSource::SystemRequirementsToml { file: requirements_toml_file, }; diff --git a/codex-rs/core/src/config_loader/fingerprint.rs b/codex-rs/config/src/config_loader/fingerprint.rs similarity index 97% rename from codex-rs/core/src/config_loader/fingerprint.rs rename to codex-rs/config/src/config_loader/fingerprint.rs index c072134fde..d8e0263389 100644 --- a/codex-rs/core/src/config_loader/fingerprint.rs +++ b/codex-rs/config/src/config_loader/fingerprint.rs @@ -34,7 +34,7 @@ pub(super) fn record_origins( } } -pub(super) fn version_for_toml(value: &TomlValue) -> String { +pub fn version_for_toml(value: &TomlValue) -> String { let json = serde_json::to_value(value).unwrap_or(JsonValue::Null); let canonical = canonical_json(&json); let serialized = serde_json::to_vec(&canonical).unwrap_or_default(); diff --git a/codex-rs/core/src/config_loader/merge.rs b/codex-rs/config/src/config_loader/merge.rs similarity index 100% rename from codex-rs/core/src/config_loader/merge.rs rename to codex-rs/config/src/config_loader/merge.rs diff --git a/codex-rs/config/src/config_loader/mod.rs b/codex-rs/config/src/config_loader/mod.rs new file mode 100644 index 0000000000..8a1d6cdfdd --- /dev/null +++ b/codex-rs/config/src/config_loader/mod.rs @@ -0,0 +1,29 @@ +pub mod cloud_requirements; +pub mod config_requirements; +pub mod fingerprint; +pub mod merge; +pub mod overrides; +pub mod requirements_exec_policy; +pub mod state; + +pub use cloud_requirements::CloudRequirementsLoader; +pub use config_requirements::ConfigRequirements; +pub use config_requirements::ConfigRequirementsToml; +pub use config_requirements::ConfigRequirementsWithSources; +pub use config_requirements::ConstrainedWithSource; +pub use config_requirements::McpServerIdentity; +pub use config_requirements::McpServerRequirement; +pub use config_requirements::NetworkConstraints; +pub use config_requirements::NetworkRequirementsToml; +pub use config_requirements::RequirementSource; +pub use config_requirements::ResidencyRequirement; +pub use config_requirements::SandboxModeRequirement; +pub use config_requirements::Sourced; +pub use config_requirements::WebSearchModeRequirement; +pub use fingerprint::version_for_toml; +pub use merge::merge_toml_values; +pub use overrides::build_cli_overrides_layer; +pub use state::ConfigLayerEntry; +pub use state::ConfigLayerStack; +pub use state::ConfigLayerStackOrdering; +pub use state::LoaderOverrides; diff --git a/codex-rs/core/src/config_loader/overrides.rs b/codex-rs/config/src/config_loader/overrides.rs similarity index 94% rename from codex-rs/core/src/config_loader/overrides.rs rename to codex-rs/config/src/config_loader/overrides.rs index a9fe8eff9e..a27caedb9a 100644 --- a/codex-rs/core/src/config_loader/overrides.rs +++ b/codex-rs/config/src/config_loader/overrides.rs @@ -4,7 +4,7 @@ pub(crate) fn default_empty_table() -> TomlValue { TomlValue::Table(Default::default()) } -pub(crate) fn build_cli_overrides_layer(cli_overrides: &[(String, TomlValue)]) -> TomlValue { +pub fn build_cli_overrides_layer(cli_overrides: &[(String, TomlValue)]) -> TomlValue { let mut root = default_empty_table(); for (path, value) in cli_overrides { apply_toml_override(&mut root, path, value.clone()); diff --git a/codex-rs/core/src/config_loader/requirements_exec_policy.rs b/codex-rs/config/src/config_loader/requirements_exec_policy.rs similarity index 99% rename from codex-rs/core/src/config_loader/requirements_exec_policy.rs rename to codex-rs/config/src/config_loader/requirements_exec_policy.rs index 74546fc426..64d60f8814 100644 --- a/codex-rs/core/src/config_loader/requirements_exec_policy.rs +++ b/codex-rs/config/src/config_loader/requirements_exec_policy.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use thiserror::Error; #[derive(Debug, Clone)] -pub(crate) struct RequirementsExecPolicy { +pub struct RequirementsExecPolicy { policy: Policy, } diff --git a/codex-rs/core/src/config_loader/state.rs b/codex-rs/config/src/config_loader/state.rs similarity index 100% rename from codex-rs/core/src/config_loader/state.rs rename to codex-rs/config/src/config_loader/state.rs diff --git a/codex-rs/config/src/constraint.rs b/codex-rs/config/src/constraint.rs new file mode 100644 index 0000000000..23b6c57c74 --- /dev/null +++ b/codex-rs/config/src/constraint.rs @@ -0,0 +1,241 @@ +use std::fmt; +use std::sync::Arc; + +use crate::config_loader::RequirementSource; +use thiserror::Error; + +#[derive(Debug, Error, PartialEq, Eq)] +pub enum ConstraintError { + #[error( + "invalid value for `{field_name}`: `{candidate}` is not in the allowed set {allowed} (set by {requirement_source})" + )] + InvalidValue { + field_name: &'static str, + candidate: String, + allowed: String, + requirement_source: RequirementSource, + }, + + #[error("field `{field_name}` cannot be empty")] + EmptyField { field_name: String }, + + #[error("invalid rules in requirements (set by {requirement_source}): {reason}")] + ExecPolicyParse { + requirement_source: RequirementSource, + reason: String, + }, +} + +impl ConstraintError { + pub fn empty_field(field_name: impl Into) -> Self { + Self::EmptyField { + field_name: field_name.into(), + } + } +} + +pub type ConstraintResult = Result; + +impl From for std::io::Error { + fn from(err: ConstraintError) -> Self { + std::io::Error::new(std::io::ErrorKind::InvalidInput, err) + } +} + +type ConstraintValidator = dyn Fn(&T) -> ConstraintResult<()> + Send + Sync; +/// A ConstraintNormalizer is a function which transforms a value into another of the same type. +/// `Constrained` uses normalizers to transform values to satisfy constraints or enforce values. +type ConstraintNormalizer = dyn Fn(T) -> T + Send + Sync; + +#[derive(Clone)] +pub struct Constrained { + value: T, + validator: Arc>, + normalizer: Option>>, +} + +impl Constrained { + pub fn new( + initial_value: T, + validator: impl Fn(&T) -> ConstraintResult<()> + Send + Sync + 'static, + ) -> ConstraintResult { + let validator: Arc> = Arc::new(validator); + validator(&initial_value)?; + Ok(Self { + value: initial_value, + validator, + normalizer: None, + }) + } + + /// normalized creates a `Constrained` value with a normalizer function and a validator that allows any value. + pub fn normalized( + initial_value: T, + normalizer: impl Fn(T) -> T + Send + Sync + 'static, + ) -> ConstraintResult { + let validator: Arc> = Arc::new(|_| Ok(())); + let normalizer: Arc> = Arc::new(normalizer); + let normalized = normalizer(initial_value); + validator(&normalized)?; + Ok(Self { + value: normalized, + validator, + normalizer: Some(normalizer), + }) + } + + pub fn allow_any(initial_value: T) -> Self { + Self { + value: initial_value, + validator: Arc::new(|_| Ok(())), + normalizer: None, + } + } + + /// Allow any value of T, using T's Default as the initial value. + pub fn allow_any_from_default() -> Self + where + T: Default, + { + Self::allow_any(T::default()) + } + + pub fn get(&self) -> &T { + &self.value + } + + pub fn value(&self) -> T + where + T: Copy, + { + self.value + } + + pub fn can_set(&self, candidate: &T) -> ConstraintResult<()> { + (self.validator)(candidate) + } + + pub fn set(&mut self, value: T) -> ConstraintResult<()> { + let value = if let Some(normalizer) = &self.normalizer { + normalizer(value) + } else { + value + }; + (self.validator)(&value)?; + self.value = value; + Ok(()) + } +} + +impl std::ops::Deref for Constrained { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.value + } +} + +impl fmt::Debug for Constrained { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Constrained") + .field("value", &self.value) + .finish() + } +} + +impl PartialEq for Constrained { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn invalid_value(candidate: impl Into, allowed: impl Into) -> ConstraintError { + ConstraintError::InvalidValue { + field_name: "", + candidate: candidate.into(), + allowed: allowed.into(), + requirement_source: RequirementSource::Unknown, + } + } + + #[test] + fn constrained_allow_any_accepts_any_value() { + let mut constrained = Constrained::allow_any(5); + constrained.set(-10).expect("allow any accepts all values"); + assert_eq!(constrained.value(), -10); + } + + #[test] + fn constrained_allow_any_default_uses_default_value() { + let constrained = Constrained::::allow_any_from_default(); + assert_eq!(constrained.value(), 0); + } + + #[test] + fn constrained_normalizer_applies_on_init_and_set() -> anyhow::Result<()> { + let mut constrained = Constrained::normalized(-1, |value| value.max(0))?; + assert_eq!(constrained.value(), 0); + constrained.set(-5)?; + assert_eq!(constrained.value(), 0); + constrained.set(10)?; + assert_eq!(constrained.value(), 10); + Ok(()) + } + + #[test] + fn constrained_new_rejects_invalid_initial_value() { + let result = Constrained::new(0, |value| { + if *value > 0 { + Ok(()) + } else { + Err(invalid_value(value.to_string(), "positive values")) + } + }); + + assert_eq!(result, Err(invalid_value("0", "positive values"))); + } + + #[test] + fn constrained_set_rejects_invalid_value_and_leaves_previous() { + let mut constrained = Constrained::new(1, |value| { + if *value > 0 { + Ok(()) + } else { + Err(invalid_value(value.to_string(), "positive values")) + } + }) + .expect("initial value should be accepted"); + + let err = constrained + .set(-5) + .expect_err("negative values should be rejected"); + assert_eq!(err, invalid_value("-5", "positive values")); + assert_eq!(constrained.value(), 1); + } + + #[test] + fn constrained_can_set_allows_probe_without_setting() { + let constrained = Constrained::new(1, |value| { + if *value > 0 { + Ok(()) + } else { + Err(invalid_value(value.to_string(), "positive values")) + } + }) + .expect("initial value should be accepted"); + + constrained + .can_set(&2) + .expect("can_set should accept positive value"); + let err = constrained + .can_set(&-1) + .expect_err("can_set should reject negative value"); + assert_eq!(err, invalid_value("-1", "positive values")); + assert_eq!(constrained.value(), 1); + } +} diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs new file mode 100644 index 0000000000..0ab4e1b199 --- /dev/null +++ b/codex-rs/config/src/lib.rs @@ -0,0 +1,33 @@ +pub mod config_loader; +mod constraint; + +pub use config_loader::CloudRequirementsLoader; +pub use config_loader::ConfigLayerEntry; +pub use config_loader::ConfigLayerStack; +pub use config_loader::ConfigLayerStackOrdering; +pub use config_loader::ConfigRequirements; +pub use config_loader::ConfigRequirementsToml; +pub use config_loader::ConfigRequirementsWithSources; +pub use config_loader::ConstrainedWithSource; +pub use config_loader::LoaderOverrides; +pub use config_loader::McpServerIdentity; +pub use config_loader::McpServerRequirement; +pub use config_loader::NetworkConstraints; +pub use config_loader::NetworkRequirementsToml; +pub use config_loader::RequirementSource; +pub use config_loader::ResidencyRequirement; +pub use config_loader::SandboxModeRequirement; +pub use config_loader::Sourced; +pub use config_loader::WebSearchModeRequirement; +pub use config_loader::build_cli_overrides_layer; +pub use config_loader::merge_toml_values; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicy; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicyDecisionToml; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicyParseError; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; +pub use config_loader::requirements_exec_policy::RequirementsExecPolicyToml; +pub use config_loader::version_for_toml; +pub use constraint::Constrained; +pub use constraint::ConstraintError; +pub use constraint::ConstraintResult; diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index e759072fac..7fa128399b 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -33,6 +33,7 @@ codex-app-server-protocol = { workspace = true } codex-apply-patch = { workspace = true } codex-async-utils = { workspace = true } codex-client = { workspace = true } +codex-config = { workspace = true } codex-shell-command = { workspace = true } codex-execpolicy = { workspace = true } codex-file-search = { workspace = true } @@ -62,7 +63,6 @@ indexmap = { workspace = true } indoc = { workspace = true } keyring = { workspace = true, features = ["crypto-rust"] } libc = { workspace = true } -multimap = { workspace = true } notify = { workspace = true } once_cell = { workspace = true } os_info = { workspace = true } diff --git a/codex-rs/core/src/config/constraint.rs b/codex-rs/core/src/config/constraint.rs index 23b6c57c74..af894c4b92 100644 --- a/codex-rs/core/src/config/constraint.rs +++ b/codex-rs/core/src/config/constraint.rs @@ -1,241 +1,3 @@ -use std::fmt; -use std::sync::Arc; - -use crate::config_loader::RequirementSource; -use thiserror::Error; - -#[derive(Debug, Error, PartialEq, Eq)] -pub enum ConstraintError { - #[error( - "invalid value for `{field_name}`: `{candidate}` is not in the allowed set {allowed} (set by {requirement_source})" - )] - InvalidValue { - field_name: &'static str, - candidate: String, - allowed: String, - requirement_source: RequirementSource, - }, - - #[error("field `{field_name}` cannot be empty")] - EmptyField { field_name: String }, - - #[error("invalid rules in requirements (set by {requirement_source}): {reason}")] - ExecPolicyParse { - requirement_source: RequirementSource, - reason: String, - }, -} - -impl ConstraintError { - pub fn empty_field(field_name: impl Into) -> Self { - Self::EmptyField { - field_name: field_name.into(), - } - } -} - -pub type ConstraintResult = Result; - -impl From for std::io::Error { - fn from(err: ConstraintError) -> Self { - std::io::Error::new(std::io::ErrorKind::InvalidInput, err) - } -} - -type ConstraintValidator = dyn Fn(&T) -> ConstraintResult<()> + Send + Sync; -/// A ConstraintNormalizer is a function which transforms a value into another of the same type. -/// `Constrained` uses normalizers to transform values to satisfy constraints or enforce values. -type ConstraintNormalizer = dyn Fn(T) -> T + Send + Sync; - -#[derive(Clone)] -pub struct Constrained { - value: T, - validator: Arc>, - normalizer: Option>>, -} - -impl Constrained { - pub fn new( - initial_value: T, - validator: impl Fn(&T) -> ConstraintResult<()> + Send + Sync + 'static, - ) -> ConstraintResult { - let validator: Arc> = Arc::new(validator); - validator(&initial_value)?; - Ok(Self { - value: initial_value, - validator, - normalizer: None, - }) - } - - /// normalized creates a `Constrained` value with a normalizer function and a validator that allows any value. - pub fn normalized( - initial_value: T, - normalizer: impl Fn(T) -> T + Send + Sync + 'static, - ) -> ConstraintResult { - let validator: Arc> = Arc::new(|_| Ok(())); - let normalizer: Arc> = Arc::new(normalizer); - let normalized = normalizer(initial_value); - validator(&normalized)?; - Ok(Self { - value: normalized, - validator, - normalizer: Some(normalizer), - }) - } - - pub fn allow_any(initial_value: T) -> Self { - Self { - value: initial_value, - validator: Arc::new(|_| Ok(())), - normalizer: None, - } - } - - /// Allow any value of T, using T's Default as the initial value. - pub fn allow_any_from_default() -> Self - where - T: Default, - { - Self::allow_any(T::default()) - } - - pub fn get(&self) -> &T { - &self.value - } - - pub fn value(&self) -> T - where - T: Copy, - { - self.value - } - - pub fn can_set(&self, candidate: &T) -> ConstraintResult<()> { - (self.validator)(candidate) - } - - pub fn set(&mut self, value: T) -> ConstraintResult<()> { - let value = if let Some(normalizer) = &self.normalizer { - normalizer(value) - } else { - value - }; - (self.validator)(&value)?; - self.value = value; - Ok(()) - } -} - -impl std::ops::Deref for Constrained { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.value - } -} - -impl fmt::Debug for Constrained { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Constrained") - .field("value", &self.value) - .finish() - } -} - -impl PartialEq for Constrained { - fn eq(&self, other: &Self) -> bool { - self.value == other.value - } -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - fn invalid_value(candidate: impl Into, allowed: impl Into) -> ConstraintError { - ConstraintError::InvalidValue { - field_name: "", - candidate: candidate.into(), - allowed: allowed.into(), - requirement_source: RequirementSource::Unknown, - } - } - - #[test] - fn constrained_allow_any_accepts_any_value() { - let mut constrained = Constrained::allow_any(5); - constrained.set(-10).expect("allow any accepts all values"); - assert_eq!(constrained.value(), -10); - } - - #[test] - fn constrained_allow_any_default_uses_default_value() { - let constrained = Constrained::::allow_any_from_default(); - assert_eq!(constrained.value(), 0); - } - - #[test] - fn constrained_normalizer_applies_on_init_and_set() -> anyhow::Result<()> { - let mut constrained = Constrained::normalized(-1, |value| value.max(0))?; - assert_eq!(constrained.value(), 0); - constrained.set(-5)?; - assert_eq!(constrained.value(), 0); - constrained.set(10)?; - assert_eq!(constrained.value(), 10); - Ok(()) - } - - #[test] - fn constrained_new_rejects_invalid_initial_value() { - let result = Constrained::new(0, |value| { - if *value > 0 { - Ok(()) - } else { - Err(invalid_value(value.to_string(), "positive values")) - } - }); - - assert_eq!(result, Err(invalid_value("0", "positive values"))); - } - - #[test] - fn constrained_set_rejects_invalid_value_and_leaves_previous() { - let mut constrained = Constrained::new(1, |value| { - if *value > 0 { - Ok(()) - } else { - Err(invalid_value(value.to_string(), "positive values")) - } - }) - .expect("initial value should be accepted"); - - let err = constrained - .set(-5) - .expect_err("negative values should be rejected"); - assert_eq!(err, invalid_value("-5", "positive values")); - assert_eq!(constrained.value(), 1); - } - - #[test] - fn constrained_can_set_allows_probe_without_setting() { - let constrained = Constrained::new(1, |value| { - if *value > 0 { - Ok(()) - } else { - Err(invalid_value(value.to_string(), "positive values")) - } - }) - .expect("initial value should be accepted"); - - constrained - .can_set(&2) - .expect("can_set should accept positive value"); - let err = constrained - .can_set(&-1) - .expect_err("can_set should reject negative value"); - assert_eq!(err, invalid_value("-1", "positive values")); - assert_eq!(constrained.value(), 1); - } -} +pub use codex_config::Constrained; +pub use codex_config::ConstraintError; +pub use codex_config::ConstraintResult; diff --git a/codex-rs/core/src/config_loader/macos.rs b/codex-rs/core/src/config_loader/macos.rs index b474e81eaa..d94b1930b7 100644 --- a/codex-rs/core/src/config_loader/macos.rs +++ b/codex-rs/core/src/config_loader/macos.rs @@ -1,6 +1,6 @@ -use super::config_requirements::ConfigRequirementsToml; -use super::config_requirements::ConfigRequirementsWithSources; -use super::config_requirements::RequirementSource; +use super::ConfigRequirementsToml; +use super::ConfigRequirementsWithSources; +use super::RequirementSource; use base64::Engine; use base64::prelude::BASE64_STANDARD; use core_foundation::base::TCFType; diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 68973718fe..6a1c97b29d 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -1,14 +1,7 @@ -mod cloud_requirements; -mod config_requirements; mod diagnostics; -mod fingerprint; mod layer_io; #[cfg(target_os = "macos")] mod macos; -mod merge; -mod overrides; -mod requirements_exec_policy; -mod state; #[cfg(test)] mod tests; @@ -16,10 +9,10 @@ mod tests; use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigToml; use crate::config::deserialize_config_toml_with_base; -use crate::config_loader::config_requirements::ConfigRequirementsWithSources; use crate::config_loader::layer_io::LoadedConfigLayers; use crate::git_info::resolve_root_git_project_for_trust; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::config_loader::ConfigRequirementsWithSources; use codex_protocol::config_types::SandboxMode; use codex_protocol::config_types::TrustLevel; use codex_protocol::protocol::AskForApproval; @@ -33,19 +26,27 @@ use std::path::Path; use std::path::PathBuf; use toml::Value as TomlValue; -pub use cloud_requirements::CloudRequirementsLoader; -pub use config_requirements::ConfigRequirements; -pub use config_requirements::ConfigRequirementsToml; -pub use config_requirements::ConstrainedWithSource; -pub use config_requirements::McpServerIdentity; -pub use config_requirements::McpServerRequirement; -pub use config_requirements::NetworkConstraints; -pub use config_requirements::NetworkRequirementsToml; -pub use config_requirements::RequirementSource; -pub use config_requirements::ResidencyRequirement; -pub use config_requirements::SandboxModeRequirement; -pub use config_requirements::Sourced; -pub use config_requirements::WebSearchModeRequirement; +pub use codex_config::config_loader::CloudRequirementsLoader; +pub use codex_config::config_loader::ConfigLayerEntry; +pub use codex_config::config_loader::ConfigLayerStack; +pub use codex_config::config_loader::ConfigLayerStackOrdering; +pub use codex_config::config_loader::ConfigRequirements; +pub use codex_config::config_loader::ConfigRequirementsToml; +pub use codex_config::config_loader::ConstrainedWithSource; +pub use codex_config::config_loader::LoaderOverrides; +pub use codex_config::config_loader::McpServerIdentity; +pub use codex_config::config_loader::McpServerRequirement; +pub use codex_config::config_loader::NetworkConstraints; +pub use codex_config::config_loader::NetworkRequirementsToml; +pub use codex_config::config_loader::RequirementSource; +pub use codex_config::config_loader::ResidencyRequirement; +pub use codex_config::config_loader::SandboxModeRequirement; +pub use codex_config::config_loader::Sourced; +pub use codex_config::config_loader::WebSearchModeRequirement; +#[cfg(test)] +pub(crate) use codex_config::config_loader::fingerprint::version_for_toml; +pub use codex_config::config_loader::merge_toml_values; +pub(crate) use codex_config::config_loader::overrides::build_cli_overrides_layer; pub use diagnostics::ConfigError; pub use diagnostics::ConfigLoadError; pub use diagnostics::TextPosition; @@ -56,12 +57,6 @@ pub(crate) use diagnostics::first_layer_config_error_from_entries; pub use diagnostics::format_config_error; pub use diagnostics::format_config_error_with_source; pub(crate) use diagnostics::io_error_from_config_error; -pub use merge::merge_toml_values; -pub(crate) use overrides::build_cli_overrides_layer; -pub use state::ConfigLayerEntry; -pub use state::ConfigLayerStack; -pub use state::ConfigLayerStackOrdering; -pub use state::LoaderOverrides; /// On Unix systems, load default settings from this file path, if present. /// Note that /etc/codex/ is treated as a "config folder," so subfolders such @@ -145,7 +140,7 @@ pub async fn load_config_layers_state( let cli_overrides_layer = if cli_overrides.is_empty() { None } else { - let cli_overrides_layer = overrides::build_cli_overrides_layer(cli_overrides); + let cli_overrides_layer = build_cli_overrides_layer(cli_overrides); let base_dir = cwd .as_ref() .map(AbsolutePathBuf::as_path) diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 79c657ad96..d90ba182e2 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -11,10 +11,10 @@ use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLoadError; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; -use crate::config_loader::config_requirements::ConfigRequirementsWithSources; -use crate::config_loader::config_requirements::RequirementSource; -use crate::config_loader::fingerprint::version_for_toml; +use crate::config_loader::ConfigRequirementsWithSources; +use crate::config_loader::RequirementSource; use crate::config_loader::load_requirements_toml; +use crate::config_loader::version_for_toml; use codex_protocol::config_types::TrustLevel; use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::AskForApproval; @@ -1246,19 +1246,19 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<() } mod requirements_exec_policy_tests { - use super::super::config_requirements::ConfigRequirementsWithSources; - use super::super::requirements_exec_policy::RequirementsExecPolicyDecisionToml; - use super::super::requirements_exec_policy::RequirementsExecPolicyParseError; - use super::super::requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; - use super::super::requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; - use super::super::requirements_exec_policy::RequirementsExecPolicyToml; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; + use crate::config_loader::ConfigRequirementsWithSources; use crate::config_loader::RequirementSource; use crate::exec_policy::load_exec_policy; use codex_app_server_protocol::ConfigLayerSource; + use codex_config::config_loader::requirements_exec_policy::RequirementsExecPolicyDecisionToml; + use codex_config::config_loader::requirements_exec_policy::RequirementsExecPolicyParseError; + use codex_config::config_loader::requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; + use codex_config::config_loader::requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; + use codex_config::config_loader::requirements_exec_policy::RequirementsExecPolicyToml; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::RuleMatch; diff --git a/codex-rs/tui/src/debug_config.rs b/codex-rs/tui/src/debug_config.rs index 01f5cf6cac..ddc155da0c 100644 --- a/codex-rs/tui/src/debug_config.rs +++ b/codex-rs/tui/src/debug_config.rs @@ -474,44 +474,46 @@ mod tests { } else { absolute_path("/etc/codex/requirements.toml") }; - let mut requirements = ConfigRequirements::default(); - requirements.approval_policy = ConstrainedWithSource::new( - Constrained::allow_any(AskForApproval::OnRequest), - Some(RequirementSource::CloudRequirements), - ); - requirements.sandbox_policy = ConstrainedWithSource::new( - Constrained::allow_any(SandboxPolicy::ReadOnly), - Some(RequirementSource::SystemRequirementsToml { - file: requirements_file.clone(), - }), - ); - requirements.mcp_servers = Some(Sourced::new( - BTreeMap::from([( - "docs".to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "codex-mcp".to_string(), + let requirements = ConfigRequirements { + approval_policy: ConstrainedWithSource::new( + Constrained::allow_any(AskForApproval::OnRequest), + Some(RequirementSource::CloudRequirements), + ), + sandbox_policy: ConstrainedWithSource::new( + Constrained::allow_any(SandboxPolicy::ReadOnly), + Some(RequirementSource::SystemRequirementsToml { + file: requirements_file.clone(), + }), + ), + mcp_servers: Some(Sourced::new( + BTreeMap::from([( + "docs".to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "codex-mcp".to_string(), + }, }, + )]), + RequirementSource::LegacyManagedConfigTomlFromMdm, + )), + enforce_residency: ConstrainedWithSource::new( + Constrained::allow_any(Some(ResidencyRequirement::Us)), + Some(RequirementSource::CloudRequirements), + ), + web_search_mode: ConstrainedWithSource::new( + Constrained::allow_any(WebSearchMode::Cached), + Some(RequirementSource::CloudRequirements), + ), + network: Some(Sourced::new( + NetworkConstraints { + enabled: Some(true), + allowed_domains: Some(vec!["example.com".to_string()]), + ..Default::default() }, - )]), - RequirementSource::LegacyManagedConfigTomlFromMdm, - )); - requirements.enforce_residency = ConstrainedWithSource::new( - Constrained::allow_any(Some(ResidencyRequirement::Us)), - Some(RequirementSource::CloudRequirements), - ); - requirements.web_search_mode = ConstrainedWithSource::new( - Constrained::allow_any(WebSearchMode::Cached), - Some(RequirementSource::CloudRequirements), - ); - requirements.network = Some(Sourced::new( - NetworkConstraints { - enabled: Some(true), - allowed_domains: Some(vec!["example.com".to_string()]), - ..Default::default() - }, - RequirementSource::CloudRequirements, - )); + RequirementSource::CloudRequirements, + )), + ..ConfigRequirements::default() + }; let requirements_toml = ConfigRequirementsToml { allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), @@ -631,11 +633,13 @@ approval_policy = "never" #[test] fn debug_config_output_normalizes_empty_web_search_mode_list() { - let mut requirements = ConfigRequirements::default(); - requirements.web_search_mode = ConstrainedWithSource::new( - Constrained::allow_any(WebSearchMode::Disabled), - Some(RequirementSource::CloudRequirements), - ); + let requirements = ConfigRequirements { + web_search_mode: ConstrainedWithSource::new( + Constrained::allow_any(WebSearchMode::Disabled), + Some(RequirementSource::CloudRequirements), + ), + ..ConfigRequirements::default() + }; let requirements_toml = ConfigRequirementsToml { allowed_approval_policies: None,