🐛 fix: Keep KV Record Edits Object-Typed so Headers Never Save as Arrays - #111
Open
dustinhealy wants to merge 2 commits into
Open
🐛 fix: Keep KV Record Edits Object-Typed so Headers Never Save as Arrays#111dustinhealy wants to merge 2 commits into
dustinhealy wants to merge 2 commits into
Conversation
Emptying a KeyValueField (removing the last header, env, or addParams row) stored a bare [] in edit state. serializeKVPairs cannot recognize an empty array as KV pairs, so the [] reached the backend as an array where the schema expects a record, failing base-config validation with "Expected object, received array" and persisting invalid values on the unvalidated profile-value route.
Record-control edits now collapse an emptied pair list to {} via a shared kvPairsEditValue helper (FieldRenderer record branches and ProfileValueModal), getDefaultValue returns {} for record fields so an untouched profile-value save stays object-typed, and FieldProfilePopover serializes modal values with deepSerializeKVPairs so no nested pairs shape can leak through the one save route that bypasses buildSavePayload.
Adds regression tests proving MCP header edits serialize to a plain record in the save payload (issue #56 repro) and that emptied KV lists emit {} rather than [].
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
dustinhealy
marked this pull request as ready for review
August 10, 2026 03:33
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Serializing profile modal values shape-based would collapse any array whose elements happen to carry key and value properties into a record, dropping sibling fields. Only the record control edits via KeyValueField pairs, so serialize only that control type through a shared serializeModalValue helper and pass every other control's value through untouched.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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.
Summary
MCP custom headers (and other KeyValueField-backed record fields like
envand azureaddParams) could reach the backend as arrays instead of records. The version current when #56 was filed serialized non-indexed save paths with the shallowserializeKVPairswhile the MCP renderer wrote whole objects, so header pairs nested inside those values persisted verbatim as[{key, value, valueType}], which LibreChat's schema rejects. #92 fixed the primary path by deep-serializing every entry inbuildSavePayload, but two leaks remain on main.First, emptying a KeyValueField (removing the last row) stores a bare
[], which shape-based serialization cannot recognize as KV pairs, so it is submitted as an array where the schema expects an object. On the base-config route this fails validation with "Expected object, received array" and blocks the whole save; on the profile-value route, which performs no field validation, the invalid array persists.Second,
FieldProfilePopoverserialized modal values with the shallowserializeKVPairsandgetDefaultValueseeded record fields with[], so an untouched "add profile value" save on a record field persisted a raw array through the unvalidated route.Record-control edits now collapse an emptied pair list to
{}via a sharedkvPairsEditValuehelper (both FieldRenderer record branches and ProfileValueModal),getDefaultValuereturns{}for record fields, and FieldProfilePopover serializes modal values withdeepSerializeKVPairsso no route can leak a pairs array. In-progress rows still round-trip as raw pairs in edit state so blank keys survive re-renders until save-time serialization.Fixes #56
Change Type
Testing
Regression tests cover the #56 repro end to end (DOM edit of an
Authorizationheader throughapplyConfigEditandbuildSavePayloadproducing{Authorization: "Bearer {{API_KEY}}"}), the emptied-list cases in the MCP renderer, FieldRenderer, and ProfileValueModal (all fail before this change), and pairs nested inside indexed array entries matching the azure "Additional parameters" report on #44. Local gates: fullvitestsuite passes (810 tests),eslint --max-warnings 0clean,tsc --noEmitclean.Checklist