Compare commits

...

2 Commits

Author SHA1 Message Date
Peter Steinberger
1a56e16a7f fix: harden slack thread file inheritance filter (#32209) (thanks @OliYeet) 2026-03-02 22:11:01 +00:00
OliYeet
a47b057283 fix(slack): filter inherited parent files from thread replies (#32203)
Slack's Events API includes the parent message's files array in every
thread reply event payload. This caused OpenClaw to re-download and
attach the parent's files to every text-only thread reply, creating
ghost media attachments.

The fix filters out files that belong to the thread starter by comparing
file IDs. The resolveSlackThreadStarter result is already cached, so
this adds no extra API calls.

Closes #32203
2026-03-02 22:08:37 +00:00
4 changed files with 100 additions and 5 deletions

View File

@@ -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.

View File

@@ -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 = {

View File

@@ -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" });

View File

@@ -515,8 +515,32 @@ export async function prepareSlackMessage(params: {
return null;
}
// When processing a thread reply, filter out files that belong to the thread
// starter (parent message). Slack's Events API includes the parent's `files`
// array in every thread reply payload, which causes ghost media attachments
// on text-only replies. We eagerly resolve the thread starter here (the result
// is cached) and exclude any file IDs that match the parent. (#32203)
let ownFiles = message.files;
if (isThreadReply && threadTs && message.files?.length) {
const starter = await resolveSlackThreadStarter({
channelId: message.channel,
threadTs,
client: ctx.app.client,
});
if (starter?.files?.length) {
const starterFileIds = new Set(starter.files.map((f) => f.id));
const filtered = message.files.filter((f) => !f.id || !starterFileIds.has(f.id));
if (filtered.length < message.files.length) {
logVerbose(
`slack: filtered ${message.files.length - filtered.length} inherited parent file(s) from thread reply`,
);
}
ownFiles = filtered.length > 0 ? filtered : undefined;
}
}
const media = await resolveSlackMedia({
files: message.files,
files: ownFiles,
token: ctx.botToken,
maxBytes: ctx.mediaMaxBytes,
});
@@ -540,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;