Skip to content

Commit 8f50f86

Browse files
devalibmdevalibm
committed
Fixes async schema genereation bug
1 parent b415fb1 commit 8f50f86

File tree

6 files changed

+114
-148
lines changed

6 files changed

+114
-148
lines changed

‎dev/io.openliberty.mcp.internal/bnd.bnd‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ Bundle-Localization: \
5050

5151

5252
-testpath: \
53-
org.hamcrest:hamcrest-all;version='1.3',\
53+
org.hamcrest:hamcrest-all;version=1.3,\
5454
../build.sharedResources/lib/junit/old/junit.jar;version=file,\
5555
io.openliberty.jakarta.jsonb.3.0;version=latest,\
5656
io.openliberty.org.eclipse.yasson.3.0;version=latest,\

‎dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/ToolMetadata.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
127127
// Unwrap ToolResponse<T> -> T
128128
Type inner = pt.getActualTypeArguments()[0];
129129
unwrappedOutputType = unwrapOutputType(inner);
130-
// hasOutputSchema = true;
131130
} else {
132131
// if it's not a String/Content, we want schema
133132
hasOutputSchema = true;
@@ -180,8 +179,10 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
180179
}
181180

182181
/**
183-
* @param outputType
184-
* @return
182+
* Unwraps the given type if it is wrapped in CompletionStage or ToolResponse.
183+
*
184+
* @param type the type to unwrap
185+
* @return the innermost unwrapped type
185186
*/
186187
private static Type unwrapOutputType(Type type) {
187188
while (type instanceof ParameterizedType pt) {
@@ -285,7 +286,10 @@ public static String getToolQualifiedName(Bean<?> bean, AnnotatedMethod<?> metho
285286
}
286287

287288
/**
288-
* Helper method
289+
* Resolves the type inside CompletionStage if applicable.
290+
*
291+
* @param returnType the return type to resolve
292+
* @return the inner type or the original type if not wrapped
289293
*/
290294
private static Type resolveOutputType(Type returnType) {
291295
if (returnType instanceof ParameterizedType pt) {

‎dev/io.openliberty.mcp.internal/test/io/openliberty/mcp/internal/test/AsyncToolSchemaTest.java‎

Lines changed: 0 additions & 71 deletions
This file was deleted.

‎dev/io.openliberty.mcp.internal_fat/bnd.bnd‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,4 @@ tested.features: \
3636
io.openliberty.jakarta.jsonb.3.0;version=latest,\
3737
io.openliberty.jakarta.jsonp.2.1;version=latest,\
3838
io.openliberty.jakarta.annotation.2.1,\
39-
io.openliberty.jakarta.concurrency.3.0,\
40-
io.openliberty.jakarta.jsonb.2.0,\
41-
jakarta.json.bind-api
39+
io.openliberty.jakarta.concurrency.3.0

‎dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/tool/AsyncToolsTest.java‎

Lines changed: 104 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,23 @@
1010
package io.openliberty.mcp.internal.fat.tool;
1111

1212
import static com.ibm.websphere.simplicity.ShrinkHelper.DeployOptions.SERVER_ONLY;
13+
import static org.junit.Assert.assertFalse;
1314
import static org.junit.Assert.assertNotNull;
1415

1516
import java.util.List;
16-
import java.util.Map;
1717

1818
import org.jboss.shrinkwrap.api.ShrinkWrap;
1919
import org.jboss.shrinkwrap.api.spec.WebArchive;
20+
import org.json.JSONArray;
21+
import org.json.JSONException;
22+
import org.json.JSONObject;
2023
import org.junit.AfterClass;
2124
import org.junit.BeforeClass;
2225
import org.junit.Rule;
2326
import org.junit.Test;
2427
import org.junit.runner.RunWith;
2528
import org.skyscreamer.jsonassert.JSONAssert;
29+
import org.skyscreamer.jsonassert.JSONParser;
2630

2731
import com.ibm.websphere.simplicity.ShrinkHelper;
2832

@@ -36,9 +40,6 @@
3640
import io.openliberty.mcp.internal.fat.utils.McpClient;
3741
import io.openliberty.mcp.internal.fat.utils.ToolStatus;
3842
import io.openliberty.mcp.internal.fat.utils.ToolStatusClient;
39-
import jakarta.enterprise.util.TypeLiteral;
40-
import jakarta.json.bind.Jsonb;
41-
import jakarta.json.bind.JsonbBuilder;
4243

4344
@RunWith(FATRunner.class)
4445
public class AsyncToolsTest extends FATServletClient {
@@ -307,7 +308,6 @@ public void testCompletionStageThatNeverCompletes() throws Exception {
307308
client.callMCP(request);
308309
}
309310

310-
@SuppressWarnings({ "unchecked", "serial" })
311311
@Test
312312
public void testAsyncObjectToolHasExpectedOutputSchema() throws Exception {
313313
String response = client.callMCP("""
@@ -318,69 +318,120 @@ public void testAsyncObjectToolHasExpectedOutputSchema() throws Exception {
318318
}
319319
""");
320320

321-
Jsonb jsonb = JsonbBuilder.create();
322-
Map<String, Object> parsed = jsonb.fromJson(response, new TypeLiteral<Map<String, Object>>() {}.getType());
323-
Map<String, Object> result = (Map<String, Object>) parsed.get("result");
324-
List<Map<String, Object>> tools = (List<Map<String, Object>>) result.get("tools");
321+
JSONObject root = (JSONObject) JSONParser.parseJSON(response);
322+
JSONArray tools = root.getJSONObject("result").getJSONArray("tools");
325323

326-
Map<String, Object> asyncObjectTool = null;
324+
JSONObject asyncObjectTool = null;
327325

328-
for (Map<String, Object> tool : tools) {
329-
if ("asyncObjectTool".equals(tool.get("name"))) {
326+
for (int i = 0; i < tools.length(); i++) {
327+
JSONObject tool = tools.getJSONObject(i);
328+
if ("asyncObjectTool".equals(tool.getString("name"))) {
330329
asyncObjectTool = tool;
331330
break;
332331
}
333332
}
334333

335334
assertNotNull("Tool 'asyncObjectTool' should be present in tool list", asyncObjectTool);
336335

337-
String actualToolJson = jsonb.toJson(asyncObjectTool);
336+
String actualToolJson = asyncObjectTool.toString();
338337

339338
String expectedToolJson = """
340-
{
341-
"name": "asyncObjectTool",
342-
"title": "Async Object Tool",
343-
"description": "Returns a city object",
344-
"inputSchema": {
345-
"type": "object",
346-
"properties": {
347-
"name": {
348-
"type": "string",
349-
"description": "City name to fetch"
350-
}
351-
},
352-
"required": ["name"]
353-
},
354-
"outputSchema": {
355-
"type": "object",
356-
"properties": {
357-
"country": { "type": "string" },
358-
"isCapital": { "type": "boolean" },
359-
"name": { "type": "string" },
360-
"population": { "type": "integer" }
361-
},
362-
"required": ["country", "isCapital", "name", "population"]
339+
{
340+
"name": "asyncObjectTool",
341+
"title": "Async asyncObjectTool",
342+
"description": "A tool to return an object of cities asynchronously",
343+
"inputSchema": {
344+
"type": "object",
345+
"properties": {
346+
"name": {
347+
"type": "string",
348+
"description": "name of your city"
363349
}
364-
}
365-
""";
350+
},
351+
"required": ["name"]
352+
},
353+
"outputSchema": {
354+
"type": "object",
355+
"properties": {
356+
"country": {"type": "string"},
357+
"isCapital": {"type": "boolean"},
358+
"name": {"type": "string"},
359+
"population": {"type": "integer"}
360+
},
361+
"required": ["name", "country", "population", "isCapital"]
362+
}
363+
}
364+
""";
366365

367366
JSONAssert.assertEquals(expectedToolJson, actualToolJson, true);
368367
}
369368

370-
// @SuppressWarnings("unchecked")
371-
// private List<Map<String, Object>> fetchToolMetadata() throws Exception {
372-
// String response = client.callMCP("""
373-
// {
374-
// "jsonrpc": "2.0",
375-
// "id": 1,
376-
// "method": "tools/list"
377-
// }
378-
// """);
379-
//
380-
// Jsonb jsonb = JsonbBuilder.create();
381-
// Map<String, Object> parsed = jsonb.fromJson(response, new TypeLiteral<Map<String, Object>>() {}.getType());
382-
// Map<String, Object> result = (Map<String, Object>) parsed.get("result");
383-
// return (List<Map<String, Object>>) result.get("tools");
384-
// }
369+
@Test
370+
public void testAsyncToolsWithoutStructuredContentHaveNoOutputSchema() throws Exception {
371+
String response = client.callMCP("""
372+
{
373+
"jsonrpc": "2.0",
374+
"id": 1,
375+
"method": "tools/list"
376+
}
377+
""");
378+
379+
JSONObject root = (JSONObject) JSONParser.parseJSON(response);
380+
JSONArray tools = root.getJSONObject("result").getJSONArray("tools");
381+
382+
List<String> toolNames = List.of(
383+
"asyncEcho",
384+
"asyncDelayedEcho",
385+
"asyncToolThatNeverCompletes",
386+
"asyncCancellationTool");
387+
388+
for (String toolName : toolNames) {
389+
JSONObject tool = findToolByName(tools, toolName);
390+
assertNotNull("Tool " + toolName + " should be present", tool);
391+
assertFalse("Tool " + toolName + " should NOT have an output schema", tool.has("outputSchema"));
392+
}
393+
}
394+
395+
private JSONObject findToolByName(JSONArray tools, String name) throws JSONException {
396+
for (int i = 0; i < tools.length(); i++) {
397+
JSONObject tool = tools.getJSONObject(i);
398+
if (name.equals(tool.getString("name"))) {
399+
return tool;
400+
}
401+
}
402+
return null;
403+
}
404+
405+
@Test
406+
public void testAsyncToolsWithBasicOutputTypesShouldNotHaveOutputSchema() throws Exception {
407+
String response = client.callMCP("""
408+
{
409+
"jsonrpc": "2.0",
410+
"id": 1,
411+
"method": "tools/list"
412+
}
413+
""");
414+
415+
JSONObject root = (JSONObject) JSONParser.parseJSON(response);
416+
JSONArray tools = root.getJSONObject("result").getJSONArray("tools");
417+
418+
// Tools that SHOULD NOT have an output schema
419+
List<String> toolsWithoutSchema = List.of(
420+
"asyncEcho",
421+
"asyncDelayedEcho",
422+
"asyncCancellationTool",
423+
"asyncToolThatNeverCompletes");
424+
425+
for (int i = 0; i < tools.length(); i++) {
426+
JSONObject tool = tools.getJSONObject(i);
427+
String name = tool.getString("name");
428+
429+
if (toolsWithoutSchema.contains(name)) {
430+
assertFalse(
431+
"Tool '" + name + "' should NOT have an output schema",
432+
tool.has("outputSchema"));
433+
}
434+
}
435+
}
385436

386437
}

‎dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/tool/basicToolApp/BasicTools.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -439,22 +439,6 @@ public boolean checkPerson(@ToolArg(name = "person", description = "Person objec
439439
return true;
440440
}
441441

442-
// @Tool(name = "addPersonToList", title = "adds person to people list", description = "adds person to people list", structuredContent = true)
443-
// public @Schema(description = "Returns list of person object") List<Person> addPersonToList(@ToolArg(name = "employeeList",
444-
// description = "List of people") List<Person> employeeList,
445-
// @ToolArg(name = "person", description = "Person object") Optional<Person> person) {
446-
// employeeList.add(person.get());
447-
// return employeeList;
448-
// }
449-
450-
// @Tool(name = "addPersonToList", title = "adds person to people list", description = "adds person to people list", structuredContent = true)
451-
// public @Schema(description = "Returns list of person object") List<Person> addPersonToList(@ToolArg(name = "employeeList",
452-
// description = "List of people") List<Person> employeeList,
453-
// @ToolArg(name = "person", description = "Person object") Optional<Person> person) {
454-
// employeeList.add(person.get());
455-
// return employeeList;
456-
// }
457-
458442
@Tool(name = "addPersonToList", title = "adds person to people list", description = "adds person to people list", structuredContent = true)
459443
@Schema(description = "Returns list of person object")
460444
public List<Person> addPersonToList(

0 commit comments

Comments
 (0)