Skip to content

feat(send_message): MEDIA tag support + DingTalk/Feishu native routing - #17371

Open
spike2204 wants to merge 1 commit into
NousResearch:mainfrom
spike2204:feat/send-message-tool-media
Open

spike2204 wants to merge 1 commit into
NousResearch:mainfrom
spike2204:feat/send-message-tool-media

Conversation

@spike2204

Copy link
Copy Markdown

Summary

Enhance send_message_tool to support rich-media attachments via MEDIA tags and bare file-path auto-detection, with native routing for DingTalk and Feishu platforms.

Changes

  • Update schema description with MEDIA attachment instructions (supported extensions, [[audio_as_voice]] directive)
  • Add two-stage media extraction: MEDIA:<path> tags + bare absolute paths via BasePlatformAdapter.extract_local_files()
  • Add native DingTalk routing block using dingtalk_send_proactive() with media upload support
  • Add native Feishu routing block with media_files passthrough
  • Rewrite _send_dingtalk() to prefer Robot OpenAPI (AppKey/Secret) over legacy DINGTALK_WEBHOOK_URL, with graceful fallback
  • Update platform support lists in error messages

Related

Part of the DingTalk adapter enhancement series — see #12769 for the umbrella PR.

Enhance send_message_tool to support rich-media attachments via MEDIA
tags and bare file-path auto-detection, with native routing for DingTalk
and Feishu platforms.

- Update schema description with MEDIA attachment instructions
- Add two-stage media extraction: MEDIA:<path> tags + bare absolute
  paths via BasePlatformAdapter.extract_local_files()
- Add native DingTalk routing block using dingtalk_send_proactive()
  with media upload support (image/video/audio/file)
- Add native Feishu routing block with media_files passthrough
- Rewrite _send_dingtalk() to prefer Robot OpenAPI (AppKey/Secret)
  over legacy DINGTALK_WEBHOOK_URL, with graceful fallback
- Update platform support lists in error messages to #
@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets platform/dingtalk DingTalk adapter platform/feishu Feishu / Lark adapter labels Apr 29, 2026
@alt-glitch

Copy link
Copy Markdown

Overlaps significantly with #14336 (proactive messaging + media support) and subsumes #14892 / #10392 (Feishu media routing fix). Consider consolidating with the #12769 umbrella series.

@spike2204

Copy link
Copy Markdown
Author

Hi @alt-glitch, thanks for the detailed cross-referencing! 🙏

You're spot on — this PR touches the send_message_tool.py to add native DingTalk and Feishu routing with media support, which overlaps with #14336's scope and subsumes #14892 / #10392.

We're the DingTalk (钉钉) Open Platform team at Alibaba@PeterGuy326 and @spike2204 are colleagues. This is the final piece of our fine-grained PR series, extracted from the original stacked #14336.

What this PR specifically does:

  1. MEDIA tag enhancement — adds MEDIA:/path/to/file tag support + bare absolute path auto-detection (BasePlatformAdapter.extract_local_files()) to the send_message tool schema
  2. DingTalk native routing — routes through dingtalk_send_proactive() (from feat(dingtalk): proactive messaging via Robot OpenAPI #17370) with full media upload support, instead of the limited legacy webhook-only path
  3. Feishu native routing — adds media_files passthrough to _send_feishu(), fixing fix(send_message): route media attachments through Feishu adapter #14892 / fix: enable Feishu media delivery in send_message #10392
  4. Legacy fallback — preserves DINGTALK_WEBHOOK_URL for single-group deployments without OpenAPI access

Regarding consolidation with #12769: We fully agree — all 8 PRs in this series (#17364#17371) are tracked under the umbrella #12769. Once reviewed, we'll close the original stacked PRs (#14333#14336). The 8 fine-grained PRs can be merged in any order since each is self-contained against main.

Looking forward to your feedback! 🚀

@spike2204

Copy link
Copy Markdown
Author

Hi @alt-glitch 👋 — just a gentle follow-up on this PR series (#17364#17371).

We'd really appreciate it if these could be prioritized for review. Here's why:

  1. Production-critical fixes — These PRs address real issues we're hitting in our production deployment serving thousands of DingTalk users daily (HTTP 429 storms from missing QPS throttle, lost conversational context from unparsed quoted-replies, broken media pipelines with 403s on signed OSS URLs, etc.).

  2. Clean, self-contained PRs — We've already re-structured the original stacked PRs (fix(dingtalk): adapter reliability — websockets proxy, card QPS throttle, inbound queue #14333feat(dingtalk): proactive messaging via Robot OpenAPI + send_message_tool media support #14336) into 8 independent, well-tested PRs against main, each reviewable and mergeable on its own — specifically to make the review process easier.

  3. Upstream alignment — We're the DingTalk Open Platform team at Alibaba. These changes are ported from our production connector and reflect official platform behaviors and best practices. We're committed to long-term maintenance and upstream contribution.

We're happy to hop on a quick call, do a live walkthrough, or address any feedback asynchronously — whatever works best for the team. Thanks again for all the work on Hermes! 🙏

@teknium1 teknium1 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.

Thanks for the focused media-routing work. Feishu attachment delivery is already present on current main, but the remaining DingTalk work needs a current-architecture salvage.

Problems

  • tools/send_message_tool.py:1362 imports dingtalk_send_proactive, but this PR changes only tools/send_message_tool.py and current main has no definition for that symbol. The proposed OpenAPI path cannot run as submitted.
  • Current main moved DingTalk and Feishu delivery into platform plugins. Feishu media already routes through its registered standalone sender at tools/send_message_tool.py:960-982; the former core helper targeted by this branch no longer exists.
  • The bare-path addition at PR tools/send_message_tool.py:284-286 appends paths directly. Current main's bare-path contract requires filter_local_delivery_paths() (gateway/platforms/base.py:3487-3497) before native upload.
  • The PR has no tests; its only changed path is tools/send_message_tool.py.

Suggested changes

  • Move the DingTalk OpenAPI upload/send implementation into plugins/platforms/dingtalk/adapter.py::_standalone_send, following the registered plugin sender contract.
  • Preserve current safe bare-path filtering and add DingTalk OpenAPI/webhook-fallback and media-routing coverage.

Automated hermes-sweeper review.


if client_id and client_secret:
try:
from gateway.platforms.dingtalk import dingtalk_send_proactive

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.

dingtalk_send_proactive is not defined by this one-file PR and is absent from current main. This import makes the intended OpenAPI route unavailable; salvage the sender and upload implementation into the DingTalk plugin's registered standalone sender instead.

# Stage 2 matters for the common case where the agent just pastes the
# file path it already knows (what the user asked about in this flow).
media_files, cleaned_message = BasePlatformAdapter.extract_media(message)
bare_paths, cleaned_message = BasePlatformAdapter.extract_local_files(cleaned_message)

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.

When salvaging this onto current main, pass extracted bare paths through BasePlatformAdapter.filter_local_delivery_paths() before adding them to media_files; current MEDIA-tag extraction is explicitly filtered before dispatch.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists platform/dingtalk DingTalk adapter platform/feishu Feishu / Lark adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants