Skip to content

Commit 49abf15

Browse files
kelsey-myerskelsey-myers
authored
Add is_suggestion + rationale to update_issue_assignees (#2821)
* Add is_suggestion + rationale to update_issue_assignees * Align assignee confidence to uppercase LOW/MEDIUM/HIGH; add non-string/object test
1 parent e5ad9de commit 49abf15

4 files changed

Lines changed: 391 additions & 20 deletions

File tree

‎docs/feature-flags.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ runtime behavior (such as output formatting) won't appear here.
150150

151151
- **update_issue_assignees** - Update Issue Assignees
152152
- **Required OAuth Scopes**: `repo`
153-
- `assignees`: GitHub usernames to assign to this issue (string[], required)
153+
- `assignees`: GitHub usernames to assign to this issue. ([], required)
154154
- `issue_number`: The issue number to update (number, required)
155155
- `owner`: Repository owner (username or organization) (string, required)
156156
- `repo`: Repository name (string, required)

‎pkg/github/__toolsnaps__/update_issue_assignees.snap‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,48 @@
66
"readOnlyHint": false,
77
"title": "Update Issue Assignees"
88
},
9-
"description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list.",
9+
"description": "Update the assignees of an existing issue. This replaces the current assignees with the provided list. When setting values, include a confidence level (LOW, MEDIUM, or HIGH) reflecting how certain you are about the choice.",
1010
"inputSchema": {
1111
"properties": {
1212
"assignees": {
13-
"description": "GitHub usernames to assign to this issue",
13+
"description": "GitHub usernames to assign to this issue.",
1414
"items": {
15-
"type": "string"
15+
"oneOf": [
16+
{
17+
"description": "GitHub username",
18+
"type": "string"
19+
},
20+
{
21+
"properties": {
22+
"confidence": {
23+
"description": "How confident you are in this choice. Use 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal.",
24+
"enum": [
25+
"LOW",
26+
"MEDIUM",
27+
"HIGH"
28+
],
29+
"type": "string"
30+
},
31+
"is_suggestion": {
32+
"description": "If true, this assignee is sent to the API as a suggestion (suggest:true) rather than an applied assignee. Whether the assignee is applied or recorded as a proposal is determined by the API.",
33+
"type": "boolean"
34+
},
35+
"login": {
36+
"description": "GitHub username",
37+
"type": "string"
38+
},
39+
"rationale": {
40+
"description": "One concise sentence explaining what specifically about the issue led you to choose this assignee. State the concrete signal (e.g. 'Authored the file the crash originates in').",
41+
"maxLength": 280,
42+
"type": "string"
43+
}
44+
},
45+
"required": [
46+
"login"
47+
],
48+
"type": "object"
49+
}
50+
]
1651
},
1752
"type": "array"
1853
},

‎pkg/github/granular_tools_test.go‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,167 @@ func TestGranularUpdateIssueAssignees(t *testing.T) {
271271
assert.False(t, result.IsError)
272272
}
273273

274+
func TestGranularUpdateIssueAssigneesObjectForm(t *testing.T) {
275+
tests := []struct {
276+
name string
277+
requestArgs map[string]any
278+
expectedReq map[string]any
279+
}{
280+
{
281+
name: "assignee objects without intent serialize as strings",
282+
requestArgs: map[string]any{
283+
"owner": "owner",
284+
"repo": "repo",
285+
"issue_number": float64(1),
286+
"assignees": []any{
287+
map[string]any{"login": "octocat"},
288+
"monalisa",
289+
},
290+
},
291+
expectedReq: map[string]any{
292+
"assignees": []any{"octocat", "monalisa"},
293+
},
294+
},
295+
{
296+
name: "assignee suggested without rationale",
297+
requestArgs: map[string]any{
298+
"owner": "owner",
299+
"repo": "repo",
300+
"issue_number": float64(1),
301+
"assignees": []any{
302+
map[string]any{"login": "octocat", "is_suggestion": true},
303+
},
304+
},
305+
expectedReq: map[string]any{
306+
"assignees": []any{
307+
map[string]any{"login": "octocat", "suggest": true},
308+
},
309+
},
310+
},
311+
{
312+
name: "suggested assignee with rationale and confidence",
313+
requestArgs: map[string]any{
314+
"owner": "owner",
315+
"repo": "repo",
316+
"issue_number": float64(1),
317+
"assignees": []any{
318+
map[string]any{"login": "octocat", "rationale": " Authored the crashing file ", "confidence": "high", "is_suggestion": true},
319+
},
320+
},
321+
expectedReq: map[string]any{
322+
"assignees": []any{
323+
map[string]any{"login": "octocat", "rationale": "Authored the crashing file", "confidence": "HIGH", "suggest": true},
324+
},
325+
},
326+
},
327+
{
328+
name: "mix of plain and suggested assignees",
329+
requestArgs: map[string]any{
330+
"owner": "owner",
331+
"repo": "repo",
332+
"issue_number": float64(1),
333+
"assignees": []any{
334+
"monalisa",
335+
map[string]any{"login": "octocat", "is_suggestion": true},
336+
},
337+
},
338+
expectedReq: map[string]any{
339+
"assignees": []any{
340+
"monalisa",
341+
map[string]any{"login": "octocat", "suggest": true},
342+
},
343+
},
344+
},
345+
}
346+
347+
for _, tc := range tests {
348+
t.Run(tc.name, func(t *testing.T) {
349+
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
350+
PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq).
351+
andThen(mockResponse(t, http.StatusOK, &gogithub.Issue{Number: gogithub.Ptr(1)})),
352+
}))
353+
deps := BaseDeps{Client: client}
354+
serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper)
355+
handler := serverTool.Handler(deps)
356+
357+
request := createMCPRequest(tc.requestArgs)
358+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
359+
require.NoError(t, err)
360+
assert.False(t, result.IsError)
361+
})
362+
}
363+
}
364+
365+
func TestGranularUpdateIssueAssigneesInvalidInput(t *testing.T) {
366+
tests := []struct {
367+
name string
368+
requestArgs map[string]any
369+
expectedErrText string
370+
}{
371+
{
372+
name: "rationale too long",
373+
requestArgs: map[string]any{
374+
"owner": "owner",
375+
"repo": "repo",
376+
"issue_number": float64(1),
377+
"assignees": []any{
378+
map[string]any{"login": "octocat", "rationale": strings.Repeat("a", 281)},
379+
},
380+
},
381+
expectedErrText: "assignee rationale must be 280 characters or less",
382+
},
383+
{
384+
name: "assignee object missing login",
385+
requestArgs: map[string]any{
386+
"owner": "owner",
387+
"repo": "repo",
388+
"issue_number": float64(1),
389+
"assignees": []any{
390+
map[string]any{"rationale": "no login provided"},
391+
},
392+
},
393+
expectedErrText: "each assignee object must have a 'login' string",
394+
},
395+
{
396+
name: "invalid confidence value",
397+
requestArgs: map[string]any{
398+
"owner": "owner",
399+
"repo": "repo",
400+
"issue_number": float64(1),
401+
"assignees": []any{
402+
map[string]any{"login": "octocat", "confidence": "maybe"},
403+
},
404+
},
405+
expectedErrText: "confidence must be one of: LOW, MEDIUM, HIGH",
406+
},
407+
{
408+
name: "assignee entry is neither string nor object",
409+
requestArgs: map[string]any{
410+
"owner": "owner",
411+
"repo": "repo",
412+
"issue_number": float64(1),
413+
"assignees": []any{float64(123)},
414+
},
415+
expectedErrText: "each assignee must be a string or an object",
416+
},
417+
}
418+
419+
for _, tc := range tests {
420+
t.Run(tc.name, func(t *testing.T) {
421+
deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil))}
422+
serverTool := GranularUpdateIssueAssignees(translations.NullTranslationHelper)
423+
handler := serverTool.Handler(deps)
424+
425+
request := createMCPRequest(tc.requestArgs)
426+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
427+
require.NoError(t, err)
428+
429+
errorContent := getErrorResult(t, result)
430+
assert.Contains(t, errorContent.Text, tc.expectedErrText)
431+
})
432+
}
433+
}
434+
274435
func TestGranularUpdateIssueLabels(t *testing.T) {
275436
tests := []struct {
276437
name string

0 commit comments

Comments
 (0)