From cd3541eecfc5aa5916d724bb371594f1becc952e Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Wed, 25 Mar 2026 11:51:38 -0500 Subject: [PATCH 1/2] fix(ui): replace placeholder state on import instead of merging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When importing an identity, the local room state starts with a default ChatRoomStateV1 that has owner_member_id: FastHash(0) — a placeholder. When the real state arrives from the network via GET, the merge fails because apply_delta rejects owner_member_id changes ("Cannot change the owner_member_id"). The imported room is stuck with a broken local state. Fix: when `is_awaiting_initial_sync()` is true (empty members + empty messages + non-owner), replace the local state wholesale with the retrieved network state instead of merging. The default state has no useful data to preserve, so replacement is correct and avoids the owner_member_id mismatch entirely. Closes #195 Co-Authored-By: Claude Opus 4.6 (1M context) --- cli/src/storage.rs | 3 +- common/tests/import_merge_test.rs | 100 ++++++++++++++++++ .../response_handler/get_response.rs | 62 ++++++----- 3 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 common/tests/import_merge_test.rs diff --git a/cli/src/storage.rs b/cli/src/storage.rs index a45b1be1..dfa05201 100644 --- a/cli/src/storage.rs +++ b/cli/src/storage.rs @@ -429,8 +429,7 @@ mod tests { let loaded = storage.load_rooms().unwrap(); let owner_key_str = bs58::encode(owner_vk.as_bytes()).into_string(); assert_eq!( - loaded.rooms[&owner_key_str].previous_contract_key, - None, + loaded.rooms[&owner_key_str].previous_contract_key, None, "previous_contract_key should be None when WASM hasn't changed" ); } diff --git a/common/tests/import_merge_test.rs b/common/tests/import_merge_test.rs new file mode 100644 index 00000000..6aeb59f5 --- /dev/null +++ b/common/tests/import_merge_test.rs @@ -0,0 +1,100 @@ +//! Regression test for #195: importing an identity creates a default state with +//! owner_member_id: FastHash(0). Merging the real network state fails because +//! apply_delta rejects owner_member_id changes. The fix: replace the state +//! wholesale when is_awaiting_initial_sync() is true instead of merging. + +use ed25519_dalek::SigningKey; +use freenet_scaffold::util::FastHash; +use freenet_scaffold::ComposableState; +use rand::rngs::OsRng; +use river_core::room_state::configuration::{AuthorizedConfigurationV1, Configuration}; +use river_core::room_state::member::{AuthorizedMember, Member, MemberId}; +use river_core::room_state::ChatRoomParametersV1; +use river_core::room_state::ChatRoomStateV1; + +#[test] +fn test_default_state_has_placeholder_owner() { + let default_state = ChatRoomStateV1::default(); + assert_eq!( + default_state.configuration.configuration.owner_member_id, + MemberId(FastHash(0)), + "Default state should have placeholder owner_member_id" + ); +} + +#[test] +fn test_merge_fails_when_owner_member_id_differs() { + let owner_sk = SigningKey::generate(&mut OsRng); + let owner_vk = owner_sk.verifying_key(); + let params = ChatRoomParametersV1 { owner: owner_vk }; + + // Simulate import: start with default state (placeholder owner) + let mut local_state = ChatRoomStateV1::default(); + let current = local_state.clone(); + + // Build network state with real owner and config version > 1 + let config = Configuration { + owner_member_id: owner_vk.into(), + configuration_version: 2, + ..Default::default() + }; + let auth_config = AuthorizedConfigurationV1::new(config, &owner_sk); + let network_state = ChatRoomStateV1 { + configuration: auth_config, + ..Default::default() + }; + + // Merge should fail because owner_member_id differs + let result = local_state.merge(¤t, ¶ms, &network_state); + assert!( + result.is_err(), + "Merge should fail due to owner_member_id mismatch" + ); + assert!( + result.unwrap_err().contains("owner_member_id"), + "Error should mention owner_member_id" + ); +} + +#[test] +fn test_wholesale_replacement_works_for_imported_rooms() { + let owner_sk = SigningKey::generate(&mut OsRng); + let owner_vk = owner_sk.verifying_key(); + let invitee_sk = SigningKey::generate(&mut OsRng); + + // Build network state with proper owner, config, and a member + let config = Configuration { + owner_member_id: owner_vk.into(), + configuration_version: 2, + ..Default::default() + }; + let auth_config = AuthorizedConfigurationV1::new(config, &owner_sk); + let member = Member { + owner_member_id: owner_vk.into(), + invited_by: owner_vk.into(), + member_vk: invitee_sk.verifying_key(), + }; + let auth_member = AuthorizedMember::new(member, &owner_sk); + let mut network_state = ChatRoomStateV1 { + configuration: auth_config, + ..Default::default() + }; + network_state.members.members.push(auth_member); + + // Start with default (import) state + let mut local_state = ChatRoomStateV1::default(); + assert!(local_state.members.members.is_empty()); + assert_eq!( + local_state.configuration.configuration.owner_member_id, + MemberId(FastHash(0)), + ); + + // Wholesale replacement (the fix for #195) + local_state = network_state; + + assert_eq!(local_state.members.members.len(), 1); + assert_eq!( + local_state.configuration.configuration.owner_member_id, + MemberId::from(owner_vk), + ); +} diff --git a/ui/src/components/app/freenet_api/response_handler/get_response.rs b/ui/src/components/app/freenet_api/response_handler/get_response.rs index d8b0e5c2..eb1eff9b 100644 --- a/ui/src/components/app/freenet_api/response_handler/get_response.rs +++ b/ui/src/components/app/freenet_api/response_handler/get_response.rs @@ -522,37 +522,41 @@ pub async fn handle_get_response( crate::util::defer(move || { ROOMS.with_mut(|rooms| { if let Some(room_data) = rooms.map.get_mut(&owner_vk) { - // Create parameters for merge let params = ChatRoomParametersV1 { owner: owner_vk }; - // Clone current state to avoid borrow issues during merge - let current_state = room_data.room_state.clone(); - - // Merge the retrieved state into the existing state - match room_data - .room_state - .merge(¤t_state, ¶ms, &retrieved_state) - { - Ok(_) => { - info!( - "Successfully merged refreshed state for room {:?}", - MemberId::from(owner_vk) - ); - // Note: we intentionally do NOT record receive times here. - // GET responses don't reflect real-time message arrival — - // we don't know when these messages actually propagated - // to our node. Only subscription UPDATE notifications - // capture the true arrival moment. - - // Migration: capture self membership data for old rooms - room_data.capture_self_membership_data(¶ms); - } - Err(e) => { - error!( - "Failed to merge refreshed state for room {:?}: {}", - MemberId::from(owner_vk), - e - ); + if room_data.is_awaiting_initial_sync() { + // Imported rooms have a placeholder default state with + // owner_member_id: FastHash(0). Merging fails because + // the retrieved state has the real owner's member ID + // and apply_delta rejects owner_member_id changes. + // Replace the state wholesale — the default has no + // useful data to preserve. + info!( + "Replacing placeholder state for imported room {:?} with network state", + MemberId::from(owner_vk) + ); + room_data.room_state = retrieved_state; + room_data.capture_self_membership_data(¶ms); + } else { + let current_state = room_data.room_state.clone(); + match room_data + .room_state + .merge(¤t_state, ¶ms, &retrieved_state) + { + Ok(_) => { + info!( + "Successfully merged refreshed state for room {:?}", + MemberId::from(owner_vk) + ); + room_data.capture_self_membership_data(¶ms); + } + Err(e) => { + error!( + "Failed to merge refreshed state for room {:?}: {}", + MemberId::from(owner_vk), + e + ); + } } } } From e86fd054fa49aa2c341b0f0fa767d3ec97832378 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Wed, 25 Mar 2026 11:54:30 -0500 Subject: [PATCH 2/2] fix(ui): restore receive-times comment removed during refactor Co-Authored-By: Claude Opus 4.6 (1M context) --- .../app/freenet_api/response_handler/get_response.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/src/components/app/freenet_api/response_handler/get_response.rs b/ui/src/components/app/freenet_api/response_handler/get_response.rs index eb1eff9b..db719191 100644 --- a/ui/src/components/app/freenet_api/response_handler/get_response.rs +++ b/ui/src/components/app/freenet_api/response_handler/get_response.rs @@ -524,6 +524,12 @@ pub async fn handle_get_response( if let Some(room_data) = rooms.map.get_mut(&owner_vk) { let params = ChatRoomParametersV1 { owner: owner_vk }; + // Note: we intentionally do NOT record receive times here. + // GET responses don't reflect real-time message arrival — + // we don't know when these messages actually propagated + // to our node. Only subscription UPDATE notifications + // capture the true arrival moment. + if room_data.is_awaiting_initial_sync() { // Imported rooms have a placeholder default state with // owner_member_id: FastHash(0). Merging fails because