Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9ef46dd
add response optimisation pkg
kerobbi Feb 16, 2026
f5291c4
add tests
kerobbi Feb 16, 2026
5d28623
write remaining list tools, make flatten recursive with per-tool dept…
kerobbi Feb 17, 2026
7859111
wire list_branches
kerobbi Feb 17, 2026
07b71f1
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 17, 2026
70deafb
unwire list_branches and list_issue_types
kerobbi Feb 18, 2026
77695c7
add prerelease and requested_teams to configs
kerobbi Feb 18, 2026
92e6f44
rewire list_branches
kerobbi Feb 18, 2026
14dd881
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 18, 2026
3a1d18d
improve comment
kerobbi Feb 18, 2026
c960f49
simplify isUrlKey
kerobbi Feb 18, 2026
b651887
refactor MarshalItems with generics and per-tool config
kerobbi Feb 19, 2026
c75ab7d
fix tests
kerobbi Feb 19, 2026
2791676
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 19, 2026
afd1eba
refactor WithPreservedFields
kerobbi Feb 20, 2026
de12f8f
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 23, 2026
86a41c2
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 23, 2026
aa635d6
wire minimal types into list tools, remove collection handling from p…
kerobbi Feb 23, 2026
67699f0
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 24, 2026
c29fcb5
fix nil check, make WithPreservedFields accumulate
kerobbi Feb 24, 2026
f21b014
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 24, 2026
a7f3373
address feedback: add nil guard, add protected to preserved fields, a…
kerobbi Feb 24, 2026
671301f
Merge branch 'main' into kerobbi/response-optimisation
kerobbi Feb 24, 2026
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 @@ -1603,9 +1604,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 utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
}

// Wrap optimized issues with pagination metadata
issueResponse := map[string]any{
"issues": json.RawMessage(optimizedIssues),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For empty issues list we used to return an empty list {"issues":[],...}
which is no longer the case after this optimization

Copy link
Contributor Author

@kerobbi kerobbi Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OptimizeList still produces [] on an empty slice 🤔 I've also just tested it against an issue-less repo and this is what was returned:

{"issues":[],"pageInfo":{"endCursor":"","hasNextPage":false,"hasPreviousPage":false,"startCursor":""},"totalCount":0}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a nil guard in OptimizeList, so that [] is returned in that case too.

"pageInfo": map[string]any{
"hasNextPage": pageInfo.HasNextPage,
"hasPreviousPage": pageInfo.HasPreviousPage,
Expand All @@ -1614,7 +1627,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 @@ -1187,7 +1187,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 @@ -1295,32 +1294,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
37 changes: 37 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 @@ -702,6 +708,37 @@ 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 {
m := MinimalTag{
Name: tag.GetName(),
}

if commit := tag.GetCommit(); commit != nil {
m.SHA = commit.GetSHA()
}

return m
}

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
30 changes: 26 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,9 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool {
minimalBranches = append(minimalBranches, convertToMinimalBranch(branch))
}

r, err := json.Marshal(minimalBranches)
r, err := response.OptimizeList(minimalBranches,
response.WithPreservedFields("protected"),
)
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
Expand Down Expand Up @@ -1497,7 +1503,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 +1683,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