Skip to content

Commit dc69ea5

Browse files
authored
fix(condition): fixed condition block else routing bug (#772)
1 parent 8b35cf5 commit dc69ea5

File tree

4 files changed

+299
-4
lines changed

4 files changed

+299
-4
lines changed

apps/sim/executor/index.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ describe('Executor', () => {
837837
{ source: 'condition1', target: 'falseTarget', sourceHandle: 'condition-false' },
838838
]
839839
const falseResult = checkDependencies(falseConnections, executedBlocks, mockContext)
840-
expect(falseResult).toBe(false) // condition executed + path NOT selected = dependency NOT met
840+
expect(falseResult).toBe(true) // unselected condition paths are treated as "not applicable" to support multi-path scenarios
841841
})
842842

843843
test('should handle regular sequential dependencies correctly', () => {

apps/sim/executor/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,12 +1123,13 @@ export class Executor {
11231123
const conditionId = conn.sourceHandle.replace('condition-', '')
11241124
const selectedCondition = context.decisions.condition.get(conn.source)
11251125

1126-
// If source is executed and this is not the selected path, dependency is NOT met
1126+
// If source is executed and this is not the selected path, treat as "not applicable"
1127+
// This allows blocks with multiple condition paths to execute via any selected path
11271128
if (sourceExecuted && selectedCondition && conditionId !== selectedCondition) {
1128-
return false
1129+
return true // Changed from false to true - unselected paths don't block execution
11291130
}
11301131

1131-
// Otherwise, this dependency is met only if source is executed and this is the selected path
1132+
// This dependency is met only if source is executed and this is the selected path
11321133
return sourceExecuted && conditionId === selectedCondition
11331134
}
11341135
}

apps/sim/executor/path/path.test.ts

Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,288 @@ describe('PathTracker', () => {
606606
})
607607
})
608608

