From 1a56e16a7f2afdd6d8cbfb539f159375253c65be Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 22:11:01 +0000 Subject: [PATCH] fix: harden slack thread file inheritance filter (#32209) (thanks @OliYeet) --- CHANGELOG.md | 1 + src/slack/monitor/media.ts | 3 +- .../monitor/message-handler/prepare.test.ts | 69 +++++++++++++++++++ src/slack/monitor/message-handler/prepare.ts | 6 +- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6d0c243c60..f08c2df64a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,7 @@ Docs: https://docs.openclaw.ai - Telegram/models picker callbacks: keep long model buttons selectable by falling back to compact callback payloads and resolving provider ids on selection (with provider re-prompt on ambiguity), avoiding Telegram 64-byte callback truncation failures. (#31857) Thanks @bmendonca3. - WhatsApp/inbound self-message context: propagate inbound `fromMe` through the web inbox pipeline and annotate direct self messages as `(self)` in envelopes so agents can distinguish owner-authored turns from contact turns. (#32167) Thanks @scoootscooob. - Slack/thread context payloads: only inject thread starter/history text on first thread turn for new sessions while preserving thread metadata, reducing repeated context-token bloat on long-lived thread sessions. (#32133) Thanks @sourman. +- Slack/thread replies: filter inherited parent file IDs before media/fallback placeholder assembly so text-only replies no longer attach ghost parent files and reply-owned files still pass through. (#32209) Thanks @OliYeet. - Slack/session routing: keep top-level channel messages in one shared session when `replyToMode=off`, while preserving thread-scoped keys for true thread replies and non-off modes. (#32193) Thanks @bmendonca3. - Slack/inbound debounce routing: isolate top-level non-DM message debounce keys by message timestamp to avoid cross-thread collisions, preserve DM batching, and flush pending top-level buffers before immediate non-debounce follow-ups to keep ordering stable. (#31951) Thanks @scoootscooob. - OpenRouter/x-ai compatibility: skip `reasoning.effort` injection for `x-ai/*` models (for example Grok) so OpenRouter requests no longer fail with invalid-arguments errors on unsupported reasoning params. (#32054) Thanks @scoootscooob. diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index fca883966a8..05711c5c602 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -403,7 +403,8 @@ export async function resolveSlackThreadStarter(params: { })) as { messages?: Array<{ text?: string; user?: string; ts?: string; files?: SlackFile[] }> }; const message = response?.messages?.[0]; const text = (message?.text ?? "").trim(); - if (!message || !text) { + const hasFiles = (message?.files?.length ?? 0) > 0; + if (!message || (!text && !hasFiles)) { return null; } const starter: SlackThreadStarter = { diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index 64f59b2e2dd..e70c38ae5f9 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -522,6 +522,75 @@ describe("slack prepareSlackMessage inbound contract", () => { ); }); + it("filters inherited parent files from thread replies (no ghost file placeholder)", async () => { + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "starter", + user: "U2", + ts: "900.000", + files: [{ id: "F_PARENT", name: "parent.png" }], + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createThreadSlackCtx({ + cfg: { + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + replies, + }); + slackCtx.resolveUserName = async () => ({ name: "Alice" }); + slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" }); + + const prepared = await prepareThreadMessage(slackCtx, { + text: "thread reply text only", + ts: "101.000", + thread_ts: "900.000", + files: [{ id: "F_PARENT", name: "parent.png" }], + }); + + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.RawBody).toContain("thread reply text only"); + expect(prepared!.ctxPayload.RawBody).not.toContain("[Slack file:"); + }); + + it("keeps reply-owned files when parent and reply file IDs differ", async () => { + const replies = vi.fn().mockResolvedValue({ + messages: [ + { + text: "starter", + user: "U2", + ts: "901.000", + files: [{ id: "F_PARENT", name: "parent.png" }], + }, + ], + response_metadata: { next_cursor: "" }, + }); + const slackCtx = createThreadSlackCtx({ + cfg: { + channels: { slack: { enabled: true, replyToMode: "all", groupPolicy: "open" } }, + } as OpenClawConfig, + replies, + }); + slackCtx.resolveUserName = async () => ({ name: "Alice" }); + slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" }); + + const prepared = await prepareThreadMessage(slackCtx, { + text: "", + ts: "101.000", + thread_ts: "901.000", + files: [ + { id: "F_PARENT", name: "parent.png" }, + { id: "F_REPLY", name: "reply.png" }, + ], + }); + + expect(prepared).toBeTruthy(); + expect(prepared!.ctxPayload.RawBody).toContain("[Slack file: reply.png]"); + expect(prepared!.ctxPayload.RawBody).not.toContain("parent.png"); + }); + it("excludes thread_ts from top-level messages", async () => { const message = createSlackMessage({ text: "hello" }); diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index b2616b63927..838aa12fc9a 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -564,9 +564,9 @@ export async function prepareSlackMessage(params: { // placeholder so the message is still delivered to the agent instead of // being silently dropped (#25064). const fileOnlyFallback = - !mediaPlaceholder && (message.files?.length ?? 0) > 0 - ? message - .files!.slice(0, MAX_SLACK_MEDIA_FILES) + !mediaPlaceholder && (ownFiles?.length ?? 0) > 0 + ? ownFiles + .slice(0, MAX_SLACK_MEDIA_FILES) .map((f) => f.name?.trim() || "file") .join(", ") : undefined;