ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-28 05:16:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(slack): retain unavailable attachments alongside downloads (#129708)

Browse Source 
* fix(slack): retain unavailable attachments alongside downloads

* fix(slack): retain original attachment identity through download

* test(slack): narrow optional attachment regression types
This commit is contained in:
Peter Steinberger
2026-08-26 00:11:09 -07:00
committed by GitHub
parent da7bad79cf
commit 8c203185d7
4 changed files with 145 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/slack/src/monitor/media.test.ts
+63 -13
View File
@@ -1275,23 +1275,72 @@ describe("Slack message file intake", () => {
expect(result?.rawBody).toContain("FRICH.png (image/png, fileId: FRICH)");
});
it("reuses the exact preloaded voice-file object across forwarded duplicates", async () => {
const voice = file("FVOICE");
const preloaded = {
path: "/tmp/preloaded-voice.ogg",
contentType: "audio/ogg",
placeholder: "[Slack file: voice.ogg (fileId: FVOICE)]",
};
it.each(["direct", "forwarded"] as const)(
"reuses the exact preloaded %s voice-file object across forwarded duplicates",
async (source) => {
const voice = file("FVOICE");
const direct = source === "direct" ? voice : file(" FVOICE ");
const forwarded = source === "forwarded" ? voice : file("FVOICE");
const preloaded = {
path: "/tmp/preloaded-voice.ogg",
contentType: "audio/ogg",
placeholder: "[Slack file: voice.ogg (fileId: FVOICE)]",
};
const result = await resolveMessageFiles({
direct: [direct],
forwarded: [[forwarded]],
preloadedMedia: new Map([[voice, preloaded]]),
});
expect(mockFetch).not.toHaveBeenCalled();
expect(result?.effectiveDirectMedia).toEqual([preloaded]);
expect(result?.effectiveDirectMedia?.[0]).toBe(preloaded);
expect(result?.rawBody.match(/fileId: FVOICE/g)).toHaveLength(1);
},
);
it("keeps failed file identities beside renamed, overlapping, and ID-less downloads", async () => {
const downloaded = file("F11");
const unavailable = { id: "F1", name: "missing-contract.pdf", mimetype: "application/pdf" };
const downloadedWithoutId = { name: "available.png", mimetype: "image/png" };
const unavailableWithSameMetadata = { ...downloadedWithoutId };
const unavailableWithoutId = { name: "missing.png", mimetype: "image/png" };
const result = await resolveMessageFiles({
direct: [voice],
forwarded: [[file("FVOICE")]],
preloadedMedia: new Map([[voice, preloaded]]),
direct: [
downloaded,
unavailable,
downloadedWithoutId,
unavailableWithSameMetadata,
unavailableWithoutId,
],
preloadedMedia: new Map([
[
downloaded,
{
path: "/tmp/renamed.png",
fileName: "renamed.png",
placeholder: "[Slack file: renamed.png (fileId: F11)]",
},
],
[
downloadedWithoutId,
{
path: "/tmp/server-renamed.png",
fileName: "server-renamed.png",
placeholder: "[Slack file: server-renamed.png (image/png)]",
},
],
]),
});
expect(mockFetch).not.toHaveBeenCalled();
expect(result?.effectiveDirectMedia).toEqual([preloaded]);
expect(result?.effectiveDirectMedia?.[0]).toBe(preloaded);
expect(result?.effectiveDirectMedia).toHaveLength(2);
expect(result?.rawBody.match(/fileId: F11/g)).toHaveLength(1);
expect(result?.rawBody.match(/server-renamed\.png/g)).toHaveLength(1);
expect(result?.rawBody.match(/available\.png/g)).toHaveLength(1);
expect(result?.rawBody).toContain("missing-contract.pdf (application/pdf, fileId: F1)");
expect(result?.rawBody).toContain("missing.png (image/png)");
});
it("applies Slack's eight-file budget once across direct and forwarded sources", async () => {
@@ -1343,6 +1392,7 @@ describe("Slack message file intake", () => {
expect(result?.rawBody).toContain(
"FDIRECT)] [Forwarded image: first-image.png] [Slack file: FFIRST",
);
expect(result?.rawBody).toContain("FFAILED.png (image/png, fileId: FFAILED)");
});
it("preserves distinct files without Slack file identifiers", async () => {
extensions/slack/src/monitor/media.ts
+22 -8
View File
@@ -334,6 +334,7 @@ export async function resolveSlackMedia(params: {
totalTimeoutMs?: number;
abortSignal?: AbortSignal;
preloadedMedia?: ReadonlyMap<SlackFile, SlackMediaResult>;
resolvedFiles?: Set<SlackFile>;
}): Promise<SlackMediaResult[] | null> {
const govSlack = isGovSlackClient(params.client);
const files = params.files ?? [];
@@ -393,7 +394,12 @@ export async function resolveSlackMedia(params: {
errorMode: "stop",
throwOnError: true,
});
const resolved = results.filter((result): result is SlackMediaResult => result !== null);
const resolved = results.filter((result, index): result is SlackMediaResult => {
if (result) {
params.resolvedFiles?.add(limitedFiles[index]!);
}
return result !== null;
});
return resolved.length > 0 ? resolved : null;
}
@@ -442,12 +448,17 @@ export async function resolveSlackAttachmentContent(params: {
.slice(0, MAX_SLACK_MEDIA_FILES)
.map((file) => {
const fileId = normalizeOptionalString(file.id);
if (
!fileId ||
params.preloadedMedia?.has(file) ||
file.url_private_download ||
file.url_private
) {
const preloaded =
fileId &&
candidates.find(
(candidate) =>
normalizeOptionalString(candidate.id) === fileId &&
params.preloadedMedia?.has(candidate),
);
if (preloaded) {
return preloaded;
}
if (!fileId || file.url_private_download || file.url_private) {
return file;
}
const downloadable = candidates.find(
@@ -460,9 +471,11 @@ export async function resolveSlackAttachmentContent(params: {
const pendingFiles = new Map<SlackFile | string, SlackFile>(
allFiles.map((file) => [normalizeOptionalString(file.id) ?? file, file]),
);
const resolvedFiles = new Set<SlackFile>();
const resolveFiles = (files?: SlackFile[]) =>
resolveSlackMedia({
...params,
resolvedFiles,
files: files?.flatMap((file) => {
const key = normalizeOptionalString(file.id) ?? file;
const selected = pendingFiles.get(key);
@@ -520,6 +533,7 @@ export async function resolveSlackAttachmentContent(params: {
}
const allMedia = [...((await directMediaPromise) ?? []), ...attachmentMedia];
const unavailableFiles = allFiles.filter((file) => !resolvedFiles.has(file));
const combinedText = textBlocks.join("\n\n");
if (
!combinedText &&
@@ -533,6 +547,6 @@ export async function resolveSlackAttachmentContent(params: {
text: combinedText,
media: allMedia,
unavailableImageCount,
...(allFiles.length > 0 ? { files: allFiles } : {}),
...(unavailableFiles.length > 0 ? { files: unavailableFiles } : {}),
};
}
extensions/slack/src/monitor/message-handler/prepare-content.ts
+4 -18
View File
@@ -116,16 +116,9 @@ export async function resolveSlackMessageContent(params: {
: null;
const effectiveDirectMedia = attachmentContent?.media.length ? attachmentContent.media : null;
const mediaPlaceholder = effectiveDirectMedia
? effectiveDirectMedia.map((item) => item.placeholder).join(" ")
: undefined;
const mediaPlaceholder = effectiveDirectMedia?.map((item) => item.placeholder).join(" ");
const fallbackFiles = attachmentContent?.files ?? [];
const fileOnlyFallback =
!mediaPlaceholder && fallbackFiles.length > 0
? fallbackFiles.map((file) => formatSlackFileReference(file)).join(", ")
: undefined;
const fileOnlyPlaceholder = fileOnlyFallback ? `[Slack file: ${fileOnlyFallback}]` : undefined;
const fileOnlyFallback = attachmentContent?.files?.map(formatSlackFileReference).join(", ");
let botAttachmentText: string | undefined;
if (params.isBotMessage && !attachmentContent?.text) {
@@ -180,7 +173,7 @@ export async function resolveSlackMessageContent(params: {
renderedAttachmentText,
renderedBotAttachmentText,
mediaPlaceholder,
fileOnlyPlaceholder,
fileOnlyFallback ? `[Slack file: ${fileOnlyFallback}]` : undefined,
]
.filter(Boolean)
.join("\n") || "";
@@ -193,12 +186,5 @@ export async function resolveSlackMessageContent(params: {
} unavailable]`,
});
}
if (!rawBody) {
return null;
}
return {
rawBody,
effectiveDirectMedia,
};
return rawBody ? { rawBody, effectiveDirectMedia } : null;
}
extensions/slack/src/monitor/message-handler/prepare.test.ts
+56
View File
@@ -2098,6 +2098,62 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`;
);
});
it("keeps a failed file recoverable when a sibling download reaches the agent", async () => {
const originalFetch = globalThis.fetch;
globalThis.fetch = vi.fn(
async (input: RequestInfo | URL) =>
new Response(Buffer.from("image contents"), {
status: 200,
headers: {
"content-type": "image/png",
...(typeof input === "string" && input.includes("original-name.png")
? { "content-disposition": 'attachment; filename="server-renamed.png"' }
: {}),
},
}),
) as typeof fetch;
let downloadedPaths: string[] = [];
try {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({
text: "Please inspect both attachments",
files: [
{
id: "F11",
name: "available.png",
mimetype: "image/png",
url_private_download: "https://files.slack.com/available.png",
},
{
name: "original-name.png",
mimetype: "image/png",
url_private_download: "https://files.slack.com/original-name.png",
},
{ name: "original-name.png", mimetype: "image/png" },
{ id: "F1", name: "missing-contract.pdf", mimetype: "application/pdf" },
],
}),
);
assertPrepared(prepared);
downloadedPaths =
prepared.ctxPayload.media?.flatMap((media) => (media.path ? [media.path] : [])) ?? [];
expect(prepared.ctxPayload.media).toHaveLength(2);
expect(prepared.ctxPayload.RawBody).toContain("available.png (image/png, fileId: F11)");
expect(prepared.ctxPayload.RawBody).toContain("server-renamed.png (image/png)");
expect(prepared.ctxPayload.RawBody?.match(/original-name\.png/g)).toHaveLength(1);
expect(prepared.ctxPayload.BodyForAgent).toContain(
"missing-contract.pdf (application/pdf, fileId: F1)",
);
} finally {
globalThis.fetch = originalFetch;
await Promise.all(
downloadedPaths.map((downloadedPath) => fs.rm(downloadedPath, { force: true })),
);
}
});
it("delivers forwarded file-only messages with metadata when media download fails", async () => {
const prepared = await prepareWithDefaultCtx(
createSlackMessage({