chore: track previous references and preserve foreign tags

crates/nip29/src/group.rs

@@ -580,6 +580,8 @@ impl Group {
 
     fn absorb(&mut self, event: &Event, me: PublicKey) -> Absorbed {
         if STATE_KINDS.contains(&event.kind) {
+            self.window.note(event);
+
             return if self.absorb_state(event, me) {
                 Absorbed::Updated
             } else {
@@ -587,6 +589,17 @@ impl Group {
             };
         }
 
+        // NIP-29: relays must reject events whose
+        // `previous` references are absent from their database.
+        let previous = event.tags.iter().find(|tag| tag.kind() == "previous");
+        if self.window.references_seen(previous) == Some(false) {
+            log::warn!(
+                "nip29: event {} carries previous references not seen on {}",
+                event.id,
+                self.key.relay()
+            );
+        }
+
         self.track(event);
 
         let updated = self.absorb_membership(event, me);

crates/nip29/src/protocol.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::collections::hash_map::DefaultHasher;
 use std::fmt;
 use std::hash::{Hash, Hasher};
@@ -665,9 +666,10 @@ pub(crate) struct GroupList {
     entries: Vec<GroupListEntry>,
     /// The raw private section of the last seen list event.
     content: Option<String>,
-    /// Whether the content was decrypted in this session; an opaque section
+    /// Whether the content was decrypted in this session.
-    /// (another client's encryption, or a refused decrypt) is carried verbatim.
     readable: bool,
+    /// Tags written by other clients.
+    foreign: Vec<Tag>,
 }
 
 impl GroupList {
@@ -677,8 +679,11 @@ impl GroupList {
         }
 
         let mut entries = Vec::new();
+        let mut foreign = Vec::new();
 
-        for tag in event.tags.iter().filter(|tag| tag.kind() == "group") {
+        for tag in event.tags.iter() {
+            match tag.kind() {
+                "group" => {
                     let fields = tag.as_slice();
 
                     let (Some(id), Some(relay)) = (fields.get(1), fields.get(2)) else {
@@ -699,6 +704,11 @@ impl GroupList {
                         private: false,
                     });
                 }
+                // Rebuilt from the entries on publish.
+                "r" => {}
+                _ => foreign.push(tag.clone()),
+            }
+        }
 
         let content = if event.content.is_empty() {
             None
@@ -710,6 +720,7 @@ impl GroupList {
             entries,
             readable: content.is_none(),
             content,
+            foreign,
         })
     }
 
@@ -777,7 +788,8 @@ impl GroupList {
 
     /// The tags of the public section of the list event.
     pub(crate) fn public_tags(&self) -> Vec<Tag> {
-        let mut tags = Vec::with_capacity(self.entries.len() * 2);
+        let mut tags = self.foreign.clone();
+        tags.reserve(self.entries.len() * 2);
 
         for entry in &self.entries {
             if entry.private {
@@ -967,28 +979,41 @@ impl Activity {
 }
 
 #[derive(Debug, Clone, Default)]
-pub(crate) struct TimelineWindow(Vec<Event>);
+pub(crate) struct TimelineWindow {
+    events: Vec<Event>,
+    /// First 4 bytes of every event seen from this relay,
+    /// for `previous` reference checks.
+    seen: HashSet<u32>,
+}
 
 impl TimelineWindow {
     pub fn new() -> Self {
-        Self(Vec::new())
+        Self::default()
     }
 
     pub fn push(&mut self, event: Event) {
-        self.0.retain(|existing| existing.id != event.id);
+        self.seen.insert(prefix(event.id));
-        let position = self.0.partition_point(|e| e.created_at > event.created_at);
+        self.events.retain(|existing| existing.id != event.id);
+        let position = self
+            .events
+            .partition_point(|e| e.created_at > event.created_at);
 
-        self.0.insert(position, event);
+        self.events.insert(position, event);
-        self.0.truncate(TIMELINE_WINDOW);
+        self.events.truncate(TIMELINE_WINDOW);
+    }
+
+    /// Note an event as seen without keeping it in the window.
+    pub fn note(&mut self, event: &Event) {
+        self.seen.insert(prefix(event.id));
     }
 
     pub fn remove(&mut self, id: EventId) {
-        self.0.retain(|event| event.id != id);
+        self.events.retain(|event| event.id != id);
     }
 
     pub fn previous_tag(&self, me: PublicKey) -> Option<Tag> {
         let references: Vec<String> = self
-            .0
+            .events
             .iter()
             .filter(|event| event.pubkey != me)
             .take(PREVIOUS_REFS)
@@ -1001,4 +1026,121 @@ impl TimelineWindow {
             Some(Tag::custom("previous", references))
         }
     }
+
+    /// Whether every `previous` reference points at an event seen from this
+    /// relay. `None` when the event carries no `previous` tag. Mirrors the
+    /// relay-side rejection the spec expects, so a relay handing out
+    /// out-of-context events can be called out.
+    pub fn references_seen(&self, tag: Option<&Tag>) -> Option<bool> {
+        let values = tag?.as_slice().iter().skip(1);
+
+        for value in values {
+            let Ok(prefix) = u32::from_str_radix(value, 16) else {
+                return Some(false);
+            };
+
+            if !self.seen.contains(&prefix) {
+                return Some(false);
+            }
+        }
+
+        Some(true)
+    }
+}
+
+/// The first 8 hex characters of an event id, as a `previous` reference.
+fn prefix(id: EventId) -> u32 {
+    let bytes = id.as_bytes();
+    u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::LOCAL_KEYS;
+
+    fn event(kind: Kind, tags: Vec<Tag>) -> Event {
+        futures::executor::block_on(
+            EventBuilder::new(kind, "")
+                .tags(tags)
+                .finalize_async(&*LOCAL_KEYS),
+        )
+        .unwrap()
+    }
+
+    fn relay(url: &str) -> RelayUrl {
+        RelayUrl::parse(url).unwrap()
+    }
+
+    #[test]
+    fn public_tags_preserve_foreign_tags() {
+        let event = event(
+            Kind::SimpleGroups,
+            vec![
+                Tag::custom("group", ["pizza", "wss://relay.example", "Pizza Lovers"]),
+                Tag::custom("d", ["favorite-groups"]),
+                Tag::custom("client", ["nostord"]),
+                Tag::custom("r", ["wss://other.example"]),
+            ],
+        );
+
+        let mut list = GroupList::parse(&event).unwrap();
+        assert_eq!(list.entries().len(), 1);
+
+        list.upsert(GroupListEntry {
+            id: GroupId::new("sushi").unwrap(),
+            relay: relay("wss://relay2.example"),
+            name: None,
+            private: false,
+        });
+
+        let tags = list.public_tags();
+
+        // Foreign tags survive a republish...
+        assert!(tags.iter().any(|tag| tag.kind() == "d"
+            && tag.as_slice().get(1).map(String::as_str) == Some("favorite-groups")));
+        assert!(tags.iter().any(|tag| tag.kind() == "client"));
+
+        // ...while `group` and `r` tags are rebuilt from the entries.
+        assert_eq!(tags.iter().filter(|tag| tag.kind() == "group").count(), 2);
+        assert_eq!(tags.iter().filter(|tag| tag.kind() == "r").count(), 2);
+    }
+
+    #[test]
+    fn previous_references_validate_against_seen_events() {
+        let mut window = TimelineWindow::new();
+        let seen = event(Kind::ChatMessage, vec![Tag::custom("t", ["one"])]);
+        let other = event(Kind::ChatMessage, vec![Tag::custom("t", ["two"])]);
+
+        window.push(seen.clone());
+
+        let reference = |id: &EventId| {
+            let bytes = id.as_bytes();
+            Tag::custom(
+                "previous",
+                [format!(
+                    "{0:02x}{1:02x}{2:02x}{3:02x}",
+                    bytes[0], bytes[1], bytes[2], bytes[3]
+                )],
+            )
+        };
+
+        assert_eq!(
+            window.references_seen(Some(&reference(&seen.id))),
+            Some(true)
+        );
+        assert_eq!(
+            window.references_seen(Some(&reference(&other.id))),
+            Some(false)
+        );
+        assert_eq!(window.references_seen(None), None);
+
+        // Any number of references is allowed, including zero.
+        let empty = Tag::custom("previous", Vec::<String>::new());
+        assert_eq!(window.references_seen(Some(&empty)), Some(true));
+
+        // A malformed reference can't be verified.
+        let malformed = Tag::custom("previous", ["nothex"]);
+        assert_eq!(window.references_seen(Some(&malformed)), Some(false));
+    }
 }

crates/workspace/src/sidebar/group_view.rs

@@ -254,11 +254,17 @@ impl Sidebar {
     ) -> AnyElement {
         let hide_avatar = AppSettings::get_hide_avatar(cx);
         let persons = PersonRegistry::global(cx);
+
         let person = persons.read(cx).get(public_key, cx);
+        let group = group.clone();
+
         let public_key = *public_key;
         let member_is_admin = !roles.is_empty();
 
-        let group = group.clone();
+        let is_self = NostrRegistry::global(cx)
+            .read(cx)
+            .current_user()
+            .is_some_and(|me| me == public_key);
 
         Nav::new(public_key.to_hex())
             .label(person.name())
@@ -294,7 +300,7 @@ impl Sidebar {
                         cx.write_to_clipboard(ClipboardItem::new_string(npub));
                     }));
 
-                if !admin {
+                if !admin || is_self {
                     return menu;
                 }
 

docs/TODO.md

@@ -1,7 +1,6 @@
 # TODO
 
-Open items from the search and lazy-loading work. Each entry notes the known
+Open items. Each entry notes the known tradeoff and where the fix would land.
-tradeoff and where the fix would land.
 
 ## nip29
 
@@ -17,3 +16,24 @@ tradeoff and where the fix would land.
   gossip relays, but the deeper fix is a per-relay scoped subscription using
   an `a` tag with `10009:<admin-pk>` coordinates. The exact coordinate format
   each relay accepts needs to be tested first.
+- [ ] **LiveKit AV.** The `/.well-known/nip29/livekit` probe gates the edit
+  toggle, but the token flow (`/.well-known/nip29/livekit/<id>` with NIP-98
+  kind 27235) and a `kind:39004` participant subscription are missing — both
+  only matter once an AV client is on the roadmap.
+- [ ] **Threads pane.** Replies to kind 11 messages send and render
+  (composer target + reply-context line), but there is no dedicated threads
+  view like nostord's.
+- [ ] **NIP-11 `nip29.subgroups` gating.** The subgroup UI (parent edit,
+  child reorder) is always shown; relays that don't support subgroups reject
+  the 9002. Reading the relay information document would gate it.
+- [ ] **Invite sync window.** The moderation subscription replays the last
+  100 moderation events, so invites older than that window appear in the
+  list only once they enter the local envelope cache.
+- [ ] **`previous` mismatches are advisory.** Incoming events whose
+  `previous` references weren't seen from the relay are logged
+  (`Group::absorb`) but still rendered: partial history makes dropping
+  false-positive-prone. Late-publication rejection remains relay-side, per
+  the spec.
+- [ ] **Download stall watchdog.** The auto-updater's streamed download
+  (`auto_update::download`) relies on the HTTP client's defaults; a
+  mid-transfer stall hangs the progress loop instead of erroring out.

docs/nip29-comparison.md

@@ -1,137 +0,0 @@
-# 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: full LiveKit AV (token endpoint +
-  `kind:39004`) and a threads pane. Invite code management, subgroup
-  reparent/reorder and reply chips landed here since the comparison.
-- Spec bugs in ours worth fixing: none open. The `duplicate:` join
-  rejection, 9008 delete-group, reaction aggregation and the `39002`/`39001`
-  membership fallback 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`, rosters resolve `Unknown` as fallback | 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.**~~ **done.** The group subscription now includes a
-   moderation filter (`9009` + `9005`, `GroupId::moderation_filter`),
-   `Group::invites()` exposes active codes (revocations arrive as 9005
-   deletions and via the publisher's own `Sent` path), and the invite dialog
-   lists codes with per-code copy-link and revoke.
-5. ~~**Membership fallback to rosters.**~~ **done.** `Group::
-   adopt_roster_membership`: while the latest own 9000/9001 leaves membership
-   `Unknown`, being listed in 39001/39002 counts as `Member`. `Pending`,
-   `Refused` and `Removed` are never overridden.
-6. **LiveKit — partially done.** The edit dialog probes
-   `/.well-known/nip29/livekit` (expecting 204) and gates the toggle with a
-   description when the relay doesn't announce support. Remaining: the token
-   flow (`/.well-known/nip29/livekit/<id>` with NIP-98 kind 27235) and a
-   `kind:39004` participant subscription — both only useful once an AV
-   client is on the roadmap.
-7. ~~**Subgroup management UI.**~~ **done.** The edit dialog has a Parent
-   field (empty = root; reparenting) and a children list with Up/Down
-   reordering (the spec's 9002 child-list replace semantics). Self-parent and
-   parent-is-child are rejected client-side; NIP-11 `nip29.subgroups` is not
-   read yet.
-8. **Reply UI — partially done.** The composer can target a reply
-   (context menu → Reply, chip with cancel), sends kind 11 with `e` tags, and
-   messages render a reply-context line (author + first line) resolved from
-   loaded rows. Remaining: a dedicated threads pane (nostord has one).
-
-### 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~~ done.
-5. ~~39002/39001 membership fallback~~ done.
-6. ~~Subgroup reparent/reorder in the edit dialog~~ done.
-7. LiveKit probe (done, gates the toggle) + token flow (larger; only if AV is
-   in scope).
-8. Reply sending + chips (done); a dedicated threads pane remains.