From 0648e36d4e26147f111ed39085c2adcaeb965d66 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 19:42:39 +1100 Subject: [PATCH 1/8] fix(threads): open heavily reacted messages by exact id Opening a message by exact id (Inbox row, search hit, reminder, deep link, message-link preview, media viewer) failed with "Selected message exceeded its evidence limit" once it had 500 reactions. Kind 7 shared the single safety-overlay filter, and the session's exact target reader rejects any capped raw response because a truncated read cannot prove the message was not edited or deleted. Reactions cannot hide or change the target, so the overlay request now reads them through a separate `[7]` filter, and the guard counts only non-reaction events. Safety kinds (5, 9005, 40003, 39005) and tombstone responses still fail closed at 500. A full reaction page is kept as a bounded supplement and marks the thread view `limited` instead of failing the target. Older reactions beyond that page are not read. The loadMore guard now relies on canLoadMore alone (limited already implied no continuation), so a capped reaction page does not block older replies. Request count and latency: unchanged for every message. Reactions ride as a second filter in the existing overlay request; the relay applies `limit` per filter. Messages with few overlays still cost the same target, overlay and (when overlays exist) tombstone round trips. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/relay-queries.md | 6 ++- .../conversation/BuzzLinkPreview.test.tsx | 39 ++++++++++++++++- src/features/relay/session.ts | 3 +- src/features/relay/thread-target.test.ts | 42 ++++++++++++++++++- src/features/relay/threads.ts | 18 +++++++- 5 files changed, 101 insertions(+), 7 deletions(-) diff --git a/docs/relay-queries.md b/docs/relay-queries.md index f51bc7cf49..f0c16f0c20 100644 --- a/docs/relay-queries.md +++ b/docs/relay-queries.md @@ -271,7 +271,11 @@ never comes from the selected row. An accessible selected reply remains availabl even if the original root is missing or the reply lies beyond the traversal cap. Reference queries omit `#h` for legacy edits/deletes but retain session visibility -checks. Raw target/overlay responses reaching 500 events fail before filtering. +checks. Raw target, edit/deletion/summary and tombstone responses reaching 500 +events fail before filtering: a truncated read cannot prove the message was not +changed or removed. Reactions cannot hide or change the target, so the overlay +request reads them through a separate filter. A full page of reactions still opens +the target, with the view marked `limited`; older reactions are not read. Evidence shares the thread's 2,000-event / 4 MiB budget and 64-view ceiling. Known tombstones survive sparse refreshes and shared-cache eviction. Exact reads share verification, admission, access epochs and live reconciliation without diff --git a/src/features/conversation/BuzzLinkPreview.test.tsx b/src/features/conversation/BuzzLinkPreview.test.tsx index 113b96cd1d..a8a5a6dc7b 100644 --- a/src/features/conversation/BuzzLinkPreview.test.tsx +++ b/src/features/conversation/BuzzLinkPreview.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { act, cleanup, render } from "@testing-library/react"; +import { act, cleanup, render, waitFor } from "@testing-library/react"; import { afterEach, expect, it, vi } from "vitest"; import { BuzzLinkPreview, previewFailures } from "./BuzzLinkPreview"; import type { RelaySession } from "../relay/session"; @@ -10,6 +10,8 @@ import type { ChannelReference, } from "../relay/contracts"; import { mediaUrl } from "../relay/transport"; +import { createRelaySession } from "../relay/session"; +import { keypair, message, scriptedTransport, signed } from "../relay/testing"; afterEach(cleanup); const text = (tree: HTMLElement) => @@ -384,3 +386,38 @@ it("asks for the relay thumbnail of the author's picture, as message rows do", ( `https://relay.test/media/${sha}.thumb.jpg`, ); }); +it("previews a heavily reacted linked message through the real exact read", async () => { + const viewer = keypair(), + alice = keypair(); + const linked = message(alice, "a", "Popular announcement", 1); + const reactions = Array.from({ length: 500 }, (_, i) => + signed(alice, { + kind: 7, + content: "+", + created_at: 2 + i, + tags: [["e", linked.id]], + }), + ); + const owner = createRelaySession({ + ...scriptedTransport(viewer.pubkey, keypair().pubkey).transport, + query: async (filters) => + filters[0]?.ids?.includes(linked.id) + ? [linked] + : filters.some((filter) => filter.kinds?.includes(7)) + ? reactions + : [], + subscribe: () => ({ update() {}, retry() {}, dispose() {} }), + }); + const mounted = render( + , + ); + await waitFor(() => + expect(mounted.container.textContent).toContain("Popular announcement"), + ); + mounted.unmount(); + owner.dispose(); +}); diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 8ef96810b2..92e6eacc82 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -2519,9 +2519,10 @@ export function createRelaySession( throw new DOMException("Stale thread target", "AbortError"); settings?.signal?.throwIfAborted(); // A capped raw target/overlay read cannot establish a safe fold. + // Reactions cannot hide or change a message; the thread caps them. if ( !filters.some((filter) => filter.depth_limit) && - events.length >= 500 + events.filter((event) => event.kind !== 7).length >= 500 ) throw new Error("Selected message exceeded its evidence limit"); // The thread owner admits the complete target fold atomically. diff --git a/src/features/relay/thread-target.test.ts b/src/features/relay/thread-target.test.ts index 6660ed0e9a..8762709669 100644 --- a/src/features/relay/thread-target.test.ts +++ b/src/features/relay/thread-target.test.ts @@ -228,14 +228,52 @@ it("distinguishes absent targets and failed lookup, then permits explicit retry" expect(h.view.snapshot().target?.id).toBe(reply.id); }); -it("rejects a capped raw overlay response before visibility filtering hides its size", async () => { +it("opens a target with a full reaction page as limited, still loading older replies", async () => { + const h = setup(); + const reading = h.view.refresh(); + h.next().respond([reply]); + await flush(); + // Reactions share the safety-overlay request: no extra round trip. + const overlays = h.next(); + expect(overlays.filters.map((filter) => filter.kinds)).toEqual([ + [39005, 40003, 5, 9005], + [7], + ]); + overlays.respond( + Array.from({ length: 500 }, (_, i) => aux(7, reply, "+", 10001 + i)), + ); + await flush(); + h.next().respond([]); + await flush(); + h.next().respond([ + root, + message(alice, "a", "First reply", 2, [["e", root.id, "", "reply"]]), + ]); + await reading; + expect(h.view.snapshot()).toMatchObject({ + targetStatus: "ready", + target: { id: reply.id }, + limited: true, + canLoadMore: true, + }); + expect(h.view.snapshot().target?.reactions[0]?.events).toHaveLength(500); + const more = h.view.loadMore(); + h.next().respond([root]); + await more; + expect(h.view.snapshot()).toMatchObject({ + targetStatus: "ready", + limited: true, + }); +}); + +it("rejects a capped raw safety-overlay response before visibility filtering hides its size", async () => { const h = setup(); const reading = h.view.refresh(); h.next().respond([reply]); await flush(); const other = message(alice, "private", "Hidden", 2); h.next().respond( - Array.from({ length: 500 }, (_, i) => aux(7, other, `${i}`)), + Array.from({ length: 500 }, (_, i) => aux(5, other, `${i}`)), ); await reading; expect(h.view.snapshot()).toMatchObject({ diff --git a/src/features/relay/threads.ts b/src/features/relay/threads.ts index a5cdaa034a..5b91b47d3e 100644 --- a/src/features/relay/threads.ts +++ b/src/features/relay/threads.ts @@ -145,6 +145,8 @@ export function createThreadView({ let disposed = false; let rootId: string | undefined; let rootUnavailable = false; + // Retained target reactions came from a full page; more may exist. + let reactionsCapped = false; let targetStatus: ThreadSnapshot["targetStatus"] = exact ? "loading" : undefined; @@ -253,6 +255,7 @@ export function createThreadView({ emit({ ...snapshot, ...patch, + ...(reactionsCapped ? { limited: true } : {}), ...(exact ? { target: targetStatus === "ready" ? snapshot.target : undefined, @@ -334,6 +337,7 @@ export function createThreadView({ emit({ ...snapshot, ...patch, + ...(reactionsCapped ? { limited: true } : {}), root: !rootUnavailable ? readable.find((row) => row.id === rootId) : undefined, @@ -348,6 +352,7 @@ export function createThreadView({ selectionController?.abort(); selectionStaged = []; remote = []; + reactionsCapped = false; staged = []; rootId = undefined; cursor = undefined; @@ -392,6 +397,7 @@ export function createThreadView({ if (exact) targetStatus = "loading"; if (clear || !canAccess()) { remote = []; + reactionsCapped = false; staged = []; rootId = undefined; cursor = undefined; @@ -446,10 +452,16 @@ export function createThreadView({ // incomplete selected fold. Existing displayed content remains untouched. if (!stage([event])) return "error"; const overlays = await targetReader.read( - [{ kinds: [5, 7, 9005, 40003, 39005], "#e": [id], limit: 500 }], + [ + { kinds: [5, 9005, 40003, 39005], "#e": [id], limit: 500 }, + // Reactions cannot hide or change the target, so a full page is a + // limited supplement rather than incomplete safety evidence. + { kinds: [7], "#e": [id], limit: 500 }, + ], { signal: owned.signal }, ); if (!active()) return "cancelled"; + const capped = overlays.filter((event) => event.kind === 7).length >= 500; if (!stage(union(pending, related(overlays, id)))) return "error"; const ids = union(remote, pending) .filter( @@ -491,6 +503,7 @@ export function createThreadView({ if (!expectedRoot) rootId = resolvedRoot; const combined = union(remote, related(accepted, id)); if (!retain(combined)) return "error"; + reactionsCapped ||= capped; stage([]); if ( expectedRoot && @@ -561,7 +574,8 @@ export function createThreadView({ purge(); return; } - if (!replace && (!snapshot.canLoadMore || snapshot.limited)) return; + // Capped target reactions leave older replies loadable. + if (!replace && !snapshot.canLoadMore) return; const owned = new AbortController(); controller = owned; // Cleared as this refresh starts, not when the stream came back: a refresh From b1320405556cf0b66ece121bde2039ca08724cc9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 20:00:05 +1100 Subject: [PATCH 2/8] fix(profiles): name the members that fit instead of refusing large rosters `profiles.ensure` threw "Profile request exceeds the directory budget" for more than 1,024 ids before reading anything. Community admin raised an unhandled rejection and its name search found nobody, and `from:name` search in a channel failed with the same message. (The Members dialog already pages its name reads, #2028.) The directory now names the first 1,024 ids of a larger request (its cache's entry capacity) and leaves the rest unrequested. Fetching more would only evict names the same request had just loaded. Members beyond the cap keep their public-key label, and no notice is shown, because Try again cannot name them. The over-budget request still reaches verified-profile lookup only for the ids that fit (#2220). Two related partial-failure fixes in the same path: - A batch whose response exceeds the reader's 8 MiB budget retries from the same author in halves, like AgentBrowse's catalog pages, instead of loading zero names. A single author still over the budget fails after one read per halving (at most 10 reads for 500 authors). A foreground promotion rejoins whichever part is in flight. - `ensure` reports a failed batch only after its other batches settle, so a caller that tolerates the failure reads every name that loaded. Callers: Community admin catches the optional name read. Message search `from:` proceeds on a partial directory, as the author picker in SearchResults already does. An unnamed member falls through to the relay name search. Not changed: touch-on-read (`get()` for `event()`/`owner()`). Displayed names come from the folded snapshot, not the LRU, so promoting there would not make eviction follow what is on screen. Both functions are also `useSyncExternalStore` snapshot getters (useAgentOwnerEvidence, useProfileOwner) and run inside projection folds, so `get()` would make pure reads reorder the cache. A use-based policy belongs in ensure() with its own eviction tests. Performance: unchanged for requests of up to 1,024 ids whose batches fit the read budget. Request count, sizes and success latency are the same, plus one array slice. A request above 1,024 ids now costs at most three reads (500/500/24) instead of failing, and asking again with those names cached reads nothing. When a batch fails, ensure now rejects once its slowest batch settles rather than at the first failure. The large-roster `from:` test takes about 1 s locally, which this scale contract needs. Docs: docs/channels.md now says a request above the 1,024-entry budget names those that fit instead of failing. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/channels.md | 2 + src/app/shell/useSearchMessages.test.tsx | 76 +++++++++++ src/app/shell/useSearchMessages.ts | 7 +- .../moderation/CommunityAdmin.test.tsx | 23 +++- src/bundled/moderation/CommunityAdmin.tsx | 1 + src/bundled/relay-notes/NotesPage.test.tsx | 3 - src/features/relay/profile-details.test.ts | 125 +++++++++++++++++- src/features/relay/profile-directory.ts | 69 +++++++--- src/features/relay/verified-profile.test.ts | 21 ++- 9 files changed, 286 insertions(+), 41 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index 9c9dab1c7f..f3e14487c2 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -1204,6 +1204,8 @@ The port retains the prepared-store implementation and its behavior tests: menu reads a whole large roster. Narrow row profile selectors, and request-warmed avatars (fetched and decoded, nothing retained; disabled under the Save-Data preference). Signature verification yields in batches. + A request for more identities than the budget holds names those that fit + instead of failing. - Account/relay-scoped IndexedDB: 64 records / 8 MiB global disk budget, 24-hour expiry. Signed cached events are reverified before display. The same database stores account/relay-scoped startup discovery and sidebar organization (a separate diff --git a/src/app/shell/useSearchMessages.test.tsx b/src/app/shell/useSearchMessages.test.tsx index 137276ebbc..e6e1985360 100644 --- a/src/app/shell/useSearchMessages.test.tsx +++ b/src/app/shell/useSearchMessages.test.tsx @@ -6,6 +6,7 @@ import { keypair, message, metadata, + profile, roster, scriptedTransport, signed, @@ -122,3 +123,78 @@ it.each([ } }, ); + +it("resolves from:name on a roster larger than the profile budget when some names fail", async () => { + const viewer = keypair(); + const relay = keypair(); + const alice = keypair(); + const wire = scriptedTransport(viewer.pubkey, relay.pubkey); + const members = [ + viewer.pubkey, + alice.pubkey, + // Keys just above Alice's sort after hers, so she is in the first batch. + ...Array.from({ length: 1098 }, (_, index) => + (BigInt(`0x${alice.pubkey}`) + BigInt(index + 1)) + .toString(16) + .padStart(64, "0"), + ), + ]; + const discovery = [ + metadata(relay, "crew", "crew"), + roster(relay, "crew", members), + ]; + const owner = createRelaySession( + { + ...wire.transport, + writer: { + sign: async (template) => signed(viewer, template), + publish: async () => {}, + }, + query(filters, signal) { + if (filters.some((filter) => filter.kinds?.includes(9))) + return wire.transport.query(filters, signal); + const names = filters.find((filter) => filter.kinds?.includes(0)); + if (names) + return names.authors?.includes(alice.pubkey) + ? Promise.resolve([profile(alice, { name: "Alice" })]) + : Promise.reject(new Error("Names unavailable")); + return Promise.resolve( + discovery.filter((event) => + filters.some((filter) => filter.kinds?.includes(event.kind)), + ), + ); + }, + }, + { outboxStorage: { load: () => [], save() {} } }, + ); + try { + owner.session.channels.ensureList(); + await waitFor(() => + expect(owner.session.channels.get?.("crew")?.members).toHaveLength(1100), + ); + const { result } = renderHook(() => + useSearchMessages(owner.session, "from:alice ", "crew"), + ); + await waitFor(() => expect(wire.pending).toHaveLength(1)); + const read = wire.next(); + expect(read.filters).toEqual([ + { + kinds: [40002, 40008, 9], + limit: 20, + "#h": ["crew"], + authors: [alice.pubkey], + }, + ]); + await act(async () => + read.respond([message(alice, "crew", "hello", 1700000000)]), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.error).toBeUndefined(); + expect(result.current.messages.map(({ preview }) => preview)).toEqual([ + "hello", + ]); + } finally { + cleanup(); + owner.dispose(); + } +}); diff --git a/src/app/shell/useSearchMessages.ts b/src/app/shell/useSearchMessages.ts index cd47415a10..ad0a5922e5 100644 --- a/src/app/shell/useSearchMessages.ts +++ b/src/app/shell/useSearchMessages.ts @@ -221,7 +221,12 @@ export function useSearchMessages( ? (session.channels.get?.(channelId)?.members ?? []) : []; if (knownMembers.length) { - await session.profiles.ensure(knownMembers, "foreground"); + try { + await session.profiles.ensure(knownMembers, "foreground"); + } catch (error) { + if (controller.signal.aborted) throw error; + // A partial directory still contains verified member profiles. + } controller.signal.throwIfAborted(); } // A key matches by its profile name or, for a selectable agent, diff --git a/src/bundled/moderation/CommunityAdmin.test.tsx b/src/bundled/moderation/CommunityAdmin.test.tsx index 7aa258360c..ddeec13987 100644 --- a/src/bundled/moderation/CommunityAdmin.test.tsx +++ b/src/bundled/moderation/CommunityAdmin.test.tsx @@ -98,7 +98,7 @@ function relay( profiles: { subscribe: () => () => {}, snapshot: () => profiles, - ensure: async () => {}, + ensure: async (_ids: readonly string[]) => {}, }, }; const value = { @@ -110,6 +110,7 @@ function relay( }; return { read, + session, relay: { snapshot: () => value, subscribe: () => () => {}, @@ -150,6 +151,26 @@ it("gates administration to owners and admins by the verified roster", async () expect(screen.queryByRole("button", { name: "Invite members" })).toBeNull(); }); +it("keeps public-key labels when member names cannot load", async () => { + broker({}); + const { relay: data, session } = relay(owner); + const requested: (readonly string[])[] = []; + // Not a mock: a spy settles the promise itself, hiding an unhandled + // rejection, which Vitest otherwise fails the run on. + session.profiles.ensure = async (ids) => { + requested.push(ids); + throw new Error("Names unavailable"); + }; + render( true} />); + const keys = publicKeyLabels([owner, admin, member]); + expect( + await screen.findByRole("button", { + name: `Actions for ${keys.get(member)}`, + }), + ).toBeVisible(); + expect(requested).toEqual([[owner, admin, member]]); +}); + it("treats a roster signed by anyone else as unavailable", async () => { broker({}); const { relay: data } = relay(owner, [snapshot(roster, owner)]); diff --git a/src/bundled/moderation/CommunityAdmin.tsx b/src/bundled/moderation/CommunityAdmin.tsx index c86548b1d2..ebfefd87c9 100644 --- a/src/bundled/moderation/CommunityAdmin.tsx +++ b/src/bundled/moderation/CommunityAdmin.tsx @@ -151,6 +151,7 @@ function Members({ const profileGeneration = useProfileGeneration(session.profiles); useEffect(() => { void profileGeneration; + // Names are optional: members without one keep their public-key label. if (members) void session.profiles .ensure(members.map((member) => member.pubkey)) diff --git a/src/bundled/relay-notes/NotesPage.test.tsx b/src/bundled/relay-notes/NotesPage.test.tsx index c09c7641bd..29e5fec3b9 100644 --- a/src/bundled/relay-notes/NotesPage.test.tsx +++ b/src/bundled/relay-notes/NotesPage.test.tsx @@ -1169,9 +1169,6 @@ it("pages beyond the real profile directory budget and retains evicted agent cla ), } as unknown as Parameters[0]); try { - await expect( - directory.ensure(entries.map((entry) => entry.pubkey)), - ).rejects.toThrow("directory budget"); const f = fixture(); let page = 0; f.readPage.mockImplementation(async ([filter]) => { diff --git a/src/features/relay/profile-details.test.ts b/src/features/relay/profile-details.test.ts index 23b9b92a1e..c467b071e8 100644 --- a/src/features/relay/profile-details.test.ts +++ b/src/features/relay/profile-details.test.ts @@ -2,7 +2,8 @@ import { createHash } from "node:crypto"; import { schnorr } from "@noble/curves/secp256k1.js"; import { bytesToHex } from "nostr-tools/utils"; import { expect, it, vi } from "vitest"; -import { eventDto, type RelayEvent } from "./events"; +import { ReadError } from "./errors"; +import { eventDto, type ReadFilter, type RelayEvent } from "./events"; import { selectProfiles } from "./profile-selection"; import { foldProfiles } from "./profiles"; import { createProfileDirectory } from "./profile-directory"; @@ -593,9 +594,16 @@ it("holds an open channel's roster on top of the base and shrinks back when rele expect(snapshot.has(ids[1] as string)).toBe(false); expect(snapshot.has(ids[500] as string)).toBe(false); expect(snapshot.get(ids[501] as string)?.name).toBe("P501"); - await expect(directory.ensure(ids.slice(0, 1100))).rejects.toThrow( - "exceeds the directory budget", + // Past the shrunk limit, a request names only the profiles that fit. + read.mockClear(); + await directory.ensure( + unsignedProfiles(1100, "X").map((event) => event.pubkey), ); + expect( + read.mock.calls.map( + (call) => (call as unknown as [ReadFilter[]])[0][0]?.authors?.length, + ), + ).toEqual([500, 500, 24]); } finally { unsubscribe(); directory.dispose(); @@ -774,3 +782,114 @@ it("publishes a background read's pages once, also when the session accepts each vi.useRealTimers(); } }); + +const hexIds = (count: number, from = 0) => + Array.from({ length: count }, (_, index) => + (from + index).toString(16).padStart(64, "0"), + ); +/** Unsigned heads: the directory trusts its reader to have verified them. */ +const named = (pubkey: string) => + ({ + id: pubkey, + pubkey, + kind: 0, + created_at: 1, + tags: [], + content: JSON.stringify({ name: `Name ${pubkey.slice(-3)}` }), + sig: "", + }) as unknown as RelayEvent; +const overBudget = () => + new ReadError("invalid-response", "Relay response exceeds the read budget"); + +it("names the identities that fit a request larger than the cache instead of failing", async () => { + const ids = hexIds(1100); + const read = vi.fn(async ([filter]: readonly ReadFilter[]) => + (filter?.authors ?? []).map(named), + ); + const directory = createProfileDirectory({ read }); + try { + await directory.ensure(ids); + expect(read.mock.calls.map(([[filter]]) => filter?.authors)).toEqual([ + ids.slice(0, 500), + ids.slice(500, 1000), + ids.slice(1000, 1024), + ]); + expect([...directory.queries.snapshot().keys()].sort()).toEqual( + ids.slice(0, 1024), + ); + // Asking again reads nothing, so a retry cannot churn the cache. + await directory.ensure(ids); + expect(read).toHaveBeenCalledTimes(3); + } finally { + directory.dispose(); + } +}); + +it("retries profiles over the read budget in halves from the same author", async () => { + const ids = hexIds(500); + const absent = ids[499] as string; + const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { + const authors = filter?.authors ?? []; + if (authors.length > 200) throw overBudget(); + // Duplicate heads fill no page but pass the page size in total. + return authors + .filter((id) => id !== absent) + .flatMap((id) => [named(id), named(id)]); + }); + const directory = createProfileDirectory({ read }); + try { + await directory.ensure(ids); + expect( + read.mock.calls.map(([[filter]]) => [ + ids.indexOf(filter?.authors?.[0] ?? ""), + filter?.authors?.length, + ]), + ).toEqual([ + [0, 500], + [0, 250], + [0, 125], + [125, 125], + [250, 125], + [375, 125], + ]); + expect(directory.queries.snapshot().size).toBe(499); + // Every halved page came back short, so the read was complete. + expect(directory.queries.unnamed?.()).toEqual(new Set([absent])); + // A read still over the budget at one author fails, after one per halving. + read.mockClear(); + read.mockImplementation(async () => { + throw overBudget(); + }); + await expect(directory.ensure(hexIds(500, 500))).rejects.toThrow( + "Relay response exceeds the read budget", + ); + expect( + read.mock.calls.map(([[filter]]) => filter?.authors?.length), + ).toEqual([500, 250, 125, 63, 32, 16, 8, 4, 2, 1]); + expect(directory.queries.snapshot().size).toBe(499); + } finally { + directory.dispose(); + } +}); + +it("reports a failed batch only after the request's other batches land", async () => { + const ids = hexIds(600); + const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { + const authors = filter?.authors ?? []; + // Not a read-budget failure, so the batch fails without halving. + if (authors.length === 500) throw new ReadError("denied", "Names denied"); + // The other batch lands in a later task than that failure. + await new Promise((resolve) => setTimeout(resolve)); + return authors.map(named); + }); + const directory = createProfileDirectory({ read }); + try { + await expect(directory.ensure(ids)).rejects.toThrow("Names denied"); + expect([...directory.queries.snapshot().keys()].sort()).toEqual( + ids.slice(500), + ); + expect(read).toHaveBeenCalledTimes(2); + } finally { + directory.dispose(); + } +}); diff --git a/src/features/relay/profile-directory.ts b/src/features/relay/profile-directory.ts index 74b9171f92..890140352c 100644 --- a/src/features/relay/profile-directory.ts +++ b/src/features/relay/profile-directory.ts @@ -7,6 +7,7 @@ import type { import { retryRead } from "./read-retry"; import { attestedOwner } from "../agents/owner-attestation"; import { hasEventProof } from "./events"; +import { ReadError } from "./errors"; import type { Outbox } from "./outbox"; import { ByteLru, @@ -43,7 +44,7 @@ export interface ProfileQueries { * next task returns the limit to the largest remaining reservation and * evicts the least recently used profiles past it. */ reserve?(members: number): () => void; - /** Profiles the cache can hold now; one ensure may ask for at most this many. */ + /** Profiles the cache can hold now; one ensure names at most this many. */ limit?(): number; /** Moves when the budget drops loaded profiles or a shrink cancels pending * background reads. A view that requested names once and skips repeats must @@ -95,7 +96,8 @@ export function createProfileDirectory( work: Promise; authors: string[]; priority: Priority; - reading: boolean; + /** Authors of the read in flight, so a promotion can rejoin it. */ + reading: readonly string[] | undefined; refresh: boolean; cancel: AbortController; }; @@ -321,9 +323,9 @@ export function createProfileDirectory( refresh = false, ) { if (closed) throw new DOMException("Session closed", "AbortError"); - const unique = [...new Set(ids)]; - if (!refresh && unique.length > limit) - throw new Error("Profile request exceeds the directory budget"); + // More ids than the cache holds would only evict names this request + // loaded, so a larger request names its first ids and leaves the rest. + const unique = [...new Set(ids)].slice(0, refresh ? undefined : limit); // A hit marks the profile recently used, so the budget evicts profiles // nobody has asked for rather than the oldest ones still on screen. for (const id of unique) events.get(id); @@ -340,7 +342,7 @@ export function createProfileDirectory( if (batch.reading) void reader .read( - [{ kinds: [0], authors: batch.authors, limit: PROFILE_PAGE }], + [{ kinds: [0], authors: batch.reading, limit: PROFILE_PAGE }], { signal, priority }, ) .catch(() => {}); @@ -363,29 +365,50 @@ export function createProfileDirectory( for (let offset = 0; offset < missing.length; offset += PROFILE_PAGE) { const authors = missing.slice(offset, offset + PROFILE_PAGE); let answered = false; + let filled = false; const batch = { authors, priority, - reading: false, + reading: undefined as readonly string[] | undefined, refresh, cancel: new AbortController(), work: Promise.resolve(), }; batch.work = retryRead( async (signal) => { - // A promotion during backoff applies to the next attempt instead. - batch.reading = true; attempts.set(signal, batch); - try { - const result = await reader.read( - [{ kinds: [0], authors, limit: PROFILE_PAGE }], - { signal, priority: batch.priority }, - ); - answered = true; - return result; - } finally { - batch.reading = false; + const result: RelayEvent[] = []; + filled = false; + // A response over the read budget retries in halves from the same + // author, like AgentBrowse's catalog pages; one profile over it fails. + for (let at = 0, size = authors.length; at < authors.length; ) { + const part = authors.slice(at, at + size); + // A promotion during backoff applies to the next attempt instead. + batch.reading = part; + try { + const page = await reader.read( + [{ kinds: [0], authors: part, limit: PROFILE_PAGE }], + { signal, priority: batch.priority }, + ); + result.push(...page); + // A page at the limit may have left out authors who do have one. + if (page.length >= PROFILE_PAGE) filled = true; + at += part.length; + } catch (error) { + if ( + part.length < 2 || + !(error instanceof ReadError) || + error.kind !== "invalid-response" || + error.message !== "Relay response exceeds the read budget" + ) + throw error; + size = Math.ceil(part.length / 2); + } finally { + batch.reading = undefined; + } } + answered = true; + return result; }, AbortSignal.any([signal, batch.cancel.signal]), ) @@ -397,8 +420,7 @@ export function createProfileDirectory( authors.includes(event.pubkey), ); take(profiles, fold); - // A page at the limit may have left out authors who do have one. - if (result.length < PROFILE_PAGE) + if (!filled) missed( authors, new Set(profiles.map((event) => event.pubkey)), @@ -421,10 +443,15 @@ export function createProfileDirectory( for (const id of authors) pending.set(id, batch); waiting.add(batch.work); } - await Promise.all(waiting); + // Every batch settles before a failure is reported, so a caller that + // tolerates it reads all the names that did load. + const failed = (await Promise.allSettled(waiting)).find( + (outcome) => outcome.status === "rejected", + ); // Every caller sees its names published when its ensure resolves, also // one that joined another caller's read. flush(); + if (failed) throw failed.reason; } function clear() { controller.abort(); diff --git a/src/features/relay/verified-profile.test.ts b/src/features/relay/verified-profile.test.ts index 611b3a49b4..04cba1b198 100644 --- a/src/features/relay/verified-profile.test.ts +++ b/src/features/relay/verified-profile.test.ts @@ -614,7 +614,7 @@ it("wires session profile demand, live updates, disconnection, and refreshed dis const wire = scriptedTransport(alice.pubkey, relay.pubkey); let live: import("./live").LiveCallbacks | undefined; let sink: VerifiedProfileSink | undefined; - let requests = 0; + const subjects = new Set(); const refresh = vi.fn(async () => null); const owner = createRelaySession({ ...wire.transport, @@ -627,8 +627,8 @@ it("wires session profile demand, live updates, disconnection, and refreshed dis update() {}, retry() {}, dispose() {}, - syncVerifiedProfiles(_filter, next) { - requests++; + syncVerifiedProfiles(filter, next) { + for (const subject of filter["#d"]) subjects.add(subject); sink = next; return () => {}; }, @@ -638,16 +638,13 @@ it("wires session profile demand, live updates, disconnection, and refreshed dis try { assert(live); live.state({ status: "connected", routes: [] }); - // An over-budget request is rejected before it reaches verification. - await expect( - owner.session.profiles.ensure( - Array.from({ length: 1025 }, (_, index) => - index.toString(16).padStart(64, "0"), - ), - ), - ).rejects.toThrow("budget"); + // An over-budget request verifies only the identities that fit. + const ids = Array.from({ length: 1025 }, (_, index) => + index.toString(16).padStart(64, "0"), + ); + await owner.session.profiles.ensure(ids); await Promise.resolve(); - expect(requests).toBe(0); + expect(subjects).toEqual(new Set(ids.slice(0, 1024))); await owner.session.profiles.ensure([alice.pubkey]); assert(sink); sink.event(projection()); From 8f6101fb277b8cc3a602952fb5c17cb9d45777a6 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 23:41:38 +1100 Subject: [PATCH 3/8] fix(relay): address review of the capacity-cliff fixes Follow-up to the capacity-cliff review of 2026-10-29 12:02 UTC. The discovery eviction and Members dialog parts of that review were dropped in the rebase onto main: #1574 replaced discovery's 1,024-entry cap with a 256 MiB byte budget and pins the shortened-id fallback, and #2028 pages the dialog's name reads. Review: threads.ts `reactionsCapped` - Dropped it. A full 500-reaction page forced `limited: true`, which ThreadPanel and MediaReviewViewer show as "Thread history limit reached." even while older replies still load. The target still opens, its 500 reactions are a lower bound, and live reactions keep arriving. relay-queries.md no longer says the view is marked `limited`. Review: halving gaps (a) and (b) - Each part is accepted as it lands. A transient failure on part k retries from part k at the halved size instead of from author 0. Kept because the code is no larger than before (the result array and `.then` go away) and is safe: - retryRead runs attempts one at a time with the caller's signal, so two attempts never share the hoisted cursor. - A part that lands after an abort is dropped, as before. - A promotion still rejoins `batch.reading`. Deliberately not changed: - Warm-cache eviction. ByteLru evicts in insertion order and nothing touches an entry on use. A request that loads enough new names therefore evicts the viewer's own profile and on-screen names, and so does a long session once it has seen 1,024 profiles. This predates the branch at every request size. The fix is a use-based policy in ensure(), or reading only the rows that render, as a follow-up. - Halving is still unmeasured against a real 8 MiB response. Performance: - Profile batches that fit the read budget are unchanged: one accept per batch. A halved batch publishes once per part rather than once per batch, so earlier parts' names show one read sooner. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/relay-queries.md | 2 +- src/features/relay/profile-details.test.ts | 49 +++++++++++++++++++++- src/features/relay/profile-directory.ts | 40 ++++++++---------- src/features/relay/thread-target.test.ts | 6 +-- src/features/relay/threads.ts | 11 +---- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/docs/relay-queries.md b/docs/relay-queries.md index f0c16f0c20..502d6c5a9e 100644 --- a/docs/relay-queries.md +++ b/docs/relay-queries.md @@ -275,7 +275,7 @@ checks. Raw target, edit/deletion/summary and tombstone responses reaching 500 events fail before filtering: a truncated read cannot prove the message was not changed or removed. Reactions cannot hide or change the target, so the overlay request reads them through a separate filter. A full page of reactions still opens -the target, with the view marked `limited`; older reactions are not read. +the target; older reactions are not read. Evidence shares the thread's 2,000-event / 4 MiB budget and 64-view ceiling. Known tombstones survive sparse refreshes and shared-cache eviction. Exact reads share verification, admission, access epochs and live reconciliation without diff --git a/src/features/relay/profile-details.test.ts b/src/features/relay/profile-details.test.ts index c467b071e8..adfda3b728 100644 --- a/src/features/relay/profile-details.test.ts +++ b/src/features/relay/profile-details.test.ts @@ -827,7 +827,7 @@ it("names the identities that fit a request larger than the cache instead of fai it("retries profiles over the read budget in halves from the same author", async () => { const ids = hexIds(500); - const absent = ids[499] as string; + const absent = ids[499]; const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { const authors = filter?.authors ?? []; if (authors.length > 200) throw overBudget(); @@ -872,6 +872,53 @@ it("retries profiles over the read budget in halves from the same author", async } }); +it("keeps the parts that landed when a later part fails, and retries only that part", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const ids = hexIds(500); + let outages = 1; + const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { + const authors = filter?.authors ?? []; + if (authors.length > 250) throw overBudget(); + if (authors[0] === ids[250] && outages-- > 0) + throw new ReadError("unavailable", "Relay unavailable"); + if (authors[0] === hexIds(1, 750)[0]) + throw new ReadError("denied", "Names denied"); + return authors.map(named); + }); + const directory = createProfileDirectory({ read }); + try { + const ensuring = directory.ensure(ids); + await vi.advanceTimersByTimeAsync(0); + // The first part is named while the second waits to retry. + expect(read).toHaveBeenCalledTimes(3); + expect([...directory.queries.snapshot().keys()].sort()).toEqual( + ids.slice(0, 250), + ); + await vi.advanceTimersByTimeAsync(1000); + await ensuring; + expect( + read.mock.calls.map(([[filter]]) => [ + ids.indexOf(filter?.authors?.[0] ?? ""), + filter?.authors?.length, + ]), + ).toEqual([ + [0, 500], + [0, 250], + [250, 250], + [250, 250], + ]); + expect(directory.queries.snapshot().size).toBe(500); + // A part that fails for good leaves the parts before it named. + await expect(directory.ensure(hexIds(500, 500))).rejects.toThrow( + "Names denied", + ); + expect(directory.queries.snapshot().size).toBe(750); + } finally { + vi.useRealTimers(); + directory.dispose(); + } +}); + it("reports a failed batch only after the request's other batches land", async () => { const ids = hexIds(600); const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { diff --git a/src/features/relay/profile-directory.ts b/src/features/relay/profile-directory.ts index 890140352c..8c0cc08e3c 100644 --- a/src/features/relay/profile-directory.ts +++ b/src/features/relay/profile-directory.ts @@ -374,14 +374,16 @@ export function createProfileDirectory( cancel: new AbortController(), work: Promise.resolve(), }; + // A response over the read budget retries in halves from the same + // author, like AgentBrowse's catalog pages; one profile over it fails. + // Each part is kept as it lands, and a retry resumes at the failed part. + let at = 0, + size = authors.length; + const returned = new Set(); batch.work = retryRead( async (signal) => { attempts.set(signal, batch); - const result: RelayEvent[] = []; - filled = false; - // A response over the read budget retries in halves from the same - // author, like AgentBrowse's catalog pages; one profile over it fails. - for (let at = 0, size = authors.length; at < authors.length; ) { + while (at < authors.length) { const part = authors.slice(at, at + size); // A promotion during backoff applies to the next attempt instead. batch.reading = part; @@ -390,7 +392,15 @@ export function createProfileDirectory( [{ kinds: [0], authors: part, limit: PROFILE_PAGE }], { signal, priority: batch.priority }, ); - result.push(...page); + if (!signal.aborted) { + const profiles = page.filter((event) => + part.includes(event.pubkey), + ); + for (const event of profiles) returned.add(event.pubkey); + // A foreground reader that joined this read gets its names at + // once. + take(profiles, batch.priority === "background"); + } // A page at the limit may have left out authors who do have one. if (page.length >= PROFILE_PAGE) filled = true; at += part.length; @@ -408,25 +418,11 @@ export function createProfileDirectory( } } answered = true; - return result; + if (!signal.aborted && !filled) + missed(authors, returned, batch.priority === "background"); }, AbortSignal.any([signal, batch.cancel.signal]), ) - .then((result) => { - if (signal.aborted || batch.cancel.signal.aborted) return; - // A foreground reader that joined this read gets its names at once. - const fold = batch.priority === "background"; - const profiles = result.filter((event) => - authors.includes(event.pubkey), - ); - take(profiles, fold); - if (!filled) - missed( - authors, - new Set(profiles.map((event) => event.pubkey)), - fold, - ); - }) .catch((error: unknown) => { // A shrink cancels a background read quietly; evictions() tells its // requester to ask again. diff --git a/src/features/relay/thread-target.test.ts b/src/features/relay/thread-target.test.ts index 8762709669..4af2a2b641 100644 --- a/src/features/relay/thread-target.test.ts +++ b/src/features/relay/thread-target.test.ts @@ -228,7 +228,7 @@ it("distinguishes absent targets and failed lookup, then permits explicit retry" expect(h.view.snapshot().target?.id).toBe(reply.id); }); -it("opens a target with a full reaction page as limited, still loading older replies", async () => { +it("opens a target with a full reaction page, still loading older replies", async () => { const h = setup(); const reading = h.view.refresh(); h.next().respond([reply]); @@ -253,7 +253,7 @@ it("opens a target with a full reaction page as limited, still loading older rep expect(h.view.snapshot()).toMatchObject({ targetStatus: "ready", target: { id: reply.id }, - limited: true, + limited: false, canLoadMore: true, }); expect(h.view.snapshot().target?.reactions[0]?.events).toHaveLength(500); @@ -262,7 +262,7 @@ it("opens a target with a full reaction page as limited, still loading older rep await more; expect(h.view.snapshot()).toMatchObject({ targetStatus: "ready", - limited: true, + limited: false, }); }); diff --git a/src/features/relay/threads.ts b/src/features/relay/threads.ts index 5b91b47d3e..6806a2c5e4 100644 --- a/src/features/relay/threads.ts +++ b/src/features/relay/threads.ts @@ -145,8 +145,6 @@ export function createThreadView({ let disposed = false; let rootId: string | undefined; let rootUnavailable = false; - // Retained target reactions came from a full page; more may exist. - let reactionsCapped = false; let targetStatus: ThreadSnapshot["targetStatus"] = exact ? "loading" : undefined; @@ -255,7 +253,6 @@ export function createThreadView({ emit({ ...snapshot, ...patch, - ...(reactionsCapped ? { limited: true } : {}), ...(exact ? { target: targetStatus === "ready" ? snapshot.target : undefined, @@ -337,7 +334,6 @@ export function createThreadView({ emit({ ...snapshot, ...patch, - ...(reactionsCapped ? { limited: true } : {}), root: !rootUnavailable ? readable.find((row) => row.id === rootId) : undefined, @@ -352,7 +348,6 @@ export function createThreadView({ selectionController?.abort(); selectionStaged = []; remote = []; - reactionsCapped = false; staged = []; rootId = undefined; cursor = undefined; @@ -397,7 +392,6 @@ export function createThreadView({ if (exact) targetStatus = "loading"; if (clear || !canAccess()) { remote = []; - reactionsCapped = false; staged = []; rootId = undefined; cursor = undefined; @@ -455,13 +449,12 @@ export function createThreadView({ [ { kinds: [5, 9005, 40003, 39005], "#e": [id], limit: 500 }, // Reactions cannot hide or change the target, so a full page is a - // limited supplement rather than incomplete safety evidence. + // lower bound rather than incomplete safety evidence. { kinds: [7], "#e": [id], limit: 500 }, ], { signal: owned.signal }, ); if (!active()) return "cancelled"; - const capped = overlays.filter((event) => event.kind === 7).length >= 500; if (!stage(union(pending, related(overlays, id)))) return "error"; const ids = union(remote, pending) .filter( @@ -503,7 +496,6 @@ export function createThreadView({ if (!expectedRoot) rootId = resolvedRoot; const combined = union(remote, related(accepted, id)); if (!retain(combined)) return "error"; - reactionsCapped ||= capped; stage([]); if ( expectedRoot && @@ -574,7 +566,6 @@ export function createThreadView({ purge(); return; } - // Capped target reactions leave older replies loadable. if (!replace && !snapshot.canLoadMore) return; const owned = new AbortController(); controller = owned; From 23a5e79dc625c9db5cb92e2b95f7948dffa2bede Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 00:14:33 +1100 Subject: [PATCH 4/8] fix(relay): address second review of the capacity-cliff fixes Follow-up covering the review of 2026-10-29 12:53 UTC. The discovery eviction, channel-link pin and Members dialog parts were dropped in the rebase onto main (#1574 replaced the 1,024-entry discovery cap with a 256 MiB byte budget; #2028 pages the dialog's name reads). Review: profile halving - Dropped. The 2 MiB profile cache keeps only about a quarter of an 8 MiB response, so the parts that landed evicted each other, and the case was never measured. Removed the hoisted cursor, the `ReadError` match and their two tests. The batch read is main's again. - Kept: the 1,024-id cap, the Community admin and `from:` search catches, and settling every batch before a failure is reported. - Docs: channels.md says a batch whose response exceeds the 8 MiB read budget (for example heavy legacy inline avatars) still loads no names, and that reading only rendered rows is a follow-up. Review: threads.ts loadMore guard - Reverted to `!snapshot.canLoadMore || snapshot.limited`. With `reactionsCapped` gone it is equivalent, and the thread diff is now only the separate reaction filter. Tombstone read over the request budget: fixed in code - The target's tombstone read sent every overlay id in one `#e` list. The reader rejects filters over 64 KiB (about 975 ids) before sending, so a target with more overlay ids failed to open. One target read can return 499 safety overlays plus 500 reactions, and `remote` can hold more. Carrying only this response's ids would still exceed the budget. It would also stop re-reading tombstones for overlays already in `remote`. So the read now goes in parts of 900 ids: a `for` loop and a slice in place of the `if`, with no new state. Each part keeps the 500-event fail-closed check. Up to 900 ids it is one request, as before. - Test: the full-reaction-page target test now also carries 480 edits (980 overlay ids). It asserts two tombstone requests of 900 and 80 ids covering every overlay, and the newest edit as the content. Before, the reader rejected the request locally and no tombstone query was sent. Performance: - No read is added anywhere that worked before. Tombstone parts only apply above 900 overlay ids, which used to fail. - Profile batches are back to one accept per batch. - thread-target.test.ts signs its 500 reactions and 480 edits once in beforeAll (about 1.45 s); before, 500 were signed inside the test. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/channels.md | 4 +- src/features/relay/profile-details.test.ts | 102 +-------------------- src/features/relay/profile-directory.ts | 78 ++++++---------- src/features/relay/thread-target.test.ts | 33 +++++-- src/features/relay/threads.ts | 7 +- 5 files changed, 63 insertions(+), 161 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index f3e14487c2..ba1fd7a761 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -1205,7 +1205,9 @@ The port retains the prepared-store implementation and its behavior tests: and request-warmed avatars (fetched and decoded, nothing retained; disabled under the Save-Data preference). Signature verification yields in batches. A request for more identities than the budget holds names those that fit - instead of failing. + instead of failing. A batch whose response exceeds the 8 MiB read budget, + such as one heavy with legacy inline avatars, still loads no names; reading + only the rendered rows is a follow-up. - Account/relay-scoped IndexedDB: 64 records / 8 MiB global disk budget, 24-hour expiry. Signed cached events are reverified before display. The same database stores account/relay-scoped startup discovery and sidebar organization (a separate diff --git a/src/features/relay/profile-details.test.ts b/src/features/relay/profile-details.test.ts index adfda3b728..3e698e5ff2 100644 --- a/src/features/relay/profile-details.test.ts +++ b/src/features/relay/profile-details.test.ts @@ -783,9 +783,9 @@ it("publishes a background read's pages once, also when the session accepts each } }); -const hexIds = (count: number, from = 0) => +const hexIds = (count: number) => Array.from({ length: count }, (_, index) => - (from + index).toString(16).padStart(64, "0"), + index.toString(16).padStart(64, "0"), ); /** Unsigned heads: the directory trusts its reader to have verified them. */ const named = (pubkey: string) => @@ -798,8 +798,6 @@ const named = (pubkey: string) => content: JSON.stringify({ name: `Name ${pubkey.slice(-3)}` }), sig: "", }) as unknown as RelayEvent; -const overBudget = () => - new ReadError("invalid-response", "Relay response exceeds the read budget"); it("names the identities that fit a request larger than the cache instead of failing", async () => { const ids = hexIds(1100); @@ -825,105 +823,11 @@ it("names the identities that fit a request larger than the cache instead of fai } }); -it("retries profiles over the read budget in halves from the same author", async () => { - const ids = hexIds(500); - const absent = ids[499]; - const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { - const authors = filter?.authors ?? []; - if (authors.length > 200) throw overBudget(); - // Duplicate heads fill no page but pass the page size in total. - return authors - .filter((id) => id !== absent) - .flatMap((id) => [named(id), named(id)]); - }); - const directory = createProfileDirectory({ read }); - try { - await directory.ensure(ids); - expect( - read.mock.calls.map(([[filter]]) => [ - ids.indexOf(filter?.authors?.[0] ?? ""), - filter?.authors?.length, - ]), - ).toEqual([ - [0, 500], - [0, 250], - [0, 125], - [125, 125], - [250, 125], - [375, 125], - ]); - expect(directory.queries.snapshot().size).toBe(499); - // Every halved page came back short, so the read was complete. - expect(directory.queries.unnamed?.()).toEqual(new Set([absent])); - // A read still over the budget at one author fails, after one per halving. - read.mockClear(); - read.mockImplementation(async () => { - throw overBudget(); - }); - await expect(directory.ensure(hexIds(500, 500))).rejects.toThrow( - "Relay response exceeds the read budget", - ); - expect( - read.mock.calls.map(([[filter]]) => filter?.authors?.length), - ).toEqual([500, 250, 125, 63, 32, 16, 8, 4, 2, 1]); - expect(directory.queries.snapshot().size).toBe(499); - } finally { - directory.dispose(); - } -}); - -it("keeps the parts that landed when a later part fails, and retries only that part", async () => { - vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); - const ids = hexIds(500); - let outages = 1; - const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { - const authors = filter?.authors ?? []; - if (authors.length > 250) throw overBudget(); - if (authors[0] === ids[250] && outages-- > 0) - throw new ReadError("unavailable", "Relay unavailable"); - if (authors[0] === hexIds(1, 750)[0]) - throw new ReadError("denied", "Names denied"); - return authors.map(named); - }); - const directory = createProfileDirectory({ read }); - try { - const ensuring = directory.ensure(ids); - await vi.advanceTimersByTimeAsync(0); - // The first part is named while the second waits to retry. - expect(read).toHaveBeenCalledTimes(3); - expect([...directory.queries.snapshot().keys()].sort()).toEqual( - ids.slice(0, 250), - ); - await vi.advanceTimersByTimeAsync(1000); - await ensuring; - expect( - read.mock.calls.map(([[filter]]) => [ - ids.indexOf(filter?.authors?.[0] ?? ""), - filter?.authors?.length, - ]), - ).toEqual([ - [0, 500], - [0, 250], - [250, 250], - [250, 250], - ]); - expect(directory.queries.snapshot().size).toBe(500); - // A part that fails for good leaves the parts before it named. - await expect(directory.ensure(hexIds(500, 500))).rejects.toThrow( - "Names denied", - ); - expect(directory.queries.snapshot().size).toBe(750); - } finally { - vi.useRealTimers(); - directory.dispose(); - } -}); - it("reports a failed batch only after the request's other batches land", async () => { const ids = hexIds(600); const read = vi.fn(async ([filter]: readonly ReadFilter[]) => { const authors = filter?.authors ?? []; - // Not a read-budget failure, so the batch fails without halving. + // Not retried, so the batch fails at once. if (authors.length === 500) throw new ReadError("denied", "Names denied"); // The other batch lands in a later task than that failure. await new Promise((resolve) => setTimeout(resolve)); diff --git a/src/features/relay/profile-directory.ts b/src/features/relay/profile-directory.ts index 8c0cc08e3c..1fafc7ac71 100644 --- a/src/features/relay/profile-directory.ts +++ b/src/features/relay/profile-directory.ts @@ -7,7 +7,6 @@ import type { import { retryRead } from "./read-retry"; import { attestedOwner } from "../agents/owner-attestation"; import { hasEventProof } from "./events"; -import { ReadError } from "./errors"; import type { Outbox } from "./outbox"; import { ByteLru, @@ -96,8 +95,7 @@ export function createProfileDirectory( work: Promise; authors: string[]; priority: Priority; - /** Authors of the read in flight, so a promotion can rejoin it. */ - reading: readonly string[] | undefined; + reading: boolean; refresh: boolean; cancel: AbortController; }; @@ -342,7 +340,7 @@ export function createProfileDirectory( if (batch.reading) void reader .read( - [{ kinds: [0], authors: batch.reading, limit: PROFILE_PAGE }], + [{ kinds: [0], authors: batch.authors, limit: PROFILE_PAGE }], { signal, priority }, ) .catch(() => {}); @@ -365,64 +363,48 @@ export function createProfileDirectory( for (let offset = 0; offset < missing.length; offset += PROFILE_PAGE) { const authors = missing.slice(offset, offset + PROFILE_PAGE); let answered = false; - let filled = false; const batch = { authors, priority, - reading: undefined as readonly string[] | undefined, + reading: false, refresh, cancel: new AbortController(), work: Promise.resolve(), }; - // A response over the read budget retries in halves from the same - // author, like AgentBrowse's catalog pages; one profile over it fails. - // Each part is kept as it lands, and a retry resumes at the failed part. - let at = 0, - size = authors.length; - const returned = new Set(); batch.work = retryRead( async (signal) => { + // A promotion during backoff applies to the next attempt instead. + batch.reading = true; attempts.set(signal, batch); - while (at < authors.length) { - const part = authors.slice(at, at + size); - // A promotion during backoff applies to the next attempt instead. - batch.reading = part; - try { - const page = await reader.read( - [{ kinds: [0], authors: part, limit: PROFILE_PAGE }], - { signal, priority: batch.priority }, - ); - if (!signal.aborted) { - const profiles = page.filter((event) => - part.includes(event.pubkey), - ); - for (const event of profiles) returned.add(event.pubkey); - // A foreground reader that joined this read gets its names at - // once. - take(profiles, batch.priority === "background"); - } - // A page at the limit may have left out authors who do have one. - if (page.length >= PROFILE_PAGE) filled = true; - at += part.length; - } catch (error) { - if ( - part.length < 2 || - !(error instanceof ReadError) || - error.kind !== "invalid-response" || - error.message !== "Relay response exceeds the read budget" - ) - throw error; - size = Math.ceil(part.length / 2); - } finally { - batch.reading = undefined; - } + try { + const result = await reader.read( + [{ kinds: [0], authors, limit: PROFILE_PAGE }], + { signal, priority: batch.priority }, + ); + answered = true; + return result; + } finally { + batch.reading = false; } - answered = true; - if (!signal.aborted && !filled) - missed(authors, returned, batch.priority === "background"); }, AbortSignal.any([signal, batch.cancel.signal]), ) + .then((result) => { + if (signal.aborted || batch.cancel.signal.aborted) return; + // A foreground reader that joined this read gets its names at once. + const fold = batch.priority === "background"; + const profiles = result.filter((event) => + authors.includes(event.pubkey), + ); + take(profiles, fold); + // A page at the limit may have left out authors who do have one. + if (result.length < PROFILE_PAGE) + missed( + authors, + new Set(profiles.map((event) => event.pubkey)), + fold, + ); + }) .catch((error: unknown) => { // A shrink cancels a background read quietly; evictions() tells its // requester to ask again. diff --git a/src/features/relay/thread-target.test.ts b/src/features/relay/thread-target.test.ts index 4af2a2b641..954b4f3706 100644 --- a/src/features/relay/thread-target.test.ts +++ b/src/features/relay/thread-target.test.ts @@ -1,4 +1,4 @@ -import { afterEach, expect, it } from "vitest"; +import { afterEach, beforeAll, expect, it } from "vitest"; import { createRelaySession } from "./session"; import { createThreadView } from "./threads"; import { ReadError } from "./errors"; @@ -27,6 +27,13 @@ const reply = message(alice, "a", "Selected reply", 10000, [ ]); const aux = (kind: number, target: RelayEvent, content = "", time = 10001) => signed(alice, { kind, content, created_at: time, tags: [["e", target.id]] }); +/** A full reaction page and 480 edits of `reply`, signed once. */ +const overlays: RelayEvent[] = []; +beforeAll(() => { + for (let i = 0; i < 500; i++) overlays.push(aux(7, reply, "+", 10001 + i)); + for (let i = 0; i < 480; i++) + overlays.push(aux(40003, reply, `Edit ${i}`, 10001 + i)); +}); function setup(id = reply.id) { const wire = scriptedTransport(viewer.pubkey, relay.pubkey); let traffic!: LiveCallbacks; @@ -228,23 +235,29 @@ it("distinguishes absent targets and failed lookup, then permits explicit retry" expect(h.view.snapshot().target?.id).toBe(reply.id); }); -it("opens a target with a full reaction page, still loading older replies", async () => { +it("opens a target with a full reaction page and hundreds of edits, still loading older replies", async () => { const h = setup(); const reading = h.view.refresh(); h.next().respond([reply]); await flush(); // Reactions share the safety-overlay request: no extra round trip. - const overlays = h.next(); - expect(overlays.filters.map((filter) => filter.kinds)).toEqual([ + const request = h.next(); + expect(request.filters.map((filter) => filter.kinds)).toEqual([ [39005, 40003, 5, 9005], [7], ]); - overlays.respond( - Array.from({ length: 500 }, (_, i) => aux(7, reply, "+", 10001 + i)), - ); - await flush(); - h.next().respond([]); + request.respond(overlays); await flush(); + // Their tombstones take two requests: one holds about 975 ids. + const asked: (readonly string[])[] = []; + for (let part = 0; part < 2; part++) { + const tombstones = h.next(); + asked.push(tombstones.filters[0]?.["#e"] ?? []); + tombstones.respond([]); + await flush(); + } + expect(asked.map((ids) => ids.length)).toEqual([900, 80]); + expect(asked.flat().sort()).toEqual(overlays.map((event) => event.id).sort()); h.next().respond([ root, message(alice, "a", "First reply", 2, [["e", root.id, "", "reply"]]), @@ -252,7 +265,7 @@ it("opens a target with a full reaction page, still loading older replies", asyn await reading; expect(h.view.snapshot()).toMatchObject({ targetStatus: "ready", - target: { id: reply.id }, + target: { id: reply.id, content: "Edit 479" }, limited: false, canLoadMore: true, }); diff --git a/src/features/relay/threads.ts b/src/features/relay/threads.ts index 6806a2c5e4..05c77db5a9 100644 --- a/src/features/relay/threads.ts +++ b/src/features/relay/threads.ts @@ -463,9 +463,10 @@ export function createThreadView({ event.tags.some(([name, value]) => name === "e" && value === id), ) .map((event) => event.id); - if (ids.length) { + // A request's 64 KiB filter budget holds about 975 ids. + for (let at = 0; at < ids.length; at += 900) { const tombstones = await targetReader.read( - [{ kinds: [5, 9005], "#e": ids, limit: 500 }], + [{ kinds: [5, 9005], "#e": ids.slice(at, at + 900), limit: 500 }], { signal: owned.signal }, ); if (!active()) return "cancelled"; @@ -566,7 +567,7 @@ export function createThreadView({ purge(); return; } - if (!replace && !snapshot.canLoadMore) return; + if (!replace && (!snapshot.canLoadMore || snapshot.limited)) return; const owned = new AbortController(); controller = owned; // Cleared as this refresh starts, not when the stream came back: a refresh From 1e5f895dba357f1b73af4edc07efd9ab40b9c1f8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 00:29:22 +1100 Subject: [PATCH 5/8] fix(relay): apply the third review's simplifications Follow-up to 042c06e06, covering the review of 2026-10-29 13:23 UTC. Review: useSearchMessages.ts rethrow after an abort - Applied. `ensure(...).catch(() => {})`, as in CommunityAdmin. The next line's `throwIfAborted()` already throws once the search is aborted, and the outer `.catch` ignores errors after an abort. Review: BuzzLinkPreview.test.tsx 500-reaction test - Dropped. thread-target.test.ts's "opens a target with a full reaction page and hundreds of edits" covers the same real-session reader guard and split reaction filter. The mocked-session test covers BuzzLinkPreview's `session.thread(channel, id, { exact: true })` wiring. The file is back to main's. Review: channel-reference.ts `reference.state` dependency - Kept. The reviewer's trace holds for revocation and cache clear: leaving a private channel bumps `accessRevision`, `applyDiscovery` revokes access, and `purgeAccess` calls `resetReferences`, which looks up every demanded link again. The requested test (a mounted, found link whose private channel the viewer leaves gets looked up again) passes with and without the dependency. - A found answer can still be lost without a reset. A public channel the viewer has left still reads `found` through its open metadata. If a newer 39000 makes it private, `accept` sees no access change, because access was already lost. Nothing resets, and the answer goes from `found` to `unknown`. Without the dependency the hook never refers again. No lookup is sent, and the link keeps showing its raw label instead of "Private channel" until it remounts or a later reset. Main's hook re-referred on that change, so dropping the dependency would regress it. - Tests: both are added to channel-references.test.ts. - "the viewer leaves its private channel" pins the reset path. - "a public channel the viewer left turns private" fails without the dependency: no lookup read is sent. It passes with it and with main's hook. Performance: no production behavior changes. The search catch behaves the same. Tests (summed test time, 042c06e06 -> this commit, local): - BuzzLinkPreview.test.tsx: 1.91 -> 0.06 s, 9 -> 8 tests - channel-references.test.ts: 2.84 -> 2.99 s, 14 -> 16 tests; the new tests take about 0.12 s each Also run: tsc, lint, and the channel-references, BuzzLinkPreview, useSearchMessages, channel-reference, MessageLink and thread-target suites (7 files, 91 passed). Not exercised in the running app. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/app/shell/useSearchMessages.ts | 10 ++--- .../conversation/BuzzLinkPreview.test.tsx | 39 +------------------ src/features/relay/channel-references.test.ts | 38 ++++++++++++++++++ 3 files changed, 43 insertions(+), 44 deletions(-) diff --git a/src/app/shell/useSearchMessages.ts b/src/app/shell/useSearchMessages.ts index ad0a5922e5..1db7403437 100644 --- a/src/app/shell/useSearchMessages.ts +++ b/src/app/shell/useSearchMessages.ts @@ -221,12 +221,10 @@ export function useSearchMessages( ? (session.channels.get?.(channelId)?.members ?? []) : []; if (knownMembers.length) { - try { - await session.profiles.ensure(knownMembers, "foreground"); - } catch (error) { - if (controller.signal.aborted) throw error; - // A partial directory still contains verified member profiles. - } + // A partial directory still contains verified member profiles. + await session.profiles + .ensure(knownMembers, "foreground") + .catch(() => {}); controller.signal.throwIfAborted(); } // A key matches by its profile name or, for a selectable agent, diff --git a/src/features/conversation/BuzzLinkPreview.test.tsx b/src/features/conversation/BuzzLinkPreview.test.tsx index a8a5a6dc7b..113b96cd1d 100644 --- a/src/features/conversation/BuzzLinkPreview.test.tsx +++ b/src/features/conversation/BuzzLinkPreview.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { act, cleanup, render, waitFor } from "@testing-library/react"; +import { act, cleanup, render } from "@testing-library/react"; import { afterEach, expect, it, vi } from "vitest"; import { BuzzLinkPreview, previewFailures } from "./BuzzLinkPreview"; import type { RelaySession } from "../relay/session"; @@ -10,8 +10,6 @@ import type { ChannelReference, } from "../relay/contracts"; import { mediaUrl } from "../relay/transport"; -import { createRelaySession } from "../relay/session"; -import { keypair, message, scriptedTransport, signed } from "../relay/testing"; afterEach(cleanup); const text = (tree: HTMLElement) => @@ -386,38 +384,3 @@ it("asks for the relay thumbnail of the author's picture, as message rows do", ( `https://relay.test/media/${sha}.thumb.jpg`, ); }); -it("previews a heavily reacted linked message through the real exact read", async () => { - const viewer = keypair(), - alice = keypair(); - const linked = message(alice, "a", "Popular announcement", 1); - const reactions = Array.from({ length: 500 }, (_, i) => - signed(alice, { - kind: 7, - content: "+", - created_at: 2 + i, - tags: [["e", linked.id]], - }), - ); - const owner = createRelaySession({ - ...scriptedTransport(viewer.pubkey, keypair().pubkey).transport, - query: async (filters) => - filters[0]?.ids?.includes(linked.id) - ? [linked] - : filters.some((filter) => filter.kinds?.includes(7)) - ? reactions - : [], - subscribe: () => ({ update() {}, retry() {}, dispose() {} }), - }); - const mounted = render( - , - ); - await waitFor(() => - expect(mounted.container.textContent).toContain("Popular announcement"), - ); - mounted.unmount(); - owner.dispose(); -}); diff --git a/src/features/relay/channel-references.test.ts b/src/features/relay/channel-references.test.ts index 542f660a91..5d2019235f 100644 --- a/src/features/relay/channel-references.test.ts +++ b/src/features/relay/channel-references.test.ts @@ -412,6 +412,44 @@ it("looks a mounted link up again when a reset drops its in-flight reply", async expect(link.result.current).toMatchObject({ state: "found", name: "launch" }); }); +it("looks a mounted link up again when the viewer leaves its private channel", async () => { + const h = await setup(); + h.emit([ + roster(relay, SECRET, [viewer.pubkey]), + metadata(relay, SECRET, "inner", 1_700_000_000, [["private"]]), + ]); + const link = renderHook(() => useChannelReference(h.channels, SECRET)); + await vi.waitFor(() => + expect(link.result.current).toMatchObject({ state: "found", joined: true }), + ); + await flush(); + expect(h.pending).toHaveLength(0); + await act(async () => { + h.emit([roster(relay, SECRET, [], 1_700_000_001)]); + }); + await h.lookup([SECRET], []); + expect(link.result.current).toEqual({ state: "withheld" }); +}); + +it("looks a mounted link up again when a public channel the viewer left turns private", async () => { + const h = await setup(); + const link = renderHook(() => useChannelReference(h.channels, MINE)); + await act(async () => { + h.emit([roster(relay, MINE, [], 1_700_000_001)]); + }); + expect(link.result.current).toMatchObject({ state: "found", joined: false }); + // Its messages stay unreadable until a lookup confirms it is still open. + await h.lookup([MINE], [open(MINE, "mine")]); + expect(link.result.current).toMatchObject({ state: "found", joined: false }); + expect(link.result.current).not.toHaveProperty("readable"); + // Access was already lost, so this revokes nothing and resets no answer. + await act(async () => { + h.emit([metadata(relay, MINE, "mine", 1_700_000_002, [["private"]])]); + }); + await h.lookup([MINE], []); + expect(link.result.current).toEqual({ state: "withheld" }); +}); + it("keeps a cancelled lookup from the old cache out of the new one's backoff", async () => { const h = await setup(); const release = h.channels.refer?.(SECRET); From 54254deccb3a0b4a8a0582153ebee38e1a550d1a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 2 Nov 2026 15:02:00 +1100 Subject: [PATCH 6/8] test: model the split overlay read in the policy relay stub Fixes the browser-journey CI failure on PR #2114 (run 41377657528). 2d4007a01 split the exact-target overlay read into a safety-kind filter and a separate `[7]` reaction filter. The policy relay stub answers only the multi-filter batches it recognises, so the new batch fell through to the sidebar-preferences branch and failed `toHaveLength(5)` in every journey that opens a thread (118 tests). The stub now recognises the batch: two filters on the same single `#e` target, each with `limit: 500`, the first carrying exactly kinds 5, 9005, 39005 and 40003 and the second only kind 7. The fixture's `answer()` serves each filter as before. The 1,100-member Members dialog test this commit also timed was dropped in the rebase onto main (#2028 pages the dialog's name reads). Tests (local): - Chromium and WebKit: right-pane-dismissal, message-navigation, inbox, navigation-thread-history, nested-replies, buzz-links, typing, message-actions, message-management, inbox-tabs, inbox-edit and presence specs, 207 of 208 passed in 8.4 min. The one failure is WebKit message-management.spec.mjs:361 (Delete dialog Close not focused), which also fails locally on main. - channel-opening.spec.mjs on chromium-measurements and webkit-measurements: passed. Not run: the full browser suite. Hosted CI will cover it. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- tests/browser/policy-relay.mjs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/browser/policy-relay.mjs b/tests/browser/policy-relay.mjs index 52e679c277..1a4adb40ec 100644 --- a/tests/browser/policy-relay.mjs +++ b/tests/browser/policy-relay.mjs @@ -245,6 +245,24 @@ export function policyRelay({ filters.flatMap((filter) => answer(communityOf(url), filter)), ); } + if (filters.length === 2 && filters[1].kinds?.join() === "7") { + // A selected target's safety overlays and its reactions, one target. + const target = filters[0]["#e"]; + expect(target).toEqual([expect.any(String)]); + expect(filters).toEqual([ + { kinds: expect.any(Array), "#e": target, limit: 500 }, + { kinds: [7], "#e": target, limit: 500 }, + ]); + expect(filters[0].kinds.toSorted((a, b) => a - b)).toEqual([ + 5, 9005, 39005, 40003, + ]); + const community = communityOf(url); + for (const filter of filters) + report.queries.push({ community, filter, at: performance.now() }); + return Response.json( + filters.flatMap((filter) => answer(community, filter)), + ); + } if (filters.length === 2 && filters[0].kinds?.includes(39000)) { // Exact channel authority lookup, distinct from sidebar preferences. const ids = filters[0]["#d"]; From 04f673ff8cdec2f34853d2979a48be49453d64d3 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 2 Nov 2026 17:04:03 +1100 Subject: [PATCH 7/8] fix(search): check from:name namesakes past the profile cap Fixes the second P2 finding in kalvinnchau's review of PR #2114 (2026-11-02 03:58 UTC, comment 4438794444). The first (discovery preview eviction keeping restrictive metadata) was dropped in the rebase onto main: #1574 replaced the 1,024-entry discovery cap with a 256 MiB byte budget, so there is no preview eviction to fix. Review: useSearchMessages.ts from:name namesakes (1608c3e48) - Real, and a regression of this branch. `scoped` matched only loaded names. When it found one member, `resolve` treated that member as unique, and the relay profile search ran only when `scoped` was empty. With more than 1,024 members (names load for the lowest 1,024 keys) or a failed batch (now caught), a namesake among the unloaded members was missed, and the search silently read the loaded member's messages. On main both cases threw, so the user saw an error, not a wrong answer. - Fix: the member names are partial when `ensure` rejects or a member is neither in the snapshot nor in `unnamed()`. `unnamed()` records members the relay answered without a kind 0, so profile-less bots and viewers do not count. This test also catches main's existing case, where the 2 MiB profile cache evicts early names during the same `ensure`. When partial, a loaded match no longer settles it: the existing relay name search runs too. Beside a loaded member match, its results count only if they are channel members, and they are merged with `scoped` before `resolve`. Two matches open the existing author picker, which already merges loaded members with the relay search. A full page of 40 still counts as ambiguous. A healthy roster takes the old path unchanged. - Residuals: in a large channel, a name with 40 or more relay matches always opens the picker. The relay does not fold accents, so an accented namesake past the cap is still missed, as on the existing relay branch. Performance: - from:name with 1,024 or fewer members whose names all load: no added request. One `some` over the roster with two Set lookups per member. - Otherwise: one `searchProfiles` (limit 40) per debounced query, the same cost the empty-`scoped` path already paid. Membership is checked with `includes` for at most 40 candidates. Tests: - useSearchMessages.test.tsx, 1,100 members with one Alice key starting with `1` (named) and another starting with `f` (past the cap): - Namesakes on both sides: the far one is never requested, one profile search, `ambiguousAuthor`, no message read. Fails on the previous code, which resolves to the loaded Alice. - A named member and a namesake outside the channel: one search, resolves to the member. Fails on the previous code (no search), and fails if the member restriction is dropped (ambiguous). - Only the member past the cap is named: resolves to her after one search. This guard passes before and after, as it should: the fix must keep this path. - Small fully read roster with a profile-less member: no profile search. A control that passes before and after. It fails if partial detection ignores `unnamed()`. - "resolves from:name on a roster larger than the profile budget when some names fail": its mock now answers the `search` filter, because the failed batches send it to the relay search. - Summed time for useSearchMessages.test.tsx: 2.04 -> 3.09 s, 6 -> 10 tests, about 0.27 s per new test. Also run: tsc, lint, and the useSearchMessages suite (10 passed). Not exercised in the running app. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- src/app/shell/useSearchMessages.test.tsx | 154 ++++++++++++++++++++++- src/app/shell/useSearchMessages.ts | 33 +++-- 2 files changed, 176 insertions(+), 11 deletions(-) diff --git a/src/app/shell/useSearchMessages.test.tsx b/src/app/shell/useSearchMessages.test.tsx index e6e1985360..cabb398492 100644 --- a/src/app/shell/useSearchMessages.test.tsx +++ b/src/app/shell/useSearchMessages.test.tsx @@ -1,8 +1,10 @@ // @vitest-environment jsdom import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; import { expect, it } from "vitest"; +import type { ReadFilter, RelayEvent } from "../../features/relay/events"; import { createRelaySession } from "../../features/relay/session"; import { + type Key, keypair, message, metadata, @@ -154,8 +156,10 @@ it("resolves from:name on a roster larger than the profile budget when some name if (filters.some((filter) => filter.kinds?.includes(9))) return wire.transport.query(filters, signal); const names = filters.find((filter) => filter.kinds?.includes(0)); + // The failed batches leave the names partial, so the relay's name + // search checks for a namesake among them. if (names) - return names.authors?.includes(alice.pubkey) + return names.search || names.authors?.includes(alice.pubkey) ? Promise.resolve([profile(alice, { name: "Alice" })]) : Promise.reject(new Error("Names unavailable")); return Promise.resolve( @@ -198,3 +202,151 @@ it("resolves from:name on a roster larger than the profile budget when some name owner.dispose(); } }); + +/** A key whose hex starts with `digit`, so it sorts to a known side of the + * 1,024-id profile budget. */ +function keyStarting(digit: string) { + for (;;) { + const key = keypair(); + if (key.pubkey.startsWith(digit)) return key; + } +} + +/** A session that has joined "crew" with `members`. Author reads answer from + * `profiles` and the relay's name search answers `found`. */ +async function crew( + members: readonly string[], + profiles: readonly RelayEvent[], + found: readonly RelayEvent[], +) { + const viewer = keypair(); + const relay = keypair(); + const wire = scriptedTransport(viewer.pubkey, relay.pubkey); + const requested = new Set(); + const searches: ReadFilter[] = []; + const discovery = [ + metadata(relay, "crew", "crew"), + roster(relay, "crew", [viewer.pubkey, ...members]), + ]; + const owner = createRelaySession( + { + ...wire.transport, + writer: { + sign: async (template) => signed(viewer, template), + publish: async () => {}, + }, + query(filters, signal) { + if (filters.some((filter) => filter.kinds?.includes(9))) + return wire.transport.query(filters, signal); + const names = filters.find((filter) => filter.kinds?.includes(0)); + if (names?.search) { + searches.push(names); + return Promise.resolve([...found]); + } + if (names) { + for (const author of names.authors ?? []) requested.add(author); + return Promise.resolve( + profiles.filter((event) => names.authors?.includes(event.pubkey)), + ); + } + return Promise.resolve( + discovery.filter((event) => + filters.some((filter) => filter.kinds?.includes(event.kind)), + ), + ); + }, + }, + { outboxStorage: { load: () => [], save() {} } }, + ); + owner.session.channels.ensureList(); + await waitFor(() => + expect(owner.session.channels.get?.("crew")?.members).toHaveLength( + members.length + 1, + ), + ); + return { owner, wire, requested, searches }; +} + +const near = keyStarting("1"); +const far = keyStarting("f"); +const outsider = keypair(); +const alice = (key: Key) => profile(key, { name: "Alice" }); +// 1,097 keys between the two, so the roster passes the profile budget with +// `near` inside it and `far` past it. +const between = Array.from( + { length: 1097 }, + (_, index) => `5${index.toString(16).padStart(63, "0")}`, +); + +it.each([ + { + roster: "namesakes on both sides of the budget", + named: [near, far], + found: [near, far], + author: undefined, + }, + { + roster: "only the member past the budget", + named: [far], + found: [far], + author: far, + }, + { + roster: "a named member and a namesake outside the channel", + named: [near], + found: [near, outsider], + author: near, + }, +])("resolves from:name on $roster", async ({ named, found, author }) => { + const { owner, wire, requested, searches } = await crew( + [near.pubkey, ...between, far.pubkey], + named.map(alice), + found.map(alice), + ); + try { + const { result } = renderHook(() => + useSearchMessages(owner.session, "from:alice ", "crew"), + ); + if (author) { + await waitFor(() => expect(wire.pending).toHaveLength(1)); + const read = wire.next(); + expect(read.filters[0]?.authors).toEqual([author.pubkey]); + await act(async () => read.respond([])); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.ambiguousAuthor).toBeUndefined(); + } else { + await waitFor(() => expect(result.current.ambiguousAuthor).toBe(true)); + expect(wire.pending).toHaveLength(0); + } + expect(requested.has(near.pubkey)).toBe(true); + expect(requested.has(far.pubkey)).toBe(false); + expect(searches).toHaveLength(1); + } finally { + cleanup(); + owner.dispose(); + } +}); + +it("resolves from:name on a fully read roster without a relay name search", async () => { + // The viewer and a member the relay answered without a profile are known + // unnamed, not missing. + const { owner, wire, searches } = await crew( + [near.pubkey, between[0] ?? ""], + [alice(near)], + [alice(near)], + ); + try { + const { result } = renderHook(() => + useSearchMessages(owner.session, "from:alice ", "crew"), + ); + await waitFor(() => expect(wire.pending).toHaveLength(1)); + const read = wire.next(); + expect(read.filters[0]?.authors).toEqual([near.pubkey]); + await act(async () => read.respond([])); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(searches).toHaveLength(0); + } finally { + cleanup(); + owner.dispose(); + } +}); diff --git a/src/app/shell/useSearchMessages.ts b/src/app/shell/useSearchMessages.ts index 1db7403437..fbfc4dd0c8 100644 --- a/src/app/shell/useSearchMessages.ts +++ b/src/app/shell/useSearchMessages.ts @@ -220,13 +220,24 @@ export function useSearchMessages( const knownMembers = channelId ? (session.channels.get?.(channelId)?.members ?? []) : []; - if (knownMembers.length) { - // A partial directory still contains verified member profiles. - await session.profiles - .ensure(knownMembers, "foreground") - .catch(() => {}); - controller.signal.throwIfAborted(); - } + // A partial directory still contains verified member profiles. + const failed = + knownMembers.length > 0 && + (await session.profiles.ensure(knownMembers, "foreground").then( + () => false, + () => true, + )); + controller.signal.throwIfAborted(); + // Members past the profile budget or in a failed batch may share + // the name, so a loaded match alone is not proof it is unique. + // A member the relay answered without a profile is known unnamed. + const loaded = session.profiles.snapshot(); + const unnamed = session.profiles.unnamed?.(); + const partial = + failed || + knownMembers.some( + (pubkey) => !loaded.has(pubkey) && !unnamed?.has(pubkey), + ); // A key matches by its profile name or, for a selectable agent, // by the agent's own name. const named = (pubkey: string, profileName = "") => @@ -245,7 +256,7 @@ export function useSearchMessages( if (unique.length > 1) return { ambiguous: true }; return { author: unique[0] }; }; - if (scoped.length) { + if (scoped.length && !partial) { const found = resolve([...scoped, ...agentMatches]); if (found.ambiguous) return { messages: [], ambiguousAuthor: true }; @@ -254,6 +265,7 @@ export function useSearchMessages( // Relay name search is limited. A match outside its first page // is unknown, so a full page is as ambiguous as duplicate names. // The relay does not fold accents, so it gets the typed text. + // Beside a member match it looks only for another member. const limit = 40; const candidates = await session.searchProfiles( typed.toLowerCase(), @@ -262,10 +274,11 @@ export function useSearchMessages( const matches = [...foldProfiles(candidates)] .filter( ([pubkey, profile]) => - named(pubkey, profile.name) || pubkey === handle, + (named(pubkey, profile.name) || pubkey === handle) && + (!scoped.length || knownMembers.includes(pubkey)), ) .map(([pubkey]) => pubkey); - const found = resolve([...matches, ...agentMatches]); + const found = resolve([...scoped, ...matches, ...agentMatches]); if (found.ambiguous || candidates.length >= limit) return { messages: [], ambiguousAuthor: true }; author = found.author; From 0295d72300df04ad6456cf21a971bcac6a4c5fd3 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 2 Nov 2026 19:15:14 +1100 Subject: [PATCH 8/8] fix(search): look for from:name namesakes among unnamed members only Fixes the P3 finding in kalvinnchau's review of PR #2114 (2026-11-02 06:26 UTC, comment 4439478824). Review: useSearchMessages.ts full-page check (c6bc52129) - Real, reproduced. When member names were partial, a loaded match sent the search to the relay-wide name search, which returns at most 40 profiles from the whole relay. Matches were then limited to channel members, but the full-page check still counted all 40. With 1,100 members, one loaded Alice and 39 nonmember Alices, the search returned `ambiguousAuthor` and read no messages. Any common name opened the picker in a channel past the profile budget, even with one matching member. - Fix: beside a loaded match, the name search covers only the members whose names did not load, as a kind 0 NIP-50 search with `authors` set to those keys. The block/buzz relay passes `authors` into its full-text query, so other users' profiles never fill the page. The keys go in parts of 900, which fit the reader's 64 KiB filter budget (about 975 keys), as the tombstone read in threads.ts does. A member who matches the name makes the author ambiguous. A part that returns a full page of 40 still counts as ambiguous, because another match could be on the next page. - Not chosen: reading every unnamed member's kind 0. That brings back the >1,024-id cliff and the 2 MiB cache churn for very large channels. The search returns at most 40 profiles per part. - With no loaded match, the relay-wide search is main's again: matches are not limited to members, and `scoped` is not merged in. That undoes c6bc52129's member filter there. `ensure`'s failure is caught as in CommunityAdmin, since the unnamed-member set already includes failed batches. - Residuals: the full-text search matches whole profile JSON, so 40 unnamed members whose `about` or `nip05` contains the name still open the picker. The relay does not fold accents, as on the relay-wide path. A typed name over about 5 KB would push a 900-key part past the filter budget, and the search would fail with an error. Request cost per debounced from:name query in a channel: - All member names loaded, or no loaded match: unchanged (no added read, or one relay-wide `searchProfiles` with limit 40, as on main). - Loaded match with U unnamed members: ceil(U / 900) kind 0 search reads with limit 40, sent in parallel, instead of one relay-wide search. For 1,100 members (76 unnamed) that is one read. A 10,000-member channel takes 10 reads. Each response holds at most 40 profiles. Tests (useSearchMessages.test.tsx, summed test time 3.21 -> 4.15 s, 10 -> 13 tests, local): - The crew mock now limits a search's results to its `authors`. - New row with the reviewer's probe: 1,100 members, one loaded Alice, 39 nonmember Alices (a full relay-wide page). It resolves to the member after one search over unnamed members only (0.33 s). On c6bc52129 it returns `ambiguousAuthor` without reading messages. - Each row now asserts the search's scope: unnamed members, never the loaded match, beside a loaded match, and relay-wide otherwise. The namesakes-on-both-sides and outside-namesake rows fail on c6bc52129 on this assertion. - "treats a full page of member name matches past the budget as ambiguous": 40 members past the budget named "Alice Smith" fill the page, so the author is ambiguous and no message is read (0.42 s). c6bc52129 was also ambiguous here, but through the relay-wide search, so it fails only on the search-scope assertion. - "searches the members whose names did not load in parts that fit the filter budget": 1,925 roster keys leave 901 unnamed, searched in parts of 900 and 1. The namesake in either part makes it ambiguous (0.27 s). Fails on c6bc52129, which sent one relay-wide search. Also run: tsc, lint, and the useSearchMessages and SearchResults suites (2 files, 108 passed). Not exercised in the running app or against a live relay. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/app/shell/useSearchMessages.test.tsx | 130 ++++++++++++++++++----- src/app/shell/useSearchMessages.ts | 73 ++++++++----- 2 files changed, 153 insertions(+), 50 deletions(-) diff --git a/src/app/shell/useSearchMessages.test.tsx b/src/app/shell/useSearchMessages.test.tsx index cabb398492..3253d20b6f 100644 --- a/src/app/shell/useSearchMessages.test.tsx +++ b/src/app/shell/useSearchMessages.test.tsx @@ -156,12 +156,14 @@ it("resolves from:name on a roster larger than the profile budget when some name if (filters.some((filter) => filter.kinds?.includes(9))) return wire.transport.query(filters, signal); const names = filters.find((filter) => filter.kinds?.includes(0)); - // The failed batches leave the names partial, so the relay's name - // search checks for a namesake among them. + // The failed batches leave the names partial, so a name search over + // their members checks for a namesake among them. if (names) - return names.search || names.authors?.includes(alice.pubkey) + return names.authors?.includes(alice.pubkey) ? Promise.resolve([profile(alice, { name: "Alice" })]) - : Promise.reject(new Error("Names unavailable")); + : names.search + ? Promise.resolve([]) + : Promise.reject(new Error("Names unavailable")); return Promise.resolve( discovery.filter((event) => filters.some((filter) => filter.kinds?.includes(event.kind)), @@ -213,7 +215,8 @@ function keyStarting(digit: string) { } /** A session that has joined "crew" with `members`. Author reads answer from - * `profiles` and the relay's name search answers `found`. */ + * `profiles` and the relay's name search answers `found`, within its + * `authors` when it has them. */ async function crew( members: readonly string[], profiles: readonly RelayEvent[], @@ -241,7 +244,11 @@ async function crew( const names = filters.find((filter) => filter.kinds?.includes(0)); if (names?.search) { searches.push(names); - return Promise.resolve([...found]); + return Promise.resolve( + found.filter( + (event) => !names.authors || names.authors.includes(event.pubkey), + ), + ); } if (names) { for (const author of names.authors ?? []) requested.add(author); @@ -278,49 +285,124 @@ const between = Array.from( (_, index) => `5${index.toString(16).padStart(63, "0")}`, ); +const outsiders = Array.from({ length: 39 }, () => keypair()); + it.each([ { roster: "namesakes on both sides of the budget", named: [near, far], found: [near, far], author: undefined, + unreadOnly: true, }, { roster: "only the member past the budget", named: [far], found: [far], author: far, + unreadOnly: false, }, { roster: "a named member and a namesake outside the channel", named: [near], found: [near, outsider], author: near, + unreadOnly: true, + }, + { + // A relay-wide search would return a full page of 40 Alices. + roster: "a named member and 39 namesakes outside the channel", + named: [near], + found: [near, ...outsiders], + author: near, + unreadOnly: true, }, -])("resolves from:name on $roster", async ({ named, found, author }) => { - const { owner, wire, requested, searches } = await crew( - [near.pubkey, ...between, far.pubkey], - named.map(alice), - found.map(alice), +])( + "resolves from:name on $roster", + async ({ named, found, author, unreadOnly }) => { + const { owner, wire, requested, searches } = await crew( + [near.pubkey, ...between, far.pubkey], + named.map(alice), + found.map(alice), + ); + try { + const { result } = renderHook(() => + useSearchMessages(owner.session, "from:alice ", "crew"), + ); + if (author) { + await waitFor(() => expect(wire.pending).toHaveLength(1)); + const read = wire.next(); + expect(read.filters[0]?.authors).toEqual([author.pubkey]); + await act(async () => read.respond([])); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.ambiguousAuthor).toBeUndefined(); + } else { + await waitFor(() => expect(result.current.ambiguousAuthor).toBe(true)); + expect(wire.pending).toHaveLength(0); + } + expect(requested.has(near.pubkey)).toBe(true); + expect(requested.has(far.pubkey)).toBe(false); + expect(searches).toHaveLength(1); + // Beside a loaded match only the members whose names did not load are + // searched; otherwise the relay-wide name search runs. + const authors = searches[0]?.authors; + expect(authors?.includes(far.pubkey) ?? false).toBe(unreadOnly); + expect(authors?.includes(near.pubkey) ?? false).toBe(false); + } finally { + cleanup(); + owner.dispose(); + } + }, +); + +it("treats a full page of member name matches past the budget as ambiguous", async () => { + // Forty members past the budget whose profiles match the typed prefix + // without naming Alice fill the page, so another Alice may be on the next. + const crowd = Array.from({ length: 40 }, () => keyStarting("f")); + const { owner, wire, searches } = await crew( + [near.pubkey, ...between.slice(0, 1057), ...crowd.map((key) => key.pubkey)], + [alice(near)], + crowd.map((key) => profile(key, { name: "Alice Smith" })), ); try { const { result } = renderHook(() => useSearchMessages(owner.session, "from:alice ", "crew"), ); - if (author) { - await waitFor(() => expect(wire.pending).toHaveLength(1)); - const read = wire.next(); - expect(read.filters[0]?.authors).toEqual([author.pubkey]); - await act(async () => read.respond([])); - await waitFor(() => expect(result.current.loading).toBe(false)); - expect(result.current.ambiguousAuthor).toBeUndefined(); - } else { - await waitFor(() => expect(result.current.ambiguousAuthor).toBe(true)); - expect(wire.pending).toHaveLength(0); - } - expect(requested.has(near.pubkey)).toBe(true); - expect(requested.has(far.pubkey)).toBe(false); + await waitFor(() => expect(result.current.ambiguousAuthor).toBe(true)); + expect(wire.pending).toHaveLength(0); expect(searches).toHaveLength(1); + expect(searches[0]?.authors).toEqual( + expect.arrayContaining(crowd.map((key) => key.pubkey)), + ); + } finally { + cleanup(); + owner.dispose(); + } +}); + +it("searches the members whose names did not load in parts that fit the filter budget", async () => { + // 1,925 roster keys, so 901 are past the 1,024-id profile budget. + const many = Array.from( + { length: 1922 }, + (_, index) => `5${index.toString(16).padStart(63, "0")}`, + ); + const { owner, wire, searches } = await crew( + [near.pubkey, ...many, far.pubkey], + [alice(near)], + [alice(near), alice(far)], + ); + try { + const { result } = renderHook(() => + useSearchMessages(owner.session, "from:alice ", "crew"), + ); + await waitFor(() => expect(result.current.ambiguousAuthor).toBe(true)); + expect(wire.pending).toHaveLength(0); + expect(searches.map(({ authors }) => authors?.length).sort()).toEqual([ + 1, 900, + ]); + expect(searches.flatMap(({ authors }) => authors ?? [])).toContain( + far.pubkey, + ); } finally { cleanup(); owner.dispose(); diff --git a/src/app/shell/useSearchMessages.ts b/src/app/shell/useSearchMessages.ts index fbfc4dd0c8..2fb3c8f8d7 100644 --- a/src/app/shell/useSearchMessages.ts +++ b/src/app/shell/useSearchMessages.ts @@ -221,23 +221,20 @@ export function useSearchMessages( ? (session.channels.get?.(channelId)?.members ?? []) : []; // A partial directory still contains verified member profiles. - const failed = - knownMembers.length > 0 && - (await session.profiles.ensure(knownMembers, "foreground").then( - () => false, - () => true, - )); - controller.signal.throwIfAborted(); - // Members past the profile budget or in a failed batch may share - // the name, so a loaded match alone is not proof it is unique. - // A member the relay answered without a profile is known unnamed. + if (knownMembers.length) { + await session.profiles + .ensure(knownMembers, "foreground") + .catch(() => {}); + controller.signal.throwIfAborted(); + } + // Members past the profile budget or in a failed batch have no + // loaded name. A member the relay answered without a profile is + // known unnamed. const loaded = session.profiles.snapshot(); const unnamed = session.profiles.unnamed?.(); - const partial = - failed || - knownMembers.some( - (pubkey) => !loaded.has(pubkey) && !unnamed?.has(pubkey), - ); + const unread = knownMembers.filter( + (pubkey) => !loaded.has(pubkey) && !unnamed?.has(pubkey), + ); // A key matches by its profile name or, for a selectable agent, // by the agent's own name. const named = (pubkey: string, profileName = "") => @@ -256,17 +253,42 @@ export function useSearchMessages( if (unique.length > 1) return { ambiguous: true }; return { author: unique[0] }; }; - if (scoped.length && !partial) { - const found = resolve([...scoped, ...agentMatches]); - if (found.ambiguous) + // Relay name search is limited. A match outside its first page is + // unknown, so a full page is as ambiguous as duplicate names. + // The relay does not fold accents, so it gets the typed text. + const limit = 40; + if (scoped.length) { + // A member whose name did not load may share it, so search only + // those members' profiles. A request's 64 KiB filter budget + // holds about 975 keys. + const pages = await Promise.all( + Array.from( + { length: Math.ceil(unread.length / 900) }, + (_, at) => + session.read( + [ + { + kinds: [0], + authors: unread.slice(at * 900, at * 900 + 900), + search: typed.toLowerCase(), + search_mode: "prefix", + limit, + }, + ], + { signal: controller.signal, fresh: true }, + ), + ), + ); + const namesakes = pages.flatMap((page) => + [...foldProfiles(page)] + .filter(([pubkey, profile]) => named(pubkey, profile.name)) + .map(([pubkey]) => pubkey), + ); + const found = resolve([...scoped, ...namesakes, ...agentMatches]); + if (found.ambiguous || pages.some((page) => page.length >= limit)) return { messages: [], ambiguousAuthor: true }; author = found.author; } else { - // Relay name search is limited. A match outside its first page - // is unknown, so a full page is as ambiguous as duplicate names. - // The relay does not fold accents, so it gets the typed text. - // Beside a member match it looks only for another member. - const limit = 40; const candidates = await session.searchProfiles( typed.toLowerCase(), { limit, signal: controller.signal }, @@ -274,11 +296,10 @@ export function useSearchMessages( const matches = [...foldProfiles(candidates)] .filter( ([pubkey, profile]) => - (named(pubkey, profile.name) || pubkey === handle) && - (!scoped.length || knownMembers.includes(pubkey)), + named(pubkey, profile.name) || pubkey === handle, ) .map(([pubkey]) => pubkey); - const found = resolve([...scoped, ...matches, ...agentMatches]); + const found = resolve([...matches, ...agentMatches]); if (found.ambiguous || candidates.length >= limit) return { messages: [], ambiguousAuthor: true }; author = found.author;