Skip to content

Conversation

@chughtapan
Copy link
Contributor

@chughtapan chughtapan commented Jul 22, 2025

Motivation and Context

This PR implements the proposal in #1034 to enable default values in elicitation schemas for all primitive types.

How Has This Been Tested?

npm run check:schema:ts
npm run check:docs

Breaking Changes

None - the default values are optional and existing flows will work as it is.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

#1034 and references within demonstrate prototype client implementation

@chughtapan chughtapan requested review from a team July 22, 2025 22:43
@chughtapan chughtapan changed the title Add default values for all primitive types in elicitation schemas SEP: Add default values for all primitive types in elicitation schemas Jul 22, 2025
@chughtapan chughtapan requested review from a team and dsp-ant July 24, 2025 07:02
@evalstate evalstate self-assigned this Jul 24, 2025
@evalstate evalstate added the draft SEP proposal with a sponsor. label Jul 24, 2025
@evalstate evalstate removed their assignment Jul 24, 2025
@evalstate evalstate removed the draft SEP proposal with a sponsor. label Jul 24, 2025
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of enumNames is not compliant with the JSON schema specification. I've called out the locations where changes are needed and what the options are to associate a title with an enumerated value.

In short, enum is a valid keyword, but there is no acceptable way to associate a human readable title with it. The alternative is oneOf and anyOf used with const + title.

@chughtapan
Copy link
Contributor Author

@cliffhall Thank you the review. I think that makes perfect sense.
I have started #1148 which updates the elicitation docs & schema based on your guidance above. Once that is accepted I can make the same changes here as well.

@evalstate
Copy link
Member

Thanks @chughtapan - I was going to say that adding optional defaults is separate to the enumNames discussion (#1035 is also not a breaking change).

@chughtapan
Copy link
Contributor Author

Thanks @evalstate - Yeah I think it makes perfect sense to keep this a non-breaking change.
@cliffhall I'll mark these comments as resolved for now- happy to incorporate these changes into this or a separate PR based on the discussion on #1148

@cliffhall
Copy link
Member

Thanks @evalstate - Yeah I think it makes perfect sense to keep this a non-breaking change. @cliffhall I'll mark these comments as resolved for now- happy to incorporate these changes into this or a separate PR based on the discussion on #1148

I added some thoughts about the actual schema changes needed in that PR. We need to make sure that the schema supports single-selection and multi-selection from an enumerated set. I proposed a schema shape that should cover that.

@evalstate evalstate changed the title SEP: Add default values for all primitive types in elicitation schemas SEP: 1034 Add default values for all primitive types in elicitation schemas Aug 7, 2025
@evalstate
Copy link
Member

@chughtapan - i see that this now has conflicts in elicitation.mdx. would you mind updating please.

@chughtapan chughtapan requested a review from evalstate August 7, 2025 18:30
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

"title": "Environment",
"enum": ["development", "staging", "production"],
"enumNames": ["Development", "Staging", "Production"]
"enumNames": ["Development", "Staging", "Production"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes counter to our other proposal for enums doesn't it?

Copy link
Member

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did one last pass over this PR and it all LGTM

(Change was accepted by the Core Maintainers on 2025-08-22 via async discord vote)

@pcarleton
Copy link
Member

this needs a Docs maintainer for final ✅

maybe @bhosmer-ant ?

@bhosmer-ant bhosmer-ant merged commit 69a292b into modelcontextprotocol:main Sep 3, 2025
2 checks passed
chughtapan pushed a commit to chughtapan/python-sdk that referenced this pull request Sep 3, 2025
Implement support for default values in elicitation schemas as specified
in SEP-1034. The Python SDK already supported this through Pydantic's
automatic inclusion of defaults in JSON schemas - only tests and
documentation updates were needed.

modelcontextprotocol/modelcontextprotocol#1034
modelcontextprotocol/modelcontextprotocol#1035
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants