Skip to content

Commit 58ad6a6

Browse files
authored
Merge branch 'main' into fix/logrus-to-slog
2 parents a683447 + cad048b commit 58ad6a6

File tree

6 files changed

+333
-106
lines changed

6 files changed

+333
-106
lines changed

github-mcp-server

17.5 MB
Binary file not shown.

pkg/github/__toolsnaps__/list_issues.snap

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
"description": "Order issues by field. If provided, the 'direction' also needs to be provided.",
3030
"enum": [
3131
"CREATED_AT",
32-
"UPDATED_AT"
32+
"UPDATED_AT",
33+
"COMMENTS"
3334
],
3435
"type": "string"
3536
},

pkg/github/issues.go

Lines changed: 42 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ type IssueFragment struct {
3838
Description githubv4.String
3939
}
4040
} `graphql:"labels(first: 100)"`
41+
Comments struct {
42+
TotalCount githubv4.Int
43+
} `graphql:"comments"`
4144
}
4245

4346
// Common interface for all issue query types
@@ -133,10 +136,11 @@ func fragmentToIssue(fragment IssueFragment) *github.Issue {
133136
User: &github.User{
134137
Login: github.Ptr(string(fragment.Author.Login)),
135138
},
136-
State: github.Ptr(string(fragment.State)),
137-
ID: github.Ptr(fragment.DatabaseID),
138-
Body: github.Ptr(string(fragment.Body)),
139-
Labels: foundLabels,
139+
State: github.Ptr(string(fragment.State)),
140+
ID: github.Ptr(fragment.DatabaseID),
141+
Body: github.Ptr(string(fragment.Body)),
142+
Labels: foundLabels,
143+
Comments: github.Ptr(int(fragment.Comments.TotalCount)),
140144
}
141145
}
142146

@@ -502,67 +506,39 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc)
502506
}
503507

504508
client, err := getClient(ctx)
505-
if err != nil {
506-
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
507-
}
508-
509-
// Create the request body
510-
requestBody := map[string]interface{}{
511-
"sub_issue_id": subIssueID,
512-
}
513-
reqBodyBytes, err := json.Marshal(requestBody)
514-
if err != nil {
515-
return nil, fmt.Errorf("failed to marshal request body: %w", err)
516-
}
517-
518-
// Create the HTTP request
519-
url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issue",
520-
client.BaseURL.String(), owner, repo, issueNumber)
521-
req, err := http.NewRequestWithContext(ctx, "DELETE", url, strings.NewReader(string(reqBodyBytes)))
522-
if err != nil {
523-
return nil, fmt.Errorf("failed to create request: %w", err)
524-
}
525-
req.Header.Set("Accept", "application/vnd.github+json")
526-
req.Header.Set("Content-Type", "application/json")
527-
req.Header.Set("X-GitHub-Api-Version", "2022-11-28")
528-
529-
httpClient := client.Client() // Use authenticated GitHub client
530-
resp, err := httpClient.Do(req)
531-
if err != nil {
532-
var ghResp *github.Response
533-
if resp != nil {
534-
ghResp = &github.Response{Response: resp}
535-
}
536-
return ghErrors.NewGitHubAPIErrorResponse(ctx,
537-
"failed to remove sub-issue",
538-
ghResp,
539-
err,
540-
), nil
541-
}
542-
defer func() { _ = resp.Body.Close() }()
543-
544-
body, err := io.ReadAll(resp.Body)
545-
if err != nil {
546-
return nil, fmt.Errorf("failed to read response body: %w", err)
547-
}
548-
549-
if resp.StatusCode != http.StatusOK {
550-
return mcp.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil
551-
}
552-
553-
// Parse and re-marshal to ensure consistent formatting
554-
var result interface{}
555-
if err := json.Unmarshal(body, &result); err != nil {
556-
return nil, fmt.Errorf("failed to unmarshal response: %w", err)
557-
}
558-
559-
r, err := json.Marshal(result)
560-
if err != nil {
561-
return nil, fmt.Errorf("failed to marshal response: %w", err)
562-
}
563-
564-
return mcp.NewToolResultText(string(r)), nil
565-
}
509+
if err != nil {
510+
return nil, fmt.Errorf("failed to get GitHub client: %w", err)
511+
}
512+
513+
subIssueRequest := github.SubIssueRequest{
514+
SubIssueID: int64(subIssueID),
515+
}
516+
517+
subIssue, resp, err := client.SubIssue.Remove(ctx, owner, repo, int64(issueNumber), subIssueRequest)
518+
if err != nil {
519+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
520+
"failed to remove sub-issue",
521+
resp,
522+
err,
523+
), nil
524+
}
525+
defer func() { _ = resp.Body.Close() }()
526+
527+
if resp.StatusCode != http.StatusOK {
528+
body, err := io.ReadAll(resp.Body)
529+
if err != nil {
530+
return nil, fmt.Errorf("failed to read response body: %w", err)
531+
}
532+
return mcp.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil
533+
}
534+
535+
r, err := json.Marshal(subIssue)
536+
if err != nil {
537+
return nil, fmt.Errorf("failed to marshal response: %w", err)
538+
}
539+
540+
return mcp.NewToolResultText(string(r)), nil
541+
}
566542
}
567543

568544
// ReprioritizeSubIssue creates a tool to reprioritize a sub-issue to a different position in the parent list.
@@ -875,7 +851,7 @@ func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFun
875851
),
876852
mcp.WithString("orderBy",
877853
mcp.Description("Order issues by field. If provided, the 'direction' also needs to be provided."),
878-
mcp.Enum("CREATED_AT", "UPDATED_AT"),
854+
mcp.Enum("CREATED_AT", "UPDATED_AT", "COMMENTS"),
879855
),
880856
mcp.WithString("direction",
881857
mcp.Description("Order direction. If provided, the 'orderBy' also needs to be provided."),

pkg/github/issues_test.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,9 @@ func Test_ListIssues(t *testing.T) {
681681
{"name": "bug", "id": "label1", "description": "Bug label"},
682682
},
683683
},
684+
"comments": map[string]any{
685+
"totalCount": 5,
686+
},
684687
},
685688
{
686689
"number": 456,
@@ -696,6 +699,9 @@ func Test_ListIssues(t *testing.T) {
696699
{"name": "enhancement", "id": "label2", "description": "Enhancement label"},
697700
},
698701
},
702+
"comments": map[string]any{
703+
"totalCount": 3,
704+
},
699705
},
700706
}
701707

@@ -713,6 +719,9 @@ func Test_ListIssues(t *testing.T) {
713719
"labels": map[string]any{
714720
"nodes": []map[string]any{},
715721
},
722+
"comments": map[string]any{
723+
"totalCount": 1,
724+
},
716725
},
717726
}
718727

@@ -875,8 +884,8 @@ func Test_ListIssues(t *testing.T) {
875884
}
876885

877886
// Define the actual query strings that match the implementation
878-
qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
879-
qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
887+
qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
888+
qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
880889

