diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 998d87b2..04812330 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -@juruen @sammorrowdrums @williammartin @toby +* @github/github-mcp-server diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 4c370ebe..35ffc47d 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -66,6 +66,18 @@ jobs: uses: docker/metadata-action@96383f45573cb7f253c731d3b3ab81c87ef81934 # v5.0.0 with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + tags: | + type=schedule + type=ref,event=branch + type=ref,event=tag + type=ref,event=pr + type=semver,pattern={{version}} + type=semver,pattern={{major}}.{{minor}} + type=semver,pattern={{major}} + type=sha + type=edge + # Custom rule to prevent pre-releases from getting latest tag + type=raw,value=latest,enable=${{ github.ref_type == 'tag' && startsWith(github.ref, 'refs/tags/v') && !contains(github.ref, '-') }} - name: Go Build Cache for Docker uses: actions/cache@v4 diff --git a/.github/workflows/goreleaser.yml b/.github/workflows/goreleaser.yml index a25a3469..263607ee 100644 --- a/.github/workflows/goreleaser.yml +++ b/.github/workflows/goreleaser.yml @@ -5,6 +5,8 @@ on: - "v*" permissions: contents: write + id-token: write + attestations: write jobs: release: @@ -33,3 +35,11 @@ jobs: workdir: . env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Generate signed build provenance attestations for workflow artifacts + uses: actions/attest-build-provenance@v2 + with: + subject-path: | + dist/*.tar.gz + dist/*.zip + dist/*.txt diff --git a/.gitignore b/.gitignore index 9fb1dca9..9371be3e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,12 @@ .idea cmd/github-mcp-server/github-mcp-server + +# VSCode +.vscode/mcp.json + # Added by goreleaser init: dist/ -__debug_bin* \ No newline at end of file +__debug_bin* + +# Go +vendor diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7e176e70..fe307d1d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,7 +2,7 @@ [fork]: https://github.com/github/github-mcp-server/fork [pr]: https://github.com/github/github-mcp-server/compare -[style]: https://github.com/github/github-mcp-server/blob/main/.golangci.yaml +[style]: https://github.com/github/github-mcp-server/blob/main/.golangci.yml Hi there! We're thrilled that you'd like to contribute to this project. Your help is essential for keeping it great. diff --git a/README.md b/README.md index cf53ce2a..5493adba 100644 --- a/README.md +++ b/README.md @@ -15,11 +15,10 @@ automation and interaction capabilities for developers and tools. ## Prerequisites 1. To run the server in a container, you will need to have [Docker](https://www.docker.com/) installed. -2. [Create a GitHub Personal Access Token](https://github.com/settings/personal-access-tokens/new). +2. Once Docker is installed, you will also need to ensure Docker is running. +3. Lastly you will need to [Create a GitHub Personal Access Token](https://github.com/settings/personal-access-tokens/new). The MCP server can use many of the GitHub APIs, so enable the permissions that you feel comfortable granting your AI tools (to learn more about access tokens, please check out the [documentation](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens)). - - ## Installation ### Usage with VS Code @@ -90,10 +89,106 @@ More about using MCP server tools in VS Code's [agent mode documentation](https: ### Build from source -If you don't have Docker, you can use `go` to build the binary in the -`cmd/github-mcp-server` directory, and use the `github-mcp-server stdio` -command with the `GITHUB_PERSONAL_ACCESS_TOKEN` environment variable set to -your token. +If you don't have Docker, you can use `go build` to build the binary in the +`cmd/github-mcp-server` directory, and use the `github-mcp-server stdio` command with the `GITHUB_PERSONAL_ACCESS_TOKEN` environment variable set to your token. To specify the output location of the build, use the `-o` flag. You should configure your server to use the built executable as its `command`. For example: + +```JSON +{ + "mcp": { + "servers": { + "github": { + "command": "/path/to/github-mcp-server", + "args": ["stdio"], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "" + } + } + } + } +} +``` + +## Tool Configuration + +The GitHub MCP Server supports enabling or disabling specific groups of functionalities via the `--toolsets` flag. This allows you to control which GitHub API capabilities are available to your AI tools. Enabling only the toolsets that you need can help the LLM with tool choice and reduce the context size. + +### Available Toolsets + +The following sets of tools are available (all are on by default): + +| Toolset | Description | +| ----------------------- | ------------------------------------------------------------- | +| `repos` | Repository-related tools (file operations, branches, commits) | +| `issues` | Issue-related tools (create, read, update, comment) | +| `users` | Anything relating to GitHub Users | +| `pull_requests` | Pull request operations (create, merge, review) | +| `code_security` | Code scanning alerts and security features | +| `experiments` | Experimental features (not considered stable) | + +#### Specifying Toolsets + +To specify toolsets you want available to the LLM, you can pass an allow-list in two ways: + +1. **Using Command Line Argument**: + + ```bash + github-mcp-server --toolsets repos,issues,pull_requests,code_security + ``` + +2. **Using Environment Variable**: + ```bash + GITHUB_TOOLSETS="repos,issues,pull_requests,code_security" ./github-mcp-server + ``` + +The environment variable `GITHUB_TOOLSETS` takes precedence over the command line argument if both are provided. + +### Using Toolsets With Docker + +When using Docker, you can pass the toolsets as environment variables: + +```bash +docker run -i --rm \ + -e GITHUB_PERSONAL_ACCESS_TOKEN= \ + -e GITHUB_TOOLSETS="repos,issues,pull_requests,code_security,experiments" \ + ghcr.io/github/github-mcp-server +``` + +### The "all" Toolset + +The special toolset `all` can be provided to enable all available toolsets regardless of any other configuration: + +```bash +./github-mcp-server --toolsets all +``` + +Or using the environment variable: + +```bash +GITHUB_TOOLSETS="all" ./github-mcp-server +``` + +## Dynamic Tool Discovery + +**Note**: This feature is currently in beta and may not be available in all environments. Please test it out and let us know if you encounter any issues. + +Instead of starting with all tools enabled, you can turn on dynamic toolset discovery. Dynamic toolsets allow the MCP host to list and enable toolsets in response to a user prompt. This should help to avoid situations where the model gets confused by the shear number of tools available. + +### Using Dynamic Tool Discovery + +When using the binary, you can pass the `--dynamic-toolsets` flag. + +```bash +./github-mcp-server --dynamic-toolsets +``` + +When using Docker, you can pass the toolsets as environment variables: + +```bash +docker run -i --rm \ + -e GITHUB_PERSONAL_ACCESS_TOKEN= \ + -e GITHUB_DYNAMIC_TOOLSETS=1 \ + ghcr.io/github/github-mcp-server +``` ## GitHub Enterprise Server @@ -287,10 +382,35 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `draft`: Create as draft PR (boolean, optional) - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) +- **add_pull_request_review_comment** - Add a review comment to a pull request or reply to an existing comment + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `pull_number`: Pull request number (number, required) + - `body`: The text of the review comment (string, required) + - `commit_id`: The SHA of the commit to comment on (string, required unless using in_reply_to) + - `path`: The relative path to the file that necessitates a comment (string, required unless using in_reply_to) + - `line`: The line of the blob in the pull request diff that the comment applies to (number, optional) + - `side`: The side of the diff to comment on (LEFT or RIGHT) (string, optional) + - `start_line`: For multi-line comments, the first line of the range (number, optional) + - `start_side`: For multi-line comments, the starting side of the diff (LEFT or RIGHT) (string, optional) + - `subject_type`: The level at which the comment is targeted (line or file) (string, optional) + - `in_reply_to`: The ID of the review comment to reply to (number, optional). When specified, only body is required and other parameters are ignored. + +- **update_pull_request** - Update an existing pull request in a GitHub repository + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `pullNumber`: Pull request number to update (number, required) + - `title`: New title (string, optional) + - `body`: New description (string, optional) + - `state`: New state ('open' or 'closed') (string, optional) + - `base`: New base branch name (string, optional) + - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) + ### Repositories - **create_or_update_file** - Create or update a single file in a repository - - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `path`: File path (string, required) @@ -299,8 +419,13 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `branch`: Branch name (string, optional) - `sha`: File SHA if updating (string, optional) -- **push_files** - Push multiple files in a single commit +- **list_branches** - List branches in a GitHub repository + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `page`: Page number (number, optional) + - `perPage`: Results per page (number, optional) +- **push_files** - Push multiple files in a single commit - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `branch`: Branch to push to (string, required) @@ -308,7 +433,6 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `message`: Commit message (string, required) - **search_repositories** - Search for GitHub repositories - - `query`: Search query (string, required) - `sort`: Sort field (string, optional) - `order`: Sort order (string, optional) @@ -316,33 +440,29 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `perPage`: Results per page (number, optional) - **create_repository** - Create a new GitHub repository - - `name`: Repository name (string, required) - `description`: Repository description (string, optional) - `private`: Whether the repository is private (boolean, optional) - `autoInit`: Auto-initialize with README (boolean, optional) - **get_file_contents** - Get contents of a file or directory - - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `path`: File path (string, required) - `ref`: Git reference (string, optional) - **fork_repository** - Fork a repository - - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `organization`: Target organization name (string, optional) - **create_branch** - Create a new branch - - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `branch`: New branch name (string, required) - `sha`: SHA to create branch from (string, required) -- **list_commits** - Gets commits of a branch in a repository +- **list_commits** - Get a list of commits of a branch in a repository - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `sha`: Branch name, tag, or commit SHA (string, optional) @@ -350,16 +470,22 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `page`: Page number (number, optional) - `perPage`: Results per page (number, optional) -### Search - -- **search_code** - Search for code across GitHub repositories +- **get_commit** - Get details for a commit from a repository + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `sha`: Commit SHA, branch name, or tag name (string, required) + - `page`: Page number, for files in the commit (number, optional) + - `perPage`: Results per page, for files in the commit (number, optional) + - **search_code** - Search for code across GitHub repositories - `query`: Search query (string, required) - `sort`: Sort field (string, optional) - `order`: Sort order (string, optional) - `page`: Page number (number, optional) - `perPage`: Results per page (number, optional) +### Users + - **search_users** - Search for GitHub users - `query`: Search query (string, required) - `sort`: Sort field (string, optional) @@ -381,6 +507,22 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `ref`: Git reference (string, optional) - `state`: Alert state (string, optional) - `severity`: Alert severity (string, optional) + - `tool_name`: The name of the tool used for code scanning (string, optional) + +### Secret Scanning + +- **get_secret_scanning_alert** - Get a secret scanning alert + + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `alertNumber`: Alert number (number, required) + +- **list_secret_scanning_alerts** - List secret scanning alerts for a repository + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `state`: Alert state (string, optional) + - `secret_type`: The secret types to be filtered for in a comma-separated list (string, optional) + - `resolution`: The resolution status (string, optional) ## Resources @@ -435,6 +577,10 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `prNumber`: Pull request number (string, required) - `path`: File or directory path (string, optional) +## Library Usage + +The exported Go API of this module should currently be considered unstable, and subject to breaking changes. In the future, we may offer stability; please file an issue if there is a use case where this would be valuable. + ## License This project is licensed under the terms of the MIT open source license. Please refer to [MIT](./LICENSE) for the full terms. diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index dd4d41a7..5ca0e21c 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -1,9 +1,7 @@ package main import ( - "bytes" "context" - "encoding/json" "fmt" "io" stdlog "log" @@ -15,6 +13,7 @@ import ( iolog "github.com/github/github-mcp-server/pkg/log" "github.com/github/github-mcp-server/pkg/translations" gogithub "github.com/google/go-github/v69/github" + "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -30,7 +29,7 @@ var ( Use: "server", Short: "GitHub MCP Server", Long: `A GitHub MCP server that handles various tools and resources.`, - Version: fmt.Sprintf("%s (%s) %s", version, commit, date), + Version: fmt.Sprintf("Version: %s\nCommit: %s\nBuild Date: %s", version, commit, date), } stdioCmd = &cobra.Command{ @@ -41,18 +40,20 @@ var ( logFile := viper.GetString("log-file") readOnly := viper.GetBool("read-only") exportTranslations := viper.GetBool("export-translations") - prettyPrintJSON := viper.GetBool("pretty-print-json") logger, err := initLogger(logFile) if err != nil { stdlog.Fatal("Failed to initialize logger:", err) } + + enabledToolsets := viper.GetStringSlice("toolsets") + logCommands := viper.GetBool("enable-command-logging") cfg := runConfig{ readOnly: readOnly, logger: logger, logCommands: logCommands, exportTranslations: exportTranslations, - prettyPrintJSON: prettyPrintJSON, + enabledToolsets: enabledToolsets, } if err := runStdioServer(cfg); err != nil { stdlog.Fatal("failed to run stdio server:", err) @@ -64,21 +65,25 @@ var ( func init() { cobra.OnInitialize(initConfig) + rootCmd.SetVersionTemplate("{{.Short}}\n{{.Version}}\n") + // Add global flags that will be shared by all commands + rootCmd.PersistentFlags().StringSlice("toolsets", github.DefaultTools, "An optional comma separated list of groups of tools to allow, defaults to enabling all") + rootCmd.PersistentFlags().Bool("dynamic-toolsets", false, "Enable dynamic toolsets") rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations") rootCmd.PersistentFlags().String("log-file", "", "Path to log file") rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file") rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") - rootCmd.PersistentFlags().Bool("pretty-print-json", false, "Pretty print JSON output") // Bind flag to viper + _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) + _ = viper.BindPFlag("dynamic_toolsets", rootCmd.PersistentFlags().Lookup("dynamic-toolsets")) _ = viper.BindPFlag("read-only", rootCmd.PersistentFlags().Lookup("read-only")) _ = viper.BindPFlag("log-file", rootCmd.PersistentFlags().Lookup("log-file")) _ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging")) _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) - _ = viper.BindPFlag("gh-host", rootCmd.PersistentFlags().Lookup("gh-host")) - _ = viper.BindPFlag("pretty-print-json", rootCmd.PersistentFlags().Lookup("pretty-print-json")) + _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) // Add subcommands rootCmd.AddCommand(stdioCmd) @@ -86,7 +91,7 @@ func init() { func initConfig() { // Initialize Viper configuration - viper.SetEnvPrefix("APP") + viper.SetEnvPrefix("github") viper.AutomaticEnv() } @@ -112,20 +117,7 @@ type runConfig struct { logger *log.Logger logCommands bool exportTranslations bool - prettyPrintJSON bool -} - -// JSONPrettyPrintWriter is a Writer that pretty prints input to indented JSON -type JSONPrettyPrintWriter struct { - writer io.Writer -} - -func (j JSONPrettyPrintWriter) Write(p []byte) (n int, err error) { - var prettyJSON bytes.Buffer - if err := json.Indent(&prettyJSON, p, "", "\t"); err != nil { - return 0, err - } - return j.writer.Write(prettyJSON.Bytes()) + enabledToolsets []string } func runStdioServer(cfg runConfig) error { @@ -134,18 +126,14 @@ func runStdioServer(cfg runConfig) error { defer stop() // Create GH client - token := os.Getenv("GITHUB_PERSONAL_ACCESS_TOKEN") + token := viper.GetString("personal_access_token") if token == "" { cfg.logger.Fatal("GITHUB_PERSONAL_ACCESS_TOKEN not set") } ghClient := gogithub.NewClient(nil).WithAuthToken(token) ghClient.UserAgent = fmt.Sprintf("github-mcp-server/%s", version) - // Check GH_HOST env var first, then fall back to viper config - host := os.Getenv("GH_HOST") - if host == "" { - host = viper.GetString("gh-host") - } + host := viper.GetString("host") if host != "" { var err error @@ -157,8 +145,51 @@ func runStdioServer(cfg runConfig) error { t, dumpTranslations := translations.TranslationHelper() - // Create - ghServer := github.NewServer(ghClient, version, cfg.readOnly, t) + beforeInit := func(_ context.Context, _ any, message *mcp.InitializeRequest) { + ghClient.UserAgent = fmt.Sprintf("github-mcp-server/%s (%s/%s)", version, message.Params.ClientInfo.Name, message.Params.ClientInfo.Version) + } + + getClient := func(_ context.Context) (*gogithub.Client, error) { + return ghClient, nil // closing over client + } + + hooks := &server.Hooks{ + OnBeforeInitialize: []server.OnBeforeInitializeFunc{beforeInit}, + } + // Create server + ghServer := github.NewServer(version, server.WithHooks(hooks)) + + enabled := cfg.enabledToolsets + dynamic := viper.GetBool("dynamic_toolsets") + if dynamic { + // filter "all" from the enabled toolsets + enabled = make([]string, 0, len(cfg.enabledToolsets)) + for _, toolset := range cfg.enabledToolsets { + if toolset != "all" { + enabled = append(enabled, toolset) + } + } + } + + // Create default toolsets + toolsets, err := github.InitToolsets(enabled, cfg.readOnly, getClient, t) + context := github.InitContextToolset(getClient, t) + + if err != nil { + stdlog.Fatal("Failed to initialize toolsets:", err) + } + + // Register resources with the server + github.RegisterResources(ghServer, getClient, t) + // Register the tools with the server + toolsets.RegisterTools(ghServer) + context.RegisterTools(ghServer) + + if dynamic { + dynamic := github.InitDynamicToolset(ghServer, toolsets, t) + dynamic.RegisterTools(ghServer) + } + stdioServer := server.NewStdioServer(ghServer) stdLogger := stdlog.New(cfg.logger.Writer(), "stdioserver", 0) @@ -179,9 +210,6 @@ func runStdioServer(cfg runConfig) error { in, out = loggedIO, loggedIO } - if cfg.prettyPrintJSON { - out = JSONPrettyPrintWriter{writer: out} - } errC <- stdioServer.Listen(ctx, in, out) }() diff --git a/cmd/mcpcurl/README.md b/cmd/mcpcurl/README.md index 95e1339a..0104a1b3 100644 --- a/cmd/mcpcurl/README.md +++ b/cmd/mcpcurl/README.md @@ -49,7 +49,7 @@ Available Commands: create_repository Create a new GitHub repository in your account fork_repository Fork a GitHub repository to your account or specified organization get_file_contents Get the contents of a file or directory from a GitHub repository - get_issue Get details of a specific issue in a GitHub repository. + get_issue Get details of a specific issue in a GitHub repository get_issue_comments Get comments for a GitHub issue list_commits Get list of commits of a branch in a GitHub repository list_issues List issues in a GitHub repository with filtering options @@ -74,7 +74,7 @@ Get help for a specific tool: ```bash % ./mcpcurl --stdio-server-cmd "docker run -i --rm -e GITHUB_PERSONAL_ACCESS_TOKEN mcp/github" tools get_issue --help -Get details of a specific issue in a GitHub repository. +Get details of a specific issue in a GitHub repository Usage: mcpcurl tools get_issue [flags] diff --git a/cmd/mcpcurl/main.go b/cmd/mcpcurl/main.go index b6676bfe..dfc639b9 100644 --- a/cmd/mcpcurl/main.go +++ b/cmd/mcpcurl/main.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "crypto/rand" "encoding/json" "fmt" "io" @@ -11,8 +12,6 @@ import ( "slices" "strings" - "crypto/rand" - "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -161,7 +160,7 @@ func main() { _ = rootCmd.MarkPersistentFlagRequired("stdio-server-cmd") // Add global flag for pretty printing - rootCmd.PersistentFlags().Bool("pretty", true, "Pretty print MCP response (only for JSON responses)") + rootCmd.PersistentFlags().Bool("pretty", true, "Pretty print MCP response (only for JSON or JSONL responses)") // Add the tools command to the root command rootCmd.AddCommand(toolsCmd) @@ -426,15 +425,26 @@ func printResponse(response string, prettyPrint bool) error { // Extract text from content items of type "text" for _, content := range resp.Result.Content { if content.Type == "text" { - // Unmarshal the text content - var textContent map[string]interface{} - if err := json.Unmarshal([]byte(content.Text), &textContent); err != nil { - return fmt.Errorf("failed to parse text content: %w", err) + var textContentObj map[string]interface{} + err := json.Unmarshal([]byte(content.Text), &textContentObj) + + if err == nil { + prettyText, err := json.MarshalIndent(textContentObj, "", " ") + if err != nil { + return fmt.Errorf("failed to pretty print text content: %w", err) + } + fmt.Println(string(prettyText)) + continue + } + + // Fallback parsing as JSONL + var textContentList []map[string]interface{} + if err := json.Unmarshal([]byte(content.Text), &textContentList); err != nil { + return fmt.Errorf("failed to parse text content as a list: %w", err) } - // Pretty print the text content - prettyText, err := json.MarshalIndent(textContent, "", " ") + prettyText, err := json.MarshalIndent(textContentList, "", " ") if err != nil { - return fmt.Errorf("failed to pretty print text content: %w", err) + return fmt.Errorf("failed to pretty print array content: %w", err) } fmt.Println(string(prettyText)) } diff --git a/go.mod b/go.mod index 858690cd..7c09fba9 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/docker/docker v28.0.4+incompatible github.com/google/go-cmp v0.7.0 github.com/google/go-github/v69 v69.2.0 - github.com/mark3labs/mcp-go v0.18.0 + github.com/mark3labs/mcp-go v0.20.1 github.com/migueleliasweb/go-github-mock v1.1.0 github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.9.1 diff --git a/go.sum b/go.sum index 19d368de..3378b4fd 100644 --- a/go.sum +++ b/go.sum @@ -57,8 +57,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/mark3labs/mcp-go v0.18.0 h1:YuhgIVjNlTG2ZOwmrkORWyPTp0dz1opPEqvsPtySXao= -github.com/mark3labs/mcp-go v0.18.0/go.mod h1:KmJndYv7GIgcPVwEKJjNcbhVQ+hJGJhrCCB/9xITzpE= +github.com/mark3labs/mcp-go v0.20.1 h1:E1Bbx9K8d8kQmDZ1QHblM38c7UU2evQ2LlkANk1U/zw= +github.com/mark3labs/mcp-go v0.20.1/go.mod h1:KmJndYv7GIgcPVwEKJjNcbhVQ+hJGJhrCCB/9xITzpE= github.com/migueleliasweb/go-github-mock v1.1.0 h1:GKaOBPsrPGkAKgtfuWY8MclS1xR6MInkx1SexJucMwE= github.com/migueleliasweb/go-github-mock v1.1.0/go.mod h1:pYe/XlGs4BGMfRY4vmeixVsODHnVDDhJ9zoi0qzSMHc= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= diff --git a/pkg/github/code_scanning.go b/pkg/github/code_scanning.go index 81ee2c31..b33f32c1 100644 --- a/pkg/github/code_scanning.go +++ b/pkg/github/code_scanning.go @@ -13,7 +13,7 @@ import ( "github.com/mark3labs/mcp-go/server" ) -func getCodeScanningAlert(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func GetCodeScanningAlert(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_code_scanning_alert", mcp.WithDescription(t("TOOL_GET_CODE_SCANNING_ALERT_DESCRIPTION", "Get details of a specific code scanning alert in a GitHub repository.")), mcp.WithString("owner", @@ -38,11 +38,16 @@ func getCodeScanningAlert(client *github.Client, t translations.TranslationHelpe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - alertNumber, err := requiredInt(request, "alertNumber") + alertNumber, err := RequiredInt(request, "alertNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + alert, resp, err := client.CodeScanning.GetAlert(ctx, owner, repo, int64(alertNumber)) if err != nil { return nil, fmt.Errorf("failed to get alert: %w", err) @@ -66,7 +71,7 @@ func getCodeScanningAlert(client *github.Client, t translations.TranslationHelpe } } -func listCodeScanningAlerts(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func ListCodeScanningAlerts(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_code_scanning_alerts", mcp.WithDescription(t("TOOL_LIST_CODE_SCANNING_ALERTS_DESCRIPTION", "List code scanning alerts in a GitHub repository.")), mcp.WithString("owner", @@ -81,11 +86,16 @@ func listCodeScanningAlerts(client *github.Client, t translations.TranslationHel mcp.Description("The Git reference for the results you want to list."), ), mcp.WithString("state", - mcp.Description("State of the code scanning alerts to list. Set to closed to list only closed code scanning alerts. Default: open"), + mcp.Description("Filter code scanning alerts by state. Defaults to open"), mcp.DefaultString("open"), + mcp.Enum("open", "closed", "dismissed", "fixed"), ), mcp.WithString("severity", - mcp.Description("Only code scanning alerts with this severity will be returned. Possible values are: critical, high, medium, low, warning, note, error."), + mcp.Description("Filter code scanning alerts by severity"), + mcp.Enum("critical", "high", "medium", "low", "warning", "note", "error"), + ), + mcp.WithString("tool_name", + mcp.Description("The name of the tool used for code scanning."), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -97,20 +107,28 @@ func listCodeScanningAlerts(client *github.Client, t translations.TranslationHel if err != nil { return mcp.NewToolResultError(err.Error()), nil } - ref, err := optionalParam[string](request, "ref") + ref, err := OptionalParam[string](request, "ref") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - state, err := optionalParam[string](request, "state") + state, err := OptionalParam[string](request, "state") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - severity, err := optionalParam[string](request, "severity") + severity, err := OptionalParam[string](request, "severity") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + toolName, err := OptionalParam[string](request, "tool_name") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - alerts, resp, err := client.CodeScanning.ListAlertsForRepo(ctx, owner, repo, &github.AlertListOptions{Ref: ref, State: state, Severity: severity}) + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + alerts, resp, err := client.CodeScanning.ListAlertsForRepo(ctx, owner, repo, &github.AlertListOptions{Ref: ref, State: state, Severity: severity, ToolName: toolName}) if err != nil { return nil, fmt.Errorf("failed to list alerts: %w", err) } diff --git a/pkg/github/code_scanning_test.go b/pkg/github/code_scanning_test.go index ec4d671e..40dabebd 100644 --- a/pkg/github/code_scanning_test.go +++ b/pkg/github/code_scanning_test.go @@ -16,7 +16,7 @@ import ( func Test_GetCodeScanningAlert(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getCodeScanningAlert(mockClient, translations.NullTranslationHelper) + tool, _ := GetCodeScanningAlert(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_code_scanning_alert", tool.Name) assert.NotEmpty(t, tool.Description) @@ -82,7 +82,7 @@ func Test_GetCodeScanningAlert(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getCodeScanningAlert(client, translations.NullTranslationHelper) + _, handler := GetCodeScanningAlert(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -118,7 +118,7 @@ func Test_GetCodeScanningAlert(t *testing.T) { func Test_ListCodeScanningAlerts(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := listCodeScanningAlerts(mockClient, translations.NullTranslationHelper) + tool, _ := ListCodeScanningAlerts(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "list_code_scanning_alerts", tool.Name) assert.NotEmpty(t, tool.Description) @@ -127,6 +127,7 @@ func Test_ListCodeScanningAlerts(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "ref") assert.Contains(t, tool.InputSchema.Properties, "state") assert.Contains(t, tool.InputSchema.Properties, "severity") + assert.Contains(t, tool.InputSchema.Properties, "tool_name") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) // Setup mock alerts for success case @@ -159,20 +160,22 @@ func Test_ListCodeScanningAlerts(t *testing.T) { mock.WithRequestMatchHandler( mock.GetReposCodeScanningAlertsByOwnerByRepo, expectQueryParams(t, map[string]string{ - "ref": "main", - "state": "open", - "severity": "high", + "ref": "main", + "state": "open", + "severity": "high", + "tool_name": "codeql", }).andThen( mockResponse(t, http.StatusOK, mockAlerts), ), ), ), requestArgs: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "ref": "main", - "state": "open", - "severity": "high", + "owner": "owner", + "repo": "repo", + "ref": "main", + "state": "open", + "severity": "high", + "tool_name": "codeql", }, expectError: false, expectedAlerts: mockAlerts, @@ -201,7 +204,7 @@ func Test_ListCodeScanningAlerts(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := listCodeScanningAlerts(client, translations.NullTranslationHelper) + _, handler := ListCodeScanningAlerts(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go new file mode 100644 index 00000000..1c91d703 --- /dev/null +++ b/pkg/github/context_tools.go @@ -0,0 +1,49 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// GetMe creates a tool to get details of the authenticated user. +func GetMe(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_me", + mcp.WithDescription(t("TOOL_GET_ME_DESCRIPTION", "Get details of the authenticated GitHub user. Use this when a request include \"me\", \"my\"...")), + mcp.WithString("reason", + mcp.Description("Optional: reason the session was created"), + ), + ), + func(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + user, resp, err := client.Users.Get(ctx, "") + if err != nil { + return nil, fmt.Errorf("failed to get user: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get user: %s", string(body))), nil + } + + r, err := json.Marshal(user) + if err != nil { + return nil, fmt.Errorf("failed to marshal user: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go new file mode 100644 index 00000000..c9d220dd --- /dev/null +++ b/pkg/github/context_tools_test.go @@ -0,0 +1,132 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GetMe(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := GetMe(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "get_me", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "reason") + assert.Empty(t, tool.InputSchema.Required) // No required parameters + + // Setup mock user response + mockUser := &github.User{ + Login: github.Ptr("testuser"), + Name: github.Ptr("Test User"), + Email: github.Ptr("test@example.com"), + Bio: github.Ptr("GitHub user for testing"), + Company: github.Ptr("Test Company"), + Location: github.Ptr("Test Location"), + HTMLURL: github.Ptr("https://github.com/testuser"), + CreatedAt: &github.Timestamp{Time: time.Now().Add(-365 * 24 * time.Hour)}, + Type: github.Ptr("User"), + Plan: &github.Plan{ + Name: github.Ptr("pro"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedUser *github.User + expectedErrMsg string + }{ + { + name: "successful get user", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUser, + mockUser, + ), + ), + requestArgs: map[string]interface{}{}, + expectError: false, + expectedUser: mockUser, + }, + { + name: "successful get user with reason", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUser, + mockUser, + ), + ), + requestArgs: map[string]interface{}{ + "reason": "Testing API", + }, + expectError: false, + expectedUser: mockUser, + }, + { + name: "get user fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUser, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Unauthorized"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{}, + expectError: true, + expectedErrMsg: "failed to get user", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := GetMe(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse result and get text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedUser github.User + err = json.Unmarshal([]byte(textContent.Text), &returnedUser) + require.NoError(t, err) + + // Verify user details + assert.Equal(t, *tc.expectedUser.Login, *returnedUser.Login) + assert.Equal(t, *tc.expectedUser.Name, *returnedUser.Name) + assert.Equal(t, *tc.expectedUser.Email, *returnedUser.Email) + assert.Equal(t, *tc.expectedUser.Bio, *returnedUser.Bio) + assert.Equal(t, *tc.expectedUser.HTMLURL, *returnedUser.HTMLURL) + assert.Equal(t, *tc.expectedUser.Type, *returnedUser.Type) + }) + } +} diff --git a/pkg/github/dynamic_tools.go b/pkg/github/dynamic_tools.go new file mode 100644 index 00000000..d4d5f27a --- /dev/null +++ b/pkg/github/dynamic_tools.go @@ -0,0 +1,125 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/github/github-mcp-server/pkg/toolsets" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +func ToolsetEnum(toolsetGroup *toolsets.ToolsetGroup) mcp.PropertyOption { + toolsetNames := make([]string, 0, len(toolsetGroup.Toolsets)) + for name := range toolsetGroup.Toolsets { + toolsetNames = append(toolsetNames, name) + } + return mcp.Enum(toolsetNames...) +} + +func EnableToolset(s *server.MCPServer, toolsetGroup *toolsets.ToolsetGroup, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("enable_toolset", + mcp.WithDescription(t("TOOL_ENABLE_TOOLSET_DESCRIPTION", "Enable one of the sets of tools the GitHub MCP server provides, use get_toolset_tools and list_available_toolsets first to see what this will enable")), + mcp.WithString("toolset", + mcp.Required(), + mcp.Description("The name of the toolset to enable"), + ToolsetEnum(toolsetGroup), + ), + ), + func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // We need to convert the toolsets back to a map for JSON serialization + toolsetName, err := requiredParam[string](request, "toolset") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + toolset := toolsetGroup.Toolsets[toolsetName] + if toolset == nil { + return mcp.NewToolResultError(fmt.Sprintf("Toolset %s not found", toolsetName)), nil + } + if toolset.Enabled { + return mcp.NewToolResultText(fmt.Sprintf("Toolset %s is already enabled", toolsetName)), nil + } + + toolset.Enabled = true + + // caution: this currently affects the global tools and notifies all clients: + // + // Send notification to all initialized sessions + // s.sendNotificationToAllClients("notifications/tools/list_changed", nil) + s.AddTools(toolset.GetActiveTools()...) + + return mcp.NewToolResultText(fmt.Sprintf("Toolset %s enabled", toolsetName)), nil + } +} + +func ListAvailableToolsets(toolsetGroup *toolsets.ToolsetGroup, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_available_toolsets", + mcp.WithDescription(t("TOOL_LIST_AVAILABLE_TOOLSETS_DESCRIPTION", "List all available toolsets this GitHub MCP server can offer, providing the enabled status of each. Use this when a task could be achieved with a GitHub tool and the currently available tools aren't enough. Call get_toolset_tools with these toolset names to discover specific tools you can call")), + ), + func(_ context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // We need to convert the toolsetGroup back to a map for JSON serialization + + payload := []map[string]string{} + + for name, ts := range toolsetGroup.Toolsets { + { + t := map[string]string{ + "name": name, + "description": ts.Description, + "can_enable": "true", + "currently_enabled": fmt.Sprintf("%t", ts.Enabled), + } + payload = append(payload, t) + } + } + + r, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("failed to marshal features: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +func GetToolsetsTools(toolsetGroup *toolsets.ToolsetGroup, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_toolset_tools", + mcp.WithDescription(t("TOOL_GET_TOOLSET_TOOLS_DESCRIPTION", "Lists all the capabilities that are enabled with the specified toolset, use this to get clarity on whether enabling a toolset would help you to complete a task")), + mcp.WithString("toolset", + mcp.Required(), + mcp.Description("The name of the toolset you want to get the tools for"), + ToolsetEnum(toolsetGroup), + ), + ), + func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // We need to convert the toolsetGroup back to a map for JSON serialization + toolsetName, err := requiredParam[string](request, "toolset") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + toolset := toolsetGroup.Toolsets[toolsetName] + if toolset == nil { + return mcp.NewToolResultError(fmt.Sprintf("Toolset %s not found", toolsetName)), nil + } + payload := []map[string]string{} + + for _, st := range toolset.GetAvailableTools() { + tool := map[string]string{ + "name": st.Tool.Name, + "description": st.Tool.Description, + "can_enable": "true", + "toolset": toolsetName, + } + payload = append(payload, tool) + } + + r, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("failed to marshal features: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 9dcffa42..40fc0b94 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -93,3 +93,115 @@ func getTextResult(t *testing.T, result *mcp.CallToolResult) mcp.TextContent { assert.Equal(t, "text", textContent.Type) return textContent } + +func TestOptionalParamOK(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + paramName string + expectedVal interface{} + expectedOk bool + expectError bool + errorMsg string + }{ + { + name: "present and correct type (string)", + args: map[string]interface{}{"myParam": "hello"}, + paramName: "myParam", + expectedVal: "hello", + expectedOk: true, + expectError: false, + }, + { + name: "present and correct type (bool)", + args: map[string]interface{}{"myParam": true}, + paramName: "myParam", + expectedVal: true, + expectedOk: true, + expectError: false, + }, + { + name: "present and correct type (number)", + args: map[string]interface{}{"myParam": float64(123)}, + paramName: "myParam", + expectedVal: float64(123), + expectedOk: true, + expectError: false, + }, + { + name: "present but wrong type (string expected, got bool)", + args: map[string]interface{}{"myParam": true}, + paramName: "myParam", + expectedVal: "", // Zero value for string + expectedOk: true, // ok is true because param exists + expectError: true, + errorMsg: "parameter myParam is not of type string, is bool", + }, + { + name: "present but wrong type (bool expected, got string)", + args: map[string]interface{}{"myParam": "true"}, + paramName: "myParam", + expectedVal: false, // Zero value for bool + expectedOk: true, // ok is true because param exists + expectError: true, + errorMsg: "parameter myParam is not of type bool, is string", + }, + { + name: "parameter not present", + args: map[string]interface{}{"anotherParam": "value"}, + paramName: "myParam", + expectedVal: "", // Zero value for string + expectedOk: false, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + request := createMCPRequest(tc.args) + + // Test with string type assertion + if _, isString := tc.expectedVal.(string); isString || tc.errorMsg == "parameter myParam is not of type string, is bool" { + val, ok, err := OptionalParamOK[string](request, tc.paramName) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errorMsg) + assert.Equal(t, tc.expectedOk, ok) // Check ok even on error + assert.Equal(t, tc.expectedVal, val) // Check zero value on error + } else { + require.NoError(t, err) + assert.Equal(t, tc.expectedOk, ok) + assert.Equal(t, tc.expectedVal, val) + } + } + + // Test with bool type assertion + if _, isBool := tc.expectedVal.(bool); isBool || tc.errorMsg == "parameter myParam is not of type bool, is string" { + val, ok, err := OptionalParamOK[bool](request, tc.paramName) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errorMsg) + assert.Equal(t, tc.expectedOk, ok) // Check ok even on error + assert.Equal(t, tc.expectedVal, val) // Check zero value on error + } else { + require.NoError(t, err) + assert.Equal(t, tc.expectedOk, ok) + assert.Equal(t, tc.expectedVal, val) + } + } + + // Test with float64 type assertion (for number case) + if _, isFloat := tc.expectedVal.(float64); isFloat { + val, ok, err := OptionalParamOK[float64](request, tc.paramName) + if tc.expectError { + // This case shouldn't happen for float64 in the defined tests + require.Fail(t, "Unexpected error case for float64") + } else { + require.NoError(t, err) + assert.Equal(t, tc.expectedOk, ok) + assert.Equal(t, tc.expectedVal, val) + } + } + }) + } +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 1632e9e8..1324bd56 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -14,21 +14,21 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// getIssue creates a tool to get details of a specific issue in a GitHub repository. -func getIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetIssue creates a tool to get details of a specific issue in a GitHub repository. +func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_issue", - mcp.WithDescription(t("TOOL_GET_ISSUE_DESCRIPTION", "Get details of a specific issue in a GitHub repository.")), + mcp.WithDescription(t("TOOL_GET_ISSUE_DESCRIPTION", "Get details of a specific issue in a GitHub repository")), mcp.WithString("owner", mcp.Required(), - mcp.Description("The owner of the repository."), + mcp.Description("The owner of the repository"), ), mcp.WithString("repo", mcp.Required(), - mcp.Description("The name of the repository."), + mcp.Description("The name of the repository"), ), mcp.WithNumber("issue_number", mcp.Required(), - mcp.Description("The number of the issue."), + mcp.Description("The number of the issue"), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -40,11 +40,15 @@ func getIssue(client *github.Client, t translations.TranslationHelperFunc) (tool if err != nil { return mcp.NewToolResultError(err.Error()), nil } - issueNumber, err := requiredInt(request, "issue_number") + issueNumber, err := RequiredInt(request, "issue_number") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber) if err != nil { return nil, fmt.Errorf("failed to get issue: %w", err) @@ -68,8 +72,8 @@ func getIssue(client *github.Client, t translations.TranslationHelperFunc) (tool } } -// addIssueComment creates a tool to add a comment to an issue. -func addIssueComment(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// AddIssueComment creates a tool to add a comment to an issue. +func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("add_issue_comment", mcp.WithDescription(t("TOOL_ADD_ISSUE_COMMENT_DESCRIPTION", "Add a comment to an existing issue")), mcp.WithString("owner", @@ -86,7 +90,7 @@ func addIssueComment(client *github.Client, t translations.TranslationHelperFunc ), mcp.WithString("body", mcp.Required(), - mcp.Description("Comment text"), + mcp.Description("Comment content"), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -98,7 +102,7 @@ func addIssueComment(client *github.Client, t translations.TranslationHelperFunc if err != nil { return mcp.NewToolResultError(err.Error()), nil } - issueNumber, err := requiredInt(request, "issue_number") + issueNumber, err := RequiredInt(request, "issue_number") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -111,6 +115,10 @@ func addIssueComment(client *github.Client, t translations.TranslationHelperFunc Body: github.Ptr(body), } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } createdComment, resp, err := client.Issues.CreateComment(ctx, owner, repo, issueNumber, comment) if err != nil { return nil, fmt.Errorf("failed to create comment: %w", err) @@ -134,8 +142,8 @@ func addIssueComment(client *github.Client, t translations.TranslationHelperFunc } } -// searchIssues creates a tool to search for issues and pull requests. -func searchIssues(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// SearchIssues creates a tool to search for issues and pull requests. +func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", mcp.WithDescription(t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues and pull requests across GitHub repositories")), mcp.WithString("q", @@ -143,7 +151,7 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) ( mcp.Description("Search query using GitHub issues search syntax"), ), mcp.WithString("sort", - mcp.Description("Sort field (comments, reactions, created, etc.)"), + mcp.Description("Sort field by number of matches of categories, defaults to best match"), mcp.Enum( "comments", "reactions", @@ -159,25 +167,25 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) ( ), ), mcp.WithString("order", - mcp.Description("Sort order ('asc' or 'desc')"), + mcp.Description("Sort order"), mcp.Enum("asc", "desc"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { query, err := requiredParam[string](request, "q") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - sort, err := optionalParam[string](request, "sort") + sort, err := OptionalParam[string](request, "sort") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - order, err := optionalParam[string](request, "order") + order, err := OptionalParam[string](request, "order") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -191,6 +199,10 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) ( }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } result, resp, err := client.Search.Issues(ctx, query, opts) if err != nil { return nil, fmt.Errorf("failed to search issues: %w", err) @@ -214,8 +226,8 @@ func searchIssues(client *github.Client, t translations.TranslationHelperFunc) ( } } -// createIssue creates a tool to create a new issue in a GitHub repository. -func createIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// CreateIssue creates a tool to create a new issue in a GitHub repository. +func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_issue", mcp.WithDescription(t("TOOL_CREATE_ISSUE_DESCRIPTION", "Create a new issue in a GitHub repository")), mcp.WithString("owner", @@ -268,25 +280,25 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t } // Optional parameters - body, err := optionalParam[string](request, "body") + body, err := OptionalParam[string](request, "body") if err != nil { return mcp.NewToolResultError(err.Error()), nil } // Get assignees - assignees, err := optionalStringArrayParam(request, "assignees") + assignees, err := OptionalStringArrayParam(request, "assignees") if err != nil { return mcp.NewToolResultError(err.Error()), nil } // Get labels - labels, err := optionalStringArrayParam(request, "labels") + labels, err := OptionalStringArrayParam(request, "labels") if err != nil { return mcp.NewToolResultError(err.Error()), nil } // Get optional milestone - milestone, err := optionalIntParam(request, "milestone") + milestone, err := OptionalIntParam(request, "milestone") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -305,6 +317,10 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t Milestone: milestoneNum, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } issue, resp, err := client.Issues.Create(ctx, owner, repo, issueRequest) if err != nil { return nil, fmt.Errorf("failed to create issue: %w", err) @@ -328,8 +344,8 @@ func createIssue(client *github.Client, t translations.TranslationHelperFunc) (t } } -// listIssues creates a tool to list and filter repository issues -func listIssues(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// ListIssues creates a tool to list and filter repository issues +func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_issues", mcp.WithDescription(t("TOOL_LIST_ISSUES_DESCRIPTION", "List issues in a GitHub repository with filtering options")), mcp.WithString("owner", @@ -341,7 +357,7 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to mcp.Description("Repository name"), ), mcp.WithString("state", - mcp.Description("Filter by state ('open', 'closed', 'all')"), + mcp.Description("Filter by state"), mcp.Enum("open", "closed", "all"), ), mcp.WithArray("labels", @@ -353,17 +369,17 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to ), ), mcp.WithString("sort", - mcp.Description("Sort by ('created', 'updated', 'comments')"), + mcp.Description("Sort order"), mcp.Enum("created", "updated", "comments"), ), mcp.WithString("direction", - mcp.Description("Sort direction ('asc', 'desc')"), + mcp.Description("Sort direction"), mcp.Enum("asc", "desc"), ), mcp.WithString("since", mcp.Description("Filter by date (ISO 8601 timestamp)"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := requiredParam[string](request, "owner") @@ -378,28 +394,28 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to opts := &github.IssueListByRepoOptions{} // Set optional parameters if provided - opts.State, err = optionalParam[string](request, "state") + opts.State, err = OptionalParam[string](request, "state") if err != nil { return mcp.NewToolResultError(err.Error()), nil } // Get labels - opts.Labels, err = optionalStringArrayParam(request, "labels") + opts.Labels, err = OptionalStringArrayParam(request, "labels") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - opts.Sort, err = optionalParam[string](request, "sort") + opts.Sort, err = OptionalParam[string](request, "sort") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - opts.Direction, err = optionalParam[string](request, "direction") + opts.Direction, err = OptionalParam[string](request, "direction") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - since, err := optionalParam[string](request, "since") + since, err := OptionalParam[string](request, "since") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -419,6 +435,10 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to opts.PerPage = int(perPage) } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } issues, resp, err := client.Issues.ListByRepo(ctx, owner, repo, opts) if err != nil { return nil, fmt.Errorf("failed to list issues: %w", err) @@ -442,8 +462,8 @@ func listIssues(client *github.Client, t translations.TranslationHelperFunc) (to } } -// updateIssue creates a tool to update an existing issue in a GitHub repository. -func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// UpdateIssue creates a tool to update an existing issue in a GitHub repository. +func UpdateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("update_issue", mcp.WithDescription(t("TOOL_UPDATE_ISSUE_DESCRIPTION", "Update an existing issue in a GitHub repository")), mcp.WithString("owner", @@ -465,7 +485,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t mcp.Description("New description"), ), mcp.WithString("state", - mcp.Description("New state ('open' or 'closed')"), + mcp.Description("New state"), mcp.Enum("open", "closed"), ), mcp.WithArray("labels", @@ -497,7 +517,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } - issueNumber, err := requiredInt(request, "issue_number") + issueNumber, err := RequiredInt(request, "issue_number") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -506,7 +526,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t issueRequest := &github.IssueRequest{} // Set optional parameters if provided - title, err := optionalParam[string](request, "title") + title, err := OptionalParam[string](request, "title") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -514,7 +534,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t issueRequest.Title = github.Ptr(title) } - body, err := optionalParam[string](request, "body") + body, err := OptionalParam[string](request, "body") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -522,7 +542,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t issueRequest.Body = github.Ptr(body) } - state, err := optionalParam[string](request, "state") + state, err := OptionalParam[string](request, "state") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -531,7 +551,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t } // Get labels - labels, err := optionalStringArrayParam(request, "labels") + labels, err := OptionalStringArrayParam(request, "labels") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -540,7 +560,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t } // Get assignees - assignees, err := optionalStringArrayParam(request, "assignees") + assignees, err := OptionalStringArrayParam(request, "assignees") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -548,7 +568,7 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t issueRequest.Assignees = &assignees } - milestone, err := optionalIntParam(request, "milestone") + milestone, err := OptionalIntParam(request, "milestone") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -557,6 +577,10 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t issueRequest.Milestone = &milestoneNum } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest) if err != nil { return nil, fmt.Errorf("failed to update issue: %w", err) @@ -580,8 +604,8 @@ func updateIssue(client *github.Client, t translations.TranslationHelperFunc) (t } } -// getIssueComments creates a tool to get comments for a GitHub issue. -func getIssueComments(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetIssueComments creates a tool to get comments for a GitHub issue. +func GetIssueComments(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_issue_comments", mcp.WithDescription(t("TOOL_GET_ISSUE_COMMENTS_DESCRIPTION", "Get comments for a GitHub issue")), mcp.WithString("owner", @@ -612,15 +636,15 @@ func getIssueComments(client *github.Client, t translations.TranslationHelperFun if err != nil { return mcp.NewToolResultError(err.Error()), nil } - issueNumber, err := requiredInt(request, "issue_number") + issueNumber, err := RequiredInt(request, "issue_number") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - page, err := optionalIntParamWithDefault(request, "page", 1) + page, err := OptionalIntParamWithDefault(request, "page", 1) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - perPage, err := optionalIntParamWithDefault(request, "per_page", 30) + perPage, err := OptionalIntParamWithDefault(request, "per_page", 30) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -632,6 +656,10 @@ func getIssueComments(client *github.Client, t translations.TranslationHelperFun }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } comments, resp, err := client.Issues.ListComments(ctx, owner, repo, issueNumber, opts) if err != nil { return nil, fmt.Errorf("failed to get issue comments: %w", err) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 485169fd..61ca0ae7 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -18,7 +18,7 @@ import ( func Test_GetIssue(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getIssue(mockClient, translations.NullTranslationHelper) + tool, _ := GetIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_issue", tool.Name) assert.NotEmpty(t, tool.Description) @@ -82,7 +82,7 @@ func Test_GetIssue(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getIssue(client, translations.NullTranslationHelper) + _, handler := GetIssue(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -114,7 +114,7 @@ func Test_GetIssue(t *testing.T) { func Test_AddIssueComment(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := addIssueComment(mockClient, translations.NullTranslationHelper) + tool, _ := AddIssueComment(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "add_issue_comment", tool.Name) assert.NotEmpty(t, tool.Description) @@ -185,7 +185,7 @@ func Test_AddIssueComment(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := addIssueComment(client, translations.NullTranslationHelper) + _, handler := AddIssueComment(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := mcp.CallToolRequest{ @@ -237,7 +237,7 @@ func Test_AddIssueComment(t *testing.T) { func Test_SearchIssues(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := searchIssues(mockClient, translations.NullTranslationHelper) + tool, _ := SearchIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "search_issues", tool.Name) assert.NotEmpty(t, tool.Description) @@ -352,7 +352,7 @@ func Test_SearchIssues(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := searchIssues(client, translations.NullTranslationHelper) + _, handler := SearchIssues(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -393,7 +393,7 @@ func Test_SearchIssues(t *testing.T) { func Test_CreateIssue(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createIssue(mockClient, translations.NullTranslationHelper) + tool, _ := CreateIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_issue", tool.Name) assert.NotEmpty(t, tool.Description) @@ -468,9 +468,10 @@ func Test_CreateIssue(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "title": "Minimal Issue", + "owner": "owner", + "repo": "repo", + "title": "Minimal Issue", + "assignees": nil, // Expect no failure with nil optional value. }, expectError: false, expectedIssue: &github.Issue{ @@ -505,7 +506,7 @@ func Test_CreateIssue(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createIssue(client, translations.NullTranslationHelper) + _, handler := CreateIssue(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -566,7 +567,7 @@ func Test_CreateIssue(t *testing.T) { func Test_ListIssues(t *testing.T) { // Verify tool definition mockClient := github.NewClient(nil) - tool, _ := listIssues(mockClient, translations.NullTranslationHelper) + tool, _ := ListIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "list_issues", tool.Name) assert.NotEmpty(t, tool.Description) @@ -697,7 +698,7 @@ func Test_ListIssues(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := listIssues(client, translations.NullTranslationHelper) + _, handler := ListIssues(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -742,7 +743,7 @@ func Test_ListIssues(t *testing.T) { func Test_UpdateIssue(t *testing.T) { // Verify tool definition mockClient := github.NewClient(nil) - tool, _ := updateIssue(mockClient, translations.NullTranslationHelper) + tool, _ := UpdateIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "update_issue", tool.Name) assert.NotEmpty(t, tool.Description) @@ -881,7 +882,7 @@ func Test_UpdateIssue(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := updateIssue(client, translations.NullTranslationHelper) + _, handler := UpdateIssue(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -999,7 +1000,7 @@ func Test_ParseISOTimestamp(t *testing.T) { func Test_GetIssueComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getIssueComments(mockClient, translations.NullTranslationHelper) + tool, _ := GetIssueComments(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_issue_comments", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1099,7 +1100,7 @@ func Test_GetIssueComments(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getIssueComments(client, translations.NullTranslationHelper) + _, handler := GetIssueComments(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 25090cb7..b1584c0e 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -13,8 +13,8 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// getPullRequest creates a tool to get details of a specific pull request. -func getPullRequest(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetPullRequest creates a tool to get details of a specific pull request. +func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request", mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_DESCRIPTION", "Get details of a specific pull request")), mcp.WithString("owner", @@ -39,11 +39,15 @@ func getPullRequest(client *github.Client, t translations.TranslationHelperFunc) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { return nil, fmt.Errorf("failed to get pull request: %w", err) @@ -67,8 +71,125 @@ func getPullRequest(client *github.Client, t translations.TranslationHelperFunc) } } -// listPullRequests creates a tool to list and filter repository pull requests. -func listPullRequests(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// UpdatePullRequest creates a tool to update an existing pull request. +func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("update_pull_request", + mcp.WithDescription(t("TOOL_UPDATE_PULL_REQUEST_DESCRIPTION", "Update an existing pull request in a GitHub repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("pullNumber", + mcp.Required(), + mcp.Description("Pull request number to update"), + ), + mcp.WithString("title", + mcp.Description("New title"), + ), + mcp.WithString("body", + mcp.Description("New description"), + ), + mcp.WithString("state", + mcp.Description("New state"), + mcp.Enum("open", "closed"), + ), + mcp.WithString("base", + mcp.Description("New base branch name"), + ), + mcp.WithBoolean("maintainer_can_modify", + mcp.Description("Allow maintainer edits"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pullNumber, err := RequiredInt(request, "pullNumber") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Build the update struct only with provided fields + update := &github.PullRequest{} + updateNeeded := false + + if title, ok, err := OptionalParamOK[string](request, "title"); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } else if ok { + update.Title = github.Ptr(title) + updateNeeded = true + } + + if body, ok, err := OptionalParamOK[string](request, "body"); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } else if ok { + update.Body = github.Ptr(body) + updateNeeded = true + } + + if state, ok, err := OptionalParamOK[string](request, "state"); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } else if ok { + update.State = github.Ptr(state) + updateNeeded = true + } + + if base, ok, err := OptionalParamOK[string](request, "base"); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } else if ok { + update.Base = &github.PullRequestBranch{Ref: github.Ptr(base)} + updateNeeded = true + } + + if maintainerCanModify, ok, err := OptionalParamOK[bool](request, "maintainer_can_modify"); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } else if ok { + update.MaintainerCanModify = github.Ptr(maintainerCanModify) + updateNeeded = true + } + + if !updateNeeded { + return mcp.NewToolResultError("No update parameters provided."), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + pr, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, update) + if err != nil { + return nil, fmt.Errorf("failed to update pull request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to update pull request: %s", string(body))), nil + } + + r, err := json.Marshal(pr) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// ListPullRequests creates a tool to list and filter repository pull requests. +func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_pull_requests", mcp.WithDescription(t("TOOL_LIST_PULL_REQUESTS_DESCRIPTION", "List and filter repository pull requests")), mcp.WithString("owner", @@ -80,7 +201,8 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun mcp.Description("Repository name"), ), mcp.WithString("state", - mcp.Description("Filter by state ('open', 'closed', 'all')"), + mcp.Description("Filter by state"), + mcp.Enum("open", "closed", "all"), ), mcp.WithString("head", mcp.Description("Filter by head user/org and branch"), @@ -89,12 +211,14 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun mcp.Description("Filter by base branch"), ), mcp.WithString("sort", - mcp.Description("Sort by ('created', 'updated', 'popularity', 'long-running')"), + mcp.Description("Sort by"), + mcp.Enum("created", "updated", "popularity", "long-running"), ), mcp.WithString("direction", - mcp.Description("Sort direction ('asc', 'desc')"), + mcp.Description("Sort direction"), + mcp.Enum("asc", "desc"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := requiredParam[string](request, "owner") @@ -105,27 +229,27 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun if err != nil { return mcp.NewToolResultError(err.Error()), nil } - state, err := optionalParam[string](request, "state") + state, err := OptionalParam[string](request, "state") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - head, err := optionalParam[string](request, "head") + head, err := OptionalParam[string](request, "head") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - base, err := optionalParam[string](request, "base") + base, err := OptionalParam[string](request, "base") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - sort, err := optionalParam[string](request, "sort") + sort, err := OptionalParam[string](request, "sort") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - direction, err := optionalParam[string](request, "direction") + direction, err := OptionalParam[string](request, "direction") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -142,6 +266,10 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } prs, resp, err := client.PullRequests.List(ctx, owner, repo, opts) if err != nil { return nil, fmt.Errorf("failed to list pull requests: %w", err) @@ -165,8 +293,8 @@ func listPullRequests(client *github.Client, t translations.TranslationHelperFun } } -// mergePullRequest creates a tool to merge a pull request. -func mergePullRequest(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// MergePullRequest creates a tool to merge a pull request. +func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("merge_pull_request", mcp.WithDescription(t("TOOL_MERGE_PULL_REQUEST_DESCRIPTION", "Merge a pull request")), mcp.WithString("owner", @@ -188,7 +316,8 @@ func mergePullRequest(client *github.Client, t translations.TranslationHelperFun mcp.Description("Extra detail for merge commit"), ), mcp.WithString("merge_method", - mcp.Description("Merge method ('merge', 'squash', 'rebase')"), + mcp.Description("Merge method"), + mcp.Enum("merge", "squash", "rebase"), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -200,19 +329,19 @@ func mergePullRequest(client *github.Client, t translations.TranslationHelperFun if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - commitTitle, err := optionalParam[string](request, "commit_title") + commitTitle, err := OptionalParam[string](request, "commit_title") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - commitMessage, err := optionalParam[string](request, "commit_message") + commitMessage, err := OptionalParam[string](request, "commit_message") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - mergeMethod, err := optionalParam[string](request, "merge_method") + mergeMethod, err := OptionalParam[string](request, "merge_method") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -222,6 +351,10 @@ func mergePullRequest(client *github.Client, t translations.TranslationHelperFun MergeMethod: mergeMethod, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } result, resp, err := client.PullRequests.Merge(ctx, owner, repo, pullNumber, commitMessage, options) if err != nil { return nil, fmt.Errorf("failed to merge pull request: %w", err) @@ -245,8 +378,8 @@ func mergePullRequest(client *github.Client, t translations.TranslationHelperFun } } -// getPullRequestFiles creates a tool to get the list of files changed in a pull request. -func getPullRequestFiles(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetPullRequestFiles creates a tool to get the list of files changed in a pull request. +func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request_files", mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_FILES_DESCRIPTION", "Get the list of files changed in a pull request")), mcp.WithString("owner", @@ -271,11 +404,15 @@ func getPullRequestFiles(client *github.Client, t translations.TranslationHelper if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } opts := &github.ListOptions{} files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) if err != nil { @@ -300,8 +437,8 @@ func getPullRequestFiles(client *github.Client, t translations.TranslationHelper } } -// getPullRequestStatus creates a tool to get the combined status of all status checks for a pull request. -func getPullRequestStatus(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetPullRequestStatus creates a tool to get the combined status of all status checks for a pull request. +func GetPullRequestStatus(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request_status", mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_STATUS_DESCRIPTION", "Get the combined status of all status checks for a pull request")), mcp.WithString("owner", @@ -326,11 +463,15 @@ func getPullRequestStatus(client *github.Client, t translations.TranslationHelpe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } // First get the PR to find the head SHA + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } pr, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) if err != nil { return nil, fmt.Errorf("failed to get pull request: %w", err) @@ -369,8 +510,8 @@ func getPullRequestStatus(client *github.Client, t translations.TranslationHelpe } } -// updatePullRequestBranch creates a tool to update a pull request branch with the latest changes from the base branch. -func updatePullRequestBranch(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// UpdatePullRequestBranch creates a tool to update a pull request branch with the latest changes from the base branch. +func UpdatePullRequestBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("update_pull_request_branch", mcp.WithDescription(t("TOOL_UPDATE_PULL_REQUEST_BRANCH_DESCRIPTION", "Update a pull request branch with the latest changes from the base branch")), mcp.WithString("owner", @@ -398,11 +539,11 @@ func updatePullRequestBranch(client *github.Client, t translations.TranslationHe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - expectedHeadSHA, err := optionalParam[string](request, "expectedHeadSha") + expectedHeadSHA, err := OptionalParam[string](request, "expectedHeadSha") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -411,6 +552,10 @@ func updatePullRequestBranch(client *github.Client, t translations.TranslationHe opts.ExpectedHeadSHA = github.Ptr(expectedHeadSHA) } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } result, resp, err := client.PullRequests.UpdateBranch(ctx, owner, repo, pullNumber, opts) if err != nil { // Check if it's an acceptedError. An acceptedError indicates that the update is in progress, @@ -439,8 +584,8 @@ func updatePullRequestBranch(client *github.Client, t translations.TranslationHe } } -// getPullRequestComments creates a tool to get the review comments on a pull request. -func getPullRequestComments(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetPullRequestComments creates a tool to get the review comments on a pull request. +func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request_comments", mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_COMMENTS_DESCRIPTION", "Get the review comments on a pull request")), mcp.WithString("owner", @@ -465,7 +610,7 @@ func getPullRequestComments(client *github.Client, t translations.TranslationHel if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -476,6 +621,10 @@ func getPullRequestComments(client *github.Client, t translations.TranslationHel }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } comments, resp, err := client.PullRequests.ListComments(ctx, owner, repo, pullNumber, opts) if err != nil { return nil, fmt.Errorf("failed to get pull request comments: %w", err) @@ -499,8 +648,178 @@ func getPullRequestComments(client *github.Client, t translations.TranslationHel } } -// getPullRequestReviews creates a tool to get the reviews on a pull request. -func getPullRequestReviews(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// AddPullRequestReviewComment creates a tool to add a review comment to a pull request. +func AddPullRequestReviewComment(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("add_pull_request_review_comment", + mcp.WithDescription(t("TOOL_ADD_PULL_REQUEST_COMMENT_DESCRIPTION", "Add a review comment to a pull request")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("pull_number", + mcp.Required(), + mcp.Description("Pull request number"), + ), + mcp.WithString("body", + mcp.Required(), + mcp.Description("The text of the review comment"), + ), + mcp.WithString("commit_id", + mcp.Description("The SHA of the commit to comment on. Required unless in_reply_to is specified."), + ), + mcp.WithString("path", + mcp.Description("The relative path to the file that necessitates a comment. Required unless in_reply_to is specified."), + ), + mcp.WithString("subject_type", + mcp.Description("The level at which the comment is targeted"), + mcp.Enum("line", "file"), + ), + mcp.WithNumber("line", + mcp.Description("The line of the blob in the pull request diff that the comment applies to. For multi-line comments, the last line of the range"), + ), + mcp.WithString("side", + mcp.Description("The side of the diff to comment on"), + mcp.Enum("LEFT", "RIGHT"), + ), + mcp.WithNumber("start_line", + mcp.Description("For multi-line comments, the first line of the range that the comment applies to"), + ), + mcp.WithString("start_side", + mcp.Description("For multi-line comments, the starting side of the diff that the comment applies to"), + mcp.Enum("LEFT", "RIGHT"), + ), + mcp.WithNumber("in_reply_to", + mcp.Description("The ID of the review comment to reply to. When specified, only body is required and all other parameters are ignored"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pullNumber, err := RequiredInt(request, "pull_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + body, err := requiredParam[string](request, "body") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Check if this is a reply to an existing comment + if replyToFloat, ok := request.Params.Arguments["in_reply_to"].(float64); ok { + // Use the specialized method for reply comments due to inconsistency in underlying go-github library: https://github.com/google/go-github/pull/950 + commentID := int64(replyToFloat) + createdReply, resp, err := client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID) + if err != nil { + return nil, fmt.Errorf("failed to reply to pull request comment: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to reply to pull request comment: %s", string(respBody))), nil + } + + r, err := json.Marshal(createdReply) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } + + // This is a new comment, not a reply + // Verify required parameters for a new comment + commitID, err := requiredParam[string](request, "commit_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + path, err := requiredParam[string](request, "path") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + comment := &github.PullRequestComment{ + Body: github.Ptr(body), + CommitID: github.Ptr(commitID), + Path: github.Ptr(path), + } + + subjectType, err := OptionalParam[string](request, "subject_type") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + if subjectType != "file" { + line, lineExists := request.Params.Arguments["line"].(float64) + startLine, startLineExists := request.Params.Arguments["start_line"].(float64) + side, sideExists := request.Params.Arguments["side"].(string) + startSide, startSideExists := request.Params.Arguments["start_side"].(string) + + if !lineExists { + return mcp.NewToolResultError("line parameter is required unless using subject_type:file"), nil + } + + comment.Line = github.Ptr(int(line)) + if sideExists { + comment.Side = github.Ptr(side) + } + if startLineExists { + comment.StartLine = github.Ptr(int(startLine)) + } + if startSideExists { + comment.StartSide = github.Ptr(startSide) + } + + if startLineExists && !lineExists { + return mcp.NewToolResultError("if start_line is provided, line must also be provided"), nil + } + if startSideExists && !sideExists { + return mcp.NewToolResultError("if start_side is provided, side must also be provided"), nil + } + } + + createdComment, resp, err := client.PullRequests.CreateComment(ctx, owner, repo, pullNumber, comment) + if err != nil { + return nil, fmt.Errorf("failed to create pull request comment: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to create pull request comment: %s", string(respBody))), nil + } + + r, err := json.Marshal(createdComment) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// GetPullRequestReviews creates a tool to get the reviews on a pull request. +func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request_reviews", mcp.WithDescription(t("TOOL_GET_PULL_REQUEST_REVIEWS_DESCRIPTION", "Get the reviews on a pull request")), mcp.WithString("owner", @@ -525,11 +844,15 @@ func getPullRequestReviews(client *github.Client, t translations.TranslationHelp if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } reviews, resp, err := client.PullRequests.ListReviews(ctx, owner, repo, pullNumber, nil) if err != nil { return nil, fmt.Errorf("failed to get pull request reviews: %w", err) @@ -553,8 +876,8 @@ func getPullRequestReviews(client *github.Client, t translations.TranslationHelp } } -// createPullRequestReview creates a tool to submit a review on a pull request. -func createPullRequestReview(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// CreatePullRequestReview creates a tool to submit a review on a pull request. +func CreatePullRequestReview(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_pull_request_review", mcp.WithDescription(t("TOOL_CREATE_PULL_REQUEST_REVIEW_DESCRIPTION", "Create a review on a pull request")), mcp.WithString("owner", @@ -574,7 +897,8 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe ), mcp.WithString("event", mcp.Required(), - mcp.Description("Review action ('APPROVE', 'REQUEST_CHANGES', 'COMMENT')"), + mcp.Description("Review action to perform"), + mcp.Enum("APPROVE", "REQUEST_CHANGES", "COMMENT"), ), mcp.WithString("commitId", mcp.Description("SHA of commit to review"), @@ -584,30 +908,30 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe map[string]interface{}{ "type": "object", "additionalProperties": false, - "required": []string{"path", "body"}, + "required": []string{"path", "body", "position", "line", "side", "start_line", "start_side"}, "properties": map[string]interface{}{ "path": map[string]interface{}{ "type": "string", "description": "path to the file", }, "position": map[string]interface{}{ - "type": "number", + "type": []string{"number", "null"}, "description": "position of the comment in the diff", }, "line": map[string]interface{}{ - "type": "number", + "type": []string{"number", "null"}, "description": "line number in the file to comment on. For multi-line comments, the end of the line range", }, "side": map[string]interface{}{ - "type": "string", + "type": []string{"string", "null"}, "description": "The side of the diff on which the line resides. For multi-line comments, this is the side for the end of the line range. (LEFT or RIGHT)", }, "start_line": map[string]interface{}{ - "type": "number", + "type": []string{"number", "null"}, "description": "The first line of the range to which the comment refers. Required for multi-line comments.", }, "start_side": map[string]interface{}{ - "type": "string", + "type": []string{"string", "null"}, "description": "The side of the diff on which the start line resides for multi-line comments. (LEFT or RIGHT)", }, "body": map[string]interface{}{ @@ -629,7 +953,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pullNumber, err := requiredInt(request, "pullNumber") + pullNumber, err := RequiredInt(request, "pullNumber") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -644,7 +968,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe } // Add body if provided - body, err := optionalParam[string](request, "body") + body, err := OptionalParam[string](request, "body") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -653,7 +977,7 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe } // Add commit ID if provided - commitID, err := optionalParam[string](request, "commitId") + commitID, err := OptionalParam[string](request, "commitId") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -722,6 +1046,10 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe reviewRequest.Comments = comments } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } review, resp, err := client.PullRequests.CreateReview(ctx, owner, repo, pullNumber, reviewRequest) if err != nil { return nil, fmt.Errorf("failed to create pull request review: %w", err) @@ -745,8 +1073,8 @@ func createPullRequestReview(client *github.Client, t translations.TranslationHe } } -// createPullRequest creates a tool to create a new pull request. -func createPullRequest(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// CreatePullRequest creates a tool to create a new pull request. +func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_pull_request", mcp.WithDescription(t("TOOL_CREATE_PULL_REQUEST_DESCRIPTION", "Create a new pull request in a GitHub repository")), mcp.WithString("owner", @@ -801,17 +1129,17 @@ func createPullRequest(client *github.Client, t translations.TranslationHelperFu return mcp.NewToolResultError(err.Error()), nil } - body, err := optionalParam[string](request, "body") + body, err := OptionalParam[string](request, "body") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - draft, err := optionalParam[bool](request, "draft") + draft, err := OptionalParam[bool](request, "draft") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - maintainerCanModify, err := optionalParam[bool](request, "maintainer_can_modify") + maintainerCanModify, err := OptionalParam[bool](request, "maintainer_can_modify") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -829,6 +1157,10 @@ func createPullRequest(client *github.Client, t translations.TranslationHelperFu newPR.Draft = github.Ptr(draft) newPR.MaintainerCanModify = github.Ptr(maintainerCanModify) + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } pr, resp, err := client.PullRequests.Create(ctx, owner, repo, newPR) if err != nil { return nil, fmt.Errorf("failed to create pull request: %w", err) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index cf1afcdc..bb372624 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -17,7 +17,7 @@ import ( func Test_GetPullRequest(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getPullRequest(mockClient, translations.NullTranslationHelper) + tool, _ := GetPullRequest(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_pull_request", tool.Name) assert.NotEmpty(t, tool.Description) @@ -94,7 +94,7 @@ func Test_GetPullRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getPullRequest(client, translations.NullTranslationHelper) + _, handler := GetPullRequest(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -126,10 +126,192 @@ func Test_GetPullRequest(t *testing.T) { } } +func Test_UpdatePullRequest(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := UpdatePullRequest(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "update_pull_request", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "pullNumber") + assert.Contains(t, tool.InputSchema.Properties, "title") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.Contains(t, tool.InputSchema.Properties, "state") + assert.Contains(t, tool.InputSchema.Properties, "base") + assert.Contains(t, tool.InputSchema.Properties, "maintainer_can_modify") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) + + // Setup mock PR for success case + mockUpdatedPR := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Updated Test PR Title"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"), + Body: github.Ptr("Updated test PR body."), + MaintainerCanModify: github.Ptr(false), + Base: &github.PullRequestBranch{ + Ref: github.Ptr("develop"), + }, + } + + mockClosedPR := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Test PR"), + State: github.Ptr("closed"), // State updated + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedPR *github.PullRequest + expectedErrMsg string + }{ + { + name: "successful PR update (title, body, base, maintainer_can_modify)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsByOwnerByRepoByPullNumber, + // Expect the flat string based on previous test failure output and API docs + expectRequestBody(t, map[string]interface{}{ + "title": "Updated Test PR Title", + "body": "Updated test PR body.", + "base": "develop", + "maintainer_can_modify": false, + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedPR), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "title": "Updated Test PR Title", + "body": "Updated test PR body.", + "base": "develop", + "maintainer_can_modify": false, + }, + expectError: false, + expectedPR: mockUpdatedPR, + }, + { + name: "successful PR update (state)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsByOwnerByRepoByPullNumber, + expectRequestBody(t, map[string]interface{}{ + "state": "closed", + }).andThen( + mockResponse(t, http.StatusOK, mockClosedPR), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "state": "closed", + }, + expectError: false, + expectedPR: mockClosedPR, + }, + { + name: "no update parameters provided", + mockedClient: mock.NewMockedHTTPClient(), // No API call expected + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + // No update fields + }, + expectError: false, // Error is returned in the result, not as Go error + expectedErrMsg: "No update parameters provided", + }, + { + name: "PR update fails (API error)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation Failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "title": "Invalid Title Causing Error", + }, + expectError: true, + expectedErrMsg: "failed to update pull request", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := UpdatePullRequest(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content + textContent := getTextResult(t, result) + + // Check for expected error message within the result text + if tc.expectedErrMsg != "" { + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + // Unmarshal and verify the successful result + var returnedPR github.PullRequest + err = json.Unmarshal([]byte(textContent.Text), &returnedPR) + require.NoError(t, err) + assert.Equal(t, *tc.expectedPR.Number, *returnedPR.Number) + if tc.expectedPR.Title != nil { + assert.Equal(t, *tc.expectedPR.Title, *returnedPR.Title) + } + if tc.expectedPR.Body != nil { + assert.Equal(t, *tc.expectedPR.Body, *returnedPR.Body) + } + if tc.expectedPR.State != nil { + assert.Equal(t, *tc.expectedPR.State, *returnedPR.State) + } + if tc.expectedPR.Base != nil && tc.expectedPR.Base.Ref != nil { + assert.NotNil(t, returnedPR.Base) + assert.Equal(t, *tc.expectedPR.Base.Ref, *returnedPR.Base.Ref) + } + if tc.expectedPR.MaintainerCanModify != nil { + assert.Equal(t, *tc.expectedPR.MaintainerCanModify, *returnedPR.MaintainerCanModify) + } + }) + } +} + func Test_ListPullRequests(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := listPullRequests(mockClient, translations.NullTranslationHelper) + tool, _ := ListPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "list_pull_requests", tool.Name) assert.NotEmpty(t, tool.Description) @@ -221,7 +403,7 @@ func Test_ListPullRequests(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := listPullRequests(client, translations.NullTranslationHelper) + _, handler := ListPullRequests(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -259,7 +441,7 @@ func Test_ListPullRequests(t *testing.T) { func Test_MergePullRequest(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := mergePullRequest(mockClient, translations.NullTranslationHelper) + tool, _ := MergePullRequest(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "merge_pull_request", tool.Name) assert.NotEmpty(t, tool.Description) @@ -336,7 +518,7 @@ func Test_MergePullRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := mergePullRequest(client, translations.NullTranslationHelper) + _, handler := MergePullRequest(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -370,7 +552,7 @@ func Test_MergePullRequest(t *testing.T) { func Test_GetPullRequestFiles(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getPullRequestFiles(mockClient, translations.NullTranslationHelper) + tool, _ := GetPullRequestFiles(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_pull_request_files", tool.Name) assert.NotEmpty(t, tool.Description) @@ -448,7 +630,7 @@ func Test_GetPullRequestFiles(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getPullRequestFiles(client, translations.NullTranslationHelper) + _, handler := GetPullRequestFiles(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -486,7 +668,7 @@ func Test_GetPullRequestFiles(t *testing.T) { func Test_GetPullRequestStatus(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getPullRequestStatus(mockClient, translations.NullTranslationHelper) + tool, _ := GetPullRequestStatus(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_pull_request_status", tool.Name) assert.NotEmpty(t, tool.Description) @@ -608,7 +790,7 @@ func Test_GetPullRequestStatus(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getPullRequestStatus(client, translations.NullTranslationHelper) + _, handler := GetPullRequestStatus(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -647,7 +829,7 @@ func Test_GetPullRequestStatus(t *testing.T) { func Test_UpdatePullRequestBranch(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := updatePullRequestBranch(mockClient, translations.NullTranslationHelper) + tool, _ := UpdatePullRequestBranch(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "update_pull_request_branch", tool.Name) assert.NotEmpty(t, tool.Description) @@ -735,7 +917,7 @@ func Test_UpdatePullRequestBranch(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := updatePullRequestBranch(client, translations.NullTranslationHelper) + _, handler := UpdatePullRequestBranch(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -763,7 +945,7 @@ func Test_UpdatePullRequestBranch(t *testing.T) { func Test_GetPullRequestComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getPullRequestComments(mockClient, translations.NullTranslationHelper) + tool, _ := GetPullRequestComments(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_pull_request_comments", tool.Name) assert.NotEmpty(t, tool.Description) @@ -851,7 +1033,7 @@ func Test_GetPullRequestComments(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getPullRequestComments(client, translations.NullTranslationHelper) + _, handler := GetPullRequestComments(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -890,7 +1072,7 @@ func Test_GetPullRequestComments(t *testing.T) { func Test_GetPullRequestReviews(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getPullRequestReviews(mockClient, translations.NullTranslationHelper) + tool, _ := GetPullRequestReviews(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_pull_request_reviews", tool.Name) assert.NotEmpty(t, tool.Description) @@ -974,7 +1156,7 @@ func Test_GetPullRequestReviews(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getPullRequestReviews(client, translations.NullTranslationHelper) + _, handler := GetPullRequestReviews(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1013,7 +1195,7 @@ func Test_GetPullRequestReviews(t *testing.T) { func Test_CreatePullRequestReview(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createPullRequestReview(mockClient, translations.NullTranslationHelper) + tool, _ := CreatePullRequestReview(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_pull_request_review", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1341,7 +1523,7 @@ func Test_CreatePullRequestReview(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createPullRequestReview(client, translations.NullTranslationHelper) + _, handler := CreatePullRequestReview(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1384,7 +1566,7 @@ func Test_CreatePullRequestReview(t *testing.T) { func Test_CreatePullRequest(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createPullRequest(mockClient, translations.NullTranslationHelper) + tool, _ := CreatePullRequest(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_pull_request", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1496,7 +1678,7 @@ func Test_CreatePullRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createPullRequest(client, translations.NullTranslationHelper) + _, handler := CreatePullRequest(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1537,3 +1719,200 @@ func Test_CreatePullRequest(t *testing.T) { }) } } + +func Test_AddPullRequestReviewComment(t *testing.T) { + mockClient := github.NewClient(nil) + tool, _ := AddPullRequestReviewComment(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "add_pull_request_review_comment", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "pull_number") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.Contains(t, tool.InputSchema.Properties, "commit_id") + assert.Contains(t, tool.InputSchema.Properties, "path") + // Since we've updated commit_id and path to be optional when using in_reply_to + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pull_number", "body"}) + + mockComment := &github.PullRequestComment{ + ID: github.Ptr(int64(123)), + Body: github.Ptr("Great stuff!"), + Path: github.Ptr("file1.txt"), + Line: github.Ptr(2), + Side: github.Ptr("RIGHT"), + } + + mockReply := &github.PullRequestComment{ + ID: github.Ptr(int64(456)), + Body: github.Ptr("Good point, will fix!"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedComment *github.PullRequestComment + expectedErrMsg string + }{ + { + name: "successful line comment creation", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsCommentsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + err := json.NewEncoder(w).Encode(mockComment) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(1), + "body": "Great stuff!", + "commit_id": "6dcb09b5b57875f334f61aebed695e2e4193db5e", + "path": "file1.txt", + "line": float64(2), + "side": "RIGHT", + }, + expectError: false, + expectedComment: mockComment, + }, + { + name: "successful reply using in_reply_to", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsCommentsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + err := json.NewEncoder(w).Encode(mockReply) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(1), + "body": "Good point, will fix!", + "in_reply_to": float64(123), + }, + expectError: false, + expectedComment: mockReply, + }, + { + name: "comment creation fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsCommentsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"message": "Validation Failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(1), + "body": "Great stuff!", + "commit_id": "6dcb09b5b57875f334f61aebed695e2e4193db5e", + "path": "file1.txt", + "line": float64(2), + }, + expectError: true, + expectedErrMsg: "failed to create pull request comment", + }, + { + name: "reply creation fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposPullsCommentsByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"message": "Comment not found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(1), + "body": "Good point, will fix!", + "in_reply_to": float64(999), + }, + expectError: true, + expectedErrMsg: "failed to reply to pull request comment", + }, + { + name: "missing required parameters for comment", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pull_number": float64(1), + "body": "Great stuff!", + // missing commit_id and path + }, + expectError: false, + expectedErrMsg: "missing required parameter: commit_id", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mockClient := github.NewClient(tc.mockedClient) + + _, handler := AddPullRequestReviewComment(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + result, err := handler(context.Background(), request) + + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + assert.NotNil(t, result) + require.Len(t, result.Content, 1) + + textContent := getTextResult(t, result) + if tc.expectedErrMsg != "" { + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + var returnedComment github.PullRequestComment + err = json.Unmarshal([]byte(getTextResult(t, result).Text), &returnedComment) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedComment.ID, *returnedComment.ID) + assert.Equal(t, *tc.expectedComment.Body, *returnedComment.Body) + + // Only check Path, Line, and Side if they exist in the expected comment + if tc.expectedComment.Path != nil { + assert.Equal(t, *tc.expectedComment.Path, *returnedComment.Path) + } + if tc.expectedComment.Line != nil { + assert.Equal(t, *tc.expectedComment.Line, *returnedComment.Line) + } + if tc.expectedComment.Side != nil { + assert.Equal(t, *tc.expectedComment.Side, *returnedComment.Side) + } + }) + } +} diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 5b8725d1..51948730 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -13,8 +13,75 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// listCommits creates a tool to get commits of a branch in a repository. -func listCommits(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func GetCommit(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_commit", + mcp.WithDescription(t("TOOL_GET_COMMITS_DESCRIPTION", "Get details for a commit from a GitHub repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithString("sha", + mcp.Required(), + mcp.Description("Commit SHA, branch name, or tag name"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + sha, err := requiredParam[string](request, "sha") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.ListOptions{ + Page: pagination.page, + PerPage: pagination.perPage, + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + commit, resp, err := client.Repositories.GetCommit(ctx, owner, repo, sha, opts) + if err != nil { + return nil, fmt.Errorf("failed to get commit: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get commit: %s", string(body))), nil + } + + r, err := json.Marshal(commit) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// ListCommits creates a tool to get commits of a branch in a repository. +func ListCommits(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_commits", mcp.WithDescription(t("TOOL_LIST_COMMITS_DESCRIPTION", "Get list of commits of a branch in a GitHub repository")), mcp.WithString("owner", @@ -26,9 +93,9 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t mcp.Description("Repository name"), ), mcp.WithString("sha", - mcp.Description("Branch name"), + mcp.Description("SHA or Branch name"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := requiredParam[string](request, "owner") @@ -39,11 +106,11 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } - sha, err := optionalParam[string](request, "sha") + sha, err := OptionalParam[string](request, "sha") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -56,6 +123,10 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } commits, resp, err := client.Repositories.ListCommits(ctx, owner, repo, opts) if err != nil { return nil, fmt.Errorf("failed to list commits: %w", err) @@ -79,8 +150,71 @@ func listCommits(client *github.Client, t translations.TranslationHelperFunc) (t } } -// createOrUpdateFile creates a tool to create or update a file in a GitHub repository. -func createOrUpdateFile(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// ListBranches creates a tool to list branches in a GitHub repository. +func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_branches", + mcp.WithDescription(t("TOOL_LIST_BRANCHES_DESCRIPTION", "List branches in a GitHub repository")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.BranchListOptions{ + ListOptions: github.ListOptions{ + Page: pagination.page, + PerPage: pagination.perPage, + }, + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + branches, resp, err := client.Repositories.ListBranches(ctx, owner, repo, opts) + if err != nil { + return nil, fmt.Errorf("failed to list branches: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to list branches: %s", string(body))), nil + } + + r, err := json.Marshal(branches) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// CreateOrUpdateFile creates a tool to create or update a file in a GitHub repository. +func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_or_update_file", mcp.WithDescription(t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION", "Create or update a single file in a GitHub repository")), mcp.WithString("owner", @@ -148,7 +282,7 @@ func createOrUpdateFile(client *github.Client, t translations.TranslationHelperF } // If SHA is provided, set it (for updates) - sha, err := optionalParam[string](request, "sha") + sha, err := OptionalParam[string](request, "sha") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -157,6 +291,10 @@ func createOrUpdateFile(client *github.Client, t translations.TranslationHelperF } // Create or update the file + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } fileContent, resp, err := client.Repositories.CreateFile(ctx, owner, repo, path, opts) if err != nil { return nil, fmt.Errorf("failed to create/update file: %w", err) @@ -180,8 +318,8 @@ func createOrUpdateFile(client *github.Client, t translations.TranslationHelperF } } -// createRepository creates a tool to create a new GitHub repository. -func createRepository(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// CreateRepository creates a tool to create a new GitHub repository. +func CreateRepository(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_repository", mcp.WithDescription(t("TOOL_CREATE_REPOSITORY_DESCRIPTION", "Create a new GitHub repository in your account")), mcp.WithString("name", @@ -203,15 +341,15 @@ func createRepository(client *github.Client, t translations.TranslationHelperFun if err != nil { return mcp.NewToolResultError(err.Error()), nil } - description, err := optionalParam[string](request, "description") + description, err := OptionalParam[string](request, "description") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - private, err := optionalParam[bool](request, "private") + private, err := OptionalParam[bool](request, "private") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - autoInit, err := optionalParam[bool](request, "autoInit") + autoInit, err := OptionalParam[bool](request, "autoInit") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -223,6 +361,10 @@ func createRepository(client *github.Client, t translations.TranslationHelperFun AutoInit: github.Ptr(autoInit), } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } createdRepo, resp, err := client.Repositories.Create(ctx, "", repo) if err != nil { return nil, fmt.Errorf("failed to create repository: %w", err) @@ -246,8 +388,8 @@ func createRepository(client *github.Client, t translations.TranslationHelperFun } } -// getFileContents creates a tool to get the contents of a file or directory from a GitHub repository. -func getFileContents(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository. +func GetFileContents(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_file_contents", mcp.WithDescription(t("TOOL_GET_FILE_CONTENTS_DESCRIPTION", "Get the contents of a file or directory from a GitHub repository")), mcp.WithString("owner", @@ -279,11 +421,15 @@ func getFileContents(client *github.Client, t translations.TranslationHelperFunc if err != nil { return mcp.NewToolResultError(err.Error()), nil } - branch, err := optionalParam[string](request, "branch") + branch, err := OptionalParam[string](request, "branch") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } opts := &github.RepositoryContentGetOptions{Ref: branch} fileContent, dirContent, resp, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) if err != nil { @@ -315,8 +461,8 @@ func getFileContents(client *github.Client, t translations.TranslationHelperFunc } } -// forkRepository creates a tool to fork a repository. -func forkRepository(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// ForkRepository creates a tool to fork a repository. +func ForkRepository(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("fork_repository", mcp.WithDescription(t("TOOL_FORK_REPOSITORY_DESCRIPTION", "Fork a GitHub repository to your account or specified organization")), mcp.WithString("owner", @@ -340,7 +486,7 @@ func forkRepository(client *github.Client, t translations.TranslationHelperFunc) if err != nil { return mcp.NewToolResultError(err.Error()), nil } - org, err := optionalParam[string](request, "organization") + org, err := OptionalParam[string](request, "organization") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -350,6 +496,10 @@ func forkRepository(client *github.Client, t translations.TranslationHelperFunc) opts.Organization = org } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } forkedRepo, resp, err := client.Repositories.CreateFork(ctx, owner, repo, opts) if err != nil { // Check if it's an acceptedError. An acceptedError indicates that the update is in progress, @@ -378,8 +528,8 @@ func forkRepository(client *github.Client, t translations.TranslationHelperFunc) } } -// createBranch creates a tool to create a new branch. -func createBranch(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// CreateBranch creates a tool to create a new branch. +func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("create_branch", mcp.WithDescription(t("TOOL_CREATE_BRANCH_DESCRIPTION", "Create a new branch in a GitHub repository")), mcp.WithString("owner", @@ -411,11 +561,16 @@ func createBranch(client *github.Client, t translations.TranslationHelperFunc) ( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - fromBranch, err := optionalParam[string](request, "from_branch") + fromBranch, err := OptionalParam[string](request, "from_branch") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + // Get the source branch SHA var ref *github.Reference @@ -458,8 +613,8 @@ func createBranch(client *github.Client, t translations.TranslationHelperFunc) ( } } -// pushFiles creates a tool to push multiple files in a single commit to a GitHub repository. -func pushFiles(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// PushFiles creates a tool to push multiple files in a single commit to a GitHub repository. +func PushFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("push_files", mcp.WithDescription(t("TOOL_PUSH_FILES_DESCRIPTION", "Push multiple files to a GitHub repository in a single commit")), mcp.WithString("owner", @@ -523,6 +678,11 @@ func pushFiles(client *github.Client, t translations.TranslationHelperFunc) (too return mcp.NewToolResultError("files parameter must be an array of objects with path and content"), nil } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + // Get the reference for the branch ref, resp, err := client.Git.GetRef(ctx, owner, repo, "refs/heads/"+branch) if err != nil { diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index f7ed8e71..5b8129fe 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -18,7 +18,7 @@ import ( func Test_GetFileContents(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := getFileContents(mockClient, translations.NullTranslationHelper) + tool, _ := GetFileContents(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "get_file_contents", tool.Name) assert.NotEmpty(t, tool.Description) @@ -132,7 +132,7 @@ func Test_GetFileContents(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := getFileContents(client, translations.NullTranslationHelper) + _, handler := GetFileContents(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := mcp.CallToolRequest{ @@ -189,7 +189,7 @@ func Test_GetFileContents(t *testing.T) { func Test_ForkRepository(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := forkRepository(mockClient, translations.NullTranslationHelper) + tool, _ := ForkRepository(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "fork_repository", tool.Name) assert.NotEmpty(t, tool.Description) @@ -259,7 +259,7 @@ func Test_ForkRepository(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := forkRepository(client, translations.NullTranslationHelper) + _, handler := ForkRepository(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -287,7 +287,7 @@ func Test_ForkRepository(t *testing.T) { func Test_CreateBranch(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createBranch(mockClient, translations.NullTranslationHelper) + tool, _ := CreateBranch(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_branch", tool.Name) assert.NotEmpty(t, tool.Description) @@ -445,7 +445,7 @@ func Test_CreateBranch(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createBranch(client, translations.NullTranslationHelper) + _, handler := CreateBranch(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -475,10 +475,135 @@ func Test_CreateBranch(t *testing.T) { } } +func Test_GetCommit(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := GetCommit(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "get_commit", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "sha") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "sha"}) + + mockCommit := &github.RepositoryCommit{ + SHA: github.Ptr("abc123def456"), + Commit: &github.Commit{ + Message: github.Ptr("First commit"), + Author: &github.CommitAuthor{ + Name: github.Ptr("Test User"), + Email: github.Ptr("test@example.com"), + Date: &github.Timestamp{Time: time.Now().Add(-48 * time.Hour)}, + }, + }, + Author: &github.User{ + Login: github.Ptr("testuser"), + }, + HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"), + Stats: &github.CommitStats{ + Additions: github.Ptr(10), + Deletions: github.Ptr(2), + Total: github.Ptr(12), + }, + Files: []*github.CommitFile{ + { + Filename: github.Ptr("file1.go"), + Status: github.Ptr("modified"), + Additions: github.Ptr(10), + Deletions: github.Ptr(2), + Changes: github.Ptr(12), + Patch: github.Ptr("@@ -1,2 +1,10 @@"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedCommit *github.RepositoryCommit + expectedErrMsg string + }{ + { + name: "successful commit fetch", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposCommitsByOwnerByRepoByRef, + mockResponse(t, http.StatusOK, mockCommit), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "sha": "abc123def456", + }, + expectError: false, + expectedCommit: mockCommit, + }, + { + name: "commit fetch fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposCommitsByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "sha": "nonexistent-sha", + }, + expectError: true, + expectedErrMsg: "failed to get commit", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := GetCommit(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedCommit github.RepositoryCommit + err = json.Unmarshal([]byte(textContent.Text), &returnedCommit) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedCommit.SHA, *returnedCommit.SHA) + assert.Equal(t, *tc.expectedCommit.Commit.Message, *returnedCommit.Commit.Message) + assert.Equal(t, *tc.expectedCommit.Author.Login, *returnedCommit.Author.Login) + assert.Equal(t, *tc.expectedCommit.HTMLURL, *returnedCommit.HTMLURL) + }) + } +} + func Test_ListCommits(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := listCommits(mockClient, translations.NullTranslationHelper) + tool, _ := ListCommits(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "list_commits", tool.Name) assert.NotEmpty(t, tool.Description) @@ -614,7 +739,7 @@ func Test_ListCommits(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := listCommits(client, translations.NullTranslationHelper) + _, handler := ListCommits(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -652,7 +777,7 @@ func Test_ListCommits(t *testing.T) { func Test_CreateOrUpdateFile(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createOrUpdateFile(mockClient, translations.NullTranslationHelper) + tool, _ := CreateOrUpdateFile(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_or_update_file", tool.Name) assert.NotEmpty(t, tool.Description) @@ -775,7 +900,7 @@ func Test_CreateOrUpdateFile(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createOrUpdateFile(client, translations.NullTranslationHelper) + _, handler := CreateOrUpdateFile(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -815,7 +940,7 @@ func Test_CreateOrUpdateFile(t *testing.T) { func Test_CreateRepository(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := createRepository(mockClient, translations.NullTranslationHelper) + tool, _ := CreateRepository(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "create_repository", tool.Name) assert.NotEmpty(t, tool.Description) @@ -923,7 +1048,7 @@ func Test_CreateRepository(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := createRepository(client, translations.NullTranslationHelper) + _, handler := CreateRepository(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -961,7 +1086,7 @@ func Test_CreateRepository(t *testing.T) { func Test_PushFiles(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := pushFiles(mockClient, translations.NullTranslationHelper) + tool, _ := PushFiles(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "push_files", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1256,7 +1381,7 @@ func Test_PushFiles(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := pushFiles(client, translations.NullTranslationHelper) + _, handler := PushFiles(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1293,3 +1418,113 @@ func Test_PushFiles(t *testing.T) { }) } } + +func Test_ListBranches(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListBranches(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_branches", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + // Setup mock branches for success case + mockBranches := []*github.Branch{ + { + Name: github.Ptr("main"), + Commit: &github.RepositoryCommit{SHA: github.Ptr("abc123")}, + }, + { + Name: github.Ptr("develop"), + Commit: &github.RepositoryCommit{SHA: github.Ptr("def456")}, + }, + } + + // Test cases + tests := []struct { + name string + args map[string]interface{} + mockResponses []mock.MockBackendOption + wantErr bool + errContains string + }{ + { + name: "success", + args: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "page": float64(2), + }, + mockResponses: []mock.MockBackendOption{ + mock.WithRequestMatch( + mock.GetReposBranchesByOwnerByRepo, + mockBranches, + ), + }, + wantErr: false, + }, + { + name: "missing owner", + args: map[string]interface{}{ + "repo": "repo", + }, + mockResponses: []mock.MockBackendOption{}, + wantErr: false, + errContains: "missing required parameter: owner", + }, + { + name: "missing repo", + args: map[string]interface{}{ + "owner": "owner", + }, + mockResponses: []mock.MockBackendOption{}, + wantErr: false, + errContains: "missing required parameter: repo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create mock client + mockClient := github.NewClient(mock.NewMockedHTTPClient(tt.mockResponses...)) + _, handler := ListBranches(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + // Create request + request := createMCPRequest(tt.args) + + // Call handler + result, err := handler(context.Background(), request) + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + require.NotNil(t, result) + + if tt.errContains != "" { + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tt.errContains) + return + } + + textContent := getTextResult(t, result) + require.NotEmpty(t, textContent.Text) + + // Verify response + var branches []*github.Branch + err = json.Unmarshal([]byte(textContent.Text), &branches) + require.NoError(t, err) + assert.Len(t, branches, 2) + assert.Equal(t, "main", *branches[0].Name) + assert.Equal(t, "develop", *branches[1].Name) + }) + } +} diff --git a/pkg/github/repository_resource.go b/pkg/github/repository_resource.go index 8b2ba7a7..949157f5 100644 --- a/pkg/github/repository_resource.go +++ b/pkg/github/repository_resource.go @@ -17,52 +17,53 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// getRepositoryResourceContent defines the resource template and handler for getting repository content. -func getRepositoryResourceContent(client *github.Client, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) { +// GetRepositoryResourceContent defines the resource template and handler for getting repository content. +func GetRepositoryResourceContent(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) { return mcp.NewResourceTemplate( "repo://{owner}/{repo}/contents{/path*}", // Resource template t("RESOURCE_REPOSITORY_CONTENT_DESCRIPTION", "Repository Content"), ), - repositoryResourceContentsHandler(client) + RepositoryResourceContentsHandler(getClient) } -// 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) { +// GetRepositoryResourceBranchContent defines the resource template and handler for getting repository content for a branch. +func GetRepositoryResourceBranchContent(getClient GetClientFn, 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"), ), - repositoryResourceContentsHandler(client) + RepositoryResourceContentsHandler(getClient) } -// 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) { +// GetRepositoryResourceCommitContent defines the resource template and handler for getting repository content for a commit. +func GetRepositoryResourceCommitContent(getClient GetClientFn, 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"), ), - repositoryResourceContentsHandler(client) + RepositoryResourceContentsHandler(getClient) } -// 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) { +// GetRepositoryResourceTagContent defines the resource template and handler for getting repository content for a tag. +func GetRepositoryResourceTagContent(getClient GetClientFn, 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"), ), - repositoryResourceContentsHandler(client) + RepositoryResourceContentsHandler(getClient) } -// 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) { +// GetRepositoryResourcePrContent defines the resource template and handler for getting repository content for a pull request. +func GetRepositoryResourcePrContent(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) { return mcp.NewResourceTemplate( "repo://{owner}/{repo}/refs/pull/{prNumber}/head/contents{/path*}", // Resource template t("RESOURCE_REPOSITORY_CONTENT_PR_DESCRIPTION", "Repository Content for specific pull request"), ), - repositoryResourceContentsHandler(client) + RepositoryResourceContentsHandler(getClient) } -func repositoryResourceContentsHandler(client *github.Client) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { +// RepositoryResourceContentsHandler returns a handler function for repository content requests. +func RepositoryResourceContentsHandler(getClient GetClientFn) 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 element // https://github.com/mark3labs/mcp-go/pull/54 @@ -106,6 +107,10 @@ func repositoryResourceContentsHandler(client *github.Client) func(ctx context.C opts.Ref = "refs/pull/" + prNumber[0] + "/head" } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } fileContent, directoryContent, _, err := client.Repositories.GetContents(ctx, owner, repo, path, opts) if err != nil { return nil, err diff --git a/pkg/github/repository_resource_test.go b/pkg/github/repository_resource_test.go index adad8744..ffd14be3 100644 --- a/pkg/github/repository_resource_test.go +++ b/pkg/github/repository_resource_test.go @@ -234,7 +234,7 @@ func Test_repositoryResourceContentsHandler(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { client := github.NewClient(tc.mockedClient) - handler := repositoryResourceContentsHandler(client) + handler := RepositoryResourceContentsHandler((stubGetClientFn(client))) request := mcp.ReadResourceRequest{ Params: struct { @@ -258,26 +258,26 @@ func Test_repositoryResourceContentsHandler(t *testing.T) { } } -func Test_getRepositoryResourceContent(t *testing.T) { - tmpl, _ := getRepositoryResourceContent(nil, translations.NullTranslationHelper) +func Test_GetRepositoryResourceContent(t *testing.T) { + tmpl, _ := GetRepositoryResourceContent(nil, translations.NullTranslationHelper) require.Equal(t, "repo://{owner}/{repo}/contents{/path*}", tmpl.URITemplate.Raw()) } -func Test_getRepositoryResourceBranchContent(t *testing.T) { - tmpl, _ := getRepositoryResourceBranchContent(nil, translations.NullTranslationHelper) +func Test_GetRepositoryResourceBranchContent(t *testing.T) { + tmpl, _ := GetRepositoryResourceBranchContent(nil, translations.NullTranslationHelper) require.Equal(t, "repo://{owner}/{repo}/refs/heads/{branch}/contents{/path*}", tmpl.URITemplate.Raw()) } -func Test_getRepositoryResourceCommitContent(t *testing.T) { - tmpl, _ := getRepositoryResourceCommitContent(nil, translations.NullTranslationHelper) +func Test_GetRepositoryResourceCommitContent(t *testing.T) { + tmpl, _ := GetRepositoryResourceCommitContent(nil, translations.NullTranslationHelper) require.Equal(t, "repo://{owner}/{repo}/sha/{sha}/contents{/path*}", tmpl.URITemplate.Raw()) } -func Test_getRepositoryResourceTagContent(t *testing.T) { - tmpl, _ := getRepositoryResourceTagContent(nil, translations.NullTranslationHelper) +func Test_GetRepositoryResourceTagContent(t *testing.T) { + tmpl, _ := GetRepositoryResourceTagContent(nil, translations.NullTranslationHelper) require.Equal(t, "repo://{owner}/{repo}/refs/tags/{tag}/contents{/path*}", tmpl.URITemplate.Raw()) } -func Test_getRepositoryResourcePrContent(t *testing.T) { - tmpl, _ := getRepositoryResourcePrContent(nil, translations.NullTranslationHelper) +func Test_GetRepositoryResourcePrContent(t *testing.T) { + tmpl, _ := GetRepositoryResourcePrContent(nil, translations.NullTranslationHelper) require.Equal(t, "repo://{owner}/{repo}/refs/pull/{prNumber}/head/contents{/path*}", tmpl.URITemplate.Raw()) } diff --git a/pkg/github/resources.go b/pkg/github/resources.go new file mode 100644 index 00000000..774261e9 --- /dev/null +++ b/pkg/github/resources.go @@ -0,0 +1,14 @@ +package github + +import ( + "github.com/github/github-mcp-server/pkg/translations" + "github.com/mark3labs/mcp-go/server" +) + +func RegisterResources(s *server.MCPServer, getClient GetClientFn, t translations.TranslationHelperFunc) { + s.AddResourceTemplate(GetRepositoryResourceContent(getClient, t)) + s.AddResourceTemplate(GetRepositoryResourceBranchContent(getClient, t)) + s.AddResourceTemplate(GetRepositoryResourceCommitContent(getClient, t)) + s.AddResourceTemplate(GetRepositoryResourceTagContent(getClient, t)) + s.AddResourceTemplate(GetRepositoryResourcePrContent(getClient, t)) +} diff --git a/pkg/github/search.go b/pkg/github/search.go index 117e8298..dc85c177 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -12,22 +12,22 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// searchRepositories creates a tool to search for GitHub repositories. -func searchRepositories(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// SearchRepositories creates a tool to search for GitHub repositories. +func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_repositories", mcp.WithDescription(t("TOOL_SEARCH_REPOSITORIES_DESCRIPTION", "Search for GitHub repositories")), mcp.WithString("query", mcp.Required(), mcp.Description("Search query"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { query, err := requiredParam[string](request, "query") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -39,6 +39,10 @@ func searchRepositories(client *github.Client, t translations.TranslationHelperF }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } result, resp, err := client.Search.Repositories(ctx, query, opts) if err != nil { return nil, fmt.Errorf("failed to search repositories: %w", err) @@ -62,8 +66,8 @@ func searchRepositories(client *github.Client, t translations.TranslationHelperF } } -// searchCode creates a tool to search for code across GitHub repositories. -func searchCode(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// SearchCode creates a tool to search for code across GitHub repositories. +func SearchCode(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_code", mcp.WithDescription(t("TOOL_SEARCH_CODE_DESCRIPTION", "Search for code across GitHub repositories")), mcp.WithString("q", @@ -74,25 +78,25 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to mcp.Description("Sort field ('indexed' only)"), ), mcp.WithString("order", - mcp.Description("Sort order ('asc' or 'desc')"), + mcp.Description("Sort order"), mcp.Enum("asc", "desc"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { query, err := requiredParam[string](request, "q") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - sort, err := optionalParam[string](request, "sort") + sort, err := OptionalParam[string](request, "sort") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - order, err := optionalParam[string](request, "order") + order, err := OptionalParam[string](request, "order") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -106,6 +110,11 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + result, resp, err := client.Search.Code(ctx, query, opts) if err != nil { return nil, fmt.Errorf("failed to search code: %w", err) @@ -129,8 +138,8 @@ func searchCode(client *github.Client, t translations.TranslationHelperFunc) (to } } -// searchUsers creates a tool to search for GitHub users. -func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +// SearchUsers creates a tool to search for GitHub users. +func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_users", mcp.WithDescription(t("TOOL_SEARCH_USERS_DESCRIPTION", "Search for GitHub users")), mcp.WithString("q", @@ -138,29 +147,29 @@ func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (t mcp.Description("Search query using GitHub users search syntax"), ), mcp.WithString("sort", - mcp.Description("Sort field (followers, repositories, joined)"), + mcp.Description("Sort field by category"), mcp.Enum("followers", "repositories", "joined"), ), mcp.WithString("order", - mcp.Description("Sort order ('asc' or 'desc')"), + mcp.Description("Sort order"), mcp.Enum("asc", "desc"), ), - withPagination(), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { query, err := requiredParam[string](request, "q") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - sort, err := optionalParam[string](request, "sort") + sort, err := OptionalParam[string](request, "sort") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - order, err := optionalParam[string](request, "order") + order, err := OptionalParam[string](request, "order") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - pagination, err := optionalPaginationParams(request) + pagination, err := OptionalPaginationParams(request) if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -174,6 +183,11 @@ func searchUsers(client *github.Client, t translations.TranslationHelperFunc) (t }, } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + result, resp, err := client.Search.Users(ctx, query, opts) if err != nil { return nil, fmt.Errorf("failed to search users: %w", err) diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index bf1bff45..b61518e4 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -16,7 +16,7 @@ import ( func Test_SearchRepositories(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := searchRepositories(mockClient, translations.NullTranslationHelper) + tool, _ := SearchRepositories(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "search_repositories", tool.Name) assert.NotEmpty(t, tool.Description) @@ -122,7 +122,7 @@ func Test_SearchRepositories(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := searchRepositories(client, translations.NullTranslationHelper) + _, handler := SearchRepositories(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -163,7 +163,7 @@ func Test_SearchRepositories(t *testing.T) { func Test_SearchCode(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := searchCode(mockClient, translations.NullTranslationHelper) + tool, _ := SearchCode(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "search_code", tool.Name) assert.NotEmpty(t, tool.Description) @@ -273,7 +273,7 @@ func Test_SearchCode(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := searchCode(client, translations.NullTranslationHelper) + _, handler := SearchCode(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -314,7 +314,7 @@ func Test_SearchCode(t *testing.T) { func Test_SearchUsers(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := searchUsers(mockClient, translations.NullTranslationHelper) + tool, _ := SearchUsers(stubGetClientFn(mockClient), translations.NullTranslationHelper) assert.Equal(t, "search_users", tool.Name) assert.NotEmpty(t, tool.Description) @@ -428,7 +428,7 @@ func Test_SearchUsers(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := searchUsers(client, translations.NullTranslationHelper) + _, handler := SearchUsers(stubGetClientFn(client), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/secret_scanning.go b/pkg/github/secret_scanning.go new file mode 100644 index 00000000..ee344061 --- /dev/null +++ b/pkg/github/secret_scanning.go @@ -0,0 +1,146 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +func GetSecretScanningAlert(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool( + "get_secret_scanning_alert", + mcp.WithDescription(t("TOOL_GET_SECRET_SCANNING_ALERT_DESCRIPTION", "Get details of a specific secret scanning alert in a GitHub repository.")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("The owner of the repository."), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("The name of the repository."), + ), + mcp.WithNumber("alertNumber", + mcp.Required(), + mcp.Description("The number of the alert."), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + alertNumber, err := RequiredInt(request, "alertNumber") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + alert, resp, err := client.SecretScanning.GetAlert(ctx, owner, repo, int64(alertNumber)) + if err != nil { + return nil, fmt.Errorf("failed to get alert: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to get alert: %s", string(body))), nil + } + + r, err := json.Marshal(alert) + if err != nil { + return nil, fmt.Errorf("failed to marshal alert: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +func ListSecretScanningAlerts(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool( + "list_secret_scanning_alerts", + mcp.WithDescription(t("TOOL_LIST_SECRET_SCANNING_ALERTS_DESCRIPTION", "List secret scanning alerts in a GitHub repository.")), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("The owner of the repository."), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("The name of the repository."), + ), + mcp.WithString("state", + mcp.Description("Filter by state"), + mcp.Enum("open", "resolved"), + ), + mcp.WithString("secret_type", + mcp.Description("A comma-separated list of secret types to return. All default secret patterns are returned. To return generic patterns, pass the token name(s) in the parameter."), + ), + mcp.WithString("resolution", + mcp.Description("Filter by resolution"), + mcp.Enum("false_positive", "wont_fix", "revoked", "pattern_edited", "pattern_deleted", "used_in_tests"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := requiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := requiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + state, err := OptionalParam[string](request, "state") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + secretType, err := OptionalParam[string](request, "secret_type") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + resolution, err := OptionalParam[string](request, "resolution") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + alerts, resp, err := client.SecretScanning.ListAlertsForRepo(ctx, owner, repo, &github.SecretScanningAlertListOptions{State: state, SecretType: secretType, Resolution: resolution}) + if err != nil { + return nil, fmt.Errorf("failed to list alerts: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to list alerts: %s", string(body))), nil + } + + r, err := json.Marshal(alerts) + if err != nil { + return nil, fmt.Errorf("failed to marshal alerts: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/secret_scanning_test.go b/pkg/github/secret_scanning_test.go new file mode 100644 index 00000000..d32cbca9 --- /dev/null +++ b/pkg/github/secret_scanning_test.go @@ -0,0 +1,243 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GetSecretScanningAlert(t *testing.T) { + mockClient := github.NewClient(nil) + tool, _ := GetSecretScanningAlert(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "get_secret_scanning_alert", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "alertNumber") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "alertNumber"}) + + // Setup mock alert for success case + mockAlert := &github.SecretScanningAlert{ + Number: github.Ptr(42), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/private-repo/security/secret-scanning/42"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedAlert *github.SecretScanningAlert + expectedErrMsg string + }{ + { + name: "successful alert fetch", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber, + mockAlert, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "alertNumber": float64(42), + }, + expectError: false, + expectedAlert: mockAlert, + }, + { + name: "alert fetch fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "alertNumber": float64(9999), + }, + expectError: true, + expectedErrMsg: "failed to get alert", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := GetSecretScanningAlert(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedAlert github.Alert + err = json.Unmarshal([]byte(textContent.Text), &returnedAlert) + assert.NoError(t, err) + assert.Equal(t, *tc.expectedAlert.Number, *returnedAlert.Number) + assert.Equal(t, *tc.expectedAlert.State, *returnedAlert.State) + assert.Equal(t, *tc.expectedAlert.HTMLURL, *returnedAlert.HTMLURL) + + }) + } +} + +func Test_ListSecretScanningAlerts(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListSecretScanningAlerts(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_secret_scanning_alerts", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "state") + assert.Contains(t, tool.InputSchema.Properties, "secret_type") + assert.Contains(t, tool.InputSchema.Properties, "resolution") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + // Setup mock alerts for success case + resolvedAlert := github.SecretScanningAlert{ + Number: github.Ptr(2), + HTMLURL: github.Ptr("https://github.com/owner/private-repo/security/secret-scanning/2"), + State: github.Ptr("resolved"), + Resolution: github.Ptr("false_positive"), + SecretType: github.Ptr("adafruit_io_key"), + } + openAlert := github.SecretScanningAlert{ + Number: github.Ptr(2), + HTMLURL: github.Ptr("https://github.com/owner/private-repo/security/secret-scanning/3"), + State: github.Ptr("open"), + Resolution: github.Ptr("false_positive"), + SecretType: github.Ptr("adafruit_io_key"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedAlerts []*github.SecretScanningAlert + expectedErrMsg string + }{ + { + name: "successful resolved alerts listing", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposSecretScanningAlertsByOwnerByRepo, + expectQueryParams(t, map[string]string{ + "state": "resolved", + }).andThen( + mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{&resolvedAlert}), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "state": "resolved", + }, + expectError: false, + expectedAlerts: []*github.SecretScanningAlert{&resolvedAlert}, + }, + { + name: "successful alerts listing", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposSecretScanningAlertsByOwnerByRepo, + expectQueryParams(t, map[string]string{}).andThen( + mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{&resolvedAlert, &openAlert}), + ), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedAlerts: []*github.SecretScanningAlert{&resolvedAlert, &openAlert}, + }, + { + name: "alerts listing fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposSecretScanningAlertsByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Unauthorized access"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: true, + expectedErrMsg: "failed to list alerts", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + _, handler := ListSecretScanningAlerts(stubGetClientFn(client), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + result, err := handler(context.Background(), request) + + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedAlerts []*github.SecretScanningAlert + err = json.Unmarshal([]byte(textContent.Text), &returnedAlerts) + assert.NoError(t, err) + assert.Len(t, returnedAlerts, len(tc.expectedAlerts)) + for i, alert := range returnedAlerts { + assert.Equal(t, *tc.expectedAlerts[i].Number, *alert.Number) + assert.Equal(t, *tc.expectedAlerts[i].HTMLURL, *alert.HTMLURL) + assert.Equal(t, *tc.expectedAlerts[i].State, *alert.State) + assert.Equal(t, *tc.expectedAlerts[i].Resolution, *alert.Resolution) + assert.Equal(t, *tc.expectedAlerts[i].SecretType, *alert.SecretType) + } + }) + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index bf3583b9..e4c24171 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -1,115 +1,56 @@ package github import ( - "context" - "encoding/json" "errors" "fmt" - "io" - "net/http" - "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v69/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) // NewServer creates a new GitHub MCP server with the specified GH client and logger. -func NewServer(client *github.Client, version string, readOnly bool, t translations.TranslationHelperFunc) *server.MCPServer { + +func NewServer(version string, opts ...server.ServerOption) *server.MCPServer { + // Add default options + defaultOpts := []server.ServerOption{ + server.WithToolCapabilities(true), + server.WithResourceCapabilities(true, true), + server.WithLogging(), + } + opts = append(defaultOpts, opts...) + // Create a new MCP server s := server.NewMCPServer( "github-mcp-server", version, - server.WithResourceCapabilities(true, true), - server.WithLogging()) - - // Add GitHub Resources - s.AddResourceTemplate(getRepositoryResourceContent(client, t)) - s.AddResourceTemplate(getRepositoryResourceBranchContent(client, t)) - s.AddResourceTemplate(getRepositoryResourceCommitContent(client, t)) - s.AddResourceTemplate(getRepositoryResourceTagContent(client, t)) - s.AddResourceTemplate(getRepositoryResourcePrContent(client, t)) - - // Add GitHub tools - Issues - s.AddTool(getIssue(client, t)) - s.AddTool(searchIssues(client, t)) - s.AddTool(listIssues(client, t)) - s.AddTool(getIssueComments(client, t)) - if !readOnly { - s.AddTool(createIssue(client, t)) - s.AddTool(addIssueComment(client, t)) - s.AddTool(updateIssue(client, t)) - } - - // Add GitHub tools - Pull Requests - s.AddTool(getPullRequest(client, t)) - s.AddTool(listPullRequests(client, t)) - s.AddTool(getPullRequestFiles(client, t)) - s.AddTool(getPullRequestStatus(client, t)) - s.AddTool(getPullRequestComments(client, t)) - s.AddTool(getPullRequestReviews(client, t)) - if !readOnly { - s.AddTool(mergePullRequest(client, t)) - s.AddTool(updatePullRequestBranch(client, t)) - s.AddTool(createPullRequestReview(client, t)) - s.AddTool(createPullRequest(client, t)) - } - - // Add GitHub tools - Repositories - s.AddTool(searchRepositories(client, t)) - s.AddTool(getFileContents(client, t)) - s.AddTool(listCommits(client, t)) - if !readOnly { - s.AddTool(createOrUpdateFile(client, t)) - s.AddTool(createRepository(client, t)) - s.AddTool(forkRepository(client, t)) - s.AddTool(createBranch(client, t)) - s.AddTool(pushFiles(client, t)) - } - - // Add GitHub tools - Search - s.AddTool(searchCode(client, t)) - s.AddTool(searchUsers(client, t)) - - // Add GitHub tools - Users - s.AddTool(getMe(client, t)) - - // Add GitHub tools - Code Scanning - s.AddTool(getCodeScanningAlert(client, t)) - s.AddTool(listCodeScanningAlerts(client, t)) + opts..., + ) return s } -// getMe creates a tool to get details of the authenticated user. -func getMe(client *github.Client, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { - return mcp.NewTool("get_me", - mcp.WithDescription(t("TOOL_GET_ME_DESCRIPTION", "Get details of the authenticated GitHub user. Use this when a request include \"me\", \"my\"...")), - mcp.WithString("reason", - mcp.Description("Optional: reason the session was created"), - ), - ), - func(ctx context.Context, _ mcp.CallToolRequest) (*mcp.CallToolResult, error) { - user, resp, err := client.Users.Get(ctx, "") - if err != nil { - return nil, fmt.Errorf("failed to get user: %w", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to get user: %s", string(body))), nil - } +// 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](r mcp.CallToolRequest, p string) (value T, ok bool, err error) { + // Check if the parameter is present in the request + val, exists := r.Params.Arguments[p] + if !exists { + // Not present, return zero value, false, no error + return + } - r, err := json.Marshal(user) - if err != nil { - return nil, fmt.Errorf("failed to marshal user: %w", err) - } + // Check if the parameter is of the expected type + value, ok = val.(T) + if !ok { + // Present but wrong type + err = fmt.Errorf("parameter %s is not of type %T, is %T", p, value, val) + ok = true // Set ok to true because the parameter *was* present, even if wrong type + return + } - return mcp.NewToolResultText(string(r)), nil - } + // Present and correct type + ok = true + return } // isAcceptedError checks if the error is an accepted error. @@ -144,12 +85,12 @@ func requiredParam[T comparable](r mcp.CallToolRequest, p string) (T, error) { return r.Params.Arguments[p].(T), nil } -// requiredInt is a helper function that can be used to fetch a requested parameter from the request. +// RequiredInt is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request. // 2. Checks if the parameter is of the expected type. // 3. Checks if the parameter is not empty, i.e: non-zero value -func requiredInt(r mcp.CallToolRequest, p string) (int, error) { +func RequiredInt(r mcp.CallToolRequest, p string) (int, error) { v, err := requiredParam[float64](r, p) if err != nil { return 0, err @@ -157,11 +98,11 @@ func requiredInt(r mcp.CallToolRequest, p string) (int, error) { return int(v), nil } -// optionalParam is a helper function that can be used to fetch a requested parameter from the request. +// OptionalParam is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request, if not, it returns its zero-value // 2. If it is present, it checks if the parameter is of the expected type and returns it -func optionalParam[T any](r mcp.CallToolRequest, p string) (T, error) { +func OptionalParam[T any](r mcp.CallToolRequest, p string) (T, error) { var zero T // Check if the parameter is present in the request @@ -177,22 +118,22 @@ func optionalParam[T any](r mcp.CallToolRequest, p string) (T, error) { return r.Params.Arguments[p].(T), nil } -// optionalIntParam is a helper function that can be used to fetch a requested parameter from the request. +// OptionalIntParam is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request, if not, it returns its zero-value // 2. If it is present, it checks if the parameter is of the expected type and returns it -func optionalIntParam(r mcp.CallToolRequest, p string) (int, error) { - v, err := optionalParam[float64](r, p) +func OptionalIntParam(r mcp.CallToolRequest, p string) (int, error) { + v, err := OptionalParam[float64](r, p) if err != nil { return 0, err } return int(v), nil } -// optionalIntParamWithDefault is a helper function that can be used to fetch a requested parameter from the request +// OptionalIntParamWithDefault is a helper function that can be used to fetch a requested parameter from the request // similar to optionalIntParam, but it also takes a default value. -func optionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, error) { - v, err := optionalIntParam(r, p) +func OptionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, error) { + v, err := OptionalIntParam(r, p) if err != nil { return 0, err } @@ -202,17 +143,19 @@ func optionalIntParamWithDefault(r mcp.CallToolRequest, p string, d int) (int, e return v, nil } -// optionalStringArrayParam is a helper function that can be used to fetch a requested parameter from the request. +// OptionalStringArrayParam is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request, if not, it returns its zero-value // 2. If it is present, iterates the elements and checks each is a string -func optionalStringArrayParam(r mcp.CallToolRequest, p string) ([]string, error) { +func OptionalStringArrayParam(r mcp.CallToolRequest, p string) ([]string, error) { // Check if the parameter is present in the request if _, ok := r.Params.Arguments[p]; !ok { return []string{}, nil } switch v := r.Params.Arguments[p].(type) { + case nil: + return []string{}, nil case []string: return v, nil case []any: @@ -230,9 +173,9 @@ func optionalStringArrayParam(r mcp.CallToolRequest, p string) ([]string, error) } } -// withPagination returns a ToolOption that adds "page" and "perPage" parameters to the tool. +// WithPagination returns a ToolOption that adds "page" and "perPage" parameters to the tool. // The "page" parameter is optional, min 1. The "perPage" parameter is optional, min 1, max 100. -func withPagination() mcp.ToolOption { +func WithPagination() mcp.ToolOption { return func(tool *mcp.Tool) { mcp.WithNumber("page", mcp.Description("Page number for pagination (min 1)"), @@ -247,26 +190,26 @@ func withPagination() mcp.ToolOption { } } -type paginationParams struct { +type PaginationParams struct { page int perPage int } -// optionalPaginationParams returns the "page" and "perPage" parameters from the request, +// OptionalPaginationParams returns the "page" and "perPage" parameters from the request, // or their default values if not present, "page" default is 1, "perPage" default is 30. // In future, we may want to make the default values configurable, or even have this // function returned from `withPagination`, where the defaults are provided alongside // the min/max values. -func optionalPaginationParams(r mcp.CallToolRequest) (paginationParams, error) { - page, err := optionalIntParamWithDefault(r, "page", 1) +func OptionalPaginationParams(r mcp.CallToolRequest) (PaginationParams, error) { + page, err := OptionalIntParamWithDefault(r, "page", 1) if err != nil { - return paginationParams{}, err + return PaginationParams{}, err } - perPage, err := optionalIntParamWithDefault(r, "perPage", 30) + perPage, err := OptionalIntParamWithDefault(r, "perPage", 30) if err != nil { - return paginationParams{}, err + return PaginationParams{}, err } - return paginationParams{ + return PaginationParams{ page: page, perPage: perPage, }, nil diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 149fb77a..58bcb9db 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -2,133 +2,16 @@ package github import ( "context" - "encoding/json" "fmt" - "net/http" "testing" - "time" - "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v69/github" - "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func Test_GetMe(t *testing.T) { - // Verify tool definition - mockClient := github.NewClient(nil) - tool, _ := getMe(mockClient, translations.NullTranslationHelper) - - assert.Equal(t, "get_me", tool.Name) - assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "reason") - assert.Empty(t, tool.InputSchema.Required) // No required parameters - - // Setup mock user response - mockUser := &github.User{ - Login: github.Ptr("testuser"), - Name: github.Ptr("Test User"), - Email: github.Ptr("test@example.com"), - Bio: github.Ptr("GitHub user for testing"), - Company: github.Ptr("Test Company"), - Location: github.Ptr("Test Location"), - HTMLURL: github.Ptr("https://github.com/testuser"), - CreatedAt: &github.Timestamp{Time: time.Now().Add(-365 * 24 * time.Hour)}, - Type: github.Ptr("User"), - Plan: &github.Plan{ - Name: github.Ptr("pro"), - }, - } - - tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]interface{} - expectError bool - expectedUser *github.User - expectedErrMsg string - }{ - { - name: "successful get user", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatch( - mock.GetUser, - mockUser, - ), - ), - requestArgs: map[string]interface{}{}, - expectError: false, - expectedUser: mockUser, - }, - { - name: "successful get user with reason", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatch( - mock.GetUser, - mockUser, - ), - ), - requestArgs: map[string]interface{}{ - "reason": "Testing API", - }, - expectError: false, - expectedUser: mockUser, - }, - { - name: "get user fails", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.GetUser, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusUnauthorized) - _, _ = w.Write([]byte(`{"message": "Unauthorized"}`)) - }), - ), - ), - requestArgs: map[string]interface{}{}, - expectError: true, - expectedErrMsg: "failed to get user", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - // Setup client with mock - client := github.NewClient(tc.mockedClient) - _, handler := getMe(client, translations.NullTranslationHelper) - - // Create call request - request := createMCPRequest(tc.requestArgs) - - // Call handler - result, err := handler(context.Background(), request) - - // Verify results - if tc.expectError { - require.Error(t, err) - assert.Contains(t, err.Error(), tc.expectedErrMsg) - return - } - - require.NoError(t, err) - - // Parse result and get text content if no error - textContent := getTextResult(t, result) - - // Unmarshal and verify the result - var returnedUser github.User - err = json.Unmarshal([]byte(textContent.Text), &returnedUser) - require.NoError(t, err) - - // Verify user details - assert.Equal(t, *tc.expectedUser.Login, *returnedUser.Login) - assert.Equal(t, *tc.expectedUser.Name, *returnedUser.Name) - assert.Equal(t, *tc.expectedUser.Email, *returnedUser.Email) - assert.Equal(t, *tc.expectedUser.Bio, *returnedUser.Bio) - assert.Equal(t, *tc.expectedUser.HTMLURL, *returnedUser.HTMLURL) - assert.Equal(t, *tc.expectedUser.Type, *returnedUser.Type) - }) +func stubGetClientFn(client *github.Client) GetClientFn { + return func(_ context.Context) (*github.Client, error) { + return client, nil } } @@ -262,7 +145,7 @@ func Test_OptionalStringParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalParam[string](request, tc.paramName) + result, err := OptionalParam[string](request, tc.paramName) if tc.expectError { assert.Error(t, err) @@ -308,7 +191,7 @@ func Test_RequiredNumberParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := requiredInt(request, tc.paramName) + result, err := RequiredInt(request, tc.paramName) if tc.expectError { assert.Error(t, err) @@ -361,7 +244,7 @@ func Test_OptionalNumberParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalIntParam(request, tc.paramName) + result, err := OptionalIntParam(request, tc.paramName) if tc.expectError { assert.Error(t, err) @@ -419,7 +302,7 @@ func Test_OptionalNumberParamWithDefault(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalIntParamWithDefault(request, tc.paramName, tc.defaultVal) + result, err := OptionalIntParamWithDefault(request, tc.paramName, tc.defaultVal) if tc.expectError { assert.Error(t, err) @@ -472,7 +355,7 @@ func Test_OptionalBooleanParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalParam[bool](request, tc.paramName) + result, err := OptionalParam[bool](request, tc.paramName) if tc.expectError { assert.Error(t, err) @@ -540,7 +423,7 @@ func TestOptionalStringArrayParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalStringArrayParam(request, tc.paramName) + result, err := OptionalStringArrayParam(request, tc.paramName) if tc.expectError { assert.Error(t, err) @@ -556,13 +439,13 @@ func TestOptionalPaginationParams(t *testing.T) { tests := []struct { name string params map[string]any - expected paginationParams + expected PaginationParams expectError bool }{ { name: "no pagination parameters, default values", params: map[string]any{}, - expected: paginationParams{ + expected: PaginationParams{ page: 1, perPage: 30, }, @@ -573,7 +456,7 @@ func TestOptionalPaginationParams(t *testing.T) { params: map[string]any{ "page": float64(2), }, - expected: paginationParams{ + expected: PaginationParams{ page: 2, perPage: 30, }, @@ -584,7 +467,7 @@ func TestOptionalPaginationParams(t *testing.T) { params: map[string]any{ "perPage": float64(50), }, - expected: paginationParams{ + expected: PaginationParams{ page: 1, perPage: 50, }, @@ -596,7 +479,7 @@ func TestOptionalPaginationParams(t *testing.T) { "page": float64(2), "perPage": float64(50), }, - expected: paginationParams{ + expected: PaginationParams{ page: 2, perPage: 50, }, @@ -607,7 +490,7 @@ func TestOptionalPaginationParams(t *testing.T) { params: map[string]any{ "page": "not-a-number", }, - expected: paginationParams{}, + expected: PaginationParams{}, expectError: true, }, { @@ -615,7 +498,7 @@ func TestOptionalPaginationParams(t *testing.T) { params: map[string]any{ "perPage": "not-a-number", }, - expected: paginationParams{}, + expected: PaginationParams{}, expectError: true, }, } @@ -623,7 +506,7 @@ func TestOptionalPaginationParams(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { request := createMCPRequest(tc.params) - result, err := optionalPaginationParams(request) + result, err := OptionalPaginationParams(request) if tc.expectError { assert.Error(t, err) diff --git a/pkg/github/tools.go b/pkg/github/tools.go new file mode 100644 index 00000000..35dabaef --- /dev/null +++ b/pkg/github/tools.go @@ -0,0 +1,123 @@ +package github + +import ( + "context" + + "github.com/github/github-mcp-server/pkg/toolsets" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/mark3labs/mcp-go/server" +) + +type GetClientFn func(context.Context) (*github.Client, error) + +var DefaultTools = []string{"all"} + +func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, t translations.TranslationHelperFunc) (*toolsets.ToolsetGroup, error) { + // Create a new toolset group + tsg := toolsets.NewToolsetGroup(readOnly) + + // Define all available features with their default state (disabled) + // Create toolsets + repos := toolsets.NewToolset("repos", "GitHub Repository related tools"). + AddReadTools( + toolsets.NewServerTool(SearchRepositories(getClient, t)), + toolsets.NewServerTool(GetFileContents(getClient, t)), + toolsets.NewServerTool(ListCommits(getClient, t)), + toolsets.NewServerTool(SearchCode(getClient, t)), + toolsets.NewServerTool(GetCommit(getClient, t)), + toolsets.NewServerTool(ListBranches(getClient, t)), + ). + AddWriteTools( + toolsets.NewServerTool(CreateOrUpdateFile(getClient, t)), + toolsets.NewServerTool(CreateRepository(getClient, t)), + toolsets.NewServerTool(ForkRepository(getClient, t)), + toolsets.NewServerTool(CreateBranch(getClient, t)), + toolsets.NewServerTool(PushFiles(getClient, t)), + ) + issues := toolsets.NewToolset("issues", "GitHub Issues related tools"). + AddReadTools( + toolsets.NewServerTool(GetIssue(getClient, t)), + toolsets.NewServerTool(SearchIssues(getClient, t)), + toolsets.NewServerTool(ListIssues(getClient, t)), + toolsets.NewServerTool(GetIssueComments(getClient, t)), + ). + AddWriteTools( + toolsets.NewServerTool(CreateIssue(getClient, t)), + toolsets.NewServerTool(AddIssueComment(getClient, t)), + toolsets.NewServerTool(UpdateIssue(getClient, t)), + ) + users := toolsets.NewToolset("users", "GitHub User related tools"). + AddReadTools( + toolsets.NewServerTool(SearchUsers(getClient, t)), + ) + pullRequests := toolsets.NewToolset("pull_requests", "GitHub Pull Request related tools"). + AddReadTools( + toolsets.NewServerTool(GetPullRequest(getClient, t)), + toolsets.NewServerTool(ListPullRequests(getClient, t)), + toolsets.NewServerTool(GetPullRequestFiles(getClient, t)), + toolsets.NewServerTool(GetPullRequestStatus(getClient, t)), + toolsets.NewServerTool(GetPullRequestComments(getClient, t)), + toolsets.NewServerTool(GetPullRequestReviews(getClient, t)), + ). + AddWriteTools( + toolsets.NewServerTool(MergePullRequest(getClient, t)), + toolsets.NewServerTool(UpdatePullRequestBranch(getClient, t)), + toolsets.NewServerTool(CreatePullRequestReview(getClient, t)), + toolsets.NewServerTool(CreatePullRequest(getClient, t)), + toolsets.NewServerTool(UpdatePullRequest(getClient, t)), + toolsets.NewServerTool(AddPullRequestReviewComment(getClient, t)), + ) + codeSecurity := toolsets.NewToolset("code_security", "Code security related tools, such as GitHub Code Scanning"). + AddReadTools( + toolsets.NewServerTool(GetCodeScanningAlert(getClient, t)), + toolsets.NewServerTool(ListCodeScanningAlerts(getClient, t)), + ) + secretProtection := toolsets.NewToolset("secret_protection", "Secret protection related tools, such as GitHub Secret Scanning"). + AddReadTools( + toolsets.NewServerTool(GetSecretScanningAlert(getClient, t)), + toolsets.NewServerTool(ListSecretScanningAlerts(getClient, t)), + ) + // Keep experiments alive so the system doesn't error out when it's always enabled + experiments := toolsets.NewToolset("experiments", "Experimental features that are not considered stable yet") + + // Add toolsets to the group + tsg.AddToolset(repos) + tsg.AddToolset(issues) + tsg.AddToolset(users) + tsg.AddToolset(pullRequests) + tsg.AddToolset(codeSecurity) + tsg.AddToolset(secretProtection) + tsg.AddToolset(experiments) + // Enable the requested features + + if err := tsg.EnableToolsets(passedToolsets); err != nil { + return nil, err + } + + return tsg, nil +} + +func InitContextToolset(getClient GetClientFn, t translations.TranslationHelperFunc) *toolsets.Toolset { + // Create a new context toolset + contextTools := toolsets.NewToolset("context", "Tools that provide context about the current user and GitHub context you are operating in"). + AddReadTools( + toolsets.NewServerTool(GetMe(getClient, t)), + ) + contextTools.Enabled = true + return contextTools +} + +// InitDynamicToolset creates a dynamic toolset that can be used to enable other toolsets, and so requires the server and toolset group as arguments +func InitDynamicToolset(s *server.MCPServer, tsg *toolsets.ToolsetGroup, t translations.TranslationHelperFunc) *toolsets.Toolset { + // Create a new dynamic toolset + // Need to add the dynamic toolset last so it can be used to enable other toolsets + dynamicToolSelection := toolsets.NewToolset("dynamic", "Discover GitHub MCP tools that can help achieve tasks by enabling additional sets of tools, you can control the enablement of any toolset to access its tools when this toolset is enabled."). + AddReadTools( + toolsets.NewServerTool(ListAvailableToolsets(tsg, t)), + toolsets.NewServerTool(GetToolsetsTools(tsg, t)), + toolsets.NewServerTool(EnableToolset(s, tsg, t)), + ) + dynamicToolSelection.Enabled = true + return dynamicToolSelection +} diff --git a/pkg/toolsets/toolsets.go b/pkg/toolsets/toolsets.go new file mode 100644 index 00000000..d4397fc9 --- /dev/null +++ b/pkg/toolsets/toolsets.go @@ -0,0 +1,154 @@ +package toolsets + +import ( + "fmt" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +func NewServerTool(tool mcp.Tool, handler server.ToolHandlerFunc) server.ServerTool { + return server.ServerTool{Tool: tool, Handler: handler} +} + +type Toolset struct { + Name string + Description string + Enabled bool + readOnly bool + writeTools []server.ServerTool + readTools []server.ServerTool +} + +func (t *Toolset) GetActiveTools() []server.ServerTool { + if t.Enabled { + if t.readOnly { + return t.readTools + } + return append(t.readTools, t.writeTools...) + } + return nil +} + +func (t *Toolset) GetAvailableTools() []server.ServerTool { + if t.readOnly { + return t.readTools + } + return append(t.readTools, t.writeTools...) +} + +func (t *Toolset) RegisterTools(s *server.MCPServer) { + if !t.Enabled { + return + } + for _, tool := range t.readTools { + s.AddTool(tool.Tool, tool.Handler) + } + if !t.readOnly { + for _, tool := range t.writeTools { + s.AddTool(tool.Tool, tool.Handler) + } + } +} + +func (t *Toolset) SetReadOnly() { + // Set the toolset to read-only + t.readOnly = true +} + +func (t *Toolset) AddWriteTools(tools ...server.ServerTool) *Toolset { + // Silently ignore if the toolset is read-only to avoid any breach of that contract + if !t.readOnly { + t.writeTools = append(t.writeTools, tools...) + } + return t +} + +func (t *Toolset) AddReadTools(tools ...server.ServerTool) *Toolset { + t.readTools = append(t.readTools, tools...) + return t +} + +type ToolsetGroup struct { + Toolsets map[string]*Toolset + everythingOn bool + readOnly bool +} + +func NewToolsetGroup(readOnly bool) *ToolsetGroup { + return &ToolsetGroup{ + Toolsets: make(map[string]*Toolset), + everythingOn: false, + readOnly: readOnly, + } +} + +func (tg *ToolsetGroup) AddToolset(ts *Toolset) { + if tg.readOnly { + ts.SetReadOnly() + } + tg.Toolsets[ts.Name] = ts +} + +func NewToolset(name string, description string) *Toolset { + return &Toolset{ + Name: name, + Description: description, + Enabled: false, + readOnly: false, + } +} + +func (tg *ToolsetGroup) IsEnabled(name string) bool { + // If everythingOn is true, all features are enabled + if tg.everythingOn { + return true + } + + feature, exists := tg.Toolsets[name] + if !exists { + return false + } + return feature.Enabled +} + +func (tg *ToolsetGroup) EnableToolsets(names []string) error { + // Special case for "all" + for _, name := range names { + if name == "all" { + tg.everythingOn = true + break + } + err := tg.EnableToolset(name) + if err != nil { + return err + } + } + // Do this after to ensure all toolsets are enabled if "all" is present anywhere in list + if tg.everythingOn { + for name := range tg.Toolsets { + err := tg.EnableToolset(name) + if err != nil { + return err + } + } + return nil + } + return nil +} + +func (tg *ToolsetGroup) EnableToolset(name string) error { + toolset, exists := tg.Toolsets[name] + if !exists { + return fmt.Errorf("toolset %s does not exist", name) + } + toolset.Enabled = true + tg.Toolsets[name] = toolset + return nil +} + +func (tg *ToolsetGroup) RegisterTools(s *server.MCPServer) { + for _, toolset := range tg.Toolsets { + toolset.RegisterTools(s) + } +} diff --git a/pkg/toolsets/toolsets_test.go b/pkg/toolsets/toolsets_test.go new file mode 100644 index 00000000..7ece1df1 --- /dev/null +++ b/pkg/toolsets/toolsets_test.go @@ -0,0 +1,230 @@ +package toolsets + +import ( + "testing" +) + +func TestNewToolsetGroup(t *testing.T) { + tsg := NewToolsetGroup(false) + if tsg == nil { + t.Fatal("Expected NewToolsetGroup to return a non-nil pointer") + } + if tsg.Toolsets == nil { + t.Fatal("Expected Toolsets map to be initialized") + } + if len(tsg.Toolsets) != 0 { + t.Fatalf("Expected Toolsets map to be empty, got %d items", len(tsg.Toolsets)) + } + if tsg.everythingOn { + t.Fatal("Expected everythingOn to be initialized as false") + } +} + +func TestAddToolset(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Test adding a toolset + toolset := NewToolset("test-toolset", "A test toolset") + toolset.Enabled = true + tsg.AddToolset(toolset) + + // Verify toolset was added correctly + if len(tsg.Toolsets) != 1 { + t.Errorf("Expected 1 toolset, got %d", len(tsg.Toolsets)) + } + + toolset, exists := tsg.Toolsets["test-toolset"] + if !exists { + t.Fatal("Feature was not added to the map") + } + + if toolset.Name != "test-toolset" { + t.Errorf("Expected toolset name to be 'test-toolset', got '%s'", toolset.Name) + } + + if toolset.Description != "A test toolset" { + t.Errorf("Expected toolset description to be 'A test toolset', got '%s'", toolset.Description) + } + + if !toolset.Enabled { + t.Error("Expected toolset to be enabled") + } + + // Test adding another toolset + anotherToolset := NewToolset("another-toolset", "Another test toolset") + tsg.AddToolset(anotherToolset) + + if len(tsg.Toolsets) != 2 { + t.Errorf("Expected 2 toolsets, got %d", len(tsg.Toolsets)) + } + + // Test overriding existing toolset + updatedToolset := NewToolset("test-toolset", "Updated description") + tsg.AddToolset(updatedToolset) + + toolset = tsg.Toolsets["test-toolset"] + if toolset.Description != "Updated description" { + t.Errorf("Expected toolset description to be updated to 'Updated description', got '%s'", toolset.Description) + } + + if toolset.Enabled { + t.Error("Expected toolset to be disabled after update") + } +} + +func TestIsEnabled(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Test with non-existent toolset + if tsg.IsEnabled("non-existent") { + t.Error("Expected IsEnabled to return false for non-existent toolset") + } + + // Test with disabled toolset + disabledToolset := NewToolset("disabled-toolset", "A disabled toolset") + tsg.AddToolset(disabledToolset) + if tsg.IsEnabled("disabled-toolset") { + t.Error("Expected IsEnabled to return false for disabled toolset") + } + + // Test with enabled toolset + enabledToolset := NewToolset("enabled-toolset", "An enabled toolset") + enabledToolset.Enabled = true + tsg.AddToolset(enabledToolset) + if !tsg.IsEnabled("enabled-toolset") { + t.Error("Expected IsEnabled to return true for enabled toolset") + } +} + +func TestEnableFeature(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Test enabling non-existent toolset + err := tsg.EnableToolset("non-existent") + if err == nil { + t.Error("Expected error when enabling non-existent toolset") + } + + // Test enabling toolset + testToolset := NewToolset("test-toolset", "A test toolset") + tsg.AddToolset(testToolset) + + if tsg.IsEnabled("test-toolset") { + t.Error("Expected toolset to be disabled initially") + } + + err = tsg.EnableToolset("test-toolset") + if err != nil { + t.Errorf("Expected no error when enabling toolset, got: %v", err) + } + + if !tsg.IsEnabled("test-toolset") { + t.Error("Expected toolset to be enabled after EnableFeature call") + } + + // Test enabling already enabled toolset + err = tsg.EnableToolset("test-toolset") + if err != nil { + t.Errorf("Expected no error when enabling already enabled toolset, got: %v", err) + } +} + +func TestEnableToolsets(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Prepare toolsets + toolset1 := NewToolset("toolset1", "Feature 1") + toolset2 := NewToolset("toolset2", "Feature 2") + tsg.AddToolset(toolset1) + tsg.AddToolset(toolset2) + + // Test enabling multiple toolsets + err := tsg.EnableToolsets([]string{"toolset1", "toolset2"}) + if err != nil { + t.Errorf("Expected no error when enabling toolsets, got: %v", err) + } + + if !tsg.IsEnabled("toolset1") { + t.Error("Expected toolset1 to be enabled") + } + + if !tsg.IsEnabled("toolset2") { + t.Error("Expected toolset2 to be enabled") + } + + // Test with non-existent toolset in the list + err = tsg.EnableToolsets([]string{"toolset1", "non-existent"}) + if err == nil { + t.Error("Expected error when enabling list with non-existent toolset") + } + + // Test with empty list + err = tsg.EnableToolsets([]string{}) + if err != nil { + t.Errorf("Expected no error with empty toolset list, got: %v", err) + } + + // Test enabling everything through EnableToolsets + tsg = NewToolsetGroup(false) + err = tsg.EnableToolsets([]string{"all"}) + if err != nil { + t.Errorf("Expected no error when enabling 'all', got: %v", err) + } + + if !tsg.everythingOn { + t.Error("Expected everythingOn to be true after enabling 'all' via EnableToolsets") + } +} + +func TestEnableEverything(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Add a disabled toolset + testToolset := NewToolset("test-toolset", "A test toolset") + tsg.AddToolset(testToolset) + + // Verify it's disabled + if tsg.IsEnabled("test-toolset") { + t.Error("Expected toolset to be disabled initially") + } + + // Enable "all" + err := tsg.EnableToolsets([]string{"all"}) + if err != nil { + t.Errorf("Expected no error when enabling 'eall', got: %v", err) + } + + // Verify everythingOn was set + if !tsg.everythingOn { + t.Error("Expected everythingOn to be true after enabling 'eall'") + } + + // Verify the previously disabled toolset is now enabled + if !tsg.IsEnabled("test-toolset") { + t.Error("Expected toolset to be enabled when everythingOn is true") + } + + // Verify a non-existent toolset is also enabled + if !tsg.IsEnabled("non-existent") { + t.Error("Expected non-existent toolset to be enabled when everythingOn is true") + } +} + +func TestIsEnabledWithEverythingOn(t *testing.T) { + tsg := NewToolsetGroup(false) + + // Enable "everything" + err := tsg.EnableToolsets([]string{"all"}) + if err != nil { + t.Errorf("Expected no error when enabling 'all', got: %v", err) + } + + // Test that any toolset name returns true with IsEnabled + if !tsg.IsEnabled("some-toolset") { + t.Error("Expected IsEnabled to return true for any toolset when everythingOn is true") + } + + if !tsg.IsEnabled("another-toolset") { + t.Error("Expected IsEnabled to return true for any toolset when everythingOn is true") + } +} diff --git a/pkg/translations/translations.go b/pkg/translations/translations.go index 6d910525..741ee2b5 100644 --- a/pkg/translations/translations.go +++ b/pkg/translations/translations.go @@ -20,9 +20,6 @@ func TranslationHelper() (TranslationHelperFunc, func()) { var translationKeyMap = map[string]string{} v := viper.New() - v.SetEnvPrefix("GITHUB_MCP_") - v.AutomaticEnv() - // Load from JSON file v.SetConfigName("github-mcp-server-config") v.SetConfigType("json") diff --git a/script/licenses b/script/licenses index f231a458..c7f8ed4c 100755 --- a/script/licenses +++ b/script/licenses @@ -10,7 +10,7 @@ trap "rm -fr ${TEMPDIR}" EXIT for goos in linux darwin windows ; do # Note: we ignore warnings because we want the command to succeed, however the output should be checked - # for any new warnings, and potentially we may need to add licence information. + # for any new warnings, and potentially we may need to add license information. # # Normally these warnings are packages containing non go code, which may or may not require explicit attribution, # depending on the license. diff --git a/script/licenses-check b/script/licenses-check index 369277ca..5ad93027 100755 --- a/script/licenses-check +++ b/script/licenses-check @@ -4,7 +4,7 @@ go install github.com/google/go-licenses@latest for goos in linux darwin windows ; do # Note: we ignore warnings because we want the command to succeed, however the output should be checked - # for any new warnings, and potentially we may need to add licence information. + # for any new warnings, and potentially we may need to add license information. # # Normally these warnings are packages containing non go code, which may or may not require explicit attribution, # depending on the license. diff --git a/third-party-licenses.darwin.md b/third-party-licenses.darwin.md index 80c6d1c4..389bb966 100644 --- a/third-party-licenses.darwin.md +++ b/third-party-licenses.darwin.md @@ -13,7 +13,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/google/go-github/v69/github](https://pkg.go.dev/github.com/google/go-github/v69/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v69.2.0/LICENSE)) - [github.com/google/go-querystring/query](https://pkg.go.dev/github.com/google/go-querystring/query) ([BSD-3-Clause](https://github.com/google/go-querystring/blob/v1.1.0/LICENSE)) - [github.com/google/uuid](https://pkg.go.dev/github.com/google/uuid) ([BSD-3-Clause](https://github.com/google/uuid/blob/v1.6.0/LICENSE)) - - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.18.0/LICENSE)) + - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.20.1/LICENSE)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.3/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE)) - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE)) diff --git a/third-party-licenses.linux.md b/third-party-licenses.linux.md index 80c6d1c4..389bb966 100644 --- a/third-party-licenses.linux.md +++ b/third-party-licenses.linux.md @@ -13,7 +13,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/google/go-github/v69/github](https://pkg.go.dev/github.com/google/go-github/v69/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v69.2.0/LICENSE)) - [github.com/google/go-querystring/query](https://pkg.go.dev/github.com/google/go-querystring/query) ([BSD-3-Clause](https://github.com/google/go-querystring/blob/v1.1.0/LICENSE)) - [github.com/google/uuid](https://pkg.go.dev/github.com/google/uuid) ([BSD-3-Clause](https://github.com/google/uuid/blob/v1.6.0/LICENSE)) - - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.18.0/LICENSE)) + - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.20.1/LICENSE)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.3/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE)) - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE)) diff --git a/third-party-licenses.windows.md b/third-party-licenses.windows.md index 5fc973d7..96d037cc 100644 --- a/third-party-licenses.windows.md +++ b/third-party-licenses.windows.md @@ -14,7 +14,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/google/go-querystring/query](https://pkg.go.dev/github.com/google/go-querystring/query) ([BSD-3-Clause](https://github.com/google/go-querystring/blob/v1.1.0/LICENSE)) - [github.com/google/uuid](https://pkg.go.dev/github.com/google/uuid) ([BSD-3-Clause](https://github.com/google/uuid/blob/v1.6.0/LICENSE)) - [github.com/inconshreveable/mousetrap](https://pkg.go.dev/github.com/inconshreveable/mousetrap) ([Apache-2.0](https://github.com/inconshreveable/mousetrap/blob/v1.1.0/LICENSE)) - - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.18.0/LICENSE)) + - [github.com/mark3labs/mcp-go](https://pkg.go.dev/github.com/mark3labs/mcp-go) ([MIT](https://github.com/mark3labs/mcp-go/blob/v0.20.1/LICENSE)) - [github.com/pelletier/go-toml/v2](https://pkg.go.dev/github.com/pelletier/go-toml/v2) ([MIT](https://github.com/pelletier/go-toml/blob/v2.2.3/LICENSE)) - [github.com/sagikazarmark/locafero](https://pkg.go.dev/github.com/sagikazarmark/locafero) ([MIT](https://github.com/sagikazarmark/locafero/blob/v0.9.0/LICENSE)) - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE))