From d87e1ab1b4b386c4093c0e00a3cf3cf9ba243666 Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 13:05:39 +0200 Subject: [PATCH 1/7] feat: classify onboarding requests mentioned to the Slack bot (CM-1841) Signed-off-by: Umberto Sgueglia --- backend/src/api/index.ts | 3 +- backend/src/api/slack/events.test.ts | 73 +++++++++++++++ backend/src/api/slack/events.ts | 77 ++++++++++++++++ backend/src/api/slack/index.ts | 29 ++++++ .../slack/requestClassificationBot.test.ts | 45 ++++++++++ .../slack/requestClassificationBot.ts | 89 +++++++++++++++++++ pnpm-lock.yaml | 3 + .../src/activities/activities.ts | 17 ++-- .../src/activities/requestClassification.ts | 8 +- services/libs/project-onboarding/package.json | 1 + services/libs/project-onboarding/src/index.ts | 1 + .../src}/requestClassificationAlert.test.ts | 2 +- .../src}/requestClassificationAlert.ts | 10 ++- .../libs/project-onboarding/tsconfig.json | 3 + 14 files changed, 340 insertions(+), 21 deletions(-) create mode 100644 backend/src/api/slack/events.test.ts create mode 100644 backend/src/api/slack/events.ts create mode 100644 backend/src/services/slack/requestClassificationBot.test.ts create mode 100644 backend/src/services/slack/requestClassificationBot.ts rename services/{apps/automatic_projects_discovery_worker/src/activities => libs/project-onboarding/src}/requestClassificationAlert.test.ts (98%) rename services/{apps/automatic_projects_discovery_worker/src/activities => libs/project-onboarding/src}/requestClassificationAlert.ts (93%) diff --git a/backend/src/api/index.ts b/backend/src/api/index.ts index fc5d68501e..4aa7d6b611 100644 --- a/backend/src/api/index.ts +++ b/backend/src/api/index.ts @@ -33,7 +33,7 @@ import { tenantMiddleware } from '../middlewares/tenantMiddleware' import { createRateLimiter } from './apiRateLimiter' import authSocial from './auth/authSocial' import { publicRouter } from './public' -import { mountInteractivityRoute } from './slack' +import { mountEventsRoute, mountInteractivityRoute } from './slack' import WebSockets from './websockets' const serviceLogger = getServiceLogger() @@ -112,6 +112,7 @@ setImmediate(async () => { // Mounted before DB/Redis/OpenSearch middleware to protect Slack's 3s ack window. mountInteractivityRoute(app) + mountEventsRoute(app) // Initializes and adds the database middleware. app.use(databaseMiddleware) diff --git a/backend/src/api/slack/events.test.ts b/backend/src/api/slack/events.test.ts new file mode 100644 index 0000000000..8895bca466 --- /dev/null +++ b/backend/src/api/slack/events.test.ts @@ -0,0 +1,73 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('./verifySignature', () => ({ verifySlackSignature: vi.fn(() => true) })) +vi.mock('@/services/slack/requestClassificationBot', () => ({ + runRequestClassificationBot: vi.fn(async () => undefined), +})) + +import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' + +import events from './events' +import { verifySlackSignature } from './verifySignature' + +function call(body: object, headers: Record = {}) { + const req = { + body, + headers, + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + } as any + const res = { sendStatus: vi.fn(), json: vi.fn() } as any + return events(req, res).then(() => ({ req, res })) +} + +const mention = { + type: 'event_callback', + event: { type: 'app_mention', text: '<@U0BOT> hi', channel: 'C1', ts: '100.1' }, +} + +describe('slack events', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(verifySlackSignature).mockReturnValue(true) + }) + + it('answers the url verification challenge', async () => { + const { res } = await call({ type: 'url_verification', challenge: 'abc' }) + + expect(res.json).toHaveBeenCalledWith({ challenge: 'abc' }) + }) + + it('acks and classifies a mention, replying in the message thread', async () => { + const { res } = await call(mention) + + expect(res.sendStatus).toHaveBeenCalledWith(200) + expect(runRequestClassificationBot).toHaveBeenCalledWith( + expect.objectContaining({ text: '<@U0BOT> hi', channelId: 'C1', threadTs: '100.1' }), + ) + }) + + it('replies in the existing thread when the mention is inside one', async () => { + await call({ ...mention, event: { ...mention.event, thread_ts: '90.0' } }) + + expect(runRequestClassificationBot).toHaveBeenCalledWith( + expect.objectContaining({ threadTs: '90.0' }), + ) + }) + + it('ignores Slack retries, bot messages and other event types', async () => { + await call(mention, { 'x-slack-retry-num': '1' }) + await call({ ...mention, event: { ...mention.event, bot_id: 'B1' } }) + await call({ ...mention, event: { ...mention.event, type: 'message' } }) + + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) + + it('does nothing for unverified requests', async () => { + vi.mocked(verifySlackSignature).mockReturnValue(false) + + const { res } = await call(mention) + + expect(res.sendStatus).toHaveBeenCalledWith(200) + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) +}) diff --git a/backend/src/api/slack/events.ts b/backend/src/api/slack/events.ts new file mode 100644 index 0000000000..18f2b9f021 --- /dev/null +++ b/backend/src/api/slack/events.ts @@ -0,0 +1,77 @@ +import type { Request, Response } from 'express' +import { z } from 'zod' + +import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' +import { validateOrThrow } from '@/utils/validation' + +import { verifySlackSignature } from './verifySignature' + +const URL_VERIFICATION_TYPE = 'url_verification' +const EVENT_CALLBACK_TYPE = 'event_callback' +const APP_MENTION_EVENT_TYPE = 'app_mention' +const RETRY_HEADER = 'x-slack-retry-num' + +const payloadSchema = z.discriminatedUnion('type', [ + z.object({ type: z.literal(URL_VERIFICATION_TYPE), challenge: z.string() }), + z.object({ + type: z.literal(EVENT_CALLBACK_TYPE), + event: z.object({ + type: z.string(), + text: z.string().optional(), + channel: z.string().optional(), + ts: z.string().optional(), + thread_ts: z.string().optional(), + bot_id: z.string().optional(), + }), + }), +]) + +type EventPayload = z.infer + +function dispatchAppMention( + payload: Extract, + req: Request, +) { + const { event } = payload + const isUserMention = event.type === APP_MENTION_EVENT_TYPE && !event.bot_id + if (!isUserMention || !event.channel || !event.ts) { + return + } + + runRequestClassificationBot({ + text: event.text ?? '', + channelId: event.channel, + threadTs: event.thread_ts ?? event.ts, + options: { log: req.log }, + }).catch((err) => req.log.error(err, 'Slack request bot failed unexpectedly!')) +} + +// Mounted ahead of responseHandlerMiddleware, so errors are handled here +// directly instead of via the global errorMiddleware. +export default async (req: Request, res: Response) => { + if (!verifySlackSignature(req)) { + req.log.warn('Received unverified Slack event!') + res.sendStatus(200) + return + } + + try { + const payload = validateOrThrow(payloadSchema, req.body) + + if (payload.type === URL_VERIFICATION_TYPE) { + res.json({ challenge: payload.challenge }) + return + } + + res.sendStatus(200) + + if (req.headers[RETRY_HEADER]) { + return + } + + dispatchAppMention(payload, req) + } catch (err) { + req.log.error(err, 'Error processing Slack event!') + res.sendStatus(200) + } +} diff --git a/backend/src/api/slack/index.ts b/backend/src/api/slack/index.ts index 428b05c2f1..9803a6b4e0 100644 --- a/backend/src/api/slack/index.ts +++ b/backend/src/api/slack/index.ts @@ -38,6 +38,35 @@ export function mountInteractivityRoute(app: Application): void { ) } +export function mountEventsRoute(app: Application): void { + if (!getSlackBotConfig().signingSecret) { + return + } + + const captureRawBody = (req: Request, _res: Response, buf: Buffer) => { + req.rawBody = buf + } + + const eventsRateLimiter = createRateLimiter({ + max: 200, + windowMs: 60 * 1000, + }) + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const handleParserError = (err: Error, req: Request, res: Response, _next: NextFunction) => { + req.log.error(err, 'Error parsing Slack event payload!') + res.sendStatus(200) + } + + app.post( + '/v1/slack/events', + eventsRateLimiter, + bodyParser.json({ limit: '5mb', verify: captureRawBody }), + handleParserError, + require('./events').default, + ) +} + export default (app) => { if ( SLACK_CONFIG.onboardingAppId && diff --git a/backend/src/services/slack/requestClassificationBot.test.ts b/backend/src/services/slack/requestClassificationBot.test.ts new file mode 100644 index 0000000000..030bed0a37 --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' + +import { IRequestClassification } from '@crowd/project-onboarding' + +import { buildClassificationReply, toRequestText } from './requestClassificationBot' + +describe('toRequestText', () => { + it('removes the bot mention and keeps the rest of the message', () => { + expect(toRequestText('<@U0BOT> onboard Acme\nnot LF')).toBe('onboard Acme\nnot LF') + }) + + it('unwraps Slack links so the parser sees plain URLs', () => { + expect( + toRequestText( + '<@U0BOT> and ', + ), + ).toBe('https://github.com/acme/one and https://github.com/acme/two') + }) + + it('returns an empty string when only the mention is left', () => { + expect(toRequestText('<@U0BOT> ')).toBe('') + }) +}) + +describe('buildClassificationReply', () => { + const classification: IRequestClassification = { + resolution: { kind: 'lf_not_in_pcc', projectName: 'Acme' }, + node: 'lf_not_in_pcc_flag_human', + trace: { + parsed: null, + pccLookup: null, + cdpLookup: null, + failure: null, + }, + } + + it('names the step reached and states that nothing was done', () => { + const { blocks } = buildClassificationReply(classification, 'https://slack.test/thread') + const text = JSON.stringify(blocks) + + expect(text).toContain('lf_not_in_pcc_flag_human') + expect(text).toContain('[DRY RUN]') + expect(text).toContain('https://slack.test/thread') + }) +}) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts new file mode 100644 index 0000000000..a9bccb894c --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -0,0 +1,89 @@ +import { Message, Section, SlackMessageDto } from 'slack-block-builder' + +import { getErrorMessage } from '@crowd/common' +import { + IRequestClassification, + buildRequestClassificationAlert, + buildRequestClassificationAlertTitle, + classifyOnboardingRequest, +} from '@crowd/project-onboarding' +import { withRequestClassifierDeps } from '@crowd/project-onboarding/src/requestClassifierDeps' +import { getSlackPermalink, postSlackMessage } from '@crowd/slack' + +import { IServiceOptions } from '../IServiceOptions' +import { getBgQx } from './slackBackground' + +const MENTION_PATTERN = /<@[A-Z0-9]+>/g +const LINK_WITH_LABEL_PATTERN = /<(https?:\/\/[^|>\s]+)\|[^>]*>/g +const LINK_PATTERN = /<(https?:\/\/[^>\s]+)>/g + +const HELP_TEXT = + 'Tell me about the project you want to onboard: its name, whether it is a Linux Foundation project and the GitHub repositories.' + +export function toRequestText(slackText: string): string { + return slackText + .replace(MENTION_PATTERN, '') + .replace(LINK_WITH_LABEL_PATTERN, '$1') + .replace(LINK_PATTERN, '$1') + .trim() +} + +export function buildClassificationReply( + classification: IRequestClassification, + requestUrl: string, +): SlackMessageDto { + const alert = { + sourceUrl: requestUrl, + repoUrls: classification.trace.parsed?.githubRepoUrls ?? [], + resolution: classification.resolution, + dryRun: true, + } + const sections = buildRequestClassificationAlert(alert) + + return Message() + .blocks( + Section({ text: `*${buildRequestClassificationAlertTitle(alert)}*` }), + ...sections.map(({ title, text }) => Section({ text: title ? `*${title}*\n${text}` : text })), + Section({ text: `_Step reached: ${classification.node}_` }), + ) + .buildToObject() +} + +export async function runRequestClassificationBot({ + text, + channelId, + threadTs, + options, +}: { + text: string + channelId: string + threadTs: string + options: Pick +}): Promise { + const { log } = options + const reply = (message: SlackMessageDto | { text: string }) => + postSlackMessage({ channel: channelId, thread_ts: threadTs, ...message }) + + const requestText = toRequestText(text) + if (!requestText) { + await reply({ text: HELP_TEXT }) + return + } + + try { + const qx = await getBgQx() + const classification = await withRequestClassifierDeps(qx, (deps) => + classifyOnboardingRequest(requestText, deps), + ) + const requestUrl = (await getSlackPermalink(channelId, threadTs)) ?? '' + + log.info( + { channelId, threadTs, node: classification.node, kind: classification.resolution.kind }, + 'Onboarding request classified from Slack.', + ) + await reply(buildClassificationReply(classification, requestUrl)) + } catch (err) { + log.error({ error: getErrorMessage(err), channelId, threadTs }, 'Slack request failed.') + await reply({ text: ':no_entry: I could not process this request, please try again later.' }) + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 74143e26dc..5612fc1d7f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2804,6 +2804,9 @@ importers: '@crowd/logging': specifier: workspace:* version: link:../logging + '@crowd/slack': + specifier: workspace:* + version: link:../slack '@crowd/snowflake': specifier: workspace:* version: link:../snowflake diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts b/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts index e3288d3ae8..b603fbc968 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts +++ b/services/apps/automatic_projects_discovery_worker/src/activities/activities.ts @@ -23,22 +23,19 @@ import { } from '@crowd/data-access-layer/src/project-catalog/types' import { pgpQx } from '@crowd/data-access-layer/src/queryExecutor' import { getServiceLogger } from '@crowd/logging' -import { countNodes } from '@crowd/project-onboarding' +import { + IRequestClassificationAlert, + buildRequestClassificationAlert, + buildRequestClassificationAlertTitle, + countNodes, +} from '@crowd/project-onboarding' import { withRequestClassifierDeps } from '@crowd/project-onboarding/src/requestClassifierDeps' import { SlackChannel, SlackPersona, sendSlackNotificationAsync } from '@crowd/slack' import { svc } from '../main' import { getAvailableSourceNames, getSource } from '../sources/registry' import { IDatasetDescriptor } from '../sources/types' -import { - IClassifiedRows, - IRequestClassificationAlert, - classifyDiscussions, -} from './requestClassification' -import { - buildRequestClassificationAlert, - buildRequestClassificationAlertTitle, -} from './requestClassificationAlert' +import { IClassifiedRows, classifyDiscussions } from './requestClassification' const log = getServiceLogger() diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts b/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts index 7f5522e4d4..f48e60d11a 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts +++ b/services/apps/automatic_projects_discovery_worker/src/activities/requestClassification.ts @@ -3,6 +3,7 @@ import { getServiceLogger } from '@crowd/logging' import { CdpIntegrationAction, ClassificationNode, + IRequestClassificationAlert, IRequestClassificationDeps, OnboardingResolution, buildClassificationLogEntry, @@ -11,13 +12,6 @@ import { const log = getServiceLogger() -export interface IRequestClassificationAlert { - sourceUrl: string - repoUrls: string[] - resolution: OnboardingResolution - dryRun: boolean -} - export interface IClassifiedRows { rows: IDbProjectCatalogCreate[] alerts: IRequestClassificationAlert[] diff --git a/services/libs/project-onboarding/package.json b/services/libs/project-onboarding/package.json index 8b1afb76c0..7196c04a21 100644 --- a/services/libs/project-onboarding/package.json +++ b/services/libs/project-onboarding/package.json @@ -7,6 +7,7 @@ "@crowd/common_services": "workspace:*", "@crowd/data-access-layer": "workspace:*", "@crowd/logging": "workspace:*", + "@crowd/slack": "workspace:*", "@crowd/snowflake": "workspace:*", "@crowd/types": "workspace:*" }, diff --git a/services/libs/project-onboarding/src/index.ts b/services/libs/project-onboarding/src/index.ts index 8ea01d9871..25379ed7f5 100644 --- a/services/libs/project-onboarding/src/index.ts +++ b/services/libs/project-onboarding/src/index.ts @@ -2,6 +2,7 @@ export * from './classificationTrace' export * from './classifyRequest' export * from './onboarder' export * from './pccLookup' +export * from './requestClassificationAlert' export * from './requestParser' export * from './requestResolver' export * from './types' diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts similarity index 98% rename from services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts rename to services/libs/project-onboarding/src/requestClassificationAlert.test.ts index 8c68c5474e..d02456927c 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.test.ts +++ b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' -import { IRequestClassificationAlert } from './requestClassification' import { + IRequestClassificationAlert, buildRequestClassificationAlert, buildRequestClassificationAlertTitle, } from './requestClassificationAlert' diff --git a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.ts b/services/libs/project-onboarding/src/requestClassificationAlert.ts similarity index 93% rename from services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.ts rename to services/libs/project-onboarding/src/requestClassificationAlert.ts index 88cde55422..50c2c73315 100644 --- a/services/apps/automatic_projects_discovery_worker/src/activities/requestClassificationAlert.ts +++ b/services/libs/project-onboarding/src/requestClassificationAlert.ts @@ -1,7 +1,13 @@ -import { IPccCandidate, OnboardingResolution } from '@crowd/project-onboarding' import { SlackMessageSection } from '@crowd/slack' -import { IRequestClassificationAlert } from './requestClassification' +import { IPccCandidate, OnboardingResolution } from './requestResolver' + +export interface IRequestClassificationAlert { + sourceUrl: string + repoUrls: string[] + resolution: OnboardingResolution + dryRun: boolean +} const ALERT_TITLES: Record = { non_github_source: 'Onboarding request without GitHub repositories', diff --git a/services/libs/project-onboarding/tsconfig.json b/services/libs/project-onboarding/tsconfig.json index 16a548dcf8..6216af6521 100644 --- a/services/libs/project-onboarding/tsconfig.json +++ b/services/libs/project-onboarding/tsconfig.json @@ -14,6 +14,9 @@ { "path": "../logging" }, + { + "path": "../slack" + }, { "path": "../snowflake" }, From 92d52612f5984d0e61a17525039d66c2bf714669 Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 15:31:25 +0200 Subject: [PATCH 2/7] fix: address review on the Slack request bot (CM-1841) Signed-off-by: Umberto Sgueglia --- backend/src/api/index.ts | 2 +- backend/src/api/slack/eventDeduplication.ts | 12 ++++ backend/src/api/slack/events.test.ts | 41 +++++++++-- backend/src/api/slack/events.ts | 59 ++++++++++------ backend/src/api/slack/index.ts | 6 +- .../requestClassificationBot.run.test.ts | 68 +++++++++++++++++++ .../slack/requestClassificationBot.ts | 12 +++- .../src/requestClassificationAlert.test.ts | 24 +++++++ .../src/requestClassificationAlert.ts | 21 ++++-- 9 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 backend/src/api/slack/eventDeduplication.ts create mode 100644 backend/src/services/slack/requestClassificationBot.run.test.ts diff --git a/backend/src/api/index.ts b/backend/src/api/index.ts index 4aa7d6b611..cad8e1ae90 100644 --- a/backend/src/api/index.ts +++ b/backend/src/api/index.ts @@ -112,7 +112,7 @@ setImmediate(async () => { // Mounted before DB/Redis/OpenSearch middleware to protect Slack's 3s ack window. mountInteractivityRoute(app) - mountEventsRoute(app) + mountEventsRoute(app, redis) // Initializes and adds the database middleware. app.use(databaseMiddleware) diff --git a/backend/src/api/slack/eventDeduplication.ts b/backend/src/api/slack/eventDeduplication.ts new file mode 100644 index 0000000000..1f3d17afaa --- /dev/null +++ b/backend/src/api/slack/eventDeduplication.ts @@ -0,0 +1,12 @@ +import type { RedisClient } from '@crowd/redis' + +const EVENT_KEY_PREFIX = 'slack_event' +const EVENT_TTL_SECONDS = 5 * 60 + +export async function claimSlackEvent(redis: RedisClient, eventId: string): Promise { + const result = await redis.set(`${EVENT_KEY_PREFIX}:${eventId}`, '1', { + NX: true, + EX: EVENT_TTL_SECONDS, + }) + return result === 'OK' +} diff --git a/backend/src/api/slack/events.test.ts b/backend/src/api/slack/events.test.ts index 8895bca466..d5ca0526c0 100644 --- a/backend/src/api/slack/events.test.ts +++ b/backend/src/api/slack/events.test.ts @@ -7,9 +7,12 @@ vi.mock('@/services/slack/requestClassificationBot', () => ({ import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' -import events from './events' +import { createEventsHandler } from './events' import { verifySlackSignature } from './verifySignature' +const claimEvent = vi.fn(async () => true) +const events = createEventsHandler(claimEvent) + function call(body: object, headers: Record = {}) { const req = { body, @@ -22,6 +25,7 @@ function call(body: object, headers: Record = {}) { const mention = { type: 'event_callback', + event_id: 'Ev1', event: { type: 'app_mention', text: '<@U0BOT> hi', channel: 'C1', ts: '100.1' }, } @@ -29,6 +33,7 @@ describe('slack events', () => { beforeEach(() => { vi.clearAllMocks() vi.mocked(verifySlackSignature).mockReturnValue(true) + claimEvent.mockResolvedValue(true) }) it('answers the url verification challenge', async () => { @@ -42,7 +47,12 @@ describe('slack events', () => { expect(res.sendStatus).toHaveBeenCalledWith(200) expect(runRequestClassificationBot).toHaveBeenCalledWith( - expect.objectContaining({ text: '<@U0BOT> hi', channelId: 'C1', threadTs: '100.1' }), + expect.objectContaining({ + text: '<@U0BOT> hi', + channelId: 'C1', + messageTs: '100.1', + threadTs: '100.1', + }), ) }) @@ -50,12 +60,35 @@ describe('slack events', () => { await call({ ...mention, event: { ...mention.event, thread_ts: '90.0' } }) expect(runRequestClassificationBot).toHaveBeenCalledWith( - expect.objectContaining({ threadTs: '90.0' }), + expect.objectContaining({ threadTs: '90.0', messageTs: '100.1' }), ) }) - it('ignores Slack retries, bot messages and other event types', async () => { + it('processes a Slack retry when the event was not handled yet', async () => { await call(mention, { 'x-slack-retry-num': '1' }) + + expect(runRequestClassificationBot).toHaveBeenCalledTimes(1) + }) + + it('ignores an event that was already handled', async () => { + claimEvent.mockResolvedValue(false) + + await call(mention, { 'x-slack-retry-num': '1' }) + + expect(claimEvent).toHaveBeenCalledWith('Ev1') + expect(runRequestClassificationBot).not.toHaveBeenCalled() + }) + + it('handles the event when deduplication is unavailable', async () => { + claimEvent.mockRejectedValue(new Error('redis down')) + + const { req } = await call(mention) + + expect(req.log.warn).toHaveBeenCalled() + expect(runRequestClassificationBot).toHaveBeenCalledTimes(1) + }) + + it('ignores bot messages and other event types', async () => { await call({ ...mention, event: { ...mention.event, bot_id: 'B1' } }) await call({ ...mention, event: { ...mention.event, type: 'message' } }) diff --git a/backend/src/api/slack/events.ts b/backend/src/api/slack/events.ts index 18f2b9f021..8798d89bec 100644 --- a/backend/src/api/slack/events.ts +++ b/backend/src/api/slack/events.ts @@ -3,18 +3,19 @@ import { z } from 'zod' import { runRequestClassificationBot } from '@/services/slack/requestClassificationBot' import { validateOrThrow } from '@/utils/validation' +import { getErrorMessage } from '@crowd/common' import { verifySlackSignature } from './verifySignature' const URL_VERIFICATION_TYPE = 'url_verification' const EVENT_CALLBACK_TYPE = 'event_callback' const APP_MENTION_EVENT_TYPE = 'app_mention' -const RETRY_HEADER = 'x-slack-retry-num' const payloadSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal(URL_VERIFICATION_TYPE), challenge: z.string() }), z.object({ type: z.literal(EVENT_CALLBACK_TYPE), + event_id: z.string(), event: z.object({ type: z.string(), text: z.string().optional(), @@ -28,6 +29,19 @@ const payloadSchema = z.discriminatedUnion('type', [ type EventPayload = z.infer +async function isFirstDelivery( + eventId: string, + claimEvent: (eventId: string) => Promise, + req: Request, +): Promise { + try { + return await claimEvent(eventId) + } catch (err) { + req.log.warn({ error: getErrorMessage(err), eventId }, 'Could not deduplicate Slack event.') + return true + } +} + function dispatchAppMention( payload: Extract, req: Request, @@ -41,6 +55,7 @@ function dispatchAppMention( runRequestClassificationBot({ text: event.text ?? '', channelId: event.channel, + messageTs: event.ts, threadTs: event.thread_ts ?? event.ts, options: { log: req.log }, }).catch((err) => req.log.error(err, 'Slack request bot failed unexpectedly!')) @@ -48,30 +63,32 @@ function dispatchAppMention( // Mounted ahead of responseHandlerMiddleware, so errors are handled here // directly instead of via the global errorMiddleware. -export default async (req: Request, res: Response) => { - if (!verifySlackSignature(req)) { - req.log.warn('Received unverified Slack event!') - res.sendStatus(200) - return - } - - try { - const payload = validateOrThrow(payloadSchema, req.body) - - if (payload.type === URL_VERIFICATION_TYPE) { - res.json({ challenge: payload.challenge }) +export function createEventsHandler(claimEvent: (eventId: string) => Promise) { + return async (req: Request, res: Response) => { + if (!verifySlackSignature(req)) { + req.log.warn('Received unverified Slack event!') + res.sendStatus(200) return } - res.sendStatus(200) + try { + const payload = validateOrThrow(payloadSchema, req.body) - if (req.headers[RETRY_HEADER]) { - return - } + if (payload.type === URL_VERIFICATION_TYPE) { + res.json({ challenge: payload.challenge }) + return + } - dispatchAppMention(payload, req) - } catch (err) { - req.log.error(err, 'Error processing Slack event!') - res.sendStatus(200) + res.sendStatus(200) + + if (!(await isFirstDelivery(payload.event_id, claimEvent, req))) { + return + } + + dispatchAppMention(payload, req) + } catch (err) { + req.log.error(err, 'Error processing Slack event!') + res.sendStatus(200) + } } } diff --git a/backend/src/api/slack/index.ts b/backend/src/api/slack/index.ts index 9803a6b4e0..ec42a8abf8 100644 --- a/backend/src/api/slack/index.ts +++ b/backend/src/api/slack/index.ts @@ -1,11 +1,13 @@ import bodyParser from 'body-parser' import type { Application, NextFunction, Request, Response } from 'express' +import type { RedisClient } from '@crowd/redis' import { getSlackBotConfig } from '@crowd/slack' import { SLACK_CONFIG } from '../../conf/index' import { safeWrap } from '../../middlewares/errorMiddleware' import { createRateLimiter } from '../apiRateLimiter' +import { claimSlackEvent } from './eventDeduplication' // Mounted ahead of the shared rate limiter and tenant/segment middleware // to protect Slack's 3-second acknowledgement window; keeps its own limiter. @@ -38,7 +40,7 @@ export function mountInteractivityRoute(app: Application): void { ) } -export function mountEventsRoute(app: Application): void { +export function mountEventsRoute(app: Application, redis: RedisClient): void { if (!getSlackBotConfig().signingSecret) { return } @@ -63,7 +65,7 @@ export function mountEventsRoute(app: Application): void { eventsRateLimiter, bodyParser.json({ limit: '5mb', verify: captureRawBody }), handleParserError, - require('./events').default, + require('./events').createEventsHandler((eventId: string) => claimSlackEvent(redis, eventId)), ) } diff --git a/backend/src/services/slack/requestClassificationBot.run.test.ts b/backend/src/services/slack/requestClassificationBot.run.test.ts new file mode 100644 index 0000000000..47c2f9458d --- /dev/null +++ b/backend/src/services/slack/requestClassificationBot.run.test.ts @@ -0,0 +1,68 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@crowd/slack', () => ({ + getSlackPermalink: vi.fn(async () => 'https://slack.test/permalink'), + postSlackMessage: vi.fn(async () => ({ ok: true })), +})) +vi.mock('@crowd/project-onboarding/src/requestClassifierDeps', () => ({ + withRequestClassifierDeps: vi.fn(async () => ({ + resolution: { kind: 'lf_not_in_pcc', projectName: 'Acme' }, + node: 'lf_not_in_pcc_flag_human', + trace: { parsed: null, pccLookup: null, cdpLookup: null, failure: null }, + })), +})) +vi.mock('./slackBackground', () => ({ getBgQx: vi.fn(async () => ({})) })) + +import { getSlackPermalink, postSlackMessage } from '@crowd/slack' + +import { runRequestClassificationBot } from './requestClassificationBot' + +const log = { info: vi.fn(), warn: vi.fn(), error: vi.fn() } as any + +function run(overrides: Record = {}) { + return runRequestClassificationBot({ + text: '<@U0BOT> onboard Acme', + channelId: 'C1', + messageTs: '100.1', + threadTs: '90.0', + options: { log }, + ...overrides, + }) +} + +describe('runRequestClassificationBot', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(postSlackMessage).mockResolvedValue({ ok: true }) + }) + + it('links the mentioned message and replies in the thread', async () => { + await run() + + expect(getSlackPermalink).toHaveBeenCalledWith('C1', '100.1') + expect(postSlackMessage).toHaveBeenCalledWith( + expect.objectContaining({ channel: 'C1', thread_ts: '90.0' }), + ) + expect(log.warn).not.toHaveBeenCalled() + }) + + it('warns when Slack does not deliver the reply', async () => { + vi.mocked(postSlackMessage).mockResolvedValue({ ok: false, error: 'channel_not_found' }) + + await run() + + expect(log.warn).toHaveBeenCalledWith( + expect.objectContaining({ channelId: 'C1', error: 'channel_not_found' }), + 'Slack bot reply was not delivered.', + ) + }) + + it('asks for the request details when only the mention is left', async () => { + await run({ text: '<@U0BOT>' }) + + expect(getSlackPermalink).not.toHaveBeenCalled() + expect(postSlackMessage).toHaveBeenCalledWith( + expect.objectContaining({ text: expect.stringContaining('Tell me about the project') }), + ) + }) +}) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts index a9bccb894c..a860fc72d4 100644 --- a/backend/src/services/slack/requestClassificationBot.ts +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -52,17 +52,23 @@ export function buildClassificationReply( export async function runRequestClassificationBot({ text, channelId, + messageTs, threadTs, options, }: { text: string channelId: string + messageTs: string threadTs: string options: Pick }): Promise { const { log } = options - const reply = (message: SlackMessageDto | { text: string }) => - postSlackMessage({ channel: channelId, thread_ts: threadTs, ...message }) + const reply = async (message: SlackMessageDto | { text: string }) => { + const result = await postSlackMessage({ channel: channelId, thread_ts: threadTs, ...message }) + if (!result.ok) { + log.warn({ channelId, threadTs, error: result.error }, 'Slack bot reply was not delivered.') + } + } const requestText = toRequestText(text) if (!requestText) { @@ -75,7 +81,7 @@ export async function runRequestClassificationBot({ const classification = await withRequestClassifierDeps(qx, (deps) => classifyOnboardingRequest(requestText, deps), ) - const requestUrl = (await getSlackPermalink(channelId, threadTs)) ?? '' + const requestUrl = (await getSlackPermalink(channelId, messageTs)) ?? '' log.info( { channelId, threadTs, node: classification.node, kind: classification.resolution.kind }, diff --git a/services/libs/project-onboarding/src/requestClassificationAlert.test.ts b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts index d02456927c..d1368bad66 100644 --- a/services/libs/project-onboarding/src/requestClassificationAlert.test.ts +++ b/services/libs/project-onboarding/src/requestClassificationAlert.test.ts @@ -99,4 +99,28 @@ describe('dry run alert body', () => { expect(sections.map((section) => section.title)).not.toContain('Dry run') }) + + it('escapes Slack control characters in untrusted values', () => { + const sections = buildRequestClassificationAlert( + alert({ + kind: 'ambiguous', + reason: 'ping & <@U1>', + candidates: [{ ...pccProject, name: '' }], + }), + ) + const text = JSON.stringify(sections) + + expect(text).toContain('ping <!channel> & <@U1>') + expect(text).toContain('<!here>') + expect(text).not.toContain('') + expect(text).not.toContain('') + }) + + it('keeps the discussion link intact', () => { + const [intro] = buildRequestClassificationAlert( + alert({ kind: 'lf_not_in_pcc', projectName: '' }), + ) + + expect(intro.text).toContain('/g, '>') +} + function formatCandidate(candidate: IPccCandidate): string { const level = candidate.isLeaf ? 'project' : 'parent group' - return `• ${candidate.name} (${candidate.slug}), score ${candidate.score.toFixed(2)}, ${level}` + return `• ${escapeSlackText(candidate.name)} (${escapeSlackText(candidate.slug)}), score ${candidate.score.toFixed(2)}, ${level}` } function formatCandidates(candidates: IPccCandidate[]): string { @@ -36,11 +40,16 @@ function formatCandidates(candidates: IPccCandidate[]): string { function resolutionSections(resolution: OnboardingResolution): SlackMessageSection[] { switch (resolution.kind) { case 'non_github_source': - return [{ title: 'Repositories', text: resolution.nonGithubRepoUrls.join('\n') }] + return [ + { + title: 'Repositories', + text: resolution.nonGithubRepoUrls.map(escapeSlackText).join('\n'), + }, + ] case 'non_lf_new_project': return [{ title: 'Outcome', text: 'Would be onboarded as a non-LF project' }] case 'lf_not_in_pcc': - return [{ title: 'Project name', text: resolution.projectName }] + return [{ title: 'Project name', text: escapeSlackText(resolution.projectName) }] case 'lf_not_in_cdp': return [{ title: 'PCC project', text: formatCandidate(resolution.pccProject) }] case 'lf_in_cdp': @@ -48,13 +57,13 @@ function resolutionSections(resolution: OnboardingResolution): SlackMessageSecti { title: 'PCC project', text: formatCandidate(resolution.pccProject) }, { title: 'CDP segment', - text: `${resolution.segment.name}\nIntegration: ${resolution.segment.integration}`, + text: `${escapeSlackText(resolution.segment.name)}\nIntegration: ${resolution.segment.integration}`, }, { title: 'Proposed action', text: INTEGRATION_ACTION_LABELS[resolution.action] }, ] case 'ambiguous': return [ - { title: 'Reason', text: resolution.reason }, + { title: 'Reason', text: escapeSlackText(resolution.reason) }, ...(resolution.candidates.length > 0 ? [{ title: 'PCC candidates', text: formatCandidates(resolution.candidates) }] : []), @@ -77,7 +86,7 @@ export function buildRequestClassificationAlert( return [ { title: '', - text: [`Requested in: ${requestedIn}`, ...alert.repoUrls].join('\n'), + text: [`Requested in: ${requestedIn}`, ...alert.repoUrls.map(escapeSlackText)].join('\n'), }, ...resolutionSections(alert.resolution), ...(alert.dryRun From c18cc5f9c1c6692fb5e01c79d608c23bc9b0b6ba Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 15:38:16 +0200 Subject: [PATCH 3/7] test: mock heavy deps in the Slack request bot test (CM-1841) Signed-off-by: Umberto Sgueglia --- .../src/services/slack/requestClassificationBot.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/backend/src/services/slack/requestClassificationBot.test.ts b/backend/src/services/slack/requestClassificationBot.test.ts index 030bed0a37..95ecccc3cc 100644 --- a/backend/src/services/slack/requestClassificationBot.test.ts +++ b/backend/src/services/slack/requestClassificationBot.test.ts @@ -1,4 +1,10 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@crowd/slack', () => ({ getSlackPermalink: vi.fn(), postSlackMessage: vi.fn() })) +vi.mock('@crowd/project-onboarding/src/requestClassifierDeps', () => ({ + withRequestClassifierDeps: vi.fn(), +})) +vi.mock('./slackBackground', () => ({ getBgQx: vi.fn() })) import { IRequestClassification } from '@crowd/project-onboarding' From d076321c4d78eb3a24fbce5bc228916af918ca2c Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 15:42:27 +0200 Subject: [PATCH 4/7] fix: address second review round on the Slack request bot (CM-1841) Signed-off-by: Umberto Sgueglia --- backend/src/api/slack/eventDeduplication.ts | 2 +- .../slack/requestClassificationBot.test.ts | 14 ++++++++++++++ .../src/services/slack/requestClassificationBot.ts | 10 +++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/backend/src/api/slack/eventDeduplication.ts b/backend/src/api/slack/eventDeduplication.ts index 1f3d17afaa..d27c53ad2f 100644 --- a/backend/src/api/slack/eventDeduplication.ts +++ b/backend/src/api/slack/eventDeduplication.ts @@ -1,7 +1,7 @@ import type { RedisClient } from '@crowd/redis' const EVENT_KEY_PREFIX = 'slack_event' -const EVENT_TTL_SECONDS = 5 * 60 +const EVENT_TTL_SECONDS = 15 * 60 export async function claimSlackEvent(redis: RedisClient, eventId: string): Promise { const result = await redis.set(`${EVENT_KEY_PREFIX}:${eventId}`, '1', { diff --git a/backend/src/services/slack/requestClassificationBot.test.ts b/backend/src/services/slack/requestClassificationBot.test.ts index 95ecccc3cc..134da99b13 100644 --- a/backend/src/services/slack/requestClassificationBot.test.ts +++ b/backend/src/services/slack/requestClassificationBot.test.ts @@ -48,4 +48,18 @@ describe('buildClassificationReply', () => { expect(text).toContain('[DRY RUN]') expect(text).toContain('https://slack.test/thread') }) + + it('keeps every section under the Slack limit when the request lists many repositories', () => { + const repoUrls = Array.from({ length: 300 }, (_, i) => `https://github.com/acme/repo-${i}`) + const { blocks } = buildClassificationReply( + { + ...classification, + trace: { ...classification.trace, parsed: { githubRepoUrls: repoUrls } as any }, + }, + 'https://slack.test/thread', + ) + + const longest = Math.max(...blocks.map((block: any) => block.text?.text.length ?? 0)) + expect(longest).toBeLessThanOrEqual(3000) + }) }) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts index a860fc72d4..834d0e8e4b 100644 --- a/backend/src/services/slack/requestClassificationBot.ts +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -17,9 +17,15 @@ const MENTION_PATTERN = /<@[A-Z0-9]+>/g const LINK_WITH_LABEL_PATTERN = /<(https?:\/\/[^|>\s]+)\|[^>]*>/g const LINK_PATTERN = /<(https?:\/\/[^>\s]+)>/g +const MAX_SECTION_TEXT = 2900 + const HELP_TEXT = 'Tell me about the project you want to onboard: its name, whether it is a Linux Foundation project and the GitHub repositories.' +function truncateSectionText(text: string): string { + return text.length > MAX_SECTION_TEXT ? `${text.slice(0, MAX_SECTION_TEXT - 1)}…` : text +} + export function toRequestText(slackText: string): string { return slackText .replace(MENTION_PATTERN, '') @@ -43,7 +49,9 @@ export function buildClassificationReply( return Message() .blocks( Section({ text: `*${buildRequestClassificationAlertTitle(alert)}*` }), - ...sections.map(({ title, text }) => Section({ text: title ? `*${title}*\n${text}` : text })), + ...sections.map(({ title, text }) => + Section({ text: truncateSectionText(title ? `*${title}*\n${text}` : text) }), + ), Section({ text: `_Step reached: ${classification.node}_` }), ) .buildToObject() From 36356aa5e6f680f009b76b13f2a00d658eefa27e Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 16:12:13 +0200 Subject: [PATCH 5/7] fix: avoid the snowflake-sdk startup crash in the backend (CM-1841) Signed-off-by: Umberto Sgueglia --- .../services/slack/requestClassificationBot.run.test.ts | 8 ++++++++ backend/src/services/slack/requestClassificationBot.ts | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/backend/src/services/slack/requestClassificationBot.run.test.ts b/backend/src/services/slack/requestClassificationBot.run.test.ts index 47c2f9458d..7b0b584fc4 100644 --- a/backend/src/services/slack/requestClassificationBot.run.test.ts +++ b/backend/src/services/slack/requestClassificationBot.run.test.ts @@ -46,6 +46,14 @@ describe('runRequestClassificationBot', () => { expect(log.warn).not.toHaveBeenCalled() }) + it('disables the Snowflake platform detection before loading the classifier deps', async () => { + delete process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION + + await run() + + expect(process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION).toBe('true') + }) + it('warns when Slack does not deliver the reply', async () => { vi.mocked(postSlackMessage).mockResolvedValue({ ok: false, error: 'channel_not_found' }) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts index 834d0e8e4b..3a8da17ee9 100644 --- a/backend/src/services/slack/requestClassificationBot.ts +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -7,7 +7,6 @@ import { buildRequestClassificationAlertTitle, classifyOnboardingRequest, } from '@crowd/project-onboarding' -import { withRequestClassifierDeps } from '@crowd/project-onboarding/src/requestClassifierDeps' import { getSlackPermalink, postSlackMessage } from '@crowd/slack' import { IServiceOptions } from '../IServiceOptions' @@ -26,6 +25,12 @@ function truncateSectionText(text: string): string { return text.length > MAX_SECTION_TEXT ? `${text.slice(0, MAX_SECTION_TEXT - 1)}…` : text } +async function loadRequestClassifierDeps() { + // TODO(CM-1841): drop once snowflake-sdk stops building a region-less STS client on import + process.env.SNOWFLAKE_DISABLE_PLATFORM_DETECTION ??= 'true' + return import('@crowd/project-onboarding/src/requestClassifierDeps') +} + export function toRequestText(slackText: string): string { return slackText .replace(MENTION_PATTERN, '') @@ -85,6 +90,7 @@ export async function runRequestClassificationBot({ } try { + const { withRequestClassifierDeps } = await loadRequestClassifierDeps() const qx = await getBgQx() const classification = await withRequestClassifierDeps(qx, (deps) => classifyOnboardingRequest(requestText, deps), From 0fe8494e26690213811f18c946089a6ded998590 Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 17:36:17 +0200 Subject: [PATCH 6/7] fix: compute IS_LEAF with a join in the PCC candidates query (CM-1841) Snowflake rejects a NOT IN subquery in the select list. Fixes the query added in #4877. Signed-off-by: Umberto Sgueglia --- .../project-onboarding/src/pccLookup.test.ts | 9 +++--- .../libs/project-onboarding/src/pccLookup.ts | 31 ++++++++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/services/libs/project-onboarding/src/pccLookup.test.ts b/services/libs/project-onboarding/src/pccLookup.test.ts index 351c120e96..336ec2e38d 100644 --- a/services/libs/project-onboarding/src/pccLookup.test.ts +++ b/services/libs/project-onboarding/src/pccLookup.test.ts @@ -29,16 +29,17 @@ describe('createPccCandidatesLookup', () => { }) it('excludes internal projects', () => { - expect(PCC_CANDIDATES_QUERY).toContain('NOT IS_INTERNAL_PROJECT') + expect(PCC_CANDIDATES_QUERY).toContain('NOT p.IS_INTERNAL_PROJECT') }) it('skips rows without a name or slug so null scores cannot outrank real matches', () => { - expect(PCC_CANDIDATES_QUERY).toContain('NAME IS NOT NULL') - expect(PCC_CANDIDATES_QUERY).toContain('SLUG IS NOT NULL') + expect(PCC_CANDIDATES_QUERY).toContain('p.NAME IS NOT NULL') + expect(PCC_CANDIDATES_QUERY).toContain('p.SLUG IS NOT NULL') }) it('flags leaf projects and orders ties deterministically', () => { expect(PCC_CANDIDATES_QUERY).toContain('AS IS_LEAF') - expect(PCC_CANDIDATES_QUERY).toContain('ORDER BY SCORE DESC, IS_LEAF DESC, PROJECT_ID') + expect(PCC_CANDIDATES_QUERY).not.toContain('NOT IN') + expect(PCC_CANDIDATES_QUERY).toContain('ORDER BY SCORE DESC, IS_LEAF DESC, p.PROJECT_ID') }) }) diff --git a/services/libs/project-onboarding/src/pccLookup.ts b/services/libs/project-onboarding/src/pccLookup.ts index 3bf5efb805..9bc4713ce3 100644 --- a/services/libs/project-onboarding/src/pccLookup.ts +++ b/services/libs/project-onboarding/src/pccLookup.ts @@ -5,23 +5,24 @@ const SNOWFLAKE_MAX_SCORE = 100 export const PCC_CANDIDATES_QUERY = ` SELECT - PROJECT_ID, - NAME, - SLUG, + p.PROJECT_ID, + p.NAME, + p.SLUG, GREATEST( - JAROWINKLER_SIMILARITY(LOWER(NAME), ?), - JAROWINKLER_SIMILARITY(LOWER(SLUG), ?) + JAROWINKLER_SIMILARITY(LOWER(p.NAME), ?), + JAROWINKLER_SIMILARITY(LOWER(p.SLUG), ?) ) AS SCORE, - PROJECT_ID NOT IN ( - SELECT DISTINCT PARENT_ID - FROM ANALYTICS.SILVER_DIM.PROJECTS - WHERE PARENT_ID IS NOT NULL - ) AS IS_LEAF - FROM ANALYTICS.SILVER_DIM.PROJECTS - WHERE NOT IS_INTERNAL_PROJECT - AND NAME IS NOT NULL - AND SLUG IS NOT NULL - ORDER BY SCORE DESC, IS_LEAF DESC, PROJECT_ID + parents.PARENT_ID IS NULL AS IS_LEAF + FROM ANALYTICS.SILVER_DIM.PROJECTS p + LEFT JOIN ( + SELECT DISTINCT PARENT_ID + FROM ANALYTICS.SILVER_DIM.PROJECTS + WHERE PARENT_ID IS NOT NULL + ) parents ON parents.PARENT_ID = p.PROJECT_ID + WHERE NOT p.IS_INTERNAL_PROJECT + AND p.NAME IS NOT NULL + AND p.SLUG IS NOT NULL + ORDER BY SCORE DESC, IS_LEAF DESC, p.PROJECT_ID LIMIT ${MAX_PCC_CANDIDATES} ` From 8ff6cb692122778350a22f9cebb6ac0cc98b799e Mon Sep 17 00:00:00 2001 From: Umberto Sgueglia Date: Fri, 2 Oct 2026 17:45:22 +0200 Subject: [PATCH 7/7] fix: hide failure details and decode entities in the Slack bot reply (CM-1841) Signed-off-by: Umberto Sgueglia --- .../slack/requestClassificationBot.test.ts | 40 ++++++++++++++++++- .../slack/requestClassificationBot.ts | 27 ++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/backend/src/services/slack/requestClassificationBot.test.ts b/backend/src/services/slack/requestClassificationBot.test.ts index 134da99b13..8d39959120 100644 --- a/backend/src/services/slack/requestClassificationBot.test.ts +++ b/backend/src/services/slack/requestClassificationBot.test.ts @@ -8,7 +8,11 @@ vi.mock('./slackBackground', () => ({ getBgQx: vi.fn() })) import { IRequestClassification } from '@crowd/project-onboarding' -import { buildClassificationReply, toRequestText } from './requestClassificationBot' +import { + buildClassificationReply, + hideFailureDetails, + toRequestText, +} from './requestClassificationBot' describe('toRequestText', () => { it('removes the bot mention and keeps the rest of the message', () => { @@ -23,6 +27,12 @@ describe('toRequestText', () => { ).toBe('https://github.com/acme/one and https://github.com/acme/two') }) + it('decodes the HTML entities Slack applies to the message text', () => { + expect(toRequestText('<@U0BOT> onboard R&D <internal>')).toBe( + 'onboard R&D ', + ) + }) + it('returns an empty string when only the mention is left', () => { expect(toRequestText('<@U0BOT> ')).toBe('') }) @@ -62,4 +72,32 @@ describe('buildClassificationReply', () => { const longest = Math.max(...blocks.map((block: any) => block.text?.text.length ?? 0)) expect(longest).toBeLessThanOrEqual(3000) }) + + it('adds a plain text fallback for clients that do not render blocks', () => { + const message = buildClassificationReply(classification, 'https://slack.test/thread') + + expect(message.text).toContain('lf_not_in_pcc_flag_human') + }) + + it('does not expose dependency errors or model output in the reply', () => { + const failed: IRequestClassification = { + resolution: { + kind: 'ambiguous', + reason: 'Classification failed: SQL compilation error at line 11', + candidates: [], + }, + node: 'ambiguous_human_review', + trace: { + ...classification.trace, + failure: { stage: 'resolve', reason: 'SQL compilation error at line 11' }, + }, + } + + const text = JSON.stringify( + buildClassificationReply(hideFailureDetails(failed), 'https://slack.test/thread'), + ) + + expect(text).not.toContain('SQL compilation error') + expect(text).toContain('could not be classified automatically') + }) }) diff --git a/backend/src/services/slack/requestClassificationBot.ts b/backend/src/services/slack/requestClassificationBot.ts index 3a8da17ee9..42839182bd 100644 --- a/backend/src/services/slack/requestClassificationBot.ts +++ b/backend/src/services/slack/requestClassificationBot.ts @@ -18,6 +18,8 @@ const LINK_PATTERN = /<(https?:\/\/[^>\s]+)>/g const MAX_SECTION_TEXT = 2900 +const FAILURE_REASON = 'The request could not be classified automatically.' + const HELP_TEXT = 'Tell me about the project you want to onboard: its name, whether it is a Linux Foundation project and the GitHub repositories.' @@ -36,9 +38,23 @@ export function toRequestText(slackText: string): string { .replace(MENTION_PATTERN, '') .replace(LINK_WITH_LABEL_PATTERN, '$1') .replace(LINK_PATTERN, '$1') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/&/g, '&') .trim() } +export function hideFailureDetails(classification: IRequestClassification): IRequestClassification { + if (!classification.trace.failure) { + return classification + } + + return { + ...classification, + resolution: { kind: 'ambiguous', reason: FAILURE_REASON, candidates: [] }, + } +} + export function buildClassificationReply( classification: IRequestClassification, requestUrl: string, @@ -52,6 +68,7 @@ export function buildClassificationReply( const sections = buildRequestClassificationAlert(alert) return Message() + .text(`${buildRequestClassificationAlertTitle(alert)}. Step reached: ${classification.node}`) .blocks( Section({ text: `*${buildRequestClassificationAlertTitle(alert)}*` }), ...sections.map(({ title, text }) => @@ -98,10 +115,16 @@ export async function runRequestClassificationBot({ const requestUrl = (await getSlackPermalink(channelId, messageTs)) ?? '' log.info( - { channelId, threadTs, node: classification.node, kind: classification.resolution.kind }, + { + channelId, + threadTs, + node: classification.node, + kind: classification.resolution.kind, + failure: classification.trace.failure, + }, 'Onboarding request classified from Slack.', ) - await reply(buildClassificationReply(classification, requestUrl)) + await reply(buildClassificationReply(hideFailureDetails(classification), requestUrl)) } catch (err) { log.error({ error: getErrorMessage(err), channelId, threadTs }, 'Slack request failed.') await reply({ text: ':no_entry: I could not process this request, please try again later.' })