From 319e339334ac1c885c667ef485537e3f8f1e35bb Mon Sep 17 00:00:00 2001 From: Wayne Smith Date: Sat, 28 Mar 2026 16:05:15 +0200 Subject: [PATCH 1/4] Add @mention resolution for comments and descriptions Resolve @FirstName patterns in comment and card description text to ActionText mention HTML before sending to the API. This triggers Fizzy notifications for mentioned users, matching the behavior of the web UI. - Add GetHTML() to client for fetching HTML endpoints - Fetch mentionable users from /prompts/users endpoint - Parse elements for sgid and name data - Replace @FirstName with mention HTML - Cache user list per CLI invocation (sync.Once) - Graceful degradation: on fetch error, text is sent unchanged - Skip email patterns (user@example.com) - Warn on ambiguous or unresolved mentions - Update SKILL.md with @mention documentation Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/client/client.go | 44 ++++++ internal/client/interface.go | 1 + internal/commands/card.go | 8 +- internal/commands/comment.go | 8 +- internal/commands/mentions.go | 207 ++++++++++++++++++++++++++ internal/commands/mentions_test.go | 200 +++++++++++++++++++++++++ internal/commands/mock_client_test.go | 14 ++ internal/skills/SKILL.md | 24 +++ skills/fizzy/SKILL.md | 24 +++ 9 files changed, 522 insertions(+), 8 deletions(-) create mode 100644 internal/commands/mentions.go create mode 100644 internal/commands/mentions_test.go diff --git a/internal/client/client.go b/internal/client/client.go index 22c3668..b9e1fa9 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -170,6 +170,50 @@ func (c *Client) Delete(path string) (*APIResponse, error) { return c.request("DELETE", path, nil) } +// GetHTML performs a GET request expecting an HTML response. +// Unlike Get, it sets Accept: text/html and does not attempt JSON parsing. +func (c *Client) GetHTML(path string) (*APIResponse, error) { + requestURL := c.buildURL(path) + req, err := http.NewRequestWithContext(context.Background(), "GET", requestURL, nil) + if err != nil { + return nil, errors.NewNetworkError(fmt.Sprintf("Failed to create request: %v", err)) + } + + req.Header.Set("Authorization", "Bearer "+c.Token) + req.Header.Set("Accept", "text/html") + req.Header.Set("User-Agent", "fizzy-cli/1.0") + + if c.Verbose { + fmt.Fprintf(os.Stderr, "> GET %s (HTML)\n", requestURL) + } + + resp, err := c.doWithRetry(req) + if err != nil { + return nil, errors.NewNetworkError(fmt.Sprintf("Request failed: %v", err)) + } + defer func() { _ = resp.Body.Close() }() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return nil, errors.NewNetworkError(fmt.Sprintf("Failed to read response: %v", err)) + } + + if c.Verbose { + fmt.Fprintf(os.Stderr, "< %d %s\n", resp.StatusCode, http.StatusText(resp.StatusCode)) + } + + apiResp := &APIResponse{ + StatusCode: resp.StatusCode, + Body: respBody, + } + + if resp.StatusCode >= 400 { + return apiResp, c.errorFromResponse(resp.StatusCode, respBody, resp.Header) + } + + return apiResp, nil +} + func (c *Client) request(method, path string, body any) (*APIResponse, error) { requestURL := c.buildURL(path) diff --git a/internal/client/interface.go b/internal/client/interface.go index 54f2163..f157c4e 100644 --- a/internal/client/interface.go +++ b/internal/client/interface.go @@ -13,6 +13,7 @@ type API interface { FollowLocation(location string) (*APIResponse, error) UploadFile(filePath string) (*APIResponse, error) DownloadFile(urlPath string, destPath string) error + GetHTML(path string) (*APIResponse, error) } // Ensure Client implements API interface diff --git a/internal/commands/card.go b/internal/commands/card.go index b0eff75..7d14e7a 100644 --- a/internal/commands/card.go +++ b/internal/commands/card.go @@ -293,9 +293,9 @@ var cardCreateCmd = &cobra.Command{ if descErr != nil { return descErr } - description = markdownToHTML(string(descContent)) + description = markdownToHTML(resolveMentions(string(descContent), getClient())) } else if cardCreateDescription != "" { - description = markdownToHTML(cardCreateDescription) + description = markdownToHTML(resolveMentions(cardCreateDescription, getClient())) } ac := getSDK() @@ -384,9 +384,9 @@ var cardUpdateCmd = &cobra.Command{ if err != nil { return err } - description = markdownToHTML(string(content)) + description = markdownToHTML(resolveMentions(string(content), getClient())) } else if cardUpdateDescription != "" { - description = markdownToHTML(cardUpdateDescription) + description = markdownToHTML(resolveMentions(cardUpdateDescription, getClient())) } // Build breadcrumbs diff --git a/internal/commands/comment.go b/internal/commands/comment.go index 00b385e..e52278a 100644 --- a/internal/commands/comment.go +++ b/internal/commands/comment.go @@ -149,9 +149,9 @@ var commentCreateCmd = &cobra.Command{ if err != nil { return err } - body = markdownToHTML(string(content)) + body = markdownToHTML(resolveMentions(string(content), getClient())) } else if commentCreateBody != "" { - body = markdownToHTML(commentCreateBody) + body = markdownToHTML(resolveMentions(commentCreateBody, getClient())) } else { return newRequiredFlagError("body or body_file") } @@ -209,9 +209,9 @@ var commentUpdateCmd = &cobra.Command{ if err != nil { return err } - body = markdownToHTML(string(content)) + body = markdownToHTML(resolveMentions(string(content), getClient())) } else if commentUpdateBody != "" { - body = markdownToHTML(commentUpdateBody) + body = markdownToHTML(resolveMentions(commentUpdateBody, getClient())) } commentID := args[0] diff --git a/internal/commands/mentions.go b/internal/commands/mentions.go new file mode 100644 index 0000000..6c4ecf5 --- /dev/null +++ b/internal/commands/mentions.go @@ -0,0 +1,207 @@ +package commands + +import ( + "fmt" + "os" + "regexp" + "strings" + "sync" + "unicode" + + "github.com/basecamp/fizzy-cli/internal/client" +) + +// mentionUser represents a mentionable user parsed from the /prompts/users endpoint. +type mentionUser struct { + FirstName string // e.g. "Wayne" + FullName string // e.g. "Wayne Smith" + SGID string // signed global ID for ActionText + AvatarSrc string // e.g. "/6103476/users/03f5awg7.../avatar" +} + +// Package-level cache: populated once per CLI invocation. +var ( + mentionUsers []mentionUser + mentionOnce sync.Once + mentionErr error +) + +// resetMentionCache resets the cache for testing. +func resetMentionCache() { + mentionOnce = sync.Once{} + mentionUsers = nil + mentionErr = nil +} + +// mentionRegex matches @Name patterns not preceded by word characters or dots +// (to avoid matching emails like user@example.com). +var mentionRegex = regexp.MustCompile(`(?:^|[^a-zA-Z0-9_.])@([a-zA-Z][a-zA-Z0-9_]*)`) + +// promptItemRegex parses tags from the /prompts/users HTML. +var promptItemRegex = regexp.MustCompile(`]*>`) + +// avatarRegex extracts the src attribute from the first in editor template. +var avatarRegex = regexp.MustCompile(`]+src="([^"]+)"`) + +// resolveMentions scans text for @FirstName patterns and replaces them with +// ActionText mention HTML. If the text contains no @ characters, it is returned +// unchanged. On any error fetching users, the original text is returned with a +// warning printed to stderr. +func resolveMentions(text string, c client.API) string { + if !strings.Contains(text, "@") { + return text + } + + mentionOnce.Do(func() { + mentionUsers, mentionErr = fetchMentionUsers(c) + }) + + if mentionErr != nil { + fmt.Fprintf(os.Stderr, "Warning: could not fetch mentionable users: %v\n", mentionErr) + return text + } + + if len(mentionUsers) == 0 { + return text + } + + // Find all @Name matches with positions + type mentionMatch struct { + start int // start of @Name (the @ character) + end int // end of @Name + name string + } + + allMatches := mentionRegex.FindAllStringSubmatchIndex(text, -1) + var matches []mentionMatch + for _, loc := range allMatches { + // loc[2]:loc[3] is the capture group (the name without @) + nameStart := loc[2] + nameEnd := loc[3] + // The @ is one character before the name + atStart := nameStart - 1 + name := text[nameStart:nameEnd] + matches = append(matches, mentionMatch{start: atStart, end: nameEnd, name: name}) + } + + // Process from end to start so replacements don't shift indices + for i := len(matches) - 1; i >= 0; i-- { + m := matches[i] + + // Find matching user by first name (case-insensitive) + var found []mentionUser + for _, u := range mentionUsers { + if strings.EqualFold(u.FirstName, m.name) { + found = append(found, u) + } + } + + switch len(found) { + case 1: + html := buildMentionHTML(found[0]) + text = text[:m.start] + html + text[m.end:] + case 0: + fmt.Fprintf(os.Stderr, "Warning: could not resolve mention @%s\n", m.name) + default: + names := make([]string, len(found)) + for j, u := range found { + names[j] = u.FullName + } + fmt.Fprintf(os.Stderr, "Warning: ambiguous mention @%s — matches: %s\n", m.name, strings.Join(names, ", ")) + } + } + + return text +} + +// fetchMentionUsers fetches the list of mentionable users from the API. +func fetchMentionUsers(c client.API) ([]mentionUser, error) { + resp, err := c.GetHTML("/prompts/users") + if err != nil { + return nil, err + } + if resp.StatusCode != 200 { + return nil, fmt.Errorf("unexpected status %d from /prompts/users", resp.StatusCode) + } + return parseMentionUsers(resp.Body), nil +} + +// parseMentionUsers extracts mentionable users from the /prompts/users HTML. +// Each user is represented as a element with search and sgid +// attributes, containing tags with avatar URLs. +func parseMentionUsers(html []byte) []mentionUser { + htmlStr := string(html) + items := promptItemRegex.FindAllStringSubmatch(htmlStr, -1) + if len(items) == 0 { + return nil + } + + var users []mentionUser + for _, item := range items { + search := strings.TrimSpace(item[1]) + sgid := item[2] + + if search == "" || sgid == "" { + continue + } + + // Parse name from search attribute. + // Format: "Full Name INITIALS [me]" + // Strip trailing "me" and all-uppercase words (initials like "WS", "FMA"). + words := strings.Fields(search) + for len(words) > 1 { + last := words[len(words)-1] + if last == "me" || isAllUpper(last) { + words = words[:len(words)-1] + } else { + break + } + } + + fullName := strings.Join(words, " ") + firstName := words[0] + + // Extract avatar URL: find the after this sgid in the HTML. + avatarSrc := "" + sgidIdx := strings.Index(htmlStr, `sgid="`+sgid+`"`) + if sgidIdx >= 0 { + // Search for the first after the sgid + remainder := htmlStr[sgidIdx:] + if m := avatarRegex.FindStringSubmatch(remainder); len(m) > 1 { + avatarSrc = m[1] + } + } + + users = append(users, mentionUser{ + FirstName: firstName, + FullName: fullName, + SGID: sgid, + AvatarSrc: avatarSrc, + }) + } + + return users +} + +// buildMentionHTML creates the ActionText attachment HTML for a mention. +func buildMentionHTML(u mentionUser) string { + return fmt.Sprintf( + ``+ + `%s`+ + ``, + u.SGID, u.FullName, u.AvatarSrc, u.FirstName, + ) +} + +// isAllUpper returns true if s is non-empty and all uppercase letters. +func isAllUpper(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if !unicode.IsUpper(r) { + return false + } + } + return true +} diff --git a/internal/commands/mentions_test.go b/internal/commands/mentions_test.go new file mode 100644 index 0000000..9bf4a78 --- /dev/null +++ b/internal/commands/mentions_test.go @@ -0,0 +1,200 @@ +package commands + +import ( + "fmt" + "strings" + "testing" + + "github.com/basecamp/fizzy-cli/internal/client" +) + +const samplePromptUsersHTML = ` + + + + + + + + + + + + +` + +func newMentionMockClient() *MockClient { + m := NewMockClient() + m.GetHTMLResponse = &client.APIResponse{ + StatusCode: 200, + Body: []byte(samplePromptUsersHTML), + } + return m +} + +func TestParseMentionUsers(t *testing.T) { + users := parseMentionUsers([]byte(samplePromptUsersHTML)) + + if len(users) != 3 { + t.Fatalf("expected 3 users, got %d", len(users)) + } + + tests := []struct { + idx int + firstName string + fullName string + sgid string + }{ + {0, "Wayne", "Wayne Smith", "wayne-sgid-123"}, + {1, "Bushra", "Bushra Gul", "bushra-sgid-456"}, + {2, "Kennedy", "Kennedy", "kennedy-sgid-789"}, + } + + for _, tt := range tests { + u := users[tt.idx] + if u.FirstName != tt.firstName { + t.Errorf("user[%d] FirstName = %q, want %q", tt.idx, u.FirstName, tt.firstName) + } + if u.FullName != tt.fullName { + t.Errorf("user[%d] FullName = %q, want %q", tt.idx, u.FullName, tt.fullName) + } + if u.SGID != tt.sgid { + t.Errorf("user[%d] SGID = %q, want %q", tt.idx, u.SGID, tt.sgid) + } + if u.AvatarSrc == "" { + t.Errorf("user[%d] AvatarSrc is empty", tt.idx) + } + } +} + +func TestParseMentionUsersEmpty(t *testing.T) { + users := parseMentionUsers([]byte("")) + if len(users) != 0 { + t.Errorf("expected 0 users from empty HTML, got %d", len(users)) + } +} + +func TestResolveMentions(t *testing.T) { + tests := []struct { + name string + input string + shouldContain []string + shouldNotMatch []string // substrings that should NOT appear + }{ + { + name: "no @ passthrough", + input: "Hello world", + shouldContain: []string{"Hello world"}, + shouldNotMatch: []string{"action-text-attachment"}, + }, + { + name: "single mention", + input: "Hey @Wayne check this", + shouldContain: []string{`sgid="wayne-sgid-123"`, `content-type="application/vnd.actiontext.mention"`, `title="Wayne Smith"`, ">Wayne<"}, + }, + { + name: "case insensitive", + input: "Hey @wayne check this", + shouldContain: []string{`sgid="wayne-sgid-123"`}, + }, + { + name: "multiple mentions", + input: "@Wayne and @Bushra please review", + shouldContain: []string{`sgid="wayne-sgid-123"`, `sgid="bushra-sgid-456"`}, + }, + { + name: "email not treated as mention", + input: "Contact user@example.com", + shouldContain: []string{"user@example.com"}, + shouldNotMatch: []string{"action-text-attachment"}, + }, + { + name: "unresolved mention stays as text", + input: "Hey @Unknown person", + shouldContain: []string{"@Unknown"}, + shouldNotMatch: []string{"action-text-attachment"}, + }, + { + name: "mention at start of text", + input: "@Kennedy can you look?", + shouldContain: []string{`sgid="kennedy-sgid-789"`}, + }, + { + name: "mention after newline", + input: "First line\n@Wayne second line", + shouldContain: []string{`sgid="wayne-sgid-123"`}, + }, + { + name: "single name user", + input: "Hey @Kennedy", + shouldContain: []string{`sgid="kennedy-sgid-789"`, `title="Kennedy"`}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resetMentionCache() + mock := newMentionMockClient() + result := resolveMentions(tt.input, mock) + + for _, s := range tt.shouldContain { + if !strings.Contains(result, s) { + t.Errorf("result should contain %q\ngot: %s", s, result) + } + } + for _, s := range tt.shouldNotMatch { + if strings.Contains(result, s) { + t.Errorf("result should NOT contain %q\ngot: %s", s, result) + } + } + }) + } +} + +func TestResolveMentionsAPIError(t *testing.T) { + resetMentionCache() + mock := NewMockClient() + mock.GetHTMLError = fmt.Errorf("server error") + + // Should return text unchanged on error + input := "Hey @Wayne" + result := resolveMentions(input, mock) + if result != input { + t.Errorf("expected unchanged text on error, got: %s", result) + } +} + +func TestResolveMentionsCaching(t *testing.T) { + resetMentionCache() + mock := newMentionMockClient() + + // First call fetches + resolveMentions("@Wayne", mock) + if len(mock.GetHTMLCalls) != 1 { + t.Errorf("expected 1 GetHTML call, got %d", len(mock.GetHTMLCalls)) + } + + // Second call uses cache + resolveMentions("@Bushra", mock) + if len(mock.GetHTMLCalls) != 1 { + t.Errorf("expected still 1 GetHTML call (cached), got %d", len(mock.GetHTMLCalls)) + } +} diff --git a/internal/commands/mock_client_test.go b/internal/commands/mock_client_test.go index 42e09ab..81ab76e 100644 --- a/internal/commands/mock_client_test.go +++ b/internal/commands/mock_client_test.go @@ -22,6 +22,7 @@ type MockClient struct { UploadFileResponse *client.APIResponse PatchMultipartResponse *client.APIResponse + GetHTMLResponse *client.APIResponse // Path-based GET response routing (checked before GetResponse) getPathResponses map[string]*client.APIResponse @@ -37,6 +38,7 @@ type MockClient struct { FollowLocationError error UploadFileError error DownloadFileError error + GetHTMLError error // Captured calls for verification GetCalls []MockCall @@ -49,6 +51,7 @@ type MockClient struct { FollowLocationCalls []string UploadFileCalls []string DownloadFileCalls []MockDownloadCall + GetHTMLCalls []MockCall } // MockDownloadCall represents a captured download call. @@ -213,6 +216,17 @@ func (m *MockClient) DownloadFile(urlPath string, destPath string) error { return nil } +func (m *MockClient) GetHTML(path string) (*client.APIResponse, error) { + m.GetHTMLCalls = append(m.GetHTMLCalls, MockCall{Path: path}) + if m.GetHTMLError != nil { + return nil, m.GetHTMLError + } + if m.GetHTMLResponse != nil { + return m.GetHTMLResponse, nil + } + return &client.APIResponse{StatusCode: 200, Body: []byte("")}, nil +} + // Helper functions for creating common responses // WithGetData sets the data returned by Get calls. diff --git a/internal/skills/SKILL.md b/internal/skills/SKILL.md index 6d4c22a..f8e0f61 100644 --- a/internal/skills/SKILL.md +++ b/internal/skills/SKILL.md @@ -67,6 +67,30 @@ Full CLI coverage: boards, cards, columns, comments, steps, reactions, tags, use 5. **Rich text fields accept HTML** — use `

` tags for paragraphs, `` for inline images 6. **Card description is a string**, but comment body is a nested object — `.description` vs `.body.plain_text` 7. **Display the welcome message for new signups** — When `signup complete --name` returns `is_new_user: true`, you MUST immediately display the `welcome_message` field prominently to the user. This is a one-time personal note from the CEO — if you skip it, the user will never see it. +8. **Use @FirstName for mentions** — Write `@FirstName` in comment or description text to mention a user. The CLI auto-resolves mentions to ActionText HTML, triggering Fizzy notifications. Case-insensitive. Emails like `user@example.com` are not treated as mentions. + +## @Mentions + +Use `@FirstName` in comments and descriptions to mention users. The CLI resolves them to Fizzy mentions with notifications — no need to construct HTML manually. + +```bash +# Mention in a comment +fizzy comment create --card 42 --body "Hey @Sarah, can you review this?" + +# Multiple mentions +fizzy comment create --card 42 --body "@Alex @Jordan please check the latest deploy" + +# Mention in a card description +fizzy card create --board ID --title "Fix bug" --description "@Pat this is the issue you mentioned" +``` + +- **Case-insensitive** — `@sarah` and `@Sarah` both work +- **Emails ignored** — `user@example.com` is not treated as a mention +- **Ambiguous names warned** — if multiple users share a first name, a warning is printed and the text is left unchanged +- **Unresolved names left as text** — `@Unknown` stays as `@Unknown` with a warning +- **Cached per invocation** — user list is fetched once and reused for the entire command + +To see available users for mentioning: `fizzy user list | jq '[.data[] | .name]'` ## Decision Trees diff --git a/skills/fizzy/SKILL.md b/skills/fizzy/SKILL.md index 6d4c22a..f8e0f61 100644 --- a/skills/fizzy/SKILL.md +++ b/skills/fizzy/SKILL.md @@ -67,6 +67,30 @@ Full CLI coverage: boards, cards, columns, comments, steps, reactions, tags, use 5. **Rich text fields accept HTML** — use `

