fix: require @-mention for follow-up replies#6
Merged
Conversation
Pookie previously answered every reply in any thread it had been engaged in, because onSubscribedMessage handled all follow-ups and the onNewMessage catch-all responded to non-mention replies whenever the bot had participated in the thread. Per the Chat SDK event model, threads stay subscribed but the bot now only responds when explicitly addressed: onSubscribedMessage and the catch-all both gate on a new isSlackBotAddressed helper (message.isMention with a raw text-scan fallback for the app_mention/message dedupe race). DMs are unchanged since they are direct 1:1 conversations. Removes the now-dead isTopLevelSlackMessage / hasBotParticipatedInThread helpers and the subscribeForFallback path. Co-authored-by: Ray Arayilakath <me@rayhanadev.com>
Member
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ebad81. Configure here.
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
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
Pookie no longer responds to thread follow-ups automatically. It now requires an explicit
@-mention to reply.Why
Previously, once Pookie was engaged in a thread (via a mention or DM), it would answer every subsequent human reply:
onSubscribedMessagehandled all follow-ups in subscribed threads, mention or not.onNewMessagecatch-all also answered non-mention replies whenever the bot had previously participated in the thread.This made Pookie chime in on conversations between humans that happened to occur in a thread it had once been pinged in.
How
Per the Chat SDK event model, threads stay subscribed (so future mentions still route to
onSubscribedMessage), but the bot only responds when explicitly addressed:isSlackBotAddressedhelper inmessage-routing.tsthat checksmessage.isMention, with a raw-text scan fallback. The fallback is needed because the Slack adapter only setsisMention=trueforapp_mention-typed events, and Slack'sapp_mention/messagededupe race can drop that flag.onSubscribedMessageand theonNewMessagecatch-all now both gate onisSlackBotAddressedand return early otherwise.onDirectMessage) are intentionally left unchanged — they are direct 1:1 conversations where requiring a mention makes no sense.isTopLevelSlackMessage/hasBotParticipatedInThreadhelpers, thesubscribeForFallbackpath, and their tests; added coverage forisSlackBotAddressed.Testing
pnpm typecheck✅pnpm test(275 tests) ✅pnpm lint✅pnpm format:check✅Slack Thread