[karthiksenv]

A server calling listRoots against an uninitialized client or one without the roots capability currently sends an unsupported roots/list request. Add the same fail-fast checks used by sampling and elicitation, returning an IllegalStateException before contacting the session.

Fixes #1067.

Both listRoots overloads are covered by regression tests that assert no session interaction before or after subscription. Roots support with listChanged=false remains valid. The shared client/server integration test now asserts the local exception and a successful tool response instead of allowing an unasserted call to pass.

Validation on Windows with Temurin 17.0.19 and embedded Tomcat 11.0.2:

• Before the production fix: 41 exchange tests ran; the four new rejection cases failed because sendRequest was called.
• After the fix: mvnw.cmd -pl mcp-core -am clean test passed all 457 core tests.
• Streamable HTTP and SSE testRootsWithoutCapability passed with both Jackson 3 and Jackson 2 (two tests per backend).
• Formatting validation and git diff --check passed.
• Full mvnw.cmd verify: core and both Jackson modules passed; mcp-test ran 822 tests with 0 assertion failures and 12 errors caused by unavailable Docker / dependent class initialization. Conformance modules were skipped after that failure. This is not a fully green local CI run.
• A fork CI dispatch was attempted but GitHub returned workflow ci.yml not found on the default branch; the Ubuntu/Docker CI environment remains unverified.

This is an AI-assisted contribution requiring human review. The repository's AI submission threshold is not met; the exact required disclosure is included in disclosure.txt.

[soyeladice-svg]

AI-assisted review: the new integration assertion appears to check the wrong failure mode. listRoots() returns a Mono<ListRootsResult>, and the new guards return Mono.error(...); they do not throw synchronously when exchange.listRoots() is called. assertThatThrownBy(exchange::listRoots) only invokes the method reference and receives the Mono, so it should not observe the IllegalStateException unless the publisher is subscribed/blocked. Could this assertion consume the publisher (for example assertThatThrownBy(() -> exchange.listRoots().block())...) or use StepVerifier as the unit tests do? Otherwise the integration test does not actually pin the new fail-fast behavior.

[karthiksenv]

@soyeladice-svg Thanks for checking this. That concern would apply to the async exchange, but this test uses SyncToolSpecification. Its handler receives McpSyncServerExchange, whose listRoots() already calls .block(). So assertThatThrownBy(exchange::listRoots) subscribes through that wrapper and observes the IllegalStateException.

I reran the 41 async exchange tests and both HTTP/SSE testRootsWithoutCapability tests. All 43 passed with no failures or errors. The async unit tests use StepVerifier and also check that the session is never contacted. I’ve kept the integration assertion as it is because the synchronous wrapper already consumes the publisher.

[soyeladice-svg]

You're right — I traced the handler type again and missed that this integration path receives McpSyncServerExchange, whose listRoots() blocks the underlying Mono. In that context assertThatThrownBy(exchange::listRoots) does exercise the intended failure, while the async unit coverage correctly uses StepVerifier. Thanks for the precise correction and the 43-test rerun; my concern is resolved.

[karthiksenv]

@soyeladice-svg Thanks for taking another look and confirming that your concern is resolved. If you're happy with the rest of the changes, could you please approve the PR? I'd appreciate your review so we can move it toward getting merged.