fix: add MCP initialize handshake to mcpcurl by ra-n-dom · Pull Request #2009 · github/github-mcp-server

ra-n-dom · 2026-02-13T04:07:51Z

mcpcurl was sending tools/list and tools/call requests without first performing the MCP initialize handshake, causing the server to silently reject all requests and discover zero tools.

Before:

$ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help
(no tools listed)

After:

$ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help
Available Commands:
  add_comment_to_pending_review ...
  add_issue_comment ...
  create_branch ...

Summary

Added MCP initialize handshake (initialize → read response → notifications/initialized) before sending actual requests
Switched from buffered stdout to line-by-line scanning via bufio.Scanner for proper JSON-RPC message handling
Added readJSONRPCResponse to skip server-initiated notifications interleaved with responses
Ignore server exit code after stdin close (expected EOF)
Added 7 unit tests in cmd/mcpcurl/main_test.go

Why

mcpcurl has never worked — it silently discovers zero tools because the MCP protocol requires an initialization handshake before any requests. The server rejects pre-handshake requests but mcpcurl swallowed the error.

What changed

cmd/mcpcurl/main.go: Rewrote executeServerCommand() to perform MCP handshake; added buildInitializeRequest(), buildInitializedNotification(), readJSONRPCResponse()
cmd/mcpcurl/main_test.go: New file with 7 unit tests

MCP impact

No tool or API changes

Security / limits

No security or limits impact

Tool renaming

I am not renaming tools as part of this PR

Lint & tests

go test -v ./cmd/mcpcurl/ — 7/7 passing
go vet ./cmd/mcpcurl/ — clean

Copilot

Pull request overview

This PR updates mcpcurl to perform the required MCP initialization handshake before issuing requests (e.g., tools/list), so the stdio server will accept and respond to requests.

Changes:

Added an MCP initialize → notifications/initialized handshake sequence before sending the main JSON-RPC request.
Switched stdout handling from buffering to line-based reading via bufio.Scanner.
Added helpers to construct the initialize request and initialized notification.

Copilot · 2026-02-13T04:12:17Z

cmd/mcpcurl/main.go

+	// Step 2: Read initialize response
+	if !scanner.Scan() {
+		scanErr := scanner.Err()
+		return "", fmt.Errorf("failed to read initialize response: %v, stderr: %s", scanErr, stderr.String())
+	}
+	// Initialize response is discarded — we only need the handshake to complete
+


The handshake reads exactly one stdout line and assumes it is the initialize response, then discards it without validating JSON-RPC id or checking for an error. If the server emits notifications (or any other message) before the response, this will desynchronize the protocol and the subsequent requests may still be rejected. Consider parsing messages and looping until you receive a response object with the expected id (skipping notifications), and fail fast if the initialize response contains an error.

Addressed — replaced single-line reads with readJSONRPCResponse() which loops over lines, skips notifications (messages without an id field), and returns the first actual JSON-RPC response. See the updated force push.

Copilot · 2026-02-13T04:12:18Z

cmd/mcpcurl/main.go

+	// Step 5: Read the actual response
+	if !scanner.Scan() {
+		scanErr := scanner.Err()
+		return "", fmt.Errorf("failed to read response: %v, stderr: %s", scanErr, stderr.String())
+	}
+	response := scanner.Text()


The actual request path also reads a single stdout line and treats it as the response. This will break if the server interleaves notifications/log messages with responses or if the first line corresponds to a different request. Parse and skip notifications, and match the JSON-RPC response id to the request you sent (returning an error if a response contains an error object).

Same fix as above — both the initialize and actual request reads now go through readJSONRPCResponse() which skips interleaved notifications.

Copilot · 2026-02-13T04:12:18Z

cmd/mcpcurl/main.go

 	// Start the command
 	if err := cmd.Start(); err != nil {
 		return "", fmt.Errorf("failed to start command: %w", err)
 	}


After cmd.Start() succeeds, several later error returns (e.g., build/write/scan failures) return without waiting for or terminating the child process, which can leave a server process running in the background. Add cleanup on early returns (close stdin, kill the process if needed, and Wait()/reap it) so failures don’t leak processes.

Added a cleanup() closure after cmd.Start() that closes stdin and waits on the process. All error paths now call it before returning.

Copilot · 2026-02-13T04:12:18Z

cmd/mcpcurl/main.go

+	_ = stdin.Close()
 	if err := cmd.Wait(); err != nil {
 		return "", fmt.Errorf("command failed: %w, stderr: %s", err, stderr.String())
 	}


PR description mentions ignoring the server exit code after stdin close (expected EOF), but cmd.Wait() errors are still treated as fatal here. If the stdio server exits non-zero on EOF, mcpcurl will still fail even after successfully reading the response; adjust the wait/error handling to ignore the expected EOF-related exit condition (and keep unexpected failures surfaced).

Fixed — cmd.Wait() error is now ignored since the server exits non-zero on stdin EOF, which is expected. The response is already captured at that point.

Copilot · 2026-02-13T04:12:18Z

cmd/mcpcurl/main.go

+// executeServerCommand runs the specified command, performs the MCP initialization
+// handshake, sends the JSON request to stdin, and returns the response from stdout.
 func executeServerCommand(cmdStr, jsonRequest string) (string, error) {


PR description says it adds 7 unit tests in cmd/mcpcurl/main_test.go, but there is no such file in this PR/repo state. If tests are intended, they need to be included (and ideally cover the initialize handshake + notification interleaving behavior); otherwise the PR description should be updated to match what’s actually changed.

Test file was missing from the initial push. It's included in the force push — 7 unit tests covering readJSONRPCResponse, buildInitializeRequest, and buildInitializedNotification.

mcpcurl was sending tools/list and tools/call requests without first performing the MCP initialize handshake, causing the server to silently reject all requests and discover zero tools. Before: $ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help (no tools listed) After: $ mcpcurl --stdio-server-cmd "github-mcp-server stdio" tools --help Available Commands: add_comment_to_pending_review ... add_issue_comment ... create_branch ...

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot · 2026-02-13T14:57:02Z

cmd/mcpcurl/main.go

+func readJSONRPCResponse(scanner *bufio.Scanner) (string, error) {
+	for scanner.Scan() {
+		line := scanner.Text()
+		// JSON-RPC responses have an "id" field; notifications do not.
+		var msg map[string]json.RawMessage
+		if err := json.Unmarshal([]byte(line), &msg); err != nil {
+			return "", fmt.Errorf("failed to parse JSON-RPC message: %w", err)
+		}
+		if _, hasID := msg["id"]; hasID {
+			return line, nil
+		}
+		// No "id" — this is a notification, skip it
+	}
+	if err := scanner.Err(); err != nil {
+		return "", err
+	}
+	return "", fmt.Errorf("unexpected end of output")
 }


The function correctly skips notifications by checking for the presence of an "id" field, but it doesn't check whether a JSON-RPC response contains an "error" field. According to the JSON-RPC 2.0 specification, responses can have either a "result" or an "error" field. If the server returns an error response (e.g., invalid method, authentication failure), this function will return it as a successful response, and downstream code may fail to parse it properly.

Consider checking for the "error" field and returning a descriptive error when present, so users get clear feedback when the server rejects a request.

SamMorrowDrums · 2026-02-13T18:28:45Z

cmd/mcpcurl/main.go


-	// Write the JSON request to stdin
+	// Ensure the child process is cleaned up on any error after Start()
+	cleanup := func() {


Hi, can't this just be deferred using defer keyword? Normally must execute functions can be cleanly deferred

drr13917-cpu · 2026-02-13T18:36:53Z

Outlook. Android<https://aka.ms/AAb9ysg>

________________________________ From: Sam Morrow ***@***.***> Sent: Friday, February 13, 2026 9:59:51 PM To: github/github-mcp-server ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [github/github-mcp-server] fix: add MCP initialize handshake to mcpcurl (PR #2009) @SamMorrowDrums commented on this pull request.

________________________________ In cmd/mcpcurl/main.go<#2009 (comment)>:

cmd.Stderr = &stderr

// Start the command if err := cmd.Start(); err != nil { return "", fmt.Errorf("failed to start command: %w", err) } - // Write the JSON request to stdin + // Ensure the child process is cleaned up on any error after Start() + cleanup := func() { Hi, can't this just be deferred using defer keyword? Normally must execute functions can be cleanly deferred — Reply to this email directly, view it on GitHub<#2009 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BVAHC3KNCKUCKLAY72FLTET4LYJZ7AVCNFSM6AAAAACU7MEUPOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTOOJYHAYDONRWGE>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

readJSONRPCResponse now checks for an "error" field in responses and returns a descriptive error instead of silently passing it through.

ra-n-dom requested a review from a team as a code owner February 13, 2026 04:07

Copilot AI review requested due to automatic review settings February 13, 2026 04:07

Copilot started reviewing on behalf of ra-n-dom February 13, 2026 04:08 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

ra-n-dom force-pushed the fix/mcpcurl-initialization branch from 6c92df9 to 5fd92a2 Compare February 13, 2026 13:37

ra-n-dom requested a review from Copilot February 13, 2026 14:53

Copilot started reviewing on behalf of ra-n-dom February 13, 2026 14:53 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

SamMorrowDrums reviewed Feb 13, 2026

View reviewed changes

fix: surface JSON-RPC error responses in mcpcurl

718b172

readJSONRPCResponse now checks for an "error" field in responses and returns a descriptive error instead of silently passing it through.

Conversation

ra-n-dom commented Feb 13, 2026

Summary

Why

What changed

MCP impact

Security / limits

Tool renaming

Lint & tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ra-n-dom Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ra-n-dom Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ra-n-dom Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ra-n-dom Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ra-n-dom Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

SamMorrowDrums Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

drr13917-cpu commented Feb 13, 2026 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants