diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 35ffc47db..cd2d923cb 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,11 +9,11 @@ on: schedule: - cron: "27 0 * * *" push: - branches: ["main"] + branches: ["main", "next"] # Publish semver tags as releases. tags: ["v*.*.*"] pull_request: - branches: ["main"] + branches: ["main", "next"] env: # Use docker.io for Docker Hub if empty diff --git a/.github/workflows/pr-base-check.yml b/.github/workflows/pr-base-check.yml new file mode 100644 index 000000000..6f48205ca --- /dev/null +++ b/.github/workflows/pr-base-check.yml @@ -0,0 +1,55 @@ +name: PR Base Branch Check + +on: + pull_request: + types: [opened, edited, synchronize] + branches: + - main + +permissions: + pull-requests: write + contents: read + +jobs: + check-base-branch: + runs-on: ubuntu-latest + if: github.event.pull_request.base.ref == 'main' + + steps: + - name: Comment on PR + uses: actions/github-script@v7 + with: + script: | + const message = `👋 Hi there! + + It looks like this PR is targeting the \`main\` branch. To help maintain our development workflow, please change the base reference to \`next\` instead. + + __If this is a bug fix that requires a patch release __ (e.g., a critical bug that needs to be fixed before the next release)__, please leave the base branch as \`main\`.__ + + You can change this by: + 1. Clicking the "Edit" button next to the PR title + 2. Changing the base branch from \`main\` to \`next\` + 3. Clicking "Update pull request" + + Thanks for your contribution! 🚀`; + + // Check if we've already commented + const comments = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const botComment = comments.data.find(comment => + comment.user.type === 'Bot' && + comment.body.includes('please change the base reference to') + ); + + if (!botComment) { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: message + }); + } diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 000000000..048e17aed --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,177 @@ +name: Release + +on: + workflow_dispatch: + inputs: + tag: + description: 'Release tag (e.g., v0.0.0)' + required: true + default: 'v0.0.0' + type: string + confirm: + description: 'Type "CONFIRM" to proceed with the release' + required: true + type: string + +permissions: + contents: write + pull-requests: write + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - name: Validate confirmation + if: ${{ github.event.inputs.confirm != 'CONFIRM' }} + run: | + echo "::error::You must type 'CONFIRM' to proceed with the release" + exit 1 + + - name: Validate tag format + run: | + TAG="${{ github.event.inputs.tag }}" + if [[ ! $TAG =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.*)?$ ]]; then + echo "::error::Tag must be in format vX.Y.Z or vX.Y.Z-suffix (e.g., v1.0.0 or v1.0.0-rc1)" + exit 1 + fi + + release: + needs: validate + runs-on: ubuntu-latest + outputs: + tag: ${{ steps.tag-release.outputs.tag }} + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Configure Git + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + - name: Switch to next branch + run: | + git checkout next + git pull origin next + + - name: Check next branch is up-to-date with main + id: branch-check + run: | + echo "Checking if next branch is up-to-date with main..." + + # Fetch latest main branch + git fetch origin main + + # Check if next is behind main + BEHIND_COUNT=$(git rev-list --count next..origin/main) + AHEAD_COUNT=$(git rev-list --count origin/main..next) + + echo "Next branch is ${AHEAD_COUNT} commits ahead of main" + echo "Next branch is ${BEHIND_COUNT} commits behind main" + + if [ "$BEHIND_COUNT" -gt 0 ]; then + echo "::error::❌ Next branch is ${BEHIND_COUNT} commits behind main. Please update next branch with the latest changes from main before creating a release." + echo "To fix this, run: git checkout next && git merge main" + exit 1 + fi + + if [ "$AHEAD_COUNT" -eq 0 ]; then + echo "::warning::⚠️ Next branch has no new commits compared to main. Are you sure you want to create a release?" + fi + + echo "✅ Next branch is up-to-date with main (${AHEAD_COUNT} commits ahead)" + echo "branch-check-success=true" >> $GITHUB_OUTPUT + + - name: Check if tag already exists + run: | + TAG="${{ github.event.inputs.tag }}" + if git tag -l | grep -q "^${TAG}$"; then + echo "::error::Tag ${TAG} already exists" + exit 1 + fi + if git ls-remote --tags origin | grep -q "refs/tags/${TAG}$"; then + echo "::error::Tag ${TAG} already exists on remote" + exit 1 + fi + + - name: Tag the release + id: tag-release + run: | + TAG="${{ github.event.inputs.tag }}" + git tag -a "${TAG}" -m "Release ${TAG}" + echo "✅ Created tag ${TAG}" + echo "tag=${TAG}" >> $GITHUB_OUTPUT + + - name: Push tag + run: | + TAG="${{ github.event.inputs.tag }}" + git push origin "${TAG}" + echo "✅ Pushed tag ${TAG}" + + - name: Wait for release to be created + run: | + TAG="${{ github.event.inputs.tag }}" + echo "Waiting for GitHub to create the draft release..." + + # Wait up to 2 minutes for the release to appear + for i in {1..24}; do + if gh release view "${TAG}" >/dev/null 2>&1; then + echo "✅ Draft release created" + break + fi + echo "Waiting for release to be created... (${i}/24)" + sleep 5 + done + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + summary: + needs: [validate, release] + runs-on: ubuntu-latest + if: always() && needs.release.result == 'success' + + steps: + - name: Release Summary + run: | + TAG="${{ needs.release.outputs.tag }}" + + echo "## 🎉 Release $TAG has been initiated!" + echo "" + echo "### Next steps:" + echo "1. 📋 Check https://github.com/${{ github.repository }}/releases for the draft release to show up" + echo "2. ✏️ Edit the new release, delete the existing notes and click the auto-generate button GitHub provides" + echo "3. ✨ Add a section at the top calling out the main features" + echo "4. 🚀 Publish the release" + echo "5. � Create a Pull Request from 'next' to 'main' branch with title 'Release $TAG'" + echo "6. �🔀 Merge the pull request into main" + echo "7. 📢 Post message in #gh-mcp-releases channel in Slack and then share to the other mcp channels" + echo "" + echo "### Resources:" + echo "- 📦 Draft Release: https://github.com/${{ github.repository }}/releases/tag/$TAG" + echo "- 🔄 Create PR: https://github.com/${{ github.repository }}/compare/main...next" + echo "" + echo "The release process is now ready for your review and completion!" + + # Also output as job summary + cat << EOF >> $GITHUB_STEP_SUMMARY + ## 🎉 Release $TAG has been initiated! + + ### Next steps: + 1. 📋 Check [releases page](https://github.com/${{ github.repository }}/releases) for the draft release to show up + 2. ✏️ Edit the new release, delete the existing notes and click the auto-generate button GitHub provides + 3. ✨ Add a section at the top calling out the main features + 4. 🚀 Publish the release + 5. � [Create a Pull Request](https://github.com/${{ github.repository }}/compare/main...next) from 'next' to 'main' branch with title 'Release $TAG' + 6. �🔀 Merge the pull request into main + 7. 📢 Post message in #gh-mcp-releases channel in Slack and then share to the other mcp channels + + ### Resources: + - 📦 [Draft Release](https://github.com/${{ github.repository }}/releases/tag/$TAG) + - 🔄 [Create PR](https://github.com/${{ github.repository }}/compare/main...next) + + The release process is now ready for your review and completion! + EOF diff --git a/.github/workflows/sync-next-branch.yml b/.github/workflows/sync-next-branch.yml new file mode 100644 index 000000000..7efe8cab2 --- /dev/null +++ b/.github/workflows/sync-next-branch.yml @@ -0,0 +1,155 @@ +name: Sync Next Branch + +on: + schedule: + # Run daily at 9:00 AM UTC (6:00 AM EST, 3:00 AM PST) + - cron: '0 9 * * *' + workflow_dispatch: + # Allow manual triggering + +permissions: + contents: write + pull-requests: write + +jobs: + check-and-sync: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Configure Git + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + + - name: Check branch status + id: branch-status + run: | + echo "Checking if next branch is up-to-date with main..." + + # Fetch latest branches + git fetch origin main + git fetch origin next + + # Check if next is behind main + BEHIND_COUNT=$(git rev-list --count origin/next..origin/main) + AHEAD_COUNT=$(git rev-list --count origin/main..origin/next) + + echo "Next branch is ${AHEAD_COUNT} commits ahead of main" + echo "Next branch is ${BEHIND_COUNT} commits behind main" + + echo "behind-count=${BEHIND_COUNT}" >> $GITHUB_OUTPUT + echo "ahead-count=${AHEAD_COUNT}" >> $GITHUB_OUTPUT + + if [ "$BEHIND_COUNT" -gt 0 ]; then + echo "needs-sync=true" >> $GITHUB_OUTPUT + echo "🔄 Next branch needs to be synced (${BEHIND_COUNT} commits behind)" + else + echo "needs-sync=false" >> $GITHUB_OUTPUT + echo "✅ Next branch is up-to-date with main" + fi + + - name: Check for existing sync PR + id: existing-pr + if: steps.branch-status.outputs.needs-sync == 'true' + run: | + # Check if there's already an open PR from main to next for syncing + EXISTING_PR=$(gh pr list \ + --base next \ + --head main \ + --state open \ + --json number,title \ + --jq '.[] | select(.title | test("^(Sync|Update) next branch")) | .number') + + if [ -n "$EXISTING_PR" ]; then + echo "existing-pr=${EXISTING_PR}" >> $GITHUB_OUTPUT + echo "⚠️ Sync PR already exists: #${EXISTING_PR}" + echo "has-existing-pr=true" >> $GITHUB_OUTPUT + else + echo "has-existing-pr=false" >> $GITHUB_OUTPUT + echo "No existing sync PR found" + fi + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Provide sync instructions + id: sync-instructions + if: steps.branch-status.outputs.needs-sync == 'true' && steps.existing-pr.outputs.has-existing-pr == 'false' + run: | + BEHIND_COUNT="${{ steps.branch-status.outputs.behind-count }}" + AHEAD_COUNT="${{ steps.branch-status.outputs.ahead-count }}" + + echo "action-needed=true" >> $GITHUB_OUTPUT + echo "🔄 Next branch needs syncing (${BEHIND_COUNT} commits behind main)" + echo "� Manual PR creation required due to organization policies" + + - name: Job Summary + if: always() + run: | + BEHIND_COUNT="${{ steps.branch-status.outputs.behind-count }}" + AHEAD_COUNT="${{ steps.branch-status.outputs.ahead-count }}" + NEEDS_SYNC="${{ steps.branch-status.outputs.needs-sync }}" + HAS_EXISTING_PR="${{ steps.existing-pr.outputs.has-existing-pr }}" + EXISTING_PR="${{ steps.existing-pr.outputs.existing-pr }}" + ACTION_NEEDED="${{ steps.sync-instructions.outputs.action-needed }}" + + cat << EOF >> $GITHUB_STEP_SUMMARY + # 🔄 Branch Sync Status + + ## Current Status: + - **Next branch**: ${AHEAD_COUNT} commits ahead, ${BEHIND_COUNT} commits behind main + - **Needs sync**: ${NEEDS_SYNC} + + EOF + + if [ "$NEEDS_SYNC" = "true" ]; then + if [ "$HAS_EXISTING_PR" = "true" ]; then + cat << EOF >> $GITHUB_STEP_SUMMARY + ## ⚠️ Action Required: + There is already an existing sync PR: [#${EXISTING_PR}](https://github.com/${{ github.repository }}/pull/${EXISTING_PR}) + + Please review and merge the existing PR to sync the next branch. + EOF + elif [ "$ACTION_NEEDED" = "true" ]; then + cat << EOF >> $GITHUB_STEP_SUMMARY + ## 📝 Manual Action Required: + + The \`next\` branch is ${BEHIND_COUNT} commits behind \`main\` and needs to be synced. + + **Please create a pull request manually:** + + 1. 🌐 [Create PR: main → next](https://github.com/${{ github.repository }}/compare/next...main) + 2. 📝 Use title: **"Sync next branch with main"** + 3. 📄 Use this description: + + \`\`\`markdown + ## 🔄 Branch Sync + + This PR syncs the \`next\` branch with the latest changes from \`main\`. + + ### Status: + - **Behind main**: ${BEHIND_COUNT} commits + - **Ahead of main**: ${AHEAD_COUNT} commits + + ### What to do: + 1. 🔍 Review the changes in this PR + 2. ✅ Ensure all checks pass + 3. 🔀 Merge this PR to sync the \`next\` branch + 4. 🗑️ The \`next\` branch will then be ready for new development + \`\`\` + + 4. 🏷️ Add labels: \`automated\`, \`sync\` + 5. ✅ Review and merge when ready + EOF + fi + else + cat << EOF >> $GITHUB_STEP_SUMMARY + ## ✅ All Good! + The next branch is up-to-date with main. No action needed. + EOF + fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 11d63a389..6fa9c2ebe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,12 +19,14 @@ These are one time installations required to be able to test your changes locall ## Submitting a pull request +> **Important**: Please open your pull request against the `next` branch, not `main`. The `next` branch is where we integrate new features and changes before they are merged to `main`. + 1. [Fork][fork] and clone the repository 1. Make sure the tests pass on your machine: `go test -v ./...` 1. Make sure linter passes on your machine: `golangci-lint run` 1. Create a new branch: `git checkout -b my-branch-name` 1. Make your change, add tests, and make sure the tests and linter still pass -1. Push to your fork and [submit a pull request][pr] +1. Push to your fork and [submit a pull request][pr] targeting the `next` branch 1. Pat yourself on the back and wait for your pull request to be reviewed and merged. Here are a few things you can do that will increase the likelihood of your pull request being accepted: diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index fb716f78d..b39a8b7df 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -4,10 +4,12 @@ import ( "errors" "fmt" "os" + "strings" "github.com/github/github-mcp-server/internal/ghmcp" "github.com/github/github-mcp-server/pkg/github" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -54,7 +56,6 @@ var ( EnableCommandLogging: viper.GetBool("enable-command-logging"), LogFilePath: viper.GetString("log-file"), } - return ghmcp.RunStdioServer(stdioServerConfig) }, } @@ -62,6 +63,7 @@ var ( func init() { cobra.OnInitialize(initConfig) + rootCmd.SetGlobalNormalizationFunc(wordSepNormalizeFunc) rootCmd.SetVersionTemplate("{{.Short}}\n{{.Version}}\n") @@ -91,6 +93,7 @@ func initConfig() { // Initialize Viper configuration viper.SetEnvPrefix("github") viper.AutomaticEnv() + } func main() { @@ -99,3 +102,12 @@ func main() { os.Exit(1) } } + +func wordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName { + from := []string{"_"} + to := "-" + for _, sep := range from { + name = strings.ReplaceAll(name, sep, to) + } + return pflag.NormalizedName(name) +} diff --git a/go.mod b/go.mod index 9cee56b5c..4cc7682fd 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.14.0 // indirect github.com/spf13/cast v1.7.1 // indirect - github.com/spf13/pflag v1.0.6 // indirect + github.com/spf13/pflag v1.0.6 github.com/subosito/gotenv v1.6.0 // indirect github.com/yosida95/uritemplate/v3 v3.0.2 // indirect go.uber.org/multierr v1.11.0 // indirect diff --git a/pkg/github/__toolsnaps__/get_pull_request_files.snap b/pkg/github/__toolsnaps__/get_pull_request_files.snap index c61f5f357..148053b17 100644 --- a/pkg/github/__toolsnaps__/get_pull_request_files.snap +++ b/pkg/github/__toolsnaps__/get_pull_request_files.snap @@ -10,6 +10,17 @@ "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, + "minimum": 1, + "type": "number" + }, "pullNumber": { "description": "Pull request number", "type": "number" diff --git a/pkg/github/__toolsnaps__/search_issues.snap b/pkg/github/__toolsnaps__/search_issues.snap index 4e2382a3c..7db502d94 100644 --- a/pkg/github/__toolsnaps__/search_issues.snap +++ b/pkg/github/__toolsnaps__/search_issues.snap @@ -3,7 +3,7 @@ "title": "Search issues", "readOnlyHint": true }, - "description": "Search for issues in GitHub repositories.", + "description": "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue", "inputSchema": { "properties": { "order": { @@ -14,6 +14,10 @@ ], "type": "string" }, + "owner": { + "description": "Optional repository owner. If provided with repo, only notifications for this repository are listed.", + "type": "string" + }, "page": { "description": "Page number for pagination (min 1)", "minimum": 1, @@ -25,10 +29,14 @@ "minimum": 1, "type": "number" }, - "q": { + "query": { "description": "Search query using GitHub issues search syntax", "type": "string" }, + "repo": { + "description": "Optional repository name. If provided with owner, only notifications for this repository are listed.", + "type": "string" + }, "sort": { "description": "Sort field by number of matches of categories, defaults to best match", "enum": [ @@ -48,7 +56,7 @@ } }, "required": [ - "q" + "query" ], "type": "object" }, diff --git a/pkg/github/__toolsnaps__/search_pull_requests.snap b/pkg/github/__toolsnaps__/search_pull_requests.snap new file mode 100644 index 000000000..6a8d8e0e6 --- /dev/null +++ b/pkg/github/__toolsnaps__/search_pull_requests.snap @@ -0,0 +1,64 @@ +{ + "annotations": { + "title": "Search pull requests", + "readOnlyHint": true + }, + "description": "Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr", + "inputSchema": { + "properties": { + "order": { + "description": "Sort order", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "owner": { + "description": "Optional repository owner. If provided with repo, only notifications for this repository are listed.", + "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, + "minimum": 1, + "type": "number" + }, + "query": { + "description": "Search query using GitHub pull request search syntax", + "type": "string" + }, + "repo": { + "description": "Optional repository name. If provided with owner, only notifications for this repository are listed.", + "type": "string" + }, + "sort": { + "description": "Sort field by number of matches of categories, defaults to best match", + "enum": [ + "comments", + "reactions", + "reactions-+1", + "reactions--1", + "reactions-smile", + "reactions-thinking_face", + "reactions-heart", + "reactions-tada", + "interactions", + "created", + "updated" + ], + "type": "string" + } + }, + "required": [ + "query" + ], + "type": "object" + }, + "name": "search_pull_requests" +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/search_users.snap b/pkg/github/__toolsnaps__/search_users.snap index aad2970b6..5cf9796f2 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", + "description": "Search for GitHub users exclusively", "inputSchema": { "properties": { "order": { @@ -25,8 +25,8 @@ "minimum": 1, "type": "number" }, - "q": { - "description": "Search query using GitHub users search syntax", + "query": { + "description": "Search query using GitHub users search syntax scoped to type:user", "type": "string" }, "sort": { @@ -40,7 +40,7 @@ } }, "required": [ - "q" + "query" ], "type": "object" }, diff --git a/pkg/github/issues.go b/pkg/github/issues.go index b4c64c8de..3242c2be9 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -153,18 +153,24 @@ func AddIssueComment(getClient GetClientFn, t translations.TranslationHelperFunc } } -// SearchIssues creates a tool to search for issues and pull requests. +// SearchIssues creates a tool to search for issues. func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("search_issues", - mcp.WithDescription(t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues in GitHub repositories.")), + mcp.WithDescription(t("TOOL_SEARCH_ISSUES_DESCRIPTION", "Search for issues in GitHub repositories using issues search syntax already scoped to is:issue")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_SEARCH_ISSUES_USER_TITLE", "Search issues"), ReadOnlyHint: ToBoolPtr(true), }), - mcp.WithString("q", + mcp.WithString("query", mcp.Required(), 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.WithString("repo", + mcp.Description("Optional repository name. If provided with owner, only notifications for this repository are listed."), + ), mcp.WithString("sort", mcp.Description("Sort field by number of matches of categories, defaults to best match"), mcp.Enum( @@ -188,56 +194,7 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) ( WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - query, err := RequiredParam[string](request, "q") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - sort, err := OptionalParam[string](request, "sort") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - order, err := OptionalParam[string](request, "order") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pagination, err := OptionalPaginationParams(request) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - - opts := &github.SearchOptions{ - Sort: sort, - Order: order, - ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, - }, - } - - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } - result, resp, err := client.Search.Issues(ctx, query, opts) - if err != nil { - return nil, fmt.Errorf("failed to search issues: %w", err) - } - 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 search issues: %s", string(body))), nil - } - - r, err := json.Marshal(result) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil + return searchHandler(ctx, getClient, request, "issue", "failed to search issues") } } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7c76d90f9..056fa7ed8 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -237,12 +237,14 @@ func Test_SearchIssues(t *testing.T) { assert.Equal(t, "search_issues", tool.Name) assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "q") + assert.Contains(t, tool.InputSchema.Properties, "query") + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.Contains(t, tool.InputSchema.Properties, "page") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"q"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results mockSearchResult := &github.IssuesSearchResult{ @@ -290,7 +292,7 @@ func Test_SearchIssues(t *testing.T) { expectQueryParams( t, map[string]string{ - "q": "repo:owner/repo is:issue is:open", + "q": "is:issue repo:owner/repo is:open", "sort": "created", "order": "desc", "page": "1", @@ -302,7 +304,7 @@ func Test_SearchIssues(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "repo:owner/repo is:issue is:open", + "query": "repo:owner/repo is:open", "sort": "created", "order": "desc", "page": float64(1), @@ -311,6 +313,83 @@ func Test_SearchIssues(t *testing.T) { expectError: false, expectedResult: mockSearchResult, }, + { + name: "issues search with owner and repo parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:test-owner/test-repo is:issue is:open", + "sort": "created", + "order": "asc", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:open", + "owner": "test-owner", + "repo": "test-repo", + "sort": "created", + "order": "asc", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "issues search with only owner parameter (should ignore it)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue bug", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "bug", + "owner": "test-owner", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "issues search with only repo parameter (should ignore it)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:issue feature", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "feature", + "repo": "test-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, { name: "issues search with minimal parameters", mockedClient: mock.NewMockedHTTPClient( @@ -320,7 +399,7 @@ func Test_SearchIssues(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "repo:owner/repo is:issue is:open", + "query": "is:issue repo:owner/repo is:open", }, expectError: false, expectedResult: mockSearchResult, @@ -337,7 +416,7 @@ func Test_SearchIssues(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "invalid:query", + "query": "invalid:query", }, expectError: true, expectedErrMsg: "failed to search issues", diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 7dcc2c4fd..bad822b13 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -533,6 +533,51 @@ func MergePullRequest(getClient GetClientFn, t translations.TranslationHelperFun } } +// SearchPullRequests creates a tool to search for pull requests. +func SearchPullRequests(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("search_pull_requests", + mcp.WithDescription(t("TOOL_SEARCH_PULL_REQUESTS_DESCRIPTION", "Search for pull requests in GitHub repositories using issues search syntax already scoped to is:pr")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_SEARCH_PULL_REQUESTS_USER_TITLE", "Search pull requests"), + ReadOnlyHint: ToBoolPtr(true), + }), + mcp.WithString("query", + mcp.Required(), + 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.WithString("repo", + mcp.Description("Optional repository name. If provided with owner, only notifications for this repository are listed."), + ), + mcp.WithString("sort", + mcp.Description("Sort field by number of matches of categories, defaults to best match"), + mcp.Enum( + "comments", + "reactions", + "reactions-+1", + "reactions--1", + "reactions-smile", + "reactions-thinking_face", + "reactions-heart", + "reactions-tada", + "interactions", + "created", + "updated", + ), + ), + mcp.WithString("order", + mcp.Description("Sort order"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + return searchHandler(ctx, getClient, request, "pr", "failed to search pull requests") + } +} + // GetPullRequestFiles creates a tool to get the list of files changed in a pull request. func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("get_pull_request_files", @@ -553,6 +598,7 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper mcp.Required(), mcp.Description("Pull request number"), ), + WithPagination(), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := RequiredParam[string](request, "owner") @@ -567,12 +613,19 @@ func GetPullRequestFiles(getClient GetClientFn, t translations.TranslationHelper if err != nil { return mcp.NewToolResultError(err.Error()), nil } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } client, err := getClient(ctx) if err != nil { return nil, fmt.Errorf("failed to get GitHub client: %w", err) } - opts := &github.ListOptions{} + opts := &github.ListOptions{ + PerPage: pagination.perPage, + Page: pagination.page, + } files, resp, err := client.PullRequests.ListFiles(ctx, owner, repo, pullNumber, opts) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 02575c439..30341e86c 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -565,6 +565,241 @@ func Test_MergePullRequest(t *testing.T) { } } +func Test_SearchPullRequests(t *testing.T) { + mockClient := github.NewClient(nil) + tool, _ := SearchPullRequests(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "search_pull_requests", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "query") + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "sort") + assert.Contains(t, tool.InputSchema.Properties, "order") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) + + mockSearchResult := &github.IssuesSearchResult{ + Total: github.Ptr(2), + IncompleteResults: github.Ptr(false), + Issues: []*github.Issue{ + { + Number: github.Ptr(42), + Title: github.Ptr("Test PR 1"), + Body: github.Ptr("Updated tests."), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"), + Comments: github.Ptr(5), + User: &github.User{ + Login: github.Ptr("user1"), + }, + }, + { + Number: github.Ptr(43), + Title: github.Ptr("Test PR 2"), + Body: github.Ptr("Updated build scripts."), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/pull/2"), + Comments: github.Ptr(3), + User: &github.User{ + Login: github.Ptr("user2"), + }, + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.IssuesSearchResult + expectedErrMsg string + }{ + { + name: "successful pull request search with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr repo:owner/repo is:open", + "sort": "created", + "order": "desc", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "repo:owner/repo is:open", + "sort": "created", + "order": "desc", + "page": float64(1), + "perPage": float64(30), + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "pull request search with owner and repo parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "repo:test-owner/test-repo is:pr draft:false", + "sort": "updated", + "order": "asc", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "draft:false", + "owner": "test-owner", + "repo": "test-repo", + "sort": "updated", + "order": "asc", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "pull request search with only owner parameter (should ignore it)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr feature", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "feature", + "owner": "test-owner", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "pull request search with only repo parameter (should ignore it)", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + expectQueryParams( + t, + map[string]string{ + "q": "is:pr review-required", + "page": "1", + "per_page": "30", + }, + ).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "review-required", + "repo": "test-repo", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "pull request search with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetSearchIssues, + mockSearchResult, + ), + ), + requestArgs: map[string]interface{}{ + "query": "is:pr repo:owner/repo is:open", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "search pull requests fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchIssues, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message": "Validation Failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "query": "invalid:query", + }, + expectError: true, + expectedErrMsg: "failed to search pull requests", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := SearchPullRequests(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedResult github.IssuesSearchResult + err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + require.NoError(t, err) + assert.Equal(t, *tc.expectedResult.Total, *returnedResult.Total) + assert.Equal(t, *tc.expectedResult.IncompleteResults, *returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Issues, len(tc.expectedResult.Issues)) + for i, issue := range returnedResult.Issues { + assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number) + assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title) + assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State) + assert.Equal(t, *tc.expectedResult.Issues[i].HTMLURL, *issue.HTMLURL) + assert.Equal(t, *tc.expectedResult.Issues[i].User.Login, *issue.User.Login) + } + }) + } + +} + func Test_GetPullRequestFiles(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) @@ -576,6 +811,8 @@ func Test_GetPullRequestFiles(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, "page") + assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "pullNumber"}) // Setup mock PR files for success case @@ -622,6 +859,24 @@ func Test_GetPullRequestFiles(t *testing.T) { expectError: false, expectedFiles: mockFiles, }, + { + name: "successful files fetch with pagination", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposPullsFilesByOwnerByRepoByPullNumber, + mockFiles, + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + "page": float64(2), + "perPage": float64(10), + }, + expectError: false, + expectedFiles: mockFiles, + }, { name: "files fetch fails", mockedClient: mock.NewMockedHTTPClient( diff --git a/pkg/github/search.go b/pkg/github/search.go index 13d017129..5106b84d8 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -168,100 +168,139 @@ type MinimalSearchUsersResult struct { Items []MinimalUser `json:"items"` } -// SearchUsers creates a tool to search for GitHub users. -func SearchUsers(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { - return mcp.NewTool("search_users", - mcp.WithDescription(t("TOOL_SEARCH_USERS_DESCRIPTION", "Search for GitHub users")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{ - Title: t("TOOL_SEARCH_USERS_USER_TITLE", "Search users"), - ReadOnlyHint: ToBoolPtr(true), - }), - mcp.WithString("q", - mcp.Required(), - mcp.Description("Search query using GitHub users search syntax"), - ), - mcp.WithString("sort", - mcp.Description("Sort field by category"), - mcp.Enum("followers", "repositories", "joined"), - ), - mcp.WithString("order", - mcp.Description("Sort order"), - mcp.Enum("asc", "desc"), - ), - WithPagination(), - ), - func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - query, err := RequiredParam[string](request, "q") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - sort, err := OptionalParam[string](request, "sort") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - order, err := OptionalParam[string](request, "order") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - pagination, err := OptionalPaginationParams(request) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } +func userOrOrgHandler(accountType string, getClient GetClientFn) server.ToolHandlerFunc { + return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + query, err := RequiredParam[string](request, "query") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + sort, err := OptionalParam[string](request, "sort") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + order, err := OptionalParam[string](request, "order") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } - opts := &github.SearchOptions{ - Sort: sort, - Order: order, - ListOptions: github.ListOptions{ - PerPage: pagination.perPage, - Page: pagination.page, - }, - } + opts := &github.SearchOptions{ + Sort: sort, + Order: order, + ListOptions: github.ListOptions{ + PerPage: pagination.perPage, + Page: pagination.page, + }, + } - client, err := getClient(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub client: %w", err) - } + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } - result, resp, err := client.Search.Users(ctx, "type:user "+query, opts) + searchQuery := "type:" + accountType + " " + query + result, resp, err := client.Search.Users(ctx, searchQuery, opts) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + fmt.Sprintf("failed to search %ss with query '%s'", accountType, query), + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + body, err := io.ReadAll(resp.Body) if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - fmt.Sprintf("failed to search users with query '%s'", query), - resp, - err, - ), nil + return nil, fmt.Errorf("failed to read response body: %w", err) } - defer func() { _ = resp.Body.Close() }() + return mcp.NewToolResultError(fmt.Sprintf("failed to search %ss: %s", accountType, string(body))), nil + } - if resp.StatusCode != 200 { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to search users: %s", string(body))), nil - } + minimalUsers := make([]MinimalUser, 0, len(result.Users)) - minimalUsers := make([]MinimalUser, 0, len(result.Users)) - for _, user := range result.Users { - mu := MinimalUser{ - Login: user.GetLogin(), - ID: user.GetID(), - ProfileURL: user.GetHTMLURL(), - AvatarURL: user.GetAvatarURL(), + for _, user := range result.Users { + if user.Login != nil { + mu := MinimalUser{Login: *user.Login} + if user.ID != nil { + mu.ID = *user.ID + } + if user.HTMLURL != nil { + mu.ProfileURL = *user.HTMLURL + } + if user.AvatarURL != nil { + mu.AvatarURL = *user.AvatarURL } - minimalUsers = append(minimalUsers, mu) } + } + minimalResp := &MinimalSearchUsersResult{ + TotalCount: result.GetTotal(), + IncompleteResults: result.GetIncompleteResults(), + Items: minimalUsers, + } + if result.Total != nil { + minimalResp.TotalCount = *result.Total + } + if result.IncompleteResults != nil { + minimalResp.IncompleteResults = *result.IncompleteResults + } - minimalResp := MinimalSearchUsersResult{ - TotalCount: result.GetTotal(), - IncompleteResults: result.GetIncompleteResults(), - Items: minimalUsers, - } - - r, err := json.Marshal(minimalResp) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - return mcp.NewToolResultText(string(r)), nil + r, err := json.Marshal(minimalResp) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) } + return mcp.NewToolResultText(string(r)), nil + } +} + +// 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.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.WithString("sort", + mcp.Description("Sort field by category"), + mcp.Enum("followers", "repositories", "joined"), + ), + mcp.WithString("order", + mcp.Description("Sort order"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), userOrOrgHandler("user", getClient) +} + +// 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.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.WithString("sort", + mcp.Description("Sort field by category"), + mcp.Enum("followers", "repositories", "joined"), + ), + mcp.WithString("order", + mcp.Description("Sort order"), + mcp.Enum("asc", "desc"), + ), + WithPagination(), + ), userOrOrgHandler("org", getClient) } diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index f206ebb44..bfd014993 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -328,12 +328,12 @@ func Test_SearchUsers(t *testing.T) { assert.Equal(t, "search_users", tool.Name) assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "q") + assert.Contains(t, tool.InputSchema.Properties, "query") assert.Contains(t, tool.InputSchema.Properties, "sort") assert.Contains(t, tool.InputSchema.Properties, "order") assert.Contains(t, tool.InputSchema.Properties, "perPage") assert.Contains(t, tool.InputSchema.Properties, "page") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"q"}) + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) // Setup mock search results mockSearchResult := &github.UsersSearchResult{ @@ -381,7 +381,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "location:finland language:go", + "query": "location:finland language:go", "sort": "followers", "order": "desc", "page": float64(1), @@ -405,7 +405,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "location:finland language:go", + "query": "location:finland language:go", }, expectError: false, expectedResult: mockSearchResult, @@ -422,7 +422,7 @@ func Test_SearchUsers(t *testing.T) { ), ), requestArgs: map[string]interface{}{ - "q": "invalid:query", + "query": "invalid:query", }, expectError: true, expectedErrMsg: "failed to search users", @@ -474,3 +474,127 @@ func Test_SearchUsers(t *testing.T) { }) } } + +func Test_SearchOrgs(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := SearchOrgs(stubGetClientFn(mockClient), translations.NullTranslationHelper) + + assert.Equal(t, "search_orgs", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "query") + assert.Contains(t, tool.InputSchema.Properties, "sort") + assert.Contains(t, tool.InputSchema.Properties, "order") + assert.Contains(t, tool.InputSchema.Properties, "perPage") + assert.Contains(t, tool.InputSchema.Properties, "page") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"query"}) + + // Setup mock search results + mockSearchResult := &github.UsersSearchResult{ + Total: github.Ptr(int(2)), + IncompleteResults: github.Ptr(false), + Users: []*github.User{ + { + Login: github.Ptr("org-1"), + ID: github.Ptr(int64(111)), + HTMLURL: github.Ptr("https://github.com/org-1"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/111?v=4"), + }, + { + Login: github.Ptr("org-2"), + ID: github.Ptr(int64(222)), + HTMLURL: github.Ptr("https://github.com/org-2"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/u/222?v=4"), + }, + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedResult *github.UsersSearchResult + expectedErrMsg string + }{ + { + name: "successful org search", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + expectQueryParams(t, map[string]string{ + "q": "type:org github", + "page": "1", + "per_page": "30", + }).andThen( + mockResponse(t, http.StatusOK, mockSearchResult), + ), + ), + ), + requestArgs: map[string]interface{}{ + "query": "github", + }, + expectError: false, + expectedResult: mockSearchResult, + }, + { + name: "org search fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetSearchUsers, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"message": "Validation Failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "query": "invalid:query", + }, + expectError: true, + expectedErrMsg: "failed to search orgs", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := SearchOrgs(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + require.NotNil(t, result) + + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedResult MinimalSearchUsersResult + err = json.Unmarshal([]byte(textContent.Text), &returnedResult) + require.NoError(t, err) + assert.Equal(t, *tc.expectedResult.Total, returnedResult.TotalCount) + assert.Equal(t, *tc.expectedResult.IncompleteResults, returnedResult.IncompleteResults) + assert.Len(t, returnedResult.Items, len(tc.expectedResult.Users)) + for i, org := range returnedResult.Items { + assert.Equal(t, *tc.expectedResult.Users[i].Login, org.Login) + assert.Equal(t, *tc.expectedResult.Users[i].ID, org.ID) + assert.Equal(t, *tc.expectedResult.Users[i].HTMLURL, org.ProfileURL) + assert.Equal(t, *tc.expectedResult.Users[i].AvatarURL, org.AvatarURL) + } + }) + } +} diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go new file mode 100644 index 000000000..6642dad8f --- /dev/null +++ b/pkg/github/search_utils.go @@ -0,0 +1,88 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + + "github.com/google/go-github/v72/github" + "github.com/mark3labs/mcp-go/mcp" +) + +func searchHandler( + ctx context.Context, + getClient GetClientFn, + request mcp.CallToolRequest, + searchType string, + errorPrefix string, +) (*mcp.CallToolResult, error) { + query, err := RequiredParam[string](request, "query") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + query = fmt.Sprintf("is:%s %s", searchType, query) + + owner, err := OptionalParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + repo, err := OptionalParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + if owner != "" && repo != "" { + query = fmt.Sprintf("repo:%s/%s %s", owner, repo, query) + } + + sort, err := OptionalParam[string](request, "sort") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + order, err := OptionalParam[string](request, "order") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + pagination, err := OptionalPaginationParams(request) + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + opts := &github.SearchOptions{ + // Default to "created" if no sort is provided, as it's a common use case. + Sort: sort, + Order: order, + ListOptions: github.ListOptions{ + Page: pagination.page, + PerPage: pagination.perPage, + }, + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("%s: failed to get GitHub client: %w", errorPrefix, err) + } + result, resp, err := client.Search.Issues(ctx, query, opts) + if err != nil { + return nil, fmt.Errorf("%s: %w", errorPrefix, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%s: failed to read response body: %w", errorPrefix, err) + } + return mcp.NewToolResultError(fmt.Sprintf("%s: %s", errorPrefix, string(body))), nil + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("%s: failed to marshal response: %w", errorPrefix, err) + } + + return mcp.NewToolResultText(string(r)), nil +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 5b970698c..76b31d477 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -64,11 +64,16 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG AddReadTools( toolsets.NewServerTool(SearchUsers(getClient, t)), ) + orgs := toolsets.NewToolset("orgs", "GitHub Organization related tools"). + AddReadTools( + toolsets.NewServerTool(SearchOrgs(getClient, t)), + ) pullRequests := toolsets.NewToolset("pull_requests", "GitHub Pull Request related tools"). AddReadTools( toolsets.NewServerTool(GetPullRequest(getClient, t)), toolsets.NewServerTool(ListPullRequests(getClient, t)), toolsets.NewServerTool(GetPullRequestFiles(getClient, t)), + toolsets.NewServerTool(SearchPullRequests(getClient, t)), toolsets.NewServerTool(GetPullRequestStatus(getClient, t)), toolsets.NewServerTool(GetPullRequestComments(getClient, t)), toolsets.NewServerTool(GetPullRequestReviews(getClient, t)), @@ -143,6 +148,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG tsg.AddToolset(contextTools) tsg.AddToolset(repos) tsg.AddToolset(issues) + tsg.AddToolset(orgs) tsg.AddToolset(users) tsg.AddToolset(pullRequests) tsg.AddToolset(actions)