fix(slack): preserve inbound attachment metadata (#114502)

This commit is contained in:
Peter Steinberger
2026-08-09 04:09:18 -07:00
committed by GitHub
parent 44a2a87134
commit d1b4ed872c
4 changed files with 87 additions and 17 deletions
@@ -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)");
},
);
});
+10 -1
View File
@@ -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 {
+20 -8
View File
@@ -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();
});
@@ -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;