Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions docs/feature-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
11 changes: 4 additions & 7 deletions pkg/github/__toolsnaps__/pull_request_review_write.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -53,10 +53,7 @@
}
},
"required": [
"method",
"owner",
"repo",
"pullNumber"
"method"
],
"type": "object"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -57,10 +57,7 @@
}
},
"required": [
"method",
"owner",
"repo",
"pullNumber"
"method"
],
"type": "object"
},
Expand Down
10 changes: 10 additions & 0 deletions pkg/github/consolidated_pull_request_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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{
Expand Down
46 changes: 42 additions & 4 deletions pkg/github/pullrequests_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions pkg/github/typed_consolidated_pull_request_outputs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
}