fix: route branch-local output handlers to safe continuations

hjothamendix · hjothamendix · commit 72f1f3a9b2a5 · 2026-04-29T10:31:37.000+02:00
Symptom: a custom error handler inside a returning IF branch could be forced to
terminate when the branch success tail declared values from the failed action
output. The resulting MDL inserted a return into the handler, and the generated
MPR no longer matched the original control flow.

Root cause: branch merge detection only treated empty custom handlers as needing
a merge, and output skip-var routing stopped at the first output-dependent
statement in void microflows. Derived variables declared from the failed output
and SHOW MESSAGE template arguments were also invisible to the skip-var scan.

Fix: treat non-terminal custom handlers as branch merge inputs, route pending
handlers from returning branches to that merge, carry skip-var state across
DECLARE-derived variables, and include SHOW MESSAGE expressions in handler
reference analysis.

Tests: added a synthetic branch-level regression covering a custom handler that
skips an output-derived success tail and rejoins at a shared error continuation.
Ran make build, make test, and make lint-go.
diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go
@@ -34,6 +34,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	hasElseBody := len(s.ElseBody) > 0
 	elseReturns := hasElseBody && lastStmtIsReturn(s.ElseBody)
 	bothReturn := hasElseBody && thenReturns && elseReturns
+	thenNeedsErrorMerge := thenReturns && bodyHasContinuingCustomErrorHandler(s.ThenBody)
+	elseNeedsErrorMerge := elseReturns && bodyHasContinuingCustomErrorHandler(s.ElseBody)
 
 	// Save/restore endsWithReturn around branch processing to avoid
 	// a branch's RETURN affecting the parent flow state prematurely
@@ -86,7 +88,7 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	needMerge := false
 	if !bothReturn {
 		if hasElseBody {
-			needMerge = !thenReturns && !elseReturns // both branches continue → 2 inputs
+			needMerge = (!thenReturns && !elseReturns) || thenNeedsErrorMerge || elseNeedsErrorMerge
 		} else {
 			needMerge = !thenReturns // THEN continues + FALSE path → 2 inputs
 		}
@@ -160,6 +162,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 				applyUserAnchors(flow, trueBranchAnchor, nil)
 				fb.flows = append(fb.flows, flow)
 			}
+		} else if thenReturns && needMerge {
+			fb.addPendingErrorHandlerFlowTo(mergeID)
 		}
 
 		// Process ELSE body (below the THEN path)
@@ -208,6 +212,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 				applyUserAnchors(flow, prevElseAnchor, nil)
 				fb.flows = append(fb.flows, flow)
 			}
+		} else if elseReturns && needMerge {
+			fb.addPendingErrorHandlerFlowTo(mergeID)
 		}
 	} else {
 		// IF WITHOUT ELSE: FALSE path horizontal (happy path), TRUE path below
@@ -322,6 +328,71 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	return splitID
 }
 
+func bodyHasContinuingCustomErrorHandler(stmts []ast.MicroflowStatement) bool {
+	for _, stmt := range stmts {
+		switch s := stmt.(type) {
+		case *ast.CallMicroflowStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.CallJavaActionStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.RestCallStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.ImportFromMappingStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.CreateObjectStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.MfCommitStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.DeleteObjectStmt:
+			if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
+				return true
+			}
+		case *ast.IfStmt:
+			if bodyHasContinuingCustomErrorHandler(s.ThenBody) || bodyHasContinuingCustomErrorHandler(s.ElseBody) {
+				return true
+			}
+		case *ast.LoopStmt:
+			if bodyHasContinuingCustomErrorHandler(s.Body) {
+				return true
+			}
+		case *ast.WhileStmt:
+			if bodyHasContinuingCustomErrorHandler(s.Body) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func isContinuingCustomErrorHandler(eh *ast.ErrorHandlingClause) bool {
+	if eh == nil {
+		return false
+	}
+	if eh.Type != ast.ErrorHandlingCustom && eh.Type != ast.ErrorHandlingCustomWithoutRollback {
+		return false
+	}
+	return len(eh.Body) == 0 || !lastStmtIsReturn(eh.Body)
+}
+
+func errorBody(eh *ast.ErrorHandlingClause) []ast.MicroflowStatement {
+	if eh == nil {
+		return nil
+	}
+	return eh.Body
+}
+
 // addLoopStatement creates a LOOP statement using LoopedActivity.
 // Layout: Auto-sizes the loop box to fit content with padding
 func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
diff --git a/mdl/executor/cmd_microflows_builder_flows.go b/mdl/executor/cmd_microflows_builder_flows.go
@@ -127,6 +127,27 @@ func (fb *flowBuilder) addPendingErrorHandlerFlowForStatement(originID, destinat
 	})
 }
 
