fix(slack): guard Slack API calls against empty threadTs to fix invalid_thread_ts

[chechunhsu]

Summary

Fixes #268

The empty threadTs for top-level DMs was intentionally added in #39 so that openDM() subscriptions match incoming DM messages. The previous approach of changing the threadTs logic to event.thread_ts || event.ts fixed the API error but broke subscription matching — DMs routed to onNewMention() instead of onSubscribedMessage().

This PR takes the two-part approach suggested in review feedback: preserve the empty threadTs for DM subscription matching, and guard Slack API calls so empty strings don't cause invalid_thread_ts errors.

What changed

Each method that calls Slack APIs (postMessage, postEphemeral, scheduleMessage, stream) now normalizes threadTs at entry:

const { channel, threadTs: rawThreadTs } = this.decodeThreadId(threadId);
const threadTs = rawThreadTs || undefined;

This follows the same pattern postEphemeral already used (thread_ts: threadTs || undefined), but applied once at method entry instead of at each call site.

Result

• openDM() subscription matching continues to work (empty threadTs in threadId)
• Slack API calls receive undefined instead of "" → no invalid_thread_ts errors
• No behavioral change for channel messages or DM thread replies

Test plan

• pnpm test --filter=@chat-adapter/slack — 297 tests pass
• pnpm test --filter=@chat-adapter/integration-tests — replay-dm.test.ts 17 tests pass (DM subscription matching verified)
• pnpm turbo build --filter=@chat-adapter/slack — builds cleanly with no type errors

[vercel]

@chechunhsu is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

Additional Suggestion:

Test description claims "top-level DM messages use empty threadTs (matches openDM subscriptions)" but the test assertion expects a non-empty threadTs using event.ts, contradicting the description.

[bensabic]

Thanks for the fix, the invalid_thread_ts bug is definitely real and needs to be addressed.

However, the empty threadTs for top-level DMs was intentional (added in #39) so that openDM() subscriptions would match incoming DM messages. This PR fixes the API error but breaks that subscription matching, DMs now route to onNewMention() instead of onSubscribedMessage(), which is a regression for apps using openDM().

The issue author actually suggested a two-part fix: use event.ts as the fallback and guard the API calls with thread_ts: threadTs || undefined (like postEphemeral already does). Could we take that approach instead? That way postMessage/chatStream won't choke on empty threadTs, and openDM() subscription matching continues to work.

[chechunhsu]

Thanks for the detailed feedback! You're right — the empty threadTs for top-level DMs is intentional for openDM() subscription matching, and my original approach broke that.

I've updated the PR to take the two-part approach you suggested: keep the existing DM threadTs logic unchanged, and guard the Slack API calls against empty strings instead.

Specifically, each method that calls Slack APIs (postMessage, postEphemeral, scheduleMessage, stream) now normalizes threadTs at entry:

const { channel, threadTs: rawThreadTs } = this.decodeThreadId(threadId);
const threadTs = rawThreadTs || undefined;

This follows the pattern postEphemeral already used, but applied once per method rather than at each call site. All existing tests pass, including the DM subscription matching tests in replay-dm.test.ts.

[bensabic]

Thanks for the updated approach, guarding with rawThreadTs || undefined instead of changing how threadTs is computed is the right call. It preserves openDM() subscription matching while fixing the API errors.

Two things on the as string casts:

1. startTyping (line 2898): This one is fine since there's already an early return on if (!threadTs) above it. The cast is redundant though, you could drop it.

2. chatStream (line 2944): This one is unsafe. There's no guard, so if threadTs is undefined, it passes undefined to chat.startStream which requires thread_ts: string. That's exactly the invalid_arguments error from DMs broken: empty threadTs causes invalid_thread_ts and invalid_arguments #268. It probably can't happen in practice today (streaming is triggered from incoming messages which always have a valid event.ts), but the cast silences TypeScript rather than handling the case. Could you add the same guard that startTyping has, early return or fallback when threadTs is undefined?

[chechunhsu]

Updated:

1. startTyping: Removed the redundant as string cast — TypeScript already narrows after the if (!threadTs) guard.

2. stream: Added a ValidationError guard when threadTs is empty (matching startTyping's pattern), so chatStream receives a properly narrowed string — no more as string cast.

Zero as string casts remaining.