Skip to content

Fix missing resultType on complete result responses#1684

Open
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan/fix-resulttype-complete-response
Open

Fix missing resultType on complete result responses#1684
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan/fix-resulttype-complete-response

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Contributor

Summary

  • set ResultType at server response construction paths to complete when omitted
  • keep Result model default nullable (no model-level default)
  • update EmptyResult.Instance to represent a completed response and reuse it at call sites
  • expand tests to assert deserialized ResultType == "complete" across the methods covered in Draft protocol: complete result responses omit required resultType #1676, including server/discover

Validation

  • dotnet build
  • targeted test runs for updated suites in ModelContextProtocol.Tests

Fixes #1676

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 <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for Result-derived responses).
  • Update EmptyResult.Instance to 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

Draft protocol: complete result responses omit required resultType

2 participants