Merge pull request #408 from hjotha/submit/merged-pr-review-followups

fix: address merged microflow review followups

Author: ako

docs/01-project/MDL_QUICK_REFERENCE.md

@@ -1055,6 +1055,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 TestListFindAttributeEqualsExpressionUsesAttributeOperation(t *testing.T) {

mdl/executor/cmd_javaactions_test.go

@@ -19,6 +19,7 @@ func TestFormatAction_JavaActionCall_EntityTypeParam(t *testing.T) {
 	action := &microflows.JavaActionCallAction{
 		JavaAction:         "MyModule.Validate",
 		ResultVariableName: "IsValid",
+		UseReturnVariable:  true,
 		ParameterMappings: []*microflows.JavaActionParameterMapping{
 			{
 				Parameter: "MyModule.Validate.InputObject",
@@ -40,6 +41,7 @@ func TestFormatAction_JavaActionCall_MixedParamTypes(t *testing.T) {
 	action := &microflows.JavaActionCallAction{
 		JavaAction:         "MyModule.ProcessEntity",
 		ResultVariableName: "Result",
+		UseReturnVariable:  true,
 		ParameterMappings: []*microflows.JavaActionParameterMapping{
 			{
 				Parameter: "MyModule.ProcessEntity.InputObject",
@@ -67,6 +69,7 @@ func TestFormatAction_JavaActionCall_EntityTypeParam_EmptyEntity(t *testing.T) {
 	action := &microflows.JavaActionCallAction{
 		JavaAction:         "MyModule.Validate",
 		ResultVariableName: "IsValid",
+		UseReturnVariable:  true,
 		ParameterMappings: []*microflows.JavaActionParameterMapping{
 			{
 				Parameter: "MyModule.Validate.InputObject",
@@ -280,6 +283,7 @@ func TestFormatAction_JavaActionCall_EntityTypeAndParameterizedParams(t *testing
 	action := &microflows.JavaActionCallAction{
 		JavaAction:         "MyModule.CopyAttributes",
 		ResultVariableName: "Result",
+		UseReturnVariable:  true,
 		ParameterMappings: []*microflows.JavaActionParameterMapping{
 			{
 				Parameter: "MyModule.CopyAttributes.EntityType",

mdl/executor/cmd_microflows_builder_actions.go

@@ -543,13 +543,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

@@ -683,7 +683,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)
@@ -753,23 +753,23 @@ func statementErrorHandling(stmt ast.MicroflowStatement) *ast.ErrorHandlingClaus
 	}
 }
 
-func containsContinueStmt(stmts []ast.MicroflowStatement) bool {
+// containsContinueForCurrentLoop reports whether stmts contain a continue
+// that targets the enclosing loop — i.e. one that is NOT inside a nested
+// LoopStmt/WhileStmt. Nested loops trap their own continues just like they
+// trap their own breaks, so the scan stops at nested-loop boundaries.
+// This mirrors containsBreakForCurrentLoop in intent; it differs only in
+// which statement type it looks for.
+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

@@ -206,6 +206,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

@@ -83,6 +83,7 @@ func formatActivity(
 		if ctx != nil && ctx.DescribingMicroflowHasReturnValue {
 			return ""
 		}
+		// Without render context, default to the void-flow form.
 		return "return;"
 
 	case *microflows.ActionActivity:
@@ -508,7 +509,7 @@ func formatAction(
 			paramStr = strings.Join(params, ", ")
 		}
 
-		if a.ResultVariableName != "" {
+		if a.UseReturnVariable && a.ResultVariableName != "" {
 			return fmt.Sprintf("$%s = call microflow %s(%s);", a.ResultVariableName, mfName, paramStr)
 		}
 		return fmt.Sprintf("call microflow %s(%s);", mfName, paramStr)
@@ -537,7 +538,7 @@ func formatAction(
 			paramStr = strings.Join(params, ", ")
 		}
 
-		if a.OutputVariableName != "" {
+		if a.UseReturnVariable && a.OutputVariableName != "" {
 			return fmt.Sprintf("$%s = call nanoflow %s(%s);", a.OutputVariableName, nfName, paramStr)
 		}
 		return fmt.Sprintf("call nanoflow %s(%s);", nfName, paramStr)
@@ -591,7 +592,7 @@ func formatAction(
 			paramStr = strings.Join(params, ", ")
 		}
 
-		if a.ResultVariableName != "" {
+		if a.UseReturnVariable && a.ResultVariableName != "" {
 			return fmt.Sprintf("$%s = call java action %s(%s);", a.ResultVariableName, javaActionName, paramStr)
 		}
 		return fmt.Sprintf("call java action %s(%s);", javaActionName, paramStr)
@@ -616,7 +617,7 @@ func formatAction(
 			paramStr = strings.Join(params, ", ")
 		}
 
-		if a.ResultVariableName != "" {
+		if a.UseReturnVariable && a.ResultVariableName != "" {
 			return fmt.Sprintf("$%s = call external action %s.%s(%s);", a.ResultVariableName, serviceName, actionName, paramStr)
 		}
 		return fmt.Sprintf("call external action %s.%s(%s);", serviceName, actionName, paramStr)
@@ -758,7 +759,7 @@ func formatAction(
 		return fmt.Sprintf("get workflow data $%s as %s;", a.WorkflowVariable, a.Workflow)
 
 	case *microflows.WorkflowCallAction:
-		if a.OutputVariableName != "" {
+		if a.UseReturnVariable && a.OutputVariableName != "" {
 			return fmt.Sprintf("$%s = call workflow %s ($%s);", a.OutputVariableName, a.Workflow, a.WorkflowContextVariable)
 		}
 		return fmt.Sprintf("call workflow %s ($%s);", a.Workflow, a.WorkflowContextVariable)
@@ -855,7 +856,7 @@ func formatAction(
 			paramStr = strings.Join(params, ", ")
 		}
 
-		if a.OutputVariableName != "" {
+		if a.UseReturnVariable && a.OutputVariableName != "" {
 			return fmt.Sprintf("$%s = call javascript action %s(%s);", a.OutputVariableName, jsActionName, paramStr)
 		}
 		return fmt.Sprintf("call javascript action %s(%s);", jsActionName, paramStr)
@@ -1591,7 +1592,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