Skip to content

repository resource tests #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 2, 2025
Merged

repository resource tests #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
383c2d2
refactor to make testing easier
mntlty Apr 2, 2025
1cb52f9
not needed in handler func
mntlty Apr 2, 2025
2aa3002
small cleanup
mntlty Apr 2, 2025
d8b0056
create repository_resource_test
mntlty Apr 2, 2025
2fdda7c
remove chatty comments
mntlty Apr 2, 2025
9680b24
comment cleanup, function rename and some more tests
mntlty Apr 2, 2025
b4e6772
fix test for ubuntu runner
mntlty Apr 2, 2025
db7a180
remove it for now
mntlty Apr 2, 2025
02ebdc7
make required args explicit instead of panic
mntlty Apr 2, 2025
ad58220
more tests and cleanup
mntlty Apr 2, 2025
ede9f22
chore: use raw repo resources (#70)
SamMorrowDrums Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 149 additions & 70 deletions pkg/github/repository_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ package github
import (
"context"
"encoding/base64"
"errors"
"fmt"
"io"
"mime"
"net/http"
"path/filepath"
"strings"

Expand All @@ -13,110 +17,185 @@ import (
"github.com/mark3labs/mcp-go/server"
)

// getRepositoryContent defines the resource template and handler for the Repository Content API.
func getRepositoryContent(client *github.Client, t translations.TranslationHelperFunc) (mainTemplate mcp.ResourceTemplate, reftemplate mcp.ResourceTemplate, shaTemplate mcp.ResourceTemplate, tagTemplate mcp.ResourceTemplate, prTemplate mcp.ResourceTemplate, handler server.ResourceTemplateHandlerFunc) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this format of returning multiple values made it quite hard to evaluate and test, instead I broke them up into individual functions


// getRepositoryResourceContent defines the resource template and handler for getting repository content.
func getRepositoryResourceContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
return mcp.NewResourceTemplate(
"repo://{owner}/{repo}/contents{/path*}", // Resource template
t("RESOURCE_REPOSITORY_CONTENT_DESCRIPTION", "Repository Content"),
), mcp.NewResourceTemplate(
),
repositoryResourceContentsHandler(client)
}

// getRepositoryContent defines the resource template and handler for getting repository content for a branch.
func getRepositoryResourceBranchContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
return mcp.NewResourceTemplate(
"repo://{owner}/{repo}/refs/heads/{branch}/contents{/path*}", // Resource template
t("RESOURCE_REPOSITORY_CONTENT_BRANCH_DESCRIPTION", "Repository Content for specific branch"),
), mcp.NewResourceTemplate(
),
repositoryResourceContentsHandler(client)
}

// getRepositoryResourceCommitContent defines the resource template and handler for getting repository content for a commit.
func getRepositoryResourceCommitContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
return mcp.NewResourceTemplate(
"repo://{owner}/{repo}/sha/{sha}/contents{/path*}", // Resource template
t("RESOURCE_REPOSITORY_CONTENT_COMMIT_DESCRIPTION", "Repository Content for specific commit"),
), mcp.NewResourceTemplate(
),
repositoryResourceContentsHandler(client)
}

// getRepositoryResourceTagContent defines the resource template and handler for getting repository content for a tag.
func getRepositoryResourceTagContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
return mcp.NewResourceTemplate(
"repo://{owner}/{repo}/refs/tags/{tag}/contents{/path*}", // Resource template
t("RESOURCE_REPOSITORY_CONTENT_TAG_DESCRIPTION", "Repository Content for specific tag"),
), mcp.NewResourceTemplate(
),
repositoryResourceContentsHandler(client)
}

// getRepositoryResourcePrContent defines the resource template and handler for getting repository content for a pull request.
func getRepositoryResourcePrContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) {
return mcp.NewResourceTemplate(
"repo://{owner}/{repo}/refs/pull/{pr_number}/head/contents{/path*}", // Resource template
t("RESOURCE_REPOSITORY_CONTENT_PR_DESCRIPTION", "Repository Content for specific pull request"),
), func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) {
// Extract parameters from request.Params.URI
),
repositoryResourceContentsHandler(client)
}

