Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add resource completion for GitHub repository resources#1493

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.

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

Already on GitHub?Sign in to your account

Open
omgitsads wants to merge7 commits intoomgitsads/go-sdk
base:omgitsads/go-sdk
Choose a base branch
Loading
fromomgitsads/go-sdk-completions
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
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
22 changes: 10 additions & 12 deletionsinternal/ghmcp/server.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -124,18 +124,6 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
// Generate instructions based on enabled toolsets
instructions := github.GenerateInstructions(enabledToolsets)

ghServer := github.NewServer(cfg.Version, &mcp.ServerOptions{
Instructions: instructions,
HasTools: true,
HasResources: true,
HasPrompts: true,
Logger: cfg.Logger,
})

// Add middlewares
ghServer.AddReceivingMiddleware(addGitHubAPIErrorToContext)
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, restClient, gqlHTTPClient))

getClient := func(_ context.Context) (*gogithub.Client, error) {
return restClient, nil // closing over client
}
Expand All@@ -152,6 +140,16 @@ func NewMCPServer(cfg MCPServerConfig) (*mcp.Server, error) {
return raw.NewClient(client, apiHost.rawURL), nil // closing over client
}

ghServer := github.NewServer(cfg.Version, &mcp.ServerOptions{
Instructions: instructions,
Logger: cfg.Logger,
CompletionHandler: github.CompletionsHandler(getClient),
})

// Add middlewares
ghServer.AddReceivingMiddleware(addGitHubAPIErrorToContext)
ghServer.AddReceivingMiddleware(addUserAgentsMiddleware(cfg, restClient, gqlHTTPClient))

// Create default toolsets
tsg := github.DefaultToolsetGroup(
cfg.ReadOnly,
Expand Down
335 changes: 335 additions & 0 deletionspkg/github/repository_resource_completions.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,335 @@
package github

import (
"context"
"errors"
"fmt"
"strings"

"github.com/google/go-github/v79/github"
"github.com/modelcontextprotocol/go-sdk/mcp"
)

// CompleteHandler defines function signature for completion handlers
type CompleteHandler func(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error)

// RepositoryResourceArgumentResolvers is a map of argument names to their completion handlers
var RepositoryResourceArgumentResolvers = map[string]CompleteHandler{
"owner": completeOwner,
"repo": completeRepo,
"branch": completeBranch,
"sha": completeSHA,
"tag": completeTag,
"prNumber": completePRNumber,
"path": completePath,
}

// RepositoryResourceCompletionHandler returns a CompletionHandlerFunc for repository resource completions.
func RepositoryResourceCompletionHandler(getClient GetClientFn) func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
return func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) {
if req.Params.Ref.Type != "ref/resource" {
return nil, nil // Not a resource completion
}

argName := req.Params.Argument.Name
argValue := req.Params.Argument.Value
resolved := req.Params.Context.Arguments
if resolved == nil {
resolved = map[string]string{}
}

client, err := getClient(ctx)
if err != nil {
return nil, err
}

// Argument resolver functions
resolvers := RepositoryResourceArgumentResolvers

resolver, ok := resolvers[argName]
if !ok {
return nil, errors.New("no resolver for argument: " + argName)
}

values, err := resolver(ctx, client, resolved, argValue)
if err != nil {
return nil, err
}
if len(values) > 100 {
values = values[:100]
}

return &mcp.CompleteResult{
Completion: mcp.CompletionResultDetails{
Values: values,
Total: len(values),
HasMore: false,
Comment on lines +58 to +66

Choose a reason for hiding this comment

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

HasMore field is always false. Line 68 setsHasMore: false even when results are truncated to 100 items (line 60-62). If more than 100 results exist, HasMore should be set to true to indicate that additional results are available.

Copilot uses AI. Check for mistakes.
},
}, nil
}
}

// --- Per-argument resolver functions ---

