diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 7e5ef062551..8ee30a2d7af 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -66,7 +66,10 @@ const overrides = new Map([ // the two p-gate filters can't drift) plus two guard unit tests. The file was // already at 995; this load-bearing correctness fix crossed 1000. Not generic // debt growth. Approved override; queued to split with the rest of this list. - ["src-tauri/src/commands/messages.rs", 1082], + // reply-RTT fix: thread_ref_from_cached_root + the cached_root_event_id + // parameter that lets replies skip the pre-send relay read of the parent. + // +24 lines of latency fix, not generic growth. Queued to split. + ["src-tauri/src/commands/messages.rs", 1106], // Residual repos_dir integration in ensure_nest_at: REPOS is provisioned // outside NEST_DIRS (it may be a symlink), so it needs its own create + // chmod-only-when-real-dir handling plus integration test coverage. The @@ -94,7 +97,9 @@ const overrides = new Map([ // #1418 read-path fix: +3 doc-only lines correcting the getThreadReplies // contract (replies-only, root excluded — the query keys on root_event_id, // which root rows lack). Documentation accuracy, not code growth. - ["src/shared/api/tauri.ts", 1340], + // reply-RTT fix: +2 lines threading cachedRootEventId through + // sendChannelMessage. Queued to split. + ["src/shared/api/tauri.ts", 1342], // harness-persona-sync feature growth, queued to split in the resolver-unify // refactor followup. discovery.rs is dominated by the new test module // (the effective_agent_command / divergent / create-time override matrix); diff --git a/desktop/src-tauri/src/commands/messages.rs b/desktop/src-tauri/src/commands/messages.rs index 3a057eaa18c..7be67864e3e 100644 --- a/desktop/src-tauri/src/commands/messages.rs +++ b/desktop/src-tauri/src/commands/messages.rs @@ -473,6 +473,23 @@ async fn resolve_thread_ref( }) } +/// Build a [`events::ThreadRef`] without the relay round-trip, from a root the +/// caller already resolved out of its local timeline cache. The cached root is +/// trusted only for tag construction — the relay still validates the reply +/// e-tags on submit, so a stale cache cannot produce an accepted-but-invalid +/// event, only a rejection identical to any other bad reply. +fn thread_ref_from_cached_root( + parent_event_id: &str, + root_event_id: &str, +) -> Result { + Ok(events::ThreadRef { + root_event_id: EventId::from_hex(root_event_id) + .map_err(|e| format!("invalid root event ID: {e}"))?, + parent_event_id: EventId::from_hex(parent_event_id) + .map_err(|e| format!("invalid parent event ID: {e}"))?, + }) +} + #[tauri::command] #[allow(clippy::too_many_arguments)] pub async fn send_channel_message( @@ -484,6 +501,7 @@ pub async fn send_channel_message( mention_tags: Option>>, mention_pubkeys: Option>, kind: Option, + cached_root_event_id: Option, state: State<'_, AppState>, ) -> Result { let channel_uuid = uuid::Uuid::parse_str(&channel_id) @@ -509,7 +527,10 @@ pub async fn send_channel_message( let parent_id = parent_event_id .as_deref() .ok_or("forum comment requires parent_event_id")?; - let thread_ref = resolve_thread_ref(parent_id, &state).await?; + let thread_ref = match cached_root_event_id.as_deref() { + Some(root) => thread_ref_from_cached_root(parent_id, root)?, + None => resolve_thread_ref(parent_id, &state).await?, + }; resolved_root = Some(thread_ref.root_event_id.to_hex()); events::build_forum_comment( channel_uuid, @@ -523,7 +544,10 @@ pub async fn send_channel_message( _ => { let thread_ref = match parent_event_id.as_deref() { Some(pid) => { - let tr = resolve_thread_ref(pid, &state).await?; + let tr = match cached_root_event_id.as_deref() { + Some(root) => thread_ref_from_cached_root(pid, root)?, + None => resolve_thread_ref(pid, &state).await?, + }; resolved_root = Some(tr.root_event_id.to_hex()); Some(tr) } diff --git a/desktop/src/features/messages/hooks.ts b/desktop/src/features/messages/hooks.ts index 81feccd0c40..e2421d1150b 100644 --- a/desktop/src/features/messages/hooks.ts +++ b/desktop/src/features/messages/hooks.ts @@ -13,6 +13,7 @@ import { getChannelIdFromTags, getThreadReference, normalizeMentionPubkeys, + resolveCachedReplyRoot, resolveReplyRootId, } from "@/features/messages/lib/threading"; import { splitOutgoingTags } from "@/features/messages/lib/imetaMediaMarkdown"; @@ -424,6 +425,12 @@ export function useSendMessageMutation( queryClient.getQueryData( channelMessagesKey(channel.id), ) ?? []; + // Resolve the thread root from the local timeline cache so the Rust + // command can skip its pre-send relay read of the parent (a full RTT + // on every reply). Null ⇒ cache miss ⇒ Rust falls back to the relay. + const cachedRootEventId = parentEventId + ? resolveCachedReplyRoot(parentEventId, cachedMessages) + : null; const result = await sendChannelMessage( channel.id, content, @@ -433,6 +440,7 @@ export function useSendMessageMutation( undefined, emojiTags, mentionTags, + cachedRootEventId, ); // Build tags matching relay-emitted shape: h, author p, mention ps, reply es, imeta, emoji. diff --git a/desktop/src/features/messages/lib/threading.resolveCachedReplyRoot.test.mjs b/desktop/src/features/messages/lib/threading.resolveCachedReplyRoot.test.mjs new file mode 100644 index 00000000000..f654d06f8bc --- /dev/null +++ b/desktop/src/features/messages/lib/threading.resolveCachedReplyRoot.test.mjs @@ -0,0 +1,90 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveCachedReplyRoot, resolveReplyRootId } from "./threading.ts"; + +// resolveCachedReplyRoot must mirror resolve_thread_ref in +// desktop/src-tauri/src/commands/messages.rs EXACTLY: a non-null result is +// what the Rust side would have fetched from the relay, so any divergence +// here silently changes reply threading. The relay's ingest-side ancestry +// check turns a wrong root into a rejected send — these tests keep us off +// that path entirely. + +const ROOT = "a".repeat(64); +const PARENT = "b".repeat(64); +const OTHER = "c".repeat(64); + +const ev = (id, kind, tags = []) => ({ id, kind, tags }); + +test("cache miss returns null (falls back to relay)", () => { + assert.equal(resolveCachedReplyRoot(PARENT, []), null); + assert.equal(resolveCachedReplyRoot(PARENT, [ev(OTHER, 9)]), null); +}); + +test("parent kind outside the Rust resolver's allowlist returns null", () => { + // resolve_thread_ref queries kinds [9, 40002, 45001, 45003, 48100] only — + // for any other cached kind the relay path would report "parent event not + // found", so the cached path must decline rather than diverge. + for (const kind of [1, 7, 45021, 40001]) { + assert.equal( + resolveCachedReplyRoot(PARENT, [ev(PARENT, kind)]), + null, + `kind ${kind} must fall back`, + ); + } + for (const kind of [9, 40002, 45001, 45003, 48100]) { + assert.equal( + resolveCachedReplyRoot(PARENT, [ev(PARENT, kind)]), + PARENT, + `kind ${kind} must resolve`, + ); + } +}); + +test("tagless parent is its own root", () => { + assert.equal(resolveCachedReplyRoot(PARENT, [ev(PARENT, 9)]), PARENT); +}); + +test("root marker wins over reply marker", () => { + const parent = ev(PARENT, 9, [ + ["e", OTHER, "", "reply"], + ["e", ROOT, "", "root"], + ]); + assert.equal(resolveCachedReplyRoot(PARENT, [parent]), ROOT); +}); + +test("reply marker used when no root marker (parent was a direct reply)", () => { + const parent = ev(PARENT, 9, [["e", ROOT, "", "reply"]]); + assert.equal(resolveCachedReplyRoot(PARENT, [parent]), ROOT); +}); + +test("last marker of each kind wins, matching the Rust tag walk", () => { + const parent = ev(PARENT, 9, [ + ["e", OTHER, "", "root"], + ["e", ROOT, "", "root"], + ]); + assert.equal(resolveCachedReplyRoot(PARENT, [parent]), ROOT); +}); + +test("marker pointing at the parent itself collapses to the parent", () => { + // Rust: `Some(hex) if hex != parent_event_id` — a self-referential tag + // means the parent IS the root. + const parent = ev(PARENT, 9, [["e", PARENT, "", "reply"]]); + assert.equal(resolveCachedReplyRoot(PARENT, [parent]), PARENT); +}); + +test("short/unmarked e-tags are ignored, as in the Rust s.len() >= 4 guard", () => { + const parent = ev(PARENT, 9, [ + ["e", OTHER], // no marker — mention-style tag + ["e", ROOT, ""], // len 3 — no marker slot + ]); + assert.equal(resolveCachedReplyRoot(PARENT, [parent]), PARENT); +}); + +test("does NOT inherit resolveReplyRootId's parent-id fallback on cache miss", () => { + // resolveReplyRootId returns the parent id when the parent isn't cached — + // safe for optimistic UI, catastrophic here: it would label a nested reply + // as a thread root. The cached resolver must return null instead. + assert.equal(resolveReplyRootId(PARENT, []), PARENT); + assert.equal(resolveCachedReplyRoot(PARENT, []), null); +}); diff --git a/desktop/src/features/messages/lib/threading.ts b/desktop/src/features/messages/lib/threading.ts index 4a18d026480..5346dcbabce 100644 --- a/desktop/src/features/messages/lib/threading.ts +++ b/desktop/src/features/messages/lib/threading.ts @@ -1,4 +1,21 @@ import type { RelayEvent } from "@/shared/api/types"; +import { + KIND_FORUM_COMMENT, + KIND_FORUM_POST, + KIND_HUDDLE_STARTED, + KIND_STREAM_MESSAGE, + KIND_STREAM_MESSAGE_V2, +} from "@/shared/constants/kinds"; + +// Kinds `resolve_thread_ref` (commands/messages.rs) accepts as reply parents. +// Keep in sync — a kind outside this set must fall back to relay resolution. +const CACHED_REPLY_PARENT_KINDS: readonly number[] = [ + KIND_STREAM_MESSAGE, // 9 + KIND_STREAM_MESSAGE_V2, // 40002 + KIND_FORUM_POST, // 45001 + KIND_FORUM_COMMENT, // 45003 + KIND_HUDDLE_STARTED, // 48100 +]; export type ThreadReference = { parentId: string | null; @@ -136,3 +153,42 @@ export function resolveReplyRootId( const thread = getThreadReference(parent.tags); return thread.rootId ?? parent.id; } + +/** + * Resolve the thread root for a reply from the local timeline cache, or + * `null` when the relay must be consulted. + * + * This mirrors `resolve_thread_ref` in `commands/messages.rs` exactly — last + * `root` marker wins, else last `reply` marker, else the parent itself — so a + * non-null result is byte-identical to what the Rust side would fetch from + * the relay. Returns `null` (⇒ caller falls back to relay resolution) when + * the parent is not cached or its kind is outside the set the Rust resolver + * queries; note {@link resolveReplyRootId}'s parent-id fallback is NOT safe + * here, as it would silently mislabel a nested reply as a thread root. + */ +export function resolveCachedReplyRoot( + parentEventId: string, + events: RelayEvent[], +): string | null { + const parent = events.find((event) => event.id === parentEventId); + if (!parent) { + return null; + } + if (!CACHED_REPLY_PARENT_KINDS.includes(parent.kind)) { + return null; + } + + let root: string | null = null; + let reply: string | null = null; + for (const tag of parent.tags) { + if (tag[0] === "e" && typeof tag[1] === "string" && tag.length >= 4) { + if (tag[3] === "root") { + root = tag[1]; + } else if (tag[3] === "reply") { + reply = tag[1]; + } + } + } + const rootHex = root ?? reply; + return rootHex && rootHex !== parentEventId ? rootHex : parent.id; +} diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index d7128eebe31..944449e6fe0 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -831,6 +831,7 @@ export async function sendChannelMessage( kind?: number, emojiTags?: string[][], mentionTags?: string[][], + cachedRootEventId?: string | null, ): Promise { const response = await invokeTauri( "send_channel_message", @@ -843,6 +844,7 @@ export async function sendChannelMessage( mentionTags: mentionTags ?? null, mentionPubkeys: mentionPubkeys ?? null, kind: kind ?? null, + cachedRootEventId: cachedRootEventId ?? null, }, );