You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Splits the session ID check in example servers so unknown session IDs return 404 (signaling clients to start a new session) while missing session IDs return 400.
Motivation and Context
Per spec, clients interpret 404 on a request with Mcp-Session-Id as "start a new session." The examples were returning 400 for both cases, breaking client recovery logic. Issue was reported by the VS Code team.
Supersedes #1331, #1560, #1678, #1707 — thanks @lraveri and @owendevereaux for the earlier work. This consolidates on a tighter diff and covers authTestServer.ts and jsonResponseStreamableHttp.ts which the previous PRs missed.
Per spec, clients interpret 404 as "start a new session" — the examples
were returning 400 for both missing and unknown session IDs, which breaks
client recovery logic. Split the check so unknown IDs return 404.
Fixes#389
The reason will be displayed to describe this comment to others. Learn more.
LGTM, straightforward mechanical split of session ID checks across example and conformance test servers.
Extended reasoning...
Overview
This PR modifies 7 example server files and 2 conformance test server files to split the combined session ID check into two separate branches: 400 for missing session ID, 404 for unknown session ID.
Security risks
None. Changes are in example and test servers only.
Level of scrutiny
Low. Mechanical, repetitive change to example and test code, not library code.
Other factors
Consolidates 4 previous PRs. Pattern is consistent across all files. No bugs found.
The reason will be displayed to describe this comment to others. Learn more.
Correct HTTP semantics — splitting the conflated !sessionId || !transports[sessionId] check into 400 (missing header = client error) vs 404 (session not found = resource gone, could be expiry/restart). Pattern applied consistently across all 8 example/test servers. LGTM.
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
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.
Splits the session ID check in example servers so unknown session IDs return 404 (signaling clients to start a new session) while missing session IDs return 400.
Motivation and Context
Per spec, clients interpret 404 on a request with
Mcp-Session-Idas "start a new session." The examples were returning 400 for both cases, breaking client recovery logic. Issue was reported by the VS Code team.Fixes#389
How Has This Been Tested?
pnpm check:allpasses. Covers all example servers plus conformance test servers.Breaking Changes
None — examples only.
Types of changes
Checklist
Additional context
Supersedes #1331, #1560, #1678, #1707 — thanks @lraveri and @owendevereaux for the earlier work. This consolidates on a tighter diff and covers
authTestServer.tsandjsonResponseStreamableHttp.tswhich the previous PRs missed.