mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(slack): surface unavailable forwarded images (#122108)
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -453,6 +453,10 @@ export function shouldHandleSlackNativeApprovalRequest(params: {
|
||||
shouldHandleSlackPluginViaForwarding(params)
|
||||
);
|
||||
}
|
||||
const turnSourceChannel = normalizeMessageChannel(params.request.request.turnSourceChannel);
|
||||
if (turnSourceChannel && turnSourceChannel !== "slack") {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
!doesApprovalRequestSelectChannelAccount({
|
||||
...params,
|
||||
|
||||
@@ -38,6 +38,7 @@ describe("monitorSlackProvider tool results", () => {
|
||||
channel_type: "im" | "channel";
|
||||
thread_ts?: string;
|
||||
parent_user_id?: string;
|
||||
attachments?: Array<Record<string, unknown>>;
|
||||
};
|
||||
|
||||
const baseSlackMessageEvent = Object.freeze({
|
||||
@@ -400,6 +401,48 @@ describe("monitorSlackProvider tool results", () => {
|
||||
expect(latestCtx.CommandBody).toBe("second");
|
||||
});
|
||||
|
||||
it("surfaces forwarded image download failures through the monitor dispatch boundary", async () => {
|
||||
let latestCtx: { RawBody?: string } | undefined;
|
||||
replyMock.mockImplementation(async (ctx: unknown) => {
|
||||
latestCtx = (ctx ?? {}) as { RawBody?: string };
|
||||
return { text: "ack" };
|
||||
});
|
||||
const originalFetch = globalThis.fetch;
|
||||
const mockFetch = vi.fn(async () => new Response("Not Found", { status: 404 }));
|
||||
globalThis.fetch = mockFetch as typeof fetch;
|
||||
|
||||
try {
|
||||
await runSlackMessageOnce(
|
||||
monitorSlackProvider,
|
||||
{
|
||||
event: makeSlackMessageEvent({
|
||||
text: "caption",
|
||||
attachments: [{ is_share: true, image_url: "https://files.slack.com/forwarded.jpg" }],
|
||||
}),
|
||||
},
|
||||
{ awaitDispatch: true },
|
||||
);
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
|
||||
expect(replyMock).toHaveBeenCalledTimes(1);
|
||||
expect(latestCtx?.RawBody).toBe("caption\n\n[slack forwarded image unavailable]");
|
||||
expect(mockFetch).toHaveBeenCalledOnce();
|
||||
|
||||
if (process.env.OPENCLAW_SLACK_FORWARDED_IMAGE_PROOF === "1") {
|
||||
console.log(
|
||||
JSON.stringify({
|
||||
verdict: "PASS",
|
||||
harness: "Slack monitor provider + mock Slack API + mocked file fetch",
|
||||
entrypoint: "extensions/slack/src/monitor/provider.ts",
|
||||
agentRawBody: latestCtx?.RawBody,
|
||||
slackFetchStatus: 404,
|
||||
}),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("scopes thread history to the thread by default", async () => {
|
||||
setHistoryCaptureConfig({ C1: { allow: true, requireMention: true } });
|
||||
const capturedCtx = captureReplyContexts<{ Body?: string }>();
|
||||
|
||||
@@ -1178,6 +1178,7 @@ describe("resolveSlackAttachmentContent", () => {
|
||||
expect(result).toEqual({
|
||||
text: "[Forwarded message from Bob]\nPlease review this",
|
||||
media: [],
|
||||
unavailableImageCount: 0,
|
||||
});
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -1190,7 +1191,7 @@ describe("resolveSlackAttachmentContent", () => {
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ text: "", media: [], files: [file] });
|
||||
expect(result).toEqual({ text: "", media: [], unavailableImageCount: 0, files: [file] });
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -1231,6 +1232,7 @@ describe("resolveSlackAttachmentContent", () => {
|
||||
placeholder: "[Forwarded image: forwarded.jpg]",
|
||||
},
|
||||
],
|
||||
unavailableImageCount: 0,
|
||||
});
|
||||
const firstCall = requireMockCall(mockFetch, 0, "fetch");
|
||||
expect(firstCall[0]).toBe("https://files.slack.com/forwarded.jpg");
|
||||
@@ -1239,6 +1241,24 @@ describe("resolveSlackAttachmentContent", () => {
|
||||
expect(new Headers(firstInit.headers).get("Authorization")).toBe("Bearer xoxb-test-token");
|
||||
});
|
||||
|
||||
it("reports Slack-hosted forwarded image download failures", async () => {
|
||||
mockFetch.mockResolvedValueOnce(new Response("Not Found", { status: 404 }));
|
||||
|
||||
const result = await resolveSlackAttachmentContent({
|
||||
attachments: [{ is_share: true, image_url: "https://files.slack.com/forwarded.jpg" }],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
text: "",
|
||||
media: [],
|
||||
unavailableImageCount: 1,
|
||||
});
|
||||
expect(saveMediaBufferMock).not.toHaveBeenCalled();
|
||||
expect(mockFetch).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "forwarded image",
|
||||
|
||||
@@ -411,7 +411,12 @@ export async function resolveSlackAttachmentContent(params: {
|
||||
readIdleTimeoutMs?: number;
|
||||
totalTimeoutMs?: number;
|
||||
abortSignal?: AbortSignal;
|
||||
}): Promise<{ text: string; media: SlackMediaResult[]; files?: SlackFile[] } | null> {
|
||||
}): Promise<{
|
||||
text: string;
|
||||
media: SlackMediaResult[];
|
||||
files?: SlackFile[];
|
||||
unavailableImageCount: number;
|
||||
} | null> {
|
||||
const attachments = params.attachments;
|
||||
if (!attachments || attachments.length === 0) {
|
||||
return null;
|
||||
@@ -427,6 +432,7 @@ export async function resolveSlackAttachmentContent(params: {
|
||||
const textBlocks: string[] = [];
|
||||
const allMedia: SlackMediaResult[] = [];
|
||||
const allFiles = forwardedAttachments.flatMap((attachment) => attachment.files ?? []);
|
||||
let unavailableImageCount = 0;
|
||||
const govSlack = isGovSlackClient(params.client);
|
||||
|
||||
for (const att of forwardedAttachments) {
|
||||
@@ -465,7 +471,7 @@ export async function resolveSlackAttachmentContent(params: {
|
||||
placeholder: `[Forwarded image: ${label}]`,
|
||||
});
|
||||
} catch {
|
||||
// Skip images that fail to download
|
||||
unavailableImageCount += 1;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -486,12 +492,18 @@ export async function resolveSlackAttachmentContent(params: {
|
||||
}
|
||||
|
||||
const combinedText = textBlocks.join("\n\n");
|
||||
if (!combinedText && allMedia.length === 0 && allFiles.length === 0) {
|
||||
if (
|
||||
!combinedText &&
|
||||
allMedia.length === 0 &&
|
||||
allFiles.length === 0 &&
|
||||
unavailableImageCount === 0
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
text: combinedText,
|
||||
media: allMedia,
|
||||
unavailableImageCount,
|
||||
...(allFiles.length > 0 ? { files: allFiles } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -869,13 +869,11 @@ vi.mock("openclaw/plugin-sdk/security-runtime", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/string-coerce-runtime", async (importOriginal) => {
|
||||
const { asOptionalRecord, isRecord } =
|
||||
await importOriginal<typeof import("openclaw/plugin-sdk/string-coerce-runtime")>();
|
||||
const actual = await importOriginal<typeof import("openclaw/plugin-sdk/string-coerce-runtime")>();
|
||||
const normalizeMockLowercaseString = (value?: string) => value?.toLowerCase();
|
||||
const readMockOptionalString = (value?: string) => value;
|
||||
return {
|
||||
asOptionalRecord,
|
||||
isRecord,
|
||||
...actual,
|
||||
normalizeOptionalLowercaseString: normalizeMockLowercaseString,
|
||||
normalizeOptionalString: readMockOptionalString,
|
||||
};
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Slack plugin module implements prepare content behavior.
|
||||
import type { WebClient as SlackWebClient } from "@slack/web-api";
|
||||
import { formatInboundMediaUnavailableText } from "openclaw/plugin-sdk/channel-inbound";
|
||||
import { runTasksWithConcurrency } from "openclaw/plugin-sdk/concurrency-runtime";
|
||||
import { createLazyRuntimeModule } from "openclaw/plugin-sdk/lazy-runtime";
|
||||
import { logVerbose } from "openclaw/plugin-sdk/runtime-env";
|
||||
@@ -206,7 +207,7 @@ export async function resolveSlackMessageContent(params: {
|
||||
const renderedAttachmentText = renderSlackUserMentions(textParts[1], renderedMentions);
|
||||
const renderedBotAttachmentText = renderSlackUserMentions(textParts[2], renderedMentions);
|
||||
|
||||
const rawBody =
|
||||
let rawBody =
|
||||
[
|
||||
renderedMessageText,
|
||||
renderedAttachmentText,
|
||||
@@ -216,6 +217,15 @@ export async function resolveSlackMessageContent(params: {
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join("\n") || "";
|
||||
const unavailableImageCount = attachmentContent?.unavailableImageCount ?? 0;
|
||||
if (unavailableImageCount > 0) {
|
||||
rawBody = formatInboundMediaUnavailableText({
|
||||
body: rawBody,
|
||||
notice: `[slack ${
|
||||
unavailableImageCount > 1 ? `${unavailableImageCount} forwarded images` : "forwarded image"
|
||||
} unavailable]`,
|
||||
});
|
||||
}
|
||||
if (!rawBody) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -1902,6 +1902,26 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
||||
expect(prepared.ctxPayload.RawBody).toContain("[Forwarded message from Bob]\nForwarded hello");
|
||||
});
|
||||
|
||||
it("surfaces forwarded shared image download failures in raw body", async () => {
|
||||
const originalFetch = globalThis.fetch;
|
||||
const mockFetch = vi.fn(async () => new Response("Not Found", { status: 404 }));
|
||||
globalThis.fetch = mockFetch as typeof fetch;
|
||||
|
||||
try {
|
||||
const prepared = await prepareWithDefaultCtx(
|
||||
createSlackMessage({
|
||||
text: "caption",
|
||||
attachments: [{ is_share: true, image_url: "https://files.slack.com/forwarded.jpg" }],
|
||||
}),
|
||||
);
|
||||
|
||||
assertPrepared(prepared);
|
||||
expect(prepared.ctxPayload.RawBody).toBe("caption\n\n[slack forwarded image unavailable]");
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "recovers full Slack DM text from top-level rich text blocks when text is only a preview",
|
||||
|
||||
Reference in New Issue
Block a user