fix(config): strip Zod defaults from per-layer config before merge#16977
Open
Altman-conquer wants to merge 2 commits intoanomalyco:devfrom
Open
fix(config): strip Zod defaults from per-layer config before merge#16977Altman-conquer wants to merge 2 commits intoanomalyco:devfrom
Altman-conquer wants to merge 2 commits intoanomalyco:devfrom
Conversation
When Info.safeParse(normalized) runs on a config layer it applies any .default() values defined in the schema to fields that are absent in that layer's input. During mergeDeep these Zod-injected defaults then overwrite explicitly-set values from lower-priority layers (e.g. the global user config), which violates the documented config hierarchy. Fix: after parsing each file layer, delete any top-level key from the parsed result that was not present in the raw input object. The one exception is $schema, which is a synthetic key added by code (not by Zod defaults) and must always be preserved. This means contributors can now add config fields with meaningful .default() values without silently breaking the merge semantics. Fixes anomalyco#16897
github-actions
bot
added
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Thanks for looking into this! I mentioned this in the issue, but just FYI, this may be a breaking change (users might notice a change in behavior with their settings). E.g. it's possible that somebody is expecting the broken behavior where defaults are applied after loading project settings files, but whether or not this is a "big deal", I'll leave that up to the leaders of the opencode project. For my setup I'd prefer it to follow the hierarchy given by the docs (as in, defaults, then user config, then project level config). I'm not sure if it merits a new schema version or some other indication that the parsing algorithm has changed. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue for this PR
Closes #16897
Type of change
What does this PR do?
When loading a config file, each layer is parsed through
Info.safeParse(normalized). The problem is that Zod's.default()values fill in every missing field with a default. These defaulted keys then go intomergeDeep, which means a higher-priority layer (e.g. project config) silently overwrites settings from lower-priority layers (e.g. global config) for fields the user never actually set.The fix strips any key from the parsed result that was not present in the original input object before Zod touched it. Only keys the user explicitly wrote in their config file participate in the merge. The
$schemakey is excluded from removal since it is injected by the code, not the user.How did you verify your code works?
Traced through the config load path manually. The fix is minimal — it only removes keys that Zod added; it does not change keys that were already there. Matches exactly what the issue describes.
Screenshots / recordings
N/A — no UI changes
Checklist