fix(email): HTML emails sent as plain text — two critical bugs - #36853
fix(email): HTML emails sent as plain text — two critical bugs#36853chtse53 wants to merge 1 commit into
Conversation
Problem: Cron job emails containing HTML (daily briefings, book summaries)
were sent as text/plain, causing email clients to display raw HTML tags
instead of rendering the HTML content. Two bugs caused this:
1. gateway/platforms/email.py _attach_body():
- rfind('</html>') picked the LAST closing tag, but models sometimes
produce duplicate </html> tags with garbage text between them
- For HTML fragments (no </html>), only stripped narrow 'Cronjob
Response' footer patterns — model commentary like 'The previous
response was already complete...' leaked through into the HTML body
- Fix: use find() for </html> (first occurrence), and for fragments
walk backwards from the last closing block-level tag to detect and
strip trailing model commentary (pure prose after </div>, </p>, etc.)
2. tools/send_message_tool.py _send_email():
- Regex had ^ anchor: ^\s*(?:<!DOCTYPE\s+html|<html[\s>])
- .match() only detected HTML at position 0 — cron model outputs
ALWAYS have preamble text before HTML, so this ALWAYS fell through
to text/plain
- Fix: use .search() (no ^ anchor), add preamble stripping, match
fragment cleanup logic from email.py, and use find() not rfind()
Both paths now produce identical multipart/alternative emails with clean
HTML + plain-text fallback, no model commentary leakage.
alt-glitch
added
type/bug
|
Clean implementation. The HTML detection regex covers common document openings (DOCTYPE, html) and block-level tags. The preamble stripping uses .search() to handle cron/model text before HTML. For fragments, walking backwards from block-level close tags correctly identifies trailing commentary. The plain-text fallback generation via tag stripping is straightforward. No issues found. |
1 similar comment
|
Clean implementation. The HTML detection regex covers common document openings (DOCTYPE, html) and block-level tags. The preamble stripping uses .search() to handle cron/model text before HTML. For fragments, walking backwards from block-level close tags correctly identifies trailing commentary. The plain-text fallback generation via tag stripping is straightforward. No issues found. |
|
Overlap note: This PR targets the old I have PR #54107 which adds HTML rendering (multipart/alternative with markdown→HTML conversion) to the new plugin path's attachment send paths. Your HTML detection and preamble stripping logic (Bug 2 in send_message_tool.py) is complementary — my PR only covers the email adapter, not the send_message tool. Happy to coordinate: I can rebase #54107 after yours lands, or we can combine approaches. |
Add HTML email rendering to _send_email_with_attachment and _send_email_with_attachments. Previously only _send_email supported multipart/alternative with HTML; attachment paths sent plain text only. - Add _attach_body/_create_body_part/_attach_parts helpers - Add _style_html_email with inline CSS for Gmail/Outlook compat - Add _HTML_PREFIX/_HERMES_EMAIL_FOOTER HTML wrapper templates - Add html_format config option (default: true, opt-out: false) - Both attachment paths now use _create_body_part for HTML support - _send_email uses _attach_body (consolidated, no duplicated logic) Lazy markdown import — adapter works without markdown installed. Graceful fallback: conversion failure → plain text + warning. Supersedes NousResearch#46619 (old gateway path) and NousResearch#54073 (bundled scope). Refs: NousResearch#11941, NousResearch#36853
|
Thanks for the concrete HTML-detection and cleanup work. The underlying plaintext behavior is still present on current main, but this branch no longer reaches the active delivery code. Problems
Suggested changes
Automated hermes-sweeper review. |
alt-glitch
added
comp/plugins
…ping) Addresses sweeper feedback on NousResearch#36853: when the body already contains HTML (from cron/model output), detect it, strip preamble text and trailing commentary, and send as multipart/alternative. Changes: - Add _HTML_RE and _BLOCK_CLOSE_RE regex constants for HTML detection - Update _attach_parts() to check for pre-existing HTML before converting markdown to HTML - Strip preamble before first HTML tag - Use find() (first </html>) not rfind() for duplicate tag handling - Strip trailing model commentary after last closing block tag - Generate plain-text fallback by stripping HTML tags Tests: - HTML document with preamble → preamble stripped - HTML fragment (no </html>) → commentary stripped - Duplicate </html> → first occurrence used - Plain text → not detected as HTML Refs: NousResearch#36853, NousResearch#54107
|
Ported to new adapter path: PR #54107 now covers all 4 active send paths in
Latest push adds the preamble/fragment handling from this PR:
Regression tests added:
Supersedes #36853 (old |
alt-glitch
added
the
sweeper:risk-message-delivery
teknium1
added
sweeper:risk-compatibility
|
Overlap note (2026-08-03): this PR addresses the same topic (HTML email delivery). PR #73294 (swissly) consolidates Markdown-to-HTML rendering for ALL 4 send paths + allowlist sanitizer + 20 tests, and was just rebased onto current main (commit 9529ee30). Please review #73294 for consolidation rather than duplicating send-path fixes. |
|
Consolidation update from #73294 (swissly, 2026-08-03): Your HTML-body handling (preamble/postamble trimming) is valuable — I've ported it into the consolidated PR #73294 on the current adapter path. Context: your diff targets What was ported (with attribution, commit 592c9b2fc): #73294 now covers: Markdown-to-HTML rendering + allowlist sanitizer + preamble/postamble trimming + SMTP_SSL 465 fix, all 4 send paths, 61 tests green. Recommend closing #36853 as consolidated — happy to answer questions or adjust the port. |
Ported from PR NousResearch#36853 (chtse53): cron wrappers ('Cronjob Response: <name>') and model commentary before the first HTML tag, plus trailing prose after </html> or the last block-level tag, are stripped before sanitizing an already-HTML body. Prevents broken plain-text wrappers from rendering inside HTML emails. NousResearch#36853 targets gateway/platforms/email.py which no longer exists on main (adapters migrated to bundled plugins in 5600105); this ports the feature to plugins/platforms/email/adapter.py. Contributor credit kept. 5 regression tests.
alt-glitch
added
comp/gateway
Ported from PR NousResearch#36853 (chtse53): cron wrappers ('Cronjob Response: <name>') and model commentary before the first HTML tag, plus trailing prose after </html> or the last block-level tag, are stripped before sanitizing an already-HTML body. Prevents broken plain-text wrappers from rendering inside HTML emails. NousResearch#36853 targets gateway/platforms/email.py which no longer exists on main (adapters migrated to bundled plugins in 5600105); this ports the feature to plugins/platforms/email/adapter.py. Contributor credit kept. 5 regression tests.
5 participants
Problem
Cron job emails containing HTML (daily briefings, book summaries) were sent as
text/plain, causing email clients to display raw HTML tags instead of rendering the HTML content.Two bugs caused this:
Bug 1:
gateway/platforms/email.py—_attach_body()rfind("</html>")picked the last closing tag, but models sometimes produce duplicate</html>tags with garbage text between them</html>), only stripped narrow"Cronjob Response"footer patterns — model commentary like"The previous response was already complete..."leaked through into the HTML bodyBug 2:
tools/send_message_tool.py—_send_email()^anchor:^\s*(?:<!DOCTYPE\s+html|<html[\s>])—.match()only detected HTML at position 0text/plainFix
email.py— new_attach_body()helper_HTML_REregex matching<!DOCTYPE html>,<html>, and common block-level tags (no^anchor)find()instead ofrfind()for</html>— uses first occurrence to avoid duplicate-tag garbage</html>, walks backwards from the last closing block-level tag to detect and strip trailing model commentarymultipart/alternativewith bothtext/plain(auto-generated fallback) andtext/htmlsend_message_tool.py— matching HTML detection_HTML_PATTERNto use.search()(no^anchor)find()for</html>, and block-tag fragment cleanup — same logic asemail.pymultipart/alternativeemailsTesting
Verified with actual cron output (15K-char daily briefing = HTML fragment, 24K-char book summary = HTML document with duplicate closing tags). Both now correctly detected as HTML, preamble stripped, trailing model commentary stripped, sent as
multipart/alternative.