From d7c0364cad577a88215b4aed04e11a19c6076174 Mon Sep 17 00:00:00 2001 From: waleed Date: Sat, 31 Jan 2026 13:45:20 -0800 Subject: [PATCH] fix(workflow): optimize loop/parallel regeneration and prevent duplicate agent tools --- .../components/tool-input/tool-input.test.ts | 443 ++++++++++++++++++ .../components/tool-input/tool-input.tsx | 168 ++++--- .../stores/workflows/workflow/store.test.ts | 347 ++++++++++++++ apps/sim/stores/workflows/workflow/store.ts | 126 +++-- 4 files changed, 981 insertions(+), 103 deletions(-) create mode 100644 apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.test.ts diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.test.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.test.ts new file mode 100644 index 000000000..8d2548c13 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.test.ts @@ -0,0 +1,443 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' + +interface StoredTool { + type: string + title?: string + toolId?: string + params?: Record + customToolId?: string + schema?: any + code?: string + operation?: string + usageControl?: 'auto' | 'force' | 'none' +} + +const isMcpToolAlreadySelected = (selectedTools: StoredTool[], mcpToolId: string): boolean => { + return selectedTools.some((tool) => tool.type === 'mcp' && tool.toolId === mcpToolId) +} + +const isCustomToolAlreadySelected = ( + selectedTools: StoredTool[], + customToolId: string +): boolean => { + return selectedTools.some( + (tool) => tool.type === 'custom-tool' && tool.customToolId === customToolId + ) +} + +const isWorkflowAlreadySelected = (selectedTools: StoredTool[], workflowId: string): boolean => { + return selectedTools.some( + (tool) => tool.type === 'workflow_input' && tool.params?.workflowId === workflowId + ) +} + +describe('isMcpToolAlreadySelected', () => { + describe('basic functionality', () => { + it.concurrent('returns false when selectedTools is empty', () => { + expect(isMcpToolAlreadySelected([], 'mcp-tool-123')).toBe(false) + }) + + it.concurrent('returns false when MCP tool is not in selectedTools', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'different-mcp-tool', title: 'Different Tool' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-123')).toBe(false) + }) + + it.concurrent('returns true when MCP tool is already selected', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-123', title: 'My MCP Tool' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-123')).toBe(true) + }) + + it.concurrent('returns true when MCP tool is one of many selected tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'custom-1' }, + { type: 'mcp', toolId: 'mcp-tool-123', title: 'My MCP Tool' }, + { type: 'workflow_input', toolId: 'workflow_executor' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-123')).toBe(true) + }) + }) + + describe('type discrimination', () => { + it.concurrent('does not match non-MCP tools with same toolId', () => { + const selectedTools: StoredTool[] = [{ type: 'http_request', toolId: 'mcp-tool-123' }] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-123')).toBe(false) + }) + + it.concurrent('does not match custom tools even with toolId set', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', toolId: 'custom-mcp-tool-123', customToolId: 'db-id' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-123')).toBe(false) + }) + }) + + describe('multiple MCP tools', () => { + it.concurrent('correctly identifies first of multiple MCP tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-1', title: 'Tool 1' }, + { type: 'mcp', toolId: 'mcp-tool-2', title: 'Tool 2' }, + { type: 'mcp', toolId: 'mcp-tool-3', title: 'Tool 3' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-1')).toBe(true) + }) + + it.concurrent('correctly identifies middle MCP tool', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-1', title: 'Tool 1' }, + { type: 'mcp', toolId: 'mcp-tool-2', title: 'Tool 2' }, + { type: 'mcp', toolId: 'mcp-tool-3', title: 'Tool 3' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-2')).toBe(true) + }) + + it.concurrent('correctly identifies last MCP tool', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-1', title: 'Tool 1' }, + { type: 'mcp', toolId: 'mcp-tool-2', title: 'Tool 2' }, + { type: 'mcp', toolId: 'mcp-tool-3', title: 'Tool 3' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-3')).toBe(true) + }) + + it.concurrent('returns false for non-existent MCP tool among many', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-1', title: 'Tool 1' }, + { type: 'mcp', toolId: 'mcp-tool-2', title: 'Tool 2' }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'mcp-tool-999')).toBe(false) + }) + }) +}) + +describe('isCustomToolAlreadySelected', () => { + describe('basic functionality', () => { + it.concurrent('returns false when selectedTools is empty', () => { + expect(isCustomToolAlreadySelected([], 'custom-tool-123')).toBe(false) + }) + + it.concurrent('returns false when custom tool is not in selectedTools', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'different-custom-tool' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(false) + }) + + it.concurrent('returns true when custom tool is already selected', () => { + const selectedTools: StoredTool[] = [{ type: 'custom-tool', customToolId: 'custom-tool-123' }] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(true) + }) + + it.concurrent('returns true when custom tool is one of many selected tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-1', title: 'MCP Tool' }, + { type: 'custom-tool', customToolId: 'custom-tool-123' }, + { type: 'http_request', toolId: 'http_request_tool' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(true) + }) + }) + + describe('type discrimination', () => { + it.concurrent('does not match non-custom tools with similar IDs', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'custom-tool-123', title: 'MCP with similar ID' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(false) + }) + + it.concurrent('does not match MCP tools even if customToolId happens to match', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-id', customToolId: 'custom-tool-123' } as StoredTool, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(false) + }) + }) + + describe('legacy inline custom tools', () => { + it.concurrent('does not match legacy inline tools without customToolId', () => { + const selectedTools: StoredTool[] = [ + { + type: 'custom-tool', + title: 'Legacy Tool', + toolId: 'custom-myFunction', + schema: { function: { name: 'myFunction' } }, + code: 'return true', + }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(false) + }) + + it.concurrent('does not false-positive on legacy tools when checking for database tool', () => { + const selectedTools: StoredTool[] = [ + { + type: 'custom-tool', + title: 'Legacy Tool', + schema: { function: { name: 'sameName' } }, + code: 'return true', + }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'db-tool-1')).toBe(false) + }) + }) + + describe('multiple custom tools', () => { + it.concurrent('correctly identifies first of multiple custom tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'custom-1' }, + { type: 'custom-tool', customToolId: 'custom-2' }, + { type: 'custom-tool', customToolId: 'custom-3' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-1')).toBe(true) + }) + + it.concurrent('correctly identifies middle custom tool', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'custom-1' }, + { type: 'custom-tool', customToolId: 'custom-2' }, + { type: 'custom-tool', customToolId: 'custom-3' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-2')).toBe(true) + }) + + it.concurrent('correctly identifies last custom tool', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'custom-1' }, + { type: 'custom-tool', customToolId: 'custom-2' }, + { type: 'custom-tool', customToolId: 'custom-3' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-3')).toBe(true) + }) + + it.concurrent('returns false for non-existent custom tool among many', () => { + const selectedTools: StoredTool[] = [ + { type: 'custom-tool', customToolId: 'custom-1' }, + { type: 'custom-tool', customToolId: 'custom-2' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-999')).toBe(false) + }) + }) + + describe('mixed tool types', () => { + it.concurrent('correctly identifies custom tool in mixed list', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-tool-1', title: 'MCP Tool' }, + { type: 'custom-tool', customToolId: 'custom-tool-123' }, + { type: 'http_request', toolId: 'http_request' }, + { type: 'workflow_input', toolId: 'workflow_executor' }, + { type: 'custom-tool', title: 'Legacy', schema: {}, code: '' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-tool-123')).toBe(true) + }) + + it.concurrent('does not confuse MCP toolId with custom customToolId', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'shared-id-123', title: 'MCP Tool' }, + { type: 'custom-tool', customToolId: 'different-id' }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'shared-id-123')).toBe(false) + }) + }) +}) + +describe('isWorkflowAlreadySelected', () => { + describe('basic functionality', () => { + it.concurrent('returns false when selectedTools is empty', () => { + expect(isWorkflowAlreadySelected([], 'workflow-123')).toBe(false) + }) + + it.concurrent('returns false when workflow is not in selectedTools', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'different-workflow' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(false) + }) + + it.concurrent('returns true when workflow is already selected', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-123' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(true) + }) + + it.concurrent('returns true when workflow is one of many selected tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'mcp-1', title: 'MCP Tool' }, + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-123' }, + }, + { type: 'custom-tool', customToolId: 'custom-1' }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(true) + }) + }) + + describe('type discrimination', () => { + it.concurrent('does not match non-workflow_input tools', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'workflow-123', params: { workflowId: 'workflow-123' } }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(false) + }) + + it.concurrent('does not match workflow_input without params', () => { + const selectedTools: StoredTool[] = [{ type: 'workflow_input', toolId: 'workflow_executor' }] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(false) + }) + + it.concurrent('does not match workflow_input with different workflowId in params', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'other-workflow' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-123')).toBe(false) + }) + }) + + describe('multiple workflows', () => { + it.concurrent('allows different workflows to be selected', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-a' }, + }, + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-b' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-a')).toBe(true) + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-b')).toBe(true) + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-c')).toBe(false) + }) + + it.concurrent('correctly identifies specific workflow among many', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-1' }, + }, + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-2' }, + }, + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-3' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-2')).toBe(true) + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-999')).toBe(false) + }) + }) +}) + +describe('duplicate prevention integration scenarios', () => { + describe('add then try to re-add', () => { + it.concurrent('prevents re-adding the same MCP tool', () => { + const selectedTools: StoredTool[] = [ + { + type: 'mcp', + toolId: 'planetscale-query', + title: 'PlanetScale Query', + params: { serverId: 'server-1' }, + }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'planetscale-query')).toBe(true) + }) + + it.concurrent('prevents re-adding the same custom tool', () => { + const selectedTools: StoredTool[] = [ + { + type: 'custom-tool', + customToolId: 'my-custom-tool-uuid', + usageControl: 'auto', + }, + ] + expect(isCustomToolAlreadySelected(selectedTools, 'my-custom-tool-uuid')).toBe(true) + }) + + it.concurrent('prevents re-adding the same workflow', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'my-workflow-uuid' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'my-workflow-uuid')).toBe(true) + }) + }) + + describe('remove then re-add', () => { + it.concurrent('allows re-adding MCP tool after removal', () => { + const selectedToolsAfterRemoval: StoredTool[] = [] + expect(isMcpToolAlreadySelected(selectedToolsAfterRemoval, 'planetscale-query')).toBe(false) + }) + + it.concurrent('allows re-adding custom tool after removal', () => { + const selectedToolsAfterRemoval: StoredTool[] = [ + { type: 'mcp', toolId: 'some-other-tool', title: 'Other' }, + ] + expect(isCustomToolAlreadySelected(selectedToolsAfterRemoval, 'my-custom-tool-uuid')).toBe( + false + ) + }) + + it.concurrent('allows re-adding workflow after removal', () => { + const selectedToolsAfterRemoval: StoredTool[] = [ + { type: 'mcp', toolId: 'some-tool', title: 'Other' }, + ] + expect(isWorkflowAlreadySelected(selectedToolsAfterRemoval, 'my-workflow-uuid')).toBe(false) + }) + }) + + describe('different tools with similar names', () => { + it.concurrent('allows adding different MCP tools from same server', () => { + const selectedTools: StoredTool[] = [ + { type: 'mcp', toolId: 'server1-tool-a', title: 'Tool A', params: { serverId: 'server1' } }, + ] + expect(isMcpToolAlreadySelected(selectedTools, 'server1-tool-b')).toBe(false) + }) + + it.concurrent('allows adding different custom tools', () => { + const selectedTools: StoredTool[] = [{ type: 'custom-tool', customToolId: 'custom-a' }] + expect(isCustomToolAlreadySelected(selectedTools, 'custom-b')).toBe(false) + }) + + it.concurrent('allows adding different workflows', () => { + const selectedTools: StoredTool[] = [ + { + type: 'workflow_input', + toolId: 'workflow_executor', + params: { workflowId: 'workflow-a' }, + }, + ] + expect(isWorkflowAlreadySelected(selectedTools, 'workflow-b')).toBe(false) + }) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index c52e247ff..cd2f342a3 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -1226,6 +1226,40 @@ export const ToolInput = memo(function ToolInput({ return selectedTools.some((tool) => tool.toolId === toolId) } + /** + * Checks if an MCP tool is already selected. + * + * @param mcpToolId - The MCP tool identifier to check + * @returns `true` if the MCP tool is already selected + */ + const isMcpToolAlreadySelected = (mcpToolId: string): boolean => { + return selectedTools.some((tool) => tool.type === 'mcp' && tool.toolId === mcpToolId) + } + + /** + * Checks if a custom tool is already selected. + * + * @param customToolId - The custom tool identifier to check + * @returns `true` if the custom tool is already selected + */ + const isCustomToolAlreadySelected = (customToolId: string): boolean => { + return selectedTools.some( + (tool) => tool.type === 'custom-tool' && tool.customToolId === customToolId + ) + } + + /** + * Checks if a workflow is already selected. + * + * @param workflowId - The workflow identifier to check + * @returns `true` if the workflow is already selected + */ + const isWorkflowAlreadySelected = (workflowId: string): boolean => { + return selectedTools.some( + (tool) => tool.type === 'workflow_input' && tool.params?.workflowId === workflowId + ) + } + /** * Checks if a block supports multiple operations. * @@ -1745,24 +1779,29 @@ export const ToolInput = memo(function ToolInput({ if (!permissionConfig.disableCustomTools && customTools.length > 0) { groups.push({ section: 'Custom Tools', - items: customTools.map((customTool) => ({ - label: customTool.title, - value: `custom-${customTool.id}`, - iconElement: createToolIcon('#3B82F6', WrenchIcon), - onSelect: () => { - const newTool: StoredTool = { - type: 'custom-tool', - customToolId: customTool.id, - usageControl: 'auto', - isExpanded: true, - } - setStoreValue([ - ...selectedTools.map((tool) => ({ ...tool, isExpanded: false })), - newTool, - ]) - setOpen(false) - }, - })), + items: customTools.map((customTool) => { + const alreadySelected = isCustomToolAlreadySelected(customTool.id) + return { + label: customTool.title, + value: `custom-${customTool.id}`, + iconElement: createToolIcon('#3B82F6', WrenchIcon), + disabled: isPreview || alreadySelected, + onSelect: () => { + if (alreadySelected) return + const newTool: StoredTool = { + type: 'custom-tool', + customToolId: customTool.id, + usageControl: 'auto', + isExpanded: true, + } + setStoreValue([ + ...selectedTools.map((tool) => ({ ...tool, isExpanded: false })), + newTool, + ]) + setOpen(false) + }, + } + }), }) } @@ -1772,11 +1811,13 @@ export const ToolInput = memo(function ToolInput({ section: 'MCP Tools', items: availableMcpTools.map((mcpTool) => { const server = mcpServers.find((s) => s.id === mcpTool.serverId) + const alreadySelected = isMcpToolAlreadySelected(mcpTool.id) return { label: mcpTool.name, value: `mcp-${mcpTool.id}`, iconElement: createToolIcon(mcpTool.bgColor || '#6366F1', mcpTool.icon || McpIcon), onSelect: () => { + if (alreadySelected) return const newTool: StoredTool = { type: 'mcp', title: mcpTool.name, @@ -1796,7 +1837,7 @@ export const ToolInput = memo(function ToolInput({ } handleMcpToolSelect(newTool, true) }, - disabled: isPreview || disabled, + disabled: isPreview || disabled || alreadySelected, } }), }) @@ -1810,12 +1851,17 @@ export const ToolInput = memo(function ToolInput({ if (builtInTools.length > 0) { groups.push({ section: 'Built-in Tools', - items: builtInTools.map((block) => ({ - label: block.name, - value: `builtin-${block.type}`, - iconElement: createToolIcon(block.bgColor, block.icon), - onSelect: () => handleSelectTool(block), - })), + items: builtInTools.map((block) => { + const toolId = getToolIdForOperation(block.type, undefined) + const alreadySelected = toolId ? isToolAlreadySelected(toolId, block.type) : false + return { + label: block.name, + value: `builtin-${block.type}`, + iconElement: createToolIcon(block.bgColor, block.icon), + disabled: isPreview || alreadySelected, + onSelect: () => handleSelectTool(block), + } + }), }) } @@ -1823,12 +1869,17 @@ export const ToolInput = memo(function ToolInput({ if (integrations.length > 0) { groups.push({ section: 'Integrations', - items: integrations.map((block) => ({ - label: block.name, - value: `builtin-${block.type}`, - iconElement: createToolIcon(block.bgColor, block.icon), - onSelect: () => handleSelectTool(block), - })), + items: integrations.map((block) => { + const toolId = getToolIdForOperation(block.type, undefined) + const alreadySelected = toolId ? isToolAlreadySelected(toolId, block.type) : false + return { + label: block.name, + value: `builtin-${block.type}`, + iconElement: createToolIcon(block.bgColor, block.icon), + disabled: isPreview || alreadySelected, + onSelect: () => handleSelectTool(block), + } + }), }) } @@ -1836,29 +1887,33 @@ export const ToolInput = memo(function ToolInput({ if (availableWorkflows.length > 0) { groups.push({ section: 'Workflows', - items: availableWorkflows.map((workflow) => ({ - label: workflow.name, - value: `workflow-${workflow.id}`, - iconElement: createToolIcon('#6366F1', WorkflowIcon), - onSelect: () => { - const newTool: StoredTool = { - type: 'workflow_input', - title: 'Workflow', - toolId: 'workflow_executor', - params: { - workflowId: workflow.id, - }, - isExpanded: true, - usageControl: 'auto', - } - setStoreValue([ - ...selectedTools.map((tool) => ({ ...tool, isExpanded: false })), - newTool, - ]) - setOpen(false) - }, - disabled: isPreview || disabled, - })), + items: availableWorkflows.map((workflow) => { + const alreadySelected = isWorkflowAlreadySelected(workflow.id) + return { + label: workflow.name, + value: `workflow-${workflow.id}`, + iconElement: createToolIcon('#6366F1', WorkflowIcon), + onSelect: () => { + if (alreadySelected) return + const newTool: StoredTool = { + type: 'workflow_input', + title: 'Workflow', + toolId: 'workflow_executor', + params: { + workflowId: workflow.id, + }, + isExpanded: true, + usageControl: 'auto', + } + setStoreValue([ + ...selectedTools.map((tool) => ({ ...tool, isExpanded: false })), + newTool, + ]) + setOpen(false) + }, + disabled: isPreview || disabled || alreadySelected, + } + }), }) } @@ -1877,6 +1932,11 @@ export const ToolInput = memo(function ToolInput({ permissionConfig.disableCustomTools, permissionConfig.disableMcpTools, availableWorkflows, + getToolIdForOperation, + isToolAlreadySelected, + isMcpToolAlreadySelected, + isCustomToolAlreadySelected, + isWorkflowAlreadySelected, ]) const toolRequiresOAuth = (toolId: string): boolean => { diff --git a/apps/sim/stores/workflows/workflow/store.test.ts b/apps/sim/stores/workflows/workflow/store.test.ts index 1ed122c23..467374ca9 100644 --- a/apps/sim/stores/workflows/workflow/store.test.ts +++ b/apps/sim/stores/workflows/workflow/store.test.ts @@ -782,6 +782,353 @@ describe('workflow store', () => { }) }) + describe('loop/parallel regeneration optimization', () => { + it('should NOT regenerate loops when adding a regular block without parentId', () => { + const { addBlock } = useWorkflowStore.getState() + + // Add a loop first + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + + const stateAfterLoop = useWorkflowStore.getState() + const loopsAfterLoop = stateAfterLoop.loops + + // Add a regular block (no parentId) + addBlock('agent-1', 'agent', 'Agent 1', { x: 200, y: 0 }) + + const stateAfterAgent = useWorkflowStore.getState() + + // Loops should be unchanged (same content) + expect(Object.keys(stateAfterAgent.loops)).toEqual(Object.keys(loopsAfterLoop)) + expect(stateAfterAgent.loops['loop-1'].nodes).toEqual(loopsAfterLoop['loop-1'].nodes) + }) + + it('should regenerate loops when adding a child to a loop', () => { + const { addBlock } = useWorkflowStore.getState() + + // Add a loop + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + + const stateAfterLoop = useWorkflowStore.getState() + expect(stateAfterLoop.loops['loop-1'].nodes).toEqual([]) + + // Add a child block to the loop + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'loop-1' }, + 'loop-1', + 'parent' + ) + + const stateAfterChild = useWorkflowStore.getState() + + // Loop should now include the child + expect(stateAfterChild.loops['loop-1'].nodes).toContain('child-1') + }) + + it('should NOT regenerate parallels when adding a child to a loop', () => { + const { addBlock } = useWorkflowStore.getState() + + // Add both a loop and a parallel + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + addBlock('parallel-1', 'parallel', 'Parallel 1', { x: 300, y: 0 }, { count: 3 }) + + const stateAfterContainers = useWorkflowStore.getState() + const parallelsAfterContainers = stateAfterContainers.parallels + + // Add a child to the loop (not the parallel) + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'loop-1' }, + 'loop-1', + 'parent' + ) + + const stateAfterChild = useWorkflowStore.getState() + + // Parallels should be unchanged + expect(stateAfterChild.parallels['parallel-1'].nodes).toEqual( + parallelsAfterContainers['parallel-1'].nodes + ) + }) + + it('should regenerate parallels when adding a child to a parallel', () => { + const { addBlock } = useWorkflowStore.getState() + + // Add a parallel + addBlock('parallel-1', 'parallel', 'Parallel 1', { x: 0, y: 0 }, { count: 3 }) + + const stateAfterParallel = useWorkflowStore.getState() + expect(stateAfterParallel.parallels['parallel-1'].nodes).toEqual([]) + + // Add a child block to the parallel + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'parallel-1' }, + 'parallel-1', + 'parent' + ) + + const stateAfterChild = useWorkflowStore.getState() + + // Parallel should now include the child + expect(stateAfterChild.parallels['parallel-1'].nodes).toContain('child-1') + }) + + it('should handle adding blocks in any order and produce correct final state', () => { + const { addBlock } = useWorkflowStore.getState() + + // Add child BEFORE the loop (simulating undo-redo edge case) + // Note: The child's parentId points to a loop that doesn't exist yet + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'loop-1' }, + 'loop-1', + 'parent' + ) + + // At this point, the child exists but loop doesn't + const stateAfterChild = useWorkflowStore.getState() + expect(stateAfterChild.blocks['child-1']).toBeDefined() + expect(stateAfterChild.loops['loop-1']).toBeUndefined() + + // Now add the loop + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + + // Final state should be correct - loop should include the child + const finalState = useWorkflowStore.getState() + expect(finalState.loops['loop-1']).toBeDefined() + expect(finalState.loops['loop-1'].nodes).toContain('child-1') + }) + }) + + describe('batchAddBlocks optimization', () => { + it('should NOT regenerate loops/parallels when adding regular blocks', () => { + const { batchAddBlocks } = useWorkflowStore.getState() + + // Set up initial state with a loop + useWorkflowStore.setState({ + blocks: { + 'loop-1': { + id: 'loop-1', + type: 'loop', + name: 'Loop 1', + position: { x: 0, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + horizontalHandles: true, + advancedMode: false, + triggerMode: false, + height: 0, + data: { loopType: 'for', count: 5 }, + }, + }, + edges: [], + loops: { + 'loop-1': { + id: 'loop-1', + nodes: [], + iterations: 5, + loopType: 'for', + enabled: true, + }, + }, + parallels: {}, + }) + + const stateBefore = useWorkflowStore.getState() + + // Add regular blocks (no parentId, not loop/parallel type) + batchAddBlocks([ + { + id: 'agent-1', + type: 'agent', + name: 'Agent 1', + position: { x: 200, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + }, + { + id: 'function-1', + type: 'function', + name: 'Function 1', + position: { x: 400, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + }, + ]) + + const stateAfter = useWorkflowStore.getState() + + // Loops should be unchanged + expect(stateAfter.loops['loop-1'].nodes).toEqual(stateBefore.loops['loop-1'].nodes) + }) + + it('should regenerate loops when batch adding a loop block', () => { + const { batchAddBlocks } = useWorkflowStore.getState() + + batchAddBlocks([ + { + id: 'loop-1', + type: 'loop', + name: 'Loop 1', + position: { x: 0, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + data: { loopType: 'for', count: 5 }, + }, + ]) + + const state = useWorkflowStore.getState() + expect(state.loops['loop-1']).toBeDefined() + expect(state.loops['loop-1'].iterations).toBe(5) + }) + + it('should regenerate loops when batch adding a child of a loop', () => { + const { batchAddBlocks } = useWorkflowStore.getState() + + // First add a loop + batchAddBlocks([ + { + id: 'loop-1', + type: 'loop', + name: 'Loop 1', + position: { x: 0, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + data: { loopType: 'for', count: 5 }, + }, + ]) + + // Then add a child + batchAddBlocks([ + { + id: 'child-1', + type: 'function', + name: 'Child 1', + position: { x: 50, y: 50 }, + subBlocks: {}, + outputs: {}, + enabled: true, + data: { parentId: 'loop-1' }, + }, + ]) + + const state = useWorkflowStore.getState() + expect(state.loops['loop-1'].nodes).toContain('child-1') + }) + + it('should correctly handle batch adding loop and its children together', () => { + const { batchAddBlocks } = useWorkflowStore.getState() + + // Add loop and child in same batch + batchAddBlocks([ + { + id: 'loop-1', + type: 'loop', + name: 'Loop 1', + position: { x: 0, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + data: { loopType: 'for', count: 5 }, + }, + { + id: 'child-1', + type: 'function', + name: 'Child 1', + position: { x: 50, y: 50 }, + subBlocks: {}, + outputs: {}, + enabled: true, + data: { parentId: 'loop-1' }, + }, + ]) + + const state = useWorkflowStore.getState() + expect(state.loops['loop-1']).toBeDefined() + expect(state.loops['loop-1'].nodes).toContain('child-1') + }) + }) + + describe('edge operations should not affect loops/parallels', () => { + it('should preserve loops when adding edges', () => { + const { addBlock, batchAddEdges } = useWorkflowStore.getState() + + // Create a loop with a child + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'loop-1' }, + 'loop-1', + 'parent' + ) + addBlock('external-1', 'function', 'External', { x: 300, y: 0 }) + + const stateBeforeEdge = useWorkflowStore.getState() + const loopsBeforeEdge = stateBeforeEdge.loops + + // Add an edge (should not affect loops) + batchAddEdges([{ id: 'e1', source: 'loop-1', target: 'external-1' }]) + + const stateAfterEdge = useWorkflowStore.getState() + + // Loops should be unchanged + expect(stateAfterEdge.loops['loop-1'].nodes).toEqual(loopsBeforeEdge['loop-1'].nodes) + expect(stateAfterEdge.loops['loop-1'].iterations).toEqual( + loopsBeforeEdge['loop-1'].iterations + ) + }) + + it('should preserve loops when removing edges', () => { + const { addBlock, batchAddEdges, batchRemoveEdges } = useWorkflowStore.getState() + + // Create a loop with a child and an edge + addBlock('loop-1', 'loop', 'Loop 1', { x: 0, y: 0 }, { loopType: 'for', count: 5 }) + addBlock( + 'child-1', + 'function', + 'Child 1', + { x: 50, y: 50 }, + { parentId: 'loop-1' }, + 'loop-1', + 'parent' + ) + addBlock('external-1', 'function', 'External', { x: 300, y: 0 }) + batchAddEdges([{ id: 'e1', source: 'loop-1', target: 'external-1' }]) + + const stateBeforeRemove = useWorkflowStore.getState() + const loopsBeforeRemove = stateBeforeRemove.loops + + // Remove the edge + batchRemoveEdges(['e1']) + + const stateAfterRemove = useWorkflowStore.getState() + + // Loops should be unchanged + expect(stateAfterRemove.loops['loop-1'].nodes).toEqual(loopsBeforeRemove['loop-1'].nodes) + }) + }) + describe('updateBlockName', () => { beforeEach(() => { useWorkflowStore.setState({ diff --git a/apps/sim/stores/workflows/workflow/store.ts b/apps/sim/stores/workflows/workflow/store.ts index 00eeac9b8..64e9a6044 100644 --- a/apps/sim/stores/workflows/workflow/store.ts +++ b/apps/sim/stores/workflows/workflow/store.ts @@ -139,30 +139,32 @@ export const useWorkflowStore = create()( ...(parentId && { parentId, extent: extent || 'parent' }), } - const newState = { - blocks: { - ...get().blocks, - [id]: { - id, - type, - name, - position, - subBlocks: {}, - outputs: {}, - enabled: blockProperties?.enabled ?? true, - horizontalHandles: blockProperties?.horizontalHandles ?? true, - advancedMode: blockProperties?.advancedMode ?? false, - triggerMode: blockProperties?.triggerMode ?? false, - height: blockProperties?.height ?? 0, - data: nodeData, - }, + const newBlocks = { + ...get().blocks, + [id]: { + id, + type, + name, + position, + subBlocks: {}, + outputs: {}, + enabled: blockProperties?.enabled ?? true, + horizontalHandles: blockProperties?.horizontalHandles ?? true, + advancedMode: blockProperties?.advancedMode ?? false, + triggerMode: blockProperties?.triggerMode ?? false, + height: blockProperties?.height ?? 0, + data: nodeData, }, - edges: [...get().edges], - loops: get().generateLoopBlocks(), - parallels: get().generateParallelBlocks(), } - set(newState) + // Only regenerate the relevant container type + set({ + blocks: newBlocks, + edges: [...get().edges], + loops: type === 'loop' ? generateLoopBlocks(newBlocks) : { ...get().loops }, + parallels: + type === 'parallel' ? generateParallelBlocks(newBlocks) : { ...get().parallels }, + }) get().updateLastSaved() return } @@ -215,31 +217,39 @@ export const useWorkflowStore = create()( const triggerMode = blockProperties?.triggerMode ?? false const outputs = getBlockOutputs(type, subBlocks, triggerMode) - const newState = { - blocks: { - ...get().blocks, - [id]: { - id, - type, - name, - position, - subBlocks, - outputs, - enabled: blockProperties?.enabled ?? true, - horizontalHandles: blockProperties?.horizontalHandles ?? true, - advancedMode: blockProperties?.advancedMode ?? false, - triggerMode: triggerMode, - height: blockProperties?.height ?? 0, - layout: {}, - data: nodeData, - }, + const currentBlocks = get().blocks + const newBlocks = { + ...currentBlocks, + [id]: { + id, + type, + name, + position, + subBlocks, + outputs, + enabled: blockProperties?.enabled ?? true, + horizontalHandles: blockProperties?.horizontalHandles ?? true, + advancedMode: blockProperties?.advancedMode ?? false, + triggerMode: triggerMode, + height: blockProperties?.height ?? 0, + layout: {}, + data: nodeData, }, - edges: [...get().edges], - loops: get().generateLoopBlocks(), - parallels: get().generateParallelBlocks(), } - set(newState) + // Only regenerate loops/parallels if adding a block as a child of a container + const parentBlock = parentId ? currentBlocks[parentId] : null + const needsLoopRegeneration = parentBlock?.type === 'loop' + const needsParallelRegeneration = parentBlock?.type === 'parallel' + + set({ + blocks: newBlocks, + edges: [...get().edges], + loops: needsLoopRegeneration ? generateLoopBlocks(newBlocks) : { ...get().loops }, + parallels: needsParallelRegeneration + ? generateParallelBlocks(newBlocks) + : { ...get().parallels }, + }) get().updateLastSaved() }, @@ -386,11 +396,27 @@ export const useWorkflowStore = create()( } } + // Only regenerate loops/parallels if we're adding blocks that affect them: + // - Adding a loop/parallel container block + // - Adding a block as a child of a loop/parallel (has parentId pointing to one) + const needsLoopRegeneration = blocks.some( + (block) => + block.type === 'loop' || + (block.data?.parentId && newBlocks[block.data.parentId]?.type === 'loop') + ) + const needsParallelRegeneration = blocks.some( + (block) => + block.type === 'parallel' || + (block.data?.parentId && newBlocks[block.data.parentId]?.type === 'parallel') + ) + set({ blocks: newBlocks, edges: newEdges, - loops: generateLoopBlocks(newBlocks), - parallels: generateParallelBlocks(newBlocks), + loops: needsLoopRegeneration ? generateLoopBlocks(newBlocks) : { ...get().loops }, + parallels: needsParallelRegeneration + ? generateParallelBlocks(newBlocks) + : { ...get().parallels }, }) if (subBlockValues && Object.keys(subBlockValues).length > 0) { @@ -529,8 +555,9 @@ export const useWorkflowStore = create()( set({ blocks: { ...blocks }, edges: newEdges, - loops: generateLoopBlocks(blocks), - parallels: generateParallelBlocks(blocks), + // Edges don't affect loop/parallel structure (determined by parentId), skip regeneration + loops: { ...get().loops }, + parallels: { ...get().parallels }, }) get().updateLastSaved() @@ -544,8 +571,9 @@ export const useWorkflowStore = create()( set({ blocks: { ...blocks }, edges: newEdges, - loops: generateLoopBlocks(blocks), - parallels: generateParallelBlocks(blocks), + // Edges don't affect loop/parallel structure (determined by parentId), skip regeneration + loops: { ...get().loops }, + parallels: { ...get().parallels }, }) get().updateLastSaved()