From 665ff491baca5710aea8238852175008cd709adf Mon Sep 17 00:00:00 2001 From: Sanghee Son Date: Sat, 25 Oct 2025 22:39:08 +0900 Subject: [PATCH] Add state metadata fields to get_discussion tool Fixes #1303 The get_discussion tool was missing important state metadata that's already available in get_issue. Added four fields to provide complete discussion status information: - state: Current discussion state (OPEN/CLOSED) - isAnswered: Whether the discussion has an accepted answer - answeredAt: Timestamp when answer was provided - answerChosenAt: Timestamp when answer was selected Changed GetDiscussion to return a map instead of github.Discussion struct since the go-github library doesn't include all these fields in its type definition. This approach is consistent with other functions in this codebase (ListDiscussions, GetDiscussionComments). All tests pass and linter checks pass. --- pkg/github/discussions.go | 64 ++++++++---- pkg/github/discussions_test.go | 177 ++++++++++++++++++--------------- 2 files changed, 141 insertions(+), 100 deletions(-) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index dc26063fd..81eb6e085 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -44,11 +44,15 @@ type DiscussionFragment struct { } type NodeFragment struct { - Number githubv4.Int - Title githubv4.String - CreatedAt githubv4.DateTime - UpdatedAt githubv4.DateTime - Author struct { + Number githubv4.Int + Title githubv4.String + CreatedAt githubv4.DateTime + UpdatedAt githubv4.DateTime + State githubv4.String + IsAnswered githubv4.Boolean + AnsweredAt *githubv4.DateTime + AnswerChosenAt *githubv4.DateTime + Author struct { Login githubv4.String } Category struct { @@ -294,12 +298,16 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper var q struct { Repository struct { Discussion struct { - Number githubv4.Int - Title githubv4.String - Body githubv4.String - CreatedAt githubv4.DateTime - URL githubv4.String `graphql:"url"` - Category struct { + Number githubv4.Int + Title githubv4.String + Body githubv4.String + CreatedAt githubv4.DateTime + State githubv4.String + IsAnswered githubv4.Boolean + AnsweredAt *githubv4.DateTime + AnswerChosenAt *githubv4.DateTime + URL githubv4.String `graphql:"url"` + Category struct { Name githubv4.String } `graphql:"category"` } `graphql:"discussion(number: $discussionNumber)"` @@ -314,17 +322,33 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper return mcp.NewToolResultError(err.Error()), nil } d := q.Repository.Discussion - discussion := &github.Discussion{ - Number: github.Ptr(int(d.Number)), - Title: github.Ptr(string(d.Title)), - Body: github.Ptr(string(d.Body)), - HTMLURL: github.Ptr(string(d.URL)), - CreatedAt: &github.Timestamp{Time: d.CreatedAt.Time}, - DiscussionCategory: &github.DiscussionCategory{ - Name: github.Ptr(string(d.Category.Name)), + + // Build response as map to include fields not present in go-github's Discussion struct. + // The go-github library's Discussion type lacks isAnswered and answeredAt fields, + // so we use map[string]interface{} for the response (consistent with other functions + // like ListDiscussions and GetDiscussionComments). + response := map[string]interface{}{ + "number": int(d.Number), + "title": string(d.Title), + "body": string(d.Body), + "url": string(d.URL), + "state": string(d.State), + "isAnswered": bool(d.IsAnswered), + "createdAt": d.CreatedAt.Time, + "category": map[string]interface{}{ + "name": string(d.Category.Name), }, } - out, err := json.Marshal(discussion) + + // Add optional timestamp fields if present + if d.AnsweredAt != nil { + response["answeredAt"] = d.AnsweredAt.Time + } + if d.AnswerChosenAt != nil { + response["answerChosenAt"] = d.AnswerChosenAt.Time + } + + out, err := json.Marshal(response) if err != nil { return nil, fmt.Errorf("failed to marshal discussion: %w", err) } diff --git a/pkg/github/discussions_test.go b/pkg/github/discussions_test.go index beef2effe..e48277015 100644 --- a/pkg/github/discussions_test.go +++ b/pkg/github/discussions_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "net/http" "testing" - "time" "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/pkg/translations" @@ -17,75 +16,89 @@ import ( var ( discussionsGeneral = []map[string]any{ - {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "updatedAt": "2023-01-01T00:00:00Z", "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}}, - {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "updatedAt": "2023-02-01T00:00:00Z", "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}}, + {"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "updatedAt": "2023-01-01T00:00:00Z", "state": "OPEN", "isAnswered": false, "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}}, + {"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "updatedAt": "2023-02-01T00:00:00Z", "state": "OPEN", "isAnswered": false, "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}}, } discussionsAll = []map[string]any{ { - "number": 1, - "title": "Discussion 1 title", - "createdAt": "2023-01-01T00:00:00Z", - "updatedAt": "2023-01-01T00:00:00Z", - "author": map[string]any{"login": "user1"}, - "url": "https://github.com/owner/repo/discussions/1", - "category": map[string]any{"name": "General"}, + "number": 1, + "title": "Discussion 1 title", + "createdAt": "2023-01-01T00:00:00Z", + "updatedAt": "2023-01-01T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "user1"}, + "url": "https://github.com/owner/repo/discussions/1", + "category": map[string]any{"name": "General"}, }, { - "number": 2, - "title": "Discussion 2 title", - "createdAt": "2023-02-01T00:00:00Z", - "updatedAt": "2023-02-01T00:00:00Z", - "author": map[string]any{"login": "user2"}, - "url": "https://github.com/owner/repo/discussions/2", - "category": map[string]any{"name": "Questions"}, + "number": 2, + "title": "Discussion 2 title", + "createdAt": "2023-02-01T00:00:00Z", + "updatedAt": "2023-02-01T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "user2"}, + "url": "https://github.com/owner/repo/discussions/2", + "category": map[string]any{"name": "Questions"}, }, { - "number": 3, - "title": "Discussion 3 title", - "createdAt": "2023-03-01T00:00:00Z", - "updatedAt": "2023-03-01T00:00:00Z", - "author": map[string]any{"login": "user3"}, - "url": "https://github.com/owner/repo/discussions/3", - "category": map[string]any{"name": "General"}, + "number": 3, + "title": "Discussion 3 title", + "createdAt": "2023-03-01T00:00:00Z", + "updatedAt": "2023-03-01T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "user3"}, + "url": "https://github.com/owner/repo/discussions/3", + "category": map[string]any{"name": "General"}, }, } discussionsOrgLevel = []map[string]any{ { - "number": 1, - "title": "Org Discussion 1 - Community Guidelines", - "createdAt": "2023-01-15T00:00:00Z", - "updatedAt": "2023-01-15T00:00:00Z", - "author": map[string]any{"login": "org-admin"}, - "url": "https://github.com/owner/.github/discussions/1", - "category": map[string]any{"name": "Announcements"}, + "number": 1, + "title": "Org Discussion 1 - Community Guidelines", + "createdAt": "2023-01-15T00:00:00Z", + "updatedAt": "2023-01-15T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/1", + "category": map[string]any{"name": "Announcements"}, }, { - "number": 2, - "title": "Org Discussion 2 - Roadmap 2023", - "createdAt": "2023-02-20T00:00:00Z", - "updatedAt": "2023-02-20T00:00:00Z", - "author": map[string]any{"login": "org-admin"}, - "url": "https://github.com/owner/.github/discussions/2", - "category": map[string]any{"name": "General"}, + "number": 2, + "title": "Org Discussion 2 - Roadmap 2023", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/2", + "category": map[string]any{"name": "General"}, }, { - "number": 3, - "title": "Org Discussion 3 - Roadmap 2024", - "createdAt": "2023-02-20T00:00:00Z", - "updatedAt": "2023-02-20T00:00:00Z", - "author": map[string]any{"login": "org-admin"}, - "url": "https://github.com/owner/.github/discussions/3", - "category": map[string]any{"name": "General"}, + "number": 3, + "title": "Org Discussion 3 - Roadmap 2024", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/3", + "category": map[string]any{"name": "General"}, }, { - "number": 4, - "title": "Org Discussion 4 - Roadmap 2025", - "createdAt": "2023-02-20T00:00:00Z", - "updatedAt": "2023-02-20T00:00:00Z", - "author": map[string]any{"login": "org-admin"}, - "url": "https://github.com/owner/.github/discussions/4", - "category": map[string]any{"name": "General"}, + "number": 4, + "title": "Org Discussion 4 - Roadmap 2025", + "createdAt": "2023-02-20T00:00:00Z", + "updatedAt": "2023-02-20T00:00:00Z", + "state": "OPEN", + "isAnswered": false, + "author": map[string]any{"login": "org-admin"}, + "url": "https://github.com/owner/.github/discussions/4", + "category": map[string]any{"name": "General"}, }, } @@ -388,10 +401,10 @@ func Test_ListDiscussions(t *testing.T) { } // Define the actual query strings that match the implementation - qBasicNoOrder := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" - qWithCategoryNoOrder := "query($after:String$categoryId:ID!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" - qBasicWithOrder := "query($after:String$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" - qWithCategoryAndOrder := "query($after:String$categoryId:ID!$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qBasicNoOrder := "query($after:String$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after){nodes{number,title,createdAt,updatedAt,state,isAnswered,answeredAt,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithCategoryNoOrder := "query($after:String$categoryId:ID!$first:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId){nodes{number,title,createdAt,updatedAt,state,isAnswered,answeredAt,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qBasicWithOrder := "query($after:String$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,state,isAnswered,answeredAt,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithCategoryAndOrder := "query($after:String$categoryId:ID!$first:Int!$orderByDirection:OrderDirection!$orderByField:DiscussionOrderField!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussions(first: $first, after: $after, categoryId: $categoryId, orderBy: { field: $orderByField, direction: $orderByDirection }){nodes{number,title,createdAt,updatedAt,state,isAnswered,answeredAt,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -484,7 +497,7 @@ func Test_GetDiscussion(t *testing.T) { assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"}) // Use exact string query that matches implementation output - qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,url,category{name}}}}" + qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,state,isAnswered,answeredAt,answerChosenAt,url,category{name}}}}" vars := map[string]interface{}{ "owner": "owner", @@ -495,31 +508,31 @@ func Test_GetDiscussion(t *testing.T) { name string response githubv4mock.GQLResponse expectError bool - expected *github.Discussion + expected map[string]interface{} errContains string }{ { name: "successful retrieval", response: githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{"discussion": map[string]any{ - "number": 1, - "title": "Test Discussion Title", - "body": "This is a test discussion", - "url": "https://github.com/owner/repo/discussions/1", - "createdAt": "2025-04-25T12:00:00Z", - "category": map[string]any{"name": "General"}, + "number": 1, + "title": "Test Discussion Title", + "body": "This is a test discussion", + "url": "https://github.com/owner/repo/discussions/1", + "createdAt": "2025-04-25T12:00:00Z", + "state": "OPEN", + "isAnswered": false, + "category": map[string]any{"name": "General"}, }}, }), expectError: false, - expected: &github.Discussion{ - HTMLURL: github.Ptr("https://github.com/owner/repo/discussions/1"), - Number: github.Ptr(1), - Title: github.Ptr("Test Discussion Title"), - Body: github.Ptr("This is a test discussion"), - CreatedAt: &github.Timestamp{Time: time.Date(2025, 4, 25, 12, 0, 0, 0, time.UTC)}, - DiscussionCategory: &github.DiscussionCategory{ - Name: github.Ptr("General"), - }, + expected: map[string]interface{}{ + "number": float64(1), + "title": "Test Discussion Title", + "body": "This is a test discussion", + "url": "https://github.com/owner/repo/discussions/1", + "state": "OPEN", + "isAnswered": false, }, }, { @@ -547,14 +560,18 @@ func Test_GetDiscussion(t *testing.T) { } require.NoError(t, err) - var out github.Discussion + var out map[string]interface{} require.NoError(t, json.Unmarshal([]byte(text), &out)) - assert.Equal(t, *tc.expected.HTMLURL, *out.HTMLURL) - assert.Equal(t, *tc.expected.Number, *out.Number) - assert.Equal(t, *tc.expected.Title, *out.Title) - assert.Equal(t, *tc.expected.Body, *out.Body) - // Check category label - assert.Equal(t, *tc.expected.DiscussionCategory.Name, *out.DiscussionCategory.Name) + assert.Equal(t, tc.expected["number"], out["number"]) + assert.Equal(t, tc.expected["title"], out["title"]) + assert.Equal(t, tc.expected["body"], out["body"]) + assert.Equal(t, tc.expected["url"], out["url"]) + assert.Equal(t, tc.expected["state"], out["state"]) + assert.Equal(t, tc.expected["isAnswered"], out["isAnswered"]) + // Check category is present + category, ok := out["category"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "General", category["name"]) }) } }