fix(diagnostics): classify stdio exit-before-initialize; surface stderr + exit code (MCP-1093, #599) (#606)

Dumbris · Paperclip-Paperclip · web-flow · commit ebd513685c28 · 2026-06-05T15:42:40.000+03:00
* fix(diagnostics): classify stdio subprocess exit-before-initialize; surface stderr + exit code When an stdio upstream subprocess starts but exits before completing the MCP initialize handshake (e.g. a docker image that prints a fatal config error and dies, like mcp/brave-search with no BRAVE_API_KEY), mcp-go reports a closed transport. Previously this fell through to MCPX_UNKNOWN_UNCLASSIFIED with cause "transport error: transport closed", and the actionable child stderr was discarded — so the UI told users to "file a bug" for a self-serviceable config error. Recurring gap (cf. #483). - New code MCPX_STDIO_EXIT_BEFORE_INITIALIZE + catalog entry (registry.go). - classifyStdio: match "transport closed" / "exited before completing the mcp initialize" under the stdio transport hint → the new code (gated so a closed transport on another transport is not misattributed). - initialize(): on a closed-transport failure, enrich the error with the captured recent stderr tail and (best-effort) the child exit code so the real cause is visible in the UI banner and per-server logs. Exit code is best-effort: on stdio the process handle is only extracted after a successful initialize, so the stderr tail is the primary signal on the failure path. - Regression test (classifier_test.go): raw + enriched inputs classify to the new code and carry exit code + stderr tail; a non-stdio "transport closed" does not match. The UI banner surfaces whatever the classifier returns, so it improves automatically — no frontend change needed. Related MCP-1093, #599 Co-Authored-By: Paperclip <noreply@paperclip.ing> * fix(diagnostics): add error doc + production enrichment test for stdio early-exit Addresses Codex review on PR #606: - Add docs/errors/MCPX_STDIO_EXIT_BEFORE_INITIALIZE.md and the STDIO index entry in docs/errors/README.md (ENG-9 docs for the new public code). - Extract enrichTransportClosedError() from the initialize() premature-exit path and unit-test it (exit code + stderr tail + wrapped cause), replacing the hard-coded-string assertion that proved nothing about the production path. Related #599 * fix(diagnostics): scope stdio early-exit enrichment to stderr (drop unreliable exit code) Addresses Codex review round 2 on PR #606: childExitInfo() only had a code when ProcessState was set, but the stdio init-failure path returns before the process is reaped, so the exit code was essentially never present. Drop the exit-code from the enrichment, docs, and test; surface the captured stderr tail (the actionable cause). Exit-code capture is a separate follow-up. Related #599 * test(diagnostics): drop stale exit-code wording in stdio early-exit test Codex review round 3 on PR #606: the enrichment no longer surfaces an exit code (stderr-only scope), so update the test comment + case wording so readers don't infer exit-code surfacing is part of acceptance. Related #599 * 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 * fix(diagnostics): classify auto-detected stdio servers (resolved transport hint) Codex review round on PR #606: the supervisor built the classifier transport hint from raw Config.Protocol, so auto-detected stdio servers (Protocol=='' + Command!='') missed the stdio-gated rules and stayed UNKNOWN. Use transport.DetermineTransportType(config) at both hint sites so the resolved transport drives classification + add a DetermineTransportType regression test. Related #599 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
diff --git a/docs/errors/MCPX_STDIO_EXIT_BEFORE_INITIALIZE.md b/docs/errors/MCPX_STDIO_EXIT_BEFORE_INITIALIZE.md
@@ -0,0 +1,70 @@
+---
+id: MCPX_STDIO_EXIT_BEFORE_INITIALIZE
+title: MCPX_STDIO_EXIT_BEFORE_INITIALIZE
+sidebar_label: EXIT_BEFORE_INITIALIZE
+description: The stdio MCP server process exited before completing the MCP initialize handshake.
+---
+
+# `MCPX_STDIO_EXIT_BEFORE_INITIALIZE`
+
+**Severity:** error
+**Domain:** STDIO
+
+## What happened
+
+mcpproxy spawned the stdio server, but the process exited before completing the
+MCP `initialize` handshake. The underlying transport reports a closed pipe / EOF
+rather than a typed exit error, which previously surfaced as a generic
+`MCPX_UNKNOWN_UNCLASSIFIED`. This code makes the cause explicit, and mcpproxy now
+folds the last lines of the child's **stderr** into the error so you can see the
+real, usually self-serviceable problem.
+
+This almost always means a **missing or invalid configuration** — a required API
+key or environment variable, a bad argument, or a missing dependency — that makes
+the server print an error and exit immediately on startup.
+
+## Common causes
+
+- A required environment variable / API key is missing (e.g. a server that needs
+  `BRAVE_API_KEY` and exits with `Error: --brave-api-key is required`).
+- A required CLI flag/argument was not passed via `args`.
+- A Docker image is missing a mandatory `env` value (cf. `MCPX_STDIO_EXIT_NONZERO`).
+- The upstream package is not installed in the resolved runtime (`uvx`/`pipx`/`npx`).
+
+## How to fix
+
+### 1. Read the captured stderr
+
+The error banner and the per-server log already include the last stderr lines.
+To see more:
+
+```bash
+mcpproxy upstream logs <server-name> --tail 100
+```
+
+The last lines almost always contain the real cause (a missing-key message, a
+traceback, a "module not found", etc.).
+
+### 2. Supply the missing configuration
+
+Add the required environment variable / argument to the server entry, e.g.:
+
+```json
+{
+  "name": "brave-search",
+  "command": "npx",
+  "args": ["-y", "@modelcontextprotocol/server-brave-search"],
+  "env": { "BRAVE_API_KEY": "your-key-here" }
+}
+```
+
+### 3. Reproduce manually
+
+Run the exact `command`, `args`, and `env` in a normal shell; the server should
+start and wait for MCP input instead of exiting. Once it starts cleanly, restart
+the server in mcpproxy.
+
+## Related codes
+
+- [`MCPX_STDIO_EXIT_NONZERO`](MCPX_STDIO_EXIT_NONZERO.md) — exited non-zero before handshake
+- [`MCPX_STDIO_HANDSHAKE_TIMEOUT`](MCPX_STDIO_HANDSHAKE_TIMEOUT.md) — no `initialize` reply in time
diff --git a/docs/errors/README.md b/docs/errors/README.md
@@ -40,6 +40,7 @@ Run `mcpproxy doctor list-codes` for the machine-readable list.
 - [`MCPX_STDIO_SPAWN_ENOENT`](MCPX_STDIO_SPAWN_ENOENT.md) — command not found on PATH
 - [`MCPX_STDIO_SPAWN_EACCES`](MCPX_STDIO_SPAWN_EACCES.md) — permission denied executing command
 - [`MCPX_STDIO_EXIT_NONZERO`](MCPX_STDIO_EXIT_NONZERO.md) — server exited before handshake
+- [`MCPX_STDIO_EXIT_BEFORE_INITIALIZE`](MCPX_STDIO_EXIT_BEFORE_INITIALIZE.md) — process exited before `initialize` (captured stderr surfaced)
 - [`MCPX_STDIO_HANDSHAKE_TIMEOUT`](MCPX_STDIO_HANDSHAKE_TIMEOUT.md) — no `initialize` reply within 30s
 - [`MCPX_STDIO_HANDSHAKE_INVALID`](MCPX_STDIO_HANDSHAKE_INVALID.md) — malformed MCP frame
 
diff --git a/internal/diagnostics/classifier.go b/internal/diagnostics/classifier.go
@@ -186,6 +186,17 @@ func classifyStdio(err error, hints ClassifierHints) Code {
 			return STDIOSpawnENOENT
 		case strings.Contains(lmsg, "permission denied"):
 			return STDIOSpawnEACCES
+		// Subprocess started but the transport closed before the MCP initialize
+		// handshake completed — the child exited early (e.g. printed a fatal
+		// config error to stderr and died). mcp-go surfaces this as a closed
+		// transport, which otherwise falls through to MCPX_UNKNOWN_UNCLASSIFIED
+		// even though the real cause is on the child's stderr (MCP-1093 / #599).
+		// Gated on the stdio hint so a "transport closed" from another transport
+		// is not misattributed.
+		case strings.Contains(lmsg, "transport closed"),
+			strings.Contains(lmsg, "exited before completing the mcp initialize"),
+			strings.Contains(lmsg, "exited before the mcp initialize"):
+			return STDIOExitBeforeInitialize
 		case strings.Contains(lmsg, "did not respond to mcp initialize"),
 			strings.Contains(lmsg, "handshake timeout"):
 			return STDIOHandshakeTimeout
diff --git a/internal/diagnostics/classifier_test.go b/internal/diagnostics/classifier_test.go
@@ -16,6 +16,52 @@ func TestClassify_Nil(t *testing.T) {
 	}
 }
 
+// TestClassify_STDIO_ExitBeforeInitialize covers GitHub #599 / MCP-1093: a
+// subprocess that exits before completing the MCP initialize handshake must
+// classify to MCPX_STDIO_EXIT_BEFORE_INITIALIZE (not MCPX_UNKNOWN_UNCLASSIFIED).
+// The surfaced error carries the captured stderr tail (the exit code is not
+// reliably available on this path and is intentionally not surfaced).
+func TestClassify_STDIO_ExitBeforeInitialize(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+	}{
+		{
+			name: "raw transport-closed under stdio hint",
+			err:  errors.New(`stdio transport (command="docker", docker_isolation=true): transport error: transport closed`),
+		},
+		{
+			name: "enriched message carrying stderr tail",
+			err: errors.New("server process exited before completing the MCP initialize handshake; recent stderr:\n" +
+				"  | Error: --brave-api-key is required: transport closed"),
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := Classify(tc.err, ClassifierHints{Transport: "stdio"})
+			if got != STDIOExitBeforeInitialize {
+				t.Fatalf("Classify(%q) = %q, want %q", tc.err, got, STDIOExitBeforeInitialize)
+			}
+		})
+	}
+
+	// The production enrichment that folds the child exit code + stderr tail into
+	// the initialize-failure error is covered by TestEnrichTransportClosedError_*
+	// in internal/upstream/core — a real test of the helper the production path
+	// calls, instead of asserting against a hard-coded string here.
+	// (Codex review on PR #606)
+}
+
+// TestClassify_STDIO_ExitBeforeInitialize_NotForHTTP guards against over-match:
+// the same "transport closed" wording on a non-stdio transport must not be
+// captured by the stdio rule.
+func TestClassify_STDIO_ExitBeforeInitialize_NotForHTTP(t *testing.T) {
+	err := errors.New("transport error: transport closed")
+	if got := Classify(err, ClassifierHints{Transport: "http"}); got == STDIOExitBeforeInitialize {
+		t.Fatalf("HTTP transport must not classify as %q", STDIOExitBeforeInitialize)
+	}
+}
+
 func TestClassify_STDIO_SpawnENOENT(t *testing.T) {
 	// os/exec wraps ENOENT into *exec.Error when the binary is missing.
 	wrapped := &exec.Error{
diff --git a/internal/diagnostics/codes.go b/internal/diagnostics/codes.go
@@ -8,11 +8,12 @@ package diagnostics
 
 // STDIO domain — stdio-transport MCP server failures.
 const (
-	STDIOSpawnENOENT      Code = "MCPX_STDIO_SPAWN_ENOENT"
-	STDIOSpawnEACCES      Code = "MCPX_STDIO_SPAWN_EACCES"
-	STDIOExitNonzero      Code = "MCPX_STDIO_EXIT_NONZERO"
-	STDIOHandshakeTimeout Code = "MCPX_STDIO_HANDSHAKE_TIMEOUT"
-	STDIOHandshakeInvalid Code = "MCPX_STDIO_HANDSHAKE_INVALID"
+	STDIOSpawnENOENT          Code = "MCPX_STDIO_SPAWN_ENOENT"
+	STDIOSpawnEACCES          Code = "MCPX_STDIO_SPAWN_EACCES"
+	STDIOExitNonzero          Code = "MCPX_STDIO_EXIT_NONZERO"
+	STDIOExitBeforeInitialize Code = "MCPX_STDIO_EXIT_BEFORE_INITIALIZE"
+	STDIOHandshakeTimeout     Code = "MCPX_STDIO_HANDSHAKE_TIMEOUT"
+	STDIOHandshakeInvalid     Code = "MCPX_STDIO_HANDSHAKE_INVALID"
 )
 
 // OAUTH domain — OAuth 2.1 / PKCE flow failures.
diff --git a/internal/diagnostics/registry.go b/internal/diagnostics/registry.go
@@ -61,6 +61,16 @@ func seedSTDIO() {
 		},
 		DocsURL: docsURL(STDIOExitNonzero),
 	})
