Skip to content

Commit 3f52d21

Browse files
almaleksiaalmaleksia
committed
Improvements & refactoring of get_file_contents
1 parent 90a1255 commit 3f52d21

File tree

2 files changed

+85
-91
lines changed

2 files changed

+85
-91
lines changed

‎pkg/github/__toolsnaps__/get_file_contents.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
},
1818
"path": {
1919
"type": "string",
20-
"description": "Path to file/directory (directories must end with a slash '/')",
20+
"description": "Path to file/directory",
2121
"default": "/"
2222
},
2323
"ref": {

‎pkg/github/repositories.go‎

Lines changed: 84 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
560560
},
561561
"path": {
562562
Type: "string",
563-
Description: "Path to file/directory (directories must end with a slash '/')",
563+
Description: "Path to file/directory",
564564
Default: json.RawMessage(`"/"`),
565565
},
566566
"ref": {
@@ -608,132 +608,126 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t
608608
return utils.NewToolResultError(fmt.Sprintf("failed to resolve git reference: %s", err)), nil, nil
609609
}
610610

611+
if rawOpts.SHA != "" {
612+
ref = rawOpts.SHA
613+
}
614+
611615
// If the path is (most likely) not to be a directory, we will
612616
// first try to get the raw content from the GitHub raw content API.
613617

614618
var rawAPIResponseCode int
615-
if path != "" && !strings.HasSuffix(path, "/") {
616-
// First, get file info from Contents API to retrieve SHA
617-
var fileSHA string
618-
opts := &github.RepositoryContentGetOptions{Ref: ref}
619-
fileContent, _, respContents, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
620-
if respContents != nil {
621-
defer func() { _ = respContents.Body.Close() }()
622-
}
623-
if err != nil {
624-
return ghErrors.NewGitHubAPIErrorResponse(ctx,
625-
"failed to get file SHA",
626-
respContents,
627-
err,
628-
), nil, nil
629-
}
630-
if fileContent == nil || fileContent.SHA == nil {
631-
return utils.NewToolResultError("file content SHA is nil, if a directory was requested, path parameters should end with a trailing slash '/'"), nil, nil
632-
}
633-
fileSHA = *fileContent.SHA
619+
// First, get file info from Contents API to retrieve SHA
620+
var fileSHA string
621+
opts := &github.RepositoryContentGetOptions{Ref: ref}
622+
fileContent, dirContent, respContents, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
623+
if respContents != nil {
624+
defer func() { _ = respContents.Body.Close() }()
625+
}
626+
if err != nil {
627+
return ghErrors.NewGitHubAPIErrorResponse(ctx,
628+
"failed to get file SHA",
629+
respContents,
630+
err,
631+
), nil, nil
632+
}
634633

635-
rawClient, err := getRawClient(ctx)
634+
// file content or file SHA is nil which means it's a directory
635+
if fileContent == nil || fileContent.SHA == nil {
636+
defer func() { _ = respContents.Body.Close() }()
637+
r, err := json.Marshal(dirContent)
636638
if err != nil {
637-
return utils.NewToolResultError("failed to get GitHub raw content client"), nil, nil
639+
return utils.NewToolResultError("failed to marshal response"), nil, nil
638640
}
639-
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
641+
return utils.NewToolResultText(string(r)), nil, nil
642+
}
643+
644+
fileSHA = *fileContent.SHA
645+
646+
rawClient, err := getRawClient(ctx)
647+
if err != nil {
648+
return utils.NewToolResultError("failed to get GitHub raw content client"), nil, nil
649+
}
650+
resp, err := rawClient.GetRawContent(ctx, owner, repo, path, rawOpts)
651+
if err != nil {
652+
return utils.NewToolResultError("failed to get raw repository content"), nil, nil
653+
}
654+
defer func() {
655+
_ = resp.Body.Close()
656+
}()
657+
658+
if resp.StatusCode == http.StatusOK {
659+
// If the raw content is found, return it directly
660+
body, err := io.ReadAll(resp.Body)
640661
if err != nil {
641-
return utils.NewToolResultError("failed to get raw repository content"), nil, nil
662+
return utils.NewToolResultError("failed to read response body"), nil, nil
642663
}
643-
defer func() {
644-
_ = resp.Body.Close()
645-
}()
664+
contentType := resp.Header.Get("Content-Type")
646665

647-
if resp.StatusCode == http.StatusOK {
648-
// If the raw content is found, return it directly
649-
body, err := io.ReadAll(resp.Body)
666+
var resourceURI string
667+
switch {
668+
case sha != "":
669+
resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path)
650670
if err != nil {
651-
return utils.NewToolResultError("failed to read response body"), nil, nil
671+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
652672
}
653-
contentType := resp.Header.Get("Content-Type")
654-
655-
var resourceURI string
656-
switch {
657-
case sha != "":
658-
resourceURI, err = url.JoinPath("repo://", owner, repo, "sha", sha, "contents", path)
659-
if err != nil {
660-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
661-
}
662-
case ref != "":
663-
resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path)
664-
if err != nil {
665-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
666-
}
667-
default:
668-
resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path)
669-
if err != nil {
670-
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
671-
}
673+
case ref != "":
674+
resourceURI, err = url.JoinPath("repo://", owner, repo, ref, "contents", path)
675+
if err != nil {
676+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
672677
}
673-
674-
// Determine if content is text or binary
675-
isTextContent := strings.HasPrefix(contentType, "text/") ||
676-
contentType == "application/json" ||
677-
contentType == "application/xml" ||
678-
strings.HasSuffix(contentType, "+json") ||
679-
strings.HasSuffix(contentType, "+xml")
680-
681-
if isTextContent {
682-
result := &mcp.ResourceContents{
683-
URI: resourceURI,
684-
Text: string(body),
685-
MIMEType: contentType,
686-
}
687-
// Include SHA in the result metadata
688-
if fileSHA != "" {
689-
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA), result), nil, nil
690-
}
691-
return utils.NewToolResultResource("successfully downloaded text file", result), nil, nil
678+
default:
679+
resourceURI, err = url.JoinPath("repo://", owner, repo, "contents", path)
680+
if err != nil {
681+
return nil, nil, fmt.Errorf("failed to create resource URI: %w", err)
692682
}
683+
}
684+
685+
// Determine if content is text or binary
686+
isTextContent := strings.HasPrefix(contentType, "text/") ||
687+
contentType == "application/json" ||
688+
contentType == "application/xml" ||
689+
strings.HasSuffix(contentType, "+json") ||
690+
strings.HasSuffix(contentType, "+xml")
693691

