Skip to content

Add support for issue fields values to issues.read and issues.write#2492

Open
iulia-b wants to merge 8 commits into
github:mainfrom
iulia-b:iunia/reapply-issue-field-commits
Open

Add support for issue fields values to issues.read and issues.write#2492
iulia-b wants to merge 8 commits into
github:mainfrom
iulia-b:iunia/reapply-issue-field-commits

Conversation

@iulia-b
Copy link
Copy Markdown
Contributor

@iulia-b iulia-b commented May 18, 2026

Summary

This PR is passing issue field values to the issue write & read tools, which now (after #2452) support this.

Why

What changed

  • the go-gothub lib was updated to pass issue field value sto rest api
  • the repo upgraded to use v0.87 of go-github
  • we're now using new params for IssueFieldValues

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

  • added tests to make sure that passing fields works, as well as retrieving them

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

@iulia-b iulia-b self-assigned this May 18, 2026
@iulia-b iulia-b marked this pull request as ready for review May 18, 2026 13:32
@iulia-b iulia-b requested a review from a team as a code owner May 18, 2026 13:32
Copilot AI review requested due to automatic review settings May 18, 2026 13:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR wires the new IssueFieldValues support (added by the go-github v0.87 upgrade in PR #2452) into the issue_write and issue_read tools. On write, users can pass an issue_fields array specifying field names plus either a raw value or a single-select option name; the server resolves names → IDs via a GraphQL metadata query before calling the REST API. On read, custom field values are now surfaced in the trimmed MinimalIssue output.

Changes:

  • Adds issue_fields input parameter to issue_write (create/update), including a GraphQL-based resolver that maps field/option names to database IDs.
  • Extends MinimalIssue with an issue_field_values array (including single-select option details) populated from the REST response.
  • Updates README, toolsnap, and unit tests covering the new read/write behavior.
Show a summary per file
File Description
pkg/github/issues.go Adds input parsing (optionalIssueWriteFields) and GraphQL-based name→ID resolution; threads IssueFieldValues through CreateIssue/UpdateIssue.
pkg/github/minimal_types.go New MinimalIssueFieldValue/MinimalIssueFieldValueSingleSelectOption types and conversion logic.
pkg/github/issues_test.go Adds tests for reading field values and writing with field-name resolution, plus validation error case.
pkg/github/toolsnaps/issue_write.snap Snapshot updated for the new issue_fields schema entry.
README.md Documents the new issue_fields parameter for issue_write.

Copilot's findings

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment thread pkg/github/issues.go
Comment on lines +60 to +66
type issueFieldMetadataQuery struct {
Repository struct {
IssueFields struct {
Nodes []issueFieldMetadataNode
} `graphql:"issueFields(first: 100)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's ok we have 25 fields / org limit

Comment thread pkg/github/issues.go
issueFields := make([]IssueWriteFieldInput, 0, len(inputMaps))
for _, itemMap := range inputMaps {
fieldName, err := RequiredParam[string](itemMap, "field_name")
if err != nil || strings.TrimSpace(fieldName) == "" {
@SamMorrowDrums
Copy link
Copy Markdown
Collaborator

Tested this end-to-end with a Docker build in HTTP mode against github/issues (which has live issue fields). Two findings worth addressing before merge — one is a real blocker.

🚨 Blocker: issueFieldMetadataQuery is malformed against the live GraphQL schema

When issue_write is called with any issue_fields, the server returns:

failed to resolve issue_fields: failed to query issue fields metadata:
Selections can't be made directly on unions (see selections on IssueFields)

issueFieldMetadataNode selects databaseId / name / dataType directly on IssueFields, which is a union (IssueFieldDate | IssueFieldNumber | IssueFieldSingleSelect | IssueFieldText). GraphQL rejects scalar selections on unions — they have to live inside inline fragments per concrete type. The existing IssueFieldRef (≈ line 141) in this same file already shows the correct pattern.

The unit tests pass only because githubv4mock doesn't validate the query against the real schema. Reproduction is trivial — pointing the built binary at any repo with fields and calling issue_write { issue_fields: [...] } produces the error above regardless of field name.

Suggested shape (mirrors IssueFieldRef):

type issueFieldMetadataNode struct {
    Date         struct { DatabaseID githubv4.Int `graphql:"databaseId"`; Name githubv4.String; DataType githubv4.String } `graphql:"... on IssueFieldDate"`
    Number       struct { DatabaseID githubv4.Int `graphql:"databaseId"`; Name githubv4.String; DataType githubv4.String } `graphql:"... on IssueFieldNumber"`
    SingleSelect struct {
        DatabaseID githubv4.Int `graphql:"databaseId"`
        Name       githubv4.String
        DataType   githubv4.String
        Options    []issueFieldMetadataOption `graphql:"options"`
    } `graphql:"... on IssueFieldSingleSelect"`
    Text         struct { DatabaseID githubv4.Int `graphql:"databaseId"`; Name githubv4.String; DataType githubv4.String } `graphql:"... on IssueFieldText"`
}

…and a small helper to pick the populated variant. A real e2e (or live smoke) for this path would have caught it — worth adding.

🟡 OpenAI strict-mode: issue_fields items schema will be rejected by Responses API strict callers

The new issue_fields.items schema in the issue_write toolsnap is:

  • missing additionalProperties: false
  • has a value property with no type

These are the same patterns that produced the HTTP 400 in #2011 (push_files) and the silent-drop in #1877 / #2417. Suggested change (verified locally that lint + tests + the served HTTP schema all look correct):

Items: &jsonschema.Schema{
    Type:                 "object",
    AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}},
    Properties: map[string]*jsonschema.Schema{
        "field_name":        {Type: "string", Description: "..."},
        "field_option_name": {Type: "string", Description: "..."},
        "value": {
            Types:       []string{"string", "number", "boolean"},
            Description: "...",
        },
    },
    Required: []string{"field_name"},
},

…then UPDATE_TOOLSNAPS=true go test ./pkg/github/ to refresh the snap.

🟡 Minor: issueFields(first: 100) silently truncates

If a repo has more than 100 field definitions, fields beyond #100 become "field not found" errors with no signal as to why. Select pageInfo { hasNextPage } and return an explicit error if the cap is hit. (The existing issueFieldValues(first: 25) is per-issue and unrelated — leave that alone.)


Note on the dual IssueFieldValues (REST) / FieldValues (GraphQL) shapes on MinimalIssue: traced both code paths, they're mutually exclusive (convertToMinimalIssue vs fragmentToMinimalIssue populate one each), so no logic bug — just a bit of API surface duplication that's fine to leave.

@SamMorrowDrums
Copy link
Copy Markdown
Collaborator

Update after testing this against the live GitHub GraphQL + REST APIs — the GraphQL bug is the tip of the iceberg, the resolver is broken in more ways. Posting findings so we can decide how to land this.

1. databaseId doesn't exist on IssueField* GraphQL types

Confirmed via introspection. IssueFieldDate, IssueFieldNumber, IssueFieldSingleSelect, IssueFieldText all implement IssueFieldCommon + Node. Available fields: createdAt, dataType, description, id, name, options, visibility. No databaseId on any of them.

gh api graphql -f query='{ repository(owner:"github",name:"issues"){ issueFields(first:3){ nodes{ ... on IssueFieldSingleSelect { databaseId name } } } } }'
=> Field 'databaseId' doesn't exist on type 'IssueFieldSingleSelect'

So the only way to get an int field_id for REST is to decode the GraphQL Node ID (IFSS_kgAB → base64 → msgpack [0,1]databaseId=1). Confirmed the decoded value matches issue_field_id returned by REST GET /repos/.../issues/N.

2. REST single-select wants the option name, not the option id

Tested directly:

PATCH .../issues/21789 {"issue_field_values":[{"field_id":1,"value":"P1"}]}    → 200 OK
PATCH .../issues/21789 {"issue_field_values":[{"field_id":1,"value":1}]}        → 422 "Issue field option with name 1 does not exist"

The resolver currently does resolvedValue = optionID (int64), which the API rejects on every single-select write. The whole option-name → option-id GraphQL lookup is unnecessary — for single-select, just pass the option name through as value (string).

3. REST does not accept field_name

Tested both {"field_name":"Priority",...} and {"name":"Priority",...} — both return "field_id" wasn't supplied. So we still need a name → int-id mapping somewhere.

4. (Recap) OpenAI strict-mode schema gap

issue_fields.items missing additionalProperties: false, and value has no type. Same patterns as #2011 / #1877. Fix is small — see my earlier comment.


Suggested redesign

  • Replace the broken metadata struct with an interface fragment that the schema actually supports:
    type issueFieldMetadataNode struct {
        Common struct { Name githubv4.String; DataType githubv4.String } `graphql:"... on IssueFieldCommon"`
        Node   struct { ID githubv4.ID }                                   `graphql:"... on Node"`
    }
  • Decode Node.ID (IFSS_kgAB → trailing int) to get field_id. There may be a tidier helper for this in the codebase already; happy to extract one if not.
  • Drop option-id resolution entirely. Both field_option_name and value (when set on a single-select) should be forwarded as the value string.
  • Add a real e2e against a repo with fields (e.g. github/issues) — none of this would have been caught by the existing mocked unit tests.

If we can't decode Node IDs comfortably (or don't want to depend on the internal encoding), an alternative is to wait for a REST list issue fields endpoint, or change the tool contract to require the user to pass a numeric field_id directly. Either of those is a meaningful scope change.

Happy to take a stab at the redesign if it's helpful, but flagging that the feature does not work against live GitHub today.

@SamMorrowDrums
Copy link
Copy Markdown
Collaborator

Hey @iulia-b — flagging that #2520 just landed a feature flag (remote_mcp_issue_fields, also auto-enabled in insiders) gating the previously merged issue-fields work (list_issue_fields, field_filters on list_issues, field_values enrichment on search_issues/list_issues). We'd like the new changes in this PR to ride the same flag so the whole capability rolls out atomically.

Here's the suggested gating for what this PR adds, mirroring the pattern from #2520:

1. get_issue (IssueRead) — output enrichment

Pure runtime gate, no schema change. Add an IsFeatureEnabled(ctx, FeatureFlagIssueFields) check around the GraphQL issueFieldValues fetch (same as searchIssuesHandler in #2520). No new toolsnap needed.

2. issue_write — input schema change (issue_fields param)

This one is trickier because issue_write is already gated by FeatureFlagDisable = FeatureFlagIssuesGranular, and we need to layer a second condition (AND FeatureFlagEnable = remote_mcp_issue_fields).

Two options:

Option A — extend flag fields to []string (recommended). Change inventory.ServerTool.FeatureFlagEnable/Disable from string to []string with AND semantics in pkg/inventory/filters.go, update tools_validation_test.go duplicate-name handling, and regenerate. Then this PR registers a flag-on issue_write clone with FeatureFlagDisable = [FeatureFlagIssuesGranular] + FeatureFlagEnable = [FeatureFlagIssueFields] alongside the legacy one. Same <tool>_ff_<flag>.snap snapshot convention from #2520 applies.

Option B — duplicate-then-delete clone (lower-infra). Keep flag fields as single strings; ship the field-aware path as a new IssueWriteWithFields tool (still named issue_write) and don't touch the existing one. Same name + both having FeatureFlagDisable = FeatureFlagIssuesGranular won't disambiguate cleanly though — so Option A is really the only clean route once a second flag axis exists.

In both cases, the legacy code path stays untouched and can be deleted wholesale when the flag is rolled out (the design we adopted in #2520 — duplication for the sake of the flag is OK).

3. MinimalFieldValue rename and CreateIssue/UpdateIssue signature changes

These are internal and used by both code paths, so they don't need gating — just keep them as-is.

Bonus

  • Add FeatureFlagIssueFields to the toolsnap name for the gated variant: pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap.
  • script/print-mcp-diff-configs already auto-includes --features=remote_mcp_issue_fields, so the MCP diff matrix will cover both states.

Happy to push a stacked PR against this branch with the gating wired up so you can just merge it in — let me know and I'll kick that off.

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.

4 participants