Skip to content

Commit 2f64ac0

Browse files
Higangsshtommaso-moro
Higangssh
and
tommaso-moro
authored
Add discussion metadata fields to get_discussion tool (#1305)
* 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. * Fix Discussion field mappings based on GitHub GraphQL API Changes: - Replace 'State' (doesn't exist) with 'Closed' (Boolean) - Remove 'AnsweredAt' (doesn't exist) - Keep 'IsAnswered' (verified to exist in GitHub GraphQL API) - Use 'AnswerChosenAt' for answer timestamp Updated both implementation and tests to match actual GitHub GraphQL schema. All tests passing. --------- Co-authored-by: tommaso-moro <[email protected]>
1 parent 88a594e commit 2f64ac0

File tree

2 files changed

+136
-100
lines changed

2 files changed

+136
-100
lines changed

‎pkg/github/discussions.go‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ type DiscussionFragment struct {
4444
}
4545

4646
type NodeFragment struct {
47-
Number githubv4.Int
48-
Title githubv4.String
49-
CreatedAt githubv4.DateTime
50-
UpdatedAt githubv4.DateTime
51-
Author struct {
47+
Number githubv4.Int
48+
Title githubv4.String
49+
CreatedAt githubv4.DateTime
50+
UpdatedAt githubv4.DateTime
51+
Closed githubv4.Boolean
52+
IsAnswered githubv4.Boolean
53+
AnswerChosenAt *githubv4.DateTime
54+
Author struct {
5255
Login githubv4.String
5356
}
5457
Category struct {
@@ -294,12 +297,15 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper
294297
var q struct {
295298
Repository struct {
296299
Discussion struct {
297-
Number githubv4.Int
298-
Title githubv4.String
299-
Body githubv4.String
300-
CreatedAt githubv4.DateTime
301-
URL githubv4.String `graphql:"url"`
302-
Category struct {
300+
Number githubv4.Int
301+
Title githubv4.String
302+
Body githubv4.String
303+
CreatedAt githubv4.DateTime
304+
Closed githubv4.Boolean
305+
IsAnswered githubv4.Boolean
306+
AnswerChosenAt *githubv4.DateTime
307+
URL githubv4.String `graphql:"url"`
308+
Category struct {
303309
Name githubv4.String
304310
} `graphql:"category"`
305311
} `graphql:"discussion(number: $discussionNumber)"`
@@ -314,17 +320,30 @@ func GetDiscussion(getGQLClient GetGQLClientFn, t translations.TranslationHelper
314320
return mcp.NewToolResultError(err.Error()), nil
315321
}
316322
d := q.Repository.Discussion
317-
discussion := &github.Discussion{
318-
Number: github.Ptr(int(d.Number)),
319-
Title: github.Ptr(string(d.Title)),
320-
Body: github.Ptr(string(d.Body)),
321-
HTMLURL: github.Ptr(string(d.URL)),
322-
CreatedAt: &github.Timestamp{Time: d.CreatedAt.Time},
323-
DiscussionCategory: &github.DiscussionCategory{
324-
Name: github.Ptr(string(d.Category.Name)),
323+
324+
// Build response as map to include fields not present in go-github's Discussion struct.
325+
// The go-github library's Discussion type lacks isAnswered and answerChosenAt fields,
326+
// so we use map[string]interface{} for the response (consistent with other functions
327+
// like ListDiscussions and GetDiscussionComments).
328+
response := map[string]interface{}{
329+
"number": int(d.Number),
330+
"title": string(d.Title),
331+
"body": string(d.Body),
332+
"url": string(d.URL),
333+
"closed": bool(d.Closed),
334+
"isAnswered": bool(d.IsAnswered),
335+
"createdAt": d.CreatedAt.Time,
336+
"category": map[string]interface{}{
337+
"name": string(d.Category.Name),
325338
},
326339
}
327-
out, err := json.Marshal(discussion)
340+
341+
// Add optional timestamp fields if present
342+
if d.AnswerChosenAt != nil {
343+
response["answerChosenAt"] = d.AnswerChosenAt.Time
344+
}
345+
346+
out, err := json.Marshal(response)
328347
if err != nil {
329348
return nil, fmt.Errorf("failed to marshal discussion: %w", err)
330349
}

‎pkg/github/discussions_test.go‎

Lines changed: 97 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"encoding/json"
66
"net/http"
77
"testing"
8-
"time"
98

109
"github.com/github/github-mcp-server/internal/githubv4mock"
1110
"github.com/github/github-mcp-server/pkg/translations"
@@ -17,75 +16,89 @@ import (
1716

1817
var (
1918
discussionsGeneral = []map[string]any{
20-
{"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"}},
21-
{"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"}},
19+
{"number": 1, "title": "Discussion 1 title", "createdAt": "2023-01-01T00:00:00Z", "updatedAt": "2023-01-01T00:00:00Z", "closed": false, "isAnswered": false, "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/1", "category": map[string]any{"name": "General"}},
20+
{"number": 3, "title": "Discussion 3 title", "createdAt": "2023-03-01T00:00:00Z", "updatedAt": "2023-02-01T00:00:00Z", "closed": false, "isAnswered": false, "author": map[string]any{"login": "user1"}, "url": "https://github.com/owner/repo/discussions/3", "category": map[string]any{"name": "General"}},
2221
}
2322
discussionsAll = []map[string]any{
2423
{
25-
"number": 1,
26-
"title": "Discussion 1 title",
27-
"createdAt": "2023-01-01T00:00:00Z",
28-
"updatedAt": "2023-01-01T00:00:00Z",
29-
"author": map[string]any{"login": "user1"},
30-
"url": "https://github.com/owner/repo/discussions/1",
31-
"category": map[string]any{"name": "General"},
24+
"number": 1,
25+
"title": "Discussion 1 title",
26+
"createdAt": "2023-01-01T00:00:00Z",
27+
"updatedAt": "2023-01-01T00:00:00Z",
28+
"closed": false,
29+
"isAnswered": false,
30+
"author": map[string]any{"login": "user1"},
31+
"url": "https://github.com/owner/repo/discussions/1",
32+
"category": map[string]any{"name": "General"},
3233
},
3334
{
34-
"number": 2,
35-
"title": "Discussion 2 title",
36-
"createdAt": "2023-02-01T00:00:00Z",
37-
"updatedAt": "2023-02-01T00:00:00Z",
38-
"author": map[string]any{"login": "user2"},
39-
"url": "https://github.com/owner/repo/discussions/2",
40-
"category": map[string]any{"name": "Questions"},
35+
"number": 2,
36+
"title": "Discussion 2 title",
37+
"createdAt": "2023-02-01T00:00:00Z",
38+
"updatedAt": "2023-02-01T00:00:00Z",
39+
"closed": false,
40+
"isAnswered": false,
41+
"author": map[string]any{"login": "user2"},
42+
"url": "https://github.com/owner/repo/discussions/2",
43+
"category": map[string]any{"name": "Questions"},
4144
},
4245
{
43-
"number": 3,
44-
"title": "Discussion 3 title",
45-
"createdAt": "2023-03-01T00:00:00Z",
46-
"updatedAt": "2023-03-01T00:00:00Z",
47-
"author": map[string]any{"login": "user3"},
48-
"url": "https://github.com/owner/repo/discussions/3",
49-
"category": map[string]any{"name": "General"},
46+
"number": 3,
47+
"title": "Discussion 3 title",
48+
"createdAt": "2023-03-01T00:00:00Z",
49+
"updatedAt": "2023-03-01T00:00:00Z",
50+
"closed": false,
51+
"isAnswered": false,
52+
"author": map[string]any{"login": "user3"},
53+
"url": "https://github.com/owner/repo/discussions/3",
54+
"category": map[string]any{"name": "General"},
5055
},
5156
}
5257

5358
discussionsOrgLevel = []map[string]any{
5459
{
55-
"number": 1,
56-
"title": "Org Discussion 1 - Community Guidelines",
57-
"createdAt": "2023-01-15T00:00:00Z",
58-
"updatedAt": "2023-01-15T00:00:00Z",
59-
"author": map[string]any{"login": "org-admin"},
60-
"url": "https://github.com/owner/.github/discussions/1",
61-
"category": map[string]any{"name": "Announcements"},
60+
"number": 1,
61+
"title": "Org Discussion 1 - Community Guidelines",
62+
"createdAt": "2023-01-15T00:00:00Z",
63+
"updatedAt": "2023-01-15T00:00:00Z",
64+
"closed": false,
65+
"isAnswered": false,
66+
"author": map[string]any{"login": "org-admin"},
67+
"url": "https://github.com/owner/.github/discussions/1",
68+
"category": map[string]any{"name": "Announcements"},
6269
},
6370
{
64-
"number": 2,
65-
"title": "Org Discussion 2 - Roadmap 2023",
66-
"createdAt": "2023-02-20T00:00:00Z",
67-
"updatedAt": "2023-02-20T00:00:00Z",
68-
"author": map[string]any{"login": "org-admin"},
69-
"url": "https://github.com/owner/.github/discussions/2",
70-
"category": map[string]any{"name": "General"},
71+
"number": 2,
72+
"title": "Org Discussion 2 - Roadmap 2023",
73+
"createdAt": "2023-02-20T00:00:00Z",
74+
"updatedAt": "2023-02-20T00:00:00Z",
75+
"closed": false,
76+
"isAnswered": false,
77+
"author": map[string]any{"login": "org-admin"},
78+
"url": "https://github.com/owner/.github/discussions/2",
79+
"category": map[string]any{"name": "General"},
7180
},
7281
{
73-
"number": 3,
74-
"title": "Org Discussion 3 - Roadmap 2024",
75-
"createdAt": "2023-02-20T00:00:00Z",
76-
"updatedAt": "2023-02-20T00:00:00Z",
77-
"author": map[string]any{"login": "org-admin"},
78-
"url": "https://github.com/owner/.github/discussions/3",
79-
"category": map[string]any{"name": "General"},
82+
"number": 3,
83+
"title": "Org Discussion 3 - Roadmap 2024",
84+
"createdAt": "2023-02-20T00:00:00Z",
85+
"updatedAt": "2023-02-20T00:00:00Z",
86+
"closed": false,
87+
"isAnswered": false,
88+
"author": map[string]any{"login": "org-admin"},
89+
"url": "https://github.com/owner/.github/discussions/3",
90+
"category": map[string]any{"name": "General"},
8091
},
8192
{
82-
"number": 4,
83-
"title": "Org Discussion 4 - Roadmap 2025",
84-
"createdAt": "2023-02-20T00:00:00Z",
85-
"updatedAt": "2023-02-20T00:00:00Z",
86-
"author": map[string]any{"login": "org-admin"},
87-
"url": "https://github.com/owner/.github/discussions/4",
88-
"category": map[string]any{"name": "General"},
93+
"number": 4,
94+
"title": "Org Discussion 4 - Roadmap 2025",
95+
"createdAt": "2023-02-20T00:00:00Z",
96+
"updatedAt": "2023-02-20T00:00:00Z",
97+
"closed": false,
98+
"isAnswered": false,
99+
"author": map[string]any{"login": "org-admin"},
100+
"url": "https://github.com/owner/.github/discussions/4",
101+
"category": map[string]any{"name": "General"},
89102
},
90103
}
91104

@@ -388,10 +401,10 @@ func Test_ListDiscussions(t *testing.T) {
388401
}
389402

390403
// Define the actual query strings that match the implementation
391-
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}}}"
392-
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}}}"
393-
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}}}"
394-
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}}}"
404+
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,closed,isAnswered,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
405+
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,closed,isAnswered,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
406+
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,closed,isAnswered,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
407+
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,closed,isAnswered,answerChosenAt,author{login},category{name},url},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
395408

396409
for _, tc := range tests {
397410
t.Run(tc.name, func(t *testing.T) {
@@ -484,7 +497,7 @@ func Test_GetDiscussion(t *testing.T) {
484497
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner", "repo", "discussionNumber"})
485498

486499
// Use exact string query that matches implementation output
487-
qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,url,category{name}}}}"
500+
qGetDiscussion := "query($discussionNumber:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){discussion(number: $discussionNumber){number,title,body,createdAt,closed,isAnswered,answerChosenAt,url,category{name}}}}"
488501

489502
vars := map[string]interface{}{
490503
"owner": "owner",
@@ -495,31 +508,31 @@ func Test_GetDiscussion(t *testing.T) {
495508
name string
496509
response githubv4mock.GQLResponse
497510
expectError bool
498-
expected *github.Discussion
511+
expected map[string]interface{}
499512
errContains string
500513
}{
501514
{
502515
name: "successful retrieval",
503516
response: githubv4mock.DataResponse(map[string]any{
504517
"repository": map[string]any{"discussion": map[string]any{
505-
"number": 1,
506-
"title": "Test Discussion Title",
507-
"body": "This is a test discussion",
508-
"url": "https://github.com/owner/repo/discussions/1",
509-
"createdAt": "2025-04-25T12:00:00Z",
510-
"category": map[string]any{"name": "General"},
518+
"number": 1,
519+
"title": "Test Discussion Title",
520+
"body": "This is a test discussion",
521+
"url": "https://github.com/owner/repo/discussions/1",
522+
"createdAt": "2025-04-25T12:00:00Z",
523+
"closed": false,
524+
"isAnswered": false,
525+
"category": map[string]any{"name": "General"},
511526
}},
512527
}),
513528
expectError: false,
514-
expected: &github.Discussion{
515-
HTMLURL: github.Ptr("https://github.com/owner/repo/discussions/1"),
516-
Number: github.Ptr(1),
517-
Title: github.Ptr("Test Discussion Title"),
518-
Body: github.Ptr("This is a test discussion"),
519-
CreatedAt: &github.Timestamp{Time: time.Date(2025, 4, 25, 12, 0, 0, 0, time.UTC)},
520-
DiscussionCategory: &github.DiscussionCategory{
521-
Name: github.Ptr("General"),
522-
},
529+
expected: map[string]interface{}{
530+
"number": float64(1),
531+
"title": "Test Discussion Title",
532+
"body": "This is a test discussion",
533+
"url": "https://github.com/owner/repo/discussions/1",
534+
"closed": false,
535+
"isAnswered": false,
523536
},
524537
},
525538
{
@@ -547,14 +560,18 @@ func Test_GetDiscussion(t *testing.T) {
547560
}
548561

549562
require.NoError(t, err)
550-
var out github.Discussion
563+
var out map[string]interface{}
551564
require.NoError(t, json.Unmarshal([]byte(text), &out))
552-
assert.Equal(t, *tc.expected.HTMLURL, *out.HTMLURL)
553-
assert.Equal(t, *tc.expected.Number, *out.Number)
554-
assert.Equal(t, *tc.expected.Title, *out.Title)
555-
assert.Equal(t, *tc.expected.Body, *out.Body)
556-
// Check category label
557-
assert.Equal(t, *tc.expected.DiscussionCategory.Name, *out.DiscussionCategory.Name)
565+
assert.Equal(t, tc.expected["number"], out["number"])
566+
assert.Equal(t, tc.expected["title"], out["title"])
567+
assert.Equal(t, tc.expected["body"], out["body"])
568+
assert.Equal(t, tc.expected["url"], out["url"])
569+
assert.Equal(t, tc.expected["closed"], out["closed"])
570+
assert.Equal(t, tc.expected["isAnswered"], out["isAnswered"])
571+
// Check category is present
572+
category, ok := out["category"].(map[string]interface{})
573+
require.True(t, ok)
574+
assert.Equal(t, "General", category["name"])
558575
})
559576
}
560577
}

0 commit comments

Comments
 (0)