Skip to content

Commit 9c35bd3

Browse files
feat(comments): add typed visibility tool inputs and outputs
Preserve legacy argument normalization and text responses while exposing protocol-gated structured results for six hide/unhide tools. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent dd50abe commit 9c35bd3

8 files changed

Lines changed: 552 additions & 79 deletions

‎pkg/github/__toolsnaps__/hide_issue_comment.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,24 @@
4444
],
4545
"type": "object"
4646
},
47-
"name": "hide_issue_comment"
47+
"name": "hide_issue_comment",
48+
"outputSchema": {
49+
"additionalProperties": false,
50+
"properties": {
51+
"is_minimized": {
52+
"type": "boolean"
53+
},
54+
"minimized_reason": {
55+
"type": "string"
56+
},
57+
"node_id": {
58+
"type": "string"
59+
}
60+
},
61+
"required": [
62+
"node_id",
63+
"is_minimized"
64+
],
65+
"type": "object"
66+
}
4867
}

‎pkg/github/__toolsnaps__/hide_pull_request_review.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,24 @@
5050
],
5151
"type": "object"
5252
},
53-
"name": "hide_pull_request_review"
53+
"name": "hide_pull_request_review",
54+
"outputSchema": {
55+
"additionalProperties": false,
56+
"properties": {
57+
"is_minimized": {
58+
"type": "boolean"
59+
},
60+
"minimized_reason": {
61+
"type": "string"
62+
},
63+
"node_id": {
64+
"type": "string"
65+
}
66+
},
67+
"required": [
68+
"node_id",
69+
"is_minimized"
70+
],
71+
"type": "object"
72+
}
5473
}

‎pkg/github/__toolsnaps__/hide_pull_request_review_comment.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,24 @@
4444
],
4545
"type": "object"
4646
},
47-
"name": "hide_pull_request_review_comment"
47+
"name": "hide_pull_request_review_comment",
48+
"outputSchema": {
49+
"additionalProperties": false,
50+
"properties": {
51+
"is_minimized": {
52+
"type": "boolean"
53+
},
54+
"minimized_reason": {
55+
"type": "string"
56+
},
57+
"node_id": {
58+
"type": "string"
59+
}
60+
},
61+
"required": [
62+
"node_id",
63+
"is_minimized"
64+
],
65+
"type": "object"
66+
}
4867
}

‎pkg/github/__toolsnaps__/unhide_issue_comment.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,24 @@
3030
],
3131
"type": "object"
3232
},
33-
"name": "unhide_issue_comment"
33+
"name": "unhide_issue_comment",
34+
"outputSchema": {
35+
"additionalProperties": false,
36+
"properties": {
37+
"is_minimized": {
38+
"type": "boolean"
39+
},
40+
"minimized_reason": {
41+
"type": "string"
42+
},
43+
"node_id": {
44+
"type": "string"
45+
}
46+
},
47+
"required": [
48+
"node_id",
49+
"is_minimized"
50+
],
51+
"type": "object"
52+
}
3453
}

‎pkg/github/__toolsnaps__/unhide_pull_request_review.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,24 @@
3636
],
3737
"type": "object"
3838
},
39-
"name": "unhide_pull_request_review"
39+
"name": "unhide_pull_request_review",
40+
"outputSchema": {
41+
"additionalProperties": false,
42+
"properties": {
43+
"is_minimized": {
44+
"type": "boolean"
45+
},
46+
"minimized_reason": {
47+
"type": "string"
48+
},
49+
"node_id": {
50+
"type": "string"
51+
}
52+
},
53+
"required": [
54+
"node_id",
55+
"is_minimized"
56+
],
57+
"type": "object"
58+
}
4059
}

‎pkg/github/__toolsnaps__/unhide_pull_request_review_comment.snap‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,24 @@
3030
],
3131
"type": "object"
3232
},
33-
"name": "unhide_pull_request_review_comment"
33+
"name": "unhide_pull_request_review_comment",
34+
"outputSchema": {
35+
"additionalProperties": false,
36+
"properties": {
37+
"is_minimized": {
38+
"type": "boolean"
39+
},
40+
"minimized_reason": {
41+
"type": "string"
42+
},
43+
"node_id": {
44+
"type": "string"
45+
}
46+
},
47+
"required": [
48+
"node_id",
49+
"is_minimized"
50+
],
51+
"type": "object"
52+
}
3453
}

‎pkg/github/comment_minimize.go‎

Lines changed: 96 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ type MinimizeCommentResult struct {
2626
MinimizedReason string `json:"minimized_reason,omitempty"`
2727
}
2828

29+
// CommentVisibilityInput identifies the comment or review whose visibility changes.
30+
// The factory's target-specific schema determines which identifiers are required.
31+
type CommentVisibilityInput struct {
32+
Owner string `json:"owner"`
33+
Repo string `json:"repo"`
34+
CommentID int64 `json:"comment_id,omitempty"`
35+
PullNumber int `json:"pullNumber,omitempty"`
36+
ReviewID int64 `json:"review_id,omitempty"`
37+
Classifier string `json:"classifier,omitempty"`
38+
}
39+
2940
var commentClassifiers = []any{"SPAM", "ABUSE", "OFF_TOPIC", "OUTDATED", "DUPLICATE", "RESOLVED", "LOW_QUALITY"}
3041

3142
const commentVisibilityPermissionNote = " Requires triage or write access to the repository, or being its author."
@@ -42,7 +53,7 @@ type commentVisibilityTarget struct {
4253
unhideDescription string
4354
properties func() map[string]*jsonschema.Schema
4455
required []string
45-
resolveNodeID func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult)
56+
resolveNodeID func(ctx context.Context, client *github.Client, input CommentVisibilityInput) (string, *mcp.CallToolResult)
4657
}
4758

4859
var issueCommentVisibilityTarget = commentVisibilityTarget{
@@ -62,12 +73,8 @@ var issueCommentVisibilityTarget = commentVisibilityTarget{
6273
}
6374
},
6475
required: []string{"comment_id"},
65-
resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) {
66-
commentID, err := requiredPositiveBigInt(args, "comment_id")
67-
if err != nil {
68-
return "", utils.NewToolResultError(err.Error())
69-
}
70-
comment, resp, err := client.Issues.GetComment(ctx, owner, repo, commentID)
76+
resolveNodeID: func(ctx context.Context, client *github.Client, input CommentVisibilityInput) (string, *mcp.CallToolResult) {
77+
comment, resp, err := client.Issues.GetComment(ctx, input.Owner, input.Repo, input.CommentID)
7178
return nodeIDFromResponse(ctx, "failed to get issue comment", comment.GetNodeID(), resp, err)
7279
},
7380
}
@@ -89,12 +96,8 @@ var pullRequestReviewCommentVisibilityTarget = commentVisibilityTarget{
8996
}
9097
},
9198
required: []string{"comment_id"},
92-
resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) {
93-
commentID, err := requiredPositiveBigInt(args, "comment_id")
94-
if err != nil {
95-
return "", utils.NewToolResultError(err.Error())
96-
}
97-
comment, resp, err := client.PullRequests.GetComment(ctx, owner, repo, commentID)
99+
resolveNodeID: func(ctx context.Context, client *github.Client, input CommentVisibilityInput) (string, *mcp.CallToolResult) {
100+
comment, resp, err := client.PullRequests.GetComment(ctx, input.Owner, input.Repo, input.CommentID)
98101
return nodeIDFromResponse(ctx, "failed to get pull request review comment", comment.GetNodeID(), resp, err)
99102
},
100103
}
@@ -121,19 +124,8 @@ var pullRequestReviewVisibilityTarget = commentVisibilityTarget{
121124
}
122125
},
123126
required: []string{"pullNumber", "review_id"},
124-
resolveNodeID: func(ctx context.Context, client *github.Client, owner, repo string, args map[string]any) (string, *mcp.CallToolResult) {
125-
pullNumber, err := RequiredInt(args, "pullNumber")
126-
if err != nil {
127-
return "", utils.NewToolResultError(err.Error())
128-
}
129-
if pullNumber < 1 {
130-
return "", utils.NewToolResultError("pullNumber must be greater than 0")
131-
}
132-
reviewID, err := requiredPositiveBigInt(args, "review_id")
133-
if err != nil {
134-
return "", utils.NewToolResultError(err.Error())
135-
}
136-
review, resp, err := client.PullRequests.GetReview(ctx, owner, repo, pullNumber, reviewID)
127+
resolveNodeID: func(ctx context.Context, client *github.Client, input CommentVisibilityInput) (string, *mcp.CallToolResult) {
128+
review, resp, err := client.PullRequests.GetReview(ctx, input.Owner, input.Repo, input.PullNumber, input.ReviewID)
137129
return nodeIDFromResponse(ctx, "failed to get pull request review", review.GetNodeID(), resp, err)
138130
},
139131
}
@@ -167,7 +159,11 @@ func commentVisibilityTool(t translations.TranslationHelperFunc, target commentV
167159
required = append(required, "classifier")
168160
}
169161

170-
st := NewTool(
162+
outputSchema, err := jsonschema.For[MinimizeCommentResult](nil)
163+
if err != nil {
164+
panic(fmt.Sprintf("failed to generate comment visibility output schema: %v", err))
165+
}
166+
st := NewTool[CommentVisibilityInput, MinimizeCommentResult](
171167
target.toolset,
172168
mcp.Tool{
173169
Name: name,
@@ -183,72 +179,121 @@ func commentVisibilityTool(t translations.TranslationHelperFunc, target commentV
183179
Properties: properties,
184180
Required: required,
185181
},
182+
OutputSchema: outputSchema,
186183
},
187184
scopes.RequireAll(scopes.Repo),
188-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
189-
return setCommentVisibility(ctx, deps, target, args, hide), nil, nil
185+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input CommentVisibilityInput) (*mcp.CallToolResult, MinimizeCommentResult, error) {
186+
result, output := setCommentVisibility(ctx, deps, target, input, hide)
187+
return result, output, nil
190188
},
189+
normalizeCommentVisibilityArguments(target, hide),
191190
)
192191
st.FeatureRule = target.featureRule
193192
return st
194193
}
195194

196-
// setCommentVisibility resolves the object identified by args and hides or unhides it.
197-
func setCommentVisibility(ctx context.Context, deps ToolDependencies, target commentVisibilityTarget, args map[string]any, hide bool) *mcp.CallToolResult {
198-
owner, err := RequiredParam[string](args, "owner")
195+
func normalizeCommentVisibilityArguments(target commentVisibilityTarget, hide bool) inventory.InputNormalizer {
196+
return func(raw json.RawMessage) (json.RawMessage, error) {
197+
var args map[string]any
198+
if err := json.Unmarshal(raw, &args); err != nil {
199+
return nil, err
200+
}
201+
if args == nil {
202+
return raw, nil
203+
}
204+
input, err := normalizeCommentVisibilityInput(args, target, hide)
205+
if err != nil {
206+
return nil, err
207+
}
208+
normalized, err := json.Marshal(input)
209+
if err != nil {
210+
return nil, fmt.Errorf("marshal normalized comment visibility arguments: %w", err)
211+
}
212+
return normalized, nil
213+
}
214+
}
215+
216+
func normalizeCommentVisibilityInput(args map[string]any, target commentVisibilityTarget, hide bool) (CommentVisibilityInput, error) {
217+
var input CommentVisibilityInput
218+
var err error
219+
input.Owner, err = RequiredParam[string](args, "owner")
199220
if err != nil {
200-
return utils.NewToolResultError(err.Error())
221+
return input, err
201222
}
202-
repo, err := RequiredParam[string](args, "repo")
223+
input.Repo, err = RequiredParam[string](args, "repo")
203224
if err != nil {
204-
return utils.NewToolResultError(err.Error())
225+
return input, err
205226
}
206-
var classifier string
207227
if hide {
208-
classifier, err = RequiredParam[string](args, "classifier")
228+
input.Classifier, err = RequiredParam[string](args, "classifier")
209229
if err != nil {
210-
return utils.NewToolResultError(err.Error())
230+
return input, err
211231
}
212-
classifier = strings.ToUpper(classifier)
213-
if !slices.Contains(commentClassifiers, any(classifier)) {
214-
return utils.NewToolResultError(fmt.Sprintf("invalid classifier %q: must be one of %v", classifier, commentClassifiers))
232+
input.Classifier = strings.ToUpper(input.Classifier)
233+
if !slices.Contains(commentClassifiers, any(input.Classifier)) {
234+
return input, fmt.Errorf("invalid classifier %q: must be one of %v", input.Classifier, commentClassifiers)
215235
}
216236
}
237+
for _, name := range target.required {
238+
if name == "pullNumber" {
239+
value, err := RequiredInt(args, name)
240+
if err != nil {
241+
return input, err
242+
}
243+
if value < 1 {
244+
return input, fmt.Errorf("pullNumber must be greater than 0")
245+
}
246+
input.PullNumber = value
247+
continue
248+
}
249+
value, err := requiredPositiveBigInt(args, name)
250+
if err != nil {
251+
return input, err
252+
}
253+
if name == "comment_id" {
254+
input.CommentID = value
255+
} else {
256+
input.ReviewID = value
257+
}
258+
}
259+
return input, nil
260+
}
217261

262+
// setCommentVisibility resolves the object identified by input and hides or unhides it.
263+
func setCommentVisibility(ctx context.Context, deps ToolDependencies, target commentVisibilityTarget, input CommentVisibilityInput, hide bool) (*mcp.CallToolResult, MinimizeCommentResult) {
218264
client, err := deps.GetClient(ctx)
219265
if err != nil {
220-
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err)
266+
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), MinimizeCommentResult{}
221267
}
222268

223-
nodeID, errResult := target.resolveNodeID(ctx, client, owner, repo, args)
269+
nodeID, errResult := target.resolveNodeID(ctx, client, input)
224270
if errResult != nil {
225-
return errResult
271+
return errResult, MinimizeCommentResult{}
226272
}
227273

228274
gqlClient, err := deps.GetGQLClient(ctx)
229275
if err != nil {
230-
return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err)
276+
return utils.NewToolResultErrorFromErr("failed to get GitHub GraphQL client", err), MinimizeCommentResult{}
231277
}
232278

233279
var result MinimizeCommentResult
234280
if hide {
235-
result, errResult = minimizeComment(ctx, gqlClient, nodeID, classifier)
281+
result, errResult = minimizeComment(ctx, gqlClient, nodeID, input.Classifier)
236282
} else {
237283
result, errResult = unminimizeComment(ctx, gqlClient, nodeID)
238284
}
239285
if errResult != nil {
240-
return errResult
286+
return errResult, MinimizeCommentResult{}
241287
}
242288

243289
r, err := json.Marshal(result)
244290
if err != nil {
245-
return utils.NewToolResultErrorFromErr("failed to marshal response", err)
291+
return utils.NewToolResultErrorFromErr("failed to marshal response", err), MinimizeCommentResult{}
246292
}
247-
return utils.NewToolResultText(string(r))
293+
return utils.NewToolResultText(string(r)), result
248294
}
249295

250-
// requiredPositiveBigInt reads a required ID argument. The schema's minimum is not enforced
251-
// when arguments are unmarshalled, so negative values are rejected here before any API call.
296+
// requiredPositiveBigInt validates legacy numeric IDs before SDK schema validation.
252297
func requiredPositiveBigInt(args map[string]any, p string) (int64, error) {
253298
v, err := RequiredBigInt(args, p)
254299
if err != nil {

0 commit comments

Comments
 (0)