feat(tools): check unknown-tool protocol errors and id correlation - #508
Open
astrogilda wants to merge 1 commit into
Open
astrogilda wants to merge 1 commit into
astrogilda wants to merge 1 commit into
Conversation
…lation Add the tools-call-protocol-error server scenario. It calls a tool the server does not have, once with a numeric and once with a string request id, and then tools/list with a string id, and checks eight things on the raw frames: the unknown tool comes back as a JSON-RPC error response and not as a CallToolResult with isError, the error code is the -32602 the specification example uses (WARNING), error.code is an integer, error.message is a string, an error response carries no result member, and both error and result responses echo the request id with its type intact. The everything-server pins SDK 1.x, whose McpServer folds its own InvalidParams error for an unknown tool into an isError result. Guard the tools/call handler so the protocol error reaches the wire, the way the 2.x SDK already does. A negative fixture answers unknown tools with isError and stringifies ids; its vitest case asserts the FAILURE, the WARNING, and the not-testable reporting of the dependent checks.
Author
|
Hey @pcarleton, the two workflow runs on this pull request are waiting for approval. I ran the test job against the merge commit on Node 24, and check, build and the test suite pass, including the new tools-call-protocol-error test. The pkg.pr.new job passes up to its publish step, which I left to the real run. Would you approve the runs when you have a moment? |
This branch has not been deployed
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.
tools-call-protocol-errorcalls a tool the server does not have and reads the raw response frame. It catches an unknown tool answered with aCallToolResultcarryingisError: trueand no JSON-RPC error response, and a request id echoed with the wrong type.The Tools page lists unknown tools under protocol errors, and the base protocol page makes the id echo a MUST for error and result responses.
603217006affe5c0The
ToolErrorraised attool_manager.py:85is folded into anisErrorresult atserver.py:437, which typescript-sdk changed in 2.0.0-alpha.1 (#1389), and the five dependent checks report "Not testable" per #248.The vendored everything-server pins SDK 1.29, which folds the same way, so this change guards its
tools/callhandler and throws theMcpErrorfirst.npm testpasses 624/624. The negative fixturetools-call-unknown-tool-as-result.tscarries a vitest case for the FAILURE.The probes are the wire half of two attack cases in agent-evidence-vectors, whose corpus runs from one command (edited 2026-09-25 to pin 0.13.0):
{"jsonrpc": "2.0", "id": 4242, "result": {"content": [{"type": "text", "text": "Unknown tool: conformance_tool_that_does_not_exist"}], "isError": true}}