Skip to content

Commit 38a7103

Browse files
almaleksiaalmaleksia
committed
Use Contents API instead of raw endpoint to fetch file content
1 parent bbc675a commit 38a7103

File tree

3 files changed

+151
-125
lines changed

3 files changed

+151
-125
lines changed

‎pkg/github/repositories.go‎

Lines changed: 46 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -715,86 +715,62 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool
715715

716716
if fileContent != nil && fileContent.SHA != nil {
717717
fileSHA = *fileContent.SHA
718+
fileSize := fileContent.GetSize()
718719

719-
rawClient, err := deps.GetRawClient(ctx)
720-
if err != nil {
721-
return utils.NewToolResultError("failed to get GitHub raw content client"), nil, nil
722-
}
723-
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
724-
if err != nil {
725-
return utils.NewToolResultError("failed to get raw repository content"), nil, nil
726-
}
727-
defer func() {
728-
_ = resp.Body.Close()
729-
}()
730-
731-
if resp.StatusCode == http.StatusOK {
732-
// If the raw content is found, return it directly
733-
body, err := io.ReadAll(resp.Body)
720+
// Build resource URI for the file
721+
var resourceURI string
722+
switch {
723+
case sha != "":
724+
resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path)
734725
if err != nil {
735-
return ghErrors.NewGitHubRawAPIErrorResponse(ctx, "failed to get raw repository content", resp, err), nil, nil
726+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
736727
}
737-
contentType := resp.Header.Get("Content-Type")
738-
739-
var resourceURI string
740-
switch {
741-
case sha != "":
742-
resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path)
743-
if err != nil {
744-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
745-
}
746-
case ref != "":
747-
resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path)
748-
if err != nil {
749-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
750-
}
751-
default:
752-
resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path)
753-
if err != nil {
754-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
755-
}
728+
case ref != "":
729+
resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path)
730+
if err != nil {
731+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
756732
}
757-
758-
// main branch ref passed in ref parameter but it doesn't exist - default branch was used
759-
var successNote string
760-
if fallbackUsed {
761-
successNote = fmt.Sprintf(" Note: the provided ref '%s' does not exist, default branch '%s' was used instead.", originalRef, rawOpts.Ref)
733+
default:
734+
resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path)
735+
if err != nil {
736+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
762737
}
738+
}
763739

764-
// Determine if content is text or binary
765-
isTextContent := strings.HasPrefix(contentType, "text/") ||
766-
contentType == "application/json" ||
767-
contentType == "application/xml" ||
768-
strings.HasSuffix(contentType, "+json") ||
769-
strings.HasSuffix(contentType, "+xml")
770-
771-
if isTextContent {
772-
result := &mcp.ResourceContents{
773-
URI: resourceURI,
774-
Text: string(body),
775-
MIMEType: contentType,
776-
}
777-
// Include SHA in the result metadata
778-
if fileSHA != "" {
779-
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA)+successNote, result), nil, nil
780-
}
781-
return utils.NewToolResultResource("successfully downloaded text file"+successNote, result), nil, nil
782-
}
740+
// main branch ref passed in ref parameter but it doesn't exist - default branch was used
741+
var successNote string
742+
if fallbackUsed {
743+
successNote = fmt.Sprintf(" Note: the provided ref '%s' does not exist, default branch '%s' was used instead.", originalRef, rawOpts.Ref)
744+
}
783745

784-
result := &mcp.ResourceContents{
785-
URI: resourceURI,
786-
Blob: body,
787-
MIMEType: contentType,
746+
// For files >= 1MB, return a ResourceLink instead of content
747+
const maxContentSize = 1024 * 1024 // 1MB
748+
if fileSize >= maxContentSize {
749+
size := int64(fileSize)
750+
resourceLink := &mcp.ResourceLink{
751+
URI: resourceURI,
752+
Name: fileContent.GetName(),
753+
Title: fmt.Sprintf("File: %s", path),
754+
Size: &size,
788755
}
789-
// Include SHA in the result metadata
790-
if fileSHA != "" {
791-
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA)+successNote, result), nil, nil
792-
}
793-
return utils.NewToolResultResource("successfully downloaded binary file"+successNote, result), nil, nil
756+
return utils.NewToolResultResourceLink(
757+
fmt.Sprintf("File %s is too large to display (%d bytes). Use the download URL to fetch the content: %s (SHA: %s)%s",
758+
path, fileSize, fileContent.GetDownloadURL(), fileSHA, successNote),
759+
resourceLink), nil, nil
794760
}
795761

796-
// Raw API call failed
797-
return matchFiles(ctx, client, owner, repo, ref, path, rawOpts, resp.StatusCode)
762+
// For files < 1MB, get content directly from Contents API
763+
content, err := fileContent.GetContent()
764+
if err != nil {
765+
return utils.NewToolResultError(fmt.Sprintf("failed to decode file content: %s", err)), nil, nil
766+
}
767+
768+
result := &mcp.ResourceContents{
769+
URI: resourceURI,
770+
Text: content,
771+
MIMEType: "text/plain",
772+
}
773+
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)%s", fileSHA, successNote), result), nil, nil
798774
} else if dirContent != nil {
799775
// file content or file SHA is nil which means it's a directory
800776
r, err := json.Marshal(dirContent)

‎pkg/github/repositories_test.go‎

Lines changed: 93 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -78,19 +78,20 @@ func Test_GetFileContents(t *testing.T) {
7878
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
7979
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
8080
w.WriteHeader(http.StatusOK)
81+
// Base64 encode the content as GitHub API does
82+
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
8183
fileContent := &github.RepositoryContent{
82-
Name: github.Ptr("README.md"),
83-
Path: github.Ptr("README.md"),
84-
SHA: github.Ptr("abc123"),
85-
Type: github.Ptr("file"),
84+
Name: github.Ptr("README.md"),
85+
Path: github.Ptr("README.md"),
86+
SHA: github.Ptr("abc123"),
87+
Type: github.Ptr("file"),
88+
Content: github.Ptr(encodedContent),
89+
Size: github.Ptr(len(mockRawContent)),
90+
Encoding: github.Ptr("base64"),
8691
}
8792
contentBytes, _ := json.Marshal(fileContent)
8893
_, _ = w.Write(contentBytes)
8994
},
90-
GetRawReposContentsByOwnerByRepoByBranchByPath: func(w http.ResponseWriter, _ *http.Request) {
91-
w.Header().Set("Content-Type", "text/markdown")
92-
_, _ = w.Write(mockRawContent)
93-
},
9495
}),
9596
requestArgs: map[string]interface{}{
9697
"owner": "owner",
@@ -102,7 +103,7 @@ func Test_GetFileContents(t *testing.T) {
102103
expectedResult: mcp.ResourceContents{
103104
URI: "repo://owner/repo/refs/heads/main/contents/README.md",
104105
Text: "# Test Repository\n\nThis is a test repository.",
105-
MIMEType: "text/markdown",
106+
MIMEType: "text/plain",
106107
},
107108
},
108109
{
@@ -112,19 +113,20 @@ func Test_GetFileContents(t *testing.T) {
112113
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
113114
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
114115
w.WriteHeader(http.StatusOK)
116+
// Base64 encode the content as GitHub API does
117+
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
115118
fileContent := &github.RepositoryContent{
116-
Name: github.Ptr("test.png"),
117-
Path: github.Ptr("test.png"),
118-
SHA: github.Ptr("def456"),
119-
Type: github.Ptr("file"),
119+
Name: github.Ptr("test.png"),
120+
Path: github.Ptr("test.png"),
121+
SHA: github.Ptr("def456"),
122+
Type: github.Ptr("file"),
123+
Content: github.Ptr(encodedContent),
124+
Size: github.Ptr(len(mockRawContent)),
125+
Encoding: github.Ptr("base64"),
120126
}
121127
contentBytes, _ := json.Marshal(fileContent)
122128
_, _ = w.Write(contentBytes)
123129
},
124-
GetRawReposContentsByOwnerByRepoByBranchByPath: func(w http.ResponseWriter, _ *http.Request) {
125-
w.Header().Set("Content-Type", "image/png")
126-
_, _ = w.Write(mockRawContent)
127-
},
128130
}),
129131
requestArgs: map[string]interface{}{
130132
"owner": "owner",
@@ -135,8 +137,8 @@ func Test_GetFileContents(t *testing.T) {
135137
expectError: false,
136138
expectedResult: mcp.ResourceContents{
137139
URI: "repo://owner/repo/refs/heads/main/contents/test.png",
138-
Blob: mockRawContent,
139-
MIMEType: "image/png",
140+
Text: "# Test Repository\n\nThis is a test repository.",
141+
MIMEType: "text/plain",
140142
},
141143
},
142144
{
@@ -146,19 +148,20 @@ func Test_GetFileContents(t *testing.T) {
146148
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
147149
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
148150
w.WriteHeader(http.StatusOK)
151+
// Base64 encode the content as GitHub API does
152+
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
149153
fileContent := &github.RepositoryContent{
150-
Name: github.Ptr("document.pdf"),
151-
Path: github.Ptr("document.pdf"),
152-
SHA: github.Ptr("pdf123"),
153-
Type: github.Ptr("file"),
154+
Name: github.Ptr("document.pdf"),
155+
Path: github.Ptr("document.pdf"),
156+
SHA: github.Ptr("pdf123"),
157+
Type: github.Ptr("file"),
158+
Content: github.Ptr(encodedContent),
159+
Size: github.Ptr(len(mockRawContent)),
160+
Encoding: github.Ptr("base64"),
154161
}
155162
contentBytes, _ := json.Marshal(fileContent)
156163
_, _ = w.Write(contentBytes)
157164
},
158-
GetRawReposContentsByOwnerByRepoByBranchByPath: func(w http.ResponseWriter, _ *http.Request) {
159-
w.Header().Set("Content-Type", "application/pdf")
160-
_, _ = w.Write(mockRawContent)
161-
},
162165
}),
163166
requestArgs: map[string]interface{}{
164167
"owner": "owner",
@@ -169,8 +172,8 @@ func Test_GetFileContents(t *testing.T) {
169172
expectError: false,
170173
expectedResult: mcp.ResourceContents{
171174
URI: "repo://owner/repo/refs/heads/main/contents/document.pdf",
172-
Blob: mockRawContent,
173-
MIMEType: "application/pdf",
175+
Text: "# Test Repository\n\nThis is a test repository.",
176+
MIMEType: "text/plain",
174177
},
175178
},
176179
{
@@ -200,19 +203,20 @@ func Test_GetFileContents(t *testing.T) {
200203
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
201204
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
202205
w.WriteHeader(http.StatusOK)
206+
// Base64 encode the content as GitHub API does
207+
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
203208
fileContent := &github.RepositoryContent{
204-
Name: github.Ptr("README.md"),
205-
Path: github.Ptr("README.md"),
206-
SHA: github.Ptr("abc123"),
207-
Type: github.Ptr("file"),
209+
Name: github.Ptr("README.md"),
210+
Path: github.Ptr("README.md"),
211+
SHA: github.Ptr("abc123"),
212+
Type: github.Ptr("file"),
213+
Content: github.Ptr(encodedContent),
214+
Size: github.Ptr(len(mockRawContent)),
215+
Encoding: github.Ptr("base64"),
208216
}
209217
contentBytes, _ := json.Marshal(fileContent)
210218
_, _ = w.Write(contentBytes)
211219
},
212-
GetRawReposContentsByOwnerByRepoByBranchByPath: func(w http.ResponseWriter, _ *http.Request) {
213-
w.Header().Set("Content-Type", "text/markdown")
214-
_, _ = w.Write(mockRawContent)
215-
},
216220
}),
217221
requestArgs: map[string]interface{}{
218222
"owner": "owner",
@@ -224,7 +228,7 @@ func Test_GetFileContents(t *testing.T) {
224228
expectedResult: mcp.ResourceContents{
225229
URI: "repo://owner/repo/refs/heads/main/contents/README.md",
226230
Text: "# Test Repository\n\nThis is a test repository.",
227-
MIMEType: "text/markdown",
231+
MIMEType: "text/plain",
228232
},
229233
},
230234
{
@@ -283,27 +287,20 @@ func Test_GetFileContents(t *testing.T) {
283287
},
284288
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
285289
w.WriteHeader(http.StatusOK)
290+
// Base64 encode the content as GitHub API does
291+
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
286292
fileContent := &github.RepositoryContent{
287-
Name: github.Ptr("README.md"),
288-
Path: github.Ptr("README.md"),
289-
SHA: github.Ptr("abc123"),
290-
Type: github.Ptr("file"),
293+
Name: github.Ptr("README.md"),
294+
Path: github.Ptr("README.md"),
295+
SHA: github.Ptr("abc123"),
296+
Type: github.Ptr("file"),
297+
Content: github.Ptr(encodedContent),
298+
Size: github.Ptr(len(mockRawContent)),
299+
Encoding: github.Ptr("base64"),
291300
}
292301
contentBytes, _ := json.Marshal(fileContent)
293302
_, _ = w.Write(contentBytes)
294303
},
295-
"GET /owner/repo/refs/heads/develop/README.md": func(w http.ResponseWriter, _ *http.Request) {
296-
w.Header().Set("Content-Type", "text/markdown")
297-
_, _ = w.Write(mockRawContent)
298-
},
299-
"GET /owner/repo/refs%2Fheads%2Fdevelop/README.md": func(w http.ResponseWriter, _ *http.Request) {
300-
w.Header().Set("Content-Type", "text/markdown")
301-
_, _ = w.Write(mockRawContent)
302-
},
303-
"GET /owner/repo/abc123def456/README.md": func(w http.ResponseWriter, _ *http.Request) {
304-
w.Header().Set("Content-Type", "text/markdown")
305-
_, _ = w.Write(mockRawContent)
306-
},
307304
}),
308305
requestArgs: map[string]interface{}{
309306
"owner": "owner",
@@ -315,10 +312,43 @@ func Test_GetFileContents(t *testing.T) {
315312
expectedResult: mcp.ResourceContents{
316313
URI: "repo://owner/repo/abc123def456/contents/README.md",
317314
Text: "# Test Repository\n\nThis is a test repository.",
318-
MIMEType: "text/markdown",
315+
MIMEType: "text/plain",
319316
},
320317
expectedMsg: " Note: the provided ref 'main' does not exist, default branch 'refs/heads/develop' was used instead.",
321318
},
319+
{
320+
name: "large file returns ResourceLink",
321+
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
322+
GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, "{\"ref\": \"refs/heads/main\", \"object\": {\"sha\": \"\"}}"),
323+
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, "{\"name\": \"repo\", \"default_branch\": \"main\"}"),
324+
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
325+
w.WriteHeader(http.StatusOK)
326+
// File larger than 1MB - Contents API returns metadata but no content
327+
fileContent := &github.RepositoryContent{
328+
Name: github.Ptr("large-file.bin"),
329+
Path: github.Ptr("large-file.bin"),
330+
SHA: github.Ptr("largesha123"),
331+
Type: github.Ptr("file"),
332+
Size: github.Ptr(2 * 1024 * 1024), // 2MB
333+
DownloadURL: github.Ptr("https://raw.githubusercontent.com/owner/repo/main/large-file.bin"),
334+
}
335+
contentBytes, _ := json.Marshal(fileContent)
336+
_, _ = w.Write(contentBytes)
337+
},
338+
}),
339+
requestArgs: map[string]interface{}{
340+
"owner": "owner",
341+
"repo": "repo",
342+
"path": "large-file.bin",
343+
"ref": "refs/heads/main",
344+
},
345+
expectError: false,
346+
expectedResult: &mcp.ResourceLink{
347+
URI: "repo://owner/repo/refs/heads/main/contents/large-file.bin",
348+
Name: "large-file.bin",
349+
Title: "File: large-file.bin",
350+
},
351+
},
322352
{
323353
name: "content fetch fails",
324354
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -395,6 +425,14 @@ func Test_GetFileContents(t *testing.T) {
395425
assert.Equal(t, *expected[i].Path, *content.Path)
396426
assert.Equal(t, *expected[i].Type, *content.Type)
397427
}
428+
case *mcp.ResourceLink:
429+
// Large file returns a ResourceLink
430+
require.Len(t, result.Content, 2)
431+
resourceLink, ok := result.Content[1].(*mcp.ResourceLink)
432+
require.True(t, ok, "expected Content[1] to be ResourceLink")
433+
assert.Equal(t, expected.URI, resourceLink.URI)
434+
assert.Equal(t, expected.Name, resourceLink.Name)
435+
assert.Equal(t, expected.Title, resourceLink.Title)
398436
case mcp.TextContent:
399437
textContent := getErrorResult(t, result)
400438
require.Equal(t, textContent, expected)

0 commit comments

Comments
 (0)