Fix missing resultType on complete result responses#1684
Conversation
Set resultType at server response construction paths instead of relying on a model default. This ensures complete responses consistently carry resultType=complete while preserving task/input-required variants. Also expand server/client tests to assert deserialized ResultType across methods covered by modelcontextprotocol#1676, including server/discover. Fixes modelcontextprotocol#1676 Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR updates the C# MCP SDK server’s result construction paths to ensure resultType is present (and deserializes as "complete") for completed responses, aligning server outputs with the July 2026 / 2026-07-28 draft schema requirements highlighted in #1676. It also strengthens test coverage to prevent regressions across multiple server methods and server/discover.
Changes:
- Ensure server response results have
ResultType = "complete"when omitted (including a centralized post-handler fix-up forResult-derived responses). - Update
EmptyResult.Instanceto represent a completed response and reuse it at call sites. - Expand server/client tests to assert
"complete"is present after deserialization for covered result types.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/ModelContextProtocol.Tests/Server/McpServerTests.cs | Adds assertions that deserialized results include ResultType == "complete" across many server response paths. |
| tests/ModelContextProtocol.Tests/Client/July2026ProtocolConnectionTests.cs | Adds ResultType == "complete" assertions for server/discover results to validate July 2026 behavior. |
| src/ModelContextProtocol.Core/Server/McpServerImpl.cs | Sets ResultType explicitly for initialize/discover and adds handler-level fix-ups to default missing result types to "complete" (and "task" for task creation results). |
| src/ModelContextProtocol.Core/Protocol/Result.cs | Updates documentation wording around resultType semantics (currently introduces a doc inconsistency). |
| src/ModelContextProtocol.Core/Protocol/EmptyResult.cs | Updates EmptyResult.Instance to be pre-initialized as a completed result (ResultType = "complete"). |
| /// <remarks> | ||
| /// <para> | ||
| /// When absent or set to <c>"complete"</c>, the result is a normal completed response. | ||
| /// When set to <c>"complete"</c>, the result is a normal completed response. |
There was a problem hiding this comment.
I don't think we want to lock into these semantics. The spec says that client should treat null as complete but that is a behavior that shouldn't leak into the model IMO.
| }; | ||
| } | ||
|
|
||
| if (typeof(Result).IsAssignableFrom(typeof(TResult))) |
There was a problem hiding this comment.
This misses custom logging/setLevel handlers. ConfigureLogging registers directly with _requestHandlers.Set, and our test/conformance servers return new EmptyResult() there, so that response can still omit resultType. Can we route that path through SetHandler too and add a raw response assertion?
Summary
resultType#1676, including server/discoverValidation
Fixes #1676