` tags for paragraphs, `` for inline images 6. **Card description is a string**, but comment body is a nested object — `.description` vs `.body.plain_text` 7. **Display the welcome message for new signups** — When `signup complete --name` returns `is_new_user: true`, you MUST immediately display the `welcome_message` field prominently to the user. This is a one-time personal note from the CEO — if you skip it, the user will never see it. +8. **Use @FirstName for mentions** — Write `@FirstName` in comment or description text to mention a user. The CLI auto-resolves mentions to ActionText HTML, triggering Fizzy notifications. Case-insensitive. Emails like `user@example.com` are not treated as mentions. + +## @Mentions + +Use `@FirstName` in comments and descriptions to mention users. The CLI resolves them to Fizzy mentions with notifications — no need to construct HTML manually. + +```bash +# Mention in a comment +fizzy comment create --card 42 --body "Hey @Sarah, can you review this?" + +# Multiple mentions +fizzy comment create --card 42 --body "@Alex @Jordan please check the latest deploy" + +# Mention in a card description +fizzy card create --board ID --title "Fix bug" --description "@Pat this is the issue you mentioned" +``` + +- **Case-insensitive** — `@sarah` and `@Sarah` both work +- **Emails ignored** — `user@example.com` is not treated as a mention +- **Ambiguous names warned** — if multiple users share a first name, a warning is printed and the text is left unchanged +- **Unresolved names left as text** — `@Unknown` stays as `@Unknown` with a warning +- **Cached per invocation** — user list is fetched once and reused for the entire command + +To see available users for mentioning: `fizzy user list | jq '[.data[] | .name]'` ## Decision Trees From c0b727e7ea5ee2e2f1c63d7d4011a92f8c79ddbd Mon Sep 17 00:00:00 2001 From: Wayne Smith Date: Sat, 28 Mar 2026 16:28:23 +0200 Subject: [PATCH 2/4] Address review feedback on mention resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Support Unicode letters and hyphens in @mention names (e.g. @José, @Mary-Jane) - Parse attributes in any order (not position-dependent) - Scope avatar lookup to each prompt-item block (prevents cross-user mismatch) - HTML-escape all user-controlled values in buildMentionHTML (prevent injection) - Skip @mentions inside markdown code spans and fenced code blocks - Reuse client.setHeaders() in GetHTML instead of duplicating header logic - Add tests: ambiguous mentions, no-fetch passthrough, attribute order, avatar scoping, code block protection, HTML escaping Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/client/client.go | 3 +- internal/commands/mentions.go | 103 +++++++++++++++++++------ internal/commands/mentions_test.go | 117 +++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 26 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index b9e1fa9..660f942 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -179,9 +179,8 @@ func (c *Client) GetHTML(path string) (*APIResponse, error) { return nil, errors.NewNetworkError(fmt.Sprintf("Failed to create request: %v", err)) } - req.Header.Set("Authorization", "Bearer "+c.Token) + c.setHeaders(req) req.Header.Set("Accept", "text/html") - req.Header.Set("User-Agent", "fizzy-cli/1.0") if c.Verbose { fmt.Fprintf(os.Stderr, "> GET %s (HTML)\n", requestURL) diff --git a/internal/commands/mentions.go b/internal/commands/mentions.go index 6c4ecf5..2337e01 100644 --- a/internal/commands/mentions.go +++ b/internal/commands/mentions.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "html" "os" "regexp" "strings" @@ -35,18 +36,38 @@ func resetMentionCache() { // mentionRegex matches @Name patterns not preceded by word characters or dots // (to avoid matching emails like user@example.com). -var mentionRegex = regexp.MustCompile(`(?:^|[^a-zA-Z0-9_.])@([a-zA-Z][a-zA-Z0-9_]*)`) +// Supports Unicode letters and hyphens in names (e.g. @José, @Mary-Jane). +var mentionRegex = regexp.MustCompile(`(?:^|[^-\p{L}\p{N}_.])@([\p{L}][\p{L}\p{N}_-]*)`) -// promptItemRegex parses tags from the /prompts/users HTML. -var promptItemRegex = regexp.MustCompile(`]*>`) +// promptItemRegex matches opening tags. +// Attributes are extracted separately to handle any order. +var promptItemRegex = regexp.MustCompile(`]*>`) -// avatarRegex extracts the src attribute from the first in editor template. +// searchAttrRegex extracts the search attribute value. +var searchAttrRegex = regexp.MustCompile(`\ssearch="([^"]+)"`) + +// sgidAttrRegex extracts the sgid attribute value. +var sgidAttrRegex = regexp.MustCompile(`\ssgid="([^"]+)"`) + +// promptItemEndRegex matches the closing tag for a prompt item block. +var promptItemEndRegex = regexp.MustCompile(``) + +// avatarRegex extracts the src attribute from the first tag. var avatarRegex = regexp.MustCompile(`]+src="([^"]+)"`) +// codeBlockRegex matches fenced code blocks (``` ... ```). +var codeBlockRegex = regexp.MustCompile("(?s)```.*?```") + +// codeSpanRegex matches inline code spans (` ... `). +var codeSpanRegex = regexp.MustCompile("`[^`]+`") + // resolveMentions scans text for @FirstName patterns and replaces them with // ActionText mention HTML. If the text contains no @ characters, it is returned // unchanged. On any error fetching users, the original text is returned with a // warning printed to stderr. +// +// Mentions inside markdown code spans (`@name`) and fenced code blocks are not +// resolved, preserving the user's intended literal text. func resolveMentions(text string, c client.API) string { if !strings.Contains(text, "@") { return text @@ -65,6 +86,18 @@ func resolveMentions(text string, c client.API) string { return text } + // Protect code blocks and code spans from mention resolution by replacing + // them with placeholders, resolving mentions, then restoring the originals. + var codeChunks []string + placeholder := func(s string) string { + idx := len(codeChunks) + codeChunks = append(codeChunks, s) + return fmt.Sprintf("\x00CODE%d\x00", idx) + } + + protected := codeBlockRegex.ReplaceAllStringFunc(text, placeholder) + protected = codeSpanRegex.ReplaceAllStringFunc(protected, placeholder) + // Find all @Name matches with positions type mentionMatch struct { start int // start of @Name (the @ character) @@ -72,7 +105,7 @@ func resolveMentions(text string, c client.API) string { name string } - allMatches := mentionRegex.FindAllStringSubmatchIndex(text, -1) + allMatches := mentionRegex.FindAllStringSubmatchIndex(protected, -1) var matches []mentionMatch for _, loc := range allMatches { // loc[2]:loc[3] is the capture group (the name without @) @@ -80,7 +113,7 @@ func resolveMentions(text string, c client.API) string { nameEnd := loc[3] // The @ is one character before the name atStart := nameStart - 1 - name := text[nameStart:nameEnd] + name := protected[nameStart:nameEnd] matches = append(matches, mentionMatch{start: atStart, end: nameEnd, name: name}) } @@ -98,8 +131,8 @@ func resolveMentions(text string, c client.API) string { switch len(found) { case 1: - html := buildMentionHTML(found[0]) - text = text[:m.start] + html + text[m.end:] + mentionHTML := buildMentionHTML(found[0]) + protected = protected[:m.start] + mentionHTML + protected[m.end:] case 0: fmt.Fprintf(os.Stderr, "Warning: could not resolve mention @%s\n", m.name) default: @@ -111,7 +144,12 @@ func resolveMentions(text string, c client.API) string { } } - return text + // Restore code blocks and spans + for i, chunk := range codeChunks { + protected = strings.Replace(protected, fmt.Sprintf("\x00CODE%d\x00", i), chunk, 1) + } + + return protected } // fetchMentionUsers fetches the list of mentionable users from the API. @@ -129,17 +167,29 @@ func fetchMentionUsers(c client.API) ([]mentionUser, error) { // parseMentionUsers extracts mentionable users from the /prompts/users HTML. // Each user is represented as a element with search and sgid // attributes, containing tags with avatar URLs. -func parseMentionUsers(html []byte) []mentionUser { - htmlStr := string(html) - items := promptItemRegex.FindAllStringSubmatch(htmlStr, -1) +func parseMentionUsers(htmlBytes []byte) []mentionUser { + htmlStr := string(htmlBytes) + items := promptItemRegex.FindAllStringIndex(htmlStr, -1) if len(items) == 0 { return nil } + // Find all closing tags for scoping avatar lookups + endIndices := promptItemEndRegex.FindAllStringIndex(htmlStr, -1) + var users []mentionUser - for _, item := range items { - search := strings.TrimSpace(item[1]) - sgid := item[2] + for itemIdx, loc := range items { + tag := htmlStr[loc[0]:loc[1]] + + // Extract search and sgid attributes (order-independent) + searchMatch := searchAttrRegex.FindStringSubmatch(tag) + sgidMatch := sgidAttrRegex.FindStringSubmatch(tag) + if searchMatch == nil || sgidMatch == nil { + continue + } + + search := strings.TrimSpace(searchMatch[1]) + sgid := sgidMatch[1] if search == "" || sgid == "" { continue @@ -161,15 +211,16 @@ func parseMentionUsers(html []byte) []mentionUser { fullName := strings.Join(words, " ") firstName := words[0] - // Extract avatar URL: find the after this sgid in the HTML. + // Extract avatar URL scoped to this prompt-item block only. avatarSrc := "" - sgidIdx := strings.Index(htmlStr, `sgid="`+sgid+`"`) - if sgidIdx >= 0 { - // Search for the first after the sgid - remainder := htmlStr[sgidIdx:] - if m := avatarRegex.FindStringSubmatch(remainder); len(m) > 1 { - avatarSrc = m[1] - } + blockStart := loc[0] + blockEnd := len(htmlStr) + if itemIdx < len(endIndices) { + blockEnd = endIndices[itemIdx][1] + } + block := htmlStr[blockStart:blockEnd] + if m := avatarRegex.FindStringSubmatch(block); len(m) > 1 { + avatarSrc = m[1] } users = append(users, mentionUser{ @@ -184,12 +235,16 @@ func parseMentionUsers(html []byte) []mentionUser { } // buildMentionHTML creates the ActionText attachment HTML for a mention. +// Values are HTML-escaped to prevent injection from user-controlled names. func buildMentionHTML(u mentionUser) string { return fmt.Sprintf( ``+ `%s`+ ``, - u.SGID, u.FullName, u.AvatarSrc, u.FirstName, + html.EscapeString(u.SGID), + html.EscapeString(u.FullName), + html.EscapeString(u.AvatarSrc), + html.EscapeString(u.FirstName), ) } diff --git a/internal/commands/mentions_test.go b/internal/commands/mentions_test.go index 9bf4a78..98e092f 100644 --- a/internal/commands/mentions_test.go +++ b/internal/commands/mentions_test.go @@ -41,6 +41,22 @@ const samplePromptUsersHTML = ` ` +// ambiguousHTML has two users with the same first name. +const ambiguousHTML = ` + + + + + + +` + func newMentionMockClient() *MockClient { m := NewMockClient() m.GetHTMLResponse = &client.APIResponse{ @@ -92,6 +108,40 @@ func TestParseMentionUsersEmpty(t *testing.T) { } } +func TestParseMentionUsersAttributeOrder(t *testing.T) { + // sgid before search — should still parse correctly + html := ` + +` + users := parseMentionUsers([]byte(html)) + if len(users) != 1 { + t.Fatalf("expected 1 user from reversed attributes, got %d", len(users)) + } + if users[0].FirstName != "Test" { + t.Errorf("FirstName = %q, want %q", users[0].FirstName, "Test") + } + if users[0].SGID != "reversed-sgid" { + t.Errorf("SGID = %q, want %q", users[0].SGID, "reversed-sgid") + } +} + +func TestParseMentionUsersAvatarScoping(t *testing.T) { + // Each user's avatar should come from their own block, not a later one + users := parseMentionUsers([]byte(samplePromptUsersHTML)) + if len(users) < 2 { + t.Fatal("expected at least 2 users") + } + if !strings.Contains(users[0].AvatarSrc, "u1") { + t.Errorf("user[0] avatar should contain u1, got %q", users[0].AvatarSrc) + } + if !strings.Contains(users[1].AvatarSrc, "u2") { + t.Errorf("user[1] avatar should contain u2, got %q", users[1].AvatarSrc) + } +} + func TestResolveMentions(t *testing.T) { tests := []struct { name string @@ -147,6 +197,23 @@ func TestResolveMentions(t *testing.T) { input: "Hey @Kennedy", shouldContain: []string{`sgid="kennedy-sgid-789"`, `title="Kennedy"`}, }, + { + name: "mention inside inline code not resolved", + input: "Use `@Wayne` to mention someone", + shouldContain: []string{"`@Wayne`"}, + shouldNotMatch: []string{"action-text-attachment"}, + }, + { + name: "mention inside fenced code block not resolved", + input: "Example:\n```\n@Wayne check this\n```", + shouldContain: []string{"@Wayne"}, + shouldNotMatch: []string{"action-text-attachment"}, + }, + { + name: "mention outside code resolved while code preserved", + input: "@Wayne see `@Bushra` example", + shouldContain: []string{`sgid="wayne-sgid-123"`, "`@Bushra`"}, + }, } for _, tt := range tests { @@ -169,6 +236,36 @@ func TestResolveMentions(t *testing.T) { } } +func TestResolveMentionsNoFetchWithoutAt(t *testing.T) { + resetMentionCache() + mock := newMentionMockClient() + + resolveMentions("Hello world, no mentions here", mock) + + if len(mock.GetHTMLCalls) != 0 { + t.Errorf("expected 0 GetHTML calls for text without @, got %d", len(mock.GetHTMLCalls)) + } +} + +func TestResolveMentionsAmbiguous(t *testing.T) { + resetMentionCache() + mock := NewMockClient() + mock.GetHTMLResponse = &client.APIResponse{ + StatusCode: 200, + Body: []byte(ambiguousHTML), + } + + result := resolveMentions("Hey @Alex check this", mock) + + // Should NOT resolve — ambiguous + if strings.Contains(result, "action-text-attachment") { + t.Errorf("ambiguous mention should not resolve, got: %s", result) + } + if !strings.Contains(result, "@Alex") { + t.Errorf("ambiguous mention should stay as @Alex, got: %s", result) + } +} + func TestResolveMentionsAPIError(t *testing.T) { resetMentionCache() mock := NewMockClient() @@ -198,3 +295,23 @@ func TestResolveMentionsCaching(t *testing.T) { t.Errorf("expected still 1 GetHTML call (cached), got %d", len(mock.GetHTMLCalls)) } } + +func TestBuildMentionHTMLEscaping(t *testing.T) { + u := mentionUser{ + FirstName: `O'Brien`, + FullName: `O'Brien