Support MCP Agents on the Gateway

[roh26it]

Draft PR

[matter-code-review]

Description

Motivation

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactoring (no functional changes)

How Has This Been Tested?

• Unit Tests
• Integration Tests
• Manual Testing

Screenshots (if applicable)

N/A

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Related Issues

N/A

Summary By MatterAI

🔄 What Changed

The tryPost function in src/handlers/handlerUtils.ts has been refactored by removing the overarching try...finally block that previously wrapped the entire request processing logic. This change also removes the explicit McpService disposal call (mcpService[Symbol.asyncDispose]()) from the function's cleanup path.

🔍 Impact of the Change

Removing the try...finally block means that McpService instances are no longer guaranteed to be disposed of, potentially leading to resource leaks (e.g., open connections, memory) if the mcpService holds state or resources. This impacts the reliability and stability of the tryPost function, as errors occurring during request handling will not trigger the necessary cleanup, and the service might accumulate unreleased resources over time.

📁 Total Files Changed

• src/handlers/handlerUtils.ts: Refactored tryPost function, removing try...finally block and McpService disposal logic.

🧪 Test Added

N/A

🔒Security Vulnerabilities

The removal of guaranteed resource disposal for McpService could lead to resource exhaustion under sustained load, potentially enabling a denial-of-service (DoS) attack if the system accumulates unreleased resources. This is a critical reliability and maintainability regression.

Tip

Quality Recommendations

1. Reintroduce robust error handling for the entire tryPost function to ensure all operations are gracefully managed.

2. Implement explicit resource disposal for McpService instances, ideally using Symbol.asyncDispose with a using declaration (if supported by the TypeScript version) or by re-establishing a try...finally block to guarantee cleanup.

3. Ensure that the mcpService[Symbol.asyncDispose]() call is wrapped in its own try...catch block to prevent disposal errors from crashing the application.

Tanka Poem ♫

Code's old try-catch gone,
Resources now freely roam,
Memory's slow creep.
Scientist sighs, a silent plea,
Clean up, for stability. 🧪

Sequence Diagram

sequenceDiagram
    participant GW as Gateway (tryPost)
    participant MCP as McpService
    participant RC as RequestContext
    participant PC as ProviderContext
    participant CS as CacheService
    participant PRS as PreRequestValidatorService
    participant RH as RequestHandler (recursiveAfterRequestHookHandler)
    participant RS as ResponseService
    participant LOG as LogObject

    GW->>MCP: new McpService(requestContext)
    alt McpService exists
        GW->>MCP: init()
        MCP-->>GW: tools
        GW->>RC: addMcpTools(tools)
    end

    GW->>PC: hasRequestHandler(requestContext)
    alt No Request Handler
        GW->>RC: transformToProviderRequestAndSave()
    end

    GW->>GW: constructRequest(providerContext, requestContext)
    GW->>CS: new CacheService(c, hooksService)
    GW->>CS: getCachedResponse(requestContext, fetchOptions.headers)
    CS-->>GW: cacheResponseObject
    GW->>LOG: addCache(cacheStatus, cacheKey)

    alt Cache Hit
        GW->>RS: create(cacheResponseObject)
        RS-->>GW: {response, originalResponseJson}
        GW->>LOG: updateRequestContext(requestContext, fetchOptions.headers)
        GW->>LOG: addResponse(response, originalResponseJson)
        GW->>LOG: log()
        GW-->>Client: response
    else Cache Miss
        GW->>PRS: new PreRequestValidatorService(c, requestContext)
        GW->>PRS: getResponse()
        PRS-->>GW: preRequestValidatorResponse
        alt Pre-request Validation Response
            GW->>RS: create(preRequestValidatorResponse)
            RS-->>GW: {response, originalResponseJson}
            GW->>LOG: updateRequestContext(requestContext, fetchOptions.headers)
            GW->>LOG: addResponse(response, originalResponseJson)
            GW->>LOG: log()
            GW-->>Client: response
        else No Pre-request Validation Response
            GW->>RH: recursiveAfterRequestHookHandler(requestContext, fetchOptions, ...)
            RH-->>GW: {mappedResponse, retryCount, createdAt, originalResponseJson}
            GW->>RS: create(mappedResponse, ...)
            RS-->>GW: {response, mappedOriginalResponseJson}
            Note over GW: Log handled inside recursiveAfterRequestHookHandler
            GW-->>Client: response
        end
    end
    Note over GW: McpService disposal logic is now absent, leading to potential resource leaks.

Loading

[matter-code-review]

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

• /matter summary: Generate AI Summary for the PR
• /matter review: Generate AI Reviews for the latest commit in the PR
• /matter review-full: Generate AI Reviews for the complete PR
• /matter release-notes: Generate AI release-notes for the PR
• /matter : Chat with your PR with Matter AI Agent
• /matter remember : Generate AI memories for the PR
• /matter explain: Get an explanation of the PR
• /matter help: Show the list of available commands and documentation
• Need help? Join our Discord server: https://discord.gg/fJU5DvanU3