diff --git a/.github/workflows/close-inactive-issues.yml b/.github/workflows/close-inactive-issues.yml new file mode 100644 index 000000000..c9ece2b6f --- /dev/null +++ b/.github/workflows/close-inactive-issues.yml @@ -0,0 +1,28 @@ +name: Close inactive issues +on: + schedule: + - cron: "30 8 * * *" + +jobs: + close-issues: + runs-on: ubuntu-latest + env: + PR_DAYS_BEFORE_STALE: 60 + PR_DAYS_BEFORE_CLOSE: 10 + PR_STALE_LABEL: stale + permissions: + issues: write + pull-requests: write + steps: + - uses: actions/stale@v9 + with: + days-before-issue-stale: ${{ env.PR_DAYS_BEFORE_STALE }} + days-before-issue-close: ${{ env.PR_DAYS_BEFORE_CLOSE }} + stale-issue-label: ${{ env.PR_STALE_LABEL }} + stale-issue-message: "This issue is stale because it has been open for ${{ env.PR_DAYS_BEFORE_STALE }} days with no activity. Leave a comment to avoid closing this issue in ${{ env.PR_DAYS_BEFORE_CLOSE }} days." + close-issue-message: "This issue was closed because it has been inactive for ${{ env.PR_DAYS_BEFORE_CLOSE }} days since being marked as stale." + days-before-pr-stale: -1 + days-before-pr-close: -1 + # Start with the oldest items first + ascending: true + repo-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/README.md b/README.md index be9288e40..ce27bdb06 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,7 @@ To keep your GitHub PAT secure and reusable across different MCP hosts: - **Minimum scopes**: Only grant necessary permissions - `repo` - Repository operations - `read:packages` - Docker image access + - `read:org` - Organization team access - **Separate tokens**: Use different PATs for different projects/environments - **Regular rotation**: Update tokens periodically - **Never commit**: Keep tokens out of version control @@ -287,6 +288,7 @@ The following sets of tools are available (all are on by default): | `dependabot` | Dependabot tools | | `discussions` | GitHub Discussions related tools | | `experiments` | Experimental features that are not considered stable yet | +| `gists` | GitHub Gist related tools | | `issues` | GitHub Issues related tools | | `notifications` | GitHub Notifications related tools | | `orgs` | GitHub Organization related tools | @@ -420,6 +422,13 @@ The following sets of tools are available (all are on by default): - **get_me** - Get my user profile - No parameters required +- **get_team_members** - Get team members + - `org`: Organization login (owner) that contains the team. (string, required) + - `team_slug`: Team slug (string, required) + +- **get_teams** - Get teams + - `user`: Username to get teams for. If not provided, uses the authenticated user. (string, optional) +
@@ -457,7 +466,7 @@ The following sets of tools are available (all are on by default): - **list_discussion_categories** - List discussion categories - `owner`: Repository owner (string, required) - - `repo`: Repository name (string, required) + - `repo`: Repository name. If not provided, discussion categories will be queried at the organisation level. (string, optional) - **list_discussions** - List discussions - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) @@ -466,7 +475,31 @@ The following sets of tools are available (all are on by default): - `orderBy`: Order discussions by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `repo`: Repository name (string, required) + - `repo`: Repository name. If not provided, discussions will be queried at the organisation level. (string, optional) + +
+ +
+ +Gists + +- **create_gist** - Create Gist + - `content`: Content for simple single-file gist creation (string, required) + - `description`: Description of the gist (string, optional) + - `filename`: Filename for simple single-file gist creation (string, required) + - `public`: Whether the gist is public (boolean, optional) + +- **list_gists** - List Gists + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `since`: Only gists updated after this time (ISO 8601 timestamp) (string, optional) + - `username`: GitHub username (omit for authenticated user's gists) (string, optional) + +- **update_gist** - Update Gist + - `content`: Content for the file (string, required) + - `description`: Updated description of the gist (string, optional) + - `filename`: Filename to update or create (string, required) + - `gist_id`: ID of the gist to update (string, required)
@@ -513,16 +546,19 @@ The following sets of tools are available (all are on by default): - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `repo`: Repository name (string, required) +- **list_issue_types** - List available issue types + - `owner`: The organization owner of the repository (string, required) + - **list_issues** - List issues - - `direction`: Sort direction (string, optional) + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) + - `direction`: Order direction. If provided, the 'orderBy' also needs to be provided. (string, optional) - `labels`: Filter by labels (string[], optional) + - `orderBy`: Order issues by field. If provided, the 'direction' also needs to be provided. (string, optional) - `owner`: Repository owner (string, required) - - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `repo`: Repository name (string, required) - `since`: Filter by date (ISO 8601 timestamp) (string, optional) - - `sort`: Sort order (string, optional) - - `state`: Filter by state (string, optional) + - `state`: Filter by state, by default both open and closed issues are returned when not provided (string, optional) - **list_sub_issues** - List sub-issues - `issue_number`: Issue number (number, required) @@ -547,11 +583,11 @@ The following sets of tools are available (all are on by default): - **search_issues** - Search issues - `order`: Sort order (string, optional) - - `owner`: Optional repository owner. If provided with repo, only notifications for this repository are listed. (string, optional) + - `owner`: Optional repository owner. If provided with repo, only issues for this repository are listed. (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `query`: Search query using GitHub issues search syntax (string, required) - - `repo`: Optional repository name. If provided with owner, only notifications for this repository are listed. (string, optional) + - `repo`: Optional repository name. If provided with owner, only issues for this repository are listed. (string, optional) - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) - **update_issue** - Edit issue @@ -611,7 +647,7 @@ The following sets of tools are available (all are on by default): - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `query`: Search query using GitHub organizations search syntax scoped to type:org (string, required) + - `query`: Organization search query. Examples: 'microsoft', 'location:california', 'created:>=2025-01-01'. Search is automatically scoped to type:org. (string, required) - `sort`: Sort field by category (string, optional) @@ -719,11 +755,11 @@ The following sets of tools are available (all are on by default): - **search_pull_requests** - Search pull requests - `order`: Sort order (string, optional) - - `owner`: Optional repository owner. If provided with repo, only notifications for this repository are listed. (string, optional) + - `owner`: Optional repository owner. If provided with repo, only pull requests for this repository are listed. (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `query`: Search query using GitHub pull request search syntax (string, required) - - `repo`: Optional repository name. If provided with owner, only notifications for this repository are listed. (string, optional) + - `repo`: Optional repository name. If provided with owner, only pull requests for this repository are listed. (string, optional) - `sort`: Sort field by number of matches of categories, defaults to best match (string, optional) - **submit_pending_pull_request_review** - Submit the requester's latest pending pull request review @@ -736,10 +772,12 @@ The following sets of tools are available (all are on by default): - **update_pull_request** - Edit pull request - `base`: New base branch name (string, optional) - `body`: New description (string, optional) + - `draft`: Mark pull request as draft (true) or ready for review (false) (boolean, optional) - `maintainer_can_modify`: Allow maintainer edits (boolean, optional) - `owner`: Repository owner (string, required) - `pullNumber`: Pull request number to update (number, required) - `repo`: Repository name (string, required) + - `reviewers`: GitHub usernames to request reviews from (string[], optional) - `state`: New state (string, optional) - `title`: New title (string, optional) @@ -802,6 +840,10 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - `sha`: Accepts optional commit SHA. If specified, it will be used instead of ref (string, optional) +- **get_latest_release** - Get latest release + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - **get_tag** - Get tag details - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) @@ -821,6 +863,12 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - `sha`: Commit SHA, branch or tag name to list commits of. If not provided, uses the default branch of the repository. If a commit SHA is provided, will list commits up to that SHA. (string, optional) +- **list_releases** - List releases + - `owner`: Repository owner (string, required) + - `page`: Page number for pagination (min 1) (number, optional) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `repo`: Repository name (string, required) + - **list_tags** - List tags - `owner`: Repository owner (string, required) - `page`: Page number for pagination (min 1) (number, optional) @@ -835,16 +883,16 @@ The following sets of tools are available (all are on by default): - `repo`: Repository name (string, required) - **search_code** - Search code - - `order`: Sort order (string, optional) + - `order`: Sort order for results (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `query`: Search query using GitHub code search syntax (string, required) + - `query`: Search query using GitHub's powerful code search syntax. Examples: 'content:Skill language:Java org:github', 'NOT is:archived language:Python OR language:go', 'repo:github/github-mcp-server'. Supports exact matching, language filters, path filters, and more. (string, required) - `sort`: Sort field ('indexed' only) (string, optional) - **search_repositories** - Search repositories - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `query`: Search query (string, required) + - `query`: Repository search query. Examples: 'machine learning in:name stars:>1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering. (string, required) @@ -874,8 +922,8 @@ The following sets of tools are available (all are on by default): - `order`: Sort order (string, optional) - `page`: Page number for pagination (min 1) (number, optional) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - - `query`: Search query using GitHub users search syntax scoped to type:user (string, required) - - `sort`: Sort field by category (string, optional) + - `query`: User search query. Examples: 'john smith', 'location:seattle', 'followers:>100'. Search is automatically scoped to type:user. (string, required) + - `sort`: Sort users by number of followers or repositories, or when the person joined GitHub. (string, optional) diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 983ed4398..7fc62b1ae 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -13,7 +13,7 @@ import ( "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/toolsets" "github.com/github/github-mcp-server/pkg/translations" - gogithub "github.com/google/go-github/v73/github" + gogithub "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" diff --git a/docs/remote-server.md b/docs/remote-server.md index 49794c605..5f57f4961 100644 --- a/docs/remote-server.md +++ b/docs/remote-server.md @@ -25,6 +25,7 @@ Below is a table of available toolsets for the remote GitHub MCP Server. Each to | Dependabot | Dependabot tools | https://api.githubcopilot.com/mcp/x/dependabot | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-dependabot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdependabot%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/dependabot/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-dependabot&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdependabot%2Freadonly%22%7D) | | Discussions | GitHub Discussions related tools | https://api.githubcopilot.com/mcp/x/discussions | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-discussions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdiscussions%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/discussions/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-discussions&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fdiscussions%2Freadonly%22%7D) | | Experiments | Experimental features that are not considered stable yet | https://api.githubcopilot.com/mcp/x/experiments | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-experiments&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fexperiments%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/experiments/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-experiments&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fexperiments%2Freadonly%22%7D) | +| Gists | GitHub Gist related tools | https://api.githubcopilot.com/mcp/x/gists | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/gists/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-gists&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fgists%2Freadonly%22%7D) | | Issues | GitHub Issues related tools | https://api.githubcopilot.com/mcp/x/issues | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/issues/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-issues&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fissues%2Freadonly%22%7D) | | Notifications | GitHub Notifications related tools | https://api.githubcopilot.com/mcp/x/notifications | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/notifications/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-notifications&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Fnotifications%2Freadonly%22%7D) | | Organizations | GitHub Organization related tools | https://api.githubcopilot.com/mcp/x/orgs | [Install](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%22%7D) | [read-only](https://api.githubcopilot.com/mcp/x/orgs/readonly) | [Install read-only](https://insiders.vscode.dev/redirect/mcp/install?name=gh-orgs&config=%7B%22type%22%3A%20%22http%22%2C%22url%22%3A%20%22https%3A%2F%2Fapi.githubcopilot.com%2Fmcp%2Fx%2Forgs%2Freadonly%22%7D) | diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 64c5729ba..24cfc7096 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -18,7 +18,7 @@ import ( "github.com/github/github-mcp-server/internal/ghmcp" "github.com/github/github-mcp-server/pkg/github" "github.com/github/github-mcp-server/pkg/translations" - gogithub "github.com/google/go-github/v73/github" + gogithub "github.com/google/go-github/v74/github" mcpClient "github.com/mark3labs/mcp-go/client" "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/require" diff --git a/github-mcp-server b/github-mcp-server new file mode 100755 index 000000000..864242c24 Binary files /dev/null and b/github-mcp-server differ diff --git a/go.mod b/go.mod index 3df6bf3d5..a2a86feaf 100644 --- a/go.mod +++ b/go.mod @@ -3,11 +3,10 @@ module github.com/github/github-mcp-server go 1.23.7 require ( - github.com/google/go-github/v73 v73.0.0 + github.com/google/go-github/v74 v74.0.0 github.com/josephburnett/jd v1.9.2 github.com/mark3labs/mcp-go v0.32.0 github.com/migueleliasweb/go-github-mock v1.3.0 - github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.9.1 github.com/spf13/viper v1.20.1 github.com/stretchr/testify v1.10.0 diff --git a/go.sum b/go.sum index d77cdf0d9..33395e6bb 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-github/v71 v71.0.0 h1:Zi16OymGKZZMm8ZliffVVJ/Q9YZreDKONCr+WUd0Z30= github.com/google/go-github/v71 v71.0.0/go.mod h1:URZXObp2BLlMjwu0O8g4y6VBneUj2bCHgnI8FfgZ51M= -github.com/google/go-github/v73 v73.0.0 h1:aR+Utnh+Y4mMkS+2qLQwcQ/cF9mOTpdwnzlaw//rG24= -github.com/google/go-github/v73 v73.0.0/go.mod h1:fa6w8+/V+edSU0muqdhCVY7Beh1M8F1IlQPZIANKIYw= +github.com/google/go-github/v74 v74.0.0 h1:yZcddTUn8DPbj11GxnMrNiAnXH14gNs559AsUpNpPgM= +github.com/google/go-github/v74 v74.0.0/go.mod h1:ubn/YdyftV80VPSI26nSJvaEsTOnsjrxG3o9kJhcyak= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -66,8 +66,6 @@ github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7 h1:cYCy18SHPKRkv github.com/shurcooL/githubv4 v0.0.0-20240727222349-48295856cce7/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE= -github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= -github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= github.com/sourcegraph/conc v0.3.0/go.mod h1:Sdozi7LEKbFPqYX2/J+iBAM6HpqSLTASQIKqDmF7Mt0= github.com/spf13/afero v1.14.0 h1:9tH6MapGnn/j0eb0yIXiLjERO8RB6xIVZRDCX7PtqWA= @@ -83,7 +81,6 @@ github.com/spf13/viper v1.20.1/go.mod h1:P9Mdzt1zoHIG8m2eZQinpiBjo6kCmZSKBClNNqj github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= @@ -98,7 +95,6 @@ golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98= golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.31.0 h1:ioabZlmFYtWhL+TRYpcnNlLwhyxaM9kWTDEmfnprqik= golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index d993b130a..5fb9582b9 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "log" + "log/slog" "net/http" "net/url" "os" @@ -17,11 +18,10 @@ import ( mcplog "github.com/github/github-mcp-server/pkg/log" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" - gogithub "github.com/google/go-github/v73/github" + gogithub "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" - "github.com/sirupsen/logrus" ) type MCPServerConfig struct { @@ -49,6 +49,8 @@ type MCPServerConfig struct { Translator translations.TranslationHelperFunc } +const stdioServerLogPrefix = "stdioserver" + func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { apiHost, err := parseAPIHost(cfg.Host) if err != nil { @@ -203,17 +205,22 @@ func RunStdioServer(cfg StdioServerConfig) error { stdioServer := server.NewStdioServer(ghServer) - logrusLogger := logrus.New() + var slogHandler slog.Handler + var logOutput io.Writer if cfg.LogFilePath != "" { file, err := os.OpenFile(cfg.LogFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { return fmt.Errorf("failed to open log file: %w", err) } - - logrusLogger.SetLevel(logrus.DebugLevel) - logrusLogger.SetOutput(file) - } - stdLogger := log.New(logrusLogger.Writer(), "stdioserver", 0) + logOutput = file + slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelDebug}) + } else { + logOutput = os.Stderr + slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelInfo}) + } + logger := slog.New(slogHandler) + logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly) + stdLogger := log.New(logOutput, stdioServerLogPrefix, 0) stdioServer.SetErrorLogger(stdLogger) if cfg.ExportTranslations { @@ -227,7 +234,7 @@ func RunStdioServer(cfg StdioServerConfig) error { in, out := io.Reader(os.Stdin), io.Writer(os.Stdout) if cfg.EnableCommandLogging { - loggedIO := mcplog.NewIOLogger(in, out, logrusLogger) + loggedIO := mcplog.NewIOLogger(in, out, logger) in, out = loggedIO, loggedIO } // enable GitHub errors in the context @@ -241,9 +248,10 @@ func RunStdioServer(cfg StdioServerConfig) error { // Wait for shutdown signal select { case <-ctx.Done(): - logrusLogger.Infof("shutting down server...") + logger.Info("shutting down server", "signal", "context done") case err := <-errC: if err != nil { + logger.Error("error running server", "error", err) return fmt.Errorf("error running server: %w", err) } } diff --git a/pkg/errors/error.go b/pkg/errors/error.go index c89ab2d79..1e15021d2 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" ) diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index 3498e3d8a..6f7fc0a3e 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -6,7 +6,7 @@ import ( "net/http" "testing" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/pkg/github/__toolsnaps__/get_team_members.snap b/pkg/github/__toolsnaps__/get_team_members.snap new file mode 100644 index 000000000..2d91bb5ea --- /dev/null +++ b/pkg/github/__toolsnaps__/get_team_members.snap @@ -0,0 +1,25 @@ +{ + "annotations": { + "title": "Get team members", + "readOnlyHint": true + }, + "description": "Get member usernames of a specific team in an organization. Limited to organizations accessible with current credentials", + "inputSchema": { + "properties": { + "org": { + "description": "Organization login (owner) that contains the team.", + "type": "string" + }, + "team_slug": { + "description": "Team slug", + "type": "string" + } + }, + "required": [ + "org", + "team_slug" + ], + "type": "object" + }, + "name": "get_team_members" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/get_teams.snap b/pkg/github/__toolsnaps__/get_teams.snap new file mode 100644 index 000000000..39ed4db35 --- /dev/null +++ b/pkg/github/__toolsnaps__/get_teams.snap @@ -0,0 +1,17 @@ +{ + "annotations": { + "title": "Get teams", + "readOnlyHint": true + }, + "description": "Get details of the teams the user is a member of. Limited to organizations accessible with current credentials", + "inputSchema": { + "properties": { + "user": { + "description": "Username to get teams for. If not provided, uses the authenticated user.", + "type": "string" + } + }, + "type": "object" + }, + "name": "get_teams" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_issue_types.snap b/pkg/github/__toolsnaps__/list_issue_types.snap new file mode 100644 index 000000000..93c3e51d9 --- /dev/null +++ b/pkg/github/__toolsnaps__/list_issue_types.snap @@ -0,0 +1,20 @@ +{ + "annotations": { + "title": "List available issue types", + "readOnlyHint": true + }, + "description": "List supported issue types for repository owner (organization).", + "inputSchema": { + "properties": { + "owner": { + "description": "The organization owner of the repository", + "type": "string" + } + }, + "required": [ + "owner" + ], + "type": "object" + }, + "name": "list_issue_types" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/list_issues.snap b/pkg/github/__toolsnaps__/list_issues.snap index 4fe155f09..5475988c2 100644 --- a/pkg/github/__toolsnaps__/list_issues.snap +++ b/pkg/github/__toolsnaps__/list_issues.snap @@ -3,14 +3,18 @@ "title": "List issues", "readOnlyHint": true }, - "description": "List issues in a GitHub repository.", + "description": "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.", "inputSchema": { "properties": { + "after": { + "description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.", + "type": "string" + }, "direction": { - "description": "Sort direction", + "description": "Order direction. If provided, the 'orderBy' also needs to be provided.", "enum": [ - "asc", - "desc" + "ASC", + "DESC" ], "type": "string" }, @@ -21,15 +25,19 @@ }, "type": "array" }, + "orderBy": { + "description": "Order issues by field. If provided, the 'direction' also needs to be provided.", + "enum": [ + "CREATED_AT", + "UPDATED_AT", + "COMMENTS" + ], + "type": "string" + }, "owner": { "description": "Repository owner", "type": "string" }, - "page": { - "description": "Page number for pagination (min 1)", - "minimum": 1, - "type": "number" - }, "perPage": { "description": "Results per page for pagination (min 1, max 100)", "maximum": 100, @@ -44,21 +52,11 @@ "description": "Filter by date (ISO 8601 timestamp)", "type": "string" }, - "sort": { - "description": "Sort order", - "enum": [ - "created", - "updated", - "comments" - ], - "type": "string" - }, "state": { - "description": "Filter by state", + "description": "Filter by state, by default both open and closed issues are returned when not provided", "enum": [ - "open", - "closed", - "all" + "OPEN", + "CLOSED" ], "type": "string" } diff --git a/pkg/github/__toolsnaps__/search_code.snap b/pkg/github/__toolsnaps__/search_code.snap index e341f3e38..4ef40c5f8 100644 --- a/pkg/github/__toolsnaps__/search_code.snap +++ b/pkg/github/__toolsnaps__/search_code.snap @@ -3,11 +3,11 @@ "title": "Search code", "readOnlyHint": true }, - "description": "Search for code across GitHub repositories", + "description": "Fast and precise code search across ALL GitHub repositories using GitHub's native search engine. Best for finding exact symbols, functions, classes, or specific code patterns.", "inputSchema": { "properties": { "order": { - "description": "Sort order", + "description": "Sort order for results", "enum": [ "asc", "desc" @@ -26,7 +26,7 @@ "type": "number" }, "query": { - "description": "Search query using GitHub code search syntax", + "description": "Search query using GitHub's powerful code search syntax. Examples: 'content:Skill language:Java org:github', 'NOT is:archived language:Python OR language:go', 'repo:github/github-mcp-server'. Supports exact matching, language filters, path filters, and more.", "type": "string" }, "sort": { diff --git a/pkg/github/__toolsnaps__/search_issues.snap b/pkg/github/__toolsnaps__/search_issues.snap index 7db502d94..bf1982411 100644 --- a/pkg/github/__toolsnaps__/search_issues.snap +++ b/pkg/github/__toolsnaps__/search_issues.snap @@ -15,7 +15,7 @@ "type": "string" }, "owner": { - "description": "Optional repository owner. If provided with repo, only notifications for this repository are listed.", + "description": "Optional repository owner. If provided with repo, only issues for this repository are listed.", "type": "string" }, "page": { @@ -34,7 +34,7 @@ "type": "string" }, "repo": { - "description": "Optional repository name. If provided with owner, only notifications for this repository are listed.", + "description": "Optional repository name. If provided with owner, only issues for this repository are listed.", "type": "string" }, "sort": { diff --git a/pkg/github/__toolsnaps__/search_pull_requests.snap b/pkg/github/__toolsnaps__/search_pull_requests.snap index 6a8d8e0e6..811aa1322 100644 --- a/pkg/github/__toolsnaps__/search_pull_requests.snap +++ b/pkg/github/__toolsnaps__/search_pull_requests.snap @@ -15,7 +15,7 @@ "type": "string" }, "owner": { - "description": "Optional repository owner. If provided with repo, only notifications for this repository are listed.", + "description": "Optional repository owner. If provided with repo, only pull requests for this repository are listed.", "type": "string" }, "page": { @@ -34,7 +34,7 @@ "type": "string" }, "repo": { - "description": "Optional repository name. If provided with owner, only notifications for this repository are listed.", + "description": "Optional repository name. If provided with owner, only pull requests for this repository are listed.", "type": "string" }, "sort": { diff --git a/pkg/github/__toolsnaps__/search_repositories.snap b/pkg/github/__toolsnaps__/search_repositories.snap index b6b6d1d44..d283a2cc0 100644 --- a/pkg/github/__toolsnaps__/search_repositories.snap +++ b/pkg/github/__toolsnaps__/search_repositories.snap @@ -3,7 +3,7 @@ "title": "Search repositories", "readOnlyHint": true }, - "description": "Search for GitHub repositories", + "description": "Find GitHub repositories by name, description, readme, topics, or other metadata. Perfect for discovering projects, finding examples, or locating specific repositories across GitHub.", "inputSchema": { "properties": { "page": { @@ -18,7 +18,7 @@ "type": "number" }, "query": { - "description": "Search query", + "description": "Repository search query. Examples: 'machine learning in:name stars:\u003e1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering.", "type": "string" } }, diff --git a/pkg/github/__toolsnaps__/search_users.snap b/pkg/github/__toolsnaps__/search_users.snap index 5cf9796f2..73ff7a43c 100644 --- a/pkg/github/__toolsnaps__/search_users.snap +++ b/pkg/github/__toolsnaps__/search_users.snap @@ -3,7 +3,7 @@ "title": "Search users", "readOnlyHint": true }, - "description": "Search for GitHub users exclusively", + "description": "Find GitHub users by username, real name, or other profile information. Useful for locating developers, contributors, or team members.", "inputSchema": { "properties": { "order": { @@ -26,11 +26,11 @@ "type": "number" }, "query": { - "description": "Search query using GitHub users search syntax scoped to type:user", + "description": "User search query. Examples: 'john smith', 'location:seattle', 'followers:\u003e100'. Search is automatically scoped to type:user.", "type": "string" }, "sort": { - "description": "Sort field by category", + "description": "Sort users by number of followers or repositories, or when the person joined GitHub.", "enum": [ "followers", "repositories", diff --git a/pkg/github/__toolsnaps__/update_pull_request.snap b/pkg/github/__toolsnaps__/update_pull_request.snap index 765983afd..25170ed5f 100644 --- a/pkg/github/__toolsnaps__/update_pull_request.snap +++ b/pkg/github/__toolsnaps__/update_pull_request.snap @@ -14,6 +14,10 @@ "description": "New description", "type": "string" }, + "draft": { + "description": "Mark pull request as draft (true) or ready for review (false)", + "type": "boolean" + }, "maintainer_can_modify": { "description": "Allow maintainer edits", "type": "boolean" @@ -30,6 +34,13 @@ "description": "Repository name", "type": "string" }, + "reviewers": { + "description": "GitHub usernames to request reviews from", + "items": { + "type": "string" + }, + "type": "array" + }, "state": { "description": "New state", "enum": [ diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 19b56389c..38719f155 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -11,7 +11,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) @@ -955,7 +955,9 @@ func CancelWorkflowRun(getClient GetClientFn, t translations.TranslationHelperFu resp, err := client.Actions.CancelWorkflowRunByID(ctx, owner, repo, runID) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to cancel workflow run", resp, err), nil + if _, ok := err.(*github.AcceptedError); !ok { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to cancel workflow run", resp, err), nil + } } defer func() { _ = resp.Body.Close() }() diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index cb33cbe6b..3d7521125 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -323,12 +323,14 @@ func Test_CancelWorkflowRun(t *testing.T) { { name: "successful workflow run cancellation", mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatch( + mock.WithRequestMatchHandler( mock.EndpointPattern{ Pattern: "/repos/owner/repo/actions/runs/12345/cancel", Method: "POST", }, - "", // Empty response body for 202 Accepted + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusAccepted) + }), ), ), requestArgs: map[string]any{ @@ -338,6 +340,27 @@ func Test_CancelWorkflowRun(t *testing.T) { }, expectError: false, }, + { + name: "conflict when cancelling a workflow run", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/repos/owner/repo/actions/runs/12345/cancel", + Method: "POST", + }, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusConflict) + }), + ), + ), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "run_id": float64(12345), + }, + expectError: true, + expectedErrMsg: "failed to cancel workflow run", + }, { name: "missing required parameter run_id", mockedClient: mock.NewMockedHTTPClient(), @@ -369,7 +392,7 @@ func Test_CancelWorkflowRun(t *testing.T) { textContent := getTextResult(t, result) if tc.expectedErrMsg != "" { - assert.Equal(t, tc.expectedErrMsg, textContent.Text) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) return } diff --git a/pkg/github/code_scanning.go b/pkg/github/code_scanning.go index 6b15c0c45..47eaa4be0 100644 --- a/pkg/github/code_scanning.go +++ b/pkg/github/code_scanning.go @@ -9,7 +9,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/code_scanning_test.go b/pkg/github/code_scanning_test.go index 66f6fd6cc..5d4cc732d 100644 --- a/pkg/github/code_scanning_test.go +++ b/pkg/github/code_scanning_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 9817fea7b..06642aa15 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -8,6 +8,7 @@ import ( "github.com/github/github-mcp-server/pkg/translations" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" + "github.com/shurcooL/githubv4" ) // UserDetails contains additional fields about a GitHub user not already @@ -90,3 +91,161 @@ func GetMe(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Too return tool, handler } + +type TeamInfo struct { + Name string `json:"name"` + Slug string `json:"slug"` + Description string `json:"description"` +} + +type OrganizationTeams struct { + Org string `json:"org"` + Teams []TeamInfo `json:"teams"` +} + +func GetTeams(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("get_teams", + mcp.WithDescription(t("TOOL_GET_TEAMS_DESCRIPTION", "Get details of the teams the user is a member of. Limited to organizations accessible with current credentials")), + mcp.WithString("user", + mcp.Description(t("TOOL_GET_TEAMS_USER_DESCRIPTION", "Username to get teams for. If not provided, uses the authenticated user.")), + ), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_TEAMS_TITLE", "Get teams"), + ReadOnlyHint: ToBoolPtr(true), + }), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + user, err := OptionalParam[string](request, "user") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + var username string + if user != "" { + username = user + } else { + client, err := getClient(ctx) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to get GitHub client", err), nil + } + + userResp, res, err := client.Users.Get(ctx, "") + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to get user", + res, + err, + ), nil + } + username = userResp.GetLogin() + } + + gqlClient, err := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil + } + + var q struct { + User struct { + Organizations struct { + Nodes []struct { + Login githubv4.String + Teams struct { + Nodes []struct { + Name githubv4.String + Slug githubv4.String + Description githubv4.String + } + } `graphql:"teams(first: 100, userLogins: [$login])"` + } + } `graphql:"organizations(first: 100)"` + } `graphql:"user(login: $login)"` + } + vars := map[string]interface{}{ + "login": githubv4.String(username), + } + if err := gqlClient.Query(ctx, &q, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to find teams", err), nil + } + + var organizations []OrganizationTeams + for _, org := range q.User.Organizations.Nodes { + orgTeams := OrganizationTeams{ + Org: string(org.Login), + Teams: make([]TeamInfo, 0, len(org.Teams.Nodes)), + } + + for _, team := range org.Teams.Nodes { + orgTeams.Teams = append(orgTeams.Teams, TeamInfo{ + Name: string(team.Name), + Slug: string(team.Slug), + Description: string(team.Description), + }) + } + + organizations = append(organizations, orgTeams) + } + + return MarshalledTextResult(organizations), nil + } +} + +func GetTeamMembers(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { + return mcp.NewTool("get_team_members", + mcp.WithDescription(t("TOOL_GET_TEAM_MEMBERS_DESCRIPTION", "Get member usernames of a specific team in an organization. Limited to organizations accessible with current credentials")), + mcp.WithString("org", + mcp.Description(t("TOOL_GET_TEAM_MEMBERS_ORG_DESCRIPTION", "Organization login (owner) that contains the team.")), + mcp.Required(), + ), + mcp.WithString("team_slug", + mcp.Description(t("TOOL_GET_TEAM_MEMBERS_TEAM_SLUG_DESCRIPTION", "Team slug")), + mcp.Required(), + ), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_TEAM_MEMBERS_TITLE", "Get team members"), + ReadOnlyHint: ToBoolPtr(true), + }), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + org, err := RequiredParam[string](request, "org") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + teamSlug, err := RequiredParam[string](request, "team_slug") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + gqlClient, err := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil + } + + var q struct { + Organization struct { + Team struct { + Members struct { + Nodes []struct { + Login githubv4.String + } + } `graphql:"members(first: 100)"` + } `graphql:"team(slug: $teamSlug)"` + } `graphql:"organization(login: $org)"` + } + vars := map[string]interface{}{ + "org": githubv4.String(org), + "teamSlug": githubv4.String(teamSlug), + } + if err := gqlClient.Query(ctx, &q, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to get team members", err), nil + } + + var members []string + for _, member := range q.Organization.Team.Members.Nodes { + members = append(members, string(member.Login)) + } + + return MarshalledTextResult(members), nil + } +} diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 56f61e936..641707a47 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -3,13 +3,16 @@ package github import ( "context" "encoding/json" + "fmt" "testing" "time" + "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -139,3 +142,358 @@ func Test_GetMe(t *testing.T) { }) } } + +func Test_GetTeams(t *testing.T) { + t.Parallel() + + tool, _ := GetTeams(nil, nil, translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "get_teams", tool.Name) + assert.True(t, *tool.Annotations.ReadOnlyHint, "get_teams tool should be read-only") + + 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"), + Hireable: github.Ptr(true), + TwitterUsername: github.Ptr("testuser_twitter"), + Plan: &github.Plan{ + Name: github.Ptr("pro"), + }, + } + + mockTeamsResponse := githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "organizations": map[string]any{ + "nodes": []map[string]any{ + { + "login": "testorg1", + "teams": map[string]any{ + "nodes": []map[string]any{ + { + "name": "team1", + "slug": "team1", + "description": "Team 1", + }, + { + "name": "team2", + "slug": "team2", + "description": "Team 2", + }, + }, + }, + }, + { + "login": "testorg2", + "teams": map[string]any{ + "nodes": []map[string]any{ + { + "name": "team3", + "slug": "team3", + "description": "Team 3", + }, + }, + }, + }, + }, + }, + }, + }) + + mockNoTeamsResponse := githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "organizations": map[string]any{ + "nodes": []map[string]any{}, + }, + }, + }) + + tests := []struct { + name string + stubbedGetClientFn GetClientFn + stubbedGetGQLClientFn GetGQLClientFn + requestArgs map[string]any + expectToolError bool + expectedToolErrMsg string + expectedTeamsCount int + }{ + { + name: "successful get teams", + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUser, + mockUser, + ), + ), + ), + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + queryStr := "query($login:String!){user(login: $login){organizations(first: 100){nodes{login,teams(first: 100, userLogins: [$login]){nodes{name,slug,description}}}}}}" + vars := map[string]interface{}{ + "login": "testuser", + } + matcher := githubv4mock.NewQueryMatcher(queryStr, vars, mockTeamsResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient), nil + }, + requestArgs: map[string]any{}, + expectToolError: false, + expectedTeamsCount: 2, + }, + { + name: "successful get teams for specific user", + stubbedGetClientFn: nil, + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + queryStr := "query($login:String!){user(login: $login){organizations(first: 100){nodes{login,teams(first: 100, userLogins: [$login]){nodes{name,slug,description}}}}}}" + vars := map[string]interface{}{ + "login": "specificuser", + } + matcher := githubv4mock.NewQueryMatcher(queryStr, vars, mockTeamsResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient), nil + }, + requestArgs: map[string]any{ + "user": "specificuser", + }, + expectToolError: false, + expectedTeamsCount: 2, + }, + { + name: "no teams found", + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUser, + mockUser, + ), + ), + ), + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + queryStr := "query($login:String!){user(login: $login){organizations(first: 100){nodes{login,teams(first: 100, userLogins: [$login]){nodes{name,slug,description}}}}}}" + vars := map[string]interface{}{ + "login": "testuser", + } + matcher := githubv4mock.NewQueryMatcher(queryStr, vars, mockNoTeamsResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient), nil + }, + requestArgs: map[string]any{}, + expectToolError: false, + expectedTeamsCount: 0, + }, + { + name: "getting client fails", + stubbedGetClientFn: stubGetClientFnErr("expected test error"), + stubbedGetGQLClientFn: nil, + requestArgs: map[string]any{}, + expectToolError: true, + expectedToolErrMsg: "failed to get GitHub client: expected test error", + }, + { + name: "get user fails", + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUser, + badRequestHandler("expected test failure"), + ), + ), + ), + stubbedGetGQLClientFn: nil, + requestArgs: map[string]any{}, + expectToolError: true, + expectedToolErrMsg: "expected test failure", + }, + { + name: "getting GraphQL client fails", + stubbedGetClientFn: stubGetClientFromHTTPFn( + mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetUser, + mockUser, + ), + ), + ), + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + return nil, fmt.Errorf("GraphQL client error") + }, + requestArgs: map[string]any{}, + expectToolError: true, + expectedToolErrMsg: "failed to get GitHub GQL client: GraphQL client error", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, handler := GetTeams(tc.stubbedGetClientFn, tc.stubbedGetGQLClientFn, translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(context.Background(), request) + require.NoError(t, err) + textContent := getTextResult(t, result) + + if tc.expectToolError { + assert.True(t, result.IsError, "expected tool call result to be an error") + assert.Contains(t, textContent.Text, tc.expectedToolErrMsg) + return + } + + var organizations []OrganizationTeams + err = json.Unmarshal([]byte(textContent.Text), &organizations) + require.NoError(t, err) + + assert.Len(t, organizations, tc.expectedTeamsCount) + + if tc.expectedTeamsCount > 0 { + assert.Equal(t, "testorg1", organizations[0].Org) + assert.Len(t, organizations[0].Teams, 2) + assert.Equal(t, "team1", organizations[0].Teams[0].Name) + assert.Equal(t, "team1", organizations[0].Teams[0].Slug) + assert.Equal(t, "Team 1", organizations[0].Teams[0].Description) + + if tc.expectedTeamsCount > 1 { + assert.Equal(t, "testorg2", organizations[1].Org) + assert.Len(t, organizations[1].Teams, 1) + assert.Equal(t, "team3", organizations[1].Teams[0].Name) + assert.Equal(t, "team3", organizations[1].Teams[0].Slug) + assert.Equal(t, "Team 3", organizations[1].Teams[0].Description) + } + } + }) + } +} + +func Test_GetTeamMembers(t *testing.T) { + t.Parallel() + + tool, _ := GetTeamMembers(nil, translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "get_team_members", tool.Name) + assert.True(t, *tool.Annotations.ReadOnlyHint, "get_team_members tool should be read-only") + + mockTeamMembersResponse := githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "team": map[string]any{ + "members": map[string]any{ + "nodes": []map[string]any{ + { + "login": "user1", + }, + { + "login": "user2", + }, + }, + }, + }, + }, + }) + + mockNoMembersResponse := githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "team": map[string]any{ + "members": map[string]any{ + "nodes": []map[string]any{}, + }, + }, + }, + }) + + tests := []struct { + name string + stubbedGetGQLClientFn GetGQLClientFn + requestArgs map[string]any + expectToolError bool + expectedToolErrMsg string + expectedMembersCount int + }{ + { + name: "successful get team members", + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + queryStr := "query($org:String!$teamSlug:String!){organization(login: $org){team(slug: $teamSlug){members(first: 100){nodes{login}}}}}" + vars := map[string]interface{}{ + "org": "testorg", + "teamSlug": "testteam", + } + matcher := githubv4mock.NewQueryMatcher(queryStr, vars, mockTeamMembersResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient), nil + }, + requestArgs: map[string]any{ + "org": "testorg", + "team_slug": "testteam", + }, + expectToolError: false, + expectedMembersCount: 2, + }, + { + name: "team with no members", + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + queryStr := "query($org:String!$teamSlug:String!){organization(login: $org){team(slug: $teamSlug){members(first: 100){nodes{login}}}}}" + vars := map[string]interface{}{ + "org": "testorg", + "teamSlug": "emptyteam", + } + matcher := githubv4mock.NewQueryMatcher(queryStr, vars, mockNoMembersResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + return githubv4.NewClient(httpClient), nil + }, + requestArgs: map[string]any{ + "org": "testorg", + "team_slug": "emptyteam", + }, + expectToolError: false, + expectedMembersCount: 0, + }, + { + name: "getting GraphQL client fails", + stubbedGetGQLClientFn: func(_ context.Context) (*githubv4.Client, error) { + return nil, fmt.Errorf("GraphQL client error") + }, + requestArgs: map[string]any{ + "org": "testorg", + "team_slug": "testteam", + }, + expectToolError: true, + expectedToolErrMsg: "failed to get GitHub GQL client: GraphQL client error", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, handler := GetTeamMembers(tc.stubbedGetGQLClientFn, translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(context.Background(), request) + require.NoError(t, err) + textContent := getTextResult(t, result) + + if tc.expectToolError { + assert.True(t, result.IsError, "expected tool call result to be an error") + assert.Contains(t, textContent.Text, tc.expectedToolErrMsg) + return + } + + var members []string + err = json.Unmarshal([]byte(textContent.Text), &members) + require.NoError(t, err) + + assert.Len(t, members, tc.expectedMembersCount) + + if tc.expectedMembersCount > 0 { + assert.Equal(t, "user1", members[0]) + + if tc.expectedMembersCount > 1 { + assert.Equal(t, "user2", members[1]) + } + } + }) + } +} diff --git a/pkg/github/dependabot.go b/pkg/github/dependabot.go index c2a4d5b0d..7b327cd77 100644 --- a/pkg/github/dependabot.go +++ b/pkg/github/dependabot.go @@ -9,7 +9,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/dependabot_test.go b/pkg/github/dependabot_test.go index 8a7270d7f..c3ec0408d 100644 --- a/pkg/github/dependabot_test.go +++ b/pkg/github/dependabot_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index fce07ecdb..dc26063fd 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -7,7 +7,7 @@ import ( "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" @@ -119,7 +119,7 @@ func getQueryType(useOrdering bool, categoryID *githubv4.ID) any { func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussions", - mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository")), + mcp.WithDescription(t("TOOL_LIST_DISCUSSIONS_DESCRIPTION", "List discussions for a repository or organisation.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_DISCUSSIONS_USER_TITLE", "List discussions"), ReadOnlyHint: ToBoolPtr(true), @@ -129,8 +129,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp mcp.Description("Repository owner"), ), mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), + mcp.Description("Repository name. If not provided, discussions will be queried at the organisation level."), ), mcp.WithString("category", mcp.Description("Optional filter by discussion category ID. If provided, only discussions with this category are listed."), @@ -150,10 +149,15 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp if err != nil { return mcp.NewToolResultError(err.Error()), nil } - repo, err := RequiredParam[string](request, "repo") + repo, err := OptionalParam[string](request, "repo") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // when not provided, default to the .github repository + // this will query discussions at the organisation level + if repo == "" { + repo = ".github" + } category, err := OptionalParam[string](request, "category") if err != nil { @@ -291,6 +295,7 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper Repository struct { Discussion struct { Number githubv4.Int + Title githubv4.String Body githubv4.String CreatedAt githubv4.DateTime URL githubv4.String `graphql:"url"` @@ -311,6 +316,7 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper d := q.Repository.Discussion discussion := &github.Discussion{ Number: github.Ptr(int(d.Number)), + Title: github.Ptr(string(d.Title)), Body: github.Ptr(string(d.Body)), HTMLURL: github.Ptr(string(d.URL)), CreatedAt: &github.Timestamp{Time: d.CreatedAt.Time}, @@ -437,7 +443,7 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_discussion_categories", - mcp.WithDescription(t("TOOL_LIST_DISCUSSION_CATEGORIES_DESCRIPTION", "List discussion categories with their id and name, for a repository")), + mcp.WithDescription(t("TOOL_LIST_DISCUSSION_CATEGORIES_DESCRIPTION", "List discussion categories with their id and name, for a repository or organisation.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_DISCUSSION_CATEGORIES_USER_TITLE", "List discussion categories"), ReadOnlyHint: ToBoolPtr(true), @@ -447,19 +453,23 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl mcp.Description("Repository owner"), ), mcp.WithString("repo", - mcp.Required(), - mcp.Description("Repository name"), + mcp.Description("Repository name. If not provided, discussion categories will be queried at the organisation level."), ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - // Decode params - var params struct { - Owner string - Repo string + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil } - if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { + repo, err := OptionalParam[string](request, "repo") + if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // when not provided, default to the .github repository + // this will query discussion categories at the organisation level + if repo == "" { + repo = ".github" + } client, err := getGQLClient(ctx) if err != nil { @@ -484,8 +494,8 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String(params.Owner), - "repo": githubv4.String(params.Repo), + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), "first": githubv4.Int(25), } if err := client.Query(ctx, &q, vars); err != nil { diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index aefaf2f8c..beef2effe 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -9,7 +9,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -50,6 +50,45 @@ var ( }, } + discussionsOrgLevel = []map[string]any{ + { + "number": 1, + "title": "Org Discussion 1 - Community Guidelines", + "createdAt": "2023-01-15T00:00:00Z", + "updatedAt": "2023-01-15T00:00:00Z", + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/1", + "category": map[string]any{"name": "Announcements"}, + }, + { + "number": 2, + "title": "Org Discussion 2 - Roadmap 2023", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/2", + "category": map[string]any{"name": "General"}, + }, + { + "number": 3, + "title": "Org Discussion 3 - Roadmap 2024", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/3", + "category": map[string]any{"name": "General"}, + }, + { + "number": 4, + "title": "Org Discussion 4 - Roadmap 2025", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/4", + "category": map[string]any{"name": "General"}, + }, + } + // Ordered mock responses discussionsOrderedCreatedAsc = []map[string]any{ discussionsAll[0], // Discussion 1 (created 2023-01-01) @@ -139,6 +178,22 @@ var ( }, }, }) + + mockResponseOrgLevel = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{ + "nodes": discussionsOrgLevel, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 4, + }, + }, + }) + mockErrorRepoNotFound = githubv4mock.ErrorResponse("repository not found") ) @@ -151,7 +206,7 @@ func Test_ListDiscussions(t *testing.T) { assert.Contains(t, toolDef.InputSchema.Properties, "repo") assert.Contains(t, toolDef.InputSchema.Properties, "orderBy") assert.Contains(t, toolDef.InputSchema.Properties, "direction") - assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"}) + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner"}) // Variables matching what GraphQL receives after JSON marshaling/unmarshaling varsListAll := map[string]interface{}{ @@ -204,6 +259,13 @@ func Test_ListDiscussions(t *testing.T) { "after": (*string)(nil), } + varsOrgLevel := map[string]interface{}{ + "owner": "owner", + "repo": ".github", // This is what gets set when repo is not provided + "first": float64(30), + "after": (*string)(nil), + } + tests := []struct { name string reqParams map[string]interface{} @@ -314,6 +376,15 @@ func Test_ListDiscussions(t *testing.T) { expectError: true, errContains: "repository not found", }, + { + name: "list org-level discussions (no repo provided)", + reqParams: map[string]interface{}{ + "owner": "owner", + // repo is not provided, it will default to ".github" + }, + expectError: false, + expectedCount: 4, + }, } // Define the actual query strings that match the implementation @@ -351,6 +422,9 @@ func Test_ListDiscussions(t *testing.T) { case "repository not found error": matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsRepoNotFound, mockErrorRepoNotFound) httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "list org-level discussions (no repo provided)": + matcher := githubv4mock.NewQueryMatcher(qBasicNoOrder, varsOrgLevel, mockResponseOrgLevel) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) } gqlClient := githubv4.NewClient(httpClient) @@ -410,7 +484,7 @@ func Test_GetDiscussion(t *testing.T) { assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) // Use exact string query that matches implementation output - qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,body,createdAt,url,category{name}}}}" + qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,url,category{name}}}}" vars := map[string]interface{}{ "owner": "owner", @@ -429,6 +503,7 @@ func Test_GetDiscussion(t *testing.T) { response: githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{"discussion": map[string]any{ "number": 1, + "title": "Test Discussion Title", "body": "This is a test discussion", "url": "https://github.com/owner/repo/discussions/1", "createdAt": "2025-04-25T12:00:00Z", @@ -439,6 +514,7 @@ func Test_GetDiscussion(t *testing.T) { expected: &github.Discussion{ HTMLURL: github.Ptr("https://github.com/owner/repo/discussions/1"), Number: github.Ptr(1), + Title: github.Ptr("Test Discussion Title"), Body: github.Ptr("This is a test discussion"), CreatedAt: &github.Timestamp{Time: time.Date(2025, 4, 25, 12, 0, 0, 0, time.UTC)}, DiscussionCategory: &github.DiscussionCategory{ @@ -475,6 +551,7 @@ func Test_GetDiscussion(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(text), &out)) assert.Equal(t, *tc.expected.HTMLURL, *out.HTMLURL) assert.Equal(t, *tc.expected.Number, *out.Number) + assert.Equal(t, *tc.expected.Title, *out.Title) assert.Equal(t, *tc.expected.Body, *out.Body) // Check category label assert.Equal(t, *tc.expected.DiscussionCategory.Name, *out.DiscussionCategory.Name) @@ -561,17 +638,33 @@ func Test_GetDiscussionComments(t *testing.T) { } func Test_ListDiscussionCategories(t *testing.T) { + mockClient := githubv4.NewClient(nil) + toolDef, _ := ListDiscussionCategories(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) + assert.Equal(t, "list_discussion_categories", toolDef.Name) + assert.NotEmpty(t, toolDef.Description) + assert.Contains(t, toolDef.Description, "or organisation") + assert.Contains(t, toolDef.InputSchema.Properties, "owner") + assert.Contains(t, toolDef.InputSchema.Properties, "repo") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner"}) + // Use exact string query that matches implementation output qListCategories := "query($first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussionCategories(first: $first){nodes{id,name},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" - // Variables matching what GraphQL receives after JSON marshaling/unmarshaling - vars := map[string]interface{}{ + // Variables for repository-level categories + varsRepo := map[string]interface{}{ "owner": "owner", "repo": "repo", "first": float64(25), } - mockResp := githubv4mock.DataResponse(map[string]any{ + // Variables for organization-level categories (using .github repo) + varsOrg := map[string]interface{}{ + "owner": "owner", + "repo": ".github", + "first": float64(25), + } + + mockRespRepo := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ "discussionCategories": map[string]any{ "nodes": []map[string]any{ @@ -588,37 +681,98 @@ func Test_ListDiscussionCategories(t *testing.T) { }, }, }) - matcher := githubv4mock.NewQueryMatcher(qListCategories, vars, mockResp) - httpClient := githubv4mock.NewMockedHTTPClient(matcher) - gqlClient := githubv4.NewClient(httpClient) - tool, handler := ListDiscussionCategories(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) - assert.Equal(t, "list_discussion_categories", tool.Name) - assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "owner") - assert.Contains(t, tool.InputSchema.Properties, "repo") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + mockRespOrg := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussionCategories": map[string]any{ + "nodes": []map[string]any{ + {"id": "789", "name": "Announcements"}, + {"id": "101", "name": "General"}, + {"id": "112", "name": "Ideas"}, + }, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 3, + }, + }, + }) - request := createMCPRequest(map[string]interface{}{"owner": "owner", "repo": "repo"}) - result, err := handler(context.Background(), request) - require.NoError(t, err) + tests := []struct { + name string + reqParams map[string]interface{} + vars map[string]interface{} + mockResponse githubv4mock.GQLResponse + expectError bool + expectedCount int + expectedCategories []map[string]string + }{ + { + name: "list repository-level discussion categories", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + vars: varsRepo, + mockResponse: mockRespRepo, + expectError: false, + expectedCount: 2, + expectedCategories: []map[string]string{ + {"id": "123", "name": "CategoryOne"}, + {"id": "456", "name": "CategoryTwo"}, + }, + }, + { + name: "list org-level discussion categories (no repo provided)", + reqParams: map[string]interface{}{ + "owner": "owner", + // repo is not provided, it will default to ".github" + }, + vars: varsOrg, + mockResponse: mockRespOrg, + expectError: false, + expectedCount: 3, + expectedCategories: []map[string]string{ + {"id": "789", "name": "Announcements"}, + {"id": "101", "name": "General"}, + {"id": "112", "name": "Ideas"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matcher := githubv4mock.NewQueryMatcher(qListCategories, tc.vars, tc.mockResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + gqlClient := githubv4.NewClient(httpClient) - text := getTextResult(t, result).Text + _, handler := ListDiscussionCategories(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) - var response struct { - Categories []map[string]string `json:"categories"` - PageInfo struct { - HasNextPage bool `json:"hasNextPage"` - HasPreviousPage bool `json:"hasPreviousPage"` - StartCursor string `json:"startCursor"` - EndCursor string `json:"endCursor"` - } `json:"pageInfo"` - TotalCount int `json:"totalCount"` + req := createMCPRequest(tc.reqParams) + res, err := handler(context.Background(), req) + text := getTextResult(t, res).Text + + if tc.expectError { + require.True(t, res.IsError) + return + } + require.NoError(t, err) + + var response struct { + Categories []map[string]string `json:"categories"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + StartCursor string `json:"startCursor"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + TotalCount int `json:"totalCount"` + } + require.NoError(t, json.Unmarshal([]byte(text), &response)) + assert.Equal(t, tc.expectedCategories, response.Categories) + }) } - require.NoError(t, json.Unmarshal([]byte(text), &response)) - assert.Len(t, response.Categories, 2) - assert.Equal(t, "123", response.Categories[0]["id"]) - assert.Equal(t, "CategoryOne", response.Categories[0]["name"]) - assert.Equal(t, "456", response.Categories[1]["id"]) - assert.Equal(t, "CategoryTwo", response.Categories[1]["name"]) } diff --git a/pkg/github/gists.go b/pkg/github/gists.go new file mode 100644 index 000000000..fce34f6a8 --- /dev/null +++ b/pkg/github/gists.go @@ -0,0 +1,259 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v74/github" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// ListGists creates a tool to list gists for a user +func ListGists(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_gists", + mcp.WithDescription(t("TOOL_LIST_GISTS_DESCRIPTION", "List gists for a user")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_GISTS", "List Gists"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("username", + mcp.Description("GitHub username (omit for authenticated user's gists)"), + ), + mcp.WithString("since", + mcp.Description("Only gists updated after this time (ISO 8601 timestamp)"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + username, err := OptionalParam[string](request, "username") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + since, err := OptionalParam[string](request, "since") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.GistListOptions{ + ListOptions: github.ListOptions{ + Page: pagination.Page, + PerPage: pagination.PerPage, + }, + } + + // Parse since timestamp if provided + if since != "" { + sinceTime, err := parseISOTimestamp(since) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("invalid since timestamp: %v", err)), nil + } + opts.Since = sinceTime + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + gists, resp, err := client.Gists.List(ctx, username, opts) + if err != nil { + return nil, fmt.Errorf("failed to list gists: %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 gists: %s", string(body))), nil + } + + r, err := json.Marshal(gists) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// CreateGist creates a tool to create a new gist +func CreateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("create_gist", + mcp.WithDescription(t("TOOL_CREATE_GIST_DESCRIPTION", "Create a new gist")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_CREATE_GIST", "Create Gist"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("description", + mcp.Description("Description of the gist"), + ), + mcp.WithString("filename", + mcp.Required(), + mcp.Description("Filename for simple single-file gist creation"), + ), + mcp.WithString("content", + mcp.Required(), + mcp.Description("Content for simple single-file gist creation"), + ), + mcp.WithBoolean("public", + mcp.Description("Whether the gist is public"), + mcp.DefaultBool(false), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + description, err := OptionalParam[string](request, "description") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + filename, err := RequiredParam[string](request, "filename") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + content, err := RequiredParam[string](request, "content") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + public, err := OptionalParam[bool](request, "public") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + files := make(map[github.GistFilename]github.GistFile) + files[github.GistFilename(filename)] = github.GistFile{ + Filename: github.Ptr(filename), + Content: github.Ptr(content), + } + + gist := &github.Gist{ + Files: files, + Public: github.Ptr(public), + Description: github.Ptr(description), + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + createdGist, resp, err := client.Gists.Create(ctx, gist) + if err != nil { + return nil, fmt.Errorf("failed to create gist: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + 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 create gist: %s", string(body))), nil + } + + r, err := json.Marshal(createdGist) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// UpdateGist creates a tool to edit an existing gist +func UpdateGist(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("update_gist", + mcp.WithDescription(t("TOOL_UPDATE_GIST_DESCRIPTION", "Update an existing gist")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_UPDATE_GIST", "Update Gist"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("gist_id", + mcp.Required(), + mcp.Description("ID of the gist to update"), + ), + mcp.WithString("description", + mcp.Description("Updated description of the gist"), + ), + mcp.WithString("filename", + mcp.Required(), + mcp.Description("Filename to update or create"), + ), + mcp.WithString("content", + mcp.Required(), + mcp.Description("Content for the file"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + gistID, err := RequiredParam[string](request, "gist_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + description, err := OptionalParam[string](request, "description") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + filename, err := RequiredParam[string](request, "filename") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + content, err := RequiredParam[string](request, "content") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + files := make(map[github.GistFilename]github.GistFile) + files[github.GistFilename(filename)] = github.GistFile{ + Filename: github.Ptr(filename), + Content: github.Ptr(content), + } + + gist := &github.Gist{ + Files: files, + Description: github.Ptr(description), + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + updatedGist, resp, err := client.Gists.Edit(ctx, gistID, gist) + if err != nil { + return nil, fmt.Errorf("failed to update gist: %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 gist: %s", string(body))), nil + } + + r, err := json.Marshal(updatedGist) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} diff --git a/pkg/github/gists_test.go b/pkg/github/gists_test.go new file mode 100644 index 000000000..49d63a252 --- /dev/null +++ b/pkg/github/gists_test.go @@ -0,0 +1,507 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v74/github" + "github.com/migueleliasweb/go-github-mock/src/mock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ListGists(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := ListGists(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_gists", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "username") + assert.Contains(t, tool.InputSchema.Properties, "since") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Empty(t, tool.InputSchema.Required) + + // Setup mock gists for success case + mockGists := []*github.Gist{ + { + ID: github.Ptr("gist1"), + Description: github.Ptr("First Gist"), + HTMLURL: github.Ptr("https://gist.github.com/user/gist1"), + Public: github.Ptr(true), + CreatedAt: &github.Timestamp{Time: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)}, + Owner: &github.User{Login: github.Ptr("user")}, + Files: map[github.GistFilename]github.GistFile{ + "file1.txt": { + Filename: github.Ptr("file1.txt"), + Content: github.Ptr("content of file 1"), + }, + }, + }, + { + ID: github.Ptr("gist2"), + Description: github.Ptr("Second Gist"), + HTMLURL: github.Ptr("https://gist.github.com/testuser/gist2"), + Public: github.Ptr(false), + CreatedAt: &github.Timestamp{Time: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC)}, + Owner: &github.User{Login: github.Ptr("testuser")}, + Files: map[github.GistFilename]github.GistFile{ + "file2.js": { + Filename: github.Ptr("file2.js"), + Content: github.Ptr("console.log('hello');"), + }, + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedGists []*github.Gist + expectedErrMsg string + }{ + { + name: "list authenticated user's gists", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetGists, + mockGists, + ), + ), + requestArgs: map[string]interface{}{}, + expectError: false, + expectedGists: mockGists, + }, + { + name: "list specific user's gists", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetUsersGistsByUsername, + mockResponse(t, http.StatusOK, mockGists), + ), + ), + requestArgs: map[string]interface{}{ + "username": "testuser", + }, + expectError: false, + expectedGists: mockGists, + }, + { + name: "list gists with pagination and since parameter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetGists, + expectQueryParams(t, map[string]string{ + "since": "2023-01-01T00:00:00Z", + "page": "2", + "per_page": "5", + }).andThen( + mockResponse(t, http.StatusOK, mockGists), + ), + ), + ), + requestArgs: map[string]interface{}{ + "since": "2023-01-01T00:00:00Z", + "page": float64(2), + "perPage": float64(5), + }, + expectError: false, + expectedGists: mockGists, + }, + { + name: "invalid since parameter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetGists, + mockGists, + ), + ), + requestArgs: map[string]interface{}{ + "since": "invalid-date", + }, + expectError: true, + expectedErrMsg: "invalid since timestamp", + }, + { + name: "list gists fails with error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetGists, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{}, + expectError: true, + expectedErrMsg: "failed to list gists", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ListGists(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + if err != nil { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For errors returned as part of the result, not as an error + assert.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, 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 returnedGists []*github.Gist + err = json.Unmarshal([]byte(textContent.Text), &returnedGists) + require.NoError(t, err) + + assert.Len(t, returnedGists, len(tc.expectedGists)) + for i, gist := range returnedGists { + assert.Equal(t, *tc.expectedGists[i].ID, *gist.ID) + assert.Equal(t, *tc.expectedGists[i].Description, *gist.Description) + assert.Equal(t, *tc.expectedGists[i].HTMLURL, *gist.HTMLURL) + assert.Equal(t, *tc.expectedGists[i].Public, *gist.Public) + } + }) + } +} + +func Test_CreateGist(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := CreateGist(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "create_gist", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "description") + assert.Contains(t, tool.InputSchema.Properties, "filename") + assert.Contains(t, tool.InputSchema.Properties, "content") + assert.Contains(t, tool.InputSchema.Properties, "public") + + // Verify required parameters + assert.Contains(t, tool.InputSchema.Required, "filename") + assert.Contains(t, tool.InputSchema.Required, "content") + + // Setup mock data for test cases + createdGist := &github.Gist{ + ID: github.Ptr("new-gist-id"), + Description: github.Ptr("Test Gist"), + HTMLURL: github.Ptr("https://gist.github.com/user/new-gist-id"), + Public: github.Ptr(false), + CreatedAt: &github.Timestamp{Time: time.Now()}, + Owner: &github.User{Login: github.Ptr("user")}, + Files: map[github.GistFilename]github.GistFile{ + "test.go": { + Filename: github.Ptr("test.go"), + Content: github.Ptr("package main\n\nfunc main() {\n\tfmt.Println(\"Hello, Gist!\")\n}"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + expectedGist *github.Gist + }{ + { + name: "create gist successfully", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostGists, + mockResponse(t, http.StatusCreated, createdGist), + ), + ), + requestArgs: map[string]interface{}{ + "filename": "test.go", + "content": "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, Gist!\")\n}", + "description": "Test Gist", + "public": false, + }, + expectError: false, + expectedGist: createdGist, + }, + { + name: "missing required filename", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "content": "test content", + "description": "Test Gist", + }, + expectError: true, + expectedErrMsg: "missing required parameter: filename", + }, + { + name: "missing required content", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "filename": "test.go", + "description": "Test Gist", + }, + expectError: true, + expectedErrMsg: "missing required parameter: content", + }, + { + name: "api returns error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostGists, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"message": "Requires authentication"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "filename": "test.go", + "content": "package main", + "description": "Test Gist", + }, + expectError: true, + expectedErrMsg: "failed to create gist", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := CreateGist(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + if err != nil { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For errors returned as part of the result, not as an error + assert.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + assert.NotNil(t, result) + + // Parse the result and get the text content + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var gist *github.Gist + err = json.Unmarshal([]byte(textContent.Text), &gist) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedGist.ID, *gist.ID) + assert.Equal(t, *tc.expectedGist.Description, *gist.Description) + assert.Equal(t, *tc.expectedGist.HTMLURL, *gist.HTMLURL) + assert.Equal(t, *tc.expectedGist.Public, *gist.Public) + + // Verify file content + for filename, expectedFile := range tc.expectedGist.Files { + actualFile, exists := gist.Files[filename] + assert.True(t, exists) + assert.Equal(t, *expectedFile.Filename, *actualFile.Filename) + assert.Equal(t, *expectedFile.Content, *actualFile.Content) + } + }) + } +} + +func Test_UpdateGist(t *testing.T) { + // Verify tool definition + mockClient := github.NewClient(nil) + tool, _ := UpdateGist(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "update_gist", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "gist_id") + assert.Contains(t, tool.InputSchema.Properties, "description") + assert.Contains(t, tool.InputSchema.Properties, "filename") + assert.Contains(t, tool.InputSchema.Properties, "content") + + // Verify required parameters + assert.Contains(t, tool.InputSchema.Required, "gist_id") + assert.Contains(t, tool.InputSchema.Required, "filename") + assert.Contains(t, tool.InputSchema.Required, "content") + + // Setup mock data for test cases + updatedGist := &github.Gist{ + ID: github.Ptr("existing-gist-id"), + Description: github.Ptr("Updated Test Gist"), + HTMLURL: github.Ptr("https://gist.github.com/user/existing-gist-id"), + Public: github.Ptr(true), + UpdatedAt: &github.Timestamp{Time: time.Now()}, + Owner: &github.User{Login: github.Ptr("user")}, + Files: map[github.GistFilename]github.GistFile{ + "updated.go": { + Filename: github.Ptr("updated.go"), + Content: github.Ptr("package main\n\nfunc main() {\n\tfmt.Println(\"Updated Gist!\")\n}"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedErrMsg string + expectedGist *github.Gist + }{ + { + name: "update gist successfully", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchGistsByGistId, + mockResponse(t, http.StatusOK, updatedGist), + ), + ), + requestArgs: map[string]interface{}{ + "gist_id": "existing-gist-id", + "filename": "updated.go", + "content": "package main\n\nfunc main() {\n\tfmt.Println(\"Updated Gist!\")\n}", + "description": "Updated Test Gist", + }, + expectError: false, + expectedGist: updatedGist, + }, + { + name: "missing required gist_id", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "filename": "updated.go", + "content": "updated content", + "description": "Updated Test Gist", + }, + expectError: true, + expectedErrMsg: "missing required parameter: gist_id", + }, + { + name: "missing required filename", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "gist_id": "existing-gist-id", + "content": "updated content", + "description": "Updated Test Gist", + }, + expectError: true, + expectedErrMsg: "missing required parameter: filename", + }, + { + name: "missing required content", + mockedClient: mock.NewMockedHTTPClient(), + requestArgs: map[string]interface{}{ + "gist_id": "existing-gist-id", + "filename": "updated.go", + "description": "Updated Test Gist", + }, + expectError: true, + expectedErrMsg: "missing required parameter: content", + }, + { + name: "api returns error", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchGistsByGistId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "gist_id": "nonexistent-gist-id", + "filename": "updated.go", + "content": "package main", + "description": "Updated Test Gist", + }, + expectError: true, + expectedErrMsg: "failed to update gist", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := UpdateGist(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + if err != nil { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + } else { + // For errors returned as part of the result, not as an error + assert.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + assert.NotNil(t, result) + + // Parse the result and get the text content + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var gist *github.Gist + err = json.Unmarshal([]byte(textContent.Text), &gist) + require.NoError(t, err) + + assert.Equal(t, *tc.expectedGist.ID, *gist.ID) + assert.Equal(t, *tc.expectedGist.Description, *gist.Description) + assert.Equal(t, *tc.expectedGist.HTMLURL, *gist.HTMLURL) + + // Verify file content + for filename, expectedFile := range tc.expectedGist.Files { + actualFile, exists := gist.Files[filename] + assert.True(t, exists) + assert.Equal(t, *expectedFile.Filename, *actualFile.Filename) + assert.Equal(t, *expectedFile.Content, *actualFile.Content) + } + }) + } +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index f718c37cb..3a1440489 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -12,12 +12,138 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" ) +// IssueFragment represents a fragment of an issue node in the GraphQL API. +type IssueFragment struct { + Number githubv4.Int + Title githubv4.String + Body githubv4.String + State githubv4.String + DatabaseID int64 + + Author struct { + Login githubv4.String + } + CreatedAt githubv4.DateTime + UpdatedAt githubv4.DateTime + Labels struct { + Nodes []struct { + Name githubv4.String + ID githubv4.String + Description githubv4.String + } + } `graphql:"labels(first: 100)"` + Comments struct { + TotalCount githubv4.Int + } `graphql:"comments"` +} + +// Common interface for all issue query types +type IssueQueryResult interface { + GetIssueFragment() IssueQueryFragment +} + +type IssueQueryFragment struct { + Nodes []IssueFragment `graphql:"nodes"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + TotalCount int +} + +// ListIssuesQuery is the root query structure for fetching issues with optional label filtering. +type ListIssuesQuery struct { + Repository struct { + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +// ListIssuesQueryTypeWithLabels is the query structure for fetching issues with optional label filtering. +type ListIssuesQueryTypeWithLabels struct { + Repository struct { + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +// ListIssuesQueryWithSince is the query structure for fetching issues without label filtering but with since filtering. +type ListIssuesQueryWithSince struct { + Repository struct { + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +// ListIssuesQueryTypeWithLabelsWithSince is the query structure for fetching issues with both label and since filtering. +type ListIssuesQueryTypeWithLabelsWithSince struct { + Repository struct { + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +// Implement the interface for all query types +func (q *ListIssuesQueryTypeWithLabels) GetIssueFragment() IssueQueryFragment { + return q.Repository.Issues +} + +func (q *ListIssuesQuery) GetIssueFragment() IssueQueryFragment { + return q.Repository.Issues +} + +func (q *ListIssuesQueryWithSince) GetIssueFragment() IssueQueryFragment { + return q.Repository.Issues +} + +func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIssueFragment() IssueQueryFragment { + return q.Repository.Issues +} + +func getIssueQueryType(hasLabels bool, hasSince bool) any { + switch { + case hasLabels && hasSince: + return &ListIssuesQueryTypeWithLabelsWithSince{} + case hasLabels: + return &ListIssuesQueryTypeWithLabels{} + case hasSince: + return &ListIssuesQueryWithSince{} + default: + return &ListIssuesQuery{} + } +} + +func fragmentToIssue(fragment IssueFragment) *github.Issue { + // Convert GraphQL labels to GitHub API labels format + var foundLabels []*github.Label + for _, labelNode := range fragment.Labels.Nodes { + foundLabels = append(foundLabels, &github.Label{ + Name: github.Ptr(string(labelNode.Name)), + NodeID: github.Ptr(string(labelNode.ID)), + Description: github.Ptr(string(labelNode.Description)), + }) + } + + return &github.Issue{ + Number: github.Ptr(int(fragment.Number)), + Title: github.Ptr(string(fragment.Title)), + CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time}, + UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time}, + User: &github.User{ + Login: github.Ptr(string(fragment.Author.Login)), + }, + State: github.Ptr(string(fragment.State)), + ID: github.Ptr(fragment.DatabaseID), + Body: github.Ptr(string(fragment.Body)), + Labels: foundLabels, + Comments: github.Ptr(int(fragment.Comments.TotalCount)), + } +} + // 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", @@ -80,6 +206,53 @@ func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool } } +// ListIssueTypes creates a tool to list defined issue types for an organization. This can be used to understand supported issue type values for creating or updating issues. +func ListIssueTypes(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + + return mcp.NewTool("list_issue_types", + mcp.WithDescription(t("TOOL_LIST_ISSUE_TYPES_FOR_ORG", "List supported issue types for repository owner (organization).")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_ISSUE_TYPES_USER_TITLE", "List available issue types"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("The organization owner of the repository"), + ), + ), + 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 + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + issueTypes, resp, err := client.Organizations.ListIssueTypes(ctx, owner) + if err != nil { + return nil, fmt.Errorf("failed to list issue types: %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 issue types: %s", string(body))), nil + } + + r, err := json.Marshal(issueTypes) + if err != nil { + return nil, fmt.Errorf("failed to marshal issue types: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // 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", @@ -380,67 +553,39 @@ func RemoveSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) } client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - - // Create the request body - requestBody := map[string]interface{}{ - "sub_issue_id": subIssueID, - } - reqBodyBytes, err := json.Marshal(requestBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal request body: %w", err) - } - - // Create the HTTP request - url := fmt.Sprintf("%srepos/%s/%s/issues/%d/sub_issue", - client.BaseURL.String(), owner, repo, issueNumber) - req, err := http.NewRequestWithContext(ctx, "DELETE", url, strings.NewReader(string(reqBodyBytes))) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - req.Header.Set("Accept", "application/vnd.github+json") - req.Header.Set("Content-Type", "application/json") - req.Header.Set("X-GitHub-Api-Version", "2022-11-28") - - httpClient := client.Client() // Use authenticated GitHub client - resp, err := httpClient.Do(req) - if err != nil { - var ghResp *github.Response - if resp != nil { - ghResp = &github.Response{Response: resp} - } - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to remove sub-issue", - ghResp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - - if resp.StatusCode != http.StatusOK { - return mcp.NewToolResultError(fmt.Sprintf("failed to remove sub-issue: %s", string(body))), nil - } - - // Parse and re-marshal to ensure consistent formatting - var result interface{} - if err := json.Unmarshal(body, &result); err != nil { - return nil, fmt.Errorf("failed to unmarshal response: %w", err) - } - - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + subIssueRequest := github.SubIssueRequest{ + SubIssueID: int64(subIssueID), + } + + subIssue, resp, err := client.SubIssue.Remove(ctx, owner, repo, int64(issueNumber), subIssueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to remove sub-issue", + resp, + err, + ), nil + } + 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 remove sub-issue: %s", string(body))), nil + } + + r, err := json.Marshal(subIssue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } } // ReprioritizeSubIssue creates a tool to reprioritize a sub-issue to a different position in the parent list. @@ -569,10 +714,10 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) ( mcp.Description("Search query using GitHub issues search syntax"), ), mcp.WithString("owner", - mcp.Description("Optional repository owner. If provided with repo, only notifications for this repository are listed."), + mcp.Description("Optional repository owner. If provided with repo, only issues for this repository are listed."), ), mcp.WithString("repo", - mcp.Description("Optional repository name. If provided with owner, only notifications for this repository are listed."), + mcp.Description("Optional repository name. If provided with owner, only issues for this repository are listed."), ), mcp.WithString("sort", mcp.Description("Sort field by number of matches of categories, defaults to best match"), @@ -724,9 +869,9 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } // ListIssues creates a tool to list and filter repository issues -func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func ListIssues(getGQLClient GetGQLClientFn, 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.")), + mcp.WithDescription(t("TOOL_LIST_ISSUES_DESCRIPTION", "List issues in a GitHub repository. For pagination, use the 'endCursor' from the previous response's 'pageInfo' in the 'after' parameter.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_ISSUES_USER_TITLE", "List issues"), ReadOnlyHint: ToBoolPtr(true), @@ -740,8 +885,8 @@ func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (to mcp.Description("Repository name"), ), mcp.WithString("state", - mcp.Description("Filter by state"), - mcp.Enum("open", "closed", "all"), + mcp.Description("Filter by state, by default both open and closed issues are returned when not provided"), + mcp.Enum("OPEN", "CLOSED"), ), mcp.WithArray("labels", mcp.Description("Filter by labels"), @@ -751,18 +896,18 @@ func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (to }, ), ), - mcp.WithString("sort", - mcp.Description("Sort order"), - mcp.Enum("created", "updated", "comments"), + mcp.WithString("orderBy", + mcp.Description("Order issues by field. If provided, the 'direction' also needs to be provided."), + mcp.Enum("CREATED_AT", "UPDATED_AT", "COMMENTS"), ), mcp.WithString("direction", - mcp.Description("Sort direction"), - mcp.Enum("asc", "desc"), + mcp.Description("Order direction. If provided, the 'orderBy' also needs to be provided."), + mcp.Enum("ASC", "DESC"), ), mcp.WithString("since", mcp.Description("Filter by date (ISO 8601 timestamp)"), ), - WithPagination(), + WithCursorPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := RequiredParam[string](request, "owner") @@ -774,74 +919,164 @@ func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(err.Error()), nil } - opts := &github.IssueListByRepoOptions{} - // Set optional parameters if provided - opts.State, err = OptionalParam[string](request, "state") + state, err := OptionalParam[string](request, "state") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // If the state has a value, cast into an array of strings + var states []githubv4.IssueState + if state != "" { + states = append(states, githubv4.IssueState(state)) + } else { + states = []githubv4.IssueState{githubv4.IssueStateOpen, githubv4.IssueStateClosed} + } + // Get labels - opts.Labels, err = OptionalStringArrayParam(request, "labels") + labels, err := OptionalStringArrayParam(request, "labels") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - opts.Sort, err = OptionalParam[string](request, "sort") + orderBy, err := OptionalParam[string](request, "orderBy") if err != nil { return mcp.NewToolResultError(err.Error()), nil } - opts.Direction, err = OptionalParam[string](request, "direction") + direction, err := OptionalParam[string](request, "direction") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + // These variables are required for the GraphQL query to be set by default + // If orderBy is empty, default to CREATED_AT + if orderBy == "" { + orderBy = "CREATED_AT" + } + // If direction is empty, default to DESC + if direction == "" { + direction = "DESC" + } + since, err := OptionalParam[string](request, "since") if err != nil { return mcp.NewToolResultError(err.Error()), nil } + + // There are two optional parameters: since and labels. + var sinceTime time.Time + var hasSince bool if since != "" { - timestamp, err := parseISOTimestamp(since) + sinceTime, err = parseISOTimestamp(since) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to list issues: %s", err.Error())), nil } - opts.Since = timestamp + hasSince = true } + hasLabels := len(labels) > 0 - if page, ok := request.GetArguments()["page"].(float64); ok { - opts.ListOptions.Page = int(page) + // Get pagination parameters and convert to GraphQL format + pagination, err := OptionalCursorPaginationParams(request) + if err != nil { + return nil, err } - if perPage, ok := request.GetArguments()["perPage"].(float64); ok { - opts.ListOptions.PerPage = int(perPage) + // Check if someone tried to use page-based pagination instead of cursor-based + if _, pageProvided := request.GetArguments()["page"]; pageProvided { + return mcp.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil } - client, err := getClient(ctx) + // Check if pagination parameters were explicitly provided + _, perPageProvided := request.GetArguments()["perPage"] + paginationExplicit := perPageProvided + + paginationParams, err := pagination.ToGraphQLParams() if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) + return nil, err } - issues, resp, err := client.Issues.ListByRepo(ctx, owner, repo, opts) + + // Use default of 30 if pagination was not explicitly provided + if !paginationExplicit { + defaultFirst := int32(DefaultGraphQLPageSize) + paginationParams.First = &defaultFirst + } + + client, err := getGQLClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to list issues: %w", err) + return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil } - 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) + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "states": states, + "orderBy": githubv4.IssueOrderField(orderBy), + "direction": githubv4.OrderDirection(direction), + "first": githubv4.Int(*paginationParams.First), + } + + if paginationParams.After != nil { + vars["after"] = githubv4.String(*paginationParams.After) + } else { + // Used within query, therefore must be set to nil and provided as $after + vars["after"] = (*githubv4.String)(nil) + } + + // Ensure optional parameters are set + if hasLabels { + // Use query with labels filtering - convert string labels to githubv4.String slice + labelStrings := make([]githubv4.String, len(labels)) + for i, label := range labels { + labelStrings[i] = githubv4.String(label) } - return mcp.NewToolResultError(fmt.Sprintf("failed to list issues: %s", string(body))), nil + vars["labels"] = labelStrings + } + + if hasSince { + vars["since"] = githubv4.DateTime{Time: sinceTime} + } + + issueQuery := getIssueQueryType(hasLabels, hasSince) + if err := client.Query(ctx, issueQuery, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil } - r, err := json.Marshal(issues) + // Extract and convert all issue nodes using the common interface + var issues []*github.Issue + var pageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + var totalCount int + + if queryResult, ok := issueQuery.(IssueQueryResult); ok { + fragment := queryResult.GetIssueFragment() + for _, issue := range fragment.Nodes { + issues = append(issues, fragmentToIssue(issue)) + } + pageInfo = fragment.PageInfo + totalCount = fragment.TotalCount + } + + // Create response with issues + response := map[string]interface{}{ + "issues": issues, + "pageInfo": map[string]interface{}{ + "hasNextPage": pageInfo.HasNextPage, + "hasPreviousPage": pageInfo.HasPreviousPage, + "startCursor": string(pageInfo.StartCursor), + "endCursor": string(pageInfo.EndCursor), + }, + "totalCount": totalCount, + } + out, err := json.Marshal(response) if err != nil { return nil, fmt.Errorf("failed to marshal issues: %w", err) } - - return mcp.NewToolResultText(string(r)), nil + return mcp.NewToolResultText(string(out)), nil } } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 2bdb89b06..249fadef8 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -5,13 +5,14 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "testing" "time" "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" @@ -410,6 +411,100 @@ func Test_SearchIssues(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing is:issue filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server is:issue is:open (label:critical OR label:urgent)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with existing repo: filter and conflicting owner/repo params - uses query filter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue repo:github/github-mcp-server critical", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server critical", + "owner": "different-owner", + "repo": "different-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with both is: and repo: filters already present", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue repo:octocat/Hello-World bug", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:issue repo:octocat/Hello-World bug", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with multiple OR operators and existing filters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server is:issue (label:critical OR label:urgent OR label:high-priority OR label:blocker)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search issues fails", mockedClient: mock.NewMockedHTTPClient( @@ -648,8 +743,8 @@ func Test_CreateIssue(t *testing.T) { func Test_ListIssues(t *testing.T) { // Verify tool definition - mockClient := github.NewClient(nil) - tool, _ := ListIssues(stubGetClientFn(mockClient), translations.NullTranslationHelper) + mockClient := githubv4.NewClient(nil) + tool, _ := ListIssues(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "list_issues", tool.Name) @@ -658,166 +753,297 @@ func Test_ListIssues(t *testing.T) { assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "state") assert.Contains(t, tool.InputSchema.Properties, "labels") - assert.Contains(t, tool.InputSchema.Properties, "sort") + assert.Contains(t, tool.InputSchema.Properties, "orderBy") assert.Contains(t, tool.InputSchema.Properties, "direction") assert.Contains(t, tool.InputSchema.Properties, "since") - assert.Contains(t, tool.InputSchema.Properties, "page") + assert.Contains(t, tool.InputSchema.Properties, "after") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) - // Setup mock issues for success case - mockIssues := []*github.Issue{ + // Mock issues data + mockIssuesAll := []map[string]any{ + { + "number": 123, + "title": "First Issue", + "body": "This is the first test issue", + "state": "OPEN", + "databaseId": 1001, + "createdAt": "2023-01-01T00:00:00Z", + "updatedAt": "2023-01-01T00:00:00Z", + "author": map[string]any{"login": "user1"}, + "labels": map[string]any{ + "nodes": []map[string]any{ + {"name": "bug", "id": "label1", "description": "Bug label"}, + }, + }, + "comments": map[string]any{ + "totalCount": 5, + }, + }, { - Number: github.Ptr(123), - Title: github.Ptr("First Issue"), - Body: github.Ptr("This is the first test issue"), - State: github.Ptr("open"), - HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), - CreatedAt: &github.Timestamp{Time: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC)}, + "number": 456, + "title": "Second Issue", + "body": "This is the second test issue", + "state": "OPEN", + "databaseId": 1002, + "createdAt": "2023-02-01T00:00:00Z", + "updatedAt": "2023-02-01T00:00:00Z", + "author": map[string]any{"login": "user2"}, + "labels": map[string]any{ + "nodes": []map[string]any{ + {"name": "enhancement", "id": "label2", "description": "Enhancement label"}, + }, + }, + "comments": map[string]any{ + "totalCount": 3, + }, }, + } + + mockIssuesOpen := []map[string]any{mockIssuesAll[0], mockIssuesAll[1]} + mockIssuesClosed := []map[string]any{ { - Number: github.Ptr(456), - Title: github.Ptr("Second Issue"), - Body: github.Ptr("This is the second test issue"), - State: github.Ptr("open"), - HTMLURL: github.Ptr("https://github.com/owner/repo/issues/456"), - Labels: []*github.Label{{Name: github.Ptr("bug")}}, - CreatedAt: &github.Timestamp{Time: time.Date(2023, 2, 1, 0, 0, 0, 0, time.UTC)}, + "number": 789, + "title": "Closed Issue", + "body": "This is a closed issue", + "state": "CLOSED", + "databaseId": 1003, + "createdAt": "2023-03-01T00:00:00Z", + "updatedAt": "2023-03-01T00:00:00Z", + "author": map[string]any{"login": "user3"}, + "labels": map[string]any{ + "nodes": []map[string]any{}, + }, + "comments": map[string]any{ + "totalCount": 1, + }, + }, + } + + // Mock responses + mockResponseListAll := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issues": map[string]any{ + "nodes": mockIssuesAll, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, + }, + }, + }) + + mockResponseOpenOnly := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issues": map[string]any{ + "nodes": mockIssuesOpen, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 2, + }, + }, + }) + + mockResponseClosedOnly := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issues": map[string]any{ + "nodes": mockIssuesClosed, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 1, + }, }, + }) + + mockErrorRepoNotFound := githubv4mock.ErrorResponse("repository not found") + + // Variables matching what GraphQL receives after JSON marshaling/unmarshaling + varsListAll := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "states": []interface{}{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + + varsOpenOnly := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "states": []interface{}{"OPEN"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + + varsClosedOnly := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "states": []interface{}{"CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + + varsWithLabels := map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "states": []interface{}{"OPEN", "CLOSED"}, + "labels": []interface{}{"bug", "enhancement"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), + } + + varsRepoNotFound := map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent-repo", + "states": []interface{}{"OPEN", "CLOSED"}, + "orderBy": "CREATED_AT", + "direction": "DESC", + "first": float64(30), + "after": (*string)(nil), } tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]interface{} - expectError bool - expectedIssues []*github.Issue - expectedErrMsg string + name string + reqParams map[string]interface{} + expectError bool + errContains string + expectedCount int + verifyOrder func(t *testing.T, issues []*github.Issue) }{ { - name: "list issues with minimal parameters", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatch( - mock.GetReposIssuesByOwnerByRepo, - mockIssues, - ), - ), - requestArgs: map[string]interface{}{ + name: "list all issues", + reqParams: map[string]interface{}{ "owner": "owner", "repo": "repo", }, - expectError: false, - expectedIssues: mockIssues, + expectError: false, + expectedCount: 2, }, { - name: "list issues with all parameters", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.GetReposIssuesByOwnerByRepo, - expectQueryParams(t, map[string]string{ - "state": "open", - "labels": "bug,enhancement", - "sort": "created", - "direction": "desc", - "since": "2023-01-01T00:00:00Z", - "page": "1", - "per_page": "30", - }).andThen( - mockResponse(t, http.StatusOK, mockIssues), - ), - ), - ), - requestArgs: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "state": "open", - "labels": []any{"bug", "enhancement"}, - "sort": "created", - "direction": "desc", - "since": "2023-01-01T00:00:00Z", - "page": float64(1), - "perPage": float64(30), + name: "filter by open state", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "state": "OPEN", }, - expectError: false, - expectedIssues: mockIssues, + expectError: false, + expectedCount: 2, }, { - name: "invalid since parameter", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatch( - mock.GetReposIssuesByOwnerByRepo, - mockIssues, - ), - ), - requestArgs: map[string]interface{}{ + name: "filter by closed state", + reqParams: map[string]interface{}{ "owner": "owner", "repo": "repo", - "since": "invalid-date", + "state": "CLOSED", }, - expectError: true, - expectedErrMsg: "invalid ISO 8601 timestamp", + expectError: false, + expectedCount: 1, }, { - name: "list issues fails with error", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.GetReposIssuesByOwnerByRepo, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNotFound) - _, _ = w.Write([]byte(`{"message": "Repository not found"}`)) - }), - ), - ), - requestArgs: map[string]interface{}{ - "owner": "nonexistent", - "repo": "repo", + name: "filter by labels", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "labels": []any{"bug", "enhancement"}, }, - expectError: true, - expectedErrMsg: "failed to list issues", + expectError: false, + expectedCount: 2, + }, + { + name: "repository not found error", + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "nonexistent-repo", + }, + expectError: true, + errContains: "repository not found", }, } + // Define the actual query strings that match the implementation + qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // Setup client with mock - client := github.NewClient(tc.mockedClient) - _, handler := ListIssues(stubGetClientFn(client), translations.NullTranslationHelper) + var httpClient *http.Client + + switch tc.name { + case "list all issues": + matcher := githubv4mock.NewQueryMatcher(qBasicNoLabels, varsListAll, mockResponseListAll) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "filter by open state": + matcher := githubv4mock.NewQueryMatcher(qBasicNoLabels, varsOpenOnly, mockResponseOpenOnly) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "filter by closed state": + matcher := githubv4mock.NewQueryMatcher(qBasicNoLabels, varsClosedOnly, mockResponseClosedOnly) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "filter by labels": + matcher := githubv4mock.NewQueryMatcher(qWithLabels, varsWithLabels, mockResponseListAll) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + case "repository not found error": + matcher := githubv4mock.NewQueryMatcher(qBasicNoLabels, varsRepoNotFound, mockErrorRepoNotFound) + httpClient = githubv4mock.NewMockedHTTPClient(matcher) + } - // Create call request - request := createMCPRequest(tc.requestArgs) + gqlClient := githubv4.NewClient(httpClient) + _, handler := ListIssues(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) - // Call handler - result, err := handler(context.Background(), request) + req := createMCPRequest(tc.reqParams) + res, err := handler(context.Background(), req) + text := getTextResult(t, res).Text - // Verify results if tc.expectError { - if err != nil { - assert.Contains(t, err.Error(), tc.expectedErrMsg) - } else { - // For errors returned as part of the result, not as an error - assert.NotNil(t, result) - textContent := getTextResult(t, result) - assert.Contains(t, textContent.Text, tc.expectedErrMsg) - } + require.True(t, res.IsError) + assert.Contains(t, text, tc.errContains) return } + require.NoError(t, err) + // Parse the structured response with pagination info + var response struct { + Issues []*github.Issue `json:"issues"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + StartCursor string `json:"startCursor"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + TotalCount int `json:"totalCount"` + } + err = json.Unmarshal([]byte(text), &response) require.NoError(t, err) - // Parse the result and get the text content if no error - textContent := getTextResult(t, result) + assert.Len(t, response.Issues, tc.expectedCount, "Expected %d issues, got %d", tc.expectedCount, len(response.Issues)) - // Unmarshal and verify the result - var returnedIssues []*github.Issue - err = json.Unmarshal([]byte(textContent.Text), &returnedIssues) - require.NoError(t, err) + // Verify order if verifyOrder function is provided + if tc.verifyOrder != nil { + tc.verifyOrder(t, response.Issues) + } - assert.Len(t, returnedIssues, len(tc.expectedIssues)) - for i, issue := range returnedIssues { - assert.Equal(t, *tc.expectedIssues[i].Number, *issue.Number) - assert.Equal(t, *tc.expectedIssues[i].Title, *issue.Title) - assert.Equal(t, *tc.expectedIssues[i].State, *issue.State) - assert.Equal(t, *tc.expectedIssues[i].HTMLURL, *issue.HTMLURL) + // Verify that returned issues have expected structure + for _, issue := range response.Issues { + assert.NotNil(t, issue.Number, "Issue should have number") + assert.NotNil(t, issue.Title, "Issue should have title") + assert.NotNil(t, issue.State, "Issue should have state") } }) } @@ -2601,3 +2827,146 @@ func Test_ReprioritizeSubIssue(t *testing.T) { }) } } + +func Test_ListIssueTypes(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := ListIssueTypes(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "list_issue_types", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner"}) + + // Setup mock issue types for success case + mockIssueTypes := []*github.IssueType{ + { + ID: github.Ptr(int64(1)), + Name: github.Ptr("bug"), + Description: github.Ptr("Something isn't working"), + Color: github.Ptr("d73a4a"), + }, + { + ID: github.Ptr(int64(2)), + Name: github.Ptr("feature"), + Description: github.Ptr("New feature or enhancement"), + Color: github.Ptr("a2eeef"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssueTypes []*github.IssueType + expectedErrMsg string + }{ + { + name: "successful issue types retrieval", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/orgs/testorg/issue-types", + Method: "GET", + }, + mockResponse(t, http.StatusOK, mockIssueTypes), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "testorg", + }, + expectError: false, + expectedIssueTypes: mockIssueTypes, + }, + { + name: "organization not found", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/orgs/nonexistent/issue-types", + Method: "GET", + }, + mockResponse(t, http.StatusNotFound, `{"message": "Organization not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "nonexistent", + }, + expectError: true, + expectedErrMsg: "failed to list issue types", + }, + { + name: "missing owner parameter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.EndpointPattern{ + Pattern: "/orgs/testorg/issue-types", + Method: "GET", + }, + mockResponse(t, http.StatusOK, mockIssueTypes), + ), + ), + requestArgs: map[string]interface{}{}, + expectError: false, // This should be handled by parameter validation, error returned in result + expectedErrMsg: "missing required parameter: owner", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := ListIssueTypes(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + if err != nil { + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + // Check if error is returned as tool result error + require.NotNil(t, result) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + // Check if it's a parameter validation error (returned as tool result error) + if result != nil && result.IsError { + errorContent := getErrorResult(t, result) + if tc.expectedErrMsg != "" && strings.Contains(errorContent.Text, tc.expectedErrMsg) { + return // This is expected for parameter validation errors + } + } + + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError) + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssueTypes []*github.IssueType + err = json.Unmarshal([]byte(textContent.Text), &returnedIssueTypes) + require.NoError(t, err) + + if tc.expectedIssueTypes != nil { + require.Equal(t, len(tc.expectedIssueTypes), len(returnedIssueTypes)) + for i, expected := range tc.expectedIssueTypes { + assert.Equal(t, *expected.Name, *returnedIssueTypes[i].Name) + assert.Equal(t, *expected.Description, *returnedIssueTypes[i].Description) + assert.Equal(t, *expected.Color, *returnedIssueTypes[i].Color) + assert.Equal(t, *expected.ID, *returnedIssueTypes[i].ID) + } + } + }) + } +} diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index fdd418098..0ee5a6b14 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -11,7 +11,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/notifications_test.go b/pkg/github/notifications_test.go index 1d2382369..268a29f6f 100644 --- a/pkg/github/notifications_test.go +++ b/pkg/github/notifications_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 47b7c6bd2..63c5594d3 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -8,7 +8,7 @@ import ( "net/http" "github.com/go-viper/mapstructure/v2" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" @@ -203,7 +203,7 @@ func CreatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu } // UpdatePullRequest creates a tool to update an existing pull request. -func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +func UpdatePullRequest(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (mcp.Tool, 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.WithToolAnnotation(mcp.ToolAnnotation{ @@ -232,12 +232,21 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu mcp.Description("New state"), mcp.Enum("open", "closed"), ), + mcp.WithBoolean("draft", + mcp.Description("Mark pull request as draft (true) or ready for review (false)"), + ), mcp.WithString("base", mcp.Description("New base branch name"), ), mcp.WithBoolean("maintainer_can_modify", mcp.Description("Allow maintainer edits"), ), + mcp.WithArray("reviewers", + mcp.Description("GitHub usernames to request reviews from"), + mcp.Items(map[string]interface{}{ + "type": "string", + }), + ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := RequiredParam[string](request, "owner") @@ -253,74 +262,211 @@ func UpdatePullRequest(getClient GetClientFn, t translations.TranslationHelperFu return mcp.NewToolResultError(err.Error()), nil } + // Check if draft parameter is provided + draftProvided := request.GetArguments()["draft"] != nil + var draftValue bool + if draftProvided { + draftValue, err = OptionalParam[bool](request, "draft") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + } + // Build the update struct only with provided fields update := &github.PullRequest{} - updateNeeded := false + restUpdateNeeded := 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 + restUpdateNeeded = 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 + restUpdateNeeded = 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 + restUpdateNeeded = 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 + restUpdateNeeded = 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 + restUpdateNeeded = true + } + + // Handle reviewers separately + reviewers, err := OptionalStringArrayParam(request, "reviewers") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil } - if !updateNeeded { + // If no updates, no draft change, and no reviewers, return error early + if !restUpdateNeeded && !draftProvided && len(reviewers) == 0 { 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) + // Handle REST API updates (title, body, state, base, maintainer_can_modify) + if restUpdateNeeded { + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + _, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, update) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to update pull request", + resp, + err, + ), nil + } + 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 + } } - pr, resp, err := client.PullRequests.Edit(ctx, owner, repo, pullNumber, update) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to update pull request", - resp, - err, - ), nil + + // Handle draft status changes using GraphQL + if draftProvided { + gqlClient, err := getGQLClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub GraphQL client: %w", err) + } + + var prQuery struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + err = gqlClient.Query(ctx, &prQuery, map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "prNum": githubv4.Int(pullNumber), // #nosec G115 - pull request numbers are always small positive integers + }) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to find pull request", err), nil + } + + currentIsDraft := bool(prQuery.Repository.PullRequest.IsDraft) + + if currentIsDraft != draftValue { + if draftValue { + // Convert to draft + var mutation struct { + ConvertPullRequestToDraft struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"convertPullRequestToDraft(input: $input)"` + } + + err = gqlClient.Mutate(ctx, &mutation, githubv4.ConvertPullRequestToDraftInput{ + PullRequestID: prQuery.Repository.PullRequest.ID, + }, nil) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to convert pull request to draft", err), nil + } + } else { + // Mark as ready for review + var mutation struct { + MarkPullRequestReadyForReview struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + } + + err = gqlClient.Mutate(ctx, &mutation, githubv4.MarkPullRequestReadyForReviewInput{ + PullRequestID: prQuery.Repository.PullRequest.ID, + }, nil) + if err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to mark pull request ready for review", err), nil + } + } + } } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) + // Handle reviewer requests + if len(reviewers) > 0 { + client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + reviewersRequest := github.ReviewersRequest{ + Reviewers: reviewers, + } + + _, resp, err := client.PullRequests.RequestReviewers(ctx, owner, repo, pullNumber, reviewersRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to request reviewers", + resp, + err, + ), nil + } + defer func() { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + }() + + if resp.StatusCode != http.StatusCreated && 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 request reviewers: %s", string(body))), nil } - return mcp.NewToolResultError(fmt.Sprintf("failed to update pull request: %s", string(body))), nil } - r, err := json.Marshal(pr) + // Get the final state of the PR to return + client, err := getClient(ctx) if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, err + } + + finalPR, resp, err := client.PullRequests.Get(ctx, owner, repo, pullNumber) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, "Failed to get pull request", resp, err), nil + } + defer func() { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } + }() + + r, err := json.Marshal(finalPR) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("Failed to marshal response: %v", err)), nil } return mcp.NewToolResultText(string(r)), nil @@ -545,10 +691,10 @@ func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperF mcp.Description("Search query using GitHub pull request search syntax"), ), mcp.WithString("owner", - mcp.Description("Optional repository owner. If provided with repo, only notifications for this repository are listed."), + mcp.Description("Optional repository owner. If provided with repo, only pull requests for this repository are listed."), ), mcp.WithString("repo", - mcp.Description("Optional repository name. If provided with owner, only notifications for this repository are listed."), + mcp.Description("Optional repository name. If provided with owner, only pull requests for this repository are listed."), ), mcp.WithString("sort", mcp.Description("Sort field by number of matches of categories, defaults to best match"), diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 42fd5bf03..ed6921477 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -10,7 +10,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/shurcooL/githubv4" "github.com/migueleliasweb/go-github-mock/src/mock" @@ -137,7 +137,7 @@ 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) + tool, _ := UpdatePullRequest(stubGetClientFn(mockClient), stubGetGQLClientFn(githubv4.NewClient(nil)), translations.NullTranslationHelper) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "update_pull_request", tool.Name) @@ -145,11 +145,13 @@ func Test_UpdatePullRequest(t *testing.T) { 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, "draft") 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.Contains(t, tool.InputSchema.Properties, "reviewers") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) // Setup mock PR for success case @@ -160,6 +162,7 @@ func Test_UpdatePullRequest(t *testing.T) { HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"), Body: github.Ptr("Updated test PR body."), MaintainerCanModify: github.Ptr(false), + Draft: github.Ptr(false), Base: &github.PullRequestBranch{ Ref: github.Ptr("develop"), }, @@ -171,6 +174,17 @@ func Test_UpdatePullRequest(t *testing.T) { State: github.Ptr("closed"), // State updated } + // Mock PR for when there are no updates but we still need a response + mockPRWithReviewers := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Test PR"), + State: github.Ptr("open"), + RequestedReviewers: []*github.User{ + {Login: github.Ptr("reviewer1")}, + {Login: github.Ptr("reviewer2")}, + }, + } + tests := []struct { name string mockedClient *http.Client @@ -194,6 +208,10 @@ func Test_UpdatePullRequest(t *testing.T) { mockResponse(t, http.StatusOK, mockUpdatedPR), ), ), + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + mockUpdatedPR, + ), ), requestArgs: map[string]interface{}{ "owner": "owner", @@ -218,6 +236,10 @@ func Test_UpdatePullRequest(t *testing.T) { mockResponse(t, http.StatusOK, mockClosedPR), ), ), + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + mockClosedPR, + ), ), requestArgs: map[string]interface{}{ "owner": "owner", @@ -228,6 +250,53 @@ func Test_UpdatePullRequest(t *testing.T) { expectError: false, expectedPR: mockClosedPR, }, + { + name: "successful PR update with reviewers", + mockedClient: mock.NewMockedHTTPClient( + // Mock for RequestReviewers call, returning the PR with reviewers + mock.WithRequestMatch( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + mockPRWithReviewers, + ), + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + mockPRWithReviewers, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "reviewers": []interface{}{"reviewer1", "reviewer2"}, + }, + expectError: false, + expectedPR: mockPRWithReviewers, + }, + { + name: "successful PR update (title only)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PatchReposPullsByOwnerByRepoByPullNumber, + expectRequestBody(t, map[string]interface{}{ + "title": "Updated Test PR Title", + }).andThen( + mockResponse(t, http.StatusOK, mockUpdatedPR), + ), + ), + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + mockUpdatedPR, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "title": "Updated Test PR Title", + }, + expectError: false, + expectedPR: mockUpdatedPR, + }, { name: "no update parameters provided", mockedClient: mock.NewMockedHTTPClient(), // No API call expected @@ -260,13 +329,34 @@ func Test_UpdatePullRequest(t *testing.T) { expectError: true, expectedErrMsg: "failed to update pull request", }, + { + name: "request reviewers fails", + mockedClient: mock.NewMockedHTTPClient( + // Then reviewer request fails + mock.WithRequestMatchHandler( + mock.PostReposPullsRequestedReviewersByOwnerByRepoByPullNumber, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Invalid reviewers"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "reviewers": []interface{}{"invalid-user"}, + }, + expectError: true, + expectedErrMsg: "failed to request reviewers", + }, } 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) + _, handler := UpdatePullRequest(stubGetClientFn(client), stubGetGQLClientFn(githubv4.NewClient(nil)), translations.NullTranslationHelper) // Create call request request := createMCPRequest(tc.requestArgs) @@ -312,6 +402,208 @@ func Test_UpdatePullRequest(t *testing.T) { if tc.expectedPR.MaintainerCanModify != nil { assert.Equal(t, *tc.expectedPR.MaintainerCanModify, *returnedPR.MaintainerCanModify) } + + // Check reviewers if they exist in the expected PR + if len(tc.expectedPR.RequestedReviewers) > 0 { + assert.NotNil(t, returnedPR.RequestedReviewers) + assert.Equal(t, len(tc.expectedPR.RequestedReviewers), len(returnedPR.RequestedReviewers)) + + // Create maps of reviewer logins for easy comparison + expectedReviewers := make(map[string]bool) + for _, reviewer := range tc.expectedPR.RequestedReviewers { + expectedReviewers[*reviewer.Login] = true + } + + actualReviewers := make(map[string]bool) + for _, reviewer := range returnedPR.RequestedReviewers { + actualReviewers[*reviewer.Login] = true + } + + // Compare the maps + assert.Equal(t, expectedReviewers, actualReviewers) + } + }) + } +} + +func Test_UpdatePullRequest_Draft(t *testing.T) { + // Setup mock PR for success case + mockUpdatedPR := &github.PullRequest{ + Number: github.Ptr(42), + Title: github.Ptr("Test PR Title"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42"), + Body: github.Ptr("Test PR body."), + MaintainerCanModify: github.Ptr(false), + Draft: github.Ptr(false), // Updated to ready for review + Base: &github.PullRequestBranch{ + Ref: github.Ptr("main"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedPR *github.PullRequest + expectedErrMsg string + }{ + { + name: "successful draft update to ready for review", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "prNum": githubv4.Int(42), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "id": "PR_kwDOA0xdyM50BPaO", + "isDraft": true, // Current state is draft + }, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + MarkPullRequestReadyForReview struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + }{}, + githubv4.MarkPullRequestReadyForReviewInput{ + PullRequestID: "PR_kwDOA0xdyM50BPaO", + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "markPullRequestReadyForReview": map[string]any{ + "pullRequest": map[string]any{ + "id": "PR_kwDOA0xdyM50BPaO", + "isDraft": false, + }, + }, + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "draft": false, + }, + expectError: false, + expectedPR: mockUpdatedPR, + }, + { + name: "successful convert pull request to draft", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } `graphql:"pullRequest(number: $prNum)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "prNum": githubv4.Int(42), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "pullRequest": map[string]any{ + "id": "PR_kwDOA0xdyM50BPaO", + "isDraft": false, // Current state is draft + }, + }, + }), + ), + githubv4mock.NewMutationMatcher( + struct { + ConvertPullRequestToDraft struct { + PullRequest struct { + ID githubv4.ID + IsDraft githubv4.Boolean + } + } `graphql:"convertPullRequestToDraft(input: $input)"` + }{}, + githubv4.ConvertPullRequestToDraftInput{ + PullRequestID: "PR_kwDOA0xdyM50BPaO", + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "convertPullRequestToDraft": map[string]any{ + "pullRequest": map[string]any{ + "id": "PR_kwDOA0xdyM50BPaO", + "isDraft": true, + }, + }, + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "draft": true, + }, + expectError: false, + expectedPR: mockUpdatedPR, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // For draft-only tests, we need to mock both GraphQL and the final REST GET call + restClient := github.NewClient(mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposPullsByOwnerByRepoByPullNumber, + mockUpdatedPR, + ), + )) + gqlClient := githubv4.NewClient(tc.mockedClient) + + _, handler := UpdatePullRequest(stubGetClientFn(restClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + + request := createMCPRequest(tc.requestArgs) + + result, err := handler(context.Background(), request) + + if tc.expectError || tc.expectedErrMsg != "" { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + if tc.expectedErrMsg != "" { + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + } + return + } + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + + // 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) }) } } @@ -738,6 +1030,77 @@ func Test_SearchPullRequests(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing is:pr filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server is:open draft:false", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:pr repo:github/github-mcp-server is:open draft:false", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "query with existing repo: filter and conflicting owner/repo params - uses query filter", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server author:octocat", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:github/github-mcp-server author:octocat", + "owner": "different-owner", + "repo": "different-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing is:pr filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search pull requests fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index ecd36d7e0..0925829a1 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -13,7 +13,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) @@ -1321,6 +1321,126 @@ func GetTag(getClient GetClientFn, t translations.TranslationHelperFunc) (tool m } } +// ListReleases creates a tool to list releases in a GitHub repository. +func ListReleases(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("list_releases", + mcp.WithDescription(t("TOOL_LIST_RELEASES_DESCRIPTION", "List releases in a GitHub repository")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_RELEASES_USER_TITLE", "List releases"), + ReadOnlyHint: ToBoolPtr(true), + }), + 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.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) + } + + releases, resp, err := client.Repositories.ListReleases(ctx, owner, repo, opts) + if err != nil { + return nil, fmt.Errorf("failed to list releases: %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 releases: %s", string(body))), nil + } + + r, err := json.Marshal(releases) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + +// GetLatestRelease creates a tool to get the latest release in a GitHub repository. +func GetLatestRelease(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_latest_release", + mcp.WithDescription(t("TOOL_GET_LATEST_RELEASE_DESCRIPTION", "Get the latest release in a GitHub repository")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_LATEST_RELEASE_USER_TITLE", "Get latest release"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + ), + 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 + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + release, resp, err := client.Repositories.GetLatestRelease(ctx, owner, repo) + if err != nil { + return nil, fmt.Errorf("failed to get latest release: %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 latest release: %s", string(body))), nil + } + + r, err := json.Marshal(release) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // filterPaths filters the entries in a GitHub tree to find paths that // match the given suffix. // maxResults limits the number of results returned to first maxResults entries, @@ -1358,36 +1478,100 @@ func filterPaths(entries []*github.TreeEntry, path string, maxResults int) []str return matchedPaths } -// resolveGitReference resolves git references with the following logic: -// 1. If SHA is provided, it takes precedence -// 2. If neither is provided, use the default branch as ref -// 3. Get commit SHA from the ref -// Refs can look like `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head` -// The function returns the resolved ref, commit SHA and any error. +// resolveGitReference takes a user-provided ref and sha and resolves them into a +// definitive commit SHA and its corresponding fully-qualified reference. +// +// The resolution logic follows a clear priority: +// +// 1. If a specific commit `sha` is provided, it takes precedence and is used directly, +// and all reference resolution is skipped. +// +// 2. If no `sha` is provided, the function resolves the `ref` +// string into a fully-qualified format (e.g., "refs/heads/main") by trying +// the following steps in order: +// a). **Empty Ref:** If `ref` is empty, the repository's default branch is used. +// b). **Fully-Qualified:** If `ref` already starts with "refs/", it's considered fully +// qualified and used as-is. +// c). **Partially-Qualified:** If `ref` starts with "heads/" or "tags/", it is +// prefixed with "refs/" to make it fully-qualified. +// d). **Short Name:** Otherwise, the `ref` is treated as a short name. The function +// first attempts to resolve it as a branch ("refs/heads/"). If that +// returns a 404 Not Found error, it then attempts to resolve it as a tag +// ("refs/tags/"). +// +// 3. **Final Lookup:** Once a fully-qualified ref is determined, a final API call +// is made to fetch that reference's definitive commit SHA. +// +// Any unexpected (non-404) errors during the resolution process are returned +// immediately. All API errors are logged with rich context to aid diagnostics. func resolveGitReference(ctx context.Context, githubClient *github.Client, owner, repo, ref, sha string) (*raw.ContentOpts, error) { - // 1. If SHA is provided, use it directly + // 1) If SHA explicitly provided, it's the highest priority. if sha != "" { return &raw.ContentOpts{Ref: "", SHA: sha}, nil } - // 2. If neither provided, use the default branch as ref - if ref == "" { + originalRef := ref // Keep original ref for clearer error messages down the line. + + // 2) If no SHA is provided, we try to resolve the ref into a fully-qualified format. + var reference *github.Reference + var resp *github.Response + var err error + + switch { + case originalRef == "": + // 2a) If ref is empty, determine the default branch. repoInfo, resp, err := githubClient.Repositories.Get(ctx, owner, repo) if err != nil { _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get repository info", resp, err) return nil, fmt.Errorf("failed to get repository info: %w", err) } ref = fmt.Sprintf("refs/heads/%s", repoInfo.GetDefaultBranch()) + case strings.HasPrefix(originalRef, "refs/"): + // 2b) Already fully qualified. The reference will be fetched at the end. + case strings.HasPrefix(originalRef, "heads/") || strings.HasPrefix(originalRef, "tags/"): + // 2c) Partially qualified. Make it fully qualified. + ref = "refs/" + originalRef + default: + // 2d) It's a short name, so we try to resolve it to either a branch or a tag. + branchRef := "refs/heads/" + originalRef + reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, branchRef) + + if err == nil { + ref = branchRef // It's a branch. + } else { + // The branch lookup failed. Check if it was a 404 Not Found error. + ghErr, isGhErr := err.(*github.ErrorResponse) + if isGhErr && ghErr.Response.StatusCode == http.StatusNotFound { + tagRef := "refs/tags/" + originalRef + reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, tagRef) + if err == nil { + ref = tagRef // It's a tag. + } else { + // The tag lookup also failed. Check if it was a 404 Not Found error. + ghErr2, isGhErr2 := err.(*github.ErrorResponse) + if isGhErr2 && ghErr2.Response.StatusCode == http.StatusNotFound { + return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef) + } + // The tag lookup failed for a different reason. + _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err) + return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err) + } + } else { + // The branch lookup failed for a different reason. + _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (branch)", resp, err) + return nil, fmt.Errorf("failed to get reference for branch '%s': %w", originalRef, err) + } + } } - // 3. Get the SHA from the ref - reference, resp, err := githubClient.Git.GetRef(ctx, owner, repo, ref) - if err != nil { - _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference", resp, err) - return nil, fmt.Errorf("failed to get reference: %w", err) + if reference == nil { + reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, ref) + if err != nil { + _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get final reference", resp, err) + return nil, fmt.Errorf("failed to get final reference for %q: %w", ref, err) + } } - sha = reference.GetObject().GetSHA() - // Use provided ref, or it will be empty which defaults to the default branch + sha = reference.GetObject().GetSHA() return &raw.ContentOpts{Ref: ref, SHA: sha}, nil } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 1572a12f4..63e577600 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -6,13 +6,14 @@ import ( "encoding/json" "net/http" "net/url" + "strings" "testing" "time" "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" @@ -2113,6 +2114,179 @@ func Test_GetTag(t *testing.T) { } } +func Test_ListReleases(t *testing.T) { + mockClient := github.NewClient(nil) + tool, _ := ListReleases(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "list_releases", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + mockReleases := []*github.RepositoryRelease{ + { + ID: github.Ptr(int64(1)), + TagName: github.Ptr("v1.0.0"), + Name: github.Ptr("First Release"), + }, + { + ID: github.Ptr(int64(2)), + TagName: github.Ptr("v0.9.0"), + Name: github.Ptr("Beta Release"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult []*github.RepositoryRelease + expectedErrMsg string + }{ + { + name: "successful releases list", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposReleasesByOwnerByRepo, + mockReleases, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedResult: mockReleases, + }, + { + name: "releases list fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposReleasesByOwnerByRepo, + 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", + }, + expectError: true, + expectedErrMsg: "failed to list releases", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + _, handler := ListReleases(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) + var returnedReleases []*github.RepositoryRelease + err = json.Unmarshal([]byte(textContent.Text), &returnedReleases) + require.NoError(t, err) + assert.Len(t, returnedReleases, len(tc.expectedResult)) + for i, rel := range returnedReleases { + assert.Equal(t, *tc.expectedResult[i].TagName, *rel.TagName) + } + }) + } +} +func Test_GetLatestRelease(t *testing.T) { + mockClient := github.NewClient(nil) + tool, _ := GetLatestRelease(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "get_latest_release", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"}) + + mockRelease := &github.RepositoryRelease{ + ID: github.Ptr(int64(1)), + TagName: github.Ptr("v1.0.0"), + Name: github.Ptr("First Release"), + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.RepositoryRelease + expectedErrMsg string + }{ + { + name: "successful latest release fetch", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposReleasesLatestByOwnerByRepo, + mockRelease, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + expectError: false, + expectedResult: mockRelease, + }, + { + name: "latest release fetch fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposReleasesLatestByOwnerByRepo, + 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", + }, + expectError: true, + expectedErrMsg: "failed to get latest release", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := github.NewClient(tc.mockedClient) + _, handler := GetLatestRelease(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) + var returnedRelease github.RepositoryRelease + err = json.Unmarshal([]byte(textContent.Text), &returnedRelease) + require.NoError(t, err) + assert.Equal(t, *tc.expectedResult.TagName, *returnedRelease.TagName) + }) + } +} + func Test_filterPaths(t *testing.T) { tests := []struct { name string @@ -2212,63 +2386,239 @@ func Test_resolveGitReference(t *testing.T) { ctx := context.Background() owner := "owner" repo := "repo" - mockedClient := mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.GetReposByOwnerByRepo, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"name": "repo", "default_branch": "main"}`)) - }), - ), - mock.WithRequestMatchHandler( - mock.GetReposGitRefByOwnerByRepoByRef, - http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "123sha456"}}`)) - }), - ), - ) tests := []struct { name string ref string sha string + mockSetup func() *http.Client expectedOutput *raw.ContentOpts + expectError bool + errorContains string }{ { name: "sha takes precedence over ref", ref: "refs/heads/main", sha: "123sha456", + mockSetup: func() *http.Client { + // No API calls should be made when SHA is provided + return mock.NewMockedHTTPClient() + }, expectedOutput: &raw.ContentOpts{ SHA: "123sha456", }, + expectError: false, }, { name: "use default branch if ref and sha both empty", ref: "", sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"name": "repo", "default_branch": "main"}`)) + }), + ), + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "/git/ref/heads/main") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "main-sha"}}`)) + }), + ), + ) + }, expectedOutput: &raw.ContentOpts{ Ref: "refs/heads/main", - SHA: "123sha456", + SHA: "main-sha", }, + expectError: false, }, { - name: "get SHA from ref", - ref: "refs/heads/main", + name: "fully qualified ref passed through unchanged", + ref: "refs/heads/feature-branch", sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "/git/ref/heads/feature-branch") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/heads/feature-branch", "object": {"sha": "feature-sha"}}`)) + }), + ), + ) + }, + expectedOutput: &raw.ContentOpts{ + Ref: "refs/heads/feature-branch", + SHA: "feature-sha", + }, + expectError: false, + }, + { + name: "short branch name resolves to refs/heads/", + ref: "main", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/git/ref/heads/main") { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/heads/main", "object": {"sha": "main-sha"}}`)) + } else { + t.Errorf("Unexpected path: %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + }), + ), + ) + }, expectedOutput: &raw.ContentOpts{ Ref: "refs/heads/main", - SHA: "123sha456", + SHA: "main-sha", + }, + expectError: false, + }, + { + name: "short tag name falls back to refs/tags/ when branch not found", + ref: "v1.0.0", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.Contains(r.URL.Path, "/git/ref/heads/v1.0.0"): + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + case strings.Contains(r.URL.Path, "/git/ref/tags/v1.0.0"): + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/tags/v1.0.0", "object": {"sha": "tag-sha"}}`)) + default: + t.Errorf("Unexpected path: %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + }), + ), + ) + }, + expectedOutput: &raw.ContentOpts{ + Ref: "refs/tags/v1.0.0", + SHA: "tag-sha", + }, + expectError: false, + }, + { + name: "heads/ prefix gets refs/ prepended", + ref: "heads/feature-branch", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "/git/ref/heads/feature-branch") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/heads/feature-branch", "object": {"sha": "feature-sha"}}`)) + }), + ), + ) }, + expectedOutput: &raw.ContentOpts{ + Ref: "refs/heads/feature-branch", + SHA: "feature-sha", + }, + expectError: false, + }, + { + name: "tags/ prefix gets refs/ prepended", + ref: "tags/v1.0.0", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "/git/ref/tags/v1.0.0") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/tags/v1.0.0", "object": {"sha": "tag-sha"}}`)) + }), + ), + ) + }, + expectedOutput: &raw.ContentOpts{ + Ref: "refs/tags/v1.0.0", + SHA: "tag-sha", + }, + expectError: false, + }, + { + name: "invalid short name that doesn't exist as branch or tag", + ref: "nonexistent", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // Both branch and tag attempts should return 404 + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }), + ), + ) + }, + expectError: true, + errorContains: "could not resolve ref \"nonexistent\" as a branch or a tag", + }, + { + name: "fully qualified pull request ref", + ref: "refs/pull/123/head", + sha: "", + mockSetup: func() *http.Client { + return mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposGitRefByOwnerByRepoByRef, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Contains(t, r.URL.Path, "/git/ref/pull/123/head") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ref": "refs/pull/123/head", "object": {"sha": "pr-sha"}}`)) + }), + ), + ) + }, + expectedOutput: &raw.ContentOpts{ + Ref: "refs/pull/123/head", + SHA: "pr-sha", + }, + expectError: false, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // Setup client with mock - client := github.NewClient(mockedClient) + client := github.NewClient(tc.mockSetup()) opts, err := resolveGitReference(ctx, client, owner, repo, tc.ref, tc.sha) + + if tc.expectError { + require.Error(t, err) + if tc.errorContains != "" { + assert.Contains(t, err.Error(), tc.errorContains) + } + return + } + require.NoError(t, err) + require.NotNil(t, opts) if tc.expectedOutput.SHA != "" { assert.Equal(t, tc.expectedOutput.SHA, opts.SHA) diff --git a/pkg/github/repository_resource.go b/pkg/github/repository_resource.go index 70ca6ba65..b76c0b1e8 100644 --- a/pkg/github/repository_resource.go +++ b/pkg/github/repository_resource.go @@ -14,7 +14,7 @@ import ( "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/repository_resource_test.go b/pkg/github/repository_resource_test.go index 2e3e911a9..1c048c000 100644 --- a/pkg/github/repository_resource_test.go +++ b/pkg/github/repository_resource_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/require" diff --git a/pkg/github/search.go b/pkg/github/search.go index 476ac0151..248f17e17 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -8,7 +8,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) @@ -16,14 +16,15 @@ import ( // 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.WithDescription(t("TOOL_SEARCH_REPOSITORIES_DESCRIPTION", "Find GitHub repositories by name, description, readme, topics, or other metadata. Perfect for discovering projects, finding examples, or locating specific repositories across GitHub.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_SEARCH_REPOSITORIES_USER_TITLE", "Search repositories"), ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("query", mcp.Required(), - mcp.Description("Search query"), + mcp.Description("Repository search query. Examples: 'machine learning in:name stars:>1000 language:python', 'topic:react', 'user:facebook'. Supports advanced search syntax for precise filtering."), ), WithPagination(), ), @@ -78,20 +79,20 @@ func SearchRepositories(getClient GetClientFn, t translations.TranslationHelperF // 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.WithDescription(t("TOOL_SEARCH_CODE_DESCRIPTION", "Fast and precise code search across ALL GitHub repositories using GitHub's native search engine. Best for finding exact symbols, functions, classes, or specific code patterns.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_SEARCH_CODE_USER_TITLE", "Search code"), ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("query", mcp.Required(), - mcp.Description("Search query using GitHub code search syntax"), + mcp.Description("Search query using GitHub's powerful code search syntax. Examples: 'content:Skill language:Java org:github', 'NOT is:archived language:Python OR language:go', 'repo:github/github-mcp-server'. Supports exact matching, language filters, path filters, and more."), ), mcp.WithString("sort", mcp.Description("Sort field ('indexed' only)"), ), mcp.WithString("order", - mcp.Description("Sort order"), + mcp.Description("Sort order for results"), mcp.Enum("asc", "desc"), ), WithPagination(), @@ -203,7 +204,10 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - searchQuery := "type:" + accountType + " " + query + searchQuery := query + if !hasTypeFilter(query) { + searchQuery = "type:" + accountType + " " + query + } result, resp, err := client.Search.Users(ctx, searchQuery, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, @@ -258,17 +262,17 @@ func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHand // 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 exclusively")), + mcp.WithDescription(t("TOOL_SEARCH_USERS_DESCRIPTION", "Find GitHub users by username, real name, or other profile information. Useful for locating developers, contributors, or team members.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_SEARCH_USERS_USER_TITLE", "Search users"), ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("query", mcp.Required(), - mcp.Description("Search query using GitHub users search syntax scoped to type:user"), + mcp.Description("User search query. Examples: 'john smith', 'location:seattle', 'followers:>100'. Search is automatically scoped to type:user."), ), mcp.WithString("sort", - mcp.Description("Sort field by category"), + mcp.Description("Sort users by number of followers or repositories, or when the person joined GitHub."), mcp.Enum("followers", "repositories", "joined"), ), mcp.WithString("order", @@ -282,14 +286,15 @@ func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (t // SearchOrgs creates a tool to search for GitHub organizations. func SearchOrgs(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_orgs", - mcp.WithDescription(t("TOOL_SEARCH_ORGS_DESCRIPTION", "Search for GitHub organizations exclusively")), + mcp.WithDescription(t("TOOL_SEARCH_ORGS_DESCRIPTION", "Find GitHub organizations by name, location, or other organization metadata. Ideal for discovering companies, open source foundations, or teams.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_SEARCH_ORGS_USER_TITLE", "Search organizations"), ReadOnlyHint: ToBoolPtr(true), }), mcp.WithString("query", mcp.Required(), - mcp.Description("Search query using GitHub organizations search syntax scoped to type:org"), + mcp.Description("Organization search query. Examples: 'microsoft', 'location:california', 'created:>=2025-01-01'. Search is automatically scoped to type:org."), ), mcp.WithString("sort", mcp.Description("Sort field by category"), diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index 9ea8e71ec..cfc87c02b 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -410,6 +410,46 @@ func Test_SearchUsers(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing type:user filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:user location:seattle followers:>100", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:user location:seattle followers:>100", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing type:user filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:user (location:seattle OR location:california) followers:>50", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:user (location:seattle OR location:california) followers:>50", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "search users fails", mockedClient: mock.NewMockedHTTPClient( @@ -537,6 +577,46 @@ func Test_SearchOrgs(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "query with existing type:org filter - no duplication", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org location:california followers:>1000", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:org location:california followers:>1000", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "complex query with existing type:org filter and OR operators", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org (location:seattle OR location:california OR location:newyork) repos:>10", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "type:org (location:seattle OR location:california OR location:newyork) repos:>10", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "org search fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index a6ff1f782..159518c91 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -6,11 +6,35 @@ import ( "fmt" "io" "net/http" + "regexp" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" ) +func hasFilter(query, filterType string) bool { + // Match filter at start of string, after whitespace, or after non-word characters like '(' + pattern := fmt.Sprintf(`(^|\s|\W)%s:\S+`, regexp.QuoteMeta(filterType)) + matched, _ := regexp.MatchString(pattern, query) + return matched +} + +func hasSpecificFilter(query, filterType, filterValue string) bool { + // Match specific filter:value at start, after whitespace, or after non-word characters + // End with word boundary, whitespace, or non-word characters like ')' + pattern := fmt.Sprintf(`(^|\s|\W)%s:%s($|\s|\W)`, regexp.QuoteMeta(filterType), regexp.QuoteMeta(filterValue)) + matched, _ := regexp.MatchString(pattern, query) + return matched +} + +func hasRepoFilter(query string) bool { + return hasFilter(query, "repo") +} + +func hasTypeFilter(query string) bool { + return hasFilter(query, "type") +} + func searchHandler( ctx context.Context, getClient GetClientFn, @@ -22,7 +46,10 @@ func searchHandler( if err != nil { return mcp.NewToolResultError(err.Error()), nil } - query = fmt.Sprintf("is:%s %s", searchType, query) + + if !hasSpecificFilter(query, "is", searchType) { + query = fmt.Sprintf("is:%s %s", searchType, query) + } owner, err := OptionalParam[string](request, "owner") if err != nil { @@ -34,7 +61,7 @@ func searchHandler( return mcp.NewToolResultError(err.Error()), nil } - if owner != "" && repo != "" { + if owner != "" && repo != "" && !hasRepoFilter(query) { query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) } diff --git a/pkg/github/search_utils_test.go b/pkg/github/search_utils_test.go new file mode 100644 index 000000000..85f953eed --- /dev/null +++ b/pkg/github/search_utils_test.go @@ -0,0 +1,352 @@ +package github + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_hasFilter(t *testing.T) { + tests := []struct { + name string + query string + filterType string + expected bool + }{ + { + name: "query has is:issue filter", + query: "is:issue bug report", + filterType: "is", + expected: true, + }, + { + name: "query has repo: filter", + query: "repo:github/github-mcp-server critical bug", + filterType: "repo", + expected: true, + }, + { + name: "query has multiple is: filters", + query: "is:issue is:open bug", + filterType: "is", + expected: true, + }, + { + name: "query has filter at the beginning", + query: "is:issue some text", + filterType: "is", + expected: true, + }, + { + name: "query has filter in the middle", + query: "some text is:issue more text", + filterType: "is", + expected: true, + }, + { + name: "query has filter at the end", + query: "some text is:issue", + filterType: "is", + expected: true, + }, + { + name: "query does not have the filter", + query: "bug report critical", + filterType: "is", + expected: false, + }, + { + name: "query has similar text but not the filter", + query: "this issue is important", + filterType: "is", + expected: false, + }, + { + name: "empty query", + query: "", + filterType: "is", + expected: false, + }, + { + name: "query has label: filter but looking for is:", + query: "label:bug critical", + filterType: "is", + expected: false, + }, + { + name: "query has author: filter", + query: "author:octocat bug", + filterType: "author", + expected: true, + }, + { + name: "query with complex OR expression", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "is", + expected: true, + }, + { + name: "query with complex OR expression checking repo", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "repo", + expected: true, + }, + { + name: "filter in parentheses at start", + query: "(label:bug OR owner:bob) is:issue", + filterType: "label", + expected: true, + }, + { + name: "filter after opening parenthesis", + query: "is:issue (label:critical OR repo:test/test)", + filterType: "label", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasFilter(tt.query, tt.filterType) + assert.Equal(t, tt.expected, result, "hasFilter(%q, %q) = %v, expected %v", tt.query, tt.filterType, result, tt.expected) + }) + } +} + +func Test_hasRepoFilter(t *testing.T) { + tests := []struct { + name string + query string + expected bool + }{ + { + name: "query with repo: filter at beginning", + query: "repo:github/github-mcp-server is:issue", + expected: true, + }, + { + name: "query with repo: filter in middle", + query: "is:issue repo:octocat/Hello-World bug", + expected: true, + }, + { + name: "query with repo: filter at end", + query: "is:issue critical repo:owner/repo-name", + expected: true, + }, + { + name: "query with complex repo name", + query: "repo:microsoft/vscode-extension-samples bug", + expected: true, + }, + { + name: "query without repo: filter", + query: "is:issue bug critical", + expected: false, + }, + { + name: "query with malformed repo: filter (no slash)", + query: "repo:github bug", + expected: true, // hasRepoFilter only checks for repo: prefix, not format + }, + { + name: "empty query", + query: "", + expected: false, + }, + { + name: "query with multiple repo: filters", + query: "repo:github/first repo:octocat/second", + expected: true, + }, + { + name: "query with repo: in text but not as filter", + query: "this repo: is important", + expected: false, + }, + { + name: "query with complex OR expression", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasRepoFilter(tt.query) + assert.Equal(t, tt.expected, result, "hasRepoFilter(%q) = %v, expected %v", tt.query, result, tt.expected) + }) + } +} + +func Test_hasSpecificFilter(t *testing.T) { + tests := []struct { + name string + query string + filterType string + filterValue string + expected bool + }{ + { + name: "query has exact is:issue filter", + query: "is:issue bug report", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:open but looking for is:issue", + query: "is:open bug report", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "query has both is:issue and is:open, looking for is:issue", + query: "is:issue is:open bug", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has both is:issue and is:open, looking for is:open", + query: "is:issue is:open bug", + filterType: "is", + filterValue: "open", + expected: true, + }, + { + name: "query has is:issue at the beginning", + query: "is:issue some text", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:issue in the middle", + query: "some text is:issue more text", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query has is:issue at the end", + query: "some text is:issue", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "query does not have is:issue", + query: "bug report critical", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "query has similar text but not the exact filter", + query: "this issue is important", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "empty query", + query: "", + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "partial match should not count", + query: "is:issues bug", // "issues" vs "issue" + filterType: "is", + filterValue: "issue", + expected: false, + }, + { + name: "complex query with parentheses", + query: "repo:github/github-mcp-server is:issue (label:critical OR label:urgent)", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "filter:value in parentheses at start", + query: "(is:issue OR is:pr) label:bug", + filterType: "is", + filterValue: "issue", + expected: true, + }, + { + name: "filter:value after opening parenthesis", + query: "repo:test/repo (is:issue AND label:bug)", + filterType: "is", + filterValue: "issue", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasSpecificFilter(tt.query, tt.filterType, tt.filterValue) + assert.Equal(t, tt.expected, result, "hasSpecificFilter(%q, %q, %q) = %v, expected %v", tt.query, tt.filterType, tt.filterValue, result, tt.expected) + }) + } +} + +func Test_hasTypeFilter(t *testing.T) { + tests := []struct { + name string + query string + expected bool + }{ + { + name: "query with type:user filter at beginning", + query: "type:user location:seattle", + expected: true, + }, + { + name: "query with type:org filter in middle", + query: "location:california type:org followers:>100", + expected: true, + }, + { + name: "query with type:user filter at end", + query: "location:seattle followers:>50 type:user", + expected: true, + }, + { + name: "query without type: filter", + query: "location:seattle followers:>50", + expected: false, + }, + { + name: "empty query", + query: "", + expected: false, + }, + { + name: "query with type: in text but not as filter", + query: "this type: is important", + expected: false, + }, + { + name: "query with multiple type: filters", + query: "type:user type:org", + expected: true, + }, + { + name: "complex query with OR expression", + query: "type:user (location:seattle OR location:california)", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasTypeFilter(tt.query) + assert.Equal(t, tt.expected, result, "hasTypeFilter(%q) = %v, expected %v", tt.query, result, tt.expected) + }) + } +} diff --git a/pkg/github/secret_scanning.go b/pkg/github/secret_scanning.go index dc199b4e6..c140c34ad 100644 --- a/pkg/github/secret_scanning.go +++ b/pkg/github/secret_scanning.go @@ -9,7 +9,7 @@ import ( ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/secret_scanning_test.go b/pkg/github/secret_scanning_test.go index 96b281830..ce33fe318 100644 --- a/pkg/github/secret_scanning_test.go +++ b/pkg/github/secret_scanning_test.go @@ -7,7 +7,7 @@ import ( "testing" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/pkg/github/server.go b/pkg/github/server.go index 193336b75..80a1bbac6 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -5,7 +5,7 @@ import ( "errors" "fmt" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" ) diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 7f8f29c0d..f38c4dc01 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -9,7 +9,7 @@ import ( "testing" "github.com/github/github-mcp-server/pkg/raw" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" ) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index e01b7cc40..3fb39ada7 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -6,7 +6,7 @@ import ( "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/toolsets" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" ) @@ -31,6 +31,8 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(ListBranches(getClient, t)), toolsets.NewServerTool(ListTags(getClient, t)), toolsets.NewServerTool(GetTag(getClient, t)), + toolsets.NewServerTool(ListReleases(getClient, t)), + toolsets.NewServerTool(GetLatestRelease(getClient, t)), ). AddWriteTools( toolsets.NewServerTool(CreateOrUpdateFile(getClient, t)), @@ -51,8 +53,9 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG AddReadTools( toolsets.NewServerTool(GetIssue(getClient, t)), toolsets.NewServerTool(SearchIssues(getClient, t)), - toolsets.NewServerTool(ListIssues(getClient, t)), + toolsets.NewServerTool(ListIssues(getGQLClient, t)), toolsets.NewServerTool(GetIssueComments(getClient, t)), + toolsets.NewServerTool(ListIssueTypes(getClient, t)), toolsets.NewServerTool(ListSubIssues(getClient, t)), ). AddWriteTools( @@ -63,7 +66,10 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(AddSubIssue(getClient, t)), toolsets.NewServerTool(RemoveSubIssue(getClient, t)), toolsets.NewServerTool(ReprioritizeSubIssue(getClient, t)), - ).AddPrompts(toolsets.NewServerPrompt(AssignCodingAgentPrompt(t))) + ).AddPrompts( + toolsets.NewServerPrompt(AssignCodingAgentPrompt(t)), + toolsets.NewServerPrompt(IssueToFixWorkflowPrompt(t)), + ) users := toolsets.NewToolset("users", "GitHub User related tools"). AddReadTools( toolsets.NewServerTool(SearchUsers(getClient, t)), @@ -87,7 +93,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(MergePullRequest(getClient, t)), toolsets.NewServerTool(UpdatePullRequestBranch(getClient, t)), toolsets.NewServerTool(CreatePullRequest(getClient, t)), - toolsets.NewServerTool(UpdatePullRequest(getClient, t)), + toolsets.NewServerTool(UpdatePullRequest(getClient, getGQLClient, t)), toolsets.NewServerTool(RequestCopilotReview(getClient, t)), // Reviews @@ -159,6 +165,17 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG 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)), + toolsets.NewServerTool(GetTeams(getClient, getGQLClient, t)), + toolsets.NewServerTool(GetTeamMembers(getGQLClient, t)), + ) + + gists := toolsets.NewToolset("gists", "GitHub Gist related tools"). + AddReadTools( + toolsets.NewServerTool(ListGists(getClient, t)), + ). + AddWriteTools( + toolsets.NewServerTool(CreateGist(getClient, t)), + toolsets.NewServerTool(UpdateGist(getClient, t)), ) // Add toolsets to the group @@ -175,6 +192,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG tsg.AddToolset(notifications) tsg.AddToolset(experiments) tsg.AddToolset(discussions) + tsg.AddToolset(gists) return tsg } diff --git a/pkg/github/workflow_prompts.go b/pkg/github/workflow_prompts.go new file mode 100644 index 000000000..8a9545a42 --- /dev/null +++ b/pkg/github/workflow_prompts.go @@ -0,0 +1,77 @@ +package github + +import ( + "context" + "fmt" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// IssueToFixWorkflowPrompt provides a guided workflow for creating an issue and then generating a PR to fix it +func IssueToFixWorkflowPrompt(t translations.TranslationHelperFunc) (tool mcp.Prompt, handler server.PromptHandlerFunc) { + return mcp.NewPrompt("IssueToFixWorkflow", + mcp.WithPromptDescription(t("PROMPT_ISSUE_TO_FIX_WORKFLOW_DESCRIPTION", "Create an issue for a problem and then generate a pull request to fix it")), + mcp.WithArgument("owner", mcp.ArgumentDescription("Repository owner"), mcp.RequiredArgument()), + mcp.WithArgument("repo", mcp.ArgumentDescription("Repository name"), mcp.RequiredArgument()), + mcp.WithArgument("title", mcp.ArgumentDescription("Issue title"), mcp.RequiredArgument()), + mcp.WithArgument("description", mcp.ArgumentDescription("Issue description"), mcp.RequiredArgument()), + mcp.WithArgument("labels", mcp.ArgumentDescription("Comma-separated list of labels to apply (optional)")), + mcp.WithArgument("assignees", mcp.ArgumentDescription("Comma-separated list of assignees (optional)")), + ), func(_ context.Context, request mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { + owner := request.Params.Arguments["owner"] + repo := request.Params.Arguments["repo"] + title := request.Params.Arguments["title"] + description := request.Params.Arguments["description"] + + labels := "" + if l, exists := request.Params.Arguments["labels"]; exists { + labels = fmt.Sprintf("%v", l) + } + + assignees := "" + if a, exists := request.Params.Arguments["assignees"]; exists { + assignees = fmt.Sprintf("%v", a) + } + + messages := []mcp.PromptMessage{ + { + Role: "system", + Content: mcp.NewTextContent("You are a development workflow assistant helping to create GitHub issues and generate corresponding pull requests to fix them. You should: 1) Create a well-structured issue with clear problem description, 2) Assign it to Copilot coding agent to generate a solution, and 3) Monitor the PR creation process."), + }, + { + Role: "user", + Content: mcp.NewTextContent(fmt.Sprintf("I need to create an issue titled '%s' in %s/%s and then have a PR generated to fix it. The issue description is: %s%s%s", + title, owner, repo, description, + func() string { + if labels != "" { + return fmt.Sprintf("\n\nLabels to apply: %s", labels) + } + return "" + }(), + func() string { + if assignees != "" { + return fmt.Sprintf("\nAssignees: %s", assignees) + } + return "" + }())), + }, + { + Role: "assistant", + Content: mcp.NewTextContent(fmt.Sprintf("I'll help you create the issue '%s' in %s/%s and then coordinate with Copilot to generate a fix. Let me start by creating the issue with the provided details.", title, owner, repo)), + }, + { + Role: "user", + Content: mcp.NewTextContent("Perfect! Please:\n1. Create the issue with the title, description, labels, and assignees\n2. Once created, assign it to Copilot coding agent to generate a solution\n3. Monitor the process and let me know when the PR is ready for review"), + }, + { + Role: "assistant", + Content: mcp.NewTextContent("Excellent plan! Here's what I'll do:\n\n1. ✅ Create the issue with all specified details\n2. 🤖 Assign to Copilot coding agent for automated fix\n3. 📋 Monitor progress and notify when PR is created\n4. 🔍 Provide PR details for your review\n\nLet me start by creating the issue."), + }, + } + return &mcp.GetPromptResult{ + Messages: messages, + }, nil + } +} diff --git a/pkg/log/io.go b/pkg/log/io.go index de2210278..44b8dc17a 100644 --- a/pkg/log/io.go +++ b/pkg/log/io.go @@ -3,7 +3,7 @@ package log import ( "io" - log "github.com/sirupsen/logrus" + "log/slog" ) // IOLogger is a wrapper around io.Reader and io.Writer that can be used @@ -11,11 +11,11 @@ import ( type IOLogger struct { reader io.Reader writer io.Writer - logger *log.Logger + logger *slog.Logger } // NewIOLogger creates a new IOLogger instance -func NewIOLogger(r io.Reader, w io.Writer, logger *log.Logger) *IOLogger { +func NewIOLogger(r io.Reader, w io.Writer, logger *slog.Logger) *IOLogger { return &IOLogger{ reader: r, writer: w, @@ -30,7 +30,7 @@ func (l *IOLogger) Read(p []byte) (n int, err error) { } n, err = l.reader.Read(p) if n > 0 { - l.logger.Infof("[stdin]: received %d bytes: %s", n, string(p[:n])) + l.logger.Info("[stdin]: received bytes", "count", n, "data", string(p[:n])) } return n, err } @@ -40,6 +40,6 @@ func (l *IOLogger) Write(p []byte) (n int, err error) { if l.writer == nil { return 0, io.ErrClosedPipe } - l.logger.Infof("[stdout]: sending %d bytes: %s", len(p), string(p)) + l.logger.Info("[stdout]: sending bytes", "count", len(p), "data", string(p)) return l.writer.Write(p) } diff --git a/pkg/log/io_test.go b/pkg/log/io_test.go index 0d0cd8959..2661de164 100644 --- a/pkg/log/io_test.go +++ b/pkg/log/io_test.go @@ -5,7 +5,8 @@ import ( "strings" "testing" - log "github.com/sirupsen/logrus" + "log/slog" + "github.com/stretchr/testify/assert" ) @@ -17,11 +18,7 @@ func TestLoggedReadWriter(t *testing.T) { // Create logger with buffer to capture output var logBuffer bytes.Buffer - logger := log.New() - logger.SetOutput(&logBuffer) - logger.SetFormatter(&log.TextFormatter{ - DisableTimestamp: true, - }) + logger := slog.New(slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{ReplaceAttr: removeTimeAttr})) lrw := NewIOLogger(reader, nil, logger) @@ -44,11 +41,7 @@ func TestLoggedReadWriter(t *testing.T) { // Create logger with buffer to capture output var logBuffer bytes.Buffer - logger := log.New() - logger.SetOutput(&logBuffer) - logger.SetFormatter(&log.TextFormatter{ - DisableTimestamp: true, - }) + logger := slog.New(slog.NewTextHandler(&logBuffer, &slog.HandlerOptions{ReplaceAttr: removeTimeAttr})) lrw := NewIOLogger(nil, &writeBuffer, logger) @@ -63,3 +56,10 @@ func TestLoggedReadWriter(t *testing.T) { assert.Contains(t, logBuffer.String(), outputData) }) } + +func removeTimeAttr(groups []string, a slog.Attr) slog.Attr { + if a.Key == slog.TimeKey && len(groups) == 0 { + return slog.Attr{} + } + return a +} diff --git a/pkg/raw/raw.go b/pkg/raw/raw.go index af669c905..ddf88b428 100644 --- a/pkg/raw/raw.go +++ b/pkg/raw/raw.go @@ -6,7 +6,7 @@ import ( "net/http" "net/url" - gogithub "github.com/google/go-github/v73/github" + gogithub "github.com/google/go-github/v74/github" ) // GetRawClientFn is a function type that returns a RawClient instance. diff --git a/pkg/raw/raw_test.go b/pkg/raw/raw_test.go index 18a48130d..4e5bdce7a 100644 --- a/pkg/raw/raw_test.go +++ b/pkg/raw/raw_test.go @@ -6,7 +6,7 @@ import ( "net/url" "testing" - "github.com/google/go-github/v73/github" + "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" "github.com/stretchr/testify/require" ) diff --git a/third-party-licenses.darwin.md b/third-party-licenses.darwin.md index 6a9f895cb..88ee6a838 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/go-openapi/swag](https://pkg.go.dev/github.com/go-openapi/swag) ([Apache-2.0](https://github.com/go-openapi/swag/blob/v0.21.1/LICENSE)) - [github.com/go-viper/mapstructure/v2](https://pkg.go.dev/github.com/go-viper/mapstructure/v2) ([MIT](https://github.com/go-viper/mapstructure/blob/v2.3.0/LICENSE)) - [github.com/google/go-github/v71/github](https://pkg.go.dev/github.com/google/go-github/v71/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v71.0.0/LICENSE)) - - [github.com/google/go-github/v73/github](https://pkg.go.dev/github.com/google/go-github/v73/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v73.0.0/LICENSE)) + - [github.com/google/go-github/v74/github](https://pkg.go.dev/github.com/google/go-github/v74/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v74.0.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/gorilla/mux](https://pkg.go.dev/github.com/gorilla/mux) ([BSD-3-Clause](https://github.com/gorilla/mux/blob/v1.8.0/LICENSE)) @@ -26,7 +26,6 @@ Some packages may only be included on certain architectures or operating systems - [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/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE)) - [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE)) - - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE)) - [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE)) - [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt)) - [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE)) diff --git a/third-party-licenses.linux.md b/third-party-licenses.linux.md index 6a9f895cb..88ee6a838 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/go-openapi/swag](https://pkg.go.dev/github.com/go-openapi/swag) ([Apache-2.0](https://github.com/go-openapi/swag/blob/v0.21.1/LICENSE)) - [github.com/go-viper/mapstructure/v2](https://pkg.go.dev/github.com/go-viper/mapstructure/v2) ([MIT](https://github.com/go-viper/mapstructure/blob/v2.3.0/LICENSE)) - [github.com/google/go-github/v71/github](https://pkg.go.dev/github.com/google/go-github/v71/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v71.0.0/LICENSE)) - - [github.com/google/go-github/v73/github](https://pkg.go.dev/github.com/google/go-github/v73/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v73.0.0/LICENSE)) + - [github.com/google/go-github/v74/github](https://pkg.go.dev/github.com/google/go-github/v74/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v74.0.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/gorilla/mux](https://pkg.go.dev/github.com/gorilla/mux) ([BSD-3-Clause](https://github.com/gorilla/mux/blob/v1.8.0/LICENSE)) @@ -26,7 +26,6 @@ Some packages may only be included on certain architectures or operating systems - [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/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE)) - [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE)) - - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE)) - [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE)) - [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt)) - [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE)) diff --git a/third-party-licenses.windows.md b/third-party-licenses.windows.md index 505c2d83e..bb742aeb0 100644 --- a/third-party-licenses.windows.md +++ b/third-party-licenses.windows.md @@ -13,7 +13,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/go-openapi/swag](https://pkg.go.dev/github.com/go-openapi/swag) ([Apache-2.0](https://github.com/go-openapi/swag/blob/v0.21.1/LICENSE)) - [github.com/go-viper/mapstructure/v2](https://pkg.go.dev/github.com/go-viper/mapstructure/v2) ([MIT](https://github.com/go-viper/mapstructure/blob/v2.3.0/LICENSE)) - [github.com/google/go-github/v71/github](https://pkg.go.dev/github.com/google/go-github/v71/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v71.0.0/LICENSE)) - - [github.com/google/go-github/v73/github](https://pkg.go.dev/github.com/google/go-github/v73/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v73.0.0/LICENSE)) + - [github.com/google/go-github/v74/github](https://pkg.go.dev/github.com/google/go-github/v74/github) ([BSD-3-Clause](https://github.com/google/go-github/blob/v74.0.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/gorilla/mux](https://pkg.go.dev/github.com/gorilla/mux) ([BSD-3-Clause](https://github.com/gorilla/mux/blob/v1.8.0/LICENSE)) @@ -27,7 +27,6 @@ Some packages may only be included on certain architectures or operating systems - [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/shurcooL/githubv4](https://pkg.go.dev/github.com/shurcooL/githubv4) ([MIT](https://github.com/shurcooL/githubv4/blob/48295856cce7/LICENSE)) - [github.com/shurcooL/graphql](https://pkg.go.dev/github.com/shurcooL/graphql) ([MIT](https://github.com/shurcooL/graphql/blob/ed46e5a46466/LICENSE)) - - [github.com/sirupsen/logrus](https://pkg.go.dev/github.com/sirupsen/logrus) ([MIT](https://github.com/sirupsen/logrus/blob/v1.9.3/LICENSE)) - [github.com/sourcegraph/conc](https://pkg.go.dev/github.com/sourcegraph/conc) ([MIT](https://github.com/sourcegraph/conc/blob/v0.3.0/LICENSE)) - [github.com/spf13/afero](https://pkg.go.dev/github.com/spf13/afero) ([Apache-2.0](https://github.com/spf13/afero/blob/v1.14.0/LICENSE.txt)) - [github.com/spf13/cast](https://pkg.go.dev/github.com/spf13/cast) ([MIT](https://github.com/spf13/cast/blob/v1.7.1/LICENSE)) diff --git a/third-party/github.com/google/go-github/v73/github/LICENSE b/third-party/github.com/google/go-github/v74/github/LICENSE similarity index 100% rename from third-party/github.com/google/go-github/v73/github/LICENSE rename to third-party/github.com/google/go-github/v74/github/LICENSE diff --git a/third-party/github.com/sirupsen/logrus/LICENSE b/third-party/github.com/sirupsen/logrus/LICENSE deleted file mode 100644 index f090cb42f..000000000 --- a/third-party/github.com/sirupsen/logrus/LICENSE +++ /dev/null @@ -1,21 +0,0 @@ -The MIT License (MIT) - -Copyright (c) 2014 Simon Eskildsen - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in -all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -THE SOFTWARE.