Skip to content

Fix Slack thread ID to use message timestamp instead of channel ID#1513

Open
kevinicolas22 wants to merge 1 commit intomainfrom
fix-slack-thread-id
Open

Fix Slack thread ID to use message timestamp instead of channel ID#1513
kevinicolas22 wants to merge 1 commit intomainfrom
fix-slack-thread-id

Conversation

@kevinicolas22
Copy link
Contributor

@kevinicolas22 kevinicolas22 commented Jan 15, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved thread context handling in Slack channel operations to correctly identify messages within conversations.

✏️ Tip: You can customize this high-level summary in your review settings.

- Changed thread assignment in invoke.ts to use props.event.ts for proper threading in Slack channels
@github-actions
Copy link
Contributor

Tagging Options

Should a new tag be published when this PR is merged?

  • 👍 for Patch 0.133.25 update
  • 🎉 for Minor 0.134.0 update
  • 🚀 for Major 1.0.0 update

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

A single-line modification in Slack channel invocation logic changes the thread identifier source from channel ID to message timestamp in the non-direct-message execution path.

Changes

Cohort / File(s) Change Summary
Thread Identifier Update
slack/actions/deco-chat/channels/invoke.ts
Modified non-direct-message thread context: shifted thread identifier from props.event.channel to props.event.ts (message timestamp instead of channel ID)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • [feat: Slack]: Migrate to ai sdk v5 #1428: Refactors streaming logic that consumes the threadId/thread identifier modified by this change, making these modifications interdependent in thread context handling.

Poem

🐰 A timestamp now marks the thread's true home,
No longer lost in channels where we roam—
One tiny shift, from ID to moment's gleam,
Makes Slack conversations flow supreme! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author, but the description template requires sections for contribution details, issue links, video, and demonstration link. Add a pull request description following the template: include what this contribution is about, link to the related issue, provide a Loom video, and include a demonstration link for testing.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing the Slack thread ID to use message timestamp instead of channel ID, which matches the file change described in the summary.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dee3782 and 7cb4ee9.

📒 Files selected for processing (1)
  • slack/actions/deco-chat/channels/invoke.ts
🔇 Additional comments (1)
slack/actions/deco-chat/channels/invoke.ts (1)

25-27: LGTM! Correct fix for thread identification.

The change correctly uses props.event.ts (message timestamp) instead of props.event.channel for the thread identifier in non-DM channels. This aligns with Slack's threading model where ts uniquely identifies messages and their threads, and is consistent with how thread_ts is already used in the Slack API calls elsewhere in this file (lines 148, 255, 269).

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants