From e110366fb3bcec9f2db4e958790ca340b7ba1e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Sat, 31 May 2025 10:12:53 -0700 Subject: [PATCH 01/11] feat: initial support for discussions - Minimal --- README.md | 28 +++ pkg/github/discussions.go | 340 ++++++++++++++++++++++++++++ pkg/github/discussions_test.go | 392 +++++++++++++++++++++++++++++++++ pkg/github/tools.go | 10 + script/get-discussions | 5 + 5 files changed, 775 insertions(+) create mode 100644 pkg/github/discussions.go create mode 100644 pkg/github/discussions_test.go create mode 100755 script/get-discussions diff --git a/README.md b/README.md index 7b9e20fc..4b7c1588 100644 --- a/README.md +++ b/README.md @@ -149,6 +149,7 @@ The following sets of tools are available (all are on by default): | ----------------------- | ------------------------------------------------------------- | | `repos` | Repository-related tools (file operations, branches, commits) | | `issues` | Issue-related tools (create, read, update, comment) | +| `discussions` | GitHub Discussions tools (list, get, comments) | | `users` | Anything relating to GitHub Users | | `pull_requests` | Pull request operations (create, merge, review) | | `code_security` | Code scanning alerts and security features | @@ -581,6 +582,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `secret_type`: The secret types to be filtered for in a comma-separated list (string, optional) - `resolution`: The resolution status (string, optional) +<<<<<<< HEAD ### Notifications - **list_notifications** – List notifications for a GitHub user @@ -614,6 +616,32 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `repo`: The name of the repository (string, required) - `action`: Action to perform: `ignore`, `watch`, or `delete` (string, required) +======= +>>>>>>> 8d6b84c (doc: add support for GitHub discussions, with an example.) +### Discussions + +> [!NOTE] +> As there is no support for discussions in the native GitHub go library, this toolset is deliberately limited to a few basic functions. The plan is to first implement native support in the GitHub go library, then use it for a better and consistent support in the MCP server. + +- **list_discussions** - List discussions for a repository + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `state`: State filter (open, closed, all) (string, optional) + - `labels`: Filter by label names (string[], optional) + - `since`: Filter by date (ISO 8601 timestamp) (string, optional) + - `page`: Page number (number, optional) + - `perPage`: Results per page (number, optional) + +- **get_discussion** - Get a specific discussion by ID + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `discussion_id`: Discussion ID (number, required) + +- **get_discussion_comments** - Get comments from a discussion + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `discussion_id`: Discussion ID (number, required) + ## Resources ### Repository Content diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go new file mode 100644 index 00000000..55ec32de --- /dev/null +++ b/pkg/github/discussions.go @@ -0,0 +1,340 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/go-viper/mapstructure/v2" + "github.com/google/go-github/v69/github" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" + "github.com/shurcooL/githubv4" +) + +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.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_DISCUSSIONS_USER_TITLE", "List discussions"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithString("categoryId", + mcp.Description("Category ID filter"), + ), + mcp.WithString("since", + mcp.Description("Filter by date (ISO 8601 timestamp)"), + ), + mcp.WithString("sort", + mcp.Description("Sort field"), + mcp.DefaultString("CREATED_AT"), + mcp.Enum("CREATED_AT", "UPDATED_AT"), + ), + mcp.WithString("direction", + mcp.Description("Sort direction"), + mcp.DefaultString("DESC"), + mcp.Enum("ASC", "DESC"), + ), + mcp.WithNumber("first", + mcp.Description("Number of discussions to return per page (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + ), + mcp.WithString("after", + mcp.Description("Cursor for pagination, use the 'after' field from the previous response"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + // Decode params + var params struct { + Owner string + Repo string + CategoryId string + Since string + Sort string + Direction string + First int32 + After string + } + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + // Get GraphQL client + client, err := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + } + // Prepare GraphQL query + var q struct { + Repository struct { + Discussions struct { + Nodes []struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` + } + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + // Build query variables + vars := map[string]interface{}{ + "owner": githubv4.String(params.Owner), + "repo": githubv4.String(params.Repo), + "categoryId": githubv4.ID(params.CategoryId), + "sort": githubv4.DiscussionOrderField(params.Sort), + "direction": githubv4.OrderDirection(params.Direction), + "first": githubv4.Int(params.First), + "after": githubv4.String(params.After), + } + // Execute query + if err := client.Query(ctx, &q, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + // Map nodes to GitHub Issue objects - there is no discussion type in the GitHub API, so we use Issue to benefit from existing code + var discussions []*github.Issue + for _, n := range q.Repository.Discussions.Nodes { + di := &github.Issue{ + Number: github.Ptr(int(n.Number)), + Title: github.Ptr(string(n.Title)), + HTMLURL: github.Ptr(string(n.URL)), + CreatedAt: &github.Timestamp{Time: n.CreatedAt.Time}, + } + discussions = append(discussions, di) + } + + // Post filtering discussions based on 'since' parameter + if params.Since != "" { + sinceTime, err := time.Parse(time.RFC3339, params.Since) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("invalid 'since' timestamp: %v", err)), nil + } + var filteredDiscussions []*github.Issue + for _, d := range discussions { + if d.CreatedAt.Time.After(sinceTime) { + filteredDiscussions = append(filteredDiscussions, d) + } + } + discussions = filteredDiscussions + } + + // Marshal and return + out, err := json.Marshal(discussions) + if err != nil { + return nil, fmt.Errorf("failed to marshal discussions: %w", err) + } + return mcp.NewToolResultText(string(out)), nil + } +} + +func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_discussion", + mcp.WithDescription(t("TOOL_GET_DISCUSSION_DESCRIPTION", "Get a specific discussion by ID")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_DISCUSSION_USER_TITLE", "Get discussion"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("discussion_id", + mcp.Required(), + mcp.Description("Discussion ID"), + ), + ), + 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 + } + discussionID, err := RequiredInt(request, "discussion_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + } + + var q struct { + Repository struct { + Discussion struct { + Number githubv4.Int + Body githubv4.String + State githubv4.String + CreatedAt githubv4.DateTime + URL githubv4.String `graphql:"url"` + } `graphql:"discussion(number: $discussionID)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "discussionID": githubv4.Int(discussionID), + } + if err := client.Query(ctx, &q, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + d := q.Repository.Discussion + discussion := &github.Issue{ + Number: github.Ptr(int(d.Number)), + Body: github.Ptr(string(d.Body)), + State: github.Ptr(string(d.State)), + HTMLURL: github.Ptr(string(d.URL)), + CreatedAt: &github.Timestamp{Time: d.CreatedAt.Time}, + } + out, err := json.Marshal(discussion) + if err != nil { + return nil, fmt.Errorf("failed to marshal discussion: %w", err) + } + + return mcp.NewToolResultText(string(out)), nil + } +} + +func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("get_discussion_comments", + mcp.WithDescription(t("TOOL_GET_DISCUSSION_COMMENTS_DESCRIPTION", "Get comments from a discussion")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_GET_DISCUSSION_COMMENTS_USER_TITLE", "Get discussion comments"), + ReadOnlyHint: toBoolPtr(true), + }), + mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")), + mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")), + mcp.WithNumber("discussion_id", mcp.Required(), mcp.Description("Discussion ID")), + ), + 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 + } + discussionID, err := RequiredInt(request, "discussion_id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + } + + var q struct { + Repository struct { + Discussion struct { + Comments struct { + Nodes []struct { + Body githubv4.String + } + } `graphql:"comments(first:100)"` + } `graphql:"discussion(number: $discussionID)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "discussionID": githubv4.Int(discussionID), + } + if err := client.Query(ctx, &q, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + var comments []*github.IssueComment + for _, c := range q.Repository.Discussion.Comments.Nodes { + comments = append(comments, &github.IssueComment{Body: github.Ptr(string(c.Body))}) + } + + out, err := json.Marshal(comments) + if err != nil { + return nil, fmt.Errorf("failed to marshal comments: %w", err) + } + + return mcp.NewToolResultText(string(out)), nil + } +} + +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 categorie with their id and name, for a repository")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_LIST_DISCUSSION_CATEGORIES_USER_TITLE", "List discussion categories"), + 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 := getGQLClient(ctx) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil + } + var q struct { + Repository struct { + DiscussionCategories struct { + Nodes []struct { + ID githubv4.ID + Name githubv4.String + } + } `graphql:"discussionCategories(first: 30)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + } + if err := client.Query(ctx, &q, vars); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + var categories []map[string]string + for _, c := range q.Repository.DiscussionCategories.Nodes { + categories = append(categories, map[string]string{ + "id": fmt.Sprint(c.ID), + "name": string(c.Name), + }) + } + out, err := json.Marshal(categories) + if err != nil { + return nil, fmt.Errorf("failed to marshal discussion categories: %w", err) + } + return mcp.NewToolResultText(string(out)), nil + } +} diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go new file mode 100644 index 00000000..8cd95a6f --- /dev/null +++ b/pkg/github/discussions_test.go @@ -0,0 +1,392 @@ +package github + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/github/github-mcp-server/internal/githubv4mock" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/go-github/v69/github" + "github.com/shurcooL/githubv4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + discussionsAll = []map[string]any{ + {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "category": map[string]any{"name": "news"}, "url": "https://github.com/owner/repo/discussions/1"}, + {"number": 2, "title": "Discussion 2 title", "createdAt": "2023-02-01T00:00:00Z", "category": map[string]any{"name": "updates"}, "url": "https://github.com/owner/repo/discussions/2"}, + {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "category": map[string]any{"name": "questions"}, "url": "https://github.com/owner/repo/discussions/3"}, + } + mockResponseListAll = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{"nodes": discussionsAll}, + }, + }) + mockResponseCategory = githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussions": map[string]any{"nodes": discussionsAll[:1]}, // Only return the first discussion for category test + }, + }) + mockErrorRepoNotFound = githubv4mock.ErrorResponse("repository not found") +) + +func Test_ListDiscussions(t *testing.T) { + // Verify tool definition and schema + toolDef, _ := ListDiscussions(nil, translations.NullTranslationHelper) + assert.Equal(t, "list_discussions", toolDef.Name) + assert.NotEmpty(t, toolDef.Description) + assert.Contains(t, toolDef.InputSchema.Properties, "owner") + assert.Contains(t, toolDef.InputSchema.Properties, "repo") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"}) + + // GraphQL query struct + var q struct { + Repository struct { + Discussions struct { + Nodes []struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + Category struct { + Name githubv4.String + } `graphql:"category"` + URL githubv4.String `graphql:"url"` + } + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + varsListAll := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "categoryId": githubv4.ID(""), + "sort": githubv4.DiscussionOrderField(""), + "direction": githubv4.OrderDirection(""), + "first": githubv4.Int(0), + "after": githubv4.String(""), + } + + varsListInvalid := map[string]interface{}{ + "owner": githubv4.String("invalid"), + "repo": githubv4.String("repo"), + "categoryId": githubv4.ID(""), + "sort": githubv4.DiscussionOrderField(""), + "direction": githubv4.OrderDirection(""), + "first": githubv4.Int(0), + "after": githubv4.String(""), + } + + varsListWithCategory := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "categoryId": githubv4.ID("12345"), + "sort": githubv4.DiscussionOrderField(""), + "direction": githubv4.OrderDirection(""), + "first": githubv4.Int(0), + "after": githubv4.String(""), + } + + tests := []struct { + name string + vars map[string]interface{} + reqParams map[string]interface{} + response githubv4mock.GQLResponse + expectError bool + expectedIds []int64 + errContains string + }{ + { + name: "list all discussions", + vars: varsListAll, + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + }, + response: mockResponseListAll, + expectError: false, + expectedIds: []int64{1, 2, 3}, + }, + { + name: "invalid owner or repo", + vars: varsListInvalid, + reqParams: map[string]interface{}{ + "owner": "invalid", + "repo": "repo", + }, + response: mockErrorRepoNotFound, + expectError: true, + errContains: "repository not found", + }, + { + name: "list discussions with category", + vars: varsListWithCategory, + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "categoryId": "12345", + }, + response: mockResponseCategory, + expectError: false, + expectedIds: []int64{1}, + }, + { + name: "list discussions with since date", + vars: varsListAll, + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "since": "2023-01-10T00:00:00Z", + }, + response: mockResponseListAll, + expectError: false, + expectedIds: []int64{2, 3}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matcher := githubv4mock.NewQueryMatcher(q, tc.vars, tc.response) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + gqlClient := githubv4.NewClient(httpClient) + _, handler := ListDiscussions(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + + req := createMCPRequest(tc.reqParams) + res, err := handler(context.Background(), req) + text := getTextResult(t, res).Text + + if tc.expectError { + require.True(t, res.IsError) + assert.Contains(t, text, tc.errContains) + return + } + require.NoError(t, err) + + var returnedDiscussions []*github.Issue + err = json.Unmarshal([]byte(text), &returnedDiscussions) + require.NoError(t, err) + + assert.Len(t, returnedDiscussions, len(tc.expectedIds), "Expected %d discussions, got %d", len(tc.expectedIds), len(returnedDiscussions)) + + // If no discussions are expected, skip further checks + if len(tc.expectedIds) == 0 { + return + } + + // Create a map of expected IDs for easier checking + expectedIDMap := make(map[int64]bool) + for _, id := range tc.expectedIds { + expectedIDMap[id] = true + } + + for _, discussion := range returnedDiscussions { + // Check if the discussion Number is in the expected list + assert.True(t, expectedIDMap[int64(*discussion.Number)], "Unexpected discussion Number: %d", *discussion.Number) + } + }) + } +} + +func Test_GetDiscussion(t *testing.T) { + // Verify tool definition and schema + toolDef, _ := GetDiscussion(nil, translations.NullTranslationHelper) + assert.Equal(t, "get_discussion", toolDef.Name) + assert.NotEmpty(t, toolDef.Description) + assert.Contains(t, toolDef.InputSchema.Properties, "owner") + assert.Contains(t, toolDef.InputSchema.Properties, "repo") + assert.Contains(t, toolDef.InputSchema.Properties, "discussion_id") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussion_id"}) + + var q struct { + Repository struct { + Discussion struct { + Number githubv4.Int + Body githubv4.String + State githubv4.String + CreatedAt githubv4.DateTime + URL githubv4.String `graphql:"url"` + } `graphql:"discussion(number: $discussionID)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "discussionID": githubv4.Int(1), + } + tests := []struct { + name string + response githubv4mock.GQLResponse + expectError bool + expected *github.Issue + errContains string + }{ + { + name: "successful retrieval", + response: githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{"discussion": map[string]any{ + "number": 1, + "body": "This is a test discussion", + "state": "open", + "url": "https://github.com/owner/repo/discussions/1", + "createdAt": "2025-04-25T12:00:00Z", + }}, + }), + expectError: false, + expected: &github.Issue{ + HTMLURL: github.Ptr("https://github.com/owner/repo/discussions/1"), + Number: github.Ptr(1), + Body: github.Ptr("This is a test discussion"), + State: github.Ptr("open"), + CreatedAt: &github.Timestamp{Time: time.Date(2025, 4, 25, 12, 0, 0, 0, time.UTC)}, + }, + }, + { + name: "discussion not found", + response: githubv4mock.ErrorResponse("discussion not found"), + expectError: true, + errContains: "discussion not found", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matcher := githubv4mock.NewQueryMatcher(q, vars, tc.response) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + gqlClient := githubv4.NewClient(httpClient) + _, handler := GetDiscussion(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + + req := createMCPRequest(map[string]interface{}{"owner": "owner", "repo": "repo", "discussion_id": float64(1)}) + res, err := handler(context.Background(), req) + text := getTextResult(t, res).Text + + if tc.expectError { + require.True(t, res.IsError) + assert.Contains(t, text, tc.errContains) + return + } + + require.NoError(t, err) + var out github.Issue + 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.Body, *out.Body) + assert.Equal(t, *tc.expected.State, *out.State) + }) + } +} + +func Test_GetDiscussionComments(t *testing.T) { + // Verify tool definition and schema + toolDef, _ := GetDiscussionComments(nil, translations.NullTranslationHelper) + assert.Equal(t, "get_discussion_comments", toolDef.Name) + assert.NotEmpty(t, toolDef.Description) + assert.Contains(t, toolDef.InputSchema.Properties, "owner") + assert.Contains(t, toolDef.InputSchema.Properties, "repo") + assert.Contains(t, toolDef.InputSchema.Properties, "discussion_id") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussion_id"}) + + var q struct { + Repository struct { + Discussion struct { + Comments struct { + Nodes []struct { + Body githubv4.String + } + } `graphql:"comments(first:100)"` + } `graphql:"discussion(number: $discussionID)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "discussionID": githubv4.Int(1), + } + mockResponse := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussion": map[string]any{ + "comments": map[string]any{ + "nodes": []map[string]any{ + {"body": "This is the first comment"}, + {"body": "This is the second comment"}, + }, + }, + }, + }, + }) + matcher := githubv4mock.NewQueryMatcher(q, vars, mockResponse) + httpClient := githubv4mock.NewMockedHTTPClient(matcher) + gqlClient := githubv4.NewClient(httpClient) + _, handler := GetDiscussionComments(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + + request := createMCPRequest(map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "discussion_id": float64(1), + }) + + result, err := handler(context.Background(), request) + require.NoError(t, err) + + textContent := getTextResult(t, result) + + var returnedComments []*github.IssueComment + err = json.Unmarshal([]byte(textContent.Text), &returnedComments) + require.NoError(t, err) + assert.Len(t, returnedComments, 2) + expectedBodies := []string{"This is the first comment", "This is the second comment"} + for i, comment := range returnedComments { + assert.Equal(t, expectedBodies[i], *comment.Body) + } +} + +func Test_ListDiscussionCategories(t *testing.T) { + var q struct { + Repository struct { + DiscussionCategories struct { + Nodes []struct { + ID githubv4.ID + Name githubv4.String + } + } `graphql:"discussionCategories(first: 30)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + vars := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + } + mockResp := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussionCategories": map[string]any{ + "nodes": []map[string]any{ + {"id": "123", "name": "CategoryOne"}, + {"id": "456", "name": "CategoryTwo"}, + }, + }, + }, + }) + matcher := githubv4mock.NewQueryMatcher(q, 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"}) + + request := createMCPRequest(map[string]interface{}{"owner": "owner", "repo": "repo"}) + result, err := handler(context.Background(), request) + require.NoError(t, err) + + text := getTextResult(t, result).Text + var categories []map[string]string + require.NoError(t, json.Unmarshal([]byte(text), &categories)) + assert.Len(t, categories, 2) + assert.Equal(t, "123", categories[0]["id"]) + assert.Equal(t, "CategoryOne", categories[0]["name"]) + assert.Equal(t, "456", categories[1]["id"]) + assert.Equal(t, "CategoryTwo", categories[1]["name"]) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 9c1ab34a..7a5494dc 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -104,6 +104,14 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, toolsets.NewServerTool(ManageRepositoryNotificationSubscription(getClient, t)), ) + discussions := toolsets.NewToolset("discussions", "GitHub Discussions related tools"). + AddReadTools( + toolsets.NewServerTool(ListDiscussions(getGQLClient, t)), + toolsets.NewServerTool(GetDiscussion(getGQLClient, t)), + toolsets.NewServerTool(GetDiscussionComments(getGQLClient, t)), + toolsets.NewServerTool(ListDiscussionCategories(getGQLClient, t)), + ) + // Keep experiments alive so the system doesn't error out when it's always enabled experiments := toolsets.NewToolset("experiments", "Experimental features that are not considered stable yet") @@ -116,6 +124,8 @@ func InitToolsets(passedToolsets []string, readOnly bool, getClient GetClientFn, tsg.AddToolset(secretProtection) tsg.AddToolset(notifications) tsg.AddToolset(experiments) + tsg.AddToolset(discussions) + // Enable the requested features if err := tsg.EnableToolsets(passedToolsets); err != nil { diff --git a/script/get-discussions b/script/get-discussions new file mode 100755 index 00000000..3e68abf2 --- /dev/null +++ b/script/get-discussions @@ -0,0 +1,5 @@ +#!/bin/bash + +# echo '{"jsonrpc":"2.0","id":3,"params":{"name":"list_discussions","arguments": {"owner": "github", "repo": "securitylab", "first": 10, "since": "2025-04-01T00:00:00Z"}},"method":"tools/call"}' | go run cmd/github-mcp-server/main.go stdio | jq . +echo '{"jsonrpc":"2.0","id":3,"params":{"name":"list_discussions","arguments": {"owner": "github", "repo": "securitylab", "first": 10, "since": "2025-04-01T00:00:00Z", "sort": "CREATED_AT", "direction": "DESC"}},"method":"tools/call"}' | go run cmd/github-mcp-server/main.go stdio | jq . + From 6de3210ceb1217df873c224bc0c6e1a1f3a9a6cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Sat, 31 May 2025 09:34:54 -0700 Subject: [PATCH 02/11] Add documentation --- README.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 4b7c1588..19af3d96 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ The following sets of tools are available (all are on by default): | ----------------------- | ------------------------------------------------------------- | | `repos` | Repository-related tools (file operations, branches, commits) | | `issues` | Issue-related tools (create, read, update, comment) | -| `discussions` | GitHub Discussions tools (list, get, comments) | +| `discussions` | GitHub Discussions tools (list, get, comments, categories) | | `users` | Anything relating to GitHub Users | | `pull_requests` | Pull request operations (create, merge, review) | | `code_security` | Code scanning alerts and security features | @@ -582,7 +582,6 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `secret_type`: The secret types to be filtered for in a comma-separated list (string, optional) - `resolution`: The resolution status (string, optional) -<<<<<<< HEAD ### Notifications - **list_notifications** – List notifications for a GitHub user @@ -616,21 +615,17 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `repo`: The name of the repository (string, required) - `action`: Action to perform: `ignore`, `watch`, or `delete` (string, required) -======= ->>>>>>> 8d6b84c (doc: add support for GitHub discussions, with an example.) ### Discussions -> [!NOTE] -> As there is no support for discussions in the native GitHub go library, this toolset is deliberately limited to a few basic functions. The plan is to first implement native support in the GitHub go library, then use it for a better and consistent support in the MCP server. - - **list_discussions** - List discussions for a repository - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - - `state`: State filter (open, closed, all) (string, optional) - - `labels`: Filter by label names (string[], optional) + - `categoryId`: Filter by category ID (string, optional) - `since`: Filter by date (ISO 8601 timestamp) (string, optional) - - `page`: Page number (number, optional) - - `perPage`: Results per page (number, optional) + - `first`: Pagination - Number of records to retrieve (number, optional) + - `after`: Pagination - Cursor to start with (string, optional) + - `sort`: Sort by ('CREATED_AT', 'UPDATED_AT') (string, optional) + - `direction`: Sort direction ('ASC', 'DESC') (string, optional) - **get_discussion** - Get a specific discussion by ID - `owner`: Repository owner (string, required) @@ -642,6 +637,10 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `repo`: Repository name (string, required) - `discussion_id`: Discussion ID (number, required) +- **list_discussion_categories** - List discussion categories for a repository, with their IDs and names + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + ## Resources ### Repository Content From 5a4838957e99d5d5c7bf0ac5c08e60d6038924c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Sat, 31 May 2025 11:40:06 -0700 Subject: [PATCH 03/11] Fix linting issues and improve naming consistency --- pkg/github/discussions.go | 59 +++++++++++++++------------------- pkg/github/discussions_test.go | 32 +++++++++--------- 2 files changed, 42 insertions(+), 49 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 55ec32de..3a1d9aa5 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -59,7 +59,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp var params struct { Owner string Repo string - CategoryId string + CategoryID string Since string Sort string Direction string @@ -94,7 +94,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp vars := map[string]interface{}{ "owner": githubv4.String(params.Owner), "repo": githubv4.String(params.Repo), - "categoryId": githubv4.ID(params.CategoryId), + "categoryId": githubv4.ID(params.CategoryID), "sort": githubv4.DiscussionOrderField(params.Sort), "direction": githubv4.OrderDirection(params.Direction), "first": githubv4.Int(params.First), @@ -155,25 +155,21 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper mcp.Required(), mcp.Description("Repository name"), ), - mcp.WithNumber("discussion_id", + mcp.WithNumber("discussionNumber", mcp.Required(), - mcp.Description("Discussion ID"), + mcp.Description("Discussion Number"), ), ), 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 + // Decode params + var params struct { + Owner string + Repo string + DiscussionNumber int32 } - discussionID, err := RequiredInt(request, "discussion_id") - if err != nil { + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } - client, err := getGQLClient(ctx) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil @@ -187,13 +183,13 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper State githubv4.String CreatedAt githubv4.DateTime URL githubv4.String `graphql:"url"` - } `graphql:"discussion(number: $discussionID)"` + } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "discussionID": githubv4.Int(discussionID), + "owner": githubv4.String(params.Owner), + "repo": githubv4.String(params.Repo), + "discussionNumber": githubv4.Int(params.DiscussionNumber), } if err := client.Query(ctx, &q, vars); err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -224,19 +220,16 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati }), mcp.WithString("owner", mcp.Required(), mcp.Description("Repository owner")), mcp.WithString("repo", mcp.Required(), mcp.Description("Repository name")), - mcp.WithNumber("discussion_id", mcp.Required(), mcp.Description("Discussion ID")), + mcp.WithNumber("discussionNumber", mcp.Required(), mcp.Description("Discussion Number")), ), 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 + // Decode params + var params struct { + Owner string + Repo string + DiscussionNumber int32 } - discussionID, err := RequiredInt(request, "discussion_id") - if err != nil { + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -253,13 +246,13 @@ func GetDiscussionComments(getGQLClient GetGQLClientFn, t translations.Translati Body githubv4.String } } `graphql:"comments(first:100)"` - } `graphql:"discussion(number: $discussionID)"` + } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), - "discussionID": githubv4.Int(discussionID), + "owner": githubv4.String(params.Owner), + "repo": githubv4.String(params.Repo), + "discussionNumber": githubv4.Int(params.DiscussionNumber), } if err := client.Query(ctx, &q, vars); err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -280,7 +273,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 categorie 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")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ Title: t("TOOL_LIST_DISCUSSION_CATEGORIES_USER_TITLE", "List discussion categories"), ReadOnlyHint: toBoolPtr(true), diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 8cd95a6f..4142e790 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -196,8 +196,8 @@ func Test_GetDiscussion(t *testing.T) { assert.NotEmpty(t, toolDef.Description) assert.Contains(t, toolDef.InputSchema.Properties, "owner") assert.Contains(t, toolDef.InputSchema.Properties, "repo") - assert.Contains(t, toolDef.InputSchema.Properties, "discussion_id") - assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussion_id"}) + assert.Contains(t, toolDef.InputSchema.Properties, "discussionNumber") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) var q struct { Repository struct { @@ -207,13 +207,13 @@ func Test_GetDiscussion(t *testing.T) { State githubv4.String CreatedAt githubv4.DateTime URL githubv4.String `graphql:"url"` - } `graphql:"discussion(number: $discussionID)"` + } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "discussionID": githubv4.Int(1), + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "discussionNumber": githubv4.Int(1), } tests := []struct { name string @@ -256,7 +256,7 @@ func Test_GetDiscussion(t *testing.T) { gqlClient := githubv4.NewClient(httpClient) _, handler := GetDiscussion(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) - req := createMCPRequest(map[string]interface{}{"owner": "owner", "repo": "repo", "discussion_id": float64(1)}) + req := createMCPRequest(map[string]interface{}{"owner": "owner", "repo": "repo", "discussionNumber": int32(1)}) res, err := handler(context.Background(), req) text := getTextResult(t, res).Text @@ -284,8 +284,8 @@ func Test_GetDiscussionComments(t *testing.T) { assert.NotEmpty(t, toolDef.Description) assert.Contains(t, toolDef.InputSchema.Properties, "owner") assert.Contains(t, toolDef.InputSchema.Properties, "repo") - assert.Contains(t, toolDef.InputSchema.Properties, "discussion_id") - assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussion_id"}) + assert.Contains(t, toolDef.InputSchema.Properties, "discussionNumber") + assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) var q struct { Repository struct { @@ -295,13 +295,13 @@ func Test_GetDiscussionComments(t *testing.T) { Body githubv4.String } } `graphql:"comments(first:100)"` - } `graphql:"discussion(number: $discussionID)"` + } `graphql:"discussion(number: $discussionNumber)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), - "discussionID": githubv4.Int(1), + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "discussionNumber": githubv4.Int(1), } mockResponse := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ @@ -321,9 +321,9 @@ func Test_GetDiscussionComments(t *testing.T) { _, handler := GetDiscussionComments(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) request := createMCPRequest(map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "discussion_id": float64(1), + "owner": "owner", + "repo": "repo", + "discussionNumber": int32(1), }) result, err := handler(context.Background(), request) From 647d0b3728409b05e3bb99a2acdb72b6eec75950 Mon Sep 17 00:00:00 2001 From: Xavier RENE-CORAIL Date: Sat, 31 May 2025 11:49:42 -0700 Subject: [PATCH 04/11] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 19af3d96..97f94448 100644 --- a/README.md +++ b/README.md @@ -630,7 +630,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - **get_discussion** - Get a specific discussion by ID - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - - `discussion_id`: Discussion ID (number, required) + - `discussionNumber`: Discussion number (required) - **get_discussion_comments** - Get comments from a discussion - `owner`: Repository owner (string, required) From 2f90a60df8c24c75e72a0e9e91c64c0302da7156 Mon Sep 17 00:00:00 2001 From: Xavier RENE-CORAIL Date: Sat, 31 May 2025 11:49:48 -0700 Subject: [PATCH 05/11] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 97f94448..b2943fa8 100644 --- a/README.md +++ b/README.md @@ -635,7 +635,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - **get_discussion_comments** - Get comments from a discussion - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - - `discussion_id`: Discussion ID (number, required) + - `discussionNumber`: Discussion number (required) - **list_discussion_categories** - List discussion categories for a repository, with their IDs and names - `owner`: Repository owner (string, required) From e10d76a8b438e999835afcc10c6c0ddcd7a46391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Wed, 4 Jun 2025 10:17:34 -0700 Subject: [PATCH 06/11] Add missing pagination parameters to ListDiscussions --- README.md | 2 ++ pkg/github/discussions.go | 26 ++++++++++++++- pkg/github/discussions_test.go | 60 +++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b2943fa8..ac4ddd80 100644 --- a/README.md +++ b/README.md @@ -623,7 +623,9 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `categoryId`: Filter by category ID (string, optional) - `since`: Filter by date (ISO 8601 timestamp) (string, optional) - `first`: Pagination - Number of records to retrieve (number, optional) + - `last`: Pagination - Number of records to retrieve from the end (number, optional) - `after`: Pagination - Cursor to start with (string, optional) + - `before`: Pagination - Cursor to end with (string, optional) - `sort`: Sort by ('CREATED_AT', 'UPDATED_AT') (string, optional) - `direction`: Sort direction ('ASC', 'DESC') (string, optional) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 3a1d9aa5..b86b6722 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -50,9 +50,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp mcp.Min(1), mcp.Max(100), ), + mcp.WithNumber("last", + mcp.Description("Number of discussions to return from the end (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + ), mcp.WithString("after", mcp.Description("Cursor for pagination, use the 'after' field from the previous response"), ), + mcp.WithString("before", + mcp.Description("Cursor for pagination, use the 'before' field from the previous response"), + ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Decode params @@ -64,11 +72,25 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp Sort string Direction string First int32 + Last int32 After string + Before string } if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } + if params.First != 0 && params.Last != 0 { + return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil + } + if params.After != "" && params.Before != "" { + return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil + } + if params.After != "" && params.Last != 0 { + return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil + } + if params.Before != "" && params.First != 0 { + return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil + } // Get GraphQL client client, err := getGQLClient(ctx) if err != nil { @@ -87,7 +109,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp } `graphql:"category"` URL githubv4.String `graphql:"url"` } - } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"` + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"` } `graphql:"repository(owner: $owner, name: $repo)"` } // Build query variables @@ -98,7 +120,9 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp "sort": githubv4.DiscussionOrderField(params.Sort), "direction": githubv4.OrderDirection(params.Direction), "first": githubv4.Int(params.First), + "last": githubv4.Int(params.Last), "after": githubv4.String(params.After), + "before": githubv4.String(params.Before), } // Execute query if err := client.Query(ctx, &q, vars); err != nil { diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 4142e790..a24ad796 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -55,7 +55,7 @@ func Test_ListDiscussions(t *testing.T) { } `graphql:"category"` URL githubv4.String `graphql:"url"` } - } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after)"` + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -66,7 +66,9 @@ func Test_ListDiscussions(t *testing.T) { "sort": githubv4.DiscussionOrderField(""), "direction": githubv4.OrderDirection(""), "first": githubv4.Int(0), + "last": githubv4.Int(0), "after": githubv4.String(""), + "before": githubv4.String(""), } varsListInvalid := map[string]interface{}{ @@ -76,7 +78,9 @@ func Test_ListDiscussions(t *testing.T) { "sort": githubv4.DiscussionOrderField(""), "direction": githubv4.OrderDirection(""), "first": githubv4.Int(0), + "last": githubv4.Int(0), "after": githubv4.String(""), + "before": githubv4.String(""), } varsListWithCategory := map[string]interface{}{ @@ -86,7 +90,9 @@ func Test_ListDiscussions(t *testing.T) { "sort": githubv4.DiscussionOrderField(""), "direction": githubv4.OrderDirection(""), "first": githubv4.Int(0), + "last": githubv4.Int(0), "after": githubv4.String(""), + "before": githubv4.String(""), } tests := []struct { @@ -144,6 +150,58 @@ func Test_ListDiscussions(t *testing.T) { expectError: false, expectedIds: []int64{2, 3}, }, + { + name: "both first and last parameters provided", + vars: varsListAll, // vars don't matter since error occurs before GraphQL call + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "first": int32(10), + "last": int32(5), + }, + response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call + expectError: true, + errContains: "only one of 'first' or 'last' may be specified", + }, + { + name: "after with last parameters provided", + vars: varsListAll, // vars don't matter since error occurs before GraphQL call + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "after": "cursor123", + "last": int32(5), + }, + response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call + expectError: true, + errContains: "'after' cannot be used with 'last'. Did you mean to use 'before' instead?", + }, + { + name: "before with first parameters provided", + vars: varsListAll, // vars don't matter since error occurs before GraphQL call + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "before": "cursor456", + "first": int32(10), + }, + response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call + expectError: true, + errContains: "'before' cannot be used with 'first'. Did you mean to use 'after' instead?", + }, + { + name: "both after and before parameters provided", + vars: varsListAll, // vars don't matter since error occurs before GraphQL call + reqParams: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "after": "cursor123", + "before": "cursor456", + }, + response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call + expectError: true, + errContains: "only one of 'after' or 'before' may be specified", + }, } for _, tc := range tests { From bc5abbd0c4ef91393e5ce58f155a526b80be4736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Wed, 4 Jun 2025 13:02:53 -0700 Subject: [PATCH 07/11] Add answered parameter to ListDiscussions --- README.md | 1 + pkg/github/discussions.go | 7 ++++++- pkg/github/discussions_test.go | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ac4ddd80..e3a14f6d 100644 --- a/README.md +++ b/README.md @@ -628,6 +628,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - `before`: Pagination - Cursor to end with (string, optional) - `sort`: Sort by ('CREATED_AT', 'UPDATED_AT') (string, optional) - `direction`: Sort direction ('ASC', 'DESC') (string, optional) + - `answered`: Filter by whether discussions have been answered or not (boolean, optional) - **get_discussion** - Get a specific discussion by ID - `owner`: Repository owner (string, required) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index b86b6722..eb48e911 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -61,6 +61,9 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp mcp.WithString("before", mcp.Description("Cursor for pagination, use the 'before' field from the previous response"), ), + mcp.WithBoolean("answered", + mcp.Description("Filter by whether discussions have been answered or not"), + ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Decode params @@ -75,6 +78,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp Last int32 After string Before string + Answered bool } if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -109,7 +113,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp } `graphql:"category"` URL githubv4.String `graphql:"url"` } - } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"` + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before, answered: $answered)"` } `graphql:"repository(owner: $owner, name: $repo)"` } // Build query variables @@ -123,6 +127,7 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp "last": githubv4.Int(params.Last), "after": githubv4.String(params.After), "before": githubv4.String(params.Before), + "answered": githubv4.Boolean(params.Answered), } // Execute query if err := client.Query(ctx, &q, vars); err != nil { diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index a24ad796..7d06eb71 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -55,7 +55,7 @@ func Test_ListDiscussions(t *testing.T) { } `graphql:"category"` URL githubv4.String `graphql:"url"` } - } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before)"` + } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before, answered: $answered)"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -69,6 +69,7 @@ func Test_ListDiscussions(t *testing.T) { "last": githubv4.Int(0), "after": githubv4.String(""), "before": githubv4.String(""), + "answered": githubv4.Boolean(false), } varsListInvalid := map[string]interface{}{ @@ -81,6 +82,7 @@ func Test_ListDiscussions(t *testing.T) { "last": githubv4.Int(0), "after": githubv4.String(""), "before": githubv4.String(""), + "answered": githubv4.Boolean(false), } varsListWithCategory := map[string]interface{}{ @@ -93,6 +95,7 @@ func Test_ListDiscussions(t *testing.T) { "last": githubv4.Int(0), "after": githubv4.String(""), "before": githubv4.String(""), + "answered": githubv4.Boolean(false), } tests := []struct { From ae5f1b22e0421ea80c92899af31a40a84119779f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Wed, 4 Jun 2025 16:28:40 -0700 Subject: [PATCH 08/11] Implement pagination for ListDiscussionCategories --- README.md | 4 +++ pkg/github/discussions.go | 55 +++++++++++++++++++++++++++++----- pkg/github/discussions_test.go | 10 +++++-- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index e3a14f6d..7a00c5f8 100644 --- a/README.md +++ b/README.md @@ -643,6 +643,10 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - **list_discussion_categories** - List discussion categories for a repository, with their IDs and names - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) + - `first`: Pagination - Number of categories to return per page (number, optional, min 1, max 100) + - `last`: Pagination - Number of categories to return from the end (number, optional, min 1, max 100) + - `after`: Pagination - Cursor to start with (string, optional) + - `before`: Pagination - Cursor to end with (string, optional) ## Resources diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index eb48e911..a6ffb35e 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -315,16 +315,51 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl mcp.Required(), mcp.Description("Repository name"), ), + mcp.WithNumber("first", + mcp.Description("Number of categories to return per page (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + ), + mcp.WithNumber("last", + mcp.Description("Number of categories to return from the end (min 1, max 100)"), + mcp.Min(1), + mcp.Max(100), + ), + mcp.WithString("after", + mcp.Description("Cursor for pagination, use the 'after' field from the previous response"), + ), + mcp.WithString("before", + mcp.Description("Cursor for pagination, use the 'before' field from the previous response"), + ), ), 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 + // Decode params + var params struct { + Owner string + Repo string + First int32 + Last int32 + After string + Before string } - repo, err := requiredParam[string](request, "repo") - if err != nil { + if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil } + + // Validate pagination parameters + if params.First != 0 && params.Last != 0 { + return mcp.NewToolResultError("only one of 'first' or 'last' may be specified"), nil + } + if params.After != "" && params.Before != "" { + return mcp.NewToolResultError("only one of 'after' or 'before' may be specified"), nil + } + if params.After != "" && params.Last != 0 { + return mcp.NewToolResultError("'after' cannot be used with 'last'. Did you mean to use 'before' instead?"), nil + } + if params.Before != "" && params.First != 0 { + return mcp.NewToolResultError("'before' cannot be used with 'first'. Did you mean to use 'after' instead?"), nil + } + client, err := getGQLClient(ctx) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to get GitHub GQL client: %v", err)), nil @@ -336,12 +371,16 @@ func ListDiscussionCategories(getGQLClient GetGQLClientFn, t translations.Transl ID githubv4.ID Name githubv4.String } - } `graphql:"discussionCategories(first: 30)"` + } `graphql:"discussionCategories(first: $first, last: $last, after: $after, before: $before)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String(owner), - "repo": githubv4.String(repo), + "owner": githubv4.String(params.Owner), + "repo": githubv4.String(params.Repo), + "first": githubv4.Int(params.First), + "last": githubv4.Int(params.Last), + "after": githubv4.String(params.After), + "before": githubv4.String(params.Before), } if err := client.Query(ctx, &q, vars); err != nil { return mcp.NewToolResultError(err.Error()), nil diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 7d06eb71..18fdce6e 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -410,12 +410,16 @@ func Test_ListDiscussionCategories(t *testing.T) { ID githubv4.ID Name githubv4.String } - } `graphql:"discussionCategories(first: 30)"` + } `graphql:"discussionCategories(first: $first, last: $last, after: $after, before: $before)"` } `graphql:"repository(owner: $owner, name: $repo)"` } vars := map[string]interface{}{ - "owner": githubv4.String("owner"), - "repo": githubv4.String("repo"), + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "first": githubv4.Int(0), + "last": githubv4.Int(0), + "after": githubv4.String(""), + "before": githubv4.String(""), } mockResp := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ From 58cd74dfaaa7b028ced0470b260715809a3cf434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Thu, 5 Jun 2025 09:12:15 -0700 Subject: [PATCH 09/11] Make ListDiscussions more user-friendly by using category name filter, not id --- README.md | 2 +- pkg/github/discussions.go | 86 ++++++++++++++++++++++++++++------ pkg/github/discussions_test.go | 73 +++++++++++++++++++++++++---- 3 files changed, 136 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 7a00c5f8..e549ea25 100644 --- a/README.md +++ b/README.md @@ -620,7 +620,7 @@ export GITHUB_MCP_TOOL_ADD_ISSUE_COMMENT_DESCRIPTION="an alternative description - **list_discussions** - List discussions for a repository - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - - `categoryId`: Filter by category ID (string, optional) + - `category`: Filter by category name (string, optional) - `since`: Filter by date (ISO 8601 timestamp) (string, optional) - `first`: Pagination - Number of records to retrieve (number, optional) - `last`: Pagination - Number of records to retrieve from the end (number, optional) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index a6ffb35e..9a9b9b52 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -14,6 +14,56 @@ import ( "github.com/shurcooL/githubv4" ) +// GetAllDiscussionCategories retrieves all discussion categories for a repository +// by paginating through all pages and returns them as a map where the key is the +// category name and the value is the category ID. +func GetAllDiscussionCategories(ctx context.Context, client *githubv4.Client, owner, repo string) (map[string]string, error) { + categories := make(map[string]string) + var after string + hasNextPage := true + + for hasNextPage { + // Prepare GraphQL query with pagination + var q struct { + Repository struct { + DiscussionCategories struct { + Nodes []struct { + ID githubv4.ID + Name githubv4.String + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"discussionCategories(first: 100, after: $after)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + vars := map[string]interface{}{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "after": githubv4.String(after), + } + + if err := client.Query(ctx, &q, vars); err != nil { + return nil, fmt.Errorf("failed to query discussion categories: %w", err) + } + + // Add categories to the map + for _, category := range q.Repository.DiscussionCategories.Nodes { + categories[string(category.Name)] = fmt.Sprint(category.ID) + } + + // Check if there are more pages + hasNextPage = bool(q.Repository.DiscussionCategories.PageInfo.HasNextPage) + if hasNextPage { + after = string(q.Repository.DiscussionCategories.PageInfo.EndCursor) + } + } + + return categories, nil +} + 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")), @@ -29,8 +79,8 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp mcp.Required(), mcp.Description("Repository name"), ), - mcp.WithString("categoryId", - mcp.Description("Category ID filter"), + mcp.WithString("category", + mcp.Description("Category filter (name)"), ), mcp.WithString("since", mcp.Description("Filter by date (ISO 8601 timestamp)"), @@ -68,17 +118,17 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Decode params var params struct { - Owner string - Repo string - CategoryID string - Since string - Sort string - Direction string - First int32 - Last int32 - After string - Before string - Answered bool + Owner string + Repo string + Category string + Since string + Sort string + Direction string + First int32 + Last int32 + After string + Before string + Answered bool } if err := mapstructure.Decode(request.Params.Arguments, ¶ms); err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -116,11 +166,19 @@ func ListDiscussions(getGQLClient GetGQLClientFn, t translations.TranslationHelp } `graphql:"discussions(categoryId: $categoryId, orderBy: {field: $sort, direction: $direction}, first: $first, after: $after, last: $last, before: $before, answered: $answered)"` } `graphql:"repository(owner: $owner, name: $repo)"` } + categories, err := GetAllDiscussionCategories(ctx, client, params.Owner, params.Repo) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to get discussion categories: %v", err)), nil + } + var categoryID githubv4.ID = categories[params.Category] + if categoryID == "" && params.Category != "" { + return mcp.NewToolResultError(fmt.Sprintf("category '%s' not found", params.Category)), nil + } // Build query variables vars := map[string]interface{}{ "owner": githubv4.String(params.Owner), "repo": githubv4.String(params.Repo), - "categoryId": githubv4.ID(params.CategoryID), + "categoryId": categoryID, "sort": githubv4.DiscussionOrderField(params.Sort), "direction": githubv4.OrderDirection(params.Direction), "first": githubv4.Int(params.First), diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 18fdce6e..e8e6b675 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -42,7 +42,48 @@ func Test_ListDiscussions(t *testing.T) { assert.Contains(t, toolDef.InputSchema.Properties, "repo") assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"}) - // GraphQL query struct + // mock for the call to list all categories: query struct, variables, response + var q_cat struct { + Repository struct { + DiscussionCategories struct { + Nodes []struct { + ID githubv4.ID + Name githubv4.String + } + PageInfo struct { + HasNextPage githubv4.Boolean + EndCursor githubv4.String + } + } `graphql:"discussionCategories(first: 100, after: $after)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + vars_cat := map[string]interface{}{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "after": githubv4.String(""), + } + + vars_cat_invalid := map[string]interface{}{ + "owner": githubv4.String("invalid"), + "repo": githubv4.String("repo"), + "after": githubv4.String(""), + } + + mockResp_cat := githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "discussionCategories": map[string]any{ + "nodes": []map[string]any{ + {"id": "123", "name": "CategoryOne"}, + {"id": "456", "name": "CategoryTwo"}, + }, + }, + }, + }) + + mockResp_cat_invalid := githubv4mock.ErrorResponse("repository not found") + + // mock for the call to ListDiscussions: query struct, variables, response var q struct { Repository struct { Discussions struct { @@ -88,7 +129,7 @@ func Test_ListDiscussions(t *testing.T) { varsListWithCategory := map[string]interface{}{ "owner": githubv4.String("owner"), "repo": githubv4.String("repo"), - "categoryId": githubv4.ID("12345"), + "categoryId": githubv4.ID("123"), "sort": githubv4.DiscussionOrderField(""), "direction": githubv4.OrderDirection(""), "first": githubv4.Int(0), @@ -98,6 +139,9 @@ func Test_ListDiscussions(t *testing.T) { "answered": githubv4.Boolean(false), } + catMatcher := githubv4mock.NewQueryMatcher(q_cat, vars_cat, mockResp_cat) + catMatcherInvalid := githubv4mock.NewQueryMatcher(q_cat, vars_cat_invalid, mockResp_cat_invalid) + tests := []struct { name string vars map[string]interface{} @@ -106,6 +150,7 @@ func Test_ListDiscussions(t *testing.T) { expectError bool expectedIds []int64 errContains string + catMatcher githubv4mock.Matcher }{ { name: "list all discussions", @@ -117,6 +162,7 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, expectError: false, expectedIds: []int64{1, 2, 3}, + catMatcher: catMatcher, }, { name: "invalid owner or repo", @@ -128,18 +174,20 @@ func Test_ListDiscussions(t *testing.T) { response: mockErrorRepoNotFound, expectError: true, errContains: "repository not found", + catMatcher: catMatcherInvalid, }, { name: "list discussions with category", vars: varsListWithCategory, reqParams: map[string]interface{}{ - "owner": "owner", - "repo": "repo", - "categoryId": "12345", + "owner": "owner", + "repo": "repo", + "category": "CategoryOne", // This should match the ID "123" in the mock response }, response: mockResponseCategory, expectError: false, expectedIds: []int64{1}, + catMatcher: catMatcher, }, { name: "list discussions with since date", @@ -152,6 +200,7 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, expectError: false, expectedIds: []int64{2, 3}, + catMatcher: catMatcher, }, { name: "both first and last parameters provided", @@ -165,6 +214,7 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call expectError: true, errContains: "only one of 'first' or 'last' may be specified", + catMatcher: catMatcher, }, { name: "after with last parameters provided", @@ -178,6 +228,7 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call expectError: true, errContains: "'after' cannot be used with 'last'. Did you mean to use 'before' instead?", + catMatcher: catMatcher, }, { name: "before with first parameters provided", @@ -191,6 +242,7 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call expectError: true, errContains: "'before' cannot be used with 'first'. Did you mean to use 'after' instead?", + catMatcher: catMatcher, }, { name: "both after and before parameters provided", @@ -204,13 +256,14 @@ func Test_ListDiscussions(t *testing.T) { response: mockResponseListAll, // response doesn't matter since error occurs before GraphQL call expectError: true, errContains: "only one of 'after' or 'before' may be specified", + catMatcher: catMatcher, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { matcher := githubv4mock.NewQueryMatcher(q, tc.vars, tc.response) - httpClient := githubv4mock.NewMockedHTTPClient(matcher) + httpClient := githubv4mock.NewMockedHTTPClient(matcher, tc.catMatcher) gqlClient := githubv4.NewClient(httpClient) _, handler := ListDiscussions(stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) @@ -416,10 +469,10 @@ func Test_ListDiscussionCategories(t *testing.T) { vars := map[string]interface{}{ "owner": githubv4.String("owner"), "repo": githubv4.String("repo"), - "first": githubv4.Int(0), - "last": githubv4.Int(0), - "after": githubv4.String(""), - "before": githubv4.String(""), + "first": githubv4.Int(0), // Default to 100 categories + "last": githubv4.Int(0), // Not used, but required by schema + "after": githubv4.String(""), // Not used, but required by schema + "before": githubv4.String(""), // Not used, but required by schema } mockResp := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ From a77b2f4bcb2bc2b0e2928afe58f4cf888d1217d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Thu, 5 Jun 2025 09:39:39 -0700 Subject: [PATCH 10/11] Fix linting errors --- pkg/github/discussions_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index e8e6b675..89296012 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -43,7 +43,7 @@ func Test_ListDiscussions(t *testing.T) { assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo"}) // mock for the call to list all categories: query struct, variables, response - var q_cat struct { + var qCat struct { Repository struct { DiscussionCategories struct { Nodes []struct { @@ -58,19 +58,19 @@ func Test_ListDiscussions(t *testing.T) { } `graphql:"repository(owner: $owner, name: $repo)"` } - vars_cat := map[string]interface{}{ + varsCat := map[string]interface{}{ "owner": githubv4.String("owner"), "repo": githubv4.String("repo"), "after": githubv4.String(""), } - vars_cat_invalid := map[string]interface{}{ + varsCatInvalid := map[string]interface{}{ "owner": githubv4.String("invalid"), "repo": githubv4.String("repo"), "after": githubv4.String(""), } - mockResp_cat := githubv4mock.DataResponse(map[string]any{ + mockRespCat := githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ "discussionCategories": map[string]any{ "nodes": []map[string]any{ @@ -81,7 +81,7 @@ func Test_ListDiscussions(t *testing.T) { }, }) - mockResp_cat_invalid := githubv4mock.ErrorResponse("repository not found") + mockRespCatInvalid := githubv4mock.ErrorResponse("repository not found") // mock for the call to ListDiscussions: query struct, variables, response var q struct { @@ -139,8 +139,8 @@ func Test_ListDiscussions(t *testing.T) { "answered": githubv4.Boolean(false), } - catMatcher := githubv4mock.NewQueryMatcher(q_cat, vars_cat, mockResp_cat) - catMatcherInvalid := githubv4mock.NewQueryMatcher(q_cat, vars_cat_invalid, mockResp_cat_invalid) + catMatcher := githubv4mock.NewQueryMatcher(qCat, varsCat, mockRespCat) + catMatcherInvalid := githubv4mock.NewQueryMatcher(qCat, varsCatInvalid, mockRespCatInvalid) tests := []struct { name string From 4d26e31336a9d29ff108579e02040a773dbcd7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Ren=C3=A9-Corail?= Date: Thu, 5 Jun 2025 09:45:51 -0700 Subject: [PATCH 11/11] Bump go-github to v72 --- pkg/github/discussions.go | 2 +- pkg/github/discussions_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 9a9b9b52..54ae6d04 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v72/github" "github.com/mark3labs/mcp-go/mcp" "github.com/mark3labs/mcp-go/server" "github.com/shurcooL/githubv4" diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index 89296012..8b10d0c9 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -8,7 +8,7 @@ import ( "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/pkg/translations" - "github.com/google/go-github/v69/github" + "github.com/google/go-github/v72/github" "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require"