[nitrnitr]

What changed

Handle bare <action-text-attachment> elements whose HTML is stored in a content attribute.

HEY can nest this element inside a Trix HTML attachment. Previously, ToMarkdown treated it as a file attachment and rendered 📎 attachment, dropping the embedded text and links.

Unnamed elements with non-empty content are now rendered through the existing bounded embedded-HTML path. Named elements remain file attachments.

Added regression coverage for both the nested HTML case and filename precedence.

Testing

• make check
• make build

[Copilot]

Pull request overview

Renders unnamed nested Action Text attachment HTML through the existing bounded Markdown path while preserving named file attachments.

Changes:

• Added nested attachment-content rendering.
• Added regression and filename-precedence tests.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
internal/htmlutil/markdown.go	Handles embedded Action Text attachment content.
internal/htmlutil/markdown_test.go	Tests nested HTML and filename precedence.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jeremy]

Rebased onto main as 6de1d33 so this applies cleanly again — #387 (decorative-image skipping) landed in the same three spots. The merge keeps #387's decorative check first in walkNode, findImages and the Markdown path, then the embedded-content branch from this PR, then the named-attachment fallback; renderActionTextAttachment is folded into main's actionTextAttachment rather than kept beside it. No behaviour change to what this PR does: an unnamed content attachment is rendered as embedded HTML, a named one stays a file attachment, and all five new tests pass as written. make check is green locally; the test workflow has not run on this PR yet (first-time-contributor approval), so the local run is the gate for now.

[robzolkos]

Thanks for the rendering fix. This recording compares the parent build with commit 6de1d33 using the same synthetic email on a local development server. Before: the email body is replaced by an attachment placeholder. After: the message text and link are visible in both the CLI and TUI. This demonstrates the nested-body rendering scenario; it is not verification of every attachment type or the authentication flow.

pilot.mp4

[jeremy]

🤖 Thank you for this, @nitrnitr! Emails whose body sits in a nested action-text-attachment were rendering as nothing, and this fixes that in every path that needs it.

The branch was a long way behind main, and its first CI run never started, so I've merged main into it (4392847) and CI has now run. Everything passes except one test, which needs a one-line update:

--- FAIL: TestToMarkdownNamedActionTextAttachmentIgnoresInlineContent
    markdown_test.go: ToMarkdown = "📄 report.pdf", want "📎 report.pdf"

Since this branch was opened, #532 started labelling attachments by kind, so a PDF now renders as 📄. Changing the expected string in internal/htmlutil/markdown_test.go to "📄 report.pdf" should turn CI green. The test is still checking the right thing: a named attachment's inline content is ignored.

The rest looks good to me:

• It's consistent across the walkers. ToText, ToMarkdown, MessageSourceText and ExtractImageURLs all treat an unnamed attachment with content as an embedded body, and a named one as a file. They also share parseEmbeddedContent's depth limit, so a chain of nested attachments still stops.
• The clip-source change is safe. Removing action-text-attachment from the exclusions in elementProvidesMessageSourceText doesn't let attachment captions count as source text, because walkMessageSourceNode returns on that element before it looks at the children.
• The tests mean something. The four tests for the new behaviour fail without the change, and the named-attachment test guards against rendering a file's content by mistake.

One optional thought: findAttachments only descends into an attachment's content when its content type is HTML (isHTMLContentType), but embeddedActionTextContent doesn't check the type. Using the same check in both would keep them in step. Rendering a non-HTML attachment's content is probably harmless, though, so it's your call.

With the test expectation updated, this is ready from my side. Thanks again!