feat: Add get_commits method to pull_request_read (#2608)
* feat: Add get_commits method to pull_request_read * Add nil check and additional test case --------- Co-authored-by: Sam Morrow <info@sam-morrow.com>
This commit is contained in:
Ross Tarrant
@@ -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)
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -36,12 +36,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",
|
||||
@@ -130,6 +131,9 @@ Possible options:
|
||||
case "get_files":
|
||||
result, err := GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination)
|
||||
return attachIFC(result), nil, err
|
||||
case "get_commits":
|
||||
result, err := GetPullRequestCommits(ctx, client, owner, repo, pullNumber, pagination)
|
||||
return attachIFC(result), nil, err
|
||||
case "get_review_comments":
|
||||
gqlClient, err := deps.GetGQLClient(ctx)
|
||||
if err != nil {
|
||||
@@ -382,6 +386,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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user