fix(upstream): detect a dead stdio transport and respawn the process (RC4-STDIO-001) - #1489
Merged
github-actions[bot] merged 3 commits intoOct 3, 2026
Merged
Conversation
…(RC4-STDIO-001)
Cause: when a stdio upstream's child process dies, mcp-go's stdio reader
hits EOF and closes the transport; every later request returns
transport.ErrTransportClosed ("transport error: transport closed").
The managed client's isConnectionError matched none of that text, so:
- the health-loop ping logged it as "timeout (high activity), ignoring"
and never flipped the state machine to Error;
- a failed tools/call took the "not a connection error" branch.
The server therefore kept reporting Ready/healthy with every call failing,
and the backoff reconnect (which only runs from StateError) never fired.
Fix:
- add isDeadTransportError (ErrTransportClosed, io.EOF/ErrUnexpectedEOF,
io.ErrClosedPipe, os.ErrClosed via errors.Is, plus text fallbacks for
flattened chains; bare "EOF" text deliberately not matched) and treat it
as a hard connection error in isConnectionError and as non-transient in
isTransientHealthCheckError, so one failed ping or call flips the server
to Error and the existing ShouldRetry backoff + tryReconnect respawns it.
- route the call-path verdict through recordDeadConnection, which applies
SetError only while the client is still Ready on the same connection
generation (under epochMu, like the ambiguous-call probe): a call racing
a deliberate Disconnect cannot flip it to Error, and a burst of calls on
one dead transport charges one retry instead of inflating the backoff.
Tests: classification table, health-ping and tools/call flips to Error,
burst/one-retry and post-disconnect guards, and an end-to-end test that
SIGKILLs a real cmd/mcpfixture stdio child and asserts the client leaves
Ready and reconnects to a new fixture instance (fails without the fix).
…verdict by epoch
Follow-ups from the zcode review of the RC4-STDIO-001 fix:
- isDeadTransportError no longer matches io.EOF / io.ErrUnexpectedEOF. The
classifier is shared by all protocols and net/http wraps those for a single
truncated/reset response on a live HTTP/SSE upstream, which must keep the
normal flap tolerance. A stdio child's death surfaces as ErrTransportClosed
(mcp-go's reader converts its EOF), so stdio coverage is unchanged.
- The text fallbacks ("transport closed", "file already closed", closed pipe)
now only count after mcp-go's "transport error: " prefix, so a live
server's JSON-RPC error message echoing errno text cannot evict it.
- performHealthCheck applies its SetError through the same epoch/epochMu
guard as the call path (setErrorIfCurrentConnection): a ping that failed
because Disconnect closed the transport no longer flips the freshly Reset
client to Error.
Tests: HTTP-shaped EOF chains and JSON-RPC errors echoing the markers are
not dead transports; HTTP EOF ping stays Ready; tool error echoing
"file already closed" stays Ready; ping racing Disconnect stays
Disconnected with retryCount 0. All fail on the previous commit.
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 37143826254 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
opencode review of RC4-STDIO-001: - ListTools, the tool-count refresh and GetPrompt now record a connection error through setErrorIfCurrentConnection with the epoch captured before the request, like the call and health paths: concurrent failures on one dead transport mark it once, and a failure from a connection Disconnect already replaced never flips the reset client to Error. - tools/call reads its connection epoch after the admission wait, so a call queued across a reconnect is charged to the connection it actually ran on instead of being discarded as stale. - the text fallback now matches only mcp-go's typed *transport.Error or a message that starts with its prefix, not server-supplied JSON-RPC text that merely contains the phrase. - test the epoch arm of the guard on its own.
Deploying mcpproxy-docs with
|
| Latest commit: |
0045fb9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96e155ce.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-fix-stdio-dead-transp.mcpproxy-docs.pages.dev |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
2 participants
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.
Pull Request
Description
Fixes RC4-STDIO-001, from the codex QA of v0.70.0-rc.4. The bug also exists in rc.2. When a stdio upstream's child process dies, every call fails with
transport error: transport closed, but REST keeps reporting the server connected, healthy and usable, and it is never respawned. Only a manual restart recovered it.Cause. After the child dies, mcp-go's stdio reader hits EOF and every request returns
transport.ErrTransportClosed.isConnectionErrorininternal/upstream/managed/client.gohad no marker for that error, so:tools/calltook the not-a-connection-error branch.SetErrorwas therefore never called, and the backoff reconnect, which only runs fromStateError, never fired.Fix (managed layer only):
isDeadTransportError: matchesErrTransportClosed,io.ErrClosedPipeandos.ErrClosedthrough the error chain. Its text fallback counts only when the text sits inside a wrappedtransport.Error, so a healthy server's JSON-RPC error text can't trigger it.io.EOF/io.ErrUnexpectedEOFare deliberately excluded, because HTTP/SSE transports wrap request EOFs that way on live connections.ShouldRetrybackoff andtryReconnectrespawn the process.recordDeadConnection) and the health path record the error only while the client is still Ready on the same connection generation, serialized underepochMu. A ping or call that races a deliberateDisconnectcan't flip a reset client to Error, and a burst of calls on one dead transport costs only one retry.Disconnectis never called from the health goroutine (no self-join), and backoff and the supervisor behave as before.Testing
I have tested these changes locally
I have added/updated tests that prove my fix is effective or my feature works
All existing tests pass
Tests:
dead_transport_test.gocovers the classification table (including negatives for JSON-RPC text and HTTP EOF), health check flips to Error, a call marks Error, a burst counts one retry, and a call after Disconnect stays Disconnected.dead_transport_fixture_test.gouses a realmcpfixturechild: SIGKILL, assert it leaves Ready, assert it respawns to a newinstance_id.Race suites:
go test -racepasses for./internal/upstream/...and./internal/runtime/supervisor/.... The fixture test passed with-count=3and-count=5.Live probe on a real core: within 5s of SIGKILLing the stdio child, REST shows
connected=false, unhealthy. By 30s a new child was spawned and the server was healthy again.Review: zcode round 1 raised three mediums (HTTP EOF, JSON-RPC text, health-path epoch). All are fixed in
603614463.Known low follow-ups, not in this PR:
runListToolsAsLeader,GetCachedToolCountandGetPromptstill callSetErroron connection errors with no epoch guard.recordDeadConnectionguard has no dedicated test.🤖 Generated with Claude Code