From 0d013746b56687b399c44c72a86d67db7efd9d32 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 23:10:22 +0200 Subject: [PATCH] fix(label): refuse duplicate label names unless --force tea label create --name bug twice created a second "bug" label, after which issue edit --add-labels bug failed as ambiguous and issue list --label bug matched nothing. label create (and --file imports) now fail with "label "bug" already exists in alice/project; pass --force to update it", ignoring case; --force updates the existing label. The ambiguity error for labels that already share a name lists their IDs. Signed-off-by: bircni --- internal/app/label_options_test.go | 36 ++++++++++++++++++++ internal/app/labelfile_test.go | 4 +++ internal/app/metadata.go | 19 +++++++---- internal/services/label_selection.go | 41 +++++++++++++++++++++-- internal/services/label_selection_test.go | 2 +- 5 files changed, 92 insertions(+), 10 deletions(-) diff --git a/internal/app/label_options_test.go b/internal/app/label_options_test.go index b2c3e5d2..69c87676 100644 --- a/internal/app/label_options_test.go +++ b/internal/app/label_options_test.go @@ -15,6 +15,10 @@ import ( func TestLabelCreateAndEditExclusiveArchived(t *testing.T) { calls := 0 server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } calls++ var payload map[string]any require.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) @@ -47,3 +51,35 @@ func TestLabelCreateAndEditExclusiveArchived(t *testing.T) { require.Equal(t, 2, calls) require.Empty(t, diagnostics.String()) } + +func TestLabelCreateRefusesDuplicateNames(t *testing.T) { + var changed []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.URL.Path { + case "GET /api/v1/repos/alice/project/labels": + if r.URL.Query().Get("page") == "1" { + _, _ = w.Write([]byte(`[{"id":11,"name":"bug","color":"ee0701","description":"Broken"}]`)) + return + } + _, _ = w.Write([]byte(`[]`)) + case "PATCH /api/v1/repos/alice/project/labels/11": + requirePayload(t, `{"color":"000000"}`, r.Body) + changed = append(changed, r.Method+" "+r.URL.Path) + _, _ = w.Write([]byte(`{"id":11,"name":"bug","color":"000000","description":"Broken"}`)) + default: + changed = append(changed, r.Method+" "+r.URL.Path) + _, _ = w.Write([]byte(`{"id":12,"name":"docs"}`)) + } + })) + defer server.Close() + deps, out, _ := serverDeps(t, server.URL) + for _, name := range []string{"bug", "BUG"} { + err := Execute(context.Background(), deps, []string{"label", "create", "-R", "alice/project", "--name", name, "--color", "000000"}) + require.EqualError(t, err, `label "bug" already exists in alice/project; pass --force to update it`) + } + require.Empty(t, changed) + require.Empty(t, out.String()) + require.NoError(t, Execute(context.Background(), deps, []string{"label", "create", "-R", "alice/project", "--name", "bug", "--color", "000000", "--force", "--json=id,color"})) + require.JSONEq(t, `{"id":11,"color":"000000"}`, out.String()) + require.Equal(t, []string{"PATCH /api/v1/repos/alice/project/labels/11"}, changed) +} diff --git a/internal/app/labelfile_test.go b/internal/app/labelfile_test.go index 35fe0d94..0d29f1e2 100644 --- a/internal/app/labelfile_test.go +++ b/internal/app/labelfile_test.go @@ -51,6 +51,10 @@ func TestLabelImportAcceptsColorPrefixedRows(t *testing.T) { var created []string server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, "/api/v1/repos/alice/project/labels", r.URL.Path) + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[]`)) + return + } payload, err := io.ReadAll(r.Body) require.NoError(t, err) var label struct{ Name string } diff --git a/internal/app/metadata.go b/internal/app/metadata.go index 4b18d327..88cc6752 100644 --- a/internal/app/metadata.go +++ b/internal/app/metadata.go @@ -45,7 +45,7 @@ func (a *application) labelCommand() *cobra.Command { group.AddCommand(list) for _, verb := range []string{"create", "edit"} { var name, color, description, file string - var exclusive, archived bool + var exclusive, archived, force bool command := &cobra.Command{Use: verb, Short: titleCase(verb) + " a label"} if verb == "edit" { command.Use += " " @@ -60,6 +60,7 @@ func (a *application) labelCommand() *cobra.Command { command.Flags().BoolVar(&archived, "archived", false, "Archive the label so it cannot be newly applied") if verb == "create" { command.Flags().StringVar(&file, "file", "", "Import color/name rows with optional ;description") + command.Flags().BoolVar(&force, "force", false, "Update the label if one with the same name exists") } command.RunE = func(cmd *cobra.Command, args []string) error { opts := services.LabelOptions{} @@ -106,10 +107,10 @@ func (a *application) labelCommand() *cobra.Command { } opts.Color = &color } - if verb == "create" || cmd.Flags().Changed("description") { + if cmd.Flags().Changed("description") { opts.Description = &description } - if verb == "create" || cmd.Flags().Changed("exclusive") { + if cmd.Flags().Changed("exclusive") { opts.Exclusive = &exclusive } if cmd.Flags().Changed("archived") { @@ -123,22 +124,28 @@ func (a *application) labelCommand() *cobra.Command { if err != nil { return err } + var existing []services.Label + if verb == "create" { + if existing, err = s.All(cmd.Context()); err != nil { + return err + } + } if file != "" { results := []services.Label{} var failures []error for _, row := range imported { - item, err := s.Create(cmd.Context(), row) + item, err := s.CreateUnique(cmd.Context(), existing, row, force) if err != nil { failures = append(failures, fmt.Errorf("label %s: %w", *row.Name, err)) continue } - results = append(results, item) + results, existing = append(results, item), append(existing, item) } return errors.Join(append(failures, writeList(a, cmd, results, labelRow))...) } var item services.Label if verb == "create" { - item, err = s.Create(cmd.Context(), opts) + item, err = s.CreateUnique(cmd.Context(), existing, opts, force) } else { item, err = s.Edit(cmd.Context(), id, opts) } diff --git a/internal/services/label_selection.go b/internal/services/label_selection.go index e1082ace..6edc89cf 100644 --- a/internal/services/label_selection.go +++ b/internal/services/label_selection.go @@ -5,11 +5,46 @@ package services import ( "context" + "errors" "fmt" + "strconv" "strings" ) -func selectLabelNames(names []string, matches map[string][]int64) ([]int64, error) { +// Scope names the repository or organization the labels belong to. +func (s Labels) Scope() string { + if s.Organization != "" { + return s.Organization + } + return s.Repository +} + +// finder addresses the labels of the scope by ID or name (D10); byName matches numeric values as +// names only. +func (s Labels) finder(byName bool) NameOrID[Label] { + return NameOrID[Label]{Kind: "label", Scope: s.Scope(), ID: func(l Label) int64 { return l.ID }, Name: func(l Label) string { return l.Name }, ByName: byName} +} + +// CreateUnique creates a label unless existing, the labels of the scope, has one with its name, +// ignoring case. Then update edits that label with the supplied fields, keeping its name, and +// otherwise CreateUnique fails: Gitea would create a second label that name filters cannot tell +// apart. +func (s Labels) CreateUnique(ctx context.Context, existing []Label, opts LabelOptions, update bool) (Label, error) { + current, err := s.finder(true).Find(existing, deref(opts.Name)) + if errors.As(err, new(*NotFoundError)) { + return s.Create(ctx, opts) + } + if err != nil { + return Label{}, err + } + if !update { + return Label{}, &ConflictError{Reason: ReasonExists, Kind: "label", Key: strconv.Quote(current.Name), Scope: s.Scope(), Hint: "pass --force to update it"} + } + opts.Name = nil + return s.Edit(ctx, current.ID, opts) +} + +func selectLabelNames(scope string, names []string, matches map[string][]int64) ([]int64, error) { ids := make([]int64, 0, len(names)) seen := map[int64]bool{} for _, name := range names { @@ -18,7 +53,7 @@ func selectLabelNames(names []string, matches map[string][]int64) ([]int64, erro return nil, fmt.Errorf("label %q does not exist; use tea label list", name) } if len(candidates) > 1 { - return nil, fmt.Errorf("label %q is ambiguous", name) + return nil, &AmbiguousError{Kind: "label", Key: strconv.Quote(name), Scope: scope, IDs: candidates} } if !seen[candidates[0]] { ids = append(ids, candidates[0]) @@ -70,7 +105,7 @@ func (s Labels) ResolveIssueNames(ctx context.Context, names []string) ([]int64, scope, name := splitLabelScope(input) matches[input] = index[labelKey{scope, name}] } - return selectLabelNames(names, matches) + return selectLabelNames(s.Repository, names, matches) } func splitLabelScope(input string) (scope, name string) { diff --git a/internal/services/label_selection_test.go b/internal/services/label_selection_test.go index 8135c9fd..2cc58647 100644 --- a/internal/services/label_selection_test.go +++ b/internal/services/label_selection_test.go @@ -66,7 +66,7 @@ func TestIssueLabelsCombineScopesWithoutSilentlyChoosingDuplicates(t *testing.T) })) service := Labels{Client: client, Repository: "company/project"} _, err := service.ResolveIssueNames(context.Background(), []string{"bug"}) - require.ErrorContains(t, err, "ambiguous") + require.EqualError(t, err, `label "bug" is ambiguous in company/project (IDs 1, 2); pass an ID`) ids, err := service.ResolveIssueNames(context.Background(), []string{"org:bug", "repo:bug", "shared", "shared"}) require.NoError(t, err) require.Equal(t, []int64{2, 1, 3}, ids)