Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/slack/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export function registerSlackEvents(
if (m.channel_type === "channel" || m.channel_type === "group" || m.channel_type === "mpim") {
if (m.channel_type === "mpim" && m.channel) syncForUnseenGroup(client, String(m.channel));
const threadReply = isThreadReply(m);
const isMention = mentionsBot(m.text ?? "", ids.botUserId);
const isMention = mentionsBot(m.text ?? "", ids.botUserId, ids.ownBotId);
const willDispatch = threadReply && !isMention && (await botHasStakeInThread(client, m.channel, m.thread_ts));
await mirrorMessageEvent(m, client, willDispatch ? { handled: true } : {});
if (!threadReply) return;
Expand Down
14 changes: 11 additions & 3 deletions src/slack/message-gating.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { LRUCache } from "lru-cache";

export function mentionsBot(text: string, botUserId: string): boolean {
return botUserId ? text.includes(`<@${botUserId}>`) : false;
export function mentionsBot(text: string, botUserId: string, ownBotId = ""): boolean {
// Both id forms mention the bot: the bot USER id (`<@U…>`, the form
// Slack's autocomplete inserts) and the BOT id (`<@B…>`, which
// hand-typed mentions and some clients encode). A B-form mention is a
// legitimate mention of this bot — `bots.info` maps it to the same
// single-bot app — but Slack fires no `app_mention` for it, so without
// recognizing it here the message is dropped at top level and
// misdispatched as ambient in threads (#630).
if (botUserId && text.includes(`<@${botUserId}>`)) return true;
return Boolean(ownBotId) && text.includes(`<@${ownBotId}>`);
}

export function threadHasBotStake(
Expand All @@ -17,7 +25,7 @@ export function threadHasBotStake(
return Boolean(
(botUserId && user === botUserId) ||
(ownBotId && bot === ownBotId) ||
(botUserId && mentionsBot(text, botUserId)),
mentionsBot(text, botUserId, ownBotId),
);
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/slack/mirror.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export function createMirror(deps: {
text,
...(Object.keys(mentions).length ? { mentions } : {}),
...(m.bot_id || m.bot_profile ? { bot: true } : {}),
...(mentionsBot(raw, ids.botUserId) ? { mentionsSelf: true } : {}),
...(mentionsBot(raw, ids.botUserId, ids.ownBotId) ? { mentionsSelf: true } : {}),
...(opts.editedAt ? { editedAt: opts.editedAt } : {}),
...(opts.handled ? { handled: true } : {}),
...(opts.containerName ? { containerName: opts.containerName } : {}),
Expand Down
11 changes: 9 additions & 2 deletions src/slack/mrkdwn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,15 @@ export function resolveMentionsInText(text: string, lookup: (id: string) => stri
});
}

export function stripMention(text: string, botUserId: string): string {
const withoutMention = botUserId ? text.replace(new RegExp(`<@${botUserId}>`, "g"), "") : text;
export function stripMention(text: string, botUserId: string, ownBotId = ""): string {
// Strip BOTH mention forms (bot user id and bot id — see
// mentionsBot) including the legacy `<@ID|label>` shape, so the core
// never receives a raw `<@B…>` token for an addressed turn (#630).
let withoutMention = text;
for (const id of [botUserId, ownBotId]) {
if (!id) continue;
withoutMention = withoutMention.replace(new RegExp(`<@${id}(?:\|[^>]*)?>`, "g"), "");
}
return decodeSlackEntities(withoutMention).trim();
}

Expand Down
2 changes: 1 addition & 1 deletion src/slack/turn-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export function createTurnHandler(deps: {
else classified = await classifyUserCached(client, inc.userId);
const actor = classified.actor;
const timezone = classified.timezone;
const text = stripMention(inc.rawText, ids.botUserId);
const text = stripMention(inc.rawText, ids.botUserId, ids.ownBotId);
if (!hasContent(text, inc.files)) return;

let audience: ActorAssertion[] = [actor];
Expand Down
13 changes: 13 additions & 0 deletions test/slack-message-gating.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,19 @@ test("dmThreadRef gives the DM main pane one continuous lane and each thread its
assert.notEqual(dmThreadRef("D1", "1699999999.000100"), dmThreadRef("D1"));
});

test("mentionsBot recognizes the bot-id form (<@B…>) alongside the user-id form (#630)", () => {
// A hand-typed mention can arrive encoded against the bot id; Slack fires
// no app_mention for it, so message-gating recognition is the only thing
// that can dispatch it.
assert.equal(mentionsBot("hey <@BOTID> do this", "UBOT", "BOTID"), true);
assert.equal(mentionsBot("<@UBOT> classic form", "UBOT", "BOTID"), true);
assert.equal(mentionsBot("both <@UBOT> <@BOTID>", "UBOT", "BOTID"), true);
assert.equal(mentionsBot("<@OTHERBOT> not me", "UBOT", "BOTID"), false);
assert.equal(mentionsBot("no ids configured", "", ""), false);
// Back-compat: the two-arg form still works.
assert.equal(mentionsBot("hey <@BOT> do this", "BOT"), true);
});

test("mentionsBot detects the bot @mention so thread-follow leaves those to app_mention", () => {
assert.equal(mentionsBot("hey <@BOT> do this", "BOT"), true);
assert.equal(mentionsBot("just a follow-up", "BOT"), false);
Expand Down
9 changes: 9 additions & 0 deletions test/slack-mrkdwn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ test("decodeSlackEntities / stripMention", () => {
assert.equal(stripMention("<@BOT>", "BOT"), "");
});

test("stripMention strips the bot-id form and the legacy label shape (#630)", () => {
// The core must not receive a raw <@B…> token for an addressed turn.
assert.equal(stripMention("<@BOTID> run the tests", "UBOT", "BOTID"), "run the tests");
assert.equal(stripMention("<@BOTID|qm> run the tests", "UBOT", "BOTID"), "run the tests");
assert.equal(stripMention("<@UBOT> classic", "UBOT", "BOTID"), "classic");
assert.equal(stripMention("<@UBOT|qm> legacy label", "UBOT", "BOTID"), "legacy label");
assert.equal(stripMention("unrelated <@OTHER> stays", "UBOT", "BOTID"), "unrelated <@OTHER> stays");
});

test("toSlackMrkdwn: bold uses single * (the screenshot bug: **x** rendered literally)", () => {
assert.equal(toSlackMrkdwn("**Git commands**"), "*Git commands*");
assert.equal(toSlackMrkdwn("__also bold__"), "*also bold*");
Expand Down