feat(slack): inbound + outbound media attachments#176
Conversation
WhatsApp was text-only: inbound media was dropped (only captions kept)
and the agent couldn't send files. Wire the channel to the existing
attachment infrastructure in both directions.
Inbound: images/video/documents are downloaded via Baileys
downloadMediaMessage and uploaded as session attachments (images become
vision input); captions ride along as text. Voice/audio notes are
transcribed via the agent's STT, and replies to a voice note are spoken
back as a WhatsApp voice note when TTS is configured. Media-only messages
now trigger the agent instead of being dropped. Media over the 25 MiB cap
is skipped.
Outbound: the agent's attachment_send deliveries (SSE 'attachment' event)
are routed to the matching Baileys send — image/video/document, and
ogg/opus audio as a native push-to-talk voice note.
No core/protocol/gateway changes — uses SDK uploadAttachment,
postMessage({attachments}), downloadAttachmentBytes, transcribeAudio,
synthesizeAudio. Bumps 0.2.0 -> 0.3.0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Discord was fully text-only: inbound attachments were ignored and the agent couldn't send files. Wire the channel to the existing attachment infrastructure both directions. Inbound: message attachments (guild messages + DM gateway dispatch) are fetched from the Discord CDN and uploaded as session attachments (images become vision input); audio attachments are transcribed via the agent's STT and appended as text. Media-only messages now trigger the agent. Attachments over the 25 MiB cap are skipped. Outbound: the agent's attachment_send deliveries (SSE 'attachment' event) are sent back as Discord file uploads via AttachmentBuilder, with any caption as the message content. Discord is bundled into the CLI (private package), so this ships with the next openhermit release rather than a standalone publish. Bumps 0.2.0 -> 0.3.0 for changelog clarity. Adds the first unit tests for this package. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Slack was fully text-only and even dropped file_share messages (the subtype guard returned early). Wire the channel to the existing attachment infrastructure both directions. Inbound: file_share uploads are now accepted; each file is fetched from url_private with bot-token auth and uploaded as a session attachment (images become vision input). Audio files are transcribed via STT and appended as text. Media-only messages now trigger the agent. Files over the 25 MiB cap are skipped. Outbound: the agent's attachment_send deliveries (SSE 'attachment' event) are uploaded back via files.uploadV2, into the originating thread when applicable. Needs files:read (download) and files:write (upload) bot scopes. Slack is bundled into the CLI (private package), so this ships with the next openhermit release. Bumps 0.2.0 -> 0.3.0 and adds the first unit tests (isProcessableMessage gating). Also corrects the manual, which previously claimed file support that did not exist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds media support across Slack, Discord, and WhatsApp adapters: file/download/upload APIs, inbound resolution (audio STT, non-audio session attachments), outbound agent attachment delivery via SSE, media-aware message filtering, tests, docs, and package bumps. ChangesChannel media handling across adapters
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/channels/slack/src/bot.ts (1)
18-24: ⚡ Quick winTrim text before processability check to avoid forwarding whitespace-only events.
Boolean(event.text)treats' 'as processable, but downstream bridge trims and drops it. Aligning this check avoids unnecessary dedupe/work inhandleMessageEvent.Proposed change
export function isProcessableMessage(event: SlackMessageEvent): boolean { if (event.subtype && event.subtype !== 'file_share') return false; if (event.bot_id) return false; if (!event.user) return false; const hasFiles = Array.isArray(event.files) && event.files.length > 0; - return Boolean(event.text) || hasFiles; + return Boolean(event.text?.trim()) || hasFiles; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/slack/src/bot.ts` around lines 18 - 24, The isProcessableMessage function currently treats whitespace-only event.text (e.g., " ") as processable; update the check to trim the text before evaluating processability so whitespace-only messages are considered non-processable. In isProcessableMessage, replace the Boolean(event.text) check with a trimmed-text check (e.g., event.text?.trim().length > 0) while preserving the existing file/sharing and bot/user filters and the hasFiles condition so messages with actual text or files remain processable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/channels/slack/package.json`:
- Line 4: The repository bumped `@openhermit/channel-slack` to 0.3.0 in
apps/channels/slack/package.json but downstream pins/lockfiles still reference
0.2.0; update any references to `@openhermit/channel-slack` (e.g., in
apps/gateway/package.json and package-lock.json) to 0.3.0 and regenerate the
lockfile (run npm install or npm ci/npm install --package-lock-only as
appropriate) so the CLI path and adapter consumers pull the new media behavior.
In `@apps/channels/slack/src/bridge.ts`:
- Around line 127-134: The current check uses file.size but may still download
oversized content; after calling this.slack.downloadFile(url) in the block that
assigns bytes, immediately validate bytes.length against MAX_MEDIA_BYTES and if
it exceeds the cap, log a skipping message (including file.name or file.id and
the actual bytes.length), discard/free the bytes and continue the loop; ensure
this path mirrors the existing oversized-file handling so oversized downloads
are not kept in memory.
---
Nitpick comments:
In `@apps/channels/slack/src/bot.ts`:
- Around line 18-24: The isProcessableMessage function currently treats
whitespace-only event.text (e.g., " ") as processable; update the check to
trim the text before evaluating processability so whitespace-only messages are
considered non-processable. In isProcessableMessage, replace the
Boolean(event.text) check with a trimmed-text check (e.g.,
event.text?.trim().length > 0) while preserving the existing file/sharing and
bot/user filters and the hasFiles condition so messages with actual text or
files remain processable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3bf6c5c-2587-4e0f-8681-07f858609717
📒 Files selected for processing (8)
apps/channels/slack/README.mdapps/channels/slack/package.jsonapps/channels/slack/src/bot.tsapps/channels/slack/src/bridge.tsapps/channels/slack/src/slack-api.tsapps/channels/slack/test/bot.test.tsdocs/channel-adapter.mddocs/manual/17-channels.md
Three fixes from PR review: 1. Version pin/lockfile: gateway optionalDependencies still pinned @openhermit/channel-slack@0.2.0 while the workspace moved to 0.3.0, so a fresh install wouldn't link the updated adapter. Bump the gateway pin and the lockfile to 0.3.0. 2. 25 MiB cap was only enforced against Slack's reported file.size, which can be missing or wrong — downloadFile() would then read the whole body into memory. Move the cap into SlackApi.downloadFile(url, maxBytes): reject an oversized content-length up front, then stream and abort the moment the body crosses the limit so an oversized/mislabeled file never fully lands in memory. The bridge passes MAX_MEDIA_BYTES (file.size stays as a cheap early-skip). 3. isProcessableMessage() now trims text, so a whitespace-only event with no files is no longer treated as processable. Adds tests: whitespace-only not processable; downloadFile rejects on content-length and aborts mid-stream over the cap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed all three points in 9b591fc:
Added tests: whitespace-only not processable, and Note on scope: the gateway pin gap (#2 in the original review) also affects the bundled Discord (#175) and Telegram (#177) adapters — their gateway pins are still |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/channels/slack/src/slack-api.ts`:
- Around line 52-55: downloadFile currently calls fetch(urlPrivate, ...) with no
timeout; add an AbortController and pass its signal into fetch
(fetch(urlPrivate, { headers: {...}, signal })) and start a timer (e.g.
configurable default like 10-15s) that calls controller.abort() on timeout;
clear the timer on success/final error, catch the abort error and rethrow a
clear timeout error (e.g. "downloadFile timed out") so callers fail fast, and
ensure existing usage of this.botToken and maxBytes inside downloadFile remains
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2fe24ef8-3b7e-4a13-a84c-bef710afa063
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
apps/channels/slack/src/bot.tsapps/channels/slack/src/bridge.tsapps/channels/slack/src/slack-api.tsapps/channels/slack/test/bot.test.tsapps/channels/slack/test/slack-api.test.tsapps/gateway/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/channels/slack/src/bot.ts
- apps/channels/slack/test/bot.test.ts
- apps/channels/slack/src/bridge.ts
The gateway optionalDependencies still pinned @openhermit/channel-discord at 0.2.0 while the workspace moved to 0.3.0, so a fresh install wouldn't link the updated adapter and the media changes wouldn't take effect in the bundled CLI. Bump the gateway pin and lockfile to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three review fixes: - package.json description no longer says "Text-only v1." (stale npm metadata after the 0.3.0 media/voice rollout). - On STT failure, log the detail but show the user a generic message instead of forwarding the raw error text into the chat. - Normalize the audio MIME (strip params like `; codecs=opus`) before push-to-talk detection, so `audio/ogg; codecs=opus` still sends as a voice note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A stalled Discord CDN connection during inbound attachment download could block the per-channel message queue indefinitely. Add a 15s AbortSignal.timeout() to the fetch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A stalled url_private connection during inbound file download could block the channel queue indefinitely. Add a 15s AbortSignal.timeout() to the downloadFile fetch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Added the remaining nitpick: a 15s |
…feat/slack-attachments
…eat/slack-attachments # Conflicts: # apps/gateway/package.json # docs/channel-adapter.md # package-lock.json
# Conflicts: # apps/gateway/package.json # docs/channel-adapter.md # package-lock.json
Ship the bundled channel media support — Discord/Slack/Telegram attachments (#175, #176, #177) and their gateway pins (now all 0.3.0) — in the published CLI. Also syncs the stale lockfile cli entry (0.9.0 -> 0.9.2) left by the earlier 0.9.1 bump. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Third channel in the attachment-support rollout (after #174 WhatsApp, #175 Discord).
Slack was fully text-only and even dropped
file_sharemessages (thesubtypeguard returned early). Wires the channel to the already-complete core attachment infrastructure both directions. No core/protocol/gateway changes.Inbound (Slack → agent)
file_shareevents are now accepted. Each file is fetched fromurl_privatewith bot-token auth and uploaded as a session attachment — images become vision input automatically.client.transcribeAudioand appended as[Transcribed voice message]text.Outbound (agent → Slack)
attachment_senddeliveries (SSEattachmentevent) are uploaded viafiles.uploadV2, into the originating thread when applicable.Notes
files:read(download inbound) +files:write(upload outbound).privatepackage bundled into theopenhermitCLI, so this ships with the next CLI release. Version bumped 0.2.0 → 0.3.0.isProcessableMessagegating).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
Tests