Skip to content

fix(mcp): fix resource leak and stale-header bug in MCP client#890

Open
h4sht wants to merge 1 commit into
CodebuffAI:mainfrom
h4sht:fix/mcp-client-resource-leak
Open

fix(mcp): fix resource leak and stale-header bug in MCP client#890
h4sht wants to merge 1 commit into
CodebuffAI:mainfrom
h4sht:fix/mcp-client-resource-leak

Conversation

@h4sht

@h4sht h4sht commented Jul 22, 2026

Copy link
Copy Markdown

Summary

Fixes two related bugs in the MCP client module that cause resource leaks and stale connections:

1. Memory/Connection Leak — No Client Cleanup

The `runningClients` map and `listToolsCache` in `common/src/mcp/client.ts` grew without bound. Once an MCP client was connected, it was never removed from the registry, even after the transport died or the session ended. In long-running sessions with MCP tools, this accumulates stale connections and never releases resources.

Fix: Added `closeMCPClient(clientId)` — closes the underlying transport (stdio process or HTTP/SSE connection) and removes entries from both `runningClients` and `listToolsCache`.

2. Stale Headers in Config Hash

`hashConfig()` did not include `config.headers` when computing the cache key for HTTP and SSE transports. Two MCP server configs with different auth tokens but the same URL were treated as the same client, returning a cached client with stale credentials.

Fix: Added `headers: config.headers` to the hash for both `http` and `sse` config types.

3. Automatic Cleanup on Tool Call Failure

When an MCP tool call failed (broken connection, dead stdio process), the dead client remained in the cache. Subsequent calls would reuse the same broken connection and fail with the same error.

Fix: In `sdk/src/run.ts`, the MCP tool call catch block now calls `closeMCPClient()` to evict the dead client so the next call reconnects with a fresh transport.

Files changed

  • `common/src/mcp/client.ts` — Fixed `hashConfig`, added `closeMCPClient` export
  • `sdk/src/run.ts` — Imported `closeMCPClient` and integrated into MCP error handling

Known limitations

  • `closeMCPClient` only evicts clients reactively (on tool call failure). There is no proactive liveness check in `getMCPClient` for transports that die silently (e.g., stdio process exits without a pending call). This means a stale entry could serve one bad request before being evicted. A follow-up could add a health check or TTL-based eviction.

- Fix hashConfig to include headers for HTTP/SSE configs, preventing
  stale auth tokens when two MCP servers differ only by headers.

- Add closeMCPClient() export to clean up client connections and
  cached tool listings, fixing the memory/connection leak where
  runningClients and listToolsCache grew without bound.

- Integrate automatic cleanup in SDK: when an MCP tool call fails,
  the dead client is evicted so the next call reconnects with a
  fresh transport instead of reusing the broken connection.
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