diff --git a/apps/sim/app/w/[id]/components/control-bar/components/deployment-controls/deployment-controls.test.ts b/apps/sim/app/w/[id]/components/control-bar/components/deployment-controls/deployment-controls.test.ts new file mode 100644 index 0000000000..b8c58096b9 --- /dev/null +++ b/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) + }) + }) +}) diff --git a/apps/sim/app/w/[id]/components/control-bar/control-bar.test.ts b/apps/sim/app/w/[id]/components/control-bar/control-bar.test.ts new file mode 100644 index 0000000000..9159594aad --- /dev/null +++ b/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) => { + 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)) + }) +}) diff --git a/apps/sim/app/w/[id]/components/control-bar/control-bar.tsx b/apps/sim/app/w/[id]/components/control-bar/control-bar.tsx index 252e7a2125..1e824189db 100644 --- a/apps/sim/app/w/[id]/components/control-bar/control-bar.tsx +++ b/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) => { 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]) diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index 507ae672d0..ca95548652 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/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.` ) }) diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index a6cf3aa5da..2d90ffc00f 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/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.` ) } }