fix(mpr): hoist $ID first in legacy unit writers — fixes 11.12 nightly mx-check load failures

The 11.12 nightly's integration tests failed to load projects on the legacy
engine for several doctypes (01-domain-model, 10-odata, 14-project-settings,
22-published-rest) with:

  System.InvalidOperationException: Expected '$ID' as the first property of a
  storage object, but got 'X'   (at StreamingBsonUnitReader)

The offending first-key varied run to run (ThemeModuleOrder, EnableRspackBundler,
Oql, MarkAsUsed, PopupResizable, …) — the signature of random Go-map key order.
Several legacy sdk/mpr writers preserve round-trip fidelity by carrying parsed
subtrees as Go maps (ProjectSettings.RawParts, ref-marking/rename/domain-model
`raw`) and marshalling them back; bson.Marshal emits map keys in random order, so
$ID only landed first by luck. Mendix ≤ 11.11 tolerated any order; 11.12 rejects
a non-$ID first key. The earlier bson.D $ID-first fix missed every map-passthrough
marshal site.

Fix: a new bsonutil.HoistStorageID normalizes a value tree so every storage
object leads with $ID/$Type, and wrap every unit-boundary bson.Marshal(doc) in
sdk/mpr via marshalUnitIDFirst (writer_order.go).

Crucially this uses a NON-sorting hoist, not the existing OrderStorageValue:
OrderStorageValue *sorts* the remaining keys, which corrupts template-derived
pluggable-widget/datagrid page trees (mx then aborts with "got 'LabelTemplate'").
HoistStorageID only lifts $ID/$Type to the front and preserves every other key's
order — a no-op on already-correct writers, fixing only the map-passthrough ones.
The page writer and page/workflow mutators are deliberately NOT normalized: their
output is already $ID-first and they embed delicate widget maps a sort would
corrupt (verified: normalizing them regressed 29-datagrid / 33-alter-page).

Verified end-to-end on Mendix 11.12 (scripts/mx-check.sh via the doctype gate):
- FIXED (both engines): 01-domain-model, 10-odata, 14-project-settings,
  22-published-rest
- NO REGRESSION across 04, 06, 07, 07b, 08, 09, 11, 12, 13, 19, 21, 24, 29, 30,
  32, 33
- HoistStorageID unit tests assert $ID-first without reordering other keys

Separately, the nightly's 31/33 WidgetProperty/WidgetValue failures are already
resolved by the numberfilter template fix (4c8d9e1, merged after that run).

Not addressed here (pre-existing, separate root causes): 03-page (LabelTemplate —
on-disk bytes read $ID-first yet mx still objects), 02/02b legacy CE0117
close-page expression, 05-database marketplace-module incompatibility, and the
10.24 OData/OQL example failures.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: ako

.claude/skills/fix-issue.md

