From 46a47b1c3b9a55ef0b3f95bef0400051c2487c98 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 05:16:54 +0000 Subject: [PATCH 01/17] fix(check): read the same widget registry exec does `mxcli check -p --references` refused MDL that `describe page` had just emitted, while `exec --no-check` wrote it without complaint. On a blank Mendix 11.12.2 project with no .mxcli/widgets: check --references -> `htmlelement` is not a widget in this project [MDL-WIDGET25] `attribute` is not a widget in this project [MDL-WIDGET25] `tagcontentcontainer` is not a widget in this project [MDL-WIDGET25] exit 1, so exec refused to run the script exec --no-check -> info: updated widget definitions for Repro1135.mpr Created page MyFirstModule.HtmlDemo check is meant to be the strict gate and exec the thing that runs; here it was inverted, and the script it blocked was one describe had just written. The two read different registries. pageBuilder.initPluggableEngine calls RefreshStaleWidgetDefinitions before LoadUserDefinitions, so exec generates .mxcli/widgets/*.def.json from the project's installed .mpk on its way past. LoadWidgetRegistry -- check, lint and the LSP -- called only LoadUserDefinitions, so a project that had never run `mxcli widget init` knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has packageInstalledFor as its escape hatch; the generic-MDL-name branch has none. LoadWidgetRegistry now refreshes stale definitions the same way, best-effort -- a project whose definitions cannot be written gets the registry it got before, which beats failing a check over a cache. Control (mdl/executor/validate_widget_kind_uninitialized_test.go): before the fix the test reports the three names above verbatim; a typo (`htmlelemnt`) in the same project is still MDL-WIDGET25 after it, and now suggests `htmlelement`, which it could not while the candidate list was the nine embedded widgets. The bug self-heals -- the first exec writes the definitions and every check after it passes -- which is why it reads as flaky. Reproduce with `rm -rf .mxcli/widgets` or it is invisible. Refs: mendixlabs/mxcli#1135 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- ...validate_widget_kind_uninitialized_test.go | 95 +++++++++++++++++++ mdl/executor/validate_widgets.go | 22 +++++ 2 files changed, 117 insertions(+) create mode 100644 mdl/executor/validate_widget_kind_uninitialized_test.go diff --git a/mdl/executor/validate_widget_kind_uninitialized_test.go b/mdl/executor/validate_widget_kind_uninitialized_test.go new file mode 100644 index 000000000..a06ca9dba --- /dev/null +++ b/mdl/executor/validate_widget_kind_uninitialized_test.go @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "os" + "path/filepath" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// projectWithInstalledHTMLElement builds the state every `mxcli new` project is +// in: the widget's .mpk sits in widgets/, and .mxcli/widgets/ has never been +// written because nobody ran `mxcli widget init`. +// +// A temp dir rather than testdata/expr-checker/minimal.mpr, which is in exactly +// this state too — the registry generates .def.json files into the project as a +// side effect, and a test must not decide whether the next test sees them. +func projectWithInstalledHTMLElement(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "widgets"), 0o755); err != nil { + t.Fatal(err) + } + const mpk = "com.mendix.widget.web.HTMLElement.mpk" + data, err := os.ReadFile(filepath.Join("..", "..", "testdata", "expr-checker", "widgets", mpk)) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "widgets", mpk), data, 0o644); err != nil { + t.Fatal(err) + } + return filepath.Join(dir, "App.mpr") +} + +// htmlElementTree is what `describe page` emits for an HTML Element the project +// already contains — the widget by its MDL name, with the two containers its +// definition declares. +func htmlElementTree() []*ast.WidgetV3 { + return []*ast.WidgetV3{{ + Type: "htmlelement", + Name: "frame", + TypeIsGeneric: true, + Properties: map[string]any{"tagName": "div"}, + Children: []*ast.WidgetV3{ + {Type: "attribute", Name: "attribute1", TypeIsGeneric: true, + Properties: map[string]any{"attributeName": "data-x"}}, + {Type: "tagcontentcontainer", Name: "tagcontentcontainer1", TypeIsGeneric: true, + Properties: map[string]any{}}, + }, + }} +} + +// mendixlabs/mxcli#1135, route 2. +// +// `check` is meant to be the strict gate and `exec` the thing that runs. Here it +// was inverted: with the .mpk installed but no .def.json extracted, `check -p +// --references` reported three MDL-WIDGET25 errors — so exec refused to run — +// while `exec --no-check` wrote the page and generated the definitions on its +// way past. Measured on a blank 11.12.2 project: +// +// check --references -> `htmlelement` is not a widget in this project +// `attribute` is not a widget in this project +// `tagcontentcontainer` is not a widget in this project +// exec --no-check -> info: updated widget definitions ... / Created page +// +// The cause is that the two read different registries. The page builder calls +// RefreshStaleWidgetDefinitions before loading user definitions +// (cmd_pages_builder.go); LoadWidgetRegistry, which check and the LSP use, did +// not — so the validator knew the nine embedded widgets and nothing else. +// +// The self-healing is what made this read as flaky: the FIRST exec writes the +// definitions, and every check after it passes. +func TestValidateWidgetKind_InstalledWidgetNeedsNoWidgetInit(t *testing.T) { + got := widgetKindViolations(t, projectWithInstalledHTMLElement(t), htmlElementTree()) + if containsRule(got, "MDL-WIDGET25") { + t.Errorf("an installed widget was reported as absent from the project: %v", got) + } + if containsRule(got, "MDL-WIDGET26") { + t.Errorf("a container the widget declares was reported as undeclared: %v", got) + } +} + +// The control for the above, and the reason the fix cannot be "stop reporting". +// A typo is still a typo in the same project: the .mpk that makes `htmlelement` +// real says nothing about `htmlelemnt`. +func TestValidateWidgetKind_TypoStillReportedInThatProject(t *testing.T) { + got := widgetKindViolations(t, projectWithInstalledHTMLElement(t), []*ast.WidgetV3{ + {Type: "htmlelemnt", Name: "frame", TypeIsGeneric: true, Properties: map[string]any{}}, + }) + if !containsRule(got, "MDL-WIDGET25") { + t.Errorf("a misspelt widget was not reported: %v", got) + } +} diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 931153fd2..d061b0dda 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -13,6 +13,7 @@ package executor import ( "fmt" + "log" "regexp" "sort" "strconv" @@ -60,6 +61,27 @@ func LoadWidgetRegistry(projectPath string) *WidgetRegistry { return nil } if projectPath != "" { + // Generate the project's .def.json files from its installed .mpk when + // they are missing or behind this build, exactly as the page builder + // does before it reads them (cmd_pages_builder.go). Without this the + // validator and the builder read DIFFERENT registries, and the + // difference pointed the wrong way: on a project that had never run + // `mxcli widget init`, `check -p --references` reported every installed + // widget as "not a widget in this project" while `exec --no-check` + // wrote the page and generated the definitions on its way past + // (mendixlabs/mxcli#1135). check is meant to be the strict gate and + // exec the thing that runs; here it was inverted, and the script it + // blocked was one describe had just emitted. + // + // The self-healing is what made it read as flaky: the first exec writes + // the definitions and every check after it passes. + // + // Best-effort. A project whose definitions cannot be written — read-only + // checkout, no widgets/ at all — gets the registry it got before, which + // is strictly better than failing the check over a cache. + if _, err := RefreshStaleWidgetDefinitions(projectPath); err != nil { + log.Printf("warning: updating widget definitions: %v", err) + } _ = registry.LoadUserDefinitions(projectPath) registry.projectPath = projectPath // The validator and DESCRIBE WIDGET must agree about which properties a From abba3d72d6fa91b16739851eb842a66d1cd3ed4d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 05:17:09 +0000 Subject: [PATCH 02/17] fix(alter page): name the route a built-in widget's SET dead-ends into MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `alter page … set '' = off on ` failed with property "Remove empty text" not found (widget has no pluggable Object) "pluggable Object" is not something the author wrote, cannot be made true by editing the script, and is not the whole truth: design properties on a built-in widget are written by ALTER STYLING, which the message never mentioned. It now says so, naming the widget and the property, so the reader has the statement rather than a bug report. A Forms$Appearance and no pluggable Object is exactly a built-in widget, which is when the advice applies. A pluggable widget keeps the error naming its own declared keys -- redirecting a mistyped pluggable key to ALTER STYLING would point at a command that cannot write it either (control: TestSetWidgetProperty_PluggableWidgetKeepsItsOwnError). Measured on a blank 11.12.2 project, and worth recording because it inverts the report: the three names in it are NOT ListView design properties in the Atlas shipped with Mendix 11. themesource/atlas_core/web/design-properties.json declares Style, Hover style and Row size, and nothing else. ALTER STYLING writes the reported key happily and mxbuild then refuses the project: alter styling … set 'Remove empty text' = on; -> mx check: 1 error, [CE6083] "Design property Remove empty text is not supported by your theme." at List view 'lvThings' alter styling … set 'Row size' = 'Small'; -> mx check: 0 errors So refusing the write was right and only the reason was wrong, which is why this is the message and not write support. Left open deliberately, and noted in the bug test: MDL-WIDGET11 ("design property not defined for this widget type") covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not ALTER STYLING, whose widget type lives only in the stored document -- so an unsupported key there is silent until mxbuild says CE6083. Resolving it needs a third $Type-to-registry mapping and belongs in its own change rather than bolted on here. Refs: mendixlabs/mxcli#1135 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .../fix-issue/findings/mdl-executor.jsonl | 1 + ...5-installed-widget-without-widget-init.mdl | 79 +++++++++++++++++++ mdl/backend/pagemutator/mutator.go | 29 ++++++- .../pagemutator/native_set_diagnostic_test.go | 65 +++++++++++++++ 4 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl create mode 100644 mdl/backend/pagemutator/native_set_diagnostic_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 02a047ee9..5c43baaba 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -639,3 +639,4 @@ {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY MICROFLOW` re-enables concurrent execution on a microflow that disallowed it \u2014 the running app's concurrency protection removed \u2014 and drops the concurrency error message (all translations) and error microflow, plus `MarkAsUsed`. Every checker is green: **CE4899 fires only on disallow-without-a-message, never on allow**, so the one error that exists in this area is exactly the one the reset switches off", "cause": "`buildMicroflowFromStmt` built the rebuild struct with `AllowConcurrentExecution: true` and `MarkAsUsed: false` literals, and `microflowToGen` wrote `SetConcurrencyErrorMicroflowQualifiedName(\"\")` + a bare `genTexts.NewText()`. The backend already READ the two flags back (the #723 \u00a7A fix), so the round-trip test passed while the bug was live \u2014 the executor overwrote them before the backend ever saw them", "file": "`mdl/executor/cmd_microflows_build.go` (buildMicroflowFromStmt), `mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `sdk/microflows/microflows.go`", "fix": "Carry all four from the stored microflow, seeding the locals with the NEW-microflow defaults (true/false) so no separate preserve flag is needed. The error message reuses the existing `textFromGen`/`textToGen` pair, so translations survive; nil still emits the bare empty `Texts$Text` the writer always wrote", "insight": "**A passing round-trip test at one layer says nothing about the layer above it.** `TestMicroflowRoundTrip_ConcurrentExecutionFlags` had guarded these two flags since #723 and was green throughout, because the executor's rebuild struct overwrites them before calling the backend. When a property is reset, locate the LAST writer on the path, not the first one that looks responsible. **And check which way a reset goes**: #723's backend bug wrote the Go zero value (allow -> disallow) and hit CE4899 immediately; the executor's literal writes the opposite (disallow -> allow), and the same CE4899 that caught the first direction is structurally blind to the second. A checker that catches a property's loss in one direction is not coverage for that property. Two methodological traps in the test itself, both hit: `bytes.Equal` on two encodes of the same microflow ALWAYS differs (fresh random sub-element `$ID`s \u2014 the reason `canon` exists), and `canon.Equal` on a whole microflow always differs too, because `StableId` is a fresh GUID *value* per encode and `Equal` does not mask \u2014 only `Reconcile` may be asked that question. Compare the sub-element under test, or use Reconcile. Controls: hardcoding the executor literals back, emptying the writer's pair, and stubbing the reader each fail a different test with the reported symptom"} {"area":"mdl/executor","date":"2026-09-17","symptom":"`DESCRIBE ENUMERATION Mod.E` prints every value with an empty caption (`MyValue ''`) although Studio Pro shows them. Re-executing that output then DESTROYS the real captions (exec reports \"Modified enumeration\" and the stored Texts$Translation goes empty). Reported on Windows, single-language project, v0.18.0 and v0.22.0","cause":"The read asked `v.Caption.GetTranslation(\"en_US\")`. Mendix has no language-neutral text: a project whose DefaultLanguageCode is nl_NL stores the caption under nl_NL and nothing else, so the lookup misses and returns \"\". #970 fixed the WRITE side to use the project language and #702 fixed the widget READ side; the enumeration/validation-rule/message-template reads were the sites neither sweep reached","file":"`mdl/executor/cmd_enumerations.go` (describeEnumeration), `cmd_diff_mdl.go` (enumerationToMDL), `describe_language.go` (pickTextTranslation's fallback now sorts), `mdl/catalog/language.go` + `builder_modules.go`","insight":"**Reproduce it with mxcli alone — no Studio Pro and no non-English project needed.** `ALTER SETTINGS LANGUAGE ADD OR MODIFY 'nl_NL' (...); ALTER SETTINGS LANGUAGE DefaultLanguageCode = 'nl_NL';` on a copy of any fixture, then CREATE the enumeration: the write side already honours the project language, so the captions land under nl_NL and DESCRIBE reads '' immediately. That also gives the impact control for free — feed the '' output back through exec and grep the .mxunit for `Texts$Translation LanguageCode nl_NL Text ` with nothing after it. **The plausible wrong turn to skip**: suspecting the codec or a gen storage-name mismatch. `EnumerationValue.Caption` is NOT in keyaudit_test.go and the strings are plainly visible in the unit — dump the .mxunit with a printable-ASCII regex FIRST (one command) and the language code tells you it is a read-side language bug, not a decode bug. **The fallback has to sort**: `for _, v := range t.Translations` returns a different language per run, so a multi-language project's DESCRIBE output was undiffable — a bug that a single-language repro can never show.","refs":["mendixlabs/mxcli#1113","mendixlabs/mxcli#970","mendixlabs/mxcli#702"],"ce":[]} {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} +{"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11: themesource/atlas_core/web/design-properties.json declares Style, Hover style, Row size and nothing else. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open and separate: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} diff --git a/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl new file mode 100644 index 000000000..d0425d44c --- /dev/null +++ b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl @@ -0,0 +1,79 @@ +-- mendixlabs/mxcli#1135 — `mxcli check` refused MDL that `describe page` had +-- just emitted, and `exec` then accepted. +-- +-- Run against a project that has the HTML Element widget installed (every +-- `mxcli new` project does) and has NEVER run `mxcli widget init`: +-- +-- rm -rf .mxcli/widgets +-- mxcli check mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl -p App.mpr --references +-- mxcli exec mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl -p App.mpr +-- +-- Before the fix, measured on a blank Mendix 11.12.2 project: +-- +-- check --references -> `htmlelement` is not a widget in this project [MDL-WIDGET25] +-- `attribute` is not a widget in this project [MDL-WIDGET25] +-- `tagcontentcontainer` is not a widget in this project [MDL-WIDGET25] +-- exit 1, so exec refused to run the script +-- exec --no-check -> info: updated widget definitions for App.mpr +-- Created page MyFirstModule.HtmlDemo +-- +-- check was STRICTER than exec, which is the inversion check exists to prevent: +-- the script it blocked is one exec writes without complaint. The cause is that +-- the two read different registries — the page builder calls +-- RefreshStaleWidgetDefinitions before reading definitions, and +-- LoadWidgetRegistry (check, lint, LSP) did not — so the validator knew the nine +-- embedded widgets and nothing else. +-- +-- The self-healing is what made it read as flaky: the first exec writes +-- .mxcli/widgets/*.def.json, and every check after it passes. Delete that +-- directory before reproducing, or the bug is invisible. +-- +-- `describe page` on the result of this script emits these same three keywords, +-- so before the fix `describe -> check` did not round-trip. + +create or replace page MyFirstModule.HtmlDemo ( + title: 'Html', + layout: 'Atlas_Core.Atlas_Default' +) { + htmlelement frame (tagName: 'div') { + attribute a1 (attributeName: 'data-x', attributeValue: 'y') + tagcontentcontainer body { + dynamictext dt (content: 'hi') + } + } +}; + +-- The second half of #1135: a design property on a NATIVE widget. +-- +-- `alter page … set '' = … on ` cannot write one — that +-- is a real capability gap, still open. What is fixed is the dead end it used to +-- be: the error said `property "Remove empty text" not found (widget has no +-- pluggable Object)`, which names an mxcli internal and omits the route that +-- exists. Run this against a page with a List View named lvThings to see it: +-- +-- alter page MyFirstModule.ThingList { set 'Remove empty text' = off on lvThings }; +-- -> property "Remove empty text" is not a property of this built-in widget — +-- `set` writes its own properties (…); for an Atlas design property use +-- `alter styling on page|snippet widget lvThings +-- set 'Remove empty text' = ` instead +-- +-- The three names in the report — 'Remove empty text', 'Remove loadmore button', +-- 'Reset list style' — are NOT List View design properties in the Atlas that +-- ships with Mendix 11. Measured on 11.12.2, themesource/atlas_core/web/ +-- design-properties.json declares exactly three for ListView: Style, Hover style +-- and Row size. Writing one of the reported names through ALTER STYLING +-- succeeds, and mxbuild then refuses the project: +-- +-- alter styling … set 'Remove empty text' = on; -> mx check: 1 error, +-- [CE6083] "Design property Remove empty text is not supported by your +-- theme." at List view 'lvThings' +-- alter styling … set 'Row size' = 'Small'; -> mx check: 0 errors +-- +-- So refusing the write was right and the reason given was wrong. Still open, +-- and tracked separately: MDL-WIDGET11 ("design property not defined for this +-- widget type") covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE +-- and NOT `ALTER STYLING`, whose widget type lives only in the stored document. +-- Until it does, an unsupported key here is silent until mxbuild says CE6083. + +alter styling on page MyFirstModule.ThingList widget lvThings + set 'Row size' = 'Small'; diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 7205b9b1a..2af3a7004 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -2786,7 +2786,7 @@ func setWidgetAttributeRefMut(widget bson.D, value any) error { func setPluggableWidgetPropertyMut(widget bson.D, propName string, value any) error { obj := bsonnav.DGetDoc(widget, "Object") if obj == nil { - return fmt.Errorf("property %q not found (widget has no pluggable Object)", propName) + return noPluggableObjectError(widget, propName) } // The same derivation buildPropKeyMap does, and it used to be spelled out a @@ -2830,6 +2830,33 @@ func setPluggableWidgetPropertyMut(widget bson.D, propName string, value any) er return fmt.Errorf("pluggable property %q not found", propName) } +// noPluggableObjectError explains a SET that reached the pluggable fallback on a +// widget that has no pluggable Object — i.e. a built-in one, whose vocabulary is +// setRawWidgetPropertyMut's switch and nothing else. +// +// The message used to be "property %q not found (widget has no pluggable +// Object)". That is true and unusable: "pluggable Object" is not something the +// author wrote, and it is not the whole truth either. An Atlas design property +// on a built-in widget — "Remove empty text" on a List View, the case reported +// as mendixlabs/mxcli#1135 — IS writable, through ALTER STYLING, which the old +// message never mentioned. A dead end that names its exit is a one-line fix for +// the reader; one that does not is a bug report. +// +// A Forms$Appearance and no Object is exactly a built-in widget, which is when +// the advice applies. A pluggable widget keeps the error that names its own +// declared keys — sending a mistyped pluggable key to ALTER STYLING would point +// at a command that cannot write it either. +func noPluggableObjectError(widget bson.D, propName string) error { + if bsonnav.DGetDoc(widget, "Appearance") == nil { + return fmt.Errorf("property %q not found on this widget", propName) + } + return fmt.Errorf("property %q is not a property of this built-in widget — "+ + "`set` writes its own properties (Caption, Class, Style, DynamicClasses, "+ + "Visible, Editable, …); for an Atlas design property use "+ + "`alter styling on page|snippet widget %s set '%s' = ` instead", + propName, bsonnav.DGetString(widget, "Name"), propName) +} + // setTranslatableText sets a translatable text value in BSON. func setTranslatableText(parent bson.D, key string, value any) { strVal, ok := value.(string) diff --git a/mdl/backend/pagemutator/native_set_diagnostic_test.go b/mdl/backend/pagemutator/native_set_diagnostic_test.go new file mode 100644 index 000000000..9f0b2b846 --- /dev/null +++ b/mdl/backend/pagemutator/native_set_diagnostic_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "strings" + "testing" +) + +// mendixlabs/mxcli#1135, route 1. +// +// `alter page … set 'Remove empty text' = off on lvThings` on a native List View +// dead-ends. The write is genuinely not supported on this path — but the message +// it dead-ends with describes mxcli's internals rather than the author's +// options: +// +// property "Remove empty text" not found (widget has no pluggable Object) +// +// "pluggable Object" is not a thing the author wrote, cannot be made true by +// editing the script, and — most of the point — is not the whole truth: the +// design property IS writable, through ALTER STYLING. Measured on a blank +// 11.12.2 project, the statement the old message did not mention: +// +// alter styling on page MyFirstModule.ThingList widget lvThings +// set 'Remove empty text' = on; +// -> Updated styling on widget "lvThings" in page MyFirstModule.ThingList +// describe styling -> DesignProperties: ['Remove empty text': on] +// +// So the error names that route. A widget carrying a Forms$Appearance and no +// pluggable Object is exactly a built-in one, which is when the advice applies. +func TestSetWidgetProperty_NativeWidgetErrorNamesTheStylingRoute(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("lvThings")) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + + err := m.SetWidgetProperty("lvThings", "Remove empty text", false) + if err == nil { + t.Fatal("setting an unknown property on a built-in widget must still fail") + } + msg := err.Error() + if strings.Contains(msg, "pluggable Object") { + t.Errorf("the message still describes mxcli's internals: %q", msg) + } + if !strings.Contains(msg, "alter styling") { + t.Errorf("the message does not name the route that works: %q", msg) + } + if !strings.Contains(msg, "Remove empty text") { + t.Errorf("the message does not name the property: %q", msg) + } +} + +// The control: a widget that IS pluggable must keep the error that names its own +// vocabulary. Redirecting a mistyped pluggable key to ALTER STYLING would send +// the author to a command that cannot write it either. +func TestSetWidgetProperty_PluggableWidgetKeepsItsOwnError(t *testing.T) { + rawData := makeRawPage(makePluggableWidget("dg1", "pageSize", "10")) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + + err := m.SetWidgetProperty("dg1", "NoSuchProperty", 1) + if err == nil { + t.Fatal("an unknown pluggable property must fail") + } + if strings.Contains(err.Error(), "alter styling") { + t.Errorf("a pluggable property was redirected to ALTER STYLING: %q", err.Error()) + } +} From 81b3e6212d8ffb4a5fb2f0fd9591c32cb77cff3a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 05:23:43 +0000 Subject: [PATCH 03/17] docs: point the #1135 follow-up at the issue that tracks it The bug test and the finding both said the ALTER STYLING design-property gap was "tracked separately" without saying where, which is how a note goes stale. It is ako/mxcli#509. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .claude/skills/fix-issue/findings/mdl-executor.jsonl | 2 +- .../widgets-1135-installed-widget-without-widget-init.mdl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 5c43baaba..0293fb36e 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -639,4 +639,4 @@ {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY MICROFLOW` re-enables concurrent execution on a microflow that disallowed it \u2014 the running app's concurrency protection removed \u2014 and drops the concurrency error message (all translations) and error microflow, plus `MarkAsUsed`. Every checker is green: **CE4899 fires only on disallow-without-a-message, never on allow**, so the one error that exists in this area is exactly the one the reset switches off", "cause": "`buildMicroflowFromStmt` built the rebuild struct with `AllowConcurrentExecution: true` and `MarkAsUsed: false` literals, and `microflowToGen` wrote `SetConcurrencyErrorMicroflowQualifiedName(\"\")` + a bare `genTexts.NewText()`. The backend already READ the two flags back (the #723 \u00a7A fix), so the round-trip test passed while the bug was live \u2014 the executor overwrote them before the backend ever saw them", "file": "`mdl/executor/cmd_microflows_build.go` (buildMicroflowFromStmt), `mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `sdk/microflows/microflows.go`", "fix": "Carry all four from the stored microflow, seeding the locals with the NEW-microflow defaults (true/false) so no separate preserve flag is needed. The error message reuses the existing `textFromGen`/`textToGen` pair, so translations survive; nil still emits the bare empty `Texts$Text` the writer always wrote", "insight": "**A passing round-trip test at one layer says nothing about the layer above it.** `TestMicroflowRoundTrip_ConcurrentExecutionFlags` had guarded these two flags since #723 and was green throughout, because the executor's rebuild struct overwrites them before calling the backend. When a property is reset, locate the LAST writer on the path, not the first one that looks responsible. **And check which way a reset goes**: #723's backend bug wrote the Go zero value (allow -> disallow) and hit CE4899 immediately; the executor's literal writes the opposite (disallow -> allow), and the same CE4899 that caught the first direction is structurally blind to the second. A checker that catches a property's loss in one direction is not coverage for that property. Two methodological traps in the test itself, both hit: `bytes.Equal` on two encodes of the same microflow ALWAYS differs (fresh random sub-element `$ID`s \u2014 the reason `canon` exists), and `canon.Equal` on a whole microflow always differs too, because `StableId` is a fresh GUID *value* per encode and `Equal` does not mask \u2014 only `Reconcile` may be asked that question. Compare the sub-element under test, or use Reconcile. Controls: hardcoding the executor literals back, emptying the writer's pair, and stubbing the reader each fail a different test with the reported symptom"} {"area":"mdl/executor","date":"2026-09-17","symptom":"`DESCRIBE ENUMERATION Mod.E` prints every value with an empty caption (`MyValue ''`) although Studio Pro shows them. Re-executing that output then DESTROYS the real captions (exec reports \"Modified enumeration\" and the stored Texts$Translation goes empty). Reported on Windows, single-language project, v0.18.0 and v0.22.0","cause":"The read asked `v.Caption.GetTranslation(\"en_US\")`. Mendix has no language-neutral text: a project whose DefaultLanguageCode is nl_NL stores the caption under nl_NL and nothing else, so the lookup misses and returns \"\". #970 fixed the WRITE side to use the project language and #702 fixed the widget READ side; the enumeration/validation-rule/message-template reads were the sites neither sweep reached","file":"`mdl/executor/cmd_enumerations.go` (describeEnumeration), `cmd_diff_mdl.go` (enumerationToMDL), `describe_language.go` (pickTextTranslation's fallback now sorts), `mdl/catalog/language.go` + `builder_modules.go`","insight":"**Reproduce it with mxcli alone — no Studio Pro and no non-English project needed.** `ALTER SETTINGS LANGUAGE ADD OR MODIFY 'nl_NL' (...); ALTER SETTINGS LANGUAGE DefaultLanguageCode = 'nl_NL';` on a copy of any fixture, then CREATE the enumeration: the write side already honours the project language, so the captions land under nl_NL and DESCRIBE reads '' immediately. That also gives the impact control for free — feed the '' output back through exec and grep the .mxunit for `Texts$Translation LanguageCode nl_NL Text ` with nothing after it. **The plausible wrong turn to skip**: suspecting the codec or a gen storage-name mismatch. `EnumerationValue.Caption` is NOT in keyaudit_test.go and the strings are plainly visible in the unit — dump the .mxunit with a printable-ASCII regex FIRST (one command) and the language code tells you it is a read-side language bug, not a decode bug. **The fallback has to sort**: `for _, v := range t.Translations` returns a different language per run, so a multi-language project's DESCRIBE output was undiffable — a bug that a single-language repro can never show.","refs":["mendixlabs/mxcli#1113","mendixlabs/mxcli#970","mendixlabs/mxcli#702"],"ce":[]} {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} -{"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11: themesource/atlas_core/web/design-properties.json declares Style, Hover style, Row size and nothing else. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open and separate: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} +{"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11: themesource/atlas_core/web/design-properties.json declares Style, Hover style, Row size and nothing else. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} diff --git a/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl index d0425d44c..7916157f7 100644 --- a/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl +++ b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl @@ -70,7 +70,7 @@ create or replace page MyFirstModule.HtmlDemo ( -- alter styling … set 'Row size' = 'Small'; -> mx check: 0 errors -- -- So refusing the write was right and the reason given was wrong. Still open, --- and tracked separately: MDL-WIDGET11 ("design property not defined for this +-- tracked as ako/mxcli#509: MDL-WIDGET11 ("design property not defined for this -- widget type") covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE -- and NOT `ALTER STYLING`, whose widget type lives only in the stored document. -- Until it does, an unsupported key here is silent until mxbuild says CE6083. From 62e175657a147b67179fcefde8123f155b08e3c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 05:48:48 +0000 Subject: [PATCH 04/17] fix(odata): type Edm.TimeOfDay, and refuse an external action Mendix cannot call (mendixlabs/mxcli#1089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CE7252 "The parameters for remote action '' have changed" survived DROP + CREATE OR REPLACE MICROFLOW and CREATE OR MODIFY EXTERNAL ENTITIES, and was reported as a stale parameter fingerprint needing a REFRESH ODATA CLIENT command. No fingerprint is stored — Mendix re-derives the alignment from the consumed service's cached contract on every build — so recreating the microflow really does rewrite everything the call holds, and its failure means the rewrite was equally wrong, or the action is not callable at all. Both turned out to be true, on different types. Measured on mxbuild 11.12.0, one action per EDM shape written by mxcli into a real app and checked on its own: String Guid Boolean Byte SByte Int16 Int32 Int64 Decimal 0 errors Double Single Date DateTime DateTimeOffset 0 errors an EnumType, an EntityType, Collection(EntityType) 0 errors Edm.TimeOfDay CE7252 / CE7269 Edm.Duration Stream Binary Geography* CE7255 (+CE7253) a ComplexType, a TypeDefinition, Collection(Edm.*) CE7255 (+CE7253) Edm.TimeOfDay is the only type that failed without Mendix also calling it unsupported, which is what identifies it as mxcli's gap rather than the platform's: it was unmapped, so the call was written with no ParameterType or no VariableDataType, and neither field is reachable from MDL — both are derived from the contract, never typed by the developer. It is now DateTime. Everything Mendix answers with CE7255 is beyond any write. Those statements executed silently and left a project that could not build, which is the dead end in the report; they are now refused, naming CE7255 and saying outright that no MDL clears it. Edm.Binary loses its mapping for the same reason — it mapped cleanly to DataTypes$BinaryType and Mendix rejects the action regardless, so a kind there only routed it past the refusal. A third case sat between the two: an entity-typed PARAMETER whose external entity was never imported. That one is fixable and the remedy is one statement, which mxcli already named for a RETURN type and not for a parameter — the parameter side being the one that reports as CE7252. The rule is applied by `mxcli check --references` and by the writer, from one function, as CheckLayoutPlaceholderNames already is. Only the checker would have missed the reported workflow: `mxcli exec` does not run the project-resolved reference pass, so the statement would still have executed. An enum-typed parameter builds at 0 errors with no type written at all, so "mxcli could not name a Mendix type for it" is not on its own grounds to refuse; the accepts-what-builds test carries that control. Controls: reverting the TimeOfDay mapping takes the probe app 0 -> 2 errors, CE7252 on the parameter and CE7269 on the return, the reported symptom verbatim; stubbing the type check restores the silent write. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TH4k6nnB86KpiuWU9x68Ba --- .../fix-issue/findings/mdl-executor.jsonl | 1 + ...-1089-external-action-unmappable-types.mdl | 148 ++++++++ mdl/executor/cmd_microflows_builder_calls.go | 72 +++- mdl/executor/external_action_types.go | 179 +++++++++ mdl/executor/external_action_types_test.go | 355 ++++++++++++++++++ .../validate_external_action_calls.go | 10 + mdl/types/edmx.go | 21 ++ 7 files changed, 783 insertions(+), 3 deletions(-) create mode 100644 mdl-examples/bug-tests/odata-1089-external-action-unmappable-types.mdl create mode 100644 mdl/executor/external_action_types.go create mode 100644 mdl/executor/external_action_types_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 02a047ee9..2dafd7c10 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -639,3 +639,4 @@ {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY MICROFLOW` re-enables concurrent execution on a microflow that disallowed it \u2014 the running app's concurrency protection removed \u2014 and drops the concurrency error message (all translations) and error microflow, plus `MarkAsUsed`. Every checker is green: **CE4899 fires only on disallow-without-a-message, never on allow**, so the one error that exists in this area is exactly the one the reset switches off", "cause": "`buildMicroflowFromStmt` built the rebuild struct with `AllowConcurrentExecution: true` and `MarkAsUsed: false` literals, and `microflowToGen` wrote `SetConcurrencyErrorMicroflowQualifiedName(\"\")` + a bare `genTexts.NewText()`. The backend already READ the two flags back (the #723 \u00a7A fix), so the round-trip test passed while the bug was live \u2014 the executor overwrote them before the backend ever saw them", "file": "`mdl/executor/cmd_microflows_build.go` (buildMicroflowFromStmt), `mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `sdk/microflows/microflows.go`", "fix": "Carry all four from the stored microflow, seeding the locals with the NEW-microflow defaults (true/false) so no separate preserve flag is needed. The error message reuses the existing `textFromGen`/`textToGen` pair, so translations survive; nil still emits the bare empty `Texts$Text` the writer always wrote", "insight": "**A passing round-trip test at one layer says nothing about the layer above it.** `TestMicroflowRoundTrip_ConcurrentExecutionFlags` had guarded these two flags since #723 and was green throughout, because the executor's rebuild struct overwrites them before calling the backend. When a property is reset, locate the LAST writer on the path, not the first one that looks responsible. **And check which way a reset goes**: #723's backend bug wrote the Go zero value (allow -> disallow) and hit CE4899 immediately; the executor's literal writes the opposite (disallow -> allow), and the same CE4899 that caught the first direction is structurally blind to the second. A checker that catches a property's loss in one direction is not coverage for that property. Two methodological traps in the test itself, both hit: `bytes.Equal` on two encodes of the same microflow ALWAYS differs (fresh random sub-element `$ID`s \u2014 the reason `canon` exists), and `canon.Equal` on a whole microflow always differs too, because `StableId` is a fresh GUID *value* per encode and `Equal` does not mask \u2014 only `Reconcile` may be asked that question. Compare the sub-element under test, or use Reconcile. Controls: hardcoding the executor literals back, emptying the writer's pair, and stubbing the reader each fail a different test with the reported symptom"} {"area":"mdl/executor","date":"2026-09-17","symptom":"`DESCRIBE ENUMERATION Mod.E` prints every value with an empty caption (`MyValue ''`) although Studio Pro shows them. Re-executing that output then DESTROYS the real captions (exec reports \"Modified enumeration\" and the stored Texts$Translation goes empty). Reported on Windows, single-language project, v0.18.0 and v0.22.0","cause":"The read asked `v.Caption.GetTranslation(\"en_US\")`. Mendix has no language-neutral text: a project whose DefaultLanguageCode is nl_NL stores the caption under nl_NL and nothing else, so the lookup misses and returns \"\". #970 fixed the WRITE side to use the project language and #702 fixed the widget READ side; the enumeration/validation-rule/message-template reads were the sites neither sweep reached","file":"`mdl/executor/cmd_enumerations.go` (describeEnumeration), `cmd_diff_mdl.go` (enumerationToMDL), `describe_language.go` (pickTextTranslation's fallback now sorts), `mdl/catalog/language.go` + `builder_modules.go`","insight":"**Reproduce it with mxcli alone — no Studio Pro and no non-English project needed.** `ALTER SETTINGS LANGUAGE ADD OR MODIFY 'nl_NL' (...); ALTER SETTINGS LANGUAGE DefaultLanguageCode = 'nl_NL';` on a copy of any fixture, then CREATE the enumeration: the write side already honours the project language, so the captions land under nl_NL and DESCRIBE reads '' immediately. That also gives the impact control for free — feed the '' output back through exec and grep the .mxunit for `Texts$Translation LanguageCode nl_NL Text ` with nothing after it. **The plausible wrong turn to skip**: suspecting the codec or a gen storage-name mismatch. `EnumerationValue.Caption` is NOT in keyaudit_test.go and the strings are plainly visible in the unit — dump the .mxunit with a printable-ASCII regex FIRST (one command) and the language code tells you it is a read-side language bug, not a decode bug. **The fallback has to sort**: `for _, v := range t.Translations` returns a different language per run, so a multi-language project's DESCRIBE output was undiffable — a bug that a single-language repro can never show.","refs":["mendixlabs/mxcli#1113","mendixlabs/mxcli#970","mendixlabs/mxcli#702"],"ce":[]} {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} +{"area":"mdl/executor","date":"2026-09-18","symptom":"CE7252 \"The parameters for remote action '' have changed\" (and CE7269 on the return) survives DROP + CREATE OR REPLACE MICROFLOW and CREATE OR MODIFY EXTERNAL ENTITIES, with no MDL that clears it. Reported as a stale 'parameter fingerprint / BSON hash' in the OData client and a request for REFRESH ODATA CLIENT Module.Client ACTIONS. Third report of this CE code after #1020 and #1073.","cause":"Two defects the same code was hiding. (1) edmReturnTypeToKind did not map Edm.TimeOfDay, so the call was written with no ParameterType / no VariableDataType - CE7252 on a parameter, CE7269 on a return - and neither field is reachable from MDL, both being derived from the contract. (2) For the types Mendix itself refuses (Edm.Duration/Stream/Binary/Geography*, a ComplexType, a TypeDefinition, Collection(Edm.*)) mxcli accepted the statement silently and left an unbuildable project; Mendix answers CE7255 'Action of service is not supported', which no BSON can change. A third case sat between them: an entity-typed PARAMETER whose external entity was never imported was unreported, though the identical case on the RETURN type already named the import statement.","file":"mdl/executor/external_action_types.go (new: classifyExternalActionType, checkExternalActionTypes); mdl/executor/cmd_microflows_builder_calls.go (edmReturnTypeToKind + refuseUntypableExternalAction); mdl/executor/validate_external_action_calls.go; mdl/types/edmx.go (FindEnumType)","insight":"**Enumerate the type space against mxbuild instead of theorising about the CE code.** One action per EDM shape, written by mxcli into a real 11.12.0 app and checked, produced a truth table in two runs, and the table is what separates OUR bug from the platform's: Edm.TimeOfDay is the only type that failed WITHOUT Mendix also calling it unsupported (CE7253/CE7255). Reasoning could not have reached that, and neither could a deny-list written from documentation. **The same table kills the obvious over-fix**: an ENUM-typed parameter builds at 0 errors with no type written at all, so 'mxcli could not name a Mendix type' is not on its own grounds to refuse - a refusal keyed on that would have rejected calls that build. **A rule wired only into the reference pass would have missed the reported workflow**: `mxcli exec` runs ValidateProgram(prog, projectPath) (the no-project linter), NOT Executor.ValidateProgram, so only `check --references` sees it; the refusal is applied at the writer too, from the same function, as CheckLayoutPlaceholderNames already does. Verified by building the faulty binary and watching exec write the call anyway. **There is no fingerprint** - the third reporter in a row believed one was stored; Mendix re-derives alignment from the cached contract every build, so 'recreate the microflow' really does rewrite everything and its failure means the rewrite is equally wrong, or the action is uncallable. **Controls**: reverting the TimeOfDay mapping alone takes the probe app 0 -> 2 errors (CE7252 + CE7269, the reported symptom verbatim) and, because the two halves are coupled, ALSO makes the refusal misfire on a call that builds - which is what the accepts-what-builds test catches. Stubbing checkExternalActionTypes to return nil restores the silent write. Repro mdl-examples/bug-tests/odata-1089-external-action-unmappable-types.mdl","refs":["mendixlabs/mxcli#1089","mendixlabs/mxcli#1073","mendixlabs/mxcli#1020"],"ce":["CE7252","CE7269","CE7255","CE7253"]} diff --git a/mdl-examples/bug-tests/odata-1089-external-action-unmappable-types.mdl b/mdl-examples/bug-tests/odata-1089-external-action-unmappable-types.mdl new file mode 100644 index 000000000..9d35dba92 --- /dev/null +++ b/mdl-examples/bug-tests/odata-1089-external-action-unmappable-types.mdl @@ -0,0 +1,148 @@ +-- ============================================================================ +-- mendixlabs/mxcli#1089: CE7252 with no MDL that clears it +-- ============================================================================ +-- +-- Report, verbatim: "When a CALL EXTERNAL ACTION activity is linked to a +-- consumed OData action whose parameter fingerprint (BSON hash) has become +-- stale, mxbuild reports CE7252 [...] mxcli has no equivalent command." Four +-- levers were tried — DROP + CREATE OR REPLACE MICROFLOW, CREATE OR MODIFY +-- EXTERNAL ENTITIES, and two invented statements (UPDATE ODATA CLIENT, SYNC +-- EXTERNAL ACTION) — and the request was for +-- +-- REFRESH ODATA CLIENT Module.Client ACTIONS; +-- +-- There is nothing to refresh. No fingerprint and no hash is stored: Mendix +-- re-derives the alignment from the consumed service's cached contract on every +-- build, so recreating the microflow really does rewrite everything the call +-- holds. If CE7252 survives that, the call was rewritten just as wrong — or the +-- contract makes the action uncallable, and no statement of any name helps. +-- +-- Both turned out to be true, on different types. +-- +-- --------------------------------------------------------------------------- +-- THE MEASUREMENT. One action per type shape, each written by mxcli into a real +-- 11.12.0 app (`mxcli new Repro1089 --version 11.12.0`) with the contract +-- reproduced below served from `python3 -m http.server`, then `mxcli docker +-- check` run on the result: +-- +-- Edm.String Guid Boolean Byte SByte Int16 Int32 Int64 0 errors +-- Edm.Decimal Double Single Date DateTime DateTimeOffset 0 errors +-- an EnumType, an EntityType, Collection(EntityType) 0 errors +-- Edm.TimeOfDay CE7252 / CE7269 +-- Edm.Duration, Edm.Stream, Edm.Binary, Edm.Geography* CE7255 (+CE7253) +-- a ComplexType, a TypeDefinition, Collection(Edm.*) CE7255 (+CE7253) +-- +-- Two separate defects, and the table is what separates them: +-- +-- 1. Edm.TimeOfDay is the ONLY type that failed without Mendix also calling +-- it unsupported. So it is our gap, not the platform's: mxcli's EDM table +-- did not map it, the call was written with no ParameterType (CE7252) or +-- no VariableDataType (CE7269), and no MDL reaches either field — both are +-- derived from the contract, never typed by the developer. Typing it as +-- DateTime clears both. Control on the same app: 2 errors with the mapping +-- reverted, 0 with it in. +-- +-- 2. Everything Mendix answers with CE7255 "Action '' of service '' +-- is not supported" is beyond any write. mxcli accepted those statements +-- silently and left a project that cannot build, which is precisely the +-- dead end in the report. They are now refused, naming CE7255 and saying +-- outright that no MDL clears it. +-- +-- A third case sits between them: an entity-typed parameter whose external +-- entity has not been imported. That one IS fixable, the remedy is one +-- statement, and mxcli said it for a RETURN type and not for a PARAMETER — the +-- parameter side being the one that reports as CE7252. +-- +-- WHERE THE RULE IS APPLIED. Both `mxcli check --references` and the writer, from +-- one function (external_action_types.go). Only the checker would have missed the +-- reported workflow entirely: `mxcli exec` does not run the project-resolved +-- reference pass, so the statement would still have executed and still have left +-- CE7252 behind. +-- +-- --------------------------------------------------------------------------- +-- The contract this script assumes (import as Ext.Probe). Every unbound action +-- needs an or Mendix does not consider it callable at all +-- (CE7251). +-- --------------------------------------------------------------------------- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- Verify: +-- mxcli exec -p app.mpr -> the four blocks below execute +-- mxcli docker check -p app.mpr -> 0 errors +-- then uncomment the REFUSED block -> exec stops, naming CE7255/CE7252 +-- ============================================================================ + +-- --------------------------------------------------------------------------- +-- The fix: a TimeOfDay parameter and a TimeOfDay return. Both were CE7252 / +-- CE7269 before, with no statement that cleared either. +-- --------------------------------------------------------------------------- +create or modify microflow Ext.ACT_1089_TimeOfDayParam () +begin + call external action Ext.Probe.PTimeOfDay(v = empty); + return; +end; + +create or modify microflow Ext.ACT_1089_TimeOfDayReturn () +begin + $When = call external action Ext.Probe.RTimeOfDay(); + return; +end; + +-- --------------------------------------------------------------------------- +-- The controls for the refusal. Both build at 0 errors and must keep doing so: +-- an ENUM-typed parameter is written with no type at all, so "mxcli could not +-- name a Mendix type for it" is not on its own grounds to refuse anything. +-- --------------------------------------------------------------------------- +create or modify microflow Ext.ACT_1089_EnumParam () +begin + call external action Ext.Probe.PEnum(v = empty); + return; +end; + +create or modify microflow Ext.ACT_1089_EntityParam () +begin + call external action Ext.Probe.PEntity(v = empty); + return; +end; + +-- --------------------------------------------------------------------------- +-- REFUSED, deliberately — uncomment one at a time to see the message. These are +-- what the report ran into. Left commented so the file executes end to end. +-- +-- PDuration Mendix does not support the type: CE7255, and nothing mxcli +-- writes changes that. The message says so rather than leaving +-- the reader looking for the refresh command that would fix it. +-- PComplex the same, for a ComplexType. +-- PUnimported fixable, and the remedy is named: +-- create or modify external entities from Ext.Probe entities (Ghost) +-- --------------------------------------------------------------------------- +-- create or modify microflow Ext.ACT_1089_Refused_Duration () +-- begin +-- call external action Ext.Probe.PDuration(v = empty); +-- return; +-- end; +-- +-- create or modify microflow Ext.ACT_1089_Refused_Complex () +-- begin +-- call external action Ext.Probe.PComplex(v = empty); +-- return; +-- end; +-- +-- create or modify microflow Ext.ACT_1089_Refused_Unimported () +-- begin +-- call external action Ext.Probe.PUnimported(v = empty); +-- return; +-- end; diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index 5d0a76e22..e4f9c7f5d 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -901,11 +901,20 @@ func edmReturnTypeToKind(edmType string) string { return "Long" case "Edm.Decimal", "Edm.Double", "Edm.Single": return "Decimal" - case "Edm.DateTime", "Edm.DateTimeOffset", "Edm.Date": + case "Edm.DateTime", "Edm.DateTimeOffset", "Edm.Date", "Edm.TimeOfDay": + // Edm.TimeOfDay is the one primitive Mendix accepts on an external + // action that mxcli did not map, so the call was written with no type + // at all and no MDL reached the field: CE7252 on a parameter, CE7269 on + // a return (mendixlabs/mxcli#1089). Measured on mxbuild 11.12.0 — + // typing it as DateTime clears both. return "DateTime" - case "Edm.Binary": - return "Binary" default: + // Edm.Binary is deliberately absent, though it maps cleanly to + // DataTypes$BinaryType: Mendix rejects an external action carrying one + // as CE7255 "Action '' ... is not supported", whatever mxcli writes. + // Returning a kind here would route it past the refusal in + // checkExternalActionTypes and back to the unfixable CE7252 of #1089. + // // Not a primitive. Entity-typed and collection returns are resolved by // resolveExternalActionReturnEntity, which needs the project to map the // contract's type onto the entity imported for it. Complex types @@ -914,9 +923,66 @@ func edmReturnTypeToKind(edmType string) string { } } +// externalActionContract returns the called service's cached contract and the +// action within it, or nils when either cannot be resolved. An unresolvable +// service or action is reported by the reference validation, not here. +func (fb *flowBuilder) externalActionContract(serviceRef ast.QualifiedName, actionName string) (*types.EdmxDocument, *types.EdmAction) { + if fb.backend == nil { + return nil, nil + } + services, err := fb.backend.ListConsumedODataServices() + if err != nil { + return nil, nil + } + for _, svc := range services { + modName := fb.hierarchy.GetModuleName(fb.hierarchy.FindModuleID(svc.ContainerID)) + if !strings.EqualFold(modName, serviceRef.Module) || !strings.EqualFold(svc.Name, serviceRef.Name) { + continue + } + if svc.Metadata == "" { + return nil, nil + } + doc, err := types.ParseEdmx(svc.Metadata) + if err != nil { + return nil, nil + } + for _, act := range doc.Actions { + if strings.EqualFold(act.Name, actionName) { + return doc, act + } + } + return doc, nil + } + return nil, nil +} + +// refuseUntypableExternalAction applies the type rule at the WRITE choke point, +// from the same function `mxcli check --references` calls. +// +// Both tiers are needed and neither substitutes for the other: `exec` does not +// run the project-resolved reference pass, so without this the statement still +// executed silently and left a project whose build fails with CE7252 and no MDL +// to clear it — which is the whole of mendixlabs/mxcli#1089. This is the shape +// CheckLayoutPlaceholderNames already has: one rule, applied by the checker and +// by the writer. +func (fb *flowBuilder) refuseUntypableExternalAction(s *ast.CallExternalActionStmt) { + doc, action := fb.externalActionContract(s.ServiceName, s.ActionName) + if doc == nil || action == nil { + return + } + svcQN := s.ServiceName.String() + err := checkExternalActionTypes(externalCall{stmt: s}, action, doc, svcQN, func(remoteName string) string { + return fb.findExternalEntityFor(svcQN, remoteName) + }) + if err != nil { + fb.addError("%s", err.Error()) + } +} + // addCallExternalActionAction creates a CALL EXTERNAL ACTION statement. func (fb *flowBuilder) addCallExternalActionAction(s *ast.CallExternalActionStmt) model.ID { serviceQN := s.ServiceName.Module + "." + s.ServiceName.Name + fb.refuseUntypableExternalAction(s) returnKind, returnEntity := fb.resolveExternalActionReturnKind(s.ServiceName, s.ActionName) // Build parameter mappings. Each carries the parameter's TYPE as well as its diff --git a/mdl/executor/external_action_types.go b/mdl/executor/external_action_types.go new file mode 100644 index 000000000..bc238b5c2 --- /dev/null +++ b/mdl/executor/external_action_types.go @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Classification of the types an external action's parameters and return value +// can have, and the refusal that follows from it. +// +// This is the third bug reported as CE7252 (mendixlabs/mxcli#1020, #1073, +// #1089) and the first whose answer is partly "you cannot". Every one of them +// arrived as "the stored fingerprint is stale, give me a command to refresh +// it". There is no fingerprint: Mendix re-derives the alignment from the +// consumed service's cached contract on every build, so a call it rejects is a +// call that was written wrong — or one the contract makes impossible. +// +// Measured on mxbuild 11.12.0, one action per shape, each written by mxcli and +// checked on its own (the probe contract is reproduced in the test file): +// +// Edm.String Guid Boolean Byte SByte Int16 Int32 Int64 0 errors +// Edm.Decimal Double Single Date DateTime DateTimeOffset 0 errors +// an EnumType, an EntityType, Collection(EntityType) 0 errors +// Edm.TimeOfDay CE7252 / CE7269 +// Edm.Duration Stream Binary Geography* CE7255 (+CE7253) +// a ComplexType, a TypeDefinition, Collection(Edm.*) CE7255 (+CE7253) +// +// Two facts drive everything below, and neither is guessable: +// +// 1. Edm.TimeOfDay is SUPPORTED. It is the only type that failed without +// Mendix also calling it unsupported, which is what identifies it as our +// gap rather than the platform's. Typing it as DateTime clears both codes. +// +// 2. An ENUM-typed parameter builds with no type written at all. So "mxcli +// could not name a Mendix type for it" is not on its own grounds to refuse +// — lumping enums in with complex types would refuse a call that builds. +// +// What Mendix will not take, it states as CE7255 "Action '' of service +// '' is not supported", with CE7252/CE7269 riding along. No BSON mxcli can +// write changes that, so the statement is refused here instead of executing and +// leaving an unbuildable project with no way back. +package executor + +import ( + "fmt" + + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/types" +) + +// externalActionTypeClass is what mxcli can do with one parameter or return type. +type externalActionTypeClass int + +const ( + // extTypeRepresentable: the call can be written. kind names the Mendix type + // for a primitive, and is empty for an enum, which needs none. + extTypeRepresentable externalActionTypeClass = iota + // extTypeEntity: an entity type. Whether the call can be written depends on + // the PROJECT — the external entity must have been imported — so this class + // is resolved by the caller, not here. + extTypeEntity + // extTypeUnsupported: Mendix refuses the action itself (CE7255). Nothing + // mxcli writes can change that. + extTypeUnsupported +) + +// externalActionType is one classified parameter or return type. +type externalActionType struct { + class externalActionTypeClass + kind string // Mendix kind ("String", "DateTime", …); empty for enum/entity + entity string // bare entity type name, for extTypeEntity + isList bool // the contract said Collection(...) +} + +// classifyExternalActionType decides what an action's parameter or return type +// is, against the contract it was declared in. +// +// The contract is needed because the EDM name alone does not say which of the +// three a `Probe.Thing` is: an entity type (representable), an enum +// (representable, and needs no type written) or a complex type / type +// definition (which Mendix refuses outright). A name the contract does not +// declare at all is refused for the same reason it is not resolvable. +func classifyExternalActionType(doc *types.EdmxDocument, edmType string) externalActionType { + if kind := edmReturnTypeToKind(edmType); kind != "" { + return externalActionType{class: extTypeRepresentable, kind: kind} + } + + bare, isList := edmBareTypeName(edmType) + if bare == "" { + // An Edm.* primitive that the table above does not map, or a collection + // of one. Both are CE7255; Collection(Edm.String) is measured. + return externalActionType{class: extTypeUnsupported} + } + if doc != nil { + if doc.FindEntityType(bare) != nil { + return externalActionType{class: extTypeEntity, entity: bare, isList: isList} + } + if doc.FindEnumType(bare) != nil && !isList { + // Collection(enum) is deliberately NOT included: every collection of + // a non-entity type measured as CE7255, and nothing here was + // measured for the enum case, so it is refused rather than assumed. + return externalActionType{class: extTypeRepresentable} + } + } + return externalActionType{class: extTypeUnsupported} +} + +// flowPrefix names the flow the call sits in, when the caller knows it. The +// writer does not — its errors are already reported under the statement — so +// the same message serves both callers rather than a second copy in the +// builder's currency. Two copies of a rule in two currencies is how a resolver +// drifts, which is the reason this is one function and not two. +func (c externalCall) flowPrefix() string { + if c.flow == "" { + return "" + } + return c.flow + ": " +} + +// checkExternalActionTypes refuses a call whose parameter or return types the +// contract makes unwritable, and says which of the two kinds of unwritable it is. +// +// importedEntity maps a remote type name onto the qualified name of the external +// entity imported for it, or "" when none has been. It is a function so the rule +// can be tested without a project. +// +// A BOUND action's first parameter is its binding parameter, supplied by Mendix +// from the object the action is called on. It is skipped for the same reason +// checkExternalActionParameters skips it: it is not the statement's to supply, +// so it is not the statement's to be refused over. +func checkExternalActionTypes( + c externalCall, + action *types.EdmAction, + doc *types.EdmxDocument, + svcQN string, + importedEntity func(remoteName string) string, +) error { + for i, p := range action.Parameters { + if action.IsBound && i == 0 { + continue + } + t := classifyExternalActionType(doc, p.Type) + switch t.class { + case extTypeUnsupported: + return unsupportedExternalActionType(c, action, svcQN, + fmt.Sprintf("parameter %q is %s", p.Name, p.Type), + "One of the parameters is not supported") + case extTypeEntity: + if importedEntity(t.entity) != "" { + continue + } + return mdlerrors.NewValidation(fmt.Sprintf( + "%sexternal action %q takes parameter %q of %s, but no external entity has been "+ + "imported for that type, so the argument cannot be typed.\n"+ + " Mendix reports this as CE7252 \"The parameters for remote action '%s' have changed\".\n"+ + " Import it first: create or modify external entities from %s entities (%s)", + c.flowPrefix(), action.Name, p.Name, p.Type, action.Name, svcQN, t.entity)) + } + } + + if t := classifyExternalActionType(doc, action.ReturnType); t.class == extTypeUnsupported { + return unsupportedExternalActionType(c, action, svcQN, + fmt.Sprintf("it returns %s", action.ReturnType), + "This action's return type is not supported") + } + // An entity-typed return with no imported entity is reported by + // checkExternalActionReturn, which already names the import statement. + return nil +} + +// unsupportedExternalActionType is the message for the case with no remedy. It +// says so outright: the report this exists to answer spent its effort looking +// for the mxcli command that would clear the error, and the useful answer is +// that the action is not callable from a microflow at all. +func unsupportedExternalActionType(c externalCall, action *types.EdmAction, svcQN, detail, mendixReason string) error { + return mdlerrors.NewValidation(fmt.Sprintf( + "%sexternal action %q cannot be called from a microflow — %s, which Mendix does not support "+ + "on a call external action.\n"+ + " Mendix reports this as CE7255 \"Action '%s' of service '%s' is not supported. %s\" "+ + "(with CE7253 on the parameter, and CE7252/CE7269 on the call itself).\n"+ + " No MDL clears it: the limitation is the service contract's, not the call's. Use a "+ + "different action, or reach the operation over a REST call instead.", + c.flowPrefix(), action.Name, detail, action.Name, svcQN, mendixReason)) +} diff --git a/mdl/executor/external_action_types_test.go b/mdl/executor/external_action_types_test.go new file mode 100644 index 000000000..ecc44c0e4 --- /dev/null +++ b/mdl/executor/external_action_types_test.go @@ -0,0 +1,355 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// probeMetadata declares one action per PARAMETER shape and one per RETURN +// shape, so each can be measured on its own. The verdicts asserted below were +// measured on mxbuild 11.12.0 against exactly this contract: every action was +// written by mxcli and `mx check` run on the result (mendixlabs/mxcli#1089). +const probeMetadata = ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +` + +func parseProbe(t *testing.T) *types.EdmxDocument { + t.Helper() + doc, err := types.ParseEdmx(probeMetadata) + if err != nil { + t.Fatalf("parse fixture: %v", err) + } + return doc +} + +// importedPerson resolves the external entity for `Person` only — `Ghost` is the +// type nobody imported, which is the difference the messages turn on. +func importedPerson(remoteName string) string { + if strings.EqualFold(remoteName, "Person") { + return "Ext.People" + } + return "" +} + +// TestEdmTimeOfDayIsADateTime is the mapping gap behind mendixlabs/mxcli#1089. +// +// Mendix SUPPORTS Edm.TimeOfDay on an external action — measured, it is the one +// type in the whole EDM primitive set that draws neither CE7253 nor CE7255 and +// still failed. mxcli did not map it, so the call was written with no +// ParameterType (CE7252) or no VariableDataType (CE7269), and no MDL reached +// either field. Typing it as DateTime clears both: 24 errors -> 22 on the probe +// app, with nothing else changed. +func TestEdmTimeOfDayIsADateTime(t *testing.T) { + if got := edmReturnTypeToKind("Edm.TimeOfDay"); got != "DateTime" { + t.Errorf("edmReturnTypeToKind(Edm.TimeOfDay) = %q, want DateTime", got) + } +} + +// TestEdmBinaryIsNotCallable pins the other half of the same table. mxcli mapped +// Edm.Binary to DataTypes$BinaryType, which cannot help: Mendix rejects the +// ACTION, not the type mxcli chose — +// +// CE7255 "Action 'PBinary' ... is not supported. One of the parameters is not supported" +// +// so the only honest answer is to refuse the statement, and a kind here would +// route it past the refusal. +func TestEdmBinaryIsNotCallable(t *testing.T) { + if got := edmReturnTypeToKind("Edm.Binary"); got != "" { + t.Errorf("edmReturnTypeToKind(Edm.Binary) = %q, want \"\" — Mendix refuses the action (CE7255)", got) + } +} + +// TestClassifyExternalActionType is the measured truth table, one row per shape. +// Every verdict was produced by mxbuild 11.12.0 on a real project. +func TestClassifyExternalActionType(t *testing.T) { + doc := parseProbe(t) + + tests := []struct { + edmType string + wantClass externalActionTypeClass + wantKind string + wantEnt string + wantList bool + note string + }{ + // Representable primitives: 0 errors, measured. + {edmType: "", wantClass: extTypeRepresentable, wantKind: "Void", note: "an action with no return type"}, + {edmType: "Edm.String", wantClass: extTypeRepresentable, wantKind: "String"}, + {edmType: "Edm.Guid", wantClass: extTypeRepresentable, wantKind: "String"}, + {edmType: "Edm.Int64", wantClass: extTypeRepresentable, wantKind: "Long"}, + {edmType: "Edm.Date", wantClass: extTypeRepresentable, wantKind: "DateTime"}, + {edmType: "Edm.DateTimeOffset", wantClass: extTypeRepresentable, wantKind: "DateTime"}, + {edmType: "Edm.TimeOfDay", wantClass: extTypeRepresentable, wantKind: "DateTime", note: "#1089"}, + + // Mendix refuses the action outright: CE7255 (+ CE7253 on some). + {edmType: "Edm.Duration", wantClass: extTypeUnsupported}, + {edmType: "Edm.Binary", wantClass: extTypeUnsupported}, + {edmType: "Edm.Stream", wantClass: extTypeUnsupported}, + {edmType: "Edm.GeographyPoint", wantClass: extTypeUnsupported}, + {edmType: "Probe.Location", wantClass: extTypeUnsupported, note: "a ComplexType"}, + {edmType: "Probe.Alias", wantClass: extTypeUnsupported, note: "a TypeDefinition"}, + {edmType: "Collection(Edm.String)", wantClass: extTypeUnsupported, note: "a collection of primitives"}, + {edmType: "Probe.Nothing", wantClass: extTypeUnsupported, note: "a type the contract does not declare"}, + + // Enums build with no type written at all — measured 0 errors, which is + // why they must not be lumped in with the unsupported set. + {edmType: "Probe.Colour", wantClass: extTypeRepresentable}, + + // Entity types resolve against the project, not the contract. + {edmType: "Probe.Person", wantClass: extTypeEntity, wantEnt: "Person"}, + {edmType: "Collection(Probe.Person)", wantClass: extTypeEntity, wantEnt: "Person", wantList: true}, + {edmType: "Probe.Ghost", wantClass: extTypeEntity, wantEnt: "Ghost"}, + } + + for _, tt := range tests { + got := classifyExternalActionType(doc, tt.edmType) + if got.class != tt.wantClass || got.kind != tt.wantKind || got.entity != tt.wantEnt || got.isList != tt.wantList { + t.Errorf("classifyExternalActionType(%q) = %+v, want {class:%v kind:%q entity:%q isList:%v} %s", + tt.edmType, got, tt.wantClass, tt.wantKind, tt.wantEnt, tt.wantList, tt.note) + } + } +} + +func probeCall(action string, args ...string) externalCall { + var as []ast.CallArgument + for _, a := range args { + as = append(as, ast.CallArgument{Name: a}) + } + return externalCall{flow: "Ext.ACT_Probe", stmt: &ast.CallExternalActionStmt{ + ActionName: action, + Arguments: as, + }} +} + +// TestCheckExternalActionTypesRefusesUnsupported is the issue's shape: the +// statement executed, the build failed with CE7252, and no MDL cleared it — +// because the limitation is the contract's, not the call's. mxcli said nothing. +func TestCheckExternalActionTypesRefusesUnsupported(t *testing.T) { + doc := parseProbe(t) + + for _, tc := range []struct { + action string + want string // the type the message has to name + }{ + {"PDuration", "Edm.Duration"}, + {"PBinary", "Edm.Binary"}, + {"PComplex", "Probe.Location"}, + {"PStrList", "Collection(Edm.String)"}, + {"PAlias", "Probe.Alias"}, + } { + err := checkExternalActionTypes(probeCall(tc.action, "v"), actionNamed(t, doc, tc.action), doc, "Ext.Probe", importedPerson) + if err == nil { + t.Errorf("%s: an unsupported parameter type must be refused", tc.action) + continue + } + for _, want := range []string{tc.want, "CE7255", `"v"`} { + if !strings.Contains(err.Error(), want) { + t.Errorf("%s: error %q does not mention %q", tc.action, err.Error(), want) + } + } + } + + // The return side of the same table. + err := checkExternalActionTypes(probeCall("RBinary"), actionNamed(t, doc, "RBinary"), doc, "Ext.Probe", importedPerson) + if err == nil { + t.Fatal("an unsupported return type must be refused") + } + if !strings.Contains(err.Error(), "Edm.Binary") || !strings.Contains(err.Error(), "CE7255") { + t.Errorf("error %q should name the return type and CE7255", err.Error()) + } +} + +// TestCheckExternalActionTypesNamesTheImport covers the fixable case: the type +// IS supported, the external entity simply has not been imported. The remedy +// existed all along and mxcli never said it — the return side said it, the +// parameter side did not, and the parameter side is CE7252, the code the issue +// is about. +func TestCheckExternalActionTypesNamesTheImport(t *testing.T) { + doc := parseProbe(t) + + err := checkExternalActionTypes(probeCall("PUnimported", "v"), actionNamed(t, doc, "PUnimported"), doc, "Ext.Probe", importedPerson) + if err == nil { + t.Fatal("an entity-typed parameter with no imported entity must be refused") + } + for _, want := range []string{"CE7252", "Ghost", "create or modify external entities from Ext.Probe entities (Ghost)"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } +} + +// TestCheckExternalActionTypesAcceptsWhatBuilds is the control that matters. A +// refusal is only worth having if it cannot fire on a call Mendix accepts, and +// every shape here was measured at 0 errors on mxbuild 11.12.0 — including the +// two that are representable with NO type written (enum) and the bound action's +// binding parameter, which Mendix supplies rather than the statement. +func TestCheckExternalActionTypesAcceptsWhatBuilds(t *testing.T) { + doc := parseProbe(t) + + for _, action := range []string{"PEnum", "PEntity", "PEntityList", "PDate", "PTimeOfDay"} { + if err := checkExternalActionTypes(probeCall(action, "v"), actionNamed(t, doc, action), doc, "Ext.Probe", importedPerson); err != nil { + t.Errorf("%s builds at 0 errors and must not be refused: %v", action, err) + } + } + if err := checkExternalActionTypes(probeCall("RTimeOfDay"), actionNamed(t, doc, "RTimeOfDay"), doc, "Ext.Probe", importedPerson); err != nil { + t.Errorf("RTimeOfDay builds at 0 errors and must not be refused: %v", err) + } + + // A bound action's binding parameter is not the statement's to supply, and + // it is not the statement's to be refused over either. + bound := &types.EdmAction{ + Name: "Touch", + IsBound: true, + Parameters: []*types.EdmActionParameter{ + {Name: "bindingParameter", Type: "Probe.Ghost"}, + {Name: "note", Type: "Edm.String"}, + }, + } + if err := checkExternalActionTypes(probeCall("Touch", "note"), bound, doc, "Ext.Probe", importedPerson); err != nil { + t.Errorf("the binding parameter must be skipped, as it is by the name check: %v", err) + } +} + +// probeFlowBuilder runs the real builder against probeMetadata, so the write +// path is exercised rather than reproduced. +func probeFlowBuilder(t *testing.T) *flowBuilder { + t.Helper() + svcID := model.ID("svc-1") + modID := model.ID("mod-1") + mb := &mock.MockBackend{ + ListConsumedODataServicesFunc: func() ([]*model.ConsumedODataService, error) { + return []*model.ConsumedODataService{{ + BaseElement: model.BaseElement{ID: svcID}, + ContainerID: modID, + Name: "Probe", + Metadata: probeMetadata, + }}, nil + }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return nil, nil }, + } + return &flowBuilder{ + posX: 100, posY: 100, spacing: HorizontalSpacing, backend: mb, + hierarchy: &ContainerHierarchy{moduleNames: map[model.ID]string{modID: "Ext"}}, + varTypes: map[string]string{}, + declaredVars: map[string]string{}, + } +} + +func buildProbeCall(t *testing.T, script string) *flowBuilder { + t.Helper() + prog, errs := visitor.Build(script) + if len(errs) > 0 { + t.Fatalf("parsing the probe script: %v", errs) + } + call := prog.Statements[0].(*ast.CreateMicroflowStmt).Body[0].(*ast.CallExternalActionStmt) + fb := probeFlowBuilder(t) + fb.addCallExternalActionAction(call) + return fb +} + +func probeCallAction(t *testing.T, fb *flowBuilder) *microflows.CallExternalAction { + t.Helper() + for _, obj := range fb.objects { + if act, ok := obj.(*microflows.ActionActivity); ok { + if call, ok := act.Action.(*microflows.CallExternalAction); ok { + return call + } + } + } + t.Fatal("the builder produced no CallExternalAction") + return nil +} + +// TestWriterTypesTimeOfDay is the write half of the #1089 fix: the type has to +// reach the BSON, not merely pass the checker. +func TestWriterTypesTimeOfDay(t *testing.T) { + fb := buildProbeCall(t, "create microflow Ext.ACT_Probe()\nbegin\n"+ + " call external action Ext.Probe.PTimeOfDay(v = empty);\nend;") + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("a TimeOfDay parameter builds at 0 errors and must not be refused: %v", errs) + } + call := probeCallAction(t, fb) + if len(call.ParameterMappings) != 1 || call.ParameterMappings[0].ParameterDataType != "DateTime" { + t.Errorf("ParameterType = %+v, want one mapping typed DateTime — untyped is CE7252", + call.ParameterMappings) + } + + fb = buildProbeCall(t, "create microflow Ext.ACT_Probe()\nbegin\n"+ + " $r = call external action Ext.Probe.RTimeOfDay();\nend;") + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("a TimeOfDay return builds at 0 errors and must not be refused: %v", errs) + } + if got := probeCallAction(t, fb).ResultDataType; got != "DateTime" { + t.Errorf("ResultDataType = %q, want DateTime — untyped is CE7269", got) + } +} + +// TestWriterRefusesUntypableExternalAction pins the tier that the reported +// workflow actually runs. `mxcli exec` does NOT run the project-resolved +// reference pass, so a rule wired only into `check --references` still lets the +// statement write an unbuildable call — the exact sequence in the report: +// exec succeeded, `docker check` failed with CE7252, and nothing in MDL cleared it. +func TestWriterRefusesUntypableExternalAction(t *testing.T) { + for _, tc := range []struct{ action, arg, want string }{ + {"PDuration", "v = empty", "CE7255"}, + {"PComplex", "v = empty", "CE7255"}, + {"PUnimported", "v = empty", "CE7252"}, + } { + fb := buildProbeCall(t, "create microflow Ext.ACT_Probe()\nbegin\n"+ + " call external action Ext.Probe."+tc.action+"("+tc.arg+");\nend;") + errs := fb.GetErrors() + if len(errs) == 0 { + t.Errorf("%s: the writer must refuse a call it cannot type", tc.action) + continue + } + if !strings.Contains(strings.Join(errs, "\n"), tc.want) { + t.Errorf("%s: errors %v do not mention %s", tc.action, errs, tc.want) + } + } +} diff --git a/mdl/executor/validate_external_action_calls.go b/mdl/executor/validate_external_action_calls.go index fbe3a5bdb..07a6bed23 100644 --- a/mdl/executor/validate_external_action_calls.go +++ b/mdl/executor/validate_external_action_calls.go @@ -168,6 +168,16 @@ func checkExternalActionCall(ctx *ExecContext, h *ContainerHierarchy, services [ if err := checkExternalActionParameters(c, action); err != nil { return err } + // The types the contract gives those parameters, and the return type. A type + // Mendix does not support on an external action is the case with no remedy + // at all, and the one mendixlabs/mxcli#1089 was filed about: the statement + // executed, the build failed with CE7252, and every MDL the reporter reached + // for was aimed at a stored fingerprint that does not exist. + if err := checkExternalActionTypes(c, action, doc, svcQN, func(remoteName string) string { + return externalEntityFor(ctx, h, svcQN, remoteName) + }); err != nil { + return err + } return checkExternalActionReturn(ctx, h, c, action, svcQN) } diff --git a/mdl/types/edmx.go b/mdl/types/edmx.go index 0c1acf6ca..714b83c94 100644 --- a/mdl/types/edmx.go +++ b/mdl/types/edmx.go @@ -190,6 +190,27 @@ func (d *EdmxDocument) FindEntityType(name string) *EdmEntityType { return nil } +// FindEnumType looks up an enum type by name (with or without namespace prefix). +// +// It exists to tell an ENUM apart from a complex type or a type definition when +// classifying an external action's parameter: Mendix builds an enum-typed +// parameter at 0 errors and refuses the other two (CE7255), so a lookup that +// cannot distinguish them refuses something that works. +func (d *EdmxDocument) FindEnumType(name string) *EdmEnumType { + shortName := name + if idx := strings.LastIndex(name, "."); idx >= 0 { + shortName = name[idx+1:] + } + for _, s := range d.Schemas { + for _, et := range s.EnumTypes { + if et.Name == shortName { + return et + } + } + } + return nil +} + // ParseEdmx parses an OData $metadata XML string into an EdmxDocument. func ParseEdmx(metadataXML string) (*EdmxDocument, error) { if metadataXML == "" { From a75952b7a7393cb248fc3fd27e3899ef6ad9ab59 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:09:53 +0000 Subject: [PATCH 05/17] =?UTF-8?q?docs:=20correct=20the=20List=20View=20des?= =?UTF-8?q?ign-property=20count=20=E2=80=94=20six,=20not=20three?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An earlier commit on this branch said Atlas declares "Style, Hover style and Row size, and nothing else" for a List View. That came from reading the `ListView` key of themesource/atlas_core/web/design-properties.json directly, and it misses the `Widget` group, which applies to every widget. Measured on 11.12.2: $ mxcli -p App.mpr -c "show design properties for listview" From: Widget (inherited) Spacing, Align self, Hide on From: ListView Style, Hover style, Row size Six. The conclusion the number was cited for is unaffected — none of the six is 'Remove empty text', so CE6083 still says mxcli was right to refuse the write and wrong only in the reason it gave. But the number is load-bearing for the follow-up in ako/mxcli#509: someone building the MDL-WIDGET11 pass for ALTER STYLING from the wrong three would reject Spacing, Align self and Hide on as unknown keys. ThemeRegistry.GetPropertiesForWidget already prepends the inherited group, so the existing validator is correct as written and only the prose was wrong. Both notes now say to count with `show design properties`, never from the JSON key. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .../fix-issue/findings/mdl-executor.jsonl | 2 +- ...5-installed-widget-without-widget-init.mdl | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 0293fb36e..f5c5558b9 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -639,4 +639,4 @@ {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY MICROFLOW` re-enables concurrent execution on a microflow that disallowed it \u2014 the running app's concurrency protection removed \u2014 and drops the concurrency error message (all translations) and error microflow, plus `MarkAsUsed`. Every checker is green: **CE4899 fires only on disallow-without-a-message, never on allow**, so the one error that exists in this area is exactly the one the reset switches off", "cause": "`buildMicroflowFromStmt` built the rebuild struct with `AllowConcurrentExecution: true` and `MarkAsUsed: false` literals, and `microflowToGen` wrote `SetConcurrencyErrorMicroflowQualifiedName(\"\")` + a bare `genTexts.NewText()`. The backend already READ the two flags back (the #723 \u00a7A fix), so the round-trip test passed while the bug was live \u2014 the executor overwrote them before the backend ever saw them", "file": "`mdl/executor/cmd_microflows_build.go` (buildMicroflowFromStmt), `mdl/backend/modelsdk/microflow_write.go` (microflowToGen), `mdl/backend/modelsdk/microflow.go` (microflowFromGen), `sdk/microflows/microflows.go`", "fix": "Carry all four from the stored microflow, seeding the locals with the NEW-microflow defaults (true/false) so no separate preserve flag is needed. The error message reuses the existing `textFromGen`/`textToGen` pair, so translations survive; nil still emits the bare empty `Texts$Text` the writer always wrote", "insight": "**A passing round-trip test at one layer says nothing about the layer above it.** `TestMicroflowRoundTrip_ConcurrentExecutionFlags` had guarded these two flags since #723 and was green throughout, because the executor's rebuild struct overwrites them before calling the backend. When a property is reset, locate the LAST writer on the path, not the first one that looks responsible. **And check which way a reset goes**: #723's backend bug wrote the Go zero value (allow -> disallow) and hit CE4899 immediately; the executor's literal writes the opposite (disallow -> allow), and the same CE4899 that caught the first direction is structurally blind to the second. A checker that catches a property's loss in one direction is not coverage for that property. Two methodological traps in the test itself, both hit: `bytes.Equal` on two encodes of the same microflow ALWAYS differs (fresh random sub-element `$ID`s \u2014 the reason `canon` exists), and `canon.Equal` on a whole microflow always differs too, because `StableId` is a fresh GUID *value* per encode and `Equal` does not mask \u2014 only `Reconcile` may be asked that question. Compare the sub-element under test, or use Reconcile. Controls: hardcoding the executor literals back, emptying the writer's pair, and stubbing the reader each fail a different test with the reported symptom"} {"area":"mdl/executor","date":"2026-09-17","symptom":"`DESCRIBE ENUMERATION Mod.E` prints every value with an empty caption (`MyValue ''`) although Studio Pro shows them. Re-executing that output then DESTROYS the real captions (exec reports \"Modified enumeration\" and the stored Texts$Translation goes empty). Reported on Windows, single-language project, v0.18.0 and v0.22.0","cause":"The read asked `v.Caption.GetTranslation(\"en_US\")`. Mendix has no language-neutral text: a project whose DefaultLanguageCode is nl_NL stores the caption under nl_NL and nothing else, so the lookup misses and returns \"\". #970 fixed the WRITE side to use the project language and #702 fixed the widget READ side; the enumeration/validation-rule/message-template reads were the sites neither sweep reached","file":"`mdl/executor/cmd_enumerations.go` (describeEnumeration), `cmd_diff_mdl.go` (enumerationToMDL), `describe_language.go` (pickTextTranslation's fallback now sorts), `mdl/catalog/language.go` + `builder_modules.go`","insight":"**Reproduce it with mxcli alone — no Studio Pro and no non-English project needed.** `ALTER SETTINGS LANGUAGE ADD OR MODIFY 'nl_NL' (...); ALTER SETTINGS LANGUAGE DefaultLanguageCode = 'nl_NL';` on a copy of any fixture, then CREATE the enumeration: the write side already honours the project language, so the captions land under nl_NL and DESCRIBE reads '' immediately. That also gives the impact control for free — feed the '' output back through exec and grep the .mxunit for `Texts$Translation LanguageCode nl_NL Text ` with nothing after it. **The plausible wrong turn to skip**: suspecting the codec or a gen storage-name mismatch. `EnumerationValue.Caption` is NOT in keyaudit_test.go and the strings are plainly visible in the unit — dump the .mxunit with a printable-ASCII regex FIRST (one command) and the language code tells you it is a read-side language bug, not a decode bug. **The fallback has to sort**: `for _, v := range t.Translations` returns a different language per run, so a multi-language project's DESCRIBE output was undiffable — a bug that a single-language repro can never show.","refs":["mendixlabs/mxcli#1113","mendixlabs/mxcli#970","mendixlabs/mxcli#702"],"ce":[]} {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} -{"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11: themesource/atlas_core/web/design-properties.json declares Style, Hover style, Row size and nothing else. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} +{"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11. **Count them with `show design properties for listview`, never by reading the design-properties.json key**: a List View has SIX — Style/Hover style/Row size under `ListView`, plus Spacing/Align self/Hide on inherited from the `Widget` group that applies to every widget. Reading the raw `ListView` key alone says three, which is the mistake this session made and had to correct; `ThemeRegistry.GetPropertiesForWidget` already prepends the inherited group, so anything built on it is right and anything built on the JSON key rejects half the real properties. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} diff --git a/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl index 7916157f7..133e13541 100644 --- a/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl +++ b/mdl-examples/bug-tests/widgets-1135-installed-widget-without-widget-init.mdl @@ -59,10 +59,22 @@ create or replace page MyFirstModule.HtmlDemo ( -- -- The three names in the report — 'Remove empty text', 'Remove loadmore button', -- 'Reset list style' — are NOT List View design properties in the Atlas that --- ships with Mendix 11. Measured on 11.12.2, themesource/atlas_core/web/ --- design-properties.json declares exactly three for ListView: Style, Hover style --- and Row size. Writing one of the reported names through ALTER STYLING --- succeeds, and mxbuild then refuses the project: +-- ships with Mendix 11. Measured on 11.12.2, a List View has SIX, and none of +-- them is one of those three: +-- +-- mxcli -p App.mpr -c "show design properties for listview" +-- From: Widget (inherited) Spacing, Align self, Hide on +-- From: ListView Style, Hover style, Row size +-- +-- Count them from the registry, not from the design-properties.json key: three +-- come from the `Widget` group, which applies to every widget, and reading only +-- the `ListView` key says three. ThemeRegistry.GetPropertiesForWidget already +-- prepends the inherited group, so a validator built on it gets all six for +-- free; a validator built on the raw JSON key would reject Spacing, Align self +-- and Hide on as unknown. +-- +-- Writing one of the reported names through ALTER STYLING succeeds, and mxbuild +-- then refuses the project: -- -- alter styling … set 'Remove empty text' = on; -> mx check: 1 error, -- [CE6083] "Design property Remove empty text is not supported by your From ba0a120287b57634c12cae68061e5289d64dc4c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:23:14 +0000 Subject: [PATCH 06/17] fix(check): stop claiming a List View's `Editable` is dropped MDL-WIDGET20 warned that `Editable` on a list view or a grid column is "silently dropped on write and the widget stays enabled". Every clause after the "but" was wrong, and the suggestion is addressed to a button --- the tell that the branch was written for a different widget. Measured on 11.12.2: exec writes it, `describe page` reads back `Editable: true`, and mxbuild reports 0 errors. Two different Mendix properties were conflated. editableWidgetTypes is the set of Pages types carrying Editability / ConditionalEditabilitySettings, which is correct for the bug the rule was written for (#928, `editable:` on a button). Pages$ListView and Pages$GridColumn carry neither: they have a plain `Editable bool`, a different property meaning "make the inputs INSIDE me editable". Parsing generated/metamodel for that shape returns exactly those two --- GridColumn was not in the report and would have been missed by fixing only what was reported. So: a second set, not more entries in the first. The bracket form `Editable: [expr]` (lowered to EditableIf) still warns on both, because neither type has ConditionalEditabilitySettings and that form genuinely is dropped. Silencing both would re-create the worse half of #928, where the shape the docs recommend vanishes without a word. TestEditableWidgetTypesMatchMetamodel could not have caught this: it enumerates types carrying Editability, and these carry none. TestPlainEditableBoolTypesMatchMetamodel is its sibling for the other property, so a new one fails a test instead of becoming a false positive. Controls, all passing: the #928 button case still fires; the bracket form on a list view still fires; input widgets stay clean. Why it was worth fixing rather than tolerating --- buildListViewV3's own comment records that a list view without `Editable` renders every input as `
`, with entity access ReadWrite and `mx check` at 0 errors. The warning steered authors away from the one property that fixes a symptom the code itself calls hard to diagnose. Fixes ako/mxcli#510 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../widgets-510-plain-editable-bool.mdl | 47 ++++++ mdl/executor/validate_widget_editability.go | 37 +++- .../widget_editable_plain_bool_test.go | 158 ++++++++++++++++++ 4 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 mdl-examples/bug-tests/widgets-510-plain-editable-bool.mdl create mode 100644 mdl/executor/widget_editable_plain_bool_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index f5c5558b9..4959baa67 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -640,3 +640,4 @@ {"area":"mdl/executor","date":"2026-09-17","symptom":"`DESCRIBE ENUMERATION Mod.E` prints every value with an empty caption (`MyValue ''`) although Studio Pro shows them. Re-executing that output then DESTROYS the real captions (exec reports \"Modified enumeration\" and the stored Texts$Translation goes empty). Reported on Windows, single-language project, v0.18.0 and v0.22.0","cause":"The read asked `v.Caption.GetTranslation(\"en_US\")`. Mendix has no language-neutral text: a project whose DefaultLanguageCode is nl_NL stores the caption under nl_NL and nothing else, so the lookup misses and returns \"\". #970 fixed the WRITE side to use the project language and #702 fixed the widget READ side; the enumeration/validation-rule/message-template reads were the sites neither sweep reached","file":"`mdl/executor/cmd_enumerations.go` (describeEnumeration), `cmd_diff_mdl.go` (enumerationToMDL), `describe_language.go` (pickTextTranslation's fallback now sorts), `mdl/catalog/language.go` + `builder_modules.go`","insight":"**Reproduce it with mxcli alone — no Studio Pro and no non-English project needed.** `ALTER SETTINGS LANGUAGE ADD OR MODIFY 'nl_NL' (...); ALTER SETTINGS LANGUAGE DefaultLanguageCode = 'nl_NL';` on a copy of any fixture, then CREATE the enumeration: the write side already honours the project language, so the captions land under nl_NL and DESCRIBE reads '' immediately. That also gives the impact control for free — feed the '' output back through exec and grep the .mxunit for `Texts$Translation LanguageCode nl_NL Text ` with nothing after it. **The plausible wrong turn to skip**: suspecting the codec or a gen storage-name mismatch. `EnumerationValue.Caption` is NOT in keyaudit_test.go and the strings are plainly visible in the unit — dump the .mxunit with a printable-ASCII regex FIRST (one command) and the language code tells you it is a read-side language bug, not a decode bug. **The fallback has to sort**: `for _, v := range t.Translations` returns a different language per run, so a multi-language project's DESCRIBE output was undiffable — a bug that a single-language repro can never show.","refs":["mendixlabs/mxcli#1113","mendixlabs/mxcli#970","mendixlabs/mxcli#702"],"ce":[]} {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} {"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11. **Count them with `show design properties for listview`, never by reading the design-properties.json key**: a List View has SIX — Style/Hover style/Row size under `ListView`, plus Spacing/Align self/Hide on inherited from the `Widget` group that applies to every widget. Reading the raw `ListView` key alone says three, which is the mistake this session made and had to correct; `ThemeRegistry.GetPropertiesForWidget` already prepends the inherited group, so anything built on it is right and anything built on the JSON key rejects half the real properties. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} +{"area":"mdl/executor","date":"2026-09-18","symptom":"`mxcli check` warns MDL-WIDGET20 that a List View's (and a grid column's) `Editable` is \"silently dropped on write and the widget stays enabled\". It is written: exec writes it, `describe page` reads back `Editable: true`, and mxbuild reports 0 errors on 11.12.2. The suggestion even reads \"buttons do support conditional visibility\"","cause":"Two different Mendix properties conflated under one MDL keyword. `editableWidgetTypes` is the set of Pages types carrying **Editability / ConditionalEditabilitySettings**, which is right for the bug the rule was written for (#928, `editable:` on a button). `Pages$ListView` and `Pages$GridColumn` carry neither — they have a plain `Editable bool`, a different property meaning \"make the inputs INSIDE me editable\" — so they fell through to the warning","file":"`mdl/executor/validate_widget_editability.go` (new `plainEditableWidgetTypes`); test `mdl/executor/widget_editable_plain_bool_test.go`","insight":"**A metamodel-sync test guards only the class it enumerates, and can lock a bug in.** `TestEditableWidgetTypesMatchMetamodel` keeps the list synced to the *Editability* set, so a type with a plain `Editable bool` and no Editability was invisible to it — the test passed throughout, and a correct fix would have made it fail if the two sets had been merged. The fix is a SECOND set with its own sibling test (`TestPlainEditableBoolTypesMatchMetamodel`), not more entries in the first. **Find the affected types by parsing generated/metamodel rather than by guessing**: scanning every `Pages*` struct for a plain `Editable bool` with no `Editability` returns exactly two (ListView, GridColumn) — the second one was not in the bug report and would have been missed. The control that keeps the fix narrow: neither type has `ConditionalEditabilitySettings`, so the BRACKET form `Editable: [expr]` (lowered to `EditableIf`) IS dropped and must keep warning — silencing both forms would re-create the worse half of #928, where the shape the docs recommend is dropped without a word. Worst-case cost of this false positive: `buildListViewV3`'s own comment records that a list view without `Editable` renders every input as `
` with entity access ReadWrite and `mx check` clean — so the warning steered authors away from the one property that fixes a symptom the code itself calls hard to diagnose","refs":["ako/mxcli#510","mendixlabs/mxcli#928"],"rules":["MDL-WIDGET20"]} diff --git a/mdl-examples/bug-tests/widgets-510-plain-editable-bool.mdl b/mdl-examples/bug-tests/widgets-510-plain-editable-bool.mdl new file mode 100644 index 000000000..d31af20ca --- /dev/null +++ b/mdl-examples/bug-tests/widgets-510-plain-editable-bool.mdl @@ -0,0 +1,47 @@ +-- ako/mxcli#510 — MDL-WIDGET20 falsely warned that `Editable` is dropped on a +-- List View and on a grid column. mxcli writes both. +-- +-- Before the fix, `mxcli check -p … --references` on this file reported: +-- +-- ⚠ widget `lvThings` (listview) has an `Editable` property, but Mendix models +-- editability on input widgets only — listview has no Editability, so this is +-- silently dropped on write and the widget stays enabled [MDL-WIDGET20] +-- → Use `visible: [ ... ]` to hide it conditionally (buttons do support +-- conditional visibility) … +-- +-- Every clause after the "but" was wrong, and the suggestion is addressed to a +-- button — the tell that the branch was written for a different widget. +-- +-- The rule's premise is right for the bug it was written for (#928, `editable:` +-- on a button): Mendix models EDITABILITY — Editability / +-- ConditionalEditabilitySettings — on input widgets only. Pages$ListView carries +-- neither. What it has is a plain `Editable bool`, a different property with a +-- different meaning: it makes the inputs INSIDE the list view editable. +-- +-- Measured on 11.12.2: exec writes it, `describe page` reads back +-- `Editable: true`, and `mxcli docker check` reports 0 errors. +-- +-- What must still warn, and is the reason this is a second set rather than more +-- entries in the first: neither type has ConditionalEditabilitySettings, so the +-- BRACKET form `Editable: [expr]` really is dropped and keeps MDL-WIDGET20. + +create entity MyFirstModule.Thing ( Name: string(200) ); + +create or replace page MyFirstModule.EditableListView ( + title: 'Editable', layout: 'Atlas_Core.Atlas_Default' +) { + listview lvThings ( + DataSource: database from MyFirstModule.Thing, + Editable: true -- written; must NOT warn + ) { + textbox tbName (Label: 'Name', Attribute: Name) + } +}; + +create or replace page MyFirstModule.EditableColumn ( + title: 'Column', layout: 'Atlas_Core.Atlas_Default' +) { + legacydatagrid dgThings (DataSource: database from MyFirstModule.Thing) { + column colName (Attribute: Name, Caption: 'Name', Editable: true) -- likewise + } +}; diff --git a/mdl/executor/validate_widget_editability.go b/mdl/executor/validate_widget_editability.go index 981f6ea5b..419c404cc 100644 --- a/mdl/executor/validate_widget_editability.go +++ b/mdl/executor/validate_widget_editability.go @@ -46,6 +46,34 @@ var editableWidgetTypes = map[string]bool{ "textbox": true, // Pages$TextBox } +// plainEditableWidgetTypes are the MDL widget types whose Mendix counterpart has +// a plain `Editable bool` and NO Editability. +// +// That is a different property from the one above, with a different meaning, and +// conflating the two was ako/mxcli#510. Pages$ListView.Editable makes the input +// widgets INSIDE the list view editable; Pages$GridColumn.Editable does the same +// for a column's cell. mxcli writes both — buildListViewV3 sets it, `describe +// page` reads it back, and mxbuild accepts the result at 0 errors on 11.12.2 — +// so warning that they are "silently dropped" was false, and told authors to +// stop setting the one property that makes an authored list view's inputs +// editable at all. buildListViewV3's own comment records how that failure looks: +// every input rendered as `
`, with entity +// access ReadWrite and `mx check` clean. +// +// Only the PLAIN form is written. Neither type has ConditionalEditabilitySettings, +// so `editable: [expr]` on one is genuinely dropped and still earns MDL-WIDGET20 +// — which is why this is a separate set rather than more entries in the one +// above. +// +// Kept in sync with generated/metamodel by TestPlainEditableBoolTypesMatchMetamodel, +// the sibling of TestEditableWidgetTypesMatchMetamodel. The older test could not +// have caught #510: it enumerates types carrying Editability, and these carry +// none, so they were invisible to it. +var plainEditableWidgetTypes = map[string]bool{ + "listview": true, // Pages$ListView + "column": true, // Pages$GridColumn +} + // ValidateWidgetEditability reports (MDL-WIDGET20) an `editable:` property on a // widget type that has no editability in the Mendix model. // @@ -65,7 +93,14 @@ func validateWidgetEditability(w *ast.WidgetV3, locationPrefix string) []linter. if !ok { return nil } - if editableWidgetTypes[strings.ToLower(w.Type)] { + typ := strings.ToLower(w.Type) + if editableWidgetTypes[typ] { + return nil + } + // A plain `Editable bool` type: the plain form is written, the bracket form + // (lowered to EditableIf, and riding on ConditionalEditabilitySettings these + // types do not have) is not. Only the second is worth reporting. + if plainEditableWidgetTypes[typ] && !strings.EqualFold(key, "editableif") { return nil } return []linter.Violation{{ diff --git a/mdl/executor/widget_editable_plain_bool_test.go b/mdl/executor/widget_editable_plain_bool_test.go new file mode 100644 index 000000000..b986d5cd8 --- /dev/null +++ b/mdl/executor/widget_editable_plain_bool_test.go @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "go/ast" + "go/parser" + "go/token" + "strings" + "testing" +) + +// ako/mxcli#510. +// +// MDL-WIDGET20 warned that a List View's `Editable` is "silently dropped on +// write and the widget stays enabled". Every clause after the "but" was wrong: +// mxcli writes it, `describe page` reads it back, and mxbuild accepts the +// result at 0 errors on 11.12.2. +// +// The rule's premise holds for the bug it was written for (#928, `editable:` on +// a button): Mendix models EDITABILITY — `Editability` / +// `ConditionalEditabilitySettings` — on input widgets only. Pages$ListView +// carries neither. What it has is a plain `Editable bool`, which is a DIFFERENT +// property with a different meaning: it makes the inputs INSIDE the list view +// editable. buildListViewV3 writes it, and its comment records what happens +// without it — every input renders as
, "a +// value, not a field", with entity access ReadWrite and `mx check` at 0 errors. +// +// So the warning told authors to stop setting the one property that fixes a +// symptom its own code comment calls hard to diagnose. Its suggestion ("buttons +// do support conditional visibility") is addressed to a different widget, which +// is the tell. +func TestMDLWIDGET20_PlainEditableBoolIsWritten(t *testing.T) { + cases := []struct { + name string + src string + }{ + { + name: "listview", + src: `create page W.P ( Title: 'P' ) +{ + LISTVIEW lv (DataSource: DATABASE W.Product, Editable: true) { + DYNAMICTEXT dt ( Content: 'x' ) + } +}`, + }, + { + name: "grid column", + src: `create page W.P ( Title: 'P' ) +{ + LEGACYDATAGRID dg (DataSource: DATABASE W.Product) { + COLUMN colName ( Attribute: Name, Caption: 'Name', Editable: true ) + } +}`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := widgetViolations(t, tc.src, "MDL-WIDGET20"); len(got) != 0 { + t.Errorf("MDL-WIDGET20 claimed a written property is dropped: %#v", got) + } + }) + } +} + +// The control that stops the fix from becoming "stop warning". A plain +// `Editable bool` is not conditional editability: Pages$ListView has no +// ConditionalEditabilitySettings, so the BRACKET form genuinely is dropped and +// must still be reported. Getting this wrong in the other direction would +// silently drop the shape the docs recommend — the worse half of #928. +func TestMDLWIDGET20_BracketFormStillReportedOnPlainEditableTypes(t *testing.T) { + src := `create page W.P ( Title: 'P' ) +{ + LISTVIEW lv (DataSource: DATABASE W.Product, Editable: [$currentObject/Name != '']) { + DYNAMICTEXT dt ( Content: 'x' ) + } +}` + got := widgetViolations(t, src, "MDL-WIDGET20") + if len(got) != 1 { + t.Fatalf("got %d MDL-WIDGET20 violations for the bracket form on a list view, want 1", len(got)) + } + if !strings.Contains(got[0].Message, "EditableIf") { + t.Errorf("message should quote the property as spelled (EditableIf): %q", got[0].Message) + } +} + +// The #928 case must keep firing, or the rule has simply been deleted. +func TestMDLWIDGET20_ButtonStillReported(t *testing.T) { + if got := widgetViolations(t, buttonPage(`editable: 'false'`), "MDL-WIDGET20"); len(got) != 1 { + t.Fatalf("got %d MDL-WIDGET20 violations on a button, want 1 — the #928 case", len(got)) + } +} + +// The sibling of TestEditableWidgetTypesMatchMetamodel, for the other property. +// +// That test keeps editableWidgetTypes in sync with the types carrying +// Editability. It cannot see this class at all, which is why the bug survived: +// a type with a plain `Editable bool` and no Editability is invisible to it. +// This reads generated/metamodel — the arbiter per CLAUDE.md — and fails when a +// new one appears, so it becomes a failing test rather than a false positive. +func TestPlainEditableBoolTypesMatchMetamodel(t *testing.T) { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "../../generated/metamodel/types.go", nil, 0) + if err != nil { + t.Fatalf("parse metamodel: %v", err) + } + + plain := map[string]bool{} + ast.Inspect(f, func(n ast.Node) bool { + ts, ok := n.(*ast.TypeSpec) + if !ok || !strings.HasPrefix(ts.Name.Name, "Pages") { + return true + } + st, ok := ts.Type.(*ast.StructType) + if !ok { + return true + } + hasPlain, hasEditability := false, false + for _, fld := range st.Fields.List { + ident, isIdent := fld.Type.(*ast.Ident) + for _, nm := range fld.Names { + switch nm.Name { + case "Editable": + if isIdent && ident.Name == "bool" { + hasPlain = true + } + case "Editability", "ConditionalEditabilitySettings": + hasEditability = true + } + } + } + if hasPlain && !hasEditability { + plain[ts.Name.Name] = true + } + return true + }) + + if len(plain) == 0 { + t.Fatal("found no Pages type with a plain Editable bool — the parse is wrong, and a passing run would prove nothing") + } + + src, err := parseRuleComments() + if err != nil { + t.Fatalf("read the rule's list: %v", err) + } + for typeName := range plain { + mendixName := "Pages$" + strings.TrimPrefix(typeName, "Pages") + if !strings.Contains(src, mendixName) { + t.Errorf("%s has a plain Editable bool and no Editability, but is not named in "+ + "validate_widget_editability.go — mxcli would wrongly warn that its `editable:` is dropped", + mendixName) + } + } + if len(plain) != 2 { + t.Errorf("the metamodel now has %d Pages types with a plain Editable bool, not the 2 measured "+ + "for #510 (%v) — re-check plainEditableWidgetTypes against it", len(plain), plain) + } +} From ab4d3792ce81cf7c1ff30b88a655dcb47a986474 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:29:21 +0000 Subject: [PATCH 07/17] fix(pages): a list view template must be a STRICT specialization `template for ` passed check, was written by exec, and mxbuild then refused the project: [CE0543] "The entity of the list view template is 'MyFirstModule.Vehicle' and this is not a specialization of the entity of the list view." The guard existed and was one case too generous. Both call sites gated on entityIsOrDescendsFrom(spec, listEntity), which returns true on its first loop iteration when spec == listEntity. Mendix requires a strict specialization: the list view's own body already renders an object no template matches, so a template for the base entity is a second, unreachable default. The belief was encoded three times and measured zero times. The guard's own message offered the case Mendix refuses --- "X is not or a specialization of it" --- and TestBuildListViewTemplateOnTheListEntityItself asserted it was "the base case Mendix permits", justified by what entityIsOrDescendsFrom returns rather than by any run. That test is now inverted, and carries the measurement. Measured on a blank 11.12.2 project, list view over MyFirstModule.Vehicle: template for mxcli exec mx check ----------------- ------------ -------- Car (specialized) writes 0 errors <- control Thing (unrelated) refused --- <- guard was not missing Vehicle (its own) WRITES CE0543 <- the bug All three rows matter: the first is the control against a guard that refuses everything, the second shows the guard was already there. entityIsOrDescendsFrom is left alone --- its other callers resolve association direction, where the reflexive case is correct. The rule lives in one new method that both CREATE PAGE and ALTER PAGE INSERT/REPLACE call, since the message and the rule were already duplicated across them and that is how two copies drift. The syntax help said the same wrong thing and is corrected. Not fixed here, and noted in ako/mxcli#514: the guard runs at exec, not at check, so all three rows still report `Check passed!`. A false green, but a safe one --- nothing is written when it refuses. Fixes ako/mxcli#514 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .../fix-issue/findings/mdl-executor.jsonl | 1 + cmd/mxcli/syntax/features_page.go | 4 +- ...istview-template-strict-specialization.mdl | 59 +++++++++++++++++ mdl/executor/cmd_alter_page.go | 12 ++-- ...d_pages_builder_listview_templates_test.go | 66 +++++++++++++++++-- mdl/executor/cmd_pages_builder_v3.go | 42 ++++++++++++ mdl/executor/cmd_pages_builder_v3_widgets.go | 14 ++-- 7 files changed, 172 insertions(+), 26 deletions(-) create mode 100644 mdl-examples/bug-tests/pages-514-listview-template-strict-specialization.mdl diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 4959baa67..63e69d24c 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -641,3 +641,4 @@ {"area": "mdl/executor", "date": "2026-09-17", "symptom": "`CREATE OR MODIFY EXTERNAL ENTITIES FROM` imports an OData entity with **none** of its ComplexType properties — `describe entity` lists only the key. `exec` reports `1 created, 0 failed` and prints nothing; `mx check` says 0 errors. The loss surfaces much later as CE1613 on a page written against the attributes Studio Pro would have made. `DESCRIBE CONTRACT ENTITY` compounded it by reporting the complex property as `String(200)`", "cause": "`mdl/types/edmx.go` never parsed `` at all, so a property typed `Shared.Uom.Quantity` was indistinguishable from one of an unknown type, and `createExternalEntities`' `if !strings.HasPrefix(p.Type, \"Edm.\")` dropped it with no `continue` message. `String(200)` was `edmToMendixType`'s default branch", "file": "`mdl/types/edmx.go` (EdmComplexType, FindComplexType, FlattenProperties, EdmProperty.RemotePath/Path), `mdl/executor/cmd_contract.go` (createExternalEntities, describeContractEntity, outputContractEntityMDL)", "insight": "**The local name and the remote name differ by SEPARATOR, and that is the core of the fix.** Studio Pro names the attribute `MaxQty_UoMNId` and reads it over the OData path `MaxQty/UoMNId`; assuming RemoteName == attribute name is the obvious wrong turn and it is silent in the model. Measured on mxbuild 11.12.1, three copies of one project: RemoteName `MaxQty/UoMNId` -> 0 errors; `MaxQty_UoMNId` -> 4x **CE6615** \"Attribute 'X' of external entity 'Definition' does not exist in the OData service\"; a deliberately bogus path -> the same 4x CE6615. So mxbuild resolves the path INTO the complex type and genuinely validates it — the 0-error run is evidence, not a rubber stamp, and CE6615 is the detector to reach for on any external-entity remote-name question. **Do not stop at a synthetic fixture.** A two-property complex type in its own namespace passed `mx check` clean and the fix still shipped four defects, all caught by the integration suite's live **TripPin** contract (`10-odata-examples.mdl`) at 11 errors. TripPin is the fixture to reach for: it has a base complex type, two types derived from it, a nested complex property, an Edm.GeographyPoint, and both a top-level entity set and types derived from it. What it taught, each measured: (1) Mendix imports a complex type's **own** properties only — flattening `AirportLocation`'s inherited `Address` is CE6615, while the same `Address` via `Person.HomeAddress` (typed `Location` directly) is accepted, so the line is inheritance, not path syntax; (2) `!strings.HasPrefix(t, \"Edm.\")` is not the supported-type test — `Edm.GeographyPoint` passes it and is **CE6622** \"The type of attribute 'Location_Loc' … is not supported\", so the importable primitives must be a closed set; (3) Creatable/Updatable are always false on a flattened attribute (against a contract annotated Insertable=true AND Updatable=true, Mendix still says False — 2x **CE6630** per attribute, matching the doc's \"can only be read or deleted\"), but **Filterable/Sortable are not**: they follow the ENTITY SET, and CE6630 fires in BOTH directions, so neither blanket answer survives. On TripPin, `Person` (entity set `People`) wants True and `Employee`/`Manager`/`Event` (derived, no entity set) want False; `Manager.BossOffice` is Manager's own property and still False, which rules out inheritance as the explanation. A test for a two-directional rule needs **both** controls — stamping false everywhere passes the derived case and fails People. Resolve complex types by QUALIFIED name: one document may declare `Quantity` in two namespaces, and FindEntityType's short-name fallback would silently hand over the other schema's properties. Also: the pre-fix control (attributes simply absent) is **0 errors**, so the build never catches the drop itself — a regression test asserting `mx check` clean would have passed against the bug. The report said 'only cross-namespace'; in fact every complex type was dropped, since nothing named `Edm.*` is complex. Repro `mdl-examples/bug-tests/1118-odata-complextype-flattening.mdl`", "file_refs": ["mdl/types/edmx.go", "mdl/executor/cmd_contract.go"], "refs": ["mendixlabs/mxcli#1118"], "ce": ["CE6615", "CE6622", "CE6630", "CE1613"]} {"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11. **Count them with `show design properties for listview`, never by reading the design-properties.json key**: a List View has SIX — Style/Hover style/Row size under `ListView`, plus Spacing/Align self/Hide on inherited from the `Widget` group that applies to every widget. Reading the raw `ListView` key alone says three, which is the mistake this session made and had to correct; `ThemeRegistry.GetPropertiesForWidget` already prepends the inherited group, so anything built on it is right and anything built on the JSON key rejects half the real properties. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} {"area":"mdl/executor","date":"2026-09-18","symptom":"`mxcli check` warns MDL-WIDGET20 that a List View's (and a grid column's) `Editable` is \"silently dropped on write and the widget stays enabled\". It is written: exec writes it, `describe page` reads back `Editable: true`, and mxbuild reports 0 errors on 11.12.2. The suggestion even reads \"buttons do support conditional visibility\"","cause":"Two different Mendix properties conflated under one MDL keyword. `editableWidgetTypes` is the set of Pages types carrying **Editability / ConditionalEditabilitySettings**, which is right for the bug the rule was written for (#928, `editable:` on a button). `Pages$ListView` and `Pages$GridColumn` carry neither — they have a plain `Editable bool`, a different property meaning \"make the inputs INSIDE me editable\" — so they fell through to the warning","file":"`mdl/executor/validate_widget_editability.go` (new `plainEditableWidgetTypes`); test `mdl/executor/widget_editable_plain_bool_test.go`","insight":"**A metamodel-sync test guards only the class it enumerates, and can lock a bug in.** `TestEditableWidgetTypesMatchMetamodel` keeps the list synced to the *Editability* set, so a type with a plain `Editable bool` and no Editability was invisible to it — the test passed throughout, and a correct fix would have made it fail if the two sets had been merged. The fix is a SECOND set with its own sibling test (`TestPlainEditableBoolTypesMatchMetamodel`), not more entries in the first. **Find the affected types by parsing generated/metamodel rather than by guessing**: scanning every `Pages*` struct for a plain `Editable bool` with no `Editability` returns exactly two (ListView, GridColumn) — the second one was not in the bug report and would have been missed. The control that keeps the fix narrow: neither type has `ConditionalEditabilitySettings`, so the BRACKET form `Editable: [expr]` (lowered to `EditableIf`) IS dropped and must keep warning — silencing both forms would re-create the worse half of #928, where the shape the docs recommend is dropped without a word. Worst-case cost of this false positive: `buildListViewV3`'s own comment records that a list view without `Editable` renders every input as `
` with entity access ReadWrite and `mx check` clean — so the warning steered authors away from the one property that fixes a symptom the code itself calls hard to diagnose","refs":["ako/mxcli#510","mendixlabs/mxcli#928"],"rules":["MDL-WIDGET20"]} +{"area":"mdl/executor","date":"2026-09-18","symptom":"`template for ` passes `mxcli check`, is written by `exec`, and mxbuild then refuses the project with **CE0543** \"The entity of the list view template is 'X' and this is not a specialization of the entity of the list view\"","cause":"The guard existed and was one case too generous. Both call sites gated on `entityIsOrDescendsFrom(spec, listEntity)`, which returns true on its FIRST loop iteration when `spec == listEntity`. Mendix requires a STRICT specialization — the list view's own body already renders an object no template matches, so a template for the base entity is a second, unreachable default","file":"`mdl/executor/cmd_pages_builder_v3.go` (new `checkListViewTemplateSpecialization`), called from `cmd_pages_builder_v3_widgets.go` (CREATE) and `cmd_alter_page.go` (ALTER INSERT/REPLACE); `cmd/mxcli/syntax/features_page.go`","insight":"**The guard's own error message named the bug, and a test asserted it.** The wording was \" is not **or a specialization of it**\" — it offered the exact case Mendix refuses — and `TestBuildListViewTemplateOnTheListEntityItself` asserted that case was \"the base case Mendix permits\", justified by what `entityIsOrDescendsFrom` returns rather than by any measurement. So the belief was encoded three times (guard, message, test) and measured zero times; reading the code confirms itself. **Three rows separate the cases and none is redundant**: a real specialization (0 errors — the control against a guard that refuses everything), an unrelated entity (already refused, so the guard was not simply missing), and the list view's own entity (written, CE0543). Do not fix `entityIsOrDescendsFrom` in place: its other callers resolve association direction, where the reflexive case is correct. One shared method for both call sites, because the message and the rule were already duplicated and this is how those drift. Left open in ako/mxcli#514: the guard runs at exec, not check, so all three rows report `Check passed!` — a false green, though a safe one since nothing is written when it refuses","refs":["ako/mxcli#514"],"ce":["CE0543"]} diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 1f855c38d..c4ebc114b 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -188,7 +188,9 @@ CREATE PAGE Sales.Detail (Title: 'Detail', Layout: Atlas_Core.Atlas_Default) { "TEMPLATE FOR Module.Entity and not TEMPLATE name. (A Gallery's TEMPLATE name is a\n" + "different thing: a named content slot.)\n\n" + "Rules:\n" + - " - the entity must be the list view's entity or a specialization of it\n" + + " - the entity must be a SPECIALIZATION of the list view's entity; the list\n" + + " view's own entity is CE0543, since its body already renders objects\n" + + " no template matches\n" + " - at most one template per entity\n" + " - templates keep their source order, which is the order Mendix stores and matches in\n" + " - inside a template the context object is the specialization, so its own attributes resolve\n\n" + diff --git a/mdl-examples/bug-tests/pages-514-listview-template-strict-specialization.mdl b/mdl-examples/bug-tests/pages-514-listview-template-strict-specialization.mdl new file mode 100644 index 000000000..a7d3dcb58 --- /dev/null +++ b/mdl-examples/bug-tests/pages-514-listview-template-strict-specialization.mdl @@ -0,0 +1,59 @@ +-- ako/mxcli#514 — a list view template for the list view's OWN entity was +-- accepted and written, and mxbuild then refused the project. +-- +-- mxcli already guarded `template for` against an entity that cannot match. The +-- rule was one case too generous, and its own wording was the bug: +-- +-- " is not or a specialization of it" +-- ^^^^^^^^^^^^^^^^^^^^^^^^^ +-- +-- Mendix requires a STRICT specialization. The list view's own body already +-- renders an object no template matches, so a template for the base entity is a +-- second, unreachable default. +-- +-- Measured on a blank Mendix 11.12.2 project, list view over MyFirstModule.Vehicle: +-- +-- template for mxcli check mxcli exec mx check +-- ----------------- ------------- ------------ ----------------------- +-- Car (specialized) passed writes 0 errors +-- Thing (unrelated) passed refused — +-- Vehicle (its own) passed WRITES CE0543 <- the bug +-- +-- [CE0543] "The entity of the list view template is 'MyFirstModule.Vehicle' +-- and this is not a specialization of the entity of the list view." +-- +-- Still true after the fix, and filed separately in #514: the guard runs at exec, +-- not at check, so `mxcli check` reports success for all three rows. Nothing is +-- written in the refused cases, so it is a false green rather than a bad model. + +create persistent entity MyFirstModule.Vehicle ( Brand: string(100) ); +create persistent entity MyFirstModule.Car extends MyFirstModule.Vehicle ( Doors: integer ); + +-- The control. A real specialization builds, and mxbuild reports 0 errors — +-- without it a guard that refused everything would look like a fix. +create or replace page MyFirstModule.TplOK ( + title: 'ok', layout: 'Atlas_Core.Atlas_Default' +) { + listview lvV (DataSource: database from MyFirstModule.Vehicle) { + dynamictext dt (Content: 'x') + template for MyFirstModule.Car { dynamictext dtc (Content: 'car') } + } +}; + +-- The bug. Uncomment to see the refusal — it is an exec-time error, so this file +-- has to keep it commented out for `make check-mdl`, which runs `check` alone: +-- +-- create or replace page MyFirstModule.TplSame ( +-- title: 'same', layout: 'Atlas_Core.Atlas_Default' +-- ) { +-- listview lvV3 (DataSource: database from MyFirstModule.Vehicle) { +-- dynamictext dt (Content: 'x') +-- template for MyFirstModule.Vehicle { dynamictext dtv (Content: 'vehicle') } +-- } +-- }; +-- +-- After the fix: +-- Error: failed to build page: failed to build widget: template for +-- MyFirstModule.Vehicle in list view lvV3: MyFirstModule.Vehicle is the list +-- view's own entity, and a template must be for a specialization of it — the +-- list view's own body already renders objects no template matches diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index b856778f6..f9a70863c 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -434,14 +434,10 @@ func buildListViewTemplatesFromAST(ctx *ExecContext, nodes []*ast.WidgetV3, modu } seen[spec] = true - // Mendix matches a template against the object's type, so a template for - // an entity outside the list view's hierarchy can never render. Refuse it - // here rather than writing a template nothing will ever reach. - if listEntity != "" && !checker.entityIsOrDescendsFrom(spec, listEntity) { - return nil, mdlerrors.NewValidation(fmt.Sprintf( - "template for %s in list view %s: %s is not %s or a specialization of it, "+ - "so the template can never match an object the list view shows", - spec, listViewRef, spec, listEntity)) + // Refuse a template nothing will ever reach, by the same rule CREATE PAGE + // applies — one function, so the two cannot drift apart. + if err := checker.checkListViewTemplateSpecialization(spec, listEntity, listViewRef); err != nil { + return nil, err } widgets, err := buildWidgetsFromAST(ctx, node.Children, moduleName, moduleID, spec, mutator) diff --git a/mdl/executor/cmd_pages_builder_listview_templates_test.go b/mdl/executor/cmd_pages_builder_listview_templates_test.go index 0f5730927..487e7c645 100644 --- a/mdl/executor/cmd_pages_builder_listview_templates_test.go +++ b/mdl/executor/cmd_pages_builder_listview_templates_test.go @@ -113,7 +113,7 @@ func TestBuildListViewTemplateRejections(t *testing.T) { { "entity is not a specialization of the list view's entity", []*ast.WidgetV3{templateWidget("Pages.Unrelated", "x")}, - "is not Pages.Vehicle or a specialization of it", + "is not a specialization of Pages.Vehicle", }, { "two templates for one specialization", @@ -144,16 +144,68 @@ func TestBuildListViewTemplateRejections(t *testing.T) { } } -// TestBuildListViewTemplateOnTheListEntityItself is allowed: a template for the -// list view's own entity is the base case Mendix permits, and -// entityIsOrDescendsFrom returns true for the entity itself. -func TestBuildListViewTemplateOnTheListEntityItself(t *testing.T) { +// ako/mxcli#514. This test used to assert the opposite — "a template for the +// list view's own entity is the base case Mendix permits" — on the strength of +// entityIsOrDescendsFrom returning true for the entity itself. That was never +// measured, and it is false. Mendix requires a STRICT specialization; the list +// view's own body is already what renders an object no template matches, so a +// template for the base entity would be a second, unreachable default. +// +// Measured on a blank Mendix 11.12.2 project, list view over MyFirstModule.Vehicle: +// +// template for MyFirstModule.Car -> mx check: 0 errors (a real specialization) +// template for MyFirstModule.Vehicle -> mx check: 1 error +// [CE0543] "The entity of the list view template is 'MyFirstModule.Vehicle' and +// this is not a specialization of the entity of the list view." +// +// mxcli check passed and exec wrote the page in both cases, so the guard existed +// and was one case too generous — the wording "or a specialization of it" was +// itself the bug. +func TestBuildListViewTemplateOnTheListEntityItselfIsRefused(t *testing.T) { pb := newVehiclePB() - lv, err := pb.buildListViewV3(listViewWidget(templateWidget("Pages.Vehicle", "base"))) + _, err := pb.buildListViewV3(listViewWidget(templateWidget("Pages.Vehicle", "base"))) + if err == nil { + t.Fatal("a template for the list view's own entity was accepted; mxbuild refuses it with CE0543") + } + // The old wording — "is not or a specialization of it" — named + // the very case Mendix refuses as one of the accepted ones. + if strings.Contains(err.Error(), "is not Pages.Vehicle or a specialization of it") { + t.Errorf("the message still offers the case it now refuses: %q", err.Error()) + } + if !strings.Contains(err.Error(), "own entity") { + t.Errorf("the message does not say why this one is different: %q", err.Error()) + } + if !strings.Contains(err.Error(), "Pages.Vehicle") { + t.Errorf("the message does not name the entity: %q", err.Error()) + } +} + +// The control, and the reason the fix is not "refuse every template": a genuine +// specialization still builds. Without this the test above passes against a +// guard that rejects everything. +func TestBuildListViewTemplateOnASpecializationIsAccepted(t *testing.T) { + pb := newVehiclePB() + lv, err := pb.buildListViewV3(listViewWidget(templateWidget("Pages.Bus", "bus"))) if err != nil { - t.Fatalf("a template for the list view's own entity was refused: %v", err) + t.Fatalf("a template for a real specialization was refused: %v", err) } if len(lv.Templates) != 1 { t.Fatalf("got %d template(s), want 1", len(lv.Templates)) } } + +// A grandchild is still a specialization. entityIsOrDescendsFrom walked the +// whole chain and the strict form must keep doing so — dropping to "is my +// immediate generalization" would refuse a legal two-level hierarchy. +func TestBuildListViewTemplateOnAGrandchildIsAccepted(t *testing.T) { + pb := newVehiclePB() + pb.execCache.domainModels[0].Entities = append(pb.execCache.domainModels[0].Entities, + &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: model.ID("e-Minibus")}, + Name: "Minibus", + GeneralizationRef: "Pages.Bus", + }) + if _, err := pb.buildListViewV3(listViewWidget(templateWidget("Pages.Minibus", "mini"))); err != nil { + t.Fatalf("a template for a grandchild specialization was refused: %v", err) + } +} diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index b4ed469a0..e0c604b57 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -1933,6 +1933,48 @@ func (pb *pageBuilder) associationDestination(assocQN, currentEntityQN string) ( } } +// checkListViewTemplateSpecialization reports why a `template for X` cannot +// belong to a list view over listEntity, or nil when it can. +// +// One function for both call sites — CREATE PAGE (buildListViewTemplateV3) and +// ALTER PAGE INSERT/REPLACE (cmd_alter_page.go) — because two copies of a guard +// is how the two drift, and this one was already wrong in both. +// +// The rule is a STRICT specialization, and that strictness is ako/mxcli#514. +// Both copies gated on entityIsOrDescendsFrom, which returns true for the entity +// itself, so `template for ` was accepted, written, +// and refused by mxbuild: +// +// [CE0543] "The entity of the list view template is 'MyFirstModule.Vehicle' and +// this is not a specialization of the entity of the list view." +// +// Measured on 11.12.2; a template for a real specialization is 0 errors. The +// list view's own body already renders an object no template matches, so a +// template for the base entity would be a second, unreachable default. +// +// An empty listEntity means the datasource did not resolve to an entity, which +// is reported elsewhere — do not report it a second time as a bogus +// specialization error. +func (pb *pageBuilder) checkListViewTemplateSpecialization(spec, listEntity, listViewName string) error { + if listEntity == "" || spec == "" { + return nil + } + if spec == listEntity { + return mdlerrors.NewValidation(fmt.Sprintf( + "template for %s in list view %s: %s is the list view's own entity, and a template "+ + "must be for a specialization of it — the list view's own body already renders "+ + "objects no template matches", + spec, listViewName, spec)) + } + if !pb.entityIsOrDescendsFrom(spec, listEntity) { + return mdlerrors.NewValidation(fmt.Sprintf( + "template for %s in list view %s: %s is not a specialization of %s, "+ + "so the template can never match an object the list view shows", + spec, listViewName, spec, listEntity)) + } + return nil +} + // entityIsOrDescendsFrom reports whether entityQN equals baseQN or is a // specialization of it (following the generalization chain transitively). Used // so an association declared on a base entity resolves from a subclass context. diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index 8dd294042..7199ffdb5 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -368,16 +368,10 @@ func (pb *pageBuilder) buildListViewTemplateV3(w *ast.WidgetV3, listViewName, li } seen[spec] = true - // The specialization must actually be one: Mendix matches a template against - // the object's type, so a template for an unrelated entity can never render. - // listEntity is empty when the datasource could not be resolved to an entity, - // and an unresolvable datasource is already reported elsewhere — do not - // report it a second time as a bogus specialization error. - if listEntity != "" && !pb.entityIsOrDescendsFrom(spec, listEntity) { - return nil, mdlerrors.NewValidation(fmt.Sprintf( - "template for %s in list view %s: %s is not %s or a specialization of it, "+ - "so the template can never match an object the list view shows", - spec, listViewName, spec, listEntity)) + // The specialization must actually be one, and strictly so — see + // checkListViewTemplateSpecialization, which the ALTER PAGE path shares. + if err := pb.checkListViewTemplateSpecialization(spec, listEntity, listViewName); err != nil { + return nil, err } tpl := &pages.ListViewTemplate{ From 3463674d661eb03a7ce7aebcaa0ceba9f586009f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:45:52 +0000 Subject: [PATCH 08/17] refactor(catalog): name the refs and permissions object-type vocabularies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalog's SourceType/TargetType, ElementType and AccessType values were ALL-CAPS string literals repeated across ~100 emit sites. They are a public vocabulary — Starlark lint rules filter on them through `reference.source_type`, `permission.element_type` and `permission.access_type` — but there was nothing for a consumer, or for documentation, to be checked against. Name them, and publish the four lists: RefSourceObjectTypes, RefTargetObjectTypes, PermissionElementTypes, PermissionAccessTypes. The source and target lists differ on purpose: a LAYOUT or a WIDGET is only ever pointed at, a SCHEDULED_EVENT or a PROJECT_SETTINGS only ever points, and one list for both would put values in front of rule authors that their filter can never match. Mechanical: every literal becomes the constant of the same value, including the 'WIDGET' baked into insertWidgetRefs' SQL, which becomes a bound parameter. No behaviour changes. This mirrors SourceObjectTypes in builder_source.go, which exists for the same reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018M6ewJmoi3dEjEHDmZoVLt --- mdl/catalog/builder_permissions.go | 67 +++++++-- mdl/catalog/builder_references.go | 234 ++++++++++++++++++----------- mdl/catalog/builder_widget_refs.go | 4 +- 3 files changed, 208 insertions(+), 97 deletions(-) diff --git a/mdl/catalog/builder_permissions.go b/mdl/catalog/builder_permissions.go index 58a4efaf9..e2d53beb6 100644 --- a/mdl/catalog/builder_permissions.go +++ b/mdl/catalog/builder_permissions.go @@ -8,6 +8,51 @@ import ( "github.com/mendixlabs/mxcli/sdk/domainmodel" ) +// Element and access types recorded in the permissions table. Like the refs +// vocabularies in builder_references.go these are a public vocabulary — Starlark +// lint rules filter on them through `permission.element_type` / +// `permission.access_type` — so they are named rather than repeated as literals, +// and the write-lint-rules skill is checked against them (mendixlabs/mxcli#1027). +const ( + PermissionElementEntity = "ENTITY" + PermissionElementMicroflow = "MICROFLOW" + PermissionElementPage = "PAGE" + PermissionElementODataService = "ODATA_SERVICE" + + AccessTypeCreate = "CREATE" + AccessTypeRead = "READ" + AccessTypeWrite = "WRITE" + AccessTypeDelete = "DELETE" + AccessTypeExecute = "EXECUTE" // microflow + AccessTypeView = "VIEW" // page + AccessTypeAccess = "ACCESS" // published OData service + AccessTypeMemberRead = "MEMBER_READ" + AccessTypeMemberWrite = "MEMBER_WRITE" +) + +// PermissionElementTypes and PermissionAccessTypes are the full vocabularies, in +// the order buildPermissions produces them. +var ( + PermissionElementTypes = []string{ + PermissionElementEntity, + PermissionElementMicroflow, + PermissionElementPage, + PermissionElementODataService, + } + + PermissionAccessTypes = []string{ + AccessTypeCreate, + AccessTypeRead, + AccessTypeWrite, + AccessTypeDelete, + AccessTypeExecute, + AccessTypeView, + AccessTypeAccess, + AccessTypeMemberRead, + AccessTypeMemberWrite, + } +) + // buildPermissions extracts security permissions from all documents. // This is only run in full mode as it requires parsing all documents. func (b *Builder) buildPermissions() error { @@ -70,19 +115,19 @@ func (b *Builder) buildEntityPermissions(stmt *sql.Stmt, projectID, snapshotID s for _, roleName := range roleNames { // Entity-level permissions if rule.AllowCreate { - stmt.Exec(roleName, "ENTITY", entityQN, nil, "CREATE", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, nil, AccessTypeCreate, xpath, moduleName, projectID, snapshotID) count++ } if hasRead { - stmt.Exec(roleName, "ENTITY", entityQN, nil, "READ", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, nil, AccessTypeRead, xpath, moduleName, projectID, snapshotID) count++ } if hasWrite { - stmt.Exec(roleName, "ENTITY", entityQN, nil, "WRITE", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, nil, AccessTypeWrite, xpath, moduleName, projectID, snapshotID) count++ } if rule.AllowDelete { - stmt.Exec(roleName, "ENTITY", entityQN, nil, "DELETE", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, nil, AccessTypeDelete, xpath, moduleName, projectID, snapshotID) count++ } @@ -142,11 +187,11 @@ func (b *Builder) emitMemberPermissions(stmt *sql.Stmt, rule *domainmodel.Access } if ma.AccessRights == domainmodel.MemberAccessRightsReadOnly || ma.AccessRights == domainmodel.MemberAccessRightsReadWrite { - stmt.Exec(roleName, "ENTITY", entityQN, memberName, "MEMBER_READ", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, memberName, AccessTypeMemberRead, xpath, moduleName, projectID, snapshotID) count++ } if ma.AccessRights == domainmodel.MemberAccessRightsReadWrite { - stmt.Exec(roleName, "ENTITY", entityQN, memberName, "MEMBER_WRITE", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, memberName, AccessTypeMemberWrite, xpath, moduleName, projectID, snapshotID) count++ } } @@ -154,11 +199,11 @@ func (b *Builder) emitMemberPermissions(stmt *sql.Stmt, rule *domainmodel.Access // Expand default to all attributes for _, attr := range ent.Attributes { if rule.DefaultMemberAccessRights == domainmodel.MemberAccessRightsReadOnly || rule.DefaultMemberAccessRights == domainmodel.MemberAccessRightsReadWrite { - stmt.Exec(roleName, "ENTITY", entityQN, attr.Name, "MEMBER_READ", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, attr.Name, AccessTypeMemberRead, xpath, moduleName, projectID, snapshotID) count++ } if rule.DefaultMemberAccessRights == domainmodel.MemberAccessRightsReadWrite { - stmt.Exec(roleName, "ENTITY", entityQN, attr.Name, "MEMBER_WRITE", xpath, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementEntity, entityQN, attr.Name, AccessTypeMemberWrite, xpath, moduleName, projectID, snapshotID) count++ } } @@ -188,7 +233,7 @@ func (b *Builder) buildMicroflowPermissions(stmt *sql.Stmt, projectID, snapshotI for _, roleID := range mf.AllowedModuleRoles { // AllowedModuleRoles are BY_NAME strings stored as model.ID roleName := string(roleID) - stmt.Exec(roleName, "MICROFLOW", mfQN, nil, "EXECUTE", nil, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementMicroflow, mfQN, nil, AccessTypeExecute, nil, moduleName, projectID, snapshotID) count++ } } @@ -217,7 +262,7 @@ func (b *Builder) buildPagePermissions(stmt *sql.Stmt, projectID, snapshotID str for _, roleID := range pg.AllowedRoles { // AllowedRoles are BY_NAME strings stored as model.ID roleName := string(roleID) - stmt.Exec(roleName, "PAGE", pgQN, nil, "VIEW", nil, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementPage, pgQN, nil, AccessTypeView, nil, moduleName, projectID, snapshotID) count++ } } @@ -244,7 +289,7 @@ func (b *Builder) buildODataServicePermissions(stmt *sql.Stmt, projectID, snapsh svcQN := moduleName + "." + svc.Name for _, roleName := range svc.AllowedModuleRoles { - stmt.Exec(roleName, "ODATA_SERVICE", svcQN, nil, "ACCESS", nil, moduleName, projectID, snapshotID) + stmt.Exec(roleName, PermissionElementODataService, svcQN, nil, AccessTypeAccess, nil, moduleName, projectID, snapshotID) count++ } } diff --git a/mdl/catalog/builder_references.go b/mdl/catalog/builder_references.go index 8f47dacb1..210ad9a19 100644 --- a/mdl/catalog/builder_references.go +++ b/mdl/catalog/builder_references.go @@ -41,6 +41,72 @@ const ( RefKindEvent = "event" // An entity event handler runs a microflow ) +// Object types recorded in refs.SourceType and refs.TargetType — the catalog's +// name for "what kind of document is at this end of the edge". They are the SDK +// names, upper-cased, never Mendix's BSON storage names. +// +// Named here because they are a public vocabulary: Starlark lint rules filter on +// them through `reference.source_type` / `target_type`, and the write-lint-rules +// skill documents them. While they were only string literals at the emit sites +// there was nothing for the documentation to be checked against, and it drifted +// into lower-case examples that match nothing (mendixlabs/mxcli#1027). +const ( + RefObjectEntity = "ENTITY" + RefObjectAssociation = "ASSOCIATION" + RefObjectMicroflow = "MICROFLOW" + RefObjectNanoflow = "NANOFLOW" + RefObjectRule = "RULE" + RefObjectPage = "PAGE" + RefObjectSnippet = "SNIPPET" + RefObjectLayout = "LAYOUT" + RefObjectWorkflow = "WORKFLOW" + RefObjectNavigation = "NAVIGATION" + RefObjectWidget = "WIDGET" + RefObjectJavaAction = "JAVA_ACTION" + RefObjectRestOperation = "REST_OPERATION" + RefObjectPublishedRestOperation = "PUBLISHED_REST_OPERATION" + RefObjectRegularExpression = "REGULAR_EXPRESSION" + RefObjectScheduledEvent = "SCHEDULED_EVENT" + RefObjectProjectSettings = "PROJECT_SETTINGS" +) + +// RefSourceObjectTypes is every value that reaches refs.SourceType, and +// RefTargetObjectTypes every value that reaches refs.TargetType. The two differ: +// a LAYOUT or a WIDGET is only ever pointed AT, and a SCHEDULED_EVENT or a +// PROJECT_SETTINGS only ever points. Documenting one list for both would put +// values in front of rule authors that their filter can never match. +var ( + RefSourceObjectTypes = []string{ + RefObjectEntity, + RefObjectAssociation, + RefObjectMicroflow, + RefObjectNanoflow, + RefObjectRule, + RefObjectPage, + RefObjectSnippet, + RefObjectWorkflow, + RefObjectNavigation, + RefObjectScheduledEvent, + RefObjectPublishedRestOperation, + RefObjectProjectSettings, + } + + RefTargetObjectTypes = []string{ + RefObjectEntity, + RefObjectAssociation, + RefObjectMicroflow, + RefObjectNanoflow, + RefObjectRule, + RefObjectPage, + RefObjectLayout, + RefObjectWorkflow, + RefObjectWidget, + RefObjectJavaAction, + RefObjectRestOperation, + RefObjectRegularExpression, + } +) + // collectActionActivities returns all ActionActivity objects from an ObjectCollection, // recursing into LoopedActivity bodies to find nested actions. func collectActionActivities(oc *microflows.MicroflowObjectCollection) []*microflows.ActionActivity { @@ -101,29 +167,29 @@ func microflowActionRef(action microflows.MicroflowAction) (targetType, targetNa switch a := action.(type) { case *microflows.MicroflowCallAction: if a.MicroflowCall != nil && a.MicroflowCall.Microflow != "" { - return "MICROFLOW", a.MicroflowCall.Microflow, RefKindCall, true + return RefObjectMicroflow, a.MicroflowCall.Microflow, RefKindCall, true } case *microflows.NanoflowCallAction: if a.NanoflowCall != nil && a.NanoflowCall.Nanoflow != "" { - return "NANOFLOW", a.NanoflowCall.Nanoflow, RefKindCall, true + return RefObjectNanoflow, a.NanoflowCall.Nanoflow, RefKindCall, true } case *microflows.JavaActionCallAction: if a.JavaAction != "" { - return "JAVA_ACTION", a.JavaAction, RefKindCall, true + return RefObjectJavaAction, a.JavaAction, RefKindCall, true } case *microflows.RestOperationCallAction: // Operation is a "Module.Service.Operation" name referencing a consumed // REST service operation. if a.Operation != "" { - return "REST_OPERATION", a.Operation, RefKindCall, true + return RefObjectRestOperation, a.Operation, RefKindCall, true } case *microflows.CreateObjectAction: if a.EntityQualifiedName != "" { - return "ENTITY", a.EntityQualifiedName, RefKindCreate, true + return RefObjectEntity, a.EntityQualifiedName, RefKindCreate, true } case *microflows.ShowPageAction: if a.PageName != "" { - return "PAGE", a.PageName, RefKindShowPage, true + return RefObjectPage, a.PageName, RefKindShowPage, true } case *microflows.RetrieveAction: if a.Source == nil { @@ -132,11 +198,11 @@ func microflowActionRef(action microflows.MicroflowAction) (targetType, targetNa switch src := a.Source.(type) { case *microflows.DatabaseRetrieveSource: if src.EntityQualifiedName != "" { - return "ENTITY", src.EntityQualifiedName, RefKindRetrieve, true + return RefObjectEntity, src.EntityQualifiedName, RefKindRetrieve, true } case *microflows.AssociationRetrieveSource: if src.AssociationQualifiedName != "" { - return "ASSOCIATION", src.AssociationQualifiedName, RefKindRetrieve, true + return RefObjectAssociation, src.AssociationQualifiedName, RefKindRetrieve, true } } } @@ -169,11 +235,11 @@ func microflowVarActionRef(action microflows.MicroflowAction, varEntity map[stri switch a := action.(type) { case *microflows.ChangeObjectAction: if qn, found := resolve(a.ChangeVariable); found { - return "ENTITY", qn, RefKindChange, true + return RefObjectEntity, qn, RefKindChange, true } case *microflows.DeleteObjectAction: if qn, found := resolve(a.DeleteVariable); found { - return "ENTITY", qn, RefKindDelete, true + return RefObjectEntity, qn, RefKindDelete, true } } return "", "", "", false @@ -246,11 +312,11 @@ func (b *Builder) buildReferences() error { // and return type even when it never create/retrieves them. for _, p := range params { if qn := entityOfDataType(p.Type); qn != "" { - emit("ENTITY", qn, RefKindParameter) + emit(RefObjectEntity, qn, RefKindParameter) } } if qn := entityOfDataType(returnType); qn != "" { - emit("ENTITY", qn, RefKindReturn) + emit(RefObjectEntity, qn, RefKindReturn) } if oc == nil { @@ -261,7 +327,7 @@ func (b *Builder) buildReferences() error { // variable, not a named entity) can resolve their target. varEntity := buildVarEntityMap(params, acts) for _, rule := range collectRuleCalls(oc) { - emit("RULE", rule, RefKindCall) + emit(RefObjectRule, rule, RefKindCall) } for _, act := range acts { if tt, tn, rk, ok := microflowActionRef(act.Action); ok { @@ -279,7 +345,7 @@ func (b *Builder) buildReferences() error { return err } for _, mf := range mfs { - emitActionRefs("MICROFLOW", string(mf.ID), mf.ContainerID, mf.Name, mf.Parameters, mf.ReturnType, mf.ObjectCollection) + emitActionRefs(RefObjectMicroflow, string(mf.ID), mf.ContainerID, mf.Name, mf.Parameters, mf.ReturnType, mf.ObjectCollection) } // Extract nanoflow references — nanoflows also call microflows/nanoflows, @@ -287,7 +353,7 @@ func (b *Builder) buildReferences() error { nfs, err := b.cachedNanoflows() if err == nil { for _, nf := range nfs { - emitActionRefs("NANOFLOW", string(nf.ID), nf.ContainerID, nf.Name, nf.Parameters, nf.ReturnType, nf.ObjectCollection) + emitActionRefs(RefObjectNanoflow, string(nf.ID), nf.ContainerID, nf.Name, nf.Parameters, nf.ReturnType, nf.ObjectCollection) } } @@ -299,7 +365,7 @@ func (b *Builder) buildReferences() error { rules, err := b.cachedRules() if err == nil { for _, rule := range rules { - emitActionRefs("RULE", string(rule.ID), rule.ContainerID, rule.Name, rule.Parameters, rule.ReturnType, rule.ObjectCollection) + emitActionRefs(RefObjectRule, string(rule.ID), rule.ContainerID, rule.Name, rule.Parameters, rule.ReturnType, rule.ObjectCollection) } } @@ -314,8 +380,8 @@ func (b *Builder) buildReferences() error { sourceQN := moduleName + "." + ent.Name // Check generalization if ent.GeneralizationRef != "" { - _, err = stmt.Exec("ENTITY", string(ent.ID), sourceQN, - "ENTITY", "", ent.GeneralizationRef, + _, err = stmt.Exec(RefObjectEntity, string(ent.ID), sourceQN, + RefObjectEntity, "", ent.GeneralizationRef, RefKindGeneralize, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -324,8 +390,8 @@ func (b *Builder) buildReferences() error { // Calculated-by: an attribute whose value is computed by a microflow. for _, attr := range ent.Attributes { if attr.Value != nil && attr.Value.MicroflowName != "" { - _, err = stmt.Exec("ENTITY", string(ent.ID), sourceQN, - "MICROFLOW", "", attr.Value.MicroflowName, + _, err = stmt.Exec(RefObjectEntity, string(ent.ID), sourceQN, + RefObjectMicroflow, "", attr.Value.MicroflowName, RefKindCalculate, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -344,8 +410,8 @@ func (b *Builder) buildReferences() error { if target == "" { continue } - if _, err := stmt.Exec("ASSOCIATION", string(assoc.ID), assocQN, - "ENTITY", "", target, + if _, err := stmt.Exec(RefObjectAssociation, string(assoc.ID), assocQN, + RefObjectEntity, "", target, RefKindAssociate, moduleName, projectID, snapshotID); err == nil { refCount++ } @@ -359,8 +425,8 @@ func (b *Builder) buildReferences() error { if target == "" { continue } - if _, err := stmt.Exec("ASSOCIATION", string(ca.ID), assocQN, - "ENTITY", "", target, + if _, err := stmt.Exec(RefObjectAssociation, string(ca.ID), assocQN, + RefObjectEntity, "", target, RefKindAssociate, moduleName, projectID, snapshotID); err == nil { refCount++ } @@ -384,8 +450,8 @@ func (b *Builder) buildReferences() error { // parsed widget tree, which no reader currently exposes — tracked as the // remaining part of #663 gap 3. if layoutRef := b.resolvePageLayoutRef(pg.ID); layoutRef != "" { - _, err = stmt.Exec("PAGE", string(pg.ID), sourceQN, - "LAYOUT", "", layoutRef, + _, err = stmt.Exec(RefObjectPage, string(pg.ID), sourceQN, + RefObjectLayout, "", layoutRef, RefKindLayout, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -395,8 +461,8 @@ func (b *Builder) buildReferences() error { // Page parameter entity types for _, param := range pg.Parameters { if param.EntityName != "" { - _, err = stmt.Exec("PAGE", string(pg.ID), sourceQN, - "ENTITY", "", param.EntityName, + _, err = stmt.Exec(RefObjectPage, string(pg.ID), sourceQN, + RefObjectEntity, "", param.EntityName, RefKindParameter, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -411,13 +477,13 @@ func (b *Builder) buildReferences() error { // DISTINCT collapses the many widgets on a page that target the same // document into a single edge. widgetProjections := []struct{ col, targetType, refKind string }{ - {"EntityRef", "ENTITY", RefKindDatasource}, - {"MicroflowRef", "MICROFLOW", RefKindAction}, - {"NanoflowRef", "NANOFLOW", RefKindAction}, + {"EntityRef", RefObjectEntity, RefKindDatasource}, + {"MicroflowRef", RefObjectMicroflow, RefKindAction}, + {"NanoflowRef", RefObjectNanoflow, RefKindAction}, // A widget action that opens a page. Without this row, a page reachable // only from a button had no inbound reference and `show callers` / // `show references` reported it as unused (issue #773). - {"PageRef", "PAGE", RefKindShowPage}, + {"PageRef", RefObjectPage, RefKindShowPage}, } for _, p := range widgetProjections { res, perr := b.tx.Exec( @@ -448,16 +514,16 @@ func (b *Builder) buildReferences() error { // Default home page if profile.HomePage != nil { if profile.HomePage.Page != "" { - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "PAGE", "", profile.HomePage.Page, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectPage, "", profile.HomePage.Page, RefKindHomePage, "", projectID, snapshotID) if err == nil { refCount++ } } if profile.HomePage.Microflow != "" { - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "MICROFLOW", "", profile.HomePage.Microflow, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectMicroflow, "", profile.HomePage.Microflow, RefKindHomePage, "", projectID, snapshotID) if err == nil { refCount++ @@ -468,16 +534,16 @@ func (b *Builder) buildReferences() error { // Role-based home pages for _, rh := range profile.RoleBasedHomePages { if rh.Page != "" { - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "PAGE", "", rh.Page, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectPage, "", rh.Page, RefKindHomePage, "", projectID, snapshotID) if err == nil { refCount++ } } if rh.Microflow != "" { - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "MICROFLOW", "", rh.Microflow, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectMicroflow, "", rh.Microflow, RefKindHomePage, "", projectID, snapshotID) if err == nil { refCount++ @@ -487,8 +553,8 @@ func (b *Builder) buildReferences() error { // Login page if profile.LoginPage != "" { - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "PAGE", "", profile.LoginPage, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectPage, "", profile.LoginPage, RefKindLoginPage, "", projectID, snapshotID) if err == nil { refCount++ @@ -514,8 +580,8 @@ func (b *Builder) buildReferences() error { if oe.Entity == "" { continue } - _, err = stmt.Exec("NAVIGATION", "", sourceName, - "ENTITY", "", oe.Entity, + _, err = stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectEntity, "", oe.Entity, RefKindSync, "", projectID, snapshotID) if err == nil { refCount++ @@ -534,8 +600,8 @@ func (b *Builder) buildReferences() error { // Parameter entity reference if wf.Parameter != nil && wf.Parameter.EntityRef != "" { - _, err = stmt.Exec("WORKFLOW", string(wf.ID), sourceQN, - "ENTITY", "", wf.Parameter.EntityRef, + _, err = stmt.Exec(RefObjectWorkflow, string(wf.ID), sourceQN, + RefObjectEntity, "", wf.Parameter.EntityRef, RefKindParameter, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -544,8 +610,8 @@ func (b *Builder) buildReferences() error { // Overview page reference if wf.OverviewPage != "" { - _, err = stmt.Exec("WORKFLOW", string(wf.ID), sourceQN, - "PAGE", "", wf.OverviewPage, + _, err = stmt.Exec(RefObjectWorkflow, string(wf.ID), sourceQN, + RefObjectPage, "", wf.OverviewPage, RefKindShowPage, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -606,8 +672,8 @@ func (b *Builder) extractRegexRuleRefs(stmt *sql.Stmt, projectID, snapshotID str count := 0 for _, r := range b.regexRuleRefs { if _, err := stmt.Exec( - "ENTITY", "", r.entityQualifiedName, - "REGULAR_EXPRESSION", "", r.regexQualifiedName, + RefObjectEntity, "", r.entityQualifiedName, + RefObjectRegularExpression, "", r.regexQualifiedName, RefKindValidate, r.moduleName, projectID, snapshotID, ); err == nil { count++ @@ -620,8 +686,8 @@ func (b *Builder) extractScheduledEventRefs(stmt *sql.Stmt, projectID, snapshotI count := 0 for _, r := range b.scheduledEventRefs { if _, err := stmt.Exec( - "SCHEDULED_EVENT", "", r.qualifiedName, - "MICROFLOW", "", r.microflow, + RefObjectScheduledEvent, "", r.qualifiedName, + RefObjectMicroflow, "", r.microflow, RefKindSchedule, r.moduleName, projectID, snapshotID, ); err == nil { count++ @@ -645,8 +711,8 @@ func (b *Builder) extractPublishedRestRefs(stmt *sql.Stmt, projectID, snapshotID count := 0 for _, r := range b.publishedRestRefs { if _, err := stmt.Exec( - "PUBLISHED_REST_OPERATION", r.sourceID, r.qualifiedName, - "MICROFLOW", "", r.microflow, + RefObjectPublishedRestOperation, r.sourceID, r.qualifiedName, + RefObjectMicroflow, "", r.microflow, RefKindPublish, r.moduleName, projectID, snapshotID, ); err == nil { count++ @@ -671,8 +737,8 @@ func (b *Builder) extractEventHandlerRefs(stmt *sql.Stmt, projectID, snapshotID count := 0 for _, r := range b.eventHandlerRefs { if _, err := stmt.Exec( - "ENTITY", "", r.entityQualifiedName, - "MICROFLOW", "", r.microflow, + RefObjectEntity, "", r.entityQualifiedName, + RefObjectMicroflow, "", r.microflow, RefKindEvent, r.moduleName, projectID, snapshotID, ); err == nil { count++ @@ -686,16 +752,16 @@ func (b *Builder) extractMenuItemRefs(stmt *sql.Stmt, items []*types.NavMenuItem refCount := 0 for _, item := range items { if item.Page != "" { - _, err := stmt.Exec("NAVIGATION", "", sourceName, - "PAGE", "", item.Page, + _, err := stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectPage, "", item.Page, RefKindMenuItem, "", projectID, snapshotID) if err == nil { refCount++ } } if item.Microflow != "" { - _, err := stmt.Exec("NAVIGATION", "", sourceName, - "MICROFLOW", "", item.Microflow, + _, err := stmt.Exec(RefObjectNavigation, "", sourceName, + RefObjectMicroflow, "", item.Microflow, RefKindMenuItem, "", projectID, snapshotID) if err == nil { refCount++ @@ -804,7 +870,7 @@ func (b *Builder) extractWidgetObjectRefs(stmt *sql.Stmt, obj *pages.WidgetObjec // Extract entity ref if val.EntityRef != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "ENTITY", "", val.EntityRef, + RefObjectEntity, "", val.EntityRef, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -812,7 +878,7 @@ func (b *Builder) extractWidgetObjectRefs(stmt *sql.Stmt, obj *pages.WidgetObjec // Extract microflow ref if val.Microflow != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "MICROFLOW", "", val.Microflow, + RefObjectMicroflow, "", val.Microflow, RefKindAction, moduleName, projectID, snapshotID) refCount++ } @@ -820,7 +886,7 @@ func (b *Builder) extractWidgetObjectRefs(stmt *sql.Stmt, obj *pages.WidgetObjec // Extract nanoflow ref if val.Nanoflow != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "NANOFLOW", "", val.Nanoflow, + RefObjectNanoflow, "", val.Nanoflow, RefKindAction, moduleName, projectID, snapshotID) refCount++ } @@ -828,7 +894,7 @@ func (b *Builder) extractWidgetObjectRefs(stmt *sql.Stmt, obj *pages.WidgetObjec // Extract form (page) ref if val.Form != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "PAGE", "", val.Form, + RefObjectPage, "", val.Form, RefKindShowPage, moduleName, projectID, snapshotID) refCount++ } @@ -862,7 +928,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou entityQN := b.resolveEntityID(src.EntityID) if entityQN != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "ENTITY", string(src.EntityID), entityQN, + RefObjectEntity, string(src.EntityID), entityQN, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -878,7 +944,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou } if entityQN != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "ENTITY", "", entityQN, + RefObjectEntity, "", entityQN, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -893,7 +959,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou // This might be a qualified name or just entity name // We store it as-is for now stmt.Exec(sourceType, sourceID, sourceQN, - "ENTITY", "", src.EntityPath, + RefObjectEntity, "", src.EntityPath, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -903,7 +969,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou // Similar to EntityPathSource if src.EntityPath != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "ENTITY", "", src.EntityPath, + RefObjectEntity, "", src.EntityPath, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -914,7 +980,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou mfQN := b.resolveMicroflowID(src.MicroflowID) if mfQN != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "MICROFLOW", string(src.MicroflowID), mfQN, + RefObjectMicroflow, string(src.MicroflowID), mfQN, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -926,7 +992,7 @@ func (b *Builder) extractDataSourceRefs(stmt *sql.Stmt, ds pages.DataSource, sou nfQN := b.resolveMicroflowID(src.NanoflowID) // Uses same table if nfQN != "" { stmt.Exec(sourceType, sourceID, sourceQN, - "NANOFLOW", string(src.NanoflowID), nfQN, + RefObjectNanoflow, string(src.NanoflowID), nfQN, RefKindDatasource, moduleName, projectID, snapshotID) refCount++ } @@ -987,16 +1053,16 @@ func (b *Builder) extractWorkflowFlowRefs(stmt *sql.Stmt, flow *workflows.Flow, switch a := act.(type) { case *workflows.UserTask: if a.Page != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "PAGE", "", a.Page, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectPage, "", a.Page, RefKindShowPage, moduleName, projectID, snapshotID) if err == nil { refCount++ } } if a.UserTaskEntity != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "ENTITY", "", a.UserTaskEntity, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectEntity, "", a.UserTaskEntity, RefKindDatasource, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -1004,8 +1070,8 @@ func (b *Builder) extractWorkflowFlowRefs(stmt *sql.Stmt, flow *workflows.Flow, } if a.UserSource != nil { if us, ok := a.UserSource.(*workflows.MicroflowBasedUserSource); ok && us.Microflow != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "MICROFLOW", "", us.Microflow, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectMicroflow, "", us.Microflow, RefKindCall, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -1018,8 +1084,8 @@ func (b *Builder) extractWorkflowFlowRefs(stmt *sql.Stmt, flow *workflows.Flow, case *workflows.CallMicroflowTask: if a.Microflow != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "MICROFLOW", "", a.Microflow, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectMicroflow, "", a.Microflow, RefKindCall, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -1031,8 +1097,8 @@ func (b *Builder) extractWorkflowFlowRefs(stmt *sql.Stmt, flow *workflows.Flow, case *workflows.SystemTask: if a.Microflow != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "MICROFLOW", "", a.Microflow, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectMicroflow, "", a.Microflow, RefKindCall, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -1044,8 +1110,8 @@ func (b *Builder) extractWorkflowFlowRefs(stmt *sql.Stmt, flow *workflows.Flow, case *workflows.CallWorkflowActivity: if a.Workflow != "" { - _, err := stmt.Exec("WORKFLOW", sourceID, sourceQN, - "WORKFLOW", "", a.Workflow, + _, err := stmt.Exec(RefObjectWorkflow, sourceID, sourceQN, + RefObjectWorkflow, "", a.Workflow, RefKindCall, moduleName, projectID, snapshotID) if err == nil { refCount++ @@ -1116,8 +1182,8 @@ func (b *Builder) extractProjectSettingsRefs(stmt *sql.Stmt, projectID, snapshot moduleName = target[:i] } if _, err := stmt.Exec( - "PROJECT_SETTINGS", "", s.setting, - "MICROFLOW", "", target, + RefObjectProjectSettings, "", s.setting, + RefObjectMicroflow, "", target, RefKindSettings, moduleName, projectID, snapshotID, ); err == nil { count++ diff --git a/mdl/catalog/builder_widget_refs.go b/mdl/catalog/builder_widget_refs.go index 8ef654ee2..6a15b467e 100644 --- a/mdl/catalog/builder_widget_refs.go +++ b/mdl/catalog/builder_widget_refs.go @@ -61,11 +61,11 @@ func insertWidgetRefs(tx CatalogTx, projectID, snapshotID string) (int, error) { res, err := tx.Exec( `INSERT INTO refs (SourceType, SourceId, SourceName, TargetType, TargetId, TargetName, RefKind, ModuleName, ProjectId, SnapshotId) SELECT DISTINCT w.ContainerType, '', w.ContainerQualifiedName, - 'WIDGET', d.WidgetId, d.MdlName, ?, w.ModuleName, ?, ? + ?, d.WidgetId, d.MdlName, ?, w.ModuleName, ?, ? FROM widgets_data w JOIN widget_definitions_data d ON d.WidgetId = w.WidgetType WHERE w.ContainerQualifiedName != '' AND d.MdlName != ''`, - RefKindWidget, projectID, snapshotID) + RefObjectWidget, RefKindWidget, projectID, snapshotID) if err != nil { return 0, err } From 59db6e7b07277bc7a29ffdaf160ce8b8ae29d2ea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:46:07 +0000 Subject: [PATCH 09/17] fix(skills): document the lint-rule values the catalog actually emits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bundled write-lint-rules skill is the only documentation of the Starlark rule API, and six of its example rows named values the API has never returned. Both failure modes are silent: a rule built from them compiles, runs, matches nothing and reports a clean pass. action_type listed Mendix's BSON *storage* names — CreateChangeAction, CommitAction, ShowFormAction — where the catalog labels an action with its SDK name (CreateObjectAction, ShowPageAction, ClosePageAction, …), derived from the parsed Go type. source_type documented lower-case ("microflow", "page") against an target_type upper-case vocabulary. element_type the same lower-casing, unreported. access_type data_type documented "string"/"integer"/"datetime"; emitted String/ Integer/DateTime. The most-used filter in a lint rule. ref_kind documented no values at all. The last three were not in the report. They are the same defect in the same tables, found by checking the siblings rather than only the rows that were named. Correct all six against the functions that produce them, and add a callout above the tables: what the case convention is, why action_type is never a storage name, and the two sqlite3 probes that answer the question against a real project. mdl/catalog/lint_rule_doc_vocabulary_test.go pins each documented value to its producer, so the tables cannot drift again. action_type needs no list — the label IS the Go type name, so the test reads the isMicroflowAction marker methods out of sdk/microflows/microflows_actions.go with go/ast; the others check against the vocabularies named in the previous commit. Each check fails rather than passes when its table row goes missing, and each scan carries a vacuity control. This is the second time this vocabulary has bitten: CONV010's allowlist held the storage names and was fixed a month ago, pinned by lint_rule_vocabulary_test.go. It recurred because the documentation the rule author copied from was never corrected. Refs: mendixlabs/mxcli#1027 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018M6ewJmoi3dEjEHDmZoVLt --- .../skills/fix-issue/findings/mdl-other.jsonl | 1 + .../skills/mendix/write-lint-rules/SKILL.md | 43 ++- .../bug-tests/1027-lint-rule-vocabulary.mdl | 86 +++++ mdl/catalog/lint_rule_doc_vocabulary_test.go | 305 ++++++++++++++++++ 4 files changed, 427 insertions(+), 8 deletions(-) create mode 100644 mdl-examples/bug-tests/1027-lint-rule-vocabulary.mdl create mode 100644 mdl/catalog/lint_rule_doc_vocabulary_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-other.jsonl b/.claude/skills/fix-issue/findings/mdl-other.jsonl index 15769f85a..24638a84e 100644 --- a/.claude/skills/fix-issue/findings/mdl-other.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-other.jsonl @@ -66,3 +66,4 @@ {"area": "mdl/exprcheck", "date": "2026-09-16", "symptom": "`$out = $out + $r/Status` inside `LOOP $r IN $reqs` (Enumeration into a String) passes `mxcli check -p --references`, is written by `exec`, and fails the native build with **CE0117** at the Change variable activity. The same mistake on a PARAMETER is refused as E004, so the checker looks like it is skipped inside LOOP bodies (mendixlabs/mxcli#1100)", "cause": "The loop BODY was walked and checked all along \u2014 the control that proves it is `'status=' + $T/Status` on a parameter written one line INSIDE the loop, which was refused before the fix. Two holes in the variable scope, in series, produced the asymmetry: (a) `buildVarEntityScope` recorded CREATE, database RETRIEVE and parameters but never `LoopStmt.LoopVariable`, so `$r/Status` resolved to no attribute and inferred KindUnknown, which every rule tolerates by design; (b) `CheckAdapter` never set `Context.Scope` at all, so a DECLARE'd `$out String` was Unknown too \u2014 and E004 needs BOTH operands typed, so closing (a) alone still reported nothing on the reported script. (b) also meant `$out = $out + $Req/Status` with no loop in sight was equally silent; the report's own case A hides that by using a string literal on the left", "file": "`mdl/exprcheck/adapters/adapter_scope.go` (`buildFlowScope` replacing `buildVarEntityScope`, `recordRetrieve`/`recordListOperation`/`recordDeclare`, `kindScope`, `StatementErrorHandling`, `DataTypeKind`), `mdl/exprcheck/adapters/check.go` (`walkFlow` passes Scope; `checkListOperationCondition`; ON ERROR bodies walked), `mdl/exprcheck/slot_resolver.go` + `slot_to_context.go` (`ListOperation.Condition`), `mdl/executor/validate_microflow.go` (delegates `astKindToExprKind` and `stmtErrorHandling`)", "insight": "**Separate \"was the walk there\" from \"did the variable resolve\" before believing a skipped-construct report.** The title said LOOP bodies were not checked; one control \u2014 the same expression on a parameter, one line deeper \u2014 showed the walk was fine and the scope was not, which changed the fix from a walk to a resolver. **A silence can need two fixes to break**: typing the loop variable alone left the reported script still reporting nothing, because the rule needs both operands. Fix one, re-measure, and do not conclude the fix failed. **The same walk already existed, correct, next door**: `mdl/executor/validate_member_refs.go` typed loop variables from the list; the expression checker's walk did not \u2014 duplicate-resolver drift, which is why `stmtErrorHandling` and the DataTypeKind table are now single copies in adapters with the executor delegating. **Order is load-bearing and silent when wrong**: parameters must seed the scope BEFORE the body walk, or an association retrieve off a parameter (and every loop over its result) stays untyped \u2014 this was written the old way first and only a test caught it. **False-positive control**: exec-then-type-check over 591 mdl-examples scripts, 11 violations before and 11 after, same rules. It earned its keep \u2014 the first cut fired E009 on `set $At = find($Hay, $Needle)`, Mendix's STRING find, which the visitor still builds as a ListOperationStmt (the flow builder disambiguates it later, ledger #63). Requiring a KNOWN element entity before checking a FIND/FILTER predicate applies the same disambiguation. Controls: each of the four scope sources reverted in turn fails a distinct test with the reported symptom (empty violations). **Still open**: a bare attribute name in a FILTER predicate resolves to nothing, `retrieve \u2026 limit 1` is typed as a list like any other retrieve, and `LOOP $r IN $T/Mod.Assoc` cannot be typed because the visitor drops the association path (`ListVariable` is empty)"} {"area": "mdl/catalog", "date": "2026-09-17", "symptom": "Every microflow behind a published REST endpoint reads as dead: `CATALOG.GRAPH_DEAD_ASSETS` lists it, `SHOW CALLERS OF` says \"(no callers found)\", `SHOW REFERENCES TO` and `impact` report nothing, and QUAL004 says \"is not called from anywhere. Remove if unused.\" On the reporter's model, 92 of 93 published operations name a microflow and all 92 were listed dead \u2014 15 percent of its dead-microflow list, pointed at the most exposed code in the app (mendixlabs/mxcli#1126)", "cause": "`buildPublishedRestServices` wrote `published_rest_operations_data.Microflow` and returned \u2014 it appended to no slice that `buildReferences` drains, and there was no RefKind for the edge. The binding was in the catalog; the edge was not. Fourth instance of one class (widget actions #773, scheduled events, project settings, this)", "file": "`mdl/catalog/builder.go` (`publishedRestRefs`), `mdl/catalog/builder_rest.go` (`publishedRestRef`, `publishedRestOpName`, collection in the operation loop), `mdl/catalog/builder_references.go` (`RefKindPublish`, `extractPublishedRestRefs`), `mdl/catalog/builder_graph.go` (`graphRefKinds`), `mdl/executor/cmd_search.go` (`callerRefKinds`), `.claude/lint-rules/orphaned_elements.star` (`MICROFLOW_ENTRY_KINDS`); tests `mdl/catalog/builder_rest_refs_test.go`, `mdl/catalog/lint_rule_vocabulary_test.go`, `mdl/executor/cmd_search_callers_test.go`", "insight": "**`GRAPH_DEAD_ASSETS` is kind-AGNOSTIC \u2014 the comment beside `schedule` in `graphRefKinds` says otherwise and is wrong.** The view is `NOT EXISTS (SELECT 1 FROM refs WHERE TargetName = \u2026)`; `git log -L` shows it has never filtered on RefKind. That false comment sent the issue's root-cause analysis down the wrong path, and it would have sent the fix there too: `graphRefKinds` matters for the ANALYSIS graph (communities/layers/cycles/centrality), not for the dead list. Measured with a one-kind insert: a `publish` row in neither `graphRefKinds` nor `callerRefKinds` still took the microflow from dead=1 to dead=0. **Check which consumers actually filter before assuming all four do**: `impact` and `SHOW REFERENCES TO` select every kind, so the refs row alone fixes them; only `SHOW CALLERS` and QUAL004 need a vocabulary edit. **The three vocabularies drift independently and nothing tests the union** \u2014 `settings` shipped in v0.22.0 into refs and into QUAL004 but NOT into `callerRefKinds`, so `show callers of ` was still blind two releases later; found only by auditing the lists while adding a fourth kind, and fixed here alongside. **`sync` looks like an entry point and is not**: it targets an ENTITY, so it belongs with `datasource`/`retrieve` in the excluded set \u2014 the test now pins it there, because the next person adding a kind will read the list, not the builder. **Carry the source's own id on the edge**: the operation's synthetic `opID` was already computed for `published_rest_operations_data`, so passing it as `SourceId` (the scheduled-event precedent passes \"\") makes 'who calls this microflow' one join from the endpoint's path and summary. **Controls**: stubbing the extractor to emit nothing reproduces \"reported dead\" verbatim, and dropping the empty-microflow guard fails the two-operation test \u2014 the suite is green against neither. Adjacent and NOT fixed: `business_events_data.PublishMicroflow`/`SubscribeMicroflow` emit no edge either, and `PublishedRestService.AuthenticationMicroflow` (and its OData sibling) is on gen but never read into the semantic model, so a REST auth handler is invisible to mxcli entirely", "refs": ["mendixlabs/mxcli#1126", "mendixlabs/mxcli#773"], "rules": ["QUAL004"]} {"area": "mdl/catalog", "date": "2026-09-17", "symptom": "A microflow that runs only as an **entity event handler** is reported as unused from three directions at once: `show callers of Mod.ACT_Order_Validate` says `(no callers found)`, `CATALOG.GRAPH_DEAD_ASSETS` lists it, and `mxcli lint` emits `[QUAL004] ... is not called from anywhere.` with the suggestion **\"Remove if unused\"** \u2014 on a microflow that runs on every commit. Reported as 32 dead of 36 handlers across 24 entities", "cause": "`mdl/catalog` touched `Entity.EventHandlers` in exactly one place and threw the list away: `hasEventHandlers = 1` in `builder_modules.go`. No `refs` row was ever emitted, and no table held the handlers, so the reference graph had no ENTITY -> MICROFLOW edge for them. The `calculate` edge two lines below in `buildReferences` is the same shape and was already there, which is why the infrastructure looked complete", "file": "`mdl/catalog/builder_entity_events.go` (new), `mdl/catalog/builder_references.go` (`RefKindEvent` + `extractEventHandlerRefs`), `mdl/catalog/tables.go` (`entity_event_handlers_data` + view, `CatalogSchemaVersion` 11->12), `mdl/catalog/catalog.go` (`Tables()`), `mdl/catalog/builder.go` (field + build step), `mdl/catalog/builder_graph.go` (`graphRefKinds`), `mdl/executor/cmd_search.go` (`callerRefKinds`), `.claude/lint-rules/orphaned_elements.star`", "insight": "**The third consumer of a new RefKind is a schema version, not a list.** Beyond the three kind lists the scheduled-event fix named (`callerRefKinds`, `graphRefKinds`, the QUAL004 rule), a new edge needs `CatalogSchemaVersion` bumped: refs are only written by REFRESH CATALOG FULL and `NewFromFile` applies the schema with CREATE TABLE IF NOT EXISTS, so without the bump an existing `.mxcli/catalog.db` gains the empty table and keeps serving the pre-fix edge set \u2014 the wrong answer, from a cache, after the fix shipped. `migrateIfSchemaMismatch` drops and rebuilds on a mismatch (verified by hand-editing catalog_meta back to '11'). **A flag is a missing table wearing a value**: `HasEventHandlers` and `NavigationProfile.OfflineEntityCount` are the same defect, and the fix is the same pair \u2014 rows for what it does, an edge for whether it is reachable. Do not encode the detail in the kind: eight kinds (`before_commit`, `after_delete`, ...) would enter every consumer's list to say one thing, so the moment/event go in the table and the edge stays one `event`. **The control has to be the binary, not the test**: stubbing `extractEventHandlerRefs` to emit nothing and rebuilding reproduced `(no callers found)` + 2 dead microflows on the same project, which is what proves the assertion detects something. mendixlabs/mxcli#1127; repro `mdl-examples/bug-tests/catalog-1127-entity-event-handler-refs.mdl`", "refs": ["mendixlabs/mxcli#1127"]} +{"area":"mdl/catalog","date":"2026-09-18","symptom":"A Starlark lint rule written from the bundled write-lint-rules skill matches zero rows and reports a clean pass — or, with an allowlist, inverts into flagging everything (138 of 282 ACT_ microflows on one real project, 49% false positives)","cause":"The skill's example tables are the only documentation of the lint API and nothing tied them to the values the catalog emits. action_type listed Mendix BSON *storage* names (CreateChangeAction, CommitAction, ShowFormAction, CloseFormAction, ShowHomeFormAction) against a catalog that labels an action with its SDK name via getMicroflowActionType; source_type/target_type/element_type/access_type were lower-cased against an upper-case vocabulary; data_type was lower-cased against TitleCase from AttributeType.GetTypeName","file":"`.claude/skills/mendix/write-lint-rules/SKILL.md` (six rows), `mdl/catalog/builder_references.go` (RefObject* constants + RefSourceObjectTypes/RefTargetObjectTypes), `mdl/catalog/builder_permissions.go` (PermissionElement*/AccessType* constants), test `mdl/catalog/lint_rule_doc_vocabulary_test.go`","insight":"**Fix the documentation the rule author reads, not just the rule that was reported.** The identical defect was found and fixed in CONV010's allowlist a month earlier (finding 2026-08-17, pinned by lint_rule_vocabulary_test.go) — and recurred, because the *source* the author copied from was never corrected. A rule pinned to the labeller and a doc that is not is one fix, not two. **Check the sibling rows before believing the report's scope**: element_type, access_type and data_type had the same lower-casing and nobody had reported them; data_type ('string' vs 'String') is the most-used filter in a lint rule, so it was the most expensive one. **A doc value is only pinnable against a named vocabulary**, so the fix is half refactor: the emitters' scattered ALL-CAPS literals became RefObject*/PermissionElement*/AccessType* constants with published lists, mirroring the SourceObjectTypes precedent already in this package. action_type needs no list — the label IS the Go type name (%T), so the test reads the isMicroflowAction marker methods out of sdk/microflows/microflows_actions.go with go/ast. **Three legs, not one, when proving a filter fix**: old values -> 7/7 flagged, corrected -> 0, corrected-minus-one -> exactly that one. Leg C is the control; without it a silent rule and a correct rule both report zero, which is the bug itself","refs":["mendixlabs/mxcli#1027"],"rules":["CONV010"]} diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 99eccf45b..35dde202d 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -119,6 +119,33 @@ def check(): ## Object Properties +> **The example values below are the real ones — do not adapt their case or their +> spelling.** A filter on a value the catalog never emits is silent: the rule +> compiles, runs, matches nothing and reports a clean pass. Two traps in +> particular: +> +> - **Case is not cosmetic.** Document and element kinds are upper-case +> (`"MICROFLOW"`, `"ENTITY"`, `"READ"`), attribute data types are TitleCase +> (`"String"`, `"DateTime"`), and `ref_kind` is lower-case (`"call"`, +> `"show_page"`). Guessing wrong matches zero rows. +> - **`action_type` is the SDK name, never Mendix's BSON storage name.** The +> catalog reports `ShowPageAction` / `ClosePageAction` / `CreateObjectAction` / +> `CommitObjectsAction`; the storage names `ShowFormAction`, `CloseFormAction`, +> `CreateChangeAction` and `CommitAction` that appear in `.mpr` documents never +> reach a rule. A rule that allow-lists the storage names flags every microflow +> that opens a page — the inversion measured at 49% false positives in +> mendixlabs/mxcli#1027. +> +> To check a value against your own project rather than trusting any list: +> +> ```bash +> sqlite3 .mxcli/catalog.db "SELECT DISTINCT ActionType FROM activities;" +> sqlite3 .mxcli/catalog.db "SELECT DISTINCT SourceType, TargetType, RefKind FROM refs;" +> ``` +> +> Absence from your project means the construct is not used there; a value absent +> from the tables below is one the catalog never produces anywhere. + ### entity | Property | Type | Example | |----------|------|---------| @@ -300,7 +327,7 @@ def count_not(node): | `entity_id` | string | Parent entity UUID | | `entity_qualified_name` | string | `"Sales.Customer"` | | `module_name` | string | `"Sales"` | -| `data_type` | string | `"string"`, `"integer"`, `"datetime"`, etc. | +| `data_type` | string | `"String"`, `"Integer"`, `"Long"`, `"Decimal"`, `"Boolean"`, `"DateTime"`, `"Date"`, `"Enumeration"`, `"AutoNumber"`, `"Binary"`, `"HashedString"` | | `length` | int | Field length (for strings) | | `is_unique` | bool | Has unique constraint | | `is_required` | bool | Is required | @@ -314,8 +341,8 @@ def count_not(node): | `id` | string | Activity UUID | | `name` | string | Activity name | | `caption` | string | Activity caption | -| `activity_type` | string | `"ActionActivity"`, `"ExclusiveSplit"`, `"LoopedActivity"`, etc. | -| `action_type` | string | `"CreateChangeAction"`, `"CommitAction"`, `"ShowFormAction"`, etc. | +| `activity_type` | string | `"ActionActivity"`, `"ExclusiveSplit"`, `"ExclusiveMerge"`, `"LoopedActivity"`, `"InheritanceSplit"`, `"StartEvent"`, `"EndEvent"` | +| `action_type` | string | The action inside an `ActionActivity`: `"CreateObjectAction"`, `"ChangeObjectAction"`, `"CommitObjectsAction"`, `"DeleteObjectAction"`, `"RetrieveAction"`, `"MicroflowCallAction"`, `"ShowPageAction"`, `"ClosePageAction"`, `"LogMessageAction"`, `"JavaActionCallAction"`. Empty for an activity that is not an action | | `microflow_id` | string | Parent microflow UUID | | `microflow_qualified_name` | string | `"Sales.ACT_Customer_Create"` | | `module_name` | string | `"Sales"` | @@ -328,11 +355,11 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific) | Property | Type | Example | |----------|------|---------| | `module_role_name` | string | `"Admin"` | -| `element_type` | string | `"entity"`, `"microflow"`, `"page"`, `"ODATA_SERVICE"` (from `permissions()` only) | +| `element_type` | string | `"ENTITY"`, `"MICROFLOW"`, `"PAGE"`, `"ODATA_SERVICE"` (from `permissions()` only) | | `element_name` | string | `"Sales.Customer"` | | `module_name` | string | `"Sales"` | | `entity_name` | string | `"Sales.Customer"` (from `permissions_for()` only) | -| `access_type` | string | `"create"`, `"read"`, `"write"`, `"delete"`, `"execute"`, `"view"`, `"access"`, `"MEMBER_READ"`, `"MEMBER_WRITE"` | +| `access_type` | string | `"CREATE"`, `"READ"`, `"WRITE"`, `"DELETE"` (entity), `"EXECUTE"` (microflow), `"VIEW"` (page), `"ACCESS"` (OData service), `"MEMBER_READ"`, `"MEMBER_WRITE"` | | `member_name` | string | Attribute name (for MEMBER_READ/MEMBER_WRITE) | | `xpath_constraint` | string | XPath constraint or empty | | `is_constrained` | bool | True if XPath constraint is set | @@ -361,13 +388,13 @@ Returned by `permissions()` (all types) or `permissions_for()` (entity-specific) ### reference | Property | Type | Example | |----------|------|---------| -| `source_type` | string | `"microflow"`, `"page"`, etc. | +| `source_type` | string | The document the edge comes FROM, upper-case: `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"SNIPPET"`, `"ENTITY"`, `"ASSOCIATION"`, `"WORKFLOW"`, `"NAVIGATION"`, `"SCHEDULED_EVENT"`, `"PUBLISHED_REST_OPERATION"`, `"PROJECT_SETTINGS"` | | `source_id` | string | Source UUID | | `source_name` | string | `"Sales.ACT_Customer_Create"` | -| `target_type` | string | `"entity"`, `"microflow"`, etc. | +| `target_type` | string | What it points AT, upper-case: `"ENTITY"`, `"ASSOCIATION"`, `"MICROFLOW"`, `"NANOFLOW"`, `"RULE"`, `"PAGE"`, `"LAYOUT"`, `"WORKFLOW"`, `"WIDGET"`, `"JAVA_ACTION"`, `"REST_OPERATION"`, `"REGULAR_EXPRESSION"`. `LAYOUT` and `WIDGET` are only ever targets; `SCHEDULED_EVENT` and `PROJECT_SETTINGS` only ever sources | | `target_id` | string | Target UUID | | `target_name` | string | `"Sales.Customer"` | -| `ref_kind` | string | Reference kind | +| `ref_kind` | string | How it references: `"call"`, `"create"`, `"retrieve"`, `"change"`, `"delete"`, `"show_page"`, `"datasource"`, `"action"`, `"layout"`, `"parameter"`, `"return"`, `"generalize"`, `"associate"`, `"home_page"`, `"login_page"`, `"menu_item"`, `"calculate"`, `"schedule"`, `"validate"`, `"settings"`, `"widget"`, `"sync"`, `"publish"`, `"event"` — lower-case, unlike the types above | | `module_name` | string | Source module | ### project_security diff --git a/mdl-examples/bug-tests/1027-lint-rule-vocabulary.mdl b/mdl-examples/bug-tests/1027-lint-rule-vocabulary.mdl new file mode 100644 index 000000000..8d274624b --- /dev/null +++ b/mdl-examples/bug-tests/1027-lint-rule-vocabulary.mdl @@ -0,0 +1,86 @@ +-- mendixlabs/mxcli#1027 — the bundled `write-lint-rules` skill documented +-- `action_type` / `source_type` values the lint API never returns, so a rule +-- written from the guide's own examples matched nothing and reported a clean +-- pass. Nothing warned that the filter never had a chance. +-- +-- REPORTED SHAPE. The guide's action_type row listed Mendix's BSON *storage* +-- names; the catalog labels an action with its *SDK* name (getMicroflowActionType +-- derives the label from the parsed Go type): +-- +-- Guide said Catalog emits +-- CreateChangeAction CreateObjectAction, ChangeObjectAction +-- CommitAction CommitObjectsAction +-- ShowFormAction ShowPageAction +-- CloseFormAction ClosePageAction +-- ShowHomeFormAction ShowHomePageAction +-- +-- and source_type was documented lower-case ("microflow", "page") against an +-- upper-case vocabulary. The same defect sat unreported in three sibling rows: +-- permission.element_type, permission.access_type and attribute.data_type. +-- +-- WHY THIS FILE IS A MODEL AND NOT A FAILING SCRIPT. The defect is in the +-- documentation, not in MDL, so there is no script that reproduces it. What the +-- repro needs is a project whose microflows exercise the disputed labels. This +-- is that project: ACT_Customer_Create uses exactly the actions the guide got +-- wrong, one of each. +-- +-- REPRODUCING (three legs — B alone proves nothing, because a rule that matches +-- nothing also reports zero): +-- +-- mxcli exec 1027-lint-rule-vocabulary.mdl -p app.mpr +-- mxcli -p app.mpr -c "refresh catalog full" +-- +-- then run a rule in .claude/lint-rules/ that allow-lists action_type values: +-- +-- A allowlist = the guide's old values -> 7 of 7 activities flagged (inverted) +-- B allowlist = the corrected values -> 0 flagged (correct) +-- C B minus "ShowPageAction" -> exactly ShowPageAction flagged +-- +-- C is the control: without it, B's silence is indistinguishable from the bug. +-- +-- The regression coverage is TestSkillDocumentsRealActionTypes and its siblings +-- in mdl/catalog/lint_rule_doc_vocabulary_test.go, which pin every documented +-- value to the function that produces it. + +create module Sales; + +create persistent entity Sales.Customer ( + Name: String(100), + Age: Integer +); + +create or replace layout Sales.App_Default ( + layouttype: 'Responsive' +) { + scrollcontainer layoutContainer { + region center { + placeholder Main + } + } +} + +create page Sales.Customer_Overview ( + Title: 'Customers', + Layout: Sales.App_Default +) { + container box { + } +} + +create or replace microflow Sales.SUB_Helper () +begin + log info 'helper'; +end; + +-- One activity per label the guide got wrong, so every disputed value appears in +-- CATALOG.ACTIVITIES.ActionType after a full refresh. +create or replace microflow Sales.ACT_Customer_Create () +begin + $cust = CREATE Sales.Customer (Name = 'x'); + CHANGE $cust (Age = 1); + COMMIT $cust WITH EVENTS; + $r = CALL MICROFLOW Sales.SUB_Helper(); + SHOW PAGE Sales.Customer_Overview; + CLOSE PAGE; + DELETE $cust; +end; diff --git a/mdl/catalog/lint_rule_doc_vocabulary_test.go b/mdl/catalog/lint_rule_doc_vocabulary_test.go new file mode 100644 index 000000000..5e68a44e2 --- /dev/null +++ b/mdl/catalog/lint_rule_doc_vocabulary_test.go @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: Apache-2.0 + +package catalog + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The bundled write-lint-rules skill is the ONLY documentation of the Starlark +// rule API, so its example tables are what a rule author copies. Nothing +// connected those tables to the values this package emits, and they drifted into +// fiction: `action_type` listed five Mendix *storage* names — CreateChangeAction, +// CommitAction, ShowFormAction, CloseFormAction, ShowHomeFormAction — that the +// catalog has never produced, and `source_type`, `target_type`, `element_type`, +// `access_type` and `data_type` were documented in the wrong case. +// +// Both failure modes are silent. A rule built from the documented values +// compiles, runs, matches nothing and reports a clean pass; nothing warns that +// the filter never had a chance. Measured on one real project, the action_type +// row inverted a shipped convention rule into 138 of 282 ACT_ microflows flagged, +// 49% false positives (mendixlabs/mxcli#1027), after which lint on that project +// was demoted to non-blocking and then stopped being run. +// +// mdl/catalog/lint_rule_vocabulary_test.go pins the bundled *rules* to these same +// vocabularies. This file pins the *documentation*, which is where the rule +// authors who are not in this repository get their values. + +func lintRuleSkillDoc(t *testing.T) string { + t.Helper() + path := filepath.Join("..", "..", ".claude", "skills", "mendix", "write-lint-rules", "SKILL.md") + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return string(b) +} + +// docRowValues returns the quoted example values documented for a field, from +// the Markdown table row whose first cell is `field`. Every field this file +// checks appears in exactly one row. +// +// It fails rather than returning nothing: a renamed row would otherwise make +// every assertion below vacuously true, which is the same silent pass the bug is. +func docRowValues(t *testing.T, doc, field string) []string { + t.Helper() + row := regexp.MustCompile(`(?m)^\| ` + "`" + regexp.QuoteMeta(field) + "`" + ` \|.*$`).FindString(doc) + if row == "" { + t.Fatalf("no table row for %q in write-lint-rules/SKILL.md — the row was renamed or removed, "+ + "which silently disables this check", field) + } + var out []string + for _, m := range regexp.MustCompile(`"([^"]*)"`).FindAllStringSubmatch(row, -1) { + out = append(out, m[1]) + } + if len(out) == 0 { + t.Fatalf("row for %q documents no example values: %s", field, row) + } + return out +} + +func assertDocumentedValuesExist(t *testing.T, field string, documented []string, real map[string]bool, produced string) { + t.Helper() + for _, v := range documented { + if !real[v] { + t.Errorf("write-lint-rules documents %s = %q, which %s never produces.\n"+ + "A rule filtering on it matches nothing and reports a clean pass.", field, v, produced) + } + } +} + +// microflowActionLabels is every label getMicroflowActionType can return for a +// modelled action. The label IS the Go type name (the function derives it with +// %T), so the authoritative set is the set of types implementing MicroflowAction +// — which is spelled in exactly one place: the isMicroflowAction marker methods. +// +// Read from source rather than hand-listed, because a hand-list beside a doc list +// is two copies of the same table and drifts the same way the doc did. +func microflowActionLabels(t *testing.T) map[string]bool { + t.Helper() + path := filepath.Join("..", "..", "sdk", "microflows", "microflows_actions.go") + file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", path, err) + } + + labels := map[string]bool{} + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name.Name != "isMicroflowAction" || fn.Recv == nil || len(fn.Recv.List) != 1 { + continue + } + typ := fn.Recv.List[0].Type + if star, ok := typ.(*ast.StarExpr); ok { + typ = star.X + } + if id, ok := typ.(*ast.Ident); ok { + labels[id.Name] = true + } + } + + // CONTROL: an AST walk that matched nothing would make every action_type + // assertion pass. sdk/microflows models dozens of actions. + if len(labels) < 40 { + t.Fatalf("found only %d MicroflowAction implementors in %s — the scan is broken, "+ + "not the documentation", len(labels), path) + } + // CONTROL: the labeller must agree with the scan on a known action, or the + // set is the right size and the wrong thing. + if got := getMicroflowActionType(µflows.ShowPageAction{}); !labels[got] { + t.Fatalf("getMicroflowActionType returns %q, which the scan did not collect", got) + } + return labels +} + +// TestSkillDocumentsRealActionTypes is the reported defect: every documented +// action_type was a storage name the catalog never emits. +func TestSkillDocumentsRealActionTypes(t *testing.T) { + doc := lintRuleSkillDoc(t) + assertDocumentedValuesExist(t, "action_type", + docRowValues(t, doc, "action_type"), + microflowActionLabels(t), + "getMicroflowActionType") +} + +// TestSkillDocumentsRealActivityTypes covers the sibling column. An activity_type +// label is derived the same way, from the MicroflowObject's Go type. +func TestSkillDocumentsRealActivityTypes(t *testing.T) { + doc := lintRuleSkillDoc(t) + real := map[string]bool{} + for _, obj := range []microflows.MicroflowObject{ + µflows.ActionActivity{}, µflows.ExclusiveSplit{}, µflows.ExclusiveMerge{}, + µflows.LoopedActivity{}, µflows.InheritanceSplit{}, + µflows.StartEvent{}, µflows.EndEvent{}, + } { + real[getMicroflowObjectType(obj)] = true + } + assertDocumentedValuesExist(t, "activity_type", + docRowValues(t, doc, "activity_type"), real, "getMicroflowObjectType") +} + +// TestSkillDocumentsRealRefObjectTypes covers the second half of the report: +// source_type and target_type were documented lower-case ("microflow", "page") +// and are emitted upper-case. +func TestSkillDocumentsRealRefObjectTypes(t *testing.T) { + doc := lintRuleSkillDoc(t) + + set := func(vals []string) map[string]bool { + m := map[string]bool{} + for _, v := range vals { + m[v] = true + } + return m + } + + assertDocumentedValuesExist(t, "source_type", + docRowValues(t, doc, "source_type"), set(RefSourceObjectTypes), "buildReferences") + assertDocumentedValuesExist(t, "target_type", + docRowValues(t, doc, "target_type"), set(RefTargetObjectTypes), "buildReferences") +} + +// TestSkillDocumentsRealPermissionVocabulary covers permission.element_type and +// permission.access_type, which had the same lower-cased examples. They were not +// in the report — they are the same defect in the same tables, found by checking +// the siblings rather than only the two rows that were named. +func TestSkillDocumentsRealPermissionVocabulary(t *testing.T) { + doc := lintRuleSkillDoc(t) + + set := func(vals []string) map[string]bool { + m := map[string]bool{} + for _, v := range vals { + m[v] = true + } + return m + } + + assertDocumentedValuesExist(t, "element_type", + docRowValues(t, doc, "element_type"), set(PermissionElementTypes), "buildPermissions") + assertDocumentedValuesExist(t, "access_type", + docRowValues(t, doc, "access_type"), set(PermissionAccessTypes), "buildPermissions") +} + +// TestSkillDocumentsRealAttributeDataTypes covers attribute.data_type, documented +// as "string"/"integer"/"datetime" and emitted as String/Integer/DateTime. This is +// the most-used filter in a lint rule, so it is the one whose silence costs most. +func TestSkillDocumentsRealAttributeDataTypes(t *testing.T) { + doc := lintRuleSkillDoc(t) + + real := map[string]bool{} + for _, at := range []domainmodel.AttributeType{ + &domainmodel.StringAttributeType{}, &domainmodel.IntegerAttributeType{}, + &domainmodel.LongAttributeType{}, &domainmodel.DecimalAttributeType{}, + &domainmodel.BooleanAttributeType{}, &domainmodel.DateTimeAttributeType{}, + &domainmodel.DateAttributeType{}, &domainmodel.EnumerationAttributeType{}, + &domainmodel.AutoNumberAttributeType{}, &domainmodel.BinaryAttributeType{}, + &domainmodel.HashedStringAttributeType{}, + } { + real[at.GetTypeName()] = true + } + + assertDocumentedValuesExist(t, "data_type", + docRowValues(t, doc, "data_type"), real, "AttributeType.GetTypeName") +} + +// TestSkillDocumentsRealRefKinds covers ref_kind, which documents no examples +// today. Whatever it documents must be a kind buildReferences emits. +func TestSkillDocumentsRealRefKinds(t *testing.T) { + doc := lintRuleSkillDoc(t) + + real := map[string]bool{} + for _, k := range []string{ + RefKindCall, RefKindCreate, RefKindRetrieve, RefKindShowPage, + RefKindGeneralize, RefKindAssociate, RefKindLayout, RefKindDatasource, + RefKindParameter, RefKindAction, RefKindHomePage, RefKindLoginPage, + RefKindMenuItem, RefKindChange, RefKindDelete, RefKindCalculate, + RefKindReturn, RefKindSchedule, RefKindValidate, RefKindSettings, + RefKindWidget, RefKindSync, RefKindPublish, RefKindEvent, + } { + real[k] = true + } + + assertDocumentedValuesExist(t, "ref_kind", + docRowValues(t, doc, "ref_kind"), real, "buildReferences") +} + +// TestEveryObjectTypeConstantIsInAPublishedList is the other half of the pin. +// The exported vocabularies are only authoritative if they are complete: a +// constant the builders emit but neither list names is a value no documentation +// can mention and no rule author can discover. +// +// Read from the declarations rather than hand-listed, so adding a constant +// without publishing it fails here instead of quietly narrowing the vocabulary. +func TestEveryObjectTypeConstantIsInAPublishedList(t *testing.T) { + for _, tc := range []struct { + file, prefix string + published []string + }{ + {"builder_references.go", "RefObject", append(append([]string{}, RefSourceObjectTypes...), RefTargetObjectTypes...)}, + {"builder_permissions.go", "PermissionElement", PermissionElementTypes}, + {"builder_permissions.go", "AccessType", PermissionAccessTypes}, + } { + t.Run(tc.prefix, func(t *testing.T) { + known := map[string]bool{} + for _, v := range tc.published { + known[v] = true + } + + declared := constantsWithPrefix(t, tc.file, tc.prefix) + // CONTROL: a scan that collected nothing would pass this test whatever + // the lists said. + if len(declared) < 4 { + t.Fatalf("found only %d %s* constants in %s — the scan is broken", len(declared), tc.prefix, tc.file) + } + for name, value := range declared { + if !known[value] { + t.Errorf("%s = %q is emitted but appears in no published vocabulary, "+ + "so nothing can document it", name, value) + } + } + }) + } +} + +// constantsWithPrefix returns the name→value pairs of the string constants in a +// file of this package whose name starts with prefix. +func constantsWithPrefix(t *testing.T, file, prefix string) map[string]string { + t.Helper() + parsed, err := parser.ParseFile(token.NewFileSet(), file, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", file, err) + } + + out := map[string]string{} + for _, decl := range parsed.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.CONST { + continue + } + for _, spec := range gen.Specs { + vs, ok := spec.(*ast.ValueSpec) + if !ok || len(vs.Names) != 1 || len(vs.Values) != 1 { + continue + } + name := vs.Names[0].Name + if !strings.HasPrefix(name, prefix) { + continue + } + lit, ok := vs.Values[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + continue + } + out[name] = strings.Trim(lit.Value, `"`) + } + } + return out +} From 728a30f19250b57553cbe6f5b1c25fede3152587 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 06:49:12 +0000 Subject: [PATCH 10/17] fix(check): resolve design properties on ALTER STYLING, and refuse a flat multi-select value Two ways a design property reached mxbuild instead of `mxcli check`. ALTER STYLING (ako/mxcli#509) was the one statement whose entire job is writing design properties, and the one statement MDL-WIDGET11 never looked at --- ValidateDesignPropertiesForStatement switches on CreatePageStmtV3, CreateSnippetStmtV3 and AlterPageStmt, and *ast.AlterStylingStmt is not among them. `set 'Remove empty text' = on` passed check, wrote, and failed the build with CE6083. The obvious fix was the wrong one. Mapping the stored $Type to a theme-registry key would be a THIRD consumer of one concept --- there is already mdlKeywordToDesignPropsKey, and an unused bsonTypeToDesignPropsKey with zero non-test callers, hence never validated --- and duplicate resolvers drifting apart is what this area keeps producing. So the pass asks the question that IS answerable without opening the document: does any widget type in the theme declare this key? A key declared nowhere cannot be right here either, which is the reported case exactly. A key declared for another widget type is accepted: under-reporting, never over-reporting, the only safe direction for a check that cannot see what it is judging. The declared set is built from every group, not one. Three of a List View's six properties come from the inherited `Widget` group, so a single-type lookup would report `Align self` as unknown. Multi-select (ako/mxcli#511) turned out not to be a missing capability. The reference document was already in the project: grep found three Studio Pro-authored Atlas pages carrying `Hide on`, and decoding one gives Forms$DesignPropertyValue Key: "Hide on" Value: Forms$CompoundDesignPropertyValue Properties: [ Forms$DesignPropertyValue{ Key: "Phone", Value: Forms$ToggleDesignPropertyValue } ] which is structurally `Spacing`, which MDL already writes. Measured: `DesignProperties: ['Hide on': ['Phone': on, 'Tablet': on]]` writes, round-trips through DESCRIBE and builds at 0 errors. Only the FLAT spelling was broken --- 'Phone' is a declared option, so it serialized as a plain Option and mxbuild refused it with CE6084. So the fix is a refusal naming the spelling that works, not a feature. `multiSelect` is now parsed (it was read nowhere), the inline path refuses a flat value, `show design properties` marks such a property instead of listing it identically to a single-select one, and both paths warn at check time. ALTER STYLING genuinely cannot express a compound --- a StylingAssignment carries one flat value and the grammar has no nesting --- so there it refuses and points at the inline form. Controls: `Align self` (same declared control type, not multi-select) still writes a plain option; the compound spelling still writes; a property the theme says nothing about is still written as asked; an empty or absent registry reports nothing at all. Both rule-ID guards --- TestWidgetRuleIDsAreNotReused and TestRuleIDHasOneOwner --- fail when a rule is raised from a second file. MDL-WIDGET11/12 are registered as shared rather than renumbered: someone suppressing MDL-WIDGET11 means both sites. Fixes ako/mxcli#509, ako/mxcli#511 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LYaTtzjmMCUjo8XAVN1sQx --- .../fix-issue/findings/mdl-executor.jsonl | 2 + ...09-511-alter-styling-design-properties.mdl | 74 +++++++ mdl/executor/cmd_pages_builder_v3.go | 63 +++++- mdl/executor/cmd_styling.go | 35 +++- mdl/executor/rule_id_uniqueness_test.go | 8 + mdl/executor/theme_reader.go | 27 ++- mdl/executor/validate_alter_styling.go | 198 ++++++++++++++++++ mdl/executor/validate_alter_styling_test.go | 118 +++++++++++ mdl/executor/validate_design_properties.go | 18 ++ ...lidate_multiselect_design_property_test.go | 143 +++++++++++++ mdl/executor/validate_program.go | 8 + mdl/executor/widget_rule_ids_test.go | 8 + 12 files changed, 696 insertions(+), 6 deletions(-) create mode 100644 mdl-examples/bug-tests/styling-509-511-alter-styling-design-properties.mdl create mode 100644 mdl/executor/validate_alter_styling.go create mode 100644 mdl/executor/validate_alter_styling_test.go create mode 100644 mdl/executor/validate_multiselect_design_property_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 63e69d24c..4183fe5be 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -642,3 +642,5 @@ {"area": "mdl/executor", "date": "2026-09-18", "symptom": "`mxcli check -p --references` reports MDL-WIDGET25 \"`htmlelement` / `attribute` / `tagcontentcontainer` is not a widget in this project\" on a page `describe page` had just emitted, while `exec --no-check` writes the same page without complaint. check is STRICTER than exec — the inversion check exists to prevent", "cause": "Two registries. The page builder (`pageBuilder.initPluggableEngine`) calls `RefreshStaleWidgetDefinitions` before `LoadUserDefinitions`, so exec generates `.mxcli/widgets/*.def.json` from the project's installed `.mpk` on its way past. `LoadWidgetRegistry` — used by check, lint and the LSP — called only `LoadUserDefinitions`, so on a project that had never run `mxcli widget init` the validator knew the nine embedded definitions and nothing else. The `pluggablewidget ''` branch of validateWidgetKind has `packageInstalledFor` as its escape hatch; the generic-MDL-name branch had none, and it also returns BEFORE the `parentDef == nil` silence guard written a few lines below for exactly this case, so an unresolvable parent's CHILDREN were reported as missing widgets", "file": "`mdl/executor/validate_widgets.go` (`LoadWidgetRegistry` now refreshes stale defs, best-effort); `mdl/backend/pagemutator/mutator.go` (`noPluggableObjectError`)", "insight": "**The bug self-heals, which is why it reads as flaky: the first `exec` writes the definitions and every `check` after it passes.** `rm -rf .mxcli/widgets` before reproducing or it is invisible — that alone cost more than the fix. The fixture `testdata/expr-checker/` is already in the reported state (HTMLElement.mpk installed, `.mxcli` gitignored), so a unit test needs only a temp dir with one 10 KB .mpk copied in; the registry never opens the .mpr, only `filepath.Dir(projectPath)`. Control that keeps the fix honest: a typo (`htmlelemnt`) in the SAME project must still be MDL-WIDGET25 — and now it can even suggest `htmlelement`, which it could not when the candidate list was the nine embedded widgets. **Second half of the same report, and the wrong turn to skip: do not take a reporter's 'these are design properties of X' on faith — check the theme.** 'Remove empty text' / 'Remove loadmore button' / 'Reset list style' are NOT ListView design properties in the Atlas shipped with Mendix 11. **Count them with `show design properties for listview`, never by reading the design-properties.json key**: a List View has SIX — Style/Hover style/Row size under `ListView`, plus Spacing/Align self/Hide on inherited from the `Widget` group that applies to every widget. Reading the raw `ListView` key alone says three, which is the mistake this session made and had to correct; `ThemeRegistry.GetPropertiesForWidget` already prepends the inherited group, so anything built on it is right and anything built on the JSON key rejects half the real properties. ALTER STYLING writes the reported key happily and mxbuild then fails **CE6083** 'Design property Remove empty text is not supported by your theme'; the same statement with 'Row size' = 'Small' is 0 errors. So refusing the write was right and only the REASON was wrong — which is why the fix is the message, not write support. Still open, tracked as ako/mxcli#509: MDL-WIDGET11 covers CREATE PAGE / CREATE SNIPPET / ALTER PAGE INSERT|REPLACE and not `ALTER STYLING`, whose widget type lives only in the stored document, so an unsupported key is silent until mxbuild says CE6083.", "refs": ["mendixlabs/mxcli#1135", "mendixlabs/mxcli#1069", "ako/mxcli#509"], "rules": ["MDL-WIDGET25", "MDL-WIDGET26"], "ce": ["CE6083"]} {"area":"mdl/executor","date":"2026-09-18","symptom":"`mxcli check` warns MDL-WIDGET20 that a List View's (and a grid column's) `Editable` is \"silently dropped on write and the widget stays enabled\". It is written: exec writes it, `describe page` reads back `Editable: true`, and mxbuild reports 0 errors on 11.12.2. The suggestion even reads \"buttons do support conditional visibility\"","cause":"Two different Mendix properties conflated under one MDL keyword. `editableWidgetTypes` is the set of Pages types carrying **Editability / ConditionalEditabilitySettings**, which is right for the bug the rule was written for (#928, `editable:` on a button). `Pages$ListView` and `Pages$GridColumn` carry neither — they have a plain `Editable bool`, a different property meaning \"make the inputs INSIDE me editable\" — so they fell through to the warning","file":"`mdl/executor/validate_widget_editability.go` (new `plainEditableWidgetTypes`); test `mdl/executor/widget_editable_plain_bool_test.go`","insight":"**A metamodel-sync test guards only the class it enumerates, and can lock a bug in.** `TestEditableWidgetTypesMatchMetamodel` keeps the list synced to the *Editability* set, so a type with a plain `Editable bool` and no Editability was invisible to it — the test passed throughout, and a correct fix would have made it fail if the two sets had been merged. The fix is a SECOND set with its own sibling test (`TestPlainEditableBoolTypesMatchMetamodel`), not more entries in the first. **Find the affected types by parsing generated/metamodel rather than by guessing**: scanning every `Pages*` struct for a plain `Editable bool` with no `Editability` returns exactly two (ListView, GridColumn) — the second one was not in the bug report and would have been missed. The control that keeps the fix narrow: neither type has `ConditionalEditabilitySettings`, so the BRACKET form `Editable: [expr]` (lowered to `EditableIf`) IS dropped and must keep warning — silencing both forms would re-create the worse half of #928, where the shape the docs recommend is dropped without a word. Worst-case cost of this false positive: `buildListViewV3`'s own comment records that a list view without `Editable` renders every input as `
` with entity access ReadWrite and `mx check` clean — so the warning steered authors away from the one property that fixes a symptom the code itself calls hard to diagnose","refs":["ako/mxcli#510","mendixlabs/mxcli#928"],"rules":["MDL-WIDGET20"]} {"area":"mdl/executor","date":"2026-09-18","symptom":"`template for ` passes `mxcli check`, is written by `exec`, and mxbuild then refuses the project with **CE0543** \"The entity of the list view template is 'X' and this is not a specialization of the entity of the list view\"","cause":"The guard existed and was one case too generous. Both call sites gated on `entityIsOrDescendsFrom(spec, listEntity)`, which returns true on its FIRST loop iteration when `spec == listEntity`. Mendix requires a STRICT specialization — the list view's own body already renders an object no template matches, so a template for the base entity is a second, unreachable default","file":"`mdl/executor/cmd_pages_builder_v3.go` (new `checkListViewTemplateSpecialization`), called from `cmd_pages_builder_v3_widgets.go` (CREATE) and `cmd_alter_page.go` (ALTER INSERT/REPLACE); `cmd/mxcli/syntax/features_page.go`","insight":"**The guard's own error message named the bug, and a test asserted it.** The wording was \" is not **or a specialization of it**\" — it offered the exact case Mendix refuses — and `TestBuildListViewTemplateOnTheListEntityItself` asserted that case was \"the base case Mendix permits\", justified by what `entityIsOrDescendsFrom` returns rather than by any measurement. So the belief was encoded three times (guard, message, test) and measured zero times; reading the code confirms itself. **Three rows separate the cases and none is redundant**: a real specialization (0 errors — the control against a guard that refuses everything), an unrelated entity (already refused, so the guard was not simply missing), and the list view's own entity (written, CE0543). Do not fix `entityIsOrDescendsFrom` in place: its other callers resolve association direction, where the reflexive case is correct. One shared method for both call sites, because the message and the rule were already duplicated and this is how those drift. Left open in ako/mxcli#514: the guard runs at exec, not check, so all three rows report `Check passed!` — a false green, though a safe one since nothing is written when it refuses","refs":["ako/mxcli#514"],"ce":["CE0543"]} +{"area":"mdl/executor","date":"2026-09-18","symptom":"`alter styling … set '' = ` passes `mxcli check`, writes, and mxbuild then fails **CE6083** \"Design property X is not supported by your theme\". MDL-WIDGET11/12 never ran on ALTER STYLING","cause":"`ValidateDesignPropertiesForStatement` switches on `CreatePageStmtV3`, `CreateSnippetStmtV3` and `AlterPageStmt` (its INSERT/REPLACE trees). `*ast.AlterStylingStmt` is not among them — the one statement whose entire job is writing design properties","file":"`mdl/executor/validate_alter_styling.go` (new), wired from `validate_program.go`","insight":"**The obvious fix — map the stored `$Type` to a theme-registry key — was the wrong one, and the repo already had the trap laid out.** `mdlKeywordToDesignPropsKey` maps MDL keyword → key and an UNUSED `bsonTypeToDesignPropsKey` maps `$Type` → key (zero non-test callers, so never validated); adding a third consumer of that concept is the drift `#1069`'s `buildPropKeyMap` records. Ask instead the question that is answerable WITHOUT the document: does any widget type in the theme declare this key? A key declared nowhere cannot be right here either, which is exactly the reported case, and a key declared for another type is accepted — under-reporting, never over-reporting, the only safe direction for a check that cannot see what it is judging. **Build the declared-key set from every group, not one**: three of a List View's six properties come from the inherited `Widget` group, so a single-type lookup reports `Align self` as unknown. Two separate rule-ID guards (`TestWidgetRuleIDsAreNotReused`, `TestRuleIDHasOneOwner`) both fail when a rule is raised from a second file — register the pair rather than renumbering, since a suppression of MDL-WIDGET11 means both sites","refs":["ako/mxcli#509"],"ce":["CE6083"],"rules":["MDL-WIDGET11"]} +{"area":"mdl/executor","date":"2026-09-18","symptom":"A design property declared `\"multiSelect\": true` (Atlas `Hide on`) written as a flat value — `'Hide on': 'Phone'` — passes check and exec and fails the build with **CE6084** \"Expected design property Hide on to be of type Toggle button group, but found Option\". The same statement with `Align self` (same declared control type) is 0 errors","cause":"`multiSelect` was parsed nowhere in mxcli — `ThemeProperty` had no such field. `resolveDesignPropertyValueType` saw 'Phone' in the declared option list and returned `option`, so the write produced a plain `Forms$OptionDesignPropertyValue` where Mendix stores a compound","file":"`mdl/executor/theme_reader.go` (`ThemeProperty.MultiSelect`, `parseDesignPropertiesJSON`); `cmd_pages_builder_v3.go` (`astDesignPropToValueChecked`); `cmd_styling.go`; `validate_design_properties.go`; `validate_alter_styling.go`","insight":"**The reference document was already in the project — do not ask for one before grepping `mprcontents/`.** `grep -rl 'Hide on' mprcontents/` found three Studio Pro-authored Atlas pages in a blank 11.12.2 app; decoding one gave the shape in minutes: `Forms$DesignPropertyValue{Key:'Hide on', Value: Forms$CompoundDesignPropertyValue{Properties:[ Forms$DesignPropertyValue{Key:'Phone', Value: Forms$ToggleDesignPropertyValue} ]}}`. **That shape is `Spacing`, which MDL already writes — so the capability was never missing and the issue as filed (\"implement multi-select\") was wrong.** `DesignProperties: ['Hide on': ['Phone': on, 'Tablet': on]]` writes, round-trips through DESCRIBE, and builds at 0 errors; only the FLAT spelling was broken. So the fix is a refusal that names the compound spelling, not a feature. Note ALTER STYLING genuinely cannot express it — a `StylingAssignment` carries one flat value and the grammar has no nesting — so there it refuses and points at the inline form rather than writing something wrong. **Check the reported thing against its nearest sibling before theorising**: `Align self` and `Hide on` are both ToggleButtonGroup, both inherited, both valued with a declared option — `multiSelect` is the only difference, and one isolated run named it","refs":["ako/mxcli#511"],"ce":["CE6084"],"rules":["MDL-WIDGET12"]} diff --git a/mdl-examples/bug-tests/styling-509-511-alter-styling-design-properties.mdl b/mdl-examples/bug-tests/styling-509-511-alter-styling-design-properties.mdl new file mode 100644 index 000000000..603c2abef --- /dev/null +++ b/mdl-examples/bug-tests/styling-509-511-alter-styling-design-properties.mdl @@ -0,0 +1,74 @@ +-- ako/mxcli#509 and #511 — two ways a design property reached mxbuild instead of +-- `mxcli check`. Both measured on a blank Mendix 11.12.2 project, plain Atlas. +-- +-- #509: ALTER STYLING was the one statement whose entire job is writing design +-- properties, and the one statement MDL-WIDGET11 never looked at. +-- +-- alter styling … set 'Remove empty text' = on; +-- check -> Check passed! +-- exec -> Updated styling on widget "lvThings" +-- build -> [CE6083] "Design property Remove empty text is not supported +-- by your theme." 1 error +-- +-- What the check can and cannot answer: the statement names a STORED widget, so +-- its $Type lives only in the document and this pass has no backend to open one. +-- It therefore asks whether ANY widget type in the theme declares the key — a key +-- declared nowhere cannot be right here either. A key declared for a DIFFERENT +-- widget type is accepted. Under-reporting, never over-reporting. +-- +-- #511: a design property declared `"multiSelect": true` (Atlas: 'Hide on') is +-- stored as a COMPOUND, one entry per selected option. Decoded from a Studio +-- Pro-authored Atlas page already in the project: +-- +-- Forms$DesignPropertyValue Key: "Hide on" +-- Value: Forms$CompoundDesignPropertyValue +-- Properties: [ Forms$DesignPropertyValue{ Key: "Phone", +-- Value: Forms$ToggleDesignPropertyValue } ] +-- +-- That is structurally `Spacing`, which MDL already writes — so the capability +-- was never missing. The FLAT spelling was the trap: 'Phone' is a declared +-- option, so it serialized as a plain Option and mxbuild refused it: +-- +-- [CE6084] "Expected design property Hide on to be of type Toggle button +-- group, but found Option." +-- +-- The same statement with 'Align self' (same declared control type, NOT +-- multi-select) is 0 errors — which is what isolates multiSelect as the cause. + +create entity MyFirstModule.Thing ( Name: string(200) ); + +-- #511 control: the compound spelling is the one that works. Verified 0 errors. +create or replace page MyFirstModule.HideOn ( + title: 'h', layout: 'Atlas_Core.Atlas_Default' +) { + listview lvH ( + DataSource: database from MyFirstModule.Thing, + DesignProperties: ['Hide on': ['Phone': on, 'Tablet': on]] + ) { + dynamictext dt (Content: 'x') + } +}; + +-- #509 control: a key the theme declares stays silent, and builds at 0 errors. +-- 'Row size' is type-specific to ListView; 'Align self' is inherited from the +-- `Widget` group, which a check built from one type's properties would wrongly +-- report — the mistake this area keeps producing. +alter styling on page MyFirstModule.HideOn widget lvH + set 'Row size' = 'Small', 'Align self' = 'Right'; + +-- The two failing shapes, kept commented so `make check-mdl` (which runs `check` +-- with no project, where there is no theme to judge against) stays green: +-- +-- alter styling on page MyFirstModule.HideOn widget lvH +-- set 'Remove empty text' = on; +-- -> MDL-WIDGET11: … no widget type in this project's theme declares +-- +-- DesignProperties: ['Hide on': 'Phone'] +-- -> MDL-WIDGET12: … takes a SET of options — mxbuild refuses that with CE6084 +-- -> exec: design property "Hide on" takes a SET of options, not one value — +-- write it as `'Hide on': ['Phone': on]` +-- +-- alter styling … set 'Hide on' = 'Phone'; +-- -> MDL-WIDGET12 + exec refusal: ALTER STYLING carries one flat value per +-- assignment and cannot express a compound at all, so it names the inline +-- form rather than pretending to write it. diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index e0c604b57..6a1e8d8d2 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -587,7 +587,15 @@ func applyWidgetAppearance(widget pages.Widget, w *ast.WidgetV3, theme *ThemeReg } var dpValues []pages.DesignPropertyValue for _, p := range astProps { - if dp, ok := astDesignPropToValue(p, themeProps); ok { + // Refuse rather than write, when the theme proves the shape wrong — + // a flat value on a multi-select property (ako/mxcli#511). Silently + // writing it produced a document mxbuild rejects with CE6084, whose + // wording names a type mismatch and not the spelling that fixes it. + dp, ok, err := astDesignPropToValueChecked(p, themeProps) + if err != nil { + return fmt.Errorf("widget %q: %w", w.Name, err) + } + if ok { dpValues = append(dpValues, dp) } } @@ -615,10 +623,61 @@ func applyWidgetAppearance(widget pages.Widget, w *ast.WidgetV3, theme *ThemeReg // the option default (findings: typed design properties). Without metadata the // prior syntactic behaviour (on→toggle, else→option) is preserved. func astDesignPropToValue(p ast.DesignPropertyEntryV3, themeProps []ThemeProperty) (pages.DesignPropertyValue, bool) { + dp, ok, _ := astDesignPropToValueChecked(p, themeProps) + return dp, ok +} + +// astDesignPropToValueChecked is astDesignPropToValue plus the one shape the +// theme can prove wrong: a FLAT value on a property declared `"multiSelect": true`. +// +// Such a property is a SET of the declared options, and Mendix stores it as a +// Forms$CompoundDesignPropertyValue holding one entry per selected option, each +// valued with a bare Forms$ToggleDesignPropertyValue — measured by decoding a +// Studio Pro-authored Atlas page in a blank 11.12.2 project. Structurally that is +// `Spacing`, which MDL already writes, so the capability is not missing: the +// compound spelling works, round-trips through DESCRIBE, and builds at 0 errors. +// +// The flat spelling is the trap. `'Hide on': 'Phone'` names a declared option, so +// resolveDesignPropertyValueType returned "option" and the write produced a +// document mxbuild refuses: +// +// [CE6084] "Expected design property Hide on to be of type Toggle button group, +// but found Option." +// +// Refusing it and naming the spelling that works is the fix; the author cannot +// derive `['Phone': on]` from CE6084's wording (ako/mxcli#511). +func astDesignPropToValueChecked(p ast.DesignPropertyEntryV3, themeProps []ThemeProperty) (pages.DesignPropertyValue, bool, error) { + if len(p.Nested) == 0 && p.Value != "" && isMultiSelectDesignProperty(p.Key, themeProps) { + return pages.DesignPropertyValue{}, false, mdlerrors.NewValidation(fmt.Sprintf( + "design property %q takes a SET of options, not one value — write it as "+ + "`'%s': ['%s': on]` (add one `'