From 832737efaed9b9b6d066777fc0d66287b72d7426 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Fri, 30 Oct 2026 15:18:15 +1100 Subject: [PATCH 1/2] test(desktop): stop four E2E specs racing scroll settles and focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Units 2-5 of the 23-30 Oct desktop Playwright flake audit. All four are test-only. origin/main (1eb67bb52, now including #8680) hasn't touched these specs since fae3f0346. Of the open PRs, #8622 adds an unrelated test at the end of navigation.spec, and #7812 (stale since 10-06) adds tests elsewhere in three of them. #8680 didn't touch them, but it carried navigation:472 mode A's product half (eb88a9814), which is not part of this commit. The code confirms each cause, so none is dropped. navigation.spec.ts:472, "composer Buzz chip labels wrap without orphaning their icons" Runs: 11 jobs in the window, 6 after the earlier fixes, and 4 of the 7 main runs that carry every fix. Run 40970053997 (Smoke shard 7) failed all three attempts. Mode A: jobs 122971493553, 123066839967, 123572467962, 123819180597 and retry 2 of 123784825042. Signature: fragmentStep is null at :634. Cause: rows key on renderKey = localKey ?? id. On main, reconcileChannelWindowMessages keeps only pending sends and thread replies from the cache, so a head refetch or live echo after the ack drops the localKey. The key flips and the row remounts, losing the 220 px width the test set in an earlier evaluate. Change: re-land the test half of closed PR #6852, which never merged. Set the width and measure in one expect.poll, and set the width again before each fragment measurement. Mode B: jobs 123573056202, 123784199542, 123784825042 (twice), 123804428893, 123818728963, 123819543461, and 122924164125 (the Infinity variant). Signature: the |tooltipCenter - cursorX| < 1 poll at :667 receives 81.35 or Infinity. In failing attempt 40970053997 the post-send settle (scrollTop 200, 296, 260, 302) took its last step 21 ms or less before focus, and the tooltip read closed 12 ms after it. In the passing retry 40980513346, scrolling had stopped at least 15 ms before focus. Cause: Radix Tooltip 1.2.16 closes when an ancestor of its trigger scrolls (a capture-phase window listener). Reopening by pointer needs a pointermove while hasPointerMoveOpenedRef is false, then the chip provider's 500 ms delay. Any close clears that timer, and only pointerleave resets the flag. useInlineTooltipPosition moves the content only on pointermove and placement. So a scroll after focus can leave the tooltip closed, or open without the fragment offset, and the spec's single mouse.move never recovers it. Change: before focusing the chip, poll until the timeline sits at its floor (scrollHeight - clientHeight - scrollTop <= 1) at the start of one poll, is still there two animation frames later, and its scrollTop hasn't moved. Waiting only for scrollTop to hold for two frames isn't enough: a timeline whose settle hasn't started yet reads as still, and the failing run's last step landed under two frames before focus. The settle's end state is the floor. useVirtualizedBottomSettle scrolls only when Virtua's ResizeObserver reports a layout change, and after the send and the 220 px wrap the timeline sits short of the floor until the settle catches up. Focusing the chip and moving the pointer over it don't change layout. The send's ack does. It removes the row's "Sending…" status, and the confirmed row can lose its header, since a pending row is never shown as a continuation (MessageRow.tsx:167). The send's bottom intent is still armed, so that re-pins the timeline. So before the floor poll, wait for the sent row's message-send-status to reach count 0. If the ack has already landed, that wait is a no-op. The standalone toBeVisible after focus is gone. positionOverFragment asserts data-state matches /open/ instead, because a closing tooltip stays visible through its exit animation. No retry. Wrapping the measure, move and check in toPass, leaving and re-entering the chip on each attempt, would also pass, but it retries around the race instead of removing it. The ack and floor waits close the scroll window. The only other way to scroll under the tooltip was a remount that drops the row's 220 px width (mode A), and #8680's retainAcknowledgedLocalKeys, on this branch's base (1eb67bb52), keeps the row's key. So positionOverFragment is a single pass, as on main, plus mode A's width re-set: set the width, measure, move, assert open, then poll the centre with the default timeouts. huddle-transcription.spec.ts:637, "animates the responding agent with the shared speaker ring" Runs: 9 jobs, always Smoke shard 4 attempt 1. Main: 39896948198, 40718743531, 40732061485, 40840949170, 40906253761, 40969965981 and 40983029157. PRs: #8715, #8666. Signature: toBeFocused() at :697 reads inactive for all 14 polls over 5 s. At +3.0 s in the failure video (40983029157, job 123827818084) the caret is in "Message #huddle". Cause: useComposerAutofocus re-runs when `focus` changes. That's focusEnd, which depends on the editor, so it runs again once the editor mounts. It backs off only for input, textarea, select and contentEditable focus, so it takes focus from the focused trigger. Change: wait for message-input to be focused before focusing the trigger. The same spec already does this at :855. scroll-history.spec.ts:1289, "fast middle-page scroll settles with continuous mounted coverage" Runs: 9 jobs in the window. After the fixes: main 40980404335 (Smoke shard 9, job 123819180518), two attempts on #8720, and the brain/native-account-setup branch. Signature: the viewportCoverage poll expects < 100 and receives 200. The failure screenshot shows the timeline at the floor. The 200 px is the bottom spacer (--composer-overlay-height) under the last row. Cause: TimelineMessageList arms useVirtualizedBottomSettle when the channel first positions. Its ResizeObserver then pins to the floor until listenForReaderScroll input or an accepted target cancels it. handleScroll deliberately doesn't cancel, and a prepend delta doesn't settle, so the intent survives the prepend and the spec's scrollTop writes. Virtua's re-measure after the jump pulls the list back down. Change: before the fast scroll, hover the timeline and wheel up 120 px. Then poll until it is more than 32 px from the floor (the timeline's at-bottom band) and scrollTop holds for two frames, so the writes stay the last scroll. custom-emoji.spec.ts:370, "reacting with a custom emoji renders via the loopback media proxy" Runs: main 40787609859 (10-28), main 40983029157 (Smoke shard 3, a run with every fix), and #8241 (agent and auth files only). Signature: the poll at :453 expects the inline "Add reaction" opacity "0" and receives "1". The error context marks button "Open reactions" [active]. Cause: the non-modal Radix Popover returns focus to its trigger when the picker closes. FocusScope does that in a setTimeout(0) after the content unmounts, and the content unmounts after its 100 ms exit animation. That can land after the spec's blur. The action bar is hidden by opacity only, so the trigger takes focus, and group-focus-within/message keeps the button visible. Change: wait for the "Open reactions" trigger to be focused, then blur. The note suggested waiting for em-emoji-picker to reach count 0. That still leaves the setTimeout window open. Focus reaches the trigger only after unmount, so this wait covers both. Verification Not run locally: the Flake Check lane will repeat the touched specs. pnpm typecheck and pnpm check pass. biome's 3 warnings are in untouched files. An ad hoc tsc ran over the four specs, using the desktop tsconfig with only those files, and over HEAD's copies of them. Both gave the same error counts: navigation 7, huddle 17, scroll-history 0, custom-emoji 2. The messages are identical once line numbers are stripped, and all are pre-existing: no Node types, findLast against the ES2022 lib, and the leadingBounds narrowing at navigation:576. With the floor wait in place and the retry dropped, navigation still gives the same 7. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- desktop/tests/e2e/custom-emoji.spec.ts | 8 +- .../tests/e2e/huddle-transcription.spec.ts | 3 + desktop/tests/e2e/navigation.spec.ts | 109 ++++++++++++------ desktop/tests/e2e/scroll-history.spec.ts | 27 +++++ 4 files changed, 108 insertions(+), 39 deletions(-) diff --git a/desktop/tests/e2e/custom-emoji.spec.ts b/desktop/tests/e2e/custom-emoji.spec.ts index 23dbb5e5f4e..9e95a8dc244 100644 --- a/desktop/tests/e2e/custom-emoji.spec.ts +++ b/desktop/tests/e2e/custom-emoji.spec.ts @@ -435,9 +435,11 @@ test("reacting with a custom emoji renders via the loopback media proxy", async ); const inlineAddReactionButton = row.getByLabel("Add reaction"); - // The picker closes with the pointer/focus position depending on animation - // timing. Put the row into a deterministic idle state before checking the - // pill's pre-existing hidden behavior. + // Radix returns focus to "Open reactions" a task after the picker's exit + // animation unmounts it, which keeps the row focus-within. Wait for that + // before blurring, then put the row into a deterministic idle state before + // checking the pill's pre-existing hidden behavior. + await expect(messageReactionTrigger(row)).toBeFocused(); await page.mouse.move(0, 0); await page.evaluate(() => { if (document.activeElement instanceof HTMLElement) { diff --git a/desktop/tests/e2e/huddle-transcription.spec.ts b/desktop/tests/e2e/huddle-transcription.spec.ts index 622392989eb..9ce06e98450 100644 --- a/desktop/tests/e2e/huddle-transcription.spec.ts +++ b/desktop/tests/e2e/huddle-transcription.spec.ts @@ -692,6 +692,9 @@ test("animates the responding agent with the shared speaker ring", async ({ /rounded-squircle-clip/, ); + // The huddle chat's composer takes focus once its editor mounts, even from + // a focused button. Let that land before focusing the trigger. + await expect(page.getByTestId("message-input")).toBeFocused(); const voiceMenuTrigger = page.getByTestId("huddle-agent-voice-menu-trigger"); await voiceMenuTrigger.focus(); await expect(voiceMenuTrigger).toBeFocused(); diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index 59a8338071e..aa54cc442f1 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -596,48 +596,85 @@ test("composer Buzz chip labels wrap without orphaning their icons", async ({ await expect(chip).toHaveText("relaytoolsobservabilityconsole-main"); await page.getByTestId("send-message").click(); - const sentChip = page.getByTestId("message-row").last().getByRole("button", { + const sentRow = page.getByTestId("message-row").last(); + const sentChip = sentRow.getByRole("button", { name: "Open repository relaytoolsobservabilityconsole-main", }); await expect(sentChip).toBeVisible(); await expect(sentChip).toHaveClass(/wrapping-inline-chip/); - await sentChip.evaluate((element) => { - const container = element.parentElement; - if (container) container.style.width = "220px"; - }); - const fragmentMetrics = await sentChip.evaluate((element) => { - const chipStyle = getComputedStyle(element); - const rects = Array.from(element.getClientRects(), (rect) => ({ - bottom: rect.bottom, - height: rect.height, - left: rect.left, - right: rect.right, - top: rect.top, - width: rect.width, - })).filter((rect) => rect.width > 0 && rect.height > 0); - const fragmentTops = Array.from( - new Set(rects.map((rect) => Math.round(rect.top))), - ).sort((a, b) => a - b); - return { - boxDecorationBreak: - chipStyle.getPropertyValue("box-decoration-break") || - chipStyle.getPropertyValue("-webkit-box-decoration-break"), - fragmentStep: - fragmentTops.length > 1 ? fragmentTops[1] - fragmentTops[0] : null, - lineHeight: Number.parseFloat(chipStyle.lineHeight), - rects, - }; - }); - expect(fragmentMetrics.boxDecorationBreak).toBe("clone"); - expect(fragmentMetrics.lineHeight).toBe(22); - expect(fragmentMetrics.fragmentStep).toBe(22); - expect(fragmentMetrics.rects.length).toBeGreaterThanOrEqual(2); + // The optimistic message row can be re-created when the relay echo lands, + // dropping any inline style applied before the swap — apply the width and + // measure in one evaluate, polling until the chip settles into wrapped + // fragments with the expected cloned-box metrics. + await expect + .poll(() => + sentChip.evaluate((element) => { + const container = element.parentElement; + if (container) container.style.width = "220px"; + const chipStyle = getComputedStyle(element); + const rects = Array.from(element.getClientRects()).filter( + (rect) => rect.width > 0 && rect.height > 0, + ); + const fragmentTops = Array.from( + new Set(rects.map((rect) => Math.round(rect.top))), + ).sort((a, b) => a - b); + return { + boxDecorationBreak: + chipStyle.getPropertyValue("box-decoration-break") || + chipStyle.getPropertyValue("-webkit-box-decoration-break"), + fragmentStep: + fragmentTops.length > 1 ? fragmentTops[1] - fragmentTops[0] : null, + lineHeight: Number.parseFloat(chipStyle.lineHeight), + wrapped: rects.length >= 2, + }; + }), + ) + .toMatchObject({ + boxDecorationBreak: "clone", + fragmentStep: 22, + lineHeight: 22, + wrapped: true, + }); + + // The send and the wrap settle the timeline at its floor over several + // frames, and Radix closes an open tooltip whenever an ancestor of its + // trigger scrolls. That settle scrolls only when layout changes, and it ends + // at the floor. So focus the chip once the timeline sits at its floor and + // holds there for two frames: a timeline that hasn't started settling yet + // isn't at the floor. The send's ack is one more layout change: it drops + // the row's "Sending…" status, and the confirmed row can lose its header. + // Wait for it first, so the floor the poll sees is the final one. + await expect(sentRow.getByTestId("message-send-status")).toHaveCount(0); + const timeline = page.getByTestId("message-timeline"); + await expect + .poll(() => + timeline.evaluate( + (element) => + new Promise((resolve) => { + const atFloor = () => + element.scrollHeight - element.clientHeight - element.scrollTop <= + 1; + if (!atFloor()) { + resolve(false); + return; + } + const scrollTop = element.scrollTop; + requestAnimationFrame(() => + requestAnimationFrame(() => + resolve(atFloor() && element.scrollTop === scrollTop), + ), + ); + }), + ), + ) + .toBe(true); const tooltip = page.getByRole("tooltip"); await sentChip.focus(); - await expect(tooltip).toBeVisible(); const positionOverFragment = async (index: number) => { const fragment = await sentChip.evaluate((element, fragmentIndex) => { + const container = element.parentElement; + if (container) container.style.width = "220px"; const rects = Array.from(element.getClientRects()).filter( (rect) => rect.width > 0 && rect.height > 0, ); @@ -652,9 +689,9 @@ test("composer Buzz chip labels wrap without orphaning their icons", async ({ }, index); if (!fragment) throw new Error(`Expected chip fragment ${index}`); const cursorX = fragment.left + fragment.width / 2; - const cursorY = fragment.top + fragment.height / 2; - await page.mouse.move(cursorX, cursorY); - await expect(tooltip).toBeVisible(); + await page.mouse.move(cursorX, fragment.top + fragment.height / 2); + // A closing tooltip stays visible through its exit animation. + await expect(tooltip).toHaveAttribute("data-state", /open/); let tooltipCenter = Number.NaN; await expect .poll(async () => { diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 19332522fdd..613a46a9cf0 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -1343,6 +1343,33 @@ test("fast middle-page scroll settles with continuous mounted coverage", async ( ) .toBeGreaterThan(scrollHeightBeforePrepend + 2_000); + // Opening the channel armed the bottom settle, and only reader input + // cancels it, not the scrollTop writes below. Without a real wheel first, + // Virtua's next re-measure pins the list back to the floor. Wait until the + // wheel has left the floor (the timeline's 32 px at-bottom band) and held + // still for two frames, so the writes below are the last scroll. + await timeline.hover(); + await page.mouse.wheel(0, -120); + await expect + .poll(() => + timeline.evaluate( + (element) => + new Promise((resolve) => { + const scrollTop = element.scrollTop; + requestAnimationFrame(() => + requestAnimationFrame(() => + resolve( + element.scrollTop === scrollTop + ? element.scrollHeight - element.clientHeight - scrollTop + : 0, + ), + ), + ); + }), + ), + ) + .toBeGreaterThan(32); + // Simulate a fast trackpad pass through several middle-page ranges, then // stop. The final evaluate emits the last scroll event; all coverage samples // after it are passive observations. From 3b102926b40eccfa555ec0a397e9792ed461aa08 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 2 Nov 2026 23:25:34 +1100 Subject: [PATCH 2/2] test(desktop): wait for the Buzz chip to take the pointer before hovering Follow-up to the 10-30 review of #8743 on 13171fdd5. "composer Buzz chip labels wrap without orphaning their icons" still failed the PR's Flake Check: run 41051895661, job 124049928438, 1 of 10 repeats. The centre poll in positionOverFragment expected < 1 and received 81.3515625. The caller was :711 of the merge commit, positionOverFragment(0), so this was the first fragment. The current head (832737efa, a rebase) has the same positionOverFragment. Evidence: the tooltip is open. data-state matches /open/, and the error context still marks the chip [active]. The screenshot has the tooltip at x 333-621, centre 477. That's the centre of the whole two-line chip, where Radix places the tooltip when focus opens it. The cursor was at 395.6, over the "relay" fragment. In the video the timeline stops moving before the tooltip fades in at that spot, and the tooltip doesn't move for the rest of the run. Cause: useInlineTooltipPosition moves the content only from the trigger's pointermove. Once a pointermove lands, the hook positions the content in that frame or the next, and later Radix placements re-apply the offset. So with the pointer still, the tooltip stays at its focus placement only if the spec's move never reached the chip. Virtua 0.49.3 causes that. While the list is scrolling, its Virtualizer sets pointer-events: none on the item container. A 150 ms timer clears it, and every scroll event restarts the timer. Scrolls Virtua makes itself, like the bottom settle's scrollToIndex, don't set the flag, but other scroll writes can. The artifacts don't show which one set it here. The floor wait only proves scrollTop held for two frames, about 33 ms. A move inside the window hit-tests past the rows. When the rows take pointer events again the pointer hasn't moved, so no pointermove follows, and in this run none did. data-state can't catch this, because focus already opened the tooltip. Fix: positionOverFragment measures the fragment and hit-tests its centre with document.elementFromPoint in one evaluate. It polls until that point lands on the chip, then moves there. elementFromPoint skips pointer-events: none, so the poll waits out Virtua's flag. After the ack and floor waits nothing scrolls, so the move reaches the chip, and the centre poll then proves the reposition landed. This waits on a precondition. It doesn't retry the hover: the move, the open check and the centre poll each still run once per fragment. A fresh hover per fragment wouldn't fix this alone, because a re-entry inside the window misses the chip the same way. Not a product change: a real pointer sends a stream of moves, so the next one after the window positions the tooltip. Verification: pnpm typecheck and pnpm check pass, with biome's 3 existing warnings. An ad hoc tsc over navigation.spec.ts with the desktop tsconfig gives the same 7 pre-existing errors, none in the changed lines. Playwright wasn't run locally. The Flake Check lane will repeat the spec. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- desktop/tests/e2e/navigation.spec.ts | 51 ++++++++++++++++++---------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index aa54cc442f1..130abf48c1f 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -672,24 +672,39 @@ test("composer Buzz chip labels wrap without orphaning their icons", async ({ const tooltip = page.getByRole("tooltip"); await sentChip.focus(); const positionOverFragment = async (index: number) => { - const fragment = await sentChip.evaluate((element, fragmentIndex) => { - const container = element.parentElement; - if (container) container.style.width = "220px"; - const rects = Array.from(element.getClientRects()).filter( - (rect) => rect.width > 0 && rect.height > 0, - ); - const rect = rects.at(fragmentIndex); - if (!rect) return null; - return { - height: rect.height, - left: rect.left, - top: rect.top, - width: rect.width, - }; - }, index); - if (!fragment) throw new Error(`Expected chip fragment ${index}`); - const cursorX = fragment.left + fragment.width / 2; - await page.mouse.move(cursorX, fragment.top + fragment.height / 2); + // Virtua sets pointer-events: none on the timeline's rows while the list + // scrolls and clears it 150 ms after the last scroll event, longer than + // the floor wait's two frames. A move in that window misses the chip, and + // nothing sends it a pointermove once the rows take pointer events again, + // so the tooltip stays where focus opened it. Move only once the + // fragment's centre hit-tests to the chip. + let cursorX = Number.NaN; + let cursorY = Number.NaN; + await expect + .poll(async () => { + const fragment = await sentChip.evaluate((element, fragmentIndex) => { + const container = element.parentElement; + if (container) container.style.width = "220px"; + const rects = Array.from(element.getClientRects()).filter( + (rect) => rect.width > 0 && rect.height > 0, + ); + const rect = rects.at(fragmentIndex); + if (!rect) return null; + const x = rect.left + rect.width / 2; + const y = rect.top + rect.height / 2; + return { + hitsChip: element.contains(document.elementFromPoint(x, y)), + x, + y, + }; + }, index); + if (!fragment) throw new Error(`Expected chip fragment ${index}`); + cursorX = fragment.x; + cursorY = fragment.y; + return fragment.hitsChip; + }) + .toBe(true); + await page.mouse.move(cursorX, cursorY); // A closing tooltip stays visible through its exit animation. await expect(tooltip).toHaveAttribute("data-state", /open/); let tooltipCenter = Number.NaN;