Skip to content

Commit ed9ac2e

Browse files
SamMorrowDrumsCopilot
SamMorrowDrums
and
Copilot
committed
feat: type discussion and notification tool outputs
Migrate all eleven tools to typed registration and compact output DTOs. Preserve legacy text, parameter compatibility, pagination, and mutation validation while publishing honest modern output unions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 836f8a1 commit ed9ac2e

8 files changed

Lines changed: 1087 additions & 93 deletions

‎pkg/github/__toolsnaps__/discussion_comment_write.snap‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,42 @@
4646
],
4747
"type": "object"
4848
},
49-
"name": "discussion_comment_write"
49+
"name": "discussion_comment_write",
50+
"outputSchema": {
51+
"oneOf": [
52+
{
53+
"additionalProperties": false,
54+
"properties": {
55+
"id": {
56+
"type": "string"
57+
},
58+
"url": {
59+
"type": "string"
60+
}
61+
},
62+
"required": [
63+
"id",
64+
"url"
65+
],
66+
"type": "object"
67+
},
68+
{
69+
"additionalProperties": false,
70+
"properties": {
71+
"discussionID": {
72+
"type": "string"
73+
},
74+
"discussionURL": {
75+
"type": "string"
76+
}
77+
},
78+
"required": [
79+
"discussionID",
80+
"discussionURL"
81+
],
82+
"type": "object"
83+
}
84+
],
85+
"type": "object"
86+
}
5087
}

‎pkg/github/__toolsnaps__/manage_notification_subscription.snap‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,79 @@
2828
],
2929
"type": "object"
3030
},
31-
"name": "manage_notification_subscription"
31+
"name": "manage_notification_subscription",
32+
"outputSchema": {
33+
"oneOf": [
34+
{
35+
"additionalProperties": false,
36+
"properties": {
37+
"subscription": {
38+
"additionalProperties": false,
39+
"properties": {
40+
"created_at": {
41+
"type": [
42+
"null",
43+
"string"
44+
]
45+
},
46+
"ignored": {
47+
"type": [
48+
"null",
49+
"boolean"
50+
]
51+
},
52+
"reason": {
53+
"type": [
54+
"null",
55+
"string"
56+
]
57+
},
58+
"repository_url": {
59+
"type": [
60+
"null",
61+
"string"
62+
]
63+
},
64+
"subscribed": {
65+
"type": [
66+
"null",
67+
"boolean"
68+
]
69+
},
70+
"thread_url": {
71+
"type": [
72+
"null",
73+
"string"
74+
]
75+
},
76+
"url": {
77+
"type": [
78+
"null",
79+
"string"
80+
]
81+
}
82+
},
83+
"type": "object"
84+
}
85+
},
86+
"required": [
87+
"subscription"
88+
],
89+
"type": "object"
90+
},
91+
{
92+
"additionalProperties": false,
93+
"properties": {
94+
"message": {
95+
"type": "string"
96+
}
97+
},
98+
"required": [
99+
"message"
100+
],
101+
"type": "object"
102+
}
103+
],
104+
"type": "object"
105+
}
32106
}

‎pkg/github/__toolsnaps__/manage_repository_notification_subscription.snap‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,79 @@
3333
],
3434
"type": "object"
3535
},
36-
"name": "manage_repository_notification_subscription"
36+
"name": "manage_repository_notification_subscription",
37+
"outputSchema": {
38+
"oneOf": [
39+
{
40+
"additionalProperties": false,
41+
"properties": {
42+
"subscription": {
43+
"additionalProperties": false,
44+
"properties": {
45+
"created_at": {
46+
"type": [
47+
"null",
48+
"string"
49+
]
50+
},
51+
"ignored": {
52+
"type": [
53+
"null",
54+
"boolean"
55+
]
56+
},
57+
"reason": {
58+
"type": [
59+
"null",
60+
"string"
61+
]
62+
},
63+
"repository_url": {
64+
"type": [
65+
"null",
66+
"string"
67+
]
68+
},
69+
"subscribed": {
70+
"type": [
71+
"null",
72+
"boolean"
73+
]
74+
},
75+
"thread_url": {
76+
"type": [
77+
"null",
78+
"string"
79+
]
80+
},
81+
"url": {
82+
"type": [
83+
"null",
84+
"string"
85+
]
86+
}
87+
},
88+
"type": "object"
89+
}
90+
},
91+
"required": [
92+
"subscription"
93+
],
94+
"type": "object"
95+
},
96+
{
97+
"additionalProperties": false,
98+
"properties": {
99+
"message": {
100+
"type": "string"
101+
}
102+
},
103+
"required": [
104+
"message"
105+
],
106+
"type": "object"
107+
}
108+
],
109+
"type": "object"
110+
}
37111
}

