From d5cd69a468e5326eb077e29136aa92e6362f027b Mon Sep 17 00:00:00 2001 From: yonaka15 Date: Sat, 12 Jul 2025 23:20:41 +0900 Subject: [PATCH 1/2] fix: Always include SHA in get_file_contents responses (#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 #605 --- pkg/github/repositories.go | 63 +++----------------- pkg/github/repositories_test.go | 101 +++++++++++++++++++++----------- 2 files changed, 74 insertions(+), 90 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 186bd2321..1487e63fc 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -2,12 +2,10 @@ package github import ( "context" - "encoding/base64" "encoding/json" "fmt" "io" "net/http" - "net/url" "strings" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -505,62 +503,17 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t } // If the path is (most likely) not to be a directory, we will - // first try to get the raw content from the GitHub raw content API. + // try to get the content using the GitHub contents API to include SHA information. if path != "" && !strings.HasSuffix(path, "/") { - - rawClient, err := getRawClient(ctx) - if err != nil { - return mcp.NewToolResultError("failed to get GitHub raw content client"), nil - } - resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts) - if err != nil { - return mcp.NewToolResultError("failed to get raw repository content"), nil - } - defer func() { - _ = resp.Body.Close() - }() - - if resp.StatusCode == http.StatusOK { - // If the raw content is found, return it directly - body, err := io.ReadAll(resp.Body) + opts := &github.RepositoryContentGetOptions{Ref: ref} + fileContent, _, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) + if err == nil && resp.StatusCode == http.StatusOK && fileContent != nil { + defer func() { _ = resp.Body.Close() }() + r, err := json.Marshal(fileContent) if err != nil { - return mcp.NewToolResultError("failed to read response body"), nil - } - contentType := resp.Header.Get("Content-Type") - - var resourceURI string - switch { - case sha != "": - resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - case ref != "": - resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - default: - resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path) - if err != nil { - return nil, fmt.Errorf("failed to create resource URI: %w", err) - } - } - - if strings.HasPrefix(contentType, "application") || strings.HasPrefix(contentType, "text") { - return mcp.NewToolResultResource("successfully downloaded text file", mcp.TextResourceContents{ - URI: resourceURI, - Text: string(body), - MIMEType: contentType, - }), nil + return mcp.NewToolResultError("failed to marshal response"), nil } - - return mcp.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{ - URI: resourceURI, - Blob: base64.StdEncoding.EncodeToString(body), - MIMEType: contentType, - }), nil - + return mcp.NewToolResultText(string(r)), nil } } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 4977bb0a9..ff7d90886 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -35,8 +35,7 @@ func Test_GetFileContents(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "sha") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) - // Mock response for raw content - mockRawContent := []byte("# Test Repository\n\nThis is a test repository.") + // Setup mock directory content for success case mockDirContent := []*github.RepositoryContent{ @@ -67,20 +66,31 @@ func Test_GetFileContents(t *testing.T) { expectStatus int }{ { - name: "successful text content fetch", + name: "successful file content fetch", mockedClient: mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( mock.GetReposGitRefByOwnerByRepoByRef, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`)) + _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "abc123"}}`)) }), ), mock.WithRequestMatchHandler( - raw.GetRawReposContentsByOwnerByRepoByBranchByPath, + mock.GetReposContentsByOwnerByRepoByPath, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "text/markdown") - _, _ = w.Write(mockRawContent) + w.WriteHeader(http.StatusOK) + fileContent := &github.RepositoryContent{ + Type: github.Ptr("file"), + Name: github.Ptr("README.md"), + Path: github.Ptr("README.md"), + SHA: github.Ptr("abc123"), + Size: github.Ptr(42), + Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("# Test Repository\n\nThis is a test repository."))), + Encoding: github.Ptr("base64"), + HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/README.md"), + } + responseBody, _ := json.Marshal(fileContent) + _, _ = w.Write(responseBody) }), ), ), @@ -91,27 +101,43 @@ func Test_GetFileContents(t *testing.T) { "ref": "refs/heads/main", }, expectError: false, - expectedResult: mcp.TextResourceContents{ - URI: "repo://owner/repo/refs/heads/main/contents/README.md", - Text: "# Test Repository\n\nThis is a test repository.", - MIMEType: "text/markdown", + expectedResult: &github.RepositoryContent{ + Type: github.Ptr("file"), + Name: github.Ptr("README.md"), + Path: github.Ptr("README.md"), + SHA: github.Ptr("abc123"), + Size: github.Ptr(42), + Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("# Test Repository\n\nThis is a test repository."))), + Encoding: github.Ptr("base64"), + HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/README.md"), }, }, { - name: "successful file blob content fetch", + name: "successful binary file content fetch", mockedClient: mock.NewMockedHTTPClient( mock.WithRequestMatchHandler( mock.GetReposGitRefByOwnerByRepoByRef, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": ""}}`)) + _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "def456"}}`)) }), ), mock.WithRequestMatchHandler( - raw.GetRawReposContentsByOwnerByRepoByBranchByPath, + mock.GetReposContentsByOwnerByRepoByPath, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Content-Type", "image/png") - _, _ = w.Write(mockRawContent) + w.WriteHeader(http.StatusOK) + fileContent := &github.RepositoryContent{ + Type: github.Ptr("file"), + Name: github.Ptr("test.png"), + Path: github.Ptr("test.png"), + SHA: github.Ptr("def456"), + Size: github.Ptr(1024), + Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("binary-image-content"))), + Encoding: github.Ptr("base64"), + HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/test.png"), + } + responseBody, _ := json.Marshal(fileContent) + _, _ = w.Write(responseBody) }), ), ), @@ -122,10 +148,15 @@ func Test_GetFileContents(t *testing.T) { "ref": "refs/heads/main", }, expectError: false, - expectedResult: mcp.BlobResourceContents{ - URI: "repo://owner/repo/refs/heads/main/contents/test.png", - Blob: base64.StdEncoding.EncodeToString(mockRawContent), - MIMEType: "image/png", + expectedResult: &github.RepositoryContent{ + Type: github.Ptr("file"), + Name: github.Ptr("test.png"), + Path: github.Ptr("test.png"), + SHA: github.Ptr("def456"), + Size: github.Ptr(1024), + Content: github.Ptr(base64.StdEncoding.EncodeToString([]byte("binary-image-content"))), + Encoding: github.Ptr("base64"), + HTMLURL: github.Ptr("https://github.com/owner/repo/blob/main/test.png"), }, }, { @@ -151,14 +182,7 @@ func Test_GetFileContents(t *testing.T) { mockResponse(t, http.StatusOK, mockDirContent), ), ), - mock.WithRequestMatchHandler( - raw.GetRawReposContentsByOwnerByRepoByPath, - expectQueryParams(t, map[string]string{ - "branch": "main", - }).andThen( - mockResponse(t, http.StatusNotFound, nil), - ), - ), + ), requestArgs: map[string]interface{}{ "owner": "owner", @@ -186,7 +210,7 @@ func Test_GetFileContents(t *testing.T) { }), ), mock.WithRequestMatchHandler( - raw.GetRawReposContentsByOwnerByRepoByPath, + mock.GetReposGitTreesByOwnerByRepoByTreeSha, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte(`{"message": "Not Found"}`)) @@ -227,12 +251,19 @@ func Test_GetFileContents(t *testing.T) { require.NoError(t, err) // Use the correct result helper based on the expected type switch expected := tc.expectedResult.(type) { - case mcp.TextResourceContents: - textResource := getTextResourceResult(t, result) - assert.Equal(t, expected, textResource) - case mcp.BlobResourceContents: - blobResource := getBlobResourceResult(t, result) - assert.Equal(t, expected, blobResource) + case *github.RepositoryContent: + // File content fetch returns a text result (JSON object) + textContent := getTextResult(t, result) + var returnedContent github.RepositoryContent + err = json.Unmarshal([]byte(textContent.Text), &returnedContent) + require.NoError(t, err, "Failed to unmarshal file content result: %v", textContent.Text) + assert.Equal(t, *expected.Name, *returnedContent.Name) + assert.Equal(t, *expected.Path, *returnedContent.Path) + assert.Equal(t, *expected.Type, *returnedContent.Type) + assert.Equal(t, *expected.SHA, *returnedContent.SHA) + if expected.Content != nil && returnedContent.Content != nil { + assert.Equal(t, *expected.Content, *returnedContent.Content) + } case []*github.RepositoryContent: // Directory content fetch returns a text result (JSON array) textContent := getTextResult(t, result) From e99500ee66fc16c07a2a63254c7c94c074b75082 Mon Sep 17 00:00:00 2001 From: yonaka15 Date: Sat, 12 Jul 2025 23:31:00 +0900 Subject: [PATCH 2/2] fix: Mark getRawClient parameter as unused The getRawClient parameter is no longer used after switching from Raw Content API to Contents API. Mark it with underscore to satisfy golangci-lint while maintaining API compatibility. Part of #605 --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 1487e63fc..562abf70f 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -444,7 +444,7 @@ func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFun } // GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository. -func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func GetFileContents(getClient GetClientFn, _ raw.GetRawClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_file_contents", mcp.WithDescription(t("TOOL_GET_FILE_CONTENTS_DESCRIPTION", "Get the contents of a file or directory from a GitHub repository")), mcp.WithToolAnnotation(mcp.ToolAnnotation{