func completeOwner(ctx context.Context, client *github.Client, _ map[string]string, argValue string) ([]string, error) {
var values []string
user, _, err := client.Users.Get(ctx, "")
if err == nil && user.GetLogin() != "" {
values = append(values, user.GetLogin())
}

orgs, _, err := client.Organizations.List(ctx, "", &github.ListOptions{PerPage: 100})
if err != nil {
return nil, err
}
for _, org := range orgs {
values = append(values, org.GetLogin())
}

// filter values based on argValue and replace values slice
if argValue != "" {
var filteredValues []string
for _, value := range values {
if strings.Contains(value, argValue) {
filteredValues = append(filteredValues, value)
}
}
values = filteredValues
}
if len(values) > 100 {
values = values[:100]
return values, nil // Limit to 100 results
}
// Else also do a client.Search.Users()
if argValue == "" {
return values, nil // No need to search if no argValue
}
users, _, err := client.Search.Users(ctx, argValue, &github.SearchOptions{ListOptions: github.ListOptions{PerPage: 100 - len(values)}})
if err != nil || users == nil {
return nil, err
}
for _, user := range users.Users {
values = append(values, user.GetLogin())
}

if len(values) > 100 {
values = values[:100]
}
return values, nil
}

func completeRepo(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
var values []string
owner := resolved["owner"]
if owner == "" {
return values, errors.New("owner not specified")
}

query := fmt.Sprintf("org:%s", owner)

Choose a reason for hiding this comment

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

Search query may not work correctly for non-organization owners. Line 133 usesorg:%s in the search query, which will fail if the owner is a user rather than an organization. Consider usinguser:%s OR org:%s or checking the owner type first.

Copilot uses AI. Check for mistakes.

if argValue != "" {
query = fmt.Sprintf("%s %s", query, argValue)
}
repos, _, err := client.Search.Repositories(ctx, query, &github.SearchOptions{ListOptions: github.ListOptions{PerPage: 100}})
if err != nil || repos == nil {
return values, errors.New("failed to get repositories")
}
Comment on lines +134 to +136

Choose a reason for hiding this comment

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

Generic error message loses context. The error fromclient.Search.Repositories should be wrapped or propagated to provide more specific failure information to the caller. Consider returning the original error withfmt.Errorf("failed to get repositories: %w", err) instead of creating a new generic error.

Suggested change
iferr!=nil||repos==nil {
returnvalues,errors.New("failed to get repositories")
}
iferr!=nil {
returnvalues,fmt.Errorf("failed to get repositories: %w",err)
}
ifrepos==nil {
returnvalues,errors.New("failed to get repositories: response was nil")
}

Copilot uses AI. Check for mistakes.
// filter repos based on argValue
for _, repo := range repos.Repositories {
name := repo.GetName()
if argValue == "" || strings.HasPrefix(name, argValue) {
values = append(values, name)
}
}

return values, nil
}

func completeBranch(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
var values []string
owner := resolved["owner"]
repo := resolved["repo"]
if owner == "" || repo == "" {
return values, errors.New("owner or repo not specified")
}
branches, _, _ := client.Repositories.ListBranches(ctx, owner, repo, nil)

Choose a reason for hiding this comment

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

API call error is silently ignored. The error fromListBranches should be checked and handled. If the call fails, it could result in returning an empty list without informing the user of the failure. Consider handling the error similar to other completion functions or at minimum checking if branches is nil before iterating.

Suggested change
branches,_,_:=client.Repositories.ListBranches(ctx,owner,repo,nil)
branches,_,err:=client.Repositories.ListBranches(ctx,owner,repo,nil)
iferr!=nil||branches==nil {
returnvalues,errors.New("failed to get branches")
}

Copilot uses AI. Check for mistakes.

for _, branch := range branches {
if argValue == "" || strings.HasPrefix(branch.GetName(), argValue) {
values = append(values, branch.GetName())
}
}
if len(values) > 100 {
values = values[:100]
}
return values, nil
}

