Skip to content

Commit 0568187

Browse files
Always include SHA in get_file_contents responses (#676)
* fix: Add SHA to get_file_contents while preserving MCP behavior (#595) Enhance get_file_contents to include SHA information without changing the existing MCP server response format. Changes: - Add Contents API call to retrieve SHA before fetching raw content - Include SHA in resourceURI (repo://owner/repo/sha/{SHA}/contents/path) - Add SHA to success messages - Update tests to verify SHA inclusion - Maintain original behavior: text files return raw text, binaries return base64 This preserves backward compatibility while providing SHA information for better file versioning support. Closes #595 * fix: Improve error handling for Contents API response Ensure response body is properly closed even when an error occurs by moving the defer statement before the error check. This prevents potential resource leaks when the Contents API returns an error with a non-nil response. Changes: - Move defer respContents.Body.Close() before error checking - Rename errContents to err for consistency - Add nil check for respContents before attempting to close body This follows Go best practices for handling HTTP responses and prevents potential goroutine/memory leaks. * revert changes to resource URI * use GraphQL API to get file SHA * refactor: mock GQL client instead of getFileSHA function to follow conventions * lint * revert GraphQL --------- Co-authored-by: LuluBeatson <lulubeatson@github.com>
1 parent be91795 commit 0568187

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

pkg/github/repositories.go

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,24 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
507507
// If the path is (most likely) not to be a directory, we will
508508
// first try to get the raw content from the GitHub raw content API.
509509
if path != "" && !strings.HasSuffix(path, "/") {
510+
// First, get file info from Contents API to retrieve SHA
511+
var fileSHA string
512+
opts := &github.RepositoryContentGetOptions{Ref: ref}
513+
fileContent, _, respContents, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
514+
if respContents != nil {
515+
defer func() { _ = respContents.Body.Close() }()
516+
}
517+
if err != nil {
518+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
519+
"failed to get file SHA",
520+
respContents,
521+
err,
522+
), nil
523+
}
524+
if fileContent == nil || fileContent.SHA == nil {
525+
return mcp.NewToolResultError("file content SHA is nil"), nil
526+
}
527+
fileSHA = *fileContent.SHA
510528

511529
rawClient, err := getRawClient(ctx)
512530
if err != nil {
@@ -548,18 +566,28 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
548566
}
549567

550568
if strings.HasPrefix(contentType, "application") || strings.HasPrefix(contentType, "text") {
551-
return mcp.NewToolResultResource("successfully downloaded text file", mcp.TextResourceContents{
569+
result := mcp.TextResourceContents{
552570
URI: resourceURI,
553571
Text: string(body),
554572
MIMEType: contentType,
555-
}), nil
573+
}
574+
// Include SHA in the result metadata
575+
if fileSHA != "" {
576+
return mcp.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA), result), nil
577+
}
578+
return mcp.NewToolResultResource("successfully downloaded text file", result), nil
556579
}
557580

558-
return mcp.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{
581+
result := mcp.BlobResourceContents{
559582
URI: resourceURI,
560583
Blob: base64.StdEncoding.EncodeToString(body),
561584
MIMEType: contentType,
562-
}), nil
585+
}
586+
// Include SHA in the result metadata
587+
if fileSHA != "" {
588+
return mcp.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA), result), nil
589+
}
590+
return mcp.NewToolResultResource("successfully downloaded binary file", result), nil
563591

564592
}
565593
}

pkg/github/repositories_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,20 @@ func Test_GetFileContents(t *testing.T) {
7676
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`))
7777
}),
7878
),
79+
mock.WithRequestMatchHandler(
80+
mock.GetReposContentsByOwnerByRepoByPath,
81+
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
82+
w.WriteHeader(http.StatusOK)
83+
fileContent := &github.RepositoryContent{
84+
Name: github.Ptr("README.md"),
85+
Path: github.Ptr("README.md"),
86+
SHA: github.Ptr("abc123"),
87+
Type: github.Ptr("file"),
88+
}
89+
contentBytes, _ := json.Marshal(fileContent)
90+
_, _ = w.Write(contentBytes)
91+
}),
92+
),
7993
mock.WithRequestMatchHandler(
8094
raw.GetRawReposContentsByOwnerByRepoByBranchByPath,
8195
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
@@ -107,6 +121,20 @@ func Test_GetFileContents(t *testing.T) {
107121
_, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`))
108122
}),
109123
),
124+
mock.WithRequestMatchHandler(
125+
mock.GetReposContentsByOwnerByRepoByPath,
126+
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
127+
w.WriteHeader(http.StatusOK)
128+
fileContent := &github.RepositoryContent{
129+
Name: github.Ptr("test.png"),
130+
Path: github.Ptr("test.png"),
131+
SHA: github.Ptr("def456"),
132+
Type: github.Ptr("file"),
133+
}
134+
contentBytes, _ := json.Marshal(fileContent)
135+
_, _ = w.Write(contentBytes)
136+
}),
137+
),
110138
mock.WithRequestMatchHandler(
111139
raw.GetRawReposContentsByOwnerByRepoByBranchByPath,
112140
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {

0 commit comments

Comments
 (0)