Skip to content

Commit d5cd69a

Browse files
yonaka15yonaka15
committed
fix: Always include SHA in get_file_contents responses (github#605)
Switch from GitHub Raw Content API to Contents API for file retrieval. This ensures SHA information is always included in the response, providing consistency across file and directory operations. Changes: - Replace rawClient.GetRawContent() with client.Repositories.GetContents() - Return JSON marshaled RepositoryContent instead of resource contents - Update tests to verify SHA presence in responses - Remove unused imports (base64, net/url) Closes github#605
1 parent d15026b commit d5cd69a

File tree

2 files changed

+74
-90
lines changed

2 files changed

+74
-90
lines changed

‎pkg/github/repositories.go

Lines changed: 8 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ package github
22

33
import (
44
"context"
5-
"encoding/base64"
65
"encoding/json"
76
"fmt"
87
"io"
98
"net/http"
10-
"net/url"
119
"strings"
1210

1311
ghErrors "github.com/github/github-mcp-server/pkg/errors"
@@ -505,62 +503,17 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
505503
}
506504

507505
// If the path is (most likely) not to be a directory, we will
508-
// first try to get the raw content from the GitHub raw content API.
506+
// try to get the content using the GitHub contents API to include SHA information.
509507
if path != "" && !strings.HasSuffix(path, "/") {
510-
511-
rawClient, err := getRawClient(ctx)
512-
if err != nil {
513-
return mcp.NewToolResultError("failed to get GitHub raw content client"), nil
514-
}
515-
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
516-
if err != nil {
517-
return mcp.NewToolResultError("failed to get raw repository content"), nil
518-
}
519-
defer func() {
520-
_ = resp.Body.Close()
521-
}()
522-
523-
if resp.StatusCode == http.StatusOK {
524-
// If the raw content is found, return it directly
525-
body, err := io.ReadAll(resp.Body)
508+
opts := &github.RepositoryContentGetOptions{Ref: ref}
509+
fileContent, _, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
510+
if err == nil && resp.StatusCode == http.StatusOK && fileContent != nil {
511+
defer func() { _ = resp.Body.Close() }()
512+
r, err := json.Marshal(fileContent)
526513
if err != nil {
527-
return mcp.NewToolResultError("failed to read response body"), nil
528-
}
529-
contentType := resp.Header.Get("Content-Type")
530-
531-
var resourceURI string
532-
switch {
533-
case sha != "":
534-
resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path)
535-
if err != nil {
536-
return nil, fmt.Errorf("failed to create resource URI: %w", err)
537-
}
538-
case ref != "":
539-
resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path)
540-
if err != nil {
541-
return nil, fmt.Errorf("failed to create resource URI: %w", err)
542-
}
543-
default:
544-
resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path)
545-
if err != nil {
546-
return nil, fmt.Errorf("failed to create resource URI: %w", err)
547-
}
548-
}
549-
550-
if strings.HasPrefix(contentType, "application") || strings.HasPrefix(contentType, "text") {
551-
return mcp.NewToolResultResource("successfully downloaded text file", mcp.TextResourceContents{
552-
URI: resourceURI,
553-
Text: string(body),
554-
MIMEType: contentType,
555-
}), nil
514+
return mcp.NewToolResultError("failed to marshal response"), nil
556515
}
557-
558-
return mcp.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{
559-
URI: resourceURI,
560-
Blob: base64.StdEncoding.EncodeToString(body),
561-
MIMEType: contentType,
562-
}), nil
563-
516+
return mcp.NewToolResultText(string(r)), nil
564517
}
565518
}
566519

‎pkg/github/repositories_test.go

Lines changed: 66 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ func Test_GetFileContents(t *testing.T) {
3535
assert.Contains(t, tool.InputSchema.Properties, "sha")
3636
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})
3737

38-
// Mock response for raw content
39-
mockRawContent := []byte("# Test Repository\n\nThis is a test repository.")
38+
4039

