From 1bdf3bdcf727f89d6dcb34ed11b739af9750b9aa Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Wed, 24 Sep 2025 14:07:27 +0000 Subject: [PATCH 1/4] Consolidating tools draft --- pkg/github/pullrequests.go | 45 +++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 8289a4e48..948e7892d 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -19,12 +19,22 @@ import ( // GetPullRequest creates a tool to get details of a specific pull request. func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_DESCRIPTION", "Get details of a specific pull request in a GitHub repository.")), + return mcp.NewTool("pull_request_read", + mcp.WithDescription(t("TOOL_PULL_REQUEST_READ_DESCRIPTION", "Get information on a specific pull request in GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_USER_TITLE", "Get pull request details"), + Title: t("TOOL_GET_PULL_REQUEST_USER_TITLE", "Get pull requests details."), ReadOnlyHint: ToBoolPtr(true), }), + mcp.WithString("method", + mcp.Required(), + mcp.Description(`Action to perform with pull requests in GitHub. +Possible options: + 1. get - Get details of a specific pull request in a GitHub repository. + 2. get_diff - Get the diff of a pull request. + 3. get_files - Get the files changed in a specific pull request. + 4. get_status - Get status of a pull request. +`), + ), mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner"), @@ -56,6 +66,8 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) if err != nil { return nil, fmt.Errorf("failed to get GitHub client: %w", err) } + + pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, @@ -1870,6 +1882,33 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe } } +func GetPullRequest() { + pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil + } + + r, err := json.Marshal(pr) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil +} + // newGQLString like takes something that approximates a string (of which there are many types in shurcooL/githubv4) // and constructs a pointer to it, or nil if the string is empty. This is extremely useful because when we parse // params from the MCP request, we need to convert them to types that are pointers of type def strings and it's From 5e3dcc64c2061389a100e9ebb7df31acb96d5c7d Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Thu, 2 Oct 2025 13:42:06 +0000 Subject: [PATCH 2/4] Consolidate pullrequest tools --- .../__toolsnaps__/get_pull_request.snap | 30 - .../__toolsnaps__/get_pull_request_diff.snap | 30 - .../__toolsnaps__/get_pull_request_files.snap | 41 -- .../get_pull_request_review_comments.snap | 30 - .../get_pull_request_reviews.snap | 30 - .../get_pull_request_status.snap | 30 - .../__toolsnaps__/pull_request_read.snap | 46 ++ pkg/github/pullrequests.go | 643 ++++++------------ pkg/github/pullrequests_test.go | 66 +- pkg/github/tools.go | 7 +- 10 files changed, 315 insertions(+), 638 deletions(-) delete mode 100644 pkg/github/__toolsnaps__/get_pull_request.snap delete mode 100644 pkg/github/__toolsnaps__/get_pull_request_diff.snap delete mode 100644 pkg/github/__toolsnaps__/get_pull_request_files.snap delete mode 100644 pkg/github/__toolsnaps__/get_pull_request_review_comments.snap delete mode 100644 pkg/github/__toolsnaps__/get_pull_request_reviews.snap delete mode 100644 pkg/github/__toolsnaps__/get_pull_request_status.snap create mode 100644 pkg/github/__toolsnaps__/pull_request_read.snap diff --git a/pkg/github/__toolsnaps__/get_pull_request.snap b/pkg/github/__toolsnaps__/get_pull_request.snap deleted file mode 100644 index cbcf1f425..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Get pull request details", - "readOnlyHint": true - }, - "description": "Get details of a specific pull request in a GitHub repository.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_pull_request_diff.snap b/pkg/github/__toolsnaps__/get_pull_request_diff.snap deleted file mode 100644 index e054eab92..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request_diff.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Get pull request diff", - "readOnlyHint": true - }, - "description": "Get the diff of a pull request.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request_diff" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_pull_request_files.snap b/pkg/github/__toolsnaps__/get_pull_request_files.snap deleted file mode 100644 index 148053b17..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request_files.snap +++ /dev/null @@ -1,41 +0,0 @@ -{ - "annotations": { - "title": "Get pull request files", - "readOnlyHint": true - }, - "description": "Get the files changed in a specific pull request.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "page": { - "description": "Page number for pagination (min 1)", - "minimum": 1, - "type": "number" - }, - "perPage": { - "description": "Results per page for pagination (min 1, max 100)", - "maximum": 100, - "minimum": 1, - "type": "number" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request_files" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_pull_request_review_comments.snap b/pkg/github/__toolsnaps__/get_pull_request_review_comments.snap deleted file mode 100644 index 92996fec2..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request_review_comments.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Get pull request review comments", - "readOnlyHint": true - }, - "description": "Get pull request review comments. They are comments made on a portion of the unified diff during a pull request review. These are different from commit comments and issue comments in a pull request.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request_review_comments" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_pull_request_reviews.snap b/pkg/github/__toolsnaps__/get_pull_request_reviews.snap deleted file mode 100644 index 61dee53ee..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request_reviews.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Get pull request reviews", - "readOnlyHint": true - }, - "description": "Get reviews for a specific pull request.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request_reviews" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_pull_request_status.snap b/pkg/github/__toolsnaps__/get_pull_request_status.snap deleted file mode 100644 index 8ffebc3a4..000000000 --- a/pkg/github/__toolsnaps__/get_pull_request_status.snap +++ /dev/null @@ -1,30 +0,0 @@ -{ - "annotations": { - "title": "Get pull request status checks", - "readOnlyHint": true - }, - "description": "Get the status of a specific pull request.", - "inputSchema": { - "properties": { - "owner": { - "description": "Repository owner", - "type": "string" - }, - "pullNumber": { - "description": "Pull request number", - "type": "number" - }, - "repo": { - "description": "Repository name", - "type": "string" - } - }, - "required": [ - "owner", - "repo", - "pullNumber" - ], - "type": "object" - }, - "name": "get_pull_request_status" -} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap new file mode 100644 index 000000000..139a23f47 --- /dev/null +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -0,0 +1,46 @@ +{ + "annotations": { + "title": "Get details for a single pull request", + "readOnlyHint": true + }, + "description": "Get information on a specific pull request in GitHub repository.", + "inputSchema": { + "properties": { + "method": { + "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get status of a pull request.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_reviews - Get the reviews on a pull request.\n", + "type": "string" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "pullNumber": { + "description": "Pull request number", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + } + }, + "required": [ + "method", + "owner", + "repo", + "pullNumber" + ], + "type": "object" + }, + "name": "pull_request_read" +} \ No newline at end of file diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 948e7892d..ea6f75e84 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -18,21 +18,23 @@ import ( ) // GetPullRequest creates a tool to get details of a specific pull request. -func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +func PullRequestRead(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("pull_request_read", mcp.WithDescription(t("TOOL_PULL_REQUEST_READ_DESCRIPTION", "Get information on a specific pull request in GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_USER_TITLE", "Get pull requests details."), + Title: t("TOOL_GET_PULL_REQUEST_USER_TITLE", "Get details for a single pull request"), ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("method", mcp.Required(), - mcp.Description(`Action to perform with pull requests in GitHub. + mcp.Description(`Action to specify what pull request data needs to be retrieved from GitHub. Possible options: - 1. get - Get details of a specific pull request in a GitHub repository. - 2. get_diff - Get the diff of a pull request. - 3. get_files - Get the files changed in a specific pull request. - 4. get_status - Get status of a pull request. + 1. get - Get details of a specific pull request. + 2. get_diff - Get the diff of a pull request. + 3. get_status - Get status of a pull request. + 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned. + 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned. + 6. get_reviews - Get the reviews on a pull request. `), ), mcp.WithString("owner", @@ -47,8 +49,14 @@ Possible options: mcp.Required(), mcp.Description("Pull request number"), ), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + method, err := RequiredParam[string](request, "method") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + owner, err := RequiredParam[string](request, "owner") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -61,38 +69,229 @@ Possible options: if err != nil { return mcp.NewToolResultError(err.Error()), nil } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } client, err := getClient(ctx) if err != nil { return nil, fmt.Errorf("failed to get GitHub client: %w", err) } + switch method { - pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request", - resp, - err, - ), nil + case "get": + return GetPullRequest(ctx, client, owner, repo, pullNumber) + case "get_diff": + return GetPullRequestDiff(ctx, client, owner, repo, pullNumber) + case "get_status": + return GetPullRequestStatus(ctx, client, owner, repo, pullNumber) + case "get_files": + return GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination) + case "get_review_comments": + return GetPullRequestReviewComments(ctx, client, owner, repo, pullNumber, pagination) + case "get_reviews": + return GetPullRequestReviews(ctx, client, owner, repo, pullNumber, pagination) + default: + return nil, fmt.Errorf("unknown method: %s", method) } - defer func() { _ = resp.Body.Close() }() + } +} - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil - } +func GetPullRequest(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { + pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(pr) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil + } - return mcp.NewToolResultText(string(r)), nil + r, err := json.Marshal(pr) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil +} + +func GetPullRequestDiff(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { + raw, resp, err := client.PullRequests.GetRaw( + ctx, + owner, + repo, + pullNumber, + github.RawOptions{Type: github.Diff}, + ) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request diff", + resp, + err, + ), nil + } + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request diff: %s", string(body))), nil + } + + defer func() { _ = resp.Body.Close() }() + + // Return the raw response + return mcp.NewToolResultText(string(raw)), nil +} + +func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { + pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil + } + + // Get combined status for the head SHA + status, resp, err := client.Repositories.GetCombinedStatus(ctx, owner, repo, *pr.Head.SHA, nil) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get combined status", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get combined status: %s", string(body))), nil + } + + r, err := json.Marshal(status) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil +} + +func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { + opts := &github.ListOptions{ + PerPage: pagination.PerPage, + Page: pagination.Page, + } + files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request files", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request files: %s", string(body))), nil + } + + r, err := json.Marshal(files) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil +} + +func GetPullRequestReviewComments(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { + opts := &github.PullRequestListCommentsOptions{ + ListOptions: github.ListOptions{ + PerPage: pagination.PerPage, + Page: pagination.Page, + }, + } + + comments, resp, err := client.PullRequests.ListComments(ctx, owner, repo, pullNumber, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request review comments", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request review comments: %s", string(body))), nil + } + + r, err := json.Marshal(comments) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil +} + +func GetPullRequestReviews(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { + reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request reviews", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request reviews: %s", string(body))), nil + } + + r, err := json.Marshal(reviews) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil } // CreatePullRequest creates a tool to create a new pull request. @@ -747,166 +946,6 @@ func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperF } } -// GetPullRequestFiles creates a tool to get the list of files changed in a pull request. -func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request_files", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_FILES_DESCRIPTION", "Get the files changed in a specific pull request.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_FILES_USER_TITLE", "Get pull request files"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - WithPagination(), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](request, "owner") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - repo, err := RequiredParam[string](request, "repo") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pullNumber, err := RequiredInt(request, "pullNumber") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pagination, err := OptionalPaginationParams(request) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - opts := &github.ListOptions{ - PerPage: pagination.PerPage, - Page: pagination.Page, - } - files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request files", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request files: %s", string(body))), nil - } - - r, err := json.Marshal(files) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } -} - -// GetPullRequestStatus creates a tool to get the combined status of all status checks for a pull request. -func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request_status", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_STATUS_DESCRIPTION", "Get the status of a specific pull request.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_STATUS_USER_TITLE", "Get pull request status checks"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](request, "owner") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - repo, err := RequiredParam[string](request, "repo") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pullNumber, err := RequiredInt(request, "pullNumber") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - // First get the PR to find the head SHA - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil - } - - // Get combined status for the head SHA - status, resp, err := client.Repositories.GetCombinedStatus(ctx, owner, repo, *pr.Head.SHA, nil) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get combined status", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get combined status: %s", string(body))), nil - } - - r, err := json.Marshal(status) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } -} - // UpdatePullRequestBranch creates a tool to update a pull request branch with the latest changes from the base branch. func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("update_pull_request_branch", @@ -989,144 +1028,6 @@ func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHe } } -// GetPullRequestReviewComments creates a tool to get the review comments on a pull request. -func GetPullRequestReviewComments(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request_review_comments", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_REVIEW_COMMENTS_DESCRIPTION", "Get pull request review comments. They are comments made on a portion of the unified diff during a pull request review. These are different from commit comments and issue comments in a pull request.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_REVIEW_COMMENTS_USER_TITLE", "Get pull request review comments"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](request, "owner") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - repo, err := RequiredParam[string](request, "repo") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pullNumber, err := RequiredInt(request, "pullNumber") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - opts := &github.PullRequestListCommentsOptions{ - ListOptions: github.ListOptions{ - PerPage: 100, - }, - } - - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - comments, resp, err := client.PullRequests.ListComments(ctx, owner, repo, pullNumber, opts) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request review comments", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request review comments: %s", string(body))), nil - } - - r, err := json.Marshal(comments) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } -} - -// GetPullRequestReviews creates a tool to get the reviews on a pull request. -func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request_reviews", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_REVIEWS_DESCRIPTION", "Get reviews for a specific pull request.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_REVIEWS_USER_TITLE", "Get pull request reviews"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](request, "owner") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - repo, err := RequiredParam[string](request, "repo") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pullNumber, err := RequiredInt(request, "pullNumber") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request reviews", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request reviews: %s", string(body))), nil - } - - r, err := json.Marshal(reviews) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } -} - func CreateAndSubmitPullRequestReview(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("create_and_submit_pull_request_review", mcp.WithDescription(t("TOOL_CREATE_AND_SUBMIT_PULL_REQUEST_REVIEW_DESCRIPTION", "Create and submit a review for a pull request without review comments.")), @@ -1741,71 +1642,6 @@ func DeletePendingPullRequestReview(getGQLClient GetGQLClientFn, t translations. } } -func GetPullRequestDiff(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { - return mcp.NewTool("get_pull_request_diff", - mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_DIFF_DESCRIPTION", "Get the diff of a pull request.")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_GET_PULL_REQUEST_DIFF_USER_TITLE", "Get pull request diff"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("owner", - mcp.Required(), - mcp.Description("Repository owner"), - ), - mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), - ), - mcp.WithNumber("pullNumber", - mcp.Required(), - mcp.Description("Pull request number"), - ), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - var params struct { - Owner string - Repo string - PullNumber int32 - } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - client, err := getClient(ctx) - if err != nil { - return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub client: %v", err)), nil - } - - raw, resp, err := client.PullRequests.GetRaw( - ctx, - params.Owner, - params.Repo, - int(params.PullNumber), - github.RawOptions{Type: github.Diff}, - ) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request diff", - resp, - err, - ), nil - } - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request diff: %s", string(body))), nil - } - - defer func() { _ = resp.Body.Close() }() - - // Return the raw response - return mcp.NewToolResultText(string(raw)), nil - } -} - // RequestCopilotReview creates a tool to request a Copilot review for a pull request. // Note that this tool will not work on GHES where this feature is unsupported. In future, we should not expose this // tool if the configured host does not support it. @@ -1882,33 +1718,6 @@ func RequestCopilotReview(getClient GetClientFn, t translations.TranslationHelpe } } -func GetPullRequest() { - pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to get pull request", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil - } - - r, err := json.Marshal(pr) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil -} - // newGQLString like takes something that approximates a string (of which there are many types in shurcooL/githubv4) // and constructs a pointer to it, or nil if the string is empty. This is extremely useful because when we parse // params from the MCP request, we need to convert them to types that are pointers of type def strings and it's diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 18fc8d87d..733c43bfb 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -21,15 +21,15 @@ import ( func Test_GetPullRequest(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequest(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR for success case mockPR := &github.PullRequest{ @@ -67,6 +67,7 @@ func Test_GetPullRequest(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -86,6 +87,7 @@ func Test_GetPullRequest(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get", "owner": "owner", "repo": "repo", "pullNumber": float64(999), @@ -99,7 +101,7 @@ func Test_GetPullRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequest(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1130,17 +1132,18 @@ func Test_SearchPullRequests(t *testing.T) { func Test_GetPullRequestFiles(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequestFiles(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request_files", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") assert.Contains(t, tool.InputSchema.Properties, "page") assert.Contains(t, tool.InputSchema.Properties, "perPage") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR files for success case mockFiles := []*github.CommitFile{ @@ -1179,6 +1182,7 @@ func Test_GetPullRequestFiles(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_files", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1195,6 +1199,7 @@ func Test_GetPullRequestFiles(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_files", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1216,6 +1221,7 @@ func Test_GetPullRequestFiles(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_files", "owner": "owner", "repo": "repo", "pullNumber": float64(999), @@ -1229,7 +1235,7 @@ func Test_GetPullRequestFiles(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequestFiles(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1270,15 +1276,16 @@ func Test_GetPullRequestFiles(t *testing.T) { func Test_GetPullRequestStatus(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequestStatus(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request_status", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR for successful PR fetch mockPR := &github.PullRequest{ @@ -1338,6 +1345,7 @@ func Test_GetPullRequestStatus(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_status", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1357,6 +1365,7 @@ func Test_GetPullRequestStatus(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_status", "owner": "owner", "repo": "repo", "pullNumber": float64(999), @@ -1380,6 +1389,7 @@ func Test_GetPullRequestStatus(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_status", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1393,7 +1403,7 @@ func Test_GetPullRequestStatus(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequestStatus(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1555,15 +1565,16 @@ func Test_UpdatePullRequestBranch(t *testing.T) { func Test_GetPullRequestComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequestReviewComments(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request_review_comments", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR comments for success case mockComments := []*github.PullRequestComment{ @@ -1612,6 +1623,7 @@ func Test_GetPullRequestComments(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_review_comments", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1631,6 +1643,7 @@ func Test_GetPullRequestComments(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_review_comments", "owner": "owner", "repo": "repo", "pullNumber": float64(999), @@ -1644,7 +1657,7 @@ func Test_GetPullRequestComments(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequestReviewComments(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1686,15 +1699,16 @@ func Test_GetPullRequestComments(t *testing.T) { func Test_GetPullRequestReviews(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequestReviews(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request_reviews", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) // Setup mock PR reviews for success case mockReviews := []*github.PullRequestReview{ @@ -1739,6 +1753,7 @@ func Test_GetPullRequestReviews(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_reviews", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -1758,6 +1773,7 @@ func Test_GetPullRequestReviews(t *testing.T) { ), ), requestArgs: map[string]interface{}{ + "method": "get_reviews", "owner": "owner", "repo": "repo", "pullNumber": float64(999), @@ -1771,7 +1787,7 @@ func Test_GetPullRequestReviews(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequestReviews(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2760,15 +2776,16 @@ func TestGetPullRequestDiff(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetPullRequestDiff(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) - assert.Equal(t, "get_pull_request_diff", tool.Name) + assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"method", "owner", "repo", "pullNumber"}) stubbedDiff := `diff --git a/README.md b/README.md index 5d6e7b2..8a4f5c3 100644 @@ -2793,6 +2810,7 @@ index 5d6e7b2..8a4f5c3 100644 { name: "successful diff retrieval", requestArgs: map[string]any{ + "method": "get_diff", "owner": "owner", "repo": "repo", "pullNumber": float64(42), @@ -2816,7 +2834,7 @@ index 5d6e7b2..8a4f5c3 100644 // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetPullRequestDiff(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 7fb5332aa..ab517c245 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -84,14 +84,9 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG ) pullRequests := toolsets.NewToolset("pull_requests", "GitHub Pull Request related tools"). AddReadTools( - toolsets.NewServerTool(GetPullRequest(getClient, t)), + toolsets.NewServerTool(PullRequestRead(getClient, t)), toolsets.NewServerTool(ListPullRequests(getClient, t)), - toolsets.NewServerTool(GetPullRequestFiles(getClient, t)), toolsets.NewServerTool(SearchPullRequests(getClient, t)), - toolsets.NewServerTool(GetPullRequestStatus(getClient, t)), - toolsets.NewServerTool(GetPullRequestReviewComments(getClient, t)), - toolsets.NewServerTool(GetPullRequestReviews(getClient, t)), - toolsets.NewServerTool(GetPullRequestDiff(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(MergePullRequest(getClient, t)), From 1379ebee6aab1c46858142bb80c15ed1d98f7d29 Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Mon, 6 Oct 2025 10:01:59 +0000 Subject: [PATCH 3/4] Prompt tweaks --- pkg/github/pullrequests.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index ea6f75e84..27c30bdd8 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -31,10 +31,10 @@ func PullRequestRead(getClient GetClientFn, t translations.TranslationHelperFunc Possible options: 1. get - Get details of a specific pull request. 2. get_diff - Get the diff of a pull request. - 3. get_status - Get status of a pull request. + 3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks. 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned. 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned. - 6. get_reviews - Get the reviews on a pull request. + 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. `), ), mcp.WithString("owner", From d381a59b76c21df49195c3172a16667849baded3 Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Mon, 6 Oct 2025 12:30:37 +0000 Subject: [PATCH 4/4] Fixes --- README.md | 48 +++++++------------ .../__toolsnaps__/pull_request_read.snap | 2 +- pkg/github/pullrequests.go | 4 +- pkg/github/pullrequests_test.go | 1 + 4 files changed, 20 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 6ed566086..5a0425ec7 100644 --- a/README.md +++ b/README.md @@ -713,38 +713,6 @@ The following sets of tools are available (all are on by default): - `pullNumber`: Pull request number (number, required) - `repo`: Repository name (string, required) -- **get_pull_request** - Get pull request details - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **get_pull_request_diff** - Get pull request diff - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **get_pull_request_files** - Get pull request files - - `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) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **get_pull_request_review_comments** - Get pull request review comments - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **get_pull_request_reviews** - Get pull request reviews - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - -- **get_pull_request_status** - Get pull request status checks - - `owner`: Repository owner (string, required) - - `pullNumber`: Pull request number (number, required) - - `repo`: Repository name (string, required) - - **list_pull_requests** - List pull requests - `base`: Filter by base branch (string, optional) - `direction`: Sort direction (string, optional) @@ -764,6 +732,22 @@ The following sets of tools are available (all are on by default): - `pullNumber`: Pull request number (number, required) - `repo`: Repository name (string, required) +- **pull_request_read** - Get details for a single pull request + - `method`: Action to specify what pull request data needs to be retrieved from GitHub. +Possible options: + 1. get - Get details of a specific pull request. + 2. get_diff - Get the diff of a pull request. + 3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks. + 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned. + 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned. + 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. + (string, required) + - `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) + - `pullNumber`: Pull request number (number, required) + - `repo`: Repository name (string, required) + - **request_copilot_review** - Request Copilot review - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number (number, required) diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap index 139a23f47..2079dd2c9 100644 --- a/pkg/github/__toolsnaps__/pull_request_read.snap +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -7,7 +7,7 @@ "inputSchema": { "properties": { "method": { - "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get status of a pull request.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_reviews - Get the reviews on a pull request.\n", + "description": "Action to specify what pull request data needs to be retrieved from GitHub. \nPossible options: \n 1. get - Get details of a specific pull request.\n 2. get_diff - Get the diff of a pull request.\n 3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks.\n 4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.\n 5. get_review_comments - Get the review comments on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method.\n", "type": "string" }, "owner": { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 27c30bdd8..867b4ee1f 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -92,7 +92,7 @@ Possible options: case "get_review_comments": return GetPullRequestReviewComments(ctx, client, owner, repo, pullNumber, pagination) case "get_reviews": - return GetPullRequestReviews(ctx, client, owner, repo, pullNumber, pagination) + return GetPullRequestReviews(ctx, client, owner, repo, pullNumber) default: return nil, fmt.Errorf("unknown method: %s", method) } @@ -267,7 +267,7 @@ func GetPullRequestReviewComments(ctx context.Context, client *github.Client, ow return mcp.NewToolResultText(string(r)), nil } -func GetPullRequestReviews(ctx context.Context, client *github.Client, owner, repo string, pullNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetPullRequestReviews(ctx context.Context, client *github.Client, owner, repo string, pullNumber int) (*mcp.CallToolResult, error) { reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 733c43bfb..417a9082a 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -26,6 +26,7 @@ func Test_GetPullRequest(t *testing.T) { assert.Equal(t, "pull_request_read", tool.Name) assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "method") assert.Contains(t, tool.InputSchema.Properties, "owner") assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "pullNumber")