+func (fb *flowBuilder) addPendingErrorHandlerFlowTo(destinationID model.ID) {
+	if destinationID == "" {
+		return
+	}
+	fb.rewritePendingErrorHandlers(func(state pendingErrorHandlerState) pendingErrorHandlerState {
+		if state.emptyFrom != "" {
+			fb.addEmptyErrorHandlerRejoinFlowFrom(state.emptyFrom, state.emptyFrom, destinationID)
+			state.emptyFrom = ""
+		}
+		if state.source != "" && state.tailFrom != "" {
+			fb.addErrorHandlerRejoinFlowForState(state, state.source, destinationID)
+			state.source = ""
+			state.tailFrom = ""
+			state.skipVar = ""
+			state.tailIsSource = false
+			state.returnValue = ""
+		}
+		return state
+	})
+}
+
 func (fb *flowBuilder) addPendingErrorHandlerFlowForState(state pendingErrorHandlerState, originID, destinationID model.ID, stmt ast.MicroflowStatement, futureReferencesSkipVar bool) pendingErrorHandlerState {
 	if destinationID == "" {
 		return state
@@ -147,13 +168,10 @@ func (fb *flowBuilder) addPendingErrorHandlerFlowForState(state pendingErrorHand
 	if state.skipVar != "" {
 		if statementReferencesVar(stmt, state.skipVar) {
 			if !fb.hasReturnValue {
-				endID := fb.addTerminalEndEventForPendingHandler(fb.returnType, "")
-				if state.tailIsSource {
-					fb.flows = append(fb.flows, newErrorHandlerFlow(state.tailFrom, endID))
-				} else {
-					fb.flows = append(fb.flows, newHorizontalFlow(state.tailFrom, endID))
+				if derivedVar := outputDerivedVariable(stmt, state.skipVar); derivedVar != "" {
+					state.skipVar = derivedVar
 				}
-				return pendingErrorHandlerState{}
+				return state
 			}
 			return state
 		}
@@ -332,6 +350,11 @@ func errorHandlerStatementVarRefs(stmt ast.MicroflowStatement) []string {
 		for _, param := range s.Template {
 			refs = append(refs, exprVarRefs(param.Value)...)
 		}
+	case *ast.ShowMessageStmt:
+		refs = append(refs, exprVarRefs(s.Message)...)
+		for _, arg := range s.TemplateArgs {
+			refs = append(refs, exprVarRefs(arg)...)
+		}
 	case *ast.IfStmt:
 		refs = append(refs, exprVarRefs(s.Condition)...)
 		refs = append(refs, errorHandlerStatementsVarRefs(s.ThenBody)...)
@@ -397,6 +420,19 @@ func errorHandlerStatementVarRefs(stmt ast.MicroflowStatement) []string {
 	return refs
 }
 
+func outputDerivedVariable(stmt ast.MicroflowStatement, sourceVar string) string {
+	declare, ok := stmt.(*ast.DeclareStmt)
+	if !ok || declare.Variable == "" {
+		return ""
+	}
+	for _, ref := range exprVarRefs(declare.InitialValue) {
+		if ref == sourceVar {
+			return declare.Variable
+		}
+	}
+	return ""
+}
+
 func errorHandlerStatementsVarRefs(stmts []ast.MicroflowStatement) []string {
 	var refs []string
 	for _, stmt := range stmts {
diff --git a/mdl/executor/cmd_microflows_builder_terminal_test.go b/mdl/executor/cmd_microflows_builder_terminal_test.go
@@ -3,6 +3,7 @@
 package executor
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
@@ -280,7 +281,7 @@ func TestBuildFlowGraph_OutputHandlerTerminatesBeforeDeclareReferencingOutput(t
 	body := []ast.MicroflowStatement{
 		&ast.CallMicroflowStmt{
 			OutputVariable: "CreatedRecord",
-			MicroflowName: ast.QualifiedName{Module: "SampleSync", Name: "CreateRecord"},
+			MicroflowName:  ast.QualifiedName{Module: "SampleSync", Name: "CreateRecord"},
 			ErrorHandling: &ast.ErrorHandlingClause{
 				Type: ast.ErrorHandlingCustomWithoutRollback,
 				Body: []ast.MicroflowStatement{
@@ -348,6 +349,128 @@ func TestBuildFlowGraph_OutputHandlerTerminatesBeforeDeclareReferencingOutput(t
 	}
 }
 
+func TestBuildFlowGraph_OutputHandlerInReturningBranchSkipsDerivedSuccessTail(t *testing.T) {
+	successMessage := &ast.VariableExpr{Name: "SuccessMessage"}
+	body := []ast.MicroflowStatement{
+		&ast.DeclareStmt{
+			Variable:     "ErrorMessage",
+			Type:         ast.DataType{Kind: ast.TypeString},
+			InitialValue: &ast.LiteralExpr{Kind: ast.LiteralString, Value: ""},
+		},
+		&ast.IfStmt{
+			Condition: &ast.VariableExpr{Name: "CanCreate"},
+			ThenBody: []ast.MicroflowStatement{
+				&ast.CallMicroflowStmt{
+					OutputVariable: "CreatedRecord",
+					MicroflowName:  ast.QualifiedName{Module: "SampleService", Name: "CreateRecord"},
+					ErrorHandling: &ast.ErrorHandlingClause{
+						Type: ast.ErrorHandlingCustomWithoutRollback,
+						Body: []ast.MicroflowStatement{
+							&ast.MfSetStmt{
+								Target: "ErrorMessage",
+								Value:  &ast.LiteralExpr{Kind: ast.LiteralString, Value: "create failed"},
+							},
+						},
+					},
+				},
+				&ast.DeclareStmt{
+					Variable: "SuccessMessage",
+					Type:     ast.DataType{Kind: ast.TypeString},
+					InitialValue: &ast.BinaryExpr{
+						Left:     &ast.LiteralExpr{Kind: ast.LiteralString, Value: "Created "},
+						Operator: "+",
+						Right:    &ast.AttributePathExpr{Variable: "CreatedRecord", Path: []string{"Name"}},
+					},
+				},
+				&ast.LogStmt{
+					Level:   ast.LogInfo,
+					Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "{1}"},
+					Template: []ast.TemplateParam{
+						{Index: 1, Value: successMessage},
+					},
+				},
+				&ast.ShowMessageStmt{
+					Message:      &ast.LiteralExpr{Kind: ast.LiteralString, Value: "{1}"},
+					Type:         "Information",
+					TemplateArgs: []ast.Expression{successMessage},
+				},
+				&ast.ClosePageStmt{},
+				&ast.ReturnStmt{},
+			},
+			ElseBody: []ast.MicroflowStatement{
+				&ast.MfSetStmt{
+					Target: "ErrorMessage",
+					Value:  &ast.LiteralExpr{Kind: ast.LiteralString, Value: "not found"},
+				},
+			},
+		},
+		&ast.LogStmt{
+			Level:   ast.LogError,
+			Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "{1}"},
+			Template: []ast.TemplateParam{
+				{Index: 1, Value: &ast.VariableExpr{Name: "ErrorMessage"}},
+			},
+		},
+		&ast.ShowMessageStmt{
+			Message:      &ast.LiteralExpr{Kind: ast.LiteralString, Value: "{1}"},
+			Type:         "Error",
+			TemplateArgs: []ast.Expression{&ast.VariableExpr{Name: "ErrorMessage"}},
+		},
+		&ast.ReturnStmt{},
+	}
+
+	if !statementReferencesVar(body[1].(*ast.IfStmt).ThenBody[3], "SuccessMessage") {
+		t.Fatal("SHOW MESSAGE arguments must be visible to custom handler skip-var routing")
+	}
+
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		declaredVars: map[string]string{"CanCreate": "Boolean"},
+		measurer:     &layoutMeasurer{},
+	}
+	oc := fb.buildFlowGraph(body, nil)
+
+	var callID, handlerSetID, successDeclareID, errorLogID model.ID
+	for _, obj := range oc.Objects {
+		activity, ok := obj.(*microflows.ActionActivity)
+		if !ok {
+			continue
+		}
+		switch action := activity.Action.(type) {
+		case *microflows.MicroflowCallAction:
+			if action.ResultVariableName == "CreatedRecord" {
+				callID = activity.ID
+			}
+		case *microflows.ChangeVariableAction:
+			if action.VariableName == "ErrorMessage" && strings.Contains(action.Value, "create failed") {
+				handlerSetID = activity.ID
+			}
+		case *microflows.CreateVariableAction:
+			if action.VariableName == "SuccessMessage" {
+				successDeclareID = activity.ID
+			}
+		case *microflows.LogMessageAction:
+			if action.LogLevel == "Error" {
+				errorLogID = activity.ID
+			}
+		}
+	}
+	if callID == "" || handlerSetID == "" || successDeclareID == "" || errorLogID == "" {
+		t.Fatalf("expected source call, handler set, success declare, and error log; got call=%q handler=%q declare=%q errorLog=%q", callID, handlerSetID, successDeclareID, errorLogID)
+	}
+	if !flowPathExists(oc.Flows, callID, handlerSetID) {
+		t.Fatal("source call must connect to the custom handler body")
+	}
+	if flowPathExists(oc.Flows, handlerSetID, successDeclareID) {
+		t.Fatal("custom handler must skip the output-derived success tail")
+	}
+	if !flowPathExists(oc.Flows, handlerSetID, errorLogID) {
+		t.Fatal("custom handler in a returning branch must rejoin at the shared safe continuation")
+	}
+}
+
 func TestBuildFlowGraph_EmptyNoOutputHandlerRejoinsAtNextAction(t *testing.T) {
 	body := []ast.MicroflowStatement{
 		&ast.CallMicroflowStmt{
