From 73a96d30087b653f0548475b66e8f0e4271f0e13 Mon Sep 17 00:00:00 2001
From: Dayna Blackwell <blackwellsystems@protonmail.com>
Date: Tue, 28 Apr 2026 01:04:46 -0700
Subject: [PATCH] mcp: suppress subprocess exit error during graceful close

When ClientSession.Close() initiates graceful shutdown via stdin close,
the subprocess may exit with a non-zero status (e.g., gopls exits with
code 2 because it treats EOF on stdin as fatal). This exit error
propagated through pipeRWC.Close() -> closeErr -> ClientSession.Close(),
causing spurious "exit status 2" errors for callers.

The fix: in pipeRWC.Close(), after closing stdin and waiting for the
subprocess to exit, suppress *exec.ExitError. The subprocess exited
because we told it to (by closing stdin); the exit code is not
meaningful for the client. Errors from SIGTERM/SIGKILL paths are still
propagated, as those indicate the subprocess did not respond to graceful
shutdown.

This matches the analysis by @adonovan in golang/go#77336: "a bug in
mcp causing a race between the active part of Close (which breaks the
connection and terminates the child process) and the waiting part of
close (which seems to observe an error from the terminated child without
realizing that close itself was responsible)."

Fixes #855
---
 mcp/cmd.go      |  9 +++++++++
 mcp/cmd_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/mcp/cmd.go b/mcp/cmd.go
index b531eaf1..e6e83716 100644
--- a/mcp/cmd.go
+++ b/mcp/cmd.go
@@ -6,6 +6,7 @@ package mcp
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os/exec"
@@ -88,6 +89,14 @@ func (s *pipeRWC) Close() error {
 		return nil, false
 	}
 	if err, ok := wait(); ok {
+		// The subprocess exited after we closed stdin. A non-zero exit code
+		// is expected here: the server may exit with an error status because
+		// it observed EOF on stdin. This is part of normal graceful shutdown,
+		// not an error in the connection.
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) {
+			return nil
+		}
 		return err
 	}
 	// Note the condition here: if sending SIGTERM fails, don't wait and just
diff --git a/mcp/cmd_test.go b/mcp/cmd_test.go
index 7c98207a..fc993957 100644
--- a/mcp/cmd_test.go
+++ b/mcp/cmd_test.go
@@ -55,6 +55,7 @@ func TestMain(m *testing.M) {
 var serverFuncs = map[string]func(){
 	"default":       runServer,
 	"cancelContext": runCancelContextServer,
+	"exitNonZero":   runExitNonZeroServer,
 }
 
 func runServer() {
@@ -67,6 +68,19 @@ func runServer() {
 	}
 }
 
+// runExitNonZeroServer runs a server that exits with a non-zero status when the
+// connection is closed, simulating servers (like gopls) that treat EOF on stdin
+// as a fatal error. This exercises the fix for #855.
+func runExitNonZeroServer() {
+	ctx := context.Background()
+	server := mcp.NewServer(testImpl, nil)
+	mcp.AddTool(server, &mcp.Tool{Name: "greet", Description: "say hi"}, SayHi)
+	if err := server.Run(ctx, &mcp.StdioTransport{}); err != nil {
+		os.Exit(2) // non-zero exit, as gopls does
+	}
+	os.Exit(2) // also exit non-zero on clean shutdown to test the fix
+}
+
 func runCancelContextServer() {
 	ctx, done := signal.NotifyContext(context.Background(), syscall.SIGINT)
 	defer done()
@@ -229,6 +243,35 @@ func TestCmdTransport(t *testing.T) {
 	}
 }
 
+// TestCmdTransportNonZeroExit verifies that ClientSession.Close does not return
+// an error when the subprocess exits with a non-zero status during graceful
+// shutdown. This is a regression test for #855: servers like gopls may exit
+// non-zero when they observe EOF on stdin, and that should not be surfaced as
+// an error from Close.
+func TestCmdTransportNonZeroExit(t *testing.T) {
+	requireExec(t)
+
+	ctx := t.Context()
+	cmd := createServerCommand(t, "exitNonZero")
+
+	client := mcp.NewClient(&mcp.Implementation{Name: "client", Version: "v0.0.1"}, nil)
+	session, err := client.Connect(ctx, &mcp.CommandTransport{Command: cmd}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Make a call to ensure the session is fully established.
+	if _, err := session.CallTool(ctx, &mcp.CallToolParams{
+		Name:      "greet",
+		Arguments: map[string]any{"name": "user"},
+	}); err != nil {
+		t.Fatal(err)
+	}
+	// Close should succeed even though the server exits non-zero.
+	if err := session.Close(); err != nil {
+		t.Fatalf("closing server that exits non-zero: %v", err)
+	}
+}
+
 // createServerCommand creates a command to fork and exec the test binary as an
 // MCP server.
 //