Skip to content

fix(cli): resolve @mentions wrapped in markdown emphasis and skip code regions #2547

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.

Sign up for GitHub

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

Open
bosoud wants to merge 1 commit into
base: main
Choose a base branch
from
+222 −31
Open

fix(cli): resolve @mentions wrapped in markdown emphasis and skip code regions #2547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 71 additions & 9 deletions crates/buzz-cli/src/commands/messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::validate::{
validate_content_size, validate_hex64, validate_uuid, MAX_DIFF_BYTES,
};
use buzz_sdk::mentions::{
extract_at_mentions_with_known, extract_nostr_uris, merge_mentions, strip_code_regions,
MENTION_CAP,
extract_at_mentions_with_known, extract_at_names, extract_nostr_uris, merge_mentions,
strip_code_regions, MENTION_CAP,
};

/// Extract the thread root event ID from a Nostr tag array.
Expand Down Expand Up @@ -121,10 +121,12 @@ async fn resolve_channel_id(client: &BuzzClient, event_id: &str) -> Result<Uuid,

/// Resolve `@name` mentions in `content` against this channel's members.
///
/// Queries kind 39002 (channel members) then kind 0 (profiles), parses
/// display names once, and feeds them to [`extract_at_mentions_with_known`]
/// for multi-word matching. On any I/O or parse failure, returns an empty
/// vec — auto-tagging is best-effort and must never block a send.
/// Strips code regions first — an `@name` inside a fenced block or backtick
/// span must not notify anyone. Queries kind 39002 (channel members) then
/// kind 0 (profiles), parses display names once, and feeds them to
/// [`extract_at_mentions_with_known`] for multi-word matching. On any I/O or
/// parse failure, returns an empty vec — auto-tagging is best-effort and
/// must never block a send.
async fn resolve_content_mentions(
client: &BuzzClient,
channel_id: &str,
Expand All @@ -133,6 +135,10 @@ async fn resolve_content_mentions(
if !content.contains('@') {
return vec![];
}
let scannable = strip_code_regions(content);
if !scannable.contains('@') {
return vec![];
}

// 1. Membership list (kind 39002 is parameterized-replaceable, addressed by `d` tag).
let members_filter = serde_json::json!({
Expand Down Expand Up @@ -188,7 +194,7 @@ async fn resolve_content_mentions(

// 4. Two-pass extraction: known multi-word names first, single-word fallback.
let known_refs: Vec<&str> = display_names.iter().map(|s| s.as_str()).collect();
let names = extract_at_mentions_with_known(content, &known_refs);
let names = extract_at_mentions_with_known(&scannable, &known_refs);

// 5. Look up matched names → pubkeys via the map we already built.
names
Expand All @@ -198,6 +204,17 @@ async fn resolve_content_mentions(
.collect()
}

/// True when code-stripped content carries `@mention` candidates but zero
/// pubkeys were resolved — the silent-failure case of issue #2526, where a
/// send exits 0 and nobody is notified.
///
/// `scannable` must already have code regions stripped, so `@names` inside
/// code samples don't trigger a spurious warning. Email addresses never
/// count as candidates (see [`extract_at_names`]).
fn mentions_went_unresolved(scannable: &str, resolved: &[String]) -> bool {
resolved.is_empty() && !extract_at_names(scannable).is_empty()
}

/// Fetch raw events for `filter` via the relay's `/query` endpoint.
/// Returns `None` on any I/O or parse failure.
async fn fetch_events(
Expand Down Expand Up @@ -535,6 +552,13 @@ pub async fn cmd_send_message(
let uri_pubkeys = extract_nostr_uris(&stripped);
merge_mentions(&mut auto_resolved, &uri_pubkeys, MENTION_CAP);

if mentions_went_unresolved(&stripped, &auto_resolved) {
eprintln!(
"warning: content contains @mentions but none resolved to a channel member; \
no one will be notified"
);
}

let mention_refs: Vec<&str> = auto_resolved.iter().map(|s| s.as_str()).collect();

let builder = match p.kind {
Expand Down Expand Up @@ -876,9 +900,12 @@ pub async fn dispatch(

#[cfg(test)]
mod tests {
use super::{find_root_from_tags, match_profiles_by_name, parse_member_pubkeys};
use super::{
find_root_from_tags, match_profiles_by_name, mentions_went_unresolved, parse_member_pubkeys,
};
use buzz_sdk::mentions::{
extract_at_mentions_with_known, extract_at_names, match_names_to_profiles, MentionProfile,
extract_at_mentions_with_known, extract_at_names, match_names_to_profiles,
strip_code_regions, MentionProfile,
};
use serde_json::json;

Expand Down Expand Up @@ -1063,6 +1090,41 @@ mod tests {
assert!(names.is_empty());
}

#[test]
fn cli_pipeline_resolves_bold_wrapped_mention() {
// Regression: issue #2526 — `**@Atlas**` through the send pipeline.
let known_refs = vec!["Atlas"];
let scannable = strip_code_regions("**@Atlas** your fix is aimed at the wrong layer");
let names = extract_at_mentions_with_known(&scannable, &known_refs);
assert_eq!(names, vec!["atlas"]);
}

#[test]
fn cli_pipeline_skips_mentions_inside_code() {
// The false-positive mirror of #2526: an `@name` in a code sample
// must not notify. resolve_content_mentions strips before extracting.
let scannable = strip_code_regions("try this:\n```\ngit blame @alice\n```\ndone");
assert!(extract_at_mentions_with_known(&scannable, &["Alice"]).is_empty());
}

#[test]
fn unresolved_warning_fires_on_candidates_with_no_matches() {
assert!(mentions_went_unresolved("**@Atlas** ping", &[]));
assert!(mentions_went_unresolved("@nobody-here hello", &[]));
}

#[test]
fn unresolved_warning_silent_when_something_resolved() {
assert!(!mentions_went_unresolved("@alice hi", &["pk1".to_string()]));
}

#[test]
fn unresolved_warning_silent_without_candidates() {
assert!(!mentions_went_unresolved("no mentions here", &[]));
assert!(!mentions_went_unresolved("mail user@example.com", &[]));
assert!(!mentions_went_unresolved("meet @ 5pm", &[]));
}

#[test]
fn parse_member_pubkeys_ignores_non_p_tags() {
let event = json!({
Expand Down
173 changes: 151 additions & 22 deletions crates/buzz-sdk/src/mentions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,21 @@
//! ## Pipeline
//!
//! ```text
//! body text ──► extract_at_names ──► names: Vec<String>
//! │
//! members + profiles (queried by caller) │
//! ▼
//! match_names_to_profiles ──► pubkeys
//! │
//! body text ──► strip_code_regions ──► extract_nostr_uris ─┤
//! ▼
//! explicit mentions ──► normalize ──► merge_mentions ──► p-tags
//! body text ──► strip_code_regions ──► extract_at_names ──► names: Vec<String>
//! │ │
//! │ members + profiles (queried by caller)│
//! │ ▼
//! │ match_names_to_profiles ──► pubkeys
//! │ │
//! └──────────────────► extract_nostr_uris ────────────┤
//! ▼
//! explicit mentions ──► normalize ──► merge_mentions ──► p-tags
//! ```
//!
//! Code regions (fenced blocks and inline spans) are stripped before **both**
//! `@name` and NIP-27 URI extraction, so mentions inside code samples never
//! produce notifications.
//!
//! When the set of known member names is available upfront,
//! [`extract_at_mentions_with_known`] replaces the first step to correctly
//! handle multi-word display names.
Expand Down Expand Up @@ -50,14 +54,47 @@ pub struct MentionProfile<'a> {
pub content_json: &'a str,
}

/// Shared leading-delimiter definition for `@mention` starts.
///
/// `prev` is the character immediately before the `@`; `prev2` is the one
/// before that (consulted only for the two-character spoiler delimiter
/// `||`). A mention may start at start-of-string, after ASCII whitespace,
/// after an opening parenthesis, after markdown emphasis markers (`*`, `_`),
/// or after a spoiler delimiter (`||`). Anything else — most importantly a
/// word character, which excludes email addresses like `user@host` — is not
/// a mention start.
///
/// This mirrors the leading group of the Desktop parser's regex in
/// `desktop/src/features/messages/lib/hasMention.ts`. The two surfaces must
/// agree on what counts as a mention; see issue #2526 for what happens when
/// they drift.
fn is_mention_lead(prev: Option<char>, prev2: Option<char>) -> bool {
match prev {
None => true,
Some(c) if c.is_ascii_whitespace() => true,
Some('(' | '*' | '_') => true,
Some('|') => prev2 == Some('|'),
_ => false,
}
}

/// [`is_mention_lead`] for an `@` at byte offset `at` of `content`.
fn is_mention_lead_at(content: &str, at: usize) -> bool {
let mut before = content[..at].chars().rev();
let prev = before.next();
is_mention_lead(prev, before.next())
}

/// Extract single-word `@mention` names from message content.
///
/// Prefer [`extract_at_mentions_with_known`] when known member names are
/// available — it correctly handles multi-word display names.
///
/// Returns lowercased names found after `@` tokens. An `@name` only matches
/// when the `@` is at start-of-string or preceded by an ASCII whitespace
/// character — this excludes things like email addresses (`user@host`).
/// when the `@` is preceded by a mention-leading delimiter (start-of-string,
/// ASCII whitespace, `(`, markdown emphasis `*`/`_`, or spoiler `||` — see
/// [`is_mention_lead`]) — this excludes things like email addresses
/// (`user@host`).
///
/// Allowed name characters: ASCII alphanumerics, `.`, `-`, `_`.
/// Duplicates are removed; first-seen order is preserved.
Expand All @@ -72,8 +109,11 @@ pub fn extract_at_names(content: &str) -> Vec<String> {
let mut i = 0;
while i < len {
if chars[i] == '@' {
let preceded_by_ws = i == 0 || chars[i - 1].is_ascii_whitespace();
if preceded_by_ws && i + 1 < len {
let preceded = is_mention_lead(
i.checked_sub(1).map(|j| chars[j]),
i.checked_sub(2).map(|j| chars[j]),
);
if preceded && i + 1 < len {
let start = i + 1;
let mut end = start;
while end < len {
Expand All @@ -100,10 +140,11 @@ pub fn extract_at_names(content: &str) -> Vec<String> {

/// Extract `@mention` names from message content using known member names.
///
/// At each `@` preceded by whitespace or start-of-string, tries known names
/// longest-first (case-insensitive, word-boundary-checked), then falls back
/// to single-word tokenization. Returns lowercased names in first-seen order,
/// deduplicated. Empty/whitespace-only entries in `known_names` are ignored.
/// At each `@` preceded by a mention-leading delimiter (see
/// [`is_mention_lead`]), tries known names longest-first (case-insensitive,
/// word-boundary-checked), then falls back to single-word tokenization.
/// Returns lowercased names in first-seen order, deduplicated.
/// Empty/whitespace-only entries in `known_names` are ignored.
pub fn extract_at_mentions_with_known(content: &str, known_names: &[&str]) -> Vec<String> {
if content.is_empty() || !content.contains('@') {
return vec![];
Expand All @@ -120,8 +161,7 @@ pub fn extract_at_mentions_with_known(content: &str, known_names: &[&str]) -> Ve
let mut seen = HashSet::new();

for (i, _) in content.match_indices('@') {
let preceded = i == 0 || content.as_bytes()[i - 1].is_ascii_whitespace();
if !preceded {
if !is_mention_lead_at(content, i) {
continue;
}
let rest = &content[i + 1..];
Expand Down Expand Up @@ -151,10 +191,21 @@ pub fn extract_at_mentions_with_known(content: &str, known_names: &[&str]) -> Ve
names
}

/// True when `s` starts at a word boundary for a matched known name.
///
/// Accepts end-of-string, ASCII whitespace, closing punctuation, trailing
/// markdown emphasis markers (`*`, `_`), and the spoiler delimiter (`||`) —
/// the counterpart of [`is_mention_lead`], mirroring the trailing lookahead
/// of the Desktop parser in `desktop/src/features/messages/lib/hasMention.ts`.
fn is_word_boundary(s: &str) -> bool {
s.chars().next().is_none_or(|c| {
c.is_ascii_whitespace() || matches!(c, ',' | ';' | '.' | '!' | '?' | ':' | ')' | ']' | '}')
})
let mut chars = s.chars();
match chars.next() {
None => true,
Some(c) if c.is_ascii_whitespace() => true,
Some(',' | ';' | '.' | '!' | '?' | ':' | ')' | ']' | '}' | '*' | '_') => true,
Some('|') => chars.next() == Some('|'),
_ => false,
}
}

/// Match extracted `@names` against channel-member profiles.
Expand Down Expand Up @@ -417,6 +468,36 @@ mod tests {
);
}

#[test]
fn extract_at_names_accepts_markdown_emphasis_lead() {
// Regression: issue #2526 — `**@Atlas**` sent zero p tags.
assert_eq!(extract_at_names("**@alice** please look"), vec!["alice"]);
assert_eq!(extract_at_names("*@bob* ping"), vec!["bob"]);
assert_eq!(extract_at_names("(@dave)"), vec!["dave"]);
assert_eq!(extract_at_names("||@eve|| spoiler"), vec!["eve"]);
}

#[test]
fn extract_at_names_underscore_lead_glues_trailing_underscore() {
// `_` is both an emphasis marker and a valid name character. The
// single-word tokenizer accepts the lead but swallows the closing
// marker into the name; only the known-names path resolves
// `_@carol_` to "carol". Pinned so the trade-off stays deliberate.
assert_eq!(extract_at_names("_@carol_ hi"), vec!["carol_"]);
}

#[test]
fn extract_at_names_single_pipe_is_not_a_lead() {
assert!(extract_at_names("a|@alice").is_empty());
// Table cells are fine — the pipe is followed by whitespace.
assert_eq!(extract_at_names("| @alice |"), vec!["alice"]);
}

#[test]
fn extract_at_names_email_rejected_even_inside_emphasis() {
assert!(extract_at_names("**user@example.com**").is_empty());
}

#[test]
fn extract_at_names_rejects_email_and_empty() {
assert!(extract_at_names("").is_empty());
Expand Down Expand Up @@ -498,6 +579,54 @@ mod tests {
assert_eq!(result, vec!["will pfleger"]);
}

#[test]
fn known_name_survives_bold_wrapping() {
// Regression: issue #2526 — the production failure was exactly this.
let result = extract_at_mentions_with_known("**@Atlas** your fix is aimed", &["Atlas"]);
assert_eq!(result, vec!["atlas"]);
}

#[test]
fn known_name_survives_italics_and_spoilers() {
assert_eq!(
extract_at_mentions_with_known("_@Atlas_ take a look", &["Atlas"]),
vec!["atlas"]
);
assert_eq!(
extract_at_mentions_with_known("*@Atlas* take a look", &["Atlas"]),
vec!["atlas"]
);
assert_eq!(
extract_at_mentions_with_known("||@Atlas|| knows", &["Atlas"]),
vec!["atlas"]
);
}

#[test]
fn known_multiword_name_survives_trailing_emphasis() {
// Trailing `**` used to fail the boundary check, dropping to the
// single-word tokenizer which emits "will" — matching nobody.
let result =
extract_at_mentions_with_known("ping **@Will Pfleger** now", &["Will Pfleger"]);
assert_eq!(result, vec!["will pfleger"]);
}

#[test]
fn single_pipe_is_not_a_boundary_for_known_name() {
// A lone `|` is neither a lead nor a boundary (the spoiler delimiter
// is two pipes) — the known match fails and fallback emits "will".
let result = extract_at_mentions_with_known("@Will Pfleger|x", &["Will Pfleger"]);
assert_eq!(result, vec!["will"]);
}

#[test]
fn at_names_inside_code_regions_not_extracted_after_strip() {
let content = "run `git blame @alice` then\n```\n@bob in code\n```\nthanks";
let stripped = strip_code_regions(content);
assert!(extract_at_names(&stripped).is_empty());
assert!(extract_at_mentions_with_known(&stripped, &["alice", "bob"]).is_empty());
}

#[test]
fn email_address_not_matched() {
let result = extract_at_mentions_with_known("user@example.com", &["example.com"]);
Expand Down