fix: address merged microflow review followups

1. Manual while-true detection over-triggered when the only continue lived
   inside a nested loop. The continue scan crossed loop boundaries while the
   break scan did not, so the outer while could be rebuilt as a manual
   back-edge. Stop the continue scan at nested loops and add a regression test.

2. Owner-both reverse retrieves feeding add/remove-to-list were still treated
   as object-only consumers. The list pre-scan ignored AddToListStmt and
   RemoveFromListStmt target lists, so AssociationRetrieveSource could be
   suppressed. Track those list consumers and add coverage for the helper.

3. Direct nanoflow describe did not set the return-value render context used
   by EndEvent formatting. Thread the wrapped nanoflow return type through the
   formatter so value-returning nanoflows do not emit bare return; for empty
   EndEvents.

Also folds in low-risk review followups: commit-action writer coverage,
change-object refresh negative coverage, download-file formatter coverage,
reverse-retrieve name validation tightening, and documentation for
MXCLI_EXEC_TIMEOUT.

Tests: make build
Tests: make test
Tests: make lint-go

Closes #404.
Closes #405.
Closes #406.

Author: hjothamendix

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",
 	}