Skip to content

fix(mcp): close existing client before reassignment to prevent leaks #8253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
rekram1-node merged 1 commit into from
Jan 13, 2026
Merged

fix(mcp): close existing client before reassignment to prevent leaks #8253

rekram1-node merged 1 commit into from
Jan 13, 2026
+14 −0

Conversation

@sauerdaniel
Copy link
Contributor

@sauerdaniel sauerdaniel commented Jan 13, 2026 •
edited
Loading

Summary

Fix memory leak in MCP client management where existing clients are orphaned when replaced.

Fixes #8257
Relates to #7261
Relates to #3013

Problem

In MCP.add() and the OAuth finishAuth() flow, when a new MCP client is created and assigned to s.clients[name], any existing client at that key is silently overwritten:

// Before (leaks existing client)
s.clients[name] = result.mcpClient

The old client is never closed, leaving:

  • Open connections/sockets
  • Active event listeners
  • Unreleased resources

This happens when:

  • Reconnecting to an MCP server
  • Completing OAuth flow for an already-connected server
  • Re-initializing MCP configuration

Solution

Check for and close existing clients before reassignment in both locations:

// Close existing client if present to prevent memory leaks
const existingClient = s.clients[name]
if (existingClient) {
  await existingClient.close().catch((error) => {
    log.error("Failed to close existing MCP client", { name, error })
  })
}
s.clients[name] = result.mcpClient

Changes

  • packages/opencode/src/mcp/index.ts: Added client cleanup in add() function
  • packages/opencode/src/mcp/index.ts: Added client cleanup in finishAuth() flow

Testing

  • ✅ Build passes for all platforms
  • ✅ Full test suite passes (652 tests, 0 failures)
  • Code-review verified to follow correct resource cleanup pattern

@sauerdaniel
fix(mcp): close existing client before reassignment to prevent leaks
393b70f 
When MCP.add() or finishAuth() assigns a new client to s.clients[name],
any existing client at that key is orphaned without being closed. This
causes connection/resource leaks over time.

Add existingClient.close() before reassignment in both locations to
properly clean up replaced clients.
@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2026

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2026

The following comment was made by an LLM, it may be inaccurate:

The search results show that PR #8253 (the current PR) is the only one directly addressing this specific issue. PR #7424 appears in the results as related work (force kill MCP processes), which the current PR already acknowledges as complementary.

No duplicate PRs found

@sauerdaniel
Copy link
Contributor Author

sauerdaniel commented Jan 13, 2026

Duplicate Analysis

The bot mentioned PR #7424 as potentially related since both modify mcp/index.ts. After investigation:

PR What it does Layer
#7424 Adds forceKillProcess() to kill orphaned MCP server child processes that don't terminate gracefully OS process cleanup
#8253 Closes existing MCP client objects before reassigning s.clients[name] JavaScript object cleanup

Verdict: Complementary, not duplicate.

These PRs address different cleanup layers:

Both fixes are valuable:

  1. When add() or finishAuth() is called for an existing MCP name, the old client reference gets overwritten
  2. Without fix(mcp): close existing client before reassignment to prevent leaks #8253, the old client object leaks (JS memory)
  3. Without fix: force kill MCP server processes on dispose to prevent orphan processes #7424, if that client's server process doesn't terminate, it becomes a zombie (OS resources)

The fixes are orthogonal and both contribute to complete MCP resource cleanup.

@rekram1-node rekram1-node merged commit 80e1173 into anomalyco:dev Jan 13, 2026
6 checks passed
dl-alexandre pushed a commit to dl-alexandre/opencode that referenced this pull request Jan 13, 2026
@sauerdaniel @dl-alexandre
fix(mcp): close existing client before reassignment to prevent leaks (a…
4fb4619 
…nomalyco#8253)
fwang pushed a commit that referenced this pull request Jan 14, 2026
@sauerdaniel @fwang
fix(mcp): close existing client before reassignment to prevent leaks (#…
4dc7266 
…8253)
@anandka anandka mentioned this pull request Jan 14, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory leak: MCP clients not closed before reassignment

2 participants

@sauerdaniel @rekram1-node