Skip to content

Commit 247ffc5

Browse files
committed
fix(condition): consecutive error logging + execution dequeuing
1 parent 28f8e0f commit 247ffc5

File tree

4 files changed

+298
-19
lines changed

4 files changed

+298
-19
lines changed

apps/sim/executor/execution/edge-manager.test.ts

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2626,6 +2626,171 @@ describe('EdgeManager', () => {
26262626
expect(readyNodes).not.toContain(afterLoopId)
26272627
})
26282628

2629+
it('should not queue sentinel-end when condition selects no-edge path (loop)', () => {
2630+
// Bug scenario: condition → (if) → sentinel_start → body → sentinel_end → (loop_exit) → after_loop
2631+
// → (else) → [NO outgoing edge]
2632+
// Condition evaluates false, else is selected but has no edge.
2633+
// With selectedOption set (routing decision made), cascadeTargets should NOT be queued.
2634+
// Previously sentinel_end was queued via cascadeTargets, causing downstream blocks to execute.
2635+
2636+
const conditionId = 'condition'
2637+
const sentinelStartId = 'sentinel-start'
2638+
const loopBodyId = 'loop-body'
2639+
const sentinelEndId = 'sentinel-end'
2640+
const afterLoopId = 'after-loop'
2641+
2642+
const conditionNode = createMockNode(conditionId, [
2643+
{ target: sentinelStartId, sourceHandle: 'condition-if-id' },
2644+
])
2645+
2646+
const sentinelStartNode = createMockNode(
2647+
sentinelStartId,
2648+
[{ target: loopBodyId }],
2649+
[conditionId]
2650+
)
2651+
2652+
const loopBodyNode = createMockNode(
2653+
loopBodyId,
2654+
[{ target: sentinelEndId }],
2655+
[sentinelStartId]
2656+
)
2657+
2658+
const sentinelEndNode = createMockNode(
2659+
sentinelEndId,
2660+
[
2661+
{ target: sentinelStartId, sourceHandle: 'loop_continue' },
2662+
{ target: afterLoopId, sourceHandle: 'loop_exit' },
2663+
],
2664+
[loopBodyId]
2665+
)
2666+
2667+
const afterLoopNode = createMockNode(afterLoopId, [], [sentinelEndId])
2668+
2669+
const nodes = new Map<string, DAGNode>([
2670+
[conditionId, conditionNode],
2671+
[sentinelStartId, sentinelStartNode],
2672+
[loopBodyId, loopBodyNode],
2673+
[sentinelEndId, sentinelEndNode],
2674+
[afterLoopId, afterLoopNode],
2675+
])
2676+
2677+
const dag = createMockDAG(nodes)
2678+
const edgeManager = new EdgeManager(dag)
2679+
2680+
// Condition selected else, but else has no outgoing edge.
2681+
// selectedOption is set (routing decision was made).
2682+
const readyNodes = edgeManager.processOutgoingEdges(conditionNode, {
2683+
selectedOption: 'else-id',
2684+
})
2685+
2686+
// Nothing should be queued -- the entire branch is intentionally dead
2687+
expect(readyNodes).not.toContain(sentinelStartId)
2688+
expect(readyNodes).not.toContain(loopBodyId)
2689+
expect(readyNodes).not.toContain(sentinelEndId)
2690+
expect(readyNodes).not.toContain(afterLoopId)
2691+
expect(readyNodes).toHaveLength(0)
2692+
})
2693+
2694+
it('should not queue sentinel-end when condition selects no-edge path (parallel)', () => {
2695+
// Same scenario with parallel instead of loop
2696+
const conditionId = 'condition'
2697+
const parallelStartId = 'parallel-start'
2698+
const branchId = 'branch-0'
2699+
const parallelEndId = 'parallel-end'
2700+
const afterParallelId = 'after-parallel'
2701+
2702+
const conditionNode = createMockNode(conditionId, [
2703+
{ target: parallelStartId, sourceHandle: 'condition-if-id' },
2704+
])
2705+
2706+
const parallelStartNode = createMockNode(
2707+
parallelStartId,
2708+
[{ target: branchId }],
2709+
[conditionId]
2710+
)
2711+
2712+
const branchNode = createMockNode(
2713+
branchId,
2714+
[{ target: parallelEndId, sourceHandle: 'parallel_exit' }],
2715+
[parallelStartId]
2716+
)
2717+
2718+
const parallelEndNode = createMockNode(
2719+
parallelEndId,
2720+
[{ target: afterParallelId, sourceHandle: 'parallel_exit' }],
2721+
[branchId]
2722+
)
2723+
2724+
const afterParallelNode = createMockNode(afterParallelId, [], [parallelEndId])
2725+
2726+
const nodes = new Map<string, DAGNode>([
2727+
[conditionId, conditionNode],
2728+
[parallelStartId, parallelStartNode],
2729+
[branchId, branchNode],
2730+
[parallelEndId, parallelEndNode],
2731+
[afterParallelId, afterParallelNode],
2732+
])
2733+
2734+
const dag = createMockDAG(nodes)
2735+
const edgeManager = new EdgeManager(dag)
2736+
2737+
const readyNodes = edgeManager.processOutgoingEdges(conditionNode, {
2738+
selectedOption: 'else-id',
2739+
})
2740+
2741+
expect(readyNodes).not.toContain(parallelStartId)
2742+
expect(readyNodes).not.toContain(branchId)
2743+
expect(readyNodes).not.toContain(parallelEndId)
2744+
expect(readyNodes).not.toContain(afterParallelId)
2745+
expect(readyNodes).toHaveLength(0)
2746+
})
2747+
2748+
it('should still queue sentinel-end inside loop when no condition matches (true dead-end)', () => {
2749+
// Contrast: condition INSIDE a loop with selectedOption null (no match, no routing decision).
2750+
// This is a true dead-end where cascadeTargets SHOULD fire so the loop sentinel can handle exit.
2751+
2752+
const sentinelStartId = 'sentinel-start'
2753+
const sentinelEndId = 'sentinel-end'
2754+
const conditionId = 'condition'
2755+
const nodeAId = 'node-a'
2756+
2757+
const sentinelStartNode = createMockNode(sentinelStartId, [{ target: conditionId }])
2758+
const conditionNode = createMockNode(
2759+
conditionId,
2760+
[{ target: nodeAId, sourceHandle: 'condition-if' }],
2761+
[sentinelStartId]
2762+
)
2763+
const nodeANode = createMockNode(nodeAId, [{ target: sentinelEndId }], [conditionId])
2764+
const sentinelEndNode = createMockNode(
2765+
sentinelEndId,
2766+
[
2767+
{ target: sentinelStartId, sourceHandle: 'loop_continue' },
2768+
{ target: 'after-loop', sourceHandle: 'loop_exit' },
2769+
],
2770+
[nodeAId]
2771+
)
2772+
2773+
const nodes = new Map<string, DAGNode>([
2774+
[sentinelStartId, sentinelStartNode],
2775+
[conditionId, conditionNode],
2776+
[nodeAId, nodeANode],
2777+
[sentinelEndId, sentinelEndNode],
2778+
])
2779+
2780+
const dag = createMockDAG(nodes)
2781+
const edgeManager = new EdgeManager(dag)
2782+
2783+
conditionNode.incomingEdges.clear()
2784+
2785+
// selectedOption: null → no routing decision, true dead-end
2786+
const readyNodes = edgeManager.processOutgoingEdges(conditionNode, {
2787+
selectedOption: null,
2788+
})
2789+
2790+
// sentinel-end SHOULD be queued (true dead-end inside loop)
2791+
expect(readyNodes).toContain(sentinelEndId)
2792+
})
2793+
26292794
it('should still correctly handle normal loop exit (not deactivate when loop runs)', () => {
26302795
// When a loop actually executes and exits normally, after_loop should become ready
26312796
const sentinelStartId = 'sentinel-start'

apps/sim/executor/execution/edge-manager.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,17 @@ export class EdgeManager {
6969
}
7070
}
7171

72+
const isUnroutedDeadEnd =
73+
activatedTargets.length === 0 && !output.selectedOption && !output.selectedRoute
74+
7275
for (const targetId of cascadeTargets) {
7376
if (!readyNodes.includes(targetId) && !activatedTargets.includes(targetId)) {
74-
// Only queue cascade terminal control nodes when ALL outgoing edges from the
75-
// current node were deactivated (dead-end scenario). When some edges are
76-
// activated, terminal control nodes on deactivated branches should NOT be
77-
// queued - they will be reached through the normal activated path's completion.
78-
// This prevents loop/parallel sentinels on fully deactivated paths (e.g., an
79-
// upstream condition took a different branch) from being spuriously executed.
80-
if (activatedTargets.length === 0 && this.isTargetReady(targetId)) {
77+
// Only queue cascade terminal control nodes in a true dead-end scenario:
78+
// all outgoing edges deactivated AND no routing decision was made. When a
79+
// condition/router deliberately selected a path that has no outgoing edge,
80+
// selectedOption/selectedRoute will be set, signaling that downstream
81+
// subflow sentinels should NOT fire -- the entire branch is intentionally dead.
82+
if (isUnroutedDeadEnd && this.isTargetReady(targetId)) {
8183
readyNodes.push(targetId)
8284
}
8385
}

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

Lines changed: 107 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ describe('ConditionBlockHandler', () => {
555555
})
556556

557557
describe('Condition with no outgoing edge', () => {
558-
it('should return null path when condition matches but has no edge', async () => {
558+
it('should set selectedOption when condition matches but has no edge', async () => {
559559
const conditions = [
560560
{ id: 'cond1', title: 'if', value: 'true' },
561561
{ id: 'else1', title: 'else', value: '' },
@@ -570,9 +570,52 @@ describe('ConditionBlockHandler', () => {
570570

571571
const result = await handler.execute(mockContext, mockBlock, inputs)
572572

573-
// Condition matches but no edge for it
574-
expect((result as any).conditionResult).toBe(false)
573+
expect((result as any).conditionResult).toBe(true)
574+
expect((result as any).selectedPath).toBeNull()
575+
expect((result as any).selectedOption).toBe('cond1')
576+
expect(mockContext.decisions.condition.get(mockBlock.id)).toBe('cond1')
577+
})
578+
579+
it('should set selectedOption when else is selected but has no edge', async () => {
580+
const conditions = [
581+
{ id: 'cond1', title: 'if', value: 'false' },
582+
{ id: 'else1', title: 'else', value: '' },
583+
]
584+
const inputs = { conditions: JSON.stringify(conditions) }
585+
586+
// Only the if branch has an edge; else has no outgoing connection
587+
mockContext.workflow!.connections = [
588+
{ source: mockSourceBlock.id, target: mockBlock.id },
589+
{ source: mockBlock.id, target: mockTargetBlock1.id, sourceHandle: 'condition-cond1' },
590+
]
591+
592+
const result = await handler.execute(mockContext, mockBlock, inputs)
593+
594+
expect((result as any).conditionResult).toBe(true)
575595
expect((result as any).selectedPath).toBeNull()
596+
expect((result as any).selectedOption).toBe('else1')
597+
expect(mockContext.decisions.condition.get(mockBlock.id)).toBe('else1')
598+
})
599+
600+
it('should deactivate if-path when else is selected with no edge', async () => {
601+
const conditions = [
602+
{ id: 'cond1', title: 'if', value: 'context.value > 100' },
603+
{ id: 'else1', title: 'else', value: '' },
604+
]
605+
const inputs = { conditions: JSON.stringify(conditions) }
606+
607+
// Only the if branch has an edge to a loop; else has nothing
608+
mockContext.workflow!.connections = [
609+
{ source: mockSourceBlock.id, target: mockBlock.id },
610+
{ source: mockBlock.id, target: mockTargetBlock1.id, sourceHandle: 'condition-cond1' },
611+
]
612+
613+
const result = await handler.execute(mockContext, mockBlock, inputs)
614+
615+
// Else was selected (value 10 is not > 100), so selectedOption should be 'else1'
616+
// This allows the edge manager to deactivate the cond1 edge
617+
expect((result as any).selectedOption).toBe('else1')
618+
expect((result as any).conditionResult).toBe(true)
576619
})
577620
})
578621

@@ -602,6 +645,67 @@ describe('ConditionBlockHandler', () => {
602645
})
603646
})
604647

648+
describe('Source output filtering', () => {
649+
it('should not propagate error field from source block output', async () => {
650+
;(mockContext.blockStates as any).set(mockSourceBlock.id, {
651+
output: { value: 10, text: 'hello', error: 'upstream block failed' },
652+
executed: true,
653+
executionTime: 100,
654+
})
655+
656+
const conditions = [
657+
{ id: 'cond1', title: 'if', value: 'context.value > 5' },
658+
{ id: 'else1', title: 'else', value: '' },
659+
]
660+
const inputs = { conditions: JSON.stringify(conditions) }
661+
662+
const result = await handler.execute(mockContext, mockBlock, inputs)
663+
664+
expect((result as any).conditionResult).toBe(true)
665+
expect((result as any).selectedOption).toBe('cond1')
666+
expect(result).not.toHaveProperty('error')
667+
})
668+
669+
it('should not propagate _pauseMetadata from source block output', async () => {
670+
;(mockContext.blockStates as any).set(mockSourceBlock.id, {
671+
output: { value: 10, _pauseMetadata: { contextId: 'abc' } },
672+
executed: true,
673+
executionTime: 100,
674+
})
675+
676+
const conditions = [
677+
{ id: 'cond1', title: 'if', value: 'context.value > 5' },
678+
{ id: 'else1', title: 'else', value: '' },
679+
]
680+
const inputs = { conditions: JSON.stringify(conditions) }
681+
682+
const result = await handler.execute(mockContext, mockBlock, inputs)
683+
684+
expect((result as any).conditionResult).toBe(true)
685+
expect(result).not.toHaveProperty('_pauseMetadata')
686+
})
687+
688+
it('should still pass through non-control fields from source output', async () => {
689+
;(mockContext.blockStates as any).set(mockSourceBlock.id, {
690+
output: { value: 10, text: 'hello', customData: { nested: true } },
691+
executed: true,
692+
executionTime: 100,
693+
})
694+
695+
const conditions = [
696+
{ id: 'cond1', title: 'if', value: 'context.value > 5' },
697+
{ id: 'else1', title: 'else', value: '' },
698+
]
699+
const inputs = { conditions: JSON.stringify(conditions) }
700+
701+
const result = await handler.execute(mockContext, mockBlock, inputs)
702+
703+
expect((result as any).value).toBe(10)
704+
expect((result as any).text).toBe('hello')
705+
expect((result as any).customData).toEqual({ nested: true })
706+
})
707+
})
708+
605709
describe('Virtual block ID handling', () => {
606710
it('should use currentVirtualBlockId for decision key when available', async () => {
607711
mockContext.currentVirtualBlockId = 'virtual-block-123'

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,7 @@ export class ConditionBlockHandler implements BlockHandler {
108108
const evalContext = this.buildEvaluationContext(ctx, sourceBlockId)
109109
const rawSourceOutput = sourceBlockId ? ctx.blockStates.get(sourceBlockId)?.output : null
110110

111-
// Filter out _pauseMetadata from source output to prevent the engine from
112-
// thinking this block is pausing (it was already resumed by the HITL block)
113-
const sourceOutput = this.filterPauseMetadata(rawSourceOutput)
111+
const sourceOutput = this.filterSourceOutput(rawSourceOutput)
114112

115113
const outgoingConnections = ctx.workflow?.connections.filter(
116114
(conn) => conn.source === baseBlockId
@@ -124,7 +122,7 @@ export class ConditionBlockHandler implements BlockHandler {
124122
block.id
125123
)
126124

127-
if (!selectedConnection || !selectedCondition) {
125+
if (!selectedCondition) {
128126
return {
129127
...((sourceOutput as any) || {}),
130128
conditionResult: false,
@@ -133,6 +131,17 @@ export class ConditionBlockHandler implements BlockHandler {
133131
}
134132
}
135133

134+
if (!selectedConnection) {
135+
const decisionKey = ctx.currentVirtualBlockId || block.id
136+
ctx.decisions.condition.set(decisionKey, selectedCondition.id)
137+
return {
138+
...((sourceOutput as any) || {}),
139+
conditionResult: true,
140+
selectedPath: null,
141+
selectedOption: selectedCondition.id,
142+
}
143+
}
144+
136145
const targetBlock = ctx.workflow?.blocks.find((b) => b.id === selectedConnection?.target)
137146
if (!targetBlock) {
138147
throw new Error(`Target block ${selectedConnection?.target} not found`)
@@ -153,11 +162,11 @@ export class ConditionBlockHandler implements BlockHandler {
153162
}
154163
}
155164

156-
private filterPauseMetadata(output: any): any {
165+
private filterSourceOutput(output: any): any {
157166
if (!output || typeof output !== 'object') {
158167
return output
159168
}
160-
const { _pauseMetadata, ...rest } = output
169+
const { _pauseMetadata, error, ...rest } = output
161170
return rest
162171
}
163172

@@ -223,8 +232,7 @@ export class ConditionBlockHandler implements BlockHandler {
223232
if (connection) {
224233
return { selectedConnection: connection, selectedCondition: condition }
225234
}
226-
// Condition is true but has no outgoing edge - branch ends gracefully
227-
return { selectedConnection: null, selectedCondition: null }
235+
return { selectedConnection: null, selectedCondition: condition }
228236
}
229237
} catch (error: any) {
230238
logger.error(`Failed to evaluate condition "${condition.title}": ${error.message}`)
@@ -238,7 +246,7 @@ export class ConditionBlockHandler implements BlockHandler {
238246
if (elseConnection) {
239247
return { selectedConnection: elseConnection, selectedCondition: elseCondition }
240248
}
241-
return { selectedConnection: null, selectedCondition: null }
249+
return { selectedConnection: null, selectedCondition: elseCondition }
242250
}
243251

244252
return { selectedConnection: null, selectedCondition: null }

0 commit comments

Comments
 (0)