fix(emails): stop the newsletter's design living in its own mj-head, and ship the MJML source - #3298
Merged
vpetersson merged 4 commits intoAug 19, 2026
Conversation
Mailjet reads MJML directly, so the source is the file to hand over, and it is the only version anyone can still edit: nobody is going to change a headline inside 35 KB of nested tables, which is how the sent mail and the repo stop being the same email. The compile still runs, as a check rather than as a deliverable, so strict validation and the Gmail size limit still gate the workflow. Drops the masthead PNG from the bundle and from the trigger paths. The template references it by URL from the site, so the .mjml is already the whole email and a copy of the image alongside it was only ever an invitation to re-host something that did not need re-hosting. Records why it is not inlined: a base64 data: URI would make the file self-contained offline, but Gmail, Outlook and Yahoo all refuse data: image URIs, so the masthead would be a broken image at the top of the newsletter for most of the list. Apple Mail and Thunderbird render them, which is exactly enough to make that invisible in testing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A commit touching both website/ and application code matches this workflow's push paths and the schema workflow's, so it fired twice for one SHA: once on push, and again about ninety seconds later when Generate OpenAPI Schema finished. Both build the same site and both want the `pages` concurrency group, so the later cancelled the earlier and left a cancelled run against master that reads as a broken deploy on a green commit. The first commit to do both was the newsletter template. Adds a triage job that stands the push copy down when a schema run exists for the same SHA. The workflow_run copy is the one worth keeping: it is the only one whose Fetch OpenAPI schema step can find a schema built from that commit, where the push copy runs before the schema workflow has finished and falls back to the previous run's, publishing an API page one commit behind. It asks the Actions API rather than inferring from changed paths. The schema workflow's filter is a long list of exclusions and a second copy of it here would go stale the first time that list was edited, silently. Concurrency is untouched, so cancel-in-progress still stops a slow older run republishing over a newer one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the newsletter email CI artifact to ship the editable MJML source (while still compiling as a validation gate), and adds a triage gate to prevent duplicate GitHub Pages deploys when both push and workflow_run triggers fire for the same commit.
Changes:
- Switch the email workflow artifact to include
emails/newsletter.mjml+emails/README.md(not compiled HTML or the masthead image), while still compiling + enforcing Gmail size limits. - Document the updated “send vs compile” workflow and why the masthead is URL-referenced instead of inlined.
- Add a “triage” job to
deploy-website.yamlto avoid double-deploys by standing down thepushrun when a matching schema workflow run exists for the same SHA.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
emails/README.md |
Updates guidance for obtaining/sending the newsletter template and documents masthead embedding constraints. |
.github/workflows/build-email.yaml |
Changes the artifact contents to ship MJML source + notes; keeps compile/size checks as validation gates. |
.github/workflows/deploy-website.yaml |
Adds a triage gate to prevent duplicate Pages deploys for a single commit when schema workflow chaining is in play. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3298 +/- ##
=========================================
Coverage ? 90.33%
=========================================
Files ? 85
Lines ? 9942
Branches ? 1098
=========================================
Hits ? 8981
Misses ? 708
Partials ? 253 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
emails/README.md:47
- This is a sentence fragment (“For looking…”). Making it a complete instruction improves readability in the doc.
For looking at the result in a browser, and for the strict validation
pass. The compiled file is not committed and is not what gets sent:
.github/workflows/build-email.yaml:8
- Spelling is inconsistent within this file: this header uses “artefact” while later comments and GitHub terminology use “artifact”. This reads like a typo in US-English repo docs/workflows.
# over the one artefact nobody can edit: a change to a headline in 35 KB
…gh's exit code Two fixes from review. The probe was unscoped, and most runs of the schema workflow are pull_request runs carrying a BRANCH head SHA: 32 of the last 40. Any commit whose SHA also had a PR run would have stood the push copy down for a run whose completion cannot trigger this workflow at all, because the workflow_run trigger is itself filtered to master. That commit would then never deploy, which is a worse failure than the duplicate run this job exists to remove. Scoped to branch=master, which matches exactly the runs that come back here. Confirmed against the API: the SHA of this branch's tip returns 1 unscoped and 0 scoped. The failure path also did not work. gh prints its error body on stdout, so `|| echo 0` appended a zero to a JSON blob and the numeric comparison then failed noisily on every retry. It reached the right answer by way of the `if` being exempt from errexit, which is not a thing to rely on. Non-digits are now rejected outright, and `set -uo pipefail` is explicit since only `-e` is a runner default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/deploy-website.yaml:93
- The comment about runner defaults is slightly inaccurate: for GitHub Actions' default bash shell,
-eand-o pipefailare already enabled by default; only-uis missing. As written, it impliespipefailis not default, which can mislead future edits/debugging.
# `-e` is already the runner default for a `run:` block; `-u`
# and `pipefail` are not, and this reads the API into a variable
# it then compares numerically.
The previous wording said `-e` was the runner default and pipefail was
not, which is true here but reads as a claim about `run:` blocks in
general. It is a claim about blocks with no `shell:` key: the Linux
default is `bash -e {0}`, and the `-eo pipefail` people remember comes
from writing `shell: bash` explicitly. Named so, since a later edit
adding that key would silently change which half of this is redundant.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
vpetersson
approved these changes
Aug 19, 2026
vpetersson-bot
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/build-email.yaml:10
- The PR description says the newsletter template no longer uses
<mj-attributes>/mj-classand that styling was moved onto each element, with guards to enforce that. In the current tree,emails/newsletter.mjmlstill contains a<mj-attributes>block and multiplemj-classusages (e.g. around the start of the head section), and the described guard tests don’t appear to be present. Either the template/tests changes are missing from this PR, or the PR description needs updating to match what actually shipped.
# Validates emails/newsletter.mjml and publishes it as a downloadable
# zip on the run.
#
# What ships is the .mjml itself, not the compiled HTML. Mailjet takes
# MJML directly, and handing over the compiled output would mean handing
# over the one artefact nobody can edit: a change to a headline in 35 KB
# of nested tables is a change nobody will make in the source, which is
# how the sent mail and the repo stop being the same email.
5 tasks
Contributor
Author
|
Superseded by #3299. GitHub stopped syncing this PR's head: it is pinned to 3299 carries the same commits plus the mj-head fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Follow-up to the newsletter template. Two things: the CI bundle was
shipping the wrong file, and merging it exposed a duplicate Pages deploy.
Description
The template was broken, and this is the fix for it.
The whole design sat in one
mj-attributesblock inmj-head: the textcolours, the section backgrounds, and every padding. Anything that takes
the body without the head drops it, and MJML falls back to its own
defaults. Reproduced by stripping the block: 19 elements render
#000000on#270035, which is 1.6:1, with no cards and 20px ofdefault padding everywhere. It still compiles, still sends, and still
looks like an email, which is exactly what made it ship.
Every colour, size, weight and space is now written on the element that
uses it. No
mj-attributes, nomj-class, and link colours on eachanchor rather than in
mj-style, which several clients drop anyway. Thehead keeps the title, preview text, web font, and one media query for the
gap between stacked cards, whose entire blast radius is that gap.
Deleting the whole head now changes nothing about colour or size.
Three guards, because the failure is silent:
test_nothing_visual_depends_on_the_headrejects anmj-attributesblock or an
mj-classreference.test_every_styled_element_carries_its_own_stylingrequires eachmj-textandmj-buttonto name its owncolor,font-family,font-sizeandline-height. Removing the block is only half of it,since an element that sets no colour still inherits MJML's black.
mj-headdeleted and fails on asingle
#000000. Only mjml can say whether the result still renders; astatic guard can only look for the causes already known.
Each was checked by causing the failure it exists for.
Design fixes found while re-reading the render:
30/20/18px, so a section heading barely outranked the paragraph above
it. Now 36/24/18, off
--text-4xland--text-2xl.and at 640 the card's copy ran to about 95 characters a line.
the email has one structural device rather than none.
On contrast: the previous round checked design tokens on paper, which
is not the same as checking the email. This one runs axe-core's
color-contrastrule, the same rule PageSpeed uses, over the renderedDOM at 760px and 412px, with and without the head: 27 text elements, zero
below 4.5:1, worst pair 5.93:1.
The bundle ships the
.mjml, not the compiled HTML.Mailjet reads MJML directly, so the source is the file to hand over, and
it is the only version anyone can still edit. Nobody is going to change a
headline inside 35 KB of nested tables, which is how the sent mail and the
repo stop being the same email. The bundle is now
newsletter.mjmlplusthe README, and the packaged template is byte-identical to the one in the
repo.
The compile still runs, as a check rather than as a deliverable, so strict
validation and the Gmail clipping limit still gate the workflow.
The masthead PNG is out of the bundle and out of the trigger paths. The
template references it by URL from the site, so the
.mjmlis already thewhole email, and a copy of the image alongside it was only ever an
invitation to re-host something that did not need re-hosting.
On inlining the masthead: it is deliberately not inlined, and the file
now records why. A base64
data:URI would make the templateself-contained offline, but Gmail, Outlook and Yahoo all refuse
data:image URIs, so the masthead would be a broken image at the top of the
newsletter for most of the list. Apple Mail and Thunderbird do render
them, which is exactly enough to make the problem invisible in testing.
The form of embedding that works everywhere is a
cid:inlineattachment, and that is send-side setup in Mailjet rather than something
the template can carry.
One commit no longer produces two competing Pages deploys.
A commit touching both
website/and application code matches thisworkflow's push paths and the schema workflow's, so it fires the deploy
twice for one SHA: once on push, and again about ninety seconds later when
Generate OpenAPI Schema finishes. Both build the same site and both want
the
pagesconcurrency group, so the later cancels the earlier and leavesa cancelled run against master that reads as a broken deploy on a green
commit. The newsletter merge was the first commit to do both; the site
itself deployed fine, from the surviving run.
A triage job now stands the push copy down when a schema run already
exists for the same SHA. The
workflow_runcopy is the one worth keeping:it is the only one whose Fetch OpenAPI schema step can find a schema built
from that commit, where the push copy runs before the schema workflow has
finished and falls back to the previous run's, publishing an API page one
commit behind.
It asks the Actions API rather than inferring from the changed paths. The
schema workflow's filter is a long list of exclusions, and a second copy
of it here would go stale the first time that list was edited, silently
and in whichever direction is worse.
Concurrency is untouched, so
cancel-in-progressstill stops a slow olderrun republishing over a newer one.
One consequence worth stating, and it is commented in the file: a
commit whose schema workflow fails now does not deploy at all, because
the push copy has stood down and the existing gate stops the
workflow_runcopy. That is the intended trade. Shipping a stale APIreference is the failure this chain exists to prevent, and a commit that
broke the schema workflow is getting fixed or reverted anyway.
Verification
newsletter.mjmlandREADME.md, anddiffconfirms the packagedtemplate is byte-identical to the repo's
03a8ae72, the commit that actually produced the cancelled run, returnsproceed=false; the website-only0467a783returnsproceed=trueafterpolling; a
workflow_runevent returnsproceed=trueimmediatelyworkflow_runcopy starts, so the run that stands down finishes beforeanything could cancel it
actionlintclean,ruff checkclean,pytest -m "not integration"unaffected
Checklist
Not applicable: CI and a marketing asset, neither ships to devices.
Not applicable, same reason.
emails/README.mdupdated for the new bundle contents, and bothworkflows carry the reasoning inline.