Repository navigation
main fix: settle Select popups before choosing agent editor options - #544
Conversation
Main run 37025966190 (f264c62) failed in Chromium because the "Host provider" click never selected anything. The option clicks ran during the Select popup's entry transition. Playwright exhausted three "element is not stable" retries and its fourth attempt called scrollIntoView({block: "start"}), scrolling the scroll-locked document from 158 to 284. The editor dialog and trigger are fixed but the popup is document-positioned, so Floating UI re-anchored it before the press, which landed on the Provider label. The popup closed with the custom value still selected and save persisted fixture-provider. Wait for the open popup's entry transition to finish before clicking editor options, and assert the shown harness/provider before saving. Users cannot scroll the locked page, so this is a test race rather than a product race. Signed-off-by: Logan Johnson <loganj@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
No changes requested. The test-only helper waits for the shared Select popup’s entry transition before normal option clicks, preserving saved-value assertions and adding visible harness/provider checks without sleeps, retries, or production changes.
Star Lord automated source review via Wes’s account — head 5e9627ca7f5b0f04a1a243a63d593160d4894dcb, base c9946be433f1957713a19cffca614947c17514ee.
Source-only: I ran no tests or app workflows; hosted CI artifacts show the full affected file passed 32/32 across Chromium/WebKit on the PR merge snapshot, but I did not independently reproduce the race, verify live focus behavior, or establish before/after timing.
wesbillman
left a comment
There was a problem hiding this comment.
No material findings. The helper waits for the Select popup’s actual entry transition to finish, then performs the normal option click. It neither forces selection nor disables animation; existing saved-value assertions remain, with additional visible harness/provider checks before saving.
Validation: reviewed the complete diff, shared Select/CSS transition, editor selection wiring, and original CI failure. Hosted CI 37031859984 passed for this head on merge snapshot c162bd115eca279708de93634806f3f073ce3e24 (the reviewed base/head). Logs confirm the full affected file passed 32/32 across Chromium and WebKit. No local test run or independent reproduction of the author’s retry/scroll experiment; this is not a blanket claim that selection cannot race in other conditions.
Reviewed head 5e9627ca7f5b0f04a1a243a63d593160d4894dcb against base c9946be433f1957713a19cffca614947c17514ee.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Emergency fix for intermittent Chromium failure on main: run 37025966190 (
f264c623),agent-control.spec.mjs:757"editor renders host choices…", line 807: saved{command: "host-command", provider: "fixture-provider"}, expectedprovider: "host-provider".Cause (test race, not product race)
From the CI trace:
element is not stablethree times for each, so its fourth attempt usedscrollIntoView({block: "start"}).html.scrollTop158 → 284). The editor dialog and its trigger areposition: fixed, but the Base UI Select popup is document-positioned, so Floating UI re-anchored the popup under the fixed trigger before the press.pointerdownit saw, so the popup dismissed with "Custom provider / current value" still selected, and save persistedfixture-provider. The final snapshot shows exactly that.Users can't scroll the locked page underneath a modal, so they don't hit this. The test clicked before the UI settled.
Fix
tests/browser/agent-control.spec.mjsonly: achooseOptionhelper waits until the open popup has leftdata-starting-styleand has no running animations (the existingopened()pattern inchannel-members-focus.spec.mjs), then clicks. It's applied to the editor-dialog option clicks in this test and in the sibling "native-supplied harness choices" test, which uses the same pattern. The failing test also asserts the harness and provider combobox text before saving. No assertions were weakened, and there are no timeouts, retries, sleeps, or skipped engines.Evidence
scrollIntoView({block:"start"}), then press at the post-scroll point) fails 5/5 in Chromium with the identicalprovider: "fixture-provider". The press lands onSPAN:Providerat scrollTop 284, matching CI.--repeat-each 20on Chromium + WebKit: 80/80 passed. Fullagent-control.spec.mjson Chromium + WebKit: 32/32 passed.scripts/check-staged.mjs(Biome) passed.scripts/check-push.mjs(plain,--design,--clippy) passed, with no unit/design/Rust inputs changed.pnpm typecheckpassed.Browser cases added/removed: none. No product code changed, so there is no unit regression test. The defect is in test synchronization.
Logan sent this to team review on 2026-10-02, so it is no longer held as a draft. Review is still required, per AGENTS.md.