From 4c279a068647b3f0933e26413ed77bcce626446b Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 11 Oct 2026 07:57:04 +0700 Subject: [PATCH] feat: add group reaction reaction and deletion --- crates/nip29/src/group.rs | 60 +++++++++++++++ crates/nip29/src/lib.rs | 33 +++++++- crates/nip29/src/protocol.rs | 6 ++ crates/nip29_ui/src/lib.rs | 48 ++++++++++++ crates/nip29_ui/src/message.rs | 50 +++++++++++- docs/nip29-comparison.md | 136 +++++++++++++++++++++++++++++++++ 6 files changed, 328 insertions(+), 5 deletions(-) create mode 100644 docs/nip29-comparison.md diff --git a/crates/nip29/src/group.rs b/crates/nip29/src/group.rs index a497150f..a476e547 100644 --- a/crates/nip29/src/group.rs +++ b/crates/nip29/src/group.rs @@ -27,6 +27,13 @@ enum Absorbed { Updated, } +#[derive(Debug, Clone)] +pub struct Reaction { + pub id: EventId, + pub emoji: String, + pub author: PublicKey, +} + #[derive(Debug, Clone)] pub enum Row { Message(Box), @@ -78,6 +85,7 @@ pub struct Group { state_ids: HashMap, rows: Vec, index: HashSet, + reactions: HashMap>, leave_requests: HashMap, window: TimelineWindow, oldest: Option, @@ -105,6 +113,7 @@ impl Group { state_ids: HashMap::new(), rows: Vec::new(), index: HashSet::new(), + reactions: HashMap::new(), leave_requests: HashMap::new(), window: TimelineWindow::new(), oldest: None, @@ -180,6 +189,10 @@ impl Group { }) } + pub fn reactions(&self, id: &EventId) -> &[Reaction] { + self.reactions.get(id).map(Vec::as_slice).unwrap_or(&[]) + } + pub fn loading_more(&self) -> bool { self.loading_more } @@ -247,6 +260,10 @@ impl Group { self.dispatch(move |gid, prev| gid.delete_event(id, prev), cx) } + pub fn delete_group(&mut self, cx: &mut Context) -> Task> { + self.dispatch(GroupId::delete_group, cx) + } + pub fn create_invite(&mut self, code: &str, cx: &mut Context) -> Task> { self.dispatch(move |id, prev| id.create_invite(code, prev), cx) } @@ -309,6 +326,15 @@ impl Group { } } + pub(crate) fn mark_member(&mut self, cx: &mut Context) { + if self.membership == Membership::Member { + return; + } + self.membership = Membership::Member; + cx.emit(GroupEvent::Updated); + cx.notify(); + } + pub(crate) fn mark_refused(&mut self, reason: String, cx: &mut Context) { if self.membership == Membership::Member { return; @@ -504,6 +530,14 @@ impl Group { } self.rows.retain(|row| row.id() != id); + self.reactions.remove(&id); + + for reactions in self.reactions.values_mut() { + reactions.retain(|reaction| reaction.id != id); + } + + self.reactions.retain(|_, reactions| !reactions.is_empty()); + cx.emit(GroupEvent::Updated); cx.notify(); } @@ -541,6 +575,14 @@ impl Group { }; } + if event.kind == Kind::Reaction { + return if self.push_reaction(event) { + Absorbed::Updated + } else { + Absorbed::Ignored + }; + } + if RENDER_KINDS.contains(&event.kind) { return Absorbed::Shown { fresh: self.push_row(event), @@ -626,6 +668,24 @@ impl Group { true } + fn push_reaction(&mut self, event: &Event) -> bool { + let Some(target) = event.tags.event_ids().next() else { + return false; + }; + + if !self.index.insert(event.id) { + return false; + } + + self.reactions.entry(target).or_default().push(Reaction { + id: event.id, + emoji: event.content.clone(), + author: event.pubkey, + }); + + true + } + fn push_activity(&mut self, event: &Event) -> bool { let activity = match Activity::parse(event) { Ok(activity) => activity, diff --git a/crates/nip29/src/lib.rs b/crates/nip29/src/lib.rs index 1f7722ae..0896a0ef 100644 --- a/crates/nip29/src/lib.rs +++ b/crates/nip29/src/lib.rs @@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::LazyLock; use std::time::Duration; -use anyhow::{Error, Result}; +use anyhow::{Error, Result, anyhow}; use gpui::{App, AppContext, Context, Entity, EventEmitter, Global, Subscription, Task}; use nostr_sdk::prelude::*; use smallvec::{SmallVec, smallvec}; @@ -186,11 +186,16 @@ impl GroupsRegistry { self.tasks.push(cx.spawn(async move |this, cx| { if let Err(error) = sent.await { let reason = error.to_string(); + let duplicate = reason.starts_with("duplicate"); if let Err(error) = this.update(cx, |this, cx| { if let Some(group) = this.group(&key, cx) { group.update(cx, |group, cx| { - group.mark_refused(reason, cx); + if duplicate { + group.mark_member(cx); + } else { + group.mark_refused(reason, cx); + } }); } }) { @@ -412,6 +417,30 @@ impl GroupsRegistry { })); } + /// Ask the relay to delete the group (kind 9008). + pub fn delete_group(&mut self, key: &GroupKey, cx: &mut Context) -> Task> { + let Some(group) = self.group(key, cx) else { + return Task::ready(Err(anyhow!("nip29: group not found"))); + }; + + let sent = group.update(cx, |group, cx| group.delete_group(cx)); + let key = key.clone(); + + cx.spawn(async move |this, cx| { + let result = sent.await; + + if result.is_ok() + && let Err(error) = this.update(cx, |this, cx| { + this.forget(key, cx); + }) + { + log::warn!("forgetting the deleted group failed: {error}"); + } + + result + }) + } + pub(crate) fn reset(&mut self, cx: &mut Context) { self.notification_listener = None; self.signal_consumer = None; diff --git a/crates/nip29/src/protocol.rs b/crates/nip29/src/protocol.rs index 46d91c0e..c0d765f5 100644 --- a/crates/nip29/src/protocol.rs +++ b/crates/nip29/src/protocol.rs @@ -212,6 +212,12 @@ impl GroupId { .tag_maybe(previous) } + pub fn delete_group(&self, previous: Option) -> EventBuilder { + EventBuilder::new(Kind::GroupDeleteGroup, "") + .tag(self.h_tag()) + .tag_maybe(previous) + } + pub fn create_invite(&self, code: &str, previous: Option) -> EventBuilder { EventBuilder::new(Kind::GroupCreateInvite, "") .tag(self.h_tag()) diff --git a/crates/nip29_ui/src/lib.rs b/crates/nip29_ui/src/lib.rs index b1ec01ce..750570f5 100644 --- a/crates/nip29_ui/src/lib.rs +++ b/crates/nip29_ui/src/lib.rs @@ -105,6 +105,7 @@ pub fn actions(group: WeakEntity, cx: &App) -> AnyElement { let edit = group.clone(); let invite = group.clone(); let channel = group.clone(); + let delete = group.clone(); let leave = group; h_flex() @@ -161,6 +162,16 @@ pub fn actions(group: WeakEntity, cx: &App) -> AnyElement { ); }), ) + .child( + Button::new("group-delete") + .icon(IconName::CloseCircle) + .tooltip("Delete group") + .ghost() + .small() + .on_click(move |_event, window, cx| { + confirm_delete(delete.clone(), window, cx); + }), + ) }) .child( Button::new("leave") @@ -211,6 +222,42 @@ pub fn confirm_leave(group: WeakEntity, window: &mut Window, cx: &mut App }); } +pub fn confirm_delete(group: WeakEntity, window: &mut Window, cx: &mut App) { + let Some(name) = group.read_with(cx, |group, _cx| group.display_name()).ok() else { + return; + }; + + let Some(key) = group.read_with(cx, |group, _cx| group.key().clone()).ok() else { + return; + }; + + window.open_dialog(cx, move |this, _window, _cx| { + let key = key.clone(); + + this.confirm() + .width(px(360.)) + .title(format!("Delete {name}?")) + .button_props( + DialogButtonProps::default() + .cancel_text("Cancel") + .ok_text("Delete") + .ok_variant(ButtonVariant::Danger), + ) + .child( + div() + .text_style(TextStyle::Callout) + .child("The relay will remove this group for everyone."), + ) + .on_ok(move |_event, window, cx| { + let registry = GroupsRegistry::global(cx); + let sent = registry.update(cx, |registry, cx| registry.delete_group(&key, cx)); + report(window, cx, sent); + + true + }) + }); +} + /// Remove a member from the group and surface any relay error as a notification. pub fn remove_member( group: WeakEntity, @@ -823,6 +870,7 @@ impl GroupPanel { message, content, show_author, + group.reactions(&message.id), message::Actions { group: self.group.clone(), panel: cx.weak_entity(), diff --git a/crates/nip29_ui/src/message.rs b/crates/nip29_ui/src/message.rs index 21b8e62e..ce440e0f 100644 --- a/crates/nip29_ui/src/message.rs +++ b/crates/nip29_ui/src/message.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use chat::Message; use common::TimestampExt; use gpui::prelude::FluentBuilder; @@ -5,11 +7,12 @@ use gpui::{ AnyElement, App, InteractiveElement, IntoElement, ObjectFit, ParentElement, SharedString, SharedUri, Styled, StyledImage, WeakEntity, div, img, px, }; -use nip29::{Activity, Group}; -use nostr_sdk::prelude::{EventId, Kind}; +use nip29::{Activity, Group, Reaction}; +use nostr_sdk::prelude::{EventId, Kind, PublicKey}; use person::PersonRegistry; use settings::AppSettings; -use theme::{ActiveTheme, TextStyle, Typeset as _, bubble_radius}; +use state::NostrRegistry; +use theme::{ActiveTheme, TextStyle, Typeset as _, bubble_radius, control_radius}; use ui::avatar::Avatar; use ui::menu::{ContextMenu, PopupMenu, PopupMenuItem}; use ui::message::MessageRow; @@ -31,6 +34,7 @@ pub(crate) fn render( message: &Message, content: AnyElement, show_author: bool, + reactions: &[Reaction], actions: Actions, cx: &App, ) -> AnyElement { @@ -52,6 +56,9 @@ pub(crate) fn render( .when(!message.media.is_empty(), |this| { this.child(media(&message.media, cx)) }) + .when(!reactions.is_empty(), |this| { + this.child(reaction_bar(reactions, cx)) + }) .into_any_element(); let id = message.id; @@ -205,6 +212,43 @@ fn action(activity: &Activity) -> String { } } +fn reaction_bar(reactions: &[Reaction], cx: &App) -> AnyElement { + let me = NostrRegistry::global(cx).read(cx).current_user(); + + let mut grouped: BTreeMap> = BTreeMap::new(); + + for reaction in reactions { + grouped + .entry(reaction.emoji.clone()) + .or_default() + .push(reaction.author); + } + + h_flex() + .mt_1() + .gap_1() + .children(grouped.into_iter().map(|(emoji, authors)| { + let mine = me.is_some_and(|me| authors.contains(&me)); + + h_flex() + .gap(px(theme::SPACE)) + .py_0p5() + .px_1() + .rounded(px(control_radius())) + .text_style(TextStyle::Caption) + .border_1() + .when(mine, |this| { + this.text_color(cx.theme().text) + .bg(cx.theme().surface_raised) + .border_color(cx.theme().element_active) + }) + .when(!mine, |this| this.border_color(cx.theme().border)) + .child(SharedString::from(emoji)) + .child(SharedString::from(authors.len().to_string())) + })) + .into_any_element() +} + fn media(media: &[SharedUri], cx: &App) -> AnyElement { if let [only] = media { return div() diff --git a/docs/nip29-comparison.md b/docs/nip29-comparison.md new file mode 100644 index 00000000..5a552cfb --- /dev/null +++ b/docs/nip29-comparison.md @@ -0,0 +1,136 @@ +# NIP-29 alignment: coop vs nostord + +Comparison of our NIP-29 implementation (`crates/nip29`, `crates/nip29_ui`, +`crates/workspace`) against [nostord](https://github.com/nostrord/nostrord) +(`main` @ `bfd933e`), checked against +[NIP-29](https://github.com/nostr-protocol/nips/blob/master/29.md). + +The two clients are architecturally similar: relay-hosted groups keyed by +`(relay, id)`, relay-generated `3900x` state events, user moderation events +`9000-9022`, and a NIP-51 `kind:10009` group list. They made different +trade-offs; each has real gaps the other closed. + +## TL;DR + +- We follow the spec more literally where nostord deviates: `previous` + timeline refs, membership-from-9000/9001 (the spec's "implementation + quirks" section), and fork/migration detection via admins' `kind:10009`. + nostord implements none of those three. +- nostord is ahead on product surface: invite code listing/revocation, full + LiveKit AV (token endpoint + `kind:39004`), threads UI, subgroup hierarchy + management, member rosters as a membership source. +- Spec bugs in ours worth fixing: no `39002`/`39001` fallback for + membership. (Since this comparison: `duplicate:` join rejection, 9008 + delete-group and reaction aggregation were implemented.) + +## 1. Protocol compliance vs NIP-29 + +| Spec item | coop | nostord | +| --- | --- | --- | +| `h` tag on all user→group events | yes, every builder (`crates/nip29/src/protocol.rs`) | yes | +| Moderation kinds 9000/9001/9002/9005/9007/9009 | yes, all | yes, all | +| **9008 delete-group** | yes, plus confirm dialog | yes | +| **9010 update-pin-list + 39005** | send + parse + UI | stub ("disabled until the pinning backend exists") | +| `previous` tag (last 50 seen, excl. self, ≥3 × 8 chars) | `TimelineWindow` (`crates/nip29/src/protocol.rs`) | not built, not validated | +| Membership = latest of own 9000/9001 | `Membership::from_events` | derived from 39001/39002 rosters + local markers instead | +| Join request `code` tag / `naddr1…?invite=` | `GroupReference::parse` | yes, plus legacy formats (`wss://host'id`, `?relay=&group=`) | +| `duplicate:` 9021 rejection = already member | treated as success (`mark_member`) | matched by wording, treated as success | +| 39000 metadata: all flags + `supported_kinds` + `parent`/`child` | yes, plus banner and content-JSON fallback | yes, minus banner; tags only | +| 39001/39002/39003 state | parsed, admins used for actions | richer use (roster as membership source) | +| **39004 livekit participants + token endpoint (NIP-98 kind 27235)** | flag parsed/editable only | full flow incl. `/.well-known` support probe | +| Kind 10009 list | NIP-44 private section, public-wins, opaque-carry | plus foreign-tag preservation and relay-consensus freshness | +| Fork/migration detection (admins' 10009) | `watch_forks` + "may have moved" banner | none | +| Metadata author check (relay-signed 3900x) | parsed from any author | same | + +Both clients ignore "clients should also check [`previous` refs] to keep +relays honest" — fine, it is a SHOULD. NIP-42 auth: we configure +`SignerAuthenticator` on the SDK client (`crates/state/src/lib.rs`), nostord +waits for AUTH explicitly before REQs; both work on auth-gating relays. + +## 2. Where coop is ahead (keep these) + +1. **Fork/migration detection.** The spec says clients SHOULD (MUST when the + primary relay is down) watch admins' `kind:10009`. + `GroupsRegistry::watch_forks` / `evaluate_forks` does this on a 15-minute + cycle with a "may have moved" banner and a rejoin button. nostord has + nothing here. Known limitation (see `docs/TODO.md`): the fork-detection + subscription is unscoped, so NIP-29 group relays refuse it by policy; it + currently works via generic and gossip relays only. +2. **Membership per the spec quirk.** Latest own 9000/9001 decides, with + `Refused` / `Removed` states nostord lacks entirely. +3. **Pinning.** Full 39005/9010 round-trip, pin chips, admin toggle. +4. **Spec-shaped `previous` refs** and a metadata parser that tolerates + content-JSON variants. +5. **Discovery primitives.** Contact-list `10009` sweep plus per-relay + browse (`Discovery`, `GroupsRegistry::browse`). nostord deliberately has + no relay browsing (join by address only). The browse core exists even + though the UI only wires contact-based discovery. + +## 3. Gaps, prioritized + +### P1 — spec correctness / interop + +1. ~~**`duplicate:` join rejection**~~ **done.** A `duplicate:`-prefixed + publish failure on 9021 now marks the group membership as `Member` + (`GroupsRegistry::join_key` → `Group::mark_member`). +2. ~~**9008 delete-group**~~ **done.** `GroupId::delete_group` builder, + `Group::delete_group`, `GroupsRegistry::delete_group` (forgets the group + on accept), and an admin-only "Delete group" button with a confirm + dialog in `nip29_ui::actions`. +3. ~~**Reactions render as message bubbles.**~~ **done.** Kind-7 events no + longer become rows: `Group::absorb` routes them to `push_reaction`, which + aggregates them per target (`Group::reactions`); `nip29_ui` renders a + grouped reaction bar under the target message. Deleting an event drops + its reactions. + +### P2 — client-parity features + +4. **Invite management.** We create a 9009 code and copy the link, but never + subscribe to 9009, so there is no list of active codes and no revoke + (nostord: active codes = 9009 minus 9005 revocations, per-code copy and + revoke). Natural fit: add 9009 to the group's state subscription and an + "Invites" section in the details dialog. +5. **Membership fallback to rosters.** We only derive membership from + 9000/9001. If a relay prunes moderation history (or the user was added + without us seeing the 9000), membership shows `Unknown` while 39002 lists + us. Consider nostord's "external add" adoption: own pubkey in 39001/39002 + → treat as member (or invite card), without weakening the 9000/9001 + override. +6. **LiveKit.** The flag is parsed and editable, but joining an AV group does + nothing. At minimum: the `/.well-known/nip29/livekit` probe (204) to gate + the toggle, and the token flow + (`/.well-known/nip29/livekit/` with NIP-98 kind 27235 plus a 39004 + participant subscription) if AV is on the roadmap. +7. **Subgroup management UI.** `parent` is set at creation (`CreateChannel`), + and `edit_metadata` already carries `parent`/`child` tags, but the edit + dialog cannot reparent or reorder children (the spec's 9002 child-list + replace semantics). nostord has a full Hierarchy tab (move under / make + root / reorder). Nothing reads NIP-11 `nip29.subgroups` to gate the UI. +8. **No reply/thread UI.** `send_message` accepts `replies` but + `GroupPanel::send` always passes empty; kind 11/1111 render inline only. + nostord has reply chips, a threads pane, and NIP-22 tag nesting. + +### P3 — polish + +9. **10009 foreign tags.** We rebuild only `group`/`r` tags on publish + (`GroupList::public_tags`), dropping anything other clients stored in the + event; nostord preserves unknown tags verbatim. Cheap fix, avoids + clobbering. +10. **No `previous` validation on incoming events** (spec SHOULD) and no + client-side late-publication guard — mostly relay-side concerns. +11. **Self-demotion guard.** Member roster UI lives in the sidebar + (`crates/workspace/src/sidebar/group_view.rs`: promote/demote/remove) at + parity with nostord minus roles beyond `admin` (both clients only assign + one role). nostord additionally blocks self-demotion/self-removal — + worth copying to avoid locking yourself out. + +## 4. Suggested order of work + +1. ~~Fix `duplicate:`-prefix handling on 9021~~ done. +2. ~~Exclude/aggregate kind-7 reactions in group rows~~ done. +3. ~~Add 9008 delete-group + confirm dialog~~ done. +4. Subscribe 9009 → invite list + revoke via 9005 (medium). +5. 39002/39001 membership fallback (medium). +6. Subgroup reparent/reorder in the edit dialog (medium, uses the existing + 9002 path). +7. LiveKit probe + token flow (larger; only if AV is in scope).