From 5e4d0b8236395bfa1eea771d147e9e9494d02e56 Mon Sep 17 00:00:00 2001 From: yonaka Date: Fri, 27 Jun 2025 10:01:22 +0900 Subject: [PATCH] fix: Return full file content object to include SHA The previous implementation of `get_file_contents` had two issues: 1. When fetching a file, it used the raw content endpoint, which does not provide the file's SHA. 2. When fetching a directory, it returned a raw JSON array, which was not easily parsable by the agent. This commit refactors the `get_file_contents` tool to always use the `client.Repositories.GetContents` method. This method returns a `RepositoryContent` object (or a slice of them for directories), which always includes the SHA and other metadata. The function now marshals this object (or slice) into a JSON string, providing a consistent and parsable output that includes the necessary SHA for subsequent file operations. --- pkg/github/repositories.go | 120 ++++++++++--------------------------- 1 file changed, 32 insertions(+), 88 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index fa5d7338a..223182ec0 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" "strconv" "strings" @@ -460,7 +458,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t ), mcp.WithString("path", mcp.Required(), - mcp.Description("Path to file/directory (directories must end with a slash '/')"), + mcp.Description("Path to file/directory"), ), mcp.WithString("ref", mcp.Description("Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head`"), @@ -491,8 +489,6 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t return mcp.NewToolResultError(err.Error()), nil } - rawOpts := &raw.RawContentOpts{} - if strings.HasPrefix(ref, "refs/pull/") { prNumber := strings.TrimSuffix(strings.TrimPrefix(ref, "refs/pull/"), "/head") if len(prNumber) > 0 { @@ -514,101 +510,49 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t } } - rawOpts.SHA = sha - rawOpts.Ref = ref - - // 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. - 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 not found, we will fall back to the GitHub API (in case it is a directory) - } else { - // If the raw content is found, return it directly - body, err := io.ReadAll(resp.Body) - 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.NewToolResultResource("successfully downloaded binary file", mcp.BlobResourceContents{ - URI: resourceURI, - Blob: base64.StdEncoding.EncodeToString(body), - MIMEType: contentType, - }), nil - - } - } - client, err := getClient(ctx) if err != nil { - return mcp.NewToolResultError("failed to get GitHub client"), nil + return nil, fmt.Errorf("failed to get GitHub client: %w", err) } if sha != "" { ref = sha } - if strings.HasSuffix(path, "/") { - opts := &github.RepositoryContentGetOptions{Ref: ref} - _, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) - if err != nil { - return mcp.NewToolResultError("failed to get file contents"), nil - } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != 200 { - body, err := io.ReadAll(resp.Body) - if err != nil { - return mcp.NewToolResultError("failed to read response body"), nil - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get file contents: %s", string(body))), nil - } + opts := &github.RepositoryContentGetOptions{Ref: ref} + fileContent, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get file contents", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(dirContent) + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) if err != nil { - return mcp.NewToolResultError("failed to marshal response"), nil + return nil, fmt.Errorf("failed to read response body: %w", err) } - return mcp.NewToolResultText(string(r)), nil + return mcp.NewToolResultError(fmt.Sprintf("failed to get file contents: %s", string(body))), nil + } + + var result any + if fileContent != nil { + result = fileContent + } else if dirContent != nil { + result = dirContent + } else { + return mcp.NewToolResultError("failed to get file contents: response was empty"), nil + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) } - return mcp.NewToolResultError("Failed to get file contents. The path does not point to a file or directory, or the file does not exist in the repository."), nil + + return mcp.NewToolResultText(string(r)), nil } }