Skip to content

Gate application result fields by protocol version - #1816

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

Gate application result fields by protocol version#1816
KirschBluteX wants to merge 2 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

Add a single typed response-emission boundary after handlers and filters for protocol-version-specific result properties.

  • Legacy requests strip resultType and, for cacheable results, ttlMs and cacheScope, and emit a warning when application or filter code supplied them.
  • 2026-07-28 and later requests preserve explicit values and add the required defaults (complete, 0, and private).
  • The boundary edits freshly serialized JSON nodes, so shared result instances are not mutated across requests or protocol versions.
  • Normal, cacheable, immediate alternate, and returned/thrown MRTR result paths retain their type metadata through emission.
  • Raw McpServerOptions.RequestHandlers are intentionally outside this typed boundary; they return already-serialized JSON and the existing API contract defines them as bypassing typed handler infrastructure.

The policy is strip plus warning: it keeps legacy wire responses schema-valid and makes application misuse visible, following the SDK's existing compatibility-warning convention without converting otherwise valid requests into errors.

Tests

  • Full ModelContextProtocol.slnx Release build: 0 warnings, 0 errors.
  • Core protocol-result wire-shape tests: 8 passed.
  • Core MRTR emission/backcompat tests: 3 passed.
  • ASP.NET Core focused HTTP wire test: 1 passed.
  • Core non-Docker suite (Execution!=Manual, net10.0): 2,360 passed, 4 skipped, 0 failed.
  • ASP.NET Core non-Docker suite (Execution!=Manual, net10.0): 616 passed, 30 skipped, 0 failed.
  • git diff --check and targeted dotnet format --verify-no-changes passed for all changed files.
  • Repository-wide format verification still reports pre-existing whitespace diagnostics in untouched files; those unrelated files were not changed.
  • Docker Everything Server connection test passed. The unmodified sampling test still calls the removed trigger-sampling-request tool and fails before the SDK sampling handler; the current v1 image exposes sampleLLM. A diagnostic rerun with only that tool name updated passed both Docker tests (2/2); no unrelated fixture change is included here.
  • GitHub Actions are currently action_required pending maintainer approval; they were not rerun.

The branch is rebased onto upstream main at 8754e19 (the stateless protocol-span fix); that change does not alter the result-emission boundary.

@KirschBluteX
KirschBluteX force-pushed the fix/protocol-result-gating-1754 branch from 08e678b to ad347ea Compare August 14, 2026 19:04
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