From e1b58c1cf013ec1e948c55391ad59a174ca57f5c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 29 Jul 2026 18:05:33 -0700 Subject: [PATCH] fix(executor): stop condition/router erroring when downstream blocks are disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manual editor runs built the executor payload from blocks-minus-disabled but sent every edge, so the server received edges pointing at blocks absent from workflow.blocks. Condition and router are the only handlers that resolve their target out of workflow.blocks, so only they threw "Target block not found"; every other block type ignores a dangling edge. Deployed, scheduled, webhook and run-from-block executions always sent full state, so the editor was the only outlier — the DAG builder already excludes disabled blocks and their edges. - keep disabled blocks in the payload; filter only for trigger resolution - condition: a disabled or missing target dead-ends the branch instead of throwing - router: exclude disabled/missing targets from the candidate list, and dead-end without calling the model when none remain (previously these runs succeeded after picking a disabled block, so throwing would have failed live workflows) - serializer: drop connections that reference a block that does not exist --- .../hooks/use-workflow-execution.test.tsx | 62 ++++++++++++++++++- .../hooks/use-workflow-execution.ts | 32 ++++++---- .../condition/condition-handler.test.ts | 26 ++++++-- .../handlers/condition/condition-handler.ts | 26 ++++++-- .../handlers/router/router-handler.test.ts | 50 +++++++++++++-- .../handlers/router/router-handler.ts | 62 +++++++++++++++---- apps/sim/serializer/index.test.ts | 29 +++++++++ apps/sim/serializer/index.ts | 29 ++++++--- 8 files changed, 270 insertions(+), 46 deletions(-) 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, }