feat: add a per-recipient Format option to convert HTML bodies (FMG-9) #13

Open
butterrobot wants to merge 3 commits from agent/full-stack-developer/7fb9f472e93e into main

2026-09-10

refactor: convert HTML with html-to-markdown instead of by hand (FMG-9)
All checks were successful
CI / goreleaser-lint (pull_request) Successful in 3s
CI / format (pull_request) Successful in 33s
CI / test (pull_request) Successful in 2m21s
CI / lint (pull_request) Successful in 3m6s
CI / build (pull_request) Successful in 3m13s
The hand-written renderer is replaced by
github.com/JohannesKaufmann/html-to-markdown/v2 plus the mail-specific policy
it has no opinion about. 986 lines of html.go become 372; the conversion
itself — CommonMark escaping, delimiter runs, fencing a code block past the
backticks inside it — is now a maintained library's problem rather than ours.

The original justification for writing it by hand was that the library would
drag in goquery and its dependencies. That was true of v1 and wrong for v2,
which dropped it: the measured cost is two modules, html-to-markdown/v2 and
JohannesKaufmann/dom, on top of the golang.org/x/net this already used.

What the library does not know is mail, because it is written for documents.
The parsed message is prepared before conversion:

- Hidden preheaders, written for the inbox list, are removed.
- Images without alt text go, which takes the tracking pixels, spacers and
  sliced-up banners with them. An inline cid: attachment leaves its alt text
  behind as ordinary words.
- Destinations a reader cannot open are dropped and the link text kept; tabs
  and line breaks are stripped from the rest, since a line break inside an
  href is invisible in the document and a fabricated line in the message.
- Table rows become lines and their cells stay apart, which the converter has
  no rule for: "Total4Failed0" otherwise.
- A link left holding nothing but a pixel falls back to its own destination
  rather than rendering as an invisible "[](url)".
- Quote and list nesting is flattened past six levels, and the output is
  capped at 64 KiB with a marker.

That last one is not something any of the candidates solved. html-to-markdown
amplifies exactly as the hand-written renderer did before it was capped, from
the same cause — a line prefix re-emitted per line and per level. Measured on
one message at the server's own 1 MB limit, nested 250 deep: 131 MB of output
over 2m16s, against 64 KiB in 1.5s and 85 MiB of peak heap with the flattening
in place.

Format = "text" is dropped, leaving raw and markdown. Markdown reads as plain
text wherever nothing renders it, so a second conversion would only have been
a worse copy of this one, and the plain-text libraries surveyed were the weak
half of the field. Nothing has shipped with "text", so no released
configuration names it; an unknown Format is still refused at startup.

The test suite carries over almost unchanged, because it asserts output rather
than internals — which is what made the swap safe to judge. Every mail-policy
and injection case still holds, and the pathological-input test is sized from
the constants now so the suite stays quick.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-10 09:09:46 +00:00
fix: address review of the HTML conversion feature (FMG-9)
All checks were successful
CI / goreleaser-lint (pull_request) Successful in 3s
CI / format (pull_request) Successful in 2m5s
CI / lint (pull_request) Successful in 2m25s
CI / test (pull_request) Successful in 2m58s
CI / build (pull_request) Successful in 2m38s
Blockers

- Cap the rendered output at 64 KiB. Block prefixes are re-emitted on every
  line, so nesting multiplied against line count: one message at the server's
  own 1 MB limit rendered to 125 MB and 894 MiB of allocation, which OOM-kills
  the process in any modest container. Now 65 KB and 30 MiB, with a "…" so a
  cut message says so.
- Stop turning an undecodable transfer encoding into a permanent 550. Real
  mailers emit unpadded base64, and a 550 tells the sender to stop retrying, so
  mail that main delivered was lost for good. base64 and quoted-printable now
  degrade to whatever decoded — and to the bytes as they arrived — exactly as
  the charset path already did. One unreadable part no longer fails a message
  that has a perfectly good alternative in hand.
- Keep the media type mime.ParseMediaType returns alongside its error. A part
  with "charset=" or an unquoted attachment file name was dropped whole, which
  delivered an empty notification with a 250 for mail main forwarded, and made
  the new attachment guard fail open on the malformed forms older MUAs emit.

