fix(workflows): harden deploy rollback recovery

PlaneInABottle · PlaneInABottle · commit 9755e6a2eb8a · 2026-03-21T14:54:38.000+03:00
diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/workflows/[id]/deploy/route.test.ts
@@ -6,11 +6,14 @@ import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
+  mockActivateWorkflowVersionById,
   mockCleanupWebhooksForWorkflow,
+  mockCleanupDeploymentVersion,
   mockRecordAudit,
   mockDbLimit,
   mockDbOrderBy,
   mockDbFrom,
+  mockDbInnerJoin,
   mockDbSelect,
   mockDbSet,
   mockDbUpdate,
@@ -19,18 +22,24 @@ const {
   mockDeployWorkflow,
   mockLoadWorkflowFromNormalizedTables,
   mockRemoveMcpToolsForWorkflow,
+  mockReactivateWorkflowVersionForRollback,
+  mockRestorePreviousVersionWebhooks,
   mockSaveTriggerWebhooksForDeploy,
   mockSyncMcpToolsForWorkflow,
+  mockDeleteDeploymentVersionById,
   mockUndeployWorkflow,
   mockValidatePublicApiAllowed,
   mockValidateWorkflowAccess,
   mockValidateWorkflowPermissions,
 } = vi.hoisted(() => ({
+  mockActivateWorkflowVersionById: vi.fn(),
   mockCleanupWebhooksForWorkflow: vi.fn(),
+  mockCleanupDeploymentVersion: vi.fn(),
   mockRecordAudit: vi.fn(),
   mockDbLimit: vi.fn(),
   mockDbOrderBy: vi.fn(),
   mockDbFrom: vi.fn(),
+  mockDbInnerJoin: vi.fn(),
   mockDbSelect: vi.fn(),
   mockDbSet: vi.fn(),
   mockDbUpdate: vi.fn(),
@@ -39,8 +48,11 @@ const {
   mockDeployWorkflow: vi.fn(),
   mockLoadWorkflowFromNormalizedTables: vi.fn(),
   mockRemoveMcpToolsForWorkflow: vi.fn(),
+  mockReactivateWorkflowVersionForRollback: vi.fn(),
+  mockRestorePreviousVersionWebhooks: vi.fn(),
   mockSaveTriggerWebhooksForDeploy: vi.fn(),
   mockSyncMcpToolsForWorkflow: vi.fn(),
+  mockDeleteDeploymentVersionById: vi.fn(),
   mockUndeployWorkflow: vi.fn(),
   mockValidatePublicApiAllowed: vi.fn(),
   mockValidateWorkflowAccess: vi.fn(),
@@ -65,7 +77,7 @@ vi.mock('@/lib/core/utils/request', () => ({
 
 vi.mock('@sim/db', () => ({
   db: { select: mockDbSelect, update: mockDbUpdate },
-  workflow: { variables: 'variables', id: 'id' },
+  workflow: { variables: 'variables', id: 'id', deployedAt: 'deployedAt' },
   workflowDeploymentVersion: {
     state: 'state',
     workflowId: 'workflowId',
@@ -88,23 +100,28 @@ vi.mock('drizzle-orm', async (importOriginal) => {
 vi.mock('@/lib/workflows/persistence/utils', () => ({
   loadWorkflowFromNormalizedTables: (...args: unknown[]) =>
     mockLoadWorkflowFromNormalizedTables(...args),
+  deleteDeploymentVersionById: (...args: unknown[]) => mockDeleteDeploymentVersionById(...args),
   deployWorkflow: (...args: unknown[]) => mockDeployWorkflow(...args),
+  reactivateWorkflowVersionForRollback: (...args: unknown[]) =>
+    mockReactivateWorkflowVersionForRollback(...args),
   undeployWorkflow: (...args: unknown[]) => mockUndeployWorkflow(...args),
+  activateWorkflowVersionById: (...args: unknown[]) => mockActivateWorkflowVersionById(...args),
 }))
 
 vi.mock('@/lib/workflows/comparison', () => ({
   hasWorkflowChanged: vi.fn().mockReturnValue(false),
 }))
 
 vi.mock('@/lib/workflows/schedules', () => ({
-  cleanupDeploymentVersion: vi.fn(),
+  cleanupDeploymentVersion: (...args: unknown[]) => mockCleanupDeploymentVersion(...args),
   createSchedulesForDeploy: (...args: unknown[]) => mockCreateSchedulesForDeploy(...args),
   validateWorkflowSchedules: vi.fn().mockReturnValue({ isValid: true }),
 }))
 
 vi.mock('@/lib/webhooks/deploy', () => ({
   cleanupWebhooksForWorkflow: (...args: unknown[]) => mockCleanupWebhooksForWorkflow(...args),
-  restorePreviousVersionWebhooks: vi.fn(),
+  restorePreviousVersionWebhooks: (...args: unknown[]) =>
+    mockRestorePreviousVersionWebhooks(...args),
   saveTriggerWebhooksForDeploy: (...args: unknown[]) => mockSaveTriggerWebhooksForDeploy(...args),
 }))
 
@@ -130,20 +147,26 @@ describe('Workflow deploy route', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     mockDbSelect.mockReturnValue({ from: mockDbFrom })
-    mockDbFrom.mockReturnValue({ where: mockDbWhere })
+    mockDbFrom.mockReturnValue({ where: mockDbWhere, innerJoin: mockDbInnerJoin })
+    mockDbInnerJoin.mockReturnValue({ where: mockDbWhere })
     mockDbWhere.mockReturnValue({ limit: mockDbLimit, orderBy: mockDbOrderBy })
     mockDbOrderBy.mockReturnValue({ limit: mockDbLimit })
     mockDbLimit.mockResolvedValue([])
     mockDbUpdate.mockReturnValue({ set: mockDbSet })
     mockDbSet.mockReturnValue({ where: mockDbWhere })
     mockCleanupWebhooksForWorkflow.mockResolvedValue(undefined)
     mockCreateSchedulesForDeploy.mockResolvedValue({ success: true })
+    mockCleanupDeploymentVersion.mockResolvedValue(undefined)
+    mockDeleteDeploymentVersionById.mockResolvedValue({ success: true })
     mockLoadWorkflowFromNormalizedTables.mockResolvedValue({
       blocks: { 'block-1': { id: 'block-1', type: 'start_trigger', name: 'Start' } },
       edges: [],
       loops: {},
       parallels: {},
     })
+    mockActivateWorkflowVersionById.mockResolvedValue({ success: true })
+    mockReactivateWorkflowVersionForRollback.mockResolvedValue({ success: true })
+    mockRestorePreviousVersionWebhooks.mockResolvedValue(undefined)
     mockSaveTriggerWebhooksForDeploy.mockResolvedValue({ success: true, warnings: [] })
     mockRemoveMcpToolsForWorkflow.mockResolvedValue(undefined)
     mockSyncMcpToolsForWorkflow.mockResolvedValue(undefined)
@@ -221,6 +244,94 @@ describe('Workflow deploy route', () => {
     expect(mockRecordAudit).toHaveBeenCalled()
   })
 
+  it('preserves prior deployedAt when failed redeploy rolls back', async () => {
+    mockDbLimit.mockResolvedValue([
+      { id: 'prev-1', deployedAt: new Date('2024-01-01T00:00:00.000Z') },
+    ])
+    mockValidateWorkflowAccess.mockResolvedValue({
+      workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
+      auth: {
+        success: true,
+        userId: 'api-user',
+        userName: 'API Key Actor',
+        userEmail: 'api@example.com',
+        authType: 'api_key',
+      },
+    })
+    mockDeployWorkflow.mockResolvedValue({
+      success: true,
+      deployedAt: '2024-02-01T00:00:00Z',
+      deploymentVersionId: 'dep-failed',
+    })
+    mockSaveTriggerWebhooksForDeploy.mockResolvedValue({
+      success: false,
+      error: { message: 'Failed to save trigger configuration', status: 500 },
+    })
+
+    const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
+      method: 'POST',
+      headers: { 'x-api-key': 'test-key' },
+    })
+    const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) })
+
+    expect(response.status).toBe(500)
+    expect(mockReactivateWorkflowVersionForRollback).toHaveBeenCalledWith({
+      workflowId: 'wf-1',
+      deploymentVersionId: 'prev-1',
+      deployedAt: new Date('2024-01-01T00:00:00.000Z'),
+    })
+    expect(mockActivateWorkflowVersionById).not.toHaveBeenCalled()
+    expect(mockRestorePreviousVersionWebhooks).toHaveBeenCalledWith(
+      expect.objectContaining({
+        workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
+        previousVersionId: 'prev-1',
+        requestId: 'req-123',
+        userId: 'api-user',
+      })
+    )
+    expect(mockDeleteDeploymentVersionById).toHaveBeenCalledWith({
+      workflowId: 'wf-1',
+      deploymentVersionId: 'dep-failed',
+    })
+  })
+
+  it('deletes failed created deployment version when first deploy rollback runs', async () => {
+    mockValidateWorkflowAccess.mockResolvedValue({
+      workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
+      auth: {
+        success: true,
+        userId: 'api-user',
+        userName: 'API Key Actor',
+        userEmail: 'api@example.com',
+        authType: 'api_key',
+      },
+    })
+    mockDeployWorkflow.mockResolvedValue({
+      success: true,
+      deployedAt: '2024-02-01T00:00:00Z',
+      deploymentVersionId: 'dep-failed',
+    })
+    mockSaveTriggerWebhooksForDeploy.mockResolvedValue({
+      success: false,
+      error: { message: 'Failed to save trigger configuration', status: 500 },
+    })
+    mockDbLimit.mockResolvedValue([])
+    mockUndeployWorkflow.mockResolvedValue({ success: true })
+
+    const req = new NextRequest('http://localhost:3000/api/workflows/wf-1/deploy', {
+      method: 'POST',
+      headers: { 'x-api-key': 'test-key' },
+    })
+    const response = await POST(req, { params: Promise.resolve({ id: 'wf-1' }) })
+
+    expect(response.status).toBe(500)
+    expect(mockDeleteDeploymentVersionById).toHaveBeenCalledWith({
+      workflowId: 'wf-1',
+      deploymentVersionId: 'dep-failed',
+    })
+    expect(mockUndeployWorkflow).toHaveBeenCalledWith({ workflowId: 'wf-1' })
+  })
+
   it('allows API-key auth for undeploy using hybrid auth userId', async () => {
     mockValidateWorkflowAccess.mockResolvedValue({
       workflow: { id: 'wf-1', name: 'Test Workflow', workspaceId: 'ws-1' },
diff --git a/apps/sim/app/api/workflows/[id]/deploy/route.ts b/apps/sim/app/api/workflows/[id]/deploy/route.ts
@@ -13,8 +13,10 @@ import {
 } from '@/lib/webhooks/deploy'
 import {
   activateWorkflowVersionById,
+  deleteDeploymentVersionById,
   deployWorkflow,
   loadWorkflowFromNormalizedTables,
+  reactivateWorkflowVersionForRollback,
   undeployWorkflow,
 } from '@/lib/workflows/persistence/utils'
 import {
@@ -115,8 +117,9 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
     }
 
     const [currentActiveVersion] = await db
-      .select({ id: workflowDeploymentVersion.id })
+      .select({ id: workflowDeploymentVersion.id, deployedAt: workflow.deployedAt })
       .from(workflowDeploymentVersion)
+      .innerJoin(workflow, eq(workflowDeploymentVersion.workflowId, workflow.id))
       .where(
         and(
           eq(workflowDeploymentVersion.workflowId, id),
@@ -125,8 +128,9 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
       )
       .limit(1)
     const previousVersionId = currentActiveVersion?.id
+    const previousDeployedAt = currentActiveVersion?.deployedAt ?? null
 
-    const rollbackDeployment = async () => {
+    const rollbackDeployment = async (failedDeploymentVersionId?: string) => {
       if (previousVersionId) {
         await restorePreviousVersionWebhooks({
           request,
@@ -135,15 +139,34 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
           previousVersionId,
           requestId,
         })
-        const reactivateResult = await activateWorkflowVersionById({
-          workflowId: id,
-          deploymentVersionId: previousVersionId,
-        })
+        const reactivateResult = previousDeployedAt
+          ? await reactivateWorkflowVersionForRollback({
+              workflowId: id,
+              deploymentVersionId: previousVersionId,
+              deployedAt: previousDeployedAt,
+            })
+          : await activateWorkflowVersionById({
+              workflowId: id,
+              deploymentVersionId: previousVersionId,
+            })
         if (reactivateResult.success) {
+          if (failedDeploymentVersionId) {
+            await deleteDeploymentVersionById({
+              workflowId: id,
+              deploymentVersionId: failedDeploymentVersionId,
+            })
+          }
           return
         }
       }
 
+      if (failedDeploymentVersionId) {
+        await deleteDeploymentVersionById({
+          workflowId: id,
+          deploymentVersionId: failedDeploymentVersionId,
+        })
+      }
+
       await undeployWorkflow({ workflowId: id })
     }
 
@@ -183,7 +206,7 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
         requestId,
         deploymentVersionId,
       })
-      await rollbackDeployment()
+      await rollbackDeployment(deploymentVersionId)
       return createErrorResponse(
         triggerSaveResult.error?.message || 'Failed to save trigger configuration',
         triggerSaveResult.error?.status || 500
@@ -207,7 +230,7 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
         requestId,
         deploymentVersionId,
       })
-      await rollbackDeployment()
+      await rollbackDeployment(deploymentVersionId)
       return createErrorResponse(scheduleResult.error || 'Failed to create schedule', 500)
     }
     if (scheduleResult.scheduleId) {
diff --git a/apps/sim/lib/workflows/persistence/utils.test.ts b/apps/sim/lib/workflows/persistence/utils.test.ts
@@ -1218,6 +1218,55 @@ describe('Database Helpers', () => {
     })
   })
 
+  describe('rollback deployment helpers', () => {
+    it('reactivates a deployment version while preserving deployedAt during rollback', async () => {
+      const preservedDeployedAt = new Date('2024-01-01T00:00:00.000Z')
+      const txWhere = vi.fn().mockResolvedValue(undefined)
+      const txSet = vi.fn().mockReturnValue({ where: txWhere })
+      const tx = {
+        update: vi.fn().mockReturnValue({ set: txSet }),
+      }
+
+      mockDb.select.mockReturnValue({
+        from: vi.fn().mockReturnValue({
+          where: vi.fn().mockReturnValue({
+            limit: vi.fn().mockResolvedValue([{ id: 'dep-1', state: { blocks: {} } }]),
+          }),
+        }),
+      })
+      mockDb.transaction = vi.fn().mockImplementation(async (callback) => callback(tx))
+
+      const result = await dbHelpers.reactivateWorkflowVersionForRollback({
+        workflowId: mockWorkflowId,
+        deploymentVersionId: 'dep-1',
+        deployedAt: preservedDeployedAt,
+      })
+
+      expect(result).toEqual({
+        success: true,
+        deployedAt: preservedDeployedAt,
+        state: { blocks: {} },
+      })
+      expect(mockDb.transaction).toHaveBeenCalledTimes(1)
+      expect(tx.update).toHaveBeenCalledTimes(3)
+      expect(txSet).toHaveBeenCalledWith({ isDeployed: true, deployedAt: preservedDeployedAt })
+    })
+
+    it('deletes a failed deployment version row by id', async () => {
+      const deleteWhere = vi.fn().mockResolvedValue(undefined)
+      mockDb.delete.mockReturnValue({ where: deleteWhere })
+
+      const result = await dbHelpers.deleteDeploymentVersionById({
+        workflowId: mockWorkflowId,
+        deploymentVersionId: 'dep-failed',
+      })
+
+      expect(result).toEqual({ success: true })
+      expect(mockDb.delete).toHaveBeenCalled()
+      expect(deleteWhere).toHaveBeenCalledTimes(1)
+    })
+  })
+
   describe('migrateAgentBlocksToMessagesFormat', () => {
     it('should migrate agent block with both systemPrompt and userPrompt', () => {
       const blocks = {
diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts
