diff --git a/.changeset/slack-thread-attachment-lookback.md b/.changeset/slack-thread-attachment-lookback.md new file mode 100644 index 000000000..e22e07396 --- /dev/null +++ b/.changeset/slack-thread-attachment-lookback.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +Fix the Slack thread attachment lookback only ever inspecting the newest non-bot message. When a user posted an image in a thread and then mentioned the agent in a text-only follow-up, the image was silently dropped and the model saw no file parts. The lookback now collects attachments from recent non-bot messages across the thread, newest first, capped at 10 file parts. diff --git a/packages/eve/src/public/channels/slack/attachments.test.ts b/packages/eve/src/public/channels/slack/attachments.test.ts index 66f6a9645..fd46c5d84 100644 --- a/packages/eve/src/public/channels/slack/attachments.test.ts +++ b/packages/eve/src/public/channels/slack/attachments.test.ts @@ -345,6 +345,108 @@ describe("collectInboundFileParts", () => { expect((parts[0]!.data as URL).href).toBe("https://files.slack.com/a/b/from-user.csv"); }); + it("looks past a newer text-only message to find files earlier in the thread", async () => { + const refresh = vi.fn().mockResolvedValue(undefined); + const thread = makeSlackThread({ + refresh, + recentMessages: [ + { + isMe: false, + raw: { + files: [ + { + id: "F500", + name: "earlier.png", + mimetype: "image/png", + url_private: "https://files.slack.com/a/b/earlier.png", + }, + ], + }, + }, + { isMe: false, raw: {} }, + ], + }); + + const parts = await collectInboundFileParts({ + mention: emptyMention, + thread, + policy: DEFAULT_UPLOAD_POLICY, + }); + + expect(parts).toHaveLength(1); + expect((parts[0]!.data as URL).href).toBe("https://files.slack.com/a/b/earlier.png"); + }); + + it("collects files from multiple thread messages, newest first", async () => { + const refresh = vi.fn().mockResolvedValue(undefined); + const thread = makeSlackThread({ + refresh, + recentMessages: [ + { + isMe: false, + raw: { + files: [ + { + id: "F600", + mimetype: "text/csv", + url_private: "https://files.slack.com/a/b/older.csv", + }, + ], + }, + }, + { + isMe: false, + raw: { + files: [ + { + id: "F601", + mimetype: "image/png", + url_private: "https://files.slack.com/a/b/newer.png", + }, + ], + }, + }, + ], + }); + + const parts = await collectInboundFileParts({ + mention: emptyMention, + thread, + policy: DEFAULT_UPLOAD_POLICY, + }); + + expect(parts).toHaveLength(2); + expect((parts[0]!.data as URL).href).toBe("https://files.slack.com/a/b/newer.png"); + expect((parts[1]!.data as URL).href).toBe("https://files.slack.com/a/b/older.csv"); + }); + + it("caps thread-history collection at 10 file parts", async () => { + const refresh = vi.fn().mockResolvedValue(undefined); + const makeFiles = (prefix: string, count: number) => + Array.from({ length: count }, (_, i) => ({ + id: `${prefix}${i}`, + mimetype: "image/png", + url_private: `https://files.slack.com/a/b/${prefix}${i}.png`, + })); + const thread = makeSlackThread({ + refresh, + recentMessages: [ + { isMe: false, raw: { files: makeFiles("old", 6) } }, + { isMe: false, raw: { files: makeFiles("new", 6) } }, + ], + }); + + const parts = await collectInboundFileParts({ + mention: emptyMention, + thread, + policy: DEFAULT_UPLOAD_POLICY, + }); + + expect(parts).toHaveLength(10); + expect((parts[0]!.data as URL).href).toBe("https://files.slack.com/a/b/new0.png"); + expect((parts[9]!.data as URL).href).toBe("https://files.slack.com/a/b/old3.png"); + }); + it.each([ ["'disabled' literal", DISABLED_POLICY], ["maxBytes: 0", ZERO_BYTES_POLICY], diff --git a/packages/eve/src/public/channels/slack/attachments.ts b/packages/eve/src/public/channels/slack/attachments.ts index 9c1c6fe24..d8470b73e 100644 --- a/packages/eve/src/public/channels/slack/attachments.ts +++ b/packages/eve/src/public/channels/slack/attachments.ts @@ -66,17 +66,24 @@ function toSlackFilePart(attachment: SlackAttachment, index: number): FilePart | }; } +/** + * Upper bound on file parts collected from thread history for a single + * turn, keeping model input bounded on long, attachment-heavy threads. + */ +const THREAD_LOOKBACK_MAX_FILES = 10; + /** * Collects file parts for an inbound mention. * * Prefers attachments on the triggering mention (the common case: user * uploads a file and mentions the bot in the same message). When the * mention has none, refreshes the thread via {@link SlackThread.refresh} - * and picks the latest non-bot message's attachments — covering the - * case where a user dropped a file in the thread first, then mentioned - * the bot in a follow-up. Any error during refresh is logged and treated - * as "no attachments" so the text portion of the mention still gets - * delivered. + * and collects attachments from non-bot messages across the thread, + * newest first, capped at {@link THREAD_LOOKBACK_MAX_FILES} — covering + * the case where a user dropped a file in the thread first, then + * mentioned the bot in a follow-up. Any error during refresh is logged + * and treated as "no attachments" so the text portion of the mention + * still gets delivered. * * Skips the thread-history lookback when the policy disables uploads, * since the refresh can't surface anything we'd deliver. @@ -100,16 +107,16 @@ export async function collectInboundFileParts(input: { } const recent = input.thread.recentMessages; - for (let i = recent.length - 1; i >= 0; i -= 1) { + const collected: FilePart[] = []; + for (let i = recent.length - 1; i >= 0 && collected.length < THREAD_LOOKBACK_MAX_FILES; i -= 1) { const candidate = recent[i]; if (!candidate || candidate.isMe) continue; const raw = candidate.raw as { files?: readonly Record[] } | undefined; const attachments = extractAttachmentsFromRaw(raw?.files); const parts = collectSlackFileParts(attachments, input.policy); - if (parts.length > 0) return parts; - return []; + collected.push(...parts.slice(0, THREAD_LOOKBACK_MAX_FILES - collected.length)); } - return []; + return collected; } function extractAttachmentsFromRaw(