609+
describe('Condition downstream path activation', () => {
610+
beforeEach(() => {
611+
// Create condition workflow with downstream connections similar to router test
612+
mockWorkflow = {
613+
version: '1.0',
614+
blocks: [
615+
{
616+
id: 'condition1',
617+
metadata: { id: BlockType.CONDITION, name: 'Condition' },
618+
position: { x: 0, y: 0 },
619+
config: { tool: BlockType.CONDITION, params: {} },
620+
inputs: {},
621+
outputs: {},
622+
enabled: true,
623+
},
624+
{
625+
id: 'knowledge1',
626+
metadata: { id: BlockType.FUNCTION, name: 'Knowledge 1' },
627+
position: { x: 0, y: 0 },
628+
config: { tool: BlockType.FUNCTION, params: {} },
629+
inputs: {},
630+
outputs: {},
631+
enabled: true,
632+
},
633+
{
634+
id: 'knowledge2',
635+
metadata: { id: BlockType.FUNCTION, name: 'Knowledge 2' },
636+
position: { x: 0, y: 0 },
637+
config: { tool: BlockType.FUNCTION, params: {} },
638+
inputs: {},
639+
outputs: {},
640+
enabled: true,
641+
},
642+
{
643+
id: 'agent1',
644+
metadata: { id: BlockType.AGENT, name: 'Agent' },
645+
position: { x: 0, y: 0 },
646+
config: { tool: BlockType.AGENT, params: {} },
647+
inputs: {},
648+
outputs: {},
649+
enabled: true,
650+
},
651+
],
652+
connections: [
653+
{ source: 'condition1', target: 'knowledge1', sourceHandle: 'condition-if-id' },
654+
{ source: 'condition1', target: 'knowledge2', sourceHandle: 'condition-else-if-id' },
655+
{ source: 'condition1', target: 'agent1', sourceHandle: 'condition-else-id' },
656+
{ source: 'knowledge1', target: 'agent1' },
657+
{ source: 'knowledge2', target: 'agent1' },
658+
],
659+
loops: {},
660+
parallels: {},
661+
}
662+
663+
pathTracker = new PathTracker(mockWorkflow)
664+
mockContext = {
665+
workflowId: 'test-condition-workflow',
666+
blockStates: new Map(),
667+
blockLogs: [],
668+
metadata: { duration: 0 },
669+
environmentVariables: {},
670+
decisions: { router: new Map(), condition: new Map() },
671+
loopIterations: new Map(),
672+
loopItems: new Map(),
673+
completedLoops: new Set(),
674+
executedBlocks: new Set(),
675+
activeExecutionPath: new Set(),
676+
workflow: mockWorkflow,
677+
}
678+
})
679+
680+
it('should recursively activate downstream paths when condition selects regular block target', () => {
681+
// Mock condition output selecting knowledge1 (if path)
682+
mockContext.blockStates.set('condition1', {
683+
output: {
684+
selectedConditionId: 'if-id',
685+
},
686+
executed: true,
687+
executionTime: 100,
688+
})
689+
690+
// Update paths for condition
691+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
692+
693+
// Both knowledge1 and agent1 should be activated (agent1 is downstream from knowledge1)
694+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(true)
695+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(true)
696+
697+
// knowledge2 should NOT be activated (not selected by condition)
698+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(false)
699+
700+
// Condition decision should be recorded
701+
expect(mockContext.decisions.condition.get('condition1')).toBe('if-id')
702+
})
703+
704+
it('should recursively activate downstream paths when condition selects else-if path', () => {
705+
// Mock condition output selecting knowledge2 (else-if path)
706+
mockContext.blockStates.set('condition1', {
707+
output: {
708+
selectedConditionId: 'else-if-id',
709+
},
710+
executed: true,
711+
executionTime: 100,
712+
})
713+
714+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
715+
716+
// Both knowledge2 and agent1 should be activated (agent1 is downstream from knowledge2)
717+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(true)
718+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(true)
719+
720+
// knowledge1 should NOT be activated (not selected by condition)
721+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(false)
722+
723+
// Condition decision should be recorded
724+
expect(mockContext.decisions.condition.get('condition1')).toBe('else-if-id')
725+
})
726+
727+
it('should activate direct path when condition selects else path', () => {
728+
// Mock condition output selecting agent1 directly (else path)
729+
mockContext.blockStates.set('condition1', {
730+
output: {
731+
selectedConditionId: 'else-id',
732+
},
733+
executed: true,
734+
executionTime: 100,
735+
})
736+
737+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
738+
739+
// Only agent1 should be activated (direct path)
740+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(true)
741+
742+
// Neither knowledge block should be activated
743+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(false)
744+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(false)
745+
746+
// Condition decision should be recorded
747+
expect(mockContext.decisions.condition.get('condition1')).toBe('else-id')
748+
})
749+
750+
it('should handle multiple levels of downstream connections', () => {
751+
// Add another level to test deep activation
752+
mockWorkflow.blocks.push({
753+
id: 'finalStep',
754+
metadata: { id: BlockType.FUNCTION, name: 'Final Step' },
755+
position: { x: 0, y: 0 },
756+
config: { tool: BlockType.FUNCTION, params: {} },
757+
inputs: {},
758+
outputs: {},
759+
enabled: true,
760+
})
761+
mockWorkflow.connections.push({ source: 'agent1', target: 'finalStep' })
762+
763+
pathTracker = new PathTracker(mockWorkflow)
764+
765+
// Mock condition output selecting knowledge1
766+
mockContext.blockStates.set('condition1', {
767+
output: {
768+
selectedConditionId: 'if-id',
769+
},
770+
executed: true,
771+
executionTime: 100,
772+
})
773+
774+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
775+
776+
// All downstream blocks should be activated
777+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(true)
778+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(true)
779+
expect(mockContext.activeExecutionPath.has('finalStep')).toBe(true)
780+
781+
// Non-selected path should not be activated
782+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(false)
783+
})
784+
785+
it('should not recursively activate when condition selects routing block', () => {
786+
// Add another condition block as a target
787+
mockWorkflow.blocks.push({
788+
id: 'condition2',
789+
metadata: { id: BlockType.CONDITION, name: 'Nested Condition' },
790+
position: { x: 0, y: 0 },
791+
config: { tool: BlockType.CONDITION, params: {} },
792+
inputs: {},
793+
outputs: {},
794+
enabled: true,
795+
})
796+
797+
// Add connection from condition1 to condition2
798+
mockWorkflow.connections.push({
799+
source: 'condition1',
800+
target: 'condition2',
801+
sourceHandle: 'condition-nested-id',
802+
})
803+
804+
pathTracker = new PathTracker(mockWorkflow)
805+
806+
// Mock condition output selecting condition2 (routing block)
807+
mockContext.blockStates.set('condition1', {
808+
output: {
809+
selectedConditionId: 'nested-id',
810+
},
811+
executed: true,
812+
executionTime: 100,
813+
})
814+
815+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
816+
817+
// Only condition2 should be activated (routing blocks don't activate downstream)
818+
expect(mockContext.activeExecutionPath.has('condition2')).toBe(true)
819+
820+
// Other blocks should not be activated
821+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(false)
822+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(false)
823+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(false)
824+
})
825+
826+
it('should not recursively activate when condition selects flow control block', () => {
827+
// Add a parallel block as a target
828+
mockWorkflow.blocks.push({
829+
id: 'parallel1',
830+
metadata: { id: BlockType.PARALLEL, name: 'Parallel Block' },
831+
position: { x: 0, y: 0 },
832+
config: { tool: BlockType.PARALLEL, params: {} },
833+
inputs: {},
834+
outputs: {},
835+
enabled: true,
836+
})
837+
838+
// Add connection from condition1 to parallel1
839+
mockWorkflow.connections.push({
840+
source: 'condition1',
841+
target: 'parallel1',
842+
sourceHandle: 'condition-parallel-id',
843+
})
844+
845+
pathTracker = new PathTracker(mockWorkflow)
846+
847+
// Mock condition output selecting parallel1 (flow control block)
848+
mockContext.blockStates.set('condition1', {
849+
output: {
850+
selectedConditionId: 'parallel-id',
851+
},
852+
executed: true,
853+
executionTime: 100,
854+
})
855+
856+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
857+
858+
// Only parallel1 should be activated (flow control blocks don't activate downstream)
859+
expect(mockContext.activeExecutionPath.has('parallel1')).toBe(true)
860+
861+
// Other blocks should not be activated
862+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(false)
863+
expect(mockContext.activeExecutionPath.has('knowledge2')).toBe(false)
864+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(false)
865+
})
866+
867+
it('should not create infinite loops in cyclic workflows', () => {
868+
// Add a cycle to test loop prevention
869+
mockWorkflow.connections.push({ source: 'agent1', target: 'knowledge1' })
870+
pathTracker = new PathTracker(mockWorkflow)
871+
872+
mockContext.blockStates.set('condition1', {
873+
output: {
874+
selectedConditionId: 'if-id',
875+
},
876+
executed: true,
877+
executionTime: 100,
878+
})
879+
880+
// This should not throw or cause infinite recursion
881+
expect(() => {
882+
pathTracker.updateExecutionPaths(['condition1'], mockContext)
883+
}).not.toThrow()
884+
885+
// Both knowledge1 and agent1 should still be activated
886+
expect(mockContext.activeExecutionPath.has('knowledge1')).toBe(true)
887+
expect(mockContext.activeExecutionPath.has('agent1')).toBe(true)
888+
})
889+
})
890+
609891
describe('RoutingStrategy integration', () => {
610892
beforeEach(() => {
611893
// Add more block types to test the new routing strategy

apps/sim/executor/path/path.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,18 @@ export class PathTracker {
235235
for (const conn of targetConnections) {
236236
context.activeExecutionPath.add(conn.target)
237237
logger.debug(`Condition ${block.id} activated path to: ${conn.target}`)
238+
239+
// Check if the selected target should activate downstream paths
240+
const selectedBlock = this.getBlock(conn.target)
241+
const selectedBlockType = selectedBlock?.metadata?.id || ''
242+
const selectedCategory = Routing.getCategory(selectedBlockType)
243+
244+
// Only activate downstream paths for regular blocks
245+
// Routing blocks make their own routing decisions when they execute
246+
// Flow control blocks manage their own path activation
247+
if (selectedCategory === 'regular') {
248+
this.activateDownstreamPathsSelectively(conn.target, context)
249+
}
238250
}
239251
}
240252

0 commit comments

Comments
 (0)