Skip to content

Commit 3c3b5d1

Browse files
committed
fix(triggers): v2 reports only triggers that run, restore executor PATCH, switch all or nothing
The v2 list and block target require the active version of a deployed workflow for both tables, since webhook processing refuses a versionless or undeployed row; the legacy row-id routes keep the editor's view. PATCH /api/webhooks/[id] admits a workflow run's executor delegation again, and setTriggerEnabled admits the executor service. A multi-row switch rolls back entirely when any row was retired or switched concurrently, so a conflict commits nothing. Adds the two new CLI fences to the docs content baseline.
1 parent caf5c95 commit 3c3b5d1

6 files changed

Lines changed: 245 additions & 122 deletions

File tree

‎apps/docs/scripts/fixtures/content-baseline.json‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12986,6 +12986,20 @@
1298612986
"bytes": 28,
1298712987
"line": 8128
1298812988
},
12989+
{
12990+
"lang": "bash",
12991+
"meta": null,
12992+
"sha256": "7579bd6ac197afa0afa5f62d9b412a7e265725bd507c1c63b438a60fd72ce1d0",
12993+
"bytes": 40,
12994+
"line": 8235
12995+
},
12996+
{
12997+
"lang": "bash",
12998+
"meta": null,
12999+
"sha256": "63aac11606dc0eeb6cc452c20c405806840960425372cf2cf7c8b4b90a7de878",
13000+
"bytes": 62,
13001+
"line": 8255
13002+
},
1298913003
{
1299013004
"lang": "bash",
1299113005
"meta": null,
@@ -17159,6 +17173,20 @@
1715917173
"bytes": 54,
1716017174
"line": 404
1716117175
},
17176+
{
17177+
"lang": "bash",
17178+
"meta": null,
17179+
"sha256": "7579bd6ac197afa0afa5f62d9b412a7e265725bd507c1c63b438a60fd72ce1d0",
17180+
"bytes": 40,
17181+
"line": 436
17182+
},
17183+
{
17184+
"lang": "bash",
17185+
"meta": null,
17186+
"sha256": "63aac11606dc0eeb6cc452c20c405806840960425372cf2cf7c8b4b90a7de878",
17187+
"bytes": 62,
17188+
"line": 454
17189+
},
1716217190
{
1716317191
"lang": "bash",
1716417192
"meta": null,

‎apps/sim/app/api/webhooks/[id]/route.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import {
2323
internalErrorResponse,
2424
internalOrchestrationErrorPolicy,
2525
internalRateLimits,
26-
internalSessionAuth,
2726
} from '@/lib/api/server/routes'
2827
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
2928
import { InsufficientWorkspacePermissionsError } from '@/lib/core/application'
@@ -32,6 +31,7 @@ import { generateRequestId } from '@/lib/core/utils/request'
3231
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
3332
import { captureServerEvent } from '@/lib/posthog/server'
3433
import { cleanupExternalWebhook } from '@/lib/webhooks/provider-subscriptions'
34+
import { internalWorkflowSessionOrExecutorAuth } from '@/lib/workflows/api'
3535
import { workflowOperations } from '@/lib/workflows/application/operations'
3636
import { setWorkflowTriggerEnabled } from '@/lib/workflows/application/triggers'
3737

@@ -103,10 +103,12 @@ export const GET = withRouteHandler(
103103
* `triggers.webhook` gate on re-enabling. The id must name the live
104104
* deployment's row: a candidate, retired, or older-version registration answers
105105
* 404 instead of being revived.
106+
* Admits a session or a workflow run's executor delegation; the executor is not
107+
* judged against the triggering member's permission group.
106108
*/
107109
export const PATCH = defineInternalJsonRoute({
108110
contract: updateWebhookContract,
109-
auth: internalSessionAuth,
111+
auth: internalWorkflowSessionOrExecutorAuth,
110112
operation: workflowOperations.setTriggerEnabled,
111113
rateLimit: internalRateLimits.none({
112114
reason: 'Webhook enablement retains its existing session admission policy.',

‎apps/sim/lib/workflows/application/operations.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ const WORKFLOW_READ_PRINCIPAL_POLICY = {
2222
delegatedServices: ['copilot', 'executor'],
2323
} as const
2424

25+
/** Every caller, including a workflow run's executor delegation. */
26+
const EXECUTOR_REACHABLE_WORKFLOW_PRINCIPAL_POLICY = {
27+
principalKinds: [
28+
'session',
29+
'personal_api_key',
30+
'oauth_access_token',
31+
'workspace_api_key',
32+
'delegated',
33+
],
34+
delegatedServices: ['copilot', 'executor'],
35+
} as const
36+
2537
const HUMAN_WORKFLOW_PRINCIPAL_POLICY = {
2638
principalKinds: ['session', 'personal_api_key', 'oauth_access_token', 'delegated'],
2739
delegatedServices: ['copilot'],
@@ -356,6 +368,8 @@ export const workflowOperations = {
356368
* Pausing or resuming one trigger of the live deployment. Not a deploy: the
357369
* deployed graph and its provider registrations are untouched, so it is a
358370
* write rather than an admin act, and a workspace key may perform it.
371+
* A workflow run's executor delegation may too, as it could before this
372+
* moved onto the use case.
359373
*
360374
* permission-group-exempt: `triggers.webhook` is asserted inside the use case and only for enabling a webhook-table row, because a group that withholds inbound webhooks must still let a member switch a live one off
361375
*/
@@ -366,7 +380,7 @@ export const workflowOperations = {
366380
workspaceApiKey: 'allow',
367381
capability: 'none',
368382
contentDelivery: 'none',
369-
...ALL_WORKFLOW_PRINCIPAL_POLICY,
383+
...EXECUTOR_REACHABLE_WORKFLOW_PRINCIPAL_POLICY,
370384
}),
371385
activateVersion: defineWorkspaceOperation({
372386
id: 'workflows.versions.activate',

‎apps/sim/lib/workflows/application/triggers.integration.ts‎

Lines changed: 72 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
2020
import { createLogger } from '@sim/logger'
2121
import {
22+
createExecutorPrincipal,
2223
createSessionPrincipal,
2324
createWorkspaceApiKeyPrincipal,
2425
} from '@sim/testing/factories/principal.factory'
@@ -29,6 +30,7 @@ import { and, eq, inArray } from 'drizzle-orm'
2930
import { afterAll, beforeAll, expect, it, vi } from 'vitest'
3031
import { markWebhookFailed } from '@/lib/webhooks/polling/utils'
3132
import { v2WorkflowErrorPolicies } from '@/lib/workflows/api'
33+
import { WORKFLOW_DELEGATION_AUDIENCE } from '@/lib/workflows/application/authorization'
3234
import {
3335
listWorkflowTriggers,
3436
setWorkflowTriggerEnabled,
@@ -72,6 +74,8 @@ const eventWebhookId = generateId()
7274
const twinWebhookIds = [generateId(), generateId()]
7375
const versionlessWebhookId = generateId()
7476
const scalarConfigWebhookId = generateId()
77+
const presaveWebhookId = generateId()
78+
const executorWebhookId = generateId()
7579
const flakyWebhookId = generateId()
7680
const hookPath = `trigger-health-${generateId()}`
7781

@@ -400,6 +404,20 @@ beforeAll(async () => {
400404
path: `${hookPath}-scalar`,
401405
providerConfig: 'legacy-scalar-config',
402406
},
407+
{
408+
...dormantWebhook,
409+
id: presaveWebhookId,
410+
workflowId: undeployedWorkflowId,
411+
deploymentVersionId: null,
412+
blockId: 'presave',
413+
path: `${hookPath}-presave`,
414+
},
415+
{
416+
...dormantWebhook,
417+
id: executorWebhookId,
418+
blockId: 'exec',
419+
path: `${hookPath}-exec`,
420+
},
403421
{
404422
...dormantWebhook,
405423
id: flakyWebhookId,
@@ -440,6 +458,7 @@ check('lists only the live deployment’s rows, disabled ones included', async (
440458
['badcron', 'schedule', 'disabled'],
441459
['done', 'schedule', 'completed'],
442460
['evt', 'event', 'active'],
461+
['exec', 'webhook', 'disabled'],
443462
['flaky', 'polling', 'active'],
444463
['hook', 'webhook', 'disabled'],
445464
['poll', 'polling', 'disabled'],
@@ -527,37 +546,54 @@ check('legacy row ids never revive a row the live deployment does not serve', as
527546
expect(viaRow.record.row.id).toBe(liveScheduleId)
528547
})
529548

530-
check('the write itself refuses a row that is no longer live', async () => {
549+
check('a switch is all or nothing: one row no longer live rolls back the rest', async () => {
531550
const updated = await setLiveTriggersEnabled({
532551
workflowId,
533552
enabled: true,
534-
schedules: [{ id: oldScheduleId, nextRunAt: new Date(Date.now() + 60_000) }],
535-
webhookIds: [oldWebhookId, candidateWebhookId],
536-
scheduleLiveness: 'editor',
553+
schedules: [
554+
{ id: badCronScheduleId, nextRunAt: new Date(Date.now() + 60_000) },
555+
{ id: oldScheduleId, nextRunAt: new Date(Date.now() + 60_000) },
556+
],
557+
webhookIds: [twinWebhookIds[1], oldWebhookId, candidateWebhookId],
558+
liveness: 'editor',
537559
now: new Date(),
538560
})
539-
expect(updated).toEqual([])
561+
expect(updated).toBeNull()
540562
expect((await scheduleRow(oldScheduleId)).status).toBe('disabled')
563+
expect((await scheduleRow(badCronScheduleId)).status).toBe('disabled')
564+
expect((await webhookRow(twinWebhookIds[1])).isActive).toBe(false)
541565
expect((await webhookRow(oldWebhookId)).isActive).toBe(false)
542566
expect((await webhookRow(candidateWebhookId)).isActive).toBe(false)
543567
})
544568

545-
check('a versionless schedule is hidden from v2 but kept for the editor row id', async () => {
546-
const { triggers } = await listWorkflowTriggers.execute({
547-
principal: reader,
548-
input: { workflowId: versionlessWorkflowId },
549-
})
550-
expect(triggers.map((trigger) => [trigger.blockId, trigger.kind])).toEqual([['hook', 'webhook']])
551-
await expect(
552-
setWorkflowTriggerEnabled.execute({
553-
principal: writer,
554-
input: {
555-
target: { kind: 'block', workflowId: versionlessWorkflowId, blockId: 'sched' },
556-
enabled: true,
557-
},
569+
check('rows that never run are hidden from v2 but kept for the editor row id', async () => {
570+
for (const id of [versionlessWorkflowId, undeployedWorkflowId]) {
571+
const { triggers } = await listWorkflowTriggers.execute({
572+
principal: reader,
573+
input: { workflowId: id },
558574
})
559-
).rejects.toMatchObject({ code: 'not_found' })
575+
expect(triggers).toEqual([])
576+
}
577+
for (const [id, blockId] of [
578+
[versionlessWorkflowId, 'sched'],
579+
[versionlessWorkflowId, 'hook'],
580+
[undeployedWorkflowId, 'presave'],
581+
]) {
582+
await expect(
583+
setWorkflowTriggerEnabled.execute({
584+
principal: writer,
585+
input: { target: { kind: 'block', workflowId: id, blockId }, enabled: true },
586+
})
587+
).rejects.toMatchObject({ code: 'not_found' })
588+
}
560589
expect((await scheduleRow(versionlessScheduleId)).status).toBe('disabled')
590+
expect((await webhookRow(presaveWebhookId)).isActive).toBe(false)
591+
592+
const presave = await setWorkflowTriggerEnabled.execute({
593+
principal: writer,
594+
input: { target: { kind: 'row', table: 'webhook', id: presaveWebhookId }, enabled: true },
595+
})
596+
expect(presave.changes).toHaveLength(1)
561597

562598
const schedule = await setWorkflowTriggerEnabled.execute({
563599
principal: writer,
@@ -642,6 +678,23 @@ check('read access lists but cannot write; other tenants see not found', async (
642678
expect((await scheduleRow(liveScheduleId)).status).toBe('active')
643679
})
644680

681+
check('a workflow run’s executor delegation may enable, ungoverned by the group', async () => {
682+
const executor = createExecutorPrincipal({
683+
audience: WORKFLOW_DELEGATION_AUDIENCE,
684+
workspaceId,
685+
subjectUserId: writerId,
686+
delegationContext: { kind: 'workflow_execution', workflowId },
687+
})
688+
await setWebhookTriggersWithheld(true)
689+
try {
690+
const enabled = await setBlock('exec', true, executor)
691+
expect(enabled.changes).toHaveLength(1)
692+
expect((await webhookRow(executorWebhookId)).isActive).toBe(true)
693+
} finally {
694+
await setWebhookTriggersWithheld(false)
695+
}
696+
})
697+
645698
check('the webhook capability gates enabling only, and only governed callers', async () => {
646699
await setWebhookTriggersWithheld(true)
647700
try {

‎apps/sim/lib/workflows/application/triggers.ts‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import {
2121
isInRequestedState,
2222
type LiveWorkflowTriggerRecord,
2323
listLiveWorkflowTriggers,
24-
type ScheduleLiveness,
2524
setLiveTriggersEnabled,
25+
type TriggerLiveness,
2626
toWorkflowTriggerHealth,
2727
type WorkflowTriggerHealth,
2828
type WorkflowTriggerStatus,
@@ -161,10 +161,10 @@ export const setWorkflowTriggerEnabled = defineAuthorizedWorkflowUseCase({
161161
operation: workflowOperations.setTriggerEnabled,
162162
resolveContext: resolveTriggerTargetContext,
163163
async execute({ principal, input, context }): Promise<SetWorkflowTriggerEnabledResult> {
164-
// A legacy row id keeps the versionless schedules the editor shows; a block target sees only what fires.
165-
const scheduleLiveness: ScheduleLiveness = context.row ? 'editor' : 'dispatched'
166-
const readLive = () => findLiveBlockTriggers({ ...context, scheduleLiveness })
167-
let [, current] = await Promise.all([requireMutableWorkflow(context.workflowId), readLive()])
164+
// A legacy row id keeps the rows the editor shows; a block target sees only what runs.
165+
const liveness: TriggerLiveness = context.row ? 'editor' : 'running'
166+
const readLive = () => findLiveBlockTriggers({ ...context, liveness })
167+
const [, current] = await Promise.all([requireMutableWorkflow(context.workflowId), readLive()])
168168
if (current.length === 0) throw new OrchestrationError('not_found', context.notFoundMessage)
169169
const pending = current.filter((record) => !isInRequestedState(record, input.enabled))
170170
if (pending.length === 0) {
@@ -190,15 +190,17 @@ export const setWorkflowTriggerEnabled = defineAuthorizedWorkflowUseCase({
190190
enabled: input.enabled,
191191
schedules,
192192
webhookIds,
193-
scheduleLiveness,
193+
liveness,
194194
now: new Date(),
195195
})
196-
if (updated.length < pending.length) {
197-
current = await readLive()
198-
if (current.length === 0) throw new OrchestrationError('not_found', context.notFoundMessage)
199-
if (current.some((record) => !isInRequestedState(record, input.enabled))) {
196+
if (!updated) {
197+
// Nothing was written. Succeed only if a concurrent request already reached the requested state.
198+
const latest = await readLive()
199+
if (latest.length === 0) throw new OrchestrationError('not_found', context.notFoundMessage)
200+
if (latest.some((record) => !isInRequestedState(record, input.enabled))) {
200201
throw new OrchestrationError('conflict', 'Trigger changed during the update; retry')
201202
}
203+
return { trigger: toWorkflowTriggerHealth(latest[0]), record: latest[0], changes: [] }
202204
}
203205

204206
const previousStatusById = new Map(

0 commit comments

Comments
 (0)