Skip to content

Commit d48ee87

Browse files
fix(deployments): retire inactive-version side effects by row with bounded outbox continuation (#7405)
* fix(deployments): retire inactive-version side effects by row with bounded outbox continuation * fix(deployments): fence webhook teardown on version inactivity and abort in undeploy cleanup
1 parent abd8f74 commit d48ee87

15 files changed

Lines changed: 874 additions & 176 deletions

apps/sim/lib/admin/member-operation.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ vi.mock('@/lib/workspaces/organization-workspaces', () => ({
5151
ownedAttachableWorkspacesWhere: vi.fn(() => undefined),
5252
}))
5353
vi.mock('@/lib/core/outbox/service', () => ({
54+
continueOutboxHandler: (reason: string) => ({
55+
outcome: 'deferred',
56+
reason,
57+
consumeAttempt: false,
58+
}),
5459
deferOutboxHandler: (reason: string, _minimum?: number, consumeAttempt = true) => ({
5560
outcome: 'deferred',
5661
reason,

apps/sim/lib/admin/member-operation.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats'
1717
import { ENTITLED_SUBSCRIPTION_STATUSES } from '@/lib/billing/subscriptions/utils'
1818
import {
19-
deferOutboxHandler,
19+
continueOutboxHandler,
2020
enqueueOutboxEvent,
2121
type OutboxHandler,
2222
outboxEventHasSourceOperationId,
@@ -688,7 +688,7 @@ export const processAdminMemberOperation: OutboxHandler<unknown> = async (rawPay
688688
}
689689

690690
if (nextWorkspaceIndex < payload.request.workspaceIds.length) {
691-
return deferOutboxHandler('Continuing bounded member workspace moves', undefined, false)
691+
return continueOutboxHandler('Continuing bounded member workspace moves')
692692
}
693693
}
694694

apps/sim/lib/billing/enterprise-provisioning.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ vi.mock('@/lib/billing/webhooks/enterprise-reconciliation-lease', () => ({
6161
),
6262
}))
6363
vi.mock('@/lib/core/outbox/service', () => ({
64+
continueOutboxHandler: (reason: string) => ({
65+
outcome: 'deferred',
66+
reason,
67+
consumeAttempt: false,
68+
}),
6469
deferOutboxHandler: (reason: string, minimumBackoffMs?: number, consumeAttempt = true) => ({
6570
outcome: 'deferred',
6671
reason,

apps/sim/lib/billing/enterprise-provisioning.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import { withEnterpriseReconciliationLease } from '@/lib/billing/webhooks/enterp
7979
import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-handlers'
8080
import { env } from '@/lib/core/config/env'
8181
import {
82+
continueOutboxHandler,
8283
deferOutboxHandler,
8384
enqueueOutboxEvent,
8485
type OutboxEventContext,
@@ -3108,7 +3109,7 @@ export const reconcileEnterpriseMembers: OutboxHandler<unknown> = async (rawPayl
31083109

31093110
if (!nextCursor) return
31103111
await context.checkpointPayload({ afterUserId: nextCursor })
3111-
return deferOutboxHandler('Continuing bounded Enterprise member reconciliation', undefined, false)
3112+
return continueOutboxHandler('Continuing bounded Enterprise member reconciliation')
31123113
}
31133114

31143115
export const enterpriseIssuanceOutboxHandlers = {

apps/sim/lib/core/outbox/service.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ vi.mock('@sim/utils/id', () => ({
2525
}))
2626

2727
import {
28+
continueOutboxHandler,
2829
deferOutboxHandler,
2930
enqueueOrReschedulePendingOutboxEvent,
3031
enqueueOutboxEvent,
@@ -387,6 +388,20 @@ describe('processOutboxEvents — handler success and retry', () => {
387388
expect(deferredUpdate).toMatchObject({ attempts: 4, lastError: null, lockedAt: null })
388389
})
389390

391+
it('re-runs a continued handler without consuming its attempt budget', async () => {
392+
const handler = vi.fn(async () => continueOutboxHandler('continuing bounded cleanup'))
393+
queueTableRows(outboxEvent, [makePendingRow({ attempts: 4, maxAttempts: 5 })])
394+
holdLease()
395+
396+
const result = await processOutboxEvents({ 'test.event': handler })
397+
398+
expect(result.retried).toBe(1)
399+
const continuedUpdate = updateSets().find(
400+
(set) => set.status === 'pending' && 'attempts' in set
401+
)
402+
expect(continuedUpdate).toMatchObject({ attempts: 4, lastError: null, lockedAt: null })
403+
})
404+
390405
it('dead-letters on failure when attempts reaches maxAttempts', async () => {
391406
const handler = vi.fn(async () => {
392407
throw new Error('permanent failure')

apps/sim/lib/core/outbox/service.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,9 @@ export interface DeferredOutboxHandlerResult {
8181
minimumBackoffMs?: number
8282
/**
8383
* Defaults to true for an external acknowledgement with a finite retry
84-
* budget. Set false only for an internal dependency whose own outbox row
85-
* independently reaches completed or dead-letter.
84+
* budget. False is reserved for waits on an internal dependency whose own
85+
* outbox row independently reaches completed or dead-letter, and for
86+
* bounded continuation after durable progress (`continueOutboxHandler`).
8687
*/
8788
consumeAttempt?: boolean
8889
}
@@ -100,6 +101,19 @@ export function deferOutboxHandler(
100101
}
101102
}
102103

104+
/**
105+
* Yields after durable progress so the worker re-runs the event without
106+
* spending an attempt. For bounded batches whose remaining work shrinks on
107+
* every run; a run that made no progress must throw or `deferOutboxHandler`
108+
* instead, or the event never reaches a terminal state.
109+
*/
110+
export function continueOutboxHandler(
111+
reason: string,
112+
minimumBackoffMs?: number
113+
): DeferredOutboxHandlerResult {
114+
return deferOutboxHandler(reason, minimumBackoffMs, false)
115+
}
116+
103117
export type OutboxHandler<T = unknown> = (
104118
payload: T,
105119
context: OutboxEventContext

apps/sim/lib/webhooks/deploy.test.ts

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { account, credential } from '@sim/db/schema'
5-
import { queueTableRows, resetDbChainMock, resetEnvFlagsMock, setEnvFlags } from '@sim/testing'
6-
import { eq } from 'drizzle-orm'
4+
import { account, credential, webhook, workflowDeploymentVersion } from '@sim/db/schema'
5+
import {
6+
dbChainMockFns,
7+
queueTableRows,
8+
resetDbChainMock,
9+
resetEnvFlagsMock,
10+
setEnvFlags,
11+
} from '@sim/testing'
12+
import { eq, ne } from 'drizzle-orm'
713
import { afterAll, beforeEach, describe, expect, it, type Mock, vi } from 'vitest'
814
import type { SubBlockConfig } from '@/blocks/types'
915
import type { BlockState } from '@/stores/workflows/workflow/types'
@@ -29,6 +35,14 @@ vi.mock('@/lib/webhooks/utils.server', () => ({
2935
vi.mock('@/lib/webhooks/pending-verification', () => ({
3036
PendingWebhookVerificationTracker: vi.fn(),
3137
}))
38+
const { mockIsDeploymentVersionActive, mockIsDeploymentVersionProtected } = vi.hoisted(() => ({
39+
mockIsDeploymentVersionActive: vi.fn(),
40+
mockIsDeploymentVersionProtected: vi.fn(),
41+
}))
42+
vi.mock('@/lib/workflows/persistence/deployment-operations', () => ({
43+
isDeploymentVersionActive: mockIsDeploymentVersionActive,
44+
isDeploymentVersionProtectedByCurrentOperation: mockIsDeploymentVersionProtected,
45+
}))
3246

3347
const {
3448
mockGetSlackBotCredential,
@@ -52,9 +66,11 @@ vi.mock('@/lib/webhooks/providers/slack', () => ({
5266

5367
import {
5468
buildProviderConfig,
69+
cleanupInactiveDeploymentWebhooks,
5570
resolveTriggerCredentialId,
5671
resolveWebhookConfigForBlock,
5772
} from '@/lib/webhooks/deploy'
73+
import { cleanupExternalWebhook } from '@/lib/webhooks/provider-subscriptions'
5874
import { getBlock } from '@/blocks'
5975
import { getTrigger } from '@/triggers'
6076

@@ -639,3 +655,107 @@ describe('resolveWebhookConfigForBlock — TikTok routing', () => {
639655
expect(result?.error.message).toContain('Reconnect')
640656
})
641657
})
658+
659+
describe('cleanupInactiveDeploymentWebhooks', () => {
660+
const workflow = { id: 'workflow-1', userId: 'user-1', workspaceId: 'workspace-1' }
661+
const input = {
662+
workflowId: 'workflow-1',
663+
workflow,
664+
requestId: 'request-1',
665+
protectedDeploymentVersionId: null,
666+
limit: 5,
667+
}
668+
669+
function staleWebhookRow(id: string) {
670+
return {
671+
id,
672+
workflowId: 'workflow-1',
673+
deploymentVersionId: 'version-1',
674+
provider: 'github',
675+
providerConfig: {},
676+
archivedAt: null,
677+
createdAt: new Date('2026-07-14T08:00:00.000Z'),
678+
}
679+
}
680+
681+
beforeEach(() => {
682+
mockIsDeploymentVersionActive.mockResolvedValue(false)
683+
mockIsDeploymentVersionProtected.mockResolvedValue(false)
684+
})
685+
686+
it('retires one bounded batch of stale rows and reports the remainder', async () => {
687+
queueTableRows(webhook, [
688+
staleWebhookRow('wh-1'),
689+
staleWebhookRow('wh-2'),
690+
staleWebhookRow('wh-3'),
691+
])
692+
queueTableRows(workflowDeploymentVersion, [{ id: 'version-1' }])
693+
queueTableRows(workflowDeploymentVersion, [{ id: 'version-1' }])
694+
695+
await expect(cleanupInactiveDeploymentWebhooks({ ...input, limit: 2 })).resolves.toEqual({
696+
hasMore: true,
697+
})
698+
699+
expect(vi.mocked(cleanupExternalWebhook)).toHaveBeenCalledTimes(2)
700+
expect(vi.mocked(cleanupExternalWebhook)).toHaveBeenCalledWith(
701+
expect.objectContaining({ id: 'wh-1' }),
702+
workflow,
703+
'request-1',
704+
{ throwOnError: true }
705+
)
706+
expect(dbChainMockFns.delete).toHaveBeenCalledTimes(2)
707+
})
708+
709+
it('reports completion once the batch drains every stale row', async () => {
710+
queueTableRows(webhook, [staleWebhookRow('wh-1')])
711+
queueTableRows(workflowDeploymentVersion, [{ id: 'version-1' }])
712+
713+
await expect(cleanupInactiveDeploymentWebhooks(input)).resolves.toEqual({ hasMore: false })
714+
715+
expect(vi.mocked(cleanupExternalWebhook)).toHaveBeenCalledTimes(1)
716+
expect(dbChainMockFns.delete).toHaveBeenCalledTimes(1)
717+
})
718+
719+
it('excludes the version the current operation is preparing from the batch', async () => {
720+
queueTableRows(webhook, [])
721+
722+
await expect(
723+
cleanupInactiveDeploymentWebhooks({ ...input, protectedDeploymentVersionId: 'version-3' })
724+
).resolves.toEqual({ hasMore: false })
725+
726+
expect(ne).toHaveBeenCalledWith(webhook.deploymentVersionId, 'version-3')
727+
})
728+
729+
it('stops before any provider call once the fence reports a change', async () => {
730+
queueTableRows(webhook, [staleWebhookRow('wh-1')])
731+
732+
await expect(
733+
cleanupInactiveDeploymentWebhooks({ ...input, shouldContinue: async () => false })
734+
).resolves.toEqual({ hasMore: true })
735+
736+
expect(vi.mocked(cleanupExternalWebhook)).not.toHaveBeenCalled()
737+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
738+
})
739+
740+
it('leaves a row alone when its version was re-activated after the batch was selected', async () => {
741+
queueTableRows(webhook, [staleWebhookRow('wh-1')])
742+
mockIsDeploymentVersionActive.mockResolvedValue(true)
743+
744+
await expect(cleanupInactiveDeploymentWebhooks(input)).resolves.toEqual({ hasMore: true })
745+
746+
expect(mockIsDeploymentVersionActive).toHaveBeenCalledWith('workflow-1', 'version-1')
747+
expect(vi.mocked(cleanupExternalWebhook)).not.toHaveBeenCalled()
748+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
749+
})
750+
751+
it('leaves a row alone when its version became the current candidate mid-batch', async () => {
752+
queueTableRows(webhook, [staleWebhookRow('wh-1')])
753+
mockIsDeploymentVersionProtected.mockResolvedValue(true)
754+
755+
await expect(cleanupInactiveDeploymentWebhooks(input)).resolves.toEqual({ hasMore: true })
756+
757+
expect(mockIsDeploymentVersionProtected).toHaveBeenCalledWith('workflow-1', 'version-1')
758+
expect(vi.mocked(cleanupExternalWebhook)).not.toHaveBeenCalled()
759+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
760+
})
761+
})

0 commit comments

Comments
 (0)