Map protocol errors onto HTTP status codes (follow-up to #110) - #128
Merged
Conversation
The transport rules that #110 covered also say an unimplemented method MUST be 404 and an unsupported protocol version MUST be 400, with the JSON-RPC error still in the body. I flagged that as deliberately out of scope in PR #126; this is it. The body is the point. A modern server saying "I do not have that method" and a legacy server that does not host this endpoint at all both answer 404 - the JSON-RPC error in the body is exactly what lets a client tell them apart, which is how the backward-compatibility probe works. A tool that FAILS is still 200: the request was served, and the failure is in the result where the model can read it. Gated on the declared revision, like everything else here: a client written against 2025-06-18 may only read the body on 200, and nothing about its world changes. Also fixes a real hole found while wiring this. DeclaresRevisionAtLeast compared ordinally, and "latest" sorts above every dated revision because l comes after 2 - so a request declaring nonsense read as NEWER than 2026-07-28 and would have been held to its rules. The shape is now checked before the order, in one place (RequestContext.IsRevisionName) that the dispatcher shares. 5 tests.
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
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.
The follow-up I flagged in #126: the same transport section says an unimplemented method MUST be
404and an unsupported protocol version MUST be400, with the JSON-RPC error still in the body.Change
-32601→ 404,-32022/-32020/-32021→ 400, everything else → 200.A real hole found while wiring it
DeclaresRevisionAtLeastcompared ordinally, and"latest"sorts above every dated revision becauselcomes after2. So a request declaring nonsense read as newer than 2026-07-28 and would have been held to its rules — demandingMcp-Methodheaders, gettingresultType, and so on. The shape is now checked before the order, in one place (RequestContext.IsRevisionName) that the dispatcher shares rather than duplicating.The dispatcher was already safe here — it rejects a non-revision before producing any result — so this was latent rather than live, but it was one code path away from mattering.
Verification
RequestContext.