fix(widgets): stop seeding placeholder rows into object lists

CHANGELOG.md

@@ -17,6 +17,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **A Barcode Scanner authored by mxcli failed the build with CE0463** (mendixlabs/mxcli#1161) — `barcodescanner bsCode (datasource: Module.Entity.Code)` was accepted by `mxcli check`, accepted by `mx check`, and then rejected by headless `mxbuild` with `[CE0463] The definition of this widget has changed`, once per instance. The app would not deploy, and the only documented repair was Studio Pro's right-click → "Update widget".
+
+  mxcli was seeding one placeholder row into the widget's `barcodeFormats` object list. The trigger is a property the author never mentions: Barcode Scanner 2.5.0 declares `<property key="barcodeFormats" type="object" isList="true">` with **no** `required` attribute, and the widget XML schema defaults an absent `required` to true (`Required: p.Required != "false"`), so the list arrived at the builder marked required. `ensureRequiredObjectLists` then auto-populated it, skipping only lists whose nested properties were Attribute/Expression/TextTemplate/Widgets/DataSource — and `barcodeFormat` is an Enumeration, so nothing skipped it. Studio Pro leaves that list empty; the extra row is the whole difference between a model that deploys and one that does not. Diffed against a Studio-Pro-repaired copy of the same page, the seeded row was the **only** structural delta in the widget's subtree, every scalar property byte-identical.
+
+  The seeding is removed rather than narrowed, because measured across the shipped widget templates it reached exactly two properties and helped neither: `barcodescanner barcodeFormats` (above) and `htmlelement events` (optional, nested Action/Boolean/Enumeration), where it wrote a phantom event handler Studio Pro never writes. The MCP write path has always had this method as a no-op, so the two engines now agree instead of the model depending on which one authored it.
+
+  This does **not** touch the sibling #891 rule: an object-list item the author DID write still gets its required TextTemplate filled from the widget's shipped translations (`buildObjectListItemBSON`), and an absent `required` in widget XML still means required. Field run (macOS, Mendix 11.14.0, Barcode Scanner 2.5.0): three Barcode Scanner instances across two pages, `mxbuild --target=deploy` at 3 × CE0463 / BUILD FAILED before and 0 × CE0463 / BUILD SUCCEEDED after, from a model authored entirely by mxcli with no Studio Pro intervention.
+
 - **A page's image-collection reference passed `mxcli check --references` and failed the build** (mendixlabs/mxcli#1149) — `staticimage imgAll (Image: 'Atlas_UI_Resources.Atlas_Icons.checkbox_checked')` in a Selection helper's custom state checked clean, exec'd cleanly and then came back as `[error] [CE1613] "The selected image … no longer exists."`, once per state. The report asks for syntax, but the syntax landed with #1057 — describe emits the three `staticimage` lines and re-running the description reports `Unchanged page`, measured on a blank 11.14.0 project. What was missing is that nothing resolved the name #1057 had made writable.
 
   Two holes, and fixing either alone leaves the reported script unchecked. The image reference was collected by **widget type** (`if w.Type == "image"`), so the pluggable widget was resolved and the two widgets #1057 gave the same property — `staticimage`'s `Image` and `dynamicimage`'s `DefaultImage` — were not; it is a table now, so adding a widget that names an image means adding a row. And a page's widgets live in **two** AST fields: `Widgets` is the bare body, while `placeholder <Name> { … }` content is held apart in `Placeholders` (#532). All three page validators walked the first alone, so **every** reference inside a placeholder block — microflow, nanoflow, page, snippet, entity, image — was validated by nothing. Measured, the same button in the two positions: inside `placeholder Main` → `✓ All references valid`; in the bare body → `microflow not found`. That is the shape mxcli's own skills, examples and DESCRIBE output write, so it was the common case rather than an edge one, and it is the third copy of one walk — `validateIconRefs` (#1008) and `forEachWidget` had each grown the placeholder arm separately — so the roots are now collected once.

mdl/backend/widgetobj/builder.go

@@ -737,9 +737,38 @@ func (ob *Builder) PrimitiveValues() map[string]string {
 // Object list defaults
 // ---------------------------------------------------------------------------
 
-func (ob *Builder) EnsureRequiredObjectLists() {
-	ob.object = ensureRequiredObjectLists(ob.object, ob.propertyTypeIDs)
-}
+// EnsureRequiredObjectLists is intentionally a no-op: mxcli does not seed a
+// placeholder row into an object list the author never wrote.
+//
+// It used to auto-populate any object list whose nested properties were all
+// "simple" (nothing Attribute/Expression/TextTemplate/Widgets/DataSource).
+// Studio Pro leaves such a list EMPTY, so the seeded row made the stored
+// instance disagree with the installed .mpk — CE0463, "the definition of this
+// widget has changed".
+//
+// Measured against the 31 shipped widget templates, the old heuristic fired on
+// exactly two properties and helped neither:
+//
+//	barcodescanner  barcodeFormats  required, nested {Enumeration}
+//	                -> seeded one row carrying the enum default AZTEC; three
+//	                   Barcode Scanners on two pages each raised CE0463 and the
+//	                   app would not deploy until Studio Pro's "Update widget"
+//	                   deleted exactly that row.
+//	htmlelement     events          optional, nested {Action,Boolean,Enumeration}
+//	                -> seeded a phantom event handler Studio Pro never writes.
+//
+// The MCP write path (mdl/backend/mcp.(*mcpWidgetBuilder)) has always had this
+// as a no-op, so removing the seeding also makes the two engines agree rather
+// than making the model depend on which one authored it.
+//
+// This is NOT the sibling fix for issue #891. That one fills an *authored*
+// object-list item's required TextTemplate with the widget's shipped
+// translations, lives in buildObjectListItemBSON, and still applies — an absent
+// `required` attribute in widget XML still means required (mpk.go:
+// `Required: p.Required != "false"`). Nothing here changes that.
+//
+// The method is kept so backend.WidgetBuilder keeps its shape.
+func (ob *Builder) EnsureRequiredObjectLists() {}
 
 // ---------------------------------------------------------------------------
 // Property visibility (#574)
@@ -1410,95 +1439,6 @@ func createDefaultClientTemplateBSON(text string) bson.D {
 // Default object lists
 // ---------------------------------------------------------------------------
 
-func ensureRequiredObjectLists(obj bson.D, propertyTypeIDs map[string]pages.PropertyTypeIDEntry) bson.D {
-	// Sort keys for deterministic BSON output.
-	keys := make([]string, 0, len(propertyTypeIDs))
-	for k := range propertyTypeIDs {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-
-	for _, propKey := range keys {
-		entry := propertyTypeIDs[propKey]
-		if entry.ObjectTypeID == "" || len(entry.NestedPropertyIDs) == 0 {
-			continue
-		}
-		if !entry.Required {
-			hasNestedDS := false
-			for _, nested := range entry.NestedPropertyIDs {
-				if nested.ValueType == "DataSource" {
-					hasNestedDS = true
-					break
-				}
-			}
-			if hasNestedDS {
-				continue
-			}
-		}
-		// Skip auto-populate when any nested property has a complex ValueType
-		// (Attribute / Expression / TextTemplate / Widgets / DataSource).
-		// Complex types have no sensible empty default — Studio Pro flags an
-		// auto-generated entry with empty Expression/Attribute as CE0463/CE0566.
-		// This also avoids over-populating mode-dependent required lists such as
-		// Combobox optionsSourceStaticDataSource (only used when source=static).
-		hasComplexNested := false
-		for _, nested := range entry.NestedPropertyIDs {
-			switch nested.ValueType {
-			case "Attribute", "Expression", "TextTemplate", "Widgets", "DataSource":
-				hasComplexNested = true
-			}
-			if hasComplexNested {
-				break
-			}
-		}
-		if hasComplexNested {
-			continue
-		}
-		obj = updateWidgetPropertyValue(obj, propertyTypeIDs, propKey, func(val bson.D) bson.D {
-			for _, elem := range val {
-				if elem.Key == "Objects" {
-					if arr, ok := elem.Value.(bson.A); ok && len(arr) <= 1 {
-						defaultObj := createDefaultWidgetObject(entry.ObjectTypeID, entry.NestedPropertyIDs)
-						newArr := bson.A{int32(2), defaultObj}
-						result := make(bson.D, 0, len(val))
-						for _, e := range val {
-							if e.Key == "Objects" {
-								result = append(result, bson.E{Key: "Objects", Value: newArr})
-							} else {
-								result = append(result, e)
-							}
-						}
-						return result
-					}
-				}
-			}
-			return val
-		})
-	}
-	return obj
-}
-
-func createDefaultWidgetObject(objectTypeID string, nestedProps map[string]pages.PropertyTypeIDEntry) bson.D {
-	propsArr := bson.A{int32(2)}
-	// Sort keys for deterministic BSON output.
-	nestedKeys := make([]string, 0, len(nestedProps))
-	for k := range nestedProps {
-		nestedKeys = append(nestedKeys, k)
-	}
-	sort.Strings(nestedKeys)
-	for _, k := range nestedKeys {
-		entry := nestedProps[k]
-		prop := createDefaultWidgetProperty(entry)
-		propsArr = append(propsArr, prop)
-	}
-	return bson.D{
-		{Key: "$ID", Value: types.UUIDToBlob(types.GenerateID())},
-		{Key: "$Type", Value: "CustomWidgets$WidgetObject"},
-		{Key: "TypePointer", Value: types.UUIDToBlob(objectTypeID)},
-		{Key: "Properties", Value: propsArr},
-	}
-}
-
 func createDefaultWidgetProperty(entry pages.PropertyTypeIDEntry) bson.D {
 	return bson.D{
 		{Key: "$ID", Value: types.UUIDToBlob(types.GenerateID())},

mdl/backend/widgetobj/objectlist_no_autoseed_test.go

@@ -0,0 +1,168 @@
+// SPDX-License-Identifier: Apache-2.0
+
+// An object list the author never wrote must stay EMPTY.
+//
+// mxcli used to seed one placeholder row into any object list whose nested
+// properties were all "simple" (nothing Attribute/Expression/TextTemplate/
+// Widgets/DataSource). Studio Pro leaves such a list empty, so the seeded row
+// made the stored instance disagree with the installed .mpk and headless
+// mxbuild rejected the page with CE0463 "the definition of this widget has
+// changed" — while `mxcli check` and `mx check` both reported zero errors.
+//
+// Measured against the shipped widget templates, the seeding reached exactly
+// two properties and helped neither:
+//
+//	barcodescanner  barcodeFormats  required, nested {Enumeration}
+//	htmlelement     events          optional, nested {Action,Boolean,Enumeration}
+//
+// Both cases are covered below. This is the inverse of the #891 rule in
+// objectlist_required_texttemplate_test.go: an object-list item the author DID
+// write still gets its required TextTemplate filled — that path is unchanged.
+package widgetobj
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/types"
+	"github.com/mendixlabs/mxcli/sdk/pages"
+	"go.mongodb.org/mongo-driver/bson"
+)
+
+// emptyObjectListWidget builds a widget object holding one object-list property
+// with no rows — the shape a template has before the author writes anything.
+func emptyObjectListWidget(entry pages.PropertyTypeIDEntry) bson.D {
+	return bson.D{
+		{Key: "$ID", Value: types.UUIDToBlob(types.GenerateID())},
+		{Key: "$Type", Value: "CustomWidgets$WidgetObject"},
+		{Key: "TypePointer", Value: types.UUIDToBlob("00000000000000000000000000000000")},
+		{Key: "Properties", Value: bson.A{
+			int32(2),
+			bson.D{
+				{Key: "$ID", Value: types.UUIDToBlob(types.GenerateID())},
+				{Key: "$Type", Value: "CustomWidgets$WidgetProperty"},
+				{Key: "TypePointer", Value: types.UUIDToBlob(entry.PropertyTypeID)},
+				{Key: "Value", Value: bson.D{
+					{Key: "$ID", Value: types.UUIDToBlob(types.GenerateID())},
+					{Key: "$Type", Value: "CustomWidgets$WidgetValue"},
+					{Key: "Objects", Value: bson.A{int32(2)}},
+					{Key: "PrimitiveValue", Value: ""},
+					{Key: "TypePointer", Value: types.UUIDToBlob(entry.ValueTypeID)},
+					{Key: "Widgets", Value: bson.A{int32(2)}},
+				}},
+			},
+		}},
+	}
+}
+
+// countWidgetObjects reports how many CustomWidgets$WidgetObject nodes sit
+// below the root — i.e. how many object-list rows exist.
+func countWidgetObjects(v any) int {
+	n := 0
+	switch node := v.(type) {
+	case bson.D:
+		for _, e := range node {
+			if e.Key == "$Type" && e.Value == "CustomWidgets$WidgetObject" {
+				n++
+			}
+		}
+		for _, e := range node {
+			n += countWidgetObjects(e.Value)
+		}
+	case bson.A:
+		for _, e := range node {
+			n += countWidgetObjects(e)
+		}
+	}
+	return n
+}
+
+// barcodeFormatsEntry mirrors Barcode Scanner 2.5.0's `barcodeFormats`: the XML
+// omits `required`, and the schema default for an absent attribute is true
+// (mpk.go: `Required: p.Required != "false"`), so it arrives here Required.
+func barcodeFormatsEntry() pages.PropertyTypeIDEntry {
+	return pages.PropertyTypeIDEntry{
+		PropertyTypeID: "00000000000000000000000000000011",
+		ValueTypeID:    "00000000000000000000000000000012",
+		ObjectTypeID:   "00000000000000000000000000000013",
+		Required:       true,
+		NestedKeyOrder: []string{"barcodeFormat"},
+		NestedPropertyIDs: map[string]pages.PropertyTypeIDEntry{
+			"barcodeFormat": {
+				PropertyTypeID: "00000000000000000000000000000014",
+				ValueTypeID:    "00000000000000000000000000000015",
+				ValueType:      "Enumeration",
+				DefaultValue:   "AZTEC",
+			},
+		},
+	}
+}
+
+// htmlElementEventsEntry mirrors HTML element's `events`: optional, and with no
+// nested DataSource it fell past the old not-required guard too.
+func htmlElementEventsEntry() pages.PropertyTypeIDEntry {
+	return pages.PropertyTypeIDEntry{
+		PropertyTypeID: "00000000000000000000000000000021",
+		ValueTypeID:    "00000000000000000000000000000022",
+		ObjectTypeID:   "00000000000000000000000000000023",
+		Required:       false,
+		NestedKeyOrder: []string{"eventName", "eventStopPropagation", "eventAction"},
+		NestedPropertyIDs: map[string]pages.PropertyTypeIDEntry{
+			"eventName": {
+				PropertyTypeID: "00000000000000000000000000000024",
+				ValueTypeID:    "00000000000000000000000000000025",
+				ValueType:      "Enumeration",
+				DefaultValue:   "onClick",
+			},
+			"eventStopPropagation": {
+				PropertyTypeID: "00000000000000000000000000000026",
+				ValueTypeID:    "00000000000000000000000000000027",
+				ValueType:      "Boolean",
+				DefaultValue:   "true",
+			},
+			"eventAction": {
+				PropertyTypeID: "00000000000000000000000000000028",
+				ValueTypeID:    "00000000000000000000000000000029",
+				ValueType:      "Action",
+			},
+		},
+	}
+}
+
+// Goes through the Builder method the write pipeline actually calls
+// (mdl/backend/mutation.go and mdl/executor/widget_engine.go), not a helper, so
+// reinstating the seeding fails this test.
+func TestEnsureRequiredObjectLists_LeavesUnwrittenListsEmpty(t *testing.T) {
+	cases := []struct {
+		name        string
+		propertyKey string
+		entry       pages.PropertyTypeIDEntry
+	}{
+		{"barcodescanner barcodeFormats (required, nested Enumeration)", "barcodeFormats", barcodeFormatsEntry()},
+		{"htmlelement events (optional, nested Action/Boolean/Enumeration)", "events", htmlElementEventsEntry()},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			root := emptyObjectListWidget(tc.entry)
+			// The root itself is one WidgetObject; any extra is a seeded row.
+			if before := countWidgetObjects(root); before != 1 {
+				t.Fatalf("fixture is wrong: expected only the root WidgetObject, got %d", before)
+			}
+
+			ob := New(
+				"com.example.widget.Test",
+				bson.D{},
+				root,
+				map[string]pages.PropertyTypeIDEntry{tc.propertyKey: tc.entry},
+				"00000000000000000000000000000013",
+				nil,
+			)
+			ob.EnsureRequiredObjectLists()
+
+			if got := countWidgetObjects(ob.object); got != 1 {
+				t.Errorf("object list %q was auto-populated: %d WidgetObject nodes, want 1 (the root only). "+
+					"A row Studio Pro does not write is CE0463 at mxbuild time.", tc.propertyKey, got)
+			}
+		})
+	}
+}