Skip to content

Commit 67f54df

Browse files
chore(slack): reconcile staging merge
- nullable webhook.path coalesced at correlation/payload/tiktok boundaries - slack dispatch delegates to staging's dispatchResolvedWebhookTarget (shared preprocess/deployment/filter/enqueue lifecycle), keeping the skip-reason diagnostics; route tests reworked around that seam - api-validation route baseline 924 -> 926
1 parent 26217bf commit 67f54df

6 files changed

Lines changed: 67 additions & 116 deletions

File tree

apps/sim/app/api/webhooks/slack/custom/[credentialId]/route.test.ts

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,17 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
66
const {
77
mockParseWebhookBody,
88
mockFindWebhooksByRoutingKey,
9-
mockCheckWebhookPreprocessing,
10-
mockQueueWebhookExecution,
11-
mockBlockExistsInDeployment,
9+
mockDispatchResolvedWebhookTarget,
1210
mockGetSlackBotCredential,
1311
mockHandleChallenge,
1412
mockVerifySignature,
15-
mockShouldSkip,
1613
} = vi.hoisted(() => ({
1714
mockParseWebhookBody: vi.fn(),
1815
mockFindWebhooksByRoutingKey: vi.fn(),
19-
mockCheckWebhookPreprocessing: vi.fn(),
20-
mockQueueWebhookExecution: vi.fn(),
21-
mockBlockExistsInDeployment: vi.fn(),
16+
mockDispatchResolvedWebhookTarget: vi.fn(),
2217
mockGetSlackBotCredential: vi.fn(),
2318
mockHandleChallenge: vi.fn(),
2419
mockVerifySignature: vi.fn(),
25-
mockShouldSkip: vi.fn(),
2620
}))
2721

