fix: address code review issues for pluggable widget engine

- Remove dead DefaultSelection field from WidgetDefinition struct
- Add warning logs for silent errors in initPluggableEngine and LoadUserDefinitions
- Refactor TestOpSelection to test real opSelection function, add TestOpSelectionEmptyValue
- Use return instead of break in buildWidgetV3 default case for clearer control flow
- Extract association attribute correctly in ComboBox DESCRIBE (extractCustomWidgetPropertyAssociation)
- Update MDL examples to reflect CE1613 fix status

Author: engalar

mdl-examples/doctype-tests/17-custom-widget-examples.mdl

@@ -8,11 +8,11 @@
 --   1. GALLERY basic       — DATABASE datasource + TEMPLATE (PASS)
 --   2. GALLERY with FILTER — TEXTFILTER in FILTER block    (KNOWN BUG: CE0463)
 --   3. COMBOBOX enum       — Enum attribute                (KNOWN BUG: CE1613)
---   4. COMBOBOX association — DataSource + CaptionAttribute (KNOWN BUG: CE1613)
+--   4. COMBOBOX association — DataSource + CaptionAttribute (FIXED: Issue #21)
 --
 -- Known engine bugs (do not affect MDL syntax correctness):
 --   - CE0463 on TEXTFILTER: embedded template property count mismatch
---   - CE1613 on COMBOBOX: enum/association attribute written as association pointer
+--   - CE1613 on COMBOBOX enum: enum attribute written as association pointer (pending)
 --
 -- ============================================================================
 
@@ -166,12 +166,12 @@ CREATE PAGE MyFirstModule.P_ComboBox_Enum
 
 -- ============================================================================
 -- Test 4: COMBOBOX association mode — DataSource + CaptionAttribute
--- Result: CE1613 (association attribute lookup fails)
+-- Result: FIXED (Issue #21) — Attribute now correctly reads back as association name
 -- ============================================================================
 
 /**
  * Task edit form with a COMBOBOX for selecting Category via association.
- * KNOWN BUG: CE1613 — association attribute incorrectly resolved.
+ * FIXED (Issue #21): Attribute now correctly roundtrips as association name.
  *
  * @param $Task The task to edit
  */

mdl/executor/cmd_pages_builder.go

@@ -4,6 +4,7 @@ package executor
 
 import (
 	"fmt"
+	"os"
 	"strings"
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
@@ -53,7 +54,7 @@ func (pb *pageBuilder) initPluggableEngine() {
 	}
 	registry, err := NewWidgetRegistry()
 	if err != nil {
-		// Non-fatal: engine won't be available, fall through to error
+		fmt.Fprintf(os.Stderr, "warning: pluggable widget registry init failed: %v\n", err)
 		return
 	}
 	if pb.reader != nil {

mdl/executor/cmd_pages_builder_v3.go

@@ -328,8 +328,7 @@ func (pb *pageBuilder) buildWidgetV3(w *ast.WidgetV3) (pages.Widget, error) {
 		pb.initPluggableEngine()
 		if pb.widgetRegistry != nil {
 			if def, ok := pb.widgetRegistry.Get(strings.ToUpper(w.Type)); ok {
-				widget, err = pb.pluggableEngine.Build(def, w)
-				break
+				return pb.pluggableEngine.Build(def, w)
 			}
 		}
 		return nil, fmt.Errorf("unsupported V3 widget type: %s", w.Type)

mdl/executor/cmd_pages_describe_parse.go

@@ -142,10 +142,13 @@ func (e *Executor) parseRawWidget(w map[string]any) []rawWidget {
 		widget.Caption = e.extractLabelText(w)
 		widget.Content = e.extractCustomWidgetAttribute(w)
 		widget.RenderMode = e.extractCustomWidgetType(w) // Store widget type in RenderMode
-		// For ComboBox, extract datasource and caption attribute for association mode
+		// For ComboBox, extract datasource and association attribute for association mode.
+		// In association mode the Attribute binding is stored as EntityRef (not AttributeRef),
+		// so we must use extractCustomWidgetPropertyAssociation instead of the generic scan.
 		if widget.RenderMode == "COMBOBOX" {
 			widget.DataSource = e.extractComboBoxDataSource(w)
 			if widget.DataSource != nil {
+				widget.Content = e.extractCustomWidgetPropertyAssociation(w, "attributeAssociation")
 				widget.CaptionAttribute = e.extractCustomWidgetPropertyAttributeRef(w, "optionsSourceAssociationCaptionAttribute")
 			}
 		}

mdl/executor/cmd_pages_describe_pluggable.go

@@ -902,6 +902,76 @@ func (e *Executor) extractCustomWidgetPropertyAttributeRef(w map[string]any, pro
 	return ""
 }
 
+// extractCustomWidgetPropertyAssociation extracts an association name from a named
+// CustomWidget property that was written by opAssociation (setAssociationRef).
+// The association is stored as EntityRef.Steps[1].Association (qualified path);
+// this function returns only the short name (last segment after the final dot).
+//
+// This is the symmetric counterpart of extractCustomWidgetPropertyAttributeRef,
+// handling the EntityRef storage format instead of AttributeRef.
+func (e *Executor) extractCustomWidgetPropertyAssociation(w map[string]any, propertyKey string) string {
+	obj, ok := w["Object"].(map[string]any)
+	if !ok {
+		return ""
+	}
+
+	// Build property key map from Type.ObjectType.PropertyTypes
+	propTypeKeyMap := make(map[string]string)
+	if widgetType, ok := w["Type"].(map[string]any); ok {
+		var propTypes []any
+		if objType, ok := widgetType["ObjectType"].(map[string]any); ok {
+			propTypes = getBsonArrayElements(objType["PropertyTypes"])
+		}
+		for _, pt := range propTypes {
+			ptMap, ok := pt.(map[string]any)
+			if !ok {
+				continue
+			}
+			key := extractString(ptMap["PropertyKey"])
+			if key == "" {
+				continue
+			}
+			id := extractBinaryID(ptMap["$ID"])
+			if id != "" {
+				propTypeKeyMap[id] = key
+			}
+		}
+	}
+
+	// Find the named property and extract EntityRef.Steps[1].Association
+	props := getBsonArrayElements(obj["Properties"])
+	for _, prop := range props {
+		propMap, ok := prop.(map[string]any)
+		if !ok {
+			continue
+		}
+		typePointerID := extractBinaryID(propMap["TypePointer"])
+		if propTypeKeyMap[typePointerID] != propertyKey {
+			continue
+		}
+		value, ok := propMap["Value"].(map[string]any)
+		if !ok {
+			continue
+		}
+		entityRef, ok := value["EntityRef"].(map[string]any)
+		if !ok || entityRef == nil {
+			return ""
+		}
+		steps := getBsonArrayElements(entityRef["Steps"])
+		// Steps layout: [int32(2), step0, step1, ...] — first element is version marker
+		for _, step := range steps {
+			stepMap, ok := step.(map[string]any)
+			if !ok {
+				continue
+			}
+			if assoc := extractString(stepMap["Association"]); assoc != "" {
+				return shortAttributeName(assoc)
+			}
+		}
+	}
+	return ""
+}
+
 // extractCustomWidgetPropertyString extracts a string property value from a CustomWidget.
 func (e *Executor) extractCustomWidgetPropertyString(w map[string]any, propertyKey string) string {
 	obj, ok := w["Object"].(map[string]any)

mdl/executor/widget_engine.go

@@ -25,7 +25,6 @@ type WidgetDefinition struct {
 	MDLName          string             `json:"mdlName"`
 	TemplateFile     string             `json:"templateFile"`
 	DefaultEditable  string             `json:"defaultEditable"`
-	DefaultSelection string             `json:"defaultSelection,omitempty"`
 	PropertyMappings []PropertyMapping  `json:"propertyMappings,omitempty"`
 	ChildSlots       []ChildSlotMapping `json:"childSlots,omitempty"`
 	Modes            []WidgetMode       `json:"modes,omitempty"`

mdl/executor/widget_engine_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
 	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/mpr"
 	"github.com/mendixlabs/mxcli/sdk/pages"
 	"go.mongodb.org/mongo-driver/bson"
 )
@@ -17,8 +18,7 @@ func TestWidgetDefinitionJSONRoundTrip(t *testing.T) {
 		WidgetID:         "com.mendix.widget.web.combobox.Combobox",
 		MDLName:          "COMBOBOX",
 		TemplateFile:     "combobox.json",
-		DefaultEditable:  "Always",
-		DefaultSelection: "Single",
+		DefaultEditable: "Always",
 		PropertyMappings: []PropertyMapping{
 			{PropertyKey: "attributeEnumeration", Source: "Attribute", Operation: "attribute"},
 			{PropertyKey: "optionsSourceType", Value: "enumeration", Operation: "primitive"},
@@ -62,9 +62,6 @@ func TestWidgetDefinitionJSONRoundTrip(t *testing.T) {
 	if decoded.DefaultEditable != original.DefaultEditable {
 		t.Errorf("DefaultEditable: got %q, want %q", decoded.DefaultEditable, original.DefaultEditable)
 	}
-	if decoded.DefaultSelection != original.DefaultSelection {
-		t.Errorf("DefaultSelection: got %q, want %q", decoded.DefaultSelection, original.DefaultSelection)
-	}
 
 	// Verify property mappings
 	if len(decoded.PropertyMappings) != len(original.PropertyMappings) {
@@ -119,10 +116,6 @@ func TestWidgetDefinitionJSONOmitsEmptyOptionalFields(t *testing.T) {
 		t.Fatalf("failed to unmarshal to map: %v", err)
 	}
 
-	// defaultSelection should be omitted when empty
-	if _, exists := raw["defaultSelection"]; exists {
-		t.Error("defaultSelection should be omitted when empty")
-	}
 }
 
 func TestOperationRegistryLookupFound(t *testing.T) {
@@ -517,29 +510,49 @@ func TestSetChildWidgets(t *testing.T) {
 }
 
 func TestOpSelection(t *testing.T) {
-	// Test opSelection directly on a Value bson.D (same level as setChildWidgets test)
-	// opSelection calls updateWidgetPropertyValue which needs TypePointer matching.
-	// Instead, test the inner logic: the Selection field update in a Value document.
-	val := bson.D{
-		{Key: "TypePointer", Value: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}},
-		{Key: "PrimitiveValue", Value: ""},
-		{Key: "Selection", Value: "None"},
+	// Call the real opSelection function with a properly structured widget BSON.
+	typePointerBytes := []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
+	typePointerUUID := mpr.BlobToUUID(typePointerBytes)
+
+	widgetObj := bson.D{
+		{Key: "Properties", Value: bson.A{
+			int32(2), // version marker
+			bson.D{
+				{Key: "TypePointer", Value: typePointerBytes},
+				{Key: "Value", Value: bson.D{
+					{Key: "PrimitiveValue", Value: ""},
+					{Key: "Selection", Value: "None"},
+				}},
+			},
+		}},
+	}
+
+	propTypeIDs := map[string]pages.PropertyTypeIDEntry{
+		"selectionType": {PropertyTypeID: typePointerUUID},
 	}
 
-	// Simulate what opSelection's inner function does
 	ctx := &BuildContext{PrimitiveVal: "Multi"}
-	result := make(bson.D, 0, len(val))
-	for _, elem := range val {
-		if elem.Key == "Selection" {
-			result = append(result, bson.E{Key: "Selection", Value: ctx.PrimitiveVal})
-		} else {
-			result = append(result, elem)
+	result := opSelection(widgetObj, propTypeIDs, "selectionType", ctx)
+
+	// Extract the updated Value from Properties
+	var props bson.A
+	for _, elem := range result {
+		if elem.Key == "Properties" {
+			props = elem.Value.(bson.A)
+		}
+	}
+	prop := props[1].(bson.D) // skip version marker at index 0
+	var val bson.D
+	for _, elem := range prop {
+		if elem.Key == "Value" {
+			val = elem.Value.(bson.D)
 		}
 	}
 
-	// Verify Selection was updated
-	for _, elem := range result {
+	selectionFound := false
+	for _, elem := range val {
 		if elem.Key == "Selection" {
+			selectionFound = true
 			if elem.Value != "Multi" {
 				t.Errorf("Selection: got %q, want %q", elem.Value, "Multi")
 			}
@@ -550,4 +563,20 @@ func TestOpSelection(t *testing.T) {
 			}
 		}
 	}
+	if !selectionFound {
+		t.Error("Selection field not found in result")
+	}
+}
+
+func TestOpSelectionEmptyValue(t *testing.T) {
+	widgetObj := bson.D{
+		{Key: "Properties", Value: bson.A{int32(2)}},
+	}
+	ctx := &BuildContext{PrimitiveVal: ""}
+	result := opSelection(widgetObj, nil, "any", ctx)
+
+	// With empty PrimitiveVal, opSelection returns obj unchanged
+	if len(result) != len(widgetObj) {
+		t.Errorf("expected unchanged obj, got different length: %d vs %d", len(result), len(widgetObj))
+	}
 }

mdl/executor/widget_registry.go

@@ -88,6 +88,8 @@ func (r *WidgetRegistry) LoadUserDefinitions(projectPath string) error {
 		if err := r.loadDefinitionsFromDir(globalDir); err != nil {
 			return fmt.Errorf("global widgets: %w", err)
 		}
+	} else {
+		fmt.Fprintf(os.Stderr, "warning: cannot determine home directory for user widget definitions: %v\n", err)
 	}
 
 	// 2. Project: <projectDir>/.mxcli/widgets/*.def.json (overrides global)

sdk/widgets/definitions/gallery.def.json

@@ -3,7 +3,6 @@
   "mdlName": "GALLERY",
   "templateFile": "gallery.json",
   "defaultEditable": "Always",
-  "defaultSelection": "Single",
   "propertyMappings": [
     {"propertyKey": "datasource", "source": "DataSource", "operation": "datasource"},
     {"propertyKey": "itemSelection", "source": "Selection", "operation": "selection", "default": "Single"}