diff --git a/crates/concord/src/cords/cord01.rs b/crates/concord/src/cords/cord01.rs index 4da781ec..ad7f96ae 100644 --- a/crates/concord/src/cords/cord01.rs +++ b/crates/concord/src/cords/cord01.rs @@ -17,8 +17,8 @@ pub const KIND_SEAL_PLAINTEXT: u16 = 20014; pub const NIP44_MAX_PLAINTEXT: usize = 65_535; const TAG_MS: &str = "ms"; -const TAG_CHANNEL: &str = "channel"; -const TAG_EPOCH: &str = "epoch"; +pub(crate) const TAG_CHANNEL: &str = "channel"; +pub(crate) const TAG_EPOCH: &str = "epoch"; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SealForm { @@ -420,7 +420,10 @@ fn check_plaintext_cap(len: usize) -> Result<(), StreamError> { Ok(()) } -fn unique_tag(rumor: &UnsignedEvent, name: &'static str) -> Result, StreamError> { +pub(crate) fn unique_tag( + rumor: &UnsignedEvent, + name: &'static str, +) -> Result, StreamError> { let mut found: Option = None; for tag in rumor.tags.iter() { diff --git a/crates/concord/src/cords/cord03.rs b/crates/concord/src/cords/cord03.rs index e5df89a4..0524b473 100644 --- a/crates/concord/src/cords/cord03.rs +++ b/crates/concord/src/cords/cord03.rs @@ -5,8 +5,9 @@ use anyhow::Result; use nostr_sdk::prelude::*; use crate::cord01::{ - KIND_WRAP, KIND_WRAP_EPHEMERAL, OpenedStream, SealForm, build_rumor_ms, build_seal, - channel_binding_tags, check_channel_binding, open_wrap, resolve_ms_strict, wrap_seal, + KIND_WRAP, KIND_WRAP_EPHEMERAL, OpenedStream, SealForm, TAG_CHANNEL, TAG_EPOCH, build_rumor_ms, + build_seal, channel_binding_tags, check_channel_binding, open_wrap, resolve_ms_strict, + unique_tag, wrap_seal, }; use crate::cord04::{AuthorityCitation, canonical_decimal, citation_tag}; pub use crate::cords::rumor::RumorError as ChatError; @@ -313,6 +314,20 @@ pub fn open( Ok((opened, chat)) } +/// Rebuild a rumor from a locally-cached copy: the binding tags name its channel and epoch. +pub fn parse_rumor(rumor: &UnsignedEvent) -> Result { + let channel: ChannelId = unique_tag(rumor, TAG_CHANNEL)? + .ok_or(ChatError::MissingTag(TAG_CHANNEL))? + .parse() + .map_err(|_| ChatError::BadTag(TAG_CHANNEL))?; + + let epoch = unique_tag(rumor, TAG_EPOCH)? + .ok_or(ChatError::MissingTag(TAG_EPOCH)) + .and_then(|raw| canonical_decimal(&raw).ok_or(ChatError::BadTag(TAG_EPOCH)))?; + + typed(rumor, &channel, Epoch(epoch)) +} + /// `secret` is the `community_root` for a public channel, its own key for a private one. pub fn plane_keys( held: &[(Epoch, [u8; 32])], diff --git a/docs/community-messages-panel-plan.md b/docs/community-messages-panel-plan.md new file mode 100644 index 00000000..ca78763d --- /dev/null +++ b/docs/community-messages-panel-plan.md @@ -0,0 +1,240 @@ +# Community messages panel + +A community opens as a panel in the center dock when its sidebar row is clicked, +shaped like every Discord-style client (Vector, Armada): + +``` ++----------------+----------------------------------+ +| Channels | | +| # general | messages | +| # random | | ++----------------+ | +| Members | | +| @alice +----------------------------------+ +| @bob | [ composer ] | ++----------------+----------------------------------+ +``` + +One panel holds both columns. The left column scrolls its two sections; the right +column is the timeline plus the composer. The panel is per community, so its +`panel_id` is `community-` and re-clicking a community focuses +the existing panel (`ui::dock::add_panel` already does this by `panel_id`). + +## What already exists + +- `sync::planes` already derives a `Plane` for the Control plane, the Guestbook, + and every public channel; `CommunityRegistry::sync_subscriptions` subscribes to + all of them as one filter, so live channel wraps already reach the client. +- `sync::fold` already opens channel wraps and calls `store::cache_rumor`, so the + local DB already holds the timeline; it folds the Control Plane and the member + list and emits `CommunityEvent::Updated` on every inbound wrap. +- `cord03::fold` turns cached rumors into `ChatMessage`s with edits, deletes and + reactions resolved; `store::query_rumors` / `store::backfill` are the read paths. +- `Community` exposes `channels()`, `members()`, `control()`, `name()`, `icon()`. +- `community::init` is already called by `desktop/src/main.rs`. + +So the panel needs no new protocol work: one reader, one writer, and an event that +asks the workspace to open the panel. + +## 1. `concord`: a cached rumor is a `ChatRumor` + +`store::query_rumors` hands back `UnsignedEvent`s, and `cord03::typed` (already +used by `open`) is private. Add one public wrapper in `crates/concord/src/cords/cord03.rs`: + +```rust +/// Rebuild a rumor from a locally-cached copy: the binding tags name its channel and epoch. +pub fn parse_rumor(rumor: &UnsignedEvent) -> Result { + let channel: ChannelId = unique_tag(rumor, TAG_CHANNEL)? + .ok_or(ChatError::MissingTag(TAG_CHANNEL))? + .parse() + .map_err(|_| ChatError::BadTag(TAG_CHANNEL))?; + + let epoch = unique_tag(rumor, TAG_EPOCH)? + .ok_or(ChatError::MissingTag(TAG_EPOCH)) + .and_then(|raw| canonical_decimal(&raw).ok_or(ChatError::BadTag(TAG_EPOCH)))?; + + typed(rumor, &channel, Epoch(epoch)) +} +``` + +The tags are read with `cord01::unique_tag`, the same reader `check_channel_binding` +uses (both become `pub(crate)`), so a cached copy is parsed by exactly the rule that +accepted it at ingest. `canonical_decimal` and `typed` are already in this file. + +## 2. `community`: channel history and sending + +All in `crates/community/src/community.rs` on `Community`, mirroring `Room`. + +```rust +const MESSAGE_LIMIT: usize = 200; + +/// The secret a channel's plane derives from, and the epoch it is held at. +/// A private channel uses the key it was granted; a public one the community root. +fn channel_secret(&self, channel: &ChannelId) -> Option<(Epoch, [u8; 32])>; + +/// Page a channel's history into the local cache, once, when the channel is opened. +pub fn backfill(&self, channel: &ChannelId, cx: &App) -> Task>; + +/// The channel's timeline, folded from the local cache. +pub fn messages(&self, channel: &ChannelId, cx: &App) -> Task>>; + +/// Seal a message to the channel plane, cache it, then publish it to the relays. +pub fn send( + &self, + channel: &ChannelId, + content: &str, + reply_to: Option, + cx: &App, +) -> Option>>; +``` + +- `backfill`: `store::backfill(&client, channel, &[(epoch, secret)], None, MESSAGE_LIMIT)`, + skipped when `store::query_rumors` already finds wraps for the channel, so it runs + once per channel. `store::backfill` walks up to `MAX_PAGES` pages itself. It + fetches through the client, so the community's relays must be in the pool — + `sync_subscriptions` already adds them on load. +- `messages`: `store::query_rumors(&client, channel, None, MESSAGE_LIMIT)`, then + `cord03::parse_rumor` over each, then `cord03::fold(&rumors, Timestamp::now(), can_delete)`. + The closure is the community's own policy: + `citation_ok(&owner, &id, actor, citation, &floors) && roles.can_act_on_member(actor, &owner, author, Permissions::MANAGE_MESSAGES)`, + built from `self.state.owner`, `self.state.id`, `self.state.floors()` and + `self.control.roles` cloned into the background task. `cord03::fold` returns + newest-first, so reverse it for the bottom-aligned list. +- `send`: `cord03::build_message(author, channel, epoch, content, reply_to.as_ref(), at_ms, timer)` + where `timer` is `control.community.message_expiration` and `at_ms` is now in ms; + `cord03::seal_rumor(&rumor, &plane, &signer, false)`; then — the order matters — + `cord03::open(&wrap, &plane, channel, epoch)` and `store::cache_rumor` *before* + `client.send_event(&wrap).to(&state.relays)`, so the author's own row exists + whether or not a relay answers. Add the community's relays with `add_relay(..) + .and_connect()` first, the way `sync::publish_wraps` does — lifting that loop into + a `pub(crate) sync::publish_wrap(client, &wrap, &relays)` keeps one copy. Publish + failures only `log::warn!`. `seal_rumor` needs the channel's `GroupKey` from + `derive::channel_group_key(secret, channel, epoch)`, and the epoch from + `channel_secret`. Returns `None` without a signer or a held secret, and the rumor + id so the panel can reload. + +`CommunityEvent` gains one variant, and the registry a way to request an open, +mirroring `ChatRegistry::emit_room`: + +```rust +pub enum CommunityEvent { + Updated(CommunityId), + Open(CommunityId), + Error(String), +} + +impl CommunityRegistry { + /// Ask the workspace to open a community's panel. + pub fn emit_community(&mut self, community: &Entity, window: &mut Window, cx: &mut Context); +} +``` + +`emit_community` reads the id and emits `CommunityEvent::Open` through +`cx.defer_in(window, ...)` so the click never re-enters the registry. + +Private channels stay out of this pass: `sync::planes` does not subscribe them and +`CommunityState` has no room for a rotated key yet, so `channel_secret` returning +the granted `key` is the only support they get. + +## 3. `community_ui`: the new crate + +`crates/community_ui`, shaped like `chat_ui` (which is the reference for every +detail: `Panel` impl, notification routing, input handling, message list). + +``` +crates/community_ui/Cargo.toml deps: community, state, ui, theme, common, person, settings, gpui, nostr-sdk, smallvec, anyhow, log +crates/community_ui/src/lib.rs init + CommunityPanel +crates/community_ui/src/message.rs one message row's rendering +``` + +```rust +pub fn init(community: Entity, window: &mut Window, cx: &mut App) -> Entity; + +pub struct CommunityPanel { + id: SharedString, // "community-" + focus_handle: FocusHandle, + community: WeakEntity, + channel: Option, // the selected channel + messages: Vec, // ascending, bottom-aligned list + message_index: HashMap, + list_state: ListState, + input: Entity, + tasks: Vec>>, + subscriptions: SmallVec<[Subscription; 2]>, +} +``` + +- `new` takes the strong `Entity`, subscribes with + `cx.subscribe_in(&community, window, ...)` while it has it, and keeps only the + weak handle afterwards (`ChatPanel::subscribe_room_events` is the same split). + It picks `channels().first()` (the genesis `#general`) and, in the subscription, + `CommunityEvent::Updated(id)` reloads the open channel while + `CommunityEvent::Error(error)` becomes a window notification. A `cx.defer_in` does + the first `backfill` + `messages` load, exactly as `ChatPanel::new` defers `connect`. +- The channel and member lists are read live in `render` through the weak entity + (as the sidebar reads `Community::channels()`), so a new channel or member needs no + invalidation; a dropped entity renders an empty state instead. +- `select_channel(channel, window, cx)` swaps the selection, resets the list and + loads: `backfill` once per channel, then `messages`. +- `reload(cx)` awaits `community.messages(&channel, cx)`, replaces `messages`, + rebuilds `message_index` and `list_state.reset(len)` (then `scroll_to_end`). + Edits, deletes and reactions are folded server-side of the UI, so a full replace + is the honest update and stays small at `MESSAGE_LIMIT`. +- `send(window, cx)` reads `self.input`, calls `community.send(...)`, clears the + input, and reloads when the task resolves. Empty input is refused with a + notification, like `ChatPanel`. +- `render`: `v_flex` holding `h_flex` + - left: `w(px(220.))`, `border_r_1`, `.overflow_y_scrollbar()` column with a + `Channels` section (row = icon `IconName::Message`, or `Lock` when private, plus + `ChannelKeyRef.name`; the selected row takes `cx.theme().ghost_element_selected`) and + a `Members` section (row = `Avatar` from + `PersonRegistry::global(cx).read(cx).get(&pk, cx)` plus the profile name, + honouring `AppSettings::get_hide_avatar` like `TreeRow`). + - right: `v_flex().flex_1().min_w_0()` with `gpui::list(self.list_state, ...)` over + `message::render(...)` and `Scrollbar::vertical(&self.list_state)`, then the + composer row: `Textarea` (`InputEvent::PressEnter` sends) and a + `Button::new("send").icon(IconName::PaperPlaneFill)`. +- A message row: author name (person profile, "Unknown" fallback), `at_ago()` from + `common::TimestampExt`, the content as plain text (no markdown, media or file + rendering in this pass), a muted `(edited)` marker when `edited_at` is set, an + emoji summary line from `reactions`, and `"Message deleted"` in + `cx.theme().text_placeholder` when `deleted`. +- `Panel`: `panel_id` = the id above, `title` = the community icon (`Avatar`) plus + `community.name()`, `closable` = true, no toolbar buttons. + +## 4. `workspace`: open the panel from the sidebar + +- `crates/workspace/Cargo.toml`: add `community_ui = { path = "../community_ui" }`. +- `crates/workspace/src/lib.rs`: subscribe to `CommunityRegistry` beside the chat + subscription and, on `CommunityEvent::Open(id)`, look the community up with + `registry.read(cx).community(&id)` and + `add_panel_to_dock(community_ui::init(community, window, cx), DockPlacement::Center, window, cx)`. + `CommunityEvent::Error` keeps its single handler in the sidebar. +- `crates/workspace/src/sidebar/mod.rs`: `open_community` keeps recording the + recent community and now ends with + `CommunityRegistry::global(cx).update(cx, |registry, cx| registry.emit_community(&community, window, cx))`, + so the row's click handler needs the `window`. + +## 5. Order of work + +1. `cord03::parse_rumor`. +2. `community`: `channel_secret`, `backfill`, `messages`, `send`, `CommunityEvent::Open`, + `emit_community`. +3. `community_ui`: `message.rs`, then the panel with the channel list, the timeline + and the composer, then the member list. +4. `workspace`: the dependency, the registry subscription, the sidebar click. +5. `cargo check -p workspace` (the panel only compiles through it), then a manual + run: create a community, click its sidebar row, send a message and see it through + a second account. + +No tests: the crate follows the "no `unwrap`, errors to the UI" rule and validation +is the manual run above. + +## Out of scope + +Files, reactions as a composer action, edits, threads, pins, typing indicators, +unread badges, notifications, message expiration purging (`store::purge_expired`), +private-channel subscriptions (a rekey cannot be persisted yet), moderation actions, +and community management (metadata, roles, invites). Also unchanged: +`crates/chat/src/lib.rs::handle_notifications` already routes kind 1059 wraps by +subscription id, so concord traffic does not land in the DM trash. diff --git a/docs/concord-community-discovery-plan.md b/docs/concord-community-discovery-plan.md deleted file mode 100644 index 8dcc6646..00000000 --- a/docs/concord-community-discovery-plan.md +++ /dev/null @@ -1,336 +0,0 @@ -# Concord discovery: why no community ever reaches `subscribe` - -Audit + fix plan. Read alongside `docs/concord-usage.md` and -`docs/concord-simplification-plan.md`. - -## Symptom - -`crates/community/src/lib.rs::subscribe` is never called, so no wrap is ever -subscribed to and the sidebar stays empty. `community load: 0 state document(s) -found` is the only clue. - -## Root cause - -`CommunityRegistry::load` only ever reads the **local database**. Nothing in the -discovery path touches a relay. - -``` -community::init - └─ SignerChanged → load - └─ sync::load - ├─ store::load_states(client) → client.database().query(..) // local only - └─ load_list(client, ..) → client.database().query(..) // local only, .limit(1) - → track([]) - → sync_subscriptions: `for community in self.communities` runs zero times - → subscribe never called - → no relay is ever queried - → the database never fills - → load stays empty forever -``` - -The loop is self-reinforcing: the local database is populated *by* the -subscriptions that the empty load prevents. That is why an account which belongs -to several communities in another client still shows nothing — a fresh install -has no `concord/*` state document, and coop has no way to ask for one. - -Confirmed by inspection: - -| Location | What it does | -| --- | --- | -| `crates/community/src/lib.rs:167-191` | `load` → `sync::load`, then `track(states)` | -| `crates/community/src/sync.rs:125-137` | `load` = `store::load_states` + `load_list` | -| `crates/concord/src/store.rs:314-341` | `load_states` queries `client.database()` only | -| `crates/community/src/sync.rs:139-153` | `load_list` queries `client.database()` only, `.limit(1)` | -| `crates/community/src/lib.rs:233-276` | `sync_subscriptions` skips everything when `communities` is empty | - -`subscribe` itself is correct. Do not debug it. - -## What the protocol actually says - -Read from the spec (`concord-protocol/concord`, the submodule referenced by -accordion.chat): `02.md` §8 and `examples.md` §6.2. - -A member's memberships live in the **Community List**, on relays: - -- **Kind `33302`**, addressable, NIP-44-encrypted to self, signed by the - member's real key, one event per **fragment** with `d` = the fragment index in - decimal (`"0"`, `"1"`, …). `13302` is explicitly **retired** ("the - single-event Community List, superseded by `33302` once it outgrew one event — - a replaceable kind cannot fragment", `02.md:314`). -- Every 32-byte value at **any depth** is unpadded base64url, not hex. This is - section-scoped: CORD-05 invite fields stay hex (`examples.md` §6.3). -- Join material is the membership subset — `owner, owner_salt, community_root, - root_epoch, control_pk, channels, relays, name`, plus `control_root` when - held. It is the *only* durable home of a member's keys. -- The two snapshots solve opposite problems: `seed` is the earliest epoch held - (backfill anchor), `current` the latest ("so a fresh device reconstructs the - Community instantly with no epoch-by-epoch walk"). `seed` is omitted when - equal to `current`; embedded snapshots omit `community_id` (inherited). -- A client holds the complete List when it holds a fragment at every index below - `frags`; it unions fragments and merges, so a partial read is safe. - -Two consequences for coop: - -1. **The state document is a coop invention.** `store::{save_state, load_state, - load_states}` write kind `30078` with `d = concord/`, signed by a - per-process `LOCAL_KEYS`, and never leave the machine. No equivalent exists - anywhere in the spec. It is a local cache and must never be treated as the - discovery source. -2. **Discovery is: subscribe to my `33302` → materialize a community from - `current` join material → subscribe to its planes → fold.** The fold produces - the authoritative state; the List only supplies the keys to start. - -## Divergences (coop vs spec) - -| # | Spec | coop today | -| --- | --- | --- | -| 1 | kind `33302`, addressable | was `cord02::list::KIND_COMMUNITY_LIST = 13302` (retired) — **fixed in Phase A** | -| 2 | one event per fragment, `d` = index, `frags` declared | was no `frags`, single event, `d` unused, `load_list` `.limit(1)` — **fixed in Phase A** | -| 3 | 32-byte values unpadded base64url at any depth | was hex for `JoinMaterial.owner`/`control_root`, `CommunityId` serde, `ChannelGrant.key` — **fixed in Phase A** | -| 4 | `seed` omitted when equal to `current`; embedded snapshot omits `community_id`; `seed`'s cosmetic fields rewritten from `current` | was both snapshots emitted verbatim, `community_id` always present — **fixed in Phase A** | -| 5 | fetch from relays | local database only — **fixed in Phase C** | -| 6 | materialize `CommunityState` from join material | was no such path; only `CommunityState::from_genesis` — **fixed in Phase B** | -| 7 | publish the List on create/join (read-modify-write) | `build_list_event` is referenced only by tests and docs — **fixed in Phase D** | -| 8 | private channel keys ride in join material | `ChannelKeyRef` has a key field, but private planes are still not subscribed | - -Divergences 1–7 are resolved. 8 remains, in the narrow sense that `planes()` -still skips private channels rather than deriving their addresses from the -granted key. - -## Plan - -Ordered so each phase is independently reviewable and testable. Nothing here -touches the frozen HKDF derivations or `cord01` envelope semantics. - -### Phase A — make the List interoperable (pure, no I/O) — DONE - -`crates/concord/src/cords/cord02/list.rs` - -1. `KIND_COMMUNITY_LIST` → `33302`; add `frags: u64` to `CommunityList` and - `is_complete(&self, frags) -> bool`. -2. Add a base64url codec for the §8 value set and apply it to every 32-byte - field at every depth. Because `JoinMaterial` currently types `owner` and - `control_pk` as `PublicKey` (nostr's hex serde), this needs either wire - newtypes or `serialize_with`/`deserialize_with` helpers. Keep it local to the - List: `cord05` stays hex. -3. Implement the two §8 MUSTs: omit `community_id` on an embedded snapshot, - omit `seed` when it byte-equals `current`, and rewrite `seed`'s cosmetic - fields (`name`, `relays`, each channel's `name`) from `current` on every - serialization. -4. `build_list_event`/`parse_list_event` take the fragment index and emit/read - the `d` tag. - -Tests: round-trip the `examples.md` §6.2 payload verbatim; `merge` convergence -for two devices and mixed-age fragments; `frags` disagreement resolves to the -larger value; a repack does not shed unknown fields. - -**As built.** The §8 rules live behind private wire structs (`WireList`, -`WireEntry`, `WireSnapshot`, `WireChannel`), so a writer re-encodes on every -serialization while the public types keep their internal hex/`PublicKey` -spellings and `cord05` stays hex. Three deviations from the sketch above: - -- `is_complete` takes the set of fragment indices a client holds, not a count: - a count is wrong when the indices are sparse. -- The reader tolerates non-zero base64url trailing bits. The spec's own §6.2 -example has five such values, so a strict decoder rejects the worked example; - the writer still emits the canonical spelling. -- The third omission MUST was implemented too: an entry whose `added_at` does - not outrun its tombstone is not written. It is a serialization rule exactly - like the other two, so it belongs here rather than in Phase D. - -`parse_list_event` validates the `d` tag but returns just the `CommunityList`; -`fragment_index(event)` reads the index, which kept `sync.rs` untouched until -Phase C. `MAX_MEMBERSHIPS = 50` is kept as a stopgap (see risks): §8 has no -membership limit, and removing the cap needs write-time fragmentation. - -### Phase B — materialize a community from join material (pure) — DONE - -`crates/concord/src/store.rs`, `crates/concord/src/cords/cord02/list.rs` - -1. `CommunityState::from_join_material(material: &JoinMaterial, added_at_ms: - u64) -> Result`: identity/owner/salt/root/root_epoch from the material; - `control_pks = { root_epoch → control_pk }`; `relays` parsed; `channels` from - the grants; `control_root` when present; `heads` empty (the first control - fold fills them); `banned` empty; `dissolved` false. -2. Carry the private channel key: add `key: Option<[u8; 32]>` to - `ChannelKeyRef` (or a parallel map) so a grant's `key` has a home. Without - this, a private channel is silently read-only-until-rekey. - -Tests: a material with and without `control_root`; a private grant's key -survives; `from_join_material` then `planes()` yields the control `control_pk` -plus the guestbook and public channels, i.e. a subscription filter that -addresses real planes. - -**As built.** `from_join_material` does not verify `community_id` against -`owner`/`owner_salt`: the List is signed by the member's own key and encrypted -to self, and the invite path already validates that binding in -`CommunityInvite::validate`. `private` on a materialized channel is simply -`key.is_some()` — the spec's `channels` carry only the Private Channel keys a -member was granted, so a grant with no key is a public channel. Nothing else -changed: `from_genesis` and `apply_fold` construct every channel with -`key: None`, and `planes()` still skips private channels, whose address derives -from the granted key rather than the `community_root`. Carrying the key is what -makes subscribing to them possible later; it is not needed to fix discovery. - -Two tests. In `concord`, `from_join_material` (with and without `control_root`, -a granted key surviving, a public grant staying keyless). In `community`, -`planes()` plus `subscription_filter` over a state built field-by-field (control -+ guestbook + public channel addressed, private skipped) — `JoinMaterial` and -`ChannelGrant` cannot be constructed from `community` because their `extra` -field's type is crate-private, so the materialization and the plane derivation -are each proved where they live. - -### Phase C — the List drives `load` — DONE - -`crates/community/src/sync.rs`, `crates/community/src/lib.rs` - -1. `subscribe_list(client, self_pk)` subscribes to `Kind::Custom(33302)` - `author(self_pk)` under a dedicated `concord/list` subscription id, using - `ReqTarget::auto`. With gossip enabled, `auto` breaks the filter down by - author, so it queries the account's NIP-65 write relays and adds/connects - them itself — bootstrap relays alone would miss a List published elsewhere. -2. `CommunityRegistry` calls `subscribe_list` once per signer (signer change and - the initial defer). It is deliberately **not** called from `load`: - re-subscribing on every List event would re-deliver the List and loop. `reset` - does not unsubscribe it either — `subscribe_list` replaces the subscription - itself, and a `reset`-issued unsubscribe could race the replacement and cancel - discovery. -3. The notification listener routes a `concord/list` event to a new `Signal::List`, - whose consumer re-runs `load`. Community planes keep using `Signal::Event(id)`. -4. `load_list` reads every `33302` event by `self_pk` from the database, keeps the - newest event per fragment index, decrypts and `merge`s them. `.limit(1)` is gone. - An incomplete List is read normally — a missing fragment is news not yet heard. -5. `load` unions two sources: every live List entry (materialized with - `from_join_material`, or refreshed if a state document already exists) and every - held local state the List does not mention. A held membership is dropped only - when a tombstone outranks its `added_at_ms`; absence from the List is never a - fact. Each list-derived state is `save_state`d, so the next `load` is warm. -6. `refresh(held, fresh)` keeps the fold's authority (`heads`, `banned`, - `dissolved`) and the control planes it learned, and takes the List's identity, - relays, and channel keys. Channels are merged by id rather than replaced, so a - public channel the fold discovered is not shed by a List snapshot that predates - it. - -**As built, deviating from the sketch above.** The plan called for -`client.fetch_events(..)`; the SDK's own recommendation is to keep the request -path on a subscription and read the database. This is safer than it sounds: a -relay's event is persisted at `nostr-sdk/src/relay/inner.rs:1291` **before** the -notification is emitted, so a subscription plus a database read loses nothing and -needs no explicit save. The subscription is set up with `ReqTarget::auto` rather -than a hand-built NIP-65 relay map, because gossip already resolves the author's -write relays and connects them on demand. - -Tests (no network, in `crates/community/src/sync.rs`): a membership the List -carries materializes a community even though no state document was ever written -for it, and discovery writes the document so the next load is warm; a held -membership the List never mentions is kept alongside the one it does; a tombstone -outranks a held membership and drops it; and the `concord/list` id is not read as -a community subscription. Fragment events are built with `store::list_entry` + -`CommunityList::joined` + `build_list_event` and saved straight into a memory -database, so the tests exercise the real seal/parse/merge path without a relay. - -### Phase D — publish — DONE - -`crates/community/src/sync.rs`, `crates/concord/src/store.rs`, -`crates/concord/src/cords/cord02/list.rs` - -1. `create` mints the genesis, folds it into a state, and saves that state locally - as before, then announces the community: the genesis wraps to its relay set, - and the membership to the account's own List. Both publishes are best-effort — - a relay that is down is a warning, not a failed create. -2. The List write is a read-modify-write over the copy already held (§8). `create` - reads the newest held fragment, unions its own entry in with - `CommunityList::joined`, builds fragment 0, and publishes it. Publishing saves - it locally as a side effect of `send_event`, before any relay is resolved, so - the fragment survives a relay that is down and no explicit database write is - needed. -3. The fragment's `created_at` is `max(now, previous + 1)`, so an addressable - relay can never quietly keep the copy the write meant to replace. - -**As built, deviating from the sketch above.** Three decisions the sketch did not -cover: - -- The List goes to the account's **NIP-65 write relays** (`.to_nip65()`), not the - community's metadata relays. The List is the member's own document, and it is - the same relay set `subscribe_list` resolves for its `author` filter — the two - halves must agree or a write can land where nothing reads. The genesis wraps, - which belong to the community and not the member, do go to the metadata relays. -- The entry is built by a new `store::list_entry(state, name)`. `JoinMaterial`' - `extra` field is crate-private, so the community crate cannot build one; `name` - is passed in because the state does not carry it — the name lives in the Control - fold, and a created community has it in the metadata. -- A List that already spans more than one fragment is **left alone**: placing a - new membership needs a repack (which fragment does it belong in?), and §8 allows - a repack only against the complete List. `load` keeps a membership the List - never mentions, so the community is still tracked locally; the remote write is - deferred with a warning rather than performed wrongly. - -Tests: `create` records a membership the List round-trips, and a second create -unions into the same document instead of replacing it. - -### Phase E — verify live - -`RUST_LOG=info cargo run -p coop`, sign in with the accordion account that -already belongs to communities. Expect `community {id}: subscribing to ..` and -rows in the sidebar. This is the first time the path can be exercised at all. - -## Validation per phase - -- `cargo test -p concord` (A, B), `cargo test -p community` (B, C, D). -- `cargo clippy --workspace --all-targets`, `cargo fmt --all -- --check`. -- A is provable against the spec's worked example, so it needs no relay. -- C is provable with `nostr-memory`: fragments are built with `build_list_event` - and saved as the subscription would have, then `load` reads them. No relay, - no `LocalRelay`. -- E is the only step that needs real relays. - -## Risks and open decisions - -- **Base64url is case-significant and coop's ids are hex everywhere else.** - Confine the codec to `cord02::list`; any normalisation that case-folds will - silently corrupt §8 values. **Resolved in Phase A**: the codec is private to - `list.rs` and never case-folds. -- **`MAX_MEMBERSHIPS = 50` is not in the spec.** §8 has no membership limit; its - only bound is the 65,536-byte *encoded event*. `fits()` still measures the - NIP-44 plaintext, which understates that by roughly a third. Phase D kept the - count cap and added a guard: a List that already spans more than one fragment is - not appended to, because placing a new membership needs a repack. So a member - with more than one fragment gets no remote write until fragmentation lands; the - community stays local and visible. -- **Relay selection is the difference between finding the account's List and - not.** Resolved in Phase C by `ReqTarget::auto`, whose gossip path resolves the - filter's author to their NIP-65 write relays and connects them. A List - published only to relays with no NIP-65 entry is still unreachable; that is a - user-visible relay setting if it ever bites. -- **Private channels stay unsubscribed until `planes()` derives their address - from the granted key** (Phase B gave `ChannelKeyRef` a home for it, but the - discovery fix does not need it). Public discovery works regardless. -- **Two writers, one key.** Once coop publishes `33302`, an account used from - both accordion and coop has both clients writing the List. §8's - read-modify-write is what keeps that from losing memberships — it is not - optional. -- **A create racing the first list sync can publish over an unseen List.** - `record_membership` unions into what the local database holds, and on a fresh - sign-in that is empty until the `concord/list` subscription has delivered. A - create in that window writes a one-entry fragment 0, and an addressable relay - then replaces the account's fuller List with it. The window is the ordinary - sign-in-to-create interval, so it is small but not zero. The honest fix is to - treat the List write as part of the sync loop — republish `list ∪ local - memberships` whenever the subscription settles — rather than doing it inside - `create`; an EOSE flag is not enough on its own, because an account with no - NIP-65 relays never reaches EOSE and would then never write at all. -- **`store::save_state` signs with a per-process random key.** Harmless while it - stays local, but it means the state document can never be published or - compared; if a future phase wants it on the wire, it needs the account signer. -- **The deployed reference client still writes the retired kind `13302`.** The - spec this plan implements (`concord-protocol/concord` `main`) moved the List to - `33302` in PR #18, merged **2026-08-15**. The `applesauce` `concord` branch that - accordion.chat builds against still declares `13302`, single-event, capped at 50 - memberships, at its head of **2026-08-05**; accordion's pin predates even that - (`0.0.0-concord-20260804145327`). So an account whose memberships were written - by that build stores them under a kind coop deliberately does not read, and will - show an empty sidebar until the client is updated to the fragmented kind. This - is not a bug in the discovery path — Phases C and D are correct against the - current spec — but it is the first thing to check if a live sign-in still shows - nothing. Supporting `13302` alongside `33302` is a deliberate non-goal until the - reference client moves. diff --git a/docs/concord-simplification-plan.md b/docs/concord-simplification-plan.md deleted file mode 100644 index 6d056bd2..00000000 --- a/docs/concord-simplification-plan.md +++ /dev/null @@ -1,358 +0,0 @@ -# Concord backend audit and simplification plan - -Audit of `crates/concord`, triggered by `CommunityRegistry` never reaching -`subscribe`: `sync::load` found zero community state documents. Tracing that -surfaced two separate things: the app only uses a fraction of the crate, and the -crate's writers take a concrete `nostr::Keys`, which the app's signer can never -produce. - -Sizes: ~10,500 lines total — ~6,750 production, ~3,750 tests. - -## Decisions taken - -- **D1 — Keep the unwired protocol surface.** `cord05`/`cord06`/`pins`/paging - stay in the tree for future use. No mass deletion. (Findings are recorded in - §4 for reference only.) -- **D2 — Replace `&Keys` with a signer boundary** for account-key operations. - Verified feasible against the pinned SDK; design in §2. - ---- - -## 1. `&Keys` cannot be replaced by a public key — but it can be replaced by a signer - -The original question was whether functions like `genesis` only need -`signer.get_public_key_async()`. They do not: they sign. - -- `cord02::genesis` (`cords/cord02/mod.rs:117`) → `seal_edition` (`:711`) → - `build_seal` (`cord01.rs:217`), which signs the seal (`.finalize(author)`, - `cord01.rs:226`), and `wrap_seal_with` (`:247`), which signs the wrap. -- Self-addressed documents use NIP-44 to self: `seal_to_self` - (`cord01.rs:201`) derives a conversation key from `keys.secret_key()`. - -A public key can produce neither a Schnorr signature nor an ECDH key, so -"public-key-only" is impossible. The real defect is the **concrete type**: the -app holds `state::UniversalSigner` (async, possibly NIP-46), and a `nostr::Keys` -can never be conjured from it. `docs/concord-usage.md:535-536` already records -this as a deliberate migration pass. - -### What the pinned SDK actually provides - -Pinned rev `b230cec` (`nostr` 0.45.4 / `nostr-sdk` 0.45.2): - -- There is **no `NostrSigner` trait in this revision.** The async signer surface - is three traits, all in the `nostr` crate: - - `AsyncGetPublicKey` — `nostr/src/key/public_key.rs:39` - - `AsyncSignEvent` — `nostr/src/event/mod.rs:366` - - `AsyncNip44` — `nostr/src/nips/nip44/traits.rs:30` -- `Keys` implements all three (`nostr/src/key/mod.rs:298,309,342`), so tests and - local key holders keep working. -- `UniversalSigner` already implements all three with - `Error = UniversalSignerError` (`crates/state/src/signer.rs:148-191`). -- SDK helpers accept them: - - `EventBuilder::finalize_async` — `S: AsyncGetPublicKey + AsyncSignEvent + ?Sized` - (`nostr/src/event/builder.rs:171-193`) - - `GiftWrapBuilder::finalize_async` — `S: AsyncGetPublicKey + AsyncSignEvent + AsyncNip44` - (`nostr/src/nips/nip59.rs:334-355`) - - `UnwrappedGift::from_gift_wrap_async` — `T: AsyncNip44` (`nip59.rs:84-90`) - -So the answer is yes: pass a signer. `UniversalSigner` works as-is. - -### Per-function bounds, not a bundle - -Each function should request only the capabilities it uses. The SDK itself is -designed this way (`UnsignedEvent::finalize_async` takes only `AsyncSignEvent`, -`EventBuilder::finalize_async` takes `AsyncGetPublicKey + AsyncSignEvent`, -NIP-59 takes all three). - -| Operation | Bounds | -| --- | --- | -| Sign a seal/edition/rekey wrap, author already known | `AsyncSignEvent` | -| Build an event where the author comes from the signer | `AsyncGetPublicKey + AsyncSignEvent` | -| To-self documents (Community List, Invite List) | `AsyncGetPublicKey + AsyncNip44`, plus `AsyncSignEvent` when the document is itself an event | -| Decrypt-only (`parse_list_event`, `unwrap_direct_invite`) | `AsyncNip44` | -| Rekey blob encrypt (`build_blob`) | `AsyncGetPublicKey + AsyncNip44` (no signing) | -| Rekey blob open (`open_blob`) | `AsyncNip44` | -| Direct invite build (`GiftWrapBuilder`) | all three | - -Use generics (`S: AsyncSignEvent + ?Sized`), never `&dyn`: the traits carry -associated `Error` types, so `dyn AsyncSignEvent` would force the concrete error -at every call site (`dyn AsyncSignEvent`), -defeating the abstraction. The SDK uses generics throughout for this reason. - -Do **not** define a supertrait bundle -`trait Signer: AsyncGetPublicKey + AsyncSignEvent + AsyncNip44 {}`: all three -supertraits declare an associated `Error`, so `Self::Error` becomes ambiguous, -and the bundle forces NIP-44 onto purely-signing callers (and vice versa). - -Inside concord, replace `builder.finalize(&keys)` with -`builder.finalize_async(signer).await`. `finalize_async` fetches the signer's -public key and uses it as the event author, exactly as `finalize` did, so the -bytes are unchanged for every caller that passes a matching signer. - -We deliberately **do not** pre-check the author against `rumor.pubkey` inside -`build_seal`. The seal's author is the signer's own public key, matching the old -`finalize` semantics; a signer that does not match the rumor is still caught by -`open_wrap_at` as `AuthorMismatch` (`cord01.rs:328`). Pre-checking would also -make it impossible to construct the hostile seals the cord suite relies on as -test vectors (`cord01.rs` `hostile_wraps_are_dropped_in_order`). - -If the repeated `::Error: Error + Send + Sync + 'static` bounds -become too noisy, the only stable-Rust way to shorten them is an owned -error-erased trait (as the app already does with -`crates/state/src/signer.rs:64-138`). That trades precision for brevity; keep -per-function bounds unless the noise proves unmanageable. - -### What must NOT go through the signer - -- **Group-key NIP-44.** `cord01::{seal_bytes, open_bytes, wrap_seal, - wrap_seal_with, rewrap_seal}` encrypt under a `ConversationKey` derived from - HKDF group secrets. `AsyncNip44` can only ECDH against a public key, so group - encryption stays on `ConversationKey` / `GroupKey::keys()`. -- **Wrap signatures.** Wraps are signed by the derived group signer key - (`GroupKey::keys()`), not the account. -- **Locally held raw secrets.** `cord05::{build_bundle_event, build_revocation}` - take a generated `link_signer` whose secret the app stores as - `signer_sk` (`docs/concord-usage.md:306-321`). `&Keys` is correct there; the - app has the secret itself. -- **Local database artifacts.** `store::{cache_rumor, save_state}` sign with the - internal random `LOCAL_KEYS` (`store.rs:18`). No user signer involved. - -### Call-site inventory - -Account-key sites to migrate: - -| Site | Today | After | Bounds | -| --- | --- | --- | --- | -| `cord02::genesis` (`cord02/mod.rs:117`) | `owner: &Keys` | `owner: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `ControlWriter::{publish, set_*}` (`cord02/mod.rs:214-425`) | `keys: &Keys` | `keys: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `seal_edition` (`cord02/mod.rs:711`, internal) | `owner: &Keys` | `owner: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `cord01::build_seal` (`cord01.rs:217`) | `author: &Keys` | `author: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `cord01::{seal_to_self, open_to_self}` (`:201,209`) | `keys: &Keys` | `&S`, async | `AsyncGetPublicKey + AsyncNip44` | -| `guestbook::seal_rumor` (`guestbook.rs:186`) | `author: &Keys` | `author: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `cord03::seal_rumor` (`cord03.rs:295`) | `author: &Keys` | `author: &S` | `AsyncGetPublicKey + AsyncSignEvent` | -| `list::build_list_event` (`list.rs:186`) | `keys: &Keys` | `keys: &S` | all three | -| `list::parse_list_event` (`list.rs:197`) | `keys: &Keys` | `keys: &S` | `AsyncGetPublicKey + AsyncNip44` | -| `cord05::{build_direct_invite, unwrap_direct_invite}` (`:451,478`) | `inviter`/`recipient: &Keys` | **done** | build: all three (`Sized`); unwrap: `AsyncNip44` (`Sized`) | -| `cord05::{build_invite_list, parse_invite_list}` (`:593,604`) | `keys: &Keys` | **done** | build: all three; parse: `AsyncGetPublicKey + AsyncNip44` | -| `cord06::build_blob` (`:302`) | `rotator: &Keys` | **done** | `AsyncGetPublicKey + AsyncNip44` | -| `cord06::open_blob` (`:319`) | `recipient: &Keys` | **done** | `AsyncNip44` | -| `cord06::{build_rekey_chunks, seal_dissolved}` (`:602,737`) | actor `&Keys` | **done** | `AsyncGetPublicKey + AsyncSignEvent` | - -Leave unchanged: `cord05::{build_bundle_event, build_revocation}`, all -`cord01` wrap functions, `GroupKey::keys()`, `store::LOCAL_KEYS`. - -### Async ripple and tests - -Every migrated function becomes `async`. `smol` is already a dev-dependency of -concord (`crates/concord/Cargo.toml:21-23`), so affected `#[test]`s become -`smol::block_on(...)` wrappers. The app's call sites are already async -background tasks. - -### Known constraints - -- `GiftWrapBuilder::finalize_async` and `UnwrappedGift::from_gift_wrap_async` - are generic over `S: Sized` (no `?Sized`), so those functions must stay - generic, never `&dyn`. -- Every converted `S::Error` must be `Error + Send + Sync + 'static` for the - SDK helpers' `Error::other` (`nostr/src/error.rs:100-105`) and for - `anyhow`; `Keys::AsyncGetPublicKey::Error = Infallible`, - `Keys::AsyncSignEvent::Error = nostr::Error`, `UniversalSignerError` - (`crates/state/src/signer.rs:10-32`) all qualify. -- `AsyncGetPublicKey` is worth requiring alongside `AsyncSignEvent` wherever the - author is embedded in the payload: `sign_event` signs the id of the given - unsigned event without rewriting its pubkey, so a mismatched signer is only - caught later by signature verification. - ---- - -## 2. Migration plan - -### Phase 1 — replace `&Keys` with per-function signer bounds (no behavior change) — DONE - -1. No new module: change the signatures listed in the inventory table to - generics over the SDK traits (`S: AsyncGetPublicKey + AsyncSignEvent`, - `S: AsyncSignEvent`, `S: AsyncGetPublicKey + AsyncNip44`, or `S: AsyncNip44`). -2. Migrate the live path only: `cord01::build_seal`, `cord01::{seal_to_self, - open_to_self}`, `seal_edition`, `genesis`, `ControlWriter`, `guestbook:: - seal_rumor`, `cord03::seal_rumor`, `list::{build,parse}_list_event`. -3. Update `docs/concord-usage.md` examples to take a signer. -4. Update concord tests to `smol::block_on`; `&Keys` keeps working because it - implements all three traits. - -Validation: `cargo test -p concord` — 46 passed, 0 failed. The one behavior -change from the plan sketch is the dropped up-front author check in §1. -`cord01::{seal_to_self, open_to_self}` now take `&str` and return `String` -(NIP-44 is UTF-8 text), so the `list` and invite-list callers read the plaintext -with `serde_json::from_str`. - -**Unplanned but forced:** `cord01::{build_seal, seal_to_self, open_to_self}` are -shared helpers, so the unwired callers had to be migrated in the same pass to -keep the crate compiling: `cord05::{build_invite_list, parse_invite_list}` and -`cord06::{build_rekey_chunks, seal_dissolved}` (Phase 3's mechanical part). -`cord05::{build_direct_invite, unwrap_direct_invite}` and -`cord06::{build_blob, open_blob}` were untouched by Phase 1 — they use the NIP-59 -and group-key paths, not the migrated helpers — and were migrated in Phase 3. - -### Phase 2 — app uses the signer — DONE - -1. `sync::create(client, signer, metadata)` (`crates/community/src/sync.rs`) runs - `cord02::genesis`, opens the genesis editions, persists the state with - `store::save_state`, and also stores the genesis wraps so the control plane - folds locally. It is generic over `S: AsyncGetPublicKey + AsyncSignEvent + ?Sized` - (the bounds `genesis` needs and no more, per D2); the app passes its - `UniversalSigner`, so no secret material is exposed and NIP-46 accounts work - too. - `CommunityRegistry::create(metadata, cx)` (`crates/community/src/lib.rs`) is - the GPUI wrapper: it refuses when no account is signed in, otherwise runs the - task off-thread and refreshes tracking, so `sync::load` now returns one state - and `subscribe` finally fires. -2. The app-side reimplementation of `list::parse_list_event` - (`crates/community/src/sync.rs:154-156`) is deleted; `load_list` calls the - real `cord02::list::parse_list_event`. -3. Relays in the metadata are persisted but the genesis is **not** published yet; - `create` is local-only. Wiring genesis/broadcast through the relay pool is the - next app step, not part of this phase. - -Validation: `cargo test -p community` (1 passed), `cargo test -p concord` -(46 passed), `cargo clippy -p community --all-targets`, `cargo fmt -p community ---check`, and `cargo check --workspace --all-targets` are all clean. - -**Deviation from the plan sketch:** the planned "drive `CommunityRegistry`" test -is instead a `sync`-layer test, `sync::tests:: -creating_a_community_persists_a_state_that_subscribes_and_folds`. A GPUI-level -test cannot construct a `NostrRegistry` — it opens LMDB at `config_dir()` and -connects bootstrap relays in `NostrRegistry::new`, which is private and not -injectable — so the test drives a `Client` on an in-memory database -(`nostr-memory`, already a dev-dependency) directly. It asserts the whole -contract the registry depends on: `create` persists a state `load` returns, the -subscription filter addresses the genesis wraps, `fold` yields the created -community, and an inbound control edit folds over it. - -### Phase 3 — migrate the remaining unwired writers — DONE - -`cord05::{build_direct_invite, unwrap_direct_invite}` and -`cord06::{build_blob, open_blob}` now take a signer. The NIP-59 pair keeps a -`Sized` `S` (`AsyncGetPublicKey + AsyncSignEvent + AsyncNip44` to build, -`AsyncNip44` to unwrap) because the SDK's `GiftWrapBuilder::finalize_async` and -`UnwrappedGift::from_gift_wrap_async` are `Sized`-bounded. The blob pair is -`AsyncGetPublicKey + AsyncNip44` to build and `AsyncNip44` to open, with `?Sized`. - -The blobs forced one behavior change, because a signer's NIP-44 is text-only -(`nip44_encrypt_async(public_key, &str)`) while the blob plaintext is a -fixed-width binary record. `build_blob` now carries that record base64-encoded -inside the NIP-44 envelope and `open_blob` decodes it again. The record layout, -the `locator`, and the envelope are unchanged; only the bytes inside the envelope -differ. There are no golden vectors for blobs and no producer or consumer other -than these two functions, so the round-trip stays self-consistent; cord06 remains -unwired and persists nothing. - -Validation: `cargo test -p concord` — 46 passed, 0 failed (the 80-blob -`a_full_send_chunk_stays_within_a_relay_event` size assertion still holds under -the base64 record). `cargo clippy -p concord --all-targets` and -`cargo fmt -p concord --check` are clean. - -### Phase 4 — duplication and hygiene (independent, low risk) — DONE - -1. DONE — `store::load_states(client)` added (with a direct `store` test), the - app-side state-document scan in `sync::load` is gone. -2. DONE — `store::STATE_PREFIX` is public; the app-side `concord/` literals are - gone, and subscription ids reuse the exported prefix. -3. DONE — the shared rumor tag readers and error live in a new `cords::rumor` - module (`RumorError`, `tag`, `required`, `value`, `pubkey`, - `optional_citation`), re-exported as `cord03::ChatError` and - `cord02::guestbook::GuestbookError`. `cord06` keeps its own narrower - `RekeyError`, which the plan scoped out. -4. RETAINED — none of the "never-varied parameters" were removed. Each is - load-bearing for a flow the fold or a writer already implements (D1): - - `complete_memberlist`'s `banned_at` is read by the fold and is exercised - with a non-empty map by `join_leave_kick_and_snapshot_converge_to_one_memberlist`; - `docs/concord-usage.md` already promises to fill it once the banlist head's - timestamp is plumbed through. - - `cache_rumor -> Result` is read by `backfill` to drop expired rumors. - - `coalesce`'s `snapshot_authority` gates which snapshots apply; passing - `None` today is a policy, not a dead parameter. - - `seal_rumor(ephemeral)` and the `until` cursors on `backfill`/`query_rumors` - select protocol modes and paging. -5. DONE — tightened `cord04` visibility: `edition_hash`, `fold`, `FoldResult`, - `bootstrap_head`, `parse_banlist`, `Role::parse` and `Grant::parse` are no - longer `pub`. `HeadSelection` stays `pub` because the public `fold_head` - returns it. -6. DONE — doc drift fixed: the store takes `&Client` throughout (including - `load_state`/`load_states`/`query_rumors`, not just the writers), `backfill` - arity, `set_pin_list`'s missing `.await`, the GPUI `init` signature and - registry names, and the "Not wired up yet" registry bullet. - -### Phase 5 — sidebar calls `create` — DONE - -The last blocker was that nothing invoked `CommunityRegistry::create`; the -running app logged `community load: 0 state document(s) found` and `subscribe` -never ran. The sidebar now: - -1. Renders `CommunityRegistry::communities()` instead of the hardcoded - `dummy_communities()`. `SidebarRow::Community` carries an `Entity`, - labelled with `Community::name()` (control-fold metadata, falling back to the - community id until the first fold). -2. Adds a "New community" row to the Community section that opens a name prompt - and calls `CommunityRegistry::create` with default metadata. Relays stay empty, - so the subscription resolves through `ReqTarget::auto` against the pool's - relays rather than a manual target that `add_relay` might not have connected. -3. Observes the registry, so a `track` or fold re-render reaches the list, and - subscribes to `CommunityEvent::Error`, which is now logged - (`log::error!("community: {error}")`) instead of vanishing. A `cx.notify()` in - the registry's per-community observer propagates the fold that fills in the - name. - -Validation: `cargo check -p workspace -p community --all-targets`, -`cargo test -p community` (1 passed), `cargo clippy -p workspace -p community ---all-targets`, and `cargo fmt -p workspace -p community --check` are clean. - -Still local-only: the genesis is persisted but not published to relays, so a -second account cannot discover the community yet. - ---- - -## 3. Retained-by-decision surface (reference only) - -Per D1 these stay, but they should be understood as unwired, not live: - -| Module | Approx. prod LOC | App use | -| --- | --- | --- | -| `cord06` rotation/refounding/dissolution | ~850 | none | -| `cord05` invites/links/direct/list | ~650 | none (types only, via unused `list::join_material`) | -| `cord04::pins` | ~550 | none | -| `cord03` write path + `fold` + `plane_keys` | ~340 | only `open` / `expiration_of` | -| guestbook / list write paths | ~240 | `open`, `coalesce`, `complete_memberlist`, `is_live` | -| `store` paging / purge / query / load_state(s) | ~180 | `cache_rumor`, `save_state`, `load_states` | - -Truly unreferenced even by tests (safe candidates, but kept per D1): -`CommunityInvite::expired`, `GroupKey::pk_hex`, `From<[u8; 32]>` impls, -`CommunityRoles::{roles, is_empty}`. - ---- - -## 4. Non-goals - -- No mass deletion of unwired modules (D1). -- No changes to frozen HKDF derivations, locators, golden vectors, or `cord01` - envelope semantics. The one exception Phase 3 forced is the blob plaintext - encoding (base64 inside the envelope, see Phase 3); the blob record layout and - `locator` are untouched. -- No group-key encryption through the signer. -- Tests move only alongside the code they cover. - -## 5. Validation - -- `cargo test -p concord` after each phase; `cargo test --workspace` before - landing. -- Phase 1 is behavior-preserving: the existing cord test suite is the oracle. -- Phase 2 adds the app-level test: seed a `CommunityState` via - `store::save_state`, drive `CommunityRegistry`, assert a subscription is made - and an inbound wrap folds into the community. -- Phase 4: `cargo test -p concord -p community` (47 + 1 passed), - `cargo clippy -p concord -p community --all-targets`, and - `cargo fmt -p concord -p community --check` are all clean. - -## 6. Immediate unblock - -Option 2 (the clean path, using `UniversalSigner`) landed in Phase 2. Option 1 -(exposing the local `Keys` from `crates/state/src/lib.rs:254`) is obsolete. diff --git a/docs/sidebar-redesign-plan.md b/docs/sidebar-redesign-plan.md deleted file mode 100644 index 1087ca9b..00000000 --- a/docs/sidebar-redesign-plan.md +++ /dev/null @@ -1,379 +0,0 @@ -# Sidebar redesign: onboarding and tabbed navigation - -The sidebar is currently one flat tree: a user header, four action rows -(Inbox / Requests / Browse / Search), and two collapsible sections (Community, -Messages) whose expansion state is persisted in settings. This plan replaces -that with two distinct states: - -- **Signed out** — a full-height onboarding sidebar with a banner, the brand - mark, and two entry points (`Join now`, `Import identity`), patterned on the - `signed` client's sidebar (`signed/crates/workspace/src/views/sidebar/mod.rs`, - `render_sign_in`). -- **Signed in** — three tabs (Recents, Chats, Communities) selected from an - icon-only tab bar that floats at the bottom of the sidebar: - `absolute`, `bottom_2`, `left_0`, `w_full`, `px_2`. - -The tab split also removes the last reason for collapsible tree sections, so -the `TreeSection` state and the `expanded_sections` setting go away. - -## Decisions taken - -- **D1 — One panel, two states.** `Sidebar` keeps its identity; the state is - chosen by `NostrRegistry::current_user()` the way `Sidebar::render` already - reads it. No second panel, no dock changes. -- **D2 — Three tabs, icons only, floating.** `Recents` (default), `Chats`, - `Communities`. Switching tabs only changes the sidebar body; the user header - stays fixed at the top. -- **D3 — Tabs replace collapsible sections.** `TreeSection`, the caret toggle, - and `AppSettings::expanded_sections` are deleted. Section headers survive as - non-interactive labels inside the tab lists. -- **D4 — "Recent communities" is the only new persisted state.** - `recent_communities: Vec` (community ids, newest first) in `Settings`, - following the removed `pinned_rooms` pattern (`9e47882`). Cap the stored list - at 10, render at most 3. -- **D5 — "Latest chats" needs no new state.** `ChatRegistry::rooms(&RoomKind::Ongoing, cx)` - is already ordered by most recent message: `Room::push_message` advances - `Room::created_at` and `ChatRegistry::sort` keeps the vector sorted. Take the - first 5. -- **D6 — The onboarding sidebar owns identity entry points.** `Workspace::new` - stops auto-opening `ImportIdentity` on `StateEvent::NoSigner`; the sidebar's - `Import identity` button opens it instead, and `Join now` gets a new - create-identity dialog. -- **D7 — Inbox and Search leave the sidebar.** They have no slot in the new IA. - Recommended relocation: two entries in the existing user dropdown menu - (`render_user`), which already hosts Profile / Contact List / Backup / Themes / - Settings. - -## 1. Current state - -| Piece | Where | Today | -| --- | --- | --- | -| Panel | `crates/workspace/src/sidebar/mod.rs` | `Sidebar` renders header + 4 nav rows + tree, signed in or out | -| Rows | `crates/workspace/src/sidebar/tree.rs` | `TreeRow` (`Section`/`Room`/`Community`/`Hint`), `h_8`, avatar, click | -| Sections | `sidebar/mod.rs` | `TreeSection::{Community, Messages}`, caret toggling, persisted in `expanded_sections` | -| Communities | `CommunityRegistry::communities()` | listed with `name()` / `icon()`, **no click handler** | -| Chats | `ChatRegistry::rooms(&RoomKind::Ongoing, cx)` | listed with avatar, name, `created_at.to_ago()` | -| Requests badge | `ChatEvent::Ping` → `new_requests` | dot on the Requests row, cleared when the panel opens | -| Signed-out state | `Sidebar::render` | no dedicated view; `Workspace` opens the `ImportIdentity` modal on `StateEvent::NoSigner` | -| Recents | — | nothing exists; ordering is registry order / message order | -| New chat / New community | — | no UI; community creation prior art is commit `0328d35` (removed in `9e47882`) | -| Search / Inbox panels | `panels/search.rs`, `panels/inbox.rs` | placeholders; `TreeRow` is shared with `SearchPanel` | -| Community view | — | does not exist anywhere (`grep` finds no community panel/view) | - -Two defects worth folding into the rewrite: - -1. The `screening` branch in `Sidebar::render_rows` is dead code: rows only come - from `rooms(&RoomKind::Ongoing)`, so `kind != RoomKind::Ongoing` never holds. -2. `Sidebar` does not observe `NostrRegistry`; it only re-renders when the - chat, community, or settings entities notify. The onboarding state needs that - subscription (and `StateEvent::Busy` is declared but never emitted, so there - is no "still checking credentials" signal — see Phase 4). - -## 2. Target design - -### 2.1 Signed out — onboarding sidebar - -Mirror `render_sign_in` from the signed client with coop's tokens -(`cx.theme().surface_background`, no `sidebar` token exists here): - -``` -v_flex().size_full().relative().bg(surface_background) -├── drag region: absolute, top_0, h_12, w_full, title_bar_drag_handlers -├── background art: absolute, inset_0, img(..).size_full().object_fit(Cover) -└── v_flex().size_full().justify_end().p_4().mb_4().gap_4() - ├── brand mark: svg("brand/coop.svg") (size_12) - ├── headline: "Welcome to Coop!" + tagline - ├── Button "Join now" primary, full width, h_8 - └── Button "Import identity" white/10%, full width, h_8 -``` - -- `Import identity` opens the existing `dialogs/import.rs` modal (the one - `Workspace::import_identity` opens today). -- `Join now` opens a new `dialogs/create_identity.rs` (see Phase 4). -- Assets: add `assets/backgrounds/banner{1..3}.jpg` and - `#[include = "backgrounds/**/*"]` to `crates/assets/src/lib.rs`, then pick one - per launch the way the signed client does (`subsec_nanos % 3`). If banners are - not wanted yet, fall back to a theme-colored background plus the brand mark; - no other layout changes. -- Keep the panel's existing right border and `image_cache(retain_all("sidebar"))`. - -### 2.2 Signed in — shell - -``` -v_flex().size_full().relative().bg(surface_background).border_r_1() -├── render_user(window, cx) // unchanged, title bar drag -├── tab content: v_flex().flex_1().min_h_0() // one uniform_list per tab -│ └── pb_12() clearance so the last row clears the floating bar -└── tab bar: absolute, bottom_2, left_0, w_full, px_2 -``` - -`uniform_list` stays the list primitive (all rows stay `h_8`). The tab bar is a -sibling of the scrolling content, not a child, so it never scrolls. Give each -tab its own `UniformListScrollHandle` so scroll position survives a tab switch. - -The "Getting messages…" pill currently sits at `absolute().bottom_2()` and would -collide with the tab bar; move it above the bar (`bottom_16()`), or render it as -a fixed row at the end of the content column. - -### 2.3 Floating tab bar - -``` -div().absolute().bottom_2().left_0().w_full().px_2() -└── h_flex().w_full().p_1().gap_1().rounded(radius_lg) - .bg(elevated_surface_background).when(shadow, |t| t.shadow_md()) - ├── Button::new("tab-recents").icon(..).ghost().selected(active == Recents) - ├── Button::new("tab-chats").icon(..).ghost().selected(..) - └── Button::new("tab-communities").icon(..).ghost().selected(..) -``` - -- Each button is icon-only, `flex_1` (wrap in `div().flex_1()` if the button's - built-in `flex_shrink_0` fights it), with `.tooltip(label)` and - `Selectable::selected(..)` (`Button::selected` already renders - `ghost_element_selected`). -- Icons: `Message` (Chats), `Group` (Communities), and a new `History` icon for - Recents (`assets/icons/history.svg` + `IconName::History`; the assets crate - already embeds `icons/**/*`). `Inbox` is the no-new-asset fallback. -- Optional: mirror the requests dot on the Chats tab icon (`new_requests`). -- Clicking a tab sets `active_tab` and calls `cx.notify()`; nothing else. - -### 2.4 Recents tab - -One `uniform_list`; empty state when both sections are empty. - -| # | Row | Content | Source | Click | -| --- | --- | --- | --- | --- | -| 1 | Section | `Communities` + count | registry | — | -| 2 | Community ×≤3 | avatar + name | `recent_communities` ∩ registry, falling back to registry order when nothing is recorded | record recent + open (see D/§9) | -| 3 | Action | `Show all communities` | — | switch to Communities tab | -| 4 | Section | `Chats` + count | registry | — | -| 5 | Room ×≤5 | avatar + name + `to_ago()` | first 5 of `rooms(&RoomKind::Ongoing)` | `ChatRegistry::emit_room` (existing path) | -| 6 | Action | `Show all chats` | — | switch to Chats tab | - -Section counts are registry totals, not the truncated row count. Action rows are -`TreeRow`-shaped (`h_8`, clickable) so the list stays uniform; a `NavItem` would -break `uniform_list`'s uniform-height assumption. - -### 2.5 Chats tab - -| Row | Kind | Action | -| --- | --- | --- | -| Contacts | `NavItem`, fixed above the list | `Command::ShowContactList` | -| Requests | `NavItem`, fixed | `Command::ShowRequests`; keep the `new_requests` dot and clear-on-click | -| New chat | `NavItem`, fixed | new `dialogs/new_chat.rs` modal | -| `Chats` + count | section label, first list row | — | -| Room ×all | `TreeRow` | `ChatRegistry::emit_room` | - -Empty list shows the existing "No conversations yet" hint. Only -`RoomKind::Ongoing` rooms are listed; requests stay in the Requests panel, so -the dead screening branch is deleted. - -### 2.6 Communities tab - -| Row | Kind | Action | -| --- | --- | --- | -| Browse | `NavItem`, fixed | `Command::ShowBrowse` | -| New community | `NavItem`, fixed | new `dialogs/new_community.rs` modal | -| `Communities` + count | section label, first list row | — | -| Community ×all | `TreeRow` | record recent + open (see §9) | - -Empty list shows the existing "No communities yet" hint. - -## 3. State and data rules - -- **Recents store.** `Settings.recent_communities: Vec` (community id - hex), newest first, `#[serde(default)]`, accessors via `setting_accessors!`. - A pure helper `record_recent(list, id, cap)` (in `settings`, unit-tested) - moves an existing id to the front and truncates at 10. -- **Rendering recents.** Read the stored list, keep ids present in - `CommunityRegistry::community(id)`, take 3. When the stored list is empty or - fully stale, fall back to the first 3 communities in registry order so the - section is useful on a fresh install. -- **Recording.** Only an explicit community click records; "Show all" rows and - tab switches do not. Account switches need no invalidation because rendering - filters against the current registry; the cap bounds cross-account residue. -- **Latest chats.** First 5 of `rooms(&RoomKind::Ongoing)` (already - newest-message-first). No persistence. -- **Tab state.** `active_tab: SidebarTab` lives on `Sidebar`, default Recents, - not persisted. -- **Identity readiness.** `Sidebar` observes `NostrRegistry` and decides: - `current_user().is_some()` → tabs; else if `NostrRegistry::ready()` → - onboarding; else → an inert sidebar. `ready` is new (Phase 4) and exists to - avoid flashing the onboarding view while the keyring/Nostr-Connect check is - still in flight. - -## 4. Implementation plan - -Each phase is independently reviewable and leaves the app runnable. - -### Phase 1 — tab shell — DONE - -Files: `crates/workspace/src/sidebar/mod.rs`, -`crates/workspace/src/sidebar/tab.rs` (new), `sidebar/tree.rs`, -`crates/settings/src/lib.rs`. - -1. Add `SidebarTab { Recents, Chats, Communities }` with `label()`, `icon()`, - `list_id()`, and `index()` in `sidebar/tab.rs`; add a `TabBar` `RenderOnce` - element implementing §2.3. -2. `Sidebar` gains `active_tab` and one `UniformListScrollHandle` per tab. - Replace `tree_rows()` with `rows_for(tab)` and render one `uniform_list` per - tab (ids `sidebar-recents|chats|communities`). -3. Move existing content into the tabs: rooms → Chats, communities → - Communities; Recents is a hint until Phase 2. Keep `TreeRow` (used by - `panels/search.rs`); replace the `TreeSection` enum with plain section labels - (`SidebarRow::Section { label, count }`, no caret, no click). -4. Delete `toggle_section`, `is_expanded`, `load_expanded`, `save_expanded`, the - `expanded_sections` setting, and the dead screening branch. -5. Add Inbox and Search entries to the user dropdown (`render_user`), per D7. - -Validation: app runs signed in and signed out; chats and communities list and -open as before; tab switching works; requests dot still clears. - -### Phase 2 — Recents tab — DONE - -Files: `crates/settings/src/lib.rs`, `crates/workspace/src/sidebar/mod.rs`, -`sidebar/tree.rs`. - -1. Add `recent_communities` to `Settings` + accessors, and the - `record_recent(..)` helper with unit tests. -2. `rows_for(Recents)`: sections + truncated rows + action rows from §2.4. -3. `Sidebar::open_community(id, ..)` records the id (capped) and notifies; - wire it to community rows in both Recents and Communities. - -Validation: `cargo test -p settings`; manually open communities, restart, and -confirm the Recents order; confirm ≤3 / ≤5 rendering and both "Show all" rows. - -### Phase 3 — tab actions — DONE - -Files: `crates/workspace/src/dialogs/new_chat.rs` (new), -`dialogs/new_community.rs` (new), `crates/workspace/src/dialogs/mod.rs`, -`crates/workspace/src/lib.rs`, `sidebar/mod.rs`. - -1. `Command::NewChat` / `Command::NewCommunity`, handled in `on_command` like - the other modal commands. -2. `new_chat.rs`: a small view (Input + inline error, modeled on - `ImportIdentity`) that parses an npub and opens a DM: - `Room::new(current_user, [peer]).kind(RoomKind::Ongoing)`, then - `chat.emit_room(&entity, window, cx)`; `Workspace` already handles - `ChatEvent::OpenRoom` by docking `chat_ui::init(room)`. -3. `new_community.rs`: restore the modal from `0328d35` (name input → confirm → - `CommunityRegistry::create(CommunityMetadata { name, ..Default::default() }, cx)`). - Surface `CommunityEvent::Error` as a notification instead of only logging it. -4. Wire the Chats/Communities nav rows from §2.5–2.6. - -Validation: create a chat from an npub and confirm the room opens; create a -community and confirm it appears in the Communities tab and in Recents; -requests/contacts/browse still dispatch. - -### Phase 4 — onboarding sidebar — DONE, except `Join now` - -Files: `crates/state/src/lib.rs`, `crates/workspace/src/sidebar/mod.rs`, -`sidebar/onboarding.rs` (new), `crates/workspace/src/dialogs/create_identity.rs` -(new), `crates/workspace/src/lib.rs`, `crates/assets/src/lib.rs` (+ new assets). - -1. `NostrRegistry`: add `ready: bool` (false in `new`), a `mark_ready` helper - called wherever the credential check concludes — `get_user_credential`'s - stored-credential and no-credential paths, the wasm `NoSigner` branch, and - `set_signer`'s completion — with `cx.notify()`; expose `pub fn ready()`. -2. `Sidebar` observes `NostrRegistry` and renders per §3's readiness rule. -3. `sidebar/onboarding.rs` renders §2.1. `Import identity` opens - `dialogs/import.rs`; move `Workspace::import_identity`'s modal construction - into a `dialogs::import::open(window, cx)` helper so both call sites can use - it, then delete the `StateEvent::NoSigner → import_identity` branch and the - now-dead `Workspace::import_identity` method (keep the - `SignerChanged → close modals` arm). -4. `create_identity.rs`: generate `Keys` in the background, show npub + nsec - with copy buttons and a "I saved my key" confirmation, then - `NostrRegistry::set_signer(keys, cx)`. Recommended: do **not** write the key - to the keyring, matching the existing nsec import behavior (see §9). -5. Optional asset work from §2.1 (banners). - -Validation: with no stored credentials the sidebar shows onboarding and no -modal; `Import identity` still signs in; `Join now` signs in with a fresh key; -with bunker credentials the tabs appear without an onboarding flash. - -**Deferred.** `dialogs/create_identity.rs` is not implemented, so `Join now` -renders without a click handler, and the §2.1 banner assets were skipped in -favor of a plain theme-colored background with the brand mark. - -### Phase 5 — polish and cleanup — DONE - -- Reposition the "Getting messages…" pill above the tab bar. -- Empty states and counts for all three tabs; truncation rules (§3). -- Remove now-unused imports (keep the sidebar `retain_all` image cache so the - onboarding banner is cached), re-run `cargo check`; update - `docs/concord-usage.md`'s sidebar paragraph if the row layout it describes - changes. - -## 5. File map - -| File | Change | -| --- | --- | -| `crates/workspace/src/sidebar/mod.rs` | tab state, subscriptions, `rows_for`, readiness gate, user menu additions | -| `crates/workspace/src/sidebar/tab.rs` (new) | `SidebarTab`, `TabBar` | -| `crates/workspace/src/sidebar/onboarding.rs` (new) | signed-out view | -| `crates/workspace/src/sidebar/tree.rs` | section label without caret; keep `TreeRow` for `SearchPanel` | -| `crates/workspace/src/dialogs/new_chat.rs` (new) | npub → DM room | -| `crates/workspace/src/dialogs/new_community.rs` (new) | name → `CommunityRegistry::create` | -| `crates/workspace/src/dialogs/create_identity.rs` (new) | `Join now` key generation + backup | -| `crates/workspace/src/dialogs/import.rs` | `open(window, cx)` helper for the onboarding button | -| `crates/workspace/src/lib.rs` | new commands; drop the auto-opened import modal | -| `crates/state/src/lib.rs` | `NostrRegistry::ready` | -| `crates/settings/src/lib.rs` | `recent_communities`; drop `expanded_sections` | -| `crates/assets/src/lib.rs` + `assets/backgrounds/*` | banner assets (optional) | -| `crates/ui/src/icon.rs` + `assets/icons/history.svg` | Recents tab icon (optional) | - -## 6. Edge cases - -- Fewer than 3 communities / 5 chats: no padding rows; sections render with - whatever exists. -- No communities and no chats: single Recents hint. -- Stale ids in `recent_communities` (community left, dissolved, or another - account): filtered out at render; do not rewrite settings on every render. -- Empty `recent_communities`: fall back to registry order (D4/§3). -- Loading chats: keep the existing pill (repositioned), independent of tabs. -- macOS: onboarding needs its own `title_bar_drag_handlers` region and the - traffic-light padding the user header uses today. -- Settings compatibility: dropping `expanded_sections` is safe (serde ignores - the stale key in `.settings`); `recent_communities` must be `#[serde(default)]`. -- Uniform rows: every list row stays `h_8`; fixed nav rows live outside the - `uniform_list`. - -## 7. Validation - -- `cargo check --workspace`; `cargo test -p settings` (new recents helper), - `cargo test -p community -p chat` to confirm no regressions. -- Manual matrix with `cargo run -p coop`: - 1. No stored credentials → onboarding, both buttons work, no auto modal. - 2. Bunker credentials → tabs on first frame after load (no flash). - 3. Tabs: switch, scroll, "Show all" rows move to the right tab. - 4. Recents: ≤3 communities / ≤5 chats; order follows recency. - 5. New chat from an npub opens the room; New community appears in both tabs. - 6. Requests dot appears on Ping and clears when Requests opens. - 7. Sign out (proxy failure path) → onboarding returns. -- GPUI tests, if any are added, must use `cx.background_executor().timer(..)` - rather than `smol::Timer`, per `AGENTS.md`. - -## 8. Non-goals - -- A community channel/thread view; until it exists, a community click only - records recency (see §9). -- Redesigning Search, Inbox, Requests, or Contact List panel content. -- Pinning chats, per-chat unread counts, or in-sidebar chat search. -- Persisting the active tab. -- Per-account recents scoping. - -## 9. Open decisions - -1. **Community click target.** No community view exists, so the handler can - only record recency. Options: (a) record-only, documented until the view - lands; (b) add a placeholder `CommunityPanel` (Browse-style) to make the - click visible. Recommendation: (a), with `open_community` as the single hook - point for the real view. -2. **Join now persistence.** Recommended: show the nsec once, require - confirmation, do not write the keyring (matches the existing nsec import - warning). Alternative: persist to `USER_KEYRING` like the bunker path. -3. **Inbox / Search relocation.** Recommended: user dropdown (D7). Alternative: - a Chats-tab header search icon for Search, inbox folded into Requests. -4. **Recents scope.** Global list filtered by the current registry - (recommended), or keyed by account public key for strict per-account order. -5. **Recents icon.** Add `History` (two small changes) or reuse `Inbox`.