From 88065088bf7c7a9e3caaa7612a02520bf9a7534b Mon Sep 17 00:00:00 2001 From: Waleed Date: Mon, 29 Dec 2025 02:09:38 -0800 Subject: [PATCH] fix(deploy): fix workflow change detection to handle old variable reference format (#2623) --- .../lib/workflows/comparison/compare.test.ts | 177 ++++++++++++++++++ apps/sim/lib/workflows/comparison/compare.ts | 16 +- .../sim/lib/workflows/comparison/normalize.ts | 24 +++ 3 files changed, 213 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/workflows/comparison/compare.test.ts b/apps/sim/lib/workflows/comparison/compare.test.ts index 1472a00d4..9b72d30c5 100644 --- a/apps/sim/lib/workflows/comparison/compare.test.ts +++ b/apps/sim/lib/workflows/comparison/compare.test.ts @@ -2523,4 +2523,181 @@ describe('hasWorkflowChanged', () => { } ) }) + + describe('Variables (UI-only fields should not trigger change)', () => { + it.concurrent('should not detect change when validationError differs', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'test', + }, + } + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'test', + validationError: undefined, + }, + } + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + }) + + it.concurrent('should not detect change when validationError has value vs missing', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'number', + value: 'invalid', + }, + } + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'number', + value: 'invalid', + validationError: 'Not a valid number', + }, + } + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + }) + + it.concurrent('should detect change when variable value differs', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'old value', + }, + } + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'new value', + validationError: undefined, + }, + } + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(true) + }) + + it.concurrent('should detect change when variable is added', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = {} + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'test', + }, + } + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(true) + }) + + it.concurrent('should detect change when variable is removed', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = { + var1: { + id: 'var1', + workflowId: 'workflow1', + name: 'myVar', + type: 'plain', + value: 'test', + }, + } + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = {} + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(true) + }) + + it.concurrent('should not detect change when empty array vs empty object', () => { + const deployedState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(deployedState as any).variables = [] + + const currentState = createWorkflowState({ + blocks: { + block1: createBlock('block1'), + }, + }) + ;(currentState as any).variables = {} + + expect(hasWorkflowChanged(currentState, deployedState)).toBe(false) + }) + }) }) diff --git a/apps/sim/lib/workflows/comparison/compare.ts b/apps/sim/lib/workflows/comparison/compare.ts index 40957962b..187894d0c 100644 --- a/apps/sim/lib/workflows/comparison/compare.ts +++ b/apps/sim/lib/workflows/comparison/compare.ts @@ -6,8 +6,10 @@ import { normalizeLoop, normalizeParallel, normalizeValue, + normalizeVariables, sanitizeInputFormat, sanitizeTools, + sanitizeVariable, sortEdges, } from './normalize' @@ -228,11 +230,17 @@ export function hasWorkflowChanged( } // 6. Compare variables - const currentVariables = (currentState as any).variables || {} - const deployedVariables = (deployedState as any).variables || {} + const currentVariables = normalizeVariables((currentState as any).variables) + const deployedVariables = normalizeVariables((deployedState as any).variables) - const normalizedCurrentVars = normalizeValue(currentVariables) - const normalizedDeployedVars = normalizeValue(deployedVariables) + const normalizedCurrentVars = normalizeValue( + Object.fromEntries(Object.entries(currentVariables).map(([id, v]) => [id, sanitizeVariable(v)])) + ) + const normalizedDeployedVars = normalizeValue( + Object.fromEntries( + Object.entries(deployedVariables).map(([id, v]) => [id, sanitizeVariable(v)]) + ) + ) if (normalizedStringify(normalizedCurrentVars) !== normalizedStringify(normalizedDeployedVars)) { return true diff --git a/apps/sim/lib/workflows/comparison/normalize.ts b/apps/sim/lib/workflows/comparison/normalize.ts index 8ba44b850..bbc60c81a 100644 --- a/apps/sim/lib/workflows/comparison/normalize.ts +++ b/apps/sim/lib/workflows/comparison/normalize.ts @@ -88,6 +88,30 @@ export function sanitizeTools(tools: any[] | undefined): any[] { return tools.map(({ isExpanded, ...rest }) => rest) } +/** + * Sanitizes a variable by removing UI-only fields like validationError + * @param variable - The variable object + * @returns Sanitized variable object + */ +export function sanitizeVariable(variable: any): any { + if (!variable || typeof variable !== 'object') return variable + const { validationError, ...rest } = variable + return rest +} + +/** + * Normalizes the variables structure to always be an object. + * Handles legacy data where variables might be stored as an empty array. + * @param variables - The variables to normalize + * @returns A normalized variables object + */ +export function normalizeVariables(variables: any): Record { + if (!variables) return {} + if (Array.isArray(variables)) return {} + if (typeof variables !== 'object') return {} + return variables +} + /** * Sanitizes inputFormat array by removing UI-only fields like value and collapsed * @param inputFormat - Array of input format configurations