Skip to content

feat(slack): native MEDIA file upload via files:write scope - #23913

Closed
dirtyren wants to merge 1 commit into
NousResearch:mainfrom
dirtyren:feat/slack-media-upload
Closed

feat(slack): native MEDIA file upload via files:write scope#23913
dirtyren wants to merge 1 commit into
NousResearch:mainfrom
dirtyren:feat/slack-media-upload

Conversation

@dirtyren

Copy link
Copy Markdown
Contributor

Summary

Adds native MEDIA:<path> file upload support for Slack in send_message_tool, matching the existing pattern for Discord, Matrix, Signal, Feishu, and Telegram.

Motivation

Slack bots with files:write scope can upload files directly to channels/threads. Previously, MEDIA: paths were silently dropped on Slack — this PR wires up the full upload path.

Changes

tools/send_message_tool.py

  • _send_slack — added thread_id parameter that was previously silently dropped
  • _send_slack_file — new async helper implementing the modern two-step Slack upload API:
    1. files.getUploadURLExternal — get a pre-signed upload URL + file ID
    2. PUT file bytes to the pre-signed URL
    3. files.completeUploadExternal — share the file to the channel/thread
    • Guesses MIME type via mimetypes so Slack renders PDFs, images, etc. correctly
    • Supports thread_id for posting into threads
    • 120s timeout for large file uploads
  • _send_slack_file_legacy — fallback to files.upload (multipart form) when getUploadURLExternal returns ok: false
  • _send_to_platform — new Slack MEDIA block added before the "non-media platforms" fallthrough:
    • Text chunks are sent first via _send_slack
    • Files are uploaded on the last chunk via _send_slack_file
    • Error propagation matches Discord/Matrix pattern
  • Error strings and tool description updated to include slack in the supported platforms list

tests/tools/test_send_message_tool.py

8 new tests in two classes:

TestSendSlackFile

  • test_returns_error_when_file_missing
  • test_modern_upload_happy_path
  • test_thread_id_passed_to_complete_payload
  • test_legacy_fallback_when_modern_endpoint_fails

TestSendToPlatformSlackMedia

  • test_slack_media_only_calls_send_slack_file
  • test_slack_text_and_media_sends_text_then_file
  • test_slack_media_thread_id_forwarded
  • test_slack_media_file_error_propagates

Usage

# Upload a PDF report to a Slack channel
send_message(
    target="slack:C0ATFHY907L",
    message="Here is the RCA report MEDIA:/home/ubuntu/reports/2026-05-11-rca.pdf"
)

# Upload into a thread
send_message(
    target="slack:C0ATFHY907L:1234567890.000100",
    message="Attached MEDIA:/tmp/report.pdf"
)

Requirements

The Slack bot token must have the files:write OAuth scope.

Test Results

8 passed in 11.65s

Extend send_message_tool to support MEDIA:<path> attachments on Slack,
using the modern two-step upload API (files.getUploadURLExternal +
files.completeUploadExternal) with automatic fallback to the legacy
files.upload endpoint.

Changes:
- _send_slack: add thread_id parameter (was silently dropped before)
- _send_slack_file: new helper -- modern 2-step upload, MIME detection,
  thread reply support, 120s timeout for large files
- _send_slack_file_legacy: fallback for workspaces where the modern
  endpoint returns ok=false
- _send_to_platform: add Slack MEDIA block (mirrors Discord/Matrix
  pattern) -- text chunks sent first, files uploaded on the last chunk
- Error strings and tool description updated to include slack

Requires the Slack bot to have the files:write OAuth scope.

Tests (8 new, all passing):
- TestSendSlackFile: missing file, modern happy path, thread_id
  forwarding, legacy fallback
- TestSendToPlatformSlackMedia: media-only, text+media, thread_id
  forwarded to both helpers, file error propagation
@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/tools Tool registry, model_tools, toolsets labels May 11, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tackling the Slack MEDIA: gap. The underlying bug is still present on current main, but this branch needs a targeted port before it can be salvaged.

Problems

  • Current main rejects media-only Slack delivery at tools/send_message_tool.py:1035-1040; text-plus-media is sent with an explicit omission warning at tools/send_message_tool.py:1043-1047.
  • The PR modifies the removed core _send_slack path. Slack now routes through plugins/platforms/slack/adapter.py::_standalone_send (tools/send_message_tool.py:1428-1429), so this cannot cherry-pick cleanly.
  • The live adapter already owns Slack upload behavior through files_upload_v2 (plugins/platforms/slack/adapter.py:1668-1695, 2293-2324). The PR commit also carries unrelated Signal, Telegram, Yuanbao, QQBot, email, and Kanban changes beyond the stated Slack feature.

Suggested changes

  • Re-scope the salvage to the Slack plugin standalone sender and the Slack MEDIA routing branch in _send_to_platform.
  • Reuse the adapter's files_upload_v2 conventions and add focused standalone-routing tests for media-only, caption/text, threads, and failures.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #69482 (merged): native files:write upload landed via #67219's files_upload_v2 path; your scope groundwork is credited.

Thanks for the work — it's credited in #69482's summary.

@teknium1 teknium1 closed this Jul 23, 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/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/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants