Skip to content

feat: Add client validation for structuredContent #302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion mcp/src/main/java/io/modelcontextprotocol/client/McpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
import java.util.function.Consumer;
import java.util.function.Function;

import com.fasterxml.jackson.databind.ObjectMapper;

import io.modelcontextprotocol.server.McpServer.AsyncSpecification;
import io.modelcontextprotocol.spec.DefaultJsonSchemaValidator;
import io.modelcontextprotocol.spec.JsonSchemaValidator;
import io.modelcontextprotocol.spec.McpClientTransport;
import io.modelcontextprotocol.spec.McpSchema;
import io.modelcontextprotocol.spec.McpTransport;
Expand Down Expand Up @@ -97,6 +102,7 @@
*
* @author Christian Tzolov
* @author Dariusz Jędrzejczyk
* @author Anurag Pant
* @see McpAsyncClient
* @see McpSyncClient
* @see McpTransport
Expand Down Expand Up @@ -183,6 +189,8 @@ class SyncSpec {

private Function<ElicitRequest, ElicitResult> elicitationHandler;

private JsonSchemaValidator jsonSchemaValidator;

private SyncSpec(McpClientTransport transport) {
Assert.notNull(transport, "Transport must not be null");
this.transport = transport;
Expand Down Expand Up @@ -409,12 +417,27 @@ public SyncSpec progressConsumers(List<Consumer<McpSchema.ProgressNotification>>
return this;
}

/**
* Sets the JSON schema validator to use for validating tool responses against
* output schemas.
* @param jsonSchemaValidator The validator to use. Must not be null.
* @return This builder instance for method chaining
* @throws IllegalArgumentException if jsonSchemaValidator is null
*/
public SyncSpec jsonSchemaValidator(JsonSchemaValidator jsonSchemaValidator) {
Assert.notNull(jsonSchemaValidator, "JsonSchemaValidator must not be null");
this.jsonSchemaValidator = jsonSchemaValidator;
return this;
}

/**
* Create an instance of {@link McpSyncClient} with the provided configurations or
* sensible defaults.
* @return a new instance of {@link McpSyncClient}.
*/
public McpSyncClient build() {
var jsonSchemaValidator = this.jsonSchemaValidator != null ? this.jsonSchemaValidator
: new DefaultJsonSchemaValidator();
McpClientFeatures.Sync syncFeatures = new McpClientFeatures.Sync(this.clientInfo, this.capabilities,
this.roots, this.toolsChangeConsumers, this.resourcesChangeConsumers, this.resourcesUpdateConsumers,
this.promptsChangeConsumers, this.loggingConsumers, this.progressConsumers, this.samplingHandler,
Expand All @@ -423,7 +446,8 @@ public McpSyncClient build() {
McpClientFeatures.Async asyncFeatures = McpClientFeatures.Async.fromSync(syncFeatures);

return new McpSyncClient(
new McpAsyncClient(transport, this.requestTimeout, this.initializationTimeout, asyncFeatures));
new McpAsyncClient(transport, this.requestTimeout, this.initializationTimeout, asyncFeatures),
jsonSchemaValidator);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
package io.modelcontextprotocol.client;

import java.time.Duration;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.modelcontextprotocol.spec.JsonSchemaValidator;
import io.modelcontextprotocol.spec.McpError;
import io.modelcontextprotocol.spec.McpSchema;
import io.modelcontextprotocol.spec.McpSchema.ClientCapabilities;
import io.modelcontextprotocol.spec.McpSchema.GetPromptRequest;
Expand Down Expand Up @@ -48,6 +53,7 @@
* @author Dariusz Jędrzejczyk
* @author Christian Tzolov
* @author Jihoon Kim
* @author Anurag Pant
* @see McpClient
* @see McpAsyncClient
* @see McpSchema
Expand All @@ -63,14 +69,23 @@ public class McpSyncClient implements AutoCloseable {

private final McpAsyncClient delegate;

private final JsonSchemaValidator jsonSchemaValidator;

/**
* Cached tool output schemas.
*/
private final ConcurrentHashMap<String, Optional<Map<String, Object>>> toolsOutputSchemaCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you wind up looking at cache abstractions like we had discussed offline a while back? Not sure if that fell through or if we just don't want to do that anymore.

I just took a look at Guava's cache implementation and they actually recommend using Caffeine now.


Separate comment: we should have a comment explaining the Optional if we're keeping that. It's not obvious why you would want an Optional value without reading the full implementation.

Copy link
Contributor Author

@pantanurag555 pantanurag555 Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't use any cache abstractions to keep the review simple for now and avoid additional dependencies. The cache implementation as part of JsonValidator was also a similar ConcurrentHashMap so wanted to stay close to that at least for the first review of the client side validation. Additionally, python SDK implementation also used a dict initially for the cache: modelcontextprotocol/python-sdk#685

I will make a note to add additional comments about Optional to better explain it in the context of the code.


/**
* Create a new McpSyncClient with the given delegate.
* @param delegate the asynchronous kernel on top of which this synchronous client
* provides a blocking API.
*/
McpSyncClient(McpAsyncClient delegate) {
McpSyncClient(McpAsyncClient delegate, JsonSchemaValidator jsonSchemaValidator) {
Assert.notNull(delegate, "The delegate can not be null");
this.delegate = delegate;
this.jsonSchemaValidator = jsonSchemaValidator;
this.toolsOutputSchemaCache = new ConcurrentHashMap<>();
}

/**
Expand Down Expand Up @@ -216,7 +231,37 @@ public Object ping() {
* Boolean indicating if the execution failed (true) or succeeded (false/absent)
*/
public McpSchema.CallToolResult callTool(McpSchema.CallToolRequest callToolRequest) {
return this.delegate.callTool(callToolRequest).block();
if (!this.toolsOutputSchemaCache.containsKey(callToolRequest.name())) {
listTools(); // Ensure tools are cached before calling
}

McpSchema.CallToolResult result = this.delegate.callTool(callToolRequest).block();
Optional<Map<String, Object>> optOutputSchema = toolsOutputSchemaCache.get(callToolRequest.name());

if (result != null && result.isError() != null && !result.isError()) {
if (optOutputSchema == null) {
// Should not be triggered but added for completeness
throw new McpError("Tool with name '" + callToolRequest.name() + "' not found");
}
else {
if (optOutputSchema.isPresent()) {
// Validate the tool output against the cached output schema
var validation = this.jsonSchemaValidator.validate(optOutputSchema.get(),
result.structuredContent());
if (!validation.valid()) {
throw new McpError("Tool call result validation failed: " + validation.errorMessage());
}
}
else if (result.structuredContent() != null) {
logger.warn(
"Calling a tool with no outputSchema is not expected to return result with structured content, but got: {}",
result.structuredContent());
}

}
}

return result;
}

/**
Expand All @@ -226,7 +271,14 @@ public McpSchema.CallToolResult callTool(McpSchema.CallToolRequest callToolReque
* pagination if more tools are available
*/
public McpSchema.ListToolsResult listTools() {
return this.delegate.listTools().block();
return this.delegate.listTools().doOnNext(result -> {
if (result.tools() != null) {
// Cache tools output schema
result.tools()
.forEach(tool -> this.toolsOutputSchemaCache.put(tool.name(),
Optional.ofNullable(tool.outputSchema())));
}
}).block();
}

/**
Expand All @@ -237,7 +289,14 @@ public McpSchema.ListToolsResult listTools() {
* pagination if more tools are available
*/
public McpSchema.ListToolsResult listTools(String cursor) {
return this.delegate.listTools(cursor).block();
return this.delegate.listTools(cursor).doOnNext(result -> {
if (result.tools() != null) {
// Cache tools output schema
result.tools()
.forEach(tool -> this.toolsOutputSchemaCache.put(tool.name(),
Optional.ofNullable(tool.outputSchema())));
}
}).block();
}

// --------------------------
Expand Down
Loading