owner := request.Params.Arguments["owner"].([]string)[0]
repo := request.Params.Arguments["repo"].([]string)[0]
// path should be a joined list of the path parts
path := strings.Join(request.Params.Arguments["path"].([]string), "/")
func repositoryResourceContentsHandler(client *github.Client) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) {
return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) {
// the matcher will give []string with one elemenent
// https://github.com/mark3labs/mcp-go/pull/54
o, ok := request.Params.Arguments["owner"].([]string)
if !ok || len(o) == 0 {
return nil, errors.New("owner is required")
}
owner := o[0]

opts := &github.RepositoryContentGetOptions{}
r, ok := request.Params.Arguments["repo"].([]string)
if !ok || len(r) == 0 {
return nil, errors.New("repo is required")
}
repo := r[0]

sha, ok := request.Params.Arguments["sha"].([]string)
if ok {
opts.Ref = sha[0]
}
// path should be a joined list of the path parts
path := ""
p, ok := request.Params.Arguments["path"].([]string)
if ok {
path = strings.Join(p, "/")
}

branch, ok := request.Params.Arguments["branch"].([]string)
if ok {
opts.Ref = "refs/heads/" + branch[0]
}
opts := &github.RepositoryContentGetOptions{}

tag, ok := request.Params.Arguments["tag"].([]string)
if ok {
opts.Ref = "refs/tags/" + tag[0]
}
prNumber, ok := request.Params.Arguments["pr_number"].([]string)
if ok {
opts.Ref = "refs/pull/" + prNumber[0] + "/head"
}
sha, ok := request.Params.Arguments["sha"].([]string)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left this as it was previously, where one function was shared by all ResourceTemplates. I think it would be better to continue the refactor by creating individual functions that only parse arguments that are expected, for example the sha argument would only be parsed when getting a commit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just wanted to quickly scale out what I had written to multiple endpoints - but in truth, the shared logic could be shared, with separate opts generation and that is likely a better direction.

tl;dr I think you're right. I just spent enough time trying to decide how to wrap it as a resource, and make it work, I didn't spend much time on how it should be written.

if ok && len(sha) > 0 {
opts.Ref = sha[0]
}

// Use the GitHub client to fetch repository content
fileContent, directoryContent, _, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
if err != nil {
return nil, err
}
branch, ok := request.Params.Arguments["branch"].([]string)
if ok && len(branch) > 0 {
opts.Ref = "refs/heads/" + branch[0]
}

tag, ok := request.Params.Arguments["tag"].([]string)
if ok && len(tag) > 0 {
opts.Ref = "refs/tags/" + tag[0]
}
prNumber, ok := request.Params.Arguments["pr_number"].([]string)
if ok && len(prNumber) > 0 {
opts.Ref = "refs/pull/" + prNumber[0] + "/head"
}

if directoryContent != nil {
// Process the directory content and return it as resource contents
var resources []mcp.ResourceContents
for _, entry := range directoryContent {
mimeType := "text/directory"
if entry.GetType() == "file" {
mimeType = mime.TypeByExtension(filepath.Ext(entry.GetName()))
fileContent, directoryContent, _, err := client.Repositories.GetContents(ctx, owner, repo, path, opts)
if err != nil {
return nil, err
}

if directoryContent != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if directoryContent and fileContent are mutually exclusive, that is the previous behavior which I haven't changed (I only removed the unnecessary else clause)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are only returning the names of things with directoryContent, and not the actual file data. When we are returning the file data, we actually serialise the payload.

That's the best I could come up with, the alternative would be to not have a way to inspect directory content, and just return the fact it's a directory.

It's not a perfect fit, but no way are we returning all the blobs in a folder 😅

var resources []mcp.ResourceContents
for _, entry := range directoryContent {
mimeType := "text/directory"
if entry.GetType() == "file" {
// this is system dependent, and a best guess
ext := filepath.Ext(entry.GetName())
mimeType = mime.TypeByExtension(ext)
if ext == ".md" {
mimeType = "text/markdown"
}
resources = append(resources, mcp.TextResourceContents{
URI: entry.GetHTMLURL(),
MIMEType: mimeType,
Text: entry.GetName(),
})
}
resources = append(resources, mcp.TextResourceContents{
URI: entry.GetHTMLURL(),
MIMEType: mimeType,
Text: entry.GetName(),
})

}
return resources, nil

}
if fileContent != nil {
if fileContent.Content != nil {
// download the file content from fileContent.GetDownloadURL() and use the content-type header to determine the MIME type
// and return the content as a blob unless it is a text file, where you can return the content as text
req, err := http.NewRequest("GET", fileContent.GetDownloadURL(), nil)
if err != nil {
return nil, fmt.Errorf("failed to create request: %w", err)
}
return resources, nil

} else if fileContent != nil {
// Process the file content and return it as a binary resource
resp, err := client.Client().Do(req)
if err != nil {
return nil, fmt.Errorf("failed to send request: %w", err)
}
defer func() { _ = resp.Body.Close() }()

if fileContent.Content != nil {
decodedContent, err := fileContent.GetContent()
if resp.StatusCode != http.StatusOK {
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to read response body: %w", err)
}
return nil, fmt.Errorf("failed to fetch file content: %s", string(body))
}

mimeType := mime.TypeByExtension(filepath.Ext(fileContent.GetName()))

// Check if the file is text-based
if strings.HasPrefix(mimeType, "text") {
// Return as TextResourceContents
return []mcp.ResourceContents{
mcp.TextResourceContents{
URI: request.Params.URI,
MIMEType: mimeType,
Text: decodedContent,
},
}, nil
ext := filepath.Ext(fileContent.GetName())
mimeType := resp.Header.Get("Content-Type")
if ext == ".md" {
mimeType = "text/markdown"
} else if mimeType == "" {
// backstop to the file extension if the content type is not set
mimeType = mime.TypeByExtension(filepath.Ext(fileContent.GetName()))
}

// if the content is a string, return it as text
if strings.HasPrefix(mimeType, "text") {
content, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to parse the response body: %w", err)
}

// Otherwise, return as BlobResourceContents
return []mcp.ResourceContents{
mcp.BlobResourceContents{
mcp.TextResourceContents{
URI: request.Params.URI,
MIMEType: mimeType,
Blob: base64.StdEncoding.EncodeToString([]byte(decodedContent)), // Encode content as Base64
Text: string(content),
},
}, nil
}
}
// otherwise, read the content and encode it as base64
decodedContent, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to parse the response body: %w", err)
}

return nil, nil
return []mcp.ResourceContents{
mcp.BlobResourceContents{
URI: request.Params.URI,
MIMEType: mimeType,
Blob: base64.StdEncoding.EncodeToString(decodedContent), // Encode content as Base64
},
}, nil
}
}

return nil, errors.New("no repository resource content found")
}
}
Loading