diff --git a/README.md b/README.md index dff62321b..454b011c4 100644 --- a/README.md +++ b/README.md @@ -1122,10 +1122,11 @@ The following sets of tools are available: 2. get_diff - Get the diff of a pull request. 3. get_status - Get combined commit status of a head commit in 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 review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results. - 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned. - 7. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned. - 8. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR. + 5. get_commits - Get the list of commits on a pull request. Use with pagination parameters to control the number of results returned. + 6. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results. + 7. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned. + 8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned. + 9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR. (string, required) - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) diff --git a/pkg/github/__toolsnaps__/pull_request_read.snap b/pkg/github/__toolsnaps__/pull_request_read.snap index d70f77e1e..f1bb855d5 100644 --- a/pkg/github/__toolsnaps__/pull_request_read.snap +++ b/pkg/github/__toolsnaps__/pull_request_read.snap @@ -11,12 +11,13 @@ "type": "string" }, "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 combined commit status of a head commit in 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 review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results.\n 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned.\n 7. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n 8. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.\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 combined commit status of a head commit in 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_commits - Get the list of commits on a pull request. Use with pagination parameters to control the number of results returned.\n 6. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results.\n 7. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned.\n 8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned.\n 9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR.\n", "enum": [ "get", "get_diff", "get_status", "get_files", + "get_commits", "get_review_comments", "get_reviews", "get_comments", diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index fdac78ce3..7f86c8b98 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -69,6 +69,7 @@ const ( // Pull request endpoints GetReposPullsByOwnerByRepo = "GET /repos/{owner}/{repo}/pulls" GetReposPullsByOwnerByRepoByPullNumber = "GET /repos/{owner}/{repo}/pulls/{pull_number}" + GetReposPullsCommitsByOwnerByRepoByPullNumber = "GET /repos/{owner}/{repo}/pulls/{pull_number}/commits" GetReposPullsFilesByOwnerByRepoByPullNumber = "GET /repos/{owner}/{repo}/pulls/{pull_number}/files" GetReposPullsReviewsByOwnerByRepoByPullNumber = "GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews" PostReposPullsByOwnerByRepo = "POST /repos/{owner}/{repo}/pulls" diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 5200be297..eff6edc13 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -123,6 +123,14 @@ type MinimalPRFile struct { PreviousFilename string `json:"previous_filename,omitempty"` } +// MinimalPullRequestCommit is the trimmed output type for commits listed on a pull request. +type MinimalPullRequestCommit struct { + SHA string `json:"sha"` + HTMLURL string `json:"html_url,omitempty"` + Message string `json:"message,omitempty"` + Author *MinimalCommitAuthor `json:"author,omitempty"` +} + // MinimalCommit is the trimmed output type for commit objects. type MinimalCommit struct { SHA string `json:"sha"` @@ -1609,6 +1617,44 @@ func convertToMinimalPRFiles(files []*github.CommitFile) []MinimalPRFile { return result } +func convertToMinimalPullRequestCommits(commits []*github.RepositoryCommit) []MinimalPullRequestCommit { + result := make([]MinimalPullRequestCommit, 0, len(commits)) + for _, commit := range commits { + if commit == nil { + continue + } + + minimalCommit := MinimalPullRequestCommit{ + SHA: commit.GetSHA(), + HTMLURL: commit.GetHTMLURL(), + } + + if commit.Commit != nil { + minimalCommit.Message = commit.Commit.GetMessage() + minimalCommit.Author = convertToMinimalCommitAuthor(commit.Commit.Author) + } + + result = append(result, minimalCommit) + } + return result +} + +func convertToMinimalCommitAuthor(author *github.CommitAuthor) *MinimalCommitAuthor { + if author == nil { + return nil + } + + minimalAuthor := &MinimalCommitAuthor{ + Name: author.GetName(), + Email: author.GetEmail(), + } + if author.Date != nil { + minimalAuthor.Date = author.Date.Format(time.RFC3339) + } + + return minimalAuthor +} + // convertToMinimalBranch converts a GitHub API Branch to MinimalBranch func convertToMinimalBranch(branch *github.Branch) MinimalBranch { return MinimalBranch{ diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 05028850d..5bbb18819 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -35,12 +35,13 @@ Possible options: 2. get_diff - Get the diff of a pull request. 3. get_status - Get combined commit status of a head commit in 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 review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results. - 6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned. - 7. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned. - 8. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR. + 5. get_commits - Get the list of commits on a pull request. Use with pagination parameters to control the number of results returned. + 6. get_review_comments - Get review threads on a pull request. Each thread contains logically grouped review comments made on the same code location during pull request reviews. Returns threads with metadata (isResolved, isOutdated, isCollapsed) and their associated comments. Use cursor-based pagination (perPage, after) to control results. + 7. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method. Use with pagination parameters to control the number of results returned. + 8. get_comments - Get comments on a pull request. Use this if user doesn't specifically want review comments. Use with pagination parameters to control the number of results returned. + 9. get_check_runs - Get check runs for the head commit of a pull request. Check runs are the individual CI/CD jobs and checks that run on the PR. `, - Enum: []any{"get", "get_diff", "get_status", "get_files", "get_review_comments", "get_reviews", "get_comments", "get_check_runs"}, + Enum: []any{"get", "get_diff", "get_status", "get_files", "get_commits", "get_review_comments", "get_reviews", "get_comments", "get_check_runs"}, }, "owner": { Type: "string", @@ -119,6 +120,9 @@ Possible options: case "get_files": result, err := GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination) return result, nil, err + case "get_commits": + result, err := GetPullRequestCommits(ctx, client, owner, repo, pullNumber, pagination) + return result, nil, err case "get_review_comments": gqlClient, err := deps.GetGQLClient(ctx) if err != nil { @@ -371,6 +375,34 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo return MarshalledTextResult(minimalFiles), nil } +func GetPullRequestCommits(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, + } + commits, resp, err := client.PullRequests.ListCommits(ctx, owner, repo, pullNumber, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get pull request commits", + 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 ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request commits", resp, body), nil + } + + minimalCommits := convertToMinimalPullRequestCommits(commits) + + return MarshalledTextResult(minimalCommits), nil +} + // GraphQL types for review threads query type reviewThreadsQuery struct { Repository struct { diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index aff71e4c1..2b911636a 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1260,6 +1260,185 @@ func Test_GetPullRequestFiles(t *testing.T) { } } +func Test_GetPullRequestCommits(t *testing.T) { + // Verify tool definition once + serverTool := PullRequestRead(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "pull_request_read", tool.Name) + assert.NotEmpty(t, tool.Description) + schema := tool.InputSchema.(*jsonschema.Schema) + assert.Contains(t, schema.Properties, "method") + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "pullNumber") + assert.Contains(t, schema.Properties, "page") + assert.Contains(t, schema.Properties, "perPage") + assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "pullNumber"}) + + authorDate := time.Date(2026, 4, 1, 12, 0, 0, 0, time.UTC) + mockCommits := []*github.RepositoryCommit{ + { + SHA: github.Ptr("abc123def456"), + HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"), + Commit: &github.Commit{ + Message: github.Ptr("feat: add commit listing"), + Author: &github.CommitAuthor{ + Name: github.Ptr("Test User"), + Email: github.Ptr("test@example.com"), + Date: &github.Timestamp{Time: authorDate}, + }, + Committer: &github.CommitAuthor{ + Name: github.Ptr("Merge Bot"), + Email: github.Ptr("merge@example.com"), + Date: &github.Timestamp{Time: authorDate.Add(30 * time.Minute)}, + }, + }, + Author: &github.User{ + Login: github.Ptr("test-user"), + ID: github.Ptr(int64(12345)), + HTMLURL: github.Ptr("https://github.com/test-user"), + AvatarURL: github.Ptr("https://github.com/test-user.png"), + }, + Committer: &github.User{ + Login: github.Ptr("merge-bot"), + ID: github.Ptr(int64(67890)), + HTMLURL: github.Ptr("https://github.com/merge-bot"), + AvatarURL: github.Ptr("https://github.com/merge-bot.png"), + }, + }, + { + SHA: github.Ptr("def456abc789"), + HTMLURL: github.Ptr("https://github.com/owner/repo/commit/def456abc789"), + Commit: &github.Commit{ + Message: github.Ptr("fix: handle pagination"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectError bool + expectedCommits []*github.RepositoryCommit + expectedErrMsg string + }{ + { + name: "successful commits fetch", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsCommitsByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockCommits), + ), + }), + requestArgs: map[string]any{ + "method": "get_commits", + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + }, + expectError: false, + expectedCommits: mockCommits, + }, + { + name: "successful commits fetch with pagination", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsCommitsByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{ + "page": "2", + "per_page": "10", + }).andThen( + mockResponse(t, http.StatusOK, mockCommits), + ), + }), + requestArgs: map[string]any{ + "method": "get_commits", + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "page": float64(2), + "perPage": float64(10), + }, + expectError: false, + expectedCommits: mockCommits, + }, + { + name: "commits fetch fails", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsCommitsByOwnerByRepoByPullNumber: expectQueryParams(t, map[string]string{ + "page": "1", + "per_page": "30", + }).andThen( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + }), + requestArgs: map[string]any{ + "method": "get_commits", + "owner": "owner", + "repo": "repo", + "pullNumber": float64(999), + }, + expectError: true, + expectedErrMsg: "failed to get pull request commits", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + serverTool := PullRequestRead(translations.NullTranslationHelper) + deps := BaseDeps{ + Client: client, + RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute), + Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}), + } + handler := serverTool.Handler(deps) + request := createMCPRequest(tc.requestArgs) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + if tc.expectError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + assert.NotContains(t, textContent.Text, `"committer"`) + assert.NotContains(t, textContent.Text, `"profile_url"`) + + var returnedCommits []MinimalPullRequestCommit + err = json.Unmarshal([]byte(textContent.Text), &returnedCommits) + require.NoError(t, err) + assert.Len(t, returnedCommits, len(tc.expectedCommits)) + for i, commit := range returnedCommits { + assert.Equal(t, tc.expectedCommits[i].GetSHA(), commit.SHA) + assert.Equal(t, tc.expectedCommits[i].GetHTMLURL(), commit.HTMLURL) + assert.Equal(t, tc.expectedCommits[i].GetCommit().GetMessage(), commit.Message) + } + + assert.Equal(t, authorDate.Format(time.RFC3339), returnedCommits[0].Author.Date) + }) + } +} + +func Test_ConvertToMinimalPullRequestCommitsSkipsNilCommit(t *testing.T) { + commits := convertToMinimalPullRequestCommits([]*github.RepositoryCommit{nil}) + + require.Empty(t, commits) +} + func Test_GetPullRequestStatus(t *testing.T) { // Verify tool definition once serverTool := PullRequestRead(translations.NullTranslationHelper)