[tris203]

Problem

ProjectFaviconPickerDialog.browser.test.tsx › "loads previews asynchronously and only acquires offscreen images after scrolling" fails intermittently on CI (item 6 of #22324). The final expectPreview(last) gets { width: undefined, fallback: true }, and waiting longer never helps.

Change

The URL published for row 39 was being thrown away. After scrollIntoView, the thumbnail's IntersectionObserver calls setInView(true) outside act(). React renders the row, and useAtomValue reads the fixture atom, which creates a registry node with no listeners. The registry schedules that node's removal with setTimeout(0), but React 19 only subscribes in a passive effect after the next frame. If the test's poll sees the createUrl call in that gap, publishUrl writes into the unsubscribed node. The node is then removed along with the value, and the row resubscribes to a fresh Initial node and keeps the fallback icon forever.

The fixture atoms are plain writable atoms, so their only state is the registry node. Making them Atom.keepAlive keeps the published value. This is test-only: real createUrl atoms compute their own value, so a dropped node is rebuilt from that rather than lost. Row 0 and the other tests in the file render inside act(), so they subscribe before publishing. All addFiles callers get the same protection.

Scope and approval

Fixes item 6 of #22324. Test-only, following @juliusmarminge's invitation to pick up the remaining #22324 flakes: #22511 (comment)

Verification

• Instrumenting row 39's atom right after the render that requested it logged microtask {listeners:0, Initial} | timeout0 removed | raf {listeners:1, Initial}. The node is removed before React subscribes.
• A scratch copy of the test that publishes right after row 39 renders failed 3/3 with the CI diff (width: undefined, fallback: true) before the fix and passed 3/3 after. The original file passes (7/7 tests). The scratch test is not committed.
• Leftover state from earlier test files was ruled out: each browser test file gets a fresh iframe, and parking the real pointer at 10 positions before the test did not reproduce it. Why the gap only lines up in full-suite CI runs (most likely heavier load stretching frame timing) is inferred, not reproduced.

Opus 5.5 via Claude Code, reviewed by GPT-6.1-Sol via Codex in T3 Code.

🤖 Generated with Claude Code

[macroscopeapp]

Approvability

Verdict: Approved at fbcac9c

Macroscope's review found this PR approvable — This is a six-line adjustment confined to an ignored browser test fixture, preserving published preview state during an asynchronous test race. It has no production runtime, product-default, schema, deployment, security, billing, or static-analysis impact.

Notes:

• No code objects were reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: f987fc0b-9120-49e2-9a06-71748c84f937

📥 Commits

Reviewing files that changed from the base of the PR and between 5bcc90e and fbcac9c.

📒 Files selected for processing: 1

• apps/web/src/components/settings/ProjectFaviconPickerDialog.browser.test.tsx

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

📝 Walkthrough

The browser test helper now creates each asset atom with Atom.keepAlive when addFiles adds files.

Changes: Favicon asset test

Layer / File(s)	Summary
Keep asset atoms alive apps/web/src/components/settings/ProjectFaviconPickerDialog.browser.test.tsx	addFiles initializes each asset atom with Atom.keepAlive.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: serrnovik

Merge Risk: ⚪ Minimal · up to fbcac

The change addresses the deferred-subscription test race without introducing a merge-blocking issue.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the test fix: offscreen favicon previews no longer lose their published URL. It also follows the conventional commit format.
Description check		The description covers the problem, root cause, change, scope and approval, focused verification, test results, limitations, and agent attribution. It is specific and complete for this test-only fix.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[t3dotgg]

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing as superseded. #22523 (merged as f9dd259) landed the same fix: addFiles now wraps the fake asset atoms in Atom.keepAlive, for the same render-before-subscribe gap you found. The two diffs are line-for-line equivalent, so this branch now conflicts with main and has nothing left to add. Thanks for tracing it; your root cause matched ours.