[codex] Move config loading into codex-config#19487
Merged
pakrym-oai merged 4 commits intomainfrom Apr 26, 2026
Merged
Conversation
bolinfest
reviewed
Apr 25, 2026
|
|
||
| #[cfg(unix)] | ||
| fn system_config_toml_file() -> io::Result<AbsolutePathBuf> { | ||
| #[doc(hidden)] |
Collaborator
There was a problem hiding this comment.
I noticed Codex likes to add these sometimes. We should add something to our AGENTS.md about this? Please don't add them: they're just distracting in our case and I worry others will copy the pattern.
Collaborator
There was a problem hiding this comment.
Current status:
$ rg '#\[doc\(hidden\)\]' | wc -l
12
bolinfest
reviewed
Apr 25, 2026
| @@ -26,17 +26,8 @@ where | |||
| let mut env_map = populate_env(vars, policy, thread_id); | |||
|
|
|||
| if cfg!(target_os = "windows") { | |||
| // This is a workaround to address the failures we are seeing in the | |||
Collaborator
There was a problem hiding this comment.
Is this correct to remove?
bolinfest
reviewed
Apr 25, 2026
| @@ -706,7 +706,7 @@ fn notification_sender(inner: &Inner) -> Option<RpcNotificationSender> { | |||
| #[cfg(test)] | |||
Collaborator
There was a problem hiding this comment.
Drive-by comment: please adopt the _tests.rs pattern in the exec-server crate!
bolinfest
reviewed
Apr 25, 2026
bolinfest
approved these changes
Apr 25, 2026
Collaborator
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
Thanks for the refactor!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Config loading had become split across crates:
codex-configowned the config types and merge logic, whilecodex-corestill owned the loader that assembled the layer stack. This change consolidates that responsibility incodex-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-serverneeds now live incodex-protocolinstead of flowing throughcodex-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
codex-coreintocodex-config::loaderand deleted the oldcore::config_loadermodule instead of leaving a compatibility shimcodex-protocol, then updatedcodex-exec-serverand other downstream crates to import them from their new homecodex-configTesting
cargo test -p codex-configcargo test -p codex-core config_loader_tests::cargo test -p codex-protocol -p codex-exec-server -p codex-cloud-requirements -p codex-rmcp-client --libcargo test --lib -p codex-app-server-client -p codex-execcargo test --no-run --lib -p codex-app-servercargo test -p codex-linux-sandbox --libcargo shearjust bazel-lock-checkNotes
cargo test -p codex-core --libstill hits unrelated proxy-sensitive failures on this machine, and Windows CI still shows unrelated long-running/timeouting test noise outside the loader migration itself.