692+
if isTextContent {
694693
result := &mcp.ResourceContents{
695694
URI: resourceURI,
696-
Blob: body,
695+
Text: string(body),
697696
MIMEType: contentType,
698697
}
699698
// Include SHA in the result metadata
700699
if fileSHA != "" {
701-
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA), result), nil, nil
700+
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded text file (SHA: %s)", fileSHA), result), nil, nil
702701
}
703-
return utils.NewToolResultResource("successfully downloaded binary file", result), nil, nil
702+
return utils.NewToolResultResource("successfully downloaded text file", result), nil, nil
704703
}
705-
rawAPIResponseCode = resp.StatusCode
706-
}
707704

708-
if rawOpts.SHA != "" {
709-
ref = rawOpts.SHA
710-
}
711-
if strings.HasSuffix(path, "/") {
712-
opts := &github.RepositoryContentGetOptions{Ref: ref}
713-
_, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
714-
if err == nil && resp.StatusCode == http.StatusOK {
715-
defer func() { _ = resp.Body.Close() }()
716-
r, err := json.Marshal(dirContent)
717-
if err != nil {
718-
return utils.NewToolResultError("failed to marshal response"), nil, nil
719-
}
720-
return utils.NewToolResultText(string(r)), nil, nil
705+
result := &mcp.ResourceContents{
706+
URI: resourceURI,
707+
Blob: body,
708+
MIMEType: contentType,
709+
}
710+
// Include SHA in the result metadata
711+
if fileSHA != "" {
712+
return utils.NewToolResultResource(fmt.Sprintf("successfully downloaded binary file (SHA: %s)", fileSHA), result), nil, nil
721713
}
714+
return utils.NewToolResultResource("successfully downloaded binary file", result), nil, nil
722715
}
716+
rawAPIResponseCode = resp.StatusCode
723717

724718
// The path does not point to a file or directory.
725719
// Instead let's try to find it in the Git Tree by matching the end of the path.
726720

727721
// Step 1: Get Git Tree recursively
728-
tree, resp, err := client.Git.GetTree(ctx, owner, repo, ref, true)
722+
tree, response, err := client.Git.GetTree(ctx, owner, repo, ref, true)
729723
if err != nil {
730724
return ghErrors.NewGitHubAPIErrorResponse(ctx,
731725
"failed to get git tree",
732-
resp,
726+
response,
733727
err,
734728
), nil, nil
735729
}
736-
defer func() { _ = resp.Body.Close() }()
730+
defer func() { _ = response.Body.Close() }()
737731

738732
// Step 2: Filter tree for matching paths
739733
const maxMatchingFiles = 3

0 commit comments

Comments
 (0)