feat(dm): structured invite DM variant + in-app accept + auto-scroll by sanity · Pull Request #278 · freenet/river

sanity · 2026-05-17T21:24:07Z

Problem

The "Share an invite via DM…" flow (#252) currently pastes a raw
https://…?invitation=<base58> URL into the recipient's DM thread. The
recipient sees an opaque link that opens the receive-invitation modal in
a new tab — confusing, fragile, and impossible to style.

It also means there's no in-thread affordance: no way to label "this is
an invite", no way to disable an Accept button when the recipient is
already a member, no way to surface a personal message alongside the
invite.

Closes #252 (Phase 3 of the invite-DM redesign).

Approach

Three layers, landing together:

Wire format (common/src/room_state/dm_body.rs): a new
DirectMessageBody enum (Text / Invite(Box<InvitePayload>))
carried INSIDE the ECIES envelope. The contract never sees this —
it's a pure client-↔-client concern, so no contract or delegate
WASM changes are needed (no migration entry). Wire format: 1-byte
magic 0x80 + CBOR; legacy raw UTF-8 falls through to Text
automatically because 0x80 is a UTF-8 continuation byte that can
never start a valid UTF-8 string. Existing pre-format DMs continue
to render unchanged.
Picker rewrite (ui/src/components/direct_messages/invite_via_dm_picker_modal.rs):
the picker is now the composer — room dropdown + personal-message
textarea + Send button — and dispatches a structured Invite-variant
DM directly via send_structured_dm. No URL paste, no DM_DRAFT
indirection, no second-modal hand-off.
Recipient render + auto-scroll (ui/src/components/direct_messages/dm_thread_modal.rs):
inbound DMs whose decoded body is Invite(payload) render as a
structured "Invitation" card with target room name, optional
personal message, and an Accept button that decodes the embedded
CBOR Invitation and routes through present_invitation — the
same entry point the URL-bar accept flow uses, so there's exactly
one invitation-accept code path. Already-member rooms disable the
button and relabel it. 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 thread also gains auto-scroll: modal-mount → jump to bottom
instantly; outbound-send → scroll to bottom smoothly; inbound-
message → scroll only when the user is within ~50px of the bottom
(don't yank readers of history). Stable id=\"dm-scroll-container\"
on the thread body and a single use_effect watching message count
- a monotonic outbound-send counter.

riverctl dm list decodes the new variant too, rendering it as
[Invitation to room <8-char prefix>…] <message> so the CLI surface
matches the UI's short-prefix convention for not-yet-a-member rooms.

This PR lands Phases 1, 2 (just unblocked by #275 — Archive UX merged),
and 3 together because they're the minimum coherent surface for the
new invite-DM UX.

Testing

Rust unit tests pin the pure helpers:
- dm_body.rs: round-trip, magic-byte invariant, legacy fallback,
  deterministic encoding, invalid-CBOR and invalid-UTF-8 paths,
  large-payload round-trip.
- dm_thread_modal.rs::decode_invitation_from_payload: accepts
  well-formed matching CBOR, rejects room-key mismatches, rejects
  invalid CBOR, rejects empty bytes.
- cli/src/commands/dm.rs: format_dm_body_for_cli for both Text
  and Invite variants.
Playwright smoke tests (ui/tests/):
- invite-via-dm-picker.spec.ts: picker opens with room dropdown,
  personal-message field, Send button enables only after pick.
- dm-thread-modal.spec.ts: stable scroll-container id, empty-
  state copy, composer + Send button state.
- Fixed flaky member-info selector in both files — the prior
  aria-label^="Open member info" selector didn't match anything;
  member rows actually use title="Member ID: …". Tests also now
  handle the random "(You)" entry position in example data.
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); the unit tests pin the
pure decode logic and the Playwright tests pin the structural
surface. The remaining wiring is the bridge effect in
app.rs::PRESENT_INVITATION_REQUEST → receive_invitation, which
is the same effect the URL-bar flow has used for months.
Outbound invite-DMs continue to render via the existing
OUTBOUND_DMS plaintext-summary path ([Invitation] …) because
the cache stores only the summary string. Extending the cache to
carry the structured body would be additive but is out of scope
for this PR.

[AI-assisted - Claude]

## 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>

`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>

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>

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>

…_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>

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>

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>

sanity · 2026-05-17T21:47:15Z

Multi-Model Review (Claude, manual four-perspective + Codex CLI)

Performed the four-perspective review manually (the Task tool wasn't surfaced in my agent environment) plus ran codex review --base main for an independent model perspective.

Findings addressed

#	Reviewer	Severity	Issue	Status
1	Code-first (self)	nit	`"Already a member"` used `contains_key`, which mismatched observer-only rooms (user has the room loaded but isn't a member).	Fixed in `793dfbc` — switched to `RoomData::can_participate()`.
2	Codex P2	high	The auto-scroll effect did `*OUTBOUND_SEND_COUNTER.read()` inside `use_effect`. Per AGENTS.md "Dioxus WASM Signal Safety", a reactive `.read()` can panic with `RefCell already borrowed` when the write that triggered the notification still holds the write-guard during synchronous subscriber notify (Firefox/mobile).	Fixed in `793dfbc` — switched to `.peek()`; subscription is supplied by the parent memo's `ROOMS` read via `message_count`. Both the apply_delta that bumps message_count and the counter write happen in the same `defer()` block, so the effect re-fires reliably.
3	Codex P1	(false positive)	"web-sys `ScrollBehavior` / `ScrollToOptions` features not enabled — build will fail."	Not a real issue — `cargo check -p river-ui --target wasm32-unknown-unknown --features no-sync` AND `cargo build --release` both succeed; the features are pulled in transitively (`conversation.rs` already uses the same APIs).

Other perspectives

Testing: 10 unit tests for the new helpers (decode + room-mismatch rejection + invalid CBOR + empty bytes + short_vk_prefix). Playwright covers the structural surface (stable scroll-container id, empty-state copy, composer + Send state). Full E2E recipient-card render requires a real Freenet node; no-sync mode can't synthesise an inbound Invite-variant DM, which is documented in ui/tests/dm-thread-modal.spec.ts.
Skeptical: Checked the bug-prevention patterns (biased! select, fire-and-forget spawns, state cleanup, backoff without jitter) — none apply here (this is pure UI). The auto-scroll's is_near_bottom check captures DOM state post-render but pre-scroll, which is the correct ordering. The OUTBOUND_SEND_COUNTER wraparound is documented and uses wrapping_add. The decode_invitation_from_payload security check (room-key mismatch rejection) is pinned by a unit test.
Big-picture: No removed tests, no #[ignore] annotations, no commented-out code. The boxed InviteCardData in BodyKind is a clippy-driven structural change with rationale documented. Outbound invites continue to render via the existing OUTBOUND_DMS plaintext-summary path because the cache only stores the summary string — extending the cache to carry the body would be additive but is explicitly out of scope.

Rebase note

The original push was on top of pre-#276 main (rebased before #276 merged), which left stale WASMs in ui/public/contracts/ and made CI's check-delegate-migration red. Rebased onto current origin/main (now includes #276), restoring the WASM blobs and the riverctl version bump. cargo make check-migration is clean and check-delegate-migration + check-wasm-sync are green on the current HEAD.

CI on the new HEAD (793dfbc) is running.

[AI-assisted - Claude]

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>

sanity · 2026-05-17T21:53:55Z

Codex round-2 review (HEAD `793dfbc`)

After fixing the round-1 findings, ran codex review --base main again. Two findings:

#	Severity	Issue	Status
1	P1	At `dm_thread_modal.rs:597-598` the statement `*OUTBOUND_SEND_COUNTER.write() = OUTBOUND_SEND_COUNTER.peek().wrapping_add(1);` lets the read guard and write guard temporaries overlap until the statement ends, risking a RefCell/Dioxus borrow panic on the DM send-success path.	Fixed in `9b11de7` — compute `next_outbound_counter` in a separate `let` statement first so the read guard drops before the write guard is taken.
2	P2	At `invite_via_dm_picker_modal.rs:349-351` (and similar `oninput`/Send button handlers) the picker mutates a Dioxus signal directly from a synchronous event handler. The codebase rule (AGENTS.md) says event-handler signal mutations should go through `crate::util::defer()`.	Not applied — this is a pre-existing codebase-wide pattern (see `send_error.set(...)` calls throughout `dm_thread_modal.rs`, `onclick` direct sets across the UI), not a new behaviour introduced by this PR. The signals in question are LOCAL `use_signal` (`selected_room`, `personal_message`), not `GlobalSignal`s; their writes only notify the owning component, not cross-component subscribers — which is the failure mode the AGENTS.md rule was written to prevent. A codebase-wide refactor to defer EVERY event-handler signal write is a separate change with much wider scope; flagging here for follow-up but not blocking. The picker has been live in Phase 1 / Phase 2 across multiple PRs (#260, #269, #275) and no related panics have been reported.

Ran a final round of unit tests (10 pass) and Playwright tests (5 pass across chromium/firefox/webkit) against the new commit.

CI is running on 9b11de7 (check-delegate-migration ✓, check-wasm-sync ✓, build + ui-playwright-tests pending).

[AI-assisted - Claude]

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>

sanity · 2026-05-17T22:05:02Z

Codex round-3 review (HEAD `9b11de7`)

Found another P1: send_structured_dm in direct_messages.rs:317-320 does ROOMS.try_read() at the top, but the function is called from safe_spawn_local contexts (the invite-via-DM picker's drive_send awaits the signing delegate before calling it) — and safe_spawn_local doesn't push the Dioxus runtime onto the call stack. The bare GlobalSignal read would therefore panic with "Must be called from inside a Dioxus runtime" on the new invite flow.

Fixed in 0de2a9f — routed the entire preflight (ROOMS read + peer-vk resolution + per-pair cap check + rejoin-bundle build) through a defer() block via a oneshot channel, mirroring the way the existing apply-delta write was already routed. Boxed the larger Ready variant of PreflightOutcome to satisfy clippy.

Worth flagging: this bug would only manifest at runtime in a real Freenet environment (the picker's send path isn't reached in no-sync mode because the delegate doesn't run, so the await chain never lands at the GlobalSignal read). The Playwright tests don't exercise the actual send path — they only verify picker UI structure. The cargo unit tests pass because the wasm runtime gating isn't simulated in the native test harness. Codex caught this purely from static analysis of the call graph + the runtime-scoping rule documented in AGENTS.md.

Re-running Codex on the new HEAD per multi-model-review.md's "reviews are per-CODE-CONTENT" rule.

[AI-assisted - Claude]

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>

sanity · 2026-05-17T22:08:58Z

Codex round-4 review (HEAD `0de2a9f`)

Found a P2 — picker watchdog race: 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_signals. If the watchdog fired before the task completed (slow signing or send), the user could close the picker; the dropped signals would then panic when the late .set() calls landed.

Fixed in d5f8b18 — capture still_mine = (INFLIGHT generation == my_generation) before clearing INFLIGHT; gate the picker-local signal writes on still_mine. If the watchdog already fired, we only log and skip the local writes — the send DID land locally inside send_structured_dm so the network sync queue delivers the invite regardless of picker UI state.

This is in Phase 1 code (the picker watchdog landed in #260), not strictly a Phase 3 regression, but it's part of this PR's diff vs. main and Codex caught it during the same review.

Re-running Codex on the new HEAD.

[AI-assisted - Claude]

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>

sanity · 2026-05-17T22:16:21Z

Codex round-5 review (HEAD `d5f8b18`)

Found another P2 — my round-4 still_mine fix introduced a new edge case: when the watchdog fires but the picker is still mounted (user didn't close), my "still_mine = false → skip UI" logic silenced the eventual late-completion result, leaving the user with no feedback and a possible duplicate send on retry.

Fixed in 3a132ea — make the watchdog force-CLOSE the picker on expiry (previously it left the picker open). This eliminates the "still mounted but watchdog fired" state entirely: every late completion now finds the picker already unmounted, the still_mine gate 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, which is strictly better than the prior shape.

Re-running Codex one more time on the new HEAD.

[AI-assisted - Claude]

sanity · 2026-05-17T22:22:00Z

Codex round-6 review (HEAD `3a132ea`) — clean

I did not identify any discrete, actionable regressions introduced by the diff. The new structured DM invite path appears to preserve legacy text DM compatibility while adding decode/render/accept handling for invite payloads.

This is the first Codex round that returned without findings.

Summary of the review cycle

Round	HEAD	Codex finding	Resolution
1	`b690792`	P1: web-sys features (false alarm — build succeeds, conversation.rs uses same pattern) + P2: reactive `.read()` on OUTBOUND_SEND_COUNTER inside use_effect	switched to `.peek()` + parent memo subscription via message_count (`793dfbc`)
2	`793dfbc`	P1: read/write borrow overlap in `*OUTBOUND_SEND_COUNTER.write() = OUTBOUND_SEND_COUNTER.peek().wrapping_add(1)`	split into separate let-statement (`9b11de7`)
3	`9b11de7`	P1: `ROOMS.try_read()` in `send_structured_dm` runs outside Dioxus runtime when invoked from safe_spawn_local	routed preflight through `defer()` via oneshot channel (`0de2a9f`)
4	`0de2a9f`	P2: late-completion `.set()` on dropped picker-local use_signals could panic	added still-mine generation gate (`d5f8b18`)
5	`d5f8b18`	P2: my round-4 gate silenced user feedback in timeout-then-late-success scenario	watchdog force-closes picker on expiry (`3a132ea`)
6	`3a132ea`	No findings	—

Self-review (code-first)

Finding	Resolution
`"Already a member"` used `contains_key`, mismatched observer-only rooms	switched to `RoomData::can_participate()` (`793dfbc`)

CI status

check-delegate-migration: pass
check-wasm-sync: pass
build: in progress on HEAD 3a132ea
ui-playwright-tests: in progress on HEAD 3a132ea

Locally verified before each push: cargo fmt, cargo clippy clean on dm_thread_modal.rs / direct_messages.rs / invite_via_dm_picker_modal.rs, 153 unit tests pass, 5 Playwright tests pass on chromium/firefox/webkit.

Waiting on CI for the merge gate.

[AI-assisted - Claude]

sanity · 2026-05-17T22:28:14Z

Ready for merge

CI all-green on HEAD 3a132eac:

build: SUCCESS (7m1s)
check-wasm-sync: SUCCESS (17s)
check-delegate-migration: SUCCESS (25s)
ui-playwright-tests: SUCCESS (10m24s)
license/cla: SUCCESS

Multi-model review complete:

Codex CLI: 6 rounds; rounds 1-5 each surfaced one finding (all fixed); round 6 returned clean.
Self-review (code-first): one nit found and fixed (already_member check needed can_participate() instead of contains_key).

Per original task spec, NOT merging this PR. Parent agent gates merge.

[AI-assisted - Claude]

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>

…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>

sanity and others added 7 commits May 17, 2026 16:42

sanity force-pushed the feat/dm-invite-variant branch from b690792 to 793dfbc Compare May 17, 2026 21:44

This was referenced May 17, 2026

fix(invite): in-app accept re-shows full nickname flow for previously-accepted-then-left rooms #279

Open

fix(invite-card): show 'banned from this room' state instead of enabled Accept #280

Open

sanity merged commit 94fcd8c into main May 17, 2026
6 checks passed

sanity deleted the feat/dm-invite-variant branch May 17, 2026 22:50

sanity mentioned this pull request May 18, 2026

feat(cli): --signing-key-file flag overrides signing identity per command #282

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dm): structured invite DM variant + in-app accept + auto-scroll#278

feat(dm): structured invite DM variant + in-app accept + auto-scroll#278
sanity merged 12 commits into
mainfrom
feat/dm-invite-variant

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

sanity commented May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

sanity commented May 17, 2026

Problem

Approach

Testing

Uh oh!

sanity commented May 17, 2026

Multi-Model Review (Claude, manual four-perspective + Codex CLI)

Findings addressed

Other perspectives

Rebase note

Uh oh!

sanity commented May 17, 2026

Codex round-2 review (HEAD 793dfbc)

Uh oh!

sanity commented May 17, 2026

Codex round-3 review (HEAD 9b11de7)

Uh oh!

sanity commented May 17, 2026

Codex round-4 review (HEAD 0de2a9f)

Uh oh!

sanity commented May 17, 2026

Codex round-5 review (HEAD d5f8b18)

Uh oh!

sanity commented May 17, 2026

Codex round-6 review (HEAD 3a132ea) — clean

Summary of the review cycle

Self-review (code-first)

CI status

Uh oh!

sanity commented May 17, 2026

Ready for merge

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Codex round-2 review (HEAD `793dfbc`)

Codex round-3 review (HEAD `9b11de7`)

Codex round-4 review (HEAD `0de2a9f`)

Codex round-5 review (HEAD `d5f8b18`)

Codex round-6 review (HEAD `3a132ea`) — clean