From d1b4ed872cfc31c7aa6a5f57692617a0151eb2fa Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 04:09:18 -0700 Subject: [PATCH] fix(slack): preserve inbound attachment metadata (#114502) --- extensions/slack/src/file-reference.test.ts | 43 +++++++++++++++++++ extensions/slack/src/file-reference.ts | 11 ++++- extensions/slack/src/monitor/media.test.ts | 28 ++++++++---- .../monitor/message-handler/prepare.test.ts | 22 ++++++---- 4 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 extensions/slack/src/file-reference.test.ts diff --git a/extensions/slack/src/file-reference.test.ts b/extensions/slack/src/file-reference.test.ts new file mode 100644 index 000000000000..dd32183894d9 --- /dev/null +++ b/extensions/slack/src/file-reference.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { formatSlackFileReference } from "./file-reference.js"; + +describe("formatSlackFileReference", () => { + it.each([ + { + name: "all Slack metadata", + file: { id: "F123", name: "report.pdf", mimetype: "application/pdf", size: 45_056 }, + expected: "report.pdf (application/pdf, 45056 bytes, fileId: F123)", + }, + { + name: "metadata without a file ID", + file: { name: "report.pdf", mimetype: "application/pdf", size: 45_056 }, + expected: "report.pdf (application/pdf, 45056 bytes)", + }, + { + name: "no optional metadata", + file: { id: "F123", name: "report.pdf" }, + expected: "report.pdf (fileId: F123)", + }, + { + name: "an empty file and blank MIME type", + file: { id: "FEMPTY", name: "empty.txt", mimetype: " ", size: 0 }, + expected: "empty.txt (0 bytes, fileId: FEMPTY)", + }, + ])("renders $name", ({ file, expected }) => { + expect(formatSlackFileReference(file)).toBe(expected); + }); + + it.each([-1, 1.5, Number.NaN, Number.POSITIVE_INFINITY, Number.MAX_SAFE_INTEGER + 1])( + "omits an invalid Slack file size (%s)", + (size) => { + expect( + formatSlackFileReference({ + id: "F123", + name: "report.pdf", + mimetype: "application/pdf", + size, + }), + ).toBe("report.pdf (application/pdf, fileId: F123)"); + }, + ); +}); diff --git a/extensions/slack/src/file-reference.ts b/extensions/slack/src/file-reference.ts index 1a568752621e..406c17b14da5 100644 --- a/extensions/slack/src/file-reference.ts +++ b/extensions/slack/src/file-reference.ts @@ -4,8 +4,17 @@ import type { SlackFile } from "./types.js"; export function formatSlackFileReference(file: SlackFile | undefined): string { const name = normalizeOptionalString(file?.name) ?? "file"; + const mimetype = normalizeOptionalString(file?.mimetype); + const size = file?.size; const fileId = normalizeOptionalString(file?.id); - return fileId ? `${name} (fileId: ${fileId})` : name; + const metadata = [ + mimetype, + typeof size === "number" && Number.isSafeInteger(size) && size >= 0 + ? `${size} bytes` + : undefined, + fileId ? `fileId: ${fileId}` : undefined, + ].filter((value): value is string => value !== undefined); + return metadata.length > 0 ? `${name} (${metadata.join(", ")})` : name; } export function formatSlackFileReferenceList(files: readonly SlackFile[] | undefined): string { diff --git a/extensions/slack/src/monitor/media.test.ts b/extensions/slack/src/monitor/media.test.ts index 88f2d65f6b65..a54a492ab3a1 100644 --- a/extensions/slack/src/monitor/media.test.ts +++ b/extensions/slack/src/monitor/media.test.ts @@ -841,7 +841,7 @@ describe("resolveSlackMedia", () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - it("returns all successfully downloaded files as an array", async () => { + it("preserves Slack metadata on every downloaded file", async () => { saveMediaBufferMock.mockImplementation(async (buffer, _contentType) => { const text = Buffer.from(buffer).toString("utf8"); if (text.includes("image a")) { @@ -873,8 +873,20 @@ describe("resolveSlackMedia", () => { const result = await resolveSlackMedia({ files: [ - { 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" }, + { + id: "FA", + url_private: "https://files.slack.com/a.jpg", + name: "a.jpg", + mimetype: "image/jpeg", + size: 12, + }, + { + id: "FB", + url_private: "https://files.slack.com/b.png", + name: "b.png", + mimetype: "image/png", + size: 34, + }, ], token: "xoxb-test-token", maxBytes: 1024 * 1024, @@ -885,9 +897,9 @@ describe("resolveSlackMedia", () => { const first = expectDefined(media[0], "first Slack media result"); const second = expectDefined(media[1], "second Slack media result"); expect(first.path).toBe("/tmp/a.jpg"); - expect(first.placeholder).toBe("[Slack file: a.jpg (fileId: FA)]"); + expect(first.placeholder).toBe("[Slack file: a.jpg (image/jpeg, 12 bytes, fileId: FA)]"); expect(second.path).toBe("/tmp/b.png"); - expect(second.placeholder).toBe("[Slack file: b.png (fileId: FB)]"); + expect(second.placeholder).toBe("[Slack file: b.png (image/png, 34 bytes, fileId: FB)]"); }); it("caps downloads to 8 files for large multi-attachment messages", async () => { @@ -1753,7 +1765,7 @@ describe("resolveSlackThreadStarter", () => { text: " ", user: "U1", ts: "1.000", - files: [{ id: "FROOT", name: "root.png", mimetype: "image/png" }], + files: [{ id: "FROOT", name: "root.png", mimetype: "image/png", size: 512 }], }, ], }); @@ -1768,11 +1780,11 @@ describe("resolveSlackThreadStarter", () => { }); expect(result).toEqual({ - text: "[attached: root.png (fileId: FROOT)]", + text: "[attached: root.png (image/png, 512 bytes, fileId: FROOT)]", userId: "U1", botId: undefined, ts: "1.000", - files: [{ id: "FROOT", name: "root.png", mimetype: "image/png" }], + files: [{ id: "FROOT", name: "root.png", mimetype: "image/png", size: 512 }], }); expect(vi.mocked(logVerbose)).not.toHaveBeenCalled(); }); diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 8d0e26fde1a1..85acf6d11439 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -1832,16 +1832,20 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; createSlackMessage({ text: "", files: [ - { id: "FVOICE", name: "voice.ogg" }, - { id: "FPHOTO", name: "photo.jpg" }, + { id: "FVOICE", name: "voice.ogg", mimetype: "audio/ogg", size: 3_210 }, + { id: "FPHOTO", name: "photo.jpg", mimetype: "image/jpeg", size: 6_543 }, ], }), ); assertPrepared(prepared); expect(prepared.ctxPayload.RawBody).toContain("[Slack file:"); - expect(prepared.ctxPayload.RawBody).toContain("voice.ogg (fileId: FVOICE)"); - expect(prepared.ctxPayload.RawBody).toContain("photo.jpg (fileId: FPHOTO)"); + expect(prepared.ctxPayload.RawBody).toContain( + "voice.ogg (audio/ogg, 3210 bytes, fileId: FVOICE)", + ); + expect(prepared.ctxPayload.RawBody).toContain( + "photo.jpg (image/jpeg, 6543 bytes, fileId: FPHOTO)", + ); }); it("delivers forwarded file-only messages with metadata when media download fails", async () => { @@ -2074,7 +2078,7 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; expect(prepared).toBeNull(); const entries = Array.from(slackCtx.channelHistories.values()).flat(); expect(entries).toHaveLength(1); - expect(entries[0]?.body).toBe("[Slack file: diagram.png (fileId: F1)]"); + expect(entries[0]?.body).toBe("[Slack file: diagram.png (image/png, fileId: F1)]"); expect(entries[0]?.media).toHaveLength(1); expect(entries[0]?.media?.[0]).toMatchObject({ contentType: "image/png", @@ -2198,7 +2202,7 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; }); const entries = Array.from(slackCtx.channelHistories.values()).flat(); expect(entries).toHaveLength(1); - expect(entries[0]?.body).toBe("[Slack file: parent.png (fileId: F-parent)]"); + expect(entries[0]?.body).toBe("[Slack file: parent.png (image/png, fileId: F-parent)]"); expect(entries[0]?.media).toBeUndefined(); expect(mockFetch).not.toHaveBeenCalled(); } finally { @@ -4286,7 +4290,9 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; expect(root.ctxPayload.CommandBody).toBe(""); expect(root.ctxPayload.Transcript).toBe("Bill /new please review this"); expect(root.ctxPayload.media?.[1]?.transcribed).toBe(true); - expect(root.ctxPayload.RawBody).toContain("[Slack file: voice.mp4 (fileId: FVOICE)]"); + expect(root.ctxPayload.RawBody).toContain( + "[Slack file: voice.mp4 (video/mp4, fileId: FVOICE)]", + ); expect(root.ctxPayload.BodyForAgent).toContain( '[Audio transcript (machine-generated, untrusted)]: "Bill /new please review this"', ); @@ -4395,7 +4401,7 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; await expect(fs.stat(downloadedPath as string)).rejects.toMatchObject({ code: "ENOENT" }); const entries = Array.from(slackCtx.channelHistories.values()).flat(); expect(entries).toHaveLength(1); - expect(entries[0]?.body).toBe("[Slack file: report.pdf (fileId: FPDF)]"); + expect(entries[0]?.body).toBe("[Slack file: report.pdf (application/pdf, fileId: FPDF)]"); expect(entries[0]?.media).toBeUndefined(); } finally { globalThis.fetch = originalFetch;