Merge remote-tracking branch 'upstream/main' into fix/issue_write-body-description-2410

Author: jluocsa

README.md

@@ -1163,7 +1163,7 @@ The following sets of tools are available:
   - `owner`: Repository owner (string, required)
   - `pullNumber`: Pull request number to update (number, required)
   - `repo`: Repository name (string, required)
-  - `reviewers`: GitHub usernames to request reviews from (string[], optional)
+  - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], optional)
   - `state`: New state (string, optional)
   - `title`: New title (string, optional)
 
@@ -1221,7 +1221,7 @@ The following sets of tools are available:
 
 - **get_commit** - Get commit details
   - **Required OAuth Scopes**: `repo`
-  - `include_diff`: Whether to include file diffs and stats in the response. Default is true. (boolean, optional)
+  - `detail`: Level of detail to include for changed files. "none" omits stats and files entirely. "stats" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. "full_patch" additionally includes the unified diff content for each file and can be very large. (string, optional)
   - `owner`: Repository owner (string, required)
   - `page`: Page number for pagination (min 1) (number, optional)
   - `perPage`: Results per page for pagination (min 1, max 100) (number, optional)

cmd/mcpcurl/main.go

@@ -1,7 +1,7 @@
 package main
 
 import (
-	"bytes"
+	"bufio"
 	"crypto/rand"
 	"encoding/json"
 	"fmt"
@@ -376,8 +376,8 @@ func buildJSONRPCRequest(method, toolName string, arguments map[string]any) (str
 	return string(jsonData), nil
 }
 
-// executeServerCommand runs the specified command, sends the JSON request to stdin,
-// and returns the response from stdout
+// 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) {
 	// Split the command string into command and arguments
 	cmdParts := strings.Fields(cmdStr)
@@ -393,28 +393,119 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) {
 		return "", fmt.Errorf("failed to create stdin pipe: %w", err)
 	}
 
-	// Setup stdout and stderr pipes
-	var stdout, stderr bytes.Buffer
-	cmd.Stdout = &stdout
+	// Setup stdout pipe for line-by-line reading
+	stdoutPipe, err := cmd.StdoutPipe()
+	if err != nil {
+		return "", fmt.Errorf("failed to create stdout pipe: %w", err)
+	}
+
+	// Stderr still uses a buffer
+	var stderr strings.Builder
 	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 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)
