Skip to content

Reject listRoots when client lacks roots capability - #1137

Open
Yerang-RB wants to merge 1 commit into
modelcontextprotocol:mainfrom
Yerang-RB:feature/reject-list-roots-without-capability
Open

Yerang-RB wants to merge 1 commit into
modelcontextprotocol:mainfrom
Yerang-RB:feature/reject-list-roots-without-capability

Conversation

@Yerang-RB

Copy link
Copy Markdown

Closes #1067.

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.

This adds the same two guards to listRoots(String cursor). The no-arg listRoots() delegates to that overload, so a single check covers both entry points.

Motivation and Context

A server calling listRoots() against a client that never declared roots had 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 initialized
  • testListRootsWithoutRootsCapabilities — client initialized without roots

Both call the no-arg listRoots(), so the delegation path is covered too, and both assert verify(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 an IllegalStateException. That matches what createMessage and createElicitation already do for sampling and elicitation.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The issue suggested only the roots() == null guard. I added the null-capabilities guard alongside it so the method matches the shape of createMessage and createElicitation — 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 listRoots should call out the new precondition.

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.
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.

Reject listRoots if not supported by client, without sending any request

1 participant