fix(elicitation): preserve enumNames through ElicitationSchema serde round-trip#905
Open
abdouloued wants to merge 2 commits into
Open
Conversation
…round-trip UntitledSingleSelectEnumSchema lacked deny_unknown_fields, so a legacy enum payload containing enumNames was silently matched by that variant (ignoring the field) rather than falling through to LegacyEnumSchema. The enumNames array was lost on re-serialization. Add deny_unknown_fields to UntitledSingleSelectEnumSchema so that any unknown field (including enumNames) causes serde to try the next untagged variant, reaching LegacyEnumSchema correctly. Also add skip_serializing_if = "Option::is_none" to LegacyEnumSchema::enum_names so that an untitled legacy enum without enumNames does not serialize "enumNames": null. Fixes modelcontextprotocol#903
github-actions
Bot
added
T-core
michaelneale
previously approved these changes
Jun 16, 2026
Contributor
|
@abdouloued do you want to fix up the failures and we can take a look again? |
…_fields deny_unknown_fields on UntitledSingleSelectEnumSchema makes schemars emit additionalProperties: false for that definition. Regenerate the golden schema fixtures to match (UPDATE_SCHEMA=1 cargo test -p rmcp --test test_message_schema --all-features).
github-actions
Bot
added
T-test
Author
Pushed a fix. The failure appears to be a stale golden-file schema snapshot ( Regenerated it with Looks like the workflow run needs approval to kick off again on this push. |
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.
Fixes #903.
Problem
EnumSchemais#[serde(untagged)]with variants orderedSingle → Multi → Legacy.SingleSelectEnumSchemais itself untagged withUntitledbeforeTitled.UntitledSingleSelectEnumSchemadid not havedeny_unknown_fields, so a legacy payload like:{"type":"string","enum":["opt1","opt2","opt3"],"enumNames":["Option One","Option Two","Option Three"]}was silently matched by
UntitledSingleSelectEnumSchema(which has noenumNamesfield). The array was dropped, andLegacyEnumSchemawas never tried. On re-serialization,enumNameswas gone.A secondary issue:
LegacyEnumSchema::enum_nameslackedskip_serializing_if = "Option::is_none", so aLegacywithenum_names: Nonewould serialize"enumNames": null.Fix
deny_unknown_fieldstoUntitledSingleSelectEnumSchema. WhenenumNames(or any unrecognised field) is present, serde rejects this variant and falls through toLegacyEnumSchema.skip_serializing_if = "Option::is_none"toLegacyEnumSchema::enum_names.Two regression tests added:
test_legacy_enum_schema_roundtrip_preserves_enum_names— mirrors the exact repro from the issue; verifies that a fullElicitationSchemaround-trip keepsenumNamesintact.test_legacy_enum_schema_no_enum_names_omits_field— verifiesLegacyEnumSchemawithenum_names: Nonedoes not emit"enumNames": null.All existing elicitation tests continue to pass.