[Schema][Server] feat: support sampling with tools - #409
Conversation
chr-hertel
left a comment
There was a problem hiding this comment.
Thanks @wWzZb for working on this - please extend the CHANGELOG.md for upcomgin release 0.8.0, extend docs and example where meaningful and double-check if there's still missing unit tests here and there - i at least caught one place where i think it makes sense to add some.
Thanks again, great addition for feature parity with the spec! 👍
| public readonly TextContent|ImageContent|AudioContent|ToolUseContent|array $content, | ||
| public readonly string $model, | ||
| public readonly ?string $stopReason = null, | ||
| public readonly SamplingStopReason|string|null $stopReason = null, |
There was a problem hiding this comment.
if we adopt the enum here, we can basically drop string, right?
| public readonly SamplingStopReason|string|null $stopReason = null, | |
| public readonly ?SamplingStopReason $stopReason = null, |
There was a problem hiding this comment.
I kept the string arm intentionally. The current MCP schema defines stopReason as an open string so clients can return provider-specific reasons, while SamplingStopReason documents the standard values. fromArray() now maps known values to the enum and preserves unknown values; the provider-specific path is covered by a unit test. See https://modelcontextprotocol.io/specification/2025-11-25/schema#create-message-result.
|
Addressed the review feedback in 991fe4f:
I kept Local checks: |
|
Follow-up spec audit in 0b1cb6d:
Checks: |
What
toolsandtoolChoiceto sampling requests and wire them throughClientGateway::sample().sampling.contextandsampling.toolsclient capabilities.Why
The SDK did not yet model the SEP-1577 protocol additions, so servers could not express tool-enabled sampling loops through the typed API.
Impact
Existing single-content sampling calls remain supported. The new constructor parameters are optional and appended to preserve current call sites.
Checks
make csmake phpstanmake tests— 938 tests, 2786 assertions, 7 existing skipsmake docsCloses #155