fix: improve @mention regex, index safety, and test coverage

- Require word boundary before @ to avoid matching inside emails/URLs
- Treat empty identifiers as not-found in ResolvePeople for safe indexing
- Add ResolvePeople tests covering happy path, errors, and edge cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: brigleb

cmd/comment/create.go

@@ -20,7 +20,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var mentionRe = regexp.MustCompile(`@[\w]+(?:\.[\w]+)*`)
+var mentionRe = regexp.MustCompile(`(?:^|[>\s])(@[\w]+(?:\.[\w]+)*)`)
 
 func newCreateCmd(f *factory.Factory) *cobra.Command {
 	var content string
@@ -127,19 +127,21 @@ You can provide comment content in several ways:
 
 			// Replace inline @Name mentions with bc-attachment tags
 			// Supports @FirstName and @First.Last for disambiguation
-			inlineMatches := mentionRe.FindAllString(richContent, -1)
-			if len(inlineMatches) > 0 {
+			submatches := mentionRe.FindAllStringSubmatch(richContent, -1)
+			if len(submatches) > 0 {
 				resolver := utils.NewUserResolver(client.Client, projectID)
-				// Convert @First.Last to "First Last" for resolution
-				identifiers := make([]string, len(inlineMatches))
-				for i, m := range inlineMatches {
-					identifiers[i] = strings.ReplaceAll(strings.TrimPrefix(m, "@"), ".", " ")
+				// Extract capture group (the @mention) and convert @First.Last to "First Last"
+				mentions := make([]string, len(submatches))
+				identifiers := make([]string, len(submatches))
+				for i, sm := range submatches {
+					mentions[i] = sm[1]
+					identifiers[i] = strings.ReplaceAll(strings.TrimPrefix(sm[1], "@"), ".", " ")
 				}
 				people, err := resolver.ResolvePeople(f.Context(), identifiers)
 				if err != nil {
 					return fmt.Errorf("failed to resolve mentions: %w", err)
 				}
-				for i, match := range inlineMatches {
+				for i, match := range mentions {
 					tag := attachments.BuildTag(people[i].AttachableSGID)
 					richContent = strings.Replace(richContent, match, tag, 1)
 				}

internal/utils/users.go

@@ -88,13 +88,14 @@ func (ur *UserResolver) ResolvePeople(ctx context.Context, identifiers []string)
 
 	for _, identifier := range identifiers {
 		identifier = strings.TrimSpace(identifier)
-		if identifier == "" {
-			continue
-		}
 
 		personID, found := ur.resolveIdentifier(identifier)
 		if !found {
-			notFound = append(notFound, identifier)
+			if identifier == "" {
+				notFound = append(notFound, "(empty)")
+			} else {
+				notFound = append(notFound, identifier)
+			}
 			continue
 		}
 

internal/utils/users_test.go

@@ -199,6 +199,90 @@ func TestUserResolver_ResolveUsers(t *testing.T) {
 	}
 }
 
+func TestUserResolver_ResolvePeople(t *testing.T) {
+	people := []api.Person{
+		{ID: 1, Name: "John Doe", EmailAddress: "john@example.com", AttachableSGID: "sgid-john"},
+		{ID: 2, Name: "Jane Smith", EmailAddress: "jane@example.com", AttachableSGID: "sgid-jane"},
+		{ID: 3, Name: "Bob Johnson", EmailAddress: "bob@company.com", AttachableSGID: "sgid-bob"},
+	}
+
+	tests := []struct {
+		name         string
+		identifiers  []string
+		mockPeople   []api.Person
+		mockError    error
+		expectPeople []api.Person
+		expectError  bool
+	}{
+		{
+			name:         "Resolve single person by name",
+			identifiers:  []string{"John"},
+			mockPeople:   people,
+			expectPeople: []api.Person{people[0]},
+			expectError:  false,
+		},
+		{
+			name:         "Resolve multiple people",
+			identifiers:  []string{"John", "Jane"},
+			mockPeople:   people,
+			expectPeople: []api.Person{people[0], people[1]},
+			expectError:  false,
+		},
+		{
+			name:        "Not found error",
+			identifiers: []string{"Unknown"},
+			mockPeople:  people,
+			expectError: true,
+		},
+		{
+			name:        "API error",
+			identifiers: []string{"John"},
+			mockError:   errors.New("API error"),
+			expectError: true,
+		},
+		{
+			name:        "Empty identifier treated as not found",
+			identifiers: []string{""},
+			mockPeople:  people,
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockClient := mock.NewMockClient()
+			mockClient.People = tt.mockPeople
+			mockClient.PeopleError = tt.mockError
+
+			resolver := NewUserResolver(mockClient, "12345")
+			ctx := context.Background()
+
+			result, err := resolver.ResolvePeople(ctx, tt.identifiers)
+
+			if tt.expectError && err == nil {
+				t.Error("Expected error but got none")
+			}
+			if !tt.expectError && err != nil {
+				t.Errorf("Unexpected error: %v", err)
+			}
+
+			if !tt.expectError {
+				if len(result) != len(tt.expectPeople) {
+					t.Fatalf("Expected %d people, got %d", len(tt.expectPeople), len(result))
+				}
+				for i, p := range result {
+					if p.ID != tt.expectPeople[i].ID {
+						t.Errorf("Expected person[%d].ID = %d, got %d", i, tt.expectPeople[i].ID, p.ID)
+					}
+					if p.AttachableSGID != tt.expectPeople[i].AttachableSGID {
+						t.Errorf("Expected person[%d].AttachableSGID = %q, got %q", i, tt.expectPeople[i].AttachableSGID, p.AttachableSGID)
+					}
+				}
+			}
+		})
+	}
+}
+
 func TestUserResolver_resolveIdentifier(t *testing.T) {
 	// Create test people
 	ur := &UserResolver{