+	scanner.Buffer(make([]byte, 0, 1024*1024), 1024*1024) // 1MB max line size
+
+	// Step 1: Send MCP initialize request
+	initReq, err := buildInitializeRequest()
+	if err != nil {
+		return "", fmt.Errorf("failed to build initialize request: %w", err)
+	}
+	if _, err := io.WriteString(stdin, initReq+"\n"); err != nil {
+		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 {
+		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 {
+		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 {
-		return "", fmt.Errorf("failed to write to stdin: %w", err)
+		return "", fmt.Errorf("failed to write request: %w", err)
 	}
-	_ = stdin.Close()
 
-	// Wait for the command to complete
-	if err := cmd.Wait(); err != nil {
-		return "", fmt.Errorf("command failed: %w, stderr: %s", err, stderr.String())
+	// Step 5: Read the actual response (skip any server notifications)
+	response, err := readJSONRPCResponse(scanner)
+	if err != nil {
+		return "", fmt.Errorf("failed to read response: %w, stderr: %s", err, stderr.String())
 	}
 
-	return stdout.String(), nil
+	return response, nil
+}
+
+// buildInitializeRequest creates the MCP initialize handshake request.
+func buildInitializeRequest() (string, error) {
+	id, err := rand.Int(rand.Reader, big.NewInt(10000))
+	if err != nil {
+		return "", fmt.Errorf("failed to generate random ID: %w", err)
+	}
+	msg := map[string]any{
+		"jsonrpc": "2.0",
+		"id":      int(id.Int64()),
+		"method":  "initialize",
+		"params": map[string]any{
+			"protocolVersion": "2024-11-05",
+			"capabilities":    map[string]any{},
+			"clientInfo": map[string]any{
+				"name":    "mcpcurl",
+				"version": "0.1.0",
+			},
+		},
+	}
+	data, err := json.Marshal(msg)
+	if err != nil {
+		return "", fmt.Errorf("failed to marshal initialize request: %w", err)
+	}
+	return string(data), nil
+}
+
+// buildInitializedNotification creates the MCP initialized notification.
+func buildInitializedNotification() string {
+	return `{"jsonrpc":"2.0","method":"notifications/initialized"}`
+}
+
+// readJSONRPCResponse reads lines from the scanner, skipping server-initiated
+// notifications (messages without an "id" field), and returns the first response.
+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 {
+			if errField, hasErr := msg["error"]; hasErr {
+				return "", fmt.Errorf("server returned error: %s", string(errField))
+			}
+			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")
 }
 
 func printResponse(response string, prettyPrint bool) error {

cmd/mcpcurl/main_test.go

@@ -0,0 +1,178 @@
+package main
+
+import (
+	"bufio"
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+func TestReadJSONRPCResponse_DirectResponse(t *testing.T) {
+	t.Parallel()
+	input := `{"jsonrpc":"2.0","id":1,"result":{"tools":[]}}` + "\n"
+	scanner := bufio.NewScanner(strings.NewReader(input))
+
+	got, err := readJSONRPCResponse(scanner)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != `{"jsonrpc":"2.0","id":1,"result":{"tools":[]}}` {
+		t.Fatalf("unexpected response: %s", got)
+	}
+}
+
+func TestReadJSONRPCResponse_SkipsNotifications(t *testing.T) {
+	t.Parallel()
+	input := strings.Join([]string{
+		`{"jsonrpc":"2.0","method":"notifications/resources/list_changed","params":{}}`,
+		`{"jsonrpc":"2.0","method":"notifications/tools/list_changed"}`,
+		`{"jsonrpc":"2.0","id":42,"result":{"content":[{"type":"text","text":"hello"}]}}`,
+	}, "\n") + "\n"
+	scanner := bufio.NewScanner(strings.NewReader(input))
+
+	got, err := readJSONRPCResponse(scanner)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	var msg map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(got), &msg); err != nil {
+		t.Fatalf("response is not valid JSON: %v", err)
+	}
+	// Verify we got the response with id:42, not a notification
+	var id int
+	if err := json.Unmarshal(msg["id"], &id); err != nil {
+		t.Fatalf("failed to parse id: %v", err)
+	}
+	if id != 42 {
+		t.Fatalf("expected id 42, got %d", id)
+	}
+}
+
+func TestReadJSONRPCResponse_NoResponse(t *testing.T) {
+	t.Parallel()
+	// Only notifications, no response
+	input := `{"jsonrpc":"2.0","method":"notifications/resources/list_changed","params":{}}` + "\n"
+	scanner := bufio.NewScanner(strings.NewReader(input))
+
+	_, err := readJSONRPCResponse(scanner)
+	if err == nil {
+		t.Fatal("expected error for missing response, got nil")
+	}
+	if !strings.Contains(err.Error(), "unexpected end of output") {
+		t.Fatalf("expected 'unexpected end of output' error, got: %v", err)
+	}
+}
+
+func TestReadJSONRPCResponse_EmptyInput(t *testing.T) {
+	t.Parallel()
+	scanner := bufio.NewScanner(strings.NewReader(""))
+
+	_, err := readJSONRPCResponse(scanner)
+	if err == nil {
+		t.Fatal("expected error for empty input, got nil")
+	}
+}
+
+func TestReadJSONRPCResponse_InvalidJSON(t *testing.T) {
+	t.Parallel()
+	input := "not valid json\n"
+	scanner := bufio.NewScanner(strings.NewReader(input))
+
+	_, err := readJSONRPCResponse(scanner)
+	if err == nil {
+		t.Fatal("expected error for invalid JSON, got nil")
+	}
+	if !strings.Contains(err.Error(), "failed to parse JSON-RPC message") {
+		t.Fatalf("expected parse error, got: %v", err)
+	}
+}
+
+func TestReadJSONRPCResponse_ServerError(t *testing.T) {
+	t.Parallel()
+	input := `{"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"method not found"}}` + "\n"
+	scanner := bufio.NewScanner(strings.NewReader(input))
+
+	_, err := readJSONRPCResponse(scanner)
+	if err == nil {
+		t.Fatal("expected error for server error response, got nil")
+	}
+	if !strings.Contains(err.Error(), "server returned error") {
+		t.Fatalf("expected 'server returned error', got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "method not found") {
+		t.Fatalf("expected error to contain server message, got: %v", err)
+	}
+}
+
+func TestBuildInitializeRequest(t *testing.T) {
+	t.Parallel()
+	got, err := buildInitializeRequest()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	var msg map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(got), &msg); err != nil {
+		t.Fatalf("result is not valid JSON: %v", err)
+	}
+
+	// Verify required fields
+	for _, field := range []string{"jsonrpc", "id", "method", "params"} {
+		if _, ok := msg[field]; !ok {
+			t.Errorf("missing required field %q", field)
+		}
+	}
+
+	// Verify method
+	var method string
+	if err := json.Unmarshal(msg["method"], &method); err != nil {
+		t.Fatalf("failed to parse method: %v", err)
+	}
+	if method != "initialize" {
+		t.Errorf("expected method 'initialize', got %q", method)
+	}
+
+	// Verify params contain protocolVersion and clientInfo
+	var params map[string]json.RawMessage
+	if err := json.Unmarshal(msg["params"], &params); err != nil {
+		t.Fatalf("failed to parse params: %v", err)
+	}
+	for _, field := range []string{"protocolVersion", "capabilities", "clientInfo"} {
+		if _, ok := params[field]; !ok {
+			t.Errorf("missing params field %q", field)
+		}
+	}
+
+	var version string
+	if err := json.Unmarshal(params["protocolVersion"], &version); err != nil {
+		t.Fatalf("failed to parse protocolVersion: %v", err)
+	}
+	if version != "2024-11-05" {
+		t.Errorf("expected protocolVersion '2024-11-05', got %q", version)
+	}
+}
+
+func TestBuildInitializedNotification(t *testing.T) {
+	t.Parallel()
+	got := buildInitializedNotification()
+
+	var msg map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(got), &msg); err != nil {
+		t.Fatalf("result is not valid JSON: %v", err)
+	}
+
+	// Must have jsonrpc and method
+	var method string
+	if err := json.Unmarshal(msg["method"], &method); err != nil {
+		t.Fatalf("failed to parse method: %v", err)
+	}
+	if method != "notifications/initialized" {
+		t.Errorf("expected method 'notifications/initialized', got %q", method)
+	}
+
+	// Must NOT have an id (it's a notification)
+	if _, hasID := msg["id"]; hasID {
+		t.Error("notification should not have an 'id' field")
+	}
+}

docs/feature-flags.md

@@ -198,6 +198,7 @@ runtime behavior (such as output formatting) won't appear here.
 
 - **update_issue_type** - Update Issue Type
   - **Required OAuth Scopes**: `repo`
+  - `confidence`: How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal. (string, optional)
   - `is_suggestion`: If true, this issue type change is sent to the API as a suggestion (suggest:true) rather than an applied value. Whether the type is applied or recorded as a proposal is determined by the API. (boolean, optional)
   - `issue_number`: The issue number to update (number, required)
   - `issue_type`: The issue type to set (string, required)
@@ -240,7 +241,7 @@ runtime behavior (such as output formatting) won't appear here.
   - `owner`: Repository owner (username or organization) (string, required)
   - `pullNumber`: The pull request number (number, required)
   - `repo`: Repository name (string, required)
-  - `reviewers`: GitHub usernames to request reviews from (string[], required)
+  - `reviewers`: GitHub usernames or ORG/team-slug team reviewers to request reviews from (string[], required)
 
 - **resolve_review_thread** - Resolve Review Thread
   - **Required OAuth Scopes**: `repo`

pkg/github/__toolsnaps__/get_commit.snap

@@ -6,10 +6,15 @@
   "description": "Get details for a commit from a GitHub repository",
   "inputSchema": {
     "properties": {
-      "include_diff": {
-        "default": true,
-        "description": "Whether to include file diffs and stats in the response. Default is true.",
-        "type": "boolean"
+      "detail": {
+        "default": "stats",
+        "description": "Level of detail to include for changed files. \"none\" omits stats and files entirely. \"stats\" (default) includes per-file metadata: filename, status, and lines-of-code counts (additions, deletions, changes), with no patch content. \"full_patch\" additionally includes the unified diff content for each file and can be very large.",
+        "enum": [
+          "none",
+          "stats",
+          "full_patch"
+        ],
+        "type": "string"
       },
       "owner": {
         "description": "Repository owner",

pkg/github/__toolsnaps__/request_pull_request_reviewers.snap

@@ -21,7 +21,7 @@
         "type": "string"
       },
       "reviewers": {
-        "description": "GitHub usernames to request reviews from",
+        "description": "GitHub usernames or ORG/team-slug team reviewers to request reviews from",
         "items": {
           "type": "string"
         },