Skip to content

Commit 95a8e75

Browse files
tgockelSamMorrowDrums
authored andcommitted
Return assignees from list_issues
The list_issues GraphQL fragment never selected assignees, so the tool could not report who an issue was assigned to. Its nearest field, user, is the issue author, which callers conflate with the assignee. Answering "is anything unassigned?" therefore cost one list_issues call plus one issue_read per candidate, and a truncated sweep invites a fabricated answer drawn from the author instead. Add an assignees selection to IssueFragment, flatten it to logins in fragmentToMinimalIssue, and add "assignees" to listIssuesItemFieldEnum so it is selectable through fields. GitHub caps issue assignees at 10, so first: 100 cannot truncate; it also matches the page size already used for assignees in copilot.go. Drop omitempty from MinimalIssue.Assignees and initialize the slice in both converters so an unassigned issue serializes as [] rather than an absent key, which is what lets a caller identify unassigned issues from a single response. This also affects issue_read, the other MinimalIssue consumer, which now reports "assignees": [] instead of omitting the key.
1 parent 3000061 commit 95a8e75

5 files changed

Lines changed: 96 additions & 9 deletions

File tree

pkg/github/__toolsnaps__/list_issues.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
"state",
5151
"user",
5252
"labels",
53+
"assignees",
5354
"comments",
5455
"created_at",
5556
"updated_at",

pkg/github/fields_filtering_test.go

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ func Test_SearchIssues_FieldsTelemetry(t *testing.T) {
274274
// getIssueQueryType; see Test_ListIssues for the canonical copies.
275275
const listIssuesFieldsFieldValuesSelection = "issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}"
276276

277-
const listIssuesFieldsQuery = "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}," + listIssuesFieldsFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
277+
const listIssuesFieldsQuery = "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + listIssuesFieldsFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
278278

279279
func listIssuesFieldsMockClient() *http.Client {
280280
vars := map[string]any{
@@ -301,6 +301,7 @@ func listIssuesFieldsMockClient() *http.Client {
301301
"updatedAt": "2023-01-01T00:00:00Z",
302302
"author": map[string]any{"login": "user1"},
303303
"labels": map[string]any{"nodes": []map[string]any{}},
304+
"assignees": map[string]any{"nodes": []map[string]any{{"login": "octocat"}}},
304305
"comments": map[string]any{"totalCount": 1},
305306
"issueFieldValues": map[string]any{"nodes": []map[string]any{}},
306307
},
@@ -353,6 +354,57 @@ func Test_ListIssues_FieldFiltering(t *testing.T) {
353354
assert.NotContains(t, textContent.Text, "\"body\"")
354355
}
355356

357+
// Test_ListIssues_AssigneesField covers the assignees field end to end: it is
358+
// selectable via fields, it is dropped when not requested, and it is always
359+
// present in an unfiltered response so that "unassigned" reads as [] rather
360+
// than an absent key.
361+
func Test_ListIssues_AssigneesField(t *testing.T) {
362+
serverTool := ListIssues(translations.NullTranslationHelper)
363+
364+
callWithFields := func(t *testing.T, fields []any) string {
365+
t.Helper()
366+
deps := BaseDeps{GQLClient: githubv4.NewClient(listIssuesFieldsMockClient())}
367+
handler := serverTool.Handler(deps)
368+
369+
args := map[string]any{"owner": "owner", "repo": "repo"}
370+
if fields != nil {
371+
args["fields"] = fields
372+
}
373+
request := createMCPRequest(args)
374+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
375+
require.NoError(t, err)
376+
require.False(t, result.IsError)
377+
return getTextResult(t, result).Text
378+
}
379+
380+
t.Run("selectable via fields", func(t *testing.T) {
381+
var returned struct {
382+
Issues []map[string]any `json:"issues"`
383+
}
384+
require.NoError(t, json.Unmarshal([]byte(callWithFields(t, []any{"number", "assignees"})), &returned))
385+
require.Len(t, returned.Issues, 1)
386+
require.Len(t, returned.Issues[0], 2, "only the two requested fields should be present")
387+
assert.Equal(t, []any{"octocat"}, returned.Issues[0]["assignees"])
388+
})
389+
390+
t.Run("omitted when not requested", func(t *testing.T) {
391+
text := callWithFields(t, []any{"number", "title"})
392+
assert.NotContains(t, text, "\"assignees\"")
393+
})
394+
395+
t.Run("unassigned issues serialize as an empty array", func(t *testing.T) {
396+
// The mock returns one assigned issue, so drive the empty case through
397+
// the conversion directly: no assignees node must still yield [], never
398+
// null and never an absent key.
399+
issue := fragmentToMinimalIssue(IssueFragment{})
400+
require.NotNil(t, issue.Assignees)
401+
402+
encoded, err := json.Marshal(issue)
403+
require.NoError(t, err)
404+
assert.Contains(t, string(encoded), "\"assignees\":[]")
405+
})
406+
}
407+
356408
func Test_ListIssues_FieldsTelemetry(t *testing.T) {
357409
serverTool := ListIssues(translations.NullTranslationHelper)
358410

pkg/github/issues.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,12 @@ type IssueFragment struct {
499499
Description githubv4.String
500500
}
501501
} `graphql:"labels(first: 100)"`
502+
// GitHub caps issue assignees at 10, so first: 100 cannot truncate.
503+
Assignees struct {
504+
Nodes []struct {
505+
Login githubv4.String
506+
}
507+
} `graphql:"assignees(first: 100)"`
502508
Comments struct {
503509
TotalCount githubv4.Int
504510
} `graphql:"comments"`

0 commit comments

Comments
 (0)