From c0b5252aea069a788213d39e7cf77aef0cfa6be5 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:37:32 -0700 Subject: [PATCH 1/2] Keep the reply when following a link within the same thread Following a link to another message in an already-open channel thread now selects that message in the existing thread view instead of rebuilding it. The unfinished reply, its reply target and expanded branches stay. Same-thread membership comes from the linked message's signed root, never the link hint; nothing changes on screen until the new message is verified. Missing or failed targets keep the current thread with a single status notice. Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- docs/relay-queries.md | 18 ++ src/bundled/channels/ChannelsPage.tsx | 4 + .../conversation/BuzzLinkPreview.test.tsx | 1 + src/features/messages/ThreadPanel.test.tsx | 171 +++++++++++++ src/features/messages/ThreadPanel.tsx | 190 ++++++++++++++- src/features/relay/session.ts | 81 +++---- src/features/relay/thread-target.test.ts | 216 +++++++++++++++++ src/features/relay/threads.ts | 224 +++++++++++++----- tests/browser/message-navigation.spec.mjs | 62 +++++ 9 files changed, 855 insertions(+), 112 deletions(-) diff --git a/docs/relay-queries.md b/docs/relay-queries.md index d32f123f5..89e7f07b1 100644 --- a/docs/relay-queries.md +++ b/docs/relay-queries.md @@ -270,6 +270,24 @@ inserting isolated rows into channel history. Explicit denial revokes the owning channel; access loss, cache clear and disposal purge the view. There is no separate reader owner, subscription or persistence. +An open thread can call `view.retarget(messageId, signal)` to select another +message without reallocating its reader. The same bounded exact-read path verifies +the candidate's signed ancestry against the already-known root; navigation hints +never establish identity. A same-root selection preserves traversal rows/cursors +and publishes only after its overlays are complete. A different root, unavailable +message or read failure leaves the prior selection intact. Selection requests +supersede each other independently of history paging and repair; cancellation, +access loss, cache clear and disposal fence late results. All evidence remains +inside the existing view budget and session authority. + +Channel thread panels opt into this retained lifetime. Same-thread links preserve +the mounted editor, reply target and expansion state; a different verified thread +still replaces the current view. Failed link selection keeps the existing reply and +shows one notice in the thread's status area. A message that no longer exists has +no retry; other failures offer Try again, which stays focused while a fresh +navigation request runs. Other embedded/preview consumers +retain their previous lifetime. This does not yet add main-pane tabs or dragging. + ## Global message search and public previews Global message searches (`kinds: [9, 40002]`, no `#h`) keep the relay's order. diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index bb3dba36d..3ede9c1f5 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1619,6 +1619,10 @@ function ChannelWorkspace({ }, content: ( + void navigator?.retry() + } sessionConversation={ current?.channelType === "session" } diff --git a/src/features/conversation/BuzzLinkPreview.test.tsx b/src/features/conversation/BuzzLinkPreview.test.tsx index 96ab790d0..6733b1de6 100644 --- a/src/features/conversation/BuzzLinkPreview.test.tsx +++ b/src/features/conversation/BuzzLinkPreview.test.tsx @@ -27,6 +27,7 @@ function setup(snapshot: ThreadSnapshot, messageId = root.id) { subscribe: () => () => {}, refresh: vi.fn(async () => {}), loadMore: vi.fn(async () => {}), + retarget: vi.fn(async () => "selected"), dispose: vi.fn(), } satisfies ThreadView; const thread = vi.fn(() => view); diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index a0f89ffa1..c461b73cd 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -155,6 +155,7 @@ function messagesHarness( listeners: Set<() => void>; refresh: ReturnType Promise>>; loadMore: ReturnType Promise>>; + retarget: ReturnType>; dispose: ReturnType; })[] = []; const makeView = () => { @@ -169,6 +170,7 @@ function messagesHarness( }, refresh: vi.fn(async () => {}), loadMore: vi.fn(async () => {}), + retarget: vi.fn(async () => "selected"), dispose: vi.fn(), listeners, }; @@ -1141,3 +1143,172 @@ it("passes the Me recipient, readiness and direct Activity contract through to t expect(composer).not.toHaveAttribute("data-personal"); expect(composer).not.toHaveAttribute("data-direct-activity"); }); + +it("retained navigation keeps the mounted editor when another message belongs to the open thread", async () => { + const controller = new AbortController(); + const first = { ...ordinaryNavigation(), signal: controller.signal }; + const h = messagesHarness(first); + const mounted = render(); + const editor = within(mounted.container).getByRole("region", { + name: "Composer", + }); + const current = h.view; + let release!: (value: "selected") => void; + current.retarget.mockImplementation( + () => + new Promise((resolve) => { + release = resolve; + }), + ); + const next = { + ...ordinaryNavigation(), + entryId: "next", + signal: new AbortController().signal, + }; + next.target.messageId = "c".repeat(64); + act(() => controller.abort()); + mounted.rerender( + , + ); + expect( + within(mounted.container).getByRole("region", { name: "Composer" }), + ).toBe(editor); + await act(async () => release("selected")); + expect( + within(mounted.container).getByRole("region", { name: "Composer" }), + ).toBe(editor); + expect(current.dispose).not.toHaveBeenCalled(); + mounted.unmount(); + expect(current.dispose).toHaveBeenCalledOnce(); +}); + +it.each(["unavailable", "error", "cancelled"] as const)( + "retained target %s keeps the same thread and reply editor", + async (result) => { + const h = messagesHarness(ordinaryNavigation()); + const retry = vi.fn(); + const mounted = render( + , + ); + const editor = screen.getByRole("region", { name: "Composer" }); + const reader = h.view; + reader.retarget.mockResolvedValue(result); + const next = { ...ordinaryNavigation(), entryId: "next" }; + next.target.messageId = "c".repeat(64); + await act(async () => + mounted.rerender( + , + ), + ); + expect(screen.getByRole("region", { name: "Composer" })).toBe(editor); + expect(reader.dispose).not.toHaveBeenCalled(); + expect(h.thread).toHaveBeenCalledOnce(); + expect(screen.getAllByRole("alert")).toHaveLength(1); + expect(screen.getByRole("alert")).toHaveTextContent( + result === "unavailable" + ? "That message is no longer available." + : "Couldn’t open that message.", + ); + expect(next.complete).toHaveBeenCalledWith({ + status: "failed", + reason: result === "unavailable" ? "not-found" : "unavailable", + }); + if (result === "unavailable") { + // Retrying can't restore a message that no longer exists. + expect(screen.queryByRole("button", { name: "Try again" })).toBeNull(); + return; + } + const again = screen.getByRole("button", { name: "Try again" }); + fireEvent.click(again); + expect(retry).toHaveBeenCalledOnce(); + // The focused control stays mounted while the fresh attempt runs. + let release!: (value: "selected") => void; + reader.retarget.mockImplementation( + () => + new Promise((resolve) => { + release = resolve; + }), + ); + const retried = { ...ordinaryNavigation(), entryId: "retried" }; + retried.target.messageId = next.target.messageId; + await act(async () => + mounted.rerender( + , + ), + ); + try { + expect(screen.getByRole("button", { name: "Try again" })).toBe(again); + expect(again).toHaveAttribute("aria-busy", "true"); + } finally { + await act(async () => release("selected")); + } + expect(screen.queryByRole("alert")).toBeNull(); + expect(screen.getByRole("region", { name: "Composer" })).toBe(editor); + expect(reader.dispose).not.toHaveBeenCalled(); + }, +); + +it("a link into a different thread replaces the reader and its editor", async () => { + const h = messagesHarness(ordinaryNavigation()); + const mounted = render(); + const editor = screen.getByRole("region", { name: "Composer" }); + const previous = h.view; + previous.retarget.mockResolvedValue("different"); + const next = { ...ordinaryNavigation(), entryId: "other-thread" }; + next.target.messageId = "d".repeat(64); + await act(async () => + mounted.rerender( + , + ), + ); + expect(previous.dispose).toHaveBeenCalledOnce(); + expect(h.thread).toHaveBeenCalledTimes(2); + expect(h.view).not.toBe(previous); + expect(screen.getByRole("region", { name: "Composer" })).not.toBe(editor); + expect(screen.queryByRole("alert")).toBeNull(); +}); + +it("an unrouted retained opening retries allocation locally", () => { + const h = messagesHarness(); + const navigationRetry = vi.fn(); + h.thread + .mockReset() + .mockImplementationOnce(() => { + throw new Error("capacity"); + }) + .mockReturnValue(h.view); + render( + , + ); + expect(screen.getByRole("alert")).toHaveTextContent("capacity"); + fireEvent.click(screen.getByRole("button", { name: "Try again" })); + expect(screen.getByRole("region", { name: "Composer" })).toBeVisible(); + expect(navigationRetry).not.toHaveBeenCalled(); + expect(h.thread).toHaveBeenCalledTimes(2); +}); diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index da23c3e4c..2594d62a1 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -52,6 +52,10 @@ export type ThreadPanelProps = { activityClickOpensPanel?: boolean | undefined; disabled?: boolean | undefined; messageId: string; + /** Channel workspaces retain the editor across same-thread link navigation. */ + retainThread?: boolean | undefined; + /** Retry a failed routed opening with fresh navigation authority. */ + onRetryNavigation?: (() => void) | undefined; replyRequest?: number | undefined; /** Inline Inbox visit retains and reveals its exact selected message. */ revealSelected?: boolean | undefined; @@ -82,7 +86,7 @@ export function ThreadPanel(props: ThreadPanelProps) { props.session, props.scope, props.channelId, - props.messageId, + props.retainThread ? undefined : props.messageId, ); return ( ); } @@ -147,6 +155,153 @@ function ThreadHeader({ /> ); } +type SelectionFailure = { + unavailable: boolean; + pending: boolean; + retry(): void; +}; +/** Reader lifetime follows the open thread; navigation owns only a reveal request. */ +function RetainedThreadPanel(props: ThreadPanelProps) { + type Opening = { + view: ThreadView; + messageId: string; + navigation: PageNavigation | undefined; + revision: number; + }; + const current = useRef(undefined); + const [opening, setOpening] = useState(); + const [error, setError] = useState(); + const [failure, setFailure] = useState<{ unavailable: boolean }>(); + // Retry keeps its control mounted and focused until the new attempt settles. + const [pending, setPending] = useState(false); + const retrying = useRef(false); + const [attempt, retry] = useState(0); + useEffect( + () => () => { + current.current?.view.dispose(); + current.current = undefined; + }, + [], + ); + const { session, channelId, messageId, navigation, onRetryNavigation } = + props; + const retryNavigation = useCallback(() => { + retrying.current = true; + setPending(true); + if (navigation && onRetryNavigation) onRetryNavigation(); + else retry((value) => value + 1); + }, [navigation, onRetryNavigation]); + // A new request never clears a previously opened editor while evidence loads. + // biome-ignore lint/correctness/useExhaustiveDependencies: attempt retries the selected request; outer identity owns session/channel lifetime. + useEffect(() => { + if (navigation?.signal.aborted) return; + const request = new AbortController(); + const signal = navigation + ? AbortSignal.any([request.signal, navigation.signal]) + : request.signal; + if (!retrying.current) { + setError(undefined); + setFailure(undefined); + } + const settle = (next?: { unavailable: boolean }, message?: string) => { + retrying.current = false; + setPending(false); + setFailure(next); + setError(message); + }; + async function select() { + const prior = current.current; + if ( + prior && + !navigation && + prior.messageId === messageId && + !retrying.current + ) { + const next = { ...prior, navigation: undefined }; + current.current = next; + setOpening(next); + return; + } + if (prior?.view.snapshot().root) { + const result = await prior.view.retarget(messageId, signal); + if (signal.aborted || current.current !== prior) return; + if (result === "selected") { + const next = { ...prior, messageId, navigation }; + current.current = next; + setOpening(next); + settle(); + return; + } + if (result !== "different") { + settle({ unavailable: result === "unavailable" }); + navigation?.complete({ + status: "failed", + reason: result === "unavailable" ? "not-found" : "unavailable", + }); + return; + } + } + try { + const exact = + navigation?.target.kind === "conversation" && + navigation.target.threadRootId !== messageId; + const view = session.thread( + channelId, + messageId, + exact ? { exact: true } : undefined, + ); + prior?.view.dispose(); + const next = { + view, + messageId, + navigation, + revision: (prior?.revision ?? 0) + 1, + }; + current.current = next; + setOpening(next); + settle(); + void view.refresh(); + } catch (error) { + // An existing thread stays usable; its notice owns the retry. + if (prior) settle({ unavailable: false }); + else settle(undefined, String(error)); + navigation?.complete({ status: "failed", reason: "unavailable" }); + } + } + void select(); + return () => request.abort(); + }, [session, channelId, messageId, navigation, attempt]); + return ( + <> + {error && !opening && ( +
+

{error}

+ +
+ )} + {opening ? ( + + ) : ( + !error && ( +

+ Loading thread… +

+ ) + )} + + ); +} function OwnedThreadPanel({ session, extensions, @@ -266,7 +421,10 @@ function ThreadMessages({ revealSelected, requireReadyRoot, onDraftSaved, + selectionFailure, }: { + /** A same-thread link that couldn't open; the current thread stays usable. */ + selectionFailure?: SelectionFailure | undefined; sessionConversation?: boolean | undefined; personalConversation?: boolean | undefined; activityClickOpensPanel?: boolean | undefined; @@ -386,6 +544,10 @@ function ThreadMessages({ >(undefined); const olderDemand = useRef(false); const targetAnchor = useRef(undefined); + // biome-ignore lint/correctness/useExhaustiveDependencies: each new target request retires the previous reveal anchor, not the reader's state. + useLayoutEffect(() => { + targetAnchor.current = undefined; + }, [messageId, navigation?.signal]); const selectedRow = useCallback( () => [ @@ -911,6 +1073,8 @@ function ThreadMessages({ captureOlderAnchor(element); void view.loadMore(); }; + const selectionNotice = + selectionFailure && !snapshot.error ? selectionFailure : undefined; const showOlderPageStatus = snapshot.direction === "older" && snapshot.readKind === "older" && @@ -1057,23 +1221,41 @@ function ThreadMessages({ {positioning || (snapshot.status === "loading" && !rows.length) ? (

Loading thread…

) : null} - {snapshot.targetStatus === "unavailable" && ( + {snapshot.targetStatus === "unavailable" && !selectionNotice && (

Selected message unavailable.

)} {snapshot.error && !showOlderPageStatus && (

{snapshot.error}

)} + {selectionNotice && ( +

+ {selectionNotice.unavailable + ? "That message is no longer available." + : "Couldn’t open that message."} +

+ )} {snapshot.limited && !snapshot.error && (

Thread history limit reached.

)} {((snapshot.error && !showOlderPageStatus) || - snapshot.targetStatus === "unavailable") && ( + (snapshot.targetStatus === "unavailable" && !selectionNotice)) && (
)} + {selectionNotice && !selectionNotice.unavailable && ( +
+ +
+ )} {snapshot.root && (!requireReadyRoot || diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 8cb8395e3..301fb1e7f 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -1786,54 +1786,49 @@ export function createRelaySession( throw new Error("Relay view capacity unavailable"); if (!/^[0-9a-f]{64}$/.test(messageId)) throw new Error("Thread needs a valid message ID"); + // Exact selection is available to an existing ordinary thread too; its + // raw evidence keeps the same access, cap and atomic-admission checks. + const targetReader: RelayReader = { + async read(filters, settings) { + if (!canReadRemote(channelId)) + throw new ReadError( + "unavailable", + "Reconnect to refresh conversation access.", + ); + const epoch = accessEpoch; + let events: readonly RelayEvent[]; + try { + events = await requests.reader.read(filters, settings); + } catch (error) { + if ( + !closed && + epoch === accessEpoch && + readErrorKind(error) === "denied" + ) + channels.denyChannel(channelId, error); + throw error; + } + if (closed || epoch !== accessEpoch || !canReadRemote(channelId)) + throw new DOMException("Stale thread target", "AbortError"); + settings?.signal?.throwIfAborted(); + // A capped raw target/overlay read cannot establish a safe fold. + if ( + !filters.some((filter) => filter.depth_limit) && + events.length >= 500 + ) + throw new Error("Selected message exceeded its evidence limit"); + // The thread owner admits the complete target fold atomically. + return events.filter((event) => event.kind !== PRODUCT_FEEDBACK_KIND); + }, + }; const thread = createThreadView({ channelId, messageId, relayAuthor: transport?.relayAuthor ?? "", signingAuthority: transport?.archiveAuthority, - reader: options?.exact - ? { - async read(filters, settings) { - if (!canReadRemote(channelId)) - throw new ReadError( - "unavailable", - "Reconnect to refresh conversation access.", - ); - const epoch = accessEpoch; - let events: readonly RelayEvent[]; - try { - events = await requests.reader.read(filters, settings); - } catch (error) { - if ( - !closed && - epoch === accessEpoch && - readErrorKind(error) === "denied" - ) - channels.denyChannel(channelId, error); - throw error; - } - if ( - closed || - epoch !== accessEpoch || - !canReadRemote(channelId) - ) - throw new DOMException("Stale thread target", "AbortError"); - settings?.signal?.throwIfAborted(); - // A capped raw target/overlay read cannot establish a safe fold. - if ( - !filters.some((filter) => filter.depth_limit) && - events.length >= 500 - ) - throw new Error( - "Selected message exceeded its evidence limit", - ); - // The thread owner admits the complete target fold atomically. - return events.filter( - (event) => event.kind !== PRODUCT_FEEDBACK_KIND, - ); - }, - } - : verified, + reader: options?.exact ? targetReader : verified, + targetReader, + admitTarget: (events) => accept(events, false), exact: options?.exact ?? false, admit: options?.exact ? (events) => accept(events, false) : undefined, seed: recent.peek(messageId)?.event ?? retainedEvent(messageId), diff --git a/src/features/relay/thread-target.test.ts b/src/features/relay/thread-target.test.ts index ca00e0355..a96e972f5 100644 --- a/src/features/relay/thread-target.test.ts +++ b/src/features/relay/thread-target.test.ts @@ -408,3 +408,219 @@ it("never retains a selected reply rejected by the shared dual-channel access ga expect(h.view.snapshot().target).toBeUndefined(); expect(h.view.snapshot().replies).toEqual([]); }); + +it("retargets a root-opened reader without reloading its history or replacing its rows", async () => { + const h = setup(root.id); + h.view.dispose(); + h.traffic.receive([root]); + const view = h.session.thread("a", root.id); + const initial = view.refresh(); + h.next().respond([root]); + await initial; + const before = view.snapshot(); + const selecting = view.retarget(reply.id, new AbortController().signal); + expect(view.snapshot()).toBe(before); + const request = h.next(); + expect(request.filters).toEqual([{ ids: [reply.id], "#h": ["a"], limit: 1 }]); + request.respond([reply]); + await flush(); + h.next().respond([aux(40003, reply, "Retargeted reply")]); + await flush(); + h.next().respond([]); + expect(await selecting).toBe("selected"); + expect(view.snapshot()).toMatchObject({ + status: "ready", + root: { id: root.id }, + targetStatus: "ready", + target: { id: reply.id, content: "Retargeted reply" }, + }); + expect(view.snapshot().root).toBe(before.root); + expect(h.pending).toHaveLength(0); +}); + +it.each(["different", "unavailable", "error", "cancelled"] as const)( + "keeps the existing thread unchanged when retarget is %s", + async (outcome) => { + const h = setup(); + const { loading } = await targetRead(h); + h.next().respond([root]); + await loading; + const before = h.view.snapshot(); + const signal = new AbortController(); + const unrelated = message(alice, "a", "Another root", 90); + const selecting = h.view.retarget( + outcome === "different" ? unrelated.id : "c".repeat(64), + signal.signal, + ); + const pending = h.next(); + if (outcome === "different") { + pending.respond([unrelated]); + } else if (outcome === "error") pending.fail(new Error("offline")); + else if (outcome === "cancelled") { + signal.abort(); + pending.respond([reply]); + } else pending.respond([]); + expect(await selecting).toBe(outcome); + expect(h.view.snapshot()).toBe(before); + expect(h.pending).toHaveLength(0); + }, +); + +it("the newest retarget wins and access loss fences an in-flight target", async () => { + const h = setup(); + h.traffic.receive([roster(relay, "a", [viewer.pubkey], 10)]); + const { loading } = await targetRead(h); + h.next().respond([root]); + await loading; + const one = message(alice, "a", "One", 3, [["e", root.id, "", "reply"]]); + const two = message(alice, "a", "Two", 4, [["e", root.id, "", "reply"]]); + const first = h.view.retarget(one.id, new AbortController().signal); + const stale = h.next(); + const second = h.view.retarget(two.id, new AbortController().signal); + expect(stale.signal?.aborted).toBe(true); + stale.respond([one]); + await flush(); + h.next().respond([two]); + await flush(); + h.next().respond([]); + expect(await first).toBe("cancelled"); + expect(await second).toBe("selected"); + expect(h.view.snapshot().target?.id).toBe(two.id); + const revoked = h.view.retarget(one.id, new AbortController().signal); + const held = h.next(); + h.traffic.receive([roster(relay, "a", [], 11)]); + expect(held.signal?.aborted).toBe(true); + held.respond([one]); + expect(await revoked).toBe("cancelled"); + expect(h.view.snapshot().root).toBeUndefined(); + expect(h.view.snapshot().target).toBeUndefined(); +}); + +it("retarget during a pending history page does not strand loading or lose the page", async () => { + const h = setup(); + const { loading } = await targetRead(h); + const sibling = message(alice, "a", "Sibling", 2, [ + ["e", root.id, "", "reply"], + ]); + h.next().respond([root, sibling]); + await loading; + const more = h.view.loadMore(); + const page = h.next(); + const selection = h.view.retarget(root.id, new AbortController().signal); + expect(page.signal?.aborted).toBe(false); + h.next().respond([root]); + await flush(); + h.next().respond([]); + expect(await selection).toBe("selected"); + page.respond([ + root, + message(alice, "a", "Later", 3, [["e", root.id, "", "reply"]]), + ]); + await more; + expect(h.view.snapshot().replies.some((row) => row.content === "Later")).toBe( + true, + ); + expect(h.view.snapshot().target?.id).toBe(root.id); + expect(h.view.snapshot().status).toBe("ready"); +}); + +it("a deleted new target does not replace the prior selection used by refresh", async () => { + const h = setup(); + const { loading } = await targetRead(h); + h.next().respond([root]); + await loading; + const deleted = message(alice, "a", "Deleted", 3, [ + ["e", root.id, "", "reply"], + ]); + const selecting = h.view.retarget(deleted.id, new AbortController().signal); + h.next().respond([deleted]); + await flush(); + h.next().respond([aux(5, deleted)]); + await flush(); + h.next().respond([]); + expect(await selecting).toBe("unavailable"); + expect(h.view.snapshot().target?.id).toBe(reply.id); + const refresh = h.view.refresh(); + const target = h.next(); + expect(target.filters[0]?.ids).toEqual([reply.id]); + target.respond([reply]); + await flush(); + h.next().respond([]); + await flush(); + h.next().respond([root]); + await refresh; + expect(h.view.snapshot().target?.id).toBe(reply.id); +}); + +it("a held initial traversal finishes after retarget without overwriting the selection", async () => { + const h = setup(); + const { loading } = await targetRead(h); + const page = h.next(); + h.traffic.receive([root]); + const selected = h.view.retarget(root.id, new AbortController().signal); + h.next().respond([root]); + await flush(); + h.next().respond([]); + expect(await selected).toBe("selected"); + expect(h.view.snapshot().status).toBe("loading"); + page.respond([root]); + await loading; + expect(h.view.snapshot()).toMatchObject({ + status: "ready", + target: { id: root.id }, + }); +}); + +it("finishing an older refresh after a failed retarget keeps the previous target usable", async () => { + const h = setup(); + const { loading } = await targetRead(h); + h.next().respond([root]); + await loading; + const refresh = h.view.refresh(); + const oldTarget = h.next(); + const next = h.view.retarget("c".repeat(64), new AbortController().signal); + h.next().respond([]); + expect(await next).toBe("unavailable"); + oldTarget.respond([reply]); + await flush(); + h.next().respond([]); + await flush(); + h.next().respond([root]); + await refresh; + expect(h.view.snapshot()).toMatchObject({ + status: "ready", + targetStatus: "ready", + target: { id: reply.id }, + }); +}); + +it("a failed retarget cannot strand a pending missing-target repair in loading", async () => { + const h = setup(); + const { loading } = await targetRead(h); + h.next().respond([root]); + await loading; + const missing = h.view.refresh(); + h.next().respond([]); + await missing; + expect(h.view.snapshot()).toMatchObject({ + root: { id: root.id }, + targetStatus: "unavailable", + }); + const repair = h.view.refresh(); + const repairingTarget = h.next(); + expect(h.view.snapshot().targetStatus).toBe("loading"); + const invalid = h.view.retarget("c".repeat(64), new AbortController().signal); + h.next().respond([]); + expect(await invalid).toBe("unavailable"); + repairingTarget.respond([reply]); + await flush(); + h.next().respond([]); + await flush(); + h.next().respond([root]); + await repair; + expect(h.view.snapshot()).toMatchObject({ + status: "ready", + targetStatus: "ready", + target: { id: reply.id }, + }); +}); diff --git a/src/features/relay/threads.ts b/src/features/relay/threads.ts index fd78b5e4b..78313a47e 100644 --- a/src/features/relay/threads.ts +++ b/src/features/relay/threads.ts @@ -47,11 +47,19 @@ export type ThreadSnapshot = Readonly<{ target?: ChannelMessage | undefined; targetStatus?: "loading" | "ready" | "unavailable" | "error" | undefined; }>; +export type ThreadRetarget = + | "selected" + | "different" + | "unavailable" + | "error" + | "cancelled"; export type ThreadView = { snapshot(): ThreadSnapshot; subscribe(listener: () => void): () => void; refresh(): Promise; loadMore(): Promise; + /** Change exact selection only when verified ancestry belongs to this thread. */ + retarget(messageId: string, signal: AbortSignal): Promise; dispose(): void; }; @@ -62,6 +70,8 @@ export function createThreadView({ relayAuthor, signingAuthority, reader, + targetReader = reader, + admitTarget, seed, local, canAccess, @@ -77,6 +87,8 @@ export function createThreadView({ /** Explicit NIP-11 self; only it may attribute messages to others. */ signingAuthority?: string | undefined; reader: RelayReader; + targetReader?: RelayReader; + admitTarget?: (events: readonly RelayEvent[]) => readonly RelayEvent[]; seed: RelayEvent | undefined; local: LocalEvents | undefined; canAccess(): boolean; @@ -90,6 +102,7 @@ export function createThreadView({ | ((events: readonly RelayEvent[]) => readonly RelayEvent[]) | undefined; }) { + const acceptTarget = admitTarget ?? admit; let disposed = false; let rootId: string | undefined; let rootUnavailable = false; @@ -98,6 +111,9 @@ export function createThreadView({ : undefined; let remote: readonly RelayEvent[] = []; let staged: readonly RelayEvent[] = []; + let selectionStaged: readonly RelayEvent[] = []; + let selectionController: AbortController | undefined; + let selectionRevision = 0; // Strict reads require transport-supplied binding authority. Missing scope is // a configuration error, not evidence of an old relay that permits fallback. let mode: "probe" | "older" | "legacy" = canonicalChannel.test(channelId) @@ -116,19 +132,22 @@ export function createThreadView({ limited: false, }); const listeners = new Set<() => void>(); - function related(events: readonly T[]): T[] { - if (!rootId && !exact) return []; + function related( + events: readonly T[], + selectedId = exact ? messageId : undefined, + ): T[] { + if (!rootId && !selectedId) return []; const rows = events.filter( (event) => !AUX.has(event.kind) && event.kind !== 39007 && inChannel(event, channelId) && (event.id === rootId || - (exact && event.id === messageId) || + (!!selectedId && event.id === selectedId) || (!!rootId && threadReference(event)?.rootId === rootId)), ); const ids = new Set([ - ...(exact ? [messageId] : []), + ...(selectedId ? [selectedId] : []), ...[...remote, ...staged, ...rows].map((event) => event.id), ]); const result = new Map(rows.map((event) => [event.id, event])); @@ -242,6 +261,8 @@ export function createThreadView({ if (events.length > MAX_EVENTS || byteSize(events) > MAX_BYTES) { // Never silently evict a deletion/ancestor then display resurrected content. controller?.abort(); + selectionController?.abort(); + selectionStaged = []; remote = []; staged = []; rootId = undefined; @@ -276,6 +297,9 @@ export function createThreadView({ } function purge(clear = false) { controller?.abort(); + selectionController?.abort(); + selectionController = undefined; + selectionStaged = []; controller = undefined; again = false; staged = []; @@ -297,6 +321,123 @@ export function createThreadView({ readKind: undefined, }); } + // Exact selection shares the same bounded evidence and live reconciliation + // as initial navigation, but never uses the selected row as a page cursor. + async function readTarget( + id: string, + owned: AbortController, + expectedRoot?: string, + ): Promise { + const active = () => + !disposed && + !owned.signal.aborted && + (expectedRoot ? selectionController : controller) === owned; + let pending: readonly RelayEvent[] = []; + const stage = (events: readonly RelayEvent[]) => { + pending = events; + if (expectedRoot) selectionStaged = events; + else staged = events; + return retain(union(remote, staged, selectionStaged), false); + }; + const response = await targetReader.read( + [{ ids: [id], "#h": [channelId], limit: 1 }], + { signal: owned.signal }, + ); + if (!active()) return "cancelled"; + const event = acceptTarget(response).find( + (event) => + event.id === id && contentKind(event) && inChannel(event, channelId), + ); + if (!active()) return "cancelled"; + if (!event) return "unavailable"; + const resolvedRoot = threadReference(event)?.rootId ?? event.id; + if (expectedRoot && resolvedRoot !== expectedRoot) return "different"; + if (!expectedRoot) rootId = resolvedRoot; + // Staging makes live overlay deletions resolvable without exposing an + // 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 }], + { signal: owned.signal }, + ); + if (!active()) return "cancelled"; + if (!stage(union(pending, related(overlays, id)))) return "error"; + const ids = union(remote, pending) + .filter( + (event) => + AUX.has(event.kind) && + event.tags.some(([name, value]) => name === "e" && value === id), + ) + .map((event) => event.id); + if (ids.length) { + const tombstones = await targetReader.read( + [{ kinds: [5, 9005], "#e": ids, limit: 500 }], + { signal: owned.signal }, + ); + if (!active()) return "cancelled"; + if (!stage(union(pending, related(tombstones, id)))) return "error"; + } + const accepted = acceptTarget(pending); + if (!active()) return "cancelled"; + const combined = union(remote, related(accepted, id)); + if (!retain(combined)) return "error"; + stage([]); + if ( + expectedRoot && + !foldMessages(channelId, relayAuthor, combined, { + includeReplies: true, + signingAuthority, + }).some((row) => row.id === id) + ) { + // Keep real tombstones, but do not replace the selected target with a + // message whose complete fold is unavailable. + publish(); + return "unavailable"; + } + return "selected"; + } + async function retarget( + id: string, + signal: AbortSignal, + ): Promise { + if (disposed || signal.aborted || !canAccess()) return "cancelled"; + if (!/^[0-9a-f]{64}$/.test(id)) return "unavailable"; + // A route hint is never evidence that two messages belong to one thread. + if (!rootId || !snapshot.root) return "different"; + selectionController?.abort(); + selectionStaged = []; + const owned = new AbortController(); + const cancel = () => owned.abort(); + signal.addEventListener("abort", cancel, { once: true }); + selectionController = owned; + try { + const result = await readTarget(id, owned, rootId); + if (disposed || owned.signal.aborted || selectionController !== owned) + return "cancelled"; + if (result !== "selected") return result; + selectionRevision++; + messageId = id; + exact = true; + targetStatus = "ready"; + publish( + controller + ? {} + : { status: "ready", error: undefined, readKind: undefined }, + ); + return snapshot.targetStatus === "ready" ? "selected" : "unavailable"; + } catch { + return disposed || owned.signal.aborted || selectionController !== owned + ? "cancelled" + : "error"; + } finally { + signal.removeEventListener("abort", cancel); + if (selectionController === owned) { + selectionStaged = []; + selectionController = undefined; + } + } + } + async function run(replace: boolean) { if (disposed) return; if (controller) { @@ -326,71 +467,21 @@ export function createThreadView({ }); try { if (exact && replace) { - const response = await reader.read( - [{ ids: [messageId], "#h": [channelId], limit: 1 }], - { signal: owned.signal }, - ); + const revision = selectionRevision; + const result = await readTarget(messageId, owned); if (!active()) return; - // Admit the immutable selected content before retention, just as normal - // reads do. Only its separately fetched overlays wait for closure. - const selected = admit(response); - if (!active()) return; - const event = selected.find( - (event) => - event.id === messageId && - contentKind(event) && - inChannel(event, channelId), - ); - if (!event) { + if (revision === selectionRevision && result !== "selected") { targetStatus = "unavailable"; publish({ status: "ready", canLoadMore: false }); return; } - rootId = threadReference(event)?.rootId ?? event.id; - if (!retain(union(remote, [event]))) return; - // ID reads do not expand overlays. Fold the selected row even when it - // lies beyond the thread's traversal cap; its ID never supplies a cursor. - const overlays = await reader.read( - [ - { - kinds: [5, 7, 9005, 40003, 39005], - "#e": [messageId], - limit: 500, - }, - ], - { signal: owned.signal }, - ); - if (!active()) return; - staged = related(overlays); - if (!retain(union(remote, staged), false)) return; - const ids = union(remote, staged) - .filter( - (event) => - AUX.has(event.kind) && - event.tags.some( - ([name, value]) => name === "e" && value === messageId, - ), - ) - .map((event) => event.id); - if (ids.length) { - const tombstones = await reader.read( - [{ kinds: [5, 9005], "#e": ids, limit: 500 }], - { signal: owned.signal }, - ); - if (!active()) return; - staged = union(staged, related(tombstones)); - if (!retain(union(remote, staged), false)) return; - } - // Keep incomplete finite overlays out of both our displayed fold and - // the shared observation path. Live evidence still reconciles immediately. - const accepted = admit([event, ...staged]); - if (!active() || !retain(union(remote, related(accepted)))) return; - staged = []; - targetStatus = "ready"; - publish(); - if (snapshot.targetStatus !== "ready") { - publish({ status: "ready", canLoadMore: false }); - return; + if (revision === selectionRevision) { + targetStatus = "ready"; + publish(); + if (snapshot.targetStatus !== "ready") { + publish({ status: "ready", canLoadMore: false }); + return; + } } } if (!rootId) { @@ -599,7 +690,9 @@ export function createThreadView({ // Staged verified IDs allow immediate live delete-of-overlay access checks, // without publishing the incomplete finite overlay into any shared view. event: (id: string) => - [...remote, ...staged].find((event) => event.id === id), + [...remote, ...staged, ...selectionStaged].find( + (event) => event.id === id, + ), receive, purge, changed: () => publish(), @@ -611,6 +704,7 @@ export function createThreadView({ listeners.delete(listener); }; }, + retarget, refresh: () => run(true), loadMore: () => run(false), dispose() { diff --git a/tests/browser/message-navigation.spec.mjs b/tests/browser/message-navigation.spec.mjs index e04abe0ad..97e3355cb 100644 --- a/tests/browser/message-navigation.spec.mjs +++ b/tests/browser/message-navigation.spec.mjs @@ -1108,3 +1108,65 @@ liveTest( ).toBeVisible(); }, ); + +// The real editor's DOM lifetime and reply context must survive same-thread +// exact navigation, not merely reload text from durable drafts. +test("opening another message in the same thread keeps the reply editor mounted", async ({ + page, + app, +}) => { + await open(page, app); + const composer = page.getByRole("textbox", { + name: "Reply to thread", + exact: true, + }); + // First convert an ordinary root opening to exact selection, then retarget + // again. This must not depend on starting with an exact-mode reader. + expect( + await openTarget(page, { + ...target(app, app.exact.root.id), + threadRootId: app.exact.root.id, + }), + ).toEqual({ status: "opened" }); + const replyRow = thread(page).locator( + `[data-message-id="${app.exact.replies.at(-2).id}"]`, + ); + await replyRow.hover(); + await replyRow.getByRole("button", { name: "Reply", exact: true }).click(); + await expect( + page.getByRole("button", { name: "Cancel reply target", exact: true }), + ).toBeVisible(); + // Capture the chosen reply's context: navigation must not swap its target. + const replyTarget = page + .locator("form") + .filter({ has: composer }) + .getByText(/^Replying to /) + .locator(".."); + const chosenTarget = await replyTarget.textContent(); + expect(chosenTarget).toBeTruthy(); + await composer.fill("Keep my unfinished reply"); + await composer.evaluate((element) => { + window.retainedReplyEditor = element; + }); + const next = app.exact.replies.at(-2).id; + expect(await openTarget(page, target(app, next))).toEqual({ + status: "opened", + }); + await expect( + thread(page).locator(`[data-message-id="${next}"]`), + ).toBeFocused(); + await expect(composer).toHaveText("Keep my unfinished reply"); + expect( + await composer.evaluate( + (element) => element === window.retainedReplyEditor, + ), + ).toBe(true); + expect(await openTarget(page, target(app))).toEqual({ status: "opened" }); + expect( + await composer.evaluate( + (element) => element === window.retainedReplyEditor, + ), + ).toBe(true); + await expect(composer).toHaveText("Keep my unfinished reply"); + await expect(replyTarget).toHaveText(chosenTarget); +}); From 40890abef86ffdeb61fa45a62a44e0dc31c921e0 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Sat, 10 Oct 2026 22:38:19 -0700 Subject: [PATCH 2/2] Close activity popups through Base UI after their actions The thread composer's Working now popup and the sidebar's channel activity popup closed by flipping their open state directly when a row action navigated. Base UI's hover and press tracking never reset, so a trigger that stayed mounted would not open again on hover. Keeping a thread open across same-thread navigation made this reachable in the thread composer; the sidebar popup already showed it as an intermittent failure in the Me sidebar journey. Both now close through Base UI's own close action. Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- .../channels/ChannelActivityPopover.tsx | 18 +++++++++++++++--- src/features/messages/TypingIndicator.tsx | 8 ++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/bundled/channels/ChannelActivityPopover.tsx b/src/bundled/channels/ChannelActivityPopover.tsx index d4fa9288e..aebf334e3 100644 --- a/src/bundled/channels/ChannelActivityPopover.tsx +++ b/src/bundled/channels/ChannelActivityPopover.tsx @@ -17,6 +17,7 @@ import { useState, useSyncExternalStore, type ReactElement, + type ComponentProps, } from "react"; import { selectProfiles } from "../../features/relay/profile-selection"; import type { Profile } from "../../features/relay/contracts"; @@ -196,6 +197,10 @@ export function ChannelActivityPopover({ const agentIds = activeAgents.join(","); const [open, setOpen] = useState(false); const triggerPointerType = useRef(null); + const actions = + useRef< + NonNullable["actionsRef"]>["current"] + >(null); const [now, setNow] = useState(() => Date.now()); useEffect(() => { if (!open || !activeAgents.length) return; @@ -236,10 +241,17 @@ export function ChannelActivityPopover({ if (!hasActivity) setOpen(false); }, [hasActivity]); const stale = items.length > 0 && snapshot.freshness === "stale"; + // Row actions close through Base UI so its hover/press state resets; the + // sidebar trigger stays mounted after the navigation they start. + const close = () => { + if (actions.current) actions.current.close(); + else setOpen(false); + }; return ( { if (next && !hasActivity) return; if (details.reason === "trigger-press") { @@ -320,13 +332,13 @@ export function ChannelActivityPopover({ } openLabel={`Open ${messageId ? "thread" : "conversation"} for ${name} in ${channelName}`} onOpen={() => { - setOpen(false); + close(); onOpenWorkingAgent?.(channelId, agent, messageId); }} onOpenActivity={ openAgentActivity ? () => { - setOpen(false); + close(); openAgentActivity(channelId, agent); } : undefined @@ -348,7 +360,7 @@ export function ChannelActivityPopover({ session={session} agents={activeAgents} onOpen={(selected) => { - setOpen(false); + close(); onOpenThread(selected); }} /> diff --git a/src/features/messages/TypingIndicator.tsx b/src/features/messages/TypingIndicator.tsx index fbac9ad83..02a351113 100644 --- a/src/features/messages/TypingIndicator.tsx +++ b/src/features/messages/TypingIndicator.tsx @@ -272,8 +272,12 @@ function AgentChooser({ // The popup action unmounts after navigation. Existing panel navigation // captures activeElement, so give it the retained avatar control instead. trigger.current?.focus({ preventScroll: true }); - if (openTarget?.(target)) setOpen(false); - else if (previous instanceof HTMLElement) + // Close through Base UI so its hover/press state resets too. A retained + // conversation keeps this control mounted after navigation. + if (openTarget?.(target)) { + if (actions.current) actions.current.close(); + else setOpen(false); + } else if (previous instanceof HTMLElement) previous.focus({ preventScroll: true }); }; return (