Skip to content

Commit d963178

Browse files
committed
fix(diagnostics): gate stdio premature-exit enrichment to stdio transport
Codex review round on PR #606: initialize() is shared by HTTP/SSE transports, so a remote server closing the connection was mislabeled 'server process exited before completing the MCP initialize handshake'. Gate the enrichment to the stdio transport via shouldEnrichStdioPrematureExit (HTTP/SSE keep generic diagnostics) + table test across transports. Related #599
1 parent 7e138a5 commit d963178

2 files changed

Lines changed: 41 additions & 8 deletions

File tree

internal/upstream/core/connection_lifecycle.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ func (c *Client) initialize(ctx context.Context) error {
6161
return fmt.Errorf("server did not respond to MCP initialize within %s and produced no stderr output (check that the command starts an MCP server and not a help banner)", waited)
6262
}
6363

64-
// Subprocess exited before completing the handshake: mcp-go reports a
65-
// closed transport / EOF on the pipe (not a typed exit error). Surface
66-
// the captured stderr — and the exit code when the process handle is
67-
// available — so the user sees the real, often self-serviceable cause
68-
// (e.g. "Error: --brave-api-key is required") instead of a bare
69-
// "transport closed" that the diagnostics layer marks UNKNOWN.
70-
// (MCP-1093 / #599)
71-
if isTransportClosedErr(err) {
64+
// STDIO subprocess exited before completing the handshake: mcp-go reports
65+
// a closed transport / EOF on the pipe (not a typed exit error). Surface
66+
// the captured stderr so the user sees the real, often self-serviceable
67+
// cause (e.g. "Error: --brave-api-key is required") instead of a bare
68+
// "transport closed" that the diagnostics layer marks UNKNOWN. Gated to
69+
// stdio: initialize() is shared by HTTP/SSE transports, which have no
70+
// local subprocess and must keep their generic diagnostics. (MCP-1093 /
71+
// #599; stdio gate per Codex review on PR #606)
72+
if shouldEnrichStdioPrematureExit(c.transportType, err) {
7273
return enrichTransportClosedError(c.formatRecentStderr(), err)
7374
}
7475

@@ -110,6 +111,15 @@ func (c *Client) initialize(ctx context.Context) error {
110111
// path mcp-go has not reaped the process (no Wait), so ProcessState is unset and
111112
// any exit code would be unreliable; the captured stderr is the actionable
112113
// signal. Surfacing the exit code is a separate follow-up.
114+
// shouldEnrichStdioPrematureExit reports whether an initialize() failure should
115+
// be enriched as a stdio subprocess premature exit. The enrichment is stdio-
116+
// specific (it describes a local "server process" and its stderr); HTTP/SSE
117+
// transports share initialize() but have no subprocess, so a closed-transport
118+
// error there must keep its generic diagnostics. (Codex review on PR #606)
119+
func shouldEnrichStdioPrematureExit(transportType string, err error) bool {
120+
return transportType == transportStdio && isTransportClosedErr(err)
121+
}
122+
113123
func enrichTransportClosedError(stderrBlock string, cause error) error {
114124
if stderrBlock != "" {
115125
return fmt.Errorf("server process exited before completing the MCP initialize handshake; recent stderr:\n%s: %w", stderrBlock, cause)

internal/upstream/core/connection_lifecycle_enrich_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,29 @@ func TestEnrichTransportClosedError_WithStderr(t *testing.T) {
2727
}
2828
}
2929

30+
// The stdio premature-exit enrichment must apply ONLY to the stdio transport —
31+
// HTTP/SSE share initialize() but have no subprocess, so a closed-transport error
32+
// there must keep generic diagnostics. (Codex review on PR #606)
33+
func TestShouldEnrichStdioPrematureExit_GatedToStdio(t *testing.T) {
34+
closed := errors.New("transport error: transport closed")
35+
cases := []struct {
36+
transport string
37+
err error
38+
want bool
39+
}{
40+
{transportStdio, closed, true},
41+
{transportStdio, io.EOF, true},
42+
{transportHTTP, closed, false},
43+
{transportSSE, closed, false},
44+
{transportStdio, errors.New("some unrelated error"), false},
45+
}
46+
for _, tc := range cases {
47+
if got := shouldEnrichStdioPrematureExit(tc.transport, tc.err); got != tc.want {
48+
t.Errorf("shouldEnrichStdioPrematureExit(%q, %v) = %v, want %v", tc.transport, tc.err, got, tc.want)
49+
}
50+
}
51+
}
52+
3053
func TestEnrichTransportClosedError_NoStderr(t *testing.T) {
3154
cause := errors.New("transport closed")
3255
got := enrichTransportClosedError("", cause)

0 commit comments

Comments
 (0)