2822
vi.mock('@/lib/core/admission/gate', () => ({
@@ -37,21 +31,15 @@ vi.mock('@/app/api/auth/oauth/utils', () => ({
3731
vi.mock('@/lib/webhooks/processor', () => ({
3832
parseWebhookBody: mockParseWebhookBody,
3933
findWebhooksByRoutingKey: mockFindWebhooksByRoutingKey,
40-
checkWebhookPreprocessing: mockCheckWebhookPreprocessing,
41-
queueWebhookExecution: mockQueueWebhookExecution,
34+
dispatchResolvedWebhookTarget: mockDispatchResolvedWebhookTarget,
4235
}))
4336

4437
vi.mock('@/lib/webhooks/providers/slack', () => ({
4538
handleSlackChallenge: mockHandleChallenge,
4639
verifySlackRequestSignature: mockVerifySignature,
47-
shouldSkipSlackTriggerEvent: mockShouldSkip,
4840
resolveSlackEventKey: () => null,
4941
}))
5042

51-
vi.mock('@/lib/workflows/persistence/utils', () => ({
52-
blockExistsInDeployment: mockBlockExistsInDeployment,
53-
}))
54-
5543
import { POST } from '@/app/api/webhooks/slack/custom/[credentialId]/route'
5644

5745
const CREDENTIAL_ID = 'cred-123'
@@ -80,7 +68,6 @@ describe('Slack custom-bot webhook route', () => {
8068
vi.clearAllMocks()
8169
mockHandleChallenge.mockReturnValue(null)
8270
mockVerifySignature.mockReturnValue(null)
83-
mockShouldSkip.mockReturnValue(false)
8471
mockParseWebhookBody.mockResolvedValue({
8572
body: messageBody,
8673
rawBody: JSON.stringify(messageBody),
@@ -91,12 +78,11 @@ describe('Slack custom-bot webhook route', () => {
9178
teamId: 'T1',
9279
})
9380
mockFindWebhooksByRoutingKey.mockResolvedValue([webhook('wh1')])
94-
mockCheckWebhookPreprocessing.mockResolvedValue({
95-
actorUserId: 'u1',
96-
executionId: 'e1',
97-
correlation: {},
81+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
82+
outcome: 'queued',
83+
response: new Response(null, { status: 200 }),
84+
reason: 'queued',
9885
})
99-
mockBlockExistsInDeployment.mockResolvedValue(true)
10086
})
10187

10288
it('echoes the url_verification challenge without loading the credential', async () => {
@@ -110,7 +96,7 @@ describe('Slack custom-bot webhook route', () => {
11096
mockGetSlackBotCredential.mockResolvedValue(null)
11197
const res = await POST(makeRequest(), context)
11298
expect(res.status).toBe(404)
113-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
99+
expect(mockDispatchResolvedWebhookTarget).not.toHaveBeenCalled()
114100
})
115101

116102
it('verifies with the credential signing secret and rejects a bad signature', async () => {
@@ -123,22 +109,28 @@ describe('Slack custom-bot webhook route', () => {
123109
expect.any(String)
124110
)
125111
expect(res.status).toBe(401)
126-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
112+
expect(mockDispatchResolvedWebhookTarget).not.toHaveBeenCalled()
127113
})
128114

129-
it('fans out by credential id (provider slack) and queues when not skipped', async () => {
130-
await POST(makeRequest(), context)
115+
it('fans out by credential id (provider slack) and dispatches each webhook', async () => {
116+
const res = await POST(makeRequest(), context)
131117
expect(mockFindWebhooksByRoutingKey).toHaveBeenCalledWith(
132118
CREDENTIAL_ID,
133119
expect.any(String),
134120
'slack'
135121
)
136-
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(1)
122+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
123+
expect(res.status).toBe(200)
137124
})
138125

139-
it('does not queue when the shared filter skips the event', async () => {
140-
mockShouldSkip.mockReturnValue(true)
141-
await POST(makeRequest(), context)
142-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
126+
it('still returns 200 when the dispatcher filters the event', async () => {
127+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
128+
outcome: 'ignored',
129+
response: new Response(null, { status: 200 }),
130+
reason: 'filtered',
131+
})
132+
const res = await POST(makeRequest(), context)
133+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
134+
expect(res.status).toBe(200)
143135
})
144136
})

apps/sim/app/api/webhooks/slack/route.test.ts

Lines changed: 26 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,12 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const {
7-
mockParseWebhookBody,
8-
mockFindWebhooksByRoutingKey,
9-
mockCheckWebhookPreprocessing,
10-
mockQueueWebhookExecution,
11-
mockBlockExistsInDeployment,
12-
mockShouldSkip,
13-
} = vi.hoisted(() => ({
14-
mockParseWebhookBody: vi.fn(),
15-
mockFindWebhooksByRoutingKey: vi.fn(),
16-
mockCheckWebhookPreprocessing: vi.fn(),
17-
mockQueueWebhookExecution: vi.fn(),
18-
mockBlockExistsInDeployment: vi.fn(),
19-
mockShouldSkip: vi.fn(),
20-
}))
6+
const { mockParseWebhookBody, mockFindWebhooksByRoutingKey, mockDispatchResolvedWebhookTarget } =
7+
vi.hoisted(() => ({
8+
mockParseWebhookBody: vi.fn(),
9+
mockFindWebhooksByRoutingKey: vi.fn(),
10+
mockDispatchResolvedWebhookTarget: vi.fn(),
11+
}))
2112

2213
vi.mock('@/lib/core/admission/gate', () => ({
2314
tryAdmit: () => ({ release: vi.fn() }),
@@ -31,21 +22,15 @@ vi.mock('@/lib/core/config/env', () => ({
3122
vi.mock('@/lib/webhooks/processor', () => ({
3223
parseWebhookBody: mockParseWebhookBody,
3324
findWebhooksByRoutingKey: mockFindWebhooksByRoutingKey,
34-
checkWebhookPreprocessing: mockCheckWebhookPreprocessing,
35-
queueWebhookExecution: mockQueueWebhookExecution,
25+
dispatchResolvedWebhookTarget: mockDispatchResolvedWebhookTarget,
3626
}))
3727

3828
vi.mock('@/lib/webhooks/providers/slack', () => ({
3929
handleSlackChallenge: () => null,
4030
verifySlackRequestSignature: () => null,
41-
shouldSkipSlackTriggerEvent: mockShouldSkip,
4231
resolveSlackEventKey: () => null,
4332
}))
4433

45-
vi.mock('@/lib/workflows/persistence/utils', () => ({
46-
blockExistsInDeployment: mockBlockExistsInDeployment,
47-
}))
48-
4934
import { POST } from '@/app/api/webhooks/slack/route'
5035

5136
function makeRequest() {
@@ -61,12 +46,6 @@ function webhook(id: string) {
6146

6247
async function run(body: Record<string, unknown>) {
6348
mockParseWebhookBody.mockResolvedValue({ body, rawBody: JSON.stringify(body) })
64-
mockCheckWebhookPreprocessing.mockResolvedValue({
65-
actorUserId: 'u1',
66-
executionId: 'e1',
67-
correlation: {},
68-
})
69-
mockBlockExistsInDeployment.mockResolvedValue(true)
7049
await POST(makeRequest())
7150
}
7251

@@ -80,22 +59,29 @@ describe('Slack app webhook route', () => {
8059
beforeEach(() => {
8160
vi.clearAllMocks()
8261
mockFindWebhooksByRoutingKey.mockResolvedValue([webhook('wh1')])
62+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
63+
outcome: 'queued',
64+
response: new Response(null, { status: 200 }),
65+
reason: 'queued',
66+
})
8367
})
8468

85-
it('queues execution when the shared filter does not skip', async () => {
86-
mockShouldSkip.mockReturnValue(false)
69+
it('dispatches each webhook resolved for the event team', async () => {
8770
await run(messageBody)
88-
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(1)
71+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
8972
})
9073

91-
it('does not queue when the shared filter skips the event', async () => {
92-
mockShouldSkip.mockReturnValue(true)
74+
it('continues cleanly when the dispatcher filters the event', async () => {
75+
mockDispatchResolvedWebhookTarget.mockResolvedValue({
76+
outcome: 'ignored',
77+
response: new Response(null, { status: 200 }),
78+
reason: 'filtered',
79+
})
9380
await run(messageBody)
94-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
81+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
9582
})
9683

9784
it('routes via Slack Connect authorizations and dedups overlapping webhooks', async () => {
98-
mockShouldSkip.mockReturnValue(false)
9985
// Two candidate teams (outer + authorization) that resolve to overlapping webhooks.
10086
mockFindWebhooksByRoutingKey.mockImplementation(async (teamId: string) =>
10187
teamId === 'T1' ? [webhook('wh1')] : [webhook('wh1'), webhook('wh2')]
@@ -105,19 +91,17 @@ describe('Slack app webhook route', () => {
10591
authorizations: [{ team_id: 'T2' }],
10692
})
10793
expect(mockFindWebhooksByRoutingKey).toHaveBeenCalledTimes(2)
108-
// wh1 (in both) is queued once, wh2 once — dedup by webhook id.
109-
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(2)
94+
// wh1 (in both) is dispatched once, wh2 once — dedup by webhook id.
95+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(2)
11096
})
11197

11298
it('returns 200 with no team_id', async () => {
113-
mockShouldSkip.mockReturnValue(false)
11499
await run({ event: { type: 'message' } })
115100
expect(mockFindWebhooksByRoutingKey).not.toHaveBeenCalled()
116-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
101+
expect(mockDispatchResolvedWebhookTarget).not.toHaveBeenCalled()
117102
})
118103

119104
it('routes an interaction payload by payload.team.id', async () => {
120-
mockShouldSkip.mockReturnValue(false)
121105
await run({
122106
type: 'block_actions',
123107
api_app_id: 'A1',
@@ -126,18 +110,17 @@ describe('Slack app webhook route', () => {
126110
actions: [{ action_id: 'approve_btn' }],
127111
})
128112
expect(mockFindWebhooksByRoutingKey).toHaveBeenCalledWith('T1', expect.anything())
129-
expect(mockQueueWebhookExecution).toHaveBeenCalledTimes(1)
113+
expect(mockDispatchResolvedWebhookTarget).toHaveBeenCalledTimes(1)
130114
})
131115

132116
it('fails closed on an interaction missing payload.team.id (never routes on user.team_id)', async () => {
133-
mockShouldSkip.mockReturnValue(false)
134117
await run({
135118
type: 'block_actions',
136119
api_app_id: 'A1',
137120
user: { id: 'U1', team_id: 'T_OTHER' },
138121
actions: [{ action_id: 'approve_btn' }],
139122
})
140123
expect(mockFindWebhooksByRoutingKey).not.toHaveBeenCalled()
141-
expect(mockQueueWebhookExecution).not.toHaveBeenCalled()
124+
expect(mockDispatchResolvedWebhookTarget).not.toHaveBeenCalled()
142125
})
143126
})

apps/sim/background/tiktok-webhook-ingress.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export async function executeTikTokWebhookIngress(
6060
request,
6161
{
6262
requestId: payload.requestId,
63-
path: webhook.path,
63+
path: webhook.path ?? undefined,
6464
receivedAt: payload.receivedAt,
6565
triggerTimestampMs: payload.envelope.create_time * 1000,
6666
}

apps/sim/lib/webhooks/processor.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,8 @@ async function queueWebhookExecutionWithResult(
676676
source: 'webhook' as const,
677677
workflowId: foundWorkflow.id,
678678
webhookId: foundWebhook.id,
679-
path: options.path || foundWebhook.path,
679+
// Routing-key webhooks (e.g. Slack) have no path.
680+
path: options.path || foundWebhook.path || undefined,
680681
provider: foundWebhook.provider,
681682
triggerType: 'webhook',
682683
} satisfies AsyncExecutionCorrelation)
@@ -692,7 +693,7 @@ async function queueWebhookExecutionWithResult(
692693
provider: foundWebhook.provider,
693694
body,
694695
headers,
695-
path: options.path || foundWebhook.path,
696+
path: options.path || foundWebhook.path || '',
696697
blockId: foundWebhook.blockId ?? undefined,
697698
workspaceId,
698699
...(credentialId ? { credentialId } : {}),
Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
33
import {
4-
checkWebhookPreprocessing,
4+
dispatchResolvedWebhookTarget,
55
type findWebhooksByRoutingKey,
6-
queueWebhookExecution,
76
} from '@/lib/webhooks/processor'
8-
import { resolveSlackEventKey, shouldSkipSlackTriggerEvent } from '@/lib/webhooks/providers/slack'
9-
import { blockExistsInDeployment } from '@/lib/workflows/persistence/utils'
7+
import { resolveSlackEventKey } from '@/lib/webhooks/providers/slack'
108

119
const logger = createLogger('SlackWebhookDispatch')
1210

@@ -19,10 +17,10 @@ interface DispatchSlackWebhooksOptions {
1917

2018
/**
2119
* Shared fan-out tail for the Slack ingest routes (native team-id route and the
22-
* custom-bot credential route): run each candidate webhook through the trigger
23-
* filter, verify its block is still deployed, preprocess, and enqueue. Keeping
24-
* this in one place stops the skip/preprocess/queue sequence drifting between
25-
* the two ingest paths.
20+
* custom-bot credential route): run each candidate webhook through the common
21+
* post-auth lifecycle (preprocess, deployment check, trigger filter, enqueue)
22+
* via {@link dispatchResolvedWebhookTarget}, logging skip diagnostics for
23+
* filtered events.
2624
*/
2725
export async function dispatchSlackWebhooks(
2826
webhooks: Awaited<ReturnType<typeof findWebhooksByRoutingKey>>,
@@ -34,12 +32,15 @@ export async function dispatchSlackWebhooks(
3432
const triggerTimestampMs = Number.isFinite(parsedTimestampMs) ? parsedTimestampMs : undefined
3533

3634
for (const { webhook: foundWebhook, workflow: foundWorkflow } of webhooks) {
37-
const providerConfig = (foundWebhook.providerConfig as Record<string, unknown>) || {}
35+
const result = await dispatchResolvedWebhookTarget(foundWebhook, foundWorkflow, body, request, {
36+
requestId,
37+
receivedAt,
38+
triggerTimestampMs,
39+
})
3840

39-
// Shared trigger filter (event, source, threads, emoji, name, channels,
40-
// interaction, self-drop, bot).
41-
if (shouldSkipSlackTriggerEvent(payload, providerConfig)) {
41+
if (result.outcome === 'ignored' && result.reason === 'filtered') {
4242
const rawEvent = payload.event as Record<string, unknown> | undefined
43+
const providerConfig = (foundWebhook.providerConfig as Record<string, unknown>) || {}
4344
logger.info(`[${requestId}] Event skipped by trigger filter for webhook ${foundWebhook.id}`, {
4445
eventKey: resolveSlackEventKey(payload),
4546
configuredEvent: providerConfig.eventType,
@@ -50,32 +51,6 @@ export async function dispatchSlackWebhooks(
5051
threadsSetting: providerConfig.threads,
5152
botId: rawEvent?.bot_id,
5253
})
53-
continue
5454
}
55-
56-
if (foundWebhook.blockId) {
57-
const blockExists = await blockExistsInDeployment(foundWorkflow.id, foundWebhook.blockId)
58-
if (!blockExists) {
59-
logger.info(
60-
`[${requestId}] Trigger block ${foundWebhook.blockId} not in deployment for ${foundWorkflow.id}`
61-
)
62-
continue
63-
}
64-
}
65-
66-
const preprocessResult = await checkWebhookPreprocessing(foundWorkflow, foundWebhook, requestId)
67-
if (preprocessResult.error) {
68-
logger.warn(`[${requestId}] Preprocessing failed for webhook ${foundWebhook.id}`)
69-
continue
70-
}
71-
72-
await queueWebhookExecution(foundWebhook, foundWorkflow, body, request, {
73-
requestId,
74-
actorUserId: preprocessResult.actorUserId,
75-
executionId: preprocessResult.executionId,
76-
correlation: preprocessResult.correlation,
77-
receivedAt,
78-
triggerTimestampMs,
79-
})
8055
}
8156
}

scripts/check-api-validation-contracts.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries')
99
const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors')
1010

1111
const BASELINE = {
12-
totalRoutes: 924,
13-
zodRoutes: 924,
12+
totalRoutes: 926,
13+
zodRoutes: 926,
1414
nonZodRoutes: 0,
1515
} as const
1616

0 commit comments

Comments
 (0)