Skip to content

Fix list_issues on GHES schemas without issue fields - #3086

Open
SamMorrowDrums wants to merge 2 commits into
mainfrom
sammorrowdrums-fix-ghes-list-issues-fields
Open

Fix list_issues on GHES schemas without issue fields#3086
SamMorrowDrums wants to merge 2 commits into
mainfrom
sammorrowdrums-fix-ghes-list-issues-fields

Conversation

@SamMorrowDrums

Copy link
Copy Markdown
Collaborator

Summary

  • keep the full IssueFieldValueFilter and Issue.issueFieldValues query on supported GitHub.com, GHEC, and GHES schemas
  • retry without both custom-field dependencies only for the two known unsupported-schema validation errors
  • preserve explicit field_filters and propagate unrelated GraphQL failures
  • add table-driven GraphQL regression coverage for supported, unsupported, and unrelated-error paths

Validation

  • script/lint
  • script/test

Fixes #3068

@SamMorrowDrums
SamMorrowDrums requested a review from a team as a code owner August 17, 2026 13:32
Copilot AI balanced review requested due to automatic review settings August 17, 2026 13:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@IrynaKulakova

Copy link
Copy Markdown
Contributor

Reviewed the branch locally and ran go test ./pkg/github -run Test_ListIssues (green). Overall the approach looks right: fallback is gated on schema-shape errors only, explicit field_filters are preserved, and the table-driven coverage is good. A few things worth considering:

1. Exact string matching is likely too narrow (main concern)

shurcooL/graphql's errors.Error() returns only the first message (graphql.go:117). On a GHES schema missing both dependencies, validation emits several errors, and the first one may not be either of the two whitelisted strings — e.g. InputObject 'IssueFilters' doesn't accept argument 'issueFieldValues', or an Argument 'filterBy' ... has an invalid value variant. In that case no fallback happens and #3068 reproduces on that GHES version.

Since the fallback is cheap and safe (any genuine failure resurfaces from the retry), a permissive match seems strictly better:

func isUnsupportedListIssuesIssueFieldsError(err error) bool {
	msg := err.Error()
	return strings.Contains(msg, "IssueFieldValueFilter") || strings.Contains(msg, "issueFieldValues")
}

2. Retry error masks the original

If the fallback query fails for an unrelated reason (rate limit, auth), the user only sees that error and never learns the primary query hit a schema gap. Consider joining both messages.

3. Result extraction via type switch is fragile

The fallback branch already knows the concrete shape, so setting resp/isPrivate there directly would avoid re-detecting it through any with an ordering-sensitive if/else if.

4. The duplicated structs look avoidable

Anonymous untagged fields are inlined by the library in both query construction (query.go:115) and unmarshalling (jsonutil/graphql.go:182), so IssueFragment could embed issueFragmentWithoutFieldValues and add only IssueFieldValues, producing an identical query. That removes ~130 lines of duplication plus the field-by-field copy in fragmentToMinimalIssue, which otherwise needs manual updating every time a field is added.

5. Two round trips per call on GHES

Including every pagination page. A per-client memo of "this schema doesn't support issue fields" would avoid the repeated probe — fine as a follow-up.

6. Scope gap

Field 'issueFieldValues' doesn't exist on type 'Issue' also affects fetchIssueFieldValuesByNodeID (search_issues), the issue_read enrichment query, and fetchExistingIssueFieldValues — all still hard-fail on GHES. Reasonable to defer, but worth tracking.

Nit: getIssueQueryType still returns any while its new sibling returns a typed interface — could be aligned.

SamMorrowDrums and others added 2 commits August 18, 2026 23:31
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>
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>
@SamMorrowDrums
SamMorrowDrums force-pushed the sammorrowdrums-fix-ghes-list-issues-fields branch from 80fc69b to 5249762 Compare August 18, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

list_issues fails on GHES: IssueFieldValueFilter isn't a defined input type

3 participants