fix(slack): download all files in multi-image messages (#15447)

* fix(slack): download all files in multi-image messages

resolveSlackMedia() previously returned after downloading the first
file, causing multi-image Slack messages to lose all but the first
attachment. This changes the function to collect all successfully
downloaded files into an array, matching the pattern already used by
Telegram, Line, Discord, and iMessage adapters.

The prepare handler now populates MediaPaths, MediaUrls, and
MediaTypes arrays so downstream media processing (vision, sandbox
staging, media notes) works correctly with multiple attachments.

Fixes #11892, #7536

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(slack): preserve MediaTypes index alignment with MediaPaths/MediaUrls

The filter(Boolean) on MediaTypes removed entries with undefined contentType,
shrinking the array and breaking index correlation with MediaPaths and MediaUrls.
Downstream code (media-note.ts, attachments.ts) requires these arrays to have
equal lengths for correct per-attachment MIME type lookup. Replace filter(Boolean)
with a nullish coalescing fallback to "application/octet-stream".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(slack): align MediaType fallback and tests (#15447) (thanks @CommanderCrowCode)

* fix: unblock plugin-sdk account-id typing (#15447)

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Tanwa Arpornthip
2026-02-14 20:16:02 +07:00
committed by GitHub
parent ef70a55b7a
commit c76288bdf1
7 changed files with 107 additions and 43 deletions

View File

@@ -267,6 +267,7 @@ describe("resolveSlackMedia", () => {
});
expect(result).not.toBeNull();
expect(result).toHaveLength(1);
// saveMediaBuffer should receive the overridden audio/mp4
expect(saveMediaBufferMock).toHaveBeenCalledWith(
expect.any(Buffer),
@@ -276,7 +277,7 @@ describe("resolveSlackMedia", () => {
);
// Returned contentType must be the overridden value, not the
// re-detected video/mp4 from saveMediaBuffer
expect(result!.contentType).toBe("audio/mp4");
expect(result![0]?.contentType).toBe("audio/mp4");
});
it("preserves original MIME for non-voice Slack files", async () => {
@@ -304,12 +305,14 @@ describe("resolveSlackMedia", () => {
});
expect(result).not.toBeNull();
expect(result).toHaveLength(1);
expect(saveMediaBufferMock).toHaveBeenCalledWith(
expect.any(Buffer),
"video/mp4",
"inbound",
16 * 1024 * 1024,
);
expect(result![0]?.contentType).toBe("video/mp4");
});
it("falls through to next file when first file returns error", async () => {
@@ -338,8 +341,41 @@ describe("resolveSlackMedia", () => {
});
expect(result).not.toBeNull();
expect(result).toHaveLength(1);
expect(mockFetch).toHaveBeenCalledTimes(2);
});
it("returns all successfully downloaded files as an array", async () => {
vi.spyOn(mediaStore, "saveMediaBuffer")
.mockResolvedValueOnce({ path: "/tmp/a.jpg", contentType: "image/jpeg" })
.mockResolvedValueOnce({ path: "/tmp/b.png", contentType: "image/png" });
const responseA = new Response(Buffer.from("image a"), {
status: 200,
headers: { "content-type": "image/jpeg" },
});
const responseB = new Response(Buffer.from("image b"), {
status: 200,
headers: { "content-type": "image/png" },
});
mockFetch.mockResolvedValueOnce(responseA).mockResolvedValueOnce(responseB);
const result = await resolveSlackMedia({
files: [
{ url_private: "https://files.slack.com/a.jpg", name: "a.jpg" },
{ url_private: "https://files.slack.com/b.png", name: "b.png" },
],
token: "xoxb-test-token",
maxBytes: 1024 * 1024,
});
expect(result).toHaveLength(2);
expect(result![0].path).toBe("/tmp/a.jpg");
expect(result![0].placeholder).toBe("[Slack file: a.jpg]");
expect(result![1].path).toBe("/tmp/b.png");
expect(result![1].placeholder).toBe("[Slack file: b.png]");
});
});
describe("resolveSlackThreadHistory", () => {

View File

@@ -132,17 +132,28 @@ function resolveSlackMediaMimetype(
return mime;
}
export type SlackMediaResult = {
path: string;
contentType?: string;
placeholder: string;
};
const MAX_SLACK_MEDIA_FILES = 8;
/**
* Downloads all files attached to a Slack message and returns them as an array.
* Returns `null` when no files could be downloaded.
*/
export async function resolveSlackMedia(params: {
files?: SlackFile[];
token: string;
maxBytes: number;
}): Promise<{
path: string;
contentType?: string;
placeholder: string;
} | null> {
}): Promise<SlackMediaResult[] | null> {
const files = params.files ?? [];
for (const file of files) {
const limitedFiles =
files.length > MAX_SLACK_MEDIA_FILES ? files.slice(0, MAX_SLACK_MEDIA_FILES) : files;
const results: SlackMediaResult[] = [];
for (const file of limitedFiles) {
const url = file.url_private_download ?? file.url_private;
if (!url) {
continue;
@@ -169,16 +180,16 @@ export async function resolveSlackMedia(params: {
params.maxBytes,
);
const label = fetched.fileName ?? file.name;
return {
results.push({
path: saved.path,
contentType: effectiveMime ?? saved.contentType,
placeholder: label ? `[Slack file: ${label}]` : "[Slack file]",
};
});
} catch {
// Ignore download failures and fall through to the next file.
// Ignore download failures and try the next file.
}
}
return null;
return results.length > 0 ? results : null;
}
export type SlackThreadStarter = {

View File

@@ -342,7 +342,8 @@ export async function prepareSlackMessage(params: {
token: ctx.botToken,
maxBytes: ctx.mediaMaxBytes,
});
const rawBody = (message.text ?? "").trim() || media?.placeholder || "";
const mediaPlaceholder = media ? media.map((m) => m.placeholder).join(" ") : undefined;
const rawBody = (message.text ?? "").trim() || mediaPlaceholder || "";
if (!rawBody) {
return null;
}
@@ -488,8 +489,9 @@ export async function prepareSlackMessage(params: {
maxBytes: ctx.mediaMaxBytes,
});
if (threadStarterMedia) {
const starterPlaceholders = threadStarterMedia.map((m) => m.placeholder).join(", ");
logVerbose(
`slack: hydrated thread starter file ${threadStarterMedia.placeholder} from root message`,
`slack: hydrated thread starter file ${starterPlaceholders} from root message`,
);
}
}
@@ -558,6 +560,10 @@ export async function prepareSlackMessage(params: {
// Use thread starter media if current message has none
const effectiveMedia = media ?? threadStarterMedia;
const firstMedia = effectiveMedia?.[0];
const firstMediaType = firstMedia
? (firstMedia.contentType ?? "application/octet-stream")
: undefined;
const inboundHistory =
isRoomish && ctx.historyLimit > 0
@@ -599,9 +605,17 @@ export async function prepareSlackMessage(params: {
ThreadLabel: threadLabel,
Timestamp: message.ts ? Math.round(Number(message.ts) * 1000) : undefined,
WasMentioned: isRoomish ? effectiveWasMentioned : undefined,
MediaPath: effectiveMedia?.path,
MediaType: effectiveMedia?.contentType,
MediaUrl: effectiveMedia?.path,
MediaPath: firstMedia?.path,
MediaType: firstMediaType,
MediaUrl: firstMedia?.path,
MediaPaths:
effectiveMedia && effectiveMedia.length > 0 ? effectiveMedia.map((m) => m.path) : undefined,
MediaUrls:
effectiveMedia && effectiveMedia.length > 0 ? effectiveMedia.map((m) => m.path) : undefined,
MediaTypes:
effectiveMedia && effectiveMedia.length > 0
? effectiveMedia.map((m) => m.contentType ?? "application/octet-stream")
: undefined,
CommandAuthorized: commandAuthorized,
OriginatingChannel: "slack" as const,
OriginatingTo: slackTo,