fix(mcp): drain stderr pipe, limit spawn concurrency, add retry with backoff#37634
Open
S23Web3 wants to merge 2 commits into
Open
fix(mcp): drain stderr pipe, limit spawn concurrency, add retry with backoff#37634S23Web3 wants to merge 2 commits into
S23Web3 wants to merge 2 commits into
Conversation
…backoff Three fixes for stdio MCP server connection failures (-32000: Connection closed): 1. **Drain stderr PassThrough stream**: The MCP SDK's StdioClientTransport creates a PassThrough stream for stderr (when stderr: 'pipe') but never reads from it. On Windows, the 4KB pipe buffer fills up within seconds for chatty MCP servers, blocking or killing the child process. Fix: attach a no-op data handler to transport.stderr after creation. 2. **Limit spawn concurrency**: Changed concurrency from 'unbounded' to 4 for MCP server initialization. Spawning all servers simultaneously creates a process stampede that causes resource contention and intermittent failures, especially on Windows where process creation is more expensive. 3. **Add retry with exponential backoff**: connectLocal now retries up to 2 additional times with 1s/2s backoff. A fresh transport is created per attempt since connectTransport closes the transport on failure. Root cause analysis: - MCP SDK StdioClientTransport (stdio.js:60-106): PassThrough created at line 61, child stderr piped to it at line 105, but no consumer attached - OpenCode connectLocal (index.ts:348): stderr: 'pipe' requested but stream never drained - Windows pipe buffer: 4KB default, fills in ~100ms for servers that log to stderr during startup (e.g., GitNexus indexes 16 repos) Fixes: -32000: Connection closed errors for stdio MCP servers
github-actions
Bot
added
needs:compliance
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves reliability of local stdio-based MCP server connections in packages/opencode by preventing stderr pipe backpressure, reducing process spawn stampedes, and retrying transient connection failures with backoff.
Changes:
- Add a retry loop for local MCP stdio connections with exponential backoff and fresh transport per attempt.
- Drain the
StdioClientTransportpiped stderr stream to avoid OS pipe-buffer deadlocks (notably on Windows). - Limit Effect
forEachconcurrency from unbounded to 4 for MCP server creation and shutdown.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+352
to
+356
| if (attempt > 0) { | ||
| // Exponential backoff: 1s, 2s | ||
| yield* Effect.sleep(`${Math.pow(2, attempt - 1)}s`) | ||
| yield* Effect.logWarning("Retrying MCP connection", { key, attempt }) | ||
| } |
Author
There was a problem hiding this comment.
Good catch. The codebase uses "X millis" format (e.g., shell.ts: Effect.sleep(\${input.timeout + 100} millis`)`). Pushing a fix now to match.
github-actions
Bot
removed
needs:compliance
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #16449
Type of change
What does this PR do?
Three fixes for stdio MCP server connection failures (
-32000: Connection closed), primarily affecting Windows:transport.stderr?.on("data", () => {})-32000.concurrency: 'unbounded'→concurrency: 4The stderr drain is the primary fix. The MCP SDK (
StdioClientTransportinstdio.js) creates aPassThroughstream for stderr whenstderr: 'pipe'is set, but nothing reads from it. On Windows, the 4KB pipe buffer fills up within seconds for chatty servers, blocking the child process.How did you verify your code works?
Reproduced by running OpenCode with a GitNexus MCP server that indexes 16 repos (heavy stderr output during startup). Before:
-32000: Connection closedwithin 2 seconds. After: stable connection for 5+ minutes.Screenshots / recordings
N/A — backend change
Checklist