Skip to content

Add MCP GW fixes#13310

Merged
msm1992 merged 3 commits intowso2:masterfrom
msm1992:master-mcp-1
Aug 26, 2025
Merged

Add MCP GW fixes#13310
msm1992 merged 3 commits intowso2:masterfrom
msm1992:master-mcp-1

Conversation

@msm1992
Copy link
Copy Markdown
Contributor

@msm1992 msm1992 commented Aug 22, 2025

This PR,

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 22, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Registers a custom Gson deserializer for MCP requests, centralizes MCP failure handling via a new MCP failure handler and sequence, simplifies MCP capability/tool-list payloads, conditions InternalKey header removal for MCP/existing APIs, and includes backend subscribed APIs in generated JWTs for MCP existing_API.

Changes

Cohort / File(s) Summary
MCP init & deserialization
components/apimgt/.../gateway/handlers/mcp/McpInitHandler.java, components/apimgt/.../gateway/mcp/request/MCPRequestDeserializer.java, components/apimgt/.../gateway/mcp/request/McpRequest.java
Introduces MCPRequestDeserializer and registers it via GsonBuilder in McpInitHandler; deserializes numeric/string ids; updates handleRequest to call MCP failure flow and return false on McpException; minor Javadoc tweak in McpRequest.
MCP failure handling & mediator
components/apimgt/.../gateway/mediators/McpMediator.java, components/apimgt/.../gateway/utils/MCPUtils.java, components/apimgt/.../impl/APIConstants.java, features/apimgt/.../api-manager.xml.j2
Adds MCPUtils.handleMCPFailure(MessageContext, McpResponseDto) and invokes it on MCP errors in mediator; adds MCP_FAILURE_HANDLER constant; sets received MCP id as raw object; registers _mcp_failure_handler_.xml in gateway template.
MCP payload model & generator
components/apimgt/.../gateway/mcp/response/InitializeResult.java, components/apimgt/.../gateway/mcp/response/ToolListResult.java, components/apimgt/.../gateway/utils/MCPPayloadGenerator.java
Removes multiple capability fields (logging, prompts, resources, completions, experimental) leaving only tools in InitializeResult; removes nextCursor from ToolListResult and stops setting it in payload generation.
Gateway auth header handling
components/apimgt/.../gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java
Makes removal of InternalKey header conditional—preserves header for MCP APIs and APIs with subtype EXISTING_API; otherwise removes it as before.
API key / JWT subscribed APIs
components/apimgt/.../impl/APIProviderImpl.java
JWT generation now uses a subscribedApiDTOList; for MCP EXISTING_API, includes both the MCP API and the mapped backend API (derived from APIOperationMapping) in the subscribed APIs list.
Logging / small refactors
components/apimgt/.../gateway/mcp/request/McpRequestProcessor.java, components/apimgt/.../gateway/mediators/McpMediator.java
Adds debug logging around MCP request processing and uses local dto variable in processing flow; mediator also calls centralized failure handler on errors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant GW as Gateway
  participant MH as McpInitHandler
  participant MM as McpMediator
  participant MU as MCPUtils
  participant SY as Synapse Seq (_mcp_failure_handler_)
  Note over GW: Incoming HTTP MCP request
  C->>GW: HTTP MCP request
  GW->>MH: handleRequest
  MH->>MH: Deserialize via MCPRequestDeserializer (handles string/number id)
  alt Deserialize/validate success
    GW->>MM: mediate (OUT_FLOW)
    MM->>MM: handleMcpRequest (INITIALIZE/TOOL_LIST/PING/PROMPTS_LIST)
    alt Response available
      MM-->>C: 200 OK + MCP JSON payload
    else No content
      MM-->>C: 204 No Content
    end
  else McpException / validation error
    MM->>MU: handleMCPFailure(messageContext, McpResponseDto)
    MU->>SY: Mediate _mcp_failure_handler_ (if present)
    MU-->>C: Fault response (JSON) / status via Utils.sendFault
  end
Loading
sequenceDiagram
  autonumber
  participant Dev as Publisher
  participant API as APIProviderImpl
  participant REG as Registry/DB
  Note over Dev: generateApiKey for MCP existing_API
  Dev->>API: generateApiKey(request)
  API->>REG: getAPIbyUUID(apiId, org)
  API->>REG: Read URI templates + APIOperationMapping
  API->>API: Build subscribedApiDTOList [MCP API, mapped backend API]
  API-->>Dev: JWT containing subscribedApiDTOList
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • pubudu538
  • tgtshanika
  • chamilaadhi
  • tharindu1st
  • npamudika
  • Krishanx92
  • tharikaGitHub
  • dushaniw
  • Arshardh
  • AnuGayan
  • senthuran16
  • HeshanSudarshana
  • HiranyaKavishani
  • hisanhunais
  • ashera96

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses several MCP (Model Context Protocol) gateway issues including removing unsupported capabilities from responses, fixing JSON-RPC ID handling, adding failure handling, and resolving authentication issues for existing API subtypes.

  • Removes unsupported MCP capabilities (logging, resources, prompts, completions, experimental) from initialization responses
  • Fixes ID field handling in MCP JSON-RPC requests to preserve original type (string/integer) instead of converting to string
  • Adds MCP failure handler sequence and error handling mechanism

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
api-manager.xml.j2 Adds MCP failure handler sequence configuration
APIProviderImpl.java Fixes API key generation for MCP existing API subtype by including backend API info
APIConstants.java Adds MCP failure handler constant
MCPUtils.java Adds failure handling method and fixes ID preservation
MCPPayloadGenerator.java Removes unsupported capabilities from initialization response
McpMediator.java Adds error handling and prompts list method support
ToolListResult.java Removes unsupported nextCursor field
InitializeResult.java Removes unsupported capability fields
MCPRequestDeserializer.java New custom deserializer for handling ID field types
InternalAPIKeyAuthenticator.java Preserves internal key header for MCP existing API subtype
McpInitHandler.java Integrates custom deserializer and improves error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (4)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (1)

