From 43bd219f1de3399851fb3fcdadc10838e9ad34ca Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 23 Jan 2026 04:54:02 +0000 Subject: [PATCH] fix: hydrate Slack thread root files (#1479) (thanks @travisirby) --- CHANGELOG.md | 1 + src/slack/monitor/media.ts | 42 ++-- .../prepare.thread-media.test.ts | 188 ++++++++++++++++++ src/slack/monitor/message-handler/prepare.ts | 121 +++++++---- 4 files changed, 299 insertions(+), 53 deletions(-) create mode 100644 src/slack/monitor/message-handler/prepare.thread-media.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 08427aa9f..9bd4f441d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Docs: https://docs.clawd.bot - Agents/TUI: honor user-pinned auth profiles during cooldown and preserve search picker ranking. (#1432) Thanks @tobiasbischoff. - Docs: fix gog auth services example to include docs scope. (#1454) Thanks @zerone0x. - Slack: read thread replies for message reads when threadId is provided (replies-only). (#1450) Thanks @rodrigouroz. +- Slack: hydrate thread root attachments for replies and include multi-file context. (#1479) Thanks @travisirby. - macOS: prefer linked channels in gateway summary to avoid false “not linked” status. - Providers: improve GitHub Copilot integration (enterprise support, base URL, and auth flow alignment). diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index 4a51e9abd..fb614be44 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -5,17 +5,28 @@ import { fetchRemoteMedia } from "../../media/fetch.js"; import { saveMediaBuffer } from "../../media/store.js"; import type { SlackFile } from "../types.js"; -export async function resolveSlackMedia(params: { - files?: SlackFile[]; - token: string; - maxBytes: number; -}): Promise<{ +export type SlackResolvedMedia = { path: string; contentType?: string; placeholder: string; -} | null> { +}; + +export function resolveSlackFilePlaceholder(files?: SlackFile[]): string | undefined { + if (!files || files.length === 0) return undefined; + const named = files.find((file) => file?.name?.trim()); + if (named?.name) return `[Slack file: ${named.name}]`; + return "[Slack file]"; +} + +export async function resolveSlackMediaList(params: { + files?: SlackFile[]; + token: string; + maxBytes: number; +}): Promise { const files = params.files ?? []; + const resolved: SlackResolvedMedia[] = []; for (const file of files) { + if (file.size && file.size > params.maxBytes) continue; const url = file.url_private_download ?? file.url_private; if (!url) continue; try { @@ -37,22 +48,23 @@ export async function resolveSlackMedia(params: { params.maxBytes, ); const label = fetched.fileName ?? file.name; - return { + resolved.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. } } - return null; + return resolved; } export type SlackThreadStarter = { - text: string; + text?: string; userId?: string; ts?: string; + files?: SlackFile[]; }; const THREAD_STARTER_CACHE = new Map(); @@ -71,14 +83,18 @@ export async function resolveSlackThreadStarter(params: { ts: params.threadTs, limit: 1, inclusive: true, - })) as { messages?: Array<{ text?: string; user?: string; ts?: string }> }; + })) as { + messages?: Array<{ text?: string; user?: string; ts?: string; files?: SlackFile[] }>; + }; const message = response?.messages?.[0]; const text = (message?.text ?? "").trim(); - if (!message || !text) return null; + const hasFiles = Boolean(message?.files && message.files.length > 0); + if (!message || (!text && !hasFiles)) return null; const starter: SlackThreadStarter = { - text, + text: text || undefined, userId: message.user, ts: message.ts, + files: message.files, }; THREAD_STARTER_CACHE.set(cacheKey, starter); return starter; diff --git a/src/slack/monitor/message-handler/prepare.thread-media.test.ts b/src/slack/monitor/message-handler/prepare.thread-media.test.ts new file mode 100644 index 000000000..74c79f463 --- /dev/null +++ b/src/slack/monitor/message-handler/prepare.thread-media.test.ts @@ -0,0 +1,188 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../media.js", async () => { + const actual = await vi.importActual("../media.js"); + return { + ...actual, + resolveSlackMediaList: vi.fn(), + resolveSlackThreadStarter: vi.fn(), + }; +}); + +import type { App } from "@slack/bolt"; + +import type { ClawdbotConfig } from "../../../config/config.js"; +import type { RuntimeEnv } from "../../../runtime.js"; +import type { ResolvedSlackAccount } from "../../accounts.js"; +import type { SlackMessageEvent } from "../../types.js"; +import { createSlackMonitorContext } from "../context.js"; +import { resolveSlackMediaList, resolveSlackThreadStarter } from "../media.js"; +import { prepareSlackMessage } from "./prepare.js"; + +const account: ResolvedSlackAccount = { + accountId: "default", + enabled: true, + botTokenSource: "config", + appTokenSource: "config", + config: {}, +} as ResolvedSlackAccount; + +const createContext = () => { + const slackCtx = createSlackMonitorContext({ + cfg: { + agents: { defaults: { model: "anthropic/claude-opus-4-5", workspace: "/tmp/clawd" } }, + channels: { slack: { enabled: true } }, + } as ClawdbotConfig, + accountId: "default", + botToken: "token", + app: { client: {} } as App, + runtime: {} as RuntimeEnv, + botUserId: "B1", + teamId: "T1", + apiAppId: "A1", + historyLimit: 0, + sessionScope: "per-sender", + mainKey: "main", + dmEnabled: true, + dmPolicy: "open", + allowFrom: [], + groupDmEnabled: true, + groupDmChannels: [], + defaultRequireMention: true, + groupPolicy: "open", + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + replyToMode: "off", + threadHistoryScope: "thread", + threadInheritParent: false, + slashCommand: { + enabled: false, + name: "clawd", + sessionPrefix: "slack:slash", + ephemeral: true, + }, + textLimit: 4000, + ackReactionScope: "off", + mediaMaxBytes: 1024, + removeAckAfterReply: false, + }); + slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" }); + slackCtx.resolveUserName = async () => ({ name: "Alice" }); + return slackCtx; +}; + +const mediaMock = vi.mocked(resolveSlackMediaList); +const starterMock = vi.mocked(resolveSlackThreadStarter); + +beforeEach(() => { + mediaMock.mockReset(); + starterMock.mockReset(); + mediaMock.mockResolvedValue([]); + starterMock.mockResolvedValue(null); +}); + +describe("prepareSlackMessage thread media", () => { + it("hydrates root files for thread replies without attachments", async () => { + const ctx = createContext(); + starterMock.mockResolvedValueOnce({ + text: "", + userId: "U2", + ts: "171234.000", + files: [{ name: "root.pdf" }], + }); + mediaMock.mockResolvedValueOnce([]).mockResolvedValueOnce([ + { + path: "/tmp/root.pdf", + contentType: "application/pdf", + placeholder: "[Slack file: root.pdf]", + }, + ]); + + const message: SlackMessageEvent = { + type: "message", + channel: "C1", + channel_type: "channel", + text: "", + user: "U1", + ts: "171234.111", + thread_ts: "171234.000", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message", wasMentioned: true }, + }); + + expect(prepared).not.toBeNull(); + expect(prepared?.ctxPayload.RawBody).toBe("[Slack file: root.pdf]"); + expect(prepared?.ctxPayload.MediaPath).toBe("/tmp/root.pdf"); + expect(prepared?.ctxPayload.MediaPaths).toEqual(["/tmp/root.pdf"]); + }); + + it("emits MediaPaths for multiple attachments", async () => { + const ctx = createContext(); + mediaMock.mockResolvedValueOnce([ + { + path: "/tmp/a.png", + contentType: "image/png", + placeholder: "[Slack file: a.png]", + }, + { + path: "/tmp/b.pdf", + contentType: "application/pdf", + placeholder: "[Slack file: b.pdf]", + }, + ]); + + const message: SlackMessageEvent = { + type: "message", + channel: "C1", + channel_type: "channel", + text: "hi", + user: "U1", + ts: "171234.111", + files: [{ name: "a.png" }, { name: "b.pdf" }], + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message", wasMentioned: true }, + }); + + expect(prepared?.ctxPayload.MediaPath).toBe("/tmp/a.png"); + expect(prepared?.ctxPayload.MediaPaths).toEqual(["/tmp/a.png", "/tmp/b.pdf"]); + expect(prepared?.ctxPayload.MediaTypes).toEqual(["image/png", "application/pdf"]); + expect(prepared?.ctxPayload.MediaUrls).toEqual(["/tmp/a.png", "/tmp/b.pdf"]); + }); + + it("keeps file-only messages when downloads fail", async () => { + const ctx = createContext(); + mediaMock.mockResolvedValueOnce([]); + + const message: SlackMessageEvent = { + type: "message", + channel: "C1", + channel_type: "channel", + text: "", + user: "U1", + ts: "171234.111", + files: [{ name: "doc.txt" }], + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx, + account, + message, + opts: { source: "message", wasMentioned: true }, + }); + + expect(prepared).not.toBeNull(); + expect(prepared?.ctxPayload.RawBody).toBe("[Slack file: doc.txt]"); + expect(prepared?.ctxPayload.MediaPath).toBeUndefined(); + }); +}); diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index c76ae4285..8f262bb47 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -39,7 +39,11 @@ 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 { + resolveSlackFilePlaceholder, + resolveSlackMediaList, + resolveSlackThreadStarter, +} from "../media.js"; import type { PreparedSlackMessage } from "./types.js"; @@ -290,11 +294,7 @@ export async function prepareSlackMessage(params: { ctx.logger.info({ channel: message.channel, reason: "no-mention" }, "skipping channel message"); if (ctx.historyLimit > 0) { const pendingText = (message.text ?? "").trim(); - const fallbackFile = message.files?.[0]?.name - ? `[Slack file: ${message.files[0].name}]` - : message.files?.length - ? "[Slack file]" - : ""; + const fallbackFile = resolveSlackFilePlaceholder(message.files) ?? ""; const pendingBody = pendingText || fallbackFile; if (pendingBody) { recordPendingHistoryEntry({ @@ -313,13 +313,80 @@ export async function prepareSlackMessage(params: { return null; } - const media = await resolveSlackMedia({ + const roomLabel = channelName ? `#${channelName}` : `#${message.channel}`; + const envelopeOptions = resolveEnvelopeFormatOptions(ctx.cfg); + + const mediaList = await resolveSlackMediaList({ files: message.files, token: ctx.botToken, maxBytes: ctx.mediaMaxBytes, }); - const rawBody = (message.text ?? "").trim() || media?.placeholder || ""; + const messageFilePlaceholder = resolveSlackFilePlaceholder(message.files); + + let threadStarter: Awaited> = null; + let threadStarterBody: string | undefined; + let threadLabel: string | undefined; + let threadStarterMediaList: Awaited> = []; + if (isThreadReply && threadTs) { + threadStarter = await resolveSlackThreadStarter({ + channelId: message.channel, + threadTs, + client: ctx.app.client, + }); + if (threadStarter?.text) { + const starterUser = threadStarter.userId + ? await ctx.resolveUserName(threadStarter.userId) + : null; + const starterName = starterUser?.name ?? threadStarter.userId ?? "Unknown"; + const starterWithId = `${threadStarter.text}\n[slack message id: ${ + threadStarter.ts ?? threadTs + } channel: ${message.channel}]`; + threadStarterBody = formatThreadStarterEnvelope({ + channel: "Slack", + author: starterName, + timestamp: threadStarter.ts ? Math.round(Number(threadStarter.ts) * 1000) : undefined, + body: starterWithId, + envelope: envelopeOptions, + }); + const snippet = threadStarter.text.replace(/\s+/g, " ").slice(0, 80); + threadLabel = `Slack thread ${roomLabel}${snippet ? `: ${snippet}` : ""}`; + } else { + threadLabel = `Slack thread ${roomLabel}`; + } + + if (!message.files?.length && threadStarter?.files?.length) { + threadStarterMediaList = await resolveSlackMediaList({ + files: threadStarter.files, + token: ctx.botToken, + maxBytes: ctx.mediaMaxBytes, + }); + if (threadStarterMediaList.length > 0) { + logVerbose( + `slack: hydrated thread starter files (${threadStarterMediaList.length}) from root message`, + ); + } + } + } + + const threadStarterFilePlaceholder = + !message.files?.length && threadStarter?.files?.length + ? resolveSlackFilePlaceholder(threadStarter.files) + : undefined; + + const effectiveMediaList = mediaList.length > 0 ? mediaList : threadStarterMediaList; + const effectiveMedia = effectiveMediaList[0]; + + const rawBody = + (message.text ?? "").trim() || + messageFilePlaceholder || + threadStarterFilePlaceholder || + effectiveMedia?.placeholder || + ""; if (!rawBody) return null; + const mediaPaths = effectiveMediaList.map((entry) => entry.path); + const mediaTypes = effectiveMediaList + .map((entry) => entry.contentType) + .filter(Boolean) as string[]; const ackReaction = resolveAckReaction(cfg, route.agentId); const ackReactionValue = ackReaction ?? ""; @@ -353,7 +420,6 @@ export async function prepareSlackMessage(params: { ) : null; - const roomLabel = channelName ? `#${channelName}` : `#${message.channel}`; const preview = rawBody.replace(/\s+/g, " ").slice(0, 160); const inboundLabel = isDirectMessage ? `Slack DM from ${senderName}` @@ -380,7 +446,6 @@ export async function prepareSlackMessage(params: { const storePath = resolveStorePath(ctx.cfg.session?.store, { agentId: route.agentId, }); - const envelopeOptions = resolveEnvelopeFormatOptions(ctx.cfg); const previousTimestamp = readSessionUpdatedAt({ storePath, sessionKey: route.sessionKey, @@ -431,33 +496,6 @@ export async function prepareSlackMessage(params: { ].filter((entry): entry is string => Boolean(entry)); const groupSystemPrompt = systemPromptParts.length > 0 ? systemPromptParts.join("\n\n") : undefined; - - let threadStarterBody: string | undefined; - let threadLabel: string | undefined; - if (isThreadReply && threadTs) { - const starter = await resolveSlackThreadStarter({ - channelId: message.channel, - threadTs, - client: ctx.app.client, - }); - if (starter?.text) { - const starterUser = starter.userId ? await ctx.resolveUserName(starter.userId) : null; - const starterName = starterUser?.name ?? starter.userId ?? "Unknown"; - const starterWithId = `${starter.text}\n[slack message id: ${starter.ts ?? threadTs} channel: ${message.channel}]`; - threadStarterBody = formatThreadStarterEnvelope({ - channel: "Slack", - author: starterName, - timestamp: starter.ts ? Math.round(Number(starter.ts) * 1000) : undefined, - body: starterWithId, - envelope: envelopeOptions, - }); - const snippet = starter.text.replace(/\s+/g, " ").slice(0, 80); - threadLabel = `Slack thread ${roomLabel}${snippet ? `: ${snippet}` : ""}`; - } else { - threadLabel = `Slack thread ${roomLabel}`; - } - } - const ctxPayload = finalizeInboundContext({ Body: combinedBody, RawBody: rawBody, @@ -483,9 +521,12 @@ export async function prepareSlackMessage(params: { ThreadLabel: threadLabel, Timestamp: message.ts ? Math.round(Number(message.ts) * 1000) : undefined, WasMentioned: isRoomish ? effectiveWasMentioned : undefined, - MediaPath: media?.path, - MediaType: media?.contentType, - MediaUrl: media?.path, + MediaPath: effectiveMedia?.path, + MediaType: effectiveMedia?.contentType, + MediaUrl: effectiveMedia?.path, + MediaPaths: mediaPaths.length > 0 ? mediaPaths : undefined, + MediaUrls: mediaPaths.length > 0 ? mediaPaths : undefined, + MediaTypes: mediaTypes.length > 0 ? mediaTypes : undefined, CommandAuthorized: commandAuthorized, OriginatingChannel: "slack" as const, OriginatingTo: slackTo,