feat(slack): add file attachment support#410
Conversation
Greptile SummaryThis PR adds file attachment support (images and documents, up to 3 files at 5 MB each) to Slack messages, forwarding them as base64-encoded payloads to the coding agent. It also wires up Key changes:
Issue found:
Important Files Changed
|
packages/slack-bot/src/index.ts
Outdated
| * Handle direct messages (DMs) to the bot. | ||
| * Similar to app_mention but users don't need to @mention the bot in DMs. | ||
| */ | ||
| async function handleDirectMessage( |
There was a problem hiding this comment.
handleDirectMessage is dead code — never invoked
handleDirectMessage is defined here but is never called anywhere in the codebase (confirmed by searching the full src/ directory). handleSlackEvent (lines 910–931) only dispatches to handleAppMention for app_mention events and publishAppHome for app_home_opened events — there is no branch for message events with channel_type === "im".
As a result, DM file attachments (and the entire DM flow described in the PR) are completely non-functional. A dispatch branch needs to be added to handleSlackEvent:
// Handle direct messages (IMs)
if (event.type === "message" && event.channel_type === "im" && event.channel && event.ts) {
await handleDirectMessage(event, env, traceId);
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1234
Comment:
**`handleDirectMessage` is dead code — never invoked**
`handleDirectMessage` is defined here but is never called anywhere in the codebase (confirmed by searching the full `src/` directory). `handleSlackEvent` (lines 910–931) only dispatches to `handleAppMention` for `app_mention` events and `publishAppHome` for `app_home_opened` events — there is no branch for `message` events with `channel_type === "im"`.
As a result, DM file attachments (and the entire DM flow described in the PR) are completely non-functional. A dispatch branch needs to be added to `handleSlackEvent`:
```
// Handle direct messages (IMs)
if (event.type === "message" && event.channel_type === "im" && event.channel && event.ts) {
await handleDirectMessage(event, env, traceId);
}
```
How can I resolve this? If you propose a fix, please make it concise.
packages/slack-bot/src/index.ts
Outdated
| const promptResult = await sendPrompt( | ||
| env, | ||
| existingSession.sessionId, | ||
| promptContent, | ||
| `slack:${userId}`, | ||
| callbackContext, | ||
| traceId | ||
| ); |
There was a problem hiding this comment.
File attachments dropped when replying to existing DM session
Even once handleDirectMessage is wired up, file attachments on messages sent into an existing DM thread are silently dropped. sendPrompt is called without the attachments argument (line 1312-1319), even though event.files is available on the event. Compare this with the handleAppMention equivalent path which correctly downloads and passes fileAttachments via startSessionAndSendPrompt.
The files need to be downloaded earlier in the function (before the existing-session check) and forwarded here, similarly to how handleAppMention does it at lines 964–968.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1312-1319
Comment:
**File attachments dropped when replying to existing DM session**
Even once `handleDirectMessage` is wired up, file attachments on messages sent into an existing DM thread are silently dropped. `sendPrompt` is called without the `attachments` argument (line 1312-1319), even though `event.files` is available on the event. Compare this with the `handleAppMention` equivalent path which correctly downloads and passes `fileAttachments` via `startSessionAndSendPrompt`.
The files need to be downloaded earlier in the function (before the existing-session check) and forwarded here, similarly to how `handleAppMention` does it at lines 964–968.
How can I resolve this? If you propose a fix, please make it concise.
packages/slack-bot/src/index.ts
Outdated
| const sessionResult = await startSessionAndSendPrompt( | ||
| env, | ||
| repo, | ||
| channel, | ||
| threadKey, | ||
| messageText, | ||
| userId, | ||
| previousMessages, | ||
| undefined, | ||
| undefined, | ||
| traceId | ||
| ); |
There was a problem hiding this comment.
File attachments not passed to new DM sessions either
startSessionAndSendPrompt is called here without a fileAttachments argument, so even when starting a brand new session from a DM with attached files, the attachments are never forwarded to the agent. The call mirrors the old signature before the PR extended it with the optional fileAttachments parameter.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1459-1470
Comment:
**File attachments not passed to new DM sessions either**
`startSessionAndSendPrompt` is called here without a `fileAttachments` argument, so even when starting a brand new session from a DM with attached files, the attachments are never forwarded to the agent. The call mirrors the old signature before the PR extended it with the optional `fileAttachments` parameter.
How can I resolve this? If you propose a fix, please make it concise.
packages/slack-bot/src/index.ts
Outdated
| previousMessages = filtered | ||
| .map((m) => { | ||
| if (m.bot_id) return `[Bot]: ${m.text}`; | ||
| const name = m.user ? userNames.get(m.user) || m.user : "Unknown"; | ||
| return `[${name}]: ${m.text}`; | ||
| }) | ||
| .slice(-10); |
There was a problem hiding this comment.
formatAttachments not applied to DM thread history
The previousMessages mapping in handleDirectMessage (lines 1281–1287) doesn't call formatAttachments for Slack attachments on thread messages, while the equivalent code in handleAppMention (lines 985–986) was updated in this same PR to include them. This means forwarded/quoted messages in DM thread context are silently omitted.
| previousMessages = filtered | |
| .map((m) => { | |
| if (m.bot_id) return `[Bot]: ${m.text}`; | |
| const name = m.user ? userNames.get(m.user) || m.user : "Unknown"; | |
| return `[${name}]: ${m.text}`; | |
| }) | |
| .slice(-10); | |
| previousMessages = filtered | |
| .map((m) => { | |
| if (m.bot_id) return `[Bot]: ${m.text}`; | |
| const name = m.user ? userNames.get(m.user) || m.user : "Unknown"; | |
| const attachmentsText = m.attachments ? formatAttachments(m.attachments) : ""; | |
| return `[${name}]: ${m.text}${attachmentsText ? `\n${attachmentsText}` : ""}`; | |
| }) | |
| .slice(-10); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1281-1287
Comment:
**`formatAttachments` not applied to DM thread history**
The `previousMessages` mapping in `handleDirectMessage` (lines 1281–1287) doesn't call `formatAttachments` for Slack `attachments` on thread messages, while the equivalent code in `handleAppMention` (lines 985–986) was updated in this same PR to include them. This means forwarded/quoted messages in DM thread context are silently omitted.
```suggestion
previousMessages = filtered
.map((m) => {
if (m.bot_id) return `[Bot]: ${m.text}`;
const name = m.user ? userNames.get(m.user) || m.user : "Unknown";
const attachmentsText = m.attachments ? formatAttachments(m.attachments) : "";
return `[${name}]: ${m.text}${attachmentsText ? `\n${attachmentsText}` : ""}`;
})
.slice(-10);
```
How can I resolve this? If you propose a fix, please make it concise.
packages/slack-bot/src/index.ts
Outdated
| function arrayBufferToBase64(buffer: ArrayBuffer): string { | ||
| const bytes = new Uint8Array(buffer); | ||
| let binary = ""; | ||
| for (let i = 0; i < bytes.byteLength; i++) { | ||
| binary += String.fromCharCode(bytes[i]); | ||
| } | ||
| return btoa(binary); | ||
| } |
There was a problem hiding this comment.
arrayBufferToBase64 string concatenation is O(n²)
Building a binary string character-by-character with += creates a new string allocation on every iteration. For a 5 MB file this is ~5 million allocations. A more efficient approach chunks the bytes and uses String.fromCharCode.apply:
| function arrayBufferToBase64(buffer: ArrayBuffer): string { | |
| const bytes = new Uint8Array(buffer); | |
| let binary = ""; | |
| for (let i = 0; i < bytes.byteLength; i++) { | |
| binary += String.fromCharCode(bytes[i]); | |
| } | |
| return btoa(binary); | |
| } | |
| function arrayBufferToBase64(buffer: ArrayBuffer): string { | |
| const bytes = new Uint8Array(buffer); | |
| let binary = ""; | |
| const chunkSize = 8192; | |
| for (let i = 0; i < bytes.byteLength; i += chunkSize) { | |
| binary += String.fromCharCode(...bytes.subarray(i, i + chunkSize)); | |
| } | |
| return btoa(binary); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 64-71
Comment:
**`arrayBufferToBase64` string concatenation is O(n²)**
Building a binary string character-by-character with `+=` creates a new string allocation on every iteration. For a 5 MB file this is ~5 million allocations. A more efficient approach chunks the bytes and uses `String.fromCharCode.apply`:
```suggestion
function arrayBufferToBase64(buffer: ArrayBuffer): string {
const bytes = new Uint8Array(buffer);
let binary = "";
const chunkSize = 8192;
for (let i = 0; i < bytes.byteLength; i += chunkSize) {
binary += String.fromCharCode(...bytes.subarray(i, i + chunkSize));
}
return btoa(binary);
}
```
How can I resolve this? If you propose a fix, please make it concise.
packages/slack-bot/src/index.ts
Outdated
| const rawText = event.text || ""; | ||
| const messageText = rawText.trim(); | ||
|
|
||
| if (!messageText) { | ||
| log.info("slack.dm.empty_message", { trace_id: traceId }); | ||
| return; |
There was a problem hiding this comment.
File-only DM messages silently dropped with no user feedback
When a user sends a DM that contains only a file attachment (no text), messageText will be empty and the function returns after a silent log — the user gets no response. The handleAppMention counterpart at least sends back "Hi! Please include a message with your request."
Given this PR specifically adds file-attachment support, the intent is likely to allow files with minimal or no text. Consider either responding to the user (e.g. "Please add a text description with your file") or allowing a file-only message to proceed with a default prompt.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1258-1263
Comment:
**File-only DM messages silently dropped with no user feedback**
When a user sends a DM that contains only a file attachment (no text), `messageText` will be empty and the function returns after a silent log — the user gets no response. The `handleAppMention` counterpart at least sends back "Hi! Please include a message with your request."
Given this PR specifically adds file-attachment support, the intent is likely to allow files with minimal or no text. Consider either responding to the user (e.g. "Please add a text description with your file") or allowing a file-only message to proceed with a default prompt.
How can I resolve this? If you propose a fix, please make it concise.|
this looks like it has changes from the direct message #411 PR |
Users can attach files (images, documents) to Slack messages and they are forwarded to the coding agent. Supports up to 3 files per message, up to 5MB each. Images are typed as "image", other files as "file". Files too large or that fail to download are skipped gracefully. Also extracts text from forwarded/quoted Slack message attachments and includes them in the prompt context.
8437133 to
2ece0c2
Compare
Summary
image, other files asfileattachments) are extracted and included in prompt contextdownloadSlackFileutility toslack-client.tsfor authenticated Slack file downloadsTest plan
file