Skip to content

Commit 30d48d8

Browse files
chechunhsuclaude
andcommitted
fix(slack): guard API calls with threadTs || undefined to fix invalid_thread_ts
Preserve empty threadTs for top-level DMs (matches openDM subscriptions) while guarding postMessage, chatStream, and setStatus calls to prevent Slack API "invalid_thread_ts" errors from empty strings. This addresses review feedback: the two-part fix keeps openDM() subscription matching working while preventing API errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b48fc21 commit 30d48d8

4 files changed

Lines changed: 42 additions & 36 deletions

File tree

.changeset/fix-dm-thread-ts.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@chat-adapter/slack": patch
33
---
44

5-
Fix DM messages failing with `invalid_thread_ts` by using `event.ts` as fallback for threadTs in DMs
5+
Fix DM messages failing with `invalid_thread_ts` by guarding Slack API calls with `threadTs || undefined`

packages/adapter-slack/src/index.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,7 +1636,7 @@ describe("DM message handling", () => {
16361636

16371637
expect(chatInstance.processMessage).toHaveBeenCalledWith(
16381638
adapter,
1639-
"slack:D_DM_CHAN:1234567890.111111",
1639+
"slack:D_DM_CHAN:",
16401640
expect.any(Function),
16411641
undefined
16421642
);
@@ -2253,7 +2253,7 @@ describe("postMessage", () => {
22532253
expect(client.chat.postMessage).toHaveBeenCalledWith(
22542254
expect.objectContaining({
22552255
channel: "C123",
2256-
thread_ts: "",
2256+
thread_ts: undefined,
22572257
})
22582258
);
22592259
});
@@ -3303,7 +3303,7 @@ describe("postChannelMessage", () => {
33033303
expect(client.chat.postMessage).toHaveBeenCalledWith(
33043304
expect.objectContaining({
33053305
channel: "C123",
3306-
thread_ts: "",
3306+
thread_ts: undefined,
33073307
})
33083308
);
33093309
});

packages/adapter-slack/src/index.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,7 +1322,11 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
13221322
return;
13231323
}
13241324

1325-
const threadTs = event.thread_ts || event.ts;
1325+
// For DMs: top-level messages use empty threadTs (matches openDM subscriptions),
1326+
// thread replies use thread_ts for per-conversation isolation.
1327+
// For channels: always use thread_ts or ts for per-thread IDs.
1328+
const isDM = event.channel_type === "im";
1329+
const threadTs = isDM ? event.thread_ts || "" : event.thread_ts || event.ts;
13261330
const threadId = this.encodeThreadId({
13271331
channel: event.channel,
13281332
threadTs,
@@ -2203,7 +2207,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
22032207
const result = await this.client.chat.postMessage(
22042208
this.withToken({
22052209
channel,
2206-
thread_ts: threadTs,
2210+
thread_ts: threadTs || undefined,
22072211
text: fallbackText, // Fallback for notifications
22082212
blocks,
22092213
unfurl_links: false,
@@ -2235,7 +2239,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
22352239
const result = await this.client.chat.postMessage(
22362240
this.withToken({
22372241
channel,
2238-
thread_ts: threadTs,
2242+
thread_ts: threadTs || undefined,
22392243
text: tableResult.text,
22402244
blocks: tableResult.blocks,
22412245
unfurl_links: false,
@@ -2270,7 +2274,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
22702274
const result = await this.client.chat.postMessage(
22712275
this.withToken({
22722276
channel,
2273-
thread_ts: threadTs,
2277+
thread_ts: threadTs || undefined,
22742278
text,
22752279
unfurl_links: false,
22762280
unfurl_media: false,
@@ -2887,7 +2891,7 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
28872891
await this.client.assistant.threads.setStatus(
28882892
this.withToken({
28892893
channel_id: channel,
2890-
thread_ts: threadTs,
2894+
thread_ts: (threadTs || undefined) as string,
28912895
status: status ?? "Typing...",
28922896
loading_messages: [status ?? "Typing..."],
28932897
})
@@ -2929,9 +2933,11 @@ export class SlackAdapter implements Adapter<SlackThreadId, unknown> {
29292933
this.logger.debug("Slack: starting stream", { channel, threadTs });
29302934

29312935
const token = this.getToken();
2936+
// Guard against empty threadTs from DM subscriptions (openDM returns empty threadTs)
2937+
// to avoid Slack API "invalid_thread_ts" errors
29322938
const streamer = this.client.chatStream({
29332939
channel,
2934-
thread_ts: threadTs,
2940+
thread_ts: (threadTs || undefined) as string,
29352941
recipient_user_id: options.recipientUserId,
29362942
recipient_team_id: options.recipientTeamId,
29372943
...(options.taskDisplayMode && {

packages/integration-tests/src/replay-dm.test.ts

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ describe("DM Replay Tests", () => {
206206
expect(state.mentionMessage).not.toBeNull();
207207
});
208208

209-
it("should receive user DM as new mention (threadTs differs from openDM subscription)", async () => {
209+
it("should receive user reply in DM when subscribed", async () => {
210210
// Step 1: Initial mention to subscribe
211211
await sendWebhook(slackFixtures.mention);
212212
expect(state.mentionMessage).not.toBeNull();
@@ -216,22 +216,23 @@ describe("DM Replay Tests", () => {
216216
expect(state.openDMCalled).toBe(true);
217217
expect(state.dmThreadId).not.toBeNull();
218218

219-
// Reset mention state to track the DM message
220-
state.mentionMessage = null;
221-
222219
// Step 3: User sends message in DM
223-
// With the threadTs fix, top-level DM messages use event.ts as threadTs,
224-
// which differs from the openDM subscription's empty threadTs.
225-
// So this arrives as a new mention rather than a subscribed message.
226220
await sendWebhook(slackFixtures.dmMessage);
227221

228-
// DM arrives as new mention (isMention=true for DMs)
229-
const dmMentionMsg = state.mentionMessage as Message | null;
230-
expect(dmMentionMsg).not.toBeNull();
231-
expect(dmMentionMsg?.text).toBe("Hey!");
222+
// Verify the DM reply was captured
223+
expect(state.dmMessage).not.toBeNull();
224+
expect(state.dmMessage?.text).toBe("Hey!");
232225

233226
// Verify the DM message is identified as im channel type
234227
expect(slackFixtures.dmMessage.event.channel_type).toBe("im");
228+
229+
// Verify bot responded to the DM
230+
expect(mockSlackClient.chat.postMessage).toHaveBeenCalledWith(
231+
expect.objectContaining({
232+
channel: slackFixtures.dmChannelId,
233+
text: expect.stringContaining("Got your DM: Hey!"),
234+
})
235+
);
235236
});
236237
});
237238

@@ -299,13 +300,13 @@ describe("DM Replay Tests", () => {
299300
expect(state.mentionMessage?.text).toBe("hello hello");
300301
});
301302

302-
it("should use event.ts as threadTs for top-level DM messages", async () => {
303+
it("should use empty threadTs for top-level DM messages", async () => {
303304
await sendWebhook(slackDirectFixtures.directDM);
304305

305306
expect(state.mentionMessage).not.toBeNull();
306-
// Top-level DM → threadId uses event.ts as threadTs
307+
// Top-level DM → threadId is "slack:<channel>:" with empty threadTs
307308
expect(state.mentionMessage?.threadId).toBe(
308-
`slack:${slackDirectFixtures.dmChannelId}:${slackDirectFixtures.directDM.event.ts}`
309+
`slack:${slackDirectFixtures.dmChannelId}:`
309310
);
310311
});
311312

@@ -320,24 +321,23 @@ describe("DM Replay Tests", () => {
320321
);
321322
});
322323

323-
it("should receive follow-up DM as new mention (different threadTs)", async () => {
324+
it("should receive follow-up DM as subscribed message", async () => {
324325
// First DM triggers onNewMention and subscribes
325326
await sendWebhook(slackDirectFixtures.directDM);
326327
expect(state.mentionMessage).not.toBeNull();
327328

328-
// Reset to track the second mention
329-
const firstMention = state.mentionMessage;
330-
state.mentionMessage = null;
331-
332-
// Second DM has different ts, so gets a different threadId
333-
// and is treated as a new mention rather than a subscribed message
329+
// Second DM (real recorded follow-up) triggers onSubscribedMessage
334330
await sendWebhook(slackDirectFixtures.followUp);
335331

336-
const followUpMention = state.mentionMessage as Message | null;
337-
expect(followUpMention).not.toBeNull();
338-
expect(followUpMention?.text).toBe("cool!!");
339-
// Verify it's a different thread than the first DM
340-
expect(followUpMention?.threadId).not.toBe(firstMention?.threadId);
332+
expect(state.dmMessage).not.toBeNull();
333+
expect(state.dmMessage?.text).toBe("cool!!");
334+
335+
expect(mockSlackClient.chat.postMessage).toHaveBeenCalledWith(
336+
expect.objectContaining({
337+
channel: slackDirectFixtures.dmChannelId,
338+
text: expect.stringContaining("Follow-up: cool!!"),
339+
})
340+
);
341341
});
342342

343343
it("should use thread_ts for DM thread replies", async () => {

0 commit comments

Comments
 (0)