diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index aee6133c6..f4e67f264 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -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 } @@ -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, diff --git a/pkg/github/repository_resource_completions.go b/pkg/github/repository_resource_completions.go new file mode 100644 index 000000000..aeb2d88a6 --- /dev/null +++ b/pkg/github/repository_resource_completions.go @@ -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, + }, + }, 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) + + 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") + } + // 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) + + 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) + + 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) + 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") + } + + 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{}{} + } + } + + // 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 +} diff --git a/pkg/github/repository_resource_completions_test.go b/pkg/github/repository_resource_completions_test.go new file mode 100644 index 000000000..b6f83f321 --- /dev/null +++ b/pkg/github/repository_resource_completions_test.go @@ -0,0 +1,372 @@ +package github + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/google/go-github/v79/github" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRepositoryResourceCompletionHandler(t *testing.T) { + tests := []struct { + name string + request *mcp.CompleteRequest + expected *mcp.CompleteResult + wantErr bool + }{ + { + name: "non-resource completion request", + request: &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "something-else", + }, + }, + }, + expected: nil, + wantErr: false, + }, + { + name: "invalid ref type", + request: &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "invalid-ref", + }, + }, + }, + expected: nil, + wantErr: false, + }, + { + name: "unknown argument", + request: &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "ref/resource", + }, + Context: &mcp.CompleteContext{}, + Argument: mcp.CompleteParamsArgument{ + Name: "unknown_arg", + Value: "test", + }, + }, + }, + expected: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + getClient := func(_ context.Context) (*github.Client, error) { + return &github.Client{}, nil + } + + handler := RepositoryResourceCompletionHandler(getClient) + result, err := handler(t.Context(), tt.request) + + if tt.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestRepositoryResourceCompletionHandler_GetClientError(t *testing.T) { + getClient := func(_ context.Context) (*github.Client, error) { + return nil, errors.New("client error") + } + + handler := RepositoryResourceCompletionHandler(getClient) + request := &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "ref/resource", + }, + Context: &mcp.CompleteContext{ + Arguments: map[string]string{ + "owner": "test", + }, + }, + Argument: mcp.CompleteParamsArgument{ + Name: "owner", + Value: "test", + }, + }, + } + + result, err := handler(t.Context(), request) + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "client error") +} + +// Test the logical behavior of complete functions with missing dependencies +func TestCompleteRepo_MissingOwner(t *testing.T) { + ctx := t.Context() + resolved := map[string]string{} // No owner + argValue := "test" + + result, err := completeRepo(ctx, nil, resolved, argValue) + require.Error(t, err) + assert.Nil(t, result) // Should return nil slice when owner is missing +} + +func TestCompleteBranch_MissingDependencies(t *testing.T) { + ctx := t.Context() + + // Test missing owner + resolved := map[string]string{"repo": "testrepo"} + result, err := completeBranch(ctx, nil, resolved, "main") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing + + // Test missing repo + resolved = map[string]string{"owner": "testowner"} + result, err = completeBranch(ctx, nil, resolved, "main") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing +} + +func TestCompleteSHA_MissingDependencies(t *testing.T) { + ctx := t.Context() + + // Test missing owner + resolved := map[string]string{"repo": "testrepo"} + result, err := completeSHA(ctx, nil, resolved, "abc123") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing + + // Test missing repo + resolved = map[string]string{"owner": "testowner"} + result, err = completeSHA(ctx, nil, resolved, "abc123") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing +} + +func TestCompleteTag_MissingDependencies(t *testing.T) { + ctx := t.Context() + + // Test missing owner + resolved := map[string]string{"repo": "testrepo"} + result, err := completeTag(ctx, nil, resolved, "v1.0") + require.Error(t, err) + assert.Nil(t, result) // completeTag returns nil for missing dependencies + + // Test missing repo + resolved = map[string]string{"owner": "testowner"} + result, err = completeTag(ctx, nil, resolved, "v1.0") + require.Error(t, err) + assert.Nil(t, result) +} + +func TestCompletePRNumber_MissingDependencies(t *testing.T) { + ctx := t.Context() + + // Test missing owner + resolved := map[string]string{"repo": "testrepo"} + result, err := completePRNumber(ctx, nil, resolved, "1") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing + + // Test missing repo + resolved = map[string]string{"owner": "testowner"} + result, err = completePRNumber(ctx, nil, resolved, "1") + require.Error(t, err) + assert.Nil(t, result) // Returns nil slice when dependencies are missing +} + +func TestCompletePath_MissingDependencies(t *testing.T) { + ctx := t.Context() + + // Test missing owner + resolved := map[string]string{"repo": "testrepo"} + result, err := completePath(ctx, nil, resolved, "src/") + require.Error(t, err) + assert.Nil(t, result) // completePath returns nil for missing dependencies + + // Test missing repo + resolved = map[string]string{"owner": "testowner"} + result, err = completePath(ctx, nil, resolved, "src/") + require.Error(t, err) + assert.Nil(t, result) +} + +func TestCompletePath_RefSelection(t *testing.T) { + // Test the logic for selecting the ref (branch, sha, tag, or HEAD) + // We test this by verifying the function handles different ref combinations + // without making API calls (since we can't mock them easily) + + ctx := t.Context() + + // Test that the function returns nil when dependencies are missing + resolved := map[string]string{ + "owner": "", + "repo": "", + } + result, err := completePath(ctx, nil, resolved, "src/") + require.Error(t, err) + assert.Nil(t, result) + + // When owner is present but repo is missing + resolved = map[string]string{ + "owner": "testowner", + "repo": "", + } + result, err = completePath(ctx, nil, resolved, "src/") + require.Error(t, err) + assert.Nil(t, result) +} + +func TestRepositoryResourceArgumentResolvers_Existence(t *testing.T) { + // Test that all expected resolvers are present + expectedResolvers := []string{ + "owner", "repo", "branch", "sha", "tag", "prNumber", "path", + } + + for _, resolver := range expectedResolvers { + t.Run(fmt.Sprintf("resolver_%s_exists", resolver), func(t *testing.T) { + _, exists := RepositoryResourceArgumentResolvers[resolver] + assert.True(t, exists, "Resolver %s should exist", resolver) + }) + } + + // Verify the total count + assert.Len(t, RepositoryResourceArgumentResolvers, len(expectedResolvers)) +} + +func TestRepositoryResourceCompletionHandler_MaxResults(t *testing.T) { + // Test that results are limited to 100 items + getClient := func(_ context.Context) (*github.Client, error) { + return &github.Client{}, nil + } + + handler := RepositoryResourceCompletionHandler(getClient) + + // Mock a resolver that returns more than 100 results + originalResolver := RepositoryResourceArgumentResolvers["owner"] + RepositoryResourceArgumentResolvers["owner"] = func(_ context.Context, _ *github.Client, _ map[string]string, _ string) ([]string, error) { + // Return 150 results + results := make([]string, 150) + for i := 0; i < 150; i++ { + results[i] = fmt.Sprintf("user%d", i) + } + return results, nil + } + + request := &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "ref/resource", + }, + Context: &mcp.CompleteContext{ + Arguments: map[string]string{ + "owner": "test", + }, + }, + Argument: mcp.CompleteParamsArgument{ + Name: "owner", + Value: "test", + }, + }, + } + + result, err := handler(t.Context(), request) + require.NoError(t, err) + assert.NotNil(t, result) + assert.LessOrEqual(t, len(result.Completion.Values), 100) + + // Restore original resolver + RepositoryResourceArgumentResolvers["owner"] = originalResolver +} + +func TestRepositoryResourceCompletionHandler_WithContext(t *testing.T) { + // Test that the handler properly passes resolved context arguments + getClient := func(_ context.Context) (*github.Client, error) { + return &github.Client{}, nil + } + + handler := RepositoryResourceCompletionHandler(getClient) + + // Mock a resolver that just returns the resolved arguments for testing + originalResolver := RepositoryResourceArgumentResolvers["repo"] + RepositoryResourceArgumentResolvers["repo"] = func(_ context.Context, _ *github.Client, resolved map[string]string, _ string) ([]string, error) { + if owner, exists := resolved["owner"]; exists { + return []string{fmt.Sprintf("repo-for-%s", owner)}, nil + } + return []string{}, nil + } + + request := &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "ref/resource", + }, + Argument: mcp.CompleteParamsArgument{ + Name: "repo", + Value: "test", + }, + Context: &mcp.CompleteContext{ + Arguments: map[string]string{ + "owner": "testowner", + }, + }, + }, + } + + result, err := handler(t.Context(), request) + require.NoError(t, err) + assert.NotNil(t, result) + assert.Contains(t, result.Completion.Values, "repo-for-testowner") + + // Restore original resolver + RepositoryResourceArgumentResolvers["repo"] = originalResolver +} + +func TestRepositoryResourceCompletionHandler_NilContext(t *testing.T) { + // Test that the handler handles nil context gracefully + getClient := func(_ context.Context) (*github.Client, error) { + return &github.Client{}, nil + } + + handler := RepositoryResourceCompletionHandler(getClient) + + // Mock a resolver that checks for empty resolved map + originalResolver := RepositoryResourceArgumentResolvers["repo"] + RepositoryResourceArgumentResolvers["repo"] = func(_ context.Context, _ *github.Client, resolved map[string]string, _ string) ([]string, error) { + assert.NotNil(t, resolved, "Resolved map should never be nil") + return []string{"test-repo"}, nil + } + + request := &mcp.CompleteRequest{ + Params: &mcp.CompleteParams{ + Ref: &mcp.CompleteReference{ + Type: "ref/resource", + }, + Argument: mcp.CompleteParamsArgument{ + Name: "repo", + Value: "test", + }, + // Context is not set, so it should default to empty map + Context: &mcp.CompleteContext{ + Arguments: map[string]string{}, + }, + }, + } + + result, err := handler(t.Context(), request) + require.NoError(t, err) + assert.NotNil(t, result) + + // Restore original resolver + RepositoryResourceArgumentResolvers["repo"] = originalResolver +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 57e4a2623..e74596906 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -1,10 +1,12 @@ package github import ( + "context" "encoding/json" "errors" "fmt" "strconv" + "strings" "github.com/github/github-mcp-server/pkg/utils" "github.com/google/go-github/v79/github" @@ -34,6 +36,22 @@ func NewServer(version string, opts *mcp.ServerOptions) *mcp.Server { return s } +func CompletionsHandler(getClient GetClientFn) func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) { + return func(ctx context.Context, req *mcp.CompleteRequest) (*mcp.CompleteResult, error) { + switch req.Params.Ref.Type { + case "ref/resource": + if strings.HasPrefix(req.Params.Ref.URI, "repo://") { + return RepositoryResourceCompletionHandler(getClient)(ctx, req) + } + return nil, fmt.Errorf("unsupported resource URI: %s", req.Params.Ref.URI) + case "ref/prompt": + return nil, nil + default: + return nil, fmt.Errorf("unsupported ref type: %s", req.Params.Ref.Type) + } + } +} + // OptionalParamOK is a helper function that can be used to fetch a requested parameter from the request. // It returns the value, a boolean indicating if the parameter was present, and an error if the type is wrong. func OptionalParamOK[T any, A map[string]any](args A, p string) (value T, ok bool, err error) {