From dfd562107bc01669ad1eeccab70ccaac694e59e6 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Tue, 13 Oct 2026 14:14:32 -0700 Subject: [PATCH 1/7] test: require MCP tool calls to close their command context --- .../jsonapi/api/v1/mcp/McpResourceTest.java | 267 ++++++++++++++++++ 1 file changed, 267 insertions(+) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java new file mode 100644 index 0000000000..89ebb611cd --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java @@ -0,0 +1,267 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.micrometer.core.instrument.MeterRegistry; +import io.quarkus.security.identity.SecurityIdentity; +import io.smallrye.mutiny.Uni; +import io.stargate.sgv2.jsonapi.TestConstants; +import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.api.model.command.KeyspaceCommand; +import io.stargate.sgv2.jsonapi.api.model.command.tracing.RequestTracing; +import io.stargate.sgv2.jsonapi.api.request.EmbeddingCredentials; +import io.stargate.sgv2.jsonapi.api.request.EmbeddingCredentialsResolver; +import io.stargate.sgv2.jsonapi.api.request.RequestContext; +import io.stargate.sgv2.jsonapi.api.request.tenant.RequestTenantResolver; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeatures; +import io.stargate.sgv2.jsonapi.metrics.JsonProcessingMetricsReporter; +import io.stargate.sgv2.jsonapi.service.cqldriver.CQLSessionCache; +import io.stargate.sgv2.jsonapi.service.cqldriver.CqlSessionCacheSupplier; +import io.stargate.sgv2.jsonapi.service.embedding.operation.EmbeddingProviderFactory; +import io.stargate.sgv2.jsonapi.service.processor.MeteredCommandProcessor; +import io.stargate.sgv2.jsonapi.service.reranking.operation.RerankingProviderFactory; +import io.stargate.sgv2.jsonapi.service.schema.SchemaObjectCache; +import io.stargate.sgv2.jsonapi.service.schema.SchemaObjectCacheSupplier; +import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; +import io.vertx.core.MultiMap; +import io.vertx.core.http.HttpServerRequest; +import io.vertx.ext.web.RoutingContext; +import jakarta.enterprise.inject.Instance; +import jakarta.inject.Provider; +import java.security.Principal; +import java.time.Duration; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +/** + * Unit tests for the request-context lifetime in {@link McpResource}: one context per tool call, + * closed exactly once when the call ends. + */ +class McpResourceTest { + + private static final TestConstants TEST_CONSTANTS = new TestConstants(); + + private final MeteredCommandProcessor commandProcessor = mock(MeteredCommandProcessor.class); + private final SchemaObjectCache schemaObjectCache = mock(SchemaObjectCache.class); + + @Test + void closesContextOnSuccess() throws Exception { + var context = commandContext(true); + var command = mock(KeyspaceCommand.class); + var result = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).build(); + when(commandProcessor.processCommand(context, command)) + .thenReturn(Uni.createFrom().item(result)); + + var response = + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5)); + + assertThat(response.isError()).isFalse(); + verify(context, times(1)).close(); + } + + @Test + void closesContextOnFailure() throws Exception { + var context = commandContext(true); + var command = mock(KeyspaceCommand.class); + when(commandProcessor.processCommand(context, command)) + .thenReturn(Uni.createFrom().failure(new RuntimeException("processor failed"))); + + assertThatThrownBy( + () -> + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5))) + .hasMessage("processor failed"); + verify(context, times(1)).close(); + } + + @Test + void closesContextWhenFeatureDisabled() throws Exception { + var context = commandContext(false); + var command = mock(KeyspaceCommand.class); + + var response = + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5)); + + assertThat(response.isError()).isTrue(); + verifyNoInteractions(commandProcessor); + verify(context, times(1)).close(); + } + + @Test + void closesContextOnCancellation() throws Exception { + var context = commandContext(true); + var command = mock(KeyspaceCommand.class); + when(commandProcessor.processCommand(context, command)) + .thenReturn(Uni.createFrom().emitter(emitter -> {})); + + var cancellable = + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .subscribe() + .with(response -> {}, failure -> {}); + cancellable.cancel(); + + verify(context, times(1)).close(); + } + + @Test + void closeFailureDoesNotReplaceResult() throws Exception { + var context = commandContext(true); + doThrow(new Exception("close failed")).when(context).close(); + var command = mock(KeyspaceCommand.class); + var result = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).build(); + when(commandProcessor.processCommand(context, command)) + .thenReturn(Uni.createFrom().item(result)); + + var response = + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5)); + + assertThat(response.isError()).isFalse(); + verify(context, times(1)).close(); + } + + @Test + void closeFailureDoesNotReplaceError() throws Exception { + var context = commandContext(true); + doThrow(new Exception("close failed")).when(context).close(); + var command = mock(KeyspaceCommand.class); + when(commandProcessor.processCommand(context, command)) + .thenReturn(Uni.createFrom().failure(new RuntimeException("processor failed"))); + + assertThatThrownBy( + () -> + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5))) + .hasMessage("processor failed"); + verify(context, times(1)).close(); + } + + @Test + void keyspaceContextUsesOneRequestContext() { + var requestContextCaptor = ArgumentCaptor.forClass(RequestContext.class); + when(schemaObjectCache.getKeyspace(requestContextCaptor.capture(), any(), any(), eq(true))) + .thenReturn(Uni.createFrom().item(TEST_CONSTANTS.KEYSPACE_SCHEMA_OBJECT)); + + var context = + mcpResourceWithRequest() + .buildKeyspaceContext(TEST_CONSTANTS.KEYSPACE_NAME, mock(KeyspaceCommand.class)) + .await() + .atMost(Duration.ofSeconds(5)); + + // The schema lookup and the built command context must see the same request, so the + // call has one request ID from start to finish. + assertThat(context.requestContext()).isSameAs(requestContextCaptor.getValue()); + assertThat(context.requestContext().requestId()) + .isEqualTo(requestContextCaptor.getValue().requestId()); + } + + @SuppressWarnings("unchecked") + private CommandContext commandContext(boolean mcpEnabled) { + CommandContext context = mock(CommandContext.class); + var apiFeatures = + mcpEnabled + ? ApiFeatures.fromConfigAndRequest(Map.of(ApiFeature.MCP, "true"), null) + : ApiFeatures.empty(); + when(context.apiFeatures()).thenReturn(apiFeatures); + when(context.requestTracing()).thenReturn(RequestTracing.NO_OP); + return context; + } + + private McpResource mcpResource() { + return mcpResource( + () -> mock(RoutingContext.class), + () -> mock(SecurityIdentity.class), + instanceOf(mock(RequestTenantResolver.class)), + instanceOf(mock(EmbeddingCredentialsResolver.class))); + } + + private McpResource mcpResource( + Provider routingContextProvider, + Provider securityIdentityProvider, + Instance tenantResolver, + Instance embeddingCredentialsResolver) { + return new McpResource( + mock(ObjectMapper.class), + commandProcessor, + mock(MeterRegistry.class), + mock(JsonProcessingMetricsReporter.class), + sessionCacheSupplier(), + mock(EmbeddingProviderFactory.class), + mock(RerankingProviderFactory.class), + schemaCacheSupplier(), + routingContextProvider, + securityIdentityProvider, + tenantResolver, + embeddingCredentialsResolver); + } + + /** An {@link McpResource} whose request can build a real {@link RequestContext}. */ + private McpResource mcpResourceWithRequest() { + var httpRequest = mock(HttpServerRequest.class); + when(httpRequest.headers()).thenReturn(mock(MultiMap.class)); + var routingContext = mock(RoutingContext.class); + when(routingContext.request()).thenReturn(httpRequest); + + var principal = mock(Principal.class); + when(principal.getName()).thenReturn("test-token"); + var securityIdentity = mock(SecurityIdentity.class); + when(securityIdentity.getPrincipal()).thenReturn(principal); + + var tenantResolver = mock(RequestTenantResolver.class); + when(tenantResolver.resolve(any(), any())).thenReturn(TEST_CONSTANTS.TENANT); + var embeddingCredentialsResolver = mock(EmbeddingCredentialsResolver.class); + when(embeddingCredentialsResolver.resolveEmbeddingCredentials(any(), any())) + .thenReturn(mock(EmbeddingCredentials.class)); + + return mcpResource( + () -> routingContext, + () -> securityIdentity, + instanceOf(tenantResolver), + instanceOf(embeddingCredentialsResolver)); + } + + private CqlSessionCacheSupplier sessionCacheSupplier() { + var supplier = mock(CqlSessionCacheSupplier.class); + when(supplier.get()).thenReturn(mock(CQLSessionCache.class)); + return supplier; + } + + private SchemaObjectCacheSupplier schemaCacheSupplier() { + var supplier = mock(SchemaObjectCacheSupplier.class); + when(supplier.get()).thenReturn(schemaObjectCache); + return supplier; + } + + @SuppressWarnings("unchecked") + private static Instance instanceOf(T value) { + Instance instance = mock(Instance.class); + when(instance.get()).thenReturn(value); + return instance; + } +} From 04adc360db900d700d069930b08776106e3695b5 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Tue, 13 Oct 2026 14:14:34 -0700 Subject: [PATCH 2/7] fix: close the MCP command context when a tool call ends --- .../sgv2/jsonapi/api/v1/mcp/McpResource.java | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) 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..d4965452a7 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 @@ -131,7 +131,7 @@ public Uni> buildKeyspaceContext(String keyspace, KeyspaceComm contextBuilderSupplier .getBuilder(keyspaceSchemaObject) .withCommandName(command.getClass().getSimpleName()) - .withRequestContext(createRequestContext()) + .withRequestContext(requestContext) .build()); } @@ -225,14 +225,32 @@ public Uni processCommand(Uni> contextUni, Comma return contextUni .flatMap( context -> { + Uni resultUni; if (!context.apiFeatures().isFeatureEnabled(ApiFeature.MCP)) { - return Uni.createFrom() - .item( - CommandResult.statusOnlyBuilder(context.requestTracing()) - .addThrowable(SchemaException.Code.MCP_FEATURE_NOT_ENABLED.get()) - .build()); + resultUni = + Uni.createFrom() + .item( + CommandResult.statusOnlyBuilder(context.requestTracing()) + .addThrowable(SchemaException.Code.MCP_FEATURE_NOT_ENABLED.get()) + .build()); + } else { + resultUni = meteredCommandProcessor.processCommand(context, command); } - return meteredCommandProcessor.processCommand(context, command); + // Same as the HTTP resources: closing removes this request's values from the + // logging context, and must happen however the call ends. + return resultUni + .onTermination() + .invoke( + () -> { + try { + context.close(); + } catch (Exception e) { + LOGGER.error( + "Error closing the command context for requestContext={}", + context.requestContext(), + e); + } + }); }) .map(CommandResult::toToolResponse); } From 777b416ab20084c17cc3c0ef3ea02d0720b4f5bb Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 22 Oct 2026 09:40:52 -0700 Subject: [PATCH 3/7] Return readable MCP tool results --- .../api/model/command/CommandResult.java | 31 ++- .../sgv2/jsonapi/api/v1/mcp/McpResource.java | 14 +- .../api/v1/mcp/McpIntegrationTestBase.java | 31 ++- .../McpReadableResultsIntegrationTest.java | 103 +++++++++ .../TableCommandToolsMcpIntegrationTest.java | 1 + .../CommandResultToToolResponseTest.java | 217 ++++++++++++++---- 6 files changed, 316 insertions(+), 81 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.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..ebc27f04eb 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; @@ -101,18 +104,21 @@ public RestResponse toRestResponse() { /** * Create a {@link ToolResponse} from this {@link CommandResult} for MCP tool responses. * - *

Mapping rules: - * - *

    - *
  • {@link #errors()} → {@link ToolResponse#isError()} and error content in {@link - * ToolResponse#structuredContent()} - *
  • {@link #data()} → {@link ToolResponse#structuredContent()} (when no errors) - *
  • {@link #status()} → {@link ToolResponse#_meta()} with key {@code "status"} - *
+ *

Text content contains the complete Data API response. Structured content and status metadata + * retain their existing shapes for clients that consume them. * + * @param objectMapper the configured Data API response mapper * @return A new {@link ToolResponse} representing this command result. */ - public ToolResponse toToolResponse() { + public ToolResponse toToolResponse(ObjectMapper objectMapper) { + + final String text; + try { + text = objectMapper.writeValueAsString(this); + } catch (JsonProcessingException e) { + // Neither the partial response nor the exception message is safe to return here. + return ToolResponse.error("Unable to serialize the command result."); + } boolean hasErrors = errors != null && !errors.isEmpty(); @@ -120,10 +126,11 @@ public ToolResponse toToolResponse() { Map meta = (status != null && !status.isEmpty()) ? Map.of(MetaKey.of("status"), status) : Map.of(); - // 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(text)), + hasErrors ? Map.of("errors", errors) : data, + meta); } /** 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..64750c2ea1 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 @@ -165,7 +165,7 @@ public Uni processCollectionCommand( CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) .addThrowable(throwable) .build(); - return Uni.createFrom().item(errorResult.toToolResponse()); + return Uni.createFrom().item(errorResult.toToolResponse(objectMapper)); } else { VectorColumnDefinition vectorColDef = null; if (schemaObject.type() == SchemaObjectType.COLLECTION) { @@ -211,14 +211,12 @@ public Uni processCollectionCommand( } /** - * Process a general or keyspace command and return the CommandResult directly. + * Process a command and map its result to an MCP tool response with readable text and structured + * content. * - *

The Quarkus MCP Server framework automatically encodes the {@link CommandResult} to JSON via - * its built-in encoder (Jackson), so no manual serialization is needed. - * - * @param contextUni The command context (from buildGeneralContext or buildKeyspaceContext) + * @param contextUni The command context * @param command The command to execute - * @return Uni of CommandResult + * @return Uni of ToolResponse */ public Uni processCommand(Uni> contextUni, Command command) { @@ -234,7 +232,7 @@ public Uni processCommand(Uni> contextUni, Comma } return meteredCommandProcessor.processCommand(context, command); }) - .map(CommandResult::toToolResponse); + .map(result -> result.toToolResponse(objectMapper)); } /** 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 e2f7addc46..b1f3a48c66 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 @@ -142,18 +142,18 @@ 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. + * structured content, and a readable Data API response. */ protected Consumer assertStatusOnlyOk() { return response -> { 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"); assertEquals(1, status.getInteger("ok"), "Status should contain ok:1"); + assertThat(readableResult(response)).isEqualTo(new JsonObject().put("status", status)); }; } @@ -168,11 +168,11 @@ 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); + assertThat(readableResult(response)).isEqualTo(new JsonObject().put("status", status)); }; } @@ -184,14 +184,13 @@ private static void assertServerInfoOnly(ToolResponse response) { } /** - * Assert error response structure is valid (server metadata and no text content), then apply + * Assert error response structure is valid with server metadata and readable content, then apply * additional assertions on the error JsonArray extracted from structuredContent. * * @param errorsAssertions additional assertions to run on the error JsonArray */ protected Consumer assertErrorOnly(Consumer errorsAssertions) { return response -> { - assertThat(response.content()).isEmpty(); assertServerInfoOnly(response); assertTrue(response.isError()); assertThat(response.structuredContent()).isNotNull(); @@ -201,11 +200,12 @@ protected Consumer assertErrorOnly(Consumer errorsAsser var errorsArray = errors.getJsonArray("errors"); assertThat(errorsArray).isNotEmpty(); errorsAssertions.accept(errorsArray); + assertThat(readableResult(response)).isEqualTo(errors); }; } /** - * Assert data only response is valid (server metadata, no text content or error), then apply + * Assert data only response is valid with server metadata and readable content, then apply * additional assertions on the structuredContent holds the data. * * @param dataAssertions additional assertions to run on the data JsonObject @@ -213,18 +213,20 @@ protected Consumer assertErrorOnly(Consumer errorsAsser protected Consumer assertDataOnly(Consumer dataAssertions) { return response -> { assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertServerInfoOnly(response); assertNotNull(response.structuredContent()); var data = (JsonObject) response.structuredContent(); dataAssertions.accept(data); + var readable = readableResult(response); + assertThat(readable.fieldNames()).containsExactly("data"); + dataAssertions.accept(readable.getJsonObject("data")); }; } /** - * 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 with readable content and no error, then apply + * additional assertions on the structuredContent holds the data and meta_ contains the status. * * @param dataAssertions additional assertions to run on the data JsonObject * @param statusAssertions additional assertions to run on the status JsonObject @@ -233,7 +235,6 @@ protected Consumer assertDataAndStatus( Consumer dataAssertions, Consumer statusAssertions) { return response -> { assertFalse(response.isError()); - assertThat(response.content()).isEmpty(); assertNotNull(response._meta()); assertNotNull(response.structuredContent()); @@ -241,9 +242,19 @@ protected Consumer assertDataAndStatus( dataAssertions.accept(data); var status = (JsonObject) response._meta().get(MetaKey.of("status")); statusAssertions.accept(status); + var readable = readableResult(response); + assertThat(readable.fieldNames()).containsExactlyInAnyOrder("data", "status"); + // The Data API and MCP encoders format float vectors differently. Check the data behavior. + dataAssertions.accept(readable.getJsonObject("data")); + assertThat(readable.getJsonObject("status")).isEqualTo(status); }; } + private static JsonObject readableResult(ToolResponse response) { + assertThat(response.content()).hasSize(1); + return new JsonObject(response.firstContent().asText().text()); + } + /** * Execute an MCP tool call and assert the response using the shared client. * diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java new file mode 100644 index 0000000000..a9747525a3 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java @@ -0,0 +1,103 @@ +package io.stargate.sgv2.jsonapi.api.v1.mcp; + +import static io.restassured.RestAssured.given; +import static org.assertj.core.api.Assertions.assertThat; + +import io.quarkus.test.junit.QuarkusIntegrationTest; +import io.quarkus.test.junit.TestProfile; +import io.restassured.http.ContentType; +import io.stargate.sgv2.jsonapi.api.model.command.CommandName; +import io.stargate.sgv2.jsonapi.config.feature.ApiFeature; +import io.stargate.sgv2.jsonapi.exception.SchemaException; +import io.stargate.sgv2.jsonapi.testresource.NoGlobalResourcesTestProfile; +import io.vertx.core.json.JsonObject; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** Checks readable results from the packaged endpoint without a client translating the response. */ +@QuarkusIntegrationTest +@TestProfile(NoGlobalResourcesTestProfile.Impl.class) +class McpReadableResultsIntegrationTest { + private static final String PROTOCOL_VERSION = "2026-07-28"; + private static final String TOOL_NAME = CommandName.Names.FIND_RERANKING_PROVIDERS; + + @Test + void successfulToolTextMatchesTheHttpResponse() { + // Provider discovery uses packaged configuration and does not need a database or provider call. + var result = callTool(true); + assertThat(result.getBoolean("isError")).isFalse(); + + var httpResponse = + given() + .contentType(ContentType.JSON) + .header("Token", "readable-result-test-credential") + .header(ApiFeature.RERANKING.httpHeaderName(), "true") + .body(Map.of(TOOL_NAME, Map.of())) + .post("/v1") + .then() + .statusCode(200) + .extract() + .asString(); + + assertThat(readableResult(result)).isEqualTo(new JsonObject(httpResponse)); + assertThat(result.getJsonObject("_meta").getJsonObject("status")) + .isEqualTo(new JsonObject(httpResponse).getJsonObject("status")); + } + + @Test + void failedToolReturnsReadableErrorsInACompleteResult() { + var result = callTool(false); + assertThat(result.getBoolean("isError")).isTrue(); + var errors = result.getJsonObject("structuredContent").getJsonArray("errors"); + assertThat(errors.getJsonObject(0).getString("errorCode")) + .isEqualTo(SchemaException.Code.MCP_FEATURE_NOT_ENABLED.name()); + assertThat(readableResult(result)).isEqualTo(result.getJsonObject("structuredContent")); + } + + private static JsonObject readableResult(JsonObject result) { + var content = result.getJsonArray("content"); + assertThat(content).hasSize(1); + assertThat(content.getJsonObject(0).getString("type")).isEqualTo("text"); + return new JsonObject(content.getJsonObject(0).getString("text")); + } + + private JsonObject callTool(boolean mcpEnabled) { + var response = + given() + .contentType(ContentType.JSON) + .accept("application/json, text/event-stream") + // Existing token fixture. OAuth admission is tracked separately in #2642. + .header("Token", "readable-result-test-credential") + .header(ApiFeature.MCP.httpHeaderName(), mcpEnabled) + .header(ApiFeature.RERANKING.httpHeaderName(), "true") + .header("MCP-Protocol-Version", PROTOCOL_VERSION) + .header("Mcp-Method", "tools/call") + .header("Mcp-Name", TOOL_NAME) + .body( + Map.of( + "jsonrpc", + "2.0", + "id", + 1, + "method", + "tools/call", + "params", + Map.of( + "name", TOOL_NAME, + "arguments", Map.of(), + "_meta", + Map.of( + "io.modelcontextprotocol/protocolVersion", + PROTOCOL_VERSION, + "io.modelcontextprotocol/clientCapabilities", + Map.of())))) + .post("/v1/mcp"); + assertThat(response.statusCode()).as(response.asString()).isEqualTo(200); + var body = new JsonObject(response.asString()); + assertThat(body.getInteger("id")).isEqualTo(1); + assertThat(body.containsKey("error")).isFalse(); + var result = body.getJsonObject("result"); + assertThat(result.getString("resultType")).isEqualTo("complete"); + return result; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/TableCommandToolsMcpIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/TableCommandToolsMcpIntegrationTest.java index 31cac658da..c3c7ccfc48 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/TableCommandToolsMcpIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/TableCommandToolsMcpIntegrationTest.java @@ -273,6 +273,7 @@ void testFindOneToolCall() { JsonArray embedding = doc.getJsonArray("embedding"); assertNotNull(embedding, "embedding should not be null"); assertEquals(5, embedding.size()); + assertEquals(0.1, embedding.getDouble(0), 1e-6); }, status -> { // Table findOne returns projectionSchema describing all returned columns 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..a31aa4ea37 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 @@ -1,105 +1,220 @@ package io.stargate.sgv2.jsonapi.api.v1.response; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.module.SimpleModule; 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.CommandErrorFactory; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import io.stargate.sgv2.jsonapi.api.model.command.ResponseData; import io.stargate.sgv2.jsonapi.api.model.command.tracing.RequestTracing; +import java.io.IOException; +import java.util.List; import java.util.Map; 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 readable content and + * the existing structured response mapping. */ public class CommandResultToToolResponseTest { + private final ObjectMapper objectMapper = new ObjectMapper(); + @Test - void successWithStatusOnly() { - // Arrange: a DDL-style result with status and no errors or data + void successWithStatusOnly() throws Exception { var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) .addStatus(CommandStatus.OK, 1) .build(); - // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(objectMapper); - // Assert: no error, no content and 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"))); + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isNull(); + assertThat(response._meta()) + .containsExactlyEntriesOf(Map.of(MetaKey.of("status"), commandResult.status())); } @Test - void successWithStatusAndData() { - // Arrange: a result with data and status + void successWithStatusAndData() throws Exception { var commandResult = CommandResult.singleDocumentBuilder(RequestTracing.NO_OP) .addStatus(CommandStatus.DELETED_COUNT, 0) .addDocument(null) .build(); - // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(objectMapper); - // Assert: no error, no content, 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"))); + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(commandResult.data()); + assertThat(response._meta()) + .containsExactlyEntriesOf(Map.of(MetaKey.of("status"), commandResult.status())); } @Test - void failWithErrorOnly() { - // Arrange: a result that contains errors + void failWithErrorOnly() throws Exception { var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) .addThrowable(new RuntimeException("test error")) .build(); - // Act - ToolResponse response = commandResult.toToolResponse(); + ToolResponse response = commandResult.toToolResponse(objectMapper); - // Assert: should be an error in structuredContent, no _meta and content - assertThat(response.content()).isEmpty(); + assertThat(response.isError()).isTrue(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(Map.of("errors", commandResult.errors())); assertThat(response._meta()).isEmpty(); - assertTrue(response.isError()); - assertThat(response.structuredContent()).isNotNull(); - @SuppressWarnings("unchecked") - var errorContent = (Map) response.structuredContent(); - assertThat(errorContent).containsKey("errors"); } @Test - void failWithErrorAndStatus() { - // Arrange: a result that contains errors and status + void failWithErrorAndStatus() throws Exception { var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) .addStatus(CommandStatus.INSERTED_IDS, 1) .addThrowable(new RuntimeException("test error")) .build(); - // Act - ToolResponse response = commandResult.toToolResponse(); - - // Assert: no content, should be an error in structuredContent, and status in _meta - assertThat(response.content()).isEmpty(); - assertTrue(response.isError()); - assertThat(response.structuredContent()).isNotNull(); - @SuppressWarnings("unchecked") - var errorContent = (Map) response.structuredContent(); - assertThat(errorContent).containsKey("errors"); - assertNotNull(response._meta()); - assertNotNull(response._meta().get(MetaKey.of("status"))); + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isTrue(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(Map.of("errors", commandResult.errors())); + assertThat(response._meta()) + .containsExactlyEntriesOf(Map.of(MetaKey.of("status"), commandResult.status())); + } + + @Test + void successWithDocumentData() throws Exception { + var document = + objectMapper.createObjectNode().put("message", "Quotes: \"hello\"\nUnicode: café 🌟"); + document.putArray("values").add(0).add(true).addNull(); + var commandResult = + CommandResult.singleDocumentBuilder(RequestTracing.NO_OP).addDocument(document).build(); + + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(commandResult.data()); + assertThat(response._meta()).isEmpty(); + } + + @Test + void successWithMultipleDocumentsAndPagination() throws Exception { + var commandResult = + CommandResult.multiDocumentBuilder(RequestTracing.NO_OP) + .addDocument(objectMapper.createObjectNode().put("_id", "first")) + .addDocument(objectMapper.createObjectNode().put("_id", "second")) + .nextPageState("next-page") + .build(); + + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(commandResult.data()); + assertThat(response._meta()).isEmpty(); + } + + @Test + void successWithEmptyResult() throws Exception { + var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).build(); + + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isNull(); + assertThat(response._meta()).isEmpty(); + } + + @Test + void errorsDoNotHidePartialDataFromReadableContent() throws Exception { + var success = + CommandResult.multiDocumentBuilder(RequestTracing.NO_OP) + .addDocument(objectMapper.createObjectNode().put("_id", "successful-document")) + .addStatus(CommandStatus.INSERTED_IDS, List.of("successful-document")) + .build(); + var failure = + CommandResult.statusOnlyBuilder(RequestTracing.NO_OP) + .addThrowable(new RuntimeException("another document failed")) + .build(); + // The builder suppresses data on errors. The response mapper must also handle partial results. + var commandResult = new CommandResult(success.data(), success.status(), failure.errors()); + + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isTrue(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isEqualTo(Map.of("errors", commandResult.errors())); + assertThat(response._meta()) + .containsExactlyEntriesOf(Map.of(MetaKey.of("status"), commandResult.status())); + } + + @Test + void warningsRemainReadableWithoutMarkingResultAsError() throws Exception { + var commandResult = CommandResult.statusOnlyBuilder(RequestTracing.NO_OP).build(); + commandResult.addWarning(CommandErrorFactory.create(new RuntimeException("test warning"))); + + ToolResponse response = commandResult.toToolResponse(objectMapper); + + assertThat(response.isError()).isFalse(); + assertReadableResult(response, commandResult); + assertThat(response.structuredContent()).isNull(); + assertThat(response._meta()) + .containsExactlyEntriesOf(Map.of(MetaKey.of("status"), commandResult.status())); + } + + @Test + void serializationFailureReturnsSafeReadableError() { + String secret = "sensitive-payload-and-exception-details"; + var commandResult = + CommandResult.singleDocumentBuilder(RequestTracing.NO_OP) + .addDocument(objectMapper.createObjectNode().put("secret", secret)) + .addStatus(CommandStatus.INSERTED_IDS, List.of(secret)) + .build(); + var failingMapper = objectMapper.copy(); + failingMapper.registerModule( + new SimpleModule() + .addSerializer( + CommandResult.class, + new JsonSerializer() { + @Override + public void serialize( + CommandResult value, JsonGenerator generator, SerializerProvider provider) + throws IOException { + generator.writeStartObject(); + generator.writeStringField("partial", secret); + throw new IOException(secret); + } + })); + + ToolResponse response = commandResult.toToolResponse(failingMapper); + + assertThat(response.isError()).isTrue(); + assertThat(readableText(response)).isNotBlank().hasSizeLessThan(200).doesNotContain(secret); + assertThat(response.structuredContent()).isNull(); + assertThat(response._meta()).isNullOrEmpty(); + } + + private void assertReadableResult(ToolResponse response, CommandResult commandResult) + throws IOException { + assertThat(objectMapper.readTree(readableText(response))) + .isEqualTo(objectMapper.valueToTree(commandResult)); + } + + private String readableText(ToolResponse response) { + assertThat(response.content()).hasSize(1); + return assertInstanceOf(TextContent.class, response.content().getFirst()).text(); } } From 1cafd99cb8887bddd7ed0880f66c39a587751005 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 22 Oct 2026 10:30:28 -0700 Subject: [PATCH 4/7] Close MCP contexts after synchronous execution failures --- .../sgv2/jsonapi/api/v1/mcp/McpResource.java | 53 +++++++++---------- .../jsonapi/api/v1/mcp/McpResourceTest.java | 34 ++++++++++++ 2 files changed, 60 insertions(+), 27 deletions(-) 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 d4965452a7..85ed09ca45 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 @@ -223,36 +223,35 @@ public Uni processCollectionCommand( public Uni processCommand(Uni> contextUni, Command command) { return contextUni - .flatMap( - context -> { - Uni resultUni; + .flatMap(context -> executeCommand(context, command)) + .map(CommandResult::toToolResponse); + } + + private Uni executeCommand(CommandContext context, Command command) { + return Uni.createFrom() + .deferred( + () -> { if (!context.apiFeatures().isFeatureEnabled(ApiFeature.MCP)) { - resultUni = - Uni.createFrom() - .item( - CommandResult.statusOnlyBuilder(context.requestTracing()) - .addThrowable(SchemaException.Code.MCP_FEATURE_NOT_ENABLED.get()) - .build()); - } else { - resultUni = meteredCommandProcessor.processCommand(context, command); + return Uni.createFrom() + .item( + CommandResult.statusOnlyBuilder(context.requestTracing()) + .addThrowable(SchemaException.Code.MCP_FEATURE_NOT_ENABLED.get()) + .build()); } - // Same as the HTTP resources: closing removes this request's values from the - // logging context, and must happen however the call ends. - return resultUni - .onTermination() - .invoke( - () -> { - try { - context.close(); - } catch (Exception e) { - LOGGER.error( - "Error closing the command context for requestContext={}", - context.requestContext(), - e); - } - }); + return meteredCommandProcessor.processCommand(context, command); }) - .map(CommandResult::toToolResponse); + .onTermination() + .invoke( + () -> { + try { + context.close(); + } catch (Exception e) { + LOGGER.error( + "Error closing the command context for requestContext={}", + context.requestContext(), + e); + } + }); } /** diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java index 89ebb611cd..bf5508e958 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java @@ -92,6 +92,40 @@ void closesContextOnFailure() throws Exception { verify(context, times(1)).close(); } + @Test + void closesContextWhenProcessorThrowsSynchronously() throws Exception { + var context = commandContext(true); + var command = mock(KeyspaceCommand.class); + var failure = new IllegalStateException("processor failed before returning a Uni"); + when(commandProcessor.processCommand(context, command)).thenThrow(failure); + + assertThatThrownBy( + () -> + mcpResource() + .processCommand(Uni.createFrom().item(context), command) + .await() + .atMost(Duration.ofSeconds(5))) + .isSameAs(failure); + verify(context).close(); + } + + @Test + void closesContextWhenFeatureConfigurationIsInvalid() throws Exception { + var context = commandContext(true); + when(context.apiFeatures()) + .thenReturn(ApiFeatures.fromConfigAndRequest(Map.of(ApiFeature.MCP, "invalid"), null)); + + assertThatThrownBy( + () -> + mcpResource() + .processCommand(Uni.createFrom().item(context), mock(KeyspaceCommand.class)) + .await() + .atMost(Duration.ofSeconds(5))) + .isInstanceOf(IllegalArgumentException.class); + verifyNoInteractions(commandProcessor); + verify(context).close(); + } + @Test void closesContextWhenFeatureDisabled() throws Exception { var context = commandContext(false); From b5a76f3e4df11718b1e8f66d1847274f5dc884c3 Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Thu, 22 Oct 2026 10:31:36 -0700 Subject: [PATCH 5/7] Reuse the MCP HTTP result assertions --- .../McpReadableResultsIntegrationTest.java | 50 ++++--------------- 1 file changed, 10 insertions(+), 40 deletions(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java index a9747525a3..7ea44fdefe 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java @@ -1,6 +1,10 @@ package io.stargate.sgv2.jsonapi.api.v1.mcp; import static io.restassured.RestAssured.given; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.completeResult; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.readableResult; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.request; +import static io.stargate.sgv2.jsonapi.api.v1.mcp.McpHttpTestSupport.toolPayload; import static org.assertj.core.api.Assertions.assertThat; import io.quarkus.test.junit.QuarkusIntegrationTest; @@ -18,7 +22,6 @@ @QuarkusIntegrationTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) class McpReadableResultsIntegrationTest { - private static final String PROTOCOL_VERSION = "2026-07-28"; private static final String TOOL_NAME = CommandName.Names.FIND_RERANKING_PROVIDERS; @Test @@ -38,10 +41,11 @@ void successfulToolTextMatchesTheHttpResponse() { .statusCode(200) .extract() .asString(); + var expected = new JsonObject(httpResponse); - assertThat(readableResult(result)).isEqualTo(new JsonObject(httpResponse)); + assertThat(readableResult(result)).isEqualTo(expected); assertThat(result.getJsonObject("_meta").getJsonObject("status")) - .isEqualTo(new JsonObject(httpResponse).getJsonObject("status")); + .isEqualTo(expected.getJsonObject("status")); } @Test @@ -54,50 +58,16 @@ void failedToolReturnsReadableErrorsInACompleteResult() { assertThat(readableResult(result)).isEqualTo(result.getJsonObject("structuredContent")); } - private static JsonObject readableResult(JsonObject result) { - var content = result.getJsonArray("content"); - assertThat(content).hasSize(1); - assertThat(content.getJsonObject(0).getString("type")).isEqualTo("text"); - return new JsonObject(content.getJsonObject(0).getString("text")); - } - private JsonObject callTool(boolean mcpEnabled) { var response = - given() - .contentType(ContentType.JSON) - .accept("application/json, text/event-stream") + request("tools/call") // Existing token fixture. OAuth admission is tracked separately in #2642. .header("Token", "readable-result-test-credential") .header(ApiFeature.MCP.httpHeaderName(), mcpEnabled) .header(ApiFeature.RERANKING.httpHeaderName(), "true") - .header("MCP-Protocol-Version", PROTOCOL_VERSION) - .header("Mcp-Method", "tools/call") .header("Mcp-Name", TOOL_NAME) - .body( - Map.of( - "jsonrpc", - "2.0", - "id", - 1, - "method", - "tools/call", - "params", - Map.of( - "name", TOOL_NAME, - "arguments", Map.of(), - "_meta", - Map.of( - "io.modelcontextprotocol/protocolVersion", - PROTOCOL_VERSION, - "io.modelcontextprotocol/clientCapabilities", - Map.of())))) + .body(toolPayload(1, TOOL_NAME, Map.of())) .post("/v1/mcp"); - assertThat(response.statusCode()).as(response.asString()).isEqualTo(200); - var body = new JsonObject(response.asString()); - assertThat(body.getInteger("id")).isEqualTo(1); - assertThat(body.containsKey("error")).isFalse(); - var result = body.getJsonObject("result"); - assertThat(result.getString("resultType")).isEqualTo("complete"); - return result; + return completeResult(response, 1); } } From f8b0f28d74fd1501227f235a76a469731cac378d Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Fri, 23 Oct 2026 11:46:17 -0700 Subject: [PATCH 6/7] Clarify MCP context cleanup and remove a redundant assertion --- .../stargate/sgv2/jsonapi/api/v1/mcp/McpResource.java | 10 ++++------ .../sgv2/jsonapi/api/v1/mcp/McpResourceTest.java | 2 -- 2 files changed, 4 insertions(+), 8 deletions(-) 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 85ed09ca45..cb66862762 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 @@ -211,14 +211,11 @@ public Uni processCollectionCommand( } /** - * Process a general or keyspace command and return the CommandResult directly. + * Process a command and map its result to an MCP tool response. * - *

The Quarkus MCP Server framework automatically encodes the {@link CommandResult} to JSON via - * its built-in encoder (Jackson), so no manual serialization is needed. - * - * @param contextUni The command context (from buildGeneralContext or buildKeyspaceContext) + * @param contextUni The command context * @param command The command to execute - * @return Uni of CommandResult + * @return Uni of ToolResponse */ public Uni processCommand(Uni> contextUni, Command command) { @@ -228,6 +225,7 @@ public Uni processCommand(Uni> contextUni, Comma } private Uni executeCommand(CommandContext context, Command command) { + // Defer the feature check and processor call so synchronous failures still close the context. return Uni.createFrom() .deferred( () -> { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java index bf5508e958..f4d603f45e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpResourceTest.java @@ -211,8 +211,6 @@ void keyspaceContextUsesOneRequestContext() { // The schema lookup and the built command context must see the same request, so the // call has one request ID from start to finish. assertThat(context.requestContext()).isSameAs(requestContextCaptor.getValue()); - assertThat(context.requestContext().requestId()) - .isEqualTo(requestContextCaptor.getValue().requestId()); } @SuppressWarnings("unchecked") From 4568b124e2f176240cd3a9a849f63552659efc2c Mon Sep 17 00:00:00 2001 From: Eric Hare Date: Fri, 23 Oct 2026 11:48:23 -0700 Subject: [PATCH 7/7] Remove an unrelated comment from MCP result coverage --- .../jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java index 7ea44fdefe..b3913a5f91 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/mcp/McpReadableResultsIntegrationTest.java @@ -61,7 +61,6 @@ void failedToolReturnsReadableErrorsInACompleteResult() { private JsonObject callTool(boolean mcpEnabled) { var response = request("tools/call") - // Existing token fixture. OAuth admission is tracked separately in #2642. .header("Token", "readable-result-test-credential") .header(ApiFeature.MCP.httpHeaderName(), mcpEnabled) .header(ApiFeature.RERANKING.httpHeaderName(), "true")