‎pkg/github/discussion_notification_inputs.go‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
package github
2+
3+
import (
4+
"encoding/json"
5+
6+
"github.com/github/github-mcp-server/pkg/inventory"
7+
"github.com/go-viper/mapstructure/v2"
8+
)
9+
10+
type ListNotificationsInput struct {
11+
Filter *string `json:"filter,omitempty"`
12+
Since *string `json:"since,omitempty"`
13+
Before *string `json:"before,omitempty"`
14+
Owner *string `json:"owner,omitempty"`
15+
Repo *string `json:"repo,omitempty"`
16+
Page *int `json:"page,omitempty"`
17+
PerPage *int `json:"perPage,omitempty"`
18+
After *string `json:"after,omitempty"`
19+
}
20+
21+
type DismissNotificationInput struct {
22+
ThreadID string `json:"threadID"`
23+
State string `json:"state"`
24+
}
25+
26+
type MarkAllNotificationsReadInput struct {
27+
LastReadAt *string `json:"lastReadAt,omitempty"`
28+
Owner *string `json:"owner,omitempty"`
29+
Repo *string `json:"repo,omitempty"`
30+
}
31+
32+
type GetNotificationDetailsInput struct {
33+
NotificationID string `json:"notificationID"`
34+
}
35+
36+
type ManageNotificationSubscriptionInput struct {
37+
NotificationID string `json:"notificationID"`
38+
Action string `json:"action"`
39+
}
40+
41+
type ManageRepositoryNotificationSubscriptionInput struct {
42+
Owner string `json:"owner"`
43+
Repo string `json:"repo"`
44+
Action string `json:"action"`
45+
}
46+
47+
type ListDiscussionsInput struct {
48+
Owner string `json:"owner"`
49+
Repo *string `json:"repo,omitempty"`
50+
Category *string `json:"category,omitempty"`
51+
OrderBy *string `json:"orderBy,omitempty"`
52+
Direction *string `json:"direction,omitempty"`
53+
PerPage *int `json:"perPage,omitempty"`
54+
After *string `json:"after,omitempty"`
55+
}
56+
57+
type GetDiscussionInput struct {
58+
Owner string `json:"owner"`
59+
Repo string `json:"repo"`
60+
DiscussionNumber int32 `json:"discussionNumber"`
61+
}
62+
63+
type GetDiscussionCommentsInput struct {
64+
Owner string `json:"owner"`
65+
Repo string `json:"repo"`
66+
DiscussionNumber int32 `json:"discussionNumber"`
67+
IncludeReplies *bool `json:"includeReplies,omitempty"`
68+
PerPage *int `json:"perPage,omitempty"`
69+
After *string `json:"after,omitempty"`
70+
}
71+
72+
type DiscussionCommentWriteInput struct {
73+
Method string `json:"method"`
74+
Owner *string `json:"owner,omitempty"`
75+
Repo *string `json:"repo,omitempty"`
76+
DiscussionNumber *int `json:"discussionNumber,omitempty"`
77+
Body *string `json:"body,omitempty"`
78+
CommentNodeID *string `json:"commentNodeID,omitempty"`
79+
}
80+
81+
type ListDiscussionCategoriesInput struct {
82+
Owner string `json:"owner"`
83+
Repo *string `json:"repo,omitempty"`
84+
}
85+
86+
// Preserve presence and the existing parameter validators at the compatibility
87+
// boundary. In particular, an explicit perPage: 0 differs from an omitted value
88+
// for discussion comments, and mutation parameters are required per method.
89+
func discussionNotificationArguments(input any) (map[string]any, error) {
90+
raw, err := json.Marshal(input)
91+
if err != nil {
92+
return nil, err
93+
}
94+
var args map[string]any
95+
err = json.Unmarshal(raw, &args)
96+
return args, err
97+
}
98+
99+
func normalizeDiscussionReadArguments(raw json.RawMessage) (json.RawMessage, error) {
100+
var args map[string]any
101+
if err := json.Unmarshal(raw, &args); err != nil {
102+
return nil, err
103+
}
104+
// These two tools historically used WeakDecode: retain its case-insensitive
105+
// field matching, numeric-string conversion and fractional truncation.
106+
var input GetDiscussionInput
107+
if err := mapstructure.WeakDecode(args, &input); err != nil {
108+
return nil, err
109+
}
110+
args["owner"] = input.Owner
111+
args["repo"] = input.Repo
112+
args["discussionNumber"] = input.DiscussionNumber
113+
return json.Marshal(args)
114+
}
115+
116+
func normalizeDiscussionNotificationIntegers(fields ...string) inventory.InputNormalizer {
117+
return func(raw json.RawMessage) (json.RawMessage, error) {
118+
var args map[string]any
119+
if err := json.Unmarshal(raw, &args); err != nil {
120+
return nil, err
121+
}
122+
123+
for _, field := range fields {
124+
if _, ok := args[field]; !ok {
125+
continue
126+
}
127+
value, err := OptionalIntParam(args, field)
128+
if err != nil {
129+
return nil, err
130+
}
131+
if value == 0 {
132+
switch field {
133+
case "perPage":
134+
value = 30
135+
case "page":
136+
value = 1
137+
}
138+
}
139+
args[field] = value
140+
}
141+
return json.Marshal(args)
142+
}
143+
}
144+
145+
func normalizeDiscussionCommentWriteArguments(raw json.RawMessage) (json.RawMessage, error) {
146+
var args map[string]any
147+
if err := json.Unmarshal(raw, &args); err != nil {
148+
return nil, err
149+
}
150+
method, _ := args["method"].(string)
151+
used := map[string]bool{"method": true}
152+
switch method {
153+
case "add", "reply":
154+
used["owner"], used["repo"], used["discussionNumber"], used["body"] = true, true, true, true
155+
if method == "reply" {
156+
used["commentNodeID"] = true
157+
}
158+
case "update":
159+
used["commentNodeID"], used["body"] = true, true
160+
case "delete", "mark_answer", "unmark_answer":
161+
used["commentNodeID"] = true
162+
}
163+
// Unused method-specific parameters were never decoded by the legacy
164+
// handler, even when their values did not match the advertised schema.
165+
for _, field := range []string{"owner", "repo", "discussionNumber", "body", "commentNodeID"} {
166+
if !used[field] {
167+
delete(args, field)
168+
}
169+
}
170+
normalized, err := json.Marshal(args)
171+
if err != nil {
172+
return nil, err
173+
}
174+
return normalizeDiscussionNotificationIntegers("discussionNumber")(normalized)
175+
}

0 commit comments

Comments
 (0)