@@ -100,6 +100,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
 | `mx check` reports CE6621 "Attribute 'X' has max length '200' which is not the same as max length in the OData service which is 'unlimited'" on every unlimited-length string attribute of a `create external entities from …` import — **only under the default modelsdk engine**; `--engine legacy` is clean | The codec's encoder only emits *dirty* properties for a new element. `attributeTypeToGen` skipped `SetLength` when `Length == 0` (unlimited), so the `Length` property stayed non-dirty and the field was omitted from the BSON entirely. Studio Pro then applies its own UI default of 200, which contradicts the OData service's "unlimited" type. Legacy's `writer_domainmodel.go` always writes `Length: t.Length` (including 0), so it never regressed | `mdl/backend/modelsdk/domainmodel_write.go` → `attributeTypeToGen` (`*domainmodel.StringAttributeType` case) | Always call `g.SetLength(int32(at.Length))` — drop the `if at.Length > 0` guard — so `Length: 0` is emitted explicitly, matching the legacy serializer. General rule for the codec write path: any field whose *zero value is meaningful* (0 = unlimited here) must be Set unconditionally, because an omitted field falls back to Studio Pro's own default, not to zero. Assert on the serialized `NewType` map (not DESCRIBE) — the reader treats a missing Length as unlimited so it hides the damage. Test `TestEntityToGen_ODataRemoteEntitySource` in `external_entity_write_test.go`. Issue #718 |
 | `mxcli oql` against a docker-deployed app returns **0 rows** for tables that clearly have data (false negative — tester concluded "saving is broken") | NOT a database-config problem (that's a red herring — proven: aligning the runtime to Postgres with data still returned 0). The OQL preview servlet (`/dev/preview_execute_oql`) is registered by `com.mendix.basis.livepreview.LivePreviewRegistry` → `AppContainer.addDevelopmentServlet`, which **only mounts the servlet when the JVM system property `mendix.running.locally.by.studiopro=true` is set** (Studio Pro sets it; a deployed `bin/start` PAD doesn't). Without it the endpoint returns HTTP 200 `{"result":-5,"message":"Action not found"}`, which mxcli silently parsed as empty data. (Found by decompiling `com.mendix.mxruntime.jar` / `com.mendix.appcontainer.jar`.) | `cmd/mxcli/docker/templates/docker-compose.yml` (the `command`) + `cmd/mxcli/docker/oql.go` (`oqlDevError`) | Add `-J -Dmendix.running.locally.by.studiopro=true` to the start command (each JVM opt needs its own `-J`), next to the existing `-Dmendix.live-preview=enabled` — this mounts the dev servlets and `mxcli oql` returns live data (verified end-to-end; DTAP mode is NOT required, DB config is irrelevant — the preview runs in-process against the runtime DataStore). Separately, make `oqlDevError` surface `{"result":<0,"message":…}` (not just `{"error":…}`) so the failure isn't swallowed as 0 rows. **Related startup bug found:** the generated `etc/Default` inherits the project's ports, which are `0` when unset → the runtime rejects port 0 and won't start; `patchRuntimePorts` (patch.go) forces `runtime.http { port = 8080 }` / `admin { port = 8090 }`. Verified with a full docker build/run/seed cycle |
 | `GRANT VIEW ON PAGE Module.Page TO Module.Role` prints success but the page's allowed roles appear to stay empty: `mxcli lint` still reports `CE0557`/`MPR007` ("used as home page … but has no allowed roles") across a freshly rebuilt catalog, while entity-access grants on the same roles verify fine (`describe entity` shows them). Reported on **modelsdk** engine (default), Mendix 11.11 | NOT a write bug — the grant **does** persist (`AllowedModuleRoles: [3, "Module.Role"]` on disk; `mx check` = 0 errors; Studio Pro would load clean). The regression was on the **read** path: modelsdk `pageFromGen` didn't populate `Page.AllowedRoles`, and `mxcli lint`'s CE0557 rule counts `len(pg.AllowedRoles)` from `ListPages()` — so it read every page as having zero roles and false-fired. `SHOW ACCESS ON PAGE` / `SHOW SECURITY MATRIX` under-reported the same way. The entity-vs-page asymmetry the reporter saw = different read paths (entity `describe` worked; page `lint` didn't), not different writes | `mdl/backend/modelsdk/page.go` → `pageFromGen` | Already fixed by `88a2d50b1` (issue #722): populate `out.AllowedRoles` from the gen `AllowedRolesQualifiedNames()`, mirroring `microflowFromGen`. Guard for the reporter's exact end-to-end flow: `TestUpdateAllowedRoles_PagePersistsForLintRead` (`mdl/backend/modelsdk/page_roles_test.go`) writes a page role then reopens fresh and asserts the lint read path (`ListPages`→`AllowedRoles`) sees it. **Diagnosis tip**: when a write "reports success but the value stays empty" *only in a read/lint/show command*, dump the raw unit (`mxcli bson dump`) — if the bytes are there, it's a read-adapter (`*FromGen`) gap, not a write bug; confirm by reverting the suspected read commit and re-running | 
+| On **Mendix 11.12**, `mx check` fails to load projects on the **legacy** engine for several doctypes (`01-domain-model`, `10-odata`, `14-project-settings`, `22-published-rest`) with `System.InvalidOperationException: Expected '$ID' as the first property of a storage object, but got 'X'` at `StreamingBsonUnitReader`. The bad first-key varies run-to-run (`ThemeModuleOrder`, `EnableRspackBundler`, `Oql`, `MarkAsUsed`, `PopupResizable`…) — the tell-tale of random map-key order. modelsdk passes; ≤ 11.11 tolerates any order | Several legacy `sdk/mpr` writers preserve round-trip fidelity by carrying parsed subtrees as Go maps (`ProjectSettings.RawParts`, ref-marking `raw`, rename `raw`, domain-model `raw`) and marshalling them back. `bson.Marshal` emits **map keys in random order**, so `$ID` only lands first by luck. 11.12 rejects any storage object whose first key isn't `$ID`. The earlier `$ID`-first fix (bson.D reorder in writer_domainmodel/security) missed every map-passthrough site | `sdk/mpr/writer_settings.go` (+ `writer_refs`, `writer_rename`, `writer_domainmodel`, `writer_odata`, `writer_rest`, `writer_customblob`, `writer_security`, `writer_modules`, `writer_microflow`, … — all unit-boundary marshals) via new `sdk/mpr/writer_order.go` | Wrap every unit-boundary `bson.Marshal(doc)` in `marshalUnitIDFirst(doc)` = `bson.Marshal(bsonutil.HoistStorageID(doc))`. **Use the non-sorting `HoistStorageID`, NOT `OrderStorageValue`**: `OrderStorageValue` *sorts* the non-`$ID` keys, which corrupts template-derived pluggable-widget/datagrid page trees (mx then aborts with `got 'LabelTemplate'`, regressing `29-datagrid`/`33-alter-page`). `HoistStorageID` only lifts `$ID`/`$Type` to the front and preserves every other key's order → no-op on already-correct writers, fixes only the map-passthrough ones. **Do NOT normalize `writer_pages.go` or the page/workflow mutators** — their output is already `$ID`-first and they embed delicate widget maps the hoist would sort-corrupt. Verify on 11.12 with the doctype gate; regression-check `29`/`33` specifically. (Left unfixed: `03-page` LabelTemplate — a separate pre-existing issue where on-disk bytes read `$ID`-first yet mx still objects) |
 | A `NUMBERFILTER` in a DataGrid column passes `mxcli check` but corrupts the `.mpr`: MxBuild / Studio Pro **fails to load** the whole project on **Mendix 11.12** with `System.InvalidOperationException: Type …CustomWidgets.WidgetProperty does not contain a constructor with a parameter of type …CustomWidgets.WidgetValue` at `StreamingBsonUnitReader.ConstructObject`. `TEXTFILTER`/`DATEFILTER`/`DROPDOWNFILTER` in identical grids load fine; ≤ 11.11 loads even the number filter. Isolating it needs dropping widgets one-by-one under `mx check` | The embedded `datagrid-number-filter.json` template authored its 3 placeholder / screen-reader `Forms$ClientTemplate` blocks with **markerless empty arrays** — `"Items": []` and `"Parameters": []`. Every Mendix list serializes with a leading marker int (`Texts$Text.Items`→`[3]`, `ClientTemplate.Parameters`→`[2]`, `Widgets`/`Objects`→`[2]`); a bare `[]` has no marker. 11.12's stricter streaming reader mis-parses the markerless array and mis-associates the following bytes, so a nested `WidgetValue` lands where the reader is constructing a `WidgetProperty`. The template shape is otherwise identical to the working filters — the only diff is the missing markers (the blocks had hand-authored `dd2b3c4d…` placeholder IDs, i.e. not cleanly extracted from Studio Pro) | `sdk/widgets/templates/mendix-11.6/datagrid-number-filter.json` **and** `modelsdk/widgets/templates/mendix-11.6/datagrid-number-filter.json` (both engines embed their own copy) | Add the marker int to every empty `TextTemplate` array: `"Items": []`→`"Items": [3]`, `"Parameters": []`→`"Parameters": [2]`. Rebuild (`go:embed`), re-exec, verify with `scripts/mx-check.sh --version 11.12.0` (0 errors; regression-check 11.9 still clean). Guard: `TestTemplates_NoMarkerlessEmptyArrays` (in both `sdk/widgets` and `modelsdk/widgets`) walks every embedded template and fails on any bare `[]` — a Mendix list must always carry its marker. Repro: `mdl-examples/bug-tests/datagrid-numberfilter-array-marker.mdl`. **Diagnosis tip**: when a widget "passes check, fails 11.12 load" with the WidgetProperty/WidgetValue constructor error, suspect a markerless empty array in the template (shape-diff the failing widget's template against a working sibling; the smoking gun is `[]` vs `[<int>]`) |
 | `mxcli check --references` flags `attribute 'Type' is a reserved word (CE7247) [MDL021]` even though the attribute is **quoted** (`"Type": String`), and the skills say "always quote to avoid reserved-word conflicts" — tester assumed quoting should exempt it | NOT a bug — the check is correct. Quoting only escapes **MDL parser** keywords (`unquoteString` strips the quotes and the *bare* name is validated). `Type`, `ID`, `GUID`, `CurrentUser`, and the audit names `CreatedDate`/`ChangedDate`/`Owner`/`ChangedBy` are reserved by the Mendix **platform**, so they fail regardless of quoting. The gap was documentation: several skills claimed quoting is "always safe" without the platform-name carve-out | `mdl/executor/cmd_enumerations.go` (`mendixReservedWords`, `mendixSystemAttributeNames`) — no code change needed; docs only | Add the carve-out to the "always quote" guidance (`check-syntax.md`, `generate-domain-model.md`, `demo-data.md`, docs-site `lexical-structure.md`) and a CLAUDE.md note: quoting is *parser*-safe, not *platform*-safe. Rename `Type`→`ResourceType`; use `AutoCreatedDate`/… pseudo-types for audit fields. Adjacent Mendix rule documented alongside: the after-startup microflow must return `Boolean` (CE0142) — a void seed microflow fails the build |
 | A clickable `CONTAINER` can't be expressed: `OnClick: MICROFLOW …` / `Click:` error in the parser (`mismatched input 'MICROFLOW' expecting {',', ')'}`), and the one form that parses (`Action: MICROFLOW …`) is silently dropped — the container's BSON always has `OnClickAction = Forms$NoAction` | Layers all hardcoding "no click": (1) only the `Action:` keyword's `actionExprV3` accepted an action value, so `OnClick:`/`Click:` had no rule taking `MICROFLOW …`; (2) `buildContainerV3` never read `GetAction()`; (3) the `Container` model had no `OnClickAction` field and both writers (`serializeContainer`, modelsdk `widgetToGen`) emitted `serializeClientAction(nil)`/`noActionGen()`; (4) DESCRIBE never read it back | grammar `mdl/grammar/domains/MDLPage.g4` (`widgetPropertyV3`) + `mdl/visitor/visitor_page_v3.go` (`buildWidgetPropertyV3`) + `sdk/pages/pages_widgets_container.go` (`Container`) + `mdl/executor/cmd_pages_builder_v3_layout.go` (`buildContainerV3`) + `sdk/mpr/writer_widgets_layout.go` (`serializeContainer`) + `mdl/backend/modelsdk/widget_write.go` (DivContainer case) + `mdl/executor/cmd_pages_describe_parse.go`/`cmd_pages_describe_output.go` | Add an `ONCLICK COLON actionExprV3` alternative aliasing `Action:` (`make grammar`), stored under the same `Properties["Action"]` key. Add `OnClickAction ClientAction` to `Container`; in `buildContainerV3` read `GetAction()` → `buildClientActionV3()`; both writers serialize the real action (nil falls back to NoAction, so non-clickable containers are unchanged). DESCRIBE: extract `w["OnClickAction"]` via `extractButtonAction` (synthesize `{"Action": …}`) and emit `Action: <action>` in the container output. Verified with `mx check` = 0 errors. `Forms$DivContainer.OnClickAction` is required & introduced 8.3.0, so no version gate. Issue #603 |

