From 73243a2b961ef0539b3ab6c020f23883b5b43bdf Mon Sep 17 00:00:00 2001 From: vincezh2000 Date: Fri, 17 Jul 2026 14:14:30 -0700 Subject: [PATCH] fix(slack): make attachments visible in thread/channel history; pass all mention images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thread and channel history serialized only msg.text, so a file-only message (an uploaded screenshot, invoice, PDF) rendered as an empty line — when a user asked "@bot summarize the invoices above", the bot could not even tell files existed and had to ask for a re-upload. - fetchThreadHistory / fetchChannelHistory now parse files[] and render one note per attachment: downloaded ones as [image saved: /path] / [file "name" saved: /path] (reusing downloadSlackFile, newest-first, capped at 6 per fetch to bound latency), voice as a marker, the rest as [attachment "name" (type) — not downloaded] so nothing is invisible. - History blocks append a one-line instruction to read saved paths when any attachment was downloaded. - Triggering messages now pass the first 5 attached images to the prompt instead of only imageFiles[0]. - New unit tests for the pure helpers (tests/slack-history.test.ts); suite result unchanged apart from the 9 new passing tests. Co-Authored-By: Claude Fable 5 --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- src/commands/slack.ts | 110 +++++++++++++++++++++++++++----- tests/slack-history.test.ts | 88 +++++++++++++++++++++++++ 4 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 tests/slack-history.test.ts diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index f5430793..a38a6a41 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,7 +8,7 @@ "name": "claudeclaw", "source": "./", "description": "Cron-like daemon that runs Claude prompts on a schedule", - "version": "1.0.40", + "version": "1.0.41", "keywords": [ "cron", "heartbeat", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 2688e3ab..e2be5931 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,5 +1,5 @@ { "name": "claudeclaw", - "version": "1.0.40", + "version": "1.0.41", "description": "Cron-like daemon that runs Claude prompts on a schedule" } diff --git a/src/commands/slack.ts b/src/commands/slack.ts index f9f03d56..453f307f 100644 --- a/src/commands/slack.ts +++ b/src/commands/slack.ts @@ -600,15 +600,18 @@ async function fetchThreadHistory( user?: string; bot_id?: string; ts: string; + files?: SlackFile[]; }>; }; if (!data.ok) { throw new Error(`conversations.replies error: ${data.error ?? "unknown"}`); } - return (data.messages ?? []).map((msg) => ({ + const msgs = data.messages ?? []; + const resolved = await downloadHistoryFiles(token, msgs); + return msgs.map((msg) => ({ role: msg.bot_id ? "assistant" : "user", - text: msg.text, + text: appendFileNotes(msg.text, msg.files, resolved), user: msg.user, ts: msg.ts, })); @@ -625,6 +628,9 @@ function formatThreadHistoryAsContext( lines.push(`[${sender}]: ${msg.text}`); } lines.push("--- End of Thread History ---"); + if (historyHasSavedAttachment(messages.map((m) => m.text))) { + lines.push("(Attachments in the history above are saved to local paths — read the relevant ones directly before answering.)"); + } lines.push(""); return lines.join("\n"); } @@ -791,16 +797,21 @@ async function fetchChannelHistory( const data = await res.json() as { ok: boolean; error?: string; - messages?: Array<{ text: string; user?: string; bot_id?: string; ts: string }>; + messages?: Array<{ text: string; user?: string; bot_id?: string; ts: string; files?: SlackFile[] }>; }; if (!data.ok) { return `Error reading channel ${channelId}: ${data.error ?? "unknown"}`; } const msgs = (data.messages ?? []).reverse(); + const resolved = await downloadHistoryFiles(token, msgs); + const rendered = msgs.map((msg) => sanitizeUserInput(appendFileNotes(msg.text, msg.files, resolved))); const lines = [`--- Channel ${channelId} History (${msgs.length} messages) ---`]; - for (const msg of msgs) { - const sender = msg.bot_id ? "Bot" : `User ${msg.user ?? "unknown"}`; - lines.push(`[${sender}]: ${sanitizeUserInput(msg.text)}`); + for (let i = 0; i < msgs.length; i++) { + const sender = msgs[i].bot_id ? "Bot" : `User ${msgs[i].user ?? "unknown"}`; + lines.push(`[${sender}]: ${rendered[i]}`); + } + if (historyHasSavedAttachment(rendered)) { + lines.push("(Attachments above are saved to local paths — read the relevant ones directly when needed.)"); } lines.push("--- End ---"); return lines.join("\n"); @@ -858,6 +869,66 @@ function isDocumentFile(f: SlackFile): boolean { return !isImageFile(f) && !isVoiceFile(f) && Boolean(f.url_private); } +// --- History attachments --- +// Thread/channel history used to serialize only msg.text, so a file-only +// message (an uploaded screenshot, invoice, PDF) rendered as an empty line — +// the model could not even know the file existed. Download a bounded number +// of history attachments (newest first) and reference their local paths +// inline so the model can read them like triggering-message attachments. + +const HISTORY_FILE_DOWNLOAD_CAP = 6; + +function formatHistoryFileNote(f: SlackFile, localPath: string | null): string { + if (isVoiceFile(f)) return "[voice message — not transcribed]"; + const name = (f.name ?? f.id).slice(0, 80); + if (localPath) { + return isImageFile(f) ? `[image saved: ${localPath}]` : `[file "${name}" saved: ${localPath}]`; + } + return `[attachment "${name}" (${f.filetype ?? f.mimetype ?? "file"}) — not downloaded]`; +} + +async function downloadHistoryFiles( + token: string, + msgs: { files?: SlackFile[] }[], + cap: number = HISTORY_FILE_DOWNLOAD_CAP, +): Promise> { + const resolved = new Map(); + let downloaded = 0; + // Iterate newest-first so the download budget favors recent files. + for (let i = msgs.length - 1; i >= 0; i--) { + for (const f of msgs[i].files ?? []) { + if (!f.id || resolved.has(f.id)) continue; + if (isVoiceFile(f) || !f.url_private || downloaded >= cap) { + resolved.set(f.id, null); + continue; + } + try { + const p = await downloadSlackFile(token, f, isImageFile(f) ? "image" : "document"); + resolved.set(f.id, p); + if (p) downloaded++; + } catch (err) { + debugLog(`History file download failed (${f.id}): ${err instanceof Error ? err.message : err}`); + resolved.set(f.id, null); + } + } + } + return resolved; +} + +function appendFileNotes( + text: string, + files: SlackFile[] | undefined, + resolved: Map, +): string { + if (!files || files.length === 0) return text; + const notes = files.map((f) => formatHistoryFileNote(f, resolved.get(f.id) ?? null)); + return [text, ...notes].filter((s) => s && s.trim()).join("\n"); +} + +function historyHasSavedAttachment(texts: string[]): boolean { + return texts.some((t) => t.includes(" saved: ")); +} + function isBotMentioned(text: string): boolean { if (!botUserId) return false; return text.includes(`<@${botUserId}>`); @@ -981,16 +1052,21 @@ async function handleMessage(event: SlackMessage): Promise { threadHistoryLoaded.set(sessionThreadId, Date.now()); } - let imagePath: string | null = null; + const imagePaths: string[] = []; let voicePath: string | null = null; let voiceTranscript: string | null = null; const docPaths: { path: string; name: string }[] = []; if (hasImage) { - try { - imagePath = await downloadSlackFile(config.botToken, imageFiles[0], "image"); - } catch (err) { - console.error(`[Slack] Failed to download image: ${err instanceof Error ? err.message : err}`); + // First 5 images, not just the first — a message can carry several + // screenshots/photos and each may matter. + for (const imageFile of imageFiles.slice(0, 5)) { + try { + const p = await downloadSlackFile(config.botToken, imageFile, "image"); + if (p) imagePaths.push(p); + } catch (err) { + console.error(`[Slack] Failed to download image: ${err instanceof Error ? err.message : err}`); + } } } @@ -1066,9 +1142,13 @@ async function handleMessage(event: SlackMessage): Promise { } else if (cleanText.trim()) { promptParts.push(`Message: ${cleanText}`); } - if (imagePath) { - promptParts.push(`Image path: ${imagePath}`); - promptParts.push("The user attached an image. Inspect this image file directly before answering."); + if (imagePaths.length > 0) { + for (const p of imagePaths) promptParts.push(`Image path: ${p}`); + promptParts.push( + imagePaths.length === 1 + ? "The user attached an image. Inspect this image file directly before answering." + : `The user attached ${imagePaths.length} images${imageFiles.length > 5 ? ` (first 5 of ${imageFiles.length})` : ""}. Inspect every image path above before answering.`, + ); } else if (hasImage) { promptParts.push("The user attached an image, but downloading it failed. Respond and ask them to resend."); } @@ -1646,7 +1726,7 @@ function scheduleReconnect(appToken: string): void { // --- Exports --- -export { sendMessage, sanitizeUserInput, extractChannelReadDirectives, extractReactionDirective, assistantKey }; +export { sendMessage, sanitizeUserInput, extractChannelReadDirectives, extractReactionDirective, assistantKey, formatHistoryFileNote, appendFileNotes, formatThreadHistoryAsContext }; export async function sendMessageToUser( token: string, diff --git a/tests/slack-history.test.ts b/tests/slack-history.test.ts new file mode 100644 index 00000000..b2b8eee2 --- /dev/null +++ b/tests/slack-history.test.ts @@ -0,0 +1,88 @@ +import { test, expect } from "bun:test"; +import { + formatHistoryFileNote, + appendFileNotes, + formatThreadHistoryAsContext, +} from "../src/commands/slack"; + +// --- formatHistoryFileNote: one line per attachment, by kind --- + +test("downloaded image renders as a saved-path note", () => { + expect( + formatHistoryFileNote({ id: "F1", name: "invoice.jpg", mimetype: "image/jpeg" }, "/tmp/inbox/F1.jpg"), + ).toBe("[image saved: /tmp/inbox/F1.jpg]"); +}); + +test("downloaded document renders with its filename", () => { + expect( + formatHistoryFileNote( + { id: "F2", name: "report.pdf", mimetype: "application/pdf", url_private: "https://x" }, + "/tmp/inbox/F2.pdf", + ), + ).toBe('[file "report.pdf" saved: /tmp/inbox/F2.pdf]'); +}); + +test("voice files are marked, never downloaded", () => { + expect(formatHistoryFileNote({ id: "F3", mimetype: "audio/ogg" }, null)).toBe( + "[voice message — not transcribed]", + ); +}); + +test("undownloaded attachment still becomes visible with name and type", () => { + expect( + formatHistoryFileNote({ id: "F4", name: "big.zip", filetype: "zip", url_private: "https://x" }, null), + ).toBe('[attachment "big.zip" (zip) — not downloaded]'); +}); + +test("nameless file falls back to its id and mimetype", () => { + expect(formatHistoryFileNote({ id: "F5", mimetype: "application/octet-stream" }, null)).toBe( + '[attachment "F5" (application/octet-stream) — not downloaded]', + ); +}); + +// --- appendFileNotes: the core fix — file-only messages are no longer blank --- + +test("a file-only message renders its attachment note instead of an empty line", () => { + const resolved = new Map([["F1", "/tmp/inbox/F1.jpg"]]); + expect(appendFileNotes("", [{ id: "F1", name: "invoice.jpg", mimetype: "image/jpeg" }], resolved)).toBe( + "[image saved: /tmp/inbox/F1.jpg]", + ); +}); + +test("text and multiple attachments combine line by line", () => { + const resolved = new Map([ + ["F1", "/tmp/inbox/F1.jpg"], + ["F2", null], + ]); + const out = appendFileNotes( + "see these", + [ + { id: "F1", name: "a.png", mimetype: "image/png" }, + { id: "F2", name: "b.pdf", filetype: "pdf", url_private: "https://x" }, + ], + resolved, + ); + expect(out).toBe('see these\n[image saved: /tmp/inbox/F1.jpg]\n[attachment "b.pdf" (pdf) — not downloaded]'); +}); + +test("messages without files pass through untouched", () => { + expect(appendFileNotes("plain text", undefined, new Map())).toBe("plain text"); + expect(appendFileNotes("plain text", [], new Map())).toBe("plain text"); +}); + +// --- formatThreadHistoryAsContext: read-your-attachments instruction --- + +test("thread history with a saved attachment appends the read instruction", () => { + const ctx = formatThreadHistoryAsContext([ + { role: "user", text: "here is the invoice\n[image saved: /tmp/inbox/F1.jpg]", user: "U1", ts: "1" }, + ]); + expect(ctx).toContain("[image saved: /tmp/inbox/F1.jpg]"); + expect(ctx).toContain("read the relevant ones directly"); +}); + +test("thread history without attachments has no instruction line", () => { + const ctx = formatThreadHistoryAsContext([ + { role: "user", text: "just words", user: "U1", ts: "1" }, + ]); + expect(ctx).not.toContain("read the relevant ones directly"); +});