Merge pull request #32850 from ttmunyunguma/feature-mcp-server-32827

ttmunyunguma · web-flow · commit eca8578939f3 · 2025-09-24T09:22:35.000+01:00
Report an error if ToolArg.name is not set #32827
diff --git a/dev/io.openliberty.jakarta/build.gradle b/dev/io.openliberty.jakarta/build.gradle
@@ -0,0 +1,16 @@
+/*******************************************************************************
+ * Copyright (c) 2025 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-2.0/
+ *******************************************************************************/
+configurations {
+  cdi40 {
+    canBeResolved = true
+  }
+}
+
+artifacts {
+  cdi40(file('build/libs/io.openliberty.jakarta.cdi.4.0.jar'))
+}
diff --git a/dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/McpCdiExtension.java b/dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/McpCdiExtension.java
@@ -63,8 +63,10 @@ void afterDeploymentValidation(@Observes AfterDeploymentValidation afterDeployme
     private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentValidation) {
         StringBuilder sbBlankArgs = new StringBuilder("Blank arguments found in MCP Tool:");
         StringBuilder sbDuplicateArgs = new StringBuilder("Duplicate arguments found in MCP Tool:");
+        StringBuilder sbMissingArgs = new StringBuilder("Missing arguments found in MCP Tool:");
         boolean blankArgumentsFound = false;
         boolean duplicateArgumentsFound = false;
+        boolean missingArgumentName = false;
 
         for (ToolMetadata tool : tools.getAllTools()) {
             Map<String, ArgumentMetadata> arguments = tool.arguments();
@@ -76,6 +78,10 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV
                 } else if (arguments.get(argName).isDuplicate()) {
                     sbDuplicateArgs.append("\n").append("Tool: " + tool.getToolQualifiedName() + " -  Argument: " + argName);
                     duplicateArgumentsFound = true;
+                } else if (argName.equals(ToolMetadata.MISSING_TOOL_ARG_NAME)) {
+                    sbMissingArgs.append("\n").append("Tool: " + tool.getToolQualifiedName());
+                    sbMissingArgs.append("\n Tool argument name was not provided for the parameter. Either add a name to the @ToolArg annotation, or to add the -parameters compiler option to use the parameter name");
+                    missingArgumentName = true;
                 }
             }
         }
@@ -85,6 +91,9 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV
         if (duplicateArgumentsFound) {
             afterDeploymentValidation.addDeploymentProblem(new Exception(sbDuplicateArgs.toString()));
         }
