fix(slack): media delivery — send_message MEDIA, DM targets, explicit post-stream uploads, workspace-scoped files - #69482
Merged
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 5f42c57 all good! |
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.
Add early auth check in _handle_slack_message() that runs BEFORE any API calls (thread context fetch, user name resolution, file downloads) or file processing. Unauthorized users could previously trigger Slack API calls and file downloads before the runner's _is_user_authorized gate rejected them. Same pattern as Telegram fix #54164: build a SessionSource and check the runner's _is_user_authorized at the adapter level before event construction consumes resources. Fixes the gap where Slack has no adapter-level auth gate while Discord and Telegram have adapter-level allowlists.
The post-stream helper (_deliver_media_from_response) rescanned the already-streamed response and promoted bare local filesystem paths into real uploads via extract_local_files. Since the visible reply was already streamed verbatim, any bare path there is either text the user has seen or stale inspected/tool content — not an attachment request. On Slack this uploaded images from stale inspected content after otherwise clean replies. Post-stream delivery now honors only explicit MEDIA: directives. The non-streaming path in gateway/platforms/base.py keeps its bare-path auto-detect, because that path controls the visible text and strips the path from the reply when it attaches — auto-attach is intentional there. Regression tests: bare image/document paths in a streamed reply produce no upload; explicit MEDIA: tags still deliver. Fixes #20834
…paths Widen the #19237 send_message fix to the live adapter: send, _upload_file, send_multiple_images, send_image, send_video, send_document, send_exec_approval, send_slash_confirm, and send_clarify now route bare Slack user IDs (U.../W...) through a shared _ensure_dm_conversation helper before calling chat.postMessage / files_upload_v2, which reject user IDs. This closes the gap in #17261 where an attachment worked when replying in a thread but failed when directed at a user DM, and extends the DM-open fallback to clarify/approval Block Kit prompts so gated actions can reach a user directly. Resolution uses the workspace-scoped client (multi-workspace installs open the DM with the right bot token), caches per (team, user), and records the opened D... channel in the channel→team map. On failure the original target passes through so the downstream API call surfaces the real Slack error. Fixes #17261 Refs #19236
…olicy) Post-rebase consolidation over the merged C7/C16/C10 work: - _ensure_dm_conversation now records workspace ownership via _remember_channel_team and bounds _dm_conversation_cache (cap 5000) - module-level _slack_dm_cache (C7 standalone path) bounded oldest-first - _user_is_bot_cache (C10) bounded with _trim_oldest_dict_entries - caption-mode contract tests updated for the C7+C8 merged media path
alt-glitch
added
type/bug
This was referenced Jul 23, 2026
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slack media delivery now works everywhere:
send_messageuploads MEDIA attachments, bare user targets (slack:U…,slack:@handle) open DMs across every send path (including clarify/approval prompts), post-stream delivery uploads only explicitly-tagged files, and file events resolve against the owning workspace.Fixes #51198, #19236, #17261, #20834.
Changes
send_messageMEDIA: routed through the plugin's standalone sender →files_upload_v2, caption-on-upload, per-file warnings with caption fallback, media-on-last-chunk for chunked text (fix(slack): support MEDIA attachments in send_message #67219).users.listhandle resolution +conversations.openported into_resolve_slack_user_target(fix(slack): open DMs for user send targets #19237); widening:_ensure_dm_conversation(workspace-scoped, cached, fail-open) wired into send, uploads, images/video/document, approval, slash-confirm, and clarify paths — attachments AND prompts now reach bare user targets ([Bug]: Hermes can send attachment (media file) when replying in thread, but not as a direct message in channel or DM #17261)._deliver_media_from_responseno longer auto-detects local paths — explicitMEDIA:tags only (bug(gateway): post-stream media delivery can upload bare local paths not intentionally present in the visible reply #20834; non-streaming path intentionally keeps auto-detect, asymmetry documented)._channel_teamcache (fix(slack): resolve file events with cached workspace team #30456).Credits
Salvaged with authorship preserved: #67219 (@HexLab98), #19237 (@davesecops), #30456 (@robzolkos), #62932 + #55272 (@necoweb3).
Supersedes #51199 (@greptile — verified human account; hard-fails on missing files), #21800 (@tony88331 — EARLIEST implementation of MEDIA-in-send_message, credited as originator; targeted the pre-plugin architecture).
Deferred: #69185/#32315 (inbound image-context plumbing, 2,400+ lines cross-cutting — own cluster).
Validation
tests/gateway/ -q -k slackNote for merge sequencing: touches
_standalone_send— C7 (#cron delivery PR) touches the same function; whichever merges second will be rebased and reconciled manually.Infographic