+	register(CatalogEntry{
+		Code:        STDIOExitBeforeInitialize,
+		Severity:    SeverityError,
+		UserMessage: "The stdio server process exited before completing the MCP initialize handshake. This usually means a missing/invalid configuration (e.g. a required API key or environment variable) — check the captured stderr for the exact cause.",
+		FixSteps: []FixStep{
+			{Type: FixStepButton, Label: "Show last server log lines", FixerKey: "stdio_show_last_logs"},
+			{Type: FixStepLink, Label: "Troubleshooting early exit", URL: docsURL(STDIOExitBeforeInitialize)},
+		},
+		DocsURL: docsURL(STDIOExitBeforeInitialize),
+	})
 	register(CatalogEntry{
 		Code:        STDIOHandshakeTimeout,
 		Severity:    SeverityError,
diff --git a/internal/runtime/supervisor/supervisor.go b/internal/runtime/supervisor/supervisor.go
@@ -15,6 +15,7 @@ import (
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/diagnostics"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/configsvc"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/stateview"
+	transportpkg "github.com/smart-mcp-proxy/mcpproxy-go/internal/transport"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/upstream/types"
 )
 
@@ -696,9 +697,12 @@ func (s *Supervisor) updateStateView(name string, state *ServerState) {
 				status.LastError = errorStr
 
 				// Spec 044: classify raw error into stable diagnostic code.
+				// Use the RESOLVED transport, not the raw Config.Protocol: an
+				// auto-detected stdio server has Protocol=="" + Command!="" and
+				// would otherwise miss the stdio-gated classifier rules (#599).
 				transport := ""
 				if state.Config != nil {
-					transport = string(state.Config.Protocol)
+					transport = transportpkg.DetermineTransportType(state.Config)
 				}
 				classifyAndAttach(status, state.ConnectionInfo.LastError, transport)
 				// Spec 044 Phase H: notify telemetry counter store.
@@ -969,9 +973,12 @@ func (s *Supervisor) updateSnapshotFromEvent(event Event) {
 						status.LastError = errorStr
 
 						// Spec 044: classify raw error into stable diagnostic code.
+						// Resolved transport (not raw Config.Protocol) so auto-detected
+						// stdio servers (Protocol=="" + Command!="") hit the stdio
+						// classifier rules (#599).
 						transport := ""
 						if status.Config != nil {
-							transport = string(status.Config.Protocol)
+							transport = transportpkg.DetermineTransportType(status.Config)
 						}
 						classifyAndAttach(status, connInfo.LastError, transport)
 						// Spec 044 Phase H: notify telemetry counter store.
diff --git a/internal/transport/transport_type_test.go b/internal/transport/transport_type_test.go
@@ -0,0 +1,33 @@
+package transport
+
+import (
+	"testing"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+)
+
+// Locks the behavior the supervisor classifier hint relies on (#599 / PR #606):
+// an auto-detected stdio server (empty/auto protocol + a command) must resolve to
+// stdio, so the stdio-gated diagnostic rules fire for it — using the raw
+// Config.Protocol would leave such servers unclassified.
+func TestDetermineTransportType(t *testing.T) {
+	cases := []struct {
+		name string
+		cfg  config.ServerConfig
+		want string
+	}{
+		{"explicit stdio", config.ServerConfig{Protocol: "stdio"}, TransportStdio},
+		{"auto-detected stdio (empty protocol + command)", config.ServerConfig{Command: "npx"}, TransportStdio},
+		{"auto protocol + command", config.ServerConfig{Protocol: "auto", Command: "docker"}, TransportStdio},
+		{"empty protocol + url", config.ServerConfig{URL: "https://example.com/mcp"}, TransportStreamableHTTP},
+		{"explicit http", config.ServerConfig{Protocol: "http", URL: "https://example.com/mcp"}, "http"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := tc.cfg
+			if got := DetermineTransportType(&cfg); got != tc.want {
+				t.Errorf("DetermineTransportType(%+v) = %q, want %q", tc.cfg, got, tc.want)
+			}
+		})
+	}
+}
diff --git a/internal/upstream/core/connection_lifecycle.go b/internal/upstream/core/connection_lifecycle.go
@@ -5,6 +5,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
+	"strings"
 	"time"
 
 	"github.com/mark3labs/mcp-go/mcp"
@@ -59,6 +61,18 @@ func (c *Client) initialize(ctx context.Context) error {
 			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)
 		}
 
+		// STDIO subprocess exited before completing the handshake: mcp-go reports
+		// a closed transport / EOF on the pipe (not a typed exit error). Surface
+		// the captured stderr so the user sees the real, often self-serviceable
+		// cause (e.g. "Error: --brave-api-key is required") instead of a bare
+		// "transport closed" that the diagnostics layer marks UNKNOWN. Gated to
+		// stdio: initialize() is shared by HTTP/SSE transports, which have no
+		// local subprocess and must keep their generic diagnostics. (MCP-1093 /
+		// #599; stdio gate per Codex review on PR #606)
+		if shouldEnrichStdioPrematureExit(c.transportType, err) {
+			return enrichTransportClosedError(c.formatRecentStderr(), err)
+		}
+
 		return err
 	}
 
@@ -85,6 +99,50 @@ func (c *Client) initialize(ctx context.Context) error {
 	return nil
 }
 
+// enrichTransportClosedError builds the actionable error for a stdio subprocess
+// that exited before completing the MCP initialize handshake, folding in the
+// captured stderr tail so the UI banner and per-server logs show the real,
+// often self-serviceable cause (e.g. a missing API key) instead of a bare
+// "transport closed". It wraps the original cause with %w so callers can still
+// errors.Is/As it. Pure (no receiver state) so the production enrichment path is
+// unit-testable. (MCP-1093 / #599)
+//
+// Note: the child exit code is intentionally NOT surfaced here. On this failure
+// path mcp-go has not reaped the process (no Wait), so ProcessState is unset and
+// any exit code would be unreliable; the captured stderr is the actionable
+// signal. Surfacing the exit code is a separate follow-up.
+// shouldEnrichStdioPrematureExit reports whether an initialize() failure should
+// be enriched as a stdio subprocess premature exit. The enrichment is stdio-
+// specific (it describes a local "server process" and its stderr); HTTP/SSE
+// transports share initialize() but have no subprocess, so a closed-transport
+// error there must keep its generic diagnostics. (Codex review on PR #606)
+func shouldEnrichStdioPrematureExit(transportType string, err error) bool {
+	return transportType == transportStdio && isTransportClosedErr(err)
+}
+
+func enrichTransportClosedError(stderrBlock string, cause error) error {
+	if stderrBlock != "" {
+		return fmt.Errorf("server process exited before completing the MCP initialize handshake; recent stderr:\n%s: %w", stderrBlock, cause)
+	}
+	return fmt.Errorf("server process exited before completing the MCP initialize handshake and produced no stderr output (transport closed before the handshake): %w", cause)
+}
+
+// isTransportClosedErr reports whether an initialize() failure indicates the
+// child process went away mid-handshake. mcp-go surfaces a premature stdio
+// exit as a closed transport / EOF on the pipe rather than a typed exit error,
+// so we match those shapes to distinguish "the process died" from a genuine
+// malformed-handshake response. (MCP-1093 / #599)
+func isTransportClosedErr(err error) bool {
+	if errors.Is(err, io.EOF) {
+		return true
+	}
+	lmsg := strings.ToLower(err.Error())
+	return strings.Contains(lmsg, "transport closed") ||
+		strings.Contains(lmsg, "broken pipe") ||
+		strings.Contains(lmsg, "file already closed") ||
+		strings.Contains(lmsg, "use of closed")
+}
+
 // registerNotificationHandler registers a handler for MCP notifications.
 // This should be called after client.Start() and initialize() succeed.
 // It handles notifications/tools/list_changed to trigger reactive tool discovery.
diff --git a/internal/upstream/core/connection_lifecycle_enrich_test.go b/internal/upstream/core/connection_lifecycle_enrich_test.go
