Skip to content

fix(slack): support MEDIA attachments in send_message - #67219

Closed
HexLab98 wants to merge 2 commits into
NousResearch:mainfrom
HexLab98:fix/slack-send-message-media
Closed

fix(slack): support MEDIA attachments in send_message#67219
HexLab98 wants to merge 2 commits into
NousResearch:mainfrom
HexLab98:fix/slack-send-message-media

Conversation

@HexLab98

Copy link
Copy Markdown
Contributor

Summary

  • Slack agents could already send files in-channel via gateway MEDIA: delivery (files_upload_v2), but send_message explicitly omitted Slack media and reported it as unsupported — so models inconsistently refused PDF/file sends (including after succeeding earlier in the same thread).
  • Wire Slack into the native send_message media path: plugin _standalone_send uploads via files_upload_v2, with caption-on-upload for single captionable files (same pattern as Discord/WhatsApp).
  • Update restriction/warning copy and tests (test_slack_send_message_media.py; Signal media tests now use SMS as the non-media platform).

Test plan

  • scripts/run_tests.sh tests/tools/test_slack_send_message_media.py tests/tools/test_signal_media.py -q
  • In a live Slack gateway session, ask the agent to send_message a local PDF with MEDIA:/path/file.pdf to a channel and confirm the file appears
  • Confirm caption-only single-file sends put text in initial_comment (no duplicate text message)
  • Confirm in-channel reply MEDIA: delivery still works unchanged

HexLab98 added 2 commits July 19, 2026 00:36
Slack could already deliver files in-channel via the gateway, but
send_message omitted MEDIA for Slack and told the model it was
unsupported — causing agents to inconsistently refuse PDF sends.
Wire Slack through files_upload_v2 in the standalone sender.
Add standalone-sender media cases and route coverage; point the
non-media platform assertions at SMS now that Slack supports MEDIA.
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets platform/slack Slack app adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #17261, #21800, and #23913. This uses the plugin standalone sender and files_upload_v2; the other open implementations take different sender/upload paths.

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

Thanks for wiring the missing Slack media route. The premise is confirmed on current main: tools/send_message_tool.py:1035-1063 classifies Slack as non-media and invokes its standalone sender without media_files.

Problems

  • plugins/platforms/slack/adapter.py:5007 constructs the media AsyncWebClient without the existing _resolve_slack_proxy_url() / _apply_slack_proxy() setup. Current Slack clients apply that configuration at plugins/platforms/slack/adapter.py:1080-1082; media delivery would bypass configured proxy and NO_PROXY behavior.

Suggested changes

  • Apply _apply_slack_proxy(client, _resolve_slack_proxy_url()) after constructing the media client, and add coverage for that path.

Automated hermes-sweeper review.

)
}

client = _AsyncWebClient(token=token)

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.

This media client bypasses Slack's established proxy setup. Please apply _apply_slack_proxy(client, _resolve_slack_proxy_url()) here, as the connected clients do at current-main adapter.py:1080-1082, and add a proxy-path test.

@teknium1 teknium1 added 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 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69482 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your MEDIA-in-send_message implementation (caption-on-upload, per-file warnings, chunked routing, tests) won the three-way adjudication and was cherry-picked directly.

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/slack Slack app 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants