fix: legacy delegate response overwrites selected room (#255)#263
Merged
Conversation
Problem ------- When a `rooms_data` GetResponse arrived from the chat delegate, the handler unconditionally restored `CURRENT_ROOM` from the snapshot's `current_room_key` (skipping only tombstoned saved rooms). On a fresh session where the user had already clicked Room A, a late-arriving delegate response carrying a stale `current_room_key = Room B` could silently switch the user's view to Room B. The race was worst for legacy-delegate responses, whose cursor is years-stale by construction. Approach -------- Extract the gating logic into a pure helper `decide_current_room_restore`, which considers four inputs: saved-key-present, saved-tombstoned, user-has-selected, is-legacy-delegate. Gates in priority order: 1. no saved key → skip 2. legacy delegate → skip (cursor is structurally stale) 3. user already selected → skip (user's click wins) 4. saved tombstoned → skip (#247) 5. otherwise → restore Both new gates implement the suggestions from the issue. The legacy-delegate gate is placed above the user-selection gate so legacy cursor restoration is blocked even on a fresh session before the user has clicked — matching the issue author's "Optionally also skip when is_legacy_delegate == true" note. Testing ------- Pure helper extracted into testable form. Seven unit tests in `response_handler::tests` cover every gate including: - `issue_255_user_selection_not_overwritten_by_current_delegate_cursor` (fails without the user-has-selected gate) - `issue_255_legacy_delegate_never_restores_cursor_even_without_prior_selection` (fails without the is-legacy-delegate gate) - precedence tests pinning gate order UI-only fix; no delegate or contract WASM changes. `cargo make build-ui` + `cargo test -p river-ui` + `cargo make test` all green. Closes #255 [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sanity
added a commit
that referenced
this pull request
May 17, 2026
Publishes the merged tip of main containing: - #263 (issue #255): legacy delegate response no longer overwrites the user's currently-selected room. - #264 (issue #251): repopulate room_data.secrets on every network state ingestion path so newly-joined private-room members don't render encrypted placeholders until refresh. - #265 (issue #261): hide stale DM threads from the left rail (local-view filter with multi-device sync via OutboundDmStore). Counter 30000306 -> 30000307. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sanity
added a commit
that referenced
this pull request
May 17, 2026
…publish Publishes the merged tip of main containing all 5 bug fixes: - #263 (#255): legacy delegate cursor overwrite - #268 (#268, Bug #2): share-invite-via-DM picker hang - #269 (#269, Bug #1): DM to inactive-but-invited members - #270 (Bug #3 PR A): room-contract accepts msgs at any known secret version - #272 (Bug #3 PR B): UI rotate back-fill + PUT join_event + #110 prune exemption - #273 (#273, Bug #5): WebSocket indicator on no-rooms screen river-core 0.1.8 published to crates.io (workspace version bump). riverctl 0.1.58 published to crates.io alongside the UI publish (addresses Bug #4 — riverctl now matches the deployed room contract WASM). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a
rooms_dataGetResponsearrived from the chat delegate, the handler inresponse_handler.rsunconditionally restoredCURRENT_ROOMfrom the snapshot'scurrent_room_key. The only existing gate filtered out tombstoned rooms (#247). It did not check whether the user had already explicitly selected a different room this session.User-visible failure mode:
CURRENT_ROOM = Some(A).current_room_key = Room B(years-stale).CURRENT_ROOM = Some(B). User's view silently switches.Issue: #255 (spun out of #254 skeptical review H1).
Approach
Extract the gating logic into a pure helper
decide_current_room_restorereturning aCurrentRoomRestoreenum. The helper takes four bools (saved_key_present,saved_tombstoned,user_has_selected,is_legacy_delegate) and applies gates in priority order:SkipNoSavedKeySkipLegacyDelegate(cursor is structurally stale)SkipUserAlreadySelected(the user's click wins — Legacy delegate response can overwrite the user's currently-selected room #255)SkipTombstoned(Leave room doesn't stick: legacy-delegate migration re-adds the room on refresh #247 / skeptical-review H2)RestoreThe is-legacy-delegate gate is placed above the user-selection gate so legacy cursor restoration is blocked even on a fresh session before the user has clicked. That matches the issue author's "Optionally also skip when
is_legacy_delegate == true" note and prevents an early legacy response from silently switching the view.This is purely a UI fix — no delegate or contract WASM changes.
Testing
Seven unit tests in
response_handler::tests(host-side, runs undercargo test -p river-ui) cover every variant ofCurrentRoomRestore, including the two regressions for this issue:issue_255_user_selection_not_overwritten_by_current_delegate_cursor— primary regression test for the user-click → late delegate response race. Fails if the user-has-selected gate is removed.issue_255_legacy_delegate_never_restores_cursor_even_without_prior_selection— pins that legacy responses never restore cursor. Fails if the legacy-delegate gate is removed.issue_255_legacy_gate_precedes_user_selection_gateanduser_selection_gate_precedes_tombstone_gatepin the priority order.Other tests cover the no-saved-key short-circuit, the baseline restore path, and the existing tombstone-skip behaviour from #247.
cargo make build-uisucceeds.cargo test -p river-uiandcargo make testare green.Closes #255
[AI-assisted - Claude]