From ac3c2fc6ce2f619723354286ae8365c0b7d50df9 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Thu, 30 Apr 2026 19:20:05 +0200 Subject: [PATCH] fix: stop nested splits at parent merge joins Symptom: describe could treat a nested split's downstream common tail as the nested merge while the parent branch was supposed to stop at an earlier merge. The emitted MDL moved shared continuation into an else branch, which could leave return-valued microflows with a path lacking a return. Root cause: nested traversal trusted the precomputed split merge even when that join was reachable only after the parent merge. The empty-then swap also ran against that over-wide join. Fix: resolve nested split merges against the parent merge when the computed join is downstream, and only apply the empty-then swap when the resolved nested merge is the active parent merge. Tests: added synthetic regression coverage for downstream-vs-local nested merge resolution and ran make build && make test. --- mdl/executor/cmd_microflows_show_helpers.go | 60 +++++++++++++++++++- mdl/executor/cmd_microflows_traverse_test.go | 40 +++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index d0505d84..2e0a15d1 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -783,10 +783,11 @@ func traverseFlowUntilMerge( nestedMergeID := splitMergeMap[currentID] trueFlow, falseFlow := findBranchFlows(flows) + nestedMergeID = resolveNestedMergeID(nestedMergeID, mergeID, trueFlow, falseFlow, flowsByOrigin) // Empty-then swap: negate when true branch is empty but false branch has content. // Skip when both branches go directly to merge (both empty). - if trueFlow != nil && falseFlow != nil && nestedMergeID != "" { + if trueFlow != nil && falseFlow != nil && nestedMergeID != "" && nestedMergeID == mergeID { if trueFlow.DestinationID == nestedMergeID && falseFlow.DestinationID != nestedMergeID { stmt = negateIfCondition(stmt) trueFlow, falseFlow = falseFlow, trueFlow @@ -938,6 +939,63 @@ func continueAfterNestedSplitJoin( traverseFlowUntilMerge(ctx, joinID, parentMergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget) } +func resolveNestedMergeID( + nestedMergeID model.ID, + parentMergeID model.ID, + trueFlow *microflows.SequenceFlow, + falseFlow *microflows.SequenceFlow, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, +) model.ID { + if nestedMergeID != "" && parentMergeID != "" && nestedMergeID != parentMergeID && + canReachNode(parentMergeID, nestedMergeID, flowsByOrigin, make(map[model.ID]bool)) { + for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} { + if flow == nil { + continue + } + if flow.DestinationID == parentMergeID || + canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) { + return parentMergeID + } + } + } + if nestedMergeID != "" || parentMergeID == "" { + return nestedMergeID + } + for _, flow := range []*microflows.SequenceFlow{trueFlow, falseFlow} { + if flow == nil { + continue + } + if canReachNode(flow.DestinationID, parentMergeID, flowsByOrigin, make(map[model.ID]bool)) { + return parentMergeID + } + } + return "" +} + +func canReachNode( + currentID model.ID, + targetID model.ID, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + visited map[model.ID]bool, +) bool { + if currentID == "" { + return false + } + if currentID == targetID { + return true + } + if visited[currentID] { + return false + } + visited[currentID] = true + for _, flow := range findNormalFlows(flowsByOrigin[currentID]) { + if canReachNode(flow.DestinationID, targetID, flowsByOrigin, visited) { + return true + } + } + return false +} + // traverseLoopBody traverses activities inside a loop body. // When sourceMap is non-nil, it also records line ranges for each activity node. func traverseLoopBody( diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index defb8a9d..9a7d72fd 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -654,6 +654,46 @@ func TestTraverseFlow_NestedTerminalGuardBranchSuppressesEmptyOuterElse(t *testi } } +func TestResolveNestedMergeID_UsesParentMergeBeforeDownstreamJoin(t *testing.T) { + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("parent_merge"): {mkFlow("parent_merge", "downstream_join")}, + } + trueFlow := mkFlow("nested_split", "parent_merge") + falseFlow := mkFlow("nested_split", "false_branch") + + got := resolveNestedMergeID( + mkID("downstream_join"), + mkID("parent_merge"), + trueFlow, + falseFlow, + flowsByOrigin, + ) + + if got != mkID("parent_merge") { + t.Fatalf("nested split used downstream join %q, want parent merge %q", got, mkID("parent_merge")) + } +} + +func TestResolveNestedMergeID_KeepsIndependentNestedJoin(t *testing.T) { + flowsByOrigin := map[model.ID][]*microflows.SequenceFlow{ + mkID("nested_join"): {mkFlow("nested_join", "parent_merge")}, + } + trueFlow := mkFlow("nested_split", "true_branch") + falseFlow := mkFlow("nested_split", "nested_join") + + got := resolveNestedMergeID( + mkID("nested_join"), + mkID("parent_merge"), + trueFlow, + falseFlow, + flowsByOrigin, + ) + + if got != mkID("nested_join") { + t.Fatalf("nested split merge changed to %q, want local nested join %q", got, mkID("nested_join")) + } +} + // ============================================================================= // collectErrorHandlerStatements // =============================================================================