[nwparker]

	Files	Added	Deleted	Net
Test	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​17	$\color{#cf222e}{\Huge{\mathbf{−}}}$​12	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​5
Prod	4	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​38	$\color{#cf222e}{\Huge{\mathbf{−}}}$​17	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​21

ELI5

Some of our browser popup tests click a button inside a small embedded frame right after creating it. On a busy Linux CI machine the click sometimes arrives before the browser knows the new frame is there, so the click lands on the page around it and nothing happens. The test then waits 10 seconds for a popup that never opens, fails, and blocks unrelated PRs. Now the tests confirm the frame actually receives the mouse before they click.

What Changed

This changes the test harness only. App code is untouched.

• Before: browser-popup-adapter-compatibility.electron.test.ts ("cannot distinguish inherited script restrictions…", "exposes inherited flags on native OpenURL link options only…") failed on fix(orcad-migration): a host fenced on a manifest no request can carry can be cancelled by its id #30864, fix(renderer): host writes route only through the strict owner, and twin projections stay on their host (review-5 #2 #9 #11) #30890 and brennanb2025/send-lifecycle-queue-derived-cards with expected false to be true … Matcher did not succeed in time.

• Mechanism: each sandboxed srcdoc iframe runs in its own renderer process. It can finish loading before the browser's hit testing routes pointer input to it. Playwright's actionability checks run inside the frame, so they pass, but the trusted click goes to the parent and the frame never sees it. I added instrumentation and reproduced this in CI: in all 4 failures the iframe recorded no pointerdown and no click.

• After: a new waitForHiddenFrameInput(locator) helper (src/main/browser/hidden-frame-input-readiness-fixture.ts) does three things before the click:

  1. installs a trusted-pointermove listener on the target;
  2. moves the mouse away and hovers again;
  3. repeats until the frame really receives input.

  The background popup probe already had an inline copy of this check for its opaque frame. That copy now uses the shared helper. The helper also covers the adapter sandbox clicks and the native opaque-subframe Shift POST.

• Cold first frame: test(browser): wait for the popup fixture's first paint during startup, not in the first test #30854 already moved the cold-start first-frame wait out of the first test, which was the reported native "Shift POST … : local" failure (runs 39324920346 and fix(ssh): two overlapping bridged-terminal attaches hand back the viewer they replace #30856's first run). This PR exports that 45s budget as a shared constant, HIDDEN_RENDERER_FIRST_FRAME_TIMEOUT_MS. It also applies the constant to popup-origin-chrome.electron.test.ts, which runs in the same CI step and still gave the same one-time startup only 10s.

Why

• Not a fixed wait, not Shift-click timing, not policy-event ordering. Both failure families are the hidden renderer not being ready for real input yet:
  • the first compositor frame of a cold launch (0.1–14s across 28 instrumented CI runs; a second launch on the same runner takes under 0.1s);
  • a new out-of-process frame that isn't yet in the browser's input routing.
• No product bug. Real users can't click a frame before it is painted and hit-testable.
• Waiting on the precondition, not on time. Raising poll timeouts would only hide a click that was never delivered. The readiness check waits for the exact thing the click needs, and it costs almost nothing once the frame is ready.

Linked Issue

N/A. This is a CI flake fix for an internal test harness; there is no issue.

Visual Proof

N/A. The change is test-only and has no UI or behaviour change.

Testing

• CI reproduction before the fix: 8 GitHub ubuntu-latest jobs, each repeating the two adapter sandbox tests 25 times in one Electron instance (temporary diagnostic workflow, now removed). 4 tests failed, and every failure showed the iframe got no pointer events.

• CI after the fix: the same 8 × 25-repetition jobs (about 2,400 sandboxed-frame clicks) gave 0 failures. 12 more cold-runner jobs ran all 5 popup Electron files (55 tests each) with 0 failures.

• Local stress loop: Linux Docker container capped at 4 CPUs, with stress-ng --cpu 8, ORCA_BACKGROUND_LAUNCH=1 and hidden windows under xvfb. Each run is a fresh launch of the native and adapter files, with the sandbox tests repeated 5×. Results are in the PR comments.

• macOS: all 5 popup Electron files pass (55 tests), with ORCA_BACKGROUND_LAUNCH=1.

• Static job: replayed locally (lint, format, anti-slop, code-quality plugins, type-aware lint, changed-code quality, typecheck, all ratchets, RPC catalog); everything passes.

• Live verification: nothing was verified on a live machine beyond these test runs, since the change is test-only.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

AI Disclosure

Review

Codex (gpt-6.1-sol, high): no findings. Independent Opus review: nothing blocking. Its nits (an explicit poll timeout, the hook budget, comment wording) are applied.

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Test-only. No SSH, mobile, wire-protocol or cross-platform runtime impact. The readiness hover uses Playwright input, so it behaves the same on every platform.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[nwparker]

Local stress loop (Linux Docker container capped at 4 CPUs, stress-ng --cpu 8, ORCA_BACKGROUND_LAUNCH=1, xvfb, hidden windows): 13 fresh-launch runs of the native and adapter popup files with the fix, sandbox tests repeated 5x per run: 13/13 passed, 0 failures (28/28 tests each). Honest caveat: this container never reproduced the flake before the fix either (25/25 native runs, 4/4 adapter runs and 360 amplified sandbox clicks all passed). The discriminating evidence is the GitHub ubuntu-latest amplification: 4 failures before the fix vs 0 of about 2,400 clicks after.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 78a24b4f-5661-4f73-b8f4-e1f72f5ac4c7

📥 Commits

Reviewing files that changed from the base of the PR and between 577df19 and de6fa83.

📒 Files selected for processing: 7

• src/main/browser/browser-popup-adapter-compatibility.electron.test.ts
• src/main/browser/browser-popup-background-cdp-probe.ts
• src/main/browser/browser-popup-compatibility-cdp-fixture.ts
• src/main/browser/browser-popup-native-compatibility.electron.test.ts
• src/main/browser/hidden-frame-input-readiness-fixture.ts
• src/main/browser/hidden-renderer-frame-capture-fixture.ts
• src/main/browser/popup-origin-chrome.electron.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

The change adds a shared helper that waits for hidden-frame input readiness and uses it in popup tests and probing. It also defines a shared 45-second hidden-renderer first-frame timeout and applies it to frame waits. The popup-origin test setup timeout increases from 60 to 90 seconds.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to de6fa

No actionable merge-blocking risk remains in the popup test changes. Normal checks are sufficient before merging.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description is complete and directly aligned with the test-harness flake fix. It includes the required explanation, changes, rationale, testing evidence, visual-proof rationale, review notes, and …
Title check		The title clearly and concisely identifies the primary change: routing input into newly created sandboxed popup iframes before clicking them.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is complete and directly aligned with the test-harness flake fix. It includes the required explanation, changes, rationale, testing evidence, visual-proof rationale, review notes, and checklist status. It marks the linked issue as N/A but explains that this is an internal CI flake fix with no issue.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}