Skip to content

feat(slack): add file attachment support#410

Open
duboff wants to merge 1 commit intoColeMurray:mainfrom
chattermill:feat/slack-file-attachments
Open

feat(slack): add file attachment support#410
duboff wants to merge 1 commit intoColeMurray:mainfrom
chattermill:feat/slack-file-attachments

Conversation

@duboff
Copy link
Copy Markdown
Contributor

@duboff duboff commented Mar 25, 2026

Summary

  • 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
  • Forwarded/quoted messages (Slack attachments) are extracted and included in prompt context
  • Adds downloadSlackFile utility to slack-client.ts for authenticated Slack file downloads

Test plan

  • Attach an image to a Slack message mentioning the bot — verify it is passed to the agent
  • Attach a non-image file — verify it is passed as type file
  • Attach a file over 5MB — verify it is skipped with a warning log
  • Forward a message to a bot thread — verify the forwarded text appears in prompt context
  • Attach more than 3 files — verify only the first 3 are processed

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This 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 handleDirectMessage for DMs, adds a downloadSlackFile utility, and includes forwarded/quoted Slack messages (attachments) in prompt context via a new formatAttachments helper. All previously flagged issues from earlier review rounds have been addressed.

Key changes:

  • downloadSlackFile in slack-client.ts: authenticated fetch with content-length pre-check and full body size guard — correctly implemented.
  • arrayBufferToBase64: uses chunked String.fromCharCode to avoid O(n²) string concatenation.
  • handleAppMention and handleDirectMessage now both download file attachments before the existing-session check so they're available on both code paths.
  • handleSlackEvent now dispatches message events with channel_type === "im" to handleDirectMessage.

Issue found:

  • When result.needsClarification || !result.repo is true, fileAttachments are downloaded but not included in the KV pending entry (in both handleAppMention around line 1104 and handleDirectMessage at line 1401). handleRepoSelection then calls startSessionAndSendPrompt without any fileAttachments, silently discarding all attached files when repo clarification is needed. The fileAttachments array needs to be stored in the pending KV entry and retrieved/passed in handleRepoSelection.

Important Files Changed

Filename Overview
packages/slack-bot/src/index.ts Adds handleDirectMessage, dispatches IM events, and extends handleAppMention with file attachment downloading, forwarded message formatting, and DM file attachment support. All previously flagged issues are addressed. One new P1 bug: fileAttachments are not persisted in the KV pending entry for the 'needs clarification' flow in both handleAppMention and handleDirectMessage, causing attachments to be silently dropped when repo selection is required.
packages/slack-bot/src/utils/slack-client.ts Adds downloadSlackFile utility with authenticated fetch, content-length pre-check, and full body size guard. Also extends getThreadMessages and getUserInfo type definitions with attachment and email fields. Implementation is correct and handles edge cases properly.

