Apply deferred update mutations in parallel for gh issue edit

The Issues 2.0 mutations (issue type, parent set/remove, sub-issues,
blocked-by, blocking) are deferred until after the main UpdateIssue
because they target IDs that the standard mutation does not handle.

Move them behind a shared api.DeferredUpdateIssue orchestrator that
fans them out in parallel and joins all errors so a single failure
does not abort the rest.

editRun no longer carries its own applyEditParent / applyEditSubIssues
/ applyEditRelationships helpers; the per-issue goroutine resolves
refs to node IDs via a small deferredUpdateIssueOptions builder, then
hands the populated DeferredUpdateIssueOptions to api.DeferredUpdateIssue.

Also moves ResolveIssueRef and ResolveIssueTypeName from the deleted
resolve.go into lookup.go.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Kynan Ware 2026-05-12 19:38:55 -06:00
parent 315dafbf74
commit 02457482a5
5 changed files with 261 additions and 182 deletions

View file

@ -2,7 +2,9 @@ package api
import (
"encoding/json"
"errors"
"fmt"
"sync"
"time"
"github.com/cli/cli/v2/internal/ghrepo"
@ -606,6 +608,113 @@ func RemoveBlockedBy(client *Client, hostname string, issueID string, blockingIs
return client.Mutate(hostname, "RemoveBlockedBy", &mutation, variables)
}
// DeferredUpdateIssueOptions updates an issue with mutations unsupported by the
// standard issue update mutations. All ID fields are node IDs.
type DeferredUpdateIssueOptions struct {
IssueID string
Hostname string
IssueTypeID string
ParentID string
ReplaceExistingParent bool
RemoveParentID string
AddSubIssueIDs []string
RemoveSubIssueIDs []string
AddBlockedByIDs []string
RemoveBlockedByIDs []string
// AddBlockingIDs / RemoveBlockingIDs name issues that this issue
// blocks. They are applied via the addBlockedBy / removeBlockedBy
// mutations with the arguments swapped.
AddBlockingIDs []string
RemoveBlockingIDs []string
}
// DeferredUpdateIssue runs issue mutations described by opts in
// parallel and returns any failures as a single joined error so a single
// failure does not abort the rest.
func DeferredUpdateIssue(client *Client, opts DeferredUpdateIssueOptions) error {
var mutations []func() error
if opts.IssueTypeID != "" {
mutations = append(mutations, func() error {
return UpdateIssueIssueType(client, opts.Hostname, opts.IssueID, opts.IssueTypeID)
})
}
if opts.ParentID != "" {
mutations = append(mutations, func() error {
return AddSubIssue(client, opts.Hostname, opts.ParentID, opts.IssueID, opts.ReplaceExistingParent)
})
} else if opts.RemoveParentID != "" {
mutations = append(mutations, func() error {
return RemoveSubIssue(client, opts.Hostname, opts.RemoveParentID, opts.IssueID)
})
}
for _, id := range opts.AddSubIssueIDs {
mutations = append(mutations, func() error {
return AddSubIssue(client, opts.Hostname, opts.IssueID, id, false)
})
}
for _, id := range opts.RemoveSubIssueIDs {
mutations = append(mutations, func() error {
return RemoveSubIssue(client, opts.Hostname, opts.IssueID, id)
})
}
for _, id := range opts.AddBlockedByIDs {
mutations = append(mutations, func() error {
return AddBlockedBy(client, opts.Hostname, opts.IssueID, id)
})
}
for _, id := range opts.RemoveBlockedByIDs {
mutations = append(mutations, func() error {
return RemoveBlockedBy(client, opts.Hostname, opts.IssueID, id)
})
}
for _, id := range opts.AddBlockingIDs {
mutations = append(mutations, func() error {
// blocking is the inverse of blocked-by: this issue blocks `id`,
// expressed as `id` is blocked by this issue.
return AddBlockedBy(client, opts.Hostname, id, opts.IssueID)
})
}
for _, id := range opts.RemoveBlockingIDs {
mutations = append(mutations, func() error {
return RemoveBlockedBy(client, opts.Hostname, id, opts.IssueID)
})
}
if len(mutations) == 0 {
return nil
}
errCh := make(chan error, len(mutations))
var wg sync.WaitGroup
for _, m := range mutations {
wg.Add(1)
go func(m func() error) {
defer wg.Done()
if err := m(); err != nil {
errCh <- err
}
}(m)
}
wg.Wait()
close(errCh)
var errs []error
for err := range errCh {
errs = append(errs, err)
}
return errors.Join(errs...)
}
// RepoIssueTypes fetches the available issue types for a repository.
func RepoIssueTypes(client *Client, repo ghrepo.Interface) ([]IssueType, error) {
query := `

View file

@ -395,37 +395,18 @@ func editRun(opts *EditOptions) error {
go func(issue *api.Issue) {
defer g.Done()
err := prShared.UpdateIssue(httpClient, baseRepo, issue.ID, issue.IsPullRequest(), editable)
if err != nil {
if err := prShared.UpdateIssue(httpClient, baseRepo, issue.ID, issue.IsPullRequest(), editable); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update %s: %s", issue.URL, err)
return
}
// Issue type mutation
if editable.IssueType.Edited && editable.IssueType.Value != "" {
if err := api.UpdateIssueIssueType(apiClient, baseRepo.RepoHost(), issue.ID, issueTypeID); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update type for %s: %s", issue.URL, err)
return
}
}
// Parent mutation
if editable.Parent.Edited {
if err := applyEditParent(apiClient, baseRepo, issue, editable.Parent.Value); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update parent for %s: %s", issue.URL, err)
return
}
}
// Sub-issue mutations
if err := applyEditSubIssues(apiClient, baseRepo, issue, opts); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update sub-issues for %s: %s", issue.URL, err)
mutations, err := deferredUpdateIssueOptions(apiClient, baseRepo, issue, opts, &editable, issueTypeID)
if err != nil {
failedIssueChan <- fmt.Sprintf("failed to update %s: %s", issue.URL, err)
return
}
// Relationship mutations
if err := applyEditRelationships(apiClient, baseRepo, issue, opts); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update relationships for %s: %s", issue.URL, err)
if err := api.DeferredUpdateIssue(apiClient, mutations); err != nil {
failedIssueChan <- fmt.Sprintf("failed to update %s:\n%s", issue.URL, err)
return
}
@ -484,101 +465,72 @@ func lookupIssueTypeID(editable *prShared.Editable) (string, error) {
return id, nil
}
func applyEditParent(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, parentRef string) error {
hostname := baseRepo.RepoHost()
func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, editOpts *EditOptions, editable *prShared.Editable, issueTypeID string) (api.DeferredUpdateIssueOptions, error) {
updateOpts := api.DeferredUpdateIssueOptions{
IssueID: issue.ID,
Hostname: baseRepo.RepoHost(),
IssueTypeID: issueTypeID,
ReplaceExistingParent: true,
}
if parentRef == "" {
// Remove parent - use the parent's ID from the fetched issue data
if issue.Parent == nil {
return nil // no parent to remove
if editable.Parent.Edited {
if editable.Parent.Value == "" {
if issue.Parent != nil {
updateOpts.RemoveParentID = issue.Parent.ID
}
} else {
parentID, err := issueShared.ResolveIssueRef(client, baseRepo, editable.Parent.Value)
if err != nil {
return updateOpts, fmt.Errorf("resolving --set-parent reference %q: %w", editable.Parent.Value, err)
}
updateOpts.ParentID = parentID
}
return api.RemoveSubIssue(client, hostname, issue.Parent.ID, issue.ID)
}
// Set parent with replaceParent=true
parentID, err := issueShared.ResolveIssueRef(client, baseRepo, parentRef)
if err != nil {
return fmt.Errorf("resolving parent: %w", err)
for _, ref := range editOpts.AddSubIssues {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --add-sub-issue reference %q: %w", ref, err)
}
updateOpts.AddSubIssueIDs = append(updateOpts.AddSubIssueIDs, id)
}
return api.AddSubIssue(client, hostname, parentID, issue.ID, true)
}
func applyEditSubIssues(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *EditOptions) error {
hostname := baseRepo.RepoHost()
for _, ref := range opts.AddSubIssues {
subID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --add-sub-issue reference %q: %w", ref, err)
}
if err := api.AddSubIssue(client, hostname, issue.ID, subID, false); err != nil {
return err
}
}
for _, ref := range opts.RemoveSubIssues {
subID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --remove-sub-issue reference %q: %w", ref, err)
}
if err := api.RemoveSubIssue(client, hostname, issue.ID, subID); err != nil {
return err
}
}
return nil
}
func applyEditRelationships(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *EditOptions) error {
hasRelationshipFlags := len(opts.AddBlockedBy) > 0 || len(opts.RemoveBlockedBy) > 0 ||
len(opts.AddBlocking) > 0 || len(opts.RemoveBlocking) > 0
if !hasRelationshipFlags {
return nil
}
hostname := baseRepo.RepoHost()
for _, ref := range opts.AddBlockedBy {
blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --add-blocked-by reference %q: %w", ref, err)
}
if err := api.AddBlockedBy(client, hostname, issue.ID, blockingID); err != nil {
return err
}
}
for _, ref := range opts.RemoveBlockedBy {
blockingID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --remove-blocked-by reference %q: %w", ref, err)
}
if err := api.RemoveBlockedBy(client, hostname, issue.ID, blockingID); err != nil {
return err
}
}
for _, ref := range opts.AddBlocking {
// --add-blocking swaps args: the OTHER issue is blocked by THIS issue
blockedID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --add-blocking reference %q: %w", ref, err)
}
if err := api.AddBlockedBy(client, hostname, blockedID, issue.ID); err != nil {
return err
}
}
for _, ref := range opts.RemoveBlocking {
// --remove-blocking swaps args: the OTHER issue is no longer blocked by THIS issue
blockedID, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return fmt.Errorf("resolving --remove-blocking reference %q: %w", ref, err)
}
if err := api.RemoveBlockedBy(client, hostname, blockedID, issue.ID); err != nil {
return err
}
}
return nil
for _, ref := range editOpts.RemoveSubIssues {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --remove-sub-issue reference %q: %w", ref, err)
}
updateOpts.RemoveSubIssueIDs = append(updateOpts.RemoveSubIssueIDs, id)
}
for _, ref := range editOpts.AddBlockedBy {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --add-blocked-by reference %q: %w", ref, err)
}
updateOpts.AddBlockedByIDs = append(updateOpts.AddBlockedByIDs, id)
}
for _, ref := range editOpts.RemoveBlockedBy {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --remove-blocked-by reference %q: %w", ref, err)
}
updateOpts.RemoveBlockedByIDs = append(updateOpts.RemoveBlockedByIDs, id)
}
for _, ref := range editOpts.AddBlocking {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --add-blocking reference %q: %w", ref, err)
}
updateOpts.AddBlockingIDs = append(updateOpts.AddBlockingIDs, id)
}
for _, ref := range editOpts.RemoveBlocking {
id, err := issueShared.ResolveIssueRef(client, baseRepo, ref)
if err != nil {
return updateOpts, fmt.Errorf("resolving --remove-blocking reference %q: %w", ref, err)
}
updateOpts.RemoveBlockingIDs = append(updateOpts.RemoveBlockingIDs, id)
}
return updateOpts, nil
}

View file

@ -2,7 +2,9 @@ package edit
import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
@ -1052,34 +1054,31 @@ func Test_editRun(t *testing.T) {
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockIssueNumberGet(t, reg, 100)
reg.Register(
httpmock.GraphQL(`query IssueNodeID\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "issue": { "id": "SUB_123_ID" } } } }
`),
issueNodeIDByNumberMatcher(123),
httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "SUB_123_ID" } } } }`),
)
reg.Register(
httpmock.GraphQL(`mutation AddSubIssue\b`),
httpmock.GraphQLMutation(`
{ "data": { "addSubIssue": { "issue": { "id": "100" } } } }`,
issueNodeIDByNumberMatcher(124),
httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "SUB_124_ID" } } } }`),
)
reg.Register(
httpmock.GraphQLMutationMatcher(`mutation AddSubIssue\b`, func(input map[string]interface{}) bool {
return input["subIssueId"] == "SUB_123_ID"
}),
httpmock.GraphQLMutation(`{ "data": { "addSubIssue": { "issue": { "id": "100" } } } }`,
func(inputs map[string]interface{}) {
assert.Equal(t, "100", inputs["issueId"])
assert.Equal(t, "SUB_123_ID", inputs["subIssueId"])
assert.Equal(t, false, inputs["replaceParent"])
}),
)
reg.Register(
httpmock.GraphQL(`query IssueNodeID\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "issue": { "id": "SUB_124_ID" } } } }
`),
)
reg.Register(
httpmock.GraphQL(`mutation AddSubIssue\b`),
httpmock.GraphQLMutation(`
{ "data": { "addSubIssue": { "issue": { "id": "100" } } } }`,
httpmock.GraphQLMutationMatcher(`mutation AddSubIssue\b`, func(input map[string]interface{}) bool {
return input["subIssueId"] == "SUB_124_ID"
}),
httpmock.GraphQLMutation(`{ "data": { "addSubIssue": { "issue": { "id": "100" } } } }`,
func(inputs map[string]interface{}) {
assert.Equal(t, "100", inputs["issueId"])
assert.Equal(t, "SUB_124_ID", inputs["subIssueId"])
assert.Equal(t, false, inputs["replaceParent"])
}),
)
},
@ -1732,3 +1731,30 @@ func TestProjectsV1Deprecation(t *testing.T) {
reg.Verify(t)
})
}
// issueNodeIDByNumberMatcher matches an IssueNodeID GraphQL query whose
// number variable equals the given value. Used by tests that issue
// multiple IssueNodeID lookups and need stubs to route by issue number
// rather than by registration order.
func issueNodeIDByNumberMatcher(number int) httpmock.Matcher {
queryMatcher := httpmock.GraphQL(`query IssueNodeID\b`)
return func(req *http.Request) bool {
if !queryMatcher(req) {
return false
}
body, err := io.ReadAll(req.Body)
if err != nil {
return false
}
req.Body = io.NopCloser(bytes.NewReader(body))
var b struct {
Variables struct {
Number int `json:"number"`
} `json:"variables"`
}
if err := json.Unmarshal(body, &b); err != nil {
return false
}
return b.Variables.Number == number
}
}

View file

@ -208,3 +208,42 @@ func FindIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, f
return resp.Repository.Issue, nil
}
// ResolveIssueRef parses an issue reference (number or URL) and returns its
// node ID. References that point at a different host than baseRepo are
// rejected because relationship mutations require IDs from the base host.
func ResolveIssueRef(client *api.Client, baseRepo ghrepo.Interface, ref string) (string, error) {
number, repo, err := ParseIssueFromArg(ref)
if err != nil {
return "", err
}
targetRepo := baseRepo
if r, ok := repo.Value(); ok {
if r.RepoHost() != baseRepo.RepoHost() {
return "", fmt.Errorf("issue reference %q belongs to a different host (%s) than the current repository (%s)", ref, r.RepoHost(), baseRepo.RepoHost())
}
targetRepo = r
}
return api.IssueNodeID(client, targetRepo, number)
}
// ResolveIssueTypeName resolves an issue type name to its node ID by
// fetching the repository's available types.
func ResolveIssueTypeName(client *api.Client, repo ghrepo.Interface, typeName string) (string, error) {
issueTypes, err := api.RepoIssueTypes(client, repo)
if err != nil {
return "", err
}
typeNames := make([]string, len(issueTypes))
for i, t := range issueTypes {
typeNames[i] = t.Name
if strings.EqualFold(t.Name, typeName) {
return t.ID, nil
}
}
return "", fmt.Errorf("type %q not found; available types: %s", typeName, strings.Join(typeNames, ", "))
}

View file

@ -1,47 +0,0 @@
package shared
import (
"fmt"
"strings"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
)
// ResolveIssueRef parses an issue reference (number or URL) and returns its node ID.
func ResolveIssueRef(client *api.Client, baseRepo ghrepo.Interface, ref string) (string, error) {
number, repo, err := ParseIssueFromArg(ref)
if err != nil {
return "", err
}
targetRepo := baseRepo
if r, ok := repo.Value(); ok {
if r.RepoHost() != baseRepo.RepoHost() {
return "", fmt.Errorf("issue reference %q belongs to a different host (%s) than the current repository (%s)", ref, r.RepoHost(), baseRepo.RepoHost())
}
targetRepo = r
}
return api.IssueNodeID(client, targetRepo, number)
}
// ResolveIssueTypeName resolves an issue type name to its ID by fetching
// available types from the repository. Returns an error listing available
// types if the name is not found.
func ResolveIssueTypeName(client *api.Client, repo ghrepo.Interface, typeName string) (string, error) {
issueTypes, err := api.RepoIssueTypes(client, repo)
if err != nil {
return "", err
}
typeNames := make([]string, len(issueTypes))
for i, t := range issueTypes {
typeNames[i] = t.Name
if strings.EqualFold(t.Name, typeName) {
return t.ID, nil
}
}
return "", fmt.Errorf("type %q not found; available types: %s", typeName, strings.Join(typeNames, ", "))
}