Run stateful and stateless conformance endpoints in a single server process#1672
Run stateful and stateless conformance endpoints in a single server process#1672halter73 wants to merge 2 commits into
Conversation
Follow-up cleanups to the conformance test infrastructure, squashed from three
working commits.
Serve both conformance lifecycles from one server. The conformance tests
previously ran two ConformanceServer instances: a stateful one
(ConformanceServerFixture, served at the default endpoint) and a stateless one
that each test spun up on its own fixed port (a StatelessMrtrConformanceServerFixture
for MRTR, plus a StatelessConformanceServer helper that the SEP-2243 and caching
tests started per-test on hand-picked port ranges). Differentiating the stateful
server per target framework while pinning the stateless servers to fixed ports
was inconsistent and prone to TCP TIME_WAIT conflicts on Windows.
Borrow TestSseServer's HandleStatelessMcp trick to host both lifecycles on a
single Kestrel port: the ConformanceServer now maps the stateful MCP server at
"/" and a separate stateless MCP server (its own ServiceCollection/
ApplicationBuilder so the isolated HttpServerTransportOptions.Stateless value
does not collide) at "/stateless". The --stateless/MCP_CONFORMANCE_STATELESS
switch is gone since one process serves everything.
A single shared ConformanceServerFixture (in its own file) now exposes ServerUrl
("/") and StatelessServerUrl ("/stateless"), and both ServerConformanceTests and
CachingConformanceTests consume it via [Collection(nameof(ConformanceServerCollection))].
This centralizes the per-framework port-binding logic, removes the now-redundant
StatelessMrtrConformanceServerFixture and StatelessConformanceServer helper, and
means no test restarts a server on a fixed port.
Also drop the stray space in the commented-out [InlineData("http-custom-headers")]
line in ClientConformanceTests so it follows the usual convention for code meant
to be uncommented later.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| services.AddSingleton(app.Services.GetRequiredService<DiagnosticListener>()); | ||
| services.AddRoutingCore(); | ||
|
|
||
| ConfigureConformanceMcpServer(services, subscriptions, stateless: true); |
There was a problem hiding this comment.
nit: the stateless server here reuses the same subscriptions dictionary as the stateful server on the main host. It's harmless today: the subscribe/unsubscribe handlers throw on the null stateless SessionId before touching the dictionary, and stateful session IDs are unique GUIDs, so the key spaces never collide. Still, these are two independent servers in separate DI containers, so sharing one mutable dictionary is a bit of a smell with no real upside. Could hand the stateless server its own new() dictionary to keep the two fully isolated.
There was a problem hiding this comment.
Good catch. Registering the subscribe handlers unconditionally flips on Capabilities.Resources.Subscribe, so the stateless server advertised resources.subscribe and then rejected every call with -32603 InternalError. Subscriptions can't work statelessly anyway until #1662 is addressed (no persistent SSE stream for notifications/resources/updated), so I gated both handlers behind if (!stateless). The stateless server no longer advertises the capability, and the subscriptions dictionary now lives in the stateful branch that is its only user.
I plan to add support for subscriptions back to the stateless endpoint as part of my PR to address #1662. Can you reapprove? Thanks!
tarekgh
left a comment
tarekgh
left a comment
There was a problem hiding this comment.
LGTM, nice cleanup. Hosting both lifecycles on one Kestrel port (stateful and stateless) gets rid of the fixed-port juggling that was causing the Windows TIME_WAIT flakiness, and consolidating onto a single shared ConformanceServerFixture means no test restarts a server on a fixed port anymore and the redundant stateless fixtures/helpers go away. I left one non-blocking nit if you want to consider it.
Addresses Tarek's review nit on #1672. The stateless server at "/stateless" shared the stateful server's subscriptions dictionary, which he flagged as a smell. The deeper issue: registering WithSubscribeToResourcesHandler / WithUnsubscribeFromResourcesHandler unconditionally set Capabilities.Resources.Subscribe = true, so the stateless server advertised resources.subscribe in its initialize result and then rejected every resources/subscribe call with -32603 (InternalError) via the null-SessionId guard -- the "server bug" error code for a capability it deliberately can't honor. Resource subscriptions are meaningless in the stateless lifecycle anyway: there is no stable SessionId to key the subscription table and no persistent SSE stream to deliver notifications/resources/updated. So gate the two handlers behind `if (!stateless)`. The stateless server no longer advertises resources.subscribe (an actual subscribe now gets the SDK's standard capability rejection), and the subscriptions dictionary is scoped to the stateful branch that is its only user -- answering "does the stateless server need a dictionary?" with a plain no. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
3 participants
Follow-up to #1671 (squash-merged). This final commit was prepared on top of that PR but not included in the merge; this PR lands it on its own.
The conformance tests previously ran two
ConformanceServerinstances: a stateful one (the shared fixture, at the default endpoint) and a stateless one that each test spun up on its own fixed port — a dedicatedStatelessMrtrConformanceServerFixturefor MRTR, plus aStatelessConformanceServerhelper that the SEP-2243 and caching tests started per-test on hand-picked port ranges. Differentiating the stateful server per target framework while pinning the stateless servers to fixed ports was inconsistent and prone to TCPTIME_WAITconflicts on Windows.This borrows
TestSseServer'sHandleStatelessMcptrick to host both lifecycles on a single Kestrel port:ConformanceServernow maps the stateful MCP server at/and a separate stateless MCP server at/stateless. The stateless server lives in its ownServiceCollection/ApplicationBuilderso its isolatedHttpServerTransportOptions.Statelessvalue doesn't collide with the stateful registration. The--stateless/MCP_CONFORMANCE_STATELESSswitch is gone since one process serves everything.ConformanceServerFixture(in its own file) exposesServerUrl(/) andStatelessServerUrl(/stateless), and bothServerConformanceTestsandCachingConformanceTestsconsume it via[Collection(nameof(ConformanceServerCollection))]. This centralizes the per-framework port-binding logic and removes the now-redundantStatelessMrtrConformanceServerFixtureandStatelessConformanceServerhelper, so no test restarts a server on a fixed port.[InlineData("http-custom-headers")]line inClientConformanceTeststo follow the usual convention for code meant to be uncommented later.