186-190: Unify expiry-check invocation (avoid mixed FQN/import usage)

Line 188 uses the fully-qualified org.wso2...GatewayUtils.isJwtTokenExpired, while Line 221 uses the imported GatewayUtils.isJwtTokenExpired. Prefer one style for consistency; keeping the import and using the short form reads cleaner.

Apply:

-                    isVerified =
-                            GatewayUtils.verifyTokenSignature(signedJWT, alias) && !org.wso2.carbon.apimgt.gateway.utils.GatewayUtils.isJwtTokenExpired(payload);
+                    isVerified =
+                            GatewayUtils.verifyTokenSignature(signedJWT, alias) && !GatewayUtils.isJwtTokenExpired(payload);
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (2)

7684-7691: Good move to a list of subscribed APIs in the JWT payload.

Initializing a SubscribedApiDTO list and always including the primary API makes sense and aligns with the PR goal of supporting multiple entries.

Minor: consider renaming subscribedApiInfo to primaryApiInfo for readability.


7692-7708: Include backend publisher and avoid relying on an arbitrary first URI template.

  • Today you pick an arbitrary template (iterator().next()) to read APIOperationMapping. If ever multiple templates differ (or if a null sneaks in), this can misrepresent the backend entry. Prefer: find the first template that actually has a non-null APIOperationMapping.
  • The backend entry lacks publisher. Since you already add publisher for the main API, keep parity by resolving the backend API via apiOperationMapping.getApiUuid() and setting its provider as publisher. This also avoids mismatches if name/context/version alone aren’t sufficient.
  • Based on recent learnings, APIOperationMapping for MCP EXISTING_API is expected to be present and populated, so skipping extra null checks is generally safe; still, selecting a non-null mapping is a cheap guard that costs nothing at runtime. (Ref: retrieved learnings indicate safe dereference of operation mapping for EXISTING_API paths.)

Apply this focused refactor inside this block:

-            if (uriTemplates != null && !uriTemplates.isEmpty()) {
-                //fetch any uri template to get the underlying API details
-                URITemplate template = uriTemplates.iterator().next();
-                APIOperationMapping apiOperationMapping = template.getAPIOperationMapping();
-
-                SubscribedApiDTO backendApiInfo = new SubscribedApiDTO();
-                backendApiInfo.setName(apiOperationMapping.getApiName());
-                backendApiInfo.setContext(apiOperationMapping.getApiContext());
-                backendApiInfo.setVersion(apiOperationMapping.getApiVersion());
-                subscribedApiDTOList.add(backendApiInfo);
-            }
+            if (uriTemplates != null && !uriTemplates.isEmpty()) {
+                // Find the first template that actually has an APIOperationMapping
+                APIOperationMapping apiOperationMapping = null;
+                for (URITemplate t : uriTemplates) {
+                    if (t.getAPIOperationMapping() != null) {
+                        apiOperationMapping = t.getAPIOperationMapping();
+                        break;
+                    }
+                }
+                if (apiOperationMapping != null) {
+                    SubscribedApiDTO backendApiInfo = new SubscribedApiDTO();
+                    backendApiInfo.setName(apiOperationMapping.getApiName());
+                    backendApiInfo.setContext(apiOperationMapping.getApiContext());
+                    backendApiInfo.setVersion(apiOperationMapping.getApiVersion());
+                    // Populate publisher for parity with the primary API entry
+                    if (StringUtils.isNotBlank(apiOperationMapping.getApiUuid())) {
+                        API backendApi = getAPIbyUUID(apiOperationMapping.getApiUuid(), organization);
+                        backendApiInfo.setPublisher(backendApi.getId().getProviderName());
+                    }
+                    subscribedApiDTOList.add(backendApiInfo);
+                }
+            }

Please confirm all MCP EXISTING_API URITemplates for a given API point to the same backend API UUID. If not guaranteed, we should deduplicate by backend UUID and add one SubscribedApiDTO per unique backend to avoid duplicates.

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java (1)

125-128: Return JSON-RPC error envelope (with original ID) in failure path.

Currently the failure handler receives only the human-readable error message; downstream will likely emit a body that isn’t a valid JSON-RPC error response and will miss the request ID. Prefer sending a full JSON-RPC error payload, including the original id when available.

Apply:

@@
-            } catch (McpException e) {
-                log.error("Error while handling MCP response", e);
-                MCPUtils.handleMCPFailure(messageContext, new McpResponseDto(e.getErrorMessage(), e.getErrorCode(), null));
+            } catch (McpException e) {
+                log.error("Error while handling MCP response", e);
+                Object id = messageContext.getProperty("RECEIVED_MCP_ID");
+                String errorPayload = MCPPayloadGenerator.getErrorResponse(id, e.getErrorCode(),
+                        e.getErrorMessage(), e.getData());
+                MCPUtils.handleMCPFailure(messageContext,
+                        new McpResponseDto(errorPayload, HttpStatus.SC_OK, null));
                 return false;
             }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between df6b8e1 and e88cf28.

📒 Files selected for processing (11)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (5 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (2 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/InitializeResult.java (0 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/ToolListResult.java (0 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java (3 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPPayloadGenerator.java (0 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (5 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (3 hunks)
  • features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2 (1 hunks)
💤 Files with no reviewable changes (3)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/ToolListResult.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPPayloadGenerator.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/InitializeResult.java
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13273
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml:2532-2533
Timestamp: 2025-08-14T16:37:58.428Z
Learning: In wso2/carbon-apimgt PR #13273, sorting for MCP servers is intentionally deferred: /mcp-servers adds sortBy/sortOrder in components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml, and McpServersApiServiceImpl.getAllMCPServers (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/McpServersApiServiceImpl.java) currently ignores them; backend support will come in a separate PR. Avoid re-flagging this mismatch in this PR.
📚 Learning: 2025-08-04T12:29:14.882Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/PublisherCommonUtils.java:469-479
Timestamp: 2025-08-04T12:29:14.882Z
Learning: In the WSO2 Carbon API Manager codebase, when updating MCP direct endpoint subtype APIs, `updateTemplatesFromDefinition()` checks for and preserves existing `schemaDefinition` values in URI templates, so schema enrichment via `syncSchemaDefinitions()` before template update is not required.

Applied to files:

  • features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2
📚 Learning: 2025-08-14T06:32:31.632Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java:0-0
Timestamp: 2025-08-14T06:32:31.632Z
Learning: In ImportUtils.importMCPServer (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java), when constructing APIDefinitionValidationResponse for the SERVER_PROXY subtype from a backend definition, ensure the response is explicitly marked valid (validationResponse.setValid(true)) or use real validation. This was addressed in commit 7bb06973a542efd9d813678ad3944eec025afe3b.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-14T03:24:15.089Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java:818-870
Timestamp: 2025-08-14T03:24:15.089Z
Learning: In ImportUtils.importMCPServer (components/apimgt/.../ImportUtils.java), MCP Server creation paths already persist backend mappings; explicit calls to apiProvider.updateMCPServerBackend(...) after creation are unnecessary. Only one backend is supported for MCP-server subtype; selecting the first backend is intentional.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
📚 Learning: 2025-08-14T06:25:45.334Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java:790-794
Timestamp: 2025-08-14T06:25:45.334Z
Learning: In ImportUtils.importMCPServer (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java), the update path intentionally iterates and updates all backends to prepare for future multi-backend support. While current MCP-server behavior elsewhere may use a single backend (e.g., creation path selecting the first), do not refactor the update path to only process the first backend.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
📚 Learning: 2025-08-04T09:07:15.107Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:2464-2464
Timestamp: 2025-08-04T09:07:15.107Z
Learning: In WSO2 Carbon API Manager codebase, API_TYPE_MCP is intentionally excluded from API_SUPPORTED_TYPE_LIST; MCP APIs are treated differently from regular API types.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java
📚 Learning: 2025-08-07T13:44:49.366Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/PublisherCommonUtils.java:945-1115
Timestamp: 2025-08-07T13:44:49.366Z
Learning: In the WSO2 Carbon API Manager codebase, when handling both APIDTO and MCPServerDTO in prepareForUpdateApi, some duplication of validation logic is acceptable due to type differences, and extracting generic helpers is not required.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
📚 Learning: 2025-08-07T13:29:57.960Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIDTOTypeWrapper.java:221-229
Timestamp: 2025-08-07T13:29:57.960Z
Learning: In WSO2 Carbon API Manager, the keyManagers field from APIDTO.getKeyManagers() and MCPServerDTO.getKeyManagers() methods always returns an array of strings, even though the method signature returns Object. The cast to List<String> is safe in this context due to architectural guarantees.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java
📚 Learning: 2025-08-13T12:26:54.692Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:3036-3041
Timestamp: 2025-08-13T12:26:54.692Z
Learning: In WSO2 APIM (carbon-apimgt), during API deletion (APIProviderImpl.deleteAPIFromDB), MCP subtype SERVER_PROXY stores its server details via BackendOperationMapping; therefore, it must be cleaned up using ApiMgtDAO.removeBackendOperationMapping (same as DIRECT_BACKEND), while EXISTING_API uses removeApiOperationMapping.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-08T16:55:52.564Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:6340-6351
Timestamp: 2025-08-08T16:55:52.564Z
Learning: In ApiMgtDAO.addURITemplates (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java), for MCP EXISTING_API flows, URITemplate.getExistingAPIOperationMapping() is always created with a non-null BackendOperation. This is ensured by population points like OAS2Parser.populateURITemplate and SubscriptionValidationDAO.populateMcpOperationMappings. Therefore accessing getExistingAPIOperationMapping().getBackendOperation() without extra null checks is safe.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-08T16:54:11.459Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:6331-6339
Timestamp: 2025-08-08T16:54:11.459Z
Learning: In WSO2 Carbon API Manager (PR wso2/carbon-apimgt#13201), BackendOperationMapping is always created with a non-null BackendOperation. Therefore, in ApiMgtDAO.addURITemplates (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java), it is safe to dereference backendOperationMapping.getBackendOperation() without extra null checks. Population points like OAS2Parser.populateURITemplate and SubscriptionValidationDAO.populateMcpOperationMappings ensure this invariant.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-13T07:07:56.953Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/URITemplate.java:497-505
Timestamp: 2025-08-13T07:07:56.953Z
Learning: In PR wso2/carbon-apimgt#13255, the renaming of URITemplate methods getExistingAPIOperationMapping/setExistingAPIOperationMapping to getAPIOperationMapping/setAPIOperationMapping was confirmed to have no existing usages in the codebase, making deprecated bridge methods unnecessary.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-08-07T13:13:38.999Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/PublisherCommonUtils.java:436-459
Timestamp: 2025-08-07T13:13:38.999Z
Learning: In PublisherCommonUtils.handleExistingApiSubtype (Java, WSO2 API Manager), null or empty API UUIDs in APIOperationMapping are already checked and handled by fetchReferencedApi(), so additional null checks before calling it are unnecessary.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
📚 Learning: 2025-08-14T02:20:31.093Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java:1166-1171
Timestamp: 2025-08-14T02:20:31.093Z
Learning: File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java
Context: fromAPIToMCPServerMetadataDTO
Learning: The codebase design passes the API UUID through APIIdentifier; therefore MCPServerMetadataDTO.id should be populated using api.getId().getUUID() (not api.getUUID()). Upstream flows populate APIIdentifier.uuid before use.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-04T10:42:40.165Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/restapi/publisher/ApisApiServiceImplUtils.java:809-819
Timestamp: 2025-08-04T10:42:40.165Z
Learning: The constant APIConstants.AI.MCP_DEFAULT_FEATURE_TYPE is defined in components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java within the AI nested class, not missing as initially analyzed.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java
📚 Learning: 2025-08-14T03:12:42.527Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:3571-3571
Timestamp: 2025-08-14T03:12:42.527Z
Learning: In APIConstants.MCP, the old constant name TOOL_DESC_KEY had no usages in the codebase; the new TOOL_DESCRIPTION_KEY is already adopted in relevant modules. No deprecated alias is necessary.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java
📚 Learning: 2025-08-07T13:15:14.163Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java:1762-1767
Timestamp: 2025-08-07T13:15:14.163Z
Learning: In WSO2 Carbon API Manager, the AM_BACKEND_OPERATION_MAPPING and AM_API_OPERATION_MAPPING tables have ON DELETE CASCADE foreign keys on URL_MAPPING_ID referencing AM_API_URL_MAPPING, ensuring that deleting a URL mapping automatically removes dependent rows and prevents orphan rows.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
🧬 Code graph analysis (5)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (1)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1)
  • APIConstants (32-3806)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)
  • MCPUtils (58-482)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (2)
  • APIConstants (32-3806)
  • MCP (3627-3726)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (2)
components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/model/APIOperationMapping.java (1)
  • APIOperationMapping (27-94)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1)
  • APIConstants (32-3806)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java (1)
  • Utils (100-871)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (2)
  • APIConstants (32-3806)
  • MCP (3627-3726)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (6)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/exception/McpException.java (1)
  • McpException (26-53)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java (1)
  • Utils (100-871)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java (1)
  • MCPRequestDeserializer (35-71)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/McpResponseDto.java (1)
  • McpResponseDto (24-58)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPPayloadGenerator.java (1)
  • MCPPayloadGenerator (40-217)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)
  • MCPUtils (58-482)
🔇 Additional comments (12)
features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2 (1)

1849-1851: MCP failure handler sequence added in the Gateway SkipList — LGTM

Including _mcp_failure_handler_.xml alongside the other well-known sequences is consistent with the new MCP failure path (MCPUtils + APIConstants.MCP.MCP_FAILURE_HANDLER) and ensures the template-driven deployments won’t attempt to sync/override this built-in sequence. Placement after _graphql_failure_handler_.xml mirrors similar handlers.

components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java (2)

64-64: Import of APIOperationMapping is appropriate and used later.

The new import is required for the EXISTING_API subtype handling below.


7729-7729: Downstream token generators confirmed to use the list field only

I’ve audited all usages of the new subscribedApiDTOList and found that:

  • JwtTokenInfoDTO defines only the subscribedApiDTOList (no legacy single-entry field remains)
  • InternalAPIKeyGenerator and DefaultApiKeyGenerator both call jwtTokenInfoDTO.getSubscribedApiDTOList() when building the JWT claims
  • APIConsumerImpl delegates to the same generators, so no consumer is expecting a single-entry field
  • No references to setSubscribedApiInfo, getSubscribedApiInfo, or a SubscribedApiInfo type were found anywhere in the token pipeline

Since there are no legacy consumers relying on a single-entry subscription field, no dual-field population is necessary. Feel free to remove the old-field patterns outright if they still exist in comments or docs.

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mediators/McpMediator.java (2)

46-46: Non-functional import addition.

Importing MCPUtils is expected for the new failure flow hook. No action needed.


141-142: LGTM: PROMPTS_LIST handled consistently with other no-op methods.

Including METHOD_PROMPTS_LIST in the short-circuit branch keeps behavior consistent with initialize/tool list/ping handling.

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (5)

31-31: Non-functional import additions.

Imports added to support the new failure flow. No action needed.

Also applies to: 33-33, 35-35, 45-45


79-80: LGTM: Better diagnostics for unknown methods.

Passing the requested method name as the error data improves troubleshooting without altering the JSON-RPC code/message semantics.


121-123: LGTM: PROMPTS_LIST support.

Returning an empty result for prompts/list is aligned with current “unsupported but spec-compliant” stance (mirrors resources/resourcetemplates).


250-251: LGTM: Preserve original JSON-RPC id type in context.

Storing the raw id object prevents the “double-string” regression and lets the response builder echo the exact id.


465-481: Confirm mediation contract of the failure sequence.

The logic relies on sequence.mediate() returning false after sending a response; otherwise Utils.sendFault(200) is invoked. Ensure the new _mcp_failure_handler sequence adheres to this contract to avoid double responses or hanging flows.

Would you like me to add a small unit/integration test or a gateway IT to assert the behavior (sequence present vs. absent) and the emitted HTTP status/body?

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (2)

141-143: LGTM: Custom deserializer registration.

Registering MCPRequestDeserializer addresses the jsonrpc id type issues during request parsing.


197-200: LGTM: Expand no-auth methods per MCP spec.

Treating resources/list, resources/templates/list, and prompts/list as no-auth is consistent with the control-plane nature of these calls.

RakhithaRR
RakhithaRR previously approved these changes Aug 22, 2025
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)

79-79: Include requested method in METHOD_NOT_FOUND error — good alignment with prior feedback

Returning “Method not found” improves diagnosability and matches the earlier suggestion.

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (1)

285-293: Restrict InternalKey header preservation to MCP EXISTING_API only (prevent credential leakage)

Current condition preserves the header for any MCP API or any EXISTING_API subtype. This can leak InternalKey to non-MCP backends (e.g., MCP DIRECT_BACKEND or non-MCP EXISTING_API). Preserve it only when both type=MCP and subtype=EXISTING_API.

Apply:

-                //Remove InternalKey header from the request except for MCP existing_API subtype
-                API matchedAPI = GatewayUtils.getAPI(mCtx);
-                if (matchedAPI != null && !APIConstants.API_TYPE_MCP.equals(matchedAPI.getApiType()) &&
-                        !APIConstants.API_SUBTYPE_EXISTING_API.equals(matchedAPI.getSubtype())) {
-                    if (log.isDebugEnabled()) {
-                        log.debug("Removing Internal Key header from request for API: " + matchedAPI.getName());
-                    }
-                    headers.remove(securityParam);
-                }
+                // Remove InternalKey header except for MCP EXISTING_API subtype
+                API matchedAPI = GatewayUtils.getAPI(mCtx);
+                boolean isMcpExistingApi = matchedAPI != null
+                        && APIConstants.API_TYPE_MCP.equals(matchedAPI.getApiType())
+                        && APIConstants.API_SUBTYPE_EXISTING_API.equals(matchedAPI.getSubtype());
+                if (!isMcpExistingApi) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Removing Internal Key header from request"
+                                + (matchedAPI != null ? (" for API: " + matchedAPI.getName()) : ""));
+                    }
+                    headers.remove(securityParam);
+                }
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (1)

