Skip to content

Commit f62be1a

Browse files
Port functional changes from main to registry pattern
Port three functional improvements from main branch: - GraphQL review comments grouped as threads (#1554) - get_file_contents description improvement (#1582) - create_or_update_file SHA validation fix (#1621) Adapted implementations to use the new registry pattern with: - BaseDeps for providing clients via ToolDependencies interface - deps.GetClient(ctx) and deps.GetGQLClient(ctx) patterns - Updated tests to use GraphQL mocks for review comments - Added SHA validation test cases for create_or_update_file
1 parent 982b12d commit f62be1a

File tree

7 files changed

+661
-202
lines changed

7 files changed

+661
-202
lines changed

pkg/github/__toolsnaps__/create_or_update_file.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"annotations": {
33
"title": "Create or update file"
44
},
5-
"description": "Create or update a single file in a GitHub repository. If updating, you must provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.",
5+
"description": "Create or update a single file in a GitHub repository. \nIf updating, you should provide the SHA of the file you want to update. Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations.\n\nIn order to obtain the SHA of original file version before updating, use the following git command:\ngit ls-tree HEAD \u003cpath to file\u003e\n\nIf the SHA is not provided, the tool will attempt to acquire it by fetching the current file contents from the repository, which may lead to rewriting latest committed changes if the file has changed since last retrieval.\n",
66
"inputSchema": {
77
"type": "object",
88
"required": [
@@ -40,7 +40,7 @@
4040
},
4141
"sha": {
4242
"type": "string",
43-
"description": "Required if updating an existing file. The blob SHA of the file being replaced."
43+
"description": "The blob SHA of the file being replaced."
4444
}
4545
}
4646
},

