From 80fc69b704db538f5c6258772c0cf1663f491032 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 17 Aug 2026 15:31:14 +0200 Subject: [PATCH] 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 | 156 +++++++++++++++++++++++++- pkg/github/issues_test.go | 213 ++++++++++++++++++++++++++++++++++++ pkg/github/minimal_types.go | 47 +++++++- 3 files changed, 404 insertions(+), 12 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index dfb823e26b..fcc369baab 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -507,12 +507,41 @@ 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)"` + 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 { @@ -524,6 +553,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 { @@ -556,6 +596,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 { @@ -593,6 +661,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: @@ -606,6 +706,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{ @@ -3003,16 +3126,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 77380e5e21..1e83bb793d 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -2524,6 +2524,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 2424823c2c..f3e8547ef3 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -855,6 +855,29 @@ 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, + 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)), @@ -872,12 +895,6 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue { m.Labels = append(m.Labels, string(label.Name)) } - for _, fv := range fragment.IssueFieldValues.Nodes { - if mfv, ok := fragmentToMinimalFieldValue(fv); ok { - m.FieldValues = append(m.FieldValues, mfv) - } - } - return m } @@ -927,6 +944,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(),