fix: use reactions.list API and post kickoff message for reaction triggers - #88
Merged
Conversation
…ggers The reaction trigger feature was broken because _check_reaction_triggers used conversations.history with a forward-moving cursor (_last_seen_ts), which never returned messages that already existed when a reaction was added. Replaced with Slack's reactions.list API which queries by reaction recency, not message age. Also fixed thread creation to post a kickoff message in the self-DM and use its ts as the thread root, matching the existing @mention-triggered session flow.
Enhanced the instructions for the end-to-end deployment process by specifying that it takes around 5-10 minutes to start when the app compute is off, ensuring users have a clearer expectation of the deployment duration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke and why
The original implementation (#86) used
conversations.historywith_last_seen_tsto scan for reactions. This was fundamentally wrong:_last_seen_tsis a forward-moving watermark — it only fetches messages newer than the last poll_last_seen_tsThe second bug: the session was keyed on the original message
tsin the original channel, not in the self-DM. This broke the assumption that all sessions live inself._channel_id(the self-DM).Fix
conversations.historywithreactions.list— the correct API for "what messages has this user recently reacted to"tsas the thread root, matching the existing @mention-triggered session flowmsg_tsas an anchor (stored in a dummy session) to prevent re-triggering on re-pollsRoot cause in our design process
We modeled reaction detection as "messages polling" (Phase 3 reusing the history-scan pattern) when it should have been modeled as "event polling" using the dedicated Slack reactions API.
Fixes #85