diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx index f62015495af..0a73facbfae 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx @@ -34,6 +34,20 @@ const { enabled: true, subBlocks: {}, }, + condition1: { + id: 'condition1', + type: 'condition', + name: 'Check', + enabled: true, + subBlocks: {}, + }, + disabledBranch: { + id: 'disabledBranch', + type: 'slack', + name: 'Send Empty', + enabled: false, + subBlocks: {}, + }, } const idleExecution = { status: 'idle', @@ -73,12 +87,20 @@ const { finishRunningEntries: vi.fn(), clearExecutionEntries: vi.fn(), } + const workflowEdges = [ + { + id: 'edge-1', + source: 'condition1', + target: 'disabledBranch', + sourceHandle: 'condition-else1', + }, + ] const workflowStoreState = { blocks: workflowBlocks, - edges: [], + edges: workflowEdges, getWorkflowState: vi.fn(() => ({ blocks: workflowBlocks, - edges: [], + edges: workflowEdges, loops: {}, parallels: {}, })), @@ -471,3 +493,39 @@ describe('useWorkflowExecution attachment uploads', () => { unmount() }) }) + +describe('useWorkflowExecution workflow state override', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.stubGlobal('fetch', mockFetch) + mockExecute.mockResolvedValue(undefined) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('sends disabled blocks so no edge points at a block the executor cannot see', async () => { + const { result, unmount } = renderWorkflowExecutionHook() + + await act(async () => { + const runResult = await result().handleRunWorkflow({ + input: 'go', + conversationId: 'conversation-1', + }) + await drainStream(runResult) + }) + + expect(mockExecute).toHaveBeenCalledTimes(1) + const { workflowStateOverride } = mockExecute.mock.calls[0][0] + const sentBlockIds = new Set(Object.keys(workflowStateOverride.blocks)) + + expect(sentBlockIds.has('disabledBranch')).toBe(true) + for (const edge of workflowStateOverride.edges) { + expect(sentBlockIds.has(edge.source)).toBe(true) + expect(sentBlockIds.has(edge.target)).toBe(true) + } + + unmount() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts index 7c708170f14..50855e38d60 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts @@ -1009,10 +1009,13 @@ export function useWorkflowExecution() { const workflowEdges = (executionWorkflowState?.edges ?? latestWorkflowState.edges) as typeof currentWorkflow.edges - // Filter out blocks without type (these are layout-only blocks) and disabled blocks + // Filter out blocks without type (these are layout-only blocks). Disabled blocks are + // deliberately kept: the server DAG excludes them (and their edges) on its own, and + // dropping them here while sending every edge leaves edges pointing at blocks that no + // longer exist — which makes condition/router target resolution fail at runtime. const validBlocks = Object.entries(workflowBlocks).reduce( (acc, [blockId, block]) => { - if (block?.type && block.enabled !== false) { + if (block?.type) { acc[blockId] = block } return acc @@ -1050,24 +1053,31 @@ export function useWorkflowExecution() { } }) - // Filter out blocks without type and disabled blocks + // Filter out blocks without type. Disabled blocks stay in the payload so blocks and + // edges remain consistent; the executor's DAG builder drops them and their edges. const filteredStates = Object.entries(mergedStates).reduce( (acc, [id, block]) => { if (!block || !block.type) { logger.warn(`Skipping block with undefined type: ${id}`, block) return acc } - // Skip disabled blocks to prevent them from being passed to executor - if (block.enabled === false) { - logger.warn(`Skipping disabled block: ${id}`) - return acc - } acc[id] = block return acc }, {} as typeof mergedStates ) + /** Trigger resolution must never select a disabled trigger. */ + const enabledStates = Object.entries(filteredStates).reduce( + (acc, [id, block]) => { + if (block.enabled !== false) { + acc[id] = block + } + return acc + }, + {} as typeof filteredStates + ) + // If this is a chat execution, get the selected outputs let selectedOutputs: string[] | undefined if (isExecutingFromChat && activeWorkflowId) { @@ -1082,7 +1092,7 @@ export function useWorkflowExecution() { if (isExecutingFromChat) { // For chat execution, find the appropriate chat trigger - const startBlock = TriggerUtils.findStartBlock(filteredStates, 'chat') + const startBlock = TriggerUtils.findStartBlock(enabledStates, 'chat') if (!startBlock) { throw new WorkflowValidationError( @@ -1096,7 +1106,7 @@ export function useWorkflowExecution() { startBlockId = startBlock.blockId } else { // Manual execution: detect and group triggers by paths - const candidates = resolveStartCandidates(filteredStates, { + const candidates = resolveStartCandidates(enabledStates, { execution: 'manual', }) @@ -1108,7 +1118,7 @@ export function useWorkflowExecution() { 'Workflow Validation' ) logger.error('No trigger blocks found for manual run', { - allBlockTypes: Object.values(filteredStates).map((b) => b.type), + allBlockTypes: Object.values(enabledStates).map((b) => b.type), }) if (activeWorkflowId) setIsExecuting(activeWorkflowId, false) throw error diff --git a/apps/sim/executor/handlers/condition/condition-handler.test.ts b/apps/sim/executor/handlers/condition/condition-handler.test.ts index f892dff84e3..092d94fdc26 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.test.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.test.ts @@ -288,7 +288,7 @@ describe('ConditionBlockHandler', () => { expect(result).toHaveProperty('selectedOption', 'cond1') }) - it('should throw error if target block is missing', async () => { + it('dead-ends the branch when the target block is missing', async () => { mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) const conditions = [{ id: 'cond1', title: 'if', value: 'true' }] @@ -296,9 +296,27 @@ describe('ConditionBlockHandler', () => { mockContext.workflow!.blocks = [mockSourceBlock, mockBlock, mockTargetBlock2] - await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( - `Target block ${mockTargetBlock1.id} not found` - ) + const result = await handler.execute(mockContext, mockBlock, inputs) + + expect(result).toHaveProperty('conditionResult', true) + expect((result as any).selectedOption).toBe('cond1') + expect((result as any).selectedPath).toBeNull() + }) + + it('dead-ends the branch when the target block is disabled', async () => { + mockExecuteTool.mockResolvedValueOnce({ success: true, output: { result: true } }) + + const conditions = [{ id: 'cond1', title: 'if', value: 'true' }] + const inputs = { conditions: JSON.stringify(conditions) } + + mockTargetBlock1.enabled = false + + const result = await handler.execute(mockContext, mockBlock, inputs) + + expect(result).toHaveProperty('conditionResult', true) + expect((result as any).selectedOption).toBe('cond1') + expect((result as any).selectedPath).toBeNull() + expect(mockContext.decisions.condition.get(mockBlock.id)).toBe('cond1') }) it('should return no-match result if no condition matches and no else exists', async () => { diff --git a/apps/sim/executor/handlers/condition/condition-handler.ts b/apps/sim/executor/handlers/condition/condition-handler.ts index d329ee4a2c2..11a465e2cd3 100644 --- a/apps/sim/executor/handlers/condition/condition-handler.ts +++ b/apps/sim/executor/handlers/condition/condition-handler.ts @@ -143,14 +143,32 @@ export class ConditionBlockHandler implements BlockHandler { } } - const targetBlock = ctx.workflow?.blocks.find((b) => b.id === selectedConnection?.target) - if (!targetBlock) { - throw new Error(`Target block ${selectedConnection?.target} not found`) - } + const targetBlock = ctx.workflow?.blocks.find((b) => b.id === selectedConnection.target) const decisionKey = ctx.currentVirtualBlockId || block.id ctx.decisions.condition.set(decisionKey, selectedCondition.id) + /** + * A branch whose target is disabled (or no longer exists) is a dead end, not a + * failure: the condition still resolves, and the DAG — which excludes disabled + * blocks and their edges — simply has nothing left to activate on this path. + */ + if (!targetBlock || targetBlock.enabled === false) { + logger.info('Condition branch target is not executable; path ends here', { + blockId: block.id, + conditionId: selectedCondition.id, + targetBlockId: selectedConnection.target, + reason: targetBlock ? 'disabled' : 'missing', + }) + + return { + ...((sourceOutput as any) || {}), + conditionResult: true, + selectedPath: null, + selectedOption: selectedCondition.id, + } + } + return { ...((sourceOutput as any) || {}), conditionResult: true, diff --git a/apps/sim/executor/handlers/router/router-handler.test.ts b/apps/sim/executor/handlers/router/router-handler.test.ts index 3c57fe2de8f..cfbf2c41f5e 100644 --- a/apps/sim/executor/handlers/router/router-handler.test.ts +++ b/apps/sim/executor/handlers/router/router-handler.test.ts @@ -232,13 +232,55 @@ describe('RouterBlockHandler', () => { }) }) - it('should throw error if target block is missing', async () => { - const inputs = { prompt: 'Test' } + it('excludes a missing target block from the routing candidates', async () => { mockContext.workflow!.blocks = [mockBlock, mockTargetBlock2] - await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( - 'Target block target-block-1 not found' + mockFetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ content: 'target-block-2', model: 'mock-model' }), + }) + ) + + const result = await handler.execute(mockContext, mockBlock, { prompt: 'Test' }) + + expect(mockGenerateRouterPrompt).toHaveBeenCalledWith('Test', [ + expect.objectContaining({ id: 'target-block-2' }), + ]) + expect((result as { selectedRoute: string }).selectedRoute).toBe('target-block-2') + }) + + it('excludes a disabled target block from the routing candidates', async () => { + mockTargetBlock1.enabled = false + + mockFetch.mockImplementationOnce(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ content: 'target-block-2', model: 'mock-model' }), + }) ) + + const result = await handler.execute(mockContext, mockBlock, { prompt: 'Test' }) + + expect(mockGenerateRouterPrompt).toHaveBeenCalledWith('Test', [ + expect.objectContaining({ id: 'target-block-2' }), + ]) + expect((result as { selectedRoute: string }).selectedRoute).toBe('target-block-2') + }) + + it('dead-ends without calling the model when every target block is disabled', async () => { + mockTargetBlock1.enabled = false + mockTargetBlock2.enabled = false + + const result = (await handler.execute(mockContext, mockBlock, { prompt: 'Test' })) as { + selectedRoute: string | null + selectedPath: unknown + cost: { total: number } + } + + expect(result.selectedRoute).toBeNull() + expect(result.selectedPath).toBeNull() + expect(result.cost.total).toBe(0) expect(mockFetch).not.toHaveBeenCalled() }) diff --git a/apps/sim/executor/handlers/router/router-handler.ts b/apps/sim/executor/handlers/router/router-handler.ts index 55184c40611..1b4e00735df 100644 --- a/apps/sim/executor/handlers/router/router-handler.ts +++ b/apps/sim/executor/handlers/router/router-handler.ts @@ -60,6 +60,31 @@ export class RouterBlockHandler implements BlockHandler { ): Promise { const targetBlocks = this.getTargetBlocks(ctx, block) + /** + * With nothing executable to route to there is no decision to make, so the path ends + * here rather than erroring — the same dead end a condition branch reaches when its + * target is disabled. Asking the model to choose from an empty list would only burn a + * call to produce an unusable answer. + */ + if (!targetBlocks || targetBlocks.length === 0) { + logger.warn('Router has no executable target blocks; path ends here', { + blockId: block.id, + }) + + return { + prompt: inputs.prompt, + model: inputs.model || ROUTER.DEFAULT_MODEL, + tokens: { + input: DEFAULTS.TOKENS.PROMPT, + output: DEFAULTS.TOKENS.COMPLETION, + total: DEFAULTS.TOKENS.TOTAL, + }, + cost: { input: 0, output: 0, total: 0 }, + selectedPath: null, + selectedRoute: null, + } as BlockOutput + } + const routerConfig = { prompt: inputs.prompt, model: inputs.model || ROUTER.DEFAULT_MODEL, @@ -380,13 +405,22 @@ export class RouterBlockHandler implements BlockHandler { } } + /** + * Candidate blocks the router may route to. Disabled (and missing) targets are + * excluded so the model is never offered a route the DAG cannot execute. + */ private getTargetBlocks(ctx: ExecutionContext, block: SerializedBlock) { return ctx.workflow?.connections .filter((conn) => conn.source === block.id) - .map((conn) => { + .flatMap((conn) => { const targetBlock = ctx.workflow?.blocks.find((b) => b.id === conn.target) - if (!targetBlock) { - throw new Error(`Target block ${conn.target} not found`) + if (!targetBlock || targetBlock.enabled === false) { + logger.info('Skipping router target that is not executable', { + blockId: block.id, + targetBlockId: conn.target, + reason: targetBlock ? 'disabled' : 'missing', + }) + return [] } let systemPrompt = '' @@ -399,17 +433,19 @@ export class RouterBlockHandler implements BlockHandler { '' } - return { - id: targetBlock.id, - type: targetBlock.metadata?.id, - title: targetBlock.metadata?.name, - description: targetBlock.metadata?.description, - subBlocks: { - ...targetBlock.config.params, - systemPrompt: systemPrompt, + return [ + { + id: targetBlock.id, + type: targetBlock.metadata?.id, + title: targetBlock.metadata?.name, + description: targetBlock.metadata?.description, + subBlocks: { + ...targetBlock.config.params, + systemPrompt: systemPrompt, + }, + currentState: ctx.blockStates.get(targetBlock.id)?.output, }, - currentState: ctx.blockStates.get(targetBlock.id)?.output, - } + ] }) } } diff --git a/apps/sim/serializer/index.test.ts b/apps/sim/serializer/index.test.ts index 12ae27b2794..e44cf574f0d 100644 --- a/apps/sim/serializer/index.test.ts +++ b/apps/sim/serializer/index.test.ts @@ -82,6 +82,35 @@ describe('Serializer', () => { expect(falsePathConnection?.target).toBe('agent2') }) + it.concurrent('should keep disabled blocks and the connections that reach them', () => { + const { blocks, edges, loops } = createConditionalWorkflowState() + const serializer = new Serializer() + + blocks.agent1.enabled = false + + const serialized = serializer.serializeWorkflow(blocks, edges, loops) + + expect(serialized.blocks.find((b) => b.id === 'agent1')?.enabled).toBe(false) + expect( + serialized.connections.find((c) => c.source === 'condition1' && c.target === 'agent1') + ).toBeDefined() + }) + + it.concurrent('should drop connections that reference a block that does not exist', () => { + const { blocks, edges, loops } = createConditionalWorkflowState() + const serializer = new Serializer() + + const { agent1: _removed, ...remainingBlocks } = blocks + + const serialized = serializer.serializeWorkflow(remainingBlocks, edges, loops) + + expect(serialized.blocks.find((b) => b.id === 'agent1')).toBeUndefined() + expect(serialized.connections.some((c) => c.target === 'agent1')).toBe(false) + expect( + serialized.connections.find((c) => c.source === 'condition1' && c.target === 'agent2') + ).toBeDefined() + }) + it.concurrent('should serialize a workflow with loops correctly', () => { const { blocks, edges, loops } = createLoopWorkflowState() const serializer = new Serializer() diff --git a/apps/sim/serializer/index.ts b/apps/sim/serializer/index.ts index f018f509ced..996e46e1469 100644 --- a/apps/sim/serializer/index.ts +++ b/apps/sim/serializer/index.ts @@ -180,17 +180,30 @@ export class Serializer { serializedBlocks.push(this.serializeBlock(block, { validateRequired, allBlocks: blocks })) } + // Every connection must reference blocks that exist. A dangling edge (dropped custom + // block, a caller that pruned blocks but not edges) otherwise reaches the executor and + // breaks handlers that resolve their target out of `workflow.blocks`. + const serializedBlockIds = new Set(serializedBlocks.map((b) => b.id)) + const isConnectedToKnownBlocks = (edge: Edge): boolean => { + if (serializedBlockIds.has(edge.source) && serializedBlockIds.has(edge.target)) return true + if (!droppedBlockIds.has(edge.source) && !droppedBlockIds.has(edge.target)) { + logger.warn('Dropping connection that references a missing block', { + source: edge.source, + target: edge.target, + }) + } + return false + } + return { version: '1.0', blocks: serializedBlocks, - connections: edges - .filter((edge) => !droppedBlockIds.has(edge.source) && !droppedBlockIds.has(edge.target)) - .map((edge) => ({ - source: edge.source, - target: edge.target, - sourceHandle: edge.sourceHandle || undefined, - targetHandle: edge.targetHandle || undefined, - })), + connections: edges.filter(isConnectedToKnownBlocks).map((edge) => ({ + source: edge.source, + target: edge.target, + sourceHandle: edge.sourceHandle || undefined, + targetHandle: edge.targetHandle || undefined, + })), loops: safeLoops, parallels: safeParallels, }