Skip to content

Gate application result fields by protocol version - #1816

Open
KirschBluteX wants to merge 4 commits into
modelcontextprotocol:mainfrom
KirschBluteX:fix/protocol-result-gating-1754
Open

Gate application result fields by protocol version#1816
KirschBluteX wants to merge 4 commits into
modelcontextprotocol:mainfrom
KirschBluteX:fix/protocol-result-gating-1754

Conversation

@KirschBluteX

@KirschBluteX KirschBluteX commented Aug 13, 2026

Copy link
Copy Markdown

Fixes #1754.

Summary

Normalize protocol-version-specific properties on typed results at a single terminal response-emission boundary, after outgoing message filters have run.

  • Legacy requests remove resultType and, for cacheable results, ttlMs and cacheScope. The server emits at most one compatibility warning per method, including under concurrent or repeated emissions.
  • Requests using protocol version 2026-07-28 or later preserve explicit values and restore required defaults (resultType: "complete", ttlMs: 0, and cacheScope: "private") if a filter removes them.
  • Protocol selection uses the request-scoped version and falls back to the negotiated version for stateful sessions.
  • The terminal boundary clones and normalizes the post-filter JSON response before transport emission. This covers filter mutation, response replacement, and repeated next calls without mutating shared result instances.
  • Normal, cacheable, immediate alternate, and returned/thrown MRTR typed results all pass through the same boundary.
  • Raw McpServerOptions.RequestHandlers remain outside this boundary because they operate on already-serialized JSON and intentionally bypass the typed handler infrastructure.

The selected policy is strip plus warning. This keeps legacy wire responses schema-valid while making application misuse visible, following the SDK's existing compatibility-warning convention without turning otherwise valid requests into errors.

Tests

  • Full ModelContextProtocol.slnx Release build: 0 warnings, 0 errors.
  • Focused protocol-result gating tests: 21 passed.
  • Core protocol, MRTR, and message-filter regression set: 45 passed.
  • ASP.NET Core non-Manual suite (Execution!=Manual, net10.0): 617 passed, 30 skipped, 0 failed.
  • MapMcp and filter-focused tests: 75 passed, 9 skipped, 0 failed.
  • git diff --check and targeted dotnet format --verify-no-changes passed.
  • Two Docker Everything Server sampling tests were attempted but are not counted as validation: the pinned image returned Unknown tool: trigger-sampling-request before exercising this change.

@KirschBluteX
KirschBluteX force-pushed the fix/protocol-result-gating-1754 branch from 08e678b to ad347ea Compare August 14, 2026 19:04
@KirschBluteX

Copy link
Copy Markdown
Author

@jeffhandley @halter73 Could you review the updated head?

Since 0bf4349, commit 7f47075 moves typed-result normalization after outgoing filters and clones the post-filter response before protocol gating. This prevents filters from reintroducing legacy-incompatible resultType, ttlMs, or cacheScope, while keeping shared result instances unchanged. Regression tests cover normal, cacheable, immediate, and MRTR paths.

Could you confirm this is the intended boundary for #1754 and approve if it looks right?

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.

Define handling for protocol-incompatible result properties set by applications

2 participants