From 2e775e95c580a28f78d5232125e174225cb6d0f5 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 6 May 2026 15:00:21 -0700 Subject: [PATCH 1/7] improvement(deploy): state transitions --- apps/sim/app/api/workflows/utils.ts | 25 +- .../components/deploy-modal/deploy-modal.tsx | 145 +++++++++-- .../panel/components/deploy/deploy.tsx | 48 +++- .../panel/components/deploy/hooks/index.ts | 2 + .../deploy/hooks/sync-local-draft.test.ts | 246 ++++++++++++++++++ .../deploy/hooks/sync-local-draft.ts | 59 +++++ .../deploy/hooks/use-change-detection.ts | 5 +- .../deploy/hooks/use-deploy-readiness.test.ts | 101 +++++++ .../deploy/hooks/use-deploy-readiness.ts | 158 +++++++++++ .../components/deploy/hooks/use-deployment.ts | 78 +++++- apps/sim/hooks/queries/deployments.test.ts | 32 ++- apps/sim/hooks/queries/deployments.ts | 20 +- apps/sim/hooks/use-collaborative-workflow.ts | 202 ++++++++++++-- .../tools/handlers/workflow/mutations.test.ts | 127 +++++++++ .../tools/handlers/workflow/mutations.ts | 1 + .../workflows/comparison/normalize.test.ts | 21 ++ .../sim/lib/workflows/comparison/normalize.ts | 12 +- .../workflows/orchestration/deploy.test.ts | 177 +++++++++++++ .../sim/lib/workflows/orchestration/deploy.ts | 32 ++- .../lib/workflows/persistence/utils.test.ts | 36 +++ apps/sim/lib/workflows/persistence/utils.ts | 51 ++-- apps/sim/stores/operation-queue/store.test.ts | 112 ++++++++ apps/sim/stores/operation-queue/store.ts | 92 ++++--- apps/sim/stores/operation-queue/types.ts | 3 + apps/sim/stores/workflow-diff/store.ts | 117 ++++++++- apps/sim/stores/workflow-diff/types.ts | 9 + apps/sim/stores/workflow-diff/utils.test.ts | 77 ++++++ apps/sim/stores/workflow-diff/utils.ts | 33 +++ 28 files changed, 1854 insertions(+), 167 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.test.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/sync-local-draft.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.test.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.ts create mode 100644 apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts create mode 100644 apps/sim/lib/workflows/orchestration/deploy.test.ts create mode 100644 apps/sim/stores/workflow-diff/utils.test.ts 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(null) const [deployWarnings, setDeployWarnings] = useState([]) + const [isFinalizingDeploy, setIsFinalizingDeploy] = useState(false) const [isChatFormValid, setIsChatFormValid] = useState(false) const [selectedStreamingOutputs, setSelectedStreamingOutputs] = useState([]) @@ -112,6 +121,7 @@ export function DeployModal({ const [chatSuccess, setChatSuccess] = useState(false) const chatSuccessTimeoutRef = useRef | null>(null) + const deployActionIdRef = useRef(0) const [isCreateKeyModalOpen, setIsCreateKeyModalOpen] = useState(false) const [isApiInfoModalOpen, setIsApiInfoModalOpen] = useState(false) @@ -176,6 +186,30 @@ 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 { + await syncLocalDraftFromServer(workflowId) + return null + } catch (error) { + 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]) + + useEffect(() => { + deployActionIdRef.current += 1 + setIsFinalizingDeploy(false) + }, [workflowId]) + const getApiKeyLabel = useCallback( (value?: string | null) => { if (value && value.trim().length > 0) { @@ -289,18 +323,38 @@ export function DeployModal({ setDeployError(null) setDeployWarnings([]) + let actionId: number | null = null 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)) return + setDeployError(deployReadiness.tooltip) + return } + if (!isWorkflowStillActive(workflowId)) return + + actionId = deployActionIdRef.current + 1 + deployActionIdRef.current = actionId + setIsFinalizingDeploy(true) + let result: Awaited> + let syncWarning: string | null + try { + result = await deployMutation.mutateAsync({ workflowId }) + syncWarning = await syncDraftAfterDeploy() + } finally { + if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + setIsFinalizingDeploy(false) + } + } + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } catch (error: unknown) { + if (actionId !== null && 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) } - }, [workflowId, deployMutation]) + }, [workflowId, deployMutation, deployReadiness, syncDraftAfterDeploy, isWorkflowStillActive]) const handlePromoteToLive = useCallback( async (version: number) => { @@ -310,15 +364,17 @@ export function DeployModal({ 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 } }, - [workflowId, activateVersionMutation] + [workflowId, activateVersionMutation, isWorkflowStillActive] ) const handleUndeploy = useCallback(async () => { @@ -326,18 +382,28 @@ export function DeployModal({ try { await undeployMutation.mutateAsync({ workflowId }) + if (!isWorkflowStillActive(workflowId)) return setShowUndeployConfirm(false) onOpenChange(false) } catch (error: unknown) { + if (!isWorkflowStillActive(workflowId)) return logger.error('Error undeploying workflow:', { error }) } - }, [workflowId, undeployMutation, onOpenChange]) + }, [workflowId, undeployMutation, onOpenChange, isWorkflowStillActive]) const handleRedeploy = useCallback(async () => { if (!workflowId) return setDeployError(null) setDeployWarnings([]) + let actionId: number | null = null + + if (!(await deployReadiness.waitUntilReady())) { + if (!isWorkflowStillActive(workflowId)) return + setDeployError(deployReadiness.tooltip) + return + } + if (!isWorkflowStillActive(workflowId)) return const { blocks, edges, loops, parallels } = useWorkflowStore.getState() const liveBlocks = mergeSubblockState(blocks, workflowId) @@ -354,16 +420,29 @@ export function DeployModal({ } try { - const result = await deployMutation.mutateAsync({ workflowId }) - if (result.warnings && result.warnings.length > 0) { - setDeployWarnings(result.warnings) + actionId = deployActionIdRef.current + 1 + deployActionIdRef.current = actionId + setIsFinalizingDeploy(true) + let result: Awaited> + let syncWarning: string | null + try { + result = await deployMutation.mutateAsync({ workflowId }) + syncWarning = await syncDraftAfterDeploy() + } finally { + if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + setIsFinalizingDeploy(false) + } } + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return + setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } catch (error: unknown) { + if (actionId !== null && 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) } - }, [workflowId, deployMutation]) + }, [workflowId, deployMutation, deployReadiness, syncDraftAfterDeploy, isWorkflowStillActive]) const handleCloseModal = useCallback(() => { setChatSubmitting(false) @@ -456,7 +535,7 @@ export function DeployModal({ // deleteTrigger?.click() // }, []) - const isSubmitting = deployMutation.isPending + const isSubmitting = deployMutation.isPending || isFinalizingDeploy const isUndeploying = undeployMutation.isPending return ( @@ -610,6 +689,8 @@ export function DeployModal({ needsRedeployment={needsRedeployment} isSubmitting={isSubmitting} isUndeploying={isUndeploying} + deployReadiness={deployReadiness} + isDeploymentSettling={isDeploymentSettling} onDeploy={onDeploy} onRedeploy={handleRedeploy} onUndeploy={() => setShowUndeployConfirm(true)} @@ -922,12 +1003,13 @@ export function DeployModal({ interface StatusBadgeProps { isWarning: boolean + isSyncing?: boolean } -function StatusBadge({ isWarning }: StatusBadgeProps) { - const label = isWarning ? 'Update deployment' : 'Live' +function StatusBadge({ isWarning, isSyncing = false }: StatusBadgeProps) { + const label = isSyncing ? 'Syncing changes' : isWarning ? 'Update deployment' : 'Live' return ( - + {label} ) @@ -961,6 +1043,8 @@ interface GeneralFooterProps { needsRedeployment: boolean isSubmitting: boolean isUndeploying: boolean + deployReadiness: DeployReadiness + isDeploymentSettling: boolean onDeploy: () => Promise onRedeploy: () => Promise onUndeploy: () => void @@ -971,17 +1055,27 @@ function GeneralFooter({ needsRedeployment, isSubmitting, isUndeploying, + deployReadiness, + isDeploymentSettling, onDeploy, onRedeploy, onUndeploy, }: GeneralFooterProps) { + const isDeployBlocked = + deployReadiness.isBlocked || isDeploymentSettling || isSubmitting || isUndeploying + const syncingLabel = deployReadiness.isSyncing ? deployReadiness.label : 'Syncing...' + const blockedMessage = + deployReadiness.isBlocked && !isDeploymentSettling && !isSubmitting && !isUndeploying + ? deployReadiness.tooltip + : null + if (!isDeployed) { return ( -
+
{blockedMessage}
-
@@ -990,14 +1084,19 @@ 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..f3ab09f4597 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) { + return deployReadiness.tooltip + } if (changeDetected) { return 'Update deployment' } @@ -89,6 +105,22 @@ export function Deploy({ return 'Deploy workflow' } + const getButtonLabel = () => { + if (isDeploymentSettling) { + return deployReadiness.isSyncing ? deployReadiness.label : 'Syncing...' + } + if (isDeployed && deployReadiness.isBlocked) { + return deployReadiness.label + } + if (changeDetected) { + return 'Update' + } + if (isDeployed) { + return 'Live' + } + return 'Deploy' + } + return ( <> @@ -97,13 +129,17 @@ export function Deploy({ @@ -117,7 +153,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/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..b71212737b3 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deploy-readiness.ts @@ -0,0 +1,158 @@ +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 diff = useWorkflowDiffStore.getState() + return ( + !queue.hasOperationError && + !queue.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..d038518ff64 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,21 @@ -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 { 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 +23,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,6 +35,19 @@ export function useDeployment({ workflowId, isDeployed }: UseDeploymentProps) { return { success: true, shouldOpenModal: true } } + 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({ @@ -44,22 +66,52 @@ export function useDeployment({ workflowId, isDeployed }: UseDeploymentProps) { return { success: false, shouldOpenModal: false } } + setIsFinalizingDeploy(true) try { - await mutateAsync({ workflowId }) + 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) { + 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 } - } catch (error) { - const errorMessage = error instanceof Error ? error.message : 'Failed to deploy workflow' - addNotification({ - level: 'error', - message: errorMessage, - workflowId, - }) - return { success: false, shouldOpenModal: false } + } finally { + setIsFinalizingDeploy(false) } - }, [workflowId, isDeployed, addNotification, mutateAsync]) + }, [workflowId, isDeployed, deployReadiness, addNotification, mutateAsync]) return { - isDeploying, + isDeploying: isDeploying || isFinalizingDeploy, handleDeployClick, } } 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..c7df8421cc5 100644 --- a/apps/sim/hooks/queries/deployments.ts +++ b/apps/sim/hooks/queries/deployments.ts @@ -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 @@ -299,15 +308,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) }, }) } diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 5f3b16e56ee..6fb1bd8bf3e 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) } } @@ -655,12 +753,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 +802,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 +825,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 +849,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/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/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/orchestration/deploy.test.ts b/apps/sim/lib/workflows/orchestration/deploy.test.ts new file mode 100644 index 00000000000..2f0740f8d96 --- /dev/null +++ b/apps/sim/lib/workflows/orchestration/deploy.test.ts @@ -0,0 +1,177 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockLimit, + mockUpdateSet, + mockSaveWorkflowToNormalizedTables, + mockRecordAudit, + mockCaptureServerEvent, +} = vi.hoisted(() => ({ + mockLimit: vi.fn(), + mockUpdateSet: vi.fn(), + mockSaveWorkflowToNormalizedTables: vi.fn(), + mockRecordAudit: vi.fn(), + mockCaptureServerEvent: vi.fn(), +})) + +vi.mock('@sim/db', () => ({ + db: { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: mockLimit, + })), + })), + })), + update: vi.fn(() => ({ + set: mockUpdateSet, + })), + }, + 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 }))) + 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', + }, + }, + }) + ) + 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..5e27f532f1e 100644 --- a/apps/sim/lib/workflows/orchestration/deploy.ts +++ b/apps/sim/lib/workflows/orchestration/deploy.ts @@ -18,7 +18,7 @@ import { activateWorkflowVersion, activateWorkflowVersionById, deployWorkflow, - loadWorkflowFromNormalizedTables, + loadWorkflowDeploymentSnapshot, saveWorkflowToNormalizedTables, undeployWorkflow, } from '@/lib/workflows/persistence/utils' @@ -96,12 +96,12 @@ export async function performFullDeploy( const requestId = params.requestId ?? generateRequestId() const request = params.request ?? new NextRequest(new URL('/api/webhooks', getBaseUrl())) - const normalizedData = await loadWorkflowFromNormalizedTables(workflowId) - if (!normalizedData) { + const deploymentSnapshot = await loadWorkflowDeploymentSnapshot(workflowId) + if (!deploymentSnapshot) { return { success: false, error: 'Failed to load workflow state', errorCode: 'not_found' } } - const scheduleValidation = validateWorkflowSchedules(normalizedData.blocks) + const scheduleValidation = validateWorkflowSchedules(deploymentSnapshot.blocks) if (!scheduleValidation.isValid) { return { success: false, @@ -156,6 +156,7 @@ export async function performFullDeploy( workflowId, deployedBy: actorId, workflowName: workflowName || workflowRecord.name || undefined, + workflowState: deploymentSnapshot, }) if (!deployResult.success) { @@ -175,7 +176,7 @@ export async function performFullDeploy( workflowId, workflow: workflowData, userId, - blocks: normalizedData.blocks, + blocks: deploymentSnapshot.blocks, requestId, deploymentVersionId, previousVersionId, @@ -197,7 +198,7 @@ export async function performFullDeploy( const scheduleResult = await createSchedulesForDeploy( workflowId, - normalizedData.blocks, + deploymentSnapshot.blocks, db, deploymentVersionId ) @@ -235,7 +236,7 @@ export async function performFullDeploy( if (workflowData.workspaceId) { try { const { pruneStaleWorkflowGroupOutputs } = await import('@/lib/table/service') - const validBlockIds = new Set(Object.keys(normalizedData.blocks)) + const validBlockIds = new Set(Object.keys(deploymentSnapshot.blocks)) await pruneStaleWorkflowGroupOutputs({ workflowId, workspaceId: workflowData.workspaceId as string, @@ -613,6 +614,7 @@ export async function performRevertToVersion( edges?: unknown[] loops?: Record parallels?: Record + variables?: WorkflowState['variables'] } if (!deployedState.blocks || !deployedState.edges) { return { @@ -623,13 +625,19 @@ export async function performRevertToVersion( } const lastSaved = Date.now() - const saveResult = await saveWorkflowToNormalizedTables(workflowId, { + const hasDeploymentVariables = Object.hasOwn(deployedState, 'variables') + const restoredState: WorkflowState = { blocks: deployedState.blocks, edges: deployedState.edges, loops: deployedState.loops || {}, parallels: deployedState.parallels || {}, lastSaved, - } as WorkflowState) + } as WorkflowState + if (hasDeploymentVariables) { + restoredState.variables = deployedState.variables || {} + } + + const saveResult = await saveWorkflowToNormalizedTables(workflowId, restoredState) if (!saveResult.success) { return { @@ -641,7 +649,11 @@ export async function performRevertToVersion( await db .update(workflowTable) - .set({ lastSynced: new Date(), updatedAt: new Date() }) + .set({ + ...(hasDeploymentVariables ? { variables: deployedState.variables || {} } : {}), + lastSynced: new Date(), + updatedAt: new Date(), + }) .where(eq(workflowTable.id, workflowId)) try { diff --git a/apps/sim/lib/workflows/persistence/utils.test.ts b/apps/sim/lib/workflows/persistence/utils.test.ts index 3a8f3c47cba..26e4c8dff0e 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() diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts index cbe1258dd7d..1449b32c43b 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -379,6 +379,37 @@ export async function loadWorkflowFromNormalizedTables( } } +export async function loadWorkflowDeploymentSnapshot( + workflowId: string +): Promise { + const [normalizedData, [workflowRecord]] = await Promise.all([ + loadWorkflowFromNormalizedTables(workflowId), + db + .select({ variables: workflow.variables }) + .from(workflow) + .where(eq(workflow.id, workflowId)) + .limit(1), + ]) + + if (!normalizedData) return null + + return buildWorkflowDeploymentSnapshot(normalizedData, workflowRecord?.variables) +} + +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, @@ -407,6 +438,7 @@ export async function deployWorkflow(params: { workflowId: string deployedBy: string workflowName?: string + workflowState?: WorkflowState }): Promise<{ success: boolean version?: number @@ -418,26 +450,11 @@ export async function deployWorkflow(params: { const { workflowId, deployedBy, workflowName } = params try { - const normalizedData = await loadWorkflowFromNormalizedTables(workflowId) - if (!normalizedData) { + const currentState = params.workflowState ?? (await loadWorkflowDeploymentSnapshot(workflowId)) + if (!currentState) { return { success: false, error: 'Failed to load workflow state' } } - const [workflowRecord] = await db - .select({ variables: workflow.variables }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1) - - const currentState = { - blocks: normalizedData.blocks, - edges: normalizedData.edges, - loops: normalizedData.loops, - parallels: normalizedData.parallels, - variables: workflowRecord?.variables || undefined, - lastSaved: Date.now(), - } - const now = new Date() const deployedVersion = await db.transaction(async (tx) => { diff --git a/apps/sim/stores/operation-queue/store.test.ts b/apps/sim/stores/operation-queue/store.test.ts index b86a3da2561..898b640eb60 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,114 @@ 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('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..565a16ac4c2 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,31 @@ 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.operation.operation === 'subblock-update' && + op.operation.target === 'subblock' && + op.operation.payload?.blockId === blockId && + op.operation.payload?.subblockId === subblockId } if ( @@ -125,20 +128,12 @@ 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.operation.operation === 'variable-update' && + op.operation.target === 'variable' && + op.operation.payload?.variableId === variableId && + op.operation.payload?.field === field } const state = get() @@ -154,6 +149,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 +190,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 +408,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 +630,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() From 53b2a06a8aceecf6292a1cfab51dcb15765f4840 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Wed, 6 May 2026 23:37:35 -0700 Subject: [PATCH 2/7] more fixes --- apps/sim/app/api/workflows/[id]/route.ts | 44 +-- .../sim/app/api/workflows/[id]/state/route.ts | 80 +++-- .../general/components/versions.tsx | 5 +- .../components/general/general.tsx | 16 +- .../components/deploy-modal/deploy-modal.tsx | 121 +++++--- .../panel/components/deploy/deploy.tsx | 5 +- .../deploy/hooks/deploy-action-lock.test.ts | 24 ++ .../deploy/hooks/deploy-action-lock.ts | 14 + .../components/deploy/hooks/use-deployment.ts | 68 +++-- apps/sim/hooks/queries/deployments.ts | 19 +- apps/sim/hooks/use-collaborative-workflow.ts | 1 + .../workflows/orchestration/deploy.test.ts | 35 ++- .../sim/lib/workflows/orchestration/deploy.ts | 287 ++++++++---------- apps/sim/lib/workflows/persistence/utils.ts | 238 +++++++++++---- apps/sim/stores/operation-queue/store.test.ts | 184 +++++++++++ apps/sim/stores/operation-queue/store.ts | 2 + packages/workflow-persistence/src/load.ts | 14 +- 17 files changed, 814 insertions(+), 343 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.test.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/deploy-action-lock.ts 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/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/components/versions.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/components/versions.tsx index 92be43d5408..228d57cd634 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/components/versions.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/components/versions.tsx @@ -32,6 +32,7 @@ interface VersionsProps { workflowId: string | null versions: WorkflowDeploymentVersionResponse[] versionsLoading: boolean + isPromotingVersion: boolean selectedVersion: number | null onSelectVersion: (version: number | null) => 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..d28b6aecaa0 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 @@ -17,6 +17,7 @@ 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 type { WorkflowState } from '@/stores/workflows/workflow/types' @@ -30,8 +31,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 +49,11 @@ export function GeneralDeploy({ isLoadingDeployedState, versions, versionsLoading, + isPromotingVersion, + deployReadiness, onPromoteToLive, onLoadDeploymentComplete, + onLoadDeploymentBlocked, }: GeneralDeployProps) { const [selectedVersion, setSelectedVersion] = useState(null) const [showActiveDespiteSelection, setShowActiveDespiteSelection] = useState(false) @@ -84,6 +91,10 @@ export function GeneralDeploy({ const confirmLoadDeployment = async () => { if (!workflowId || versionToLoad === null) return + if (!(await deployReadiness.waitUntilReady())) { + onLoadDeploymentBlocked(deployReadiness.tooltip) + return + } setShowLoadDialog(false) const version = versionToLoad @@ -98,7 +109,7 @@ export function GeneralDeploy({ } const confirmPromoteToLive = async () => { - if (versionToPromote === null) return + if (versionToPromote === null || isPromotingVersion) return setShowPromoteDialog(false) const version = versionToPromote @@ -221,6 +232,7 @@ export function GeneralDeploy({ workflowId={workflowId} versions={versions} versionsLoading={versionsLoading} + isPromotingVersion={isPromotingVersion} selectedVersion={selectedVersion} onSelectVersion={handleSelectVersion} onPromoteToLive={handlePromoteToLive} @@ -274,7 +286,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 d0118947661..e5f6b7d94fd 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 @@ -22,6 +22,10 @@ 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' @@ -106,6 +110,7 @@ export function DeployModal({ 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([]) @@ -122,6 +127,7 @@ 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) @@ -194,16 +200,20 @@ export function DeployModal({ if (!workflowId) return null try { - await syncLocalDraftFromServer(workflowId) + 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]) + }, [workflowId, isWorkflowStillActive]) useEffect(() => { deployActionIdRef.current += 1 @@ -319,47 +329,53 @@ 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([]) - let actionId: number | null = null try { if (!(await deployReadiness.waitUntilReady())) { - if (!isWorkflowStillActive(workflowId)) return + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return setDeployError(deployReadiness.tooltip) return } - if (!isWorkflowStillActive(workflowId)) return + if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return - actionId = deployActionIdRef.current + 1 - deployActionIdRef.current = actionId - setIsFinalizingDeploy(true) - let result: Awaited> - let syncWarning: string | null try { - result = await deployMutation.mutateAsync({ workflowId }) - syncWarning = await syncDraftAfterDeploy() + 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 && isWorkflowStillActive(workflowId)) { setIsFinalizingDeploy(false) } } - if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return - setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } catch (error: unknown) { - if (actionId !== null && deployActionIdRef.current !== actionId) return + if (deployActionIdRef.current !== actionId) return if (!isWorkflowStillActive(workflowId)) return logger.error('Error deploying workflow:', { error }) const errorMessage = toError(error).message || 'Failed to deploy workflow' setDeployError(errorMessage) + } finally { + releaseDeployAction(workflowId) + if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + setIsFinalizingDeploy(false) + } } }, [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 { @@ -372,6 +388,9 @@ export function DeployModal({ if (!isWorkflowStillActive(workflowId)) return logger.error('Error promoting version:', { error }) throw error + } finally { + activateVersionInFlightRef.current = false + setIsActivatingVersion(false) } }, [workflowId, activateVersionMutation, isWorkflowStillActive] @@ -393,58 +412,63 @@ export function DeployModal({ const handleRedeploy = useCallback(async () => { if (!workflowId) return + if (!tryAcquireDeployAction(workflowId)) return + const actionId = deployActionIdRef.current + 1 + deployActionIdRef.current = actionId + setIsFinalizingDeploy(true) setDeployError(null) setDeployWarnings([]) - let actionId: number | null = null - - if (!(await deployReadiness.waitUntilReady())) { - if (!isWorkflowStillActive(workflowId)) return - setDeployError(deployReadiness.tooltip) - return - } - if (!isWorkflowStillActive(workflowId)) 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 { - actionId = deployActionIdRef.current + 1 - deployActionIdRef.current = actionId - setIsFinalizingDeploy(true) - let result: Awaited> - let syncWarning: string | null + 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 { - result = await deployMutation.mutateAsync({ workflowId }) - syncWarning = await syncDraftAfterDeploy() + 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 && isWorkflowStillActive(workflowId)) { setIsFinalizingDeploy(false) } } - if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return - setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } catch (error: unknown) { - if (actionId !== null && deployActionIdRef.current !== actionId) return + if (deployActionIdRef.current !== actionId) return if (!isWorkflowStillActive(workflowId)) return logger.error('Error redeploying workflow:', { error }) const errorMessage = toError(error).message || 'Failed to redeploy workflow' setDeployError(errorMessage) + } finally { + releaseDeployAction(workflowId) + if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + setIsFinalizingDeploy(false) + } } }, [workflowId, deployMutation, deployReadiness, syncDraftAfterDeploy, isWorkflowStillActive]) const handleCloseModal = useCallback(() => { + deployActionIdRef.current += 1 + setIsFinalizingDeploy(false) setChatSubmitting(false) setDeployError(null) setDeployWarnings([]) @@ -593,8 +617,11 @@ export function DeployModal({ isLoadingDeployedState={isLoadingDeployedState} versions={versions} versionsLoading={versionsLoading} + isPromotingVersion={isActivatingVersion || activateVersionMutation.isPending} + deployReadiness={deployReadiness} onPromoteToLive={handlePromoteToLive} onLoadDeploymentComplete={handleCloseModal} + onLoadDeploymentBlocked={setDeployError} /> 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 f3ab09f4597..08a5e548c1e 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 @@ -93,7 +93,7 @@ export function Deploy({ if (isChangeDetectionSettling) { return 'Syncing deployment state...' } - if (deployReadiness.isBlocked) { + if (deployReadiness.isBlocked && !isDeployed) { return deployReadiness.tooltip } if (changeDetected) { @@ -109,9 +109,6 @@ export function Deploy({ if (isDeploymentSettling) { return deployReadiness.isSyncing ? deployReadiness.label : 'Syncing...' } - if (isDeployed && deployReadiness.isBlocked) { - return deployReadiness.label - } if (changeDetected) { return 'Update' } 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/use-deployment.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts index d038518ff64..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 @@ -7,6 +7,7 @@ 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' @@ -35,39 +36,48 @@ export function useDeployment({ workflowId, isDeployed, deployReadiness }: UseDe return { success: true, shouldOpenModal: true } } - const isReady = await deployReadiness.waitUntilReady() - if (useWorkflowRegistry.getState().activeWorkflowId !== workflowId) { - return { success: false, shouldOpenModal: false } - } - if (!isReady) { + if (!tryAcquireDeployAction(workflowId)) { addNotification({ - level: deployReadiness.status === 'error' ? 'error' : 'info', - message: deployReadiness.tooltip, + level: 'info', + message: 'Deployment is already in progress.', 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, - }) - if (!checkResult.passed) { - addNotification({ - level: 'error', - message: checkResult.error || 'Pre-deploy validation failed', + setIsFinalizingDeploy(true) + try { + 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 } + } - setIsFinalizingDeploy(true) - try { try { await mutateAsync({ workflowId }) } catch (error) { @@ -86,6 +96,15 @@ export function useDeployment({ workflowId, isDeployed, deployReadiness }: UseDe 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) { @@ -106,6 +125,7 @@ export function useDeployment({ workflowId, isDeployed, deployReadiness }: UseDe return { success: true, shouldOpenModal: true } } finally { + releaseDeployAction(workflowId) setIsFinalizingDeploy(false) } }, [workflowId, isDeployed, deployReadiness, addNotification, mutateAsync]) diff --git a/apps/sim/hooks/queries/deployments.ts b/apps/sim/hooks/queries/deployments.ts index c7df8421cc5..1ee0827f612 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, @@ -118,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 }), }) } @@ -150,7 +149,6 @@ export function useDeployedWorkflowState( queryFn: ({ signal }) => fetchDeployedWorkflowState(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, - placeholderData: keepPreviousData, }) } @@ -180,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, }) } @@ -214,7 +211,6 @@ export function useChatDeploymentStatus( queryFn: ({ signal }) => fetchChatDeploymentStatus(workflowId!, signal), enabled: Boolean(workflowId) && (options?.enabled ?? true), staleTime: 30 * 1000, // 30 seconds - placeholderData: keepPreviousData, }) } @@ -238,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, }) } @@ -248,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 @@ -258,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: diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 6fb1bd8bf3e..3ac879f7eed 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -739,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') diff --git a/apps/sim/lib/workflows/orchestration/deploy.test.ts b/apps/sim/lib/workflows/orchestration/deploy.test.ts index 2f0740f8d96..eeccd84ff9c 100644 --- a/apps/sim/lib/workflows/orchestration/deploy.test.ts +++ b/apps/sim/lib/workflows/orchestration/deploy.test.ts @@ -9,12 +9,29 @@ const { 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) })), + })), + }, })) vi.mock('@sim/db', () => ({ @@ -29,6 +46,7 @@ vi.mock('@sim/db', () => ({ update: vi.fn(() => ({ set: mockUpdateSet, })), + transaction: mockTransaction, }, workflow: { id: 'workflow.id' }, workflowDeploymentVersion: { @@ -90,6 +108,20 @@ 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 }) }) @@ -133,7 +165,8 @@ describe('performRevertToVersion', () => { value: 'deployed-value', }, }, - }) + }), + mockTx ) expect(mockUpdateSet).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/apps/sim/lib/workflows/orchestration/deploy.ts b/apps/sim/lib/workflows/orchestration/deploy.ts index 5e27f532f1e..5caaf5ad620 100644 --- a/apps/sim/lib/workflows/orchestration/deploy.ts +++ b/apps/sim/lib/workflows/orchestration/deploy.ts @@ -18,7 +18,6 @@ import { activateWorkflowVersion, activateWorkflowVersionById, deployWorkflow, - loadWorkflowDeploymentSnapshot, saveWorkflowToNormalizedTables, undeployWorkflow, } from '@/lib/workflows/persistence/utils' @@ -55,6 +54,25 @@ export async function notifySocketDeploymentChanged(workflowId: string): Promise } } +async function isDeploymentVersionActive( + workflowId: string, + deploymentVersionId: string +): Promise { + const [row] = 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(row) +} + export interface PerformFullDeployParams { workflowId: string userId: string @@ -96,20 +114,6 @@ export async function performFullDeploy( const requestId = params.requestId ?? generateRequestId() const request = params.request ?? new NextRequest(new URL('/api/webhooks', getBaseUrl())) - const deploymentSnapshot = await loadWorkflowDeploymentSnapshot(workflowId) - if (!deploymentSnapshot) { - return { success: false, error: 'Failed to load workflow state', errorCode: 'not_found' } - } - - const scheduleValidation = validateWorkflowSchedules(deploymentSnapshot.blocks) - if (!scheduleValidation.isValid) { - return { - success: false, - error: `Invalid schedule configuration: ${scheduleValidation.error}`, - errorCode: 'validation', - } - } - const [workflowRecord] = await db .select() .from(workflowTable) @@ -122,17 +126,36 @@ 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 deployResult = await deployWorkflow({ + workflowId, + deployedBy: actorId, + workflowName: workflowName || workflowRecord.name || undefined, + validateWorkflowState: (workflowState) => { + const scheduleValidation = validateWorkflowSchedules(workflowState.blocks) + return scheduleValidation.isValid + ? null + : `Invalid schedule configuration: ${scheduleValidation.error}` + }, + }) + + if (!deployResult.success) { + const error = deployResult.error || 'Failed to deploy workflow' + return { + success: false, + error, + errorCode: error.startsWith('Invalid schedule configuration') ? 'validation' : undefined, + } + } + + const deployedAt = deployResult.deployedAt! + const deploymentVersionId = deployResult.deploymentVersionId + const previousVersionId = deployResult.previousVersionId + const deploymentSnapshot = deployResult.currentState + + if (!deploymentVersionId || !deploymentSnapshot) { + await undeployWorkflow({ workflowId }) + return { success: false, error: 'Failed to resolve deployment version' } + } const rollbackDeployment = async () => { if (previousVersionId) { @@ -152,25 +175,6 @@ export async function performFullDeploy( await undeployWorkflow({ workflowId }) } - const deployResult = await deployWorkflow({ - workflowId, - deployedBy: actorId, - workflowName: workflowName || workflowRecord.name || undefined, - workflowState: deploymentSnapshot, - }) - - if (!deployResult.success) { - return { success: false, error: deployResult.error || 'Failed to deploy workflow' } - } - - const deployedAt = deployResult.deployedAt! - const deploymentVersionId = deployResult.deploymentVersionId - - if (!deploymentVersionId) { - await undeployWorkflow({ workflowId }) - return { success: false, error: 'Failed to resolve deployment version' } - } - const triggerSaveResult = await saveTriggerWebhooksForDeploy({ request, workflowId, @@ -179,7 +183,6 @@ export async function performFullDeploy( blocks: deploymentSnapshot.blocks, requestId, deploymentVersionId, - previousVersionId, }) if (!triggerSaveResult.success) { @@ -214,14 +217,17 @@ export async function performFullDeploy( return { success: false, error: scheduleResult.error || 'Failed to create schedule' } } - if (previousVersionId && previousVersionId !== deploymentVersionId) { + if ( + previousVersionId && + previousVersionId !== deploymentVersionId && + !(await isDeploymentVersionActive(workflowId, previousVersionId)) + ) { try { await cleanupDeploymentVersion({ workflowId, workflow: workflowData, requestId, deploymentVersionId: previousVersionId, - skipExternalCleanup: true, }) } catch (cleanupError) { logger.error(`[${requestId}] Failed to clean up previous version`, cleanupError) @@ -426,18 +432,6 @@ export async function performActivateVersion( 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 { @@ -455,20 +449,10 @@ export async function performActivateVersion( blocks: blocks as Record, requestId, deploymentVersionId: versionRow.id, - previousVersionId, forceRecreateSubscriptions: true, }) if (!triggerSaveResult.success) { - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow, - userId, - previousVersionId, - requestId, - }) - } return { success: false, error: triggerSaveResult.error?.message || 'Failed to sync trigger configuration', @@ -489,19 +473,11 @@ export async function performActivateVersion( requestId, deploymentVersionId: versionRow.id, }) - if (previousVersionId) { - await restorePreviousVersionWebhooks({ - request, - workflow, - userId, - previousVersionId, - requestId, - }) - } return { success: false, error: scheduleResult.error || 'Failed to sync schedules' } } const result = await activateWorkflowVersion({ workflowId, version }) + const previousVersionId = result.previousVersionId if (!result.success) { await cleanupDeploymentVersion({ workflowId, @@ -509,26 +485,20 @@ export async function performActivateVersion( 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) { + if ( + previousVersionId && + previousVersionId !== versionRow.id && + !(await isDeploymentVersionActive(workflowId, previousVersionId)) + ) { try { await cleanupDeploymentVersion({ workflowId, workflow, requestId, deploymentVersionId: previousVersionId, - skipExternalCleanup: true, }) } catch (cleanupError) { logger.error(`[${requestId}] Failed to clean up previous version`, cleanupError) @@ -578,84 +548,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 - variables?: WorkflowState['variables'] - } - 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 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 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)) - const saveResult = await saveWorkflowToNormalizedTables(workflowId, restoredState) + 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({ - ...(hasDeploymentVariables ? { variables: deployedState.variables || {} } : {}), - 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.ts b/apps/sim/lib/workflows/persistence/utils.ts index 1449b32c43b..f133954d451 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -25,6 +25,28 @@ const logger = createLogger('WorkflowDBHelpers') export type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types' export type WorkflowDeploymentVersion = InferSelectModel +async function lockWorkflowForUpdate(tx: DbOrTx, workflowId: string): Promise { + if ('execute' in tx && typeof tx.execute === 'function') { + await tx.execute(sql`SELECT id FROM workflow WHERE id = ${workflowId} FOR UPDATE`) + return true + } + + 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 Array.isArray(rows) ? rows.length > 0 : true + } + + const rows = await query + + return Array.isArray(rows) ? rows.length > 0 : true +} + export interface WorkflowDeploymentVersionResponse { id: string version: number @@ -342,9 +364,10 @@ 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) @@ -380,20 +403,32 @@ export async function loadWorkflowFromNormalizedTables( } export async function loadWorkflowDeploymentSnapshot( - workflowId: string + workflowId: string, + externalTx?: DbOrTx ): Promise { - const [normalizedData, [workflowRecord]] = await Promise.all([ - loadWorkflowFromNormalizedTables(workflowId), - db - .select({ variables: workflow.variables }) - .from(workflow) - .where(eq(workflow.id, workflowId)) - .limit(1), - ]) + 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 + if (!normalizedData) return null + + return buildWorkflowDeploymentSnapshot(normalizedData, workflowRecord?.variables) + } - 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( @@ -415,7 +450,20 @@ export async function saveWorkflowToNormalizedTables( 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 { @@ -439,25 +487,49 @@ export async function deployWorkflow(params: { deployedBy: string workflowName?: string workflowState?: WorkflowState + validateWorkflowState?: (workflowState: WorkflowState) => string | null }): Promise<{ success: boolean version?: number deploymentVersionId?: string deployedAt?: Date - currentState?: any + previousVersionId?: string + currentState?: WorkflowState error?: string }> { const { workflowId, deployedBy, workflowName } = params try { - const currentState = params.workflowState ?? (await loadWorkflowDeploymentSnapshot(workflowId)) - if (!currentState) { - return { success: false, error: 'Failed to load workflow state' } - } - const now = new Date() + let currentState: WorkflowState | null = null const deployedVersion = await db.transaction(async (tx) => { + if (!(await lockWorkflowForUpdate(tx, workflowId))) { + return { success: false as const, error: 'Workflow not found' } + } + + currentState = params.workflowState ?? (await loadWorkflowDeploymentSnapshot(workflowId, tx)) + if (!currentState) { + return { success: false as const, error: 'Failed to load workflow state' } + } + + const validationError = params.validateWorkflowState?.(currentState) + if (validationError) { + return { success: false as const, error: validationError } + } + + 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 [{ maxVersion }] = await tx .select({ maxVersion: sql`COALESCE(MAX("version"), 0)` }) .from(workflowDeploymentVersion) @@ -488,9 +560,18 @@ export async function deployWorkflow(params: { await tx.update(workflow).set(updateData).where(eq(workflow.id, workflowId)) - return { version: nextVersion, deploymentVersionId } + return { + success: true as const, + version: nextVersion, + deploymentVersionId, + previousVersionId, + } }) + if (!deployedVersion.success) { + return { success: false, error: deployedVersion.error } + } + logger.info(`Deployed workflow ${workflowId} as v${deployedVersion.version}`) if (workflowName) { @@ -522,8 +603,9 @@ export async function deployWorkflow(params: { success: true, version: deployedVersion.version, deploymentVersionId: deployedVersion.deploymentVersionId, + previousVersionId: deployedVersion.previousVersionId, deployedAt: now, - currentState, + currentState: currentState || undefined, } } catch (error) { logger.error(`Error deploying workflow ${workflowId}:`, error) @@ -689,6 +771,8 @@ export async function undeployWorkflow(params: { workflowId: string; tx?: DbOrTx const { workflowId, tx } = params const executeUndeploy = async (dbCtx: DbOrTx) => { + await lockWorkflowForUpdate(dbCtx, workflowId) + const { deleteSchedulesForWorkflow } = await import('@/lib/workflows/schedules/deploy') await deleteSchedulesForWorkflow(workflowId, dbCtx) @@ -730,29 +814,47 @@ export async function activateWorkflowVersion(params: { 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 }) @@ -777,14 +879,21 @@ export async function activateWorkflowVersion(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 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) @@ -802,29 +911,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 }) @@ -844,14 +971,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/stores/operation-queue/store.test.ts b/apps/sim/stores/operation-queue/store.test.ts index 898b640eb60..b18439f60cc 100644 --- a/apps/sim/stores/operation-queue/store.test.ts +++ b/apps/sim/stores/operation-queue/store.test.ts @@ -108,6 +108,190 @@ describe('operation queue room gating', () => { ).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', diff --git a/apps/sim/stores/operation-queue/store.ts b/apps/sim/stores/operation-queue/store.ts index 565a16ac4c2..bf0d9533e85 100644 --- a/apps/sim/stores/operation-queue/store.ts +++ b/apps/sim/stores/operation-queue/store.ts @@ -117,6 +117,7 @@ export const useOperationQueueStore = create((set, get) => const { blockId, subblockId } = operation.operation.payload shouldDropPendingOperation = (op) => op.status === 'pending' && + op.workflowId === operation.workflowId && op.operation.operation === 'subblock-update' && op.operation.target === 'subblock' && op.operation.payload?.blockId === blockId && @@ -130,6 +131,7 @@ export const useOperationQueueStore = create((set, get) => const { variableId, field } = operation.operation.payload shouldDropPendingOperation = (op) => op.status === 'pending' && + op.workflowId === operation.workflowId && op.operation.operation === 'variable-update' && op.operation.target === 'variable' && op.operation.payload?.variableId === variableId && diff --git a/packages/workflow-persistence/src/load.ts b/packages/workflow-persistence/src/load.ts index 8f19375c811..c187b0d76ec 100644 --- a/packages/workflow-persistence/src/load.ts +++ b/packages/workflow-persistence/src/load.ts @@ -4,7 +4,7 @@ import type { BlockState, Loop, Parallel } from '@sim/workflow-types/workflow' import { SUBFLOW_TYPES } from '@sim/workflow-types/workflow' import { eq } from 'drizzle-orm' import type { Edge } from 'reactflow' -import type { NormalizedWorkflowData } from './types' +import type { DbOrTx, NormalizedWorkflowData } from './types' const logger = createLogger('WorkflowPersistenceLoad') @@ -27,14 +27,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)) From 8b40252d29d01787e846046d27af74304457edb9 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 7 May 2026 14:38:53 -0700 Subject: [PATCH 3/7] improvements and shift to outbox policy with eager call --- apps/sim/app/api/chat/manage/[id]/route.ts | 43 +- apps/sim/app/api/form/route.ts | 36 +- .../app/api/schedules/execute/route.test.ts | 12 +- apps/sim/app/api/schedules/execute/route.ts | 102 ++- apps/sim/app/api/v1/admin/types.ts | 1 + .../v1/admin/workflows/[id]/deploy/route.ts | 2 + .../versions/[versionId]/activate/route.ts | 1 + .../app/api/webhooks/outbox/process/route.ts | 8 +- .../app/api/workflows/[id]/deploy/route.ts | 2 + .../[id]/deployments/[version]/route.ts | 1 + .../components/general/general.tsx | 87 ++- .../components/deploy-modal/deploy-modal.tsx | 39 +- apps/sim/background/schedule-execution.ts | 126 +++- apps/sim/executor/execution/block-executor.ts | 31 +- apps/sim/executor/variables/resolver.test.ts | 9 +- apps/sim/executor/variables/resolver.ts | 93 ++- apps/sim/hooks/queries/deployments.ts | 4 +- .../lib/api/contracts/v1/admin/workflows.ts | 1 + apps/sim/lib/core/outbox/service.test.ts | 2 +- apps/sim/lib/core/outbox/service.ts | 140 +++- apps/sim/lib/mcp/workflow-mcp-sync.ts | 55 +- apps/sim/lib/table/service.ts | 8 +- apps/sim/lib/webhooks/deploy.ts | 324 +++++--- .../lib/webhooks/provider-subscriptions.ts | 11 +- apps/sim/lib/webhooks/providers/airtable.ts | 36 +- apps/sim/lib/webhooks/providers/ashby.ts | 4 + apps/sim/lib/webhooks/providers/attio.ts | 12 +- apps/sim/lib/webhooks/providers/calendly.ts | 6 + apps/sim/lib/webhooks/providers/emailbison.ts | 3 + apps/sim/lib/webhooks/providers/fathom.ts | 5 + apps/sim/lib/webhooks/providers/grain.ts | 4 + apps/sim/lib/webhooks/providers/lemlist.ts | 15 + apps/sim/lib/webhooks/providers/linear.ts | 41 + .../lib/webhooks/providers/microsoft-teams.ts | 5 + apps/sim/lib/webhooks/providers/monday.ts | 24 + apps/sim/lib/webhooks/providers/resend.ts | 3 + apps/sim/lib/webhooks/providers/telegram.ts | 45 ++ apps/sim/lib/webhooks/providers/typeform.ts | 5 + apps/sim/lib/webhooks/providers/types.ts | 3 +- apps/sim/lib/webhooks/providers/vercel.ts | 3 + apps/sim/lib/webhooks/providers/webflow.ts | 17 +- apps/sim/lib/workflows/deployment-outbox.ts | 700 ++++++++++++++++++ .../workflows/orchestration/deploy.test.ts | 1 + .../sim/lib/workflows/orchestration/deploy.ts | 352 ++++----- apps/sim/lib/workflows/persistence/utils.ts | 91 ++- apps/sim/lib/workflows/schedules/deploy.ts | 26 +- packages/workflow-persistence/src/load.ts | 23 +- 47 files changed, 2006 insertions(+), 556 deletions(-) create mode 100644 apps/sim/lib/workflows/deployment-outbox.ts diff --git a/apps/sim/app/api/chat/manage/[id]/route.ts b/apps/sim/app/api/chat/manage/[id]/route.ts index 933636dd12b..6e8ff8d33d2 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') @@ -122,21 +122,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 @@ -152,11 +139,31 @@ 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(), } - if (workflowId) updateData.workflowId = workflowId if (identifier) updateData.identifier = identifier if (title) updateData.title = title if (description !== undefined) updateData.description = description diff --git a/apps/sim/app/api/form/route.ts b/apps/sim/app/api/form/route.ts index 1abc86e264b..582ae98d836 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,6 +20,7 @@ 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 @@ -106,21 +106,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 +146,23 @@ export const POST = withRouteHandler(async (request: NextRequest) => { updatedAt: new Date(), }) + const result = await performFullDeploy({ + workflowId, + userId: session.user.id, + request, + }) + + if (!result.success) { + await db.delete(form).where(eq(form.id, 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..f695fa7b52e 100644 --- a/apps/sim/app/api/schedules/execute/route.test.ts +++ b/apps/sim/app/api/schedules/execute/route.test.ts @@ -69,6 +69,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 +167,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 +183,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 +197,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 +220,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 +233,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 +250,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()) diff --git a/apps/sim/app/api/schedules/execute/route.ts b/apps/sim/app/api/schedules/execute/route.ts index 584425196f5..8fee06d31e7 100644 --- a/apps/sim/app/api/schedules/execute/route.ts +++ b/apps/sim/app/api/schedules/execute/route.ts @@ -2,7 +2,7 @@ import { db, workflowDeploymentVersion, workflowSchedule } from '@sim/db' import { createLogger } from '@sim/logger' 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 { 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' @@ -17,6 +17,9 @@ import { export const dynamic = 'force-dynamic' 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 dueFilter = (queuedAt: Date) => and( @@ -30,27 +33,42 @@ const dueFilter = (queuedAt: Date) => ) ) -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 +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 +80,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 +118,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 +173,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, 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/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 d28b6aecaa0..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, @@ -20,6 +20,7 @@ import type { WorkflowDeploymentVersionResponse } from '@/lib/workflows/persiste 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' @@ -63,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) @@ -79,44 +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 || isPromotingVersion) 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) } @@ -248,7 +293,7 @@ export function GeneralDeploy({

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

Are you sure you want to promote{' '} - {versionToPromoteInfo?.name || `v${versionToPromote}`} + {versionToPromoteInfo?.name || `v${versionToPromote?.version}`} {' '} to live?{' '} 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 e5f6b7d94fd..42f10f69499 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 @@ -114,7 +114,7 @@ export function DeployModal({ 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) @@ -218,6 +218,7 @@ export function DeployModal({ useEffect(() => { deployActionIdRef.current += 1 setIsFinalizingDeploy(false) + setUndeployTargetWorkflowId(null) }, [workflowId]) const getApiKeyLabel = useCallback( @@ -397,18 +398,29 @@ export function DeployModal({ ) 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 }) - if (!isWorkflowStillActive(workflowId)) return - 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(workflowId)) return + if (!isWorkflowStillActive(targetWorkflowId)) return logger.error('Error undeploying workflow:', { error }) } - }, [workflowId, undeployMutation, onOpenChange, isWorkflowStillActive]) + }, [workflowId, undeployTargetWorkflowId, undeployMutation, onOpenChange, isWorkflowStillActive]) const handleRedeploy = useCallback(async () => { if (!workflowId) return @@ -720,7 +732,9 @@ export function DeployModal({ isDeploymentSettling={isDeploymentSettling} onDeploy={onDeploy} onRedeploy={handleRedeploy} - onUndeploy={() => setShowUndeployConfirm(true)} + onUndeploy={() => { + if (workflowId) setUndeployTargetWorkflowId(workflowId) + }} /> )} {activeTab === 'api' && ( @@ -949,7 +963,12 @@ export function DeployModal({ - + { + if (!nextOpen) setUndeployTargetWorkflowId(null) + }} + > Undeploy API @@ -963,7 +982,7 @@ export function DeployModal({

@@ -1140,9 +1142,10 @@ function GeneralFooter({ - {needsRedeployment && ( + {(needsRedeployment || isDeploymentSettling) && ( )} 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 08a5e548c1e..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 @@ -106,8 +106,8 @@ export function Deploy({ } const getButtonLabel = () => { - if (isDeploymentSettling) { - return deployReadiness.isSyncing ? deployReadiness.label : 'Syncing...' + if (isDeployed && (changeDetected || isDeploymentSettling)) { + return 'Update' } if (changeDetected) { return 'Update' @@ -135,7 +135,9 @@ export function Deploy({ onClick={onDeployClick} disabled={isRegistryLoading || isDisabled} > - {isDeploying && } + {(isDeploying || isDeploymentSettling) && ( + + )} {getButtonLabel()} From af621cf327f4d8e26c2847baf289a877090eb22e Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 7 May 2026 15:57:02 -0700 Subject: [PATCH 6/7] address comments --- .../app/api/chat/manage/[id]/route.test.ts | 5 +- apps/sim/app/api/workflows/[id]/route.test.ts | 21 +++++++- .../components/deploy-modal/deploy-modal.tsx | 8 +-- .../deploy/hooks/use-deploy-readiness.ts | 5 +- .../lib/workflows/persistence/utils.test.ts | 52 +++++++++++++++++++ apps/sim/lib/workflows/persistence/utils.ts | 22 +++++--- 6 files changed, 95 insertions(+), 18 deletions(-) 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/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/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 e60770041c8..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 @@ -353,7 +353,7 @@ export function DeployModal({ if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } finally { - if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + if (deployActionIdRef.current === actionId) { setIsFinalizingDeploy(false) } } @@ -365,7 +365,7 @@ export function DeployModal({ setDeployError(errorMessage) } finally { releaseDeployAction(workflowId) - if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + if (deployActionIdRef.current === actionId) { setIsFinalizingDeploy(false) } } @@ -461,7 +461,7 @@ export function DeployModal({ if (!isWorkflowStillActive(workflowId) || deployActionIdRef.current !== actionId) return setDeployWarnings([...(result.warnings || []), ...(syncWarning ? [syncWarning] : [])]) } finally { - if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + if (deployActionIdRef.current === actionId) { setIsFinalizingDeploy(false) } } @@ -473,7 +473,7 @@ export function DeployModal({ setDeployError(errorMessage) } finally { releaseDeployAction(workflowId) - if (deployActionIdRef.current === actionId && isWorkflowStillActive(workflowId)) { + if (deployActionIdRef.current === actionId) { setIsFinalizingDeploy(false) } } 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 index b71212737b3..23490ce63b3 100644 --- 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 @@ -134,10 +134,11 @@ export function useDeployReadiness(workflowId: string | null): DeployReadiness { const drained = await queue.waitForWorkflowOperations(workflowId) if (!drained) return false + const latestQueue = useOperationQueueStore.getState() const diff = useWorkflowDiffStore.getState() return ( - !queue.hasOperationError && - !queue.hasPendingOperations(workflowId) && + !latestQueue.hasOperationError && + !latestQueue.hasPendingOperations(workflowId) && !diff.hasActiveDiff && !diff.pendingExternalUpdates[workflowId] && !diff.reconcilingWorkflows[workflowId] && diff --git a/apps/sim/lib/workflows/persistence/utils.test.ts b/apps/sim/lib/workflows/persistence/utils.test.ts index 26e4c8dff0e..82997c4f518 100644 --- a/apps/sim/lib/workflows/persistence/utils.test.ts +++ b/apps/sim/lib/workflows/persistence/utils.test.ts @@ -798,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 fe178638146..e9b409c8c67 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -25,12 +25,18 @@ const logger = createLogger('WorkflowDBHelpers') export type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types' export type WorkflowDeploymentVersion = InferSelectModel -async function lockWorkflowForUpdate(tx: DbOrTx, workflowId: string): Promise { - if ('execute' in tx && typeof tx.execute === 'function') { - await tx.execute(sql`SELECT id FROM workflow WHERE id = ${workflowId} FOR UPDATE`) - return true +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') { @@ -39,12 +45,12 @@ async function lockWorkflowForUpdate(tx: DbOrTx, workflowId: string): Promise 0 : true + return hasReturnedRows(rows) } const rows = await query - return Array.isArray(rows) ? rows.length > 0 : true + return hasReturnedRows(rows) } export interface WorkflowDeploymentVersionResponse { @@ -816,7 +822,9 @@ export async function undeployWorkflow(params: { const { workflowId, tx } = params const executeUndeploy = async (dbCtx: DbOrTx) => { - await lockWorkflowForUpdate(dbCtx, workflowId) + if (!(await lockWorkflowForUpdate(dbCtx, workflowId))) { + throw new Error('Workflow not found') + } const deploymentVersions = await dbCtx .select({ id: workflowDeploymentVersion.id }) From f542637e25bfe5eca48a3ce455f7b4b18ac0a8d0 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 7 May 2026 16:03:39 -0700 Subject: [PATCH 7/7] address bugbot --- apps/sim/app/api/form/route.test.ts | 97 +++++++++++++++++++++++++++++ apps/sim/app/api/form/route.ts | 26 ++++++-- 2 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 apps/sim/app/api/form/route.test.ts 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 582ae98d836..c0592bc366b 100644 --- a/apps/sim/app/api/form/route.ts +++ b/apps/sim/app/api/form/route.ts @@ -26,6 +26,14 @@ 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() @@ -146,14 +154,20 @@ export const POST = withRouteHandler(async (request: NextRequest) => { updatedAt: new Date(), }) - const result = await performFullDeploy({ - workflowId, - userId: session.user.id, - request, - }) + let result: Awaited> + try { + result = await performFullDeploy({ + workflowId, + userId: session.user.id, + request, + }) + } catch (error) { + await cleanupFormAfterDeployFailure(id) + throw error + } if (!result.success) { - await db.delete(form).where(eq(form.id, id)) + await cleanupFormAfterDeployFailure(id) const status = result.errorCode === 'validation' ? 400 : result.errorCode === 'not_found' ? 404 : 500 return createErrorResponse(result.error || 'Failed to deploy workflow', status)