Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 29, 2025

  • Add DefaultSamplingMaxTokens property to McpServerOptions with a default value of 1000 tokens
  • Update sampling code in McpServer.Methods.cs to use the default instead of int.MaxValue
  • Fix test to mock ServerOptions property
  • Augment existing test to verify DefaultSamplingMaxTokens is respected by SampleAsync method
  • Build and test the changes - all 719 tests passing

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on audit protocol types PR Add configurable default for sampling MaxTokens Oct 29, 2025
Copilot AI requested a review from MackinnonBuck October 29, 2025 18:09
Copilot finished work on behalf of MackinnonBuck October 29, 2025 18:09
Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
Copilot AI requested a review from MackinnonBuck October 29, 2025 23:12
Copilot finished work on behalf of MackinnonBuck October 29, 2025 23:12
…endRequestAsync

Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
@stephentoub
Copy link
Contributor

What is this addressing / what is the impetus for this?

Copilot AI requested a review from MackinnonBuck October 29, 2025 23:24
Copilot finished work on behalf of MackinnonBuck October 29, 2025 23:24
@MackinnonBuck
Copy link
Collaborator

@stephentoub The CreateMessageRequestParams protocol type had an incorrectly optional MaxTokens property (the schema shows it as being required). #892 changes MaxTokens from an int? to a required int, but this means a value needs to be specified where it didn't previously. To avoid restructuring APIs like McpServer.SampleAsync(...) to always require a MaxTokens value, this new DefaultSamplingMaxTokens option controls the default.

See also #892 (comment) and #892 (comment)

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.

3 participants