Comments Outside Diff (1)

  1. packages/slack-bot/src/index.ts, line 1401-1410 (link)

    P1 File attachments silently dropped in the "needs clarification" flow

    When result.needsClarification || !result.repo is true, fileAttachments are downloaded and base64-encoded, but then silently discarded: the KV pending entry stored here (pending:${channel}:${threadTs}) does not include them. When the user later selects a repo via the dropdown, handleRepoSelection (line ~1599) retrieves this KV entry and calls startSessionAndSendPrompt without any fileAttachments, so the attached files are never forwarded to the agent.

    The same problem exists in handleAppMention at line 1104–1114 for the app_mention clarification path.

    To fix this, fileAttachments should be included in the KV entry in both places:

    await env.SLACK_KV.put(
      pendingKey,
      JSON.stringify({
        message: messageText,
        userId,
        previousMessages,
        fileAttachments: fileAttachments.length > 0 ? fileAttachments : undefined,
      }),
      { expirationTtl: 3600 }
    );
    

    And in handleRepoSelection, fileAttachments should be extracted from the pending data and passed to startSessionAndSendPrompt:

    const { message: messageText, userId, previousMessages, channelName, channelDescription, fileAttachments } = pendingData as { ...; fileAttachments?: FileAttachment[] };
    // ...
    await startSessionAndSendPrompt(..., traceId, fileAttachments);
    
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/slack-bot/src/index.ts
    Line: 1401-1410
    
    Comment:
    **File attachments silently dropped in the "needs clarification" flow**
    
    When `result.needsClarification || !result.repo` is true, `fileAttachments` are downloaded and base64-encoded, but then silently discarded: the KV pending entry stored here (`pending:${channel}:${threadTs}`) does not include them. When the user later selects a repo via the dropdown, `handleRepoSelection` (line ~1599) retrieves this KV entry and calls `startSessionAndSendPrompt` without any `fileAttachments`, so the attached files are never forwarded to the agent.
    
    The same problem exists in `handleAppMention` at line 1104–1114 for the `app_mention` clarification path.
    
    To fix this, `fileAttachments` should be included in the KV entry in both places:
    
    ```
    await env.SLACK_KV.put(
      pendingKey,
      JSON.stringify({
        message: messageText,
        userId,
        previousMessages,
        fileAttachments: fileAttachments.length > 0 ? fileAttachments : undefined,
      }),
      { expirationTtl: 3600 }
    );
    ```
    
    And in `handleRepoSelection`, `fileAttachments` should be extracted from the pending data and passed to `startSessionAndSendPrompt`:
    ```
    const { message: messageText, userId, previousMessages, channelName, channelDescription, fileAttachments } = pendingData as { ...; fileAttachments?: FileAttachment[] };
    // ...
    await startSessionAndSendPrompt(..., traceId, fileAttachments);
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/slack-bot/src/index.ts
Line: 1401-1410

Comment:
**File attachments silently dropped in the "needs clarification" flow**

When `result.needsClarification || !result.repo` is true, `fileAttachments` are downloaded and base64-encoded, but then silently discarded: the KV pending entry stored here (`pending:${channel}:${threadTs}`) does not include them. When the user later selects a repo via the dropdown, `handleRepoSelection` (line ~1599) retrieves this KV entry and calls `startSessionAndSendPrompt` without any `fileAttachments`, so the attached files are never forwarded to the agent.

The same problem exists in `handleAppMention` at line 1104–1114 for the `app_mention` clarification path.

To fix this, `fileAttachments` should be included in the KV entry in both places:

```
await env.SLACK_KV.put(
  pendingKey,
  JSON.stringify({
    message: messageText,
    userId,
    previousMessages,
    fileAttachments: fileAttachments.length > 0 ? fileAttachments : undefined,
  }),
  { expirationTtl: 3600 }
);
```

And in `handleRepoSelection`, `fileAttachments` should be extracted from the pending data and passed to `startSessionAndSendPrompt`:
```
const { message: messageText, userId, previousMessages, channelName, channelDescription, fileAttachments } = pendingData as { ...; fileAttachments?: FileAttachment[] };
// ...
await startSessionAndSendPrompt(..., traceId, fileAttachments);
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "feat(slack): add file attachment support" | Re-trigger Greptile

* 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(
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.

P1 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.

Comment on lines +1312 to +1319
const promptResult = await sendPrompt(
env,
existingSession.sessionId,
promptContent,
`slack:${userId}`,
callbackContext,
traceId
);
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.

P1 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.

Comment on lines 1459 to 1470
const sessionResult = await startSessionAndSendPrompt(
env,
repo,
channel,
threadKey,
messageText,
userId,
previousMessages,
undefined,
undefined,
traceId
);
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.

P1 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.

Comment on lines +1281 to +1287
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);
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.

P2 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.

Suggested change
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.

Comment on lines +64 to +71
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);
}
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.

P2 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:

Suggested change
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.

Comment on lines +1258 to +1263
const rawText = event.text || "";
const messageText = rawText.trim();

if (!messageText) {
log.info("slack.dm.empty_message", { trace_id: traceId });
return;
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.

P2 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.

@ColeMurray
Copy link
Copy Markdown
Owner

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.
@duboff
Copy link
Copy Markdown
Contributor Author

duboff commented Mar 27, 2026

Superseded by #418 — same file attachment changes but without the DM handling code (now in #411).

@duboff duboff closed this Mar 27, 2026
@duboff duboff reopened this Mar 27, 2026
@duboff duboff force-pushed the feat/slack-file-attachments branch from 8437133 to 2ece0c2 Compare March 27, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants