ci: Settle the E2E Reply Before the Double-Click Quote (#14840)

* test(e2e): grant MULTI_CONVO.USE in the mock e2e config

`agent-skills-added.spec.ts` drives the composer's `+` command, which opens the
added-model popover. That path is gated on MULTI_CONVO.USE:

    if (!hasMultiConvoAccess || !plusCommandEnabled || isAssistantsEndpoint(endpoint)) return;

The mock config never sets `interface.multiConvo`, so the permission falls
through to the seeded role default and `handlePlusCommand` returns before
opening the popover. The spec then fails on a popover that is absent from the
DOM entirely, which reads as a selector or timing problem rather than a missing
permission.

Set it explicitly, the same way `contextCost` is set just above for the usage
gauge — the mock config's job is to make each exercised feature's gate explicit
rather than inherit a default.

* test(e2e): wait for the reply to settle before the double-click quote

`quotes.spec.ts` › 'summons the popup from a native double-click word
selection' double-clicks a word as soon as `mockReply` becomes visible. But
`sendMessage` resolves on the stream *response*, not on the final render, so the
reply can still be re-rendering.

A streaming markdown re-render swaps out the text node the selection points at,
which collapses the selection — the same mechanism the sibling
`selectionchange` test documents deliberately. A double-click landing mid-stream
therefore loses its selection before the popup can be clicked, and because the
whole gesture is wrapped in `toPass`, every retry re-runs into the same
still-streaming reply rather than recovering from a one-off.

This is a different race from the one #14777 fixed. That one is the *selection*
still settling (touch long-press, native handle drags, block-granularity
gestures) and is handled inside QuoteButton. This one is the *reply* still
streaming, which no amount of component-side settling can absorb.

Observed on a downstream fork running this suite on slower hardware: the test
fails all three attempts, deterministically on the in-memory stream store while
the Redis lane passes the same shard — the in-memory store's final re-renders
land late enough to outlive the gesture. Four separate runs, same split.

Wait for the reply text to hold steady before selecting.

* ci(e2e): install ffmpeg so first-retry video actually records (#14841)

`playwright.config.mock.ts` sets `video: 'on-first-retry'`, but the runner only
installs `install-deps chrome`, which does not include ffmpeg. Without it the
first retry fails inside `browserContext.newPage` while setting up video
recording — before the test body runs.

The cost is the retry itself: a genuinely flaky test loses the attempt that
would have recovered it, and the reported failure is a video-setup error rather
than the original symptom.

Bounded and non-fatal on purpose. The CLI has been observed hanging after the
download completes on these runners, so the step is wrapped in `timeout` and
its failure is swallowed — if ffmpeg cannot be installed the job proceeds
exactly as it does today, and no lane is blocked on it.

Applied to both jobs that run Playwright (`e2e_shards` and
`mcp_tool_list_changed`), since both configure retries.
This commit is contained in:
Danny Avila 2026-08-15 10:48:11 -04:00 committed by GitHub
parent fe71ffdf42
commit e1ac7d2bda
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 44 additions and 0 deletions

View file

@ -144,6 +144,16 @@ jobs:
- name: Verify Chrome is present
run: google-chrome --version
# `video: 'on-first-retry'` needs ffmpeg; without it the first retry dies in
# browserContext.newPage before the test body runs, so a flaky test loses the
# retry that would have recovered it. The CLI can hang after the download
# finishes on these runners, so bound it and keep it non-fatal — worst case is
# today's behaviour of retrying without video.
- name: Install Playwright ffmpeg (best effort)
timeout-minutes: 3
continue-on-error: true
run: timeout -k 10 90 npx playwright install ffmpeg
# The runner's Chrome is an apt package, so its real library dependencies are
# already satisfied; all `install-deps` adds here are optional CJK/Thai/Cyrillic
# font packages (~21MB from azure.archive.ubuntu.com). Nothing in CI asserts on
@ -235,6 +245,12 @@ jobs:
- name: Verify Chrome is present
run: google-chrome --version
# ffmpeg for retry video — see the note in the e2e_shards job.
- name: Install Playwright ffmpeg (best effort)
timeout-minutes: 3
continue-on-error: true
run: timeout -k 10 90 npx playwright install ffmpeg
# Optional fonts only — see the note in the e2e_shards job.
- name: Install optional Playwright font dependencies (best effort)
timeout-minutes: 4

View file

@ -98,6 +98,33 @@ async function doubleClickWord(page: Page, needle: string) {
await page.mouse.dblclick(point.x, point.y);
}
/**
* Wait for the reply to stop re-rendering before selecting text inside it.
*
* Distinct from the settle window `QuoteButton` applies to a *selection*: this
* is the reply itself still streaming. A markdown re-render swaps out the text
* node the selection points at, which collapses it so a double-click landing
* mid-stream loses its selection before the popup can be clicked, and every
* `toPass` retry loses the same race rather than recovering from it.
*
* `sendMessage` resolves on the stream *response*, not on the final render, so
* the wait has to be explicit.
*/
async function waitForReplyToSettle(page: Page, needle: string) {
const readReply = () =>
page.evaluate((text) => {
const renders = Array.from(document.querySelectorAll('.message-render'));
const host = [...renders].reverse().find((el) => (el.textContent ?? '').includes(text));
return host?.textContent ?? '';
}, needle);
await expect(async () => {
const before = await readReply();
await page.waitForTimeout(250);
expect(await readReply()).toBe(before);
}).toPass({ timeout: 20000 });
}
/**
* Triple-click the block containing `needle` with native mouse events, which
* makes Chromium select that whole block and park the selection's far boundary
@ -311,6 +338,7 @@ test.describe('quote references', () => {
const response = await sendMessage(page, 'seed for dblclick');
expect(response.ok()).toBeTruthy();
await expect(mockReply(page)).toBeVisible({ timeout: 20000 });
await waitForReplyToSettle(page, MOCK_REPLY_TEXT);
// A real double-click selects the word under the cursor. Chromium commits
// that selection on `dblclick`, AFTER `mouseup` fires, so only a `dblclick`