fix: describe silently dropped trailing XPath constraint groups (#772)

.claude/skills/fix-issue.md

@@ -316,6 +316,7 @@ cases for these three BSON types — they fell to `default: return nil`.
 | `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases |
 | `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests |
 | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests |
+| `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 |
 
 **Key insight:** `microflows$ListRange` stores offset/limit inside a nested
 `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before

mdl-examples/bug-tests/772-xpath-constraint-groups.mdl

@@ -0,0 +1,68 @@
+-- Bug #772: describe silently dropped trailing XPath constraint groups
+--
+-- Mendix stores sibling predicate groups concatenated in one XPathConstraint field:
+--
+--   [Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]
+--   [Status != 'Completed']
+--   [CompletionDate = empty]
+--
+-- The MDL grammar rule matches ONE bracket group. With the error listeners removed,
+-- ANTLR parsed the first group, left the rest on the token stream, and still
+-- reported success. enrichXPathConstraintForDescribe read that as a full parse and
+-- re-rendered only what came back, so describe emitted:
+--
+--   where Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit];
+--
+-- The last two predicates vanished with no warning. That is worse than a crash: the
+-- output looks complete while describing a materially LESS RESTRICTIVE query than
+-- the project actually contains, which makes correct defensive code read as buggy.
+--
+-- Two fixes, defence in depth:
+--   1. ParseXPathConstraint now reports a partial parse as a failure, so a caller
+--      falls back to the stored string instead of silently truncating it.
+--   2. visitor.SplitXPathPredicateGroups splits the constraint into top-level groups
+--      — tracking nesting depth and quoting, so a nested [..] or a ']' inside a
+--      string literal is handled — and each group is enriched and rendered on its
+--      own. Splitting on "][" mangled both of those.
+--
+-- Verify:
+--   1. mxcli exec 772-xpath-constraint-groups.mdl -p app.mpr
+--   2. mxcli -p app.mpr -c "describe microflow Bug772.MF_GetOpenTasks"
+--      All three groups must appear, and Status must render as the qualified
+--      enum value Bug772.TaskStatus.Completed (enrichment reaches later groups too).
+--   3. Feed that describe output back through exec: it re-creates the same flow.
+--
+
+create module Bug772;
+create module role Bug772.User;
+
+create enumeration Bug772.TaskStatus (
+  Open 'Open',
+  Completed 'Completed'
+);
+
+create persistent entity Bug772.TaskGroup (
+  Name: String(100),
+  EndDate: DateTime
+);
+
+create persistent entity Bug772.Task (
+  Title: String(100),
+  Status: Enumeration(Bug772.TaskStatus),
+  CompletionDate: DateTime
+);
+
+create association Bug772.Task_TaskGroup
+  from Bug772.Task to Bug772.TaskGroup
+  type Reference;
+
+create microflow Bug772.MF_GetOpenTasks (
+  EndDateLimit: DateTime
+)
+returns list of Bug772.Task as $Tasks
+begin
+  retrieve $Tasks from Bug772.Task
+    where '[Bug772.Task_TaskGroup/Bug772.TaskGroup[EndDate = $EndDateLimit]][Status != Bug772.TaskStatus.Completed][CompletionDate = empty]';
+  return $Tasks;
+end;
+/

mdl/executor/cmd_microflows_format_action.go

@@ -429,24 +429,18 @@ func formatAction(
 				// (e.g. Status = 'Open' → Status = Module.OrderStatus.Open) when
 				// the entity is known and we are connected to a project.
 				constraint = enrichXPathConstraintForDescribe(ctx, entityName, constraint)
-				// 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)+"]")
-						}
-						constraint = strings.Join(wrapped, "\n    ")
+				// XPath may hold several sibling predicate groups — [a][b], or
+				// separated by newlines. Emit each on its own line so the parser
+				// sees separate xpathConstraint nodes. The splitter is nesting- and
+				// quote-aware: the previous "][" split mangled a group containing a
+				// nested bracket or a literal with a ']' in it (#772).
+				if groups := visitor.SplitXPathPredicateGroups(constraint); len(groups) > 0 {
+					if len(groups) > 1 {
+						constraint = strings.Join(groups, "\n    ")
 					} else {
-						constraint = parts[0]
+						// A lone group renders without its outer brackets, matching
+						// the `where <expr>` form the MDL grammar expects.
+						constraint = strings.TrimSuffix(strings.TrimPrefix(groups[0], "["), "]")
 					}
 				}
 				stmt += fmt.Sprintf("\n    where %s", constraint)
