From f0b67a3fcd9d05d4b8730b76a35a173cc245a255 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 11 Jun 2026 19:08:58 -0700 Subject: [PATCH] fix: MCP server issues in testing --- .../api/model/command/CommandResult.java | 30 +++- .../api/v1/mcp/CollectionCommandTools.java | 89 +++++++----- .../api/v1/mcp/McpClauseSchemaCustomizer.java | 132 ++++++++++++++++++ .../sgv2/jsonapi/api/v1/mcp/McpResource.java | 112 ++++++++++----- ...lectionCommandToolsMcpIntegrationTest.java | 83 +++++++++++ ...GeneralCommandToolsMcpIntegrationTest.java | 18 +++ ...eyspaceCommandToolsMcpIntegrationTest.java | 17 +++ .../v1/mcp/McpClauseSchemaCustomizerTest.java | 118 ++++++++++++++++ .../api/v1/mcp/McpIntegrationTestBase.java | 118 +++++++++++++--- .../CommandResultToToolResponseTest.java | 68 ++++++--- 10 files changed, 677 insertions(+), 108 deletions(-) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizer.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizerTest.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java index de4ef0665b..1598767318 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java @@ -1,7 +1,10 @@ package io.stargate.sgv2.jsonapi.api.model.command; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import io.quarkiverse.mcp.server.MetaKey; +import io.quarkiverse.mcp.server.TextContent; import io.quarkiverse.mcp.server.ToolResponse; import io.stargate.sgv2.jsonapi.api.model.command.tracing.RequestTracing; import jakarta.ws.rs.core.Response; @@ -104,15 +107,21 @@ public RestResponse toRestResponse() { *

Mapping rules: * *

* + * @param objectMapper The mapper used to serialize this command result envelope into the text + * content part. * @return A new {@link ToolResponse} representing this command result. */ - public ToolResponse toToolResponse() { + public ToolResponse toToolResponse(ObjectMapper objectMapper) { boolean hasErrors = errors != null && !errors.isEmpty(); @@ -120,10 +129,27 @@ public ToolResponse toToolResponse() { Map meta = (status != null && !status.isEmpty()) ? Map.of(MetaKey.of("status"), status) : Map.of(); + String envelopeJson; + try { + envelopeJson = objectMapper.writeValueAsString(this); + } catch (JsonProcessingException e) { + // Fallback must still be a parseable JSON envelope: content is the primary path MCP + // clients read. ObjectNode handles escaping of the exception message. + var fallback = objectMapper.createObjectNode(); + fallback + .putArray("errors") + .addObject() + .put("message", "Failed to serialize command result to JSON: " + e.getMessage()); + envelopeJson = fallback.toString(); + } + // Map "errors" or "data" to structuredContent // Also, structuredContent is expected to be a Record (a plain JSON object {}) return new ToolResponse( - hasErrors, List.of(), hasErrors ? Map.of("errors", errors) : data, meta); + hasErrors, + List.of(new TextContent(envelopeJson)), + hasErrors ? Map.of("errors", errors) : data, + meta); } /** diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java index 529fc910ec..a177c952b3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandTools.java @@ -25,6 +25,35 @@ */ public class CollectionCommandTools { + /** + * Tool argument descriptions for the clause arguments shared by many commands. These describe the + * plain user-facing Data API JSON shape: the MCP input schema advertises these clause types as + * plain objects (see {@link McpClauseSchemaCustomizer}), so the descriptions carry the syntax + * guidance the client (LLM) needs. + */ + private static final String FILTER_ARG_DESCRIPTION = + "Filter as a plain Data API filter object, e.g. {\"age\": {\"$gt\": 40}}: maps document" + + " paths to exact values or operator expressions ($eq, $ne, $gt, $gte, $lt, $lte, $in," + + " $nin, $exists, $and, $or, $not)."; + + private static final String SORT_ARG_DESCRIPTION = + "Sort as a plain Data API sort object, e.g. {\"age\": -1, \"name\": 1}: maps document paths" + + " to 1 (ascending) or -1 (descending); use {\"$vector\": [...]} or {\"$vectorize\":" + + " \"text\"} for vector search."; + + private static final String UPDATE_ARG_DESCRIPTION = + "Update as a plain Data API update object keyed by update operators, e.g. {\"$set\":" + + " {\"location\": \"New York\"}, \"$unset\": {\"old_field\": 1}}."; + + private static final String PROJECTION_ARG_DESCRIPTION = + "Projection as a plain object selecting the fields to include or exclude, e.g. {\"name\": 1," + + " \"_id\": 0}."; + + private static final String HYBRID_SORT_ARG_DESCRIPTION = + "Hybrid sort as a plain object, e.g. {\"$hybrid\": \"query text\"} to use the same query for" + + " vector and lexical search, or {\"$hybrid\": {\"$vectorize\": \"...\", \"$lexical\":" + + " \"...\"}}."; + @Inject McpResource mcpResource; @Tool(description = "Command that alters the column definition in a table.") @@ -41,7 +70,7 @@ public Uni alterTable( public Uni countDocuments( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter) { + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter) { var command = new CountDocumentsCommand(filter); return mcpResource.processCollectionCommand(keyspace, collection, command); @@ -118,10 +147,7 @@ public Uni createVectorIndex( public Uni deleteMany( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg( - description = "Filter clause based on which documents are identified", - required = false) - FilterDefinition filter) { + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter) { var command = new DeleteManyCommand(filter); return mcpResource.processCollectionCommand(keyspace, collection, command); @@ -131,9 +157,8 @@ public Uni deleteMany( public Uni deleteOne( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "Filter clause based on which documents are identified") - FilterDefinition filter, - @ToolArg(description = "Sort clause", required = false) SortDefinition sort) { + @ToolArg(description = FILTER_ARG_DESCRIPTION) FilterDefinition filter, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort) { var command = new DeleteOneCommand(filter, sort); return mcpResource.processCollectionCommand(keyspace, collection, command); @@ -154,9 +179,9 @@ public Uni estimatedDocumentCount( public Uni findAndRerank( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "projection", required = false) JsonNode projection, - @ToolArg(description = "sort", required = false) FindAndRerankSort sort, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection, + @ToolArg(description = HYBRID_SORT_ARG_DESCRIPTION, required = false) FindAndRerankSort sort, @ToolArg(description = "options", required = false) FindAndRerankCommand.Options options) { var command = new FindAndRerankCommand(filter, projection, sort, options); @@ -167,9 +192,9 @@ public Uni findAndRerank( public Uni find( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "projection", required = false) JsonNode projection, - @ToolArg(description = "sort", required = false) SortDefinition sort, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, @ToolArg(description = "options", required = false) FindCommand.Options options) { var command = new FindCommand(filter, projection, sort, options); @@ -182,9 +207,9 @@ public Uni find( public Uni findOneAndDelete( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "sort", required = false) SortDefinition sort, - @ToolArg(description = "projection", required = false) JsonNode projection) { + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection) { var command = new FindOneAndDeleteCommand(filter, sort, projection); return mcpResource.processCollectionCommand(keyspace, collection, command); @@ -196,9 +221,9 @@ public Uni findOneAndDelete( public Uni findOneAndReplace( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "sort", required = false) SortDefinition sort, - @ToolArg(description = "projection", required = false) JsonNode projection, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection, @ToolArg(description = "replacement") ObjectNode replacement, @ToolArg(description = "options", required = false) FindOneAndReplaceCommand.Options options) { @@ -213,10 +238,10 @@ public Uni findOneAndReplace( public Uni findOneAndUpdate( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "projection", required = false) JsonNode projection, - @ToolArg(description = "sort", required = false) SortDefinition sort, - @ToolArg(description = "update") UpdateClause update, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, + @ToolArg(description = UPDATE_ARG_DESCRIPTION) UpdateClause update, @ToolArg(description = "options", required = false) FindOneAndUpdateCommand.Options options) { var command = new FindOneAndUpdateCommand(filter, projection, sort, update, options); @@ -227,9 +252,9 @@ public Uni findOneAndUpdate( public Uni findOne( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "projection", required = false) JsonNode projection, - @ToolArg(description = "sort", required = false) SortDefinition sort, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = PROJECTION_ARG_DESCRIPTION, required = false) JsonNode projection, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, @ToolArg(description = "options", required = false) FindOneCommand.Options options) { var command = new FindOneCommand(filter, projection, sort, options); @@ -275,8 +300,8 @@ public Uni insertOne( public Uni updateMany( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "update") UpdateClause update, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = UPDATE_ARG_DESCRIPTION) UpdateClause update, @ToolArg(description = "options", required = false) UpdateManyCommand.Options options) { var command = new UpdateManyCommand(filter, update, options); @@ -289,9 +314,9 @@ public Uni updateMany( public Uni updateOne( @ToolArg(description = "Name of the keyspace") String keyspace, @ToolArg(description = "Name of the collection/table") String collection, - @ToolArg(description = "filter", required = false) FilterDefinition filter, - @ToolArg(description = "update") UpdateClause update, - @ToolArg(description = "sort", required = false) SortDefinition sort, + @ToolArg(description = FILTER_ARG_DESCRIPTION, required = false) FilterDefinition filter, + @ToolArg(description = UPDATE_ARG_DESCRIPTION) UpdateClause update, + @ToolArg(description = SORT_ARG_DESCRIPTION, required = false) SortDefinition sort, @ToolArg(description = "options", required = false) UpdateOneCommand.Options options) { var command = new UpdateOneCommand(filter, update, sort, options); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizer.java new file mode 100644 index 0000000000..20de012083 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizer.java @@ -0,0 +1,132 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.github.victools.jsonschema.generator.CustomDefinition; +import com.github.victools.jsonschema.generator.SchemaGeneratorConfigBuilder; +import com.github.victools.jsonschema.generator.SchemaKeyword; +import io.quarkiverse.mcp.server.runtime.SchemaGeneratorConfigCustomizer; +import io.stargate.sgv2.jsonapi.api.model.command.JsonDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateClause; +import jakarta.inject.Singleton; +import java.util.Map; + +/** + * Advertises the user-facing Data API JSON shape for command clause types in MCP tool input + * schemas, instead of the internal Java shape. + * + *

Clause types such as {@link FilterDefinition} are deserialized from the raw user JSON (via + * delegating creators or custom deserializers), so their Java fields ({@code filterClause}, {@code + * json}, {@code updateOperationDefs}, ...) are not the wire format. Without this customizer, the + * victools generator used by Quarkus MCP Server reflects over those fields and advertises a schema + * that the server cannot actually deserialize — schema-compliant MCP clients then send arguments + * like {@code {"filter": {"json": {...}}}} that fail, instead of the plain Data API clause {@code + * {"filter": {"age": {"$gt": 40}}}}. + * + *

For these types this customizer emits a plain {@code {"type": "object"}} schema with a + * description and an example of the accepted Data API clause syntax. + */ +@Singleton +public class McpClauseSchemaCustomizer implements SchemaGeneratorConfigCustomizer { + + /** Description and example of the user-facing Data API JSON shape for a clause type. */ + record PlainObjectSchema(String description, String exampleJson) {} + + /** + * Clause types whose MCP schema must be the plain user-facing Data API JSON object. Examples must + * be valid JSON: they are embedded into the generated schema's {@code examples}. + */ + private static final Map, PlainObjectSchema> PLAIN_OBJECT_SCHEMAS = + Map.of( + FilterDefinition.class, + new PlainObjectSchema( + "Filter as a plain Data API filter object: maps document paths to exact values" + + " or to operator expressions such as $eq, $ne, $gt, $gte, $lt, $lte, $in," + + " $nin, $exists, $and, $or, $not.", + """ + {"name": "Aaron", "country": {"$eq": "NZ"}, "age": {"$gt": 40}} + """), + SortDefinition.class, + new PlainObjectSchema( + "Sort as a plain Data API sort object: maps document paths to 1 (ascending) or" + + " -1 (descending), or uses $vector / $vectorize for vector search.", + """ + {"user.age": -1, "user.name": 1} + """), + UpdateClause.class, + new PlainObjectSchema( + "Update as a plain Data API update object keyed by update operators such as" + + " $set, $unset, $inc, $push, $pull.", + """ + {"$set": {"location": "New York"}, "$unset": {"new_data": 1}} + """), + FindAndRerankSort.class, + new PlainObjectSchema( + "Hybrid sort as a plain object: {\"$hybrid\": \"query text\"} to use the same" + + " query for vector and lexical search, or separate queries via $vectorize /" + + " $lexical / $vector.", + """ + {"$hybrid": {"$vectorize": "vectorize sort query", "$lexical": "lexical sort"}} + """)); + + /** Fallback for {@link JsonDefinition} subtypes without an entry in the map above. */ + private static final PlainObjectSchema GENERIC_PLAIN_OBJECT_SCHEMA = + new PlainObjectSchema( + "A plain JSON object using the same syntax as the equivalent Data API command clause.", + null); + + @Override + public void customize(SchemaGeneratorConfigBuilder builder) { + builder + .forTypesInGeneral() + .withCustomDefinitionProvider( + (javaType, context) -> { + var plainObjectSchema = plainObjectSchemaFor(javaType.getErasedType()); + if (plainObjectSchema == null) { + return null; + } + + ObjectNode schemaNode = context.getGeneratorConfig().createObjectNode(); + schemaNode.put( + context.getKeyword(SchemaKeyword.TAG_TYPE), + context.getKeyword(SchemaKeyword.TAG_TYPE_OBJECT)); + schemaNode.put("description", plainObjectSchema.description()); + JsonNode example = + parseExample( + context.getGeneratorConfig().getObjectMapper(), + plainObjectSchema.exampleJson()); + if (example != null) { + schemaNode.putArray("examples").add(example); + } + return new CustomDefinition( + schemaNode, + CustomDefinition.DefinitionType.INLINE, + CustomDefinition.AttributeInclusion.YES); + }); + } + + private static PlainObjectSchema plainObjectSchemaFor(Class erasedType) { + var plainObjectSchema = PLAIN_OBJECT_SCHEMAS.get(erasedType); + if (plainObjectSchema == null && JsonDefinition.class.isAssignableFrom(erasedType)) { + return GENERIC_PLAIN_OBJECT_SCHEMA; + } + return plainObjectSchema; + } + + private static JsonNode parseExample(ObjectMapper objectMapper, String exampleJson) { + if (exampleJson == null) { + return null; + } + try { + return objectMapper.readTree(exampleJson); + } catch (Exception e) { + // An unparseable example is a bug in PLAIN_OBJECT_SCHEMAS (covered by unit tests); omit + // the example rather than fail tool listing. + return null; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java index 9b14d63d7f..67a21c7385 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java @@ -93,46 +93,62 @@ public McpResource( /** * Build a CommandContext for database-level (general) commands. similar to {@link * io.stargate.sgv2.jsonapi.api.v1.GeneralResource} + * + *

Deferred so synchronous failures (e.g. request context resolution) surface as a failed Uni + * that {@link #processCommand} can recover into an error ToolResponse, instead of escaping to the + * MCP framework as an opaque JSON-RPC internal error. */ public Uni> buildDatabaseContext(GeneralCommand command) { - var requestContext = createRequestContext(); - var dbIdentifier = SchemaObjectIdentifier.forDatabase(requestContext.tenant()); + return Uni.createFrom() + .deferred( + () -> { + var requestContext = createRequestContext(); + var dbIdentifier = SchemaObjectIdentifier.forDatabase(requestContext.tenant()); - return schemaObjectCacheSupplier - .get() - .getDatabase(requestContext, dbIdentifier, requestContext.userAgent()) - .map( - databaseSchemaObject -> - contextBuilderSupplier - .getBuilder(databaseSchemaObject) - .withCommandName(command.getClass().getSimpleName()) - .withRequestContext(requestContext) - .build()); + return schemaObjectCacheSupplier + .get() + .getDatabase(requestContext, dbIdentifier, requestContext.userAgent()) + .map( + databaseSchemaObject -> + contextBuilderSupplier + .getBuilder(databaseSchemaObject) + .withCommandName(command.getClass().getSimpleName()) + .withRequestContext(requestContext) + .build()); + }); } /** * Build a CommandContext for keyspace-level (keyspace) commands. similar to {@link * io.stargate.sgv2.jsonapi.api.v1.KeyspaceResource} + * + *

Deferred so synchronous failures (e.g. invalid keyspace identifiers) surface as a failed Uni + * that {@link #processCommand} can recover into an error ToolResponse, instead of escaping to the + * MCP framework as an opaque JSON-RPC internal error. */ public Uni> buildKeyspaceContext(String keyspace, KeyspaceCommand command) { - var requestContext = createRequestContext(); - var keyspaceIdentifier = - SchemaObjectIdentifier.forKeyspace( - requestContext.tenant(), cqlIdentifierFromUserInput(keyspace)); + return Uni.createFrom() + .deferred( + () -> { + var requestContext = createRequestContext(); + var keyspaceIdentifier = + SchemaObjectIdentifier.forKeyspace( + requestContext.tenant(), cqlIdentifierFromUserInput(keyspace)); - // Force refresh on all keyspace commands because they are all DDL commands - return schemaObjectCacheSupplier - .get() - .getKeyspace(requestContext, keyspaceIdentifier, requestContext.userAgent(), true) - .map( - keyspaceSchemaObject -> - contextBuilderSupplier - .getBuilder(keyspaceSchemaObject) - .withCommandName(command.getClass().getSimpleName()) - .withRequestContext(createRequestContext()) - .build()); + // Force refresh on all keyspace commands because they are all DDL commands + return schemaObjectCacheSupplier + .get() + .getKeyspace(requestContext, keyspaceIdentifier, requestContext.userAgent(), true) + .map( + keyspaceSchemaObject -> + contextBuilderSupplier + .getBuilder(keyspaceSchemaObject) + .withCommandName(command.getClass().getSimpleName()) + .withRequestContext(requestContext) + .build()); + }); } /** @@ -143,6 +159,22 @@ public Uni> buildKeyspaceContext(String keyspace, KeyspaceComm public Uni processCollectionCommand( String keyspace, String collection, CollectionCommand command) { + return Uni.createFrom() + .deferred(() -> processCollectionCommandInternal(keyspace, collection, command)) + .onFailure() + .recoverWithItem(this::errorToolResponse); + } + + /** + * See {@link #processCollectionCommand(String, String, CollectionCommand)}. + * + *

Must only be called via the deferred + recovery wrapper in {@link + * #processCollectionCommand}: this method may throw synchronously (e.g. identifier parsing, + * embedding provider creation) and has no failure recovery of its own. + */ + private Uni processCollectionCommandInternal( + String keyspace, String collection, CollectionCommand command) { + var requestContext = createRequestContext(); var unscopedSchemaIdentifier = @@ -161,11 +193,7 @@ public Uni processCollectionCommand( (schemaObject, throwable) -> { if (throwable != null) { // If schema resolution or authorization fails, return an error ToolResponse - CommandResult errorResult = - CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) - .addThrowable(throwable) - .build(); - return Uni.createFrom().item(errorResult.toToolResponse()); + return Uni.createFrom().item(errorToolResponse(throwable)); } else { VectorColumnDefinition vectorColDef = null; if (schemaObject.type() == SchemaObjectType.COLLECTION) { @@ -234,7 +262,25 @@ public Uni processCommand(Uni> contextUni, Comma } return meteredCommandProcessor.processCommand(context, command); }) - .map(CommandResult::toToolResponse); + .map(commandResult -> commandResult.toToolResponse(objectMapper)) + // Failures not already recovered into an error CommandResult (e.g. context building + // failures) must become error ToolResponses: anything escaping here surfaces to MCP + // clients as an opaque JSON-RPC -32603 "Internal error" that agents cannot act on. + .onFailure() + .recoverWithItem(this::errorToolResponse); + } + + /** + * Convert a {@link Throwable} into an error {@link ToolResponse} (with {@code isError} and the + * error message in the content) so MCP clients receive an actionable tool error instead of an + * opaque JSON-RPC internal error. + */ + private ToolResponse errorToolResponse(Throwable throwable) { + LOGGER.warn("MCP tool call failed outside of command processing", throwable); + return CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(throwable) + .build() + .toToolResponse(objectMapper); } /** diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandToolsMcpIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandToolsMcpIntegrationTest.java index 3d1d0bc9e4..8f2650b7d3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandToolsMcpIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/CollectionCommandToolsMcpIntegrationTest.java @@ -358,4 +358,87 @@ void testDeleteManyToolCall() { "filter", Map.of("active", false)), assertStatusOnlyWithJson(status -> assertEquals(1, status.getInteger("deletedCount")))); } + + /** + * The advertised inputSchema for `find` must describe `filter` as a plain Data API filter object. + * The internal Java shape (`filterClause`, `json`) must not leak: schema-compliant MCP clients + * send exactly what the schema advertises, and the server must accept it. + */ + @Test + @Order(15) + void testAdvertisedFindFilterSchemaIsPlainObject() { + var findTool = findTool(CommandName.Names.FIND); + + JsonObject filterSchema = + findTool.inputSchema().getJsonObject("properties").getJsonObject("filter"); + assertNotNull(filterSchema, "find tool must advertise a filter argument"); + assertEquals("object", filterSchema.getString("type"), "filter must be a plain object"); + + JsonObject filterProperties = filterSchema.getJsonObject("properties"); + if (filterProperties != null) { + assertFalse( + filterProperties.containsKey("filterClause"), + "Internal field filterClause must not leak into the advertised filter schema"); + assertFalse( + filterProperties.containsKey("json"), + "Internal field json must not leak into the advertised filter schema"); + } + } + + /** + * Calls `find` with the example filter taken verbatim from the advertised inputSchema and asserts + * the call succeeds: the advertised schema and the accepted wire format must agree. + */ + @Test + @Order(16) + void testFindWithFilterExampleFromAdvertisedSchema() { + var findTool = findTool(CommandName.Names.FIND); + JsonObject filterSchema = + findTool.inputSchema().getJsonObject("properties").getJsonObject("filter"); + + JsonArray examples = filterSchema.getJsonArray("examples"); + assertNotNull(examples, "filter schema should advertise an example"); + assertFalse(examples.isEmpty(), "filter schema examples should not be empty"); + JsonObject exampleFilter = examples.getJsonObject(0); + + callToolAndAssert( + CommandName.Names.FIND, + Map.of( + "keyspace", + keyspaceName, + "collection", + collectionName, + "filter", + exampleFilter.getMap()), + assertDataOnly( + data -> + assertNotNull( + data.getJsonArray("documents"), + "find with the advertised example filter must succeed"))); + } + + /** + * An invalid filter must surface as a proper MCP tool error (isError with the error message in + * content) so agents can read it and self-correct: not as an opaque JSON-RPC -32603 internal + * error, which would fail the tool call at the protocol level. + */ + @Test + @Order(17) + void testFindWithInvalidFilterReturnsToolError() { + callToolAndAssert( + CommandName.Names.FIND, + Map.of( + "keyspace", + keyspaceName, + "collection", + collectionName, + "filter", + Map.of("age", Map.of("$invalidOperator", 40))), + assertErrorOnly( + errors -> { + String message = errors.getJsonObject(0).getString("message"); + assertNotNull(message, "Tool error must carry a message for self-correction"); + assertFalse(message.isBlank(), "Tool error message must not be blank"); + })); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/GeneralCommandToolsMcpIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/GeneralCommandToolsMcpIntegrationTest.java index 7a94fcd73f..191983856f 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/GeneralCommandToolsMcpIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/GeneralCommandToolsMcpIntegrationTest.java @@ -71,6 +71,24 @@ void testFindKeyspaceToolCallAfterDropKeyspace() { } } + /** + * Status-only commands must return a non-empty {@code content}: per the MCP spec, clients (and + * the LLMs behind them) read tool results from {@code content}, while {@code _meta} is ignored by + * mainstream clients. An empty content list makes results invisible to standard MCP clients. + */ + @Test + void findKeyspacesToolCallReturnsNonEmptyContent() { + callToolAndAssert( + CommandName.Names.FIND_KEYSPACES, + Map.of(), + response -> { + assertFalse(response.isError()); + JsonObject envelope = contentEnvelope(response); + JsonArray keyspaces = envelope.getJsonObject("status").getJsonArray("keyspaces"); + assertNotNull(keyspaces, "Keyspaces must be visible in content, not only in _meta"); + }); + } + @Test void findEmbeddingProvidersToolCall() { callToolAndAssert( diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/KeyspaceCommandToolsMcpIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/KeyspaceCommandToolsMcpIntegrationTest.java index 8a100f154d..3099f8fa5c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/KeyspaceCommandToolsMcpIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/KeyspaceCommandToolsMcpIntegrationTest.java @@ -89,6 +89,23 @@ void testFindCollectionToolCallAfterDeleteCollection() { } } + /** + * Failures while building the keyspace command context (here: keyspace does not exist) must + * surface as a proper MCP tool error (isError with the error message in content) so agents can + * read it and self-correct: not as an opaque JSON-RPC -32603 internal error. + */ + @Test + void testFindCollectionsOnMissingKeyspaceReturnsToolError() { + callToolAndAssert( + CommandName.Names.FIND_COLLECTIONS, + Map.of("keyspace", "no_such_keyspace"), + assertErrorOnly( + errors -> { + String message = errors.getJsonObject(0).getString("message"); + assertNotNull(message, "Tool error must carry a message for self-correction"); + })); + } + @Nested @TestMethodOrder(MethodOrderer.OrderAnnotation.class) class CreateFindAndDropTableToolCall { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizerTest.java new file mode 100644 index 0000000000..200acc7db7 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpClauseSchemaCustomizerTest.java @@ -0,0 +1,118 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.databind.JsonNode; +import com.github.victools.jsonschema.generator.Option; +import com.github.victools.jsonschema.generator.OptionPreset; +import com.github.victools.jsonschema.generator.SchemaGenerator; +import com.github.victools.jsonschema.generator.SchemaGeneratorConfigBuilder; +import com.github.victools.jsonschema.generator.SchemaVersion; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.FilterDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.SortDefinition; +import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.FindAndRerankSort; +import io.stargate.sgv2.jsonapi.api.model.command.clause.update.UpdateClause; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Unit tests for {@link McpClauseSchemaCustomizer}. Verifies that user-facing JSON clause types are + * advertised as plain JSON objects in MCP tool schemas, instead of leaking their internal Java + * shape (e.g. {@code filterClause} / {@code json} for {@link FilterDefinition}), which the server + * cannot deserialize. + */ +class McpClauseSchemaCustomizerTest { + + static Stream clauseTypesAndLeakedProperties() { + return Stream.of( + Arguments.of(FilterDefinition.class, new String[] {"filterClause", "json"}), + Arguments.of(SortDefinition.class, new String[] {"sortClause", "json"}), + Arguments.of(UpdateClause.class, new String[] {"updateOperationDefs"}), + Arguments.of( + FindAndRerankSort.class, + new String[] {"commandFeatures", "lexicalSort", "vectorSort", "vectorizeSort"})); + } + + @ParameterizedTest + @MethodSource("clauseTypesAndLeakedProperties") + void shouldAdvertiseClauseTypeAsPlainObject(Class clauseType, String[] leakedProperties) { + JsonNode schema = generateSchema(clauseType); + + assertEquals("object", schema.path("type").asText(), "Schema type should be a plain object"); + assertFalse( + schema.path("description").asText().isBlank(), + "Schema should carry a description of the Data API clause syntax"); + + JsonNode properties = schema.get("properties"); + for (String leaked : leakedProperties) { + assertTrue( + properties == null || properties.get(leaked) == null, + "Internal Java field '%s' must not leak into the advertised schema".formatted(leaked)); + } + } + + @ParameterizedTest + @MethodSource("clauseTypesAndLeakedProperties") + void shouldEmbedValidJsonExample(Class clauseType, String[] ignored) { + JsonNode schema = generateSchema(clauseType); + + JsonNode examples = schema.get("examples"); + assertNotNull(examples, "Schema should embed an example of the accepted clause syntax"); + assertTrue(examples.isArray() && !examples.isEmpty(), "examples should be a non-empty array"); + assertTrue(examples.get(0).isObject(), "Example should be a JSON object"); + } + + @Test + void shouldNotAffectRegularTypes() { + record RegularOptions(Boolean ifNotExists) {} + + JsonNode schema = generateSchema(RegularOptions.class); + + assertNotNull( + schema.path("properties").get("ifNotExists"), + "Regular types should still expose their fields"); + } + + /** A {@link JsonDefinition} subtype without an explicit entry in the customizer's registry. */ + private static class UnregisteredDefinition + extends io.stargate.sgv2.jsonapi.api.model.command.JsonDefinition { + UnregisteredDefinition(com.fasterxml.jackson.databind.JsonNode json) { + super(json); + } + + @Override + public Object build(io.stargate.sgv2.jsonapi.api.model.command.CommandContext ctx) { + return null; + } + } + + @Test + void shouldFallBackToGenericPlainObjectForUnregisteredJsonDefinitions() { + JsonNode schema = generateSchema(UnregisteredDefinition.class); + + assertEquals("object", schema.path("type").asText(), "Schema type should be a plain object"); + assertFalse( + schema.path("description").asText().isBlank(), + "Fallback schema should carry a generic description"); + JsonNode properties = schema.get("properties"); + assertTrue( + properties == null || properties.get("json") == null, + "Internal field json must not leak into the fallback schema"); + assertNull(schema.get("examples"), "Fallback schema has no example to advertise"); + } + + /** + * Generates a schema with a {@link SchemaGeneratorConfigBuilder} matching the configuration used + * by Quarkus MCP Server's {@code DefaultSchemaGenerator}, with the customizer under test applied. + */ + private static JsonNode generateSchema(Class type) { + var configBuilder = + new SchemaGeneratorConfigBuilder(SchemaVersion.DRAFT_2020_12, OptionPreset.PLAIN_JSON) + .without(Option.SCHEMA_VERSION_INDICATOR); + new McpClauseSchemaCustomizer().customize(configBuilder); + return new SchemaGenerator(configBuilder.build()).generateSchema(type); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpIntegrationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpIntegrationTestBase.java index 392bf69c8f..c55e454c19 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpIntegrationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpIntegrationTestBase.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.*; import io.quarkiverse.mcp.server.MetaKey; +import io.quarkiverse.mcp.server.TextContent; import io.quarkiverse.mcp.server.ToolResponse; import io.quarkiverse.mcp.server.test.McpAssured; import io.quarkiverse.mcp.server.test.McpAssured.McpStreamableTestClient; @@ -17,6 +18,7 @@ import java.time.Duration; import java.util.Base64; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; @@ -141,25 +143,35 @@ protected void dropTable(String keyspace, String table) { } /** - * Assert that the response is a successful status-only (ok:1) response with no error, no - * structured content, and no text content. + * Assert the response carries a non-empty content list whose first item is a {@link TextContent} + * holding the JSON-serialized command result envelope {@code {data, status, errors}}, and return + * the parsed envelope. Per the MCP spec, {@code content} is what clients read, so every tool + * response must carry the result there. */ - protected Consumer assertStatusOnlyOk() { - return response -> { - assertFalse(response.isError()); - assertNotNull(response._meta()); - assertNull(response.structuredContent()); - assertThat(response.content()).isEmpty(); + protected JsonObject contentEnvelope(ToolResponse response) { + assertThat(response.content()) + .as("content must not be empty: MCP clients read tool results from content") + .isNotEmpty(); + var first = response.content().get(0); + var text = assertInstanceOf(TextContent.class, first); + return new JsonObject(text.text()); + } - var status = (JsonObject) response._meta().get(MetaKey.of("status")); - assertNotNull(status, "Status should not be null"); - assertEquals(1, status.getInteger("ok"), "Status should contain ok:1"); - }; + /** + * Assert that the response is a successful status-only (ok:1) response with no error and no + * structured content, and the envelope in content carries the status. + */ + protected Consumer assertStatusOnlyOk() { + return assertStatusOnlyWithJson( + status -> assertEquals(1, status.getInteger("ok"), "Status should contain ok:1")); } /** * Assert status only response structure is valid, then apply additional assertions on the status - * JsonObject extracted from _meta. + * JsonObject extracted from both _meta and the content envelope. + * + *

The assertions are applied twice (to the structuredContent/_meta view and to the content + * envelope), so they must be side-effect free. * * @param statusAssertions additional assertions to run on the status JsonObject */ @@ -168,23 +180,29 @@ protected Consumer assertStatusOnlyWithJson(Consumer s assertFalse(response.isError()); assertNotNull(response._meta()); assertNull(response.structuredContent()); - assertThat(response.content()).isEmpty(); var status = (JsonObject) response._meta().get(MetaKey.of("status")); assertNotNull(status, "Status should not be null"); statusAssertions.accept(status); + + var envelopeStatus = contentEnvelope(response).getJsonObject("status"); + assertNotNull(envelopeStatus, "Status should be visible in the content envelope"); + statusAssertions.accept(envelopeStatus); }; } /** - * Assert error response structure is valid (no meta_ and content), then apply additional - * assertions on the error JsonArray extracted from structuredContent. + * Assert error response structure is valid (error visible in content and structuredContent, no + * meta_), then apply additional assertions on the error JsonArray extracted from + * structuredContent and from the content envelope. + * + *

The assertions are applied twice (to the structuredContent/_meta view and to the content + * envelope), so they must be side-effect free. * * @param errorsAssertions additional assertions to run on the error JsonArray */ protected Consumer assertErrorOnly(Consumer errorsAssertions) { return response -> { - assertThat(response.content()).isEmpty(); assertThat(response._meta()).isEmpty(); assertTrue(response.isError()); assertThat(response.structuredContent()).isNotNull(); @@ -194,30 +212,46 @@ protected Consumer assertErrorOnly(Consumer errorsAsser var errorsArray = errors.getJsonArray("errors"); assertThat(errorsArray).isNotEmpty(); errorsAssertions.accept(errorsArray); + + var envelopeErrors = contentEnvelope(response).getJsonArray("errors"); + assertThat(envelopeErrors) + .as("Errors should be visible in the content envelope") + .isNotEmpty(); + errorsAssertions.accept(envelopeErrors); }; } /** - * Assert data only response is valid (no meta_, content and error), then apply additional - * assertions on the structuredContent holds the data. + * Assert data only response is valid (no meta_ and error), then apply additional assertions on + * the structuredContent holds the data and the content envelope carries it as well. + * + *

The assertions are applied twice (to the structuredContent/_meta view and to the content + * envelope), so they must be side-effect free. * * @param dataAssertions additional assertions to run on the data JsonObject */ protected Consumer assertDataOnly(Consumer dataAssertions) { return response -> { assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertThat(response._meta()).isEmpty(); assertNotNull(response.structuredContent()); var data = (JsonObject) response.structuredContent(); dataAssertions.accept(data); + + var envelopeData = contentEnvelope(response).getJsonObject("data"); + assertNotNull(envelopeData, "Data should be visible in the content envelope"); + dataAssertions.accept(envelopeData); }; } /** - * Assert data and status response is valid (no content and error), then apply additional - * assertions on the structuredContent holds the data and meta_ contains the status. + * Assert data and status response is valid (no error), then apply additional assertions on the + * structuredContent holds the data and meta_ contains the status; both must also be visible in + * the content envelope. + * + *

The assertions are applied twice (to the structuredContent/_meta view and to the content + * envelope), so they must be side-effect free. * * @param dataAssertions additional assertions to run on the data JsonObject * @param statusAssertions additional assertions to run on the status JsonObject @@ -226,7 +260,6 @@ protected Consumer assertDataAndStatus( Consumer dataAssertions, Consumer statusAssertions) { return response -> { assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertNotNull(response._meta()); assertNotNull(response.structuredContent()); @@ -234,6 +267,14 @@ protected Consumer assertDataAndStatus( dataAssertions.accept(data); var status = (JsonObject) response._meta().get(MetaKey.of("status")); statusAssertions.accept(status); + + var envelope = contentEnvelope(response); + var envelopeData = envelope.getJsonObject("data"); + assertNotNull(envelopeData, "Data should be visible in the content envelope"); + dataAssertions.accept(envelopeData); + var envelopeStatus = envelope.getJsonObject("status"); + assertNotNull(envelopeStatus, "Status should be visible in the content envelope"); + statusAssertions.accept(envelopeStatus); }; } @@ -248,4 +289,35 @@ protected void callToolAndAssert( String toolName, Map args, Consumer assertFn) { mcpClient.when().toolsCall(toolName, args, assertFn).thenAssertResults(); } + + /** + * Look up a tool by name via tools/list, following pagination cursors if needed. + * + * @param toolName the MCP tool name to look up + * @return the {@link McpAssured.ToolInfo} for the tool, never null + */ + protected McpAssured.ToolInfo findTool(String toolName) { + AtomicReference found = new AtomicReference<>(); + AtomicReference cursor = new AtomicReference<>(); + do { + var message = mcpClient.when().toolsList(); + if (cursor.get() != null) { + message = message.withCursor(cursor.get()); + } + message + .withAssert( + page -> { + cursor.set(page.nextCursor()); + page.tools().stream() + .filter(tool -> tool.name().equals(toolName)) + .findFirst() + .ifPresent(found::set); + }) + .send() + .thenAssertResults(); + } while (found.get() == null && cursor.get() != null); + + assertNotNull(found.get(), "Tool not found in tools/list: " + toolName); + return found.get(); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/response/CommandResultToToolResponseTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/response/CommandResultToToolResponseTest.java index a6e0d02457..6cb137b7be 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/response/CommandResultToToolResponseTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/response/CommandResultToToolResponseTest.java @@ -4,7 +4,10 @@ import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertNotNull; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import io.quarkiverse.mcp.server.MetaKey; +import io.quarkiverse.mcp.server.TextContent; import io.quarkiverse.mcp.server.ToolResponse; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; @@ -14,13 +17,15 @@ import org.junit.jupiter.api.Test; /** - * Unit tests for {@link CommandResult#toToolResponse()} verifying the CommandResult → ToolResponse - * mapping logic. + * Unit tests for {@link CommandResult#toToolResponse(ObjectMapper)} verifying the CommandResult → + * ToolResponse mapping logic. */ public class CommandResultToToolResponseTest { + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + @Test - void successWithStatusOnly() { + void successWithStatusOnly() throws Exception { // Arrange: a DDL-style result with status and no errors or data var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) @@ -28,18 +33,21 @@ void successWithStatusOnly() { .build(); // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(OBJECT_MAPPER); - // Assert: no error, no content and structuredContent, status should be mapped into _meta + // Assert: no error, no structuredContent, status should be mapped into _meta assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertNull(response.structuredContent()); assertNotNull(response._meta()); assertNotNull(response._meta().get(MetaKey.of("status"))); + + // Assert: the full envelope is visible in content (what MCP clients read) + JsonNode envelope = contentEnvelope(response); + assertEquals(1, envelope.path("status").path("ok").asInt()); } @Test - void successWithStatusAndData() { + void successWithStatusAndData() throws Exception { // Arrange: a result with data and status var commandResult = CommandResult.singleDocumentBuilder(RequestTracing.NO_OP) @@ -48,19 +56,23 @@ void successWithStatusAndData() { .build(); // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(OBJECT_MAPPER); - // Assert: no error, no content, data in structuredContent, status should be mapped into _meta + // Assert: no error, data in structuredContent, status should be mapped into _meta assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertThat(response.structuredContent()).isNotNull(); assertInstanceOf(ResponseData.class, response.structuredContent()); assertNotNull(response._meta()); assertNotNull(response._meta().get(MetaKey.of("status"))); + + // Assert: the full envelope is visible in content, with both data and status + JsonNode envelope = contentEnvelope(response); + assertTrue(envelope.has("data"), "Envelope should contain data"); + assertEquals(0, envelope.path("status").path("deletedCount").asInt()); } @Test - void failWithErrorOnly() { + void failWithErrorOnly() throws Exception { // Arrange: a result that contains errors var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) @@ -68,20 +80,24 @@ void failWithErrorOnly() { .build(); // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(OBJECT_MAPPER); - // Assert: should be an error in structuredContent, no _meta and content - assertThat(response.content()).isEmpty(); + // Assert: should be an error in structuredContent, no _meta assertThat(response._meta()).isEmpty(); assertTrue(response.isError()); assertThat(response.structuredContent()).isNotNull(); @SuppressWarnings("unchecked") var errorContent = (Map) response.structuredContent(); assertThat(errorContent).containsKey("errors"); + + // Assert: errors (including the message) are visible in content so agents can self-correct + JsonNode envelope = contentEnvelope(response); + assertThat(envelope.path("errors").isArray()).isTrue(); + assertThat(envelope.path("errors").get(0).path("message").asText()).contains("test error"); } @Test - void failWithErrorAndStatus() { + void failWithErrorAndStatus() throws Exception { // Arrange: a result that contains errors and status var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) @@ -90,10 +106,9 @@ void failWithErrorAndStatus() { .build(); // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(OBJECT_MAPPER); - // Assert: no content, should be an error in structuredContent, and status in _meta - assertThat(response.content()).isEmpty(); + // Assert: should be an error in structuredContent, and status in _meta assertTrue(response.isError()); assertThat(response.structuredContent()).isNotNull(); @SuppressWarnings("unchecked") @@ -101,5 +116,22 @@ void failWithErrorAndStatus() { assertThat(errorContent).containsKey("errors"); assertNotNull(response._meta()); assertNotNull(response._meta().get(MetaKey.of("status"))); + + // Assert: both errors and status are visible in content + JsonNode envelope = contentEnvelope(response); + assertThat(envelope.path("errors").isArray()).isTrue(); + assertTrue(envelope.has("status"), "Envelope should contain status"); + } + + /** + * Asserts the response carries a non-empty content list whose first item is a {@link TextContent} + * holding the JSON-serialized command result envelope, and returns the parsed envelope. + */ + private static JsonNode contentEnvelope(ToolResponse response) throws Exception { + assertThat(response.content()) + .as("content must not be empty: MCP clients read tool results from content") + .isNotEmpty(); + var text = assertInstanceOf(TextContent.class, response.content().get(0)); + return OBJECT_MAPPER.readTree(text.text()); } }