pkg/github/__toolsnaps__/get_file_contents.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
},
1818
"path": {
1919
"type": "string",
20-
"description": "Path to file/directory (directories must end with a slash '/')",
20+
"description": "Path to file/directory",
2121
"default": "/"
2222
},
2323
"ref": {

pkg/github/__toolsnaps__/pull_request_read.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"properties": {
1616
"method": {
1717
"type": "string",
18-
"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. They are comments made on a portion of the unified diff during a pull request review. 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 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",
18+
"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 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.\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",
1919
"enum": [
2020
"get",
2121
"get_diff",

pkg/github/pullrequests.go

Lines changed: 113 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Possible options:
3434
2. get_diff - Get the diff of a pull request.
3535
3. get_status - Get status of a head commit in a pull request. This reflects status of builds and checks.
3636
4. get_files - Get the list of files changed in a pull request. Use with pagination parameters to control the number of results returned.
37-
5. get_review_comments - Get the review comments on a pull request. They are comments made on a portion of the unified diff during a pull request review. Use with pagination parameters to control the number of results returned.
37+
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.
3838
6. get_reviews - Get the reviews on a pull request. When asked for review comments, use get_review_comments method.
3939
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.
4040
`,
@@ -111,7 +111,15 @@ Possible options:
111111
result, err := GetPullRequestFiles(ctx, client, owner, repo, pullNumber, pagination)
112112
return result, nil, err
113113
case "get_review_comments":
114-
result, err := GetPullRequestReviewComments(ctx, client, deps.GetRepoAccessCache(), owner, repo, pullNumber, pagination, deps.GetFlags())
114+
gqlClient, err := deps.GetGQLClient(ctx)
115+
if err != nil {
116+
return utils.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil, nil
117+
}
118+
cursorPagination, err := OptionalCursorPaginationParams(args)
119+
if err != nil {
120+
return utils.NewToolResultError(err.Error()), nil, nil
121+
}
122+
result, err := GetPullRequestReviewComments(ctx, gqlClient, deps.GetRepoAccessCache(), owner, repo, pullNumber, cursorPagination, deps.GetFlags())
115123
return result, nil, err
116124
case "get_reviews":
117125
result, err := GetPullRequestReviews(ctx, client, deps.GetRepoAccessCache(), owner, repo, pullNumber, deps.GetFlags())
@@ -287,54 +295,124 @@ func GetPullRequestFiles(ctx context.Context, client *github.Client, owner, repo
287295
return utils.NewToolResultText(string(r)), nil
288296
}
289297

290-
func GetPullRequestReviewComments(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner, repo string, pullNumber int, pagination PaginationParams, ff FeatureFlags) (*mcp.CallToolResult, error) {
291-
opts := &github.PullRequestListCommentsOptions{
292-
ListOptions: github.ListOptions{
293-
PerPage: pagination.PerPage,
294-
Page: pagination.Page,
295-
},
298+
// GraphQL types for review threads query
299+
type reviewThreadsQuery struct {
300+
Repository struct {
301+
PullRequest struct {
302+
ReviewThreads struct {
303+
Nodes []reviewThreadNode
304+
PageInfo pageInfoFragment
305+
TotalCount githubv4.Int
306+
} `graphql:"reviewThreads(first: $first, after: $after)"`
307+
} `graphql:"pullRequest(number: $prNum)"`
308+
} `graphql:"repository(owner: $owner, name: $repo)"`
309+
}
310+
311+
type reviewThreadNode struct {
312+
ID githubv4.ID
313+
IsResolved githubv4.Boolean
314+
IsOutdated githubv4.Boolean
315+
IsCollapsed githubv4.Boolean
316+
Comments struct {
317+
Nodes []reviewCommentNode
318+
TotalCount githubv4.Int
319+
} `graphql:"comments(first: $commentsPerThread)"`
320+
}
321+
322+
type reviewCommentNode struct {
323+
ID githubv4.ID
324+
Body githubv4.String
325+
Path githubv4.String
326+
Line *githubv4.Int
327+
Author struct {
328+
Login githubv4.String
296329
}
330+
CreatedAt githubv4.DateTime
331+
UpdatedAt githubv4.DateTime
332+
URL githubv4.URI
333+
}
297334

298-
comments, resp, err := client.PullRequests.ListComments(ctx, owner, repo, pullNumber, opts)
335+
type pageInfoFragment struct {
336+
HasNextPage githubv4.Boolean
337+
HasPreviousPage githubv4.Boolean
338+
StartCursor githubv4.String
339+
EndCursor githubv4.String
340+
}
341+
342+
func GetPullRequestReviewComments(ctx context.Context, gqlClient *githubv4.Client, cache *lockdown.RepoAccessCache, owner, repo string, pullNumber int, pagination CursorPaginationParams, ff FeatureFlags) (*mcp.CallToolResult, error) {
343+
// Convert pagination parameters to GraphQL format
344+
gqlParams, err := pagination.ToGraphQLParams()
299345
if err != nil {
300-
return ghErrors.NewGitHubAPIErrorResponse(ctx,
301-
"failed to get pull request review comments",
302-
resp,
303-
err,
304-
), nil
346+
return utils.NewToolResultError(fmt.Sprintf("invalid pagination parameters: %v", err)), nil
305347
}
306-
defer func() { _ = resp.Body.Close() }()
307348

308-
if resp.StatusCode != http.StatusOK {
309-
body, err := io.ReadAll(resp.Body)
310-
if err != nil {
311-
return nil, fmt.Errorf("failed to read response body: %w", err)
312-
}
313-
return utils.NewToolResultError(fmt.Sprintf("failed to get pull request review comments: %s", string(body))), nil
349+
// Build variables for GraphQL query
350+
vars := map[string]any{
351+
"owner": githubv4.String(owner),
352+
"repo": githubv4.String(repo),
353+
"prNum": githubv4.Int(int32(pullNumber)), //nolint:gosec // pullNumber is controlled by user input validation
354+
"first": githubv4.Int(*gqlParams.First),
355+
"commentsPerThread": githubv4.Int(100),
356+
}
357+
358+
// Add cursor if provided
359+
if gqlParams.After != nil {
360+
vars["after"] = githubv4.String(*gqlParams.After)
361+
} else {
362+
vars["after"] = (*githubv4.String)(nil)
314363
}
315364

365+
// Execute GraphQL query
366+
var query reviewThreadsQuery
367+
if err := gqlClient.Query(ctx, &query, vars); err != nil {
368+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx,
369+
"failed to get pull request review threads",
370+
err,
371+
), nil
372+
}
373+
374+
// Lockdown mode filtering
316375
if ff.LockdownMode {
317376
if cache == nil {
318377
return nil, fmt.Errorf("lockdown cache is not configured")
319378
}
320-
filteredComments := make([]*github.PullRequestComment, 0, len(comments))
321-
for _, comment := range comments {
322-
user := comment.GetUser()
323-
if user == nil {
324-
continue
325-
}
326-
isSafeContent, err := cache.IsSafeContent(ctx, user.GetLogin(), owner, repo)
327-
if err != nil {
328-
return utils.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil
329-
}
330-
if isSafeContent {
331-
filteredComments = append(filteredComments, comment)
379+
380+
// Iterate through threads and filter comments
381+
for i := range query.Repository.PullRequest.ReviewThreads.Nodes {
382+
thread := &query.Repository.PullRequest.ReviewThreads.Nodes[i]
383+
filteredComments := make([]reviewCommentNode, 0, len(thread.Comments.Nodes))
384+
385+
for _, comment := range thread.Comments.Nodes {
386+
login := string(comment.Author.Login)
387+
if login != "" {
388+
isSafeContent, err := cache.IsSafeContent(ctx, login, owner, repo)
389+
if err != nil {
390+
return nil, fmt.Errorf("failed to check lockdown mode: %w", err)
391+
}
392+
if isSafeContent {
393+
filteredComments = append(filteredComments, comment)
394+
}
395+
}
332396
}
397+
398+
thread.Comments.Nodes = filteredComments
399+
thread.Comments.TotalCount = githubv4.Int(int32(len(filteredComments))) //nolint:gosec // comment count is bounded by API limits
333400
}
334-
comments = filteredComments
335401
}
336402

337-
r, err := json.Marshal(comments)
403+
// Build response with review threads and pagination info
404+
response := map[string]any{
405+
"reviewThreads": query.Repository.PullRequest.ReviewThreads.Nodes,
406+
"pageInfo": map[string]any{
407+
"hasNextPage": query.Repository.PullRequest.ReviewThreads.PageInfo.HasNextPage,
408+
"hasPreviousPage": query.Repository.PullRequest.ReviewThreads.PageInfo.HasPreviousPage,
409+
"startCursor": string(query.Repository.PullRequest.ReviewThreads.PageInfo.StartCursor),
410+
"endCursor": string(query.Repository.PullRequest.ReviewThreads.PageInfo.EndCursor),
411+
},
412+
"totalCount": int(query.Repository.PullRequest.ReviewThreads.TotalCount),
413+
}
414+
415+
r, err := json.Marshal(response)
338416
if err != nil {
339417
return nil, fmt.Errorf("failed to marshal response: %w", err)
340418
}

0 commit comments

Comments
 (0)