Skip to content

Commit 7971ade

Browse files
fix(slack): finalize custom bot migration rollout
1 parent bcfac33 commit 7971ade

6 files changed

Lines changed: 155 additions & 88 deletions

File tree

apps/sim/blocks/blocks/slack.test.ts

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@ import {
1313
const EXTENDED_OPERATION_IDS = ['set_status', 'set_title', 'set_suggested_prompts']
1414
const EXTENDED_TOOL_IDS = ['slack_set_status', 'slack_set_title', 'slack_set_suggested_prompts']
1515

16-
function operationIds(extendedScopesEnabled: boolean): string[] {
17-
const operation = getSlackV2ActionSubBlocks(extendedScopesEnabled).find(
18-
(subBlock) => subBlock.id === 'operation'
19-
)
16+
function operationIds(): string[] {
17+
const operation = getSlackV2ActionSubBlocks().find((subBlock) => subBlock.id === 'operation')
2018
return operation?.options?.map((option) => option.id) ?? []
2119
}
2220

23-
describe('Slack block extended-scope capability', () => {
21+
describe('Slack block release', () => {
2422
it('releases slack_v2 and keeps the legacy block executable but hidden', () => {
2523
expect(SlackBlock.hideFromToolbar).toBe(true)
2624
expect(SlackBlock.sunset).toEqual({ status: 'legacy', replacedBy: 'slack_v2' })
@@ -29,18 +27,10 @@ describe('Slack block extended-scope capability', () => {
2927
expect(SlackV2Block.sunset).toBeUndefined()
3028
})
3129

32-
it('removes extended-scope operations and tools when the capability is disabled', () => {
33-
expect(operationIds(false)).not.toEqual(expect.arrayContaining(EXTENDED_OPERATION_IDS))
34-
expect(getSlackV2ToolAccess(false)).not.toEqual(expect.arrayContaining(EXTENDED_TOOL_IDS))
35-
expect(Object.keys(getSlackV2OperationSentences(false))).not.toEqual(
36-
expect.arrayContaining(EXTENDED_OPERATION_IDS)
37-
)
38-
})
39-
40-
it('restores extended-scope operations and tools when the capability is enabled', () => {
41-
expect(operationIds(true)).toEqual(expect.arrayContaining(EXTENDED_OPERATION_IDS))
42-
expect(getSlackV2ToolAccess(true)).toEqual(expect.arrayContaining(EXTENDED_TOOL_IDS))
43-
expect(Object.keys(getSlackV2OperationSentences(true))).toEqual(
30+
it('keeps custom-bot operations available independently of native OAuth scopes', () => {
31+
expect(operationIds()).toEqual(expect.arrayContaining(EXTENDED_OPERATION_IDS))
32+
expect(getSlackV2ToolAccess()).toEqual(expect.arrayContaining(EXTENDED_TOOL_IDS))
33+
expect(Object.keys(getSlackV2OperationSentences())).toEqual(
4434
expect.arrayContaining(EXTENDED_OPERATION_IDS)
4535
)
4636
})

apps/sim/blocks/blocks/slack.ts

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { BookOpen, ClipboardList, File, Table, Users } from '@sim/emcn/icons'
22
import { GoogleTranslateIcon, GreptileIcon, SlackIcon } from '@/components/icons'
3-
import { isSlackExtendedScopesEnabled } from '@/lib/core/config/env-flags'
43
import { getScopesForService } from '@/lib/oauth/utils'
54
import type { BlockConfig, BlockMeta, SubBlockConfig } from '@/blocks/types'
65
import { AuthMode, IntegrationType } from '@/blocks/types'
@@ -16,18 +15,6 @@ import { getTrigger } from '@/triggers'
1615
/** The operations that offer a channel/DM switch, and so honour it. */
1716
const DESTINATION_SWITCH_OPERATIONS = ['send', 'read', 'schedule_message'] as const
1817

19-
const SLACK_EXTENDED_SCOPE_OPERATION_IDS = new Set([
20-
'set_status',
21-
'set_title',
22-
'set_suggested_prompts',
23-
])
24-
25-
const SLACK_EXTENDED_SCOPE_TOOL_IDS = new Set([
26-
'slack_set_status',
27-
'slack_set_title',
28-
'slack_set_suggested_prompts',
29-
])
30-
3118
const CHANNEL_FIELD = ['channel', 'manualChannel'] as const
3219

3320
/**
@@ -2934,16 +2921,8 @@ const SLACK_WEBHOOK_TRIGGER_SUBBLOCK_IDS = new Set(
29342921
* Adapts a v1 subblock for slack_v2's merged credential picker: fields gated on
29352922
* the removed `authMethod` dropdown now depend on the single `credential` field.
29362923
*/
2937-
function adaptSubBlockForV2(sb: SubBlockConfig, extendedScopesEnabled: boolean): SubBlockConfig {
2924+
function adaptSubBlockForV2(sb: SubBlockConfig): SubBlockConfig {
29382925
const { dependsOn, condition, ...rest } = sb
2939-
if (sb.id === 'operation' && !extendedScopesEnabled) {
2940-
const options = typeof sb.options === 'function' ? sb.options() : sb.options
2941-
if (!options) throw new Error('Slack operation subblock must define options')
2942-
return {
2943-
...sb,
2944-
options: options.filter((option) => !SLACK_EXTENDED_SCOPE_OPERATION_IDS.has(option.id)),
2945-
}
2946-
}
29472926
if (sb.id === 'credential') {
29482927
return {
29492928
...rest,
@@ -2966,31 +2945,24 @@ function adaptSubBlockForV2(sb: SubBlockConfig, extendedScopesEnabled: boolean):
29662945
return sb
29672946
}
29682947

2969-
export function getSlackV2ActionSubBlocks(extendedScopesEnabled: boolean): SubBlockConfig[] {
2948+
export function getSlackV2ActionSubBlocks(): SubBlockConfig[] {
29702949
return SlackBlock.subBlocks.flatMap((sb) => {
29712950
if (SLACK_WEBHOOK_TRIGGER_SUBBLOCK_IDS.has(sb.id)) return []
29722951
if (sb.id === 'authMethod') return []
2973-
return [adaptSubBlockForV2(sb, extendedScopesEnabled)]
2952+
return [adaptSubBlockForV2(sb)]
29742953
})
29752954
}
29762955

2977-
export function getSlackV2ToolAccess(extendedScopesEnabled: boolean): string[] {
2978-
if (extendedScopesEnabled) return [...SlackBlock.tools.access]
2979-
return SlackBlock.tools.access.filter((toolId) => !SLACK_EXTENDED_SCOPE_TOOL_IDS.has(toolId))
2956+
export function getSlackV2ToolAccess(): string[] {
2957+
return [...SlackBlock.tools.access]
29802958
}
29812959

2982-
export function getSlackV2OperationSentences(extendedScopesEnabled: boolean) {
2960+
export function getSlackV2OperationSentences() {
29832961
const operationSentences = SlackBlock.canvasPresentation?.sentences?.byOperation
29842962
if (!operationSentences) {
29852963
throw new Error('Slack action sentences must be defined before building slack_v2')
29862964
}
2987-
if (extendedScopesEnabled) return { ...operationSentences }
2988-
2989-
const scopedSentences = { ...operationSentences }
2990-
for (const operationId of SLACK_EXTENDED_SCOPE_OPERATION_IDS) {
2991-
delete scopedSentences[operationId]
2992-
}
2993-
return scopedSentences
2965+
return { ...operationSentences }
29942966
}
29952967

29962968
const {
@@ -3017,7 +2989,7 @@ export const SlackV2Block: BlockConfig<SlackResponse> = {
30172989
defaultTitle: 'Slack',
30182990
sentences: {
30192991
...SlackBlock.canvasPresentation?.sentences,
3020-
byOperation: getSlackV2OperationSentences(isSlackExtendedScopesEnabled),
2992+
byOperation: getSlackV2OperationSentences(),
30212993
},
30222994
/*
30232995
* Unlike v1, this trigger picks one event and scopes it, so the card names
@@ -3038,13 +3010,10 @@ export const SlackV2Block: BlockConfig<SlackResponse> = {
30383010
],
30393011
},
30403012
},
3041-
subBlocks: [
3042-
...getSlackV2ActionSubBlocks(isSlackExtendedScopesEnabled),
3043-
...getTrigger('slack_oauth').subBlocks,
3044-
],
3013+
subBlocks: [...getSlackV2ActionSubBlocks(), ...getTrigger('slack_oauth').subBlocks],
30453014
tools: {
30463015
...SlackBlock.tools,
3047-
access: getSlackV2ToolAccess(isSlackExtendedScopesEnabled),
3016+
access: getSlackV2ToolAccess(),
30483017
},
30493018
inputs: {
30503019
...slackV2Inputs,
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { NextRequest, NextResponse } from 'next/server'
5+
import { describe, expect, it, vi } from 'vitest'
6+
7+
const { mockDispatchResolvedWebhookTarget } = vi.hoisted(() => ({
8+
mockDispatchResolvedWebhookTarget: vi.fn(),
9+
}))
10+
11+
vi.mock('@/lib/webhooks/processor', () => ({
12+
dispatchResolvedWebhookTarget: mockDispatchResolvedWebhookTarget,
13+
}))
14+
15+
vi.mock('@/lib/webhooks/providers/slack', () => ({
16+
resolveSlackEventKey: vi.fn(),
17+
}))
18+
19+
import { dispatchResolvedWebhookTarget } from '@/lib/webhooks/processor'
20+
import { dispatchSlackWebhooks } from '@/lib/webhooks/slack-dispatch'
21+
22+
describe('dispatchSlackWebhooks', () => {
23+
it('dispatches at most ten targets concurrently and preserves result order', async () => {
24+
let active = 0
25+
let peak = 0
26+
const releases: Array<() => void> = []
27+
28+
vi.mocked(dispatchResolvedWebhookTarget).mockImplementation(async (foundWebhook) => {
29+
active += 1
30+
peak = Math.max(peak, active)
31+
await new Promise<void>((resolve) => releases.push(resolve))
32+
active -= 1
33+
34+
const index = Number(foundWebhook.id.replace('webhook-', ''))
35+
return {
36+
outcome: 'queued',
37+
response: new NextResponse(null, { status: 200 + index }),
38+
reason: 'queued',
39+
}
40+
})
41+
42+
const webhooks = Array.from({ length: 12 }, (_, index) => ({
43+
webhook: { id: `webhook-${index}`, providerConfig: {} },
44+
workflow: { id: `workflow-${index}` },
45+
}))
46+
const dispatchPromise = dispatchSlackWebhooks(webhooks as never, {
47+
body: { event: { type: 'message' } },
48+
request: new NextRequest('http://localhost/api/webhooks/slack'),
49+
requestId: 'request-1',
50+
receivedAt: Date.now(),
51+
})
52+
53+
await vi.waitFor(() => expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(10))
54+
expect(active).toBe(10)
55+
56+
for (let index = 0; index < 10; index += 1) {
57+
releases.shift()?.()
58+
}
59+
60+
await vi.waitFor(() => expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(12))
61+
while (releases.length > 0) {
62+
releases.shift()?.()
63+
}
64+
65+
const results = await dispatchPromise
66+
expect(peak).toBe(10)
67+
expect(results.map(({ response }) => response.status)).toEqual(
68+
Array.from({ length: 12 }, (_, index) => 200 + index)
69+
)
70+
})
71+
})
Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
3+
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
34
import {
45
dispatchResolvedWebhookTarget,
56
type findWebhooksByRoutingKey,
@@ -8,6 +9,7 @@ import {
89
import { resolveSlackEventKey } from '@/lib/webhooks/providers/slack'
910

1011
const logger = createLogger('SlackWebhookDispatch')
12+
const SLACK_WEBHOOK_DISPATCH_CONCURRENCY = 10
1113

1214
interface DispatchSlackWebhooksOptions {
1315
body: unknown
@@ -31,31 +33,41 @@ export async function dispatchSlackWebhooks(
3133
const slackRequestTimestamp = request.headers.get('x-slack-request-timestamp')
3234
const parsedTimestampMs = slackRequestTimestamp ? Number(slackRequestTimestamp) * 1000 : undefined
3335
const triggerTimestampMs = Number.isFinite(parsedTimestampMs) ? parsedTimestampMs : undefined
34-
const results: WebhookDispatchResult[] = []
36+
return mapWithConcurrency(
37+
webhooks,
38+
SLACK_WEBHOOK_DISPATCH_CONCURRENCY,
39+
async ({ webhook: foundWebhook, workflow: foundWorkflow }) => {
40+
const result = await dispatchResolvedWebhookTarget(
41+
foundWebhook,
42+
foundWorkflow,
43+
body,
44+
request,
45+
{
46+
requestId,
47+
receivedAt,
48+
triggerTimestampMs,
49+
}
50+
)
3551

36-
for (const { webhook: foundWebhook, workflow: foundWorkflow } of webhooks) {
37-
const result = await dispatchResolvedWebhookTarget(foundWebhook, foundWorkflow, body, request, {
38-
requestId,
39-
receivedAt,
40-
triggerTimestampMs,
41-
})
52+
if (result.outcome === 'ignored' && result.reason === 'filtered') {
53+
const rawEvent = payload.event as Record<string, unknown> | undefined
54+
const providerConfig = (foundWebhook.providerConfig as Record<string, unknown>) || {}
55+
logger.info(
56+
`[${requestId}] Event skipped by trigger filter for webhook ${foundWebhook.id}`,
57+
{
58+
eventKey: resolveSlackEventKey(payload),
59+
configuredEvent: providerConfig.eventType,
60+
channelType: rawEvent?.channel_type,
61+
subtype: rawEvent?.subtype,
62+
isThreadReply:
63+
typeof rawEvent?.thread_ts === 'string' && rawEvent.thread_ts !== rawEvent.ts,
64+
threadsSetting: providerConfig.threads,
65+
botId: rawEvent?.bot_id,
66+
}
67+
)
68+
}
4269

43-
if (result.outcome === 'ignored' && result.reason === 'filtered') {
44-
const rawEvent = payload.event as Record<string, unknown> | undefined
45-
const providerConfig = (foundWebhook.providerConfig as Record<string, unknown>) || {}
46-
logger.info(`[${requestId}] Event skipped by trigger filter for webhook ${foundWebhook.id}`, {
47-
eventKey: resolveSlackEventKey(payload),
48-
configuredEvent: providerConfig.eventType,
49-
channelType: rawEvent?.channel_type,
50-
subtype: rawEvent?.subtype,
51-
isThreadReply:
52-
typeof rawEvent?.thread_ts === 'string' && rawEvent.thread_ts !== rawEvent.ts,
53-
threadsSetting: providerConfig.threads,
54-
botId: rawEvent?.bot_id,
55-
})
70+
return result
5671
}
57-
results.push(result)
58-
}
59-
60-
return results
72+
)
6173
}

packages/db/scripts/migrate-slack-custom-bots.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,13 @@ describe('planLegacySlackTriggerLink', () => {
261261
).toEqual({ updateTriggerBlock: true, webhookIdsToUpdate: ['webhook-1'] })
262262
})
263263

264+
it('links an undeployed trigger even when there is no webhook to mark', () => {
265+
expect(planLegacySlackTriggerLink(triggerSource, existingCredential, [])).toEqual({
266+
updateTriggerBlock: true,
267+
webhookIdsToUpdate: [],
268+
})
269+
})
270+
264271
it('is idempotent after the block and webhook are linked', () => {
265272
expect(
266273
planLegacySlackTriggerLink(

0 commit comments

Comments
 (0)