From e5f19db688fd56fa4fbbf62061a944ebff61347e Mon Sep 17 00:00:00 2001
From: ra-n-dom <129428390+ra-n-dom@users.noreply.github.com>
Date: Sun, 31 May 2026 11:18:56 +0200
Subject: [PATCH] review: switch cleanup to defer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per @SamMorrowDrums review — replace the manual cleanup() calls before
each error return with a single defer right after cmd.Start(). Same
behaviour, less code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 cmd/mcpcurl/main.go | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go
index 0dad1ea1..f40e8425 100644
--- a/cmd/mcpcurl/main.go
+++ b/cmd/mcpcurl/main.go
@@ -408,11 +408,13 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) {
 		return "", fmt.Errorf("failed to start command: %w", err)
 	}
 
-	// Ensure the child process is cleaned up on any error after Start()
-	cleanup := func() {
+	// Ensure the child process is cleaned up on every return path.
+	// stdin must be closed before Wait so the server sees EOF and exits;
+	// its non-zero exit status on EOF is expected, so we ignore the error.
+	defer func() {
 		_ = stdin.Close()
 		_ = cmd.Wait()
-	}
+	}()
 
 	// Use a scanner with a large buffer for reading JSON-RPC responses
 	scanner := bufio.NewScanner(stdoutPipe)
@@ -421,43 +423,33 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) {
 	// Step 1: Send MCP initialize request
 	initReq, err := buildInitializeRequest()
 	if err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to build initialize request: %w", err)
 	}
 	if _, err := io.WriteString(stdin, initReq+"\n"); err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to write initialize request: %w", err)
 	}
 
 	// Step 2: Read initialize response (skip any server notifications)
 	if _, err := readJSONRPCResponse(scanner); err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to read initialize response: %w, stderr: %s", err, stderr.String())
 	}
 
 	// Step 3: Send initialized notification
 	if _, err := io.WriteString(stdin, buildInitializedNotification()+"\n"); err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to write initialized notification: %w", err)
 	}
 
 	// Step 4: Send the actual request
 	if _, err := io.WriteString(stdin, jsonRequest+"\n"); err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to write request: %w", err)
 	}
 
 	// Step 5: Read the actual response (skip any server notifications)
 	response, err := readJSONRPCResponse(scanner)
 	if err != nil {
-		cleanup()
 		return "", fmt.Errorf("failed to read response: %w, stderr: %s", err, stderr.String())
 	}
 
-	// Close stdin and wait for process to exit. The server will see EOF and
-	// exit with a non-zero status, which is expected — we already have the response.
-	cleanup()
-
 	return response, nil
 }