-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: client-side support for SEP-1577 sampling with tools #1722
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
feat: client-side support for SEP-1577 sampling with tools #1722
Conversation
src/mcp/client/session.py
Outdated
| write_stream: MemoryObjectSendStream[SessionMessage], | ||
| read_timeout_seconds: timedelta | None = None, | ||
| sampling_callback: SamplingFnT | None = None, | ||
| sampling_capabilities: types.SamplingCapability | None = None, |
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.
thoughts on moving to the bottom of kwargs or keyword only to avoid breaking positional uses of the constructor? I'm probably leaning towards positional keyword only
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.
Good call! (assuming you meant leaning "keyword" only, and I agree - will update)
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.
woops yea meant keyword only haha
SEP-1577 added tool calling support to sampling (via #1594). This PR adds the necessary client-side support:
toolsortoolChoiceare provided butclientCapabilities.sampling.toolsis missing. The SDK's types already support this, butClientSessionhad no way to advertise it—servers would always see the capability as missing and error. This adds asampling_capabilitiesparameter. If not provided, the previous logic applies: the presence of asampling_callbackenables basic sampling support without tools.CreateMessageResultWithTools(which supports array content). TheSamplingFnTcallback andClientResultTypedidn't include this type, so the client's response validation would reject it before it could be sent back to the server. This addsCreateMessageResultWithToolsto both.FWIW, with these changes we have full agentic sampling wired up in a branch of FastMCP, which is extremely exciting!