feat(dm): archive UX overhaul + delete confirmation#275
Merged
Conversation
Issue #266 (and closes the gap left by #261): the Hide button in the DM thread modal header sat next to the close ✕ and was repeatedly mistaken for it; the "Delete their messages" footer button fired destructively on a single click with no undo path. Replace both with safer, more conventional surfaces: - Per-row rollover ✕ in `DmRailSection`, with `group-hover` / `group-focus-within:opacity-100` on desktop and `md:opacity-40` on mobile so touch users keep the affordance. Tooltip and aria-label read "Archive this conversation. It will return if either of you sends a new DM." - "Archived (N)" link at the bottom of the rail. Click expands an inline list of currently-archived threads, each with an Un-archive button (sorted by room name, then peer nickname for stable rendering). - "Archived — Undo" toast (~5s) shows after the rollover ✕ click. Undo restores the row immediately; the auto-dismiss tick uses an `expires_at_ms` identity check so a rapid second archive doesn't cancel the second toast early. - Hide button removed from the DM thread modal header. Header is now just the title and the close ✕ — no visual collision. - "Delete their messages" now opens a Cancel/Delete confirmation dialog (Cancel default-focused, Esc closes) before firing `purge_thread`. Renaming is UX-only — the on-wire `OutboundDmStore.hidden_threads` shape, the `HIDDEN_DM_THREADS` signal, and the `hide_dm_thread`/`unhide_dm_thread` API names stay as they were. Renaming them would force a delegate migration for zero functional benefit. The user-facing surface is "Archive" everywhere visible; the implementation noun stays "hide." Test coverage: - Existing `filter_rail_entries_*` regression tests in `dm_rail_section` still pin the strict-`<=` revive semantics (no behavioural change to the filter). - New `build_archived_rows_projects_and_sorts` and `build_archived_rows_falls_back_when_room_missing` pin the archived-viewer projection (including the "(unknown room)" fallback for threads whose owning room has been left). - New `build_archive_toast_advances_expiry_per_call` and `build_archive_toast_saturates_on_overflow` pin the toast expiry math. - Smoke playwright spec (`dm-archive-ux.spec.ts`) verifies the app boots cleanly with the new code paths, the "Hide" button is gone from the rendered DOM, and the rail's no-DMs early-return still kicks in after the archive viewer was added. End-to-end DM flow tests (archive → row disappears → un-archive → row returns; confirmation modal opens and Cancel keeps state) are deliberately out of scope for the example-data Playwright build, which has no DMs in its fixture. The behavioural invariants are covered by the Rust unit tests above. Updates AGENTS.md "Phase 6" to spell out the Archive/hide terminology split so future contributors don't try to rename the on-wire blob. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three issues caught by Codex's review of PR #275: **P1: `ARCHIVE_TOAST.read()` violated the repo's signal-safety rule.** On Firefox/mobile, the toast write's Drop handler fires subscriber notifications synchronously, which could re-enter the rail's read while the write guard's RefCell borrow is still held. Switched to `try_read().ok().and_then(...)` — same pattern AGENTS.md mandates for GlobalSignal reads everywhere else. **P2: "Archived (N)" count and viewer didn't apply the revival filter.** A hidden entry whose thread has since been revived by a strictly-newer message is correctly shown on the rail (filter pass) but stayed in `HIDDEN_DM_THREADS`, so the naive `len()` count and the `build_archived_rows` projection both treated it as still archived — confusing the user about whether the thread was archived or not. Fix: - Added `current_archived_count()` (walks ROOMS, computes per-pair `last_any_ts`, applies `is_thread_hidden_for`) and use_memo'd it in the rail. - `build_archived_rows` now takes a `last_any_ts` map and drops revived entries before projecting display data. - New `count_currently_archived` pure helper backs the count and is pinned by `count_currently_archived_keeps_stale_hidden_entries`. - New `build_archived_rows_skips_revived_thread` pins the viewer-side invariant. **P2: Confirmation dialog had no focus trap.** The `onkeydown: Escape` handler was on the wrapping div, but the div had no `tabindex`, so it never captured focus on mount. After a backdrop click or any Tab cycle out of the dialog buttons, Escape stopped working. Fix: applied the existing `member_info_modal.rs` pattern — `tabindex: "0"` + `onmounted -> set_focus` puts focus on the wrapper on mount, so Escape always fires. Also added `prevent_default` so a future Escape handler on the outer DM modal doesn't double-fire. Reviewer summary: 2 BLOCKING (P1 signal-safety + P2 archived-state consistency), 1 IMPORTANT (P2 dialog Escape). All addressed in this commit; the original review's no-issue findings (sibling-button structure, toast expiry identity check, etc.) needed no change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex round-2 review on PR #275 noted the focus-trap fix from the previous commit only puts initial focus on the wrapping div — Tab can still cycle focus out (to Cancel, Delete, then onward to the composer textarea), at which point Escape no longer reaches the dialog-scoped `onkeydown`. Implementing a full focus trap is significant complexity for a 2-button dialog. The simpler fix: add an Escape backstop on the outer modal's wrapping div, gated on `confirm_delete_open` being true. KeyDown events on any child element bubble up to this handler, so Escape closes the dialog regardless of which element currently has focus. The dialog-scoped handler still runs first when its subtree has focus (it fires `prevent_default + stop_propagation`), so the new backstop only kicks in when the dialog-scoped handler missed it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
Multi-model review — round 1 + 2Ran the four-perspective code-first / testing / skeptical / big-picture analysis plus a Codex CLI pass per Round 1 (Codex + internal)Three issues, all addressed in commit
Round 2 (Codex re-review on
|
The previous version of `dm-archive-ux.spec.ts` asserted that no console error matching `/panic|wasm|RefCell/i` was logged during page load. CI tripped on a pre-existing wasm-bindgen warning from the example-data startup path: "wasm-bindgen: imported JS function that was not marked as `catch` threw an error: expected a string argument, found undefined" This is unrelated to PR #275's archive UX changes — same warning fires on `main`. The regex caught it because the warning text starts with "wasm-bindgen:". Two reasonable fixes: (a) narrow the filter to only flag actual panic markers, or (b) drop the assertion. Going with (b) — no other spec in `ui/tests/` does console-error filtering, so this one was already an outlier. The remaining assertions (DM rail hidden when no DMs exist, "Hide" button no longer rendered) cover the no-regression invariants the PR needs to pin. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
M1: use try_read() instead of read() on confirm_delete_open in the outer-modal Escape backstop. confirm_delete_open is a local use_signal (not a GlobalSignal) so practical re-entrancy risk is low, but AGENTS.md Dioxus signal-safety rule is universal and the round-1 ARCHIVE_TOAST fix established the pattern — leaving one .read() inconsistent. M2: replace expires_at_ms-based toast identity with a monotonic atomic-counter token. Previously two same-millisecond archives produced identical expires_at_ms; the first auto-dismiss tick could clear the second toast early. Token comes from ARCHIVE_TOAST_TOKEN.fetch_add(1, Relaxed) so collisions are structurally impossible. Added regression test build_archive_toast_same_ms_has_distinct_tokens. M3: write ARCHIVE_TOAST BEFORE calling hide_dm_thread in archive_row. Previously the order was hide-first / toast-second, both as separate defers. The render between the two defers can satisfy the rail's empty-state early-return predicate when the user archives their LAST DM thread — rail unmounts, toast write arrives at a detached component, user sees no confirmation. Writing the toast first keeps toast.is_none() = false at every intermediate render so the rail stays mounted across the hide. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sanity
added a commit
that referenced
this pull request
May 17, 2026
## Problem
The "Share an invite via DM" flow pastes an invitation URL as plain
text into the recipient's DM thread. The recipient clicks → full
browser navigation → page reload, losing UI state. The wire body
conflates intent with text, riverctl renders just the URL with no
card, and URL-format changes would break detection.
## Approach
Encode the DM body as a structured `DirectMessageBody` enum carried
INSIDE the ECIES ciphertext (the contract is indifferent — it only
validates the outer envelope). New wire format prepends magic byte
`0x80` (a UTF-8 continuation byte that can never lead a valid
plaintext string) + CBOR of the enum. Bytes without the magic decode
as `Text` via lossy UTF-8 → existing plaintext DMs keep rendering.
Two variants:
* `Text { text }` — equivalent to today's body.
* `Invite { room_owner_vk, invitation_payload, personal_message }` —
the structured invite-card variant.
Phase 1 lands the wire format + picker rewrite + accept-flow
extraction. `dm_thread_modal.rs` (which Archive UX is actively
rewriting in PR #275) stays untouched here; Invite-card rendering
and auto-scroll fold-in come in Phase 3 after Archive merges.
### Picker rewrite
`InviteViaDmPickerModal` becomes the composer: room dropdown +
optional personal-message textarea + Send button. Sends the
`Invite` variant DM directly via the new `send_structured_dm`
helper — no `DM_DRAFT` paste, no thread-modal hand-off in the
middle of the flow. On success the picker shows an inline toast
and closes the parent member-info modal.
### Accept-flow extraction
`receive_invitation_modal::present_invitation(Invitation)` is the
new public entry point. It writes to a new `PRESENT_INVITATION_REQUEST`
GlobalSignal that `App` bridges into the local `receive_invitation`
signal driving the modal. The DM-thread Accept button (Phase 3) will
call `present_invitation` — same modal, same nickname flow, no full
page reload.
### Why no contract migration entry
`DirectMessageBody` lives in client-decrypted plaintext, NEVER in the
contract WASM validation path. The room contract sees opaque ciphertext.
Adding the enum to `river-core` doesn't add anything to
`ChatRoomStateV1::verify` / `apply_delta`. The contract WASM hash is
unchanged.
## Testing
* 10 new unit tests in `common/src/room_state/dm_body.rs` covering
round-trips for both variants, legacy text fallback, malformed-body
err, magic-byte invariant, empty-body handling.
* Full common-crate test suite: 168 passed (was 158).
* UI compiles with `cargo make build-ui-example-no-sync` — minor warns
for the not-yet-wired `present_invitation` helper (used in Phase 3).
* Pre-existing rsx! macro continues compiling against the rewritten
picker.
Outstanding for Phase 3 (after Archive UX PR #275 merges):
- Render `DirectMessageBody::Invite` as an in-thread card with Accept
button in `dm_thread_modal.rs`.
- Auto-scroll behaviour: bottom on mount, bottom on send, conditional
bottom on inbound.
- Refactor `do_send` in `dm_thread_modal.rs` to use the shared
`send_structured_dm` helper.
[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
## Problem
The "Share an invite via DM" flow pastes an invitation URL as plain
text into the recipient's DM thread. The recipient clicks → full
browser navigation → page reload, losing UI state. The wire body
conflates intent with text, riverctl renders just the URL with no
card, and URL-format changes would break detection.
## Approach
Encode the DM body as a structured `DirectMessageBody` enum carried
INSIDE the ECIES ciphertext (the contract is indifferent — it only
validates the outer envelope). New wire format prepends magic byte
`0x80` (a UTF-8 continuation byte that can never lead a valid
plaintext string) + CBOR of the enum. Bytes without the magic decode
as `Text` via lossy UTF-8 → existing plaintext DMs keep rendering.
Two variants:
* `Text { text }` — equivalent to today's body.
* `Invite { room_owner_vk, invitation_payload, personal_message }` —
the structured invite-card variant.
Phase 1 lands the wire format + picker rewrite + accept-flow
extraction. `dm_thread_modal.rs` (which Archive UX is actively
rewriting in PR #275) stays untouched here; Invite-card rendering
and auto-scroll fold-in come in Phase 3 after Archive merges.
### Picker rewrite
`InviteViaDmPickerModal` becomes the composer: room dropdown +
optional personal-message textarea + Send button. Sends the
`Invite` variant DM directly via the new `send_structured_dm`
helper — no `DM_DRAFT` paste, no thread-modal hand-off in the
middle of the flow. On success the picker shows an inline toast
and closes the parent member-info modal.
### Accept-flow extraction
`receive_invitation_modal::present_invitation(Invitation)` is the
new public entry point. It writes to a new `PRESENT_INVITATION_REQUEST`
GlobalSignal that `App` bridges into the local `receive_invitation`
signal driving the modal. The DM-thread Accept button (Phase 3) will
call `present_invitation` — same modal, same nickname flow, no full
page reload.
### Why no contract migration entry
`DirectMessageBody` lives in client-decrypted plaintext, NEVER in the
contract WASM validation path. The room contract sees opaque ciphertext.
Adding the enum to `river-core` doesn't add anything to
`ChatRoomStateV1::verify` / `apply_delta`. The contract WASM hash is
unchanged.
## Testing
* 10 new unit tests in `common/src/room_state/dm_body.rs` covering
round-trips for both variants, legacy text fallback, malformed-body
err, magic-byte invariant, empty-body handling.
* Full common-crate test suite: 168 passed (was 158).
* UI compiles with `cargo make build-ui-example-no-sync` — minor warns
for the not-yet-wired `present_invitation` helper (used in Phase 3).
* Pre-existing rsx! macro continues compiling against the rewritten
picker.
Outstanding for Phase 3 (after Archive UX PR #275 merges):
- Render `DirectMessageBody::Invite` as an in-thread card with Accept
button in `dm_thread_modal.rs`.
- Auto-scroll behaviour: bottom on mount, bottom on send, conditional
bottom on inbound.
- Refactor `do_send` in `dm_thread_modal.rs` to use the shared
`send_structured_dm` helper.
[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
…278) * feat(dm): Phase 1 — structured invite-DM variant (wire + picker rewrite) ## Problem The "Share an invite via DM" flow pastes an invitation URL as plain text into the recipient's DM thread. The recipient clicks → full browser navigation → page reload, losing UI state. The wire body conflates intent with text, riverctl renders just the URL with no card, and URL-format changes would break detection. ## Approach Encode the DM body as a structured `DirectMessageBody` enum carried INSIDE the ECIES ciphertext (the contract is indifferent — it only validates the outer envelope). New wire format prepends magic byte `0x80` (a UTF-8 continuation byte that can never lead a valid plaintext string) + CBOR of the enum. Bytes without the magic decode as `Text` via lossy UTF-8 → existing plaintext DMs keep rendering. Two variants: * `Text { text }` — equivalent to today's body. * `Invite { room_owner_vk, invitation_payload, personal_message }` — the structured invite-card variant. Phase 1 lands the wire format + picker rewrite + accept-flow extraction. `dm_thread_modal.rs` (which Archive UX is actively rewriting in PR #275) stays untouched here; Invite-card rendering and auto-scroll fold-in come in Phase 3 after Archive merges. ### Picker rewrite `InviteViaDmPickerModal` becomes the composer: room dropdown + optional personal-message textarea + Send button. Sends the `Invite` variant DM directly via the new `send_structured_dm` helper — no `DM_DRAFT` paste, no thread-modal hand-off in the middle of the flow. On success the picker shows an inline toast and closes the parent member-info modal. ### Accept-flow extraction `receive_invitation_modal::present_invitation(Invitation)` is the new public entry point. It writes to a new `PRESENT_INVITATION_REQUEST` GlobalSignal that `App` bridges into the local `receive_invitation` signal driving the modal. The DM-thread Accept button (Phase 3) will call `present_invitation` — same modal, same nickname flow, no full page reload. ### Why no contract migration entry `DirectMessageBody` lives in client-decrypted plaintext, NEVER in the contract WASM validation path. The room contract sees opaque ciphertext. Adding the enum to `river-core` doesn't add anything to `ChatRoomStateV1::verify` / `apply_delta`. The contract WASM hash is unchanged. ## Testing * 10 new unit tests in `common/src/room_state/dm_body.rs` covering round-trips for both variants, legacy text fallback, malformed-body err, magic-byte invariant, empty-body handling. * Full common-crate test suite: 168 passed (was 158). * UI compiles with `cargo make build-ui-example-no-sync` — minor warns for the not-yet-wired `present_invitation` helper (used in Phase 3). * Pre-existing rsx! macro continues compiling against the rewritten picker. Outstanding for Phase 3 (after Archive UX PR #275 merges): - Render `DirectMessageBody::Invite` as an in-thread card with Accept button in `dm_thread_modal.rs`. - Auto-scroll behaviour: bottom on mount, bottom on send, conditional bottom on inbound. - Refactor `do_send` in `dm_thread_modal.rs` to use the shared `send_structured_dm` helper. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(dm): riverctl decodes Invite variant + formats as compact card `riverctl dm list` now runs every inbound DM body through `river_core::room_state::dm_body::decode_body` and renders Invite- variant bodies as a one-line "[Invitation to room <prefix>…] <message>" card instead of garbled CBOR bytes. Text-variant DMs go through the legacy raw-UTF-8 path (`dm send` keeps calling `compose_direct_message` with `message.as_bytes()` directly, no magic byte prefix) — older `riverctl` builds in the wild continue to render those correctly. Only Invite variants opt into the magic-byte + CBOR wire shape, which old clients can't render anyway. 4 new unit tests pin the CLI render path for Text and Invite (with / without / blank personal message). [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(dm): Playwright smoke test for redesigned invite-via-DM picker Pins the picker's new visible structure after the rewrite to the structured Invite-DM variant: * Header reads "Invite <nickname> to another room" * Personal-message textarea is present * Send button starts disabled, enables after a room is selected * Candidate rows reflect aria-pressed selection state * Close button dismisses the picker End-to-end "send → recipient renders card → click Accept → modal opens" is not exercised here because no-sync mode has no chat delegate to persist the outbound DM. That path is covered by the Rust unit tests on `send_structured_dm` outcomes + by manual verification against a live local node before merging. The spec test.skip()s cleanly if example data doesn't surface the "Share an invite via DM…" entry point (e.g. local user is observer-only in every loaded room). [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(dm-body): add determinism, large-payload, invalid-UTF-8 coverage Four additional unit tests: * encoding_is_deterministic_for_text / _for_invite — pin that `encode_body` produces stable bytes across calls. ciborium emits canonical CBOR today; this catches a future version bump that loosens it. * invite_with_large_payload_round_trips — 16 KiB invitation payload round-trips cleanly (well above typical encoded `Invitation` sizes but inside the body cap). * legacy_text_with_invalid_utf8_decodes_lossily — defensive: any pre-format DM with mangled UTF-8 still surfaces as `Text` via `from_utf8_lossy`. 14 dm_body tests total; 172 common-crate tests passing. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(dm-body): box `Invite` variant payload to silence large_enum_variant `clippy::large_enum_variant` was firing on `DirectMessageBody`: the `Invite` variant carried `VerifyingKey` (32 bytes plus alignment), `Vec<u8>` (24 bytes), and `Option<String>` (24 bytes) all inline, giving the enum a 240-byte stack footprint regardless of which variant was active. Wrapped the variant's inner fields in a boxed `InvitePayload` sub-struct. ciborium serialises `Box<T>` exactly the same as `T`, so the wire format is unchanged (round-trip tests still pass on the same byte representation). All 14 dm_body unit tests + 10 CLI tests still pass; clippy is now clean on river-core / riverctl. Pre-existing UI clippy warns are unrelated to this work. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(dm): Phase 3 — render invite-DM card + auto-scroll Inbound DMs with `body: DirectMessageBody::Invite(payload)` now render as a structured "Invitation" card with the target room name, an optional personal message, and an Accept button. Accept decodes the embedded CBOR `Invitation` payload and hands off via the existing `present_invitation` entry point — the same code path the URL-bar accept flow uses, so there's exactly one invitation-accept implementation. When the local user is already a member of the target room the Accept button is disabled and relabelled "Already a member". A room-key mismatch between the card's outer `room_owner_vk` and the embedded invitation's `room` field is rejected at decode time so a malicious sender can't mislabel the destination. The DM thread modal also gets auto-scroll behaviour: 1. Modal mount → jump to bottom instantly. 2. Outbound send → scroll to bottom smoothly. 3. Inbound new message → scroll only when the user is within ~50px of the bottom (don't yank readers of history). Implementation lives in a single `use_effect` watching the rendered message count and a monotonic outbound-send counter, plus a stable `id="dm-scroll-container"` on the thread body. Pattern adapted from `conversation.rs`'s chat-scroll-container. Outbound invite-DMs continue to render via the existing OUTBOUND_DMS plaintext-summary path (`[Invitation] …`) because the cache stores only the summary string, not the structured body. Extending the cache to carry the body would be additive but out of scope for this PR. Tests: * Rust unit tests pin the pure helpers — `decode_invitation_from_payload` accepts well-formed matching CBOR, rejects room-key mismatches, rejects invalid CBOR, and rejects empty bytes; `short_vk_prefix` pins the 8-char prefix length. * Playwright smoke tests pin the structural surface — stable scroll container id, empty-state copy, composer + Send button state. Full end-to-end "recipient sees card → click Accept → modal opens" coverage requires a real Freenet node (no-sync mode can't synthesise an inbound invite-variant DM). * Fixed flaky member-info selector in the existing `invite-via-dm-picker.spec.ts` — the previous `aria-label^="Open member info"` selector matched 0 elements; member rows actually use `title="Member ID: …"`. Also handle the random "(You)" entry position in example-data. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(dm): address review findings on Phase 3 invite card + auto-scroll Two issues surfaced during multi-model review: 1. Self-review (code-first): the "Already a member" check used `rooms.map.contains_key(payload.room_owner_vk)` — but that also matched rooms the local user has loaded as an Observer (not a member). Switched to `RoomData::can_participate()`, the canonical membership check used elsewhere in the codebase. An invite to a room where the user is Observer-only now shows the Accept button enabled (correctly — they CAN accept and become a member), not the "Already a member" disabled state. 2. Codex P2: the auto-scroll effect previously did `*OUTBOUND_SEND_COUNTER.read()` inside the use_effect body to subscribe to bumps. Per AGENTS.md "Dioxus WASM Signal Safety Rules", a subscriber's `.read()` can panic with `RefCell already borrowed` when the write that triggered the notification still holds the write-guard borrow — which can happen on Firefox/mobile because Dioxus runs subscriber notifications synchronously during the write guard's Drop. Switched to `.peek()` (non-reactive) and document that the reactive subscription comes via the parent memo's ROOMS read (which is guaranteed to fire on every DM because apply_delta and the counter write happen in the same defer block). Tests: 10 existing unit tests still pass. No new tests needed — both changes preserve existing behaviour for the test paths and the new behaviour is documented in the code. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(dm): split OUTBOUND_SEND_COUNTER read/write to avoid borrow overlap Codex P1 finding on round-2 review of PR #278: the statement `*OUTBOUND_SEND_COUNTER.write() = OUTBOUND_SEND_COUNTER.peek().wrapping_add(1);` lets the read guard and the write guard temporaries overlap until the statement ends, risking a RefCell/Dioxus borrow panic on the send- success path. Fix: compute the next value in a separate `let` statement first, so the read guard drops before the write guard is taken. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(dm): route send_structured_dm preflight reads through defer Codex P1 finding on round-3 review of PR #278: `send_structured_dm` is called from `safe_spawn_local` contexts (notably the invite-via-DM picker's `drive_send`), which doesn't push the Dioxus runtime onto the call stack. The preflight `ROOMS.try_read()` at the top of the function therefore ran outside the Dioxus runtime and could panic with "Must be called from inside a Dioxus runtime" on the new invite flow. Fix: move the entire preflight (ROOMS read + peer-vk resolution + per-pair cap check + rejoin-bundle build) inside a `defer()` block, the same way the apply-delta write was already routed. A oneshot channel funnels the result back to the awaiting caller. Boxed the larger `Ready` variant of `PreflightOutcome` to satisfy `clippy::large_enum_variant` (the snapshot holds a SigningKey + VerifyingKey + two Option<…> rejoin fields, > 100 bytes). Tests: 153 unit tests pass (15 in the direct_messages tree). The preflight logic is unchanged — just relocated — and the existing SendDmOutcome shapes are preserved on every rejection branch. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(dm): gate picker-local signal writes on still-mine generation Codex P2 finding on round-4 review of PR #278: the invite-via-DM picker's terminal `defer` (after `drive_send` returns) calls `last_success_label.set(...)` and `send_error.set(...)` — these are component-local `use_signal`s. If the watchdog fired before the task completed (slow signing or send), `INVITE_VIA_DM_PICKER_INFLIGHT` was cleared, the user could then close the picker, dropping those signals; the late `.set()` calls would then panic. Fix: capture `still_mine = (INFLIGHT generation == my_generation)` before clearing INFLIGHT. If `still_mine`, do the full picker-local + global cleanup; otherwise only log and skip the local-signal writes. The send itself already landed locally (ROOMS write inside `send_structured_dm`) so the network sync queue delivers the invite regardless of UI state. Tests: 153 unit tests pass. The watchdog race is hard to reproduce deterministically in a unit test (requires Dioxus runtime + a real component lifecycle); behaviour pinned by the generation-check pattern established earlier in the file. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(dm): watchdog force-closes picker to give user clear feedback Codex P2 finding on round-5 review of PR #278: the previous shape (watchdog clears INFLIGHT but leaves picker open + still_mine gate silences late completion UI updates) left the user with zero feedback in the timeout-then-late-success scenario, and could result in duplicate invite sends on retry. Fix: have the watchdog force-close the picker on expiry. Every late completion then finds the picker already unmounted, the still_mine gate (from round-4 fix) skips the .set() calls (no panic), and the user sees a closed modal — a clear "something happened, check the DM thread" signal. Trade-off: user loses their typed personal message on timeout. Strictly better than the prior shape (no feedback + possible duplicate sends). Tests: 153 pass. The timeout edge case still isn't deterministically reproducible in a unit test (requires Dioxus runtime + a Future that hangs past PICKER_WATCHDOG_SECS = 15s), but the existing generation- check pattern is exercised by the merge_invite_into_draft tests. [AI-assisted - Claude] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(dm): clean up stale comments from PR #278 review Pure comment-only changes; no functional change. Per multi-model-review.md, pure-additive commits with clearly trivial scope (comment edits) are exempt from re-review. - direct_messages.rs:288 — removed reference to non-existent test `send_structured_dm_text_matches_modal`. Replaced with a clearer note that the equivalence is hand-verified and document the no-refactor-without-recheck constraint. - dm_thread_modal.rs:336-342 — fixed stale block comment that said OUTBOUND_SEND_COUNTER is read via `.read()` (the Codex round-1 fix swapped to `.peek()` but didn't update this comment). The per-line comment below already explains `.peek()` correctly; the block comment now matches. - app.rs (bridge effect) — added a comment explaining why `try_read() -> Err` on the first render is benign: writers of PRESENT_INVITATION_REQUEST always defer via setTimeout(0), so the writer is never holding the borrow at the moment the effect first reads. Pinned the "writes always defer" invariant so a future caller doesn't break the assumption silently. Filed #279 (in-app accept re-shows nickname flow for previously-LeaveRoom'd rooms) and #280 (banned users see enabled Accept button on invite card) as follow-up UX issues per the skeptical review's Important findings. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- 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 this session's PRs: - #275 (Archive UX overhaul): Hide→Archive rename, rollover ✕ on rail rows, Archived (N) viewer, undo toast, removed modal Hide button, added "Delete their messages" confirm modal. - #276 (Bug #6): delegate now reliably emits encrypted_secrets for new members on first sync — fixes the "[Encrypted message - secret v0 not available]" symptom Ivvor reported. Adds request_id to EnsureRoomSubscription wire format (Migration V23). - #278 (Invite redesign + auto-scroll): structured Invite DM variant (dm_body magic+CBOR), in-app accept (no page reload), invitation card UI in the DM thread modal, auto-scroll on mount/send/inbound. river-core 0.1.9 published to crates.io (new types from #276 and #278). riverctl 0.1.60 published to crates.io alongside the UI publish. 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
Two follow-ups to the in-room DM work (#243 / #265):
Neither was a data-loss bug, but both were UX traps that real users tripped on the day after #261 shipped.
Approach
UI-only changes; no contract / delegate / WASM touched. Keeps the on-wire shape (
OutboundDmStore.hidden_threads) and the internal Rust API names (hide_dm_thread,HIDDEN_DM_THREADS, etc.) — renaming them would force a delegate migration for zero functional benefit. The UX surface is renamed everywhere visible: "Archive" matches Gmail / WhatsApp / Signal convention.Changes:
DmRailSection. Desktop: hidden until row hover/focus (group-hover:opacity-100 group-focus-within:opacity-100). Mobile (<md): dimmed always-visible (opacity-40) so touch users keep the affordance. Tooltip: "Archive this conversation. It will return if either of you sends a new DM."z-50, with Undo and Dismiss buttons. The auto-dismiss reaction uses anexpires_at_msidentity check so a rapid second archive doesn't cancel the second toast early.purge_threadflow.References #261. Closes #266.
Testing
Unit tests (Rust, run by
cargo make test)New tests in
dm_rail_section:build_archived_rows_projects_and_sorts— archived viewer's per-room display data projection, including the (room_name, peer_nickname) sort order and the short-id fallback for missing nicknames.build_archived_rows_falls_back_when_room_missing— a thread whose owning room is no longer inROOMS(e.g. user left the room) still surfaces in the viewer with the "(unknown room)" placeholder, so the user can still un-archive.build_archive_toast_advances_expiry_per_call— pins the auto-dismiss identity-check math: back-to-back archives must produce distinctexpires_at_msso the first toast's tick doesn't cancel the second's.build_archive_toast_saturates_on_overflow— defensive:Date.now()nearu64::MAXdoesn't wrap to a toast that auto-dismisses instantly.All existing
filter_rail_entries_*regression tests still pass (no behavioural change to the filter; the new UX feeds the samehide_dm_thread(cutoff)cutoff as the old one did).Playwright (
ui/tests/dm-archive-ux.spec.ts)group-hover/md:opacity-*classes.Test plan
cargo make test— all 4 new unit tests pass, no regressionscargo fmt --all -- --checkcleancargo check -p river-ui --target wasm32-unknown-unknown --features no-syncpassescargo make check-migrationclean)End-to-end Playwright coverage of the archive / un-archive flow is deliberately out of scope: the example-data build has no DMs in its fixture, and adding real ECIES-sealed DMs to example data was more invasive than the value it would add given the strong unit-test coverage above.
[AI-assisted - Claude]