fix(slack): support multiple file attachments in messages
This commit is contained in:
parent
4583f88626
commit
f95194ac01
@ -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", () => {
|
describe("resolveSlackMedia", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mockFetch = vi.fn();
|
mockFetch = vi.fn();
|
||||||
|
|||||||
@ -37,16 +37,19 @@ export async function fetchWithSlackAuth(url: string, token: string): Promise<Re
|
|||||||
return fetch(resolvedUrl, { redirect: "follow" });
|
return fetch(resolvedUrl, { redirect: "follow" });
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function resolveSlackMedia(params: {
|
export type SlackMediaRef = {
|
||||||
files?: SlackFile[];
|
|
||||||
token: string;
|
|
||||||
maxBytes: number;
|
|
||||||
}): Promise<{
|
|
||||||
path: string;
|
path: string;
|
||||||
contentType?: string;
|
contentType?: string;
|
||||||
placeholder: string;
|
placeholder: string;
|
||||||
} | null> {
|
};
|
||||||
|
|
||||||
|
export async function resolveSlackMediaAll(params: {
|
||||||
|
files?: SlackFile[];
|
||||||
|
token: string;
|
||||||
|
maxBytes: number;
|
||||||
|
}): Promise<SlackMediaRef[]> {
|
||||||
const files = params.files ?? [];
|
const files = params.files ?? [];
|
||||||
|
const results: SlackMediaRef[] = [];
|
||||||
for (const file of files) {
|
for (const file of files) {
|
||||||
const url = file.url_private_download ?? file.url_private;
|
const url = file.url_private_download ?? file.url_private;
|
||||||
if (!url) continue;
|
if (!url) continue;
|
||||||
@ -71,16 +74,26 @@ export async function resolveSlackMedia(params: {
|
|||||||
params.maxBytes,
|
params.maxBytes,
|
||||||
);
|
);
|
||||||
const label = fetched.fileName ?? file.name;
|
const label = fetched.fileName ?? file.name;
|
||||||
return {
|
results.push({
|
||||||
path: saved.path,
|
path: saved.path,
|
||||||
contentType: saved.contentType,
|
contentType: saved.contentType,
|
||||||
placeholder: label ? `[Slack file: ${label}]` : "[Slack file]",
|
placeholder: label ? `[Slack file: ${label}]` : "[Slack file]",
|
||||||
};
|
});
|
||||||
} catch {
|
} 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<SlackMediaRef | null> {
|
||||||
|
const all = await resolveSlackMediaAll(params);
|
||||||
|
return all[0] ?? null;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type SlackThreadStarter = {
|
export type SlackThreadStarter = {
|
||||||
|
|||||||
@ -44,7 +44,7 @@ import { resolveSlackAllowListMatch, resolveSlackUserAllowed } from "../allow-li
|
|||||||
import { resolveSlackEffectiveAllowFrom } from "../auth.js";
|
import { resolveSlackEffectiveAllowFrom } from "../auth.js";
|
||||||
import { resolveSlackChannelConfig } from "../channel-config.js";
|
import { resolveSlackChannelConfig } from "../channel-config.js";
|
||||||
import { normalizeSlackChannelType, type SlackMonitorContext } from "../context.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";
|
import type { PreparedSlackMessage } from "./types.js";
|
||||||
|
|
||||||
@ -331,12 +331,13 @@ export async function prepareSlackMessage(params: {
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
const media = await resolveSlackMedia({
|
const allMedia = await resolveSlackMediaAll({
|
||||||
files: message.files,
|
files: message.files,
|
||||||
token: ctx.botToken,
|
token: ctx.botToken,
|
||||||
maxBytes: ctx.mediaMaxBytes,
|
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;
|
if (!rawBody) return null;
|
||||||
|
|
||||||
const ackReaction = resolveAckReaction(cfg, route.agentId);
|
const ackReaction = resolveAckReaction(cfg, route.agentId);
|
||||||
@ -453,7 +454,7 @@ export async function prepareSlackMessage(params: {
|
|||||||
|
|
||||||
let threadStarterBody: string | undefined;
|
let threadStarterBody: string | undefined;
|
||||||
let threadLabel: string | undefined;
|
let threadLabel: string | undefined;
|
||||||
let threadStarterMedia: Awaited<ReturnType<typeof resolveSlackMedia>> = null;
|
let threadStarterMedia: SlackMediaRef[] = [];
|
||||||
if (isThreadReply && threadTs) {
|
if (isThreadReply && threadTs) {
|
||||||
const starter = await resolveSlackThreadStarter({
|
const starter = await resolveSlackThreadStarter({
|
||||||
channelId: message.channel,
|
channelId: message.channel,
|
||||||
@ -474,15 +475,15 @@ export async function prepareSlackMessage(params: {
|
|||||||
const snippet = starter.text.replace(/\s+/g, " ").slice(0, 80);
|
const snippet = starter.text.replace(/\s+/g, " ").slice(0, 80);
|
||||||
threadLabel = `Slack thread ${roomLabel}${snippet ? `: ${snippet}` : ""}`;
|
threadLabel = `Slack thread ${roomLabel}${snippet ? `: ${snippet}` : ""}`;
|
||||||
// If current message has no files but thread starter does, fetch starter's files
|
// If current message has no files but thread starter does, fetch starter's files
|
||||||
if (!media && starter.files && starter.files.length > 0) {
|
if (allMedia.length === 0 && starter.files && starter.files.length > 0) {
|
||||||
threadStarterMedia = await resolveSlackMedia({
|
threadStarterMedia = await resolveSlackMediaAll({
|
||||||
files: starter.files,
|
files: starter.files,
|
||||||
token: ctx.botToken,
|
token: ctx.botToken,
|
||||||
maxBytes: ctx.mediaMaxBytes,
|
maxBytes: ctx.mediaMaxBytes,
|
||||||
});
|
});
|
||||||
if (threadStarterMedia) {
|
if (threadStarterMedia.length > 0) {
|
||||||
logVerbose(
|
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
|
// Use thread starter media if current message has none
|
||||||
const effectiveMedia = media ?? threadStarterMedia;
|
const effectiveMedia = allMedia.length > 0 ? allMedia : threadStarterMedia;
|
||||||
|
|
||||||
const ctxPayload = finalizeInboundContext({
|
const ctxPayload = finalizeInboundContext({
|
||||||
Body: combinedBody,
|
Body: combinedBody,
|
||||||
@ -519,9 +520,15 @@ export async function prepareSlackMessage(params: {
|
|||||||
ThreadLabel: threadLabel,
|
ThreadLabel: threadLabel,
|
||||||
Timestamp: message.ts ? Math.round(Number(message.ts) * 1000) : undefined,
|
Timestamp: message.ts ? Math.round(Number(message.ts) * 1000) : undefined,
|
||||||
WasMentioned: isRoomish ? effectiveWasMentioned : undefined,
|
WasMentioned: isRoomish ? effectiveWasMentioned : undefined,
|
||||||
MediaPath: effectiveMedia?.path,
|
MediaPath: effectiveMedia[0]?.path,
|
||||||
MediaType: effectiveMedia?.contentType,
|
MediaType: effectiveMedia[0]?.contentType,
|
||||||
MediaUrl: effectiveMedia?.path,
|
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,
|
CommandAuthorized: commandAuthorized,
|
||||||
OriginatingChannel: "slack" as const,
|
OriginatingChannel: "slack" as const,
|
||||||
OriginatingTo: slackTo,
|
OriginatingTo: slackTo,
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user