Skip to content

Commit 6e89459

Browse files
committed
Warn when large review comment payload may truncate
1 parent 94ee074 commit 6e89459

File tree

2 files changed

+72
-2
lines changed

2 files changed

+72
-2
lines changed

pkg/github/pullrequests.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ import (
2020
"github.com/github/github-mcp-server/pkg/utils"
2121
)
2222

23+
// Some MCP clients truncate large tool responses. This heuristic adds a leading
24+
// warning when a marshaled payload is likely to exceed those limits so agents
25+
// can retry with smaller pages.
26+
const mcpLargePayloadWarningThresholdBytes = 8_000
27+
2328
// PullRequestRead creates a tool to get details of a specific pull request.
2429
func PullRequestRead(getClient GetClientFn, cache *lockdown.RepoAccessCache, t translations.TranslationHelperFunc, flags FeatureFlags) (mcp.Tool, mcp.ToolHandlerFor[map[string]any, any]) {
2530
schema := &jsonschema.Schema{
@@ -329,12 +334,25 @@ func GetPullRequestReviewComments(ctx context.Context, client *github.Client, ca
329334
comments = filteredComments
330335
}
331336

332-
r, err := json.Marshal(comments)
337+
payload, err := json.Marshal(comments)
333338
if err != nil {
334339
return nil, fmt.Errorf("failed to marshal response: %w", err)
335340
}
336341

337-
return utils.NewToolResultText(string(r)), nil
342+
if len(payload) > mcpLargePayloadWarningThresholdBytes {
343+
warning := fmt.Sprintf(
344+
"WARNING: pull request review comments payload is %d bytes and may be truncated by some MCP clients. If you don't see all expected comments, retry with a smaller perPage (e.g., 1–5).",
345+
len(payload),
346+
)
347+
return &mcp.CallToolResult{
348+
Content: []mcp.Content{
349+
&mcp.TextContent{Text: warning},
350+
&mcp.TextContent{Text: string(payload)},
351+
},
352+
}, nil
353+
}
354+
355+
return utils.NewToolResultText(string(payload)), nil
338356
}
339357

340358
func GetPullRequestReviews(ctx context.Context, client *github.Client, cache *lockdown.RepoAccessCache, owner, repo string, pullNumber int, ff FeatureFlags) (*mcp.CallToolResult, error) {

pkg/github/pullrequests_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/json"
66
"net/http"
7+
"strings"
78
"testing"
89
"time"
910

@@ -12,6 +13,7 @@ import (
1213
"github.com/github/github-mcp-server/pkg/translations"
1314
"github.com/google/go-github/v79/github"
1415
"github.com/google/jsonschema-go/jsonschema"
16+
"github.com/modelcontextprotocol/go-sdk/mcp"
1517
"github.com/shurcooL/githubv4"
1618

1719
"github.com/migueleliasweb/go-github-mock/src/mock"
@@ -1758,6 +1760,56 @@ func Test_GetPullRequestComments(t *testing.T) {
17581760
}
17591761
}
17601762

1763+
func Test_GetPullRequestReviewComments_WarnsOnLargePayload(t *testing.T) {
1764+
largeText := strings.Repeat("x", mcpLargePayloadWarningThresholdBytes+1_000)
1765+
largeComments := []*github.PullRequestComment{
1766+
{
1767+
ID: github.Ptr(int64(999)),
1768+
Body: github.Ptr(largeText),
1769+
DiffHunk: github.Ptr(largeText),
1770+
User: &github.User{Login: github.Ptr("reviewer")},
1771+
},
1772+
}
1773+
1774+
mockedClient := mock.NewMockedHTTPClient(
1775+
mock.WithRequestMatch(
1776+
mock.GetReposPullsCommentsByOwnerByRepoByPullNumber,
1777+
largeComments,
1778+
),
1779+
)
1780+
1781+
client := github.NewClient(mockedClient)
1782+
cache := stubRepoAccessCache(githubv4.NewClient(nil), 5*time.Minute)
1783+
flags := stubFeatureFlags(map[string]bool{"lockdown-mode": false})
1784+
_, handler := PullRequestRead(stubGetClientFn(client), cache, translations.NullTranslationHelper, flags)
1785+
1786+
args := map[string]interface{}{
1787+
"method": "get_review_comments",
1788+
"owner": "owner",
1789+
"repo": "repo",
1790+
"pullNumber": float64(42),
1791+
}
1792+
request := createMCPRequest(args)
1793+
result, _, err := handler(context.Background(), &request, args)
1794+
1795+
require.NoError(t, err)
1796+
require.False(t, result.IsError)
1797+
require.Len(t, result.Content, 2)
1798+
1799+
warningContent, ok := result.Content[0].(*mcp.TextContent)
1800+
require.True(t, ok)
1801+
assert.Contains(t, warningContent.Text, "WARNING")
1802+
1803+
dataContent, ok := result.Content[1].(*mcp.TextContent)
1804+
require.True(t, ok)
1805+
1806+
var returnedComments []*github.PullRequestComment
1807+
err = json.Unmarshal([]byte(dataContent.Text), &returnedComments)
1808+
require.NoError(t, err)
1809+
require.Len(t, returnedComments, 1)
1810+
assert.Equal(t, largeComments[0].GetID(), returnedComments[0].GetID())
1811+
}
1812+
17611813
func Test_GetPullRequestReviews(t *testing.T) {
17621814
// Verify tool definition once
17631815
mockClient := github.NewClient(nil)

0 commit comments

Comments
 (0)