diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index 290cc59fa8..af182a5227 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -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. @@ -121,10 +121,12 @@ async fn resolve_channel_id(client: &BuzzClient, event_id: &str) -> Result = 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 @@ -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( @@ -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 { @@ -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; @@ -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!({ diff --git a/crates/buzz-sdk/src/mentions.rs b/crates/buzz-sdk/src/mentions.rs index e59580c7ae..ea608963b4 100644 --- a/crates/buzz-sdk/src/mentions.rs +++ b/crates/buzz-sdk/src/mentions.rs @@ -7,17 +7,21 @@ //! ## Pipeline //! //! ```text -//! body text ──► extract_at_names ──► names: Vec -//! │ -//! 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 +//! │ │ +//! │ 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. @@ -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, prev2: Option) -> 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. @@ -72,8 +109,11 @@ pub fn extract_at_names(content: &str) -> Vec { 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 { @@ -100,10 +140,11 @@ pub fn extract_at_names(content: &str) -> Vec { /// 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 { if content.is_empty() || !content.contains('@') { return vec![]; @@ -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..]; @@ -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. @@ -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()); @@ -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"]);