Renderer

- Merge two emphasis spans that meet with nothing between them, and drop a
  marker nested inside itself. Splitting a bolded label across two <b> runs is
  what every Word and Outlook export does, and it put a literal "**" in front
  of the reader — the exact thing this feature exists to prevent.
- Render code spans and fenced blocks from their text, so escapes stay literal
  where Markdown makes them literal, and fence past the longest backtick run
  inside. A sender could otherwise close the fence and have the rest of the
  message render as live Markdown — a heading or a link inside what the reader
  trusts as a forwarded notification.
- Strip line breaks from href and src and collapse them in alt text. A newline
  in a URL is invisible in the document and a fabricated line in the message,
  and it destroyed the destination as well.
- Keep the blank lines inside <pre>, which a diff and a stack trace are shaped
  by; recognise visibility:hidden, mso-hide:all and font-size:0 preheaders;
  de-duplicate a bare URL label on a line that opens with a bullet.
- Raise the depth guard above the parser's own limit on open elements, so
  content is never dropped silently between the two.

Elsewhere

- Forward the raw HTML when a body renders to nothing, as the render-error path
  already did. An image-only newsletter handed shoutrrr "", which Mattermost
  and Discord reject, so every target failed and the sender retried forever.
- Scope body selection to its container per RFC 2046: the parts of a
  multipart/alternative are one content in several forms, the parts of any
  other multipart are cumulative. "Plain beats HTML" applied across a mixed
  container picked a list's unsubscribe footer over the newsletter itself.
- Pass the message by pointer. Its body is a single-use stream cached on the
  value, so a copy would forward an empty notification to every recipient after
  the first.
- Normalize in BodyFormat.valid(), so a Config assembled in Go rather than
  loaded from a file does not fail on a Format nobody set.

Tests and docs

- A realistic transactional message pinned end to end in both formats. Every
  fault above lived in a combination of features rather than in one of them,
  which is why statement coverage did not catch any of them.
- TestRenderHTMLSurvivesBrokenMarkup asserts output rather than only err == nil,
  and the nesting-limit test asserts the cap rather than NotContains, which
  passed on empty output.
- README: the conversion section is its own, the startup rejection and the
  case-insensitivity are documented, the copy-paste example no longer enables a
  non-default, and "exactly as it arrived" is corrected — raw is decoded first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-10 07:28:45 +00:00

2026-09-09

feat: add a per-recipient Format option to convert HTML bodies (FMG-9)
All checks were successful
CI / goreleaser-lint (pull_request) Successful in 3s
CI / format (pull_request) Successful in 2m36s
CI / test (pull_request) Successful in 1m53s
CI / lint (pull_request) Successful in 5m39s
CI / build (pull_request) Successful in 5m35s
Targets that render no HTML — a Mattermost direct message, for one — used to
receive the markup of an HTML-only message verbatim. `Format` on a recipient
(or on `[CatchAll]`) now renders an HTML body as `text` or `markdown` before
it is forwarded; the default `raw` keeps forwarding the message unchanged.

Only an HTML body is ever rewritten: a message that arrived as plain text is
what the sender chose to write, and is forwarded untouched whatever the
recipient asked for.

Converting a body first required reading it correctly, which fixes three
faults that were also spoiling raw delivery:

- Content-Transfer-Encoding was never undone outside multipart parts, so a
  quoted-printable or base64 body reached the target as "=E2=80=99".
- A non-UTF-8 charset was forwarded as its raw bytes, turning every accented
  character into mojibake.
- The multipart walk did not descend into nested containers, so the
  multipart/mixed wrapping a multipart/alternative that every forwarded
  message is produced an empty notification.

Attachments are now skipped when picking the body, an empty text/plain
alternative no longer wins over the HTML the sender actually wrote, and a
multipart Content-Type without a boundary is refused as malformed rather than
silently yielding nothing.

The renderer is built on golang.org/x/net/html, already an indirect
dependency, and aims at chat and push notifications rather than at
reproducing the document: links keep their destination, images without alt
text and hidden preheaders are dropped, and layout tables become one line per
row. A body too deeply nested for the parser is forwarded unchanged, since
reformatting is a courtesy to the target rather than a condition of delivery.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 20:58:52 +00:00