added tests

apps/sim/app/w/[id]/components/control-bar/components/deployment-controls/deployment-controls.test.ts

@@ -0,0 +1,225 @@
+/**
+ * Deployment Controls Change Detection Logic Tests
+ *
+ * This file tests the core logic of how DeploymentControls handles change detection,
+ * specifically focusing on the needsRedeployment prop handling and state management.
+ */
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+// Mock the workflow registry store
+const mockDeploymentStatus = {
+  isDeployed: false,
+  needsRedeployment: false,
+}
+
+const mockWorkflowRegistry = {
+  getState: vi.fn(() => ({
+    getWorkflowDeploymentStatus: vi.fn((workflowId) => mockDeploymentStatus),
+  })),
+}
+
+vi.mock('@/stores/workflows/registry/store', () => ({
+  useWorkflowRegistry: vi.fn((selector) => {
+    if (typeof selector === 'function') {
+      return selector(mockWorkflowRegistry.getState())
+    }
+    return mockWorkflowRegistry.getState()
+  }),
+}))
+
+describe('DeploymentControls Change Detection Logic', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockDeploymentStatus.isDeployed = false
+    mockDeploymentStatus.needsRedeployment = false
+  })
+
+  afterEach(() => {
+    vi.resetAllMocks()
+  })
+
+  describe('needsRedeployment Priority Logic', () => {
+    it('should prioritize parent needsRedeployment over workflow registry', () => {
+      // Simulate the logic from DeploymentControls component
+      const parentNeedsRedeployment = true
+      const workflowRegistryNeedsRedeployment = false
+
+      // The component logic: Trust the parent's change detection
+      const workflowNeedsRedeployment = parentNeedsRedeployment
+
+      expect(workflowNeedsRedeployment).toBe(true)
+      expect(workflowNeedsRedeployment).not.toBe(workflowRegistryNeedsRedeployment)
+    })
+
+    it('should handle false needsRedeployment correctly', () => {
+      const parentNeedsRedeployment = false
+      const workflowNeedsRedeployment = parentNeedsRedeployment
+
+      expect(workflowNeedsRedeployment).toBe(false)
+    })
+
+    it('should maintain consistency with parent state changes', () => {
+      // Simulate state changes
+      let parentNeedsRedeployment = false
+      let workflowNeedsRedeployment = parentNeedsRedeployment
+
+      expect(workflowNeedsRedeployment).toBe(false)
+
+      // Parent detects changes
+      parentNeedsRedeployment = true
+      workflowNeedsRedeployment = parentNeedsRedeployment
+
+      expect(workflowNeedsRedeployment).toBe(true)
+
+      // Parent clears changes (after redeployment)
+      parentNeedsRedeployment = false
+      workflowNeedsRedeployment = parentNeedsRedeployment
+
+      expect(workflowNeedsRedeployment).toBe(false)
+    })
+  })
+
+  describe('Deployment Status Integration', () => {
+    it('should handle deployment status correctly', () => {
+      // Mock deployment status
+      mockDeploymentStatus.isDeployed = true
+      mockDeploymentStatus.needsRedeployment = false
+
+      const deploymentStatus = mockWorkflowRegistry
+        .getState()
+        .getWorkflowDeploymentStatus('test-id')
+
+      expect(deploymentStatus.isDeployed).toBe(true)
+      expect(deploymentStatus.needsRedeployment).toBe(false)
+    })
+
+    it('should handle missing deployment status', () => {
+      // Create a separate mock for this test case
+      const tempMockRegistry = {
+        getState: vi.fn(() => ({
+          getWorkflowDeploymentStatus: vi.fn(() => null),
+        })),
+      }
+
+      // Temporarily replace the mock
+      const originalMock = mockWorkflowRegistry.getState
+      mockWorkflowRegistry.getState = tempMockRegistry.getState as any
+
+      const deploymentStatus = mockWorkflowRegistry
+        .getState()
+        .getWorkflowDeploymentStatus('test-id')
+
+      expect(deploymentStatus).toBe(null)
+
+      // Restore original mock
+      mockWorkflowRegistry.getState = originalMock
+    })
+
+    it('should handle undefined deployment status properties', () => {
+      mockWorkflowRegistry.getState = vi.fn(() => ({
+        getWorkflowDeploymentStatus: vi.fn(() => ({})),
+      })) as any
+
+      const deploymentStatus = mockWorkflowRegistry
+        .getState()
+        .getWorkflowDeploymentStatus('test-id')
+
+      // Should handle undefined properties gracefully
+      const isDeployed = deploymentStatus?.isDeployed || false
+      expect(isDeployed).toBe(false)
+    })
+  })
+
+  describe('Change Detection Scenarios', () => {
+    it('should handle the redeployment cycle correctly', () => {
+      // Scenario 1: Initial state - deployed, no changes
+      mockDeploymentStatus.isDeployed = true
+      let parentNeedsRedeployment = false
+      let shouldShowIndicator = mockDeploymentStatus.isDeployed && parentNeedsRedeployment
+
+      expect(shouldShowIndicator).toBe(false)
+
+      // Scenario 2: User makes changes
+      parentNeedsRedeployment = true
+      shouldShowIndicator = mockDeploymentStatus.isDeployed && parentNeedsRedeployment
+
+      expect(shouldShowIndicator).toBe(true)
+
+      // Scenario 3: User redeploys
+      parentNeedsRedeployment = false // Reset after redeployment
+      shouldShowIndicator = mockDeploymentStatus.isDeployed && parentNeedsRedeployment
+
+      expect(shouldShowIndicator).toBe(false)
+    })
+
+    it('should not show indicator when workflow is not deployed', () => {
+      mockDeploymentStatus.isDeployed = false
+      const parentNeedsRedeployment = true
+      const shouldShowIndicator = mockDeploymentStatus.isDeployed && parentNeedsRedeployment
+
+      expect(shouldShowIndicator).toBe(false)
+    })
+
+    it('should show correct tooltip messages based on state', () => {
+      const getTooltipMessage = (isDeployed: boolean, needsRedeployment: boolean) => {
+        if (isDeployed && needsRedeployment) {
+          return 'Workflow changes detected'
+        }
+        if (isDeployed) {
+          return 'Deployment Settings'
+        }
+        return 'Deploy as API'
+      }
+
+      // Not deployed
+      expect(getTooltipMessage(false, false)).toBe('Deploy as API')
+      expect(getTooltipMessage(false, true)).toBe('Deploy as API')
+
+      // Deployed, no changes
+      expect(getTooltipMessage(true, false)).toBe('Deployment Settings')
+
+      // Deployed, changes detected
+      expect(getTooltipMessage(true, true)).toBe('Workflow changes detected')
+    })
+  })
+
+  describe('Error Handling', () => {
+    it('should handle null activeWorkflowId gracefully', () => {
+      const deploymentStatus = mockWorkflowRegistry.getState().getWorkflowDeploymentStatus(null)
+
+      // Should return the mocked result without throwing
+      expect(deploymentStatus).toBeDefined()
+    })
+  })
+
+  describe('Props Integration', () => {
+    it('should correctly pass needsRedeployment to child components', () => {
+      const parentNeedsRedeployment = true
+      const propsToModal = {
+        needsRedeployment: parentNeedsRedeployment,
+        workflowId: 'test-id',
+      }
+
+      expect(propsToModal.needsRedeployment).toBe(true)
+    })
+
+    it('should maintain prop consistency across re-renders', () => {
+      let needsRedeployment = false
+
+      // Initial render
+      let componentProps = { needsRedeployment }
+      expect(componentProps.needsRedeployment).toBe(false)
+
+      // State change
+      needsRedeployment = true
+      componentProps = { needsRedeployment }
+      expect(componentProps.needsRedeployment).toBe(true)
+
+      // State change back
+      needsRedeployment = false
+      componentProps = { needsRedeployment }
+      expect(componentProps.needsRedeployment).toBe(false)
+    })
+  })
+})

apps/sim/app/w/[id]/components/control-bar/control-bar.test.ts

@@ -0,0 +1,363 @@
+/**
+ * @vitest-environment jsdom
+ *
+ * Control Bar Change Detection Tests
+ *
+ * This file tests the core change detection logic in the ControlBar component,
+ * specifically focusing on the normalizeBlocksForComparison function and
+ * semantic comparison of workflow states.
+ */
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+// Mock the stores
+const mockWorkflowStore = {
+  getState: vi.fn(),
+  subscribe: vi.fn(),
+}
+
+const mockSubBlockStore = {
+  getState: vi.fn(),
+  subscribe: vi.fn(),
+}
+
+const mockWorkflowRegistry = {
+  getState: vi.fn(),
+  subscribe: vi.fn(),
+}
+
+vi.mock('@/stores/workflows/workflow/store', () => ({
+  useWorkflowStore: vi.fn((selector) => {
+    if (typeof selector === 'function') {
+      return selector(mockWorkflowStore.getState())
+    }
+    return mockWorkflowStore
+  }),
+}))
+
+vi.mock('@/stores/workflows/subblock/store', () => ({
+  useSubBlockStore: vi.fn((selector) => {
+    if (typeof selector === 'function') {
+      return selector(mockSubBlockStore.getState())
+    }
+    return mockSubBlockStore
+  }),
+}))
+
+vi.mock('@/stores/workflows/registry/store', () => ({
+  useWorkflowRegistry: vi.fn(() => mockWorkflowRegistry.getState()),
+}))
+
+vi.mock('@/stores/workflows/utils', () => ({
+  mergeSubblockState: vi.fn((blocks) => blocks),
+}))
+
+// Mock other dependencies
+vi.mock('@/lib/logs/console-logger', () => ({
+  createLogger: () => ({
+    error: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    debug: vi.fn(),
+  }),
+}))
+
+// Import the function we want to test
+// Since it's inside a component, we'll extract it for testing
+const normalizeBlocksForComparison = (blocks: Record<string, any>) => {
+  if (!blocks) return []
+
+  return Object.values(blocks)
+    .map((block: any) => ({
+      type: block.type,
+      name: block.name,
+      subBlocks: block.subBlocks || {},
+    }))
+    .sort((a, b) => {
+      // Sort by type first, then by name for consistent comparison
+      const typeA = a.type || ''
+      const typeB = b.type || ''
+      if (typeA !== typeB) return typeA.localeCompare(typeB)
+      return (a.name || '').localeCompare(b.name || '')
+    })
+}
+
+describe('normalizeBlocksForComparison', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  afterEach(() => {
+    vi.resetAllMocks()
+  })
+
+  it('should extract only functional properties from blocks', () => {
+    const blocks = {
+      'block-1': {
+        id: 'block-1',
+        type: 'agent',
+        name: 'Agent 1',
+        position: { x: 100, y: 200 },
+        height: 668,
+        enabled: true,
+        subBlocks: {
+          systemPrompt: { id: 'systemPrompt', type: 'text', value: 'You are helpful' },
+        },
+      },
+      'block-2': {
+        id: 'block-2',
+        type: 'api',
+        name: 'API 1',
+        position: { x: 300, y: 400 },
+        height: 400,
+        enabled: true,
+        subBlocks: {
+          url: { id: 'url', type: 'short-input', value: 'https://api.example.com' },
+        },
+      },
+    }
+
+    const result = normalizeBlocksForComparison(blocks)
+
+    expect(result).toHaveLength(2)
+
+    // Should only contain type, name, and subBlocks
+    result.forEach((block) => {
+      expect(block).toHaveProperty('type')
+      expect(block).toHaveProperty('name')
+      expect(block).toHaveProperty('subBlocks')
+
+      // Should NOT contain metadata properties
+      expect(block).not.toHaveProperty('id')
+      expect(block).not.toHaveProperty('position')
+      expect(block).not.toHaveProperty('height')
+      expect(block).not.toHaveProperty('enabled')
+    })
+  })
+
+  it('should sort blocks consistently by type then name', () => {
+    const blocks = {
+      'block-1': { type: 'api', name: 'API 2', subBlocks: {} },
+      'block-2': { type: 'agent', name: 'Agent 1', subBlocks: {} },
+      'block-3': { type: 'api', name: 'API 1', subBlocks: {} },
+      'block-4': { type: 'agent', name: 'Agent 2', subBlocks: {} },
+    }
+
+    const result = normalizeBlocksForComparison(blocks)
+
+    // Should be sorted: agent blocks first (by name), then api blocks (by name)
+    expect(result[0]).toEqual({ type: 'agent', name: 'Agent 1', subBlocks: {} })
+    expect(result[1]).toEqual({ type: 'agent', name: 'Agent 2', subBlocks: {} })
+    expect(result[2]).toEqual({ type: 'api', name: 'API 1', subBlocks: {} })
+    expect(result[3]).toEqual({ type: 'api', name: 'API 2', subBlocks: {} })
+  })
+
+  it('should handle blocks with undefined or null properties', () => {
+    const blocks = {
+      'block-1': {
+        type: undefined,
+        name: null,
+        subBlocks: {
+          field1: { value: 'test' },
+        },
+      },
+      'block-2': {
+        type: 'agent',
+        name: 'Agent 1',
+        // subBlocks missing
+      },
+    }
+
+    const result = normalizeBlocksForComparison(blocks)
+
+    expect(result).toHaveLength(2)
+    expect(result[0]).toEqual({
+      type: undefined,
+      name: null,
+      subBlocks: { field1: { value: 'test' } },
+    })
+    expect(result[1]).toEqual({
+      type: 'agent',
+      name: 'Agent 1',
+      subBlocks: {},
+    })
+  })
+
+  it('should return empty array for null or undefined input', () => {
+    expect(normalizeBlocksForComparison(null as any)).toEqual([])
+    expect(normalizeBlocksForComparison(undefined as any)).toEqual([])
+    expect(normalizeBlocksForComparison({})).toEqual([])
+  })
+
+  it('should preserve subBlock structure completely', () => {
+    const blocks = {
+      'agent-block': {
+        type: 'agent',
+        name: 'Test Agent',
+        subBlocks: {
+          systemPrompt: {
+            id: 'systemPrompt',
+            type: 'textarea',
+            value: 'You are a helpful assistant',
+          },
+          model: {
+            id: 'model',
+            type: 'dropdown',
+            value: 'gpt-4',
+          },
+          temperature: {
+            id: 'temperature',
+            type: 'slider',
+            value: 0.7,
+          },
+        },
+      },
+    }
+
+    const result = normalizeBlocksForComparison(blocks)
+
+    expect(result[0].subBlocks).toEqual(blocks['agent-block'].subBlocks)
+  })
+})
+
+describe('Change Detection Scenarios', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('should detect no changes when blocks are functionally identical', () => {
+    const currentBlocks = {
+      'new-id-123': {
+        id: 'new-id-123',
+        type: 'agent',
+        name: 'Agent 1',
+        position: { x: 100, y: 200 },
+        subBlocks: { systemPrompt: { value: 'Test prompt' } },
+      },
+    }
+
+    const deployedBlocks = {
+      'old-id-456': {
+        id: 'old-id-456',
+        type: 'agent',
+        name: 'Agent 1',
+        position: { x: 300, y: 400 },
+        subBlocks: { systemPrompt: { value: 'Test prompt' } },
+      },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    expect(JSON.stringify(normalizedCurrent)).toBe(JSON.stringify(normalizedDeployed))
+  })
+
+  it('should detect changes when block types differ', () => {
+    const currentBlocks = {
+      'block-1': { type: 'agent', name: 'Block 1', subBlocks: {} },
+    }
+
+    const deployedBlocks = {
+      'block-1': { type: 'api', name: 'Block 1', subBlocks: {} },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    expect(JSON.stringify(normalizedCurrent)).not.toBe(JSON.stringify(normalizedDeployed))
+  })
+
+  it('should detect changes when block names differ', () => {
+    const currentBlocks = {
+      'block-1': { type: 'agent', name: 'Agent Updated', subBlocks: {} },
+    }
+
+    const deployedBlocks = {
+      'block-1': { type: 'agent', name: 'Agent 1', subBlocks: {} },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    expect(JSON.stringify(normalizedCurrent)).not.toBe(JSON.stringify(normalizedDeployed))
+  })
+
+  it('should detect changes when subBlock values differ', () => {
+    const currentBlocks = {
+      'block-1': {
+        type: 'agent',
+        name: 'Agent 1',
+        subBlocks: {
+          systemPrompt: { value: 'Updated prompt' },
+        },
+      },
+    }
+
+    const deployedBlocks = {
+      'block-1': {
+        type: 'agent',
+        name: 'Agent 1',
+        subBlocks: {
+          systemPrompt: { value: 'Original prompt' },
+        },
+      },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    expect(JSON.stringify(normalizedCurrent)).not.toBe(JSON.stringify(normalizedDeployed))
+  })
+
+  it('should detect changes when number of blocks differs', () => {
+    const currentBlocks = {
+      'block-1': { type: 'agent', name: 'Agent 1', subBlocks: {} },
+      'block-2': { type: 'api', name: 'API 1', subBlocks: {} },
+    }
+
+    const deployedBlocks = {
+      'block-1': { type: 'agent', name: 'Agent 1', subBlocks: {} },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    expect(normalizedCurrent).toHaveLength(2)
+    expect(normalizedDeployed).toHaveLength(1)
+    expect(JSON.stringify(normalizedCurrent)).not.toBe(JSON.stringify(normalizedDeployed))
+  })
+
+  it('should ignore position and metadata changes', () => {
+    const currentBlocks = {
+      'block-1': {
+        id: 'new-id',
+        type: 'agent',
+        name: 'Agent 1',
+        position: { x: 500, y: 600 },
+        height: 800,
+        enabled: false,
+        data: { someMetadata: 'changed' },
+        subBlocks: { systemPrompt: { value: 'Test' } },
+      },
+    }
+
+    const deployedBlocks = {
+      'block-1': {
+        id: 'old-id',
+        type: 'agent',
+        name: 'Agent 1',
+        position: { x: 100, y: 200 },
+        height: 600,
+        enabled: true,
+        data: { someMetadata: 'original' },
+        subBlocks: { systemPrompt: { value: 'Test' } },
+      },
+    }
+
+    const normalizedCurrent = normalizeBlocksForComparison(currentBlocks)
+    const normalizedDeployed = normalizeBlocksForComparison(deployedBlocks)
+
+    // Should be identical since only metadata changed
+    expect(JSON.stringify(normalizedCurrent)).toBe(JSON.stringify(normalizedDeployed))
+  })
+})

apps/sim/app/w/[id]/components/control-bar/control-bar.tsx

@@ -45,6 +45,8 @@ import { useNotificationStore } from '@/stores/notifications/store'
 import { usePanelStore } from '@/stores/panel/store'
 import { useGeneralStore } from '@/stores/settings/general/store'
 import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
+import { useSubBlockStore } from '@/stores/workflows/subblock/store'
+import { mergeSubblockState } from '@/stores/workflows/utils'
 import { useWorkflowStore } from '@/stores/workflows/workflow/store'
 import type { WorkflowState } from '@/stores/workflows/workflow/types'
 import {
@@ -56,8 +58,6 @@ import { DeploymentControls } from './components/deployment-controls/deployment-
 import { HistoryDropdownItem } from './components/history-dropdown-item/history-dropdown-item'
 import { MarketplaceModal } from './components/marketplace-modal/marketplace-modal'
 import { NotificationDropdownItem } from './components/notification-dropdown-item/notification-dropdown-item'
-import { useSubBlockStore } from '@/stores/workflows/subblock/store'
-import { mergeSubblockState } from '@/stores/workflows/utils'
 
 const logger = createLogger('ControlBar')
 
@@ -88,7 +88,8 @@ export function ControlBar() {
     showNotification,
     removeNotification,
   } = useNotificationStore()
-  const { history, revertToHistoryState, lastSaved, setNeedsRedeploymentFlag, blocks } = useWorkflowStore()
+  const { history, revertToHistoryState, lastSaved, setNeedsRedeploymentFlag, blocks } =
+    useWorkflowStore()
   const { workflowValues } = useSubBlockStore()
   const {
     workflows,
@@ -270,7 +271,7 @@ export function ControlBar() {
 
   // Get current store state for change detection
   const currentBlocks = useWorkflowStore((state) => state.blocks)
-  const subBlockValues = useSubBlockStore((state) => 
+  const subBlockValues = useSubBlockStore((state) =>
     activeWorkflowId ? state.workflowValues[activeWorkflowId] : null
   )
 
@@ -281,7 +282,7 @@ export function ControlBar() {
    */
   const normalizeBlocksForComparison = (blocks: Record<string, any>) => {
     if (!blocks) return []
-    
+
     return Object.values(blocks)
       .map((block: any) => ({
         type: block.type,
@@ -312,20 +313,21 @@ export function ControlBar() {
 
     // Get current workflow state merged with user inputs
     const currentMergedState = mergeSubblockState(currentBlocks, activeWorkflowId)
-    
+
     // Compare current state vs deployed state
     const deployedBlocks = deployedState?.blocks
     if (!deployedBlocks) {
       setChangeDetected(false)
       return
     }
-    
+
     // Normalize blocks for semantic comparison
     const normalizedCurrentBlocks = normalizeBlocksForComparison(currentMergedState)
     const normalizedDeployedBlocks = normalizeBlocksForComparison(deployedBlocks)
-    
+
     // Compare normalized states
-    const hasChanges = JSON.stringify(normalizedCurrentBlocks) !== JSON.stringify(normalizedDeployedBlocks)
+    const hasChanges =
+      JSON.stringify(normalizedCurrentBlocks) !== JSON.stringify(normalizedDeployedBlocks)
     setChangeDetected(hasChanges)
   }, [activeWorkflowId, deployedState, currentBlocks, subBlockValues, isLoadingDeployedState])
 

apps/sim/executor/handlers/condition/condition-handler.test.ts

@@ -303,7 +303,7 @@ describe('ConditionBlockHandler', () => {
       .mockReturnValueOnce('context.value === 99')
 
     await expect(handler.execute(mockBlock, inputs, mockContext)).rejects.toThrow(
-      `No matching path found for condition block ${mockBlock.id}, and no 'else' block exists.`
+      `No matching path found for condition block "${mockBlock.metadata?.name}", and no 'else' block exists.`
     )
   })
 

apps/sim/executor/handlers/condition/condition-handler.ts

@@ -173,12 +173,12 @@ export class ConditionBlockHandler implements BlockHandler {
           selectedCondition = elseCondition
         } else {
           throw new Error(
-            `No path found for condition block ${block.id}, and 'else' connection missing.`
+            `No path found for condition block "${block.metadata?.name}", and 'else' connection missing.`
           )
         }
       } else {
         throw new Error(
-          `No matching path found for condition block ${block.id}, and no 'else' block exists.`
+          `No matching path found for condition block "${block.metadata?.name}", and no 'else' block exists.`
         )
       }
     }