diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index c88148de..188a9600 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -20,8 +20,8 @@ use dioxus::prelude::*; use dioxus_free_icons::icons::fa_solid_icons::{FaBars, FaCircleInfo, FaUsers}; use dioxus_free_icons::Icon; use freenet_scaffold::ComposableState; -use river_core::room_state::member::MemberId; -use river_core::room_state::member_info::MemberInfoV1; +use river_core::room_state::member::{MemberId, MembersDelta}; +use river_core::room_state::member_info::{AuthorizedMemberInfo, MemberInfoV1}; use river_core::room_state::message::{ AuthorizedMessageV1, MessageId, MessageV1, MessagesV1, RoomMessageBody, }; @@ -33,6 +33,25 @@ use wasm_bindgen::JsCast; use wasm_bindgen_futures::spawn_local; use web_sys; +/// Try to build a rejoin delta for the current user in the given room. +/// Returns (None, None) if the user is already a member or ROOMS is busy. +fn try_rejoin_delta( + room_key: &ed25519_dalek::VerifyingKey, + action: &str, +) -> (Option, Option>) { + let rooms_guard = ROOMS.try_read(); + if let Ok(rooms_read) = rooms_guard { + if let Some(room_data) = rooms_read.map.get(room_key) { + room_data.build_rejoin_delta() + } else { + (None, None) + } + } else { + warn!("ROOMS signal busy during {action}, skipping re-add check"); + (None, None) + } +} + /// Context for a reply-in-progress (held in a signal) #[derive(Clone, PartialEq, Debug)] struct ReplyContext { @@ -737,8 +756,12 @@ pub fn Conversation() -> Element { // Apply all messages in one delta if !auth_messages.is_empty() { + let (members_delta, member_info_delta) = + try_rejoin_delta(¤t_room, "reaction"); let delta = ChatRoomStateV1Delta { recent_messages: Some(auth_messages), + members: members_delta, + member_info: member_info_delta, ..Default::default() }; info!( @@ -837,8 +860,12 @@ pub fn Conversation() -> Element { .await; let auth_message = AuthorizedMessageV1::with_signature(message, signature); + let (members_delta, member_info_delta) = + try_rejoin_delta(¤t_room, "delete"); let delta = ChatRoomStateV1Delta { recent_messages: Some(vec![auth_message]), + members: members_delta, + member_info: member_info_delta, ..Default::default() }; info!("Sending delete action"); @@ -939,8 +966,12 @@ pub fn Conversation() -> Element { .await; let auth_message = AuthorizedMessageV1::with_signature(message, signature); + let (members_delta, member_info_delta) = + try_rejoin_delta(¤t_room, "edit"); let delta = ChatRoomStateV1Delta { recent_messages: Some(vec![auth_message]), + members: members_delta, + member_info: member_info_delta, ..Default::default() }; info!("Sending edit action"); @@ -1123,100 +1154,11 @@ pub fn Conversation() -> Element { let auth_message = AuthorizedMessageV1::with_signature(message, signature); - // Check if we need to re-add ourselves (pruned for inactivity) - // Use try_read() instead of read() to avoid RefCell - // re-entrant borrow panics inside spawn_local (see - // AGENTS.md "Dioxus WASM Signal Safety Rules"). - let (members_delta, member_info_delta) = { - let rooms_guard = ROOMS.try_read(); - if let Ok(rooms_read) = rooms_guard { - if let Some(room_data) = rooms_read.map.get(¤t_room) { - let self_vk = room_data.self_sk.verifying_key(); - let is_in_members = self_vk == current_room - || room_data - .room_state - .members - .members - .iter() - .any(|m| m.member.member_vk == self_vk); - - if !is_in_members { - if let Some(ref authorized_member) = - room_data.self_authorized_member - { - let current_member_ids: std::collections::HashSet<_> = - room_data - .room_state - .members - .members - .iter() - .map(|m| m.member.id()) - .collect(); - let mut members_to_add = vec![authorized_member.clone()]; - for chain_member in &room_data.invite_chain { - if !current_member_ids - .contains(&chain_member.member.id()) - { - members_to_add.push(chain_member.clone()); - } - } - - // Use stored member_info to preserve nickname, or fall back to "Member" - use river_core::room_state::member_info::{ - AuthorizedMemberInfo, MemberInfo, - }; - let authorized_info = if let Some(ref stored_info) = - room_data.self_member_info - { - stored_info.clone() - } else { - use river_core::room_state::privacy::SealedBytes; - let member_id = MemberId::from(&self_vk); - let existing_version = room_data - .room_state - .member_info - .member_info - .iter() - .find(|i| i.member_info.member_id == member_id) - .map(|i| i.member_info.version) - .unwrap_or(0); - let member_info = MemberInfo { - member_id, - version: existing_version, - preferred_nickname: SealedBytes::public( - "Member".to_string().into_bytes(), - ), - }; - AuthorizedMemberInfo::new_with_member_key( - member_info, - &room_data.self_sk, - ) - }; - - ( - Some( - river_core::room_state::member::MembersDelta::new( - members_to_add, - ), - ), - Some(vec![authorized_info]), - ) - } else { - (None, None) - } - } else { - (None, None) - } - } else { - (None, None) - } - } else { - // Safe to skip: message still sends without the re-add delta. - // If the user was pruned, the next message send will retry. - warn!("ROOMS signal busy during send, skipping re-add check"); - (None, None) - } - }; + // Re-add ourselves if pruned for inactivity. + // Uses try_read() to avoid RefCell re-entrant borrow panics + // inside spawn_local (see AGENTS.md "Dioxus WASM Signal Safety Rules"). + let (members_delta, member_info_delta) = + try_rejoin_delta(¤t_room, "send"); // Build message list. No join event here — join events are // published at invitation acceptance time (in get_response.rs). diff --git a/ui/src/components/members/member_info_modal/nickname_field.rs b/ui/src/components/members/member_info_modal/nickname_field.rs index 298ef2aa..eafe6c09 100644 --- a/ui/src/components/members/member_info_modal/nickname_field.rs +++ b/ui/src/components/members/member_info_modal/nickname_field.rs @@ -82,47 +82,16 @@ pub fn NicknameField(member_info: AuthorizedMemberInfo) -> Element { let new_authorized_member_info = AuthorizedMemberInfo::new_with_member_key(new_member_info, &signing_key); // Check if user needs to re-add themselves (pruned for inactivity) + // Re-add ourselves if pruned (only need members delta; + // the member_info delta is the nickname change itself). let members_delta = { let Ok(rooms) = ROOMS.try_read() else { return; }; let current_room = CURRENT_ROOM.read(); - if let (Some(owner_key), Some(room_data)) = ( - current_room.owner_key, - current_room.owner_key.and_then(|k| rooms.map.get(&k)), - ) { - let self_vk = signing_key.verifying_key(); - let is_in_members = self_vk == owner_key - || room_data - .room_state - .members - .members - .iter() - .any(|m| m.member.member_vk == self_vk); - if !is_in_members { - if let Some(ref authorized_member) = room_data.self_authorized_member { - let current_member_ids: std::collections::HashSet<_> = room_data - .room_state - .members - .members - .iter() - .map(|m| m.member.id()) - .collect(); - let mut members_to_add = vec![authorized_member.clone()]; - for chain_member in &room_data.invite_chain { - if !current_member_ids.contains(&chain_member.member.id()) { - members_to_add.push(chain_member.clone()); - } - } - Some(river_core::room_state::member::MembersDelta::new( - members_to_add, - )) - } else { - None - } - } else { - None - } + if let Some(room_data) = current_room.owner_key.and_then(|k| rooms.map.get(&k)) + { + room_data.build_rejoin_delta().0 } else { None } diff --git a/ui/src/room_data.rs b/ui/src/room_data.rs index 8e7ede89..186e02f5 100644 --- a/ui/src/room_data.rs +++ b/ui/src/room_data.rs @@ -314,6 +314,78 @@ impl RoomData { } } + /// Build the members + member_info deltas needed to re-add ourselves to the room + /// after being pruned for inactivity. Returns (None, None) if we're already a member + /// or don't have stored credentials to re-add. + pub fn build_rejoin_delta( + &self, + ) -> ( + Option, + Option>, + ) { + let self_vk = self.self_sk.verifying_key(); + + // Owner is never pruned + let is_in_members = self_vk == self.owner_vk + || self + .room_state + .members + .members + .iter() + .any(|m| m.member.member_vk == self_vk); + + if is_in_members { + return (None, None); + } + + let Some(ref authorized_member) = self.self_authorized_member else { + return (None, None); + }; + + let current_member_ids: std::collections::HashSet<_> = self + .room_state + .members + .members + .iter() + .map(|m| m.member.id()) + .collect(); + + let mut members_to_add = vec![authorized_member.clone()]; + for chain_member in &self.invite_chain { + if !current_member_ids.contains(&chain_member.member.id()) { + members_to_add.push(chain_member.clone()); + } + } + + // Use stored member_info to preserve nickname, or fall back to "Member" + let authorized_info = if let Some(ref stored_info) = self.self_member_info { + stored_info.clone() + } else { + let member_id = MemberId::from(&self_vk); + let existing_version = self + .room_state + .member_info + .member_info + .iter() + .find(|i| i.member_info.member_id == member_id) + .map(|i| i.member_info.version) + .unwrap_or(0); + let member_info = MemberInfo { + member_id, + version: existing_version, + preferred_nickname: SealedBytes::public("Member".to_string().into_bytes()), + }; + AuthorizedMemberInfo::new_with_member_key(member_info, &self.self_sk) + }; + + ( + Some(river_core::room_state::member::MembersDelta::new( + members_to_add, + )), + Some(vec![authorized_info]), + ) + } + pub fn owner_id(&self) -> MemberId { self.owner_vk.into() } @@ -1010,4 +1082,161 @@ mod tests { .push(dummy_msg); assert!(!pruned_room.is_awaiting_initial_sync()); } + + /// Helper to build a RoomData for rejoin tests. + fn make_rejoin_test_room( + owner_sk: &SigningKey, + invitee_sk: &SigningKey, + include_member: bool, + ) -> RoomData { + let owner_vk = owner_sk.verifying_key(); + let invitee_vk = invitee_sk.verifying_key(); + + let config = AuthorizedConfigurationV1::new(Configuration::default(), owner_sk); + let mut room_state = ChatRoomStateV1 { + configuration: config, + ..Default::default() + }; + + let member = Member { + owner_member_id: owner_vk.into(), + invited_by: owner_vk.into(), + member_vk: invitee_vk, + }; + let authorized_member = AuthorizedMember::new(member, owner_sk); + + if include_member { + room_state.members.members.push(authorized_member.clone()); + } + + let params = ChatRoomParametersV1 { owner: owner_vk }; + let params_bytes = to_cbor_vec(¶ms); + let contract_code = ContractCode::from(ROOM_CONTRACT_WASM); + let contract_key = + ContractKey::from_params_and_code(Parameters::from(params_bytes), &contract_code); + + RoomData { + owner_vk, + room_state, + self_sk: invitee_sk.clone(), + contract_key, + last_read_message_id: None, + secrets: HashMap::new(), + current_secret_version: None, + last_secret_rotation: None, + key_migrated_to_delegate: false, + self_authorized_member: Some(authorized_member), + invite_chain: vec![], + self_member_info: None, + previous_contract_key: None, + } + } + + #[test] + fn test_build_rejoin_delta_returns_none_when_member_present() { + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + let invitee_sk = SigningKey::generate(&mut rng); + + let room = make_rejoin_test_room(&owner_sk, &invitee_sk, true); + let (members, info) = room.build_rejoin_delta(); + assert!(members.is_none()); + assert!(info.is_none()); + } + + #[test] + fn test_build_rejoin_delta_returns_none_for_owner() { + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + + // Owner as self_sk, not in members list (owner is never explicitly listed) + let mut room = make_rejoin_test_room(&owner_sk, &owner_sk, false); + room.self_authorized_member = None; + let (members, info) = room.build_rejoin_delta(); + assert!(members.is_none()); + assert!(info.is_none()); + } + + #[test] + fn test_build_rejoin_delta_returns_none_without_credentials() { + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + let invitee_sk = SigningKey::generate(&mut rng); + + let mut room = make_rejoin_test_room(&owner_sk, &invitee_sk, false); + room.self_authorized_member = None; + let (members, info) = room.build_rejoin_delta(); + assert!(members.is_none()); + assert!(info.is_none()); + } + + #[test] + fn test_build_rejoin_delta_constructs_delta_when_pruned() { + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + let invitee_sk = SigningKey::generate(&mut rng); + let invitee_vk = invitee_sk.verifying_key(); + + // Member NOT in members list but has stored credentials + let room = make_rejoin_test_room(&owner_sk, &invitee_sk, false); + let (members, info) = room.build_rejoin_delta(); + + let members = members.expect("should have members delta"); + assert_eq!(members.added().len(), 1); + assert_eq!(members.added()[0].member.member_vk, invitee_vk); + + let info = info.expect("should have member_info delta"); + assert_eq!(info.len(), 1); + assert_eq!(info[0].member_info.member_id, MemberId::from(&invitee_vk)); + } + + #[test] + fn test_build_rejoin_delta_uses_stored_member_info() { + use river_core::room_state::privacy::SealedBytes; + + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + let invitee_sk = SigningKey::generate(&mut rng); + let invitee_vk = invitee_sk.verifying_key(); + let member_id = MemberId::from(&invitee_vk); + + let mut room = make_rejoin_test_room(&owner_sk, &invitee_sk, false); + + // Store member_info with a custom nickname + let info = MemberInfo { + member_id, + version: 5, + preferred_nickname: SealedBytes::public("Alice".to_string().into_bytes()), + }; + room.self_member_info = Some(AuthorizedMemberInfo::new_with_member_key(info, &invitee_sk)); + + let (_, member_info) = room.build_rejoin_delta(); + let member_info = member_info.expect("should have member_info delta"); + assert_eq!(member_info[0].member_info.version, 5); + } + + #[test] + fn test_build_rejoin_delta_includes_missing_invite_chain() { + let mut rng = rand::thread_rng(); + let owner_sk = SigningKey::generate(&mut rng); + let invitee_sk = SigningKey::generate(&mut rng); + let chain_sk = SigningKey::generate(&mut rng); + let chain_vk = chain_sk.verifying_key(); + + let mut room = make_rejoin_test_room(&owner_sk, &invitee_sk, false); + + // Add a chain member that's also missing from the room + let chain_member = Member { + owner_member_id: owner_sk.verifying_key().into(), + invited_by: owner_sk.verifying_key().into(), + member_vk: chain_vk, + }; + room.invite_chain + .push(AuthorizedMember::new(chain_member, &owner_sk)); + + let (members, _) = room.build_rejoin_delta(); + let members = members.expect("should have members delta"); + // Should include both self and the missing chain member + assert_eq!(members.added().len(), 2); + } }