diff --git a/.changeset/fix-dm-thread-ts.md b/.changeset/fix-dm-thread-ts.md new file mode 100644 index 00000000..e8c23c10 --- /dev/null +++ b/.changeset/fix-dm-thread-ts.md @@ -0,0 +1,5 @@ +--- +"@chat-adapter/slack": patch +--- + +Fix DM messages failing with `invalid_thread_ts` by using `event.ts` as fallback for threadTs in DMs diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 91cd0989..e646c99e 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -1636,7 +1636,7 @@ describe("DM message handling", () => { expect(chatInstance.processMessage).toHaveBeenCalledWith( adapter, - "slack:D_DM_CHAN:", + "slack:D_DM_CHAN:1234567890.111111", expect.any(Function), undefined ); diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index ebc419d4..ab8c41de 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -1322,11 +1322,7 @@ export class SlackAdapter implements Adapter { return; } - // For DMs: top-level messages use empty threadTs (matches openDM subscriptions), - // thread replies use thread_ts for per-conversation isolation. - // For channels: always use thread_ts or ts for per-thread IDs. - const isDM = event.channel_type === "im"; - const threadTs = isDM ? event.thread_ts || "" : event.thread_ts || event.ts; + const threadTs = event.thread_ts || event.ts; const threadId = this.encodeThreadId({ channel: event.channel, threadTs, diff --git a/packages/integration-tests/src/replay-dm.test.ts b/packages/integration-tests/src/replay-dm.test.ts index 401b2f3d..56081329 100644 --- a/packages/integration-tests/src/replay-dm.test.ts +++ b/packages/integration-tests/src/replay-dm.test.ts @@ -206,7 +206,7 @@ describe("DM Replay Tests", () => { expect(state.mentionMessage).not.toBeNull(); }); - it("should receive user reply in DM when subscribed", async () => { + it("should receive user DM as new mention (threadTs differs from openDM subscription)", async () => { // Step 1: Initial mention to subscribe await sendWebhook(slackFixtures.mention); expect(state.mentionMessage).not.toBeNull(); @@ -216,23 +216,22 @@ describe("DM Replay Tests", () => { expect(state.openDMCalled).toBe(true); expect(state.dmThreadId).not.toBeNull(); + // Reset mention state to track the DM message + state.mentionMessage = null; + // Step 3: User sends message in DM + // With the threadTs fix, top-level DM messages use event.ts as threadTs, + // which differs from the openDM subscription's empty threadTs. + // So this arrives as a new mention rather than a subscribed message. await sendWebhook(slackFixtures.dmMessage); - // Verify the DM reply was captured - expect(state.dmMessage).not.toBeNull(); - expect(state.dmMessage?.text).toBe("Hey!"); + // DM arrives as new mention (isMention=true for DMs) + const dmMentionMsg = state.mentionMessage as Message | null; + expect(dmMentionMsg).not.toBeNull(); + expect(dmMentionMsg?.text).toBe("Hey!"); // Verify the DM message is identified as im channel type expect(slackFixtures.dmMessage.event.channel_type).toBe("im"); - - // Verify bot responded to the DM - expect(mockSlackClient.chat.postMessage).toHaveBeenCalledWith( - expect.objectContaining({ - channel: slackFixtures.dmChannelId, - text: expect.stringContaining("Got your DM: Hey!"), - }) - ); }); }); @@ -300,13 +299,13 @@ describe("DM Replay Tests", () => { expect(state.mentionMessage?.text).toBe("hello hello"); }); - it("should use empty threadTs for top-level DM messages", async () => { + it("should use event.ts as threadTs for top-level DM messages", async () => { await sendWebhook(slackDirectFixtures.directDM); expect(state.mentionMessage).not.toBeNull(); - // Top-level DM → threadId is "slack::" with empty threadTs + // Top-level DM → threadId uses event.ts as threadTs expect(state.mentionMessage?.threadId).toBe( - `slack:${slackDirectFixtures.dmChannelId}:` + `slack:${slackDirectFixtures.dmChannelId}:${slackDirectFixtures.directDM.event.ts}` ); }); @@ -321,23 +320,24 @@ describe("DM Replay Tests", () => { ); }); - it("should receive follow-up DM as subscribed message", async () => { + it("should receive follow-up DM as new mention (different threadTs)", async () => { // First DM triggers onNewMention and subscribes await sendWebhook(slackDirectFixtures.directDM); expect(state.mentionMessage).not.toBeNull(); - // Second DM (real recorded follow-up) triggers onSubscribedMessage - await sendWebhook(slackDirectFixtures.followUp); + // Reset to track the second mention + const firstMention = state.mentionMessage; + state.mentionMessage = null; - expect(state.dmMessage).not.toBeNull(); - expect(state.dmMessage?.text).toBe("cool!!"); + // Second DM has different ts, so gets a different threadId + // and is treated as a new mention rather than a subscribed message + await sendWebhook(slackDirectFixtures.followUp); - expect(mockSlackClient.chat.postMessage).toHaveBeenCalledWith( - expect.objectContaining({ - channel: slackDirectFixtures.dmChannelId, - text: expect.stringContaining("Follow-up: cool!!"), - }) - ); + const followUpMention = state.mentionMessage as Message | null; + expect(followUpMention).not.toBeNull(); + expect(followUpMention?.text).toBe("cool!!"); + // Verify it's a different thread than the first DM + expect(followUpMention?.threadId).not.toBe(firstMention?.threadId); }); it("should use thread_ts for DM thread replies", async () => {