Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Bradley Axen <baxen@squareup.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
baxen
force-pushed
the
mbot/conversation-warmup
branch
from
November 8, 2026 01:36
7ce28d9 to
34dc89d
Compare
Collaborator
Author
|
🤖 Folded into #3015 so the CI fixes land together. |
baxen
added a commit
that referenced
this pull request
Nov 8, 2026
Fixes today's broken and flaky buzz-app CI in one PR: red main plus two flakes that hit several PRs. Supersedes #3017 and #3018. # 1. Red main: `solo-agent-tagging.spec.mjs` Main is red: `tests/browser/solo-agent-tagging.spec.mjs` (#2941) fails on main and on every PR CI'd since. The test waits for a composer `alert` holding a "Retry agents" button. #2948 removed that standing notice; a failed agent read now offers "Retry agent list" inside the Choose agent picker. #2941 was written before #2948 landed and merged without rerunning against it. The test now opens the Choose agent picker and asserts and clicks "Retry agent list" there, then checks the retry goes away once the agent is addressed. The behaviour it covers (tag nobody without complete inventory, address the lone agent after retry, unchanged once the background snapshot completes) is the same. Verified locally: `playwright test solo-agent-tagging` passes on chromium and webkit. ## Follow-up: trigger shape The first push failed this PR's webkit Flake check: 8 of 10 repeats found no "Choose agent" button. The screenshot shows the composer's "Mention a member" (@) button instead. The chat only shows the agent pill when every member is a known agent. With the inventory missing, that depends on whether the agent's profile has loaded, which is a background read. Both trigger shapes are correct while the inventory is missing, and both open the same picker with "Retry agent list". The test now opens the picker from either trigger. To check the @ path deterministically, I held kind-0 profile reads after the gap until Retry. The trigger was then "Mention a member", and the updated test passed on chromium and webkit. Without that hold, it passes `--repeat-each=15 --workers=6` on both engines (30 passed). # 2. `conversation.spec.mjs` page-load flake (from #3017) ## Cause `tests/fixtures/conversation.tsx` imports `bundledPlugins`, so its first load makes the fresh dev server transform about 1,650 modules. A failing CI trace (run 42422209266, repeat7) shows no reload and no failed request: the module graph just finishes transforming about 15 s in, while the other worker runs the same test's `pnpm build`s. `prepareFixtureDependencies` already keeps dependency bundling out of navigation budgets. Source transforms had no equivalent. ## Change `warmUp(server, entry)` in `vite-server.mjs` asks Vite to transform the entry and waits for its static-import crawl: `server.warmupRequest` + `server.waitForRequestsIdle`, which are Vite's own APIs. The conversation spec calls it after `listen()`, before `page.goto`. The navigation timeout, retries and assertions are unchanged. ## Evidence (local, macOS, chromium, `--repeat-each=4 --workers=2`) | | goto → load | |---|---| | before | 3.6–3.9 s | | after | 1.03–1.07 s (warm-up takes ~2.7 s, outside the budget) | The full `conversation.spec.mjs` passes on chromium and webkit (22 passed). I didn't reproduce the CI timeout itself locally. The before/after split is the evidence that compile work has moved out of the budget. No browser cases are added or removed. # 3. `huddles.spec.mjs` roster animation flake (from #3018) `huddles.spec.mjs` › "capsule and companion presentation share mute and leave" fails intermittently in webkit Flake check lanes on unrelated PRs today: `locator.evaluate: AbortError: The operation was aborted.` at the roster settle step (jobs 128522170397 on mbot/bottom-activity and 128560887574 on mbot/community-denied, repeats 9 and 5). ## Cause The step awaits `Promise.all(getAnimations().map((a) => a.finished))`. When Motion replaces an animation it cancels the old one, and a cancelled animation's `finished` promise rejects with `AbortError`. So any interruption while the roster settles fails the test, even though the roster ends up correct. A standalone webkit check shows the same message: cancel a running animation, and the old wait throws `locator.evaluate: AbortError: The operation was aborted.` The new wait resolves. ## Change Wait until no animation in the roster is still `running`, using `Promise.allSettled` so a cancelled one ends the wait for that animation rather than failing the test. The geometry assertions after this step are unchanged. ## Evidence - The fixed test passes `--repeat-each=10` on chromium and webkit locally (20 passed). - I couldn't reproduce the cancellation locally (the old code passed 30/30 on webkit), so the evidence for the fix is the matching error and the standalone check above. Two other sites in this file use the same `Promise.all(... finished)` wait (around lines 698 and 2019). Neither has failed in today's runs, so I left them alone. --------- Signed-off-by: Bradley Axen <baxen@squareup.com> Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
conversation.spec.mjs› "independent packed author consumer…" fails today's advisory Flake check (chromium) lanes on PRs that reach it (jtennant/thomas/history-window-slide, kylez_NEXUS-4311_owner-member-endpoints, joah/link-previews-off, tho/page-record-signing, mbot/community-denied, and others). Every one of those failures ispage.goto: Timeout 15000ms exceededonconversation.html.Cause
tests/fixtures/conversation.tsximportsbundledPlugins, so its first load makes the fresh dev server transform about 1,650 modules. A failing CI trace (run 42422209266, repeat7) shows no reload and no failed request: the module graph just finishes transforming about 15 s in, while the other worker runs the same test'spnpm builds.prepareFixtureDependenciesalready keeps dependency bundling out of navigation budgets. Source transforms had no equivalent.Change
warmUp(server, entry)invite-server.mjsasks Vite to transform the entry and waits for its static-import crawl:server.warmupRequest+server.waitForRequestsIdle, which are Vite's own APIs. The conversation spec calls it afterlisten(), beforepage.goto. The navigation timeout, retries and assertions are unchanged.Evidence (local, macOS, chromium,
--repeat-each=4 --workers=2)The full
conversation.spec.mjspasses on chromium and webkit (22 passed). I didn't reproduce the CI timeout itself locally. The before/after split is the evidence that compile work has moved out of the budget.No browser cases are added or removed.