+        if (missingArgumentName) {
+            afterDeploymentValidation.addDeploymentProblem(new Exception(sbMissingArgs.toString()));
+        }
     }
 
     private void reportOnDuplicateTools(AfterDeploymentValidation afterDeploymentValidation) {
diff --git a/dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/ToolMetadata.java b/dev/io.openliberty.mcp.internal/src/io/openliberty/mcp/internal/ToolMetadata.java
@@ -29,6 +29,8 @@ public record ToolMetadata(Tool annotation, Bean<?> bean, AnnotatedMethod<?> met
                            String name, String title, String description,
                            List<Class<? extends Throwable>> businessExceptions) {
 
+    public static final String MISSING_TOOL_ARG_NAME = "<<<MISSING TOOL_ARG NAME>>>";
+
     public record ArgumentMetadata(Type type, int index, String description, boolean required, boolean isDuplicate) {}
 
     public record SpecialArgumentMetadata(SpecialArgumentType.Resolution typeResolution, int index) {}
@@ -51,17 +53,42 @@ public static ToolMetadata createFrom(Tool annotation, Bean<?> bean, AnnotatedMe
 
     private static Map<String, ArgumentMetadata> getArgumentMap(AnnotatedMethod<?> method) {
         Map<String, ArgumentMetadata> result = new HashMap<>();
-        for (AnnotatedParameter<?> p : method.getParameters()) {
-            ToolArg pInfo = p.getAnnotation(ToolArg.class);
-            if (pInfo != null) {
-                String toolArgName = (pInfo.name().equals(ToolArg.ELEMENT_NAME)) ? p.getJavaParameter().getName() : pInfo.name(); // p.getJavaParameter().getName() needs java compiler -parameter flag to work
-                boolean isDuplicateArg = result.containsKey(toolArgName);
-                result.put(toolArgName, new ArgumentMetadata(p.getBaseType(), p.getPosition(), pInfo.description(), pInfo.required(), isDuplicateArg));
+
+        for (AnnotatedParameter<?> param : method.getParameters()) {
+
+            ToolArg argAnnotation = param.getAnnotation(ToolArg.class);
+
+            if (argAnnotation == null) {
+                continue;
             }
+
+            String argName = resolveArgumentName(param, argAnnotation);
+            boolean isDuplicateArg = result.containsKey(argName);
+
+            result.put(argName, new ArgumentMetadata(param.getBaseType(),
+                                                     param.getPosition(),
+                                                     argAnnotation.description(),
+                                                     argAnnotation.required(),
+                                                     isDuplicateArg));
         }
         return result.isEmpty() ? Collections.emptyMap() : result;
     }
 
+    private static String resolveArgumentName(AnnotatedParameter<?> param, ToolArg argAnnotation) {
+        String argAnnotationName = argAnnotation.name();
+
+        if (!argAnnotationName.equals(ToolArg.ELEMENT_NAME)) {
+            return argAnnotationName;
+        }
+
+        if (param.getJavaParameter().isNamePresent()) {
+            // needs java compiler -parameter flag to work
+            return param.getJavaParameter().getName();
+        }
+
+        return MISSING_TOOL_ARG_NAME;
+    }
+
     private static List<SpecialArgumentMetadata> getSpecialArgumentList(AnnotatedMethod<?> method) {
         List<SpecialArgumentMetadata> result = new ArrayList<>();
         for (AnnotatedParameter<?> p : method.getParameters()) {
diff --git a/dev/io.openliberty.mcp.internal_fat/.classpath b/dev/io.openliberty.mcp.internal_fat/.classpath
@@ -5,6 +5,11 @@
 			<attribute name="test" value="true"/>
 		</attributes>
 	</classpathentry>
+	<classpathentry kind="src" path="noParamNames">
+		<attributes>
+			<attribute name="test" value="true"/>
+		</attributes>
+	</classpathentry>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17">
 		<attributes>
 			<attribute name="module" value="true"/>
diff --git a/dev/io.openliberty.mcp.internal_fat/build.gradle b/dev/io.openliberty.mcp.internal_fat/build.gradle
@@ -15,12 +15,38 @@ apply from: '../wlp-gradle/subprojects/repos.gradle'
 configurations.requiredLibs {
   transitive = false
 }
+
+sourceSets {
+  // Source set where we don't include parameter names
+  noParamNames {
+    java {
+      srcDirs = ['noParamNames']
+    }
+  }
+}
+
 dependencies {
   requiredLibs 'org.json:json:20080701',
                'org.skyscreamer:jsonassert:2.0-rc1'
+  noParamNamesCompileOnly project(':io.openliberty.mcp'),
+                              project(path: ':io.openliberty.jakarta', configuration: 'cdi40')
+  implementation sourceSets.noParamNames.output
 }
+
 /* Added to allow method parmaters to be available for MCP Tool Server*/ 
 // Allow us to test getting the name of a tool argument directly from the Java 
 compileJava {
-    options.compilerArgs << '-parameters'
-}
+  options.compilerArgs << '-parameters'
+}
+
+// Build the noParamNames code into a jar
+tasks.register('noParamNamesJar', Jar) {
+  dependsOn noParamNamesClasses
+  from sourceSets.noParamNames.output
+  archiveFileName = 'noParamNames.jar'
+}
+
+// Build the noParamNames jar before the autoFVT zip so that it gets included in the zip
+tasks.named('autoFVT') {
+  dependsOn noParamNamesJar
+}
diff --git a/dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/FATSuite.java b/dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/FATSuite.java
@@ -25,6 +25,7 @@
 import io.openliberty.mcp.internal.fat.protocol.ProtocolVersionTest;
 import io.openliberty.mcp.internal.fat.tool.CancellationTest;
 import io.openliberty.mcp.internal.fat.tool.DeploymentProblemTest;
+import io.openliberty.mcp.internal.fat.tool.NoParamNameTest;
 import io.openliberty.mcp.internal.fat.tool.ToolErrorHandlingTest;
 import io.openliberty.mcp.internal.fat.tool.ToolTest;
 
@@ -38,6 +39,7 @@
                 CancellationTest.class,
                 HttpTest.class,
                 LifecycleTest.class,
+                NoParamNameTest.class,
                 ProtocolVersionTest.class,
                 ToolErrorHandlingTest.class,
                 ToolTest.class
diff --git a/dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/tool/NoParamNameTest.java b/dev/io.openliberty.mcp.internal_fat/fat/src/io/openliberty/mcp/internal/fat/tool/NoParamNameTest.java
@@ -0,0 +1,46 @@
+/*******************************************************************************
+ * Copyright (c) 2025 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package io.openliberty.mcp.internal.fat.tool;
+
+import java.util.List;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import componenttest.annotation.Server;
+import componenttest.custom.junit.runner.FATRunner;
+import componenttest.topology.impl.LibertyServer;
+import componenttest.topology.utils.FATServletClient;
+import io.openliberty.mcp.internal.fat.noparamtool.NoParamTools;
+
+@RunWith(FATRunner.class)
+public class NoParamNameTest extends FATServletClient {
+    @Server("mcp-server")
+    public static LibertyServer server;
+
+    @BeforeClass
+    public static void setup() throws Exception {
+        ExpectedAppFailureValidator.deployAppToAssertFailure(server, "ExpectedNoParamNameFailureTest", NoParamTools.class.getPackage());
+    }
+
+    @AfterClass
+    public static void teardown() throws Exception {
+        server.stopServer(ExpectedAppFailureValidator.APP_START_FAILED_CODE);
+    }
+
+    @Test
+    public void testNoParamNameToolArg() throws Exception {
+        String expectedErrorHeader = "Missing arguments found in MCP Tool:";
+        List<String> expectedErrorList = List.of("io.openliberty.mcp.internal.fat.noparamtool.NoParamTools.missingToolArgNameTool");
+        ExpectedAppFailureValidator.findAndAssertExpectedErrorsInLogs("Missing arguments found in MCP Tool: ", expectedErrorHeader, expectedErrorList, server);
+    }
+}
diff --git a/dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/NoParamTools.java b/dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/NoParamTools.java
@@ -0,0 +1,23 @@
+/*******************************************************************************
+ * Copyright (c) 2025 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package io.openliberty.mcp.internal.fat.noparamtool;
+
+import io.openliberty.mcp.annotations.Tool;
+import io.openliberty.mcp.annotations.ToolArg;
+import jakarta.enterprise.context.ApplicationScoped;
+
+@ApplicationScoped
+public class NoParamTools {
+
+    @Tool(name = "missingToolArgNameTool", title = "missing ToolArgName Tool", description = "ToolArgName is missing so app wont start")
+    public String missingToolArgNameTool(@ToolArg(description = "input to echo") String input) {
+        return input;
+    }
+}
diff --git a/dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/README.md b/dev/io.openliberty.mcp.internal_fat/noParamNames/io/openliberty/mcp/internal/fat/noparamtool/README.md
@@ -0,0 +1,4 @@
+# No Param Names
+
+This source set contains a basic tool called *missingToolArgNameTool*. The reason the tool has been separated out of the the rest of the application is because we need this to be compiled without the -parameters flag. This allows us to test that if parameter names are not included, then we report an error.
+ 

Original file line number	Diff line number	Diff line change
`@@ -63,8 +63,10 @@ void afterDeploymentValidation(@Observes AfterDeploymentValidation afterDeployme`
`63`	`63`	`private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentValidation) {`
`64`	`64`	`StringBuilder sbBlankArgs = new StringBuilder("Blank arguments found in MCP Tool:");`
`65`	`65`	`StringBuilder sbDuplicateArgs = new StringBuilder("Duplicate arguments found in MCP Tool:");`
	`66`	`+ StringBuilder sbMissingArgs = new StringBuilder("Missing arguments found in MCP Tool:");`
`66`	`67`	`boolean blankArgumentsFound = false;`
`67`	`68`	`boolean duplicateArgumentsFound = false;`
	`69`	`+ boolean missingArgumentName = false;`
`68`	`70`
`69`	`71`	`for (ToolMetadata tool : tools.getAllTools()) {`
`70`	`72`	`Map<String, ArgumentMetadata> arguments = tool.arguments();`
`@@ -76,6 +78,10 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV`
`76`	`78`	`} else if (arguments.get(argName).isDuplicate()) {`
`77`	`79`	`sbDuplicateArgs.append("\n").append("Tool: " + tool.getToolQualifiedName() + " - Argument: " + argName);`
`78`	`80`	`duplicateArgumentsFound = true;`
	`81`	`+ } else if (argName.equals(ToolMetadata.MISSING_TOOL_ARG_NAME)) {`
	`82`	`+ sbMissingArgs.append("\n").append("Tool: " + tool.getToolQualifiedName());`
	`83`	`+ sbMissingArgs.append("\n Tool argument name was not provided for the parameter. Either add a name to the @ToolArg annotation, or to add the -parameters compiler option to use the parameter name");`
	`84`	`+ missingArgumentName = true;`
`79`	`85`	`}`
`80`	`86`	`}`
`81`	`87`	`}`
`@@ -85,6 +91,9 @@ private void reportOnToolArgEdgeCases(AfterDeploymentValidation afterDeploymentV`
`85`	`91`	`if (duplicateArgumentsFound) {`
`86`	`92`	`afterDeploymentValidation.addDeploymentProblem(new Exception(sbDuplicateArgs.toString()));`
`87`	`93`	`}`
	`94`	`+ if (missingArgumentName) {`
	`95`	`+ afterDeploymentValidation.addDeploymentProblem(new Exception(sbMissingArgs.toString()));`
	`96`	`+ }`
`88`	`97`	`}`
`89`	`98`
`90`	`99`	`private void reportOnDuplicateTools(AfterDeploymentValidation afterDeploymentValidation) {`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +# No Param Names
++
 +This source set contains a basic tool called *missingToolArgNameTool*. The reason the tool has been separated out of the the rest of the application is because we need this to be compiled without the -parameters flag. This allows us to test that if parameter names are not included, then we report an error.
++