mdl/bsonutil/order.go

@@ -55,6 +55,81 @@ func OrderStorageValue(v any) any {
 	}
 }
 
+// HoistStorageID is the minimal counterpart to OrderStorageValue: it moves "$ID"
+// first and "$Type" second in every document, but PRESERVES the original relative
+// order of all other keys (for ordered bson.D input) instead of sorting them.
+//
+// Prefer this over OrderStorageValue on trees that contain pluggable-widget /
+// datagrid page objects: those carry template-derived structures whose field
+// order Mendix is sensitive to, and a blind sort corrupts them (mx aborts with
+// "Expected '$ID' as the first property … but got 'LabelTemplate'"). This hoist
+// changes only what Mendix 11.12 actually requires — "$ID" first — and leaves
+// everything else byte-for-byte where it was. Go maps (which have no inherent
+// order) still fall back to sorted output for stability.
+func HoistStorageID(v any) any {
+	switch t := v.(type) {
+	case bson.D:
+		return hoistPairs(t)
+	case bson.M:
+		return hoistPairs(dFromMap(t))
+	case map[string]any:
+		return hoistPairs(dFromMap(t))
+	case bson.A:
+		out := make(bson.A, len(t))
+		for i, e := range t {
+			out[i] = HoistStorageID(e)
+		}
+		return out
+	case []any:
+		out := make([]any, len(t))
+		for i, e := range t {
+			out[i] = HoistStorageID(e)
+		}
+		return out
+	default:
+		return v
+	}
+}
+
+// dFromMap converts a Go map to a bson.D with keys sorted (a bare map carries no
+// order, so a stable sort is the only sensible input ordering); hoistPairs then
+// lifts "$ID"/"$Type" to the front.
+func dFromMap(m map[string]any) bson.D {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	d := make(bson.D, len(keys))
+	for i, k := range keys {
+		d[i] = bson.E{Key: k, Value: m[k]}
+	}
+	return d
+}
+
+// hoistPairs emits "$ID" first, "$Type" second, then every other element in its
+// original order, recursing into values via HoistStorageID.
+func hoistPairs(d bson.D) bson.D {
+	out := make(bson.D, 0, len(d))
+	for _, e := range d {
+		if e.Key == "$ID" {
+			out = append(out, bson.E{Key: e.Key, Value: HoistStorageID(e.Value)})
+		}
+	}
+	for _, e := range d {
+		if e.Key == "$Type" {
+			out = append(out, bson.E{Key: e.Key, Value: HoistStorageID(e.Value)})
+		}
+	}
+	for _, e := range d {
+		if e.Key == "$ID" || e.Key == "$Type" {
+			continue
+		}
+		out = append(out, bson.E{Key: e.Key, Value: HoistStorageID(e.Value)})
+	}
+	return out
+}
+
 // orderMap converts a map into an ordered bson.D with "$ID" first, "$Type"
 // second, and all other keys sorted. Values are recursed through
 // OrderStorageValue so nested storage objects gain the same ordering.

