fix(threads): read an already-open thread when Activity opens it

src/bundled/channels/ChannelActivityPopover.tsx

@@ -201,6 +201,10 @@ export function ChannelActivityPopover({
       NonNullable<ComponentProps<typeof PopoverRoot>["actionsRef"]>["current"]
     >(null);
   const pressedOpen = useRef(false);
+  // An opened thread takes focus itself. Base UI returns focus to the trigger
+  // in a microtask after closing, which can land after, and steal it from, the
+  // thread.
+  const handoff = useRef(false);
   // Hover opens a transient preview. Selecting its row may still take focus.
   const [preview, setPreview] = useState(false);
   const [now, setNow] = useState(() => Date.now());
@@ -275,7 +279,10 @@ export function ChannelActivityPopover({
         pressedOpen.current = next && details.reason === "trigger-press";
         setPreview(next && details.reason === "trigger-hover");
         setOpen(next);
-        if (next) setNow(Date.now());
+        if (next) {
+          handoff.current = false;
+          setNow(Date.now());
+        }
         if (next && items.length)
           void session.profiles
             .ensure(
@@ -310,6 +317,7 @@ export function ChannelActivityPopover({
         size="wide"
         padding="list"
         aria-label={`Activity in ${channelName}`}
+        finalFocus={() => !handoff.current}
       >
         {open && (
           <div className={activityStyles.activityList}>
@@ -366,6 +374,7 @@ export function ChannelActivityPopover({
                     session={session}
                     agents={activeAgents}
                     onOpen={(selected) => {
+                      handoff.current = true;
                       close();
                       onOpenThread(selected);
                     }}

src/features/messages/ThreadPanel.tsx

@@ -752,6 +752,34 @@ function ThreadMessages({
   const rootTarget =
     navigation?.target.kind === "conversation" &&
     navigation.target.threadRootId === messageId;
+  // Opening a thread that is already shown selects nothing new, so no tab
+  // hands focus to it. Without focus inside, reading never starts. A first
+  // opening (not yet positioned) is focused by its tab and must be left alone.
+  // The signal identifies one request; the navigation object is re-bound per
+  // relay snapshot, so a re-render of the same request must not refocus.
+  const handledRequest = useRef(navigation?.signal);
+  // biome-ignore lint/correctness/useExhaustiveDependencies: only a new request moves focus; the others merely qualify it.
+  useLayoutEffect(() => {
+    if (handledRequest.current === navigation?.signal) return;
+    handledRequest.current = navigation?.signal;
+    const element = scroller.current;
+    if (
+      !navigation ||
+      navigation.signal.aborted ||
+      navigation.focus === "preserve" ||
+      !rootTarget ||
+      !active ||
+      !positioned.current ||
+      !element
+    )
+      return;
+    // Only focus the opener left behind (its control unmounted) moves in.
+    // Focus the person has since placed anywhere, even a moment after a first
+    // opening whose request arrived late, is not stolen.
+    const focused = document.activeElement;
+    if (!focused || focused === document.body)
+      element.focus({ preventScroll: true });
+  }, [navigation?.signal]);
   // Ordinary opens follow the latest reply after bounded history finishes.
   // Keep that first positioning invisible; exact-message navigation reveals itself.
   const positioning =

tests/browser/thread-unread.spec.mjs

@@ -47,6 +47,161 @@ test.describe("mentioned reply priority", () => {
       new Set(["Broadcast descendant", "Unread reply 1"]),
     );
   });
+
+  test("opening an already-open thread from Activity reads it", async ({
+    page,
+    app,
+  }) => {
+    // Reading needs a 300ms dwell (use-reading.ts); hold the clock so the
+    // thread cannot earn it while it opens.
+    await page.clock.install();
+    await holdReadingFocus(page);
+    await open(page, app);
+    const root = app.histories
+      .get("primary/alpha")
+      .find((row) => row.content === "Thread root 1");
+    const alpha = page.locator('button[data-channel-id="alpha"]');
+    const panel = page.getByRole("complementary", {
+      name: "Thread",
+      exact: true,
+    });
+    const unread = () =>
+      page.evaluate(
+        (rootId) =>
+          window.fixtureRelay.snapshot().session.unread.snapshot({
+            kind: "thread",
+            channelId: "alpha",
+            rootId,
+          }),
+        root.id,
+      );
+    await page.clock.pauseAt((await page.evaluate(() => Date.now())) + 1000);
+    await page
+      .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`)
+      .getByRole("button", { name: /^View thread:/ })
+      .click();
+    await expect(panel).toBeVisible();
+    // The reader is looking elsewhere: the open thread stays unread.
+    await page.evaluate(() => document.activeElement?.blur());
+    await page.clock.resume();
+    expect((await unread()).observedCount).toBeGreaterThan(0);
+    await alpha.hover();
+    const item = page
+      .getByRole("dialog", { name: "Activity in Alpha" })
+      .getByRole("button", { name: /Open unread thread from/ })
+      .filter({ hasText: "Unread reply 1" });
+    await expect(item).toBeVisible();
+    await releaseReadingFocus(page);
+    const previousAttempt = await page.evaluate(
+      () => window.fixtureNavigation.snapshot().attempt.id,
+    );
+    // Activity opens the thread that is already selected.
+    await item.click();
+    // The thread is already on screen, so its visibility proves nothing. Wait
+    // for the click's own navigation, so a failure below means the thread was
+    // opened and still not read, never that the click missed.
+    await expect
+      .poll(() =>
+        page.evaluate((previous) => {
+          const { status, attempt } = window.fixtureNavigation.snapshot();
+          return { status, freshAttempt: attempt.id !== previous };
+        }, previousAttempt),
+      )
+      .toEqual({ status: "opened", freshAttempt: true });
+    try {
+      await expect.poll(async () => (await unread()).observedCount).toBe(0);
+    } catch (error) {
+      // Reading needs focus inside the thread; say where it actually was.
+      const focus = await page.evaluate(() => ({
+        active:
+          document.activeElement?.getAttribute("aria-label") ??
+          document.activeElement?.tagName,
+        inThread: !!document.activeElement?.closest("[data-reading-surface]"),
+        windowFocused: document.hasFocus(),
+      }));
+      throw new Error(
+        `The thread opened but stayed unread. Focus: ${JSON.stringify(focus)}`,
+        { cause: error },
+      );
+    }
+  });
+
+  test("an Activity reopening leaves focus the person moved meanwhile", async ({
+    page,
+    app,
+  }) => {
+    // As above, hold the clock so the thread cannot be read while it opens.
+    await page.clock.install();
+    await holdReadingFocus(page);
+    await open(page, app);
+    const root = app.histories
+      .get("primary/alpha")
+      .find((row) => row.content === "Thread root 1");
+    const panel = page.getByRole("complementary", {
+      name: "Thread",
+      exact: true,
+    });
+    const composer = page.getByRole("textbox", {
+      name: "Message #Alpha",
+      exact: true,
+    });
+    await page.clock.pauseAt((await page.evaluate(() => Date.now())) + 1000);
+    await page
+      .locator(`[data-channel-timeline] [data-message-id="${root.id}"]`)
+      .getByRole("button", { name: /^View thread:/ })
+      .click();
+    await expect(panel).toBeVisible();
+    await page.evaluate(() => document.activeElement?.blur());
+    await page.clock.resume();
+    await page.locator('button[data-channel-id="alpha"]').hover();
+    const item = page
+      .getByRole("dialog", { name: "Activity in Alpha" })
+      .getByRole("button", { name: /Open unread thread from/ })
+      .filter({ hasText: "Unread reply 1" });
+    await expect(item).toBeVisible();
+    await releaseReadingFocus(page);
+    const previousAttempt = await page.evaluate(
+      () => window.fixtureNavigation.snapshot().attempt.id,
+    );
+    // Hold the reopening's reads so the person can move focus before the
+    // thread receives its request.
+    let held = false;
+    let release;
+    let sawRead;
+    const gate = new Promise((resolve) => {
+      release = resolve;
+    });
+    const read = new Promise((resolve) => {
+      sawRead = resolve;
+    });
+    await page.route("**/api/relay/**/query", async (route) => {
+      if (held) {
+        sawRead();
+        await gate;
+      }
+      await route.continue().catch(() => {});
+    });
+    held = true;
+    try {
+      await item.click();
+      await read;
+      await composer.click();
+      await page.keyboard.type("Still typing");
+    } finally {
+      held = false;
+      release();
+    }
+    await expect
+      .poll(() =>
+        page.evaluate((previous) => {
+          const { status, attempt } = window.fixtureNavigation.snapshot();
+          return { status, freshAttempt: attempt.id !== previous };
+        }, previousAttempt),
+      )
+      .toEqual({ status: "opened", freshAttempt: true });
+    await expect(composer).toBeFocused();
+    await expect(composer).toHaveText("Still typing");
+  });
 });
 
 test("thread buttons show observed unread independently, clear only after reading, and expose hover/focus affordance", async ({