-
Notifications
You must be signed in to change notification settings - Fork 1.2k
SEP-1330 Elicitation Enum Schema Improvements and Standards Compliance #1148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The enumNames property is not a valid JSON Schema keyword. Updated EnumSchema to use the standard-compliant oneOf pattern with const and title keywords for each option, as recommended by JSON Schema specification.
|
@chughtapan thanks for this! he current For instance, when eliciting user input, with an enum of colors, you might say "Choose your favorite color" OR "Choose your favorite colors". The actual JSON schema for an enum with title and optional description of its purpose would look something like this: "EnumSchema": {
"oneOf": [
{
"type": "object",
"required": ["type", "oneOf"],
"properties": {
"type": { "const": "string" },
"title": { "type": "string" },
"description": { "type": "string" },
"oneOf": {
"type": "array",
"items": {
"type": "object",
"required": ["const", "title"],
"properties": {
"const": { "type": "string" },
"title": { "type": "string" }
},
"additionalProperties": false
}
}
},
"additionalProperties": false
},
{
"type": "object",
"required": ["type", "anyOf"],
"properties": {
"type": { "const": "array" },
"title": { "type": "string" },
"description": { "type": "string" },
"anyOf": {
"type": "array",
"items": {
"type": "object",
"required": ["const", "title"],
"properties": {
"const": { "type": "string" },
"title": { "type": "string" }
},
"additionalProperties": false
}
}
},
"additionalProperties": false
}
]
}Which would allow single selection... {
"type": "string",
"title": "Color Selection",
"description": "Choose your favorite color",
"oneOf": [
{ "const": "#FF0000", "title": "Red" },
{ "const": "#00FF00", "title": "Green" },
{ "const": "#0000FF", "title": "Blue" }
]
}or multi-selection... {
"type": "array",
"title": "Color Selection",
"description": "Choose your favorite colors",
"items": {
"oneOf": [
{ "const": "#FF0000", "title": "Red" },
{ "const": "#00FF00", "title": "Green" },
{ "const": "#0000FF", "title": "Blue" }
]
}
}as valid enums sent to the client for elicitation (or whatever purpose). |
|
@cliffhall I like your proposal, but I guess that means it's a broader change around the semantics of enums in MCP. IIUC the current expectation matches the proposed oneOf semantics. I think the current docs state that
But maybe it is a valid proposal to change that only for enums and support anyOf semantics as well. Thoughts? |
@chughtapan For context, where in the spec did you find this particular note?
I think it is worthwhile running this up the flagpole. Enums are equally useful for multi-select as they are for single selection. Certainly in elicitation, it would be nice to have. For rigor, I decided to test the
|
|
@cliffhall Thanks!
Yeah, I think I agree with you on that. Thanks for validating the schema - I'll try to implement the client logic whenever I get a moment. I think it should be trivial to automatically switch between a checkbox or a radio selection based on the schema type.
Another thought is that multi-selection enums can have far more complex validators than single choice selection. For example, choose upto two colors or can't choose black and blue together - things like that. IIUC we want to keep the elicitation schemas simple enough that clients can do the syntactic validation atleast. Do you think it's possible to add things like choose at max two or exactly two or things like that in the schema as well or we will have to fallback to server side code for that. |
Ah, yes. We pushed back on the complex objects in the UI because generating forms of nested objects is hard and can be ugly at render time. But this is an enum as a single or multi-select dropdown (or checkboxes vs radio buttons). That won't degrade UX, and opens up possibilities for elicitation, e.g, "Choose all that apply".
It would be possible to allow choosing up to some limit with {
"type": "array",
"title": "Color Selection",
"description": "Choose one or two colors",
"minItems": 1,
"maxItems": 2,
"items": {
"oneOf": [
{ "const": "#FF0000", "title": "Red" },
{ "const": "#00FF00", "title": "Green" },
{ "const": "#0000FF", "title": "Blue" }
]
}
}
|
|
@cliffhall I did some more digging: for choices without titles, we should always prefer to use enum instead of oneOf/anyOf because that's default serialization would create. I created a basic script to validate the default serialization from Python to JSON: https://gist.github.com/chughtapan/2f8738f69098f35f877d1cb3de38aeb2 The outputs are here: https://gist.github.com/chughtapan/c80f175b880f0badc40b133611ab4956 I guess the current schema is pretty clean that way: enums with title just have the additional enumNames field, while the default enums dont. In our current proposal we would have to support both enums (for without title) and one/anyOf for enums with titles. Does that seem reasonable or would it make the schema too complex? The other thought (related to multi-choice): currently we serialize enums with type: string - but that doesn't make sense for the multi-choice, they should be serialized to Arrays. So maybe those should be two independent elicitation types instead of trying to overload the same enum type? |
|
@cliffhall Working demo: https://asciinema.org/a/anBvJdqEmTjw0JkKYOooQa5Ta Proof-of-concept implementation: https://github.com/evalstate/fast-agent/pull/324/files |
@chughtapan In order to head off an SEP that ends up with a winding discussion, let's start in this thread in the MCP Contributors Discord. |
- SingleSelectEnumSchema: Supports both plain enum[] and oneOf with titles - MultiSelectEnumSchema: Array type for multi-selection with/without titles - DeprecatedEnumSchema: Maintains backward compatibility with enumNames This replaces the non-standard enumNames approach with JSON Schema-compliant patterns using oneOf/const for titled enums, while preserving backward compatibility and adding multi-select support.
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small request for now
|
@chughtapan @cliffhall hey guys, thanks for all this work! any updates on where this might land? |
We have moved this into SEP-1330 which is currently under an async review by core maintainers. We're hoping for a vote this week. |
|
@cliffhall -- good catch. fixed this |
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weirdness in the PrimitiveSchemaDefinition. It's not referencing EnumSchema, it's inlining it.
cliffhall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
|
👋 just as a reminder, we're aiming to ensure that all SEPs that are on-deck for the |
I believe all the comments (primarily about removing reference to "deprecation") have been implemented. |
dsp-ant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paul said this is good to go.
@localden @felixweinberger done. We've replaced the "deprecation" wording with the term "legacy". We still don't want to encourage the use of the old schema. |
Apologies, hadn't finished my sentence there - "all comments have been implemented" is what I meant to say there. Looks like we're good to go and this is merged - reviewing modelcontextprotocol/typescript-sdk#1077 atm |











Motivation and Context
The use of enumNames property is not a valid JSON Schema keyword in elicitation schema.
This PR implements the changes in #1330
EnumSchema"Legacy"ElicitResponse, add array as anadditionalPropertytypeHow Has This Been Tested?
Breaking Changes
No, the existing schemas remain and can be deprecated/removed in a future version
Types of changes
Checklist
Additional context
This came up as a request in #1035