mdl/bsonutil/order_test.go

@@ -89,3 +89,84 @@ func findKey(t *testing.T, d bson.D, key string) any {
 	t.Fatalf("key %q not found in %v", key, d)
 	return nil
 }
+
+// TestHoistStorageID_PreservesOrderExceptID is the key distinction from
+// OrderStorageValue: HoistStorageID lifts "$ID" first and "$Type" second but must
+// NOT reorder the remaining keys. A blind sort (as OrderStorageValue does)
+// corrupts template-derived pluggable-widget page trees, so the nightly's
+// datagrid pages must round-trip with their widget field order intact.
+func TestHoistStorageID_PreservesOrderExceptID(t *testing.T) {
+	// A widget-shaped doc: $ID appears late, and the non-$ID keys are in a
+	// deliberately non-alphabetical order that must be preserved.
+	in := bson.D{
+		{Key: "Name", Value: "w"},
+		{Key: "LabelTemplate", Value: "lt"},
+		{Key: "$Type", Value: "Forms$TextBox"},
+		{Key: "TabIndex", Value: int32(0)},
+		{Key: "$ID", Value: "id-1"},
+		{Key: "Attribute", Value: "Attr"},
+	}
+	got, ok := HoistStorageID(in).(bson.D)
+	if !ok {
+		t.Fatalf("HoistStorageID returned %T, want bson.D", HoistStorageID(in))
+	}
+	gotKeys := make([]string, len(got))
+	for i, e := range got {
+		gotKeys[i] = e.Key
+	}
+	// $ID first, $Type second, then the rest in ORIGINAL order (not sorted).
+	want := []string{"$ID", "$Type", "Name", "LabelTemplate", "TabIndex", "Attribute"}
+	if len(gotKeys) != len(want) {
+		t.Fatalf("keys = %v, want %v", gotKeys, want)
+	}
+	for i := range want {
+		if gotKeys[i] != want[i] {
+			t.Errorf("key[%d] = %q, want %q (full: %v)", i, gotKeys[i], want[i], gotKeys)
+		}
+	}
+}
+
+// TestHoistStorageID_RecursesAndMarshalsIDFirst verifies nested objects are
+// hoisted too and the marshalled bytes lead with $ID at every level.
+func TestHoistStorageID_RecursesAndMarshalsIDFirst(t *testing.T) {
+	in := bson.D{
+		{Key: "Widget", Value: bson.D{
+			{Key: "LabelTemplate", Value: "lt"},
+			{Key: "$Type", Value: "T"},
+			{Key: "$ID", Value: "child"},
+		}},
+		{Key: "$ID", Value: "root"},
+	}
+	raw, err := bson.Marshal(HoistStorageID(in))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var d bson.D
+	if err := bson.Unmarshal(raw, &d); err != nil {
+		t.Fatal(err)
+	}
+	if d[0].Key != "$ID" {
+		t.Errorf("root first key = %q, want $ID", d[0].Key)
+	}
+	child := findKey(t, d, "Widget").(bson.D)
+	if child[0].Key != "$ID" {
+		t.Errorf("child first key = %q, want $ID", child[0].Key)
+	}
+}
+
+// TestHoistStorageID_MapFallbackHoistsID confirms a Go map (no inherent order)
+// still comes out $ID-first after marshalling.
+func TestHoistStorageID_MapFallbackHoistsID(t *testing.T) {
+	in := map[string]any{"Zeta": 1, "LabelTemplate": 2, "$ID": "x", "$Type": "T"}
+	raw, err := bson.Marshal(HoistStorageID(in))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var d bson.D
+	if err := bson.Unmarshal(raw, &d); err != nil {
+		t.Fatal(err)
+	}
+	if d[0].Key != "$ID" || d[1].Key != "$Type" {
+		t.Errorf("first two keys = %q,%q, want $ID,$Type", d[0].Key, d[1].Key)
+	}
+}

