Skip to content

Commit 922c457

Browse files
committed
fix: include file body in get_file_contents text content
For text files the tool returned the body only inside the EmbeddedResource while TextContent carried just the SHA line, so client surfaces that drop embedded resources (e.g. Claude voice mode) left the model with no file content. Carry the body in TextContent as well; resource, order and schema unchanged. Binary, large-file, empty-file and symlink-metadata paths untouched. Fixes #3189
1 parent 9205304 commit 922c457

2 files changed

Lines changed: 85 additions & 1 deletion

File tree

‎pkg/github/repositories.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,14 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool
11571157
Text: string(contentBytes),
11581158
MIMEType: contentType,
11591159
}
1160-
message := fmt.Sprintf("successfully downloaded text file (SHA: %s)%s", fileSHA, successNote)
1160+
// Carry the body in the text content as well as the embedded
1161+
// resource. Client surfaces that drop embedded resources
1162+
// (e.g. Claude voice mode, issue #3189) would otherwise show
1163+
// only the SHA line while the model never sees the file. When
1164+
// read.Metadata != nil, repositoryReadMessage replaces the
1165+
// fallback with JSON metadata, which must stay parseable, so
1166+
// only plain files gain the inline body.
1167+
message := fmt.Sprintf("successfully downloaded text file (SHA: %s)%s\n\n%s", fileSHA, successNote, string(contentBytes))
11611168
message = repositoryReadMessage(read, message, successNote)
11621169
return attachIFC(utils.NewToolResultResource(message, result)), nil, nil
11631170
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"net/http"
7+
"net/url"
8+
"testing"
9+
10+
"github.com/github/github-mcp-server/pkg/raw"
11+
"github.com/github/github-mcp-server/pkg/translations"
12+
"github.com/google/go-github/v89/github"
13+
"github.com/modelcontextprotocol/go-sdk/mcp"
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
16+
)
17+
18+
// Regression test for https://github.com/github/github-mcp-server/issues/3189.
19+
//
20+
// get_file_contents returns text-file bodies ONLY inside an EmbeddedResource.
21+
// Client surfaces that drop embedded resources (e.g. Claude voice mode) then
22+
// show just "[non-text content: resource]" plus the short "successfully
23+
// downloaded text file (SHA: ...)" TextContent, so the model never sees the
24+
// file. The TextContent must also carry the file body.
25+
func Test_GetFileContents_TextContentIncludesFileBody(t *testing.T) {
26+
t.Parallel()
27+
28+
fileBody := "# Test Repository\n\nThis is a test repository."
29+
serverTool := GetFileContents(translations.NullTranslationHelper)
30+
31+
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
32+
GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, "{\"ref\": \"refs/heads/main\", \"object\": {\"sha\": \"\"}}"),
33+
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
34+
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
35+
w.WriteHeader(http.StatusOK)
36+
fileContent := &github.RepositoryContent{
37+
Name: github.Ptr("README.md"),
38+
Path: github.Ptr("README.md"),
39+
SHA: github.Ptr(gitBlobSHA([]byte(fileBody))),
40+
Type: github.Ptr("file"),
41+
Content: github.Ptr(fileBody),
42+
Size: github.Ptr(len(fileBody)),
43+
}
44+
contentBytes, _ := json.Marshal(fileContent)
45+
_, _ = w.Write(contentBytes)
46+
},
47+
})
48+
49+
client := mustNewGHClient(t, mockedClient)
50+
mockRawClient, err := raw.NewClient(client, &url.URL{Scheme: "https", Host: "raw.example.com", Path: "/"})
51+
require.NoError(t, err)
52+
deps := BaseDeps{Client: client, RawClient: mockRawClient}
53+
handler := serverTool.Handler(deps)
54+
request := createMCPRequest(map[string]any{
55+
"owner": "owner",
56+
"repo": "repo",
57+
"path": "README.md",
58+
"ref": "refs/heads/main",
59+
})
60+
61+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
62+
require.NoError(t, err)
63+
require.False(t, result.IsError)
64+
require.Len(t, result.Content, 2)
65+
66+
// The embedded resource keeps carrying the full body (existing contract).
67+
resource, ok := result.Content[1].(*mcp.EmbeddedResource)
68+
require.True(t, ok, "expected Content[1] to be EmbeddedResource")
69+
require.NotNil(t, resource.Resource)
70+
assert.Equal(t, fileBody, resource.Resource.Text)
71+
72+
// The text content must ALSO carry the body so resource-dropping clients
73+
// still expose the file to the model (issue #3189).
74+
text, ok := result.Content[0].(*mcp.TextContent)
75+
require.True(t, ok, "expected Content[0] to be TextContent")
76+
assert.Contains(t, text.Text, fileBody)
77+
}

0 commit comments

Comments
 (0)