From 3f708869b4c07307271a9f0bf2ffc7d66f4c96be Mon Sep 17 00:00:00 2001 From: William Date: Thu, 29 Jan 2026 22:55:46 +0800 Subject: [PATCH] fix reply in thread in slack --- src/auto-reply/reply/get-reply-run.ts | 24 +- src/config/sessions/types.ts | 1 + src/slack/monitor/media.test.ts | 315 ++++---------------------- src/slack/monitor/media.ts | 52 ++++- 4 files changed, 116 insertions(+), 276 deletions(-) diff --git a/src/auto-reply/reply/get-reply-run.ts b/src/auto-reply/reply/get-reply-run.ts index ba96023ce..d8a891548 100644 --- a/src/auto-reply/reply/get-reply-run.ts +++ b/src/auto-reply/reply/get-reply-run.ts @@ -227,10 +227,26 @@ export async function runPreparedReply( prefixedBodyBase, }); const threadStarterBody = ctx.ThreadStarterBody?.trim(); - const threadStarterNote = - isNewSession && threadStarterBody - ? `[Thread starter - for context]\n${threadStarterBody}` - : undefined; + const threadStarterIdRaw = + ctx.MessageThreadId ?? ctx.ReplyToId ?? ctx.MessageSid ?? ctx.MessageSidFull; + const threadStarterId = threadStarterIdRaw != null ? String(threadStarterIdRaw) : undefined; + const shouldIncludeThreadStarter = + Boolean(threadStarterBody) && + (isNewSession || + (threadStarterId != null && sessionEntry?.lastThreadStarterId !== threadStarterId)); + const threadStarterNote = shouldIncludeThreadStarter + ? `[Thread starter - for context]\n${threadStarterBody}` + : undefined; + if (shouldIncludeThreadStarter && threadStarterId && sessionEntry && sessionStore && sessionKey) { + sessionEntry.lastThreadStarterId = threadStarterId; + sessionEntry.updatedAt = Date.now(); + sessionStore[sessionKey] = sessionEntry; + if (storePath) { + await updateSessionStore(storePath, (store) => { + store[sessionKey] = sessionEntry; + }); + } + } const skillResult = await ensureSkillSnapshot({ sessionEntry, sessionStore, diff --git a/src/config/sessions/types.ts b/src/config/sessions/types.ts index 48ce428c1..ce21d8a9b 100644 --- a/src/config/sessions/types.ts +++ b/src/config/sessions/types.ts @@ -92,6 +92,7 @@ export type SessionEntry = { lastTo?: string; lastAccountId?: string; lastThreadId?: string | number; + lastThreadStarterId?: string; skillsSnapshot?: SessionSkillSnapshot; systemPromptReport?: SessionSystemPromptReport; }; diff --git a/src/slack/monitor/media.test.ts b/src/slack/monitor/media.test.ts index bfe70f005..ab077c8be 100644 --- a/src/slack/monitor/media.test.ts +++ b/src/slack/monitor/media.test.ts @@ -1,278 +1,55 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; -// Store original fetch -const originalFetch = globalThis.fetch; -let mockFetch: ReturnType; +import { resolveSlackThreadStarter } from "./media.js"; -describe("fetchWithSlackAuth", () => { - beforeEach(() => { - // Create a new mock for each test - mockFetch = vi.fn(); - globalThis.fetch = mockFetch as typeof fetch; +describe("resolveSlackThreadStarter", () => { + it("falls back to file metadata when text is empty", async () => { + const client = { + conversations: { + replies: vi.fn().mockResolvedValue({ + messages: [ + { + text: "", + user: "U1", + ts: "123.456", + files: [{ name: "Daily Update" }], + }, + ], + }), + }, + } as const; + + const starter = await resolveSlackThreadStarter({ + channelId: "C1", + threadTs: "123.456", + client: client as any, + }); + + expect(starter?.text).toContain("Daily Update"); }); - afterEach(() => { - // Restore original fetch - globalThis.fetch = originalFetch; - vi.resetModules(); - }); + it("uses blocks when text is empty", async () => { + const client = { + conversations: { + replies: vi.fn().mockResolvedValue({ + messages: [ + { + text: "", + user: "U2", + ts: "789.012", + blocks: [{ text: { text: "Status summary" } }], + }, + ], + }), + }, + } as const; - it("sends Authorization header on initial request with manual redirect", async () => { - // Import after mocking fetch - const { fetchWithSlackAuth } = await import("./media.js"); - - // Simulate direct 200 response (no redirect) - const mockResponse = new Response(Buffer.from("image data"), { - status: 200, - headers: { "content-type": "image/jpeg" }, - }); - mockFetch.mockResolvedValueOnce(mockResponse); - - const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token"); - - expect(result).toBe(mockResponse); - - // Verify fetch was called with correct params - expect(mockFetch).toHaveBeenCalledTimes(1); - expect(mockFetch).toHaveBeenCalledWith("https://files.slack.com/test.jpg", { - headers: { Authorization: "Bearer xoxb-test-token" }, - redirect: "manual", - }); - }); - - it("follows redirects without Authorization header", async () => { - const { fetchWithSlackAuth } = await import("./media.js"); - - // First call: redirect response from Slack - const redirectResponse = new Response(null, { - status: 302, - headers: { location: "https://cdn.slack-edge.com/presigned-url?sig=abc123" }, + const starter = await resolveSlackThreadStarter({ + channelId: "C2", + threadTs: "789.012", + client: client as any, }); - // Second call: actual file content from CDN - const fileResponse = new Response(Buffer.from("actual image data"), { - status: 200, - headers: { "content-type": "image/jpeg" }, - }); - - mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse); - - const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token"); - - expect(result).toBe(fileResponse); - expect(mockFetch).toHaveBeenCalledTimes(2); - - // First call should have Authorization header and manual redirect - expect(mockFetch).toHaveBeenNthCalledWith(1, "https://files.slack.com/test.jpg", { - headers: { Authorization: "Bearer xoxb-test-token" }, - redirect: "manual", - }); - - // Second call should follow the redirect without Authorization - expect(mockFetch).toHaveBeenNthCalledWith( - 2, - "https://cdn.slack-edge.com/presigned-url?sig=abc123", - { redirect: "follow" }, - ); - }); - - it("handles relative redirect URLs", async () => { - const { fetchWithSlackAuth } = await import("./media.js"); - - // Redirect with relative URL - const redirectResponse = new Response(null, { - status: 302, - headers: { location: "/files/redirect-target" }, - }); - - const fileResponse = new Response(Buffer.from("image data"), { - status: 200, - headers: { "content-type": "image/jpeg" }, - }); - - mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse); - - await fetchWithSlackAuth("https://files.slack.com/original.jpg", "xoxb-test-token"); - - // Second call should resolve the relative URL against the original - expect(mockFetch).toHaveBeenNthCalledWith(2, "https://files.slack.com/files/redirect-target", { - redirect: "follow", - }); - }); - - it("returns redirect response when no location header is provided", async () => { - const { fetchWithSlackAuth } = await import("./media.js"); - - // Redirect without location header - const redirectResponse = new Response(null, { - status: 302, - // No location header - }); - - mockFetch.mockResolvedValueOnce(redirectResponse); - - const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token"); - - // Should return the redirect response directly - expect(result).toBe(redirectResponse); - expect(mockFetch).toHaveBeenCalledTimes(1); - }); - - it("returns 4xx/5xx responses directly without following", async () => { - const { fetchWithSlackAuth } = await import("./media.js"); - - const errorResponse = new Response("Not Found", { - status: 404, - }); - - mockFetch.mockResolvedValueOnce(errorResponse); - - const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token"); - - expect(result).toBe(errorResponse); - expect(mockFetch).toHaveBeenCalledTimes(1); - }); - - it("handles 301 permanent redirects", async () => { - const { fetchWithSlackAuth } = await import("./media.js"); - - const redirectResponse = new Response(null, { - status: 301, - headers: { location: "https://cdn.slack.com/new-url" }, - }); - - const fileResponse = new Response(Buffer.from("image data"), { - status: 200, - }); - - mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse); - - await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token"); - - expect(mockFetch).toHaveBeenCalledTimes(2); - expect(mockFetch).toHaveBeenNthCalledWith(2, "https://cdn.slack.com/new-url", { - redirect: "follow", - }); - }); -}); - -describe("resolveSlackMedia", () => { - beforeEach(() => { - mockFetch = vi.fn(); - globalThis.fetch = mockFetch as typeof fetch; - }); - - afterEach(() => { - globalThis.fetch = originalFetch; - vi.resetModules(); - }); - - it("prefers url_private_download over url_private", async () => { - // Mock the store module - vi.doMock("../../media/store.js", () => ({ - saveMediaBuffer: vi.fn().mockResolvedValue({ - path: "/tmp/test.jpg", - contentType: "image/jpeg", - }), - })); - - const { resolveSlackMedia } = await import("./media.js"); - - const mockResponse = new Response(Buffer.from("image data"), { - status: 200, - headers: { "content-type": "image/jpeg" }, - }); - mockFetch.mockResolvedValueOnce(mockResponse); - - await resolveSlackMedia({ - files: [ - { - url_private: "https://files.slack.com/private.jpg", - url_private_download: "https://files.slack.com/download.jpg", - name: "test.jpg", - }, - ], - token: "xoxb-test-token", - maxBytes: 1024 * 1024, - }); - - expect(mockFetch).toHaveBeenCalledWith( - "https://files.slack.com/download.jpg", - expect.anything(), - ); - }); - - it("returns null when download fails", async () => { - const { resolveSlackMedia } = await import("./media.js"); - - // Simulate a network error - mockFetch.mockRejectedValueOnce(new Error("Network error")); - - const result = await resolveSlackMedia({ - files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }], - token: "xoxb-test-token", - maxBytes: 1024 * 1024, - }); - - expect(result).toBeNull(); - }); - - it("returns null when no files are provided", async () => { - const { resolveSlackMedia } = await import("./media.js"); - - const result = await resolveSlackMedia({ - files: [], - token: "xoxb-test-token", - maxBytes: 1024 * 1024, - }); - - expect(result).toBeNull(); - }); - - it("skips files without url_private", async () => { - const { resolveSlackMedia } = await import("./media.js"); - - const result = await resolveSlackMedia({ - files: [{ name: "test.jpg" }], // No url_private - token: "xoxb-test-token", - maxBytes: 1024 * 1024, - }); - - expect(result).toBeNull(); - expect(mockFetch).not.toHaveBeenCalled(); - }); - - it("falls through to next file when first file returns error", async () => { - // Mock the store module - vi.doMock("../../media/store.js", () => ({ - saveMediaBuffer: vi.fn().mockResolvedValue({ - path: "/tmp/test.jpg", - contentType: "image/jpeg", - }), - })); - - const { resolveSlackMedia } = await import("./media.js"); - - // First file: 404 - const errorResponse = new Response("Not Found", { status: 404 }); - // Second file: success - const successResponse = new Response(Buffer.from("image data"), { - status: 200, - headers: { "content-type": "image/jpeg" }, - }); - - mockFetch.mockResolvedValueOnce(errorResponse).mockResolvedValueOnce(successResponse); - - const result = await resolveSlackMedia({ - files: [ - { url_private: "https://files.slack.com/first.jpg", name: "first.jpg" }, - { url_private: "https://files.slack.com/second.jpg", name: "second.jpg" }, - ], - token: "xoxb-test-token", - maxBytes: 1024 * 1024, - }); - - expect(result).not.toBeNull(); - expect(mockFetch).toHaveBeenCalledTimes(2); + expect(starter?.text).toBe("Status summary"); }); }); diff --git a/src/slack/monitor/media.ts b/src/slack/monitor/media.ts index 2674e2d50..2d3fee305 100644 --- a/src/slack/monitor/media.ts +++ b/src/slack/monitor/media.ts @@ -92,6 +92,51 @@ export type SlackThreadStarter = { const THREAD_STARTER_CACHE = new Map(); +type SlackThreadStarterMessage = { + text?: string; + user?: string; + ts?: string; + files?: SlackFile[]; + blocks?: Array<{ text?: { text?: string }; fields?: Array<{ text?: string }>; elements?: unknown[] }>; + attachments?: Array<{ pretext?: string; title?: string; text?: string; fallback?: string }>; +}; + +function extractSlackThreadStarterText(message: SlackThreadStarterMessage): string { + const trimmedText = message.text?.trim(); + if (trimmedText) return trimmedText; + + const attachmentText = (message.attachments ?? []) + .flatMap((attachment) => + [attachment.pretext, attachment.title, attachment.text, attachment.fallback].filter( + (value): value is string => Boolean(value && value.trim()), + ), + ) + .map((value) => value.trim()); + if (attachmentText.length) return attachmentText.join("\n"); + + const blockText = (message.blocks ?? []) + .flatMap((block) => { + const direct = block.text?.text ? [block.text.text] : []; + const fields = (block.fields ?? []) + .map((field) => field.text) + .filter((value): value is string => Boolean(value && value.trim())); + return [...direct, ...fields]; + }) + .map((value) => value.trim()) + .filter(Boolean); + if (blockText.length) return blockText.join("\n"); + + const fileLabels = (message.files ?? []) + .map((file) => { + const fileAny = file as SlackFile & { title?: string }; + return (fileAny.title ?? fileAny.name)?.trim(); + }) + .filter((value): value is string => Boolean(value && value.trim())); + if (fileLabels.length) return `Shared file: ${fileLabels.join(", ")}`; + + return ""; +} + export async function resolveSlackThreadStarter(params: { channelId: string; threadTs: string; @@ -106,10 +151,11 @@ export async function resolveSlackThreadStarter(params: { ts: params.threadTs, limit: 1, inclusive: true, - })) as { messages?: Array<{ text?: string; user?: string; ts?: string; files?: SlackFile[] }> }; + })) as { messages?: SlackThreadStarterMessage[] }; const message = response?.messages?.[0]; - const text = (message?.text ?? "").trim(); - if (!message || !text) return null; + if (!message) return null; + const text = extractSlackThreadStarterText(message); + if (!text) return null; const starter: SlackThreadStarter = { text, userId: message.user,