Skip to content

Deflake dialog outside-dismiss scenario - #1847

Merged
RhysSullivan merged 1 commit into
mainfrom
deflake-dialog-outside-dismiss
Aug 29, 2026
Merged

Deflake dialog outside-dismiss scenario#1847
RhysSullivan merged 1 commit into
mainfrom
deflake-dialog-outside-dismiss

Conversation

@RhysSullivan

Copy link
Copy Markdown
Collaborator

The scenario failed its last step on every selfhost CI run since the merge (main and four PR branches, all at the same assertion): the field-free revoke confirm stayed open after the outside click, and the detach wait timed out.

The trace from a failing run shows the click was dispatched 2ms after the dialog reported visible, and the DOM afterwards is byte-identical — the pointerdown was never seen by anything. "Visible" is not "listening": Radix arms outside-pointerdown dismissal via a document listener attached in a passive effect plus a 0ms timer after the content mounts. At the click moment the body already carried the modal scroll lock (the effect flush had run), so only that timer was pending; a busy renderer services injected input ahead of pending timers, and a pre-arming pointerdown simply vanishes. The test clicked once, so a missed click meant open forever. The keep-open steps could also pass vacuously the same way.

The fix synchronizes instead of sleeping: wait for the modal scroll lock (proves the arming timer is queued), then one 0ms timer round-trip through the page (same-source timers are FIFO, so the arming timer has fired), then click. Applies to all three outside clicks, which also makes the keep-open assertions provably non-vacuous. Assertions unchanged.

Not reproducible on an idle 18-core machine after 40+ attempts, including CPU throttling and saturation — the inversion needs a backlogged renderer main thread. An instrumented run that timestamps listener attachment confirmed the synchronization orders every click after arming (30/30 under saturation). Fixed scenario 10x green, plus 5x green under CPU saturation. Test-only change.

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Torn down — the PR is closed.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing 26e20c3 Commit Preview URL

Branch Preview URL
Aug 29 2026, 08:13 AM

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud 26e20c3 Aug 29 2026, 08:14 AM

@pkg-pr-new

pkg-pr-new Bot commented Aug 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1847

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1847

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1847

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1847

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1847

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1847

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1847

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1847

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1847

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1847

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1847

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1847

executor

npm i https://pkg.pr.new/executor@1847

commit: 26e20c3

@RhysSullivan
RhysSullivan marked this pull request as ready for review August 29, 2026 08:18
@RhysSullivan
RhysSullivan merged commit 22d06aa into main Aug 29, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant