-
Notifications
You must be signed in to change notification settings - Fork 21
Add @mention resolution for comments and descriptions #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
319e339
c0b727e
dc96a14
f84f9d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,262 @@ | ||||||||||||||||||||||||||||||
| package commands | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||
| "html" | ||||||||||||||||||||||||||||||
| "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). | ||||||||||||||||||||||||||||||
| // 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 matches opening <lexxy-prompt-item> tags. | ||||||||||||||||||||||||||||||
| // Attributes are extracted separately to handle any order. | ||||||||||||||||||||||||||||||
| var promptItemRegex = regexp.MustCompile(`<lexxy-prompt-item\s[^>]*>`) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // 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(`</lexxy-prompt-item>`) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // avatarRegex extracts the src attribute from the first <img> tag. | ||||||||||||||||||||||||||||||
| var avatarRegex = regexp.MustCompile(`<img[^>]+src="([^"]+)"`) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // codeBlockRegex matches fenced code blocks (``` ... ```). | ||||||||||||||||||||||||||||||
| var codeBlockRegex = regexp.MustCompile("(?s)```.*?```") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // codeSpanRegex matches inline code spans (` ... `). | ||||||||||||||||||||||||||||||
| var codeSpanRegex = regexp.MustCompile("`[^`]+`") | ||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+62
|
||||||||||||||||||||||||||||||
| // codeBlockRegex matches fenced code blocks (``` ... ```). | |
| var codeBlockRegex = regexp.MustCompile("(?s)```.*?```") | |
| // codeSpanRegex matches inline code spans (` ... `). | |
| var codeSpanRegex = regexp.MustCompile("`[^`]+`") | |
| // codeBlockRegex matches fenced code blocks with backticks or tildes, | |
| // including variable-length fences (e.g. ``` ... ```, ```` ... ````, ~~~ ... ~~~). | |
| // The same fence character and length must be used to open and close the block. | |
| var codeBlockRegex = regexp.MustCompile(`(?s)(` + "`" + `{3,}|~{3,}).*?\1`) | |
| // codeSpanRegex matches inline code spans delimited by one or more backticks | |
| // (e.g. `code`, ``code``, ```code```), preventing mentions inside them | |
| // from being resolved. | |
| var codeSpanRegex = regexp.MustCompile(`(` + "`" + `+)[^` + "`" + `]*?\1`) |
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolveMentions runs on the raw markdown input before markdownToHTML(), so it will also rewrite @Name occurrences inside markdown code spans / fenced code blocks (e.g., @Sarah), changing what the user intended to show as literal code into an escaped <action-text-attachment ...> snippet. Add logic to skip mention resolution inside markdown code (or move mention resolution to operate on the post-markdown HTML while ignoring /
content), and add a unit test for the backtick/code-fence case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in c0b727e. Code spans and fenced code blocks are now replaced with null-byte placeholders before mention resolution runs, then restored afterward. This prevents @name inside backticks or code fences from being transformed. Three new test cases cover this.
Copilot
AI
Mar 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildMentionHTML interpolates SGID, FullName, AvatarSrc, and FirstName directly into HTML attributes / text without escaping. User names can legally contain characters like quotes or '<', which can break the generated markup or introduce HTML injection in the rich-text body. Use proper HTML escaping for attribute values and inner text (e.g., html.EscapeString) before formatting the attachment HTML, and add a test covering a name containing quotes or '<' to prevent regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in c0b727e. All user-controlled values (SGID, FullName, AvatarSrc, FirstName) are now escaped with html.EscapeString() before interpolation. Added TestBuildMentionHTMLEscaping covering names with quotes and <script> tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetHTML()is a new request path with important behavior differences fromGet()(differentAcceptheader and no JSON parsing). Sinceinternal/clienthas extensive request/response tests already, it would be good to add a focused unit test forGetHTML()to prevent regressions (e.g., assert the server seesAccept: text/html, the raw body is returned, and non-2xx responses are surfaced viaerrorFromResponse).