From da502cd22204895580100fdc2dc9678b644a9f74 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 26 Oct 2026 12:29:30 +1100 Subject: [PATCH 01/13] feat(relay): add a channel auto_join setting for default channels Add `channels.auto_join` (migration 0076, constant default false) so a community's relay owners and admins can mark the channels the relay should join new community members into. This is the contract half (R1): the setting, who may change it, and how it is published. The auto-add at the join sites lands separately (R2). - kind:9002 accepts `["auto_join", "true"|"false"]`. A new authorization tier in the 9002 validator takes the actor's relay roster role (owner|admin) rather than a channel role, so a relay admin who is not a channel member can set it, and an event carrying only that tag needs neither channel-owner nor member authority. Only stream channels qualify. API tokens need `AdminChannels` for it, like `archived`. - `apply_channel_metadata_event` refuses `auto_join=true` unless the state the event leaves behind is open and unarchived, decided under the channel row lock: one event may open and flag a channel, and a concurrent archive cannot slip past a pre-lock check. Every change is announced by one `auto_join_changed` kind:40099 notice. - kind:39000 carries a bare `auto_join` tag only while the setting is effective (set, open, unarchived), so what the event says is what the relay will do, with no cascade logic on visibility flips or archive. - `Db::list_auto_join_channels` (effective channels, oldest first, capped at 32) is the loader R2 will call. - SDK `build_update_channel(.., auto_join)`, `buzz channels update --auto-join `, a NOSTR.md "Default channels" section and the NIP-CL tag table row. Tests: a Postgres-lane journey in edit_metadata_postgres_tests covering the relay-role tier, the 40099 notice, the effective-state tag across a flip to private, the combined open+flag event, and the private, archived and forum refusals; group_metadata_tags unit rows; SDK and CLI builder cases; and auto_join assertions in the announce-channel e2e wire test. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- NOSTR.md | 30 +- crates/buzz-cli/src/commands/channels.rs | 17 +- crates/buzz-cli/src/lib.rs | 9 +- crates/buzz-db/src/runtime/migration.rs | 6 +- crates/buzz-db/src/store/channel.rs | 138 +++++++++- crates/buzz-db/src/store/channel_members.rs | 2 +- crates/buzz-db/src/store/dm.rs | 7 +- .../handlers/edit_metadata_postgres_tests.rs | 259 +++++++++++++++++- crates/buzz-relay/src/handlers/ingest.rs | 13 +- .../buzz-relay/src/handlers/side_effects.rs | 71 ++++- crates/buzz-sdk/src/builders.rs | 63 ++++- crates/buzz-test-client/tests/e2e_relay.rs | 40 ++- docs/nips/NIP-CL.md | 1 + migrations/0076_channel_auto_join.sql | 11 + schema/tables/public/channels.sql | 1 + 15 files changed, 621 insertions(+), 47 deletions(-) create mode 100644 migrations/0076_channel_auto_join.sql diff --git a/NOSTR.md b/NOSTR.md index d47e5c479bf..6ec8856a4a8 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -53,11 +53,11 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **Group creation (kind:9007)** | ✅ | NIP-29; include `name` tag, optional `visibility`, `channel_type`, label `t` and `unlisted` tags | | **Add user (kind:9000)** | ✅ | Open: any user, subject to target's `channel_add_policy` (`owner_only`/`nobody` can block). Private: owner/admin only. Self-add bypasses agent policy but not private-channel auth. A guest cannot add anyone. A new member with no role tag gets the channel's `join_role`; a non-member adding themselves may ask for `join_role` or a lower role. One event may name up to 100 people: one `p` tag each, with an optional role as the `p` tag's third element (NIP-29), falling back to the event's `role` tag. Each person is checked and added on their own. When the event names several people, the OK message is `members: ` followed by JSON listing the non-empty `added`, `updated` (role changed), `already_member` and `rejected` (`[{"reason", "pubkeys"}]`) results. The event is refused only when it is malformed or nobody it names may be added; then one person keeps the single-add error, and several get `invalid: members: {…}`. The event and its membership changes are stored together, so an `error:` rejection stores nothing and the same event can be resent; a resent stored event returns `duplicate:` without results. Re-adding someone at their current role posts no second `member_joined` notice. | | **Remove user (kind:9001)** | ✅ | Self-remove allowed (with last-owner guard). Removing others: owner/admin only. | -| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | +| **Edit group metadata (kind:9002)** | ✅ | `name`/`about`/label `t`/`visibility`/`ttl`/`archived`/`join_role`/`unlisted` tags: owner/admin. `topic`/`purpose` tags: any member except a guest. `auto_join` tag: relay owner/admin (see [Default channels](#default-channels)). Buzz applies 9002 as a partial update: an absent tag leaves that setting unchanged. | | **Admin delete event (kind:9005)** | ✅ | Event author can always delete own. Otherwise owner/admin required. Target must be in same channel. | | **Group deletion (kind:9008)** | ✅ | Owner only. | | **Leave group (kind:9022)** | ✅ | Any member. Last-owner guard prevents orphaned groups. | -| **Group metadata (kind:39000)** | ✅ | Relay-signed; always `d`, `name`, `t`, `P`, `created_at`, `join_role`; `public` or `private`; `restricted` and `closed` per the access flags below; `unlisted` when the channel is unlisted; `about` only if description non-empty; `hidden` for DM channels and private system channels | +| **Group metadata (kind:39000)** | ✅ | Relay-signed; always `d`, `name`, `t`, `P`, `created_at`, `join_role`; `public` or `private`; `restricted` and `closed` per the access flags below; `unlisted` when the channel is unlisted; `about` only if description non-empty; `hidden` for DM channels and private system channels; `auto_join` while the channel is a default channel | | **Unlisted channels** | ✅ | kind:9002 `["unlisted", "true"\|"false"]`, owner/admin, or at creation on kind:9007. An unlisted open channel is left out of every read that does not name it, for non-members: channel lists, kind:39000 without `#d`, messages, COUNT, NIP-50 search and artifact queries. Naming it by `#h` (or `#d` on 39000–39003) reads it, and kind:9021 joins it, as for any open channel. See [Unlisted channels](#unlisted-channels). | | **Group admins (kind:39001)** | ✅ | Relay-signed; `d`, channel type `t` and creator `P` tags + `p` tags with roles (`owner`, `admin`) | | **Group members (kind:39002)** | ✅ | Relay-signed; `d`, channel type `t` and creator `P` tags + `p` tags for all members | @@ -192,6 +192,30 @@ and join it, but it does not show up for people who are not members. channel as unlisted. - Unlisted is not an access control. Anyone with the id can read the channel. +### Default channels + +A community's owners and admins can mark a channel as a **default channel**: +one the relay joins every new community member into. The setting is +`auto_join` on the channel. + +- Only a *relay* owner or admin (the NIP-43 roster role, not a channel role) + can change it, with kind:9002 `["auto_join", "true"]` or + `["auto_join", "false"]`. The actor need not be a member of the channel. + A missing tag means "no change". +- It is accepted only on an open, unarchived `stream` channel. The relay + decides that in the same transaction as the other kind:9002 changes, so one + event may open a channel and flag it. Making a flagged channel private or + archiving it is allowed and leaves the stored value in place. +- kind:39000 carries a bare `["auto_join"]` tag only while the setting is + effective: set, open and unarchived. What the event says is what the relay + does, so a client never has to combine the flag with the access flags. +- Nothing is a default channel until someone asks for it: new channels and + existing channels start with `auto_join` off. `buzz channels update + --channel --auto-join true` sets it. + +Every change is announced by one kind:40099 `auto_join_changed` system +message in the channel. + ### Group Discovery The relay emits NIP-29 group state events when channels are created, updated, or membership changes. @@ -201,7 +225,7 @@ All discovery events include a `d` tag set to the channel UUID (NIP-29 addressab | Kind | Tags | Content | |------|------|---------| -| **39000** | `d=`, `name`, `t=`, `P=`, `created_at=`, `join_role` (always); `public` or `private`; `restricted` (private channels, and `join_role=guest`); `closed` (private channels only); `unlisted` (unlisted channels); `about` (if description non-empty); `hidden` (DM channels and private system channels) | Group metadata. The access flags follow the channel settings (see [Announce channels](#announce-channels)). | +| **39000** | `d=`, `name`, `t=`, `P=`, `created_at=`, `join_role` (always); `public` or `private`; `restricted` (private channels, and `join_role=guest`); `closed` (private channels only); `unlisted` (unlisted channels); `about` (if description non-empty); `hidden` (DM channels and private system channels); `auto_join` (open, unarchived channels with the setting on, see [Default channels](#default-channels)) | Group metadata. The access flags follow the channel settings (see [Announce channels](#announce-channels)). | | **39001** | `d=`, `t=`, `P=`, `p` tags with role label (`owner`, `admin`) | Admin list | | **39002** | `d=`, `t=`, `P=`, `p` tags for all members | Member list | diff --git a/crates/buzz-cli/src/commands/channels.rs b/crates/buzz-cli/src/commands/channels.rs index d4a8d6fe936..40de86b6862 100644 --- a/crates/buzz-cli/src/commands/channels.rs +++ b/crates/buzz-cli/src/commands/channels.rs @@ -1640,6 +1640,7 @@ fn validate_update_channel_fields( ttl_change: Option>, join_role: Option, unlisted: Option, + auto_join: Option, ) -> Result<(), CliError> { if name.is_none() && description.is_none() @@ -1647,9 +1648,10 @@ fn validate_update_channel_fields( && ttl_change.is_none() && join_role.is_none() && unlisted.is_none() + && auto_join.is_none() { return Err(CliError::Usage( - "at least one field required (--name, --description, --visibility, --ttl, --no-ttl, --join-role, --unlisted)" + "at least one field required (--name, --description, --visibility, --ttl, --no-ttl, --join-role, --unlisted, --auto-join)" .into(), )); } @@ -1667,6 +1669,7 @@ pub async fn cmd_update_channel( no_ttl: bool, join_role: Option, unlisted: Option, + auto_join: Option, ) -> Result<(), CliError> { // Outer Option: None leaves TTL unchanged. Inner: Some(secs) sets it, // None (from --no-ttl) clears it, making the channel permanent. @@ -1683,6 +1686,7 @@ pub async fn cmd_update_channel( ttl_change, join_role, unlisted, + auto_join, )?; let channel_uuid = parse_uuid(channel_id)?; // `--unlisted false` needs no check: a relay without the extension lists @@ -1699,6 +1703,7 @@ pub async fn cmd_update_channel( ttl_change, join_role, unlisted, + auto_join, ) .map_err(|e| CliError::Other(format!("build_update_channel failed: {e}")))?; @@ -2006,6 +2011,7 @@ pub async fn dispatch( no_ttl, join_role, unlisted, + auto_join, } => { let visibility = visibility.as_ref().map(|v| v.to_string()); cmd_update_channel( @@ -2018,6 +2024,7 @@ pub async fn dispatch( no_ttl, join_role.map(Into::into), unlisted, + auto_join, ) .await } @@ -2207,7 +2214,7 @@ mod tests { #[test] fn update_channel_fields_rejects_empty_update() { - let result = validate_update_channel_fields(None, None, None, None, None, None); + let result = validate_update_channel_fields(None, None, None, None, None, None, None); assert!(matches!(result, Err(CliError::Usage(_)))); let msg = result.unwrap_err().to_string(); assert!(msg.contains("at least one field required")); @@ -2216,7 +2223,8 @@ mod tests { #[test] fn update_channel_fields_accepts_visibility_only_update() { - let result = validate_update_channel_fields(None, None, Some("open"), None, None, None); + let result = + validate_update_channel_fields(None, None, Some("open"), None, None, None, None); assert!(result.is_ok(), "visibility-only update should be accepted"); } @@ -4823,6 +4831,7 @@ mod unlisted_support_tests { false, None, Some(true), + None, ) .await .unwrap_err(); @@ -4849,6 +4858,7 @@ mod unlisted_support_tests { false, None, Some(true), + None, ) .await .unwrap(); @@ -4877,6 +4887,7 @@ mod unlisted_support_tests { false, None, Some(false), + None, ) .await .unwrap(); diff --git a/crates/buzz-cli/src/lib.rs b/crates/buzz-cli/src/lib.rs index 1dc1438e1b0..c529db3e695 100644 --- a/crates/buzz-cli/src/lib.rs +++ b/crates/buzz-cli/src/lib.rs @@ -699,9 +699,9 @@ pub enum ChannelsCmd { #[arg(long, value_name = "PATH")] templates_file: Option, }, - /// Update channel name, description, visibility, join role, unlisted, or ephemeral TTL + /// Update channel name, description, visibility, join role, unlisted, auto-join, or ephemeral TTL #[command( - after_help = "Examples:\n buzz channels update --channel --name general\n buzz channels update --channel --visibility open\n buzz channels update --channel --visibility private\n buzz channels update --channel --join-role guest\n buzz channels update --channel --visibility open --unlisted true" + after_help = "Examples:\n buzz channels update --channel --name general\n buzz channels update --channel --visibility open\n buzz channels update --channel --visibility private\n buzz channels update --channel --join-role guest\n buzz channels update --channel --visibility open --unlisted true\n buzz channels update --channel --auto-join true" )] Update { /// Channel UUID @@ -735,6 +735,11 @@ pub enum ChannelsCmd { /// does not advertise `buzz-unlisted-channels`. #[arg(long, value_name = "BOOL", action = clap::ArgAction::Set)] unlisted: Option, + /// Whether the relay joins every new community member into this + /// channel (a default channel). Relay owner/admin only; accepted only + /// on an open, unarchived stream channel. + #[arg(long, value_name = "true|false")] + auto_join: Option, }, /// Set the channel topic Topic { diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 7a1a551b7c0..af6e6bbdc31 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -776,10 +776,14 @@ mod postgres_tests { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); migrations.sort_by_key(|migration| migration.version); - assert_eq!(migrations.len(), 75); + assert_eq!(migrations.len(), 76); for (index, migration) in migrations.iter().enumerate() { assert_eq!(migration.version, (index + 1) as i64); } + assert!(migrations[75] + .sql + .as_str() + .contains("ADD COLUMN auto_join BOOLEAN NOT NULL DEFAULT false")); assert!(migrations[70] .sql .as_str() diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 8907b224a93..b13aaa5115a 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -89,6 +89,10 @@ pub struct ChannelRecord { /// join an unlisted open channel by naming it, and private channels are /// never listed to non-members. pub unlisted: bool, + /// Whether the relay joins new community members into this channel. Only + /// effective while the channel is open and unarchived; see + /// [`group_metadata_tags`] and [`list_auto_join_channels`]. + pub auto_join: bool, } impl ChannelRecord { @@ -182,7 +186,7 @@ pub async fn create_channel( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND id = $2 "#, ) @@ -245,7 +249,7 @@ pub async fn create_channel_with_id( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND id = $2 "#, ) @@ -488,7 +492,7 @@ async fn get_channel_with_operation( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND id = $2 AND deleted_at IS NULL "#, ) @@ -577,7 +581,7 @@ async fn list_channels_with_operation( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND deleted_at IS NULL AND visibility::text = $2 ORDER BY created_at DESC @@ -597,7 +601,7 @@ async fn list_channels_with_operation( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND deleted_at IS NULL ORDER BY created_at DESC @@ -612,6 +616,51 @@ async fn list_channels_with_operation( rows.into_iter().map(row_to_channel_record).collect() } +/// The most channels the relay joins a new community member into. +pub const AUTO_JOIN_CHANNEL_LIMIT: i64 = 32; + +/// Lists the channels whose `auto_join` is effective: set, open, unarchived +/// and not deleted. Oldest first, so the first default channel a member sees +/// is the one that was flagged first. Bounded by [`AUTO_JOIN_CHANNEL_LIMIT`]. +pub async fn list_auto_join_channels( + pool: &PgPool, + community_id: CommunityId, +) -> Result> { + let mut connection = crate::observability::acquire_writer( + pool, + crate::observability::WriterOperation::Authorization, + ) + .await?; + let rows = sqlx::query( + r#" + SELECT id, name, channel_type::text AS channel_type, visibility::text AS visibility, + description, canvas, + created_by, created_at, updated_at, archived_at, deleted_at, + nip29_group_id, topic_required, max_members, + topic, topic_set_by, topic_set_at, + purpose, purpose_set_by, purpose_set_at, + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join + FROM channels + WHERE community_id = $1 AND auto_join AND visibility = 'open' + AND archived_at IS NULL AND deleted_at IS NULL + ORDER BY created_at ASC + LIMIT $2 + "#, + ) + .bind(community_id.as_uuid()) + .bind(AUTO_JOIN_CHANNEL_LIMIT) + .fetch_all(&mut *connection) + .await?; + if rows.len() as i64 == AUTO_JOIN_CHANNEL_LIMIT { + tracing::warn!( + community = %community_id, + limit = AUTO_JOIN_CHANNEL_LIMIT, + "auto_join channel list hit its cap; later channels are not applied" + ); + } + rows.into_iter().map(row_to_channel_record).collect() +} + /// A channel archived by the ephemeral-channel reaper. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ReapedEphemeralChannel { @@ -667,6 +716,7 @@ pub(crate) fn row_to_channel_record(row: sqlx::postgres::PgRow) -> Result Result Vec> { let tag = |parts: &[&str]| parts.iter().map(|p| (*p).to_owned()).collect::>(); let mut tags = vec![ @@ -746,6 +800,9 @@ pub fn group_metadata_tags(channel: &ChannelRecord, members: &[MemberRecord]) -> &channel.created_at.timestamp().to_string(), ])); tags.push(tag(&["join_role", join_role.as_str()])); + if channel.auto_join && open && channel.archived_at.is_none() { + tags.push(tag(&["auto_join"])); + } // Optional topic / purpose for richer client UX if let Some(topic) = channel.topic.as_deref().filter(|t| !t.is_empty()) { tags.push(tag(&["topic", topic])); @@ -789,6 +846,10 @@ pub struct ChannelUpdate { /// New unlisted flag, or `None` to leave unchanged. It decides listing, /// not admission, so it does not take the membership lock. pub unlisted: Option, + /// Whether the relay joins new community members into the channel, or + /// `None` to leave unchanged. `Some(true)` needs an open, unarchived + /// channel; `apply_channel_metadata_event` refuses it otherwise. + pub auto_join: Option, } impl ChannelUpdate { @@ -801,6 +862,7 @@ impl ChannelUpdate { && self.labels.is_none() && self.join_role.is_none() && self.unlisted.is_none() + && self.auto_join.is_none() } /// True when the update changes a setting that decides admission: @@ -1041,6 +1103,10 @@ async fn update_channel_on( set_parts.push(format!("unlisted = ${param_idx}")); param_idx += 1; } + if updates.auto_join.is_some() { + set_parts.push(format!("auto_join = ${param_idx}")); + param_idx += 1; + } if let Some(ref ttl) = updates.ttl_seconds { // Set ttl_seconds, then reset the deadline from now (or clear both). set_parts.push(format!("ttl_seconds = ${param_idx}")); @@ -1079,6 +1145,9 @@ async fn update_channel_on( if let Some(unlisted) = updates.unlisted { q = q.bind(unlisted); } + if let Some(auto_join) = updates.auto_join { + q = q.bind(auto_join); + } if let Some(ref ttl) = updates.ttl_seconds { q = q.bind(*ttl); } @@ -1276,7 +1345,7 @@ async fn set_archived_on(w: &mut ChannelSettingsWriter<'_>, archive: bool) -> Re /// unchanged. #[derive(Default)] pub struct ChannelMetadataEdit { - /// Name, description, visibility, TTL and labels. + /// Name, description, visibility, TTL, labels, `join_role` and `auto_join`. pub columns: ChannelUpdate, /// New topic, recorded as set by the event's author. pub topic: Option, @@ -1346,6 +1415,24 @@ pub(crate) async fn apply_channel_metadata_event( return Err(DbError::AccessDenied("channel is not archived".to_string())); } } + // `auto_join` only makes sense where a kind:9021 self-join would work: + // an open, unarchived channel. Decide it here, under the row lock and + // after the archive decision, on the state this event leaves behind, so + // one event may open a channel and flag it, and a concurrent archive + // cannot slip past a check made before the lock. + if edit.columns.auto_join == Some(true) { + let visibility = edit + .columns + .visibility + .as_deref() + .unwrap_or(locked.previous_visibility()); + let archived = edit.archived.unwrap_or(locked.archived()); + if visibility != ChannelVisibility::Open.as_str() || archived { + return Err(DbError::AccessDenied( + "auto_join requires an open, unarchived channel".to_string(), + )); + } + } crate::insert_mentions_in_transaction(locked.tx(), event, Some(channel_id)).await?; let ChannelMetadataEdit { @@ -1696,6 +1783,16 @@ impl Db { list_channels(&self.pool, community_id, visibility).await } + /// Lists the channels the relay joins new community members into: + /// `auto_join` set, open, unarchived and not deleted; oldest first. + #[datastore_span(name = "list_auto_join_channels", system = "postgresql")] + pub async fn list_auto_join_channels( + &self, + community_id: CommunityId, + ) -> Result> { + list_auto_join_channels(&self.pool, community_id).await + } + /// Lists channels during startup reconciliation. #[datastore_span(name = "list_channels_for_bootstrap", system = "postgresql")] pub async fn list_channels_for_bootstrap( @@ -1820,6 +1917,7 @@ mod group_metadata_tag_tests { labels: Vec::new(), join_role: join_role.into(), unlisted: false, + auto_join: false, } } @@ -1863,6 +1961,26 @@ mod group_metadata_tag_tests { assert!(tags.contains(&vec!["join_role".to_string(), "guest".to_string()])); } + /// The bare `auto_join` tag is effective-state: published only while the + /// column is set and the channel is open and unarchived. + #[test] + fn publishes_auto_join_only_while_effective() { + let has_tag = |channel: &ChannelRecord| { + group_metadata_tags(channel, &[]).contains(&vec!["auto_join".to_string()]) + }; + let mut flagged = channel("open", "member"); + assert!(!has_tag(&flagged), "unset"); + flagged.auto_join = true; + assert!(has_tag(&flagged), "open, unarchived"); + let mut archived = channel("open", "member"); + archived.auto_join = true; + archived.archived_at = Some(Utc::now()); + assert!(!has_tag(&archived), "archived"); + let mut private = channel("private", "member"); + private.auto_join = true; + assert!(!has_tag(&private), "private"); + } + /// The type `t` tag comes first among `t` tags, then the creator `P`, /// as on 39001 and 39002; kind:39000 also carries the creation time. #[test] diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index 480054fa91f..a6a2bcf01c5 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -1739,7 +1739,7 @@ async fn get_channel_tx( nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, purpose, purpose_set_by, purpose_set_at, - ttl_seconds, ttl_deadline, labels, join_role, unlisted + ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND id = $2 AND deleted_at IS NULL "#, ) diff --git a/crates/buzz-db/src/store/dm.rs b/crates/buzz-db/src/store/dm.rs index 0c86e13f050..0c1526bebca 100644 --- a/crates/buzz-db/src/store/dm.rs +++ b/crates/buzz-db/src/store/dm.rs @@ -76,7 +76,7 @@ pub async fn find_dm_by_participants( created_by, created_at, updated_at, archived_at, deleted_at, nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, - purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted + purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND participant_hash = $2 @@ -142,7 +142,7 @@ pub async fn create_dm( created_by, created_at, updated_at, archived_at, deleted_at, nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, - purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted + purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND participant_hash = $2 @@ -212,7 +212,7 @@ pub async fn create_dm( created_by, created_at, updated_at, archived_at, deleted_at, nip29_group_id, topic_required, max_members, topic, topic_set_by, topic_set_at, - purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted + purpose, purpose_set_by, purpose_set_at, labels, join_role, unlisted, auto_join FROM channels WHERE community_id = $1 AND id = $2 "#, ) @@ -535,6 +535,7 @@ fn row_to_channel_record(row: sqlx::postgres::PgRow) -> Result { labels: row.try_get("labels")?, join_role: row.try_get("join_role")?, unlisted: row.try_get("unlisted")?, + auto_join: row.try_get("auto_join")?, }) } diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index 806e740b9a4..89d71977559 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -6,7 +6,7 @@ use nostr::{EventBuilder, Keys, Kind, Tag}; use uuid::Uuid; use crate::handlers::group_state_postgres_tests::{ - community, create_channel, edit_channel, http_auth, state, + community, create_channel, edit_channel, group_state_tags, http_auth, state, }; use crate::handlers::ingest::ingest_event; @@ -350,6 +350,263 @@ async fn join_role_edit_is_applied_and_announced() { assert!(announced, "one join_role_changed system message"); } +/// `auto_join` is a community setting: only a relay owner or admin may set +/// it, on an open, unarchived stream channel, and kind:39000 advertises it +/// only while it is effective. Mutations: drop the relay-role tier → the +/// channel owner's edit is accepted (RED); drop the in-transaction guard → +/// `true` on a private or archived channel is accepted (RED); mirror the +/// column instead of the effective state in `group_metadata_tags` → the tag +/// survives the flip to private (RED). +#[tokio::test] +#[ignore = "requires Postgres"] +async fn auto_join_is_relay_admin_only_and_advertised_while_effective() { + let state = state().await; + let tenant = community(&state, "edit-auto-join").await; + let owner = Keys::generate(); + let channel_id = create_channel(&state, &tenant, &owner, &[]).await; + let notices_before = count_kind(&state, &tenant, channel_id, 40099).await; + let advertised = |tags: Vec>| tags.contains(&vec!["auto_join".to_string()]); + + // The channel owner is not a relay owner or admin: refused, nothing changes. + let refused = edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["auto_join", "true"]], + ) + .await + .expect_err("channel owner must not set auto_join"); + assert!( + refused.contains("relay owner or admin"), + "channel owner refusal: {refused}" + ); + assert!( + !state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("channel") + .auto_join + ); + assert!(!advertised( + group_state_tags(&state, &tenant, channel_id, 39000).await + )); + + // A relay admin who is not a channel member sets it. One transaction + // stores the event, the column and one `auto_join_changed` notice, and + // the republished kind:39000 advertises it. + let admin = Keys::generate(); + state + .db + .add_relay_member( + tenant.community(), + &admin.public_key().to_hex(), + "admin", + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed relay admin"); + edit_channel( + &state, + &tenant, + &admin, + channel_id, + &[&["auto_join", "true"]], + ) + .await + .expect("relay admin sets auto_join"); + assert!( + state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("channel") + .auto_join + ); + assert!(advertised( + group_state_tags(&state, &tenant, channel_id, 39000).await + )); + let notices = state + .db + .query_events(&EventQuery { + kinds: Some(vec![40099]), + channel_id: Some(channel_id), + ..EventQuery::for_community(tenant.community()) + }) + .await + .expect("query events"); + assert_eq!(notices.len(), notices_before + 1); + assert!( + notices + .iter() + .filter_map( + |stored| serde_json::from_str::(&stored.event.content).ok() + ) + .any(|content| content["type"] == "auto_join_changed" && content["auto_join"] == true), + "one auto_join_changed system message" + ); + let listed = state + .db + .list_auto_join_channels(tenant.community()) + .await + .expect("list auto_join channels"); + assert_eq!( + listed.iter().map(|c| c.id).collect::>(), + [channel_id] + ); + + // Private: the stored value stays, but it is no longer effective, so the + // tag disappears and the relay would not join anyone. + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["visibility", "private"]], + ) + .await + .expect("make private"); + assert!( + state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("channel") + .auto_join + ); + assert!(!advertised( + group_state_tags(&state, &tenant, channel_id, 39000).await + )); + assert!(state + .db + .list_auto_join_channels(tenant.community()) + .await + .expect("list auto_join channels") + .is_empty()); + + // `true` is refused on a private channel; `false` is not. Opening and + // flagging in one event is accepted: the guard sees the event's result. + // `visibility` still needs the channel owner, so that event comes from + // the owner once they are also on the relay roster: both tiers run. + // The nonce keeps this from signing the same event id as the accepted + // edit above, which the relay would drop as a duplicate. + let refused = edit_channel( + &state, + &tenant, + &admin, + channel_id, + &[&["auto_join", "true"], &["nonce", "private"]], + ) + .await + .expect_err("auto_join=true on a private channel"); + assert!( + refused.contains("open, unarchived"), + "private refusal: {refused}" + ); + edit_channel( + &state, + &tenant, + &admin, + channel_id, + &[&["auto_join", "false"]], + ) + .await + .expect("auto_join=false on a private channel"); + state + .db + .add_relay_member( + tenant.community(), + &owner.public_key().to_hex(), + "owner", + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed relay owner"); + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["visibility", "open"], &["auto_join", "true"]], + ) + .await + .expect("open and flag in one event"); + assert!(advertised( + group_state_tags(&state, &tenant, channel_id, 39000).await + )); + + // Archived and forum channels cannot be default channels. Archiving and + // flagging in one event is refused by the guard on the event's result, + // and nothing of that event applies; on a channel that is already + // archived the archive fence refuses the edit first. + let archived_id = create_channel(&state, &tenant, &owner, &[]).await; + let refused = edit_channel( + &state, + &tenant, + &owner, + archived_id, + &[&["archived", "true"], &["auto_join", "true"]], + ) + .await + .expect_err("archive and flag in one event"); + assert!( + refused.contains("open, unarchived"), + "archive-and-flag refusal: {refused}" + ); + assert!( + state + .db + .get_channel(tenant.community(), archived_id) + .await + .expect("channel") + .archived_at + .is_none(), + "a refused event archives nothing" + ); + edit_channel( + &state, + &tenant, + &owner, + archived_id, + &[&["archived", "true"]], + ) + .await + .expect("archive"); + let refused = edit_channel( + &state, + &tenant, + &admin, + archived_id, + &[&["auto_join", "true"]], + ) + .await + .expect_err("auto_join=true on an archived channel"); + assert!(refused.contains("archived"), "archived refusal: {refused}"); + let forum_id = create_channel(&state, &tenant, &owner, &[&["channel_type", "forum"]]).await; + let refused = edit_channel(&state, &tenant, &admin, forum_id, &[&["auto_join", "true"]]) + .await + .expect_err("auto_join=true on a forum"); + assert!( + refused.contains("stream channels"), + "forum refusal: {refused}" + ); + for id in [archived_id, forum_id] { + assert!( + !state + .db + .get_channel(tenant.community(), id) + .await + .expect("channel") + .auto_join + ); + } +} + /// Sign a kind:9002 that sets `archived` to `value` on `channel_id`. fn archive_event(owner: &Keys, channel_id: Uuid, value: &str) -> nostr::Event { EventBuilder::new(Kind::Custom(9002), "") diff --git a/crates/buzz-relay/src/handlers/ingest.rs b/crates/buzz-relay/src/handlers/ingest.rs index 0e44eb8c1e5..3a820f9a934 100644 --- a/crates/buzz-relay/src/handlers/ingest.rs +++ b/crates/buzz-relay/src/handlers/ingest.rs @@ -612,12 +612,13 @@ fn required_scope_for_kind(kind: u32, event: &Event) -> Result Ok(Scope::UsersWrite), KIND_NIP29_EDIT_METADATA => { - // kind:9002 scope split: archived tag → AdminChannels, else ChannelsWrite - let has_archived = event - .tags - .iter() - .any(|t| t.kind().to_string() == "archived"); - if has_archived { + // kind:9002 scope split: archived or auto_join tag → AdminChannels, + // else ChannelsWrite + let has_admin_tag = event.tags.iter().any(|t| { + let k = t.kind().to_string(); + k == "archived" || k == "auto_join" + }); + if has_admin_tag { Ok(Scope::AdminChannels) } else { Ok(Scope::ChannelsWrite) diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index add1ddcd9b2..8737a70fd35 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -642,6 +642,7 @@ pub async fn validate_admin_event( "t", "join_role", "unlisted", + "auto_join", ]; let has_recognized = event .tags @@ -649,7 +650,7 @@ pub async fn validate_admin_event( .any(|t| RECOGNIZED_TAGS.contains(&t.kind().to_string().as_str())); if !has_recognized { return Err(anyhow::anyhow!( - "kind:9002 must include at least one metadata tag (name, about, archived, topic, purpose, visibility, ttl, t, join_role, unlisted)" + "kind:9002 must include at least one metadata tag (name, about, archived, topic, purpose, visibility, ttl, t, join_role, unlisted, auto_join)" )); } @@ -724,6 +725,47 @@ pub async fn validate_admin_event( } } + // `auto_join` is a community setting, not a channel one: the + // relay joins every new community member into the channel. So it + // takes the actor's *relay* role (owner or admin), not a channel + // role, and the actor need not be a channel member. Only a + // stream channel can be a default channel; the open-and- + // unarchived requirement is decided in the write, under the + // channel row lock, on the state the event leaves behind. + let mut sets_auto_join = false; + for t in event.tags.iter() { + if t.kind().to_string() == "auto_join" { + match t.content() { + Some("true") | Some("false") => sets_auto_join = true, + Some(v) => { + return Err(anyhow::anyhow!( + "invalid auto_join value: {v} (must be \"true\" or \"false\")" + )); + } + None => { + return Err(anyhow::anyhow!("auto_join tag must have a value")); + } + } + } + } + if sets_auto_join { + let relay_role = state + .db + .get_relay_member(tenant.community(), &event.pubkey.to_hex()) + .await? + .map(|m| m.role); + if !matches!(relay_role.as_deref(), Some("owner" | "admin")) { + return Err(anyhow::anyhow!( + "actor not authorized: auto_join requires a relay owner or admin" + )); + } + if channel.channel_type != buzz_db::channel::ChannelType::Stream.as_str() { + return Err(anyhow::anyhow!( + "auto_join is only available on stream channels" + )); + } + } + // Validate ttl values before storage. Empty string clears the TTL // (channel becomes permanent); any other value must parse as a // positive integer number of seconds. A bare tag with no value is @@ -755,8 +797,13 @@ pub async fn validate_admin_event( } // name/about/archived/visibility/ttl/labels/join_role/unlisted - // require owner/admin; - // topic/purpose allow any member who may post. + // require owner/admin; topic/purpose allow any member who may + // post; auto_join was decided above by relay role, so an event + // carrying only that tag needs neither. + let has_member_tag = event.tags.iter().any(|t| { + let k = t.kind().to_string(); + k == "topic" || k == "purpose" + }); let has_privileged_tag = event.tags.iter().any(|t| { let k = t.kind().to_string(); k == "name" @@ -792,7 +839,7 @@ pub async fn validate_admin_event( )) } } - } else { + } else if has_member_tag { // topic/purpose: any member whose role may post (never a // guest). let is_member = state @@ -819,6 +866,8 @@ pub async fn validate_admin_event( } else { Err(anyhow::anyhow!("only members can change topic or purpose")) } + } else { + Ok(()) } } 9005 => { @@ -2051,6 +2100,15 @@ pub(crate) async fn persist_edit_metadata( val.parse().map_err(|e: String| anyhow::anyhow!(e))?; edit.columns.join_role = Some(join_role.as_str().to_string()); } + "auto_join" => { + // Validated during authorization; fail closed on a resend + // that somehow carries another value. + edit.columns.auto_join = Some(match val { + "true" => true, + "false" => false, + _ => return Err(anyhow::anyhow!("invalid auto_join value: {val}")), + }); + } "ttl" => { // Empty string clears the TTL (permanent); otherwise it is a // positive integer of seconds, validated during authorization. @@ -2113,6 +2171,11 @@ pub(crate) async fn persist_edit_metadata( "type": "unlisted_changed", "actor": actor_hex, "unlisted": unlisted })); } + if let Some(auto_join) = edit.columns.auto_join { + notices.push(serde_json::json!({ + "type": "auto_join_changed", "actor": actor_hex, "auto_join": auto_join + })); + } if let Some(ttl_seconds) = edit.columns.ttl_seconds { notices.push(serde_json::json!({ "type": "ttl_changed", "actor": actor_hex, "ttl_seconds": ttl_seconds diff --git a/crates/buzz-sdk/src/builders.rs b/crates/buzz-sdk/src/builders.rs index 0791ebaa98d..d0fbf02decc 100644 --- a/crates/buzz-sdk/src/builders.rs +++ b/crates/buzz-sdk/src/builders.rs @@ -824,12 +824,17 @@ pub fn build_leave(channel_id: Uuid) -> Result { } /// Build a NIP-29 edit-metadata event for -/// name/about/visibility/ttl/join_role/unlisted (kind 9002). +/// name/about/visibility/ttl/join_role/unlisted/auto_join (kind 9002). /// /// `ttl`: outer `None` leaves it unchanged; `Some(Some(secs))` sets the /// ephemeral timeout; `Some(None)` clears it (emits `["ttl", ""]`). /// `unlisted`: `None` leaves it unchanged; `Some(b)` emits /// `["unlisted", "true"|"false"]`. +/// +/// `auto_join`: whether the relay joins new community members into the +/// channel. Relay owner/admin only; accepted only on an open, unarchived +/// stream channel. +#[allow(clippy::too_many_arguments)] pub fn build_update_channel( channel_id: Uuid, name: Option<&str>, @@ -838,6 +843,7 @@ pub fn build_update_channel( ttl: Option>, join_role: Option, unlisted: Option, + auto_join: Option, ) -> Result { if name.is_none() && about.is_none() @@ -845,9 +851,10 @@ pub fn build_update_channel( && ttl.is_none() && join_role.is_none() && unlisted.is_none() + && auto_join.is_none() { return Err(SdkError::InvalidTag( - "at least one of name, about, visibility, ttl, join_role, or unlisted must be provided" + "at least one of name, about, visibility, ttl, join_role, unlisted, or auto_join must be provided" .into(), )); } @@ -891,6 +898,12 @@ pub fn build_update_channel( if let Some(unlisted) = unlisted { tags.push(tag(&["unlisted", if unlisted { "true" } else { "false" }])?); } + if let Some(auto_join) = auto_join { + tags.push(tag(&[ + "auto_join", + if auto_join { "true" } else { "false" }, + ])?); + } Ok(EventBuilder::new(Kind::Custom(9002), "").tags(tags)) } @@ -3668,6 +3681,7 @@ mod tests { None, None, None, + None, ) .unwrap(), ); @@ -3687,6 +3701,7 @@ mod tests { None, None, None, + None, ) .unwrap(), ); @@ -3696,7 +3711,7 @@ mod tests { #[test] fn update_channel_rejects_hash_only_name() { assert!(matches!( - build_update_channel(uuid(), Some(" ### "), None, None, None, None, None), + build_update_channel(uuid(), Some(" ### "), None, None, None, None, None, None), Err(SdkError::InvalidTag(_)) )); } @@ -3713,6 +3728,7 @@ mod tests { Some(Some(3600)), None, None, + None, ) .unwrap(), ); @@ -3724,7 +3740,9 @@ mod tests { #[test] fn update_channel_clears_ttl() { let cid = uuid(); - let ev = sign(build_update_channel(cid, None, None, None, Some(None), None, None).unwrap()); + let ev = sign( + build_update_channel(cid, None, None, None, Some(None), None, None, None).unwrap(), + ); assert!(has_tag(&ev, "ttl", "")); } @@ -3732,7 +3750,7 @@ mod tests { fn update_channel_invalid_visibility_rejected() { let cid = uuid(); assert!(matches!( - build_update_channel(cid, None, None, Some("secret"), None, None, None), + build_update_channel(cid, None, None, Some("secret"), None, None, None, None), Err(SdkError::InvalidTag(_)) )); } @@ -3750,6 +3768,7 @@ mod tests { None, Some(ChannelJoinRole::Guest), None, + None, ) .unwrap(), ); @@ -3763,12 +3782,13 @@ mod tests { None, Some(ChannelJoinRole::Member), None, + None, ) .unwrap(), ); assert!(has_tag(&off, "join_role", "member")); let other = - sign(build_update_channel(cid, Some("n"), None, None, None, None, None).unwrap()); + sign(build_update_channel(cid, Some("n"), None, None, None, None, None, None).unwrap()); assert!(!other .tags .iter() @@ -3779,26 +3799,47 @@ mod tests { fn update_channel_unlisted_is_always_explicit() { let cid = uuid(); let on = sign( - build_update_channel(cid, None, None, Some("open"), None, None, Some(true)).unwrap(), + build_update_channel(cid, None, None, Some("open"), None, None, Some(true), None) + .unwrap(), ); assert!(has_tag(&on, "visibility", "open")); assert!(has_tag(&on, "unlisted", "true")); - let off = - sign(build_update_channel(cid, None, None, None, None, None, Some(false)).unwrap()); + let off = sign( + build_update_channel(cid, None, None, None, None, None, Some(false), None).unwrap(), + ); assert!(has_tag(&off, "unlisted", "false")); let other = - sign(build_update_channel(cid, Some("n"), None, None, None, None, None).unwrap()); + sign(build_update_channel(cid, Some("n"), None, None, None, None, None, None).unwrap()); assert!(!other .tags .iter() .any(|t| t.kind().to_string() == "unlisted")); } + #[test] + fn update_channel_auto_join_is_always_explicit() { + let cid = uuid(); + let on = sign( + build_update_channel(cid, None, None, None, None, None, None, Some(true)).unwrap(), + ); + assert!(has_tag(&on, "auto_join", "true")); + let off = sign( + build_update_channel(cid, None, None, None, None, None, None, Some(false)).unwrap(), + ); + assert!(has_tag(&off, "auto_join", "false")); + let other = + sign(build_update_channel(cid, Some("n"), None, None, None, None, None, None).unwrap()); + assert!(!other + .tags + .iter() + .any(|t| t.kind().to_string() == "auto_join")); + } + #[test] fn update_channel_no_fields_rejected() { let cid = uuid(); assert!(matches!( - build_update_channel(cid, None, None, None, None, None, None), + build_update_channel(cid, None, None, None, None, None, None, None), Err(SdkError::InvalidTag(_)) )); } diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index 1cdd80435ea..ab1c4772afd 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -3132,20 +3132,31 @@ async fn set_join_role_ws( channel_id: &str, signer: &Keys, join_role: &str, +) -> (bool, String) { + set_metadata_ws(url, channel_id, signer, "join_role", join_role).await +} + +/// Submit a kind:9002 with one `[tag, value]` setting and return the OK frame. +async fn set_metadata_ws( + url: &str, + channel_id: &str, + signer: &Keys, + tag: &str, + value: &str, ) -> (bool, String) { // The nonce keeps a repeated setting within one second from signing the // same event id as the earlier one, which the relay drops as a duplicate. let event = EventBuilder::new(Kind::Custom(9002), "") .tags([ Tag::parse(["h", channel_id]).unwrap(), - Tag::parse(["join_role", join_role]).unwrap(), + Tag::parse([tag, value]).unwrap(), Tag::parse(["nonce", &uuid::Uuid::new_v4().to_string()]).unwrap(), ]) .sign_with_keys(signer) .unwrap(); let mut client = BuzzTestClient::connect(url, signer) .await - .expect("connect join_role actor"); + .expect("connect kind:9002 actor"); let ok = client.send_event(event).await.expect("send kind:9002"); client.disconnect().await.ok(); (ok.accepted, ok.message) @@ -3713,6 +3724,31 @@ async fn test_nip29_announce_channel_wire() { !tags.contains(&vec!["closed".to_string()]), "open channels accept kind:9021 joins: {tags:?}" ); + assert!( + !tags.contains(&vec!["auto_join".to_string()]), + "no channel is a default channel until a relay admin asks: {tags:?}" + ); + + // `auto_join` is a community setting: the channel owner, who is not on + // the relay roster, cannot set it; a relay owner can, and kind:39000 + // then advertises the bare tag. + let (accepted, msg) = set_metadata_ws(&url, &channel_id, &owner, "auto_join", "true").await; + assert!(!accepted, "channel owner must not set auto_join"); + assert!(msg.contains("relay owner or admin"), "{msg}"); + let relay_owner = Keys::generate(); + seed_relay_owner(&relay_owner).await; + assert_eq!( + set_metadata_ws(&url, &channel_id, &relay_owner, "auto_join", "true").await, + (true, String::new()) + ); + let tags = group_metadata_tags(&url, &owner, &channel_id).await; + assert!(tags.contains(&vec!["auto_join".to_string()]), "{tags:?}"); + assert_eq!( + set_metadata_ws(&url, &channel_id, &relay_owner, "auto_join", "false").await, + (true, String::new()) + ); + let tags = group_metadata_tags(&url, &owner, &channel_id).await; + assert!(!tags.contains(&vec!["auto_join".to_string()]), "{tags:?}"); // A non-member of an open announce channel cannot write, and cannot // self-add as a member. diff --git a/docs/nips/NIP-CL.md b/docs/nips/NIP-CL.md index 9b988292803..27ed3a8a400 100644 --- a/docs/nips/NIP-CL.md +++ b/docs/nips/NIP-CL.md @@ -64,6 +64,7 @@ Example kind:39000 (signing fields and unrelated tags omitted): | later `t` | 39000–39002 | Channel labels. | | `P` | 39000–39002 | Public key that signed the kind:9007 that created the channel. | | `created_at` | 39000 | Time the channel was created, in unix seconds, as a decimal string. | +| `auto_join` | 39000 | Present (bare, no value) while the relay joins new community members into this channel: the channel's `auto_join` setting is on and it is open and unarchived. | **Channel type.** Clients MUST read the channel type from the first `t` tag. Later `t` tags are labels. diff --git a/migrations/0076_channel_auto_join.sql b/migrations/0076_channel_auto_join.sql new file mode 100644 index 00000000000..43554b3369f --- /dev/null +++ b/migrations/0076_channel_auto_join.sql @@ -0,0 +1,11 @@ +-- Whether the relay joins every new community member into this channel. +-- Only a relay owner or admin sets it, with kind:9002 ["auto_join", "true"| +-- "false"]; it is accepted only on an open, unarchived stream channel. +-- Published in kind:39000 as a bare `auto_join` tag while it is effective +-- (set, open and unarchived), so what the event says is what the relay does. +-- The constant default fills existing rows without a table rewrite: no +-- channel is a default channel until someone asks for it. +SET LOCAL lock_timeout = '5s'; + +ALTER TABLE channels + ADD COLUMN auto_join BOOLEAN NOT NULL DEFAULT false; diff --git a/schema/tables/public/channels.sql b/schema/tables/public/channels.sql index 102a441e8fb..859bd3d3d3d 100644 --- a/schema/tables/public/channels.sql +++ b/schema/tables/public/channels.sql @@ -41,6 +41,7 @@ CREATE TABLE channels ( -- An unlisted open channel is readable and joinable by id but left out of -- channel listings for non-members. Matters only while the channel is open. unlisted BOOLEAN NOT NULL DEFAULT FALSE, + auto_join BOOLEAN NOT NULL DEFAULT false, PRIMARY KEY (community_id, id), CONSTRAINT chk_channels_id_not_nil CHECK (id <> '00000000-0000-0000-0000-000000000000'::uuid) ); From 9e446e30171ea9d3a79bc875b0bbc8757e5ec1dc Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 26 Oct 2026 12:50:42 +1100 Subject: [PATCH 02/13] feat(relay): auto-join new community members to default channels Join every new community member into the channels a relay owner or admin flagged with `channels.auto_join` (R1, d65bf983a), at the moment they become a member. This is the behaviour half (R2) of default channels: clients see the flagged channels in their sidebar, unread state and push with no client work, because the relay runs the kind:9021 body for them. - `join_channel_as_self` is kind:9021's body extracted from `handle_join_request` and returns `Joined | AlreadyMember`: open and unarchived checks, skip-if-member, then `put_member(.., None, None)` so the channel's `join_role` applies, cache invalidation, the `member_joined` kind:40099, 39000/39002 and the kind:44100. The handler keeps its exact external behaviour; the archived guard is new in the shared body because only the 9021 validator held it before. - `auto_join_default_channels` iterates `Db::list_auto_join_channels` (effective channels only) and calls the body per channel, best effort: a failure is logged and counted (`buzz_auto_join_failures_total`), a join counted (`buzz_auto_join_channels_total`), and the caller never fails, because the relay membership has already committed. No retry after that commit: re-running at any later point would resurrect kind:9022 leaves, and the kind:39000 `auto_join` tag is the record a client reconciles against. This is an accepted Review-Proven Rule 1 deviation, documented in NOSTR.md. - Called inline right after `publish_nip43_member_added` at the three user-facing join sites: the v2 invite claim `Joined` arm, the v1 claim `was_inserted` branch, and kind:9030 when the roster row was inserted. Never at login or reconnect. Inline means the kind:39002 roster already lists the member when the claim response returns. - `buzz-admin add-member` is unchanged except for help text noting it writes the roster row only; it has no `AppState` to sign 39002/44100. Tests: `claim_auto_joins_default_channels_once` (Postgres lane) covers a v2 claim against a flagged, an archived-after-flagging, a private-after-flagging and an unflagged channel: membership and 39002 in the flagged one only, one 44100 and one `member_joined` 40099, then a kind:9022 leave and re-claim that stays `already_member` without re-adding; the v1 token path; and the shared body's archived refusal directly. `adding_a_member_auto_joins_default_channels` is the first Postgres test for kind:9030 add, with the same leave-then-repeat step. Deleting the call at any of the three sites fails the matching test (verified by mutation). `test_invite_claim_auto_joins_default_channel` is the e2e smoke, run against a local relay: the claimer receives the kind:44100 and is on the kind:39002 roster. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- NOSTR.md | 12 +- crates/buzz-admin/src/main.rs | 4 +- crates/buzz-relay/src/api/invites.rs | 284 +++++++++++++++++- crates/buzz-relay/src/handlers/relay_admin.rs | 106 ++++++- .../buzz-relay/src/handlers/side_effects.rs | 101 ++++++- crates/buzz-test-client/tests/e2e_relay.rs | 96 ++++++ 6 files changed, 571 insertions(+), 32 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 6ec8856a4a8..5537e72e152 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -72,7 +72,7 @@ PGPASSWORD=buzz_dev psql -h localhost -U buzz -d buzz -c \ | **NIP-10 threads** | ✅ | WS-submitted replies with `["e","","","reply"]` tags create `thread_metadata` atomically. Visible in REST thread queries. Unknown parents rejected. | | **NIP-17 DMs (gift wrap)** | ✅ | kind:1059 accepted with ephemeral signing keys. Stored community-globally (`channel_id=None` inside the connected community). Delivered via `#p`-filtered subscriptions. Not indexed in search. | | **DM discovery** | ✅ | DM creation emits kind:39000 (with `hidden` tag) + kind:44100 membership notifications. NIP-29 clients discover DMs via standard group discovery flow. | -| **Join request (kind:9021)** | ✅ | Open channels only. Adds the joiner with the channel's `join_role` (`member` by default), emits system message + group discovery events + kind:44100 membership notification. Private channels rejected at ingest. | +| **Join request (kind:9021)** | ✅ | Open channels only. Adds the joiner with the channel's `join_role` (`member` by default), emits system message + group discovery events + kind:44100 membership notification. Private channels rejected at ingest. The same body joins new community members to [default channels](#default-channels). | | **Edits (kind:40003)** | ⚠️ | Works on the wire but Buzz-only — no standard NIP-29 client renders these | | **Rich content (kind:40002)** | ⚠️ | Works on the wire but Buzz-only — no standard NIP-29 client renders these | @@ -216,6 +216,16 @@ one the relay joins every new community member into. The setting is Every change is announced by one kind:40099 `auto_join_changed` system message in the channel. +When someone becomes a community member (an invite claim or a kind:9030 add) +the relay joins them to every effective default channel as a self-join: the +body of kind:9021, so the channel's `join_role` applies and the channel gets +its `member_joined` kind:40099, kind:39002 and kind:44100. It does this once, +at that moment, and never again on login or reconnect, so a later kind:9022 +leave stands. A channel that fails to join is logged and counted +(`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the +record a client reconciles against. `buzz-admin add-member` writes the +roster row only and does not join default channels. + ### Group Discovery The relay emits NIP-29 group state events when channels are created, updated, or membership changes. diff --git a/crates/buzz-admin/src/main.rs b/crates/buzz-admin/src/main.rs index 13e7f6df82e..b286037ac1d 100644 --- a/crates/buzz-admin/src/main.rs +++ b/crates/buzz-admin/src/main.rs @@ -56,7 +56,9 @@ enum Command { /// /// Accepts a bech32 npub or 64-char hex pubkey. After inserting the DB row, /// publishes a kind:13534 membership roster via Redis so live clients see - /// the updated list immediately. + /// the updated list immediately. Does not join the member to the + /// community's default (`auto_join`) channels: the relay does that on an + /// invite claim or a kind:9030 add, which this command bypasses. AddMember { /// Nostr public key — bech32 npub or 64-char hex. #[arg(long)] diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index e184a35c41b..dc3e48b3b6b 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -24,7 +24,9 @@ use axum::{ use serde::Deserialize; use serde_json::Value; -use crate::handlers::side_effects::{fan_out_nip43_membership_list, publish_nip43_member_added}; +use crate::handlers::side_effects::{ + auto_join_default_channels, fan_out_nip43_membership_list, publish_nip43_member_added, +}; use crate::nip_fi_http::admit_nip_fi_http_on_state; use buzz_core::invite::{ hash_v2_code, validate_v2_code, DEFAULT_INVITE_TTL_SECS, MAX_INVITE_TTL_SECS, MAX_INVITE_USES, @@ -496,6 +498,7 @@ pub async fn claim_invite( "failed to publish NIP-43 member-added delta after v2 claim: {e}" ); } + auto_join_default_channels(&tenant, &state, &claimer_hex).await; Ok(Json(serde_json::json!({ "status": "joined", "community_id": tenant.community().to_string(), @@ -589,6 +592,7 @@ pub async fn claim_invite( if let Err(e) = publish_nip43_member_added(&tenant, &state, &claimer_hex).await { tracing::warn!("failed to publish NIP-43 member-added delta after claim: {e}"); } + auto_join_default_channels(&tenant, &state, &claimer_hex).await; } Ok(Json(serde_json::json!({ @@ -1205,6 +1209,257 @@ mod postgres_tests { ); } + /// How many kind:44100 notifications name `member_hex` across the + /// community, one per channel the relay joined them to. + async fn added_notification_count( + state: &AppState, + community: buzz_core::CommunityId, + member_hex: &str, + ) -> i64 { + state + .db + .count_events(&buzz_db::EventQuery { + kinds: Some(vec![buzz_core::kind::KIND_MEMBER_ADDED_NOTIFICATION as i32]), + p_tag_hex: Some(member_hex.to_string()), + global_only: true, + ..buzz_db::EventQuery::for_community(community) + }) + .await + .expect("count 44100") + } + + /// How many `member_joined` kind:40099 notices in `channel_id` target + /// `member_hex`. + async fn member_joined_notice_count( + state: &AppState, + community: buzz_core::CommunityId, + channel_id: Uuid, + member_hex: &str, + ) -> usize { + state + .db + .query_events(&buzz_db::EventQuery { + kinds: Some(vec![40099]), + channel_id: Some(channel_id), + ..buzz_db::EventQuery::for_community(community) + }) + .await + .expect("query 40099") + .iter() + .filter_map(|stored| serde_json::from_str::(&stored.event.content).ok()) + .filter(|content| content["type"] == "member_joined" && content["target"] == member_hex) + .count() + } + + /// The channel roles the relay holds for `keys` in `channels`, in order. + async fn channel_roles( + state: &AppState, + community: buzz_core::CommunityId, + channels: &[Uuid], + keys: &Keys, + ) -> Vec> { + let mut roles = Vec::new(); + for channel_id in channels { + roles.push( + state + .db + .get_member_role(community, *channel_id, &keys.public_key().to_bytes()) + .await + .expect("member role"), + ); + } + roles + } + + /// Claiming an invite joins the new member to the community's effective + /// default channels, once. Four channels flagged by the relay owner: + /// one open, one archived after flagging, one made private after + /// flagging, plus an unflagged control. A v2 claim joins the open one + /// only, with its kind:39002 roster, one kind:44100 and one + /// `member_joined` notice; a kind:9022 leave followed by a re-claim is + /// `already_member` and does not re-add. The v1 token path joins the same + /// way. The archived guard the shared join body owns is bound directly. + /// Mutation: delete `auto_join_default_channels` at either claim site → + /// RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn claim_auto_joins_default_channels_once() { + use crate::handlers::group_state_postgres_tests::{ + create_channel, edit_channel, group_state_tags, http_auth, + }; + use crate::handlers::ingest::ingest_event; + use crate::handlers::side_effects::join_channel_as_self; + + let host = format!("invites-auto-join-{}.example", Uuid::new_v4().simple()); + let owner = Keys::generate(); + let state = invite_test_state(&host) + .await + .expect("requires reachable Postgres and relay test state"); + let community = state + .db + .lookup_community_by_host(&host) + .await + .expect("lookup") + .expect("community exists") + .id; + let tenant = buzz_core::TenantContext::resolved(community, &host); + state + .db + .add_relay_member( + community, + &owner.public_key().to_hex(), + "owner", + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed owner"); + + let flagged = create_channel(&state, &tenant, &owner, &[]).await; + let archived = create_channel(&state, &tenant, &owner, &[]).await; + let private = create_channel(&state, &tenant, &owner, &[]).await; + let control = create_channel(&state, &tenant, &owner, &[]).await; + for channel_id in [flagged, archived, private] { + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["auto_join", "true"]], + ) + .await + .expect("relay owner flags channel"); + } + edit_channel(&state, &tenant, &owner, archived, &[&["archived", "true"]]) + .await + .expect("archive after flagging"); + edit_channel( + &state, + &tenant, + &owner, + private, + &[&["visibility", "private"]], + ) + .await + .expect("make private after flagging"); + let channels = [flagged, archived, private, control]; + + let joiner = Keys::generate(); + let joiner_hex = joiner.public_key().to_hex(); + let archived_refusal = + join_channel_as_self(&tenant, &state, archived, &joiner.public_key().to_bytes()) + .await + .expect_err("self-join of an archived channel"); + assert!( + archived_refusal.to_string().contains("archived"), + "{archived_refusal}" + ); + assert_eq!( + added_notification_count(&state, community, &joiner_hex).await, + 0 + ); + + let code = mint_code( + state.clone(), + &host, + &owner, + serde_json::json!({ "max_uses": 1 }), + ) + .await; + let claim_body = serde_json::json!({ "code": code }).to_string(); + let response = post_json( + state.clone(), + &host, + "/api/invites/claim", + &joiner, + claim_body.clone(), + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(read_json(response).await["status"], "joined"); + assert_eq!( + channel_roles(&state, community, &channels, &joiner).await, + [Some("member".to_string()), None, None, None], + "joined to the effective default channel only" + ); + assert!( + group_state_tags(&state, &tenant, flagged, 39002) + .await + .iter() + .any(|tag| tag[0] == "p" && tag[1] == joiner_hex), + "kind:39002 of the default channel lists the new member" + ); + assert_eq!( + added_notification_count(&state, community, &joiner_hex).await, + 1 + ); + assert_eq!( + member_joined_notice_count(&state, community, flagged, &joiner_hex).await, + 1 + ); + + // Leaving is respected: a re-claim is a no-op, not a re-add. + let leave = EventBuilder::new(Kind::Custom(9022), "") + .tags([Tag::parse(["h", &flagged.to_string()]).unwrap()]) + .sign_with_keys(&joiner) + .expect("sign 9022"); + let left = ingest_event(&state, &tenant, leave, http_auth(&joiner)) + .await + .expect("ingest 9022"); + assert!(left.accepted, "{:?}", left.message); + assert_eq!( + channel_roles(&state, community, &[flagged], &joiner).await, + [None] + ); + let response = post_json( + state.clone(), + &host, + "/api/invites/claim", + &joiner, + claim_body, + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(read_json(response).await["status"], "already_member"); + assert_eq!( + channel_roles(&state, community, &channels, &joiner).await, + [None, None, None, None], + "a re-claim must not resurrect a kind:9022 leave" + ); + assert_eq!( + added_notification_count(&state, community, &joiner_hex).await, + 1 + ); + assert_eq!( + member_joined_notice_count(&state, community, flagged, &joiner_hex).await, + 1 + ); + + // The v1 HMAC token path joins the same way. + let v1_joiner = Keys::generate(); + let v1_joiner_hex = v1_joiner.public_key().to_hex(); + let v1_code = forge_v1_invite_code(&state, community, unix_now() + 3600); + let response = post_json( + state.clone(), + &host, + "/api/invites/claim", + &v1_joiner, + serde_json::json!({ "code": v1_code }).to_string(), + ) + .await; + assert_eq!(response.status(), StatusCode::OK); + assert_eq!(read_json(response).await["status"], "joined"); + assert_eq!( + channel_roles(&state, community, &channels, &v1_joiner).await, + [Some("member".to_string()), None, None, None] + ); + assert_eq!( + added_notification_count(&state, community, &v1_joiner_hex).await, + 1 + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn owner_mints_and_new_pubkey_claims() { @@ -1618,24 +1873,27 @@ mod postgres_tests { assert_eq!(response.status(), StatusCode::FORBIDDEN); } - /// Forge an already-expired invite payload signed with the relay's derived - /// invite key. `mint_invite` clamps ttl to 60s minimum, so the only way to - /// produce an expired code is to build the payload by hand at the token - /// layer. - fn forge_expired_invite_code( + fn unix_now() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("time") + .as_secs() + } + + /// Forge a v1 HMAC invite payload expiring at `expires_at`, signed with + /// the relay's derived invite key. `mint_invite` only mints v2 codes now + /// and clamps ttl to 60s minimum, so v1 and expired codes are built by + /// hand at the token layer. + fn forge_v1_invite_code( state: &AppState, community: buzz_core::CommunityId, - seconds_ago: u64, + expires_at: u64, ) -> String { let key = derive_invite_key(&state.relay_keypair); - let now = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .expect("time") - .as_secs(); let payload = InvitePayload { c: community.as_uuid().to_string(), r: "member".to_string(), - e: now.saturating_sub(seconds_ago), + e: expires_at, n: "test-nonce".to_string(), }; let payload_bytes = serde_json::to_vec(&payload).expect("payload serializes"); @@ -1667,7 +1925,7 @@ mod postgres_tests { .await .expect("lookup") .expect("community exists"); - let code = forge_expired_invite_code(&state, community.id, 10); + let code = forge_v1_invite_code(&state, community.id, unix_now() - 10); let body = serde_json::json!({ "code": code }).to_string(); let response = post_json(state.clone(), &host, "/api/invites/claim", &joiner, body).await; diff --git a/crates/buzz-relay/src/handlers/relay_admin.rs b/crates/buzz-relay/src/handlers/relay_admin.rs index 16648595f8b..9856e3fd92b 100644 --- a/crates/buzz-relay/src/handlers/relay_admin.rs +++ b/crates/buzz-relay/src/handlers/relay_admin.rs @@ -25,7 +25,8 @@ use buzz_core::tenant::TenantContext; use buzz_db::relay_members::RemoveResult; use crate::handlers::side_effects::{ - fan_out_nip43_membership_list, publish_nip43_member_added, publish_nip43_member_removed, + auto_join_default_channels, fan_out_nip43_membership_list, publish_nip43_member_added, + publish_nip43_member_removed, }; use crate::state::AppState; @@ -362,6 +363,7 @@ async fn execute_relay_admin_command( if let Err(e) = publish_nip43_member_added(tenant, state, &target_hex).await { warn!(error = %e, "failed to publish NIP-43 member added event"); } + auto_join_default_channels(tenant, state, &target_hex).await; } fan_out_nip43_membership_list(tenant, state, write.snapshot).await; } @@ -988,6 +990,108 @@ mod postgres_tests { assert!(!bystander_socket.cancellation_token().is_cancelled()); } + /// A kind:9030 add joins the new relay member to the community's + /// effective default channels, once: the flagged open channel gains them + /// as a member with one kind:44100; the unflagged channel does not. A + /// kind:9022 leave followed by a repeat kind:9030 (an idempotent no-op on + /// the roster) does not re-add them. Mutation: delete the + /// `auto_join_default_channels` call → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn adding_a_member_auto_joins_default_channels() { + use crate::handlers::group_state_postgres_tests::{ + create_channel, edit_channel, http_auth, + }; + use crate::handlers::ingest::ingest_event; + + let host = format!("add-auto-join-{}.example", uuid::Uuid::new_v4().simple()); + let (state, tenant) = workspace_profile_test_state(&host, true).await; + let (owner, target) = (Keys::generate(), Keys::generate()); + state + .db + .add_relay_member( + tenant.community(), + &owner.public_key().to_hex(), + "owner", + None, + &nostr::Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed owner"); + let flagged = create_channel(&state, &tenant, &owner, &[]).await; + let control = create_channel(&state, &tenant, &owner, &[]).await; + edit_channel(&state, &tenant, &owner, flagged, &[&["auto_join", "true"]]) + .await + .expect("relay owner flags channel"); + + let roles = |keys: &Keys| { + let state = state.clone(); + let community = tenant.community(); + let pubkey = keys.public_key().to_bytes(); + async move { + let mut roles = Vec::new(); + for channel_id in [flagged, control] { + roles.push( + state + .db + .get_member_role(community, channel_id, &pubkey) + .await + .expect("member role"), + ); + } + roles + } + }; + let added_notifications = || { + let state = state.clone(); + let community = tenant.community(); + let target_hex = target.public_key().to_hex(); + async move { + state + .db + .count_events(&buzz_db::EventQuery { + kinds: Some(vec![buzz_core::kind::KIND_MEMBER_ADDED_NOTIFICATION as i32]), + p_tag_hex: Some(target_hex), + global_only: true, + ..buzz_db::EventQuery::for_community(community) + }) + .await + .expect("count 44100") + } + }; + let add = || { + EventBuilder::new(Kind::Custom(RELAY_ADMIN_ADD_MEMBER as u16), "") + .tags(vec![Tag::public_key(target.public_key())]) + .sign_with_keys(&owner) + .expect("sign 9030") + }; + + handle_relay_admin_event(&tenant, &state, &add()) + .await + .expect("owner adds member"); + assert_eq!(roles(&target).await, [Some("member".to_string()), None]); + assert_eq!(added_notifications().await, 1); + + let leave = EventBuilder::new(Kind::Custom(9022), "") + .tags([Tag::parse(["h", &flagged.to_string()]).expect("h tag")]) + .sign_with_keys(&target) + .expect("sign 9022"); + let left = ingest_event(&state, &tenant, leave, http_auth(&target)) + .await + .expect("ingest 9022"); + assert!(left.accepted, "{:?}", left.message); + handle_relay_admin_event(&tenant, &state, &add()) + .await + .expect("repeat add is a roster no-op"); + assert_eq!( + roles(&target).await, + [None, None], + "a repeat kind:9030 must not resurrect a kind:9022 leave" + ); + assert_eq!(added_notifications().await, 1); + } + /// A pod-local root socket registered and bound to `keys`, as root AUTH /// leaves it. fn bound_root_socket( diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 8737a70fd35..ef797505f6a 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2679,15 +2679,25 @@ async fn handle_delete_group( Ok(()) } -async fn handle_join_request( +/// What [`join_channel_as_self`] did for the member. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum SelfJoinOutcome { + /// The member was added and every join side effect was emitted. + Joined, + /// The member was already active in the channel; nothing changed. + AlreadyMember, +} + +/// Join `pubkey` to an open, unarchived channel as a self-join: no inviter, +/// the channel's `join_role`, and the kind:40099, 39000/39002 and 44100 +/// side effects of a kind:9021. Shared by the kind:9021 handler and the +/// default-channel auto-join on new community memberships. +pub(crate) async fn join_channel_as_self( tenant: &TenantContext, - event: &Event, state: &Arc, -) -> anyhow::Result<()> { - let channel_id = - extract_h_tag_channel(event).ok_or_else(|| anyhow::anyhow!("missing h tag"))?; - let actor_bytes = event.pubkey.to_bytes().to_vec(); - + channel_id: Uuid, + pubkey: &[u8], +) -> anyhow::Result { // Only open channels allow self-join via kind:9021. let channel = state .db @@ -2699,15 +2709,17 @@ async fn handle_join_request( "channel is private — request an invitation" )); } + if channel.archived_at.is_some() { + return Err(anyhow::anyhow!("channel is archived")); + } // Skip if already an active member — prevents duplicate join notifications. // Fail closed on DB errors rather than falling through to add_member. if state - .is_member_cached(tenant.community(), channel_id, &actor_bytes) + .is_member_cached(tenant.community(), channel_id, pubkey) .await? { - info!(channel = %channel_id, "kind:9021 join — already a member, skipping"); - return Ok(()); + return Ok(SelfJoinOutcome::AlreadyMember); } // A self-join gets the channel's `join_role`, read by `put_member` under @@ -2715,11 +2727,11 @@ async fn handle_join_request( // channel, where a member must then make them a member. state .db - .put_member(tenant.community(), channel_id, &actor_bytes, None, None) + .put_member(tenant.community(), channel_id, pubkey, None, None) .await?; - state.invalidate_membership(tenant, channel_id, &actor_bytes); + state.invalidate_membership(tenant, channel_id, pubkey); - let actor_hex = hex::encode(&actor_bytes); + let actor_hex = hex::encode(pubkey); emit_system_message( tenant, state, @@ -2741,8 +2753,8 @@ async fn handle_join_request( tenant, state, channel_id, - &actor_bytes, - &actor_bytes, + pubkey, + pubkey, buzz_core::kind::KIND_MEMBER_ADDED_NOTIFICATION, ) .await @@ -2750,7 +2762,64 @@ async fn handle_join_request( warn!("membership notification for join failed: {e}"); } - info!(channel = %channel_id, "kind:9021 join processed"); + Ok(SelfJoinOutcome::Joined) +} + +/// Join a new community member to every effective `auto_join` channel, as +/// kind:9021 self-joins. Runs inline at the membership write sites (invite +/// claim, kind:9030) and nowhere else, so a later kind:9022 leave stands. +/// +/// Best effort: the relay membership has already committed, so a channel +/// that fails is logged and counted, never retried, and never fails the +/// caller. The `auto_join` tag on kind:39000 is the durable record a client +/// reconciles against. +pub(crate) async fn auto_join_default_channels( + tenant: &TenantContext, + state: &Arc, + pubkey_hex: &str, +) { + let Ok(pubkey) = hex::decode(pubkey_hex) else { + warn!(member = %pubkey_hex, "auto-join skipped: member pubkey is not hex"); + return; + }; + let channels = match state.db.list_auto_join_channels(tenant.community()).await { + Ok(channels) => channels, + Err(e) => { + warn!(member = %pubkey_hex, error = %e, "auto-join skipped: listing default channels failed"); + metrics::counter!("buzz_auto_join_failures_total").increment(1); + return; + } + }; + for channel in channels { + match join_channel_as_self(tenant, state, channel.id, &pubkey).await { + Ok(SelfJoinOutcome::Joined) => { + metrics::counter!("buzz_auto_join_channels_total").increment(1); + info!(channel = %channel.id, member = %pubkey_hex, "auto-joined default channel"); + } + Ok(SelfJoinOutcome::AlreadyMember) => {} + Err(e) => { + metrics::counter!("buzz_auto_join_failures_total").increment(1); + warn!(channel = %channel.id, member = %pubkey_hex, error = %e, "auto-join of default channel failed"); + } + } + } +} + +async fn handle_join_request( + tenant: &TenantContext, + event: &Event, + state: &Arc, +) -> anyhow::Result<()> { + let channel_id = + extract_h_tag_channel(event).ok_or_else(|| anyhow::anyhow!("missing h tag"))?; + match join_channel_as_self(tenant, state, channel_id, &event.pubkey.to_bytes()).await? { + SelfJoinOutcome::AlreadyMember => { + info!(channel = %channel_id, "kind:9021 join — already a member, skipping"); + } + SelfJoinOutcome::Joined => { + info!(channel = %channel_id, "kind:9021 join processed"); + } + } Ok(()) } diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index ab1c4772afd..39d7d74a6db 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -302,6 +302,102 @@ async fn test_invite_mint_and_claim_admits_new_pubkey() { ); } +/// Claiming an invite joins the new member to the community's default +/// channels: the relay owner flags one, the claimer receives its kind:44100 +/// and is on its kind:39002 roster, and a repeat claim is a no-op. +#[tokio::test] +#[ignore] +async fn test_invite_claim_auto_joins_default_channel() { + let url = relay_url(); + let relay_owner = Keys::generate(); + let joiner = Keys::generate(); + let joiner_hex = joiner.public_key().to_hex(); + seed_relay_owner(&relay_owner).await; + let channel_id = create_test_channel(&relay_owner).await; + assert_eq!( + set_metadata_ws(&url, &channel_id, &relay_owner, "auto_join", "true").await, + (true, String::new()) + ); + + let mut joiner_client = BuzzTestClient::connect(&url, &joiner) + .await + .expect("connect joiner"); + let sid = sub_id("auto-join-notif"); + let filter = Filter::new() + .kind(Kind::Custom(44100)) + .custom_tag(SingleLetterTag::lowercase(Alphabet::P), joiner_hex.as_str()) + .since(nostr::Timestamp::now() - 5u64); + joiner_client + .subscribe(&sid, vec![filter]) + .await + .expect("subscribe to membership notifications"); + joiner_client + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("EOSE for membership sub"); + + let mint_response = invite_post(&relay_owner, "/api/invites", "{}").await; + assert_eq!(mint_response.status(), reqwest::StatusCode::OK); + let mint_json: serde_json::Value = mint_response.json().await.expect("mint JSON"); + let code = mint_json + .get("code") + .and_then(serde_json::Value::as_str) + .expect("mint response includes code"); + let claim_body = serde_json::json!({ "code": code }).to_string(); + let claim_response = invite_post(&joiner, "/api/invites/claim", &claim_body).await; + assert_eq!(claim_response.status(), reqwest::StatusCode::OK); + let claim_json: serde_json::Value = claim_response.json().await.expect("claim JSON"); + assert_eq!( + claim_json.get("status").and_then(serde_json::Value::as_str), + Some("joined") + ); + + // Other tests may leave their own default channels flagged on this + // shared relay; wait for the notification about ours. + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + loop { + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + let msg = joiner_client + .recv_event(remaining) + .await + .expect("recv kind:44100 notification for the default channel"); + if let RelayMessage::Event { event, .. } = msg { + let channel = event + .tags + .iter() + .find(|tag| tag.as_slice().first().map(String::as_str) == Some("h")) + .and_then(|tag| tag.as_slice().get(1).cloned()); + if channel.as_deref() == Some(channel_id.as_str()) { + break; + } + } + } + joiner_client.disconnect().await.ok(); + assert_eq!( + member_role(&url, &relay_owner, &channel_id, &joiner_hex) + .await + .as_deref(), + Some("member"), + "the relay-signed kind:39002 roster lists the claimer" + ); + + let repeat_response = invite_post(&joiner, "/api/invites/claim", &claim_body).await; + assert_eq!(repeat_response.status(), reqwest::StatusCode::OK); + let repeat_json: serde_json::Value = repeat_response.json().await.expect("repeat claim JSON"); + assert_eq!( + repeat_json + .get("status") + .and_then(serde_json::Value::as_str), + Some("already_member") + ); + + // Leave the shared relay as found. + assert_eq!( + set_metadata_ws(&url, &channel_id, &relay_owner, "auto_join", "false").await, + (true, String::new()) + ); +} + #[tokio::test] #[ignore] async fn test_invite_claim_rejects_invalid_code() { From e145803db61f654433e145058adb30e779721e7a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 26 Oct 2026 13:14:47 +1100 Subject: [PATCH 03/13] fix(relay): close the auto_join review gaps in the channel loader and 9002 authz Address the review of d65bf983a (R1) and 848cf49e4 (R2). - `get_accessible_channels` now selects `c.auto_join`. It feeds `row_to_channel_record`, which reads that column unconditionally since R1, so every call returned `ColumnNotFound` and `legacy_channel_types_postgres_tests` was red. Every other SELECT that feeds either channel-record mapper (channel.rs, dm.rs, `get_channel_tx`) already carried the column. - The kind:9002 validator's trailing authorization arm fails closed: an event whose only recognized tags are neither privileged nor member tags is accepted only when it sets `auto_join` (decided above by relay role), and refused otherwise. Behaviour today is unchanged; a tag added to `RECOGNIZED_TAGS` but to neither list can no longer pass with no authorization. - `list_auto_join_channels` filters `channel_type = 'stream'` itself, so an out-of-band `auto_join` on a forum or DM row cannot auto-join anyone even though the 9002 validator already refuses to set it there. The cap warning now says what the `rows.len() == LIMIT` check can prove. - NOSTR.md "Default channels" states that auto-join fires only at the roster-insert sites, so a community that does not require relay membership advertises the kind:39000 tag but auto-joins nobody, and a client wanting parity there self-joins on the tag. Verified: fmt; clippy -D warnings on buzz-db and buzz-relay; the buzz-db Postgres lane (580 passed, legacy_channel_types_postgres_tests included); the relay side_effects unit tests and the auto_join / edit_metadata Postgres tests. `unarchive_of_an_active_channel_is_refused` fails with `Sqlx(PoolTimedOut)` on this machine with and without this change (reproduced on untouched HEAD); it is the known lane flake, not a regression. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- NOSTR.md | 5 ++++- crates/buzz-db/src/store/channel.rs | 13 +++++++------ crates/buzz-db/src/store/channel_members.rs | 2 +- crates/buzz-relay/src/handlers/side_effects.rs | 9 ++++++++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 5537e72e152..7b6641779f5 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -224,7 +224,10 @@ at that moment, and never again on login or reconnect, so a later kind:9022 leave stands. A channel that fails to join is logged and counted (`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the record a client reconciles against. `buzz-admin add-member` writes the -roster row only and does not join default channels. +roster row only and does not join default channels. Auto-join fires only at +those roster-insert sites, so a community that does not require relay +membership advertises the kind:39000 tag but auto-joins nobody; a client that +wants the same result there self-joins (kind:9021) on the tag. ### Group Discovery diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index b13aaa5115a..005dda072b5 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -619,9 +619,10 @@ async fn list_channels_with_operation( /// The most channels the relay joins a new community member into. pub const AUTO_JOIN_CHANNEL_LIMIT: i64 = 32; -/// Lists the channels whose `auto_join` is effective: set, open, unarchived -/// and not deleted. Oldest first, so the first default channel a member sees -/// is the one that was flagged first. Bounded by [`AUTO_JOIN_CHANNEL_LIMIT`]. +/// Lists the channels whose `auto_join` is effective: set on a stream channel +/// that is open, unarchived and not deleted. Oldest first, so the first +/// default channel a member sees is the one that was flagged first. Bounded by +/// [`AUTO_JOIN_CHANNEL_LIMIT`]. pub async fn list_auto_join_channels( pool: &PgPool, community_id: CommunityId, @@ -641,8 +642,8 @@ pub async fn list_auto_join_channels( purpose, purpose_set_by, purpose_set_at, ttl_seconds, ttl_deadline, labels, join_role, unlisted, auto_join FROM channels - WHERE community_id = $1 AND auto_join AND visibility = 'open' - AND archived_at IS NULL AND deleted_at IS NULL + WHERE community_id = $1 AND auto_join AND channel_type = 'stream' + AND visibility = 'open' AND archived_at IS NULL AND deleted_at IS NULL ORDER BY created_at ASC LIMIT $2 "#, @@ -655,7 +656,7 @@ pub async fn list_auto_join_channels( tracing::warn!( community = %community_id, limit = AUTO_JOIN_CHANNEL_LIMIT, - "auto_join channel list hit its cap; later channels are not applied" + "auto_join channel list reached its cap; any channel beyond it is not applied" ); } rows.into_iter().map(row_to_channel_record).collect() diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index a6a2bcf01c5..796031fb696 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -1837,7 +1837,7 @@ pub async fn get_accessible_channels( c.nip29_group_id, c.topic_required, c.max_members, c.topic, c.topic_set_by, c.topic_set_at, c.purpose, c.purpose_set_by, c.purpose_set_at, - c.ttl_seconds, c.ttl_deadline, c.labels, c.join_role, c.unlisted, + c.ttl_seconds, c.ttl_deadline, c.labels, c.join_role, c.unlisted, c.auto_join, (cm.channel_id IS NOT NULL) AS is_member FROM channels c LEFT JOIN channel_members cm diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index ef797505f6a..3e078c8e018 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -866,8 +866,15 @@ pub async fn validate_admin_event( } else { Err(anyhow::anyhow!("only members can change topic or purpose")) } - } else { + } else if sets_auto_join { Ok(()) + } else { + // Every recognized tag must be authorized by one of the arms + // above; a tag added to `RECOGNIZED_TAGS` but to neither list + // is refused rather than accepted with no authorization. + Err(anyhow::anyhow!( + "kind:9002 metadata tag has no authorization rule" + )) } } 9005 => { From 6ce1afd1b218fc39c51485f32067ebe9ee572d38 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 26 Oct 2026 21:00:21 +1100 Subject: [PATCH 04/13] fix(relay): join new members to every default channel, not the first 32 `list_auto_join_channels` stopped at 32 rows while kind:9002 kept accepting `auto_join=true` on any number of channels, so with 33 flagged channels the newest one carried the kind:39000 `auto_join` tag but no claim or kind:9030 add ever joined anyone to it, and nothing retried because the person was already a relay member (Codex review on #8594). Drop the cap rather than enforce it at flag time: the kind:9002 transaction holds only the shared community admission lock and the one channel's row lock, so a count-based refusal would need a new community-wide lock to be race-free, and a private or archive flip on a flagged channel would still let the stored count drift past it. The loader now returns every effective channel, oldest first; the set is bounded by the community's channels, each flagged by a relay owner or admin, and NOSTR.md says so. Test: `every_default_channel_is_listed` (Postgres lane) flags 33 channels and asserts the loader lists all of them in creation order; putting the `LIMIT` back fails it (verified by mutation). Verified: fmt; clippy -D warnings on buzz-db and buzz-relay (all targets); the four auto_join Postgres tests. `auto_join_is_relay_admin_only_...` failed once at its first query with the known `PoolTimedOut` lane flake and passed on a solo rerun. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- NOSTR.md | 5 +- crates/buzz-db/src/store/channel.rs | 21 +++----- .../handlers/edit_metadata_postgres_tests.rs | 49 +++++++++++++++++++ 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 7b6641779f5..856b6f414a2 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -219,7 +219,10 @@ message in the channel. When someone becomes a community member (an invite claim or a kind:9030 add) the relay joins them to every effective default channel as a self-join: the body of kind:9021, so the channel's `join_role` applies and the channel gets -its `member_joined` kind:40099, kind:39002 and kind:44100. It does this once, +its `member_joined` kind:40099, kind:39002 and kind:44100. There is no cap on +how many channels that is: whatever carries the kind:39000 tag is joined, and +the work a new member costs grows with the number of channels an owner or +admin flags. It does this once, at that moment, and never again on login or reconnect, so a later kind:9022 leave stands. A channel that fails to join is logged and counted (`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 005dda072b5..3487ce14c48 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -616,13 +616,15 @@ async fn list_channels_with_operation( rows.into_iter().map(row_to_channel_record).collect() } -/// The most channels the relay joins a new community member into. -pub const AUTO_JOIN_CHANNEL_LIMIT: i64 = 32; - /// Lists the channels whose `auto_join` is effective: set on a stream channel /// that is open, unarchived and not deleted. Oldest first, so the first -/// default channel a member sees is the one that was flagged first. Bounded by -/// [`AUTO_JOIN_CHANNEL_LIMIT`]. +/// default channel a member sees is the one that was flagged first. +/// +/// Every effective channel, not a page of them: kind:9002 accepts the flag on +/// any number of channels, and the kind:39000 `auto_join` tag promises the +/// relay joins new members there, so a cap here would silently break that +/// promise for the channels past it. The set is bounded by the community's +/// channels, each flagged by a relay owner or admin. pub async fn list_auto_join_channels( pool: &PgPool, community_id: CommunityId, @@ -645,20 +647,11 @@ pub async fn list_auto_join_channels( WHERE community_id = $1 AND auto_join AND channel_type = 'stream' AND visibility = 'open' AND archived_at IS NULL AND deleted_at IS NULL ORDER BY created_at ASC - LIMIT $2 "#, ) .bind(community_id.as_uuid()) - .bind(AUTO_JOIN_CHANNEL_LIMIT) .fetch_all(&mut *connection) .await?; - if rows.len() as i64 == AUTO_JOIN_CHANNEL_LIMIT { - tracing::warn!( - community = %community_id, - limit = AUTO_JOIN_CHANNEL_LIMIT, - "auto_join channel list reached its cap; any channel beyond it is not applied" - ); - } rows.into_iter().map(row_to_channel_record).collect() } diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index 89d71977559..f13a3c1a73c 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -618,6 +618,55 @@ fn archive_event(owner: &Keys, channel_id: Uuid, value: &str) -> nostr::Event { .expect("sign 9002") } +/// The loader returns every effective default channel. It used to stop at 32 +/// while kind:9002 kept accepting the flag past that, so the kind:39000 tag +/// promised a join the relay never made for the newest channels. Mutation: +/// put a `LIMIT` back on `list_auto_join_channels` → RED. +#[tokio::test] +#[ignore = "requires Postgres"] +async fn every_default_channel_is_listed() { + let state = state().await; + let tenant = community(&state, "auto-join-all").await; + let admin = Keys::generate(); + state + .db + .add_relay_member( + tenant.community(), + &admin.public_key().to_hex(), + "admin", + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed relay admin"); + + let mut flagged = Vec::new(); + for _ in 0..33 { + let channel_id = create_channel(&state, &tenant, &admin, &[]).await; + edit_channel( + &state, + &tenant, + &admin, + channel_id, + &[&["auto_join", "true"]], + ) + .await + .expect("relay admin sets auto_join"); + flagged.push(channel_id); + } + + let listed: Vec = state + .db + .list_auto_join_channels(tenant.community()) + .await + .expect("list auto_join channels") + .iter() + .map(|channel| channel.id) + .collect(); + assert_eq!(listed, flagged, "every flagged channel, oldest first"); +} + /// A resend checked after its first copy committed is a duplicate. /// Validation runs on the new archive state and must not refuse the resend; /// the write must then report it as a duplicate, with no change and no From 03651ae9bc0cb07a9adcd8dc2be9cb6ef45c8983 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:37:55 +1100 Subject: [PATCH 05/13] test(db): derive the latest migration in the 0075 invite upgrade test Rebasing onto main put this branch's `channels.auto_join` migration at 0076, after main's 0074 (user deletion fences) and 0075 (invite revocation). `migration_schema_0075_adds_invite_columns_without_rewriting_history` asserted that a full migrate ends at version 75, so it failed once 0076 existed. It now reads the latest version from the migrations directory, as `thread_window_postgres_tests` already does, so the next migration does not break it again. The renumbering itself (0074 -> 0076, with the migrator pin test resolved to main's contiguity loop at 76) was applied in the replayed "add a channel auto_join setting" commit. Every SELECT that feeds `row_to_channel_record` already selects `auto_join`, and the three auto-join call sites sit in the same arms after main's invite-revocation rewrite, so nothing else needed to change. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- .../src/store/relay_invite/upgrade_postgres_tests.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs b/crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs index fd06aa0e62f..13abbb68bfd 100644 --- a/crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs +++ b/crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs @@ -506,12 +506,21 @@ async fn apply_pgschema_to_existing_database(pool: &PgPool) { async fn migration_schema_0075_adds_invite_columns_without_rewriting_history() { let (pool, cases) = legacy_fixture().await; crate::migration::run_migrations(&pool).await.unwrap(); + let latest = sqlx::migrate::Migrator::new( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../migrations"), + ) + .await + .unwrap() + .iter() + .map(|m| m.version) + .max() + .unwrap(); assert_eq!( sqlx::query_scalar::<_, i64>("SELECT max(version) FROM _sqlx_migrations WHERE success") .fetch_one(&pool) .await .unwrap(), - 75 + latest ); assert_admission_column(&pool).await; assert_join_role_preserved(&pool, cases[0].community).await; From 4e81ffe145993937df07683401e36b6a70e5ccb7 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 16:38:34 +1100 Subject: [PATCH 06/13] fix(db): refuse new members of an archived channel under the channel row lock An auto-join could add a new community member to a default channel after it was archived (jedwards27, second review on #8594). The auto-join lists the effective default channels and `join_channel_as_self` checks the archive state before the write, but `put_members` never read `archived_at`, and nothing ordered an archive against the membership write. An archive landing between the list and the insert still created the row and emitted the kind:40099, 39000/39002 and 44100. `put_members` and `put_members_with_event` now take the membership lock, the shared TTL lock and then the channel row `FOR NO KEY UPDATE`, the order a huddle join already uses, and refuse a target who is not an active member with `AccessDenied("channel is archived")` while `archived_at` is set. An auto-join that loses the race is logged and counted as a failed channel, with no row and no side effects. Active members keep their rows; only admission is refused. The review suggested treating archive and unarchive as admission changes so they take the membership lock. This fix locks the row instead, because the row is what every archive writes: `archive_channel`, a kind:9002 `archived` tag and the TTL reaper's single UPDATE, which takes no advisory lock at all and so would still race under the membership-lock version. It also leaves every archive writer unchanged. The refusal is not scoped to self-joins: the only production callers are kind:9000 (already refused on an archived channel at ingest), kind:9021 and the auto-join. No restore or import path re-adds members through `put_members`, and no test relies on adding to an archived channel. The cost is that a membership write now holds the channel row for its transaction. Writers of that row (settings writes, huddle joins, TTL refreshes, the reaper) wait for it, and reads do not. NOSTR.md "Default channels" now says a channel archived or made private while the join runs is one of the logged, unretried failures. Tests: - `put_member_waits_for_a_concurrent_archive` (buzz-db Postgres): an uncommitted reaper-style archive makes a join wait. Once the archive commits the join is refused with no row, and re-adding a removed member is refused too. In the other order, `archive_channel` waits for an admission that holds the locks, and the admitted member stays. - `auto_join_skips_a_default_channel_archived_while_it_joins` (buzz-relay Postgres): a flagged channel is archived in a held transaction. The auto-join lists it, passes the join body's check, then waits in `put_member`. After the archive commits there is no membership row, no kind:39002 entry, no kind:44100 and no `member_joined` notice. - Mutations: dropping the row lock from `lock_channel_for_member_put` fails both tests, and so does dropping the archived refusal from `put_members_on`. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- NOSTR.md | 4 +- crates/buzz-db/src/store/channel.rs | 8 +- crates/buzz-db/src/store/channel_members.rs | 174 +++++++++++++++++- crates/buzz-relay/src/api/invites.rs | 125 +++++++++++++ .../buzz-relay/src/handlers/side_effects.rs | 8 +- 5 files changed, 305 insertions(+), 14 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 856b6f414a2..0973bba412e 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -226,7 +226,9 @@ admin flags. It does this once, at that moment, and never again on login or reconnect, so a later kind:9022 leave stands. A channel that fails to join is logged and counted (`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the -record a client reconciles against. `buzz-admin add-member` writes the +record a client reconciles against. That includes a channel archived or made +private while the join runs: the membership write decides both again under +the channel's locks, so the member is not added and nothing is announced. `buzz-admin add-member` writes the roster row only and does not join default channels. Auto-join fires only at those roster-insert sites, so a community that does not require relay membership advertises the kind:39000 tag but auto-joins nobody; a client that diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 3487ce14c48..69db951b2aa 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -988,9 +988,11 @@ impl ChannelSettingsWriter<'_> { /// 3. the channel row, `FOR NO KEY UPDATE`. /// /// Roster publication holds the membership lock and then, at commit, takes -/// the TTL lock and updates the row. A huddle join takes the membership -/// lock, then the TTL lock, then the row. A writer that took these locks in -/// any other order could wait for one of them while it waits for this one. +/// the TTL lock and updates the row. A huddle join and a `put_members` +/// membership write take the membership lock, then the TTL lock, then the +/// row; the row lock is what orders them against an archive, which does not +/// take the membership lock. A writer that took these locks in any other +/// order could wait for one of them while it waits for this one. /// /// Returns `ChannelNotFound` when the channel is missing or deleted. pub(crate) async fn lock_channel_for_settings_write<'tx>( diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index 796031fb696..ca95fb88fae 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -651,6 +651,11 @@ pub async fn add_member( /// neither a membership write nor a setting change can slip between the /// decision and the write. /// +/// An archived channel admits nobody new: a target who is not an active +/// member is refused with `AccessDenied` while `archived_at` is set. The +/// archive state is read under the channel row lock, which every archive +/// write needs too (see [`lock_channel_for_member_put`]). +/// /// There is no creator exception: channel creation installs the creator as /// owner in the same transaction, so a creator without an active row was /// removed and is admitted like anyone else. @@ -718,9 +723,10 @@ pub async fn put_members( ) .await?; - // First statement: serialize the whole decision / owner-count / upsert - // sequence against concurrent membership writes on this channel. - acquire_channel_membership_lock(tx.conn(), community_id, channel_id).await?; + // First statements: serialize the whole decision / owner-count / upsert + // sequence against concurrent membership writes and archives of this + // channel. + lock_channel_for_member_put(&mut tx, channel_id).await?; let results = put_members_on(&mut tx, channel_id, targets, invited_by).await?; tx.commit().await?; Ok(results) @@ -747,9 +753,8 @@ pub enum MemberPutEventWrite { /// transaction. If any write fails, nothing is stored, so the client can /// resend the same event and have it applied. /// -/// The membership lock comes first, as in [`put_members`] and the channel -/// settings writers, so a second copy of the event waits for the first and -/// then finds it stored. +/// The locks of [`put_members`] come first, so a second copy of the event +/// waits for the first and then finds it stored. pub(crate) async fn put_members_with_event( pool: &PgPool, push_enqueue: Option, @@ -767,7 +772,7 @@ pub(crate) async fn put_members_with_event( ) .await?; tx.set_push_enqueue(push_enqueue); - acquire_channel_membership_lock(tx.conn(), community_id, channel_id).await?; + lock_channel_for_member_put(&mut tx, channel_id).await?; // The same writes as `Db::insert_event_with_thread_metadata`. let (stored, inserted) = crate::event::insert_event_with_thread_metadata_tx( @@ -795,6 +800,23 @@ pub(crate) async fn put_members_with_event( Ok(MemberPutEventWrite::Applied { stored, results }) } +/// Take the locks of a [`put_members`] write on `tx`, in the channel lock +/// order (see `channel::lock_channel_for_settings_write`): the membership +/// lock, the shared TTL lock, then the channel row `FOR NO KEY UPDATE`. +/// +/// The row lock is what orders the write against an archive. Every archive +/// and unarchive updates that row (`archive_channel`, a kind:9002 `archived` +/// tag, the TTL reaper's single UPDATE), and none of them takes the +/// membership lock. So an archive either commits before [`put_members_on`] +/// reads `archived_at`, or waits until this write commits. +async fn lock_channel_for_member_put(tx: &mut AdmittedTx, channel_id: Uuid) -> Result<()> { + let community_id = tx.community(); + acquire_channel_membership_lock(tx.conn(), community_id, channel_id).await?; + lock_channel_ttl_shared_in_transaction(tx, channel_id).await?; + lock_channel_archived_at_in_transaction(tx, channel_id).await?; + Ok(()) +} + fn check_distinct_targets(targets: &[MemberPut<'_>]) -> Result<()> { let mut seen = std::collections::HashSet::with_capacity(targets.len()); match targets.iter().find(|t| !seen.insert(t.pubkey)) { @@ -807,7 +829,8 @@ fn check_distinct_targets(targets: &[MemberPut<'_>]) -> Result<()> { } /// The decisions and upsert of [`put_members`], on a transaction that holds -/// `channel_id`'s membership lock. The caller owns the commit. +/// `channel_id`'s [`lock_channel_for_member_put`] locks. The caller owns the +/// commit. async fn put_members_on( tx: &mut AdmittedTx, channel_id: Uuid, @@ -816,6 +839,7 @@ async fn put_members_on( ) -> Result>> { let community_id = tx.community(); let channel = get_channel_tx(tx.conn(), community_id, channel_id).await?; + let archived = channel.archived_at.is_some(); let private = channel.visibility == "private"; let join_role = buzz_core::channel::ChannelJoinRole::parse_fail_closed(&channel.join_role); @@ -853,6 +877,12 @@ async fn put_members_on( } let actor = invited_by.unwrap_or(target.pubkey); let current_role = roles.get(target.pubkey).cloned(); + if archived && current_role.is_none() { + decided.push(Err(DbError::AccessDenied( + "channel is archived".to_string(), + ))); + continue; + } let actor_role: Option = roles .get(actor) .map(|r| { @@ -3852,6 +3882,134 @@ mod postgres_tests { .expect("join_role change"); } + /// An archive and an admission are ordered by the channel row lock, and an + /// archived channel admits nobody new. A join that waits on an uncommitted + /// archive is refused once it commits, with no row, and re-adding a + /// removed member is refused too. The other order: an archive waits for + /// an admission that holds the locks. + /// The archive here is the TTL reaper's single UPDATE, which takes no + /// advisory lock. Mutations: drop the row lock from + /// `lock_channel_for_member_put`, or the archived refusal from + /// `put_members_on` → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn put_member_waits_for_a_concurrent_archive() { + let pool = setup_pool().await; + let community_id = make_test_community(&pool).await; + let community = CommunityId::from_uuid(community_id); + let owner = random_pubkey(); + let joiner = random_pubkey(); + let removed = random_pubkey(); + for pk in [&owner, &joiner, &removed] { + ensure_user(&pool, community, pk) + .await + .expect("ensure user"); + } + let channel = create_test_channel( + &pool, + community_id, + "put-member-archive-race", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner, + None, + ) + .await + .expect("create channel"); + put_member(&pool, community, channel.id, &removed, None, None) + .await + .expect("join before the archive"); + remove_member(&pool, community, channel.id, &removed, &removed) + .await + .expect("leave before the archive"); + + let mut archive = pool.begin().await.expect("begin archive"); + sqlx::query( + "UPDATE channels SET archived_at = NOW() \ + WHERE community_id = $1 AND id = $2 AND archived_at IS NULL", + ) + .bind(community_id) + .bind(channel.id) + .execute(&mut *archive) + .await + .expect("archive"); + + let join_pool = crate::test_support::named_pool("put-member-archive-race").await; + let join = tokio::spawn({ + let channel_id = channel.id; + let joiner = joiner.clone(); + async move { put_member(&join_pool, community, channel_id, &joiner, None, None).await } + }); + crate::test_support::wait_for_lock_wait(&pool, "put-member-archive-race").await; + assert!(!join.is_finished(), "the join must wait for the archive"); + archive.commit().await.expect("commit archive"); + + let err = join + .await + .expect("join task") + .expect_err("the join is decided on the archived channel"); + assert!( + matches!(&err, DbError::AccessDenied(reason) if reason == "channel is archived"), + "{err:?}" + ); + let readd = put_member(&pool, community, channel.id, &removed, None, Some(&owner)) + .await + .expect_err("an archived channel does not reactivate a removed member"); + assert!(matches!(readd, DbError::AccessDenied(_)), "{readd:?}"); + let mut roles = Vec::new(); + for pk in [&owner, &joiner, &removed] { + roles.push( + get_member_role(&pool, community, channel.id, pk) + .await + .expect("read role"), + ); + } + assert_eq!(roles, [Some("owner".to_string()), None, None]); + + // The other order: an archive waits for an admission that holds the + // locks, and the member it admitted stays a member. + crate::channel::unarchive_channel(&pool, community, channel.id) + .await + .expect("unarchive"); + let mut admission = crate::begin_community_write_transaction( + &pool, + community, + crate::observability::WriterOperation::EventWrite, + ) + .await + .expect("begin admission"); + lock_channel_for_member_put(&mut admission, channel.id) + .await + .expect("lock for the admission"); + let targets = [MemberPut { + pubkey: &joiner, + requested: None, + }]; + let admitted = put_members_on(&mut admission, channel.id, &targets, None) + .await + .expect("admission"); + assert!(admitted[0].is_ok(), "{admitted:?}"); + let archive_pool = crate::test_support::named_pool("archive-vs-admission").await; + let archive = tokio::spawn({ + let channel_id = channel.id; + async move { crate::channel::archive_channel(&archive_pool, community, channel_id).await } + }); + crate::test_support::wait_for_lock_wait(&pool, "archive-vs-admission").await; + assert!( + !archive.is_finished(), + "an archive must wait for an admission in progress" + ); + admission.commit().await.expect("commit admission"); + archive.await.expect("archive task").expect("archive"); + assert_eq!( + get_member_role(&pool, community, channel.id, &joiner) + .await + .expect("read role"), + Some("member".to_string()) + ); + } + /// A `join_role` change on an ephemeral channel and a roster publication /// take the channel's locks in one order. The roster holds the membership /// lock and refreshes the channel's TTL (TTL lock, then row) at commit. A diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index dc3e48b3b6b..7d8cc7648dd 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -1460,6 +1460,131 @@ mod postgres_tests { ); } + /// A default channel archived after the auto-join listed it is skipped: + /// no membership row, no kind:39002 roster entry, no kind:44100 and no + /// `member_joined` notice. The test holds an uncommitted archive of the + /// listed channel, so the auto-join passes the list and the shared join + /// body's open/unarchived check on the committed state and then waits in + /// `put_member`; the archive commits and the join resumes on an archived + /// channel. Mutation: drop the channel row lock or the archived refusal + /// from `put_members` → RED. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn auto_join_skips_a_default_channel_archived_while_it_joins() { + use crate::handlers::group_state_postgres_tests::{ + create_channel, edit_channel, group_state_tags, + }; + use crate::handlers::side_effects::auto_join_default_channels; + + let host = format!( + "invites-auto-join-archive-{}.example", + Uuid::new_v4().simple() + ); + let owner = Keys::generate(); + let state = invite_test_state(&host) + .await + .expect("requires reachable Postgres and relay test state"); + let community = state + .db + .lookup_community_by_host(&host) + .await + .expect("lookup") + .expect("community exists") + .id; + let tenant = buzz_core::TenantContext::resolved(community, &host); + let joiner = Keys::generate(); + let joiner_hex = joiner.public_key().to_hex(); + for (keys, role) in [(&owner, "owner"), (&joiner, "member")] { + state + .db + .add_relay_member( + community, + &keys.public_key().to_hex(), + role, + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed relay member"); + } + let starter = create_channel(&state, &tenant, &owner, &[]).await; + edit_channel(&state, &tenant, &owner, starter, &[&["auto_join", "true"]]) + .await + .expect("relay owner flags channel"); + + let pool = state.db.pool().clone(); + let mut archive = pool.begin().await.expect("begin archive"); + let archive_pid: i32 = sqlx::query_scalar("SELECT pg_backend_pid()") + .fetch_one(&mut *archive) + .await + .expect("archive backend pid"); + sqlx::query("UPDATE channels SET archived_at = NOW() WHERE community_id = $1 AND id = $2") + .bind(community.as_uuid()) + .bind(starter) + .execute(&mut *archive) + .await + .expect("archive the default channel"); + + let auto_join = tokio::spawn({ + let state = Arc::clone(&state); + let tenant = tenant.clone(); + let joiner_hex = joiner_hex.clone(); + async move { auto_join_default_channels(&tenant, &state, &joiner_hex).await } + }); + // Wait until the join is blocked behind the archive, so it has + // already listed the channel and passed the join body's check. + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + let waiting: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_stat_activity \ + WHERE $1 = ANY(pg_blocking_pids(pid)))", + ) + .bind(archive_pid) + .fetch_one(&pool) + .await + .expect("read lock waiters"); + if waiting { + break; + } + assert!( + !auto_join.is_finished(), + "the auto-join finished without waiting for the archive" + ); + assert!( + std::time::Instant::now() < deadline, + "the auto-join never waited for the archive" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + archive.commit().await.expect("commit archive"); + tokio::time::timeout(Duration::from_secs(10), auto_join) + .await + .expect("the auto-join returns after the archive commits") + .expect("auto-join task must not panic"); + + assert_eq!( + channel_roles(&state, community, &[starter], &joiner).await, + [None], + "an archived default channel must not admit the new member" + ); + assert!( + !group_state_tags(&state, &tenant, starter, 39002) + .await + .iter() + .any(|tag| tag[0] == "p" && tag[1] == joiner_hex), + "kind:39002 must not list the new member" + ); + assert_eq!( + added_notification_count(&state, community, &joiner_hex).await, + 0 + ); + assert_eq!( + member_joined_notice_count(&state, community, starter, &joiner_hex).await, + 0 + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn owner_mints_and_new_pubkey_claims() { diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 3e078c8e018..b3ccabc7e4d 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2731,7 +2731,10 @@ pub(crate) async fn join_channel_as_self( // A self-join gets the channel's `join_role`, read by `put_member` under // the membership lock: member in a normal channel, guest in an announce - // channel, where a member must then make them a member. + // channel, where a member must then make them a member. `put_member` + // also decides visibility and archive state again under its locks, so a + // channel made private or archived since the checks above is refused + // here, with no side effects. state .db .put_member(tenant.community(), channel_id, pubkey, None, None) @@ -2779,7 +2782,8 @@ pub(crate) async fn join_channel_as_self( /// Best effort: the relay membership has already committed, so a channel /// that fails is logged and counted, never retried, and never fails the /// caller. The `auto_join` tag on kind:39000 is the durable record a client -/// reconciles against. +/// reconciles against. A channel archived or made private after the list +/// was read is one such failure: `put_member` refuses it under its locks. pub(crate) async fn auto_join_default_channels( tenant: &TenantContext, state: &Arc, From 2da7cac273c69ec61c850c1829366e3c6599d873 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 17:06:26 +1100 Subject: [PATCH 07/13] test(db): cover the kind:9000 archive race and tighten the archive-race waits Address the review of af4730a54 on #8594. The lock order holds; these are the comment, coverage and doc gaps it found. - `put_member_waits_for_a_concurrent_archive` now drives `put_members_with_event` against a second uncommitted archive. The kind:9000 add waits for it, then returns `Refused` with one `AccessDenied("channel is archived")` and stores no event. It gets its own archive because a write queued behind the join's membership lock would wait whether or not its own call site took the row lock. Reverting that call site to the membership lock alone fails the test (verified by mutation): the add never waits. - The re-add refusal in the same test is pinned to `"channel is archived"`, not any `AccessDenied`. - `auto_join_skips_a_default_channel_archived_while_it_joins` waits for a `Lock` wait on the `SELECT archived_at FROM channels ... FOR NO KEY UPDATE` statement, not for any session the archive blocks. - The `CHANNEL_MEMBERSHIP_LOCK_NAMESPACE` rationale says admissions also lock the channel row (`lock_channel_for_member_put`) so an archive is ordered against them, and that only removals take just this key. - NOSTR.md "Default channels": the auto-join paragraph is rewrapped, and its dangling "It" now says "The relay". Verified: fmt; clippy -D warnings --all-targets on buzz-db and buzz-relay; the buzz-db `channel_members` Postgres tests (32 passed) and the four buzz-relay auto_join Postgres tests. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- NOSTR.md | 22 ++--- crates/buzz-db/src/store/channel_members.rs | 99 ++++++++++++++++++--- crates/buzz-relay/src/api/invites.rs | 9 +- 3 files changed, 103 insertions(+), 27 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 0973bba412e..4047e2248fe 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -222,17 +222,17 @@ body of kind:9021, so the channel's `join_role` applies and the channel gets its `member_joined` kind:40099, kind:39002 and kind:44100. There is no cap on how many channels that is: whatever carries the kind:39000 tag is joined, and the work a new member costs grows with the number of channels an owner or -admin flags. It does this once, -at that moment, and never again on login or reconnect, so a later kind:9022 -leave stands. A channel that fails to join is logged and counted -(`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the -record a client reconciles against. That includes a channel archived or made -private while the join runs: the membership write decides both again under -the channel's locks, so the member is not added and nothing is announced. `buzz-admin add-member` writes the -roster row only and does not join default channels. Auto-join fires only at -those roster-insert sites, so a community that does not require relay -membership advertises the kind:39000 tag but auto-joins nobody; a client that -wants the same result there self-joins (kind:9021) on the tag. +admin flags. The relay does this once, at that moment, and never again on +login or reconnect, so a later kind:9022 leave stands. A channel that fails +to join is logged and counted (`buzz_auto_join_failures_total`), not retried: +the kind:39000 tag is the record a client reconciles against. That includes a +channel archived or made private while the join runs: the membership write +decides both again under the channel's locks, so the member is not added and +nothing is announced. `buzz-admin add-member` writes the roster row only and +does not join default channels. Auto-join fires only at those roster-insert +sites, so a community that does not require relay membership advertises the +kind:39000 tag but auto-joins nobody; a client that wants the same result +there self-joins (kind:9021) on the tag. ### Group Discovery diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index ca95fb88fae..272c0cbf149 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -41,9 +41,11 @@ pub struct MemberRecord { /// concurrent demotions (or a demotion racing a removal) to each observe two /// owners, each pass, and together leave zero — the exact governance loss the /// guards exist to prevent. An advisory key rather than `SELECT ... FOR UPDATE` -/// on the channel row: membership is its own contention domain and must not -/// serialize against unrelated channel metadata writers (`update_channel`, -/// `set_topic`, the TTL transition). Distinct key domain from +/// on the channel row, so a removal, which takes only this key, does not +/// serialize against the row's writers that leave admission alone +/// (`set_topic`, `set_canvas`, the TTL transition). Admissions also lock the +/// row, after this key (see [`lock_channel_for_member_put`]), because the row +/// is what orders an archive against them. Distinct key domain from /// `buzz_channel_ttl:`. const CHANNEL_MEMBERSHIP_LOCK_NAMESPACE: &str = "buzz_channel_membership:"; @@ -3885,22 +3887,28 @@ mod postgres_tests { /// An archive and an admission are ordered by the channel row lock, and an /// archived channel admits nobody new. A join that waits on an uncommitted /// archive is refused once it commits, with no row, and re-adding a - /// removed member is refused too. The other order: an archive waits for - /// an admission that holds the locks. + /// removed member is refused too. A kind:9000 add that waits on an + /// uncommitted archive is refused the same way, and its event is not + /// stored. The other order: an archive waits for an admission that holds + /// the locks. /// The archive here is the TTL reaper's single UPDATE, which takes no - /// advisory lock. Mutations: drop the row lock from - /// `lock_channel_for_member_put`, or the archived refusal from - /// `put_members_on` → RED. + /// advisory lock. The join and the kind:9000 add each wait on their own + /// archive, so neither queues behind the other's membership lock. + /// Mutations: drop the row lock from `lock_channel_for_member_put`, take + /// only the membership lock in `put_members_with_event`, or drop the + /// archived refusal from `put_members_on` → RED. #[tokio::test] #[ignore = "requires Postgres"] async fn put_member_waits_for_a_concurrent_archive() { let pool = setup_pool().await; let community_id = make_test_community(&pool).await; let community = CommunityId::from_uuid(community_id); - let owner = random_pubkey(); + let owner_keys = Keys::generate(); + let owner = owner_keys.public_key().to_bytes().to_vec(); let joiner = random_pubkey(); let removed = random_pubkey(); - for pk in [&owner, &joiner, &removed] { + let invitee = random_pubkey(); + for pk in [&owner, &joiner, &removed, &invitee] { ensure_user(&pool, community, pk) .await .expect("ensure user"); @@ -3956,16 +3964,81 @@ mod postgres_tests { let readd = put_member(&pool, community, channel.id, &removed, None, Some(&owner)) .await .expect_err("an archived channel does not reactivate a removed member"); - assert!(matches!(readd, DbError::AccessDenied(_)), "{readd:?}"); + assert!( + matches!(&readd, DbError::AccessDenied(reason) if reason == "channel is archived"), + "{readd:?}" + ); + + // A kind:9000 add stores its event in the same transaction as the + // write, so it takes the same locks: it waits on an uncommitted + // archive, then is refused and stores nothing. + crate::channel::unarchive_channel(&pool, community, channel.id) + .await + .expect("unarchive before the kind:9000 race"); + let mut archive = pool.begin().await.expect("begin second archive"); + sqlx::query( + "UPDATE channels SET archived_at = NOW() \ + WHERE community_id = $1 AND id = $2 AND archived_at IS NULL", + ) + .bind(community_id) + .bind(channel.id) + .execute(&mut *archive) + .await + .expect("archive again"); + let add = nostr::EventBuilder::new(nostr::Kind::Custom(9000), "") + .tags([ + nostr::Tag::parse(["h", &channel.id.to_string()]).unwrap(), + nostr::Tag::parse(["p", &hex::encode(&invitee)]).unwrap(), + ]) + .sign_with_keys(&owner_keys) + .expect("sign 9000"); + let add_pool = crate::test_support::named_pool("put-members-event-archive-race").await; + let add_write = tokio::spawn({ + let channel_id = channel.id; + let add = add.clone(); + let invitee = invitee.clone(); + async move { + let targets = [MemberPut { + pubkey: &invitee, + requested: None, + }]; + put_members_with_event(&add_pool, None, community, channel_id, &add, &targets, None) + .await + } + }); + crate::test_support::wait_for_lock_wait(&pool, "put-members-event-archive-race").await; + assert!( + !add_write.is_finished(), + "the kind:9000 add must wait for the archive" + ); + archive.commit().await.expect("commit second archive"); + let write = add_write + .await + .expect("kind:9000 task") + .expect("kind:9000 write"); + assert!( + matches!(&write, MemberPutEventWrite::Refused(errors) + if matches!(errors.as_slice(), + [DbError::AccessDenied(reason)] if reason == "channel is archived")), + "expected an archived refusal" + ); + assert!( + crate::event::get_event_by_id(&pool, community, add.id.as_bytes()) + .await + .expect("read event") + .is_none(), + "a refused kind:9000 is not stored" + ); + let mut roles = Vec::new(); - for pk in [&owner, &joiner, &removed] { + for pk in [&owner, &joiner, &removed, &invitee] { roles.push( get_member_role(&pool, community, channel.id, pk) .await .expect("read role"), ); } - assert_eq!(roles, [Some("owner".to_string()), None, None]); + assert_eq!(roles, [Some("owner".to_string()), None, None, None]); // The other order: an archive waits for an admission that holds the // locks, and the member it admitted stays a member. diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index 7d8cc7648dd..c770448bddd 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -1532,13 +1532,16 @@ mod postgres_tests { let joiner_hex = joiner_hex.clone(); async move { auto_join_default_channels(&tenant, &state, &joiner_hex).await } }); - // Wait until the join is blocked behind the archive, so it has - // already listed the channel and passed the join body's check. + // Wait until the join is blocked on the channel row lock in + // `put_member`, behind the archive, so it has already listed the + // channel and passed the join body's check. let deadline = std::time::Instant::now() + Duration::from_secs(10); loop { let waiting: bool = sqlx::query_scalar( "SELECT EXISTS (SELECT 1 FROM pg_stat_activity \ - WHERE $1 = ANY(pg_blocking_pids(pid)))", + WHERE wait_event_type = 'Lock' \ + AND $1 = ANY(pg_blocking_pids(pid)) \ + AND query LIKE 'SELECT archived_at FROM channels %FOR NO KEY UPDATE')", ) .bind(archive_pid) .fetch_one(&pool) From e25f9dc45c4a12b9348ed5fe0a2f4b715329e7bd Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 10:27:35 +1100 Subject: [PATCH 08/13] fix(relay): guard auto_join=false, the kind:9030 add policy and claim disconnects Address wesbillman's three review threads on #8594 (PRRT_kwDORgXb2s6xrYtL, PRRT_kwDORgXb2s6xrYtO, PRRT_kwDORgXb2s6xrYtV). - kind:9002 `auto_join=false` no longer lands in a private channel (PRRT_kwDORgXb2s6xrYtL). The relay-role tier lets a relay admin who is not a channel member send an event carrying only `auto_join`, and the in-transaction guard checked only `true`, so a `false` on a private channel was stored, fanned out, recorded mentions for its `p` tags and got an `auto_join_changed` 40099. `apply_channel_metadata_event` now refuses either value, under the channel row lock, when the event leaves the channel private; `true` still also needs it unarchived. Both values get the same refusal, so the response no longer tells them apart. A relay admin takes a private channel out of the defaults before making it private or once it is open again. - The kind:9030 auto-join honours the added member's `channel_add_policy` (PRRT_kwDORgXb2s6xrYtO). `auto_join_default_channels` takes the actor of a third-party add. kind:9030 passes its sender, and the policy is checked once with `decide_channel_add_policy`, as that sender's kind:9000 would be. A refusal skips every default channel, logged and counted in `buzz_auto_join_failures_total`; the roster add stands. Invite claims pass no actor: the claimer joins for themselves. - A client disconnect no longer cuts an invite claim's auto-join short (PRRT_kwDORgXb2s6xrYtV). Both claim paths now run their post-commit follow-up (the NIP-43 membership-list fan-out, and on a join the member-added delta and the auto-join) in `finish_committed_claim`, which spawns it before the first of those awaits and awaits the `JoinHandle`. The response still waits for the joins, but dropping the handler future on a disconnect detaches the task instead of cancelling it partway. - NOSTR.md "Default channels" documents the `false` refusal on private channels, the policy check on kind:9030 and that a claimer's disconnect does not stop the joins. Tests (buzz-relay Postgres lane): - `auto_join_is_relay_admin_only_and_advertised_while_effective`: the row that accepted the relay admin's `false` on the private channel now expects the same refusal as `true`, with no kind:9002 or 40099 stored and the stored flag unchanged. - `adding_a_member_auto_joins_default_channels`: a new step adds a `nobody` agent (no default channel joined) and an `owner_only` agent owned by the adder (joined), each with its roster add standing. - `claim_follow_up_survives_a_client_disconnect` (new): for a v2 and a v1 claim, the test holds the first default channel's row lock. It waits until the auto-join is blocked in `put_member` behind that lock, then aborts the request task, which drops the handler future as a disconnect does. After it releases the lock, the claimer joins both default channels. The lock-wait loop is shared with `auto_join_skips_a_default_channel_archived_while_it_joins`. Mutations, each reverted after it failed its test: guarding only `true` in the transaction; passing no actor at kind:9030; checking the policy against the relay key instead of the adder; running the claim follow-up inline. Verified: fmt; clippy -D warnings --all-targets on buzz-db and buzz-relay; the six auto_join / edit_metadata / kind:9030 / claim Postgres tests above, plus `bounded_v2_claims_publish_side_effects_only_for_joined`, `owner_mints_and_new_pubkey_claims` and `join_policy_gate_end_to_end` for the restructured claim handler. The full Postgres lanes and the e2e suite were not run. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- NOSTR.md | 29 ++- crates/buzz-db/src/store/channel.rs | 11 +- crates/buzz-relay/src/api/invites.rs | 229 +++++++++++++++--- .../handlers/edit_metadata_postgres_tests.rs | 74 +++--- crates/buzz-relay/src/handlers/relay_admin.rs | 69 +++++- .../buzz-relay/src/handlers/side_effects.rs | 37 ++- 6 files changed, 364 insertions(+), 85 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 4047e2248fe..194ddf0ab55 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -202,10 +202,14 @@ one the relay joins every new community member into. The setting is can change it, with kind:9002 `["auto_join", "true"]` or `["auto_join", "false"]`. The actor need not be a member of the channel. A missing tag means "no change". -- It is accepted only on an open, unarchived `stream` channel. The relay - decides that in the same transaction as the other kind:9002 changes, so one - event may open a channel and flag it. Making a flagged channel private or - archiving it is allowed and leaves the stored value in place. +- It is accepted only on a `stream` channel the event leaves open, and `true` + also needs it unarchived. `false` is refused on a private channel too: the + actor need not be a member, so neither value may write into a channel they + cannot read. The relay decides this in the same transaction as the other + kind:9002 changes, so one event may open a channel and flag it. Making a + flagged channel private or archiving it is allowed and leaves the stored + value in place; to take a private channel out of the defaults, clear the + flag before making it private or once it is open again. - kind:39000 carries a bare `["auto_join"]` tag only while the setting is effective: set, open and unarchived. What the event says is what the relay does, so a client never has to combine the flag with the access flags. @@ -223,12 +227,17 @@ its `member_joined` kind:40099, kind:39002 and kind:44100. There is no cap on how many channels that is: whatever carries the kind:39000 tag is joined, and the work a new member costs grows with the number of channels an owner or admin flags. The relay does this once, at that moment, and never again on -login or reconnect, so a later kind:9022 leave stands. A channel that fails -to join is logged and counted (`buzz_auto_join_failures_total`), not retried: -the kind:39000 tag is the record a client reconciles against. That includes a -channel archived or made private while the join runs: the membership write -decides both again under the channel's locks, so the member is not added and -nothing is announced. `buzz-admin add-member` writes the roster row only and +login or reconnect, so a later kind:9022 leave stands. A kind:9030 add is +made by someone else, so the new member's `channel_add_policy` applies first, +as it would to that actor's kind:9000: if it refuses them, no default channel +is joined. The claim response waits for the joins, and a claimer who +disconnects before it arrives does not cut them short. A channel that fails +to join, or an add the member's policy refuses, is logged and counted +(`buzz_auto_join_failures_total`), not retried: the kind:39000 tag is the +record a client reconciles against. That includes a channel archived or made +private while the join runs: the membership write decides both again under +the channel's locks, so the member is not added and nothing is announced. +`buzz-admin add-member` writes the roster row only and does not join default channels. Auto-join fires only at those roster-insert sites, so a community that does not require relay membership advertises the kind:39000 tag but auto-joins nobody; a client that wants the same result diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 69db951b2aa..c25c61d287d 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -1415,15 +1415,20 @@ pub(crate) async fn apply_channel_metadata_event( // an open, unarchived channel. Decide it here, under the row lock and // after the archive decision, on the state this event leaves behind, so // one event may open a channel and flag it, and a concurrent archive - // cannot slip past a check made before the lock. - if edit.columns.auto_join == Some(true) { + // cannot slip past a check made before the lock. Either value needs the + // event to leave the channel open: the relay role that authorizes + // `auto_join` needs no channel membership, so a `false` on a private + // channel would store and fan out an event, with its mentions and + // notice, in a channel its sender cannot read. One refusal for both + // values also keeps the response from telling them apart. + if let Some(auto_join) = edit.columns.auto_join { let visibility = edit .columns .visibility .as_deref() .unwrap_or(locked.previous_visibility()); let archived = edit.archived.unwrap_or(locked.archived()); - if visibility != ChannelVisibility::Open.as_str() || archived { + if visibility != ChannelVisibility::Open.as_str() || (auto_join && archived) { return Err(DbError::AccessDenied( "auto_join requires an open, unarchived channel".to_string(), )); diff --git a/crates/buzz-relay/src/api/invites.rs b/crates/buzz-relay/src/api/invites.rs index c770448bddd..3710f5747e8 100644 --- a/crates/buzz-relay/src/api/invites.rs +++ b/crates/buzz-relay/src/api/invites.rs @@ -484,7 +484,13 @@ pub async fn claim_invite( if write.outcome.owner_link_created { state.disconnect_unowned_agent_clusterwide(&tenant, pubkey.as_bytes()); } - fan_out_nip43_membership_list(&tenant, &state, write.snapshot).await; + // The member-added delta and the auto-join only on Joined, never on + // other outcomes; the membership-list fan-out on every outcome. + let joined = matches!( + write.outcome.outcome, + buzz_db::relay_invite::ClaimOutcome::Joined { .. } + ); + finish_committed_claim(&tenant, &state, &claimer_hex, write.snapshot, joined).await; return match write.outcome.outcome { buzz_db::relay_invite::ClaimOutcome::Joined { .. } => { tracing::info!( @@ -492,13 +498,6 @@ pub async fn claim_invite( member = %claimer_hex, "relay member added via v2 invite" ); - // NIP-43 side effects only on Joined, never on other outcomes. - if let Err(e) = publish_nip43_member_added(&tenant, &state, &claimer_hex).await { - tracing::warn!( - "failed to publish NIP-43 member-added delta after v2 claim: {e}" - ); - } - auto_join_default_channels(&tenant, &state, &claimer_hex).await; Ok(Json(serde_json::json!({ "status": "joined", "community_id": tenant.community().to_string(), @@ -581,7 +580,7 @@ pub async fn claim_invite( state.disconnect_unowned_agent_clusterwide(&tenant, pubkey.as_bytes()); } let was_inserted = write.outcome.inserted; - fan_out_nip43_membership_list(&tenant, &state, write.snapshot).await; + finish_committed_claim(&tenant, &state, &claimer_hex, write.snapshot, was_inserted).await; if was_inserted { tracing::info!( @@ -589,10 +588,6 @@ pub async fn claim_invite( member = %claimer_hex, "relay member added via invite" ); - if let Err(e) = publish_nip43_member_added(&tenant, &state, &claimer_hex).await { - tracing::warn!("failed to publish NIP-43 member-added delta after claim: {e}"); - } - auto_join_default_channels(&tenant, &state, &claimer_hex).await; } Ok(Json(serde_json::json!({ @@ -603,6 +598,42 @@ pub async fn claim_invite( }))) } +/// The follow-up of a claim whose roster write committed: the NIP-43 +/// membership-list fan-out and, when the claim added the member (`joined`), +/// the member-added delta and the default-channel auto-join. +/// +/// It runs on its own task, which the handler awaits, so the response still +/// waits for the joins. A client that disconnects first makes the server +/// drop the handler's future: that detaches the task rather than cancelling +/// it partway, which nothing would retry, because a re-claim is +/// `already_member`. +async fn finish_committed_claim( + tenant: &buzz_core::TenantContext, + state: &Arc, + claimer_hex: &str, + snapshot: Option, + joined: bool, +) { + let follow_up = tokio::spawn({ + let (tenant, state, claimer_hex) = + (tenant.clone(), Arc::clone(state), claimer_hex.to_owned()); + async move { + fan_out_nip43_membership_list(&tenant, &state, snapshot).await; + if !joined { + return; + } + if let Err(e) = publish_nip43_member_added(&tenant, &state, &claimer_hex).await { + tracing::warn!("failed to publish NIP-43 member-added delta after claim: {e}"); + } + // A claimer joins for themselves: no third-party add policy. + auto_join_default_channels(&tenant, &state, &claimer_hex, None).await; + } + }); + if let Err(e) = follow_up.await { + tracing::error!(member = %claimer_hex, error = %e, "invite claim follow-up task failed"); + } +} + /// Fixed-window rate limit on claim attempts, keyed by community and claimer /// pubkey so traffic for one tenant cannot consume another tenant's allowance. /// @@ -1271,6 +1302,40 @@ mod postgres_tests { roles } + /// Wait until a session blocked by `blocker_pid` waits for the channel + /// row lock `put_member` takes, failing if `task` finishes first. + async fn wait_for_put_member_behind( + pool: &sqlx::PgPool, + blocker_pid: i32, + task: &tokio::task::JoinHandle, + ) { + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + let waiting: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_stat_activity \ + WHERE wait_event_type = 'Lock' \ + AND $1 = ANY(pg_blocking_pids(pid)) \ + AND query LIKE 'SELECT archived_at FROM channels %FOR NO KEY UPDATE')", + ) + .bind(blocker_pid) + .fetch_one(pool) + .await + .expect("read lock waiters"); + if waiting { + return; + } + assert!( + !task.is_finished(), + "the join finished without waiting for the held channel row" + ); + assert!( + std::time::Instant::now() < deadline, + "the join never waited for the held channel row" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + /// Claiming an invite joins the new member to the community's effective /// default channels, once. Four channels flagged by the relay owner: /// one open, one archived after flagging, one made private after @@ -1530,36 +1595,12 @@ mod postgres_tests { let state = Arc::clone(&state); let tenant = tenant.clone(); let joiner_hex = joiner_hex.clone(); - async move { auto_join_default_channels(&tenant, &state, &joiner_hex).await } + async move { auto_join_default_channels(&tenant, &state, &joiner_hex, None).await } }); // Wait until the join is blocked on the channel row lock in // `put_member`, behind the archive, so it has already listed the // channel and passed the join body's check. - let deadline = std::time::Instant::now() + Duration::from_secs(10); - loop { - let waiting: bool = sqlx::query_scalar( - "SELECT EXISTS (SELECT 1 FROM pg_stat_activity \ - WHERE wait_event_type = 'Lock' \ - AND $1 = ANY(pg_blocking_pids(pid)) \ - AND query LIKE 'SELECT archived_at FROM channels %FOR NO KEY UPDATE')", - ) - .bind(archive_pid) - .fetch_one(&pool) - .await - .expect("read lock waiters"); - if waiting { - break; - } - assert!( - !auto_join.is_finished(), - "the auto-join finished without waiting for the archive" - ); - assert!( - std::time::Instant::now() < deadline, - "the auto-join never waited for the archive" - ); - tokio::time::sleep(Duration::from_millis(10)).await; - } + wait_for_put_member_behind(&pool, archive_pid, &auto_join).await; archive.commit().await.expect("commit archive"); tokio::time::timeout(Duration::from_secs(10), auto_join) .await @@ -1588,6 +1629,116 @@ mod postgres_tests { ); } + /// A client that disconnects mid-claim does not cut the auto-join short. + /// The server drops the handler's future on a disconnect; the test does + /// the same by aborting the request while the auto-join waits in + /// `put_member` behind a held lock on the first default channel's row. + /// Once the lock is released, the claimer still joins both default + /// channels, for a v2 and a v1 claim. Mutation: run the follow-up inline + /// in the handler again → RED (the claimer joins no channel). + #[tokio::test] + #[ignore = "requires Postgres"] + async fn claim_follow_up_survives_a_client_disconnect() { + use crate::handlers::group_state_postgres_tests::{create_channel, edit_channel}; + + let host = format!( + "invites-auto-join-disconnect-{}.example", + Uuid::new_v4().simple() + ); + let owner = Keys::generate(); + let state = invite_test_state(&host) + .await + .expect("requires reachable Postgres and relay test state"); + let community = state + .db + .lookup_community_by_host(&host) + .await + .expect("lookup") + .expect("community exists") + .id; + let tenant = buzz_core::TenantContext::resolved(community, &host); + state + .db + .add_relay_member( + community, + &owner.public_key().to_hex(), + "owner", + None, + &Keys::generate(), + ) + .await + .map(|write| write.outcome) + .expect("seed owner"); + let first = create_channel(&state, &tenant, &owner, &[]).await; + let second = create_channel(&state, &tenant, &owner, &[]).await; + for channel_id in [first, second] { + edit_channel( + &state, + &tenant, + &owner, + channel_id, + &[&["auto_join", "true"]], + ) + .await + .expect("relay owner flags channel"); + } + + let v2_code = mint_code( + state.clone(), + &host, + &owner, + serde_json::json!({ "max_uses": 1 }), + ) + .await; + let v1_code = forge_v1_invite_code(&state, community, unix_now() + 3600); + let pool = state.db.pool().clone(); + for code in [v2_code, v1_code] { + let joiner = Keys::generate(); + let joiner_hex = joiner.public_key().to_hex(); + let mut hold = pool.begin().await.expect("begin hold"); + let hold_pid: i32 = sqlx::query_scalar("SELECT pg_backend_pid()") + .fetch_one(&mut *hold) + .await + .expect("hold backend pid"); + sqlx::query("SELECT 1 FROM channels WHERE community_id = $1 AND id = $2 FOR UPDATE") + .bind(community.as_uuid()) + .bind(first) + .execute(&mut *hold) + .await + .expect("hold the first default channel's row"); + + let claim = tokio::spawn({ + let (state, host, joiner) = (state.clone(), host.clone(), joiner.clone()); + let body = serde_json::json!({ "code": code }).to_string(); + async move { post_json(state, &host, "/api/invites/claim", &joiner, body).await } + }); + wait_for_put_member_behind(&pool, hold_pid, &claim).await; + claim.abort(); + assert!( + claim + .await + .expect_err("the claim was still waiting") + .is_cancelled(), + "the claim request was dropped" + ); + hold.rollback().await.expect("release the held row"); + + let deadline = std::time::Instant::now() + Duration::from_secs(10); + while added_notification_count(&state, community, &joiner_hex).await < 2 { + assert!( + std::time::Instant::now() < deadline, + "the auto-join stopped with the dropped claim: {:?}", + channel_roles(&state, community, &[first, second], &joiner).await + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + assert_eq!( + channel_roles(&state, community, &[first, second], &joiner).await, + [Some("member".to_string()), Some("member".to_string())] + ); + } + } + #[tokio::test] #[ignore = "requires Postgres"] async fn owner_mints_and_new_pubkey_claims() { diff --git a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs index f13a3c1a73c..54283d5f86c 100644 --- a/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/edit_metadata_postgres_tests.rs @@ -354,9 +354,10 @@ async fn join_role_edit_is_applied_and_announced() { /// it, on an open, unarchived stream channel, and kind:39000 advertises it /// only while it is effective. Mutations: drop the relay-role tier → the /// channel owner's edit is accepted (RED); drop the in-transaction guard → -/// `true` on a private or archived channel is accepted (RED); mirror the -/// column instead of the effective state in `group_metadata_tags` → the tag -/// survives the flip to private (RED). +/// `true` on a private or archived channel is accepted (RED); guard only +/// `true` again → the relay admin's `false` is stored in the private +/// channel (RED); mirror the column instead of the effective state in +/// `group_metadata_tags` → the tag survives the flip to private (RED). #[tokio::test] #[ignore = "requires Postgres"] async fn auto_join_is_relay_admin_only_and_advertised_while_effective() { @@ -487,34 +488,51 @@ async fn auto_join_is_relay_admin_only_and_advertised_while_effective() { .expect("list auto_join channels") .is_empty()); - // `true` is refused on a private channel; `false` is not. Opening and - // flagging in one event is accepted: the guard sees the event's result. - // `visibility` still needs the channel owner, so that event comes from - // the owner once they are also on the relay roster: both tiers run. - // The nonce keeps this from signing the same event id as the accepted - // edit above, which the relay would drop as a duplicate. - let refused = edit_channel( - &state, - &tenant, - &admin, - channel_id, + // Both values are refused on a private channel, the same way, and + // nothing of either event is stored: the relay admin is not a member, + // so storing even a `false` would write into a channel they cannot + // read. Opening and flagging in one event is accepted: the guard sees + // the event's result. `visibility` still needs the channel owner, so + // that event comes from the owner once they are also on the relay + // roster: both tiers run. The nonce keeps the `true` from signing the + // same event id as the accepted edit above, which the relay would drop + // as a duplicate. + let (events_before, notices_before) = ( + count_kind(&state, &tenant, channel_id, 9002).await, + count_kind(&state, &tenant, channel_id, 40099).await, + ); + let refused_edits: [&[&[&str]]; 2] = [ &[&["auto_join", "true"], &["nonce", "private"]], - ) - .await - .expect_err("auto_join=true on a private channel"); + &[&["auto_join", "false"]], + ]; + for tags in refused_edits { + let refused = edit_channel(&state, &tenant, &admin, channel_id, tags) + .await + .expect_err("auto_join on a private channel"); + assert!( + refused.contains("open, unarchived"), + "private refusal of {tags:?}: {refused}" + ); + } + assert_eq!( + count_kind(&state, &tenant, channel_id, 9002).await, + events_before, + "a refused auto_join event is not stored" + ); + assert_eq!( + count_kind(&state, &tenant, channel_id, 40099).await, + notices_before, + "a refused auto_join event announces nothing" + ); assert!( - refused.contains("open, unarchived"), - "private refusal: {refused}" + state + .db + .get_channel(tenant.community(), channel_id) + .await + .expect("channel") + .auto_join, + "the refused `false` leaves the stored value in place" ); - edit_channel( - &state, - &tenant, - &admin, - channel_id, - &[&["auto_join", "false"]], - ) - .await - .expect("auto_join=false on a private channel"); state .db .add_relay_member( diff --git a/crates/buzz-relay/src/handlers/relay_admin.rs b/crates/buzz-relay/src/handlers/relay_admin.rs index 9856e3fd92b..83c9138e1fd 100644 --- a/crates/buzz-relay/src/handlers/relay_admin.rs +++ b/crates/buzz-relay/src/handlers/relay_admin.rs @@ -363,7 +363,15 @@ async fn execute_relay_admin_command( if let Err(e) = publish_nip43_member_added(tenant, state, &target_hex).await { warn!(error = %e, "failed to publish NIP-43 member added event"); } - auto_join_default_channels(tenant, state, &target_hex).await; + // The sender adds someone else, so the target's + // `channel_add_policy` decides whether they are auto-joined. + auto_join_default_channels( + tenant, + state, + &target_hex, + Some(&event.pubkey.to_bytes()), + ) + .await; } fan_out_nip43_membership_list(tenant, state, write.snapshot).await; } @@ -994,8 +1002,12 @@ mod postgres_tests { /// effective default channels, once: the flagged open channel gains them /// as a member with one kind:44100; the unflagged channel does not. A /// kind:9022 leave followed by a repeat kind:9030 (an idempotent no-op on - /// the roster) does not re-add them. Mutation: delete the - /// `auto_join_default_channels` call → RED. + /// the roster) does not re-add them. An added agent's `channel_add_policy` + /// holds against the adder. Mutations: delete the + /// `auto_join_default_channels` call → RED; pass no actor, or drop the + /// policy check → the `nobody` agent is joined (RED); check the policy + /// against anyone but the adder → the owned `owner_only` agent is not + /// (RED). #[tokio::test] #[ignore = "requires Postgres"] async fn adding_a_member_auto_joins_default_channels() { @@ -1090,6 +1102,57 @@ mod postgres_tests { "a repeat kind:9030 must not resurrect a kind:9022 leave" ); assert_eq!(added_notifications().await, 1); + + // The adder acts for someone else, so the added agent's + // `channel_add_policy` decides, as for the same actor's kind:9000: + // `nobody` refuses every default channel; `owner_only` admits only + // the agent's owner. + let (refusing, owned) = (Keys::generate(), Keys::generate()); + for keys in [&owner, &refusing, &owned] { + state + .db + .ensure_user(tenant.community(), &keys.public_key().to_bytes()) + .await + .expect("user row"); + } + for (agent, policy) in [(&refusing, "nobody"), (&owned, "owner_only")] { + state + .db + .set_channel_add_policy(tenant.community(), &agent.public_key().to_bytes(), policy) + .await + .expect("agent policy"); + } + state + .db + .set_agent_owner( + tenant.community(), + &owned.public_key().to_bytes(), + &owner.public_key().to_bytes(), + ) + .await + .expect("agent owner"); + for (agent, expected) in [ + (&refusing, [None, None]), + (&owned, [Some("member".to_string()), None]), + ] { + let add = EventBuilder::new(Kind::Custom(RELAY_ADMIN_ADD_MEMBER as u16), "") + .tags(vec![Tag::public_key(agent.public_key())]) + .sign_with_keys(&owner) + .expect("sign 9030"); + handle_relay_admin_event(&tenant, &state, &add) + .await + .expect("owner adds agent"); + assert!( + state + .db + .get_relay_member(tenant.community(), &agent.public_key().to_hex()) + .await + .expect("relay member") + .is_some(), + "the roster add stands either way" + ); + assert_eq!(roles(agent).await, expected); + } } /// A pod-local root socket registered and bound to `keys`, as root AUTH diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index b3ccabc7e4d..5ff8bda6c0c 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -730,8 +730,9 @@ pub async fn validate_admin_event( // takes the actor's *relay* role (owner or admin), not a channel // role, and the actor need not be a channel member. Only a // stream channel can be a default channel; the open-and- - // unarchived requirement is decided in the write, under the - // channel row lock, on the state the event leaves behind. + // unarchived requirement (just open, for `false`) is decided in + // the write, under the channel row lock, on the state the event + // leaves behind. let mut sets_auto_join = false; for t in event.tags.iter() { if t.kind().to_string() == "auto_join" { @@ -2784,10 +2785,16 @@ pub(crate) async fn join_channel_as_self( /// caller. The `auto_join` tag on kind:39000 is the durable record a client /// reconciles against. A channel archived or made private after the list /// was read is one such failure: `put_member` refuses it under its locks. +/// +/// `added_by` is the actor of a third-party add (kind:9030), `None` when the +/// member joined by themselves (an invite claim). A third-party add first +/// asks the member's `channel_add_policy`, once, as a kind:9000 from the same +/// actor would; a refusal skips every default channel, logged and counted. pub(crate) async fn auto_join_default_channels( tenant: &TenantContext, state: &Arc, pubkey_hex: &str, + added_by: Option<&[u8]>, ) { let Ok(pubkey) = hex::decode(pubkey_hex) else { warn!(member = %pubkey_hex, "auto-join skipped: member pubkey is not hex"); @@ -2801,6 +2808,32 @@ pub(crate) async fn auto_join_default_channels( return; } }; + if channels.is_empty() { + return; + } + if let Some(actor) = added_by.filter(|actor| *actor != pubkey.as_slice()) { + match state + .db + .get_agent_channel_policy(tenant.community(), &pubkey) + .await + { + Ok(None) => {} + Ok(Some((policy, owner))) => { + if let Err(denied) = + channel_authz::decide_channel_add_policy(&policy, owner.as_deref(), actor) + { + warn!(member = %pubkey_hex, error = %denied, "auto-join skipped: the member's channel_add_policy refuses the actor"); + metrics::counter!("buzz_auto_join_failures_total").increment(1); + return; + } + } + Err(e) => { + warn!(member = %pubkey_hex, error = %e, "auto-join skipped: reading the member's channel_add_policy failed"); + metrics::counter!("buzz_auto_join_failures_total").increment(1); + return; + } + } + } for channel in channels { match join_channel_as_self(tenant, state, channel.id, &pubkey).await { Ok(SelfJoinOutcome::Joined) => { From d237cd98cf01ad61bf13f86f734c3cf124b91d78 Mon Sep 17 00:00:00 2001 From: tornquist Date: Fri, 30 Oct 2026 23:25:16 +0000 Subject: [PATCH 09/13] Point migration test at auto_join migration 0079 The merge renumbered the auto_join migration to 0079, but the consolidated-schema test still read it from index 75. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: tornquist --- crates/buzz-db/src/runtime/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 8bf7d7f021b..f059537ff0c 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -780,7 +780,7 @@ mod postgres_tests { for (index, migration) in migrations.iter().enumerate() { assert_eq!(migration.version, (index + 1) as i64); } - assert!(migrations[75] + assert!(migrations[78] .sql .as_str() .contains("ADD COLUMN auto_join BOOLEAN NOT NULL DEFAULT false")); From 9cab1e9ecaa5e88f9e3711754d0f124839aa8ea7 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Sun, 1 Nov 2026 08:12:27 -0500 Subject: [PATCH 10/13] Renumber the auto_join migration to 0080 main took 0079 for nip_fi_verified_profiles, so the auto_join migration moves to 0080 and the consolidated-schema test now expects 80 migrations with auto_join at index 79. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-db/src/runtime/migration.rs | 4 ++-- ...{0079_channel_auto_join.sql => 0080_channel_auto_join.sql} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename migrations/{0079_channel_auto_join.sql => 0080_channel_auto_join.sql} (100%) diff --git a/crates/buzz-db/src/runtime/migration.rs b/crates/buzz-db/src/runtime/migration.rs index 9a2053d0264..c78a99eab5b 100644 --- a/crates/buzz-db/src/runtime/migration.rs +++ b/crates/buzz-db/src/runtime/migration.rs @@ -777,11 +777,11 @@ mod postgres_tests { let mut migrations: Vec<_> = MIGRATOR.iter().collect(); migrations.sort_by_key(|migration| migration.version); - assert_eq!(migrations.len(), 79); + assert_eq!(migrations.len(), 80); for (index, migration) in migrations.iter().enumerate() { assert_eq!(migration.version, (index + 1) as i64); } - assert!(migrations[78] + assert!(migrations[79] .sql .as_str() .contains("ADD COLUMN auto_join BOOLEAN NOT NULL DEFAULT false")); diff --git a/migrations/0079_channel_auto_join.sql b/migrations/0080_channel_auto_join.sql similarity index 100% rename from migrations/0079_channel_auto_join.sql rename to migrations/0080_channel_auto_join.sql From 41e109324e763c57766f1ac2801840453be4629d Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Sun, 1 Nov 2026 08:18:12 -0500 Subject: [PATCH 11/13] feat(relay): join people to default channels on admission without a roster A community that does not require relay membership has no invite claim or kind:9030 to run auto-join at, so #8594 alone joined nobody there. On such a community the relay now joins a person, after the AUTH OK, to every effective auto_join channel they have never had a membership row in. The channel's own membership history is the once-only record: a kind:9022 leave or kind:9001 removal keeps the row, so the person is not joined back, and a new device or relay pod finds it and does nothing. put_first_member checks the row again under the put_member locks, so racing admissions join once. A channel flagged later is joined on the next admission, and a failed membership write leaves no row, so the next admission retries it. Agents (an admitted NIP-OA, NIP-FI token or stored owner) are skipped. The kind:9021 side effects move into announce_self_join so both paths emit the same member_joined, 39000/39002 and 44100. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- NOSTR.md | 23 ++- crates/buzz-db/src/store/channel.rs | 50 ++++++ crates/buzz-db/src/store/channel_members.rs | 158 ++++++++++++++++++ crates/buzz-relay/src/handlers/auth.rs | 153 ++++++++++++++++- .../buzz-relay/src/handlers/side_effects.rs | 75 ++++++++- 5 files changed, 442 insertions(+), 17 deletions(-) diff --git a/NOSTR.md b/NOSTR.md index 194ddf0ab55..1450c0cdf33 100644 --- a/NOSTR.md +++ b/NOSTR.md @@ -226,8 +226,9 @@ body of kind:9021, so the channel's `join_role` applies and the channel gets its `member_joined` kind:40099, kind:39002 and kind:44100. There is no cap on how many channels that is: whatever carries the kind:39000 tag is joined, and the work a new member costs grows with the number of channels an owner or -admin flags. The relay does this once, at that moment, and never again on -login or reconnect, so a later kind:9022 leave stands. A kind:9030 add is +admin flags. On a community that requires relay membership, the relay does +this once, at that moment, and never again on login or reconnect, so a later +kind:9022 leave stands. A kind:9030 add is made by someone else, so the new member's `channel_add_policy` applies first, as it would to that actor's kind:9000: if it refuses them, no default channel is joined. The claim response waits for the joins, and a claimer who @@ -238,10 +239,20 @@ record a client reconciles against. That includes a channel archived or made private while the join runs: the membership write decides both again under the channel's locks, so the member is not added and nothing is announced. `buzz-admin add-member` writes the roster row only and -does not join default channels. Auto-join fires only at those roster-insert -sites, so a community that does not require relay membership advertises the -kind:39000 tag but auto-joins nobody; a client that wants the same result -there self-joins (kind:9021) on the tag. +does not join default channels. + +A community that does not require relay membership has no roster insert to +join people at, so there the relay joins a person when it admits their +WebSocket AUTH, after the OK, to every effective default channel they have +never been a member of. "Never a member" is the channel's own membership +history: a kind:9022 leave or a kind:9001 removal keeps the row, so the +person is not joined back, and a new device or another relay pod finds the +row and does nothing. The check runs again under the channel's membership +lock, so two admissions of the same person join them once. A channel flagged +later is joined on each person's next admission. Agents are skipped: an AUTH +with a NIP-OA owner, a NIP-FI agent token, or a stored agent owner joins +nothing. A membership write that fails leaves no row, so the next admission +tries that channel again. ### Group Discovery diff --git a/crates/buzz-db/src/store/channel.rs b/crates/buzz-db/src/store/channel.rs index 9efa08facdc..36ae4a1dc6b 100644 --- a/crates/buzz-db/src/store/channel.rs +++ b/crates/buzz-db/src/store/channel.rs @@ -657,6 +657,44 @@ pub async fn list_auto_join_channels( rows.into_iter().map(row_to_channel_record).collect() } +/// The effective `auto_join` channels (see [`list_auto_join_channels`]) where +/// `pubkey` has never had a membership row, active or removed; oldest first. +/// +/// A kind:9022 leave or a kind:9001 removal keeps the row with `removed_at` +/// set, so a channel the person left is not listed. Read without the +/// membership lock: [`crate::channel_members::put_first_member`] decides +/// again under it. +pub async fn list_never_joined_auto_join_channels( + pool: &PgPool, + community_id: CommunityId, + pubkey: &[u8], +) -> Result> { + let mut connection = crate::observability::acquire_writer( + pool, + crate::observability::WriterOperation::Authorization, + ) + .await?; + let ids = sqlx::query_scalar( + r#" + SELECT c.id + FROM channels c + WHERE c.community_id = $1 AND c.auto_join AND c.channel_type = 'stream' + AND c.visibility = 'open' AND c.archived_at IS NULL AND c.deleted_at IS NULL + AND NOT EXISTS ( + SELECT 1 FROM channel_members m + WHERE m.community_id = c.community_id AND m.channel_id = c.id + AND m.pubkey = $2 + ) + ORDER BY c.created_at ASC + "#, + ) + .bind(community_id.as_uuid()) + .bind(pubkey) + .fetch_all(&mut *connection) + .await?; + Ok(ids) +} + /// A channel archived by the ephemeral-channel reaper. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ReapedEphemeralChannel { @@ -1819,6 +1857,18 @@ impl Db { list_auto_join_channels(&self.pool, community_id).await } + /// Lists the effective `auto_join` channels `pubkey` has never been a + /// member of, active or removed (see + /// [`list_never_joined_auto_join_channels`]). + #[datastore_span(name = "list_never_joined_auto_join_channels", system = "postgresql")] + pub async fn list_never_joined_auto_join_channels( + &self, + community_id: CommunityId, + pubkey: &[u8], + ) -> Result> { + list_never_joined_auto_join_channels(&self.pool, community_id, pubkey).await + } + /// Lists channels during startup reconciliation. #[datastore_span(name = "list_channels_for_bootstrap", system = "postgresql")] pub async fn list_channels_for_bootstrap( diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index b2d78521b66..0e0adefd0eb 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -735,6 +735,62 @@ pub async fn put_members( Ok(results) } +/// Self-join `pubkey` to `channel_id` as [`put_member`] with no actor and no +/// requested role (so the channel's `join_role` applies), but only if +/// `pubkey` has never had a membership row in the channel, active or removed. +/// Returns `None`, writing nothing, when such a row exists. +/// +/// The row is checked under the [`put_member`] locks, so a leave, a join or +/// another first join of the same person commits wholly before or after this +/// write: two sign-ins racing each other write once, and a leave that commits +/// first keeps the person out. A refusal (an archived or private channel) is +/// the [`put_member`] error. +pub async fn put_first_member( + pool: &PgPool, + community_id: CommunityId, + channel_id: Uuid, + pubkey: &[u8], +) -> Result> { + let mut tx = crate::begin_community_write_transaction( + pool, + community_id, + crate::observability::WriterOperation::EventWrite, + ) + .await?; + lock_channel_for_member_put(&mut tx, channel_id).await?; + let seen: bool = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM channel_members \ + WHERE community_id = $1 AND channel_id = $2 AND pubkey = $3)", + ) + .bind(community_id.as_uuid()) + .bind(channel_id) + .bind(pubkey) + .fetch_one(tx.conn()) + .await?; + if seen { + tx.rollback().await?; + return Ok(None); + } + let target = MemberPut { + pubkey, + requested: None, + }; + let put = put_members_on(&mut tx, channel_id, &[target], None) + .await? + .pop() + .ok_or_else(|| DbError::InvalidData("membership put returned no result".to_string()))?; + match put { + Ok(put) => { + tx.commit().await?; + Ok(Some(put.member)) + } + Err(refused) => { + tx.rollback().await?; + Err(refused) + } + } +} + /// The result of [`Db::put_members_with_event`]. pub enum MemberPutEventWrite { /// The event was stored and at least one person was written. One result @@ -2174,6 +2230,18 @@ impl Db { .await } + /// Self-joins `pubkey` with the channel's `join_role` only if they have + /// never been a member of the channel (see [`put_first_member`]). + #[datastore_span(name = "put_first_member", system = "postgresql")] + pub async fn put_first_member( + &self, + community_id: CommunityId, + channel_id: Uuid, + pubkey: &[u8], + ) -> Result> { + put_first_member(&self.pool, community_id, channel_id, pubkey).await + } + /// Stores a kind:9000 `event` and applies its [`put_members`] writes, /// with the event's author as the actor, in one transaction (see /// [`put_members_with_event`]). @@ -2552,6 +2620,96 @@ mod postgres_tests { crate::channel::get_channel(pool, CommunityId::from_uuid(community_id), id).await } + /// A person who has never been in a flagged channel is listed for it and + /// joined once, with the channel's `join_role`; a second put finds the + /// row; after a leave the channel is neither listed nor joined again. An + /// unflagged channel is never listed. + /// + /// Mutations: drop the row check from `put_first_member` → the leave is + /// undone (RED); drop the `NOT EXISTS` from + /// `list_never_joined_auto_join_channels` → the left channel is still + /// listed (RED). + #[tokio::test] + #[ignore = "requires Postgres"] + async fn first_member_put_joins_once_and_never_undoes_a_leave() { + use crate::channel::{list_never_joined_auto_join_channels, update_channel, ChannelUpdate}; + let pool = setup_pool().await; + let community = CommunityId::from_uuid(make_test_community(&pool).await); + let (owner, person) = (random_pubkey(), random_pubkey()); + let mut channels = Vec::new(); + for name in ["general", "announcements", "random"] { + let channel = create_test_channel( + &pool, + *community.as_uuid(), + name, + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner, + None, + ) + .await + .expect("create channel"); + channels.push(channel.id); + } + let (general, announcements, unflagged) = (channels[0], channels[1], channels[2]); + for (channel, join_role) in [(general, None), (announcements, Some("guest"))] { + update_channel( + &pool, + community, + channel, + ChannelUpdate { + auto_join: Some(true), + join_role: join_role.map(str::to_string), + ..Default::default() + }, + ) + .await + .expect("flag channel"); + } + + let never_joined = || list_never_joined_auto_join_channels(&pool, community, &person); + assert_eq!( + never_joined().await.expect("list"), + [general, announcements] + ); + + let put_first = |channel| put_first_member(&pool, community, channel, &person); + assert_eq!( + put_first(general).await.expect("first put").map(|m| m.role), + Some("member".to_string()) + ); + assert_eq!( + put_first(announcements) + .await + .expect("first put") + .map(|m| m.role), + Some("guest".to_string()), + "the channel's join_role applies" + ); + assert!(put_first(general).await.expect("second put").is_none()); + assert!(never_joined().await.expect("list").is_empty()); + + remove_member(&pool, community, general, &person, &person) + .await + .expect("leave"); + assert!(never_joined().await.expect("list").is_empty()); + assert!(put_first(general).await.expect("put after leave").is_none()); + assert_eq!( + get_member_role(&pool, community, general, &person) + .await + .expect("role"), + None, + "a leave stands" + ); + assert_eq!( + get_member_role(&pool, community, unflagged, &person) + .await + .expect("role"), + None + ); + } + #[tokio::test] #[ignore = "requires Postgres"] async fn get_users_bulk_is_scoped_when_pubkey_exists_in_multiple_communities() { diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index dd1111ee631..81f4f3fe193 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -750,14 +750,20 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: .as_ref() .map(crate::nip_fi_core::NipFiSession::from_assertion), ); - match admitted_owner(&state, conn.tenant.community(), pubkey, nip_oa_owner).await { - Ok(Some(owner)) => state.conn_manager.set_admitted_owner(conn_id, owner), - Ok(None) => {} - Err(denial) => { - deny_admission(&conn, &event_id_hex, denial); - return; - } - } + // An admitted owner (NIP-OA, a NIP-FI agent token, or a stored + // `agent_owner_pubkey`) makes this socket an agent's. + let admitted_agent = + match admitted_owner(&state, conn.tenant.community(), pubkey, nip_oa_owner).await { + Ok(Some(owner)) => { + state.conn_manager.set_admitted_owner(conn_id, owner); + true + } + Ok(None) => false, + Err(denial) => { + deny_admission(&conn, &event_id_hex, denial); + return; + } + }; let denial = match final_admission_denial( &state, conn.tenant.community(), @@ -830,6 +836,21 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: &conn.tenant, conn.nip_fi_assertion.as_ref(), ); + // A community without a membership roster has no roster insert to + // join default channels at, so a person is joined on admission to + // the ones they have never been in. Agents are not. + if !state.config.require_relay_membership && !admitted_agent { + let state = Arc::clone(&state); + let tenant = conn.tenant.clone(); + tokio::spawn(async move { + crate::handlers::side_effects::auto_join_never_joined_channels( + &tenant, + &state, + &pubkey.to_bytes(), + ) + .await; + }); + } // _auth_permit drops here — expiry's write guard may proceed. } Err(e) => { @@ -2346,6 +2367,122 @@ mod tests { assert!(!conns[2].cancel.is_cancelled(), "the bystander stays"); } + /// On a community without a membership roster, a person's admission + /// joins them to the default channels they have never been in. An + /// agent's (NIP-OA) admission does not, and a person who left is not + /// joined back when they are admitted again. The joins run after the + /// AUTH reply, so each negative check waits for a later person's + /// join first. + /// + /// Mutations: delete the `auto_join_never_joined_channels` spawn → + /// nobody is joined (RED); drop `!admitted_agent` → the agent is + /// joined (RED). + #[tokio::test] + #[ignore = "requires Postgres — runs in postgres-ci nextest lane"] + async fn root_auth_joins_people_to_default_channels_without_a_roster() { + use buzz_db::channel::{ChannelType, ChannelUpdate, ChannelVisibility}; + use std::sync::Arc; + let state = auth_test_state_real_db_expect().await; + let community = seeded_community(&state).await; + let (owner, agent, person, later) = ( + Keys::generate(), + Keys::generate(), + Keys::generate(), + Keys::generate(), + ); + state + .db + .ensure_user_for_authorization(community, owner.public_key().as_bytes()) + .await + .expect("seed owner"); + let channel = state + .db + .create_channel( + community, + "general", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner.public_key().to_bytes(), + None, + ) + .await + .expect("create channel") + .id; + state + .db + .update_channel( + community, + channel, + ChannelUpdate { + auto_join: Some(true), + ..Default::default() + }, + ) + .await + .expect("flag channel"); + let role = |keys: &Keys| { + let state = Arc::clone(&state); + let pubkey = keys.public_key().to_bytes(); + async move { + state + .db + .get_member_role(community, channel, &pubkey) + .await + .expect("member role") + } + }; + let admit = |keys: &Keys, challenge: &'static str, tag: Option>| { + let state = Arc::clone(&state); + let event = signed_auth(keys, challenge, tag); + async move { + let (conn, _ctrl) = registered_pending_conn(&state, community, challenge); + handle_auth(event, Arc::clone(&conn), state).await; + assert!( + matches!(conn.auth_state_snapshot(), AuthState::Authenticated(_)), + "{challenge} is admitted" + ); + } + }; + let wait_joined = |keys: &Keys| { + let state = Arc::clone(&state); + let pubkey = keys.public_key().to_bytes(); + async move { + for _ in 0..250 { + if let Some(role) = state + .db + .get_member_role(community, channel, &pubkey) + .await + .expect("member role") + { + return role; + } + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + panic!("not joined within 5s"); + } + }; + let auth_tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "") + .expect("sign NIP-OA credential"); + let auth_tag: Vec = serde_json::from_str(&auth_tag).expect("tag JSON"); + + admit(&agent, "auto-join-agent", Some(auth_tag)).await; + admit(&person, "auto-join-person", None).await; + assert_eq!(wait_joined(&person).await, "member"); + assert_eq!(role(&agent).await, None, "an agent is not joined"); + + let person_key = person.public_key().to_bytes(); + state + .db + .remove_member(community, channel, &person_key, &person_key) + .await + .expect("leave"); + admit(&person, "auto-join-person-again", None).await; + admit(&later, "auto-join-later", None).await; + assert_eq!(wait_joined(&later).await, "member"); + assert_eq!(role(&person).await, None, "a leave stands"); + } + /// AUTH with NIP-OA records a previously ownerless agent's owner. That /// closes the agent's earlier ownerless socket so it reconnects with /// the owner attached, but not the socket being admitted, even when diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index ac0c6b38089..1eff427e011 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2763,6 +2763,19 @@ pub(crate) async fn join_channel_as_self( .db .put_member(tenant.community(), channel_id, pubkey, None, None) .await?; + announce_self_join(tenant, state, channel_id, pubkey).await?; + Ok(SelfJoinOutcome::Joined) +} + +/// The side effects of a committed self-join: the membership cache, the +/// `member_joined` kind:40099, kind:39000/39002 and kind:44100. Only the +/// system message is fatal, as for kind:9021. +async fn announce_self_join( + tenant: &TenantContext, + state: &Arc, + channel_id: Uuid, + pubkey: &[u8], +) -> anyhow::Result<()> { state.invalidate_membership(tenant, channel_id, pubkey); let actor_hex = hex::encode(pubkey); @@ -2796,12 +2809,14 @@ pub(crate) async fn join_channel_as_self( warn!("membership notification for join failed: {e}"); } - Ok(SelfJoinOutcome::Joined) + Ok(()) } /// Join a new community member to every effective `auto_join` channel, as -/// kind:9021 self-joins. Runs inline at the membership write sites (invite -/// claim, kind:9030) and nowhere else, so a later kind:9022 leave stands. +/// kind:9021 self-joins. Runs inline at the roster write sites (invite +/// claim, kind:9030) and nowhere else, so a later kind:9022 leave stands. A +/// community without a roster joins on admission instead +/// ([`auto_join_never_joined_channels`]). /// /// Best effort: the relay membership has already committed, so a channel /// that fails is logged and counted, never retried, and never fails the @@ -2872,6 +2887,60 @@ pub(crate) async fn auto_join_default_channels( } } +/// Join a person admitted to a community that keeps no membership roster +/// (`require_relay_membership = false`) to every effective `auto_join` +/// channel they have never been a member of, as kind:9021 self-joins. +/// +/// Runs on every human WebSocket admission there, because such a community +/// has no roster insert to run [`auto_join_default_channels`] at. "Never a +/// member" is the channel's own membership history: a kind:9022 leave or a +/// kind:9001 removal keeps a removed row, so the person is not joined back, +/// and a channel flagged later is joined on their next admission. +/// +/// A membership write that fails leaves no row, so the next admission tries +/// that channel again. Failures are logged and counted +/// (`buzz_auto_join_failures_total`). +pub(crate) async fn auto_join_never_joined_channels( + tenant: &TenantContext, + state: &Arc, + pubkey: &[u8], +) { + let member = hex::encode(pubkey); + let channels = match state + .db + .list_never_joined_auto_join_channels(tenant.community(), pubkey) + .await + { + Ok(channels) => channels, + Err(e) => { + warn!(%member, error = %e, "auto-join skipped: listing default channels failed"); + metrics::counter!("buzz_auto_join_failures_total").increment(1); + return; + } + }; + for channel_id in channels { + let joined = match state + .db + .put_first_member(tenant.community(), channel_id, pubkey) + .await + { + Ok(None) => continue, + Ok(Some(_)) => announce_self_join(tenant, state, channel_id, pubkey).await, + Err(e) => Err(e.into()), + }; + match joined { + Ok(()) => { + metrics::counter!("buzz_auto_join_channels_total").increment(1); + info!(channel = %channel_id, %member, "auto-joined default channel on admission"); + } + Err(e) => { + metrics::counter!("buzz_auto_join_failures_total").increment(1); + warn!(channel = %channel_id, %member, error = %e, "auto-join of default channel on admission failed"); + } + } + } +} + async fn handle_join_request( tenant: &TenantContext, event: &Event, From e7dc71b877b0c593311f13fd42bedc732ef2bafe Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Sun, 1 Nov 2026 08:28:39 -0500 Subject: [PATCH 12/13] fix(relay): hold the session's effect permit for each admission auto-join The admission auto-join ran detached after the AUTH permit dropped, so a session that expired (or a socket that was cancelled) while it waited could still gain memberships and publish join events after teardown. The task now takes the admitting socket's gate and acquires an effect permit for each channel, held through the membership write and its announcement, and stops at the first refusal. A channel it stops before has no row, so the next admission joins it. Tests now wait on the admission's own join task through test-only hooks instead of on a later person's join, cover a session ending before the first channel and between two channels, and race two first joins of the same person in buzz-db. Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-db/src/store/channel_members.rs | 19 +- crates/buzz-relay/src/handlers/auth.rs | 252 ++++++++++++------ .../buzz-relay/src/handlers/side_effects.rs | 38 ++- crates/buzz-relay/src/nip_fi_test_hooks.rs | 13 + 4 files changed, 231 insertions(+), 91 deletions(-) diff --git a/crates/buzz-db/src/store/channel_members.rs b/crates/buzz-db/src/store/channel_members.rs index 0e0adefd0eb..2c139228a8e 100644 --- a/crates/buzz-db/src/store/channel_members.rs +++ b/crates/buzz-db/src/store/channel_members.rs @@ -2625,8 +2625,11 @@ mod postgres_tests { /// row; after a leave the channel is neither listed nor joined again. An /// unflagged channel is never listed. /// + /// Two racing first joins write once. + /// /// Mutations: drop the row check from `put_first_member` → the leave is - /// undone (RED); drop the `NOT EXISTS` from + /// undone (RED); take the locks after the row check → racing joins both + /// write (RED); drop the `NOT EXISTS` from /// `list_never_joined_auto_join_channels` → the left channel is still /// listed (RED). #[tokio::test] @@ -2690,6 +2693,20 @@ mod postgres_tests { assert!(put_first(general).await.expect("second put").is_none()); assert!(never_joined().await.expect("list").is_empty()); + // Two first joins of the same person racing each other write once. + for _ in 0..5 { + let racer = random_pubkey(); + let (a, b) = tokio::join!( + put_first_member(&pool, community, general, &racer), + put_first_member(&pool, community, general, &racer), + ); + let written = [a.expect("racing put"), b.expect("racing put")] + .iter() + .filter(|put| put.is_some()) + .count(); + assert_eq!(written, 1, "racing first joins write once"); + } + remove_member(&pool, community, general, &person, &person) .await .expect("leave"); diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index 81f4f3fe193..c7f80ee9ecb 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -838,15 +838,18 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: ); // A community without a membership roster has no roster insert to // join default channels at, so a person is joined on admission to - // the ones they have never been in. Agents are not. + // the ones they have never been in, under this socket's effect + // permits. Agents are not. if !state.config.require_relay_membership && !admitted_agent { let state = Arc::clone(&state); let tenant = conn.tenant.clone(); + let gate = Arc::clone(&conn.nip_fi_gate); tokio::spawn(async move { crate::handlers::side_effects::auto_join_never_joined_channels( &tenant, &state, &pubkey.to_bytes(), + &gate, ) .await; }); @@ -2367,62 +2370,102 @@ mod tests { assert!(!conns[2].cancel.is_cancelled(), "the bystander stays"); } + /// Flags `names` as default channels in `community`, oldest first. + async fn default_channels( + state: &crate::state::AppState, + community: buzz_core::tenant::CommunityId, + owner: &Keys, + names: &[&str], + ) -> Vec { + use buzz_db::channel::{ChannelType, ChannelUpdate, ChannelVisibility}; + state + .db + .ensure_user_for_authorization(community, owner.public_key().as_bytes()) + .await + .expect("seed owner"); + let mut channels = Vec::new(); + for name in names { + let channel = state + .db + .create_channel( + community, + name, + ChannelType::Stream, + ChannelVisibility::Open, + None, + &owner.public_key().to_bytes(), + None, + ) + .await + .expect("create channel") + .id; + state + .db + .update_channel( + community, + channel, + ChannelUpdate { + auto_join: Some(true), + ..Default::default() + }, + ) + .await + .expect("flag channel"); + channels.push(channel); + } + channels + } + + /// Admits `keys` on a fresh root socket and waits for the admission's + /// auto-join task, if it runs one, to finish. Returns the socket. + async fn admit_and_settle( + state: &std::sync::Arc, + community: buzz_core::tenant::CommunityId, + keys: &Keys, + challenge: &str, + tag: Option>, + ) -> std::sync::Arc { + use crate::nip_fi_test_hooks::auto_join_done_hook; + let (done, release) = auto_join_done_hook::arm(community); + let (conn, _ctrl) = registered_pending_conn(state, community, challenge); + handle_auth( + signed_auth(keys, challenge, tag), + std::sync::Arc::clone(&conn), + std::sync::Arc::clone(state), + ) + .await; + assert!( + matches!(conn.auth_state_snapshot(), AuthState::Authenticated(_)), + "{challenge} is admitted" + ); + // An agent's admission runs no task; a bounded wait lets one that + // runs anyway finish before the caller looks. + match tokio::time::timeout(std::time::Duration::from_secs(1), done).await { + Ok(_) => release.notify_one(), + Err(_) => auto_join_done_hook::disarm(community), + } + conn + } + /// On a community without a membership roster, a person's admission /// joins them to the default channels they have never been in. An - /// agent's (NIP-OA) admission does not, and a person who left is not - /// joined back when they are admitted again. The joins run after the - /// AUTH reply, so each negative check waits for a later person's - /// join first. + /// agent's (NIP-OA) admission runs no join, and a person who left is + /// not joined back when they are admitted again. Each check waits for + /// the admission's own join task to finish. /// /// Mutations: delete the `auto_join_never_joined_channels` spawn → /// nobody is joined (RED); drop `!admitted_agent` → the agent is - /// joined (RED). + /// joined (RED); drop the row check in `put_first_member` → the + /// leave is undone (RED). #[tokio::test] #[ignore = "requires Postgres — runs in postgres-ci nextest lane"] async fn root_auth_joins_people_to_default_channels_without_a_roster() { - use buzz_db::channel::{ChannelType, ChannelUpdate, ChannelVisibility}; - use std::sync::Arc; let state = auth_test_state_real_db_expect().await; let community = seeded_community(&state).await; - let (owner, agent, person, later) = ( - Keys::generate(), - Keys::generate(), - Keys::generate(), - Keys::generate(), - ); - state - .db - .ensure_user_for_authorization(community, owner.public_key().as_bytes()) - .await - .expect("seed owner"); - let channel = state - .db - .create_channel( - community, - "general", - ChannelType::Stream, - ChannelVisibility::Open, - None, - &owner.public_key().to_bytes(), - None, - ) - .await - .expect("create channel") - .id; - state - .db - .update_channel( - community, - channel, - ChannelUpdate { - auto_join: Some(true), - ..Default::default() - }, - ) - .await - .expect("flag channel"); + let (owner, agent, person) = (Keys::generate(), Keys::generate(), Keys::generate()); + let channel = default_channels(&state, community, &owner, &["general"]).await[0]; let role = |keys: &Keys| { - let state = Arc::clone(&state); + let state = std::sync::Arc::clone(&state); let pubkey = keys.public_key().to_bytes(); async move { state @@ -2432,57 +2475,102 @@ mod tests { .expect("member role") } }; - let admit = |keys: &Keys, challenge: &'static str, tag: Option>| { - let state = Arc::clone(&state); - let event = signed_auth(keys, challenge, tag); - async move { - let (conn, _ctrl) = registered_pending_conn(&state, community, challenge); - handle_auth(event, Arc::clone(&conn), state).await; - assert!( - matches!(conn.auth_state_snapshot(), AuthState::Authenticated(_)), - "{challenge} is admitted" - ); - } - }; - let wait_joined = |keys: &Keys| { - let state = Arc::clone(&state); - let pubkey = keys.public_key().to_bytes(); - async move { - for _ in 0..250 { - if let Some(role) = state - .db - .get_member_role(community, channel, &pubkey) - .await - .expect("member role") - { - return role; - } - tokio::time::sleep(std::time::Duration::from_millis(20)).await; - } - panic!("not joined within 5s"); - } - }; let auth_tag = buzz_sdk::nip_oa::compute_auth_tag(&owner, &agent.public_key(), "") .expect("sign NIP-OA credential"); let auth_tag: Vec = serde_json::from_str(&auth_tag).expect("tag JSON"); - admit(&agent, "auto-join-agent", Some(auth_tag)).await; - admit(&person, "auto-join-person", None).await; - assert_eq!(wait_joined(&person).await, "member"); + admit_and_settle(&state, community, &agent, "auto-join-agent", Some(auth_tag)).await; assert_eq!(role(&agent).await, None, "an agent is not joined"); + admit_and_settle(&state, community, &person, "auto-join-person", None).await; + assert_eq!(role(&person).await.as_deref(), Some("member")); + let person_key = person.public_key().to_bytes(); state .db .remove_member(community, channel, &person_key, &person_key) .await .expect("leave"); - admit(&person, "auto-join-person-again", None).await; - admit(&later, "auto-join-later", None).await; - assert_eq!(wait_joined(&later).await, "member"); + admit_and_settle(&state, community, &person, "auto-join-again", None).await; assert_eq!(role(&person).await, None, "a leave stands"); } + /// The admission auto-join holds the socket's effect permit for each + /// channel, so a session that ends before a channel's write joins + /// nothing more: cancelled before the first channel, nothing; between + /// the two channels, only the first. The next admission then joins + /// the channel that was skipped. + /// + /// Mutation: drop the per-channel `acquire_effect` → the cancelled + /// session is joined anyway (RED). + #[tokio::test] + #[ignore = "requires Postgres — runs in postgres-ci nextest lane"] + async fn admission_auto_join_stops_when_the_session_ends() { + use crate::nip_fi_test_hooks::{auto_join_channel_hook, auto_join_done_hook}; + let state = auth_test_state_real_db_expect().await; + let community = seeded_community(&state).await; + let owner = Keys::generate(); + let channels = + default_channels(&state, community, &owner, &["general", "announcements"]).await; + let roles = |keys: &Keys| { + let state = std::sync::Arc::clone(&state); + let pubkey = keys.public_key().to_bytes(); + let channels = channels.clone(); + async move { + let mut roles = Vec::new(); + for channel in channels { + roles.push( + state + .db + .get_member_role(community, channel, &pubkey) + .await + .expect("member role"), + ); + } + roles + } + }; + let member = Some("member".to_string()); + + // `stop_before` channels pass, then the socket is cancelled. + for (stop_before, expected) in [(0, [None, None]), (1, [member.clone(), None])] { + let person = Keys::generate(); + let (done, done_release) = auto_join_done_hook::arm(community); + let (mut reached, mut release) = auto_join_channel_hook::arm(community); + let (conn, _ctrl) = registered_pending_conn(&state, community, "auto-join-stop"); + handle_auth( + signed_auth(&person, "auto-join-stop", None), + std::sync::Arc::clone(&conn), + std::sync::Arc::clone(&state), + ) + .await; + assert!(matches!( + conn.auth_state_snapshot(), + AuthState::Authenticated(_) + )); + for _ in 0..stop_before { + reached.await.expect("task reaches a channel"); + // Re-arm before the task can run again: this task does + // not yield between the release and the arm. + release.notify_one(); + (reached, release) = auto_join_channel_hook::arm(community); + } + reached.await.expect("task reaches the stop channel"); + conn.cancel.cancel(); + release.notify_one(); + done.await.expect("task finishes"); + done_release.notify_one(); + assert_eq!(roles(&person).await, expected, "stop before {stop_before}"); + + admit_and_settle(&state, community, &person, "auto-join-resume", None).await; + assert_eq!( + roles(&person).await, + [member.clone(), member.clone()], + "the next admission joins what was skipped" + ); + } + } + /// AUTH with NIP-OA records a previously ownerless agent's owner. That /// closes the agent's earlier ownerless socket so it reconnects with /// the owner attached, but not the socket being admitted, even when diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index 1eff427e011..8f2977bb04e 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2891,19 +2891,35 @@ pub(crate) async fn auto_join_default_channels( /// (`require_relay_membership = false`) to every effective `auto_join` /// channel they have never been a member of, as kind:9021 self-joins. /// -/// Runs on every human WebSocket admission there, because such a community -/// has no roster insert to run [`auto_join_default_channels`] at. "Never a -/// member" is the channel's own membership history: a kind:9022 leave or a -/// kind:9001 removal keeps a removed row, so the person is not joined back, -/// and a channel flagged later is joined on their next admission. +/// Runs after every human WebSocket admission there, because such a +/// community has no roster insert to run [`auto_join_default_channels`] at. +/// "Never a member" is the channel's own membership history: a kind:9022 +/// leave or a kind:9001 removal keeps a removed row, so the person is not +/// joined back, and a channel flagged later is joined on their next +/// admission. /// -/// A membership write that fails leaves no row, so the next admission tries -/// that channel again. Failures are logged and counted -/// (`buzz_auto_join_failures_total`). +/// Each channel's write and announcement hold an effect permit from the +/// admitting socket's `gate`, as every other session effect does, so nothing +/// is written once the session has expired or the socket is cancelled; the +/// task stops at the first refusal. A channel it stops before, or whose +/// membership write fails, has no row, so the next admission tries it again. +/// Failures are logged and counted (`buzz_auto_join_failures_total`). pub(crate) async fn auto_join_never_joined_channels( tenant: &TenantContext, state: &Arc, pubkey: &[u8], + gate: &Arc, +) { + join_never_joined_channels(tenant, state, pubkey, gate).await; + #[cfg(test)] + crate::nip_fi_test_hooks::after_auto_join(tenant.community()).await; +} + +async fn join_never_joined_channels( + tenant: &TenantContext, + state: &Arc, + pubkey: &[u8], + gate: &Arc, ) { let member = hex::encode(pubkey); let channels = match state @@ -2919,6 +2935,12 @@ pub(crate) async fn auto_join_never_joined_channels( } }; for channel_id in channels { + #[cfg(test)] + crate::nip_fi_test_hooks::before_auto_join_channel(tenant.community()).await; + let Ok(_permit) = gate.acquire_effect().await else { + info!(channel = %channel_id, %member, "auto-join stopped: the session ended"); + return; + }; let joined = match state .db .put_first_member(tenant.community(), channel_id, pubkey) diff --git a/crates/buzz-relay/src/nip_fi_test_hooks.rs b/crates/buzz-relay/src/nip_fi_test_hooks.rs index cfcce6155b0..b37209b451d 100644 --- a/crates/buzz-relay/src/nip_fi_test_hooks.rs +++ b/crates/buzz-relay/src/nip_fi_test_hooks.rs @@ -133,6 +133,12 @@ macro_rules! make_hook { (rx, release) } + /// Remove an armed barrier that production code never reached. + #[allow(dead_code)] + pub(crate) fn disarm(community: CommunityId) { + GATE.lock().unwrap().remove(&community); + } + pub(crate) async fn trigger(community: CommunityId) { let gate = GATE.lock().unwrap().remove(&community); if let Some(g) = gate { @@ -198,6 +204,13 @@ make_hook!(audio_membership_check_hook, before_membership_check); // immediately before the relay membership lookup. Closing the pool here makes // that lookup fail with the tenant already bound. make_hook!(git_membership_hook, before_git_membership); +// `before_auto_join_channel`: fires in the admission auto-join before each +// default channel's effect permit. Cancelling the socket here proves that +// channel is not joined. +make_hook!(auto_join_channel_hook, before_auto_join_channel); +// `after_auto_join`: fires when an admission auto-join task has finished, so +// a test can assert what it did (or did not) write. +make_hook!(auto_join_done_hook, after_auto_join); // `before_http_ban_recheck`: fires in `buzz_v1` and Git settings after the // shared membership check passes, immediately before the fresh ban reread. // A ban or closed pool here makes only the reread refuse. From 051edf2cabeb4fc04a6eb21f1b5327a4a54203b0 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Sun, 1 Nov 2026 08:33:27 -0500 Subject: [PATCH 13/13] test(relay): wait on every admission's auto-join outcome A skipped admission reports through the test hook as the spawned task does when it finishes, and admit_and_settle fails instead of treating a timeout as settled, so a slow mutated task can no longer pass an absence check. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Luke Tornquist --- crates/buzz-relay/src/handlers/auth.rs | 43 ++++++++++++++++---------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/crates/buzz-relay/src/handlers/auth.rs b/crates/buzz-relay/src/handlers/auth.rs index c7f80ee9ecb..b2bd26efe7a 100644 --- a/crates/buzz-relay/src/handlers/auth.rs +++ b/crates/buzz-relay/src/handlers/auth.rs @@ -840,7 +840,8 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: // join default channels at, so a person is joined on admission to // the ones they have never been in, under this socket's effect // permits. Agents are not. - if !state.config.require_relay_membership && !admitted_agent { + let auto_join = !state.config.require_relay_membership && !admitted_agent; + if auto_join { let state = Arc::clone(&state); let tenant = conn.tenant.clone(); let gate = Arc::clone(&conn.nip_fi_gate); @@ -854,6 +855,12 @@ pub async fn handle_auth(event: nostr::Event, conn: Arc, state: .await; }); } + // A test waits on every admission's auto-join outcome, so a skip + // reports here as the spawned task does when it finishes. + #[cfg(test)] + if !auto_join { + crate::nip_fi_test_hooks::after_auto_join(conn.tenant.community()).await; + } // _auth_permit drops here — expiry's write guard may proceed. } Err(e) => { @@ -2417,7 +2424,8 @@ mod tests { } /// Admits `keys` on a fresh root socket and waits for the admission's - /// auto-join task, if it runs one, to finish. Returns the socket. + /// auto-join outcome: the spawned task finishing, or the skip. + /// Panics if neither arrives. Returns the socket. async fn admit_and_settle( state: &std::sync::Arc, community: buzz_core::tenant::CommunityId, @@ -2428,22 +2436,25 @@ mod tests { use crate::nip_fi_test_hooks::auto_join_done_hook; let (done, release) = auto_join_done_hook::arm(community); let (conn, _ctrl) = registered_pending_conn(state, community, challenge); - handle_auth( - signed_auth(keys, challenge, tag), - std::sync::Arc::clone(&conn), - std::sync::Arc::clone(state), - ) - .await; + // A skip reports inside `handle_auth`, so wait alongside it. + tokio::join!( + handle_auth( + signed_auth(keys, challenge, tag), + std::sync::Arc::clone(&conn), + std::sync::Arc::clone(state), + ), + async { + tokio::time::timeout(std::time::Duration::from_secs(10), done) + .await + .unwrap_or_else(|_| panic!("{challenge}: no auto-join outcome")) + .expect("auto-join hook dropped"); + release.notify_one(); + }, + ); assert!( matches!(conn.auth_state_snapshot(), AuthState::Authenticated(_)), "{challenge} is admitted" ); - // An agent's admission runs no task; a bounded wait lets one that - // runs anyway finish before the caller looks. - match tokio::time::timeout(std::time::Duration::from_secs(1), done).await { - Ok(_) => release.notify_one(), - Err(_) => auto_join_done_hook::disarm(community), - } conn } @@ -2451,10 +2462,10 @@ mod tests { /// joins them to the default channels they have never been in. An /// agent's (NIP-OA) admission runs no join, and a person who left is /// not joined back when they are admitted again. Each check waits for - /// the admission's own join task to finish. + /// that admission's own auto-join outcome (skip or finished task). /// /// Mutations: delete the `auto_join_never_joined_channels` spawn → - /// nobody is joined (RED); drop `!admitted_agent` → the agent is + /// no outcome arrives (RED); drop `!admitted_agent` → the agent is /// joined (RED); drop the row check in `put_first_member` → the /// leave is undone (RED). #[tokio::test]