From 2c29dbe290186af6d0e71b72d5587c1dd9afd0a3 Mon Sep 17 00:00:00 2001 From: cyberzero000 Date: Sat, 22 Aug 2026 11:48:10 -0700 Subject: [PATCH 1/3] feat(backend): say what a reply's p tags mean, and read it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A NIP-10 reply should `p`-tag the author it answers — `require_mention` subscriptions add `#p` to the relay-side REQ filter (`crates/buzz-acp/src/relay.rs`), so without that tag the relay never transmits the event and an agent replied to in a thread hears nothing. But as a bare tag, `["p", ]` added because a reply addresses you is byte-identical to one added because someone typed `@you`, and the two must behave differently: a mention pierces a channel or thread mute and raises dock-badge priority, being replied to does not. Without a marker a receiver had to fetch the parent message and check who wrote it — a relay round trip to recover something the sender knew for free, plus the caching, chunking, retry and fail-open handling that round trip needs. Replies now mark each `p` tag with the role it plays, in the fourth position, the way `e` tags already carry `root` and `reply`: ["p", , "", "mention"] someone typed as @name ["p", , "", "reply"] the author being answered ["p", ] addressed by the channel The third shape is what a DM needs: a DM tags every other participant whether or not anyone typed their names, so neither marker is true of those tags, and claiming `mention` would let a DM thread reply pierce a mute and take a slot in the mention feed ahead of a real `@you`. They stay bare, which under the read rule below means "ask the parent" — the answer they already got. Four properties make this safe to land without coordinating clients: - Read one-way. A marker that is present is authoritative. An absent marker means "ask the parent", never "this is a mention". Senders that predate the markers keep working exactly as before, and there is no flag day. - `#p` delivery is untouched. Relay tag filters compare only a tag's second element (`crates/buzz-core/src/filter.rs`), so a marker cannot affect which agents receive the event. - Top-level messages stay bare. Without a parent there is nothing to disambiguate, and a `p` tag there can only be a mention. - The undecidable case is settled. When you are both the author being answered and typed in the body, one tag cannot be marked and unmarked at once. The sender emits the mention marker and mention wins — the answer that preserves the stronger signal, and one no amount of parent-fetching could reach. Emitted by the Tauri backend, and by `buzz-sdk` for the CLI and the ACP harness. Typed mentions and channel-addressed recipients travel as a named pair (`Recipients { typed, addressed }`) rather than two adjacent same-typed lists — conflating them is the exact mistake the markers exist to prevent. Read by the native catch-up scan and the home feed. `p_tag_role` is the shared vocabulary; `unread_notify` holds the gates, ported from `shouldNotify.ts` function for function, and `unread_parent_authors` resolves a parent only when no marker answers the question. A marked reply costs no round trip. Two contracts became load-bearing. `ThreadRef.parent_author` must be the author of `parent_event_id`, not of the thread root, because receivers read the addressing marker as "this answers a message you wrote". And the self-null that suppresses the addressing tag on a self-reply is judged against the *signing* key: a managed agent posts under its own key, so comparing against the desktop owner's would strip the tag off a reply to the owner. The backend is the source of the addressing tag rather than the frontend, which closes a gap in the delivery fix itself. `resolve_thread_ref` already fetches the parent event on its way to the thread root, so `parent.pubkey` costs no extra query and is strictly more reliable than the frontend cache the tag was previously read from — that cache silently missed for any channel not opened in the current session, which would have shipped the reply with no addressing tag at all. High-priority classification fails closed on an unresolved parent. The flag is persisted and drops the channel's top-level items from the dock badge, so guessing "high" after a relay flap would silently hide an approval request until the channel is read. Event ids are validated and case-normalized at the filter boundary. Any member can publish an event whose `e` tag is not a 64-char hex id — the relay's NIP-10 resolver ignores such a tag rather than rejecting the event. Put into an `ids` REQ filter the relay answers a bare NOTICE, and this client only resolves `rate-limited:` notices, so the request hangs the full 25s history timeout. One such event in a channel made every parent lookup there fail, leaving the channel with no badge, no unread events and no thread activity for the session, and it did not clear on restart. `commands/feed.rs` is `get_feed` moved out of `commands/messages.rs`, which main leaves at 988 lines against the 1000-line ratchet — adding the role check trips the guard, and AGENTS.md says split rather than raise. `events/message_tags.rs` and `events/identity_archive.rs` are the same story for `events.rs`, and `unread_notify.rs` for `unread_catch_up.rs`. `NOSTR.md`'s NIP-10 row and its `nak` reply example now describe the markers, since that document is what third-party clients read and it previously taught them to emit an unmarked reply. Signed-off-by: cyberzero000 --- NOSTR.md | 5 +- crates/buzz-acp/src/pool.rs | 3 + crates/buzz-acp/src/setup_mode.rs | 104 +++-- crates/buzz-cli/src/commands/messages.rs | 90 +++- crates/buzz-sdk/src/builders.rs | 104 ++++- crates/buzz-sdk/src/lib.rs | 7 + .../agent_discovery/relay_directory.rs | 6 +- desktop/src-tauri/src/commands/feed.rs | 411 ++++++++++++++++++ desktop/src-tauri/src/commands/messages.rs | 201 +++------ .../src-tauri/src/commands/messages/forum.rs | 6 +- .../src/commands/messages/thread_ref.rs | 26 +- desktop/src-tauri/src/commands/mod.rs | 2 + desktop/src-tauri/src/egress_guard_tests.rs | 4 +- desktop/src-tauri/src/events.rs | 193 ++------ .../src-tauri/src/events/identity_archive.rs | 129 ++++++ desktop/src-tauri/src/events/message_tags.rs | 335 +++++++++++++- desktop/src-tauri/src/huddle/pipeline.rs | 6 +- desktop/src-tauri/src/lib.rs | 3 + desktop/src-tauri/src/models.rs | 6 + desktop/src-tauri/src/p_tag_role.rs | 122 ++++++ desktop/src-tauri/src/unread_catch_up.rs | 319 +++++++++++--- desktop/src-tauri/src/unread_notify.rs | 282 ++++++++++++ .../src-tauri/src/unread_parent_authors.rs | 218 ++++++++++ 23 files changed, 2162 insertions(+), 420 deletions(-) create mode 100644 desktop/src-tauri/src/commands/feed.rs create mode 100644 desktop/src-tauri/src/events/identity_archive.rs create mode 100644 desktop/src-tauri/src/p_tag_role.rs create mode 100644 desktop/src-tauri/src/unread_notify.rs create mode 100644 desktop/src-tauri/src/unread_parent_authors.rs diff --git a/NOSTR.md b/NOSTR.md index cce70f2f77f..440c016404d 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -67,7 +67,7 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **NIP-11 relay info** | ✅ | `GET /` with `Accept: application/nostr+json` | | **Blossom media** | ✅ | `PUT /media/upload` (BUD-02), `GET /media/{sha256}.{ext}` (BUD-01) | | **NIP-50 search** | ✅ | One-shot search REQs: `{"search":"query","kinds":[9],"#h":[""]}` → relevance-sorted results → EOSE. Not registered as persistent subscriptions. | -| **NIP-10 threads** | ✅ | WS-submitted replies with `["e","","","reply"]` tags create `thread_metadata` atomically. Visible in REST thread queries. Unknown parents rejected. | +| **NIP-10 threads** | ✅ | WS-submitted replies with `["e","","","reply"]` tags create `thread_metadata` atomically. Visible in REST thread queries. Unknown parents rejected. A reply should also mark why each `p` tag is there: `["p","","","reply"]` for the author being answered and `["p","","","mention"]` for someone typed as `@name`. Both are accepted bare, but an unmarked `p` tag is byte-identical to a typed mention, so the recipient has to fetch the parent to tell them apart — and where that lookup cannot answer, the tag reads as a mention, piercing their mute. | | **NIP-17 DMs (gift wrap)** | ✅ | kind:1059 accepted with ephemeral signing keys. Stored community-globally (`channel_id=None` inside the connected community). Delivered via `#p`-filtered subscriptions. Not indexed in search. | | **DM discovery** | ✅ | DM creation emits kind:39000 (with `hidden` tag) + kind:44100 membership notifications. NIP-29 clients discover DMs via standard group discovery flow. | | **Join request (kind:9021)** | ✅ | Open channels only. Adds member, emits system message + group discovery events + kind:44100 membership notification. Private channels rejected at ingest. | @@ -180,8 +180,11 @@ nak req -k 9 --tag "h=" --search "search query" -l 20 \ --auth --sec ws://localhost:3000 # Reply to a message (NIP-10 threading) +# The `p` tag marked `reply` names the author being answered — omit the marker +# and the recipient must fetch the parent to tell it from a typed @mention. nak event -k 9 -c "Reply text" --tag "h=" \ --tag "e=;;reply" \ + --tag "p=;;reply" \ --auth --sec ws://localhost:3000 # Fetch gift-wrapped DMs (NIP-17) diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 38749577398..55b68520549 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -4560,6 +4560,9 @@ pub(crate) async fn post_failure_notice( Some(buzz_sdk::ThreadRef { root_event_id: root_id, parent_event_id: parent_id, + // A failure notice addresses the channel, not a person, and the + // parsed thread tags do not carry the parent's author anyway. + parent_author: None, }) }); let builder = diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea46..c6723106406 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -588,10 +588,36 @@ async fn handle_setup_membership( } } -/// Build and publish a setup nudge reply to the triggering event. +/// Thread placement for a setup nudge answering `triggering_event`. /// -/// Threading: flat reply to the thread root if one exists; otherwise reply -/// to the triggering event itself. P-tags the asker. +/// The nudge replies to the triggering event, rooted at that event's thread when +/// it has one. `parent_event_id` and `parent_author` must describe the same +/// event: receivers read the addressing marker on the asker's `p` tag as "this +/// answers something I wrote", so pointing the `e` tag at the thread root while +/// naming the triggering event's author made that claim false for anyone who +/// asked inside somebody else's thread. +fn nudge_thread_ref(triggering_event: &nostr::Event) -> Result { + use buzz_sdk::ThreadRef; + + let thread_tags = crate::queue::parse_thread_tags(triggering_event); + let root_event_id = match &thread_tags.root_event_id { + Some(root_str) => nostr::EventId::from_hex(root_str) + .map_err(|e| anyhow::anyhow!("invalid root event id: {e}"))?, + // Top-level event: it is its own thread root. + None => triggering_event.id, + }; + + Ok(ThreadRef { + root_event_id, + parent_event_id: triggering_event.id, + // The asker. Carried here, marked as addressing, rather than passed as a + // mention: the nudge answers them, it does not `@` them, and a bare `p` + // tag cannot say which. + parent_author: Some(triggering_event.pubkey), + }) +} + +/// Build and publish a setup nudge reply to the triggering event. async fn publish_setup_nudge( publisher: &RelayEventPublisher, keys: &nostr::Keys, @@ -599,35 +625,15 @@ async fn publish_setup_nudge( triggering_event: &nostr::Event, payload: &SetupPayload, ) -> Result<()> { - use buzz_sdk::ThreadRef; - - // Parse NIP-10 thread tags to determine reply target. - let thread_tags = crate::queue::parse_thread_tags(triggering_event); - - let thread_ref = if let Some(root_str) = &thread_tags.root_event_id { - // Threaded event: reply flat to the root. - let root_id = nostr::EventId::from_hex(root_str) - .map_err(|e| anyhow::anyhow!("invalid root event id: {e}"))?; - Some(ThreadRef { - root_event_id: root_id, - parent_event_id: root_id, - }) - } else { - // Top-level event: reply to the triggering event. - Some(ThreadRef { - root_event_id: triggering_event.id, - parent_event_id: triggering_event.id, - }) - }; + let thread_ref = Some(nudge_thread_ref(triggering_event)?); let body = payload.nudge_body(); - let author_hex = triggering_event.pubkey.to_hex(); let event_builder = buzz_sdk::build_message( channel_id, &body, thread_ref.as_ref(), - &[&author_hex], // p-tag the asker + &[], // the asker is p-tagged via `thread_ref.parent_author` false, &[], ) @@ -651,6 +657,54 @@ async fn publish_setup_nudge( mod tests { use super::*; + /// Sign an event carrying the given tags, as the triggering ask would arrive. + fn asked(tags: Vec>) -> nostr::Event { + let keys = nostr::Keys::generate(); + nostr::EventBuilder::new(nostr::Kind::Custom(9), "@eva do the thing") + .tags(tags.into_iter().map(|t| nostr::Tag::parse(t).unwrap())) + .sign_with_keys(&keys) + .unwrap() + } + + #[test] + fn a_nudge_answers_the_ask_it_replies_to_not_the_thread_root() { + // The ask is a reply inside someone else's thread. `parent_author` names + // the asker, so `parent_event_id` has to be the asker's event: the + // addressing marker on the resulting `p` tag tells receivers "this + // answers something you wrote", and naming the root would make that + // false for everyone but the root's author. + // Distinct ids, or `root_event_id` would match whichever tag the code + // happened to read and the assertion below would prove nothing. + let root = nostr::EventId::from_hex(&format!("{:064x}", 0xd00du64)).unwrap(); + let answered = nostr::EventId::from_hex(&format!("{:064x}", 0xa11cu64)).unwrap(); + let ask = asked(vec![ + vec!["h", "5e0f6b1c-0000-4000-8000-000000000000"], + vec!["e", &root.to_hex(), "", "root"], + vec!["e", &answered.to_hex(), "", "reply"], + ]); + + let tr = nudge_thread_ref(&ask).unwrap(); + assert_eq!( + tr.root_event_id, root, + "the nudge stays in the ask's thread" + ); + assert_eq!( + tr.parent_event_id, ask.id, + "the nudge replies to the ask, not to the root" + ); + assert_eq!(tr.parent_author, Some(ask.pubkey)); + } + + #[test] + fn a_nudge_to_a_top_level_ask_roots_the_thread_at_that_ask() { + let ask = asked(vec![vec!["h", "5e0f6b1c-0000-4000-8000-000000000000"]]); + + let tr = nudge_thread_ref(&ask).unwrap(); + assert_eq!(tr.root_event_id, ask.id); + assert_eq!(tr.parent_event_id, ask.id); + assert_eq!(tr.parent_author, Some(ask.pubkey)); + } + #[test] fn setup_payload_from_raw_returns_none_when_absent() { // None → Ok(None): normal startup, no setup payload. diff --git a/crates/buzz-cli/src/commands/messages.rs b/crates/buzz-cli/src/commands/messages.rs index ea273336e38..94975d027d4 100644 --- a/crates/buzz-cli/src/commands/messages.rs +++ b/crates/buzz-cli/src/commands/messages.rs @@ -39,6 +39,27 @@ fn find_root_from_tags(tags: &serde_json::Value) -> Option { .map(|(root, _)| root) } +/// Drop the parent author when it is this identity: a reply to one's own +/// message must not carry a self `p` tag, which would put it in the author's +/// own `#p` mention feed. +fn parent_author_excluding_self(client: &BuzzClient, author: Option) -> Option { + let self_hex = client.keys().public_key().to_hex(); + author.filter(|pubkey| !pubkey.eq_ignore_ascii_case(&self_hex)) +} + +/// The pubkey of the event a reply answers. +/// +/// A reply must `p`-tag it: agent `require_mention` subscriptions are `#p` REQ +/// filters, so an untagged reply is never transmitted to the agent being +/// answered. +fn parent_author_from_event(event: &serde_json::Value) -> Option { + event + .get("pubkey") + .and_then(serde_json::Value::as_str) + .filter(|pubkey| pubkey.len() == 64) + .map(str::to_string) +} + fn thread_ref_from_parent_tags( parent_eid: nostr::EventId, parent_event_id: &str, @@ -52,6 +73,8 @@ fn thread_ref_from_parent_tags( Ok(ThreadRef { root_event_id: root_eid, parent_event_id: parent_eid, + // Filled in by the callers, which drop self first. + parent_author: None, }) } @@ -75,12 +98,14 @@ async fn fetch_event(client: &BuzzClient, event_id: &str) -> Result Result { +) -> Result<(ThreadRef, Option), CliError> { let event = fetch_event(client, parent_event_id).await?; - thread_ref_from_event(parent_event_id, &event) + let thread_ref = thread_ref_from_event(parent_event_id, &event)?; + Ok((thread_ref, parent_author_from_event(&event))) } fn thread_ref_from_event(event_id: &str, event: &serde_json::Value) -> Result { @@ -672,12 +697,42 @@ pub async fn cmd_send_message( // Build thread ref if replying. `--reply-to` is the immediate parent; the // thread root is derived from the parent's NIP-10 tags via the relay. - let thread_ref = if let Some(ref r) = p.reply_to { - Some(resolve_thread_ref(client, r).await?) - } else { - None + let (thread_ref, parent_author) = match p.reply_to { + Some(ref r) => { + let (tr, author) = resolve_thread_ref(client, r).await?; + (Some(tr), author) + } + None => (None, None), }; + // A reply addresses the author it answers (NIP-10). Resolved after the + // membership check above so answering someone who has since left the + // channel still works, and carried on the thread ref rather than folded + // into the mentions: as a bare `p` tag the two roles are indistinguishable, + // and the receiver has to fetch the parent to guess which one this is. Self + // is dropped — our own reply must not land in our own mention feed. + let mut mention_pubkeys = mention_pubkeys; + let mut thread_ref = thread_ref; + if let (Some(tr), Some(author)) = ( + thread_ref.as_mut(), + parent_author_excluding_self(client, parent_author), + ) { + let already_typed = mention_pubkeys + .iter() + .any(|pk| pk.eq_ignore_ascii_case(&author)); + if !already_typed { + // `merge_message_mentions` has already truncated to MENTION_CAP, so + // the addressing tag would put the list one over and the builder + // rejects the whole send. That tag is the one that must survive — + // without it the agent being answered never receives the reply — so + // drop a body mention to make room rather than failing. + if mention_pubkeys.len() >= buzz_sdk::mentions::MENTION_CAP { + mention_pubkeys.truncate(buzz_sdk::mentions::MENTION_CAP - 1); + } + tr.parent_author = nostr::PublicKey::from_hex(&author).ok(); + } + } + let mention_refs: Vec<&str> = mention_pubkeys.iter().map(String::as_str).collect(); let builder = match p.kind { @@ -782,10 +837,14 @@ pub async fn cmd_send_diff_message(client: &BuzzClient, p: SendDiffParams) -> Re // `--reply-to` is the immediate parent; the thread root is derived from // the parent's NIP-10 tags via the relay. - let thread_ref = if let Some(r) = &p.reply_to { - Some(resolve_thread_ref(client, r).await?) - } else { - None + // A diff reply addresses the author it answers, same as any other reply: + // an agent's `require_mention` subscription is a `#p` REQ filter. + let (thread_ref, parent_author) = match &p.reply_to { + Some(r) => { + let (tr, author) = resolve_thread_ref(client, r).await?; + (Some(tr), author) + } + None => (None, None), }; let branch = match (&p.source_branch, &p.target_branch) { @@ -806,8 +865,17 @@ pub async fn cmd_send_diff_message(client: &BuzzClient, p: SendDiffParams) -> Re alt_text: Some(alt), }; + // Carried on the thread ref, and marked, so the receiver does not have to + // fetch the parent to learn this `p` tag is addressing and not a mention. + let mut thread_ref = thread_ref; + if let (Some(tr), Some(author)) = ( + thread_ref.as_mut(), + parent_author_excluding_self(client, parent_author), + ) { + tr.parent_author = nostr::PublicKey::from_hex(&author).ok(); + } let builder = - buzz_sdk::build_diff_message(channel_uuid, &diff, &diff_meta, thread_ref.as_ref()) + buzz_sdk::build_diff_message(channel_uuid, &diff, &diff_meta, thread_ref.as_ref(), &[]) .map_err(|e| CliError::Other(format!("build_diff_message failed: {e}")))?; let event = client.sign_event(builder)?; diff --git a/crates/buzz-sdk/src/builders.rs b/crates/buzz-sdk/src/builders.rs index 71c0f1e73db..315fa6c68f2 100644 --- a/crates/buzz-sdk/src/builders.rs +++ b/crates/buzz-sdk/src/builders.rs @@ -189,7 +189,63 @@ fn thread_tags(thread_ref: &ThreadRef, tags: &mut Vec) -> Result<(), SdkErr Ok(()) } -/// Deduplicate and cap mentions, emitting p-tags. +/// Marker on a `p` tag naming the author this reply answers. +/// +/// Mirrors `P_TAG_ADDRESSING_MARKER` in +/// `desktop/src/features/messages/lib/threading.ts`. +pub const P_TAG_ADDRESSING_MARKER: &str = "reply"; + +/// Marker on a `p` tag naming someone the author typed as `@name`. +/// +/// Mirrors `P_TAG_MENTION_MARKER` in the same TypeScript module. +pub const P_TAG_MENTION_MARKER: &str = "mention"; + +/// `p` tags for a reply, each marked with the role it plays. +/// +/// NIP-10 addressing and a typed `@mention` are byte-identical as bare `p` +/// tags, so a receiver has to fetch the parent and check who wrote it just to +/// tell them apart. These markers record what the sender already knows. +/// +/// A pubkey that is both typed and the parent's author is emitted once as a +/// mention: that is the one case fetching the parent cannot decide, and mention +/// is the role that keeps the stronger signal. Relay tag filters match only the +/// second element, so no marker affects `#p` delivery to an agent. +fn reply_mention_tags( + mentions: &[&str], + parent_author: Option<&str>, + tags: &mut Vec, +) -> Result<(), SdkError> { + let mut seen = std::collections::HashSet::new(); + let mut lowered = Vec::new(); + for &hex in mentions { + let lower = hex.to_ascii_lowercase(); + if seen.insert(lower.clone()) { + lowered.push(lower); + } + } + let addressing = parent_author + .map(|hex| hex.to_ascii_lowercase()) + .filter(|lower| !seen.contains(lower)); + + // The addressing tag must survive the cap: without it the agent being + // answered never receives the reply, while a dropped body mention costs one + // notification. + if lowered.len() > crate::mentions::MENTION_CAP - usize::from(addressing.is_some()) { + return Err(SdkError::TooManyMentions); + } + for lower in &lowered { + tags.push(tag(&["p", lower, "", P_TAG_MENTION_MARKER])?); + } + if let Some(lower) = &addressing { + tags.push(tag(&["p", lower, "", P_TAG_ADDRESSING_MARKER])?); + } + Ok(()) +} + +/// Deduplicate and cap mentions, emitting bare `p` tags. +/// +/// Bare because a top-level message has no parent to answer: every `p` tag on +/// it was typed by the author, so there is no second role to distinguish. fn mention_tags(mentions: &[&str], tags: &mut Vec) -> Result<(), SdkError> { if mentions.len() > crate::mentions::MENTION_CAP { return Err(SdkError::TooManyMentions); @@ -234,7 +290,15 @@ pub fn build_message( if let Some(tr) = thread_ref { thread_tags(tr, &mut tags)?; } - mention_tags(mentions, &mut tags)?; + match thread_ref { + Some(tr) => reply_mention_tags( + mentions, + tr.parent_author.map(|pk| pk.to_hex()).as_deref(), + &mut tags, + )?, + // Top-level: a `p` tag can only be a mention, so nothing to mark. + None => mention_tags(mentions, &mut tags)?, + } if broadcast { tags.push(tag(&["broadcast", "1"])?); } @@ -308,7 +372,11 @@ pub fn build_forum_comment( check_content(content, 64 * 1024)?; let mut tags = vec![tag(&["h", &channel_id.to_string()])?]; thread_tags(thread_ref, &mut tags)?; - mention_tags(mentions, &mut tags)?; + reply_mention_tags( + mentions, + thread_ref.parent_author.map(|pk| pk.to_hex()).as_deref(), + &mut tags, + )?; imeta_tags(media_tags, &mut tags)?; Ok(EventBuilder::new(Kind::Custom(45003), content) .tags(tags) @@ -321,6 +389,7 @@ pub fn build_diff_message( content: &str, diff_meta: &DiffMeta, thread_ref: Option<&ThreadRef>, + mention_pubkeys: &[&str], ) -> Result { check_content(content, 60 * 1024)?; @@ -382,6 +451,14 @@ pub fn build_diff_message( if let Some(tr) = thread_ref { thread_tags(tr, &mut tags)?; } + match thread_ref { + Some(tr) => reply_mention_tags( + mention_pubkeys, + tr.parent_author.map(|pk| pk.to_hex()).as_deref(), + &mut tags, + )?, + None => mention_tags(mention_pubkeys, &mut tags)?, + } Ok(EventBuilder::new(Kind::Custom(40008), content).tags(tags)) } @@ -2424,6 +2501,7 @@ mod tests { let tr = ThreadRef { root_event_id: root, parent_event_id: root, + parent_author: None, }; let builder = build_forum_comment(cid, "self-canary", &tr, &[&self_pk], &[]).unwrap(); let ev = builder.sign_with_keys(&sender).expect("sign"); @@ -2484,6 +2562,7 @@ mod tests { let tr = ThreadRef { root_event_id: eid, parent_event_id: eid, + parent_author: None, }; let ev = sign(build_message(cid, "reply", Some(&tr), &[], false, &[]).unwrap()); // Direct reply: only one e-tag with "reply" marker @@ -2507,6 +2586,7 @@ mod tests { let tr = ThreadRef { root_event_id: root, parent_event_id: parent, + parent_author: None, }; let ev = sign(build_message(cid, "nested", Some(&tr), &[], false, &[]).unwrap()); let e_tags: Vec<_> = ev @@ -2599,6 +2679,7 @@ mod tests { let tr = ThreadRef { root_event_id: eid, parent_event_id: eid, + parent_author: None, }; let ev = sign(build_forum_comment(cid, "comment", &tr, &[], &[]).unwrap()); assert_eq!(ev.kind.as_u16(), 45003); @@ -2623,7 +2704,8 @@ mod tests { #[test] fn diff_message_happy_path() { let cid = uuid(); - let ev = sign(build_diff_message(cid, "diff content", &good_diff_meta(), None).unwrap()); + let ev = + sign(build_diff_message(cid, "diff content", &good_diff_meta(), None, &[]).unwrap()); assert_eq!(ev.kind.as_u16(), 40008); assert!(has_tag(&ev, "repo", "https://github.com/example/repo")); assert!(has_tag(&ev, "commit", "abc1234")); @@ -2636,7 +2718,7 @@ mod tests { let mut meta = good_diff_meta(); meta.repo_url = "ftp://bad.url".into(); assert!(matches!( - build_diff_message(cid, "x", &meta, None), + build_diff_message(cid, "x", &meta, None, &[]), Err(SdkError::InvalidDiffMeta(_)) )); } @@ -2647,7 +2729,7 @@ mod tests { let mut meta = good_diff_meta(); meta.commit_sha = "abc12".into(); // only 5 chars assert!(matches!( - build_diff_message(cid, "x", &meta, None), + build_diff_message(cid, "x", &meta, None, &[]), Err(SdkError::InvalidDiffMeta(_)) )); } @@ -2658,7 +2740,7 @@ mod tests { let mut meta = good_diff_meta(); meta.commit_sha = "xyz1234".into(); // 'x', 'y', 'z' not hex assert!(matches!( - build_diff_message(cid, "x", &meta, None), + build_diff_message(cid, "x", &meta, None, &[]), Err(SdkError::InvalidDiffMeta(_)) )); } @@ -2669,7 +2751,7 @@ mod tests { let mut meta = good_diff_meta(); meta.branch = Some(("main".into(), "".into())); // target empty assert!(matches!( - build_diff_message(cid, "x", &meta, None), + build_diff_message(cid, "x", &meta, None, &[]), Err(SdkError::InvalidDiffMeta(_)) )); } @@ -2680,7 +2762,7 @@ mod tests { let mut meta = good_diff_meta(); meta.pr_number = Some(0); assert!(matches!( - build_diff_message(cid, "x", &meta, None), + build_diff_message(cid, "x", &meta, None, &[]), Err(SdkError::InvalidDiffMeta(_)) )); } @@ -2690,7 +2772,7 @@ mod tests { let cid = uuid(); let big = "x".repeat(60 * 1024 + 1); assert!(matches!( - build_diff_message(cid, &big, &good_diff_meta(), None), + build_diff_message(cid, &big, &good_diff_meta(), None, &[]), Err(SdkError::ContentTooLarge { .. }) )); } @@ -2710,7 +2792,7 @@ mod tests { truncated: true, alt_text: Some("patch for bug fix".into()), }; - let ev = sign(build_diff_message(cid, "diff", &meta, None).unwrap()); + let ev = sign(build_diff_message(cid, "diff", &meta, None, &[]).unwrap()); assert!(has_tag(&ev, "file", "src/lib.rs")); assert!(has_tag(&ev, "parent-commit", "1234567")); assert!(has_tag(&ev, "pr", "42")); diff --git a/crates/buzz-sdk/src/lib.rs b/crates/buzz-sdk/src/lib.rs index 4ee0cd4c882..d12a05cddae 100644 --- a/crates/buzz-sdk/src/lib.rs +++ b/crates/buzz-sdk/src/lib.rs @@ -30,6 +30,13 @@ pub struct ThreadRef { pub root_event_id: nostr::EventId, /// The immediate parent being replied to. pub parent_event_id: nostr::EventId, + /// Author of `parent_event_id`, when the caller resolved the parent event. + /// + /// Emitted as a `p` tag marked with the addressing role rather than folded + /// into the mention list. As a bare tag the two are byte-identical, which + /// forces every receiver to fetch the parent just to tell "answered you" + /// from "mentioned you". + pub parent_author: Option, } /// Metadata for diff/patch messages (kind 40008). diff --git a/desktop/src-tauri/src/commands/agent_discovery/relay_directory.rs b/desktop/src-tauri/src/commands/agent_discovery/relay_directory.rs index db0573acd7c..b521866c3d6 100644 --- a/desktop/src-tauri/src/commands/agent_discovery/relay_directory.rs +++ b/desktop/src-tauri/src/commands/agent_discovery/relay_directory.rs @@ -544,7 +544,11 @@ mod real_relay_tests { channel_id, "Ask @Agent Probe to reply", None, - &[mention_pubkey], + // Typed in the body, so this is a real mention. + events::Recipients { + typed: &[mention_pubkey], + ..Default::default() + }, &[], &[], &[], diff --git a/desktop/src-tauri/src/commands/feed.rs b/desktop/src-tauri/src/commands/feed.rs new file mode 100644 index 00000000000..fc62e8a94cd --- /dev/null +++ b/desktop/src-tauri/src/commands/feed.rs @@ -0,0 +1,411 @@ +//! Home-feed assembly (`get_feed`). +//! +//! Split out of `messages.rs`: the mention feed is a raw `#p` query, and +//! separating a reply's addressing `p` tag from a real mention needs its own +//! parent lookup, which does not belong in the general message command file. + +use tauri::State; + +use crate::app_state::AppState; +use crate::models::{FeedItemInfo, FeedMeta, FeedResponse, FeedSections}; + +use super::messages::forum::{ + apply_link_preview_suppression, fetch_agent_owner_pubkeys, link_preview_suppression_targets, +}; +use super::messages::{feed_item_from_event, reply_parent_id}; +use crate::relay::query_relay; + +/// Kinds the live thread-reply path can take ownership of when this feed marks +/// an item `reply_to_self`. +/// +/// Mirrors `CHANNEL_MESSAGE_EVENT_KINDS` (the desktop unread-trigger set) in +/// `desktop/src/shared/constants/kinds.ts`. Deliberately narrower than the +/// mention query above, which also returns kind:1 and git events — the live +/// path never sees those, so handing one over would lose it entirely. +const LIVE_OWNED_REPLY_KINDS: [u16; 4] = [9, 40002, 45001, 45003]; + +/// How much wider than the caller's cap the `#p` mention query runs. +/// +/// Replies-to-you share the `#p` window with real mentions and are filtered out +/// only after the query, so the window has to hold both. 4x covers a thread +/// answering the user three times for every mention it delivers. +const MENTION_OVERFETCH_FACTOR: u64 = 4; + +/// Ceiling on the over-fetched mention window. +/// +/// Not the relay's page clamp (`DEFAULT_MAX_PAGE_LIMIT`, 1000) — the returned +/// ids flow straight into two unchunked follow-up filters, the `#e` edits query +/// and the reply-parent `ids` query, and `buzz-db`'s batch fetch documents a +/// caller-bounded batch of 500. Keeping the window at 200 leaves both well +/// inside that while still holding four times the default page of mentions. +const MENTION_OVERFETCH_CEILING: u64 = 200; + +/// Trims an over-fetched mention list to `cap`, giving real mentions the slots +/// before replies-to-self get any. +/// +/// Both kinds of item share one `#p` window, so a plain newest-first truncation +/// lets a chatty thread push every real `@you` out of the response. Order within +/// the result stays newest-first, as callers expect. +fn trim_mentions_preferring_real(mut items: Vec, cap: usize) -> Vec { + if items.len() <= cap { + return items; + } + // Sorted before selecting, not only before returning. Both the truncation + // and the reply fill-in take from the front of their half, and `partition` + // preserves input order — so on an input that is not already newest-first + // this dropped the newer item and kept the older one, then sorted the wrong + // survivors into the right order. The relay does answer `created_at DESC` + // today, which is the only reason it looked correct; nothing here should + // depend on that. + items.sort_by_key(|item| std::cmp::Reverse(item.created_at)); + let (mut kept, replies): (Vec, Vec) = + items.into_iter().partition(|item| !item.reply_to_self); + kept.truncate(cap); + let room = cap.saturating_sub(kept.len()); + kept.extend(replies.into_iter().take(room)); + kept.sort_by_key(|item| std::cmp::Reverse(item.created_at)); + kept +} + +use crate::p_tag_role::{p_tag_role_for_event, PTagRole}; + +/// Whether a string is a well-formed 64-char hex event id, and so safe to put in +/// an `ids` filter. +fn is_event_id_hex(value: &str) -> bool { + value.len() == 64 && value.chars().all(|c| c.is_ascii_hexdigit()) +} + +/// Mirrors `isBroadcastReply` in `desktop/src/features/messages/lib/threading.ts`. +fn is_broadcast_reply(ev: &nostr::Event) -> bool { + ev.tags.iter().any(|tag| { + let s = tag.as_slice(); + s.len() >= 2 && s[0] == "broadcast" && s[1] == "1" + }) +} + +#[tauri::command] +pub async fn get_feed( + since: Option, + limit: Option, + types: Option, + state: State<'_, AppState>, +) -> Result { + let cap = limit.unwrap_or(50).min(100); + + // Parse types filter — if absent, run all sub-queries. + // Comma-separated: e.g. "mentions,needs_action". + let want_mentions = types + .as_deref() + .map(|t| t.split(',').any(|s| s.trim() == "mentions")) + .unwrap_or(true); + let want_needs_action = types + .as_deref() + .map(|t| t.split(',').any(|s| s.trim() == "needs_action")) + .unwrap_or(true); + + let my_pubkey = { + let keys = state.keys.lock().map_err(|e| e.to_string())?; + keys.public_key().to_hex() + }; + + // Mentions: messages that reference me via #p. + let mut mention_filter = serde_json::json!({ + "kinds": [ + 9, + 40002, + 1, + 45001, + 45003, + buzz_core_pkg::kind::KIND_GIT_PULL_REQUEST, + buzz_core_pkg::kind::KIND_GIT_PR_UPDATE, + buzz_core_pkg::kind::KIND_GIT_ISSUE, + buzz_core_pkg::kind::KIND_GIT_STATUS_OPEN, + buzz_core_pkg::kind::KIND_GIT_STATUS_MERGED, + buzz_core_pkg::kind::KIND_GIT_STATUS_CLOSED, + buzz_core_pkg::kind::KIND_GIT_STATUS_DRAFT, + ], + "#p": [my_pubkey], + // Over-fetch. A reply carries a `p` tag naming the author it answers, so + // this `#p` query now returns replies-to-you as well as real mentions, + // and the replies are only discarded *after* the query by `reply_to_self` + // filtering. At `limit: cap` a thread with `cap` recent replies to your + // own messages evicts every real mention from the window, so an `@you` + // from earlier the same day never reaches the Inbox, the badge, or the + // mention toast — and the live path cannot compensate, because it only + // sees events published while the app was connected. + // + "limit": (cap as u64 * MENTION_OVERFETCH_FACTOR).min(MENTION_OVERFETCH_CEILING), + }); + if let Some(s) = since { + mention_filter["since"] = serde_json::json!(s); + } + // Needs-action: workflow approval-request events sent to me. + let mut approval_filter = serde_json::json!({ + "kinds": [46010, 46011, 46012], + "#p": [my_pubkey], + "limit": 20, + }); + if let Some(s) = since { + approval_filter["since"] = serde_json::json!(s); + } + + let mention_events = if want_mentions { + query_relay(&state, &[mention_filter]) + .await + .unwrap_or_default() + } else { + Vec::new() + }; + let approval_events = if want_needs_action { + query_relay(&state, &[approval_filter]) + .await + .unwrap_or_default() + } else { + Vec::new() + }; + + let mention_ids = mention_events + .iter() + .map(|event| event.id.to_hex()) + .collect::>(); + let mention_edits = if mention_ids.is_empty() { + Vec::new() + } else { + query_relay( + &state, + &[serde_json::json!({ "kinds": [40003], "#e": mention_ids })], + ) + .await + .unwrap_or_default() + }; + // A reply p-tags the author it answers, so the `#p` mention query above + // also returns every answer to one of the user's own messages. Resolve the + // parents so the client can tell "you were mentioned" from "someone + // replied to you" — they differ on whether a channel mute applies. + // Parents already in this batch are answered from it rather than skipped: + // a message can both p-tag the user and be authored by them, and treating + // that as "not self-authored" would leave the reply in this feed while the + // live path also claims it — two toasts for one event. + let mut self_authored_parent_ids: std::collections::HashSet = mention_events + .iter() + .filter(|ev| ev.pubkey.to_hex() == my_pubkey) + .map(|ev| ev.id.to_hex()) + .collect(); + let reply_parent_ids: Vec = mention_events + .iter() + .filter(|ev| !is_broadcast_reply(ev)) + // The round trip the markers exist to remove. A sender that told us + // which role its `p` tag plays has already answered the only question + // this query asks. + .filter(|ev| p_tag_role_for_event(ev, &my_pubkey) == PTagRole::Unknown) + .filter_map(reply_parent_id) + // `reply_parent_id` lowercases the tag value but does not validate it, + // and the relay *accepts* an event whose `e` value is not hex at all — its + // thread-meta resolver silently ignores such a tag rather than rejecting + // the event. Passing one into an `ids` filter makes the relay reject the + // whole filter as malformed, so a single junk event in the mention window + // would take out every later query built from it. + .filter(|id| is_event_id_hex(id)) + .filter(|id| !mention_ids.contains(id)) + .collect::>() + .into_iter() + .collect(); + let queried_parent_ids: std::collections::HashSet = if reply_parent_ids.is_empty() { + std::collections::HashSet::new() + } else { + query_relay( + &state, + &[serde_json::json!({ + "ids": reply_parent_ids, + // Keep in sync with REPLY_PARENT_EVENT_KINDS in + // desktop/src/shared/constants/kinds.ts. The live desktop path + // answers the same question from its own lookup, so a kind + // missing here but present there makes both paths claim the + // event and notify twice. + // 40001 is a legacy pre-migration stream message. It is still a + // repliable parent, and omitting it made every reply to one + // resolve as "parent absent" — which reads as a real mention and + // pierces the mute the lookup exists to protect. + "kinds": [9, 40001, 40002, 40008, 1, 45001, 45003], + "authors": [my_pubkey], + "limit": reply_parent_ids.len(), + })], + ) + .await + // Deliberately not `unwrap_or_default()` like the queries above. Their + // failure only shortens the feed, which is harmless. This one's failure + // *flips a classification*: an empty result is indistinguishable from + // "none of these parents are mine", so every reply in the batch would be + // relabelled a real mention. The frontend fails open on the documented + // assumption that this feed already dropped what it resolved to us, so + // that relabelling would double-notify in an unmuted channel and pierce + // the mute in a muted one. + // + // Failing the poll is the right response, and is safe *because* the ids + // above are validated: a malformed id used to make the relay reject the + // filter outright, which — with no `since` on the mention query — failed + // every later poll forever. What is left is transient, and React Query + // keeps the previous `data` on error, so the `feed` reference does not + // change, the notification effect does not re-run, no id is consumed from + // the seen set, and the next good poll delivers normally. + // + // Do not "degrade" by guessing here. Marking the batch `reply_to_self` + // hands it to the live path, but `collectHomeAlertItems` still adds every + // declined item to the persisted seen set — so the guess consumes the + // notification slot and the next poll drops the item as already-seen. A + // genuine typed `@mention` inside a thread is silently lost for good, + // across restarts, because its parent belongs to a third party and so + // looks unresolved. Events in channels absent from the local list are not + // deferrable at all: the live path returns early for those. + .map_err(|e| { + format!("could not resolve reply parents, so this feed poll cannot tell a mention from a reply: {e}") + })? + .iter() + .map(|event| event.id.to_hex()) + .collect() + }; + self_authored_parent_ids.extend(queried_parent_ids); + let mention_owner_pubkeys = fetch_agent_owner_pubkeys(&state, &mention_events).await; + let suppressed_mentions = + link_preview_suppression_targets(&mention_events, &mention_edits, &mention_owner_pubkeys); + let mentions: Vec = mention_events + .iter() + .map(|ev| { + // Canonical singular category, matching `FeedItemCategory` on the TS + // side. The plural spelling here silently disabled every + // `category === "mention"` check downstream. + let mut item = feed_item_from_event(ev, "mention"); + // `reply_to_self` hands the event to the live thread-reply path, + // which never sees a broadcast reply — `isThreadReply` excludes + // them by design. Marking one here would drop it from this feed + // with nothing on the other side to pick it up. + // + // Restricted to the kinds the live path can actually own. The + // mention query is wider than its unread-trigger set, so marking a + // bridged kind:1 note or a git event would drop it here with + // nothing on the other side to pick it up. + item.reply_to_self = LIVE_OWNED_REPLY_KINDS.contains(&ev.kind.as_u16()) + && !is_broadcast_reply(ev) + && match p_tag_role_for_event(ev, &my_pubkey) { + // The sender said so, and it knew without asking anyone. + PTagRole::Addressing => true, + // Typed in the body. This is the case the parent lookup + // below cannot decide, because the recipient is both the + // author being answered and someone the author named. + PTagRole::Mention => false, + PTagRole::Unknown | PTagRole::None => reply_parent_id(ev) + .is_some_and(|parent| self_authored_parent_ids.contains(&parent)), + }; + apply_link_preview_suppression(&mut item.tags, &item.id, &suppressed_mentions); + item + }) + .collect(); + // Back down to what the caller asked for. The query above deliberately + // over-fetches, so trimming has to prefer real mentions — taking the newest + // `cap` items would reintroduce the eviction the over-fetch exists to + // prevent. + let mentions = trim_mentions_preferring_real(mentions, cap as usize); + let needs_action: Vec = approval_events + .iter() + .map(|ev| feed_item_from_event(ev, "needs_action")) + .collect(); + + let total = (mentions.len() + needs_action.len()) as u64; + Ok(FeedResponse { + feed: FeedSections { + mentions, + needs_action, + activity: Vec::new(), + agent_activity: Vec::new(), + }, + meta: FeedMeta { + since: since.unwrap_or(0), + total, + generated_at: chrono::Utc::now().timestamp(), + }, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn item(id: &str, created_at: u64, reply_to_self: bool) -> FeedItemInfo { + FeedItemInfo { + id: id.to_string(), + kind: 9, + pubkey: String::new(), + content: String::new(), + created_at, + channel_id: None, + channel_name: String::new(), + channel_type: None, + tags: Vec::new(), + category: "mention".to_string(), + reply_to_self, + } + } + + #[test] + fn trim_keeps_a_real_mention_a_burst_of_replies_would_evict() { + // The regression: replies-to-you share the `#p` window with real + // mentions, so newest-first truncation lets a chatty thread push every + // `@you` out of the response entirely. + let mut items: Vec = (0..5) + .map(|i| item(&format!("reply-{i}"), 200 + i, true)) + .collect(); + items.push(item("real-mention", 100, false)); + + let trimmed = trim_mentions_preferring_real(items, 3); + + // The real mention survives, and the two replies that keep it company + // are the newest ones — not whichever two happened to come first. + assert_eq!( + trimmed.iter().map(|i| i.id.as_str()).collect::>(), + vec!["reply-4", "reply-3", "real-mention"], + ); + } + + #[test] + fn trim_returns_newest_first() { + let items = vec![ + item("old-reply", 100, true), + item("new-mention", 300, false), + item("mid-reply", 200, true), + ]; + + let trimmed = trim_mentions_preferring_real(items, 2); + + assert_eq!( + trimmed.iter().map(|i| i.id.as_str()).collect::>(), + vec!["new-mention", "mid-reply"], + ); + } + + #[test] + fn trim_is_a_no_op_under_the_cap() { + let items = vec![item("a", 100, true), item("b", 200, false)]; + + let trimmed = trim_mentions_preferring_real(items, 10); + + assert_eq!(trimmed.len(), 2); + // Untouched, so the original order survives. + assert_eq!(trimmed[0].id, "a"); + } + + #[test] + fn trim_fills_remaining_slots_with_replies() { + let items = vec![ + item("mention", 100, false), + item("reply-new", 300, true), + item("reply-old", 200, true), + ]; + + let trimmed = trim_mentions_preferring_real(items, 2); + + assert_eq!( + trimmed.iter().map(|i| i.id.as_str()).collect::>(), + vec!["reply-new", "mention"], + ); + } +} diff --git a/desktop/src-tauri/src/commands/messages.rs b/desktop/src-tauri/src/commands/messages.rs index 31559777d2b..55f2b64fe81 100644 --- a/desktop/src-tauri/src/commands/messages.rs +++ b/desktop/src-tauri/src/commands/messages.rs @@ -1,21 +1,15 @@ use nostr::{Event, EventId, Keys, PublicKey}; use tauri::{AppHandle, State}; -mod forum; +pub(super) mod forum; -use forum::{ - apply_link_preview_suppression, fetch_agent_owner_pubkeys, link_preview_suppression_targets, -}; pub use forum::{get_forum_posts, get_forum_thread}; use crate::{ app_state::AppState, events, managed_agents::{find_managed_agent_mut, load_managed_agents, ManagedAgentRecord}, - models::{ - FeedItemInfo, FeedMeta, FeedResponse, FeedSections, SearchResponse, - SendChannelMessageResponse, ThreadRepliesResponse, - }, + models::{FeedItemInfo, SearchResponse, SendChannelMessageResponse, ThreadRepliesResponse}, nostr_convert, relay::{ assert_expected_relay_scope, assert_expected_signer, query_relay, submit_event, @@ -46,124 +40,6 @@ const TIMELINE_KINDS: [u32; 11] = [ buzz_core_pkg::kind::KIND_HUDDLE_STARTED, ]; -#[tauri::command] -pub async fn get_feed( - since: Option, - limit: Option, - types: Option, - state: State<'_, AppState>, -) -> Result { - let cap = limit.unwrap_or(50).min(100); - - // Parse types filter — if absent, run all sub-queries. - // Comma-separated: e.g. "mentions,needs_action". - let want_mentions = types - .as_deref() - .map(|t| t.split(',').any(|s| s.trim() == "mentions")) - .unwrap_or(true); - let want_needs_action = types - .as_deref() - .map(|t| t.split(',').any(|s| s.trim() == "needs_action")) - .unwrap_or(true); - - let my_pubkey = { - let keys = state.keys.lock().map_err(|e| e.to_string())?; - keys.public_key().to_hex() - }; - - // Mentions: messages that reference me via #p. - let mut mention_filter = serde_json::json!({ - "kinds": [ - 9, - 40002, - 1, - 45001, - 45003, - buzz_core_pkg::kind::KIND_GIT_PULL_REQUEST, - buzz_core_pkg::kind::KIND_GIT_PR_UPDATE, - buzz_core_pkg::kind::KIND_GIT_ISSUE, - buzz_core_pkg::kind::KIND_GIT_STATUS_OPEN, - buzz_core_pkg::kind::KIND_GIT_STATUS_MERGED, - buzz_core_pkg::kind::KIND_GIT_STATUS_CLOSED, - buzz_core_pkg::kind::KIND_GIT_STATUS_DRAFT, - ], - "#p": [my_pubkey], - "limit": cap, - }); - if let Some(s) = since { - mention_filter["since"] = serde_json::json!(s); - } - // Needs-action: workflow approval-request events sent to me. - let mut approval_filter = serde_json::json!({ - "kinds": [46010, 46011, 46012], - "#p": [my_pubkey], - "limit": 20, - }); - if let Some(s) = since { - approval_filter["since"] = serde_json::json!(s); - } - - let mention_events = if want_mentions { - query_relay(&state, &[mention_filter]) - .await - .unwrap_or_default() - } else { - Vec::new() - }; - let approval_events = if want_needs_action { - query_relay(&state, &[approval_filter]) - .await - .unwrap_or_default() - } else { - Vec::new() - }; - - let mention_ids = mention_events - .iter() - .map(|event| event.id.to_hex()) - .collect::>(); - let mention_edits = if mention_ids.is_empty() { - Vec::new() - } else { - query_relay( - &state, - &[serde_json::json!({ "kinds": [40003], "#e": mention_ids })], - ) - .await - .unwrap_or_default() - }; - let mention_owner_pubkeys = fetch_agent_owner_pubkeys(&state, &mention_events).await; - let suppressed_mentions = - link_preview_suppression_targets(&mention_events, &mention_edits, &mention_owner_pubkeys); - let mentions: Vec = mention_events - .iter() - .map(|ev| { - let mut item = feed_item_from_event(ev, "mentions"); - apply_link_preview_suppression(&mut item.tags, &item.id, &suppressed_mentions); - item - }) - .collect(); - let needs_action: Vec = approval_events - .iter() - .map(|ev| feed_item_from_event(ev, "needs_action")) - .collect(); - - let total = (mentions.len() + needs_action.len()) as u64; - Ok(FeedResponse { - feed: FeedSections { - mentions, - needs_action, - activity: Vec::new(), - agent_activity: Vec::new(), - }, - meta: FeedMeta { - since: since.unwrap_or(0), - total, - generated_at: chrono::Utc::now().timestamp(), - }, - }) -} - fn build_search_messages_filter( q: &str, cap: u32, @@ -437,6 +313,13 @@ pub async fn get_event(event_id: String, state: State<'_, AppState>) -> Result>>, sent_from_thread_tag: Option>, mention_pubkeys: Option>, + recipient_pubkeys: Option>, kind: Option, expected_relay_url: Option, expected_signer_pubkey: Option, @@ -458,6 +342,12 @@ pub async fn send_channel_message( .map_err(|_| format!("invalid channel UUID: {channel_id}"))?; let mentions = mention_pubkeys.unwrap_or_default(); let mention_refs: Vec<&str> = mentions.iter().map(|s| s.as_str()).collect(); + let addressed = recipient_pubkeys.unwrap_or_default(); + let addressed_refs: Vec<&str> = addressed.iter().map(|s| s.as_str()).collect(); + let recipients = events::Recipients { + typed: &mention_refs, + addressed: &addressed_refs, + }; let media = media_tags.unwrap_or_default(); let emoji = emoji_tags.unwrap_or_default(); let mention_refs_only = mention_tags.unwrap_or_default(); @@ -498,14 +388,20 @@ pub async fn send_channel_message( let parent_id = parent_event_id .as_deref() .ok_or("forum comment requires parent_event_id")?; - let thread_ref = - resolve_thread_ref(parent_id, &state, &relay_base, Some(&signing_keys)).await?; + let thread_ref = resolve_thread_ref( + parent_id, + &state, + &relay_base, + Some(&signing_keys), + Some(signing_keys.public_key()), + ) + .await?; resolved_root = Some(thread_ref.root_event_id.to_hex()); events::build_forum_comment( channel_uuid, content.trim(), &thread_ref, - &mention_refs, + recipients, &media, &mention_refs_only, )? @@ -513,8 +409,14 @@ pub async fn send_channel_message( _ => { let thread_ref = match parent_event_id.as_deref() { Some(pid) => { - let tr = - resolve_thread_ref(pid, &state, &relay_base, Some(&signing_keys)).await?; + let tr = resolve_thread_ref( + pid, + &state, + &relay_base, + Some(&signing_keys), + Some(signing_keys.public_key()), + ) + .await?; resolved_root = Some(tr.root_event_id.to_hex()); Some(tr) } @@ -524,7 +426,7 @@ pub async fn send_channel_message( channel_uuid, content.trim(), thread_ref.as_ref(), - &mention_refs, + recipients, &media, &emoji, &mention_refs_only, @@ -699,7 +601,10 @@ fn build_managed_agent_channel_message( channel_id, content, thread_ref, - &mention_refs, + events::Recipients { + typed: &mention_refs, + ..Default::default() + }, &[], &[], &[], @@ -761,12 +666,15 @@ pub async fn send_managed_agent_channel_message( Some(parent_id) => Some( // Same active-relay resolution as before — this path has no // caller-captured tenant scope (yet), so resolve the override - // here and read through it with the active identity. + // here and read through it with the active identity. The agent + // still *signs* as itself, so that is the key the self-reply check + // is judged against, not the identity that authed the read. resolve_thread_ref( parent_id, &state, &crate::relay::relay_api_base_url_with_override(&state), None, + Some(keys.public_key()), ) .await?, ), @@ -968,7 +876,29 @@ fn tags_to_vec(ev: &nostr::Event) -> Vec> { ev.tags.iter().map(|t| t.as_slice().to_vec()).collect() } -fn feed_item_from_event(ev: &nostr::Event, category: &str) -> FeedItemInfo { +/// NIP-10 reply target of an event, if it has one. +/// +/// Lowercased, because hex decodes case-insensitively: an uppercase `e` value is +/// a valid id the relay will resolve, but every set it is tested against here is +/// built from `Event::id().to_hex()`, which is always lowercase. Comparing the +/// raw value made such a reply look like it had no self-authored parent, which +/// relabels it a real mention — a second notification on top of the live path's, +/// and a pierced channel mute. +pub(super) fn reply_parent_id(ev: &nostr::Event) -> Option { + ev.tags + .iter() + .filter_map(|tag| { + let s = tag.as_slice(); + (s.len() >= 4 && s[0] == "e" && s[3] == "reply").then(|| s[1].to_ascii_lowercase()) + }) + // Last, not first — `getThreadReference` in + // desktop/src/features/messages/lib/threading.ts reverses before it + // searches. The two must resolve the same parent or they disagree + // about who owns a reply notification and it fires twice or not at all. + .next_back() +} + +pub(super) fn feed_item_from_event(ev: &nostr::Event, category: &str) -> FeedItemInfo { let channel_id = channel_id_from_tags(ev); FeedItemInfo { id: ev.id.to_hex(), @@ -981,6 +911,7 @@ fn feed_item_from_event(ev: &nostr::Event, category: &str) -> FeedItemInfo { channel_type: None, tags: tags_to_vec(ev), category: category.to_string(), + reply_to_self: false, } } #[cfg(test)] diff --git a/desktop/src-tauri/src/commands/messages/forum.rs b/desktop/src-tauri/src/commands/messages/forum.rs index 086e8c9f793..9243d2349e8 100644 --- a/desktop/src-tauri/src/commands/messages/forum.rs +++ b/desktop/src-tauri/src/commands/messages/forum.rs @@ -9,7 +9,7 @@ use crate::{ relay::query_relay, }; -pub(super) async fn fetch_agent_owner_pubkeys( +pub(in crate::commands) async fn fetch_agent_owner_pubkeys( state: &AppState, events: &[nostr::Event], ) -> std::collections::HashMap { @@ -106,7 +106,7 @@ pub(super) fn forum_reply_from_event( } } -pub(super) fn link_preview_suppression_targets( +pub(in crate::commands) fn link_preview_suppression_targets( originals: &[nostr::Event], edits: &[nostr::Event], owner_pubkeys: &std::collections::HashMap, @@ -140,7 +140,7 @@ pub(super) fn link_preview_suppression_targets( .collect() } -pub(super) fn apply_link_preview_suppression( +pub(in crate::commands) fn apply_link_preview_suppression( tags: &mut Vec>, event_id: &str, suppressed: &std::collections::HashSet, diff --git a/desktop/src-tauri/src/commands/messages/thread_ref.rs b/desktop/src-tauri/src/commands/messages/thread_ref.rs index 97a03fdad5b..0808190210b 100644 --- a/desktop/src-tauri/src/commands/messages/thread_ref.rs +++ b/desktop/src-tauri/src/commands/messages/thread_ref.rs @@ -14,11 +14,18 @@ use crate::{ /// pinned a signer snapshot pass it as `keys` so this read's NIP-98 auth is /// minted by the same identity that signs the eventual event; `None` /// preserves the active-identity read for unpinned callers. +/// +/// `signer` is the key that will sign the *reply*, used only to suppress the +/// addressing tag on a self-reply. It is deliberately separate from `keys`: a +/// managed agent reads as the active identity but signs as itself, so the two +/// differ on that path and comparing against the wrong one strips the tag off a +/// reply to whoever the other key belongs to. pub(super) async fn resolve_thread_ref( parent_event_id: &str, state: &AppState, api_base_url: &str, keys: Option<&nostr::Keys>, + signer: Option, ) -> Result { let parent_eid = EventId::from_hex(parent_event_id).map_err(|e| format!("invalid parent event ID: {e}"))?; @@ -58,8 +65,23 @@ pub(super) async fn resolve_thread_ref( _ => parent_eid, }; - Ok(events::ThreadRef { + let mut thread_ref = events::ThreadRef { root_event_id: root_eid, parent_event_id: parent_eid, - }) + // Free: the parent event is already in hand from the root walk above, + // and unlike the frontend's cache it never misses. + parent_author: Some(parent.pubkey), + }; + + // Answering ourselves needs no addressing tag: it exists so the parent's + // author is notified, and we are not notified about our own writes. Applied + // here rather than at the call sites because a caller that forgets still + // reserves a slot against the mention cap, so an identical message succeeds + // as a stream reply and fails as a forum comment. `Some(author) == None` is + // false, so an unknown signer leaves the tag alone. + if thread_ref.parent_author == signer { + thread_ref.parent_author = None; + } + + Ok(thread_ref) } diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 7cb2d8e3b83..8a50664784c 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -20,6 +20,7 @@ mod clipboard; mod dms; mod engrams; mod export_util; +mod feed; mod global_agent_config; mod identity; mod identity_archive; @@ -89,6 +90,7 @@ pub use channels::*; pub use clipboard::*; pub use dms::*; pub use engrams::*; +pub use feed::*; pub use global_agent_config::*; pub use identity::*; pub use identity_archive::*; diff --git a/desktop/src-tauri/src/egress_guard_tests.rs b/desktop/src-tauri/src/egress_guard_tests.rs index 0c2a9573af6..33e51d5161f 100644 --- a/desktop/src-tauri/src/egress_guard_tests.rs +++ b/desktop/src-tauri/src/egress_guard_tests.rs @@ -160,7 +160,7 @@ fn boundary_huddle_stt_blocks_ncryptsec() { channel, NCRYPTSEC, None, - &[], + crate::events::Recipients::default(), &[], &[], &[], @@ -177,7 +177,7 @@ fn boundary_huddle_stt_blocks_ncryptsec() { channel, "hello huddle", None, - &[], + crate::events::Recipients::default(), &[], &[], &[], diff --git a/desktop/src-tauri/src/events.rs b/desktop/src-tauri/src/events.rs index 1828b3f5605..0be49096acd 100644 --- a/desktop/src-tauri/src/events.rs +++ b/desktop/src-tauri/src/events.rs @@ -8,14 +8,20 @@ //! //! Each function validates inputs and returns a nostr::EventBuilder. //! Signing and submission happen in relay::submit_event. -use buzz_core_pkg::kind::{KIND_IA_ARCHIVE_REQUEST, KIND_IA_UNARCHIVE_REQUEST}; use nostr::{EventBuilder, EventId, Kind, Tag}; use uuid::Uuid; +mod identity_archive; mod message_tags; +pub(crate) use identity_archive::{ + build_archive_identity_request, build_unarchive_identity_request, +}; +pub(crate) use message_tags::{Recipients, P_TAG_ADDRESSING_MARKER, P_TAG_MENTION_MARKER}; + use message_tags::{ - append_client_tags, append_sent_from_thread_tag, emoji_tags, imeta_tags, mention_reference_tags, + append_client_tags, append_sent_from_thread_tag, check_pubkey, emoji_tags, imeta_tags, + mention_reference_tags, mention_tags, reply_mention_tags, top_level_recipient_tags, }; // ── Constants ──────────────────────────────────────────────────────────────── @@ -49,6 +55,14 @@ fn check_content(content: &str) -> Result<(), String> { pub struct ThreadRef { pub root_event_id: EventId, pub parent_event_id: EventId, + /// Author of `parent_event_id`, when the resolver saw the parent event. + /// + /// Resolving the root already fetches the parent, so this costs nothing and + /// is more reliable than asking the sender: the client's cache misses for + /// any channel it has not opened, and a miss there used to mean the reply + /// shipped with no addressing tag at all — the exact delivery gap that + /// `p`-tagging the parent's author exists to close. + pub parent_author: Option, } fn thread_tags(tr: &ThreadRef) -> Result, String> { @@ -64,33 +78,6 @@ fn thread_tags(tr: &ThreadRef) -> Result, String> { } } -fn mention_tags(mentions: &[&str]) -> Result, String> { - if mentions.len() > MAX_MENTIONS { - return Err(format!("too many mentions (max {MAX_MENTIONS})")); - } - let mut seen = std::collections::HashSet::new(); - let mut tags = Vec::new(); - for &hex in mentions { - check_pubkey(hex)?; - let lower = hex.to_ascii_lowercase(); - if seen.insert(lower.clone()) { - tags.push(tag(vec!["p", &lower])?); - } - } - Ok(tags) -} - -/// Validate a hex pubkey is exactly 64 hex characters. -fn check_pubkey(pubkey: &str) -> Result<(), String> { - if pubkey.len() != 64 || !pubkey.chars().all(|c| c.is_ascii_hexdigit()) { - return Err(format!( - "pubkey must be a 64-character hex string (got {} chars)", - pubkey.len() - )); - } - Ok(()) -} - // ── Channel operations ─────────────────────────────────────────────────────── /// Kind 9007 — create channel. @@ -253,7 +240,7 @@ pub fn build_message( channel_id: Uuid, content: &str, thread_ref: Option<&ThreadRef>, - mentions: &[&str], + recipients: Recipients<'_>, media_tags: &[Vec], custom_emoji_tags: &[Vec], mention_ref_tags: &[Vec], @@ -265,7 +252,7 @@ pub fn build_message( channel_id, content, thread_ref, - mentions, + recipients, media_tags, custom_emoji_tags, mention_ref_tags, @@ -286,7 +273,7 @@ pub fn build_message_with_client_tags( channel_id: Uuid, content: &str, thread_ref: Option<&ThreadRef>, - mentions: &[&str], + recipients: Recipients<'_>, media_tags: &[Vec], custom_emoji_tags: &[Vec], mention_ref_tags: &[Vec], @@ -300,10 +287,17 @@ pub fn build_message_with_client_tags( } check_content(content)?; let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; - if let Some(tr) = thread_ref { - tags.extend(thread_tags(tr)?); + match thread_ref { + Some(tr) => { + tags.extend(thread_tags(tr)?); + tags.extend(reply_mention_tags( + recipients, + tr.parent_author.map(|pk| pk.to_hex()).as_deref(), + )?); + } + // Top-level: nothing to disambiguate, so no marker to add. + None => tags.extend(top_level_recipient_tags(recipients)?), } - tags.extend(mention_tags(mentions)?); imeta_tags(media_tags, &mut tags)?; emoji_tags(custom_emoji_tags, &mut tags)?; mention_reference_tags(mention_ref_tags, &mut tags)?; @@ -334,14 +328,17 @@ pub fn build_forum_comment( channel_id: Uuid, content: &str, thread_ref: &ThreadRef, - mentions: &[&str], + recipients: Recipients<'_>, media_tags: &[Vec], mention_ref_tags: &[Vec], ) -> Result { check_content(content)?; let mut tags = vec![tag(vec!["h", &channel_id.to_string()])?]; tags.extend(thread_tags(thread_ref)?); - tags.extend(mention_tags(mentions)?); + tags.extend(reply_mention_tags( + recipients, + thread_ref.parent_author.map(|pk| pk.to_hex()).as_deref(), + )?); imeta_tags(media_tags, &mut tags)?; mention_reference_tags(mention_ref_tags, &mut tags)?; Ok(EventBuilder::new(Kind::Custom(45003), content).tags(tags)) @@ -583,127 +580,6 @@ pub fn build_relay_admin_change_role( Ok(EventBuilder::new(Kind::Custom(9032), "").tags(tags)) } -// ── NIP-IA identity archival ───────────────────────────────────────────────── -// -// kind:9035 archive request, kind:9036 unarchive request. -// Both protected by NIP-70 (`["-"]`), p-tag the target, and may carry -// optional `reason` (machine-readable code), `replaced-by` (9035 only), -// and a NIP-OA `auth` tag for owner-of-agent requests. -// -// See docs/nips/NIP-IA.md §Event Formats. The relay verifies; the desktop's -// job is to produce a well-formed, signed request — consent path is selected -// by the relay, not declared here. - -fn check_reason(reason: &str) -> Result<(), String> { - // Reason codes are machine-readable strings; the spec doesn't cap length - // but we keep them short to discourage stuffing prose where `content` goes. - if reason.len() > 64 { - return Err(format!( - "reason code exceeds maximum length of 64 chars (got {})", - reason.len() - )); - } - if reason.chars().any(|c| c.is_control()) { - return Err("reason code must not contain control characters".into()); - } - Ok(()) -} - -fn identity_archive_tags( - target_pubkey: &str, - reason: Option<&str>, - replaced_by: Option<&str>, - auth_tag: Option<&[String; 4]>, -) -> Result, String> { - check_pubkey(target_pubkey)?; - let target_lower = target_pubkey.to_ascii_lowercase(); - - let mut tags = Vec::with_capacity(5); - // NIP-70: mark as protected administrative state. - tags.push(tag(vec!["-"])?); - tags.push(tag(vec!["p", &target_lower])?); - - if let Some(r) = reason { - check_reason(r)?; - tags.push(tag(vec!["reason", r])?); - } - - if let Some(rb) = replaced_by { - check_pubkey(rb)?; - let rb_lower = rb.to_ascii_lowercase(); - if rb_lower == target_lower { - return Err("replaced-by must differ from the target".into()); - } - tags.push(tag(vec!["replaced-by", &rb_lower])?); - } - - if let Some(auth) = auth_tag { - // Structural check only — the relay performs full NIP-OA verification. - // We require the label, a 64-hex owner pubkey, and a 128-hex signature. - if auth[0] != "auth" { - return Err(format!( - "auth tag label must be \"auth\" (got \"{}\")", - auth[0] - )); - } - check_pubkey(&auth[1])?; - if auth[3].len() != 128 || !auth[3].chars().all(|c| c.is_ascii_hexdigit()) { - return Err("auth tag signature must be 128-character hex".into()); - } - tags.push(tag(vec!["auth", &auth[1], &auth[2], &auth[3]])?); - } - - Ok(tags) -} - -/// Kind 9035 — NIP-IA archive request. -/// -/// `content` is an optional human-readable reason (clients MUST NOT parse -/// authorization semantics from it). `reason` is the machine-readable code -/// (`rotated`, `retired`, `bot-rebuilt`, `left-organization`, `spam`, ...). -/// `replaced_by` is the rotation pointer. `auth` is a NIP-OA owner-attestation -/// tag required only for the owner-of-agent consent path. -/// -/// `.allow_self_tagging()` is required: NIP-IA's self path has `actor==target`, -/// which means the request's `["p", target]` matches the signer. nostr 0.44 -/// strips matching `p` tags by default — we need the wire form intact. -pub fn build_archive_identity_request( - target_pubkey: &str, - content: &str, - reason: Option<&str>, - replaced_by: Option<&str>, - auth: Option<&[String; 4]>, -) -> Result { - check_content(content)?; - let tags = identity_archive_tags(target_pubkey, reason, replaced_by, auth)?; - Ok( - EventBuilder::new(Kind::Custom(KIND_IA_ARCHIVE_REQUEST as u16), content) - .tags(tags) - .allow_self_tagging(), - ) -} - -/// Kind 9036 — NIP-IA unarchive request. -/// -/// Same shape as 9035 minus `replaced-by` (which has no defined meaning on -/// unarchive per spec). `auth` is used for owner-of-agent unarchive paths. -/// See `build_archive_identity_request` for the rationale on -/// `.allow_self_tagging()`. -pub fn build_unarchive_identity_request( - target_pubkey: &str, - content: &str, - reason: Option<&str>, - auth: Option<&[String; 4]>, -) -> Result { - check_content(content)?; - let tags = identity_archive_tags(target_pubkey, reason, None, auth)?; - Ok( - EventBuilder::new(Kind::Custom(KIND_IA_UNARCHIVE_REQUEST as u16), content) - .tags(tags) - .allow_self_tagging(), - ) -} - /// Maximum contacts per contact list event. const MAX_CONTACTS: usize = 10_000; @@ -768,6 +644,7 @@ pub use workflows::{ #[cfg(test)] mod tests { use super::*; + use buzz_core_pkg::kind::{KIND_IA_ARCHIVE_REQUEST, KIND_IA_UNARCHIVE_REQUEST}; use nostr::Keys; #[test] fn channel_builders_reject_hash_only_names() { diff --git a/desktop/src-tauri/src/events/identity_archive.rs b/desktop/src-tauri/src/events/identity_archive.rs new file mode 100644 index 00000000000..7ab699c2d25 --- /dev/null +++ b/desktop/src-tauri/src/events/identity_archive.rs @@ -0,0 +1,129 @@ +//! NIP-IA identity archival requests — kind:9035 archive, kind:9036 unarchive. +//! +//! Split out of `events.rs` to keep that module under the file-size ratchet. + +use buzz_core_pkg::kind::{KIND_IA_ARCHIVE_REQUEST, KIND_IA_UNARCHIVE_REQUEST}; +use nostr::{EventBuilder, Kind, Tag}; + +use super::{check_content, message_tags::check_pubkey, tag}; + +// ── NIP-IA identity archival ───────────────────────────────────────────────── +// +// kind:9035 archive request, kind:9036 unarchive request. +// Both protected by NIP-70 (`["-"]`), p-tag the target, and may carry +// optional `reason` (machine-readable code), `replaced-by` (9035 only), +// and a NIP-OA `auth` tag for owner-of-agent requests. +// +// See docs/nips/NIP-IA.md §Event Formats. The relay verifies; the desktop's +// job is to produce a well-formed, signed request — consent path is selected +// by the relay, not declared here. + +fn check_reason(reason: &str) -> Result<(), String> { + // Reason codes are machine-readable strings; the spec doesn't cap length + // but we keep them short to discourage stuffing prose where `content` goes. + if reason.len() > 64 { + return Err(format!( + "reason code exceeds maximum length of 64 chars (got {})", + reason.len() + )); + } + if reason.chars().any(|c| c.is_control()) { + return Err("reason code must not contain control characters".into()); + } + Ok(()) +} + +fn identity_archive_tags( + target_pubkey: &str, + reason: Option<&str>, + replaced_by: Option<&str>, + auth_tag: Option<&[String; 4]>, +) -> Result, String> { + check_pubkey(target_pubkey)?; + let target_lower = target_pubkey.to_ascii_lowercase(); + + let mut tags = Vec::with_capacity(5); + // NIP-70: mark as protected administrative state. + tags.push(tag(vec!["-"])?); + tags.push(tag(vec!["p", &target_lower])?); + + if let Some(r) = reason { + check_reason(r)?; + tags.push(tag(vec!["reason", r])?); + } + + if let Some(rb) = replaced_by { + check_pubkey(rb)?; + let rb_lower = rb.to_ascii_lowercase(); + if rb_lower == target_lower { + return Err("replaced-by must differ from the target".into()); + } + tags.push(tag(vec!["replaced-by", &rb_lower])?); + } + + if let Some(auth) = auth_tag { + // Structural check only — the relay performs full NIP-OA verification. + // We require the label, a 64-hex owner pubkey, and a 128-hex signature. + if auth[0] != "auth" { + return Err(format!( + "auth tag label must be \"auth\" (got \"{}\")", + auth[0] + )); + } + check_pubkey(&auth[1])?; + if auth[3].len() != 128 || !auth[3].chars().all(|c| c.is_ascii_hexdigit()) { + return Err("auth tag signature must be 128-character hex".into()); + } + tags.push(tag(vec!["auth", &auth[1], &auth[2], &auth[3]])?); + } + + Ok(tags) +} + +/// Kind 9035 — NIP-IA archive request. +/// +/// `content` is an optional human-readable reason (clients MUST NOT parse +/// authorization semantics from it). `reason` is the machine-readable code +/// (`rotated`, `retired`, `bot-rebuilt`, `left-organization`, `spam`, ...). +/// `replaced_by` is the rotation pointer. `auth` is a NIP-OA owner-attestation +/// tag required only for the owner-of-agent consent path. +/// +/// `.allow_self_tagging()` is required: NIP-IA's self path has `actor==target`, +/// which means the request's `["p", target]` matches the signer. nostr 0.44 +/// strips matching `p` tags by default — we need the wire form intact. +pub(crate) fn build_archive_identity_request( + target_pubkey: &str, + content: &str, + reason: Option<&str>, + replaced_by: Option<&str>, + auth: Option<&[String; 4]>, +) -> Result { + check_content(content)?; + let tags = identity_archive_tags(target_pubkey, reason, replaced_by, auth)?; + Ok( + EventBuilder::new(Kind::Custom(KIND_IA_ARCHIVE_REQUEST as u16), content) + .tags(tags) + .allow_self_tagging(), + ) +} + +/// Kind 9036 — NIP-IA unarchive request. +/// +/// Same shape as 9035 minus `replaced-by` (which has no defined meaning on +/// unarchive per spec). `auth` is used for owner-of-agent unarchive paths. +/// See `build_archive_identity_request` for the rationale on +/// `.allow_self_tagging()`. +pub(crate) fn build_unarchive_identity_request( + target_pubkey: &str, + content: &str, + reason: Option<&str>, + auth: Option<&[String; 4]>, +) -> Result { + check_content(content)?; + let tags = identity_archive_tags(target_pubkey, reason, None, auth)?; + Ok( + EventBuilder::new(Kind::Custom(KIND_IA_UNARCHIVE_REQUEST as u16), content) + .tags(tags) + .allow_self_tagging(), + ) +} diff --git a/desktop/src-tauri/src/events/message_tags.rs b/desktop/src-tauri/src/events/message_tags.rs index 1d719beaa66..026031c3a23 100644 --- a/desktop/src-tauri/src/events/message_tags.rs +++ b/desktop/src-tauri/src/events/message_tags.rs @@ -1,6 +1,17 @@ use nostr::{EventId, Tag}; -use super::check_pubkey; +use super::{tag, MAX_MENTIONS}; + +/// Validate a hex pubkey is exactly 64 hex characters. +pub(super) fn check_pubkey(pubkey: &str) -> Result<(), String> { + if pubkey.len() != 64 || !pubkey.chars().all(|c| c.is_ascii_hexdigit()) { + return Err(format!( + "pubkey must be a 64-character hex string (got {} chars)", + pubkey.len() + )); + } + Ok(()) +} const MAX_THREAD_ROOT_EXCERPT_CHARS: usize = 64; const SENT_FROM_THREAD_TAG: &str = "buzz:sent-from-thread"; @@ -180,3 +191,325 @@ mod tests { assert!(append_sent_from_thread_tag(Some(&invalid_root_tag), &mut Vec::new()).is_err()); } } + +pub(super) fn mention_tags(mentions: &[&str]) -> Result, String> { + if mentions.len() > MAX_MENTIONS { + return Err(format!("too many mentions (max {MAX_MENTIONS})")); + } + let mut seen = std::collections::HashSet::new(); + let mut tags = Vec::new(); + for &hex in mentions { + check_pubkey(hex)?; + let lower = hex.to_ascii_lowercase(); + if seen.insert(lower.clone()) { + tags.push(tag(vec!["p", &lower])?); + } + } + Ok(tags) +} + +/// Marker on a `p` tag naming the author this reply answers. +/// +/// Mirrors `P_TAG_ADDRESSING_MARKER` in +/// `desktop/src/features/messages/lib/threading.ts`. +pub(crate) const P_TAG_ADDRESSING_MARKER: &str = "reply"; + +/// Marker on a `p` tag naming someone the author typed as `@name`. +/// +/// Mirrors `P_TAG_MENTION_MARKER` in the same TypeScript module. Distinct from +/// the `["mention", pk]` reference tag built by `mention_reference_tags`, which +/// is a different tag kind meaning "render the chip, do not notify". +pub(crate) const P_TAG_MENTION_MARKER: &str = "mention"; + +/// Who an outgoing message `p`-tags, split by *why* it tags them. +/// +/// The two lists are both `&[&str]` of pubkeys and mean different things on the +/// wire, so they are named rather than positional: folding one into the other is +/// exactly the mistake the role markers exist to prevent. +#[derive(Default, Clone, Copy)] +pub(crate) struct Recipients<'a> { + /// Written as `@name` in the body. Marked `mention` on a reply. + pub typed: &'a [&'a str], + /// Addressed by the *channel* rather than by the message — every other + /// participant in a DM, who is tagged whether or not anyone typed their + /// name. Never marked, because neither role is true of it. + pub addressed: &'a [&'a str], +} + +/// Bare `p` tags for a top-level message. +/// +/// Nothing to disambiguate without a parent, so a typed mention and a channel +/// recipient look the same here — which is what they both looked like before +/// markers existed. +pub(super) fn top_level_recipient_tags(recipients: Recipients<'_>) -> Result, String> { + let mut all: Vec<&str> = + Vec::with_capacity(recipients.typed.len() + recipients.addressed.len()); + all.extend_from_slice(recipients.typed); + all.extend_from_slice(recipients.addressed); + mention_tags(&all) +} + +/// `p` tags for a reply, each marked with the role it plays. +/// +/// NIP-10 addressing and a typed `@mention` are byte-identical as bare `p` +/// tags, which forces every receiver to fetch the parent and check who wrote it +/// just to tell them apart. The markers record what the sender already knows. +/// +/// Three roles, three shapes: +/// +/// - `recipients.typed` → `mention`. +/// - `parent_author` → `reply`. +/// - `recipients.addressed` → left **bare**. Neither marker is true of a DM +/// counterpart who did not write the parent and was never typed, and under the +/// one-way read a bare tag means "ask the parent" — the same answer these tags +/// got before markers existed. Claiming `mention` here would let a DM thread +/// reply pierce a mute and outrank a real `@you` in the mention feed. +/// +/// A pubkey that is both typed and the parent's author is emitted once, marked +/// as a mention — that is the case no amount of parent-fetching can decide, and +/// mention is the answer that preserves the stronger signal. Relay tag filters +/// match only the second element, so no marker affects `#p` delivery. +pub(super) fn reply_mention_tags( + recipients: Recipients<'_>, + parent_author: Option<&str>, +) -> Result, String> { + let mut seen = std::collections::HashSet::new(); + let mut lowered = Vec::new(); + for &hex in recipients.typed { + check_pubkey(hex)?; + let lower = hex.to_ascii_lowercase(); + if seen.insert(lower.clone()) { + lowered.push(lower); + } + } + + let addressing = match parent_author { + Some(hex) => { + check_pubkey(hex)?; + let lower = hex.to_ascii_lowercase(); + // Already typed in the body, so it is a mention and not merely + // addressing. Emitting both would double-tag the same pubkey. + if seen.contains(&lower) { + None + } else { + seen.insert(lower.clone()); + Some(lower) + } + } + None => None, + }; + + // A channel recipient who is also the parent's author is already covered by + // the addressing tag, and one who was typed is already a mention. + let mut bare = Vec::new(); + for &hex in recipients.addressed { + check_pubkey(hex)?; + let lower = hex.to_ascii_lowercase(); + if seen.insert(lower.clone()) { + bare.push(lower); + } + } + + // The addressing tag is the one that must survive the cap: without it an + // agent's `require_mention` subscription never receives the reply at all, + // while a dropped body mention only costs one notification. + let room = MAX_MENTIONS - usize::from(addressing.is_some()); + if lowered.len() + bare.len() > room { + return Err(format!("too many recipients (max {room} on a reply)")); + } + + let mut tags = Vec::new(); + for lower in &lowered { + tags.push(tag(vec!["p", lower, "", P_TAG_MENTION_MARKER])?); + } + for lower in &bare { + tags.push(tag(vec!["p", lower])?); + } + if let Some(lower) = &addressing { + tags.push(tag(vec!["p", lower, "", P_TAG_ADDRESSING_MARKER])?); + } + Ok(tags) +} + +#[cfg(test)] +mod reply_tag_tests { + use super::*; + + const PARENT_AUTHOR: &str = "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798"; + const TYPED: &str = "c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5"; + + const COUNTERPART: &str = "f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9"; + + fn tag_rows(tags: &[Tag]) -> Vec> { + tags.iter().map(|t| t.as_slice().to_vec()).collect() + } + + fn typed<'a>(pubkeys: &'a [&'a str]) -> Recipients<'a> { + Recipients { + typed: pubkeys, + addressed: &[], + } + } + + fn row(pubkey: &str, marker: Option<&str>) -> Vec { + match marker { + Some(marker) => vec![ + "p".to_string(), + pubkey.to_string(), + String::new(), + marker.to_string(), + ], + None => vec!["p".to_string(), pubkey.to_string()], + } + } + + #[test] + fn reply_marks_each_p_tag_with_the_role_it_plays() { + let tags = reply_mention_tags(typed(&[TYPED]), Some(PARENT_AUTHOR)).unwrap(); + assert_eq!( + tag_rows(&tags), + vec![ + vec![ + "p".to_string(), + TYPED.to_string(), + String::new(), + P_TAG_MENTION_MARKER.to_string() + ], + vec![ + "p".to_string(), + PARENT_AUTHOR.to_string(), + String::new(), + P_TAG_ADDRESSING_MARKER.to_string() + ], + ] + ); + } + + #[test] + fn a_typed_parent_author_is_tagged_once_as_a_mention() { + // The case no amount of parent-fetching can decide: the recipient is + // both the author being answered and someone typed in the body. Mention + // is the answer that keeps the stronger signal, and emitting an + // addressing tag as well would double-tag the same pubkey. + let tags = reply_mention_tags(typed(&[PARENT_AUTHOR]), Some(PARENT_AUTHOR)).unwrap(); + assert_eq!( + tag_rows(&tags), + vec![vec![ + "p".to_string(), + PARENT_AUTHOR.to_string(), + String::new(), + P_TAG_MENTION_MARKER.to_string() + ]] + ); + } + + #[test] + fn the_addressing_tag_keeps_its_slot_under_the_cap() { + // Past the cap the relay rejects the whole event rather than trimming. + // Losing the addressing tag costs an agent the reply entirely, so the + // body list is what has to give. + // Distinct, or dedup would collapse them and the cap would never trip. + let full: Vec = (1..=MAX_MENTIONS).map(|i| format!("{i:064x}")).collect(); + let refs: Vec<&str> = full.iter().map(String::as_str).collect(); + assert!(reply_mention_tags(typed(&refs), Some(PARENT_AUTHOR)).is_err()); + assert!(reply_mention_tags(typed(&refs[..MAX_MENTIONS - 1]), Some(PARENT_AUTHOR)).is_ok()); + } + + #[test] + fn a_top_level_message_leaves_its_p_tags_bare() { + // Nothing to disambiguate without a parent, so no marker is added and + // older readers see exactly what they saw before. + let tags = mention_tags(&[TYPED]).unwrap(); + assert_eq!( + tag_rows(&tags), + vec![vec!["p".to_string(), TYPED.to_string()]] + ); + } + + #[test] + fn a_channel_recipient_is_tagged_bare_not_as_a_mention() { + // A DM tags every other participant whether or not anyone typed their + // name. Marking that `mention` would be a lie the receivers act on: it + // pierces a mute and takes a slot in the mention feed ahead of a real + // `@you`. Bare means "ask the parent", which is the honest answer. + let tags = reply_mention_tags( + Recipients { + typed: &[], + addressed: &[COUNTERPART], + }, + None, + ) + .unwrap(); + assert_eq!(tag_rows(&tags), vec![row(COUNTERPART, None)]); + } + + #[test] + fn a_channel_recipient_who_wrote_the_parent_is_addressing() { + // The usual DM reply: the counterpart is both the channel's other + // participant and the author being answered. One tag, marked `reply`. + let tags = reply_mention_tags( + Recipients { + typed: &[], + addressed: &[COUNTERPART], + }, + Some(COUNTERPART), + ) + .unwrap(); + assert_eq!( + tag_rows(&tags), + vec![row(COUNTERPART, Some(P_TAG_ADDRESSING_MARKER))] + ); + } + + #[test] + fn a_channel_recipient_who_was_also_typed_is_a_mention() { + let tags = reply_mention_tags( + Recipients { + typed: &[COUNTERPART], + addressed: &[COUNTERPART], + }, + None, + ) + .unwrap(); + assert_eq!( + tag_rows(&tags), + vec![row(COUNTERPART, Some(P_TAG_MENTION_MARKER))] + ); + } + + #[test] + fn channel_recipients_count_against_the_cap_too() { + let full: Vec = (1..MAX_MENTIONS).map(|i| format!("{i:064x}")).collect(); + let refs: Vec<&str> = full.iter().map(String::as_str).collect(); + // MAX_MENTIONS - 1 typed + 1 bare + 1 addressing = one over. + assert!(reply_mention_tags( + Recipients { + typed: &refs, + addressed: &[COUNTERPART], + }, + Some(PARENT_AUTHOR) + ) + .is_err()); + assert!(reply_mention_tags( + Recipients { + typed: &refs, + addressed: &[COUNTERPART], + }, + None + ) + .is_ok()); + } + + #[test] + fn a_top_level_message_leaves_every_recipient_bare() { + let tags = top_level_recipient_tags(Recipients { + typed: &[TYPED], + addressed: &[COUNTERPART], + }) + .unwrap(); + assert_eq!( + tag_rows(&tags), + vec![row(TYPED, None), row(COUNTERPART, None)] + ); + } +} diff --git a/desktop/src-tauri/src/huddle/pipeline.rs b/desktop/src-tauri/src/huddle/pipeline.rs index 47d4aeb43d1..ed9b6e51c95 100644 --- a/desktop/src-tauri/src/huddle/pipeline.rs +++ b/desktop/src-tauri/src/huddle/pipeline.rs @@ -669,7 +669,11 @@ pub(crate) fn spawn_transcription_task( channel_uuid, &t, None, - &p_tags, + // The huddle addresses its agents; nobody typed their names. + events::Recipients { + addressed: &p_tags, + ..Default::default() + }, &[], &[], &[], diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 71a5eb3806e..98ae8381efc 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -32,6 +32,7 @@ mod native_websocket_batch; mod nostr_bind; pub mod nostr_convert; mod observed_unread; +mod p_tag_role; mod persona_catalog; mod prevent_sleep; mod ptt_shortcut; @@ -47,6 +48,8 @@ mod terminal_transport; #[cfg(target_os = "macos")] mod tray_menu; mod unread_catch_up; +mod unread_notify; +mod unread_parent_authors; mod util; #[cfg(target_os = "linux")] pub mod webkit_rendering; diff --git a/desktop/src-tauri/src/models.rs b/desktop/src-tauri/src/models.rs index 768b2ad7db3..e71b650413b 100644 --- a/desktop/src-tauri/src/models.rs +++ b/desktop/src-tauri/src/models.rs @@ -191,6 +191,12 @@ pub struct FeedItemInfo { pub channel_type: Option, pub tags: Vec>, pub category: String, + /// True when this item only reaches the user because it replies to one of + /// their messages. A reply `p`-tags the author it answers so agent + /// `require_mention` subscriptions receive it, which makes the mention + /// feed's `#p` query indistinguishable from a real mention without this. + #[serde(default)] + pub reply_to_self: bool, } #[derive(Serialize, Deserialize)] diff --git a/desktop/src-tauri/src/p_tag_role.rs b/desktop/src-tauri/src/p_tag_role.rs new file mode 100644 index 00000000000..51534648213 --- /dev/null +++ b/desktop/src-tauri/src/p_tag_role.rs @@ -0,0 +1,122 @@ +//! Reading the role marker a sender put on a `p` tag. +//! +//! NIP-10 addressing and a typed `@mention` are byte-identical as bare `p` +//! tags, so a reply marks each one with the role it plays. This module is the +//! single reader for that marker on the Tauri side; the writers live in +//! [`crate::events::message_tags`]. +//! +//! Mirrors `pTagRoleFor` in `desktop/src/features/messages/lib/threading.ts`. + +/// What the `p` tags naming a pubkey say this event is to them. +/// +/// [`PTagRole::Unknown`] must never be collapsed into either answer: a sender +/// that predates these markers emits a bare `p` tag for both roles, so an absent +/// marker means "ask the parent", not "this is a mention". +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub(crate) enum PTagRole { + Addressing, + Mention, + Unknown, + None, +} + +/// The role `pubkey` plays on this event, read from its `p` tags. +/// +/// Takes the tags as string slices so both `nostr::Event` (whose tags are +/// `Tag`) and the flat `Vec>` shape the native commands carry can +/// share one implementation. +pub(crate) fn p_tag_role<'a>( + tags: impl IntoIterator, + pubkey: &str, +) -> PTagRole { + let target = pubkey.to_ascii_lowercase(); + let mut saw_addressing = false; + let mut saw_bare = false; + for tag in tags { + if tag.len() < 2 || tag[0] != "p" || !tag[1].eq_ignore_ascii_case(&target) { + continue; + } + match tag.get(3).map(String::as_str) { + // Mention wins outright: it is the only marker a sender emits when + // the recipient is both the parent's author and typed in the body. + Some(crate::events::P_TAG_MENTION_MARKER) => return PTagRole::Mention, + Some(crate::events::P_TAG_ADDRESSING_MARKER) => saw_addressing = true, + _ => saw_bare = true, + } + } + if saw_bare { + return PTagRole::Unknown; + } + if saw_addressing { + PTagRole::Addressing + } else { + PTagRole::None + } +} + +/// [`p_tag_role`] for a signed event. +pub(crate) fn p_tag_role_for_event(ev: &nostr::Event, pubkey: &str) -> PTagRole { + p_tag_role(ev.tags.iter().map(|tag| tag.as_slice()), pubkey) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn tags(rows: &[&[&str]]) -> Vec> { + rows.iter() + .map(|row| row.iter().map(|part| (*part).to_string()).collect()) + .collect() + } + + fn role(rows: &[&[&str]], pubkey: &str) -> PTagRole { + let owned = tags(rows); + p_tag_role(owned.iter().map(|tag| tag.as_slice()), pubkey) + } + + const ME: &str = "aa"; + const OTHER: &str = "bb"; + + #[test] + fn a_marked_mention_is_a_mention() { + assert_eq!(role(&[&["p", ME, "", "mention"]], ME), PTagRole::Mention); + } + + #[test] + fn a_marked_addressing_tag_is_addressing() { + assert_eq!(role(&[&["p", ME, "", "reply"]], ME), PTagRole::Addressing); + } + + #[test] + fn a_bare_tag_is_unknown_not_a_mention() { + // The whole point of the one-way read: absent means "ask the parent". + assert_eq!(role(&[&["p", ME]], ME), PTagRole::Unknown); + } + + #[test] + fn mention_wins_over_addressing_on_the_same_pubkey() { + assert_eq!( + role(&[&["p", ME, "", "reply"], &["p", ME, "", "mention"]], ME), + PTagRole::Mention, + ); + } + + #[test] + fn a_bare_tag_alongside_a_marked_one_still_reads_unknown() { + // Two senders' shapes cannot be mixed into a confident answer. + assert_eq!( + role(&[&["p", ME, "", "reply"], &["p", ME]], ME), + PTagRole::Unknown, + ); + } + + #[test] + fn tags_naming_someone_else_are_ignored() { + assert_eq!(role(&[&["p", OTHER, "", "mention"]], ME), PTagRole::None); + } + + #[test] + fn matching_is_case_insensitive() { + assert_eq!(role(&[&["p", "AA", "", "mention"]], ME), PTagRole::Mention); + } +} diff --git a/desktop/src-tauri/src/unread_catch_up.rs b/desktop/src-tauri/src/unread_catch_up.rs index f8609ef1f60..fcf2eebddd3 100644 --- a/desktop/src-tauri/src/unread_catch_up.rs +++ b/desktop/src-tauri/src/unread_catch_up.rs @@ -17,7 +17,11 @@ use serde::{Deserialize, Serialize}; use tauri::{AppHandle, State}; use tokio::{sync::Semaphore, task::JoinSet}; -use crate::{app_state::AppState, native_relay_client::NativeRelayClient}; +use crate::{ + app_state::AppState, + native_relay_client::NativeRelayClient, + unread_notify::{has_authored_mention, is_high_priority, should_notify, NotifyGate}, +}; const CATCH_UP_LIMIT: usize = 1_000; const ACTIVITY_LIMIT: usize = 100; @@ -28,15 +32,15 @@ const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); pub(crate) struct UnreadCatchUpRequest { channels: Vec, self_pubkey: String, - muted_channel_ids: HashSet, + pub(crate) muted_channel_ids: HashSet, } #[derive(Clone, Deserialize)] #[serde(rename_all = "camelCase")] -struct CatchUpChannel { +pub(crate) struct CatchUpChannel { id: String, #[serde(rename = "type")] - channel_type: String, + pub(crate) channel_type: String, name: String, read_at: Option, } @@ -99,20 +103,20 @@ struct DiscoveredRoots { mentioned: Vec, } -struct FetchedChannel { - order: usize, - channel: CatchUpChannel, - events: Vec, +pub(crate) struct FetchedChannel { + pub(crate) order: usize, + pub(crate) channel: CatchUpChannel, + pub(crate) events: Vec, } #[derive(Clone)] -struct EventView { - id: String, - kind: u16, - pubkey: String, - content: String, - created_at: u64, - tags: Vec>, +pub(crate) struct EventView { + pub(crate) id: String, + pub(crate) kind: u16, + pub(crate) pubkey: String, + pub(crate) content: String, + pub(crate) created_at: u64, + pub(crate) tags: Vec>, } impl From for EventView { @@ -231,7 +235,36 @@ pub(crate) async fn unread_catch_up( relay_url, }, )?; - let mut channels = classify_batch(&request, fetched, &membership); + // Resolve the parents of replies that tag us, before classifying. A reply + // addresses the author it answers with a `p` tag byte-identical to a typed + // `@mention`, so without the parent's author this batch cannot tell "someone + // mentioned you" from "someone answered you" — and it persists the verdict. + // + // A failed lookup fails the whole batch rather than proceeding on a guess: + // the renderer releases the claim for an errored channel and retries, which + // is the recoverable outcome. Guessing marks threads mentioned forever. + let parent_authors = match crate::unread_parent_authors::resolve_parent_authors( + &session, + &fetched, + &request.self_pubkey, + ) + .await + { + Ok(authors) => authors, + Err(error) => { + let mut channels: Vec = fetched + .into_iter() + .map(|item| ChannelResult::Error { + channel_id: item.channel.id, + error: format!("reply parent lookup failed: {error}"), + }) + .collect(); + channels.extend(failures); + return Ok(UnreadCatchUpResponse { channels }); + } + }; + + let mut channels = classify_batch(&request, fetched, &membership, &parent_authors); channels.extend(failures); Ok(UnreadCatchUpResponse { channels }) } @@ -240,6 +273,7 @@ fn classify_batch( request: &UnreadCatchUpRequest, fetched: Vec, membership: &std::collections::HashMap>, + parent_authors: &std::collections::HashMap, ) -> Vec { let self_pubkey = request.self_pubkey.to_lowercase(); let mut participated = membership.get("participated").cloned().unwrap_or_default(); @@ -262,7 +296,11 @@ fn classify_batch( } else if authored.insert(event.id.clone()) { discovered.authored.push(event.id.clone()); } - } else if has_tag_value(&event.tags, "p", &self_pubkey) { + } else if has_authored_mention( + &event.tags, + &self_pubkey, + parent_author_for(&item.channel, &event.tags, parent_authors), + ) { if let Some(root_id) = thread_reference(&event.tags).root_id { if mentioned.insert(root_id.clone()) { discovered.mentioned.push(root_id); @@ -288,10 +326,15 @@ fn classify_batch( || !should_notify( &event, &self_pubkey, - request, - membership, - &participated, - &authored, + &NotifyGate { + muted_channel_ids: &request.muted_channel_ids, + membership, + participated: &participated, + authored: &authored, + mentioned: &mentioned, + }, + item.channel.channel_type == "dm", + parent_author_for(&item.channel, &event.tags, parent_authors), ) { continue; @@ -300,8 +343,11 @@ fn classify_batch( let broadcast = has_exact_tag(&event.tags, "broadcast", "1"); let threaded = reference.parent_id.is_some() && !broadcast; let high_priority = item.channel.channel_type == "dm" - || broadcast - || has_tag_value(&event.tags, "p", &self_pubkey); + || is_high_priority( + &event.tags, + &self_pubkey, + parent_author_for(&item.channel, &event.tags, parent_authors), + ); max_trigger = max_trigger.max(event.created_at); observed_events.push(ObservedUnreadEvent { id: event.id.clone(), @@ -364,12 +410,12 @@ fn classify_batch( .collect() } -struct ThreadReference { - parent_id: Option, - root_id: Option, +pub(crate) struct ThreadReference { + pub(crate) parent_id: Option, + pub(crate) root_id: Option, } -fn thread_reference(tags: &[Vec]) -> ThreadReference { +pub(crate) fn thread_reference(tags: &[Vec]) -> ThreadReference { let event_tags: Vec<_> = tags .iter() .filter(|tag| tag.first().is_some_and(|v| v == "e") && tag.get(1).is_some()) @@ -396,53 +442,31 @@ fn thread_reference(tags: &[Vec]) -> ThreadReference { } } -fn should_notify( - event: &EventView, - self_pubkey: &str, - request: &UnreadCatchUpRequest, - membership: &std::collections::HashMap>, - participated: &HashSet, - authored: &HashSet, -) -> bool { - if has_exact_tag(&event.tags, "broadcast", "1") || has_tag_value(&event.tags, "p", self_pubkey) - { - return true; - } - let event_channel_id = event - .tags - .iter() - .find(|tag| tag.first().is_some_and(|part| part == "h")) - .and_then(|tag| tag.get(1)); - if event_channel_id.is_some_and(|id| request.muted_channel_ids.contains(id)) { - return false; - } - let reference = thread_reference(&event.tags); - if reference.parent_id.is_none() { - return true; - } - let Some(root_id) = reference.root_id else { - return false; - }; - if membership - .get("muted_root") - .is_some_and(|set| set.contains(&root_id)) - { - return false; +/// The author of the message this event answers, or `None` when there is no +/// parent, the lookup did not resolve it, or the channel is a DM. +/// +/// DMs are excluded the same way `needsResolvedParentAuthor` excludes them: +/// every DM message `p`-tags both participants, so no consumer reads the +/// parent's author there. +fn parent_author_for<'a>( + channel: &CatchUpChannel, + tags: &[Vec], + parent_authors: &'a std::collections::HashMap, +) -> Option<&'a str> { + if channel.channel_type == "dm" { + return None; } - participated.contains(&root_id) - || membership - .get("followed") - .is_some_and(|set| set.contains(&root_id)) - || authored.contains(&root_id) + let parent_id = thread_reference(tags).parent_id?; + parent_authors.get(&parent_id).map(String::as_str) } -fn has_exact_tag(tags: &[Vec], name: &str, value: &str) -> bool { +pub(crate) fn has_exact_tag(tags: &[Vec], name: &str, value: &str) -> bool { tags.iter().any(|tag| { tag.first().is_some_and(|part| part == name) && tag.get(1).is_some_and(|part| part == value) }) } -fn has_tag_value(tags: &[Vec], name: &str, value: &str) -> bool { +pub(crate) fn has_tag_value(tags: &[Vec], name: &str, value: &str) -> bool { tags.iter().any(|tag| { tag.first().is_some_and(|part| part == name) && tag @@ -452,7 +476,7 @@ fn has_tag_value(tags: &[Vec], name: &str, value: &str) -> bool { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use std::collections::HashMap; use super::*; @@ -506,7 +530,7 @@ mod tests { ), ], }]; - let result = classify_batch(&req, fetched, &HashMap::new()); + let result = classify_batch(&req, fetched, &HashMap::new(), &HashMap::new()); let ChannelResult::Success { observed_events, discovered, @@ -555,7 +579,7 @@ mod tests { ), ], }]; - let result = classify_batch(&req, fetched, &membership); + let result = classify_batch(&req, fetched, &membership, &HashMap::new()); let ChannelResult::Success { observed_events, max_trigger, @@ -665,4 +689,161 @@ mod tests { assert_eq!(actual, expected); } + + // ---- Role markers: what a `p` tag on a reply actually claims ---------- + // + // Every case below turns on the same ambiguity: a reply `p`-tags the author + // it answers, and that tag is byte-identical to a typed `@mention`. Reading + // it as a mention marks the thread mentioned forever (persisted) and makes + // the channel high-priority, which drops its top-level items out of the dock + // badge. + + pub(crate) fn stream_channel() -> CatchUpChannel { + CatchUpChannel { + id: "ch".into(), + channel_type: "stream".into(), + name: "Ch".into(), + read_at: None, + } + } + + pub(crate) fn dm_channel() -> CatchUpChannel { + CatchUpChannel { + id: "dm".into(), + channel_type: "dm".into(), + name: "DM".into(), + read_at: None, + } + } + + /// Classify one channel's batch and return its discovered mention roots. + fn mentioned_roots( + channel: CatchUpChannel, + events: Vec, + parent_authors: &[(&str, &str)], + ) -> Vec { + let authors: HashMap = parent_authors + .iter() + .map(|(id, author)| ((*id).to_string(), (*author).to_string())) + .collect(); + let fetched = vec![FetchedChannel { + order: 0, + channel, + events, + }]; + let result = classify_batch(&request(), fetched, &HashMap::new(), &authors); + let ChannelResult::Success { discovered, .. } = &result[0] else { + panic!("expected success") + }; + discovered.mentioned.clone() + } + + fn reply_tags<'a>(parent: &'a str, p_tag: &[&'a str]) -> Vec> { + let mut tags: Vec> = vec![ + vec!["e".into(), parent.into(), String::new(), "reply".into()], + vec!["h".into(), "ch".into()], + ]; + tags.push(p_tag.iter().map(|part| (*part).to_string()).collect()); + tags + } + + pub(crate) fn reply(id: &str, author: &str, parent: &str, p_tag: &[&str]) -> EventView { + EventView { + id: id.into(), + kind: 9, + pubkey: author.into(), + content: id.into(), + created_at: 10, + tags: reply_tags(parent, p_tag), + } + } + + #[test] + fn a_bare_p_tag_on_a_reply_answering_us_is_not_a_mention() { + // The regression: this reply tags us only because NIP-10 addressing + // names the author it answers. Recorded as a mention, the thread renders + // as "Following" while nothing ever notifies for it. + assert!(mentioned_roots( + stream_channel(), + vec![reply("r", "other", "parent", &["p", "self"])], + &[("parent", "self")], + ) + .is_empty()); + } + + #[test] + fn a_bare_p_tag_on_a_reply_answering_someone_else_is_a_mention() { + assert_eq!( + mentioned_roots( + stream_channel(), + vec![reply("r", "other", "parent", &["p", "self"])], + &[("parent", "third-party")], + ), + vec!["parent".to_string()], + ); + } + + #[test] + fn a_mention_marker_beats_the_parent_author() { + // The case the parent cannot decide: we wrote the message being answered + // *and* the sender typed our name. Mention is the stronger signal. + assert_eq!( + mentioned_roots( + stream_channel(), + vec![reply("r", "other", "parent", &["p", "self", "", "mention"])], + &[("parent", "self")], + ), + vec!["parent".to_string()], + ); + } + + #[test] + fn an_addressing_marker_needs_no_parent_lookup_to_be_demoted() { + // The point of the markers: the sender already knew, so no round trip. + assert!(mentioned_roots( + stream_channel(), + vec![reply("r", "other", "parent", &["p", "self", "", "reply"])], + &[], + ) + .is_empty()); + } + + #[test] + fn an_unresolved_parent_still_reads_as_a_mention() { + // Fails open, matching `hasAuthoredMentionForEvent`: notification + // delivery would rather over-report than silently drop a real mention. + assert_eq!( + mentioned_roots( + stream_channel(), + vec![reply("r", "other", "parent", &["p", "self"])], + &[], + ), + vec!["parent".to_string()], + ); + } + + #[test] + fn a_dm_reply_answering_us_is_never_demoted() { + // Every DM message p-tags both participants, so the addressing tag is + // simply how a DM is addressed. Demoting it would silence answers to us + // while letting new messages through. + let mut event = reply("r", "other", "parent", &["p", "self"]); + event.tags[1] = vec!["h".into(), "dm".into()]; + assert_eq!( + mentioned_roots(dm_channel(), vec![event], &[("parent", "self")]), + vec!["parent".to_string()], + ); + } + + #[test] + fn a_broadcast_reply_answering_us_is_still_a_mention() { + // `should_notify` admits a broadcast reply before it ever reads the + // parent, so demoting it here would leave the surfaces disagreeing. + let mut event = reply("r", "other", "parent", &["p", "self"]); + event.tags.push(vec!["broadcast".into(), "1".into()]); + assert_eq!( + mentioned_roots(stream_channel(), vec![event], &[("parent", "self")]), + vec!["parent".to_string()], + ); + } } diff --git a/desktop/src-tauri/src/unread_notify.rs b/desktop/src-tauri/src/unread_notify.rs new file mode 100644 index 00000000000..4146a23dff7 --- /dev/null +++ b/desktop/src-tauri/src/unread_notify.rs @@ -0,0 +1,282 @@ +//! The gates that decide what a `p` tag on a reply means. +//! +//! A reply `p`-tags the author it answers, and that tag is byte-identical to a +//! typed `@mention`. Every gate here exists to tell those apart: from the role +//! marker the sender left, and failing that from the parent's author. +//! +//! Mirrors `desktop/src/features/notifications/lib/shouldNotify.ts`. The two +//! decide notification ownership independently from the same question, so a +//! divergence here makes them disagree and notify twice — or not at all. + +use std::collections::HashSet; + +use crate::p_tag_role::{p_tag_role, PTagRole}; +use crate::unread_catch_up::{has_exact_tag, has_tag_value, thread_reference, EventView}; + +/// Whether this event names `self_pubkey` in a `p` tag at all. +/// +/// Mirrors `hasMentionForEvent` in +/// `desktop/src/features/notifications/lib/shouldNotify.ts`. Necessary but not +/// sufficient for a mention: a reply addresses the author it answers with the +/// same tag. +pub(crate) fn has_p_tag_for(tags: &[Vec], self_pubkey: &str) -> bool { + has_tag_value(tags, "p", self_pubkey) +} + +pub(crate) fn role_for(tags: &[Vec], self_pubkey: &str) -> PTagRole { + p_tag_role(tags.iter().map(|tag| tag.as_slice()), self_pubkey) +} + +/// Whether someone actually mentioned the user, as opposed to answering them. +/// +/// Mirrors `hasAuthoredMentionForEvent` in `shouldNotify.ts`. `parent_author` is +/// `None` for a DM, where every message `p`-tags both participants and the +/// addressing tag *is* the addressing — demoting it there would silence answers +/// to the user while letting new messages through. +pub(crate) fn has_authored_mention( + tags: &[Vec], + self_pubkey: &str, + parent_author: Option<&str>, +) -> bool { + if !has_p_tag_for(tags, self_pubkey) { + return false; + } + // A broadcast reply is addressed to the channel, not just to the parent's + // author, and `should_notify` admits it before ever reading the parent. + if has_exact_tag(tags, "broadcast", "1") { + return true; + } + // The sender's own answer, when it gave one. Only a marker that is actually + // present is authoritative; `Unknown` and `None` fall through to the parent. + match role_for(tags, self_pubkey) { + PTagRole::Mention => return true, + PTagRole::Addressing => return false, + PTagRole::Unknown | PTagRole::None => {} + } + let reference = thread_reference(tags); + !(reference.parent_id.is_some() + && parent_author.is_some_and(|author| author.eq_ignore_ascii_case(self_pubkey))) +} + +/// Mirrors `isHighPriorityEventForUser` in `shouldNotify.ts`. +/// +/// Fails closed where notification delivery fails open: the parent is resolved +/// for exactly the replies that tag the user, so an unresolved parent here means +/// the lookup was tried and failed. This flag is persisted and makes the +/// channel's top-level items drop out of the dock badge, so guessing "high" +/// after a relay flap would silently hide them. +pub(crate) fn is_high_priority( + tags: &[Vec], + self_pubkey: &str, + parent_author: Option<&str>, +) -> bool { + if has_exact_tag(tags, "broadcast", "1") { + return true; + } + if !has_p_tag_for(tags, self_pubkey) { + return false; + } + match role_for(tags, self_pubkey) { + PTagRole::Mention => return true, + PTagRole::Addressing => return false, + PTagRole::Unknown | PTagRole::None => {} + } + if thread_reference(tags).parent_id.is_some() && parent_author.is_none() { + return false; + } + has_authored_mention(tags, self_pubkey, parent_author) +} + +/// The sets the notify gate consults, gathered once per batch. +pub(crate) struct NotifyGate<'a> { + /// Channels the user muted. The only thing the gate reads off the request. + pub(crate) muted_channel_ids: &'a HashSet, + pub(crate) membership: &'a std::collections::HashMap>, + pub(crate) participated: &'a HashSet, + pub(crate) authored: &'a HashSet, + pub(crate) mentioned: &'a HashSet, +} + +/// Mirrors `shouldNotifyForEvent` in +/// `desktop/src/features/notifications/lib/shouldNotify.ts`. +pub(crate) fn should_notify( + event: &EventView, + self_pubkey: &str, + gate: &NotifyGate<'_>, + is_dm: bool, + parent_author: Option<&str>, +) -> bool { + if has_exact_tag(&event.tags, "broadcast", "1") { + return true; + } + let reference = thread_reference(&event.tags); + // A reply we authored the parent of always carries our `p` tag, so that tag + // alone cannot mean "this message mentions you". Only a real mention skips + // the mute gates below; a reply answering us is re-admitted after them. + // Never in a DM: there the addressing tag is the whole point, so demoting it + // would silence answers to us while letting new messages through. + let is_reply_to_self = !is_dm + && reference.parent_id.is_some() + && !self_pubkey.is_empty() + && match role_for(&event.tags, self_pubkey) { + PTagRole::Addressing => true, + // The case the parent cannot decide: the recipient is both the + // author being answered and someone typed in the body. + PTagRole::Mention => false, + PTagRole::Unknown | PTagRole::None => { + parent_author.is_some_and(|author| author.eq_ignore_ascii_case(self_pubkey)) + } + }; + if !is_reply_to_self && has_p_tag_for(&event.tags, self_pubkey) { + return true; + } + let event_channel_id = event + .tags + .iter() + .find(|tag| tag.first().is_some_and(|part| part == "h")) + .and_then(|tag| tag.get(1)); + if event_channel_id.is_some_and(|id| gate.muted_channel_ids.contains(id)) { + return false; + } + if reference.parent_id.is_none() { + return true; + } + let Some(root_id) = reference.root_id else { + return false; + }; + if gate + .membership + .get("muted_root") + .is_some_and(|set| set.contains(&root_id)) + { + return false; + } + // Past the mute gates, a reply answering us always notifies. The + // participated/authored sets are local and rebuilt from the unread window, + // so on a fresh install they can be empty for a thread we started. + if is_reply_to_self { + return true; + } + gate.participated.contains(&root_id) + || gate.membership + .get("followed") + .is_some_and(|set| set.contains(&root_id)) + || gate.authored.contains(&root_id) + // Below the mute gates on purpose: being mentioned in a thread + // subscribes you to it, but muting it afterwards still wins. + || gate.mentioned.contains(&root_id) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + + fn reply_tags<'a>(parent: &'a str, p_tag: &[&'a str]) -> Vec> { + vec![ + vec!["e".into(), parent.into(), String::new(), "reply".into()], + vec!["h".into(), "ch".into()], + p_tag.iter().map(|part| (*part).to_string()).collect(), + ] + } + + fn event_with(tags: Vec>) -> EventView { + EventView { + id: "r".into(), + kind: 9, + pubkey: "other".into(), + content: "r".into(), + created_at: 10, + tags, + } + } + + #[test] + fn high_priority_fails_closed_when_the_parent_is_unresolved() { + // This flag is persisted and drops the channel's top-level items from + // the dock badge. Missing a red dot after a relay flap is recoverable; + // silently hiding an approval request until the channel is read is not. + assert!(!is_high_priority( + &reply_tags("parent", &["p", "self"]), + "self", + None, + )); + } + + #[test] + fn high_priority_holds_for_a_reply_answering_someone_else() { + assert!(is_high_priority( + &reply_tags("parent", &["p", "self"]), + "self", + Some("third-party"), + )); + } + + #[test] + fn high_priority_is_dropped_for_a_reply_answering_us() { + assert!(!is_high_priority( + &reply_tags("parent", &["p", "self"]), + "self", + Some("self"), + )); + } + + #[test] + fn high_priority_reads_the_marker_before_the_parent() { + assert!(is_high_priority( + &reply_tags("parent", &["p", "self", "", "mention"]), + "self", + Some("self"), + )); + assert!(!is_high_priority( + &reply_tags("parent", &["p", "self", "", "reply"]), + "self", + Some("third-party"), + )); + } + + // ---- The mute gate ---------------------------------------------------- + + fn notifies(p_tag: &[&str], parent_author: Option<&str>, muted_channel: bool) -> bool { + let mut muted: HashSet = HashSet::new(); + if muted_channel { + muted.insert("ch".to_string()); + } + let event = event_with(reply_tags("parent", p_tag)); + should_notify( + &event, + "self", + &NotifyGate { + muted_channel_ids: &muted, + membership: &HashMap::new(), + participated: &HashSet::new(), + authored: &HashSet::new(), + mentioned: &HashSet::new(), + }, + false, + parent_author, + ) + } + + #[test] + fn a_reply_answering_us_does_not_pierce_a_muted_channel() { + // Only a real mention skips the mute gate. Before the markers this tag + // pierced it, which is what made a muted channel keep notifying. + assert!(!notifies(&["p", "self"], Some("self"), true)); + assert!(!notifies(&["p", "self", "", "reply"], None, true)); + } + + #[test] + fn a_real_mention_still_pierces_a_muted_channel() { + assert!(notifies(&["p", "self"], Some("third-party"), true)); + assert!(notifies(&["p", "self", "", "mention"], Some("self"), true)); + } + + #[test] + fn a_reply_answering_us_still_notifies_in_an_unmuted_channel() { + // Re-admitted after the mute gates: the participated/authored sets are + // rebuilt from the unread window and can be empty for our own thread. + assert!(notifies(&["p", "self"], Some("self"), false)); + } +} diff --git a/desktop/src-tauri/src/unread_parent_authors.rs b/desktop/src-tauri/src/unread_parent_authors.rs new file mode 100644 index 00000000000..9d2faa204bb --- /dev/null +++ b/desktop/src-tauri/src/unread_parent_authors.rs @@ -0,0 +1,218 @@ +//! Resolving the authors of the messages a batch's replies answer. +//! +//! Split out of `unread_catch_up` so the round-trip decision — which replies +//! are ambiguous enough to be worth asking the relay about — sits next to the +//! request that acts on it, and can be tested without a live session. + +use std::collections::HashSet; +use std::time::Duration; + +use buzz_core_pkg::kind::{ + KIND_FORUM_COMMENT, KIND_FORUM_POST, KIND_STREAM_MESSAGE, KIND_STREAM_MESSAGE_V2, +}; + +use crate::p_tag_role::PTagRole; +use crate::unread_catch_up::{thread_reference, FetchedChannel}; +use crate::unread_notify::{has_p_tag_for, role_for}; + +/// Kinds a reply's parent can be, for looking one up by id. +/// +/// Deliberately wider than the catch-up kinds: a reply can answer a diff +/// message (40008), a legacy stream message (40001), or a NIP-01 note bridged in +/// from another client (1). A kind missing here leaves the parent unresolved, +/// which reads as a mention and pierces the mute this lookup exists to protect. +/// +/// Keep in sync with `REPLY_PARENT_EVENT_KINDS` in +/// `desktop/src/shared/constants/kinds.ts`. +const REPLY_PARENT_KINDS: &[u32] = &[ + KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_V2, + KIND_FORUM_POST, + KIND_FORUM_COMMENT, + 1, + 40001, + 40008, +]; +const PARENT_LOOKUP_CHUNK: usize = 200; +const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); + +/// Parent ids worth a round trip, given what the batch already answers. +/// +/// A reply is ambiguous only when its `p` tag for the user carries no role +/// marker. Every consumer of the resolved map — `has_authored_mention`, +/// `is_high_priority`, `should_notify` — reads `parent_author` in the unmarked +/// arm alone, so a marked reply's parent would be fetched and never consulted. +/// Skipping it here is where the markers actually save the round trip they were +/// introduced to remove. +/// +/// A DM is excluded wholesale: there every message `p`-tags both participants, +/// so the addressing tag is the addressing and no consumer asks for a parent. +fn wanted_parent_ids( + fetched: &[FetchedChannel], + self_pubkey: &str, + known: &std::collections::HashMap, +) -> HashSet { + let mut wanted: HashSet = HashSet::new(); + for item in fetched { + if item.channel.channel_type == "dm" { + continue; + } + for event in &item.events { + if event.pubkey.eq_ignore_ascii_case(self_pubkey) + || !has_p_tag_for(&event.tags, self_pubkey) + { + continue; + } + if !matches!( + role_for(&event.tags, self_pubkey), + PTagRole::Unknown | PTagRole::None + ) { + continue; + } + if let Some(parent_id) = thread_reference(&event.tags).parent_id { + if !known.contains_key(&parent_id) && is_event_id_hex(&parent_id) { + wanted.insert(parent_id); + } + } + } + } + wanted +} + +/// Authors of the messages this batch's replies answer, keyed by parent id. +/// +/// Only replies whose `p` tag is genuinely ambiguous are worth the round trip. +/// Parents already present in the batch are answered locally. +pub(crate) async fn resolve_parent_authors( + session: &crate::native_relay_client::SessionLease, + fetched: &[FetchedChannel], + self_pubkey: &str, +) -> Result, String> { + let self_pubkey = self_pubkey.to_lowercase(); + let mut authors: std::collections::HashMap = std::collections::HashMap::new(); + for item in fetched { + for event in &item.events { + authors.insert(event.id.clone(), event.pubkey.clone()); + } + } + let wanted = wanted_parent_ids(fetched, &self_pubkey, &authors); + if wanted.is_empty() { + return Ok(authors); + } + let ids: Vec = wanted.into_iter().collect(); + for chunk in ids.chunks(PARENT_LOOKUP_CHUNK) { + let filter = serde_json::json!({ + "kinds": REPLY_PARENT_KINDS, + "ids": chunk, + "limit": chunk.len(), + }); + let events = session + .handle() + .fetch_events(filter, REQUEST_TIMEOUT) + .await?; + for event in events { + authors.insert(event.id.to_hex(), event.pubkey.to_hex()); + } + } + Ok(authors) +} + +/// Whether a string is a well-formed 64-char hex event id, and so safe to put in +/// an `ids` filter. A relay rejects the whole REQ over one malformed id. +fn is_event_id_hex(value: &str) -> bool { + value.len() == 64 && value.chars().all(|c| c.is_ascii_hexdigit()) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + use crate::unread_catch_up::tests::{dm_channel, reply, stream_channel}; + use crate::unread_catch_up::{CatchUpChannel, EventView}; + + const PARENT: &str = "aa11bb22cc33dd44ee55ff6600778899aabbccddeeff00112233445566778899"; + const OTHER_PARENT: &str = "bb11cc22dd33ee44ff5500667788990011223344556677889900aabbccddeeff"; + + /// One channel's batch, seeded with whatever the batch already answers. + fn wanted( + channel: CatchUpChannel, + events: Vec, + known: &[(&str, &str)], + ) -> Vec { + let known: HashMap = known + .iter() + .map(|(id, author)| ((*id).to_string(), (*author).to_string())) + .collect(); + let fetched = vec![FetchedChannel { + order: 0, + channel, + events, + }]; + let mut ids: Vec = wanted_parent_ids(&fetched, "self", &known) + .into_iter() + .collect(); + ids.sort(); + ids + } + + #[test] + fn an_unmarked_p_tag_needs_its_parent_fetched() { + // The only ambiguous shape: a pre-marker sender's reply. Nothing in the + // event says whether we were typed or merely answered. + let events = vec![reply("r1", "other", PARENT, &["p", "self"])]; + assert_eq!(wanted(stream_channel(), events, &[]), vec![PARENT]); + } + + #[test] + fn a_marked_reply_costs_no_round_trip() { + // `role_for` already answers both of these, so fetching the parent + // would resolve an author no consumer goes on to read. + let addressing = vec![reply("r1", "other", PARENT, &["p", "self", "", "reply"])]; + assert!(wanted(stream_channel(), addressing, &[]).is_empty()); + let mention = vec![reply( + "r2", + "other", + OTHER_PARENT, + &["p", "self", "", "mention"], + )]; + assert!(wanted(stream_channel(), mention, &[]).is_empty()); + } + + #[test] + fn one_unmarked_reply_in_a_marked_batch_is_still_fetched() { + // The skip is per event, not per batch: a mixed window must not let a + // marked reply suppress the lookup an unmarked one needs. + let events = vec![ + reply("r1", "other", PARENT, &["p", "self", "", "reply"]), + reply("r2", "other", OTHER_PARENT, &["p", "self"]), + ]; + assert_eq!(wanted(stream_channel(), events, &[]), vec![OTHER_PARENT]); + } + + #[test] + fn a_dm_never_asks_for_a_parent() { + let events = vec![reply("r1", "other", PARENT, &["p", "self"])]; + assert!(wanted(dm_channel(), events, &[]).is_empty()); + } + + #[test] + fn a_parent_already_in_the_batch_is_answered_locally() { + let events = vec![reply("r1", "other", PARENT, &["p", "self"])]; + assert!(wanted(stream_channel(), events, &[(PARENT, "other")]).is_empty()); + } + + #[test] + fn our_own_reply_needs_no_parent() { + let events = vec![reply("r1", "self", PARENT, &["p", "other"])]; + assert!(wanted(stream_channel(), events, &[]).is_empty()); + } + + #[test] + fn a_malformed_parent_id_is_never_put_in_a_filter() { + // The relay accepts a junk `e` tag; feeding it back as an `ids` filter + // earns a bare NOTICE and a hung request. + let events = vec![reply("r1", "other", "not-hex", &["p", "self"])]; + assert!(wanted(stream_channel(), events, &[]).is_empty()); + } +} From 6518642ee0cbab9346f27f89792f2c679c480e80 Mon Sep 17 00:00:00 2001 From: cyberzero000 Date: Sat, 22 Aug 2026 11:50:20 -0700 Subject: [PATCH 2/3] feat(mobile): mark what a reply's p tags mean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_buildReplyTags` emitted `e` tags for the thread and `p` tags for explicit mentions, but never a `p` tag for the author being replied to, so a reply to an agent never reached it — `require_mention` subscriptions filter on `#p` relay-side, and the agent cannot compensate for an event it never receives. Adding that tag makes it indistinguishable from a typed `@mention`, so both channel and forum providers now mark each `p` tag with its role, matching the backend and `buzz-sdk`: ["p", , "", "mention"] ["p", , "", "reply"] ["p", ] `message_recipients.dart` replaces `message_mention_pubkeys.dart`: it returns the two groups separately rather than one merged list, because merging them is the mistake the markers exist to prevent. Emission order — mentions, then bare, then addressing — matches the Rust and TypeScript senders, so a reply's tags are byte-identical whichever client sent it. Mobile never had the eligibility half of this bug: it already resolves `@name` against relay membership rather than the agent's self-declared `channel_ids`. Signed-off-by: cyberzero000 --- .../channels/message_mention_pubkeys.dart | 26 -- .../features/channels/message_recipients.dart | 50 ++++ .../channels/send_message_provider.dart | 57 ++++- .../features/channels/thread_detail_page.dart | 1 + mobile/lib/features/forum/forum_provider.dart | 30 ++- .../lib/features/forum/forum_thread_page.dart | 1 + mobile/lib/shared/mentions/mention_tags.dart | 12 + .../message_mention_pubkeys_test.dart | 82 ------- .../channels/message_recipients_test.dart | 90 +++++++ .../channels/send_message_provider_test.dart | 226 ++++++++++++++++++ .../forum/forum_reply_delivery_test.dart | 199 +++++++++++++++ 11 files changed, 656 insertions(+), 118 deletions(-) delete mode 100644 mobile/lib/features/channels/message_mention_pubkeys.dart create mode 100644 mobile/lib/features/channels/message_recipients.dart delete mode 100644 mobile/test/features/channels/message_mention_pubkeys_test.dart create mode 100644 mobile/test/features/channels/message_recipients_test.dart create mode 100644 mobile/test/features/forum/forum_reply_delivery_test.dart diff --git a/mobile/lib/features/channels/message_mention_pubkeys.dart b/mobile/lib/features/channels/message_mention_pubkeys.dart deleted file mode 100644 index 59f28b903d6..00000000000 --- a/mobile/lib/features/channels/message_mention_pubkeys.dart +++ /dev/null @@ -1,26 +0,0 @@ -import 'channel.dart'; - -/// Semantic recipients for an outgoing mobile message. -/// -/// Explicit mentions are always preserved. In a DM, every current recipient -/// is also addressed with a `p` tag without inserting visible `@mentions` into -/// the composer. Non-DM channels remain explicit-only. -List messageMentionPubkeys({ - required Channel channel, - required String? senderPubkey, - required Iterable explicitMentions, - required Iterable dmRecipientPubkeys, -}) { - final sender = senderPubkey?.toLowerCase(); - final candidates = [ - ...explicitMentions, - if (channel.isDm) ...dmRecipientPubkeys, - ]; - - final seen = {?sender}; - return [ - for (final candidate in candidates) - if (candidate.trim().isNotEmpty && seen.add(candidate.toLowerCase())) - candidate.toLowerCase(), - ]; -} diff --git a/mobile/lib/features/channels/message_recipients.dart b/mobile/lib/features/channels/message_recipients.dart new file mode 100644 index 00000000000..24aa83ca73b --- /dev/null +++ b/mobile/lib/features/channels/message_recipients.dart @@ -0,0 +1,50 @@ +import 'channel.dart'; + +/// Who an outgoing message `p`-tags, split by why it tags them. +class MessageRecipients { + /// Typed as `@name` in the body. Marked `mention` on a reply. + final List mentions; + + /// Addressed by the *channel* rather than by the message — every other + /// participant in a DM, tagged whether or not anyone typed their name. + /// Never marked, because neither role is true of it. + final List addressed; + + const MessageRecipients({required this.mentions, required this.addressed}); +} + +/// Semantic recipients for an outgoing mobile message, split by role. +/// +/// Explicit mentions are always preserved. In a DM, every current recipient is +/// also addressed with a `p` tag without inserting visible `@mentions` into the +/// composer. Non-DM channels remain explicit-only. +/// +/// The two groups stay apart because a reply marks its `p` tags with the role +/// each one plays. Returning them as one list made every DM thread reply claim +/// its counterpart had been `@`-mentioned — which pierces a mute and takes a +/// slot in the mention feed ahead of a real `@you`. Mirrors +/// `messageRecipients` in `desktop/src/features/messages/lib/messageRecipients.ts`. +MessageRecipients messageRecipients({ + required Channel channel, + required String? senderPubkey, + required Iterable explicitMentions, + required Iterable dmRecipientPubkeys, +}) { + final sender = senderPubkey?.toLowerCase(); + final seen = {?sender}; + + bool take(String candidate) => + candidate.trim().isNotEmpty && seen.add(candidate.toLowerCase()); + + final mentions = [ + for (final candidate in explicitMentions) + if (take(candidate)) candidate.toLowerCase(), + ]; + final addressed = [ + if (channel.isDm) + for (final candidate in dmRecipientPubkeys) + if (take(candidate)) candidate.toLowerCase(), + ]; + + return MessageRecipients(mentions: mentions, addressed: addressed); +} diff --git a/mobile/lib/features/channels/send_message_provider.dart b/mobile/lib/features/channels/send_message_provider.dart index 757275c60ac..1b3e80ededf 100644 --- a/mobile/lib/features/channels/send_message_provider.dart +++ b/mobile/lib/features/channels/send_message_provider.dart @@ -1,13 +1,14 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import '../../shared/mentions/mention_tags.dart'; import '../../shared/relay/relay.dart'; import '../channels/channel_management_provider.dart'; import '../../shared/profile/user_cache_provider.dart'; import '../../shared/profile/user_profile.dart'; import 'channel.dart'; import 'channel_messages_provider.dart'; -import 'message_mention_pubkeys.dart'; import 'local_message_send_animation_provider.dart'; +import 'message_recipients.dart'; /// Sends messages by signing an event with the user's nsec and publishing it /// over the relay's NIP-42-authenticated WebSocket session. @@ -56,6 +57,7 @@ class SendMessage { required String content, String? parentEventId, String? rootEventId, + String? parentAuthorPubkey, List? mentionPubkeys, Channel? channel, List> mediaTags = const [], @@ -69,28 +71,67 @@ class SendMessage { final dmRecipientPubkeys = channel?.isDm == true ? await _fetchDmRecipientPubkeys(channelId, channel!, authorPubkey) : null; - final resolvedMentions = dmRecipientPubkeys != null - ? messageMentionPubkeys( + // Split by role, not merged: a DM addresses its other participants whether + // or not anyone typed their names, and a reply marks each `p` tag with the + // role it plays. One list cannot say which is which. + final recipients = dmRecipientPubkeys != null + ? messageRecipients( channel: channel!, senderPubkey: authorPubkey, explicitMentions: explicitMentions, dmRecipientPubkeys: dmRecipientPubkeys, ) - : explicitMentions; + : MessageRecipients( + mentions: [for (final pk in explicitMentions) pk.toLowerCase()], + addressed: const [], + ); // Normalize mentions: lowercase, deduplicate, exclude self (matching // the desktop's normalizeMentionPubkeys). final selfLower = authorPubkey?.toLowerCase(); - final seenMentions = {?selfLower}; + final seen = {?selfLower}; final normalizedMentions = [ - for (final pk in resolvedMentions) - if (seenMentions.add(pk.toLowerCase())) pk, + for (final pk in recipients.mentions) + if (seen.add(pk.toLowerCase())) pk.toLowerCase(), + ]; + + // A reply also addresses the author it answers (NIP-10). Agent harnesses + // subscribe with `#p`, so without this tag the relay never delivers the + // reply and the agent cannot see that it was answered. + // + // Kept out of the mention list and marked instead: as a bare `p` tag it is + // byte-identical to a typed @mention, which forces every receiver to fetch + // the parent just to tell them apart. `seen` already holds self, so one + // lookup covers "answering myself" and "already typed in the body" — and in + // the latter case mention is the role that survives. + final isReply = parentEventId != null; + final parentLower = isPubkeyShaped(parentAuthorPubkey) + ? parentAuthorPubkey!.toLowerCase() + : null; + final addressingPubkey = + isReply && parentLower != null && seen.add(parentLower) + ? parentLower + : null; + + // Whatever is left is addressed by the channel alone. Resolved after the + // addressing tag so a DM counterpart who wrote the parent keeps the `reply` + // marker instead of falling back to a bare tag — the same precedence the + // desktop and CLI builders apply. + final normalizedRecipients = [ + for (final pk in recipients.addressed) + if (seen.add(pk.toLowerCase())) pk.toLowerCase(), ]; final tags = >[ ['h', channelId], if (parentEventId != null) ..._buildReplyTags(parentEventId, rootEventId), - for (final pk in normalizedMentions) ['p', pk], + for (final pk in normalizedMentions) + if (isReply) ['p', pk, '', 'mention'] else ['p', pk], + // Addressed by the channel, so bare in either case: neither marker is + // true of them, and a bare tag means "ask the parent" — the answer they + // had before markers existed. + for (final pk in normalizedRecipients) ['p', pk], + if (addressingPubkey != null) ['p', addressingPubkey, '', 'reply'], ...mediaTags, ]; diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index cbac3cff843..e8cf226b802 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -961,6 +961,7 @@ class ThreadDetailPage extends HookConsumerWidget { mentionPubkeys: mentionPubkeys, channel: channel, parentEventId: threadHead.id, + parentAuthorPubkey: threadHead.pubkey, rootEventId: effectiveRootId, mediaTags: mediaTags, ), diff --git a/mobile/lib/features/forum/forum_provider.dart b/mobile/lib/features/forum/forum_provider.dart index 59cd72d077c..21b9659bdef 100644 --- a/mobile/lib/features/forum/forum_provider.dart +++ b/mobile/lib/features/forum/forum_provider.dart @@ -1,5 +1,6 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import '../../shared/mentions/mention_tags.dart'; import '../../shared/relay/relay.dart'; import '../channels/channel_management_provider.dart'; import '../../shared/custom_emoji/custom_emoji.dart'; @@ -118,6 +119,7 @@ class ForumEventDelivery { required String channelId, required String parentEventId, required String content, + String? parentAuthorPubkey, List mentionPubkeys = const [], List> mediaTags = const [], }) async { @@ -125,6 +127,7 @@ class ForumEventDelivery { kind: EventKind.forumComment, channelId: channelId, parentEventId: parentEventId, + parentAuthorPubkey: parentAuthorPubkey, content: content, mentionPubkeys: mentionPubkeys, mediaTags: mediaTags, @@ -140,6 +143,7 @@ class ForumEventDelivery { required String channelId, required String content, String? parentEventId, + String? parentAuthorPubkey, required List mentionPubkeys, required List> mediaTags, }) async { @@ -154,16 +158,38 @@ class ForumEventDelivery { final seen = {?selfPubkey}; final normalizedMentions = [ for (final pk in mentionPubkeys) - if (seen.add(pk.toLowerCase())) pk, + if (seen.add(pk.toLowerCase())) pk.toLowerCase(), ]; + // A comment also addresses the author it answers (NIP-10). Forum channels + // are mention-eligible for agents, and an agent's `require_mention` + // subscription is a `#p` filter — without this tag the relay never + // delivers the comment to the agent being replied to. + // + // Marked rather than folded into the mentions: as a bare `p` tag it cannot + // be told from a typed @mention without fetching the parent. `seen` already + // holds self, so one check covers both "answering myself" and "already + // typed in the body", where mention is the role that survives. + final parentLower = isPubkeyShaped(parentAuthorPubkey) + ? parentAuthorPubkey!.toLowerCase() + : null; + final addressingPubkey = + parentEventId != null && + parentLower != null && + !seen.contains(parentLower) + ? parentLower + : null; + final isReply = parentEventId != null; + await _relay.submit( kind: kind, content: content, tags: [ ['h', channelId], if (parentEventId != null) ['e', parentEventId, '', 'reply'], - for (final pk in normalizedMentions) ['p', pk], + for (final pk in normalizedMentions) + if (isReply) ['p', pk, '', 'mention'] else ['p', pk], + if (addressingPubkey != null) ['p', addressingPubkey, '', 'reply'], ...mediaTags, ...buildCustomEmojiTags(content, _customEmoji), ], diff --git a/mobile/lib/features/forum/forum_thread_page.dart b/mobile/lib/features/forum/forum_thread_page.dart index 7c1f4c3cea0..00d0c1dd3df 100644 --- a/mobile/lib/features/forum/forum_thread_page.dart +++ b/mobile/lib/features/forum/forum_thread_page.dart @@ -311,6 +311,7 @@ class _ThreadContent extends HookConsumerWidget { }) => forumDelivery.createReply( channelId: channelId, parentEventId: post.eventId, + parentAuthorPubkey: post.pubkey, content: content, mentionPubkeys: mentionPubkeys, mediaTags: mediaTags, diff --git a/mobile/lib/shared/mentions/mention_tags.dart b/mobile/lib/shared/mentions/mention_tags.dart index bf21282715b..62d7c78d034 100644 --- a/mobile/lib/shared/mentions/mention_tags.dart +++ b/mobile/lib/shared/mentions/mention_tags.dart @@ -4,3 +4,15 @@ Set mentionedPubkeysFromTags(Iterable> tags) => { if (tag.length >= 2 && (tag[0] == 'p' || tag[0] == 'mention')) tag[1].toLowerCase(), }; + +final _pubkeyHex = RegExp(r'^[0-9a-f]{64}$'); + +/// Whether [pubkey] is shaped like a Nostr pubkey: 64 hex characters. +/// +/// The `p` tag naming the author a reply answers is what delivers that reply to +/// the agent being answered, so a malformed value costs the delivery instead of +/// failing loudly. The Rust builder rejects one in `check_pubkey` and the +/// desktop builder drops it before tagging; this is the same gate for the +/// mobile builders. +bool isPubkeyShaped(String? pubkey) => + pubkey != null && _pubkeyHex.hasMatch(pubkey.toLowerCase()); diff --git a/mobile/test/features/channels/message_mention_pubkeys_test.dart b/mobile/test/features/channels/message_mention_pubkeys_test.dart deleted file mode 100644 index 8bc260a7aca..00000000000 --- a/mobile/test/features/channels/message_mention_pubkeys_test.dart +++ /dev/null @@ -1,82 +0,0 @@ -import 'package:buzz/features/channels/channel.dart'; -import 'package:buzz/features/channels/message_mention_pubkeys.dart'; -import 'package:flutter_test/flutter_test.dart'; - -const _self = 'self'; -const _agent = 'agent'; -const _human = 'human'; - -void main() { - test('implicitly addresses every participating DM recipient', () { - expect( - messageMentionPubkeys( - channel: _channel( - type: 'dm', - participantPubkeys: const [_self, _agent, _human], - ), - senderPubkey: _self, - explicitMentions: const [], - dmRecipientPubkeys: const [_agent, _human], - ), - [_agent, _human], - ); - }); - - test('preserves and deduplicates explicit mentions with DM recipients', () { - expect( - messageMentionPubkeys( - channel: _channel( - type: 'dm', - participantPubkeys: const [_self, _agent, _human], - ), - senderPubkey: _self, - explicitMentions: const [_human, _agent], - dmRecipientPubkeys: const [_agent], - ), - [_human, _agent], - ); - }); - - test('addresses human DMs but not ordinary channel members', () { - expect( - messageMentionPubkeys( - channel: _channel( - type: 'dm', - participantPubkeys: const [_self, _human], - ), - senderPubkey: _self, - explicitMentions: const [], - dmRecipientPubkeys: const [_human], - ), - [_human], - ); - expect( - messageMentionPubkeys( - channel: _channel( - type: 'stream', - participantPubkeys: const [_self, _agent], - ), - senderPubkey: _self, - explicitMentions: const [], - dmRecipientPubkeys: const [_agent], - ), - isEmpty, - ); - }); -} - -Channel _channel({ - required String type, - required List participantPubkeys, -}) => Channel( - id: 'channel', - name: 'Conversation', - channelType: type, - visibility: 'private', - description: '', - createdBy: _self, - createdAt: DateTime(2025), - memberCount: participantPubkeys.length, - participantPubkeys: participantPubkeys, - isMember: true, -); diff --git a/mobile/test/features/channels/message_recipients_test.dart b/mobile/test/features/channels/message_recipients_test.dart new file mode 100644 index 00000000000..67e529f9851 --- /dev/null +++ b/mobile/test/features/channels/message_recipients_test.dart @@ -0,0 +1,90 @@ +import 'package:buzz/features/channels/channel.dart'; +import 'package:buzz/features/channels/message_recipients.dart'; +import 'package:flutter_test/flutter_test.dart'; + +const _self = 'self'; +const _agent = 'agent'; +const _human = 'human'; + +void main() { + test('implicitly addresses every participating DM recipient', () { + final recipients = messageRecipients( + channel: _channel( + type: 'dm', + participantPubkeys: const [_self, _agent, _human], + ), + senderPubkey: _self, + explicitMentions: const [], + dmRecipientPubkeys: const [_agent, _human], + ); + // Addressed by the channel, not mentioned: nobody typed their names, so + // marking these as mentions would pierce a mute and outrank a real `@you`. + expect(recipients.mentions, isEmpty); + expect(recipients.addressed, [_agent, _human]); + }); + + test('preserves and deduplicates explicit mentions with DM recipients', () { + final recipients = messageRecipients( + channel: _channel( + type: 'dm', + participantPubkeys: const [_self, _agent, _human], + ), + senderPubkey: _self, + explicitMentions: const [_human, _agent], + dmRecipientPubkeys: const [_agent], + ); + // Typed wins: a participant who was also written as `@name` is a mention, + // and is not repeated as a channel recipient. + expect(recipients.mentions, [_human, _agent]); + expect(recipients.addressed, isEmpty); + }); + + test('addresses human DMs but not ordinary channel members', () { + final dm = messageRecipients( + channel: _channel(type: 'dm', participantPubkeys: const [_self, _human]), + senderPubkey: _self, + explicitMentions: const [], + dmRecipientPubkeys: const [_human], + ); + expect(dm.addressed, [_human]); + + final stream = messageRecipients( + channel: _channel( + type: 'stream', + participantPubkeys: const [_self, _agent], + ), + senderPubkey: _self, + explicitMentions: const [], + dmRecipientPubkeys: const [_agent], + ); + expect(stream.mentions, isEmpty); + expect(stream.addressed, isEmpty); + }); + + test('never addresses the sender', () { + final recipients = messageRecipients( + channel: _channel(type: 'dm', participantPubkeys: const [_self, _human]), + senderPubkey: _self, + explicitMentions: const [_self], + dmRecipientPubkeys: const [_self, _human], + ); + expect(recipients.mentions, isEmpty); + expect(recipients.addressed, [_human]); + }); +} + +Channel _channel({ + required String type, + required List participantPubkeys, +}) => Channel( + id: 'channel', + name: 'Conversation', + channelType: type, + visibility: 'private', + description: '', + createdBy: _self, + createdAt: DateTime(2025), + memberCount: participantPubkeys.length, + participantPubkeys: participantPubkeys, + isMember: true, +); diff --git a/mobile/test/features/channels/send_message_provider_test.dart b/mobile/test/features/channels/send_message_provider_test.dart index 5766fab2551..bb169511d71 100644 --- a/mobile/test/features/channels/send_message_provider_test.dart +++ b/mobile/test/features/channels/send_message_provider_test.dart @@ -239,9 +239,235 @@ void main() { ), ); }); + + test('a thread reply p-tags the author it answers', () async { + // Agent harnesses subscribe with `#p`, so a reply without this tag is + // never delivered to the agent being replied to. + final session = _PendingPublishRelaySession(); + final send = SendMessage( + signedEventRelay: SignedEventRelay( + session: session, + nsec: nostr.Keys.generate().nsec, + ), + fetchMembers: (_) async => const [], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'thanks', + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + ); + await session.published; + session.accept(); + await result; + + // `containsAll` with matchers: Dart lists compare by identity, so a bare + // `contains(['p', ...])` never matches an equal-but-distinct list. + expect( + session.event.tags, + containsAll([ + // Marked, not bare: a bare `p` tag here is byte-identical to a typed + // @mention, which forces the receiver to fetch the parent just to tell + // them apart. Relay tag filters match only the second element, so the + // marker cannot affect the agent's `#p` delivery. + equals(['p', _agentPubkey, '', 'reply']), + equals(['e', _parentEventId, '', 'reply']), + ]), + ); + }); + + test( + 'a DM reply marks the counterpart as addressing, not as a mention', + () async { + // The counterpart is both the DM's other participant and the author being + // answered. One tag, marked `reply` — marking it `mention` would claim they + // had been typed as `@name`, which pierces a mute and outranks a real + // `@you` in the mention feed. + final session = _PendingPublishRelaySession(); + final signingKey = nostr.Keys.generate().nsec; + final sender = nostr.Keys( + nostr.Nip19.decode(payload: signingKey).data, + ).public; + final counterpart = 'c' * 64; + final send = SendMessage( + signedEventRelay: SignedEventRelay(session: session, nsec: signingKey), + fetchMembers: (_) async => [_member(sender), _member(counterpart)], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'thanks', + channel: _dmChannel([sender, counterpart]), + parentEventId: _parentEventId, + parentAuthorPubkey: counterpart, + mentionPubkeys: const [], + ); + await session.published; + + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', counterpart, '', 'reply'], + ]); + + session.accept(); + await result; + }, + ); + + test('a DM reply to your own message leaves the counterpart bare', () async { + // Nobody typed the counterpart's name and they did not write the parent, so + // neither marker is true of them. Bare means "ask the parent", which is the + // answer they had before markers existed. + final session = _PendingPublishRelaySession(); + final signingKey = nostr.Keys.generate().nsec; + final sender = nostr.Keys( + nostr.Nip19.decode(payload: signingKey).data, + ).public; + final counterpart = 'c' * 64; + final send = SendMessage( + signedEventRelay: SignedEventRelay(session: session, nsec: signingKey), + fetchMembers: (_) async => [_member(sender), _member(counterpart)], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'following up on my own note', + channel: _dmChannel([sender, counterpart]), + parentEventId: _parentEventId, + parentAuthorPubkey: sender, + mentionPubkeys: const [], + ); + await session.published; + + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', counterpart], + ]); + + session.accept(); + await result; + }); + + test('a reply marks a typed mention apart from the author it answers', () async { + // The two roles are byte-identical as bare `p` tags, and this is the case + // the receiver cannot resolve by fetching the parent: one of these pubkeys + // was typed in the body and the other wrote the message being answered. + final session = _PendingPublishRelaySession(); + final typed = 'd' * 64; + final send = SendMessage( + signedEventRelay: SignedEventRelay( + session: session, + nsec: nostr.Keys.generate().nsec, + ), + fetchMembers: (_) async => const [], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'thanks @typed', + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + mentionPubkeys: [typed], + ); + await session.published; + + // Mentions first, addressing last — the order the Rust builder emits, so an + // optimistic copy of this event is tag-identical to what the relay stores. + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', typed, '', 'mention'], + ['p', _agentPubkey, '', 'reply'], + ]); + + session.accept(); + await result; + }); + + test('a typed parent author is tagged once, as a mention', () async { + // Mention outranks addressing: typing someone's name is a stronger claim + // than answering them, and two tags for one pubkey would let the reply + // count twice. + final session = _PendingPublishRelaySession(); + final send = SendMessage( + signedEventRelay: SignedEventRelay( + session: session, + nsec: nostr.Keys.generate().nsec, + ), + fetchMembers: (_) async => const [], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'thanks @agent', + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + mentionPubkeys: const [_agentPubkey], + ); + await session.published; + + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', _agentPubkey, '', 'mention'], + ]); + + session.accept(); + await result; + }); + + test('a malformed parent author adds no addressing tag', () async { + // A `p` tag is what delivers the reply to the agent being answered, so a + // malformed value there would be published as a tag naming nobody. + final session = _PendingPublishRelaySession(); + final send = SendMessage( + signedEventRelay: SignedEventRelay( + session: session, + nsec: nostr.Keys.generate().nsec, + ), + fetchMembers: (_) async => const [], + readUserCache: () => const {}, + addLocalMessage: (_, _) {}, + completeLocalMessage: (_, _) {}, + removeLocalMessage: (_, _) {}, + ); + + final result = send( + channelId: _channelId, + content: 'thanks', + parentEventId: _parentEventId, + parentAuthorPubkey: 'not-a-pubkey', + mentionPubkeys: const [], + ); + await session.published; + + expect(session.event.tags.where((tag) => tag.first == 'p'), isEmpty); + + session.accept(); + await result; + }); } const _channelId = '11111111-1111-4111-8111-111111111111'; +const _parentEventId = + 'fdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfd'; +const _agentPubkey = + 'aeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeae'; Channel _dmChannel(List participantPubkeys) => Channel( id: _channelId, diff --git a/mobile/test/features/forum/forum_reply_delivery_test.dart b/mobile/test/features/forum/forum_reply_delivery_test.dart new file mode 100644 index 00000000000..44fa9cb8fb7 --- /dev/null +++ b/mobile/test/features/forum/forum_reply_delivery_test.dart @@ -0,0 +1,199 @@ +import 'dart:async'; + +import 'package:buzz/features/forum/forum_provider.dart'; +import 'package:buzz/shared/relay/relay.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:nostr/nostr.dart' as nostr; + +void main() { + test('a forum comment p-tags the post author it answers', () async { + // Forum channels are mention-eligible for agents, and an agent's + // `require_mention` subscription is a `#p` REQ filter — an untagged + // comment never reaches the agent being replied to. + final session = _PendingPublishRelaySession(); + final container = ProviderContainer( + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + container + .read(relayConfigProvider.notifier) + .update( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ); + + final delivery = ForumEventDelivery.capture(container); + final result = delivery.createReply( + channelId: _channelId, + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + content: 'thanks', + ); + await session.published; + session.accept(); + await result; + + // Dart lists compare by identity, so matchers are required here. + expect( + session.event.tags, + containsAll([ + // Marked, not bare: a bare `p` tag here is byte-identical to a typed + // @mention, which forces the receiver to fetch the parent just to tell + // them apart. Relay tag filters match only the second element, so the + // marker cannot affect the agent's `#p` delivery. + equals(['p', _agentPubkey, '', 'reply']), + equals(['e', _parentEventId, '', 'reply']), + ]), + ); + }); + + test('a forum comment without a parent author adds no extra p-tag', () async { + final session = _PendingPublishRelaySession(); + final container = ProviderContainer( + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + container + .read(relayConfigProvider.notifier) + .update( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ); + + final delivery = ForumEventDelivery.capture(container); + final result = delivery.createReply( + channelId: _channelId, + parentEventId: _parentEventId, + content: 'thanks', + ); + await session.published; + session.accept(); + await result; + + expect(session.event.tags.where((tag) => tag.first == 'p'), isEmpty); + }); + + test( + 'a forum comment marks a typed mention apart from the post author', + () async { + // Same two roles as the channel builder, and the same reason they cannot be + // told apart as bare tags: one pubkey was typed, the other wrote the post. + final session = _PendingPublishRelaySession(); + final typed = 'd' * 64; + final container = ProviderContainer( + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + container + .read(relayConfigProvider.notifier) + .update( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ); + + final delivery = ForumEventDelivery.capture(container); + final result = delivery.createReply( + channelId: _channelId, + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + content: 'thanks @typed', + mentionPubkeys: [typed], + ); + await session.published; + session.accept(); + await result; + + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', typed, '', 'mention'], + ['p', _agentPubkey, '', 'reply'], + ]); + }, + ); + + test('a typed post author is tagged once, as a mention', () async { + final session = _PendingPublishRelaySession(); + final container = ProviderContainer( + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + container + .read(relayConfigProvider.notifier) + .update( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ); + + final delivery = ForumEventDelivery.capture(container); + final result = delivery.createReply( + channelId: _channelId, + parentEventId: _parentEventId, + parentAuthorPubkey: _agentPubkey, + content: 'thanks @agent', + mentionPubkeys: const [_agentPubkey], + ); + await session.published; + session.accept(); + await result; + + expect(session.event.tags.where((tag) => tag.first == 'p').toList(), [ + ['p', _agentPubkey, '', 'mention'], + ]); + }); + + test('a malformed post author adds no addressing tag', () async { + final session = _PendingPublishRelaySession(); + final container = ProviderContainer( + overrides: [relaySessionProvider.overrideWith(() => session)], + ); + addTearDown(container.dispose); + container + .read(relayConfigProvider.notifier) + .update( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + ); + + final delivery = ForumEventDelivery.capture(container); + final result = delivery.createReply( + channelId: _channelId, + parentEventId: _parentEventId, + parentAuthorPubkey: 'not-a-pubkey', + content: 'thanks', + ); + await session.published; + session.accept(); + await result; + + expect(session.event.tags.where((tag) => tag.first == 'p'), isEmpty); + }); +} + +const _channelId = '11111111-1111-4111-8111-111111111111'; +const _parentEventId = + 'fdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfd'; +const _agentPubkey = + 'aeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeaeae'; + +class _PendingPublishRelaySession extends RelaySessionNotifier { + final Completer _result = Completer(); + final Completer _published = Completer(); + late NostrEvent event; + + Future get published => _published.future; + + @override + SessionState build() => const SessionState(status: SessionStatus.connected); + + @override + Future publish( + NostrEvent event, { + Duration timeout = const Duration(seconds: 8), + }) { + this.event = event; + _published.complete(); + return _result.future; + } + + void accept() => _result.complete(event); +} From 0dc864171a56efeae1e48d948c839185c7a1b894 Mon Sep 17 00:00:00 2001 From: cyberzero000 Date: Sat, 22 Aug 2026 11:52:46 -0700 Subject: [PATCH 3/3] fix(desktop): keep a reply out of the mention feed and the mute bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Teaching the senders to emit an addressing `p` tag means teaching every path that reads a `p` tag to tell it apart from a mention. There are seven of them — the live notify path, the dock badge, the home feed, the channel unread scan, the live channel updates, the community observer, and the app-shell notification effect. Miss one and that path treats every reply as a mention: it pierces mutes, takes a slot in the mention feed ahead of a real `@you`, and raises the dock badge. `shouldNotify.ts` reads the sender's marker when there is one and falls back to the parent's author when there is not, matching `unread_notify.rs` function for function. The two decide notification ownership independently from the same question, so a divergence makes them disagree and notify twice, or not at all. Grouped by what was wrong: - Double-notify. A reply could be counted by both the backend feed poll and the frontend live path. Exactly one owner notifies per event now. - Mutes. A muted channel or thread leaked replies; a muted DM inverted; a real `@mention` inside a muted thread was reported as notifying but did not. - Priority. High-priority classification failed open on an unresolved parent, so an addressing tag could raise the dock badge as if it were a mention. It fails closed, and the addressing tag is out of both priority and badge math. - Dock badge. A DM thread reply counted twice. - Deferral. Handing an unresolvable reply to the live path lost the notification instead of deferring it: `collectHomeAlertItems` returns the whole mention list unfiltered and the notification effect added every item it saw — including ones just declined as `replyToSelf` — to the persisted seen set, so the declining poll consumed the slot and the next poll dropped the item as already-seen. A genuine typed `@mention` inside a thread was lost for good, across restarts, because its parent belongs to a third party and so looks unresolved. - Robustness. Event ids are validated and case-normalized at the filter boundary, matching the Rust side. A single channel's failed parent lookup no longer aborts the whole community poll and drops the community to `state: "error"`, which would clear its dot and badge outright — undercounting one channel for 30 seconds is the smaller wrong answer. `messageRecipients.ts` replaces `messageMentionPubkeys.ts` and returns typed mentions and channel-addressed recipients as a named pair rather than one merged list. `replyContextEvents.ts` resolves a reply's parent from every cache that can hold it — the channel timeline is not the only one, the thread panel keeps its replies under a separate key and the Inbox can answer a message in a channel the user never opened this session, and looking only at the channel cache silently missed both. `unreadReadMarker.ts` is a split of `useUnreadChannels.ts`, which crossed the 1000-line ratchet. Signed-off-by: cyberzero000 --- .../app/useAppShellDesktopNotifications.ts | 59 ++- .../src/features/channels/unreadReadMarker.ts | 46 ++ .../channels/useLiveChannelUpdates.ts | 157 +++++-- .../features/channels/useUnreadChannels.ts | 96 ++--- .../communityUnreadObserver.test.mjs | 55 +++ .../communities/communityUnreadObserver.ts | 72 +++- .../features/communities/useCommunityInit.ts | 2 + desktop/src/features/forum/hooks.ts | 33 +- desktop/src/features/forum/ui/ForumView.tsx | 10 +- .../features/home/lib/inboxReplyRecipients.ts | 84 ++++ desktop/src/features/home/ui/HomeView.tsx | 57 ++- .../src/features/home/ui/InboxDetailPane.tsx | 35 +- desktop/src/features/messages/hooks.ts | 78 +++- .../lib/messageMentionPubkeys.test.mjs | 48 --- .../messages/lib/messageMentionPubkeys.ts | 30 -- .../messages/lib/messageRecipients.test.mjs | 124 ++++++ .../messages/lib/messageRecipients.ts | 77 ++++ .../messages/lib/replyContextEvents.test.mjs | 293 +++++++++++++ .../messages/lib/replyContextEvents.ts | 212 ++++++++++ .../lib/replyRecipientPubkeys.test.mjs | 68 +++ .../features/messages/lib/threading.test.mjs | 192 +++++++++ .../src/features/messages/lib/threading.ts | 182 +++++++- .../src/features/notifications/hooks.test.mjs | 24 ++ desktop/src/features/notifications/hooks.ts | 32 +- .../features/notifications/lib/feed.test.mjs | 52 +++ .../src/features/notifications/lib/feed.ts | 6 +- .../notifications/lib/homeBadge.test.mjs | 173 ++++++++ .../features/notifications/lib/homeBadge.ts | 88 +++- .../lib/replyParentAuthors.test.mjs | 115 +++++ .../notifications/lib/replyParentAuthors.ts | 105 +++++ .../notifications/lib/shouldNotify.test.mjs | 392 +++++++++++++++++- .../notifications/lib/shouldNotify.ts | 200 ++++++++- .../use-feed-desktop-notifications.ts | 3 + desktop/src/shared/api/feedTypes.ts | 54 +++ desktop/src/shared/api/tauri.ts | 32 +- desktop/src/shared/api/tauriFeedMapping.ts | 38 ++ desktop/src/shared/api/tauriMessages.ts | 5 + desktop/src/shared/api/types.ts | 50 +-- desktop/src/shared/constants/kinds.ts | 17 + desktop/src/testing/e2eBridge.ts | 26 +- 40 files changed, 3102 insertions(+), 320 deletions(-) create mode 100644 desktop/src/features/channels/unreadReadMarker.ts create mode 100644 desktop/src/features/home/lib/inboxReplyRecipients.ts delete mode 100644 desktop/src/features/messages/lib/messageMentionPubkeys.test.mjs delete mode 100644 desktop/src/features/messages/lib/messageMentionPubkeys.ts create mode 100644 desktop/src/features/messages/lib/messageRecipients.test.mjs create mode 100644 desktop/src/features/messages/lib/messageRecipients.ts create mode 100644 desktop/src/features/messages/lib/replyContextEvents.test.mjs create mode 100644 desktop/src/features/messages/lib/replyContextEvents.ts create mode 100644 desktop/src/features/messages/lib/replyRecipientPubkeys.test.mjs create mode 100644 desktop/src/features/messages/lib/threading.test.mjs create mode 100644 desktop/src/features/notifications/lib/homeBadge.test.mjs create mode 100644 desktop/src/features/notifications/lib/replyParentAuthors.test.mjs create mode 100644 desktop/src/features/notifications/lib/replyParentAuthors.ts create mode 100644 desktop/src/shared/api/feedTypes.ts create mode 100644 desktop/src/shared/api/tauriFeedMapping.ts diff --git a/desktop/src/app/useAppShellDesktopNotifications.ts b/desktop/src/app/useAppShellDesktopNotifications.ts index b86b95363cd..001a465ddaa 100644 --- a/desktop/src/app/useAppShellDesktopNotifications.ts +++ b/desktop/src/app/useAppShellDesktopNotifications.ts @@ -1,12 +1,20 @@ import * as React from "react"; +import { useQueryClient } from "@tanstack/react-query"; import { activateDesktopNotificationTarget, createDesktopNotificationActivationQueue, shouldBounceForChannelNotification, } from "@/app/AppShell.helpers"; +import { getThreadReference } from "@/features/messages/lib/threading"; import { useCommunityJoinAlerts } from "@/features/community-members/useCommunityJoinAlerts"; -import { hasMentionForEvent } from "@/features/notifications/lib/shouldNotify"; +import { + hasAuthoredMentionForEvent, + hasMentionForEvent, +} from "@/features/notifications/lib/shouldNotify"; +import { resolveReplyParentAuthor } from "@/features/messages/lib/replyContextEvents"; +import { relayClient } from "@/shared/api/relayClient"; +import { REPLY_PARENT_EVENT_KINDS } from "@/shared/constants/kinds"; import type { NotificationSettings } from "@/features/notifications/hooks"; import { listenForDesktopNotificationActions, @@ -49,6 +57,20 @@ export function useAppShellDesktopNotifications({ pubkey?: string; silentChannelIds?: ReadonlySet; }) { + const queryClient = useQueryClient(); + // Guards the reply handler, which resumes after an awaited parent lookup. + // AppShell sits under ``, so switching + // communities unmounts it and disconnects the relay — which rejects that + // lookup, and the deliberate "keep the reply when the lookup failed" branch + // would then toast for the community the user just left, with a + // click-through to a channel id the new community does not have. + const isMountedRef = React.useRef(true); + React.useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + }; + }, []); // Roster alerts are owner/admin-only and self-gating; mounted here because // it shares this hook's "desktop notifications are on" precondition and // AppShell sits at the file-size ratchet ceiling. @@ -103,7 +125,7 @@ export function useAppShellDesktopNotifications({ ); const handleThreadReplyDesktopNotification = React.useEffectEvent( - (channelId: string, event: RelayEvent) => { + async (channelId: string, event: RelayEvent) => { if (!enabled) return; if ( !notificationSettings.desktopEnabled || @@ -113,9 +135,38 @@ export function useAppShellDesktopNotifications({ } // Replies that @-mention the user are owned by the home-feed mention - // path — skip them here so they don't notify (and sound) twice. + // path — skip them here so they don't notify (and sound) twice. Every + // reply now p-tags the author it answers, so that tag alone would hand + // the whole slot over and silence replies for anyone with the mention + // slot off. Resolving the parent's author is what separates the two, and + // it falls back to the relay because an unopened channel has no cache. const normalizedPubkey = pubkey?.trim().toLowerCase() ?? ""; - if (hasMentionForEvent(event, normalizedPubkey)) { + // Only a reply that tags the user can be handed back to the mention + // feed, so only that one is worth a lookup. Without this guard every + // reply in a followed thread pays a relay round trip — behind the same + // rate-limit gate as foreground history — and delays its own toast by it. + const parentAuthor = !hasMentionForEvent(event, normalizedPubkey) + ? null + : await resolveReplyParentAuthor({ + channelId, + fetchEvents: (filter) => relayClient.fetchEvents(filter), + kinds: REPLY_PARENT_EVENT_KINDS, + parentEventId: getThreadReference(event.tags).parentId, + queryClient, + }); + // Only hand the event back to the mention feed when we could actually + // answer who the parent belongs to. The feed runs its own server-side + // lookup and has already dropped everything it resolved to us, so + // deferring on a *failed* lookup means one relay hiccup loses the + // notification on both paths. Notifying twice is the better failure. + // The lookup above is the only await before we notify; bail if the + // community changed under it. + if (!isMountedRef.current) return; + if ( + parentAuthor !== null && + parentAuthor.status !== "unavailable" && + hasAuthoredMentionForEvent(event, normalizedPubkey, parentAuthor.pubkey) + ) { return; } diff --git a/desktop/src/features/channels/unreadReadMarker.ts b/desktop/src/features/channels/unreadReadMarker.ts new file mode 100644 index 00000000000..e7925d7b60a --- /dev/null +++ b/desktop/src/features/channels/unreadReadMarker.ts @@ -0,0 +1,46 @@ +import { + getThreadReference, + isBroadcastReply, +} from "@/features/messages/lib/threading"; + +function parseTimestamp(value: string | null | undefined) { + if (!value) { + return null; + } + + const timestamp = Date.parse(value); + return Number.isNaN(timestamp) ? null : timestamp; +} + +function toUnixSeconds(isoOrMs: string | null | undefined): number | null { + const ms = parseTimestamp(isoOrMs); + return ms === null ? null : Math.floor(ms / 1_000); +} + +// Resolve where the read marker should land when a channel is marked read. +// Folds the caller's timeline position together with the newest event this +// client has observed live (`observedLatest`), so an explicit "mark read" still +// covers messages that arrived faster than channel metadata — this fold is +// load-bearing for the Esc shortcut, sidebar mark-read, and empty-channel open, +// all of which pass a null/stale caller value. `clearObserved` reports whether +// the resulting marker covers the observed timestamp, signalling the caller to +// drop its observed refs so the unread memo sees `latest === undefined` until a +// genuinely newer event arrives. +export function resolveChannelReadMarker( + callerReadAt: string | null | undefined, + observedLatest: number | undefined, +): { markAt: number | null; clearObserved: boolean } { + const callerUnix = toUnixSeconds(callerReadAt); + const markAt = Math.max(callerUnix ?? 0, observedLatest ?? 0) || null; + return { + markAt, + clearObserved: + markAt !== null && + observedLatest !== undefined && + observedLatest <= markAt, + }; +} + +export function resolveObservedUnreadRootId(tags: string[][]): string | null { + return isBroadcastReply(tags) ? null : getThreadReference(tags).rootId; +} diff --git a/desktop/src/features/channels/useLiveChannelUpdates.ts b/desktop/src/features/channels/useLiveChannelUpdates.ts index 7598e8db3e5..92e31a11e86 100644 --- a/desktop/src/features/channels/useLiveChannelUpdates.ts +++ b/desktop/src/features/channels/useLiveChannelUpdates.ts @@ -7,13 +7,22 @@ import { mergeTimelineCacheMessages } from "@/features/messages/hooks"; import { channelMessagesKey } from "@/features/messages/lib/messageQueryKeys"; import { getChannelIdFromTags, + getThreadReference, isThreadReply, } from "@/features/messages/lib/threading"; -import { shouldNotifyForEvent } from "@/features/notifications/lib/shouldNotify"; +import { + lookupReplyParentAuthor, + resolveReplyParentAuthor, +} from "@/features/messages/lib/replyContextEvents"; +import { + needsResolvedParentAuthor, + shouldNotifyForEvent, +} from "@/features/notifications/lib/shouldNotify"; import { relayClient } from "@/shared/api/relayClient"; import { CHANNEL_EVENT_KINDS, CHANNEL_MESSAGE_EVENT_KINDS, + REPLY_PARENT_EVENT_KINDS, } from "@/shared/constants/kinds"; import type { Channel, RelayEvent } from "@/shared/api/types"; import { @@ -40,11 +49,19 @@ export type UseLiveChannelUpdatesOptions = { * drive the observed unread-event map that powers sidebar unread state. * See `UNREAD_TRIGGER_KINDS` for the exact kind set. */ - onChannelMessage?: (channelId: string, event: RelayEvent) => void; + onChannelMessage?: ( + channelId: string, + event: RelayEvent, + parentAuthorPubkey: string | null, + ) => void; /** * Fired for thread replies that should be surfaced as Home inbox activity. */ - onThreadReplyNotification?: (channelId: string, event: RelayEvent) => void; + onThreadReplyNotification?: ( + channelId: string, + event: RelayEvent, + parentAuthorPubkey: string | null, + ) => void; /** * Fired for external thread replies that do not match the locally-known * interest sets. Callers can perform an async backfill and then decide @@ -65,6 +82,7 @@ export type UseLiveChannelUpdatesOptions = { participatedRootIds?: ReadonlySet; followedRootIds?: ReadonlySet; authoredRootIds?: ReadonlySet; + mentionedRootIds?: ReadonlySet; mutedRootIds?: ReadonlySet; mutedChannelIds?: ReadonlySet; }; @@ -147,6 +165,17 @@ export function useLiveChannelUpdates( // effect: the same event can be replayed repeatedly while a relay flaps, and // mention events also arrive through both the channel and mention filters. const seenNotificationEventIdsRef = React.useRef(new Set()); + // Guards the one notification path that resumes after an await. Switching + // communities remounts this subtree and disconnects the relay, which rejects + // any in-flight parent lookup — without this the continuation would still + // toast for a channel in the community the user just left. + const isMountedRef = React.useRef(true); + React.useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + }; + }, []); const channelsInvalidateRef = React.useRef(null); if (channelsInvalidateRef.current === null) { channelsInvalidateRef.current = createTrailingDebounce(() => { @@ -288,37 +317,103 @@ export function useLiveChannelUpdates( handleDmEvent(event, isFirstNotificationDelivery); if (isExternalTriggerEvent && isFirstNotificationDelivery) { - const shouldNotify = shouldNotifyForEvent( - event, - normalizedCurrentPubkey, - { - participatedRootIds: options.participatedRootIds ?? EMPTY_SET, - followedRootIds: options.followedRootIds ?? EMPTY_SET, - authoredRootIds: options.authoredRootIds ?? EMPTY_SET, - mutedRootIds: options.mutedRootIds ?? EMPTY_SET, - mutedChannelIds: options.mutedChannelIds ?? EMPTY_SET, - channelId, - }, - ); - - if (!shouldNotify) { - if (isThreadedReply) { - options.onThreadReplyCandidate?.(channelId, event); - } - } else { - options.onChannelMessage?.(channelId, event); - if (isHomeActivityEvent(isDmChannel, isThreadedReply)) { - options.onThreadReplyNotification?.(channelId, event); + const mutedRootIds = options.mutedRootIds ?? EMPTY_SET; + const mutedChannelIds = options.mutedChannelIds ?? EMPTY_SET; + const { parentId } = getThreadReference(event.tags); + // Capture everything the decision reads before the (rare) await below, + // so an escalated lookup cannot resolve against a later render's props. + const onChannelMessage = options.onChannelMessage; + const onThreadReplyCandidate = options.onThreadReplyCandidate; + const onThreadReplyNotification = options.onThreadReplyNotification; + const onThreadReplyDesktopNotification = + options.onThreadReplyDesktopNotification; + const notifyForActiveChannel = options.notifyForActiveChannel; + const isDmTarget = dmChannelMap.has(channelId); + const isActiveChannel = channelId === activeChannelId; + + const deliver = (parentAuthorPubkey: string | null) => { + const shouldNotify = shouldNotifyForEvent( + event, + normalizedCurrentPubkey, + { + participatedRootIds: options.participatedRootIds ?? EMPTY_SET, + followedRootIds: options.followedRootIds ?? EMPTY_SET, + mentionedRootIds: options.mentionedRootIds ?? EMPTY_SET, + authoredRootIds: options.authoredRootIds ?? EMPTY_SET, + mutedRootIds, + mutedChannelIds, + channelId, + parentAuthorPubkey, + isDmChannel, + }, + ); + + if (!shouldNotify) { + if (isThreadedReply) { + onThreadReplyCandidate?.(channelId, event); + } + } else { + onChannelMessage?.(channelId, event, parentAuthorPubkey); + if (isHomeActivityEvent(isDmChannel, isThreadedReply)) { + // Same parent author `onChannelMessage` just got. This callback + // records mentioned thread roots too, and without the author it + // re-recorded the very event the guarded call above declined — + // marking a thread "mentioned" for a plain reply-to-you. `null` in + // a DM, where the addressing tag is the addressing and every other + // path deliberately declines to demote it. + onThreadReplyNotification?.( + channelId, + event, + isDmChannel ? null : parentAuthorPubkey, + ); + } } - } - if (shouldNotify && isThreadedReply) { - if ( - !dmChannelMap.has(channelId) && - (channelId !== activeChannelId || options.notifyForActiveChannel) - ) { - options.onThreadReplyDesktopNotification?.(channelId, event); + if (shouldNotify && isThreadedReply) { + if (!isDmTarget && (!isActiveChannel || notifyForActiveChannel)) { + onThreadReplyDesktopNotification?.(channelId, event); + } } + }; + + // Replies p-tag the author they answer. Without the parent's author that + // tag reads as a mention and pierces the channel/thread mute. + const cachedParentAuthor = lookupReplyParentAuthor( + queryClient, + channelId, + parentId, + ); + if ( + needsResolvedParentAuthor(event, normalizedCurrentPubkey, { + cachedParentAuthor, + isDmChannel, + }) + ) { + void resolveReplyParentAuthor({ + channelId, + fetchEvents: (filter) => relayClient.fetchEvents(filter), + kinds: REPLY_PARENT_EVENT_KINDS, + parentEventId: parentId, + queryClient, + }) + .then((parentAuthor) => { + if (!isMountedRef.current) return; + // The event's delivery slot stays consumed even when the lookup + // came back `unavailable`. `deliver` fails open on a null author, so + // an unresolved parent still notifies — releasing the slot would let + // the relay's ~5s reconnect overlap deliver the same reply a second + // time, and the desktop-notification path has no dedupe of its own. + deliver(parentAuthor.pubkey); + }) + .catch((error) => { + // `resolveReplyParentAuthor` never rejects — every failure path + // returns `unavailable` — so the only way here is `deliver` itself + // throwing inside a consumer callback, by which point the + // notification has already fired. Log it; do not re-deliver. + console.error("reply parent lookup delivery failed", error); + }); + } else { + deliver(cachedParentAuthor); } } diff --git a/desktop/src/features/channels/useUnreadChannels.ts b/desktop/src/features/channels/useUnreadChannels.ts index b956b672d0e..8d9ad58ba3c 100644 --- a/desktop/src/features/channels/useUnreadChannels.ts +++ b/desktop/src/features/channels/useUnreadChannels.ts @@ -25,7 +25,7 @@ import { isBroadcastReply, } from "@/features/messages/lib/threading"; import { - hasMentionForEvent, + hasAuthoredMentionForEvent, isHighPriorityEventForUser, } from "@/features/notifications/lib/shouldNotify"; import type { RelayClient } from "@/shared/api/relayClientSession"; @@ -74,6 +74,15 @@ type UseUnreadChannelsOptions = UseLiveChannelUpdatesOptions & { const CATCH_UP_LIMIT = 1000; const EMPTY_ROOT_IDS: ReadonlySet = new Set(); +export { + resolveChannelReadMarker, + resolveObservedUnreadRootId, +} from "@/features/channels/unreadReadMarker"; +import { + resolveChannelReadMarker, + resolveObservedUnreadRootId, +} from "@/features/channels/unreadReadMarker"; + export function channelCatchUpEventKinds( channelType: Channel["channelType"] | undefined, ) { @@ -82,48 +91,6 @@ export function channelCatchUpEventKinds( : CHANNEL_MESSAGE_EVENT_KINDS; } -function parseTimestamp(value: string | null | undefined) { - if (!value) { - return null; - } - - const timestamp = Date.parse(value); - return Number.isNaN(timestamp) ? null : timestamp; -} - -function toUnixSeconds(isoOrMs: string | null | undefined): number | null { - const ms = parseTimestamp(isoOrMs); - return ms === null ? null : Math.floor(ms / 1_000); -} - -// Resolve where the read marker should land when a channel is marked read. -// Folds the caller's timeline position together with the newest event this -// client has observed live (`observedLatest`), so an explicit "mark read" still -// covers messages that arrived faster than channel metadata — this fold is -// load-bearing for the Esc shortcut, sidebar mark-read, and empty-channel open, -// all of which pass a null/stale caller value. `clearObserved` reports whether -// the resulting marker covers the observed timestamp, signalling the caller to -// drop its observed refs so the unread memo sees `latest === undefined` until a -// genuinely newer event arrives. -export function resolveChannelReadMarker( - callerReadAt: string | null | undefined, - observedLatest: number | undefined, -): { markAt: number | null; clearObserved: boolean } { - const callerUnix = toUnixSeconds(callerReadAt); - const markAt = Math.max(callerUnix ?? 0, observedLatest ?? 0) || null; - return { - markAt, - clearObserved: - markAt !== null && - observedLatest !== undefined && - observedLatest <= markAt, - }; -} - -export function resolveObservedUnreadRootId(tags: string[][]): string | null { - return isBroadcastReply(tags) ? null : getThreadReference(tags).rootId; -} - export function useUnreadChannels( channels: Channel[], activeChannel: Channel | null, @@ -377,10 +344,18 @@ export function useUnreadChannels( // are ignored — thread badges only exist for replies. Returns true when the // set actually grew so callers can decide whether to bump the gate snapshot. const recordMentionedRoot = React.useCallback( - (event: RelayEvent): boolean => { + (event: RelayEvent, parentAuthorPubkey?: string | null): boolean => { if (normalizedPubkey === null) return false; if (event.pubkey.toLowerCase() === normalizedPubkey) return false; - if (!hasMentionForEvent(event, normalizedPubkey)) return false; + // Not `hasMentionForEvent`: a reply carries an addressing `p` tag naming + // the author it answers, byte-identical to a typed `@mention`. Recording + // that as a mention subscribes the user to every thread they were + // answered in, rendering it "Following" while nothing ever notifies. + if ( + !hasAuthoredMentionForEvent(event, normalizedPubkey, parentAuthorPubkey) + ) { + return false; + } const { rootId } = getThreadReference(event.tags); if (rootId === null) return false; const target = mentionedRootIdsRef.current; @@ -424,12 +399,24 @@ export function useUnreadChannels( [observedPersistence], ); const handleChannelMessage = React.useCallback( - (channelId: string, event: RelayEvent) => { + ( + channelId: string, + event: RelayEvent, + parentAuthorPubkey: string | null = null, + ) => { const channel = channelsRef.current.find((ch) => ch.id === channelId); + // The parent's author comes from the live path, which has already + // resolved it. Without it a reply's addressing p-tag marks the whole + // channel high-priority, and `shouldCountTowardHomeBadgeSubtotal` then + // drops top-level items there from the dock badge. const isHighPriority = channel?.channelType === "dm" || (normalizedPubkey !== null && - isHighPriorityEventForUser(event, normalizedPubkey)); + isHighPriorityEventForUser( + event, + normalizedPubkey, + parentAuthorPubkey, + )); const isThreadedReply = getThreadReference(event.tags).parentId !== null && !isBroadcastReply(event.tags); @@ -461,7 +448,7 @@ export function useUnreadChannels( // A mention on a reply makes its thread badge-eligible even when the // user never participated/authored/followed (the gate's missing term). - if (recordMentionedRoot(event)) { + if (recordMentionedRoot(event, parentAuthorPubkey)) { bumpMembershipVersion(); } @@ -472,7 +459,7 @@ export function useUnreadChannels( bumpLatestVersion(); } - callerOnChannelMessage?.(channelId, event); + callerOnChannelMessage?.(channelId, event, parentAuthorPubkey); }, [ callerOnChannelMessage, @@ -538,7 +525,11 @@ export function useUnreadChannels( ); const handleThreadReplyNotification = React.useCallback( - (channelId: string, event: RelayEvent) => { + ( + channelId: string, + event: RelayEvent, + parentAuthorPubkey: string | null = null, + ) => { // Guard: don't merge into a buffer whose scope has drifted from the // current identity. isScopeLoaded() also rejects an empty scope, so a // writer can never fire before the first valid scope is seeded. @@ -558,7 +549,10 @@ export function useUnreadChannels( }; const added = addThreadActivityItems(threadActivityRef.current, [item]); if (!added.didAdd) return; - const didRecordMentionedRoot = recordMentionedRoot(event); + const didRecordMentionedRoot = recordMentionedRoot( + event, + parentAuthorPubkey, + ); threadActivityRef.current = added.items; activityPersistence.schedule(currentActivityScope); if (didRecordMentionedRoot) { diff --git a/desktop/src/features/communities/communityUnreadObserver.test.mjs b/desktop/src/features/communities/communityUnreadObserver.test.mjs index 45c19813d18..bf26d8b9671 100644 --- a/desktop/src/features/communities/communityUnreadObserver.test.mjs +++ b/desktop/src/features/communities/communityUnreadObserver.test.mjs @@ -18,6 +18,7 @@ const EMPTY_RELATIONSHIPS = { participatedRootIds: new Set(), followedRootIds: new Set(), authoredRootIds: new Set(), + mentionedRootIds: new Set(), mutedRootIds: new Set(), }; @@ -919,3 +920,57 @@ test("fetchCommunityUnread forced-unread with null baseline + synced marker pres assert.deepEqual(result, { hasUnread: false, mentionCount: 0 }); }); + +test("a DM reply answering the user still counts toward the community badge", async () => { + // Every DM message p-tags both participants, so the "is this a real mention + // or just an addressing tag?" test would throw away exactly the messages the + // badge exists for. In a DM the addressing tag is the point. + const DM_CHANNEL = "dm-channel-1"; + const MY_MESSAGE = "mine".padEnd(64, "0"); + const relay = relayFor([ + // 1. member events + () => [event({ tags: [["d", DM_CHANNEL]] })], + // 2. metadata events + () => [ + event({ + tags: [ + ["d", DM_CHANNEL], + ["t", "dm"], + ], + }), + ], + // 3. visibility events + () => [], + // 4. read-state events + () => [], + // 5. mutes events + () => [], + // 6. unread events + () => [], + // 7. mention events — a reply to one of the user's own DM messages + () => [ + event({ + id: "dmreply".padEnd(64, "0"), + created_at: 30, + tags: [ + ["h", DM_CHANNEL], + ["p", PUBKEY], + ["e", MY_MESSAGE, "", "reply"], + ], + }), + ], + // 8. reply-parent lookup — the parent is the user's own message + () => [event({ id: MY_MESSAGE, pubkey: PUBKEY })], + ]); + + const result = await fetchCommunityUnread({ + client: relay, + pubkey: PUBKEY, + nowSeconds: 100, + decryptReadState: async (value) => value, + decryptMutes: async (value) => value, + readThreadRelationships: readRelationships(), + }); + + assert.deepEqual(result, { hasUnread: true, mentionCount: 1 }); +}); diff --git a/desktop/src/features/communities/communityUnreadObserver.ts b/desktop/src/features/communities/communityUnreadObserver.ts index e729831eac6..a2b8e14f814 100644 --- a/desktop/src/features/communities/communityUnreadObserver.ts +++ b/desktop/src/features/communities/communityUnreadObserver.ts @@ -14,7 +14,11 @@ import { getThreadReference, isBroadcastReply, } from "@/features/messages/lib/threading"; -import { shouldNotifyForEvent } from "@/features/notifications/lib/shouldNotify"; +import { + hasAuthoredMentionForEvent, + shouldNotifyForEvent, +} from "@/features/notifications/lib/shouldNotify"; +import { collectReplyParentAuthors } from "@/features/notifications/lib/replyParentAuthors"; import { mutedChannelIdsFromStore, parseMutePayload, @@ -30,6 +34,7 @@ import { KIND_CHANNEL_MUTES, KIND_DM_VISIBILITY, KIND_READ_STATE, + REPLY_PARENT_EVENT_KINDS, } from "@/shared/constants/kinds"; const KIND_NIP29_GROUP_METADATA = 39000; @@ -39,6 +44,7 @@ const KIND_NIP29_GROUP_MEMBERS = 39002; // so they read correctly from the same origin regardless of which community is active. const participationStore = makeRootIdStore("buzz-thread-participation.v1"); const authoredStore = makeRootIdStore("buzz-thread-authored.v1"); +const mentionedStore = makeRootIdStore("buzz-thread-mentioned.v1"); const mutedRootsStore = makeRootIdStore("buzz-thread-muted.v1"); const FOLLOWS_STORAGE_KEY_PREFIX = "buzz-thread-follows.v1"; @@ -46,6 +52,7 @@ export type ThreadRelationships = { participatedRootIds: ReadonlySet; followedRootIds: ReadonlySet; authoredRootIds: ReadonlySet; + mentionedRootIds: ReadonlySet; mutedRootIds: ReadonlySet; }; @@ -78,6 +85,9 @@ function defaultReadThreadRelationships(pubkey: string): ThreadRelationships { participatedRootIds: participationStore.read(pubkey), followedRootIds: readFollowedRootIds(pubkey), authoredRootIds: authoredStore.read(pubkey), + // Same key `useUnreadChannels` persists to. Read here so this community's + // sidebar dot cannot disagree with the in-app gate about the same thread. + mentionedRootIds: mentionedStore.read(pubkey), mutedRootIds: mutedRootsStore.read(pubkey), }; } @@ -215,6 +225,7 @@ export async function fetchCommunityUnread(args: { participatedRootIds, followedRootIds, authoredRootIds, + mentionedRootIds, mutedRootIds, } = readRelationships(normalizedPubkey); @@ -274,6 +285,43 @@ export async function fetchCommunityUnread(args: { mentionEventsPromise, ]); + // In a DM the addressing tag is the whole point, so no consumer below reads + // the parent's author: `shouldNotifyForEvent` gates `isReplyToCurrentUser` + // on `!isDmChannel`, and the mention filter short-circuits on `isDmChannel` + // outright. Resolving it anyway costs one REQ per DM channel per poll for an + // answer nothing reads — the same round trip `needsResolvedParentAuthor` and + // `collectCatchUpParentAuthors` already decline for this exact reason. + const isDmChannel = channel.channelType === "dm"; + + // Replies p-tag the author they answer, so a reply is indistinguishable + // from a mention until the parent's author is known. Every reply left + // above needs resolving: an unresolved parent counts as a mention and inflates + // the community badge, and `unreadEvents` is empty once an earlier channel + // set `hasUnread`, so a mute-only rule would make the count depend on + // channel order. + // Scoped to this channel: a failure here would otherwise abort the whole + // poll, discarding the counts already accumulated from earlier channels + // and dropping the community to `state: "error"` — which clears its dot + // and badge entirely until the next poll. Undercounting one channel for + // 30 seconds is the smaller wrong answer. + let authorByEventId: Map; + try { + authorByEventId = await collectReplyParentAuthors({ + events: [...unreadEvents, ...mentionEvents], + fetchEvents: (filter) => client.fetchEvents(filter), + // Parent lookup, not an unread query — see REPLY_PARENT_EVENT_KINDS. The + // channel's own unread kinds would miss a diff-message parent and let + // the reply count as a mention. + kinds: REPLY_PARENT_EVENT_KINDS, + shouldResolveParent: () => !isDmChannel, + }); + } catch { + continue; + } + const parentAuthorOf = (event: RelayEvent) => + authorByEventId.get(getThreadReference(event.tags).parentId ?? "") ?? + null; + if (!hasUnread) { hasUnread = unreadEvents.some( (event) => @@ -282,15 +330,33 @@ export async function fetchCommunityUnread(args: { participatedRootIds, followedRootIds, authoredRootIds, + mentionedRootIds, mutedRootIds, mutedChannelIds: mutedIds, channelId: channel.id, + parentAuthorPubkey: parentAuthorOf(event), + isDmChannel, }), ); } - mentionCount += mentionEvents.filter((event) => - isUnreadExternalEvent(event, readState, readAt, normalizedPubkey), + // Count only events that genuinely mention the user — a reply's addressing + // `p` tag would otherwise inflate the community mention badge with every + // answer to one of the user's messages. + // + // DMs are the exception: every message in a DM `p`-tags both participants + // by construction, so that test would throw away exactly the messages the + // badge exists for — someone answering you in a one-to-one conversation. + // In a DM the addressing tag *is* the point. + mentionCount += mentionEvents.filter( + (event) => + isUnreadExternalEvent(event, readState, readAt, normalizedPubkey) && + (isDmChannel || + hasAuthoredMentionForEvent( + event, + normalizedPubkey, + parentAuthorOf(event), + )), ).length; } diff --git a/desktop/src/features/communities/useCommunityInit.ts b/desktop/src/features/communities/useCommunityInit.ts index 5493a47b1e3..6725f599665 100644 --- a/desktop/src/features/communities/useCommunityInit.ts +++ b/desktop/src/features/communities/useCommunityInit.ts @@ -21,6 +21,7 @@ import { initDraftStore, } from "@/features/messages/lib/useDrafts"; import { resetRenderScopedReactionHydration } from "@/features/messages/lib/renderScopedReactions"; +import { resetReplyParentAuthorCache } from "@/features/messages/lib/replyContextEvents"; import { resetBackgroundMediaUploads } from "@/features/messages/lib/backgroundMediaUploadStore"; import { resetLinkPreviewPreparations } from "@/features/messages/lib/linkPreviewPreparationStore"; import { resetPersistentAgentAudienceStore } from "@/features/messages/lib/persistentAgentAudience"; @@ -75,6 +76,7 @@ async function resetCommunityState({ resetLinkPreviewMetadataCache(); resetVideoPlayerState(); resetRenderScopedReactionHydration(); + resetReplyParentAuthorCache(); resetBackgroundMediaUploads(); resetLinkPreviewPreparations(); resetPersistentAgentAudienceStore(); diff --git a/desktop/src/features/forum/hooks.ts b/desktop/src/features/forum/hooks.ts index 2918a694fd0..bcb93b03a24 100644 --- a/desktop/src/features/forum/hooks.ts +++ b/desktop/src/features/forum/hooks.ts @@ -3,6 +3,7 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { getForumPosts, getForumThread } from "@/shared/api/forum"; import { useFocusedRefetchInterval } from "@/shared/lib/useDocumentVisible"; import { useRelaySelfQuery } from "@/features/moderation/hooks"; +import { useProfileQuery } from "@/features/profile/hooks"; import { deleteMessage, sendChannelMessage } from "@/shared/api/tauri"; import type { Channel, @@ -10,6 +11,7 @@ import type { ForumThreadResponse, } from "@/shared/api/types"; import { KIND_FORUM_COMMENT, KIND_FORUM_POST } from "@/shared/constants/kinds"; +import { replyRecipientPubkeys } from "@/features/messages/lib/threading"; /** Keeps focused polling for forum posts at the established 15-second cadence. */ export const FORUM_POSTS_REFETCH_INTERVAL_MS = 15_000; @@ -165,18 +167,29 @@ export function useDeleteForumReplyMutation( }); } -export function useCreateForumReplyMutation(channel: Channel | null) { +export function useCreateForumReplyMutation( + channel: Channel | null, + currentPubkey?: string | null, +) { const queryClient = useQueryClient(); + const profileQuery = useProfileQuery(); return useMutation({ mutationFn: async ({ content, parentEventId, + parentAuthorPubkey, mentionPubkeys, mediaTags, }: { content: string; parentEventId: string; + /** + * Author of the post being answered. Forum channels are mention-eligible + * for agents, and an agent's `require_mention` subscription is a `#p` + * REQ filter — an untagged comment never reaches it. + */ + parentAuthorPubkey?: string | null; mentionPubkeys?: string[]; mediaTags?: string[][]; }) => { @@ -184,12 +197,28 @@ export function useCreateForumReplyMutation(channel: Channel | null) { throw new Error("No channel selected."); } + // Self must be dropped here, not by the caller: `ForumView` compares + // against a `currentPubkey` that is `undefined` until identity resolves, + // so replying to your own post early would self-`p`-tag. Falling back to + // the profile query matters for the same reason — passing `""` would + // seed the dedupe set with the empty string and drop nobody. + // If identity is somehow still unknown, keep the tag: losing it costs + // the agent the comment entirely, which is worse than a stray self-tag. + const selfPubkey = currentPubkey ?? profileQuery.data?.pubkey ?? ""; + const recipientPubkeys = parentAuthorPubkey + ? replyRecipientPubkeys({ + currentPubkey: selfPubkey, + mentionPubkeys: mentionPubkeys ?? [], + parentAuthorPubkey, + }) + : mentionPubkeys; + return sendChannelMessage( channel.id, content, parentEventId, mediaTags, - mentionPubkeys, + recipientPubkeys, KIND_FORUM_COMMENT, ); }, diff --git a/desktop/src/features/forum/ui/ForumView.tsx b/desktop/src/features/forum/ui/ForumView.tsx index 9efada55492..68efa6067ff 100644 --- a/desktop/src/features/forum/ui/ForumView.tsx +++ b/desktop/src/features/forum/ui/ForumView.tsx @@ -58,7 +58,11 @@ export function ForumView({ selectedPostId, ); const createPostMutation = useCreateForumPostMutation(channel); - const createReplyMutation = useCreateForumReplyMutation(channel); + const effectiveCurrentPubkey = currentPubkey ?? profileQuery.data?.pubkey; + const createReplyMutation = useCreateForumReplyMutation( + channel, + effectiveCurrentPubkey, + ); const deletePostMutation = useDeleteForumPostMutation(channel); const deleteReplyMutation = useDeleteForumReplyMutation( channel, @@ -104,7 +108,6 @@ export function ForumView({ const profilesQuery = useUsersBatchQuery(allPubkeys, { enabled: allPubkeys.length > 0, }); - const effectiveCurrentPubkey = currentPubkey ?? profileQuery.data?.pubkey; const profiles = React.useMemo( () => mergeCurrentProfileIntoLookup( @@ -149,6 +152,9 @@ export function ForumView({ createReplyMutation.mutateAsync({ content, parentEventId: selectedPostId, + // A forum comment answers the post root, so p-tag its author. The + // mutation drops self, which is the only case that must not tag. + parentAuthorPubkey: threadQuery.data?.post.pubkey ?? null, mentionPubkeys, mediaTags, }) diff --git a/desktop/src/features/home/lib/inboxReplyRecipients.ts b/desktop/src/features/home/lib/inboxReplyRecipients.ts new file mode 100644 index 00000000000..7ecdf2131e7 --- /dev/null +++ b/desktop/src/features/home/lib/inboxReplyRecipients.ts @@ -0,0 +1,84 @@ +import { + formatInboxFullTimestamp, + type InboxReply, +} from "@/features/home/lib/inbox"; +import { formatTime } from "@/features/messages/lib/dateFormatters"; +import { replyRecipientPubkeys } from "@/features/messages/lib/threading"; +import { resolveUserLabel } from "@/features/profile/lib/identity"; +import type { UserProfileLookup } from "@/features/profile/lib/identity"; +import type { SendChannelMessageResult } from "@/shared/api/types"; +import { normalizePubkey } from "@/shared/lib/pubkey"; + +/** + * Recipient `p` tags for an Inbox reply. + * + * A reply addresses the author it answers (NIP-10). That is not cosmetic here: + * an agent's `require_mention` subscription is a `#p` REQ filter, so an Inbox + * reply without this tag never reaches the agent being answered. The Inbox + * sends without the channel timeline loaded, so the caller supplies the parent + * author rather than the send path resolving it from cache. + */ +export function inboxReplyRecipientPubkeys({ + currentPubkey, + mentionPubkeys, + parentAuthorPubkey, + parentEventId, +}: { + currentPubkey?: string | null; + mentionPubkeys: string[]; + parentAuthorPubkey: string | null; + parentEventId: string | null; +}): string[] { + if (!parentEventId || !parentAuthorPubkey) { + return mentionPubkeys; + } + return replyRecipientPubkeys({ + currentPubkey: currentPubkey ?? "", + mentionPubkeys, + parentAuthorPubkey, + }); +} + +/** + * Optimistic Inbox reply row for the just-sent message. + * + * The Inbox renders replies from its own local list rather than the channel + * timeline, so a sent reply needs this row to appear before the feed refresh + * lands. + */ +export function buildOptimisticInboxReply({ + content, + currentPubkey, + fallbackAuthorPubkey, + profiles, + result, + tags, +}: { + content: string; + currentPubkey?: string | null; + fallbackAuthorPubkey: string; + profiles?: UserProfileLookup; + result: SendChannelMessageResult; + tags: string[][]; +}): InboxReply { + const authorPubkey = currentPubkey ?? fallbackAuthorPubkey; + return { + authorLabel: currentPubkey + ? resolveUserLabel({ currentPubkey, profiles, pubkey: authorPubkey }) + : "You", + authorPubkey, + avatarUrl: + currentPubkey && profiles + ? (profiles[normalizePubkey(currentPubkey)]?.avatarUrl ?? null) + : null, + content, + createdAt: result.createdAt, + depth: result.depth, + fullTimestampLabel: formatInboxFullTimestamp(result.createdAt), + id: result.eventId, + parentId: result.parentEventId, + rootId: result.rootEventId, + tags, + timeLabel: formatTime(result.createdAt), + }; +} diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx index 893b3c309c6..458c89b68f8 100644 --- a/desktop/src/features/home/ui/HomeView.tsx +++ b/desktop/src/features/home/ui/HomeView.tsx @@ -12,7 +12,6 @@ import { type InboxReply, buildInboxItems, findInboxItemByEventId, - formatInboxFullTimestamp, getInboxItemConversationId, } from "@/features/home/lib/inbox"; import { useInboxSelectionAnchor } from "@/features/home/useInboxSelectionAnchor"; @@ -52,13 +51,15 @@ import { useToggleReactionMutation, } from "@/features/messages/hooks"; import { collectMessageMentionPubkeys } from "@/features/messages/lib/formatTimelineMessages"; -import { formatTime } from "@/features/messages/lib/dateFormatters"; import { DeleteMessageConfirmDialog } from "@/features/messages/ui/DeleteMessageConfirmDialog"; import { splitOutgoingTags } from "@/features/messages/lib/imetaMediaMarkdown"; import { getThreadReference } from "@/features/messages/lib/threading"; -import { useUsersBatchQuery } from "@/features/profile/hooks"; +import { + buildOptimisticInboxReply, + inboxReplyRecipientPubkeys, +} from "@/features/home/lib/inboxReplyRecipients"; +import { useProfileQuery, useUsersBatchQuery } from "@/features/profile/hooks"; import { useRelaySelfQuery } from "@/features/moderation/hooks"; -import { resolveUserLabel } from "@/features/profile/lib/identity"; import { useRemindLater } from "@/features/reminders/ui/RemindMeLaterProvider"; import { deleteMessage, sendChannelMessage } from "@/shared/api/tauri"; import type { Channel, HomeFeedResponse } from "@/shared/api/types"; @@ -105,6 +106,8 @@ export function HomeView({ onRefresh, }: HomeViewProps) { const relaySelfPubkey = useRelaySelfQuery().data; + // Identity fallback for the reply path — see its call site below. + const selfProfileQuery = useProfileQuery(); const [homeInboxRef, homeInboxWidthPx] = useElementWidth(); const isNarrowHomeViewport = homeInboxWidthPx > 0 && @@ -834,6 +837,7 @@ export function HomeView({ content, mediaTags, mentionPubkeys, + parentAuthorPubkey, parentEventId, }) => { const channelId = selectedItem?.item.channelId; @@ -849,43 +853,36 @@ export function HomeView({ emojiTags, mentionTags, } = splitOutgoingTags(mediaTags); + const recipientPubkeys = inboxReplyRecipientPubkeys({ + // Falls back to the profile query: `currentPubkey` is + // undefined until the identity query settles, and an empty + // self pubkey makes `normalizeMentionPubkeys` drop nobody + // — so replying to your own Inbox message in that window + // would self-`p`-tag it into your own mention feed. + currentPubkey: + currentPubkey ?? selfProfileQuery.data?.pubkey, + mentionPubkeys, + parentAuthorPubkey, + parentEventId, + }); const result = await sendChannelMessage( channelId, content, parentEventId, imetaTags, - mentionPubkeys, + recipientPubkeys, undefined, emojiTags, mentionTags, ); - const authorPubkey = currentPubkey ?? itemToReply.item.pubkey; - const reply: InboxReply = { - authorLabel: currentPubkey - ? resolveUserLabel({ - currentPubkey, - profiles: feedProfiles, - pubkey: authorPubkey, - }) - : "You", - authorPubkey, - avatarUrl: - currentPubkey && feedProfiles - ? (feedProfiles[currentPubkey.trim().toLowerCase()] - ?.avatarUrl ?? null) - : null, + const reply = buildOptimisticInboxReply({ content, - createdAt: result.createdAt, - depth: result.depth, - fullTimestampLabel: formatInboxFullTimestamp( - result.createdAt, - ), - id: result.eventId, - parentId: result.parentEventId, - rootId: result.rootEventId, + currentPubkey, + fallbackAuthorPubkey: itemToReply.item.pubkey, + profiles: feedProfiles, + result, tags: [...imetaTags, ...emojiTags, ...mentionTags], - timeLabel: formatTime(result.createdAt), - }; + }); setLocalRepliesByItemId((current) => ({ ...current, [itemToReply.conversationId]: [ diff --git a/desktop/src/features/home/ui/InboxDetailPane.tsx b/desktop/src/features/home/ui/InboxDetailPane.tsx index 9192c649521..becbb8cc8a1 100644 --- a/desktop/src/features/home/ui/InboxDetailPane.tsx +++ b/desktop/src/features/home/ui/InboxDetailPane.tsx @@ -130,6 +130,12 @@ type InboxDetailPaneProps = { mediaTags?: string[][]; mentionPubkeys: string[]; parentEventId: string | null; + /** + * Author of `parentEventId`. The Inbox sends without the channel timeline + * loaded, so the caller supplies it — a reply must p-tag the author it + * answers or an agent's `require_mention` subscription never sees it. + */ + parentAuthorPubkey: string | null; }) => Promise; onToggleReaction?: ( message: TimelineMessage, @@ -267,14 +273,27 @@ function InboxMessageDetailPane({ mentionPubkeys: string[], mediaTags?: string[][], parentEventId?: string, - ) => - onSendReply({ + ) => { + const parentId = parentEventId ?? message.id; + // A reply addresses the author it answers, and the Inbox sends without a + // channel timeline loaded, so the caller has to supply that author — the + // backend can only mark the tag it is given. + const commentedOnAuthor = + parentId === message.id ? message.pubkey : undefined; + const parentAuthorPubkey = + displayMessages.find((candidate) => candidate.id === parentId) + ?.authorPubkey ?? + commentedOnAuthor ?? + null; + return onSendReply({ content, mediaTags, mentionPubkeys, - parentEventId: parentEventId ?? message.id, - }), - [onSendReply], + parentAuthorPubkey, + parentEventId: parentId, + }); + }, + [displayMessages, onSendReply], ); const videoReviewPresentation = React.useMemo( () => @@ -471,6 +490,11 @@ function InboxMessageDetailPane({ const composerParentEventId = replyTarget?.id ?? (isDirectMessage ? null : (capturedDefaultParentId ?? item.id)); + const composerParentAuthorPubkey = + displayMessages.find((message) => message.id === composerParentEventId) + ?.authorPubkey ?? + (composerParentEventId === item.id ? item.item.pubkey : null) ?? + null; const composerReplyTarget = replyTarget && replyTarget.id !== item.id ? { @@ -801,6 +825,7 @@ function InboxMessageDetailPane({ content, mediaTags, mentionPubkeys, + parentAuthorPubkey: composerParentAuthorPubkey, parentEventId: composerParentEventId, }) } diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 8b457a7adf8..523d17a66a6 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -31,7 +31,11 @@ import { export { mergeMessages, mergeTimelineCacheMessages }; import { splitOutgoingTags } from "@/features/messages/lib/imetaMediaMarkdown"; -import { messageMentionPubkeys } from "@/features/messages/lib/messageMentionPubkeys"; +import { messageRecipients } from "@/features/messages/lib/messageRecipients"; +import { + findReplyParentAuthor, + getReplyContextEvents, +} from "@/features/messages/lib/replyContextEvents"; import { buildSentFromThreadTag } from "@/features/messages/lib/sentFromThread"; import { clearTimeoutState, @@ -95,11 +99,19 @@ export function createOptimisticMessage( mediaTags: string[][] = [], sentFromThreadRootId: string | null = null, sentFromThreadRootExcerpt: string | null = null, + addressedPubkeys: string[] = [], ): RelayEvent { const localKey = `optimistic-${crypto.randomUUID()}`; const tags: string[][] = []; if (parentEventId) { + // Mirror the published event: a reply p-tags the author it answers. + // `buildReplyTags` dedupes and drops self, so passing it unconditionally + // is safe when the parent is our own message or is not cached. + const parentAuthorPubkey = findReplyParentAuthor( + currentMessages, + parentEventId, + ); tags.push( ...buildReplyTags( channelId, @@ -107,13 +119,15 @@ export function createOptimisticMessage( parentEventId, resolveReplyRootId(parentEventId, currentMessages), mentionPubkeys, + parentAuthorPubkey, + addressedPubkeys, ), ); } else { tags.push(["h", channelId]); tags.push(["p", identity.pubkey]); for (const pubkey of normalizeMentionPubkeys( - mentionPubkeys, + [...mentionPubkeys, ...addressedPubkeys], identity.pubkey, )) { tags.push(["p", pubkey]); @@ -505,10 +519,24 @@ export function useSendMessageMutation( mentionTags, linkPreviewTags, } = splitOutgoingTags(mediaTags); - const recipientPubkeys = messageMentionPubkeys( + // A reply addresses the message it answers (NIP-10). The relay turns + // `require_mention` into a `#p` REQ filter, so omitting this tag makes + // the reply invisible to agent harnesses rather than merely un-notified. + // The thread panel replies to events the channel cache never holds, so + // the lookup spans the thread caches too. + const replyContextEvents = getReplyContextEvents( + queryClient, + effectiveChannel.id, + ); + const parentAuthorPubkey = findReplyParentAuthor( + replyContextEvents, + parentEventId, + ); + const recipients = messageRecipients( effectiveChannel, identity.pubkey, mentionPubkeys, + parentAuthorPubkey, ); if (sentFromThreadRootId && parentEventId) { throw new Error( @@ -534,21 +562,20 @@ export function useSendMessageMutation( emojiTags.length > 0 || linkPreviewTags.length > 0 ) { - const cachedMessages = - queryClient.getQueryData( - channelMessagesKey(effectiveChannel.id), - ) ?? []; const result = await sendChannelMessage( effectiveChannel.id, content, parentEventId ?? null, imetaTags, - recipientPubkeys, + recipients.mentions, undefined, emojiTags, mentionTags, linkPreviewTags, sentFromThreadTag, + undefined, // expectedRelayUrl — this path is not tenant-pinned + undefined, // expectedSignerPubkey + recipients.addressed, ); // Build tags matching relay-emitted shape: h, author p, mention ps, reply es, imeta, emoji. @@ -559,8 +586,10 @@ export function useSendMessageMutation( effectiveChannel.id, identity.pubkey, parentEventId, - resolveReplyRootId(parentEventId, cachedMessages), - recipientPubkeys, + resolveReplyRootId(parentEventId, replyContextEvents), + recipients.mentions, + parentAuthorPubkey, + recipients.addressed, ) : []; const baseTags = parentEventId @@ -579,9 +608,10 @@ export function useSendMessageMutation( ...baseTags, // For non-replies, add mention p-tags here (replies get them via buildReplyTags) ...(!parentEventId - ? normalizeMentionPubkeys(recipientPubkeys, identity.pubkey).map( - (pk) => ["p", pk], - ) + ? normalizeMentionPubkeys( + [...recipients.mentions, ...recipients.addressed], + identity.pubkey, + ).map((pk) => ["p", pk]) : []), ...imetaTags, ...emojiTags, @@ -597,7 +627,7 @@ export function useSendMessageMutation( return relayClient.sendMessage( effectiveChannel.id, content, - recipientPubkeys, + [...recipients.mentions, ...recipients.addressed], [...mentionTags, ...(sentFromThreadTag ? [sentFromThreadTag] : [])], ); }, @@ -637,16 +667,34 @@ export function useSendMessageMutation( const windowKey = channelWindowKey(effectiveChannel.id); const previousWindow = queryClient.getQueryData(windowKey); + // Wider than `previousMessages` (the rollback snapshot) on purpose: the + // optimistic row must resolve the same parent author and root as the + // published event, including thread-panel parents. + const replyContextEvents = getReplyContextEvents( + queryClient, + effectiveChannel.id, + ); const optimisticMessage = createOptimisticMessage( effectiveChannel.id, content.trim(), identity, - previousMessages, + replyContextEvents, mentionPubkeys ?? [], parentEventId ?? null, mediaTags ?? [], sentFromThreadRootId ?? null, sentFromThreadRootExcerpt ?? null, + // A DM tags its other participants whether or not anyone typed their + // names; mirror that here so the optimistic row matches what is sent. + // The parent's author goes in too: it reserves a cap slot in the send + // path, so omitting it here lets the optimistic row carry an addressed + // tag the published event dropped. + messageRecipients( + effectiveChannel, + identity.pubkey, + mentionPubkeys ?? [], + findReplyParentAuthor(replyContextEvents, parentEventId), + ).addressed, ); const nextWindow = mergeLiveChannelWindowEvent( diff --git a/desktop/src/features/messages/lib/messageMentionPubkeys.test.mjs b/desktop/src/features/messages/lib/messageMentionPubkeys.test.mjs deleted file mode 100644 index 34cf76a9994..00000000000 --- a/desktop/src/features/messages/lib/messageMentionPubkeys.test.mjs +++ /dev/null @@ -1,48 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { messageMentionPubkeys } from "./messageMentionPubkeys.ts"; - -function channel(overrides = {}) { - return { - id: "dm-1", - name: "DM", - channelType: "dm", - visibility: "private", - description: "", - topic: null, - purpose: null, - memberCount: 2, - memberPubkeys: ["OWNER", "AGENT"], - participantPubkeys: ["owner", "agent"], - participants: [], - lastMessageAt: null, - archivedAt: null, - isMember: true, - ttlSeconds: null, - ttlDeadline: null, - ...overrides, - }; -} - -test("plain DM messages p-tag every recipient except the sender", () => { - assert.deepEqual(messageMentionPubkeys(channel(), "owner"), ["agent"]); -}); - -test("DM recipients and explicit mentions are normalized and deduplicated", () => { - assert.deepEqual( - messageMentionPubkeys(channel(), "OWNER", ["AGENT", "third"]), - ["agent", "third"], - ); -}); - -test("stream messages preserve explicit-mention semantics", () => { - assert.deepEqual( - messageMentionPubkeys( - channel({ channelType: "stream", memberPubkeys: ["owner", "agent"] }), - "owner", - [], - ), - [], - ); -}); diff --git a/desktop/src/features/messages/lib/messageMentionPubkeys.ts b/desktop/src/features/messages/lib/messageMentionPubkeys.ts deleted file mode 100644 index 638d8d16940..00000000000 --- a/desktop/src/features/messages/lib/messageMentionPubkeys.ts +++ /dev/null @@ -1,30 +0,0 @@ -import type { Channel } from "@/shared/api/types"; -import { normalizePubkey } from "@/shared/lib/pubkey"; - -/** - * Return the semantic recipients for an outgoing message. - * - * Stream messages notify only explicit mentions. A DM addresses every other - * participant, so it must carry recipient `p` tags even when the composer text - * contains no `@mention`. Agent harnesses and human notification subscriptions - * both rely on those tags. - */ -export function messageMentionPubkeys( - channel: Channel, - senderPubkey: string, - explicitMentions: readonly string[] = [], -): string[] { - const candidates = - channel.channelType === "dm" - ? [ - ...explicitMentions, - ...channel.memberPubkeys, - ...channel.participantPubkeys, - ] - : explicitMentions; - const sender = normalizePubkey(senderPubkey); - - return [...new Set(candidates.map(normalizePubkey))].filter( - (pubkey) => pubkey.length > 0 && pubkey !== sender, - ); -} diff --git a/desktop/src/features/messages/lib/messageRecipients.test.mjs b/desktop/src/features/messages/lib/messageRecipients.test.mjs new file mode 100644 index 00000000000..b6da8165397 --- /dev/null +++ b/desktop/src/features/messages/lib/messageRecipients.test.mjs @@ -0,0 +1,124 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { messageRecipients } from "./messageRecipients.ts"; +import { MENTION_TAG_CAP } from "./threading.ts"; + +function channel(overrides = {}) { + return { + id: "dm-1", + name: "DM", + channelType: "dm", + visibility: "private", + description: "", + topic: null, + purpose: null, + memberCount: 2, + memberPubkeys: ["OWNER", "AGENT"], + participantPubkeys: ["owner", "agent"], + participants: [], + lastMessageAt: null, + archivedAt: null, + isMember: true, + ttlSeconds: null, + ttlDeadline: null, + ...overrides, + }; +} + +const stream = (overrides = {}) => + channel({ + channelType: "stream", + memberPubkeys: ["owner", "agent"], + ...overrides, + }); + +test("plain DM messages p-tag every recipient except the sender", () => { + assert.deepEqual(messageRecipients(channel(), "owner"), { + mentions: [], + addressed: ["agent"], + }); +}); + +test("a DM counterpart is addressed by the channel, not mentioned", () => { + // The distinction is the point: as a mention this tag would pierce a mute and + // outrank a real `@you` in the mention feed. Nobody typed the counterpart's + // name — the channel has exactly one other participant. + const { mentions, addressed } = messageRecipients(channel(), "owner"); + assert.deepEqual(mentions, []); + assert.deepEqual(addressed, ["agent"]); +}); + +test("a DM participant who was also typed counts only as a mention", () => { + assert.deepEqual(messageRecipients(channel(), "OWNER", ["AGENT", "third"]), { + mentions: ["agent", "third"], + addressed: [], + }); +}); + +test("stream messages preserve explicit-mention semantics", () => { + assert.deepEqual(messageRecipients(stream(), "owner", []), { + mentions: [], + addressed: [], + }); +}); + +test("stream replies leave the addressing tag to the backend", () => { + // The parent's author is no longer folded in here. `resolve_thread_ref` + // already fetches the parent event on the way to the thread root, so the + // backend emits that tag itself — marked, and without depending on a client + // cache that misses for every channel not opened this session. + assert.deepEqual(messageRecipients(stream(), "owner", [], "AGENT"), { + mentions: [], + addressed: [], + }); +}); + +test("replying to your own message reserves no addressing slot", () => { + // Only observable at the cap. Away from it both branches return the same + // empty result, so the self check has nothing to prove: the backend never + // emits an addressing tag for the signer, so no slot is held back for one. + const typed = Array.from({ length: 60 }, (_, i) => `mention-${i}`); + const { mentions, addressed } = messageRecipients( + stream(), + "owner", + typed, + "owner", + ); + assert.equal(mentions.length, MENTION_TAG_CAP); + assert.deepEqual(addressed, []); + assert.ok(!mentions.includes("owner")); +}); + +test("a reply to someone already mentioned reserves no second slot", () => { + // The parent author is already typed, so they are emitted once as a mention + // and cost one slot rather than two — again visible only where the cap bites. + const typed = Array.from({ length: 60 }, (_, i) => `mention-${i}`); + const { mentions } = messageRecipients(stream(), "owner", typed, "mention-0"); + assert.equal(mentions.length, MENTION_TAG_CAP); + assert.equal(mentions.filter((pubkey) => pubkey === "mention-0").length, 1); +}); + +test("mentions keep their slots ahead of channel recipients at the cap", () => { + // 50 typed mentions fill the cap outright, so a DM counterpart who was never + // typed is what gives — the same order the single merged list produced. + const typed = Array.from({ length: 60 }, (_, i) => `mention-${i}`); + const { mentions, addressed } = messageRecipients(channel(), "owner", typed); + assert.equal(mentions.length, 50); + assert.deepEqual(addressed, []); +}); + +test("the addressing slot is reserved out of the mention list", () => { + const typed = Array.from({ length: 60 }, (_, i) => `mention-${i}`); + const { mentions } = messageRecipients(stream(), "owner", typed, "parent"); + assert.equal(mentions.length, 49); +}); + +test("a reserved slot is not taken twice when the parent is a DM recipient", () => { + // `agent` is both the DM's other participant and the parent's author. The + // backend emits one tag for them, marked as addressing, so no extra slot is + // needed and the mention list keeps its full cap. + const typed = Array.from({ length: 60 }, (_, i) => `mention-${i}`); + const { mentions } = messageRecipients(channel(), "owner", typed, "agent"); + assert.equal(mentions.length, 50); +}); diff --git a/desktop/src/features/messages/lib/messageRecipients.ts b/desktop/src/features/messages/lib/messageRecipients.ts new file mode 100644 index 00000000000..5e264ffbdec --- /dev/null +++ b/desktop/src/features/messages/lib/messageRecipients.ts @@ -0,0 +1,77 @@ +import type { Channel } from "@/shared/api/types"; +import { normalizePubkey } from "@/shared/lib/pubkey"; +import { MENTION_TAG_CAP } from "@/features/messages/lib/threading"; + +/** Who an outgoing message `p`-tags, split by why it tags them. */ +export interface MessageRecipients { + /** Typed as `@name` in the body. Marked `mention` on a reply. */ + mentions: string[]; + /** + * Addressed by the *channel* rather than by the message — every other + * participant in a DM, tagged whether or not anyone typed their name. + * Never marked: neither role is true of it. + */ + addressed: string[]; +} + +/** + * Return the semantic recipients for an outgoing message, split by role. + * + * Stream messages notify only explicit mentions. A DM addresses every other + * participant, so it must carry recipient `p` tags even when the composer text + * contains no `@mention`. Agent harnesses and human notification subscriptions + * both rely on those tags. + * + * The two groups stay apart because a reply marks its `p` tags with the role + * each one plays. Folding DM participants in with typed mentions made every DM + * thread reply claim its counterpart had been `@`-mentioned — which pierces a + * mute and takes a slot in the mention feed ahead of a real `@you`. + * + * A thread reply also addresses the author it replies to, per NIP-10, but that + * tag is **not** added here. The backend adds it, marked, from the parent event + * `resolve_thread_ref` already fetches — which is both more reliable (this + * function only ever saw a *cached* parent author, and the cache misses for any + * channel not opened this session) and unambiguous on the wire. All + * `parentAuthorPubkey` does here is reserve the tag's slot against the cap. + */ +export function messageRecipients( + channel: Channel, + senderPubkey: string, + explicitMentions: readonly string[] = [], + parentAuthorPubkey?: string | null, +): MessageRecipients { + const sender = normalizePubkey(senderPubkey); + const keep = (pubkey: string) => pubkey.length > 0 && pubkey !== sender; + + const mentions = [...new Set(explicitMentions.map(normalizePubkey))].filter( + keep, + ); + + const addressed = + channel.channelType === "dm" + ? [ + ...new Set( + [...channel.memberPubkeys, ...channel.participantPubkeys].map( + normalizePubkey, + ), + ), + ].filter((pubkey) => keep(pubkey) && !mentions.includes(pubkey)) + : []; + + // Past the cap the builder rejects the whole event rather than trimming, so + // an over-full list is a failed send. The addressing tag the backend appends + // counts toward that cap, so leave it a slot — an agent's `require_mention` + // subscription never sees an untagged reply, while a dropped body mention + // only costs one notification. + const parent = normalizePubkey(parentAuthorPubkey ?? ""); + const reservesAddressingSlot = + keep(parent) && !mentions.includes(parent) && !addressed.includes(parent); + const cap = reservesAddressingSlot ? MENTION_TAG_CAP - 1 : MENTION_TAG_CAP; + + // Mentions keep their places ahead of channel recipients, as they did when + // the two were one list sliced from the front. + return { + mentions: mentions.slice(0, cap), + addressed: addressed.slice(0, Math.max(0, cap - mentions.length)), + }; +} diff --git a/desktop/src/features/messages/lib/replyContextEvents.test.mjs b/desktop/src/features/messages/lib/replyContextEvents.test.mjs new file mode 100644 index 00000000000..c1c70ef7649 --- /dev/null +++ b/desktop/src/features/messages/lib/replyContextEvents.test.mjs @@ -0,0 +1,293 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + findReplyParentAuthor, + getReplyContextEvents, + lookupReplyParentAuthor, + resetReplyParentAuthorCache, + resolveReplyParentAuthor, +} from "./replyContextEvents.ts"; + +const CHANNEL_ID = "11111111-1111-4111-8111-111111111111"; +const OTHER_CHANNEL_ID = "22222222-2222-4222-8222-222222222222"; +const AUTHOR = "a".repeat(64); + +// Real event ids, because `resolveReplyParentAuthor` now rejects anything that +// cannot identify a relay event before it reaches an `ids` filter. +const M1 = "1".repeat(64); +const M2 = "2".repeat(64); + +function makeEvent(id, pubkey = AUTHOR) { + return { + id, + pubkey, + created_at: 1700000000, + kind: 9, + tags: [], + content: "hi", + sig: "s".repeat(128), + }; +} + +/** Minimal QueryClient stand-in covering the two reads the helper makes. */ +function makeQueryClient({ channelMessages = [], threadCaches = {} } = {}) { + return { + getQueryData: (key) => + key[0] === "channel-messages" && key[1] === CHANNEL_ID + ? channelMessages + : undefined, + getQueriesData: ({ queryKey }) => + Object.entries(threadCaches) + .filter(([cacheKey]) => cacheKey.startsWith(`${queryKey[1]}:`)) + .map(([cacheKey, events]) => [ + ["thread-replies", ...cacheKey.split(":")], + events, + ]), + }; +} + +test("returns the channel timeline when no thread cache exists", () => { + const message = makeEvent(M1); + const client = makeQueryClient({ channelMessages: [message] }); + assert.deepEqual(getReplyContextEvents(client, CHANNEL_ID), [message]); +}); + +test("includes thread-panel replies the channel cache never holds", () => { + const timeline = makeEvent("root-1"); + const nested = makeEvent("reply-1", "b".repeat(64)); + const client = makeQueryClient({ + channelMessages: [timeline], + threadCaches: { [`${CHANNEL_ID}:root-1`]: [nested] }, + }); + const events = getReplyContextEvents(client, CHANNEL_ID); + assert.deepEqual( + events.map((event) => event.id), + ["root-1", "reply-1"], + ); +}); + +test("does not leak another channel's thread cache", () => { + const client = makeQueryClient({ + channelMessages: [], + threadCaches: { [`${OTHER_CHANNEL_ID}:root-1`]: [makeEvent("reply-1")] }, + }); + assert.deepEqual(getReplyContextEvents(client, CHANNEL_ID), []); +}); + +test("tolerates an empty cache", () => { + const client = { getQueryData: () => undefined, getQueriesData: () => [] }; + assert.deepEqual(getReplyContextEvents(client, CHANNEL_ID), []); +}); + +test("findReplyParentAuthor resolves the author of the parent event", () => { + const events = [makeEvent(M1), makeEvent(M2, "c".repeat(64))]; + assert.equal(findReplyParentAuthor(events, M2), "c".repeat(64)); +}); + +test("findReplyParentAuthor returns null for an uncached or absent parent", () => { + assert.equal(findReplyParentAuthor([makeEvent(M1)], "missing"), null); + assert.equal(findReplyParentAuthor([makeEvent(M1)], null), null); + assert.equal(findReplyParentAuthor([makeEvent(M1)], undefined), null); +}); + +test("findReplyParentAuthor treats a blank pubkey as absent", () => { + assert.equal(findReplyParentAuthor([makeEvent(M1, " ")], M1), null); +}); + +test("lookupReplyParentAuthor short-circuits when there is no parent", () => { + const client = { + getQueryData: () => { + throw new Error("should not read"); + }, + getQueriesData: () => { + throw new Error("should not scan"); + }, + }; + assert.equal(lookupReplyParentAuthor(client, CHANNEL_ID, null), null); + assert.equal(lookupReplyParentAuthor(client, CHANNEL_ID, undefined), null); +}); + +test("lookupReplyParentAuthor prefers the channel cache and skips the scan", () => { + const client = { + getQueryData: () => [makeEvent(M1, "d".repeat(64))], + getQueriesData: () => { + throw new Error("should not scan on a channel-cache hit"); + }, + }; + assert.equal(lookupReplyParentAuthor(client, CHANNEL_ID, M1), "d".repeat(64)); +}); + +test("lookupReplyParentAuthor falls back to the thread caches on a miss", () => { + const client = makeQueryClient({ + channelMessages: [makeEvent("root-1")], + threadCaches: { + [`${CHANNEL_ID}:root-1`]: [makeEvent("nested", "e".repeat(64))], + }, + }); + assert.equal( + lookupReplyParentAuthor(client, CHANNEL_ID, "nested"), + "e".repeat(64), + ); + assert.equal(lookupReplyParentAuthor(client, CHANNEL_ID, "absent"), null); +}); + +test("resolveReplyParentAuthor answers from the cache without a fetch", async () => { + const client = makeQueryClient({ channelMessages: [makeEvent(M1)] }); + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: () => { + throw new Error("should not fetch"); + }, + kinds: [9], + parentEventId: M1, + queryClient: client, + }); + assert.deepEqual(result, { pubkey: AUTHOR, status: "resolved" }); +}); + +test("resolveReplyParentAuthor falls back to the relay with kinds set", async () => { + resetReplyParentAuthorCache(); + const client = makeQueryClient(); + const requested = []; + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async (filter) => { + requested.push(filter); + return [makeEvent(M1)]; + }, + kinds: [9, 40008], + parentEventId: M1, + queryClient: client, + }); + assert.deepEqual(result, { pubkey: AUTHOR, status: "resolved" }); + // `kinds` is required — an open-ended filter hits the relay p-gate (403). + assert.deepEqual(requested[0].kinds, [9, 40008]); + assert.deepEqual(requested[0].ids, [M1]); +}); + +test("a parent that is genuinely gone reports absent, not unavailable", async () => { + resetReplyParentAuthorCache(); + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => [], + kinds: [9], + parentEventId: M1, + queryClient: makeQueryClient(), + }); + assert.deepEqual(result, { pubkey: null, status: "absent" }); +}); + +test("a failed fetch retries before reporting unavailable", async () => { + resetReplyParentAuthorCache(); + let attempts = 0; + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => { + attempts += 1; + throw new Error("relay down"); + }, + kinds: [9], + parentEventId: M1, + queryClient: makeQueryClient(), + }); + assert.deepEqual(result, { pubkey: null, status: "unavailable" }); + // The caller persists its verdict per event id and never recomputes it, so + // a guess made during a brief blip would be permanent. + assert.equal(attempts, 3); +}); + +test("a transient failure resolves once the relay recovers", async () => { + resetReplyParentAuthorCache(); + let attempts = 0; + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => { + attempts += 1; + if (attempts === 1) throw new Error("relay flap"); + return [makeEvent(M1)]; + }, + kinds: [9], + parentEventId: M1, + queryClient: makeQueryClient(), + }); + assert.deepEqual(result, { pubkey: AUTHOR, status: "resolved" }); +}); + +test("an absent parent is not cached, so a later lookup can still find it", async () => { + resetReplyParentAuthorCache(); + const client = makeQueryClient(); + let calls = 0; + const resolve = () => + resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => { + calls += 1; + // A parent missing from one query may simply be a kind outside + // `kinds`; caching that would poison every later reply to it. + return calls === 1 ? [] : [makeEvent(M1)]; + }, + kinds: [9], + parentEventId: M1, + queryClient: client, + }); + assert.deepEqual(await resolve(), { pubkey: null, status: "absent" }); + assert.deepEqual(await resolve(), { pubkey: AUTHOR, status: "resolved" }); +}); + +test("no parent short-circuits to absent without a fetch", async () => { + const result = await resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: () => { + throw new Error("should not fetch"); + }, + kinds: [9], + parentEventId: null, + queryClient: makeQueryClient(), + }); + assert.deepEqual(result, { pubkey: null, status: "absent" }); +}); + +test("concurrent lookups of one parent share a single fetch", async () => { + resetReplyParentAuthorCache(); + const client = makeQueryClient(); + let fetches = 0; + const resolve = () => + resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => { + fetches += 1; + return [makeEvent(M1)]; + }, + kinds: [9], + parentEventId: M1, + queryClient: client, + }); + // 30 replies to one message must not cost 30 identical relay subscriptions. + const results = await Promise.all(Array.from({ length: 30 }, resolve)); + assert.equal(fetches, 1); + for (const result of results) { + assert.deepEqual(result, { pubkey: AUTHOR, status: "resolved" }); + } +}); + +test("a lookup that exhausts its retries is not cached", async () => { + resetReplyParentAuthorCache(); + const client = makeQueryClient(); + let down = true; + const resolve = () => + resolveReplyParentAuthor({ + channelId: CHANNEL_ID, + fetchEvents: async () => { + if (down) throw new Error("relay down"); + return [makeEvent(M1)]; + }, + kinds: [9], + parentEventId: M1, + queryClient: client, + }); + assert.deepEqual(await resolve(), { pubkey: null, status: "unavailable" }); + down = false; + // Caching the failure would make one outage sticky for the whole session. + assert.deepEqual(await resolve(), { pubkey: AUTHOR, status: "resolved" }); +}); diff --git a/desktop/src/features/messages/lib/replyContextEvents.ts b/desktop/src/features/messages/lib/replyContextEvents.ts new file mode 100644 index 00000000000..160fe0967f9 --- /dev/null +++ b/desktop/src/features/messages/lib/replyContextEvents.ts @@ -0,0 +1,212 @@ +import type { QueryClient } from "@tanstack/react-query"; + +import type { RelayEvent } from "@/shared/api/types"; +import { channelMessagesKey } from "./messageQueryKeys"; +import { normalizeEventId } from "./threading"; + +/** + * Every cached event we might be replying to in `channelId`. + * + * The channel timeline is not the only place a reply target lives: the thread + * panel keeps its replies in a separate `["thread-replies", channelId, rootId]` + * cache, and the Inbox can answer a message in a channel the user never opened + * this session. Looking only at the channel cache silently misses both, which + * costs the reply its parent-author `p` tag and collapses its root to the + * parent id. + */ +export function getReplyContextEvents( + queryClient: QueryClient, + channelId: string, +): RelayEvent[] { + const channelMessages = + queryClient.getQueryData(channelMessagesKey(channelId)) ?? []; + const threadReplies = queryClient + .getQueriesData({ queryKey: ["thread-replies", channelId] }) + .flatMap(([, events]) => events ?? []); + + return threadReplies.length === 0 + ? channelMessages + : [...channelMessages, ...threadReplies]; +} + +/** + * Author of the message a reply answers, or `null` when it is not cached. + */ +export function findReplyParentAuthor( + events: readonly RelayEvent[], + parentEventId: string | null | undefined, +): string | null { + if (!parentEventId) { + return null; + } + return ( + events.find((event) => event.id === parentEventId)?.pubkey?.trim() || null + ); +} + +/** + * Parent author for a single reply, without materializing the whole context. + * + * The channel timeline holds the parent for nearly every reply, so the + * thread-reply caches — whose lookup scans the entire query cache — are only + * consulted on a miss. Returns `null` immediately when there is no parent, so + * the common non-reply message costs nothing. + */ +export function lookupReplyParentAuthor( + queryClient: QueryClient, + channelId: string, + parentEventId: string | null | undefined, +): string | null { + if (!parentEventId) { + return null; + } + const fromChannel = findReplyParentAuthor( + queryClient.getQueryData(channelMessagesKey(channelId)) ?? [], + parentEventId, + ); + if (fromChannel !== null) { + return fromChannel; + } + for (const [, events] of queryClient.getQueriesData({ + queryKey: ["thread-replies", channelId], + })) { + const found = findReplyParentAuthor(events ?? [], parentEventId); + if (found !== null) { + return found; + } + } + return null; +} + +/** + * Outcome of resolving the author a reply answers. + * + * `absent` and `unavailable` have to stay distinct. Ownership of a reply + * notification is decided twice — once here and once by the Inbox feed's + * server-side lookup — and the two only agree if a lookup that *failed* is + * treated differently from a parent that genuinely is not there. Collapsing + * both to `null` makes one relay hiccup drop the event on both sides. + */ +export type ReplyParentAuthorResult = + | { pubkey: string; status: "resolved" } + | { pubkey: null; status: "absent" } + | { pubkey: null; status: "unavailable" }; + +const PARENT_ABSENT = { pubkey: null, status: "absent" } as const; +const PARENT_UNAVAILABLE = { pubkey: null, status: "unavailable" } as const; + +/** + * In-flight and settled parent lookups, keyed by parent event id. + * + * A parent is answered once, not once per reply. Thirty replies to the same + * message would otherwise issue thirty identical `#ids` REQs, each a fresh + * subscription queued behind the rate-limit gate that foreground channel + * history also uses. Failures are deliberately not cached — `unavailable` + * means "try again", and caching it would make one relay flap sticky. + * + * Community-scoped: cleared by `resetCommunityState()`. + */ +const parentAuthorCache = new Map>(); +const PARENT_AUTHOR_CACHE_LIMIT = 500; + +/** Backoff between parent-lookup attempts. Length is the retry count. */ +const PARENT_FETCH_RETRY_DELAYS_MS = [500, 2_000] as const; + +const sleep = (ms: number) => + new Promise((resolve) => setTimeout(resolve, ms)); + +/** Clears the parent-author cache. Wired into `resetCommunityState()`. */ +export function resetReplyParentAuthorCache() { + parentAuthorCache.clear(); +} + +/** + * Parent author for a single reply, falling back to the relay on a cache miss. + * + * The query caches only cover channels the user has opened this session, and + * `useLiveChannelUpdates` deliberately does not seed them. Without the relay + * fallback, "did this reply answer me?" is unanswerable for every unopened + * channel — which is where the reply notifications matter most. + */ +export async function resolveReplyParentAuthor({ + channelId, + fetchEvents, + kinds, + parentEventId, + queryClient, +}: { + channelId: string; + fetchEvents: (filter: { + kinds: number[]; + ids: string[]; + limit: number; + }) => Promise; + kinds: readonly number[]; + parentEventId: string | null | undefined; + queryClient: QueryClient; +}): Promise { + // Validated, not just presence-checked. The relay stores an `e` value it cannot + // parse as an event id, and puts into an `ids` filter it answers with a bare + // NOTICE — which this client never resolves, so the request hangs the history + // timeout before rejecting. Treat an unlookupable parent as absent instead. + parentEventId = normalizeEventId(parentEventId); + if (!parentEventId) { + return PARENT_ABSENT; + } + const cached = lookupReplyParentAuthor(queryClient, channelId, parentEventId); + if (cached !== null) { + return { pubkey: cached, status: "resolved" }; + } + + const inFlight = parentAuthorCache.get(parentEventId); + if (inFlight) { + return inFlight; + } + + const pending = (async (): Promise => { + for (let attempt = 0; ; attempt += 1) { + let events: RelayEvent[]; + try { + // `kinds` is required — an open-ended filter hits the relay p-gate. + events = await fetchEvents({ + kinds: [...kinds], + ids: [parentEventId], + limit: 1, + }); + } catch { + // Worth retrying rather than guessing. The caller's verdict is + // persisted per event id and never recomputed, so a guess made during + // a two-second relay blip is permanent for as long as the channel + // stays unread — the toast and the mention badge would disagree + // forever. Retries are per parent id, not per reply, because + // concurrent callers share this promise. + if (attempt < PARENT_FETCH_RETRY_DELAYS_MS.length) { + await sleep(PARENT_FETCH_RETRY_DELAYS_MS[attempt] as number); + continue; + } + parentAuthorCache.delete(parentEventId); + return PARENT_UNAVAILABLE; + } + + const author = findReplyParentAuthor(events, parentEventId); + if (author !== null) { + return { pubkey: author, status: "resolved" }; + } + // A parent the relay does not return is not necessarily gone — it may + // simply be a kind outside `kinds`. Caching that would poison every + // later reply to the same parent, and the two consumers read a null + // author in opposite directions. + parentAuthorCache.delete(parentEventId); + return PARENT_ABSENT; + } + })(); + + if (parentAuthorCache.size >= PARENT_AUTHOR_CACHE_LIMIT) { + const oldest = parentAuthorCache.keys().next().value; + if (oldest !== undefined) { + parentAuthorCache.delete(oldest); + } + } + parentAuthorCache.set(parentEventId, pending); + return pending; +} diff --git a/desktop/src/features/messages/lib/replyRecipientPubkeys.test.mjs b/desktop/src/features/messages/lib/replyRecipientPubkeys.test.mjs new file mode 100644 index 00000000000..ba1791d3fe0 --- /dev/null +++ b/desktop/src/features/messages/lib/replyRecipientPubkeys.test.mjs @@ -0,0 +1,68 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { MENTION_TAG_CAP, replyRecipientPubkeys } from "./threading.ts"; + +const ME = "a".repeat(64); +const PARENT = "b".repeat(64); +const body = (n) => + Array.from({ length: n }, (_, i) => `${i}`.padStart(64, "c")); + +test("appends the parent author and drops self", () => { + assert.deepEqual( + replyRecipientPubkeys({ + currentPubkey: ME, + mentionPubkeys: [ME, PARENT.toUpperCase()], + parentAuthorPubkey: PARENT, + }), + [PARENT], + ); +}); + +test("a full mention list leaves room for the addressing tag", () => { + // The addressing tag is no longer in this list — the backend appends it, + // marked, from the parent event it already fetched. What this function still + // owes is the slot: the builder rejects the whole event past the cap rather + // than trimming, so a full body list plus the backend's tag is a failed send, + // and losing the addressing tag would cost the agent the reply entirely. + const result = replyRecipientPubkeys({ + currentPubkey: ME, + mentionPubkeys: body(MENTION_TAG_CAP), + parentAuthorPubkey: PARENT, + }); + assert.equal(result.length, MENTION_TAG_CAP - 1); + assert.ok(!result.includes(PARENT)); +}); + +test("an under-cap list is left alone", () => { + const result = replyRecipientPubkeys({ + currentPubkey: ME, + mentionPubkeys: body(3), + parentAuthorPubkey: PARENT, + }); + assert.equal(result.length, 3); + assert.ok(!result.includes(PARENT)); +}); + +test("a parent already mentioned in the body is not duplicated", () => { + const result = replyRecipientPubkeys({ + currentPubkey: ME, + mentionPubkeys: [PARENT, ...body(2)], + parentAuthorPubkey: PARENT, + }); + assert.deepEqual( + result.filter((pk) => pk === PARENT), + [PARENT], + ); +}); + +test("no parent author leaves the list untouched", () => { + assert.deepEqual( + replyRecipientPubkeys({ + currentPubkey: ME, + mentionPubkeys: body(2), + parentAuthorPubkey: null, + }), + body(2), + ); +}); diff --git a/desktop/src/features/messages/lib/threading.test.mjs b/desktop/src/features/messages/lib/threading.test.mjs new file mode 100644 index 00000000000..7e3512d907d --- /dev/null +++ b/desktop/src/features/messages/lib/threading.test.mjs @@ -0,0 +1,192 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + buildReplyTags, + getThreadReference, + normalizeEventId, + pTagRoleFor, + P_TAG_ADDRESSING_MARKER, + P_TAG_MENTION_MARKER, +} from "./threading.ts"; + +test("a malformed e-tag value still groups, but never reaches a filter", () => { + // Two separate concerns, deliberately split. `getThreadReference` is the general + // thread-grouping primitive, and a value that cannot identify a relay event is + // still a usable grouping key — so it passes through here. What must not happen + // is that value reaching an `ids` REQ filter: the relay stores such an event + // (its NIP-10 resolver ignores the tag rather than rejecting the event, so any + // member can publish one) and answers the resulting filter with a bare NOTICE — + // no CLOSED, no EOSE — which this client never resolves, so the request hangs + // the history timeout and then rejects. `normalizeEventId` is the gate for that, + // and every filter-building caller must use it. + for (const bad of ["not-a-real-id", "abc", "z".repeat(64), "a".repeat(63)]) { + const ref = getThreadReference([ + ["e", "b".repeat(64), "", "root"], + ["e", bad, "", "reply"], + ]); + assert.equal(ref.parentId, bad, `expected ${bad} to survive grouping`); + assert.equal( + normalizeEventId(ref.parentId), + null, + `expected ${bad} to be gated`, + ); + } +}); + +test("an uppercase e-tag value is normalized to lowercase", () => { + // Hex decodes case-insensitively, so the relay resolves an uppercase id fine — + // but every comparison is against `event.id`, which is always lowercase. The + // mismatch read as "parent absent", which relabels a reply a real mention: + // a second notification on top of the live path's, and a pierced mute. + const upper = "A".repeat(64); + const ref = getThreadReference([["e", upper, "", "reply"]]); + + assert.equal(ref.parentId, "a".repeat(64)); + assert.equal(ref.rootId, "a".repeat(64)); +}); + +test("a valid lowercase reference passes through unchanged", () => { + const root = "1".repeat(64); + const parent = "2".repeat(64); + const ref = getThreadReference([ + ["e", root, "", "root"], + ["e", parent, "", "reply"], + ]); + + assert.deepEqual(ref, { parentId: parent, rootId: root }); +}); + +test("normalizeEventId accepts only canonical 64-char hex", () => { + assert.equal(normalizeEventId("f".repeat(64)), "f".repeat(64)); + assert.equal(normalizeEventId("F".repeat(64)), "f".repeat(64)); + assert.equal(normalizeEventId(null), null); + assert.equal(normalizeEventId(undefined), null); + assert.equal(normalizeEventId("g".repeat(64)), null); +}); + +test("a p-tag role is only what the sender actually marked", () => { + const ME = "a".repeat(64); + const OTHER = "b".repeat(64); + const e = (tags) => tags; + + // No marker at all — a sender that predates the markers. This must stay + // "unknown" and fall through to the parent lookup. Reading it as a mention + // would put every reply back through the mute it exists to respect. + assert.equal(pTagRoleFor(e([["p", ME]]), ME), "unknown"); + assert.equal(pTagRoleFor(e([["p", ME, "", "reply"]]), ME), "addressing"); + assert.equal(pTagRoleFor(e([["p", ME, "", "mention"]]), ME), "mention"); + assert.equal(pTagRoleFor(e([["p", OTHER, "", "mention"]]), ME), "none"); + assert.equal(pTagRoleFor(e([]), ME), "none"); + + // Mention wins: the sender emits only this marker when we are both the + // author being answered and someone typed in the body. + assert.equal( + pTagRoleFor( + e([ + ["p", ME, "", "reply"], + ["p", ME, "", "mention"], + ]), + ME, + ), + "mention", + ); + + // One bare tag among marked ones still means "ask the parent" for us. + assert.equal( + pTagRoleFor( + e([ + ["p", OTHER, "", "mention"], + ["p", ME], + ]), + ME, + ), + "unknown", + ); + + // Case is normalized on both sides. + assert.equal( + pTagRoleFor(e([["p", ME.toUpperCase(), "", "reply"]]), ME), + "addressing", + ); +}); + +const SELF = "a".repeat(64); +const COUNTERPART = "b".repeat(64); +const TYPED = "c".repeat(64); +const PARENT = "d".repeat(64); + +const pTags = (tags) => + tags.filter(([name, pubkey]) => name === "p" && pubkey !== SELF); + +test("a channel recipient is tagged bare, never as a mention", () => { + // A DM tags its other participants whether or not anyone typed their names. + // Claiming `mention` would let a DM thread reply pierce a mute and outrank a + // real `@you`; bare means "ask the parent", which is the honest answer and the + // one these tags already got before markers existed. + const tags = buildReplyTags( + "chan", + SELF, + "parent-id", + "parent-id", + [], + PARENT, + [COUNTERPART], + ); + assert.deepEqual(pTags(tags), [ + ["p", COUNTERPART], + ["p", PARENT, "", P_TAG_ADDRESSING_MARKER], + ]); + assert.equal(pTagRoleFor(tags, COUNTERPART), "unknown"); + assert.equal(pTagRoleFor(tags, PARENT), "addressing"); +}); + +test("a channel recipient who wrote the parent gets one addressing tag", () => { + // The ordinary DM reply: the counterpart is both the other participant and the + // author being answered. + const tags = buildReplyTags( + "chan", + SELF, + "parent-id", + "parent-id", + [], + COUNTERPART, + [COUNTERPART], + ); + assert.deepEqual(pTags(tags), [ + ["p", COUNTERPART, "", P_TAG_ADDRESSING_MARKER], + ]); +}); + +test("a channel recipient who was also typed is a mention", () => { + const tags = buildReplyTags( + "chan", + SELF, + "parent-id", + "parent-id", + [COUNTERPART], + null, + [COUNTERPART], + ); + assert.deepEqual(pTags(tags), [["p", COUNTERPART, "", P_TAG_MENTION_MARKER]]); + assert.equal(pTagRoleFor(tags, COUNTERPART), "mention"); +}); + +test("reply p-tags come out in the order the backend emits them", () => { + // The optimistic row must be tag-identical to the event the relay stores: + // typed mentions, then bare channel recipients, then the addressing tag. + const tags = buildReplyTags( + "chan", + SELF, + "parent-id", + "parent-id", + [TYPED], + PARENT, + [COUNTERPART], + ); + assert.deepEqual(pTags(tags), [ + ["p", TYPED, "", P_TAG_MENTION_MARKER], + ["p", COUNTERPART], + ["p", PARENT, "", P_TAG_ADDRESSING_MARKER], + ]); +}); diff --git a/desktop/src/features/messages/lib/threading.ts b/desktop/src/features/messages/lib/threading.ts index 95694f49897..76948b26d22 100644 --- a/desktop/src/features/messages/lib/threading.ts +++ b/desktop/src/features/messages/lib/threading.ts @@ -9,6 +9,96 @@ function getEventTags(tags: string[][]) { return tags.filter((tag) => tag[0] === "e" && typeof tag[1] === "string"); } +const EVENT_ID_HEX_RE = /^[0-9a-f]{64}$/; + +/** + * An `e` tag value if it can identify a relay event, else `null`. + * + * **Every caller that puts an event id into a REQ filter must go through this.** + * The relay *stores* whatever an `e` tag contains — its NIP-10 resolver ignores a + * malformed value instead of rejecting the event — so any community member can + * publish one. `nostr::Filter` parses `ids` into event ids and the relay answers + * a bare `NOTICE` when that fails: no `CLOSED`, no `EOSE`. The client ignores + * notices that are not `rate-limited:`, so the request hangs the full history + * timeout and then rejects. One such event in a channel is enough to make every + * parent lookup there time out for the rest of the session, and it does not clear + * on restart. + * + * `getThreadReference` deliberately does not apply this — a value that cannot + * identify an event is still a usable thread-grouping key. + */ +export function normalizeEventId(value: string | null | undefined) { + if (typeof value !== "string") { + return null; + } + const lower = value.toLowerCase(); + return EVENT_ID_HEX_RE.test(lower) ? lower : null; +} + +/** + * Marker on a `p` tag that names the author this reply answers. + * + * NIP-10 addressing and a typed `@mention` produce byte-identical `p` tags, so + * a receiver has to fetch the parent message and check who wrote it to tell them + * apart — a relay round trip to recover something the sender knew for free. + * These markers record it instead, in the fourth position, exactly as `e` tags + * already carry `root` and `reply`. + * + * Relay tag filters match only the tag's second element + * (`crates/buzz-core/src/filter.rs`), so a marker cannot affect `#p` delivery — + * an agent's `require_mention` subscription still receives the reply. + */ +export const P_TAG_ADDRESSING_MARKER = "reply"; + +/** + * Marker on a `p` tag that names someone the author typed as `@name`. + * + * Distinct from the `["mention", pk]` *reference* tag in + * `shared/lib/resolveMentionNames.ts`, which is a different tag kind meaning + * "render the chip but do not notify". This is a marker on a real `p` tag and + * does notify. + * + * Emitted alongside {@link P_TAG_ADDRESSING_MARKER} rather than instead of it. + * Marking only the addressing tag would leave the one case inference cannot + * reach still unreachable: when the recipient is both the parent's author and + * typed in the body, a single tag can only be marked or unmarked. With both + * markers the sender simply emits this one, and mention wins. + */ +export const P_TAG_MENTION_MARKER = "mention"; + +export type PTagRole = "addressing" | "mention" | "unknown" | "none"; + +/** + * What the `p` tags naming `pubkey` say this event is to them. + * + * `unknown` is load-bearing and must never be collapsed into either answer. A + * sender that predates these markers emits a bare `p` tag for both roles, so + * absence of a marker means "ask the parent", not "this is a mention". Only a + * marker that is actually present is authoritative. + */ +export function pTagRoleFor(tags: string[][], pubkey: string): PTagRole { + const target = pubkey.toLowerCase(); + let sawAddressing = false; + let sawBare = false; + for (const tag of tags) { + if (tag[0] !== "p" || tag[1]?.toLowerCase() !== target) { + continue; + } + // Mention wins outright: it is the only marker a sender emits when the + // recipient is both the parent's author and typed in the body. + if (tag[3] === P_TAG_MENTION_MARKER) { + return "mention"; + } + if (tag[3] === P_TAG_ADDRESSING_MARKER) { + sawAddressing = true; + } else { + sawBare = true; + } + } + if (sawBare) return "unknown"; + return sawAddressing ? "addressing" : "none"; +} + export function getChannelIdFromTags(tags: string[][]) { return tags.find((tag) => tag[0] === "h")?.[1] ?? null; } @@ -43,11 +133,19 @@ export function getThreadReference(tags: string[][]): ThreadReference { }; } - const parentId = replyTag[1] ?? null; + // Lowercased, not validated. Case has to be normalized here because these ids + // are compared against `event.id`, which is always lowercase, and a mismatch + // reads as "parent absent" — which relabels a reply a real mention. + // + // Validation deliberately does NOT happen here: this is the general + // thread-grouping primitive, and a value that cannot identify a relay event is + // still a usable grouping key. Callers that put the id into a REQ filter must + // run it through `normalizeEventId` themselves. + const parentId = replyTag[1]?.toLowerCase() ?? null; return { parentId, - rootId: rootTag?.[1] ?? parentId, + rootId: rootTag?.[1]?.toLowerCase() ?? parentId, }; } @@ -104,17 +202,48 @@ export function buildReplyTags( parentEventId: string, rootEventId: string, mentionPubkeys: string[] = [], + parentAuthorPubkey?: string | null, + addressedPubkeys: string[] = [], ) { const tags: string[][] = [ ["p", authorPubkey], ["h", channelId], ]; + const parentAuthor = parentAuthorPubkey?.trim().toLowerCase() ?? ""; + const mentions = normalizeMentionPubkeys(mentionPubkeys, authorPubkey); + const addressing = + parentAuthor && + parentAuthor !== authorPubkey.toLowerCase() && + !mentions.includes(parentAuthor) + ? parentAuthor + : ""; + // Add p-tags for mentioned users so mention-filtered subscriptions // (e.g. ACP agent harness) receive the reply event. // Best-effort normalization — relay performs authoritative validation. - for (const pubkey of normalizeMentionPubkeys(mentionPubkeys, authorPubkey)) { - tags.push(["p", pubkey]); + // + // Marked so the recipient does not have to fetch the parent to learn which + // role each tag plays. A pubkey that is both typed and the parent's author + // gets the mention marker only, and is not repeated as an addressing tag. + // Order matches the backend builder so this optimistic copy is tag-identical + // to the event the relay stores. + for (const pubkey of mentions) { + tags.push(["p", pubkey, "", P_TAG_MENTION_MARKER]); + } + // Recipients the channel addresses rather than the message — DM participants. + // Left bare: neither marker is true of them, and under the one-way read a bare + // tag means "ask the parent", which is the answer they had before markers. + for (const pubkey of normalizeMentionPubkeys( + addressedPubkeys, + authorPubkey, + )) { + if (!mentions.includes(pubkey) && pubkey !== addressing) { + tags.push(["p", pubkey]); + } + } + if (addressing) { + tags.push(["p", addressing, "", P_TAG_ADDRESSING_MARKER]); } if (parentEventId === rootEventId) { @@ -160,3 +289,48 @@ export function resolveReplyRootId( const thread = getThreadReference(parent.tags); return thread.rootId ?? parent.id; } + +/** + * Hard cap the relay applies to `p` tags on one event. + * + * Mirrors `MENTION_CAP` in `crates/buzz-sdk/src/mentions.rs`. + */ +export const MENTION_TAG_CAP = 50; + +/** + * Typed-mention `p` tags for a reply, with room reserved for the addressing tag. + * + * The addressing tag itself is **not** in this list. The backend appends it, + * marked, from the parent event `resolve_thread_ref` already fetches — which is + * both unambiguous on the wire and more reliable than any caller here, since + * every caller sourced the parent author from a cache that misses for channels + * the user has not opened. A miss used to mean the reply shipped with no + * addressing tag at all, and an agent's `require_mention` subscription never + * received it. + * + * What survives is the reservation. The builder rejects the *whole* event past + * the cap rather than trimming, so a reply composed with 50 mentions plus the + * addressing tag would simply fail to send. Dropping a body mention costs one + * notification; dropping the addressing tag costs the agent the reply. + */ +export function replyRecipientPubkeys({ + currentPubkey, + mentionPubkeys, + parentAuthorPubkey, +}: { + currentPubkey: string; + mentionPubkeys: readonly string[]; + parentAuthorPubkey: string | null | undefined; +}): string[] { + const parent = parentAuthorPubkey?.trim().toLowerCase() ?? ""; + const normalized = normalizeMentionPubkeys( + [...mentionPubkeys], + currentPubkey, + ); + const reservesAddressingSlot = + parent.length > 0 && + parent !== currentPubkey.toLowerCase() && + !normalized.includes(parent); + const cap = reservesAddressingSlot ? MENTION_TAG_CAP - 1 : MENTION_TAG_CAP; + return normalized.length <= cap ? normalized : normalized.slice(0, cap); +} diff --git a/desktop/src/features/notifications/hooks.test.mjs b/desktop/src/features/notifications/hooks.test.mjs index 26fc4fedb5c..4a6158852ce 100644 --- a/desktop/src/features/notifications/hooks.test.mjs +++ b/desktop/src/features/notifications/hooks.test.mjs @@ -88,6 +88,30 @@ test("home badge subtotal excludes channel-counted high-priority items", () => { ), false, ); + // Production shape: the backend never populates `channel_type` on a feed item, + // so asserting only the hand-supplied "dm" above left this guard dead at + // runtime and double-counted every DM thread reply on the dock badge. The + // channel list is the authoritative source. + assert.equal( + shouldCountTowardHomeBadgeSubtotal( + { channelId: "dm-channel", channelType: undefined, tags: ROOT_TAGS }, + highPriorityChannelIds, + false, + new Set(["dm-channel"]), + ), + false, + ); + // Without the channel list it cannot tell, and a non-DM thread reply must + // still count. + assert.equal( + shouldCountTowardHomeBadgeSubtotal( + { channelId: "stream-channel", channelType: undefined, tags: ROOT_TAGS }, + new Set(["stream-channel"]), + false, + new Set(["dm-channel"]), + ), + true, + ); }); test("home badge subtotal still counts non-DM thread-only rows", () => { diff --git a/desktop/src/features/notifications/hooks.ts b/desktop/src/features/notifications/hooks.ts index 1a2cb4a9a53..30375536e8b 100644 --- a/desktop/src/features/notifications/hooks.ts +++ b/desktop/src/features/notifications/hooks.ts @@ -28,6 +28,7 @@ import { import { buildHomeBadgeFeedItems, isHomeBadgeFeedItemUnread, + isMutedOutOfBadgeCount, shouldCountTowardHomeBadgeSubtotal, } from "./lib/homeBadge"; @@ -428,9 +429,29 @@ export function useHomeFeedNotificationState( const [seenFeedIds, setSeenFeedIds] = React.useState(() => readStoredSeenFeedIds(normalizedPubkey), ); + // The backend leaves `channel_type` unset on feed items, so the DM carve-out in + // `buildHomeBadgeFeedItems` has to come from the channel list. + // + // Keyed on a joined string rather than on `channels`: the parameter defaults to + // a fresh `[]` and callers pass a refetched array, so depending on the array + // itself would rebuild this Set every render and invalidate `currentFeedItems` + // with it. + const dmChannelIdKey = channels + .filter((channel) => channel.channelType === "dm") + .map((channel) => channel.id) + .join(","); + const dmChannelIds = React.useMemo( + () => new Set(dmChannelIdKey.length === 0 ? [] : dmChannelIdKey.split(",")), + [dmChannelIdKey], + ); const currentFeedItems = React.useMemo(() => { - return buildHomeBadgeFeedItems(feed, extraInboxItems, localUnreadFeedIds); - }, [extraInboxItems, feed, localUnreadFeedIds]); + return buildHomeBadgeFeedItems( + feed, + extraInboxItems, + localUnreadFeedIds, + dmChannelIds, + ); + }, [dmChannelIds, extraInboxItems, feed, localUnreadFeedIds]); const currentFeedIds = React.useMemo( () => currentFeedItems.map((item) => item.id), [currentFeedItems], @@ -476,11 +497,7 @@ export function useHomeFeedNotificationState( if (isHomeActive && !isLocallyUnread) { continue; } - if ( - item.channelId && - mutedChannelIds?.has(item.channelId) && - item.category !== "mention" - ) { + if (isMutedOutOfBadgeCount(item, mutedChannelIds)) { continue; } const isUnread = isHomeBadgeFeedItemUnread(item, { @@ -497,6 +514,7 @@ export function useHomeFeedNotificationState( item, highPriorityChannelIds, isLocallyUnread, + dmChannelIds, ) ) { excludingHighPriority++; diff --git a/desktop/src/features/notifications/lib/feed.test.mjs b/desktop/src/features/notifications/lib/feed.test.mjs index d4903241d4c..344e1451987 100644 --- a/desktop/src/features/notifications/lib/feed.test.mjs +++ b/desktop/src/features/notifications/lib/feed.test.mjs @@ -120,3 +120,55 @@ test("resolves and excludes a DM whose feed item has a name but no type", () => assert.equal(items.length, 0); }); + +test("excludes a reply that only reaches the mention feed via its addressing p-tag", () => { + // The live thread-reply path owns those. Notifying here too would toast + // and sound twice for one event. + const replyItem = feedItem({ + category: "mention", + channelId: "general", + channelName: "general", + channelType: "stream", + replyToSelf: true, + }); + const items = eligibleFeedNotificationItems( + feedResponse([replyItem]), + allSlots, + [{ id: "general", name: "general", channelType: "stream" }], + ); + + assert.equal(items.length, 0); +}); + +test("keeps a real mention that also happens to be a reply", () => { + const mentionInReply = feedItem({ + category: "mention", + channelId: "general", + channelName: "general", + channelType: "stream", + replyToSelf: false, + }); + const items = eligibleFeedNotificationItems( + feedResponse([mentionInReply]), + allSlots, + [{ id: "general", name: "general", channelType: "stream" }], + ); + + assert.equal(items.length, 1); +}); + +test("keeps a mention with no replyToSelf field (older backend payloads)", () => { + const legacyItem = feedItem({ + category: "mention", + channelId: "general", + channelName: "general", + channelType: "stream", + }); + const items = eligibleFeedNotificationItems( + feedResponse([legacyItem]), + allSlots, + [{ id: "general", name: "general", channelType: "stream" }], + ); + + assert.equal(items.length, 1); +}); diff --git a/desktop/src/features/notifications/lib/feed.ts b/desktop/src/features/notifications/lib/feed.ts index 4c87cb99d4f..8b97fbebb75 100644 --- a/desktop/src/features/notifications/lib/feed.ts +++ b/desktop/src/features/notifications/lib/feed.ts @@ -71,7 +71,11 @@ export function eligibleFeedNotificationItems( items.push( ...feed.feed.mentions .map((item) => enrichFeedItemChannel(item, channels)) - .filter((item) => item.channelType !== "dm"), + .filter((item) => item.channelType !== "dm") + // A reply reaches the mention feed only because it p-tags the author + // it answers. The live thread-reply path owns those, so notifying + // here too would toast and sound twice for one event. + .filter((item) => item.replyToSelf !== true), ); } diff --git a/desktop/src/features/notifications/lib/homeBadge.test.mjs b/desktop/src/features/notifications/lib/homeBadge.test.mjs new file mode 100644 index 00000000000..0323e8b7c37 --- /dev/null +++ b/desktop/src/features/notifications/lib/homeBadge.test.mjs @@ -0,0 +1,173 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + buildHomeBadgeFeedItems, + isMutedOutOfBadgeCount, +} from "./homeBadge.ts"; + +const ROOT = "a".repeat(64); +const PARENT = "b".repeat(64); + +function feedWith(mentions) { + return { + feed: { + mentions, + needsAction: [], + activity: [], + agentActivity: [], + }, + }; +} + +function replyItem(id, overrides = {}) { + return { + id, + channelId: "chan-1", + channelType: "channel", + category: "mention", + createdAt: 100, + tags: [ + ["e", ROOT, "", "root"], + ["e", PARENT, "", "reply"], + ], + ...overrides, + }; +} + +test("a reply to your own message is kept out of the badge count", () => { + // It reaches the mention feed only via its NIP-10 addressing `p` tag, which is + // byte-identical to a typed @mention. The toast path already skips it, and + // this count has no thread-mute input — so counting it would both disagree + // with what the user was shown and let a muted thread drive the dock badge. + const items = buildHomeBadgeFeedItems( + feedWith([replyItem("reply-1", { replyToSelf: true })]), + [], + new Set(), + ); + + assert.deepEqual(items, []); +}); + +test("a typed mention that is also a reply still counts", () => { + // Mentions are meant to pierce mutes; the backend marks these replyToSelf:false. + const items = buildHomeBadgeFeedItems( + feedWith([replyItem("mention-1", { replyToSelf: false })]), + [], + new Set(), + ); + + assert.deepEqual( + items.map((item) => item.id), + ["mention-1"], + ); +}); + +test("an explicit mark-as-unread outranks the reply exclusion", () => { + // The Inbox still renders these rows, so dropping one the user deliberately + // marked unread left the row's dot showing while the numeral stayed at 0. + const items = buildHomeBadgeFeedItems( + feedWith([replyItem("reply-1", { replyToSelf: true })]), + [], + new Set(["reply-1"]), + ); + + assert.deepEqual( + items.map((item) => item.id), + ["reply-1"], + ); +}); + +test("a top-level mention with no reply tags counts even if replyToSelf is set", () => { + // replyToSelf only means "addressing tag" on something that is actually a + // reply. Without the thread tags there is no addressing tag to discount. + const items = buildHomeBadgeFeedItems( + feedWith([ + { + id: "top-1", + channelId: "chan-1", + channelType: "channel", + category: "mention", + createdAt: 100, + tags: [["p", "c".repeat(64)]], + replyToSelf: true, + }, + ]), + [], + new Set(), + ); + + assert.deepEqual( + items.map((item) => item.id), + ["top-1"], + ); +}); + +test("a mark-as-unread reply survives BOTH badge gates in a muted channel", () => { + // The regression this guards: the two gates cancelled each other. The build + // step admitted the item, then the counting loop's mute clause re-tested + // `replyToSelf` and dropped it again — so the numeral stayed at zero while the + // Inbox row showed its dot. Testing `buildHomeBadgeFeedItems` alone missed it, + // which is why both halves are asserted together here. + const muted = new Set(["chan-1"]); + const item = replyItem("reply-1", { replyToSelf: true }); + + const items = buildHomeBadgeFeedItems( + feedWith([item]), + [], + new Set(["reply-1"]), + ); + assert.deepEqual( + items.map((entry) => entry.id), + ["reply-1"], + ); + assert.equal(isMutedOutOfBadgeCount(items[0], muted), false); +}); + +test("a muted channel's non-mention activity stays out of the count", () => { + assert.equal( + isMutedOutOfBadgeCount( + { category: "activity", channelId: "chan-1" }, + new Set(["chan-1"]), + ), + true, + ); +}); + +test("a real mention pierces a channel mute", () => { + assert.equal( + isMutedOutOfBadgeCount( + { category: "mention", channelId: "chan-1" }, + new Set(["chan-1"]), + ), + false, + ); +}); + +test("an answer inside a DM still counts toward the numeral", () => { + // Every DM message p-tags both participants, so `replyToSelf` carries no + // information there — the addressing tag *is* the addressing. Without the + // carve-out, Alice's first DM counted but her reply to my answer did not. + const items = buildHomeBadgeFeedItems( + feedWith([replyItem("dm-reply", { replyToSelf: true })]), + [], + new Set(), + new Set(["chan-1"]), + ); + + assert.deepEqual( + items.map((item) => item.id), + ["dm-reply"], + ); +}); + +test("an answer in a non-DM channel is still excluded", () => { + const items = buildHomeBadgeFeedItems( + feedWith([replyItem("chan-reply", { replyToSelf: true })]), + [], + new Set(), + new Set(["some-other-dm"]), + ); + + assert.deepEqual(items, []); +}); diff --git a/desktop/src/features/notifications/lib/homeBadge.ts b/desktop/src/features/notifications/lib/homeBadge.ts index b98db88e0bb..85ce2dd254a 100644 --- a/desktop/src/features/notifications/lib/homeBadge.ts +++ b/desktop/src/features/notifications/lib/homeBadge.ts @@ -6,6 +6,8 @@ import { isThreadReply, } from "@/features/messages/lib/threading"; +const EMPTY_DM_CHANNEL_IDS: ReadonlySet = new Set(); + function dedupeFeedItemsById(items: readonly FeedItem[]): FeedItem[] { const seen = new Set(); const result: FeedItem[] = []; @@ -19,10 +21,55 @@ function dedupeFeedItemsById(items: readonly FeedItem[]): FeedItem[] { return result; } +/** + * Whether a mention-feed item belongs in the Home/dock badge count. + * + * A reply reaches the mention feed only because of its NIP-10 addressing `p` + * tag, which is byte-identical to a typed `@mention`. The backend marks those + * with `replyToSelf`, and the toast path already skips them — so counting them + * here would make the numeral disagree with what the user was actually shown. + * + * It also closes a mute bypass: this count is not thread-mute aware (it has no + * `mutedRootIds` input, and the only mute check downstream is per-channel), so + * a reply to your own message in a muted thread inside an *unmuted* channel + * would increment the Inbox numeral and the macOS dock badge while the sidebar + * and toasts correctly stayed silent. Dropping `replyToSelf` items removes that + * whole class, because a reply that is muted-but-still-p-tags-you is exactly a + * reply to your own message. + * + * A typed `@mention` inside a muted thread is *not* dropped — mentions are meant + * to pierce mutes, and those carry `replyToSelf: false`. + */ +function isBadgeCountableMention( + item: FeedItem, + localUnreadFeedIds: ReadonlySet, + dmChannelIds: ReadonlySet, +): boolean { + // An explicit "Mark as unread" outranks the rule. The Inbox still renders + // these rows, so without this the row shows its unread dot while the numeral + // and the dock badge stay at zero for as long as the user leaves it marked. + if (localUnreadFeedIds.has(item.id)) { + return true; + } + // DMs are exempt, matching every other path in this feature + // (`shouldNotifyForEvent`, `communityUnreadObserver`, `catchUpParentAuthors`): + // every DM message p-tags both participants, so there the addressing tag *is* + // the addressing and `replyToSelf` carries no information. The backend cannot + // make this call for us — `feed_item_from_event` never populates + // `channel_type` — so without the channel list an answer inside a DM would + // stop counting toward the Home numeral while the first message in the same + // conversation still counted. + if (item.channelId !== null && dmChannelIds.has(item.channelId)) { + return true; + } + return !(item.replyToSelf === true && isThreadReply(item.tags)); +} + export function buildHomeBadgeFeedItems( feed: HomeFeedResponse | undefined, extraInboxItems: readonly FeedItem[], localUnreadFeedIds: ReadonlySet, + dmChannelIds: ReadonlySet = EMPTY_DM_CHANNEL_IDS, ): FeedItem[] { // Thread activity is surfaced directly on its channel's hover preview. It // should not also inflate the Inbox numeral, which is reserved for the @@ -32,7 +79,9 @@ export function buildHomeBadgeFeedItems( ); const items = feed ? [ - ...feed.feed.mentions, + ...feed.feed.mentions.filter((item) => + isBadgeCountableMention(item, localUnreadFeedIds, dmChannelIds), + ), ...feed.feed.needsAction, ...nonThreadExtraInboxItems, ] @@ -50,10 +99,35 @@ export function buildHomeBadgeFeedItems( return dedupeFeedItemsById(items); } +/** + * Whether a channel mute keeps this item out of the badge count. + * + * The other half of {@link isBadgeCountableMention}'s decision, kept in the same + * file because the two can cancel each other. Re-testing `replyToSelf` here as + * well used to undo the mark-as-unread override for every muted channel: the + * only replies that survive `isBadgeCountableMention` are ones the user + * explicitly marked unread, and dropping those again left the Inbox row dotted + * while the numeral read zero. + * + * A real `@mention` deliberately survives a channel mute — mentions are meant to + * pierce mutes. + */ +export function isMutedOutOfBadgeCount( + item: Pick, + mutedChannelIds: ReadonlySet | undefined, +): boolean { + return Boolean( + item.channelId && + mutedChannelIds?.has(item.channelId) && + item.category !== "mention", + ); +} + export function shouldCountTowardHomeBadgeSubtotal( item: Pick, highPriorityChannelIds: ReadonlySet, forceHomeCount = false, + dmChannelIds: ReadonlySet = EMPTY_DM_CHANNEL_IDS, ): boolean { if (forceHomeCount) { return true; @@ -66,7 +140,17 @@ export function shouldCountTowardHomeBadgeSubtotal( const threadRef = getThreadReference(item.tags); const isThreadedReply = threadRef.parentId !== null && !isBroadcastReply(item.tags); - return isThreadedReply && item.channelType !== "dm"; + // `channelType` alone is not enough: the backend never populates it on a feed + // item (`feed_item_from_event` emits `channel_type: None`) and, unlike the toast + // path, nothing enriches it from the channel list here. Testing only that field + // made this a dead guard in production — a DM thread reply counted in this + // subtotal *and* in the channel-side count, so the dock badge read 2 for one + // message. `dmChannelIds` is the authoritative source, the same one + // `isBadgeCountableMention` uses. + const isDm = + item.channelType === "dm" || + (item.channelId !== null && dmChannelIds.has(item.channelId)); + return isThreadedReply && !isDm; } type FeedItemReadState = Pick< diff --git a/desktop/src/features/notifications/lib/replyParentAuthors.test.mjs b/desktop/src/features/notifications/lib/replyParentAuthors.test.mjs new file mode 100644 index 00000000000..f05a3a59937 --- /dev/null +++ b/desktop/src/features/notifications/lib/replyParentAuthors.test.mjs @@ -0,0 +1,115 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { collectReplyParentAuthors } from "./replyParentAuthors.ts"; + +const ME = "a".repeat(64); +const OTHER = "b".repeat(64); + +// Real event ids: an `e` value that cannot identify a relay event is skipped +// before it reaches an `ids` filter, so short labels would no longer be fetched. +const M1 = "1".repeat(64); +const M2 = "2".repeat(64); +const M3 = "3".repeat(64); +const OLDER_PARENT = "e".repeat(64); + +function makeEvent(id, pubkey, tags = []) { + return { + id, + pubkey, + created_at: 1700000000, + kind: 9, + tags, + content: "hi", + sig: "s".repeat(128), + }; +} + +const replyTo = (parentId, rootId = parentId) => + rootId === parentId + ? [["e", parentId, "", "reply"]] + : [ + ["e", rootId, "", "root"], + ["e", parentId, "", "reply"], + ]; + +const neverFetch = () => { + throw new Error("should not fetch"); +}; + +test("maps every event in the batch by id without fetching", async () => { + const events = [makeEvent(M1, ME), makeEvent(M2, OTHER, replyTo(M1))]; + const authors = await collectReplyParentAuthors({ + events, + fetchEvents: neverFetch, + kinds: [9], + shouldResolveParent: () => true, + }); + assert.equal(authors.get(M1), ME); + assert.equal(authors.get(M2), OTHER); +}); + +test("skips the fetch when shouldResolveParent returns false", async () => { + // A real parent id, absent from the batch: with the gate open this is exactly + // the case that fetches. `neverFetch` is the assertion — a label that + // `normalizeEventId` rejects would skip the fetch on its own and prove + // nothing about the gate. + const events = [makeEvent(M2, OTHER, replyTo(OLDER_PARENT))]; + const authors = await collectReplyParentAuthors({ + events, + fetchEvents: neverFetch, + kinds: [9], + shouldResolveParent: () => false, + }); + assert.equal(authors.has(OLDER_PARENT), false); +}); + +test("fetches parents missing from the batch when the gate is open", async () => { + const events = [ + makeEvent(M2, OTHER, replyTo(OLDER_PARENT)), + makeEvent(M3, OTHER, replyTo(OLDER_PARENT)), + ]; + const requested = []; + const authors = await collectReplyParentAuthors({ + events, + fetchEvents: async (filter) => { + requested.push(filter); + return [makeEvent(OLDER_PARENT, ME)]; + }, + kinds: [9, 40002], + shouldResolveParent: () => true, + }); + // One request, deduped, with the kinds the p-gate requires. + assert.equal(requested.length, 1); + assert.deepEqual(requested[0].ids, [OLDER_PARENT]); + assert.deepEqual(requested[0].kinds, [9, 40002]); + assert.equal(authors.get(OLDER_PARENT), ME); +}); + +test("a failed parent fetch propagates so the caller can retry", async () => { + // Degrading to a batch-only map looks graceful but is not: the unresolved + // parent is guessed at and the guess is persisted per event id, never + // recomputed. Both callers treat a throw as "retry this channel". + const events = [makeEvent(M2, OTHER, replyTo(OLDER_PARENT))]; + await assert.rejects( + collectReplyParentAuthors({ + events, + fetchEvents: async () => { + throw new Error("relay down"); + }, + kinds: [9], + shouldResolveParent: () => true, + }), + /relay down/, + ); +}); + +test("top-level events never trigger a fetch", async () => { + const authors = await collectReplyParentAuthors({ + events: [makeEvent(M1, OTHER)], + fetchEvents: neverFetch, + kinds: [9], + shouldResolveParent: () => true, + }); + assert.equal(authors.size, 1); +}); diff --git a/desktop/src/features/notifications/lib/replyParentAuthors.ts b/desktop/src/features/notifications/lib/replyParentAuthors.ts new file mode 100644 index 00000000000..7b4cfc193dd --- /dev/null +++ b/desktop/src/features/notifications/lib/replyParentAuthors.ts @@ -0,0 +1,105 @@ +import { + getThreadReference, + normalizeEventId, +} from "@/features/messages/lib/threading"; +import type { RelayEvent } from "@/shared/api/types"; + +/** + * Ids per parent-lookup REQ. Mirrors `AUX_BACKFILL_CHUNK_SIZE` — the same relay + * filter limit applies, and this query has the same shape (many ids, one kind + * set). + */ +const PARENT_LOOKUP_CHUNK_SIZE = 100; + +/** + * Authors of the events a batch of replies answers, keyed by event id. + * + * Catch-up windows start strictly after the read marker, so the parent of a + * reply — typically our own older message — is never in the batch that + * contains the reply. `shouldNotifyForEvent` needs that author to tell a real + * mention from a reply's addressing `p` tag. + * + * `shouldResolveParent` decides which replies are worth the extra relay round + * trip. The useful rule is "does this event `p`-tag the current user?" — that + * is the only case where the parent's author changes any answer, and it is + * needed whether or not the channel is muted, because high-priority marking + * depends on it too. A caller that cannot cheaply tell may resolve everything. + */ +export async function collectReplyParentAuthors({ + events, + fetchEvents, + kinds, + shouldResolveParent, +}: { + events: readonly RelayEvent[]; + fetchEvents: (filter: { + kinds: number[]; + ids: string[]; + limit: number; + }) => Promise; + kinds: readonly number[]; + shouldResolveParent: ( + ref: { parentId: string | null; rootId: string | null }, + event: RelayEvent, + ) => boolean; +}): Promise> { + const authorByEventId = new Map( + events.map((event) => [event.id, event.pubkey]), + ); + const missing = [ + ...new Set( + events + .map((event) => ({ event, ref: getThreadReference(event.tags) })) + .filter( + ({ event, ref }) => + ref.parentId !== null && + !authorByEventId.has(ref.parentId) && + shouldResolveParent(ref, event), + ) + // A tag value the relay stored but cannot parse as an event id would make + // it answer a bare NOTICE, which this client never resolves — the request + // hangs the history timeout, then rejects, and the caller retries the whole + // channel forever. Skip it: an unlookupable parent is simply unresolved. + .map(({ ref }) => normalizeEventId(ref.parentId)) + .filter((id): id is string => id !== null), + ), + ]; + if (missing.length === 0) { + return authorByEventId; + } + + // Deliberately not caught. Swallowing the failure and returning a + // batch-only map looks like a graceful degrade but is not one: an + // unresolved parent is guessed at, and both callers persist or cache that + // guess. Both of them already treat a throw as "retry this channel", which + // is the only outcome that self-corrects. + // + // Chunked for the same reason as `AUX_BACKFILL_CHUNK_SIZE`: an `ids` filter + // this wide exceeds the relay's filter limits. `useUnreadChannels` can reach + // `CATCH_UP_LIMIT` (1000) ids and the community observer 150 per channel, and + // a truncated or rejected REQ is worse here than a slow one — an unreturned + // parent reads as a mention, which inflates the mention count in one caller + // and silently clears `highPriority` in the other, where it is persisted. + // + // Sequential, not `Promise.all`: these run behind the same rate-limit gate as + // foreground channel history, and a 10-chunk burst per channel would starve + // the UI. A throw propagates and the caller retries the whole channel. + // + // `kinds` is required — an open-ended filter hits the relay p-gate (403). + for ( + let index = 0; + index < missing.length; + index += PARENT_LOOKUP_CHUNK_SIZE + ) { + const ids = missing.slice(index, index + PARENT_LOOKUP_CHUNK_SIZE); + const parents = await fetchEvents({ + kinds: [...kinds], + ids, + limit: ids.length, + }); + for (const parent of parents) { + authorByEventId.set(parent.id, parent.pubkey); + } + } + return authorByEventId; +} diff --git a/desktop/src/features/notifications/lib/shouldNotify.test.mjs b/desktop/src/features/notifications/lib/shouldNotify.test.mjs index 2642b9b204f..0e8bdc0b595 100644 --- a/desktop/src/features/notifications/lib/shouldNotify.test.mjs +++ b/desktop/src/features/notifications/lib/shouldNotify.test.mjs @@ -2,7 +2,9 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + hasAuthoredMentionForEvent, isHighPriorityEventForUser, + needsResolvedParentAuthor, shouldNotifyForEvent, } from "./shouldNotify.ts"; @@ -201,6 +203,146 @@ test("muted thread reply still notifies when currentPubkey is mentioned via p-ta ); }); +test("muted thread reply answering us does not pierce the mute via its p-tag", () => { + // Replies p-tag the author they answer so agent `require_mention` + // subscriptions receive them. That tag must not read as a mention. + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ mutedRootIds: new Set([ROOT_ID]), parentAuthorPubkey: PUBKEY }), + ), + false, + ); +}); + +test("unmuted thread reply answering us still notifies via authoredRootIds", () => { + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + authoredRootIds: new Set([ROOT_ID]), + parentAuthorPubkey: PUBKEY, + }), + ), + true, + ); +}); + +test("muted thread reply answering someone else still notifies us on mention", () => { + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + mutedRootIds: new Set([ROOT_ID]), + parentAuthorPubkey: OTHER_PUBKEY, + }), + ), + true, + ); +}); + +test("muted channel reply answering us is suppressed", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + mutedChannelIds: new Set(["channel-1"]), + channelId: "channel-1", + parentAuthorPubkey: PUBKEY.toUpperCase(), + }), + ), + false, + ); +}); + +test("parentAuthorPubkey is ignored on a top-level message", () => { + const event = makeEvent([pTag(PUBKEY)]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + mutedChannelIds: new Set(["channel-1"]), + channelId: "channel-1", + parentAuthorPubkey: PUBKEY, + }), + ), + true, + ); +}); + +test("unmuted reply answering us notifies with no local thread state at all", () => { + // The participated/authored sets are local and rebuilt from the unread + // window, so a fresh install must not lose "someone replied to you". + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal( + shouldNotifyForEvent(event, PUBKEY, opts({ parentAuthorPubkey: PUBKEY })), + true, + ); +}); + +test("hasAuthoredMentionForEvent: a reply answering us is not an authored mention", () => { + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, PUBKEY), false); +}); + +test("hasAuthoredMentionForEvent: a reply answering someone else is a mention", () => { + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, OTHER_PUBKEY), true); +}); + +test("hasAuthoredMentionForEvent: falls back to the raw p-tag without a parent author", () => { + const event = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, null), true); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY), true); +}); + +test("hasAuthoredMentionForEvent: a top-level message we authored the parent of is still a mention", () => { + const event = makeEvent([pTag(PUBKEY)]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, PUBKEY), true); +}); + +test("hasAuthoredMentionForEvent: no p-tag is never a mention", () => { + const event = makeEvent([replyTag(PARENT_ID)]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, OTHER_PUBKEY), false); +}); + test("muted rootId does not suppress a top-level (non-reply) message", () => { const event = makeEvent([]); assert.equal( @@ -297,8 +439,10 @@ test("empty currentPubkey with participated thread still notifies (no mute)", () }); test("isHighPriorityEventForUser returns true when p-tag matches currentPubkey", () => { + // A reply needs its parent resolved to prove the tag is a real mention; + // pass an author other than the user so it is one. const event = makeEvent([replyTag(ROOT_ID), pTag(PUBKEY)]); - assert.equal(isHighPriorityEventForUser(event, PUBKEY), true); + assert.equal(isHighPriorityEventForUser(event, PUBKEY, OTHER_PUBKEY), true); }); test("isHighPriorityEventForUser returns true for broadcast reply", () => { @@ -313,7 +457,7 @@ test("isHighPriorityEventForUser returns false when no matching p-tag and no bro test("isHighPriorityEventForUser p-tag matching is case-insensitive", () => { const event = makeEvent([replyTag(ROOT_ID), pTag(PUBKEY.toUpperCase())]); - assert.equal(isHighPriorityEventForUser(event, PUBKEY), true); + assert.equal(isHighPriorityEventForUser(event, PUBKEY, OTHER_PUBKEY), true); }); test("isHighPriorityEventForUser returns false when currentPubkey is empty", () => { @@ -326,3 +470,247 @@ test("isHighPriorityEventForUser returns false for event with no tags at all", ( const event = makeEvent([]); assert.equal(isHighPriorityEventForUser(event, PUBKEY), false); }); + +const CHANNEL_ID = "11111111-1111-4111-8111-111111111111"; +const parentOpts = (overrides = {}) => ({ + cachedParentAuthor: null, + ...overrides, +}); + +test("an uncached reply that p-tags the user escalates to a parent lookup", () => { + // The regression this guards: the cache only holds the viewed channel, so a + // channel the user never opened always misses, and the addressing p-tag then + // reads as a mention. Asserted for both reply shapes — with and without a + // root tag — because the escalation is decided from the reply marker. + const bare = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal(needsResolvedParentAuthor(bare, PUBKEY, parentOpts()), true); + + const rooted = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(PUBKEY), + ]); + assert.equal(needsResolvedParentAuthor(rooted, PUBKEY, parentOpts()), true); +}); + +test("a broadcast reply never escalates to a parent lookup", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY), broadcastTag()]); + assert.equal(needsResolvedParentAuthor(event, PUBKEY, parentOpts()), false); +}); + +test("no escalation in a DM, where no consumer reads the parent author", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + needsResolvedParentAuthor(event, PUBKEY, parentOpts({ isDmChannel: true })), + false, + ); +}); + +test("no escalation once the cache already answered", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + needsResolvedParentAuthor( + event, + PUBKEY, + parentOpts({ cachedParentAuthor: OTHER_PUBKEY }), + ), + false, + ); +}); + +test("no escalation for a reply that does not p-tag the user", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(OTHER_PUBKEY)]); + assert.equal(needsResolvedParentAuthor(event, PUBKEY, parentOpts()), false); +}); + +test("no escalation for a top-level message", () => { + const event = makeEvent([pTag(PUBKEY)]); + assert.equal(needsResolvedParentAuthor(event, PUBKEY, parentOpts()), false); +}); + +test("a broadcast reply answering us is still an authored mention", () => { + // shouldNotifyForEvent admits broadcast replies before it ever reads the + // parent, and the live thread-reply path never sees them. Demoting one here + // left the community badge and the Home feed disagreeing about the event. + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY), broadcastTag()]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, PUBKEY), true); +}); + +test("a non-broadcast reply answering us is still not an authored mention", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal(hasAuthoredMentionForEvent(event, PUBKEY, PUBKEY), false); +}); + +test("a muted DM notifies for a reply answering us, like any other DM message", () => { + // Every DM message p-tags both participants. Demoting the addressing tag + // there silenced answers to you while new messages still came through. + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + mutedChannelIds: new Set([CHANNEL_ID]), + channelId: CHANNEL_ID, + parentAuthorPubkey: PUBKEY, + isDmChannel: true, + }), + ), + true, + ); +}); + +test("outside a DM the same reply is still suppressed by the channel mute", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + shouldNotifyForEvent( + event, + PUBKEY, + opts({ + mutedChannelIds: new Set([CHANNEL_ID]), + channelId: CHANNEL_ID, + parentAuthorPubkey: PUBKEY, + isDmChannel: false, + }), + ), + false, + ); +}); + +test("a reply answering us is not high priority", () => { + // High priority marks the whole channel, and the home badge then drops + // top-level items there — so a stray reply could hide an approval request. + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal(isHighPriorityEventForUser(event, PUBKEY, PUBKEY), false); +}); + +test("a real mention on someone else's reply is still high priority", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal(isHighPriorityEventForUser(event, PUBKEY, OTHER_PUBKEY), true); +}); + +test("a reply with an unresolved parent is not high priority", () => { + // Callers resolve the parent for exactly the replies that tag the user, so a + // missing author means the lookup failed. High priority is persisted and + // drops the channel's top-level items from the dock badge, so it fails + // closed — unlike notification delivery, which fails open. + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal(isHighPriorityEventForUser(event, PUBKEY), false); +}); + +test("a top-level p-tag is high priority with no parent to resolve", () => { + assert.equal( + isHighPriorityEventForUser(makeEvent([pTag(PUBKEY)]), PUBKEY), + true, + ); +}); + +test("a broadcast reply is high priority even with an unresolved parent", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY), broadcastTag()]); + assert.equal(isHighPriorityEventForUser(event, PUBKEY), true); +}); + +test("a muted DM does not escalate to a parent lookup it would ignore", () => { + const event = makeEvent([replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + needsResolvedParentAuthor(event, PUBKEY, parentOpts({ isDmChannel: true })), + false, + ); +}); + +test("a thread you were mentioned in keeps notifying, and muting it still wins", () => { + // `isNotifiedForThread` counts a mention as following the thread: it renders + // "Following" and removes the Follow action. This gate has to agree, or the + // user is told they are subscribed to a thread that never notifies again and + // the control that would have subscribed them is gone. + const laterReply = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + pTag(OTHER_PUBKEY), + ]); + + assert.equal( + shouldNotifyForEvent(laterReply, PUBKEY, opts()), + false, + "a thread we have no relationship with stays silent", + ); + + assert.equal( + shouldNotifyForEvent( + laterReply, + PUBKEY, + opts({ mentionedRootIds: new Set([ROOT_ID]) }), + ), + true, + "a reply in a thread we were mentioned in notifies", + ); + + // Same precedence isNotifiedForThread applies: mutedRootIds short-circuits + // the whole membership test, so an explicit thread mute outranks the mention + // that subscribed us. + assert.equal( + shouldNotifyForEvent( + laterReply, + PUBKEY, + opts({ + mentionedRootIds: new Set([ROOT_ID]), + mutedRootIds: new Set([ROOT_ID]), + }), + ), + false, + "muting the thread outranks the mention that subscribed us", + ); +}); + +test("a sender's marker settles the mention question without the parent", () => { + const muted = { mutedChannelIds: new Set(["c1"]), channelId: "c1" }; + + // Marked as addressing: a plain reply. Does not pierce the channel mute, + // and — this is the payoff — needs no parent lookup to know that. + const addressed = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + [...pTag(PUBKEY), "", "reply"], + ]); + assert.equal( + shouldNotifyForEvent(addressed, PUBKEY, opts(muted)), + false, + "an addressing tag does not pierce a channel mute", + ); + assert.equal( + needsResolvedParentAuthor(addressed, PUBKEY, { cachedParentAuthor: null }), + false, + "the marker removes the round trip", + ); + assert.equal(hasAuthoredMentionForEvent(addressed, PUBKEY), false); + assert.equal(isHighPriorityEventForUser(addressed, PUBKEY, null), false); + + // Marked as a mention: pierces the mute even though the parent is ours. + // This is the case the parent lookup cannot decide — passing PUBKEY as the + // parent author would previously have forced "reply". + const typed = makeEvent([ + rootTag(ROOT_ID), + replyTag(PARENT_ID), + [...pTag(PUBKEY), "", "mention"], + ]); + assert.equal( + shouldNotifyForEvent(typed, PUBKEY, { + ...opts(muted), + parentAuthorPubkey: PUBKEY, + }), + true, + "a typed mention pierces the mute even when it answers our own message", + ); + assert.equal(hasAuthoredMentionForEvent(typed, PUBKEY, PUBKEY), true); + assert.equal(isHighPriorityEventForUser(typed, PUBKEY, PUBKEY), true); + + // Unmarked stays exactly as it was: ask the parent. + const bare = makeEvent([rootTag(ROOT_ID), replyTag(PARENT_ID), pTag(PUBKEY)]); + assert.equal( + needsResolvedParentAuthor(bare, PUBKEY, { cachedParentAuthor: null }), + true, + "an unmarked tag still needs the parent", + ); + assert.equal(hasAuthoredMentionForEvent(bare, PUBKEY, PUBKEY), false); + assert.equal(hasAuthoredMentionForEvent(bare, PUBKEY, OTHER_PUBKEY), true); +}); diff --git a/desktop/src/features/notifications/lib/shouldNotify.ts b/desktop/src/features/notifications/lib/shouldNotify.ts index 9ceb9c2df86..4f74fab6c9f 100644 --- a/desktop/src/features/notifications/lib/shouldNotify.ts +++ b/desktop/src/features/notifications/lib/shouldNotify.ts @@ -2,6 +2,7 @@ import type { RelayEvent } from "@/shared/api/types"; import { getThreadReference, isBroadcastReply, + pTagRoleFor, } from "@/features/messages/lib/threading"; export function hasMentionForEvent( @@ -16,13 +17,84 @@ export function hasMentionForEvent( ); } +/** + * True only for a `p` tag that means "you were mentioned", not one that means + * "this answers you". + * + * A reply p-tags the author it answers so agent `require_mention` + * subscriptions receive it, which makes the raw tag useless for telling the + * two apart. Pass the parent's author to separate them; without it this is + * `hasMentionForEvent`. + */ +export function hasAuthoredMentionForEvent( + event: RelayEvent, + currentPubkey: string, + parentAuthorPubkey?: string | null, +): boolean { + if (!hasMentionForEvent(event, currentPubkey)) { + return false; + } + // A broadcast reply is addressed to the channel, not just to the parent's + // author, and `shouldNotifyForEvent` admits it before ever reading the + // parent. The live thread-reply path never sees one either — `isThreadReply` + // excludes them — so demoting it here would leave every surface disagreeing + // about the same event. + if (isBroadcastReply(event.tags)) { + return true; + } + // The sender's own answer, when it gave one. Only a marker that is actually + // present is authoritative — `unknown` and `none` fall through to the parent. + const role = pTagRoleFor(event.tags, currentPubkey); + if (role === "mention") { + return true; + } + if (role === "addressing") { + return false; + } + const { parentId } = getThreadReference(event.tags); + return !( + parentId !== null && + parentAuthorPubkey != null && + parentAuthorPubkey.toLowerCase() === currentPubkey.toLowerCase() + ); +} + export type NotifyOptions = { participatedRootIds: ReadonlySet; followedRootIds: ReadonlySet; authoredRootIds: ReadonlySet; + /** + * Threads where someone `@mentioned` the user. + * + * A term in this gate because it is already a term in `isNotifiedForThread`, + * which renders the thread as "Following" and hides its Follow action. Left + * out, the two disagreed: the user was told they were subscribed to a thread + * that never notified them again, and the control that would have subscribed + * them was gone. + */ + mentionedRootIds?: ReadonlySet; mutedRootIds?: ReadonlySet; mutedChannelIds?: ReadonlySet; channelId?: string | null; + /** + * Author of the event this one replies to, when the caller can resolve it. + * + * Replies p-tag the author they answer so agent `require_mention` + * subscriptions receive them. That tag is indistinguishable from a typed + * `@mention` in the event itself, so without this hint every direct reply + * would pierce a channel or thread mute. Supplying it keeps the mention + * override for real mentions only. Omit it to keep the previous behaviour. + */ + parentAuthorPubkey?: string | null; + /** + * Whether `channelId` is a DM. + * + * Every DM message p-tags both participants, so the "is this p tag a real + * mention or just addressing?" question has no meaning there — the tag is + * how a DM is addressed at all. Without this, a muted DM notifies for a new + * message but stays silent when someone answers you, which is backwards. + */ + isDmChannel?: boolean; }; export function shouldNotifyForEvent( @@ -34,9 +106,12 @@ export function shouldNotifyForEvent( participatedRootIds, followedRootIds, authoredRootIds, + mentionedRootIds = new Set(), mutedRootIds = new Set(), mutedChannelIds = new Set(), channelId = null, + parentAuthorPubkey = null, + isDmChannel = false, } = options; const { parentId, rootId } = getThreadReference(event.tags); @@ -44,7 +119,26 @@ export function shouldNotifyForEvent( return true; } - if (hasMentionForEvent(event, currentPubkey)) { + // A reply we authored the parent of always carries our `p` tag, so that tag + // alone cannot mean "this message mentions you". Only a real mention skips + // the mute gates below; a reply answering us is re-admitted after them. + // Never in a DM: there the addressing tag is the whole point, so demoting it + // would silence answers to you while letting new messages through. + const role = pTagRoleFor(event.tags, currentPubkey); + const isReplyToCurrentUser = + !isDmChannel && + parentId !== null && + currentPubkey.length > 0 && + // The sender's marker when it left one, the parent's author otherwise. A + // `mention` marker settles it the other way: that is the case the parent + // cannot decide, because the recipient is both the author being answered + // and someone typed in the body. + (role === "addressing" || + (role !== "mention" && + parentAuthorPubkey !== null && + parentAuthorPubkey.toLowerCase() === currentPubkey.toLowerCase())); + + if (!isReplyToCurrentUser && hasMentionForEvent(event, currentPubkey)) { return true; } @@ -60,6 +154,14 @@ export function shouldNotifyForEvent( return false; } + // Past the mute gates, a reply answering us always notifies. The + // participated/authored sets below are local and rebuilt from the unread + // window, so on a fresh install they can be empty for a thread we started — + // without this, "someone replied to you" would go unreported. + if (isReplyToCurrentUser) { + return true; + } + if (rootId !== null && participatedRootIds.has(rootId)) { return true; } @@ -72,23 +174,103 @@ export function shouldNotifyForEvent( return true; } + // Below the mute gates on purpose. Being mentioned in a thread subscribes you + // to it, but muting that thread afterwards still wins — the same precedence + // `isNotifiedForThread` applies, where `mutedRootIds` short-circuits the whole + // membership test. + if (rootId !== null && mentionedRootIds.has(rootId)) { + return true; + } + return false; } +/** + * Whether the parent's author is worth a relay round trip for this event. + * + * The query caches only hold the channel being viewed, so a cache lookup + * always misses for a channel the user has not opened this session. Without + * the parent, a reply's addressing `p` tag is indistinguishable from a + * mention — it pierces a mute, and it marks the whole channel high-priority, + * which then hides top-level items there from the dock badge. + * + * The trigger is the `p` tag, not a mute. An earlier version gated on the mute + * alone, which was right while `shouldNotifyForEvent` was the only consumer + * and wrong as soon as `isHighPriorityEventForUser` started depending on the + * parent too — that one needs the answer in unmuted channels as well. Every + * event that does not tag the user still stays on the synchronous path. + */ +export function needsResolvedParentAuthor( + event: RelayEvent, + currentPubkey: string, + options: { + cachedParentAuthor: string | null; + isDmChannel?: boolean; + }, +): boolean { + if (options.cachedParentAuthor !== null) { + return false; + } + // In a DM the addressing tag is the whole point, so every consumer ignores + // the parent's author there. + if (options.isDmChannel === true) { + return false; + } + // A broadcast reply is admitted before the parent is ever consulted, so the + // round trip could not change the answer — and `deliver` (with the unread + // bump behind it) would be stalled on it for nothing. + if (isBroadcastReply(event.tags)) { + return false; + } + // The whole point of the markers: when the sender said which role its `p` tag + // plays, there is nothing to look up. This is the round trip that goes away. + const role = pTagRoleFor(event.tags, currentPubkey); + if (role === "addressing" || role === "mention") { + return false; + } + const { parentId } = getThreadReference(event.tags); + return parentId !== null && hasMentionForEvent(event, currentPubkey); +} + +/** + * High priority means "this is addressed at you personally". + * + * `parentAuthorPubkey` matters here for the same reason it does everywhere + * else: a reply p-tags the author it answers, and treating that tag as a + * mention marks the whole channel high-priority. `shouldCountTowardHomeBadge + * Subtotal` then drops top-level items in a high-priority channel, so a stray + * reply to you could hide a real approval request from the dock badge. + */ export function isHighPriorityEventForUser( event: RelayEvent, currentPubkey: string, + parentAuthorPubkey?: string | null, ): boolean { - if ( - currentPubkey.length > 0 && - event.tags.some( - (tag) => tag[0] === "p" && tag[1]?.toLowerCase() === currentPubkey, - ) - ) { + if (isBroadcastReply(event.tags)) { return true; } - if (isBroadcastReply(event.tags)) { + if (!hasMentionForEvent(event, currentPubkey)) { + return false; + } + // The sender's marker, when present, is the answer — and it short-circuits + // the fail-closed branch below, which only exists because a null parent + // author is ambiguous between "not resolved" and "not us". + const role = pTagRoleFor(event.tags, currentPubkey); + if (role === "mention") { return true; } - return false; + if (role === "addressing") { + return false; + } + // Fails closed where notification delivery fails open. Callers resolve the + // parent for exactly the replies that tag the user, so a null here means the + // lookup was tried and failed — and this flag is persisted and makes + // `shouldCountTowardHomeBadgeSubtotal` drop the channel's top-level items + // from the dock badge. Missing a red dot after a relay flap is recoverable; + // silently hiding an approval request until the channel is read is not. + const { parentId } = getThreadReference(event.tags); + if (parentId !== null && parentAuthorPubkey == null) { + return false; + } + return hasAuthoredMentionForEvent(event, currentPubkey, parentAuthorPubkey); } diff --git a/desktop/src/features/notifications/use-feed-desktop-notifications.ts b/desktop/src/features/notifications/use-feed-desktop-notifications.ts index 4a0865437ed..19811c1a52f 100644 --- a/desktop/src/features/notifications/use-feed-desktop-notifications.ts +++ b/desktop/src/features/notifications/use-feed-desktop-notifications.ts @@ -168,6 +168,9 @@ export function useFeedDesktopNotifications( (item) => !item.channelId || !mutedChannelIds?.has(item.channelId) || + // Mentions pierce a channel mute. Replies that only reach this + // feed via their addressing p-tag are already filtered out by + // `eligibleFeedNotificationItems`. item.category === "mention", ) : []; diff --git a/desktop/src/shared/api/feedTypes.ts b/desktop/src/shared/api/feedTypes.ts new file mode 100644 index 00000000000..8870d09ae52 --- /dev/null +++ b/desktop/src/shared/api/feedTypes.ts @@ -0,0 +1,54 @@ +/** + * Home-feed wire types. Split out of `types.ts` so the feed shape can grow + * without pushing that barrel past the file-size ratchet. + */ + +export type FeedItemCategory = + | "mention" + | "needs_action" + | "activity" + | "agent_activity"; + +export type FeedItem = { + id: string; + kind: number; + pubkey: string; + content: string; + createdAt: number; + channelId: string | null; + channelName: string; + channelType?: string; + tags: string[][]; + category: FeedItemCategory; + /** + * True when this item is in the mention feed only because it replies to one + * of the user's messages. A reply `p`-tags the author it answers so agent + * `require_mention` subscriptions receive it, which the mention feed's `#p` + * query cannot tell apart from a real mention. + */ + replyToSelf?: boolean; +}; + +export type HomeFeed = { + mentions: FeedItem[]; + needsAction: FeedItem[]; + activity: FeedItem[]; + agentActivity: FeedItem[]; +}; + +export type HomeFeedMeta = { + since: number; + total: number; + generatedAt: number; +}; + +export type HomeFeedResponse = { + feed: HomeFeed; + meta: HomeFeedMeta; +}; + +export type GetHomeFeedInput = { + since?: number; + limit?: number; + types?: string; +}; diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index b4f0df6fc09..7257de9f3ac 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -54,18 +54,8 @@ type RawAddChannelMembersResult = { }>; }; -type RawFeedItem = { - id: string; - kind: number; - pubkey: string; - content: string; - created_at: number; - channel_id: string | null; - channel_name: string; - channel_type: string | null; - tags: string[][]; - category: "mention" | "needs_action" | "activity" | "agent_activity"; -}; +import { fromRawFeedItem, type RawFeedItem } from "./tauriFeedMapping"; +export { fromRawFeedItem }; type RawHomeFeedResponse = { feed: { @@ -307,24 +297,6 @@ export async function invokeTauri( } } -export function fromRawFeedItem(item: RawFeedItem) { - return { - id: item.id, - kind: item.kind, - pubkey: item.pubkey, - content: item.content, - createdAt: item.created_at, - channelId: item.channel_id, - channelName: item.channel_name, - // Canonicalize the wire `null` to undefined so FeedItem's optional - // channelType contract holds at runtime (enrichment and the DM - // notification filter both key off `=== undefined`). - channelType: item.channel_type ?? undefined, - tags: item.tags, - category: item.category, - }; -} - function fromRawSearchHit(hit: RawSearchHit) { return { eventId: hit.event_id, diff --git a/desktop/src/shared/api/tauriFeedMapping.ts b/desktop/src/shared/api/tauriFeedMapping.ts new file mode 100644 index 00000000000..43f63cbf060 --- /dev/null +++ b/desktop/src/shared/api/tauriFeedMapping.ts @@ -0,0 +1,38 @@ +/** + * Home-feed wire shapes and their camelCase mapping. Split out of `tauri.ts` + * so the feed payload can grow without pushing that barrel past the + * file-size ratchet. + */ + +export type RawFeedItem = { + id: string; + kind: number; + pubkey: string; + content: string; + created_at: number; + channel_id: string | null; + channel_name: string; + channel_type: string | null; + tags: string[][]; + category: "mention" | "needs_action" | "activity" | "agent_activity"; + reply_to_self?: boolean; +}; + +export function fromRawFeedItem(item: RawFeedItem) { + return { + id: item.id, + kind: item.kind, + pubkey: item.pubkey, + content: item.content, + createdAt: item.created_at, + channelId: item.channel_id, + channelName: item.channel_name, + // Canonicalize the wire `null` to undefined so FeedItem's optional + // channelType contract holds at runtime (enrichment and the DM + // notification filter both key off `=== undefined`). + channelType: item.channel_type ?? undefined, + tags: item.tags, + category: item.category, + replyToSelf: item.reply_to_self === true, + }; +} diff --git a/desktop/src/shared/api/tauriMessages.ts b/desktop/src/shared/api/tauriMessages.ts index 4abe03ee09b..96ab1fdb4e5 100644 --- a/desktop/src/shared/api/tauriMessages.ts +++ b/desktop/src/shared/api/tauriMessages.ts @@ -15,6 +15,7 @@ export async function sendChannelMessage( sentFromThreadTag?: string[], expectedRelayUrl?: string, expectedSignerPubkey?: string, + recipientPubkeys?: string[], ): Promise { const response = await invokeTauri( "send_channel_message", @@ -36,6 +37,10 @@ export async function sendChannelMessage( // closed when the active identity no longer matches, so a community // switch cannot re-sign the captured tenant's content as the new one. expectedSignerPubkey: expectedSignerPubkey ?? null, + // Addressed by the channel rather than typed in the body — every other + // participant in a DM. Kept apart from `mentionPubkeys` so a reply does + // not mark their `p` tag as a mention nobody wrote. + recipientPubkeys: recipientPubkeys ?? null, }, ); return { diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index d1e624ad530..c14ecc61e68 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -203,48 +203,14 @@ export type SendChannelMessageResult = { createdAt: number; }; -export type FeedItemCategory = - | "mention" - | "needs_action" - | "activity" - | "agent_activity"; - -export type FeedItem = { - id: string; - kind: number; - pubkey: string; - content: string; - createdAt: number; - channelId: string | null; - channelName: string; - channelType?: string; - tags: string[][]; - category: FeedItemCategory; -}; - -export type HomeFeed = { - mentions: FeedItem[]; - needsAction: FeedItem[]; - activity: FeedItem[]; - agentActivity: FeedItem[]; -}; - -export type HomeFeedMeta = { - since: number; - total: number; - generatedAt: number; -}; - -export type HomeFeedResponse = { - feed: HomeFeed; - meta: HomeFeedMeta; -}; - -export type GetHomeFeedInput = { - since?: number; - limit?: number; - types?: string; -}; +export type { + FeedItem, + FeedItemCategory, + GetHomeFeedInput, + HomeFeed, + HomeFeedMeta, + HomeFeedResponse, +} from "./feedTypes"; export type { SearchHit, diff --git a/desktop/src/shared/constants/kinds.ts b/desktop/src/shared/constants/kinds.ts index 4f8b7afe2bd..874cea12e57 100644 --- a/desktop/src/shared/constants/kinds.ts +++ b/desktop/src/shared/constants/kinds.ts @@ -92,6 +92,23 @@ export const CHANNEL_MESSAGE_EVENT_KINDS = [ // Keep this in sync with the Home-feed mention query in buzz-db. export const HOME_MENTION_EVENT_KINDS = [...CHANNEL_MESSAGE_EVENT_KINDS]; +// Kinds a reply's parent can have. Deliberately wider than +// CHANNEL_MESSAGE_EVENT_KINDS: a reply can answer a diff message (40008) or a +// plain NIP-01 note (1), and neither is a phantom-unread risk here because +// this set is only ever used to look a parent up by id. +// +// Keep in sync with the reply-parent query in +// `desktop/src-tauri/src/commands/feed.rs`. The backend feed and the live +// desktop path decide notification ownership independently from the same +// question ("did this answer one of my messages?"), so a kind present in one +// list and missing from the other makes them disagree and notify twice. +export const REPLY_PARENT_EVENT_KINDS = [ + ...CHANNEL_MESSAGE_EVENT_KINDS, + KIND_TEXT_NOTE, // 1 — NIP-01 notes bridged in from other clients + 40001, // legacy: pre-migration stream messages, still repliable + KIND_STREAM_MESSAGE_DIFF, // 40008 — `buzz messages send-diff` +] as const; + export const CHANNEL_EVENT_KINDS = [ KIND_DELETION, // 5 — NIP-09 event deletions KIND_REACTION, // 7 — NIP-25 reactions diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index e4028f01716..7b8df9ed934 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -4301,9 +4301,16 @@ function buildTopLevelMessageTags( channelId: string, mentionPubkeys: string[] | undefined, selfPubkey: string, + recipientPubkeys?: string[], ) { const tags: string[][] = [["h", channelId]]; - appendMentionTags(tags, mentionPubkeys, selfPubkey); + // Typed mentions and channel-addressed recipients (DM participants) are both + // bare `p` tags on a top-level message, exactly as the backend emits them. + appendMentionTags( + tags, + [...(mentionPubkeys ?? []), ...(recipientPubkeys ?? [])], + selfPubkey, + ); return tags; } @@ -4313,14 +4320,24 @@ function buildReplyMessageTags( parentEventId: string, rootEventId: string, mentionPubkeys: string[] | undefined, + recipientPubkeys?: string[], ) { // Preserve the reply tag ordering that the desktop message hooks already // expect locally: author p, h, mention ps, then thread e-tags. + // + // The mock does not add the real backend's role markers, so every `p` tag here + // reads as `unknown` — which is the pre-marker fallback path, not a wrong + // answer. Channel recipients are included so a DM reply keeps tagging its + // counterpart. const tags: string[][] = [ ["p", authorPubkey], ["h", channelId], ]; - appendMentionTags(tags, mentionPubkeys, authorPubkey); + appendMentionTags( + tags, + [...(mentionPubkeys ?? []), ...(recipientPubkeys ?? [])], + authorPubkey, + ); if (parentEventId === rootEventId) { tags.push(["e", rootEventId, "", "reply"]); @@ -9579,6 +9596,7 @@ async function handleSendChannelMessage( parentEventId?: string | null; kind?: number | null; mentionPubkeys?: string[]; + recipientPubkeys?: string[] | null; mediaTags?: string[][] | null; emojiTags?: string[][] | null; mentionTags?: string[][] | null; @@ -9674,6 +9692,7 @@ async function handleSendChannelMessage( args.channelId, args.mentionPubkeys, mockPubkey, + args.recipientPubkeys ?? undefined, ), ...extraTags, ]); @@ -9733,6 +9752,7 @@ async function handleSendChannelMessage( args.parentEventId, rootEventId, args.mentionPubkeys, + args.recipientPubkeys ?? undefined, ), ...extraTags, ], @@ -9760,11 +9780,13 @@ async function handleSendChannelMessage( args.parentEventId, args.parentEventId, args.mentionPubkeys, + args.recipientPubkeys ?? undefined, ) : buildTopLevelMessageTags( args.channelId, args.mentionPubkeys, relayIdentity.pubkey, + args.recipientPubkeys ?? undefined, ); const result = await submitSignedEvent(config, {