fix: loop-back retry error handler to merge before source activity (#506)

mdl/executor/cmd_microflows_builder.go

@@ -32,6 +32,13 @@ type flowBuilder struct {
 	errors              []string          // Validation errors collected during build
 	measurer            *layoutMeasurer   // For measuring statement dimensions
 	nextConnectionPoint model.ID          // For compound statements: the exit point differs from entry point
+	// incomingRedirect, when set, instructs the next enclosing flow emission to
+	// terminate at this merge/activity ID instead of the most recently emitted
+	// activity. Used by retry-loop error handlers (where a merge is inserted
+	// before the source activity so the normal inbound flow enters the merge
+	// and the error-handler tail loops back to the same merge). Cleared after
+	// the outer loop consumes it for the next flow.
+	incomingRedirect model.ID
 	nextFlowCase        string            // If set, next connecting flow uses this case value (for merge-less splits)
 	// nextFlowAnchor carries the branch-specific FlowAnchors that should be
 	// applied to the flow created by the NEXT iteration of buildFlowGraph.

mdl/executor/cmd_microflows_builder_flows.go

@@ -51,13 +51,115 @@ func (fb *flowBuilder) finishCustomErrorHandler(activityID model.ID, activityX i
 		return
 	}
 	if len(eh.Body) > 0 {
+		// Retry-loop pattern: error body ends with an IF whose non-terminating
+		// branch should loop back to the source activity (Studio Pro authors
+		// this as a merge placed before the source, with the handler tail
+		// returning to that merge). MDL cannot express the loop-back directly,
+		// so we detect the shape and wire the topology ourselves.
+		if isRetryLoopErrorHandler(eh.Body) {
+			fb.buildRetryLoopErrorHandler(activityID, activityX, eh.Body)
+			return
+		}
 		mergeID := fb.addErrorHandlerFlow(activityID, activityX, eh.Body)
 		fb.handleErrorHandlerMergeWithSkip(mergeID, activityID, outputVar)
 		return
 	}
 	fb.registerEmptyCustomErrorHandlerWithSkip(activityID, eh, outputVar)
 }
 
+// isRetryLoopErrorHandler reports whether the error-handler body looks like
+// a retry loop: the last statement is an IF whose else branch terminates
+// (via RAISE ERROR or RETURN) and whose then branch continues. That shape
+// mirrors the Studio Pro retry pattern where the non-terminating branch
+// loops back to the source activity to re-attempt it.
+func isRetryLoopErrorHandler(body []ast.MicroflowStatement) bool {
+	if len(body) == 0 {
+		return false
+	}
+	ifStmt, ok := body[len(body)-1].(*ast.IfStmt)
+	if !ok {
+		return false
+	}
+	if len(ifStmt.ThenBody) == 0 || len(ifStmt.ElseBody) == 0 {
+		return false
+	}
+	thenTerminates := bodyTerminates(ifStmt.ThenBody)
+	elseTerminates := bodyTerminates(ifStmt.ElseBody)
+	// Exactly one branch must terminate (the "error" branch) and one must
+	// continue (the "retry" branch). If both or neither terminate, the shape
+	// is some other IF and the standard merge-forward path still applies.
+	return thenTerminates != elseTerminates
+}
+
+// bodyTerminates reports whether a statement body ends with a terminator
+// (RAISE ERROR, RETURN, or nested IF where every branch terminates).
+func bodyTerminates(body []ast.MicroflowStatement) bool {
+	if len(body) == 0 {
+		return false
+	}
+	last := body[len(body)-1]
+	switch s := last.(type) {
+	case *ast.RaiseErrorStmt:
+		return true
+	case *ast.ReturnStmt:
+		return true
+	case *ast.IfStmt:
+		if len(s.ElseBody) == 0 {
+			return false
+		}
+		return bodyTerminates(s.ThenBody) && bodyTerminates(s.ElseBody)
+	}
+	return false
+}
+
+// buildRetryLoopErrorHandler wires a retry-loop topology for an error handler
+// whose body ends with a terminating/continuing IF. The non-terminating
+// branch's tail loops back to a new merge placed before the source activity;
+// the merge then feeds into the source. The outer loop consumes
+// fb.incomingRedirect so the normal inbound flow also terminates at the
+// merge instead of directly at the source.
+func (fb *flowBuilder) buildRetryLoopErrorHandler(sourceActivityID model.ID, sourceX int, errorBody []ast.MicroflowStatement) {
+	// Build the handler activities with tracking of where the non-terminating
+	// branch's tail ends up. We reuse addErrorHandlerFlow; it returns the tail
+	// that would have forwarded to the next main-path activity. For a retry
+	// loop, that tail is the continue-branch exit of the trailing IF.
+	tail := fb.addErrorHandlerFlow(sourceActivityID, sourceX, errorBody)
+	if tail.id == "" {
+		// The handler terminates unexpectedly — nothing to loop back.
+		return
+	}
+
+	// Insert a merge just left of the source activity, on the main flow row.
+	merge := &microflows.ExclusiveMerge{
+		BaseMicroflowObject: microflows.BaseMicroflowObject{
+			BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
+			Position:    model.Point{X: sourceX - fb.spacing/2, Y: fb.baseY},
+			Size:        model.Size{Width: MergeSize, Height: MergeSize},
+		},
+	}
+	fb.objects = append(fb.objects, merge)
+
+	// Merge -> source (normal flow into the REST/microflow call)
+	fb.flows = append(fb.flows, newHorizontalFlow(merge.ID, sourceActivityID))
+
+	// Handler tail -> merge (loop-back). Authored Studio Pro flows mark this
+	// edge as a normal SequenceFlow (not IsErrorHandler) — the error flow
+	// marker only applies to the SOURCE → first-handler-activity edge, which
+	// addErrorHandlerFlow already emitted. Using a plain horizontal flow here
+	// avoids triggering spurious CE0136/CE0019 diagnostics that fire when a
+	// retrieve's start variable is deemed to flow through an error edge.
+	loopFlow := newHorizontalFlow(tail.id, merge.ID)
+	if tail.caseValue != "" {
+		applyDeferredFlowCase(loopFlow, tail.caseValue, tail.flowAnchor)
+	} else if tail.flowAnchor != nil {
+		applyDeferredFlowCase(loopFlow, "", tail.flowAnchor)
+	}
+	fb.flows = append(fb.flows, loopFlow)
+
+	// Next inbound normal flow must terminate at the merge, not at the source.
+	fb.incomingRedirect = merge.ID
+}
+
 func (fb *flowBuilder) registerEmptyCustomErrorHandlerWithSkip(activityID model.ID, eh *ast.ErrorHandlingClause, skipVar string) {
 	if !isEmptyCustomErrorHandler(eh) {
 		return

mdl/executor/cmd_microflows_builder_graph.go

@@ -76,13 +76,21 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
 		activityID := fb.addStatement(stmt)
 		if activityID != "" {
 			fb.applyPendingAnnotations(activityID)
-			// Connect to previous object with horizontal SequenceFlow
+			// Connect to previous object with horizontal SequenceFlow.
+			// When incomingRedirect is set (retry-loop error handler built a
+			// merge before the activity), the inbound flow terminates at the
+			// merge instead of the activity itself.
+			inboundTarget := activityID
+			if fb.incomingRedirect != "" {
+				inboundTarget = fb.incomingRedirect
+				fb.incomingRedirect = ""
+			}
 			var flow *microflows.SequenceFlow
 			if pendingCase != "" {
-				flow = newHorizontalFlowWithCase(lastID, activityID, pendingCase)
+				flow = newHorizontalFlowWithCase(lastID, inboundTarget, pendingCase)
 				pendingCase = ""
 			} else {
-				flow = newHorizontalFlow(lastID, activityID)
+				flow = newHorizontalFlow(lastID, inboundTarget)
 			}
 			// Prefer the pendingFlowAnchor (carried from a guard-pattern IF's
 			// branch) over the previous statement's own anchor — it encodes

mdl/executor/cmd_microflows_retry_loop_test.go

@@ -0,0 +1,213 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// Retry-loop error handler: a call activity with a custom error handler
+// whose body ends with an IF where the ELSE raises and the THEN performs
+// retry actions must produce a merge placed before the source activity,
+// with the handler tail looping back to the merge. Without this topology
+// the merge falls after the source and subsequent activities referencing
+// the call's output variable trigger CE0108 "variable not in scope".
+func TestRetryLoopErrorHandlerLoopsBackToSource(t *testing.T) {
+	fb := &flowBuilder{
+		spacing:      HorizontalSpacing,
+		measurer:     &layoutMeasurer{},
+		declaredVars: map[string]string{"R": "String", "RetryCount": "Integer"},
+	}
+
+	oc := fb.buildFlowGraph([]ast.MicroflowStatement{
+		&ast.DeclareStmt{
+			Variable:     "RetryCount",
+			Type:         ast.DataType{Kind: ast.TypeInteger},
+			InitialValue: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "0"},
+		},
+		&ast.CallMicroflowStmt{
+			MicroflowName:  ast.QualifiedName{Module: "M", Name: "Fetch"},
+			OutputVariable: "R",
+			ErrorHandling: &ast.ErrorHandlingClause{
+				Type: ast.ErrorHandlingCustomWithoutRollback,
+				Body: []ast.MicroflowStatement{
+					&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "fetch failed"}},
+					&ast.IfStmt{
+						Condition: &ast.BinaryExpr{
+							Operator: "<",
+							Left:     &ast.VariableExpr{Name: "RetryCount"},
+							Right:    &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "3"},
+						},
+						ThenBody: []ast.MicroflowStatement{
+							&ast.MfSetStmt{
+								Target: "RetryCount",
+								Value: &ast.BinaryExpr{
+									Operator: "+",
+									Left:     &ast.VariableExpr{Name: "RetryCount"},
+									Right:    &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "1"},
+								},
+							},
+						},
+						ElseBody: []ast.MicroflowStatement{
+							&ast.RaiseErrorStmt{},
+						},
+					},
+				},
+			},
+		},
+		&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "ok"}},
+		&ast.ReturnStmt{},
+	}, nil)
+
+	// Find the call activity, the retry-body log/set activities, and the merge.
+	var callID, logAfterID model.ID
+	var mergeIDs []model.ID
+	logSeen := 0
+	for _, obj := range oc.Objects {
+		switch o := obj.(type) {
+		case *microflows.ActionActivity:
+			if _, ok := o.Action.(*microflows.MicroflowCallAction); ok {
+				callID = o.ID
+			}
+			if _, ok := o.Action.(*microflows.LogMessageAction); ok {
+				logSeen++
+				// logSeen==1 is the error-handler log inside the handler body.
+				// logSeen==2 is the main-path log after the call.
+				if logSeen == 2 {
+					logAfterID = o.ID
+				}
+			}
+		case *microflows.ExclusiveMerge:
+			mergeIDs = append(mergeIDs, o.ID)
+		}
+	}
+	if callID == "" {
+		t.Fatal("no call activity found")
+	}
+	if logAfterID == "" {
+		t.Fatal("no post-call log activity found")
+	}
+	if len(mergeIDs) == 0 {
+		t.Fatal("no merge node found — retry-loop topology was not built")
+	}
+
+	// The merge must sit on the call's inbound path: there must be an edge
+	// prev->merge (not prev->call) and an edge merge->call. The handler
+	// tail loops back to the merge as a plain SequenceFlow (not marked
+	// IsErrorHandler — the error marker applies only to the source→first
+	// handler activity edge, which addErrorHandlerFlow emits separately).
+	var mergeToCallFound bool
+	var prevToMergeFound bool
+	var tailToMergeFound bool
+	// Find the retry-branch tail activity (the ChangeVariable that
+	// increments the retry counter in the test fixture).
+	var tailID model.ID
+	for _, obj := range oc.Objects {
+		if a, ok := obj.(*microflows.ActionActivity); ok {
+			if _, isChange := a.Action.(*microflows.ChangeVariableAction); isChange {
+				tailID = a.ID
+			}
+		}
+	}
+	for _, f := range oc.Flows {
+		for _, mID := range mergeIDs {
+			if f.DestinationID == callID && f.OriginID == mID {
+				mergeToCallFound = true
+			}
+			if f.DestinationID == mID && f.OriginID == tailID && !f.IsErrorHandler {
+				tailToMergeFound = true
+			}
+			if f.DestinationID == mID && !f.IsErrorHandler && f.OriginID != mID && f.OriginID != tailID {
+				prevToMergeFound = true
+			}
+		}
+		// No flow should terminate directly at the call from a non-merge origin.
+		if f.DestinationID == callID && !f.IsErrorHandler {
+			originIsMerge := false
+			for _, mID := range mergeIDs {
+				if f.OriginID == mID {
+					originIsMerge = true
+				}
+			}
+			if !originIsMerge {
+				t.Errorf("normal inbound flow to call %s came from non-merge origin %s; retry-loop merge was not inserted",
+					shortID(callID), shortID(f.OriginID))
+			}
+		}
+	}
+	if !mergeToCallFound {
+		t.Error("missing merge -> call flow")
+	}
+	if !prevToMergeFound {
+		t.Error("missing prev -> merge normal flow")
+	}
+	if !tailToMergeFound {
+		t.Error("missing handler-tail -> merge (loop-back) flow")
+	}
+}
+
+// Non-retry error handlers (both branches return, neither branch raises,
+// or no trailing IF) must still use the forward-merge path.
+func TestNonRetryErrorHandlerUsesForwardMerge(t *testing.T) {
+	fb := &flowBuilder{
+		spacing:      HorizontalSpacing,
+		measurer:     &layoutMeasurer{},
+		declaredVars: map[string]string{"R": "String"},
+	}
+
+	oc := fb.buildFlowGraph([]ast.MicroflowStatement{
+		&ast.CallMicroflowStmt{
+			MicroflowName:  ast.QualifiedName{Module: "M", Name: "Fetch"},
+			OutputVariable: "R",
+			ErrorHandling: &ast.ErrorHandlingClause{
+				Type: ast.ErrorHandlingCustomWithoutRollback,
+				Body: []ast.MicroflowStatement{
+					// Single log — no trailing IF, no retry shape.
+					&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "failed"}},
+				},
+			},
+		},
+		&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "ok"}},
+		&ast.ReturnStmt{},
+	}, nil)
+
+	// The call should have an inbound flow from the previous activity (the
+	// StartEvent or a fb-generated start), NOT via a merge. If a merge were
+	// incorrectly inserted, every such non-retry microflow would gain an
+	// extra unnecessary node.
+	var callID model.ID
+	for _, obj := range oc.Objects {
+		if a, ok := obj.(*microflows.ActionActivity); ok {
+			if _, isCall := a.Action.(*microflows.MicroflowCallAction); isCall {
+				callID = a.ID
+			}
+		}
+	}
+	if callID == "" {
+		t.Fatal("no call activity")
+	}
+	for _, f := range oc.Flows {
+		if f.DestinationID == callID && !f.IsErrorHandler {
+			// Origin should be a non-merge (StartEvent or a plain activity).
+			for _, obj := range oc.Objects {
+				if obj.GetID() == f.OriginID {
+					if _, isMerge := obj.(*microflows.ExclusiveMerge); isMerge {
+						t.Errorf("non-retry handler unexpectedly inserted a merge before call")
+					}
+				}
+			}
+		}
+	}
+}
+
+func shortID(id model.ID) string {
+	s := string(id)
+	if len(s) > 8 {
+		return s[:8]
+	}
+	return s
+}