881890
for _, tc := range tests {
882891
t.Run(tc.name, func(t *testing.T) {

pkg/github/repositories.go

Lines changed: 80 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,36 +1358,100 @@ func filterPaths(entries []*github.TreeEntry, path string, maxResults int) []str
13581358
return matchedPaths
13591359
}
13601360

1361-
// resolveGitReference resolves git references with the following logic:
1362-
// 1. If SHA is provided, it takes precedence
1363-
// 2. If neither is provided, use the default branch as ref
1364-
// 3. Get commit SHA from the ref
1365-
// Refs can look like `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head`
1366-
// The function returns the resolved ref, commit SHA and any error.
1361+
// resolveGitReference takes a user-provided ref and sha and resolves them into a
1362+
// definitive commit SHA and its corresponding fully-qualified reference.
1363+
//
1364+
// The resolution logic follows a clear priority:
1365+
//
1366+
// 1. If a specific commit `sha` is provided, it takes precedence and is used directly,
1367+
// and all reference resolution is skipped.
1368+
//
1369+
// 2. If no `sha` is provided, the function resolves the `ref`
1370+
// string into a fully-qualified format (e.g., "refs/heads/main") by trying
1371+
// the following steps in order:
1372+
// a). **Empty Ref:** If `ref` is empty, the repository's default branch is used.
1373+
// b). **Fully-Qualified:** If `ref` already starts with "refs/", it's considered fully
1374+
// qualified and used as-is.
1375+
// c). **Partially-Qualified:** If `ref` starts with "heads/" or "tags/", it is
1376+
// prefixed with "refs/" to make it fully-qualified.
1377+
// d). **Short Name:** Otherwise, the `ref` is treated as a short name. The function
1378+
// first attempts to resolve it as a branch ("refs/heads/<ref>"). If that
1379+
// returns a 404 Not Found error, it then attempts to resolve it as a tag
1380+
// ("refs/tags/<ref>").
1381+
//
1382+
// 3. **Final Lookup:** Once a fully-qualified ref is determined, a final API call
1383+
// is made to fetch that reference's definitive commit SHA.
1384+
//
1385+
// Any unexpected (non-404) errors during the resolution process are returned
1386+
// immediately. All API errors are logged with rich context to aid diagnostics.
13671387
func resolveGitReference(ctx context.Context, githubClient *github.Client, owner, repo, ref, sha string) (*raw.ContentOpts, error) {
1368-
// 1. If SHA is provided, use it directly
1388+
// 1) If SHA explicitly provided, it's the highest priority.
13691389
if sha != "" {
13701390
return &raw.ContentOpts{Ref: "", SHA: sha}, nil
13711391
}
13721392

1373-
// 2. If neither provided, use the default branch as ref
1374-
if ref == "" {
1393+
originalRef := ref // Keep original ref for clearer error messages down the line.
1394+
1395+
// 2) If no SHA is provided, we try to resolve the ref into a fully-qualified format.
1396+
var reference *github.Reference
1397+
var resp *github.Response
1398+
var err error
1399+
1400+
switch {
1401+
case originalRef == "":
1402+
// 2a) If ref is empty, determine the default branch.
13751403
repoInfo, resp, err := githubClient.Repositories.Get(ctx, owner, repo)
13761404
if err != nil {
13771405
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get repository info", resp, err)
13781406
return nil, fmt.Errorf("failed to get repository info: %w", err)
13791407
}
13801408
ref = fmt.Sprintf("refs/heads/%s", repoInfo.GetDefaultBranch())
1409+
case strings.HasPrefix(originalRef, "refs/"):
1410+
// 2b) Already fully qualified. The reference will be fetched at the end.
1411+
case strings.HasPrefix(originalRef, "heads/") || strings.HasPrefix(originalRef, "tags/"):
1412+
// 2c) Partially qualified. Make it fully qualified.
1413+
ref = "refs/" + originalRef
1414+
default:
1415+
// 2d) It's a short name, so we try to resolve it to either a branch or a tag.
1416+
branchRef := "refs/heads/" + originalRef
1417+
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, branchRef)
1418+
1419+
if err == nil {
1420+
ref = branchRef // It's a branch.
1421+
} else {
1422+
// The branch lookup failed. Check if it was a 404 Not Found error.
1423+
ghErr, isGhErr := err.(*github.ErrorResponse)
1424+
if isGhErr && ghErr.Response.StatusCode == http.StatusNotFound {
1425+
tagRef := "refs/tags/" + originalRef
1426+
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, tagRef)
1427+
if err == nil {
1428+
ref = tagRef // It's a tag.
1429+
} else {
1430+
// The tag lookup also failed. Check if it was a 404 Not Found error.
1431+
ghErr2, isGhErr2 := err.(*github.ErrorResponse)
1432+
if isGhErr2 && ghErr2.Response.StatusCode == http.StatusNotFound {
1433+
return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef)
1434+
}
1435+
// The tag lookup failed for a different reason.
1436+
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err)
1437+
return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err)
1438+
}
1439+
} else {
1440+
// The branch lookup failed for a different reason.
1441+
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (branch)", resp, err)
1442+
return nil, fmt.Errorf("failed to get reference for branch '%s': %w", originalRef, err)
1443+
}
1444+
}
13811445
}
13821446

1383-
// 3. Get the SHA from the ref
1384-
reference, resp, err := githubClient.Git.GetRef(ctx, owner, repo, ref)
1385-
if err != nil {
1386-
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference", resp, err)
1387-
return nil, fmt.Errorf("failed to get reference: %w", err)
1447+
if reference == nil {
1448+
reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, ref)
1449+
if err != nil {
1450+
_, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get final reference", resp, err)
1451+
return nil, fmt.Errorf("failed to get final reference for %q: %w", ref, err)
1452+
}
13881453
}
1389-
sha = reference.GetObject().GetSHA()
13901454

1391-
// Use provided ref, or it will be empty which defaults to the default branch
1455+
sha = reference.GetObject().GetSHA()
13921456
return &raw.ContentOpts{Ref: ref, SHA: sha}, nil
13931457
}

0 commit comments

Comments
 (0)