From ff9a20ccc3986478ce4f7fe6551d0dd5bc66572b Mon Sep 17 00:00:00 2001 From: baubakg Date: Tue, 5 May 2026 14:53:37 +0200 Subject: [PATCH 1/5] Add three-level Javadoc quality gate for MCP tool discovery (issue #40) Extends IBS.MCP.REQUIRE_JAVADOC from a boolean to a three-value setting: - false: expose all public static methods - true: requires non-empty Javadoc comment (previous default) - strict (new default): requires comment + @param for every parameter Methods missing @param tags previously exposed "description":"String" as the parameter description, giving AI agents no useful guidance. The strict default prevents such tools from appearing in tools/list. Startup log now reports the active gate level and its effect. Diagnostics field renamed from javadocRequired (boolean) to javadocQualityGate (string) to accurately reflect the three-way setting. Co-Authored-By: Claude Sonnet 4.6 --- docs/MCP.md | 50 ++++++++---- .../bridge/service/ConfigValueHandlerIBS.java | 8 +- .../bridge/service/MCPRequestHandler.java | 13 +++- .../bridge/service/MCPToolDiscovery.java | 53 ++++++++++--- .../bridge/service/MCPBridgeServerTest.java | 78 ++++++++++++++++++- 5 files changed, 168 insertions(+), 34 deletions(-) diff --git a/docs/MCP.md b/docs/MCP.md index cc58426..e12f0a2 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -43,7 +43,7 @@ using the built-in demo data, then from an external project that hosts its own J |---|---|---| | `IBS.MCP.ENABLED` | `false` | Enables the MCP endpoint at `/mcp`. Must be `true` for any MCP usage. | | `IBS.MCP.PRECHAIN` | — | JSON `callContent` fragment prepended to every `java_call` invocation. Used for server-wide setup such as shared authentication. Can also be supplied per-client via the `ibs-prechain` HTTP header (env var takes precedence). | -| `IBS.MCP.REQUIRE_JAVADOC` | `true` | When `true`, only methods with a non-empty Javadoc comment are included in the tool catalog. Methods without Javadoc are silently excluded from `tools/list`. | +| `IBS.MCP.REQUIRE_JAVADOC` | `strict` | Controls which methods are exposed based on Javadoc quality. `false` = expose all public static methods. `true` = requires a non-empty Javadoc comment. `strict` (default) = requires a comment **and** a non-empty `@param` tag for every parameter. | See the relevant sections below for full configuration details and examples. @@ -558,7 +558,7 @@ Response: "mcpConfig": { "packagesConfigured": "com.example.services", "prechainActive": true, - "javadocRequired": true + "javadocQualityGate": "strict" }, "headers": { "secretHeaderKeys": ["ibs-secret-login", "ibs-secret-pass", "ibs-secret-url"], @@ -580,7 +580,7 @@ Response: | `deploymentMode` | `TEST` or `PRODUCTION` | | `mcpConfig.packagesConfigured` | Value of `IBS.CLASSLOADER.STATIC.INTEGRITY.PACKAGES` | | `mcpConfig.prechainActive` | Whether a prechain is configured (env var or header) | -| `mcpConfig.javadocRequired` | Whether `IBS.MCP.REQUIRE_JAVADOC` is enabled | +| `mcpConfig.javadocQualityGate` | Active value of `IBS.MCP.REQUIRE_JAVADOC` (`false`, `true`, or `strict`) | | `headers.secretHeaderKeys` | Names of `ibs-secret-*` headers received (values suppressed) | | `headers.envVarHeaders` | Decoded env-var headers (`ibs-env-*` prefix stripped, uppercased) | | `headers.regularHeaderCount` | Count of headers that are neither secret nor env-var | @@ -883,16 +883,36 @@ Without the dependency, tools are still fully functional; only the description q ### Javadoc quality gate -By default (`IBS.MCP.REQUIRE_JAVADOC=true`), BridgeService **only exposes methods that have a -non-empty Javadoc comment**. Methods without Javadoc are silently skipped at startup and will not -appear in `tools/list`. +BridgeService enforces a configurable documentation quality gate via `IBS.MCP.REQUIRE_JAVADOC`. +Methods that do not satisfy the active gate are silently skipped at startup and will not appear +in `tools/list`. -This is intentional. A method with no Javadoc would receive a generic fallback description such as -`"Calls com.example.MyClass.method()"`, which gives an AI agent no useful information about when -or why to call it. Exposing such tools increases the risk of accidental invocations. +The three levels are: -**To opt out** (expose all public static methods regardless of documentation): +| Value | What is required to be exposed | +|---|---| +| `false` | Nothing — all public static methods are exposed regardless of documentation | +| `true` | A non-empty Javadoc comment on the method | +| `strict` *(default)* | A non-empty comment **and** a non-empty `@param` tag for every parameter | + +**Why `strict` is the default:** a method with parameters but no `@param` tags causes BridgeService +to fall back to the Java type name as the parameter description (e.g. `"description": "String"`). +This gives the AI agent almost no guidance on what to pass, which leads to incorrect calls and +wasted round-trips. `strict` prevents such tools from appearing in the catalog. + +At startup, BridgeService logs the active gate level and its effect so you can confirm your +configuration is applied: +``` +MCPRequestHandler ready: 12 individual tool(s) + java_call + ibs_diagnostics. +Javadoc quality gate: strict — only methods with Javadoc comment + @param for every parameter are exposed +``` + +**To use the previous default** (non-empty comment only): +``` +IBS.MCP.REQUIRE_JAVADOC=true +``` +**To expose all public static methods** (no documentation required): ``` IBS.MCP.REQUIRE_JAVADOC=false ``` @@ -1113,11 +1133,11 @@ tool invocation. 3. **What each parameter expects** — use `@param` tags; BridgeService uses them as argument descriptions in the tool schema. 4. **When to use it vs similar methods** — if overloads or related methods exist, say which scenario each is for. -**The quality gate enforces the minimum bar.** `IBS.MCP.REQUIRE_JAVADOC=true` (the default) -silently drops any method with no Javadoc from the catalog entirely — it will not appear in -`tools/list` and cannot be called via auto-discovery. Passing the gate (a non-empty comment) -is necessary but not sufficient: a one-word description passes the gate but still produces a -useless tool entry. +**The quality gate enforces the minimum bar.** `IBS.MCP.REQUIRE_JAVADOC=strict` (the default) +silently drops any method that lacks a comment or is missing `@param` tags — it will not appear in +`tools/list` and cannot be called via auto-discovery. Passing the gate is necessary but not +sufficient: a one-word description with perfunctory `@param` tags passes the gate but still +produces a low-quality tool entry. **Good Javadoc pays compound interest.** A well-described method is discovered correctly the first time, requires no follow-up prompting, and stays reliable as the AI session context diff --git a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/ConfigValueHandlerIBS.java b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/ConfigValueHandlerIBS.java index 26919fc..f6a1bfb 100644 --- a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/ConfigValueHandlerIBS.java +++ b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/ConfigValueHandlerIBS.java @@ -73,9 +73,11 @@ public void activate(String in_value) { "IBS.DESERIALIZATION.DATE.FORMAT", "NONE", false, "The date format to be used for deserialization."), MCP_ENABLED("IBS.MCP.ENABLED", "false", false, "When set to true, enables the MCP server endpoint at POST /mcp, exposing configured packages as tools."), - MCP_REQUIRE_JAVADOC("IBS.MCP.REQUIRE_JAVADOC", "true", false, - "When true (default), only methods with a non-empty Javadoc comment are exposed as MCP tools. " - + "Methods without Javadoc are silently skipped. Set to false to expose all public static methods."), + MCP_REQUIRE_JAVADOC("IBS.MCP.REQUIRE_JAVADOC", "strict", false, + "Controls which methods are exposed as MCP tools based on Javadoc quality. " + + "Accepted values: 'false' (expose all public static methods), " + + "'true' (requires non-empty Javadoc comment), " + + "'strict' (requires comment + non-empty @param for every parameter, default)."), MCP_PRECHAIN("IBS.MCP.PRECHAIN", null, false, "JSON callContent fragment prepended to every auto-discovered MCP tool invocation. " + "Entries execute in the same isolated context as the actual call, so call-chaining " diff --git a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPRequestHandler.java b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPRequestHandler.java index 52815ae..dddc3ba 100644 --- a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPRequestHandler.java +++ b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPRequestHandler.java @@ -137,8 +137,13 @@ public MCPRequestHandler() { l_diagnosticsTool.put("inputSchema", DIAGNOSTICS_SCHEMA_MAP); tools.add(l_diagnosticsTool); this.toolList = Collections.unmodifiableList(tools); - log.info("MCPRequestHandler ready: {} individual tool(s) + java_call + ibs_diagnostics.", - discoveredToolCount); + String l_javadocExplained = ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("strict") + ? "strict — only methods with Javadoc comment + @param for every parameter are exposed" + : ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("false") + ? "false — all public static methods are exposed regardless of documentation" + : "true — only methods with a non-empty Javadoc comment are exposed"; + log.info("MCPRequestHandler ready: {} individual tool(s) + java_call + ibs_diagnostics. " + + "Javadoc quality gate: {}", discoveredToolCount, l_javadocExplained); } /** @@ -338,8 +343,8 @@ private String handleDiagnostics(Object id, Map headers) { ConfigValueHandlerIBS.STATIC_INTEGRITY_PACKAGES.fetchValue()); String prechain = ConfigValueHandlerIBS.MCP_PRECHAIN.fetchValue(); mcpConfig.put("prechainActive", prechain != null && !prechain.isBlank()); - mcpConfig.put("javadocRequired", - Boolean.parseBoolean(ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.fetchValue())); + mcpConfig.put("javadocQualityGate", + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.fetchValue()); diag.put("mcpConfig", mcpConfig); String secretPrefix = ConfigValueHandlerIBS.SECRETS_FILTER_PREFIX.fetchValue(); diff --git a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java index c95b760..a776a8c 100644 --- a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java +++ b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java @@ -95,10 +95,7 @@ public static DiscoveryResult discoverTools(String packagesCsv) { if (overloads.size() == 1) { // Unique method name on this class — use simple tool name Method method = overloads.get(0); - if (ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("true") && !hasJavadoc(method)) { - log.debug("Skipping {}.{} — no Javadoc (IBS.MCP.REQUIRE_JAVADOC=true)", - clazz.getSimpleName(), methodName); - } else { + if (!shouldSkipForJavadoc(method, clazz.getSimpleName(), methodName)) { String toolName = clazz.getSimpleName() + "_" + methodName; registerTool(tools, registry, toolName, method); } @@ -113,13 +110,10 @@ public static DiscoveryResult discoverTools(String packagesCsv) { + "use the java_call tool to invoke them directly.", clazz.getName(), methodName, countEntry.getKey()); } else { - Method method = countEntry.getValue().get(0); - if (ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("true") && !hasJavadoc(method)) { - log.debug("Skipping {}.{} — no Javadoc (IBS.MCP.REQUIRE_JAVADOC=true)", - clazz.getSimpleName(), methodName); - } else { - String toolName = clazz.getSimpleName() + "_" + methodName + "_" + countEntry.getKey(); - registerTool(tools, registry, toolName, method); + Method lt_method = countEntry.getValue().get(0); + if (!shouldSkipForJavadoc(lt_method, clazz.getSimpleName(), methodName)) { + String lt_toolName = clazz.getSimpleName() + "_" + methodName + "_" + countEntry.getKey(); + registerTool(tools, registry, lt_toolName, lt_method); } } } @@ -182,6 +176,43 @@ static boolean hasJavadoc(Method method) { } } + /** + * Returns true if the method has a non-empty Javadoc comment AND a non-empty + * {@code @param} tag for every parameter. Methods with no parameters pass if + * they have a non-empty comment. + */ + static boolean hasAdequateJavadoc(Method method) { + if (!hasJavadoc(method)) return false; + int l_paramCount = method.getParameterCount(); + if (l_paramCount == 0) return true; + try { + MethodJavadoc l_javadoc = RuntimeJavadoc.getJavadoc(method); + if (l_javadoc == null) return false; + List l_params = l_javadoc.getParams(); + if (l_params.size() < l_paramCount) return false; + return l_params.stream().allMatch(p -> !COMMENT_FORMATTER.format(p.getComment()).isEmpty()); + } catch (Exception e) { + return false; + } + } + + private static boolean shouldSkipForJavadoc(Method in_method, String in_clazz, String in_method_name) { + if (ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("strict")) { + if (!hasAdequateJavadoc(in_method)) { + log.debug("Skipping {}.{} — Javadoc quality gate (strict) not met: missing comment or @param", + in_clazz, in_method_name); + return true; + } + } else if (ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.is("true")) { + if (!hasJavadoc(in_method)) { + log.debug("Skipping {}.{} — Javadoc quality gate (true) not met: no Javadoc comment", + in_clazz, in_method_name); + return true; + } + } + return false; + } + /** * Builds a JSON Schema object describing the input parameters of a method. * Parameter names are generated as arg0, arg1, ... since Java reflection diff --git a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java index a58d439..47f8fde 100644 --- a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java +++ b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java @@ -8,6 +8,7 @@ */ package com.adobe.campaign.tests.bridge.service; +import com.adobe.campaign.tests.bridge.testdata.one.SimpleStaticMethods; import io.restassured.response.Response; import org.hamcrest.Matchers; import org.testng.annotations.AfterGroups; @@ -16,6 +17,10 @@ import org.testng.annotations.Test; import io.javalin.Javalin; +import java.lang.reflect.Method; +import java.util.List; +import java.util.Map; + import static io.restassured.RestAssured.given; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -201,7 +206,7 @@ public void testDiagnosticsTool_mcpConfigReflectsCurrentState() { assertThat(text, containsString("packagesConfigured")); assertThat(text, containsString(TESTDATA_ONE_PACKAGE)); assertThat(text, containsString("\"prechainActive\":false")); - assertThat(text, containsString("\"javadocRequired\":true")); + assertThat(text, containsString("\"javadocQualityGate\"")); } @Test(groups = "MCP") @@ -1092,6 +1097,77 @@ public void testToolsList_constructorsNotExposedAsIndividualTools() { .body("result.tools.name", not(hasItem("Instantiable_Instantiable"))); } + // ---- hasAdequateJavadoc unit tests ---- + + @Test(groups = "MCP") + public void testHasAdequateJavadoc_methodWithCommentAndAllParams_returnsTrue() throws Exception { + Method l_method = SimpleStaticMethods.class.getMethod("methodAcceptingStringArgument", String.class); + assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(true)); + } + + @Test(groups = "MCP") + public void testHasAdequateJavadoc_noArgMethodWithComment_returnsTrue() throws Exception { + Method l_method = SimpleStaticMethods.class.getMethod("methodReturningString"); + assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(true)); + } + + @Test(groups = "MCP") + public void testHasAdequateJavadoc_methodWithNoJavadoc_returnsFalse() throws Exception { + Method l_method = SimpleStaticMethods.class.getMethod("overLoadedMethod1Arg", String.class); + assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(false)); + } + + // ---- Javadoc quality gate integration tests ---- + + @Test(groups = "MCP") + public void testDiscoverTools_strictMode_documentedMethodsIncluded() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("strict"); + try { + MCPToolDiscovery.DiscoveryResult l_result = + MCPToolDiscovery.discoverTools(TESTDATA_ONE_PACKAGE); + List> l_tools = l_result.tools; + assertThat("Documented method with @param must appear under strict", + l_tools.stream().anyMatch(t -> "SimpleStaticMethods_methodAcceptingStringArgument".equals(t.get("name"))), + is(true)); + assertThat("Undocumented method must be absent under strict", + l_tools.stream().noneMatch(t -> ("SimpleStaticMethods_overLoadedMethod1Arg").equals(t.get("name"))), + is(true)); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + + @Test(groups = "MCP") + public void testDiscoverTools_falseMode_undocumentedMethodsIncluded() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("false"); + try { + MCPToolDiscovery.DiscoveryResult l_result = + MCPToolDiscovery.discoverTools(TESTDATA_ONE_PACKAGE); + // methodThrowingLinkageError has no Javadoc — excluded under strict/true, included under false + assertThat("Undocumented method must appear when gate is disabled", + l_result.tools.stream().anyMatch(t -> + "SimpleStaticMethods_methodThrowingLinkageError".equals(t.get("name"))), + is(true)); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + + @Test(groups = "MCP") + public void testToolsList_strictDefault_exposesFullyDocumentedMethods() { + // With the default (strict), all documented SimpleStaticMethods with @param should be present. + given() + .contentType(CONTENT_TYPE_JSON) + .body("{\"jsonrpc\":\"2.0\",\"id\":200,\"method\":\"tools/list\",\"params\":{}}") + .when() + .post(MCP_ENDPOINT) + .then() + .statusCode(200) + .body("result.tools.name", hasItem("SimpleStaticMethods_methodAcceptingStringArgument")) + .body("result.tools.name", hasItem("SimpleStaticMethods_methodAcceptingTwoArguments")) + .body("result.tools.name", hasItem("SimpleStaticMethods_methodAcceptingIntArgument")); + } + @Test(groups = "MCP") public void testJavaCall_constructorChain_instantiableThenStaticMethod() { // Constructors are not available as individual tools — use java_call to instantiate From 203bc041b9a200e0ce15a1ed72cdf915c05288ac Mon Sep 17 00:00:00 2001 From: baubakg Date: Tue, 5 May 2026 14:59:46 +0200 Subject: [PATCH 2/5] Report skipped method count in MCP tool discovery log The discovery log now shows how many methods were filtered out by the active quality gate, e.g. "21 registered, 5 skipped by quality gate (strict)". Added skippedCount field to DiscoveryResult for use by callers. Co-Authored-By: Claude Sonnet 4.6 --- .../bridge/service/MCPToolDiscovery.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java index a776a8c..9d39dc1 100644 --- a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java +++ b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java @@ -40,10 +40,13 @@ public static class DiscoveryResult { public final List> tools; /** Maps each tool name to the Java Method it represents, used to build the catalog in the java_call description. */ public final Map methodRegistry; + /** Number of methods skipped by the active Javadoc quality gate. */ + public final int skippedCount; - public DiscoveryResult(List> tools, Map methodRegistry) { + public DiscoveryResult(List> tools, Map methodRegistry, int skippedCount) { this.tools = Collections.unmodifiableList(tools); this.methodRegistry = Collections.unmodifiableMap(methodRegistry); + this.skippedCount = skippedCount; } } @@ -57,11 +60,12 @@ public DiscoveryResult(List> tools, Map meth public static DiscoveryResult discoverTools(String packagesCsv) { List> tools = new ArrayList<>(); Map registry = new LinkedHashMap<>(); + int[] l_skipped = {0}; if (packagesCsv == null || packagesCsv.trim().isEmpty()) { log.warn("IBS.CLASSLOADER.STATIC.INTEGRITY.PACKAGES is not set — no tools will be discovered for MCP. " + "Set this property to enable tool discovery."); - return new DiscoveryResult(tools, registry); + return new DiscoveryResult(tools, registry, 0); } // Strip trailing dots that IBS uses as package separators (e.g. "com.example.") @@ -95,7 +99,9 @@ public static DiscoveryResult discoverTools(String packagesCsv) { if (overloads.size() == 1) { // Unique method name on this class — use simple tool name Method method = overloads.get(0); - if (!shouldSkipForJavadoc(method, clazz.getSimpleName(), methodName)) { + if (shouldSkipForJavadoc(method, clazz.getSimpleName(), methodName)) { + l_skipped[0]++; + } else { String toolName = clazz.getSimpleName() + "_" + methodName; registerTool(tools, registry, toolName, method); } @@ -111,7 +117,9 @@ public static DiscoveryResult discoverTools(String packagesCsv) { clazz.getName(), methodName, countEntry.getKey()); } else { Method lt_method = countEntry.getValue().get(0); - if (!shouldSkipForJavadoc(lt_method, clazz.getSimpleName(), methodName)) { + if (shouldSkipForJavadoc(lt_method, clazz.getSimpleName(), methodName)) { + l_skipped[0]++; + } else { String lt_toolName = clazz.getSimpleName() + "_" + methodName + "_" + countEntry.getKey(); registerTool(tools, registry, lt_toolName, lt_method); } @@ -121,9 +129,9 @@ public static DiscoveryResult discoverTools(String packagesCsv) { } } - log.info("MCP tool discovery complete: {} tool(s) registered from {} class(es).", - tools.size(), allClasses.size()); - return new DiscoveryResult(tools, registry); + log.info("MCP tool discovery complete: {} tool(s) registered, {} skipped by quality gate ({}), from {} class(es).", + tools.size(), l_skipped[0], ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.fetchValue(), allClasses.size()); + return new DiscoveryResult(tools, registry, l_skipped[0]); } private static void registerTool(List> tools, Map registry, From 38b19955b4055b2ea6b8a916006e0f65edace26d Mon Sep 17 00:00:00 2001 From: baubakg Date: Tue, 5 May 2026 15:10:50 +0200 Subject: [PATCH 3/5] Improve coverage for Javadoc quality gate and document coverage rules Add tests for the 'true' mode path of shouldSkipForJavadoc (previously unreachable since the default changed to 'strict') and for skippedCount reporting. Update CLAUDE.md with explicit 80% patch coverage rule so future contributors understand what codecov/patch enforces. Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 5 +++ .../bridge/service/MCPBridgeServerTest.java | 34 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 822ad20..50a73bd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,6 +33,11 @@ mvn package **Quality gates that must pass before merging:** unit tests pass, no coverage decrease, SonarCloud gate green, license headers present. +**Coverage rules (enforced by SonarCloud and codecov/patch in CI):** +- Overall line coverage must not decrease from the baseline on `main`. +- New/changed code (the PR patch) must reach **80% line coverage** — this is the `codecov/patch` check. If it fails, add targeted tests for the uncovered branches in your new code. +- JaCoCo reports are generated at `target/site/jacoco/` after `mvn test`. Check method-level branch coverage there before pushing. + ## Module Structure ``` diff --git a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java index 47f8fde..7059664 100644 --- a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java +++ b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java @@ -1119,6 +1119,19 @@ public void testHasAdequateJavadoc_methodWithNoJavadoc_returnsFalse() throws Exc // ---- Javadoc quality gate integration tests ---- + @Test(groups = "MCP") + public void testDiscoverTools_strictMode_skippedCountIsPositive() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("strict"); + try { + MCPToolDiscovery.DiscoveryResult l_result = + MCPToolDiscovery.discoverTools(TESTDATA_ONE_PACKAGE); + assertThat("skippedCount must be positive — test data contains undocumented methods", + l_result.skippedCount, greaterThan(0)); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + @Test(groups = "MCP") public void testDiscoverTools_strictMode_documentedMethodsIncluded() { ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("strict"); @@ -1137,6 +1150,27 @@ public void testDiscoverTools_strictMode_documentedMethodsIncluded() { } } + @Test(groups = "MCP") + public void testDiscoverTools_trueMode_excludesUndocumentedIncludesDocumented() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("true"); + try { + MCPToolDiscovery.DiscoveryResult l_result = + MCPToolDiscovery.discoverTools(TESTDATA_ONE_PACKAGE); + assertThat("Undocumented method must be absent under true mode", + l_result.tools.stream().noneMatch(t -> + "SimpleStaticMethods_methodThrowingLinkageError".equals(t.get("name"))), + is(true)); + assertThat("Documented method must appear under true mode", + l_result.tools.stream().anyMatch(t -> + "SimpleStaticMethods_methodAcceptingStringArgument".equals(t.get("name"))), + is(true)); + assertThat("Skip count must reflect filtered methods", + l_result.skippedCount, greaterThan(0)); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + @Test(groups = "MCP") public void testDiscoverTools_falseMode_undocumentedMethodsIncluded() { ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("false"); From 7190c3cab80ea85a3ed207bb3058f5b5f15a691d Mon Sep 17 00:00:00 2001 From: baubakg Date: Tue, 5 May 2026 15:24:04 +0200 Subject: [PATCH 4/5] Fix codecov/patch coverage gaps in Javadoc quality gate Three gaps addressed: - Lines 119-124: overloaded-method path with unique param counts was never exercised; added overLoadedMethodDifferentArity(String) and (String,String) to SimpleStaticMethods to trigger this path - Lines 143-144: MCPRequestHandler startup log 'false'/'true' ternary branches were only reached under strict mode; added handler construction tests for both modes - Lines 202-203: hasAdequateJavadoc exception catch now covered via Mockito MockedStatic with thenAnswer/thenThrow chaining Co-Authored-By: Claude Sonnet 4.6 --- .../testdata/one/SimpleStaticMethods.java | 17 ++++++++ .../bridge/service/MCPBridgeServerTest.java | 39 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/bridgeService-data/src/main/java/com/adobe/campaign/tests/bridge/testdata/one/SimpleStaticMethods.java b/bridgeService-data/src/main/java/com/adobe/campaign/tests/bridge/testdata/one/SimpleStaticMethods.java index c2235b3..a95d082 100644 --- a/bridgeService-data/src/main/java/com/adobe/campaign/tests/bridge/testdata/one/SimpleStaticMethods.java +++ b/bridgeService-data/src/main/java/com/adobe/campaign/tests/bridge/testdata/one/SimpleStaticMethods.java @@ -159,6 +159,23 @@ public static String overLoadedMethod1Arg(int in_intArgument) { return in_intArgument + SUCCESS_VAL; } + // For testing MCP tool discovery of overloads with different arity. + // The 1-arg variant has no Javadoc (exercises the skip path for overloads). + public static String overLoadedMethodDifferentArity(String in_arg) { + return in_arg + SUCCESS_VAL; + } + + /** + * Two-argument overload for testing MCP tool name disambiguation by arity. + * + * @param in_arg1 first string argument + * @param in_arg2 second string argument + * @return concatenation of both arguments with the success suffix + */ + public static String overLoadedMethodDifferentArity(String in_arg1, String in_arg2) { + return in_arg1 + in_arg2 + SUCCESS_VAL; + } + //For impossible Objects exception public static String complexMethodAcceptor(Instantiable in_arg) { return SUCCESS_VAL; diff --git a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java index 7059664..c7a4bf4 100644 --- a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java +++ b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java @@ -9,8 +9,11 @@ package com.adobe.campaign.tests.bridge.service; import com.adobe.campaign.tests.bridge.testdata.one.SimpleStaticMethods; +import com.github.therapi.runtimejavadoc.RuntimeJavadoc; import io.restassured.response.Response; import org.hamcrest.Matchers; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.testng.annotations.AfterGroups; import org.testng.annotations.BeforeGroups; import org.testng.annotations.BeforeMethod; @@ -1117,6 +1120,18 @@ public void testHasAdequateJavadoc_methodWithNoJavadoc_returnsFalse() throws Exc assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(false)); } + @Test(groups = "MCP") + public void testHasAdequateJavadoc_runtimeJavadocThrows_returnsFalse() throws Exception { + Method l_method = SimpleStaticMethods.class.getMethod("methodAcceptingStringArgument", String.class); + try (MockedStatic l_mock = Mockito.mockStatic(RuntimeJavadoc.class)) { + // First call (from hasJavadoc) delegates to real impl; second call (inside hasAdequateJavadoc) throws. + l_mock.when(() -> RuntimeJavadoc.getJavadoc(l_method)) + .thenAnswer(invocation -> invocation.callRealMethod()) + .thenThrow(new RuntimeException("simulated Javadoc read failure")); + assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(false)); + } + } + // ---- Javadoc quality gate integration tests ---- @Test(groups = "MCP") @@ -1187,6 +1202,30 @@ public void testDiscoverTools_falseMode_undocumentedMethodsIncluded() { } } + @Test(groups = "MCP") + public void testMCPRequestHandler_falseMode_startupLogBranchCovered() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("false"); + try { + // Constructing MCPRequestHandler exercises the 'false' branch of the startup log ternary. + MCPRequestHandler l_handler = new MCPRequestHandler(); + assertThat(l_handler, notNullValue()); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + + @Test(groups = "MCP") + public void testMCPRequestHandler_trueModeStartupLogBranchCovered() { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.activate("true"); + try { + // Constructing MCPRequestHandler exercises the 'true' branch of the startup log ternary. + MCPRequestHandler l_handler = new MCPRequestHandler(); + assertThat(l_handler, notNullValue()); + } finally { + ConfigValueHandlerIBS.MCP_REQUIRE_JAVADOC.reset(); + } + } + @Test(groups = "MCP") public void testToolsList_strictDefault_exposesFullyDocumentedMethods() { // With the default (strict), all documented SimpleStaticMethods with @param should be present. From e168b50f168d329748b597636a30c1290ce272ed Mon Sep 17 00:00:00 2001 From: baubakg Date: Tue, 5 May 2026 15:56:48 +0200 Subject: [PATCH 5/5] Fix codecov/patch: inline hasAdequateJavadoc to cover exception path hasAdequateJavadoc previously called hasJavadoc() first then called RuntimeJavadoc.getJavadoc() a second time inside its own try block. The Mockito thenAnswer(callRealMethod).thenThrow() pattern failed to delegate the first call correctly in static mock mode, so hasJavadoc() returned false and the catch block was never reached. Inline the comment check into a single try/catch so a simple thenThrow mock hits the catch block directly, covering lines 202-203. Co-Authored-By: Claude Sonnet 4.6 --- .../campaign/tests/bridge/service/MCPToolDiscovery.java | 7 +++---- .../campaign/tests/bridge/service/MCPBridgeServerTest.java | 2 -- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java index 9d39dc1..ab79654 100644 --- a/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java +++ b/integroBridgeService/src/main/java/com/adobe/campaign/tests/bridge/service/MCPToolDiscovery.java @@ -190,12 +190,11 @@ static boolean hasJavadoc(Method method) { * they have a non-empty comment. */ static boolean hasAdequateJavadoc(Method method) { - if (!hasJavadoc(method)) return false; - int l_paramCount = method.getParameterCount(); - if (l_paramCount == 0) return true; try { MethodJavadoc l_javadoc = RuntimeJavadoc.getJavadoc(method); - if (l_javadoc == null) return false; + if (l_javadoc == null || COMMENT_FORMATTER.format(l_javadoc.getComment()).isEmpty()) return false; + int l_paramCount = method.getParameterCount(); + if (l_paramCount == 0) return true; List l_params = l_javadoc.getParams(); if (l_params.size() < l_paramCount) return false; return l_params.stream().allMatch(p -> !COMMENT_FORMATTER.format(p.getComment()).isEmpty()); diff --git a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java index c7a4bf4..96b88e2 100644 --- a/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java +++ b/integroBridgeService/src/test/java/com/adobe/campaign/tests/bridge/service/MCPBridgeServerTest.java @@ -1124,9 +1124,7 @@ public void testHasAdequateJavadoc_methodWithNoJavadoc_returnsFalse() throws Exc public void testHasAdequateJavadoc_runtimeJavadocThrows_returnsFalse() throws Exception { Method l_method = SimpleStaticMethods.class.getMethod("methodAcceptingStringArgument", String.class); try (MockedStatic l_mock = Mockito.mockStatic(RuntimeJavadoc.class)) { - // First call (from hasJavadoc) delegates to real impl; second call (inside hasAdequateJavadoc) throws. l_mock.when(() -> RuntimeJavadoc.getJavadoc(l_method)) - .thenAnswer(invocation -> invocation.callRealMethod()) .thenThrow(new RuntimeException("simulated Javadoc read failure")); assertThat(MCPToolDiscovery.hasAdequateJavadoc(l_method), is(false)); }