From ebfd05b9d00d26ecc3c7090830f4961e67c0ba4b Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:59:50 +0200 Subject: [PATCH 1/3] Fix exclusive label conflict handling Validate label groups before issue mutations, preserve metadata through resolver caches, and render grouped label listings. Sanitize GraphQL errors and preserve explicit empty label updates while adding regression coverage. Co-Authored-By: Claude --- internal/cli/issues.go | 42 ++-- internal/format/labels.go | 198 ++++++++++++++++++ internal/format/labels_test.go | 58 ++++++ internal/service/client_interfaces.go | 1 + internal/service/issue.go | 159 +++++++++------ internal/service/issue_create_test.go | 23 +++ internal/service/issue_delegate_test.go | 11 +- internal/service/issue_relation_test.go | 3 + internal/service/label.go | 11 +- internal/service/label_validation.go | 111 +++++++++++ internal/service/label_validation_test.go | 42 ++++ internal/service/search_resolve_test.go | 9 +- internal/service/team.go | 22 +- pkg/linear/client.go | 8 +- pkg/linear/core/base_client.go | 57 +++--- pkg/linear/core/base_client_test.go | 127 ++++++++++++ pkg/linear/issues/client.go | 232 +++++++++++----------- pkg/linear/issues/client_test.go | 15 ++ pkg/linear/resolver.go | 119 +++++++---- pkg/linear/resolver_cache.go | 80 +++++++- pkg/linear/resolver_cache_test.go | 42 ++++ pkg/linear/resolver_label_test.go | 58 ++++++ plan.md | 170 ++++++++++++++++ 23 files changed, 1281 insertions(+), 317 deletions(-) create mode 100644 internal/format/labels.go create mode 100644 internal/format/labels_test.go create mode 100644 internal/service/label_validation.go create mode 100644 internal/service/label_validation_test.go create mode 100644 pkg/linear/core/base_client_test.go create mode 100644 pkg/linear/resolver_cache_test.go create mode 100644 pkg/linear/resolver_label_test.go create mode 100644 plan.md diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 9567429..c3e7cbc 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -6,8 +6,8 @@ import ( "strconv" "github.com/joa23/linear-cli/internal/format" - paginationutil "github.com/joa23/linear-cli/pkg/linear/pagination" "github.com/joa23/linear-cli/internal/service" + paginationutil "github.com/joa23/linear-cli/pkg/linear/pagination" "github.com/spf13/cobra" ) @@ -40,21 +40,21 @@ func newIssuesCmd() *cobra.Command { func newIssuesListCmd() *cobra.Command { var ( - teamID string - project string - state string - priority string - assignee string - cycle string - labels string + teamID string + project string + state string + priority string + assignee string + cycle string + labels string excludeLabels string - sortBy string + sortBy string createdSince string createdAfter string createdBefore string - limit int - formatStr string - outputType string + limit int + formatStr string + outputType string ) cmd := &cobra.Command{ @@ -243,7 +243,7 @@ Images in the description (uploads.linear.app/...) require auth — use: # Download a private image from the issue description linear attachments download "https://uploads.linear.app/..."`, - Args: cobra.ExactArgs(1), + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { issueID := args[0] @@ -442,7 +442,7 @@ TIP: Run 'linear init' first to set default team.`, output, err := deps.Issues.Create(input, outType) if err != nil { - return fmt.Errorf("failed to create issue: %w", err) + return err } fmt.Println(output) @@ -534,15 +534,16 @@ LABEL MODES: return err } -// Get team from flag or config (for cycle resolution) + // Get team from flag or config (for cycle resolution) if team == "" { team = GetDefaultTeam() } // Note: team can still be "" if no .linear.yaml, will fallback to issue identifier // Check if any updates provided (description="-" means stdin) + labelsChanged := cmd.Flags().Changed("labels") hasFlags := title != "" || description != "" || state != "" || - priority != "" || estimate != "" || labels != "" || + priority != "" || estimate != "" || labelsChanged || addLabels != "" || removeLabels != "" || cycle != "" || project != "" || assignee != "" || dueDate != "" || parent != "" || dependsOn != "" || blockedBy != "" || @@ -553,7 +554,7 @@ LABEL MODES: } // Validate mutual exclusivity: --labels cannot be used with --add-labels or --remove-labels - if labels != "" && (addLabels != "" || removeLabels != "") { + if labelsChanged && (addLabels != "" || removeLabels != "") { return fmt.Errorf("--labels cannot be combined with --add-labels or --remove-labels. Use --labels to replace all labels, or --add-labels/--remove-labels for incremental changes") } @@ -597,8 +598,11 @@ LABEL MODES: } input.Estimate = &e } - if labels != "" { + if labelsChanged { input.LabelIDs = parseCommaSeparated(labels) + if input.LabelIDs == nil { + input.LabelIDs = []string{} + } } if addLabels != "" { input.AddLabelIDs = parseCommaSeparated(addLabels) @@ -636,7 +640,7 @@ LABEL MODES: output, err := deps.Issues.Update(issueID, input) if err != nil { - return fmt.Errorf("failed to update issue: %w", err) + return err } fmt.Println(output) diff --git a/internal/format/labels.go b/internal/format/labels.go new file mode 100644 index 0000000..074bd38 --- /dev/null +++ b/internal/format/labels.go @@ -0,0 +1,198 @@ +package format + +import ( + "fmt" + "sort" + "strings" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +// LabelListOptions controls the fields included in text label listings. +type LabelListOptions struct { + // IncludeIDs includes the Linear ID on each label and group header. + IncludeIDs bool +} + +// FormatLabels renders labels as a deterministic, grouped text list. Labels +// with a Parent are rendered as alternatives under one parent header, even +// when the API returns the parent after its children (or omits it entirely). +// The input labels are never modified. +func FormatLabels(labels []core.Label, options LabelListOptions) string { + if len(labels) == 0 { + return "No labels found." + } + + parents := make(map[string]core.Label) + groups := make(map[string][]core.Label) + for _, label := range labels { + if label.Parent == nil { + parents[label.ID] = label + continue + } + + // Parent ID is the authoritative group key. Keep a name fallback for + // incomplete fixtures/responses so children are not accidentally merged. + key := label.Parent.ID + if key == "" { + key = "name:" + label.Parent.Name + } + groups[key] = append(groups[key], label) + } + + // A parent that has children is a group header, not also a standalone + // record. Other parent labels remain standalone labels. + groupParents := make(map[string]core.Label) + for key := range groups { + if parent, ok := parents[key]; ok { + groupParents[key] = parent + delete(parents, key) + } + groups[key] = sortLabels(groups[key]) + } + standalone := make([]core.Label, 0, len(parents)) + for _, label := range parents { + standalone = append(standalone, label) + } + standalone = sortLabels(standalone) + + type entry struct { + key string + parent *core.Label + ref *core.LabelRef + labels []core.Label + } + entries := make([]entry, 0, len(groups)+len(standalone)) + for key, children := range groups { + if parent, ok := groupParents[key]; ok { + parentCopy := parent + entries = append(entries, entry{key: labelSortKey(parent), parent: &parentCopy, labels: children}) + continue + } + ref := groupReference(labels, key) + entries = append(entries, entry{key: labelSortKeyRef(ref), ref: ref, labels: children}) + } + for _, label := range standalone { + labelCopy := label + entries = append(entries, entry{key: labelSortKey(label), parent: &labelCopy}) + } + sort.SliceStable(entries, func(i, j int) bool { return entries[i].key < entries[j].key }) + + var b strings.Builder + b.WriteString(fmt.Sprintf("LABELS (%d)\n", len(labels))) + b.WriteString(line(40)) + b.WriteByte('\n') + for _, item := range entries { + if item.parent != nil && len(item.labels) > 0 { + b.WriteString(" GROUP: ") + writeLabel(&b, *item.parent, options.IncludeIDs) + writeDescription(&b, " ", item.parent.Description) + for _, child := range item.labels { + writeLabelIndented(&b, child, " ", options.IncludeIDs) + writeDescription(&b, " ", child.Description) + } + continue + } + if item.ref != nil { + b.WriteString(" GROUP: ") + b.WriteString(labelReferenceName(*item.ref)) + if options.IncludeIDs && item.ref.ID != "" { + b.WriteString(" [") + b.WriteString(item.ref.ID) + b.WriteString("]") + } + b.WriteByte('\n') + for _, child := range item.labels { + writeLabelIndented(&b, child, " ", options.IncludeIDs) + writeDescription(&b, " ", child.Description) + } + continue + } + writeLabelIndented(&b, *item.parent, " ", options.IncludeIDs) + writeDescription(&b, " ", item.parent.Description) + } + return b.String() +} + +// LabelList renders the label-list text format used by services. +func (f *Formatter) LabelList(labels []core.Label, includeIDs bool) string { + return FormatLabels(labels, LabelListOptions{IncludeIDs: includeIDs}) +} + +// Labels renders the compact label-list text format. +func (f *Formatter) Labels(labels []core.Label) string { + return FormatLabels(labels, LabelListOptions{}) +} + +func sortLabels(labels []core.Label) []core.Label { + sort.SliceStable(labels, func(i, j int) bool { + return labelSortKey(labels[i]) < labelSortKey(labels[j]) + }) + return labels +} + +func labelSortKey(label core.Label) string { + return strings.ToLower(label.Name) + "\x00" + label.Name + "\x00" + label.ID +} + +func labelSortKeyRef(ref *core.LabelRef) string { + if ref == nil { + return "" + } + return strings.ToLower(ref.Name) + "\x00" + ref.Name + "\x00" + ref.ID +} + +func groupReference(labels []core.Label, key string) *core.LabelRef { + var refs []core.LabelRef + for _, label := range labels { + if label.Parent == nil { + continue + } + parentKey := label.Parent.ID + if parentKey == "" { + parentKey = "name:" + label.Parent.Name + } + if parentKey == key { + refs = append(refs, *label.Parent) + } + } + if len(refs) == 0 { + return nil + } + sort.Slice(refs, func(i, j int) bool { return labelSortKeyRef(&refs[i]) < labelSortKeyRef(&refs[j]) }) + return &refs[0] +} + +func labelReferenceName(ref core.LabelRef) string { + if ref.Name != "" { + return ref.Name + } + return ref.ID +} + +func writeLabelIndented(b *strings.Builder, label core.Label, indent string, includeID bool) { + b.WriteString(indent) + writeLabel(b, label, includeID) +} + +func writeLabel(b *strings.Builder, label core.Label, includeID bool) { + b.WriteString(label.Name) + if label.Color != "" { + b.WriteString(" [") + b.WriteString(label.Color) + b.WriteString("]") + } + if includeID && label.ID != "" { + b.WriteByte(' ') + b.WriteString(label.ID) + } + b.WriteByte('\n') +} + +func writeDescription(b *strings.Builder, indent, description string) { + if description != "" { + b.WriteString(indent) + b.WriteString(description) + b.WriteByte('\n') + } +} diff --git a/internal/format/labels_test.go b/internal/format/labels_test.go new file mode 100644 index 0000000..c20d325 --- /dev/null +++ b/internal/format/labels_test.go @@ -0,0 +1,58 @@ +package format + +import ( + "strings" + "testing" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +func TestFormatLabels_GroupsAndSortsIndependentOfAPIOrder(t *testing.T) { + labels := []core.Label{ + {ID: "child-z", Name: "Zulu", Color: "#z", Description: "last", Parent: &core.LabelRef{ID: "group-2", Name: "Priority"}}, + {ID: "standalone", Name: "Bug", Color: "#b", Description: "standalone"}, + {ID: "child-a", Name: "Alpha", Color: "#a", Parent: &core.LabelRef{ID: "group-2", Name: "Priority"}}, + {ID: "group-2", Name: "Priority", Color: "#p", Description: "choose one"}, + {ID: "child-c", Name: "Gamma", Color: "#g", Parent: &core.LabelRef{ID: "missing", Name: "Severity"}}, + } + + got := FormatLabels(labels, LabelListOptions{IncludeIDs: true}) + if strings.Count(got, "GROUP: Priority") != 1 { + t.Fatalf("expected one priority group header, got:\n%s", got) + } + if strings.Index(got, " Alpha") > strings.Index(got, " Zulu") { + t.Errorf("children should be sorted: \n%s", got) + } + if strings.Index(got, "GROUP: Priority") > strings.Index(got, "GROUP: Severity") { + t.Errorf("groups should be sorted: \n%s", got) + } + for _, want := range []string{"group-2", "child-a", "last", "standalone", "missing"} { + if !strings.Contains(got, want) { + t.Errorf("grouped output missing %q:\n%s", want, got) + } + } +} + +func TestFormatLabels_CompactPreservesDescriptionsAndOmitsIDs(t *testing.T) { + labels := []core.Label{ + {ID: "parent", Name: "Type", Color: "#fff"}, + {ID: "child", Name: "Feature", Color: "#000", Description: "a feature", Parent: &core.LabelRef{ID: "parent", Name: "Type"}}, + } + + got := New().Labels(labels) + if strings.Contains(got, "parent") || strings.Contains(got, "child") { + t.Errorf("compact output should omit IDs:\n%s", got) + } + if !strings.Contains(got, "GROUP: Type") || !strings.Contains(got, " Feature [#000]") { + t.Errorf("compact output should show group and child:\n%s", got) + } + if !strings.Contains(got, " a feature") { + t.Errorf("child description should be retained:\n%s", got) + } +} + +func TestFormatLabels_Empty(t *testing.T) { + if got := FormatLabels(nil, LabelListOptions{}); got != "No labels found." { + t.Errorf("unexpected empty output: %q", got) + } +} diff --git a/internal/service/client_interfaces.go b/internal/service/client_interfaces.go index b97ee84..f3af8a8 100644 --- a/internal/service/client_interfaces.go +++ b/internal/service/client_interfaces.go @@ -29,6 +29,7 @@ type IssueClientOperations interface { ResolveUserIdentifier(nameOrEmail string) (*linear.ResolvedUser, error) ResolveCycleIdentifier(numberOrNameOrID, teamID string) (string, error) ResolveLabelIdentifier(labelName, teamID string) (string, error) + ResolveLabelMetadata(labelName, teamID string) (*core.Label, error) ResolveProjectIdentifier(nameOrID, teamID string) (string, error) // Relation operations diff --git a/internal/service/issue.go b/internal/service/issue.go index 2a1537d..73b285a 100644 --- a/internal/service/issue.go +++ b/internal/service/issue.go @@ -5,9 +5,9 @@ import ( "sort" "github.com/joa23/linear-cli/internal/format" + "github.com/joa23/linear-cli/pkg/linear/core" "github.com/joa23/linear-cli/pkg/linear/identifiers" paginationutil "github.com/joa23/linear-cli/pkg/linear/pagination" - "github.com/joa23/linear-cli/pkg/linear/core" ) // IssueService handles issue-related operations @@ -26,16 +26,16 @@ func NewIssueService(client IssueClientOperations, formatter *format.Formatter) // SearchFilters represents filters for searching issues type SearchFilters struct { - TeamID string - ProjectID string - AssigneeID string - CycleID string - StateIDs []string - LabelIDs []string + TeamID string + ProjectID string + AssigneeID string + CycleID string + StateIDs []string + LabelIDs []string ExcludeLabelIDs []string - Priority *int - SearchTerm string - OrderBy string + Priority *int + SearchTerm string + OrderBy string // Date filters (RFC3339 timestamps). Set via CLI --created-since/--created-after/--created-before. CreatedAfter string CreatedBefore string @@ -516,21 +516,20 @@ func (s *IssueService) Create(input *CreateIssueInput, outputType format.OutputT } if len(input.LabelIDs) > 0 { - resolvedLabelIDs := make([]string, 0, len(input.LabelIDs)) - for _, labelName := range input.LabelIDs { - labelID, err := s.client.ResolveLabelIdentifier(labelName, teamID) - if err != nil { - return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) - } - resolvedLabelIDs = append(resolvedLabelIDs, labelID) + resolvedLabels, err := s.resolveLabelMetadata(input.LabelIDs, teamID) + if err != nil { + return "", fmt.Errorf("failed to create issue: %w", err) + } + if err := validateLabelSelection(resolvedLabels); err != nil { + return "", fmt.Errorf("failed to create issue: %w", err) } - createInput.LabelIDs = resolvedLabelIDs + createInput.LabelIDs = labelIDs(resolvedLabels) } // Single atomic API call — if this fails, no orphaned issue is created. issue, err := s.client.CreateIssue(&createInput) if err != nil { - return "", fmt.Errorf("failed to create issue: %w", err) + return "", err } // Create native relations for dependencies @@ -694,22 +693,20 @@ func (s *IssueService) Update(identifier string, input *UpdateIssueInput) (strin } linearInput.CycleID = &cycleID } - hasLabelChanges := len(input.LabelIDs) > 0 || len(input.AddLabelIDs) > 0 || len(input.RemoveLabelIDs) > 0 + hasLabelChanges := input.LabelIDs != nil || len(input.AddLabelIDs) > 0 || len(input.RemoveLabelIDs) > 0 if hasLabelChanges { - // Resolve team ID for label resolution + // Resolve team ID for label resolution. var teamIDForLabels string var err error - if input.TeamID != nil && *input.TeamID != "" { teamIDForLabels, err = s.client.ResolveTeamIdentifier(*input.TeamID) if err != nil { return "", fmt.Errorf("could not resolve team '%s': %w", *input.TeamID, err) } } else { - // Extract from issue identifier - teamKey, _, err := identifiers.ParseIssueIdentifier(issue.Identifier) - if err != nil { - return "", fmt.Errorf("invalid issue identifier '%s': %w", issue.Identifier, err) + teamKey, _, parseErr := identifiers.ParseIssueIdentifier(issue.Identifier) + if parseErr != nil { + return "", fmt.Errorf("invalid issue identifier '%s': %w", issue.Identifier, parseErr) } teamIDForLabels, err = s.client.ResolveTeamIdentifier(teamKey) if err != nil { @@ -717,51 +714,55 @@ func (s *IssueService) Update(identifier string, input *UpdateIssueInput) (strin } } - if len(input.LabelIDs) > 0 { - // Replace mode: resolve label names to IDs and set directly - resolvedLabelIDs := make([]string, 0, len(input.LabelIDs)) - for _, labelName := range input.LabelIDs { - labelID, err := s.client.ResolveLabelIdentifier(labelName, teamIDForLabels) - if err != nil { - return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) - } - resolvedLabelIDs = append(resolvedLabelIDs, labelID) + if input.TeamID != nil && *input.TeamID != "" && (len(input.AddLabelIDs) > 0 || len(input.RemoveLabelIDs) > 0) { + return "", fmt.Errorf("cannot add or remove labels while changing an issue's team; use --labels to replace all labels") + } + + if input.LabelIDs != nil { + resolvedLabels, err := s.resolveLabelMetadata(input.LabelIDs, teamIDForLabels) + if err != nil { + return "", fmt.Errorf("failed to update issue: %w", err) + } + if err := validateLabelSelection(resolvedLabels); err != nil { + return "", fmt.Errorf("failed to update issue: %w", err) } - linearInput.LabelIDs = resolvedLabelIDs + linearInput.LabelIDs = labelIDs(resolvedLabels) } else { - // Additive/subtractive mode: fetch current labels, merge/remove, then set - currentLabelIDs := s.extractCurrentLabelIDs(issue) - - // Build a set from current labels for efficient merge/remove - labelSet := make(map[string]bool, len(currentLabelIDs)) - for _, id := range currentLabelIDs { - labelSet[id] = true + currentLabels, err := s.currentLabelMetadata(issue, teamIDForLabels, len(input.AddLabelIDs) > 0) + if err != nil { + return "", fmt.Errorf("failed to update issue: %w", err) + } + labelSet := make(map[string]core.Label, len(currentLabels)) + for _, label := range currentLabels { + labelSet[label.ID] = label } - // Add new labels for _, labelName := range input.AddLabelIDs { - labelID, err := s.client.ResolveLabelIdentifier(labelName, teamIDForLabels) + label, err := s.client.ResolveLabelMetadata(labelName, teamIDForLabels) if err != nil { return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) } - labelSet[labelID] = true + labelSet[label.ID] = *label } - - // Remove labels for _, labelName := range input.RemoveLabelIDs { - labelID, err := s.client.ResolveLabelIdentifier(labelName, teamIDForLabels) + label, err := s.client.ResolveLabelMetadata(labelName, teamIDForLabels) if err != nil { return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) } - delete(labelSet, labelID) + delete(labelSet, label.ID) } - // Convert set back to slice - finalLabelIDs := make([]string, 0, len(labelSet)) - for id := range labelSet { - finalLabelIDs = append(finalLabelIDs, id) + finalLabels := make([]core.Label, 0, len(labelSet)) + for _, label := range labelSet { + finalLabels = append(finalLabels, label) } - linearInput.LabelIDs = finalLabelIDs + finalLabels = sortLabels(finalLabels) + if len(input.AddLabelIDs) > 0 { + if err := validateLabelSelection(finalLabels); err != nil { + return "", fmt.Errorf("failed to update issue: %w", err) + } + } + linearInput.LabelIDs = labelIDs(finalLabels) } } @@ -770,7 +771,7 @@ func (s *IssueService) Update(identifier string, input *UpdateIssueInput) (strin if hasServiceFieldsToUpdate(linearInput) { updatedIssue, err = s.client.UpdateIssue(issue.ID, linearInput) if err != nil { - return "", fmt.Errorf("failed to update issue: %w", err) + return "", err } } @@ -868,7 +869,7 @@ func hasServiceFieldsToUpdate(input core.UpdateIssueInput) bool { input.ParentID != nil || input.TeamID != nil || input.CycleID != nil || - len(input.LabelIDs) > 0 + input.LabelIDs != nil } // extractCurrentLabelIDs extracts the current label IDs from an issue @@ -883,6 +884,50 @@ func (s *IssueService) extractCurrentLabelIDs(issue *core.Issue) []string { return ids } +func labelIDs(labels []core.Label) []string { + labels = sortLabels(labels) + ids := make([]string, 0, len(labels)) + for _, label := range labels { + ids = append(ids, label.ID) + } + sort.Strings(ids) + return ids +} + +func (s *IssueService) resolveLabelMetadata(names []string, teamID string) ([]core.Label, error) { + resolved := make([]core.Label, 0, len(names)) + for _, name := range names { + label, err := s.client.ResolveLabelMetadata(name, teamID) + if err != nil { + return nil, fmt.Errorf("failed to resolve label '%s': %w", name, err) + } + if label == nil { + return nil, fmt.Errorf("failed to resolve label '%s': resolver returned no label", name) + } + resolved = append(resolved, *label) + } + return sortLabels(resolved), nil +} + +func (s *IssueService) currentLabelMetadata(issue *core.Issue, teamID string, hydrate bool) ([]core.Label, error) { + if issue.Labels == nil { + return nil, nil + } + labels := make([]core.Label, 0, len(issue.Labels.Nodes)) + for _, label := range issue.Labels.Nodes { + if label.Parent != nil && label.Parent.ID != "" || !hydrate { + labels = append(labels, label) + continue + } + resolved, err := s.client.ResolveLabelMetadata(label.ID, teamID) + if err != nil { + return nil, fmt.Errorf("could not resolve existing label '%s': %w", label.Name, err) + } + labels = append(labels, *resolved) + } + return sortLabels(labels), nil +} + // resolveStateID resolves a state name to a valid state ID func (s *IssueService) resolveStateID(stateName, teamID string) (string, error) { // Always resolve by name - no UUID support diff --git a/internal/service/issue_create_test.go b/internal/service/issue_create_test.go index c538e8c..00c3f45 100644 --- a/internal/service/issue_create_test.go +++ b/internal/service/issue_create_test.go @@ -54,6 +54,12 @@ func (m *mockIssueClientForCreate) ResolveCycleIdentifier(num, team string) (str func (m *mockIssueClientForCreate) ResolveLabelIdentifier(label, team string) (string, error) { return "label-uuid-" + label, nil } +func (m *mockIssueClientForCreate) ResolveLabelMetadata(label, team string) (*core.Label, error) { + if label == "Tests" || label == "Improvement" { + return &core.Label{ID: "label-uuid-" + label, Name: label, Parent: &core.LabelRef{ID: "group-uuid", Name: "Issue Type"}}, nil + } + return &core.Label{ID: "label-uuid-" + label, Name: label}, nil +} // Unused interface methods. func (m *mockIssueClientForCreate) GetIssue(id string) (*core.Issue, error) { return nil, nil } @@ -84,6 +90,23 @@ func makeIssueServiceForCreate(mock *mockIssueClientForCreate) *IssueService { return NewIssueService(mock, format.New()) } +func TestIssueService_Create_RejectsExclusiveLabelConflictBeforeMutation(t *testing.T) { + mock := &mockIssueClientForCreate{} + svc := makeIssueServiceForCreate(mock) + + _, err := svc.Create(&CreateIssueInput{ + Title: "Conflict", + TeamID: "TL", + LabelIDs: []string{"Tests", "Improvement"}, + }, format.OutputText) + if err == nil || !strings.Contains(err.Error(), "Issue Type") || !strings.Contains(err.Error(), "Tests") || !strings.Contains(err.Error(), "Improvement") { + t.Fatalf("error = %v, want actionable label conflict", err) + } + if mock.createCalled { + t.Fatal("CreateIssue called despite local label conflict") + } +} + func TestIssueService_Create_AtomicFields(t *testing.T) { priority := 1 estimate := 3.0 diff --git a/internal/service/issue_delegate_test.go b/internal/service/issue_delegate_test.go index c253fc9..b0ea329 100644 --- a/internal/service/issue_delegate_test.go +++ b/internal/service/issue_delegate_test.go @@ -58,6 +58,9 @@ func (m *mockIssueClientForDelegate) ResolveCycleIdentifier(num, team string) (s func (m *mockIssueClientForDelegate) ResolveLabelIdentifier(label, team string) (string, error) { return "label-uuid", nil } +func (m *mockIssueClientForDelegate) ResolveLabelMetadata(label, team string) (*core.Label, error) { + return &core.Label{ID: "label-uuid", Name: label}, nil +} func (m *mockIssueClientForDelegate) ResolveProjectIdentifier(nameOrID, teamID string) (string, error) { return "project-uuid", nil } @@ -149,10 +152,10 @@ func TestIssueService_Update_DelegateVsAssignee(t *testing.T) { func TestResolvedUser_ApplicationDetection(t *testing.T) { tests := []struct { - name string - user *linear.ResolvedUser - expectHuman bool - expectApp bool + name string + user *linear.ResolvedUser + expectHuman bool + expectApp bool }{ { name: "human user", diff --git a/internal/service/issue_relation_test.go b/internal/service/issue_relation_test.go index 1125d65..9338d2d 100644 --- a/internal/service/issue_relation_test.go +++ b/internal/service/issue_relation_test.go @@ -80,6 +80,9 @@ func (m *mockIssueClientForRelation) ResolveCycleIdentifier(num, team string) (s func (m *mockIssueClientForRelation) ResolveLabelIdentifier(label, team string) (string, error) { return "label-uuid", nil } +func (m *mockIssueClientForRelation) ResolveLabelMetadata(label, team string) (*core.Label, error) { + return &core.Label{ID: "label-uuid", Name: label}, nil +} func (m *mockIssueClientForRelation) ResolveProjectIdentifier(nameOrID, teamID string) (string, error) { return "project-uuid", nil } diff --git a/internal/service/label.go b/internal/service/label.go index e56e4ed..08e4e46 100644 --- a/internal/service/label.go +++ b/internal/service/label.go @@ -49,16 +49,7 @@ func (s *LabelService) List(teamID string, verbosity format.Verbosity, outputTyp return string(data), nil } - // Text output - output := fmt.Sprintf("LABELS (%d)\n────────────────────────────────────────\n", len(labels)) - for _, label := range labels { - output += fmt.Sprintf(" %-30s %s %s\n", label.Name, label.Color, label.ID) - if label.Description != "" { - output += fmt.Sprintf(" %s\n", label.Description) - } - } - - return output, nil + return s.formatter.LabelList(labels, true), nil } // errLabelMutationRequiresUser is returned when label mutations are attempted in agent mode diff --git a/internal/service/label_validation.go b/internal/service/label_validation.go new file mode 100644 index 0000000..6bd529d --- /dev/null +++ b/internal/service/label_validation.go @@ -0,0 +1,111 @@ +package service + +import ( + "fmt" + "sort" + "strings" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +// LabelConflictGroup describes multiple child labels selected from one +// mutually exclusive parent group. +type LabelConflictGroup struct { + Labels []core.Label + Parent core.LabelRef +} + +// LabelConflictError is returned before a mutation when a label selection +// contains multiple children from one exclusive parent group. +type LabelConflictError struct { + Groups []LabelConflictGroup +} + +func (e *LabelConflictError) Error() string { + parts := make([]string, 0, len(e.Groups)) + for _, group := range e.Groups { + names := make([]string, 0, len(group.Labels)) + for _, label := range group.Labels { + names = append(names, fmt.Sprintf("%q", label.Name)) + } + groupName := group.Parent.Name + if groupName == "" { + groupName = group.Parent.ID + } + parts = append(parts, fmt.Sprintf("labels %s belong to the exclusive group %q — pick one", joinLabelNames(names), groupName)) + } + return strings.Join(parts, "; ") +} + +func validateLabelSelection(labels []core.Label) error { + groups := make(map[string]LabelConflictGroup) + seen := make(map[string]struct{}, len(labels)) + for _, label := range labels { + if _, ok := seen[label.ID]; ok { + continue + } + seen[label.ID] = struct{}{} + if label.Parent == nil || label.Parent.ID == "" { + continue + } + group := groups[label.Parent.ID] + group.Parent = *label.Parent + group.Labels = append(group.Labels, label) + groups[label.Parent.ID] = group + } + + conflicts := make([]LabelConflictGroup, 0) + for _, group := range groups { + if len(group.Labels) < 2 { + continue + } + sort.Slice(group.Labels, func(i, j int) bool { + left, right := strings.ToLower(group.Labels[i].Name), strings.ToLower(group.Labels[j].Name) + if left == right { + return group.Labels[i].ID < group.Labels[j].ID + } + return left < right + }) + conflicts = append(conflicts, group) + } + if len(conflicts) == 0 { + return nil + } + sort.Slice(conflicts, func(i, j int) bool { + left, right := strings.ToLower(conflicts[i].Parent.Name), strings.ToLower(conflicts[j].Parent.Name) + if left == right { + return conflicts[i].Parent.ID < conflicts[j].Parent.ID + } + return left < right + }) + return &LabelConflictError{Groups: conflicts} +} + +func joinLabelNames(names []string) string { + if len(names) < 2 { + return strings.Join(names, ", ") + } + if len(names) == 2 { + return names[0] + " and " + names[1] + } + return strings.Join(names[:len(names)-1], ", ") + ", and " + names[len(names)-1] +} + +func sortLabels(labels []core.Label) []core.Label { + byID := make(map[string]core.Label, len(labels)) + for _, label := range labels { + byID[label.ID] = label + } + result := make([]core.Label, 0, len(byID)) + for _, label := range byID { + result = append(result, label) + } + sort.Slice(result, func(i, j int) bool { + left, right := strings.ToLower(result[i].Name), strings.ToLower(result[j].Name) + if left == right { + return result[i].ID < result[j].ID + } + return left < right + }) + return result +} diff --git a/internal/service/label_validation_test.go b/internal/service/label_validation_test.go new file mode 100644 index 0000000..82a2b27 --- /dev/null +++ b/internal/service/label_validation_test.go @@ -0,0 +1,42 @@ +package service + +import ( + "strings" + "testing" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +func TestValidateLabelSelectionReportsDeterministicSiblingConflicts(t *testing.T) { + labels := []core.Label{ + {ID: "b", Name: "Beta", Parent: &core.LabelRef{ID: "group-2", Name: "Review"}}, + {ID: "a", Name: "A, quoted", Parent: &core.LabelRef{ID: "group-1", Name: "Issue Type"}}, + {ID: "c", Name: "Alpha", Parent: &core.LabelRef{ID: "group-2", Name: "Review"}}, + {ID: "d", Name: "Tests", Parent: &core.LabelRef{ID: "group-1", Name: "Issue Type"}}, + {ID: "standalone", Name: "Standalone"}, + {ID: "d", Name: "Tests", Parent: &core.LabelRef{ID: "group-1", Name: "Issue Type"}}, + } + + err := validateLabelSelection(labels) + if err == nil { + t.Fatal("validateLabelSelection returned nil for conflicting siblings") + } + message := err.Error() + if !strings.Contains(message, `"A, quoted"`) || !strings.Contains(message, `"Tests"`) || !strings.Contains(message, `"Issue Type"`) { + t.Fatalf("conflict message = %q, want labels and group", message) + } + if strings.Index(message, `"Issue Type"`) > strings.Index(message, `"Review"`) { + t.Fatalf("groups are not deterministic: %q", message) + } +} + +func TestValidateLabelSelectionAllowsStandaloneAndDistinctGroups(t *testing.T) { + labels := []core.Label{ + {ID: "a", Name: "Alpha", Parent: &core.LabelRef{ID: "group-1", Name: "One"}}, + {ID: "b", Name: "Beta", Parent: &core.LabelRef{ID: "group-2", Name: "Two"}}, + {ID: "c", Name: "Standalone"}, + } + if err := validateLabelSelection(labels); err != nil { + t.Fatalf("unexpected conflict: %v", err) + } +} diff --git a/internal/service/search_resolve_test.go b/internal/service/search_resolve_test.go index d88a6e5..235d36a 100644 --- a/internal/service/search_resolve_test.go +++ b/internal/service/search_resolve_test.go @@ -54,6 +54,9 @@ func (m *mockIssueClient) ResolveCycleIdentifier(num, team string) (string, erro func (m *mockIssueClient) ResolveLabelIdentifier(label, team string) (string, error) { return m.resolveLabelResult, m.resolveLabelErr } +func (m *mockIssueClient) ResolveLabelMetadata(label, team string) (*core.Label, error) { + return &core.Label{ID: m.resolveLabelResult, Name: label}, m.resolveLabelErr +} func (m *mockIssueClient) ResolveProjectIdentifier(nameOrID, teamID string) (string, error) { return m.resolveProjectResult, m.resolveProjectErr } @@ -103,9 +106,9 @@ func (m *mockSearchClient) ResolveProjectIdentifier(nameOrID, teamID string) (st return m.resolveProjectResult, m.resolveProjectErr } func (m *mockSearchClient) IssueClient() *issues.Client { return nil } -func (m *mockSearchClient) ProjectClient() *projects.Client { return nil } -func (m *mockSearchClient) TeamClient() *teams.Client { return nil } -func (m *mockSearchClient) WorkflowClient() *workflows.Client { return m.workflowClient } +func (m *mockSearchClient) ProjectClient() *projects.Client { return nil } +func (m *mockSearchClient) TeamClient() *teams.Client { return nil } +func (m *mockSearchClient) WorkflowClient() *workflows.Client { return m.workflowClient } // --- IssueService.Search tests --- diff --git a/internal/service/team.go b/internal/service/team.go index 7f05a27..a8b6f25 100644 --- a/internal/service/team.go +++ b/internal/service/team.go @@ -91,16 +91,7 @@ func (s *TeamService) GetLabels(identifier string) (string, error) { return "No labels found.", nil } - // Format labels as simple list - output := fmt.Sprintf("LABELS (%d)\n────────────────────────────────────────\n", len(labels)) - for _, label := range labels { - output += fmt.Sprintf(" %s [%s]\n", label.Name, label.Color) - if label.Description != "" { - output += fmt.Sprintf(" %s\n", label.Description) - } - } - - return output, nil + return s.formatter.LabelList(labels, false), nil } // GetLabelsWithOutput returns labels for a team with new renderer architecture @@ -128,16 +119,7 @@ func (s *TeamService) GetLabelsWithOutput(identifier string, verbosity format.Ve return string(data), nil } - // Format labels as simple list - output := fmt.Sprintf("LABELS (%d)\n────────────────────────────────────────\n", len(labels)) - for _, label := range labels { - output += fmt.Sprintf(" %s [%s]\n", label.Name, label.Color) - if label.Description != "" { - output += fmt.Sprintf(" %s\n", label.Description) - } - } - - return output, nil + return s.formatter.LabelList(labels, false), nil } // GetWorkflowStates returns workflow states for a team (legacy method) diff --git a/pkg/linear/client.go b/pkg/linear/client.go index c20f8d0..5b3f0b4 100644 --- a/pkg/linear/client.go +++ b/pkg/linear/client.go @@ -6,6 +6,8 @@ import ( "os" "github.com/joa23/linear-cli/internal/config" + "github.com/joa23/linear-cli/internal/oauth" + "github.com/joa23/linear-cli/internal/token" "github.com/joa23/linear-cli/pkg/linear/attachments" "github.com/joa23/linear-cli/pkg/linear/comments" "github.com/joa23/linear-cli/pkg/linear/core" @@ -16,8 +18,6 @@ import ( "github.com/joa23/linear-cli/pkg/linear/teams" "github.com/joa23/linear-cli/pkg/linear/users" "github.com/joa23/linear-cli/pkg/linear/workflows" - "github.com/joa23/linear-cli/internal/oauth" - "github.com/joa23/linear-cli/internal/token" ) // Client represents the main Linear API client that orchestrates all sub-clients. @@ -666,6 +666,10 @@ func (c *Client) ResolveLabelIdentifier(labelName string, teamID string) (string return c.resolver.ResolveLabel(labelName, teamID) } +func (c *Client) ResolveLabelMetadata(labelName string, teamID string) (*core.Label, error) { + return c.resolver.ResolveLabelMetadata(labelName, teamID) +} + func (c *Client) ResolveProjectIdentifier(nameOrID string, teamID string) (string, error) { return c.resolver.ResolveProject(nameOrID, teamID) } diff --git a/pkg/linear/core/base_client.go b/pkg/linear/core/base_client.go index dda5f44..cce4781 100644 --- a/pkg/linear/core/base_client.go +++ b/pkg/linear/core/base_client.go @@ -78,7 +78,7 @@ func (bc *BaseClient) GetToken() (string, error) { func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, error) { const maxRetries = 5 const baseDelay = 100 * time.Millisecond - + // Store the original body so we can recreate the request for retries // Why: HTTP request bodies can only be read once. Since we might retry // the request multiple times, we need to preserve the original body data @@ -92,7 +92,7 @@ func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, e } req.Body.Close() } - + // Set Content-Type header once req.Header.Set("Content-Type", "application/json") @@ -118,7 +118,7 @@ func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, e } resp, err := bc.HTTPClient.Do(req) - + // Handle network errors with retry logic // Why: Network errors are often transient (e.g., connection reset, // DNS failures, timeouts). Retrying with backoff gives the network @@ -144,7 +144,7 @@ func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, e } return nil, fmt.Errorf("network error: %w", err) } - + // Success - return the response // Why: 2xx status codes indicate successful API calls that don't // need retry logic. We return immediately to avoid unnecessary delays. @@ -209,7 +209,7 @@ func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, e continue } } - + // Handle server errors (5xx) with retry // Why: Server errors are often temporary (e.g., deployments, database // issues, load problems). Retrying gives the server time to recover @@ -218,19 +218,19 @@ func (bc *BaseClient) makeRequestWithRetry(req *http.Request) (*http.Response, e body, _ := io.ReadAll(resp.Body) resp.Body.Close() lastErr = fmt.Errorf("server error %d: %s", resp.StatusCode, string(body)) - + delay := time.Duration(math.Pow(2, float64(attempt))) * baseDelay time.Sleep(delay) continue } - + // For client errors or final attempt, return the response // Why: 4xx errors (except 429) indicate client issues that won't // be fixed by retrying. We return these immediately to let the // caller handle the error appropriately. return resp, nil } - + if lastErr != nil { return nil, fmt.Errorf("request failed after %d retries: %w", maxRetries, lastErr) } @@ -251,12 +251,12 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac if variables != nil { payload["variables"] = variables } - + body, err := json.Marshal(payload) if err != nil { return fmt.Errorf("failed to marshal request: %w", err) } - + // Create the HTTP request // Why: We need to construct a proper HTTP POST request with the // GraphQL payload to send to Linear's API endpoint. @@ -264,7 +264,7 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac if err != nil { return fmt.Errorf("failed to create request: %w", err) } - + // Execute the request with retry logic // Why: We delegate to our retry-aware method to handle transient // failures gracefully without failing the entire operation. @@ -273,7 +273,7 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac return fmt.Errorf("failed to execute request: %w", err) } defer resp.Body.Close() - + // Read the response body // Why: We need to consume the entire response to check for errors // and decode the result, even if the status code indicates failure. @@ -281,7 +281,7 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac if err != nil { return fmt.Errorf("failed to read response: %w", err) } - + // Check for non-2xx status codes // Why: Even though makeRequestWithRetry handles retries, it still // returns error responses that we need to handle appropriately. @@ -291,39 +291,28 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac Body: string(respBody), } } - + // Decode the GraphQL response // Why: GraphQL responses have a standard structure with "data" and // "errors" fields. We need to decode this to extract the actual result. var graphQLResp struct { Data json.RawMessage `json:"data"` - Errors []struct { - Message string `json:"message"` - } `json:"errors"` + Errors []GraphQLError `json:"errors"` } - + if err := json.Unmarshal(respBody, &graphQLResp); err != nil { return fmt.Errorf("failed to decode response: %w", err) } - + // Check for GraphQL errors // Why: GraphQL can return a 200 OK status but still contain errors // in the response. We need to check for these and surface them. if len(graphQLResp.Errors) > 0 { - // Enhanced error with query context for better debugging - errMsg := graphQLResp.Errors[0].Message - - // Add query context for debugging - queryPreview := query - if len(queryPreview) > 100 { - queryPreview = queryPreview[:100] + "..." - } - - return &GraphQLError{ - Message: fmt.Sprintf("%s (query: %s)", errMsg, queryPreview), - } + // Keep the API-provided message and extensions, but do not include the + // request query because it may contain sensitive input or implementation details. + return &graphQLResp.Errors[0] } - + // Decode the data portion into the result // Why: The actual query result is nested under the "data" field. // We decode this into the caller's provided result structure. @@ -332,7 +321,7 @@ func (bc *BaseClient) ExecuteRequest(query string, variables map[string]interfac return fmt.Errorf("failed to decode response data: %w", err) } } - + return nil } @@ -343,4 +332,4 @@ func NewTestBaseClient(apiToken string, baseURL string, httpClient *http.Client) HTTPClient: httpClient, baseURL: baseURL, } -} \ No newline at end of file +} diff --git a/pkg/linear/core/base_client_test.go b/pkg/linear/core/base_client_test.go new file mode 100644 index 0000000..ab08ed9 --- /dev/null +++ b/pkg/linear/core/base_client_test.go @@ -0,0 +1,127 @@ +package core + +import ( + "errors" + "io" + "net/http" + "strings" + "testing" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +func TestBaseClientExecuteRequestGraphQLErrorPreservesExtensionsWithoutQuery(t *testing.T) { + const query = `mutation SecretOperation($token: String!) { + issueCreate(input: {description: $token}) { success } + }` + const message = "You do not have permission to perform this action" + + client := newBaseClientForResponse(`{ + "errors": [{ + "message": "` + message + `", + "extensions": { + "code": "FORBIDDEN", + "classification": "authorization", + "details": {"reason": "insufficient_scope"} + } + }] + }`) + + err := client.ExecuteRequest(query, nil, nil) + if err == nil { + t.Fatal("expected GraphQL error") + } + + var graphQLError *GraphQLError + if !errors.As(err, &graphQLError) { + t.Fatalf("expected GraphQLError, got %T: %v", err, err) + } + if !IsGraphQLError(err) { + t.Fatal("expected IsGraphQLError to classify the error") + } + if graphQLError.Message != message { + t.Errorf("message = %q, want %q", graphQLError.Message, message) + } + if got := graphQLError.Extensions["code"]; got != "FORBIDDEN" { + t.Errorf("extensions[code] = %#v, want FORBIDDEN", got) + } + if got := graphQLError.Extensions["classification"]; got != "authorization" { + t.Errorf("extensions[classification] = %#v, want authorization", got) + } + details, ok := graphQLError.Extensions["details"].(map[string]interface{}) + if !ok || details["reason"] != "insufficient_scope" { + t.Errorf("extensions[details] = %#v, want reason insufficient_scope", graphQLError.Extensions["details"]) + } + + errString := err.Error() + if !strings.Contains(errString, "code: FORBIDDEN") { + t.Errorf("error = %q, want GraphQL code classification", errString) + } + for _, queryFragment := range []string{"SecretOperation", "issueCreate", "$token", "query:"} { + if strings.Contains(errString, queryFragment) { + t.Errorf("error = %q contains query fragment %q", errString, queryFragment) + } + } +} + +func TestBaseClientExecuteRequestGraphQLErrorWithoutExtensionsRemainsTyped(t *testing.T) { + client := newBaseClientForResponse(`{"errors":[{"message":"validation failed"}]}`) + + err := client.ExecuteRequest("query PrivateQuery { viewer { id } }", nil, nil) + if err == nil { + t.Fatal("expected GraphQL error") + } + + var graphQLError *GraphQLError + if !errors.As(err, &graphQLError) { + t.Fatalf("expected GraphQLError, got %T: %v", err, err) + } + if graphQLError.Extensions != nil { + t.Errorf("extensions = %#v, want nil", graphQLError.Extensions) + } + if got, want := err.Error(), "GraphQL error: validation failed"; got != want { + t.Errorf("error = %q, want %q", got, want) + } + if strings.Contains(err.Error(), "PrivateQuery") { + t.Errorf("error = %q contains query text", err) + } +} + +func TestBaseClientExecuteRequestHTTPErrorRemainsClassified(t *testing.T) { + client := newBaseClientForResponseWithStatus(http.StatusBadRequest, `{"error":"bad request"}`) + + err := client.ExecuteRequest("query PrivateQuery { viewer { id } }", nil, nil) + if err == nil { + t.Fatal("expected HTTP error") + } + + var httpError *HTTPError + if !errors.As(err, &httpError) { + t.Fatalf("expected HTTPError, got %T: %v", err, err) + } + if httpError.StatusCode != http.StatusBadRequest { + t.Errorf("status code = %d, want %d", httpError.StatusCode, http.StatusBadRequest) + } + if httpError.Body != `{"error":"bad request"}` { + t.Errorf("body = %q, want response body preserved", httpError.Body) + } +} + +func newBaseClientForResponse(body string) *BaseClient { + return newBaseClientForResponseWithStatus(http.StatusOK, body) +} + +func newBaseClientForResponseWithStatus(statusCode int, body string) *BaseClient { + httpClient := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: statusCode, + Body: io.NopCloser(strings.NewReader(body)), + Header: make(http.Header), + }, nil + })} + return NewTestBaseClient("test-token", "https://example.test/graphql", httpClient) +} diff --git a/pkg/linear/issues/client.go b/pkg/linear/issues/client.go index 1631f88..7672d09 100644 --- a/pkg/linear/issues/client.go +++ b/pkg/linear/issues/client.go @@ -133,30 +133,30 @@ linear_create_issue("Task title", "Description", teams[0].id)`) if input.DueDate != "" { gqlInput["dueDate"] = input.DueDate } - if len(input.LabelIDs) > 0 { + if input.LabelIDs != nil { gqlInput["labelIds"] = input.LabelIDs } variables := map[string]interface{}{ "input": gqlInput, } - + var response struct { IssueCreate struct { - Success bool `json:"success"` + Success bool `json:"success"` Issue core.Issue `json:"issue"` } `json:"issueCreate"` } - + err := ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return nil, fmt.Errorf("failed to create issue: %w", err) } - + if !response.IssueCreate.Success { return nil, fmt.Errorf("issue creation was not successful") } - + // Extract metadata from description if present // Why: We store metadata in issue descriptions as hidden markdown. // After creating an issue, we need to extract this metadata to populate @@ -166,7 +166,7 @@ linear_create_issue("Task title", "Description", teams[0].id)`) response.IssueCreate.Issue.Metadata = metadata response.IssueCreate.Issue.Description = cleanDesc } - + return &response.IssueCreate.Issue, nil } @@ -180,7 +180,7 @@ func (ic *Client) GetIssue(issueID string) (*core.Issue, error) { if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + const query = ` query GetIssue($id: String!) { issue(id: $id) { @@ -275,11 +275,11 @@ func (ic *Client) GetIssue(issueID string) (*core.Issue, error) { } } ` - + variables := map[string]interface{}{ "id": issueID, } - + var response struct { Issue core.Issue `json:"issue"` } @@ -342,7 +342,7 @@ func (ic *Client) getIssueWithProjectContextInternal(issueID string) (*core.Issu if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + const query = ` query GetIssueWithProject($id: String!) { issue(id: $id) { @@ -454,14 +454,14 @@ func (ic *Client) getIssueWithProjectContextInternal(issueID string) (*core.Issu if err != nil { return nil, fmt.Errorf("failed to get issue with project context: %w", err) } - + // Extract metadata from issue description if response.Issue.Description != "" { metadata, cleanDesc := metadata.ExtractMetadataFromDescription(response.Issue.Description) response.Issue.Metadata = metadata response.Issue.Description = cleanDesc } - + // Extract metadata from project description if project exists // Why: Projects can also have metadata. When fetching project context, // we want to ensure project metadata is also extracted and available. @@ -470,7 +470,7 @@ func (ic *Client) getIssueWithProjectContextInternal(issueID string) (*core.Issu response.Issue.Project.Metadata = projectMetadata response.Issue.Project.Description = cleanProjectDesc } - + return &response.Issue, nil } @@ -500,7 +500,7 @@ func (ic *Client) getIssueWithParentContextInternal(issueID string) (*core.Issue if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + const query = ` query GetIssueWithParent($id: String!) { issue(id: $id) { @@ -613,14 +613,14 @@ func (ic *Client) getIssueWithParentContextInternal(issueID string) (*core.Issue if err != nil { return nil, fmt.Errorf("failed to get issue with parent context: %w", err) } - + // Extract metadata from issue description if response.Issue.Description != "" { metadata, cleanDesc := metadata.ExtractMetadataFromDescription(response.Issue.Description) response.Issue.Metadata = metadata response.Issue.Description = cleanDesc } - + // Extract metadata from parent description if parent exists // Why: Parent issues may contain metadata that provides context for // sub-tasks. Extracting it ensures complete metadata visibility. @@ -629,7 +629,7 @@ func (ic *Client) getIssueWithParentContextInternal(issueID string) (*core.Issue response.Issue.Parent.Metadata = parentMetadata response.Issue.Parent.Description = cleanParentDesc } - + return &response.Issue, nil } @@ -646,7 +646,7 @@ func (ic *Client) UpdateIssueState(issueID, stateID string) error { if stateID == "" { return &core.ValidationError{Field: "stateID", Message: "stateID cannot be empty"} } - + const mutation = ` mutation UpdateIssueState($issueId: String!, $stateId: String!) { issueUpdate( @@ -664,12 +664,12 @@ func (ic *Client) UpdateIssueState(issueID, stateID string) error { } } ` - + variables := map[string]interface{}{ "issueId": issueID, "stateId": stateID, } - + var response struct { IssueUpdate struct { Success bool `json:"success"` @@ -682,19 +682,19 @@ func (ic *Client) UpdateIssueState(issueID, stateID string) error { } `json:"issue"` } `json:"issueUpdate"` } - + err := ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { // Check if this is a state ID not found error // Why: The Linear API returns specific error messages when state IDs // are invalid. We want to provide helpful guidance to users. - if strings.Contains(err.Error(), "Entity not found in validateAccess: stateId") || - strings.Contains(err.Error(), "does not exist") && strings.Contains(err.Error(), "state") { + if strings.Contains(err.Error(), "Entity not found in validateAccess: stateId") || + strings.Contains(err.Error(), "does not exist") && strings.Contains(err.Error(), "state") { return guidance.InvalidStateIDError(stateID, err) } return guidance.EnhanceGenericError("update issue state", err) } - + if !response.IssueUpdate.Success { return guidance.OperationFailedError("Update issue state", "issue", []string{ "Verify the issue ID exists using linear_get_issue", @@ -702,7 +702,7 @@ func (ic *Client) UpdateIssueState(issueID, stateID string) error { "Ensure you have permission to update this issue", }) } - + return nil } @@ -713,7 +713,7 @@ func (ic *Client) AssignIssue(issueID, assigneeID string) error { if issueID == "" { return &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + const mutation = ` mutation AssignIssue($issueId: String!, $assigneeId: String) { issueUpdate( @@ -752,22 +752,22 @@ func (ic *Client) AssignIssue(issueID, assigneeID string) error { "issueId": issueID, "assigneeId": assigneeInput, } - + var response struct { IssueUpdate struct { Success bool `json:"success"` } `json:"issueUpdate"` } - + err := ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return fmt.Errorf("failed to assign issue: %w", err) } - + if !response.IssueUpdate.Success { return fmt.Errorf("issue assignment was not successful") } - + return nil } @@ -781,7 +781,7 @@ func (ic *Client) ListAssignedIssues(limit int) ([]core.Issue, error) { if limit <= 0 { limit = 50 } - + const query = ` query ListAssignedIssues($filter: IssueFilter, $first: Int) { issues(filter: $filter, first: $first) { @@ -831,7 +831,7 @@ func (ic *Client) ListAssignedIssues(limit int) ([]core.Issue, error) { } } ` - + // Filter for issues assigned to the current user // Why: The "me" identifier is Linear's way of referring to the // authenticated user without needing to know their specific ID. @@ -842,23 +842,23 @@ func (ic *Client) ListAssignedIssues(limit int) ([]core.Issue, error) { }, }, } - + variables := map[string]interface{}{ "filter": filter, "first": limit, } - + var response struct { Issues struct { Nodes []core.Issue `json:"nodes"` } `json:"issues"` } - + err := ic.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to list assigned issues: %w", err) } - + // Extract metadata from descriptions // Why: Each issue might have metadata. We extract it here to ensure // consistent metadata access across all retrieval methods. @@ -869,7 +869,7 @@ func (ic *Client) ListAssignedIssues(limit int) ([]core.Issue, error) { response.Issues.Nodes[i].Description = cleanDesc } } - + return response.Issues.Nodes, nil } @@ -884,7 +884,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. if filters.Limit <= 0 { filters.Limit = 10 // Reduced from 50 to minimize token usage } - + const query = ` query SearchIssuesEnhanced($filter: IssueFilter, $first: Int, $after: String, $includeArchived: Boolean, $orderBy: PaginationOrderBy) { issues(filter: $filter, first: $first, after: $after, includeArchived: $includeArchived, orderBy: $orderBy) { @@ -958,7 +958,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. // Build filter object filter := make(map[string]interface{}) - + // Team filter if filters.TeamID != "" { // Linear's team filter requires IDComparator format @@ -994,7 +994,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. }, } } - + // Label filters (include and/or exclude) hasIncludeLabels := len(filters.LabelIDs) > 0 hasExcludeLabels := len(filters.ExcludeLabelIDs) > 0 @@ -1029,7 +1029,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. }, } } - + // Assignee filter if filters.AssigneeID != "" { filter["assignee"] = map[string]interface{}{ @@ -1038,7 +1038,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. }, } } - + // Priority filter if filters.Priority != nil { filter["priority"] = map[string]interface{}{ @@ -1096,7 +1096,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. } filter["updatedAt"].(map[string]interface{})["lte"] = filters.UpdatedBefore } - + variables := map[string]interface{}{ "first": filters.Limit, } @@ -1113,7 +1113,7 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. // Always include the includeArchived parameter (defaults to false) variables["includeArchived"] = filters.IncludeArchived - + // Add orderBy if specified if filters.OrderBy != "" { variables["orderBy"] = filters.OrderBy @@ -1128,12 +1128,12 @@ func (ic *Client) SearchIssuesEnhanced(filters *core.IssueSearchFilters) (*core. } `json:"pageInfo"` } `json:"issues"` } - + err := ic.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to search issues: %w", err) } - + return &core.IssueSearchResult{ Issues: response.Issues.Nodes, HasNextPage: response.Issues.PageInfo.HasNextPage, @@ -1148,7 +1148,7 @@ func (ic *Client) BatchUpdateIssues(issueIDs []string, update core.BatchIssueUpd if len(issueIDs) == 0 { return nil, fmt.Errorf("no issue IDs provided") } - + const mutation = ` mutation BatchUpdateIssues($issueIds: [String!]!, $input: IssueUpdateInput!) { issueBatchUpdate(ids: $issueIds, input: $input) { @@ -1191,10 +1191,10 @@ func (ic *Client) BatchUpdateIssues(issueIDs []string, update core.BatchIssueUpd } } ` - + // Build the update input input := make(map[string]interface{}) - + if update.StateID != "" { input["stateId"] = update.StateID } @@ -1210,25 +1210,25 @@ func (ic *Client) BatchUpdateIssues(issueIDs []string, update core.BatchIssueUpd if update.ProjectID != "" { input["projectId"] = update.ProjectID } - + if len(input) == 0 { return nil, fmt.Errorf("no update fields provided") } - + variables := map[string]interface{}{ "issueIds": issueIDs, "input": input, } - + var response struct { IssueBatchUpdate core.BatchIssueUpdateResult `json:"issueBatchUpdate"` } - + err := ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return nil, fmt.Errorf("failed to batch update issues: %w", err) } - + return &response.IssueBatchUpdate, nil } @@ -1240,13 +1240,13 @@ func (ic *Client) GetIssueWithBestContext(issueID string) (*core.Issue, error) { if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + // First, get basic issue info to determine what context to fetch issue, err := ic.GetIssue(issueID) if err != nil { return nil, fmt.Errorf("failed to get issue: %w", err) } - + // Determine the best context based on what the issue has if issue.Parent != nil && issue.Parent.ID != "" { // Issue has a parent - fetch with parent context for sibling information @@ -1256,7 +1256,7 @@ func (ic *Client) GetIssueWithBestContext(issueID string) (*core.Issue, error) { return issue, nil } return parentContextIssue, nil - + } else if issue.Project != nil && issue.Project.ID != "" { // Issue has a project but no parent - fetch with project context projectContextIssue, err := ic.GetIssueWithProjectContext(issueID) @@ -1266,7 +1266,7 @@ func (ic *Client) GetIssueWithBestContext(issueID string) (*core.Issue, error) { } return projectContextIssue, nil } - + // Standalone issue - we already have all the data we need return issue, nil } @@ -1278,7 +1278,7 @@ func (ic *Client) GetSubIssues(parentIssueID string) ([]core.SubIssue, error) { if parentIssueID == "" { return nil, &core.ValidationError{Field: "parentIssueID", Message: "parentIssueID cannot be empty"} } - + const query = ` query GetSubIssues($id: String!) { issue(id: $id) { @@ -1296,11 +1296,11 @@ func (ic *Client) GetSubIssues(parentIssueID string) ([]core.SubIssue, error) { } } ` - + variables := map[string]interface{}{ "id": parentIssueID, } - + var response struct { Issue struct { Children struct { @@ -1308,12 +1308,12 @@ func (ic *Client) GetSubIssues(parentIssueID string) ([]core.SubIssue, error) { } `json:"children"` } `json:"issue"` } - + err := ic.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to get sub-issues: %w", err) } - + return response.Issue.Children.Nodes, nil } @@ -1324,7 +1324,7 @@ func (ic *Client) UpdateIssueDescription(issueID, newDescription string) error { if issueID == "" { return &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + // First, get the current issue to preserve metadata // Why: We need to extract existing metadata before updating the description // to ensure we don't lose any stored metadata during the update. @@ -1332,7 +1332,7 @@ func (ic *Client) UpdateIssueDescription(issueID, newDescription string) error { if err != nil { return fmt.Errorf("failed to get current issue: %w", err) } - + // Preserve existing metadata // Why: The issue.Metadata field contains the extracted metadata from the // current description. We need to inject this back into the new description. @@ -1340,7 +1340,7 @@ func (ic *Client) UpdateIssueDescription(issueID, newDescription string) error { if issue.Metadata != nil && len(issue.Metadata) > 0 { descriptionWithMetadata = metadata.InjectMetadataIntoDescription(newDescription, issue.Metadata) } - + const mutation = ` mutation UpdateIssueDescription($issueId: String!, $description: String!) { issueUpdate( @@ -1351,27 +1351,27 @@ func (ic *Client) UpdateIssueDescription(issueID, newDescription string) error { } } ` - + variables := map[string]interface{}{ "issueId": issueID, "description": descriptionWithMetadata, } - + var response struct { IssueUpdate struct { Success bool `json:"success"` } `json:"issueUpdate"` } - + err = ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return fmt.Errorf("failed to update issue description: %w", err) } - + if !response.IssueUpdate.Success { return fmt.Errorf("issue description update was not successful") } - + return nil } @@ -1396,7 +1396,7 @@ func (ic *Client) UpdateIssueMetadataKey(issueID, key string, value interface{}) if err != nil { return fmt.Errorf("failed to get current issue: %w", err) } - + // Initialize metadata if needed and update the key // Why: The issue might not have any metadata yet. We initialize // it as an empty map if needed before adding the new key. @@ -1404,12 +1404,12 @@ func (ic *Client) UpdateIssueMetadataKey(issueID, key string, value interface{}) issue.Metadata = make(map[string]interface{}) } issue.Metadata[key] = value - + // Update the description with new metadata // Why: Metadata is stored in the description field. We need to // inject the updated metadata back into the description. descriptionWithMetadata := metadata.InjectMetadataIntoDescription(issue.Description, issue.Metadata) - + const mutation = ` mutation UpdateIssueDescription($issueId: String!, $description: String!) { issueUpdate( @@ -1420,27 +1420,27 @@ func (ic *Client) UpdateIssueMetadataKey(issueID, key string, value interface{}) } } ` - + variables := map[string]interface{}{ "issueId": issueID, "description": descriptionWithMetadata, } - + var response struct { IssueUpdate struct { Success bool `json:"success"` } `json:"issueUpdate"` } - + err = ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return fmt.Errorf("failed to update issue metadata: %w", err) } - + if !response.IssueUpdate.Success { return fmt.Errorf("issue metadata update was not successful") } - + return nil } @@ -1454,13 +1454,13 @@ func (ic *Client) RemoveIssueMetadataKey(issueID, key string) error { if key == "" { return &core.ValidationError{Field: "key", Message: "key cannot be empty"} } - + // Get current issue issue, err := ic.GetIssue(issueID) if err != nil { return fmt.Errorf("failed to get current issue: %w", err) } - + // Remove the key if metadata exists // Why: We only proceed if there's metadata and the key exists. // No need to update if there's nothing to remove. @@ -1530,7 +1530,7 @@ func (ic *Client) GetIssueSimplified(issueID string) (*core.Issue, error) { if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + const query = ` query GetIssueSimplified($id: String!) { issue(id: $id) { @@ -1591,30 +1591,30 @@ func (ic *Client) GetIssueSimplified(issueID string) (*core.Issue, error) { } } ` - + variables := map[string]interface{}{ "id": issueID, } - + var response struct { Issue core.Issue `json:"issue"` } - + err := ic.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to get issue (simplified): %w", err) } - + // Extract metadata from description if response.Issue.Description != "" { metadata, cleanDesc := metadata.ExtractMetadataFromDescription(response.Issue.Description) response.Issue.Metadata = metadata response.Issue.Description = cleanDesc } - + // Initialize empty children to maintain consistency response.Issue.Children.Nodes = []core.SubIssue{} - + return &response.Issue, nil } @@ -1628,7 +1628,7 @@ func (ic *Client) GetIssueWithFallback(issueID string) (*core.Issue, error) { if err == nil { return issue, nil } - + // Check if it's a server error (500) or complexity error // Need to unwrap the error to check for HTTPError var httpErr *core.HTTPError @@ -1636,7 +1636,7 @@ func (ic *Client) GetIssueWithFallback(issueID string) (*core.Issue, error) { // Try simplified query return ic.GetIssueSimplified(issueID) } - + // For other errors, return the original error return nil, err } @@ -1650,31 +1650,31 @@ func (ic *Client) UpdateIssue(issueID string, input core.UpdateIssueInput) (*cor if issueID == "" { return nil, &core.ValidationError{Field: "issueID", Message: "issueID cannot be empty"} } - + // Check if there are any fields to update if !hasFieldsToUpdate(input) { return nil, &core.ValidationError{Field: "input", Message: "no fields to update"} } - + // Validate priority if provided if input.Priority != nil && (*input.Priority < 0 || *input.Priority > 4) { return nil, &core.ValidationError{Field: "priority", Message: fmt.Sprintf("invalid priority value: %d (must be between 0-4)", *input.Priority)} } - + // If updating description, preserve existing metadata if input.Description != nil { issue, err := ic.GetIssue(issueID) if err != nil { return nil, fmt.Errorf("failed to get current issue for metadata preservation: %w", err) } - + // Preserve metadata in the new description if issue.Metadata != nil && len(issue.Metadata) > 0 { descWithMetadata := metadata.InjectMetadataIntoDescription(*input.Description, issue.Metadata) input.Description = &descWithMetadata } } - + // Build the GraphQL mutation const mutation = ` mutation UpdateIssue($issueId: String!, $input: IssueUpdateInput!) { @@ -1737,38 +1737,38 @@ func (ic *Client) UpdateIssue(issueID string, input core.UpdateIssueInput) (*cor } } ` - + // Build the input object updateInput := buildUpdateInput(input) - + variables := map[string]interface{}{ "issueId": issueID, "input": updateInput, } - + var response struct { IssueUpdate struct { - Success bool `json:"success"` + Success bool `json:"success"` Issue core.Issue `json:"issue"` } `json:"issueUpdate"` } - + err := ic.base.ExecuteRequest(mutation, variables, &response) if err != nil { return nil, fmt.Errorf("failed to update issue: %w", err) } - + if !response.IssueUpdate.Success { return nil, fmt.Errorf("issue update was not successful") } - + // Extract metadata from description if present if response.IssueUpdate.Issue.Description != "" { metadata, cleanDesc := metadata.ExtractMetadataFromDescription(response.IssueUpdate.Issue.Description) response.IssueUpdate.Issue.Metadata = metadata response.IssueUpdate.Issue.Description = cleanDesc } - + return &response.IssueUpdate.Issue, nil } @@ -1786,7 +1786,7 @@ func hasFieldsToUpdate(input core.UpdateIssueInput) bool { input.ParentID != nil || input.TeamID != nil || input.CycleID != nil || - len(input.LabelIDs) > 0 + input.LabelIDs != nil } // buildUpdateInput builds the GraphQL input object from UpdateIssueInput @@ -1820,7 +1820,7 @@ func buildUpdateInput(input core.UpdateIssueInput) map[string]interface{} { if input.TeamID != nil { updateInput["teamId"] = *input.TeamID } - if len(input.LabelIDs) > 0 { + if input.LabelIDs != nil { updateInput["labelIds"] = input.LabelIDs } if input.CycleID != nil { @@ -1967,20 +1967,20 @@ func (ic *Client) ListAllIssues(filter *core.IssueFilter) (*core.ListAllIssuesRe var response struct { Issues struct { Nodes []struct { - ID string `json:"id"` - Identifier string `json:"identifier"` - Title string `json:"title"` - Description string `json:"description"` - Priority int `json:"priority"` - CreatedAt string `json:"createdAt"` - UpdatedAt string `json:"updatedAt"` + ID string `json:"id"` + Identifier string `json:"identifier"` + Title string `json:"title"` + Description string `json:"description"` + Priority int `json:"priority"` + CreatedAt string `json:"createdAt"` + UpdatedAt string `json:"updatedAt"` State core.WorkflowState `json:"state"` Assignee *core.User `json:"assignee"` Labels struct { Nodes []core.Label `json:"nodes"` } `json:"labels"` Project *core.Project `json:"project"` - Team core.Team `json:"team"` + Team core.Team `json:"team"` } `json:"nodes"` PageInfo struct { HasNextPage bool `json:"hasNextPage"` @@ -2169,7 +2169,7 @@ func buildOrderByObject(field, direction string) map[string]interface{} { func (ic *Client) ListIssueAttachments(issueID string) ([]core.Attachment, error) { // Validate input if issueID == "" { - return nil, guidance.ValidationErrorWithExample("issueID", "cannot be empty", + return nil, guidance.ValidationErrorWithExample("issueID", "cannot be empty", `// First get an issue ID issue = linear_get_issue("some-issue-id") // Then list its attachments @@ -2421,4 +2421,4 @@ func (ic *Client) GetTeamIssuesWithRelations(teamID string, limit int) ([]core.I } return response.Issues.Nodes, nil -} \ No newline at end of file +} diff --git a/pkg/linear/issues/client_test.go b/pkg/linear/issues/client_test.go index 14f63d0..4d300c2 100644 --- a/pkg/linear/issues/client_test.go +++ b/pkg/linear/issues/client_test.go @@ -7,6 +7,21 @@ import ( "github.com/joa23/linear-cli/pkg/linear/core" ) +func TestBuildUpdateInput_PreservesExplicitEmptyLabels(t *testing.T) { + input := core.UpdateIssueInput{LabelIDs: []string{}} + result := buildUpdateInput(input) + labels, ok := result["labelIds"] + if !ok { + t.Fatal("labelIds missing for explicit empty label list") + } + if got := labels.([]string); got == nil || len(got) != 0 { + t.Fatalf("labelIds = %#v, want non-nil empty slice", labels) + } + if !hasFieldsToUpdate(input) { + t.Fatal("explicit empty label list should trigger update") + } +} + func TestBuildUpdateInput_DelegateID(t *testing.T) { tests := []struct { name string diff --git a/pkg/linear/resolver.go b/pkg/linear/resolver.go index 327b007..b469c35 100644 --- a/pkg/linear/resolver.go +++ b/pkg/linear/resolver.go @@ -6,6 +6,7 @@ import ( "github.com/joa23/linear-cli/pkg/linear/identifiers" "fmt" + "sort" "strings" "time" ) @@ -545,65 +546,95 @@ func (r *Resolver) ResolveProject(nameOrID string, teamID string) (string, error return project.ID, nil } -// ResolveLabel resolves a label name to a label UUID within a specific team -// Labels are team-scoped, so teamID is required -// -// Returns error if label not found +// ResolveLabel resolves a label name or UUID to a label UUID within a specific team. +// Labels are team-scoped, so teamID is required. func (r *Resolver) ResolveLabel(labelName string, teamID string) (string, error) { - // Validate input - if labelName == "" { - return "", &core.ValidationError{ - Field: "label", - Message: "label name cannot be empty", - } + label, err := r.ResolveLabelMetadata(labelName, teamID) + if err != nil { + return "", err } + return label.ID, nil +} +// ResolveLabelMetadata resolves a label and retains its parent metadata for +// callers that need to validate relationships between labels. +func (r *Resolver) ResolveLabelMetadata(labelName string, teamID string) (*core.Label, error) { + labelName = strings.TrimSpace(labelName) + teamID = strings.TrimSpace(teamID) + if labelName == "" { + return nil, &core.ValidationError{Field: "label", Message: "label name cannot be empty"} + } if teamID == "" { - return "", &core.ValidationError{ - Field: "teamId", - Message: "team ID is required for label resolution", - } + return nil, &core.ValidationError{Field: "teamId", Message: "team ID is required for label resolution"} } - // Check cache first - if labelID, found := r.cache.getLabelByName(teamID, labelName); found { - return labelID, nil + if identifiers.IsUUID(labelName) { + if label, found := r.cache.getLabelByID(teamID, labelName); found { + return &label, nil + } + } else if labelID, found := r.cache.getLabelByName(teamID, labelName); found { + if label, found := r.cache.getLabelByID(teamID, labelID); found { + return &label, nil + } } - // Fetch labels for the team labels, err := r.client.Teams.ListLabels(teamID) if err != nil { - return "", fmt.Errorf("failed to list labels for resolution: %w", err) + return nil, fmt.Errorf("failed to list labels for resolution: %w", err) } + r.cache.setLabels(teamID, labels) - // Find matching label by name (case-insensitive) - nameLower := strings.ToLower(labelName) - for _, label := range labels { - if strings.ToLower(label.Name) == nameLower { - // Cache and return - r.cache.setLabelByName(teamID, labelName, label.ID) - return label.ID, nil + var matches []core.Label + if identifiers.IsUUID(labelName) { + for _, label := range labels { + if strings.EqualFold(label.ID, labelName) { + matches = append(matches, label) + break + } + } + } else { + nameLower := strings.ToLower(labelName) + for _, label := range labels { + if strings.ToLower(label.Name) == nameLower { + matches = append(matches, label) + } } } - // No match found - build helpful error with suggestions - var availableLabels []string - for _, label := range labels { - availableLabels = append(availableLabels, label.Name) + if len(matches) == 0 { + availableLabels := make([]string, 0, len(labels)) + for _, label := range labels { + availableLabels = append(availableLabels, label.Name) + } + return nil, &guidance.ErrorWithGuidance{ + Operation: "Resolve label", + Reason: fmt.Sprintf("label '%s' not found in team", labelName), + Guidance: []string{ + "Check the label name spelling", + "Use 'linear teams labels ' to see available labels", + "Create the label in Linear if it doesn't exist", + }, + Example: fmt.Sprintf("Available labels: %s", strings.Join(availableLabels, ", ")), + OriginalErr: &core.NotFoundError{ResourceType: "label", ResourceID: labelName}, + } } - - return "", &guidance.ErrorWithGuidance{ - Operation: "Resolve label", - Reason: fmt.Sprintf("label '%s' not found in team", labelName), - Guidance: []string{ - "Check the label name spelling", - "Use 'linear teams labels ' to see available labels", - "Create the label in Linear if it doesn't exist", - }, - Example: fmt.Sprintf("Available labels: %s", strings.Join(availableLabels, ", ")), - OriginalErr: &core.NotFoundError{ - ResourceType: "label", - ResourceID: labelName, - }, + if len(matches) > 1 { + matching := make([]string, 0, len(matches)) + for _, label := range matches { + matching = append(matching, fmt.Sprintf("%s (ID: %s)", label.Name, label.ID)) + } + sort.Strings(matching) + return nil, &guidance.ErrorWithGuidance{ + Operation: "Resolve label", + Reason: fmt.Sprintf("multiple labels match '%s'", labelName), + Guidance: []string{"Use the label UUID for exact matching", "Choose from the matching labels below"}, + Example: fmt.Sprintf("Matching labels: %s", strings.Join(matching, ", ")), + OriginalErr: &core.ValidationError{ + Field: "label", Value: labelName, Reason: "ambiguous label name", + }, + } } + + label := matches[0] + return &label, nil } diff --git a/pkg/linear/resolver_cache.go b/pkg/linear/resolver_cache.go index d2abb55..01ece23 100644 --- a/pkg/linear/resolver_cache.go +++ b/pkg/linear/resolver_cache.go @@ -1,9 +1,11 @@ package linear import ( - + "strings" "sync" "time" + + "github.com/joa23/linear-cli/pkg/linear/core" ) // cacheEntry represents a cached value with expiration time @@ -12,6 +14,11 @@ type cacheEntry struct { expiresAt time.Time } +type labelCacheEntry struct { + label core.Label + expiresAt time.Time +} + // isExpired checks if the cache entry has expired func (e *cacheEntry) isExpired() bool { return time.Now().After(e.expiresAt) @@ -34,8 +41,10 @@ type resolverCache struct { // Issue resolution cache issueByIdentifier map[string]*cacheEntry // CEN-123 → issueID - // Label resolution cache (keyed by teamID:labelName) - labelByName map[string]*cacheEntry // teamID:labelName → labelID + // Label resolution caches are keyed by normalized team and label values. + labelByName map[string]*cacheEntry // teamID:normalized-name → labelID + labelByID map[string]*cacheEntry // teamID:normalized-id → labelID + labelData map[string]*labelCacheEntry // teamID:normalized-id → label metadata // Project resolution cache projectByName map[string]*cacheEntry // project name → projectID @@ -53,6 +62,8 @@ func newResolverCache(ttl time.Duration) *resolverCache { teamByKey: make(map[string]*cacheEntry), issueByIdentifier: make(map[string]*cacheEntry), labelByName: make(map[string]*cacheEntry), + labelByID: make(map[string]*cacheEntry), + labelData: make(map[string]*labelCacheEntry), projectByName: make(map[string]*cacheEntry), ttl: ttl, } @@ -176,29 +187,71 @@ func (rc *resolverCache) setIssueByIdentifier(identifier, issueID string) { // Label cache methods +func labelKey(teamID, value string) string { + return strings.ToLower(strings.TrimSpace(teamID)) + ":" + strings.ToLower(strings.TrimSpace(value)) +} + +func cloneLabel(label core.Label) core.Label { + copy := label + if label.Parent != nil { + parent := *label.Parent + copy.Parent = &parent + } + return copy +} + func (rc *resolverCache) getLabelByName(teamID, labelName string) (string, bool) { rc.mu.RLock() defer rc.mu.RUnlock() - key := teamID + ":" + labelName - entry, exists := rc.labelByName[key] + entry, exists := rc.labelByName[labelKey(teamID, labelName)] if !exists || entry.isExpired() { return "", false } return entry.value, true } +func (rc *resolverCache) getLabelByID(teamID, labelID string) (core.Label, bool) { + rc.mu.RLock() + defer rc.mu.RUnlock() + + entry, exists := rc.labelData[labelKey(teamID, labelID)] + if !exists || entry.expiresAt.Before(time.Now()) { + return core.Label{}, false + } + return cloneLabel(entry.label), true +} + func (rc *resolverCache) setLabelByName(teamID, labelName, labelID string) { rc.mu.Lock() defer rc.mu.Unlock() - key := teamID + ":" + labelName - rc.labelByName[key] = &cacheEntry{ + rc.labelByName[labelKey(teamID, labelName)] = &cacheEntry{ value: labelID, expiresAt: time.Now().Add(rc.ttl), } } +func (rc *resolverCache) setLabels(teamID string, labels []core.Label) { + rc.mu.Lock() + defer rc.mu.Unlock() + + expiresAt := time.Now().Add(rc.ttl) + nameCounts := make(map[string]int, len(labels)) + for _, label := range labels { + nameCounts[labelKey(teamID, label.Name)]++ + } + for _, label := range labels { + copy := cloneLabel(label) + idKey := labelKey(teamID, label.ID) + rc.labelByID[idKey] = &cacheEntry{value: label.ID, expiresAt: expiresAt} + rc.labelData[idKey] = &labelCacheEntry{label: copy, expiresAt: expiresAt} + if nameCounts[labelKey(teamID, label.Name)] == 1 { + rc.labelByName[labelKey(teamID, label.Name)] = &cacheEntry{value: label.ID, expiresAt: expiresAt} + } + } +} + // Project cache methods func (rc *resolverCache) getProjectByName(name string) (string, bool) { @@ -265,12 +318,21 @@ func (rc *resolverCache) cleanup() { } } - // Clean up label cache for key, entry := range rc.labelByName { if entry.expiresAt.Before(now) { delete(rc.labelByName, key) } } + for key, entry := range rc.labelByID { + if entry.expiresAt.Before(now) { + delete(rc.labelByID, key) + } + } + for key, entry := range rc.labelData { + if entry.expiresAt.Before(now) { + delete(rc.labelData, key) + } + } // Clean up project cache for name, entry := range rc.projectByName { @@ -304,5 +366,7 @@ func (rc *resolverCache) clear() { rc.teamByKey = make(map[string]*cacheEntry) rc.issueByIdentifier = make(map[string]*cacheEntry) rc.labelByName = make(map[string]*cacheEntry) + rc.labelByID = make(map[string]*cacheEntry) + rc.labelData = make(map[string]*labelCacheEntry) rc.projectByName = make(map[string]*cacheEntry) } diff --git a/pkg/linear/resolver_cache_test.go b/pkg/linear/resolver_cache_test.go new file mode 100644 index 0000000..e16bfa8 --- /dev/null +++ b/pkg/linear/resolver_cache_test.go @@ -0,0 +1,42 @@ +package linear + +import ( + "testing" + "time" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +func TestResolverCacheStoresNormalizedLabelMetadata(t *testing.T) { + cache := newResolverCache(time.Hour) + defer cache.clear() + + parent := &core.LabelRef{ID: "parent-1", Name: "Issue Type"} + cache.setLabels("TEAM-1", []core.Label{{ID: "LABEL-1", Name: "Tests", Parent: parent}}) + + id, ok := cache.getLabelByName("team-1", " TESTS ") + if !ok || id != "LABEL-1" { + t.Fatalf("cached name = %q, %v; want LABEL-1, true", id, ok) + } + label, ok := cache.getLabelByID("team-1", "label-1") + if !ok || label.Parent == nil || label.Parent.Name != "Issue Type" { + t.Fatalf("cached metadata = %#v, %v", label, ok) + } + label.Parent.Name = "mutated" + again, ok := cache.getLabelByID("team-1", "LABEL-1") + if !ok || again.Parent.Name != "Issue Type" { + t.Fatalf("cache returned mutable metadata: %#v, %v", again, ok) + } +} + +func TestResolverCacheDoesNotCacheAmbiguousLabelNames(t *testing.T) { + cache := newResolverCache(time.Hour) + defer cache.clear() + cache.setLabels("team", []core.Label{ + {ID: "one", Name: "Duplicate"}, + {ID: "two", Name: "duplicate"}, + }) + if _, ok := cache.getLabelByName("TEAM", "duplicate"); ok { + t.Fatal("ambiguous label name should not be cached") + } +} diff --git a/pkg/linear/resolver_label_test.go b/pkg/linear/resolver_label_test.go new file mode 100644 index 0000000..4fdfa5b --- /dev/null +++ b/pkg/linear/resolver_label_test.go @@ -0,0 +1,58 @@ +package linear + +import ( + "encoding/json" + "net/http" + "sync/atomic" + "testing" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +func TestResolverResolveLabelMetadataSupportsNamesUUIDsAndCache(t *testing.T) { + var requests atomic.Int32 + server, client := setupMockServer(t, func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + var request struct { + Variables struct { + TeamID string `json:"teamId"` + } `json:"variables"` + } + _ = json.NewDecoder(r.Body).Decode(&request) + nodes := []core.Label{} + if request.Variables.TeamID == "team-1" { + nodes = []core.Label{ + {ID: "11111111-1111-1111-1111-111111111111", Name: "Tests", Parent: &core.LabelRef{ID: "22222222-2222-2222-2222-222222222222", Name: "Issue Type"}}, + {ID: "33333333-3333-3333-3333-333333333333", Name: "Standalone"}, + } + } + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "data": map[string]interface{}{ + "team": map[string]interface{}{ + "labels": map[string]interface{}{"nodes": nodes}, + }, + }, + }) + }) + defer server.Close() + + resolver := NewResolver(client) + label, err := resolver.ResolveLabelMetadata(" tests ", "team-1") + if err != nil || label.ID != "11111111-1111-1111-1111-111111111111" || label.Parent == nil { + t.Fatalf("name resolution = %#v, %v", label, err) + } + label.Parent.Name = "mutated" + byID, err := resolver.ResolveLabelMetadata("11111111-1111-1111-1111-111111111111", "team-1") + if err != nil || byID.Parent == nil || byID.Parent.Name != "Issue Type" { + t.Fatalf("UUID resolution = %#v, %v", byID, err) + } + if requests.Load() != 1 { + t.Fatalf("label endpoint requests = %d, want 1", requests.Load()) + } + if _, err := resolver.ResolveLabel("11111111-1111-1111-1111-111111111111", "other-team"); err == nil { + t.Fatal("cross-team UUID unexpectedly resolved") + } + if requests.Load() != 2 { + t.Fatalf("cross-team lookup requests = %d, want 2", requests.Load()) + } +} diff --git a/plan.md b/plan.md new file mode 100644 index 0000000..0e3de68 --- /dev/null +++ b/plan.md @@ -0,0 +1,170 @@ +# Implementation Plan + +## Task Overview +- **Source**: TL-562 +- **Title**: Prevent exclusive-label conflicts and improve label/error output +- **Description**: Detect mutually exclusive child-label selections locally before issue create/update mutations, report the conflicting labels and parent group clearly, remove duplicated/raw GraphQL error output, and make both default label listings communicate parent-child exclusivity while preserving JSON compatibility. + +## Requirements Analysis +- **Core Functionality**: + - Resolve labels by case-insensitive name or UUID while preserving canonical label metadata (`ID`, `Name`, and optional `Parent`). + - Treat two or more distinct child labels with the same non-nil parent as an exclusive-group conflict. Parent/group labels (`Parent == nil`) and standalone labels do not conflict. + - Validate create selections before `CreateIssue` and validate the final label set for update replace/add operations. Additive updates must include labels already attached to the issue; remove-only updates must not reject a set merely because it reduces or preserves an existing selection. + - Report every conflicting sibling deterministically, including three-way conflicts, multiple groups, duplicate inputs, case variants, UUID inputs, and names containing punctuation. + - Render grouped text output for both `linear teams labels` and `linear labels list`; retain the existing machine-readable label JSON structure and parent metadata. + - Ensure normal CLI errors contain one operation context and never expose raw GraphQL query/mutation previews. +- **Acceptance Criteria**: + - Invalid create/replace/add selections fail locally and make zero relevant mutation calls. + - Conflict errors identify the shared group and all conflicting child labels in stable order with actionable wording. + - Duplicate/case-variant inputs do not create duplicate conflict entries; valid selections from different groups and standalone labels continue to work. + - Add/remove update behavior accounts for the final set, and removing the final label still sends an explicit empty label list. + - UUID and existing name resolution contracts work, including parent/group-label resolution and unknown-label guidance. + - Text listings clearly distinguish group headers and child alternatives; JSON output remains structurally usable for automation. + - GraphQL errors retain useful structured/type information but contain no query text; create/update output has no duplicated operation prefix. + - Automated tests cover resolver/cache behavior, conflict validation, mutation suppression, update modes, grouped output, JSON compatibility, and unrelated API errors. +- **Technical Constraints**: + - Keep public `ResolveLabel`/`ResolveLabelIdentifier` string-returning APIs compatible where possible; add a metadata-returning resolver API for service validation rather than forcing metadata through existing callers. + - The parent relationship returned by Linear is the authoritative exclusivity signal for this ticket. + - Preserve nil-versus-non-nil label slices so `nil` means no label field supplied while a non-nil empty slice means clear all labels. + - Do not run mutating live Linear commands. Verification is repository build/unit testing only; live read-only checks are optional. +- **Integration Points**: + - `internal/service/issue.go` and `internal/cli/issues.go` issue flows. + - `pkg/linear/resolver.go`, `pkg/linear/resolver_cache.go`, and the service-facing client interface. + - `pkg/linear/core/types.go` label metadata and `pkg/linear/core/errors.go` GraphQL errors. + - `pkg/linear/issues/client.go` issue mutation wrappers and update input serialization. + - `internal/service/team.go`, `internal/service/label.go`, and a shared `internal/format` label renderer. + +## Codebase Analysis +- **Existing Patterns**: + - `Resolver.ResolveLabel` currently fetches team labels and returns only an ID; `resolverCache` caches only `teamID:labelName -> labelID` and does not normalize keys or retain parent metadata. + - `core.Label` already contains `Parent *LabelRef`, and `teams.Client.ListLabels` and issue reads already query parent IDs/names. + - `IssueService.Create` resolves labels before its single create mutation. `IssueService.Update` fetches the issue, then builds replace/add/remove label sets, but currently loses ordering and skips an explicit empty final set. + - Team and label services marshal raw labels for JSON but duplicate flat text formatting logic. + - `BaseClient.ExecuteRequest` currently adds a truncated query preview to `GraphQLError`; issue client, service, and CLI each add overlapping create/update context. + - Existing test infrastructure includes service mocks and GraphQL mock transport/server helpers, but lacks conflict, formatter, and GraphQL error regression coverage. +- **Available Infrastructure**: + - `make build` and `make test` invoke Go build and `go test -v ./...`. + - `pkg/linear/resolver_test.go`, `internal/service/issue_create_test.go`, `internal/service/issue_relation_test.go`, `pkg/linear/issues/client_test.go`, and `pkg/linear/teams/client_test.go` provide nearby test patterns. + - `pkg/linear/testutil/mock_transport.go`/`mock_server.go` can assert request counts and payloads without a network round trip. + - Existing `guidance.ErrorWithGuidance`, `core.ValidationError`, and typed `core.GraphQLError` should be reused/extended rather than introducing ad hoc string-only errors. +- **Dependencies**: No new external dependency is expected. Changes to `IssueClientOperations` or resolver return types require updating all service mock implementations and client delegates. +- **Architecture Notes**: + - Add a resolver method such as `ResolveLabelMetadata(labelName, teamID) (*core.Label, error)` and a corresponding internal/public client delegate. Keep the existing ID-only methods delegating to it. Exact UUID matches must be constrained to the requested team's returned label set; case-insensitive name collisions must produce deterministic ambiguity guidance instead of selecting an arbitrary API-order match. Parent/group labels remain resolvable exactly as they are today; the validator only treats labels with non-nil `Parent` as children. + - Populate metadata caches for all labels returned by one team listing, keyed by normalized name and ID, so resolving multiple labels or ID inputs does not discard metadata or cause avoidable repeat requests. Normalize at lookup time, extend cleanup to every new index, and return deep copies (including `Parent`) so callers cannot mutate cached records. Because conflict detection depends on a complete team label set, either add pagination to `ListLabels` or document and test the API's complete-result guarantee before relying on it. + - Add a pure validator/typed conflict error in the service/domain layer. Deduplicate resolved labels by ID for both validation and mutation payloads, group children by `Parent.ID`, sort groups and canonical `Label.Name` values, and quote canonical names/group names safely in user-facing output. + - For create, resolve all labels to metadata, validate, then pass canonical, deduplicated, sorted IDs to the mutation. For update replace, resolve supplied labels and validate that set. For add/remove, reuse parent metadata already returned on the existing issue when available, hydrate missing metadata from the target team's complete label set, construct a deterministic final set from current issue labels plus additions minus removals, and validate when additions can introduce a conflict. If metadata remains unavailable, fail validation clearly rather than silently treating the label as standalone. Preserve explicit empty output for remove-all. If an update changes teams, permit replacement labels resolved in the target team but reject additive/removal label operations combined with a team change before mutation so source-team IDs cannot be submitted to the target team. + - Define the error boundary explicitly: the CLI adds no create/update wrapper; `IssueService` adds one `failed to create/update issue` context for local validation failures; `pkg/linear/issues.Client` retains one context for remote create/update mutation failures; the service must propagate those remote errors without wrapping them again. Resolver and unrelated service errors retain their existing operation-specific guidance. + - Remove query previews from `BaseClient` globally and decode/preserve GraphQL `extensions`; test that typed errors remain useful without implementation internals. + - Add a shared label text formatter that reconstructs groups independent of API order, renders each exclusive parent header once with indented children, preserves descriptions/IDs appropriate to each command, uses `Parent.Name`/ID as a fallback header when a parent record is absent, and sorts groups/children deterministically. Keep JSON paths as direct `core.Label` marshaling. + +## Implementation Strategy +- **Problem Complexity**: Complex, cross-layer bugfix with compatibility-sensitive resolver, update, formatter, and shared error behavior. +- **Core Problem**: Label parent metadata is available at the API boundary but discarded before validation, while multiple layers expose the same low-level mutation error and list labels without their group structure. +- **Approach**: Preserve existing public ID-based APIs, add metadata-aware resolution and caching, centralize pure conflict validation before mutations, make update label-set semantics explicit, centralize grouped text formatting, and establish one user-facing error boundary. Implement and verify in gated phases. +- **Testing Approach**: Automated unit and mock-transport tests after each implementation phase, followed by `make build` and `make test`. Do not perform live mutating verification. +- **Phases**: + 1. **Metadata-aware label resolution and conflict domain logic** + - **Implementation**: + - Extend resolver/cache storage to retain complete `core.Label` records, normalize name/ID lookup keys at lookup and population time, populate name and UUID indexes from a complete/paginated team-label fetch, and accept UUID inputs while retaining case-insensitive name matching and guidance for unknown or ambiguous labels. Extend cache cleanup for every new index and return deep copies of cached metadata. + - Add the metadata resolver delegate and update the minimal service interface plus all mocks/delegates. + - Add a deterministic conflict validator/error that groups distinct child IDs by non-nil parent, reports all siblings/groups in stable order using canonical names, handles duplicates/case variants, and leaves parent labels/standalone labels valid. Define a clear failure when an existing label cannot be hydrated with authoritative parent metadata. + - **Verification**: + - Add resolver/cache tests for name, case, UUID, punctuation, duplicate lookups, cache reuse, unknown labels, ambiguous names, cross-team UUID rejection, parent labels, deep-copy safety, and cleanup of all label indexes. + - Add pure validator tests for two-way/three-way conflicts, multiple groups, duplicate inputs, different groups, standalone labels, unavailable metadata, and deterministic actionable formatting. Run the focused Go tests. + 2. **Create and update validation before mutation** + - **Implementation**: + - Change issue create resolution to retain metadata, validate all requested labels, and only then invoke `CreateIssue`. Treat an explicitly supplied empty replacement as a deliberate clear operation rather than as an omitted flag. + - Update replace/add/remove processing to resolve metadata, combine current issue labels (reusing their fetched parent references) for additive operations, validate the final set when additions/replace can introduce conflicts, and preserve canonical, deduplicated, sorted IDs. + - Define CLI flag-presence behavior for `--labels ""` and related replace/add/remove exclusivity checks using whether the flag changed, not only whether the parsed slice is non-empty. + - Fix nil versus non-nil label slice handling in service/client update-field checks and GraphQL input construction so removing the last label sends `labelIds: []`; use one shared field-presence helper or audit all service/client checks and serialization sites. + - If `--team` changes the issue's team in the same update, allow replacement labels from the target team but reject additive/removal label modes before mutation to prevent mixing source-team existing IDs with target-team IDs. + - Keep remove-only operations from producing false conflict failures and ensure invalid selections result in zero create/update mutation calls. + - Remove the CLI's generic create/update wrappers. Add exactly one issue-operation context for local validation in `IssueService`, retain exactly one remote mutation context in `pkg/linear/issues.Client`, and propagate remote errors through the service unchanged. + - **Verification**: + - Extend service/CLI tests to assert zero mutation calls for invalid create, replace, and add selections; cover valid cross-group/standalone selections, three-way and UUID conflicts, additive updates using existing labels, remove-only updates, remove-all payloads, explicit empty `--labels`, and simultaneous team-change/label-mode behavior. + - Assert operation context occurs exactly once for both local validation and remote mutation failures, validation errors do not include raw query text, duplicate inputs are deduplicated in the mutation payload, and all label-field presence checks still distinguish nil from empty. Run focused service, CLI, and issue-client tests. + 3. **Grouped label-list text output with JSON compatibility** + - **Implementation**: + - Add a reusable formatter in `internal/format` for grouped labels, reconstructing parent groups from `Parent.ID` regardless of response order. + - Render exclusive group headers and indented child alternatives deterministically, retain parent/group-label and standalone records without making JSON changes, and preserve command-specific useful fields (colors, descriptions, IDs). + - Replace duplicated text formatting in `TeamService.GetLabels`/`GetLabelsWithOutput` and `LabelService.List`; keep empty and JSON behavior intentional and compatible. + - **Verification**: + - Add formatter/service tests for group ordering, child ordering, three or more siblings, parent records arriving after children, missing parent records (fallback header), standalone labels, descriptions/IDs, and both commands. + - Assert JSON output still contains the raw parent metadata fields and remains valid machine-readable JSON. Run focused formatter/service tests. + 4. **GraphQL error sanitization and regression coverage** + - **Implementation**: + - Change `BaseClient.ExecuteRequest` GraphQL decoding to add `extensions` to the response error shape, retain it in `core.GraphQLError`, and return the error without embedding any query/mutation preview. + - Ensure the explicit issue-client/service/CLI boundary yields one operation context for local and remote create/update failures while unrelated GraphQL operations retain clear typed errors. + - Preserve existing error classification behavior and avoid changing unrelated HTTP/retry handling. + - **Verification**: + - Add mock-response tests for GraphQL messages with extensions, assert no query fragment leaks, and verify typed/code details remain available. + - Test create/update wrapping for exactly one operation prefix and representative unrelated API errors for non-regressed diagnostics. Run focused core/client tests. + 5. **Full regression and release-quality verification** + - **Implementation**: + - Review all changed interfaces/mocks and formatting paths for compatibility, update comments/help text only where behavior changed, and add any missing edge-case tests discovered by prior phases. + - Keep implementation within the existing package architecture and avoid live mutation checks. + - **Verification**: + - Run `make build` and `make test` (equivalent to `go test -v ./...`). If the Go toolchain is unavailable, report that limitation rather than substituting an unverified claim. + - Optionally run read-only label listing commands against a configured team to inspect grouped output, but do not run issue create/update mutations. + +**Phase Verification Approach**: Each phase must pass its focused automated tests before the next phase begins. The final phase requires the repository build and full unit suite; no manual GUI work is applicable. Live API mutation verification is explicitly out of scope. + +## Quality Assurance Plan +- **Testing Strategy**: Pure unit tests for resolution/cache/validation/formatting, service tests with mutation-call spies, GraphQL mock transport tests for serialization/error boundaries, then full Go build and unit suite. +- **Edge Cases**: + - Two, three, or more siblings in one group; conflicts in multiple groups; duplicate IDs and case variants. + - UUID inputs, names with punctuation, unknown or ambiguous labels, cross-team UUIDs, parent/group labels, standalone labels, and labels from different groups. + - Existing conflicting labels during additive updates; replace versus add/remove semantics; explicit empty replacement flags; simultaneous team changes; remove-only and remove-all updates. + - API responses whose parent records appear after children or are absent; complete/paginated label retrieval; deep-copy/cache expiry; JSON parent references; GraphQL errors with and without extensions. + - Exact operation-context count, deduplicated/sorted mutation IDs, and absence of raw query/mutation text. +- **Regression Prevention**: Preserve public ID-only resolver methods and raw label JSON schema, retain typed GraphQL errors and error classification, keep valid issue mutations unchanged, and update every interface mock in the repository. +- **Success Verification**: All TL-562 acceptance criteria are represented by focused tests, `make build` succeeds, and `make test` passes without live mutation calls. + +## Development Environment +- **Setup Requirements**: Go toolchain and repository dependencies from `go.mod`; no new dependency expected. Use existing mock transport/server helpers and current branch `tl-562`. +- **Debugging Strategy**: Use typed errors, focused mock response/request assertions, and deterministic output snapshots/string assertions. Never add query text to user-facing errors. +- **Iteration Approach**: Run focused package tests after each phase, then `make build`/`make test`; inspect `git diff` for interface and JSON compatibility changes. + +## Risk Assessment +- **Potential Issues**: + - Adding a metadata resolver method can break multiple mocks and consumers. + - Existing callers may rely on exact duplicated error strings or query previews. + - Issue label data may lack parent metadata in some paths, especially fixtures or older responses. + - The label endpoint may paginate despite the current query exposing only `nodes`. + - Reordering text output may affect snapshot/string tests and scripts that incorrectly parse human output. + - Non-nil empty label slices may interact with GraphQL input omission logic in more than one layer. + - Simultaneous team and label updates can mix source-team and target-team IDs if not rejected or modeled explicitly. +- **Mitigation Strategies**: + - Keep existing resolver signatures as adapters and update all compile-time mocks in one phase. + - Preserve typed underlying errors and test only the intended single context; remove query previews deliberately and cover the global behavior with regression tests. + - Hydrate missing metadata through the complete/paginated team-label resolver only when needed; if authoritative metadata cannot be obtained, return a clear validation error; use parent ID as the sole conflict key. + - Leave JSON unchanged and make text ordering explicit and deterministic; use parent name/ID fallback headers when group records are absent. + - Add request-payload tests specifically for `labelIds: []`, use one shared field-presence helper or audit every check, and test CLI flag-changed semantics for empty replacement. + - Reject additive/removal label modes combined with a team change unless the implementation can prove all final IDs belong to the target team. +- **Backup Approaches**: + - If changing the service interface proves too invasive, introduce a narrow optional metadata resolver capability while retaining the existing ID-only interface and fail validation only when metadata cannot be obtained. + - If shared formatter integration causes command-specific regressions, keep one shared grouping primitive with thin team/label renderers rather than duplicating grouping logic or forcing one service to own the other's command behavior. + - If the three nil/empty checks drift during implementation, centralize label-field presence in a small helper used by service gating, client gating, and GraphQL input construction. + +## Files Likely to Change +- `pkg/linear/resolver.go` — metadata-aware, UUID-capable label resolution and compatibility adapter. +- `pkg/linear/resolver_cache.go` — complete label metadata cache and normalized name/ID indexes. +- `pkg/linear/client.go` — public/internal metadata resolver delegate. +- `pkg/linear/core/errors.go` — any typed conflict/error helpers or GraphQL extension handling needed. +- `pkg/linear/core/base_client.go` — remove query previews and preserve GraphQL extensions. +- `pkg/linear/issues/client.go` — update label field presence/empty-array serialization and remote operation error boundary. +- `pkg/linear/teams/client.go` — paginate label retrieval if required by the API contract while preserving parent fields. +- `internal/service/client_interfaces.go` — metadata resolver capability. +- `internal/service/issue.go` — create/update conflict validation, final label-set construction, and wrapper cleanup. +- `internal/cli/issues.go` — remove duplicate create/update error wrapping if still present after service boundary changes. +- `internal/service/team.go` — use grouped text label formatter. +- `internal/service/label.go` — use grouped text label formatter. +- `internal/format/labels.go` — new shared grouped text renderer. +- `pkg/linear/resolver_test.go` — resolution/cache behavior. +- `internal/service/issue_create_test.go` and `internal/service/issue_relation_test.go` (or a focused new update test) — create/update conflict and mutation suppression. +- `internal/cli/issues.go` and its CLI tests/helpers — empty replacement flag presence, label-mode exclusivity, and wrapper behavior. +- `pkg/linear/issues/client_test.go` — update payload and operation-error behavior. +- `pkg/linear/core/base_client_test.go` — GraphQL sanitization/extensions regression tests. +- `internal/format/labels_test.go` — grouped rendering tests. +- `internal/service/team_test.go` and/or `internal/service/label_test.go` — command integration and JSON compatibility tests. +- Other repository service mocks implementing `IssueClientOperations` — compile/test updates for the metadata resolver method. From ef130ad1f13fdc45ade33bcf83cbc3c953474f55 Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:08:05 +0200 Subject: [PATCH 2/3] Fix label update verification regressions Allow label additions and removals when the requested team is unchanged, reject incomplete metadata safely, paginate label retrieval, and refresh resolver cache snapshots. Add update, pagination, cache, and CLI flag regression coverage while preserving valid empty-label and JSON behavior. Co-Authored-By: Claude --- internal/cli/issues.go | 16 ++- internal/cli/issues_update_test.go | 18 +++ internal/format/labels.go | 1 + internal/service/issue.go | 35 +++-- internal/service/issue_update_test.go | 158 ++++++++++++++++++++++ internal/service/label.go | 8 +- internal/service/label_validation.go | 8 +- internal/service/label_validation_test.go | 12 ++ internal/service/team.go | 8 +- pkg/linear/issues/client.go | 4 +- pkg/linear/resolver_cache.go | 29 ++-- pkg/linear/resolver_cache_test.go | 14 ++ pkg/linear/teams/client.go | 107 +++++++++------ pkg/linear/teams/client_test.go | 39 ++++++ 14 files changed, 370 insertions(+), 87 deletions(-) create mode 100644 internal/cli/issues_update_test.go create mode 100644 internal/service/issue_update_test.go diff --git a/internal/cli/issues.go b/internal/cli/issues.go index c3e7cbc..643c9af 100644 --- a/internal/cli/issues.go +++ b/internal/cli/issues.go @@ -542,9 +542,10 @@ LABEL MODES: // Check if any updates provided (description="-" means stdin) labelsChanged := cmd.Flags().Changed("labels") + addLabelsChanged := cmd.Flags().Changed("add-labels") + removeLabelsChanged := cmd.Flags().Changed("remove-labels") hasFlags := title != "" || description != "" || state != "" || - priority != "" || estimate != "" || labelsChanged || - addLabels != "" || removeLabels != "" || + priority != "" || estimate != "" || labelsChanged || addLabelsChanged || removeLabelsChanged || cycle != "" || project != "" || assignee != "" || dueDate != "" || parent != "" || dependsOn != "" || blockedBy != "" || len(attachFiles) > 0 @@ -554,8 +555,8 @@ LABEL MODES: } // Validate mutual exclusivity: --labels cannot be used with --add-labels or --remove-labels - if labelsChanged && (addLabels != "" || removeLabels != "") { - return fmt.Errorf("--labels cannot be combined with --add-labels or --remove-labels. Use --labels to replace all labels, or --add-labels/--remove-labels for incremental changes") + if err := validateIssueLabelModeFlags(labelsChanged, addLabelsChanged, removeLabelsChanged); err != nil { + return err } // Get description from flag or stdin @@ -670,6 +671,13 @@ LABEL MODES: return cmd } +func validateIssueLabelModeFlags(labelsChanged, addLabelsChanged, removeLabelsChanged bool) error { + if labelsChanged && (addLabelsChanged || removeLabelsChanged) { + return fmt.Errorf("--labels cannot be combined with --add-labels or --remove-labels. Use --labels to replace all labels, or --add-labels/--remove-labels for incremental changes") + } + return nil +} + func newIssuesCommentCmd() *cobra.Command { var ( body string diff --git a/internal/cli/issues_update_test.go b/internal/cli/issues_update_test.go new file mode 100644 index 0000000..e814243 --- /dev/null +++ b/internal/cli/issues_update_test.go @@ -0,0 +1,18 @@ +package cli + +import "testing" + +func TestValidateIssueLabelModeFlagsUsesFlagPresence(t *testing.T) { + if err := validateIssueLabelModeFlags(true, true, false); err == nil { + t.Fatal("expected empty --add-labels presence to conflict with --labels") + } + if err := validateIssueLabelModeFlags(true, false, true); err == nil { + t.Fatal("expected empty --remove-labels presence to conflict with --labels") + } + if err := validateIssueLabelModeFlags(false, true, true); err != nil { + t.Fatalf("add and remove modes should be compatible: %v", err) + } + if err := validateIssueLabelModeFlags(true, false, false); err != nil { + t.Fatalf("replace-only mode returned %v", err) + } +} diff --git a/internal/format/labels.go b/internal/format/labels.go index 074bd38..8af67e2 100644 --- a/internal/format/labels.go +++ b/internal/format/labels.go @@ -120,6 +120,7 @@ func (f *Formatter) LabelList(labels []core.Label, includeIDs bool) string { } // Labels renders the compact label-list text format. +// Deprecated compatibility entry point; new callers should use LabelList. func (f *Formatter) Labels(labels []core.Label) string { return FormatLabels(labels, LabelListOptions{}) } diff --git a/internal/service/issue.go b/internal/service/issue.go index 73b285a..dbbdf2f 100644 --- a/internal/service/issue.go +++ b/internal/service/issue.go @@ -715,7 +715,17 @@ func (s *IssueService) Update(identifier string, input *UpdateIssueInput) (strin } if input.TeamID != nil && *input.TeamID != "" && (len(input.AddLabelIDs) > 0 || len(input.RemoveLabelIDs) > 0) { - return "", fmt.Errorf("cannot add or remove labels while changing an issue's team; use --labels to replace all labels") + teamKey, _, parseErr := identifiers.ParseIssueIdentifier(issue.Identifier) + if parseErr != nil { + return "", fmt.Errorf("failed to update issue: cannot determine current team: %w", parseErr) + } + currentTeamID, resolveErr := s.client.ResolveTeamIdentifier(teamKey) + if resolveErr != nil { + return "", fmt.Errorf("failed to update issue: cannot determine current team: %w", resolveErr) + } + if currentTeamID != teamIDForLabels { + return "", fmt.Errorf("failed to update issue: cannot add or remove labels while changing an issue's team; use --labels to replace all labels") + } } if input.LabelIDs != nil { @@ -740,14 +750,20 @@ func (s *IssueService) Update(identifier string, input *UpdateIssueInput) (strin for _, labelName := range input.AddLabelIDs { label, err := s.client.ResolveLabelMetadata(labelName, teamIDForLabels) if err != nil { - return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': %w", labelName, err) + } + if label == nil { + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': resolver returned no label", labelName) } labelSet[label.ID] = *label } for _, labelName := range input.RemoveLabelIDs { label, err := s.client.ResolveLabelMetadata(labelName, teamIDForLabels) if err != nil { - return "", fmt.Errorf("failed to resolve label '%s': %w", labelName, err) + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': %w", labelName, err) + } + if label == nil { + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': resolver returned no label", labelName) } delete(labelSet, label.ID) } @@ -915,15 +931,10 @@ func (s *IssueService) currentLabelMetadata(issue *core.Issue, teamID string, hy } labels := make([]core.Label, 0, len(issue.Labels.Nodes)) for _, label := range issue.Labels.Nodes { - if label.Parent != nil && label.Parent.ID != "" || !hydrate { - labels = append(labels, label) - continue - } - resolved, err := s.client.ResolveLabelMetadata(label.ID, teamID) - if err != nil { - return nil, fmt.Errorf("could not resolve existing label '%s': %w", label.Name, err) - } - labels = append(labels, *resolved) + // The issue query requests parent metadata. A nil parent is authoritative + // for standalone labels, while a non-nil parent with an empty ID is + // rejected by validateLabelSelection as incomplete metadata. + labels = append(labels, label) } return sortLabels(labels), nil } diff --git a/internal/service/issue_update_test.go b/internal/service/issue_update_test.go new file mode 100644 index 0000000..3cad8ef --- /dev/null +++ b/internal/service/issue_update_test.go @@ -0,0 +1,158 @@ +package service + +import ( + "strings" + "testing" + + "github.com/joa23/linear-cli/pkg/linear/core" +) + +type mockIssueClientForUpdate struct { + *mockIssueClientForCreate + issue *core.Issue + teamIDs map[string]string + labels map[string]*core.Label + updateCalls int + updateInput core.UpdateIssueInput +} + +func (m *mockIssueClientForUpdate) GetIssue(string) (*core.Issue, error) { + return m.issue, nil +} + +func (m *mockIssueClientForUpdate) ResolveTeamIdentifier(key string) (string, error) { + return m.teamIDs[key], nil +} + +func (m *mockIssueClientForUpdate) ResolveLabelMetadata(name, _ string) (*core.Label, error) { + return m.labels[name], nil +} + +func (m *mockIssueClientForUpdate) UpdateIssue(_ string, input core.UpdateIssueInput) (*core.Issue, error) { + m.updateCalls++ + m.updateInput = input + return m.issue, nil +} + +func makeIssueServiceForUpdate(mock *mockIssueClientForUpdate) *IssueService { + return NewIssueService(mock, makeIssueServiceForCreate(mock.mockIssueClientForCreate).formatter) +} + +func TestIssueService_Update_AddWithUnchangedTeamSucceeds(t *testing.T) { + mock := newUpdateMock() + mock.labels["Bug"] = &core.Label{ID: "label-bug", Name: "Bug"} + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + TeamID: stringPtr("TL"), + AddLabelIDs: []string{"Bug"}, + }) + if err != nil { + t.Fatalf("Update() returned unexpected error: %v", err) + } + if mock.updateCalls != 1 { + t.Fatalf("UpdateIssue calls = %d, want 1", mock.updateCalls) + } + if len(mock.updateInput.LabelIDs) != 1 || mock.updateInput.LabelIDs[0] != "label-bug" { + t.Fatalf("label IDs = %#v, want [label-bug]", mock.updateInput.LabelIDs) + } +} + +func TestIssueService_Update_AddWithTeamChangeFailsBeforeMutation(t *testing.T) { + mock := newUpdateMock() + mock.teamIDs["NEW"] = "team-2" + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + TeamID: stringPtr("NEW"), + AddLabelIDs: []string{"Bug"}, + }) + if err == nil || !strings.Contains(err.Error(), "cannot add or remove labels while changing") { + t.Fatalf("error = %v, want team-change validation", err) + } + if mock.updateCalls != 0 { + t.Fatal("UpdateIssue called despite team-change validation") + } +} + +func TestIssueService_Update_ReplaceWithTeamChangeSucceeds(t *testing.T) { + mock := newUpdateMock() + mock.teamIDs["NEW"] = "team-2" + mock.labels["Bug"] = &core.Label{ID: "label-bug", Name: "Bug"} + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + TeamID: stringPtr("NEW"), + LabelIDs: []string{"Bug"}, + }) + if err != nil { + t.Fatalf("Update() returned unexpected error: %v", err) + } + if mock.updateCalls != 1 { + t.Fatalf("UpdateIssue calls = %d, want 1", mock.updateCalls) + } +} + +func TestIssueService_Update_RemoveOnlySendsExplicitEmptyLabels(t *testing.T) { + mock := newUpdateMock() + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + RemoveLabelIDs: []string{"Bug"}, + }) + if err != nil { + t.Fatalf("Update() returned unexpected error: %v", err) + } + if mock.updateCalls != 1 { + t.Fatalf("UpdateIssue calls = %d, want 1", mock.updateCalls) + } + if mock.updateInput.LabelIDs == nil || len(mock.updateInput.LabelIDs) != 0 { + t.Fatalf("label IDs = %#v, want non-nil empty slice", mock.updateInput.LabelIDs) + } +} + +func TestIssueService_Update_AddIncludesExistingConflict(t *testing.T) { + mock := newUpdateMock() + mock.issue.Labels = &core.LabelConnection{Nodes: []core.Label{ + {ID: "existing-a", Name: "Alpha", Parent: &core.LabelRef{ID: "group", Name: "Type"}}, + }} + mock.labels["Beta"] = &core.Label{ID: "existing-b", Name: "Beta", Parent: &core.LabelRef{ID: "group", Name: "Type"}} + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + AddLabelIDs: []string{"Beta"}, + }) + if err == nil || !strings.Contains(err.Error(), "exclusive group") { + t.Fatalf("error = %v, want existing-label conflict", err) + } + if mock.updateCalls != 0 { + t.Fatal("UpdateIssue called despite additive conflict") + } +} + +func TestIssueService_Update_NilResolverResultIsSafe(t *testing.T) { + mock := newUpdateMock() + + _, err := makeIssueServiceForUpdate(mock).Update("TL-1", &UpdateIssueInput{ + AddLabelIDs: []string{"Missing"}, + }) + if err == nil || !strings.Contains(err.Error(), "resolver returned no label") { + t.Fatalf("error = %v, want nil resolver validation", err) + } + if mock.updateCalls != 0 { + t.Fatal("UpdateIssue called despite resolver failure") + } +} + +func newUpdateMock() *mockIssueClientForUpdate { + base := &mockIssueClientForCreate{} + return &mockIssueClientForUpdate{ + mockIssueClientForCreate: base, + issue: &core.Issue{ + ID: "issue-1", + Identifier: "TL-1", + Title: "Issue", + }, + teamIDs: map[string]string{"TL": "team-1"}, + labels: map[string]*core.Label{"Bug": {ID: "label-bug", Name: "Bug"}}, + } +} + +func stringPtr(value string) *string { + return &value +} diff --git a/internal/service/label.go b/internal/service/label.go index 08e4e46..be1dd53 100644 --- a/internal/service/label.go +++ b/internal/service/label.go @@ -36,10 +36,6 @@ func (s *LabelService) List(teamID string, verbosity format.Verbosity, outputTyp return "", fmt.Errorf("failed to list labels: %w", err) } - if len(labels) == 0 { - return "No labels found.", nil - } - // JSON output if outputType.IsJSON() { data, err := json.MarshalIndent(labels, "", " ") @@ -49,6 +45,10 @@ func (s *LabelService) List(teamID string, verbosity format.Verbosity, outputTyp return string(data), nil } + if len(labels) == 0 { + return "No labels found.", nil + } + return s.formatter.LabelList(labels, true), nil } diff --git a/internal/service/label_validation.go b/internal/service/label_validation.go index 6bd529d..732d4a5 100644 --- a/internal/service/label_validation.go +++ b/internal/service/label_validation.go @@ -41,13 +41,19 @@ func validateLabelSelection(labels []core.Label) error { groups := make(map[string]LabelConflictGroup) seen := make(map[string]struct{}, len(labels)) for _, label := range labels { + if label.ID == "" { + return fmt.Errorf("label metadata is incomplete for %q: label ID is missing", label.Name) + } if _, ok := seen[label.ID]; ok { continue } seen[label.ID] = struct{}{} - if label.Parent == nil || label.Parent.ID == "" { + if label.Parent == nil { continue } + if label.Parent.ID == "" { + return fmt.Errorf("label metadata is incomplete for %q: parent ID is missing", label.Name) + } group := groups[label.Parent.ID] group.Parent = *label.Parent group.Labels = append(group.Labels, label) diff --git a/internal/service/label_validation_test.go b/internal/service/label_validation_test.go index 82a2b27..e41ed98 100644 --- a/internal/service/label_validation_test.go +++ b/internal/service/label_validation_test.go @@ -30,6 +30,18 @@ func TestValidateLabelSelectionReportsDeterministicSiblingConflicts(t *testing.T } } +func TestValidateLabelSelectionRejectsIncompleteMetadata(t *testing.T) { + tests := []core.Label{ + {ID: "", Name: "Missing ID"}, + {ID: "child", Name: "Missing parent ID", Parent: &core.LabelRef{Name: "Type"}}, + } + for _, label := range tests { + if err := validateLabelSelection([]core.Label{label}); err == nil || !strings.Contains(err.Error(), "metadata is incomplete") { + t.Fatalf("label %#v error = %v, want incomplete metadata error", label, err) + } + } +} + func TestValidateLabelSelectionAllowsStandaloneAndDistinctGroups(t *testing.T) { labels := []core.Label{ {ID: "a", Name: "Alpha", Parent: &core.LabelRef{ID: "group-1", Name: "One"}}, diff --git a/internal/service/team.go b/internal/service/team.go index a8b6f25..8104c1c 100644 --- a/internal/service/team.go +++ b/internal/service/team.go @@ -107,10 +107,6 @@ func (s *TeamService) GetLabelsWithOutput(identifier string, verbosity format.Ve return "", fmt.Errorf("failed to list labels: %w", err) } - if len(labels) == 0 { - return "No labels found.", nil - } - if outputType.IsJSON() { data, err := json.MarshalIndent(labels, "", " ") if err != nil { @@ -119,6 +115,10 @@ func (s *TeamService) GetLabelsWithOutput(identifier string, verbosity format.Ve return string(data), nil } + if len(labels) == 0 { + return "No labels found.", nil + } + return s.formatter.LabelList(labels, false), nil } diff --git a/pkg/linear/issues/client.go b/pkg/linear/issues/client.go index 7672d09..fc21544 100644 --- a/pkg/linear/issues/client.go +++ b/pkg/linear/issues/client.go @@ -154,7 +154,7 @@ linear_create_issue("Task title", "Description", teams[0].id)`) } if !response.IssueCreate.Success { - return nil, fmt.Errorf("issue creation was not successful") + return nil, fmt.Errorf("failed to create issue: mutation was not successful") } // Extract metadata from description if present @@ -1759,7 +1759,7 @@ func (ic *Client) UpdateIssue(issueID string, input core.UpdateIssueInput) (*cor } if !response.IssueUpdate.Success { - return nil, fmt.Errorf("issue update was not successful") + return nil, fmt.Errorf("failed to update issue: mutation was not successful") } // Extract metadata from description if present diff --git a/pkg/linear/resolver_cache.go b/pkg/linear/resolver_cache.go index 01ece23..cdfefd7 100644 --- a/pkg/linear/resolver_cache.go +++ b/pkg/linear/resolver_cache.go @@ -43,7 +43,6 @@ type resolverCache struct { // Label resolution caches are keyed by normalized team and label values. labelByName map[string]*cacheEntry // teamID:normalized-name → labelID - labelByID map[string]*cacheEntry // teamID:normalized-id → labelID labelData map[string]*labelCacheEntry // teamID:normalized-id → label metadata // Project resolution cache @@ -62,7 +61,6 @@ func newResolverCache(ttl time.Duration) *resolverCache { teamByKey: make(map[string]*cacheEntry), issueByIdentifier: make(map[string]*cacheEntry), labelByName: make(map[string]*cacheEntry), - labelByID: make(map[string]*cacheEntry), labelData: make(map[string]*labelCacheEntry), projectByName: make(map[string]*cacheEntry), ttl: ttl, @@ -222,19 +220,21 @@ func (rc *resolverCache) getLabelByID(teamID, labelID string) (core.Label, bool) return cloneLabel(entry.label), true } -func (rc *resolverCache) setLabelByName(teamID, labelName, labelID string) { +func (rc *resolverCache) setLabels(teamID string, labels []core.Label) { rc.mu.Lock() defer rc.mu.Unlock() - rc.labelByName[labelKey(teamID, labelName)] = &cacheEntry{ - value: labelID, - expiresAt: time.Now().Add(rc.ttl), + teamPrefix := strings.ToLower(strings.TrimSpace(teamID)) + ":" + for key := range rc.labelByName { + if strings.HasPrefix(key, teamPrefix) { + delete(rc.labelByName, key) + } + } + for key := range rc.labelData { + if strings.HasPrefix(key, teamPrefix) { + delete(rc.labelData, key) + } } -} - -func (rc *resolverCache) setLabels(teamID string, labels []core.Label) { - rc.mu.Lock() - defer rc.mu.Unlock() expiresAt := time.Now().Add(rc.ttl) nameCounts := make(map[string]int, len(labels)) @@ -244,7 +244,6 @@ func (rc *resolverCache) setLabels(teamID string, labels []core.Label) { for _, label := range labels { copy := cloneLabel(label) idKey := labelKey(teamID, label.ID) - rc.labelByID[idKey] = &cacheEntry{value: label.ID, expiresAt: expiresAt} rc.labelData[idKey] = &labelCacheEntry{label: copy, expiresAt: expiresAt} if nameCounts[labelKey(teamID, label.Name)] == 1 { rc.labelByName[labelKey(teamID, label.Name)] = &cacheEntry{value: label.ID, expiresAt: expiresAt} @@ -323,11 +322,6 @@ func (rc *resolverCache) cleanup() { delete(rc.labelByName, key) } } - for key, entry := range rc.labelByID { - if entry.expiresAt.Before(now) { - delete(rc.labelByID, key) - } - } for key, entry := range rc.labelData { if entry.expiresAt.Before(now) { delete(rc.labelData, key) @@ -366,7 +360,6 @@ func (rc *resolverCache) clear() { rc.teamByKey = make(map[string]*cacheEntry) rc.issueByIdentifier = make(map[string]*cacheEntry) rc.labelByName = make(map[string]*cacheEntry) - rc.labelByID = make(map[string]*cacheEntry) rc.labelData = make(map[string]*labelCacheEntry) rc.projectByName = make(map[string]*cacheEntry) } diff --git a/pkg/linear/resolver_cache_test.go b/pkg/linear/resolver_cache_test.go index e16bfa8..867ee67 100644 --- a/pkg/linear/resolver_cache_test.go +++ b/pkg/linear/resolver_cache_test.go @@ -29,6 +29,20 @@ func TestResolverCacheStoresNormalizedLabelMetadata(t *testing.T) { } } +func TestResolverCacheRefreshRemovesStaleLabelNames(t *testing.T) { + cache := newResolverCache(time.Hour) + defer cache.clear() + + cache.setLabels("team", []core.Label{{ID: "one", Name: "Only"}}) + cache.setLabels("team", []core.Label{{ID: "two", Name: "Only"}, {ID: "three", Name: "only"}}) + if _, ok := cache.getLabelByName("team", "Only"); ok { + t.Fatal("stale unique name should not survive an ambiguous refresh") + } + if _, ok := cache.getLabelByID("team", "one"); ok { + t.Fatal("removed label metadata should not survive a refresh") + } +} + func TestResolverCacheDoesNotCacheAmbiguousLabelNames(t *testing.T) { cache := newResolverCache(time.Hour) defer cache.clear() diff --git a/pkg/linear/teams/client.go b/pkg/linear/teams/client.go index 4c09403..e0612d6 100644 --- a/pkg/linear/teams/client.go +++ b/pkg/linear/teams/client.go @@ -38,18 +38,18 @@ func (tc *Client) GetTeams() ([]core.Team, error) { } } ` - + var response struct { Teams struct { Nodes []core.Team `json:"nodes"` } `json:"teams"` } - + err := tc.base.ExecuteRequest(query, nil, &response) if err != nil { return nil, fmt.Errorf("failed to get teams: %w", err) } - + return response.Teams.Nodes, nil } @@ -109,16 +109,16 @@ func (tc *Client) GetViewer() (*core.User, error) { } } ` - + var response struct { Viewer core.User `json:"viewer"` } - + err := tc.base.ExecuteRequest(query, nil, &response) if err != nil { return nil, fmt.Errorf("failed to get viewer: %w", err) } - + return &response.Viewer, nil } @@ -428,24 +428,24 @@ func (tc *Client) GetUser(userID string) (*core.User, error) { } } ` - + variables := map[string]interface{}{ "userId": userID, } - + var response struct { User *core.User `json:"user"` } - + err := tc.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to get user: %w", err) } - + if response.User == nil { return nil, fmt.Errorf("user not found") } - + return response.User, nil } @@ -478,30 +478,30 @@ func (tc *Client) GetUserByEmail(email string) (*core.User, error) { } } ` - + variables := map[string]interface{}{ "email": email, } - + var response struct { Users struct { Nodes []core.User `json:"nodes"` } `json:"users"` } - + err := tc.base.ExecuteRequest(query, variables, &response) if err != nil { return nil, fmt.Errorf("failed to get user by email: %w", err) } - + if len(response.Users.Nodes) == 0 { return nil, fmt.Errorf("user not found with email: %s", email) } - + if len(response.Users.Nodes) > 1 { return nil, fmt.Errorf("multiple users found with email: %s", email) } - + return &response.Users.Nodes[0], nil } @@ -565,9 +565,9 @@ func (tc *Client) ListUsersWithDisplayNameFilter(displayName string, activeOnly // discover available labels for issue tagging and organization. func (tc *Client) ListLabels(teamID string) ([]core.Label, error) { const query = ` - query GetTeamLabels($teamId: String!) { + query GetTeamLabels($teamId: String!, $first: Int!, $after: String) { team(id: $teamId) { - labels { + labels(first: $first, after: $after) { nodes { id name @@ -578,33 +578,56 @@ func (tc *Client) ListLabels(teamID string) ([]core.Label, error) { name } } + pageInfo { + hasNextPage + endCursor + } } } } ` - - variables := map[string]interface{}{ - "teamId": teamID, - } - - var response struct { - Team *struct { - Labels struct { - Nodes []core.Label `json:"nodes"` - } `json:"labels"` - } `json:"team"` - } - - err := tc.base.ExecuteRequest(query, variables, &response) - if err != nil { - return nil, fmt.Errorf("failed to list labels: %w", err) - } - - if response.Team == nil { - return nil, fmt.Errorf("team not found") + + const first = 50 + var labels []core.Label + var after string + for { + variables := map[string]interface{}{ + "teamId": teamID, + "first": first, + } + if after != "" { + variables["after"] = after + } + + var response struct { + Team *struct { + Labels struct { + Nodes []core.Label `json:"nodes"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + EndCursor string `json:"endCursor"` + } `json:"pageInfo"` + } `json:"labels"` + } `json:"team"` + } + + if err := tc.base.ExecuteRequest(query, variables, &response); err != nil { + return nil, fmt.Errorf("failed to list labels: %w", err) + } + if response.Team == nil { + return nil, fmt.Errorf("team not found") + } + + page := response.Team.Labels + labels = append(labels, page.Nodes...) + if !page.PageInfo.HasNextPage { + return labels, nil + } + if page.PageInfo.EndCursor == "" || page.PageInfo.EndCursor == after { + return nil, fmt.Errorf("failed to list labels: pagination returned no advancing cursor") + } + after = page.PageInfo.EndCursor } - - return response.Team.Labels.Nodes, nil } // CreateLabel creates a new label for a team @@ -763,4 +786,4 @@ func (tc *Client) DeleteLabel(labelID string) error { } return nil -} \ No newline at end of file +} diff --git a/pkg/linear/teams/client_test.go b/pkg/linear/teams/client_test.go index c981259..68b97bb 100644 --- a/pkg/linear/teams/client_test.go +++ b/pkg/linear/teams/client_test.go @@ -2,6 +2,10 @@ package teams import ( "encoding/json" + "fmt" + "io" + "net/http" + "strings" "testing" "github.com/joa23/linear-cli/pkg/linear/core" @@ -76,3 +80,38 @@ func TestListLabelsResponseStruct_ParentField(t *testing.T) { t.Errorf("nodes[1].Parent = %+v, want nil", second.Parent) } } + +func TestListLabelsPaginatesAllPages(t *testing.T) { + responses := []string{ + `{"data":{"team":{"labels":{"nodes":[{"id":"one","name":"One"}],"pageInfo":{"hasNextPage":true,"endCursor":"cursor-1"}}}}}`, + `{"data":{"team":{"labels":{"nodes":[{"id":"two","name":"Two"}],"pageInfo":{"hasNextPage":false,"endCursor":"cursor-2"}}}}}`, + } + calls := 0 + base := core.NewBaseClient("token") + base.SetHTTPClient(&http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + if calls >= len(responses) { + return nil, fmt.Errorf("unexpected request") + } + body := responses[calls] + calls++ + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + Header: make(http.Header), + }, nil + })}) + + labels, err := NewClient(base).ListLabels("team-1") + if err != nil { + t.Fatalf("ListLabels() returned error: %v", err) + } + if calls != 2 || len(labels) != 2 || labels[1].ID != "two" { + t.Fatalf("calls = %d, labels = %#v; want two pages and both labels", calls, labels) + } +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} From 0dae179abaf0a23c9022de15b8151d9d114b1db3 Mon Sep 17 00:00:00 2001 From: Felix Lisczyk <5102728+FelixLisczyk@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:20:43 +0200 Subject: [PATCH 3/3] Deleted plan.md --- plan.md | 170 -------------------------------------------------------- 1 file changed, 170 deletions(-) delete mode 100644 plan.md diff --git a/plan.md b/plan.md deleted file mode 100644 index 0e3de68..0000000 --- a/plan.md +++ /dev/null @@ -1,170 +0,0 @@ -# Implementation Plan - -## Task Overview -- **Source**: TL-562 -- **Title**: Prevent exclusive-label conflicts and improve label/error output -- **Description**: Detect mutually exclusive child-label selections locally before issue create/update mutations, report the conflicting labels and parent group clearly, remove duplicated/raw GraphQL error output, and make both default label listings communicate parent-child exclusivity while preserving JSON compatibility. - -## Requirements Analysis -- **Core Functionality**: - - Resolve labels by case-insensitive name or UUID while preserving canonical label metadata (`ID`, `Name`, and optional `Parent`). - - Treat two or more distinct child labels with the same non-nil parent as an exclusive-group conflict. Parent/group labels (`Parent == nil`) and standalone labels do not conflict. - - Validate create selections before `CreateIssue` and validate the final label set for update replace/add operations. Additive updates must include labels already attached to the issue; remove-only updates must not reject a set merely because it reduces or preserves an existing selection. - - Report every conflicting sibling deterministically, including three-way conflicts, multiple groups, duplicate inputs, case variants, UUID inputs, and names containing punctuation. - - Render grouped text output for both `linear teams labels` and `linear labels list`; retain the existing machine-readable label JSON structure and parent metadata. - - Ensure normal CLI errors contain one operation context and never expose raw GraphQL query/mutation previews. -- **Acceptance Criteria**: - - Invalid create/replace/add selections fail locally and make zero relevant mutation calls. - - Conflict errors identify the shared group and all conflicting child labels in stable order with actionable wording. - - Duplicate/case-variant inputs do not create duplicate conflict entries; valid selections from different groups and standalone labels continue to work. - - Add/remove update behavior accounts for the final set, and removing the final label still sends an explicit empty label list. - - UUID and existing name resolution contracts work, including parent/group-label resolution and unknown-label guidance. - - Text listings clearly distinguish group headers and child alternatives; JSON output remains structurally usable for automation. - - GraphQL errors retain useful structured/type information but contain no query text; create/update output has no duplicated operation prefix. - - Automated tests cover resolver/cache behavior, conflict validation, mutation suppression, update modes, grouped output, JSON compatibility, and unrelated API errors. -- **Technical Constraints**: - - Keep public `ResolveLabel`/`ResolveLabelIdentifier` string-returning APIs compatible where possible; add a metadata-returning resolver API for service validation rather than forcing metadata through existing callers. - - The parent relationship returned by Linear is the authoritative exclusivity signal for this ticket. - - Preserve nil-versus-non-nil label slices so `nil` means no label field supplied while a non-nil empty slice means clear all labels. - - Do not run mutating live Linear commands. Verification is repository build/unit testing only; live read-only checks are optional. -- **Integration Points**: - - `internal/service/issue.go` and `internal/cli/issues.go` issue flows. - - `pkg/linear/resolver.go`, `pkg/linear/resolver_cache.go`, and the service-facing client interface. - - `pkg/linear/core/types.go` label metadata and `pkg/linear/core/errors.go` GraphQL errors. - - `pkg/linear/issues/client.go` issue mutation wrappers and update input serialization. - - `internal/service/team.go`, `internal/service/label.go`, and a shared `internal/format` label renderer. - -## Codebase Analysis -- **Existing Patterns**: - - `Resolver.ResolveLabel` currently fetches team labels and returns only an ID; `resolverCache` caches only `teamID:labelName -> labelID` and does not normalize keys or retain parent metadata. - - `core.Label` already contains `Parent *LabelRef`, and `teams.Client.ListLabels` and issue reads already query parent IDs/names. - - `IssueService.Create` resolves labels before its single create mutation. `IssueService.Update` fetches the issue, then builds replace/add/remove label sets, but currently loses ordering and skips an explicit empty final set. - - Team and label services marshal raw labels for JSON but duplicate flat text formatting logic. - - `BaseClient.ExecuteRequest` currently adds a truncated query preview to `GraphQLError`; issue client, service, and CLI each add overlapping create/update context. - - Existing test infrastructure includes service mocks and GraphQL mock transport/server helpers, but lacks conflict, formatter, and GraphQL error regression coverage. -- **Available Infrastructure**: - - `make build` and `make test` invoke Go build and `go test -v ./...`. - - `pkg/linear/resolver_test.go`, `internal/service/issue_create_test.go`, `internal/service/issue_relation_test.go`, `pkg/linear/issues/client_test.go`, and `pkg/linear/teams/client_test.go` provide nearby test patterns. - - `pkg/linear/testutil/mock_transport.go`/`mock_server.go` can assert request counts and payloads without a network round trip. - - Existing `guidance.ErrorWithGuidance`, `core.ValidationError`, and typed `core.GraphQLError` should be reused/extended rather than introducing ad hoc string-only errors. -- **Dependencies**: No new external dependency is expected. Changes to `IssueClientOperations` or resolver return types require updating all service mock implementations and client delegates. -- **Architecture Notes**: - - Add a resolver method such as `ResolveLabelMetadata(labelName, teamID) (*core.Label, error)` and a corresponding internal/public client delegate. Keep the existing ID-only methods delegating to it. Exact UUID matches must be constrained to the requested team's returned label set; case-insensitive name collisions must produce deterministic ambiguity guidance instead of selecting an arbitrary API-order match. Parent/group labels remain resolvable exactly as they are today; the validator only treats labels with non-nil `Parent` as children. - - Populate metadata caches for all labels returned by one team listing, keyed by normalized name and ID, so resolving multiple labels or ID inputs does not discard metadata or cause avoidable repeat requests. Normalize at lookup time, extend cleanup to every new index, and return deep copies (including `Parent`) so callers cannot mutate cached records. Because conflict detection depends on a complete team label set, either add pagination to `ListLabels` or document and test the API's complete-result guarantee before relying on it. - - Add a pure validator/typed conflict error in the service/domain layer. Deduplicate resolved labels by ID for both validation and mutation payloads, group children by `Parent.ID`, sort groups and canonical `Label.Name` values, and quote canonical names/group names safely in user-facing output. - - For create, resolve all labels to metadata, validate, then pass canonical, deduplicated, sorted IDs to the mutation. For update replace, resolve supplied labels and validate that set. For add/remove, reuse parent metadata already returned on the existing issue when available, hydrate missing metadata from the target team's complete label set, construct a deterministic final set from current issue labels plus additions minus removals, and validate when additions can introduce a conflict. If metadata remains unavailable, fail validation clearly rather than silently treating the label as standalone. Preserve explicit empty output for remove-all. If an update changes teams, permit replacement labels resolved in the target team but reject additive/removal label operations combined with a team change before mutation so source-team IDs cannot be submitted to the target team. - - Define the error boundary explicitly: the CLI adds no create/update wrapper; `IssueService` adds one `failed to create/update issue` context for local validation failures; `pkg/linear/issues.Client` retains one context for remote create/update mutation failures; the service must propagate those remote errors without wrapping them again. Resolver and unrelated service errors retain their existing operation-specific guidance. - - Remove query previews from `BaseClient` globally and decode/preserve GraphQL `extensions`; test that typed errors remain useful without implementation internals. - - Add a shared label text formatter that reconstructs groups independent of API order, renders each exclusive parent header once with indented children, preserves descriptions/IDs appropriate to each command, uses `Parent.Name`/ID as a fallback header when a parent record is absent, and sorts groups/children deterministically. Keep JSON paths as direct `core.Label` marshaling. - -## Implementation Strategy -- **Problem Complexity**: Complex, cross-layer bugfix with compatibility-sensitive resolver, update, formatter, and shared error behavior. -- **Core Problem**: Label parent metadata is available at the API boundary but discarded before validation, while multiple layers expose the same low-level mutation error and list labels without their group structure. -- **Approach**: Preserve existing public ID-based APIs, add metadata-aware resolution and caching, centralize pure conflict validation before mutations, make update label-set semantics explicit, centralize grouped text formatting, and establish one user-facing error boundary. Implement and verify in gated phases. -- **Testing Approach**: Automated unit and mock-transport tests after each implementation phase, followed by `make build` and `make test`. Do not perform live mutating verification. -- **Phases**: - 1. **Metadata-aware label resolution and conflict domain logic** - - **Implementation**: - - Extend resolver/cache storage to retain complete `core.Label` records, normalize name/ID lookup keys at lookup and population time, populate name and UUID indexes from a complete/paginated team-label fetch, and accept UUID inputs while retaining case-insensitive name matching and guidance for unknown or ambiguous labels. Extend cache cleanup for every new index and return deep copies of cached metadata. - - Add the metadata resolver delegate and update the minimal service interface plus all mocks/delegates. - - Add a deterministic conflict validator/error that groups distinct child IDs by non-nil parent, reports all siblings/groups in stable order using canonical names, handles duplicates/case variants, and leaves parent labels/standalone labels valid. Define a clear failure when an existing label cannot be hydrated with authoritative parent metadata. - - **Verification**: - - Add resolver/cache tests for name, case, UUID, punctuation, duplicate lookups, cache reuse, unknown labels, ambiguous names, cross-team UUID rejection, parent labels, deep-copy safety, and cleanup of all label indexes. - - Add pure validator tests for two-way/three-way conflicts, multiple groups, duplicate inputs, different groups, standalone labels, unavailable metadata, and deterministic actionable formatting. Run the focused Go tests. - 2. **Create and update validation before mutation** - - **Implementation**: - - Change issue create resolution to retain metadata, validate all requested labels, and only then invoke `CreateIssue`. Treat an explicitly supplied empty replacement as a deliberate clear operation rather than as an omitted flag. - - Update replace/add/remove processing to resolve metadata, combine current issue labels (reusing their fetched parent references) for additive operations, validate the final set when additions/replace can introduce conflicts, and preserve canonical, deduplicated, sorted IDs. - - Define CLI flag-presence behavior for `--labels ""` and related replace/add/remove exclusivity checks using whether the flag changed, not only whether the parsed slice is non-empty. - - Fix nil versus non-nil label slice handling in service/client update-field checks and GraphQL input construction so removing the last label sends `labelIds: []`; use one shared field-presence helper or audit all service/client checks and serialization sites. - - If `--team` changes the issue's team in the same update, allow replacement labels from the target team but reject additive/removal label modes before mutation to prevent mixing source-team existing IDs with target-team IDs. - - Keep remove-only operations from producing false conflict failures and ensure invalid selections result in zero create/update mutation calls. - - Remove the CLI's generic create/update wrappers. Add exactly one issue-operation context for local validation in `IssueService`, retain exactly one remote mutation context in `pkg/linear/issues.Client`, and propagate remote errors through the service unchanged. - - **Verification**: - - Extend service/CLI tests to assert zero mutation calls for invalid create, replace, and add selections; cover valid cross-group/standalone selections, three-way and UUID conflicts, additive updates using existing labels, remove-only updates, remove-all payloads, explicit empty `--labels`, and simultaneous team-change/label-mode behavior. - - Assert operation context occurs exactly once for both local validation and remote mutation failures, validation errors do not include raw query text, duplicate inputs are deduplicated in the mutation payload, and all label-field presence checks still distinguish nil from empty. Run focused service, CLI, and issue-client tests. - 3. **Grouped label-list text output with JSON compatibility** - - **Implementation**: - - Add a reusable formatter in `internal/format` for grouped labels, reconstructing parent groups from `Parent.ID` regardless of response order. - - Render exclusive group headers and indented child alternatives deterministically, retain parent/group-label and standalone records without making JSON changes, and preserve command-specific useful fields (colors, descriptions, IDs). - - Replace duplicated text formatting in `TeamService.GetLabels`/`GetLabelsWithOutput` and `LabelService.List`; keep empty and JSON behavior intentional and compatible. - - **Verification**: - - Add formatter/service tests for group ordering, child ordering, three or more siblings, parent records arriving after children, missing parent records (fallback header), standalone labels, descriptions/IDs, and both commands. - - Assert JSON output still contains the raw parent metadata fields and remains valid machine-readable JSON. Run focused formatter/service tests. - 4. **GraphQL error sanitization and regression coverage** - - **Implementation**: - - Change `BaseClient.ExecuteRequest` GraphQL decoding to add `extensions` to the response error shape, retain it in `core.GraphQLError`, and return the error without embedding any query/mutation preview. - - Ensure the explicit issue-client/service/CLI boundary yields one operation context for local and remote create/update failures while unrelated GraphQL operations retain clear typed errors. - - Preserve existing error classification behavior and avoid changing unrelated HTTP/retry handling. - - **Verification**: - - Add mock-response tests for GraphQL messages with extensions, assert no query fragment leaks, and verify typed/code details remain available. - - Test create/update wrapping for exactly one operation prefix and representative unrelated API errors for non-regressed diagnostics. Run focused core/client tests. - 5. **Full regression and release-quality verification** - - **Implementation**: - - Review all changed interfaces/mocks and formatting paths for compatibility, update comments/help text only where behavior changed, and add any missing edge-case tests discovered by prior phases. - - Keep implementation within the existing package architecture and avoid live mutation checks. - - **Verification**: - - Run `make build` and `make test` (equivalent to `go test -v ./...`). If the Go toolchain is unavailable, report that limitation rather than substituting an unverified claim. - - Optionally run read-only label listing commands against a configured team to inspect grouped output, but do not run issue create/update mutations. - -**Phase Verification Approach**: Each phase must pass its focused automated tests before the next phase begins. The final phase requires the repository build and full unit suite; no manual GUI work is applicable. Live API mutation verification is explicitly out of scope. - -## Quality Assurance Plan -- **Testing Strategy**: Pure unit tests for resolution/cache/validation/formatting, service tests with mutation-call spies, GraphQL mock transport tests for serialization/error boundaries, then full Go build and unit suite. -- **Edge Cases**: - - Two, three, or more siblings in one group; conflicts in multiple groups; duplicate IDs and case variants. - - UUID inputs, names with punctuation, unknown or ambiguous labels, cross-team UUIDs, parent/group labels, standalone labels, and labels from different groups. - - Existing conflicting labels during additive updates; replace versus add/remove semantics; explicit empty replacement flags; simultaneous team changes; remove-only and remove-all updates. - - API responses whose parent records appear after children or are absent; complete/paginated label retrieval; deep-copy/cache expiry; JSON parent references; GraphQL errors with and without extensions. - - Exact operation-context count, deduplicated/sorted mutation IDs, and absence of raw query/mutation text. -- **Regression Prevention**: Preserve public ID-only resolver methods and raw label JSON schema, retain typed GraphQL errors and error classification, keep valid issue mutations unchanged, and update every interface mock in the repository. -- **Success Verification**: All TL-562 acceptance criteria are represented by focused tests, `make build` succeeds, and `make test` passes without live mutation calls. - -## Development Environment -- **Setup Requirements**: Go toolchain and repository dependencies from `go.mod`; no new dependency expected. Use existing mock transport/server helpers and current branch `tl-562`. -- **Debugging Strategy**: Use typed errors, focused mock response/request assertions, and deterministic output snapshots/string assertions. Never add query text to user-facing errors. -- **Iteration Approach**: Run focused package tests after each phase, then `make build`/`make test`; inspect `git diff` for interface and JSON compatibility changes. - -## Risk Assessment -- **Potential Issues**: - - Adding a metadata resolver method can break multiple mocks and consumers. - - Existing callers may rely on exact duplicated error strings or query previews. - - Issue label data may lack parent metadata in some paths, especially fixtures or older responses. - - The label endpoint may paginate despite the current query exposing only `nodes`. - - Reordering text output may affect snapshot/string tests and scripts that incorrectly parse human output. - - Non-nil empty label slices may interact with GraphQL input omission logic in more than one layer. - - Simultaneous team and label updates can mix source-team and target-team IDs if not rejected or modeled explicitly. -- **Mitigation Strategies**: - - Keep existing resolver signatures as adapters and update all compile-time mocks in one phase. - - Preserve typed underlying errors and test only the intended single context; remove query previews deliberately and cover the global behavior with regression tests. - - Hydrate missing metadata through the complete/paginated team-label resolver only when needed; if authoritative metadata cannot be obtained, return a clear validation error; use parent ID as the sole conflict key. - - Leave JSON unchanged and make text ordering explicit and deterministic; use parent name/ID fallback headers when group records are absent. - - Add request-payload tests specifically for `labelIds: []`, use one shared field-presence helper or audit every check, and test CLI flag-changed semantics for empty replacement. - - Reject additive/removal label modes combined with a team change unless the implementation can prove all final IDs belong to the target team. -- **Backup Approaches**: - - If changing the service interface proves too invasive, introduce a narrow optional metadata resolver capability while retaining the existing ID-only interface and fail validation only when metadata cannot be obtained. - - If shared formatter integration causes command-specific regressions, keep one shared grouping primitive with thin team/label renderers rather than duplicating grouping logic or forcing one service to own the other's command behavior. - - If the three nil/empty checks drift during implementation, centralize label-field presence in a small helper used by service gating, client gating, and GraphQL input construction. - -## Files Likely to Change -- `pkg/linear/resolver.go` — metadata-aware, UUID-capable label resolution and compatibility adapter. -- `pkg/linear/resolver_cache.go` — complete label metadata cache and normalized name/ID indexes. -- `pkg/linear/client.go` — public/internal metadata resolver delegate. -- `pkg/linear/core/errors.go` — any typed conflict/error helpers or GraphQL extension handling needed. -- `pkg/linear/core/base_client.go` — remove query previews and preserve GraphQL extensions. -- `pkg/linear/issues/client.go` — update label field presence/empty-array serialization and remote operation error boundary. -- `pkg/linear/teams/client.go` — paginate label retrieval if required by the API contract while preserving parent fields. -- `internal/service/client_interfaces.go` — metadata resolver capability. -- `internal/service/issue.go` — create/update conflict validation, final label-set construction, and wrapper cleanup. -- `internal/cli/issues.go` — remove duplicate create/update error wrapping if still present after service boundary changes. -- `internal/service/team.go` — use grouped text label formatter. -- `internal/service/label.go` — use grouped text label formatter. -- `internal/format/labels.go` — new shared grouped text renderer. -- `pkg/linear/resolver_test.go` — resolution/cache behavior. -- `internal/service/issue_create_test.go` and `internal/service/issue_relation_test.go` (or a focused new update test) — create/update conflict and mutation suppression. -- `internal/cli/issues.go` and its CLI tests/helpers — empty replacement flag presence, label-mode exclusivity, and wrapper behavior. -- `pkg/linear/issues/client_test.go` — update payload and operation-error behavior. -- `pkg/linear/core/base_client_test.go` — GraphQL sanitization/extensions regression tests. -- `internal/format/labels_test.go` — grouped rendering tests. -- `internal/service/team_test.go` and/or `internal/service/label_test.go` — command integration and JSON compatibility tests. -- Other repository service mocks implementing `IssueClientOperations` — compile/test updates for the metadata resolver method.