fix(delegate): per-room CAS keys to stop multi-tab room loss (#345)

[sanity]

Problem

Multi-tab data loss (#345): accepting an invite to a new room in one browser tab
left the room missing from other tabs, and it was lost entirely on refresh. Root
cause (confirmed byte-level on technic): the chat delegate stored the entire
room list in a single rooms_data blob, written with a blind last-writer-wins
overwrite. Each tab has its own in-memory ROOMS, so across tabs the last save
clobbered the others — a stale tab silently erased a room a fresh tab had just
added.

Approach

Two layers, both in this PR:

1. A compare-and-swap primitive in the chat-delegate KV store. New wire
variants GetVersionedRequest/CasStoreRequest (+ GetVersionedResponse/
CasStoreResponse{CasStoreResult}), appended to the externally-tagged enums so
old clients never receive them. Every value is wrapped in a versioned envelope
[tag=1][generation u64 LE][data]; plain Store/Get transparently wrap/unwrap,
CAS stores only when the expected generation matches. Pure, unit-tested logic in
delegates/chat-delegate/src/versioning.rs. Delegate WASM rebuilt → V24
migration entry in legacy_delegates.toml.

2. Per-room keys (realizes #65). Each room is its own
room:<base58(owner_vk)> delegate key holding a RoomSlot::{Present|Tombstone},
CAS-versioned independently; list-level view prefs (current room, notification
modes, room order) live in a single rooms_meta key. The single rooms_data
blob is no longer written (still read once for migration). A save for room X
never touches room Y's key, so the clobber is structurally impossible.

• Save (do_save_rooms_to_delegate): content-diffs each room and CAS-writes
  only changed slots; reconcile_room_present/_tombstone/_meta are the
  read-merge-write reconcilers; present_action is the pure rejoin-vs-leave
  decision. Per-room error isolation (one room's CAS exhaustion doesn't abort the
  pass).
• Load (response_handler.rs): fire_list_rooms_request (a ListRequest)
  → load_rooms_per_room plain-GETs each slot + rooms_meta, reconstructs
  Rooms, and hydrates via the shared hydrate_loaded_rooms. Plain-GET
  correlation is distinct from the save's CAS correlation, so a concurrent
  save can't steal a load's response slot.
• Rejoin vs leave (round-9): an explicit rejoin (mark_room_rejoined at the
  invitation-accept and identity-import sites) overwrites a remote Tombstone;
  a background content update to a room left elsewhere adopts the leave. The
  rejoin flag is consumed once the rejoin is persisted, and cleared on leave, so
  a stale flag can't resurrect a room left in another tab.

Transparent, non-destructive migration. On the V24 upgrade the new delegate
key is empty → load falls through to the legacy-delegate probe and recovers the
old single rooms_data blob, re-saving it as per-room keys (the old blob is left
in place as a rollback fallback). Because per-room keys are dynamic,
fire_legacy_migration_request also fires a ListRequest to each legacy
delegate so a future delegate-WASM bump can recover the per-room keys the
now-legacy delegate holds (migrate_legacy_per_room) — without this, the next
bump would strand every room.

Testing

All native unit tests green (cargo test -p river-ui --bins 336, chat-delegate
40, river-core chat_delegate 19); cargo fmt + clippy clean.

• CAS loop (stores/abort/conflict-retry/exhaustion/failed + wrote-bool), envelope
  encode/decode, generation arithmetic, handler dispatch.
• present_action truth table + reconcile_room_present at byte level (absent,
  tombstone-background-adopts-leave, tombstone-explicit-rejoin-overwrites,
  diverged-identity-keeps-local, unparseable-errs).
• plan_load_from_keys (all branches), reconstruct_rooms, RoomSlot/
  RoomsMeta CBOR round-trips, single-blob→per-room explosion+reconstruct
  round-trip (the on-node upgrade format contract).
• Rejoin flag set/clear + a source-grep pin on the mark_room_rejoined /
  clear_room_rejoined wiring; parse_room_storage_key round-trip/negatives;
  correlation-key divergence (load vs save).

Reviewed by a multi-model pass (4 perspectives + external codex); findings
addressed in the follow-up commit (legacy per-room migration, rejoin/cache
correctness, per-room error isolation, invitation_secrets union, doc updates).

Still TODO before merge/deploy: two-tab Playwright on technic + an on-node
upgrade check (load the new UI against a node holding old single-blob state →
rooms reappear), and re-confirming deploy authorization (scope grew from
single-blob to per-room).

Closes #345. Realizes #65.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #347

Summary

• PR Title: fix(delegate): compare-and-swap rooms_data to stop multi-tab room loss (#345)
• Type: fix (delegate WASM + wire format + concurrency + data migration)
• CI Status: was green on the reviewed commit 1acac35c; re-running on the fix commit 31dd7ba1
• Linked Issues: #345 (closes); relates to #65 (PR2), #307 (CLI analog)
• Review tier: Full (high-risk surfaces: concurrency, wire format/serialization, delegate WASM, data migration)
• Reviewers run: four Freenet perspectives (code-first, testing, skeptical, big-picture) + external Codex (non-Claude). Findings synthesized, every cited file:line verified.

This review found one critical data-loss bug and several should-fixes; all are addressed in 31dd7ba1. A re-review on the new HEAD follows.

---

Findings and resolutions

Must Fix (Blocking) — addressed

1. [Codex P1 — critical] Merged rooms were not written back to the ROOMS signal after a conflict-resolved store. After a CAS conflict the loop folded the other tab's rooms into the local working copy and stored the union, but the success path only updated ROOMS_GENERATION. The in-memory ROOMS never received the merged rooms, so the next save snapshotted the stale ROOMS (now with the correct generation, so no conflict) and overwrote the delegate — silently re-deleting the just-merged rooms. This defeated the fix in the multi-save case. Verified against chat_delegate.rs:1633-1647.
   • Fix (31dd7ba1): refactored the loop into a testable run_rooms_cas_loop returning CasSaveOutcome { stored, generation, merged_remote }; when merged_remote, do_save_rooms_to_delegate defers a ROOMS.merge(stored) write-back (Dioxus-safe). Side benefit: the other tab's room now appears without a reload.

Should Fix — addressed

2. [skeptical M1] Pending-request slot theft. A plain GetResponse for rooms_data (cold-start load / legacy probe) shared the "rooms_data" correlation key with an in-flight CAS save and could complete that save's oneshot with the wrong response, failing the save (and coalesce_save does not auto-retry a failed save). Pre-existing mechanism, but the CAS rewrite made it a hard mid-loop error.
   • Fix: CAS/GetVersioned ops now use distinct correlation keys (cas_store_correlation_key / get_versioned_correlation_key), applied in both get_request_key and the response-handler routing.
3. [testing / skeptical M3 / code-first] CAS loop conflict/exhaustion/error paths untested.
   • Fix: added run_rooms_cas_loop tests (stored-on-first-attempt, conflict→fold→retry carrying both writers' state at the adopted generation, 8-attempt exhaustion error, Failed propagation) and a populated-map Rooms::merge test pinning the #345 headline (two distinct rooms both survive).

Consider / Documented (not code-blocking)

4. [Codex P2] Delete resets a key's generation (ABA). Not reachable in PR1 (no CAS-tracked key is ever deleted). Documented in versioning.rs and the delete handler; the proper handling belongs in PR2 (where per-room-key leave-deletes appear, guarded by the removed_rooms tombstone).
5. [skeptical M2] An un-mergeable remote aborts the whole save. This is intentional and safe — refusing to clobber data we can't parse/merge is preferable to overwriting it; in-memory rooms are untouched and the next mutation re-attempts. Documented at the fold site. Per-room isolation comes with PR2.
6. [code-first] migrated_rooms/current_room_key are intentionally not folded from the remote (transient/per-tab; merge re-derives upgrade pointers). Noted in fold_conflicting_rooms. Also tightened the envelope-tag invariant comment (real CBOR payloads start 0xA0..0xBF, never the 0x01 tag) and corrected the misleading coalesce_save "will retry" comment.

Dropped — false positive

• [big-picture] PR description base58 typo (358wv7Kc…). Verified incorrect: the recorded V24 hex re-encodes to 358xw7Ld…, which matches the deployed delegate. The PR text was already correct.

---

Verified safe (by skeptical + code-first, independently confirmed)

• Envelope defensive-decode cannot misread real CBOR data; migration old→new delegate is correct (V24 delegate_key = BLAKE3(code_hash) and equals the deployed delegate by base58 decode); plain Get/Store envelope back-compat keeps the outbound-DM cache working; ROOMS_GENERATION is single-flight under coalesce_save; the CAS loop never mutates the ROOMS signal mid-loop (Dioxus-safe); riverctl is decoupled (own rooms.json). Room-contract WASM untouched.

Testing

Test Type	Status	Notes
Unit	✅	Pure CAS/envelope (versioning.rs), handler dispatch, CBOR wire round-trips, fold_conflicting_rooms, run_rooms_cas_loop, Rooms::merge distinct-rooms
Integration	⚠️	Native delegate set_secret/get_secret are no-ops, so end-to-end persistence isn't host-testable; invariants pinned at the pure layer
Simulation	N/A
E2E	⏳	Two-tab live-node Playwright verification to run before deploy (incl. outbound-DM envelope round-trip + legacy-migration re-save)

Verdict

State: Needs Changes — Re-review Required After Fix → fixes applied in 31dd7ba1; re-review running on the new HEAD.
HEAD SHA reviewed: 1acac35c (original); fixes on 31dd7ba1.

[AI-assisted - Claude]

[sanity]

Comprehensive PR Review: #347 — Final (converged)

Review tier: Full (concurrency + wire format + delegate WASM + data migration).
Reviewers: four Freenet perspectives + external Codex, run iteratively across 6 commits as fixes landed.
HEAD reviewed: clean pass on fabdf21d; current HEAD 357d6b23 is doc-only on top of it.

Outcome

The iterative multi-model review found and fixed four distinct data-loss / data-corruption bugs that the first implementation missed — most surfaced by the external Codex pass, whose blind spots don't correlate with the (Claude-authored) code:

1. Merged rooms never written back → next save re-deleted them. (Codex round 1)
2. Deferred write-back didn't cover the coalesce catch-up iteration → same loss, tighter window. (Codex + skeptical round 2) → fixed with the synchronous PENDING_MERGED_ROOMS holding cell folded into every save.
3. Stale tombstone resurrected over an explicit rejoin → a rejoined room re-removed on first save after reload. (Codex round 3) → fixed with Rooms::merge_reconciling (local map membership wins over a stale remote tombstone; #247 leave-stickiness preserved).
4. The write-back surfaced under-hydrated, unsubscribed rooms and could still undo a rejoin. (Codex + skeptical round 4) → removed the write-back entirely; persistence now rests solely on the holding cell, and the merged room reappears fully hydrated via the load path on reload (the documented "fixed after refresh" PR1 behavior; live cross-tab surfacing is PR2's scope).

Plus M1 (a plain GetResponse could steal an in-flight CAS save's response slot → distinct correlation keys).

Final verdict

No data-loss windows remain in the reviewed paths (both round-5 reviewers concur). The CAS conflict resolution is local-authoritative ("keep my decisions, absorb the rooms I'm missing") — strictly better than the prior blind overwrite, which it replaces. All findings are fixed or have a justified dismissal.

Remaining items — non-blocking, deferred to PR2 (per-room key split), tracked on #345:

• Per-room scalar metadata (last_read_message_id, self_nickname, invitation_secrets) keeps the local value on a same-room conflict — not a regression (the prior blind overwrite reverted every room's scalars on a stale save; CAS is no worse and better elsewhere). Documented on Rooms::merge_reconciling.
• A CAS-only-merged room's contract-WASM-upgrade pointer is deferred to reload (pre-existing, rare path).

Testing

• Unit: pure CAS/envelope (versioning.rs), handler dispatch, CBOR wire round-trips (incl. legacy-variant pin), correlation-key divergence + round-trip, run_rooms_cas_loop (conflict→fold→retry, exhaustion, failure), merge_reconciling (rejoin preserved with a plain-merge-drops-it precondition; leave stays sticky), distinct-rooms union, envelope-tag CBOR-collision. chat-delegate 40, river-core 210, river-ui --bins 309 — all green.
• Migration: check-delegate-migration + check-room-contract-migration green on HEAD; V24 delegate_key verified == the deployed delegate by base58 decode.
• E2E: two-tab live-node Playwright verification (incl. outbound-DM envelope round-trip + legacy-migration re-save) to run before deploy.

Verdict

State: Ready to Merge once CI is green on the exact current HEAD (357d6b23).
Deploy note: delegate-WASM change → publish via cargo make publish-all from main after merge, gated on green CI at the merge commit + the E2E pass. PR2 (per-room key split, #65) follows after PR1 merges.

[AI-assisted - Claude]