jamesnetherton commented on code in PR #9341:
URL: https://github.com/apache/camel-quarkus/pull/9341#discussion_r4294788906
##########
extensions/mcp-server/runtime/src/main/doc/usage.adoc:
##########
@@ -33,6 +33,18 @@ the JSON type (`string`, `integer`, `number`, `boolean`),
and the
`parameter.NAME.enum` options refine the generated input schema. Arguments
arrive as message headers in the route (`${header.customerId}`).
+=== Structured tool output
+
+Tools declaring their output with the `outputSchema` or `outputParameter.NAME`
+options publish it as the MCP `outputSchema`. The route body must then be
Review Comment:
Nit: "The route body must then be JSON" could be "must then be a JSON
object", since MCP expects `structuredContent` to be an object.
##########
extensions/mcp-server/runtime/src/main/java/org/apache/camel/quarkus/component/mcp/server/QuarkusMcpServerEngine.java:
##########
@@ -80,12 +81,17 @@ public void toolAdded(McpServerTool tool) {
if (tool.inputSchemaJson() != null) {
definition.setInputSchema(new JsonObject(tool.inputSchemaJson()));
}
+ if (tool.outputSchemaJson() != null) {
+ definition.setOutputSchema(new
JsonObject(tool.outputSchemaJson()));
+ }
applyAnnotations(definition, tool.annotations());
definition.setHandler(arguments -> {
Map<String, Object> args = arguments.args() != null ?
arguments.args() : Map.of();
McpToolCallResult result = tool.handler().call(args);
+ // the text content is kept next to structuredContent for clients
without structured output support
return result.isError()
- ? ToolResponse.error(result.text()) :
ToolResponse.success(new TextContent(result.text()));
+ ? ToolResponse.error(result.text())
+ : new ToolResponse(false, List.of(new
TextContent(result.text())), result.structuredContent(), null);
Review Comment:
Question: `result.structuredContent()` is passed through as-is. As far as I
can tell, Camel's `AiToolParameterHelper.parseStructuredOutput` can also return
a `List`, `Number` or `Boolean` (for example, a route body of `[1,2]` or `42`).
The MCP spec requires `structuredContent` to be a JSON object, so a strict
client could reject that response. Should we guard here, passing through only
`Map`/`JsonObject` and returning `ToolResponse.error(...)` otherwise? Or should
Camel enforce this instead?
##########
integration-tests/mcp-server/src/test/java/org/apache/camel/quarkus/component/mcp/server/it/McpServerTest.java:
##########
@@ -115,6 +117,54 @@ void testArgSchemaToolExecutesWithNestedArguments() {
.thenAssertResults();
}
+ @Test
+ void testOutputSchemaIsPublished() {
+ client().when()
+ .toolsList(page -> {
+ // outputSchema resource, published as-is with its nested
structure
+ JsonObject orderStatus = toolByName(page,
"order_status").outputSchema();
+ assertThat(orderStatus).isNotNull();
+
assertThat(orderStatus.getJsonObject("properties").getJsonObject("items")
+
.getJsonObject("items").getString("type")).isEqualTo("string");
+
assertThat(orderStatus.getJsonArray("required")).containsExactly("orderId",
"status", "items");
+
+ // flat outputParameter.* options, published as the
generated schema
+ JsonObject weather = toolByName(page,
"get_weather").outputSchema();
+ assertThat(weather).isNotNull();
+
assertThat(weather.getJsonObject("properties").getJsonObject("temperature")
+ .getString("type")).isEqualTo("number");
+
assertThat(weather.getJsonObject("properties").getJsonObject("unit").getJsonArray("enum"))
+ .containsExactly("celsius", "fahrenheit");
+
assertThat(weather.getJsonArray("required")).containsExactly("temperature");
+
+ // tools without an output declaration are unchanged
+ assertThat(toolByName(page,
"say_hello").outputSchema()).isNull();
+ })
+ .thenAssertResults();
+ }
+
+ @Test
+ void testStructuredContentIsReturned() {
Review Comment:
Nit: could we add a case for the error path? For example, a tool with an
output schema whose route returns a body that isn't valid JSON, asserting that
`isError()` is true and `structuredContent()` is null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
