Skip to content

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Dec 18, 2025

Introduce ConfigBuilder as an alternative to our existing Config constructors.

I noticed that the existing constructors, Config::load_with_cli_overrides() and Config::load_with_cli_overrides_and_harness_overrides(), did not take codex_home as a parameter, which can be a problem.

Historically, when Codex was purely a CLI, we wanted to be extra sure that the creation of codex_home was always done via find_codex_home(), so we did not expose codex_home as a parameter when creating Config in business logic. But in integration tests, codex_home nearly always needs to be configured (as a temp directory), which is why callers would have to go through Config::load_from_base_config_with_overrides() instead.

Now that the Codex harness also functions as an app server, which could conceivably load multiple threads where codex_home is parameterized differently in each one, I think it makes sense to make this configurable. Going to a builder pattern makes it more flexible to ensure an arbitrary permutation of options can be set when constructing a Config while using the appropriate defaults for the options that aren't set explicitly.

Ultimately, I think this should make it possible for us to make Config::load_from_base_config_with_overrides() private because all integration tests should be able to leverage ConfigBuilder instead. Though there could be edge cases, so I'll pursue that migration after we get through the current config overhaul.


Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest merged commit 580c59a into main Dec 18, 2025
45 of 47 checks passed
@bolinfest bolinfest deleted the pr8235 branch December 18, 2025 07:45
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants