Fixed config merging issue with profiles #9509
Merged
Conversation
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
This PR fixes a small issue with chained (nested) config.toml file merging. The old logic didn't properly handle profiles. The change adds a unit test for this case. It also exposes a function from the config crate that is needed by the app server code paths to implement support for chained configs.
etraut-openai
added a commit
that referenced
this pull request
Jan 20, 2026
This PR adds support for chained (layered) config.toml file merging for clients that use the app server interface. It does the following: * Changes code paths for new thread, resume thread, and fork thread to use the effective config based on the cwd. * Updates the `config/read` API to accept an optional `cwd` parameter. If specified, the API returns the effective config based on that cwd path. Also optionally includes all layers including project config files. If cwd is not specified, the API falls back on its older behavior where it considers only the global (non-project) config files when computing the effective config. The changes in codex_message_processor.rs look deceptively large. They mostly just involve moving existing blocks of code to a later point in some functions so it can use the cwd to calculate the config. This PR builds upon #9509 and should be reviewed and merged after that PR.
bolinfest
reviewed
Jan 20, 2026
bolinfest
reviewed
Jan 20, 2026
Collaborator
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
@etraut-openai could you please expand the PR body to explain what the fix was? I see the new unit test, which is great, but I also see a bunch of changes to TUI code that I did not expect. It seems like there is some relationship between restoring cwd and profile correctness, which is not obvious, so I think that should be explained in the PR body.
bolinfest
approved these changes
Jan 20, 2026
…rusted workspaces
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR fixes a small issue with chained (layered) config.toml file merging. The old logic didn't properly handle profiles.
In particular, if a lower-layer config overrides a profile defined in a higher-layer config, the override did not take effect. This prevents users from having project-specific profile overrides and contradicts the (soon-to-be) documented behavior of config merging.
The change adds a unit test for this case. It also exposes a function from the config crate that is needed by the app server code paths to implement support for layered configs.