Add support for issue fields values to issues.read and issues.write#2551
Add support for issue fields values to issues.read and issues.write#2551kelsey-myers wants to merge 6 commits into
issues.read and issues.write#2551Conversation
Tools tested on this branch
|
| param | type | required | notes |
|---|---|---|---|
owner |
string | yes | repo owner or org login |
repo |
string | no | if omitted, returns org-level fields |
Output: array of issue field definitions
[
{
"id": "IFSS_1",
"full_database_id": 99,
"name": "Priority",
"data_type": "SINGLE_SELECT",
"visibility": "ALL",
"options": [
{ "id": "OPT_1", "name": "High", "color": "red" },
{ "id": "OPT_2", "name": "Low", "color": "blue" }
]
},
{
"id": "IFT_2",
"full_database_id": 42,
"name": "DRI",
"data_type": "TEXT",
"visibility": "ORG_ONLY"
}
]issue_write (create / update)
New input: issue_fields — array of field values to set on the issue. Each item requires field_name and either value or field_option_name (not both).
{
"issue_fields": [
{ "field_name": "Priority", "field_option_name": "High" },
{ "field_name": "DRI", "value": "monalisa" },
{ "field_name": "Due Date", "value": "2026-06-01" }
]
}- Use
field_option_nameforSINGLE_SELECTfields — the server validates the option exists before calling the API - Use
valueforTEXT,NUMBER, andDATEfields
Output: unchanged — { "id": "...", "url": "..." }
issue_read (method: get)
Inputs: unchanged
Output: the response now includes a field_values array alongside the standard issue fields:
{
"number": 123,
"title": "...",
"field_values": [
{ "field_name": "Priority", "value": "High" },
{ "field_name": "DRI", "value": "monalisa" }
]
}The raw issue_field_values from the REST response is replaced with this normalised GraphQL-sourced field_values for consistency with list_issues and search_issues.
…abaseId - Expose fullDatabaseId (BigInt) in list_issue_fields - Add issue_fields parameter to issue_write for setting field values - Support single-select fields via field_option_name resolution - Add REST API field value extraction in get_issue responses - Update minimal types with IssueFieldValue for REST responses Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…turn minimal fields
70e8866 to
7b1b06c
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the issues tool surface to support reading and writing GitHub Issues 2.0 custom field values, aligning issue_read/issue_write behavior with the newer REST/GraphQL capabilities introduced alongside the go-github upgrade.
Changes:
- Added
issue_fieldsinput toissue_write, including name-based resolution of field IDs and (for single-select) option validation. - Expanded
issue_readoutput handling to include RESTissue_field_values, and added best-effort GraphQL enrichment to normalize tofield_valueswhere possible. - Updated minimal/DTO types and unit tests to cover issue field value parsing and write request shaping.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new issue_fields input for issue_write. |
| pkg/github/minimal_types.go | Adds REST-backed minimal issue field value types and separates REST issue_field_values from GraphQL-normalized field_values. |
| pkg/github/issues.go | Implements parsing + GraphQL metadata resolution for issue_fields, wires field values into create/update, and adjusts search/get enrichment paths. |
| pkg/github/issues_test.go | Adds/updates tests for get/write behavior involving issue field values. |
| pkg/github/issue_fields.go | Fetches/stores GraphQL fullDatabaseId for issue fields and adds parsing helper. |
| pkg/github/issue_fields_test.go | Updates expectations to include fullDatabaseId/DatabaseID. |
| pkg/github/toolsnaps/issue_write.snap | Updates tool schema snapshot to include issue_fields. |
| docs/insiders-features.md | Reflects the issue_fields parameter in generated docs. |
| docs/feature-flags.md | Reflects the issue_fields parameter in generated docs. |
Copilot's findings
Comments suppressed due to low confidence (1)
pkg/github/issues.go:1599
searchIssuesHandlernow always performs GraphQL field_values enrichment whenever there are results, and returns an error if GraphQL is unavailable or the query fails. This looks like a regression of the documented behavior ofFeatureFlagIssueFields(which is meant to gate search_issues field_values enrichment); please restore the feature-flag check so search_issues works without GraphQL issue fields features enabled.
var fieldValuesByID map[string][]MinimalFieldValue
if len(result.Issues) > 0 {
gqlClient, err := deps.GetGQLClient(ctx)
if err != nil {
return utils.NewToolResultErrorFromErr(errorPrefix+": failed to get GitHub GraphQL client", err), nil
}
fieldValuesByID, err = fetchIssueFieldValuesByNodeID(ctx, gqlClient, result.Issues)
if err != nil {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, errorPrefix+": failed to fetch issue field values", err), nil
}
}
- Files reviewed: 9/9 changed files
- Comments generated: 2
…ds schema with oneOf
|
cc @SamMorrowDrums 👋🏾 gated these changes behind the fields flag in #2553 to keep the diffs manageable |
| { | ||
| IssueFieldID: 1001, | ||
| NodeID: "FV_node_1", | ||
| DataType: "single_select", | ||
| Value: "High", | ||
| SingleSelectOption: &github.IssueFieldValueSingleSelectOption{ | ||
| ID: 42, | ||
| Name: "High", | ||
| Color: "red", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
I am thinking we should update the structure and only expose relevant fields
| { | |
| IssueFieldID: 1001, | |
| NodeID: "FV_node_1", | |
| DataType: "single_select", | |
| Value: "High", | |
| SingleSelectOption: &github.IssueFieldValueSingleSelectOption{ | |
| ID: 42, | |
| Name: "High", | |
| Color: "red", | |
| }, | |
| }, | |
| { | |
| DataType: "single_select", | |
| Value: "High", # this should be name | |
| SingleSelectOption: &github.IssueFieldValueSingleSelectOption{ | |
| Name: "High", # this should be also the name, not the value, which is the ID for single select options | |
| Color: "red", | |
| }, | |
| }, |
There was a problem hiding this comment.
Ah 👍🏼 you did this
IssueFieldSingleSelect struct {
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
Name githubv4.String
DataType githubv4.String
Options []struct {
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
Name githubv4.String
}
SamMorrowDrums
left a comment
There was a problem hiding this comment.
Tried this end-to-end against github/plan-track-agentic-toolkit with a local build of this branch — two things I'd want to resolve before this ships, plus a smaller follow-up.
🚨 issue_write with issue_fields is destructive: it wipes all unspecified field values
The REST PATCH /repos/{o}/{r}/issues/{n} fields array is a full replacement of the issue's custom field values, not a merge. The PR forwards the caller's subset straight through, so setting one field silently clears every other field.
Reproduced live against issue #33 in that repo:
- Starting state via
issue_read:field_values: [{Priority: P2}]. - Called
issue_writewithissue_fields: [{field_name:"Effort", field_option_name:"Low"}]. - Resulting state (confirmed via GraphQL):
[{Effort: Low}]— Priority is gone.
This is silent: the issue_write response is only {"id":..., "url":...} so it does not echo the resulting field set, and the model has no way to notice that other fields were destroyed. Omitting issue_fields entirely is safe (early return), and issue_fields: [] is also safe — the problem is specifically "supply a non-empty subset".
This feels like a release blocker on its own — agents calling issue_write to set one priority will routinely wipe DRI, Target Date, etc.
Suggested fixes, in order of preference:
- Fetch current field values, merge with the caller's input, and send the full merged set to REST. Closest match to the implicit contract of an MCP "update issue" tool.
- Or, at minimum, surface a very loud warning in the tool description that
issue_fieldsis a full replacement, and have the response echo the final set of field values so destruction is at least visible.
⚠️ No mechanism to unset / clear a field value
There's no path to clear a value. I tried:
value: null→ rejected client-side:value cannot be null for field "Effort".value: ""→ forwarded to REST, which 422s:Issue field option with name does not exist.- omitting the field from
issue_fields→ only "works" because of the destructive-overwrite bug above, which also wipes everything else. Not a real solution.
We need a documented delete: true (or equivalent) shape — the existing granular set_issue_fields tool already supports delete: true, so it would be good for the new issue_fields input to match. Without an unset path, agents told "remove the priority on this issue" have no way to do it.
⚠️ Server instructions don't teach the workflow (ideally gated by the same flag)
generateIssuesToolsetInstructions (the issues toolset's InstructionsFunc, rendered into the server's initialize instructions string) is static and still only mentions list_issue_types, search_issues, and state_reason. With this PR, when remote_mcp_issue_fields is on, the model suddenly sees field_values enriched into list_issues / issue_read output and a new issue_fields write input — but nothing in the server instructions tells it to call list_issue_fields (or read existing field_values first), prefer field_option_name over raw value for single-selects, or be aware of the replace-vs-merge semantics.
Other toolsets (e.g. projects) use this slot to teach the "list fields → use exact names → then write" pattern. Worth doing the same here. Ideally the additional issue-fields-specific guidance is appended only when the feature flag is enabled, so legacy clients aren't told about a surface they don't have. The InstructionsFunc(inv *Inventory) signature already gets the inventory, so flag-aware composition should be straightforward.
Happy to pair on any of the above if useful. The schema (oneOf of value xor field_option_name, additionalProperties: false), the field-name lookup with the nice "Known fields: …" error on list_issues, and the toolsnap coverage all look great — it's really just the write semantics I'd want pinned down before this lands.
|
Follow-up on blocker #1 — I checked whether this is REST behavior or wrapper behavior. It's the REST endpoint. Direct So the API contract for That doesn't change the recommendation, just shifts where the workaround has to live:
Worth checking with whoever owns the issues REST endpoint whether merge + per-field delete is on the table before we decide which workaround to ship. |
|
Dug into the monolith to figure out where the "Add" vs "Set" surprise comes from. tl;dr: the upstream already has the API shape we want, this PR is just calling the wrong endpoint. What's actually on the server (
|
| Endpoint | Mode | Behavior |
|---|---|---|
POST /repos/{o}/{r}/issues/{n}/issue-field-values |
add |
Additive merge — never deletes a value not in the payload (only clears all when body is []) |
PUT /repos/{o}/{r}/issues/{n}/issue-field-values |
set |
Full replace — anything not in the payload is deleted |
DELETE /repos/{o}/{r}/issues/{n}/issue-field-values/{field_id} |
n/a | Clears a single value |
PATCH /repos/{o}/{r}/issues/{n} with issue_field_values:[…] |
set |
Same destructive replace as PUT (gated on IssueFieldsFeature for the org, otherwise silently ignored) |
So the destructive overwrite I reported is intentional behavior of PATCH /issues/{n}, but it's only one of four documented shapes — and the "additive" shape we want for issue_write already exists as a first-class endpoint.
Why the surprise: PATCH vs POST share a helper, opposite modes
The Copilot review on the monolith PR that introduced these (#395944) summarises it as "adds POST and PUT endpoints for managing issue field values" and explicitly contrasts "POST for adding field values and PUT for setting/replacing". So:
- If you tested by hitting the dedicated POST endpoint → additive, no surprises.
- If you tested by hitting PATCH on the issue → destructive replace.
Both paths route into the same get_issue_field_update_attributes helper but with mode: "add" vs mode: "set" respectively. Easy to miss.
Simple recommendation
Stop putting issue_field_values in the PATCH /issues/{n} body, and instead call the dedicated endpoints from the issue_write handler:
issue_write …, issue_fields: [ {field_name, value|field_option_name, delete?: bool} ]
│
├── any items with delete: true → for each: DELETE …/issue-field-values/{field_id}
└── any items with values → POST …/issue-field-values (additive merge)
That gives us:
- Non-destructive merge for the common case ("set Priority" doesn't wipe DRI).
- A real unset path via the
DELETEsub-resource — addresses the open question about clearing a value. - The existing granular
set_issue_fieldstool already exposesdelete: trueper field via the GraphQLsetIssueFieldValuemutation, so the input shape stays consistent across both tools. - One extra HTTP call only when the caller actually wants to delete; the additive POST handles add+update in a single round-trip just like the current PATCH.
Trade-off: the monolith endpoints aren't in go-github v0.87 yet, but they're small enough to hit via the http client directly. Happy to sketch that out if useful.
PATCH /issues/{n} with issue_field_values would stay available for callers who explicitly want full replace, but issue_write shouldn't be that caller.
Summary
This PR is passing issue field values to the issue write & read tools, which now (after #2452) support this.
issues_writeis using that endpointWhy
What changed
MCP impact
Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: 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
./script/lint./script/testDocs