You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Rooms::merge_from_source iterates other.map and uses ? on a fallible ChatRoomStateV1::merge in two places — the adopt-incoming branch (beforeself.map.insert) and the already-present branch. Either error returns Err from the whole function. The caller logs and continues, so every room later in the iteration order is never inserted. other.map is a HashMap, so which rooms are lost is arbitrary and unstable between runs.
Two consequences:
Silent partial hydration. The user sees a subset of their rooms with no error surfaced, and which subset varies run to run.
Those rooms become vacant slots, and a vacant slot is exactly where the Lost IDs and rooms to River Update across two nodes. #527 generation-rank check does not run — merge_from_source compares ranks only if let Some(existing) = self.map.get(&vk). So a legacy generation's copy is adopted with no rank check, and reconcile_room_present's diverged-identity branch then CAS-writes that older identity over the current one.
Reachability needs self.map non-empty at merge time with a matching-self_sk room, so the already-present branch is taken rather than Entry::Vacant — e.g. the user creates or joins a room before the current delegate's ListResponse lands, or any reconnect pass.
Suggested fix
Per-room isolation: log the failing room and continue, rather than aborting the whole merge. That matches the precedent in do_save_rooms_to_delegate, which deliberately accumulates per-key errors and keeps going so one room's failure cannot skip the others.
Wants a test that a merge error on one room does not prevent the remaining rooms from being inserted.
Found while reviewing #587.
Rooms::merge_from_sourceiteratesother.mapand uses?on a fallibleChatRoomStateV1::mergein two places — the adopt-incoming branch (beforeself.map.insert) and the already-present branch. Either error returnsErrfrom the whole function. The caller logs and continues, so every room later in the iteration order is never inserted.other.mapis aHashMap, so which rooms are lost is arbitrary and unstable between runs.Two consequences:
merge_from_sourcecompares ranks onlyif let Some(existing) = self.map.get(&vk). So a legacy generation's copy is adopted with no rank check, andreconcile_room_present's diverged-identity branch then CAS-writes that older identity over the current one.Reachability needs
self.mapnon-empty at merge time with a matching-self_skroom, so the already-present branch is taken rather thanEntry::Vacant— e.g. the user creates or joins a room before the current delegate'sListResponselands, or any reconnect pass.Suggested fix
Per-room isolation: log the failing room and continue, rather than aborting the whole merge. That matches the precedent in
do_save_rooms_to_delegate, which deliberately accumulates per-key errors and keeps going so one room's failure cannot skip the others.Wants a test that a merge error on one room does not prevent the remaining rooms from being inserted.
[AI-assisted - Claude]