diff --git a/internal/cli/issues.go b/internal/cli/issues.go index 9567429..643c9af 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,16 +534,18 @@ 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") + addLabelsChanged := cmd.Flags().Changed("add-labels") + removeLabelsChanged := cmd.Flags().Changed("remove-labels") hasFlags := title != "" || description != "" || state != "" || - priority != "" || estimate != "" || labels != "" || - addLabels != "" || removeLabels != "" || + priority != "" || estimate != "" || labelsChanged || addLabelsChanged || removeLabelsChanged || cycle != "" || project != "" || assignee != "" || dueDate != "" || parent != "" || dependsOn != "" || blockedBy != "" || len(attachFiles) > 0 @@ -553,8 +555,8 @@ LABEL MODES: } // Validate mutual exclusivity: --labels cannot be used with --add-labels or --remove-labels - if labels != "" && (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 @@ -597,8 +599,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 +641,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) @@ -666,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 new file mode 100644 index 0000000..8af67e2 --- /dev/null +++ b/internal/format/labels.go @@ -0,0 +1,199 @@ +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. +// Deprecated compatibility entry point; new callers should use LabelList. +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..dbbdf2f 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,71 @@ 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) { + teamKey, _, parseErr := identifiers.ParseIssueIdentifier(issue.Identifier) + if parseErr != nil { + return "", fmt.Errorf("failed to update issue: cannot determine current team: %w", parseErr) } - linearInput.LabelIDs = resolvedLabelIDs - } else { - // Additive/subtractive mode: fetch current labels, merge/remove, then set - currentLabelIDs := s.extractCurrentLabelIDs(issue) + 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") + } + } - // Build a set from current labels for efficient merge/remove - labelSet := make(map[string]bool, len(currentLabelIDs)) - for _, id := range currentLabelIDs { - labelSet[id] = true + 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 = labelIDs(resolvedLabels) + } else { + 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) + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': %w", labelName, err) } - labelSet[labelID] = true + if label == nil { + return "", fmt.Errorf("failed to update issue: failed to resolve label '%s': resolver returned no label", labelName) + } + 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) + 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, 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 +787,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 +885,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 +900,45 @@ 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 { + // 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 +} + // 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/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 e56e4ed..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,16 +45,11 @@ 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) - } + if len(labels) == 0 { + return "No labels found.", nil } - 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..732d4a5 --- /dev/null +++ b/internal/service/label_validation.go @@ -0,0 +1,117 @@ +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 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 { + 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) + 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..e41ed98 --- /dev/null +++ b/internal/service/label_validation_test.go @@ -0,0 +1,54 @@ +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 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"}}, + {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..8104c1c 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 @@ -116,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 { @@ -128,16 +115,11 @@ 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) - } + if len(labels) == 0 { + return "No labels found.", nil } - 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..fc21544 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") + return nil, fmt.Errorf("failed to create issue: mutation 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") + return nil, fmt.Errorf("failed to update issue: mutation 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..cdfefd7 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,9 @@ 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 + labelData map[string]*labelCacheEntry // teamID:normalized-id → label metadata // Project resolution cache projectByName map[string]*cacheEntry // project name → projectID @@ -53,6 +61,7 @@ func newResolverCache(ttl time.Duration) *resolverCache { teamByKey: make(map[string]*cacheEntry), issueByIdentifier: make(map[string]*cacheEntry), labelByName: make(map[string]*cacheEntry), + labelData: make(map[string]*labelCacheEntry), projectByName: make(map[string]*cacheEntry), ttl: ttl, } @@ -176,26 +185,69 @@ 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) setLabelByName(teamID, labelName, labelID string) { +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) setLabels(teamID string, labels []core.Label) { rc.mu.Lock() defer rc.mu.Unlock() - key := teamID + ":" + labelName - rc.labelByName[key] = &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) + } + } + + 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.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} + } } } @@ -265,12 +317,16 @@ 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.labelData { + if entry.expiresAt.Before(now) { + delete(rc.labelData, key) + } + } // Clean up project cache for name, entry := range rc.projectByName { @@ -304,5 +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.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..867ee67 --- /dev/null +++ b/pkg/linear/resolver_cache_test.go @@ -0,0 +1,56 @@ +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 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() + 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/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) +}