test(e2e): audit transport-restricted requirements for the entry arms; admit two and harden entryModern#2312
Merged
felixweinberger merged 2 commits intoJun 16, 2026
Conversation
…e drift - assert right after connect that an entryModern connection actually negotiated 2026-07-28, so a broken negotiation pin fails as one attributable arm-level error instead of hundreds of downstream assertion failures - attachModernEnvelope now captures the envelope from the latest enveloped message (plain assignment instead of first-capture), so a renegotiated connection would not keep stamping a stale protocol-version claim
…y arms flow:tool-result:resource-link-follow and custom-methods:notification-handler were restricted to the stateful transports before the createMcpHandler entry arms existed, but their bodies only need request-scoped delivery (plain client-to-server requests, with related notifications observed after the call completes), so the per-request entry serves them unmodified. Add the entryStateless/entryModern arms to their transports and explain the inclusion in their notes (+4 cells, all passing). The remaining transport-restricted requirements were reviewed and stay restricted: they need a server-to-client back-channel, a persistent session or standalone stream, or assert genuinely transport-specific behavior.
|
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audits every transport-restricted requirement in the e2e manifest for applicability to the createMcpHandler entry arms, admits the two whose bodies the per-request entry can already serve, and hardens the entryModern arm against negotiation/envelope drift.
Motivation and Context
The entry arms added in #2309 opt in every requirement without an explicit
transportsrestriction; the ~240 rows that carry one were authored before the entry existed and never reach the arms. This pass reviews each of them so that nothing is skipped silently: requirements the per-request entry can serve today are admitted, and the rest are confirmed as blocked on missing entry features (server→client requests, sessions/standalone streams) or as genuinely transport-specific.flow:tool-result:resource-link-followandcustom-methods:notification-handler. Both bodies need only request-scoped delivery, so the per-request entry serves them unmodified on both eras; their notes explain the inclusion. No scenario body was changed and no knownFailure or exclusion was added.createMcpHandlerneeds dedicated entry-side scenarios, which this review calls out as follow-up coverage alongside the other generic HTTP-mechanics behaviors that deserve dedicated entry-side cells later (several already have them).How Has This Been Tested?
Full e2e matrix before/after on the same machine: 2536 passed / 205 expected-fail / 2741 cells → 2540 / 205 / 2745, zero unexpected failures in both runs. Package typecheck and lint for the e2e workspace are green. No package source, examples, or conformance changes.
Breaking Changes
None — test-only.
Types of changes
Checklist
Additional context
One candidate that looked admissible analytically — logging emitted via the request context (
ctx.mcpReq.log) while the handler is still running — stays restricted, and the reason is sharper than a missing delivery channel: those notifications are emitted without arelatedRequestId, and the per-request transport drops messages that are not related to the single in-flight request, so handler-context logs are never delivered when serving throughcreateMcpHandler. Request-related notifications and progress do ride the in-flight exchange (which is why those cells pass on the entry arms). Whether handler-context logging should be emitted as request-related on per-request serving is left as an explicit follow-up rather than changed here.