fix: make transport Start() idempotent to resolve issue #583

client/client.go

@@ -86,15 +86,10 @@ func (c *Client) Start(ctx context.Context) error {
 		return fmt.Errorf("transport is nil")
 	}
 
-	if _, ok := c.transport.(*transport.Stdio); !ok {
-		// the stdio transport from NewStdioMCPClientWithOptions
-		// is already started, dont start again.
-		//
-		// Start the transport for other transport types
-		err := c.transport.Start(ctx)
-		if err != nil {
-			return err
-		}
+	// Start is idempotent - transports handle being called multiple times
+	err := c.transport.Start(ctx)
+	if err != nil {
+		return err
 	}
 
 	c.transport.SetNotificationHandler(func(notification mcp.JSONRPCNotification) {

client/issue583_test.go

@@ -0,0 +1,184 @@
+package client
+
+import (
+	"context"
+	"os"
+	"runtime"
+	"testing"
+	"time"
+
+	"github.com/mark3labs/mcp-go/client/transport"
+	"github.com/mark3labs/mcp-go/mcp"
+)
+
+// TestDirectTransportCreation tests the bug reported in issue #583
+// where using transport.NewStdioWithOptions directly followed by client.Start()
+// would fail with "stdio client not started" error
+func TestDirectTransportCreation(t *testing.T) {
+	tempFile, err := os.CreateTemp("", "mockstdio_server")
+	if err != nil {
+		t.Fatalf("Failed to create temp file: %v", err)
+	}
+	tempFile.Close()
+	mockServerPath := tempFile.Name()
+
+	if runtime.GOOS == "windows" {
+		os.Remove(mockServerPath)
+		mockServerPath += ".exe"
+	}
+
+	if compileErr := compileTestServer(mockServerPath); compileErr != nil {
+		t.Fatalf("Failed to compile mock server: %v", compileErr)
+	}
+	defer os.Remove(mockServerPath)
+
+	// This is the pattern from issue #583 that was broken
+	tport := transport.NewStdioWithOptions(mockServerPath, nil, nil)
+	cli := NewClient(tport)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	// This should work now (was failing before fix)
+	if err := cli.Start(ctx); err != nil {
+		t.Fatalf("client.Start() failed: %v", err)
+	}
+	defer cli.Close()
+
+	// Verify Initialize works (was failing with "stdio client not started")
+	initCtx, initCancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer initCancel()
+
+	request := mcp.InitializeRequest{}
+	request.Params.ProtocolVersion = mcp.LATEST_PROTOCOL_VERSION
+	request.Params.ClientInfo = mcp.Implementation{
+		Name:    "test-client",
+		Version: "1.0.0",
+	}
+
+	result, err := cli.Initialize(initCtx, request)
+	if err != nil {
+		t.Fatalf("Initialize failed: %v (this was the bug in issue #583)", err)
+	}
+
+	if result == nil {
+		t.Fatal("Expected result, got nil")
+	}
+}
+
+// TestNewStdioMCPClientWithOptions tests that the old pattern still works
+func TestNewStdioMCPClientWithOptions(t *testing.T) {
+	tempFile, err := os.CreateTemp("", "mockstdio_server")
+	if err != nil {
+		t.Fatalf("Failed to create temp file: %v", err)
+	}
+	tempFile.Close()
+	mockServerPath := tempFile.Name()
+
+	if runtime.GOOS == "windows" {
+		os.Remove(mockServerPath)
+		mockServerPath += ".exe"
+	}
+
+	if compileErr := compileTestServer(mockServerPath); compileErr != nil {
+		t.Fatalf("Failed to compile mock server: %v", compileErr)
+	}
+	defer os.Remove(mockServerPath)
+
+	// This pattern was already working
+	cli, err := NewStdioMCPClientWithOptions(mockServerPath, nil, nil)
+	if err != nil {
+		t.Fatalf("NewStdioMCPClientWithOptions failed: %v", err)
+	}
+	defer cli.Close()
+
+	// Calling Start() again should be idempotent (no error)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	if err := cli.Start(ctx); err != nil {
+		t.Fatalf("client.Start() should be idempotent, got error: %v", err)
+	}
+
+	// Verify Initialize works
+	initCtx, initCancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer initCancel()
+
+	request := mcp.InitializeRequest{}
+	request.Params.ProtocolVersion = mcp.LATEST_PROTOCOL_VERSION
+	request.Params.ClientInfo = mcp.Implementation{
+		Name:    "test-client",
+		Version: "1.0.0",
+	}
+
+	result, err := cli.Initialize(initCtx, request)
+	if err != nil {
+		t.Fatalf("Initialize failed: %v", err)
+	}
+
+	if result == nil {
+		t.Fatal("Expected result, got nil")
+	}
+}
+
+// TestMultipleClientStartCalls tests that calling client.Start() multiple times is safe
+func TestMultipleClientStartCalls(t *testing.T) {
+	tempFile, err := os.CreateTemp("", "mockstdio_server")
+	if err != nil {
+		t.Fatalf("Failed to create temp file: %v", err)
+	}
+	tempFile.Close()
+	mockServerPath := tempFile.Name()
+
+	if runtime.GOOS == "windows" {
+		os.Remove(mockServerPath)
+		mockServerPath += ".exe"
+	}
+
+	if compileErr := compileTestServer(mockServerPath); compileErr != nil {
+		t.Fatalf("Failed to compile mock server: %v", compileErr)
+	}
+	defer os.Remove(mockServerPath)
+
+	tport := transport.NewStdioWithOptions(mockServerPath, nil, nil)
+	cli := NewClient(tport)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	// First Start()
+	if err := cli.Start(ctx); err != nil {
+		t.Fatalf("First client.Start() failed: %v", err)
+	}
+	defer cli.Close()
+
+	// Second Start() - should be idempotent
+	if err := cli.Start(ctx); err != nil {
+		t.Errorf("Second client.Start() should be idempotent, got error: %v", err)
+	}
+
+	// Third Start() - should still be idempotent
+	if err := cli.Start(ctx); err != nil {
+		t.Errorf("Third client.Start() should be idempotent, got error: %v", err)
+	}
+
+	// Verify client still works
+	initCtx, initCancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer initCancel()
+
+	request := mcp.InitializeRequest{}
+	request.Params.ProtocolVersion = mcp.LATEST_PROTOCOL_VERSION
+	request.Params.ClientInfo = mcp.Implementation{
+		Name:    "test-client",
+		Version: "1.0.0",
+	}
+
+	result, err := cli.Initialize(initCtx, request)
+	if err != nil {
+		t.Fatalf("Initialize failed after multiple Start() calls: %v", err)
+	}
+
+	if result == nil {
+		t.Fatal("Expected result, got nil")
+	}
+}

client/transport/idempotent_all_transports_test.go

@@ -0,0 +1,111 @@
+package transport
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/mark3labs/mcp-go/server"
+)
+
+// TestSSE_StartIdempotency tests that SSE Start() is idempotent
+func TestSSE_StartIdempotency(t *testing.T) {
+	t.Skip("SSE requires a real HTTP server - tested in integration tests")
+}
+
+// TestStreamableHTTP_StartIdempotency tests that StreamableHTTP Start() is idempotent
+func TestStreamableHTTP_StartIdempotency(t *testing.T) {
+	client, err := NewStreamableHTTP("http://localhost:8080")
+	if err != nil {
+		t.Fatalf("Failed to create StreamableHTTP client: %v", err)
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	defer cancel()
+
+	// First Start() - should succeed
+	err = client.Start(ctx)
+	if err != nil {
+		t.Fatalf("First Start() failed: %v", err)
+	}
+	defer client.Close()
+
+	// Second Start() - should be idempotent (no error)
+	err = client.Start(ctx)
+	if err != nil {
+		t.Errorf("Second Start() should be idempotent, got error: %v", err)
+	}
+
+	// Third Start() - should still be idempotent
+	err = client.Start(ctx)
+	if err != nil {
+		t.Errorf("Third Start() should be idempotent, got error: %v", err)
+	}
+}
+
+// TestInProcessTransport_StartIdempotency tests that InProcess Start() is idempotent
+func TestInProcessTransport_StartIdempotency(t *testing.T) {
+	mcpServer := server.NewMCPServer(
+		"test-server",
+		"1.0.0",
+	)
+
+	transport := NewInProcessTransport(mcpServer)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	defer cancel()
+
+	// First Start() - should succeed
+	err := transport.Start(ctx)
+	if err != nil {
+		t.Fatalf("First Start() failed: %v", err)
+	}
+	defer transport.Close()
+
+	// Second Start() - should be idempotent (no error)
+	err = transport.Start(ctx)
+	if err != nil {
+		t.Errorf("Second Start() should be idempotent, got error: %v", err)
+	}
+
+	// Third Start() - should still be idempotent
+	err = transport.Start(ctx)
+	if err != nil {
+		t.Errorf("Third Start() should be idempotent, got error: %v", err)
+	}
+}
+
+// TestInProcessTransport_StartFailureReset tests that a failed Start() can be retried
+func TestInProcessTransport_StartFailureReset(t *testing.T) {
+	// This test verifies that if Start() fails, the started flag is reset
+	// and Start() can be called again
+
+	// For InProcessTransport, Start() only fails if session registration fails
+	// which is hard to simulate without mocking the server
+	// So we just verify that multiple successful starts work
+	mcpServer := server.NewMCPServer(
+		"test-server",
+		"1.0.0",
+	)
+
+	transport := NewInProcessTransport(mcpServer)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
+	defer cancel()
+
+	// Should be able to start successfully
+	err := transport.Start(ctx)
+	if err != nil {
+		t.Fatalf("Start() failed: %v", err)
+	}
+	defer transport.Close()
+
+	// Verify started flag is set
+	transport.startedMu.Lock()
+	started := transport.started
+	transport.startedMu.Unlock()
+
+	if !started {
+		t.Error("Started flag should be true after successful Start()")
+	}
+}

client/transport/inprocess.go

@@ -19,6 +19,8 @@ type InProcessTransport struct {
 
 	onNotification func(mcp.JSONRPCNotification)
 	notifyMu       sync.RWMutex
+	started        bool
+	startedMu      sync.Mutex
 }
 
 type InProcessOption func(*InProcessTransport)
@@ -55,10 +57,21 @@ func NewInProcessTransportWithOptions(server *server.MCPServer, opts ...InProces
 }
 
 func (c *InProcessTransport) Start(ctx context.Context) error {
+	c.startedMu.Lock()
+	if c.started {
+		c.startedMu.Unlock()
+		return nil
+	}
+	c.started = true
+	c.startedMu.Unlock()
+
 	// Create and register session if we have handlers
 	if c.samplingHandler != nil || c.elicitationHandler != nil {
 		c.session = server.NewInProcessSessionWithHandlers(c.sessionID, c.samplingHandler, c.elicitationHandler)
 		if err := c.server.RegisterSession(ctx, c.session); err != nil {
+			c.startedMu.Lock()
+			c.started = false
+			c.startedMu.Unlock()
 			return fmt.Errorf("failed to register session: %w", err)
 		}
 	}

client/transport/sse.go

@@ -115,7 +115,7 @@ func NewSSE(baseURL string, options ...ClientOption) (*SSE, error) {
 // Returns an error if the connection fails or times out waiting for the endpoint.
 func (c *SSE) Start(ctx context.Context) error {
 	if c.started.Load() {
-		return fmt.Errorf("has already started")
+		return nil
 	}
 
 	ctx, cancel := context.WithCancel(ctx)

client/transport/stdio.go

@@ -40,6 +40,8 @@ type Stdio struct {
 	ctx            context.Context
 	ctxMu          sync.RWMutex
 	logger         util.Logger
+	started        bool
+	startedMu      sync.Mutex
 }
 
 // StdioOption defines a function that configures a Stdio transport instance.
@@ -124,12 +126,23 @@ func NewStdioWithOptions(
 }
 
 func (c *Stdio) Start(ctx context.Context) error {
+	c.startedMu.Lock()
+	if c.started {
+		c.startedMu.Unlock()
+		return nil
+	}
+	c.started = true
+	c.startedMu.Unlock()
+
 	// Store the context for use in request handling
 	c.ctxMu.Lock()
 	c.ctx = ctx
 	c.ctxMu.Unlock()
 
 	if err := c.spawnCommand(ctx); err != nil {
+		c.startedMu.Lock()
+		c.started = false
+		c.startedMu.Unlock()
 		return err
 	}