Repository navigation
Serve the email templates' fonts and logo locally in matrix tests - #6883
Conversation
The Synapse email and validation-page templates load three Google Fonts stylesheets, a fonts.cdnfonts.com stylesheet and the Boxel logo from the internet. The stylesheets block rendering. When one is slow, the mail client shows the email as a blank pane and the screenshot assertion fails. In one failure the Poppins 700 stylesheet had no response when the screenshot was taken. The shared test fixture now routes those URLs, for every page in the context, to the bytes the browser received from them. A request to those hosts that is not stored gets a quick 404 and a warning, so a template change cannot bring the network dependency back without notice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdM45xnz3vvTmA7HN8nGmK
Codex Review SummaryThis 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. |
FadhlanR
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] This review checked whether routing the template's fonts and logo locally removes the blank-pane mechanism behind the email screenshot flake, whether anything outside the matrix tests changes, and the fix against the repo's flaky-test procedure. It did not re-run the matrix suite.
No blocking issues. Every email and validation-page screenshot runs in a page opened from the fixture's context (validateEmail uses appPage.context().newPage(), and the validation page comes from context.waitForEvent('page')), so the route reaches the iframe that was painting blank. The Synapse templates, and the workflow that syncs them to deployed environments, are untouched.
- Diagnostics in case another cause still blanks the pane. The new 404 warning catches a template that brings the network dependency back. It says nothing if the pane is blank for some other reason. Untraced first attempts are the ones that fail, so nothing would explain such a failure. In
validateEmail, beforeonEmailPage, could you log the message iframe'sdocument.readyStateanddocument.fonts.status(or awaitdocument.fonts.readythere)? Then a recurrence shows whether the page or its fonts had finished loading. Non-blocking, this PR or a follow-up. - The route covers every page in every matrix test, not only the email. See the inline comment on the warning in
helpers/email-assets.ts. Non-blocking.
The route is on the whole browser context, so a host page that loads from the same hosts, such as a card theme's web font, reached it too and got a 404 and a warning that called it an email asset. An unstored request now gets the 404 and the warning only when it comes from the mail client, the email in its iframe, or a page Synapse serves. Other pages' requests go to the network as before. Stored URLs are still served locally for every page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdM45xnz3vvTmA7HN8nGmK
If the email pane is blank for a reason other than the external stylesheets, the output now shows whether the email's document, stylesheets, images and fonts had loaded. A first attempt that fails has no trace, so this line is the only record of that state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdM45xnz3vvTmA7HN8nGmK
|
[Claude Code 🤖] Thanks for the review. Both points are addressed in this PR.
|
Background and Goal
profile.spec.ts › it can change email in settingsfailed 4 times in 69mainruns in a week and passed on retry. Every first attempt failed the same way:email-change-verification-email.pngdiffered by about 26,600 pixels (ratio 0.03). In the actual screenshot, the mail client's message pane is blank.The Synapse email templates (
support/synapse/templates/_base.html) load three Google Fonts stylesheets, afonts.cdnfonts.comstylesheet and the Boxel logo from the internet. The stylesheets are in the email's<head>, so the browser paints nothing in the email's iframe until they load. The test checks only that the text "Verify Email" is in the iframe's DOM, and that is true before anything is painted. In the trace of one failure, thePoppins:wght@700stylesheet had no response when the screenshot was taken.The shared fixture now routes those URLs, for every page in the browser context, to the bytes the browser received from them. These are stored in
packages/matrix/support/email-assets/, with the Poppins license. A request to those hosts that is not stored gets a quick 404 and a console warning, so a template change cannot bring the network dependency back without notice. The stored CSS is in.prettierignoreso it stays byte-for-byte.Verification
Three throwaway draft PRs hold the Poppins 700 stylesheet for 8 seconds and run the three email screenshot tests 5 times on each of the 3 matrix shards, with one worker and no retries:
main, no fix):it can change email in settingsfailed 15 of 15 with the same 0.03 pixel diff as the CI flake.Fixes CS-14183
🤖 Generated with Claude Code
https://claude.ai/code/session_01WdM45xnz3vvTmA7HN8nGmK