From 87b3d6c230773d7e764e285454472ffba87ff7d5 Mon Sep 17 00:00:00 2001 From: Kai Armstrong Date: Tue, 9 Dec 2025 14:22:04 -0600 Subject: [PATCH] fix: resolve host from URL for all issue operations When issue commands are invoked with a self-managed GitLab URL outside of a git repository, subsequent API operations (such as fetching comments, creating notes, or updating issues) would incorrectly fall back to gitlab.com instead of using the host from the provided URL. This occurred because IssueFromArg created a new API client with the correct host from the URL, but this client was assigned to a local parameter variable and was not returned to the caller. The caller continued using the original client pointing to the wrong host. Fixes https://gitlab.com/gitlab-org/cli/-/issues/8084 --- .../commands/issuable/close/issuable_close.go | 2 +- .../issuable/note/issuable_note_create.go | 2 +- .../issuable/reopen/issuable_reopen.go | 2 +- .../issuable/subscribe/issuable_subscribe.go | 2 +- .../unsubscribe/issuable_unsubscribe.go | 2 +- .../commands/issuable/view/issuable_view.go | 2 +- .../commands/issue/delete/issue_delete.go | 2 +- internal/commands/issue/issueutils/utils.go | 57 +++++++++++++------ .../commands/issue/update/issue_update.go | 4 +- internal/commands/mr/create/mr_create.go | 2 +- 10 files changed, 50 insertions(+), 27 deletions(-) diff --git a/internal/commands/issuable/close/issuable_close.go b/internal/commands/issuable/close/issuable_close.go index af56593ae..96ddc7866 100644 --- a/internal/commands/issuable/close/issuable_close.go +++ b/internal/commands/issuable/close/issuable_close.go @@ -53,7 +53,7 @@ func NewCmdClose(f cmdutils.Factory, issueType issuable.IssueType) *cobra.Comman return err } - issues, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) + issues, client, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) if err != nil { return err } diff --git a/internal/commands/issuable/note/issuable_note_create.go b/internal/commands/issuable/note/issuable_note_create.go index b6a55843e..73ddf271e 100644 --- a/internal/commands/issuable/note/issuable_note_create.go +++ b/internal/commands/issuable/note/issuable_note_create.go @@ -35,7 +35,7 @@ func NewCmdNote(f cmdutils.Factory, issueType issuable.IssueType) *cobra.Command return err } - issue, repo, err := issueutils.IssueFromArg(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args[0]) + issue, client, repo, err := issueutils.IssueFromArg(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args[0]) if err != nil { return err } diff --git a/internal/commands/issuable/reopen/issuable_reopen.go b/internal/commands/issuable/reopen/issuable_reopen.go index fb94399e9..1836d80cc 100644 --- a/internal/commands/issuable/reopen/issuable_reopen.go +++ b/internal/commands/issuable/reopen/issuable_reopen.go @@ -63,7 +63,7 @@ func NewCmdReopen(f cmdutils.Factory, issueType issuable.IssueType) *cobra.Comma return err } - issues, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) + issues, client, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) if err != nil { return err } diff --git a/internal/commands/issuable/subscribe/issuable_subscribe.go b/internal/commands/issuable/subscribe/issuable_subscribe.go index c63c05667..e1605fb3f 100644 --- a/internal/commands/issuable/subscribe/issuable_subscribe.go +++ b/internal/commands/issuable/subscribe/issuable_subscribe.go @@ -52,7 +52,7 @@ func NewCmdSubscribe(f cmdutils.Factory, issueType issuable.IssueType) *cobra.Co return err } - issues, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) + issues, client, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) if err != nil { return err } diff --git a/internal/commands/issuable/unsubscribe/issuable_unsubscribe.go b/internal/commands/issuable/unsubscribe/issuable_unsubscribe.go index 0aa87a03d..a345d48aa 100644 --- a/internal/commands/issuable/unsubscribe/issuable_unsubscribe.go +++ b/internal/commands/issuable/unsubscribe/issuable_unsubscribe.go @@ -52,7 +52,7 @@ func NewCmdUnsubscribe(f cmdutils.Factory, issueType issuable.IssueType) *cobra. return err } - issues, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) + issues, client, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) if err != nil { return err } diff --git a/internal/commands/issuable/view/issuable_view.go b/internal/commands/issuable/view/issuable_view.go index 39fc08911..cb19515e7 100644 --- a/internal/commands/issuable/view/issuable_view.go +++ b/internal/commands/issuable/view/issuable_view.go @@ -110,7 +110,7 @@ func (o *options) run(issueType issuable.IssueType, args []string) error { } cfg := o.config() - issue, baseRepo, err := issueutils.IssueFromArg(o.apiClient, client, o.baseRepo, o.defaultHostname, args[0]) + issue, client, baseRepo, err := issueutils.IssueFromArg(o.apiClient, client, o.baseRepo, o.defaultHostname, args[0]) if err != nil { return err } diff --git a/internal/commands/issue/delete/issue_delete.go b/internal/commands/issue/delete/issue_delete.go index 0644aecc0..ed053d110 100644 --- a/internal/commands/issue/delete/issue_delete.go +++ b/internal/commands/issue/delete/issue_delete.go @@ -46,7 +46,7 @@ func NewCmdDelete(f cmdutils.Factory) *cobra.Command { return err } - issues, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) + issues, client, repo, err := issueutils.IssuesFromArgs(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args) if err != nil { return err } diff --git a/internal/commands/issue/issueutils/utils.go b/internal/commands/issue/issueutils/utils.go index 87988d7fc..0d708b99e 100644 --- a/internal/commands/issue/issueutils/utils.go +++ b/internal/commands/issue/issueutils/utils.go @@ -65,50 +65,73 @@ func IssueState(c *iostreams.ColorPalette, i *gitlab.Issue) string { } } -func IssuesFromArgs(apiClientFunc func(repoHost string) (*api.Client, error), gitlabClient *gitlab.Client, baseRepoFn func() (glrepo.Interface, error), defaultHostname string, args []string) ([]*gitlab.Issue, glrepo.Interface, error) { - var baseRepo glrepo.Interface - +func IssuesFromArgs(apiClientFunc func(repoHost string) (*api.Client, error), gitlabClient *gitlab.Client, baseRepoFn func() (glrepo.Interface, error), defaultHostname string, args []string) ([]*gitlab.Issue, *gitlab.Client, glrepo.Interface, error) { if len(args) <= 1 { if len(args) == 1 { args = strings.Split(args[0], ",") } if len(args) <= 1 { - issue, repo, err := IssueFromArg(apiClientFunc, gitlabClient, baseRepoFn, defaultHostname, args[0]) + issue, client, baseRepo, err := IssueFromArg(apiClientFunc, gitlabClient, baseRepoFn, defaultHostname, args[0]) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - baseRepo = repo - return []*gitlab.Issue{issue}, baseRepo, err + return []*gitlab.Issue{issue}, client, baseRepo, err } } + // For multiple issues, we fetch them concurrently but they should all be from the same repo. + // We use the first successful result to determine the client and repo. + type result struct { + issue *gitlab.Issue + client *gitlab.Client + repo glrepo.Interface + index int + } + errGroup, _ := errgroup.WithContext(context.Background()) + results := make(chan result, len(args)) issues := make([]*gitlab.Issue, len(args)) + for i, arg := range args { i, arg := i, arg errGroup.Go(func() error { - issue, repo, err := IssueFromArg(apiClientFunc, gitlabClient, baseRepoFn, defaultHostname, arg) + issue, cl, repo, err := IssueFromArg(apiClientFunc, gitlabClient, baseRepoFn, defaultHostname, arg) if err != nil { return err } - baseRepo = repo - issues[i] = issue + results <- result{issue: issue, client: cl, repo: repo, index: i} return nil }) } + + // Wait for all goroutines to complete if err := errGroup.Wait(); err != nil { - return nil, nil, err + close(results) + return nil, nil, nil, err } - return issues, baseRepo, nil + close(results) + + // Collect results and use the first one for client and repo + var client *gitlab.Client + var baseRepo glrepo.Interface + for res := range results { + issues[res.index] = res.issue + if client == nil { + client = res.client + baseRepo = res.repo + } + } + + return issues, client, baseRepo, nil } -func IssueFromArg(apiClientFunc func(repoHost string) (*api.Client, error), client *gitlab.Client, baseRepoFn func() (glrepo.Interface, error), defaultHostname, arg string) (*gitlab.Issue, glrepo.Interface, error) { +func IssueFromArg(apiClientFunc func(repoHost string) (*api.Client, error), client *gitlab.Client, baseRepoFn func() (glrepo.Interface, error), defaultHostname, arg string) (*gitlab.Issue, *gitlab.Client, glrepo.Interface, error) { issueIID, baseRepo := issueMetadataFromURL(arg, defaultHostname) if issueIID == 0 { var err error issueIIDInt, err := strconv.Atoi(strings.TrimPrefix(arg, "#")) if err != nil { - return nil, nil, fmt.Errorf("invalid issue format: %q", arg) + return nil, nil, nil, fmt.Errorf("invalid issue format: %q", arg) } issueIID = int64(issueIIDInt) } @@ -117,18 +140,18 @@ func IssueFromArg(apiClientFunc func(repoHost string) (*api.Client, error), clie var err error baseRepo, err = baseRepoFn() if err != nil { - return nil, nil, fmt.Errorf("could not determine base repository: %w", err) + return nil, nil, nil, fmt.Errorf("could not determine base repository: %w", err) } } else { a, err := apiClientFunc(baseRepo.RepoHost()) if err != nil { - return nil, nil, err + return nil, nil, nil, err } client = a.Lab() } issue, err := issueFromIID(client, baseRepo, issueIID) - return issue, baseRepo, err + return issue, client, baseRepo, err } // issueURLPathRE is a regex which matches the following patterns: diff --git a/internal/commands/issue/update/issue_update.go b/internal/commands/issue/update/issue_update.go index 7e9611425..857d2340a 100644 --- a/internal/commands/issue/update/issue_update.go +++ b/internal/commands/issue/update/issue_update.go @@ -67,7 +67,7 @@ func NewCmdUpdate(f cmdutils.Factory) *cobra.Command { if err != nil { return err } - issue, repo, err := issueutils.IssueFromArg(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args[0]) + issue, client, repo, err := issueutils.IssueFromArg(f.ApiClient, client, f.BaseRepo, f.DefaultHostname(), args[0]) if err != nil { return err } @@ -105,7 +105,7 @@ func NewCmdUpdate(f cmdutils.Factory) *cobra.Command { if err != nil { return err } - issue, _, err := issueutils.IssueFromArg(f.ApiClient, gitlabClient, f.BaseRepo, f.DefaultHostname(), args[0]) + issue, _, _, err := issueutils.IssueFromArg(f.ApiClient, gitlabClient, f.BaseRepo, f.DefaultHostname(), args[0]) if err != nil { return err } diff --git a/internal/commands/mr/create/mr_create.go b/internal/commands/mr/create/mr_create.go index 90058b88b..dc335bfbe 100644 --- a/internal/commands/mr/create/mr_create.go +++ b/internal/commands/mr/create/mr_create.go @@ -210,7 +210,7 @@ func (o *options) validate(cmd *cobra.Command) error { } func parseIssue(apiClientFunc func(repoHost string) (*api.Client, error), gitlabClient *gitlab.Client, opts *options) (*gitlab.Issue, error) { - issue, _, err := issueutils.IssueFromArg(apiClientFunc, gitlabClient, opts.baseRepo, opts.defaultHostname, opts.RelatedIssue) + issue, _, _, err := issueutils.IssueFromArg(apiClientFunc, gitlabClient, opts.baseRepo, opts.defaultHostname, opts.RelatedIssue) if err != nil { return nil, err } -- GitLab