agent/codex
2
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] Move config loading into codex-config (#19487)

Browse Source 
## Why

Config loading had become split across crates: `codex-config` owned the
config types and merge logic, while `codex-core` still owned the loader
that assembled the layer stack. This change consolidates that
responsibility in `codex-config`, so the crate that defines config
behavior also owns how configs are discovered and loaded.

To make that move possible without reintroducing the old dependency
cycle, the shell-environment policy types and helpers that
`codex-exec-server` needs now live in `codex-protocol` instead of
flowing through `codex-config`.

This also makes the migrated loader tests more deterministic on machines
that already have managed or system Codex config installed by letting
tests override the system config and requirements paths instead of
reading the host's `/etc/codex`.

## What Changed

- moved the config loader implementation from `codex-core` into
`codex-config::loader` and deleted the old `core::config_loader` module
instead of leaving a compatibility shim
- moved shell-environment policy types and helpers into
`codex-protocol`, then updated `codex-exec-server` and other downstream
crates to import them from their new home
- updated downstream callers to use loader/config APIs from
`codex-config`
- added test-only loader overrides for system config and requirements
paths so loader-focused tests do not depend on host-managed config state
- cleaned up now-unused dependency entries and platform-specific cfgs
that were surfaced by post-push CI

## Testing

- `cargo test -p codex-config`
- `cargo test -p codex-core config_loader_tests::`
- `cargo test -p codex-protocol -p codex-exec-server -p
codex-cloud-requirements -p codex-rmcp-client --lib`
- `cargo test --lib -p codex-app-server-client -p codex-exec`
- `cargo test --no-run --lib -p codex-app-server`
- `cargo test -p codex-linux-sandbox --lib`
- `cargo shear`
- `just bazel-lock-check`

## Notes

- I did not chase unrelated full-suite failures outside the migrated
loader surface.
- `cargo test -p codex-core --lib` still hits unrelated proxy-sensitive
failures on this machine, and Windows CI still shows unrelated
long-running/timeouting test noise outside the loader migration itself.
This commit is contained in:
pakrym-oai
2026-04-26 15:10:53 -07:00
committed by GitHub
parent 2a020f1a0a
commit 9c3abcd46c
70 changed files with 483 additions and 491 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/config/agent_roles.rs
+2 -2
View File
@@ -1,6 +1,6 @@
use super::AgentRoleConfig;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::config_toml::AgentRoleToml;
use codex_config::config_toml::AgentsToml;
use codex_config::config_toml::ConfigToml;
codex-rs/core/src/config/config_loader_tests.rs
+2389
View File
File diff suppressed because it is too large Load Diff
codex-rs/core/src/config/config_tests.rs
+40 -44
View File
@@ -4,11 +4,10 @@ use crate::config::ThreadStoreConfig;
use crate::config::edit::ConfigEdit;
use crate::config::edit::ConfigEditsBuilder;
use crate::config::edit::apply_blocking;
use crate::config_loader::RequirementSource;
use crate::config_loader::project_trust_key;
use crate::plugins::PluginsManager;
use assert_matches::assert_matches;
use codex_config::CONFIG_TOML_FILE;
use codex_config::RequirementSource;
use codex_config::config_toml::AgentRoleToml;
use codex_config::config_toml::AgentsToml;
use codex_config::config_toml::AutoReviewToml;
@@ -21,6 +20,7 @@ use codex_config::config_toml::RealtimeTransport;
use codex_config::config_toml::RealtimeWsMode;
use codex_config::config_toml::RealtimeWsVersion;
use codex_config::config_toml::ToolsToml;
use codex_config::loader::project_trust_key;
use codex_config::permissions_toml::FilesystemPermissionToml;
use codex_config::permissions_toml::FilesystemPermissionsToml;
use codex_config::permissions_toml::NetworkDomainPermissionToml;
@@ -3981,7 +3981,7 @@ async fn load_config_uses_requirements_guardian_policy_config() -> std::io::Resu
let config_layer_stack = ConfigLayerStack::new(
Vec::new(),
Default::default(),
crate::config_loader::ConfigRequirementsToml {
codex_config::ConfigRequirementsToml {
guardian_policy_config: Some(
" Use the workspace-managed guardian policy. ".to_string(),
),
@@ -4062,7 +4062,7 @@ async fn requirements_guardian_policy_beats_auto_review() -> std::io::Result<()>
let config_layer_stack = ConfigLayerStack::new(
Vec::new(),
Default::default(),
crate::config_loader::ConfigRequirementsToml {
codex_config::ConfigRequirementsToml {
guardian_policy_config: Some("Use the managed guardian policy.".to_string()),
..Default::default()
},
@@ -4126,7 +4126,7 @@ async fn load_config_ignores_empty_requirements_guardian_policy_config() -> std:
let config_layer_stack = ConfigLayerStack::new(
Vec::new(),
Default::default(),
crate::config_loader::ConfigRequirementsToml {
codex_config::ConfigRequirementsToml {
guardian_policy_config: Some(" ".to_string()),
..Default::default()
},
@@ -4258,15 +4258,15 @@ config_file = "./agents/researcher.toml"
"#,
)
.expect("agent role layer config should parse");
let config_layer_stack = crate::config_loader::ConfigLayerStack::new(
vec![crate::config_loader::ConfigLayerEntry::new(
let config_layer_stack = codex_config::ConfigLayerStack::new(
vec![codex_config::ConfigLayerEntry::new(
codex_app_server_protocol::ConfigLayerSource::User {
file: codex_home.path().join(CONFIG_TOML_FILE).abs(),
},
layer_config,
)],
Default::default(),
crate::config_loader::ConfigRequirementsToml::default(),
codex_config::ConfigRequirementsToml::default(),
)
.map_err(std::io::Error::other)?;
@@ -6035,14 +6035,12 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
{
let fixture = create_test_fixture()?;
let requirements_toml = crate::config_loader::ConfigRequirementsToml {
let requirements_toml = codex_config::ConfigRequirementsToml {
allowed_approval_policies: None,
allowed_approvals_reviewers: None,
allowed_sandbox_modes: None,
remote_sandbox_config: None,
allowed_web_search_modes: Some(vec![
crate::config_loader::WebSearchModeRequirement::Cached,
]),
allowed_web_search_modes: Some(vec![codex_config::WebSearchModeRequirement::Cached]),
feature_requirements: None,
hooks: None,
mcp_servers: None,
@@ -6053,7 +6051,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
permissions: None,
guardian_policy_config: None,
};
let requirement_source = crate::config_loader::RequirementSource::Unknown;
let requirement_source = codex_config::RequirementSource::Unknown;
let requirement_source_for_error = requirement_source.clone();
let allowed = vec![WebSearchMode::Disabled, WebSearchMode::Cached];
let constrained = Constrained::new(WebSearchMode::Cached, move |candidate| {
@@ -6068,15 +6066,15 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
})
}
})?;
let requirements = crate::config_loader::ConfigRequirements {
web_search_mode: crate::config_loader::ConstrainedWithSource::new(
let requirements = codex_config::ConfigRequirements {
web_search_mode: codex_config::ConstrainedWithSource::new(
constrained,
Some(requirement_source),
),
..Default::default()
};
let config_layer_stack =
crate::config_loader::ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
codex_config::ConfigLayerStack::new(Vec::new(), requirements, requirements_toml)
.expect("config layer stack");
let config = Config::load_config_with_layer_stack(
@@ -6688,10 +6686,8 @@ async fn requirements_disallowing_default_sandbox_falls_back_to_required_default
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
allowed_sandbox_modes: Some(vec![
crate::config_loader::SandboxModeRequirement::ReadOnly,
]),
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_sandbox_modes: Some(vec![codex_config::SandboxModeRequirement::ReadOnly]),
..Default::default()
}))
}))
@@ -6713,10 +6709,10 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s
"#,
)?;
let requirements = crate::config_loader::ConfigRequirementsToml {
let requirements = codex_config::ConfigRequirementsToml {
allowed_approval_policies: None,
allowed_approvals_reviewers: None,
allowed_sandbox_modes: Some(vec![crate::config_loader::SandboxModeRequirement::ReadOnly]),
allowed_sandbox_modes: Some(vec![codex_config::SandboxModeRequirement::ReadOnly]),
remote_sandbox_config: None,
allowed_web_search_modes: None,
feature_requirements: None,
@@ -6793,9 +6789,9 @@ async fn requirements_web_search_mode_overrides_danger_full_access_default() ->
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_web_search_modes: Some(vec![
crate::config_loader::WebSearchModeRequirement::Cached,
codex_config::WebSearchModeRequirement::Cached,
]),
..Default::default()
}))
@@ -6834,7 +6830,7 @@ trust_level = "untrusted"
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(workspace.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approval_policies: Some(vec![AskForApproval::OnRequest]),
..Default::default()
}))
@@ -6863,7 +6859,7 @@ async fn explicit_approval_policy_falls_back_when_disallowed_by_requirements() -
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approval_policies: Some(vec![AskForApproval::OnRequest]),
..Default::default()
}))
@@ -6884,8 +6880,8 @@ async fn feature_requirements_normalize_effective_feature_values() -> std::io::R
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([
("personality".to_string(), true),
("shell_tool".to_string(), false),
@@ -6918,8 +6914,8 @@ async fn feature_requirements_auto_review_disables_guardian_approval() -> std::i
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([("auto_review".to_string(), false)]),
}),
..Default::default()
@@ -6940,8 +6936,8 @@ async fn browser_feature_requirements_are_valid() -> std::io::Result<()> {
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([
("in_app_browser".to_string(), false),
("browser_use".to_string(), false),
@@ -6975,8 +6971,8 @@ shell_tool = true
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([
("personality".to_string(), true),
("shell_tool".to_string(), false),
@@ -7122,7 +7118,7 @@ async fn requirements_disallowing_default_approvals_reviewer_falls_back_to_requi
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::AutoReview]),
..Default::default()
}))
@@ -7148,7 +7144,7 @@ async fn root_approvals_reviewer_falls_back_when_disallowed_by_requirements() ->
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::AutoReview]),
..Default::default()
}))
@@ -7185,7 +7181,7 @@ approvals_reviewer = "user"
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approvals_reviewers: Some(vec![ApprovalsReviewer::AutoReview]),
..Default::default()
}))
@@ -7211,7 +7207,7 @@ async fn approvals_reviewer_preserves_valid_user_choice_when_allowed_by_requirem
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
allowed_approvals_reviewers: Some(vec![
ApprovalsReviewer::User,
ApprovalsReviewer::AutoReview,
@@ -7363,8 +7359,8 @@ async fn feature_requirements_normalize_runtime_feature_mutations() -> std::io::
let mut config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([
("personality".to_string(), true),
("shell_tool".to_string(), false),
@@ -7399,8 +7395,8 @@ async fn feature_requirements_warn_on_collab_legacy_alias() -> std::io::Result<(
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([("collab".to_string(), true)]),
}),
..Default::default()
@@ -7429,8 +7425,8 @@ async fn feature_requirements_warn_and_ignore_unknown_feature() -> std::io::Resu
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
Ok(Some(codex_config::ConfigRequirementsToml {
feature_requirements: Some(codex_config::FeatureRequirementsToml {
entries: BTreeMap::from([("made_up_feature".to_string(), true)]),
}),
..Default::default()
codex-rs/core/src/config/mod.rs
+28 -21
View File
@@ -1,20 +1,6 @@
use crate::agents_md::AgentsMdManager;
use crate::config::edit::ConfigEdit;
use crate::config::edit::ConfigEditsBuilder;
use crate::config_loader::CloudRequirementsLoader;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use crate::config_loader::ConstrainedWithSource;
use crate::config_loader::FeatureRequirementsToml;
use crate::config_loader::LoaderOverrides;
use crate::config_loader::McpServerIdentity;
use crate::config_loader::McpServerRequirement;
use crate::config_loader::ResidencyRequirement;
use crate::config_loader::Sourced;
use crate::config_loader::load_config_layers_state;
use crate::config_loader::project_trust_key;
use crate::memories::memory_root;
use crate::path_utils::normalize_for_native_workdir;
use crate::unified_exec::DEFAULT_MAX_BACKGROUND_TERMINAL_TIMEOUT_MS;
@@ -22,6 +8,18 @@ use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS;
use crate::windows_sandbox::WindowsSandboxLevelExt;
use crate::windows_sandbox::resolve_windows_sandbox_mode;
use crate::windows_sandbox::resolve_windows_sandbox_private_desktop;
use codex_config::CloudRequirementsLoader;
use codex_config::ConfigLayerStack;
use codex_config::ConfigLayerStackOrdering;
use codex_config::ConfigRequirements;
use codex_config::ConfigRequirementsToml;
use codex_config::ConstrainedWithSource;
use codex_config::FeatureRequirementsToml;
use codex_config::LoaderOverrides;
use codex_config::McpServerIdentity;
use codex_config::McpServerRequirement;
use codex_config::ResidencyRequirement;
use codex_config::Sourced;
use codex_config::ThreadConfigLoader;
use codex_config::config_toml::ConfigToml;
use codex_config::config_toml::ProjectConfig;
@@ -29,6 +27,8 @@ use codex_config::config_toml::RealtimeAudioConfig;
use codex_config::config_toml::RealtimeConfig;
use codex_config::config_toml::ThreadStoreToml;
use codex_config::config_toml::validate_model_providers;
use codex_config::loader::load_config_layers_state;
use codex_config::loader::project_trust_key;
use codex_config::profile_toml::ConfigProfile;
use codex_config::types::ApprovalsReviewer;
use codex_config::types::AuthCredentialsStoreMode;
@@ -44,7 +44,6 @@ use codex_config::types::OAuthCredentialsStoreMode;
use codex_config::types::OtelConfig;
use codex_config::types::OtelConfigToml;
use codex_config::types::OtelExporterKind;
use codex_config::types::ShellEnvironmentPolicy;
use codex_config::types::ToolSuggestConfig;
use codex_config::types::ToolSuggestDiscoverable;
use codex_config::types::TuiNotificationSettings;
@@ -74,6 +73,7 @@ use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::config_types::ShellEnvironmentPolicy;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::config_types::Verbosity;
use codex_protocol::config_types::WebSearchConfig;
@@ -876,10 +876,13 @@ impl ConfigBuilder {
let config_toml: ConfigToml = match merged_toml.try_into() {
Ok(config_toml) => config_toml,
Err(err) => {
if let Some(config_error) =
crate::config_loader::first_layer_config_error(&config_layer_stack).await
if let Some(config_error) = codex_config::first_layer_config_error::<ConfigToml>(
&config_layer_stack,
codex_config::CONFIG_TOML_FILE,
)
.await
{
return Err(crate::config_loader::io_error_from_config_error(
return Err(codex_config::io_error_from_config_error(
std::io::ErrorKind::InvalidData,
config_error,
Some(err),
@@ -979,8 +982,8 @@ impl Config {
format!("failed to serialize default config: {e}"),
)
})?;
let cli_layer = crate::config_loader::build_cli_overrides_layer(&cli_overrides);
crate::config_loader::merge_toml_values(&mut merged, &cli_layer);
let cli_layer = codex_config::build_cli_overrides_layer(&cli_overrides);
codex_config::merge_toml_values(&mut merged, &cli_layer);
let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home)?;
let config_toml = deserialize_config_toml_with_base(merged, &codex_home)?;
Self::load_config_with_layer_stack(
@@ -1462,7 +1465,7 @@ fn resolve_permission_config_syntax(
fn apply_managed_filesystem_constraints(
file_system_sandbox_policy: &mut FileSystemSandboxPolicy,
filesystem_constraints: &crate::config_loader::FilesystemConstraints,
filesystem_constraints: &codex_config::FilesystemConstraints,
) {
for deny_read in &filesystem_constraints.deny_read {
let deny_entry = if deny_read.contains_glob() {
@@ -2801,3 +2804,7 @@ pub fn log_dir(cfg: &Config) -> std::io::Result<PathBuf> {
#[cfg(test)]
#[path = "config_tests.rs"]
mod tests;
#[cfg(test)]
#[path = "config_loader_tests.rs"]
mod config_loader_tests;
codex-rs/core/src/config/network_proxy_spec.rs
+1 -1
View File
@@ -1,5 +1,5 @@
use crate::config_loader::NetworkConstraints;
use async_trait::async_trait;
use codex_config::NetworkConstraints;
use codex_execpolicy::Policy;
use codex_network_proxy::BlockedRequestObserver;
use codex_network_proxy::ConfigReloader;
codex-rs/core/src/config/network_proxy_spec_tests.rs
+2 -2
View File
@@ -1,6 +1,6 @@
use super::*;
use crate::config_loader::NetworkDomainPermissionToml;
use crate::config_loader::NetworkDomainPermissionsToml;
use codex_config::NetworkDomainPermissionToml;
use codex_config::NetworkDomainPermissionsToml;
use codex_network_proxy::NetworkDomainPermission;
use pretty_assertions::assert_eq;