Add multi_select issue field support#2659
Open
owenniblock wants to merge 4 commits into
Open
Conversation
Threads the multi_select issue field type through list_issue_fields, issue_write, and list_issues (read + filter): - C0: list_issue_fields surfaces IssueFieldMultiSelect definitions. - C1: issue_write accepts a new `field_option_names` slot for multi_select fields, validates options against the field definition, and sends an array of option names to the REST API (matching IssueField#build_value_attributes from github/github#433220). - C2: list_issues + GraphQL value fragment understand IssueFieldMultiSelectValue, and field_filters accepts a `values` array that maps to the new `multiSelectOptionValues` GraphQL input added in github/github#435047. Filter uses all-of (AND) semantics; documented in the tool description. Wraps fetchIssueFieldValuesByNodeID in the issue_fields/ repo_issue_fields GraphQL feature context so the new fragment is honoured by the API. Refs github/issues#21956. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion
The REST update endpoint uses set semantics for issue_field_values: sending
{"issue_field_values": [...]} overwrites the entire list. We were relying on
that to clear deleted fields by omitting them from the merged payload, but
Go's omitempty strips an empty []*IssueRequestFieldValue, so when the only
remaining field was the one being deleted, nothing got sent and the field
kept its old value.
Capture each field's GraphQL node ID alongside its database ID at resolve
time, and after the REST PATCH succeeds run deleteIssueFieldValue per
deletion. This is idempotent and works even when the kept list is empty.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous wording said 'Setting an empty array clears the field — use delete: true for that instead', which was contradictory. An empty array is actually rejected; clearing requires delete: true. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…field, add combined test Three small fixes from code review: 1. optionalIssueWriteFields now returns a targeted error when field_option_names is present but empty, pointing the caller at delete:true. The generic 'must specify one of' message was confusing because the caller did specify field_option_names. 2. Drop the dead MultiSelectOptionIDs field from IssueFieldValueFilter. resolveFieldFilters only ever sets MultiSelectOptionValues; the IDs variant was declared but never wired. 3. Add Test_UpdateIssue_DeleteAndSetFieldsInSameCall covering a single UpdateIssue call that deletes one multi_select field and sets another field in the same request. Asserts the REST PATCH carries only the kept set (no null clearing) and that the deleteIssueFieldValue mutation fires for the deleted field with the right node IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for multi_select issue fields across the MCP server’s issue-field surfaces (list field definitions, set/clear values via issue_write, and read/filter via list_issues), aligning behavior and schema with the existing single-select plumbing and ensuring GraphQL feature-gated fragments are actually exercised.
Changes:
- Threaded
multi_selectthrough GraphQL fragments + minimal output mapping (includingMinimalFieldValue.Values). - Extended
issue_writeto acceptfield_option_names: []stringfor multi-select, with option validation + improved delete semantics. - Extended
list_issuesfield filtering to acceptvalues: []stringfor multi-select (AND semantics) and updated toolsnaps/docs/tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/minimal_types.go | Adds multi-select support in GraphQL→minimal field-value conversion (populates Values). |
| pkg/github/issues.go | Adds multi-select fragments, issue_write parsing/validation, list_issues multi-select filtering, and GraphQL-backed delete-on-clear behavior. |
| pkg/github/issues_test.go | Updates hardcoded GraphQL query string expectations and error-message assertions for new fragments/validation. |
| pkg/github/issues_multiselect_test.go | New targeted tests for multi-select parsing, filter resolution, fragment conversion, and delete mutation behavior. |
| pkg/github/issues_granular.go | Introduces DeleteIssueFieldValueInput type used by the new delete mutation path. |
| pkg/github/issue_fields.go | Extends list_issue_fields to surface multi_select field definitions and options. |
| pkg/github/toolsnaps/list_issues_ff_remote_mcp_issue_fields.snap | Updates schema snapshot for new field_filters[].values and revised description/required fields. |
| pkg/github/toolsnaps/list_issue_fields.snap | Updates schema snapshot to include multi_select in description/output expectations. |
| pkg/github/toolsnaps/issue_write_ff_remote_mcp_issue_fields.snap | Updates schema snapshot to include field_option_names and updated mutual-exclusion wording. |
| docs/insiders-features.md | Regenerates tool docs reflecting new issue_write.issue_fields[].field_option_names and list_issues.field_filters[].values. |
| docs/feature-flags.md | Regenerates feature-flag inventory/docs to reflect the updated tool schemas/descriptions. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 3
Comment on lines
+2590
to
+2614
| // Clear any fields marked with delete:true via the GraphQL deleteIssueFieldValue | ||
| // mutation. The REST PATCH above can't do this reliably — Go's omitempty drops an | ||
| // empty issue_field_values array, leaving the old values intact. | ||
| if len(fieldsToDelete) > 0 { | ||
| issueID, _, err := fetchIssueIDs(ctx, gqlClient, owner, repo, issueNumber, 0) | ||
| if err != nil { | ||
| return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to look up issue for field deletion", err), nil | ||
| } | ||
| for _, deletion := range fieldsToDelete { | ||
| var mutation struct { | ||
| DeleteIssueFieldValue struct { | ||
| Issue struct { | ||
| Number githubv4.Int | ||
| } | ||
| } `graphql:"deleteIssueFieldValue(input: $input)"` | ||
| } | ||
| input := DeleteIssueFieldValueInput{ | ||
| IssueID: issueID, | ||
| FieldID: deletion.NodeID, | ||
| } | ||
| if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { | ||
| return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to delete issue field value", err), nil | ||
| } | ||
| } | ||
| } |
Comment on lines
+297
to
+315
| var fieldOptionNames []string | ||
| _, hasNamesKey := itemMap["field_option_names"] | ||
| if rawNames := itemMap["field_option_names"]; hasNamesKey && rawNames != nil { | ||
| switch v := rawNames.(type) { | ||
| case []string: | ||
| fieldOptionNames = v | ||
| case []any: | ||
| fieldOptionNames = make([]string, 0, len(v)) | ||
| for _, item := range v { | ||
| s, ok := item.(string) | ||
| if !ok { | ||
| return nil, fmt.Errorf("field_option_names for field %q must be an array of strings", fieldName) | ||
| } | ||
| fieldOptionNames = append(fieldOptionNames, s) | ||
| } | ||
| default: | ||
| return nil, fmt.Errorf("field_option_names for field %q must be an array of strings", fieldName) | ||
| } | ||
| } |
Comment on lines
+3352
to
+3363
| func matchOption(field IssueField, value string) (string, error) { | ||
| for _, o := range field.Options { | ||
| if strings.EqualFold(o.Name, value) { | ||
| return o.Name, nil | ||
| } | ||
| } | ||
| optionNames := make([]string, 0, len(field.Options)) | ||
| for _, o := range field.Options { | ||
| optionNames = append(optionNames, o.Name) | ||
| } | ||
| return "", fmt.Errorf("field_filters: %q is not a valid option for %q. Valid options: %s", value, field.Name, strings.Join(optionNames, ", ")) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Threads the new
multi_selectissue field type throughlist_issue_fields,issue_write, andlist_issues(read + filter), mirroring howsingle_selectis wired today.Why
Phase C of github/issues#21956. Multi-select issue field support has shipped on the GraphQL/REST side in github/github#433220; GraphQL filter inputs are landing in github/github#435047 (by @akenneth). Until this PR the MCP server silently dropped multi-select fields everywhere.
Refs github/issues#21956 (multi-phase, don't auto-close).
What changed
list_issue_fields— addedIssueFieldMultiSelectinline fragment + typename case so multi-select field definitions are surfaced.issue_write— newfield_option_names: []stringslot onissue_fields[]items. Server resolves the names against the field's options (case-insensitive, returns canonical names) and sends[]stringto REST, matchingIssueField#build_value_attributes. Mutual-exclusion validation extended to "exactly one ofvalue/field_option_name/field_option_names/delete: true". Multi-select rejects rawvaluewith a hint to usefield_option_names.list_issues—IssueFieldMultiSelectValue(plain list ofoptions { name }, not a connection — confirmed againstapp/graphql/objects/issue_field_multi_select_value.rb);MinimalFieldValue.Valuesis populated.field_filters[]accepts a newvalues: []stringslot mapped toMultiSelectOptionValues *[]githubv4.String, mirroring the input shape from github/github#435047.list_issuescalls and union the results.fetchIssueFieldValuesByNodeIDwas missing theghcontext.WithGraphQLFeatures("issue_fields", "repo_issue_fields")wrap, so without it the new fragment would have silently no-op'd on dotcom even with the right feature flags enabled.Out of scope / known follow-ups
multi_select_optionstyped slot onMinimalIssueFieldValue— deliberately skipped.issue_readclears REST field values and always re-fetches via GraphQL (fetchIssueFieldValuesByNodeID), so a typed REST slot would always be empty. The GraphQL fragment change in this PR covers the read path end-to-end. (go-github v87 also doesn't yet decode the REST field; worth picking up when it does.)set_issue_fieldsgranular tool inpkg/github/issues_granular.go— not touched in this PR. GraphQLIssueFieldCreateOrUpdateInputalready hasmulti_select_option_idsper github/github#433220, so wiring it through is a natural follow-up.field_option_names: []—IssueField#build_value_attributesrejects this with 422 (must contain at least one option) per github/github#433220. This PR matches that. If callers ever want "empty = clear the field" semantics (consistent with ProjectV2 / assignees / labels), that's a dotcom-side decision — usedelete: trueto clear today.MCP impact
issue_writeaddsfield_option_names;list_issuesfield_filtersaddsvalues;list_issue_fieldsreturns multi-select fields it previously dropped. Toolsnaps + generated docs updated.Prompts tested (tool changes only)
Validated via unit tests + existing GraphQL-string integration tests. End-to-end against dotcom needs a review-lab — see @kelsey-myers's testing guide. Example prompts the new code paths cover:
issue_writewithfield_option_names: ["Auth", "Billing"].list_issueswithfield_filters: [{field_name: "Components", values: ["Auth", "Billing"]}]. Note: filter end-to-end behaviour depends on github/github#435047 landing — the Go shape is final per that PR, but dotcom won't honour the input until it merges.Security / limits
issue_fields/repo_issue_fieldsGraphQL feature flags and the existing field-resolution / option-validation paths.Tool renaming
Lint & tests
./script/lint./script/testNew:
pkg/github/issues_multiselect_test.go(5 functions coveringparseRawFieldFilters,resolveFieldFilters,fragmentToMinimalFieldValue,IssueFieldRef.Name/FullDatabaseIDStr,optionalIssueWriteFields,resolveIssueRequestFieldValuesviagithubv4mock). Existing hardcoded GraphQL-string expectations inissues_test.gopatched to include the new inline fragments.Docs
script/generate-docsregeneratedREADME.md,docs/remote-server.md,docs/insiders-features.md,docs/feature-flags.md,docs/tool-renaming.md, plus 3 toolsnaps.cc @github/github-mcp-server