fix: address merged microflow review followups

docs/01-project/MDL_QUICK_REFERENCE.md

@@ -1047,6 +1047,8 @@ Cross-reference commands require `refresh catalog full` to populate reference da
 | Setup mxcli | `mxcli setup mxcli [--os linux]` | Download platform-specific mxcli binary |
 | LSP server | `mxcli lsp --stdio` | Language server for VS Code |
 
+Set `MXCLI_EXEC_TIMEOUT` to override the per-statement execution timeout used by `mxcli exec` (for example `MXCLI_EXEC_TIMEOUT=12m` or `MXCLI_EXEC_TIMEOUT=900`).
+
 ## ANTLR4 Parser Architecture
 
 The MDL parser uses ANTLR4 for grammar definition, enabling cross-language grammar sharing (Go, TypeScript, Java, Python).

mdl/executor/bugfix_regression_test.go

@@ -694,6 +694,28 @@ func TestEmptyChangeObjectRefreshesInClient(t *testing.T) {
 	if !action.RefreshInClient {
 		t.Fatal("empty change object must refresh in client to remain valid without member changes or commit")
 	}
+
+	id = fb.addChangeObjectAction(&ast.ChangeObjectStmt{
+		Variable: "Object",
+		Changes: []ast.ChangeItem{{
+			Attribute: "Name",
+			Value:     &ast.LiteralExpr{Kind: ast.LiteralString, Value: "changed"},
+		}},
+	})
+	if id == "" || len(fb.objects) != 2 {
+		t.Fatalf("expected second change object activity, got id=%q objects=%d", id, len(fb.objects))
+	}
+	activity, ok = fb.objects[1].(*microflows.ActionActivity)
+	if !ok {
+		t.Fatalf("object type = %T, want *microflows.ActionActivity", fb.objects[1])
+	}
+	action, ok = activity.Action.(*microflows.ChangeObjectAction)
+	if !ok {
+		t.Fatalf("action type = %T, want *microflows.ChangeObjectAction", activity.Action)
+	}
+	if action.RefreshInClient {
+		t.Fatal("non-empty change object must not infer refresh in client")
+	}
 }
 
 func TestCallMicroflowUnknownResultTypeStillDeclaresVariable(t *testing.T) {

mdl/executor/cmd_microflows_builder.go

@@ -53,7 +53,7 @@ type flowBuilder struct {
 	microflowsCacheLoaded bool
 	nanoflowsCache        []*microflows.Nanoflow
 	nanoflowsCacheLoaded  bool
-	manualLoopBackTarget model.ID
+	manualLoopBackTarget  model.ID
 }
 
 // addError records a validation error during flow building.

mdl/executor/cmd_microflows_builder_actions.go

@@ -308,13 +308,14 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID {
 		outputUsedAsObject := fb.objectInputVariables != nil && fb.objectInputVariables[s.Variable]
 		// Owner-both Reference associations need later usage context: the same
 		// compact retrieve can be consumed as either a list or a single object.
+		// Owner="" means metadata was unavailable, so keep the association source.
 		expandReverseReference := assocInfo != nil &&
 			assocInfo.Type == domainmodel.AssociationTypeReference &&
 			assocInfo.Owner != "" &&
 			assocInfo.parentPersistable &&
 			assocInfo.childEntityQN != "" &&
 			startVarType == assocInfo.childEntityQN &&
-			(assocInfo.Owner != domainmodel.AssociationOwnerBoth || outputUsedAsList && !outputUsedAsObject)
+			(assocInfo.Owner != domainmodel.AssociationOwnerBoth || (outputUsedAsList && !outputUsedAsObject))
 
 		if expandReverseReference {
 			// Reverse traversal on Reference: child → parent (one-to-many)

mdl/executor/cmd_microflows_builder_collectlistinputs_test.go

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+)
+
+// TestCollectListInputVariables_AddRemoveFromList pins issue #405:
+// `add $X to $List` and `remove $Y from $List` consume the target list, so
+// the list variable must be tracked as a list input. Without it, the output
+// of an Owner=Both reverse retrieve fed straight into add/remove was
+// misclassified as object-only and the AssociationRetrieveSource was
+// suppressed, re-introducing the original #383 bug for this usage shape.
+func TestCollectListInputVariables_AddRemoveFromList(t *testing.T) {
+	stmts := []ast.MicroflowStatement{
+		&ast.AddToListStmt{Item: "NewItem", List: "Items"},
+		&ast.RemoveFromListStmt{Item: "OldItem", List: "Backlog"},
+	}
+
+	got := collectListInputVariables(stmts)
+
+	if !got["Items"] {
+		t.Errorf("AddToListStmt target `Items` must be marked as list input; got %v", got)
+	}
+	if !got["Backlog"] {
+		t.Errorf("RemoveFromListStmt target `Backlog` must be marked as list input; got %v", got)
+	}
+}

mdl/executor/cmd_microflows_builder_control.go

@@ -441,7 +441,7 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID {
 }
 
 func isManualWhileTrueCandidate(s *ast.WhileStmt) bool {
-	if s == nil || containsBreakForCurrentLoop(s.Body) || (!containsContinueStmt(s.Body) && !containsTerminalStmt(s.Body)) {
+	if s == nil || containsBreakForCurrentLoop(s.Body) || (!containsContinueForCurrentLoop(s.Body) && !containsTerminalStmt(s.Body)) {
 		return false
 	}
 	lit, ok := s.Condition.(*ast.LiteralExpr)
@@ -470,23 +470,19 @@ func containsBreakForCurrentLoop(stmts []ast.MicroflowStatement) bool {
 	return false
 }
 
-func containsContinueStmt(stmts []ast.MicroflowStatement) bool {
+// containsContinueForCurrentLoop mirrors containsBreakForCurrentLoop:
+// a continue inside a nested loop targets that nested loop, not this one.
+func containsContinueForCurrentLoop(stmts []ast.MicroflowStatement) bool {
 	for _, stmt := range stmts {
 		switch s := stmt.(type) {
 		case *ast.ContinueStmt:
 			return true
 		case *ast.IfStmt:
-			if containsContinueStmt(s.ThenBody) || containsContinueStmt(s.ElseBody) {
-				return true
-			}
-		case *ast.LoopStmt:
-			if containsContinueStmt(s.Body) {
-				return true
-			}
-		case *ast.WhileStmt:
-			if containsContinueStmt(s.Body) {
+			if containsContinueForCurrentLoop(s.ThenBody) || containsContinueForCurrentLoop(s.ElseBody) {
 				return true
 			}
+		case *ast.LoopStmt, *ast.WhileStmt:
+			continue
 		}
 	}
 	return false

mdl/executor/cmd_microflows_builder_graph.go

@@ -116,6 +116,8 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
 
 	// Handle leftover pending annotations (free-floating annotation text)
 	if fb.pendingAnnotations != nil {
+		// Free annotations before a statement stay unattached; trailing free
+		// annotations are drained after the statement loop below.
 		for _, text := range freeAnnotationTexts(fb.pendingAnnotations) {
 			fb.attachFreeAnnotation(text)
 		}
@@ -185,6 +187,14 @@ func collectListInputVariables(stmts []ast.MicroflowStatement) map[string]bool {
 					inputs[s.ListVariable] = true
 				}
 				walk(s.Body)
+			case *ast.AddToListStmt:
+				if s.List != "" {
+					inputs[s.List] = true
+				}
+			case *ast.RemoveFromListStmt:
+				if s.List != "" {
+					inputs[s.List] = true
+				}
 			case *ast.WhileStmt:
 				walk(s.Body)
 			case *ast.IfStmt:

mdl/executor/cmd_microflows_builder_manual_while_nested_test.go

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// TestBuildFlowGraph_ManualWhileTrueIgnoresNestedLoopContinue pins issue #404:
+// a `while true` whose only `continue` lives inside a nested collection loop
+// must NOT be classified as a manual back-edge candidate. The outer flow
+// should be built as a regular LoopedActivity (with a WhileLoopCondition).
+//
+// Before the fix, containsContinueStmt recursed into nested LoopStmt bodies
+// asymmetrically with containsBreakForCurrentLoop, so isManualWhileTrueCandidate
+// returned true and the outer while was rebuilt as an ExclusiveMerge back-edge,
+// creating an unconditional infinite loop in the BSON graph.
+func TestBuildFlowGraph_ManualWhileTrueIgnoresNestedLoopContinue(t *testing.T) {
+	body := []ast.MicroflowStatement{
+		&ast.WhileStmt{
+			Condition: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true},
+			Body: []ast.MicroflowStatement{
+				&ast.LoopStmt{
+					LoopVariable: "item",
+					ListVariable: "items",
+					Body: []ast.MicroflowStatement{
+						&ast.ContinueStmt{},
+					},
+				},
+				// No outer-scope continue / return / raise: the outer while
+				// has no terminal signal of its own.
+			},
+		},
+	}
+
+	fb := &flowBuilder{
+		posX:         100,
+		posY:         100,
+		spacing:      HorizontalSpacing,
+		measurer:     &layoutMeasurer{},
+		varTypes:     map[string]string{"items": "List of Sample.Item"},
+		declaredVars: map[string]string{"items": "List of Sample.Item"},
+	}
+	oc := fb.buildFlowGraph(body, nil)
+
+	var (
+		outerLoop  *microflows.LoopedActivity
+		mergeCount int
+	)
+	for _, obj := range oc.Objects {
+		switch o := obj.(type) {
+		case *microflows.LoopedActivity:
+			// The first looped activity at this scope is the outer while.
+			if outerLoop == nil {
+				outerLoop = o
+			}
+		case *microflows.ExclusiveMerge:
+			mergeCount++
+		}
+	}
+
+	if outerLoop == nil {
+		t.Fatal("outer `while true` must be built as a LoopedActivity, not an ExclusiveMerge back-edge")
+	}
+	if outerLoop.LoopSource == nil {
+		t.Fatal("outer LoopedActivity must have a LoopSource (WhileLoopCondition for `while true`)")
+	}
+	if mergeCount != 0 {
+		t.Errorf("manual back-edge ExclusiveMerge must not be emitted; got %d ExclusiveMerge node(s)", mergeCount)
+	}
+}

mdl/executor/cmd_microflows_format_action.go

@@ -79,6 +79,7 @@ func formatActivity(
 		if ctx != nil && ctx.DescribingMicroflowHasReturnValue {
 			return ""
 		}
+		// Without render context, default to the void-flow form.
 		return "return;"
 
 	case *microflows.ActionActivity:
@@ -351,30 +352,30 @@ func formatAction(
 
 			stmt := fmt.Sprintf("retrieve $%s from %s", outputVar, entityName)
 
-		if dbSource.XPathConstraint != "" {
-			constraint := strings.TrimSpace(dbSource.XPathConstraint)
-			// XPath may contain multiple predicates like [a][b] or [a]\n[b].
-			// Split them and join with MDL 'and' so the parser sees
-			// separate xpathConstraint nodes.
-			if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") {
-				// Split on "][" boundary (possibly separated by \n literals),
-				// then re-wrap each predicate.
-				inner := constraint[1 : len(constraint)-1]
-				// Normalise real newlines between predicates: ]\n[ → ][
-				inner = strings.ReplaceAll(inner, "]\n[", "][")
-				parts := strings.Split(inner, "][")
-				if len(parts) > 1 {
-					var wrapped []string
-					for _, p := range parts {
-						wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]")
+			if dbSource.XPathConstraint != "" {
+				constraint := strings.TrimSpace(dbSource.XPathConstraint)
+				// XPath may contain multiple predicates like [a][b] or [a]\n[b].
+				// Split them and join with MDL 'and' so the parser sees
+				// separate xpathConstraint nodes.
+				if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") {
+					// Split on "][" boundary (possibly separated by \n literals),
+					// then re-wrap each predicate.
+					inner := constraint[1 : len(constraint)-1]
+					// Normalise real newlines between predicates: ]\n[ to ][
+					inner = strings.ReplaceAll(inner, "]\n[", "][")
+					parts := strings.Split(inner, "][")
+					if len(parts) > 1 {
+						var wrapped []string
+						for _, p := range parts {
+							wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]")
+						}
+						constraint = strings.Join(wrapped, "\n    ")
+					} else {
+						constraint = parts[0]
 					}
-					constraint = strings.Join(wrapped, "\n    ")
-				} else {
-					constraint = parts[0]
 				}
+				stmt += fmt.Sprintf("\n    where %s", constraint)
 			}
-			stmt += fmt.Sprintf("\n    where %s", constraint)
-		}
 
 			// Output SORT BY clause if present
 			if len(dbSource.Sorting) > 0 {
@@ -1378,7 +1379,13 @@ func isSimpleMendixName(name string) bool {
 		return false
 	}
 	for i, r := range name {
-		if r == '_' || r >= 'A' && r <= 'Z' || r >= 'a' && r <= 'z' || i > 0 && r >= '0' && r <= '9' {
+		if i == 0 {
+			if r >= 'A' && r <= 'Z' || r >= 'a' && r <= 'z' {
+				continue
+			}
+			return false
+		}
+		if r == '_' || r >= 'A' && r <= 'Z' || r >= 'a' && r <= 'z' || r >= '0' && r <= '9' {
 			continue
 		}
 		return false

mdl/executor/cmd_microflows_format_action_test.go

@@ -507,6 +507,19 @@ func TestFormatAction_DownloadFile(t *testing.T) {
 	}
 }
 
+func TestFormatAction_DownloadFileWithoutBrowserFlag(t *testing.T) {
+	e := newTestExecutor()
+	action := &microflows.DownloadFileAction{
+		FileDocument: "GeneratedReport",
+	}
+
+	got := e.formatAction(action, nil, nil)
+	want := "download file $GeneratedReport;"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
+
 func TestFormatAction_ValidationFeedback(t *testing.T) {
 	e := newTestExecutor()
 	action := &microflows.ValidationFeedbackAction{
@@ -752,6 +765,9 @@ func TestParseReverseAssociationXPathRejectsComplexPredicates(t *testing.T) {
 		"[SampleRuntime.Domain_Runtime != $Runtime]",
 		"[SampleRuntime.Domain_Runtime = $Runtime/Other.Assoc]",
 		"[SampleRuntime.Domain_Runtime = 'literal']",
+		"[_SampleRuntime.Domain_Runtime = $Runtime]",
+		"[SampleRuntime._Domain_Runtime = $Runtime]",
+		"[SampleRuntime.Domain_Runtime = $_Runtime]",
 		"SampleRuntime.Domain_Runtime = $Runtime",
 	}
 

mdl/executor/cmd_microflows_show.go

@@ -427,10 +427,17 @@ func describeNanoflow(ctx *ExecContext, name ast.QualifiedName) error {
 	lines = append(lines, "begin")
 
 	// Wrap nanoflow in a Microflow to reuse formatMicroflowActivities
+	wrapperMf := &microflows.Microflow{
+		ReturnType:       targetNf.ReturnType,
+		ObjectCollection: targetNf.ObjectCollection,
+	}
+	prevDescribingReturnValue := ctx.DescribingMicroflowHasReturnValue
+	ctx.DescribingMicroflowHasReturnValue = microflowHasReturnValue(wrapperMf)
+	defer func() {
+		ctx.DescribingMicroflowHasReturnValue = prevDescribingReturnValue
+	}()
+
 	if targetNf.ObjectCollection != nil && len(targetNf.ObjectCollection.Objects) > 0 {
-		wrapperMf := &microflows.Microflow{
-			ObjectCollection: targetNf.ObjectCollection,
-		}
 		activityLines := formatMicroflowActivities(ctx, wrapperMf, entityNames, microflowNames)
 		for _, line := range activityLines {
 			lines = append(lines, "  "+line)

mdl/executor/cmd_microflows_show_helpers_test.go

@@ -362,7 +362,7 @@ func TestFormatActivity_StartEvent(t *testing.T) {
 	}
 }
 
-func TestFormatActivity_EndEvent_NoReturn(t *testing.T) {
+func TestFormatActivity_EndEvent_VoidOrUnknownContext(t *testing.T) {
 	e := newTestExecutor()
 	obj := &microflows.EndEvent{BaseMicroflowObject: mkObj("1")}
 	got := e.formatActivity(obj, nil, nil)