From d1cc54866d6deea8ef5f13b8d6540a28c18c7755 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 00:55:46 +0100 Subject: [PATCH] fix(slack): return non-image downloads as files --- CHANGELOG.md | 1 + docs/channels/slack.md | 4 +- extensions/slack/src/action-runtime.test.ts | 39 ++++++++++++++- extensions/slack/src/action-runtime.ts | 23 ++++++++- .../slack/src/actions.download-file.test.ts | 48 ++++++++++++++++++- extensions/slack/src/file-reference.ts | 15 ++++++ extensions/slack/src/monitor/media.test.ts | 18 +++---- extensions/slack/src/monitor/media.ts | 3 +- .../message-handler/prepare-content.ts | 3 +- .../monitor/message-handler/prepare.test.ts | 9 ++-- .../src/monitor/message-handler/prepare.ts | 9 ++-- extensions/slack/src/monitor/thread.ts | 3 +- 12 files changed, 150 insertions(+), 25 deletions(-) create mode 100644 extensions/slack/src/file-reference.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 79620828f4d..39b8342afdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Slack/files: return non-image `download-file` results as local file paths instead of image payloads, and include Slack file IDs in inbound file placeholders so agents can call `download-file`. Fixes #71212. Thanks @teamrazo. - Discord/replies: run `message_sending` plugin hooks for Discord reply delivery, including DM targets, so plugins can transform or cancel outbound Discord replies consistently with other channels. Fixes #59350. (#71094) Thanks @wei840222. - Control UI/commands: carry provider-owned thinking option ids/labels in session rows and defaults so fresh sessions show and accept dynamic modes such as `adaptive`, `xhigh`, and `max`. Fixes #71269. Thanks @Young-Khalil. - Image generation: make explicit `model=` overrides exact-only so failed `openai/gpt-image-2` requests no longer fall through to Gemini or other configured providers, and update `image_generate list` to mention OpenAI Codex OAuth as valid auth for `openai/gpt-image-2`. Fixes #71290 and #71231. Thanks @Young-Khalil and @steipete. diff --git a/docs/channels/slack.md b/docs/channels/slack.md index b3cb20ae311..1e4c3fd7971 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -436,7 +436,7 @@ Available action groups in current Slack tooling: | memberInfo | enabled | | emojiList | enabled | -Current Slack message actions include `send`, `upload-file`, `download-file`, `read`, `edit`, `delete`, `pin`, `unpin`, `list-pins`, `member-info`, and `emoji-list`. +Current Slack message actions include `send`, `upload-file`, `download-file`, `read`, `edit`, `delete`, `pin`, `unpin`, `list-pins`, `member-info`, and `emoji-list`. `download-file` accepts Slack file IDs shown in inbound file placeholders and returns image previews for images or local file metadata for other file types. ## Access control and routing @@ -606,7 +606,7 @@ Notes: - Slack file attachments are downloaded from Slack-hosted private URLs (token-authenticated request flow) and written to the media store when fetch succeeds and size limits permit. + Slack file attachments are downloaded from Slack-hosted private URLs (token-authenticated request flow) and written to the media store when fetch succeeds and size limits permit. File placeholders include the Slack `fileId` so agents can fetch the original file with `download-file`. Runtime inbound size cap defaults to `20MB` unless overridden by `channels.slack.mediaMaxMb`. diff --git a/extensions/slack/src/action-runtime.test.ts b/extensions/slack/src/action-runtime.test.ts index 0c12ebef143..fc9bd251748 100644 --- a/extensions/slack/src/action-runtime.test.ts +++ b/extensions/slack/src/action-runtime.test.ts @@ -5,7 +5,7 @@ import { parseSlackBlocksInput } from "./blocks-input.js"; const originalSlackActionRuntime = { ...slackActionRuntime }; const deleteSlackMessage = vi.fn(async (..._args: unknown[]) => ({})); -const downloadSlackFile = vi.fn(async (..._args: unknown[]) => null); +const downloadSlackFile = vi.fn(async (..._args: unknown[]): Promise => null); const editSlackMessage = vi.fn(async (..._args: unknown[]) => ({})); const getSlackMemberInfo = vi.fn(async (..._args: unknown[]) => ({})); const listSlackEmojis = vi.fn(async (..._args: unknown[]) => ({})); @@ -266,6 +266,43 @@ describe("handleSlackAction", () => { ); }); + it("returns non-image downloadFile results as file metadata instead of image content", async () => { + downloadSlackFile.mockResolvedValueOnce({ + path: "/tmp/openclaw-media/report.pdf", + contentType: "application/pdf", + placeholder: "[Slack file: report.pdf (fileId: F123)]", + }); + + const result = await handleSlackAction( + { + action: "downloadFile", + fileId: "F123", + }, + slackConfig(), + ); + + expect(result.content).toHaveLength(1); + expect(result.content[0]).toEqual( + expect.objectContaining({ + type: "text", + text: expect.stringContaining("/tmp/openclaw-media/report.pdf"), + }), + ); + expect(result.content.some((entry) => entry.type === "image")).toBe(false); + expect(result.details).toEqual( + expect.objectContaining({ + ok: true, + fileId: "F123", + path: "/tmp/openclaw-media/report.pdf", + contentType: "application/pdf", + media: { + mediaUrl: "/tmp/openclaw-media/report.pdf", + contentType: "application/pdf", + }, + }), + ); + }); + it("forwards resolved botToken to action functions instead of relying on config re-read", async () => { downloadSlackFile.mockResolvedValueOnce(null); await handleSlackAction({ action: "downloadFile", fileId: "F123" }, slackConfig()); diff --git a/extensions/slack/src/action-runtime.ts b/extensions/slack/src/action-runtime.ts index d9931b3a002..5ce3af67ee7 100644 --- a/extensions/slack/src/action-runtime.ts +++ b/extensions/slack/src/action-runtime.ts @@ -137,6 +137,10 @@ function readSlackBlocksParam(params: Record) { return slackActionRuntime.parseSlackBlocksInput(params.blocks); } +function isImageContentType(value: string | undefined): boolean { + return value?.trim().toLowerCase().startsWith("image/") === true; +} + export async function handleSlackAction( params: Record, cfg: OpenClawConfig, @@ -395,11 +399,28 @@ export async function handleSlackAction( error: "File could not be downloaded (not found, too large, or inaccessible).", }); } + if (!isImageContentType(downloaded.contentType)) { + return jsonResult({ + ok: true, + fileId, + path: downloaded.path, + contentType: downloaded.contentType, + placeholder: downloaded.placeholder, + media: { + mediaUrl: downloaded.path, + ...(downloaded.contentType ? { contentType: downloaded.contentType } : {}), + }, + }); + } return await imageResultFromFile({ label: "slack-file", path: downloaded.path, extraText: downloaded.placeholder, - details: { fileId, path: downloaded.path }, + details: { + fileId, + path: downloaded.path, + ...(downloaded.contentType ? { contentType: downloaded.contentType } : {}), + }, }); } default: diff --git a/extensions/slack/src/actions.download-file.test.ts b/extensions/slack/src/actions.download-file.test.ts index 8e23e2752c1..c73bad4e397 100644 --- a/extensions/slack/src/actions.download-file.test.ts +++ b/extensions/slack/src/actions.download-file.test.ts @@ -38,11 +38,12 @@ function makeSlackFileInfo(overrides?: Record) { }; } -function makeResolvedSlackMedia() { +function makeResolvedSlackMedia(overrides?: Record) { return { path: "/tmp/image.png", contentType: "image/png", placeholder: "[Slack file: image.png]", + ...overrides, }; } @@ -118,6 +119,51 @@ describe("downloadSlackFile", () => { expect(result).toEqual(makeResolvedSlackMedia()); }); + it("preserves non-image download metadata", async () => { + const client = createClient(); + client.files.info.mockResolvedValueOnce({ + file: makeSlackFileInfo({ + name: "report.pdf", + mimetype: "application/pdf", + url_private_download: "https://files.slack.com/files-pri/T1-F123/report.pdf", + }), + }); + resolveSlackMedia.mockResolvedValueOnce([ + makeResolvedSlackMedia({ + path: "/tmp/report.pdf", + contentType: "application/pdf", + placeholder: "[Slack file: report.pdf (fileId: F123)]", + }), + ]); + + const result = await downloadSlackFile("F123", { + client, + token: "xoxb-test", + maxBytes: 1024, + }); + + expect(resolveSlackMedia).toHaveBeenCalledWith({ + files: [ + { + id: "F123", + name: "report.pdf", + mimetype: "application/pdf", + url_private: undefined, + url_private_download: "https://files.slack.com/files-pri/T1-F123/report.pdf", + }, + ], + token: "xoxb-test", + maxBytes: 1024, + }); + expect(result).toEqual( + makeResolvedSlackMedia({ + path: "/tmp/report.pdf", + contentType: "application/pdf", + placeholder: "[Slack file: report.pdf (fileId: F123)]", + }), + ); + }); + it("returns null when channel scope definitely mismatches file shares", async () => { const client = createClient(); client.files.info.mockResolvedValueOnce({ diff --git a/extensions/slack/src/file-reference.ts b/extensions/slack/src/file-reference.ts new file mode 100644 index 00000000000..ddc46e412e9 --- /dev/null +++ b/extensions/slack/src/file-reference.ts @@ -0,0 +1,15 @@ +import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; +import type { SlackFile } from "./types.js"; + +export function formatSlackFileReference(file: SlackFile | undefined): string { + const name = normalizeOptionalString(file?.name) ?? "file"; + const fileId = normalizeOptionalString(file?.id); + return fileId ? `${name} (fileId: ${fileId})` : name; +} + +export function formatSlackFileReferenceList(files: readonly SlackFile[] | undefined): string { + if (!files?.length) { + return "file"; + } + return files.map((file) => formatSlackFileReference(file)).join(", "); +} diff --git a/extensions/slack/src/monitor/media.test.ts b/extensions/slack/src/monitor/media.test.ts index e5112a9482d..55bb6ce8887 100644 --- a/extensions/slack/src/monitor/media.test.ts +++ b/extensions/slack/src/monitor/media.test.ts @@ -558,8 +558,8 @@ describe("resolveSlackMedia", () => { 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" }, + { id: "FA", url_private: "https://files.slack.com/a.jpg", name: "a.jpg" }, + { id: "FB", url_private: "https://files.slack.com/b.png", name: "b.png" }, ], token: "xoxb-test-token", maxBytes: 1024 * 1024, @@ -567,9 +567,9 @@ describe("resolveSlackMedia", () => { expect(result).toHaveLength(2); expect(result![0].path).toBe("/tmp/a.jpg"); - expect(result![0].placeholder).toBe("[Slack file: a.jpg]"); + expect(result![0].placeholder).toBe("[Slack file: a.jpg (fileId: FA)]"); expect(result![1].path).toBe("/tmp/b.png"); - expect(result![1].placeholder).toBe("[Slack file: b.png]"); + expect(result![1].placeholder).toBe("[Slack file: b.png (fileId: FB)]"); }); it("caps downloads to 8 files for large multi-attachment messages", async () => { @@ -867,7 +867,7 @@ describe("resolveSlackThreadHistory", () => { it("includes file-only messages and drops empty-only entries", async () => { const replies = vi.fn().mockResolvedValueOnce({ messages: [ - { text: " ", ts: "1.000", files: [{ name: "screenshot.png" }] }, + { text: " ", ts: "1.000", files: [{ id: "FSCREEN", name: "screenshot.png" }] }, { text: " ", ts: "2.000" }, { text: "hello", ts: "3.000", user: "U1" }, ], @@ -885,7 +885,7 @@ describe("resolveSlackThreadHistory", () => { }); expect(result).toHaveLength(2); - expect(result[0]?.text).toBe("[attached: screenshot.png]"); + expect(result[0]?.text).toBe("[attached: screenshot.png (fileId: FSCREEN)]"); expect(result[1]?.text).toBe("hello"); }); @@ -982,7 +982,7 @@ describe("resolveSlackThreadStarter", () => { text: " ", user: "U1", ts: "1.000", - files: [{ name: "root.png", mimetype: "image/png" }], + files: [{ id: "FROOT", name: "root.png", mimetype: "image/png" }], }, ], }); @@ -997,11 +997,11 @@ describe("resolveSlackThreadStarter", () => { }); expect(result).toEqual({ - text: "[attached: root.png]", + text: "[attached: root.png (fileId: FROOT)]", userId: "U1", botId: undefined, ts: "1.000", - files: [{ name: "root.png", mimetype: "image/png" }], + files: [{ id: "FROOT", name: "root.png", mimetype: "image/png" }], }); expect(vi.mocked(logVerbose)).not.toHaveBeenCalled(); }); diff --git a/extensions/slack/src/monitor/media.ts b/extensions/slack/src/monitor/media.ts index 89242beefc1..e4177087b39 100644 --- a/extensions/slack/src/monitor/media.ts +++ b/extensions/slack/src/monitor/media.ts @@ -1,5 +1,6 @@ import { normalizeHostname } from "openclaw/plugin-sdk/host-runtime"; import { resolveRequestUrl } from "openclaw/plugin-sdk/request-url"; +import { formatSlackFileReference } from "../file-reference.js"; import type { SlackAttachment, SlackFile } from "../types.js"; export { MAX_SLACK_MEDIA_FILES, type SlackMediaResult } from "./media-types.js"; import { MAX_SLACK_MEDIA_FILES, type SlackMediaResult } from "./media-types.js"; @@ -281,7 +282,7 @@ export async function resolveSlackMedia(params: { return { path: saved.path, ...(contentType ? { contentType } : {}), - placeholder: label ? `[Slack file: ${label}]` : "[Slack file]", + placeholder: `[Slack file: ${formatSlackFileReference({ ...file, name: label })}]`, }; } catch { return null; diff --git a/extensions/slack/src/monitor/message-handler/prepare-content.ts b/extensions/slack/src/monitor/message-handler/prepare-content.ts index 1ff718459d0..47079dd65ba 100644 --- a/extensions/slack/src/monitor/message-handler/prepare-content.ts +++ b/extensions/slack/src/monitor/message-handler/prepare-content.ts @@ -1,6 +1,7 @@ import { runTasksWithConcurrency } from "openclaw/plugin-sdk/infra-runtime"; import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; +import { formatSlackFileReference } from "../../file-reference.js"; import type { SlackFile, SlackMessageEvent } from "../../types.js"; import { MAX_SLACK_MEDIA_FILES, type SlackMediaResult } from "../media-types.js"; import type { SlackThreadStarter } from "../thread.js"; @@ -125,7 +126,7 @@ export async function resolveSlackMessageContent(params: { !mediaPlaceholder && fallbackFiles.length > 0 ? fallbackFiles .slice(0, MAX_SLACK_MEDIA_FILES) - .map((file) => normalizeOptionalString(file.name) ?? "file") + .map((file) => formatSlackFileReference(file)) .join(", ") : undefined; const fileOnlyPlaceholder = fileOnlyFallback ? `[Slack file: ${fileOnlyFallback}]` : undefined; diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 88fd7606ce2..708cc1abfdf 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -364,14 +364,17 @@ describe("slack prepareSlackMessage inbound contract", () => { const prepared = await prepareWithDefaultCtx( createSlackMessage({ text: "", - files: [{ name: "voice.ogg" }, { name: "photo.jpg" }], + files: [ + { id: "FVOICE", name: "voice.ogg" }, + { id: "FPHOTO", name: "photo.jpg" }, + ], }), ); expect(prepared).toBeTruthy(); expect(prepared!.ctxPayload.RawBody).toContain("[Slack file:"); - expect(prepared!.ctxPayload.RawBody).toContain("voice.ogg"); - expect(prepared!.ctxPayload.RawBody).toContain("photo.jpg"); + expect(prepared!.ctxPayload.RawBody).toContain("voice.ogg (fileId: FVOICE)"); + expect(prepared!.ctxPayload.RawBody).toContain("photo.jpg (fileId: FPHOTO)"); }); it("falls back to generic file label when a Slack file name is empty", async () => { diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index 37a8c172b33..c453caba827 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -31,6 +31,7 @@ import { } from "openclaw/plugin-sdk/text-runtime"; import type { ResolvedSlackAccount } from "../../accounts.js"; import { reactSlackMessage } from "../../actions.js"; +import { formatSlackFileReference } from "../../file-reference.js"; import { hasSlackThreadParticipation } from "../../sent-thread-cache.js"; import type { SlackMessageEvent } from "../../types.js"; import { @@ -455,11 +456,9 @@ export async function prepareSlackMessage(params: { if (isRoom && shouldRequireMention && mentionDecision.shouldSkip) { ctx.logger.info({ channel: message.channel, reason: "no-mention" }, "skipping channel message"); const pendingText = (message.text ?? "").trim(); - const fallbackFile = message.files?.[0]?.name - ? `[Slack file: ${message.files[0].name}]` - : message.files?.length - ? "[Slack file]" - : ""; + const fallbackFile = message.files?.length + ? `[Slack file: ${formatSlackFileReference(message.files[0])}]` + : ""; const pendingBody = pendingText || fallbackFile; recordPendingHistoryEntryIfEnabled({ historyMap: ctx.channelHistories, diff --git a/extensions/slack/src/monitor/thread.ts b/extensions/slack/src/monitor/thread.ts index a24117ee53c..b2fdee74991 100644 --- a/extensions/slack/src/monitor/thread.ts +++ b/extensions/slack/src/monitor/thread.ts @@ -1,6 +1,7 @@ import type { WebClient as SlackWebClient } from "@slack/web-api"; import { pruneMapToMaxSize } from "openclaw/plugin-sdk/collection-runtime"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; +import { formatSlackFileReferenceList } from "../file-reference.js"; import type { SlackFile } from "../types.js"; import { logVerbose } from "./thread.runtime.js"; @@ -32,7 +33,7 @@ function evictThreadStarterCache(): void { } function formatSlackFilePlaceholder(files: SlackFile[] | undefined): string { - return `[attached: ${files?.map((file) => file.name ?? "file").join(", ") ?? "file"}]`; + return `[attached: ${formatSlackFileReferenceList(files)}]`; } export async function resolveSlackThreadStarter(params: {