Skip to content

my browser agent repo i have 2 MCP, one which i created myself which has all bro#380

Open
shivammittal274 wants to merge 2 commits intomainfrom
feat/k0jwkDy_-my-browser-agent-repo-i-have-2-mcp-one-w
Open

my browser agent repo i have 2 MCP, one which i created myself which has all bro#380
shivammittal274 wants to merge 2 commits intomainfrom
feat/k0jwkDy_-my-browser-agent-repo-i-have-2-mcp-one-w

Conversation

@shivammittal274
Copy link
Contributor

Summary

my browser agent repo i have 2 MCP, one which i created myself which has all browser mcp tools, and second i have klavis mcp, which is used to check all like gmail, slack etc, klavis exposes all third party tools directly, so if you check tool loop agent or single agent, i am passng klavis mcp urls directly, i need to expose klavis mcp tools within browser mcp as well, so that this can be unified, current agent has thode 2 MCP, i am thinking can we unify in one, lets thonk and try to work

Changes

apps/server/src/api/routes/mcp.ts                  |  23 ++
 apps/server/src/api/server.ts                      |   1 +
 .../api/services/mcp/klavis-strata-pool.test.ts    | 297 +++++++++++++++++++++
 .../src/api/services/mcp/klavis-strata-pool.ts     | 254 ++++++++++++++++++
 .../src/api/services/mcp/klavis-tool-proxy.test.ts | 215 +++++++++++++++
 .../src/api/services/mcp/klavis-tool-proxy.ts      | 129 +++++++++
 6 files changed, 919 insertions(+)

Agent Metadata

  • Total cost: $19.9787
  • Stages:
    • ok setup ($0.0000, 61.1s)
    • ok plan ($4.6157, 851.2s)
    • ok implement ($9.0131, 2346.1s)

Generated by coding-agent v3

BrowserOS Coding Agent added 2 commits March 1, 2026 15:58
@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2026

Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement.

To sign the CLA, please add a comment to this PR with the following text:

I have read the CLA Document and I hereby sign the CLA

You only need to sign once. After signing, this check will pass automatically.


Troubleshooting
  • Already signed but still failing? Comment recheck to trigger a re-verification.
  • Signed with a different email? Make sure your commit email matches your GitHub account email, or add your commit email to your GitHub account.
- - - I have read the CLA Document and I hereby sign the CLA - - - **BrowserOS Coding Agent** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).
You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 1, 2026

Greptile Summary

This PR adds Klavis MCP tool integration to unify external tools (Gmail, Slack, etc.) with the browser MCP server. It introduces a pooling mechanism with TTL-based eviction to manage Klavis Strata instances and proxy their tools into the main MCP server.

Key Changes:

  • Created KlavisStrataPool to manage Klavis Strata instances with automatic eviction after 30 minutes
  • Added registerKlavisTools function to proxy Klavis tools with automatic name collision handling (prefixes with klavis_ when needed)
  • Integrated pool into MCP route handler to dynamically register Klavis tools per request based on X-Enabled-MCP-Servers header
  • Comprehensive test coverage for both pool management and tool registration

Issues Found:

  • Critical: Memory leak in mcp.ts - pool instance is never disposed, causing the eviction timer to run indefinitely
  • Type casting issue using overly restrictive Record<string, never> instead of Record<string, unknown>
  • Missing timeout/abort signal handling in fetch calls to Klavis Strata endpoints
  • Unused AbortSignal parameter in tool proxy handler

Confidence Score: 2/5

  • This PR has a critical memory leak that will cause resource exhaustion over time
  • The memory leak in apps/server/src/api/routes/mcp.ts where the pool's eviction timer is never cleared is a critical issue that will cause the server to leak resources. The pool is instantiated once but never disposed, and its setInterval timer will continue running indefinitely. This must be fixed before merging.
  • apps/server/src/api/routes/mcp.ts requires immediate attention to fix the memory leak. apps/server/src/api/services/mcp/klavis-tool-proxy.ts needs type casting fix.

Important Files Changed

Filename Overview
apps/server/src/api/routes/mcp.ts Added Klavis pool integration but has memory leak - pool instance never disposed
apps/server/src/api/server.ts Added browserosId parameter to MCP routes - minimal change
apps/server/src/api/services/mcp/klavis-strata-pool.ts Pool manager with TTL eviction - well-structured but depends on caller to invoke dispose()
apps/server/src/api/services/mcp/klavis-tool-proxy.ts Tool proxy with name collision handling - has type casting issue and unused AbortSignal

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant Route as MCP Route Handler
    participant Pool as KlavisStrataPool
    participant KlavisAPI as Klavis API
    participant Strata as Klavis Strata
    participant MCP as MCP Server

    Client->>Route: POST /mcp (with X-Enabled-MCP-Servers)
    Route->>Pool: ensureTools(browserosId, enabledServers)
    
    alt Cache miss or expired
        Pool->>KlavisAPI: createStrata(userId, servers)
        KlavisAPI-->>Pool: strataServerUrl
        Pool->>Strata: POST tools/list
        Strata-->>Pool: [tool descriptors]
        Pool->>MCP: registerTool(resolvedName, handler)
        Note over Pool,MCP: Handles name collisions<br/>by prefixing with klavis_
        Pool-->>Route: Tools registered
    else Cache hit
        Pool-->>Route: Tools already registered
    end
    
    Route->>MCP: connect(transport)
    MCP-->>Client: Tool list (browser + Klavis)
    
    Client->>MCP: Call Klavis tool
    MCP->>Pool: executeToolCall(toolName, args)
    Pool->>Strata: POST tools/call
    Strata-->>Pool: result
    Pool-->>MCP: content + isError
    MCP-->>Client: Tool result
Loading

Last reviewed commit: 87c4519

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +30 to +32
const pool = deps.browserosId
? new KlavisStrataPool(new KlavisClient(), mcpServer, browserToolNames)
: undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

Memory leak - pool instance is never disposed. The pool has a setInterval timer (line 34 in klavis-strata-pool.ts) that will run indefinitely. Store the pool at module or server level and call pool.dispose() on server shutdown.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/api/routes/mcp.ts
Line: 30-32

Comment:
Memory leak - `pool` instance is never disposed. The pool has a `setInterval` timer (line 34 in klavis-strata-pool.ts) that will run indefinitely. Store the pool at module or server level and call `pool.dispose()` on server shutdown.

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant