Skip to content

test(e2e): hosting-entry-http arm-posture fix; widen method-405 probe set #2334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
felixweinberger wants to merge 2 commits into
base: fweinberger/on-m12
Choose a base branch
from
+8 −6
Open

test(e2e): hosting-entry-http arm-posture fix; widen method-405 probe set #2334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5cbdfe
chore: design-proposal round marker (#19 SEP-2106, #39 client caching)
felixweinberger Jun 19, 2026
9fcdcc2
test(e2e): hosting-entry-http arm-posture fix; widen method-405 probe…
felixweinberger Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/e2e/requirements.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2405,9 +2405,9 @@ export const REQUIREMENTS: Record<string, Requirement> = {
'typescript:hosting:entry:method-405': {
source: 'sdk',
behavior:
'An unsupported HTTP method (PUT, PATCH) on a createMcpHandler endpoint is answered 405 with a JSON-RPC Method-not-allowed body on both legs: the stateless legacy fallback rejects every non-POST method, and the modern-only strict path rejects body-less non-POST traffic via the modern-only-method-not-allowed cell.',
'A non-POST HTTP method (GET, DELETE, PUT, PATCH) on a createMcpHandler endpoint is answered 405 with a JSON-RPC Method-not-allowed body on both legs: the stateless legacy fallback rejects every non-POST method, and the modern-only strict path rejects body-less non-POST traffic via the modern-only-method-not-allowed cell.',
transports: ['entryStateless', 'entryModern'],
note: 'Runs on the createMcpHandler entry arms; the unsupported methods are POSTed through wired.fetch so the HTTP status and body are observed directly. The entry does not emit an Allow header (the per-session server transport does), so only the status and JSON-RPC error shape are pinned.'
note: 'Runs on the createMcpHandler entry arms; each non-POST method is sent through wired.fetch so the HTTP status and body are observed directly. The entry does not emit an Allow header (the per-session server transport does), so only the status and JSON-RPC error shape are pinned.'
},
Comment on lines 2405 to 2411

@claude claude Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟣 Pre-existing SDK gap, unrelated to this test-only PR: both 405 paths reachable through createMcpHandler (the legacy stateless fallback's non-POST rejection and the modern-only strict modern-only-method-not-allowed rejection) return 405 without the Allow header that RFC 9110 §15.5.6 requires, even though the per-session WebStandardStreamableHTTPServerTransport already emits Allow: GET, POST, DELETE on its 405. The eventual fix is to emit Allow: POST from the two entry-side 405 sites — at which point the requirement note reworded here ("The entry does not emit an Allow header…") will need updating.

Extended reasoning...

What the gap is. RFC 9110 §15.5.6 says an origin server generating a 405 (Method Not Allowed) response MUST generate an Allow header field listing the methods the target resource supports. Both 405 paths reachable through createMcpHandler omit it:

  1. The legacy stateless fallback's non-POST rejection at packages/server/src/server/createMcpHandler.ts:309-311 returns jsonRpcErrorResponse(405, -32000, 'Method not allowed.'), and jsonRpcErrorResponse (lines 261-270) builds the Response.json with only a status — no headers.
  2. The modern-only strict path's modern-only-method-not-allowed rejection at packages/core/src/shared/inboundClassification.ts:856-858 is rendered through rejectionResponse() (createMcpHandler.ts:272-274), which delegates to the same header-less jsonRpcErrorResponse.

Why it's an inconsistency, not a design choice. The per-session WebStandardStreamableHTTPServerTransport already complies: its handleUnsupportedRequest emits Allow: 'GET, POST, DELETE' on its 405 (packages/server/src/server/streamableHttp.ts:625). So the SDK follows the RFC on the sibling transport but not on the two entry-side sites — a parity gap rather than a deliberate omission.

How this PR interacts with it. The PR is test-only and does not introduce the gap — the sentence "The entry does not emit an Allow header (the per-session server transport does)" already appears verbatim in the line being replaced. But the PR widens the method-405 probe loop from ['PUT','PATCH'] to ['GET','DELETE','PUT','PATCH'] and rewords the requirement note at test/e2e/requirements.ts:2405-2411, so it now documents and observes (without asserting) the non-compliant 405 shape for four methods on both entry arms.

Step-by-step proof. (1) The widened loop sends GET to the entryStateless arm's URL via wired.fetch. (2) createMcpHandler routes the body-less non-POST request to legacyStatelessFallback, which hits the request.method !== 'POST' guard at createMcpHandler.ts:309 and returns jsonRpcErrorResponse(405, -32000, 'Method not allowed.'). (3) jsonRpcErrorResponse constructs the response with { status: 405 } only, so response.headers.get('allow') is null — violating the RFC 9110 MUST. (4) On the entryModern arm the same GET is classified as modern-only-method-not-allowed (inboundClassification.ts:856-858) and rendered by rejectionResponse() → the same header-less builder, so Allow is again absent. (5) By contrast, sending PUT to a hand-hosted per-session transport reaches handleUnsupportedRequest at streamableHttp.ts:611-625 and gets Allow: GET, POST, DELETE back.

Impact and fix. Impact is HTTP-compliance/interop polish: well-behaved generic HTTP clients and proxies use Allow to discover supported methods after a 405. The fix is a one-liner per site — emit Allow: 'POST' (the only method either entry leg serves) from the two entry-side 405 paths. That change belongs in an SDK PR, not this test-only one; when it lands, the requirement note text touched here (and the note that the test deliberately doesn't pin the header) should be updated to assert Allow: POST. Filing as pre_existing so it's tracked without blocking this PR.

'typescript:hosting:entry:parse-error-400': {
source: 'https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#sending-messages-to-the-server',
Expand Down
10 changes: 6 additions & 4 deletions test/e2e/scenarios/hosting-entry-http.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ function echoFactory(_ctx?: McpRequestContext): McpServer {

verifies('typescript:hosting:entry:method-405', async ({ transport }: TestArgs) => {
const client = new Client({ name: 'method-405-client', version: '1.0.0' });
await using wired = await wire(transport, echoFactory, client, { entry: { legacy: 'stateless' } });
// No `entry` override: the arm posture (`stateless` on entryStateless,
// `reject` on entryModern) is the configuration under test.
await using wired = await wire(transport, echoFactory, client);
Comment on lines 33 to +37

@claude claude Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The same arm-posture override this PR removes still survives in the sibling file at test/e2e/scenarios/hosting-entry.test.ts:126 — the notification-202 cell passes { entry: { legacy: 'stateless' } }, so its entryModern arm silently runs legacy: 'stateless' instead of the documented 'reject' posture. The assertion still passes either way, so this is just a coverage-fidelity cleanup: drop the override there too (the one at line 39, dual-era-one-factory, is intentional and should stay).

Extended reasoning...

What the bug is. This PR removes the wire(..., { entry: { legacy: 'stateless' } }) override from method-405, parse-error-400, and no-session-id in hosting-entry-http.test.ts, on the rationale that the helper spreads the override after the arm posture, so on entryModern the handler silently runs dual-era stateless instead of the arm's documented modern-only strict (legacy: 'reject'). Exactly one more leftover instance of that same pattern survives outside the diff: the typescript:hosting:entry:notification-202 cell at test/e2e/scenarios/hosting-entry.test.ts:126.

The code path. wire() builds the entry config at test/e2e/helpers/index.ts:151 as transport === 'entryStateless' ? { legacy: 'stateless', ...sniff.entry } : { legacy: 'reject', ...sniff.entry } — sniff.entry wins. notification-202 runs on both ['entryStateless', 'entryModern'] arms (requirements.ts), so on the entryModern arm the passed { legacy: 'stateless' } replaces 'reject' and the cell hosts a dual-era stateless handler rather than the modern-only strict endpoint the file header and test/e2e/CLAUDE.md document for that arm.

Step-by-step proof. (1) The matrix schedules notification-202 on entryModern. (2) wire('entryModern', greetFactory, client, { entry: { legacy: 'stateless' } }) reaches helpers/index.ts:151 and evaluates { legacy: 'reject', ...{ legacy: 'stateless' } } → { legacy: 'stateless' }. (3) createMcpHandler is therefore created with the legacy-stateless slot enabled — the same configuration the entryStateless arm already covers. (4) The test then sends the enveloped 2026-era notifications/cancelled, which rides the modern leg under either posture and gets 202, so the test still passes — but the 202-for-enveloped-notification behavior is never observed under the strict (legacy: 'reject') posture this arm exists to test.

Why nothing prevents it. The override is silent: nothing warns that a per-test entry override contradicts the arm posture, and the assertion is posture-insensitive, so the matrix stays green while the entryModern cell is effectively a config-near-duplicate of entryStateless for the handler configuration.

Why line 39 is excluded. The other surviving site, hosting-entry.test.ts:39 (dual-era-one-factory), is intentional — its requirement explicitly pins the same one-factory, legacy-stateless-slot handler shape on both arms, so the override there is the configuration under test. The notification-202 requirement note only says the arm selects which leg the notification rides; it never asks for the stateless slot on entryModern.

Impact and fix. No test failure — purely a coverage-fidelity gap and an inconsistency with the principle this PR states ('the arm posture is the configuration under test'). Fix is the same one-line change applied to the three cells in this PR: drop the fourth argument at hosting-entry.test.ts:126 so the entryModern arm actually exercises legacy: 'reject'. Worth folding into this PR since it completes the cleanup, but non-blocking.


for (const method of ['PUT', 'PATCH']) {
for (const method of ['GET', 'DELETE', 'PUT', 'PATCH']) {
const response = await wired.fetch!(wired.url!, { method });
expect(response.status).toBe(405);
const body = (await response.json()) as { jsonrpc: string; error: { code: number; message: string } };
Expand All @@ -46,7 +48,7 @@ verifies('typescript:hosting:entry:method-405', async ({ transport }: TestArgs)

verifies('typescript:hosting:entry:parse-error-400', async ({ transport }: TestArgs) => {
const client = new Client({ name: 'parse-error-client', version: '1.0.0' });
await using wired = await wire(transport, echoFactory, client, { entry: { legacy: 'stateless' } });
await using wired = await wire(transport, echoFactory, client);

const response = await wired.fetch!(wired.url!, {
method: 'POST',
Expand Down Expand Up @@ -138,7 +140,7 @@ verifies('typescript:hosting:entry:legacy-protocol-version-default', async ({ tr

verifies('typescript:hosting:entry:no-session-id', async ({ transport }: TestArgs) => {
const client = new Client({ name: 'no-session-id-client', version: '1.0.0' });
await using wired = await wire(transport, echoFactory, client, { entry: { legacy: 'stateless' } });
await using wired = await wire(transport, echoFactory, client);

// A typed round trip through the wired client (so both the connect-time
// negotiation and a follow-up request are recorded), then assert no
Expand Down
Loading