4140
// Setup mock directory content for success case
4241
mockDirContent := []*github.RepositoryContent{
@@ -67,20 +66,31 @@ func Test_GetFileContents(t *testing.T) {
6766
expectStatus int
6867
}{
6968
{
70-
name: "successful text content fetch",
69+
name: "successful file content fetch",
7170
mockedClient: mock.NewMockedHTTPClient(
7271
mock.WithRequestMatchHandler(
7372
mock.GetReposGitRefByOwnerByRepoByRef,
7473
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
7574
w.WriteHeader(http.StatusOK)
76-
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`))
75+
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "abc123"}}`))
7776
}),
7877
),
7978
mock.WithRequestMatchHandler(
80-
raw.GetRawReposContentsByOwnerByRepoByBranchByPath,
79+
mock.GetReposContentsByOwnerByRepoByPath,
8180
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
82-
w.Header().Set("Content-Type", "text/markdown")
83-
_, _ = w.Write(mockRawContent)
81+
w.WriteHeader(http.StatusOK)
82+
fileContent := &github.RepositoryContent{
83+
Type: github.Ptr("file"),
84+
Name: github.Ptr("README.md"),
85+
Path: github.Ptr("README.md"),
86+
SHA: github.Ptr("abc123"),
87+
Size: github.Ptr(42),
88+
Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("# Test Repository\n\nThis is a test repository."))),
89+
Encoding: github.Ptr("base64"),
90+
HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/README.md"),
91+
}
92+
responseBody, _ := json.Marshal(fileContent)
93+
_, _ = w.Write(responseBody)
8494
}),
8595
),
8696
),
@@ -91,27 +101,43 @@ func Test_GetFileContents(t *testing.T) {
91101
"ref": "refs/heads/main",
92102
},
93103
expectError: false,
94-
expectedResult: mcp.TextResourceContents{
95-
URI: "repo://owner/repo/refs/heads/main/contents/README.md",
96-
Text: "# Test Repository\n\nThis is a test repository.",
97-
MIMEType: "text/markdown",
104+
expectedResult: &github.RepositoryContent{
105+
Type: github.Ptr("file"),
106+
Name: github.Ptr("README.md"),
107+
Path: github.Ptr("README.md"),
108+
SHA: github.Ptr("abc123"),
109+
Size: github.Ptr(42),
110+
Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("# Test Repository\n\nThis is a test repository."))),
111+
Encoding: github.Ptr("base64"),
112+
HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/README.md"),
98113
},
99114
},
100115
{
101-
name: "successful file blob content fetch",
116+
name: "successful binary file content fetch",
102117
mockedClient: mock.NewMockedHTTPClient(
103118
mock.WithRequestMatchHandler(
104119
mock.GetReposGitRefByOwnerByRepoByRef,
105120
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
106121
w.WriteHeader(http.StatusOK)
107-
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`))
122+
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "def456"}}`))
108123
}),
109124
),
110125
mock.WithRequestMatchHandler(
111-
raw.GetRawReposContentsByOwnerByRepoByBranchByPath,
126+
mock.GetReposContentsByOwnerByRepoByPath,
112127
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
113-
w.Header().Set("Content-Type", "image/png")
114-
_, _ = w.Write(mockRawContent)
128+
w.WriteHeader(http.StatusOK)
129+
fileContent := &github.RepositoryContent{
130+
Type: github.Ptr("file"),
131+
Name: github.Ptr("test.png"),
132+
Path: github.Ptr("test.png"),
133+
SHA: github.Ptr("def456"),
134+
Size: github.Ptr(1024),
135+
Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("binary-image-content"))),
136+
Encoding: github.Ptr("base64"),
137+
HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/test.png"),
138+
}
139+
responseBody, _ := json.Marshal(fileContent)
140+
_, _ = w.Write(responseBody)
115141
}),
116142
),
117143
),
@@ -122,10 +148,15 @@ func Test_GetFileContents(t *testing.T) {
122148
"ref": "refs/heads/main",
123149
},
124150
expectError: false,
125-
expectedResult: mcp.BlobResourceContents{
126-
URI: "repo://owner/repo/refs/heads/main/contents/test.png",
127-
Blob: base64.StdEncoding.EncodeToString(mockRawContent),
128-
MIMEType: "image/png",
151+
expectedResult: &github.RepositoryContent{
152+
Type: github.Ptr("file"),
153+
Name: github.Ptr("test.png"),
154+
Path: github.Ptr("test.png"),
155+
SHA: github.Ptr("def456"),
156+
Size: github.Ptr(1024),
157+
Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("binary-image-content"))),
158+
Encoding: github.Ptr("base64"),
159+
HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/test.png"),
129160
},
130161
},
131162
{
@@ -151,14 +182,7 @@ func Test_GetFileContents(t *testing.T) {
151182
mockResponse(t, http.StatusOK, mockDirContent),
152183
),
153184
),
154-
mock.WithRequestMatchHandler(
155-
raw.GetRawReposContentsByOwnerByRepoByPath,
156-
expectQueryParams(t, map[string]string{
157-
"branch": "main",
158-
}).andThen(
159-
mockResponse(t, http.StatusNotFound, nil),
160-
),
161-
),
185+
162186
),
163187
requestArgs: map[string]interface{}{
164188
"owner": "owner",
@@ -186,7 +210,7 @@ func Test_GetFileContents(t *testing.T) {
186210
}),
187211
),
188212
mock.WithRequestMatchHandler(
189-
raw.GetRawReposContentsByOwnerByRepoByPath,
213+
mock.GetReposGitTreesByOwnerByRepoByTreeSha,
190214
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
191215
w.WriteHeader(http.StatusNotFound)
192216
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
@@ -227,12 +251,19 @@ func Test_GetFileContents(t *testing.T) {
227251
require.NoError(t, err)
228252
// Use the correct result helper based on the expected type
229253
switch expected := tc.expectedResult.(type) {
230-
case mcp.TextResourceContents:
231-
textResource := getTextResourceResult(t, result)
232-
assert.Equal(t, expected, textResource)
233-
case mcp.BlobResourceContents:
234-
blobResource := getBlobResourceResult(t, result)
235-
assert.Equal(t, expected, blobResource)
254+
case *github.RepositoryContent:
255+
// File content fetch returns a text result (JSON object)
256+
textContent := getTextResult(t, result)
257+
var returnedContent github.RepositoryContent
258+
err = json.Unmarshal([]byte(textContent.Text), &returnedContent)
259+
require.NoError(t, err, "Failed to unmarshal file content result: %v", textContent.Text)
260+
assert.Equal(t, *expected.Name, *returnedContent.Name)
261+
assert.Equal(t, *expected.Path, *returnedContent.Path)
262+
assert.Equal(t, *expected.Type, *returnedContent.Type)
263+
assert.Equal(t, *expected.SHA, *returnedContent.SHA)
264+
if expected.Content != nil && returnedContent.Content != nil {
265+
assert.Equal(t, *expected.Content, *returnedContent.Content)
266+
}
236267
case []*github.RepositoryContent:
237268
// Directory content fetch returns a text result (JSON array)
238269
textContent := getTextResult(t, result)

0 commit comments

Comments
 (0)