Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions pkg/github/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

ghErrors "github.com/github/github-mcp-server/pkg/errors"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/response"
"github.com/github/github-mcp-server/pkg/sanitize"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
Expand Down Expand Up @@ -1598,9 +1599,21 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
totalCount = fragment.TotalCount
}

// Create response with issues
response := map[string]any{
"issues": issues,
minimalIssues := make([]MinimalIssue, 0, len(issues))
for _, issue := range issues {
if issue != nil {
minimalIssues = append(minimalIssues, convertToMinimalIssue(issue))
}
}

optimizedIssues, err := response.OptimizeList(minimalIssues)
if err != nil {
return nil, nil, fmt.Errorf("failed to optimize issues: %w", err)
}

// Wrap optimized issues with pagination metadata
issueResponse := map[string]any{
"issues": json.RawMessage(optimizedIssues),
"pageInfo": map[string]any{
"hasNextPage": pageInfo.HasNextPage,
"hasPreviousPage": pageInfo.HasPreviousPage,
Expand All @@ -1609,7 +1622,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
},
"totalCount": totalCount,
}
out, err := json.Marshal(response)
out, err := json.Marshal(issueResponse)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal issues: %w", err)
}
Expand Down
38 changes: 17 additions & 21 deletions pkg/github/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,6 @@ func Test_ListIssues(t *testing.T) {
expectError bool
errContains string
expectedCount int
verifyOrder func(t *testing.T, issues []*github.Issue)
}{
{
name: "list all issues",
Expand Down Expand Up @@ -1296,32 +1295,29 @@ func Test_ListIssues(t *testing.T) {
}
require.NoError(t, err)

// Parse the structured response with pagination info
var response struct {
Issues []*github.Issue `json:"issues"`
PageInfo struct {
HasNextPage bool `json:"hasNextPage"`
HasPreviousPage bool `json:"hasPreviousPage"`
StartCursor string `json:"startCursor"`
EndCursor string `json:"endCursor"`
} `json:"pageInfo"`
TotalCount int `json:"totalCount"`
}
// Parse the response
var response map[string]any
err = json.Unmarshal([]byte(text), &response)
require.NoError(t, err)

assert.Len(t, response.Issues, tc.expectedCount, "Expected %d issues, got %d", tc.expectedCount, len(response.Issues))
// Metadata should be preserved
assert.NotNil(t, response["totalCount"])
pageInfo, ok := response["pageInfo"].(map[string]any)
require.True(t, ok, "pageInfo should be a map")
assert.Contains(t, pageInfo, "hasNextPage")
assert.Contains(t, pageInfo, "endCursor")

// Verify order if verifyOrder function is provided
if tc.verifyOrder != nil {
tc.verifyOrder(t, response.Issues)
}
issues, ok := response["issues"].([]any)
require.True(t, ok)
assert.Len(t, issues, tc.expectedCount, "Expected %d issues, got %d", tc.expectedCount, len(issues))

// Verify that returned issues have expected structure
for _, issue := range response.Issues {
assert.NotNil(t, issue.Number, "Issue should have number")
assert.NotNil(t, issue.Title, "Issue should have title")
assert.NotNil(t, issue.State, "Issue should have state")
for _, issue := range issues {
m, ok := issue.(map[string]any)
require.True(t, ok)
assert.NotNil(t, m["number"], "Issue should have number")
assert.NotNil(t, m["title"], "Issue should have title")
assert.NotNil(t, m["state"], "Issue should have state")
}
})
}
Expand Down
32 changes: 32 additions & 0 deletions pkg/github/minimal_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ type MinimalBranch struct {
Protected bool `json:"protected"`
}

// MinimalTag is the trimmed output type for tag objects.
type MinimalTag struct {
Name string `json:"name"`
SHA string `json:"sha"`
}

// MinimalResponse represents a minimal response for all CRUD operations.
// Success is implicit in the HTTP response status, and all other information
// can be derived from the URL or fetched separately if needed.
Expand Down Expand Up @@ -672,6 +678,32 @@ func convertToMinimalBranch(branch *github.Branch) MinimalBranch {
}
}

func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
m := MinimalRelease{
ID: release.GetID(),
TagName: release.GetTagName(),
Name: release.GetName(),
Body: release.GetBody(),
HTMLURL: release.GetHTMLURL(),
Prerelease: release.GetPrerelease(),
Draft: release.GetDraft(),
Author: convertToMinimalUser(release.GetAuthor()),
}

if release.PublishedAt != nil {
m.PublishedAt = release.PublishedAt.Format(time.RFC3339)
}

return m
}

func convertToMinimalTag(tag *github.RepositoryTag) MinimalTag {
return MinimalTag{
Name: tag.GetName(),
SHA: tag.GetCommit().GetSHA(),
}
}

func convertToMinimalReviewThreadsResponse(query reviewThreadsQuery) MinimalReviewThreadsResponse {
threads := query.Repository.PullRequest.ReviewThreads

Expand Down
12 changes: 11 additions & 1 deletion pkg/github/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
ghErrors "github.com/github/github-mcp-server/pkg/errors"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/octicons"
"github.com/github/github-mcp-server/pkg/response"
"github.com/github/github-mcp-server/pkg/sanitize"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
Expand Down Expand Up @@ -1148,7 +1149,16 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool
}
}

r, err := json.Marshal(prs)
minimalPRs := make([]MinimalPullRequest, 0, len(prs))
for _, pr := range prs {
if pr != nil {
minimalPRs = append(minimalPRs, convertToMinimalPullRequest(pr))
}
}

r, err := response.OptimizeList(minimalPRs,
response.WithPreservedFields("html_url", "draft"),
)
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
}
Expand Down
28 changes: 24 additions & 4 deletions pkg/github/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
ghErrors "github.com/github/github-mcp-server/pkg/errors"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/octicons"
"github.com/github/github-mcp-server/pkg/response"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/github/github-mcp-server/pkg/utils"
Expand Down Expand Up @@ -216,7 +217,10 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
minimalCommits[i] = convertToMinimalCommit(commit, false)
}

r, err := json.Marshal(minimalCommits)
r, err := response.OptimizeList(minimalCommits,
response.WithMaxDepth(3),
response.WithPreservedFields("html_url"),
)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
Expand Down Expand Up @@ -303,7 +307,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool {
minimalBranches = append(minimalBranches, convertToMinimalBranch(branch))
}

r, err := json.Marshal(minimalBranches)
r, err := response.OptimizeList(minimalBranches)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
Expand Down Expand Up @@ -1497,7 +1501,14 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list tags", resp, body), nil, nil
}

r, err := json.Marshal(tags)
minimalTags := make([]MinimalTag, 0, len(tags))
for _, tag := range tags {
if tag != nil {
minimalTags = append(minimalTags, convertToMinimalTag(tag))
}
}

r, err := response.OptimizeList(minimalTags)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
Expand Down Expand Up @@ -1670,7 +1681,16 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list releases", resp, body), nil, nil
}

r, err := json.Marshal(releases)
minimalReleases := make([]MinimalRelease, 0, len(releases))
for _, release := range releases {
if release != nil {
minimalReleases = append(minimalReleases, convertToMinimalRelease(release))
}
}

r, err := response.OptimizeList(minimalReleases,
response.WithPreservedFields("html_url", "prerelease"),
)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
Expand Down
27 changes: 17 additions & 10 deletions pkg/github/repositories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,23 +1053,30 @@ func Test_ListCommits(t *testing.T) {
textContent := getTextResult(t, result)

// Unmarshal and verify the result
var returnedCommits []MinimalCommit
var returnedCommits []map[string]any
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].GetSHA(), commit["sha"])
assert.Equal(t, tc.expectedCommits[i].GetHTMLURL(), commit["html_url"])
if tc.expectedCommits[i].Commit != nil {
assert.Equal(t, tc.expectedCommits[i].Commit.GetMessage(), commit.Commit.Message)
assert.Equal(t, tc.expectedCommits[i].Commit.GetMessage(), commit["commit.message"])
if tc.expectedCommits[i].Commit.Author != nil {
assert.Equal(t, tc.expectedCommits[i].Commit.Author.GetName(), commit["commit.author.name"])
assert.Equal(t, tc.expectedCommits[i].Commit.Author.GetEmail(), commit["commit.author.email"])
if tc.expectedCommits[i].Commit.Author.Date != nil {
assert.NotEmpty(t, commit["commit.author.date"], "commit.author.date should be present")
}
}
}
if tc.expectedCommits[i].Author != nil {
assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit.Author.Login)
assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit["author.login"])
}

// Files and stats are never included in list_commits
assert.Nil(t, commit.Files)
assert.Nil(t, commit.Stats)
assert.Nil(t, commit["files"])
assert.Nil(t, commit["stats"])
}
})
}
Expand Down Expand Up @@ -2791,15 +2798,15 @@ func Test_ListTags(t *testing.T) {
textContent := getTextResult(t, result)

// Parse and verify the result
var returnedTags []*github.RepositoryTag
var returnedTags []map[string]any
err = json.Unmarshal([]byte(textContent.Text), &returnedTags)
require.NoError(t, err)

// Verify each tag
require.Equal(t, len(tc.expectedTags), len(returnedTags))
for i, expectedTag := range tc.expectedTags {
assert.Equal(t, *expectedTag.Name, *returnedTags[i].Name)
assert.Equal(t, *expectedTag.Commit.SHA, *returnedTags[i].Commit.SHA)
assert.Equal(t, *expectedTag.Name, returnedTags[i]["name"])
assert.Equal(t, *expectedTag.Commit.SHA, returnedTags[i]["sha"])
}
})
}
Expand Down
Loading