Skip to content

Commit aa94970

Browse files
committed
fix: keep terminal false branches structured
Symptom: describe could collapse a real terminal false branch into a guard-style continuation when the branch happened to sit on the split centerline, moving statements outside their original ELSE block. Root cause: guard continuation detection looked only at layout Y position. That made a branch-shaped false flow indistinguishable from the horizontal split-to-tail flow produced for guard-style IF statements. Fix: require the false flow to also use the builder's horizontal right-to-left anchor pair before treating it as a guard continuation. Terminal false branches without that guard-tail flow shape remain inside an explicit ELSE. Tests: make build, make test, make lint-go.
1 parent 2b20796 commit aa94970

2 files changed

Lines changed: 52 additions & 5 deletions

File tree

mdl/executor/cmd_microflows_show_helpers.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,10 +1442,13 @@ func flowLooksLikeGuardContinuation(
14421442
return false
14431443
}
14441444
// Builder-generated guard continuations sit on the split's horizontal
1445-
// centerline. This intentionally relies on mxcli's layout contract so a
1446-
// real branch that returns to a merge below the split is not collapsed into
1447-
// a guard-style continuation during describe.
1448-
return dest.GetPosition().Y == split.GetPosition().Y
1445+
// centerline and use the builder's horizontal split→tail flow. This
1446+
// intentionally relies on mxcli's layout/anchor contract so a real false
1447+
// branch whose activities happen to be aligned with the split is not
1448+
// collapsed into a guard-style continuation during describe.
1449+
return dest.GetPosition().Y == split.GetPosition().Y &&
1450+
flow.OriginConnectionIndex == AnchorRight &&
1451+
flow.DestinationConnectionIndex == AnchorLeft
14491452
}
14501453

14511454
// findErrorHandlerFlow returns the error handler flow from an activity's outgoing flows.

mdl/executor/cmd_microflows_traverse_test.go

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,9 @@ func TestTraverseFlow_SequentialIfWithoutElseKeepsContinuationOutsideFirstIf(t *
563563

564564
func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideElse(t *testing.T) {
565565
e := newTestExecutor()
566+
falseFlow := mkBranchFlow("split", "tail_log", &microflows.ExpressionCase{Expression: "false"})
567+
falseFlow.OriginConnectionIndex = AnchorRight
568+
falseFlow.DestinationConnectionIndex = AnchorLeft
566569

567570
activityMap := map[model.ID]microflows.MicroflowObject{
568571
mkID("start"): &microflows.StartEvent{BaseMicroflowObject: mkObj("start")},
@@ -585,7 +588,7 @@ func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideE
585588
mkID("start"): {mkFlow("start", "split")},
586589
mkID("split"): {
587590
mkBranchFlow("split", "then_log", &microflows.ExpressionCase{Expression: "true"}),
588-
mkBranchFlow("split", "tail_log", &microflows.ExpressionCase{Expression: "false"}),
591+
falseFlow,
589592
},
590593
mkID("then_log"): {mkFlow("then_log", "then_return")},
591594
mkID("tail_log"): {mkFlow("tail_log", "end")},
@@ -607,6 +610,47 @@ func TestTraverseFlow_GuardBranchWithMultipleActivitiesKeepsContinuationOutsideE
607610
}
608611
}
609612

613+
func TestTraverseFlow_TerminalFalseBranchIsNotGuardContinuation(t *testing.T) {
614+
e := newTestExecutor()
615+
616+
activityMap := map[model.ID]microflows.MicroflowObject{
617+
mkID("start"): &microflows.StartEvent{BaseMicroflowObject: mkObj("start")},
618+
mkID("split"): &microflows.ExclusiveSplit{
619+
BaseMicroflowObject: mkObj("split"),
620+
SplitCondition: &microflows.ExpressionSplitCondition{Expression: "$IsAllowed"},
621+
},
622+
mkID("then_log"): &microflows.ActionActivity{
623+
BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("then_log")},
624+
Action: &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "allowed"}}},
625+
},
626+
mkID("then_return"): &microflows.EndEvent{BaseMicroflowObject: mkObj("then_return")},
627+
mkID("false_log"): &microflows.ActionActivity{BaseActivity: microflows.BaseActivity{BaseMicroflowObject: mkObj("false_log")}, Action: &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'App'", MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "blocked"}}}},
628+
mkID("false_return"): &microflows.EndEvent{BaseMicroflowObject: mkObj("false_return")},
629+
}
630+
631+
flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{
632+
mkID("start"): {mkFlow("start", "split")},
633+
mkID("split"): {
634+
mkBranchFlow("split", "then_log", &microflows.ExpressionCase{Expression: "true"}),
635+
mkBranchFlow("split", "false_log", &microflows.ExpressionCase{Expression: "false"}),
636+
},
637+
mkID("then_log"): {mkFlow("then_log", "then_return")},
638+
mkID("false_log"): {mkFlow("false_log", "false_return")},
639+
}
640+
641+
var lines []string
642+
visited := make(map[model.ID]bool)
643+
e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, visited, nil, nil, &lines, 0, nil, 0, nil)
644+
645+
out := strings.Join(lines, "\n")
646+
if !strings.Contains(out, "\nelse\n") {
647+
t.Fatalf("terminal false branch must stay inside an explicit ELSE:\n%s", out)
648+
}
649+
if strings.Index(out, "blocked") < strings.Index(out, "else") {
650+
t.Fatalf("false branch body was emitted outside ELSE:\n%s", out)
651+
}
652+
}
653+
610654
func TestTraverseFlow_NestedTerminalGuardBranchSuppressesEmptyOuterElse(t *testing.T) {
611655
e := newTestExecutor()
612656

0 commit comments

Comments
 (0)