From f95194ac014b8812e9170ec269d9f4b5fbc2a6b1 Mon Sep 17 00:00:00 2001 From: HirokiKobayashi-R Date: Thu, 29 Jan 2026 17:16:33 +0900 Subject: [PATCH] fix(slack): support multiple file attachments in messages --- src/slack/monitor/media.test.ts | 103 +++++++++++++++++++ src/slack/monitor/media.ts | 33 ++++-- src/slack/monitor/message-handler/prepare.ts | 31 +++--- 3 files changed, 145 insertions(+), 22 deletions(-) diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts index bfe70f005..ea1f3f033 100644 --- a/src/slack/monitor/media.test.ts +++ b/src/slack/monitor/media.test.ts @@ -156,6 +156,109 @@ describe("fetchWithSlackAuth", () => { }); }); +describe("resolveSlackMediaAll", () => { + beforeEach(() => { + mockFetch = vi.fn(); + globalThis.fetch = mockFetch as typeof fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + vi.resetModules(); + }); + + it("returns all successfully downloaded files", async () => { + // Mock the store module + vi.doMock("../../media/store.js", () => ({ + saveMediaBuffer: vi + .fn() + .mockResolvedValueOnce({ + path: "/tmp/first.jpg", + contentType: "image/jpeg", + }) + .mockResolvedValueOnce({ + path: "/tmp/second.png", + contentType: "image/png", + }), + })); + + const { resolveSlackMediaAll } = await import("./media.js"); + + const response1 = new Response(Buffer.from("image1"), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }); + const response2 = new Response(Buffer.from("image2"), { + status: 200, + headers: { "content-type": "image/png" }, + }); + + mockFetch.mockResolvedValueOnce(response1).mockResolvedValueOnce(response2); + + const result = await resolveSlackMediaAll({ + files: [ + { url_private: "https://files.slack.com/first.jpg", name: "first.jpg" }, + { url_private: "https://files.slack.com/second.png", name: "second.png" }, + ], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toHaveLength(2); + expect(result[0].path).toBe("/tmp/first.jpg"); + expect(result[1].path).toBe("/tmp/second.png"); + }); + + it("continues processing when one file fails", async () => { + // Mock the store module + vi.doMock("../../media/store.js", () => ({ + saveMediaBuffer: vi.fn().mockResolvedValue({ + path: "/tmp/second.jpg", + contentType: "image/jpeg", + }), + })); + + const { resolveSlackMediaAll } = await import("./media.js"); + + // First file fails, second succeeds + mockFetch.mockRejectedValueOnce(new Error("Network error")).mockResolvedValueOnce( + new Response(Buffer.from("image"), { + status: 200, + headers: { "content-type": "image/jpeg" }, + }), + ); + + const result = await resolveSlackMediaAll({ + files: [ + { url_private: "https://files.slack.com/first.jpg", name: "first.jpg" }, + { url_private: "https://files.slack.com/second.jpg", name: "second.jpg" }, + ], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toHaveLength(1); + expect(result[0].path).toBe("/tmp/second.jpg"); + }); + + it("returns empty array when all files fail", async () => { + const { resolveSlackMediaAll } = await import("./media.js"); + + mockFetch.mockRejectedValue(new Error("Network error")); + + const result = await resolveSlackMediaAll({ + files: [ + { url_private: "https://files.slack.com/first.jpg", name: "first.jpg" }, + { url_private: "https://files.slack.com/second.jpg", name: "second.jpg" }, + ], + token: "xoxb-test-token", + maxBytes: 1024 * 1024, + }); + + expect(result).toHaveLength(0); + }); +}); + describe("resolveSlackMedia", () => { beforeEach(() => { mockFetch = vi.fn(); diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index 2674e2d50..7425dd860 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -37,16 +37,19 @@ export async function fetchWithSlackAuth(url: string, token: string): Promise { +}; + +export async function resolveSlackMediaAll(params: { + files?: SlackFile[]; + token: string; + maxBytes: number; +}): Promise { const files = params.files ?? []; + const results: SlackMediaRef[] = []; for (const file of files) { const url = file.url_private_download ?? file.url_private; if (!url) continue; @@ -71,16 +74,26 @@ export async function resolveSlackMedia(params: { params.maxBytes, ); const label = fetched.fileName ?? file.name; - return { + results.push({ path: saved.path, contentType: saved.contentType, placeholder: label ? `[Slack file: ${label}]` : "[Slack file]", - }; + }); } catch { - // Ignore download failures and fall through to the next file. + // Ignore download failures and continue to the next file. } } - return null; + return results; +} + +/** @deprecated Use resolveSlackMediaAll for multiple file support */ +export async function resolveSlackMedia(params: { + files?: SlackFile[]; + token: string; + maxBytes: number; +}): Promise { + const all = await resolveSlackMediaAll(params); + return all[0] ?? null; } export type SlackThreadStarter = { diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 8a2a9e111..9c2f9f6ae 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -44,7 +44,7 @@ import { resolveSlackAllowListMatch, resolveSlackUserAllowed } from "../allow-li import { resolveSlackEffectiveAllowFrom } from "../auth.js"; import { resolveSlackChannelConfig } from "../channel-config.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "../context.js"; -import { resolveSlackMedia, resolveSlackThreadStarter } from "../media.js"; +import { resolveSlackMediaAll, resolveSlackThreadStarter, type SlackMediaRef } from "../media.js"; import type { PreparedSlackMessage } from "./types.js"; @@ -331,12 +331,13 @@ export async function prepareSlackMessage(params: { return null; } - const media = await resolveSlackMedia({ + const allMedia = await resolveSlackMediaAll({ files: message.files, token: ctx.botToken, maxBytes: ctx.mediaMaxBytes, }); - const rawBody = (message.text ?? "").trim() || media?.placeholder || ""; + const mediaPlaceholder = allMedia.length > 0 ? allMedia.map((m) => m.placeholder).join(" ") : ""; + const rawBody = (message.text ?? "").trim() || mediaPlaceholder || ""; if (!rawBody) return null; const ackReaction = resolveAckReaction(cfg, route.agentId); @@ -453,7 +454,7 @@ export async function prepareSlackMessage(params: { let threadStarterBody: string | undefined; let threadLabel: string | undefined; - let threadStarterMedia: Awaited> = null; + let threadStarterMedia: SlackMediaRef[] = []; if (isThreadReply && threadTs) { const starter = await resolveSlackThreadStarter({ channelId: message.channel, @@ -474,15 +475,15 @@ export async function prepareSlackMessage(params: { const snippet = starter.text.replace(/\s+/g, " ").slice(0, 80); threadLabel = `Slack thread ${roomLabel}${snippet ? `: ${snippet}` : ""}`; // If current message has no files but thread starter does, fetch starter's files - if (!media && starter.files && starter.files.length > 0) { - threadStarterMedia = await resolveSlackMedia({ + if (allMedia.length === 0 && starter.files && starter.files.length > 0) { + threadStarterMedia = await resolveSlackMediaAll({ files: starter.files, token: ctx.botToken, maxBytes: ctx.mediaMaxBytes, }); - if (threadStarterMedia) { + if (threadStarterMedia.length > 0) { logVerbose( - `slack: hydrated thread starter file ${threadStarterMedia.placeholder} from root message`, + `slack: hydrated ${threadStarterMedia.length} thread starter file(s) from root message`, ); } } @@ -492,7 +493,7 @@ export async function prepareSlackMessage(params: { } // Use thread starter media if current message has none - const effectiveMedia = media ?? threadStarterMedia; + const effectiveMedia = allMedia.length > 0 ? allMedia : threadStarterMedia; const ctxPayload = finalizeInboundContext({ Body: combinedBody, @@ -519,9 +520,15 @@ 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: effectiveMedia[0]?.path, + MediaType: effectiveMedia[0]?.contentType, + MediaUrl: effectiveMedia[0]?.path, + MediaPaths: effectiveMedia.length > 0 ? effectiveMedia.map((m) => m.path) : undefined, + MediaUrls: effectiveMedia.length > 0 ? effectiveMedia.map((m) => m.path) : undefined, + MediaTypes: + effectiveMedia.length > 0 + ? (effectiveMedia.map((m) => m.contentType).filter(Boolean) as string[]) + : undefined, CommandAuthorized: commandAuthorized, OriginatingChannel: "slack" as const, OriginatingTo: slackTo,