Skip to content

fix: use negotiated protocol version in server-sse-multiple-streams raw POSTs - #415

Merged
pcarleton merged 1 commit into
mainfrom
fix/sse-multiple-streams-negotiated-version
Jul 27, 2026
Merged

fix: use negotiated protocol version in server-sse-multiple-streams raw POSTs#415
pcarleton merged 1 commit into
mainfrom
fix/sse-multiple-streams-negotiated-version

Conversation

@claude

@claude claude Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Before

Running the stable server-sse-multiple-streams scenario against a stateful server that enforces the protocol version negotiated during initialize (as the MCP lifecycle requires) false-failed with:

Server rejected some requests. Statuses: 400, 400, 400

The scenario initialized through the SDK's StreamableHTTPClientTransport (negotiating e.g. 2025-11-25), but its three raw concurrent follow-up POSTs hard-coded mcp-protocol-version: 2025-03-26. A version-enforcing server correctly rejected all three, so the scenario failed before ever exercising the multi-stream concurrency behavior under test.

After

The raw POSTs carry the version actually negotiated during initialize, so the scenario passes against version-enforcing servers and tests what it is meant to test. Servers that don't enforce the version are unaffected.

How

In src/scenarios/server/sse-multiple-streams.ts, after client.connect(transport) the scenario now reads the SDK transport's public protocolVersion getter into negotiatedProtocolVersion (alongside the existing session-id extraction) and uses it for the stateful mcp-protocol-version request header, falling back to the run's specVersion if the transport exposes none. The stateless (draft) branch is unchanged.

src/scenarios/server/sse-multiple-streams.test.ts adds regression coverage: a fixture HTTP server (built in the test file, following the pattern in http-standard-headers.test.ts) stores the protocolVersion it returns from initialize and 400s any later request whose MCP-Protocol-Version header doesn't match. The scenario now passes against it (verified to fail with the previous hard-coded header), and a second test pins that the fixture rejects the supported-but-non-negotiated 2025-03-26.

Fixes #412

The stateful server-sse-multiple-streams scenario hard-coded
mcp-protocol-version: 2025-03-26 on its three raw concurrent POSTs
instead of the version negotiated during initialize. A server that
enforces the negotiated session version correctly returns 400 to all
three POSTs, so the scenario false-failed before exercising the
multi-stream behavior under test.

Read transport.protocolVersion after client.connect() and use it for
the raw POST headers, falling back to the run's spec version if the
transport does not expose one. Add a regression test driving the
scenario against a fixture server that 400s any request whose
MCP-Protocol-Version header does not match what it negotiated.

Fixes #412

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0178cz8nAyFRHQyq9cg2XgZz
@pkg-pr-new

pkg-pr-new Bot commented Jul 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

npx https://pkg.pr.new/@modelcontextprotocol/conformance@415

commit: 0c21832

@pcarleton
pcarleton marked this pull request as ready for review July 27, 2026 10:59
@pcarleton
pcarleton merged commit 19a97f0 into main Jul 27, 2026
8 checks passed
@pcarleton
pcarleton deleted the fix/sse-multiple-streams-negotiated-version branch July 27, 2026 10:59
pcarleton pushed a commit that referenced this pull request Jul 27, 2026
…ts (#416)

* fix: use negotiated protocol version in sse-polling raw requests

The server-sse-polling scenario hard-coded mcp-protocol-version:
2025-11-25 on its raw tools/call POST and Last-Event-ID reconnection
GET instead of the version negotiated during initialize — the same
bug class as #412. A server that enforces the negotiated session
version rejects those requests once it negotiates any other version.

Read transport.protocolVersion after client.connect() and use it for
both raw request headers, falling back to the run's spec version,
mirroring the sse-multiple-streams fix (#415). Add a regression test
whose fixture server down-negotiates to 2025-06-18 and 400s any
request whose MCP-Protocol-Version header does not match, so a fix
that hard-codes the latest version cannot pass.

Follow-up to #415.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0178cz8nAyFRHQyq9cg2XgZz

* test: harden sse-polling regression fixture per review panel

Enforce session id alongside protocol version in the fixture, record
Last-Event-ID presence per GET so the SDK's automatic standalone GET
cannot satisfy the reconnect assertion, normalize the header type
instead of casting, return a descriptive 400 body on the GET path,
add Allow to 405s, use the spec-shaped empty-data priming event, and
read sessionId via the SDK's public getter in the scenario.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0178cz8nAyFRHQyq9cg2XgZz

---------

Co-authored-by: Claude <noreply@anthropic.com>
pcarleton added a commit to pugsatoshi/conformance that referenced this pull request Jul 31, 2026
Review follow-ups on the session-termination change:

- terminateSessionRaw: abort the DELETE after 5s so a wedged server
  cannot stall suite teardown.
- terminateSessionBestEffort: shared SDK-path teardown helper that
  delegates to terminateSessionRaw with the transport's negotiated
  protocol version, so the socket is truly released at the bound and
  the version-mismatch 400 on strict servers (the failure mode of
  issue modelcontextprotocol#79) goes away. Used by connectToServer close() and both SSE
  scenarios, replacing their inline try/catch blocks.
- lifecycle.ts: the raw session-id probe and its teardown DELETE now
  use ctx.specVersion instead of a hardcoded 2025-11-25, matching the
  negotiated-version convention from modelcontextprotocol#415/modelcontextprotocol#416.
- Add sdk-client.test.ts regression tests asserting close() sends a
  DELETE with the issued session ID (and none without one), so a bad
  future merge cannot silently drop the fix.
pcarleton added a commit that referenced this pull request Jul 31, 2026
#316)

* fix: send HTTP DELETE to terminate server sessions after each scenario

The conformance test harness opened sessions against servers under test but never issued an HTTP DELETE to terminate them, leaving a dangling session on the server after each scenario.
This violates the "hermetic" expectation called out in the Streamable HTTP transport spec.

- `connection/sdk-client.connectToServer` now calls `transport.terminateSession()` before `client.close()`. A 405 response (DELETE not supported) is tolerated because the spec allows the server to refuse.
- A new `terminateSessionRaw` helper handles scenarios that bypass the SDK client and open sessions via raw `fetch` (`lifecycle.ts`, `http-standard-headers.ts`).
- `sse-polling.ts` and `sse-multiple-streams.ts`, which manage their own transport, now terminate the session in their existing `finally` block before closing the client.
- `lifecycle.test.ts` gains two cases asserting that DELETE is sent with the issued session ID and that no DELETE is sent when the server did not assign one.

Verified manually against `examples/servers/typescript/everything-server.ts` that running a server scenario now causes the server to log `Received session termination request for session <id>`.

Fixes #79

Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>

* Use negotiated protocol version for teardown DELETE and bound it

Review follow-ups on the session-termination change:

- terminateSessionRaw: abort the DELETE after 5s so a wedged server
  cannot stall suite teardown.
- terminateSessionBestEffort: shared SDK-path teardown helper that
  delegates to terminateSessionRaw with the transport's negotiated
  protocol version, so the socket is truly released at the bound and
  the version-mismatch 400 on strict servers (the failure mode of
  issue #79) goes away. Used by connectToServer close() and both SSE
  scenarios, replacing their inline try/catch blocks.
- lifecycle.ts: the raw session-id probe and its teardown DELETE now
  use ctx.specVersion instead of a hardcoded 2025-11-25, matching the
  negotiated-version convention from #415/#416.
- Add sdk-client.test.ts regression tests asserting close() sends a
  DELETE with the issued session ID (and none without one), so a bad
  future merge cannot silently drop the fix.

---------

Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>
Co-authored-by: Paul Carleton <paulc@anthropic.com>
pcarleton added a commit to pugsatoshi/conformance that referenced this pull request Jul 31, 2026
- Adapt run() to the RunContext signature introduced in modelcontextprotocol#318.
- Send the protocol version the server negotiated in the initialize
  result on follow-up requests instead of hardcoding 2025-11-25,
  falling back to the requested spec version (the sse-multiple-streams
  pattern from modelcontextprotocol#415/modelcontextprotocol#416). Parsing the initialize body is bounded by a
  5s timeout, and every fetch shares one AbortController aborted when
  the scenario ends, so a server answering with a never-ending SSE
  stream can neither hang the run nor leak a held-open connection.
- Fail the scenario when initialize itself fails; previously a broken
  server (401/500) was reported as stateless and passed.
- Check the initialized notification response and fail when it is
  rejected, emitting SKIPPED placeholders for the lifecycle checks so
  per-check aggregation keeps its rows.
- Report unexpected DELETE statuses as WARNING instead of FAILURE (the
  spec never pins a success status for session termination).
- Treat 404 on DELETE as an already-terminated session only when the
  follow-up request confirms the session is gone; a 404 from a server
  whose session still responds (likely no DELETE route) is a WARNING
  with the 404 check skipped, rather than a misattributed failure.
- Exclude the scenario from the draft spec version via removedIn:
  sessions were removed entirely by SEP-2575.
pcarleton added a commit that referenced this pull request Jul 31, 2026
…322)

* feat: add session-lifecycle conformance scenario for streamable HTTP

This commits a `server-session-lifecycle` scenario that verifies the server honors the two RFC 2119 statements the Streamable HTTP transport spec places on session termination:

1. After receiving an HTTP DELETE bearing the issued Mcp-Session-Id, the server accepts it (2xx) or signals that explicit termination is not supported (405).
2. Subsequent requests bearing the terminated session ID MUST get HTTP 404 Not Found.

The scenario inlines a raw `fetch` initialize/terminate/probe flow so the DELETE is the test action (not background cleanup).
Stateless servers that never issue a session ID are reported as INFO, and the lifecycle checks are SKIPPED.
Servers that return 405 on DELETE skip both checks without flagging a failure — the spec allows servers to refuse explicit termination.

Refs #79

Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>

* fix: address review feedback on session-lifecycle scenario

- Adapt run() to the RunContext signature introduced in #318.
- Send the protocol version the server negotiated in the initialize
  result on follow-up requests instead of hardcoding 2025-11-25,
  falling back to the requested spec version (the sse-multiple-streams
  pattern from #415/#416). Parsing the initialize body is bounded by a
  5s timeout, and every fetch shares one AbortController aborted when
  the scenario ends, so a server answering with a never-ending SSE
  stream can neither hang the run nor leak a held-open connection.
- Fail the scenario when initialize itself fails; previously a broken
  server (401/500) was reported as stateless and passed.
- Check the initialized notification response and fail when it is
  rejected, emitting SKIPPED placeholders for the lifecycle checks so
  per-check aggregation keeps its rows.
- Report unexpected DELETE statuses as WARNING instead of FAILURE (the
  spec never pins a success status for session termination).
- Treat 404 on DELETE as an already-terminated session only when the
  follow-up request confirms the session is gone; a 404 from a server
  whose session still responds (likely no DELETE route) is a WARNING
  with the 404 check skipped, rather than a misattributed failure.
- Exclude the scenario from the draft spec version via removedIn:
  sessions were removed entirely by SEP-2575.

---------

Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>
Co-authored-by: Paul Carleton <paulc@anthropic.com>
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.

server-sse-multiple-streams sends a non-negotiated protocol version

2 participants