Skip to content

fix(wecom): add media attachment support to send_message tool#43493

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/wecom-send-message-media
Open

fix(wecom): add media attachment support to send_message tool#43493
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/wecom-send-message-media

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds native media attachment support to the WeCom send_message tool path. Previously, MEDIA:<file_path> directives targeting WeCom were silently dropped — only text was delivered.

Related Issue

Fixes #37364

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/send_message_tool.py: Added a WeCom media branch in _send_to_platform() before the "Non-media platforms" catch-all, routing media files through adapter._send_media_source(). Updated _send_wecom() to accept optional media_files parameter. Updated warning messages to include "wecom" in the supported platform list.
  • tests/tools/test_send_message_tool.py: Added TestSendWecomMedia class with 5 tests covering media+text, text-only, media-only, missing-file, and routing integration.

How to Test

  1. Configure a WeCom bot with valid credentials
  2. Run pytest tests/tools/test_send_message_tool.py::TestSendWecomMedia -xvs — all 5 tests should pass
  3. Run pytest tests/tools/test_send_message_tool.py -x — all existing tests should still pass (138 total)
  4. In a live session: use send_message with target="wecom" and a MEDIA:/path/to/image.png directive — the image should be uploaded and delivered before the text message

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 pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: tools/send_message_tool.py::_send_to_platform, tools/send_message_tool.py::_send_wecom, gateway/platforms/wecom.py::_send_media_source
  • Blast radius: LOW — single platform (WeCom) media path, no changes to existing text-only flow
  • Related patterns: mirrors the existing Signal/Matrix/Yuanbao/Feishu media branch pattern

WeCom's `_send_to_platform()` had no media branch — media files were
silently dropped with a warning.  Add a dedicated WeCom media path that
uploads each file via the adapter's `_send_media_source()` pipeline
before delivering the text message.

Fixes NousResearch#37364
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets platform/wecom WeCom / WeChat Work adapter labels Jun 10, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ APPROVE — genuine bug fix, not net-new capability

Bug or new capability? This is a genuine bug fix. The WeCom adapter already implements a full media pipeline: _send_media_source, send_image, send_image_file, send_document, send_voice, send_video — all backed by _prepare_outbound_media + _upload_media_bytes. However, the _send_wecom function in send_message_tool.py only called adapter.send() (text only), so even when media_files was passed to _send_to_platform for a WeCom target, it fell through to the generic "unsupported platform for media" error path. The adapter capability existed; the tool-layer wire-up did not. This is a broken-delivery bug.

Error handling on upload failure. _send_media_source wraps the upload in a try/except for both asyncio.TimeoutError and generic Exception, returns SendResult(success=False, error=...) on failure. The tool-layer code logs a warning on media failure but does not abort — it continues to send the text message. This is intentionally lenient (best-effort media delivery). For a P2 bug fix this is acceptable, though a stricter "fail on media error" option could be a follow-up.

File size limits. The adapter enforces WeCom's documented limits via _apply_file_size_limits:

  • Images: 10 MB (downgrades to file on exceed, not rejected)
  • Video: 10 MB (downgrades to file)
  • Voice: 2 MB
  • Files: 20 MB hard limit (rejected with error message)
  • Absolute max: 20 MB
    These match WeCom's documented API constraints. No hard-coded limits in the new tool-layer code; it defers entirely to the adapter.

Media type coverage. _send_media_source handles images, video, voice (audio), and documents (files) via _prepare_outbound_media, which detects content type from magic bytes / extension. The _is_voice flag from media_files tuples is not forwarded to _send_media_source (the adapter auto-detects). This is consistent with how other platform adapters in send_message_tool handle the flag (many ignore it and let type detection win). Not a regression.

File-not-found handling. The tool checks os.path.exists(media_path) before calling the adapter and logs+skips missing files while still delivering the text. This prevents silent crashes on stale temp paths.

Backward compatibility. _send_wecom signature adds media_files=None — fully backward compatible. The "only supported for …" error strings are updated to include wecom.

Tests. Five tests: with media, without media (backward compat), media-only (no text), file-not-found skip, and platform routing with media_files. All key paths covered.

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 platform/wecom WeCom / WeChat Work adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WeCom] send_message MEDIA files silently dropped — no media branch in _send_to_platform

3 participants