From 70cf4924322876686fa6f3a113a0c32a60bd690e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 5 Nov 2026 12:27:12 +1100 Subject: [PATCH 1/7] fix(channels): open the unread reply from sidebar Activity Choosing an Unread threads row in a channel's or Solo chat's sidebar Activity popover opened the thread root. That open is positioned at the bottom of the thread. A reply nested under another reply renders only inside its top-level ancestor's collapsed branch, so it stayed out of sight. It also stayed unread, and the popover kept offering the same click. The row now opens the reply it previews (latestMessageId) with its thread root, in both sidebars. ThreadPanel's exact reveal then expands the branch, scrolls to the reply and focuses it, and reading marks it read. The channel sidebar passes the reply straight to openActivityMessage. The Solo sidebar adds the thread to its buzz://message link. Exact targets reveal and focus their row. They do not use the positioned open's focus handoff. For a thread that is already open, the request reaches the reveal only after the retarget read finishes. By then the person may have moved focus, for example into the channel composer. useMessageReveal now records where focus was when a request began and counts presses and clicks. Its first reveal leaves focus that the person moved since the request. RetainedThreadPanel records the request before awaiting retarget. This keeps "an Activity reopening leaves focus the person moved meanwhile" passing. Focus that an opening moves by itself, such as a tab's composer handoff, is still taken as before. Tests: - ChannelInventory and SoloInventory: an Activity thread row navigates with messageId = latestMessageId and threadRootId = root. Both fail on the old wiring. - use-message-reveal: moved focus is kept for pointer and key input, including when the request began before the hook received it. Opening-moved focus is still taken. The three new kept-focus cases fail without the guard. - thread-unread.spec.mjs: new journey. A nested reply opened from Activity is in view and focused, then read and gone from the popover. It fails on the old wiring in Chromium and WebKit. - "same-thread sidebar activity replaces timeline focus return" now checks that Activity addresses the nested reply in the same thread. It no longer checks for a reused history entry, because an exact reply target is a new visit. The focus-return assertion is unchanged. The help guide now describes choosing an unread thread from a channel's activity. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- docs/buzz-help.md | 3 + src/bundled/channels/ChannelSidebarItem.tsx | 6 +- .../ChannelInventory.test.tsx | 64 +++++++++++++++ .../channel-navigation/ChannelInventory.tsx | 7 +- .../channel-navigation/SoloInventory.test.tsx | 4 +- .../channel-navigation/SoloInventory.tsx | 8 +- src/features/messages/ThreadPanel.tsx | 8 +- .../messages/use-message-reveal.test.tsx | 54 ++++++++++++- src/features/messages/use-message-reveal.ts | 37 ++++++++- tests/browser/thread-unread.spec.mjs | 79 +++++++++++++++---- 10 files changed, 237 insertions(+), 33 deletions(-) diff --git a/docs/buzz-help.md b/docs/buzz-help.md index ece8eec1e1..a47a30106d 100644 --- a/docs/buzz-help.md +++ b/docs/buzz-help.md @@ -821,6 +821,9 @@ for 30 days expires. **Open Activity** opens the page. The card's **Unread only** switch is separate from the Activity page's filters and hides reactions when on. The preview is not shown on touch devices or while the Activity page is open. +- **Channel activity**: rest the pointer on a channel or Solo chat in the + sidebar to see its working agents and **Unread threads**. Choosing an unread + thread opens it at the reply shown, expanding the replies it is nested under. - **Sidebar width**: drag the sidebar's right edge, or focus its resize handle and use the Left/Right arrow keys. Double-click the handle to reset the width. Messages and plugin pages share the saved width within the same community diff --git a/src/bundled/channels/ChannelSidebarItem.tsx b/src/bundled/channels/ChannelSidebarItem.tsx index 29d3857a45..87d931c030 100644 --- a/src/bundled/channels/ChannelSidebarItem.tsx +++ b/src/bundled/channels/ChannelSidebarItem.tsx @@ -47,7 +47,7 @@ type ItemProps = { working: boolean; selected: string | undefined; onSelect: (id: string) => void; - onOpenThread: (channelId: string, rootId: string) => void; + onOpenThread: (channelId: string, messageId: string, rootId: string) => void; onOpenMessage: (channelId: string, messageId: string) => void; onOpenWorkingAgent?: ( channelId: string, @@ -268,7 +268,9 @@ function ChannelSidebarItemCore({ agentProfiles={agentProfiles} {...(onOpenWorkingAgent ? { onOpenWorkingAgent } : {})} {...(onOpenAgentActivity ? { onOpenAgentActivity } : {})} - onOpenThread={(item) => onOpenThread(item.channelId, item.rootId)} + onOpenThread={(item) => + onOpenThread(item.channelId, item.latestMessageId, item.rootId) + } onOpenMention={(item) => onOpenMessage(item.channelId, item.messageId) } diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index 6fc0df6832..3c99618350 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -907,6 +907,70 @@ it("copies a channel link that opens the same channel", async () => { ).toBeInTheDocument(); }); +it("opens an Activity thread row at the reply it previews, in its thread", async () => { + const root = "d".repeat(64); + const reply = "e".repeat(64); + const h = fixture(); + const item = { + channelId: "beta", + rootId: root, + latestMessageId: reply, + authorId: "b".repeat(64), + preview: "Nested reply", + unreadCount: 1, + createdAt: 1, + }; + // Stable per channel, as external-store snapshots must be. + const snapshots = new Map( + ["alpha", "beta", "gamma"].map((id) => [ + id, + { + channelId: id, + items: id === "beta" ? [item] : [], + coverage: "observed" as const, + freshness: "observed" as const, + }, + ]), + ); + const unread = h.session.unread; + h.session.unread = { + ...unread, + activity: (id) => snapshots.get(id) ?? unread.activity(id), + subscribeActivity: () => () => {}, + }; + // The sidebar reads back the attempt it started to hand off focus return. + let opened: unknown; + vi.mocked(h.navigator.open).mockImplementation(async (next) => { + opened = next; + return { status: "opened" }; + }); + Object.assign(h.navigator, { + snapshot: () => ({ + attempt: { + id: "visit", + entry: { id: "entry", target: opened }, + signal: new AbortController().signal, + }, + }), + }); + render(h.view("alpha")); + const beta = await screen.findByRole("button", { name: /^beta/ }); + beta.focus(); + await userEvent.keyboard("{Enter}"); + const popup = await screen.findByRole("dialog", { name: "Activity in beta" }); + await userEvent.click( + within(popup).getByRole("button", { name: /^Open unread thread from/ }), + ); + expect(h.navigator.open).toHaveBeenLastCalledWith( + expect.objectContaining({ + kind: "conversation", + channelId: "beta", + messageId: reply, + threadRootId: root, + }), + ); +}); + it("offers DMs a Move conversation menu and relocates them into a saved group", async () => { const saved: SidebarPreferences = { sections: [{ id: "work", name: "Work", order: 0 }], diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index b451594107..c93f68779f 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -467,11 +467,6 @@ export function ChannelInventory({ }, [viewer, relay, queries, handoff, sidebar.list, navigator, scope], ); - const openActivityThread = useCallback( - (channelId: string, rootId: string) => - openActivityMessage(channelId, rootId, rootId), - [openActivityMessage], - ); const openWorkingAgent = useCallback( (channelId: string, _agent: string, messageId: string | undefined) => { const root = @@ -1543,7 +1538,7 @@ export function ChannelInventory({ working={workingChannels.has(channel.id)} selected={composingMessage ? undefined : selected} onSelect={select} - onOpenThread={openActivityThread} + onOpenThread={openActivityMessage} onOpenMessage={openActivityMessage} onOpenWorkingAgent={openWorkingAgent} onOpenAgentActivity={openAgentActivity} diff --git a/src/features/channel-navigation/SoloInventory.test.tsx b/src/features/channel-navigation/SoloInventory.test.tsx index b7323f674d..7dff1360b4 100644 --- a/src/features/channel-navigation/SoloInventory.test.tsx +++ b/src/features/channel-navigation/SoloInventory.test.tsx @@ -543,11 +543,13 @@ it("offers unread thread navigation and mark-read through the shared activity po name: "Open unread thread from Owned Agent: Needs review", }), ); + // The row opens the reply it previews, in its thread, not the thread root. expect(f.open).toHaveBeenLastCalledWith( expect.objectContaining({ kind: "conversation", channelId: id, - messageId: root, + messageId: "e".repeat(64), + threadRootId: root, }), ); }); diff --git a/src/features/channel-navigation/SoloInventory.tsx b/src/features/channel-navigation/SoloInventory.tsx index 6c61336004..7041d6ad7c 100644 --- a/src/features/channel-navigation/SoloInventory.tsx +++ b/src/features/channel-navigation/SoloInventory.tsx @@ -169,12 +169,12 @@ export function SoloInventory({ [navigator, scope], ); const openMessage = useCallback( - (channelId: string, messageId?: string) => { + (channelId: string, messageId?: string, threadRootId?: string) => { if (handoff && handoff.session !== session) return; const destination = messageId && session.viewer ? buzzLinkTarget( - `buzz://message?channel=${channelId}&id=${messageId}`, + `buzz://message?channel=${channelId}&id=${messageId}${threadRootId ? `&thread=${threadRootId}` : ""}`, { viewer: session.viewer, communityOrigin: scope.slice(0, -(session.viewer.length + 1)), @@ -224,7 +224,9 @@ export function SoloInventory({ Icon={ChatIcon} onSelect={() => void open(item.id)} onRename={rename} - onOpenThread={(thread) => openMessage(thread.channelId, thread.rootId)} + onOpenThread={(thread) => + openMessage(thread.channelId, thread.latestMessageId, thread.rootId) + } onOpenMention={(mention) => openMessage(mention.channelId, mention.messageId) } diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index f210954af5..ccd7eb9c28 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -58,7 +58,7 @@ import { useListedChannel } from "../relay/listed-channel"; import type { CSSProperties } from "react"; import styles from "./Messages.module.css"; import { Reading, readingPositioned } from "./use-reading"; -import { useMessageReveal } from "./use-message-reveal"; +import { beginReveal, useMessageReveal } from "./use-message-reveal"; import { usePendingRequest } from "./use-pending-request"; import type { PageNavigation } from "../navigation/service"; import { messageViewKey } from "./view-key"; @@ -297,7 +297,11 @@ function RetainedThreadPanel(props: ThreadPanelProps) { return; } if (prior?.view.snapshot().root) { - if (navigation) setRouting(navigation); + if (navigation) { + setRouting(navigation); + // The request starts now; its reveal waits for the retarget. + beginReveal(navigation.signal); + } let result: ThreadRetarget; try { result = await prior.view.retarget(messageId, signal); diff --git a/src/features/messages/use-message-reveal.test.tsx b/src/features/messages/use-message-reveal.test.tsx index 883c43f56d..47215711ae 100644 --- a/src/features/messages/use-message-reveal.test.tsx +++ b/src/features/messages/use-message-reveal.test.tsx @@ -1,10 +1,16 @@ // @vitest-environment jsdom -import { act, cleanup, render, screen } from "@testing-library/react"; +import { + act, + cleanup, + fireEvent, + render, + screen, +} from "@testing-library/react"; import { useRef } from "react"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import styles from "./Messages.module.css"; import { HIGHLIGHT_MAX_WAIT_MS, HIGHLIGHT_QUIET_MS } from "./message-highlight"; -import { useMessageReveal } from "./use-message-reveal"; +import { beginReveal, useMessageReveal } from "./use-message-reveal"; // Control frame/mutation ordering; browser journeys own real layout and inert focus. const frames = new Map(); @@ -112,8 +118,8 @@ function mount( outside, restart: () => result.rerender( {}} />), remountRow: () => result.rerender(), - next: () => - result.rerender(), + next: (signal = new AbortController().signal) => + result.rerender(), other: () => result.rerender( , @@ -263,6 +269,46 @@ it("preserves focus ownership through an effect restart, but a new request can f expect(complete).toHaveBeenCalledTimes(2); }); +it.each(["pointerDown", "keyDown"] as const)( + "leaves focus the person moves before the first reveal (%s)", + async (input) => { + const { outside, complete } = mount(); + fireEvent[input](outside); + outside.focus(); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(outside); + expect(complete).toHaveBeenCalledOnce(); + }, +); + +it("takes focus an opening moved before the first reveal", async () => { + const { row, outside, complete } = mount(); + outside.focus(); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(row); + expect(complete).toHaveBeenCalledOnce(); +}); + +it("counts the person's move from when the request began, before the hook receives it", async () => { + const { outside, complete, next } = mount(); + await idle(); + // An owner that waits before passing the request on, as a thread retarget does. + const request = new AbortController(); + beginReveal(request.signal); + fireEvent.pointerDown(outside); + outside.focus(); + next(request.signal); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(outside); + expect(complete).toHaveBeenCalledTimes(2); +}); + it("retries a focus attempt that did not acquire ownership", async () => { const { row, complete } = mount(); const blocked = vi.spyOn(row, "focus").mockImplementation(() => {}); diff --git a/src/features/messages/use-message-reveal.ts b/src/features/messages/use-message-reveal.ts index e8b8e1b832..e4a2aaf435 100644 --- a/src/features/messages/use-message-reveal.ts +++ b/src/features/messages/use-message-reveal.ts @@ -2,6 +2,30 @@ import { useLayoutEffect, useRef, type RefObject } from "react"; import { highlightWhenSettled, onScreen } from "./message-highlight"; import { readingPositioned } from "./use-reading"; +// Presses and clicks, counted from the first request. An opening may move +// focus itself; a move the person makes while the target loads is theirs. +let inputs = 0; +let counting = false; +const countInput = () => { + inputs++; +}; +const requests = new WeakMap< + AbortSignal, + { focus: Element | null; inputs: number } +>(); + +/** Records where focus was when a reveal request began. Owners that wait + * before passing a request on call this when they first receive it. */ +export function beginReveal(signal: AbortSignal) { + if (!counting) { + counting = true; + document.addEventListener("pointerdown", countInput, true); + document.addEventListener("keydown", countInput, true); + } + if (!requests.has(signal)) + requests.set(signal, { focus: document.activeElement, inputs }); +} + /** One reveal per request signal, never per row/profile/image update. */ export function useMessageReveal({ scroller, @@ -54,6 +78,7 @@ export function useMessageReveal({ [], ); useLayoutEffect(() => { + if (signal && !signal.aborted) beginReveal(signal); if ( !ready || !messageId || @@ -92,10 +117,20 @@ export function useMessageReveal({ placed?.(); settled.current = true; // A replacement may recover lost focus, but must not take it from another - // control after this request already focused a row successfully. + // control after this request already focused a row successfully. Nor + // does the first reveal take focus the person moved since the request. + const start = requests.get(signal); + const active = document.activeElement; + const moved = + !!start && + inputs > start.inputs && + !!active && + active !== document.body && + active !== start.focus; let focusPending = false; if ( focus && + !moved && (focused.current?.signal !== signal || (focused.current.row !== row && (!document.activeElement || diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 3bc9816513..015263fad5 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -418,6 +418,52 @@ test.describe("mentioned reply priority", () => { }); }); +// Browser-only: a collapsed branch mounts no rows, so real layout and focus +// show whether Activity's nested reply is revealed in view and then read. +test("opening a nested reply from Activity reveals it in its collapsed branch, then reads it", async ({ + page, + app, +}) => { + await holdReadingFocus(page); + await open(page, app); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + const alpha = page.locator('button[data-channel-id="alpha"]'); + const previews = () => + page.evaluate(() => + window.fixtureRelay + .snapshot() + .session.unread.activity("alpha") + .items?.map((item) => item.preview), + ); + await alpha.hover(); + const popover = page.getByRole("dialog", { name: "Activity in Alpha" }); + const item = (text) => + popover + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: text }); + // Nested under the top-level "Broadcast reply", whose branch starts collapsed. + await expect(item("Broadcast descendant")).toBeVisible(); + await releaseReadingFocus(page); + await item("Broadcast descendant").click(); + const target = await page.evaluate( + () => window.fixtureNavigation.snapshot().entry.target, + ); + expect(target).toMatchObject({ channelId: "alpha", threadRootId: root.id }); + expect(target.messageId).not.toBe(root.id); + const reply = page + .getByRole("complementary", { name: "Thread", exact: true }) + .locator(`[data-message-id="${target.messageId}"]`); + await expect(reply).toContainText("Broadcast descendant"); + await expect(reply).toBeInViewport(); + await expect(reply).toBeFocused(); + await expect.poll(previews).not.toContain("Broadcast descendant"); + await alpha.hover(); + await expect(item("Unread reply 1")).toBeVisible(); + await expect(item("Broadcast descendant")).toHaveCount(0); +}); + test("thread buttons show observed unread independently, clear only after reading, and expose hover/focus affordance", async ({ page, app, @@ -733,8 +779,8 @@ test("thread buttons show observed unread independently, clear only after readin ).toEqual([]); }); -// Browser focus plus actual controller/React wiring: a same-target open must not -// leave the timeline's trigger installed merely because the history entry is reused. +// Browser focus plus actual controller/React wiring: Activity retargeting the +// thread the timeline is still opening must not leave the timeline's trigger. test("same-thread sidebar activity replaces timeline focus return", async ({ page, app, @@ -774,10 +820,9 @@ test("same-thread sidebar activity replaces timeline focus return", async ({ await releaseReadingFocus(page); await trigger.click(); await expect.poll(() => requested).toBe(true); - const before = await page.evaluate(() => { - const { entry, attempt } = window.fixtureNavigation.snapshot(); - return { entry: entry.id, attempt: attempt.id }; - }); + const before = await page.evaluate( + () => window.fixtureNavigation.snapshot().attempt.id, + ); await alpha.hover(); const activity = page.getByRole("dialog", { name: "Activity in Alpha" }); await activity @@ -785,17 +830,23 @@ test("same-thread sidebar activity replaces timeline focus return", async ({ name: /Open unread thread from.*Broadcast descendant/, }) .click(); + // Activity addresses its nested reply, in the same thread. await expect .poll(() => - page.evaluate((before) => { - const { entry, attempt } = window.fixtureNavigation.snapshot(); - return { - sameEntry: entry.id === before.entry, - freshAttempt: attempt.id !== before.attempt, - }; - }, before), + page.evaluate( + ({ before, rootId }) => { + const { entry, attempt } = window.fixtureNavigation.snapshot(); + return { + reply: + entry.target.messageId !== rootId && + entry.target.threadRootId === rootId, + freshAttempt: attempt.id !== before, + }; + }, + { before, rootId: root.id }, + ), ) - .toEqual({ sameEntry: true, freshAttempt: true }); + .toEqual({ reply: true, freshAttempt: true }); release(); await expect .poll(() => From fceece05d7ff85967d1fd4d89a786e3dafcea7b5 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 5 Nov 2026 12:58:53 +1100 Subject: [PATCH 2/7] fix(messages): count only focus a press moved while a reveal loads Review of 310aa4314 found two gaps in the reveal's moved-focus guard. The guard gave up a request's focus claim when any press or click landed during the load and focus was no longer where it was when the request began. Nothing tied the press to the move. Suppose an opening moved focus itself, such as a menu or dialog restoring focus to its trigger or a tab's composer handoff, and then the person made an unrelated press: Shift, Escape, an auto-repeated Enter, or a click on message text that focuses the scroller. The reveal then left its row unfocused. useMessageReveal now records the element that gains focus within a counted press or click's own task. A reveal leaves focus only while it is still on the element the person's input focused after the request began. Focus that arrives later is the opening's, whatever was pressed meanwhile. Each request now records only the input count when it began. The request start was also recorded only when the hook first saw the signal. A cold message visit waits for channel resolution and the head read before the timeline mounts, so a click during that wait counted as coming before the request. ChannelsPage now starts a message visit's request when it receives the visit. Thread visits that it routes start at the same point. beginReveal's JSDoc now says that the first call for a signal fixes its start, and names the requests that still start late: - conversation-tab and Inbox-preview reveals start when their timeline mounts; - visits made while the community is connecting start when the channel workspace mounts. Those paths take focus as they did before 310aa4314. Tests: - use-message-reveal: an opening's focus move is taken after a press that moved no focus (Shift, Escape, repeated Enter) and after a click that focused the timeline scroller. All four cases fail on the previous rule. Focus moved within the press that began the request is still taken; this catches a >= off-by-one. - message-navigation.spec.mjs: new journey. An exact message opened into a channel whose head read is held keeps the composer focus the person clicked into meanwhile, and the message is revealed in view. It fails without the ChannelsPage start in Chromium and WebKit. - thread-unread.spec.mjs: "same-thread sidebar activity replaces timeline focus return" now runs for an unread reply (a new visit) and for a working agent's top-level message. The working-agent variant reopens the root, so it again covers a reused history entry with a fresh attempt. Both fail in Chromium when ChannelsPage's Activity trigger handoff is removed. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src/bundled/channels/ChannelsPage.tsx | 7 + .../messages/use-message-reveal.test.tsx | 53 +++++ src/features/messages/use-message-reveal.ts | 48 +++-- tests/browser/message-navigation.spec.mjs | 49 +++++ tests/browser/thread-unread.spec.mjs | 195 +++++++++++------- 5 files changed, 255 insertions(+), 97 deletions(-) diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index d172dd057c..59f1c43458 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -24,6 +24,7 @@ import { Avatar } from "../../shared/design-system/ui/Avatar"; import { useToastNotification } from "../../shared/design-system/ui/Toast"; import { useReturnHighlight } from "../../features/messages/use-return-highlight"; import { useMessageOpening } from "../../features/messages/use-message-opening"; +import { beginReveal } from "../../features/messages/use-message-reveal"; import { ChannelBody } from "./ChannelBody"; import { entryPane, @@ -677,6 +678,12 @@ function ChannelWorkspace({ navigation?.target.kind === "conversation" ? navigation.target.threadRootId : undefined; + useLayoutEffect(() => { + // The message's reveal waits for its channel, timeline or thread to load; + // focus the person moves meanwhile counts from now. + if (requestedMessage && navigation && !navigation.signal.aborted) + beginReveal(navigation.signal); + }, [requestedMessage, navigation]); const membersRequested = navigation?.target.kind === "conversation" && navigation.target.panel === "members"; diff --git a/src/features/messages/use-message-reveal.test.tsx b/src/features/messages/use-message-reveal.test.tsx index 47215711ae..a79a1b9657 100644 --- a/src/features/messages/use-message-reveal.test.tsx +++ b/src/features/messages/use-message-reveal.test.tsx @@ -309,6 +309,59 @@ it("counts the person's move from when the request began, before the hook receiv expect(complete).toHaveBeenCalledTimes(2); }); +/** End the press's task, as the browser does before an opening's later work. */ +const nextTask = () => + act(() => new Promise((resolve) => setTimeout(resolve))); + +it.each([{ key: "Shift" }, { key: "Escape" }, { key: "Enter", repeat: true }])( + "takes focus an opening moves after a press that moved none (%o)", + async (key) => { + const { row, outside, complete } = mount(); + fireEvent.keyDown(document.body, key); + await nextTask(); + // The opening's own move, as a closing menu restoring its trigger. + outside.focus(); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(row); + expect(complete).toHaveBeenCalledOnce(); + }, +); + +it("takes focus an opening moves after a click focused the timeline", async () => { + const { row, outside, complete } = mount(); + const scroller = row.parentElement as HTMLElement; + scroller.tabIndex = -1; + // Clicking message text focuses its scroller, not a control. + fireEvent.pointerDown(screen.getByText("Other")); + scroller.focus(); + await nextTask(); + outside.focus(); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(row); + expect(complete).toHaveBeenCalledOnce(); +}); + +it("takes focus moved by the press that began the request", async () => { + const { row, outside, complete, next } = mount(); + await idle(); + const request = new AbortController(); + // Enter on a menu item opens the message, and the closing menu restores + // focus to its trigger within the same press. + fireEvent.keyDown(document.body, { key: "Enter" }); + beginReveal(request.signal); + outside.focus(); + next(request.signal); + await frame(); + await frame(); + await frame(); + expect(document.activeElement).toBe(row); + expect(complete).toHaveBeenCalledTimes(2); +}); + it("retries a focus attempt that did not acquire ownership", async () => { const { row, complete } = mount(); const blocked = vi.spyOn(row, "focus").mockImplementation(() => {}); diff --git a/src/features/messages/use-message-reveal.ts b/src/features/messages/use-message-reveal.ts index e4a2aaf435..b05706d826 100644 --- a/src/features/messages/use-message-reveal.ts +++ b/src/features/messages/use-message-reveal.ts @@ -2,28 +2,44 @@ import { useLayoutEffect, useRef, type RefObject } from "react"; import { highlightWhenSettled, onScreen } from "./message-highlight"; import { readingPositioned } from "./use-reading"; -// Presses and clicks, counted from the first request. An opening may move -// focus itself; a move the person makes while the target loads is theirs. +// Presses and clicks, counted from the first request, and the element the +// latest of them focused. A press moves focus within its own task (mousedown, +// Tab); focus that arrives later, as an opening's handoff or a closing menu's +// restore, is not the person's choice, whatever they pressed meanwhile. let inputs = 0; +let pressing = 0; +let chosen: { element: EventTarget | null; input: number } | undefined; let counting = false; const countInput = () => { - inputs++; + const input = ++inputs; + pressing = input; + setTimeout(() => { + if (pressing === input) pressing = 0; + }); +}; +const noteFocus = (event: FocusEvent) => { + if (pressing) chosen = { element: event.target, input: pressing }; }; -const requests = new WeakMap< - AbortSignal, - { focus: Element | null; inputs: number } ->(); +const requests = new WeakMap(); -/** Records where focus was when a reveal request began. Owners that wait - * before passing a request on call this when they first receive it. */ +/** + * Starts a reveal request. Its reveal leaves focus that a press or click made + * since, while focus is still there. The first call for a signal fixes the + * start; later calls are no-ops. The hook calls this when it receives a + * signal, so owners that hold a request before then call it on receipt: the + * channels page for a message visit, a retained thread before its retarget. + * Other requests start late: a conversation tab's or Inbox preview's reveal + * when its timeline mounts, and a visit made while the community is still + * connecting when the channel workspace mounts. + */ export function beginReveal(signal: AbortSignal) { if (!counting) { counting = true; document.addEventListener("pointerdown", countInput, true); document.addEventListener("keydown", countInput, true); + document.addEventListener("focusin", noteFocus, true); } - if (!requests.has(signal)) - requests.set(signal, { focus: document.activeElement, inputs }); + if (!requests.has(signal)) requests.set(signal, inputs); } /** One reveal per request signal, never per row/profile/image update. */ @@ -120,13 +136,11 @@ export function useMessageReveal({ // control after this request already focused a row successfully. Nor // does the first reveal take focus the person moved since the request. const start = requests.get(signal); - const active = document.activeElement; const moved = - !!start && - inputs > start.inputs && - !!active && - active !== document.body && - active !== start.focus; + start !== undefined && + !!chosen && + chosen.input > start && + chosen.element === document.activeElement; let focusPending = false; if ( focus && diff --git a/tests/browser/message-navigation.spec.mjs b/tests/browser/message-navigation.spec.mjs index 05b7db377a..64037239dd 100644 --- a/tests/browser/message-navigation.spec.mjs +++ b/tests/browser/message-navigation.spec.mjs @@ -536,6 +536,55 @@ test("superseding a held exact read cancels it; a late response cannot steal foc await expect.poll(() => status(page)).toBe("opened"); }); +test.describe("cold channel", () => { + test.use({ historyCounts: { alpha: 103, beta: 1 } }); + // Browser-only: the timeline mounts after the channel's head read, so only a + // real load shows that a click made meanwhile keeps its focus. + test("an exact message opened into a loading channel keeps focus the person moved meanwhile", async ({ + page, + app, + }) => { + const beta = Promise.withResolvers(); + await page.route("**/query", async (route) => { + const filters = route.request().postDataJSON() ?? []; + if (filters.some((filter) => filter["#h"]?.includes("beta"))) + await beta.promise; + await route.fallback(); + }); + const [message] = app.histories.get("primary/beta"); + const composer = page.getByRole("textbox", { + name: "Message #Beta", + exact: true, + }); + try { + await open(page, app); + await page.evaluate( + (value) => { + window.fixtureNavigation.open(value); + }, + { + ...target(app, message.id), + channelId: "beta", + threadRootId: undefined, + }, + ); + await expect( + page.getByRole("status").filter({ hasText: "Loading messages…" }), + ).toBeVisible(); + await composer.click(); + } finally { + beta.resolve(); + } + await expect.poll(() => status(page)).toBe("opened"); + await expect( + page.locator( + `[data-channel-timeline="beta"] [data-message-id="${message.id}"]`, + ), + ).toBeInViewport(); + await expect(composer).toBeFocused(); + }); +}); + for (const selected of ["reply", "root"]) { test(`same-scope replacement waits for fresh membership before reopening the exact ${selected}`, async ({ page, diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 015263fad5..1082c45afe 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -1,6 +1,7 @@ import { pauseClock } from "./clock.mjs"; import { openChannelDetails } from "./channel-details.mjs"; import { test, expect } from "./fixture.mjs"; +import { generateSecretKey } from "nostr-tools"; import { clickRow, open, virtuaIdle } from "./timeline.mjs"; import { holdReadingFocus, @@ -781,88 +782,122 @@ test("thread buttons show observed unread independently, clear only after readin // Browser focus plus actual controller/React wiring: Activity retargeting the // thread the timeline is still opening must not leave the timeline's trigger. -test("same-thread sidebar activity replaces timeline focus return", async ({ - page, - app, -}) => { - await holdReadingFocus(page); - await open(page, app); - const root = app.histories - .get("primary/alpha") - .find((row) => row.content === "Thread root 0"); - const alpha = page.locator('button[data-channel-id="alpha"]'); - // The open channel's row is quiet; its activity popover still lists threads. - await expect - .poll(async () => (await channelUnread(page, "alpha")).threads) - .toBeGreaterThan(0); - let release; - const held = new Promise((resolve) => { - release = resolve; - }); - let requested = false; - await page.route("**/api/relay/**/query", async (route) => { - if ( - route - .request() - .postDataJSON() - .some((filter) => filter.depth_limit) - ) { - requested = true; - await held; +// An unread reply is a new visit; a working agent's top-level message reuses +// the timeline's history entry. +for (const source of ["unread reply", "working agent"]) { + test(`same-thread sidebar activity (${source}) replaces timeline focus return`, async ({ + page, + app, + }) => { + await holdReadingFocus(page); + await open(page, app); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + const alpha = page.locator('button[data-channel-id="alpha"]'); + if (source === "working agent") { + await expect + .poll(() => app.relay.hasRoute("primary", "observer")) + .toBe(true); + app.observer( + { + kind: "turn_started", + seq: 1, + timestamp: new Date().toISOString(), + channelId: "alpha", + sessionId: "S", + turnId: "focus-return", + payload: { triggeringEventIds: [root.id] }, + }, + generateSecretKey(), + ); + await expect( + alpha.getByRole("img", { name: /working in Alpha/ }), + ).toBeVisible(); + } else { + // The open channel's row is quiet; its activity popover still lists + // threads. + await expect + .poll(async () => (await channelUnread(page, "alpha")).threads) + .toBeGreaterThan(0); + } + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let requested = false; + await page.route("**/api/relay/**/query", async (route) => { + if ( + route + .request() + .postDataJSON() + .some((filter) => filter.depth_limit) + ) { + requested = true; + await held; + } + await route.continue().catch(() => {}); + }); + try { + const trigger = page + .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) + .getByRole("button", { name: /^View thread:/ }); + await virtuaIdle(page); + await releaseReadingFocus(page); + await trigger.click(); + await expect.poll(() => requested).toBe(true); + const before = await page.evaluate(() => { + const { entry, attempt } = window.fixtureNavigation.snapshot(); + return { entry: entry.id, attempt: attempt.id }; + }); + await alpha.hover(); + const activity = page.getByRole("dialog", { name: "Activity in Alpha" }); + await activity + .getByRole("button", { + name: + source === "working agent" + ? /^Open thread for .* in Alpha$/ + : /Open unread thread from.*Broadcast descendant/, + }) + .click(); + await expect + .poll(() => + page.evaluate( + ({ before, rootId }) => { + const { entry, attempt } = window.fixtureNavigation.snapshot(); + return { + sameEntry: entry.id === before.entry, + rootTarget: entry.target.messageId === rootId, + sameThread: entry.target.threadRootId === rootId, + freshAttempt: attempt.id !== before.attempt, + }; + }, + { before, rootId: root.id }, + ), + ) + .toEqual({ + sameEntry: source === "working agent", + rootTarget: source === "working agent", + sameThread: true, + freshAttempt: true, + }); + release(); + await expect + .poll(() => + page.evaluate(() => window.fixtureNavigation.snapshot().status), + ) + .toBe("opened"); + await page + .locator( + '.buzz-tabs-item:has([data-tab-value^="thread:"][aria-selected="true"]) > button:not([role="tab"]), button[aria-label="Close thread"]', + ) + .click(); + await expect(alpha).toBeFocused(); + } finally { + release(); } - await route.continue().catch(() => {}); }); - try { - const trigger = page - .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`) - .getByRole("button", { name: /^View thread:/ }); - await virtuaIdle(page); - await releaseReadingFocus(page); - await trigger.click(); - await expect.poll(() => requested).toBe(true); - const before = await page.evaluate( - () => window.fixtureNavigation.snapshot().attempt.id, - ); - await alpha.hover(); - const activity = page.getByRole("dialog", { name: "Activity in Alpha" }); - await activity - .getByRole("button", { - name: /Open unread thread from.*Broadcast descendant/, - }) - .click(); - // Activity addresses its nested reply, in the same thread. - await expect - .poll(() => - page.evaluate( - ({ before, rootId }) => { - const { entry, attempt } = window.fixtureNavigation.snapshot(); - return { - reply: - entry.target.messageId !== rootId && - entry.target.threadRootId === rootId, - freshAttempt: attempt.id !== before, - }; - }, - { before, rootId: root.id }, - ), - ) - .toEqual({ reply: true, freshAttempt: true }); - release(); - await expect - .poll(() => - page.evaluate(() => window.fixtureNavigation.snapshot().status), - ) - .toBe("opened"); - await page - .locator( - '.buzz-tabs-item:has([data-tab-value^="thread:"][aria-selected="true"]) > button:not([role="tab"]), button[aria-label="Close thread"]', - ) - .click(); - await expect(alpha).toBeFocused(); - } finally { - release(); - } -}); +} // Browser-only contract: fitting rows generate no scroll/resize when an empty // legacy continuation finishes. Opening alone must still earn reading dwell. From a08a973b3cdfcad24cdd672eeb9b8debf8f6357e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 5 Nov 2026 16:41:59 +1100 Subject: [PATCH 3/7] fix(messages): finish a reveal whose row is still sliding on screen A reveal verifies its row on a frame after scrolling and focusing it. If the row is not yet on screen at that frame it gives up, and only a DOM mutation could reschedule it. Opening a thread from Activity into a collapsed pane slides the dock in from the right edge, so on a slow runner the row was still outside the window at the check (observed row x=1460 against a 1440px window). Moving a row onto the screen is a transform/layout change, not a mutation, so the reveal never completed: the thread panel never settled, reading was never positioned, and the thread stayed unread (thread-unread.spec.mjs "opening a thread from Activity reads it when its channel's pane was collapsed"). When verification finds the row off screen, observe it with an IntersectionObserver and reschedule the reveal once any of it intersects the screen. No timers or retries are involved, and the observer is released on completion, abort and effect cleanup. Reproduced in the Linux WebKit CI image (7 of 16 failed before, 16 of 16 pass after). Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Matt Toohey --- .../messages/use-message-reveal.test.tsx | 63 +++++++++++++++++++ src/features/messages/use-message-reveal.ts | 18 ++++++ 2 files changed, 81 insertions(+) diff --git a/src/features/messages/use-message-reveal.test.tsx b/src/features/messages/use-message-reveal.test.tsx index a79a1b9657..205bc26d86 100644 --- a/src/features/messages/use-message-reveal.test.tsx +++ b/src/features/messages/use-message-reveal.test.tsx @@ -189,6 +189,69 @@ it("does not mark a row that fails verification", async () => { expect(highlighted(row)).toBe(false); }); +it("completes a reveal that finds its row off screen once the row shows", async () => { + const watchers: { + callback: IntersectionObserverCallback; + target: Element; + disconnected: boolean; + }[] = []; + vi.stubGlobal( + "IntersectionObserver", + class { + constructor(private callback: IntersectionObserverCallback) {} + observe(target: Element) { + watchers.push({ + callback: this.callback, + target, + disconnected: false, + }); + } + disconnect() { + for (const watcher of watchers) + if (watcher.callback === this.callback) watcher.disconnected = true; + } + }, + ); + const { row, complete } = mount(); + const onscreen = row.getBoundingClientRect(); + const slidingIn = vi.spyOn(row, "getBoundingClientRect").mockReturnValue({ + ...onscreen, + left: 2000, + right: 2100, + x: 2000, + }); + await frame(); // Attach observers. + await frame(); // Reveal. + await frame(); // Verify: the row is still sliding in from the right. + expect(complete).not.toHaveBeenCalled(); + expect(watchers).toHaveLength(1); + expect(watchers[0]?.target).toBe(row); + const [watcher] = watchers; + // Still outside the window: no retry, no frame spent. + watcher?.callback( + [ + { + intersectionRect: { width: 0, height: 0 }, + } as IntersectionObserverEntry, + ], + {} as IntersectionObserver, + ); + expect(frames.size).toBe(0); + slidingIn.mockReturnValue(onscreen); + watcher?.callback( + [ + { + intersectionRect: { width: 40, height: 100 }, + } as IntersectionObserverEntry, + ], + {} as IntersectionObserver, + ); + await idle(); + expect(complete).toHaveBeenCalledOnce(); + expect(watchers[0]?.disconnected).toBe(true); + expect(frames.size).toBe(0); +}); + it("does not mark a reveal behind a covering panel", async () => { const { row, complete } = mount(undefined, { focus: false }); await frame(); diff --git a/src/features/messages/use-message-reveal.ts b/src/features/messages/use-message-reveal.ts index b05706d826..428d8b0329 100644 --- a/src/features/messages/use-message-reveal.ts +++ b/src/features/messages/use-message-reveal.ts @@ -107,10 +107,12 @@ export function useMessageReveal({ if (!container) return; const target = messageId; let frame = 0; + let arrival: IntersectionObserver | undefined; const cancel = () => { cancelAnimationFrame(frame); observer.disconnect(); inertObserver.disconnect(); + arrival?.disconnect(); }; const schedule = () => { cancelAnimationFrame(frame); @@ -182,6 +184,22 @@ export function useMessageReveal({ ) { holding.current?.(); holding.current = undefined; + // A panel can still be sliding in. Moving a row off or onto the + // screen is not a DOM mutation, so wait for it to show. + arrival?.disconnect(); + if (typeof IntersectionObserver === "undefined") return; + arrival = new IntersectionObserver((entries) => { + if ( + entries.some( + ({ intersectionRect: { width, height } }) => + width > 0 && height > 0, + ) + ) { + arrival?.disconnect(); + schedule(); + } + }); + arrival.observe(row); return; } revealed.current = signal; From 4c08bc8a8b30c4ee6a7e724b779b824f388028ba Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 9 Nov 2026 17:08:05 +1100 Subject: [PATCH 4/7] fix(messages): expand the activity history that absorbs an exact failure target Review of f41773c71 found that an exact Activity target can be a row the thread never mounts. When an agent's public execution failure directly follows its agent-only update, the thread folds the failure into that update's activity history, and the collapsed history hides it. The failure is unread, so the Activity row previews it and, since fed46f688, opens it as an exact target. ThreadPanel's visit effect searched raw activityRuns, which hold only agent-only rows, so it never found the failure's group. The history stayed collapsed, the reveal never found its row, nothing was focused or read, and the visit could not complete. The visit effect now resolves the target's group with the same activityPresentation call that renders each displayed level, including its absorbed failure and the same no-replies absorption rule. One presentLevel helper owns that call for both; the effect moved below the agent pubkeys it needs. activityRuns is no longer imported here. Tests: - ThreadPanel.test: an exact reveal of a failure its activity history absorbs mounts the failure with the history expanded. Fails before the fix: the failure never renders. - thread-unread.spec.mjs: two journeys publish an agent-only update and the agent's public execution failure under the viewer's thread root, then open the failure from the sidebar Activity row of a channel and of a Solo chat. The failure row is in view and focused, the history is expanded, the visit reaches opened, and the thread leaves the popover. Both fail before the fix in Chromium: the row never mounts. Signed-off-by: Matt Toohey Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- src/features/messages/ThreadPanel.test.tsx | 39 ++++++ src/features/messages/ThreadPanel.tsx | 86 +++++++----- tests/browser/thread-unread.spec.mjs | 151 ++++++++++++++++++++- 3 files changed, 238 insertions(+), 38 deletions(-) diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index 9a1f5c0530..1e8d6cd465 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -2523,3 +2523,42 @@ it("reveals a completed step selected by an exact thread link", async () => { screen.getByRole("button", { name: /Collapse activity history/ }), ).toBeVisible(); }); + +it("reveals a public failure that its activity history absorbs by expanding the history", async () => { + const h = messagesHarness(); + const agent = "d".repeat(64); + const update = { + ...row, + id: "e".repeat(64), + authorId: agent, + content: "Read the release notes.", + audience: "agents" as const, + threadRootId: row.id, + replyParentId: row.id, + createdAt: 2, + }; + const target = { + ...row, + id: "f".repeat(64), + authorId: agent, + content: "The selected model could not complete this request.", + notice: "execution-failure" as const, + threadRootId: row.id, + replyParentId: row.id, + createdAt: 3, + }; + h.snapshot.replies = [update, target]; + h.snapshot.target = target; + h.snapshot.targetStatus = "ready"; + Object.assign(h.props, { + messageId: target.id, + retainThread: true, + revealSelected: true, + revealRequest: 1, + }); + h.render(); + expect(await screen.findByText(target.content)).toBeVisible(); + expect( + screen.getByRole("button", { name: /Collapse activity history/ }), + ).toBeVisible(); +}); diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index ccd7eb9c28..d7d45fd2ca 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -8,7 +8,6 @@ import { useChannelActivitySnapshot, useStableRowActivity, } from "./activity-presentation"; -import { activityRuns } from "./activity-groups"; import { DayDivider } from "./MessageTimestamp"; import { workflowLabel } from "../relay/workflow-attribution"; import { usePanelTabHost } from "../panels/PanelWorkspace"; @@ -658,25 +657,6 @@ function ThreadMessages({ [session.unread, channelId, snapshot.root?.id, messageId], ); useSyncExternalStore(subscribeUnread, unreadSnapshot, unreadSnapshot); - const [expandedActivity, setExpandedActivity] = useState>( - new Set(), - ); - const activityVisit = useRef([]); - useLayoutEffect(() => { - const request = [messageId, navigation?.signal, revealRequest]; - if ( - !messageId || - request.every((part, index) => part === activityVisit.current[index]) || - !snapshot.replies.some((row) => row.id === messageId) - ) - return; - activityVisit.current = request; - const group = [...tree.children.values()] - .flatMap((rows) => activityRuns(rows)) - .find((group) => group.some((row) => row.id === messageId)); - const first = group?.[0]?.id; - if (first) setExpandedActivity((current) => new Set([...current, first])); - }, [messageId, navigation?.signal, revealRequest, snapshot.replies, tree]); const [expanded, setExpanded] = useState>(new Set()); const [replyParent, setReplyParent] = useState(); const resolveName = useChannelIdentityNames(session, channelId); @@ -718,6 +698,55 @@ function ThreadMessages({ }, [session.profiles, authors, profileGeneration]); const profiles = useRowProfiles(session.profiles, rows); const agentPubkeys = useKnownAgentPubkeys(session, profiles); + // Each displayed level is projected alone, but a signed claim may name a + // request loaded at another level. + const loadedRows = useMemo( + () => + snapshot.root ? [snapshot.root, ...snapshot.replies] : snapshot.replies, + [snapshot.root, snapshot.replies], + ); + // One displayed level's activity grouping, shared by rendering and by the + // visit that must expand the group hiding its target: a public failure an + // activity run absorbs is hidden with that run. + const presentLevel = useCallback( + (parent: string | undefined) => + activityPresentation( + tree.children.get(parent) ?? [], + activitySnapshot, + agentPubkeys, + parent ?? snapshot.root?.id, + // A failure with replies needs its own visible branch controls. + (failure) => !tree.children.get(failure.id)?.length, + loadedRows, + ), + [tree, activitySnapshot, agentPubkeys, snapshot.root?.id, loadedRows], + ); + const [expandedActivity, setExpandedActivity] = useState>( + new Set(), + ); + const activityVisit = useRef([]); + useLayoutEffect(() => { + const request = [messageId, navigation?.signal, revealRequest]; + if ( + !messageId || + request.every((part, index) => part === activityVisit.current[index]) || + !snapshot.replies.some((row) => row.id === messageId) + ) + return; + activityVisit.current = request; + const group = [...tree.children.keys()] + .flatMap((parent) => presentLevel(parent).groups) + .find((group) => group.ids.includes(messageId)); + const first = group?.ids[0]; + if (first) setExpandedActivity((current) => new Set([...current, first])); + }, [ + messageId, + navigation?.signal, + revealRequest, + snapshot.replies, + tree, + presentLevel, + ]); const scroller = useRef(null); const positioned = useRef(false); const shown = useRef(active); @@ -1061,13 +1090,6 @@ function ThreadMessages({ const rootActivity = rootId ? rootPresentation.byRow.get(rootId) : undefined; // A stopped note may follow only a reply that is always shown: collapsed // branches do not mount their rows. - // Each displayed level is projected alone, but a signed claim may name a - // request loaded at another level. - const loadedRows = useMemo( - () => - snapshot.root ? [snapshot.root, ...snapshot.replies] : snapshot.replies, - [snapshot.root, snapshot.replies], - ); const rootReplies = useMemo( () => activityPresentation( @@ -1489,15 +1511,7 @@ function ThreadMessages({ ); if (parent === undefined && snapshot.root) requestRows.set(snapshot.root.id, snapshot.root); - const presentation = activityPresentation( - chronologicalSiblings, - activitySnapshot, - agentPubkeys, - parent ?? rootId, - // A failure with replies needs its own visible branch controls. - (failure) => !tree.children.get(failure.id)?.length, - loadedRows, - ); + const presentation = presentLevel(parent); const groups = presentation.groups.filter((group) => group.ids.every((id) => !tree.children.get(id)?.length), ); diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 1082c45afe..55210d5fab 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -1,14 +1,14 @@ import { pauseClock } from "./clock.mjs"; import { openChannelDetails } from "./channel-details.mjs"; import { test, expect } from "./fixture.mjs"; -import { generateSecretKey } from "nostr-tools"; +import { finalizeEvent, generateSecretKey } from "nostr-tools"; import { clickRow, open, virtuaIdle } from "./timeline.mjs"; import { holdReadingFocus, readJournal, releaseReadingFocus, } from "./reading.mjs"; -import { openPage } from "./navigation.mjs"; +import { newChat, openPage } from "./navigation.mjs"; import { channelUnread } from "./unread-state.mjs"; test.use({ @@ -465,6 +465,87 @@ test("opening a nested reply from Activity reveals it in its collapsed branch, t await expect(item("Broadcast descendant")).toHaveCount(0); }); +// An agent's public failure right after its agent-only update folds into that +// update's collapsed activity history, while Activity previews the failure +// itself. Opening it must expand the history before the reveal looks for its +// row; otherwise the row never mounts and the visit cannot complete. +test("opening an absorbed failure from Activity expands its activity history, then reads it", async ({ + page, + app, +}) => { + const key = generateSecretKey(); + app.serveProfile(key, { name: "Remote tester", is_agent: true }); + await holdReadingFocus(page); + await open(page, app); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + const failure = publishAbsorbedFailure(app, "alpha", root.id, key); + const alpha = page.locator('button[data-channel-id="alpha"]'); + const previews = () => + page.evaluate(() => + window.fixtureRelay + .snapshot() + .session.unread.activity("alpha") + .items?.map((item) => item.preview), + ); + await alpha.hover(); + const popover = page.getByRole("dialog", { name: "Activity in Alpha" }); + const item = popover + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: failure.content }); + await expect(item).toBeVisible(); + await releaseReadingFocus(page); + await item.click(); + const thread = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const row = thread.locator(`[data-message-id="${failure.id}"]`); + await expect(row).toBeInViewport(); + await expect(row).toBeFocused(); + await expect( + thread.getByRole("button", { name: /^Collapse activity history:/ }), + ).toBeVisible(); + await expect + .poll(() => page.evaluate(() => window.fixtureNavigation.snapshot().status)) + .toBe("opened"); + await expect.poll(previews).not.toContain(failure.content); + await alpha.hover(); + await expect(item).toHaveCount(0); +}); + +/** An agent-only update, then the same agent's public execution failure, as + * direct replies to the viewer's thread root. The failure is unread; the + * thread folds it into the update's activity history. */ +function publishAbsorbedFailure(app, channel, rootId, key) { + const history = app.histories.get(`primary/${channel}`); + const publish = (content, tags, offset) => { + const event = finalizeEvent( + { + kind: tags.some(([name]) => name === "audience") ? 40002 : 9, + content, + tags: [["h", channel], ["e", rootId, "", "reply"], ...tags], + created_at: Math.floor(Date.now() / 1000) + offset, + }, + key, + ); + history.push(event); + app.relay.publish("primary", event); + return event; + }; + publish( + "Read the release notes and checked the headings.", + [["audience", "agents"]], + 0, + ); + return publish( + "The selected model could not complete this request.", + [["buzz:notice", "execution-failure"]], + 1, + ); +} + test("thread buttons show observed unread independently, clear only after reading, and expose hover/focus affordance", async ({ page, app, @@ -1400,3 +1481,69 @@ test.describe("top-level mention activity", () => { await expect(row.getByRole("img", { name: "Unread" })).toHaveCount(0); }); }); + +// The Solo sidebar opens the same exact target through its buzz://message link. +const soloChannel = "33333333-3333-4333-8333-333333333333"; +test.describe("Solo Activity", () => { + test.use({ + threadUnread: false, + threadUnreadJoined: false, + channelIds: ["beta", soloChannel], + channelNames: { [soloChannel]: "Alpha" }, + soloChannels: [soloChannel], + ownSoloChannels: [soloChannel], + historyCounts: { beta: 1, [soloChannel]: 1 }, + }); + + test("opening an absorbed failure from a Solo chat's Activity expands its activity history, then reads it", async ({ + page, + app, + }) => { + const key = generateSecretKey(); + app.serveProfile(key, { name: "Remote tester", is_agent: true }); + await page.goto(app.origin); + await newChat(page); + const chat = page + .getByRole("navigation", { name: "Solo conversations" }) + .locator(`[data-solo-channel-id="${soloChannel}"]`); + await expect(chat).toBeVisible(); + const root = app.append( + "primary", + soloChannel, + "Summarize the release notes.", + ); + const failure = publishAbsorbedFailure(app, soloChannel, root.id, key); + await chat.hover(); + const popover = page.getByRole("dialog", { + name: "Activity in Alpha", + exact: true, + }); + const item = popover + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: failure.content }); + await expect(item).toBeVisible(); + await item.click(); + const thread = page.getByRole("region", { + name: "Thread messages", + exact: true, + }); + const row = thread.locator(`[data-message-id="${failure.id}"]`); + await expect(row).toBeInViewport(); + await expect(row).toBeFocused(); + await expect( + thread.getByRole("button", { name: /^Collapse activity history:/ }), + ).toBeVisible(); + await expect + .poll(() => + page.evaluate(() => window.fixtureNavigation.snapshot().status), + ) + .toBe("opened"); + await expect + .poll(async () => (await channelUnread(page, soloChannel)).threads) + .toBe(0); + await chat.hover(); + await expect( + popover.getByRole("button", { name: /Open unread thread from/ }), + ).toHaveCount(0); + }); +}); From cb6b1e06511d3b42e2075ecede6cafc2aabe93d2 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 9 Nov 2026 20:22:16 +1100 Subject: [PATCH 5/7] fix(messages): keep an exact failure target's history expansion pending until its group forms Review of 34dff868d found an ordering gap in the visit effect that expands the activity history absorbing an exact failure target. A cold exact visit verifies its target before it reads the thread root and window, so the thread first holds the failure alone. The effect ran on that snapshot, recorded the request as handled, and found no group. When the window then supplied the preceding agent-only update, presentLevel absorbed the failure into that update's history, but the unchanged-request guard kept the effect from expanding it. Rendering hid the failure inside the collapsed history: a reveal still waiting for its row could never complete, and an already revealed row vanished. The effect now records the request only once it has found and expanded a containing group. Until then each message-list change runs the lookup again. A reader's explicit collapse after that expansion still holds, because the request is recorded by then and the effect ignores it. For a target that no group ever absorbs, the lookup repeats on each snapshot; it is the same grouping pass that rendering already performs per level. Tests: - ThreadPanel.test: the thread holds only the failure under an exact request, then the same request receives the update and the failure. The history opens with the failure mounted. The reader collapses it, a later reply arrives, and it stays collapsed with the failure hidden. Fails before the fix: the history never opens. - thread-unread.spec.mjs: a cold buzz target link to an absorbed failure. The journey seeds the update and failure into history, keeps the update out of the head reads, and holds the thread read until the thread shows the verified failure alone. Releasing it must expand the history, leave the failure row in view and focused, reach the opened status, and drop the thread from the channel's Activity. Fails before the fix in Chromium: the history stays collapsed. publishAbsorbedFailure gains a live option because a cold load has no live stream to publish to. Signed-off-by: Matt Toohey Co-Authored-By: Claude Fable 5.1 --- src/features/messages/ThreadPanel.test.tsx | 75 +++++++++++++++ src/features/messages/ThreadPanel.tsx | 9 +- tests/browser/thread-unread.spec.mjs | 104 ++++++++++++++++++++- 3 files changed, 182 insertions(+), 6 deletions(-) diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index 1e8d6cd465..23dd45e3cc 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -2562,3 +2562,78 @@ it("reveals a public failure that its activity history absorbs by expanding the screen.getByRole("button", { name: /Collapse activity history/ }), ).toBeVisible(); }); + +// A cold exact visit verifies its target before the thread window arrives, so +// the update that absorbs the failure can land on a later snapshot of the same +// request. The expansion must wait for that group, and the reader's collapse +// of it must then hold. +it("expands the activity history that absorbs an exact failure target once its update arrives, then keeps a reader's collapse", async () => { + const h = messagesHarness(); + const agent = "d".repeat(64); + const update = { + ...row, + id: "e".repeat(64), + authorId: agent, + content: "Read the release notes.", + audience: "agents" as const, + threadRootId: row.id, + replyParentId: row.id, + createdAt: 2, + }; + const target = { + ...row, + id: "f".repeat(64), + authorId: agent, + content: "The selected model could not complete this request.", + notice: "execution-failure" as const, + threadRootId: row.id, + replyParentId: row.id, + createdAt: 3, + }; + h.snapshot.status = "loading"; + h.snapshot.replies = [target]; + h.snapshot.target = target; + h.snapshot.targetStatus = "ready"; + Object.assign(h.props, { + messageId: target.id, + retainThread: true, + revealSelected: true, + revealRequest: 1, + }); + h.render(); + expect(await screen.findByText(target.content)).toBeVisible(); + expect( + screen.queryByRole("button", { name: /Collapse activity history/ }), + ).toBeNull(); + h.snapshot.status = "ready"; + h.snapshot.replies = [update, target]; + h.render(); + expect( + await screen.findByRole("button", { name: /Collapse activity history/ }), + ).toBeVisible(); + expect(screen.getByText(target.content)).toBeVisible(); + fireEvent.click( + screen.getByRole("button", { name: /Collapse activity history/ }), + ); + expect( + screen.getByRole("button", { name: /Show activity history/ }), + ).toHaveAttribute("aria-expanded", "false"); + h.snapshot.replies = [ + update, + target, + { + ...row, + id: "a".repeat(64), + content: "A later reply", + threadRootId: row.id, + replyParentId: row.id, + createdAt: 4, + }, + ]; + h.render(); + expect(await screen.findByText("A later reply")).toBeVisible(); + expect( + screen.getByRole("button", { name: /Show activity history/ }), + ).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByText(target.content)).toBeNull(); +}); diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index d7d45fd2ca..74d2b1284e 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -724,6 +724,10 @@ function ThreadMessages({ const [expandedActivity, setExpandedActivity] = useState>( new Set(), ); + // The request whose target's group this visit expanded. A cold exact visit + // verifies its target before the window arrives, so the group that absorbs + // it may form only on a later snapshot: the request stays pending until a + // group is found, and once expanded, the reader's collapse of it is final. const activityVisit = useRef([]); useLayoutEffect(() => { const request = [messageId, navigation?.signal, revealRequest]; @@ -733,12 +737,13 @@ function ThreadMessages({ !snapshot.replies.some((row) => row.id === messageId) ) return; - activityVisit.current = request; const group = [...tree.children.keys()] .flatMap((parent) => presentLevel(parent).groups) .find((group) => group.ids.includes(messageId)); const first = group?.ids[0]; - if (first) setExpandedActivity((current) => new Set([...current, first])); + if (!first) return; + activityVisit.current = request; + setExpandedActivity((current) => new Set([...current, first])); }, [ messageId, navigation?.signal, diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index 55210d5fab..c14efe0603 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -1,7 +1,7 @@ import { pauseClock } from "./clock.mjs"; import { openChannelDetails } from "./channel-details.mjs"; import { test, expect } from "./fixture.mjs"; -import { finalizeEvent, generateSecretKey } from "nostr-tools"; +import { finalizeEvent, generateSecretKey, getPublicKey } from "nostr-tools"; import { clickRow, open, virtuaIdle } from "./timeline.mjs"; import { holdReadingFocus, @@ -515,10 +515,106 @@ test("opening an absorbed failure from Activity expands its activity history, th await expect(item).toHaveCount(0); }); +// A cold exact visit verifies the failure before it reads the thread window, +// so the thread first shows the failure alone and only then the update that +// absorbs it. The expansion must still happen once that group forms; otherwise +// the revealed failure vanishes into the collapsed history. +test("a cold link to an absorbed failure expands its activity history once the window arrives, then reads it", async ({ + page, + app, +}) => { + const key = generateSecretKey(); + app.serveProfile(key, { name: "Remote tester", is_agent: true }); + const root = app.histories + .get("primary/alpha") + .find((row) => row.content === "Thread root 0"); + const failure = publishAbsorbedFailure(app, "alpha", root.id, key, { + live: false, + }); + // The fixture's head reads carry recent replies, which would load the + // update before the thread read. Keep the update to the thread read, and + // hold that read until the thread has shown the verified failure. + const agent = getPublicKey(key); + const windowRead = Promise.withResolvers(); + const held = Promise.withResolvers(); + await page.route("**/api/relay/**/query", async (route) => { + const filters = route.request().postDataJSON() ?? []; + if ( + filters.some( + (filter) => filter.depth_limit && filter["#e"]?.includes(root.id), + ) + ) { + windowRead.resolve(); + await held.promise; + return route.continue().catch(() => {}); + } + const response = await route.fetch(); + const events = await response.json(); + const kept = Array.isArray(events) + ? events.filter( + (event) => !(event.kind === 40002 && event.pubkey === agent), + ) + : events; + await route.fulfill({ response, json: kept }).catch(() => {}); + }); + const thread = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const row = thread.locator(`[data-message-id="${failure.id}"]`); + await page.goto( + `${app.origin}/#buzz=${encodeURIComponent( + JSON.stringify({ + version: 1, + kind: "conversation", + channelId: "alpha", + messageId: failure.id, + threadRootId: root.id, + scope: { + viewer: app.viewer, + communityOrigin: "https://primary.example", + }, + }), + )}`, + ); + await windowRead.promise; + // The verified failure shows alone: no history has absorbed it yet. + await expect(row).toBeAttached(); + await expect( + thread.getByRole("button", { name: /^Collapse activity history:/ }), + ).toHaveCount(0); + held.resolve(); + await expect( + thread.getByRole("button", { name: /^Collapse activity history:/ }), + ).toBeVisible(); + await expect(row).toBeInViewport(); + await expect(row).toBeFocused(); + await expect + .poll(() => page.evaluate(() => window.fixtureNavigation.snapshot().status)) + .toBe("opened"); + await expect + .poll(() => + page.evaluate(() => + window.fixtureRelay + .snapshot() + .session.unread.activity("alpha") + .items?.map((item) => item.preview), + ), + ) + .not.toContain(failure.content); +}); + /** An agent-only update, then the same agent's public execution failure, as * direct replies to the viewer's thread root. The failure is unread; the - * thread folds it into the update's activity history. */ -function publishAbsorbedFailure(app, channel, rootId, key) { + * thread folds it into the update's activity history. Before the page loads + * there is no live stream to publish to: history alone serves a cold load. */ +function publishAbsorbedFailure( + app, + channel, + rootId, + key, + { live = true } = {}, +) { const history = app.histories.get(`primary/${channel}`); const publish = (content, tags, offset) => { const event = finalizeEvent( @@ -531,7 +627,7 @@ function publishAbsorbedFailure(app, channel, rootId, key) { key, ); history.push(event); - app.relay.publish("primary", event); + if (live) app.relay.publish("primary", event); return event; }; publish( From 8e7597229a1a2c2a77ed9bec935d0a436bbcde90 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 9 Nov 2026 22:04:50 +1100 Subject: [PATCH 6/7] test(messages): release the held thread read in finally Review of 9586384db found that the cold absorbed-failure journey in thread-unread.spec.mjs resolved its held thread read only after the navigation and the two pre-release assertions. If any of them threw, the route callback stayed suspended on the gate. The deterministic-test rule requires held operations to be released in finally. The navigation and the assertions that run while the read is held now sit in a try block whose finally resolves the gate. The post-release recovery assertions follow the block unchanged. The other held gates in this PR (the held head read in message-navigation.spec.mjs and the held reads elsewhere in thread-unread.spec.mjs) already release in finally. Checks: node --check and biome check on the file. Tests not run on this machine. Signed-off-by: Matt Toohey Co-Authored-By: Claude Fable 5.1 --- tests/browser/thread-unread.spec.mjs | 47 +++++++++++++++------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index c14efe0603..c7a0fc1832 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -562,28 +562,31 @@ test("a cold link to an absorbed failure expands its activity history once the w exact: true, }); const row = thread.locator(`[data-message-id="${failure.id}"]`); - await page.goto( - `${app.origin}/#buzz=${encodeURIComponent( - JSON.stringify({ - version: 1, - kind: "conversation", - channelId: "alpha", - messageId: failure.id, - threadRootId: root.id, - scope: { - viewer: app.viewer, - communityOrigin: "https://primary.example", - }, - }), - )}`, - ); - await windowRead.promise; - // The verified failure shows alone: no history has absorbed it yet. - await expect(row).toBeAttached(); - await expect( - thread.getByRole("button", { name: /^Collapse activity history:/ }), - ).toHaveCount(0); - held.resolve(); + try { + await page.goto( + `${app.origin}/#buzz=${encodeURIComponent( + JSON.stringify({ + version: 1, + kind: "conversation", + channelId: "alpha", + messageId: failure.id, + threadRootId: root.id, + scope: { + viewer: app.viewer, + communityOrigin: "https://primary.example", + }, + }), + )}`, + ); + await windowRead.promise; + // The verified failure shows alone: no history has absorbed it yet. + await expect(row).toBeAttached(); + await expect( + thread.getByRole("button", { name: /^Collapse activity history:/ }), + ).toHaveCount(0); + } finally { + held.resolve(); + } await expect( thread.getByRole("button", { name: /^Collapse activity history:/ }), ).toBeVisible(); From 70b93469962cf7c524db3bae87fef4ed433efa7f Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 13 Nov 2026 17:33:35 +1100 Subject: [PATCH 7/7] fix(messages): keep an exact target's activity history open when late evidence regroups it Review of 8e7597229 found that the visit effect disclosed its target's activity history by the group's first row, once per request. Later evidence can regroup the level without the reader collapsing anything. The reported ordering is real. An agent's normal send signs the request it answers and links its reply to the thread root. Suppose its agent-only update claims a human request nested under an older sibling, and that request has not loaded. Then one history holds that update, a second agent's update and the second agent's public execution failure, and the visit expands it by the first update. When the request loads, the signed claim moves the first update to a history of its own. The failure's history now starts at the second update and is collapsed, so the failure unmounts. A reveal still waiting for the row cannot complete. A completed visit loses its focused row with nothing mounted to take the focus. Each claim in the comment was checked against the source and none was wrong. The new journey reproduces the split in the running app. The visit now discloses its target rather than a group. ThreadMessages records each request's target, and a history is open while it holds a disclosed target or the reader opened it. The disclosure therefore follows the target through a split, through a group that forms on a later snapshot, and through a first row that changes. The reader's collapse of a history removes its targets, and holds until the next request. The comment suggested tracking the reader's collapse separately; keying the disclosure by target gives the same result with less state. The visit no longer groups rows, so it needs neither presentLevel nor the per-request ref. The render loop calls activityPresentation inline again and the loadedRows memo is back where main declares it. That removes the conflict resolution from the last rebase. Histories the reader opens are still keyed by first row, as on main. Tests: - ThreadPanel.test: three cases share one fixture for the ordering above. The request loads while the reveal is held with its row off screen, and after the visit completed. In both, the failure stays mounted and visible in the history that now holds it and keeps focus through the move. The held reveal then completes the visit as opened once the row shows. Both fail before the fix: the failure is gone. The third case collapses the history first; both histories then stay closed after the split. It passes before and after the fix. - thread-unread.spec.mjs: new journey. Two agents and a requester publish the ordering through the fixture relay, and the failure is opened from the channel's Activity. The late request arrives live and splits the history; the failure row stays in view and focused. It fails before the fix in Chromium: the row is not found. This covers the signed request tag reaching the thread's grouping, which the mounted cases supply directly. publishAbsorbedFailure now shares a publishReply helper with the journey. Checks run on this machine: - tsc --noEmit: passed. - biome check on the three touched files: passed. - ThreadPanel.test.tsx alone: 96 passed. - The new journey alone: passed once in Chromium and once in WebKit. Not run: the rest of the unit suite, every other browser journey including the other absorbed-failure journeys in thread-unread.spec.mjs, and the push hooks. Signed-off-by: Matt Toohey Co-Authored-By: Claude Fable 5.2 --- src/features/messages/ThreadPanel.test.tsx | 168 +++++++++++++++++++++ src/features/messages/ThreadPanel.tsx | 108 ++++++------- tests/browser/thread-unread.spec.mjs | 123 +++++++++++++-- 3 files changed, 324 insertions(+), 75 deletions(-) diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index 23dd45e3cc..041ebf33c5 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -2637,3 +2637,171 @@ it("expands the activity history that absorbs an exact failure target once its u ).toHaveAttribute("aria-expanded", "false"); expect(screen.queryByText(target.content)).toBeNull(); }); + +/** A normal agent send signs its request but flattens its reply link to the + * thread root. The first agent's update claims a request nested under an older + * sibling; a second agent's update and public failure answer the root. Until + * the request loads, one history holds all three. Once it does, the claim + * moves the first update to its own group, and the failure's history starts + * at the second update. */ +function splitHistoryFixture() { + const [firstAgent, secondAgent] = ["d".repeat(64), "e".repeat(64)]; + const reply = (id: string, fields: Partial) => ({ + ...row, + id: id.repeat(64), + threadRootId: row.id, + replyParentId: row.id, + ...fields, + }); + const sibling = reply("1", { content: "An older sibling", createdAt: 2 }); + const request = reply("2", { + content: "A nested request", + replyParentId: sibling.id, + createdAt: 3, + }); + const claimed = reply("3", { + authorId: firstAgent, + content: "Checked the headings.", + audience: "agents", + request: request.id, + createdAt: 4, + }); + const update = reply("4", { + authorId: secondAgent, + content: "Read the release notes.", + audience: "agents", + createdAt: 5, + }); + const target = reply("5", { + authorId: secondAgent, + content: "The selected model could not complete this request.", + notice: "execution-failure", + createdAt: 6, + }); + return { + target, + before: [sibling, claimed, update, target], + after: [sibling, request, claimed, update, target], + }; +} + +// The request can load while the reveal still waits for its row to show, or +// after the visit completed with the row focused. Either way the reader +// collapsed nothing, so the failure stays mounted in whichever history now +// holds it, keeps focus through the move, and a pending visit still completes. +it.each([ + ["before the reveal completes", false], + ["after the reveal completes", true], +])( + "keeps an exact failure target shown and focused when a late request splits its activity history %s", + async (_, completed) => { + const { target, before, after } = splitHistoryFixture(); + const navigation = visitPlaces().linked( + "thread-visit", + undefined, + target.id, + ); + const h = messagesHarness(navigation); + h.props.messageId = target.id; + h.snapshot.replies = before; + h.snapshot.target = target; + h.snapshot.targetStatus = "ready"; + // A reveal completes only once its row is on screen. Hold the row off + // screen, and record the rows the reveal then waits to see arrive. + let onScreen = completed; + vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation( + () => new DOMRect(0, onScreen ? 0 : 2000, 500, 500), + ); + const awaited = new Map(); + vi.stubGlobal( + "IntersectionObserver", + class { + constructor(private callback: IntersectionObserverCallback) {} + observe(row: Element) { + awaited.set(this.callback, row); + } + disconnect() { + awaited.delete(this.callback); + } + }, + ); + const shown = () => + h.element.querySelector(`[data-message-id="${target.id}"]`); + const opened = () => + expect(navigation.complete).toHaveBeenCalledExactlyOnceWith({ + status: "opened", + }); + // The reveal verified the row it focused and found it off screen. + const held = () => + waitFor(() => expect([...awaited.values()]).toEqual([shown()])); + h.render(); + if (completed) await waitFor(opened); + else await held(); + const first = shown(); + expect(first).toHaveFocus(); + expect( + screen.getAllByRole("button", { name: /activity history/ }), + ).toHaveLength(1); + + h.snapshot.replies = after; + h.render(); + // The failure moved to the history that starts at the second update. + expect( + screen.getByRole("button", { name: /Show activity history/ }), + ).toHaveAttribute("aria-expanded", "false"); + expect( + screen.getByRole("button", { name: /Collapse activity history/ }), + ).toHaveAttribute("aria-expanded", "true"); + expect(shown()).not.toBe(first); + expect(screen.getByText(target.content)).toBeVisible(); + expect(shown()).toHaveFocus(); + if (completed) return opened(); + + // The reveal follows the row to its new place and still waits for it. + await held(); + expect(navigation.complete).not.toHaveBeenCalled(); + onScreen = true; + for (const arrive of [...awaited.keys()]) + arrive( + [ + { + intersectionRect: { width: 500, height: 500 }, + } as IntersectionObserverEntry, + ], + {} as IntersectionObserver, + ); + await waitFor(opened); + expect(shown()).toHaveFocus(); + expect(screen.getByText(target.content)).toBeVisible(); + }, +); + +it("keeps a reader's collapse of an exact failure target's activity history when a late request splits it", async () => { + const { target, before, after } = splitHistoryFixture(); + const h = messagesHarness(); + h.snapshot.replies = before; + h.snapshot.target = target; + h.snapshot.targetStatus = "ready"; + Object.assign(h.props, { + messageId: target.id, + retainThread: true, + revealSelected: true, + revealRequest: 1, + }); + h.render(); + fireEvent.click( + await screen.findByRole("button", { name: /Collapse activity history/ }), + ); + expect(screen.queryByText(target.content)).toBeNull(); + h.snapshot.replies = after; + h.render(); + // The request rendered under its sibling, and both histories it split the + // collapsed one into stay closed. + expect(screen.getByRole("button", { name: /^View 1 reply/ })).toBeVisible(); + expect( + screen + .getAllByRole("button", { name: /activity history/ }) + .map((button) => button.getAttribute("aria-expanded")), + ).toEqual(["false", "false"]); + expect(screen.queryByText(target.content)).toBeNull(); +}); diff --git a/src/features/messages/ThreadPanel.tsx b/src/features/messages/ThreadPanel.tsx index 74d2b1284e..7444372fa1 100644 --- a/src/features/messages/ThreadPanel.tsx +++ b/src/features/messages/ThreadPanel.tsx @@ -657,6 +657,27 @@ function ThreadMessages({ [session.unread, channelId, snapshot.root?.id, messageId], ); useSyncExternalStore(subscribeUnread, unreadSnapshot, unreadSnapshot); + // Histories the reader opened, by their first row. + const [expandedActivity, setExpandedActivity] = useState>( + new Set(), + ); + // Exact targets whose activity history a visit opened. The disclosure + // follows the target, not the group that held it when the visit began: + // later evidence regroups a level while the reader collapses nothing. A cold + // visit's window forms the group that absorbs a failure, and a late request + // splits it. The reader's collapse drops the target until the next request. + const [disclosedTargets, setDisclosedTargets] = useState>( + new Set(), + ); + // biome-ignore lint/correctness/useExhaustiveDependencies: each request discloses its target once, so a reader's collapse holds. + useLayoutEffect(() => { + setDisclosedTargets((current) => + current.has(messageId) ? current : new Set([...current, messageId]), + ); + }, [messageId, navigation?.signal, revealRequest]); + const activityExpanded = (ids: readonly string[]) => + expandedActivity.has(ids[0] ?? "") || + ids.some((id) => disclosedTargets.has(id)); const [expanded, setExpanded] = useState>(new Set()); const [replyParent, setReplyParent] = useState(); const resolveName = useChannelIdentityNames(session, channelId); @@ -698,60 +719,6 @@ function ThreadMessages({ }, [session.profiles, authors, profileGeneration]); const profiles = useRowProfiles(session.profiles, rows); const agentPubkeys = useKnownAgentPubkeys(session, profiles); - // Each displayed level is projected alone, but a signed claim may name a - // request loaded at another level. - const loadedRows = useMemo( - () => - snapshot.root ? [snapshot.root, ...snapshot.replies] : snapshot.replies, - [snapshot.root, snapshot.replies], - ); - // One displayed level's activity grouping, shared by rendering and by the - // visit that must expand the group hiding its target: a public failure an - // activity run absorbs is hidden with that run. - const presentLevel = useCallback( - (parent: string | undefined) => - activityPresentation( - tree.children.get(parent) ?? [], - activitySnapshot, - agentPubkeys, - parent ?? snapshot.root?.id, - // A failure with replies needs its own visible branch controls. - (failure) => !tree.children.get(failure.id)?.length, - loadedRows, - ), - [tree, activitySnapshot, agentPubkeys, snapshot.root?.id, loadedRows], - ); - const [expandedActivity, setExpandedActivity] = useState>( - new Set(), - ); - // The request whose target's group this visit expanded. A cold exact visit - // verifies its target before the window arrives, so the group that absorbs - // it may form only on a later snapshot: the request stays pending until a - // group is found, and once expanded, the reader's collapse of it is final. - const activityVisit = useRef([]); - useLayoutEffect(() => { - const request = [messageId, navigation?.signal, revealRequest]; - if ( - !messageId || - request.every((part, index) => part === activityVisit.current[index]) || - !snapshot.replies.some((row) => row.id === messageId) - ) - return; - const group = [...tree.children.keys()] - .flatMap((parent) => presentLevel(parent).groups) - .find((group) => group.ids.includes(messageId)); - const first = group?.ids[0]; - if (!first) return; - activityVisit.current = request; - setExpandedActivity((current) => new Set([...current, first])); - }, [ - messageId, - navigation?.signal, - revealRequest, - snapshot.replies, - tree, - presentLevel, - ]); const scroller = useRef(null); const positioned = useRef(false); const shown = useRef(active); @@ -1095,6 +1062,13 @@ function ThreadMessages({ const rootActivity = rootId ? rootPresentation.byRow.get(rootId) : undefined; // A stopped note may follow only a reply that is always shown: collapsed // branches do not mount their rows. + // Each displayed level is projected alone, but a signed claim may name a + // request loaded at another level. + const loadedRows = useMemo( + () => + snapshot.root ? [snapshot.root, ...snapshot.replies] : snapshot.replies, + [snapshot.root, snapshot.replies], + ); const rootReplies = useMemo( () => activityPresentation( @@ -1516,7 +1490,15 @@ function ThreadMessages({ ); if (parent === undefined && snapshot.root) requestRows.set(snapshot.root.id, snapshot.root); - const presentation = presentLevel(parent); + const presentation = activityPresentation( + chronologicalSiblings, + activitySnapshot, + agentPubkeys, + parent ?? rootId, + // A failure with replies needs its own visible branch controls. + (failure) => !tree.children.get(failure.id)?.length, + loadedRows, + ); const groups = presentation.groups.filter((group) => group.ids.every((id) => !tree.children.get(id)?.length), ); @@ -1524,9 +1506,7 @@ function ThreadMessages({ const groupsByFirst = new Map(groups.map((group) => [group.ids[0], group])); const groupedIds = new Set(groups.flatMap((group) => group.ids)); const hidden = new Set( - groups.flatMap((group) => - expandedActivity.has(group.ids[0] ?? "") ? [] : group.ids, - ), + groups.flatMap((group) => (activityExpanded(group.ids) ? [] : group.ids)), ); return siblings.map((row, index) => { const group = groupsByFirst.get(row.id); @@ -1662,17 +1642,25 @@ function ThreadMessages({ profiles={profiles} media={session.media} layout="thread" - expanded={expandedActivity.has(group.ids[0] ?? "")} + expanded={activityExpanded(group.ids)} onToggle={() => { follow.current = false; targetAnchor.current = undefined; + const open = activityExpanded(group.ids); setExpandedActivity((current) => { const next = new Set(current); const first = group.ids[0] ?? row.id; - if (next.has(first)) next.delete(first); + if (open) next.delete(first); else next.add(first); return next; }); + if (open) + setDisclosedTargets( + (current) => + new Set( + [...current].filter((id) => !group.ids.includes(id)), + ), + ); }} /> diff --git a/tests/browser/thread-unread.spec.mjs b/tests/browser/thread-unread.spec.mjs index c7a0fc1832..7dfa0d4825 100644 --- a/tests/browser/thread-unread.spec.mjs +++ b/tests/browser/thread-unread.spec.mjs @@ -607,6 +607,112 @@ test("a cold link to an absorbed failure expands its activity history once the w .not.toContain(failure.content); }); +// A normal agent send signs the request it answers but links its reply to the +// thread root. The first agent's update claims a request nested under another +// reply, which the thread has not received yet, so one history holds that +// update with the second agent's update and failure. When the request +// arrives, the claim moves the first update to a history of its own. The +// reader collapsed nothing, so the opened failure must stay shown and focused +// in the history that now starts at the second agent's update. +test("a late request that splits an opened failure's activity history leaves the failure shown and focused", async ({ + page, + app, +}) => { + const [first, second, requester] = [0, 1, 2].map(() => generateSecretKey()); + app.serveProfile(first, { name: "First agent", is_agent: true }); + app.serveProfile(second, { name: "Remote tester", is_agent: true }); + app.serveProfile(requester, { name: "Requester" }); + await holdReadingFocus(page); + await open(page, app); + const history = app.histories.get("primary/alpha"); + const root = history.find((row) => row.content === "Thread root 0"); + const sibling = history.find((row) => row.content === "Broadcast reply"); + const request = finalizeEvent( + { + kind: 9, + content: "Check the headings too.", + tags: [ + ["h", "alpha"], + ["e", root.id, "", "root"], + ["e", sibling.id, "", "reply"], + ], + created_at: Math.floor(Date.now() / 1000) - 2, + }, + requester, + ); + publishReply( + app, + "alpha", + root.id, + first, + "Checked the headings.", + [ + ["audience", "agents"], + ["buzz:request", request.id], + ], + -1, + ); + const failure = publishAbsorbedFailure(app, "alpha", root.id, second); + const alpha = page.locator('button[data-channel-id="alpha"]'); + await alpha.hover(); + const item = page + .getByRole("dialog", { name: "Activity in Alpha" }) + .getByRole("button", { name: /Open unread thread from/ }) + .filter({ hasText: failure.content }); + await expect(item).toBeVisible(); + await releaseReadingFocus(page); + await item.click(); + const thread = page.getByRole("complementary", { + name: "Thread", + exact: true, + }); + const row = thread.locator(`[data-message-id="${failure.id}"]`); + const histories = (state) => + thread.getByRole("button", { + name: new RegExp(`^${state} activity history:`), + }); + await expect(row).toBeFocused(); + await expect + .poll(() => page.evaluate(() => window.fixtureNavigation.snapshot().status)) + .toBe("opened"); + // One history holds both agents' updates and the failure. + await expect(histories("Collapse")).toHaveCount(1); + await expect(histories("Show")).toHaveCount(0); + history.push(request); + app.relay.publish("primary", request); + // The first agent's update left for its own, collapsed history. + await expect(histories("Show")).toHaveCount(1); + await expect(histories("Collapse")).toHaveCount(1); + await expect(row).toBeInViewport(); + await expect(row).toBeFocused(); +}); + +/** One reply to a thread root, in history and, once the page has a live + * stream, published to it. Tags with an audience make it an agent-only update. */ +function publishReply( + app, + channel, + rootId, + key, + content, + tags, + offset, + live = true, +) { + const event = finalizeEvent( + { + kind: tags.some(([name]) => name === "audience") ? 40002 : 9, + content, + tags: [["h", channel], ["e", rootId, "", "reply"], ...tags], + created_at: Math.floor(Date.now() / 1000) + offset, + }, + key, + ); + app.histories.get(`primary/${channel}`).push(event); + if (live) app.relay.publish("primary", event); + return event; +} + /** An agent-only update, then the same agent's public execution failure, as * direct replies to the viewer's thread root. The failure is unread; the * thread folds it into the update's activity history. Before the page loads @@ -618,21 +724,8 @@ function publishAbsorbedFailure( key, { live = true } = {}, ) { - const history = app.histories.get(`primary/${channel}`); - const publish = (content, tags, offset) => { - const event = finalizeEvent( - { - kind: tags.some(([name]) => name === "audience") ? 40002 : 9, - content, - tags: [["h", channel], ["e", rootId, "", "reply"], ...tags], - created_at: Math.floor(Date.now() / 1000) + offset, - }, - key, - ); - history.push(event); - if (live) app.relay.publish("primary", event); - return event; - }; + const publish = (content, tags, offset) => + publishReply(app, channel, rootId, key, content, tags, offset, live); publish( "Read the release notes and checked the headings.", [["audience", "agents"]],