preserve unknown config keys in ‘chunk config set’ - #497
Open
jesseworld22 wants to merge 5 commits into
Open
Conversation
`chunk config set` rewrote the whole config file through a typed struct, so
every field the struct did not model was dropped with no warning. Keys
hand-added to `commands[]` entries — the reported case was `fileExt`, `limit`,
and `always` on four commands — disappeared after a single unrelated write.
Writes now merge onto the file they are about to replace, via a new
`internal/jsonmerge`. The struct still owns every key it models, so a modeled
key it omits is deleted rather than restored: `chunk auth clear` depends on
that to remove a credential. Unknown keys are copied over verbatim as raw
bytes, after the modeled keys of the same object, so numbers and escapes are
not reformatted on the way through. Array entries are paired between the two
documents by `name`, and only entries the struct has survive — that is what
keeps the deliberately dropped `environment.setup` test step from coming back
while extra keys on the entries that remain are kept.
`config set` now also names the keys it kept but does not recognize, so a typo
is visible instead of silently doing nothing forever.
Fixes the adjacent clobber in the same paths: every writer downgraded any load
error to an empty config, so one stray brace plus one `config set` replaced the
whole file with `{}`. `LoadProjectConfigForUpdate` yields an empty config for a
missing file but refuses one that does not parse, and the write paths use it.
`validate --list` says so rather than reporting that no commands are
configured.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jesseworld22
changed the title
hanabel1
reviewed
Aug 12, 2026
`marshalPreserving` merged onto the file it was about to replace and dropped the error if the merge failed, so a file that did not parse was silently flattened to the keys chunk models — the unknown keys it was supposed to preserve went with it. The project config was covered by `LoadProjectConfigForUpdate` refusing at load; the user config leaned on `Load` doing the same, which cannot cover a file that breaks between the load and the write. The merge error is now returned. `config.Save` therefore refuses an unparseable user config and leaves the file alone. `SaveProjectConfig` keeps replacing it, via a new `marshalOverwriting`, because `chunk init --force` exists to overwrite a config nobody can fix by hand; its other callers still guard at load. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hanabel1
reviewed
Aug 12, 2026
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.
chunk config setrewrote.chunk/config.jsonthrough a typed struct and silently dropped every field it didn't model -including keys hand-added tocommands[].config writes now merge onto the existing file, preserving unrecognized keys verbatim while still letting the struct delete the ones it owns.