From ea2c469068bf2721338ab10edb9116356dd1d174 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 29 Jun 2026 09:06:25 -0500 Subject: [PATCH 1/2] feat(mentions): reference members by truncated-base32 in mention tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mention wire tokens now reference a member by their 8-char truncated-base32 Display label (e.g. `rv:FPVN6PUN`) instead of the 16-char hex id (`rv:422a2a8d3edfea2b`). FPVN6PUN is the same short id River shows everywhere else to name a member, so mentions now read consistently with the rest of the app. The base32 label is lossy (40 of 64 bits), so a parsed mention carries a `MemberRef` that callers resolve against the room's known members (the same assumption the CLI/UI already make when naming members by short id) rather than a decoded `MemberId`. Backward compatible: the legacy `rv:` form is still parsed for messages already in circulation, via `MemberRef::Legacy`. Both are flagged for eventual removal (TODO(mentions) on the module and the variant). The two forms can't collide — current is exactly 8 base32 chars, legacy up to 16 hex, base32 tried first. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv --- cli/src/api.rs | 4 +- common/src/mention.rs | 227 ++++++++++++++++++++++++++---- ui/src/components/conversation.rs | 105 ++++++++++++-- 3 files changed, 291 insertions(+), 45 deletions(-) diff --git a/cli/src/api.rs b/cli/src/api.rs index 969842f9..b5523a54 100644 --- a/cli/src/api.rs +++ b/cli/src/api.rs @@ -111,12 +111,12 @@ pub(crate) fn message_display_text( /// unknown or their nickname is encrypted (riverctl does not decrypt /// private-room nicknames). Plain text without tokens is returned unchanged. pub(crate) fn render_mentions_for_terminal(room_state: &ChatRoomStateV1, text: &str) -> String { - river_core::mention::render_plaintext(text, |id| { + river_core::mention::render_plaintext(text, |r| { room_state .member_info .member_info .iter() - .find(|info| info.member_info.member_id == id) + .find(|info| r.matches(info.member_info.member_id)) .and_then(|info| info.member_info.preferred_nickname.as_public_bytes()) .map(|bytes| String::from_utf8_lossy(bytes).to_string()) }) diff --git a/common/src/mention.rs b/common/src/mention.rs index ea23ebe1..55c1e5e5 100644 --- a/common/src/mention.rs +++ b/common/src/mention.rs @@ -6,19 +6,34 @@ //! A mention is stored inline in the plaintext message `text` field as: //! //! ```text -//! @[Display Name](rv:8f3a2b1c0000d4e5) +//! @[Display Name](rv:FPVN6PUN) //! ``` //! -//! - `rv:` is the **authoritative** reference: 16 lowercase hex digits -//! encoding the member's [`MemberId`] (a 64-bit value). Clients re-resolve -//! the member's *current* nickname from `member_info` by this id, so the -//! rendered chip follows renames. +//! - `rv:` is the **authoritative** reference. The current form is the +//! member's 8-char truncated-base32 [`MemberId`] `Display` string (e.g. +//! `FPVN6PUN`) — the *same* short label used everywhere else in the UI and +//! CLI to name a member, so mentions read consistently with the rest of the +//! app. Clients re-resolve the member's *current* nickname from `member_info` +//! by matching this label, so the rendered chip follows renames. The label is +//! *lossy* (40 of the id's 64 bits), so callers resolve it against the room's +//! known members rather than decoding it back to a [`MemberId`] directly. //! - `Display Name` is a **fallback snapshot** captured at send time, used only //! when re-resolution is impossible (member not in `member_info`, an //! undecryptable private-room nickname, or a client too old to parse the //! token). Old clients render the raw token via markdown as a plain //! `@Display Name` link — acceptable graceful degradation. //! +//! ### Legacy reference form (compat — remove eventually) +//! +//! Messages sent before this change carry the reference as 16 lowercase hex +//! digits (the *lossless* full 64-bit id), e.g. `@[Name](rv:8f3a2b1c0000d4e5)`. +//! The parser still accepts that form so old messages keep resolving. New +//! tokens are never emitted in hex. **TODO(mentions): once no message in +//! circulation still carries a legacy `rv:` token, drop the hex branch of +//! [`member_ref_from_str`] and the [`MemberRef::Legacy`] variant.** The two +//! forms are unambiguous: the current form is exactly 8 base32 chars, the +//! legacy form is up to 16 hex chars, and the parser tries base32 first. +//! //! The token rides inside the existing `text` field, so it needs **no** change //! to `RoomMessageBody` / the contract, works in both public and private rooms //! (it sits inside the encrypted text), and applies equally to plain text and @@ -44,13 +59,51 @@ pub const MAX_SNAPSHOT_NAME_LEN: usize = 64; /// A parsed mention: the authoritative member reference plus the snapshot name /// carried in the token (which may be empty, or stale relative to the member's -/// current nickname — callers should prefer a fresh lookup by `member_id`). +/// current nickname — callers should prefer a fresh lookup by `member_ref`). #[derive(Clone, PartialEq, Eq, Debug)] pub struct Mention { - pub member_id: MemberId, + pub member_ref: MemberRef, pub display_name: String, } +/// The member a mention token points at. The current wire form carries the +/// member's truncated-base32 `Display` label, which is *lossy* and so cannot be +/// turned back into a full [`MemberId`] on its own — it is matched against the +/// room's known members. The legacy hex form carries the full id directly. +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum MemberRef { + /// Current form: the member's 8-char truncated-base32 `Display` label + /// (e.g. `FPVN6PUN`). Resolve it against the room via [`MemberRef::resolve`]. + Short(String), + /// Legacy form: a full 64-bit id recovered from a 16-hex-digit token. + /// TODO(mentions): remove once no legacy `rv:` token remains in + /// circulation (see the module-level note). + Legacy(MemberId), +} + +impl MemberRef { + /// Whether this reference names `id`. The current (short) form compares the + /// truncated-base32 label the rest of the app uses to identify a member + /// (`MemberId`'s `Display`); the legacy form compares the full 64-bit id. + pub fn matches(&self, id: MemberId) -> bool { + match self { + MemberRef::Short(label) => id.to_string() == *label, + MemberRef::Legacy(full) => *full == id, + } + } + + /// The full [`MemberId`] this reference denotes, if recoverable. A legacy + /// ref carries it directly; a short ref is matched against `candidates` (the + /// room's known members) and yields `None` when this client doesn't know the + /// named member — in which case the mention degrades to its snapshot name. + pub fn resolve(&self, candidates: impl IntoIterator) -> Option { + match self { + MemberRef::Legacy(full) => Some(*full), + MemberRef::Short(_) => candidates.into_iter().find(|id| self.matches(*id)), + } + } +} + /// One piece of a message body: either a run of plain text or a mention token. #[derive(Clone, PartialEq, Eq, Debug)] pub enum MentionSegment { @@ -58,15 +111,20 @@ pub enum MentionSegment { Mention(Mention), } -/// Encode a [`MemberId`] as the 16-char lowercase-hex reference body (no scheme -/// prefix). Lossless — unlike `MemberId`'s `Display`, which is *truncated*. +/// Encode a [`MemberId`] as the 16-char lowercase-hex form. This is the +/// *lossless* full-id encoding. It is NOT the wire-token reference (that is the +/// truncated-base32 [`member_id_to_short`]); it is used only for in-session, +/// full-precision handoffs that never persist — e.g. the rendered chip's +/// `data-member-id` DOM attribute, read straight back by the click interceptor. pub fn member_id_to_hex(id: MemberId) -> String { // MemberId(FastHash(i64)); encode the raw 64 bits. format!("{:016x}", id.0 .0 as u64) } -/// Parse a 1..=16 digit hex reference body back into a [`MemberId`]. Returns -/// `None` for empty input, over-long input, or non-hex characters. +/// Parse a 1..=16 digit hex string back into a [`MemberId`]. Returns `None` for +/// empty input, over-long input, or non-hex characters. Counterpart of +/// [`member_id_to_hex`] (the lossless full-id encoding), and also the decoder +/// for the legacy `rv:` wire-token form (see [`member_ref_from_str`]). pub fn member_id_from_hex(s: &str) -> Option { if s.is_empty() || s.len() > 16 || !s.bytes().all(|b| b.is_ascii_hexdigit()) { return None; @@ -75,6 +133,38 @@ pub fn member_id_from_hex(s: &str) -> Option { Some(MemberId(FastHash(raw as i64))) } +/// Encode a [`MemberId`] as its canonical short reference: the 8-char +/// truncated-base32 `Display` label (e.g. `FPVN6PUN`) used everywhere else to +/// name a member. This is the current wire-token reference form. It is *lossy* +/// (40 of 64 bits), so it round-trips through [`MemberRef`] (resolved against +/// the room's members), not back into a [`MemberId`] directly. +pub fn member_id_to_short(id: MemberId) -> String { + id.to_string() +} + +/// Parse a mention reference body (the part after `rv:`) into a [`MemberRef`]. +/// +/// Accepts both wire forms, current first: an 8-char truncated-base32 label +/// (the [`member_id_to_short`] form) becomes [`MemberRef::Short`]; otherwise a +/// 1..=16 hex-digit legacy id becomes [`MemberRef::Legacy`]. Returns `None` for +/// anything else. The two forms can't collide — the current form is exactly 8 +/// base32 chars and is tried first, the legacy form is up to 16 hex chars. +pub fn member_ref_from_str(s: &str) -> Option { + if is_short_ref(s) { + return Some(MemberRef::Short(s.to_string())); + } + // TODO(mentions): remove this legacy hex fallback once no message in + // circulation still carries an `rv:` token (see module-level note). + member_id_from_hex(s).map(MemberRef::Legacy) +} + +/// Whether `s` is a current-form short reference: exactly 8 characters from the +/// RFC4648 base32 alphabet (uppercase `A`–`Z` and digits `2`–`7`), matching the +/// `truncated_base32` output that backs `MemberId`'s `Display`. +fn is_short_ref(s: &str) -> bool { + s.len() == 8 && s.bytes().all(|b| matches!(b, b'A'..=b'Z' | b'2'..=b'7')) +} + /// Strip characters that would break token parsing (the bracket/paren /// delimiters and control chars), trim, and cap the length. Applied to the /// snapshot name at encode time so a name can never break the wire token. @@ -86,13 +176,14 @@ pub fn sanitize_name(name: &str) -> String { cleaned.trim().chars().take(MAX_SNAPSHOT_NAME_LEN).collect() } -/// Build the wire token `@[name](rv:hex)` for a mention. The name is sanitized. +/// Build the wire token `@[name](rv:FPVN6PUN)` for a mention, using the +/// member's short (truncated-base32) reference. The name is sanitized. pub fn encode_mention(id: MemberId, display_name: &str) -> String { format!( "@[{}]({}{})", sanitize_name(display_name), REF_SCHEME, - member_id_to_hex(id) + member_id_to_short(id) ) } @@ -154,7 +245,7 @@ fn try_parse_token_at(text: &str, at: usize) -> Option<(Mention, usize)> { { return None; } - // Hex id runs to the closing `)`. + // The reference body runs to the closing `)`. let id_start = after + prefix_len; let mut k = id_start; while k < bytes.len() && bytes[k] != b')' { @@ -163,10 +254,10 @@ fn try_parse_token_at(text: &str, at: usize) -> Option<(Mention, usize)> { if k >= bytes.len() { return None; // unterminated `(` } - let member_id = member_id_from_hex(&text[id_start..k])?; + let member_ref = member_ref_from_str(&text[id_start..k])?; Some(( Mention { - member_id, + member_ref, display_name: name.to_string(), }, k + 1, @@ -186,22 +277,26 @@ pub fn parse_mentions(text: &str) -> Vec { /// Whether `text` mentions the member with the given id. pub fn contains_mention_of(text: &str, id: MemberId) -> bool { - parse_mentions(text).iter().any(|m| m.member_id == id) + parse_mentions(text) + .iter() + .any(|m| m.member_ref.matches(id)) } /// Render the text for a plain-text surface (e.g. the CLI), replacing each -/// mention token with `@`. `resolve` supplies the member's *current* -/// display name; when it returns `None` the snapshot name in the token is used. +/// mention token with `@`. `resolve` maps the token's [`MemberRef`] to the +/// member's *current* display name (typically by scanning the room's members +/// with [`MemberRef::matches`]); when it returns `None` the snapshot name in the +/// token is used. pub fn render_plaintext(text: &str, mut resolve: F) -> String where - F: FnMut(MemberId) -> Option, + F: FnMut(&MemberRef) -> Option, { let mut out = String::with_capacity(text.len()); for seg in parse_segments(text) { match seg { MentionSegment::Text(t) => out.push_str(&t), MentionSegment::Mention(m) => { - let name = resolve(m.member_id).unwrap_or(m.display_name); + let name = resolve(&m.member_ref).unwrap_or(m.display_name); out.push('@'); out.push_str(&name); } @@ -300,13 +395,83 @@ mod tests { fn encode_parse_round_trip() { let id = mid(0x8f3a_2b1c_0000_d4e5u64 as i64); let token = encode_mention(id, "Alice"); - assert_eq!(token, "@[Alice](rv:8f3a2b1c0000d4e5)"); + // The current form is the member's truncated-base32 Display label, not + // hex — consistent with how members are named everywhere else. + assert_eq!(token, format!("@[Alice](rv:{})", member_id_to_short(id))); + assert!(!token.contains(&member_id_to_hex(id)), "must not emit hex"); let mentions = parse_mentions(&token); assert_eq!(mentions.len(), 1); - assert_eq!(mentions[0].member_id, id); + assert_eq!( + mentions[0].member_ref, + MemberRef::Short(member_id_to_short(id)) + ); + assert!(mentions[0].member_ref.matches(id)); assert_eq!(mentions[0].display_name, "Alice"); } + #[test] + fn current_token_uses_truncated_base32_matching_display() { + // The token reference is byte-identical to the member's Display label + // (`MemberId`'s `Display`), which is exactly what the rest of the app + // shows. 8 base32 chars, no lowercase hex. + let id = mid(0x422a_2a8d_3edf_ea2bu64 as i64); + let short = member_id_to_short(id); + assert_eq!(short, id.to_string()); + assert_eq!(short.len(), 8); + let token = encode_mention(id, "Ivvor"); + assert!(token.contains(&format!("(rv:{short})")), "token: {token}"); + } + + #[test] + fn legacy_hex_token_still_parses_and_matches() { + // A token sent by an old client carries the lossless 16-hex-digit id. + let id = mid(0x8f3a_2b1c_0000_d4e5u64 as i64); + let legacy = format!("@[Name](rv:{})", member_id_to_hex(id)); + let mentions = parse_mentions(&legacy); + assert_eq!(mentions.len(), 1); + assert_eq!(mentions[0].member_ref, MemberRef::Legacy(id)); + assert!( + mentions[0].member_ref.matches(id), + "legacy ref resolves to its id" + ); + } + + #[test] + fn member_ref_from_str_disambiguates_base32_from_hex() { + let id = mid(0x422a_2a8d_3edf_ea2bu64 as i64); + // 8-char base32 -> Short (tried first). + assert_eq!( + member_ref_from_str(&member_id_to_short(id)), + Some(MemberRef::Short(member_id_to_short(id))) + ); + // 16-char hex -> Legacy. + assert_eq!( + member_ref_from_str(&member_id_to_hex(id)), + Some(MemberRef::Legacy(id)) + ); + // An 8-char body of only base32 digits (2-7) is the current form, NOT + // mis-read as hex — base32 is tried first and these are valid base32. + assert_eq!( + member_ref_from_str("23456723"), + Some(MemberRef::Short("23456723".to_string())) + ); + // Garbage / empty / wrong length -> no ref. + assert_eq!(member_ref_from_str(""), None); + assert_eq!(member_ref_from_str("zz"), None); + } + + #[test] + fn member_ref_resolve_recovers_member_id() { + let id = mid(0x1234_5678_9abc_def0u64 as i64); + let other = mid(99); + // Short ref recovers the full id only when the member is among candidates. + let short = MemberRef::Short(member_id_to_short(id)); + assert_eq!(short.resolve([other, id]), Some(id)); + assert_eq!(short.resolve([other]), None); + // Legacy ref carries the id directly, no candidates needed. + assert_eq!(MemberRef::Legacy(id).resolve(std::iter::empty()), Some(id)); + } + #[test] fn sanitize_strips_delimiters_and_caps_length() { assert_eq!(sanitize_name("a]b)c(d[e"), "abcde"); @@ -320,7 +485,7 @@ mod tests { let token = encode_mention(id, "ev)il](rv:dead)"); let parsed = parse_mentions(&token); assert_eq!(parsed.len(), 1); - assert_eq!(parsed[0].member_id, id); + assert!(parsed[0].member_ref.matches(id)); assert!(!parsed[0].display_name.contains([']', '(', ')'])); } @@ -339,12 +504,12 @@ mod tests { vec![ MentionSegment::Text("hi ".to_string()), MentionSegment::Mention(Mention { - member_id: a, + member_ref: MemberRef::Short(member_id_to_short(a)), display_name: "Ann".to_string() }), MentionSegment::Text(" and ".to_string()), MentionSegment::Mention(Mention { - member_id: b, + member_ref: MemberRef::Short(member_id_to_short(b)), display_name: "Bob".to_string() }), MentionSegment::Text("!".to_string()), @@ -355,12 +520,12 @@ mod tests { #[test] fn empty_snapshot_name_is_valid() { let id = mid(42); - let token = format!("@[](rv:{})", member_id_to_hex(id)); + let token = format!("@[](rv:{})", member_id_to_short(id)); let mentions = parse_mentions(&token); assert_eq!( mentions, vec![Mention { - member_id: id, + member_ref: MemberRef::Short(member_id_to_short(id)), display_name: String::new() }] ); @@ -419,8 +584,8 @@ mod tests { encode_mention(known, "OldName"), encode_mention(unknown, "Ghost") ); - let rendered = render_plaintext(&text, |id| { - if id == known { + let rendered = render_plaintext(&text, |r| { + if r.matches(known) { Some("NewName".to_string()) // current nickname overrides snapshot } else { None // unknown member -> fall back to snapshot @@ -452,7 +617,7 @@ mod tests { assert_eq!( parse_mentions(&out), vec![Mention { - member_id: alice, + member_ref: MemberRef::Short(member_id_to_short(alice)), display_name: "Alice".to_string() }] ); diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 6bbaeee6..71b87aa0 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -430,8 +430,12 @@ pub(crate) fn decrypt_message_content( /// the token snapshot as fallback) and strip markdown formatting, so the preview /// reads as plain text. Caller truncates the result. fn clean_reply_preview(text: &str, member_names: &HashMap) -> String { - let with_mentions = - river_core::mention::render_plaintext(text, |id| member_names.get(&id).cloned()); + let with_mentions = river_core::mention::render_plaintext(text, |r| { + member_names + .iter() + .find(|(id, _)| r.matches(**id)) + .map(|(_, name)| name.clone()) + }); strip_markdown(&with_mentions) } @@ -589,14 +593,18 @@ pub(crate) fn message_to_html_with_mentions( } MentionSegment::Mention(m) => { let idx = chips.len(); - let name = member_names - .get(&m.member_id) - .cloned() + // The token's reference is the member's short (truncated-base32) + // label; recover the full id by matching it against the room's + // known members so the chip stays clickable and self-highlighted. + // An unknown member yields `None` -> non-clickable snapshot chip. + let resolved = m.member_ref.resolve(member_names.keys().copied()); + let name = resolved + .and_then(|id| member_names.get(&id).cloned()) .unwrap_or(m.display_name); chips.push(render_mention_chip_html( - m.member_id, + resolved, &name, - m.member_id == self_member_id, + resolved == Some(self_member_id), )); working.push(OPEN); working.push_str(&idx.to_string()); @@ -617,18 +625,30 @@ pub(crate) fn message_to_html_with_mentions( /// Build the inline chip markup for one mention. `name` is the resolved current /// nickname (or snapshot fallback) and is HTML-escaped — nicknames are /// attacker-controlled and this string goes through `dangerous_inner_html` -/// (freenet/river#227). The `data-member-id` carries the lossless hex id so the -/// document-level click interceptor can open the member-info modal. -fn render_mention_chip_html(id: MemberId, name: &str, is_self: bool) -> String { +/// (freenet/river#227). +/// +/// `id` is the resolved member id, or `None` when the token's short reference +/// names a member this client doesn't know. When present, `data-member-id` +/// carries the lossless hex id (an in-session, full-precision handoff — NOT the +/// wire token) so the document-level click interceptor can open the member-info +/// modal. When absent the chip still renders the `@name` but is inert (nothing +/// to open), which is the correct degradation for an unknown member. +fn render_mention_chip_html(id: Option, name: &str, is_self: bool) -> String { let class = if is_self { "river-mention river-mention-self" } else { "river-mention" }; + let data_member_id = match id { + Some(id) => format!( + " data-member-id=\"{}\"", + river_core::mention::member_id_to_hex(id) + ), + None => String::new(), + }; format!( - "@{label}", - hex = river_core::mention::member_id_to_hex(id), title = escape_html_attr(name), label = escape_html(name), ) @@ -3558,4 +3578,65 @@ mod tests { "{html}" ); } + + #[test] + fn current_token_chip_carries_full_id_resolved_from_short_ref() { + // The wire token now carries only the 8-char short ref; the chip must + // still recover the FULL id (for the click interceptor) by matching the + // short ref against the known members. + let id = mid_from("00000000000000aa"); + let mut names = HashMap::new(); + names.insert(id, "Alice".to_string()); + let token = river_core::mention::encode_mention(id, "Alice"); + assert!( + token.contains(&format!( + "rv:{}", + river_core::mention::member_id_to_short(id) + )), + "token uses the short base32 ref: {token}" + ); + let html = message_to_html_with_mentions(&token, &names, mid_from("00000000000000ff")); + assert!( + html.contains("data-member-id=\"00000000000000aa\""), + "chip recovers the lossless id from the short ref: {html}" + ); + } + + #[test] + fn legacy_hex_mention_chip_is_clickable_even_when_member_unknown() { + // A legacy `rv:` token carries the full id, so its chip stays + // clickable (data-member-id present) even for a member we can't name. + let id = mid_from("0000000000000abc"); + let names = HashMap::new(); // member not resolvable by name + let legacy = format!( + "hi @[Bob]({}{})!", + river_core::mention::REF_SCHEME, + river_core::mention::member_id_to_hex(id) + ); + let html = message_to_html_with_mentions(&legacy, &names, mid_from("0000000000000001")); + assert!( + html.contains("data-member-id=\"0000000000000abc\""), + "legacy chip keeps the full id: {html}" + ); + assert!(html.contains(">@Bob"), "snapshot name used: {html}"); + } + + #[test] + fn unknown_short_mention_renders_inert_chip_without_member_id() { + // A current (short) token naming a member this client doesn't know + // cannot recover a full id, so the chip renders the snapshot name but + // carries no data-member-id (nothing for the interceptor to open). + let id = mid_from("0000000000000abc"); + let names = HashMap::new(); + let token = river_core::mention::encode_mention(id, "Ghost"); + let html = message_to_html_with_mentions(&token, &names, mid_from("0000000000000001")); + assert!( + html.contains(">@Ghost"), + "snapshot name shown: {html}" + ); + assert!( + !html.contains("data-member-id"), + "unknown short ref must not fabricate a member id: {html}" + ); + } } From f6bfbe3926d9df8171045212de42700b251f0f14 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 29 Jun 2026 09:16:32 -0500 Subject: [PATCH 2/2] test(mentions): deterministic collision resolve + back-compat coverage Address multi-model review findings (all MINOR, no blockers): - MemberRef::resolve now tie-breaks on the lowest id instead of returning the first HashMap match, so a ~2^-40 truncated-label collision resolves to one well-defined member regardless of iteration order (was nondeterministic for the chip's data-member-id / self-highlight). Same systemic limit the app already accepts when naming members by short id. - Fix REF_SCHEME rustdoc example (was still hex). - Add coverage: deterministic collision resolution, a single body mixing a legacy hex token and a current base32 token, legacy self-mention detection via contains_mention_of, a multibyte snapshot name around a base32 ref, a legacy self-mention highlight in the UI chip, and an explicit CLI assertion that the send path emits base32 and never hex. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv --- cli/src/api.rs | 21 ++++++++ common/src/mention.rs | 90 ++++++++++++++++++++++++++++++- ui/src/components/conversation.rs | 19 +++++++ 3 files changed, 128 insertions(+), 2 deletions(-) diff --git a/cli/src/api.rs b/cli/src/api.rs index b5523a54..3eb815a5 100644 --- a/cli/src/api.rs +++ b/cli/src/api.rs @@ -5246,6 +5246,27 @@ mod mention_cli_tests { ); } + #[test] + fn resolved_outgoing_mention_uses_base32_ref_not_hex() { + // The CLI send path must emit the truncated-base32 ref, never hex — + // this pins the property directly (not transitively via encode_mention). + let alice = SigningKey::from_bytes(&[1u8; 32]); + let state = state_with_members(&[(alice.clone(), SealedBytes::public(b"alice".to_vec()))]); + let out = resolve_outgoing_mentions(&state, "hi @alice!"); + let id = member_id(&alice); + assert!( + out.contains(&format!( + "rv:{}", + river_core::mention::member_id_to_short(id) + )), + "CLI send path emits the base32 ref: {out}" + ); + assert!( + !out.contains(&river_core::mention::member_id_to_hex(id)), + "CLI send path must not emit hex: {out}" + ); + } + #[test] fn leaves_ambiguous_nickname_as_plain_text() { // Two members share the nickname "alice" → cannot disambiguate. diff --git a/common/src/mention.rs b/common/src/mention.rs index 55c1e5e5..9eca73a5 100644 --- a/common/src/mention.rs +++ b/common/src/mention.rs @@ -49,7 +49,7 @@ use crate::room_state::member::MemberId; use freenet_scaffold::util::FastHash; -/// Scheme prefix inside the mention token's reference, e.g. `rv:8f3a…`. +/// Scheme prefix inside the mention token's reference, e.g. `rv:FPVN6PUN`. pub const REF_SCHEME: &str = "rv:"; /// Maximum number of characters retained from the snapshot display name. The @@ -96,10 +96,18 @@ impl MemberRef { /// ref carries it directly; a short ref is matched against `candidates` (the /// room's known members) and yields `None` when this client doesn't know the /// named member — in which case the mention degrades to its snapshot name. + /// + /// If two members in `candidates` share the same 8-char label (a ~2⁻⁴⁰ + /// truncation collision — astronomically rare, and the same limit the rest + /// of the app already accepts when naming a member by short id), the lowest + /// id wins, so resolution is deterministic and reproducible rather than + /// dependent on candidate iteration order. pub fn resolve(&self, candidates: impl IntoIterator) -> Option { match self { MemberRef::Legacy(full) => Some(*full), - MemberRef::Short(_) => candidates.into_iter().find(|id| self.matches(*id)), + // `min()` (not `find()`) so a short-label collision resolves to one + // well-defined member regardless of candidate iteration order. + MemberRef::Short(_) => candidates.into_iter().filter(|id| self.matches(*id)).min(), } } } @@ -472,6 +480,84 @@ mod tests { assert_eq!(MemberRef::Legacy(id).resolve(std::iter::empty()), Some(id)); } + #[test] + fn resolve_is_deterministic_on_truncated_label_collision() { + // The short label encodes the low 40 bits of the id, so two ids that + // share those bits collide. `resolve` must pick one well-defined member + // (lowest id) regardless of candidate order — not whatever iterates + // first. (~2⁻⁴⁰ event; this asserts the behaviour is at least defined.) + let low = mid(0x42); + let high = mid(0x42 + (1i64 << 40)); + assert_eq!( + member_id_to_short(low), + member_id_to_short(high), + "low-40-bit-equal ids must share a label" + ); + let r = MemberRef::Short(member_id_to_short(low)); + assert_eq!(r.resolve([low, high]), Some(low)); + assert_eq!(r.resolve([high, low]), Some(low), "order-independent"); + } + + #[test] + fn parses_mixed_legacy_and_current_tokens_in_one_body() { + // A single message may interleave an old hex token and a new base32 one + // (e.g. an edit, or a quote of an old message). Each parses to the right + // form independently and segment ordering is preserved. + let a = mid(0x0a); + let b = mid(0x0b); + let text = format!( + "x @[A](rv:{}) y {} z", + member_id_to_hex(a), // legacy hex form + encode_mention(b, "B") // current base32 form + ); + let segs = parse_segments(&text); + assert_eq!( + segs, + vec![ + MentionSegment::Text("x ".to_string()), + MentionSegment::Mention(Mention { + member_ref: MemberRef::Legacy(a), + display_name: "A".to_string() + }), + MentionSegment::Text(" y ".to_string()), + MentionSegment::Mention(Mention { + member_ref: MemberRef::Short(member_id_to_short(b)), + display_name: "B".to_string() + }), + MentionSegment::Text(" z".to_string()), + ] + ); + } + + #[test] + fn contains_mention_of_matches_legacy_hex_token() { + // A self-mention arriving in an OLD (hex) message must still be detected + // so it drives a notification. Hand-build the legacy form. + let me = mid(0x8f3a_2b1c_0000_d4e5u64 as i64); + let other = mid(7); + let text = format!("ping @[Me](rv:{})", member_id_to_hex(me)); + assert!(contains_mention_of(&text, me)); + assert!(!contains_mention_of(&text, other)); + } + + #[test] + fn multibyte_snapshot_name_round_trips_around_base32_ref() { + // The byte-scanning parser must handle multibyte text around a token and + // a multibyte snapshot name without splitting a codepoint. + let id = mid(0x1234_5678_9abc_def0u64 as i64); + let token = encode_mention(id, "名前🎉"); + let segs = parse_segments(&format!("こんにちは {token}!")); + let m = segs + .iter() + .find_map(|s| match s { + MentionSegment::Mention(m) => Some(m), + MentionSegment::Text(_) => None, + }) + .expect("exactly one mention"); + assert!(m.member_ref.matches(id)); + assert_eq!(m.display_name, "名前🎉"); + } + #[test] fn sanitize_strips_delimiters_and_caps_length() { assert_eq!(sanitize_name("a]b)c(d[e"), "abcde"); diff --git a/ui/src/components/conversation.rs b/ui/src/components/conversation.rs index 71b87aa0..bbfbebf3 100644 --- a/ui/src/components/conversation.rs +++ b/ui/src/components/conversation.rs @@ -3639,4 +3639,23 @@ mod tests { "unknown short ref must not fabricate a member id: {html}" ); } + + #[test] + fn legacy_hex_self_mention_gets_highlight() { + // A self-mention in an OLD (hex) message must still resolve to self and + // get the self-highlight class, just like the current short form. + let me = mid_from("0000000000000007"); + let mut names = HashMap::new(); + names.insert(me, "Me".to_string()); + let legacy = format!( + "@[Me]({}{})", + river_core::mention::REF_SCHEME, + river_core::mention::member_id_to_hex(me) + ); + let html = message_to_html_with_mentions(&legacy, &names, me); + assert!( + html.contains("river-mention-self"), + "legacy self-mention must get the self class: {html}" + ); + } }