fix(docker): stop docker build converting MPRv2 projects to MPRv1 (#808)

.claude/skills/fix-issue.md

@@ -107,7 +107,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
 | CE0463 on a `datagrid` whose column uses `ColumnWidth: manual` + `Size: N` — Studio Pro resets the column `size` to `1` | The MDL `ColumnWidth:` keyword isn't mapped to the schema `width` enum, so `width` stays at its `autoFill` default; `size` only applies when `width=manual`, so the value is inconsistent. Regression from the Stream B keyword-path consolidation (the deleted `datagrid_builder.go` did `colPropString(col.Properties, "ColumnWidth", "autoFill")`) | `mdl/executor/widget_defs.go` `itemPropertyAliases` | Add the MDL→schema alias under `[datagrid]["columns"]`: `"width": {"ColumnWidth"}`. Bump `WidgetDefGeneratorVersion` so stale `.def.json` regenerate. General rule when a column/object-list property's MDL keyword differs from the `.mpk` schema key (not just case), add it to `itemPropertyAliases`; cross-check against the pre-B3 `datagrid_builder.go` `colProp*` calls for any other dropped mappings |
 | CE0463 on an MDL-created **Combobox** (or other platform widget) at `mxcli docker build/check` time — but the SAME widget passes `mx check` on a project whose installed `.mpk` matches mxcli's embedded template (Mendix 11.6 / combobox 2.5.0). NOT the old incomplete-template bug (#112, fixed — a matching-version combobox is clean) | Widget-VERSION mismatch: mxcli emits the embedded 2.5.0-shaped PropertyTypes, but the project has a NEWER combobox (e.g. 2.8.1) that reorders/regroups properties. `augmentFromMPK` patches presence + enum values but can't restructure the baseline; `GenerateFromMPK` is less faithful still (fails even on a matching version). The designed remediation is `mx update-widgets` (docker build/check runs it before `mx check`) | `cmd/mxcli/docker/check.go` + `build.go` (`updateWidgetsPathArg`) — the update-widgets *invocation*, not the widget emission | The real trap was that `mx update-widgets <bare-app.mpr>` **crashed** (`AddProjectDirAsAllowedPath` → `Path.GetDirectoryName("app.mpr")` = "" → `System.ArgumentNullException`) and some mx builds exit 0 after printing it, so the migration silently no-op'd and CE0463 survived. Pass an **absolute** path to update-widgets (always has a directory component). `mx check` is unaffected. Diagnosis: run the bundled `mx update-widgets <bare.mpr>` yourself — if it throws ArgumentNullException on AddProjectDirAsAllowedPath, the path lacks a dir. Faithful multi-version widget emission is the larger fix (#529). Issue #112; repro `mdl-examples/bug-tests/112-combobox-enum-ce0463-widget-version.mdl` |
 | **CE0463 "widget definition changed" at an `Image` widget** on Mendix 11.7+ (`mx check` without `update-widgets`) — a plain mxcli-authored Image, no custom config. `update-widgets` clears it (and on v2 destroys `mprcontents/`, so it's a data-loss trap) | NOT a version stamp, Type `$ID`s, property order, or missing properties (all ruled out empirically). The embedded `image.json` carried a **spurious default value**: a `WidgetValue.Image` pointing at `Atlas_Core.Content.Mendix` (Atlas's Mendix logo), captured when the template was extracted from a project that had it set. The installed 11.12 Image widget expects that field empty, so MxBuild flags the definition as changed | `modelsdk/widgets/templates/mendix-11.6/image.json` + `sdk/widgets/templates/mendix-11.6/image.json` (line ~63) | Clear the stale default: `"Image": "Atlas_Core.Content.Mendix"` → `"Image": ""` in both engine templates. **Diagnosis method for this whole CE0463 class**: dump the widget BSON, `mx update-widgets` on a COPY, dump again, diff the `CustomWidgets$CustomWidget` subtree **order-independently** (canonicalise key order + mask `$ID`/`TypePointer` blobs). Reordering and generic instance chrome (`LabelTemplate`, `Appearance.DesignProperties`) are cosmetic — a *passing* widget gets reordered/those-added too; the real cause is whatever value/structure survives that normalisation. Other hand-extracted templates likely hide similar stale defaults (audit with the same diff). Repro `mdl-examples/bug-tests/image-ce0463-stale-default.mdl`. DataGrid2 custom-content CE0463 (#600) is a *separate*, more complex delta — same method, own fix |
-| `mxcli docker check`/`build` (or a bare `mx update-widgets`) **silently deletes `mprcontents/`** and rewrites an MPRv2 project into single-file v1 — `check` reports **0 errors** and looks successful, but the git working tree diverges from tracked files, a running `mxcli run --local` loop breaks (it watches `mprcontents/`), and Studio Pro may crash on open (`LibGit2RepositoryProvider.WriteBaseFile`). Triggered by following the CE0463 remediation on a `mxcli new` (always v2) project | The pre-check `mx update-widgets` step (run to suppress false CE0463) **performs the conversion**: it inlines every unit into the `.mpr` (`Unit.Contents` column) and deletes `mprcontents/`. The `check` itself is read-only; `update-widgets` is the mutator. docker check/build invoked it with no storage-format protection | `cmd/mxcli/docker/check.go` (`snapshotStorageFormat` / `copyFile` / `copyDir`) + `build.go` | Snapshot `.mpr` + `mprcontents/` to a temp dir before `update-widgets`, `defer restore()` after the check (restore removes the post-conversion single-file `.mpr` residue and puts the v2 tree back); MPRv1 projects need no protection. The check still runs against the widget-normalized model, so CE0463 stays suppressed — only the on-disk format is preserved. **Never tell an agent to run bare `mx update-widgets` on a v2 project** — the synced skills (`create-page.md`, `custom-widgets.md`, `migrate-design-prototype.md`, `download-marketplace-content.md`) + dev `debug-bson.md` route to `mxcli docker check`/`build` (v2-safe) instead. Issue #763 / PR #764 |
+| `mxcli docker check`/`build` (or a bare `mx update-widgets`) **silently deletes `mprcontents/`** and rewrites an MPRv2 project into single-file v1 — `check` reports **0 errors** and looks successful, but the git working tree diverges from tracked files, a running `mxcli run --local` loop breaks (it watches `mprcontents/`), and Studio Pro may crash on open (`LibGit2RepositoryProvider.WriteBaseFile`). Triggered by following the CE0463 remediation on a `mxcli new` (always v2) project | The pre-check `mx update-widgets` step (run to suppress false CE0463) **performs the conversion**: it inlines every unit into the `.mpr` (`Unit.Contents` column) and deletes `mprcontents/`. The `check` itself is read-only; `update-widgets` is the mutator. docker check/build invoked it with no storage-format protection | `cmd/mxcli/docker/update_widgets.go` (`runUpdateWidgets` / `snapshotStorageFormat`) — call sites in `check.go` and `build.go` | Snapshot `.mpr` + `mprcontents/` to a temp dir before `update-widgets`, `defer restore()` after the check (restore removes the post-conversion single-file `.mpr` residue and puts the v2 tree back); MPRv1 projects need no protection. The check still runs against the widget-normalized model, so CE0463 stays suppressed — only the on-disk format is preserved. **Never tell an agent to run bare `mx update-widgets` on a v2 project** — the synced skills (`create-page.md`, `custom-widgets.md`, `migrate-design-prototype.md`, `download-marketplace-content.md`) + dev `debug-bson.md` route to `mxcli docker check`/`build` (v2-safe) instead. **Fix the operation, not the call site**: PR #764 wrapped the invocation inside `Check` only, and `Build` had a second bare copy — so `docker build`/`run`/`reload` kept converting projects for another 40 issues, until #808. The snapshot now lives in `runUpdateWidgets`, which is the only place that may exec `update-widgets`; grep for `"update-widgets"` should return exactly one hit. When a mutating external step is guarded, put the guard in a function that also *performs* the step, so a new caller cannot get it wrong. Issues #763 / PR #764, #808 |
 | Nightly `mx check` reports `CE0117 "Error(s) in expression." at Log message activity 'Log message (warning)'` on Mendix 10.24.19+ but not 10.24.16 or 11.x | Mendix 10.24.19 tightened expression validation: `toString(<string>)` is now a type error (toString expects a non-string input). An example called `toString($OrderNumber)` where `$OrderNumber` was already a string parameter | The offending `log warning ... with ({1} = toString($stringVar))` — find via `~/.mxcli/mxbuild/{ver}/modeler/mx check`, then bisect with `drop microflow ...` until CE0117 disappears | Remove the redundant `toString()` wrapper around already-string values. Only wrap non-string values (integers, decimals, dates, enums) in `toString()`. The Mendix 11.x parser is more lenient and lets this slide, but 10.24.19+ rejects it |
 | A page-level property can't be set — `ALTER PAGE X { SET PopupWidth = 800; }` (or any page-level prop other than Title/Url) fails with `unsupported page-level property: …` | The page-level SET handler only special-cased a couple of properties; everything else fell through to the default error | `mdl/backend/pagemutator/mutator.go` → `applyPageLevelSetMut` (shared by both engines) | Add a `case` writing the field at the top level of the Forms$Page doc via `dSetOrAppend` with the on-disk BSON type (int64 for PopupWidth/PopupHeight, bool for PopupResizable — verify against a Studio Pro page with `mxcli bson dump --format bson`). Page-level prop names are **case-sensitive**. For DESCRIBE roundtrip, emit the values back in the CREATE PAGE header. CREATE-time support: add a generic `IDENTIFIER COLON propertyValueV3` to `pageHeaderPropertyV3` (regen grammar), recognize the keys in `parsePageHeaderV3` (`applyGenericPageHeaderProp`, error on unknown), carry `*int`/`*bool` on `CreatePageStmtV3`, default to 600/600/false in `buildPageV3`, and have both writers honour `page.Popup*` (legacy `sdk/mpr/writer_pages.go` int64; codec `mdl/backend/modelsdk/page_write.go` int32 via gen — tolerated by mx check). The MCP backend has its **own** `mcpPageMutator` (pg content tree, not raw BSON) — page-level SET there reaches `SetWidgetProperty("")`; map it or reject honestly (it rejects, pending a `pg_read_page` probe of the pop-up keys). Issue #661 |
 | `PopupWidth: 0` / `PopupHeight: 0` rejected ("must be a positive number") on CREATE or ALTER PAGE; user can't make an auto-size pop-up | 0 is actually Studio Pro's **default** for pop-up dimensions (auto-size) — verified live on 11.12: a pg-created PopupLayout page stores 0/0 and `mx check` = 0 errors. Two validators rejected ≤0, and both writers coerced ≤0→600, so even an allowed 0 became 600 | `mdl/visitor/visitor_page_v3.go` (`popupDimensionValue`) + `mdl/backend/pagemutator/mutator.go` (`coercePopupDimension`) + `mdl/executor/cmd_pages_builder_v3.go` (builder default) + `sdk/mpr/writer_pages.go` & `mdl/backend/modelsdk/page_write.go` (`popupDimension`) + `mdl/executor/cmd_pages_describe.go` | Relax both validators to reject only **negative**; default the builder to **0** (not 600, matching Studio Pro); drop the `≤0→600` coercion in both writers (clamp only negatives to 0); have DESCRIBE suppress only the real default 0 (emit an explicit 600). No `*int` needed — 0 is a valid stored value. Bug-test `mdl-examples/bug-tests/713-popup-zero-dimensions.mdl`. Issue #713 |

cmd/mxcli/docker/build.go

@@ -100,16 +100,15 @@ func Build(opts BuildOptions) error {
 		if err != nil {
 			fmt.Fprintf(w, "  Skipping check: %v\n", err)
 		} else {
-			// Run update-widgets before check to prevent false CE0463 errors
+			// Run update-widgets before check to prevent false CE0463 errors.
+			// runUpdateWidgets preserves the project's on-disk storage format: the bare
+			// invocation this replaced converted MPRv2 projects to MPRv1 and deleted
+			// mprcontents/ (mendixlabs/mxcli#808). restore is deferred to Build's exit
+			// rather than run here, so both `mx check` and MxBuild below see the
+			// widget-normalized model; only the on-disk format is put back.
 			if !opts.SkipUpdateWidgets {
-				fmt.Fprintln(w, "  Updating widget definitions...")
-				uwCmd := exec.Command(mxPath, "update-widgets", updateWidgetsPathArg(opts.ProjectPath))
-				uwCmd.Stdout = w
-				uwCmd.Stderr = os.Stderr
-				PrepareMxCommand(uwCmd)
-				if err := uwCmd.Run(); err != nil {
-					fmt.Fprintf(w, "  Warning: update-widgets failed (continuing): %v\n", err)
-				}
+				restore := runUpdateWidgets(mxPath, opts.ProjectPath, w, os.Stderr)
+				defer restore()
 			}
 
 			cmd := exec.Command(mxPath, "check", opts.ProjectPath)

cmd/mxcli/docker/build_integration_test.go

@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build integration
+
+package docker
+
+import (
+	"bytes"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/mendixlabs/mxcli/sdk/mpr"
+)
+
+// TestBuild_PreservesMPRv2StorageFormat is the end-to-end guard for
+// mendixlabs/mxcli#808, the counterpart to TestCheck_PreservesMPRv2StorageFormat
+// (#763). Build's pre-check step ran its own bare `mx update-widgets`, which
+// rewrites an MPRv2 project into the self-contained MPRv1 format — inlining every
+// unit into the .mpr and deleting mprcontents/. #764 protected Check only, so
+// `mxcli docker build`, `docker run` and `docker reload` kept converting projects
+// while reporting success.
+//
+// DryRun stops Build immediately after the check step, so this exercises the whole
+// buggy path (update-widgets + mx check) without paying for a full MxBuild. The
+// deferred restore still runs on the DryRun return.
+//
+// Requires a resolvable mx/MxBuild and a JDK 21 (provided by the CI integration
+// job); skips otherwise.
+func TestBuild_PreservesMPRv2StorageFormat(t *testing.T) {
+	mxPath, err := ResolveMx("")
+	if err != nil {
+		t.Skipf("mx not resolvable: %v", err)
+	}
+	if _, err := resolveJDK21(); err != nil {
+		t.Skipf("JDK 21 not resolvable: %v", err)
+	}
+
+	// Scaffold a fresh project. `mx create-project` (no template arg) writes App.mpr
+	// into the working directory and produces MPRv2 storage.
+	dir := t.TempDir()
+	scaffold := exec.Command(mxPath, "create-project")
+	scaffold.Dir = dir
+	if out, err := scaffold.CombinedOutput(); err != nil {
+		t.Skipf("mx create-project failed, cannot scaffold fixture: %v\n%s", err, out)
+	}
+	mprPath := filepath.Join(dir, "App.mpr")
+	if _, err := os.Stat(mprPath); err != nil {
+		t.Skipf("mx create-project did not produce App.mpr: %v", err)
+	}
+
+	// Precondition: the fixture must be MPRv2, or the test proves nothing.
+	if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 {
+		t.Skipf("scaffolded project is %v, not MPRv2 — nothing to protect", v)
+	}
+
+	var stdout bytes.Buffer
+	if err := Build(BuildOptions{
+		ProjectPath: mprPath,
+		DryRun:      true,
+		Stdout:      &stdout,
+	}); err != nil {
+		t.Fatalf("Build failed: %v\nstdout:\n%s", err, stdout.String())
+	}
+
+	// Postcondition: still MPRv2. Without the fix, update-widgets would have left it
+	// MPRv1 with mprcontents/ deleted.
+	if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 {
+		t.Errorf("Build converted the project to %v; the MPRv2 storage format must be preserved (#808)", v)
+	}
+	if _, err := os.Stat(filepath.Join(dir, "mprcontents")); err != nil {
+		t.Errorf("mprcontents/ missing after Build, storage format was not preserved: %v", err)
+	}
+}