sdk/mpr/writer_businessevents.go

@@ -68,7 +68,7 @@ func (w *Writer) serializeBusinessEventService(svc *model.BusinessEventService)
 	// SourceApi is null for service definitions
 	doc = append(doc, bson.E{Key: "SourceApi", Value: nil})
 
-	return bson.Marshal(doc)
+	return marshalUnitIDFirst(doc)
 }
 
 func serializeBusinessEventDefinition(def *model.BusinessEventDefinition) bson.D {

sdk/mpr/writer_customblob.go

@@ -71,7 +71,7 @@ func (w *Writer) writeCustomBlobDocument(in customBlobInput) error {
 		{Key: "Name", Value: in.Name},
 	}
 
-	contents, err := bson.Marshal(doc)
+	contents, err := marshalUnitIDFirst(doc)
 	if err != nil {
 		return fmt.Errorf("failed to marshal CustomBlobDocument BSON: %w", err)
 	}
@@ -111,7 +111,7 @@ func (w *Writer) updateCustomBlobDocument(in customBlobInput) error {
 		{Key: "Name", Value: in.Name},
 	}
 
-	contents, err := bson.Marshal(doc)
+	contents, err := marshalUnitIDFirst(doc)
 	if err != nil {
 		return fmt.Errorf("failed to marshal CustomBlobDocument BSON: %w", err)
 	}

sdk/mpr/writer_datatransformer.go

@@ -116,5 +116,5 @@ func serializeDataTransformer(dt *model.DataTransformer) ([]byte, error) {
 		{Key: "Steps", Value: steps},
 	}
 
-	return bson.Marshal(doc)
+	return marshalUnitIDFirst(doc)
 }

sdk/mpr/writer_dbconnection.go

@@ -80,7 +80,7 @@ func (w *Writer) serializeDatabaseConnection(conn *model.DatabaseConnection) ([]
 	// LastSelectedQuery (empty ref)
 	doc = append(doc, bson.E{Key: "LastSelectedQuery", Value: ""})
 
-	return bson.Marshal(doc)
+	return marshalUnitIDFirst(doc)
 }
 
 func serializeDBQuery(q *model.DatabaseQuery) bson.D {