@@ -1879,10 +1873,38 @@ func enrichXPathConstraintForDescribe(ctx *ExecContext, entityQN, constraint str
 	if len(enumAttrs) == 0 {
 		return constraint
 	}
-	expr, ok := visitor.ParseXPathConstraint(constraint)
-	if !ok || expr == nil {
+	return enrichXPathGroups(constraint, enumAttrs)
+}
+
+// enrichXPathGroups applies enum enrichment to every top-level predicate group of a
+// stored constraint.
+//
+// Mendix stores sibling groups concatenated (`[a][b][c]`), but the grammar rule
+// matches one group. Enriching the whole string at once parsed only the first and
+// re-rendered just that, silently discarding the rest — describe then showed a
+// materially less restrictive query than the project actually contains
+// (mendixlabs/mxcli#772). ParseXPathConstraint now refuses that partial parse, which
+// stops the data loss on its own; splitting first is what keeps enrichment working
+// for every group rather than only the first.
+func enrichXPathGroups(constraint string, enumAttrs map[string]string) string {
+	groups := visitor.SplitXPathPredicateGroups(constraint)
+	if len(groups) == 0 {
 		return constraint
 	}
-	enriched := enrichXPathExprWithEnums(expr, enumAttrs)
-	return "[" + xpathExprToMDLString(enriched) + "]"
+	out := make([]string, 0, len(groups))
+	for _, g := range groups {
+		out = append(out, enrichXPathGroup(g, enumAttrs))
+	}
+	return strings.Join(out, "")
+}
+
+// enrichXPathGroup enriches one bracket group, returning it unchanged when it does
+// not parse — a group mxcli cannot read is passed through verbatim rather than
+// dropped or guessed at.
+func enrichXPathGroup(group string, enumAttrs map[string]string) string {
+	expr, ok := visitor.ParseXPathConstraint(group)
+	if !ok || expr == nil {
+		return group
+	}
+	return "[" + xpathExprToMDLString(enrichXPathExprWithEnums(expr, enumAttrs)) + "]"
 }

mdl/executor/xpath_enrich_772_test.go

@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestEnrichXPathGroups_KeepsEveryGroup is the regression test for
+// mendixlabs/mxcli#772. The stored constraint holds three sibling predicate groups;
+// enriching the whole string at once parsed only the first, re-rendered just that,
+// and dropped the other two — so `describe` showed a materially less restrictive
+// query than the project contains, with no warning.
+func TestEnrichXPathGroups_KeepsEveryGroup(t *testing.T) {
+	enumAttrs := map[string]string{"Status": "Reminders.TaskStatus"}
+
+	// Exactly the shape from the issue: a group with a nested bracket, then two
+	// flat ones, newline-separated.
+	const stored = "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]\n" +
+		"[Status != 'Completed']\n[CompletionDate = empty]"
+
+	got := enrichXPathGroups(stored, enumAttrs)
+
+	for _, want := range []string{
+		"Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]",
+		"CompletionDate = empty",
+	} {
+		if !strings.Contains(got, want) {
+			t.Errorf("enrichXPathGroups dropped %q\ngot: %s", want, got)
+		}
+	}
+	// The enum comparison must be enriched to a qualified value — in the *second*
+	// group, which is the one the old code never reached.
+	if !strings.Contains(got, "Status != Reminders.TaskStatus.Completed") {
+		t.Errorf("enum enrichment not applied to a later group\ngot: %s", got)
+	}
+	if strings.Contains(got, "'Completed'") {
+		t.Errorf("enum literal left unenriched\ngot: %s", got)
+	}
+}
+
+// TestEnrichXPathGroups_PassesThroughUnparseable: a constraint mxcli cannot split
+// into groups is returned untouched rather than mangled or dropped.
+func TestEnrichXPathGroups_PassesThroughUnparseable(t *testing.T) {
+	enumAttrs := map[string]string{"Status": "Mod.S"}
+	for _, in := range []string{
+		"Status = 'Open'",     // no brackets
+		"[Status = 'Open'",    // unbalanced
+		"[a = 1] and [b = 2]", // content between groups
+	} {
+		if got := enrichXPathGroups(in, enumAttrs); got != in {
+			t.Errorf("enrichXPathGroups(%q) = %q, want it returned verbatim", in, got)
+		}
+	}
+}
+
+// TestEnrichXPathGroups_SingleGroupStillEnriched guards the fix from regressing the
+// original single-group behaviour.
+func TestEnrichXPathGroups_SingleGroupStillEnriched(t *testing.T) {
+	got := enrichXPathGroups("[Status = 'Open']", map[string]string{"Status": "Mod.S"})
+	if got != "[Status = Mod.S.Open]" {
+		t.Errorf("enrichXPathGroups = %q, want %q", got, "[Status = Mod.S.Open]")
+	}
+}

mdl/visitor/visitor_xpath_public.go

@@ -12,6 +12,14 @@ import (
 // [ ] brackets stored by Mendix in the XPathConstraint BSON field — and returns the
 // AST expression. Returns (nil, false) if the input cannot be parsed (e.g. empty,
 // malformed, or not starting with '[').
+//
+// The rule matches a SINGLE bracket group. Mendix stores sibling groups
+// concatenated — `[a][b][c]` — and with the error listeners removed ANTLR happily
+// parsed the first and left the rest on the stream, returning true. Callers read
+// that as "fully parsed" and re-rendered only what came back, silently dropping
+// every later group (mendixlabs/mxcli#772). A partial parse is therefore reported
+// as a failure so callers fall back to the untouched string; use
+// SplitXPathPredicateGroups to handle each group in turn.
 func ParseXPathConstraint(input string) (ast.Expression, bool) {
 	if input == "" {
 		return nil, false
@@ -32,5 +40,9 @@ func ParseXPathConstraint(input string) (ast.Expression, bool) {
 	if xpathExpr == nil {
 		return nil, false
 	}
+	// Anything left on the stream means the rule consumed only a prefix.
+	if stream.LA(1) != antlr.TokenEOF {
+		return nil, false
+	}
 	return buildXPathExpr(xpathExpr), true
 }

mdl/visitor/xpath_groups.go

@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package visitor
+
+import "strings"
+
+// SplitXPathPredicateGroups splits a stored XPathConstraint into its top-level
+// predicate groups, each returned with its enclosing brackets. Mendix concatenates
+// sibling groups — `[a][b][c]`, often separated by newlines — and the whole string
+// is what the BSON field holds.
+//
+// Splitting on "][" is not enough: a group may nest brackets of its own
+// (`[Mod.Assoc/Mod.Entity[EndDate = $Limit]]`), and a string literal may contain a
+// bracket (`[Name = 'a]b']`). Both appear in real projects, and mishandling either
+// silently changes the meaning of a query (mendixlabs/mxcli#772). This tracks
+// nesting depth and quoting instead.
+//
+// Returns nil when the input is not a well-formed sequence of bracket groups —
+// unbalanced, empty, or with content outside the brackets — so callers can fall
+// back to using the string verbatim rather than emit something they invented.
+func SplitXPathPredicateGroups(constraint string) []string {
+	s := strings.TrimSpace(constraint)
+	if s == "" || !strings.HasPrefix(s, "[") {
+		return nil
+	}
+
+	var groups []string
+	var depth int
+	var start int
+	var inQuote bool
+
+	for i := 0; i < len(s); i++ {
+		c := s[i]
+		if inQuote {
+			// Mendix escapes a quote inside a literal by doubling it.
+			if c == '\'' {
+				if i+1 < len(s) && s[i+1] == '\'' {
+					i++
+					continue
+				}
+				inQuote = false
+			}
+			continue
+		}
+		switch c {
+		case '\'':
+			inQuote = true
+		case '[':
+			if depth == 0 {
+				// Anything between groups must be whitespace only.
+				if strings.TrimSpace(s[start:i]) != "" {
+					return nil
+				}
+				start = i
+			}
+			depth++
+		case ']':
+			depth--
+			if depth < 0 {
+				return nil
+			}
+			if depth == 0 {
+				groups = append(groups, s[start:i+1])
+				start = i + 1
+			}
+		}
+	}
+
+	if depth != 0 || inQuote {
+		return nil
+	}
+	if strings.TrimSpace(s[start:]) != "" {
+		return nil
+	}
+	if len(groups) == 0 {
+		return nil
+	}
+	return groups
+}