This is an automated email from the ASF dual-hosted git repository. robertlazarski pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/axis-axis2-java-core.git
commit a02a94aac9db8c84baefaca327fc8010eeb67d6b Author: Robert Lazarski <[email protected]> AuthorDate: Tue Apr 7 02:09:14 2026 -1000 MCP catalog: fix InOnly test semantics, add description/annotation tuning Fix failing test: testInOnlyMalformedJsonBodyReturnsBadRequestWithCorrelationId was asserting that "Bad Request" appears in the HTTP response body for InOnly (fire-and- forget) operations. InOnly MEP swallows the AxisFault at the Axis2 dispatch layer — the response is always empty (same as success). Rename to testInOnlyMalformedJsonBodyDoesNotLeakExceptionDetails; assert only that no exception class name or stack trace appears in whatever body is returned. Phase A1 — natural language tool descriptions (mcpDescription parameter): getMcpStringParam() reads "mcpDescription" from AxisOperation first, then AxisService, then falls back to "ServiceName: operationName". Set in services.xml at <operation> or <service> level. 9 new tests covering operation-level override, service-level default, precedence, and fallback. Phase A2 — per-service annotation tuning (mcpReadOnly et al.): getMcpBoolParam() reads mcpReadOnly, mcpDestructive, mcpIdempotent, mcpOpenWorld from AxisOperation then AxisService; maps to MCP 2025 annotations readOnlyHint/destructiveHint/idempotentHint/openWorldHint. Conservative false defaults preserved when no parameters set. 7 new tests covering each hint and the default-preservation invariant. Update json-rpc-mcp-guide.xml test table for renamed InOnly test. Co-Authored-By: Claude Sonnet 4.6 <[email protected]> --- .../json/gson/rpc/JSONRPCIntegrationTest.java | 21 ++- .../apache/axis2/openapi/OpenApiSpecGenerator.java | 71 ++++++++- .../axis2/openapi/McpCatalogGeneratorTest.java | 158 +++++++++++++++++++++ src/site/xdoc/docs/json-rpc-mcp-guide.xml | 2 +- 4 files changed, 237 insertions(+), 15 deletions(-) diff --git a/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java b/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java index 514c62b327..0dc98bec23 100644 --- a/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java +++ b/modules/json/test/org/apache/axis2/json/gson/rpc/JSONRPCIntegrationTest.java @@ -134,17 +134,24 @@ public class JSONRPCIntegrationTest { } /** - * The InOnly receiver (fire-and-forget) must apply the same correlation ID - * pattern for malformed requests — no exception leak on that path either. + * InOnly (fire-and-forget) operations do not return fault messages to the client — + * the AxisFault is caught by the Axis2 InOnly dispatch and the HTTP response is + * empty (identical to a successful InOnly call). The errorRef UUID is logged + * server-side; clients have no observable fault body to inspect. + * + * What IS testable: nothing in the response (empty or otherwise) leaks a Java + * exception class name or stack trace. */ @Test - public void testInOnlyMalformedJsonBodyReturnsBadRequestWithCorrelationId() throws Exception { + public void testInOnlyMalformedJsonBodyDoesNotLeakExceptionDetails() throws Exception { String pingUrl = server.getEndpoint("JSONPOJOService") + "ping"; UtilTest.TestResponse result = UtilTest.postForResponse("NOT_VALID_JSON", pingUrl); String body = result.body; - Assert.assertTrue("InOnly malformed request must return 'Bad Request'", - body.contains("Bad Request")); - Assert.assertTrue("InOnly malformed request must contain an errorRef", - body.contains("errorRef=")); + Assert.assertFalse("InOnly error response must not leak 'MalformedJsonException'", + body.contains("MalformedJsonException")); + Assert.assertFalse("InOnly error response must not leak 'IOException'", + body.contains("IOException")); + Assert.assertFalse("InOnly error response must not leak stack trace", + body.contains("at org.apache")); } } diff --git a/modules/openapi/src/main/java/org/apache/axis2/openapi/OpenApiSpecGenerator.java b/modules/openapi/src/main/java/org/apache/axis2/openapi/OpenApiSpecGenerator.java index c147685c99..ffcab1d0bd 100644 --- a/modules/openapi/src/main/java/org/apache/axis2/openapi/OpenApiSpecGenerator.java +++ b/modules/openapi/src/main/java/org/apache/axis2/openapi/OpenApiSpecGenerator.java @@ -633,6 +633,47 @@ public class OpenApiSpecGenerator { * Uses the same service filtering as {@link #generatePaths()} so the tool * catalog is consistent with the OpenAPI spec. */ + /** + * Reads a string-valued MCP metadata parameter, checking the operation first + * then the service, falling back to {@code defaultValue}. + * + * <p>Callers set this in {@code services.xml}: + * <pre> + * <operation name="doFoo"> + * <parameter name="mcpDescription">Natural language description</parameter> + * </operation> + * </pre> + */ + private String getMcpStringParam(AxisOperation operation, AxisService service, + String paramName, String defaultValue) { + org.apache.axis2.description.Parameter p = operation.getParameter(paramName); + if (p == null) p = service.getParameter(paramName); + if (p != null && p.getValue() != null) { + String v = p.getValue().toString().trim(); + if (!v.isEmpty()) return v; + } + return defaultValue; + } + + /** + * Reads a boolean-valued MCP metadata parameter, checking the operation first + * then the service, falling back to {@code defaultValue}. + * + * <p>Accepts "true" / "false" (case-insensitive). Any other value is treated as + * {@code defaultValue}. + */ + private boolean getMcpBoolParam(AxisOperation operation, AxisService service, + String paramName, boolean defaultValue) { + org.apache.axis2.description.Parameter p = operation.getParameter(paramName); + if (p == null) p = service.getParameter(paramName); + if (p != null && p.getValue() != null) { + String v = p.getValue().toString().trim().toLowerCase(java.util.Locale.ROOT); + if ("true".equals(v)) return true; + if ("false".equals(v)) return false; + } + return defaultValue; + } + /** * Generates the MCP tool catalog JSON for the {@code /openapi-mcp.json} endpoint. * @@ -684,7 +725,17 @@ public class OpenApiSpecGenerator { com.fasterxml.jackson.databind.node.ObjectNode toolNode = toolsArray.addObject(); toolNode.put("name", opName); - toolNode.put("description", service.getName() + ": " + opName); + + // Description: prefer operation-level "mcpDescription" parameter, + // then service-level "mcpDescription", then auto-generated fallback. + // Set in services.xml: + // <operation name="doFoo"> + // <parameter name="mcpDescription">Human-readable tool description</parameter> + // </operation> + // or at service level for a default across all operations. + String description = getMcpStringParam(operation, service, "mcpDescription", + service.getName() + ": " + opName); + toolNode.put("description", description); // inputSchema: minimal MCP-compliant structure. Richer schemas are // produced when services carry @McpTool annotations (future work). @@ -706,14 +757,20 @@ public class OpenApiSpecGenerator { // Whether the caller must supply a Bearer token (from doLogin). toolNode.put("x-requiresAuth", requiresAuth); - // MCP 2025-03-26 tool annotations — conservative defaults. - // Override via @McpTool when richer metadata is available. + // MCP 2025-03-26 tool annotations. + // Tunable via services.xml parameters at operation or service level: + // mcpReadOnly → readOnlyHint (true for GET-equivalent operations) + // mcpDestructive → destructiveHint + // mcpIdempotent → idempotentHint (true for pure reads / PUT-equivalent) + // mcpOpenWorld → openWorldHint (true for operations with side effects + // outside the Axis2 service boundary) + // Conservative false defaults are preserved when parameters are absent. com.fasterxml.jackson.databind.node.ObjectNode annotations = toolNode.putObject("annotations"); - annotations.put("readOnlyHint", false); - annotations.put("destructiveHint", false); - annotations.put("idempotentHint", false); - annotations.put("openWorldHint", false); + annotations.put("readOnlyHint", getMcpBoolParam(operation, service, "mcpReadOnly", false)); + annotations.put("destructiveHint", getMcpBoolParam(operation, service, "mcpDestructive", false)); + annotations.put("idempotentHint", getMcpBoolParam(operation, service, "mcpIdempotent", false)); + annotations.put("openWorldHint", getMcpBoolParam(operation, service, "mcpOpenWorld", false)); } } diff --git a/modules/openapi/src/test/java/org/apache/axis2/openapi/McpCatalogGeneratorTest.java b/modules/openapi/src/test/java/org/apache/axis2/openapi/McpCatalogGeneratorTest.java index ec669d5799..9713d7b891 100644 --- a/modules/openapi/src/test/java/org/apache/axis2/openapi/McpCatalogGeneratorTest.java +++ b/modules/openapi/src/test/java/org/apache/axis2/openapi/McpCatalogGeneratorTest.java @@ -521,6 +521,164 @@ public class McpCatalogGeneratorTest extends TestCase { } } + // ── A1: mcpDescription parameter ───────────────────────────────────────── + + /** + * When an operation has a {@code mcpDescription} parameter, that value + * replaces the auto-generated "ServiceName: operationName" description. + * This is the primary way to make tool descriptions useful to LLMs. + */ + public void testOperationLevelMcpDescriptionOverridesDefault() throws Exception { + AxisService svc = new AxisService("GetAssetCalculationsService"); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("getAssetCalculations")); + op.addParameter(new org.apache.axis2.description.Parameter( + "mcpDescription", + "Get calculated portfolio metrics (OPS, PWR, Kelly) for assets in a fund.")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode tool = getCatalogTools().get(0); + assertEquals("Operation-level mcpDescription must be used as tool description", + "Get calculated portfolio metrics (OPS, PWR, Kelly) for assets in a fund.", + tool.path("description").asText()); + } + + /** + * When no operation-level parameter is set but the service has + * {@code mcpDescription}, that value is used as the description. + */ + public void testServiceLevelMcpDescriptionUsedWhenNoOperationLevel() throws Exception { + AxisService svc = new AxisService("SearchService"); + svc.addParameter(new org.apache.axis2.description.Parameter( + "mcpDescription", "Service-level default description")); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("search")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode tool = getCatalogTools().get(0); + assertEquals("Service-level mcpDescription must be used when no operation-level param", + "Service-level default description", + tool.path("description").asText()); + } + + /** + * Operation-level {@code mcpDescription} takes precedence over a service-level one. + */ + public void testOperationLevelMcpDescriptionTakesPrecedenceOverServiceLevel() throws Exception { + AxisService svc = new AxisService("SearchService"); + svc.addParameter(new org.apache.axis2.description.Parameter( + "mcpDescription", "Service-level description")); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("search")); + op.addParameter(new org.apache.axis2.description.Parameter( + "mcpDescription", "Operation-level description")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode tool = getCatalogTools().get(0); + assertEquals("Operation-level mcpDescription must win over service-level", + "Operation-level description", + tool.path("description").asText()); + } + + /** + * When no {@code mcpDescription} parameter is set at either level, the + * auto-generated "ServiceName: operationName" fallback is still produced. + */ + public void testDescriptionFallsBackToAutoGeneratedWhenNoMcpDescriptionParam() throws Exception { + addService("MyService", "myOperation"); + JsonNode tool = getCatalogTools().get(0); + assertEquals("Auto-generated fallback must be 'ServiceName: operationName'", + "MyService: myOperation", + tool.path("description").asText()); + } + + // ── A2: mcpReadOnly / mcpIdempotent annotation tuning ──────────────────── + + /** + * When a service sets {@code mcpReadOnly=true}, the catalog must publish + * {@code readOnlyHint: true} for all its operations. Read-only services + * (GetAsset*, Search*) should set this so MCP hosts can safely auto-approve + * them without human confirmation. + */ + public void testServiceLevelMcpReadOnlySetsTrueOnAnnotation() throws Exception { + AxisService svc = new AxisService("GetAssetCalculationsService"); + svc.addParameter(new org.apache.axis2.description.Parameter("mcpReadOnly", "true")); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("getAssetCalculations")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode annotations = getCatalogTools().get(0).path("annotations"); + assertTrue("readOnlyHint must be true when mcpReadOnly=true on service", + annotations.path("readOnlyHint").asBoolean()); + } + + /** + * Operation-level {@code mcpReadOnly=true} overrides a service-level + * {@code false} (or absent) — per-operation tuning takes precedence. + */ + public void testOperationLevelMcpReadOnlyOverridesServiceLevel() throws Exception { + AxisService svc = new AxisService("MixedService"); + // no service-level mcpReadOnly + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("readOnlyOp")); + op.addParameter(new org.apache.axis2.description.Parameter("mcpReadOnly", "true")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode annotations = getCatalogTools().get(0).path("annotations"); + assertTrue("readOnlyHint must be true when operation sets mcpReadOnly=true", + annotations.path("readOnlyHint").asBoolean()); + } + + /** + * {@code mcpIdempotent=true} maps to {@code idempotentHint: true}. + */ + public void testMcpIdempotentParamSetsIdempotentHint() throws Exception { + AxisService svc = new AxisService("QueryService"); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("getByKey")); + op.addParameter(new org.apache.axis2.description.Parameter("mcpIdempotent", "true")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode annotations = getCatalogTools().get(0).path("annotations"); + assertTrue("idempotentHint must be true when mcpIdempotent=true", + annotations.path("idempotentHint").asBoolean()); + } + + /** + * {@code mcpDestructive=true} maps to {@code destructiveHint: true}. + */ + public void testMcpDestructiveParamSetsDestructiveHint() throws Exception { + AxisService svc = new AxisService("AdminService"); + AxisOperation op = new InOutAxisOperation(); + op.setName(QName.valueOf("deleteAllData")); + op.addParameter(new org.apache.axis2.description.Parameter("mcpDestructive", "true")); + svc.addOperation(op); + axisConfig.addService(svc); + + JsonNode annotations = getCatalogTools().get(0).path("annotations"); + assertTrue("destructiveHint must be true when mcpDestructive=true", + annotations.path("destructiveHint").asBoolean()); + } + + /** + * Conservative defaults remain intact when no MCP annotation parameters + * are set — no existing behaviour is changed by the new parameter support. + */ + public void testAnnotationDefaultsAreConservativeWhenNoParamsSet() throws Exception { + addService("NoParamService", "doSomething"); + JsonNode annotations = getCatalogTools().get(0).path("annotations"); + assertFalse("readOnlyHint default must be false", annotations.path("readOnlyHint").asBoolean()); + assertFalse("destructiveHint default must be false", annotations.path("destructiveHint").asBoolean()); + assertFalse("idempotentHint default must be false", annotations.path("idempotentHint").asBoolean()); + assertFalse("openWorldHint default must be false", annotations.path("openWorldHint").asBoolean()); + } + // ── tool list mirrors existing OpenAPI paths ────────────────────────────── /** diff --git a/src/site/xdoc/docs/json-rpc-mcp-guide.xml b/src/site/xdoc/docs/json-rpc-mcp-guide.xml index 516d3ea70e..44c5128cf1 100644 --- a/src/site/xdoc/docs/json-rpc-mcp-guide.xml +++ b/src/site/xdoc/docs/json-rpc-mcp-guide.xml @@ -449,7 +449,7 @@ conformance checklist when extending the catalog or adding new MCP client code.< <tr><td><code>testMalformedJsonBodyCorrelationIdIsUuid</code></td><td>errorRef matches 8-4-4-4-12 hex UUID format</td></tr> <tr><td><code>testMissingOuterArrayReturnsBadRequestWithCorrelationId</code></td><td>Valid JSON but wrong envelope structure → correlated Bad Request</td></tr> <tr><td><code>testMalformedJsonBodyDoesNotLeakExceptionClassName</code></td><td>No "MalformedJsonException", "IOException", or "at org.apache" in response</td></tr> -<tr><td><code>testInOnlyMalformedJsonBodyReturnsBadRequestWithCorrelationId</code></td><td>InOnly receiver applies same error hardening</td></tr> +<tr><td><code>testInOnlyMalformedJsonBodyDoesNotLeakExceptionDetails</code></td><td>InOnly receiver returns empty body on fault (MEP semantics); no exception class name or stack trace leaked in response</td></tr> </table> <h3>6.5 pyRapi Authentication — test_auth.py (18 tests)</h3>
