Skip to content

Fix protocol version tag for stateless server spans - #1810

Open
luisangelrod wants to merge 4 commits into
modelcontextprotocol:mainfrom
luisangelrod:fix/protocol-version-activity-tag
Open

Fix protocol version tag for stateless server spans#1810
luisangelrod wants to merge 4 commits into
modelcontextprotocol:mainfrom
luisangelrod:fix/protocol-version-activity-tag

Conversation

@luisangelrod

Copy link
Copy Markdown
Contributor

Summary

  • Use the per-request protocol version when tagging server activities before session-level negotiation state is populated
  • Add stateless HTTP coverage for the 2026-07-28 protocol version tag

Fixes #1800

Testing

  • Focused stateless protocol-version activity test: 1 passed
  • ModelContextProtocol.Tests DiagnosticTests on net10.0: 3 passed
  • MapMcpStatelessTests on net10.0: 32 passed, 20 skipped as expected

@jeffhandley

Copy link
Copy Markdown
Contributor

Thanks for submitting this, @luisangelrod! Tagging @tarekgh for review.

Comment on lines +39 to +42
Assert.Contains(activities, activity =>
activity.DisplayName == "tools/list" &&
activity.Kind == ActivityKind.Server &&
activity.GetTagItem("mcp.protocol.version") as string == McpProtocolVersions.July2026ProtocolVersion);

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.

From what I can tell, this assert could have a false-positive here if the negotiated version is also the July2026ProtocolVersion.

Please augment the test(s) to force scenarios of the negotiated version and the per-request version being different (in both directions if possible) and also being the same on multiple versions, asserting the tag item and also asserting the negotiated version.

@tarekgh tarekgh Aug 13, 2026

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.

I updated the tests to address the false positive and the coverage you asked for:

  • Replaced the single-version fact with a theory over 2025-11-25 and 2026-07-28 in stateless mode. In stateless mode each request is served by a fresh, un-negotiated server, so NegotiatedProtocolVersion is null when the span is tagged. That means the tag can only come from the per-request header/_meta value, and running two versions rules out a coincidental match on a single version. It would regress to an absent tag if the per-request fallback were removed.
  • Both tests now assert the tag item and also capture and assert NegotiatedProtocolVersion.
  • Added a stateful test (2025-11-25) that runs in both stateful and stateless modes. In stateful mode the session actually holds a negotiated version, and the test asserts the span tag and the negotiated version agree.

On forcing the negotiated and per-request versions to differ: I looked into this and did not add it, because I do not think it can be exercised through a valid flow. A single MCP session must not change protocol versions, so the server rejects any follow-up request whose header/_meta version differs from the negotiated one, and a conformant client never sends a differing header. The only way to observe divergence is to hand-craft a request that fails version-change validation, and asserting the tag on that path would pin the current ordering of tagging relative to validation, which is an implementation detail rather than a guarantee. Instead I cover the two inputs to the tag independently: the per-request path in stateless mode (negotiated is null at tagging time) and the negotiated-only fallback over a header-less transport (DiagnosticTests).

If you would still like an explicit divergence assertion via a raw crafted request, I am happy to add one. Let me know which you prefer.

Cover per-request protocol version tagging across multiple versions in stateless mode, assert the negotiated version alongside the tag, and add a stateful test asserting the negotiated version drives the tag.

@tarekgh tarekgh 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.

I pushed small test changes, LGTM otherwise.

Thanks @luisangelrod for helping with this fix.

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.

mcp.protocol.version activity tag not emitted on server spans in stateless HTTP mode

4 participants