Skip to content

Commit 82ad877

Browse files
HarshMehta112HarshMehta112
committed
fix: clarify tool input validation error message
Signed-off-by: Harsh Mehta <harshmehta010102@gmail.com>
1 parent c49a994 commit 82ad877

8 files changed

Lines changed: 140 additions & 13 deletions

File tree

‎mcp-core/src/main/java/io/modelcontextprotocol/json/schema/JsonSchemaValidator.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,26 @@ public static ValidationResponse asInvalid(String message) {
6464
*/
6565
ValidationResponse validate(Map<String, Object> schema, Object structuredContent);
6666

67+
/**
68+
* Validates the structured content against the provided JSON schema, describing the
69+
* validated data in any resulting error message.
70+
* <p>
71+
* Use this overload when the caller knows which data is being validated (for example
72+
* tool input arguments versus tool output) so that error messages are not misleading
73+
* to consumers or LLMs.
74+
* @param schema The JSON schema to validate against.
75+
* @param structuredContent The structured content to validate.
76+
* @param dataDescription A short, human-readable description of the data being
77+
* validated and the schema it is checked against, for example
78+
* {@code "input arguments do not match tool inputSchema"}. Included verbatim in
79+
* validation error messages.
80+
* @return A ValidationResponse indicating whether the validation was successful or
81+
* not.
82+
*/
83+
default ValidationResponse validate(Map<String, Object> schema, Object structuredContent, String dataDescription) {
84+
return validate(schema, structuredContent);
85+
}
86+
6787
/**
6888
* Validates that the given schema document itself conforms to JSON Schema 2020-12
6989
* (SEP-1613). Schemas that declare an explicit non-2020-12 {@code $schema} dialect

‎mcp-core/src/main/java/io/modelcontextprotocol/util/ToolInputValidator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public static CallToolResult validate(McpSchema.Tool tool, Map<String, Object> a
4040
return null;
4141
}
4242
Map<String, Object> args = arguments != null ? arguments : Map.of();
43-
var validation = validator.validate(tool.inputSchema(), args);
43+
var validation = validator.validate(tool.inputSchema(), args, "input arguments do not match tool inputSchema");
4444
if (!validation.valid()) {
4545
logger.warn("Tool '{}' input validation failed: {}", tool.name(), validation.errorMessage());
4646
return CallToolResult.builder()

‎mcp-core/src/test/java/io/modelcontextprotocol/util/ToolInputValidatorTests.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.mockito.ArgumentMatchers.any;
19+
import static org.mockito.ArgumentMatchers.eq;
1920
import static org.mockito.Mockito.*;
2021

2122
/**
@@ -39,17 +40,17 @@ void validate_whenDisabled_returnsNull() {
3940
CallToolResult result = ToolInputValidator.validate(toolWithSchema, Map.of("name", "test"), false, validator);
4041

4142
assertThat(result).isNull();
42-
verify(validator, never()).validate(any(), any());
43+
verify(validator, never()).validate(any(), any(), any());
4344
}
4445

4546
@Test
4647
void validate_whenNoSchema_returnsNull() {
47-
when(validator.validate(any(), any())).thenReturn(ValidationResponse.asValid(null));
48+
when(validator.validate(any(), any(), any())).thenReturn(ValidationResponse.asValid(null));
4849

4950
CallToolResult result = ToolInputValidator.validate(toolWithoutSchema, Map.of("name", "test"), true, validator);
5051

5152
assertThat(result).isNull();
52-
verify(validator).validate(any(), any());
53+
verify(validator).validate(any(), any(), any());
5354
}
5455

5556
@Test
@@ -61,7 +62,7 @@ void validate_whenNoValidator_returnsNull() {
6162

6263
@Test
6364
void validate_withValidInput_returnsNull() {
64-
when(validator.validate(any(), any())).thenReturn(ValidationResponse.asValid(null));
65+
when(validator.validate(any(), any(), any())).thenReturn(ValidationResponse.asValid(null));
6566

6667
CallToolResult result = ToolInputValidator.validate(toolWithSchema, Map.of("name", "test"), true, validator);
6768

@@ -70,24 +71,36 @@ void validate_withValidInput_returnsNull() {
7071

7172
@Test
7273
void validate_withInvalidInput_returnsErrorResult() {
73-
when(validator.validate(any(), any())).thenReturn(ValidationResponse.asInvalid("missing required: 'name'"));
74+
when(validator.validate(any(), any(), any()))
75+
.thenReturn(ValidationResponse.asInvalid("missing required: 'name'"));
7476

7577
CallToolResult result = ToolInputValidator.validate(toolWithSchema, Map.of(), true, validator);
7678

7779
assertThat(result).isNotNull();
7880
assertThat(result.isError()).isTrue();
7981
assertThat(((TextContent) result.content().get(0)).text()).contains("missing required: 'name'");
80-
verify(validator).validate(any(), any());
82+
verify(validator).validate(any(), any(), any());
83+
}
84+
85+
@Test
86+
void validate_passesInputDataDescriptionToValidator() {
87+
when(validator.validate(any(), any(), any())).thenReturn(ValidationResponse.asValid(null));
88+
89+
ToolInputValidator.validate(toolWithSchema, Map.of("name", "test"), true, validator);
90+
91+
// The data description must reference input/inputSchema so error messages are not
92+
// misleading to consumers/LLMs.
93+
verify(validator).validate(any(), any(), eq("input arguments do not match tool inputSchema"));
8194
}
8295

8396
@Test
8497
void validate_withNullArguments_usesEmptyMap() {
85-
when(validator.validate(any(), any())).thenReturn(ValidationResponse.asValid(null));
98+
when(validator.validate(any(), any(), any())).thenReturn(ValidationResponse.asValid(null));
8699

87100
CallToolResult result = ToolInputValidator.validate(toolWithSchema, null, true, validator);
88101

89102
assertThat(result).isNull();
90-
verify(validator).validate(any(), any());
103+
verify(validator).validate(any(), any(), any());
91104
}
92105

93106
}

‎mcp-json-jackson2/src/main/java/io/modelcontextprotocol/json/schema/jackson2/DefaultJsonSchemaValidator.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ public class DefaultJsonSchemaValidator implements JsonSchemaValidator {
3434

3535
private static final Logger logger = LoggerFactory.getLogger(DefaultJsonSchemaValidator.class);
3636

37+
/**
38+
* Default description used by the two-argument {@code validate} overload, which is
39+
* primarily used for tool output validation.
40+
*/
41+
private static final String DEFAULT_DATA_DESCRIPTION = "structuredContent does not match tool outputSchema";
42+
3743
private final ObjectMapper objectMapper;
3844

3945
private final SchemaRegistry schemaFactory;
@@ -57,6 +63,11 @@ public DefaultJsonSchemaValidator(ObjectMapper objectMapper) {
5763

5864
@Override
5965
public ValidationResponse validate(Map<String, Object> schema, Object structuredContent) {
66+
return validate(schema, structuredContent, DEFAULT_DATA_DESCRIPTION);
67+
}
68+
69+
@Override
70+
public ValidationResponse validate(Map<String, Object> schema, Object structuredContent, String dataDescription) {
6071

6172
if (schema == null) {
6273
throw new IllegalArgumentException("Schema must not be null");
@@ -76,8 +87,7 @@ public ValidationResponse validate(Map<String, Object> schema, Object structured
7687
// Check if validation passed
7788
if (!validationResult.isEmpty()) {
7889
return ValidationResponse
79-
.asInvalid("Validation failed: structuredContent does not match tool outputSchema. "
80-
+ "Validation errors: " + validationResult);
90+
.asInvalid("Validation failed: " + dataDescription + ". Validation errors: " + validationResult);
8191
}
8292

8393
return ValidationResponse.asValid(jsonStructuredOutput.toString());

‎mcp-json-jackson2/src/test/java/io/modelcontextprotocol/json/jackson2/DefaultJsonSchemaValidatorTests.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,40 @@ void testValidateWithInvalidTypeSchema() {
311311
assertTrue(response.errorMessage().contains("structuredContent does not match tool outputSchema"));
312312
}
313313

314+
@Test
315+
void testValidateWithCustomDataDescription() {
316+
String schemaJson = """
317+
{
318+
"type": "object",
319+
"properties": {
320+
"name": {"type": "string"},
321+
"age": {"type": "integer"}
322+
},
323+
"required": ["name", "age"]
324+
}
325+
""";
326+
327+
String contentJson = """
328+
{
329+
"name": "John Doe"
330+
}
331+
""";
332+
333+
Map<String, Object> schema = toMap(schemaJson);
334+
Map<String, Object> structuredContent = toMap(contentJson);
335+
336+
ValidationResponse response = validator.validate(schema, structuredContent,
337+
"input arguments do not match tool inputSchema");
338+
339+
assertFalse(response.valid());
340+
assertNotNull(response.errorMessage());
341+
assertTrue(response.errorMessage().contains("Validation failed"));
342+
assertTrue(response.errorMessage().contains("input arguments do not match tool inputSchema"));
343+
assertFalse(response.errorMessage().contains("outputSchema"));
344+
assertFalse(response.errorMessage().contains("structuredContent"));
345+
assertTrue(response.errorMessage().contains("age"));
346+
}
347+
314348
@Test
315349
void testValidateWithMissingRequiredField() {
316350
String schemaJson = """

‎mcp-json-jackson3/src/main/java/io/modelcontextprotocol/json/schema/jackson3/DefaultJsonSchemaValidator.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ public class DefaultJsonSchemaValidator implements JsonSchemaValidator {
3333

3434
private static final Logger logger = LoggerFactory.getLogger(DefaultJsonSchemaValidator.class);
3535

36+
/**
37+
* Default description used by the two-argument {@code validate} overload, which is
38+
* primarily used for tool output validation.
39+
*/
40+
private static final String DEFAULT_DATA_DESCRIPTION = "structuredContent does not match tool outputSchema";
41+
3642
private final JsonMapper jsonMapper;
3743

3844
private final SchemaRegistry schemaFactory;
@@ -56,6 +62,11 @@ public DefaultJsonSchemaValidator(JsonMapper jsonMapper) {
5662

5763
@Override
5864
public ValidationResponse validate(Map<String, Object> schema, Object structuredContent) {
65+
return validate(schema, structuredContent, DEFAULT_DATA_DESCRIPTION);
66+
}
67+
68+
@Override
69+
public ValidationResponse validate(Map<String, Object> schema, Object structuredContent, String dataDescription) {
5970

6071
if (schema == null) {
6172
throw new IllegalArgumentException("Schema must not be null");
@@ -75,8 +86,7 @@ public ValidationResponse validate(Map<String, Object> schema, Object structured
7586
// Check if validation passed
7687
if (!validationResult.isEmpty()) {
7788
return ValidationResponse
78-
.asInvalid("Validation failed: structuredContent does not match tool outputSchema. "
79-
+ "Validation errors: " + validationResult);
89+
.asInvalid("Validation failed: " + dataDescription + ". Validation errors: " + validationResult);
8090
}
8191

8292
return ValidationResponse.asValid(jsonStructuredOutput.toString());

‎mcp-json-jackson3/src/test/java/io/modelcontextprotocol/json/DefaultJsonSchemaValidatorTests.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,40 @@ void testValidateWithInvalidTypeSchema() {
311311
assertTrue(response.errorMessage().contains("structuredContent does not match tool outputSchema"));
312312
}
313313

314+
@Test
315+
void testValidateWithCustomDataDescription() {
316+
String schemaJson = """
317+
{
318+
"type": "object",
319+
"properties": {
320+
"name": {"type": "string"},
321+
"age": {"type": "integer"}
322+
},
323+
"required": ["name", "age"]
324+
}
325+
""";
326+
327+
String contentJson = """
328+
{
329+
"name": "John Doe"
330+
}
331+
""";
332+
333+
Map<String, Object> schema = toMap(schemaJson);
334+
Map<String, Object> structuredContent = toMap(contentJson);
335+
336+
ValidationResponse response = validator.validate(schema, structuredContent,
337+
"input arguments do not match tool inputSchema");
338+
339+
assertFalse(response.valid());
340+
assertNotNull(response.errorMessage());
341+
assertTrue(response.errorMessage().contains("Validation failed"));
342+
assertTrue(response.errorMessage().contains("input arguments do not match tool inputSchema"));
343+
assertFalse(response.errorMessage().contains("outputSchema"));
344+
assertFalse(response.errorMessage().contains("structuredContent"));
345+
assertTrue(response.errorMessage().contains("age"));
346+
}
347+
314348
@Test
315349
void testValidateWithMissingRequiredField() {
316350
String schemaJson = """

‎mcp-test/src/test/java/io/modelcontextprotocol/server/ToolInputValidationIntegrationTests.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,12 @@ void invalidInput_withDefaultValidation_shouldReturnToolError(String serverType,
183183
assertThat(result.isError()).isTrue();
184184
String errorMessage = ((TextContent) result.content().get(0)).text();
185185
assertThat(errorMessage).containsIgnoringCase(expectedErrorSubstring);
186+
// The message must make clear the failure refers to tool input, not output,
187+
// otherwise it is misleading to consumers/LLMs.
188+
assertThat(errorMessage).contains("Validation failed")
189+
.contains("input arguments do not match tool inputSchema")
190+
.doesNotContain("outputSchema")
191+
.doesNotContain("structuredContent");
186192
}
187193
finally {
188194
closeServer(server, serverType);

0 commit comments

Comments
 (0)