diff --git a/apps/sim/app/api/chat/manage/[id]/route.test.ts b/apps/sim/app/api/chat/manage/[id]/route.test.ts index 32f3a6a8311..e11b1b548e7 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.test.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.test.ts @@ -16,7 +16,6 @@ import { workflowsOrchestrationMock, workflowsOrchestrationMockFns, workflowsPersistenceUtilsMock, - workflowsPersistenceUtilsMockFns, } from '@sim/testing' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -28,7 +27,7 @@ const { mockCheckChatAccess } = vi.hoisted(() => ({ const mockCreateSuccessResponse = workflowsApiUtilsMockFns.mockCreateSuccessResponse const mockCreateErrorResponse = workflowsApiUtilsMockFns.mockCreateErrorResponse const mockEncryptSecret = encryptionMockFns.mockEncryptSecret -const mockDeployWorkflow = workflowsPersistenceUtilsMockFns.mockDeployWorkflow +const mockPerformFullDeploy = workflowsOrchestrationMockFns.mockPerformFullDeploy const mockPerformChatUndeploy = workflowsOrchestrationMockFns.mockPerformChatUndeploy const mockNotifySocketDeploymentChanged = workflowsOrchestrationMockFns.mockNotifySocketDeploymentChanged @@ -73,7 +72,7 @@ describe('Chat Edit API Route', () => { }) mockEncryptSecret.mockResolvedValue({ encrypted: 'encrypted-password' }) - mockDeployWorkflow.mockResolvedValue({ success: true, version: 1 }) + mockPerformFullDeploy.mockResolvedValue({ success: true, version: 1 }) mockNotifySocketDeploymentChanged.mockResolvedValue(undefined) }) diff --git a/apps/sim/app/api/chat/manage/[id]/route.ts b/apps/sim/app/api/chat/manage/[id]/route.ts index 8a70bcb9775..1864d99c108 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.ts @@ -11,12 +11,12 @@ import { isDev } from '@/lib/core/config/feature-flags' import { encryptSecret } from '@/lib/core/security/encryption' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { notifySocketDeploymentChanged, performChatUndeploy } from '@/lib/workflows/orchestration' -import { deployWorkflow } from '@/lib/workflows/persistence/utils' +import { performChatUndeploy, performFullDeploy } from '@/lib/workflows/orchestration' import { checkChatAccess } from '@/app/api/chat/utils' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' export const dynamic = 'force-dynamic' +export const maxDuration = 120 const logger = createLogger('ChatDetailAPI') @@ -126,21 +126,8 @@ export const PATCH = withRouteHandler( } } - // Redeploy the workflow to ensure latest version is active - const deployResult = await deployWorkflow({ - workflowId: existingChat[0].workflowId, - deployedBy: session.user.id, - }) - - if (!deployResult.success) { - logger.warn( - `Failed to redeploy workflow for chat update: ${deployResult.error}, continuing with chat update` - ) - } else { - logger.info( - `Redeployed workflow ${existingChat[0].workflowId} for chat update (v${deployResult.version})` - ) - await notifySocketDeploymentChanged(existingChat[0].workflowId) + if (workflowId && workflowId !== existingChat[0].workflowId) { + return createErrorResponse('Changing a chat deployment workflow is not supported', 400) } let encryptedPassword @@ -156,6 +143,27 @@ export const PATCH = withRouteHandler( logger.info('Keeping existing password') } + // Redeploy the workflow to ensure latest version is active + const deployResult = await performFullDeploy({ + workflowId: existingChat[0].workflowId, + userId: session.user.id, + request, + }) + + if (!deployResult.success) { + logger.warn(`Failed to redeploy workflow for chat update: ${deployResult.error}`) + const status = + deployResult.errorCode === 'validation' + ? 400 + : deployResult.errorCode === 'not_found' + ? 404 + : 500 + return createErrorResponse(deployResult.error || 'Failed to redeploy workflow', status) + } + logger.info( + `Redeployed workflow ${existingChat[0].workflowId} for chat update (v${deployResult.version})` + ) + const updateData: Record = { updatedAt: new Date(), } diff --git a/apps/sim/app/api/form/route.test.ts b/apps/sim/app/api/form/route.test.ts new file mode 100644 index 00000000000..4be49a941eb --- /dev/null +++ b/apps/sim/app/api/form/route.test.ts @@ -0,0 +1,97 @@ +/** + * @vitest-environment node + */ +import { + authMockFns, + dbChainMock, + dbChainMockFns, + resetDbChainMock, + workflowsApiUtilsMock, + workflowsApiUtilsMockFns, + workflowsOrchestrationMock, + workflowsOrchestrationMockFns, +} from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckWorkflowAccessForFormCreation } = vi.hoisted(() => ({ + mockCheckWorkflowAccessForFormCreation: vi.fn(), +})) + +const mockCreateErrorResponse = workflowsApiUtilsMockFns.mockCreateErrorResponse +const mockPerformFullDeploy = workflowsOrchestrationMockFns.mockPerformFullDeploy + +vi.mock('@sim/db', () => dbChainMock) + +vi.mock('@sim/utils/id', () => ({ + generateId: vi.fn(() => 'form-123'), +})) + +vi.mock('@/app/api/form/utils', () => ({ + checkWorkflowAccessForFormCreation: mockCheckWorkflowAccessForFormCreation, + DEFAULT_FORM_CUSTOMIZATIONS: {}, +})) + +vi.mock('@/app/api/workflows/utils', () => workflowsApiUtilsMock) + +vi.mock('@/lib/core/config/feature-flags', () => ({ + isDev: true, +})) + +vi.mock('@/lib/core/utils/urls', () => ({ + getEmailDomain: vi.fn(() => 'localhost:3000'), +})) + +vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock) + +import { POST } from '@/app/api/form/route' + +describe('Form API Route', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + + authMockFns.mockGetSession.mockResolvedValue({ + user: { + id: 'user-123', + email: 'user@example.com', + name: 'Test User', + }, + }) + mockCreateErrorResponse.mockImplementation((message, status = 500) => { + return new Response(JSON.stringify({ error: message }), { + status, + headers: { 'Content-Type': 'application/json' }, + }) + }) + mockCheckWorkflowAccessForFormCreation.mockResolvedValue({ + hasAccess: true, + workflow: { + id: 'workflow-123', + isDeployed: false, + workspaceId: 'workspace-123', + }, + }) + dbChainMockFns.limit.mockResolvedValue([]) + }) + + it('cleans up inserted form when deploy throws', async () => { + mockPerformFullDeploy.mockRejectedValue(new Error('Deploy exploded')) + + const request = new NextRequest('http://localhost:3000/api/form', { + method: 'POST', + body: JSON.stringify({ + workflowId: 'workflow-123', + identifier: 'test-form', + title: 'Test Form', + }), + }) + + const response = await POST(request) + + expect(response.status).toBe(500) + expect(dbChainMockFns.insert).toHaveBeenCalled() + expect(dbChainMockFns.delete).toHaveBeenCalled() + expect(mockCreateErrorResponse).toHaveBeenCalledWith('Deploy exploded', 500) + }) +}) diff --git a/apps/sim/app/api/form/route.ts b/apps/sim/app/api/form/route.ts index 1abc86e264b..c0592bc366b 100644 --- a/apps/sim/app/api/form/route.ts +++ b/apps/sim/app/api/form/route.ts @@ -12,8 +12,7 @@ import { isDev } from '@/lib/core/config/feature-flags' import { encryptSecret } from '@/lib/core/security/encryption' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { notifySocketDeploymentChanged } from '@/lib/workflows/orchestration' -import { deployWorkflow } from '@/lib/workflows/persistence/utils' +import { performFullDeploy } from '@/lib/workflows/orchestration' import { checkWorkflowAccessForFormCreation, DEFAULT_FORM_CUSTOMIZATIONS, @@ -21,11 +20,20 @@ import { import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' const logger = createLogger('FormAPI') +export const maxDuration = 120 function getErrorMessage(error: unknown, fallback: string): string { return error instanceof Error ? error.message : fallback } +async function cleanupFormAfterDeployFailure(formId: string) { + try { + await db.delete(form).where(eq(form.id, formId)) + } catch (cleanupError) { + logger.error('Failed to clean up form after deploy failure:', cleanupError) + } +} + export const GET = withRouteHandler(async (request: NextRequest) => { try { const session = await getSession() @@ -106,21 +114,6 @@ export const POST = withRouteHandler(async (request: NextRequest) => { return createErrorResponse('Workflow not found or access denied', 404) } - const result = await deployWorkflow({ - workflowId, - deployedBy: session.user.id, - }) - - if (!result.success) { - return createErrorResponse(result.error || 'Failed to deploy workflow', 500) - } - - logger.info( - `${workflowRecord.isDeployed ? 'Redeployed' : 'Auto-deployed'} workflow ${workflowId} for form (v${result.version})` - ) - - await notifySocketDeploymentChanged(workflowId) - let encryptedPassword = null if (authType === 'password' && password) { const { encrypted } = await encryptSecret(password) @@ -161,6 +154,29 @@ export const POST = withRouteHandler(async (request: NextRequest) => { updatedAt: new Date(), }) + let result: Awaited> + try { + result = await performFullDeploy({ + workflowId, + userId: session.user.id, + request, + }) + } catch (error) { + await cleanupFormAfterDeployFailure(id) + throw error + } + + if (!result.success) { + await cleanupFormAfterDeployFailure(id) + const status = + result.errorCode === 'validation' ? 400 : result.errorCode === 'not_found' ? 404 : 500 + return createErrorResponse(result.error || 'Failed to deploy workflow', status) + } + + logger.info( + `${workflowRecord.isDeployed ? 'Redeployed' : 'Auto-deployed'} workflow ${workflowId} for form (v${result.version})` + ) + const baseDomain = getEmailDomain() const protocol = isDev ? 'http' : 'https' const formUrl = `${protocol}://${baseDomain}/form/${identifier}` diff --git a/apps/sim/app/api/schedules/execute/route.test.ts b/apps/sim/app/api/schedules/execute/route.test.ts index d5c50c6c647..05434276654 100644 --- a/apps/sim/app/api/schedules/execute/route.test.ts +++ b/apps/sim/app/api/schedules/execute/route.test.ts @@ -20,6 +20,7 @@ const { mockExecuteJobInline, mockFeatureFlags, mockEnqueue, + mockGetJob, mockStartJob, mockCompleteJob, mockMarkJobFailed, @@ -34,6 +35,7 @@ const { isDev: true, }, mockEnqueue: vi.fn().mockResolvedValue('job-id-1'), + mockGetJob: vi.fn().mockResolvedValue(null), mockStartJob: vi.fn().mockResolvedValue(undefined), mockCompleteJob: vi.fn().mockResolvedValue(undefined), mockMarkJobFailed: vi.fn().mockResolvedValue(undefined), @@ -54,6 +56,7 @@ vi.mock('@/lib/core/config/feature-flags', () => mockFeatureFlags) vi.mock('@/lib/core/async-jobs', () => ({ getJobQueue: vi.fn().mockResolvedValue({ enqueue: mockEnqueue, + getJob: mockGetJob, startJob: mockStartJob, completeJob: mockCompleteJob, markJobFailed: mockMarkJobFailed, @@ -69,6 +72,7 @@ vi.mock('drizzle-orm', () => ({ ne: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'ne' })), lte: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'lte' })), lt: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'lt' })), + inArray: vi.fn((field: unknown, values: unknown[]) => ({ field, values, type: 'inArray' })), not: vi.fn((condition: unknown) => ({ type: 'not', condition })), isNull: vi.fn((field: unknown) => ({ type: 'isNull', field })), or: vi.fn((...conditions: unknown[]) => ({ type: 'or', conditions })), @@ -166,6 +170,8 @@ function createMockRequest(): NextRequest { describe('Scheduled Workflow Execution API Route', () => { beforeEach(() => { vi.clearAllMocks() + dbChainMockFns.limit.mockReset() + dbChainMockFns.returning.mockReset() resetDbChainMock() requestUtilsMockFns.mockGenerateRequestId.mockReturnValue('test-request-id') workflowsUtilsMockFns.mockGetWorkflowById.mockResolvedValue({ @@ -180,6 +186,7 @@ describe('Scheduled Workflow Execution API Route', () => { }) it('should execute scheduled workflows with Trigger.dev disabled', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'schedule-1' }]).mockResolvedValueOnce([]) dbChainMockFns.returning.mockReturnValueOnce(SINGLE_SCHEDULE).mockReturnValueOnce([]) const response = await GET(createMockRequest()) @@ -193,6 +200,7 @@ describe('Scheduled Workflow Execution API Route', () => { it('should queue schedules to Trigger.dev when enabled', async () => { mockFeatureFlags.isTriggerDevEnabled = true + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'schedule-1' }]).mockResolvedValueOnce([]) dbChainMockFns.returning.mockReturnValueOnce(SINGLE_SCHEDULE).mockReturnValueOnce([]) const response = await GET(createMockRequest()) @@ -215,6 +223,9 @@ describe('Scheduled Workflow Execution API Route', () => { }) it('should execute multiple schedules in parallel', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([{ id: 'schedule-1' }, { id: 'schedule-2' }]) + .mockResolvedValueOnce([]) dbChainMockFns.returning.mockReturnValueOnce(MULTIPLE_SCHEDULES).mockReturnValueOnce([]) const response = await GET(createMockRequest()) @@ -225,7 +236,8 @@ describe('Scheduled Workflow Execution API Route', () => { }) it('should execute mothership jobs inline', async () => { - dbChainMockFns.returning.mockReturnValueOnce([]).mockReturnValueOnce(SINGLE_JOB) + dbChainMockFns.limit.mockResolvedValueOnce([]).mockResolvedValueOnce([{ id: 'job-1' }]) + dbChainMockFns.returning.mockReturnValueOnce(SINGLE_JOB) const response = await GET(createMockRequest()) @@ -241,6 +253,7 @@ describe('Scheduled Workflow Execution API Route', () => { }) it('should enqueue schedule with correlation metadata via job queue', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'schedule-1' }]).mockResolvedValueOnce([]) dbChainMockFns.returning.mockReturnValueOnce(SINGLE_SCHEDULE).mockReturnValueOnce([]) const response = await GET(createMockRequest()) @@ -255,6 +268,8 @@ describe('Scheduled Workflow Execution API Route', () => { requestId: 'test-request-id', }), expect.objectContaining({ + jobId: expect.stringMatching(/^schedule_[0-9a-f]{32}$/), + concurrencyKey: expect.stringMatching(/^schedule_[0-9a-f]{32}$/), metadata: expect.objectContaining({ workflowId: 'workflow-1', workspaceId: 'workspace-1', diff --git a/apps/sim/app/api/schedules/execute/route.ts b/apps/sim/app/api/schedules/execute/route.ts index 584425196f5..eb2d9bc96b5 100644 --- a/apps/sim/app/api/schedules/execute/route.ts +++ b/apps/sim/app/api/schedules/execute/route.ts @@ -1,11 +1,14 @@ import { db, workflowDeploymentVersion, workflowSchedule } from '@sim/db' import { createLogger } from '@sim/logger' +import { sha256Hex } from '@sim/security/hash' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, eq, isNull, lt, lte, ne, not, or, sql } from 'drizzle-orm' +import { Cron } from 'croner' +import { and, eq, inArray, isNull, lt, lte, ne, not, or, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { verifyCronAuth } from '@/lib/auth/internal' import { getJobQueue, shouldExecuteInline } from '@/lib/core/async-jobs' +import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { @@ -15,8 +18,13 @@ import { } from '@/background/schedule-execution' export const dynamic = 'force-dynamic' +export const maxDuration = 3600 const logger = createLogger('ScheduledExecuteAPI') +const MAX_CRON_CLAIMS = 20 +const RESERVED_WORKFLOW_CLAIMS = 10 +const RESERVED_JOB_CLAIMS = MAX_CRON_CLAIMS - RESERVED_WORKFLOW_CLAIMS +const STALE_SCHEDULE_CLAIM_MS = getMaxExecutionTimeout() const dueFilter = (queuedAt: Date) => and( @@ -26,31 +34,63 @@ const dueFilter = (queuedAt: Date) => ne(workflowSchedule.status, 'completed'), or( isNull(workflowSchedule.lastQueuedAt), - lt(workflowSchedule.lastQueuedAt, workflowSchedule.nextRunAt) + lt(workflowSchedule.lastQueuedAt, workflowSchedule.nextRunAt), + lt(workflowSchedule.lastQueuedAt, new Date(queuedAt.getTime() - STALE_SCHEDULE_CLAIM_MS)) ) ) -export const GET = withRouteHandler(async (request: NextRequest) => { - const requestId = generateRequestId() - logger.info(`[${requestId}] Scheduled execution triggered at ${new Date().toISOString()}`) +const activeWorkflowDeploymentFilter = () => + sql`${workflowSchedule.deploymentVersionId} = (select ${workflowDeploymentVersion.id} from ${workflowDeploymentVersion} where ${workflowDeploymentVersion.workflowId} = ${workflowSchedule.workflowId} and ${workflowDeploymentVersion.isActive} = true)` - const authError = verifyCronAuth(request, 'Schedule execution') - if (authError) { - return authError - } +const workflowScheduleFilter = (queuedAt: Date) => + and( + dueFilter(queuedAt), + or(eq(workflowSchedule.sourceType, 'workflow'), isNull(workflowSchedule.sourceType)), + activeWorkflowDeploymentFilter() + ) - const queuedAt = new Date() +const jobScheduleFilter = (queuedAt: Date) => + and(dueFilter(queuedAt), eq(workflowSchedule.sourceType, 'job')) - try { - // Workflow schedules (require active deployment) - const dueSchedules = await db +function buildScheduleExecutionJobId(schedule: { + id: string + nextRunAt?: Date | null + lastQueuedAt?: Date | null +}): string { + const occurrence = + schedule.nextRunAt?.toISOString() ?? schedule.lastQueuedAt?.toISOString() ?? 'due' + return `schedule_${sha256Hex(`${schedule.id}:${occurrence}`).slice(0, 32)}` +} + +function getNextRunFromCronExpression(cronExpression?: string | null): Date | null { + if (!cronExpression) return null + const cron = new Cron(cronExpression) + return cron.nextRun() +} + +async function claimWorkflowSchedules(queuedAt: Date, limit: number) { + if (limit <= 0) return [] + + return db.transaction(async (tx) => { + const rows = await tx + .select({ id: workflowSchedule.id }) + .from(workflowSchedule) + .where(workflowScheduleFilter(queuedAt)) + .for('update', { skipLocked: true }) + .limit(limit) + + if (rows.length === 0) return [] + + return tx .update(workflowSchedule) .set({ lastQueuedAt: queuedAt, updatedAt: queuedAt }) .where( and( - dueFilter(queuedAt), - or(eq(workflowSchedule.sourceType, 'workflow'), isNull(workflowSchedule.sourceType)), - sql`${workflowSchedule.deploymentVersionId} = (select ${workflowDeploymentVersion.id} from ${workflowDeploymentVersion} where ${workflowDeploymentVersion.workflowId} = ${workflowSchedule.workflowId} and ${workflowDeploymentVersion.isActive} = true)` + workflowScheduleFilter(queuedAt), + inArray( + workflowSchedule.id, + rows.map((row) => row.id) + ) ) ) .returning({ @@ -62,14 +102,37 @@ export const GET = withRouteHandler(async (request: NextRequest) => { failedCount: workflowSchedule.failedCount, nextRunAt: workflowSchedule.nextRunAt, lastQueuedAt: workflowSchedule.lastQueuedAt, + deploymentVersionId: workflowSchedule.deploymentVersionId, sourceType: workflowSchedule.sourceType, }) + }) +} - // Jobs (no deployment, dispatch inline) - const dueJobs = await db +async function claimJobSchedules(queuedAt: Date, limit: number) { + if (limit <= 0) return [] + + return db.transaction(async (tx) => { + const rows = await tx + .select({ id: workflowSchedule.id }) + .from(workflowSchedule) + .where(jobScheduleFilter(queuedAt)) + .for('update', { skipLocked: true }) + .limit(limit) + + if (rows.length === 0) return [] + + return tx .update(workflowSchedule) .set({ lastQueuedAt: queuedAt, updatedAt: queuedAt }) - .where(and(dueFilter(queuedAt), eq(workflowSchedule.sourceType, 'job'))) + .where( + and( + jobScheduleFilter(queuedAt), + inArray( + workflowSchedule.id, + rows.map((row) => row.id) + ) + ) + ) .returning({ id: workflowSchedule.id, cronExpression: workflowSchedule.cronExpression, @@ -77,6 +140,30 @@ export const GET = withRouteHandler(async (request: NextRequest) => { lastQueuedAt: workflowSchedule.lastQueuedAt, sourceType: workflowSchedule.sourceType, }) + }) +} + +export const GET = withRouteHandler(async (request: NextRequest) => { + const requestId = generateRequestId() + logger.info(`[${requestId}] Scheduled execution triggered at ${new Date().toISOString()}`) + + const authError = verifyCronAuth(request, 'Schedule execution') + if (authError) { + return authError + } + + const queuedAt = new Date() + + try { + const dueSchedules = await claimWorkflowSchedules(queuedAt, RESERVED_WORKFLOW_CLAIMS) + const dueJobs = await claimJobSchedules(queuedAt, RESERVED_JOB_CLAIMS) + const remainingClaimBudget = Math.max(0, MAX_CRON_CLAIMS - dueSchedules.length - dueJobs.length) + + if (remainingClaimBudget > 0 && dueSchedules.length === RESERVED_WORKFLOW_CLAIMS) { + dueSchedules.push(...(await claimWorkflowSchedules(queuedAt, remainingClaimBudget))) + } else if (remainingClaimBudget > 0 && dueJobs.length === RESERVED_JOB_CLAIMS) { + dueJobs.push(...(await claimJobSchedules(queuedAt, remainingClaimBudget))) + } const totalCount = dueSchedules.length + dueJobs.length logger.info( @@ -108,6 +195,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { requestId, correlation, blockId: schedule.blockId || undefined, + deploymentVersionId: schedule.deploymentVersionId || undefined, cronExpression: schedule.cronExpression || undefined, lastRanAt: schedule.lastRanAt?.toISOString(), failedCount: schedule.failedCount || 0, @@ -116,12 +204,40 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } try { + const scheduleJobId = buildScheduleExecutionJobId(schedule) + const existingJob = await jobQueue.getJob(scheduleJobId) + if (existingJob && ['pending', 'processing'].includes(existingJob.status)) { + logger.info(`[${requestId}] Schedule execution job already exists`, { + scheduleId: schedule.id, + jobId: scheduleJobId, + status: existingJob.status, + }) + return + } + if (existingJob) { + logger.info(`[${requestId}] Releasing stale schedule claim for finished job`, { + scheduleId: schedule.id, + jobId: scheduleJobId, + status: existingJob.status, + }) + await releaseScheduleLock( + schedule.id, + requestId, + queuedAt, + `Released stale schedule ${schedule.id} for finished job ${scheduleJobId}`, + getNextRunFromCronExpression(schedule.cronExpression) + ) + return + } + const resolvedWorkflow = schedule.workflowId ? await workflowUtils?.getWorkflowById(schedule.workflowId) : null const resolvedWorkspaceId = resolvedWorkflow?.workspaceId const jobId = await jobQueue.enqueue('schedule-execution', payload, { + jobId: scheduleJobId, + concurrencyKey: scheduleJobId, metadata: { workflowId: schedule.workflowId ?? undefined, workspaceId: resolvedWorkspaceId ?? undefined, @@ -132,6 +248,23 @@ export const GET = withRouteHandler(async (request: NextRequest) => { `[${requestId}] Queued schedule execution task ${jobId} for workflow ${schedule.workflowId}` ) + const queuedJob = await jobQueue.getJob(jobId) + if (queuedJob && !['pending', 'processing'].includes(queuedJob.status)) { + logger.info(`[${requestId}] Schedule execution job already finished`, { + scheduleId: schedule.id, + jobId, + status: queuedJob.status, + }) + await releaseScheduleLock( + schedule.id, + requestId, + queuedAt, + `Released stale schedule ${schedule.id} for finished job ${jobId}`, + getNextRunFromCronExpression(schedule.cronExpression) + ) + return + } + if (shouldExecuteInline()) { try { await jobQueue.startJob(jobId) diff --git a/apps/sim/app/api/v1/admin/types.ts b/apps/sim/app/api/v1/admin/types.ts index 0a542820179..13dafe7db70 100644 --- a/apps/sim/app/api/v1/admin/types.ts +++ b/apps/sim/app/api/v1/admin/types.ts @@ -645,6 +645,7 @@ export interface AdminDeployResult { export interface AdminUndeployResult { isDeployed: boolean + warnings?: string[] } // ============================================================================= diff --git a/apps/sim/app/api/v1/admin/workflows/[id]/deploy/route.ts b/apps/sim/app/api/v1/admin/workflows/[id]/deploy/route.ts index 281c9836caf..0f088980489 100644 --- a/apps/sim/app/api/v1/admin/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/v1/admin/workflows/[id]/deploy/route.ts @@ -18,6 +18,7 @@ import { import type { AdminDeployResult, AdminUndeployResult } from '@/app/api/v1/admin/types' const logger = createLogger('AdminWorkflowDeployAPI') +export const maxDuration = 120 interface RouteParams { id: string @@ -109,6 +110,7 @@ export const DELETE = withRouteHandler( const response: AdminUndeployResult = { isDeployed: false, + warnings: result.warnings, } return singleResponse(response) diff --git a/apps/sim/app/api/v1/admin/workflows/[id]/versions/[versionId]/activate/route.ts b/apps/sim/app/api/v1/admin/workflows/[id]/versions/[versionId]/activate/route.ts index aee76a0599b..686cbc71211 100644 --- a/apps/sim/app/api/v1/admin/workflows/[id]/versions/[versionId]/activate/route.ts +++ b/apps/sim/app/api/v1/admin/workflows/[id]/versions/[versionId]/activate/route.ts @@ -14,6 +14,7 @@ import { } from '@/app/api/v1/admin/responses' const logger = createLogger('AdminWorkflowActivateVersionAPI') +export const maxDuration = 120 interface RouteParams { id: string diff --git a/apps/sim/app/api/webhooks/outbox/process/route.ts b/apps/sim/app/api/webhooks/outbox/process/route.ts index 6a5f2b385ee..4ac098c3a2d 100644 --- a/apps/sim/app/api/webhooks/outbox/process/route.ts +++ b/apps/sim/app/api/webhooks/outbox/process/route.ts @@ -6,6 +6,7 @@ import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' import { processOutboxEvents } from '@/lib/core/outbox/service' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { workflowDeploymentOutboxHandlers } from '@/lib/workflows/deployment-outbox' const logger = createLogger('OutboxProcessorAPI') @@ -14,6 +15,7 @@ export const maxDuration = 120 const handlers = { ...billingOutboxHandlers, + ...workflowDeploymentOutboxHandlers, } as const export const GET = withRouteHandler(async (request: NextRequest) => { @@ -25,7 +27,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => { return authError } - const result = await processOutboxEvents(handlers, { batchSize: 20 }) + const result = await processOutboxEvents(handlers, { + batchSize: 20, + maxRuntimeMs: 110_000, + minRemainingMs: 95_000, + }) logger.info('Outbox processing completed', { requestId, ...result }) diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts index 5a188ba9443..1ea8721a825 100644 --- a/apps/sim/app/api/workflows/[id]/deploy/route.ts +++ b/apps/sim/app/api/workflows/[id]/deploy/route.ts @@ -24,6 +24,7 @@ const logger = createLogger('WorkflowDeployAPI') export const dynamic = 'force-dynamic' export const runtime = 'nodejs' +export const maxDuration = 120 export const GET = withRouteHandler( async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { @@ -240,6 +241,7 @@ export const DELETE = withRouteHandler( isDeployed: false, deployedAt: null, apiKey: null, + warnings: result.warnings, }) } catch (error: unknown) { if (error instanceof WorkflowLockedError) { diff --git a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts index e48390dcd12..f8a4113021a 100644 --- a/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts @@ -15,6 +15,7 @@ const logger = createLogger('WorkflowDeploymentVersionAPI') export const dynamic = 'force-dynamic' export const runtime = 'nodejs' +export const maxDuration = 120 export const GET = withRouteHandler( async ( diff --git a/apps/sim/app/api/workflows/[id]/route.test.ts b/apps/sim/app/api/workflows/[id]/route.test.ts index 6ccae0bfe7e..68a0ae3e57d 100644 --- a/apps/sim/app/api/workflows/[id]/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/route.test.ts @@ -27,8 +27,12 @@ const mockGetWorkflowById = workflowsUtilsMockFns.mockGetWorkflowById const mockAuthorizeWorkflowByWorkspacePermission = workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission const mockPerformDeleteWorkflow = workflowsOrchestrationMockFns.mockPerformDeleteWorkflow -const mockDbUpdate = vi.fn() -const mockDbSelect = vi.fn() + +const { mockDbUpdate, mockDbSelect, mockDbTransaction } = vi.hoisted(() => ({ + mockDbUpdate: vi.fn(), + mockDbSelect: vi.fn(), + mockDbTransaction: vi.fn(), +})) /** * Helper to set mock auth state consistently across getSession and hybrid auth. @@ -65,6 +69,7 @@ vi.mock('@sim/db', () => ({ db: { update: () => mockDbUpdate(), select: () => mockDbSelect(), + transaction: mockDbTransaction, }, workflow: {}, })) @@ -80,6 +85,18 @@ describe('Workflow By ID API Route', () => { }) mockLoadWorkflowFromNormalizedTables.mockResolvedValue(null) + mockDbTransaction.mockImplementation(async (callback) => + callback({ + execute: vi.fn().mockResolvedValue(undefined), + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockReturnValue({ + limit: vi.fn().mockResolvedValue([]), + }), + }), + }), + }) + ) }) describe('GET /api/workflows/[id]', () => { diff --git a/apps/sim/app/api/workflows/[id]/route.ts b/apps/sim/app/api/workflows/[id]/route.ts index 6feac3d767d..20c9c896ba5 100644 --- a/apps/sim/app/api/workflows/[id]/route.ts +++ b/apps/sim/app/api/workflows/[id]/route.ts @@ -8,7 +8,7 @@ import { FolderLockedError, WorkflowLockedError, } from '@sim/workflow-authz' -import { and, eq, isNull, ne } from 'drizzle-orm' +import { and, eq, isNull, ne, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { updateWorkflowContract } from '@/lib/api/contracts/workflows' import { parseRequest } from '@/lib/api/server' @@ -85,7 +85,15 @@ export const GET = withRouteHandler( } } - const normalizedData = await loadWorkflowFromNormalizedTables(workflowId) + const snapshot = await db.transaction(async (tx) => { + await tx.execute(sql`SET TRANSACTION ISOLATION LEVEL REPEATABLE READ`) + const [normalizedData, [workflowRecord]] = await Promise.all([ + loadWorkflowFromNormalizedTables(workflowId, tx), + tx.select().from(workflow).where(eq(workflow.id, workflowId)).limit(1), + ]) + return { normalizedData, workflowRecord } + }) + const responseWorkflowData = snapshot.workflowRecord ?? workflowData // Stamp `workflowId` from the path param on each variable so the // global client-side variables store can filter by workflow without @@ -93,7 +101,7 @@ export const GET = withRouteHandler( // The persisted blob may or may not include `workflowId` depending on // when the variable was last written; the path param is authoritative. const persistedVariables = - (workflowData.variables as Record>) || {} + (responseWorkflowData.variables as Record>) || {} const stampedVariables: Record> = {} for (const [variableId, variable] of Object.entries(persistedVariables)) { if (variable && typeof variable === 'object') { @@ -101,20 +109,20 @@ export const GET = withRouteHandler( } } - if (normalizedData) { + if (snapshot.normalizedData) { const finalWorkflowData = { - ...workflowData, + ...responseWorkflowData, state: { - blocks: normalizedData.blocks, - edges: normalizedData.edges, - loops: normalizedData.loops, - parallels: normalizedData.parallels, + blocks: snapshot.normalizedData.blocks, + edges: snapshot.normalizedData.edges, + loops: snapshot.normalizedData.loops, + parallels: snapshot.normalizedData.parallels, lastSaved: Date.now(), - isDeployed: workflowData.isDeployed || false, - deployedAt: workflowData.deployedAt, + isDeployed: responseWorkflowData.isDeployed || false, + deployedAt: responseWorkflowData.deployedAt, metadata: { - name: workflowData.name, - description: workflowData.description, + name: responseWorkflowData.name, + description: responseWorkflowData.description, }, }, variables: stampedVariables, @@ -128,18 +136,18 @@ export const GET = withRouteHandler( } const emptyWorkflowData = { - ...workflowData, + ...responseWorkflowData, state: { blocks: {}, edges: [], loops: {}, parallels: {}, lastSaved: Date.now(), - isDeployed: workflowData.isDeployed || false, - deployedAt: workflowData.deployedAt, + isDeployed: responseWorkflowData.isDeployed || false, + deployedAt: responseWorkflowData.deployedAt, metadata: { - name: workflowData.name, - description: workflowData.description, + name: responseWorkflowData.name, + description: responseWorkflowData.description, }, }, variables: stampedVariables, diff --git a/apps/sim/app/api/workflows/[id]/state/route.ts b/apps/sim/app/api/workflows/[id]/state/route.ts index a28da778e3d..5aa8010084a 100644 --- a/apps/sim/app/api/workflows/[id]/state/route.ts +++ b/apps/sim/app/api/workflows/[id]/state/route.ts @@ -7,7 +7,7 @@ import { authorizeWorkflowByWorkspacePermission, WorkflowLockedError, } from '@sim/workflow-authz' -import { eq } from 'drizzle-orm' +import { eq, sql } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { putWorkflowNormalizedStateContract } from '@/lib/api/contracts/workflows' import { parseRequest } from '@/lib/api/server' @@ -52,8 +52,20 @@ export const GET = withRouteHandler( return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) } - const normalized = await loadWorkflowFromNormalizedTables(workflowId) - if (!normalized) { + const snapshot = await db.transaction(async (tx) => { + await tx.execute(sql`SET TRANSACTION ISOLATION LEVEL REPEATABLE READ`) + const [normalized, [workflowRecord]] = await Promise.all([ + loadWorkflowFromNormalizedTables(workflowId, tx), + tx + .select({ variables: workflow.variables }) + .from(workflow) + .where(eq(workflow.id, workflowId)) + .limit(1), + ]) + return { normalized, variables: workflowRecord?.variables } + }) + + if (!snapshot.normalized) { return NextResponse.json({ error: 'Workflow state not found' }, { status: 404 }) } @@ -62,7 +74,7 @@ export const GET = withRouteHandler( // requiring clients to thread the path param through. The read // contract requires this server-stamped field. const persistedVariables = - (authorization.workflow?.variables as Record>) || {} + (snapshot.variables as Record>) || {} const variables: Record> = {} for (const [variableId, variable] of Object.entries(persistedVariables)) { if (variable && typeof variable === 'object') { @@ -71,10 +83,10 @@ export const GET = withRouteHandler( } return NextResponse.json({ - blocks: normalized.blocks, - edges: normalized.edges, - loops: normalized.loops || {}, - parallels: normalized.parallels || {}, + blocks: snapshot.normalized.blocks, + edges: snapshot.normalized.edges, + loops: snapshot.normalized.loops || {}, + parallels: snapshot.normalized.parallels || {}, variables, }) } catch (error) { @@ -185,10 +197,41 @@ export const PUT = withRouteHandler( deployedAt: state.deployedAt, } - const saveResult = await saveWorkflowToNormalizedTables( - workflowId, - workflowState as WorkflowState - ) + const saveResult = await db.transaction(async (tx) => { + await tx + .select({ id: workflow.id }) + .from(workflow) + .where(eq(workflow.id, workflowId)) + .limit(1) + .for('update') + + const result = await saveWorkflowToNormalizedTables( + workflowId, + workflowState as WorkflowState, + tx + ) + + if (!result.success) return result + + // Update workflow's lastSynced timestamp and variables if provided + const updateData: { + lastSynced: Date + updatedAt: Date + variables?: typeof state.variables + } = { + lastSynced: new Date(), + updatedAt: new Date(), + } + + // If variables are provided in the state, update them in the workflow record + if (state.variables !== undefined) { + updateData.variables = state.variables + } + + await tx.update(workflow).set(updateData).where(eq(workflow.id, workflowId)) + + return result + }) if (!saveResult.success) { logger.error( @@ -235,19 +278,6 @@ export const PUT = withRouteHandler( logger.error(`[${requestId}] Failed to persist custom tools`, { error, workflowId }) } - // Update workflow's lastSynced timestamp and variables if provided - const updateData: any = { - lastSynced: new Date(), - updatedAt: new Date(), - } - - // If variables are provided in the state, update them in the workflow record - if (state.variables !== undefined) { - updateData.variables = state.variables - } - - await db.update(workflow).set(updateData).where(eq(workflow.id, workflowId)) - const elapsed = Date.now() - startTime logger.info(`[${requestId}] Successfully saved workflow ${workflowId} state in ${elapsed}ms`) diff --git a/apps/sim/app/api/workflows/utils.ts b/apps/sim/app/api/workflows/utils.ts index f460f551b1f..223f6b1e02a 100644 --- a/apps/sim/app/api/workflows/utils.ts +++ b/apps/sim/app/api/workflows/utils.ts @@ -1,9 +1,9 @@ -import { db, workflow, workflowDeploymentVersion } from '@sim/db' +import { db, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' import { and, desc, eq } from 'drizzle-orm' import { NextResponse } from 'next/server' import { hasWorkflowChanged } from '@/lib/workflows/comparison' -import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils' +import { loadWorkflowDeploymentSnapshot } from '@/lib/workflows/persistence/utils' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -46,25 +46,10 @@ export async function checkNeedsRedeployment(workflowId: string): Promise void onPromoteToLive: (version: number) => void @@ -46,6 +47,7 @@ export function Versions({ workflowId, versions, versionsLoading, + isPromotingVersion, selectedVersion, onSelectVersion, onPromoteToLive, @@ -115,6 +117,7 @@ export function Versions({ } const handlePromote = (version: number) => { + if (isPromotingVersion) return setOpenDropdown(null) onPromoteToLive(version) } @@ -320,7 +323,7 @@ export function Versions({ onOpenChange={(open) => setOpenDropdown(open ? v.version : null)} > - diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx index 695d6c95cf5..1a41d9f6299 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx @@ -1,6 +1,6 @@ 'use client' -import { useCallback, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { createLogger } from '@sim/logger' import { Button, @@ -17,8 +17,10 @@ import { Tooltip, } from '@/components/emcn' import type { WorkflowDeploymentVersionResponse } from '@/lib/workflows/persistence/utils' +import type { DeployReadiness } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness' import { Preview, PreviewWorkflow } from '@/app/workspace/[workspaceId]/w/components/preview' import { useDeploymentVersionState, useRevertToVersion } from '@/hooks/queries/workflows' +import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import type { WorkflowState } from '@/stores/workflows/workflow/types' import { Versions } from './components' @@ -30,8 +32,11 @@ interface GeneralDeployProps { isLoadingDeployedState: boolean versions: WorkflowDeploymentVersionResponse[] versionsLoading: boolean + isPromotingVersion: boolean + deployReadiness: DeployReadiness onPromoteToLive: (version: number) => Promise onLoadDeploymentComplete: () => void + onLoadDeploymentBlocked: (message: string) => void } type PreviewMode = 'active' | 'selected' @@ -45,8 +50,11 @@ export function GeneralDeploy({ isLoadingDeployedState, versions, versionsLoading, + isPromotingVersion, + deployReadiness, onPromoteToLive, onLoadDeploymentComplete, + onLoadDeploymentBlocked, }: GeneralDeployProps) { const [selectedVersion, setSelectedVersion] = useState(null) const [showActiveDespiteSelection, setShowActiveDespiteSelection] = useState(false) @@ -56,12 +64,18 @@ export function GeneralDeploy({ const [showLoadDialog, setShowLoadDialog] = useState(false) const [showPromoteDialog, setShowPromoteDialog] = useState(false) const [showExpandedPreview, setShowExpandedPreview] = useState(false) - const [versionToLoad, setVersionToLoad] = useState(null) - const [versionToPromote, setVersionToPromote] = useState(null) + const [versionToLoad, setVersionToLoad] = useState<{ + workflowId: string + version: number + } | null>(null) + const [versionToPromote, setVersionToPromote] = useState<{ + workflowId: string + version: number + } | null>(null) const selectedVersionInfo = versions.find((v) => v.version === selectedVersion) - const versionToPromoteInfo = versions.find((v) => v.version === versionToPromote) - const versionToLoadInfo = versions.find((v) => v.version === versionToLoad) + const versionToPromoteInfo = versions.find((v) => v.version === versionToPromote?.version) + const versionToLoadInfo = versions.find((v) => v.version === versionToLoad?.version) const { data: selectedVersionState } = useDeploymentVersionState(workflowId, selectedVersion) @@ -72,40 +86,82 @@ export function GeneralDeploy({ setShowActiveDespiteSelection(false) }, []) - const handleLoadDeployment = useCallback((version: number) => { - setVersionToLoad(version) - setShowLoadDialog(true) - }, []) + const handleLoadDeployment = useCallback( + (version: number) => { + if (!workflowId) return + setVersionToLoad({ workflowId, version }) + setShowLoadDialog(true) + }, + [workflowId] + ) - const handlePromoteToLive = useCallback((version: number) => { - setVersionToPromote(version) - setShowPromoteDialog(true) - }, []) + const handlePromoteToLive = useCallback( + (version: number) => { + if (!workflowId) return + setVersionToPromote({ workflowId, version }) + setShowPromoteDialog(true) + }, + [workflowId] + ) const confirmLoadDeployment = async () => { - if (!workflowId || versionToLoad === null) return + if (!versionToLoad) return + const target = versionToLoad + if (!(await deployReadiness.waitUntilReady())) { + if ( + workflowId !== target.workflowId || + useWorkflowRegistry.getState().activeWorkflowId !== target.workflowId + ) { + setShowLoadDialog(false) + setVersionToLoad(null) + return + } + onLoadDeploymentBlocked(deployReadiness.tooltip) + return + } + if ( + workflowId !== target.workflowId || + useWorkflowRegistry.getState().activeWorkflowId !== target.workflowId + ) { + setShowLoadDialog(false) + setVersionToLoad(null) + return + } setShowLoadDialog(false) - const version = versionToLoad setVersionToLoad(null) try { - await revertMutation.mutateAsync({ workflowId, version }) + await revertMutation.mutateAsync({ workflowId: target.workflowId, version: target.version }) onLoadDeploymentComplete() } catch (error) { logger.error('Failed to load deployment:', error) } } + useEffect(() => { + setShowLoadDialog(false) + setVersionToLoad(null) + setShowPromoteDialog(false) + setVersionToPromote(null) + }, [workflowId]) + const confirmPromoteToLive = async () => { - if (versionToPromote === null) return + if (!versionToPromote || isPromotingVersion) return + const target = versionToPromote setShowPromoteDialog(false) - const version = versionToPromote setVersionToPromote(null) + if ( + workflowId !== target.workflowId || + useWorkflowRegistry.getState().activeWorkflowId !== target.workflowId + ) { + return + } + try { - await onPromoteToLive(version) + await onPromoteToLive(target.version) } catch (error) { logger.error('Failed to promote version:', error) } @@ -221,6 +277,7 @@ export function GeneralDeploy({ workflowId={workflowId} versions={versions} versionsLoading={versionsLoading} + isPromotingVersion={isPromotingVersion} selectedVersion={selectedVersion} onSelectVersion={handleSelectVersion} onPromoteToLive={handlePromoteToLive} @@ -236,7 +293,7 @@ export function GeneralDeploy({

Are you sure you want to load{' '} - {versionToLoadInfo?.name || `v${versionToLoad}`} + {versionToLoadInfo?.name || `v${versionToLoad?.version}`} ?{' '} @@ -262,7 +319,7 @@ export function GeneralDeploy({

Are you sure you want to promote{' '} - {versionToPromoteInfo?.name || `v${versionToPromote}`} + {versionToPromoteInfo?.name || `v${versionToPromote?.version}`} {' '} to live?{' '} @@ -274,7 +331,7 @@ export function GeneralDeploy({ - diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx index 38df5590737..1103e3d7fe6 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx @@ -2,11 +2,13 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' import { useQueryClient } from '@tanstack/react-query' import { useParams } from 'next/navigation' import { Badge, Button, + Loader, Modal, ModalBody, ModalContent, @@ -21,6 +23,12 @@ import { getBaseUrl } from '@/lib/core/utils/urls' import { getInputFormatExample as getInputFormatExampleUtil } from '@/lib/workflows/operations/deployment-utils' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { CreateApiKeyModal } from '@/app/workspace/[workspaceId]/settings/components/api-keys/components' +import { + releaseDeployAction, + tryAcquireDeployAction, +} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock' +import { syncLocalDraftFromServer } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft' +import type { DeployReadiness } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness' import { runPreDeployChecks } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-predeploy-checks' import { startsWithUuid } from '@/executor/constants' import { useA2AAgentByWorkflow } from '@/hooks/queries/a2a/agents' @@ -40,6 +48,7 @@ import { useWorkflowMap } from '@/hooks/queries/workflows' import { useWorkspaceSettings } from '@/hooks/queries/workspace' import { usePermissionConfig } from '@/hooks/use-permission-config' import { useSettingsNavigation } from '@/hooks/use-settings-navigation' +import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { mergeSubblockState } from '@/stores/workflows/utils' import { useWorkflowStore } from '@/stores/workflows/workflow/store' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -62,6 +71,8 @@ interface DeployModalProps { needsRedeployment: boolean deployedState?: WorkflowState | null isLoadingDeployedState: boolean + deployReadiness: DeployReadiness + isDeploymentSettling: boolean } interface WorkflowDeploymentInfoUI { @@ -84,6 +95,8 @@ export function DeployModal({ needsRedeployment, deployedState, isLoadingDeployedState, + deployReadiness, + isDeploymentSettling, }: DeployModalProps) { const queryClient = useQueryClient() const params = useParams() @@ -97,10 +110,12 @@ export function DeployModal({ const [chatSubmitting, setChatSubmitting] = useState(false) const [deployError, setDeployError] = useState(null) const [deployWarnings, setDeployWarnings] = useState([]) + const [isFinalizingDeploy, setIsFinalizingDeploy] = useState(false) + const [isActivatingVersion, setIsActivatingVersion] = useState(false) const [isChatFormValid, setIsChatFormValid] = useState(false) const [selectedStreamingOutputs, setSelectedStreamingOutputs] = useState([]) - const [showUndeployConfirm, setShowUndeployConfirm] = useState(false) + const [undeployTargetWorkflowId, setUndeployTargetWorkflowId] = useState(null) // const [templateFormValid, setTemplateFormValid] = useState(false) // const [templateSubmitting, setTemplateSubmitting] = useState(false) const [mcpToolSubmitting, setMcpToolSubmitting] = useState(false) @@ -112,6 +127,8 @@ export function DeployModal({ const [chatSuccess, setChatSuccess] = useState(false) const chatSuccessTimeoutRef = useRef | null>(null) + const deployActionIdRef = useRef(0) + const activateVersionInFlightRef = useRef(false) const [isCreateKeyModalOpen, setIsCreateKeyModalOpen] = useState(false) const [isApiInfoModalOpen, setIsApiInfoModalOpen] = useState(false) @@ -176,6 +193,35 @@ export function DeployModal({ const versions = versionsData?.versions ?? [] + const isWorkflowStillActive = useCallback((targetWorkflowId: string) => { + return useWorkflowRegistry.getState().activeWorkflowId === targetWorkflowId + }, []) + + const syncDraftAfterDeploy = useCallback(async (): Promise => { + if (!workflowId) return null + + try { + const syncedActiveWorkflow = await syncLocalDraftFromServer(workflowId) + if (!syncedActiveWorkflow && isWorkflowStillActive(workflowId)) { + return 'Deployment succeeded, but local sync is still catching up. Refresh if the status looks stale.' + } + return null + } catch (error) { + if (!isWorkflowStillActive(workflowId)) return null + logger.warn('Workflow deployed, but local draft sync failed', { + workflowId, + error: toError(error).message, + }) + return 'Deployment succeeded, but local sync failed. Refresh if the status looks stale.' + } + }, [workflowId, isWorkflowStillActive]) + + useEffect(() => { + deployActionIdRef.current += 1 + setIsFinalizingDeploy(false) + setUndeployTargetWorkflowId(null) + }, [workflowId]) + const getApiKeyLabel = useCallback( (value?: string | null) => { if (value && value.trim().length > 0) { @@ -285,87 +331,157 @@ export function DeployModal({ const onDeploy = useCallback(async () => { if (!workflowId) return + if (!tryAcquireDeployAction(workflowId)) return + const actionId = deployActionIdRef.current + 1 + deployActionIdRef.current = actionId + setIsFinalizingDeploy(true) setDeployError(null) setDeployWarnings([]) try { - // Deploy mutation handles query invalidation in its onSuccess callback - const result = await deployMutation.mutateAsync({ workflowId }) - if (result.warnings && result.warnings.length > 0) { - setDeployWarnings(result.warnings) + if (!(await deployReadiness.waitUntilReady())) { + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployError(deployReadiness.tooltip) + return + } + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + + try { + const result = await deployMutation.mutateAsync({ workflowId }) + const syncWarning = await syncDraftAfterDeploy() + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) + } finally { + if (deployActionIdRef.current === actionId) { + setIsFinalizingDeploy(false) + } } } catch (error: unknown) { + if (deployActionIdRef.current !== actionId) return + if (!isWorkflowStillActive(workflowId)) return logger.error('Error deploying workflow:', { error }) - const errorMessage = error instanceof Error ? error.message : 'Failed to deploy workflow' + const errorMessage = toError(error).message || 'Failed to deploy workflow' setDeployError(errorMessage) + } finally { + releaseDeployAction(workflowId) + if (deployActionIdRef.current === actionId) { + setIsFinalizingDeploy(false) + } } - }, [workflowId, deployMutation]) + }, [workflowId, deployMutation, deployReadiness, syncDraftAfterDeploy, isWorkflowStillActive]) const handlePromoteToLive = useCallback( async (version: number) => { if (!workflowId) return + if (activateVersionInFlightRef.current) return + activateVersionInFlightRef.current = true + setIsActivatingVersion(true) setDeployWarnings([]) try { const result = await activateVersionMutation.mutateAsync({ workflowId, version }) + if (!isWorkflowStillActive(workflowId)) return if (result.warnings && result.warnings.length > 0) { setDeployWarnings(result.warnings) } } catch (error) { + if (!isWorkflowStillActive(workflowId)) return logger.error('Error promoting version:', { error }) throw error + } finally { + activateVersionInFlightRef.current = false + setIsActivatingVersion(false) } }, - [workflowId, activateVersionMutation] + [workflowId, activateVersionMutation, isWorkflowStillActive] ) const handleUndeploy = useCallback(async () => { - if (!workflowId) return + if (!undeployTargetWorkflowId) return + const targetWorkflowId = undeployTargetWorkflowId + if (workflowId !== targetWorkflowId || !isWorkflowStillActive(targetWorkflowId)) { + setUndeployTargetWorkflowId(null) + return + } + + setDeployWarnings([]) try { - await undeployMutation.mutateAsync({ workflowId }) - setShowUndeployConfirm(false) + const result = await undeployMutation.mutateAsync({ workflowId: targetWorkflowId }) + if (!isWorkflowStillActive(targetWorkflowId)) return + setUndeployTargetWorkflowId(null) + if (result.warnings && result.warnings.length > 0) { + setDeployWarnings(result.warnings) + return + } onOpenChange(false) } catch (error: unknown) { + if (!isWorkflowStillActive(targetWorkflowId)) return logger.error('Error undeploying workflow:', { error }) } - }, [workflowId, undeployMutation, onOpenChange]) + }, [workflowId, undeployTargetWorkflowId, undeployMutation, onOpenChange, isWorkflowStillActive]) const handleRedeploy = useCallback(async () => { if (!workflowId) return + if (!tryAcquireDeployAction(workflowId)) return + const actionId = deployActionIdRef.current + 1 + deployActionIdRef.current = actionId + setIsFinalizingDeploy(true) setDeployError(null) setDeployWarnings([]) - const { blocks, edges, loops, parallels } = useWorkflowStore.getState() - const liveBlocks = mergeSubblockState(blocks, workflowId) - const checkResult = runPreDeployChecks({ - blocks: liveBlocks, - edges, - loops, - parallels, - workflowId, - }) - if (!checkResult.passed) { - setDeployError(checkResult.error || 'Pre-deploy validation failed') - return - } - try { - const result = await deployMutation.mutateAsync({ workflowId }) - if (result.warnings && result.warnings.length > 0) { - setDeployWarnings(result.warnings) + if (!(await deployReadiness.waitUntilReady())) { + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployError(deployReadiness.tooltip) + return + } + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + + const { blocks, edges, loops, parallels } = useWorkflowStore.getState() + const liveBlocks = mergeSubblockState(blocks, workflowId) + const checkResult = runPreDeployChecks({ + blocks: liveBlocks, + edges, + loops, + parallels, + workflowId, + }) + if (!checkResult.passed) { + setDeployError(checkResult.error || 'Pre-deploy validation failed') + return + } + + try { + const result = await deployMutation.mutateAsync({ workflowId }) + const syncWarning = await syncDraftAfterDeploy() + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) + } finally { + if (deployActionIdRef.current === actionId) { + setIsFinalizingDeploy(false) + } } } catch (error: unknown) { + if (deployActionIdRef.current !== actionId) return + if (!isWorkflowStillActive(workflowId)) return logger.error('Error redeploying workflow:', { error }) - const errorMessage = error instanceof Error ? error.message : 'Failed to redeploy workflow' + const errorMessage = toError(error).message || 'Failed to redeploy workflow' setDeployError(errorMessage) + } finally { + releaseDeployAction(workflowId) + if (deployActionIdRef.current === actionId) { + setIsFinalizingDeploy(false) + } } - }, [workflowId, deployMutation]) + }, [workflowId, deployMutation, deployReadiness, syncDraftAfterDeploy, isWorkflowStillActive]) const handleCloseModal = useCallback(() => { + deployActionIdRef.current += 1 + setIsFinalizingDeploy(false) setChatSubmitting(false) setDeployError(null) setDeployWarnings([]) @@ -456,7 +572,7 @@ export function DeployModal({ // deleteTrigger?.click() // }, []) - const isSubmitting = deployMutation.isPending + const isSubmitting = deployMutation.isPending || isFinalizingDeploy const isUndeploying = undeployMutation.isPending return ( @@ -514,8 +630,11 @@ export function DeployModal({ isLoadingDeployedState={isLoadingDeployedState} versions={versions} versionsLoading={versionsLoading} + isPromotingVersion={isActivatingVersion || activateVersionMutation.isPending} + deployReadiness={deployReadiness} onPromoteToLive={handlePromoteToLive} onLoadDeploymentComplete={handleCloseModal} + onLoadDeploymentBlocked={setDeployError} /> @@ -610,9 +729,13 @@ export function DeployModal({ needsRedeployment={needsRedeployment} isSubmitting={isSubmitting} isUndeploying={isUndeploying} + deployReadiness={deployReadiness} + isDeploymentSettling={isDeploymentSettling} onDeploy={onDeploy} onRedeploy={handleRedeploy} - onUndeploy={() => setShowUndeployConfirm(true)} + onUndeploy={() => { + if (workflowId) setUndeployTargetWorkflowId(workflowId) + }} /> )} {activeTab === 'api' && ( @@ -841,7 +964,12 @@ export function DeployModal({ - + { + if (!nextOpen) setUndeployTargetWorkflowId(null) + }} + > Undeploy API @@ -855,7 +983,7 @@ export function DeployModal({ @@ -990,14 +1132,20 @@ function GeneralFooter({ return ( - +

+ + {blockedMessage && ( +
{blockedMessage}
+ )} +
- {needsRedeployment && ( - )}
diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx index 5d225bbd7f5..ec630e08781 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx @@ -6,6 +6,7 @@ import { DeployModal } from '@/app/workspace/[workspaceId]/w/[workflowId]/compon import { useChangeDetection, useDeployment, + useDeployReadiness, } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks' import { useCurrentWorkflow } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-current-workflow' import { useDeployedWorkflowState, useDeploymentInfo } from '@/hooks/queries/deployments' @@ -42,21 +43,30 @@ export function Deploy({ isFetching: isFetchingDeployedState, } = useDeployedWorkflowState(activeWorkflowId, { enabled: isDeployedStateEnabled }) const deployedState = isDeployedStateEnabled ? (deployedStateData ?? null) : null + const deployReadiness = useDeployReadiness(activeWorkflowId) - const { changeDetected } = useChangeDetection({ + const { changeDetected, isChangeDetectionSettling } = useChangeDetection({ workflowId: activeWorkflowId, deployedState, isLoadingDeployedState: isLoadingDeployedState || isFetchingDeployedState, }) + const isDeploymentSettling = isChangeDetectionSettling || deployReadiness.isSyncing const { isDeploying, handleDeployClick } = useDeployment({ workflowId: activeWorkflowId, isDeployed, + deployReadiness, }) const isEmpty = !hasBlocks() const canDeploy = userPermissions.canAdmin - const isDisabled = disabled || isDeploying || !canDeploy || isEmpty + const isDisabled = + disabled || + isDeploying || + !canDeploy || + isEmpty || + isDeploymentSettling || + (!isDeployed && deployReadiness.isBlocked) const onDeployClick = async () => { if (disabled || !canDeploy || !activeWorkflowId) return @@ -80,6 +90,12 @@ export function Deploy({ if (isDeploying) { return 'Deploying...' } + if (isChangeDetectionSettling) { + return 'Syncing deployment state...' + } + if (deployReadiness.isBlocked && !isDeployed) { + return deployReadiness.tooltip + } if (changeDetected) { return 'Update deployment' } @@ -89,6 +105,19 @@ export function Deploy({ return 'Deploy workflow' } + const getButtonLabel = () => { + if (isDeployed && (changeDetected || isDeploymentSettling)) { + return 'Update' + } + if (changeDetected) { + return 'Update' + } + if (isDeployed) { + return 'Live' + } + return 'Deploy' + } + return ( <> @@ -97,13 +126,19 @@ export function Deploy({ @@ -117,7 +152,9 @@ export function Deploy({ isDeployed={isDeployed} needsRedeployment={changeDetected} deployedState={deployedState} - isLoadingDeployedState={isLoadingDeployedState} + isLoadingDeployedState={isLoadingDeployedState || isFetchingDeployedState} + deployReadiness={deployReadiness} + isDeploymentSettling={isDeploymentSettling} /> ) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.test.ts new file mode 100644 index 00000000000..92f1f23890b --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.test.ts @@ -0,0 +1,24 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + releaseDeployAction, + tryAcquireDeployAction, +} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock' + +describe('deploy action lock', () => { + it('serializes deploy actions per workflow', () => { + try { + expect(tryAcquireDeployAction('workflow-a')).toBe(true) + expect(tryAcquireDeployAction('workflow-a')).toBe(false) + expect(tryAcquireDeployAction('workflow-b')).toBe(true) + + releaseDeployAction('workflow-a') + expect(tryAcquireDeployAction('workflow-a')).toBe(true) + } finally { + releaseDeployAction('workflow-a') + releaseDeployAction('workflow-b') + } + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.ts new file mode 100644 index 00000000000..d2de6485b17 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.ts @@ -0,0 +1,14 @@ +const activeDeployActions = new Set() + +export function tryAcquireDeployAction(workflowId: string): boolean { + if (activeDeployActions.has(workflowId)) { + return false + } + + activeDeployActions.add(workflowId) + return true +} + +export function releaseDeployAction(workflowId: string): void { + activeDeployActions.delete(workflowId) +} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/index.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/index.ts index 9bd9bf02271..171b8718bbc 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/index.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/index.ts @@ -1,2 +1,4 @@ export { useChangeDetection } from './use-change-detection' +export type { DeployReadiness } from './use-deploy-readiness' +export { getDeployReadinessState, useDeployReadiness } from './use-deploy-readiness' export { useDeployment } from './use-deployment' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.test.ts new file mode 100644 index 00000000000..6abc9a694d0 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.test.ts @@ -0,0 +1,246 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockRequestJson, + mockApplyWorkflowStateToStores, + mockGetRegistryState, + mockHasPendingOperations, + mockGetOperationQueueState, + mockGetWorkflowDiffState, +} = vi.hoisted(() => ({ + mockRequestJson: vi.fn(), + mockApplyWorkflowStateToStores: vi.fn(), + mockGetRegistryState: vi.fn(() => ({ activeWorkflowId: 'workflow-a' })), + mockHasPendingOperations: vi.fn(() => false), + mockGetOperationQueueState: vi.fn(() => ({ + hasPendingOperations: mockHasPendingOperations, + workflowOperationVersions: {}, + })), + mockGetWorkflowDiffState: vi.fn(() => ({ + hasActiveDiff: false, + pendingExternalUpdates: {}, + reconcilingWorkflows: {}, + reconciliationErrors: {}, + remoteUpdateVersions: {}, + })), +})) + +vi.mock('@/lib/api/client/request', () => ({ + requestJson: mockRequestJson, +})) + +vi.mock('@/lib/api/contracts', () => ({ + getWorkflowStateContract: {}, +})) + +vi.mock('@/stores/workflow-diff/utils', () => ({ + applyWorkflowStateToStores: mockApplyWorkflowStateToStores, +})) + +vi.mock('@/stores/workflow-diff/store', () => ({ + useWorkflowDiffStore: { + getState: mockGetWorkflowDiffState, + }, +})) + +vi.mock('@/stores/operation-queue/store', () => ({ + useOperationQueueStore: { + getState: mockGetOperationQueueState, + }, +})) + +vi.mock('@/stores/workflows/registry/store', () => ({ + useWorkflowRegistry: { + getState: mockGetRegistryState, + }, +})) + +import { syncLocalDraftFromServer } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft' + +describe('syncLocalDraftFromServer', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetRegistryState.mockReturnValue({ activeWorkflowId: 'workflow-a' }) + mockHasPendingOperations.mockReturnValue(false) + mockGetOperationQueueState.mockImplementation(() => ({ + hasPendingOperations: mockHasPendingOperations, + workflowOperationVersions: {}, + })) + mockGetWorkflowDiffState.mockReturnValue({ + hasActiveDiff: false, + pendingExternalUpdates: {}, + reconcilingWorkflows: {}, + reconciliationErrors: {}, + remoteUpdateVersions: {}, + }) + }) + + it('hydrates sibling workflow variables into the applied workflow state', async () => { + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + variables: { + 'variable-a': { + id: 'variable-a', + name: 'API_KEY', + type: 'plain', + value: 'secret', + }, + }, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(true) + + expect(mockApplyWorkflowStateToStores).toHaveBeenCalledWith( + 'workflow-a', + expect.objectContaining({ + variables: { + 'variable-a': { + id: 'variable-a', + name: 'API_KEY', + type: 'plain', + value: 'secret', + }, + }, + }), + { updateLastSaved: true } + ) + }) + + it('does not apply a fetched draft after navigation changes the active workflow', async () => { + mockGetRegistryState + .mockReturnValueOnce({ activeWorkflowId: 'workflow-a' }) + .mockReturnValueOnce({ activeWorkflowId: 'workflow-b' }) + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + variables: {}, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(false) + + expect(mockApplyWorkflowStateToStores).not.toHaveBeenCalled() + }) + + it('does not synthesize an empty variables object when the server omits variables', async () => { + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(true) + + const appliedState = mockApplyWorkflowStateToStores.mock.calls[0][1] + expect(Object.hasOwn(appliedState, 'variables')).toBe(false) + }) + + it('does not apply a fetched draft over newly queued local operations', async () => { + mockHasPendingOperations.mockReturnValueOnce(false).mockReturnValueOnce(true) + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + variables: {}, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(false) + + expect(mockApplyWorkflowStateToStores).not.toHaveBeenCalled() + }) + + it('does not apply a fetched draft when a newer remote update arrives during fetch', async () => { + mockGetWorkflowDiffState + .mockReturnValueOnce({ + hasActiveDiff: false, + pendingExternalUpdates: {}, + reconcilingWorkflows: {}, + reconciliationErrors: {}, + remoteUpdateVersions: {}, + }) + .mockReturnValueOnce({ + hasActiveDiff: false, + pendingExternalUpdates: {}, + reconcilingWorkflows: {}, + reconciliationErrors: {}, + remoteUpdateVersions: { 'workflow-a': 1 }, + }) + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + variables: {}, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(false) + + expect(mockApplyWorkflowStateToStores).not.toHaveBeenCalled() + }) + + it('does not apply a fetched draft when local operations queue and drain during fetch', async () => { + mockGetOperationQueueState + .mockReturnValueOnce({ + hasPendingOperations: mockHasPendingOperations, + workflowOperationVersions: {}, + }) + .mockReturnValueOnce({ + hasPendingOperations: mockHasPendingOperations, + workflowOperationVersions: {}, + }) + .mockReturnValueOnce({ + hasPendingOperations: mockHasPendingOperations, + workflowOperationVersions: { 'workflow-a': 1 }, + }) + mockRequestJson.mockResolvedValue({ + data: { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + lastSaved: 1, + }, + variables: {}, + }, + }) + + await expect(syncLocalDraftFromServer('workflow-a')).resolves.toBe(false) + + expect(mockApplyWorkflowStateToStores).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.ts new file mode 100644 index 00000000000..ad308ee549c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.ts @@ -0,0 +1,59 @@ +import { requestJson } from '@/lib/api/client/request' +import { getWorkflowStateContract } from '@/lib/api/contracts' +import { useOperationQueueStore } from '@/stores/operation-queue/store' +import { useWorkflowDiffStore } from '@/stores/workflow-diff/store' +import { applyWorkflowStateToStores } from '@/stores/workflow-diff/utils' +import { useWorkflowRegistry } from '@/stores/workflows/registry/store' +import type { WorkflowState } from '@/stores/workflows/workflow/types' + +function canApplyServerSnapshot( + workflowId: string, + remoteVersionAtStart: number, + localOperationVersionAtStart: number +): boolean { + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) return false + const operationQueueState = useOperationQueueStore.getState() + if (operationQueueState.hasPendingOperations(workflowId)) return false + if ( + (operationQueueState.workflowOperationVersions[workflowId] ?? 0) !== + localOperationVersionAtStart + ) { + return false + } + + const diffState = useWorkflowDiffStore.getState() + return ( + !diffState.hasActiveDiff && + !diffState.pendingExternalUpdates[workflowId] && + !diffState.reconcilingWorkflows[workflowId] && + !diffState.reconciliationErrors[workflowId] && + (diffState.remoteUpdateVersions[workflowId] ?? 0) === remoteVersionAtStart + ) +} + +export async function syncLocalDraftFromServer(workflowId: string): Promise { + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) return false + if (useOperationQueueStore.getState().hasPendingOperations(workflowId)) return false + const localOperationVersionAtStart = + useOperationQueueStore.getState().workflowOperationVersions[workflowId] ?? 0 + const remoteVersionAtStart = useWorkflowDiffStore.getState().remoteUpdateVersions[workflowId] ?? 0 + + const responseData = await requestJson(getWorkflowStateContract, { + params: { id: workflowId }, + }) + const wireState = responseData.data?.state + if (!canApplyServerSnapshot(workflowId, remoteVersionAtStart, localOperationVersionAtStart)) { + return false + } + if (!wireState) { + throw new Error('No workflow state was returned while syncing the local draft') + } + + // double-cast-allowed: workflowStateSchema is a wire supertype; normalized workflow state is persisted in store-compatible shape + const workflowState = wireState as unknown as WorkflowState + if (Object.hasOwn(responseData.data, 'variables')) { + workflowState.variables = responseData.data.variables || {} + } + applyWorkflowStateToStores(workflowId, workflowState, { updateLastSaved: true }) + return true +} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts index a9a319688e1..a66bc19990e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts @@ -70,5 +70,8 @@ export function useChangeDetection({ return hasWorkflowChanged(currentState, deployedState) }, [currentState, deployedState, isLoadingDeployedState]) - return { changeDetected } + return { + changeDetected, + isChangeDetectionSettling: Boolean(workflowId && isLoadingDeployedState), + } } diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.test.ts new file mode 100644 index 00000000000..39b0a3849b2 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.test.ts @@ -0,0 +1,101 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/stores/operation-queue/store', () => ({ + useOperationQueueStore: Object.assign( + () => ({ hasPendingOperations: false, hasOperationError: false }), + { + getState: () => ({ + hasOperationError: false, + hasPendingOperations: () => false, + waitForWorkflowOperations: () => Promise.resolve(true), + }), + } + ), +})) + +vi.mock('@/stores/workflow-diff/store', () => ({ + useWorkflowDiffStore: Object.assign( + () => ({ + hasActiveDiff: false, + hasPendingExternalUpdate: false, + isReconciling: false, + }), + { + getState: () => ({ + hasActiveDiff: false, + pendingExternalUpdates: {}, + reconcilingWorkflows: {}, + reconciliationErrors: {}, + }), + } + ), +})) + +import { getDeployReadinessState } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness' + +const baseInput = { + workflowId: 'workflow-a', + hasPendingOperations: false, + hasOperationError: false, + hasActiveDiff: false, + hasPendingExternalUpdate: false, + isReconciling: false, + reconciliationError: undefined, +} + +describe('getDeployReadinessState', () => { + it('allows deploy when no local persistence or reconciliation is pending', () => { + expect(getDeployReadinessState(baseInput).status).toBe('ready') + }) + + it('blocks deploy while active workflow operations are pending', () => { + const readiness = getDeployReadinessState({ + ...baseInput, + hasPendingOperations: true, + }) + + expect(readiness.status).toBe('saving') + expect(readiness.label).toBe('Saving...') + }) + + it('ignores queued operations before they are scoped to the active workflow', () => { + expect( + getDeployReadinessState({ + ...baseInput, + hasPendingOperations: false, + }).status + ).toBe('ready') + }) + + it('uses a neutral syncing state while external updates reconcile', () => { + const readiness = getDeployReadinessState({ + ...baseInput, + hasPendingExternalUpdate: true, + }) + + expect(readiness.status).toBe('syncing') + expect(readiness.label).toBe('Syncing...') + }) + + it('blocks deploy while copilot diff changes are under review', () => { + expect( + getDeployReadinessState({ + ...baseInput, + hasActiveDiff: true, + }).status + ).toBe('reviewing-diff') + }) + + it('surfaces reconciliation failures as deploy-blocking sync errors', () => { + const readiness = getDeployReadinessState({ + ...baseInput, + reconciliationError: 'Latest workflow changes failed to sync', + }) + + expect(readiness.status).toBe('error') + expect(readiness.tooltip).toBe('Latest workflow changes failed to sync') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.ts new file mode 100644 index 00000000000..23490ce63b3 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.ts @@ -0,0 +1,159 @@ +import { useCallback, useMemo } from 'react' +import { useShallow } from 'zustand/react/shallow' +import { useOperationQueueStore } from '@/stores/operation-queue/store' +import { useWorkflowDiffStore } from '@/stores/workflow-diff/store' + +export type DeployReadinessStatus = + | 'ready' + | 'missing-workflow' + | 'saving' + | 'reviewing-diff' + | 'syncing' + | 'error' + +interface DeployReadinessInput { + workflowId: string | null + hasPendingOperations: boolean + hasOperationError: boolean + hasActiveDiff: boolean + hasPendingExternalUpdate: boolean + isReconciling: boolean + reconciliationError?: string +} + +export interface DeployReadiness { + status: DeployReadinessStatus + isReady: boolean + isBlocked: boolean + isSyncing: boolean + label: string + tooltip: string + waitUntilReady: () => Promise +} + +export function getDeployReadinessState(input: DeployReadinessInput) { + if (!input.workflowId) { + return { + status: 'missing-workflow' as const, + label: 'Deploy', + tooltip: 'No workflow selected', + } + } + + if (input.hasOperationError || input.reconciliationError) { + return { + status: 'error' as const, + label: 'Sync failed', + tooltip: + input.reconciliationError || + 'Some changes failed to save. Reconnect or refresh before deploying.', + } + } + + if (input.hasPendingOperations) { + return { + status: 'saving' as const, + label: 'Saving...', + tooltip: 'Saving workflow changes before deployment', + } + } + + if (input.hasActiveDiff) { + return { + status: 'reviewing-diff' as const, + label: 'Reviewing...', + tooltip: 'Accept or reject the current copilot changes before deploying', + } + } + + if (input.hasPendingExternalUpdate || input.isReconciling) { + return { + status: 'syncing' as const, + label: 'Syncing...', + tooltip: 'Syncing the latest workflow changes before deployment', + } + } + + return { + status: 'ready' as const, + label: 'Ready', + tooltip: 'Ready to deploy', + } +} + +export function useDeployReadiness(workflowId: string | null): DeployReadiness { + const { hasPendingOperations, hasOperationError } = useOperationQueueStore( + useShallow((state) => ({ + hasPendingOperations: workflowId + ? state.operations.some((op) => op.workflowId === workflowId) + : false, + hasOperationError: state.hasOperationError, + })) + ) + + const { hasActiveDiff, hasPendingExternalUpdate, isReconciling, reconciliationError } = + useWorkflowDiffStore( + useShallow((state) => ({ + hasActiveDiff: state.hasActiveDiff, + hasPendingExternalUpdate: workflowId + ? Boolean(state.pendingExternalUpdates[workflowId]) + : false, + isReconciling: workflowId ? Boolean(state.reconcilingWorkflows[workflowId]) : false, + reconciliationError: workflowId ? state.reconciliationErrors[workflowId] : undefined, + })) + ) + + const readiness = useMemo( + () => + getDeployReadinessState({ + workflowId, + hasPendingOperations, + hasOperationError, + hasActiveDiff, + hasPendingExternalUpdate, + isReconciling, + reconciliationError, + }), + [ + workflowId, + hasPendingOperations, + hasOperationError, + hasActiveDiff, + hasPendingExternalUpdate, + isReconciling, + reconciliationError, + ] + ) + + const waitUntilReady = useCallback(async () => { + if (!workflowId) return false + + const queue = useOperationQueueStore.getState() + if (queue.hasOperationError) return false + + const drained = await queue.waitForWorkflowOperations(workflowId) + if (!drained) return false + + const latestQueue = useOperationQueueStore.getState() + const diff = useWorkflowDiffStore.getState() + return ( + !latestQueue.hasOperationError && + !latestQueue.hasPendingOperations(workflowId) && + !diff.hasActiveDiff && + !diff.pendingExternalUpdates[workflowId] && + !diff.reconcilingWorkflows[workflowId] && + !diff.reconciliationErrors[workflowId] + ) + }, [workflowId]) + + const isReady = readiness.status === 'ready' + const isSyncing = readiness.status === 'saving' || readiness.status === 'syncing' + + return { + ...readiness, + isReady, + isBlocked: !isReady, + isSyncing, + waitUntilReady, + } +} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts index 3b894847c4b..9982a1d0595 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts @@ -1,13 +1,22 @@ -import { useCallback } from 'react' +import { useCallback, useState } from 'react' +import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' import { runPreDeployChecks } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-predeploy-checks' import { useDeployWorkflow } from '@/hooks/queries/deployments' import { useNotificationStore } from '@/stores/notifications' +import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { mergeSubblockState } from '@/stores/workflows/utils' import { useWorkflowStore } from '@/stores/workflows/workflow/store' +import { releaseDeployAction, tryAcquireDeployAction } from './deploy-action-lock' +import { syncLocalDraftFromServer } from './sync-local-draft' +import type { DeployReadiness } from './use-deploy-readiness' + +const logger = createLogger('UseDeployment') interface UseDeploymentProps { workflowId: string | null isDeployed: boolean + deployReadiness: DeployReadiness } /** @@ -15,8 +24,9 @@ interface UseDeploymentProps { * First deploy: runs pre-deploy checks, then deploys via mutation and opens modal. * Already deployed: opens modal directly (validation happens on Update in modal). */ -export function useDeployment({ workflowId, isDeployed }: UseDeploymentProps) { +export function useDeployment({ workflowId, isDeployed, deployReadiness }: UseDeploymentProps) { const { mutateAsync, isPending: isDeploying } = useDeployWorkflow() + const [isFinalizingDeploy, setIsFinalizingDeploy] = useState(false) const addNotification = useNotificationStore((state) => state.addNotification) const handleDeployClick = useCallback(async () => { @@ -26,40 +36,102 @@ export function useDeployment({ workflowId, isDeployed }: UseDeploymentProps) { return { success: true, shouldOpenModal: true } } - const { blocks, edges, loops, parallels } = useWorkflowStore.getState() - const liveBlocks = mergeSubblockState(blocks, workflowId) - const checkResult = runPreDeployChecks({ - blocks: liveBlocks, - edges, - loops, - parallels, - workflowId, - }) - if (!checkResult.passed) { + if (!tryAcquireDeployAction(workflowId)) { addNotification({ - level: 'error', - message: checkResult.error || 'Pre-deploy validation failed', + level: 'info', + message: 'Deployment is already in progress.', workflowId, }) return { success: false, shouldOpenModal: false } } + setIsFinalizingDeploy(true) try { - await mutateAsync({ workflowId }) - return { success: true, shouldOpenModal: true } - } catch (error) { - const errorMessage = error instanceof Error ? error.message : 'Failed to deploy workflow' - addNotification({ - level: 'error', - message: errorMessage, + const isReady = await deployReadiness.waitUntilReady() + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) { + return { success: false, shouldOpenModal: false } + } + if (!isReady) { + addNotification({ + level: deployReadiness.status === 'error' ? 'error' : 'info', + message: deployReadiness.tooltip, + workflowId, + }) + return { success: false, shouldOpenModal: false } + } + + const { blocks, edges, loops, parallels } = useWorkflowStore.getState() + const liveBlocks = mergeSubblockState(blocks, workflowId) + const checkResult = runPreDeployChecks({ + blocks: liveBlocks, + edges, + loops, + parallels, workflowId, }) - return { success: false, shouldOpenModal: false } + if (!checkResult.passed) { + addNotification({ + level: 'error', + message: checkResult.error || 'Pre-deploy validation failed', + workflowId, + }) + return { success: false, shouldOpenModal: false } + } + + try { + await mutateAsync({ workflowId }) + } catch (error) { + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) { + return { success: false, shouldOpenModal: false } + } + const errorMessage = toError(error).message || 'Failed to deploy workflow' + addNotification({ + level: 'error', + message: errorMessage, + workflowId, + }) + return { success: false, shouldOpenModal: false } + } + + try { + const syncedActiveWorkflow = await syncLocalDraftFromServer(workflowId) + if (!syncedActiveWorkflow) { + if (useWorkflowRegistry.getState().activeWorkflowId === workflowId) { + logger.warn('Workflow deployed, but local draft sync was deferred', { workflowId }) + addNotification({ + level: 'info', + message: + 'Deployment succeeded, but local sync is still catching up. Refresh if the status looks stale.', + workflowId, + }) + } + return { success: true, shouldOpenModal: false } + } + } catch (error) { + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) { + return { success: true, shouldOpenModal: false } + } + logger.warn('Workflow deployed, but local draft sync failed', { + workflowId, + error: toError(error).message, + }) + addNotification({ + level: 'info', + message: + 'Deployment succeeded, but local sync failed. Refresh if the status looks stale.', + workflowId, + }) + } + + return { success: true, shouldOpenModal: true } + } finally { + releaseDeployAction(workflowId) + setIsFinalizingDeploy(false) } - }, [workflowId, isDeployed, addNotification, mutateAsync]) + }, [workflowId, isDeployed, deployReadiness, addNotification, mutateAsync]) return { - isDeploying, + isDeploying: isDeploying || isFinalizingDeploy, handleDeployClick, } } diff --git a/apps/sim/background/schedule-execution.ts b/apps/sim/background/schedule-execution.ts index 3b6c4cd5bfd..7da80cab40f 100644 --- a/apps/sim/background/schedule-execution.ts +++ b/apps/sim/background/schedule-execution.ts @@ -1,12 +1,23 @@ -import { db, jobExecutionLogs, workflow, workflowSchedule } from '@sim/db' +import { + db, + jobExecutionLogs, + workflow, + workflowDeploymentVersion, + workflowSchedule, +} from '@sim/db' import { createLogger, runWithRequestContext } from '@sim/logger' import { toError } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { task } from '@trigger.dev/sdk' import { Cron } from 'croner' import { and, eq, isNull } from 'drizzle-orm' +import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription' import type { AsyncExecutionCorrelation } from '@/lib/core/async-jobs/types' -import { createTimeoutAbortController, getTimeoutErrorMessage } from '@/lib/core/execution-limits' +import { + createTimeoutAbortController, + getExecutionTimeout, + getTimeoutErrorMessage, +} from '@/lib/core/execution-limits' import { preprocessExecution } from '@/lib/execution/preprocessing' import { LoggingSession } from '@/lib/logs/execution/logging-session' import { buildTraceSpans } from '@/lib/logs/execution/trace-spans/trace-spans' @@ -15,10 +26,7 @@ import { wasExecutionFinalizedByCore, } from '@/lib/workflows/executor/execution-core' import { handlePostExecutionPauseState } from '@/lib/workflows/executor/pause-persistence' -import { - blockExistsInDeployment, - loadDeployedWorkflowState, -} from '@/lib/workflows/persistence/utils' +import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils' import { type BlockState, calculateNextRunTime as calculateNextTime, @@ -40,7 +48,11 @@ type WorkflowScheduleUpdate = Partial type ExecutionCoreResult = Awaited> type RunWorkflowResult = - | { status: 'skip'; blocks: Record } + | { + status: 'skip' + reason: 'stale_deployment' | 'invalid_schedule' + blocks: Record + } | { status: 'success'; blocks: Record; executionResult: ExecutionCoreResult } | { status: 'failure'; blocks: Record; executionResult: ExecutionCoreResult } @@ -137,6 +149,25 @@ async function determineNextRunAfterError( return new Date(now.getTime() + 24 * 60 * 60 * 1000) } +async function isScheduleDeploymentVersionActive( + workflowId: string, + deploymentVersionId: string +): Promise { + const [activeDeployment] = await db + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.id, deploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + return Boolean(activeDeployment) +} + async function runWorkflowExecution({ payload, correlation, @@ -164,16 +195,32 @@ async function runWorkflowExecution({ const blocks = deployedData.blocks const { deploymentVersionId } = deployedData + if (payload.deploymentVersionId && deploymentVersionId !== payload.deploymentVersionId) { + logger.info(`[${requestId}] Loaded deployment no longer matches queued schedule, skipping`, { + scheduleId: payload.scheduleId, + workflowId: payload.workflowId, + queuedDeploymentVersionId: payload.deploymentVersionId, + loadedDeploymentVersionId: deploymentVersionId, + }) + return { + status: 'skip', + reason: 'stale_deployment', + blocks: {} as Record, + } + } logger.info(`[${requestId}] Loaded deployed workflow ${payload.workflowId}`) if (payload.blockId) { - const blockExists = await blockExistsInDeployment(payload.workflowId, payload.blockId) - if (!blockExists) { + if (!blocks[payload.blockId]) { logger.warn( `[${requestId}] Schedule trigger block ${payload.blockId} not found in deployed workflow ${payload.workflowId}. Skipping execution.` ) - return { status: 'skip', blocks: {} as Record } + return { + status: 'skip', + reason: 'invalid_schedule', + blocks: {} as Record, + } } } @@ -199,6 +246,13 @@ async function runWorkflowExecution({ triggerType: 'schedule', triggerBlockId: payload.blockId || undefined, useDraftState: false, + workflowStateOverride: { + blocks: deployedData.blocks, + edges: deployedData.edges, + loops: deployedData.loops, + parallels: deployedData.parallels, + deploymentVersionId, + }, startTime: new Date().toISOString(), isClientSession: false, correlation, @@ -216,6 +270,22 @@ async function runWorkflowExecution({ let executionResult try { + if ( + payload.deploymentVersionId && + !(await isScheduleDeploymentVersionActive(payload.workflowId, payload.deploymentVersionId)) + ) { + logger.info(`[${requestId}] Schedule deployment changed before execution, skipping`, { + scheduleId: payload.scheduleId, + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + }) + return { + status: 'skip', + reason: 'stale_deployment', + blocks: {} as Record, + } + } + executionResult = await executeWorkflowCore({ snapshot, callbacks: {}, @@ -289,6 +359,7 @@ export type ScheduleExecutionPayload = { requestId?: string correlation?: AsyncExecutionCorrelation blockId?: string + deploymentVersionId?: string cronExpression?: string lastRanAt?: string failedCount?: number @@ -340,6 +411,7 @@ export async function executeScheduleJob(payload: ScheduleExecutionPayload) { .select({ id: workflowSchedule.id, workflowId: workflowSchedule.workflowId, + deploymentVersionId: workflowSchedule.deploymentVersionId, status: workflowSchedule.status, archivedAt: workflowSchedule.archivedAt, }) @@ -367,6 +439,37 @@ export async function executeScheduleJob(payload: ScheduleExecutionPayload) { return } + const expectedDeploymentVersionId = + payload.deploymentVersionId ?? scheduleRecord.deploymentVersionId ?? undefined + if (expectedDeploymentVersionId) { + const [activeDeployment] = await db + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, payload.workflowId), + eq(workflowDeploymentVersion.id, expectedDeploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + if (!activeDeployment) { + logger.info(`[${requestId}] Schedule deployment version is no longer active, skipping`, { + scheduleId: payload.scheduleId, + workflowId: payload.workflowId, + deploymentVersionId: expectedDeploymentVersionId, + }) + await releaseScheduleLock( + payload.scheduleId, + requestId, + now, + `Failed to release stale deployment schedule ${payload.scheduleId}` + ) + return + } + } + const loggingSession = new LoggingSession( payload.workflowId, executionId, @@ -538,6 +641,16 @@ export async function executeScheduleJob(payload: ScheduleExecutionPayload) { }) if (executionResult.status === 'skip') { + if (executionResult.reason === 'stale_deployment') { + await releaseScheduleLock( + payload.scheduleId, + requestId, + now, + `Failed to release stale schedule ${payload.scheduleId} after deployment version changed` + ) + return + } + await applyScheduleUpdate( payload.scheduleId, { @@ -899,6 +1012,8 @@ export async function executeJobInline(payload: JobExecutionPayload) { const promptText = buildJobPrompt(jobRecord) try { + const userSubscription = await getHighestPrioritySubscription(jobRecord.sourceUserId) + const mothershipJobTimeoutMs = getExecutionTimeout(userSubscription?.plan, 'sync') const url = buildAPIUrl('/api/mothership/execute') const headers = await buildAuthHeaders(jobRecord.sourceUserId) @@ -910,16 +1025,52 @@ export async function executeJobInline(payload: JobExecutionPayload) { } const startTime = new Date() - const response = await fetch(url.toString(), { - method: 'POST', - headers, - body: JSON.stringify(body), - }) - const endTime = new Date() - const durationMs = endTime.getTime() - startTime.getTime() + const timeoutController = createTimeoutAbortController(mothershipJobTimeoutMs) + try { + const response = await fetch(url.toString(), { + method: 'POST', + headers, + body: JSON.stringify(body), + signal: timeoutController.signal, + }) + + if (!response.ok) { + const errorText = await response.text().catch(() => { + if (timeoutController.isTimedOut()) { + throw new Error(getTimeoutErrorMessage(null, timeoutController.timeoutMs)) + } + return 'Unknown error' + }) + const endTime = new Date() + const durationMs = endTime.getTime() - startTime.getTime() + + await createJobLogEntry({ + scheduleId: payload.scheduleId, + workspaceId: jobRecord.sourceWorkspaceId, + jobTitle: jobRecord.jobTitle, + startTime, + endTime, + durationMs, + success: false, + errorMessage: errorText, + }) - if (!response.ok) { - const errorText = await response.text().catch(() => 'Unknown error') + throw new Error(`Mothership execution failed (${response.status}): ${errorText}`) + } + + let responseBody: Record = {} + let wasCompletedByTool = false + try { + responseBody = await response.json() + const toolCalls = responseBody?.toolCalls as Array<{ name?: string }> | undefined + wasCompletedByTool = toolCalls?.some((tc) => tc.name === 'complete_job') ?? false + } catch { + if (timeoutController.isTimedOut()) { + throw new Error(getTimeoutErrorMessage(null, timeoutController.timeoutMs)) + } + } + const endTime = new Date() + const durationMs = endTime.getTime() - startTime.getTime() await createJobLogEntry({ scheduleId: payload.scheduleId, @@ -928,92 +1079,71 @@ export async function executeJobInline(payload: JobExecutionPayload) { startTime, endTime, durationMs, - success: false, - errorMessage: errorText, + success: true, + responseBody, }) - throw new Error(`Mothership execution failed (${response.status}): ${errorText}`) - } + const newRunCount = (jobRecord.runCount || 0) + 1 - let responseBody: Record = {} - let wasCompletedByTool = false - try { - responseBody = await response.json() - const toolCalls = responseBody?.toolCalls as Array<{ name?: string }> | undefined - wasCompletedByTool = toolCalls?.some((tc) => tc.name === 'complete_job') ?? false - } catch { - // Response may not be JSON; proceed with normal flow - } + logger.info(`[${requestId}] Job executed successfully`, { + scheduleId: payload.scheduleId, + runCount: newRunCount, + wasCompletedByTool, + }) - await createJobLogEntry({ - scheduleId: payload.scheduleId, - workspaceId: jobRecord.sourceWorkspaceId, - jobTitle: jobRecord.jobTitle, - startTime, - endTime, - durationMs, - success: true, - responseBody, - }) + if (wasCompletedByTool) { + await applyScheduleUpdate( + payload.scheduleId, + { + lastRanAt: now, + updatedAt: now, + runCount: newRunCount, + failedCount: 0, + lastQueuedAt: null, + }, + requestId, + `Error updating job ${payload.scheduleId} after completion` + ) + return + } - const newRunCount = (jobRecord.runCount || 0) + 1 + const isOneTime = !jobRecord.cronExpression + let nextRunAt: Date | null = null - logger.info(`[${requestId}] Job executed successfully`, { - scheduleId: payload.scheduleId, - runCount: newRunCount, - wasCompletedByTool, - }) + if (!isOneTime && jobRecord.cronExpression) { + const validation = validateCronExpression( + jobRecord.cronExpression, + jobRecord.timezone || 'UTC' + ) + nextRunAt = validation.nextRun || null + } + + const maxRunsReached = jobRecord.maxRuns && newRunCount >= jobRecord.maxRuns + if (maxRunsReached) { + logger.info(`[${requestId}] Job hit maxRuns limit`, { + scheduleId: payload.scheduleId, + maxRuns: jobRecord.maxRuns, + runCount: newRunCount, + }) + } - if (wasCompletedByTool) { await applyScheduleUpdate( payload.scheduleId, { lastRanAt: now, updatedAt: now, - runCount: newRunCount, + nextRunAt: isOneTime || maxRunsReached ? null : nextRunAt, failedCount: 0, lastQueuedAt: null, + runCount: newRunCount, + status: isOneTime || maxRunsReached ? 'completed' : 'active', }, requestId, - `Error updating job ${payload.scheduleId} after completion` - ) - return - } - - const isOneTime = !jobRecord.cronExpression - let nextRunAt: Date | null = null - - if (!isOneTime && jobRecord.cronExpression) { - const validation = validateCronExpression( - jobRecord.cronExpression, - jobRecord.timezone || 'UTC' + `Error updating job ${payload.scheduleId} after success` ) - nextRunAt = validation.nextRun || null - } - - const maxRunsReached = jobRecord.maxRuns && newRunCount >= jobRecord.maxRuns - if (maxRunsReached) { - logger.info(`[${requestId}] Job hit maxRuns limit`, { - scheduleId: payload.scheduleId, - maxRuns: jobRecord.maxRuns, - runCount: newRunCount, - }) + } finally { + timeoutController.cleanup() } - - await applyScheduleUpdate( - payload.scheduleId, - { - lastRanAt: now, - updatedAt: now, - nextRunAt: isOneTime || maxRunsReached ? null : nextRunAt, - failedCount: 0, - lastQueuedAt: null, - runCount: newRunCount, - status: isOneTime || maxRunsReached ? 'completed' : 'active', - }, - requestId, - `Error updating job ${payload.scheduleId} after success` - ) } catch (error) { const errorMessage = toError(error).message logger.error(`[${requestId}] Job execution failed`, { diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 3803e53ffd6..48d1e970410 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -103,6 +103,7 @@ export class BlockExecutor { } let resolvedInputs: Record = {} + let inputsForLog: Record = {} const nodeMetadata = { ...this.buildNodeMetadata(node), @@ -120,15 +121,20 @@ export class BlockExecutor { } if (block.metadata?.id === BlockType.FUNCTION) { - const { resolvedInputs: fnInputs, contextVariables } = - this.resolver.resolveInputsForFunctionBlock(ctx, node.id, block.config.params, block) + const { + resolvedInputs: fnInputs, + displayInputs, + contextVariables, + } = this.resolver.resolveInputsForFunctionBlock(ctx, node.id, block.config.params, block) resolvedInputs = { ...fnInputs, [FUNCTION_BLOCK_CONTEXT_VARS_KEY]: contextVariables } + inputsForLog = displayInputs } else { resolvedInputs = this.resolver.resolveInputs(ctx, node.id, block.config.params, block) + inputsForLog = resolvedInputs } if (blockLog) { - blockLog.input = this.sanitizeInputsForLog(resolvedInputs) + blockLog.input = this.sanitizeInputsForLog(inputsForLog) } } catch (error) { cleanupSelfReference?.() @@ -139,7 +145,7 @@ export class BlockExecutor { block, startTime, blockLog, - resolvedInputs, + inputsForLog, isSentinel, 'input_resolution' ) @@ -212,7 +218,7 @@ export class BlockExecutor { ctx, node, block, - this.sanitizeInputsForLog(resolvedInputs), + this.sanitizeInputsForLog(inputsForLog), displayOutput, duration, blockLog.startedAt, @@ -231,7 +237,7 @@ export class BlockExecutor { block, startTime, blockLog, - resolvedInputs, + inputsForLog, isSentinel, 'execution' ) @@ -262,19 +268,18 @@ export class BlockExecutor { block: SerializedBlock, startTime: number, blockLog: BlockLog | undefined, - resolvedInputs: Record, + inputsForLog: Record, isSentinel: boolean, phase: 'input_resolution' | 'execution' ): Promise { const endedAt = new Date().toISOString() const duration = performance.now() - startTime const errorMessage = normalizeError(error) - const hasResolvedInputs = - resolvedInputs && typeof resolvedInputs === 'object' && Object.keys(resolvedInputs).length > 0 - const input = - hasResolvedInputs && resolvedInputs - ? resolvedInputs - : ((block.config?.params as Record | undefined) ?? {}) + const hasLogInputs = + inputsForLog && typeof inputsForLog === 'object' && Object.keys(inputsForLog).length > 0 + const input = hasLogInputs + ? inputsForLog + : ((block.config?.params as Record | undefined) ?? {}) const errorOutput: NormalizedBlockOutput = { error: errorMessage, diff --git a/apps/sim/executor/variables/resolver.test.ts b/apps/sim/executor/variables/resolver.test.ts index 915afa54ae6..545d4fa91de 100644 --- a/apps/sim/executor/variables/resolver.test.ts +++ b/apps/sim/executor/variables/resolver.test.ts @@ -66,7 +66,7 @@ describe('VariableResolver function block inputs', () => { const result = resolver.resolveInputsForFunctionBlock(ctx, 'function', undefined, block) - expect(result).toEqual({ resolvedInputs: {}, contextVariables: {} }) + expect(result).toEqual({ resolvedInputs: {}, displayInputs: {}, contextVariables: {} }) }) it('resolves JavaScript block references through globalThis context variables', () => { @@ -80,6 +80,7 @@ describe('VariableResolver function block inputs', () => { ) expect(result.resolvedInputs.code).toBe('return globalThis["__blockRef_0"]') + expect(result.displayInputs.code).toBe('return ') expect(result.contextVariables).toEqual({ __blockRef_0: 'hello world' }) }) @@ -94,6 +95,7 @@ describe('VariableResolver function block inputs', () => { ) expect(result.resolvedInputs.code).toBe('return globals()["__blockRef_0"]') + expect(result.displayInputs.code).toBe('return ') expect(result.contextVariables).toEqual({ __blockRef_0: 'hello world' }) }) @@ -110,6 +112,7 @@ describe('VariableResolver function block inputs', () => { expect(result.resolvedInputs.code).toBe( 'a = globals()["__blockRef_0"]\nb = globals()["__blockRef_1"]\nreturn b' ) + expect(result.displayInputs.code).toBe('a = \nb = \nreturn b') expect(result.contextVariables).toEqual({ __blockRef_0: ['a', 'b'], __blockRef_1: ['a', 'b'], @@ -129,6 +132,9 @@ describe('VariableResolver function block inputs', () => { expect(result.resolvedInputs.code).toBe( `echo "\${__blockRef_0}"suffix && echo "\${__blockRef_1}"` ) + expect(result.displayInputs.code).toBe( + 'echo suffix && echo ""' + ) expect(result.contextVariables).toEqual({ __blockRef_0: 'hello world', __blockRef_1: 'hello world', @@ -148,6 +154,7 @@ describe('VariableResolver function block inputs', () => { expect(result.resolvedInputs.code).toBe( `# don't confuse quote tracking\necho "\${__blockRef_0}"` ) + expect(result.displayInputs.code).toBe("# don't confuse quote tracking\necho ") expect(result.contextVariables).toEqual({ __blockRef_0: 'hello world' }) }) }) diff --git a/apps/sim/executor/variables/resolver.ts b/apps/sim/executor/variables/resolver.ts index 2cc9fd89e5b..d8566278e82 100644 --- a/apps/sim/executor/variables/resolver.ts +++ b/apps/sim/executor/variables/resolver.ts @@ -47,27 +47,32 @@ export class VariableResolver { * are stored as named context variables instead of being embedded as JavaScript * literals, preventing large values from bloating the code string. * - * Returns the resolved inputs and a `contextVariables` map. Callers should inject - * contextVariables into the function execution request body so the isolated VM can - * access them as global variables. + * Returns runtime inputs, display inputs, and a `contextVariables` map. Callers + * should inject contextVariables into the function execution request body so the + * isolated VM can access them as global variables. */ resolveInputsForFunctionBlock( ctx: ExecutionContext, currentNodeId: string, params: Record | null | undefined, block: SerializedBlock - ): { resolvedInputs: Record; contextVariables: Record } { + ): { + resolvedInputs: Record + displayInputs: Record + contextVariables: Record + } { const contextVariables: Record = {} const resolved: Record = {} + const display: Record = {} if (!params) { - return { resolvedInputs: resolved, contextVariables } + return { resolvedInputs: resolved, displayInputs: display, contextVariables } } for (const [key, value] of Object.entries(params)) { if (key === 'code') { if (typeof value === 'string') { - resolved[key] = this.resolveCodeWithContextVars( + const code = this.resolveCodeWithContextVars( ctx, currentNodeId, value, @@ -75,32 +80,47 @@ export class VariableResolver { block, contextVariables ) + resolved[key] = code.resolvedCode + display[key] = code.displayCode } else if (Array.isArray(value)) { - resolved[key] = value.map((item: any) => { + const resolvedItems: any[] = [] + const displayItems: any[] = [] + for (const item of value) { if (item && typeof item === 'object' && typeof item.content === 'string') { - return { + const code = this.resolveCodeWithContextVars( + ctx, + currentNodeId, + item.content, + undefined, + block, + contextVariables + ) + resolvedItems.push({ ...item, - content: this.resolveCodeWithContextVars( - ctx, - currentNodeId, - item.content, - undefined, - block, - contextVariables - ), - } + content: code.resolvedCode, + }) + displayItems.push({ + ...item, + content: code.displayCode, + }) + continue } - return item - }) + resolvedItems.push(item) + displayItems.push(item) + } + resolved[key] = resolvedItems + display[key] = displayItems } else { resolved[key] = this.resolveValue(ctx, currentNodeId, value, undefined, block) + display[key] = resolved[key] } } else { resolved[key] = this.resolveValue(ctx, currentNodeId, value, undefined, block) + display[key] = resolved[key] } } - return { resolvedInputs: resolved, contextVariables } + return { resolvedInputs: resolved, displayInputs: display, contextVariables } } resolveInputs( @@ -230,7 +250,7 @@ export class VariableResolver { loopScope: LoopScope | undefined, block: SerializedBlock, contextVarAccumulator: Record - ): string { + ): { resolvedCode: string; displayCode: string } { const resolutionContext: ResolutionContext = { executionContext: ctx, executionState: this.state, @@ -243,14 +263,21 @@ export class VariableResolver { | undefined let replacementError: Error | null = null + let displayResult = '' + let displayCursor = 0 let result = replaceValidReferences(template, (match, index) => { if (replacementError) return match + displayResult += template.slice(displayCursor, index) + displayCursor = index + match.length try { if (this.blockResolver.canResolve(match)) { const resolved = this.resolveReference(match, resolutionContext) - if (resolved === undefined) return match + if (resolved === undefined) { + displayResult += match + return match + } const effectiveValue = resolved === RESOLVED_EMPTY ? null : resolved @@ -265,21 +292,33 @@ export class VariableResolver { index, effectiveValue ) + displayResult += match return replacement } const resolved = this.resolveReference(match, resolutionContext) - if (resolved === undefined) return match + if (resolved === undefined) { + displayResult += match + return match + } const effectiveValue = resolved === RESOLVED_EMPTY ? null : resolved // Non-block reference (loop, parallel, workflow, env): embed as literal - return this.blockResolver.formatValueForBlock(effectiveValue, BlockType.FUNCTION, language) + const replacement = this.blockResolver.formatValueForBlock( + effectiveValue, + BlockType.FUNCTION, + language + ) + displayResult += replacement + return replacement } catch (error) { replacementError = error instanceof Error ? error : new Error(String(error)) + displayResult += match return match } }) + displayResult += template.slice(displayCursor) if (replacementError !== null) { throw replacementError @@ -289,8 +328,12 @@ export class VariableResolver { const resolved = this.resolveReference(match, resolutionContext) return typeof resolved === 'string' ? resolved : match }) + displayResult = displayResult.replace(createEnvVarPattern(), (match) => { + const resolved = this.resolveReference(match, resolutionContext) + return typeof resolved === 'string' ? resolved : match + }) - return result + return { resolvedCode: result, displayCode: displayResult } } private formatContextVariableReference( diff --git a/apps/sim/hooks/queries/deployments.test.ts b/apps/sim/hooks/queries/deployments.test.ts index 20701175468..0e9559d2087 100644 --- a/apps/sim/hooks/queries/deployments.test.ts +++ b/apps/sim/hooks/queries/deployments.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node */ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { invalidateDeploymentQueries } from '@/hooks/queries/deployments' +import { invalidateDeploymentQueries, refetchDeploymentBoundary } from '@/hooks/queries/deployments' import { fetchDeploymentVersionState } from '@/hooks/queries/utils/fetch-deployment-version-state' describe('deployment query helpers', () => { @@ -10,21 +10,41 @@ describe('deployment query helpers', () => { vi.clearAllMocks() }) - it('invalidates the deployment info, state, and versions queries', async () => { + it('invalidates the deployment info, state, versions, and public surface queries', async () => { const queryClient = { invalidateQueries: vi.fn().mockResolvedValue(undefined), } await invalidateDeploymentQueries(queryClient as any, 'wf-1') - expect(queryClient.invalidateQueries).toHaveBeenNthCalledWith(1, { + expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(5) + expect(queryClient.invalidateQueries.mock.calls.map(([call]) => call)).toEqual( + expect.arrayContaining([ + { queryKey: ['deployments', 'info', 'wf-1'] }, + { queryKey: ['deployments', 'deployedState', 'wf-1'] }, + { queryKey: ['deployments', 'versions', 'wf-1'] }, + { queryKey: ['deployments', 'chatStatus', 'wf-1'] }, + { queryKey: ['deployments', 'formStatus', 'wf-1'] }, + ]) + ) + }) + + it('refetches the deploy comparison boundary after invalidating it', async () => { + const queryClient = { + invalidateQueries: vi.fn().mockResolvedValue(undefined), + refetchQueries: vi.fn().mockResolvedValue(undefined), + } + + await refetchDeploymentBoundary(queryClient as any, 'wf-1') + + expect(queryClient.refetchQueries).toHaveBeenCalledWith({ queryKey: ['deployments', 'info', 'wf-1'], }) - expect(queryClient.invalidateQueries).toHaveBeenNthCalledWith(2, { + expect(queryClient.refetchQueries).toHaveBeenCalledWith({ queryKey: ['deployments', 'deployedState', 'wf-1'], }) - expect(queryClient.invalidateQueries).toHaveBeenNthCalledWith(3, { - queryKey: ['deployments', 'versions', 'wf-1'], + expect(queryClient.refetchQueries).toHaveBeenCalledWith({ + queryKey: ['workflows', 'state', 'wf-1'], }) }) diff --git a/apps/sim/hooks/queries/deployments.ts b/apps/sim/hooks/queries/deployments.ts index e7b838a3e5e..8370f394320 100644 --- a/apps/sim/hooks/queries/deployments.ts +++ b/apps/sim/hooks/queries/deployments.ts @@ -1,7 +1,7 @@ import { useCallback } from 'react' import { createLogger } from '@sim/logger' import type { QueryClient } from '@tanstack/react-query' -import { keepPreviousData, useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { requestJson, requestRaw } from '@/lib/api/client/request' import { type ActivateDeploymentVersionResponse, @@ -69,6 +69,15 @@ export function invalidateDeploymentQueries(queryClient: QueryClient, workflowId ]) } +export async function refetchDeploymentBoundary(queryClient: QueryClient, workflowId: string) { + await invalidateDeploymentQueries(queryClient, workflowId) + await Promise.all([ + queryClient.refetchQueries({ queryKey: deploymentKeys.info(workflowId) }), + queryClient.refetchQueries({ queryKey: deploymentKeys.deployedState(workflowId) }), + queryClient.refetchQueries({ queryKey: workflowKeys.state(workflowId) }), + ]) +} + export type WorkflowDeploymentInfo = DeploymentInfoResponse & { deployedAt: string | null apiKey: string | null @@ -109,7 +118,6 @@ export function useDeploymentInfo( queryFn: ({ signal }) => fetchDeploymentInfo(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, // 30 seconds - placeholderData: keepPreviousData, ...(options?.refetchOnMount !== undefined && { refetchOnMount: options.refetchOnMount }), }) } @@ -141,7 +149,6 @@ export function useDeployedWorkflowState( queryFn: ({ signal }) => fetchDeployedWorkflowState(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, - placeholderData: keepPreviousData, }) } @@ -171,7 +178,6 @@ export function useDeploymentVersions(workflowId: string | null, options?: { ena queryFn: ({ signal }) => fetchDeploymentVersions(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, // 30 seconds - placeholderData: keepPreviousData, }) } @@ -205,7 +211,6 @@ export function useChatDeploymentStatus( queryFn: ({ signal }) => fetchChatDeploymentStatus(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, // 30 seconds - placeholderData: keepPreviousData, }) } @@ -229,7 +234,6 @@ export function useChatDetail(chatId: string | null, options?: { enabled?: boole queryFn: ({ signal }) => fetchChatDetail(chatId!, signal), enabled: Boolean(chatId) && (options?.enabled ?? true), staleTime: 30 * 1000, // 30 seconds - placeholderData: keepPreviousData, }) } @@ -239,6 +243,7 @@ export function useChatDetail(chatId: string | null, options?: { enabled?: boole * Returns the combined result. */ export function useChatDeploymentInfo(workflowId: string | null, options?: { enabled?: boolean }) { + const queryClient = useQueryClient() const statusQuery = useChatDeploymentStatus(workflowId, options) const chatId = statusQuery.data?.deployment?.id ?? null @@ -249,10 +254,15 @@ export function useChatDeploymentInfo(workflowId: string | null, options?: { ena const refetch = useCallback(async () => { const statusResult = await statusQuery.refetch() - if (statusResult.data?.deployment?.id) { - await detailQuery.refetch() + const nextChatId = statusResult.data?.deployment?.id + if (nextChatId) { + await queryClient.fetchQuery({ + queryKey: deploymentKeys.chatDetail(nextChatId), + queryFn: ({ signal }) => fetchChatDetail(nextChatId, signal), + staleTime: 30 * 1000, + }) } - }, [statusQuery.refetch, detailQuery.refetch]) + }, [queryClient, statusQuery.refetch]) return { isLoading: @@ -299,15 +309,10 @@ export function useDeployWorkflow() { onSettled: (_data, error, variables) => { if (error) { logger.error('Failed to deploy workflow', { error }) - } else { - logger.info('Workflow deployed successfully', { workflowId: variables.workflowId }) + return invalidateDeploymentQueries(queryClient, variables.workflowId) } - return Promise.all([ - invalidateDeploymentQueries(queryClient, variables.workflowId), - queryClient.invalidateQueries({ - queryKey: workflowKeys.state(variables.workflowId), - }), - ]) + logger.info('Workflow deployed successfully', { workflowId: variables.workflowId }) + return refetchDeploymentBoundary(queryClient, variables.workflowId) }, }) } @@ -327,8 +332,8 @@ export function useUndeployWorkflow() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ workflowId }: UndeployWorkflowVariables): Promise => { - await requestJson(undeployWorkflowContract, { + mutationFn: async ({ workflowId }: UndeployWorkflowVariables) => { + return requestJson(undeployWorkflowContract, { params: { id: workflowId }, }) }, diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 5f3b16e56ee..3ac879f7eed 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -24,11 +24,19 @@ import { normalizeName, RESERVED_BLOCK_NAMES } from '@/executor/constants' import { invalidateDeploymentQueries } from '@/hooks/queries/deployments' import { useUndoRedo } from '@/hooks/use-undo-redo' import { useNotificationStore } from '@/stores/notifications' -import { registerEmitFunctions, useOperationQueue } from '@/stores/operation-queue/store' +import { + registerEmitFunctions, + useOperationQueue, + useOperationQueueStore, +} from '@/stores/operation-queue/store' import { usePanelEditorStore } from '@/stores/panel' import { useCodeUndoRedoStore, useUndoRedoStore } from '@/stores/undo-redo' import { useVariablesStore } from '@/stores/variables/store' import { useWorkflowDiffStore } from '@/stores/workflow-diff/store' +import { + applyWorkflowStateToStores, + WORKFLOW_DIFF_SETTLED_EVENT, +} from '@/stores/workflow-diff/utils' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { useSubBlockStore } from '@/stores/workflows/subblock/store' import { filterNewEdges, filterValidEdges, mergeSubblockState } from '@/stores/workflows/utils' @@ -154,6 +162,7 @@ export function useCollaborativeWorkflow() { // Track if we're applying remote changes to avoid infinite loops const isApplyingRemoteChange = useRef(false) + const reloadSequencesRef = useRef>({}) const { addToQueue, @@ -563,6 +572,26 @@ export function useCollaborativeWorkflow() { } const reloadWorkflowFromApi = async (workflowId: string, reason: string): Promise => { + const reloadSequence = (reloadSequencesRef.current[workflowId] ?? 0) + 1 + reloadSequencesRef.current[workflowId] = reloadSequence + const isLatestReload = () => reloadSequencesRef.current[workflowId] === reloadSequence + const pendingExternalUpdateAtStart = + useWorkflowDiffStore.getState().pendingExternalUpdates[workflowId] ?? 0 + useWorkflowDiffStore.getState().setWorkflowReconciliationInProgress(workflowId, true) + const failLatestReconciliation = (message: string) => { + if (!isLatestReload()) return + const diffStore = useWorkflowDiffStore.getState() + if ((diffStore.pendingExternalUpdates[workflowId] ?? 0) <= pendingExternalUpdateAtStart) { + diffStore.clearExternalUpdatePending(workflowId) + } + diffStore.setWorkflowReconciliationInProgress(workflowId, false) + diffStore.setWorkflowReconciliationError(workflowId, message) + if ((useWorkflowDiffStore.getState().pendingExternalUpdates[workflowId] ?? 0) > 0) { + window.dispatchEvent( + new CustomEvent(WORKFLOW_DIFF_SETTLED_EVENT, { detail: { workflowId } }) + ) + } + } // The contract's `state` is `workflowStateSchema` (loose at the wire // level — `subBlocks.value` is `unknown`, optional flags omitted), // but downstream consumers (replaceWorkflowState, the undo/redo @@ -579,41 +608,72 @@ export function useCollaborativeWorkflow() { if (wireState) { // double-cast-allowed: workflowStateSchema is structurally a supertype of the store's WorkflowState (subBlocks.value is `unknown`, optional booleans, etc.); the server persists store-shaped values so the runtime shape matches workflowState = wireState as unknown as WorkflowState + if (Object.hasOwn(responseData.data, 'variables')) { + workflowState.variables = responseData.data.variables || {} + } } } catch (error) { logger.error(`Failed to fetch workflow data after ${reason}`, { error }) + failLatestReconciliation( + 'Failed to sync the latest workflow changes. Refresh and try again.' + ) + return false + } + + if (!isLatestReload()) { + logger.debug(`Ignoring stale workflow reload after ${reason}`, { workflowId }) return false } if (!workflowState) { logger.error(`No state found in workflow data after ${reason}`, { workflowId }) + failLatestReconciliation('No workflow state was returned while syncing latest changes.') + return false + } + + if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) { + logger.debug(`Ignoring workflow reload after active workflow changed`, { workflowId }) + if (isLatestReload()) { + useWorkflowDiffStore.getState().setWorkflowReconciliationInProgress(workflowId, false) + } + return false + } + + const diffStateBeforeApply = useWorkflowDiffStore.getState() + const pendingExternalUpdateBeforeApply = + diffStateBeforeApply.pendingExternalUpdates[workflowId] ?? 0 + if ( + diffStateBeforeApply.hasActiveDiff || + pendingExternalUpdateBeforeApply > pendingExternalUpdateAtStart || + useOperationQueueStore.getState().hasPendingOperations(workflowId) + ) { + logger.info(`Deferring workflow reload apply after ${reason}`, { workflowId }) + useWorkflowDiffStore.getState().markExternalUpdatePending(workflowId) + if (isLatestReload()) { + useWorkflowDiffStore.getState().setWorkflowReconciliationInProgress(workflowId, false) + if (useWorkflowRegistry.getState().activeWorkflowId === workflowId) { + void replayPendingExternalUpdate( + workflowId, + 'deferred external update after reload apply was skipped' + ) + } + } return false } isApplyingRemoteChange.current = true try { - useWorkflowStore.getState().replaceWorkflowState({ + const stateToApply: WorkflowState = { blocks: workflowState.blocks || {}, edges: workflowState.edges || [], loops: workflowState.loops || {}, parallels: workflowState.parallels || {}, lastSaved: workflowState.lastSaved || Date.now(), - }) - - const subblockValues: Record> = {} - Object.entries(workflowState.blocks || {}).forEach(([blockId, block]) => { - subblockValues[blockId] = {} - Object.entries(block.subBlocks || {}).forEach(([subblockId, subblock]) => { - subblockValues[blockId][subblockId] = subblock?.value - }) - }) - - useSubBlockStore.setState((state) => ({ - workflowValues: { - ...state.workflowValues, - [workflowId]: subblockValues, - }, - })) + } + if (Object.hasOwn(workflowState, 'variables')) { + stateToApply.variables = workflowState.variables || {} + } + applyWorkflowStateToStores(workflowId, stateToApply) const graph = { blocksById: workflowState.blocks || {}, @@ -630,9 +690,47 @@ export function useCollaborativeWorkflow() { }) logger.info(`Successfully reloaded workflow state after ${reason}`, { workflowId }) + const diffStore = useWorkflowDiffStore.getState() + const pendingExternalUpdate = diffStore.pendingExternalUpdates[workflowId] ?? 0 + if (pendingExternalUpdate <= pendingExternalUpdateAtStart) { + diffStore.clearExternalUpdatePending(workflowId) + } + diffStore.setWorkflowReconciliationError(workflowId, null) return true } finally { isApplyingRemoteChange.current = false + if (isLatestReload()) { + useWorkflowDiffStore.getState().setWorkflowReconciliationInProgress(workflowId, false) + if (useWorkflowRegistry.getState().activeWorkflowId === workflowId) { + void replayPendingExternalUpdate( + workflowId, + 'deferred external update after reconciliation' + ) + } + } + } + } + + const replayPendingExternalUpdate = async (workflowId: string, reason: string) => { + const diffStore = useWorkflowDiffStore.getState() + if ( + useWorkflowRegistry.getState().activeWorkflowId !== workflowId || + diffStore.hasActiveDiff || + diffStore.reconcilingWorkflows[workflowId] || + !diffStore.pendingExternalUpdates[workflowId] + ) { + return + } + + const queueStore = useOperationQueueStore.getState() + if (queueStore.hasPendingOperations(workflowId)) { + return + } + + try { + await reloadWorkflowFromApi(workflowId, reason) + } catch (error) { + logger.error(`Error reloading workflow state after ${reason}:`, error) } } @@ -641,6 +739,7 @@ export function useCollaborativeWorkflow() { logger.info(`Workflow ${workflowId} has been reverted to deployed state`) if (activeWorkflowId !== workflowId) return + useWorkflowDiffStore.getState().markRemoteUpdateSeen(workflowId) try { await reloadWorkflowFromApi(workflowId, 'revert') @@ -655,12 +754,48 @@ export function useCollaborativeWorkflow() { if (activeWorkflowId !== workflowId) return - const { hasActiveDiff } = useWorkflowDiffStore.getState() + const diffStore = useWorkflowDiffStore.getState() + const { hasActiveDiff } = diffStore if (hasActiveDiff) { - logger.info('Skipping workflow-updated: active diff in progress', { workflowId }) + logger.info('Deferring workflow-updated: active diff in progress', { workflowId }) + diffStore.markExternalUpdatePending(workflowId) + return + } + + if (diffStore.reconcilingWorkflows[workflowId]) { + logger.info('Deferring workflow-updated: workflow reconciliation is in progress', { + workflowId, + }) + diffStore.markExternalUpdatePending(workflowId) + return + } + + const operationQueue = useOperationQueueStore.getState() + if (operationQueue.hasPendingOperations(workflowId)) { + logger.info('Deferring workflow-updated: local operations are still pending', { + workflowId, + }) + diffStore.markExternalUpdatePending(workflowId) + void operationQueue.waitForWorkflowOperations(workflowId).then((ready) => { + if (!ready) { + const latestQueue = useOperationQueueStore.getState() + if (latestQueue.hasPendingOperations(workflowId) && !latestQueue.hasOperationError) { + return + } + const diffStore = useWorkflowDiffStore.getState() + diffStore.clearExternalUpdatePending(workflowId) + diffStore.setWorkflowReconciliationError( + workflowId, + 'Failed to save local workflow changes before syncing external updates.' + ) + return + } + void replayPendingExternalUpdate(workflowId, 'deferred external update after local save') + }) return } + diffStore.markRemoteUpdateSeen(workflowId) try { await reloadWorkflowFromApi(workflowId, 'external update') } catch (error) { @@ -668,6 +803,16 @@ export function useCollaborativeWorkflow() { } } + const handleDiffSettled = async (event: Event) => { + const customEvent = event as CustomEvent<{ workflowId?: string }> + const workflowId = customEvent.detail?.workflowId + if (!workflowId || activeWorkflowId !== workflowId) return + const diffStore = useWorkflowDiffStore.getState() + if (!diffStore.pendingExternalUpdates[workflowId]) return + + await replayPendingExternalUpdate(workflowId, 'deferred external update') + } + const handleWorkflowDeployed = (data: any) => { const { workflowId } = data logger.info(`Workflow ${workflowId} deployment state changed`) @@ -681,6 +826,12 @@ export function useCollaborativeWorkflow() { const { operationId } = data logger.debug('Operation confirmed', { operationId }) confirmOperation(operationId) + if (activeWorkflowId) { + void replayPendingExternalUpdate( + activeWorkflowId, + 'deferred external update after operation confirm' + ) + } } const handleOperationFailed = (data: any) => { @@ -699,6 +850,18 @@ export function useCollaborativeWorkflow() { onWorkflowDeployed(handleWorkflowDeployed) onOperationConfirmed(handleOperationConfirmed) onOperationFailed(handleOperationFailed) + window.addEventListener(WORKFLOW_DIFF_SETTLED_EVENT, handleDiffSettled) + + if (activeWorkflowId) { + void replayPendingExternalUpdate( + activeWorkflowId, + 'pending external update after workflow activation' + ) + } + + return () => { + window.removeEventListener(WORKFLOW_DIFF_SETTLED_EVENT, handleDiffSettled) + } }, [ onWorkflowOperation, onSubblockUpdate, diff --git a/apps/sim/lib/api/contracts/v1/admin/workflows.ts b/apps/sim/lib/api/contracts/v1/admin/workflows.ts index 8ae9dffe35f..daed3f008f2 100644 --- a/apps/sim/lib/api/contracts/v1/admin/workflows.ts +++ b/apps/sim/lib/api/contracts/v1/admin/workflows.ts @@ -141,6 +141,7 @@ export const adminV1DeployResultSchema = z.object({ export const adminV1UndeployResultSchema = z.object({ isDeployed: z.literal(false), + warnings: z.array(z.string()).optional(), }) export const adminV1ExportWorkflowsBodySchema = z.object({ diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts new file mode 100644 index 00000000000..8ca65bbdf2c --- /dev/null +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts @@ -0,0 +1,127 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { ensureWorkflowAccessMock, setWorkflowVariablesMock, recordAuditMock } = vi.hoisted(() => ({ + ensureWorkflowAccessMock: vi.fn(), + setWorkflowVariablesMock: vi.fn(), + recordAuditMock: vi.fn(), +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { WORKFLOW_VARIABLES_UPDATED: 'WORKFLOW_VARIABLES_UPDATED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: recordAuditMock, +})) + +vi.mock('@sim/db', () => ({ + db: {}, + workflow: {}, +})) + +vi.mock('@/lib/api-key/auth', () => ({ + createWorkspaceApiKey: vi.fn(), +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: { INTERNAL_API_SECRET: 'secret' }, +})) + +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: () => 'request-1', +})) + +vi.mock('@/lib/core/utils/urls', () => ({ + getSocketServerUrl: () => 'http://socket.test', +})) + +vi.mock('@/lib/workflows/executor/execute-workflow', () => ({ + executeWorkflow: vi.fn(), +})) + +vi.mock('@/lib/workflows/executor/execution-state', () => ({ + getExecutionState: vi.fn(), + getLatestExecutionState: vi.fn(), +})) + +vi.mock('@/lib/workflows/orchestration', () => ({ + performDeleteFolder: vi.fn(), + performDeleteWorkflow: vi.fn(), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + loadWorkflowFromNormalizedTables: vi.fn(), + saveWorkflowToNormalizedTables: vi.fn(), +})) + +vi.mock('@/lib/workflows/sanitization/json-sanitizer', () => ({ + sanitizeForCopilot: vi.fn((state) => state), +})) + +vi.mock('@/lib/workflows/utils', () => ({ + checkForCircularReference: vi.fn(), + createFolderRecord: vi.fn(), + createWorkflowRecord: vi.fn(), + listFolders: vi.fn(), + setWorkflowVariables: setWorkflowVariablesMock, + updateFolderRecord: vi.fn(), + updateWorkflowRecord: vi.fn(), + verifyFolderWorkspace: vi.fn(), +})) + +vi.mock('@/executor/utils/errors', () => ({ + hasExecutionResult: vi.fn(() => false), +})) + +vi.mock('../access', () => ({ + ensureWorkflowAccess: ensureWorkflowAccessMock, + ensureWorkspaceAccess: vi.fn(), + getDefaultWorkspaceId: vi.fn(), +})) + +import { executeSetGlobalWorkflowVariables } from './mutations' + +describe('executeSetGlobalWorkflowVariables', () => { + beforeEach(() => { + vi.clearAllMocks() + global.fetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 })) as typeof fetch + ensureWorkflowAccessMock.mockResolvedValue({ + workflow: { + id: 'workflow-1', + variables: {}, + }, + }) + setWorkflowVariablesMock.mockResolvedValue(undefined) + }) + + it('persists variable changes and notifies clients that workflow state changed', async () => { + const result = await executeSetGlobalWorkflowVariables( + { + workflowId: 'workflow-1', + operations: [{ operation: 'add', name: 'threshold', type: 'number', value: '5' }], + }, + { userId: 'user-1' } as any + ) + + expect(result.success).toBe(true) + const [, variables] = setWorkflowVariablesMock.mock.calls[0] + expect(Object.values(variables)).toEqual([ + expect.objectContaining({ + workflowId: 'workflow-1', + name: 'threshold', + type: 'number', + value: 5, + }), + ]) + expect(global.fetch).toHaveBeenCalledWith('http://socket.test/api/workflow-updated', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-api-key': 'secret', + }, + body: JSON.stringify({ workflowId: 'workflow-1' }), + }) + expect(recordAuditMock).toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts index f70d5ff64bf..9973c8b4db7 100644 --- a/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts +++ b/apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts @@ -464,6 +464,7 @@ export async function executeSetGlobalWorkflowVariables( assertWorkflowMutationNotAborted(context) await setWorkflowVariables(workflowId, nextVarsRecord) + notifyWorkflowUpdated(workflowId) recordAudit({ actorId: context.userId, diff --git a/apps/sim/lib/core/async-jobs/backends/database.ts b/apps/sim/lib/core/async-jobs/backends/database.ts index 4c96bb86e94..afaa44a3641 100644 --- a/apps/sim/lib/core/async-jobs/backends/database.ts +++ b/apps/sim/lib/core/async-jobs/backends/database.ts @@ -73,20 +73,23 @@ export class DatabaseJobQueue implements JobQueueBackend { payload: TPayload, options?: EnqueueOptions ): Promise { - const jobId = `run_${generateId().replace(/-/g, '').slice(0, 20)}` + const jobId = options?.jobId ?? `run_${generateId().replace(/-/g, '').slice(0, 20)}` const now = new Date() - await db.insert(asyncJobs).values({ - id: jobId, - type, - payload: payload as Record, - status: JOB_STATUS.PENDING, - createdAt: now, - attempts: 0, - maxAttempts: options?.maxAttempts ?? 3, - metadata: (options?.metadata ?? {}) as Record, - updatedAt: now, - }) + await db + .insert(asyncJobs) + .values({ + id: jobId, + type, + payload: payload as Record, + status: JOB_STATUS.PENDING, + createdAt: now, + attempts: 0, + maxAttempts: options?.maxAttempts ?? 3, + metadata: (options?.metadata ?? {}) as Record, + updatedAt: now, + }) + .onConflictDoNothing() logger.debug('Enqueued job', { jobId, type }) if (options?.runner) { diff --git a/apps/sim/lib/core/async-jobs/backends/trigger-dev.ts b/apps/sim/lib/core/async-jobs/backends/trigger-dev.ts index e7d04bb7352..97a7428b310 100644 --- a/apps/sim/lib/core/async-jobs/backends/trigger-dev.ts +++ b/apps/sim/lib/core/async-jobs/backends/trigger-dev.ts @@ -1,5 +1,5 @@ import { createLogger } from '@sim/logger' -import { runs, tasks } from '@trigger.dev/sdk' +import { runs, type TriggerOptions, tasks } from '@trigger.dev/sdk' import { type EnqueueOptions, JOB_STATUS, @@ -73,9 +73,13 @@ export class TriggerDevJobQueue implements JobQueueBackend { : payload const tags = buildTags(options) - const triggerOptions: Parameters[2] = {} + const triggerOptions: TriggerOptions = {} if (tags.length > 0) triggerOptions.tags = tags if (options?.concurrencyKey) triggerOptions.concurrencyKey = options.concurrencyKey + if (options?.jobId) { + triggerOptions.idempotencyKey = options.jobId + triggerOptions.idempotencyKeyTTL = '14d' + } const handle = await tasks.trigger(taskId, enrichedPayload, triggerOptions) logger.debug('Enqueued job via trigger.dev', { jobId: handle.id, type, taskId, tags }) diff --git a/apps/sim/lib/core/outbox/service.test.ts b/apps/sim/lib/core/outbox/service.test.ts index 338c645ac36..48adcd0fcc5 100644 --- a/apps/sim/lib/core/outbox/service.test.ts +++ b/apps/sim/lib/core/outbox/service.test.ts @@ -58,7 +58,8 @@ const { state, mockDb } = vi.hoisted(() => { if ( row.set.status === 'completed' || row.set.status === 'dead_letter' || - (row.set.status === 'pending' && 'attempts' in row.set && 'availableAt' in row.set) + (row.set.status === 'pending' && 'attempts' in row.set && 'availableAt' in row.set) || + (!('status' in row.set) && 'attempts' in row.set && 'lockedAt' in row.set) ) { return state.leaseHeld ? [{ id: 'evt-1' }] : [] } @@ -75,7 +76,7 @@ const { state, mockDb } = vi.hoisted(() => { chain.where = vi.fn(self) chain.orderBy = vi.fn(self) chain.limit = vi.fn(self) - chain.for = vi.fn(async () => state.claimedRows) + chain.for = vi.fn(async () => state.claimedRows.splice(0, 1)) return chain } @@ -339,7 +340,7 @@ describe('processOutboxEvents — handler timeout', () => { vi.useRealTimers() }) - it('times out a stuck handler and schedules retry', async () => { + it('times out a stuck handler without releasing it for overlapping retry', async () => { const neverResolves = vi.fn(() => new Promise(() => {})) state.claimedRows = [makePendingRow({ attempts: 0 })] @@ -349,9 +350,12 @@ describe('processOutboxEvents — handler timeout', () => { await vi.advanceTimersByTimeAsync(90 * 1000 + 1) const result = await promise - expect(result.retried).toBe(1) - const retryUpdate = state.updates.find((u) => u.set.status === 'pending' && 'attempts' in u.set) - expect(retryUpdate?.set.lastError).toMatch(/timed out/) + expect(result.leaseLost).toBe(1) + const timeoutUpdate = state.updates.find( + (u) => !('status' in u.set) && 'attempts' in u.set && 'lockedAt' in u.set + ) + expect(timeoutUpdate?.set.attempts).toBe(1) + expect(timeoutUpdate?.set.lastError).toMatch(/timed out/) }) }) diff --git a/apps/sim/lib/core/outbox/service.ts b/apps/sim/lib/core/outbox/service.ts index eb087d02c8e..eaf32974800 100644 --- a/apps/sim/lib/core/outbox/service.ts +++ b/apps/sim/lib/core/outbox/service.ts @@ -18,6 +18,13 @@ const BASE_BACKOFF_MS = 1000 // 1 second, doubled per attempt // a worker is still actively processing. const DEFAULT_HANDLER_TIMEOUT_MS = 90 * 1000 // 90 seconds +class OutboxHandlerTimeoutError extends Error { + constructor(timeoutMs: number) { + super(`Outbox handler timed out after ${timeoutMs}ms`) + this.name = 'OutboxHandlerTimeoutError' + } +} + /** * Context passed to every outbox handler. Use `eventId` as the Stripe * (or any external service) idempotency key so that handler retries @@ -60,6 +67,14 @@ export interface ProcessOutboxResult { reaped: number } +export type ProcessSingleOutboxResult = + | 'completed' + | 'pending' + | 'dead_letter' + | 'lease_lost' + | 'not_found' + | 'processing' + /** * Transactional outbox for reliable "DB write + external system" flows. * @@ -104,23 +119,25 @@ export async function enqueueOutboxEvent( */ export async function processOutboxEvents( handlers: OutboxHandlerRegistry, - options: { batchSize?: number } = {} + options: { batchSize?: number; maxRuntimeMs?: number; minRemainingMs?: number } = {} ): Promise { const batchSize = options.batchSize ?? 10 + const deadline = options.maxRuntimeMs ? Date.now() + options.maxRuntimeMs : undefined + const minRemainingMs = options.minRemainingMs ?? DEFAULT_HANDLER_TIMEOUT_MS + 5000 const reaped = await reapStuckProcessingRows() - const claimed = await claimBatch(batchSize) - if (claimed.length === 0) { - return { processed: 0, retried: 0, deadLettered: 0, leaseLost: 0, reaped } - } - let processed = 0 let retried = 0 let deadLettered = 0 let leaseLost = 0 - for (const event of claimed) { + for (let i = 0; i < batchSize; i++) { + if (deadline && Date.now() + minRemainingMs > deadline) break + + const [event] = await claimBatch(1) + if (!event) break + const result = await runHandler(event, handlers) if (result === 'completed') processed++ else if (result === 'dead_letter') deadLettered++ @@ -131,6 +148,52 @@ export async function processOutboxEvents( return { processed, retried, deadLettered, leaseLost, reaped } } +/** + * Process a specific outbox event immediately after its surrounding + * transaction commits. Safe to race with the cron worker: the claim uses + * `FOR UPDATE SKIP LOCKED`, and non-pending rows are left alone. + */ +export async function processOutboxEventById( + eventId: string, + handlers: OutboxHandlerRegistry +): Promise { + const now = new Date() + const event = await db.transaction(async (tx) => { + const [row] = await tx + .select() + .from(outboxEvent) + .where(eq(outboxEvent.id, eventId)) + .limit(1) + .for('update', { skipLocked: true }) + + if (!row) return null + if (row.status !== 'pending') return row.status as ProcessSingleOutboxResult + if (row.availableAt > now) return 'pending' as const + + await tx + .update(outboxEvent) + .set({ status: 'processing', lockedAt: now }) + .where(eq(outboxEvent.id, eventId)) + + return { + ...row, + status: 'processing' as const, + lockedAt: now, + } + }) + + if (!event) { + const [current] = await db + .select({ status: outboxEvent.status }) + .from(outboxEvent) + .where(eq(outboxEvent.id, eventId)) + .limit(1) + return current ? (current.status as ProcessSingleOutboxResult) : 'not_found' + } + if (typeof event === 'string') return event + return runHandler(event, handlers) +} + /** * Reaper: move `processing` rows whose worker died (stale `lockedAt`) * back to `pending` so another worker can pick them up. Without this, @@ -249,6 +312,10 @@ async function runHandler( }) return 'completed' } catch (error) { + if (error instanceof OutboxHandlerTimeoutError) { + return recordTimedOutAttempt(event, error.message) + } + const nextAttempts = event.attempts + 1 const isDead = nextAttempts >= event.maxAttempts const errMsg = toError(error).message @@ -277,33 +344,134 @@ async function runHandler( return 'dead_letter' } - // Exponential backoff, capped at MAX_BACKOFF_MS. - const backoffMs = Math.min(MAX_BACKOFF_MS, BASE_BACKOFF_MS * 2 ** nextAttempts) - const nextAvailableAt = new Date(Date.now() + backoffMs) + return scheduleRetry(event, errMsg) + } +} + +async function recordTimedOutAttempt( + event: typeof outboxEvent.$inferSelect, + errMsg: string +): Promise<'dead_letter' | 'lease_lost'> { + const nextAttempts = event.attempts + 1 + const isDead = nextAttempts >= event.maxAttempts + + if (isDead) { + const updated = await updateIfLeaseHeld(event, { + attempts: nextAttempts, + status: 'dead_letter', + lastError: errMsg, + processedAt: new Date(), + lockedAt: null, + }) + if (!updated) return 'lease_lost' + logger.error('Outbox event dead-lettered after handler timeout max attempts', { + eventId: event.id, + eventType: event.eventType, + attempts: nextAttempts, + error: errMsg, + }) + return 'dead_letter' + } + + const updated = await updateProcessingIfLeaseHeld(event, { + attempts: nextAttempts, + lastError: errMsg, + lockedAt: new Date(), + }) + if (!updated) return 'lease_lost' + + logger.warn('Outbox event handler timed out; leaving lease for stuck-row reaper', { + eventId: event.id, + eventType: event.eventType, + attempts: nextAttempts, + reaperThresholdMs: STUCK_PROCESSING_THRESHOLD_MS, + error: errMsg, + }) + return 'lease_lost' +} + +async function scheduleRetry( + event: typeof outboxEvent.$inferSelect, + errMsg: string, + minimumBackoffMs = 0 +): Promise<'pending' | 'dead_letter' | 'lease_lost'> { + const nextAttempts = event.attempts + 1 + const isDead = nextAttempts >= event.maxAttempts + + if (isDead) { const updated = await updateIfLeaseHeld(event, { attempts: nextAttempts, - status: 'pending', + status: 'dead_letter', lastError: errMsg, - availableAt: nextAvailableAt, + processedAt: new Date(), lockedAt: null, }) if (!updated) { - logger.warn('Outbox event retry-schedule skipped — lease lost', { + logger.warn('Outbox event dead-letter skipped — lease lost', { eventId: event.id, eventType: event.eventType, }) return 'lease_lost' } - logger.warn('Outbox event failed, scheduled retry', { + logger.error('Outbox event dead-lettered after max attempts', { eventId: event.id, eventType: event.eventType, attempts: nextAttempts, - backoffMs, - nextAvailableAt: nextAvailableAt.toISOString(), error: errMsg, }) - return 'pending' + return 'dead_letter' + } + + const backoffMs = Math.max( + minimumBackoffMs, + Math.min(MAX_BACKOFF_MS, BASE_BACKOFF_MS * 2 ** nextAttempts) + ) + const nextAvailableAt = new Date(Date.now() + backoffMs) + const updated = await updateIfLeaseHeld(event, { + attempts: nextAttempts, + status: 'pending', + lastError: errMsg, + availableAt: nextAvailableAt, + lockedAt: null, + }) + if (!updated) { + logger.warn('Outbox event retry-schedule skipped — lease lost', { + eventId: event.id, + eventType: event.eventType, + }) + return 'lease_lost' + } + logger.warn('Outbox event failed, scheduled retry', { + eventId: event.id, + eventType: event.eventType, + attempts: nextAttempts, + backoffMs, + nextAvailableAt: nextAvailableAt.toISOString(), + error: errMsg, + }) + return 'pending' +} + +async function updateProcessingIfLeaseHeld( + event: typeof outboxEvent.$inferSelect, + patch: { + attempts: number + lastError: string + lockedAt: Date } +): Promise { + const whereClauses = [eq(outboxEvent.id, event.id), eq(outboxEvent.status, 'processing')] + if (event.lockedAt) { + whereClauses.push(eq(outboxEvent.lockedAt, event.lockedAt)) + } + + const result = await db + .update(outboxEvent) + .set(patch) + .where(and(...whereClauses)) + .returning({ id: outboxEvent.id }) + + return result.length > 0 } function runHandlerWithTimeout( @@ -319,7 +487,7 @@ function runHandlerWithTimeout( return new Promise((resolve, reject) => { const timeout = setTimeout(() => { - reject(new Error(`Outbox handler timed out after ${timeoutMs}ms`)) + reject(new OutboxHandlerTimeoutError(timeoutMs)) }, timeoutMs) handler(event.payload, context) diff --git a/apps/sim/lib/mcp/workflow-mcp-sync.ts b/apps/sim/lib/mcp/workflow-mcp-sync.ts index 930697bb94e..06b6e28e6a4 100644 --- a/apps/sim/lib/mcp/workflow-mcp-sync.ts +++ b/apps/sim/lib/mcp/workflow-mcp-sync.ts @@ -1,6 +1,7 @@ import { db, workflowMcpServer, workflowMcpTool } from '@sim/db' import { createLogger } from '@sim/logger' import { and, eq, inArray, isNull } from 'drizzle-orm' +import type { DbOrTx } from '@/lib/db/types' import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils' import { hasValidStartBlockInState } from '@/lib/workflows/triggers/trigger-utils' import type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -46,6 +47,9 @@ interface SyncOptions { state?: { blocks?: Record } /** Context for logging (e.g., 'deploy', 'revert', 'activate') */ context?: string + tx?: DbOrTx + notify?: boolean + throwOnError?: boolean } /** @@ -58,17 +62,27 @@ interface SyncOptions { * @param options.state - Optional workflow state (if not provided, loads from DB) * @param options.context - Optional context for log messages */ -export async function syncMcpToolsForWorkflow(options: SyncOptions): Promise { - const { workflowId, requestId, state, context = 'sync' } = options +export async function syncMcpToolsForWorkflow( + options: SyncOptions +): Promise> { + const { + workflowId, + requestId, + state, + context = 'sync', + tx = db, + notify = true, + throwOnError = false, + } = options try { - const tools = await db + const tools = await tx .select({ id: workflowMcpTool.id, serverId: workflowMcpTool.serverId }) .from(workflowMcpTool) .where(and(eq(workflowMcpTool.workflowId, workflowId), isNull(workflowMcpTool.archivedAt))) if (tools.length === 0) { - return + return [] } let workflowState: { blocks?: Record } | null = state ?? null @@ -77,19 +91,19 @@ export async function syncMcpToolsForWorkflow(options: SyncOptions): Promise { + requestId: string, + tx: DbOrTx = db, + notify = true, + throwOnError = false +): Promise> { try { - const tools = await db + const tools = await tx .select({ id: workflowMcpTool.id, serverId: workflowMcpTool.serverId }) .from(workflowMcpTool) .where(and(eq(workflowMcpTool.workflowId, workflowId), isNull(workflowMcpTool.archivedAt))) - if (tools.length === 0) return + if (tools.length === 0) return [] - await db.delete(workflowMcpTool).where(eq(workflowMcpTool.workflowId, workflowId)) + await tx.delete(workflowMcpTool).where(eq(workflowMcpTool.workflowId, workflowId)) logger.info(`[${requestId}] Removed MCP tools for workflow: ${workflowId}`) - notifyAffectedServers(tools) + if (notify) notifyMcpToolServers(tools) + return tools } catch (error) { logger.error(`[${requestId}] Error removing MCP tools:`, error) + if (throwOnError) throw error + return [] } } @@ -136,7 +159,7 @@ export async function removeMcpToolsForWorkflow( * Publish pubsub events for each unique server affected by a tool change. * Resolves workspace IDs from the server table so callers don't need to pass them. */ -function notifyAffectedServers(tools: Array<{ serverId: string }>): void { +export function notifyMcpToolServers(tools: Array<{ serverId: string }>): void { if (!mcpPubSub) return const uniqueServerIds = [...new Set(tools.map((t) => t.serverId))] diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index cfdb544f7c5..987e2ff2014 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -14,6 +14,7 @@ import { getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, count, eq, gt, gte, inArray, isNull, type SQL, sql } from 'drizzle-orm' import { generateRestoreName } from '@/lib/core/utils/restore-name' +import type { DbOrTx } from '@/lib/db/types' import { COLUMN_TYPES, NAME_PATTERN, TABLE_LIMITS, USER_TABLE_ROWS_SQL_NAME } from './constants' import { buildFilterClause, buildSortClause } from './sql' import { fireTableTrigger } from './trigger' @@ -688,13 +689,16 @@ export async function pruneStaleWorkflowGroupOutputs({ workspaceId, validBlockIds, requestId, + tx, }: { workflowId: string workspaceId: string validBlockIds: Set requestId: string + tx?: DbOrTx }): Promise { - const tables = await db + const executor = tx ?? db + const tables = await executor .select({ id: userTableDefinitions.id, schema: userTableDefinitions.schema, @@ -729,7 +733,7 @@ export async function pruneStaleWorkflowGroupOutputs({ if (!mutated) continue - await db + await executor .update(userTableDefinitions) .set({ schema: { ...schema, workflowGroups: nextGroups }, diff --git a/apps/sim/lib/webhooks/deploy.ts b/apps/sim/lib/webhooks/deploy.ts index 2a5c5e3dc0d..6f9b97b84f3 100644 --- a/apps/sim/lib/webhooks/deploy.ts +++ b/apps/sim/lib/webhooks/deploy.ts @@ -1,8 +1,8 @@ import { db } from '@sim/db' -import { webhook } from '@sim/db/schema' +import { account, credentialSetMember, webhook, workflowDeploymentVersion } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' -import { and, eq, inArray, isNull } from 'drizzle-orm' +import { and, eq, inArray, isNotNull, isNull, or } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { getProviderIdFromServiceId } from '@/lib/oauth' import { PendingWebhookVerificationTracker } from '@/lib/webhooks/pending-verification' @@ -27,13 +27,101 @@ interface TriggerSaveError { message: string status: number } - interface TriggerSaveResult { success: boolean error?: TriggerSaveError warnings?: string[] } +export async function validateTriggerWebhookConfigForDeploy( + blocks: Record +): Promise { + const triggerBlocks = Object.values(blocks || {}).filter((b) => b && b.enabled !== false) + + for (const block of triggerBlocks) { + const triggerId = resolveTriggerId(block) + if (!triggerId || !isTriggerValid(triggerId)) continue + + const triggerDef = getTrigger(triggerId) + const provider = triggerDef.provider + const { providerConfig, missingFields } = buildProviderConfig(block, triggerId, triggerDef) + + if (missingFields.length > 0) { + return { + success: false, + error: { + message: `Missing required fields for ${triggerDef.name || triggerId}: ${missingFields.join(', ')}`, + status: 400, + }, + } + } + + if (providerConfig.requireAuth && !providerConfig.token) { + return { + success: false, + error: { + message: + 'Authentication is enabled but no token is configured. Please set an authentication token or disable authentication.', + status: 400, + }, + } + } + + if (providerConfig.credentialSetId) { + const oauthProviderId = getProviderIdFromServiceId(provider) + const hasCredential = await credentialSetHasProviderCredential( + providerConfig.credentialSetId as string, + oauthProviderId + ) + if (!hasCredential) { + return { + success: false, + error: { + message: `No valid credentials found in credential set for ${provider}. Please connect accounts and try again.`, + status: 400, + }, + } + } + } + } + + return { success: true } +} + +async function credentialSetHasProviderCredential( + credentialSetId: string, + providerId: string +): Promise { + const members = await db + .select({ userId: credentialSetMember.userId }) + .from(credentialSetMember) + .where( + and( + eq(credentialSetMember.credentialSetId, credentialSetId), + eq(credentialSetMember.status, 'active') + ) + ) + + if (members.length === 0) return false + + const [credential] = await db + .select({ id: account.id }) + .from(account) + .where( + and( + inArray( + account.userId, + members.map((member) => member.userId) + ), + eq(account.providerId, providerId), + or(isNotNull(account.accessToken), isNotNull(account.refreshToken)) + ) + ) + .limit(1) + + return Boolean(credential) +} + interface CredentialSetSyncResult { error: TriggerSaveError | null warnings: string[] @@ -47,16 +135,12 @@ interface SaveTriggerWebhooksInput { blocks: Record requestId: string deploymentVersionId?: string - /** - * The previous active version's ID. Only this version's external subscriptions - * will be cleaned up (along with draft webhooks). If not provided, skips cleanup. - */ - previousVersionId?: string /** * When true, forces recreation of external subscriptions even if webhook config is unchanged. * Used when activating a previous deployment version whose subscriptions were cleaned up. */ forceRecreateSubscriptions?: boolean + strictExternalCleanup?: boolean } function getSubBlockValue(block: BlockState, subBlockId: string): unknown { @@ -371,9 +455,12 @@ export async function saveTriggerWebhooksForDeploy({ blocks, requestId, deploymentVersionId, - previousVersionId, forceRecreateSubscriptions = false, + strictExternalCleanup = false, }: SaveTriggerWebhooksInput): Promise { + const validationResult = await validateTriggerWebhookConfigForDeploy(blocks) + if (!validationResult.success) return validationResult + const triggerBlocks = Object.values(blocks || {}).filter((b) => b && b.enabled !== false) const currentBlockIds = new Set(triggerBlocks.map((b) => b.id)) @@ -392,36 +479,6 @@ export async function saveTriggerWebhooksForDeploy({ } } - if (previousVersionId) { - const webhooksToCleanup = allWorkflowWebhooks.filter( - (wh) => wh.deploymentVersionId === previousVersionId - ) - - if (webhooksToCleanup.length > 0) { - logger.info( - `[${requestId}] Cleaning up ${webhooksToCleanup.length} external subscription(s) from previous version` - ) - for (const wh of webhooksToCleanup) { - try { - await cleanupExternalWebhook(wh, workflow, requestId) - } catch (cleanupError) { - logger.warn(`[${requestId}] Failed to cleanup external webhook ${wh.id}`, cleanupError) - } - } - } - } - - const restorePreviousSubscriptions = async () => { - if (!previousVersionId) return - await restorePreviousVersionWebhooks({ - request, - workflow, - userId, - previousVersionId, - requestId, - }) - } - const webhooksByBlockId = new Map() for (const wh of existingWebhooks) { if (!wh.blockId) continue @@ -461,7 +518,6 @@ export async function saveTriggerWebhooksForDeploy({ ) if (missingFields.length > 0) { - await restorePreviousSubscriptions() return { success: false, error: { @@ -472,7 +528,6 @@ export async function saveTriggerWebhooksForDeploy({ } if (providerConfig.requireAuth && !providerConfig.token) { - await restorePreviousSubscriptions() return { success: false, error: { @@ -544,15 +599,20 @@ export async function saveTriggerWebhooksForDeploy({ }) for (const wh of webhooksToDelete) { + let cleanupSucceeded = false try { - await cleanupExternalWebhook(wh, workflow, requestId) + await cleanupExternalWebhook(wh, workflow, requestId, { + throwOnError: strictExternalCleanup, + }) + cleanupSucceeded = true } catch (cleanupError) { logger.warn(`[${requestId}] Failed to cleanup external webhook ${wh.id}`, cleanupError) + if (strictExternalCleanup) throw cleanupError + } + if (!strictExternalCleanup || cleanupSucceeded) { + await db.delete(webhook).where(eq(webhook.id, wh.id)) } } - - const idsToDelete = webhooksToDelete.map((wh) => wh.id) - await db.delete(webhook).where(inArray(webhook.id, idsToDelete)) } const collectedWarnings: string[] = [] @@ -579,12 +639,10 @@ export async function saveTriggerWebhooksForDeploy({ } if (syncResult.error) { - await restorePreviousSubscriptions() return { success: false, error: syncResult.error, warnings: collectedWarnings } } } catch (error: unknown) { logger.error(`[${requestId}] Failed to create webhook for ${block.id}`, error) - await restorePreviousSubscriptions() return { success: false, error: { @@ -648,6 +706,7 @@ export async function saveTriggerWebhooksForDeploy({ } catch (error: unknown) { logger.error(`[${requestId}] Failed to create external subscription for ${block.id}`, error) await pendingVerificationTracker.clearAll() + let cleanupFailure: unknown for (const sub of createdSubscriptions) { if (sub.externalSubscriptionCreated) { try { @@ -659,21 +718,31 @@ export async function saveTriggerWebhooksForDeploy({ providerConfig: sub.updatedProviderConfig, }, workflow, - requestId + requestId, + { throwOnError: strictExternalCleanup } ) } catch (cleanupError) { + cleanupFailure = cleanupError logger.warn( `[${requestId}] Failed to cleanup external subscription for ${sub.block.id}`, cleanupError ) + await persistCreatedWebhookRecordAfterCleanupFailure({ + workflowId, + deploymentVersionId, + sub, + requestId, + }) } } } - await restorePreviousSubscriptions() return { success: false, error: { - message: (error as Error)?.message || 'Failed to create external subscription', + message: + (cleanupFailure as Error)?.message || + (error as Error)?.message || + 'Failed to create external subscription', status: 500, }, } @@ -714,6 +783,7 @@ export async function saveTriggerWebhooksForDeploy({ `[${requestId}] Polling configuration failed for ${sub.block.id}`, pollingError ) + const cleanedWebhookIds: string[] = [] for (const otherSub of createdSubscriptions) { if (otherSub.webhookId === sub.webhookId) continue if (otherSub.externalSubscriptionCreated) { @@ -726,29 +796,30 @@ export async function saveTriggerWebhooksForDeploy({ providerConfig: otherSub.updatedProviderConfig, }, workflow, - requestId + requestId, + { throwOnError: strictExternalCleanup } ) + cleanedWebhookIds.push(otherSub.webhookId) } catch (cleanupError) { logger.warn( `[${requestId}] Failed to cleanup external subscription for ${otherSub.block.id}`, cleanupError ) } + } else { + cleanedWebhookIds.push(otherSub.webhookId) } } - const otherWebhookIds = createdSubscriptions - .filter((s) => s.webhookId !== sub.webhookId) - .map((s) => s.webhookId) - if (otherWebhookIds.length > 0) { - await db.delete(webhook).where(inArray(webhook.id, otherWebhookIds)) + if (cleanedWebhookIds.length > 0) { + await db.delete(webhook).where(inArray(webhook.id, cleanedWebhookIds)) } - await restorePreviousSubscriptions() return { success: false, error: pollingError } } } } catch (error: unknown) { await pendingVerificationTracker.clearAll() logger.error(`[${requestId}] Failed to insert webhook records`, error) + let cleanupFailure: unknown for (const sub of createdSubscriptions) { if (sub.externalSubscriptionCreated) { try { @@ -760,21 +831,31 @@ export async function saveTriggerWebhooksForDeploy({ providerConfig: sub.updatedProviderConfig, }, workflow, - requestId + requestId, + { throwOnError: strictExternalCleanup } ) } catch (cleanupError) { + cleanupFailure = cleanupError logger.warn( `[${requestId}] Failed to cleanup external subscription for ${sub.block.id}`, cleanupError ) + await persistCreatedWebhookRecordAfterCleanupFailure({ + workflowId, + deploymentVersionId, + sub, + requestId, + }) } } } - await restorePreviousSubscriptions() return { success: false, error: { - message: (error as Error)?.message || 'Failed to save webhook records', + message: + (cleanupFailure as Error)?.message || + (error as Error)?.message || + 'Failed to save webhook records', status: 500, }, } @@ -783,6 +864,45 @@ export async function saveTriggerWebhooksForDeploy({ return { success: true, warnings: collectedWarnings.length > 0 ? collectedWarnings : undefined } } +async function persistCreatedWebhookRecordAfterCleanupFailure({ + workflowId, + deploymentVersionId, + sub, + requestId, +}: { + workflowId: string + deploymentVersionId?: string + sub: { + webhookId: string + block: BlockState + provider: string + triggerPath: string + updatedProviderConfig: Record + } + requestId: string +}): Promise { + try { + await db.insert(webhook).values({ + id: sub.webhookId, + workflowId, + deploymentVersionId: deploymentVersionId || null, + blockId: sub.block.id, + path: sub.triggerPath, + provider: sub.provider, + providerConfig: sub.updatedProviderConfig, + credentialSetId: (sub.updatedProviderConfig.credentialSetId as string | undefined) || null, + isActive: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + } catch (persistError) { + logger.error( + `[${requestId}] Failed to persist webhook record after external cleanup failure`, + persistError + ) + } +} + /** * Clean up all webhooks for a workflow during undeploy. * Removes external subscriptions and deletes webhook records from the database. @@ -793,8 +913,10 @@ export async function cleanupWebhooksForWorkflow( workflowId: string, workflow: Record, requestId: string, - deploymentVersionId?: string, - skipExternalCleanup = false + deploymentVersionId?: string | null, + skipExternalCleanup = false, + strictExternalCleanup = false, + shouldDeleteWebhook?: () => Promise ): Promise { const existingWebhooks = await db .select() @@ -806,7 +928,13 @@ export async function cleanupWebhooksForWorkflow( eq(webhook.deploymentVersionId, deploymentVersionId), isNull(webhook.archivedAt) ) - : and(eq(webhook.workflowId, workflowId), isNull(webhook.archivedAt)) + : deploymentVersionId === null + ? and( + eq(webhook.workflowId, workflowId), + isNull(webhook.deploymentVersionId), + isNull(webhook.archivedAt) + ) + : and(eq(webhook.workflowId, workflowId), isNull(webhook.archivedAt)) ) if (existingWebhooks.length === 0) { @@ -824,26 +952,59 @@ export async function cleanupWebhooksForWorkflow( if (!skipExternalCleanup) { for (const wh of existingWebhooks) { + if (shouldDeleteWebhook && !(await shouldDeleteWebhook())) { + logger.info(`[${requestId}] Stopping webhook cleanup because deployment became active`, { + workflowId, + deploymentVersionId, + webhookId: wh.id, + }) + return + } + try { - await cleanupExternalWebhook(wh, workflow, requestId) + await cleanupExternalWebhook(wh, workflow, requestId, { + throwOnError: strictExternalCleanup, + }) } catch (cleanupError) { logger.warn(`[${requestId}] Failed to cleanup external webhook ${wh.id}`, cleanupError) + if (strictExternalCleanup) throw cleanupError // Continue with other webhooks even if one fails } + + const deleted = await deleteWebhookRecordAfterCleanup({ + workflowId, + deploymentVersionId, + webhookId: wh.id, + shouldDeleteWebhook, + }) + if (!deleted) { + logger.info(`[${requestId}] Stopping webhook DB cleanup because deployment became active`, { + workflowId, + deploymentVersionId, + webhookId: wh.id, + }) + return + } + } + } else { + for (const wh of existingWebhooks) { + const deleted = await deleteWebhookRecordAfterCleanup({ + workflowId, + deploymentVersionId, + webhookId: wh.id, + shouldDeleteWebhook, + }) + if (!deleted) { + logger.info(`[${requestId}] Stopping webhook DB cleanup because deployment became active`, { + workflowId, + deploymentVersionId, + webhookId: wh.id, + }) + return + } } } - await db - .delete(webhook) - .where( - deploymentVersionId - ? and( - eq(webhook.workflowId, workflowId), - eq(webhook.deploymentVersionId, deploymentVersionId) - ) - : eq(webhook.workflowId, workflowId) - ) - logger.info( deploymentVersionId ? `[${requestId}] Cleaned up webhooks for workflow ${workflowId} deployment ${deploymentVersionId}` @@ -851,60 +1012,50 @@ export async function cleanupWebhooksForWorkflow( ) } -/** - * Restore external subscriptions for a previous deployment version. - * Used when activation/deployment fails after webhooks were created, - * to restore the previous version's external subscriptions. - */ -export async function restorePreviousVersionWebhooks(params: { - request: NextRequest - workflow: Record - userId: string - previousVersionId: string - requestId: string -}): Promise { - const { request, workflow, userId, previousVersionId, requestId } = params - - const previousWebhooks = await db - .select() - .from(webhook) - .where(and(eq(webhook.deploymentVersionId, previousVersionId), isNull(webhook.archivedAt))) - - if (previousWebhooks.length === 0) { - return +async function deleteWebhookRecordAfterCleanup(params: { + workflowId: string + deploymentVersionId?: string | null + webhookId: string + shouldDeleteWebhook?: () => Promise +}): Promise { + if (params.shouldDeleteWebhook && !(await params.shouldDeleteWebhook())) { + return false } - logger.info( - `[${requestId}] Restoring ${previousWebhooks.length} external subscription(s) for previous version ${previousVersionId}` - ) + if (!params.shouldDeleteWebhook || typeof params.deploymentVersionId !== 'string') { + await db + .delete(webhook) + .where(and(eq(webhook.workflowId, params.workflowId), eq(webhook.id, params.webhookId))) + return true + } - for (const wh of previousWebhooks) { - try { - const result = await createExternalWebhookSubscription( - request, - { - id: wh.id, - path: wh.path, - provider: wh.provider, - providerConfig: (wh.providerConfig as Record) || {}, - }, - workflow, - userId, - requestId + const deploymentVersionId = params.deploymentVersionId + + return db.transaction(async (tx) => { + const [inactiveVersion] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + eq(workflowDeploymentVersion.id, deploymentVersionId), + eq(workflowDeploymentVersion.isActive, false) + ) ) - await db - .update(webhook) - .set({ - providerConfig: result.updatedProviderConfig, - updatedAt: new Date(), - }) - .where(eq(webhook.id, wh.id)) - logger.info(`[${requestId}] Restored external subscription for webhook ${wh.id}`) - } catch (restoreError) { - logger.error( - `[${requestId}] Failed to restore external subscription for webhook ${wh.id}`, - restoreError + .limit(1) + .for('update') + + if (!inactiveVersion) return false + + await tx + .delete(webhook) + .where( + and( + eq(webhook.workflowId, params.workflowId), + eq(webhook.id, params.webhookId), + eq(webhook.deploymentVersionId, deploymentVersionId) + ) ) - } - } + return true + }) } diff --git a/apps/sim/lib/webhooks/provider-subscriptions.ts b/apps/sim/lib/webhooks/provider-subscriptions.ts index e2e8eceebac..05c3eab64c6 100644 --- a/apps/sim/lib/webhooks/provider-subscriptions.ts +++ b/apps/sim/lib/webhooks/provider-subscriptions.ts @@ -119,12 +119,14 @@ export async function createExternalWebhookSubscription( /** * Clean up external webhook subscriptions for a webhook. - * Errors are swallowed — cleanup failure should not block webhook deletion. + * By default, cleanup failure is logged but non-fatal for legacy best-effort callers. + * Deployment outbox cleanup passes `throwOnError` so provider failures stay retryable. */ export async function cleanupExternalWebhook( webhook: Record, workflow: Record, - requestId: string + requestId: string, + options: { throwOnError?: boolean } = {} ): Promise { const provider = webhook.provider as string const handler = getProviderHandler(provider) @@ -134,12 +136,15 @@ export async function cleanupExternalWebhook( } try { - await handler.deleteSubscription({ webhook, workflow, requestId }) + await handler.deleteSubscription({ webhook, workflow, requestId, strict: options.throwOnError }) } catch (error) { logger.warn(`[${requestId}] Error cleaning up external webhook (non-fatal)`, { provider, webhookId: webhook.id, error: toError(error).message, }) + if (options.throwOnError) { + throw error + } } } diff --git a/apps/sim/lib/webhooks/providers/airtable.ts b/apps/sim/lib/webhooks/providers/airtable.ts index 61daafa4f30..fd0463b4b95 100644 --- a/apps/sim/lib/webhooks/providers/airtable.ts +++ b/apps/sim/lib/webhooks/providers/airtable.ts @@ -572,6 +572,7 @@ export const airtableHandler: WebhookProviderHandler = { webhook: webhookRecord, workflow, requestId, + strict, }: DeleteSubscriptionContext): Promise { try { const config = getProviderConfig(webhookRecord) @@ -584,6 +585,7 @@ export const airtableHandler: WebhookProviderHandler = { logger.warn(`[${requestId}] Missing baseId for Airtable webhook deletion`, { webhookId: webhookRecord.id, }) + if (strict) throw new Error('Missing Airtable baseId for webhook deletion') return } @@ -593,6 +595,7 @@ export const airtableHandler: WebhookProviderHandler = { webhookId: webhookRecord.id, baseId: baseId.substring(0, 20), }) + if (strict) throw new Error('Invalid Airtable baseId for webhook deletion') return } @@ -601,6 +604,7 @@ export const airtableHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing credentialId for Airtable webhook deletion ${webhookRecord.id}` ) + if (strict) throw new Error('Missing Airtable credentialId for webhook deletion') return } @@ -613,14 +617,22 @@ export const airtableHandler: WebhookProviderHandler = { ) : null if (!accessToken) { - logger.warn( - `[${requestId}] Could not retrieve Airtable access token. Cannot delete webhook in Airtable.`, - { webhookId: webhookRecord.id } - ) + const message = `[${requestId}] Could not retrieve Airtable access token. Cannot delete webhook in Airtable.` + logger.warn(message, { webhookId: webhookRecord.id }) + if (strict) throw new Error(message) return } let resolvedExternalId: string | undefined = externalId + let externalIdLookupFailed = false + + if (!resolvedExternalId && strict) { + logger.warn( + `[${requestId}] Missing Airtable externalId during strict cleanup; skipping unsafe URL-based remote deletion`, + { webhookId: webhookRecord.id, baseId } + ) + throw new Error('Missing Airtable externalId for strict cleanup') + } if (!resolvedExternalId) { try { @@ -656,6 +668,7 @@ export const airtableHandler: WebhookProviderHandler = { }) } } else { + externalIdLookupFailed = true logger.warn(`[${requestId}] Failed to list Airtable webhooks to resolve externalId`, { baseId, status: listResp.status, @@ -663,6 +676,7 @@ export const airtableHandler: WebhookProviderHandler = { }) } } catch (e: unknown) { + externalIdLookupFailed = true logger.warn(`[${requestId}] Error attempting to resolve Airtable externalId`, { error: (e as Error)?.message, }) @@ -672,7 +686,11 @@ export const airtableHandler: WebhookProviderHandler = { if (!resolvedExternalId) { logger.info(`[${requestId}] Airtable externalId not found; skipping remote deletion`, { baseId, + confirmedAbsent: !externalIdLookupFailed, }) + if (strict && externalIdLookupFailed) { + throw new Error('Could not resolve Airtable externalId for strict cleanup') + } return } @@ -682,6 +700,7 @@ export const airtableHandler: WebhookProviderHandler = { webhookId: webhookRecord.id, externalId: resolvedExternalId.substring(0, 20), }) + if (strict) throw new Error('Invalid Airtable webhook ID for deletion') return } @@ -693,22 +712,22 @@ export const airtableHandler: WebhookProviderHandler = { }, }) - if (!airtableResponse.ok) { + if (!airtableResponse.ok && airtableResponse.status !== 404) { let responseBody: unknown = null try { responseBody = await airtableResponse.json() - } catch { - // Ignore parse errors - } + } catch {} logger.warn( `[${requestId}] Failed to delete Airtable webhook in Airtable. Status: ${airtableResponse.status}`, { baseId, externalId: resolvedExternalId, response: responseBody } ) + if (strict) throw new Error(`Failed to delete Airtable webhook: ${airtableResponse.status}`) } else { logger.info(`[${requestId}] Successfully deleted Airtable webhook in Airtable`, { baseId, externalId: resolvedExternalId, + alreadyDeleted: airtableResponse.status === 404, }) } } catch (error: unknown) { @@ -718,6 +737,7 @@ export const airtableHandler: WebhookProviderHandler = { error: err.message, stack: err.stack, }) + if (strict) throw error } }, diff --git a/apps/sim/lib/webhooks/providers/ashby.ts b/apps/sim/lib/webhooks/providers/ashby.ts index f75fa58b00a..77fabe6e024 100644 --- a/apps/sim/lib/webhooks/providers/ashby.ts +++ b/apps/sim/lib/webhooks/providers/ashby.ts @@ -225,6 +225,7 @@ export const ashbyHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing apiKey for Ashby webhook deletion ${ctx.webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Ashby apiKey for webhook deletion') return } @@ -232,6 +233,7 @@ export const ashbyHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing externalId for Ashby webhook deletion ${ctx.webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Ashby externalId for webhook deletion') return } @@ -262,9 +264,11 @@ export const ashbyHandler: WebhookProviderHandler = { `[${ctx.requestId}] Failed to delete Ashby webhook (non-fatal): ${ashbyResponse.status}`, { response: responseBody } ) + if (ctx.strict) throw new Error(`Failed to delete Ashby webhook: ${ashbyResponse.status}`) } } catch (error) { logger.warn(`[${ctx.requestId}] Error deleting Ashby webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/attio.ts b/apps/sim/lib/webhooks/providers/attio.ts index ea9debff8f5..1022e885d8f 100644 --- a/apps/sim/lib/webhooks/providers/attio.ts +++ b/apps/sim/lib/webhooks/providers/attio.ts @@ -251,6 +251,7 @@ export const attioHandler: WebhookProviderHandler = { webhook: webhookRecord, workflow, requestId, + strict, }: DeleteSubscriptionContext): Promise { try { const config = getProviderConfig(webhookRecord) @@ -261,6 +262,7 @@ export const attioHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing externalId for Attio webhook deletion ${webhookRecord.id}, skipping cleanup` ) + if (strict) throw new Error('Missing Attio externalId for webhook deletion') return } @@ -268,6 +270,7 @@ export const attioHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing credentialId for Attio webhook deletion ${webhookRecord.id}, skipping cleanup` ) + if (strict) throw new Error('Missing Attio credentialId for webhook deletion') return } @@ -281,10 +284,9 @@ export const attioHandler: WebhookProviderHandler = { : null if (!accessToken) { - logger.warn( - `[${requestId}] Could not retrieve Attio access token. Cannot delete webhook.`, - { webhookId: webhookRecord.id } - ) + const message = `[${requestId}] Could not retrieve Attio access token. Cannot delete webhook.` + logger.warn(message, { webhookId: webhookRecord.id }) + if (strict) throw new Error(message) return } @@ -301,11 +303,13 @@ export const attioHandler: WebhookProviderHandler = { `[${requestId}] Failed to delete Attio webhook (non-fatal): ${attioResponse.status}`, { response: responseBody } ) + if (strict) throw new Error(`Failed to delete Attio webhook: ${attioResponse.status}`) } else { logger.info(`[${requestId}] Successfully deleted Attio webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Attio webhook (non-fatal)`, error) + if (strict) throw error } }, diff --git a/apps/sim/lib/webhooks/providers/calendly.ts b/apps/sim/lib/webhooks/providers/calendly.ts index a85b108c5bf..d574d6e9534 100644 --- a/apps/sim/lib/webhooks/providers/calendly.ts +++ b/apps/sim/lib/webhooks/providers/calendly.ts @@ -174,6 +174,7 @@ export const calendlyHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing apiKey for Calendly webhook deletion ${ctx.webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Calendly apiKey for webhook deletion') return } @@ -181,6 +182,7 @@ export const calendlyHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing externalId for Calendly webhook deletion ${ctx.webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Calendly externalId for webhook deletion') return } @@ -199,6 +201,9 @@ export const calendlyHandler: WebhookProviderHandler = { `[${ctx.requestId}] Failed to delete Calendly webhook (non-fatal): ${calendlyResponse.status}`, { response: responseBody } ) + if (ctx.strict) { + throw new Error(`Failed to delete Calendly webhook: ${calendlyResponse.status}`) + } } else { logger.info( `[${ctx.requestId}] Successfully deleted Calendly webhook subscription ${externalId}` @@ -206,6 +211,7 @@ export const calendlyHandler: WebhookProviderHandler = { } } catch (error) { logger.warn(`[${ctx.requestId}] Error deleting Calendly webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/emailbison.ts b/apps/sim/lib/webhooks/providers/emailbison.ts index 116e9f7337c..1601cb30eb5 100644 --- a/apps/sim/lib/webhooks/providers/emailbison.ts +++ b/apps/sim/lib/webhooks/providers/emailbison.ts @@ -206,6 +206,7 @@ export const emailBisonHandler: WebhookProviderHandler = { hasApiBaseUrl: Boolean(apiBaseUrl), hasExternalId: Boolean(externalId), }) + if (ctx.strict) throw new Error('Missing Email Bison webhook cleanup configuration') return } @@ -223,6 +224,7 @@ export const emailBisonHandler: WebhookProviderHandler = { status: response.status, response: responseBody, }) + if (ctx.strict) throw new Error(`Failed to delete Email Bison webhook: ${response.status}`) return } @@ -235,6 +237,7 @@ export const emailBisonHandler: WebhookProviderHandler = { logger.warn(`[${requestId}] Error deleting Email Bison webhook`, { message: toError(error).message, }) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/fathom.ts b/apps/sim/lib/webhooks/providers/fathom.ts index c158c73e369..341f50c4592 100644 --- a/apps/sim/lib/webhooks/providers/fathom.ts +++ b/apps/sim/lib/webhooks/providers/fathom.ts @@ -131,6 +131,7 @@ export const fathomHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing apiKey for Fathom webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Fathom apiKey for webhook deletion') return } @@ -138,6 +139,7 @@ export const fathomHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing externalId for Fathom webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Fathom externalId for webhook deletion') return } @@ -146,6 +148,7 @@ export const fathomHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Invalid externalId format for Fathom webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Invalid Fathom externalId for webhook deletion') return } @@ -163,11 +166,13 @@ export const fathomHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Failed to delete Fathom webhook (non-fatal): ${fathomResponse.status}` ) + if (ctx.strict) throw new Error(`Failed to delete Fathom webhook: ${fathomResponse.status}`) } else { logger.info(`[${requestId}] Successfully deleted Fathom webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Fathom webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/grain.ts b/apps/sim/lib/webhooks/providers/grain.ts index 39be11cab66..8b32fb17db9 100644 --- a/apps/sim/lib/webhooks/providers/grain.ts +++ b/apps/sim/lib/webhooks/providers/grain.ts @@ -215,6 +215,7 @@ export const grainHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing apiKey for Grain webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Grain apiKey for webhook deletion') return } @@ -222,6 +223,7 @@ export const grainHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing externalId for Grain webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Grain externalId for webhook deletion') return } @@ -241,11 +243,13 @@ export const grainHandler: WebhookProviderHandler = { `[${requestId}] Failed to delete Grain webhook (non-fatal): ${grainResponse.status}`, { response: responseBody } ) + if (ctx.strict) throw new Error(`Failed to delete Grain webhook: ${grainResponse.status}`) } else { logger.info(`[${requestId}] Successfully deleted Grain webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Grain webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/lemlist.ts b/apps/sim/lib/webhooks/providers/lemlist.ts index 2215839b8de..24759354a36 100644 --- a/apps/sim/lib/webhooks/providers/lemlist.ts +++ b/apps/sim/lib/webhooks/providers/lemlist.ts @@ -131,6 +131,7 @@ export const lemlistHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing apiKey for Lemlist webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Lemlist apiKey for webhook deletion') return } @@ -142,6 +143,7 @@ export const lemlistHandler: WebhookProviderHandler = { logger.warn(`[${requestId}] Invalid Lemlist hook ID format, skipping deletion`, { id: id.substring(0, 30), }) + if (ctx.strict) throw new Error('Invalid Lemlist hook ID for deletion') return } @@ -159,6 +161,9 @@ export const lemlistHandler: WebhookProviderHandler = { `[${requestId}] Failed to delete Lemlist webhook (non-fatal): ${lemlistResponse.status}`, { response: responseBody } ) + if (ctx.strict) { + throw new Error(`Failed to delete Lemlist webhook: ${lemlistResponse.status}`) + } } else { logger.info(`[${requestId}] Successfully deleted Lemlist webhook ${id}`) } @@ -169,6 +174,14 @@ export const lemlistHandler: WebhookProviderHandler = { return } + if (ctx.strict) { + logger.warn( + `[${requestId}] Missing Lemlist externalId during strict cleanup; skipping unsafe URL-based remote deletion`, + { webhookId: webhook.id } + ) + throw new Error('Missing Lemlist externalId for strict cleanup') + } + const notificationUrl = getNotificationUrl(webhook) const listResponse = await fetch('https://api.lemlist.com/api/hooks', { method: 'GET', @@ -181,6 +194,7 @@ export const lemlistHandler: WebhookProviderHandler = { logger.warn(`[${requestId}] Failed to list Lemlist webhooks for cleanup ${webhook.id}`, { status: listResponse.status, }) + if (ctx.strict) throw new Error(`Failed to list Lemlist webhooks: ${listResponse.status}`) return } @@ -213,6 +227,7 @@ export const lemlistHandler: WebhookProviderHandler = { } } catch (error) { logger.warn(`[${requestId}] Error deleting Lemlist webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/linear.ts b/apps/sim/lib/webhooks/providers/linear.ts index 60fb176c28b..4402659f71d 100644 --- a/apps/sim/lib/webhooks/providers/linear.ts +++ b/apps/sim/lib/webhooks/providers/linear.ts @@ -249,10 +249,16 @@ export const linearHandler: WebhookProviderHandler = { async deleteSubscription(ctx: DeleteSubscriptionContext): Promise { const config = getProviderConfig(ctx.webhook) + const triggerId = config.triggerId as string | undefined + if (!triggerId || !triggerId.endsWith('_v2')) { + return + } + const externalId = config.externalId as string | undefined const apiKey = config.apiKey as string | undefined if (!externalId || !apiKey) { + if (ctx.strict) throw new Error('Missing Linear webhook deletion credentials') return } @@ -275,6 +281,7 @@ export const linearHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Linear API returned HTTP ${response.status} during webhook deletion for ${externalId}` ) + if (ctx.strict) throw new Error(`Linear webhook deletion failed: ${response.status}`) return } @@ -284,14 +291,48 @@ export const linearHandler: WebhookProviderHandler = { `[${ctx.requestId}] Deleted Linear webhook ${externalId} for webhook ${ctx.webhook.id}` ) } else { + const errorMessages = getGraphQLErrorMessages(data) + if (errorMessages.some(isAlreadyAbsentWebhookMessage)) { + logger.info( + `[${ctx.requestId}] Linear webhook ${externalId} was already absent during deletion` + ) + return + } + logger.warn( `[${ctx.requestId}] Linear webhook deletion returned unsuccessful for ${externalId}` ) + if (ctx.strict) throw new Error('Linear webhook deletion returned unsuccessful') } } catch (error) { logger.warn(`[${ctx.requestId}] Error deleting Linear webhook ${externalId} (non-fatal)`, { error: toError(error).message, }) + if (ctx.strict) throw error } }, } + +function getGraphQLErrorMessages(data: unknown): string[] { + if (!data || typeof data !== 'object' || Array.isArray(data)) return [] + const errors = (data as Record).errors + if (!Array.isArray(errors)) return [] + + return errors + .map((error) => { + if (!error || typeof error !== 'object' || Array.isArray(error)) return null + const message = (error as Record).message + return typeof message === 'string' ? message : null + }) + .filter((message): message is string => Boolean(message)) +} + +function isAlreadyAbsentWebhookMessage(message: string): boolean { + const normalized = message.toLowerCase() + return ( + normalized.includes('not found') || + normalized.includes('not_found') || + normalized.includes('does not exist') || + normalized.includes('already deleted') + ) +} diff --git a/apps/sim/lib/webhooks/providers/microsoft-teams.ts b/apps/sim/lib/webhooks/providers/microsoft-teams.ts index 887747c242e..be749f4e990 100644 --- a/apps/sim/lib/webhooks/providers/microsoft-teams.ts +++ b/apps/sim/lib/webhooks/providers/microsoft-teams.ts @@ -675,6 +675,7 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { webhook, workflow, requestId, + strict, }: DeleteSubscriptionContext): Promise { try { const config = getProviderConfig(webhook) @@ -688,6 +689,7 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { if (!externalSubscriptionId || !credentialId) { logger.info(`[${requestId}] No external subscription to delete for webhook ${webhook.id}`) + if (strict) throw new Error('Missing Teams subscription cleanup configuration') return } @@ -703,6 +705,7 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Could not get access token to delete Teams subscription for webhook ${webhook.id}` ) + if (strict) throw new Error('Missing Teams access token for subscription deletion') return } @@ -723,12 +726,14 @@ export const microsoftTeamsHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Failed to delete Teams subscription ${externalSubscriptionId} for webhook ${webhook.id}. Status: ${res.status}` ) + if (strict) throw new Error(`Failed to delete Teams subscription: ${res.status}`) } } catch (error) { logger.error( `[${requestId}] Error deleting Teams subscription for webhook ${webhook.id}`, error ) + if (strict) throw error } }, diff --git a/apps/sim/lib/webhooks/providers/monday.ts b/apps/sim/lib/webhooks/providers/monday.ts index da7f872c6fa..87c1f49994a 100644 --- a/apps/sim/lib/webhooks/providers/monday.ts +++ b/apps/sim/lib/webhooks/providers/monday.ts @@ -181,6 +181,7 @@ export const mondayHandler: WebhookProviderHandler = { const externalId = config.externalId as string | undefined if (!externalId) { + if (ctx.strict) throw new Error('Missing Monday externalId for webhook deletion') return } @@ -189,6 +190,7 @@ export const mondayHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Invalid externalId format for Monday webhook deletion: ${externalId}` ) + if (ctx.strict) throw new Error('Invalid Monday externalId for webhook deletion') return } @@ -210,12 +212,14 @@ export const mondayHandler: WebhookProviderHandler = { `[${ctx.requestId}] Could not resolve credentials for Monday webhook deletion (non-fatal)`, { error: toError(error).message } ) + if (ctx.strict) throw error } if (!accessToken) { logger.warn( `[${ctx.requestId}] No access token available for Monday webhook deletion ${externalId} (non-fatal)` ) + if (ctx.strict) throw new Error('Missing Monday access token for webhook deletion') return } @@ -236,6 +240,7 @@ export const mondayHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Monday API returned HTTP ${response.status} during webhook deletion for ${externalId}` ) + if (ctx.strict) throw new Error(`Monday webhook deletion failed: ${response.status}`) return } @@ -246,9 +251,16 @@ export const mondayHandler: WebhookProviderHandler = { data.errors?.map((e: { message: string }) => e.message).join(', ') || data.error_message || 'Unknown error' + if (isAlreadyAbsentWebhookMessage(errorMsg)) { + logger.info( + `[${ctx.requestId}] Monday webhook ${externalId} was already absent during deletion` + ) + return + } logger.warn( `[${ctx.requestId}] Monday webhook deletion GraphQL error for ${externalId}: ${errorMsg}` ) + if (ctx.strict) throw new Error(`Monday webhook deletion failed: ${errorMsg}`) return } @@ -258,11 +270,13 @@ export const mondayHandler: WebhookProviderHandler = { ) } else { logger.warn(`[${ctx.requestId}] Monday webhook deletion returned no data for ${externalId}`) + if (ctx.strict) throw new Error('Monday webhook deletion returned no data') } } catch (error) { logger.warn(`[${ctx.requestId}] Error deleting Monday webhook ${externalId} (non-fatal)`, { error: toError(error).message, }) + if (ctx.strict) throw error } }, @@ -331,3 +345,13 @@ export const mondayHandler: WebhookProviderHandler = { return null }, } + +function isAlreadyAbsentWebhookMessage(message: string): boolean { + const normalized = message.toLowerCase() + return ( + normalized.includes('not found') || + normalized.includes('not_found') || + normalized.includes('does not exist') || + normalized.includes('already deleted') + ) +} diff --git a/apps/sim/lib/webhooks/providers/resend.ts b/apps/sim/lib/webhooks/providers/resend.ts index f0da305381e..c04289e3a00 100644 --- a/apps/sim/lib/webhooks/providers/resend.ts +++ b/apps/sim/lib/webhooks/providers/resend.ts @@ -270,6 +270,7 @@ export const resendHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing apiKey or externalId for Resend webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Resend webhook deletion credentials') return } @@ -286,11 +287,13 @@ export const resendHandler: WebhookProviderHandler = { `[${requestId}] Failed to delete Resend webhook (non-fatal): ${resendResponse.status}`, { response: responseBody } ) + if (ctx.strict) throw new Error(`Failed to delete Resend webhook: ${resendResponse.status}`) } else { logger.info(`[${requestId}] Successfully deleted Resend webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Resend webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/telegram.ts b/apps/sim/lib/webhooks/providers/telegram.ts index 77bb623be16..9720bccd60b 100644 --- a/apps/sim/lib/webhooks/providers/telegram.ts +++ b/apps/sim/lib/webhooks/providers/telegram.ts @@ -1,4 +1,6 @@ +import { db, webhook, workflowDeploymentVersion } from '@sim/db' import { createLogger } from '@sim/logger' +import { and, eq, isNull, ne } from 'drizzle-orm' import { getNotificationUrl, getProviderConfig } from '@/lib/webhooks/provider-subscription-utils' import type { AuthContext, @@ -184,6 +186,15 @@ export const telegramHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing botToken for Telegram webhook deletion ${ctx.webhook.id}` ) + if (ctx.strict) throw new Error('Missing Telegram botToken for webhook deletion') + return + } + + if (await activeTelegramWebhookUsesBot(ctx.webhook, botToken)) { + logger.info( + `[${ctx.requestId}] Skipping Telegram webhook deletion because an active deployment uses the same bot token`, + { webhookId: ctx.webhook.id } + ) return } @@ -199,6 +210,7 @@ export const telegramHandler: WebhookProviderHandler = { responseBody.description || `Failed to delete Telegram webhook. Status: ${telegramResponse.status}` logger.error(`[${ctx.requestId}] ${errorMessage}`, { response: responseBody }) + if (ctx.strict) throw new Error(errorMessage) } else { logger.info( `[${ctx.requestId}] Successfully deleted Telegram webhook for webhook ${ctx.webhook.id}` @@ -209,6 +221,39 @@ export const telegramHandler: WebhookProviderHandler = { `[${ctx.requestId}] Error deleting Telegram webhook for webhook ${ctx.webhook.id}`, error ) + if (ctx.strict) throw error } }, } + +async function activeTelegramWebhookUsesBot( + webhookRecord: Record, + botToken: string +): Promise { + const workflowId = webhookRecord.workflowId + const webhookId = webhookRecord.id + if (typeof workflowId !== 'string' || typeof webhookId !== 'string') return false + + const activeWebhooks = await db + .select({ id: webhook.id, providerConfig: webhook.providerConfig }) + .from(webhook) + .innerJoin( + workflowDeploymentVersion, + eq(webhook.deploymentVersionId, workflowDeploymentVersion.id) + ) + .where( + and( + eq(webhook.workflowId, workflowId), + ne(webhook.id, webhookId), + eq(webhook.provider, 'telegram'), + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.isActive, true), + isNull(webhook.archivedAt) + ) + ) + + return activeWebhooks.some((activeWebhook) => { + const config = getProviderConfig({ providerConfig: activeWebhook.providerConfig }) + return config.botToken === botToken + }) +} diff --git a/apps/sim/lib/webhooks/providers/typeform.ts b/apps/sim/lib/webhooks/providers/typeform.ts index 16df0e6c47d..e8a7384009b 100644 --- a/apps/sim/lib/webhooks/providers/typeform.ts +++ b/apps/sim/lib/webhooks/providers/typeform.ts @@ -186,6 +186,7 @@ export const typeformHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Missing formId or apiKey for Typeform webhook deletion ${ctx.webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Typeform webhook deletion credentials') return } @@ -203,11 +204,15 @@ export const typeformHandler: WebhookProviderHandler = { logger.warn( `[${ctx.requestId}] Failed to delete Typeform webhook (non-fatal): ${typeformResponse.status}` ) + if (ctx.strict) { + throw new Error(`Failed to delete Typeform webhook: ${typeformResponse.status}`) + } } else { logger.info(`[${ctx.requestId}] Successfully deleted Typeform webhook with tag ${tag}`) } } catch (error) { logger.warn(`[${ctx.requestId}] Error deleting Typeform webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, } diff --git a/apps/sim/lib/webhooks/providers/types.ts b/apps/sim/lib/webhooks/providers/types.ts index dee3e8aca19..d25037117d9 100644 --- a/apps/sim/lib/webhooks/providers/types.ts +++ b/apps/sim/lib/webhooks/providers/types.ts @@ -75,6 +75,7 @@ export interface DeleteSubscriptionContext { webhook: Record workflow: Record requestId: string + strict?: boolean } /** Context for configuring polling after webhook creation. */ @@ -127,7 +128,7 @@ export interface WebhookProviderHandler { /** Create an external webhook subscription (e.g., register with Telegram, Airtable, etc.). */ createSubscription?(ctx: SubscriptionContext): Promise - /** Delete an external webhook subscription during cleanup. Errors should not throw. */ + /** Delete an external webhook subscription during cleanup. Strict outbox cleanup should throw. */ deleteSubscription?(ctx: DeleteSubscriptionContext): Promise /** Configure polling after webhook creation (gmail, outlook, rss, imap). */ diff --git a/apps/sim/lib/webhooks/providers/vercel.ts b/apps/sim/lib/webhooks/providers/vercel.ts index edf5f9d6220..099931c4ad4 100644 --- a/apps/sim/lib/webhooks/providers/vercel.ts +++ b/apps/sim/lib/webhooks/providers/vercel.ts @@ -219,6 +219,7 @@ export const vercelHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing apiKey or externalId for Vercel webhook deletion ${webhook.id}, skipping cleanup` ) + if (ctx.strict) throw new Error('Missing Vercel webhook deletion credentials') return } @@ -237,12 +238,14 @@ export const vercelHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Failed to delete Vercel webhook (non-fatal): ${response.status}` ) + if (ctx.strict) throw new Error(`Failed to delete Vercel webhook: ${response.status}`) } else { await response.body?.cancel() logger.info(`[${requestId}] Successfully deleted Vercel webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Vercel webhook (non-fatal)`, error) + if (ctx.strict) throw error } }, diff --git a/apps/sim/lib/webhooks/providers/webflow.ts b/apps/sim/lib/webhooks/providers/webflow.ts index 9399bcd54e3..7494ae39568 100644 --- a/apps/sim/lib/webhooks/providers/webflow.ts +++ b/apps/sim/lib/webhooks/providers/webflow.ts @@ -150,6 +150,7 @@ export const webflowHandler: WebhookProviderHandler = { webhook: webhookRecord, workflow, requestId, + strict, }: DeleteSubscriptionContext): Promise { try { const config = getProviderConfig(webhookRecord) @@ -160,6 +161,7 @@ export const webflowHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing siteId for Webflow webhook deletion ${webhookRecord.id}, skipping cleanup` ) + if (strict) throw new Error('Missing Webflow siteId for webhook deletion') return } @@ -167,6 +169,7 @@ export const webflowHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing externalId for Webflow webhook deletion ${webhookRecord.id}, skipping cleanup` ) + if (strict) throw new Error('Missing Webflow externalId for webhook deletion') return } @@ -176,6 +179,7 @@ export const webflowHandler: WebhookProviderHandler = { webhookId: webhookRecord.id, siteId: siteId.substring(0, 30), }) + if (strict) throw new Error('Invalid Webflow siteId for webhook deletion') return } @@ -185,6 +189,7 @@ export const webflowHandler: WebhookProviderHandler = { webhookId: webhookRecord.id, externalId: externalId.substring(0, 30), }) + if (strict) throw new Error('Invalid Webflow webhook ID for deletion') return } @@ -193,6 +198,7 @@ export const webflowHandler: WebhookProviderHandler = { logger.warn( `[${requestId}] Missing credentialId for Webflow webhook deletion ${webhookRecord.id}` ) + if (strict) throw new Error('Missing Webflow credentialId for webhook deletion') return } @@ -205,10 +211,9 @@ export const webflowHandler: WebhookProviderHandler = { ) : null if (!accessToken) { - logger.warn( - `[${requestId}] Could not retrieve Webflow access token. Cannot delete webhook.`, - { webhookId: webhookRecord.id } - ) + const message = `[${requestId}] Could not retrieve Webflow access token. Cannot delete webhook.` + logger.warn(message, { webhookId: webhookRecord.id }) + if (strict) throw new Error(message) return } @@ -228,11 +233,15 @@ export const webflowHandler: WebhookProviderHandler = { `[${requestId}] Failed to delete Webflow webhook (non-fatal): ${webflowResponse.status}`, { response: responseBody } ) + if (strict) { + throw new Error(`Failed to delete Webflow webhook: ${webflowResponse.status}`) + } } else { logger.info(`[${requestId}] Successfully deleted Webflow webhook ${externalId}`) } } catch (error) { logger.warn(`[${requestId}] Error deleting Webflow webhook (non-fatal)`, error) + if (strict) throw error } }, diff --git a/apps/sim/lib/workflows/comparison/normalize.test.ts b/apps/sim/lib/workflows/comparison/normalize.test.ts index 9aa6c9b1209..0cdac720978 100644 --- a/apps/sim/lib/workflows/comparison/normalize.test.ts +++ b/apps/sim/lib/workflows/comparison/normalize.test.ts @@ -13,6 +13,7 @@ import { normalizeValue, sanitizeInputFormat, sanitizeTools, + sanitizeVariable, sortEdges, } from './normalize' @@ -152,6 +153,26 @@ describe('Workflow Normalization Utilities', () => { }) }) + describe('sanitizeVariable', () => { + it.concurrent('removes UI-only fields without changing persisted values', () => { + expect( + sanitizeVariable({ + id: 'variable-a', + workflowId: 'workflow-a', + name: 'Optional payload', + type: 'object', + value: null, + validationError: 'invalid', + }) + ).toEqual({ + id: 'variable-a', + name: 'Optional payload', + type: 'object', + value: null, + }) + }) + }) + describe('normalizeLoop', () => { it.concurrent('should normalize null/undefined to undefined', () => { // null and undefined are semantically equivalent diff --git a/apps/sim/lib/workflows/comparison/normalize.ts b/apps/sim/lib/workflows/comparison/normalize.ts index 741208e62ed..effb4770a89 100644 --- a/apps/sim/lib/workflows/comparison/normalize.ts +++ b/apps/sim/lib/workflows/comparison/normalize.ts @@ -201,19 +201,19 @@ export function sanitizeTools(tools: unknown[] | undefined): Record | null | undefined { + variable: VariableWithUiFields | null | undefined +): Omit | null | undefined { if (!variable || typeof variable !== 'object') return variable - const { validationError, ...rest } = variable + const { validationError: _validationError, workflowId: _workflowId, ...rest } = variable return rest } diff --git a/apps/sim/lib/workflows/deployment-outbox.ts b/apps/sim/lib/workflows/deployment-outbox.ts new file mode 100644 index 00000000000..ef6f003066d --- /dev/null +++ b/apps/sim/lib/workflows/deployment-outbox.ts @@ -0,0 +1,705 @@ +import { db, workflowDeploymentVersion, workflow as workflowTable } from '@sim/db' +import { createLogger } from '@sim/logger' +import { and, eq, ne } from 'drizzle-orm' +import { NextRequest } from 'next/server' +import { + enqueueOutboxEvent, + type OutboxHandlerRegistry, + type ProcessSingleOutboxResult, + processOutboxEventById, +} from '@/lib/core/outbox/service' +import { generateRequestId } from '@/lib/core/utils/request' +import { getBaseUrl } from '@/lib/core/utils/urls' +import { + notifyMcpToolServers, + removeMcpToolsForWorkflow, + syncMcpToolsForWorkflow, +} from '@/lib/mcp/workflow-mcp-sync' +import { cleanupWebhooksForWorkflow, saveTriggerWebhooksForDeploy } from '@/lib/webhooks/deploy' +import { createSchedulesForDeploy, deleteSchedulesForWorkflow } from '@/lib/workflows/schedules' +import type { BlockState } from '@/stores/workflows/workflow/types' + +const logger = createLogger('WorkflowDeploymentOutbox') + +export const WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS = { + SYNC_ACTIVE_SIDE_EFFECTS: 'workflow.deployment.sync-active-side-effects', + CLEANUP_INACTIVE_SIDE_EFFECTS: 'workflow.deployment.cleanup-inactive-side-effects', + CLEANUP_UNDEPLOYED_SIDE_EFFECTS: 'workflow.deployment.cleanup-undeployed-side-effects', +} as const + +interface SyncActiveSideEffectsPayload { + workflowId: string + deploymentVersionId: string + userId: string + requestId?: string + forceRecreateSubscriptions?: boolean +} + +interface CleanupUndeployedSideEffectsPayload { + workflowId: string + deploymentVersionIds: string[] + userId: string + requestId?: string +} + +interface CleanupInactiveSideEffectsPayload { + workflowId: string + activeDeploymentVersionId: string + userId: string + requestId?: string +} + +export async function enqueueWorkflowDeploymentSideEffects( + executor: Pick, + payload: SyncActiveSideEffectsPayload +): Promise { + return enqueueOutboxEvent( + executor, + WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.SYNC_ACTIVE_SIDE_EFFECTS, + payload, + { maxAttempts: 10 } + ) +} + +export async function enqueueWorkflowUndeploySideEffects( + executor: Pick, + payload: CleanupUndeployedSideEffectsPayload +): Promise { + return enqueueOutboxEvent( + executor, + WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.CLEANUP_UNDEPLOYED_SIDE_EFFECTS, + payload, + { maxAttempts: 10 } + ) +} + +export async function enqueueWorkflowInactiveDeploymentCleanup( + executor: Pick, + payload: CleanupInactiveSideEffectsPayload +): Promise { + return enqueueOutboxEvent( + executor, + WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.CLEANUP_INACTIVE_SIDE_EFFECTS, + payload, + { maxAttempts: 10 } + ) +} + +export async function processWorkflowDeploymentOutboxEvent( + eventId: string +): Promise { + return processOutboxEventById(eventId, workflowDeploymentOutboxHandlers) +} + +const syncActiveSideEffects = async (rawPayload: unknown): Promise => { + const payload = parseSyncActiveSideEffectsPayload(rawPayload) + const requestId = payload.requestId ?? generateRequestId() + const [workflowRecord] = await db + .select() + .from(workflowTable) + .where(eq(workflowTable.id, payload.workflowId)) + .limit(1) + + if (!workflowRecord) { + logger.warn(`[${requestId}] Workflow missing during deployment side-effect sync`, { + workflowId: payload.workflowId, + }) + return + } + + const [versionRow] = await db + .select({ + id: workflowDeploymentVersion.id, + state: workflowDeploymentVersion.state, + isActive: workflowDeploymentVersion.isActive, + }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, payload.workflowId), + eq(workflowDeploymentVersion.id, payload.deploymentVersionId) + ) + ) + .limit(1) + + if (!versionRow?.isActive) { + logger.info(`[${requestId}] Skipping stale deployment side-effect sync`, { + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + }) + if (versionRow) { + await cleanupDeploymentVersionIfInactive({ + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + workflow: workflowRecord as Record, + userId: payload.userId, + requestId, + }) + } + return + } + + const state = versionRow.state as { blocks?: Record } + const blocks = state.blocks ?? {} + const workflowData = workflowRecord as Record + + if (!(await cleanupStaleDeploymentIfNeeded({ payload, workflow: workflowData, requestId }))) { + return + } + + const request = new NextRequest(new URL('/api/webhooks', getBaseUrl())) + const triggerSaveResult = await saveTriggerWebhooksForDeploy({ + request, + workflowId: payload.workflowId, + workflow: workflowData, + userId: payload.userId, + blocks, + requestId, + deploymentVersionId: payload.deploymentVersionId, + forceRecreateSubscriptions: payload.forceRecreateSubscriptions ?? false, + strictExternalCleanup: true, + }) + + if (!triggerSaveResult.success) { + throw new Error(triggerSaveResult.error?.message || 'Failed to sync trigger configuration') + } + + if (!(await cleanupStaleDeploymentIfNeeded({ payload, workflow: workflowData, requestId }))) { + return + } + + const scheduleResult = await createSchedulesIfStillActive({ + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + blocks, + }) + if (!scheduleResult.success) { + throw new Error(scheduleResult.error || 'Failed to sync schedules') + } + + if (!(await cleanupStaleDeploymentIfNeeded({ payload, workflow: workflowData, requestId }))) { + return + } + + await syncMcpToolsIfStillActive({ + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + requestId, + state, + }) + + if (!(await cleanupStaleDeploymentIfNeeded({ payload, workflow: workflowData, requestId }))) { + return + } + + if (workflowRecord.workspaceId) { + await pruneWorkflowGroupOutputsIfStillActive({ + workflowId: payload.workflowId, + deploymentVersionId: payload.deploymentVersionId, + workspaceId: workflowRecord.workspaceId, + validBlockIds: new Set(Object.keys(blocks)), + requestId, + }) + } + + if (!(await cleanupStaleDeploymentIfNeeded({ payload, workflow: workflowData, requestId }))) { + return + } + + await enqueueWorkflowInactiveDeploymentCleanup(db, { + workflowId: payload.workflowId, + activeDeploymentVersionId: payload.deploymentVersionId, + userId: payload.userId, + requestId, + }) +} + +const cleanupInactiveSideEffects = async (rawPayload: unknown): Promise => { + const payload = parseCleanupInactiveSideEffectsPayload(rawPayload) + const requestId = payload.requestId ?? generateRequestId() + const [workflowRecord] = await db + .select() + .from(workflowTable) + .where(eq(workflowTable.id, payload.workflowId)) + .limit(1) + + if (!workflowRecord) return + + await cleanupInactiveDeploymentVersions({ + workflowId: payload.workflowId, + activeDeploymentVersionId: payload.activeDeploymentVersionId, + workflow: workflowRecord as Record, + userId: payload.userId, + requestId, + }) +} + +const cleanupUndeployedSideEffects = async (rawPayload: unknown): Promise => { + const payload = parseCleanupUndeployedSideEffectsPayload(rawPayload) + const requestId = payload.requestId ?? generateRequestId() + const [workflowRecord] = await db + .select() + .from(workflowTable) + .where(eq(workflowTable.id, payload.workflowId)) + .limit(1) + + if (!workflowRecord) return + const workflowData = workflowRecord as Record + + for (const deploymentVersionId of payload.deploymentVersionIds) { + const [versionRow] = await db + .select({ isActive: workflowDeploymentVersion.isActive }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, payload.workflowId), + eq(workflowDeploymentVersion.id, deploymentVersionId) + ) + ) + .limit(1) + + if (!versionRow || versionRow.isActive) continue + await cleanupDeploymentVersionIfInactive({ + workflowId: payload.workflowId, + workflow: workflowData, + userId: payload.userId, + requestId, + deploymentVersionId, + }) + } + + await cleanupNullVersionWebhooksIfStillUndeployed({ + workflowId: payload.workflowId, + workflow: workflowData, + requestId, + }) + + await removeMcpToolsIfStillUndeployed(payload.workflowId, requestId) +} + +async function cleanupInactiveDeploymentVersions(params: { + workflowId: string + activeDeploymentVersionId: string + workflow: Record + userId: string + requestId: string +}): Promise { + const inactiveVersions = await db + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + ne(workflowDeploymentVersion.id, params.activeDeploymentVersionId), + eq(workflowDeploymentVersion.isActive, false) + ) + ) + + for (const version of inactiveVersions) { + await cleanupDeploymentVersionIfInactive({ + workflowId: params.workflowId, + workflow: params.workflow, + userId: params.userId, + requestId: params.requestId, + deploymentVersionId: version.id, + }) + } +} + +async function cleanupDeploymentVersionIfInactive(params: { + workflowId: string + deploymentVersionId: string + workflow: Record + userId: string + requestId: string +}): Promise { + if (await isDeploymentVersionActive(params.workflowId, params.deploymentVersionId)) { + await enqueueWorkflowDeploymentSideEffects(db, { + workflowId: params.workflowId, + deploymentVersionId: params.deploymentVersionId, + userId: params.userId, + requestId: params.requestId, + forceRecreateSubscriptions: true, + }) + return + } + + const isStillInactive = async () => + !(await isDeploymentVersionActive(params.workflowId, params.deploymentVersionId)) + + await cleanupWebhooksForWorkflow( + params.workflowId, + params.workflow, + params.requestId, + params.deploymentVersionId, + false, + true, + isStillInactive + ) + + if (!(await isStillInactive())) { + await enqueueWorkflowDeploymentSideEffects(db, { + workflowId: params.workflowId, + deploymentVersionId: params.deploymentVersionId, + userId: params.userId, + requestId: params.requestId, + forceRecreateSubscriptions: true, + }) + return + } + + const deletedSchedules = await deleteSchedulesForDeploymentIfInactive({ + workflowId: params.workflowId, + deploymentVersionId: params.deploymentVersionId, + }) + if (!deletedSchedules) { + if (await isDeploymentVersionActive(params.workflowId, params.deploymentVersionId)) { + await enqueueWorkflowDeploymentSideEffects(db, { + workflowId: params.workflowId, + deploymentVersionId: params.deploymentVersionId, + userId: params.userId, + requestId: params.requestId, + forceRecreateSubscriptions: true, + }) + } + return + } + + if (await isDeploymentVersionActive(params.workflowId, params.deploymentVersionId)) { + await enqueueWorkflowDeploymentSideEffects(db, { + workflowId: params.workflowId, + deploymentVersionId: params.deploymentVersionId, + userId: params.userId, + requestId: params.requestId, + forceRecreateSubscriptions: true, + }) + } +} + +async function deleteSchedulesForDeploymentIfInactive(params: { + workflowId: string + deploymentVersionId: string +}): Promise { + return db.transaction(async (tx) => { + const [versionRow] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + eq(workflowDeploymentVersion.id, params.deploymentVersionId), + eq(workflowDeploymentVersion.isActive, false) + ) + ) + .limit(1) + .for('update') + + if (!versionRow) return false + + await deleteSchedulesForWorkflow(params.workflowId, tx, params.deploymentVersionId) + return true + }) +} + +async function cleanupStaleDeploymentIfNeeded(params: { + payload: SyncActiveSideEffectsPayload + workflow: Record + requestId: string +}): Promise { + if ( + await isDeploymentVersionActive(params.payload.workflowId, params.payload.deploymentVersionId) + ) { + return true + } + + logger.info(`[${params.requestId}] Cleaning up stale deployment side effects`, { + workflowId: params.payload.workflowId, + deploymentVersionId: params.payload.deploymentVersionId, + }) + await cleanupDeploymentVersionIfInactive({ + workflowId: params.payload.workflowId, + workflow: params.workflow, + userId: params.payload.userId, + requestId: params.requestId, + deploymentVersionId: params.payload.deploymentVersionId, + }) + return false +} + +async function isDeploymentVersionActive( + workflowId: string, + deploymentVersionId: string +): Promise { + const [versionRow] = await db + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.id, deploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + return Boolean(versionRow) +} + +async function removeMcpToolsIfStillUndeployed( + workflowId: string, + requestId: string +): Promise { + const tools = await db.transaction(async (tx) => { + const [workflowRecord] = await tx + .select({ id: workflowTable.id, isDeployed: workflowTable.isDeployed }) + .from(workflowTable) + .where(eq(workflowTable.id, workflowId)) + .limit(1) + .for('update') + + if (!workflowRecord || workflowRecord.isDeployed) return [] + return removeMcpToolsForWorkflow(workflowId, requestId, tx, false, true) + }) + notifyMcpToolServers(tools) +} + +async function cleanupNullVersionWebhooksIfStillUndeployed(params: { + workflowId: string + workflow: Record + requestId: string +}): Promise { + const isStillUndeployed = async () => { + const [workflowRecord] = await db + .select({ isDeployed: workflowTable.isDeployed }) + .from(workflowTable) + .where(eq(workflowTable.id, params.workflowId)) + .limit(1) + + return Boolean(workflowRecord && !workflowRecord.isDeployed) + } + + if (!(await isStillUndeployed())) return + await cleanupWebhooksForWorkflow( + params.workflowId, + params.workflow, + params.requestId, + null, + false, + true, + isStillUndeployed + ) +} + +async function syncMcpToolsIfStillActive(params: { + workflowId: string + deploymentVersionId: string + requestId: string + state: { blocks?: Record } +}): Promise { + const tools = await db.transaction(async (tx) => { + const [workflowRecord] = await tx + .select({ id: workflowTable.id }) + .from(workflowTable) + .where(eq(workflowTable.id, params.workflowId)) + .limit(1) + .for('update') + + if (!workflowRecord) return [] + + const [versionRow] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + eq(workflowDeploymentVersion.id, params.deploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + if (!versionRow) return [] + + return syncMcpToolsForWorkflow({ + workflowId: params.workflowId, + requestId: params.requestId, + state: params.state, + context: 'deployment-outbox', + tx, + notify: false, + throwOnError: true, + }) + }) + notifyMcpToolServers(tools) +} + +async function createSchedulesIfStillActive(params: { + workflowId: string + deploymentVersionId: string + blocks: Record +}) { + return db.transaction(async (tx) => { + const [workflowRecord] = await tx + .select({ id: workflowTable.id }) + .from(workflowTable) + .where(eq(workflowTable.id, params.workflowId)) + .limit(1) + .for('update') + + if (!workflowRecord) { + return { success: true as const } + } + + const [versionRow] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + eq(workflowDeploymentVersion.id, params.deploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + if (!versionRow) { + return { success: true as const } + } + + const result = await createSchedulesForDeploy( + params.workflowId, + params.blocks, + tx, + params.deploymentVersionId + ) + if (!result.success) { + throw new Error(result.error || 'Failed to sync schedules') + } + return result + }) +} + +async function pruneWorkflowGroupOutputsIfStillActive(params: { + workflowId: string + deploymentVersionId: string + workspaceId: string + validBlockIds: Set + requestId: string +}): Promise { + await db.transaction(async (tx) => { + const [workflowRecord] = await tx + .select({ id: workflowTable.id }) + .from(workflowTable) + .where(eq(workflowTable.id, params.workflowId)) + .limit(1) + .for('update') + + if (!workflowRecord) return + + const [versionRow] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, params.workflowId), + eq(workflowDeploymentVersion.id, params.deploymentVersionId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + + if (!versionRow) return + + const { pruneStaleWorkflowGroupOutputs } = await import('@/lib/table/service') + await pruneStaleWorkflowGroupOutputs({ + workflowId: params.workflowId, + workspaceId: params.workspaceId, + validBlockIds: params.validBlockIds, + requestId: params.requestId, + tx, + }) + }) +} + +function parseSyncActiveSideEffectsPayload(payload: unknown): SyncActiveSideEffectsPayload { + const record = parsePayloadRecord(payload) + const workflowId = parseRequiredString(record.workflowId, 'workflowId') + const deploymentVersionId = parseRequiredString(record.deploymentVersionId, 'deploymentVersionId') + const userId = parseRequiredString(record.userId, 'userId') + const requestId = + typeof record.requestId === 'string' && record.requestId.length > 0 + ? record.requestId + : undefined + const forceRecreateSubscriptions = + typeof record.forceRecreateSubscriptions === 'boolean' + ? record.forceRecreateSubscriptions + : undefined + + return { workflowId, deploymentVersionId, userId, requestId, forceRecreateSubscriptions } +} + +function parseCleanupUndeployedSideEffectsPayload( + payload: unknown +): CleanupUndeployedSideEffectsPayload { + const record = parsePayloadRecord(payload) + const workflowId = parseRequiredString(record.workflowId, 'workflowId') + const userId = parseRequiredString(record.userId, 'userId') + const deploymentVersionIds = parseRequiredStringArray( + record.deploymentVersionIds, + 'deploymentVersionIds' + ) + const requestId = + typeof record.requestId === 'string' && record.requestId.length > 0 + ? record.requestId + : undefined + + return { workflowId, deploymentVersionIds, userId, requestId } +} + +function parseCleanupInactiveSideEffectsPayload( + payload: unknown +): CleanupInactiveSideEffectsPayload { + const record = parsePayloadRecord(payload) + const workflowId = parseRequiredString(record.workflowId, 'workflowId') + const activeDeploymentVersionId = parseRequiredString( + record.activeDeploymentVersionId, + 'activeDeploymentVersionId' + ) + const userId = parseRequiredString(record.userId, 'userId') + const requestId = + typeof record.requestId === 'string' && record.requestId.length > 0 + ? record.requestId + : undefined + + return { workflowId, activeDeploymentVersionId, userId, requestId } +} + +function parsePayloadRecord(payload: unknown): Record { + if (!payload || typeof payload !== 'object' || Array.isArray(payload)) { + throw new Error('Deployment outbox payload must be an object') + } + return payload as Record +} + +function parseRequiredString(value: unknown, fieldName: string): string { + if (typeof value !== 'string' || value.length === 0) { + throw new Error(`Deployment outbox payload is missing ${fieldName}`) + } + return value +} + +function parseRequiredStringArray(value: unknown, fieldName: string): string[] { + if ( + !Array.isArray(value) || + value.some((item) => typeof item !== 'string' || item.length === 0) + ) { + throw new Error(`Deployment outbox payload is missing ${fieldName}`) + } + return value +} + +export const workflowDeploymentOutboxHandlers: OutboxHandlerRegistry = { + [WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.SYNC_ACTIVE_SIDE_EFFECTS]: syncActiveSideEffects, + [WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.CLEANUP_INACTIVE_SIDE_EFFECTS]: cleanupInactiveSideEffects, + [WORKFLOW_DEPLOYMENT_OUTBOX_EVENTS.CLEANUP_UNDEPLOYED_SIDE_EFFECTS]: cleanupUndeployedSideEffects, +} diff --git a/apps/sim/lib/workflows/orchestration/deploy.test.ts b/apps/sim/lib/workflows/orchestration/deploy.test.ts new file mode 100644 index 00000000000..2fff78e9122 --- /dev/null +++ b/apps/sim/lib/workflows/orchestration/deploy.test.ts @@ -0,0 +1,211 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockLimit, + mockUpdateSet, + mockSaveWorkflowToNormalizedTables, + mockRecordAudit, + mockCaptureServerEvent, + mockTransaction, + mockTx, +} = vi.hoisted(() => ({ + mockLimit: vi.fn(), + mockUpdateSet: vi.fn(), + mockSaveWorkflowToNormalizedTables: vi.fn(), + mockRecordAudit: vi.fn(), + mockCaptureServerEvent: vi.fn(), + mockTransaction: vi.fn(), + mockTx: { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: vi.fn(() => ({ + for: vi.fn().mockResolvedValue([{ id: 'workflow-1' }]), + })), + })), + })), + })), + update: vi.fn(() => ({ + set: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })), + })), + execute: vi.fn().mockResolvedValue(undefined), + }, +})) + +vi.mock('@sim/db', () => ({ + db: { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: mockLimit, + })), + })), + })), + update: vi.fn(() => ({ + set: mockUpdateSet, + })), + transaction: mockTransaction, + }, + workflow: { id: 'workflow.id' }, + workflowDeploymentVersion: { + workflowId: 'workflowDeploymentVersion.workflowId', + version: 'workflowDeploymentVersion.version', + isActive: 'workflowDeploymentVersion.isActive', + state: 'workflowDeploymentVersion.state', + }, +})) + +vi.mock('@sim/audit', () => ({ + AuditAction: { WORKFLOW_DEPLOYMENT_REVERTED: 'WORKFLOW_DEPLOYMENT_REVERTED' }, + AuditResourceType: { WORKFLOW: 'WORKFLOW' }, + recordAudit: mockRecordAudit, +})) + +vi.mock('@/lib/core/config/env', () => ({ + env: { INTERNAL_API_SECRET: 'secret' }, +})) + +vi.mock('@/lib/core/utils/urls', () => ({ + getBaseUrl: () => 'http://localhost:3000', + getSocketServerUrl: () => 'http://localhost:3002', +})) + +vi.mock('@/lib/posthog/server', () => ({ + captureServerEvent: mockCaptureServerEvent, +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + activateWorkflowVersion: vi.fn(), + activateWorkflowVersionById: vi.fn(), + deployWorkflow: vi.fn(), + loadWorkflowDeploymentSnapshot: vi.fn(), + saveWorkflowToNormalizedTables: mockSaveWorkflowToNormalizedTables, + undeployWorkflow: vi.fn(), +})) + +vi.mock('@/lib/mcp/workflow-mcp-sync', () => ({ + removeMcpToolsForWorkflow: vi.fn(), + syncMcpToolsForWorkflow: vi.fn(), +})) + +vi.mock('@/lib/webhooks/deploy', () => ({ + cleanupWebhooksForWorkflow: vi.fn(), + restorePreviousVersionWebhooks: vi.fn(), + saveTriggerWebhooksForDeploy: vi.fn(), +})) + +vi.mock('@/lib/workflows/schedules', () => ({ + cleanupDeploymentVersion: vi.fn(), + createSchedulesForDeploy: vi.fn(), + validateWorkflowSchedules: vi.fn(), +})) + +import { performRevertToVersion } from '@/lib/workflows/orchestration/deploy' + +describe('performRevertToVersion', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(null, { status: 200 }))) + mockTransaction.mockImplementation(async (callback) => callback(mockTx)) + mockTx.select.mockImplementation((selection?: Record) => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: + selection && Object.hasOwn(selection, 'state') + ? mockLimit + : vi.fn(() => ({ + for: vi.fn().mockResolvedValue([{ id: 'workflow-1' }]), + })), + })), + })), + })) + mockTx.update.mockReturnValue({ set: mockUpdateSet }) + mockUpdateSet.mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) }) + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + }) + + it('restores variables when the deployment snapshot includes them', async () => { + mockLimit.mockResolvedValue([ + { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + variables: { + variableA: { + id: 'variableA', + name: 'API_KEY', + type: 'plain', + value: 'deployed-value', + }, + }, + }, + }, + ]) + + const result = await performRevertToVersion({ + workflowId: 'workflow-1', + version: 3, + userId: 'user-1', + workflow: { id: 'workflow-1', name: 'Workflow', workspaceId: 'workspace-1' }, + }) + + expect(result.success).toBe(true) + expect(mockSaveWorkflowToNormalizedTables).toHaveBeenCalledWith( + 'workflow-1', + expect.objectContaining({ + variables: { + variableA: { + id: 'variableA', + name: 'API_KEY', + type: 'plain', + value: 'deployed-value', + }, + }, + }), + mockTx + ) + expect(mockUpdateSet).toHaveBeenCalledWith( + expect.objectContaining({ + variables: { + variableA: { + id: 'variableA', + name: 'API_KEY', + type: 'plain', + value: 'deployed-value', + }, + }, + }) + ) + }) + + it('preserves existing variables when reverting a legacy snapshot without variables', async () => { + mockLimit.mockResolvedValue([ + { + state: { + blocks: {}, + edges: [], + loops: {}, + parallels: {}, + }, + }, + ]) + + const result = await performRevertToVersion({ + workflowId: 'workflow-1', + version: 2, + userId: 'user-1', + workflow: { id: 'workflow-1', name: 'Workflow', workspaceId: 'workspace-1' }, + }) + + expect(result.success).toBe(true) + const savedState = mockSaveWorkflowToNormalizedTables.mock.calls[0][1] + expect(Object.hasOwn(savedState, 'variables')).toBe(false) + const workflowUpdate = mockUpdateSet.mock.calls[0][0] + expect(Object.hasOwn(workflowUpdate, 'variables')).toBe(false) + }) +}) diff --git a/apps/sim/lib/workflows/orchestration/deploy.ts b/apps/sim/lib/workflows/orchestration/deploy.ts index 926ed2eeeb4..78b4db442bf 100644 --- a/apps/sim/lib/workflows/orchestration/deploy.ts +++ b/apps/sim/lib/workflows/orchestration/deploy.ts @@ -2,31 +2,25 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db, workflowDeploymentVersion, workflow as workflowTable } from '@sim/db' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' -import { NextRequest } from 'next/server' +import type { NextRequest } from 'next/server' import { env } from '@/lib/core/config/env' import { generateRequestId } from '@/lib/core/utils/request' -import { getBaseUrl, getSocketServerUrl } from '@/lib/core/utils/urls' -import { removeMcpToolsForWorkflow, syncMcpToolsForWorkflow } from '@/lib/mcp/workflow-mcp-sync' +import { getSocketServerUrl } from '@/lib/core/utils/urls' import { captureServerEvent } from '@/lib/posthog/server' +import { validateTriggerWebhookConfigForDeploy } from '@/lib/webhooks/deploy' import { - cleanupWebhooksForWorkflow, - restorePreviousVersionWebhooks, - saveTriggerWebhooksForDeploy, -} from '@/lib/webhooks/deploy' + enqueueWorkflowDeploymentSideEffects, + enqueueWorkflowUndeploySideEffects, + processWorkflowDeploymentOutboxEvent, +} from '@/lib/workflows/deployment-outbox' import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' import { activateWorkflowVersion, - activateWorkflowVersionById, deployWorkflow, - loadWorkflowFromNormalizedTables, saveWorkflowToNormalizedTables, undeployWorkflow, } from '@/lib/workflows/persistence/utils' -import { - cleanupDeploymentVersion, - createSchedulesForDeploy, - validateWorkflowSchedules, -} from '@/lib/workflows/schedules' +import { validateWorkflowSchedules } from '@/lib/workflows/schedules' import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types' const logger = createLogger('DeployOrchestration') @@ -83,10 +77,10 @@ export interface PerformFullDeployResult { } /** - * Performs a full workflow deployment: creates a deployment version, syncs - * trigger webhooks, creates schedules, cleans up the previous version, and - * syncs MCP tools. Both the deploy API route and the copilot deploy tools - * must use this single function so behaviour stays consistent. + * Performs a full workflow deployment: creates a deployment version, queues + * external side effects transactionally, processes that outbox event after + * commit, and notifies clients. Both the deploy API route and the copilot + * deploy tools must use this single function so behaviour stays consistent. */ export async function performFullDeploy( params: PerformFullDeployParams @@ -94,21 +88,6 @@ export async function performFullDeploy( const { workflowId, userId, workflowName } = params const actorId = params.actorId ?? userId const requestId = params.requestId ?? generateRequestId() - const request = params.request ?? new NextRequest(new URL('/api/webhooks', getBaseUrl())) - - const normalizedData = await loadWorkflowFromNormalizedTables(workflowId) - if (!normalizedData) { - return { success: false, error: 'Failed to load workflow state', errorCode: 'not_found' } - } - - const scheduleValidation = validateWorkflowSchedules(normalizedData.blocks) - if (!scheduleValidation.isValid) { - return { - success: false, - error: `Invalid schedule configuration: ${scheduleValidation.error}`, - errorCode: 'validation', - } - } const [workflowRecord] = await db .select() @@ -121,135 +100,60 @@ export async function performFullDeploy( } const workflowData = workflowRecord as Record - - const [currentActiveVersion] = await db - .select({ id: workflowDeploymentVersion.id }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.isActive, true) - ) - ) - .limit(1) - const previousVersionId = currentActiveVersion?.id - - const rollbackDeployment = async () => { - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow: workflowData, - userId, - previousVersionId, - requestId, - }) - const reactivateResult = await activateWorkflowVersionById({ - workflowId, - deploymentVersionId: previousVersionId, - }) - if (reactivateResult.success) return - } - await undeployWorkflow({ workflowId }) - } + let outboxEventId: string | undefined const deployResult = await deployWorkflow({ workflowId, deployedBy: actorId, workflowName: workflowName || workflowRecord.name || undefined, + validateWorkflowState: async (workflowState) => { + const scheduleValidation = validateWorkflowSchedules(workflowState.blocks) + if (!scheduleValidation.isValid) { + return { + success: false, + error: `Invalid schedule configuration: ${scheduleValidation.error}`, + errorCode: 'validation', + } + } + const triggerValidation = await validateTriggerWebhookConfigForDeploy(workflowState.blocks) + if (!triggerValidation.success) { + return { + success: false, + error: triggerValidation.error?.message || 'Invalid trigger configuration', + errorCode: 'validation', + } + } + return { success: true } + }, + onDeployTransaction: async (tx, result) => { + outboxEventId = await enqueueWorkflowDeploymentSideEffects(tx, { + workflowId, + deploymentVersionId: result.deploymentVersionId, + userId, + requestId, + }) + }, }) if (!deployResult.success) { - return { success: false, error: deployResult.error || 'Failed to deploy workflow' } + const error = deployResult.error || 'Failed to deploy workflow' + return { + success: false, + error, + errorCode: deployResult.errorCode, + } } const deployedAt = deployResult.deployedAt! const deploymentVersionId = deployResult.deploymentVersionId + const previousVersionId = deployResult.previousVersionId + const deploymentSnapshot = deployResult.currentState - if (!deploymentVersionId) { + if (!deploymentVersionId || !deploymentSnapshot) { await undeployWorkflow({ workflowId }) return { success: false, error: 'Failed to resolve deployment version' } } - const triggerSaveResult = await saveTriggerWebhooksForDeploy({ - request, - workflowId, - workflow: workflowData, - userId, - blocks: normalizedData.blocks, - requestId, - deploymentVersionId, - previousVersionId, - }) - - if (!triggerSaveResult.success) { - await cleanupDeploymentVersion({ - workflowId, - workflow: workflowData, - requestId, - deploymentVersionId, - }) - await rollbackDeployment() - return { - success: false, - error: triggerSaveResult.error?.message || 'Failed to save trigger configuration', - } - } - - const scheduleResult = await createSchedulesForDeploy( - workflowId, - normalizedData.blocks, - db, - deploymentVersionId - ) - if (!scheduleResult.success) { - logger.error(`[${requestId}] Failed to create schedule: ${scheduleResult.error}`) - await cleanupDeploymentVersion({ - workflowId, - workflow: workflowData, - requestId, - deploymentVersionId, - }) - await rollbackDeployment() - return { success: false, error: scheduleResult.error || 'Failed to create schedule' } - } - - if (previousVersionId && previousVersionId !== deploymentVersionId) { - try { - await cleanupDeploymentVersion({ - workflowId, - workflow: workflowData, - requestId, - deploymentVersionId: previousVersionId, - skipExternalCleanup: true, - }) - } catch (cleanupError) { - logger.error(`[${requestId}] Failed to clean up previous version`, cleanupError) - } - } - - await syncMcpToolsForWorkflow({ workflowId, requestId, context: 'deploy' }) - - // Drop stale block refs from any table workflow column targeting this workflow, - // so columns that referenced just-removed blocks collapse cleanly instead of - // showing perpetual "Waiting" indicators on future row runs. - if (workflowData.workspaceId) { - try { - const { pruneStaleWorkflowGroupOutputs } = await import('@/lib/table/service') - const validBlockIds = new Set(Object.keys(normalizedData.blocks)) - await pruneStaleWorkflowGroupOutputs({ - workflowId, - workspaceId: workflowData.workspaceId as string, - validBlockIds, - requestId, - }) - } catch (pruneError) { - logger.warn( - `[${requestId}] Failed to prune stale workflow column outputs for ${workflowId}`, - pruneError - ) - } - } - recordAudit({ workspaceId: (workflowData.workspaceId as string) || null, actorId: actorId, @@ -262,11 +166,11 @@ export async function performFullDeploy( deploymentVersionId, version: deployResult.version, previousVersionId: previousVersionId || undefined, - triggerWarnings: triggerSaveResult.warnings?.length ? triggerSaveResult.warnings : undefined, }, - request, + request: params.request, }) + const sideEffectWarning = await processDeploymentSideEffectsNow(outboxEventId, requestId) await notifySocketDeploymentChanged(workflowId) return { @@ -274,7 +178,7 @@ export async function performFullDeploy( deployedAt, version: deployResult.version, deploymentVersionId, - warnings: triggerSaveResult.warnings, + warnings: sideEffectWarning ? [sideEffectWarning] : undefined, } } @@ -289,13 +193,14 @@ export interface PerformFullUndeployParams { export interface PerformFullUndeployResult { success: boolean error?: string + warnings?: string[] } /** - * Performs a full workflow undeploy: marks the workflow as undeployed, cleans up - * webhook records and external subscriptions, removes MCP tools, emits a - * telemetry event, and records an audit log entry. Both the deploy API DELETE - * handler and the copilot undeploy tools must use this single function. + * Performs a full workflow undeploy: marks the workflow as undeployed, queues + * external cleanup transactionally, emits a telemetry event, and records an + * audit log entry. Both the deploy API DELETE handler and the copilot undeploy + * tools must use this single function. */ export async function performFullUndeploy( params: PerformFullUndeployParams @@ -315,15 +220,23 @@ export async function performFullUndeploy( } const workflowData = workflowRecord as Record + let outboxEventId: string | undefined - const result = await undeployWorkflow({ workflowId }) + const result = await undeployWorkflow({ + workflowId, + onUndeployTransaction: async (tx, undeploy) => { + outboxEventId = await enqueueWorkflowUndeploySideEffects(tx, { + workflowId, + deploymentVersionIds: undeploy.deploymentVersionIds, + userId, + requestId, + }) + }, + }) if (!result.success) { return { success: false, error: result.error || 'Failed to undeploy workflow' } } - await cleanupWebhooksForWorkflow(workflowId, workflowData, requestId) - await removeMcpToolsForWorkflow(workflowId, requestId) - logger.info(`[${requestId}] Workflow undeployed successfully: ${workflowId}`) try { @@ -344,8 +257,9 @@ export async function performFullUndeploy( }) await notifySocketDeploymentChanged(workflowId) + const sideEffectWarning = await processDeploymentSideEffectsNow(outboxEventId, requestId) - return { success: true } + return { success: true, warnings: sideEffectWarning ? [sideEffectWarning] : undefined } } export interface PerformActivateVersionParams { @@ -387,11 +301,10 @@ export interface PerformRevertToVersionResult { } /** - * Activates an existing deployment version: validates schedules, syncs trigger - * webhooks (with forced subscription recreation), creates schedules, activates - * the version, cleans up the previous version, syncs MCP tools, and records - * an audit entry. Both the deployment version PATCH handler and the admin - * activate route must use this function. + * Activates an existing deployment version: validates schedules, activates the + * version, queues external side effects transactionally, processes that outbox + * event after commit, and records an audit entry. Both the deployment version + * PATCH handler and the admin activate route must use this function. */ export async function performActivateVersion( params: PerformActivateVersionParams @@ -399,12 +312,12 @@ export async function performActivateVersion( const { workflowId, version, userId, workflow } = params const actorId = params.actorId ?? userId const requestId = params.requestId ?? generateRequestId() - const request = params.request ?? new NextRequest(new URL('/api/webhooks', getBaseUrl())) const [versionRow] = await db .select({ id: workflowDeploymentVersion.id, state: workflowDeploymentVersion.state, + isActive: workflowDeploymentVersion.isActive, }) .from(workflowDeploymentVersion) .where( @@ -419,24 +332,22 @@ export async function performActivateVersion( return { success: false, error: 'Deployment version not found', errorCode: 'not_found' } } + if (versionRow.isActive) { + const [workflowDeployment] = await db + .select({ deployedAt: workflowTable.deployedAt }) + .from(workflowTable) + .where(eq(workflowTable.id, workflowId)) + .limit(1) + + return { success: true, deployedAt: workflowDeployment?.deployedAt ?? new Date(), warnings: [] } + } + const deployedState = versionRow.state as { blocks?: Record } const blocks = deployedState.blocks if (!blocks || typeof blocks !== 'object') { return { success: false, error: 'Invalid deployed state structure', errorCode: 'validation' } } - const [currentActiveVersion] = await db - .select({ id: workflowDeploymentVersion.id }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.isActive, true) - ) - ) - .limit(1) - const previousVersionId = currentActiveVersion?.id - const scheduleValidation = validateWorkflowSchedules(blocks as Record) if (!scheduleValidation.isValid) { return { @@ -446,101 +357,35 @@ export async function performActivateVersion( } } - const triggerSaveResult = await saveTriggerWebhooksForDeploy({ - request, - workflowId, - workflow, - userId, - blocks: blocks as Record, - requestId, - deploymentVersionId: versionRow.id, - previousVersionId, - forceRecreateSubscriptions: true, - }) - - if (!triggerSaveResult.success) { - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow, - userId, - previousVersionId, - requestId, - }) - } + const triggerValidation = await validateTriggerWebhookConfigForDeploy( + blocks as Record + ) + if (!triggerValidation.success) { return { success: false, - error: triggerSaveResult.error?.message || 'Failed to sync trigger configuration', + error: triggerValidation.error?.message || 'Invalid trigger configuration', + errorCode: 'validation', } } - const scheduleResult = await createSchedulesForDeploy( + let outboxEventId: string | undefined + const result = await activateWorkflowVersion({ workflowId, - blocks as Record, - db, - versionRow.id - ) - - if (!scheduleResult.success) { - await cleanupDeploymentVersion({ - workflowId, - workflow, - requestId, - deploymentVersionId: versionRow.id, - }) - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow, + version, + onActivateTransaction: async (tx, activation) => { + outboxEventId = await enqueueWorkflowDeploymentSideEffects(tx, { + workflowId, + deploymentVersionId: activation.deploymentVersionId, userId, - previousVersionId, requestId, + forceRecreateSubscriptions: true, }) - } - return { success: false, error: scheduleResult.error || 'Failed to sync schedules' } - } - - const result = await activateWorkflowVersion({ workflowId, version }) + }, + }) if (!result.success) { - await cleanupDeploymentVersion({ - workflowId, - workflow, - requestId, - deploymentVersionId: versionRow.id, - }) - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow, - userId, - previousVersionId, - requestId, - }) - } return { success: false, error: result.error || 'Failed to activate version' } } - if (previousVersionId && previousVersionId !== versionRow.id) { - try { - await cleanupDeploymentVersion({ - workflowId, - workflow, - requestId, - deploymentVersionId: previousVersionId, - skipExternalCleanup: true, - }) - } catch (cleanupError) { - logger.error(`[${requestId}] Failed to clean up previous version`, cleanupError) - } - } - - await syncMcpToolsForWorkflow({ - workflowId, - requestId, - state: versionRow.state as { blocks?: Record }, - context: 'activate', - }) - recordAudit({ workspaceId: (workflow.workspaceId as string) || null, actorId: actorId, @@ -552,16 +397,50 @@ export async function performActivateVersion( metadata: { version, deploymentVersionId: versionRow.id, - previousVersionId: previousVersionId || undefined, + previousVersionId: result.previousVersionId || undefined, }, }) + const sideEffectWarning = await processDeploymentSideEffectsNow(outboxEventId, requestId) await notifySocketDeploymentChanged(workflowId) return { success: true, deployedAt: result.deployedAt, - warnings: triggerSaveResult.warnings, + warnings: sideEffectWarning ? [sideEffectWarning] : undefined, + } +} + +async function processDeploymentSideEffectsNow( + outboxEventId: string | undefined, + requestId: string +): Promise { + if (!outboxEventId) { + return 'Deployment state changed, but side-effect sync was not queued. Redeploy if triggers or schedules look stale.' + } + + try { + const result = await processWorkflowDeploymentOutboxEvent(outboxEventId) + if (result === 'completed') return undefined + if (result === 'dead_letter' || result === 'not_found') { + logger.error(`[${requestId}] Deployment side-effect sync cannot be retried automatically`, { + outboxEventId, + result, + }) + return 'Deployment saved, but trigger, schedule, and MCP sync could not be queued. Redeploy if triggers or schedules look stale.' + } + + logger.warn(`[${requestId}] Deployment side-effect sync queued for retry`, { + outboxEventId, + result, + }) + return 'Deployment saved. Trigger, schedule, and MCP sync is queued and may finish shortly.' + } catch (error) { + logger.warn(`[${requestId}] Deployment side-effect sync queued for retry`, { + outboxEventId, + error, + }) + return 'Deployment saved. Trigger, schedule, and MCP sync is queued and may finish shortly.' } } @@ -577,73 +456,93 @@ export async function performRevertToVersion( const actorId = params.actorId ?? userId const versionLabel = String(version) - let stateRow: { state: unknown } | null = null - if (version === 'active') { - const [row] = await db - .select({ state: workflowDeploymentVersion.state }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.isActive, true) - ) - ) - .limit(1) - stateRow = row || null - } else { - const [row] = await db - .select({ state: workflowDeploymentVersion.state }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.version, version) - ) - ) + const lastSaved = Date.now() + const saveResult = await db.transaction(async (tx) => { + await tx + .select({ id: workflowTable.id }) + .from(workflowTable) + .where(eq(workflowTable.id, workflowId)) .limit(1) - stateRow = row || null - } + .for('update') + + const [stateRow] = + version === 'active' + ? await tx + .select({ state: workflowDeploymentVersion.state }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + : await tx + .select({ state: workflowDeploymentVersion.state }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.version, version) + ) + ) + .limit(1) + + if (!stateRow?.state) { + return { success: false, error: 'Deployment version not found' } + } - if (!stateRow?.state) { - return { success: false, error: 'Deployment version not found', errorCode: 'not_found' } - } + const deployedState = stateRow.state as { + blocks?: Record + edges?: unknown[] + loops?: Record + parallels?: Record + variables?: WorkflowState['variables'] + } + if (!deployedState.blocks || !deployedState.edges) { + return { success: false, error: 'Invalid deployed state structure' } + } - const deployedState = stateRow.state as { - blocks?: Record - edges?: unknown[] - loops?: Record - parallels?: Record - } - if (!deployedState.blocks || !deployedState.edges) { - return { - success: false, - error: 'Invalid deployed state structure', - errorCode: 'internal', + const hasDeploymentVariables = Object.hasOwn(deployedState, 'variables') + const restoredState: WorkflowState = { + blocks: deployedState.blocks, + edges: deployedState.edges, + loops: deployedState.loops || {}, + parallels: deployedState.parallels || {}, + lastSaved, + } as WorkflowState + if (hasDeploymentVariables) { + restoredState.variables = deployedState.variables || {} } - } - const lastSaved = Date.now() - const saveResult = await saveWorkflowToNormalizedTables(workflowId, { - blocks: deployedState.blocks, - edges: deployedState.edges, - loops: deployedState.loops || {}, - parallels: deployedState.parallels || {}, - lastSaved, - } as WorkflowState) + const result = await saveWorkflowToNormalizedTables(workflowId, restoredState, tx) + if (!result.success) return result + + await tx + .update(workflowTable) + .set({ + ...(hasDeploymentVariables ? { variables: deployedState.variables || {} } : {}), + lastSynced: new Date(), + updatedAt: new Date(), + }) + .where(eq(workflowTable.id, workflowId)) + + return result + }) if (!saveResult.success) { return { success: false, error: saveResult.error || 'Failed to save deployed state', - errorCode: 'internal', + errorCode: + saveResult.error === 'Deployment version not found' + ? 'not_found' + : saveResult.error === 'Invalid deployed state structure' + ? 'internal' + : 'internal', } } - await db - .update(workflowTable) - .set({ lastSynced: new Date(), updatedAt: new Date() }) - .where(eq(workflowTable.id, workflowId)) - try { await fetch(`${getSocketServerUrl()}/api/workflow-reverted`, { method: 'POST', diff --git a/apps/sim/lib/workflows/persistence/utils.test.ts b/apps/sim/lib/workflows/persistence/utils.test.ts index 3a8f3c47cba..82997c4f518 100644 --- a/apps/sim/lib/workflows/persistence/utils.test.ts +++ b/apps/sim/lib/workflows/persistence/utils.test.ts @@ -305,6 +305,42 @@ describe('Database Helpers', () => { vi.resetAllMocks() }) + describe('buildWorkflowDeploymentSnapshot', () => { + it('combines normalized workflow state with persisted variables', () => { + const snapshot = dbHelpers.buildWorkflowDeploymentSnapshot( + { + blocks: asAppBlocks({ block: createStarterBlock({ id: 'block' }) }), + edges: [], + loops: {}, + parallels: {}, + isFromNormalizedTables: true, + }, + { + variable: { + id: 'variable', + name: 'threshold', + type: 'number', + value: 5, + }, + } + ) + + expect(snapshot.blocks.block).toBeDefined() + expect(snapshot.edges).toEqual([]) + expect(snapshot.loops).toEqual({}) + expect(snapshot.parallels).toEqual({}) + expect(snapshot.variables).toEqual({ + variable: { + id: 'variable', + name: 'threshold', + type: 'number', + value: 5, + }, + }) + expect(snapshot.lastSaved).toEqual(expect.any(Number)) + }) + }) + describe('loadWorkflowFromNormalizedTables', () => { it('should successfully load workflow data from normalized tables', async () => { vi.clearAllMocks() @@ -762,6 +798,58 @@ describe('Database Helpers', () => { }) }) + describe('workflow row locking', () => { + function createMissingWorkflowTx() { + const lockFor = vi.fn().mockResolvedValue([]) + const limit = vi.fn(() => ({ for: lockFor })) + const where = vi.fn(() => ({ limit })) + const from = vi.fn(() => ({ where })) + const select = vi.fn(() => ({ from })) + const update = vi.fn() + + return { + tx: { + execute: vi.fn().mockResolvedValue([{ id: mockWorkflowId }]), + select, + update, + }, + lockFor, + update, + } + } + + it('returns not_found when deploy cannot lock a workflow row', async () => { + const { tx, lockFor } = createMissingWorkflowTx() + mockDb.transaction = vi.fn().mockImplementation(async (callback) => callback(tx)) + + const result = await dbHelpers.deployWorkflow({ + workflowId: mockWorkflowId, + deployedBy: 'user-123', + }) + + expect(result).toEqual({ + success: false, + error: 'Workflow not found', + errorCode: 'not_found', + }) + expect(lockFor).toHaveBeenCalledWith('update') + expect(tx.execute).not.toHaveBeenCalled() + }) + + it('returns an error when undeploy cannot lock a workflow row', async () => { + const { tx, update } = createMissingWorkflowTx() + mockDb.transaction = vi.fn().mockImplementation(async (callback) => callback(tx)) + + const result = await dbHelpers.undeployWorkflow({ workflowId: mockWorkflowId }) + + expect(result).toEqual({ + success: false, + error: 'Workflow not found', + }) + expect(update).not.toHaveBeenCalled() + }) + }) + describe('error handling and edge cases', () => { it('should handle very large workflow data', async () => { const blocks: Record> = {} diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts index c8f4d883c38..e9b409c8c67 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -25,6 +25,34 @@ const logger = createLogger('WorkflowDBHelpers') export type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types' export type WorkflowDeploymentVersion = InferSelectModel +function hasReturnedRows(result: unknown): boolean { + if (Array.isArray(result)) return result.length > 0 + + if (result && typeof result === 'object') { + const rows = 'rows' in result ? result.rows : undefined + if (Array.isArray(rows)) return rows.length > 0 + } + + return Boolean(result) +} + +async function lockWorkflowForUpdate(tx: DbOrTx, workflowId: string): Promise { + const query = tx.select({ id: workflow.id }).from(workflow).where(eq(workflow.id, workflowId)) + + if ('limit' in query && typeof query.limit === 'function') { + const limited = query.limit(1) + const rows = + 'for' in limited && typeof limited.for === 'function' + ? await limited.for('update') + : await limited + return hasReturnedRows(rows) + } + + const rows = await query + + return hasReturnedRows(rows) +} + export interface WorkflowDeploymentVersionResponse { id: string version: number @@ -343,16 +371,17 @@ async function migrateCredentialIds( * has not been migrated to normalized tables yet. */ export async function loadWorkflowFromNormalizedTables( - workflowId: string + workflowId: string, + externalTx?: DbOrTx ): Promise { - const raw = await loadWorkflowFromNormalizedTablesRaw(workflowId) + const raw = await loadWorkflowFromNormalizedTablesRaw(workflowId, externalTx) if (!raw) return null const { blocks: finalBlocks, migrated } = await applyBlockMigrations(raw.blocks, raw.workspaceId) if (migrated) { Promise.resolve().then(() => - persistMigratedBlocks(workflowId, raw.blocks, finalBlocks, raw.blockUpdatedAt) + persistMigratedBlocks(workflowId, raw.blocks, finalBlocks, raw.blockUpdatedAtById) ) } @@ -382,12 +411,68 @@ export async function loadWorkflowFromNormalizedTables( } } +export async function loadWorkflowDeploymentSnapshot( + workflowId: string, + externalTx?: DbOrTx +): Promise { + const loadSnapshot = async (tx: DbOrTx) => { + const [normalizedData, [workflowRecord]] = await Promise.all([ + loadWorkflowFromNormalizedTables(workflowId, tx), + tx + .select({ variables: workflow.variables }) + .from(workflow) + .where(eq(workflow.id, workflowId)) + .limit(1), + ]) + + if (!normalizedData) return null + + return buildWorkflowDeploymentSnapshot(normalizedData, workflowRecord?.variables) + } + + if (externalTx) { + return loadSnapshot(externalTx) + } + + return db.transaction(async (tx) => { + await tx.execute(sql`SET TRANSACTION ISOLATION LEVEL REPEATABLE READ`) + return loadSnapshot(tx) + }) +} + +export function buildWorkflowDeploymentSnapshot( + normalizedData: NormalizedWorkflowData, + variables: unknown +): WorkflowState { + return { + blocks: normalizedData.blocks, + edges: normalizedData.edges, + loops: normalizedData.loops, + parallels: normalizedData.parallels, + variables: (variables as WorkflowState['variables']) || {}, + lastSaved: Date.now(), + } +} + export async function saveWorkflowToNormalizedTables( workflowId: string, state: WorkflowState, externalTx?: DbOrTx ): Promise<{ success: boolean; error?: string }> { - return saveWorkflowToNormalizedTablesRaw(workflowId, state, externalTx) + if (externalTx) { + return saveWorkflowToNormalizedTablesRaw(workflowId, state, externalTx) + } + + try { + return await db.transaction(async (tx) => { + await lockWorkflowForUpdate(tx, workflowId) + return saveWorkflowToNormalizedTablesRaw(workflowId, state, tx) + }) + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to save workflow state' + logger.error(`Error saving workflow ${workflowId} to normalized tables:`, error) + return { success: false, error: message } + } } export async function workflowExistsInNormalizedTables(workflowId: string): Promise { @@ -406,44 +491,77 @@ export async function workflowExistsInNormalizedTables(workflowId: string): Prom } } +type DeployWorkflowValidationResult = + | { success: true } + | { success: false; error: string; errorCode?: 'validation' } + export async function deployWorkflow(params: { workflowId: string deployedBy: string workflowName?: string + workflowState?: WorkflowState + validateWorkflowState?: ( + workflowState: WorkflowState + ) => DeployWorkflowValidationResult | Promise + onDeployTransaction?: ( + tx: DbOrTx, + result: { deploymentVersionId: string; version: number; previousVersionId?: string } + ) => Promise }): Promise<{ success: boolean version?: number deploymentVersionId?: string deployedAt?: Date - currentState?: any + previousVersionId?: string + currentState?: WorkflowState error?: string + errorCode?: 'validation' | 'not_found' }> { const { workflowId, deployedBy, workflowName } = params try { - const normalizedData = await loadWorkflowFromNormalizedTables(workflowId) - if (!normalizedData) { - return { success: false, error: 'Failed to load workflow state' } - } + const now = new Date() + let currentState: WorkflowState | null = null - const [workflowRecord] = await db - .select({ variables: workflow.variables }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) + const deployedVersion = await db.transaction(async (tx) => { + if (!(await lockWorkflowForUpdate(tx, workflowId))) { + return { + success: false as const, + error: 'Workflow not found', + errorCode: 'not_found' as const, + } + } - const currentState = { - blocks: normalizedData.blocks, - edges: normalizedData.edges, - loops: normalizedData.loops, - parallels: normalizedData.parallels, - variables: workflowRecord?.variables || undefined, - lastSaved: Date.now(), - } + currentState = params.workflowState ?? (await loadWorkflowDeploymentSnapshot(workflowId, tx)) + if (!currentState) { + return { + success: false as const, + error: 'Failed to load workflow state', + errorCode: 'validation' as const, + } + } - const now = new Date() + const validationError = await params.validateWorkflowState?.(currentState) + if (validationError && !validationError.success) { + return { + success: false as const, + error: validationError.error, + errorCode: validationError.errorCode, + } + } + + const [currentActiveVersion] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.isActive, true) + ) + ) + .limit(1) + const previousVersionId = currentActiveVersion?.id - const deployedVersion = await db.transaction(async (tx) => { const [{ maxVersion }] = await tx .select({ maxVersion: sql`COALESCE(MAX("version"), 0)` }) .from(workflowDeploymentVersion) @@ -474,9 +592,33 @@ export async function deployWorkflow(params: { await tx.update(workflow).set(updateData).where(eq(workflow.id, workflowId)) - return { version: nextVersion, deploymentVersionId } + await params.onDeployTransaction?.(tx, { + deploymentVersionId, + version: nextVersion, + previousVersionId, + }) + + return { + success: true as const, + version: nextVersion, + deploymentVersionId, + previousVersionId, + currentState, + } }) + if (!deployedVersion.success) { + return { + success: false, + error: deployedVersion.error, + errorCode: deployedVersion.errorCode, + } + } + const deployedState = deployedVersion.currentState + if (!deployedState) { + return { success: false, error: 'Failed to load workflow state' } + } + logger.info(`Deployed workflow ${workflowId} as v${deployedVersion.version}`) if (workflowName) { @@ -484,7 +626,7 @@ export async function deployWorkflow(params: { const { PlatformEvents } = await import('@/lib/core/telemetry') const blockTypeCounts: Record = {} - for (const block of Object.values(currentState.blocks)) { + for (const block of Object.values(deployedState.blocks)) { const blockType = block.type || 'unknown' blockTypeCounts[blockType] = (blockTypeCounts[blockType] || 0) + 1 } @@ -492,11 +634,11 @@ export async function deployWorkflow(params: { PlatformEvents.workflowDeployed({ workflowId, workflowName, - blocksCount: Object.keys(currentState.blocks).length, - edgesCount: currentState.edges.length, + blocksCount: Object.keys(deployedState.blocks).length, + edgesCount: deployedState.edges.length, version: deployedVersion.version, - loopsCount: Object.keys(currentState.loops).length, - parallelsCount: Object.keys(currentState.parallels).length, + loopsCount: Object.keys(deployedState.loops).length, + parallelsCount: Object.keys(deployedState.parallels).length, blockTypes: JSON.stringify(blockTypeCounts), }) } catch (telemetryError) { @@ -508,8 +650,9 @@ export async function deployWorkflow(params: { success: true, version: deployedVersion.version, deploymentVersionId: deployedVersion.deploymentVersionId, + previousVersionId: deployedVersion.previousVersionId, deployedAt: now, - currentState, + currentState: deployedState, } } catch (error) { logger.error(`Error deploying workflow ${workflowId}:`, error) @@ -668,13 +811,27 @@ export function regenerateWorkflowStateIds(state: RegenerateStateInput): Regener } } -export async function undeployWorkflow(params: { workflowId: string; tx?: DbOrTx }): Promise<{ +export async function undeployWorkflow(params: { + workflowId: string + tx?: DbOrTx + onUndeployTransaction?: (tx: DbOrTx, result: { deploymentVersionIds: string[] }) => Promise +}): Promise<{ success: boolean error?: string }> { const { workflowId, tx } = params const executeUndeploy = async (dbCtx: DbOrTx) => { + if (!(await lockWorkflowForUpdate(dbCtx, workflowId))) { + throw new Error('Workflow not found') + } + + const deploymentVersions = await dbCtx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where(eq(workflowDeploymentVersion.workflowId, workflowId)) + const deploymentVersionIds = deploymentVersions.map((version) => version.id) + const { deleteSchedulesForWorkflow } = await import('@/lib/workflows/schedules/deploy') await deleteSchedulesForWorkflow(workflowId, dbCtx) @@ -687,6 +844,8 @@ export async function undeployWorkflow(params: { workflowId: string; tx?: DbOrTx .update(workflow) .set({ isDeployed: false, deployedAt: null }) .where(eq(workflow.id, workflowId)) + + await params.onUndeployTransaction?.(dbCtx, { deploymentVersionIds }) } try { @@ -712,33 +871,55 @@ export async function undeployWorkflow(params: { workflowId: string; tx?: DbOrTx export async function activateWorkflowVersion(params: { workflowId: string version: number + onActivateTransaction?: ( + tx: DbOrTx, + result: { deploymentVersionId: string; previousVersionId?: string } + ) => Promise }): Promise<{ success: boolean deployedAt?: Date state?: unknown + previousVersionId?: string error?: string }> { const { workflowId, version } = params try { - const [versionData] = await db - .select({ id: workflowDeploymentVersion.id, state: workflowDeploymentVersion.state }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.version, version) + const now = new Date() + let versionState: unknown + + const result = await db.transaction(async (tx) => { + if (!(await lockWorkflowForUpdate(tx, workflowId))) { + return { success: false as const, error: 'Workflow not found' } + } + + const [currentActiveVersion] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.isActive, true) + ) ) - ) - .limit(1) + .limit(1) - if (!versionData) { - return { success: false, error: 'Deployment version not found' } - } + const [versionData] = await tx + .select({ id: workflowDeploymentVersion.id, state: workflowDeploymentVersion.state }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.version, version) + ) + ) + .limit(1) - const now = new Date() + if (!versionData) { + return { success: false as const, error: 'Deployment version not found' } + } + versionState = versionData.state - await db.transaction(async (tx) => { await tx .update(workflowDeploymentVersion) .set({ isActive: false }) @@ -763,14 +944,26 @@ export async function activateWorkflowVersion(params: { .update(workflow) .set({ isDeployed: true, deployedAt: now }) .where(eq(workflow.id, workflowId)) + + await params.onActivateTransaction?.(tx, { + deploymentVersionId: versionData.id, + previousVersionId: currentActiveVersion?.id, + }) + + return { success: true as const, previousVersionId: currentActiveVersion?.id } }) + if (!result.success) { + return { success: false, error: result.error } + } + logger.info(`Activated version ${version} for workflow ${workflowId}`) return { success: true, deployedAt: now, - state: versionData.state, + state: versionState, + previousVersionId: result.previousVersionId, } } catch (error) { logger.error(`Error activating version ${version} for workflow ${workflowId}:`, error) @@ -788,29 +981,47 @@ export async function activateWorkflowVersionById(params: { success: boolean deployedAt?: Date state?: unknown + previousVersionId?: string error?: string }> { const { workflowId, deploymentVersionId } = params try { - const [versionData] = await db - .select({ id: workflowDeploymentVersion.id, state: workflowDeploymentVersion.state }) - .from(workflowDeploymentVersion) - .where( - and( - eq(workflowDeploymentVersion.workflowId, workflowId), - eq(workflowDeploymentVersion.id, deploymentVersionId) + const now = new Date() + let versionState: unknown + + const result = await db.transaction(async (tx) => { + if (!(await lockWorkflowForUpdate(tx, workflowId))) { + return { success: false as const, error: 'Workflow not found' } + } + + const [currentActiveVersion] = await tx + .select({ id: workflowDeploymentVersion.id }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.isActive, true) + ) ) - ) - .limit(1) + .limit(1) - if (!versionData) { - return { success: false, error: 'Deployment version not found' } - } + const [versionData] = await tx + .select({ id: workflowDeploymentVersion.id, state: workflowDeploymentVersion.state }) + .from(workflowDeploymentVersion) + .where( + and( + eq(workflowDeploymentVersion.workflowId, workflowId), + eq(workflowDeploymentVersion.id, deploymentVersionId) + ) + ) + .limit(1) - const now = new Date() + if (!versionData) { + return { success: false as const, error: 'Deployment version not found' } + } + versionState = versionData.state - await db.transaction(async (tx) => { await tx .update(workflowDeploymentVersion) .set({ isActive: false }) @@ -830,14 +1041,21 @@ export async function activateWorkflowVersionById(params: { .update(workflow) .set({ isDeployed: true, deployedAt: now }) .where(eq(workflow.id, workflowId)) + + return { success: true as const, previousVersionId: currentActiveVersion?.id } }) + if (!result.success) { + return { success: false, error: result.error } + } + logger.info(`Activated deployment version ${deploymentVersionId} for workflow ${workflowId}`) return { success: true, deployedAt: now, - state: versionData.state, + state: versionState, + previousVersionId: result.previousVersionId, } } catch (error) { logger.error( diff --git a/apps/sim/lib/workflows/schedules/deploy.ts b/apps/sim/lib/workflows/schedules/deploy.ts index f413b2665e6..466c4ee171e 100644 --- a/apps/sim/lib/workflows/schedules/deploy.ts +++ b/apps/sim/lib/workflows/schedules/deploy.ts @@ -28,7 +28,7 @@ export interface ScheduleDeployResult { export async function createSchedulesForDeploy( workflowId: string, blocks: Record, - _tx: DbOrTx, + dbCtx: DbOrTx, deploymentVersionId?: string ): Promise { const scheduleBlocks = findScheduleBlocks(blocks) @@ -72,7 +72,7 @@ export async function createSchedulesForDeploy( } | null = null try { - await db.transaction(async (tx) => { + const writeSchedules = async (tx: DbOrTx) => { const currentBlockIds = new Set(validatedBlocks.map((b) => b.blockId)) const existingSchedules = await tx @@ -151,7 +151,13 @@ export async function createSchedulesForDeploy( lastScheduleInfo = { scheduleId: values.id, cronExpression, nextRunAt, timezone } } - }) + } + + if (dbCtx === db || !hasScheduleWriteMethods(dbCtx)) { + await db.transaction(writeSchedules) + } else { + await writeSchedules(dbCtx) + } } catch (error) { logger.error(`Failed to create schedules for workflow ${workflowId}`, error) return { @@ -166,6 +172,15 @@ export async function createSchedulesForDeploy( } } +function hasScheduleWriteMethods(value: DbOrTx): boolean { + const candidate = value as Partial> + return ( + typeof candidate.select === 'function' && + typeof candidate.insert === 'function' && + typeof candidate.delete === 'function' + ) +} + /** * Delete all schedules for a workflow * This should be called within a database transaction during undeploy @@ -204,6 +219,7 @@ export async function cleanupDeploymentVersion(params: { * Only deletes DB records. */ skipExternalCleanup?: boolean + strictExternalCleanup?: boolean }): Promise { const { workflowId, @@ -211,13 +227,15 @@ export async function cleanupDeploymentVersion(params: { requestId, deploymentVersionId, skipExternalCleanup = false, + strictExternalCleanup = false, } = params await cleanupWebhooksForWorkflow( workflowId, workflow, requestId, deploymentVersionId, - skipExternalCleanup + skipExternalCleanup, + strictExternalCleanup ) await deleteSchedulesForWorkflow(workflowId, db, deploymentVersionId) } diff --git a/apps/sim/stores/operation-queue/store.test.ts b/apps/sim/stores/operation-queue/store.test.ts index b86a3da2561..b18439f60cc 100644 --- a/apps/sim/stores/operation-queue/store.test.ts +++ b/apps/sim/stores/operation-queue/store.test.ts @@ -9,6 +9,7 @@ describe('operation queue room gating', () => { vi.clearAllMocks() useOperationQueueStore.setState({ operations: [], + workflowOperationVersions: {}, isProcessing: false, hasOperationError: false, }) @@ -18,6 +19,7 @@ describe('operation queue room gating', () => { afterEach(() => { useOperationQueueStore.setState({ operations: [], + workflowOperationVersions: {}, isProcessing: false, hasOperationError: false, }) @@ -71,4 +73,298 @@ describe('operation queue room gating', () => { useOperationQueueStore.getState().confirmOperation('op-1') }) + + it('reports pending operations per workflow', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + expect(useOperationQueueStore.getState().hasPendingOperations('workflow-a')).toBe(true) + expect(useOperationQueueStore.getState().hasPendingOperations('workflow-b')).toBe(false) + }) + + it('tracks local operation activity per workflow', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + expect(useOperationQueueStore.getState().workflowOperationVersions['workflow-a']).toBe(1) + expect( + useOperationQueueStore.getState().workflowOperationVersions['workflow-b'] + ).toBeUndefined() + }) + + it('coalesces pending subblock updates to the latest value for the same field', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'subblock-update', + target: 'subblock', + payload: { + blockId: 'block-1', + subblockId: 'prompt', + value: 'old value', + }, + }, + }) + useOperationQueueStore.getState().addToQueue({ + id: 'op-2', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'subblock-update', + target: 'subblock', + payload: { + blockId: 'block-1', + subblockId: 'prompt', + value: 'new value', + }, + }, + }) + + expect(useOperationQueueStore.getState().operations).toEqual([ + expect.objectContaining({ + id: 'op-2', + operation: expect.objectContaining({ + payload: expect.objectContaining({ value: 'new value' }), + }), + }), + ]) + }) + + it('does not coalesce matching subblock updates across workflows', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'subblock-update', + target: 'subblock', + payload: { + blockId: 'block-1', + subblockId: 'prompt', + value: 'workflow a value', + }, + }, + }) + useOperationQueueStore.getState().addToQueue({ + id: 'op-2', + workflowId: 'workflow-b', + userId: 'user-1', + operation: { + operation: 'subblock-update', + target: 'subblock', + payload: { + blockId: 'block-1', + subblockId: 'prompt', + value: 'workflow b value', + }, + }, + }) + + expect(useOperationQueueStore.getState().operations).toEqual([ + expect.objectContaining({ + id: 'op-1', + workflowId: 'workflow-a', + }), + expect.objectContaining({ + id: 'op-2', + workflowId: 'workflow-b', + }), + ]) + }) + + it('coalesces variable field updates without dropping unrelated fields', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'variable-update', + target: 'variable', + payload: { + variableId: 'variable-1', + field: 'value', + value: 'old value', + }, + }, + }) + useOperationQueueStore.getState().addToQueue({ + id: 'op-2', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'variable-update', + target: 'variable', + payload: { + variableId: 'variable-1', + field: 'name', + value: 'Variable Name', + }, + }, + }) + useOperationQueueStore.getState().addToQueue({ + id: 'op-3', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'variable-update', + target: 'variable', + payload: { + variableId: 'variable-1', + field: 'value', + value: 'new value', + }, + }, + }) + + expect(useOperationQueueStore.getState().operations).toEqual([ + expect.objectContaining({ + id: 'op-2', + operation: expect.objectContaining({ + payload: expect.objectContaining({ field: 'name', value: 'Variable Name' }), + }), + }), + expect.objectContaining({ + id: 'op-3', + operation: expect.objectContaining({ + payload: expect.objectContaining({ field: 'value', value: 'new value' }), + }), + }), + ]) + }) + + it('does not coalesce matching variable updates across workflows', () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'variable-update', + target: 'variable', + payload: { + variableId: 'variable-1', + field: 'value', + value: 'workflow a value', + }, + }, + }) + useOperationQueueStore.getState().addToQueue({ + id: 'op-2', + workflowId: 'workflow-b', + userId: 'user-1', + operation: { + operation: 'variable-update', + target: 'variable', + payload: { + variableId: 'variable-1', + field: 'value', + value: 'workflow b value', + }, + }, + }) + + expect(useOperationQueueStore.getState().operations).toEqual([ + expect.objectContaining({ + id: 'op-1', + workflowId: 'workflow-a', + }), + expect.objectContaining({ + id: 'op-2', + workflowId: 'workflow-b', + }), + ]) + }) + + it('waits for matching workflow operations to drain', async () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + const drained = useOperationQueueStore.getState().waitForWorkflowOperations('workflow-a') + useOperationQueueStore.getState().confirmOperation('op-1') + + await expect(drained).resolves.toBe(true) + }) + + it('does not wait on operations from other workflows', async () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + await expect( + useOperationQueueStore.getState().waitForWorkflowOperations('workflow-b') + ).resolves.toBe(true) + }) + + it('stops waiting when an operation error is reported', async () => { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + const drained = useOperationQueueStore.getState().waitForWorkflowOperations('workflow-a') + useOperationQueueStore.setState({ hasOperationError: true }) + + await expect(drained).resolves.toBe(false) + }) + + it('stops waiting when matching workflow operations do not drain before timeout', async () => { + vi.useFakeTimers() + try { + useOperationQueueStore.getState().addToQueue({ + id: 'op-1', + workflowId: 'workflow-a', + userId: 'user-1', + operation: { + operation: 'replace-state', + target: 'workflow', + payload: { state: {} }, + }, + }) + + const drained = useOperationQueueStore.getState().waitForWorkflowOperations('workflow-a', 100) + await vi.advanceTimersByTimeAsync(100) + + await expect(drained).resolves.toBe(false) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/apps/sim/stores/operation-queue/store.ts b/apps/sim/stores/operation-queue/store.ts index 052bf9c1213..bf0d9533e85 100644 --- a/apps/sim/stores/operation-queue/store.ts +++ b/apps/sim/stores/operation-queue/store.ts @@ -29,6 +29,7 @@ const RETRY_DELAY_BASE_MS = 1000 const retryTimeouts = new Map() const operationTimeouts = new Map() +const DEFAULT_WORKFLOW_DRAIN_TIMEOUT_MS = 20000 let emitWorkflowOperation: | (( @@ -95,29 +96,32 @@ let currentRegisteredWorkflowId: string | null = null export const useOperationQueueStore = create((set, get) => ({ operations: [], + workflowOperationVersions: {}, isProcessing: false, hasOperationError: false, addToQueue: (operation) => { + set((state) => ({ + workflowOperationVersions: { + ...state.workflowOperationVersions, + [operation.workflowId]: (state.workflowOperationVersions[operation.workflowId] ?? 0) + 1, + }, + })) + + let shouldDropPendingOperation = (_op: QueuedOperation) => false + if ( operation.operation.operation === 'subblock-update' && operation.operation.target === 'subblock' ) { const { blockId, subblockId } = operation.operation.payload - set((state) => ({ - operations: [ - ...state.operations.filter( - (op) => - !( - op.status === 'pending' && - op.operation.operation === 'subblock-update' && - op.operation.target === 'subblock' && - op.operation.payload?.blockId === blockId && - op.operation.payload?.subblockId === subblockId - ) - ), - ], - })) + shouldDropPendingOperation = (op) => + op.status === 'pending' && + op.workflowId === operation.workflowId && + op.operation.operation === 'subblock-update' && + op.operation.target === 'subblock' && + op.operation.payload?.blockId === blockId && + op.operation.payload?.subblockId === subblockId } if ( @@ -125,20 +129,13 @@ export const useOperationQueueStore = create((set, get) => operation.operation.target === 'variable' ) { const { variableId, field } = operation.operation.payload - set((state) => ({ - operations: [ - ...state.operations.filter( - (op) => - !( - op.status === 'pending' && - op.operation.operation === 'variable-update' && - op.operation.target === 'variable' && - op.operation.payload?.variableId === variableId && - op.operation.payload?.field === field - ) - ), - ], - })) + shouldDropPendingOperation = (op) => + op.status === 'pending' && + op.workflowId === operation.workflowId && + op.operation.operation === 'variable-update' && + op.operation.target === 'variable' && + op.operation.payload?.variableId === variableId && + op.operation.payload?.field === field } const state = get() @@ -154,6 +151,7 @@ export const useOperationQueueStore = create((set, get) => const duplicateContent = state.operations.find( (op) => + !shouldDropPendingOperation(op) && op.operation.operation === operation.operation.operation && op.operation.target === operation.operation.target && op.workflowId === operation.workflowId && @@ -194,7 +192,7 @@ export const useOperationQueueStore = create((set, get) => }) set((state) => ({ - operations: [...state.operations, queuedOp], + operations: [...state.operations.filter((op) => !shouldDropPendingOperation(op)), queuedOp], })) get().processNextOperation() @@ -412,6 +410,42 @@ export const useOperationQueueStore = create((set, get) => operationTimeouts.set(nextOperation.id, timeoutId) }, + hasPendingOperations: (workflowId: string) => { + return get().operations.some((op) => op.workflowId === workflowId) + }, + + waitForWorkflowOperations: ( + workflowId: string, + timeoutMs = DEFAULT_WORKFLOW_DRAIN_TIMEOUT_MS + ) => { + if (!get().hasPendingOperations(workflowId)) { + return Promise.resolve(true) + } + + return new Promise((resolve) => { + let unsubscribe = () => {} + const timeout = setTimeout(() => { + unsubscribe() + resolve(false) + }, timeoutMs) + + unsubscribe = useOperationQueueStore.subscribe((state) => { + if (state.hasOperationError) { + clearTimeout(timeout) + unsubscribe() + resolve(false) + return + } + + if (!state.operations.some((op) => op.workflowId === workflowId)) { + clearTimeout(timeout) + unsubscribe() + resolve(true) + } + }) + }) + }, + cancelOperationsForBlock: (blockId: string) => { logger.debug('Canceling all operations for block', { blockId }) @@ -598,6 +632,8 @@ export function useOperationQueue() { confirmOperation: actions.confirmOperation, failOperation: actions.failOperation, processNextOperation: actions.processNextOperation, + hasPendingOperations: actions.hasPendingOperations, + waitForWorkflowOperations: actions.waitForWorkflowOperations, cancelOperationsForBlock: actions.cancelOperationsForBlock, cancelOperationsForVariable: actions.cancelOperationsForVariable, triggerOfflineMode: actions.triggerOfflineMode, diff --git a/apps/sim/stores/operation-queue/types.ts b/apps/sim/stores/operation-queue/types.ts index 7122e6a40a6..e59731a82b5 100644 --- a/apps/sim/stores/operation-queue/types.ts +++ b/apps/sim/stores/operation-queue/types.ts @@ -14,6 +14,7 @@ export interface QueuedOperation { export interface OperationQueueState { operations: QueuedOperation[] + workflowOperationVersions: Record isProcessing: boolean hasOperationError: boolean @@ -22,6 +23,8 @@ export interface OperationQueueState { failOperation: (operationId: string, retryable?: boolean) => void handleOperationTimeout: (operationId: string) => void processNextOperation: () => void + hasPendingOperations: (workflowId: string) => boolean + waitForWorkflowOperations: (workflowId: string, timeoutMs?: number) => Promise cancelOperationsForBlock: (blockId: string) => void cancelOperationsForVariable: (variableId: string) => void diff --git a/apps/sim/stores/workflow-diff/store.ts b/apps/sim/stores/workflow-diff/store.ts index b97f2951894..7f5ebce96d1 100644 --- a/apps/sim/stores/workflow-diff/store.ts +++ b/apps/sim/stores/workflow-diff/store.ts @@ -18,6 +18,7 @@ import { createBatchedUpdater, getLatestUserMessageId, persistWorkflowStateToServer, + WORKFLOW_DIFF_SETTLED_EVENT, } from './utils' const logger = createLogger('WorkflowDiffStore') @@ -60,6 +61,11 @@ function isEmptyDiffAnalysis( return !hasBlockChanges && !hasEdgeChanges && !hasFieldChanges } +function notifyDiffSettled(workflowId: string | null | undefined): void { + if (!workflowId || typeof window === 'undefined') return + window.dispatchEvent(new CustomEvent(WORKFLOW_DIFF_SETTLED_EVENT, { detail: { workflowId } })) +} + export const useWorkflowDiffStore = create()( devtools( (set, get) => { @@ -74,6 +80,10 @@ export const useWorkflowDiffStore = create { @@ -340,6 +351,7 @@ export const useWorkflowDiffStore = create { @@ -372,7 +384,8 @@ export const useWorkflowDiffStore = create { - logger.error('Failed to persist baseline workflow state:', error) - }) + const pendingGenerationBeforePersist = + get().pendingExternalUpdates[baselineWorkflowId] ?? 0 + const persisted = await persistWorkflowStateToServer(baselineWorkflowId, baselineWorkflow) + if (!persisted) { + logger.error('Failed to persist baseline workflow state') + if ( + (get().pendingExternalUpdates[baselineWorkflowId] ?? 0) <= + pendingGenerationBeforePersist + ) { + get().clearExternalUpdatePending(baselineWorkflowId) + } + get().setWorkflowReconciliationInProgress(baselineWorkflowId, false) + get().setWorkflowReconciliationError( + baselineWorkflowId, + 'Failed to save rejected copilot changes. Refresh and try again.' + ) + if ( + (get().pendingExternalUpdates[baselineWorkflowId] ?? 0) > + pendingGenerationBeforePersist + ) { + notifyDiffSettled(baselineWorkflowId) + } + return + } + if ( + (get().pendingExternalUpdates[baselineWorkflowId] ?? 0) <= + pendingGenerationBeforePersist + ) { + get().clearExternalUpdatePending(baselineWorkflowId) + } if (_triggerMessageId) { fetch(COPILOT_STATS_API_PATH, { method: 'POST', @@ -433,6 +470,9 @@ export const useWorkflowDiffStore = create { @@ -505,6 +545,65 @@ export const useWorkflowDiffStore = create { + set((state) => ({ + remoteUpdateVersions: { + ...state.remoteUpdateVersions, + [workflowId]: (state.remoteUpdateVersions[workflowId] ?? 0) + 1, + }, + reconciliationErrors: Object.fromEntries( + Object.entries(state.reconciliationErrors).filter(([id]) => id !== workflowId) + ), + })) + }, + + markExternalUpdatePending: (workflowId) => { + const current = get() + set({ + pendingExternalUpdates: { + ...current.pendingExternalUpdates, + [workflowId]: (current.pendingExternalUpdates[workflowId] ?? 0) + 1, + }, + remoteUpdateVersions: { + ...current.remoteUpdateVersions, + [workflowId]: (current.remoteUpdateVersions[workflowId] ?? 0) + 1, + }, + reconciliationErrors: Object.fromEntries( + Object.entries(current.reconciliationErrors).filter(([id]) => id !== workflowId) + ), + }) + }, + + clearExternalUpdatePending: (workflowId) => { + set((state) => { + const { [workflowId]: _removed, ...pendingExternalUpdates } = + state.pendingExternalUpdates + return { pendingExternalUpdates } + }) + }, + + setWorkflowReconciliationInProgress: (workflowId, isReconciling) => { + set((state) => { + const { [workflowId]: _removed, ...reconcilingWorkflows } = state.reconcilingWorkflows + return { + reconcilingWorkflows: isReconciling + ? { ...reconcilingWorkflows, [workflowId]: true } + : reconcilingWorkflows, + } + }) + }, + + setWorkflowReconciliationError: (workflowId, error) => { + set((state) => { + const { [workflowId]: _removed, ...reconciliationErrors } = state.reconciliationErrors + return { + reconciliationErrors: error + ? { ...reconciliationErrors, [workflowId]: error } + : reconciliationErrors, + } + }) + }, } }, { name: 'workflow-diff-store' } diff --git a/apps/sim/stores/workflow-diff/types.ts b/apps/sim/stores/workflow-diff/types.ts index b6cc01203fd..2beb4253c76 100644 --- a/apps/sim/stores/workflow-diff/types.ts +++ b/apps/sim/stores/workflow-diff/types.ts @@ -10,6 +10,10 @@ export interface WorkflowDiffState { diffAnalysis: DiffAnalysis | null diffMetadata: WorkflowDiff['metadata'] | null diffError?: string | null + pendingExternalUpdates: Record + remoteUpdateVersions: Record + reconcilingWorkflows: Record + reconciliationErrors: Record _triggerMessageId?: string | null } @@ -37,5 +41,10 @@ export interface WorkflowDiffActions { acceptChanges: (options?: DiffActionOptions) => Promise rejectChanges: (options?: DiffActionOptions) => Promise reapplyDiffMarkers: () => void + markRemoteUpdateSeen: (workflowId: string) => void + markExternalUpdatePending: (workflowId: string) => void + clearExternalUpdatePending: (workflowId: string) => void + setWorkflowReconciliationInProgress: (workflowId: string, isReconciling: boolean) => void + setWorkflowReconciliationError: (workflowId: string, error: string | null) => void _batchedStateUpdate: (updates: Partial) => void } diff --git a/apps/sim/stores/workflow-diff/utils.test.ts b/apps/sim/stores/workflow-diff/utils.test.ts new file mode 100644 index 00000000000..c40787ce678 --- /dev/null +++ b/apps/sim/stores/workflow-diff/utils.test.ts @@ -0,0 +1,77 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it } from 'vitest' +import { useVariablesStore } from '@/stores/variables/store' +import { applyWorkflowVariablesToStore } from '@/stores/workflow-diff/utils' + +describe('applyWorkflowVariablesToStore', () => { + beforeEach(() => { + useVariablesStore.setState({ + variables: {}, + isLoading: false, + error: null, + isEditing: null, + }) + }) + + it('hydrates variables for the target workflow and preserves other workflows', () => { + useVariablesStore.setState({ + variables: { + old: { + id: 'old', + workflowId: 'workflow-a', + name: 'oldValue', + type: 'plain', + value: 'stale', + }, + other: { + id: 'other', + workflowId: 'workflow-b', + name: 'otherValue', + type: 'plain', + value: 'kept', + }, + }, + }) + + applyWorkflowVariablesToStore('workflow-a', { + next: { + id: 'next', + name: 'nextValue', + type: 'number', + value: 42, + }, + }) + + expect(useVariablesStore.getState().variables).toEqual({ + other: { + id: 'other', + workflowId: 'workflow-b', + name: 'otherValue', + type: 'plain', + value: 'kept', + }, + next: { + id: 'next', + workflowId: 'workflow-a', + name: 'nextValue', + type: 'number', + value: 42, + }, + }) + }) + + it('preserves null variable values from persisted workflow state', () => { + applyWorkflowVariablesToStore('workflow-a', { + next: { + id: 'next', + name: 'nullableValue', + type: 'object', + value: null, + }, + }) + + expect(useVariablesStore.getState().variables.next.value).toBeNull() + }) +}) diff --git a/apps/sim/stores/workflow-diff/utils.ts b/apps/sim/stores/workflow-diff/utils.ts index f7c8ab7c393..68d93bc1451 100644 --- a/apps/sim/stores/workflow-diff/utils.ts +++ b/apps/sim/stores/workflow-diff/utils.ts @@ -7,6 +7,8 @@ import { type WorkflowStateContractInput, } from '@/lib/api/contracts/workflows' import { stripWorkflowDiffMarkers } from '@/lib/workflows/diff' +import { useVariablesStore } from '@/stores/variables/store' +import type { Variable } from '@/stores/variables/types' import { useWorkflowRegistry } from '../workflows/registry/store' import { useSubBlockStore } from '../workflows/subblock/store' import { mergeSubblockState } from '../workflows/utils' @@ -15,6 +17,7 @@ import type { WorkflowState } from '../workflows/workflow/types' import type { WorkflowDiffState } from './types' const logger = createLogger('WorkflowDiffStore') +export const WORKFLOW_DIFF_SETTLED_EVENT = 'workflow-diff-settled' export function cloneWorkflowState(state: WorkflowState): WorkflowState { return { @@ -58,6 +61,9 @@ export function applyWorkflowStateToStores( workflowStore.replaceWorkflowState(cloned, options) const subBlockValues = extractSubBlockValues(workflowState) useSubBlockStore.getState().setWorkflowValues(workflowId, subBlockValues) + if (Object.hasOwn(workflowState, 'variables')) { + applyWorkflowVariablesToStore(workflowId, workflowState.variables) + } // Verify what's in the store after apply const afterState = workflowStore.getWorkflowState() @@ -67,6 +73,33 @@ export function applyWorkflowStateToStores( }) } +export function applyWorkflowVariablesToStore( + workflowId: string, + variables?: WorkflowState['variables'] | null +) { + const stampedVariables: Record = {} + + Object.entries(variables || {}).forEach(([id, variable]) => { + if (!variable?.name) return + stampedVariables[id] = { + id: variable.id || id, + workflowId, + name: variable.name, + type: variable.type || 'plain', + value: Object.hasOwn(variable, 'value') ? variable.value : '', + } + }) + + useVariablesStore.setState((state) => ({ + variables: { + ...Object.fromEntries( + Object.entries(state.variables).filter(([, variable]) => variable.workflowId !== workflowId) + ), + ...stampedVariables, + }, + })) +} + export function captureBaselineSnapshot(workflowId: string): WorkflowState { const workflowStore = useWorkflowStore.getState() const currentState = workflowStore.getWorkflowState() diff --git a/packages/workflow-persistence/src/load.ts b/packages/workflow-persistence/src/load.ts index 2e6d864b9f7..3f6f8d2de39 100644 --- a/packages/workflow-persistence/src/load.ts +++ b/packages/workflow-persistence/src/load.ts @@ -2,15 +2,15 @@ import { db, workflow, workflowBlocks, workflowEdges, workflowSubflows } from '@ import { createLogger } from '@sim/logger' import type { BlockState, Loop, Parallel } from '@sim/workflow-types/workflow' import { SUBFLOW_TYPES } from '@sim/workflow-types/workflow' -import { and, eq } from 'drizzle-orm' +import { and, eq, isNull } from 'drizzle-orm' import type { Edge } from 'reactflow' -import type { NormalizedWorkflowData } from './types' +import type { DbOrTx, NormalizedWorkflowData } from './types' const logger = createLogger('WorkflowPersistenceLoad') export interface RawNormalizedWorkflow extends NormalizedWorkflowData { workspaceId: string - blockUpdatedAt: Record + blockUpdatedAtById: Record } /** @@ -28,14 +28,16 @@ export interface RawNormalizedWorkflow extends NormalizedWorkflowData { * config on the returned object will silently diverge from the migrated block. */ export async function loadWorkflowFromNormalizedTablesRaw( - workflowId: string + workflowId: string, + externalTx?: DbOrTx ): Promise { try { + const tx = externalTx ?? db const [blocks, edges, subflows, [workflowRow]] = await Promise.all([ - db.select().from(workflowBlocks).where(eq(workflowBlocks.workflowId, workflowId)), - db.select().from(workflowEdges).where(eq(workflowEdges.workflowId, workflowId)), - db.select().from(workflowSubflows).where(eq(workflowSubflows.workflowId, workflowId)), - db + tx.select().from(workflowBlocks).where(eq(workflowBlocks.workflowId, workflowId)), + tx.select().from(workflowEdges).where(eq(workflowEdges.workflowId, workflowId)), + tx.select().from(workflowSubflows).where(eq(workflowSubflows.workflowId, workflowId)), + tx .select({ workspaceId: workflow.workspaceId }) .from(workflow) .where(eq(workflow.id, workflowId)) @@ -51,7 +53,7 @@ export async function loadWorkflowFromNormalizedTablesRaw( } const blocksMap: Record = {} - const blockUpdatedAt: Record = {} + const blockUpdatedAtById: Record = {} blocks.forEach((block) => { const blockData = (block.data ?? {}) as BlockState['data'] @@ -75,7 +77,7 @@ export async function loadWorkflowFromNormalizedTablesRaw( } blocksMap[block.id] = assembled - blockUpdatedAt[block.id] = block.updatedAt + blockUpdatedAtById[block.id] = block.updatedAt ?? null }) const edgesArray: Edge[] = edges.map((edge) => ({ @@ -154,7 +156,7 @@ export async function loadWorkflowFromNormalizedTablesRaw( parallels, isFromNormalizedTables: true, workspaceId: workflowRow.workspaceId, - blockUpdatedAt, + blockUpdatedAtById, } } catch (error) { logger.error(`Error loading workflow ${workflowId} from normalized tables:`, error) @@ -166,11 +168,23 @@ export async function persistMigratedBlocks( workflowId: string, originalBlocks: Record, migratedBlocks: Record, - originalBlockUpdatedAt: Record = {} + blockUpdatedAtById: Record = {} ): Promise { try { for (const [blockId, block] of Object.entries(migratedBlocks)) { if (block !== originalBlocks[blockId]) { + const hasExpectedUpdatedAt = Object.hasOwn(blockUpdatedAtById, blockId) + const expectedUpdatedAt = blockUpdatedAtById[blockId] + const whereClause = hasExpectedUpdatedAt + ? and( + eq(workflowBlocks.id, blockId), + eq(workflowBlocks.workflowId, workflowId), + expectedUpdatedAt === null + ? isNull(workflowBlocks.updatedAt) + : eq(workflowBlocks.updatedAt, expectedUpdatedAt) + ) + : and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)) + await db .update(workflowBlocks) .set({ @@ -178,15 +192,7 @@ export async function persistMigratedBlocks( data: block.data, updatedAt: new Date(), }) - .where( - originalBlockUpdatedAt[blockId] - ? and( - eq(workflowBlocks.id, blockId), - eq(workflowBlocks.workflowId, workflowId), - eq(workflowBlocks.updatedAt, originalBlockUpdatedAt[blockId]) - ) - : and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)) - ) + .where(whereClause) } } } catch (err) {