Skip to content

Commit 8c16c3b

Browse files
owenniblockCopilot
andcommitted
Fix delete:true on issue fields by calling deleteIssueFieldValue mutation
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 from the JSON, so when the kept list ended up empty (e.g. deleting the only remaining field value) 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. The GraphQL mutation is idempotent, per-field, and not subject to the omitempty silent-no-op. It also sidesteps the race window in the old fetch-merge-PATCH approach (where a concurrent edit between our fetch and our PATCH would be silently clobbered by the set semantics). Verified end-to-end: - Test_IssueRequest_EmptyFieldValues_OmittedByJSON pins the omitempty contract so we know if go-github ever drops the tag. - Test_UpdateIssue_DeleteFieldValueRunsGraphQLMutation deletes the only existing field value, captures the REST PATCH body, asserts it has no issue_field_values key, and asserts (via mutation matcher) that the GraphQL clear fires. - Test_UpdateIssue_DeleteAndSetFieldsInSameCall does a combined set-one-delete-another in a single call and verifies the REST PATCH carries only the set operation while the GraphQL mutation fires for the deletion. Empirical confirmation of the bug on main (pre-fix): running an equivalent flow there results in a literal PATCH body of '{}' and no follow-up mutation — the field is never cleared on the server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a378370 commit 8c16c3b

3 files changed

Lines changed: 389 additions & 15 deletions

File tree

pkg/github/issues.go

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -121,21 +121,25 @@ func getCloseStateReason(stateReason string) IssueClosedStateReason {
121121
type issueFieldWriteMetadataNode struct {
122122
TypeName githubv4.String `graphql:"__typename"`
123123
IssueFieldText struct {
124+
ID githubv4.ID
124125
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
125126
Name githubv4.String
126127
DataType githubv4.String
127128
} `graphql:"... on IssueFieldText"`
128129
IssueFieldNumber struct {
130+
ID githubv4.ID
129131
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
130132
Name githubv4.String
131133
DataType githubv4.String
132134
} `graphql:"... on IssueFieldNumber"`
133135
IssueFieldDate struct {
136+
ID githubv4.ID
134137
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
135138
Name githubv4.String
136139
DataType githubv4.String
137140
} `graphql:"... on IssueFieldDate"`
138141
IssueFieldSingleSelect struct {
142+
ID githubv4.ID
139143
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
140144
Name githubv4.String
141145
DataType githubv4.String
@@ -299,7 +303,15 @@ func optionalIssueWriteFields(args map[string]any) ([]issueWriteFieldInput, erro
299303
return issueFields, nil
300304
}
301305

302-
func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]*github.IssueRequestFieldValue, []int64, error) {
306+
// fieldDeletion carries both identifiers needed to clear an issue field value:
307+
// the REST/v3 database ID (used to exclude the field from the REST PATCH payload)
308+
// and the GraphQL node ID (used as input to the deleteIssueFieldValue mutation).
309+
type fieldDeletion struct {
310+
DatabaseID int64
311+
NodeID githubv4.ID
312+
}
313+
314+
func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]*github.IssueRequestFieldValue, []fieldDeletion, error) {
303315
if len(issueFields) == 0 {
304316
return nil, nil, nil
305317
}
@@ -334,27 +346,32 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli
334346
}
335347

336348
resolved := make([]*github.IssueRequestFieldValue, 0, len(issueFields))
337-
var fieldIDsToDelete []int64
349+
var fieldsToDelete []fieldDeletion
338350
for _, fieldInput := range issueFields {
339351
node, ok := fieldByName[strings.ToLower(strings.TrimSpace(fieldInput.FieldName))]
340352
if !ok {
341353
return nil, nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo)
342354
}
343355

344356
var fullDatabaseIDStr, dataType string
357+
var nodeID githubv4.ID
345358
switch string(node.TypeName) {
346359
case "IssueFieldText":
347360
fullDatabaseIDStr = string(node.IssueFieldText.FullDatabaseID)
348361
dataType = string(node.IssueFieldText.DataType)
362+
nodeID = node.IssueFieldText.ID
349363
case "IssueFieldNumber":
350364
fullDatabaseIDStr = string(node.IssueFieldNumber.FullDatabaseID)
351365
dataType = string(node.IssueFieldNumber.DataType)
366+
nodeID = node.IssueFieldNumber.ID
352367
case "IssueFieldDate":
353368
fullDatabaseIDStr = string(node.IssueFieldDate.FullDatabaseID)
354369
dataType = string(node.IssueFieldDate.DataType)
370+
nodeID = node.IssueFieldDate.ID
355371
case "IssueFieldSingleSelect":
356372
fullDatabaseIDStr = string(node.IssueFieldSingleSelect.FullDatabaseID)
357373
dataType = string(node.IssueFieldSingleSelect.DataType)
374+
nodeID = node.IssueFieldSingleSelect.ID
358375
}
359376

360377
fieldID := parseFullDatabaseID(fullDatabaseIDStr)
@@ -363,7 +380,7 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli
363380
}
364381

365382
if fieldInput.Delete {
366-
fieldIDsToDelete = append(fieldIDsToDelete, fieldID)
383+
fieldsToDelete = append(fieldsToDelete, fieldDeletion{DatabaseID: fieldID, NodeID: nodeID})
367384
continue
368385
}
369386

@@ -394,7 +411,7 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli
394411
})
395412
}
396413

397-
return resolved, fieldIDsToDelete, nil
414+
return resolved, fieldsToDelete, nil
398415
}
399416

400417
// fetchExistingIssueFieldValues retrieves the current field values for an issue
@@ -1999,9 +2016,9 @@ Options are:
19992016
}
20002017

20012018
var issueFieldValues []*github.IssueRequestFieldValue
2002-
var fieldIDsToDelete []int64
2019+
var fieldsToDelete []fieldDeletion
20032020
if len(issueFields) > 0 {
2004-
issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields)
2021+
issueFieldValues, fieldsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields)
20052022
if err != nil {
20062023
return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil
20072024
}
@@ -2016,7 +2033,7 @@ Options are:
20162033
if err != nil {
20172034
return utils.NewToolResultError(err.Error()), nil, nil
20182035
}
2019-
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{
2036+
result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{
20202037
AssigneesProvided: assigneesProvided,
20212038
LabelsProvided: labelsProvided,
20222039
})
@@ -2091,7 +2108,7 @@ type UpdateIssueOptions struct {
20912108
LabelsProvided bool
20922109
}
20932110

2094-
func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) {
2111+
func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldsToDelete []fieldDeletion, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) {
20952112
updateOptions := UpdateIssueOptions{
20962113
AssigneesProvided: len(assignees) > 0,
20972114
LabelsProvided: len(labels) > 0,
@@ -2129,19 +2146,23 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
21292146
issueRequest.Type = github.Ptr(issueType)
21302147
}
21312148

2132-
if len(issueFieldValues) > 0 || len(fieldIDsToDelete) > 0 {
2149+
if len(issueFieldValues) > 0 {
21332150
// The REST update endpoint uses "set" semantics — it overwrites all existing
2134-
// field values with whatever is sent. Fetch the current values first, merge in
2135-
// the new values, then remove any explicitly deleted fields.
2151+
// field values with whatever is sent. Fetch the current values first and
2152+
// merge the new values on top so untouched fields survive. Anything marked
2153+
// for deletion is NOT included in the REST payload (the REST PATCH can't
2154+
// clear values — Go's `omitempty` strips an empty issue_field_values array
2155+
// from the JSON, so even sending the cleared list is a silent no-op); we
2156+
// follow up with deleteIssueFieldValue GraphQL mutations below.
21362157
existing, err := fetchExistingIssueFieldValues(ctx, gqlClient, owner, repo, issueNumber)
21372158
if err != nil {
21382159
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to fetch existing issue field values", err), nil
21392160
}
21402161
merged := mergeIssueFieldValues(existing, issueFieldValues)
2141-
if len(fieldIDsToDelete) > 0 {
2142-
deleteSet := make(map[int64]bool, len(fieldIDsToDelete))
2143-
for _, id := range fieldIDsToDelete {
2144-
deleteSet[id] = true
2162+
if len(fieldsToDelete) > 0 {
2163+
deleteSet := make(map[int64]bool, len(fieldsToDelete))
2164+
for _, d := range fieldsToDelete {
2165+
deleteSet[d.DatabaseID] = true
21452166
}
21462167
kept := make([]*github.IssueRequestFieldValue, 0, len(merged))
21472168
for _, v := range merged {
@@ -2172,6 +2193,33 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4
21722193
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to update issue", resp, body), nil
21732194
}
21742195

2196+
// Clear any fields marked with delete:true via the GraphQL deleteIssueFieldValue
2197+
// mutation. REST alone can't do this — sending an empty `issue_field_values: []`
2198+
// would be stripped by omitempty before reaching the server. One mutation per
2199+
// deleted field; failures bubble up immediately so callers see the first error.
2200+
if len(fieldsToDelete) > 0 {
2201+
issueID, _, err := fetchIssueIDs(ctx, gqlClient, owner, repo, issueNumber, 0)
2202+
if err != nil {
2203+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to look up issue for field deletion", err), nil
2204+
}
2205+
for _, deletion := range fieldsToDelete {
2206+
var mutation struct {
2207+
DeleteIssueFieldValue struct {
2208+
Issue struct {
2209+
Number githubv4.Int
2210+
}
2211+
} `graphql:"deleteIssueFieldValue(input: $input)"`
2212+
}
2213+
input := DeleteIssueFieldValueInput{
2214+
IssueID: issueID,
2215+
FieldID: deletion.NodeID,
2216+
}
2217+
if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil {
2218+
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "failed to clear issue field value", err), nil
2219+
}
2220+
}
2221+
}
2222+
21752223
// Use GraphQL API for state updates
21762224
if state != "" {
21772225
// Mandate specifying duplicateOf when trying to close as duplicate

0 commit comments

Comments
 (0)