diff --git a/packages/twenty-apps/public/slack/src/logic-functions/types/slack-events-request-body.type.ts b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-events-request-body.type.ts index bdfb7ae0bf..ee6d40e0ca 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/types/slack-events-request-body.type.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/types/slack-events-request-body.type.ts @@ -1,3 +1,8 @@ +type SlackEventAuthorization = { + user_id?: string; + is_bot?: boolean; +}; + type SlackInboundEvent = { type?: string; subtype?: string; @@ -15,5 +20,6 @@ export type SlackEventsRequestBody = { challenge?: string; event_id?: string; team_id?: string; + authorizations?: SlackEventAuthorization[]; event?: SlackInboundEvent; }; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-assistant-request.test.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-assistant-request.test.ts index a0e68932e3..d1c9b2dfe9 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-assistant-request.test.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/parse-slack-assistant-request.test.ts @@ -2,18 +2,28 @@ import { describe, expect, it } from 'vitest'; import { parseSlackAssistantRequest } from 'src/logic-functions/utils/parse-slack-assistant-request'; -const buildMentionBody = (overrides: Record = {}) => ({ +const BOT_AUTHORIZATIONS = [{ user_id: 'UBOT', is_bot: true }]; + +const buildMentionBody = ({ + eventOverrides = {}, + bodyOverrides = {}, +}: { + eventOverrides?: Record; + bodyOverrides?: Record; +} = {}) => ({ type: 'event_callback', event_id: 'Ev123', team_id: 'T123', + authorizations: BOT_AUTHORIZATIONS, event: { type: 'app_mention', user: 'U123', text: '<@UBOT> create an invoice for ACME', ts: '1700000000.000100', channel: 'C123', - ...overrides, + ...eventOverrides, }, + ...bodyOverrides, }); describe('parseSlackAssistantRequest', () => { @@ -34,10 +44,10 @@ describe('parseSlackAssistantRequest', () => { }); }); - it('should keep other user mentions when stripping the leading bot mention', () => { + it('should keep other user mentions when stripping the bot mention', () => { const result = parseSlackAssistantRequest( buildMentionBody({ - text: '<@UBOT> ask <@UALICE> about the ACME deal', + eventOverrides: { text: '<@UBOT> ask <@UALICE> about the ACME deal' }, }), ); @@ -46,6 +56,81 @@ describe('parseSlackAssistantRequest', () => { ); }); + it('should replace a mid-text bot mention with you', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { text: 'hey <@UBOT>, who owns ACME?' }, + }), + ); + + expect(result.request?.requestText).toBe('hey you, who owns ACME?'); + }); + + it('should drop the punctuation left behind by a leading bot mention', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { text: '<@UBOT>, who owns ACME?' }, + }), + ); + + expect(result.request?.requestText).toBe('who owns ACME?'); + }); + + it('should drop the leading bot mention and replace the mid-text one', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { + text: '<@UBOT> can <@UBOT> list open deals for ACME?', + }, + }), + ); + + expect(result.request?.requestText).toBe( + 'can you list open deals for ACME?', + ); + }); + + it('should strip a repeated bot mention using the leading mention when authorizations are missing', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { + text: '<@UBOT> what does <@UBOT|twenty> know about ACME?', + }, + bodyOverrides: { authorizations: undefined }, + }), + ); + + expect(result.request?.requestText).toBe('what does you know about ACME?'); + }); + + it('should keep other user mentions when stripping a mid-text bot mention', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { + text: 'hey <@UBOT> ask <@UALICE> about the ACME deal', + }, + }), + ); + + expect(result.request?.requestText).toBe( + 'hey you ask <@UALICE> about the ACME deal', + ); + }); + + it('should keep a leading other-user mention when the bot id is known', () => { + const result = parseSlackAssistantRequest( + buildMentionBody({ + eventOverrides: { + text: '<@UALICE> and <@UBOT> should review the ACME deal', + }, + }), + ); + + expect(result.request?.requestText).toBe( + '<@UALICE> and you should review the ACME deal', + ); + }); + it('should preserve user mentions on unmentioned thread follow-ups', () => { const result = parseSlackAssistantRequest({ type: 'event_callback', @@ -77,7 +162,7 @@ describe('parseSlackAssistantRequest', () => { it('should keep the thread timestamp when mentioned inside a thread', () => { const result = parseSlackAssistantRequest( - buildMentionBody({ thread_ts: '1699999999.000001' }), + buildMentionBody({ eventOverrides: { thread_ts: '1699999999.000001' } }), ); expect(result.request?.slackThreadTimestamp).toBe('1699999999.000001'); @@ -108,9 +193,46 @@ describe('parseSlackAssistantRequest', () => { }); }); + it('should strip the bot mention from a direct message', () => { + const result = parseSlackAssistantRequest({ + type: 'event_callback', + event_id: 'Ev456', + authorizations: BOT_AUTHORIZATIONS, + event: { + type: 'message', + channel_type: 'im', + user: 'U123', + text: 'hey <@UBOT>, how many open opportunities do we have?', + ts: '1700000000.000200', + channel: 'D123', + }, + }); + + expect(result.request?.requestText).toBe( + 'hey you, how many open opportunities do we have?', + ); + }); + + it('should keep mentions in a direct message when the bot id is unknown', () => { + const result = parseSlackAssistantRequest({ + type: 'event_callback', + event_id: 'Ev456', + event: { + type: 'message', + channel_type: 'im', + user: 'U123', + text: 'ping <@UBOT> about the ACME deal', + ts: '1700000000.000200', + channel: 'D123', + }, + }); + + expect(result.request?.requestText).toBe('ping <@UBOT> about the ACME deal'); + }); + it('should skip messages sent by bots so the assistant never answers itself', () => { const result = parseSlackAssistantRequest( - buildMentionBody({ bot_id: 'B123' }), + buildMentionBody({ eventOverrides: { bot_id: 'B123' } }), ); expect(result.request).toBeNull(); @@ -118,7 +240,7 @@ describe('parseSlackAssistantRequest', () => { it('should skip message subtypes such as edits', () => { const result = parseSlackAssistantRequest( - buildMentionBody({ subtype: 'message_changed' }), + buildMentionBody({ eventOverrides: { subtype: 'message_changed' } }), ); expect(result.request).toBeNull(); @@ -143,7 +265,7 @@ describe('parseSlackAssistantRequest', () => { it('should flag a mention with no remaining text for a hint reply', () => { const result = parseSlackAssistantRequest( - buildMentionBody({ text: '<@UBOT>' }), + buildMentionBody({ eventOverrides: { text: '<@UBOT>' } }), ); expect(result).toEqual({ @@ -160,7 +282,9 @@ describe('parseSlackAssistantRequest', () => { it('should target the existing thread when an empty mention is inside one', () => { const result = parseSlackAssistantRequest( - buildMentionBody({ text: '<@UBOT>', thread_ts: '1699999999.000001' }), + buildMentionBody({ + eventOverrides: { text: '<@UBOT>', thread_ts: '1699999999.000001' }, + }), ); expect(result).toMatchObject({ diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/strip-slack-bot-mention.test.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/strip-slack-bot-mention.test.ts new file mode 100644 index 0000000000..9903424d83 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/__tests__/strip-slack-bot-mention.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest'; + +import { stripSlackBotMention } from 'src/logic-functions/utils/strip-slack-bot-mention'; + +const stripBotMention = (text: string): string => + stripSlackBotMention({ text, botUserId: 'UBOT' }); + +describe('stripSlackBotMention', () => { + it('should drop a leading bot mention', () => { + expect(stripBotMention('<@UBOT> list open deals')).toBe('list open deals'); + }); + + it('should replace a mid-text bot mention with you', () => { + expect(stripBotMention('can <@UBOT> list open deals?')).toBe( + 'can you list open deals?', + ); + }); + + it('should drop the leading mention and replace the mid-text one', () => { + expect(stripBotMention('<@UBOT> can <@UBOT> list open deals?')).toBe( + 'can you list open deals?', + ); + }); + + it('should replace the labelled mention form', () => { + expect(stripBotMention('ask <@UBOT|twenty> about ACME')).toBe( + 'ask you about ACME', + ); + }); + + it('should keep the punctuation tight around the replacement', () => { + expect(stripBotMention('hey <@UBOT>, who owns ACME?')).toBe( + 'hey you, who owns ACME?', + ); + }); + + it('should drop the punctuation left behind by a leading mention', () => { + expect(stripBotMention('<@UBOT>, who owns ACME?')).toBe('who owns ACME?'); + }); + + it('should collapse consecutive mentions into a single replacement', () => { + expect(stripBotMention('hey <@UBOT> <@UBOT>, who owns ACME?')).toBe( + 'hey you, who owns ACME?', + ); + }); + + it('should keep word boundaries around a mention glued to text', () => { + expect(stripBotMention('please<@UBOT>review the ACME deal')).toBe( + 'please you review the ACME deal', + ); + }); + + it('should keep other user mentions', () => { + expect(stripBotMention('<@UBOT> ask <@UALICE> about ACME')).toBe( + 'ask <@UALICE> about ACME', + ); + }); + + it('should not touch a mention whose id merely starts with the bot id', () => { + expect(stripBotMention('ping <@UBOTHER> about ACME')).toBe( + 'ping <@UBOTHER> about ACME', + ); + }); + + it('should return the text unchanged for a malformed bot id', () => { + expect( + stripSlackBotMention({ text: 'hey <@UBOT> there', botUserId: 'U+.*' }), + ).toBe('hey <@UBOT> there'); + }); +}); diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/get-slack-bot-user-id-from-event-body.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/get-slack-bot-user-id-from-event-body.ts new file mode 100644 index 0000000000..15c91c62ec --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/get-slack-bot-user-id-from-event-body.ts @@ -0,0 +1,11 @@ +import { isNonEmptyString } from '@sniptt/guards'; + +import { type SlackEventsRequestBody } from 'src/logic-functions/types/slack-events-request-body.type'; + +export const getSlackBotUserIdFromEventBody = ( + body: Pick, +): string | undefined => + body.authorizations?.find( + (authorization) => + authorization.is_bot && isNonEmptyString(authorization.user_id), + )?.user_id; diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-assistant-request.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-assistant-request.ts index 43477ae9c4..b23a7e98f3 100644 --- a/packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-assistant-request.ts +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/parse-slack-assistant-request.ts @@ -4,8 +4,10 @@ import { type SlackAssistantEmptyRequest } from 'src/logic-functions/types/slack import { type SlackAssistantRequestDraft } from 'src/logic-functions/types/slack-assistant-request-draft.type'; import { type SlackEventsRequestBody } from 'src/logic-functions/types/slack-events-request-body.type'; import { getSlackAssistantParentMessageTimestamp } from 'src/logic-functions/utils/get-slack-assistant-parent-message-timestamp'; +import { getSlackBotUserIdFromEventBody } from 'src/logic-functions/utils/get-slack-bot-user-id-from-event-body'; +import { stripSlackBotMention } from 'src/logic-functions/utils/strip-slack-bot-mention'; -const LEADING_BOT_MENTION_PATTERN = /^<@[A-Z0-9]+(\|[^>]*)?>\s*/; +const LEADING_MENTION_PATTERN = /^<@([A-Z0-9]+)(\|[^>]*)?>/; type SlackInboundEvent = NonNullable; @@ -47,19 +49,28 @@ const classifySlackAssistantEvent = ( return null; }; -const stripLeadingBotMention = (text: string): string => - text.replace(LEADING_BOT_MENTION_PATTERN, '').replace(/\s+/g, ' ').trim(); +const getBotUserIdFromLeadingMention = (text: string): string | undefined => + text.trimStart().match(LEADING_MENTION_PATTERN)?.[1]; const normalizeSlackRequestText = ({ text, kind, + botUserId, }: { text: string; kind: SlackAssistantEventKind; -}): string => - kind === 'mention' - ? stripLeadingBotMention(text) - : text.replace(/\s+/g, ' ').trim(); + botUserId: string | undefined; +}): string => { + const resolvedBotUserId = + botUserId ?? + (kind === 'mention' ? getBotUserIdFromLeadingMention(text) : undefined); + + const strippedText = isNonEmptyString(resolvedBotUserId) + ? stripSlackBotMention({ text, botUserId: resolvedBotUserId }) + : text; + + return strippedText.replace(/\s+/g, ' ').trim(); +}; export const parseSlackAssistantRequest = ( body: SlackEventsRequestBody, @@ -96,6 +107,7 @@ export const parseSlackAssistantRequest = ( const requestText = normalizeSlackRequestText({ text: event.text ?? '', kind, + botUserId: getSlackBotUserIdFromEventBody(body), }); if (!isNonEmptyString(requestText)) { diff --git a/packages/twenty-apps/public/slack/src/logic-functions/utils/strip-slack-bot-mention.ts b/packages/twenty-apps/public/slack/src/logic-functions/utils/strip-slack-bot-mention.ts new file mode 100644 index 0000000000..d2ca49bc99 --- /dev/null +++ b/packages/twenty-apps/public/slack/src/logic-functions/utils/strip-slack-bot-mention.ts @@ -0,0 +1,32 @@ +const SLACK_USER_ID_PATTERN = /^[A-Z0-9]+$/; +const PUNCTUATION_PATTERN = '[,.!?;:]'; +const MID_TEXT_MENTION_REPLACEMENT = 'you'; + +export const stripSlackBotMention = ({ + text, + botUserId, +}: { + text: string; + botUserId: string; +}): string => { + if (!SLACK_USER_ID_PATTERN.test(botUserId)) { + return text; + } + + const mentionPattern = `<@${botUserId}(?:\\|[^>]*)?>`; + const mentionRunPattern = `${mentionPattern}(?:\\s*${mentionPattern})*`; + + return text + .replace( + new RegExp(`^\\s*${mentionRunPattern}\\s*${PUNCTUATION_PATTERN}*\\s*`), + '', + ) + .replace( + new RegExp(`\\s*${mentionRunPattern}\\s*(${PUNCTUATION_PATTERN})`, 'g'), + ` ${MID_TEXT_MENTION_REPLACEMENT}$1`, + ) + .replace( + new RegExp(`\\s*${mentionRunPattern}\\s*`, 'g'), + ` ${MID_TEXT_MENTION_REPLACEMENT} `, + ); +};