Skip to content

fix(tools): regex-fallback Matrix formatted_body when markdown lib missing - #32492

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/matrix-send-markdown-fallback-32486
Closed

fix(tools): regex-fallback Matrix formatted_body when markdown lib missing#32492
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/matrix-send-markdown-fallback-32486

Conversation

@briandevans

@briandevans briandevans commented May 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

_send_matrix() in tools/send_message_tool.py (the cron / send_message-tool path to Matrix) imports the optional markdown library and silently degrades to plain text when it isn't installed. The official Docker image installs [all] + [messaging] only, neither of which pulls Markdown — so out-of-the-box cron deliveries render in Element as raw ##, **, |...| despite PR #5271.

MatrixAdapter._markdown_to_html_fallback() already exists in gateway/platforms/matrix.py as a comprehensive regex converter (fenced code, inline code, headers, bold, italic, strikethrough, links, blockquotes, lists, hr) — it's a @staticmethod that depends only on re and html.escape, safe to import without mautrix installed.

Mirror MatrixAdapter._markdown_to_html()'s precedence in _send_matrix(): markdown library when available, regex fallback otherwise. The Element X heading→bold post-processing already runs after both branches.

Mirrors MatrixAdapter._markdown_to_html precedence: markdown lib > _markdown_to_html_fallback regex converter. Element X <h*><strong> rewrite applies to both branches.

Related Issue

Fixes #32486

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tools/send_message_tool.py — when import markdown fails inside _send_matrix(), fall back to MatrixAdapter._markdown_to_html_fallback() instead of silently dropping formatted_body. The Element X <h*><strong> post-processing is hoisted out of the success-only branch so it applies to both paths.
  • tests/tools/test_send_message_tool.py — new TestSendMatrixFormattedBody class with two cases: (1) markdown lib path produces <strong>Heading</strong> + <li>bullet</li> in formatted_body; (2) lib-missing path (patch.dict(sys.modules, {"markdown": None})) still produces <strong>Heading</strong> + <li>bullet</li>, with no raw ## Heading surviving into formatted_body.

How to Test

  1. Reproduction (without this PR): in a venv that lacks markdown, call _send_matrix(token, extra, room_id, "## H\n\n- a\n- b") and observe the captured aiohttp PUT payload — formatted_body and format are absent; Element renders raw markdown.

  2. Focused tests:

    uv run --with pytest --with pytest-xdist --with pytest-asyncio --with pytest-timeout \
      --with 'python-telegram-bot[webhooks]==22.6' --with 'discord.py[voice]==2.7.1' \
      --with 'aiohttp==3.13.3' --with 'markdown==3.10.2' \
      python3 -m pytest tests/tools/test_send_message_tool.py::TestSendMatrixFormattedBody -v
    
  3. Regression sweep:

    uv run [...same plugins...] python3 -m pytest tests/tools/test_send_message_tool.py -v
    uv run [...same plugins with mautrix==0.21.0...] python3 -m pytest tests/gateway/test_matrix.py -k markdown -v
    

    123 tests in test_send_message_tool.py pass; 29 markdown tests in test_matrix.py pass.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run focused tests for the touched code and all pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.x

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A (inline docstring updated to describe the fallback)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A (no schema change; only payload shape)

Screenshots / Logs

Before (no markdown lib in venv):

PUT .../send/m.room.message/...
{"msgtype": "m.text", "body": "## Heading\n\n- a\n- b"}

After (no markdown lib in venv):

PUT .../send/m.room.message/...
{
  "msgtype": "m.text",
  "body": "## Heading\n\n- a\n- b",
  "format": "org.matrix.custom.html",
  "formatted_body": "<strong>Heading</strong>...<ul><li>a</li><li>b</li></ul>..."
}

Related / Positioning

The reporter listed three options in #32486 — (1) ship markdown in the Docker image, (2) make markdown a hard dependency, (3) align the cron-path fallback with the interactive path's regex _markdown_to_html_fallback. This PR is option (3): smallest blast-radius change (no pyproject.toml / Dockerfile / uv.lock churn, no transitive-dep movement) and removes the asymmetry between the two Matrix HTML emitters at the same time. Options (1) and (2) remain viable as follow-ups if the maintainers prefer a dep-level fix; this PR doesn't block either.

PR #20259 (DanAsBjorn) proposes routing all Matrix text sends through the adapter — a wider scope that has been idle 21 days. If it lands, _send_matrix is removed wholesale and this fallback goes with it; the two PRs don't conflict.

Audited siblings: the only other import markdown as _md site in the repo is MatrixAdapter._markdown_to_html in gateway/platforms/matrix.py, which already has the regex fallback this PR copies. No widening needed.

Copilot AI review requested due to automatic review settings May 26, 2026 08:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves Matrix message rendering by always generating formatted_body HTML from markdown, using the optional markdown library when available and a shared regex-based fallback when it isn’t (notably in the default Docker image scenario mentioned in issue #32486).

Changes:

  • Update _send_matrix to produce formatted_body HTML via markdown or a fallback converter.
  • Apply Element X heading compatibility conversion for both rendering paths.
  • Add tests covering both the markdown-installed path and the fallback path.

Reviewed changes

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

File Description
tools/send_message_tool.py Always populate Matrix formatted_body using markdown rendering or a fallback converter.
tests/tools/test_send_message_tool.py Add regression tests for formatted_body generation with and without the markdown dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1601 to +1602
# Hide the markdown library to force the ImportError branch.
with patch.dict(sys.modules, {"markdown": None}):
Comment on lines +1392 to +1393
from gateway.platforms.matrix import MatrixAdapter
html = MatrixAdapter._markdown_to_html_fallback(message)
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets comp/gateway Gateway runner, session dispatch, delivery labels May 26, 2026
@briandevans
briandevans force-pushed the fix/matrix-send-markdown-fallback-32486 branch from 38fe6e4 to 0eacb6d Compare May 28, 2026 10:12
…ssing

The `_send_matrix()` cron-delivery path in tools/send_message_tool.py
imports the optional `markdown` library and silently degrades to plain
text when it isn't installed. The official Docker image installs
`[all]` + `[messaging]` only, neither of which pulls `Markdown` — so
out-of-the-box cron deliveries render in Element as raw `##`, `**`,
`|...|` despite PR NousResearch#5271.

`MatrixAdapter._markdown_to_html_fallback()` already exists in
gateway/platforms/matrix.py as a comprehensive regex converter for
exactly this case (fenced code, inline code, headers, bold, italic,
strikethrough, links, blockquotes, lists, hr). It's a staticmethod
that depends only on `re` and `html.escape`, safe to import without
mautrix installed.

Mirror `MatrixAdapter._markdown_to_html()`'s precedence in
`_send_matrix()`: prefer the markdown library when available, fall
back to the regex converter otherwise. Element X heading→bold
post-processing applies to both branches.

Fixes NousResearch#32486
@briandevans
briandevans force-pushed the fix/matrix-send-markdown-fallback-32486 branch from 0eacb6d to 3a8b0d4 Compare May 29, 2026 23:10
@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up.

@briandevans briandevans closed this Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Default Docker image missing markdown dep — cron delivery silently falls back to plain text despite PR #5271

3 participants