McpAsyncServerExchange#createMessage and #createElicitation both fail fast
with an IllegalStateException when the client hasn't declared the relevant
capability, avoiding an unnecessary round trip to the client.
#listRoots(String) didn't follow the same pattern: it would send a
roots/list request to the client even when the client never advertised
roots support, only to fail later (or behave unexpectedly) depending on the
client's own handling of an unsupported method.
This PR aligns #listRoots(String) with the existing fail-fast convention
already used by createMessage/createElicitation:
If clientCapabilities is null (client not yet initialized), fail with
"Client must be initialized. Call the initialize method first!"
If clientCapabilities.roots() is null (client didn't declare roots
support), fail with "Client must be configured with roots capabilities"
Otherwise, proceed with the request as before.
listRoots() (no-arg, paginated variant) and McpSyncServerExchange#listRoots
both delegate to listRoots(String), so they're covered automatically.
Testing
Added three unit tests to McpAsyncServerExchangeTests mirroring the existing
capability-check tests for createMessage:
Each verifies the correct IllegalStateException is raised and that
session.sendRequest(...) is never invoked in these cases.
I audited all existing callers of listRoots in the repo (integration tests
in AbstractMcpClientServerIntegrationTests, and the roots-changed handler in
McpAsyncServer) — all already configure/assume roots capability, so this
change shouldn't affect existing behavior anywhere else in the codebase.
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.
Reject listRoots when client lacks roots capability, without sending a request
Fixes #1067
What changed
McpAsyncServerExchange#createMessage and #createElicitation both fail fast
with an IllegalStateException when the client hasn't declared the relevant
capability, avoiding an unnecessary round trip to the client.
#listRoots(String) didn't follow the same pattern: it would send a
roots/list request to the client even when the client never advertised
roots support, only to fail later (or behave unexpectedly) depending on the
client's own handling of an unsupported method.
This PR aligns #listRoots(String) with the existing fail-fast convention
already used by createMessage/createElicitation:
"Client must be initialized. Call the initialize method first!"
support), fail with "Client must be configured with roots capabilities"
listRoots() (no-arg, paginated variant) and McpSyncServerExchange#listRoots
both delegate to listRoots(String), so they're covered automatically.
Testing
Added three unit tests to McpAsyncServerExchangeTests mirroring the existing
capability-check tests for createMessage:
Each verifies the correct IllegalStateException is raised and that
session.sendRequest(...) is never invoked in these cases.
I audited all existing callers of listRoots in the repo (integration tests
in AbstractMcpClientServerIntegrationTests, and the roots-changed handler in
McpAsyncServer) — all already configure/assume roots capability, so this
change shouldn't affect existing behavior anywhere else in the codebase.