From 436e5c4fae96e0ca4d1759e2b66b17bd148be622 Mon Sep 17 00:00:00 2001 From: severus0044 <288351380+severus0044@users.noreply.github.com> Date: Fri, 6 Nov 2026 18:24:41 +0800 Subject: [PATCH 1/4] fix(get_file_contents): use serialized size for inline limit --- pkg/github/repositories.go | 73 +++++++++++++----------- pkg/github/repositories_helper.go | 45 +++++++++++++++ pkg/github/repositories_test.go | 92 ++++++++++++++++++++++++++++++- 3 files changed, 178 insertions(+), 32 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index b884f4888d..e0301c1712 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -26,6 +26,8 @@ import ( "github.com/shurcooL/githubv4" ) + + type GetCommitInput struct { Owner string `json:"owner"` Repo string `json:"repo"` @@ -1024,7 +1026,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool InputSchema: schema, }, scopes.PublicRead(scopes.Repo), - func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input GetFileContentsInput) (*mcp.CallToolResult, *RepositoryContentsOutput, error) { + func(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, input GetFileContentsInput) (*mcp.CallToolResult, *RepositoryContentsOutput, error) { owner := input.Owner if owner == "" { return utils.NewToolResultError("missing required parameter: owner"), nil, nil @@ -1101,7 +1103,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool if fallbackUsed { successNote = fmt.Sprintf(" Note: the provided ref '%s' does not exist, default branch '%s' was used instead.", originalRef, rawOpts.Ref) } - const maxContentSize = 1024 * 1024 // 1MB + const maxResultSize = 1024 * 1024 // 1MB read, respInspect, err := inspectRepositoryFile(ctx, client, owner, repo, ref, path, fileContent) if err != nil { @@ -1133,26 +1135,6 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) } - // For files >= 1MB, return a ResourceLink instead of content - if fileSize >= maxContentSize { - size := int64(fileSize) - resourceLink := &mcp.ResourceLink{ - URI: resourceURI, - Name: fileContent.GetName(), - Title: fmt.Sprintf("File: %s", path), - Size: &size, - } - message := fmt.Sprintf("File %s is too large to display (%d bytes). Use the download URL to fetch the content: %s (SHA: %s)%s", - path, fileSize, fileContent.GetDownloadURL(), fileSHA, successNote) - if read.Metadata != nil { - resourceLink.Title = fmt.Sprintf("Dereferenced target %s via symlink %s", read.Metadata.ResolvedTargetPath, path) - } - message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResourceLink( - message, - resourceLink))) - } - if !read.ContentAvailable { return utils.NewToolResultError("failed to inspect repository file: content unavailable"), nil, nil } @@ -1170,25 +1152,54 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool strings.HasSuffix(contentType, "+json") || strings.HasSuffix(contentType, "+xml") + resourceContents := &mcp.ResourceContents{} if isTextContent { result := &mcp.ResourceContents{ URI: resourceURI, Text: string(contentBytes), MIMEType: contentType, } - message := fmt.Sprintf("successfully downloaded text file (SHA: %s)%s", fileSHA, successNote) - message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) + resourceContents = result + } else { + result := &mcp.ResourceContents{ + URI: resourceURI, + Blob: contentBytes, + MIMEType: contentType, + } + resourceContents = result + } + inlineResult := utils.NewToolResultResource( + fmt.Sprintf("successfully downloaded %s file (SHA: %s)%s", map[bool]string{true: "text", false: "binary"}[isTextContent], fileSHA, successNote), + resourceContents) + if message := repositoryReadMessage(read, inlineResult.Content[0].(*mcp.TextContent).Text, successNote); message != inlineResult.Content[0].(*mcp.TextContent).Text { + inlineResult.Content[0].(*mcp.TextContent).Text = message + } + inlineTypedOutput, err := repositoryContentOutput(inlineResult) + if err != nil { + return nil, nil, err + } + wireSize, err := inlineToolResultWireSize(inlineResult, inlineTypedOutput, requestProtocolEra(ctx, req) == inventory.ProtocolEraModern) + if err != nil { + return nil, nil, err + } + if wireSize <= maxResultSize { + return repositoryContentResult(attachIFC(inlineResult)) } - result := &mcp.ResourceContents{ - URI: resourceURI, - Blob: contentBytes, - MIMEType: contentType, + size := int64(fileSize) + resourceLink := &mcp.ResourceLink{ + URI: resourceURI, + Name: fileContent.GetName(), + Title: fmt.Sprintf("File: %s", path), + Size: &size, + } + message := fmt.Sprintf("File %s is too large to display (%d bytes). Use the download URL to fetch the content: %s (SHA: %s)%s", + path, fileSize, fileContent.GetDownloadURL(), fileSHA, successNote) + if read.Metadata != nil { + resourceLink.Title = fmt.Sprintf("Dereferenced target %s via symlink %s", read.Metadata.ResolvedTargetPath, path) } - message := fmt.Sprintf("successfully downloaded binary file (SHA: %s)%s", fileSHA, successNote) message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) + return repositoryContentResult(attachIFC(utils.NewToolResultResourceLink(message, resourceLink))) } else if dirContent != nil { // file content or file SHA is nil which means it's a directory filtered := false diff --git a/pkg/github/repositories_helper.go b/pkg/github/repositories_helper.go index fe1983252c..29cef47312 100644 --- a/pkg/github/repositories_helper.go +++ b/pkg/github/repositories_helper.go @@ -13,12 +13,57 @@ import ( "unicode/utf8" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" + ghcontext "github.com/github/github-mcp-server/v2/pkg/context" + "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/raw" "github.com/github/github-mcp-server/v2/pkg/utils" "github.com/google/go-github/v92/github" + "github.com/modelcontextprotocol/go-sdk/jsonrpc" "github.com/modelcontextprotocol/go-sdk/mcp" ) +// requestProtocolEra mirrors inventory's request-era selection so a handler can +// account for SDK behavior that depends on the negotiated protocol. Metadata is +// authoritative; the HTTP-injected method info is the compatibility fallback. +func requestProtocolEra(ctx context.Context, req *mcp.CallToolRequest) inventory.ProtocolEra { + version := "" + if req != nil && req.Params != nil { + version, _ = req.Params.Meta[mcp.MetaKeyProtocolVersion].(string) + } + if version == "" { + if info, ok := ghcontext.MCPMethod(ctx); ok && info != nil { + version = info.ProtocolVersion + } + } + return inventory.ProtocolEraForVersion(version) +} + +// inlineToolResultWireSize measures the JSON size of the MCP result as closely +// as possible, including the structuredContent copy added for typed outputs on +// modern protocol versions and the JSON-RPC response envelope overhead. +func inlineToolResultWireSize(result *mcp.CallToolResult, typedOutput *RepositoryContentsOutput, modernProtocol bool) (int, error) { + if result == nil || result.IsError || result.InputRequests != nil { + return 0, fmt.Errorf("result is not a successful inline candidate") + } + if !modernProtocol { + typedOutput = nil + } + if modernProtocol && typedOutput == nil { + typedOutput = &RepositoryContentsOutput{} + } + response := struct { + JSONRPC string `json:"jsonrpc"` + ID jsonrpc.ID `json:"id"` + Result *mcp.CallToolResult `json:"result"` + StructuredContent *RepositoryContentsOutput `json:"structuredContent,omitempty"` + }{JSONRPC: "2.0", Result: result, StructuredContent: typedOutput} + encoded, err := json.Marshal(response) + if err != nil { + return 0, err + } + return len(encoded), nil +} + // initializeRepository creates an initial commit in an empty repository and returns the default branch ref and base commit func initializeRepository(ctx context.Context, client *github.Client, owner, repo string) (ref *github.Reference, baseCommit *github.Commit, err error) { // First, we need to check what the default branch in this empty repo should be: diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 9b40c932ff..c0c3658707 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2,6 +2,7 @@ package github import ( "context" + "bytes" "encoding/base64" "encoding/json" "net/http" @@ -13,6 +14,7 @@ import ( "github.com/github/github-mcp-server/v2/internal/githubv4mock" "github.com/github/github-mcp-server/v2/internal/requeststate" "github.com/github/github-mcp-server/v2/internal/toolsnaps" + ghcontext "github.com/github/github-mcp-server/v2/pkg/context" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/raw" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -40,6 +42,8 @@ type repositoryReadFixture struct { blobs map[string][]byte trees map[string]*github.Tree inspect func(*http.Request) + ctx context.Context + metadataProtocol string } func runRepositoryReadFixture(t *testing.T, fixture repositoryReadFixture, args map[string]any) (*mcp.CallToolResult, int) { @@ -73,7 +77,14 @@ func runRepositoryReadFixture(t *testing.T, fixture repositoryReadFixture, args tool := GetFileContents(translations.NullTranslationHelper) handler := tool.Handler(deps) request := createMCPRequest(args) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) + if fixture.metadataProtocol != "" { + request.Params.Meta = mcp.Meta{mcp.MetaKeyProtocolVersion: fixture.metadataProtocol} + } + ctx := fixture.ctx + if ctx == nil { + ctx = context.Background() + } + result, err := handler(ContextWithDeps(ctx, deps), &request) require.NoError(t, err) return result, counter.count } @@ -772,6 +783,85 @@ func Test_GetFileContents_SymlinkDisclosure(t *testing.T) { }) } +func Test_GetFileContents_ModernProtocolInlineLimit(t *testing.T) { + commitSHA := strings.Repeat("c", 40) + content := strings.Repeat("a", 600*1024) + encoded := base64.StdEncoding.EncodeToString([]byte(content)) + fileSHA := gitBlobSHA([]byte(content)) + args := map[string]any{"owner": "owner", "repo": "repo", "path": "medium.txt", "sha": commitSHA} + + fixture := repositoryReadFixture{ + contents: &github.RepositoryContent{ + Type: new("file"), Path: new("medium.txt"), SHA: new(fileSHA), Size: new(len(content)), + Content: new(encoded), Encoding: new("base64"), + }, + } + + t.Run("legacy protocol inlines the file", func(t *testing.T) { + result, _ := runRepositoryReadFixture(t, fixture, args) + require.False(t, result.IsError) + _, ok := result.Content[1].(*mcp.EmbeddedResource) + require.True(t, ok, "expected an inline resource on the legacy protocol") + }) + + t.Run("modern protocol returns a ResourceLink", func(t *testing.T) { + modern := fixture + modern.ctx = ghcontext.WithMCPMethodInfo(context.Background(), &ghcontext.MCPMethodInfo{ + ProtocolVersion: inventory.ProtocolVersionMultiRoundTrip, + }) + result, _ := runRepositoryReadFixture(t, modern, args) + require.False(t, result.IsError) + _, ok := result.Content[1].(*mcp.ResourceLink) + require.True(t, ok, "expected a ResourceLink on the modern protocol") + }) + + t.Run("modern request metadata returns a ResourceLink", func(t *testing.T) { + modern := fixture + modern.metadataProtocol = inventory.ProtocolVersionMultiRoundTrip + result, _ := runRepositoryReadFixture(t, modern, args) + require.False(t, result.IsError) + _, ok := result.Content[1].(*mcp.ResourceLink) + require.True(t, ok, "expected a ResourceLink when the modern protocol is metadata-only") + }) + + t.Run("modern protocol serializes oversized binary as a link", func(t *testing.T) { + binary := bytes.Repeat([]byte{0xff, 0xd8, 0xff, 0xe0}, 100*1024) + encoded := base64.StdEncoding.EncodeToString(binary) + fileSHA := gitBlobSHA(binary) + binaryFixture := repositoryReadFixture{ + metadataProtocol: inventory.ProtocolVersionMultiRoundTrip, + contents: &github.RepositoryContent{ + Type: new("file"), Path: new("small.bin"), SHA: new(fileSHA), Size: new(len(binary)), + Content: new(encoded), Encoding: new("base64"), + }, + } + result, _ := runRepositoryReadFixture(t, binaryFixture, map[string]any{ + "owner": "owner", "repo": "repo", "path": "small.bin", "sha": commitSHA, + }) + require.False(t, result.IsError) + _, ok := result.Content[1].(*mcp.ResourceLink) + require.True(t, ok, "expected a ResourceLink after measuring serialized output") + }) + + t.Run("legacy protocol keeps under-limit binary inline", func(t *testing.T) { + binary := bytes.Repeat([]byte{0xff, 0xd8, 0xff, 0xe0}, 100*1024) + encoded := base64.StdEncoding.EncodeToString(binary) + fileSHA := gitBlobSHA(binary) + binaryFixture := repositoryReadFixture{ + contents: &github.RepositoryContent{ + Type: new("file"), Path: new("binary.bin"), SHA: new(fileSHA), Size: new(len(binary)), + Content: new(encoded), Encoding: new("base64"), + }, + } + result, _ := runRepositoryReadFixture(t, binaryFixture, map[string]any{ + "owner": "owner", "repo": "repo", "path": "binary.bin", "sha": commitSHA, + }) + require.False(t, result.IsError) + _, ok := result.Content[1].(*mcp.EmbeddedResource) + require.True(t, ok, "expected inline content on legacy protocol") + }) +} + func Test_GetFileContents_ContentlessRequestCounts(t *testing.T) { commitSHA := strings.Repeat("c", 40) fileSHA := strings.Repeat("a", 40) From 3318fa32e4708c80c29f835fc2536bf1fb768a02 Mon Sep 17 00:00:00 2001 From: Severus <1529378848@qq.com> Date: Sun, 8 Nov 2026 21:52:56 +0800 Subject: [PATCH 2/4] style: drop stray blank lines in repositories.go imports --- pkg/github/repositories.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index e0301c1712..6f376174bf 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -26,8 +26,6 @@ import ( "github.com/shurcooL/githubv4" ) - - type GetCommitInput struct { Owner string `json:"owner"` Repo string `json:"repo"` From 719c11d756e9b0a5f39df9d5d10d1a3fb4edf97f Mon Sep 17 00:00:00 2001 From: Severus <1529378848@qq.com> Date: Mon, 9 Nov 2026 15:13:43 +0800 Subject: [PATCH 3/4] style: gofmt repositories_helper.go (#3709) --- pkg/github/repositories_helper.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/github/repositories_helper.go b/pkg/github/repositories_helper.go index 29cef47312..b948aea3d9 100644 --- a/pkg/github/repositories_helper.go +++ b/pkg/github/repositories_helper.go @@ -12,8 +12,8 @@ import ( "strings" "unicode/utf8" - ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" ghcontext "github.com/github/github-mcp-server/v2/pkg/context" + ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/raw" "github.com/github/github-mcp-server/v2/pkg/utils" @@ -52,9 +52,9 @@ func inlineToolResultWireSize(result *mcp.CallToolResult, typedOutput *Repositor typedOutput = &RepositoryContentsOutput{} } response := struct { - JSONRPC string `json:"jsonrpc"` - ID jsonrpc.ID `json:"id"` - Result *mcp.CallToolResult `json:"result"` + JSONRPC string `json:"jsonrpc"` + ID jsonrpc.ID `json:"id"` + Result *mcp.CallToolResult `json:"result"` StructuredContent *RepositoryContentsOutput `json:"structuredContent,omitempty"` }{JSONRPC: "2.0", Result: result, StructuredContent: typedOutput} encoded, err := json.Marshal(response) From 523091a10964dccc4d2110893d5d56c70dde6bcd Mon Sep 17 00:00:00 2001 From: Severus <1529378848@qq.com> Date: Mon, 9 Nov 2026 15:14:09 +0800 Subject: [PATCH 4/4] style: gofmt repositories_test.go (#3709) --- pkg/github/repositories_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index c0c3658707..34d428b1a1 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -1,8 +1,8 @@ package github import ( - "context" "bytes" + "context" "encoding/base64" "encoding/json" "net/http" @@ -38,11 +38,11 @@ func (t *repositoryRequestCountingTransport) RoundTrip(req *http.Request) (*http } type repositoryReadFixture struct { - contents any - blobs map[string][]byte - trees map[string]*github.Tree - inspect func(*http.Request) - ctx context.Context + contents any + blobs map[string][]byte + trees map[string]*github.Tree + inspect func(*http.Request) + ctx context.Context metadataProtocol string } @@ -598,8 +598,8 @@ func Test_GetFileContents_UsesUTF8ResourceURI(t *testing.T) { sha := strings.Repeat("a", 40) result, _ := runRepositoryReadFixture(t, repositoryReadFixture{ contents: &github.RepositoryContent{ - Name: new("Þfoo.go"), - Path: new("test/Þfoo.go"), + Name: new("脼foo.go"), + Path: new("test/脼foo.go"), SHA: new(gitBlobSHA(content)), Type: new("file"), Content: new(string(content)), @@ -608,7 +608,7 @@ func Test_GetFileContents_UsesUTF8ResourceURI(t *testing.T) { }, map[string]any{ "owner": "owner", "repo": "repo", - "path": "test/Þfoo.go", + "path": "test/脼foo.go", "sha": sha, }) @@ -620,8 +620,8 @@ func Test_GetFileContents_LargeFileUsesUTF8ResourceURI(t *testing.T) { sha := strings.Repeat("a", 40) result, _ := runRepositoryReadFixture(t, repositoryReadFixture{ contents: &github.RepositoryContent{ - Name: new("Þlarge.bin"), - Path: new("test/Þlarge.bin"), + Name: new("脼large.bin"), + Path: new("test/脼large.bin"), SHA: new(sha), Type: new("file"), Size: new(2 * 1024 * 1024), @@ -635,7 +635,7 @@ func Test_GetFileContents_LargeFileUsesUTF8ResourceURI(t *testing.T) { SHA: new("b"), }}}, "b": {Entries: []*github.TreeEntry{{ - Path: new("Þlarge.bin"), + Path: new("脼large.bin"), Mode: new("100644"), Type: new("blob"), SHA: new(sha), @@ -644,7 +644,7 @@ func Test_GetFileContents_LargeFileUsesUTF8ResourceURI(t *testing.T) { }, map[string]any{ "owner": "owner", "repo": "repo", - "path": "test/Þlarge.bin", + "path": "test/脼large.bin", "sha": sha, })