fix(pull-requests): relax pull_request_review_write required fields for resolve_thread

resolve_thread and unresolve_thread only need threadId, but the advertised
inputSchema required owner/repo/pullNumber for every method. Schema-validating
MCP clients therefore rejected valid calls. Require only method at the schema
root, document per-method fields, and validate create/submit/delete vs
resolve/unresolve in the argument normalizer.

Fixes #3501

Author: ningyixi

README.md

@@ -1337,9 +1337,9 @@ For negotiated supported protocol versions `2026-07-28` or newer, the consolidat
   - `commitID`: SHA of commit to review (string, optional)
   - `event`: Review action to perform. (string, optional)
   - `method`: The write operation to perform on pull request review. (string, required)
-  - `owner`: Repository owner (string, required)
-  - `pullNumber`: Pull request number (number, required)
-  - `repo`: Repository name (string, required)
+  - `owner`: Repository owner (required for create, submit_pending, and delete_pending methods) (string, optional)
+  - `pullNumber`: Pull request number (required for create, submit_pending, and delete_pending methods) (number, optional)
+  - `repo`: Repository name (required for create, submit_pending, and delete_pending methods) (string, optional)
   - `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional)
 
 - **search_pull_requests** - Search pull requests

docs/feature-flags.md

@@ -414,9 +414,9 @@ as output formatting) won't appear here.
   - `commitID`: SHA of commit to review (string, optional)
   - `event`: Review action to perform. (string, optional)
   - `method`: The write operation to perform on pull request review. (string, required)
-  - `owner`: Repository owner (string, required)
-  - `pullNumber`: Pull request number (number, required)
-  - `repo`: Repository name (string, required)
+  - `owner`: Repository owner (required for create, submit_pending, and delete_pending methods) (string, optional)
+  - `pullNumber`: Pull request number (required for create, submit_pending, and delete_pending methods) (number, optional)
+  - `repo`: Repository name (required for create, submit_pending, and delete_pending methods) (string, optional)
   - `resolutionReason`: Optional reason for resolving a Copilot code review thread: addressed, wont-fix, or invalid. (string, optional)
   - `threadId`: The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments. (string, optional)
 

pkg/github/__toolsnaps__/pull_request_review_write.snap

@@ -36,15 +36,15 @@
         "type": "string"
       },
       "owner": {
-        "description": "Repository owner",
+        "description": "Repository owner (required for create, submit_pending, and delete_pending methods)",
         "type": "string"
       },
       "pullNumber": {
-        "description": "Pull request number",
+        "description": "Pull request number (required for create, submit_pending, and delete_pending methods)",
         "type": "number"
       },
       "repo": {
-        "description": "Repository name",
+        "description": "Repository name (required for create, submit_pending, and delete_pending methods)",
         "type": "string"
       },
       "threadId": {
@@ -53,10 +53,7 @@
       }
     },
     "required": [
-      "method",
-      "owner",
-      "repo",
-      "pullNumber"
+      "method"
     ],
     "type": "object"
   },

pkg/github/__toolsnaps__/pull_request_review_write_resolution_reason.snap

@@ -36,15 +36,15 @@
         "type": "string"
       },
       "owner": {
-        "description": "Repository owner",
+        "description": "Repository owner (required for create, submit_pending, and delete_pending methods)",
         "type": "string"
       },
       "pullNumber": {
-        "description": "Pull request number",
+        "description": "Pull request number (required for create, submit_pending, and delete_pending methods)",
         "type": "number"
       },
       "repo": {
-        "description": "Repository name",
+        "description": "Repository name (required for create, submit_pending, and delete_pending methods)",
         "type": "string"
       },
       "resolutionReason": {
@@ -57,10 +57,7 @@
       }
     },
     "required": [
-      "method",
-      "owner",
-      "repo",
-      "pullNumber"
+      "method"
     ],
     "type": "object"
   },

pkg/github/consolidated_pull_request_types.go

@@ -594,6 +594,16 @@ func normalizePullRequestReviewWriteArguments(raw json.RawMessage) (json.RawMess
 	if !slices.Contains([]string{"create", "submit_pending", "delete_pending", "resolve_thread", "unresolve_thread"}, params.Method) {
 		return nil, fmt.Errorf("unknown method: %s", params.Method)
 	}
+	switch params.Method {
+	case "resolve_thread", "unresolve_thread":
+		if params.ThreadID == "" {
+			return nil, fmt.Errorf("threadId is required for resolve_thread and unresolve_thread methods")
+		}
+	case "create", "submit_pending", "delete_pending":
+		if params.Owner == "" || params.Repo == "" || params.PullNumber == 0 {
+			return nil, fmt.Errorf("owner, repo, and pullNumber are required for %s method", params.Method)
+		}
+	}
 	canonical := map[string]any{
 		"method": params.Method, "owner": params.Owner, "repo": params.Repo, "pullNumber": params.PullNumber,
 	}

pkg/github/pullrequests.go

@@ -2015,15 +2015,15 @@ func pullRequestReviewWrite(t translations.TranslationHelperFunc, withResolution
 			},
 			"owner": {
 				Type:        "string",
-				Description: "Repository owner",
+				Description: "Repository owner (required for create, submit_pending, and delete_pending methods)",
 			},
 			"repo": {
 				Type:        "string",
-				Description: "Repository name",
+				Description: "Repository name (required for create, submit_pending, and delete_pending methods)",
 			},
 			"pullNumber": {
 				Type:        "number",
-				Description: "Pull request number",
+				Description: "Pull request number (required for create, submit_pending, and delete_pending methods)",
 			},
 			"body": {
 				Type:        "string",
@@ -2043,7 +2043,9 @@ func pullRequestReviewWrite(t translations.TranslationHelperFunc, withResolution
 				Description: "The node ID of the review thread (e.g., PRRT_kwDOxxx). Required for resolve_thread and unresolve_thread methods. Get thread IDs from pull_request_read with method get_review_comments.",
 			},
 		},
-		Required: []string{"method", "owner", "repo", "pullNumber"},
+		// Only method is globally required: resolve_thread / unresolve_thread use
+		// threadId alone, matching discussion_comment_write and the granular tools.
+		Required: []string{"method"},
 	}
 	if withResolutionReason {
 		schema.Properties["resolutionReason"] = &jsonschema.Schema{

pkg/github/pullrequests_test.go

@@ -3203,7 +3203,7 @@ func TestCreateAndSubmitPullRequestReview(t *testing.T) {
 	assert.Contains(t, schema.Properties, "body")
 	assert.Contains(t, schema.Properties, "event")
 	assert.Contains(t, schema.Properties, "commitID")
-	assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
+	assert.ElementsMatch(t, schema.Required, []string{"method"})
 
 	tests := []struct {
 		name               string
@@ -3460,7 +3460,7 @@ func TestCreatePendingPullRequestReview(t *testing.T) {
 	assert.Contains(t, schema.Properties, "repo")
 	assert.Contains(t, schema.Properties, "pullNumber")
 	assert.Contains(t, schema.Properties, "commitID")
-	assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
+	assert.ElementsMatch(t, schema.Required, []string{"method"})
 
 	tests := []struct {
 		name               string
@@ -3918,7 +3918,7 @@ func TestSubmitPendingPullRequestReview(t *testing.T) {
 	assert.Contains(t, schema.Properties, "pullNumber")
 	assert.Contains(t, schema.Properties, "event")
 	assert.Contains(t, schema.Properties, "body")
-	assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
+	assert.ElementsMatch(t, schema.Required, []string{"method"})
 
 	tests := []struct {
 		name               string
@@ -4019,7 +4019,7 @@ func TestDeletePendingPullRequestReview(t *testing.T) {
 	assert.Contains(t, schema.Properties, "owner")
 	assert.Contains(t, schema.Properties, "repo")
 	assert.Contains(t, schema.Properties, "pullNumber")
-	assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"})
+	assert.ElementsMatch(t, schema.Required, []string{"method"})
 
 	tests := []struct {
 		name               string
@@ -4681,6 +4681,12 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
 func TestResolveReviewThread(t *testing.T) {
 	t.Parallel()
 
+	serverTool := PullRequestReviewWrite(translations.NullTranslationHelper)
+	schema := serverTool.Tool.InputSchema.(*jsonschema.Schema)
+	assert.ElementsMatch(t, schema.Required, []string{"method"})
+	assert.Contains(t, schema.Properties["owner"].Description, "required for create")
+	assert.Contains(t, schema.Properties["threadId"].Description, "Required for resolve_thread")
+
 	tests := []struct {
 		name                 string
 		requestArgs          map[string]any
@@ -4725,6 +4731,38 @@ func TestResolveReviewThread(t *testing.T) {
 			),
 			expectedResult: "review thread resolved successfully",
 		},
+		{
+			name: "successful resolve thread without owner repo pullNumber",
+			requestArgs: map[string]any{
+				"method":   "resolve_thread",
+				"threadId": "PRRT_kwDOTest123",
+			},
+			mockedClient: githubv4mock.NewMockedHTTPClient(
+				githubv4mock.NewMutationMatcher(
+					struct {
+						ResolveReviewThread struct {
+							Thread struct {
+								ID         githubv4.ID
+								IsResolved githubv4.Boolean
+							}
+						} `graphql:"resolveReviewThread(input: $input)"`
+					}{},
+					ResolveReviewThreadInput{
+						ThreadID: githubv4.ID("PRRT_kwDOTest123"),
+					},
+					nil,
+					githubv4mock.DataResponse(map[string]any{
+						"resolveReviewThread": map[string]any{
+							"thread": map[string]any{
+								"id":         "PRRT_kwDOTest123",
+								"isResolved": true,
+							},
+						},
+					}),
+				),
+			),
+			expectedResult: "review thread resolved successfully",
+		},
 		{
 			name:                 "successful resolve thread with resolution reason",
 			withResolutionReason: true,

pkg/github/typed_consolidated_pull_request_outputs_test.go

@@ -632,3 +632,47 @@ func TestConsolidatedPullRequestOutputTypesAreCurated(t *testing.T) {
 		check(typ)
 	}
 }
+
+func TestPullRequestReviewWriteResolveThreadSchema(t *testing.T) {
+	// Advertised schema must not require owner/repo/pullNumber so MCP clients can
+	// call resolve_thread / unresolve_thread with only threadId (#3501).
+	for _, protocol := range typedPullRequestProtocols {
+		t.Run("protocol="+protocol, func(t *testing.T) {
+			session, _ := consolidatedPullRequestSession(t, consolidatedPullRequestDeps(t), protocol, false)
+			list, err := session.ListTools(context.Background(), nil)
+			require.NoError(t, err)
+			var schema map[string]any
+			for _, tool := range list.Tools {
+				if tool.Name != "pull_request_review_write" {
+					continue
+				}
+				require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, tool.InputSchema)), &schema))
+			}
+			require.NotEmpty(t, schema)
+			required, _ := schema["required"].([]any)
+			assert.Equal(t, []any{"method"}, required)
+
+			result, err := session.CallTool(context.Background(), &mcp.CallToolParams{
+				Name: "pull_request_review_write",
+				Arguments: map[string]any{
+					"method":   "resolve_thread",
+					"threadId": "T_1",
+				},
+			})
+			require.NoError(t, err)
+			require.False(t, result.IsError, mustMarshalJSON(t, result))
+			assert.Contains(t, getTextResult(t, result).Text, "review thread resolved successfully")
+
+			missing, err := session.CallTool(context.Background(), &mcp.CallToolParams{
+				Name: "pull_request_review_write",
+				Arguments: map[string]any{
+					"method":     "create",
+					"pullNumber": 1,
+				},
+			})
+			require.NoError(t, err)
+			require.True(t, missing.IsError)
+			assert.Contains(t, getErrorResult(t, missing).Text, "owner, repo, and pullNumber are required for create method")
+		})
+	}
+}