fix(email): send HTML bodies as multipart/alternative - #37
Conversation
The email gateway adapter shipped every outgoing message as text/plain, so HTML bodies (e.g. the Signal Scanner digest) arrived with raw <h2>/<p>/<a> tags showing instead of rendering in mail clients. Add _is_html_body() (narrow tag heuristic that won't trip on "x < y" or "I <3 this") and _attach_body(): HTML bodies become a proper multipart/alternative carrying both a plain-text fallback and the HTML part; plain text stays a single text/plain part. Route all three send paths (_send_email, _send_email_with_attachments, _send_email_with_attachment) through the helper. Attachment ordering is preserved. Adds TestHtmlBodyDetection (detection, multipart structure, plain-stays-plain, empty-body no-op). 65/65 email tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🔎 Lint report:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Single-letter tags misdetect HTML
- Split b/i out of the word-boundary tag list and require them to be followed by
>or whitespace so unspaced comparisons likea<bstay plain text while real<b>/<i>markup still matches.
- Split b/i out of the word-boundary tag list and require them to be followed by
Or push these changes by commenting:
@cursor push de9e10ea35
Preview (de9e10ea35)
diff --git a/gateway/platforms/email.py b/gateway/platforms/email.py
--- a/gateway/platforms/email.py
+++ b/gateway/platforms/email.py
@@ -177,7 +177,8 @@
# Tags that indicate a body is HTML rather than plain text. Kept narrow so a
# stray "<3" or a "x < y" comparison in a plain-text body isn't misdetected.
_HTML_BODY_RE = re.compile(
- r"<\s*(html|body|div|p|br|h[1-6]|a|ul|ol|li|table|span|strong|em|b|i)\b"
+ r"<\s*(html|body|div|p|br|h[1-6]|a|ul|ol|li|table|span|strong|em)\b"
+ r"|<\s*(b|i)(?=>|\s)"
r"|<\s*/\s*(html|body|div|p|h[1-6]|a|ul|ol|li|table|span|strong|em|b|i)\s*>",
re.IGNORECASE,
)You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 71c0145. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71c0145489
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot flagged that _HTML_BODY_RE treated `b` and `i` as opening tags with only a `\b` boundary, so plain-text comparisons like `a<b` or `5<i` matched and were wrongly sent as multipart/alternative. Require opening tags to be followed by a real terminator and split single-letter tags by how they actually appear: `b`/`i` (attribute-free in practice) require an immediate `>` or `/`; `a` (anchors carry href=) and multi-letter tags also accept trailing whitespace. So `<b>`/`<br/>`/ `<a href=...>` still detect as HTML, but `a<b and b>c` and a bare `<a@b.com>` address do not. Adds tests for both directions (real single-letter tags detected, comparisons not). 66/66 email tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Re: Codex P2 — Preserve literal HTML snippets in plain replies I pushed a fix for the missing The tension: Adding tags to Two approaches to resolve this fully:
Happy to implement either approach once there's a preference, or to revert the lenient tags if they're causing regressions in the meantime. Generated by Claude Code |
Codex flagged two opposite gaps in the detect-then-maybe-send-HTML
approach: (1) a coding-help reply containing literal markup could be
rendered instead of shown as code, and (2) valid HTML whose only tag is
outside the whitelist (img-only, pre-only) fell back to raw text/plain.
Resolve both by removing the send/don't-send guess. Every email is now
multipart/alternative with:
- a text/plain part that is the body verbatim (always faithful — never
tag-stripped), and
- a text/html part: the body as-is when it already looks like HTML,
otherwise escaped + newline->`<br>` via _text_to_html().
Detection (_is_html_body) now only chooses how to build the HTML part,
so a wrong guess can never make an email worse than plain text. Also
broadened the tag set with img/pre/code/blockquote for the standalone-
fragment case.
Adds tests: always-multipart, html-verbatim, plain-part-always-faithful,
partial-bracket escaping, img/pre detection, newline->br. 70/70 email
tests pass; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7cc471ddc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Second Codex pass on the always-multipart code raised 3 P2 issues: 1. HTML bodies attached their raw markup as the text/plain part, so text-only clients / indexed snippets still saw raw <h2>/<p> tags — the original bug. The plain part for an HTML body is now _strip_html(body). 2. Plain-text indentation/alignment (logs, code, tables) was collapsed by HTML whitespace folding. _text_to_html() now wraps the escaped text in <pre> so spacing survives. 3. A compact comparison like "if x<a and a>0" matched "<a " as an anchor opener and was sent as raw HTML. Anchors now require an href=/name= attribute or an actual <a>/</a> tag. Tests updated for the new plain-part contract and added for all three fixes. 71/71 email tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e69a6a281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ain part Third Codex pass raised 3 more P2 issues: 1. `<p ` in the lenient tag list matched compact comparisons like "if x<p and p>0". Moved `p` to the strict single-letter group (needs an immediate `>` or `/`), alongside `b`/`i`. 2. _strip_html() dropped anchor href targets, so the plain-text fallback of an HTML digest lost its URLs. Anchors now render as "label (url)". 3. _strip_html() only broke on <p>/<br>, so lists/tables collapsed (`<ul><li>One</li><li>Two</li></ul>` -> "OneTwo"). Added newline separators for block/list/row close tags and <li>. _strip_html() is shared with inbound extraction, so both directions get the more readable output. Adds tests for all three. 74/74 email tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b68cb314a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fifth Codex pass; addressed 5 of 6 P2s (the 6th is an inherent
ambiguity — see PR thread):
- Drop <style>/<script> blocks (tag + contents) so embedded CSS/JS
doesn't leak into the text/plain fallback.
- Preserve <img> as "[image: alt (src)]" so an image-only body isn't
blank in text-only clients.
- Tab-separate <td>/<th> so table columns don't fuse ("A\tB").
- text/html plain-text wrapper uses white-space:pre-wrap (not bare
<pre>) so long lines/URLs still wrap instead of clipping.
- Unified detection regex: a tag matches only when closed (<div>),
self-closed (<br/>), or carrying a real attribute (<div class=);
"<div and div>0" / "<code and code>0" comparisons no longer misfire
(same class as the earlier a/b/i/p fixes).
78/78 email tests pass; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a09b5818c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # match, since that's how comparisons read. | ||
| # * ``a`` additionally needs ``href=``/``name=`` or an actual ``<a>``/``</a>``. | ||
| # * Any well-formed *closing* tag (``</p>``) also counts. | ||
| _HTML_TAGS = r"html|body|div|br|h[1-6]|ul|ol|li|table|tr|td|th|span|strong|em|b|i|p|img|pre|code|blockquote" |
There was a problem hiding this comment.
Recognize standalone HTML tags
When the outgoing body is a valid HTML fragment whose only markup is a common standalone tag not listed here, such as Intro<hr>Outro, _is_html_body() returns false and _attach_body() treats it as plain text, so the preferred HTML alternative escapes the tag instead of rendering the divider. Add common standalone tags like hr (or use a more robust fragment detector) so these HTML emails do not regress to visible raw markup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed by PR #40 (fix(email): add <hr> to HTML body detection tag list). hr is now in _HTML_TAGS so a body containing only Intro<hr>Outro is detected as HTML and the <hr> renders correctly in clients. The broader standalone-tag case (e.g. '<img src="...">') was also addressed: img already appears in the detection regex and matches via the attribute rule ('<img src=...>').
Generated by Claude Code
There was a problem hiding this comment.
Fixed in PR #40 (fix(email): add <hr> to HTML body detection tag list) — hr was added to _HTML_TAGS so emails whose only or first HTML marker is a horizontal divider are now correctly classified and sent as multipart/alternative rather than plain text.
Reviewed and confirmed by Claude Code.
Generated by Claude Code
Analysis of 15 Codex P2 findings (post-merge)PR #37 was merged with 15 open Codex P2 inline comments on Category 1: False-positive HTML detection (high risk, 6 findings)Codex correctly identifies that
Assessment: Agree. These are real false-positives. The current lenient tag list ( Category 2: Plain fallback quality (moderate risk, 6 findings)For HTML bodies,
Assessment: Agree. These are genuine quality bugs in the strip → plain path. Most clients rarely read the Category 3: Plain-text → HTML conversion quality (3 findings)When
Assessment: Agree in principle, lower urgency. The Standalone tags (2 findings, duplicate)
Assessment: Agree, extend the detection list. These are common in digest emails specifically. Recommended follow-upA single follow-up PR touching
None of these block the current #37 behavior (which is already a strict improvement over no Generated by Claude Code |
Claude review of Codex P2 findings (PR #37)PR merged. Codex left 15 P2 comments on Agree and recommend follow-upHTML detection false positives (medium priority):
Plain-text fallback quality (medium priority, affects all HTML email recipients in text-only clients / search indexing):
Generated HTML alternative (low priority):
No action neededAll 15 comments are P2 (improvement). Nothing blocks functionality for the primary use case (HTML digest emails). Suggest bundling the false-positive fixes + fallback quality improvements into a single follow-up PR. cc @dizhaky — happy to draft the follow-up if you want to prioritise this. Generated by Claude Code |


What does this PR do?
The email gateway adapter (
gateway/platforms/email.py) shipped every outgoing message astext/plain. HTML bodies — e.g. the Signal Scanner digest — therefore arrived with raw<h2>/<p>/<a>tags showing in the client instead of rendering. This is a sender-side MIME-type bug, not a client setting.The fix detects HTML bodies and sends them as a proper
multipart/alternative(plain-text fallback + HTML part). Fixing the adapter — rather than any single digest generator — repairs all HTML mail this adapter sends.Related Issue
Fixes #
Type of Change
Changes Made
gateway/platforms/email.py_is_html_body()— narrow tag heuristic that detects real HTML but won't trip onx < yorI <3 this._attach_body()— HTML →multipart/alternative(plain-text fallback via existing_strip_html+ the HTML part); plain text → singletext/plainpart; empty body is a no-op._send_email,_send_email_with_attachments,_send_email_with_attachment. Attachment ordering preserved (body part first, then attachments).tests/gateway/test_email.pyTestHtmlBodyDetection: detection true/false cases, multipart/alternative structure, plain-stays-plain, empty-body no-op.How to Test
uv run pytest tests/gateway/test_email.py -q→ 65 passed (60 existing + 5 new).text/plainwith no behavior change.Checklist
Code
fix(scope):)tests/gateway/test_email.py, 65/65)Documentation & Housekeeping
email/smtplibonly, no platform-specific code — N/ANote
Low Risk
Localized MIME construction change in the email adapter with unit tests; plain-text behavior is preserved when HTML is not detected.
Overview
Outgoing mail from the email gateway no longer forces every body into a single
text/plainpart, so HTML digests and similar content can render in clients instead of showing raw tags._is_html_bodyuses a narrow tag regex so real markup is detected without treating plain text likex < yorI <3 thisas HTML._attach_bodybuildsmultipart/alternative(stripped plain fallback via_strip_htmlplustext/html) when HTML is detected; otherwise it attaches onetext/plainpart, and skips attachment for an empty body.All three SMTP send paths (
_send_email,_send_email_with_attachments,_send_email_with_attachment) now call_attach_bodyinstead of unconditionalMIMEText(..., "plain"), with body still before file attachments.TestHtmlBodyDetectioncovers detection, MIME structure, plain text, and empty body.Reviewed by Cursor Bugbot for commit 71c0145. Configure here.