Conversation
McpAsyncServerExchange#createMessage and #createElicitation both fail fast when the client is not initialized or has not declared the matching capability. listRoots sent the request regardless. Add the same two guards to listRoots(String cursor). The no-arg listRoots() delegates to it, so both overloads are covered by the single check.
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.
Closes #1067.
McpAsyncServerExchange#createMessageand#createElicitationboth fail fast when the client is not initialized or has not declared the matching capability.listRootssent the request regardless.This adds the same two guards to
listRoots(String cursor). The no-arglistRoots()delegates to that overload, so a single check covers both entry points.Motivation and Context
A server calling
listRoots()against a client that never declaredrootshad no way to tell that up front: the request went out and failed somewhere downstream, or hung, depending on how the client handled an unsupported method. The two neighbouring methods on the same class already guard against exactly this, so the inconsistency was the bug.How Has This Been Tested?
Unit tests only — not exercised in a real application.
Two tests added to
McpAsyncServerExchangeTests, mirroring the existing elicitation guard tests:testListRootsWithNullCapabilities— client not initializedtestListRootsWithoutRootsCapabilities— client initialized withoutrootsBoth call the no-arg
listRoots(), so the delegation path is covered too, and both assertverify(mockSession, never()).sendRequest(eq(METHOD_ROOTS_LIST), ...)— the point of the issue is that no request is sent at all, not merely that an error surfaces.Verified locally with
./mvnw -pl mcp-core -am test: 420 tests, green. Reverting only the main-code change makes exactly the two new tests fail, and nothing else.Breaking Changes
Behaviour changes, though in the direction the class already established. A server that previously called
listRoots()against a client which supports roots but does not declare the capability would have gotten a response; it now gets anIllegalStateException. That matches whatcreateMessageandcreateElicitationalready do for sampling and elicitation.Types of changes
Checklist
Additional context
The issue suggested only the
roots() == nullguard. I added the null-capabilities guard alongside it so the method matches the shape ofcreateMessageandcreateElicitation— happy to drop it if you would rather keep the change minimal.No documentation change: the guard makes an undocumented failure mode explicit rather than altering the documented contract. Say the word if the Javadoc on
listRootsshould call out the new precondition.