From bedb44e7592eaac24eb5901a6aa1dfd22d24696c Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 17 Aug 2026 15:31:14 +0200 Subject: [PATCH 1/3] fix(issues): fall back on unsupported field schemas Retry list_issues without custom issue field dependencies only when the host schema lacks them. Preserve explicit field filters and propagate unrelated GraphQL errors. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 161 ++++++++++++++++++++++++++- pkg/github/issues_test.go | 213 ++++++++++++++++++++++++++++++++++++ pkg/github/minimal_types.go | 48 +++++++- 3 files changed, 410 insertions(+), 12 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 70b6ecd648..58d19a07b2 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -513,12 +513,46 @@ type IssueFragment struct { } `graphql:"issueFieldValues(first: 25)"` } +type issueFragmentWithoutFieldValues struct { + Number githubv4.Int + Title githubv4.String + Body githubv4.String + State githubv4.String + DatabaseID int64 + + Author struct { + Login githubv4.String + } + CreatedAt githubv4.DateTime + UpdatedAt githubv4.DateTime + Labels struct { + Nodes []struct { + Name githubv4.String + ID githubv4.String + Description githubv4.String + } + } `graphql:"labels(first: 100)"` + Assignees struct { + Nodes []struct { + Login githubv4.String + } + } `graphql:"assignees(first: 100)"` + Comments struct { + TotalCount githubv4.Int + } `graphql:"comments"` +} + // Common interface for all issue query types type IssueQueryResult interface { GetIssueFragment() IssueQueryFragment GetIsPrivate() bool } +type issueQueryResultWithoutFieldValues interface { + getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues + GetIsPrivate() bool +} + type IssueQueryFragment struct { Nodes []IssueFragment `graphql:"nodes"` PageInfo struct { @@ -530,6 +564,17 @@ type IssueQueryFragment struct { TotalCount int } +type issueQueryFragmentWithoutFieldValues struct { + Nodes []issueFragmentWithoutFieldValues `graphql:"nodes"` + PageInfo struct { + HasNextPage githubv4.Boolean + HasPreviousPage githubv4.Boolean + StartCursor githubv4.String + EndCursor githubv4.String + } + TotalCount int +} + // ListIssuesQuery is the root query structure for fetching issues with optional label filtering. type ListIssuesQuery struct { Repository struct { @@ -562,6 +607,34 @@ type ListIssuesQueryTypeWithLabelsWithSince struct { } `graphql:"repository(owner: $owner, name: $repo)"` } +type listIssuesQueryWithoutFieldValues struct { + Repository struct { + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type listIssuesQueryWithLabelsWithoutFieldValues struct { + Repository struct { + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type listIssuesQueryWithSinceWithoutFieldValues struct { + Repository struct { + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + +type listIssuesQueryWithLabelsAndSinceWithoutFieldValues struct { + Repository struct { + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $repo)"` +} + // IssueFieldValueFilter mirrors the GraphQL IssueFieldValueFilter input. Exactly one typed value // field should be set per filter (the monolith resolver rejects multiple). type IssueFieldValueFilter struct { @@ -599,6 +672,38 @@ func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) } +func (q *listIssuesQueryWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { + return q.Repository.Issues +} + +func (q *listIssuesQueryWithoutFieldValues) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func (q *listIssuesQueryWithLabelsWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { + return q.Repository.Issues +} + +func (q *listIssuesQueryWithLabelsWithoutFieldValues) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func (q *listIssuesQueryWithSinceWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { + return q.Repository.Issues +} + +func (q *listIssuesQueryWithSinceWithoutFieldValues) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + +func (q *listIssuesQueryWithLabelsAndSinceWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { + return q.Repository.Issues +} + +func (q *listIssuesQueryWithLabelsAndSinceWithoutFieldValues) GetIsPrivate() bool { + return bool(q.Repository.IsPrivate) +} + func getIssueQueryType(hasLabels bool, hasSince bool) any { switch { case hasLabels && hasSince: @@ -612,6 +717,29 @@ func getIssueQueryType(hasLabels bool, hasSince bool) any { } } +func getIssueQueryTypeWithoutFieldValues(hasLabels bool, hasSince bool) issueQueryResultWithoutFieldValues { + switch { + case hasLabels && hasSince: + return &listIssuesQueryWithLabelsAndSinceWithoutFieldValues{} + case hasLabels: + return &listIssuesQueryWithLabelsWithoutFieldValues{} + case hasSince: + return &listIssuesQueryWithSinceWithoutFieldValues{} + default: + return &listIssuesQueryWithoutFieldValues{} + } +} + +func isUnsupportedListIssuesIssueFieldsError(err error) bool { + switch err.Error() { + case "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)", + "Field 'issueFieldValues' doesn't exist on type 'Issue'": + return true + default: + return false + } +} + // IssueRead creates a tool to get details of a specific issue in a GitHub repository. func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { schema := &jsonschema.Schema{ @@ -3032,16 +3160,37 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { // is a no-op once the flags are globally rolled out. ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") if err := client.Query(ctxWithFeatures, issueQuery, vars); err != nil { - return ghErrors.NewGitHubGraphQLErrorResponse( - ctx, - "failed to list issues", - err, - ), nil, nil + if len(fieldFilters) > 0 || !isUnsupportedListIssuesIssueFieldsError(err) { + return ghErrors.NewGitHubGraphQLErrorResponse( + ctx, + "failed to list issues", + err, + ), nil, nil + } + + issueQueryWithoutFieldValues := getIssueQueryTypeWithoutFieldValues(hasLabels, hasSince) + varsWithoutFieldValues := make(map[string]any, len(vars)-1) + for name, value := range vars { + if name != "issueFieldValues" { + varsWithoutFieldValues[name] = value + } + } + if err := client.Query(ctx, issueQueryWithoutFieldValues, varsWithoutFieldValues); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse( + ctx, + "failed to list issues", + err, + ), nil, nil + } + issueQuery = issueQueryWithoutFieldValues } var resp MinimalIssuesResponse var isPrivate bool - if queryResult, ok := issueQuery.(IssueQueryResult); ok { + if queryResult, ok := issueQuery.(issueQueryResultWithoutFieldValues); ok { + resp = convertToMinimalIssuesResponseWithoutFieldValues(queryResult.getIssueFragmentWithoutFieldValues()) + isPrivate = queryResult.GetIsPrivate() + } else if queryResult, ok := issueQuery.(IssueQueryResult); ok { resp = convertToMinimalIssuesResponse(queryResult.GetIssueFragment()) isPrivate = queryResult.GetIsPrivate() } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 035c7f6720..b636832dae 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2558,6 +2558,219 @@ func Test_ListIssues(t *testing.T) { } } +func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { + t.Parallel() + + responseBody := func(t *testing.T, includeFieldValues bool) string { + t.Helper() + issue := map[string]any{ + "number": 1, + "title": "An issue", + "body": "body", + "state": "OPEN", + "databaseId": 1, + "createdAt": "2026-01-01T00:00:00Z", + "updatedAt": "2026-01-01T00:00:00Z", + "author": map[string]any{"login": "octocat"}, + "labels": map[string]any{"nodes": []any{}}, + "comments": map[string]any{"totalCount": 0}, + } + if includeFieldValues { + issue["issueFieldValues"] = map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelectValue", + "field": map[string]any{"name": "Priority"}, + "value": "P1", + }, + }, + } + } + + body, err := json.Marshal(map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "issues": map[string]any{ + "nodes": []any{issue}, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + "totalCount": 1, + }, + "isPrivate": false, + }, + }, + }) + require.NoError(t, err) + return string(body) + } + + errorBody := func(t *testing.T, message string) string { + t.Helper() + body, err := json.Marshal(map[string]any{ + "errors": []any{map[string]any{"message": message}}, + }) + require.NoError(t, err) + return string(body) + } + + tests := []struct { + name string + args map[string]any + primaryError string + wantFallback bool + wantError bool + wantFieldValues bool + }{ + { + name: "supported schema uses issue fields", + args: map[string]any{"owner": "owner", "repo": "repo"}, + wantFieldValues: true, + }, + { + name: "missing filter input type falls back", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)", + wantFallback: true, + }, + { + name: "missing selected field falls back with labels and since", + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "labels": []any{"bug"}, + "since": "2026-01-01T00:00:00Z", + }, + primaryError: "Field 'issueFieldValues' doesn't exist on type 'Issue'", + wantFallback: true, + }, + { + name: "unrelated GraphQL error is returned", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "Resource not accessible by integration", + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + responses := []func(capturedGraphQLRequest) (int, string){ + func(req capturedGraphQLRequest) (int, string) { + assert.Contains(t, req.Query, "IssueFieldValueFilter") + assert.Contains(t, req.Query, "issueFieldValues(first: 25)") + assert.Contains(t, req.Variables, "issueFieldValues") + if tt.primaryError != "" { + return http.StatusOK, errorBody(t, tt.primaryError) + } + return http.StatusOK, responseBody(t, true) + }, + } + if tt.wantFallback { + responses = append(responses, func(req capturedGraphQLRequest) (int, string) { + assert.NotContains(t, req.Query, "IssueFieldValueFilter") + assert.NotContains(t, req.Query, "issueFieldValues") + assert.NotContains(t, req.Variables, "issueFieldValues") + if _, hasLabels := tt.args["labels"]; hasLabels { + assert.Contains(t, req.Query, "labels: $labels") + } + if _, hasSince := tt.args["since"]; hasSince { + assert.Contains(t, req.Query, "filterBy: {since: $since}") + } + return http.StatusOK, responseBody(t, false) + }) + } + + graphqlTransport := &sequencedGraphQLTransport{t: t, responses: responses} + deps := BaseDeps{ + GQLClient: githubv4.NewClient(&http.Client{Transport: graphqlTransport}), + } + serverTool := ListIssues(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + req := createMCPRequest(tt.args) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + + if tt.wantError { + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, tt.primaryError) + assert.Len(t, graphqlTransport.calls, 1) + return + } + + require.False(t, res.IsError, getTextResult(t, res).Text) + var response MinimalIssuesResponse + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, res).Text), &response)) + require.Len(t, response.Issues, 1) + if tt.wantFieldValues { + assert.Equal(t, []MinimalFieldValue{{Field: "Priority", Value: "P1"}}, response.Issues[0].FieldValues) + } else { + assert.Empty(t, response.Issues[0].FieldValues) + } + assert.Len(t, graphqlTransport.calls, len(responses)) + }) + } + + t.Run("explicit field filters are never dropped", func(t *testing.T) { + fieldsBody, err := json.Marshal(map[string]any{ + "data": map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "id": "IFSS_1", + "name": "Priority", + "dataType": "SINGLE_SELECT", + "visibility": "ALL", + "options": []any{ + map[string]any{"id": "OPT_P1", "name": "P1", "color": "red"}, + }, + }, + }, + }, + }, + }, + }) + require.NoError(t, err) + + const unsupported = "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)" + graphqlTransport := &sequencedGraphQLTransport{ + t: t, + responses: []func(capturedGraphQLRequest) (int, string){ + func(req capturedGraphQLRequest) (int, string) { + assert.Contains(t, req.Query, "issueFields") + return http.StatusOK, string(fieldsBody) + }, + func(req capturedGraphQLRequest) (int, string) { + assert.Contains(t, req.Query, "IssueFieldValueFilter") + assert.NotEmpty(t, req.Variables["issueFieldValues"]) + return http.StatusOK, errorBody(t, unsupported) + }, + }, + } + deps := BaseDeps{ + GQLClient: githubv4.NewClient(&http.Client{Transport: graphqlTransport}), + } + serverTool := ListIssues(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, unsupported) + assert.Len(t, graphqlTransport.calls, 2) + }) +} + func Test_ListIssues_FieldFilters(t *testing.T) { t.Parallel() diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index b27f5e4a2e..6902170c1c 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -861,6 +861,30 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue { } func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { + m := fragmentWithoutFieldValuesToMinimalIssue(issueFragmentWithoutFieldValues{ + Number: fragment.Number, + Title: fragment.Title, + Body: fragment.Body, + State: fragment.State, + DatabaseID: fragment.DatabaseID, + Author: fragment.Author, + CreatedAt: fragment.CreatedAt, + UpdatedAt: fragment.UpdatedAt, + Labels: fragment.Labels, + Assignees: fragment.Assignees, + Comments: fragment.Comments, + }) + + for _, fv := range fragment.IssueFieldValues.Nodes { + if mfv, ok := fragmentToMinimalFieldValue(fv); ok { + m.FieldValues = append(m.FieldValues, mfv) + } + } + + return m +} + +func fragmentWithoutFieldValuesToMinimalIssue(fragment issueFragmentWithoutFieldValues) MinimalIssue { m := MinimalIssue{ Number: int(fragment.Number), Title: sanitize.Sanitize(string(fragment.Title)), @@ -883,12 +907,6 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { m.Assignees = append(m.Assignees, string(assignee.Login)) } - for _, fv := range fragment.IssueFieldValues.Nodes { - if mfv, ok := fragmentToMinimalFieldValue(fv); ok { - m.FieldValues = append(m.FieldValues, mfv) - } - } - return m } @@ -938,6 +956,24 @@ func convertToMinimalIssuesResponse(fragment IssueQueryFragment) MinimalIssuesRe } } +func convertToMinimalIssuesResponseWithoutFieldValues(fragment issueQueryFragmentWithoutFieldValues) MinimalIssuesResponse { + minimalIssues := make([]MinimalIssue, 0, len(fragment.Nodes)) + for _, issue := range fragment.Nodes { + minimalIssues = append(minimalIssues, fragmentWithoutFieldValuesToMinimalIssue(issue)) + } + + return MinimalIssuesResponse{ + Issues: minimalIssues, + TotalCount: fragment.TotalCount, + PageInfo: MinimalPageInfo{ + HasNextPage: bool(fragment.PageInfo.HasNextPage), + HasPreviousPage: bool(fragment.PageInfo.HasPreviousPage), + StartCursor: string(fragment.PageInfo.StartCursor), + EndCursor: string(fragment.PageInfo.EndCursor), + }, + } +} + func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment { m := MinimalIssueComment{ ID: comment.GetID(), From e7356d1cc3bd8db594d787ef64d2968f975ae167 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Tue, 18 Aug 2026 23:36:23 +0200 Subject: [PATCH 2/3] fix(issues): harden GHES schema fallback Handle alternate issue-field validation messages, preserve primary and retry errors, and avoid runtime result type switches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues.go | 42 +++++++------ pkg/github/issues_test.go | 129 +++++++++++++++++++++++++++++--------- 2 files changed, 121 insertions(+), 50 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 58d19a07b2..b53ee6d596 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -704,7 +704,7 @@ func (q *listIssuesQueryWithLabelsAndSinceWithoutFieldValues) GetIsPrivate() boo return bool(q.Repository.IsPrivate) } -func getIssueQueryType(hasLabels bool, hasSince bool) any { +func getIssueQueryType(hasLabels bool, hasSince bool) IssueQueryResult { switch { case hasLabels && hasSince: return &ListIssuesQueryTypeWithLabelsWithSince{} @@ -731,13 +731,16 @@ func getIssueQueryTypeWithoutFieldValues(hasLabels bool, hasSince bool) issueQue } func isUnsupportedListIssuesIssueFieldsError(err error) bool { - switch err.Error() { - case "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)", - "Field 'issueFieldValues' doesn't exist on type 'Issue'": + message := err.Error() + if strings.Contains(message, "IssueFieldValueFilter") { return true - default: + } + if !strings.Contains(message, "issueFieldValues") { return false } + return strings.Contains(message, "doesn't exist on type") || + strings.Contains(message, "doesn't accept argument") || + (strings.Contains(message, "Argument 'filterBy'") && strings.Contains(message, "invalid value")) } // IssueRead creates a tool to get details of a specific issue in a GitHub repository. @@ -3159,12 +3162,19 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { // input type unconditionally, so we always opt into the feature via header. This // is a no-op once the flags are globally rolled out. ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") - if err := client.Query(ctxWithFeatures, issueQuery, vars); err != nil { - if len(fieldFilters) > 0 || !isUnsupportedListIssuesIssueFieldsError(err) { + issueFieldsErr := client.Query(ctxWithFeatures, issueQuery, vars) + + var resp MinimalIssuesResponse + var isPrivate bool + if issueFieldsErr == nil { + resp = convertToMinimalIssuesResponse(issueQuery.GetIssueFragment()) + isPrivate = issueQuery.GetIsPrivate() + } else { + if len(fieldFilters) > 0 || !isUnsupportedListIssuesIssueFieldsError(issueFieldsErr) { return ghErrors.NewGitHubGraphQLErrorResponse( ctx, "failed to list issues", - err, + issueFieldsErr, ), nil, nil } @@ -3175,24 +3185,16 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { varsWithoutFieldValues[name] = value } } - if err := client.Query(ctx, issueQueryWithoutFieldValues, varsWithoutFieldValues); err != nil { + if fallbackErr := client.Query(ctx, issueQueryWithoutFieldValues, varsWithoutFieldValues); fallbackErr != nil { return ghErrors.NewGitHubGraphQLErrorResponse( ctx, "failed to list issues", - err, + fmt.Errorf("issue-fields query failed: %w; fallback query failed: %w", issueFieldsErr, fallbackErr), ), nil, nil } - issueQuery = issueQueryWithoutFieldValues - } - var resp MinimalIssuesResponse - var isPrivate bool - if queryResult, ok := issueQuery.(issueQueryResultWithoutFieldValues); ok { - resp = convertToMinimalIssuesResponseWithoutFieldValues(queryResult.getIssueFragmentWithoutFieldValues()) - isPrivate = queryResult.GetIsPrivate() - } else if queryResult, ok := issueQuery.(IssueQueryResult); ok { - resp = convertToMinimalIssuesResponse(queryResult.GetIssueFragment()) - isPrivate = queryResult.GetIsPrivate() + resp = convertToMinimalIssuesResponseWithoutFieldValues(issueQueryWithoutFieldValues.getIssueFragmentWithoutFieldValues()) + isPrivate = issueQueryWithoutFieldValues.GetIsPrivate() } filtered := false diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index b636832dae..ec8c9ff902 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2621,6 +2621,7 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { name string args map[string]any primaryError string + fallbackError string wantFallback bool wantError bool wantFieldValues bool @@ -2647,12 +2648,54 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { primaryError: "Field 'issueFieldValues' doesn't exist on type 'Issue'", wantFallback: true, }, + { + name: "issue filters input rejects issue field values", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "InputObject 'IssueFilters' doesn't accept argument 'issueFieldValues'", + wantFallback: true, + }, + { + name: "invalid filter by issue field values falls back", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "Argument 'filterBy' on Field 'issues' has an invalid value ({issueFieldValues: $issueFieldValues}). Expected type 'IssueFilters'.", + wantFallback: true, + }, + { + name: "invalid filter by since and issue field values falls back", + args: map[string]any{ + "owner": "owner", + "repo": "repo", + "since": "2026-01-01T00:00:00Z", + }, + primaryError: "Argument 'filterBy' on Field 'issues' has an invalid value ({since: $since, issueFieldValues: $issueFieldValues}). Expected type 'IssueFilters'.", + wantFallback: true, + }, { name: "unrelated GraphQL error is returned", args: map[string]any{"owner": "owner", "repo": "repo"}, primaryError: "Resource not accessible by integration", wantError: true, }, + { + name: "unrelated invalid filter by error is returned", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "Argument 'filterBy' on Field 'issues' has an invalid value ({since: $since}). Expected type 'IssueFilters'.", + wantError: true, + }, + { + name: "issue field values resolver error is returned", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "Something went wrong while resolving 'issueFieldValues'", + wantError: true, + }, + { + name: "fallback failure preserves both errors", + args: map[string]any{"owner": "owner", "repo": "repo"}, + primaryError: "InputObject 'IssueFilters' doesn't accept argument 'issueFieldValues'", + fallbackError: "Resource not accessible by integration", + wantFallback: true, + wantError: true, + }, } for _, tt := range tests { @@ -2679,6 +2722,9 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { if _, hasSince := tt.args["since"]; hasSince { assert.Contains(t, req.Query, "filterBy: {since: $since}") } + if tt.fallbackError != "" { + return http.StatusOK, errorBody(t, tt.fallbackError) + } return http.StatusOK, responseBody(t, false) }) } @@ -2696,7 +2742,10 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { if tt.wantError { require.True(t, res.IsError) assert.Contains(t, getTextResult(t, res).Text, tt.primaryError) - assert.Len(t, graphqlTransport.calls, 1) + if tt.fallbackError != "" { + assert.Contains(t, getTextResult(t, res).Text, tt.fallbackError) + } + assert.Len(t, graphqlTransport.calls, len(responses)) return } @@ -2736,38 +2785,58 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { }) require.NoError(t, err) - const unsupported = "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)" - graphqlTransport := &sequencedGraphQLTransport{ - t: t, - responses: []func(capturedGraphQLRequest) (int, string){ - func(req capturedGraphQLRequest) (int, string) { - assert.Contains(t, req.Query, "issueFields") - return http.StatusOK, string(fieldsBody) - }, - func(req capturedGraphQLRequest) (int, string) { - assert.Contains(t, req.Query, "IssueFieldValueFilter") - assert.NotEmpty(t, req.Variables["issueFieldValues"]) - return http.StatusOK, errorBody(t, unsupported) - }, + unsupportedErrors := []struct { + name string + message string + }{ + { + name: "missing input type", + message: "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)", + }, + { + name: "issue filters input rejects issue field values", + message: "InputObject 'IssueFilters' doesn't accept argument 'issueFieldValues'", + }, + { + name: "invalid filter by issue field values", + message: "Argument 'filterBy' on Field 'issues' has an invalid value ({issueFieldValues: $issueFieldValues}). Expected type 'IssueFilters'.", }, } - deps := BaseDeps{ - GQLClient: githubv4.NewClient(&http.Client{Transport: graphqlTransport}), + for _, unsupported := range unsupportedErrors { + t.Run(unsupported.name, func(t *testing.T) { + graphqlTransport := &sequencedGraphQLTransport{ + t: t, + responses: []func(capturedGraphQLRequest) (int, string){ + func(req capturedGraphQLRequest) (int, string) { + assert.Contains(t, req.Query, "issueFields") + return http.StatusOK, string(fieldsBody) + }, + func(req capturedGraphQLRequest) (int, string) { + assert.Contains(t, req.Query, "IssueFieldValueFilter") + assert.NotEmpty(t, req.Variables["issueFieldValues"]) + return http.StatusOK, errorBody(t, unsupported.message) + }, + }, + } + deps := BaseDeps{ + GQLClient: githubv4.NewClient(&http.Client{Transport: graphqlTransport}), + } + serverTool := ListIssues(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "field_filters": []any{ + map[string]any{"field_name": "Priority", "value": "P1"}, + }, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, res.IsError) + assert.Contains(t, getTextResult(t, res).Text, unsupported.message) + assert.Len(t, graphqlTransport.calls, 2) + }) } - serverTool := ListIssues(translations.NullTranslationHelper) - handler := serverTool.Handler(deps) - req := createMCPRequest(map[string]any{ - "owner": "owner", - "repo": "repo", - "field_filters": []any{ - map[string]any{"field_name": "Priority", "value": "P1"}, - }, - }) - res, err := handler(ContextWithDeps(context.Background(), deps), &req) - require.NoError(t, err) - require.True(t, res.IsError) - assert.Contains(t, getTextResult(t, res).Text, unsupported) - assert.Len(t, graphqlTransport.calls, 2) }) } From 1acc2a04a8a21a0f4fbc6f688867518ee02d7c94 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 11:45:03 +0200 Subject: [PATCH 3/3] test(issues): cover GHES fallback assignees Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/github/issues_test.go | 121 +++++++++++++++++++++++++++++++++++--- 1 file changed, 114 insertions(+), 7 deletions(-) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index ec8c9ff902..fb747a1283 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2563,7 +2563,7 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { responseBody := func(t *testing.T, includeFieldValues bool) string { t.Helper() - issue := map[string]any{ + assignedIssue := map[string]any{ "number": 1, "title": "An issue", "body": "body", @@ -2573,10 +2573,11 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { "updatedAt": "2026-01-01T00:00:00Z", "author": map[string]any{"login": "octocat"}, "labels": map[string]any{"nodes": []any{}}, + "assignees": map[string]any{"nodes": []any{map[string]any{"login": "hubot"}}}, "comments": map[string]any{"totalCount": 0}, } if includeFieldValues { - issue["issueFieldValues"] = map[string]any{ + assignedIssue["issueFieldValues"] = map[string]any{ "nodes": []any{ map[string]any{ "__typename": "IssueFieldSingleSelectValue", @@ -2586,19 +2587,32 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { }, } } + unassignedIssue := map[string]any{ + "number": 2, + "title": "An unassigned issue", + "body": "body", + "state": "OPEN", + "databaseId": 2, + "createdAt": "2026-01-02T00:00:00Z", + "updatedAt": "2026-01-02T00:00:00Z", + "author": map[string]any{"login": "octocat"}, + "labels": map[string]any{"nodes": []any{}}, + "assignees": map[string]any{"nodes": []any{}}, + "comments": map[string]any{"totalCount": 0}, + } body, err := json.Marshal(map[string]any{ "data": map[string]any{ "repository": map[string]any{ "issues": map[string]any{ - "nodes": []any{issue}, + "nodes": []any{assignedIssue, unassignedIssue}, "pageInfo": map[string]any{ "hasNextPage": false, "hasPreviousPage": false, "startCursor": "", "endCursor": "", }, - "totalCount": 1, + "totalCount": 2, }, "isPrivate": false, }, @@ -2608,6 +2622,20 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { return string(body) } + fallbackQuery := func(hasLabels, hasSince bool) string { + const selection = "{nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}" + switch { + case hasLabels && hasSince: + return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + ",isPrivate}}" + case hasLabels: + return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})" + selection + ",isPrivate}}" + case hasSince: + return "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + ",isPrivate}}" + default: + return "query($after:String$direction:OrderDirection!$first:Int!$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})" + selection + ",isPrivate}}" + } + } + errorBody := func(t *testing.T, message string) string { t.Helper() body, err := json.Marshal(map[string]any{ @@ -2648,6 +2676,12 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { primaryError: "Field 'issueFieldValues' doesn't exist on type 'Issue'", wantFallback: true, }, + { + name: "missing filter input type falls back with labels", + args: map[string]any{"owner": "owner", "repo": "repo", "labels": []any{"bug"}}, + primaryError: "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)", + wantFallback: true, + }, { name: "issue filters input rejects issue field values", args: map[string]any{"owner": "owner", "repo": "repo"}, @@ -2713,13 +2747,17 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { } if tt.wantFallback { responses = append(responses, func(req capturedGraphQLRequest) (int, string) { + _, hasLabels := tt.args["labels"] + _, hasSince := tt.args["since"] + assert.Equal(t, fallbackQuery(hasLabels, hasSince), req.Query) + assert.Contains(t, req.Query, "assignees(first: 100){nodes{login}}") assert.NotContains(t, req.Query, "IssueFieldValueFilter") assert.NotContains(t, req.Query, "issueFieldValues") assert.NotContains(t, req.Variables, "issueFieldValues") - if _, hasLabels := tt.args["labels"]; hasLabels { + if hasLabels { assert.Contains(t, req.Query, "labels: $labels") } - if _, hasSince := tt.args["since"]; hasSince { + if hasSince { assert.Contains(t, req.Query, "filterBy: {since: $since}") } if tt.fallbackError != "" { @@ -2752,7 +2790,10 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { require.False(t, res.IsError, getTextResult(t, res).Text) var response MinimalIssuesResponse require.NoError(t, json.Unmarshal([]byte(getTextResult(t, res).Text), &response)) - require.Len(t, response.Issues, 1) + require.Len(t, response.Issues, 2) + assert.Equal(t, []string{"hubot"}, response.Issues[0].Assignees) + assert.NotNil(t, response.Issues[1].Assignees) + assert.Empty(t, response.Issues[1].Assignees) if tt.wantFieldValues { assert.Equal(t, []MinimalFieldValue{{Field: "Priority", Value: "P1"}}, response.Issues[0].FieldValues) } else { @@ -2762,6 +2803,72 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { }) } + t.Run("fallback applies fields filtering to assignees", func(t *testing.T) { + tests := []struct { + name string + fields []any + wantAssignees bool + wantAssigned []any + wantUnassigned []any + }{ + { + name: "includes assignees", + fields: []any{"number", "assignees"}, + wantAssignees: true, + wantAssigned: []any{"hubot"}, + wantUnassigned: []any{}, + }, + { + name: "excludes assignees", + fields: []any{"number"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + graphqlTransport := &sequencedGraphQLTransport{ + t: t, + responses: []func(capturedGraphQLRequest) (int, string){ + func(_ capturedGraphQLRequest) (int, string) { + return http.StatusOK, errorBody(t, "IssueFieldValueFilter isn't a defined input type (on $issueFieldValues)") + }, + func(req capturedGraphQLRequest) (int, string) { + assert.Equal(t, fallbackQuery(false, false), req.Query) + return http.StatusOK, responseBody(t, false) + }, + }, + } + deps := BaseDeps{ + GQLClient: githubv4.NewClient(&http.Client{Transport: graphqlTransport}), + } + serverTool := ListIssues(translations.NullTranslationHelper) + handler := serverTool.Handler(deps) + req := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "fields": tt.fields, + }) + res, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, res.IsError, getTextResult(t, res).Text) + + var response struct { + Issues []map[string]any `json:"issues"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, res).Text), &response)) + require.Len(t, response.Issues, 2) + if tt.wantAssignees { + assert.Equal(t, tt.wantAssigned, response.Issues[0]["assignees"]) + assert.Equal(t, tt.wantUnassigned, response.Issues[1]["assignees"]) + } else { + assert.NotContains(t, response.Issues[0], "assignees") + assert.NotContains(t, response.Issues[1], "assignees") + } + assert.Len(t, graphqlTransport.calls, 2) + }) + } + }) + t.Run("explicit field filters are never dropped", func(t *testing.T) { fieldsBody, err := json.Marshal(map[string]any{ "data": map[string]any{