93-96: Return a proper JSON-RPC error body and 200 status via failure handler

Passing only e.getErrorMessage() as the response yields a plain string, not a JSON-RPC error object. Also, the HTTP status for JSON-RPC errors should be 200. Build the JSON-RPC error via MCPPayloadGenerator and set status to SC_OK in the DTO.

Apply:

-        } catch (McpException e) {
-            log.error("MCP init failed: " + String.valueOf(e.getData()), e);
-            MCPUtils.handleMCPFailure(messageContext, new McpResponseDto(e.getErrorMessage(), e.getErrorCode(), null));
-            return false;
+        } catch (McpException e) {
+            log.error("MCP init failed: " + String.valueOf(e.getData()), e);
+            MCPUtils.handleMCPFailure(messageContext,
+                    new McpResponseDto(
+                            MCPPayloadGenerator.getErrorResponse(null, e.getErrorCode(), e.getErrorMessage(), e.getData()),
+                            HttpStatus.SC_OK,
+                            null));
+            return false;
         }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e88cf28 and a50a91a.

📒 Files selected for processing (5)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (5 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (5 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13273
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml:2532-2533
Timestamp: 2025-08-14T16:37:58.428Z
Learning: In wso2/carbon-apimgt PR #13273, sorting for MCP servers is intentionally deferred: /mcp-servers adds sortBy/sortOrder in components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml, and McpServersApiServiceImpl.getAllMCPServers (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/McpServersApiServiceImpl.java) currently ignores them; backend support will come in a separate PR. Avoid re-flagging this mismatch in this PR.
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class automatically sets the jsonrpc version via a final field initialized with the constant MCP.RpcConstants.JSON_RPC_VERSION ("2.0"). Custom deserializers should not attempt to deserialize or set the jsonrpc field as it's handled automatically.
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class sets the jsonrpc version via a constant mechanism rather than requiring deserialization from the JSON input. The deserializer should not attempt to set the jsonrpc field from the incoming JSON.
📚 Learning: 2025-08-22T10:38:42.372Z
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class automatically sets the jsonrpc version via a final field initialized with the constant MCP.RpcConstants.JSON_RPC_VERSION ("2.0"). Custom deserializers should not attempt to deserialize or set the jsonrpc field as it's handled automatically.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-22T10:38:42.372Z
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class sets the jsonrpc version via a constant mechanism rather than requiring deserialization from the JSON input. The deserializer should not attempt to set the jsonrpc field from the incoming JSON.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-08T16:54:11.459Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:6331-6339
Timestamp: 2025-08-08T16:54:11.459Z
Learning: In WSO2 Carbon API Manager (PR wso2/carbon-apimgt#13201), BackendOperationMapping is always created with a non-null BackendOperation. Therefore, in ApiMgtDAO.addURITemplates (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java), it is safe to dereference backendOperationMapping.getBackendOperation() without extra null checks. Population points like OAS2Parser.populateURITemplate and SubscriptionValidationDAO.populateMcpOperationMappings ensure this invariant.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-08T16:55:52.564Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java:6340-6351
Timestamp: 2025-08-08T16:55:52.564Z
Learning: In ApiMgtDAO.addURITemplates (components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/ApiMgtDAO.java), for MCP EXISTING_API flows, URITemplate.getExistingAPIOperationMapping() is always created with a non-null BackendOperation. This is ensured by population points like OAS2Parser.populateURITemplate and SubscriptionValidationDAO.populateMcpOperationMappings. Therefore accessing getExistingAPIOperationMapping().getBackendOperation() without extra null checks is safe.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-13T12:26:54.692Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIProviderImpl.java:3036-3041
Timestamp: 2025-08-13T12:26:54.692Z
Learning: In WSO2 APIM (carbon-apimgt), during API deletion (APIProviderImpl.deleteAPIFromDB), MCP subtype SERVER_PROXY stores its server details via BackendOperationMapping; therefore, it must be cleaned up using ApiMgtDAO.removeBackendOperationMapping (same as DIRECT_BACKEND), while EXISTING_API uses removeApiOperationMapping.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-07T13:15:14.163Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java:1762-1767
Timestamp: 2025-08-07T13:15:14.163Z
Learning: In WSO2 Carbon API Manager, the AM_BACKEND_OPERATION_MAPPING and AM_API_OPERATION_MAPPING tables have ON DELETE CASCADE foreign keys on URL_MAPPING_ID referencing AM_API_URL_MAPPING, ensuring that deleting a URL mapping automatically removes dependent rows and prevents orphan rows.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java
📚 Learning: 2025-08-14T06:32:31.632Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java:0-0
Timestamp: 2025-08-14T06:32:31.632Z
Learning: In ImportUtils.importMCPServer (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java), when constructing APIDefinitionValidationResponse for the SERVER_PROXY subtype from a backend definition, ensure the response is explicitly marked valid (validationResponse.setValid(true)) or use real validation. This was addressed in commit 7bb06973a542efd9d813678ad3944eec025afe3b.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-04T09:07:15.107Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:2464-2464
Timestamp: 2025-08-04T09:07:15.107Z
Learning: In WSO2 Carbon API Manager codebase, API_TYPE_MCP is intentionally excluded from API_SUPPORTED_TYPE_LIST; MCP APIs are treated differently from regular API types.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-04T10:42:40.165Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/restapi/publisher/ApisApiServiceImplUtils.java:809-819
Timestamp: 2025-08-04T10:42:40.165Z
Learning: The constant APIConstants.AI.MCP_DEFAULT_FEATURE_TYPE is defined in components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java within the AI nested class, not missing as initially analyzed.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-14T02:20:31.093Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java:1166-1171
Timestamp: 2025-08-14T02:20:31.093Z
Learning: File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java
Context: fromAPIToMCPServerMetadataDTO
Learning: The codebase design passes the API UUID through APIIdentifier; therefore MCPServerMetadataDTO.id should be populated using api.getId().getUUID() (not api.getUUID()). Upstream flows populate APIIdentifier.uuid before use.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
🧬 Code graph analysis (3)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/security/authenticator/InternalAPIKeyAuthenticator.java (1)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (1)
  • APIConstants (32-3806)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (8)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/APIMgtGatewayConstants.java (1)
  • APIMgtGatewayConstants (22-216)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/exception/McpException.java (1)
  • McpException (26-53)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java (1)
  • Utils (100-871)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java (1)
  • MCPRequestDeserializer (37-92)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/response/McpResponseDto.java (1)
  • McpResponseDto (24-58)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPPayloadGenerator.java (1)
  • MCPPayloadGenerator (40-217)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)
  • MCPUtils (58-482)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (2)
  • APIConstants (32-3806)
  • MCP (3627-3726)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java (1)
  • Utils (100-871)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (2)
  • APIConstants (32-3806)
  • MCP (3627-3726)
🔇 Additional comments (4)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (2)

31-35: Imports added for failure-path handling — OK

The new imports (HttpStatus, Mediator, SynapseConstants, Utils) are necessary for the failure handler logic introduced below. No issues.

Also applies to: 45-45


250-251: No further action needed: raw MCP ID usage is correct
The code already stores RECEIVED_MCP_ID as an Object and downstream (in McpMediator.buildMCPResponse) retrieves it via getProperty into an Object and passes it directly to MCPPayloadGenerator.generateMCPResponsePayload. There are no String casts on the ID, so type fidelity is preserved and there’s no ClassCastException risk.

components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/mcp/McpInitHandler.java (2)

141-143: Custom Gson deserializer registration — correct and consistent with MCPRequest design

Registering MCPRequestDeserializer ensures ID type is preserved (string/integer) while leaving jsonrpc version to the McpRequest default. This matches the internal invariant that jsonrpc is auto-set (no need to deserialize/set it).


197-200: Expanding no-auth methods — OK

Adding resources/list, resources/templates/list, and prompts/list to the no-auth set aligns with the current MCP capabilities exposure model.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (2)

392-401: Avoid NPE and narrow casting in header processing

  • TRANSPORT_HEADERS can be null; initialize it if needed.
  • Don’t cast argument values to String directly; use toString() to support numerics/booleans.
  • Minor grammar fix in the error message.
-            Map headers = (Map) axis2MessageContext.getProperty(org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS);
+            Map headers = (Map) axis2MessageContext.getProperty(
+                    org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS);
+            if (headers == null) {
+                headers = new java.util.HashMap();
+                axis2MessageContext.setProperty(
+                        org.apache.axis2.context.MessageContext.TRANSPORT_HEADERS, headers);
+            }
             for (Param param : headerParams) {
                 String paramName = param.getName();
                 boolean isParamRequired = param.isRequired();
 
-                if (argumentObj.get(paramName) != null) {
-                    String paramValue = (String) argumentObj.get(paramName);
+                if (argumentObj.get(paramName) != null) {
+                    Object paramValueObj = argumentObj.get(paramName);
+                    String paramValue = paramValueObj != null ? paramValueObj.toString() : "";
                     headers.put(paramName, paramValue);
                 } else {
                     if (isParamRequired) {
                         throw new McpException(APIConstants.MCP.RpcConstants.INVALID_PARAMS_CODE,
-                                APIConstants.MCP.RpcConstants.INVALID_PARAMS_MESSAGE, "Required param " + paramName +
-                                "is not defined");
+                                APIConstants.MCP.RpcConstants.INVALID_PARAMS_MESSAGE, "Required param " + paramName +
+                                " is not defined");
                     }
                 }
             }

Also applies to: 403-407


339-347: URL-encode query and path parameter values

Raw concatenation risks malformed URLs for reserved characters and spaces. Encode query values and path segments.

-                    String paramValue = paramValueObj != null ? paramValueObj.toString() : "";
+                    String paramValue = paramValueObj != null ? paramValueObj.toString() : "";
+                    String encodedValue = java.net.URLEncoder.encode(paramValue, java.nio.charset.StandardCharsets.UTF_8);
                     if (!StringUtils.isEmpty(paramValue)) {
                         if (queryString.length() == 0) {
-                            queryString.append(paramName).append("=").append(paramValue);
+                            queryString.append(paramName).append("=").append(encodedValue);
                         } else {
-                            queryString.append("&").append(paramName).append("=").append(paramValue);
+                            queryString.append("&").append(paramName).append("=").append(encodedValue);
                         }
                     } else {
-                String paramValue = paramValueObj.toString();
-                resourcePathString = resourcePathString.replace("{" + pathParam + "}", paramValue);
+                String paramValue = paramValueObj.toString();
+                // URLEncoder encodes spaces as '+', normalize to %20 for path segments
+                String encodedPathValue = java.net.URLEncoder
+                        .encode(paramValue, java.nio.charset.StandardCharsets.UTF_8)
+                        .replace("+", "%20");
+                resourcePathString = resourcePathString.replace("{" + pathParam + "}", encodedPathValue);

Also applies to: 373-374

♻️ Duplicate comments (1)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)

127-129: Unify METHOD_NOT_FOUND “data” with validateRequest path

Default branch still returns generic “Method not found”. Include the method name for consistency with validateRequest.

-                    throw new McpException(APIConstants.MCP.RpcConstants.METHOD_NOT_FOUND_CODE,
-                            APIConstants.MCP.RpcConstants.METHOD_NOT_FOUND_MESSAGE, "Method not found");
+                    throw new McpException(APIConstants.MCP.RpcConstants.METHOD_NOT_FOUND_CODE,
+                            APIConstants.MCP.RpcConstants.METHOD_NOT_FOUND_MESSAGE, "Method " + method + " not found");
🧹 Nitpick comments (5)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequestProcessor.java (2)

34-37: Harden pre-log extraction and include request id for traceability

Move method/id extraction before logging, null-guard them, and log id alongside method for easier correlation across logs.

-        if (log.isDebugEnabled()) {
-            log.debug("Processing MCP request: " + requestBody.getMethod() + " for API: " +
-                    matchedMcpApi.getName() + "-" + matchedMcpApi.getVersion());
-        }
-
-        String method = requestBody.getMethod();
+        String method = requestBody != null ? requestBody.getMethod() : null;
+        Object id = requestBody != null ? requestBody.getId() : null;
+        if (log.isDebugEnabled()) {
+            log.debug("Processing MCP request: method=" + method + ", id=" + id + " for API: " +
+                    matchedMcpApi.getName() + "-" + matchedMcpApi.getVersion());
+        }

Also applies to: 39-41


42-45: Log outcome details (status, id) to complete the picture

Add status code (and id) to the success log for parity with the pre-log and to aid debugging.

-        if (log.isDebugEnabled()) {
-            log.debug("Successfully processed MCP request: " + requestBody.getMethod() + " for API: " +
-                    matchedMcpApi.getName() + "-" + matchedMcpApi.getVersion());
-        }
+        if (log.isDebugEnabled()) {
+            log.debug("Successfully processed MCP request: method=" + method + ", id=" + id + " for API: " +
+                    matchedMcpApi.getName() + "-" + matchedMcpApi.getVersion() + ", status=" + dto.getStatusCode());
+        }
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (3)

484-492: Use the provided error payload directly to avoid double‑nesting JSON-RPC errors

Wrapping an already-serialized JSON-RPC error payload inside another error object can confuse clients. Prefer the payload from responseDto.

-            JsonUtil.getNewJsonPayload(axis2MC, MCPPayloadGenerator.getErrorResponse(null,
-                    responseDto.getStatusCode(), "MCP Failure", responseDto.getResponse()), true, true);
+            JsonUtil.getNewJsonPayload(axis2MC, responseDto.getResponse(), true, true);

471-475: Consider centralizing message-context property keys

"MCP_ID", "MCP_ERROR_CODE" (and "RECEIVED_MCP_ID") are string literals in multiple places. Extract constants to avoid typos and ease future changes.

You can add near the top of this class (outside this hunk):

private static final String PROP_MCP_ID = "MCP_ID";
private static final String PROP_MCP_ERROR_CODE = "MCP_ERROR_CODE";
private static final String PROP_RECEIVED_MCP_ID = "RECEIVED_MCP_ID";

and use those instead of literals here and in transformMcpRequest.


424-446: Be tolerant to contentType case and missing requestBody schema

Minor robustness:

  • Compare content types case-insensitively.
  • If requestBody.contentType is absent in schema, either default to JSON or return INVALID_PARAMS.

Example tweaks in-place:

  • Use equalsIgnoreCase for JSON/XML checks.
  • In processMcpSchema, guard requestBodySchema.get("contentType") and handle null.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a50a91a and 1073d66.

📒 Files selected for processing (3)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequest.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequestProcessor.java (1 hunks)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (5 hunks)
✅ Files skipped from review due to trivial changes (1)
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequest.java
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2:1849-1851
Timestamp: 2025-08-22T10:58:02.212Z
Learning: In wso2/carbon-apimgt PR #13310, MCP failure handler naming is finalized: APIConstants.MCP.MCP_FAILURE_HANDLER = "_mcp_failure_handler_" and the template lists <Sequence>_mcp_failure_handler_.xml</Sequence>. Avoid re-flagging naming drift for this handler in future reviews.
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13273
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml:2532-2533
Timestamp: 2025-08-14T16:37:58.428Z
Learning: In wso2/carbon-apimgt PR #13273, sorting for MCP servers is intentionally deferred: /mcp-servers adds sortBy/sortOrder in components/apimgt/org.wso2.carbon.apimgt.rest.api.util/src/main/resources/publisher-api.yaml, and McpServersApiServiceImpl.getAllMCPServers (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/impl/McpServersApiServiceImpl.java) currently ignores them; backend support will come in a separate PR. Avoid re-flagging this mismatch in this PR.
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class automatically sets the jsonrpc version via a final field initialized with the constant MCP.RpcConstants.JSON_RPC_VERSION ("2.0"). Custom deserializers should not attempt to deserialize or set the jsonrpc field as it's handled automatically.
📚 Learning: 2025-08-22T10:38:42.372Z
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/MCPRequestDeserializer.java:47-71
Timestamp: 2025-08-22T10:38:42.372Z
Learning: In wso2/carbon-apimgt, the McpRequest class automatically sets the jsonrpc version via a final field initialized with the constant MCP.RpcConstants.JSON_RPC_VERSION ("2.0"). Custom deserializers should not attempt to deserialize or set the jsonrpc field as it's handled automatically.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequestProcessor.java
  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-22T10:58:02.212Z
Learnt from: msm1992
PR: wso2/carbon-apimgt#13310
File: features/apimgt/org.wso2.carbon.apimgt.core.feature/src/main/resources/conf_templates/templates/repository/conf/api-manager.xml.j2:1849-1851
Timestamp: 2025-08-22T10:58:02.212Z
Learning: In wso2/carbon-apimgt PR #13310, MCP failure handler naming is finalized: APIConstants.MCP.MCP_FAILURE_HANDLER = "_mcp_failure_handler_" and the template lists <Sequence>_mcp_failure_handler_.xml</Sequence>. Avoid re-flagging naming drift for this handler in future reviews.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-14T06:32:31.632Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java:0-0
Timestamp: 2025-08-14T06:32:31.632Z
Learning: In ImportUtils.importMCPServer (components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/ImportUtils.java), when constructing APIDefinitionValidationResponse for the SERVER_PROXY subtype from a backend definition, ensure the response is explicitly marked valid (validationResponse.setValid(true)) or use real validation. This was addressed in commit 7bb06973a542efd9d813678ad3944eec025afe3b.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-08T18:04:07.174Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:494-494
Timestamp: 2025-08-08T18:04:07.174Z
Learning: In PR wso2/carbon-apimgt#13201, the removal of the duplicate constant APIConstants.API_TYPE_PROP (impl module) and replacing its usage in TemplateBuilderUtil with APIConstants.API_TYPE is deferred to a separate follow-up PR.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-08T06:17:39.942Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIDTOTypeWrapper.java:1-17
Timestamp: 2025-08-08T06:17:39.942Z
Learning: In PR wso2/carbon-apimgt#13201, the existing license header in components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIDTOTypeWrapper.java is correct; the alternative header variant suggested by the bot is older and should not be applied.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-08T07:10:32.169Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/dao/constants/SQLConstants.java:1563-1566
Timestamp: 2025-08-08T07:10:32.169Z
Learning: In PR wso2/carbon-apimgt#13201 (SQLConstants.java), scope is limited to adding DESCRIPTION and SCHEMA_DEFINITION to AM_API_URL_MAPPING inserts. MEDIATION_SCRIPT alignment across INSERT_URL_MAPPINGS and INSERT_URL_MAPPINGS_CURRENT_API is out of scope for this PR to maintain backward compatibility; consider addressing in a separate issue/PR.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-04T09:07:15.107Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java:2464-2464
Timestamp: 2025-08-04T09:07:15.107Z
Learning: In WSO2 Carbon API Manager codebase, API_TYPE_MCP is intentionally excluded from API_SUPPORTED_TYPE_LIST; MCP APIs are treated differently from regular API types.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-04T10:42:40.165Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13201
File: components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/restapi/publisher/ApisApiServiceImplUtils.java:809-819
Timestamp: 2025-08-04T10:42:40.165Z
Learning: The constant APIConstants.AI.MCP_DEFAULT_FEATURE_TYPE is defined in components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java within the AI nested class, not missing as initially analyzed.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
📚 Learning: 2025-08-14T02:20:31.093Z
Learnt from: PasanT9
PR: wso2/carbon-apimgt#13255
File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java:1166-1171
Timestamp: 2025-08-14T02:20:31.093Z
Learning: File: components/apimgt/org.wso2.carbon.apimgt.rest.api.publisher.v1.common/src/main/java/org/wso2/carbon/apimgt/rest/api/publisher/v1/common/mappings/APIMappingUtil.java
Context: fromAPIToMCPServerMetadataDTO
Learning: The codebase design passes the API UUID through APIIdentifier; therefore MCPServerMetadataDTO.id should be populated using api.getId().getUUID() (not api.getUUID()). Upstream flows populate APIIdentifier.uuid before use.

Applied to files:

  • components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java
🧬 Code graph analysis (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/mcp/request/McpRequestProcessor.java (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (1)
  • MCPUtils (58-495)
components/apimgt/org.wso2.carbon.apimgt.keymgt/src/main/java/org/wso2/carbon/apimgt/keymgt/model/entity/API.java (1)
  • API (33-378)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (3)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/handlers/Utils.java (1)
  • Utils (100-871)
components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/APIConstants.java (3)
  • APIConstants (32-3806)
  • MCP (3627-3726)
  • RpcConstants (3710-3725)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPPayloadGenerator.java (1)
  • MCPPayloadGenerator (40-217)
🔇 Additional comments (2)
components/apimgt/org.wso2.carbon.apimgt.gateway/src/main/java/org/wso2/carbon/apimgt/gateway/utils/MCPUtils.java (2)

78-80: Good: include requested method in the error “data” for validateRequest

This aligns the METHOD_NOT_FOUND response with helpful context.


250-251: LGTM: store RECEIVED_MCP_ID as the raw id object

Matches the design where McpRequest sets jsonrpc internally and id may be string or number. This also aligns with the recorded learning about not coercing jsonrpc/id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error while calling tools in MCP Server MCP (From Existing API) Publisher MCP Playground Fails

4 participants