func completeSHA(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
var values []string
owner := resolved["owner"]
repo := resolved["repo"]
if owner == "" || repo == "" {
return values, errors.New("owner or repo not specified")
}
commits, _, _ := client.Repositories.ListCommits(ctx, owner, repo, nil)

Choose a reason for hiding this comment

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

API call error is silently ignored. The error fromListCommits should be checked and handled. If the call fails, it could result in returning an empty list without informing the user of the failure. Consider handling the error similar to how it's done incompletePRNumber or at minimum checking if commits is nil before iterating.

Suggested change
commits,_,_:=client.Repositories.ListCommits(ctx,owner,repo,nil)
commits,_,err:=client.Repositories.ListCommits(ctx,owner,repo,nil)
iferr!=nil||commits==nil {
returnvalues,errors.New("failed to get commits")
}

Copilot uses AI. Check for mistakes.

for _, commit := range commits {
sha := commit.GetSHA()
if argValue == "" || strings.HasPrefix(sha, argValue) {
values = append(values, sha)
}
}
if len(values) > 100 {
values = values[:100]
}
return values, nil
}

func completeTag(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
owner := resolved["owner"]
repo := resolved["repo"]
if owner == "" || repo == "" {
return nil, errors.New("owner or repo not specified")
}
tags, _, _ := client.Repositories.ListTags(ctx, owner, repo, nil)

Choose a reason for hiding this comment

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

API call error is silently ignored. The error fromListTags should be checked and handled. If the call fails, it could result in returning an empty list without informing the user of the failure. Consider handling the error similar to how it's done incompletePRNumber or at minimum checking if tags is nil before iterating.

Suggested change
tags,_,_:=client.Repositories.ListTags(ctx,owner,repo,nil)
tags,_,err:=client.Repositories.ListTags(ctx,owner,repo,nil)
iferr!=nil {
returnnil,err
}

Copilot uses AI. Check for mistakes.
var values []string
for _, tag := range tags {
if argValue == "" || strings.Contains(tag.GetName(), argValue) {
values = append(values, tag.GetName())
}
}
if len(values) > 100 {
values = values[:100]
}
return values, nil
}

func completePRNumber(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
var values []string
owner := resolved["owner"]
repo := resolved["repo"]
if owner == "" || repo == "" {
return values, errors.New("owner or repo not specified")
}

Choose a reason for hiding this comment

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

[nitpick] Only returns open pull requests. The query on line 221 filters foris:open is:pr, which means closed PRs won't be included in completions. Consider whether this is the intended behavior or if closed/merged PRs should also be available for completion. If this is intentional, consider adding a comment explaining the rationale.

Suggested change
// Only open PRs are returned for completion, as closed/merged PRs are typically not relevant for selection in this context.

Copilot uses AI. Check for mistakes.
prs, _, err := client.Search.Issues(ctx, fmt.Sprintf("repo:%s/%s is:open is:pr", owner, repo), &github.SearchOptions{ListOptions: github.ListOptions{PerPage: 100}})
if err != nil {
return values, err
}
for _, pr := range prs.Issues {
num := fmt.Sprintf("%d", pr.GetNumber())
if argValue == "" || strings.HasPrefix(num, argValue) {
values = append(values, num)
}
}
if len(values) > 100 {
values = values[:100]
}
return values, nil
}

func completePath(ctx context.Context, client *github.Client, resolved map[string]string, argValue string) ([]string, error) {
owner := resolved["owner"]
repo := resolved["repo"]
if owner == "" || repo == "" {
return nil, errors.New("owner or repo not specified")
}
refVal := resolved["branch"]
if refVal == "" {
refVal = resolved["sha"]
}
if refVal == "" {
refVal = resolved["tag"]
}
if refVal == "" {
refVal = "HEAD"
}

// Determine the prefix to complete (directory path or file path)
prefix := argValue
if prefix != "" && !strings.HasSuffix(prefix, "/") {
lastSlash := strings.LastIndex(prefix, "/")
if lastSlash >= 0 {
prefix = prefix[:lastSlash+1]
} else {
prefix = ""
}
}

// Get the tree for the ref (recursive)
tree, _, err := client.Git.GetTree(ctx, owner, repo, refVal, true)
if err != nil || tree == nil {
return nil, errors.New("failed to get file tree")
}

// Collect immediate children of the prefix (files and directories, no duplicates)
dirs := map[string]struct{}{}
files := map[string]struct{}{}
prefixLen := len(prefix)
for _, entry := range tree.Entries {
if !strings.HasPrefix(entry.GetPath(), prefix) {
continue
}
rel := entry.GetPath()[prefixLen:]
if rel == "" {
continue
}
// Only immediate children
slashIdx := strings.Index(rel, "/")
if slashIdx >= 0 {
// Directory: only add the directory name (with trailing slash), prefixed with full path
dirName := prefix + rel[:slashIdx+1]
dirs[dirName] = struct{}{}
} else if entry.GetType() == "blob" {
// File: add as-is, prefixed with full path
fileName := prefix + rel
files[fileName] = struct{}{}
Comment on lines +260 to +287

Choose a reason for hiding this comment

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

Fetching recursive tree could be inefficient for large repositories. Line 266 fetches the entire repository tree recursively (recursive: true), which can be very large for repositories with many files and deeply nested directories. This could cause performance issues or API rate limiting. Consider:

  1. Fetching only the specific directory level needed based on the path prefix
  2. Adding pagination or limiting the tree depth
  3. Implementing client-side caching if this is called frequently
Suggested change
// Get the tree for the ref (recursive)
tree,_,err:=client.Git.GetTree(ctx,owner,repo,refVal,true)
iferr!=nil||tree==nil {
returnnil,errors.New("failed to get file tree")
}
// Collect immediate children of the prefix (files and directories, no duplicates)
dirs:=map[string]struct{}{}
files:=map[string]struct{}{}
prefixLen:=len(prefix)
for_,entry:=range tree.Entries {
if!strings.HasPrefix(entry.GetPath(),prefix) {
continue
}
rel:=entry.GetPath()[prefixLen:]
ifrel=="" {
continue
}
// Only immediate children
slashIdx:=strings.Index(rel,"/")
ifslashIdx>=0 {
// Directory: only add the directory name (with trailing slash), prefixed with full path
dirName:=prefix+rel[:slashIdx+1]
dirs[dirName]=struct{}{}
}elseifentry.GetType()=="blob" {
// File: add as-is, prefixed with full path
fileName:=prefix+rel
files[fileName]=struct{}{}
// Get the contents of the directory at the prefix for the ref
contents,_,_,err:=client.Repositories.GetContents(ctx,owner,repo,prefix,&github.RepositoryContentGetOptions{Ref:refVal})
iferr!=nil {
returnnil,errors.New("failed to get directory contents")
}
// Collect immediate children of the prefix (files and directories, no duplicates)
dirs:=map[string]struct{}{}
files:=map[string]struct{}{}
for_,entry:=rangecontents {
ifentry.GetType()=="dir" {
// Directory: add with trailing slash
dirName:=entry.GetPath()
if!strings.HasSuffix(dirName,"/") {
dirName+="/"
}
dirs[dirName]=struct{}{}
}elseifentry.GetType()=="file" {
files[entry.GetPath()]=struct{}{}

Copilot uses AI. Check for mistakes.
}
}

// Optionally filter by argValue (if user is typing after last slash)
var filter string
if argValue != "" {
if lastSlash := strings.LastIndex(argValue, "/"); lastSlash >= 0 {
filter = argValue[lastSlash+1:]
} else {
filter = argValue
}
}

var values []string
// Add directories first, then files, both filtered
for dir := range dirs {
// Only filter on the last segment after the last slash
if filter == "" {
values = append(values, dir)
} else {
last := dir
if idx := strings.LastIndex(strings.TrimRight(dir, "/"), "/"); idx >= 0 {
last = dir[idx+1:]
}
if strings.HasPrefix(last, filter) {
values = append(values, dir)
}
}
}
for file := range files {
if filter == "" {
values = append(values, file)
} else {
last := file
if idx := strings.LastIndex(file, "/"); idx >= 0 {
last = file[idx+1:]
}
if strings.HasPrefix(last, filter) {
values = append(values, file)
}
}
}

if len(values) > 100 {
values = values[:100]
}
return values, nil
}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp