Skip to content
5 changes: 3 additions & 2 deletions pkg/github/issues.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/sanitize"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
Expand Down Expand Up @@ -1602,7 +1603,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
}

// Create response with issues
response := map[string]any{
issueResponse := map[string]any{
"issues": issues,
"pageInfo": map[string]any{
"hasNextPage": pageInfo.HasNextPage,
Expand All @@ -1612,7 +1613,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
},
"totalCount": totalCount,
}
out, err := json.Marshal(response)
out, err := response.MarshalItems(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
3 changes: 2 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 @@ -1168,7 +1169,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool
}
}

r, err := json.Marshal(prs)
r, err := response.MarshalItems(prs)
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
}
Expand Down
9 changes: 5 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,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
minimalCommits[i] = convertToMinimalCommit(commit, false)
}

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

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

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

r, err := json.Marshal(releases)
r, err := response.MarshalItems(releases)
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]["commit.sha"])
}
})
}
Expand Down
Loading